From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52423) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TgvC6-0002Xx-JR for qemu-devel@nongnu.org; Fri, 07 Dec 2012 05:30:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TgvBy-00021d-Gg for qemu-devel@nongnu.org; Fri, 07 Dec 2012 05:30:09 -0500 Received: from thoth.sbs.de ([192.35.17.2]:26962) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TgvBy-0001yy-7C for qemu-devel@nongnu.org; Fri, 07 Dec 2012 05:30:02 -0500 Message-ID: <50C1C51F.80807@siemens.com> Date: Fri, 07 Dec 2012 11:29:51 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <1354843535-10912-1-git-send-email-lig.fnst@cn.fujitsu.com> <1354843535-10912-2-git-send-email-lig.fnst@cn.fujitsu.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/3] target-i386:make hw_breakpoint_enabled return bool type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: "ehabkost@redhat.com" , "qemu-devel@nongnu.org" , "blauwirbel@gmail.com" , "imammedo@redhat.com" , "afaerber@suse.de" , liguang On 2012-12-07 11:24, Peter Maydell wrote: > On 7 December 2012 01:25, liguang wrote: >> Signed-off-by: liguang >> --- >> target-i386/cpu.h | 15 +++++++++++++-- >> 1 files changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h >> index 29245d1..3646128 100644 >> --- a/target-i386/cpu.h >> +++ b/target-i386/cpu.h >> @@ -996,9 +996,20 @@ int cpu_x86_handle_mmu_fault(CPUX86State *env, target_ulong addr, >> #define cpu_handle_mmu_fault cpu_x86_handle_mmu_fault >> void cpu_x86_set_a20(CPUX86State *env, int a20_state); >> >> -static inline int hw_breakpoint_enabled(unsigned long dr7, int index) >> +static inline bool hw_local_breakpoint_enabled(unsigned long dr7, int index) >> { >> - return (dr7 >> (index * 2)) & 3; >> + return !(((dr7 >> (index * 2)) ^ 1) & 3); > > This is pretty confusing and I'm pretty sure the function is > misnamed too. If we're checking "is local breakpoint enabled" > then we only want to check one of the two enable bits, not both. > Yes, and I already asked to define the proper constants that allow checking for local vs. global enable bit. They have to be used here instead of all the magic & 3 or ^ 1 stuff. BTW, there is no need for "converting" ("!!") the result of the (value & mask) to bool, the compiler will do this already. Jan > >> +} >> + >> +static inline bool hw_global_breakpoint_enabled(unsigned long dr7, int index) >> +{ >> + return !!((dr7 >> (index * 2)) & 2); >> +} >> + >> +static inline bool hw_breakpoint_enabled(unsigned long dr7, int index) >> +{ >> + return (hw_global_breakpoint_enabled(dr7, index) || >> + hw_local_breakpoint_enabled(dr7, index)); >> } > > -- PMM > -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux