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

Re: [Xen-devel] [PATCH 4/7] vm-event/x86: use vm_event_vcpu_enter properly



>>> On 16.06.16 at 22:20, <czuzu@xxxxxxxxxxxxxxx> wrote:
> On 6/16/2016 6:00 PM, Jan Beulich wrote:
>>>>> On 16.06.16 at 16:09, <czuzu@xxxxxxxxxxxxxxx> wrote:
>>> @@ -1432,18 +1430,16 @@ static void vmx_update_guest_cr(struct vcpu *v, 
>>> unsigned int cr)
>>>           if ( paging_mode_hap(v->domain) )
>>>           {
>>>               /* Manage GUEST_CR3 when CR0.PE=0. */
>>> +            uint32_t old_ctls = v->arch.hvm_vmx.exec_control;
>>>               uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
>>>                                    CPU_BASED_CR3_STORE_EXITING);
>>> +
>>>               v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
>>>               if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
>>>                   v->arch.hvm_vmx.exec_control |= cr3_ctls;
>>>   
>>> -            /* Trap CR3 updates if CR3 memory events are enabled. */
>>> -            if ( v->domain->arch.monitor.write_ctrlreg_enabled &
>>> -                 monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
>>> -                v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING;
>>> -
>>> -            vmx_update_cpu_exec_control(v);
>>> +            if ( old_ctls != v->arch.hvm_vmx.exec_control )
>>> +                vmx_update_cpu_exec_control(v);
>>>           }
>> How does this match up with the rest of this patch?
> 
> And by 'this' you mean slightly optimizing this sequence by adding in 
> old_ctls?
> It seems pretty straight-forward to me, I figured if I am to move the 
> monitor.write_ctrlreg_enabled part from here
> it wouldn't be much of a stretch to also do this little 
> optimization...what would have been appropriate?
> To do this in a separate patch? To mention it in the commit message?

At least the latter, and perhaps better the former. Without even
mentioning it the readers (reviewers) have to guess whether this
is an integral part of the change, or - as you now confirm - just a
minor optimization done along the road.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.