From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2712 invoked by alias); 2 Jul 2013 17:59:57 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 2687 invoked by uid 89); 2 Jul 2013 17:59:53 -0000 X-Spam-SWARE-Status: No, score=-4.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL autolearn=ham version=3.3.1 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 02 Jul 2013 17:59:52 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1Uu4rm-0005xg-JS from Luis_Gustavo@mentor.com ; Tue, 02 Jul 2013 10:59:50 -0700 Received: from NA1-MAIL.mgc.mentorg.com ([147.34.98.181]) by svr-orw-fem-01.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Tue, 2 Jul 2013 10:59:49 -0700 Received: from [172.30.15.158] ([172.30.15.158]) by NA1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 2 Jul 2013 10:59:49 -0700 Message-ID: <51D31512.7020005@codesourcery.com> Date: Tue, 02 Jul 2013 17:59:00 -0000 From: Luis Machado Reply-To: lgustavo@codesourcery.com User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130623 Thunderbird/17.0.7 MIME-Version: 1.0 To: Edjunior Barbosa Machado CC: gdb-patches@sourceware.org, Ulrich Weigand Subject: Re: [PATCH] Enable hw watchpoint with longer ranges using DAWR on Power References: <1372786761-29726-1-git-send-email-emachado@linux.vnet.ibm.com> In-Reply-To: <1372786761-29726-1-git-send-email-emachado@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2013-07/txt/msg00087.txt.bz2 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 > > * 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