This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Enable hw watchpoint with longer ranges using DAWR on Power
- From: Luis Machado <lgustavo at codesourcery dot com>
- To: Edjunior Barbosa Machado <emachado at linux dot vnet dot ibm dot com>
- Cc: gdb-patches at sourceware dot org, Ulrich Weigand <Ulrich dot Weigand at de dot ibm dot com>
- Date: Tue, 02 Jul 2013 14:59:46 -0300
- Subject: Re: [PATCH] Enable hw watchpoint with longer ranges using DAWR on Power
- References: <1372786761-29726-1-git-send-email-emachado at linux dot vnet dot ibm dot com>
- Reply-to: lgustavo at codesourcery dot com
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