From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54888) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TgXsS-00080q-KK for qemu-devel@nongnu.org; Thu, 06 Dec 2012 04:36:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TgXsJ-0006Ou-0T for qemu-devel@nongnu.org; Thu, 06 Dec 2012 04:36:20 -0500 Received: from cantor2.suse.de ([195.135.220.15]:49038 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TgXsI-0006Op-Na for qemu-devel@nongnu.org; Thu, 06 Dec 2012 04:36:10 -0500 Message-ID: <50C06706.2010808@suse.de> Date: Thu, 06 Dec 2012 10:36:06 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1354762999-4135-1-git-send-email-lig.fnst@cn.fujitsu.com> <1354762999-4135-3-git-send-email-lig.fnst@cn.fujitsu.com> <1354785393.9018.3.camel@liguang.fnst.cn.fujitsu.com> <1354786064.9165.2.camel@liguang.fnst.cn.fujitsu.com> In-Reply-To: <1354786064.9165.2.camel@liguang.fnst.cn.fujitsu.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: li guang Cc: Peter Maydell , ehabkost@redhat.com, jan.kiszka@siemens.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, imammedo@redhat.com Am 06.12.2012 10:27, schrieb li guang: > =E5=9C=A8 2012-12-06=E5=9B=9B=E7=9A=84 09:23 +0000=EF=BC=8CPeter Maydel= l=E5=86=99=E9=81=93=EF=BC=9A >> On 6 December 2012 09:16, li guang wrote: >>> =E5=9C=A8 2012-12-06=E5=9B=9B=E7=9A=84 08:54 +0000=EF=BC=8CPeter Mayd= ell=E5=86=99=E9=81=93=EF=BC=9A >>>> On 6 December 2012 03:03, liguang wrote: >>>>> Signed-off-by: liguang >>>>> --- a/target-i386/seg_helper.c >>>>> +++ b/target-i386/seg_helper.c >>>>> @@ -465,9 +465,9 @@ static void switch_tss(CPUX86State *env, int ts= s_selector, >>>>> >>>>> #ifndef CONFIG_USER_ONLY >>>>> /* reset local breakpoints */ >>>>> - if (env->dr[7] & 0x55) { >>>>> - for (i =3D 0; i < 4; i++) { >>>>> - if (hw_breakpoint_enabled(env->dr[7], i) =3D=3D 0x1) { >>>>> + if (env->dr[7] & DR7_LOCAL_BP_MASK) { >>>>> + for (i =3D 0; i < DR7_MAX_BP; i++) { >>>>> + if (hw_breakpoint_enabled(env->dr[7], i)) { >>>>> hw_breakpoint_remove(env, i); >>>>> } >>>>> } >>>> >>>> This is still wrong. >>> >>> do you mean the use of 'hw_breakpoint_enabled'? or others? >>> maybe a mistake, I change it to 'hw_local_breakpoint_enabled'. >>> if it is I'll re-send a corrected patch. >> >> I mean that in the comments on the previous version of this >> patchseet we explained that this check is specifically checking >> for whether the breakpoint is enabled locally, and that your >> change to just returning bool broke this. And in this version >> of the patch there is still exactly the same problem. >=20 > why broke? > this function just ask if breakpoint 'i' was enable, > so we answer enabled or not? 2 simple cases, any problem? The code comment specifically says "reset local breakpoints". IIUC you are also resetting global breakpoints, which you shouldn't. Personally I'd be fine with a hw_local_breakpoint_enabled(). Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg