[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related f
From: |
li guang |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function |
Date: |
Fri, 07 Dec 2012 08:53:05 +0800 |
在 2012-12-06四的 09:48 +0000,Peter Maydell写道:
> On 6 December 2012 09:36, Andreas Färber <address@hidden> wrote:
> > Am 06.12.2012 10:27, schrieb li guang:
> >> 在 2012-12-06四的 09:23 +0000,Peter Maydell写道:
> >>> On 6 December 2012 09:16, li guang <address@hidden> wrote:
> >>>> 在 2012-12-06四的 08:54 +0000,Peter Maydell写道:
> >>>>> On 6 December 2012 03:03, liguang <address@hidden> wrote:
> >>>>>> Signed-off-by: liguang <address@hidden>
> >>>>>> --- 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 = 0; i < 4; i++) {
> >>>>>> - if (hw_breakpoint_enabled(env->dr[7], i) == 0x1) {
> >>>>>> + if (env->dr[7] & DR7_LOCAL_BP_MASK) {
> >>>>>> + for (i = 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.
> >>
> >> 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().
>
> The check you want is
> (hw_local_breakpoint_enabled() && !hw_global_breakpoint_enabled())
>
Yes, it's the right choice.
Thanks!
> if you're going to do it like that. [We don't want to take out the
> bp if it was enabled globally as well as locally.]
>
> -- PMM
>
--
regards!
li guang
- [Qemu-devel] [PATCH 2/3] target-i386:make hw_breakpoint_enabled return bool type, (continued)
- [Qemu-devel] [PATCH 2/3] target-i386:make hw_breakpoint_enabled return bool type, liguang, 2012/12/05
- [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function, liguang, 2012/12/05
- Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function, Peter Maydell, 2012/12/06
- Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function, li guang, 2012/12/06
- Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function, Peter Maydell, 2012/12/06
- Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function, li guang, 2012/12/06
- Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function, Wei-Ren Chen, 2012/12/06
- Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function, Andreas Färber, 2012/12/06
- Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function, Peter Maydell, 2012/12/06
- Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function, Andreas Färber, 2012/12/06
- Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function,
li guang <=
- Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function, Peter Maydell, 2012/12/06