Re: [PATCH 4/4] KVM: nVMX: Implement Enlightened MSR Bitmap feature

[Maxim Levitsky]

On Fri, 2021-09-10 at 18:06 +0200, Vitaly Kuznetsov wrote:
> Updating MSR bitmap for L2 is not cheap and rearly needed. TLFS for Hyper-V
> offers 'Enlightened MSR Bitmap' feature which allows L1 hypervisor to
> inform L0 when it changes MSR bitmap, this eliminates the need to examine
> L1's MSR bitmap for L2 every time when 'real' MSR bitmap for L2 gets
> constructed.
> 
> Use 'vmx->nested.msr_bitmap_changed' flag to implement the feature.
> 
> Note, KVM already uses 'Enlightened MSR bitmap' feature when it runs as a
> nested hypervisor on top of Hyper-V. The newly introduced feature is going
> to be used by Hyper-V guests on KVM.
> 
> When the feature is enabled for Win10+WSL2, it shaves off around 700 CPU
> cycles from a nested vmexit cost (tight cpuid loop test).
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> ---
>  arch/x86/kvm/hyperv.c     |  2 ++
>  arch/x86/kvm/vmx/nested.c | 19 +++++++++++++++++--
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 232a86a6faaf..7124dbe79ac2 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -2517,6 +2517,8 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>  
>  		case HYPERV_CPUID_NESTED_FEATURES:
>  			ent->eax = evmcs_ver;
> +			if (evmcs_ver)
> +				ent->eax |= HV_X64_NESTED_MSR_BITMAP;
>  
>  			break;
>  
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 42cd95611892..5ac5ba2f6191 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -607,15 +607,30 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
>  						 struct vmcs12 *vmcs12)
>  {
>  	int msr;
> +	struct vcpu_vmx *vmx = to_vmx(vcpu);
>  	unsigned long *msr_bitmap_l1;
> -	unsigned long *msr_bitmap_l0 = to_vmx(vcpu)->nested.vmcs02.msr_bitmap;
> -	struct kvm_host_map *map = &to_vmx(vcpu)->nested.msr_bitmap_map;
> +	unsigned long *msr_bitmap_l0 = vmx->nested.vmcs02.msr_bitmap;
> +	struct hv_enlightened_vmcs *evmcs = vmx->nested.hv_evmcs;
> +	struct kvm_host_map *map = &vmx->nested.msr_bitmap_map;
>  
>  	/* Nothing to do if the MSR bitmap is not in use.  */
>  	if (!cpu_has_vmx_msr_bitmap() ||
>  	    !nested_cpu_has(vmcs12, CPU_BASED_USE_MSR_BITMAPS))
>  		return false;
>  
> +	/*
> +	 * MSR bitmap update can be skipped when:
> +	 * - MSR bitmap for L1 hasn't changed.
> +	 * - Nested hypervisor (L1) is attempting to launch the same L2 as
> +	 *   before.
> +	 * - Nested hypervisor (L1) has enabled 'Enlightened MSR Bitmap' feature
> +	 *   and tells KVM (L0) there were no changes in MSR bitmap for L2.
> +	 */
> +	if (!vmx->nested.msr_bitmap_changed && evmcs &&
> +	    evmcs->hv_enlightenments_control.msr_bitmap &&
> +	    evmcs->hv_clean_fields & HV_VMX_ENLIGHTENED_CLEAN_FIELD_MSR_BITMAP)
> +		return true;
> +
>  	if (kvm_vcpu_map(vcpu, gpa_to_gfn(vmcs12->msr_bitmap), map))
>  		return false;
>  

I am not an expert on HyperV, but it looks OK to me.

Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>

Best regards,
	Maxim Levitsky