[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC v12 03/21] Remove an unnecessary assert from vmx_update_debug_state
>>> On 18.09.13 at 14:53, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote: > On 18/09/13 13:38, Jan Beulich wrote: >>>>> On 18.09.13 at 12:39, George Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote: >>> On Mon, Sep 16, 2013 at 10:09 PM, Mukesh Rathor >>> <mukesh.rathor@xxxxxxxxxx> wrote: >>>> On Fri, 13 Sep 2013 17:25:03 +0100 >>>> George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote: >>>> >>>>> As far as I can tell, there's nothing here that requires v to be >>>>> current. vmx_update_exception_bitmap() is updated in other situations >>>>> where v!=current. >>>>> >>>>> Removing this allows the PVH code to call this during vmcs >>>>> construction in a later patch, making the code more robust by removing >>>>> duplicate code. >>>>> >>>>> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx> >>>>> CC: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> >>>>> CC: Jan Beulich <jan.beulich@xxxxxxxx> >>>>> CC: Tim Deegan <tim@xxxxxxx> >>>>> CC: Keir Fraser <keir@xxxxxxx> >>>>> --- >>>>> xen/arch/x86/hvm/vmx/vmx.c | 2 -- >>>>> 1 file changed, 2 deletions(-) >>>>> >>>>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c >>>>> index 8ed7026..f02e47a 100644 >>>>> --- a/xen/arch/x86/hvm/vmx/vmx.c >>>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c >>>>> @@ -1041,8 +1041,6 @@ void vmx_update_debug_state(struct vcpu *v) >>>>> { >>>>> unsigned long mask; >>>>> >>>>> - ASSERT(v == current); >>>>> - >>>>> mask = 1u << TRAP_int3; >>>>> if ( !cpu_has_monitor_trap_flag ) >>>>> mask |= 1u << TRAP_debug; >>>> >>>> vmx_update_exception_bitmap() writes EXCEPTION_BITMAP of current vmcs, >>>> ie of current vcpu. If v != current, you'd be writing the bitmap of >>>> wrong vcpu. If the assertion is removed, then the function would need >>>> to vmx enter... >>> You mean, callers would need to call vmcs_enter before calling it. >>> Hmm... is there an assert for that? >> This is basically what the assertion you're removing does, albeit >> restricting it more than it needs to be. But - do you have a use >> case for the function when v != current? > > Yes -- in patch 8, I add a call to this from > xen/arch/x86/hvm/vmx/vmcs.c:construct_vmcs(). As the comment there > says, this update normally happens when the guest enables paging. But > since PVH guests start out with paging enabled, this never happens so we > do it at start-of-day. (That's what Mukesh's code did, but by > duplicating the code inside the function, so I followed suit, but called > the function itself instead.) At that point vmx_vmcs_enter() has > already been called, so the ASSERT is overly restrictive. Perhaps it's then better to indeed call vmx_vmcs_enter() here (as I think Mukesh also already suggested), which is mostly a no-op when called redundantly. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |