Re: [Xen-devel] [PATCH v3] VMX: Eliminate cr3 save/loading exiting when UG enabled

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Andrew Cooper wrote on 2014-01-08:
> On 08/01/2014 01:13, Zhang, Yang Z wrote:
>> Zhang, Yang Z wrote on 2013-12-19:
>>> From: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>>> 
>>> With the feature of unrestricted guest, there should be no vmexit
>>> be triggered when guest accesses the cr3 in non-paging mode. This
>>> patch will clear the cr3 save/loading bit in vmcs control filed to
>>> eliminate
>>> cr3 access vmexit on UG avaliable hardware.
>>> 
>>> The previous patch (commit c9efe34c119418a5ac776e5d91aeefcce4576518)
>>> did the same thing compare to this one. But it will cause guest fail
>>> to boot up on non-UG hardware which is repoted by Jan and it has been
>>> reverted (commit 1e2bf05ec37cf04b0e01585eae524509179f165e).
>>> 
>> Hi, Jun.
>> 
>> Can you help to review this patch?
> 
> This got committed earlier today
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=f4fed540e78ac8
> a2b d3b1dee53a5206dde25f613) as you are a maintainer, and I (as an
> independent party as far as the patch goes) reviewed it.

I remember in old day, after a patch got committed, the maintainer will reply 
the mail to tell the author it is applied. Or else, it's hard for author to 
know it in time. Should we still follow this rule?

BTW: KVM always follow this rule.

> 
>> 
>>> This patch incorporate the fixing and guest are working well both
>>> in UG and non-UG platform with this patch.
>>> 
>>> Reported-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>>> ---
>>> changes in v3:
>>> Revise the patch description according Jan's suggestion
>>> 
>>> changes in v2:
>>> Fix the guest boot failure on non-UG platform.
>>> 
>>> There are some discussions around the first patch, please see the
>>> following link:
>>> http://www.gossamer-threads.com/lists/xen/devel/302810
>>> 
>>> ---
>>>  xen/arch/x86/hvm/vmx/vmx.c |    9 +++++----
>>>  1 files changed, 5 insertions(+), 4 deletions(-) diff --git
>>> a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index
>>> dfff628..f6409d6 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++
>>> b/xen/arch/x86/hvm/vmx/vmx.c @@ -1157,7 +1157,7 @@ static void
>>> vmx_update_guest_cr(struct vcpu *v, unsigned int cr)
>>>              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) )
>>> +            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.
>>> */ @@ -1231,7 +1231,7 @@ static void vmx_update_guest_cr(struct
>>> vcpu *v, unsigned int cr)
>>>      case 3:
>>>          if ( paging_mode_hap(v->domain) )
>>>          {
>>> -            if ( !hvm_paging_enabled(v) )
>>> +            if ( !hvm_paging_enabled(v) &&
>>> + !vmx_unrestricted_guest(v)
>>> + )
>>>                  v->arch.hvm_vcpu.hw_cr[3] =
>>> v->domain->arch.hvm_domain.params[HVM_PARAM_IDENT_PT];
>>>              vmx_load_pdptrs(v);
>>> @@ -2487,10 +2487,11 @@ void vmx_vmexit_handler(struct cpu_user_regs
>>> *regs)
>>> 
>>>      hvm_invalidate_regs_fields(regs);
>>> -    if ( paging_mode_hap(v->domain) && hvm_paging_enabled(v) )
>>> +    if ( paging_mode_hap(v->domain) )
>>>      {
>>>          __vmread(GUEST_CR3, &v->arch.hvm_vcpu.hw_cr[3]);
>>> -        v->arch.hvm_vcpu.guest_cr[3] = v->arch.hvm_vcpu.hw_cr[3];
>>> +        if ( vmx_unrestricted_guest(v) || hvm_paging_enabled(v) )
>>> +            v->arch.hvm_vcpu.guest_cr[3] =
>>> + v->arch.hvm_vcpu.hw_cr[3];
>>>      }
>>>      
>>>      __vmread(VM_EXIT_REASON, &exit_reason);
>> 
>> Best regards,
>> Yang
>> 
>>


Best regards,
Yang



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