public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Apply fixme notes for multi-target support
@ 2022-01-13 15:21 Christina Schimpe
  2022-01-13 15:21 ` [PATCH 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Christina Schimpe @ 2022-01-13 15:21 UTC (permalink / raw)
  To: gdb-patches

Hi all, 

this is a set of patches that apply the fixme notes described in commit 5b6d1e4
"Multi-target support".

"You'll notice that remote.c includes some FIXME notes.  These refer to
the fact that the global arrays that hold data for the remote packets
supported are still globals.  For example, if we connect to two
different servers/stubs, then each might support different remote
protocol features.  They might even be different architectures, like
e.g., one ARM baremetal stub, and a x86 gdbserver, to debug a
host/controller scenario as a single program.  That isn't going to
work correctly today, because of said globals.  I'm leaving fixing
that for another pass, since it does not appear to be trivial, and I'd
rather land the base work first.  It's already useful to be able to
debug multiple instances of the same server (e.g., a distributed
cluster, where you have full control over the servers installed), so I
think as is it's already reasonable incremental progress."

Due to the global array and variables used by all remote targets, the command
line configuration for remote targets' features before this set of patches
affected existing remote targets and future remote targets.  It was not
possible to individually configure a remote target's feature.
This behavior is now different.  With this patch series, the currently selected
remote target's as well as a future remote target's feature set can be
configured, without affecting the feature sets of the existing remote targets.

Any feedback about this series of patches is appreciated.

Regards,
Christina

Christina Schimpe (3):
  gdb: Make global feature array a per-remote target array
  gdb: Add per-remote target variables for memory read and write config
  gdb: Remove workaround for the vCont packet

 gdb/remote.c                                  | 1004 +++++++++--------
 gdb/testsuite/gdb.base/remote.exp             |    7 +-
 .../gdb.multi/multi-target-info-inferiors.exp |    6 +-
 gdb/testsuite/gdb.multi/multi-target.exp.tcl  |    7 +-
 4 files changed, 570 insertions(+), 454 deletions(-)

-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] gdb: Make global feature array a per-remote target array
  2022-01-13 15:21 [PATCH 0/3] Apply fixme notes for multi-target support Christina Schimpe
@ 2022-01-13 15:21 ` Christina Schimpe
  2022-01-14 19:30   ` Tom Tromey
  2022-01-18 11:30   ` Andrew Burgess
  2022-01-13 15:21 ` [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe
  2022-01-13 15:21 ` [PATCH 3/3] gdb: Remove workaround for the vCont packet Christina Schimpe
  2 siblings, 2 replies; 14+ messages in thread
From: Christina Schimpe @ 2022-01-13 15:21 UTC (permalink / raw)
  To: gdb-patches

This patch applies the appropriate FIXME notes described in commit
5b6d1e4 "Multi-target support".

"You'll notice that remote.c includes some FIXME notes.  These refer to
the fact that the global arrays that hold data for the remote packets
supported are still globals.  For example, if we connect to two
different servers/stubs, then each might support different remote
protocol features.  They might even be different architectures, like
e.g., one ARM baremetal stub, and a x86 gdbserver, to debug a
host/controller scenario as a single program.  That isn't going to
work correctly today, because of said globals.  I'm leaving fixing
that for another pass, since it does not appear to be trivial, and I'd
rather land the base work first.  It's already useful to be able to
debug multiple instances of the same server (e.g., a distributed
cluster, where you have full control over the servers installed), so I
think as is it's already reasonable incremental progress."

Using this patch it is possible to configure per-remote targets'
feature packets.

Given the following setup for two gdbservers:

~~~~
gdbserver --multi :1234
gdbserver --disable-packet=vCont --multi :2345
~~~~

Before this patch configuring of range-stepping was not possible for one
of two connected remote targets with different support for the vCont
packet.  As one of the target supports vCont, it should be possible to
configure "set range-stepping".  However, the output of GDB looks like:

(gdb) target extended-remote :1234
Remote debugging using :1234
(gdb) add-inferior -no-connection
[New inferior 2]
Added inferior 2
(gdb) inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) target extended-remote :2345
Remote debugging using :2345
(gdb) set range-stepping
warning: Range stepping is not supported by the current target
(gdb) inferior 1
[Switching to inferior 1 [<null>] (<noexec>)]
(gdb) set range-stepping
warning: Range stepping is not supported by the current target
~~~~

Two warnings are shown.  The warning for inferior 1 should not appear
as it is connected to a target supporting the vCont package.

~~~~
(gdb) target extended-remote :1234
Remote debugging using :1234
(gdb) add-inferior -no-connection
[New inferior 2]
Added inferior 2
(gdb) inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) target extended-remote :2345
Remote debugging using :2345
(gdb) set range-stepping
warning: Range stepping is not supported by the current target
(gdb) inferior 1
[Switching to inferior 1 [<null>] (<noexec>)]
(gdb) set range-stepping
(gdb)
~~~~

Now only one warning is shown for inferior 2, which is connected to
a target not supporting vCont.

The per-remote target feature array is realized by a new class
remote_features, which stores the per-remote target array and
provides functions to determine supported features of the target.
A remote_target object now has a new member of that class.

Each time a new remote_target object is initialized, a new per-remote
target array is constructed based on the global remote_protocol_packets
array.  The global array is initialized in the function _initialize_remote
and can be configured using the command line.  However, the command line
configuration before this patch affected also existing remote connections
(due to the global feature array used by all remote targets).  This
behavior is now different.  Now the currently selected target's feature
array will be configured and future connections' feature packets.  All
other existing remote targets' features are not affected.
The show command always displays the current remote target's
configuration.  If no remote target is selected the default
configuration for future connections is shown.

If we have for instance the following setup with inferior 2 being
selected:
~~~~
(gdb) info inferiors
  Num  Description       Connection                Executable
  1    <null>             1 (extended-remote :1234)
* 2    <null>             2 (extended-remote :2345)
~~~~

Before this patch, if we run 'set remote multiprocess-feature-packet', the
following configuration was set:
The feature array of all remote targets (in this setup the two connected
targets) and all future remote connections are affected.

After this patch, it will be configured as follows:
The feature array of target with port :2345 which is currently selected
and all future remote connections are affected. The show command 'show
remote multiprocess-feature-packet' will display the configuration of
target with port :2345.

It is therefore required to adapt the test
"gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp" to configure the
multiprocess-feature-packet before the connections are created.
---
 gdb/remote.c                                  | 899 ++++++++++--------
 .../gdb.multi/multi-target-info-inferiors.exp |   6 +-
 gdb/testsuite/gdb.multi/multi-target.exp.tcl  |   7 +-
 3 files changed, 511 insertions(+), 401 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index b126532af4..10f9226827 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -71,7 +71,6 @@
 #include "gdbsupport/agent.h"
 #include "btrace.h"
 #include "record-btrace.h"
-#include <algorithm>
 #include "gdbsupport/scoped_restore.h"
 #include "gdbsupport/environ.h"
 #include "gdbsupport/byte-vector.h"
@@ -127,6 +126,153 @@ enum packet_result
   PACKET_UNKNOWN
 };
 
