Re: [PATCH v2 08/12] aarch64: Add an aarch64_nat_target mixin class.

[Luis Machado <luis.machado@arm.com>]

Hi,

LGTM from aarch64's side. Just a nit below, if you'd like to address it.

On 3/16/22 20:19, John Baldwin wrote:
> This class includes platform-independent target methods for hardware
> breakpoints and watchpoints using routines from nat/aarch64-hw-point.c.
> 
> stopped_data_address is not platform-independent since the FAR register
> holding the address for a breakpoint hit must be fetched in a
> platform-specific manner.  However, aarch64_stopped_data_address is
> provided as a helper routine which performs platform-independent
> validation given the value of the FAR register.
> 
> For tracking the per-process debug register mirror state, use an
> unordered_map indexed by pid as recently adopted in x86-nat.c rather
> than a manual linked-list.
> ---
>   gdb/aarch64-linux-nat.c | 356 +---------------------------------------
>   gdb/aarch64-nat.c       | 302 ++++++++++++++++++++++++++++++++++
>   gdb/aarch64-nat.h       | 109 ++++++++++++
>   gdb/configure.nat       |   2 +-
>   4 files changed, 418 insertions(+), 351 deletions(-)
>   create mode 100644 gdb/aarch64-nat.c
>   create mode 100644 gdb/aarch64-nat.h
> 
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index dd072d9315e..7bb82d17cc8 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -27,6 +27,7 @@
>   #include "target-descriptions.h"
>   #include "auxv.h"
>   #include "gdbcmd.h"
> +#include "aarch64-nat.h"
>   #include "aarch64-tdep.h"
>   #include "aarch64-linux-tdep.h"
>   #include "aarch32-linux-nat.h"
> @@ -58,7 +59,8 @@
>   #define TRAP_HWBKPT 0x0004
>   #endif
>   
> -class aarch64_linux_nat_target final : public linux_nat_target
> +class aarch64_linux_nat_target final
> +  : public aarch64_nat_target<linux_nat_target>
>   {
>   public:
>     /* Add our register access methods.  */
> @@ -68,17 +70,8 @@ class aarch64_linux_nat_target final : public linux_nat_target
>     const struct target_desc *read_description () override;
>   
>     /* Add our hardware breakpoint and watchpoint implementation.  */
> -  int can_use_hw_breakpoint (enum bptype, int, int) override;
> -  int insert_hw_breakpoint (struct gdbarch *, struct bp_target_info *) override;
> -  int remove_hw_breakpoint (struct gdbarch *, struct bp_target_info *) override;
> -  int region_ok_for_hw_watchpoint (CORE_ADDR, int) override;
> -  int insert_watchpoint (CORE_ADDR, int, enum target_hw_bp_type,
> -			 struct expression *) override;
> -  int remove_watchpoint (CORE_ADDR, int, enum target_hw_bp_type,
> -			 struct expression *) override;
>     bool stopped_by_watchpoint () override;
>     bool stopped_data_address (CORE_ADDR *) override;
> -  bool watchpoint_addr_within_range (CORE_ADDR, CORE_ADDR, int) override;
>   
>     int can_do_single_step () override;
>   
> @@ -118,103 +111,13 @@ class aarch64_linux_nat_target final : public linux_nat_target
>   
>   static aarch64_linux_nat_target the_aarch64_linux_nat_target;
>   
> -/* Per-process data.  We don't bind this to a per-inferior registry
> -   because of targets like x86 GNU/Linux that need to keep track of
> -   processes that aren't bound to any inferior (e.g., fork children,
> -   checkpoints).  */
> -
> -struct aarch64_process_info
> -{
> -  /* Linked list.  */
> -  struct aarch64_process_info *next;
> -
> -  /* The process identifier.  */
> -  pid_t pid;
> -
> -  /* Copy of aarch64 hardware debug registers.  */
> -  struct aarch64_debug_reg_state state;
> -};
> -
> -static struct aarch64_process_info *aarch64_process_list = NULL;
> -
> -/* Find process data for process PID.  */
> -
> -static struct aarch64_process_info *
> -aarch64_find_process_pid (pid_t pid)
> -{
> -  struct aarch64_process_info *proc;
> -
> -  for (proc = aarch64_process_list; proc; proc = proc->next)
> -    if (proc->pid == pid)
> -      return proc;
> -
> -  return NULL;
> -}
> -
> -/* Add process data for process PID.  Returns newly allocated info
> -   object.  */
> -
> -static struct aarch64_process_info *
> -aarch64_add_process (pid_t pid)
> -{
> -  struct aarch64_process_info *proc;
> -
> -  proc = XCNEW (struct aarch64_process_info);
> -  proc->pid = pid;
> -
> -  proc->next = aarch64_process_list;
> -  aarch64_process_list = proc;
> -
> -  return proc;
> -}
> -
> -/* Get data specific info for process PID, creating it if necessary.
> -   Never returns NULL.  */
> -
> -static struct aarch64_process_info *
> -aarch64_process_info_get (pid_t pid)
> -{
> -  struct aarch64_process_info *proc;
> -
> -  proc = aarch64_find_process_pid (pid);
> -  if (proc == NULL)
> -    proc = aarch64_add_process (pid);
> -
> -  return proc;
> -}
> -
>   /* Called whenever GDB is no longer debugging process PID.  It deletes
>      data structures that keep track of debug register state.  */
>   
>   void
>   aarch64_linux_nat_target::low_forget_process (pid_t pid)
>   {
> -  struct aarch64_process_info *proc, **proc_link;
> -
> -  proc = aarch64_process_list;
> -  proc_link = &aarch64_process_list;
> -
> -  while (proc != NULL)
> -    {
> -      if (proc->pid == pid)
> -	{
> -	  *proc_link = proc->next;
> -
> -	  xfree (proc);
> -	  return;
> -	}
> -
> -      proc_link = &proc->next;
> -      proc = *proc_link;
> -    }
> -}
> -
> -/* Get debug registers state for process PID.  */
> -
> -struct aarch64_debug_reg_state *
> -aarch64_get_debug_reg_state (pid_t pid)
> -{
> -  return &aarch64_process_info_get (pid)->state;
> +  aarch64_remove_debug_reg_state (pid);
>   }
>   
>   /* Fill GDB's register array with the general-purpose register values
> @@ -775,192 +678,12 @@ aarch64_linux_nat_target::low_siginfo_fixup (siginfo_t *native, gdb_byte *inf,
>     return false;
>   }
>   
> -/* Returns the number of hardware watchpoints of type TYPE that we can
> -   set.  Value is positive if we can set CNT watchpoints, zero if
> -   setting watchpoints of type TYPE is not supported, and negative if
> -   CNT is more than the maximum number of watchpoints of type TYPE
> -   that we can support.  TYPE is one of bp_hardware_watchpoint,
> -   bp_read_watchpoint, bp_write_watchpoint, or bp_hardware_breakpoint.
> -   CNT is the number of such watchpoints used so far (including this
> -   one).  OTHERTYPE is non-zero if other types of watchpoints are
> -   currently enabled.  */
> -
> -int
> -aarch64_linux_nat_target::can_use_hw_breakpoint (enum bptype type,
> -						 int cnt, int othertype)
> -{
> -  if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
> -      || type == bp_access_watchpoint || type == bp_watchpoint)
> -    {
> -      if (aarch64_num_wp_regs == 0)
> -	return 0;
> -    }
> -  else if (type == bp_hardware_breakpoint)
> -    {
> -      if (aarch64_num_bp_regs == 0)
> -	return 0;
> -    }
> -  else
> -    gdb_assert_not_reached ("unexpected breakpoint type");
> -
> -  /* We always return 1 here because we don't have enough information
> -     about possible overlap of addresses that they want to watch.  As an
> -     extreme example, consider the case where all the watchpoints watch
> -     the same address and the same region length: then we can handle a
> -     virtually unlimited number of watchpoints, due to debug register
> -     sharing implemented via reference counts.  */
> -  return 1;
> -}
> -
> -/* Insert a hardware-assisted breakpoint at BP_TGT->reqstd_address.
> -   Return 0 on success, -1 on failure.  */
> -
> -int
> -aarch64_linux_nat_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
> -						struct bp_target_info *bp_tgt)
> -{
> -  int ret;
> -  CORE_ADDR addr = bp_tgt->placed_address = bp_tgt->reqstd_address;
> -  int len;
> -  const enum target_hw_bp_type type = hw_execute;
> -  struct aarch64_debug_reg_state *state
> -    = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> -
> -  gdbarch_breakpoint_from_pc (gdbarch, &addr, &len);
> -
> -  if (show_debug_regs)
> -    fprintf_unfiltered
> -      (gdb_stdlog,
> -       "insert_hw_breakpoint on entry (addr=0x%08lx, len=%d))\n",
> -       (unsigned long) addr, len);
> -
> -  ret = aarch64_handle_breakpoint (type, addr, len, 1 /* is_insert */,
> -				   inferior_ptid, state);
> -
> -  if (show_debug_regs)
> -    {
> -      aarch64_show_debug_reg_state (state,
> -				    "insert_hw_breakpoint", addr, len, type);
> -    }
> -
> -  return ret;
> -}
> -
> -/* Remove a hardware-assisted breakpoint at BP_TGT->placed_address.
> -   Return 0 on success, -1 on failure.  */
> -
> -int
> -aarch64_linux_nat_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
> -						struct bp_target_info *bp_tgt)
> -{
> -  int ret;
> -  CORE_ADDR addr = bp_tgt->placed_address;
> -  int len = 4;
> -  const enum target_hw_bp_type type = hw_execute;
> -  struct aarch64_debug_reg_state *state
> -    = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> -
> -  gdbarch_breakpoint_from_pc (gdbarch, &addr, &len);
> -
> -  if (show_debug_regs)
> -    fprintf_unfiltered
> -      (gdb_stdlog, "remove_hw_breakpoint on entry (addr=0x%08lx, len=%d))\n",
> -       (unsigned long) addr, len);
> -
> -  ret = aarch64_handle_breakpoint (type, addr, len, 0 /* is_insert */,
> -				   inferior_ptid, state);
> -
> -  if (show_debug_regs)
> -    {
> -      aarch64_show_debug_reg_state (state,
> -				    "remove_hw_watchpoint", addr, len, type);
> -    }
> -
> -  return ret;
> -}
> -
> -/* Implement the "insert_watchpoint" target_ops method.
> -
> -   Insert a watchpoint to watch a memory region which starts at
> -   address ADDR and whose length is LEN bytes.  Watch memory accesses
> -   of the type TYPE.  Return 0 on success, -1 on failure.  */
> -
> -int
> -aarch64_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
> -					     enum target_hw_bp_type type,
> -					     struct expression *cond)
> -{
> -  int ret;
> -  struct aarch64_debug_reg_state *state
> -    = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> -
> -  if (show_debug_regs)
> -    fprintf_unfiltered (gdb_stdlog,
> -			"insert_watchpoint on entry (addr=0x%08lx, len=%d)\n",
> -			(unsigned long) addr, len);
> -
> -  gdb_assert (type != hw_execute);
> -
> -  ret = aarch64_handle_watchpoint (type, addr, len, 1 /* is_insert */,
> -				   inferior_ptid, state);
> -
> -  if (show_debug_regs)
> -    {
> -      aarch64_show_debug_reg_state (state,
> -				    "insert_watchpoint", addr, len, type);
> -    }
> -
> -  return ret;
> -}
> -
> -/* Implement the "remove_watchpoint" target_ops method.
> -   Remove a watchpoint that watched the memory region which starts at
> -   address ADDR, whose length is LEN bytes, and for accesses of the
> -   type TYPE.  Return 0 on success, -1 on failure.  */
> -
> -int
> -aarch64_linux_nat_target::remove_watchpoint (CORE_ADDR addr, int len,
> -					     enum target_hw_bp_type type,
> -					     struct expression *cond)
> -{
> -  int ret;
> -  struct aarch64_debug_reg_state *state
> -    = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> -
> -  if (show_debug_regs)
> -    fprintf_unfiltered (gdb_stdlog,
> -			"remove_watchpoint on entry (addr=0x%08lx, len=%d)\n",
> -			(unsigned long) addr, len);
> -
> -  gdb_assert (type != hw_execute);
> -
> -  ret = aarch64_handle_watchpoint (type, addr, len, 0 /* is_insert */,
> -				   inferior_ptid, state);
> -
> -  if (show_debug_regs)
> -    {
> -      aarch64_show_debug_reg_state (state,
> -				    "remove_watchpoint", addr, len, type);
> -    }
> -
> -  return ret;
> -}
> -
> -/* Implement the "region_ok_for_hw_watchpoint" target_ops method.  */
> -
> -int
> -aarch64_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
> -{
> -  return aarch64_region_ok_for_watchpoint (addr, len);
> -}
> -
>   /* Implement the "stopped_data_address" target_ops method.  */
>   
>   bool
>   aarch64_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
>   {
>     siginfo_t siginfo;
> -  int i;
>     struct aarch64_debug_reg_state *state;
>   
>     if (!linux_nat_get_siginfo (inferior_ptid, &siginfo))
> @@ -980,44 +703,7 @@ aarch64_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
>   
>     /* Check if the address matches any watched address.  */
>     state = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> -  for (i = aarch64_num_wp_regs - 1; i >= 0; --i)
> -    {
> -      const unsigned int offset
> -	= aarch64_watchpoint_offset (state->dr_ctrl_wp[i]);
> -      const unsigned int len = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
> -      const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
> -      const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 8);
> -      const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
> -
> -      if (state->dr_ref_count_wp[i]
> -	  && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])
> -	  && addr_trap >= addr_watch_aligned
> -	  && addr_trap < addr_watch + len)
> -	{
> -	  /* ADDR_TRAP reports the first address of the memory range
> -	     accessed by the CPU, regardless of what was the memory
> -	     range watched.  Thus, a large CPU access that straddles
> -	     the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
> -	     ADDR_TRAP that is lower than the
> -	     ADDR_WATCH..ADDR_WATCH+LEN range.  E.g.:
> -
> -	     addr: |   4   |   5   |   6   |   7   |   8   |
> -				   |---- range watched ----|
> -		   |----------- range accessed ------------|
> -
> -	     In this case, ADDR_TRAP will be 4.
> -
> -	     To match a watchpoint known to GDB core, we must never
> -	     report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
> -	     range.  ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
> -	     positive on kernels older than 4.10.  See PR
> -	     external/20207.  */
> -	  *addr_p = addr_orig;
> -	  return true;
> -	}
> -    }
> -
> -  return false;
> +  return aarch64_stopped_data_address (state, addr_trap, addr_p);
>   }
>   
>   /* Implement the "stopped_by_watchpoint" target_ops method.  */
> @@ -1030,15 +716,6 @@ aarch64_linux_nat_target::stopped_by_watchpoint ()
>     return stopped_data_address (&addr);
>   }
>   
> -/* Implement the "watchpoint_addr_within_range" target_ops method.  */
> -
> -bool
> -aarch64_linux_nat_target::watchpoint_addr_within_range (CORE_ADDR addr,
> -							CORE_ADDR start, int length)
> -{
> -  return start <= addr && start + length - 1 >= addr;
> -}
> -
>   /* Implement the "can_do_single_step" target_ops method.  */
>   
>   int
> @@ -1114,32 +791,11 @@ aarch64_linux_nat_target::store_memtags (CORE_ADDR address, size_t len,
>     return false;
>   }
>   
> -/* Define AArch64 maintenance commands.  */
> -
> -static void
> -add_show_debug_regs_command (void)
> -{
> -  /* A maintenance command to enable printing the internal DRi mirror
> -     variables.  */
> -  add_setshow_boolean_cmd ("show-debug-regs", class_maintenance,
> -			   &show_debug_regs, _("\
> -Set whether to show variables that mirror the AArch64 debug registers."), _("\
> -Show whether to show variables that mirror the AArch64 debug registers."), _("\
> -Use \"on\" to enable, \"off\" to disable.\n\
> -If enabled, the debug registers values are shown when GDB inserts\n\
> -or removes a hardware breakpoint or watchpoint, and when the inferior\n\
> -triggers a breakpoint or watchpoint."),
> -			   NULL,
> -			   NULL,
> -			   &maintenance_set_cmdlist,
> -			   &maintenance_show_cmdlist);
> -}
> -
>   void _initialize_aarch64_linux_nat ();
>   void
>   _initialize_aarch64_linux_nat ()
>   {
> -  add_show_debug_regs_command ();
> +  aarch64_initialize_hw_point ();
>   
>     /* Register the target.  */
>     linux_target = &the_aarch64_linux_nat_target;
> diff --git a/gdb/aarch64-nat.c b/gdb/aarch64-nat.c
> new file mode 100644
> index 00000000000..85cf7f2011a
> --- /dev/null
> +++ b/gdb/aarch64-nat.c
> @@ -0,0 +1,302 @@
> +/* Native-dependent code for AArch64.
> +
> +   Copyright (C) 2011-2022 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "gdbarch.h"
> +#include "inferior.h"
> +#include "cli/cli-cmds.h"
> +#include "aarch64-nat.h"
> +
> +#include <unordered_map>
> +
> +/* Hash table storing per-process data.  We don't bind this to a
> +   per-inferior registry because of targets like x86 GNU/Linux that
> +   need to keep track of processes that aren't bound to any inferior
> +   (e.g., fork children, checkpoints).  */
> +
> +static std::unordered_map<pid_t, aarch64_debug_reg_state>
> +aarch64_debug_process_state;
> +
> +/* See aarch64-nat.h.  */
> +
> +struct aarch64_debug_reg_state *
> +aarch64_lookup_debug_reg_state (pid_t pid)
> +{
> +  auto it = aarch64_debug_process_state.find (pid);
> +  if (it != aarch64_debug_process_state.end ())
> +    return &it->second;
> +
> +  return nullptr;
> +}
> +
> +/* See aarch64-nat.h.  */
> +
> +struct aarch64_debug_reg_state *
> +aarch64_get_debug_reg_state (pid_t pid)
> +{
> +  return &aarch64_debug_process_state[pid];
> +}
> +
> +/* See aarch64-nat.h.  */
> +
> +void
> +aarch64_remove_debug_reg_state (pid_t pid)
> +{
> +  aarch64_debug_process_state.erase (pid);
> +}
> +
> +/* Returns the number of hardware watchpoints of type TYPE that we can
> +   set.  Value is positive if we can set CNT watchpoints, zero if
> +   setting watchpoints of type TYPE is not supported, and negative if
> +   CNT is more than the maximum number of watchpoints of type TYPE
> +   that we can support.  TYPE is one of bp_hardware_watchpoint,
> +   bp_read_watchpoint, bp_write_watchpoint, or bp_hardware_breakpoint.
> +   CNT is the number of such watchpoints used so far (including this
> +   one).  OTHERTYPE is non-zero if other types of watchpoints are
> +   currently enabled.  */
> +
> +int
> +aarch64_can_use_hw_breakpoint (enum bptype type, int cnt, int othertype)
> +{
> +  if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
> +      || type == bp_access_watchpoint || type == bp_watchpoint)
> +    {
> +      if (aarch64_num_wp_regs == 0)
> +	return 0;
> +    }
> +  else if (type == bp_hardware_breakpoint)
> +    {
> +      if (aarch64_num_bp_regs == 0)
> +	return 0;
> +    }
> +  else
> +    gdb_assert_not_reached ("unexpected breakpoint type");
> +
> +  /* We always return 1 here because we don't have enough information
> +     about possible overlap of addresses that they want to watch.  As an
> +     extreme example, consider the case where all the watchpoints watch
> +     the same address and the same region length: then we can handle a
> +     virtually unlimited number of watchpoints, due to debug register
> +     sharing implemented via reference counts.  */
> +  return 1;
> +}
> +
> +/* Insert a hardware-assisted breakpoint at BP_TGT->reqstd_address.
> +   Return 0 on success, -1 on failure.  */
> +
> +int
> +aarch64_insert_hw_breakpoint (struct gdbarch *gdbarch,
> +			      struct bp_target_info *bp_tgt)
> +{
> +  int ret;
> +  CORE_ADDR addr = bp_tgt->placed_address = bp_tgt->reqstd_address;
> +  int len;
> +  const enum target_hw_bp_type type = hw_execute;
> +  struct aarch64_debug_reg_state *state
> +    = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> +
> +  gdbarch_breakpoint_from_pc (gdbarch, &addr, &len);
> +
> +  if (show_debug_regs)
> +    fprintf_unfiltered
> +      (gdb_stdlog,
> +       "insert_hw_breakpoint on entry (addr=0x%08lx, len=%d))\n",
> +       (unsigned long) addr, len);
> +
> +  ret = aarch64_handle_breakpoint (type, addr, len, 1 /* is_insert */,
> +				   inferior_ptid, state);
> +
> +  if (show_debug_regs)
> +    {
> +      aarch64_show_debug_reg_state (state,
> +				    "insert_hw_breakpoint", addr, len, type);
> +    }
> +
> +  return ret;
> +}
> +
> +/* Remove a hardware-assisted breakpoint at BP_TGT->placed_address.
> +   Return 0 on success, -1 on failure.  */
> +
> +int
> +aarch64_remove_hw_breakpoint (struct gdbarch *gdbarch,
> +			      struct bp_target_info *bp_tgt)
> +{
> +  int ret;
> +  CORE_ADDR addr = bp_tgt->placed_address;
> +  int len = 4;
> +  const enum target_hw_bp_type type = hw_execute;
> +  struct aarch64_debug_reg_state *state
> +    = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> +
> +  gdbarch_breakpoint_from_pc (gdbarch, &addr, &len);
> +
> +  if (show_debug_regs)
> +    fprintf_unfiltered
> +      (gdb_stdlog, "remove_hw_breakpoint on entry (addr=0x%08lx, len=%d))\n",
> +       (unsigned long) addr, len);
> +
> +  ret = aarch64_handle_breakpoint (type, addr, len, 0 /* is_insert */,
> +				   inferior_ptid, state);
> +
> +  if (show_debug_regs)
> +    {
> +      aarch64_show_debug_reg_state (state,
> +				    "remove_hw_watchpoint", addr, len, type);
> +    }
> +
> +  return ret;
> +}
> +
> +/* Insert a watchpoint to watch a memory region which starts at
> +   address ADDR and whose length is LEN bytes.  Watch memory accesses
> +   of the type TYPE.  Return 0 on success, -1 on failure.  */
> +
> +int
> +aarch64_insert_watchpoint (CORE_ADDR addr, int len, enum target_hw_bp_type type,
> +			   struct expression *cond)
> +{
> +  int ret;
> +  struct aarch64_debug_reg_state *state
> +    = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> +
> +  if (show_debug_regs)
> +    fprintf_unfiltered (gdb_stdlog,
> +			"insert_watchpoint on entry (addr=0x%08lx, len=%d)\n",
> +			(unsigned long) addr, len);
> +
> +  gdb_assert (type != hw_execute);
> +
> +  ret = aarch64_handle_watchpoint (type, addr, len, 1 /* is_insert */,
> +				   inferior_ptid, state);
> +
> +  if (show_debug_regs)
> +    {
> +      aarch64_show_debug_reg_state (state,
> +				    "insert_watchpoint", addr, len, type);
> +    }
> +
> +  return ret;
> +}
> +
> +/* Remove a watchpoint that watched the memory region which starts at
> +   address ADDR, whose length is LEN bytes, and for accesses of the
> +   type TYPE.  Return 0 on success, -1 on failure.  */
> +
> +int
> +aarch64_remove_watchpoint (CORE_ADDR addr, int len, enum target_hw_bp_type type,
> +			   struct expression *cond)
> +{
> +  int ret;
> +  struct aarch64_debug_reg_state *state
> +    = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> +
> +  if (show_debug_regs)
> +    fprintf_unfiltered (gdb_stdlog,
> +			"remove_watchpoint on entry (addr=0x%08lx, len=%d)\n",
> +			(unsigned long) addr, len);
> +
> +  gdb_assert (type != hw_execute);
> +
> +  ret = aarch64_handle_watchpoint (type, addr, len, 0 /* is_insert */,
> +				   inferior_ptid, state);
> +
> +  if (show_debug_regs)
> +    {
> +      aarch64_show_debug_reg_state (state,
> +				    "remove_watchpoint", addr, len, type);
> +    }
> +
> +  return ret;
> +}
> +
> +/* See aarch64-nat.h.  */
> +
> +bool
> +aarch64_stopped_data_address (const struct aarch64_debug_reg_state *state,
> +			      CORE_ADDR addr_trap, CORE_ADDR *addr_p)
> +{
> +  int i;

Nit: We could declare i in the for loop below.

> +
> +  for (i = aarch64_num_wp_regs - 1; i >= 0; --i)
> +    {
> +      const unsigned int offset
> +	= aarch64_watchpoint_offset (state->dr_ctrl_wp[i]);
> +      const unsigned int len = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
> +      const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
> +      const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 8);
> +      const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
> +
> +      if (state->dr_ref_count_wp[i]
> +	  && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])
> +	  && addr_trap >= addr_watch_aligned
> +	  && addr_trap < addr_watch + len)
> +	{
> +	  /* ADDR_TRAP reports the first address of the memory range
> +	     accessed by the CPU, regardless of what was the memory
> +	     range watched.  Thus, a large CPU access that straddles
> +	     the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
> +	     ADDR_TRAP that is lower than the
> +	     ADDR_WATCH..ADDR_WATCH+LEN range.  E.g.:
> +
> +	     addr: |   4   |   5   |   6   |   7   |   8   |
> +				   |---- range watched ----|
> +		   |----------- range accessed ------------|
> +
> +	     In this case, ADDR_TRAP will be 4.
> +
> +	     To match a watchpoint known to GDB core, we must never
> +	     report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
> +	     range.  ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
> +	     positive on kernels older than 4.10.  See PR
> +	     external/20207.  */
> +	  *addr_p = addr_orig;
> +	  return true;
> +	}
> +    }
> +
> +  return false;
> +}
> +
> +/* Define AArch64 maintenance commands.  */
> +
> +static void
> +add_show_debug_regs_command (void)
> +{
> +  /* A maintenance command to enable printing the internal DRi mirror
> +     variables.  */
> +  add_setshow_boolean_cmd ("show-debug-regs", class_maintenance,
> +			   &show_debug_regs, _("\
> +Set whether to show variables that mirror the AArch64 debug registers."), _("\
> +Show whether to show variables that mirror the AArch64 debug registers."), _("\
> +Use \"on\" to enable, \"off\" to disable.\n\
> +If enabled, the debug registers values are shown when GDB inserts\n\
> +or removes a hardware breakpoint or watchpoint, and when the inferior\n\
> +triggers a breakpoint or watchpoint."),
> +			   NULL,
> +			   NULL,
> +			   &maintenance_set_cmdlist,
> +			   &maintenance_show_cmdlist);
> +}
> +
> +void
> +aarch64_initialize_hw_point ()
> +{
> +  add_show_debug_regs_command ();
> +}
> diff --git a/gdb/aarch64-nat.h b/gdb/aarch64-nat.h
> new file mode 100644
> index 00000000000..56e720f02ee
> --- /dev/null
> +++ b/gdb/aarch64-nat.h
> @@ -0,0 +1,109 @@
> +/* Native-dependent code for AArch64.
> +
> +   Copyright (C) 2011-2022 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef AARCH64_NAT_H
> +#define AARCH64_NAT_H
> +
> +#include "breakpoint.h"
> +#include "nat/aarch64-hw-point.h"
> +#include "target.h"
> +
> +/* Hardware-assisted breakpoints and watchpoints.  */
> +
> +/* Initialize platform-independent state for hardware-assisted
> +   breakpoints and watchpoints.  */
> +
> +void aarch64_initialize_hw_point ();
> +
> +/* Return the debug register state for process PID.  If no existing
> +   state is found for this process, return nullptr.  */
> +
> +struct aarch64_debug_reg_state *aarch64_lookup_debug_reg_state (pid_t pid);
> +
> +/* Return the debug register state for process PID.  If no existing
> +   state is found for this process, create new state.  */
> +
> +struct aarch64_debug_reg_state *aarch64_get_debug_reg_state (pid_t pid);
> +
> +/* Remove any existing per-process debug state for process PID.  */
> +
> +void aarch64_remove_debug_reg_state (pid_t pid);
> +
> +/* Helper for the "stopped_data_address" target method.  Returns TRUE
> +   if a hardware watchpoint trap at ADDR_TRAP matches a set
> +   watchpoint.  The address of the matched watchpoint is returned in
> +   *ADDR_P.  */
> +
> +bool aarch64_stopped_data_address (const struct aarch64_debug_reg_state *state,
> +				   CORE_ADDR addr_trap, CORE_ADDR *addr_p);
> +
> +/* Helper functions used by aarch64_nat_target below.  See their
> +   definitions.  */
> +
> +int aarch64_can_use_hw_breakpoint (enum bptype type, int cnt, int othertype);
> +int aarch64_insert_watchpoint (CORE_ADDR addr, int len,
> +			       enum target_hw_bp_type type,
> +			       struct expression *cond);
> +int aarch64_remove_watchpoint (CORE_ADDR addr, int len,
> +			       enum target_hw_bp_type type,
> +			       struct expression *cond);
> +int aarch64_insert_hw_breakpoint (struct gdbarch *gdbarch,
> +				  struct bp_target_info *bp_tgt);
> +int aarch64_remove_hw_breakpoint (struct gdbarch *gdbarch,
> +				  struct bp_target_info *bp_tgt);
> +int aarch64_stopped_by_hw_breakpoint ();
> +
> +/* Convenience template mixin used to add aarch64 watchpoints support to a
> +   target.  */
> +
> +template <typename BaseTarget>
> +struct aarch64_nat_target : public BaseTarget
> +{
> +  /* Hook in common aarch64 hardware watchpoints/breakpoints support.  */
> +
> +  int can_use_hw_breakpoint (enum bptype type, int cnt, int othertype) override
> +  { return aarch64_can_use_hw_breakpoint (type, cnt, othertype); }
> +
> +  int region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) override
> +  { return aarch64_region_ok_for_watchpoint (addr, len); }
> +
> +  int insert_watchpoint (CORE_ADDR addr, int len,
> +			 enum target_hw_bp_type type,
> +			 struct expression *cond) override
> +  { return aarch64_insert_watchpoint (addr, len, type, cond); }
> +
> +  int remove_watchpoint (CORE_ADDR addr, int len,
> +			 enum target_hw_bp_type type,
> +			 struct expression *cond) override
> +  { return aarch64_remove_watchpoint (addr, len, type, cond); }
> +
> +  int insert_hw_breakpoint (struct gdbarch *gdbarch,
> +			    struct bp_target_info *bp_tgt) override
> +  { return aarch64_insert_hw_breakpoint (gdbarch, bp_tgt); }
> +
> +  int remove_hw_breakpoint (struct gdbarch *gdbarch,
> +			    struct bp_target_info *bp_tgt) override
> +  { return aarch64_remove_hw_breakpoint (gdbarch, bp_tgt); }
> +
> +  bool watchpoint_addr_within_range (CORE_ADDR addr, CORE_ADDR start,
> +				     int length) override
> +  { return start <= addr && start + length - 1 >= addr; }
> +};
> +
> +#endif /* AARCH64_NAT_H */
> diff --git a/gdb/configure.nat b/gdb/configure.nat
> index ad6d35babc2..4f5850dd595 100644
> --- a/gdb/configure.nat
> +++ b/gdb/configure.nat
> @@ -233,7 +233,7 @@ case ${gdb_host} in
>   	case ${gdb_host_cpu} in
>   	    aarch64)
>   		#  Host: AArch64 based machine running GNU/Linux
> -		NATDEPFILES="${NATDEPFILES} aarch64-linux-nat.o \
> +		NATDEPFILES="${NATDEPFILES} aarch64-nat.o aarch64-linux-nat.o \
>   		aarch32-linux-nat.o nat/aarch64-hw-point.o \
>   		nat/aarch64-linux-hw-point.o \
>   		nat/aarch64-linux.o \