[PATCH] Enable hw watchpoint with longer ranges using DAWR on Power

[PATCH] Enable hw watchpoint with longer ranges using DAWR on Power

[Edjunior Barbosa Machado]

Hi,

The new DAWR interface provided by the next generation of Power processors
(Power ISA Version 2.07) included in the kernel this year allows gdb to use
hardware watchpoint with longer ranges (up to 512 bytes wide), which can't cross
a 512 byte boundary [1]. The patch below enables the usage of this new feature,
currently exported to the userspace as PPC_DEBUG_FEATURE_DATA_BP_DAWR via struct
ppc_debug_info [2]. Ok?

Thanks and regards,
--
Edjunior

[1] http://patchwork.ozlabs.org/patch/215520/
[2] http://patchwork.ozlabs.org/patch/229888/

gdb/ChangeLog
2013-07-02  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>

	* ppc-linux-nat.c (PPC_DEBUG_FEATURE_DATA_BP_DAWR): New define.
	(ppc_linux_region_ok_for_hw_watchpoint): Add checking to use the new
	DAWR interface for longer ranges hardware watchpoint (up to 512 bytes).

---
 gdb/ppc-linux-nat.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 65d4f4a..dd35624 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -177,7 +177,11 @@ struct ppc_hw_breakpoint
         (1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
 #endif /* PPC_PTRACE_GETHWDBGINFO */
 
-
+/* New feature defined on Linux kernel v3.9: DAWR interface, that enables
+   wider watchpoint (up to 512 bytes).  */
+#ifndef PPC_DEBUG_FEATURE_DATA_BP_DAWR
+#define PPC_DEBUG_FEATURE_DATA_BP_DAWR	0x10
+#endif /* PPC_DEBUG_FEATURE_DATA_BP_DAWR */
 
 /* Similarly for the general-purpose (gp0 -- gp31)
    and floating-point registers (fp0 -- fp31).  */
@@ -1502,6 +1506,7 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
      to determine the hardcoded watchable region for watchpoints.  */
   if (have_ptrace_booke_interface ())
     {
+      int region_size;
       /* Embedded DAC-based processors, like the PowerPC 440 have ranged
 	 watchpoints and can watch any access within an arbitrary memory
 	 region. This is useful to watch arrays and structs, for instance.  It
@@ -1510,11 +1515,17 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
 	  && booke_debug_info.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE
 	  && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
 	return 2;
+      /* Check if the processor provides DAWR interface.  */
+      if (booke_debug_info.features & PPC_DEBUG_FEATURE_DATA_BP_DAWR)
+	/* DAWR interface allows to watch up to 512 byte wide ranges which
+ 	   can't cross a 512 byte boundary.  */
+        region_size = 512;
+      else
+	region_size = booke_debug_info.data_bp_alignment;
       /* Server processors provide one hardware watchpoint and addr+len should
          fall in the watchable region provided by the ptrace interface.  */
-      if (booke_debug_info.data_bp_alignment
-	  && (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1))
-	      + booke_debug_info.data_bp_alignment))
+      if (region_size
+	  && (addr + len > (addr & ~(region_size - 1)) + region_size))
 	return 0;
     }
   /* addr+len must fall in the 8 byte watchable region for DABR-based
-- 
1.8.2.1

Re: [PATCH] Enable hw watchpoint with longer ranges using DAWR on Power

[Luis Machado]

Hi,

The patch looks sane enough. A few nits.

On 07/02/2013 02:39 PM, Edjunior Barbosa Machado wrote:
> Hi,
>
> The new DAWR interface provided by the next generation of Power processors
> (Power ISA Version 2.07) included in the kernel this year allows gdb to use
> hardware watchpoint with longer ranges (up to 512 bytes wide), which can't cross
> a 512 byte boundary [1]. The patch below enables the usage of this new feature,
> currently exported to the userspace as PPC_DEBUG_FEATURE_DATA_BP_DAWR via struct
> ppc_debug_info [2]. Ok?
>
> Thanks and regards,
> --
> Edjunior
>
> [1] http://patchwork.ozlabs.org/patch/215520/
> [2] http://patchwork.ozlabs.org/patch/229888/
>
> gdb/ChangeLog
> 2013-07-02  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>
>
> 	* ppc-linux-nat.c (PPC_DEBUG_FEATURE_DATA_BP_DAWR): New define.
> 	(ppc_linux_region_ok_for_hw_watchpoint): Add checking to use the new
> 	DAWR interface for longer ranges hardware watchpoint (up to 512 bytes).
>
> ---
>   gdb/ppc-linux-nat.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
> index 65d4f4a..dd35624 100644
> --- a/gdb/ppc-linux-nat.c
> +++ b/gdb/ppc-linux-nat.c
> @@ -177,7 +177,11 @@ struct ppc_hw_breakpoint
>           (1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
>   #endif /* PPC_PTRACE_GETHWDBGINFO */
>
> -
> +/* New feature defined on Linux kernel v3.9: DAWR interface, that enables
> +   wider watchpoint (up to 512 bytes).  */

"New feature" will get old in the future. I'd just mention the feature 
itself without assigning any notion of how recent/old it is.

> +#ifndef PPC_DEBUG_FEATURE_DATA_BP_DAWR
> +#define PPC_DEBUG_FEATURE_DATA_BP_DAWR	0x10
> +#endif /* PPC_DEBUG_FEATURE_DATA_BP_DAWR */
>

Are older kernels/libraries that don't define these constants going to 
be used with these POWER processors in the future? If not, maybe they 
can be dropped instead of forcing their definitions here.

>   /* Similarly for the general-purpose (gp0 -- gp31)
>      and floating-point registers (fp0 -- fp31).  */
> @@ -1502,6 +1506,7 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
>        to determine the hardcoded watchable region for watchpoints.  */
>     if (have_ptrace_booke_interface ())
>       {
> +      int region_size;
>         /* Embedded DAC-based processors, like the PowerPC 440 have ranged
>   	 watchpoints and can watch any access within an arbitrary memory
>   	 region. This is useful to watch arrays and structs, for instance.  It
> @@ -1510,11 +1515,17 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
>   	  && booke_debug_info.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE
>   	  && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
>   	return 2;
> +      /* Check if the processor provides DAWR interface.  */
> +      if (booke_debug_info.features & PPC_DEBUG_FEATURE_DATA_BP_DAWR)
> +	/* DAWR interface allows to watch up to 512 byte wide ranges which
> + 	   can't cross a 512 byte boundary.  */
> +        region_size = 512;
> +      else
> +	region_size = booke_debug_info.data_bp_alignment;
>         /* Server processors provide one hardware watchpoint and addr+len should
>            fall in the watchable region provided by the ptrace interface.  */
> -      if (booke_debug_info.data_bp_alignment
> -	  && (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1))
> -	      + booke_debug_info.data_bp_alignment))
> +      if (region_size
> +	  && (addr + len > (addr & ~(region_size - 1)) + region_size))
>   	return 0;
>       }
>     /* addr+len must fall in the 8 byte watchable region for DABR-based
>

We should really really get this booke thing sorted out. :-)

Otherwise we will be calling everything "booke" and that may lead to 
confusion in the future.

Thanks for the patch.

Luis

Re: [PATCH] Enable hw watchpoint with longer ranges using DAWR on Power

[Ulrich Weigand]

Luis Machado wrote:
> On 07/02/2013 02:39 PM, Edjunior Barbosa Machado wrote:
> > gdb/ChangeLog
> > 2013-07-02  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>
> >
> > 	* ppc-linux-nat.c (PPC_DEBUG_FEATURE_DATA_BP_DAWR): New define.
> > 	(ppc_linux_region_ok_for_hw_watchpoint): Add checking to use the new
> > 	DAWR interface for longer ranges hardware watchpoint (up to 512 bytes).

This looks good to me too, but we should indeed probably sort out
the booke renaming first :-)

> > +#ifndef PPC_DEBUG_FEATURE_DATA_BP_DAWR
> > +#define PPC_DEBUG_FEATURE_DATA_BP_DAWR	0x10
> > +#endif /* PPC_DEBUG_FEATURE_DATA_BP_DAWR */
> >
> 
> Are older kernels/libraries that don't define these constants going to 
> be used with these POWER processors in the future? If not, maybe they 
> can be dropped instead of forcing their definitions here.

In general, we usually always provide such defines.  There should be
no harm in doing so, since this is kernel ABI that will never change,
so it is fine to just use the well-known binary value ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Enable hw watchpoint with longer ranges using DAWR on Power

[Eli Zaretskii]

> From: Edjunior Barbosa Machado <emachado@linux.vnet.ibm.com>
> Cc: Edjunior Barbosa Machado <emachado@linux.vnet.ibm.com>,        Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
> Date: Tue,  2 Jul 2013 14:39:21 -0300
> 
> The new DAWR interface provided by the next generation of Power processors
> (Power ISA Version 2.07) included in the kernel this year allows gdb to use
> hardware watchpoint with longer ranges (up to 512 bytes wide), which can't cross
> a 512 byte boundary [1]. The patch below enables the usage of this new feature,
> currently exported to the userspace as PPC_DEBUG_FEATURE_DATA_BP_DAWR via struct
> ppc_debug_info [2]. Ok?

Is only one watchpoint of this kind possible at any given time?  If
more than one such watchpoint can be used at the same time, I don't
see why we should expose the 512-byte limitation to users.  Just use
more than one such watchpoint to cover the range of addresses
requested by the user command.

Re: [PATCH] Enable hw watchpoint with longer ranges using DAWR on Power

[Edjunior Barbosa Machado]

On 07/02/2013 05:03 PM, Eli Zaretskii wrote:
> Is only one watchpoint of this kind possible at any given time?  If
> more than one such watchpoint can be used at the same time, I don't
> see why we should expose the 512-byte limitation to users.  Just use
> more than one such watchpoint to cover the range of addresses
> requested by the user command.

Yes, currently there is only one hw watchpoint available on Power servers. The
new feature just allows to watch longer ranges using this hw watchpoint, up to
512 bytes, instead of previous 8 bytes limit.

I'm resending the patch fixing the comment as Luis suggested. And I also hope
we can get the 'booke' renaming upstream soon :)

Thanks for the feedback.
--
Edjunior

gdb/ChangeLog
2013-07-02  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>

	* ppc-linux-nat.c (PPC_DEBUG_FEATURE_DATA_BP_DAWR): New define.
	(ppc_linux_region_ok_for_hw_watchpoint): Add checking to use the new
	DAWR interface for longer ranges hardware watchpoint (up to 512 bytes).

