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

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

Jan Beulich wrote on 2013-12-18:
>>>> On 18.12.13 at 05:00, Yang Zhang <yang.z.zhang@xxxxxxxxx> wrote:
>> From: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>> 
>> With the feature of unrestricted guest, there should no vmexit be
>> triggered when guest accesses the cr3 in non-paging mode.
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>> ---
>> changes in v2:
>> Fix the guest boot failure on non-UG platform.
>> 
>> The previous patch doesn't consider the non-UG platform and will
>> cause guest boot failure on non-ug platform.
> 
> Which "previous patch"? This one being v2, yet there not being a
> v1 with the same title on the list back until the beginning of
> October, what you say here is rather confusing. Please be a little
> more considerate of other people's time and provide more precise
> information: If a patch needing fixing went in already, name its
> commit ID and title. If the patch needing fixing didn't go in yet, integrate 
> the fix with that patch (or series) and submit a new version.
> And if you post a v2 of a patch under a different title than the v1
> was posted, please provide enough information to be able to identify that 
> patch.

Don't know why you cannot find the first patch:
http://www.gossamer-threads.com/lists/xen/devel/302810


> 
> Jan
> 
> Jan
> 
>> During developing this patch, i was thinking whether the reading of
>> GUEST_CR3 is necessary on each vmexit. I thought the answer should
>> be no. But current implemention relies the {hw,guest}_cr3 in many
>> places and it hard to do a cleanup.(If anyone interesting, welcome
>> :))
>> 
>> BTW: this patch will improve performance, especially for context
>> switching intensive workload, we saw about 1% improvement. So please
>> backport it to 4.3.
>> ---
>>  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);
>> --
>> 1.7.1
> 
>


Best regards,
Yang



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