+enum {
+  PACKET_vCont = 0,
+  PACKET_X,
+  PACKET_qSymbol,
+  PACKET_P,
+  PACKET_p,
+  PACKET_Z0,
+  PACKET_Z1,
+  PACKET_Z2,
+  PACKET_Z3,
+  PACKET_Z4,
+  PACKET_vFile_setfs,
+  PACKET_vFile_open,
+  PACKET_vFile_pread,
+  PACKET_vFile_pwrite,
+  PACKET_vFile_close,
+  PACKET_vFile_unlink,
+  PACKET_vFile_readlink,
+  PACKET_vFile_fstat,
+  PACKET_qXfer_auxv,
+  PACKET_qXfer_features,
+  PACKET_qXfer_exec_file,
+  PACKET_qXfer_libraries,
+  PACKET_qXfer_libraries_svr4,
+  PACKET_qXfer_memory_map,
+  PACKET_qXfer_osdata,
+  PACKET_qXfer_threads,
+  PACKET_qXfer_statictrace_read,
+  PACKET_qXfer_traceframe_info,
+  PACKET_qXfer_uib,
+  PACKET_qGetTIBAddr,
+  PACKET_qGetTLSAddr,
+  PACKET_qSupported,
+  PACKET_qTStatus,
+  PACKET_QPassSignals,
+  PACKET_QCatchSyscalls,
+  PACKET_QProgramSignals,
+  PACKET_QSetWorkingDir,
+  PACKET_QStartupWithShell,
+  PACKET_QEnvironmentHexEncoded,
+  PACKET_QEnvironmentReset,
+  PACKET_QEnvironmentUnset,
+  PACKET_qCRC,
+  PACKET_qSearch_memory,
+  PACKET_vAttach,
+  PACKET_vRun,
+  PACKET_QStartNoAckMode,
+  PACKET_vKill,
+  PACKET_qXfer_siginfo_read,
+  PACKET_qXfer_siginfo_write,
+  PACKET_qAttached,
+
+  /* Support for conditional tracepoints.  */
+  PACKET_ConditionalTracepoints,
+
+  /* Support for target-side breakpoint conditions.  */
+  PACKET_ConditionalBreakpoints,
+
+  /* Support for target-side breakpoint commands.  */
+  PACKET_BreakpointCommands,
+
+  /* Support for fast tracepoints.  */
+  PACKET_FastTracepoints,
+
+  /* Support for static tracepoints.  */
+  PACKET_StaticTracepoints,
+
+  /* Support for installing tracepoints while a trace experiment is
+     running.  */
+  PACKET_InstallInTrace,
+
+  PACKET_bc,
+  PACKET_bs,
+  PACKET_TracepointSource,
+  PACKET_QAllow,
+  PACKET_qXfer_fdpic,
+  PACKET_QDisableRandomization,
+  PACKET_QAgent,
+  PACKET_QTBuffer_size,
+  PACKET_Qbtrace_off,
+  PACKET_Qbtrace_bts,
+  PACKET_Qbtrace_pt,
+  PACKET_qXfer_btrace,
+
+  /* Support for the QNonStop packet.  */
+  PACKET_QNonStop,
+
+  /* Support for the QThreadEvents packet.  */
+  PACKET_QThreadEvents,
+
+  /* Support for multi-process extensions.  */
+  PACKET_multiprocess_feature,
+
+  /* Support for enabling and disabling tracepoints while a trace
+     experiment is running.  */
+  PACKET_EnableDisableTracepoints_feature,
+
+  /* Support for collecting strings using the tracenz bytecode.  */
+  PACKET_tracenz_feature,
+
+  /* Support for continuing to run a trace experiment while GDB is
+     disconnected.  */
+  PACKET_DisconnectedTracing_feature,
+
+  /* Support for qXfer:libraries-svr4:read with a non-empty annex.  */
+  PACKET_augmented_libraries_svr4_read_feature,
+
+  /* Support for the qXfer:btrace-conf:read packet.  */
+  PACKET_qXfer_btrace_conf,
+
+  /* Support for the Qbtrace-conf:bts:size packet.  */
+  PACKET_Qbtrace_conf_bts_size,
+
+  /* Support for swbreak+ feature.  */
+  PACKET_swbreak_feature,
+
+  /* Support for hwbreak+ feature.  */
+  PACKET_hwbreak_feature,
+
+  /* Support for fork events.  */
+  PACKET_fork_event_feature,
+
+  /* Support for vfork events.  */
+  PACKET_vfork_event_feature,
+
+  /* Support for the Qbtrace-conf:pt:size packet.  */
+  PACKET_Qbtrace_conf_pt_size,
+
+  /* Support for exec events.  */
+  PACKET_exec_event_feature,
+
+  /* Support for query supported vCont actions.  */
+  PACKET_vContSupported,
+
+  /* Support remote CTRL-C.  */
+  PACKET_vCtrlC,
+
+  /* Support TARGET_WAITKIND_NO_RESUMED.  */
+  PACKET_no_resumed,
+
+  /* Support for memory tagging, allocation tag fetch/store
+     packets and the tag violation stop replies.  */
+  PACKET_memory_tagging_feature,
+
+  PACKET_MAX
+};
+
 struct threads_listing_context;
 
 /* Stub vCont actions support.
@@ -395,6 +541,59 @@ static const target_info remote_target_info = {
   remote_doc
 };
 
+struct packet_config
+  {
+    const char *name;
+    const char *title;
+
+    /* If auto, GDB auto-detects support for this packet or feature,
+       either through qSupported, or by trying the packet and looking
+       at the response.  If true, GDB assumes the target supports this
+       packet.  If false, the packet is disabled.  Configs that don't
+       have an associated command always have this set to auto.  */
+    enum auto_boolean detect;
+
+    /* The "show remote foo-packet" command created for this packet.  */
+    cmd_list_element *show_cmd;
+
+    /* The "set remote foo-packet" command created for this packet.  */
+    cmd_list_element *set_cmd;
+
+    /* Does the target support this packet?  */
+    enum packet_support support;
+  };
+
+/* This global array contains the default configuration for every new
+   per-remote target array.  */
+static packet_config remote_protocol_packets[PACKET_MAX];
+
+class remote_features
+{
+public:
+
+  remote_features ()
+  {
+    std::copy (std::begin (remote_protocol_packets),
+	       std::end (remote_protocol_packets),
+	       std::begin (m_protocol_packets));
+  }
+  ~remote_features () = default;
+
+  DISABLE_COPY_AND_ASSIGN (remote_features);
+
+  enum packet_support packet_support (int) const;
+  enum auto_boolean packet_set_cmd_state (int) const;
+  int remote_multi_process_p (remote_state *rs) const;
+  int remote_fork_event_p (remote_state*) const;
+  int remote_vfork_event_p (remote_state*) const;
+  int remote_exec_event_p (remote_state*) const;
+  bool remote_memory_tagging_p () const;
+
+  void reset_all_packet_configs_support (void);
+
+  packet_config m_protocol_packets[PACKET_MAX];
+};
+
 class remote_target : public process_stratum_target
 {
 public:
@@ -585,8 +784,16 @@ class remote_target : public process_stratum_target
 
   bool supports_string_tracing () override;
 
+  int remote_supports_cond_tracepoints ();
+
   bool supports_evaluation_of_breakpoint_conditions () override;
 
+  int remote_supports_fast_tracepoints ();
+
+  int remote_supports_static_tracepoints ();
+
+  int remote_supports_install_in_trace ();
+
   bool can_run_breakpoint_commands () override;
 
   void trace_init () override;
@@ -946,6 +1153,8 @@ class remote_target : public process_stratum_target
 
   bool vcont_r_supported ();
 
+  remote_features m_features;
+
 private:
 
   bool start_remote_1 (int from_tty, int extended_p);
@@ -1061,7 +1270,8 @@ static CORE_ADDR remote_address_masked (CORE_ADDR);
 
 static int stub_unpack_int (const char *buff, int fieldlength);
 
-struct packet_config;
+static void show_packet_config_cmd (ui_file *file,
+				    struct packet_config *config);
 
 static void show_remote_protocol_packet_cmd (struct ui_file *file,
 					     int from_tty,
@@ -1884,29 +2094,31 @@ remote_target::get_memory_read_packet_size ()
   return size;
 }
 
-\f
+static enum packet_support packet_config_support
+  (const packet_config *config);
 
-struct packet_config
-  {
-    const char *name;
-    const char *title;
-
-    /* If auto, GDB auto-detects support for this packet or feature,
-       either through qSupported, or by trying the packet and looking
-       at the response.  If true, GDB assumes the target supports this
-       packet.  If false, the packet is disabled.  Configs that don't
-       have an associated command always have this set to auto.  */
-    enum auto_boolean detect;
-
-    /* The "show remote foo-packet" command created for this packet.  */
-    cmd_list_element *show_cmd;
 
-    /* Does the target support this packet?  */
-    enum packet_support support;
-  };
+static void
+set_remote_protocol_packet_cmd (const char *args, int from_tty,
+				cmd_list_element *c)
+{
+  remote_target *remote = get_current_remote_target ();
+  gdb_assert (c->var.has_value ());
 
-static enum packet_support packet_config_support (struct packet_config *config);
-static enum packet_support packet_support (int packet);
+   for (int i = 0; i < PACKET_MAX; ++i)
+    {
+       if (c == remote_protocol_packets[i].set_cmd)
+	 {
+	    enum auto_boolean value = c->var->get<enum auto_boolean> ();
+	    if (remote != nullptr)
+	      remote->m_features.m_protocol_packets[i].detect = value;
+	    remote_protocol_packets[i].detect = value;
+	    return;
+	 }
+    }
+  internal_error (__FILE__, __LINE__, _("Could not find config for %s"),
+		  c->name);
+}
 
 static void
 show_packet_config_cmd (ui_file *file, struct packet_config *config)
@@ -1960,10 +2172,11 @@ add_packet_config_cmd (struct packet_config *config, const char *name,
     = add_setshow_auto_boolean_cmd (cmd_name.release (), class_obscure,
 				    &config->detect, set_doc.get (),
 				    show_doc.get (), NULL, /* help_doc */
-				    NULL,
+				    set_remote_protocol_packet_cmd,
 				    show_remote_protocol_packet_cmd,
 				    &remote_set_cmdlist, &remote_show_cmdlist);
   config->show_cmd = cmds.show;
+  config->set_cmd = cmds.set;
 
   /* set/show remote NAME-packet {auto,on,off} -- legacy.  */
   if (legacy)
@@ -2067,165 +2280,13 @@ packet_ok (const gdb::char_vector &buf, struct packet_config *config)
   return packet_ok (buf.data (), config);
 }
 
-enum {
-  PACKET_vCont = 0,
-  PACKET_X,
-  PACKET_qSymbol,
-  PACKET_P,
-  PACKET_p,
-  PACKET_Z0,
-  PACKET_Z1,
-  PACKET_Z2,
-  PACKET_Z3,
-  PACKET_Z4,
-  PACKET_vFile_setfs,
-  PACKET_vFile_open,
-  PACKET_vFile_pread,
-  PACKET_vFile_pwrite,
-  PACKET_vFile_close,
-  PACKET_vFile_unlink,
-  PACKET_vFile_readlink,
-  PACKET_vFile_fstat,
-  PACKET_qXfer_auxv,
-  PACKET_qXfer_features,
-  PACKET_qXfer_exec_file,
-  PACKET_qXfer_libraries,
-  PACKET_qXfer_libraries_svr4,
-  PACKET_qXfer_memory_map,
-  PACKET_qXfer_osdata,
-  PACKET_qXfer_threads,
-  PACKET_qXfer_statictrace_read,
-  PACKET_qXfer_traceframe_info,
-  PACKET_qXfer_uib,
-  PACKET_qGetTIBAddr,
-  PACKET_qGetTLSAddr,
-  PACKET_qSupported,
-  PACKET_qTStatus,
-  PACKET_QPassSignals,
-  PACKET_QCatchSyscalls,
-  PACKET_QProgramSignals,
-  PACKET_QSetWorkingDir,
-  PACKET_QStartupWithShell,
-  PACKET_QEnvironmentHexEncoded,
-  PACKET_QEnvironmentReset,
-  PACKET_QEnvironmentUnset,
-  PACKET_qCRC,
-  PACKET_qSearch_memory,
-  PACKET_vAttach,
-  PACKET_vRun,
-  PACKET_QStartNoAckMode,
-  PACKET_vKill,
-  PACKET_qXfer_siginfo_read,
-  PACKET_qXfer_siginfo_write,
-  PACKET_qAttached,
-
-  /* Support for conditional tracepoints.  */
-  PACKET_ConditionalTracepoints,
-
-  /* Support for target-side breakpoint conditions.  */
-  PACKET_ConditionalBreakpoints,
-
-  /* Support for target-side breakpoint commands.  */
-  PACKET_BreakpointCommands,
-
-  /* Support for fast tracepoints.  */
-  PACKET_FastTracepoints,
-
-  /* Support for static tracepoints.  */
-  PACKET_StaticTracepoints,
-
-  /* Support for installing tracepoints while a trace experiment is
-     running.  */
-  PACKET_InstallInTrace,
-
-  PACKET_bc,
-  PACKET_bs,
-  PACKET_TracepointSource,
-  PACKET_QAllow,
-  PACKET_qXfer_fdpic,
-  PACKET_QDisableRandomization,
-  PACKET_QAgent,
-  PACKET_QTBuffer_size,
-  PACKET_Qbtrace_off,
-  PACKET_Qbtrace_bts,
-  PACKET_Qbtrace_pt,
-  PACKET_qXfer_btrace,
-
-  /* Support for the QNonStop packet.  */
-  PACKET_QNonStop,
-
-  /* Support for the QThreadEvents packet.  */
-  PACKET_QThreadEvents,
-
-  /* Support for multi-process extensions.  */
-  PACKET_multiprocess_feature,
-
-  /* Support for enabling and disabling tracepoints while a trace
-     experiment is running.  */
-  PACKET_EnableDisableTracepoints_feature,
-
-  /* Support for collecting strings using the tracenz bytecode.  */
-  PACKET_tracenz_feature,
-
-  /* Support for continuing to run a trace experiment while GDB is
-     disconnected.  */
-  PACKET_DisconnectedTracing_feature,
-
-  /* Support for qXfer:libraries-svr4:read with a non-empty annex.  */
-  PACKET_augmented_libraries_svr4_read_feature,
-
-  /* Support for the qXfer:btrace-conf:read packet.  */
-  PACKET_qXfer_btrace_conf,
-
-  /* Support for the Qbtrace-conf:bts:size packet.  */
-  PACKET_Qbtrace_conf_bts_size,
-
-  /* Support for swbreak+ feature.  */
-  PACKET_swbreak_feature,
-
-  /* Support for hwbreak+ feature.  */
-  PACKET_hwbreak_feature,
-
-  /* Support for fork events.  */
-  PACKET_fork_event_feature,
-
-  /* Support for vfork events.  */
-  PACKET_vfork_event_feature,
-
-  /* Support for the Qbtrace-conf:pt:size packet.  */
-  PACKET_Qbtrace_conf_pt_size,
-
-  /* Support for exec events.  */
-  PACKET_exec_event_feature,
-
-  /* Support for query supported vCont actions.  */
-  PACKET_vContSupported,
-
-  /* Support remote CTRL-C.  */
-  PACKET_vCtrlC,
-
-  /* Support TARGET_WAITKIND_NO_RESUMED.  */
-  PACKET_no_resumed,
-
-  /* Support for memory tagging, allocation tag fetch/store
-     packets and the tag violation stop replies.  */
-  PACKET_memory_tagging_feature,
-
-  PACKET_MAX
-};
-
-/* FIXME: needs to be per-remote-target.  Ignoring this for now,
-   assuming all remote targets are the same server (thus all support
-   the same packets).  */
-static struct packet_config remote_protocol_packets[PACKET_MAX];
-
 /* Returns the packet's corresponding "set remote foo-packet" command
    state.  See struct packet_config for more details.  */
 
-static enum auto_boolean
-packet_set_cmd_state (int packet)
+enum auto_boolean
+remote_features::packet_set_cmd_state (int packet) const
 {
-  return remote_protocol_packets[packet].detect;
+  return m_protocol_packets[packet].detect;
 }
 
 /* Returns whether a given packet or feature is supported.  This takes
@@ -2233,7 +2294,7 @@ packet_set_cmd_state (int packet)
    command, which may be used to bypass auto-detection.  */
 
 static enum packet_support
-packet_config_support (struct packet_config *config)
+packet_config_support (const packet_config *config)
 {
   switch (config->detect)
     {
@@ -2251,10 +2312,10 @@ packet_config_support (struct packet_config *config)
 /* Same as packet_config_support, but takes the packet's enum value as
    argument.  */
 
-static enum packet_support
-packet_support (int packet)
+enum packet_support
+remote_features::packet_support (int packet) const
 {
-  struct packet_config *config = &remote_protocol_packets[packet];
+  const packet_config *config = &m_protocol_packets[packet];
 
   return packet_config_support (config);
 }
@@ -2264,16 +2325,18 @@ show_remote_protocol_packet_cmd (struct ui_file *file, int from_tty,
 				 struct cmd_list_element *c,
 				 const char *value)
 {
-  struct packet_config *packet;
+  remote_target *remote = get_current_remote_target ();
   gdb_assert (c->var.has_value ());
 
-  for (packet = remote_protocol_packets;
-       packet < &remote_protocol_packets[PACKET_MAX];
-       packet++)
+   for (int i = 0; i < PACKET_MAX; ++i)
     {
-      if (c == packet->show_cmd)
+      if (c == remote_protocol_packets[i].show_cmd)
 	{
-	  show_packet_config_cmd (file, packet);
+	  if (remote != nullptr)
+	    show_packet_config_cmd
+	      (file, &remote->m_features.m_protocol_packets[i]);
+	  else
+	    show_packet_config_cmd (file, &remote_protocol_packets[i]);
 	  return;
 	}
     }
@@ -2302,10 +2365,16 @@ static void
 set_remote_protocol_Z_packet_cmd (const char *args, int from_tty,
 				  struct cmd_list_element *c)
 {
+  remote_target *remote = get_current_remote_target ();
   int i;
 
   for (i = 0; i < NR_Z_PACKET_TYPES; i++)
-    remote_protocol_packets[PACKET_Z0 + i].detect = remote_Z_packet_detect;
+    {
+      if (remote != nullptr)
+	remote->m_features.m_protocol_packets[PACKET_Z0 + i].detect
+	  = remote_Z_packet_detect;
+      remote_protocol_packets[PACKET_Z0 + i].detect = remote_Z_packet_detect;
+    }
 }
 
 static void
@@ -2313,50 +2382,55 @@ show_remote_protocol_Z_packet_cmd (struct ui_file *file, int from_tty,
 				   struct cmd_list_element *c,
 				   const char *value)
 {
+  remote_target *remote = get_current_remote_target ();
   int i;
 
   for (i = 0; i < NR_Z_PACKET_TYPES; i++)
     {
-      show_packet_config_cmd (file, &remote_protocol_packets[PACKET_Z0 + i]);
+      if (remote != nullptr)
+	show_packet_config_cmd
+	  (file, &remote->m_features.m_protocol_packets[PACKET_Z0 + i]);
+      else
+	show_packet_config_cmd (file, &remote_protocol_packets[PACKET_Z0 + i]);
     }
 }
 
 /* Returns true if the multi-process extensions are in effect.  */
 
-static int
-remote_multi_process_p (struct remote_state *rs)
+int
+remote_features::remote_multi_process_p (remote_state *rs) const
 {
   return packet_support (PACKET_multiprocess_feature) == PACKET_ENABLE;
 }
 
 /* Returns true if fork events are supported.  */
 
-static int
-remote_fork_event_p (struct remote_state *rs)
+int
+remote_features::remote_fork_event_p (remote_state *rs) const
 {
   return packet_support (PACKET_fork_event_feature) == PACKET_ENABLE;
 }
 
 /* Returns true if vfork events are supported.  */
 
-static int
-remote_vfork_event_p (struct remote_state *rs)
+int
+remote_features::remote_vfork_event_p (remote_state *rs) const
 {
   return packet_support (PACKET_vfork_event_feature) == PACKET_ENABLE;
 }
 
 /* Returns true if exec events are supported.  */
 
-static int
-remote_exec_event_p (struct remote_state *rs)
+int
+remote_features::remote_exec_event_p (remote_state *rs) const
 {
   return packet_support (PACKET_exec_event_feature) == PACKET_ENABLE;
 }
 
 /* Returns true if memory tagging is supported, false otherwise.  */
 
-static bool
-remote_memory_tagging_p ()
+bool
+remote_features::remote_memory_tagging_p () const
 {
   return packet_support (PACKET_memory_tagging_feature) == PACKET_ENABLE;
 }
@@ -2367,9 +2441,9 @@ remote_memory_tagging_p ()
 int
 remote_target::insert_fork_catchpoint (int pid)
 {
-  struct remote_state *rs = get_remote_state ();
+  remote_state *rs = get_remote_state ();
 
-  return !remote_fork_event_p (rs);
+  return !m_features.remote_fork_event_p (rs);
 }
 
 /* Remove fork catchpoint target routine.  Nothing to do, just
@@ -2387,9 +2461,9 @@ remote_target::remove_fork_catchpoint (int pid)
 int
 remote_target::insert_vfork_catchpoint (int pid)
 {
-  struct remote_state *rs = get_remote_state ();
+  remote_state *rs = get_remote_state ();
 
-  return !remote_vfork_event_p (rs);
+  return !m_features.remote_vfork_event_p (rs);
 }
 
 /* Remove vfork catchpoint target routine.  Nothing to do, just
@@ -2407,9 +2481,9 @@ remote_target::remove_vfork_catchpoint (int pid)
 int
 remote_target::insert_exec_catchpoint (int pid)
 {
-  struct remote_state *rs = get_remote_state ();
+  remote_state *rs = get_remote_state ();
 
-  return !remote_exec_event_p (rs);
+  return !m_features.remote_exec_event_p (rs);
 }
 
 /* Remove exec catchpoint target routine.  Nothing to do, just
@@ -2438,10 +2512,10 @@ remote_target::remote_query_attached (int pid)
   struct remote_state *rs = get_remote_state ();
   size_t size = get_remote_packet_size ();
 
-  if (packet_support (PACKET_qAttached) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_qAttached) == PACKET_DISABLE)
     return 0;
 
-  if (remote_multi_process_p (rs))
+  if (m_features.remote_multi_process_p (rs))
     xsnprintf (rs->buf.data (), size, "qAttached:%x", pid);
   else
     xsnprintf (rs->buf.data (), size, "qAttached");
@@ -2450,7 +2524,7 @@ remote_target::remote_query_attached (int pid)
   getpkt (&rs->buf, 0);
 
   switch (packet_ok (rs->buf,
-		     &remote_protocol_packets[PACKET_qAttached]))
+		     &m_features.m_protocol_packets[PACKET_qAttached]))
     {
     case PACKET_OK:
       if (strcmp (rs->buf.data (), "1") == 0)
@@ -2651,7 +2725,7 @@ remote_target::remote_notice_new_inferior (ptid_t currthread, bool executing)
       if (find_inferior_pid (this, currthread.pid ()) == NULL)
 	{
 	  struct remote_state *rs = get_remote_state ();
-	  bool fake_pid_p = !remote_multi_process_p (rs);
+	  bool fake_pid_p = !m_features.remote_multi_process_p (rs);
 
 	  inf = remote_add_inferior (fake_pid_p,
 				     currthread.pid (), -1, 1);
@@ -2714,7 +2788,7 @@ record_currthread (struct remote_state *rs, ptid_t currthread)
 void
 remote_target::pass_signals (gdb::array_view<const unsigned char> pass_signals)
 {
-  if (packet_support (PACKET_QPassSignals) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_QPassSignals) != PACKET_DISABLE)
     {
       char *pass_packet, *p;
       int count = 0;
@@ -2748,7 +2822,8 @@ remote_target::pass_signals (gdb::array_view<const unsigned char> pass_signals)
 	{
 	  putpkt (pass_packet);
 	  getpkt (&rs->buf, 0);
-	  packet_ok (rs->buf, &remote_protocol_packets[PACKET_QPassSignals]);
+	  packet_ok (rs->buf,
+		     &m_features.m_protocol_packets[PACKET_QPassSignals]);
 	  xfree (rs->last_pass_packet);
 	  rs->last_pass_packet = pass_packet;
 	}
@@ -2768,7 +2843,7 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
   enum packet_result result;
   int n_sysno = 0;
 
-  if (packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE)
     {
       /* Not supported.  */
       return 1;
@@ -2821,7 +2896,8 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
 
   putpkt (catch_packet);
   getpkt (&rs->buf, 0);
-  result = packet_ok (rs->buf, &remote_protocol_packets[PACKET_QCatchSyscalls]);
+  result = packet_ok (rs->buf,
+		      &m_features.m_protocol_packets[PACKET_QCatchSyscalls]);
   if (result == PACKET_OK)
     return 0;
   else
@@ -2834,7 +2910,7 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
 void
 remote_target::program_signals (gdb::array_view<const unsigned char> signals)
 {
-  if (packet_support (PACKET_QProgramSignals) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_QProgramSignals) != PACKET_DISABLE)
     {
       char *packet, *p;
       int count = 0;
@@ -2869,7 +2945,8 @@ remote_target::program_signals (gdb::array_view<const unsigned char> signals)
 	{
 	  putpkt (packet);
 	  getpkt (&rs->buf, 0);
-	  packet_ok (rs->buf, &remote_protocol_packets[PACKET_QProgramSignals]);
+	  packet_ok (rs->buf,
+		     &m_features.m_protocol_packets[PACKET_QProgramSignals]);
 	  xfree (rs->last_program_signals_packet);
 	  rs->last_program_signals_packet = packet;
 	}
@@ -2938,7 +3015,7 @@ remote_target::set_general_process ()
   struct remote_state *rs = get_remote_state ();
 
   /* If the remote can't handle multiple processes, don't bother.  */
-  if (!remote_multi_process_p (rs))
+  if (!m_features.remote_multi_process_p (rs))
     return;
 
   /* We only need to change the remote current thread if it's pointing
@@ -3092,7 +3169,7 @@ remote_target::write_ptid (char *buf, const char *endbuf, ptid_t ptid)
   int pid, tid;
   struct remote_state *rs = get_remote_state ();
 
-  if (remote_multi_process_p (rs))
+  if (m_features.remote_multi_process_p (rs))
     {
       pid = ptid.pid ();
       if (pid < 0)
@@ -3853,7 +3930,7 @@ int
 remote_target::remote_get_threads_with_qxfer (threads_listing_context *context)
 {
 #if defined(HAVE_LIBEXPAT)
-  if (packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
+  if (m_features.packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
     {
       gdb::optional<gdb::char_vector> xml
 	= target_read_stralloc (this, TARGET_OBJECT_THREADS, NULL);
@@ -4046,7 +4123,7 @@ remote_target::extra_thread_info (thread_info *tp)
   if (!extra.empty ())
     return extra.c_str ();
 
-  if (packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
+  if (m_features.packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
     {
       /* If we're using qXfer:threads:read, then the extra info is
 	 included in the XML.  So if we didn't have anything cached,
@@ -4483,7 +4560,7 @@ remote_target::add_current_inferior_and_thread (const char *wait_status)
 
   if (curr_ptid != null_ptid)
     {
-      if (!remote_multi_process_p (rs))
+      if (!m_features.remote_multi_process_p (rs))
 	fake_pid_p = true;
     }
   else
@@ -4763,7 +4840,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
   remote_query_supported ();
 
   /* If the stub wants to get a QAllow, compose one and send it.  */
-  if (packet_support (PACKET_QAllow) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_QAllow) != PACKET_DISABLE)
     set_permissions ();
 
   /* gdbserver < 7.7 (before its fix from 2013-12-11) did reply to any
@@ -4779,7 +4856,10 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
     putpkt (v_mustreplyempty);
     getpkt (&rs->buf, 0);
     if (strcmp (rs->buf.data (), "OK") == 0)
-      remote_protocol_packets[PACKET_vFile_setfs].support = PACKET_DISABLE;
+      {
+	m_features.m_protocol_packets[PACKET_vFile_setfs].support
+	  = PACKET_DISABLE;
+      }
     else if (strcmp (rs->buf.data (), "") != 0)
       error (_("Remote replied unexpectedly to '%s': %s"), v_mustreplyempty,
 	     rs->buf.data ());
@@ -4798,7 +4878,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
      If FALSE, then don't activate noack mode, regardless of what the
      stub claimed should be the default with qSupported.  */
 
-  noack_config = &remote_protocol_packets[PACKET_QStartNoAckMode];
+  noack_config = &m_features.m_protocol_packets[PACKET_QStartNoAckMode];
   if (packet_config_support (noack_config) != PACKET_DISABLE)
     {
       putpkt ("QStartNoAckMode");
@@ -4833,7 +4913,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
 
   if (target_is_non_stop_p ())
     {
-      if (packet_support (PACKET_QNonStop) != PACKET_ENABLE)
+      if (m_features.packet_support (PACKET_QNonStop) != PACKET_ENABLE)
 	error (_("Non-stop mode requested, but remote "
 		 "does not support non-stop"));
 
@@ -4850,7 +4930,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
 	 stopped.  */
       this->update_thread_list ();
     }
-  else if (packet_support (PACKET_QNonStop) == PACKET_ENABLE)
+  else if (m_features.packet_support (PACKET_QNonStop) == PACKET_ENABLE)
     {
       /* Don't assume that the stub can operate in all-stop mode.
 	 Request it explicitly.  */
@@ -5082,13 +5162,13 @@ extended_remote_target::open (const char *name, int from_tty)
 /* Reset all packets back to "unknown support".  Called when opening a
    new connection to a remote target.  */
 
-static void
-reset_all_packet_configs_support (void)
+void
+remote_features::reset_all_packet_configs_support (void)
 {
   int i;
 
   for (i = 0; i < PACKET_MAX; i++)
-    remote_protocol_packets[i].support = PACKET_SUPPORT_UNKNOWN;
+    m_protocol_packets[i].support = PACKET_SUPPORT_UNKNOWN;
 }
 
 /* Initialize all packet configs.  */
@@ -5121,7 +5201,7 @@ remote_target::remote_check_symbols ()
   if (!target_has_execution ())
     return;
 
-  if (packet_support (PACKET_qSymbol) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_qSymbol) == PACKET_DISABLE)
     return;
 
   /* Make sure the remote is pointing at the right process.  Note
@@ -5137,7 +5217,7 @@ remote_target::remote_check_symbols ()
 
   putpkt ("qSymbol::");
   getpkt (&reply, 0);
-  packet_ok (reply, &remote_protocol_packets[PACKET_qSymbol]);
+  packet_ok (reply, &m_features.m_protocol_packets[PACKET_qSymbol]);
 
   while (startswith (reply.data (), "qSymbol:"))
     {
@@ -5261,7 +5341,7 @@ remote_supported_packet (remote_target *remote,
       return;
     }
 
-  remote_protocol_packets[feature->packet].support = support;
+  remote->m_features.m_protocol_packets[feature->packet].support = support;
 }
 
 void
@@ -5469,47 +5549,57 @@ remote_target::remote_query_supported ()
      containing no features.  */
 
   rs->buf[0] = 0;
-  if (packet_support (PACKET_qSupported) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_qSupported) != PACKET_DISABLE)
     {
       std::string q;
 
-      if (packet_set_cmd_state (PACKET_multiprocess_feature) != AUTO_BOOLEAN_FALSE)
+      if (m_features.packet_set_cmd_state (PACKET_multiprocess_feature)
+	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "multiprocess+");
 
-      if (packet_set_cmd_state (PACKET_swbreak_feature) != AUTO_BOOLEAN_FALSE)
+      if (m_features.packet_set_cmd_state (PACKET_swbreak_feature)
+	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "swbreak+");
-      if (packet_set_cmd_state (PACKET_hwbreak_feature) != AUTO_BOOLEAN_FALSE)
+
+      if (m_features.packet_set_cmd_state (PACKET_hwbreak_feature)
+	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "hwbreak+");
 
       remote_query_supported_append (&q, "qRelocInsn+");
 
-      if (packet_set_cmd_state (PACKET_fork_event_feature)
+      if (m_features.packet_set_cmd_state (PACKET_fork_event_feature)
 	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "fork-events+");
-      if (packet_set_cmd_state (PACKET_vfork_event_feature)
+
+      if (m_features.packet_set_cmd_state (PACKET_vfork_event_feature)
 	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "vfork-events+");
-      if (packet_set_cmd_state (PACKET_exec_event_feature)
+
+      if (m_features.packet_set_cmd_state (PACKET_exec_event_feature)
 	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "exec-events+");
 
-      if (packet_set_cmd_state (PACKET_vContSupported) != AUTO_BOOLEAN_FALSE)
+      if (m_features.packet_set_cmd_state (PACKET_vContSupported)
+	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "vContSupported+");
 
-      if (packet_set_cmd_state (PACKET_QThreadEvents) != AUTO_BOOLEAN_FALSE)
+      if (m_features.packet_set_cmd_state (PACKET_QThreadEvents)
+	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "QThreadEvents+");
 
-      if (packet_set_cmd_state (PACKET_no_resumed) != AUTO_BOOLEAN_FALSE)
+      if (m_features.packet_set_cmd_state (PACKET_no_resumed)
+	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "no-resumed+");
 
-      if (packet_set_cmd_state (PACKET_memory_tagging_feature)
+      if (m_features.packet_set_cmd_state (PACKET_memory_tagging_feature)
 	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "memory-tagging+");
 
       /* Keep this one last to work around a gdbserver <= 7.10 bug in
 	 the qSupported:xmlRegisters=i386 handling.  */
       if (remote_support_xml != NULL
-	  && packet_support (PACKET_qXfer_features) != PACKET_DISABLE)
+	  && (m_features.packet_support (PACKET_qXfer_features)
+	      != PACKET_DISABLE))
 	remote_query_supported_append (&q, remote_support_xml);
 
       q = "qSupported:" + q;
@@ -5519,7 +5609,7 @@ remote_target::remote_query_supported ()
 
       /* If an error occured, warn, but do not return - just reset the
 	 buffer to empty and go on to disable features.  */
-      if (packet_ok (rs->buf, &remote_protocol_packets[PACKET_qSupported])
+      if (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_qSupported])
 	  == PACKET_ERROR)
 	{
 	  warning (_("Remote failure reply: %s"), rs->buf.data ());
@@ -5797,7 +5887,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p)
 
   /* Reset the target state; these things will be queried either by
      remote_query_supported or as they are needed.  */
-  reset_all_packet_configs_support ();
+  remote->m_features.reset_all_packet_configs_support ();
   rs->explicit_packet_size = 0;
   rs->noack_mode = 0;
   rs->extended = extended_p;
@@ -5910,7 +6000,7 @@ remote_target::remote_detach_pid (int pid)
      GDBserver to select GDB's current process.  */
   set_general_process ();
 
-  if (remote_multi_process_p (rs))
+  if (m_features.remote_multi_process_p (rs))
     xsnprintf (rs->buf.data (), get_remote_packet_size (), "D;%x", pid);
   else
     strcpy (rs->buf.data (), "D");
@@ -6043,8 +6133,10 @@ remote_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
 
   struct remote_state *rs = get_remote_state ();
 
-  if ((fork_kind == TARGET_WAITKIND_FORKED && remote_fork_event_p (rs))
-      || (fork_kind == TARGET_WAITKIND_VFORKED && remote_vfork_event_p (rs)))
+  if ((fork_kind == TARGET_WAITKIND_FORKED
+       && m_features.remote_fork_event_p (rs))
+      || (fork_kind == TARGET_WAITKIND_VFORKED
+	  && m_features.remote_vfork_event_p (rs)))
     {
       /* When following the parent and detaching the child, we detach
 	 the child here.  For the case of following the child and
@@ -6110,7 +6202,7 @@ extended_remote_target::attach (const char *args, int from_tty)
   /* Remote PID can be freely equal to getpid, do not check it here the same
      way as in other targets.  */
 
-  if (packet_support (PACKET_vAttach) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_vAttach) == PACKET_DISABLE)
     error (_("This target does not support attaching to a process"));
 
   target_announce_attach (from_tty, pid);
@@ -6120,7 +6212,7 @@ extended_remote_target::attach (const char *args, int from_tty)
   getpkt (&rs->buf, 0);
 
   switch (packet_ok (rs->buf,
-		     &remote_protocol_packets[PACKET_vAttach]))
+		     &m_features.m_protocol_packets[PACKET_vAttach]))
     {
     case PACKET_OK:
       if (!target_is_non_stop_p ())
@@ -6265,7 +6357,7 @@ remote_target::remote_vcont_probe ()
 	buf[0] = 0;
     }
 
-  packet_ok (rs->buf, &remote_protocol_packets[PACKET_vCont]);
+  packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vCont]);
   rs->supports_vCont_probed = true;
 }
 
@@ -6296,7 +6388,7 @@ remote_target::append_resumption (char *p, char *endp,
 	      threads with a wildcard (though the protocol allows it,
 	      so stubs shouldn't make an active effort to forbid
 	      it).  */
-	   && !(remote_multi_process_p (rs) && ptid.is_pid ()))
+	   && !(m_features.remote_multi_process_p (rs) && ptid.is_pid ()))
     {
       struct thread_info *tp;
 
@@ -6330,7 +6422,7 @@ remote_target::append_resumption (char *p, char *endp,
   else
     p += xsnprintf (p, endp - p, ";c");
 
-  if (remote_multi_process_p (rs) && ptid.is_pid ())
+  if (m_features.remote_multi_process_p (rs) && ptid.is_pid ())
     {
       ptid_t nptid;
 
@@ -6414,9 +6506,9 @@ remote_target::remote_resume_with_hc (ptid_t ptid, int step,
 	warning (_(" - Can't pass signal %d to target in reverse: ignored."),
 		 siggnal);
 
-      if (step && packet_support (PACKET_bs) == PACKET_DISABLE)
+      if (step && m_features.packet_support (PACKET_bs) == PACKET_DISABLE)
 	error (_("Remote reverse-step not supported."));
-      if (!step && packet_support (PACKET_bc) == PACKET_DISABLE)
+      if (!step && m_features.packet_support (PACKET_bc) == PACKET_DISABLE)
 	error (_("Remote reverse-continue not supported."));
 
       strcpy (buf, step ? "bs" : "bc");
@@ -6456,10 +6548,10 @@ remote_target::remote_resume_with_vcont (ptid_t ptid, int step,
   if (::execution_direction == EXEC_REVERSE)
     return 0;
 
-  if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
+  if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
     remote_vcont_probe ();
 
-  if (packet_support (PACKET_vCont) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_vCont) == PACKET_DISABLE)
     return 0;
 
   p = rs->buf.data ();
@@ -7002,7 +7094,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
 
   /* FIXME: This supports_vCont_probed check is a workaround until
      packet_support is per-connection.  */
-  if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN
+  if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN
       || !rs->supports_vCont_probed)
     remote_vcont_probe ();
 
@@ -7010,7 +7102,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
     error (_("Remote server does not support stopping threads"));
 
   if (ptid == minus_one_ptid
-      || (!remote_multi_process_p (rs) && ptid.is_pid ()))
+      || (!m_features.remote_multi_process_p (rs) && ptid.is_pid ()))
     p += xsnprintf (p, endp - p, "vCont;t");
   else
     {
@@ -7084,7 +7176,7 @@ remote_target::remote_interrupt_ns ()
   putpkt (rs->buf);
   getpkt (&rs->buf, 0);
 
-  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vCtrlC]))
+  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vCtrlC]))
     {
     case PACKET_OK:
       break;
@@ -7591,7 +7683,8 @@ Packet: '%s'\n"),
 
 	      /* Make sure the stub doesn't forget to indicate support
 		 with qSupported.  */
-	      if (packet_support (PACKET_swbreak_feature) != PACKET_ENABLE)
+	      if (m_features.packet_support (PACKET_swbreak_feature)
+		  != PACKET_ENABLE)
 		error (_("Unexpected swbreak stop reason"));
 
 	      /* The value part is documented as "must be empty",
@@ -7605,7 +7698,8 @@ Packet: '%s'\n"),
 
 	      /* Make sure the stub doesn't forget to indicate support
 		 with qSupported.  */
-	      if (packet_support (PACKET_hwbreak_feature) != PACKET_ENABLE)
+	      if (m_features.packet_support (PACKET_hwbreak_feature)
+		  != PACKET_ENABLE)
 		error (_("Unexpected hwbreak stop reason"));
 
 	      /* See above.  */
@@ -8376,7 +8470,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
   gdb_byte *regp = (gdb_byte *) alloca (register_size (gdbarch, reg->regnum));
   int i;
 
-  if (packet_support (PACKET_p) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_p) == PACKET_DISABLE)
     return 0;
 
   if (reg->pnum == -1)
@@ -8391,7 +8485,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
 
   buf = rs->buf.data ();
 
-  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_p]))
+  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_p]))
     {
     case PACKET_OK:
       break;
@@ -8658,7 +8752,7 @@ remote_target::prepare_to_store (struct regcache *regcache)
   int i;
 
   /* Make sure the entire registers array is valid.  */
-  switch (packet_support (PACKET_P))
+  switch (m_features.packet_support (PACKET_P))
     {
     case PACKET_DISABLE:
     case PACKET_SUPPORT_UNKNOWN:
@@ -8686,7 +8780,7 @@ remote_target::store_register_using_P (const struct regcache *regcache,
   gdb_byte *regp = (gdb_byte *) alloca (register_size (gdbarch, reg->regnum));
   char *p;
 
-  if (packet_support (PACKET_P) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_P) == PACKET_DISABLE)
     return 0;
 
   if (reg->pnum == -1)
@@ -8699,7 +8793,7 @@ remote_target::store_register_using_P (const struct regcache *regcache,
   putpkt (rs->buf);
   getpkt (&rs->buf, 0);
 
-  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_P]))
+  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_P]))
     {
     case PACKET_OK:
       return 1;
@@ -8882,7 +8976,7 @@ remote_target::check_binary_download (CORE_ADDR addr)
 {
   struct remote_state *rs = get_remote_state ();
 
-  switch (packet_support (PACKET_X))
+  switch (m_features.packet_support (PACKET_X))
     {
     case PACKET_DISABLE:
       break;
@@ -8906,12 +9000,12 @@ remote_target::check_binary_download (CORE_ADDR addr)
 	if (rs->buf[0] == '\0')
 	  {
 	    remote_debug_printf ("binary downloading NOT supported by target");
-	    remote_protocol_packets[PACKET_X].support = PACKET_DISABLE;
+	    m_features.m_protocol_packets[PACKET_X].support = PACKET_DISABLE;
 	  }
 	else
 	  {
 	    remote_debug_printf ("binary downloading supported by target");
-	    remote_protocol_packets[PACKET_X].support = PACKET_ENABLE;
+	    m_features.m_protocol_packets[PACKET_X].support = PACKET_ENABLE;
 	  }
 	break;
       }
@@ -9138,7 +9232,7 @@ remote_target::remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr,
   /* Check whether the target supports binary download.  */
   check_binary_download (memaddr);
 
-  switch (packet_support (PACKET_X))
+  switch (m_features.packet_support (PACKET_X))
     {
     case PACKET_ENABLE:
       packet_format = "X";
@@ -10107,7 +10201,7 @@ remote_target::kill ()
 
   gdb_assert (inf != nullptr);
 
-  if (packet_support (PACKET_vKill) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_vKill) != PACKET_DISABLE)
     {
       /* If we're stopped while forking and we haven't followed yet,
 	 kill the child task.  We need to do this before killing the
@@ -10126,7 +10220,7 @@ remote_target::kill ()
   /* If we are in 'target remote' mode and we are killing the only
      inferior, then we will tell gdbserver to exit and unpush the
      target.  */
-  if (res == -1 && !remote_multi_process_p (rs)
+  if (res == -1 && !m_features.remote_multi_process_p (rs)
       && number_of_live_inferiors (this) == 1)
     {
       remote_kill_k ();
@@ -10148,7 +10242,7 @@ remote_target::kill ()
 int
 remote_target::remote_vkill (int pid)
 {
-  if (packet_support (PACKET_vKill) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_vKill) == PACKET_DISABLE)
     return -1;
 
   remote_state *rs = get_remote_state ();
@@ -10159,7 +10253,7 @@ remote_target::remote_vkill (int pid)
   getpkt (&rs->buf, 0);
 
   switch (packet_ok (rs->buf,
-		     &remote_protocol_packets[PACKET_vKill]))
+		     &m_features.m_protocol_packets[PACKET_vKill]))
     {
     case PACKET_OK:
       return 0;
@@ -10255,7 +10349,8 @@ remote_target::mourn_inferior ()
 bool
 extended_remote_target::supports_disable_randomization ()
 {
-  return packet_support (PACKET_QDisableRandomization) == PACKET_ENABLE;
+  return (m_features.packet_support (PACKET_QDisableRandomization)
+	  == PACKET_ENABLE);
 }
 
 void
@@ -10283,7 +10378,7 @@ remote_target::extended_remote_run (const std::string &args)
 
   /* If the user has disabled vRun support, or we have detected that
      support is not available, do not try it.  */
-  if (packet_support (PACKET_vRun) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_vRun) == PACKET_DISABLE)
     return -1;
 
   strcpy (rs->buf.data (), "vRun;");
@@ -10314,7 +10409,7 @@ remote_target::extended_remote_run (const std::string &args)
   putpkt (rs->buf);
   getpkt (&rs->buf, 0);
 
-  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vRun]))
+  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vRun]))
     {
     case PACKET_OK:
       /* We have a wait response.  All is well.  */
@@ -10367,7 +10462,7 @@ remote_target::extended_remote_environment_support ()
 {
   remote_state *rs = get_remote_state ();
 
-  if (packet_support (PACKET_QEnvironmentReset) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_QEnvironmentReset) != PACKET_DISABLE)
     {
       putpkt ("QEnvironmentReset");
       getpkt (&rs->buf, 0);
@@ -10377,12 +10472,16 @@ remote_target::extended_remote_environment_support ()
 
   gdb_environ *e = &current_inferior ()->environment;
 
-  if (packet_support (PACKET_QEnvironmentHexEncoded) != PACKET_DISABLE)
-    for (const std::string &el : e->user_set_env ())
-      send_environment_packet ("set", "QEnvironmentHexEncoded",
-			       el.c_str ());
+  if (m_features.packet_support (PACKET_QEnvironmentHexEncoded)
+      != PACKET_DISABLE)
+    {
+      for (const std::string &el : e->user_set_env ())
+	send_environment_packet ("set", "QEnvironmentHexEncoded",
+				 el.c_str ());
+    }
 
-  if (packet_support (PACKET_QEnvironmentUnset) != PACKET_DISABLE)
+
+  if (m_features.packet_support (PACKET_QEnvironmentUnset) != PACKET_DISABLE)
     for (const std::string &el : e->user_unset_env ())
       send_environment_packet ("unset", "QEnvironmentUnset", el.c_str ());
 }
@@ -10393,7 +10492,7 @@ remote_target::extended_remote_environment_support ()
 void
 remote_target::extended_remote_set_inferior_cwd ()
 {
-  if (packet_support (PACKET_QSetWorkingDir) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_QSetWorkingDir) != PACKET_DISABLE)
     {
       const std::string &inferior_cwd = current_inferior ()->cwd ();
       remote_state *rs = get_remote_state ();
@@ -10418,7 +10517,7 @@ remote_target::extended_remote_set_inferior_cwd ()
       putpkt (rs->buf);
       getpkt (&rs->buf, 0);
       if (packet_ok (rs->buf,
-		     &remote_protocol_packets[PACKET_QSetWorkingDir])
+		     &m_features.m_protocol_packets[PACKET_QSetWorkingDir])
 	  != PACKET_OK)
 	error (_("\
 Remote replied unexpectedly while setting the inferior's working\n\
@@ -10455,7 +10554,7 @@ extended_remote_target::create_inferior (const char *exec_file,
 
   /* If startup-with-shell is on, we inform gdbserver to start the
      remote inferior using a shell.  */
-  if (packet_support (PACKET_QStartupWithShell) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_QStartupWithShell) != PACKET_DISABLE)
     {
       xsnprintf (rs->buf.data (), get_remote_packet_size (),
 		 "QStartupWithShell:%d", startup_with_shell ? 1 : 0);
@@ -10561,7 +10660,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
      fails, and the user has explicitly requested the Z support then
      report an error, otherwise, mark it disabled and go on.  */
 
-  if (packet_support (PACKET_Z0) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_Z0) != PACKET_DISABLE)
     {
       CORE_ADDR addr = bp_tgt->reqstd_address;
       struct remote_state *rs;
@@ -10592,7 +10691,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
       putpkt (rs->buf);
       getpkt (&rs->buf, 0);
 
-      switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z0]))
+      switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_Z0]))
 	{
 	case PACKET_ERROR:
 	  return -1;
@@ -10620,7 +10719,7 @@ remote_target::remove_breakpoint (struct gdbarch *gdbarch,
   CORE_ADDR addr = bp_tgt->placed_address;
   struct remote_state *rs = get_remote_state ();
 
-  if (packet_support (PACKET_Z0) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_Z0) != PACKET_DISABLE)
     {
       char *p = rs->buf.data ();
       char *endbuf = p + get_remote_packet_size ();
@@ -10676,7 +10775,7 @@ remote_target::insert_watchpoint (CORE_ADDR addr, int len,
   char *p;
   enum Z_packet_type packet = watchpoint_to_Z_packet (type);
 
-  if (packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
     return 1;
 
   /* Make sure the remote is pointing at the right process, if
@@ -10693,7 +10792,8 @@ remote_target::insert_watchpoint (CORE_ADDR addr, int len,
   putpkt (rs->buf);
   getpkt (&rs->buf, 0);
 
-  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z0 + packet]))
+  switch (packet_ok (rs->buf,
+		     &m_features.m_protocol_packets[PACKET_Z0 + packet]))
     {
     case PACKET_ERROR:
       return -1;
@@ -10725,7 +10825,7 @@ remote_target::remove_watchpoint (CORE_ADDR addr, int len,
   char *p;
   enum Z_packet_type packet = watchpoint_to_Z_packet (type);
 
-  if (packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
     return -1;
 
   /* Make sure the remote is pointing at the right process, if
@@ -10741,7 +10841,8 @@ remote_target::remove_watchpoint (CORE_ADDR addr, int len,
   putpkt (rs->buf);
   getpkt (&rs->buf, 0);
 
-  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z0 + packet]))
+  switch (packet_ok (rs->buf,
+		     &m_features.m_protocol_packets[PACKET_Z0 + packet]))
     {
     case PACKET_ERROR:
     case PACKET_UNKNOWN:
@@ -10815,7 +10916,7 @@ remote_target::stopped_by_sw_breakpoint ()
 bool
 remote_target::supports_stopped_by_sw_breakpoint ()
 {
-  return (packet_support (PACKET_swbreak_feature) == PACKET_ENABLE);
+  return (m_features.packet_support (PACKET_swbreak_feature) == PACKET_ENABLE);
 }
 
 /* The to_stopped_by_hw_breakpoint method of target remote.  */
@@ -10836,7 +10937,7 @@ remote_target::stopped_by_hw_breakpoint ()
 bool
 remote_target::supports_stopped_by_hw_breakpoint ()
 {
-  return (packet_support (PACKET_hwbreak_feature) == PACKET_ENABLE);
+  return (m_features.packet_support (PACKET_hwbreak_feature) == PACKET_ENABLE);
 }
 
 bool
@@ -10875,7 +10976,7 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
   char *p, *endbuf;
   char *message;
 
-  if (packet_support (PACKET_Z1) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_Z1) == PACKET_DISABLE)
     return -1;
 
   /* Make sure the remote is pointing at the right process, if
@@ -10904,7 +11005,7 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
   putpkt (rs->buf);
   getpkt (&rs->buf, 0);
 
-  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z1]))
+  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_Z1]))
     {
     case PACKET_ERROR:
       if (rs->buf[1] == '.')
@@ -10933,7 +11034,7 @@ remote_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
   char *p = rs->buf.data ();
   char *endbuf = p + get_remote_packet_size ();
 
-  if (packet_support (PACKET_Z1) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_Z1) == PACKET_DISABLE)
     return -1;
 
   /* Make sure the remote is pointing at the right process, if
@@ -10952,7 +11053,7 @@ remote_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
   putpkt (rs->buf);
   getpkt (&rs->buf, 0);
 
-  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z1]))
+  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_Z1]))
     {
     case PACKET_ERROR:
     case PACKET_UNKNOWN:
@@ -10976,7 +11077,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
   /* It doesn't make sense to use qCRC if the remote target is
      connected but not running.  */
   if (target_has_execution ()
-      && packet_support (PACKET_qCRC) != PACKET_DISABLE)
+      && m_features.packet_support (PACKET_qCRC) != PACKET_DISABLE)
     {
       enum packet_result result;
 
@@ -10995,7 +11096,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
       getpkt (&rs->buf, 0);
 
       result = packet_ok (rs->buf,
-			  &remote_protocol_packets[PACKET_qCRC]);
+			  &m_features.m_protocol_packets[PACKET_qCRC]);
       if (result == PACKET_ERROR)
 	return -1;
       else if (result == PACKET_OK)
@@ -11255,12 +11356,12 @@ remote_target::xfer_partial (enum target_object object,
     {
       if (readbuf)
 	return remote_read_qxfer ("siginfo", annex, readbuf, offset, len,
-				  xfered_len, &remote_protocol_packets
+				  xfered_len, &m_features.m_protocol_packets
 				  [PACKET_qXfer_siginfo_read]);
       else
 	return remote_write_qxfer ("siginfo", annex,
 				   writebuf, offset, len, xfered_len,
-				   &remote_protocol_packets
+				   &m_features.m_protocol_packets
 				   [PACKET_qXfer_siginfo_write]);
     }
 
@@ -11269,7 +11370,7 @@ remote_target::xfer_partial (enum target_object object,
       if (readbuf)
 	return remote_read_qxfer ("statictrace", annex,
 				  readbuf, offset, len, xfered_len,
-				  &remote_protocol_packets
+				  &m_features.m_protocol_packets
 				  [PACKET_qXfer_statictrace_read]);
       else
 	return TARGET_XFER_E_IO;
@@ -11299,74 +11400,74 @@ remote_target::xfer_partial (enum target_object object,
 
     case TARGET_OBJECT_AUXV:
       gdb_assert (annex == NULL);
-      return remote_read_qxfer ("auxv", annex, readbuf, offset, len,
-				xfered_len,
-				&remote_protocol_packets[PACKET_qXfer_auxv]);
+      return remote_read_qxfer
+	("auxv", annex, readbuf, offset, len, xfered_len,
+	 &m_features.m_protocol_packets[PACKET_qXfer_auxv]);
 
     case TARGET_OBJECT_AVAILABLE_FEATURES:
       return remote_read_qxfer
 	("features", annex, readbuf, offset, len, xfered_len,
-	 &remote_protocol_packets[PACKET_qXfer_features]);
+	 &m_features.m_protocol_packets[PACKET_qXfer_features]);
 
     case TARGET_OBJECT_LIBRARIES:
       return remote_read_qxfer
 	("libraries", annex, readbuf, offset, len, xfered_len,
-	 &remote_protocol_packets[PACKET_qXfer_libraries]);
+	 &m_features.m_protocol_packets[PACKET_qXfer_libraries]);
 
     case TARGET_OBJECT_LIBRARIES_SVR4:
       return remote_read_qxfer
 	("libraries-svr4", annex, readbuf, offset, len, xfered_len,
-	 &remote_protocol_packets[PACKET_qXfer_libraries_svr4]);
+	 &m_features.m_protocol_packets[PACKET_qXfer_libraries_svr4]);
 
     case TARGET_OBJECT_MEMORY_MAP:
       gdb_assert (annex == NULL);
-      return remote_read_qxfer ("memory-map", annex, readbuf, offset, len,
-				 xfered_len,
-				&remote_protocol_packets[PACKET_qXfer_memory_map]);
+      return remote_read_qxfer
+	("memory-map", annex, readbuf, offset, len, xfered_len,
+	 &m_features.m_protocol_packets[PACKET_qXfer_memory_map]);
 
     case TARGET_OBJECT_OSDATA:
       /* Should only get here if we're connected.  */
       gdb_assert (rs->remote_desc);
       return remote_read_qxfer
 	("osdata", annex, readbuf, offset, len, xfered_len,
-	&remote_protocol_packets[PACKET_qXfer_osdata]);
+	&m_features.m_protocol_packets[PACKET_qXfer_osdata]);
 
     case TARGET_OBJECT_THREADS:
       gdb_assert (annex == NULL);
-      return remote_read_qxfer ("threads", annex, readbuf, offset, len,
-				xfered_len,
-				&remote_protocol_packets[PACKET_qXfer_threads]);
+      return remote_read_qxfer
+	("threads", annex, readbuf, offset, len, xfered_len,
+	 &m_features.m_protocol_packets[PACKET_qXfer_threads]);
 
     case TARGET_OBJECT_TRACEFRAME_INFO:
       gdb_assert (annex == NULL);
       return remote_read_qxfer
 	("traceframe-info", annex, readbuf, offset, len, xfered_len,
-	 &remote_protocol_packets[PACKET_qXfer_traceframe_info]);
+	 &m_features.m_protocol_packets[PACKET_qXfer_traceframe_info]);
 
     case TARGET_OBJECT_FDPIC:
-      return remote_read_qxfer ("fdpic", annex, readbuf, offset, len,
-				xfered_len,
-				&remote_protocol_packets[PACKET_qXfer_fdpic]);
+      return remote_read_qxfer
+	("fdpic", annex, readbuf, offset, len, xfered_len,
+	 &m_features.m_protocol_packets[PACKET_qXfer_fdpic]);
 
     case TARGET_OBJECT_OPENVMS_UIB:
-      return remote_read_qxfer ("uib", annex, readbuf, offset, len,
-				xfered_len,
-				&remote_protocol_packets[PACKET_qXfer_uib]);
+      return remote_read_qxfer
+	("uib", annex, readbuf, offset, len, xfered_len,
+	 &m_features.m_protocol_packets[PACKET_qXfer_uib]);
 
     case TARGET_OBJECT_BTRACE:
-      return remote_read_qxfer ("btrace", annex, readbuf, offset, len,
-				xfered_len,
-	&remote_protocol_packets[PACKET_qXfer_btrace]);
+      return remote_read_qxfer
+	("btrace", annex, readbuf, offset, len, xfered_len,
+	 &m_features.m_protocol_packets[PACKET_qXfer_btrace]);
 
     case TARGET_OBJECT_BTRACE_CONF:
-      return remote_read_qxfer ("btrace-conf", annex, readbuf, offset,
-				len, xfered_len,
-	&remote_protocol_packets[PACKET_qXfer_btrace_conf]);
+      return remote_read_qxfer
+	("btrace-conf", annex, readbuf, offset, len, xfered_len,
+	&m_features.m_protocol_packets[PACKET_qXfer_btrace_conf]);
 
     case TARGET_OBJECT_EXEC_FILE:
-      return remote_read_qxfer ("exec-file", annex, readbuf, offset,
-				len, xfered_len,
-	&remote_protocol_packets[PACKET_qXfer_exec_file]);
+      return remote_read_qxfer
+	("exec-file", annex, readbuf, offset, len, xfered_len,
+	 &m_features.m_protocol_packets[PACKET_qXfer_exec_file]);
 
     default:
       return TARGET_XFER_E_IO;
