qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/3] target-i386:make hw_breakpoint_enabled retu


From: li guang
Subject: Re: [Qemu-devel] [PATCH 2/3] target-i386:make hw_breakpoint_enabled return bool type
Date: Thu, 06 Dec 2012 10:08:27 +0800

在 2012-12-05三的 09:53 +0100,Jan Kiszka写道:
> On 2012-12-05 01:51, li guang wrote:
> > 在 2012-12-04二的 11:26 +0000,Peter Maydell写道:
> >> On 4 December 2012 11:11, Jan Kiszka <address@hidden> wrote:
> >>> On 2012-12-04 11:23, Peter Maydell wrote:
> >>>> Doesn't this break the use of this function in target-i386/seg_helper.c:
> >>>>
> >>>>   if (hw_breakpoint_enabled(env->dr[7], i) == 0x1) {
> >>>>
> >>>> which specifically wants to determine whether the breakpoint is
> >>>> enabled only locally?
> > 
> >  It was changed to 'if (hw_breakpoint_enabled(env->dr[7], i)) {'
> >  in patch 3/3
> 
> Which is broken as it neglects the different types of "enabled".
> 
> > 
> >>>
> >>> It does. And that also indicates the function is misnamed. Something
> >>> like hw_breakpoint_state might be better.
> >>
> > 
> > misnamed? I think hw_breakpoint_enabled is ask whether breakpoint
> >                                  ^^^^^^^^
> > is enabled or not, so it's almost suitable.
> 
> There are two types of enabled breakpoints: task-local and global. The
> current hw_breakpoint_enabled returns both as a bitmask, and that is
> causing the confusing and regression in your patches.
> 

It is no doubt that 'hw_breakpoint_enabled' is only check local
breakpoint, you know, so, do we really have to handle all the cases
since seems there's nowhere uses global breapoints at present?

> Jan
> 

-- 
regards!
li guang




reply via email to

[Prev in Thread] Current Thread [Next in Thread]