---
 gdb/ppc-linux-nat.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 65d4f4a..ce437dd 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -177,7 +177,11 @@ struct ppc_hw_breakpoint
         (1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
 #endif /* PPC_PTRACE_GETHWDBGINFO */
 
-
+/* Feature defined on Linux kernel v3.9: DAWR interface, that enables wider
+   watchpoint (up to 512 bytes).  */
+#ifndef PPC_DEBUG_FEATURE_DATA_BP_DAWR
+#define PPC_DEBUG_FEATURE_DATA_BP_DAWR	0x10
+#endif /* PPC_DEBUG_FEATURE_DATA_BP_DAWR */
 
 /* Similarly for the general-purpose (gp0 -- gp31)
    and floating-point registers (fp0 -- fp31).  */
@@ -1502,6 +1506,7 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
      to determine the hardcoded watchable region for watchpoints.  */
   if (have_ptrace_booke_interface ())
     {
+      int region_size;
       /* Embedded DAC-based processors, like the PowerPC 440 have ranged
 	 watchpoints and can watch any access within an arbitrary memory
 	 region. This is useful to watch arrays and structs, for instance.  It
@@ -1510,11 +1515,17 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
 	  && booke_debug_info.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE
 	  && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
 	return 2;
+      /* Check if the processor provides DAWR interface.  */
+      if (booke_debug_info.features & PPC_DEBUG_FEATURE_DATA_BP_DAWR)
+	/* DAWR interface allows to watch up to 512 byte wide ranges which
+ 	   can't cross a 512 byte boundary.  */
+        region_size = 512;
+      else
+	region_size = booke_debug_info.data_bp_alignment;
       /* Server processors provide one hardware watchpoint and addr+len should
          fall in the watchable region provided by the ptrace interface.  */
-      if (booke_debug_info.data_bp_alignment
-	  && (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1))
-	      + booke_debug_info.data_bp_alignment))
+      if (region_size
+	  && (addr + len > (addr & ~(region_size - 1)) + region_size))
 	return 0;
     }
   /* addr+len must fall in the 8 byte watchable region for DABR-based
-- 
1.8.2.1

Re: [PATCH] Enable hw watchpoint with longer ranges using DAWR on Power

[Edjunior Barbosa Machado]

Resending the patch rebased on current CVS head, with the latest updates on
gdb/ppc-linux-nat.c (thanks Ulrich for committing the patch renaming the
'booke' interface!).

Ok to commit?

Thanks,
--
Edjunior

gdb/ChangeLog
2013-07-16  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>

	* ppc-linux-nat.c (PPC_DEBUG_FEATURE_DATA_BP_DAWR): New define.
	(ppc_linux_region_ok_for_hw_watchpoint): Add checking to use the new
	DAWR interface for longer ranges hardware watchpoint (up to 512 bytes).

---
 gdb/ppc-linux-nat.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 4b3c78c..1f3f080 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -177,7 +177,11 @@ struct ppc_hw_breakpoint
         (1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
 #endif /* PPC_PTRACE_GETHWDBGINFO */
 
-
+/* Feature defined on Linux kernel v3.9: DAWR interface, that enables wider
+   watchpoint (up to 512 bytes).  */
+#ifndef PPC_DEBUG_FEATURE_DATA_BP_DAWR
+#define PPC_DEBUG_FEATURE_DATA_BP_DAWR	0x10
+#endif /* PPC_DEBUG_FEATURE_DATA_BP_DAWR */
 
 /* Similarly for the general-purpose (gp0 -- gp31)
    and floating-point registers (fp0 -- fp31).  */
@@ -1504,6 +1508,7 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
      watchpoints.  */
   if (have_ptrace_hwdebug_interface ())
     {
+      int region_size;
       /* Embedded DAC-based processors, like the PowerPC 440 have ranged
 	 watchpoints and can watch any access within an arbitrary memory
 	 region. This is useful to watch arrays and structs, for instance.  It
@@ -1512,11 +1517,17 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
 	  && hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE
 	  && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
 	return 2;
+      /* Check if the processor provides DAWR interface.  */
+      if (hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_DAWR)
+	/* DAWR interface allows to watch up to 512 byte wide ranges which
+	   can't cross a 512 byte boundary.  */
+	region_size = 512;
+      else
+	region_size = hwdebug_info.data_bp_alignment;
       /* Server processors provide one hardware watchpoint and addr+len should
          fall in the watchable region provided by the ptrace interface.  */
-      if (hwdebug_info.data_bp_alignment
-	  && (addr + len > (addr & ~(hwdebug_info.data_bp_alignment - 1))
-	      + hwdebug_info.data_bp_alignment))
+      if (region_size
+	  && (addr + len > (addr & ~(region_size - 1)) + region_size))
 	return 0;
     }
   /* addr+len must fall in the 8 byte watchable region for DABR-based
-- 
1.8.2.1

Re: [PATCH] Enable hw watchpoint with longer ranges using DAWR on Power

[Ulrich Weigand]

Edjunior Barbosa Machado wrote:

> 	* ppc-linux-nat.c (PPC_DEBUG_FEATURE_DATA_BP_DAWR): New define.
> 	(ppc_linux_region_ok_for_hw_watchpoint): Add checking to use the new
> 	DAWR interface for longer ranges hardware watchpoint (up to 512 bytes).

This is OK.  I've checked the patch in now.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com