@@ -11433,7 +11534,7 @@ remote_target::search_memory (CORE_ADDR start_addr, ULONGEST search_space_len,
   struct remote_state *rs = get_remote_state ();
   int max_size = get_memory_write_packet_size ();
   struct packet_config *packet =
-    &remote_protocol_packets[PACKET_qSearch_memory];
+    &m_features.m_protocol_packets[PACKET_qSearch_memory];
   /* Number of packet bytes used to encode the pattern;
      this could be more than PATTERN_LEN due to escape characters.  */
   int escaped_pattern_len;
@@ -11858,7 +11959,7 @@ remote_target::pid_to_str (ptid_t ptid)
 	 connecting with extended-remote and the stub already being
 	 attached to a process, and reporting yes to qAttached, hence
 	 no smart special casing here.  */
-      if (!remote_multi_process_p (rs))
+      if (!m_features.remote_multi_process_p (rs))
 	return "Remote target";
 
       return normal_pid_to_str (ptid);
@@ -11867,7 +11968,7 @@ remote_target::pid_to_str (ptid_t ptid)
     {
       if (magic_null_ptid == ptid)
 	return "Thread <main>";
-      else if (remote_multi_process_p (rs))
+      else if (m_features.remote_multi_process_p (rs))
 	if (ptid.lwp () == 0)
 	  return normal_pid_to_str (ptid);
 	else
@@ -11885,7 +11986,7 @@ CORE_ADDR
 remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
 					 CORE_ADDR offset)
 {
-  if (packet_support (PACKET_qGetTLSAddr) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_qGetTLSAddr) != PACKET_DISABLE)
     {
       struct remote_state *rs = get_remote_state ();
       char *p = rs->buf.data ();
@@ -11904,7 +12005,7 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
       putpkt (rs->buf);
       getpkt (&rs->buf, 0);
       result = packet_ok (rs->buf,
-			  &remote_protocol_packets[PACKET_qGetTLSAddr]);
+			  &m_features.m_protocol_packets[PACKET_qGetTLSAddr]);
       if (result == PACKET_OK)
 	{
 	  ULONGEST addr;
@@ -11932,7 +12033,7 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
 bool
 remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
 {
-  if (packet_support (PACKET_qGetTIBAddr) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_qGetTIBAddr) != PACKET_DISABLE)
     {
       struct remote_state *rs = get_remote_state ();
       char *p = rs->buf.data ();
@@ -11947,7 +12048,7 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
       putpkt (rs->buf);
       getpkt (&rs->buf, 0);
       result = packet_ok (rs->buf,
-			  &remote_protocol_packets[PACKET_qGetTIBAddr]);
+			  &m_features.m_protocol_packets[PACKET_qGetTIBAddr]);
       if (result == PACKET_OK)
 	{
 	  ULONGEST val;
@@ -12200,7 +12301,7 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
   int ret, bytes_read;
   const char *attachment_tmp;
 
-  if (packet_support (which_packet) == PACKET_DISABLE)
+  if (m_features.packet_support (which_packet) == PACKET_DISABLE)
     {
       *remote_errno = FILEIO_ENOSYS;
       return -1;
@@ -12217,7 +12318,7 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
       return -1;
     }
 
-  switch (packet_ok (rs->buf, &remote_protocol_packets[which_packet]))
+  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[which_packet]))
     {
     case PACKET_ERROR:
       *remote_errno = FILEIO_EINVAL;
@@ -12287,7 +12388,7 @@ remote_target::remote_hostio_set_filesystem (struct inferior *inf,
   char arg[9];
   int ret;
 
-  if (packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
     return 0;
 
   if (rs->fs_pid != -1 && required_pid == rs->fs_pid)
@@ -12301,7 +12402,7 @@ remote_target::remote_hostio_set_filesystem (struct inferior *inf,
   ret = remote_hostio_send_command (p - rs->buf.data (), PACKET_vFile_setfs,
 				    remote_errno, NULL, NULL);
 
-  if (packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
     return 0;
 
   if (ret == 0)
@@ -12673,7 +12774,7 @@ remote_target::filesystem_is_local ()
      does not support vFile:open.  */
   if (gdb_sysroot == TARGET_SYSROOT_PREFIX)
     {
-      enum packet_support ps = packet_support (PACKET_vFile_open);
+      enum packet_support ps = m_features.packet_support (PACKET_vFile_open);
 
       if (ps == PACKET_SUPPORT_UNKNOWN)
 	{
@@ -12689,7 +12790,7 @@ remote_target::filesystem_is_local ()
 	  if (fd >= 0)
 	    remote_hostio_close (fd, &remote_errno);
 
-	  ps = packet_support (PACKET_vFile_open);
+	  ps = m_features.packet_support (PACKET_vFile_open);
 	}
 
       if (ps == PACKET_DISABLE)
@@ -13042,8 +13143,8 @@ remote_delete_command (const char *args, int from_tty)
 bool
 remote_target::can_execute_reverse ()
 {
-  if (packet_support (PACKET_bs) == PACKET_ENABLE
-      || packet_support (PACKET_bc) == PACKET_ENABLE)
+  if (m_features.packet_support (PACKET_bs) == PACKET_ENABLE
+      || m_features.packet_support (PACKET_bc) == PACKET_ENABLE)
     return true;
   else
     return false;
@@ -13067,56 +13168,58 @@ remote_target::supports_multi_process ()
 {
   struct remote_state *rs = get_remote_state ();
 
-  return remote_multi_process_p (rs);
+  return m_features.remote_multi_process_p (rs);
 }
 
-static int
-remote_supports_cond_tracepoints ()
+int
+remote_target::remote_supports_cond_tracepoints ()
 {
-  return packet_support (PACKET_ConditionalTracepoints) == PACKET_ENABLE;
+  return (m_features.packet_support (PACKET_ConditionalTracepoints)
+	  == PACKET_ENABLE);
 }
 
 bool
 remote_target::supports_evaluation_of_breakpoint_conditions ()
 {
-  return packet_support (PACKET_ConditionalBreakpoints) == PACKET_ENABLE;
+  return (m_features.packet_support (PACKET_ConditionalBreakpoints)
+	  == PACKET_ENABLE);
 }
 
-static int
-remote_supports_fast_tracepoints ()
+int
+remote_target::remote_supports_fast_tracepoints ()
 {
-  return packet_support (PACKET_FastTracepoints) == PACKET_ENABLE;
+  return m_features.packet_support (PACKET_FastTracepoints) == PACKET_ENABLE;
 }
 
-static int
-remote_supports_static_tracepoints ()
+int
+remote_target::remote_supports_static_tracepoints ()
 {
-  return packet_support (PACKET_StaticTracepoints) == PACKET_ENABLE;
+  return m_features.packet_support (PACKET_StaticTracepoints) == PACKET_ENABLE;
 }
 
-static int
-remote_supports_install_in_trace ()
+int
+remote_target::remote_supports_install_in_trace ()
 {
-  return packet_support (PACKET_InstallInTrace) == PACKET_ENABLE;
+  return m_features.packet_support (PACKET_InstallInTrace) == PACKET_ENABLE;
 }
 
 bool
 remote_target::supports_enable_disable_tracepoint ()
 {
-  return (packet_support (PACKET_EnableDisableTracepoints_feature)
+  return (m_features.packet_support (PACKET_EnableDisableTracepoints_feature)
 	  == PACKET_ENABLE);
 }
 
 bool
 remote_target::supports_string_tracing ()
 {
-  return packet_support (PACKET_tracenz_feature) == PACKET_ENABLE;
+  return m_features.packet_support (PACKET_tracenz_feature) == PACKET_ENABLE;
 }
 
 bool
 remote_target::can_run_breakpoint_commands ()
 {
-  return packet_support (PACKET_BreakpointCommands) == PACKET_ENABLE;
+  return m_features.packet_support (PACKET_BreakpointCommands) == PACKET_ENABLE;
 }
 
 void
@@ -13361,7 +13464,7 @@ remote_target::download_tracepoint (struct bp_location *loc)
 	error (_("Error on target while setting tracepoints."));
     }
 
-  if (packet_support (PACKET_TracepointSource) == PACKET_ENABLE)
+  if (m_features.packet_support (PACKET_TracepointSource) == PACKET_ENABLE)
     {
       if (b->location != NULL)
 	{
@@ -13517,7 +13620,8 @@ remote_target::trace_set_readonly_regions ()
       sec_length = 1 + strlen (tmp1) + 1 + strlen (tmp2);
       if (offset + sec_length + 1 > rs->buf.size ())
 	{
-	  if (packet_support (PACKET_qXfer_traceframe_info) != PACKET_ENABLE)
+	  if (m_features.packet_support (PACKET_qXfer_traceframe_info)
+	      != PACKET_ENABLE)
 	    warning (_("\
 Too many sections for read-only sections definition packet."));
 	  break;
@@ -13554,7 +13658,7 @@ remote_target::get_trace_status (struct trace_status *ts)
   enum packet_result result;
   struct remote_state *rs = get_remote_state ();
 
-  if (packet_support (PACKET_qTStatus) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_qTStatus) == PACKET_DISABLE)
     return -1;
 
   /* FIXME we need to get register block size some other way.  */
@@ -13577,7 +13681,7 @@ remote_target::get_trace_status (struct trace_status *ts)
       throw;
     }
 
-  result = packet_ok (p, &remote_protocol_packets[PACKET_qTStatus]);
+  result = packet_ok (p, &m_features.m_protocol_packets[PACKET_qTStatus]);
 
   /* If the remote target doesn't do tracing, flag it.  */
   if (result == PACKET_UNKNOWN)
@@ -13832,7 +13936,8 @@ remote_target::set_disconnected_tracing (int val)
 {
   struct remote_state *rs = get_remote_state ();
 
-  if (packet_support (PACKET_DisconnectedTracing_feature) == PACKET_ENABLE)
+  if (m_features.packet_support (PACKET_DisconnectedTracing_feature)
+      == PACKET_ENABLE)
     {
       char *reply;
 
@@ -13926,7 +14031,7 @@ remote_target::get_min_fast_tracepoint_insn_len ()
 void
 remote_target::set_trace_buffer_size (LONGEST val)
 {
-  if (packet_support (PACKET_QTBuffer_size) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_QTBuffer_size) != PACKET_DISABLE)
     {
       struct remote_state *rs = get_remote_state ();
       char *buf = rs->buf.data ();
@@ -13947,7 +14052,7 @@ remote_target::set_trace_buffer_size (LONGEST val)
       putpkt (rs->buf);
       remote_get_noisy_reply ();
       result = packet_ok (rs->buf,
-		  &remote_protocol_packets[PACKET_QTBuffer_size]);
+		  &m_features.m_protocol_packets[PACKET_QTBuffer_size]);
 
       if (result != PACKET_OK)
 	warning (_("Bogus reply from target: %s"), rs->buf.data ());
@@ -14003,7 +14108,7 @@ remote_target::set_trace_notes (const char *user, const char *notes,
 bool
 remote_target::use_agent (bool use)
 {
-  if (packet_support (PACKET_QAgent) != PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_QAgent) != PACKET_DISABLE)
     {
       struct remote_state *rs = get_remote_state ();
 
@@ -14025,7 +14130,7 @@ remote_target::use_agent (bool use)
 bool
 remote_target::can_use_agent ()
 {
-  return (packet_support (PACKET_QAgent) != PACKET_DISABLE);
+  return (m_features.packet_support (PACKET_QAgent) != PACKET_DISABLE);
 }
 
 struct btrace_target_info
@@ -14058,7 +14163,7 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
   buf = rs->buf.data ();
   endbuf = buf + get_remote_packet_size ();
 
-  packet = &remote_protocol_packets[PACKET_Qbtrace_conf_bts_size];
+  packet = &m_features.m_protocol_packets[PACKET_Qbtrace_conf_bts_size];
   if (packet_config_support (packet) == PACKET_ENABLE
       && conf->bts.size != rs->btrace_config.bts.size)
     {
@@ -14080,7 +14185,7 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
       rs->btrace_config.bts.size = conf->bts.size;
     }
 
-  packet = &remote_protocol_packets[PACKET_Qbtrace_conf_pt_size];
+  packet = &m_features.m_protocol_packets[PACKET_Qbtrace_conf_pt_size];
   if (packet_config_support (packet) == PACKET_ENABLE
       && conf->pt.size != rs->btrace_config.pt.size)
     {
@@ -14129,7 +14234,7 @@ remote_target::remote_btrace_maybe_reopen ()
 
   /* Don't bother walking the entirety of the remote thread list when
      we know the feature isn't supported by the remote.  */
-  if (packet_support (PACKET_qXfer_btrace_conf) != PACKET_ENABLE)
+  if (m_features.packet_support (PACKET_qXfer_btrace_conf) != PACKET_ENABLE)
     return;
 
   scoped_restore_current_thread restore_thread;
@@ -14189,11 +14294,11 @@ remote_target::enable_btrace (ptid_t ptid, const struct btrace_config *conf)
   switch (conf->format)
     {
       case BTRACE_FORMAT_BTS:
-	packet = &remote_protocol_packets[PACKET_Qbtrace_bts];
+	packet = &m_features.m_protocol_packets[PACKET_Qbtrace_bts];
 	break;
 
       case BTRACE_FORMAT_PT:
-	packet = &remote_protocol_packets[PACKET_Qbtrace_pt];
+	packet = &m_features.m_protocol_packets[PACKET_Qbtrace_pt];
 	break;
     }
 
@@ -14241,7 +14346,8 @@ remote_target::enable_btrace (ptid_t ptid, const struct btrace_config *conf)
 void
 remote_target::disable_btrace (struct btrace_target_info *tinfo)
 {
-  struct packet_config *packet = &remote_protocol_packets[PACKET_Qbtrace_off];
+  packet_config *packet
+    = &m_features.m_protocol_packets[PACKET_Qbtrace_off];
   struct remote_state *rs = get_remote_state ();
   char *buf = rs->buf.data ();
   char *endbuf = buf + get_remote_packet_size ();
@@ -14284,7 +14390,7 @@ remote_target::read_btrace (struct btrace_data *btrace,
 			    struct btrace_target_info *tinfo,
 			    enum btrace_read_type type)
 {
-  struct packet_config *packet = &remote_protocol_packets[PACKET_qXfer_btrace];
+  packet_config *packet = &m_features.m_protocol_packets[PACKET_qXfer_btrace];
   const char *annex;
 
   if (packet_config_support (packet) != PACKET_ENABLE)
@@ -14331,8 +14437,9 @@ remote_target::btrace_conf (const struct btrace_target_info *tinfo)
 bool
 remote_target::augmented_libraries_svr4_read ()
 {
-  return (packet_support (PACKET_augmented_libraries_svr4_read_feature)
-	  == PACKET_ENABLE);
+  return
+    (m_features.packet_support (PACKET_augmented_libraries_svr4_read_feature)
+     == PACKET_ENABLE);
 }
 
 /* Implementation of to_load.  */
@@ -14353,7 +14460,7 @@ remote_target::pid_to_exec_file (int pid)
   static gdb::optional<gdb::char_vector> filename;
   char *annex = NULL;
 
-  if (packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
+  if (m_features.packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
     return NULL;
 
   inferior *inf = find_inferior_pid (this, pid);
@@ -14385,11 +14492,11 @@ remote_target::can_do_single_step ()
      feature.  If the stub doesn't support vContSupported feature,
      we have conservatively to think target doesn't supports single
      step.  */
-  if (packet_support (PACKET_vContSupported) == PACKET_ENABLE)
+  if (m_features.packet_support (PACKET_vContSupported) == PACKET_ENABLE)
     {
       struct remote_state *rs = get_remote_state ();
 
-      if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
+      if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
 	remote_vcont_probe ();
 
       return rs->supports_vCont.s && rs->supports_vCont.S;
@@ -14529,7 +14636,7 @@ remote_target::thread_events (int enable)
   struct remote_state *rs = get_remote_state ();
   size_t size = get_remote_packet_size ();
 
-  if (packet_support (PACKET_QThreadEvents) == PACKET_DISABLE)
+  if (m_features.packet_support (PACKET_QThreadEvents) == PACKET_DISABLE)
     return;
 
   xsnprintf (rs->buf.data (), size, "QThreadEvents:%x", enable ? 1 : 0);
@@ -14537,7 +14644,7 @@ remote_target::thread_events (int enable)
   getpkt (&rs->buf, 0);
 
   switch (packet_ok (rs->buf,
-		     &remote_protocol_packets[PACKET_QThreadEvents]))
+		     &m_features.m_protocol_packets[PACKET_QThreadEvents]))
     {
     case PACKET_OK:
       if (strcmp (rs->buf.data (), "OK") != 0)
@@ -14674,10 +14781,10 @@ show_range_stepping (struct ui_file *file, int from_tty,
 bool
 remote_target::vcont_r_supported ()
 {
-  if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
+  if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
     remote_vcont_probe ();
 
-  return (packet_support (PACKET_vCont) == PACKET_ENABLE
+  return (m_features.packet_support (PACKET_vCont) == PACKET_ENABLE
 	  && get_remote_state ()->supports_vCont.r);
 }
 
@@ -14720,7 +14827,7 @@ show_remote_timeout (struct ui_file *file, int from_tty,
 bool
 remote_target::supports_memory_tagging ()
 {
-  return remote_memory_tagging_p ();
+  return m_features.remote_memory_tagging_p ();
 }
 
 /* Create the qMemTags packet given ADDRESS, LEN and TYPE.  */
@@ -14786,7 +14893,7 @@ remote_target::fetch_memtags (CORE_ADDR address, size_t len,
 			      gdb::byte_vector &tags, int type)
 {
   /* Make sure the qMemTags packet is supported.  */
-  if (!remote_memory_tagging_p ())
+  if (!m_features.remote_memory_tagging_p ())
     gdb_assert_not_reached ("remote fetch_memtags called with packet disabled");
 
   struct remote_state *rs = get_remote_state ();
@@ -14806,7 +14913,7 @@ remote_target::store_memtags (CORE_ADDR address, size_t len,
 			      const gdb::byte_vector &tags, int type)
 {
   /* Make sure the QMemTags packet is supported.  */
-  if (!remote_memory_tagging_p ())
+  if (!m_features.remote_memory_tagging_p ())
     gdb_assert_not_reached ("remote store_memtags called with packet disabled");
 
   struct remote_state *rs = get_remote_state ();
@@ -14841,7 +14948,7 @@ test_memory_tagging_functions ()
   remote_target remote;
 
   struct packet_config *config
-    = &remote_protocol_packets[PACKET_memory_tagging_feature];
+    = &remote.m_features.m_protocol_packets[PACKET_memory_tagging_feature];
 
   scoped_restore restore_memtag_support_
     = make_scoped_restore (&config->support);
diff --git a/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp b/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp
index 8b0c6c91a2..712dd0b7e5 100644
--- a/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp
+++ b/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp
@@ -30,15 +30,13 @@ set run_python_tests [expr ! [skip_python_tests]]
 # indicates whether the multi-process feature of remote targets is
 # turned off or on.
 proc test_info_inferiors {multi_process} {
-    setup "off"
+
+    setup "off" $multi_process
 
     if { $::run_python_tests } {
 	gdb_test_no_output "source ${::remote_python_file}" "load python file"
     }
 
-    gdb_test_no_output \
-	"set remote multiprocess-feature-packet $multi_process"
-
     # Get the description for inferior INF for when the current
     # inferior id is CURRENT.
     proc inf_desc {inf current} {
diff --git a/gdb/testsuite/gdb.multi/multi-target.exp.tcl b/gdb/testsuite/gdb.multi/multi-target.exp.tcl
index 8e0c39679b..f9bd6412d7 100644
--- a/gdb/testsuite/gdb.multi/multi-target.exp.tcl
+++ b/gdb/testsuite/gdb.multi/multi-target.exp.tcl
@@ -104,7 +104,7 @@ proc cleanup_gdbservers { } {
 
 # Return true on success, false otherwise.
 
-proc setup {non-stop} {
+proc setup {non-stop {multi_process ""}} {
     global gcorefile gcore_created
     global binfile
 
@@ -123,6 +123,11 @@ proc setup {non-stop} {
 
     gdb_test_no_output "set non-stop ${non-stop}"
 
+    if {${multi_process} ne ""} then {
+	gdb_test_no_output \
+	    "set remote multiprocess-feature-packet $multi_process"
+    }
+
     if ![runto all_started] then {
 	return 0
     }
-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config
  2022-01-13 15:21 [PATCH 0/3] Apply fixme notes for multi-target support Christina Schimpe
  2022-01-13 15:21 ` [PATCH 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
@ 2022-01-13 15:21 ` Christina Schimpe
  2022-01-14 19:33   ` Tom Tromey
  2022-01-18 11:39   ` Andrew Burgess
  2022-01-13 15:21 ` [PATCH 3/3] gdb: Remove workaround for the vCont packet Christina Schimpe
  2 siblings, 2 replies; 14+ messages in thread
From: Christina Schimpe @ 2022-01-13 15:21 UTC (permalink / raw)
  To: gdb-patches

This patch adds per-remote target variables for the configuration of
memory read- and write packet size. It is a further change in addition
to commit "gdb: Make global feature array a per-remote target array"
to apply the fixme notes described in commit 5b6d1e4.

The former global variables for that configuration are still available
to allow the command line configuration for all future remote
connections.  Similar to the command line configuration of the per-
remote target feature array, the commands

- set remotewritesize (deprecated)
- set remote memory-read-packet-size
- set remote memory-write-packet-size

will configure the current target (if available) and future remote
connections. The show command will display the global configuration
and the packet size of the current remote target, if available.

It is required to adapt the test gdb.base/remote.exp which is failing
for --target_board=native-extended-gdbserver.  With that board GDB
connects to gdbserver at gdb start time.  Due to this patch two loggings
"The target may not be able to.." are shown if the command 'set remote
memory-write-packet-size fixed' is executed while a target is connected
for the current inferior.  To fix this, the clean_restart command is
moved to a later time point of the test.  It is sufficient to be
connected to the server when "runto_main" is executed.  Now the
connection time is similar to a testrun with
--target_board=native-gdbserver.

To allow the user to distinguish between the packet-size configuration
for future connections and for the currently selected target, the
logging of the command 'set remote memory-write-packet-size fixed' is
adapted.
---
 gdb/remote.c                      | 94 +++++++++++++++++++------------
 gdb/testsuite/gdb.base/remote.exp |  7 ++-
 2 files changed, 61 insertions(+), 40 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 10f9226827..a270cb688c 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -563,8 +563,31 @@ struct packet_config
     enum packet_support support;
   };
 
-/* This global array contains the default configuration for every new
-   per-remote target array.  */
+/* User configurable variables for the number of characters in a
+   memory read/write packet.  MIN (rsa->remote_packet_size,
+   rsa->sizeof_g_packet) is the default.  Some targets need smaller
+   values (fifo overruns, et.al.) and some users need larger values
+   (speed up transfers).  The variables ``preferred_*'' (the user
+   request), ``current_*'' (what was actually set) and ``forced_*''
+   (Positive - a soft limit, negative - a hard limit).  */
+
+struct memory_packet_config
+{
+  const char *name;
+  long size;
+  int fixed_p;
+};
+
+/* These global variables contain the default configuration for every new
+   remote_feature object.  */
+static memory_packet_config memory_read_packet_config =
+{
+  "memory-read-packet-size",
+};
+static memory_packet_config memory_write_packet_config =
+{
+  "memory-write-packet-size",
+};
 static packet_config remote_protocol_packets[PACKET_MAX];
 
 class remote_features
@@ -573,6 +596,9 @@ class remote_features
 
   remote_features ()
   {
+    m_memory_read_packet_config = memory_read_packet_config;
+    m_memory_write_packet_config = memory_write_packet_config;
+
     std::copy (std::begin (remote_protocol_packets),
 	       std::end (remote_protocol_packets),
 	       std::begin (m_protocol_packets));
@@ -591,6 +617,9 @@ class remote_features
 
   void reset_all_packet_configs_support (void);
 
+  memory_packet_config m_memory_read_packet_config;
+  memory_packet_config m_memory_write_packet_config;
+
   packet_config m_protocol_packets[PACKET_MAX];
 };
 
@@ -1842,21 +1871,6 @@ show_remotebreak (struct ui_file *file, int from_tty,
 static unsigned int remote_address_size;
 
 \f
-/* User configurable variables for the number of characters in a
-   memory read/write packet.  MIN (rsa->remote_packet_size,
-   rsa->sizeof_g_packet) is the default.  Some targets need smaller
-   values (fifo overruns, et.al.) and some users need larger values
-   (speed up transfers).  The variables ``preferred_*'' (the user
-   request), ``current_*'' (what was actually set) and ``forced_*''
-   (Positive - a soft limit, negative - a hard limit).  */
-
-struct memory_packet_config
-{
-  const char *name;
-  long size;
-  int fixed_p;
-};
-
 /* The default max memory-write-packet-size, when the setting is
    "fixed".  The 16k is historical.  (It came from older GDB's using
    alloca for buffers and the knowledge (folklore?) that some hosts
@@ -1922,7 +1936,8 @@ remote_target::get_memory_packet_size (struct memory_packet_config *config)
    something really big then do a sanity check.  */
 
 static void
-set_memory_packet_size (const char *args, struct memory_packet_config *config)
+set_memory_packet_size (const char *args, struct memory_packet_config *config,
+			bool target_connected)
 {
   int fixed_p = config->fixed_p;
   long size = config->size;
@@ -1956,9 +1971,16 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config)
 			 ? DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED
 			 : size);
 
-      if (! query (_("The target may not be able to correctly handle a %s\n"
-		   "of %ld bytes. Change the packet size? "),
-		   config->name, query_size))
+      if (target_connected
+	  && ! query (_("The target may not be able to correctly handle a %s\n"
+		      "of %ld bytes.  Change the packet size? "),
+		      config->name, query_size))
+	error (_("Packet size not changed."));
+      else if (! target_connected
+	       && ! query (_("Future targets may not be able to correctly "
+			   "handle a %s\nof %ld bytes.  Change the packet size "
+			   "for future remote targets? "),
+			   config->name, query_size))
 	error (_("Packet size not changed."));
     }
   /* Update the config.  */
@@ -1989,16 +2011,15 @@ show_memory_packet_size (struct memory_packet_config *config)
     }
 }
 
-/* FIXME: needs to be per-remote-target.  */
-static struct memory_packet_config memory_write_packet_config =
-{
-  "memory-write-packet-size",
-};
-
 static void
 set_memory_write_packet_size (const char *args, int from_tty)
 {
-  set_memory_packet_size (args, &memory_write_packet_config);
+  remote_target *remote = get_current_remote_target ();
+  if (remote != nullptr)
+    set_memory_packet_size
+      (args, &remote->m_features.m_memory_write_packet_config, true);
+
+  set_memory_packet_size (args, &memory_write_packet_config, false);
 }
 
 static void
@@ -2060,19 +2081,18 @@ show_remote_packet_max_chars (struct ui_file *file, int from_tty,
 long
 remote_target::get_memory_write_packet_size ()
 {
-  return get_memory_packet_size (&memory_write_packet_config);
+  return get_memory_packet_size (&m_features.m_memory_write_packet_config);
 }
 
-/* FIXME: needs to be per-remote-target.  */
-static struct memory_packet_config memory_read_packet_config =
-{
-  "memory-read-packet-size",
-};
-
 static void
 set_memory_read_packet_size (const char *args, int from_tty)
 {
-  set_memory_packet_size (args, &memory_read_packet_config);
+  remote_target *remote = get_current_remote_target ();
+  if (remote != nullptr)
+    set_memory_packet_size
+      (args, &remote->m_features.m_memory_read_packet_config, true);
+
+  set_memory_packet_size (args, &memory_read_packet_config, false);
 }
 
 static void
@@ -2084,7 +2104,7 @@ show_memory_read_packet_size (const char *args, int from_tty)
 long
 remote_target::get_memory_read_packet_size ()
 {
-  long size = get_memory_packet_size (&memory_read_packet_config);
+  long size = get_memory_packet_size (&m_features.m_memory_read_packet_config);
 
   /* FIXME: cagney/1999-11-07: Functions like getpkt() need to get an
      extra buffer size argument before the memory read size can be
diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp
index 1f0869433f..31c5adfd0d 100644
--- a/gdb/testsuite/gdb.base/remote.exp
+++ b/gdb/testsuite/gdb.base/remote.exp
@@ -62,7 +62,7 @@ gdb_test "show remote memory-write-packet-size" \
 
 set test "set remote memory-write-packet-size fixed"
 gdb_test_multiple $test $test {
-    -re "Change the packet size. .y or n. " {
+    -re "Change the packet size for future remote targets. .y or n. " {
 	gdb_test_multiple "y" $test {
 	    -re "$gdb_prompt $" {
 		pass $test
@@ -70,6 +70,7 @@ gdb_test_multiple $test $test {
 	}
     }
 }
+
 gdb_test "show remote memory-write-packet-size" \
 	"The memory-write-packet-size is 0 \\(default\\). Packets are fixed at 16384 bytes\." \
 	"write-packet default fixed"
@@ -129,8 +130,6 @@ proc gdb_load_timed {executable class writesize} {
     pass $test
 }
 
-clean_restart $binfile
-
 # These download tests won't actually download anything on !is_remote
 # target boards, but we run them anyway because it's simpler, and
 # harmless.
@@ -155,6 +154,8 @@ gdb_load_timed $binfile "limit" 0
 # Get the size of random_data table (defaults to 48K).
 set sizeof_random_data [get_sizeof "random_data" 48*1024]
 
+clean_restart $binfile
+
 #
 # Part THREE: Check the upload behavour
 #
-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/3] gdb: Remove workaround for the vCont packet
  2022-01-13 15:21 [PATCH 0/3] Apply fixme notes for multi-target support Christina Schimpe
  2022-01-13 15:21 ` [PATCH 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
  2022-01-13 15:21 ` [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe
@ 2022-01-13 15:21 ` Christina Schimpe
  2022-01-14 19:42   ` Tom Tromey
  2 siblings, 1 reply; 14+ messages in thread
From: Christina Schimpe @ 2022-01-13 15:21 UTC (permalink / raw)
  To: gdb-patches

The workaround for the vCont packet is no longer required due to the
former commit "gdb: Make global feature array a per-remote target array".
The vCont packet is now checked once when the connection is started and
the supported vCont actions are set to the target's remote state
attribute.
---
 gdb/remote.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index a270cb688c..4024801559 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -422,9 +422,6 @@ class remote_state
 
   /* The status of the stub support for the various vCont actions.  */
   vCont_action_support supports_vCont;
-  /* Whether vCont support was probed already.  This is a workaround
-     until packet_support is per-connection.  */
-  bool supports_vCont_probed;
 
   /* True if the user has pressed Ctrl-C, but the target hasn't
      responded to that.  */
@@ -4859,6 +4856,10 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
      which later probes to skip.  */
   remote_query_supported ();
 
+  /* Check vCont support and set the remote state's vCont_action_support
+     attribute.  */
+  remote_vcont_probe ();
+
   /* If the stub wants to get a QAllow, compose one and send it.  */
   if (m_features.packet_support (PACKET_QAllow) != PACKET_DISABLE)
     set_permissions ();
@@ -6378,7 +6379,6 @@ remote_target::remote_vcont_probe ()
     }
 
   packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vCont]);
-  rs->supports_vCont_probed = true;
 }
 
 /* Helper function for building "vCont" resumptions.  Write a
@@ -6568,9 +6568,6 @@ remote_target::remote_resume_with_vcont (ptid_t ptid, int step,
   if (::execution_direction == EXEC_REVERSE)
     return 0;
 
-  if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
-    remote_vcont_probe ();
-
   if (m_features.packet_support (PACKET_vCont) == PACKET_DISABLE)
     return 0;
 
@@ -7112,12 +7109,6 @@ remote_target::remote_stop_ns (ptid_t ptid)
 	  }
       }
 
-  /* FIXME: This supports_vCont_probed check is a workaround until
-     packet_support is per-connection.  */
-  if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN
-      || !rs->supports_vCont_probed)
-    remote_vcont_probe ();
-
   if (!rs->supports_vCont.t)
     error (_("Remote server does not support stopping threads"));
 
@@ -14516,9 +14507,6 @@ remote_target::can_do_single_step ()
     {
       struct remote_state *rs = get_remote_state ();
 
-      if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
-	remote_vcont_probe ();
-
       return rs->supports_vCont.s && rs->supports_vCont.S;
     }
   else
@@ -14801,9 +14789,6 @@ show_range_stepping (struct ui_file *file, int from_tty,
 bool
 remote_target::vcont_r_supported ()
 {
-  if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
-    remote_vcont_probe ();
-
   return (m_features.packet_support (PACKET_vCont) == PACKET_ENABLE
 	  && get_remote_state ()->supports_vCont.r);
 }
-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] gdb: Make global feature array a per-remote target array
  2022-01-13 15:21 ` [PATCH 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
@ 2022-01-14 19:30   ` Tom Tromey
  2022-01-20 16:32     ` Schimpe, Christina
  2022-01-18 11:30   ` Andrew Burgess
  1 sibling, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2022-01-14 19:30 UTC (permalink / raw)
  To: Christina Schimpe via Gdb-patches

>>>>> ">" == Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org> writes:

>> This patch applies the appropriate FIXME notes described in commit
>> 5b6d1e4 "Multi-target support".

Thank you for the patch.  I have a couple of small things, but nothing
too serious.  On the whole, this approach makes sense to me.

>> +  int remote_multi_process_p (remote_state *rs) const;
>> +  int remote_fork_event_p (remote_state*) const;
>> +  int remote_vfork_event_p (remote_state*) const;
>> +  int remote_exec_event_p (remote_state*) const;

It seems like these methods don't actually need the 'remote_state *' parameter.

>> +  void reset_all_packet_configs_support (void);

New code shouldn't use "(void)", just "()" is fine.  Also, normally if
we touch a function definition or declaration, we'll also remove a
'void' if there is one.

>> +static void
>> +set_remote_protocol_packet_cmd (const char *args, int from_tty,
>> +				cmd_list_element *c)

>> +   for (int i = 0; i < PACKET_MAX; ++i)
>> +    {
>> +       if (c == remote_protocol_packets[i].set_cmd)
>> +	 {

The packet number can be passed in using the 'context' attached to the
command.  See cmd_list_element::set_context and cmd_list_element::context.
This is simple to do and would remove the need for this loop.

It's mildly annoying that this only accepts a pointer and not a scalar,
but it can either be cast or you could pass the address the slot in
remote_protocol_packets and then do some pointer math to reconstruct the
index.

The same applies to the 'show' command.

thanks,
Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config
  2022-01-13 15:21 ` [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe
@ 2022-01-14 19:33   ` Tom Tromey
  2022-01-18 11:39   ` Andrew Burgess
  1 sibling, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2022-01-14 19:33 UTC (permalink / raw)
  To: Christina Schimpe via Gdb-patches

>>>>> ">" == Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org> writes:

>> This patch adds per-remote target variables for the configuration of
>> memory read- and write packet size. It is a further change in addition
>> to commit "gdb: Make global feature array a per-remote target array"
>> to apply the fixme notes described in commit 5b6d1e4.

Thank you.

This patch looks good to me.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] gdb: Remove workaround for the vCont packet
  2022-01-13 15:21 ` [PATCH 3/3] gdb: Remove workaround for the vCont packet Christina Schimpe
@ 2022-01-14 19:42   ` Tom Tromey
  2022-01-28 13:28     ` Schimpe, Christina
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2022-01-14 19:42 UTC (permalink / raw)
  To: Christina Schimpe via Gdb-patches

>>>>> ">" == Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org> writes:

>> The workaround for the vCont packet is no longer required due to the
>> former commit "gdb: Make global feature array a per-remote target array".
>> The vCont packet is now checked once when the connection is started and
>> the supported vCont actions are set to the target's remote state
>> attribute.

Thank you for the patch.

>> +  /* Check vCont support and set the remote state's vCont_action_support
>> +     attribute.  */
>> +  remote_vcont_probe ();
 
>> -  if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
>> -    remote_vcont_probe ();

It seems like the new code should probably check whether the packet is
supported like the old code did, in case the gdb user disabled it using
the appropriate "set remote" command.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] gdb: Make global feature array a per-remote target array
  2022-01-13 15:21 ` [PATCH 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
  2022-01-14 19:30   ` Tom Tromey
@ 2022-01-18 11:30   ` Andrew Burgess
  2022-01-20 17:24     ` Schimpe, Christina
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2022-01-18 11:30 UTC (permalink / raw)
  To: Christina Schimpe; +Cc: gdb-patches

* Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org> [2022-01-13 16:21:16 +0100]:

> This patch applies the appropriate FIXME notes described in commit
> 5b6d1e4 "Multi-target support".
> 
> "You'll notice that remote.c includes some FIXME notes.  These refer to
> the fact that the global arrays that hold data for the remote packets
> supported are still globals.  For example, if we connect to two
> different servers/stubs, then each might support different remote
> protocol features.  They might even be different architectures, like
> e.g., one ARM baremetal stub, and a x86 gdbserver, to debug a
> host/controller scenario as a single program.  That isn't going to
> work correctly today, because of said globals.  I'm leaving fixing
> that for another pass, since it does not appear to be trivial, and I'd
> rather land the base work first.  It's already useful to be able to
> debug multiple instances of the same server (e.g., a distributed
> cluster, where you have full control over the servers installed), so I
> think as is it's already reasonable incremental progress."
> 
> Using this patch it is possible to configure per-remote targets'
> feature packets.
> 
> Given the following setup for two gdbservers:
> 
> ~~~~
> gdbserver --multi :1234
> gdbserver --disable-packet=vCont --multi :2345
> ~~~~
> 
> Before this patch configuring of range-stepping was not possible for one
> of two connected remote targets with different support for the vCont
> packet.  As one of the target supports vCont, it should be possible to
> configure "set range-stepping".  However, the output of GDB looks like:
> 
> (gdb) target extended-remote :1234
> Remote debugging using :1234
> (gdb) add-inferior -no-connection
> [New inferior 2]
> Added inferior 2
> (gdb) inferior 2
> [Switching to inferior 2 [<null>] (<noexec>)]
> (gdb) target extended-remote :2345
> Remote debugging using :2345
> (gdb) set range-stepping

Could you update your examples to not use implicit values here
please.  This just makes reading these that little bit harder for no
reason.

> warning: Range stepping is not supported by the current target
> (gdb) inferior 1
> [Switching to inferior 1 [<null>] (<noexec>)]
> (gdb) set range-stepping
> warning: Range stepping is not supported by the current target
> ~~~~
> 
> Two warnings are shown.  The warning for inferior 1 should not appear
> as it is connected to a target supporting the vCont package.
> 
> ~~~~
> (gdb) target extended-remote :1234
> Remote debugging using :1234
> (gdb) add-inferior -no-connection
> [New inferior 2]
> Added inferior 2
> (gdb) inferior 2
> [Switching to inferior 2 [<null>] (<noexec>)]
> (gdb) target extended-remote :2345
> Remote debugging using :2345
> (gdb) set range-stepping
> warning: Range stepping is not supported by the current target
> (gdb) inferior 1
> [Switching to inferior 1 [<null>] (<noexec>)]
> (gdb) set range-stepping
> (gdb)
> ~~~~
> 
> Now only one warning is shown for inferior 2, which is connected to
> a target not supporting vCont.
> 
> The per-remote target feature array is realized by a new class
> remote_features, which stores the per-remote target array and
> provides functions to determine supported features of the target.
> A remote_target object now has a new member of that class.
> 
> Each time a new remote_target object is initialized, a new per-remote
> target array is constructed based on the global remote_protocol_packets
> array.  The global array is initialized in the function _initialize_remote
> and can be configured using the command line.  However, the command line
> configuration before this patch affected also existing remote connections
> (due to the global feature array used by all remote targets).  This
> behavior is now different.  Now the currently selected target's feature
> array will be configured and future connections' feature packets.  All
> other existing remote targets' features are not affected.
> The show command always displays the current remote target's
> configuration.  If no remote target is selected the default
> configuration for future connections is shown.

Initially I didn't really like this approach, not for any particular
reason, it just seemed a little ... weird I guess.

But having thought about this for a couple of days, I don't have any
better suggestions.  I did consider schemes with extra settings, and
new flags to the existing commands, but it all got really
complicated.  In the end I think what you've proposed is probably the
simplest to understand.

However, I do think there are some things we could do to make it clear
what's going on, I think that the string printed for 'show remote
PACKET-NAME' should be updated to reflect what it's showing, as in:

  (gdb) show remote kill-packet
  Support for the `vKill' packet on the current remote target, is "auto" (currently unknown).

OR

  (gdb) show remote kill-packet
  Support for the `vKill' packet on newly created remote targets, is "auto".

Notice, I dropped the 'currently unknown' as that makes no sense when
talking about the future targets.

I also think we should have the set command print something to tell
the user what changed, so they can know if they messed up, maybe:

 (gdb) set remote kill-packet off
 Use of the 'vKill' packet for the current, and future remote targets, set to "off".

OR

 (gdb) set remote kill-packet off
 Use of the 'vKill' packet for future remote targets, set to "off".

> 
> If we have for instance the following setup with inferior 2 being
> selected:
> ~~~~
> (gdb) info inferiors
>   Num  Description       Connection                Executable
>   1    <null>             1 (extended-remote :1234)
> * 2    <null>             2 (extended-remote :2345)
> ~~~~
> 
> Before this patch, if we run 'set remote multiprocess-feature-packet', the
> following configuration was set:
> The feature array of all remote targets (in this setup the two connected
> targets) and all future remote connections are affected.
> 
> After this patch, it will be configured as follows:
> The feature array of target with port :2345 which is currently selected
> and all future remote connections are affected. The show command 'show
> remote multiprocess-feature-packet' will display the configuration of
> target with port :2345.
> 
> It is therefore required to adapt the test
> "gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp" to configure the
> multiprocess-feature-packet before the connections are created.
> ---
>  gdb/remote.c                                  | 899 ++++++++++--------
>  .../gdb.multi/multi-target-info-inferiors.exp |   6 +-
>  gdb/testsuite/gdb.multi/multi-target.exp.tcl  |   7 +-

This change absolutely needs a NEWS entry and also a documentation
update.

With one exception the rest of my feedback is basically: there should
be more comments added.  I know that sucks, especially when working in
older files like remote.c, where the initial code is poorly commented
anyway, but for me good comments do help understand what the intention
of a field or struct is, so I think, when we change stuff, we should
take the opportunity to document what's going on.  So...

>  3 files changed, 511 insertions(+), 401 deletions(-)
> 
> diff --git a/gdb/remote.c b/gdb/remote.c
> index b126532af4..10f9226827 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -71,7 +71,6 @@
>  #include "gdbsupport/agent.h"
>  #include "btrace.h"
>  #include "record-btrace.h"
> -#include <algorithm>
>  #include "gdbsupport/scoped_restore.h"
>  #include "gdbsupport/environ.h"
>  #include "gdbsupport/byte-vector.h"
> @@ -127,6 +126,153 @@ enum packet_result
>    PACKET_UNKNOWN
>  };
>  
> +enum {

Top level entities should have a comment attached explaining what they
represent.

> +  PACKET_vCont = 0,
> +  PACKET_X,
> +  PACKET_qSymbol,
> +  PACKET_P,
> +  PACKET_p,
> +  PACKET_Z0,
> +  PACKET_Z1,
> +  PACKET_Z2,
> +  PACKET_Z3,
> +  PACKET_Z4,
> +  PACKET_vFile_setfs,
> +  PACKET_vFile_open,
> +  PACKET_vFile_pread,
> +  PACKET_vFile_pwrite,
> +  PACKET_vFile_close,
> +  PACKET_vFile_unlink,
> +  PACKET_vFile_readlink,
> +  PACKET_vFile_fstat,
> +  PACKET_qXfer_auxv,
> +  PACKET_qXfer_features,
> +  PACKET_qXfer_exec_file,
> +  PACKET_qXfer_libraries,
> +  PACKET_qXfer_libraries_svr4,
> +  PACKET_qXfer_memory_map,
> +  PACKET_qXfer_osdata,
> +  PACKET_qXfer_threads,
> +  PACKET_qXfer_statictrace_read,
> +  PACKET_qXfer_traceframe_info,
> +  PACKET_qXfer_uib,
> +  PACKET_qGetTIBAddr,
> +  PACKET_qGetTLSAddr,
> +  PACKET_qSupported,
> +  PACKET_qTStatus,
> +  PACKET_QPassSignals,
> +  PACKET_QCatchSyscalls,
> +  PACKET_QProgramSignals,
> +  PACKET_QSetWorkingDir,
> +  PACKET_QStartupWithShell,
> +  PACKET_QEnvironmentHexEncoded,
> +  PACKET_QEnvironmentReset,
> +  PACKET_QEnvironmentUnset,
> +  PACKET_qCRC,
> +  PACKET_qSearch_memory,
> +  PACKET_vAttach,
> +  PACKET_vRun,
> +  PACKET_QStartNoAckMode,
> +  PACKET_vKill,
> +  PACKET_qXfer_siginfo_read,
> +  PACKET_qXfer_siginfo_write,
> +  PACKET_qAttached,
> +
> +  /* Support for conditional tracepoints.  */
> +  PACKET_ConditionalTracepoints,
> +
> +  /* Support for target-side breakpoint conditions.  */
> +  PACKET_ConditionalBreakpoints,
> +
> +  /* Support for target-side breakpoint commands.  */
> +  PACKET_BreakpointCommands,
> +
> +  /* Support for fast tracepoints.  */
> +  PACKET_FastTracepoints,
> +
> +  /* Support for static tracepoints.  */
> +  PACKET_StaticTracepoints,
> +
> +  /* Support for installing tracepoints while a trace experiment is
> +     running.  */
> +  PACKET_InstallInTrace,
> +
> +  PACKET_bc,
> +  PACKET_bs,
> +  PACKET_TracepointSource,
> +  PACKET_QAllow,
> +  PACKET_qXfer_fdpic,
> +  PACKET_QDisableRandomization,
> +  PACKET_QAgent,
> +  PACKET_QTBuffer_size,
> +  PACKET_Qbtrace_off,
> +  PACKET_Qbtrace_bts,
> +  PACKET_Qbtrace_pt,
> +  PACKET_qXfer_btrace,
> +
> +  /* Support for the QNonStop packet.  */
> +  PACKET_QNonStop,
> +
> +  /* Support for the QThreadEvents packet.  */
> +  PACKET_QThreadEvents,
> +
> +  /* Support for multi-process extensions.  */
> +  PACKET_multiprocess_feature,
> +
> +  /* Support for enabling and disabling tracepoints while a trace
> +     experiment is running.  */
> +  PACKET_EnableDisableTracepoints_feature,
> +
> +  /* Support for collecting strings using the tracenz bytecode.  */
> +  PACKET_tracenz_feature,
> +
> +  /* Support for continuing to run a trace experiment while GDB is
> +     disconnected.  */
> +  PACKET_DisconnectedTracing_feature,
> +
> +  /* Support for qXfer:libraries-svr4:read with a non-empty annex.  */
> +  PACKET_augmented_libraries_svr4_read_feature,
> +
> +  /* Support for the qXfer:btrace-conf:read packet.  */
> +  PACKET_qXfer_btrace_conf,
> +
> +  /* Support for the Qbtrace-conf:bts:size packet.  */
> +  PACKET_Qbtrace_conf_bts_size,
> +
> +  /* Support for swbreak+ feature.  */
> +  PACKET_swbreak_feature,
> +
> +  /* Support for hwbreak+ feature.  */
> +  PACKET_hwbreak_feature,
> +
> +  /* Support for fork events.  */
> +  PACKET_fork_event_feature,
> +
> +  /* Support for vfork events.  */
> +  PACKET_vfork_event_feature,
> +
> +  /* Support for the Qbtrace-conf:pt:size packet.  */
> +  PACKET_Qbtrace_conf_pt_size,
> +
> +  /* Support for exec events.  */
> +  PACKET_exec_event_feature,
> +
> +  /* Support for query supported vCont actions.  */
> +  PACKET_vContSupported,
> +
> +  /* Support remote CTRL-C.  */
> +  PACKET_vCtrlC,
> +
> +  /* Support TARGET_WAITKIND_NO_RESUMED.  */
> +  PACKET_no_resumed,
> +
> +  /* Support for memory tagging, allocation tag fetch/store
> +     packets and the tag violation stop replies.  */
> +  PACKET_memory_tagging_feature,
> +
> +  PACKET_MAX
> +};
> +
>  struct threads_listing_context;
>  
>  /* Stub vCont actions support.
> @@ -395,6 +541,59 @@ static const target_info remote_target_info = {
>    remote_doc
>  };
>  
> +struct packet_config
> +  {
> +    const char *name;
> +    const char *title;

Again, this struct should have a top level comment, and each of the
fields should have a comment too.

> +
> +    /* If auto, GDB auto-detects support for this packet or feature,
> +       either through qSupported, or by trying the packet and looking
> +       at the response.  If true, GDB assumes the target supports this
> +       packet.  If false, the packet is disabled.  Configs that don't
> +       have an associated command always have this set to auto.  */
> +    enum auto_boolean detect;
> +
> +    /* The "show remote foo-packet" command created for this packet.  */
> +    cmd_list_element *show_cmd;
> +
> +    /* The "set remote foo-packet" command created for this packet.  */
> +    cmd_list_element *set_cmd;
> +
> +    /* Does the target support this packet?  */
> +    enum packet_support support;
> +  };
> +
> +/* This global array contains the default configuration for every new
> +   per-remote target array.  */
> +static packet_config remote_protocol_packets[PACKET_MAX];
> +
> +class remote_features
> +{
> +public:

Just declare this as 'struct remote_features' and remove the initial
'public:'.  Also header comment.

> +
> +  remote_features ()
> +  {
> +    std::copy (std::begin (remote_protocol_packets),
> +	       std::end (remote_protocol_packets),
> +	       std::begin (m_protocol_packets));
> +  }
> +  ~remote_features () = default;
> +
> +  DISABLE_COPY_AND_ASSIGN (remote_features);
> +
> +  enum packet_support packet_support (int) const;
> +  enum auto_boolean packet_set_cmd_state (int) const;
> +  int remote_multi_process_p (remote_state *rs) const;
> +  int remote_fork_event_p (remote_state*) const;
> +  int remote_vfork_event_p (remote_state*) const;
> +  int remote_exec_event_p (remote_state*) const;
> +  bool remote_memory_tagging_p () const;

I see you left all the comments for these functions at their
definitions, but I think the GDB style is to have the comments on the
declarations.

Most of these functions are so small they could probably be placed
inline here anyway, which might be nicer.

Thanks,
Andrew

> +
> +  void reset_all_packet_configs_support (void);
> +
> +  packet_config m_protocol_packets[PACKET_MAX];
> +};
> +
>  class remote_target : public process_stratum_target
>  {
>  public:
> @@ -585,8 +784,16 @@ class remote_target : public process_stratum_target
>  
>    bool supports_string_tracing () override;
>  
> +  int remote_supports_cond_tracepoints ();
> +
>    bool supports_evaluation_of_breakpoint_conditions () override;
>  
> +  int remote_supports_fast_tracepoints ();
> +
> +  int remote_supports_static_tracepoints ();
> +
> +  int remote_supports_install_in_trace ();
> +
>    bool can_run_breakpoint_commands () override;
>  
>    void trace_init () override;
> @@ -946,6 +1153,8 @@ class remote_target : public process_stratum_target
>  
>    bool vcont_r_supported ();
>  
> +  remote_features m_features;
> +
>  private:
>  
>    bool start_remote_1 (int from_tty, int extended_p);
> @@ -1061,7 +1270,8 @@ static CORE_ADDR remote_address_masked (CORE_ADDR);
>  
>  static int stub_unpack_int (const char *buff, int fieldlength);
>  
> -struct packet_config;
> +static void show_packet_config_cmd (ui_file *file,
> +				    struct packet_config *config);
>  
>  static void show_remote_protocol_packet_cmd (struct ui_file *file,
>  					     int from_tty,
> @@ -1884,29 +2094,31 @@ remote_target::get_memory_read_packet_size ()
>    return size;
>  }
>  
> -\f
> +static enum packet_support packet_config_support
> +  (const packet_config *config);
>  
> -struct packet_config
> -  {
> -    const char *name;
> -    const char *title;
> -
> -    /* If auto, GDB auto-detects support for this packet or feature,
> -       either through qSupported, or by trying the packet and looking
> -       at the response.  If true, GDB assumes the target supports this
> -       packet.  If false, the packet is disabled.  Configs that don't
> -       have an associated command always have this set to auto.  */
> -    enum auto_boolean detect;
> -
> -    /* The "show remote foo-packet" command created for this packet.  */
> -    cmd_list_element *show_cmd;
>  
> -    /* Does the target support this packet?  */
> -    enum packet_support support;
> -  };
> +static void
> +set_remote_protocol_packet_cmd (const char *args, int from_tty,
> +				cmd_list_element *c)
> +{
> +  remote_target *remote = get_current_remote_target ();
> +  gdb_assert (c->var.has_value ());
>  
> -static enum packet_support packet_config_support (struct packet_config *config);
> -static enum packet_support packet_support (int packet);
> +   for (int i = 0; i < PACKET_MAX; ++i)
> +    {
> +       if (c == remote_protocol_packets[i].set_cmd)
> +	 {
> +	    enum auto_boolean value = c->var->get<enum auto_boolean> ();
> +	    if (remote != nullptr)
> +	      remote->m_features.m_protocol_packets[i].detect = value;
> +	    remote_protocol_packets[i].detect = value;
> +	    return;
> +	 }
> +    }
> +  internal_error (__FILE__, __LINE__, _("Could not find config for %s"),
> +		  c->name);
> +}
>  
>  static void
>  show_packet_config_cmd (ui_file *file, struct packet_config *config)
> @@ -1960,10 +2172,11 @@ add_packet_config_cmd (struct packet_config *config, const char *name,
>      = add_setshow_auto_boolean_cmd (cmd_name.release (), class_obscure,
>  				    &config->detect, set_doc.get (),
>  				    show_doc.get (), NULL, /* help_doc */
> -				    NULL,
> +				    set_remote_protocol_packet_cmd,
>  				    show_remote_protocol_packet_cmd,
>  				    &remote_set_cmdlist, &remote_show_cmdlist);
>    config->show_cmd = cmds.show;
> +  config->set_cmd = cmds.set;
>  
>    /* set/show remote NAME-packet {auto,on,off} -- legacy.  */
>    if (legacy)
> @@ -2067,165 +2280,13 @@ packet_ok (const gdb::char_vector &buf, struct packet_config *config)
>    return packet_ok (buf.data (), config);
>  }
>  
> -enum {
> -  PACKET_vCont = 0,
> -  PACKET_X,
> -  PACKET_qSymbol,
> -  PACKET_P,
> -  PACKET_p,
> -  PACKET_Z0,
> -  PACKET_Z1,
> -  PACKET_Z2,
> -  PACKET_Z3,
> -  PACKET_Z4,
> -  PACKET_vFile_setfs,
> -  PACKET_vFile_open,
> -  PACKET_vFile_pread,
> -  PACKET_vFile_pwrite,
> -  PACKET_vFile_close,
> -  PACKET_vFile_unlink,
> -  PACKET_vFile_readlink,
> -  PACKET_vFile_fstat,
> -  PACKET_qXfer_auxv,
> -  PACKET_qXfer_features,
> -  PACKET_qXfer_exec_file,
> -  PACKET_qXfer_libraries,
> -  PACKET_qXfer_libraries_svr4,
> -  PACKET_qXfer_memory_map,
> -  PACKET_qXfer_osdata,
> -  PACKET_qXfer_threads,
> -  PACKET_qXfer_statictrace_read,
> -  PACKET_qXfer_traceframe_info,
> -  PACKET_qXfer_uib,
> -  PACKET_qGetTIBAddr,
> -  PACKET_qGetTLSAddr,
> -  PACKET_qSupported,
> -  PACKET_qTStatus,
> -  PACKET_QPassSignals,
> -  PACKET_QCatchSyscalls,
> -  PACKET_QProgramSignals,
> -  PACKET_QSetWorkingDir,
> -  PACKET_QStartupWithShell,
> -  PACKET_QEnvironmentHexEncoded,
> -  PACKET_QEnvironmentReset,
> -  PACKET_QEnvironmentUnset,
> -  PACKET_qCRC,
> -  PACKET_qSearch_memory,
> -  PACKET_vAttach,
> -  PACKET_vRun,
> -  PACKET_QStartNoAckMode,
> -  PACKET_vKill,
> -  PACKET_qXfer_siginfo_read,
> -  PACKET_qXfer_siginfo_write,
> -  PACKET_qAttached,
> -
> -  /* Support for conditional tracepoints.  */
> -  PACKET_ConditionalTracepoints,
> -
> -  /* Support for target-side breakpoint conditions.  */
> -  PACKET_ConditionalBreakpoints,
> -
> -  /* Support for target-side breakpoint commands.  */
> -  PACKET_BreakpointCommands,
> -
> -  /* Support for fast tracepoints.  */
> -  PACKET_FastTracepoints,
> -
> -  /* Support for static tracepoints.  */
> -  PACKET_StaticTracepoints,
> -
> -  /* Support for installing tracepoints while a trace experiment is
> -     running.  */
> -  PACKET_InstallInTrace,
> -
> -  PACKET_bc,
> -  PACKET_bs,
> -  PACKET_TracepointSource,
> -  PACKET_QAllow,
> -  PACKET_qXfer_fdpic,
> -  PACKET_QDisableRandomization,
> -  PACKET_QAgent,
> -  PACKET_QTBuffer_size,
> -  PACKET_Qbtrace_off,
> -  PACKET_Qbtrace_bts,
> -  PACKET_Qbtrace_pt,
> -  PACKET_qXfer_btrace,
> -
> -  /* Support for the QNonStop packet.  */
> -  PACKET_QNonStop,
> -
> -  /* Support for the QThreadEvents packet.  */
> -  PACKET_QThreadEvents,
> -
> -  /* Support for multi-process extensions.  */
> -  PACKET_multiprocess_feature,
> -
> -  /* Support for enabling and disabling tracepoints while a trace
> -     experiment is running.  */
> -  PACKET_EnableDisableTracepoints_feature,
> -
> -  /* Support for collecting strings using the tracenz bytecode.  */
> -  PACKET_tracenz_feature,
> -
> -  /* Support for continuing to run a trace experiment while GDB is
> -     disconnected.  */
> -  PACKET_DisconnectedTracing_feature,
> -
> -  /* Support for qXfer:libraries-svr4:read with a non-empty annex.  */
> -  PACKET_augmented_libraries_svr4_read_feature,
> -
> -  /* Support for the qXfer:btrace-conf:read packet.  */
> -  PACKET_qXfer_btrace_conf,
> -
> -  /* Support for the Qbtrace-conf:bts:size packet.  */
> -  PACKET_Qbtrace_conf_bts_size,
> -
> -  /* Support for swbreak+ feature.  */
> -  PACKET_swbreak_feature,
> -
> -  /* Support for hwbreak+ feature.  */
> -  PACKET_hwbreak_feature,
> -
> -  /* Support for fork events.  */
> -  PACKET_fork_event_feature,
> -
> -  /* Support for vfork events.  */
> -  PACKET_vfork_event_feature,
> -
> -  /* Support for the Qbtrace-conf:pt:size packet.  */
> -  PACKET_Qbtrace_conf_pt_size,
> -
> -  /* Support for exec events.  */
> -  PACKET_exec_event_feature,
> -
> -  /* Support for query supported vCont actions.  */
> -  PACKET_vContSupported,
> -
> -  /* Support remote CTRL-C.  */
> -  PACKET_vCtrlC,
> -
> -  /* Support TARGET_WAITKIND_NO_RESUMED.  */
> -  PACKET_no_resumed,
> -
> -  /* Support for memory tagging, allocation tag fetch/store
> -     packets and the tag violation stop replies.  */
> -  PACKET_memory_tagging_feature,
> -
> -  PACKET_MAX
> -};
> -
> -/* FIXME: needs to be per-remote-target.  Ignoring this for now,
> -   assuming all remote targets are the same server (thus all support
> -   the same packets).  */
> -static struct packet_config remote_protocol_packets[PACKET_MAX];
> -
>  /* Returns the packet's corresponding "set remote foo-packet" command
>     state.  See struct packet_config for more details.  */
>  
> -static enum auto_boolean
> -packet_set_cmd_state (int packet)
> +enum auto_boolean
> +remote_features::packet_set_cmd_state (int packet) const
>  {
> -  return remote_protocol_packets[packet].detect;
> +  return m_protocol_packets[packet].detect;
>  }
>  
>  /* Returns whether a given packet or feature is supported.  This takes
> @@ -2233,7 +2294,7 @@ packet_set_cmd_state (int packet)
>     command, which may be used to bypass auto-detection.  */
>  
>  static enum packet_support
> -packet_config_support (struct packet_config *config)
> +packet_config_support (const packet_config *config)
>  {
>    switch (config->detect)
>      {
> @@ -2251,10 +2312,10 @@ packet_config_support (struct packet_config *config)
>  /* Same as packet_config_support, but takes the packet's enum value as
>     argument.  */
>  
> -static enum packet_support
> -packet_support (int packet)
> +enum packet_support
> +remote_features::packet_support (int packet) const
>  {
> -  struct packet_config *config = &remote_protocol_packets[packet];
> +  const packet_config *config = &m_protocol_packets[packet];
>  
>    return packet_config_support (config);
>  }
> @@ -2264,16 +2325,18 @@ show_remote_protocol_packet_cmd (struct ui_file *file, int from_tty,
>  				 struct cmd_list_element *c,
>  				 const char *value)
>  {
> -  struct packet_config *packet;
> +  remote_target *remote = get_current_remote_target ();
>    gdb_assert (c->var.has_value ());
>  
> -  for (packet = remote_protocol_packets;
> -       packet < &remote_protocol_packets[PACKET_MAX];
> -       packet++)
> +   for (int i = 0; i < PACKET_MAX; ++i)
>      {
> -      if (c == packet->show_cmd)
> +      if (c == remote_protocol_packets[i].show_cmd)
>  	{
> -	  show_packet_config_cmd (file, packet);
> +	  if (remote != nullptr)
> +	    show_packet_config_cmd
> +	      (file, &remote->m_features.m_protocol_packets[i]);
> +	  else
> +	    show_packet_config_cmd (file, &remote_protocol_packets[i]);
>  	  return;
>  	}
>      }
> @@ -2302,10 +2365,16 @@ static void
>  set_remote_protocol_Z_packet_cmd (const char *args, int from_tty,
>  				  struct cmd_list_element *c)
>  {
> +  remote_target *remote = get_current_remote_target ();
>    int i;
>  
>    for (i = 0; i < NR_Z_PACKET_TYPES; i++)
> -    remote_protocol_packets[PACKET_Z0 + i].detect = remote_Z_packet_detect;
> +    {
> +      if (remote != nullptr)
> +	remote->m_features.m_protocol_packets[PACKET_Z0 + i].detect
> +	  = remote_Z_packet_detect;
> +      remote_protocol_packets[PACKET_Z0 + i].detect = remote_Z_packet_detect;
> +    }
>  }
>  
>  static void
> @@ -2313,50 +2382,55 @@ show_remote_protocol_Z_packet_cmd (struct ui_file *file, int from_tty,
>  				   struct cmd_list_element *c,
>  				   const char *value)
>  {
> +  remote_target *remote = get_current_remote_target ();
>    int i;
>  
>    for (i = 0; i < NR_Z_PACKET_TYPES; i++)
>      {
> -      show_packet_config_cmd (file, &remote_protocol_packets[PACKET_Z0 + i]);
> +      if (remote != nullptr)
> +	show_packet_config_cmd
> +	  (file, &remote->m_features.m_protocol_packets[PACKET_Z0 + i]);
> +      else
> +	show_packet_config_cmd (file, &remote_protocol_packets[PACKET_Z0 + i]);
>      }
>  }
>  
>  /* Returns true if the multi-process extensions are in effect.  */
>  
> -static int
> -remote_multi_process_p (struct remote_state *rs)
> +int
> +remote_features::remote_multi_process_p (remote_state *rs) const
>  {
>    return packet_support (PACKET_multiprocess_feature) == PACKET_ENABLE;
>  }
>  
>  /* Returns true if fork events are supported.  */
>  
> -static int
> -remote_fork_event_p (struct remote_state *rs)
> +int
> +remote_features::remote_fork_event_p (remote_state *rs) const
>  {
>    return packet_support (PACKET_fork_event_feature) == PACKET_ENABLE;
>  }
>  
>  /* Returns true if vfork events are supported.  */
>  
> -static int
> -remote_vfork_event_p (struct remote_state *rs)
> +int
> +remote_features::remote_vfork_event_p (remote_state *rs) const
>  {
>    return packet_support (PACKET_vfork_event_feature) == PACKET_ENABLE;
>  }
>  
>  /* Returns true if exec events are supported.  */
>  
> -static int
> -remote_exec_event_p (struct remote_state *rs)
> +int
> +remote_features::remote_exec_event_p (remote_state *rs) const
>  {
>    return packet_support (PACKET_exec_event_feature) == PACKET_ENABLE;
>  }
>  
>  /* Returns true if memory tagging is supported, false otherwise.  */
>  
> -static bool
> -remote_memory_tagging_p ()
> +bool
> +remote_features::remote_memory_tagging_p () const
>  {
>    return packet_support (PACKET_memory_tagging_feature) == PACKET_ENABLE;
>  }
> @@ -2367,9 +2441,9 @@ remote_memory_tagging_p ()
>  int
>  remote_target::insert_fork_catchpoint (int pid)
>  {
> -  struct remote_state *rs = get_remote_state ();
> +  remote_state *rs = get_remote_state ();
>  
> -  return !remote_fork_event_p (rs);
> +  return !m_features.remote_fork_event_p (rs);
>  }
>  
>  /* Remove fork catchpoint target routine.  Nothing to do, just
> @@ -2387,9 +2461,9 @@ remote_target::remove_fork_catchpoint (int pid)
>  int
>  remote_target::insert_vfork_catchpoint (int pid)
>  {
> -  struct remote_state *rs = get_remote_state ();
> +  remote_state *rs = get_remote_state ();
>  
> -  return !remote_vfork_event_p (rs);
> +  return !m_features.remote_vfork_event_p (rs);
>  }
>  
>  /* Remove vfork catchpoint target routine.  Nothing to do, just
> @@ -2407,9 +2481,9 @@ remote_target::remove_vfork_catchpoint (int pid)
>  int
>  remote_target::insert_exec_catchpoint (int pid)
>  {
> -  struct remote_state *rs = get_remote_state ();
> +  remote_state *rs = get_remote_state ();
>  
> -  return !remote_exec_event_p (rs);
> +  return !m_features.remote_exec_event_p (rs);
>  }
>  
>  /* Remove exec catchpoint target routine.  Nothing to do, just
> @@ -2438,10 +2512,10 @@ remote_target::remote_query_attached (int pid)
>    struct remote_state *rs = get_remote_state ();
>    size_t size = get_remote_packet_size ();
>  
> -  if (packet_support (PACKET_qAttached) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_qAttached) == PACKET_DISABLE)
>      return 0;
>  
> -  if (remote_multi_process_p (rs))
> +  if (m_features.remote_multi_process_p (rs))
>      xsnprintf (rs->buf.data (), size, "qAttached:%x", pid);
>    else
>      xsnprintf (rs->buf.data (), size, "qAttached");
> @@ -2450,7 +2524,7 @@ remote_target::remote_query_attached (int pid)
>    getpkt (&rs->buf, 0);
>  
>    switch (packet_ok (rs->buf,
> -		     &remote_protocol_packets[PACKET_qAttached]))
> +		     &m_features.m_protocol_packets[PACKET_qAttached]))
>      {
>      case PACKET_OK:
>        if (strcmp (rs->buf.data (), "1") == 0)
> @@ -2651,7 +2725,7 @@ remote_target::remote_notice_new_inferior (ptid_t currthread, bool executing)
>        if (find_inferior_pid (this, currthread.pid ()) == NULL)
>  	{
>  	  struct remote_state *rs = get_remote_state ();
> -	  bool fake_pid_p = !remote_multi_process_p (rs);
> +	  bool fake_pid_p = !m_features.remote_multi_process_p (rs);
>  
>  	  inf = remote_add_inferior (fake_pid_p,
>  				     currthread.pid (), -1, 1);
> @@ -2714,7 +2788,7 @@ record_currthread (struct remote_state *rs, ptid_t currthread)
>  void
>  remote_target::pass_signals (gdb::array_view<const unsigned char> pass_signals)
>  {
> -  if (packet_support (PACKET_QPassSignals) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_QPassSignals) != PACKET_DISABLE)
>      {
>        char *pass_packet, *p;
>        int count = 0;
> @@ -2748,7 +2822,8 @@ remote_target::pass_signals (gdb::array_view<const unsigned char> pass_signals)
>  	{
>  	  putpkt (pass_packet);
>  	  getpkt (&rs->buf, 0);
> -	  packet_ok (rs->buf, &remote_protocol_packets[PACKET_QPassSignals]);
> +	  packet_ok (rs->buf,
> +		     &m_features.m_protocol_packets[PACKET_QPassSignals]);
>  	  xfree (rs->last_pass_packet);
>  	  rs->last_pass_packet = pass_packet;
>  	}
> @@ -2768,7 +2843,7 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
>    enum packet_result result;
>    int n_sysno = 0;
>  
> -  if (packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE)
>      {
>        /* Not supported.  */
>        return 1;
> @@ -2821,7 +2896,8 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
>  
>    putpkt (catch_packet);
>    getpkt (&rs->buf, 0);
> -  result = packet_ok (rs->buf, &remote_protocol_packets[PACKET_QCatchSyscalls]);
> +  result = packet_ok (rs->buf,
> +		      &m_features.m_protocol_packets[PACKET_QCatchSyscalls]);
>    if (result == PACKET_OK)
>      return 0;
>    else
> @@ -2834,7 +2910,7 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
>  void
>  remote_target::program_signals (gdb::array_view<const unsigned char> signals)
>  {
> -  if (packet_support (PACKET_QProgramSignals) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_QProgramSignals) != PACKET_DISABLE)
>      {
>        char *packet, *p;
>        int count = 0;
> @@ -2869,7 +2945,8 @@ remote_target::program_signals (gdb::array_view<const unsigned char> signals)
>  	{
>  	  putpkt (packet);
>  	  getpkt (&rs->buf, 0);
> -	  packet_ok (rs->buf, &remote_protocol_packets[PACKET_QProgramSignals]);
> +	  packet_ok (rs->buf,
> +		     &m_features.m_protocol_packets[PACKET_QProgramSignals]);
>  	  xfree (rs->last_program_signals_packet);
>  	  rs->last_program_signals_packet = packet;
>  	}
> @@ -2938,7 +3015,7 @@ remote_target::set_general_process ()
>    struct remote_state *rs = get_remote_state ();
>  
>    /* If the remote can't handle multiple processes, don't bother.  */
> -  if (!remote_multi_process_p (rs))
> +  if (!m_features.remote_multi_process_p (rs))
>      return;
>  
>    /* We only need to change the remote current thread if it's pointing
> @@ -3092,7 +3169,7 @@ remote_target::write_ptid (char *buf, const char *endbuf, ptid_t ptid)
>    int pid, tid;
>    struct remote_state *rs = get_remote_state ();
>  
> -  if (remote_multi_process_p (rs))
> +  if (m_features.remote_multi_process_p (rs))
>      {
>        pid = ptid.pid ();
>        if (pid < 0)
> @@ -3853,7 +3930,7 @@ int
>  remote_target::remote_get_threads_with_qxfer (threads_listing_context *context)
>  {
>  #if defined(HAVE_LIBEXPAT)
> -  if (packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
> +  if (m_features.packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
>      {
>        gdb::optional<gdb::char_vector> xml
>  	= target_read_stralloc (this, TARGET_OBJECT_THREADS, NULL);
> @@ -4046,7 +4123,7 @@ remote_target::extra_thread_info (thread_info *tp)
>    if (!extra.empty ())
>      return extra.c_str ();
>  
> -  if (packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
> +  if (m_features.packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
>      {
>        /* If we're using qXfer:threads:read, then the extra info is
>  	 included in the XML.  So if we didn't have anything cached,
> @@ -4483,7 +4560,7 @@ remote_target::add_current_inferior_and_thread (const char *wait_status)
>  
>    if (curr_ptid != null_ptid)
>      {
> -      if (!remote_multi_process_p (rs))
> +      if (!m_features.remote_multi_process_p (rs))
>  	fake_pid_p = true;
>      }
>    else
> @@ -4763,7 +4840,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
>    remote_query_supported ();
>  
>    /* If the stub wants to get a QAllow, compose one and send it.  */
> -  if (packet_support (PACKET_QAllow) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_QAllow) != PACKET_DISABLE)
>      set_permissions ();
>  
>    /* gdbserver < 7.7 (before its fix from 2013-12-11) did reply to any
> @@ -4779,7 +4856,10 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
>      putpkt (v_mustreplyempty);
>      getpkt (&rs->buf, 0);
>      if (strcmp (rs->buf.data (), "OK") == 0)
> -      remote_protocol_packets[PACKET_vFile_setfs].support = PACKET_DISABLE;
> +      {
> +	m_features.m_protocol_packets[PACKET_vFile_setfs].support
> +	  = PACKET_DISABLE;
> +      }
>      else if (strcmp (rs->buf.data (), "") != 0)
>        error (_("Remote replied unexpectedly to '%s': %s"), v_mustreplyempty,
>  	     rs->buf.data ());
> @@ -4798,7 +4878,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
>       If FALSE, then don't activate noack mode, regardless of what the
>       stub claimed should be the default with qSupported.  */
>  
> -  noack_config = &remote_protocol_packets[PACKET_QStartNoAckMode];
> +  noack_config = &m_features.m_protocol_packets[PACKET_QStartNoAckMode];
>    if (packet_config_support (noack_config) != PACKET_DISABLE)
>      {
>        putpkt ("QStartNoAckMode");
> @@ -4833,7 +4913,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
>  
>    if (target_is_non_stop_p ())
>      {
> -      if (packet_support (PACKET_QNonStop) != PACKET_ENABLE)
> +      if (m_features.packet_support (PACKET_QNonStop) != PACKET_ENABLE)
>  	error (_("Non-stop mode requested, but remote "
>  		 "does not support non-stop"));
>  
> @@ -4850,7 +4930,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
>  	 stopped.  */
>        this->update_thread_list ();
>      }
> -  else if (packet_support (PACKET_QNonStop) == PACKET_ENABLE)
> +  else if (m_features.packet_support (PACKET_QNonStop) == PACKET_ENABLE)
>      {
>        /* Don't assume that the stub can operate in all-stop mode.
>  	 Request it explicitly.  */
> @@ -5082,13 +5162,13 @@ extended_remote_target::open (const char *name, int from_tty)
>  /* Reset all packets back to "unknown support".  Called when opening a
>     new connection to a remote target.  */
>  
> -static void
> -reset_all_packet_configs_support (void)
> +void
> +remote_features::reset_all_packet_configs_support (void)
>  {
>    int i;
>  
>    for (i = 0; i < PACKET_MAX; i++)
> -    remote_protocol_packets[i].support = PACKET_SUPPORT_UNKNOWN;
> +    m_protocol_packets[i].support = PACKET_SUPPORT_UNKNOWN;
>  }
>  
>  /* Initialize all packet configs.  */
> @@ -5121,7 +5201,7 @@ remote_target::remote_check_symbols ()
>    if (!target_has_execution ())
>      return;
>  
> -  if (packet_support (PACKET_qSymbol) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_qSymbol) == PACKET_DISABLE)
>      return;
>  
>    /* Make sure the remote is pointing at the right process.  Note
> @@ -5137,7 +5217,7 @@ remote_target::remote_check_symbols ()
>  
>    putpkt ("qSymbol::");
>    getpkt (&reply, 0);
> -  packet_ok (reply, &remote_protocol_packets[PACKET_qSymbol]);
> +  packet_ok (reply, &m_features.m_protocol_packets[PACKET_qSymbol]);
>  
>    while (startswith (reply.data (), "qSymbol:"))
>      {
> @@ -5261,7 +5341,7 @@ remote_supported_packet (remote_target *remote,
>        return;
>      }
>  
> -  remote_protocol_packets[feature->packet].support = support;
> +  remote->m_features.m_protocol_packets[feature->packet].support = support;
>  }
>  
>  void
> @@ -5469,47 +5549,57 @@ remote_target::remote_query_supported ()
>       containing no features.  */
>  
>    rs->buf[0] = 0;
> -  if (packet_support (PACKET_qSupported) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_qSupported) != PACKET_DISABLE)
>      {
>        std::string q;
>  
> -      if (packet_set_cmd_state (PACKET_multiprocess_feature) != AUTO_BOOLEAN_FALSE)
> +      if (m_features.packet_set_cmd_state (PACKET_multiprocess_feature)
> +	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "multiprocess+");
>  
> -      if (packet_set_cmd_state (PACKET_swbreak_feature) != AUTO_BOOLEAN_FALSE)
> +      if (m_features.packet_set_cmd_state (PACKET_swbreak_feature)
> +	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "swbreak+");
> -      if (packet_set_cmd_state (PACKET_hwbreak_feature) != AUTO_BOOLEAN_FALSE)
> +
> +      if (m_features.packet_set_cmd_state (PACKET_hwbreak_feature)
> +	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "hwbreak+");
>  
>        remote_query_supported_append (&q, "qRelocInsn+");
>  
> -      if (packet_set_cmd_state (PACKET_fork_event_feature)
> +      if (m_features.packet_set_cmd_state (PACKET_fork_event_feature)
>  	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "fork-events+");
> -      if (packet_set_cmd_state (PACKET_vfork_event_feature)
> +
> +      if (m_features.packet_set_cmd_state (PACKET_vfork_event_feature)
>  	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "vfork-events+");
> -      if (packet_set_cmd_state (PACKET_exec_event_feature)
> +
> +      if (m_features.packet_set_cmd_state (PACKET_exec_event_feature)
>  	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "exec-events+");
>  
> -      if (packet_set_cmd_state (PACKET_vContSupported) != AUTO_BOOLEAN_FALSE)
> +      if (m_features.packet_set_cmd_state (PACKET_vContSupported)
> +	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "vContSupported+");
>  
> -      if (packet_set_cmd_state (PACKET_QThreadEvents) != AUTO_BOOLEAN_FALSE)
> +      if (m_features.packet_set_cmd_state (PACKET_QThreadEvents)
> +	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "QThreadEvents+");
>  
> -      if (packet_set_cmd_state (PACKET_no_resumed) != AUTO_BOOLEAN_FALSE)
> +      if (m_features.packet_set_cmd_state (PACKET_no_resumed)
> +	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "no-resumed+");
>  
> -      if (packet_set_cmd_state (PACKET_memory_tagging_feature)
> +      if (m_features.packet_set_cmd_state (PACKET_memory_tagging_feature)
>  	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "memory-tagging+");
>  
>        /* Keep this one last to work around a gdbserver <= 7.10 bug in
>  	 the qSupported:xmlRegisters=i386 handling.  */
>        if (remote_support_xml != NULL
> -	  && packet_support (PACKET_qXfer_features) != PACKET_DISABLE)
> +	  && (m_features.packet_support (PACKET_qXfer_features)
> +	      != PACKET_DISABLE))
>  	remote_query_supported_append (&q, remote_support_xml);
>  
>        q = "qSupported:" + q;
> @@ -5519,7 +5609,7 @@ remote_target::remote_query_supported ()
>  
>        /* If an error occured, warn, but do not return - just reset the
>  	 buffer to empty and go on to disable features.  */
> -      if (packet_ok (rs->buf, &remote_protocol_packets[PACKET_qSupported])
> +      if (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_qSupported])
>  	  == PACKET_ERROR)
>  	{
>  	  warning (_("Remote failure reply: %s"), rs->buf.data ());
> @@ -5797,7 +5887,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p)
>  
>    /* Reset the target state; these things will be queried either by
>       remote_query_supported or as they are needed.  */
> -  reset_all_packet_configs_support ();
> +  remote->m_features.reset_all_packet_configs_support ();
>    rs->explicit_packet_size = 0;
>    rs->noack_mode = 0;
>    rs->extended = extended_p;
> @@ -5910,7 +6000,7 @@ remote_target::remote_detach_pid (int pid)
>       GDBserver to select GDB's current process.  */
>    set_general_process ();
>  
> -  if (remote_multi_process_p (rs))
> +  if (m_features.remote_multi_process_p (rs))
>      xsnprintf (rs->buf.data (), get_remote_packet_size (), "D;%x", pid);
>    else
>      strcpy (rs->buf.data (), "D");
> @@ -6043,8 +6133,10 @@ remote_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
>  
>    struct remote_state *rs = get_remote_state ();
>  
> -  if ((fork_kind == TARGET_WAITKIND_FORKED && remote_fork_event_p (rs))
> -      || (fork_kind == TARGET_WAITKIND_VFORKED && remote_vfork_event_p (rs)))
> +  if ((fork_kind == TARGET_WAITKIND_FORKED
> +       && m_features.remote_fork_event_p (rs))
> +      || (fork_kind == TARGET_WAITKIND_VFORKED
> +	  && m_features.remote_vfork_event_p (rs)))
>      {
>        /* When following the parent and detaching the child, we detach
>  	 the child here.  For the case of following the child and
> @@ -6110,7 +6202,7 @@ extended_remote_target::attach (const char *args, int from_tty)
>    /* Remote PID can be freely equal to getpid, do not check it here the same
>       way as in other targets.  */
>  
> -  if (packet_support (PACKET_vAttach) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_vAttach) == PACKET_DISABLE)
>      error (_("This target does not support attaching to a process"));
>  
>    target_announce_attach (from_tty, pid);
> @@ -6120,7 +6212,7 @@ extended_remote_target::attach (const char *args, int from_tty)
>    getpkt (&rs->buf, 0);
>  
>    switch (packet_ok (rs->buf,
> -		     &remote_protocol_packets[PACKET_vAttach]))
> +		     &m_features.m_protocol_packets[PACKET_vAttach]))
>      {
>      case PACKET_OK:
>        if (!target_is_non_stop_p ())
> @@ -6265,7 +6357,7 @@ remote_target::remote_vcont_probe ()
>  	buf[0] = 0;
>      }
>  
> -  packet_ok (rs->buf, &remote_protocol_packets[PACKET_vCont]);
> +  packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vCont]);
>    rs->supports_vCont_probed = true;
>  }
>  
> @@ -6296,7 +6388,7 @@ remote_target::append_resumption (char *p, char *endp,
>  	      threads with a wildcard (though the protocol allows it,
>  	      so stubs shouldn't make an active effort to forbid
>  	      it).  */
> -	   && !(remote_multi_process_p (rs) && ptid.is_pid ()))
> +	   && !(m_features.remote_multi_process_p (rs) && ptid.is_pid ()))
>      {
>        struct thread_info *tp;
>  
> @@ -6330,7 +6422,7 @@ remote_target::append_resumption (char *p, char *endp,
>    else
>      p += xsnprintf (p, endp - p, ";c");
>  
> -  if (remote_multi_process_p (rs) && ptid.is_pid ())
> +  if (m_features.remote_multi_process_p (rs) && ptid.is_pid ())
>      {
>        ptid_t nptid;
>  
> @@ -6414,9 +6506,9 @@ remote_target::remote_resume_with_hc (ptid_t ptid, int step,
>  	warning (_(" - Can't pass signal %d to target in reverse: ignored."),
>  		 siggnal);
>  
> -      if (step && packet_support (PACKET_bs) == PACKET_DISABLE)
> +      if (step && m_features.packet_support (PACKET_bs) == PACKET_DISABLE)
>  	error (_("Remote reverse-step not supported."));
> -      if (!step && packet_support (PACKET_bc) == PACKET_DISABLE)
> +      if (!step && m_features.packet_support (PACKET_bc) == PACKET_DISABLE)
>  	error (_("Remote reverse-continue not supported."));
>  
>        strcpy (buf, step ? "bs" : "bc");
> @@ -6456,10 +6548,10 @@ remote_target::remote_resume_with_vcont (ptid_t ptid, int step,
>    if (::execution_direction == EXEC_REVERSE)
>      return 0;
>  
> -  if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
> +  if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
>      remote_vcont_probe ();
>  
> -  if (packet_support (PACKET_vCont) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_vCont) == PACKET_DISABLE)
>      return 0;
>  
>    p = rs->buf.data ();
> @@ -7002,7 +7094,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
>  
>    /* FIXME: This supports_vCont_probed check is a workaround until
>       packet_support is per-connection.  */
> -  if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN
> +  if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN
>        || !rs->supports_vCont_probed)
>      remote_vcont_probe ();
>  
> @@ -7010,7 +7102,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
>      error (_("Remote server does not support stopping threads"));
>  
>    if (ptid == minus_one_ptid
> -      || (!remote_multi_process_p (rs) && ptid.is_pid ()))
> +      || (!m_features.remote_multi_process_p (rs) && ptid.is_pid ()))
>      p += xsnprintf (p, endp - p, "vCont;t");
>    else
>      {
> @@ -7084,7 +7176,7 @@ remote_target::remote_interrupt_ns ()
>    putpkt (rs->buf);
>    getpkt (&rs->buf, 0);
>  
> -  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vCtrlC]))
> +  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vCtrlC]))
>      {
>      case PACKET_OK:
>        break;
> @@ -7591,7 +7683,8 @@ Packet: '%s'\n"),
>  
>  	      /* Make sure the stub doesn't forget to indicate support
>  		 with qSupported.  */
> -	      if (packet_support (PACKET_swbreak_feature) != PACKET_ENABLE)
> +	      if (m_features.packet_support (PACKET_swbreak_feature)
> +		  != PACKET_ENABLE)
>  		error (_("Unexpected swbreak stop reason"));
>  
>  	      /* The value part is documented as "must be empty",
> @@ -7605,7 +7698,8 @@ Packet: '%s'\n"),
>  
>  	      /* Make sure the stub doesn't forget to indicate support
>  		 with qSupported.  */
> -	      if (packet_support (PACKET_hwbreak_feature) != PACKET_ENABLE)
> +	      if (m_features.packet_support (PACKET_hwbreak_feature)
> +		  != PACKET_ENABLE)
>  		error (_("Unexpected hwbreak stop reason"));
>  
>  	      /* See above.  */
> @@ -8376,7 +8470,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
>    gdb_byte *regp = (gdb_byte *) alloca (register_size (gdbarch, reg->regnum));
>    int i;
>  
> -  if (packet_support (PACKET_p) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_p) == PACKET_DISABLE)
>      return 0;
>  
>    if (reg->pnum == -1)
> @@ -8391,7 +8485,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
>  
>    buf = rs->buf.data ();
>  
> -  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_p]))
> +  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_p]))
>      {
>      case PACKET_OK:
>        break;
> @@ -8658,7 +8752,7 @@ remote_target::prepare_to_store (struct regcache *regcache)
>    int i;
>  
>    /* Make sure the entire registers array is valid.  */
> -  switch (packet_support (PACKET_P))
> +  switch (m_features.packet_support (PACKET_P))
>      {
>      case PACKET_DISABLE:
>      case PACKET_SUPPORT_UNKNOWN:
> @@ -8686,7 +8780,7 @@ remote_target::store_register_using_P (const struct regcache *regcache,
>    gdb_byte *regp = (gdb_byte *) alloca (register_size (gdbarch, reg->regnum));
>    char *p;
>  
> -  if (packet_support (PACKET_P) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_P) == PACKET_DISABLE)
>      return 0;
>  
>    if (reg->pnum == -1)
> @@ -8699,7 +8793,7 @@ remote_target::store_register_using_P (const struct regcache *regcache,
>    putpkt (rs->buf);
>    getpkt (&rs->buf, 0);
>  
> -  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_P]))
> +  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_P]))
>      {
>      case PACKET_OK:
>        return 1;
> @@ -8882,7 +8976,7 @@ remote_target::check_binary_download (CORE_ADDR addr)
>  {
>    struct remote_state *rs = get_remote_state ();
>  
> -  switch (packet_support (PACKET_X))
> +  switch (m_features.packet_support (PACKET_X))
>      {
>      case PACKET_DISABLE:
>        break;
> @@ -8906,12 +9000,12 @@ remote_target::check_binary_download (CORE_ADDR addr)
>  	if (rs->buf[0] == '\0')
>  	  {
>  	    remote_debug_printf ("binary downloading NOT supported by target");
> -	    remote_protocol_packets[PACKET_X].support = PACKET_DISABLE;
> +	    m_features.m_protocol_packets[PACKET_X].support = PACKET_DISABLE;
>  	  }
>  	else
>  	  {
>  	    remote_debug_printf ("binary downloading supported by target");
> -	    remote_protocol_packets[PACKET_X].support = PACKET_ENABLE;
> +	    m_features.m_protocol_packets[PACKET_X].support = PACKET_ENABLE;
>  	  }
>  	break;
>        }
> @@ -9138,7 +9232,7 @@ remote_target::remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr,
>    /* Check whether the target supports binary download.  */
>    check_binary_download (memaddr);
>  
> -  switch (packet_support (PACKET_X))
> +  switch (m_features.packet_support (PACKET_X))
>      {
>      case PACKET_ENABLE:
>        packet_format = "X";
> @@ -10107,7 +10201,7 @@ remote_target::kill ()
>  
>    gdb_assert (inf != nullptr);
>  
> -  if (packet_support (PACKET_vKill) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_vKill) != PACKET_DISABLE)
>      {
>        /* If we're stopped while forking and we haven't followed yet,
>  	 kill the child task.  We need to do this before killing the
> @@ -10126,7 +10220,7 @@ remote_target::kill ()
>    /* If we are in 'target remote' mode and we are killing the only
>       inferior, then we will tell gdbserver to exit and unpush the
>       target.  */
> -  if (res == -1 && !remote_multi_process_p (rs)
> +  if (res == -1 && !m_features.remote_multi_process_p (rs)
>        && number_of_live_inferiors (this) == 1)
>      {
>        remote_kill_k ();
> @@ -10148,7 +10242,7 @@ remote_target::kill ()
>  int
>  remote_target::remote_vkill (int pid)
>  {
> -  if (packet_support (PACKET_vKill) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_vKill) == PACKET_DISABLE)
>      return -1;
>  
>    remote_state *rs = get_remote_state ();
> @@ -10159,7 +10253,7 @@ remote_target::remote_vkill (int pid)
>    getpkt (&rs->buf, 0);
>  
>    switch (packet_ok (rs->buf,
> -		     &remote_protocol_packets[PACKET_vKill]))
> +		     &m_features.m_protocol_packets[PACKET_vKill]))
>      {
>      case PACKET_OK:
>        return 0;
> @@ -10255,7 +10349,8 @@ remote_target::mourn_inferior ()
>  bool
>  extended_remote_target::supports_disable_randomization ()
>  {
> -  return packet_support (PACKET_QDisableRandomization) == PACKET_ENABLE;
> +  return (m_features.packet_support (PACKET_QDisableRandomization)
> +	  == PACKET_ENABLE);
>  }
>  
>  void
> @@ -10283,7 +10378,7 @@ remote_target::extended_remote_run (const std::string &args)
>  
>    /* If the user has disabled vRun support, or we have detected that
>       support is not available, do not try it.  */
> -  if (packet_support (PACKET_vRun) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_vRun) == PACKET_DISABLE)
>      return -1;
>  
>    strcpy (rs->buf.data (), "vRun;");
> @@ -10314,7 +10409,7 @@ remote_target::extended_remote_run (const std::string &args)
>    putpkt (rs->buf);
>    getpkt (&rs->buf, 0);
>  
> -  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vRun]))
> +  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vRun]))
>      {
>      case PACKET_OK:
>        /* We have a wait response.  All is well.  */
> @@ -10367,7 +10462,7 @@ remote_target::extended_remote_environment_support ()
>  {
>    remote_state *rs = get_remote_state ();
>  
> -  if (packet_support (PACKET_QEnvironmentReset) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_QEnvironmentReset) != PACKET_DISABLE)
>      {
>        putpkt ("QEnvironmentReset");
>        getpkt (&rs->buf, 0);
> @@ -10377,12 +10472,16 @@ remote_target::extended_remote_environment_support ()
>  
>    gdb_environ *e = &current_inferior ()->environment;
>  
> -  if (packet_support (PACKET_QEnvironmentHexEncoded) != PACKET_DISABLE)
> -    for (const std::string &el : e->user_set_env ())
> -      send_environment_packet ("set", "QEnvironmentHexEncoded",
> -			       el.c_str ());
> +  if (m_features.packet_support (PACKET_QEnvironmentHexEncoded)
> +      != PACKET_DISABLE)
> +    {
> +      for (const std::string &el : e->user_set_env ())
> +	send_environment_packet ("set", "QEnvironmentHexEncoded",
> +				 el.c_str ());
> +    }
>  
> -  if (packet_support (PACKET_QEnvironmentUnset) != PACKET_DISABLE)
> +
> +  if (m_features.packet_support (PACKET_QEnvironmentUnset) != PACKET_DISABLE)
>      for (const std::string &el : e->user_unset_env ())
>        send_environment_packet ("unset", "QEnvironmentUnset", el.c_str ());
>  }
> @@ -10393,7 +10492,7 @@ remote_target::extended_remote_environment_support ()
>  void
>  remote_target::extended_remote_set_inferior_cwd ()
>  {
> -  if (packet_support (PACKET_QSetWorkingDir) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_QSetWorkingDir) != PACKET_DISABLE)
>      {
>        const std::string &inferior_cwd = current_inferior ()->cwd ();
>        remote_state *rs = get_remote_state ();
> @@ -10418,7 +10517,7 @@ remote_target::extended_remote_set_inferior_cwd ()
>        putpkt (rs->buf);
>        getpkt (&rs->buf, 0);
>        if (packet_ok (rs->buf,
> -		     &remote_protocol_packets[PACKET_QSetWorkingDir])
> +		     &m_features.m_protocol_packets[PACKET_QSetWorkingDir])
>  	  != PACKET_OK)
>  	error (_("\
>  Remote replied unexpectedly while setting the inferior's working\n\
> @@ -10455,7 +10554,7 @@ extended_remote_target::create_inferior (const char *exec_file,
>  
>    /* If startup-with-shell is on, we inform gdbserver to start the
>       remote inferior using a shell.  */
> -  if (packet_support (PACKET_QStartupWithShell) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_QStartupWithShell) != PACKET_DISABLE)
>      {
>        xsnprintf (rs->buf.data (), get_remote_packet_size (),
>  		 "QStartupWithShell:%d", startup_with_shell ? 1 : 0);
> @@ -10561,7 +10660,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
>       fails, and the user has explicitly requested the Z support then
>       report an error, otherwise, mark it disabled and go on.  */
>  
> -  if (packet_support (PACKET_Z0) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_Z0) != PACKET_DISABLE)
>      {
>        CORE_ADDR addr = bp_tgt->reqstd_address;
>        struct remote_state *rs;
> @@ -10592,7 +10691,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
>        putpkt (rs->buf);
>        getpkt (&rs->buf, 0);
>  
> -      switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z0]))
> +      switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_Z0]))
>  	{
>  	case PACKET_ERROR:
>  	  return -1;
> @@ -10620,7 +10719,7 @@ remote_target::remove_breakpoint (struct gdbarch *gdbarch,
>    CORE_ADDR addr = bp_tgt->placed_address;
>    struct remote_state *rs = get_remote_state ();
>  
> -  if (packet_support (PACKET_Z0) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_Z0) != PACKET_DISABLE)
>      {
>        char *p = rs->buf.data ();
>        char *endbuf = p + get_remote_packet_size ();
> @@ -10676,7 +10775,7 @@ remote_target::insert_watchpoint (CORE_ADDR addr, int len,
>    char *p;
>    enum Z_packet_type packet = watchpoint_to_Z_packet (type);
>  
> -  if (packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
>      return 1;
>  
>    /* Make sure the remote is pointing at the right process, if
> @@ -10693,7 +10792,8 @@ remote_target::insert_watchpoint (CORE_ADDR addr, int len,
>    putpkt (rs->buf);
>    getpkt (&rs->buf, 0);
>  
> -  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z0 + packet]))
> +  switch (packet_ok (rs->buf,
> +		     &m_features.m_protocol_packets[PACKET_Z0 + packet]))
>      {
>      case PACKET_ERROR:
>        return -1;
> @@ -10725,7 +10825,7 @@ remote_target::remove_watchpoint (CORE_ADDR addr, int len,
>    char *p;
>    enum Z_packet_type packet = watchpoint_to_Z_packet (type);
>  
> -  if (packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
>      return -1;
>  
>    /* Make sure the remote is pointing at the right process, if
> @@ -10741,7 +10841,8 @@ remote_target::remove_watchpoint (CORE_ADDR addr, int len,
>    putpkt (rs->buf);
>    getpkt (&rs->buf, 0);
>  
> -  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z0 + packet]))
> +  switch (packet_ok (rs->buf,
> +		     &m_features.m_protocol_packets[PACKET_Z0 + packet]))
>      {
>      case PACKET_ERROR:
>      case PACKET_UNKNOWN:
> @@ -10815,7 +10916,7 @@ remote_target::stopped_by_sw_breakpoint ()
>  bool
>  remote_target::supports_stopped_by_sw_breakpoint ()
>  {
> -  return (packet_support (PACKET_swbreak_feature) == PACKET_ENABLE);
> +  return (m_features.packet_support (PACKET_swbreak_feature) == PACKET_ENABLE);
>  }
>  
>  /* The to_stopped_by_hw_breakpoint method of target remote.  */
> @@ -10836,7 +10937,7 @@ remote_target::stopped_by_hw_breakpoint ()
>  bool
>  remote_target::supports_stopped_by_hw_breakpoint ()
>  {
> -  return (packet_support (PACKET_hwbreak_feature) == PACKET_ENABLE);
> +  return (m_features.packet_support (PACKET_hwbreak_feature) == PACKET_ENABLE);
>  }
>  
>  bool
> @@ -10875,7 +10976,7 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
>    char *p, *endbuf;
>    char *message;
>  
> -  if (packet_support (PACKET_Z1) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_Z1) == PACKET_DISABLE)
>      return -1;
>  
>    /* Make sure the remote is pointing at the right process, if
> @@ -10904,7 +11005,7 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
>    putpkt (rs->buf);
>    getpkt (&rs->buf, 0);
>  
> -  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z1]))
> +  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_Z1]))
>      {
>      case PACKET_ERROR:
>        if (rs->buf[1] == '.')
> @@ -10933,7 +11034,7 @@ remote_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
>    char *p = rs->buf.data ();
>    char *endbuf = p + get_remote_packet_size ();
>  
> -  if (packet_support (PACKET_Z1) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_Z1) == PACKET_DISABLE)
>      return -1;
>  
>    /* Make sure the remote is pointing at the right process, if
> @@ -10952,7 +11053,7 @@ remote_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
>    putpkt (rs->buf);
>    getpkt (&rs->buf, 0);
>  
> -  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z1]))
> +  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_Z1]))
>      {
>      case PACKET_ERROR:
>      case PACKET_UNKNOWN:
> @@ -10976,7 +11077,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
>    /* It doesn't make sense to use qCRC if the remote target is
>       connected but not running.  */
>    if (target_has_execution ()
> -      && packet_support (PACKET_qCRC) != PACKET_DISABLE)
> +      && m_features.packet_support (PACKET_qCRC) != PACKET_DISABLE)
>      {
>        enum packet_result result;
>  
> @@ -10995,7 +11096,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
>        getpkt (&rs->buf, 0);
>  
>        result = packet_ok (rs->buf,
> -			  &remote_protocol_packets[PACKET_qCRC]);
> +			  &m_features.m_protocol_packets[PACKET_qCRC]);
>        if (result == PACKET_ERROR)
>  	return -1;
>        else if (result == PACKET_OK)
> @@ -11255,12 +11356,12 @@ remote_target::xfer_partial (enum target_object object,
>      {
>        if (readbuf)
>  	return remote_read_qxfer ("siginfo", annex, readbuf, offset, len,
> -				  xfered_len, &remote_protocol_packets
> +				  xfered_len, &m_features.m_protocol_packets
>  				  [PACKET_qXfer_siginfo_read]);
>        else
>  	return remote_write_qxfer ("siginfo", annex,
>  				   writebuf, offset, len, xfered_len,
> -				   &remote_protocol_packets
> +				   &m_features.m_protocol_packets
>  				   [PACKET_qXfer_siginfo_write]);
>      }
>  
> @@ -11269,7 +11370,7 @@ remote_target::xfer_partial (enum target_object object,
>        if (readbuf)
>  	return remote_read_qxfer ("statictrace", annex,
>  				  readbuf, offset, len, xfered_len,
> -				  &remote_protocol_packets
> +				  &m_features.m_protocol_packets
>  				  [PACKET_qXfer_statictrace_read]);
>        else
>  	return TARGET_XFER_E_IO;
> @@ -11299,74 +11400,74 @@ remote_target::xfer_partial (enum target_object object,
>  
>      case TARGET_OBJECT_AUXV:
>        gdb_assert (annex == NULL);
> -      return remote_read_qxfer ("auxv", annex, readbuf, offset, len,
> -				xfered_len,
> -				&remote_protocol_packets[PACKET_qXfer_auxv]);
> +      return remote_read_qxfer
> +	("auxv", annex, readbuf, offset, len, xfered_len,
> +	 &m_features.m_protocol_packets[PACKET_qXfer_auxv]);
>  
>      case TARGET_OBJECT_AVAILABLE_FEATURES:
>        return remote_read_qxfer
>  	("features", annex, readbuf, offset, len, xfered_len,
> -	 &remote_protocol_packets[PACKET_qXfer_features]);
> +	 &m_features.m_protocol_packets[PACKET_qXfer_features]);
>  
>      case TARGET_OBJECT_LIBRARIES:
>        return remote_read_qxfer
>  	("libraries", annex, readbuf, offset, len, xfered_len,
> -	 &remote_protocol_packets[PACKET_qXfer_libraries]);
> +	 &m_features.m_protocol_packets[PACKET_qXfer_libraries]);
>  
>      case TARGET_OBJECT_LIBRARIES_SVR4:
>        return remote_read_qxfer
>  	("libraries-svr4", annex, readbuf, offset, len, xfered_len,
> -	 &remote_protocol_packets[PACKET_qXfer_libraries_svr4]);
> +	 &m_features.m_protocol_packets[PACKET_qXfer_libraries_svr4]);
>  
>      case TARGET_OBJECT_MEMORY_MAP:
>        gdb_assert (annex == NULL);
> -      return remote_read_qxfer ("memory-map", annex, readbuf, offset, len,
> -				 xfered_len,
> -				&remote_protocol_packets[PACKET_qXfer_memory_map]);
> +      return remote_read_qxfer
> +	("memory-map", annex, readbuf, offset, len, xfered_len,
> +	 &m_features.m_protocol_packets[PACKET_qXfer_memory_map]);
>  
>      case TARGET_OBJECT_OSDATA:
>        /* Should only get here if we're connected.  */
>        gdb_assert (rs->remote_desc);
>        return remote_read_qxfer
>  	("osdata", annex, readbuf, offset, len, xfered_len,
> -	&remote_protocol_packets[PACKET_qXfer_osdata]);
> +	&m_features.m_protocol_packets[PACKET_qXfer_osdata]);
>  
>      case TARGET_OBJECT_THREADS:
>        gdb_assert (annex == NULL);
> -      return remote_read_qxfer ("threads", annex, readbuf, offset, len,
> -				xfered_len,
> -				&remote_protocol_packets[PACKET_qXfer_threads]);
> +      return remote_read_qxfer
> +	("threads", annex, readbuf, offset, len, xfered_len,
> +	 &m_features.m_protocol_packets[PACKET_qXfer_threads]);
>  
>      case TARGET_OBJECT_TRACEFRAME_INFO:
>        gdb_assert (annex == NULL);
>        return remote_read_qxfer
>  	("traceframe-info", annex, readbuf, offset, len, xfered_len,
> -	 &remote_protocol_packets[PACKET_qXfer_traceframe_info]);
> +	 &m_features.m_protocol_packets[PACKET_qXfer_traceframe_info]);
>  
>      case TARGET_OBJECT_FDPIC:
> -      return remote_read_qxfer ("fdpic", annex, readbuf, offset, len,
> -				xfered_len,
> -				&remote_protocol_packets[PACKET_qXfer_fdpic]);
> +      return remote_read_qxfer
> +	("fdpic", annex, readbuf, offset, len, xfered_len,
> +	 &m_features.m_protocol_packets[PACKET_qXfer_fdpic]);
>  
>      case TARGET_OBJECT_OPENVMS_UIB:
> -      return remote_read_qxfer ("uib", annex, readbuf, offset, len,
> -				xfered_len,
> -				&remote_protocol_packets[PACKET_qXfer_uib]);
> +      return remote_read_qxfer
> +	("uib", annex, readbuf, offset, len, xfered_len,
> +	 &m_features.m_protocol_packets[PACKET_qXfer_uib]);
>  
>      case TARGET_OBJECT_BTRACE:
> -      return remote_read_qxfer ("btrace", annex, readbuf, offset, len,
> -				xfered_len,
> -	&remote_protocol_packets[PACKET_qXfer_btrace]);
> +      return remote_read_qxfer
> +	("btrace", annex, readbuf, offset, len, xfered_len,
> +	 &m_features.m_protocol_packets[PACKET_qXfer_btrace]);
>  
>      case TARGET_OBJECT_BTRACE_CONF:
> -      return remote_read_qxfer ("btrace-conf", annex, readbuf, offset,
> -				len, xfered_len,
> -	&remote_protocol_packets[PACKET_qXfer_btrace_conf]);
> +      return remote_read_qxfer
> +	("btrace-conf", annex, readbuf, offset, len, xfered_len,
> +	&m_features.m_protocol_packets[PACKET_qXfer_btrace_conf]);
>  
>      case TARGET_OBJECT_EXEC_FILE:
> -      return remote_read_qxfer ("exec-file", annex, readbuf, offset,
> -				len, xfered_len,
> -	&remote_protocol_packets[PACKET_qXfer_exec_file]);
> +      return remote_read_qxfer
> +	("exec-file", annex, readbuf, offset, len, xfered_len,
> +	 &m_features.m_protocol_packets[PACKET_qXfer_exec_file]);
>  
>      default:
>        return TARGET_XFER_E_IO;
> @@ -11433,7 +11534,7 @@ remote_target::search_memory (CORE_ADDR start_addr, ULONGEST search_space_len,
>    struct remote_state *rs = get_remote_state ();
>    int max_size = get_memory_write_packet_size ();
>    struct packet_config *packet =
> -    &remote_protocol_packets[PACKET_qSearch_memory];
> +    &m_features.m_protocol_packets[PACKET_qSearch_memory];
>    /* Number of packet bytes used to encode the pattern;
>       this could be more than PATTERN_LEN due to escape characters.  */
>    int escaped_pattern_len;
> @@ -11858,7 +11959,7 @@ remote_target::pid_to_str (ptid_t ptid)
>  	 connecting with extended-remote and the stub already being
>  	 attached to a process, and reporting yes to qAttached, hence
>  	 no smart special casing here.  */
> -      if (!remote_multi_process_p (rs))
> +      if (!m_features.remote_multi_process_p (rs))
>  	return "Remote target";
>  
>        return normal_pid_to_str (ptid);
> @@ -11867,7 +11968,7 @@ remote_target::pid_to_str (ptid_t ptid)
>      {
>        if (magic_null_ptid == ptid)
>  	return "Thread <main>";
> -      else if (remote_multi_process_p (rs))
> +      else if (m_features.remote_multi_process_p (rs))
>  	if (ptid.lwp () == 0)
>  	  return normal_pid_to_str (ptid);
>  	else
> @@ -11885,7 +11986,7 @@ CORE_ADDR
>  remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
>  					 CORE_ADDR offset)
>  {
> -  if (packet_support (PACKET_qGetTLSAddr) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_qGetTLSAddr) != PACKET_DISABLE)
>      {
>        struct remote_state *rs = get_remote_state ();
>        char *p = rs->buf.data ();
> @@ -11904,7 +12005,7 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
>        putpkt (rs->buf);
>        getpkt (&rs->buf, 0);
>        result = packet_ok (rs->buf,
> -			  &remote_protocol_packets[PACKET_qGetTLSAddr]);
> +			  &m_features.m_protocol_packets[PACKET_qGetTLSAddr]);
>        if (result == PACKET_OK)
>  	{
>  	  ULONGEST addr;
> @@ -11932,7 +12033,7 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
>  bool
>  remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
>  {
> -  if (packet_support (PACKET_qGetTIBAddr) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_qGetTIBAddr) != PACKET_DISABLE)
>      {
>        struct remote_state *rs = get_remote_state ();
>        char *p = rs->buf.data ();
> @@ -11947,7 +12048,7 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
>        putpkt (rs->buf);
>        getpkt (&rs->buf, 0);
>        result = packet_ok (rs->buf,
> -			  &remote_protocol_packets[PACKET_qGetTIBAddr]);
> +			  &m_features.m_protocol_packets[PACKET_qGetTIBAddr]);
>        if (result == PACKET_OK)
>  	{
>  	  ULONGEST val;
> @@ -12200,7 +12301,7 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
>    int ret, bytes_read;
>    const char *attachment_tmp;
>  
> -  if (packet_support (which_packet) == PACKET_DISABLE)
> +  if (m_features.packet_support (which_packet) == PACKET_DISABLE)
>      {
>        *remote_errno = FILEIO_ENOSYS;
>        return -1;
> @@ -12217,7 +12318,7 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
>        return -1;
>      }
>  
> -  switch (packet_ok (rs->buf, &remote_protocol_packets[which_packet]))
> +  switch (packet_ok (rs->buf, &m_features.m_protocol_packets[which_packet]))
>      {
>      case PACKET_ERROR:
>        *remote_errno = FILEIO_EINVAL;
> @@ -12287,7 +12388,7 @@ remote_target::remote_hostio_set_filesystem (struct inferior *inf,
>    char arg[9];
>    int ret;
>  
> -  if (packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
>      return 0;
>  
>    if (rs->fs_pid != -1 && required_pid == rs->fs_pid)
> @@ -12301,7 +12402,7 @@ remote_target::remote_hostio_set_filesystem (struct inferior *inf,
>    ret = remote_hostio_send_command (p - rs->buf.data (), PACKET_vFile_setfs,
>  				    remote_errno, NULL, NULL);
>  
> -  if (packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
>      return 0;
>  
>    if (ret == 0)
> @@ -12673,7 +12774,7 @@ remote_target::filesystem_is_local ()
>       does not support vFile:open.  */
>    if (gdb_sysroot == TARGET_SYSROOT_PREFIX)
>      {
> -      enum packet_support ps = packet_support (PACKET_vFile_open);
> +      enum packet_support ps = m_features.packet_support (PACKET_vFile_open);
>  
>        if (ps == PACKET_SUPPORT_UNKNOWN)
>  	{
> @@ -12689,7 +12790,7 @@ remote_target::filesystem_is_local ()
>  	  if (fd >= 0)
>  	    remote_hostio_close (fd, &remote_errno);
>  
> -	  ps = packet_support (PACKET_vFile_open);
> +	  ps = m_features.packet_support (PACKET_vFile_open);
>  	}
>  
>        if (ps == PACKET_DISABLE)
> @@ -13042,8 +13143,8 @@ remote_delete_command (const char *args, int from_tty)
>  bool
>  remote_target::can_execute_reverse ()
>  {
> -  if (packet_support (PACKET_bs) == PACKET_ENABLE
> -      || packet_support (PACKET_bc) == PACKET_ENABLE)
> +  if (m_features.packet_support (PACKET_bs) == PACKET_ENABLE
> +      || m_features.packet_support (PACKET_bc) == PACKET_ENABLE)
>      return true;
>    else
>      return false;
> @@ -13067,56 +13168,58 @@ remote_target::supports_multi_process ()
>  {
>    struct remote_state *rs = get_remote_state ();
>  
> -  return remote_multi_process_p (rs);
> +  return m_features.remote_multi_process_p (rs);
>  }
>  
> -static int
> -remote_supports_cond_tracepoints ()
> +int
> +remote_target::remote_supports_cond_tracepoints ()
>  {
> -  return packet_support (PACKET_ConditionalTracepoints) == PACKET_ENABLE;
> +  return (m_features.packet_support (PACKET_ConditionalTracepoints)
> +	  == PACKET_ENABLE);
>  }
>  
>  bool
>  remote_target::supports_evaluation_of_breakpoint_conditions ()
>  {
> -  return packet_support (PACKET_ConditionalBreakpoints) == PACKET_ENABLE;
> +  return (m_features.packet_support (PACKET_ConditionalBreakpoints)
> +	  == PACKET_ENABLE);
>  }
>  
> -static int
> -remote_supports_fast_tracepoints ()
> +int
> +remote_target::remote_supports_fast_tracepoints ()
>  {
> -  return packet_support (PACKET_FastTracepoints) == PACKET_ENABLE;
> +  return m_features.packet_support (PACKET_FastTracepoints) == PACKET_ENABLE;
>  }
>  
> -static int
> -remote_supports_static_tracepoints ()
> +int
> +remote_target::remote_supports_static_tracepoints ()
>  {
> -  return packet_support (PACKET_StaticTracepoints) == PACKET_ENABLE;
> +  return m_features.packet_support (PACKET_StaticTracepoints) == PACKET_ENABLE;
>  }
>  
> -static int
> -remote_supports_install_in_trace ()
> +int
> +remote_target::remote_supports_install_in_trace ()
>  {
> -  return packet_support (PACKET_InstallInTrace) == PACKET_ENABLE;
> +  return m_features.packet_support (PACKET_InstallInTrace) == PACKET_ENABLE;
>  }
>  
>  bool
>  remote_target::supports_enable_disable_tracepoint ()
>  {
> -  return (packet_support (PACKET_EnableDisableTracepoints_feature)
> +  return (m_features.packet_support (PACKET_EnableDisableTracepoints_feature)
>  	  == PACKET_ENABLE);
>  }
>  
>  bool
>  remote_target::supports_string_tracing ()
>  {
> -  return packet_support (PACKET_tracenz_feature) == PACKET_ENABLE;
> +  return m_features.packet_support (PACKET_tracenz_feature) == PACKET_ENABLE;
>  }
>  
>  bool
>  remote_target::can_run_breakpoint_commands ()
>  {
> -  return packet_support (PACKET_BreakpointCommands) == PACKET_ENABLE;
> +  return m_features.packet_support (PACKET_BreakpointCommands) == PACKET_ENABLE;
>  }
>  
>  void
> @@ -13361,7 +13464,7 @@ remote_target::download_tracepoint (struct bp_location *loc)
>  	error (_("Error on target while setting tracepoints."));
>      }
>  
> -  if (packet_support (PACKET_TracepointSource) == PACKET_ENABLE)
> +  if (m_features.packet_support (PACKET_TracepointSource) == PACKET_ENABLE)
>      {
>        if (b->location != NULL)
>  	{
> @@ -13517,7 +13620,8 @@ remote_target::trace_set_readonly_regions ()
>        sec_length = 1 + strlen (tmp1) + 1 + strlen (tmp2);
>        if (offset + sec_length + 1 > rs->buf.size ())
>  	{
> -	  if (packet_support (PACKET_qXfer_traceframe_info) != PACKET_ENABLE)
> +	  if (m_features.packet_support (PACKET_qXfer_traceframe_info)
> +	      != PACKET_ENABLE)
>  	    warning (_("\
>  Too many sections for read-only sections definition packet."));
>  	  break;
> @@ -13554,7 +13658,7 @@ remote_target::get_trace_status (struct trace_status *ts)
>    enum packet_result result;
>    struct remote_state *rs = get_remote_state ();
>  
> -  if (packet_support (PACKET_qTStatus) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_qTStatus) == PACKET_DISABLE)
>      return -1;
>  
>    /* FIXME we need to get register block size some other way.  */
> @@ -13577,7 +13681,7 @@ remote_target::get_trace_status (struct trace_status *ts)
>        throw;
>      }
>  
> -  result = packet_ok (p, &remote_protocol_packets[PACKET_qTStatus]);
> +  result = packet_ok (p, &m_features.m_protocol_packets[PACKET_qTStatus]);
>  
>    /* If the remote target doesn't do tracing, flag it.  */
>    if (result == PACKET_UNKNOWN)
> @@ -13832,7 +13936,8 @@ remote_target::set_disconnected_tracing (int val)
>  {
>    struct remote_state *rs = get_remote_state ();
>  
> -  if (packet_support (PACKET_DisconnectedTracing_feature) == PACKET_ENABLE)
> +  if (m_features.packet_support (PACKET_DisconnectedTracing_feature)
> +      == PACKET_ENABLE)
>      {
>        char *reply;
>  
> @@ -13926,7 +14031,7 @@ remote_target::get_min_fast_tracepoint_insn_len ()
>  void
>  remote_target::set_trace_buffer_size (LONGEST val)
>  {
> -  if (packet_support (PACKET_QTBuffer_size) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_QTBuffer_size) != PACKET_DISABLE)
>      {
>        struct remote_state *rs = get_remote_state ();
>        char *buf = rs->buf.data ();
> @@ -13947,7 +14052,7 @@ remote_target::set_trace_buffer_size (LONGEST val)
>        putpkt (rs->buf);
>        remote_get_noisy_reply ();
>        result = packet_ok (rs->buf,
> -		  &remote_protocol_packets[PACKET_QTBuffer_size]);
> +		  &m_features.m_protocol_packets[PACKET_QTBuffer_size]);
>  
>        if (result != PACKET_OK)
>  	warning (_("Bogus reply from target: %s"), rs->buf.data ());
> @@ -14003,7 +14108,7 @@ remote_target::set_trace_notes (const char *user, const char *notes,
>  bool
>  remote_target::use_agent (bool use)
>  {
> -  if (packet_support (PACKET_QAgent) != PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_QAgent) != PACKET_DISABLE)
>      {
>        struct remote_state *rs = get_remote_state ();
>  
> @@ -14025,7 +14130,7 @@ remote_target::use_agent (bool use)
>  bool
>  remote_target::can_use_agent ()
>  {
> -  return (packet_support (PACKET_QAgent) != PACKET_DISABLE);
> +  return (m_features.packet_support (PACKET_QAgent) != PACKET_DISABLE);
>  }
>  
>  struct btrace_target_info
> @@ -14058,7 +14163,7 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
>    buf = rs->buf.data ();
>    endbuf = buf + get_remote_packet_size ();
>  
> -  packet = &remote_protocol_packets[PACKET_Qbtrace_conf_bts_size];
> +  packet = &m_features.m_protocol_packets[PACKET_Qbtrace_conf_bts_size];
>    if (packet_config_support (packet) == PACKET_ENABLE
>        && conf->bts.size != rs->btrace_config.bts.size)
>      {
> @@ -14080,7 +14185,7 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
>        rs->btrace_config.bts.size = conf->bts.size;
>      }
>  
> -  packet = &remote_protocol_packets[PACKET_Qbtrace_conf_pt_size];
> +  packet = &m_features.m_protocol_packets[PACKET_Qbtrace_conf_pt_size];
>    if (packet_config_support (packet) == PACKET_ENABLE
>        && conf->pt.size != rs->btrace_config.pt.size)
>      {
> @@ -14129,7 +14234,7 @@ remote_target::remote_btrace_maybe_reopen ()
>  
>    /* Don't bother walking the entirety of the remote thread list when
>       we know the feature isn't supported by the remote.  */
> -  if (packet_support (PACKET_qXfer_btrace_conf) != PACKET_ENABLE)
> +  if (m_features.packet_support (PACKET_qXfer_btrace_conf) != PACKET_ENABLE)
>      return;
>  
>    scoped_restore_current_thread restore_thread;
> @@ -14189,11 +14294,11 @@ remote_target::enable_btrace (ptid_t ptid, const struct btrace_config *conf)
>    switch (conf->format)
>      {
>        case BTRACE_FORMAT_BTS:
> -	packet = &remote_protocol_packets[PACKET_Qbtrace_bts];
> +	packet = &m_features.m_protocol_packets[PACKET_Qbtrace_bts];
>  	break;
>  
>        case BTRACE_FORMAT_PT:
> -	packet = &remote_protocol_packets[PACKET_Qbtrace_pt];
> +	packet = &m_features.m_protocol_packets[PACKET_Qbtrace_pt];
>  	break;
>      }
>  
> @@ -14241,7 +14346,8 @@ remote_target::enable_btrace (ptid_t ptid, const struct btrace_config *conf)
>  void
>  remote_target::disable_btrace (struct btrace_target_info *tinfo)
>  {
> -  struct packet_config *packet = &remote_protocol_packets[PACKET_Qbtrace_off];
> +  packet_config *packet
> +    = &m_features.m_protocol_packets[PACKET_Qbtrace_off];
>    struct remote_state *rs = get_remote_state ();
>    char *buf = rs->buf.data ();
>    char *endbuf = buf + get_remote_packet_size ();
> @@ -14284,7 +14390,7 @@ remote_target::read_btrace (struct btrace_data *btrace,
>  			    struct btrace_target_info *tinfo,
>  			    enum btrace_read_type type)
>  {
> -  struct packet_config *packet = &remote_protocol_packets[PACKET_qXfer_btrace];
> +  packet_config *packet = &m_features.m_protocol_packets[PACKET_qXfer_btrace];
>    const char *annex;
>  
>    if (packet_config_support (packet) != PACKET_ENABLE)
> @@ -14331,8 +14437,9 @@ remote_target::btrace_conf (const struct btrace_target_info *tinfo)
>  bool
>  remote_target::augmented_libraries_svr4_read ()
>  {
> -  return (packet_support (PACKET_augmented_libraries_svr4_read_feature)
> -	  == PACKET_ENABLE);
> +  return
> +    (m_features.packet_support (PACKET_augmented_libraries_svr4_read_feature)
> +     == PACKET_ENABLE);
>  }
>  
>  /* Implementation of to_load.  */
> @@ -14353,7 +14460,7 @@ remote_target::pid_to_exec_file (int pid)
>    static gdb::optional<gdb::char_vector> filename;
>    char *annex = NULL;
>  
> -  if (packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
> +  if (m_features.packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
>      return NULL;
>  
>    inferior *inf = find_inferior_pid (this, pid);
> @@ -14385,11 +14492,11 @@ remote_target::can_do_single_step ()
>       feature.  If the stub doesn't support vContSupported feature,
>       we have conservatively to think target doesn't supports single
>       step.  */
> -  if (packet_support (PACKET_vContSupported) == PACKET_ENABLE)
> +  if (m_features.packet_support (PACKET_vContSupported) == PACKET_ENABLE)
>      {
>        struct remote_state *rs = get_remote_state ();
>  
> -      if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
> +      if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
>  	remote_vcont_probe ();
>  
>        return rs->supports_vCont.s && rs->supports_vCont.S;
> @@ -14529,7 +14636,7 @@ remote_target::thread_events (int enable)
>    struct remote_state *rs = get_remote_state ();
>    size_t size = get_remote_packet_size ();
>  
> -  if (packet_support (PACKET_QThreadEvents) == PACKET_DISABLE)
> +  if (m_features.packet_support (PACKET_QThreadEvents) == PACKET_DISABLE)
>      return;
>  
>    xsnprintf (rs->buf.data (), size, "QThreadEvents:%x", enable ? 1 : 0);
> @@ -14537,7 +14644,7 @@ remote_target::thread_events (int enable)
>    getpkt (&rs->buf, 0);
>  
>    switch (packet_ok (rs->buf,
> -		     &remote_protocol_packets[PACKET_QThreadEvents]))
> +		     &m_features.m_protocol_packets[PACKET_QThreadEvents]))
>      {
>      case PACKET_OK:
>        if (strcmp (rs->buf.data (), "OK") != 0)
> @@ -14674,10 +14781,10 @@ show_range_stepping (struct ui_file *file, int from_tty,
>  bool
>  remote_target::vcont_r_supported ()
>  {
> -  if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
> +  if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
>      remote_vcont_probe ();
>  
> -  return (packet_support (PACKET_vCont) == PACKET_ENABLE
> +  return (m_features.packet_support (PACKET_vCont) == PACKET_ENABLE
>  	  && get_remote_state ()->supports_vCont.r);
>  }
>  
> @@ -14720,7 +14827,7 @@ show_remote_timeout (struct ui_file *file, int from_tty,
>  bool
>  remote_target::supports_memory_tagging ()
>  {
> -  return remote_memory_tagging_p ();
> +  return m_features.remote_memory_tagging_p ();
>  }
>  
>  /* Create the qMemTags packet given ADDRESS, LEN and TYPE.  */
> @@ -14786,7 +14893,7 @@ remote_target::fetch_memtags (CORE_ADDR address, size_t len,
>  			      gdb::byte_vector &tags, int type)
>  {
>    /* Make sure the qMemTags packet is supported.  */
> -  if (!remote_memory_tagging_p ())
> +  if (!m_features.remote_memory_tagging_p ())
>      gdb_assert_not_reached ("remote fetch_memtags called with packet disabled");
>  
>    struct remote_state *rs = get_remote_state ();
> @@ -14806,7 +14913,7 @@ remote_target::store_memtags (CORE_ADDR address, size_t len,
>  			      const gdb::byte_vector &tags, int type)
>  {
>    /* Make sure the QMemTags packet is supported.  */
> -  if (!remote_memory_tagging_p ())
> +  if (!m_features.remote_memory_tagging_p ())
>      gdb_assert_not_reached ("remote store_memtags called with packet disabled");
>  
>    struct remote_state *rs = get_remote_state ();
> @@ -14841,7 +14948,7 @@ test_memory_tagging_functions ()
>    remote_target remote;
>  
>    struct packet_config *config
> -    = &remote_protocol_packets[PACKET_memory_tagging_feature];
> +    = &remote.m_features.m_protocol_packets[PACKET_memory_tagging_feature];
>  
>    scoped_restore restore_memtag_support_
>      = make_scoped_restore (&config->support);
> diff --git a/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp b/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp
> index 8b0c6c91a2..712dd0b7e5 100644
> --- a/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp
> +++ b/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp
> @@ -30,15 +30,13 @@ set run_python_tests [expr ! [skip_python_tests]]
>  # indicates whether the multi-process feature of remote targets is
>  # turned off or on.
>  proc test_info_inferiors {multi_process} {
> -    setup "off"
> +
> +    setup "off" $multi_process
>  
>      if { $::run_python_tests } {
>  	gdb_test_no_output "source ${::remote_python_file}" "load python file"
>      }
>  
> -    gdb_test_no_output \
> -	"set remote multiprocess-feature-packet $multi_process"
> -
>      # Get the description for inferior INF for when the current
>      # inferior id is CURRENT.
>      proc inf_desc {inf current} {
> diff --git a/gdb/testsuite/gdb.multi/multi-target.exp.tcl b/gdb/testsuite/gdb.multi/multi-target.exp.tcl
> index 8e0c39679b..f9bd6412d7 100644
> --- a/gdb/testsuite/gdb.multi/multi-target.exp.tcl
> +++ b/gdb/testsuite/gdb.multi/multi-target.exp.tcl
> @@ -104,7 +104,7 @@ proc cleanup_gdbservers { } {
>  
>  # Return true on success, false otherwise.
>  
> -proc setup {non-stop} {
> +proc setup {non-stop {multi_process ""}} {
>      global gcorefile gcore_created
>      global binfile
>  
> @@ -123,6 +123,11 @@ proc setup {non-stop} {
>  
>      gdb_test_no_output "set non-stop ${non-stop}"
>  
> +    if {${multi_process} ne ""} then {
> +	gdb_test_no_output \
> +	    "set remote multiprocess-feature-packet $multi_process"
> +    }
> +
>      if ![runto all_started] then {
>  	return 0
>      }
> -- 
> 2.25.1
> 
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config
  2022-01-13 15:21 ` [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe
  2022-01-14 19:33   ` Tom Tromey
@ 2022-01-18 11:39   ` Andrew Burgess
  2022-01-24 15:06     ` Schimpe, Christina
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2022-01-18 11:39 UTC (permalink / raw)
  To: Christina Schimpe; +Cc: gdb-patches

* Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org> [2022-01-13 16:21:17 +0100]:

> This patch adds per-remote target variables for the configuration of
> memory read- and write packet size. It is a further change in addition
> to commit "gdb: Make global feature array a per-remote target array"
> to apply the fixme notes described in commit 5b6d1e4.
> 
> The former global variables for that configuration are still available
> to allow the command line configuration for all future remote
> connections.  Similar to the command line configuration of the per-
> remote target feature array, the commands
> 
> - set remotewritesize (deprecated)
> - set remote memory-read-packet-size
> - set remote memory-write-packet-size
> 
> will configure the current target (if available) and future remote
> connections. The show command will display the global configuration
> and the packet size of the current remote target, if available.
> 
> It is required to adapt the test gdb.base/remote.exp which is failing
> for --target_board=native-extended-gdbserver.  With that board GDB
> connects to gdbserver at gdb start time.  Due to this patch two loggings
> "The target may not be able to.." are shown if the command 'set remote
> memory-write-packet-size fixed' is executed while a target is connected
> for the current inferior.  To fix this, the clean_restart command is
> moved to a later time point of the test.  It is sufficient to be
> connected to the server when "runto_main" is executed.  Now the
> connection time is similar to a testrun with
> --target_board=native-gdbserver.
> 
> To allow the user to distinguish between the packet-size configuration
> for future connections and for the currently selected target, the
> logging of the command 'set remote memory-write-packet-size fixed' is
> adapted.
> ---
>  gdb/remote.c                      | 94 +++++++++++++++++++------------
>  gdb/testsuite/gdb.base/remote.exp |  7 ++-
>  2 files changed, 61 insertions(+), 40 deletions(-)

This needs a NEWS entry and a docs update.

Like with the previous patch, I think we should consider having the
set/show commands print more informative messages about which targets
are being changed or displayed.

> 
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 10f9226827..a270cb688c 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -563,8 +563,31 @@ struct packet_config
>      enum packet_support support;
>    };
>  
> -/* This global array contains the default configuration for every new
> -   per-remote target array.  */
> +/* User configurable variables for the number of characters in a
> +   memory read/write packet.  MIN (rsa->remote_packet_size,
> +   rsa->sizeof_g_packet) is the default.  Some targets need smaller
> +   values (fifo overruns, et.al.) and some users need larger values
> +   (speed up transfers).  The variables ``preferred_*'' (the user
> +   request), ``current_*'' (what was actually set) and ``forced_*''
> +   (Positive - a soft limit, negative - a hard limit).  */
> +
> +struct memory_packet_config
> +{
> +  const char *name;
> +  long size;
> +  int fixed_p;
> +};
> +
> +/* These global variables contain the default configuration for every new
> +   remote_feature object.  */
> +static memory_packet_config memory_read_packet_config =
> +{
> +  "memory-read-packet-size",
> +};
> +static memory_packet_config memory_write_packet_config =
> +{
> +  "memory-write-packet-size",
> +};
>  static packet_config remote_protocol_packets[PACKET_MAX];
>  
>  class remote_features
> @@ -573,6 +596,9 @@ class remote_features
>  
>    remote_features ()
>    {
> +    m_memory_read_packet_config = memory_read_packet_config;
> +    m_memory_write_packet_config = memory_write_packet_config;
> +
>      std::copy (std::begin (remote_protocol_packets),
>  	       std::end (remote_protocol_packets),
>  	       std::begin (m_protocol_packets));
> @@ -591,6 +617,9 @@ class remote_features
>  
>    void reset_all_packet_configs_support (void);
>  
> +  memory_packet_config m_memory_read_packet_config;
> +  memory_packet_config m_memory_write_packet_config;

Comment new fields please.

> +
>    packet_config m_protocol_packets[PACKET_MAX];
>  };
>  
> @@ -1842,21 +1871,6 @@ show_remotebreak (struct ui_file *file, int from_tty,
>  static unsigned int remote_address_size;
>  
>  \f
> -/* User configurable variables for the number of characters in a
> -   memory read/write packet.  MIN (rsa->remote_packet_size,
> -   rsa->sizeof_g_packet) is the default.  Some targets need smaller
> -   values (fifo overruns, et.al.) and some users need larger values
> -   (speed up transfers).  The variables ``preferred_*'' (the user
> -   request), ``current_*'' (what was actually set) and ``forced_*''
> -   (Positive - a soft limit, negative - a hard limit).  */
> -
> -struct memory_packet_config
> -{
> -  const char *name;
> -  long size;
> -  int fixed_p;
> -};
> -
>  /* The default max memory-write-packet-size, when the setting is
>     "fixed".  The 16k is historical.  (It came from older GDB's using
>     alloca for buffers and the knowledge (folklore?) that some hosts
> @@ -1922,7 +1936,8 @@ remote_target::get_memory_packet_size (struct memory_packet_config *config)
>     something really big then do a sanity check.  */
>  
>  static void
> -set_memory_packet_size (const char *args, struct memory_packet_config *config)
> +set_memory_packet_size (const char *args, struct memory_packet_config *config,
> +			bool target_connected)
>  {
>    int fixed_p = config->fixed_p;
>    long size = config->size;
> @@ -1956,9 +1971,16 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config)
>  			 ? DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED
>  			 : size);
>  
> -      if (! query (_("The target may not be able to correctly handle a %s\n"
> -		   "of %ld bytes. Change the packet size? "),
> -		   config->name, query_size))
> +      if (target_connected
> +	  && ! query (_("The target may not be able to correctly handle a %s\n"

The space after '!' is not gdb style, could you remove it please.
Also the two below please.

Thanks,
Andrew

> +		      "of %ld bytes.  Change the packet size? "),
> +		      config->name, query_size))
> +	error (_("Packet size not changed."));
> +      else if (! target_connected
> +	       && ! query (_("Future targets may not be able to correctly "
> +			   "handle a %s\nof %ld bytes.  Change the packet size "
> +			   "for future remote targets? "),
> +			   config->name, query_size))
>  	error (_("Packet size not changed."));
>      }
>    /* Update the config.  */
> @@ -1989,16 +2011,15 @@ show_memory_packet_size (struct memory_packet_config *config)
>      }
>  }
>  
> -/* FIXME: needs to be per-remote-target.  */
> -static struct memory_packet_config memory_write_packet_config =
> -{
> -  "memory-write-packet-size",
> -};
> -
>  static void
>  set_memory_write_packet_size (const char *args, int from_tty)
>  {
> -  set_memory_packet_size (args, &memory_write_packet_config);
> +  remote_target *remote = get_current_remote_target ();
> +  if (remote != nullptr)
> +    set_memory_packet_size
> +      (args, &remote->m_features.m_memory_write_packet_config, true);
> +
> +  set_memory_packet_size (args, &memory_write_packet_config, false);
>  }
>  
>  static void
> @@ -2060,19 +2081,18 @@ show_remote_packet_max_chars (struct ui_file *file, int from_tty,
>  long
>  remote_target::get_memory_write_packet_size ()
>  {
> -  return get_memory_packet_size (&memory_write_packet_config);
> +  return get_memory_packet_size (&m_features.m_memory_write_packet_config);
>  }
>  
> -/* FIXME: needs to be per-remote-target.  */
> -static struct memory_packet_config memory_read_packet_config =
> -{
> -  "memory-read-packet-size",
> -};
> -
>  static void
>  set_memory_read_packet_size (const char *args, int from_tty)
>  {
> -  set_memory_packet_size (args, &memory_read_packet_config);
> +  remote_target *remote = get_current_remote_target ();
> +  if (remote != nullptr)
> +    set_memory_packet_size
> +      (args, &remote->m_features.m_memory_read_packet_config, true);
> +
> +  set_memory_packet_size (args, &memory_read_packet_config, false);
>  }
>  
>  static void
> @@ -2084,7 +2104,7 @@ show_memory_read_packet_size (const char *args, int from_tty)
>  long
>  remote_target::get_memory_read_packet_size ()
>  {
> -  long size = get_memory_packet_size (&memory_read_packet_config);
> +  long size = get_memory_packet_size (&m_features.m_memory_read_packet_config);
>  
>    /* FIXME: cagney/1999-11-07: Functions like getpkt() need to get an
>       extra buffer size argument before the memory read size can be
> diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp
> index 1f0869433f..31c5adfd0d 100644
> --- a/gdb/testsuite/gdb.base/remote.exp
> +++ b/gdb/testsuite/gdb.base/remote.exp
> @@ -62,7 +62,7 @@ gdb_test "show remote memory-write-packet-size" \
>  
>  set test "set remote memory-write-packet-size fixed"
>  gdb_test_multiple $test $test {
> -    -re "Change the packet size. .y or n. " {
> +    -re "Change the packet size for future remote targets. .y or n. " {
>  	gdb_test_multiple "y" $test {
>  	    -re "$gdb_prompt $" {
>  		pass $test
> @@ -70,6 +70,7 @@ gdb_test_multiple $test $test {
>  	}
>      }
>  }
> +
>  gdb_test "show remote memory-write-packet-size" \
>  	"The memory-write-packet-size is 0 \\(default\\). Packets are fixed at 16384 bytes\." \
>  	"write-packet default fixed"
> @@ -129,8 +130,6 @@ proc gdb_load_timed {executable class writesize} {
>      pass $test
>  }
>  
> -clean_restart $binfile
> -
>  # These download tests won't actually download anything on !is_remote
>  # target boards, but we run them anyway because it's simpler, and
>  # harmless.
> @@ -155,6 +154,8 @@ gdb_load_timed $binfile "limit" 0
>  # Get the size of random_data table (defaults to 48K).
>  set sizeof_random_data [get_sizeof "random_data" 48*1024]
>  
> +clean_restart $binfile
> +
>  #
>  # Part THREE: Check the upload behavour
>  #
> -- 
> 2.25.1
> 
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/3] gdb: Make global feature array a per-remote target array
  2022-01-14 19:30   ` Tom Tromey
@ 2022-01-20 16:32     ` Schimpe, Christina
  0 siblings, 0 replies; 14+ messages in thread
From: Schimpe, Christina @ 2022-01-20 16:32 UTC (permalink / raw)
  To: Tom Tromey, Christina Schimpe via Gdb-patches

Thank you for the review.

> >> +  int remote_multi_process_p (remote_state *rs) const;
> >> +  int remote_fork_event_p (remote_state*) const;
> >> +  int remote_vfork_event_p (remote_state*) const;
> >> +  int remote_exec_event_p (remote_state*) const;
> 
> It seems like these methods don't actually need the 'remote_state *'
> parameter.
> 
> >> +  void reset_all_packet_configs_support (void);
> 
> New code shouldn't use "(void)", just "()" is fine.  Also, normally if
> we touch a function definition or declaration, we'll also remove a
> 'void' if there is one.

Yes, I agree. I will remove it.

> The packet number can be passed in using the 'context' attached to the
> command.  See cmd_list_element::set_context and
> cmd_list_element::context.
> This is simple to do and would remove the need for this loop.
> 
> It's mildly annoying that this only accepts a pointer and not a scalar,
> but it can either be cast or you could pass the address the slot in
> remote_protocol_packets and then do some pointer math to reconstruct the
> index.
> 
> The same applies to the 'show' command.

Ah, thanks for the hint. I will use the 'context' and remove the loop.

Regards,
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/3] gdb: Make global feature array a per-remote target array
  2022-01-18 11:30   ` Andrew Burgess
@ 2022-01-20 17:24     ` Schimpe, Christina
  0 siblings, 0 replies; 14+ messages in thread
From: Schimpe, Christina @ 2022-01-20 17:24 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

Thanks for the feedback.

> Could you update your examples to not use implicit values here
> please.  This just makes reading these that little bit harder for no
> reason.

Yes, you are right. I will update this.
 
> However, I do think there are some things we could do to make it clear
> what's going on, I think that the string printed for 'show remote
> PACKET-NAME' should be updated to reflect what it's showing, as in:
> 
>   (gdb) show remote kill-packet
>   Support for the `vKill' packet on the current remote target, is "auto"
> (currently unknown).
> 
> OR
> 
>   (gdb) show remote kill-packet
>   Support for the `vKill' packet on newly created remote targets, is "auto".
> 
> Notice, I dropped the 'currently unknown' as that makes no sense when
> talking about the future targets.
> 
> I also think we should have the set command print something to tell
> the user what changed, so they can know if they messed up, maybe:
> 
>  (gdb) set remote kill-packet off
>  Use of the 'vKill' packet for the current, and future remote targets, set to
> "off".
> 
> OR
> 
>  (gdb) set remote kill-packet off
>  Use of the 'vKill' packet for future remote targets, set to "off".

Yes, that makes sense to me. I will adapt the set/show command prints,
as well as all the tests using the set/show commands. 

> 
> This change absolutely needs a NEWS entry and also a documentation
> update.
> 
> With one exception the rest of my feedback is basically: there should
> be more comments added.  I know that sucks, especially when working in
> older files like remote.c, where the initial code is poorly commented
> anyway, but for me good comments do help understand what the intention
> of a field or struct is, so I think, when we change stuff, we should
> take the opportunity to document what's going on.  So...

I also agree with the rest of your comments and will send a V2 for this soon.

Thanks,
Christina

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config
  2022-01-18 11:39   ` Andrew Burgess
@ 2022-01-24 15:06     ` Schimpe, Christina
  2022-02-04 18:03       ` Andrew Burgess
  0 siblings, 1 reply; 14+ messages in thread
From: Schimpe, Christina @ 2022-01-24 15:06 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

Hi Andrew,

Thanks a lot for your feedback.
Before sending a V2 for this patch, there is still one point I would like to discuss
(refer to my answer to your comment below).

I also noticed that I missed an adaption in the function
static void
show_memory_packet_size (struct memory_packet_config *config)

at 
if (remote != NULL)
  printf_filtered (_("Packets are limited to %ld bytes.\n"),
		 remote->get_memory_packet_size (config));

In the function call get_memory_packet_size the global configuration is passed,
but the current target's configuration should be used.
I will correct this with the next version of this patch. 

> > The former global variables for that configuration are still available
> > to allow the command line configuration for all future remote
> > connections.  Similar to the command line configuration of the per-
> > remote target feature array, the commands
> >
> > - set remotewritesize (deprecated)
> > - set remote memory-read-packet-size
> > - set remote memory-write-packet-size
> >
> > will configure the current target (if available) and future remote
> > connections. The show command will display the global configuration
> > and the packet size of the current remote target, if available.
> >
> > It is required to adapt the test gdb.base/remote.exp which is failing
> > for --target_board=native-extended-gdbserver.  With that board GDB
> > connects to gdbserver at gdb start time.  Due to this patch two
> > loggings "The target may not be able to.." are shown if the command
> > 'set remote memory-write-packet-size fixed' is executed while a target
> > is connected for the current inferior.  To fix this, the clean_restart
> > command is moved to a later time point of the test.  It is sufficient
> > to be connected to the server when "runto_main" is executed.  Now the
> > connection time is similar to a testrun with
> > --target_board=native-gdbserver.
> >
> > To allow the user to distinguish between the packet-size configuration
> > for future connections and for the currently selected target, the
> > logging of the command 'set remote memory-write-packet-size fixed' is
> > adapted.
> > ---
> >  gdb/remote.c                      | 94 +++++++++++++++++++------------
> >  gdb/testsuite/gdb.base/remote.exp |  7 ++-
> >  2 files changed, 61 insertions(+), 40 deletions(-)
> 
> This needs a NEWS entry and a docs update.
> 
> Like with the previous patch, I think we should consider having the set/show
> commands print more informative messages about which targets are being
> changed or displayed.
> 

Yes, I agree, this could be improved.  I was wondering if we should then adapt
the show command such that it behaves similar to the previous patch e.g. 
"The show command always displays the current remote target's
configuration.  If no remote target is selected the default
configuration for future connections is shown.". 
For the GDB user it might be less confusing if we keep the behaviour of the
"show remote"  commands  consistent. 

> >
> > +  memory_packet_config m_memory_read_packet_config;
> > + memory_packet_config m_memory_write_packet_config;
> 
> Comment new fields please.

Yes, I will adapt it.

> > @@ -1956,9 +1971,16 @@ set_memory_packet_size (const char *args,
> struct memory_packet_config *config)
> >  			 ? DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED
> >  			 : size);
> >
> > -      if (! query (_("The target may not be able to correctly handle a %s\n"
> > -		   "of %ld bytes. Change the packet size? "),
> > -		   config->name, query_size))
> > +      if (target_connected
> > +	  && ! query (_("The target may not be able to correctly handle a
> %s\n"
> 
> The space after '!' is not gdb style, could you remove it please.
> Also the two below please.
> 
> Thanks,
> Andrew

Thank you, I will adapt it.

Best Regards,
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 3/3] gdb: Remove workaround for the vCont packet
  2022-01-14 19:42   ` Tom Tromey
@ 2022-01-28 13:28     ` Schimpe, Christina
  0 siblings, 0 replies; 14+ messages in thread
From: Schimpe, Christina @ 2022-01-28 13:28 UTC (permalink / raw)
  To: Tom Tromey, Christina Schimpe via Gdb-patches

Hi Tom,

Thanks a lot for your review. 

> 
> >> +  /* Check vCont support and set the remote state's
> vCont_action_support
> >> +     attribute.  */
> >> +  remote_vcont_probe ();
> 
> >> -  if (m_features.packet_support (PACKET_vCont) ==
> PACKET_SUPPORT_UNKNOWN)
> >> -    remote_vcont_probe ();
> 
> It seems like the new code should probably check whether the packet is
> supported like the old code did, in case the gdb user disabled it using the
> appropriate "set remote" command.

With the current patch, we do not check if the vCont packet is enabled when
the supports_vCont attribute is set for the target in the function
remote_vcont_probe ().  We do check if the packet is enabled, when the
attribute is used later on. 

To be consistent with the definition of the set/show remote commands in commit
"gdb: Make global feature array a per-remote target array", we should probably
allow the user to enable/disable the vCont packet after the connection is made
for the current target. 
In this case, we have to make sure that we check the vCont packet enabling
every time the supports_vCont attribute is used. 
I think for this approach, there should not be a check for packet support
before the function call remote_vcont_probe (), because the user could
enable the packet after the connection is made and then the 
supports_vCont attribute should be configured correctly. So I don't think this
patch removed any of the required checks for vCont support based on the 
approach I described for this patch.

But there may be other situations for which a check for vCont support is missing. 
For instance, when the supports_vCont attribute is used in the function
set_range_stepping (const char *ignore_args, int from_tty, 
struct cmd_list_element *c) and potentially also other places in the code.
However,  I would prefer to address them in another patch.

So in case the approach I described before sounds reasonable to you, 
would the current version of this patch be acceptable? 

Best Regards + thanks a lot in advance,
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config
  2022-01-24 15:06     ` Schimpe, Christina
@ 2022-02-04 18:03       ` Andrew Burgess
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Burgess @ 2022-02-04 18:03 UTC (permalink / raw)
  To: Schimpe, Christina; +Cc: gdb-patches

* Schimpe, Christina via Gdb-patches <gdb-patches@sourceware.org> [2022-01-24 15:06:31 +0000]:

> Hi Andrew,
> 
> Thanks a lot for your feedback.
> Before sending a V2 for this patch, there is still one point I would like to discuss
> (refer to my answer to your comment below).
> 
> I also noticed that I missed an adaption in the function
> static void
> show_memory_packet_size (struct memory_packet_config *config)
> 
> at 
> if (remote != NULL)
>   printf_filtered (_("Packets are limited to %ld bytes.\n"),
> 		 remote->get_memory_packet_size (config));
> 
> In the function call get_memory_packet_size the global configuration is passed,
> but the current target's configuration should be used.
> I will correct this with the next version of this patch. 
> 
> > > The former global variables for that configuration are still available
> > > to allow the command line configuration for all future remote
> > > connections.  Similar to the command line configuration of the per-
> > > remote target feature array, the commands
> > >
> > > - set remotewritesize (deprecated)
> > > - set remote memory-read-packet-size
> > > - set remote memory-write-packet-size
> > >
> > > will configure the current target (if available) and future remote
> > > connections. The show command will display the global configuration
> > > and the packet size of the current remote target, if available.
> > >
> > > It is required to adapt the test gdb.base/remote.exp which is failing
> > > for --target_board=native-extended-gdbserver.  With that board GDB
> > > connects to gdbserver at gdb start time.  Due to this patch two
> > > loggings "The target may not be able to.." are shown if the command
> > > 'set remote memory-write-packet-size fixed' is executed while a target
> > > is connected for the current inferior.  To fix this, the clean_restart
> > > command is moved to a later time point of the test.  It is sufficient
> > > to be connected to the server when "runto_main" is executed.  Now the
> > > connection time is similar to a testrun with
> > > --target_board=native-gdbserver.
> > >
> > > To allow the user to distinguish between the packet-size configuration
> > > for future connections and for the currently selected target, the
> > > logging of the command 'set remote memory-write-packet-size fixed' is
> > > adapted.
> > > ---
> > >  gdb/remote.c                      | 94 +++++++++++++++++++------------
> > >  gdb/testsuite/gdb.base/remote.exp |  7 ++-
> > >  2 files changed, 61 insertions(+), 40 deletions(-)
> > 
> > This needs a NEWS entry and a docs update.
> > 
> > Like with the previous patch, I think we should consider having the set/show
> > commands print more informative messages about which targets are being
> > changed or displayed.
> > 
> 
> Yes, I agree, this could be improved.  I was wondering if we should then adapt
> the show command such that it behaves similar to the previous patch e.g. 
> "The show command always displays the current remote target's
> configuration.  If no remote target is selected the default
> configuration for future connections is shown.".

That sounds reasonable to me, and inline with the previous patch.

> For the GDB user it might be less confusing if we keep the behaviour of the
> "show remote"  commands  consistent.

I agree that being consistent across commands, especially related
commands is super important.

Thanks,
Andrew



> 
> > >
> > > +  memory_packet_config m_memory_read_packet_config;
> > > + memory_packet_config m_memory_write_packet_config;
> > 
> > Comment new fields please.
> 
> Yes, I will adapt it.
> 
> > > @@ -1956,9 +1971,16 @@ set_memory_packet_size (const char *args,
> > struct memory_packet_config *config)
> > >  			 ? DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED
> > >  			 : size);
> > >
> > > -      if (! query (_("The target may not be able to correctly handle a %s\n"
> > > -		   "of %ld bytes. Change the packet size? "),
> > > -		   config->name, query_size))
> > > +      if (target_connected
> > > +	  && ! query (_("The target may not be able to correctly handle a
> > %s\n"
> > 
> > The space after '!' is not gdb style, could you remove it please.
> > Also the two below please.
> > 
> > Thanks,
> > Andrew
> 
> Thank you, I will adapt it.
> 
> Best Regards,
> Christina
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-02-04 18:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 15:21 [PATCH 0/3] Apply fixme notes for multi-target support Christina Schimpe
2022-01-13 15:21 ` [PATCH 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
2022-01-14 19:30   ` Tom Tromey
2022-01-20 16:32     ` Schimpe, Christina
2022-01-18 11:30   ` Andrew Burgess
2022-01-20 17:24     ` Schimpe, Christina
2022-01-13 15:21 ` [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe
2022-01-14 19:33   ` Tom Tromey
2022-01-18 11:39   ` Andrew Burgess
2022-01-24 15:06     ` Schimpe, Christina
2022-02-04 18:03       ` Andrew Burgess
2022-01-13 15:21 ` [PATCH 3/3] gdb: Remove workaround for the vCont packet Christina Schimpe
2022-01-14 19:42   ` Tom Tromey
2022-01-28 13:28     ` Schimpe, Christina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).