From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54813) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TgXru-0007Bu-A1 for qemu-devel@nongnu.org; Thu, 06 Dec 2012 04:35:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TgXrq-0006LR-SE for qemu-devel@nongnu.org; Thu, 06 Dec 2012 04:35:46 -0500 Received: from csmailer.cs.nctu.edu.tw ([140.113.235.130]:31611) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TgXrq-0006Jf-2e for qemu-devel@nongnu.org; Thu, 06 Dec 2012 04:35:42 -0500 Date: Thu, 6 Dec 2012 17:35:29 +0800 From: =?utf-8?B?6Zmz6Z+L5Lu7IChXZWktUmVuIENoZW4p?= Message-ID: <20121206093529.GA38209@cs.nctu.edu.tw> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1354786064.9165.2.camel@liguang.fnst.cn.fujitsu.com> 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, afaerber@suse.de On Thu, Dec 06, 2012 at 05:27:44PM +0800, li guang wrote: > =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 Ma= ydell=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= tss_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. > >=20 > > 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? I don't read this patch from the starting. But Peter, do you mean the return value matters here? I see the original version compares the return value with 0x1, do you mean we *need* this comparsion here? Regards, chenwj --=20 Wei-Ren Chen (=E9=99=B3=E9=9F=8B=E4=BB=BB) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj