RE: [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing

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

 



> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@xxxxxxxxx] On Behalf Of Paolo Bonzini
> Sent: Friday, January 5, 2018 11:43 PM
> To: linux-kernel@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx
> Cc: Kang, Luwei <luwei.kang@xxxxxxxxx>
> Subject: [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing
> 
> KVM has a small optimization where it doesn't save/restore MSR_KERNEL_GS_BASE if the guest is in 32-bit mode.  However, this
> complicates the code noticeably by doubling the number of possible MSR bitmaps.  In addition, pt_disable_intercept_for_msr was
> only updating the "basic" MSR bitmap, because x2apic and x2apic_apicv are memcpy'd just once in hardware_setup.
> 
> Remove the long-mode bitmaps set, and touch all the three remaining bitmaps in vmx_disable_intercept_for_msr and
> vmx_enable_intercept_for_msr.
> 
> Fixes: 3bd1f85e893daec4f3982d1d45b6bfc0683442c4
> Cc: Luwei Kang <luwei.kang@xxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  arch/x86/kvm/vmx.c | 126 ++++++++++++++++++++---------------------------------
>  1 file changed, 48 insertions(+), 78 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 11321cc4129a..ea5b52dc2d78 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -935,12 +935,9 @@ static bool nested_vmx_is_page_fault_vmexit(struct vmcs12 *vmcs12,  static
> DEFINE_PER_CPU(spinlock_t, blocked_vcpu_on_cpu_lock);
> 
>  enum {
> -	VMX_MSR_BITMAP_LEGACY,
> -	VMX_MSR_BITMAP_LONGMODE,
> -	VMX_MSR_BITMAP_LEGACY_X2APIC_APICV,
> -	VMX_MSR_BITMAP_LONGMODE_X2APIC_APICV,
> -	VMX_MSR_BITMAP_LEGACY_X2APIC,
> -	VMX_MSR_BITMAP_LONGMODE_X2APIC,
> +	VMX_MSR_BITMAP_NOX2APIC,
> +	VMX_MSR_BITMAP_X2APIC_APICV,
> +	VMX_MSR_BITMAP_X2APIC,
>  	VMX_VMREAD_BITMAP,
>  	VMX_VMWRITE_BITMAP,
>  	VMX_BITMAP_NR
> @@ -948,12 +945,9 @@ enum {
> 
>  static unsigned long *vmx_bitmap[VMX_BITMAP_NR];
> 
> -#define vmx_msr_bitmap_legacy                (vmx_bitmap[VMX_MSR_BITMAP_LEGACY])
> -#define vmx_msr_bitmap_longmode              (vmx_bitmap[VMX_MSR_BITMAP_LONGMODE])
> -#define vmx_msr_bitmap_legacy_x2apic_apicv   (vmx_bitmap[VMX_MSR_BITMAP_LEGACY_X2APIC_APICV])
> -#define vmx_msr_bitmap_longmode_x2apic_apicv (vmx_bitmap[VMX_MSR_BITMAP_LONGMODE_X2APIC_APICV])
> -#define vmx_msr_bitmap_legacy_x2apic         (vmx_bitmap[VMX_MSR_BITMAP_LEGACY_X2APIC])
> -#define vmx_msr_bitmap_longmode_x2apic       (vmx_bitmap[VMX_MSR_BITMAP_LONGMODE_X2APIC])
> +#define vmx_msr_bitmap_nox2apic              (vmx_bitmap[VMX_MSR_BITMAP_NOX2APIC])
> +#define vmx_msr_bitmap_x2apic_apicv          (vmx_bitmap[VMX_MSR_BITMAP_X2APIC_APICV])
> +#define vmx_msr_bitmap_x2apic                (vmx_bitmap[VMX_MSR_BITMAP_X2APIC])
>  #define vmx_vmread_bitmap                    (vmx_bitmap[VMX_VMREAD_BITMAP])
>  #define vmx_vmwrite_bitmap                   (vmx_bitmap[VMX_VMWRITE_BITMAP])
> 
> @@ -2203,8 +2197,7 @@ static void vmx_save_host_state(struct kvm_vcpu *vcpu)
> 
>  #ifdef CONFIG_X86_64
>  	rdmsrl(MSR_KERNEL_GS_BASE, vmx->msr_host_kernel_gs_base);
> -	if (is_long_mode(&vmx->vcpu))
> -		wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
> +	wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
>  #endif
>  	if (boot_cpu_has(X86_FEATURE_MPX))
>  		rdmsrl(MSR_IA32_BNDCFGS, vmx->host_state.msr_host_bndcfgs);
> @@ -2222,8 +2215,7 @@ static void __vmx_load_host_state(struct vcpu_vmx *vmx)
>  	++vmx->vcpu.stat.host_state_reload;
>  	vmx->host_state.loaded = 0;
>  #ifdef CONFIG_X86_64
> -	if (is_long_mode(&vmx->vcpu))
> -		rdmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
> +	rdmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
>  #endif
>  	if (vmx->host_state.gs_ldt_reload_needed) {
>  		kvm_load_ldt(vmx->host_state.ldt_sel);
> @@ -2635,21 +2627,12 @@ static void vmx_set_msr_bitmap(struct kvm_vcpu *vcpu)
>  		 (vmcs_read32(SECONDARY_VM_EXEC_CONTROL) &
>  		  SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE)) {
>  		if (enable_apicv && kvm_vcpu_apicv_active(vcpu)) {
> -			if (is_long_mode(vcpu))
> -				msr_bitmap = vmx_msr_bitmap_longmode_x2apic_apicv;
> -			else
> -				msr_bitmap = vmx_msr_bitmap_legacy_x2apic_apicv;
> +			msr_bitmap = vmx_msr_bitmap_x2apic_apicv;
>  		} else {
> -			if (is_long_mode(vcpu))
> -				msr_bitmap = vmx_msr_bitmap_longmode_x2apic;
> -			else
> -				msr_bitmap = vmx_msr_bitmap_legacy_x2apic;
> +			msr_bitmap = vmx_msr_bitmap_x2apic;
>  		}
>  	} else {
> -		if (is_long_mode(vcpu))
> -			msr_bitmap = vmx_msr_bitmap_longmode;
> -		else
> -			msr_bitmap = vmx_msr_bitmap_legacy;
> +		msr_bitmap = vmx_msr_bitmap_nox2apic;
>  	}
> 
>  	vmcs_write64(MSR_BITMAP, __pa(msr_bitmap)); @@ -5211,22 +5194,24 @@ static void
> nested_vmx_disable_intercept_for_msr(unsigned long *msr_bitmap_l1,
>  	}
>  }
> 
> -static void vmx_disable_intercept_for_msr(u32 msr, bool longmode_only)
> +static void vmx_disable_intercept_for_msr(u32 msr)
>  {
> -	if (!longmode_only)
> -		__vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy,
> -						msr, MSR_TYPE_R | MSR_TYPE_W);
> -	__vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode,
> -						msr, MSR_TYPE_R | MSR_TYPE_W);
> +	__vmx_disable_intercept_for_msr(vmx_msr_bitmap_nox2apic,
> +					msr, MSR_TYPE_R | MSR_TYPE_W);
> +	__vmx_disable_intercept_for_msr(vmx_msr_bitmap_x2apic,
> +					msr, MSR_TYPE_R | MSR_TYPE_W);
> +	__vmx_disable_intercept_for_msr(vmx_msr_bitmap_x2apic_apicv,
> +					msr, MSR_TYPE_R | MSR_TYPE_W);
>  }
> 
> -static void vmx_enable_intercept_for_msr(u32 msr, bool longmode_only)
> +static void vmx_enable_intercept_for_msr(u32 msr)
>  {
> -	if (!longmode_only)
> -		__vmx_enable_intercept_for_msr(vmx_msr_bitmap_legacy,
> -						msr, MSR_TYPE_R | MSR_TYPE_W);
> -	__vmx_enable_intercept_for_msr(vmx_msr_bitmap_longmode,
> -						msr, MSR_TYPE_R | MSR_TYPE_W);
> +	__vmx_enable_intercept_for_msr(vmx_msr_bitmap_nox2apic,
> +				       msr, MSR_TYPE_R | MSR_TYPE_W);
> +	__vmx_enable_intercept_for_msr(vmx_msr_bitmap_x2apic,
> +				       msr, MSR_TYPE_R | MSR_TYPE_W);
> +	__vmx_enable_intercept_for_msr(vmx_msr_bitmap_x2apic_apicv,
> +				       msr, MSR_TYPE_R | MSR_TYPE_W);
>  }
> 
>  static void pt_disable_intercept_for_msr(bool flag) @@ -5235,36 +5220,29 @@ static void pt_disable_intercept_for_msr(bool
> flag)
>  	unsigned int addr_num = kvm_get_pt_addr_cnt();
> 
>  	if (flag) {
> -		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_STATUS, false);
> -		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_BASE, false);
> -		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_MASK, false);
> -		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_CR3_MATCH, false);
> +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_STATUS);
> +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_BASE);
> +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_MASK);
> +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_CR3_MATCH);
>  		for (i = 0; i < addr_num; i++)
> -			vmx_disable_intercept_for_msr(MSR_IA32_RTIT_ADDR0_A + i,
> -									false);
> +			vmx_disable_intercept_for_msr(MSR_IA32_RTIT_ADDR0_A + i);
>  	} else {
> -		vmx_enable_intercept_for_msr(MSR_IA32_RTIT_STATUS, false);
> -		vmx_enable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_BASE, false);
> -		vmx_enable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_MASK, false);
> -		vmx_enable_intercept_for_msr(MSR_IA32_RTIT_CR3_MATCH, false);
> +		vmx_enable_intercept_for_msr(MSR_IA32_RTIT_STATUS);
> +		vmx_enable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_BASE);
> +		vmx_enable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_MASK);
> +		vmx_enable_intercept_for_msr(MSR_IA32_RTIT_CR3_MATCH);
>  		for (i = 0; i < addr_num; i++)
> -			vmx_enable_intercept_for_msr(MSR_IA32_RTIT_ADDR0_A + i,
> -									false);
> +			vmx_enable_intercept_for_msr(MSR_IA32_RTIT_ADDR0_A + i);
>  	}
>  }
> 
>  static void vmx_disable_intercept_msr_x2apic(u32 msr, int type, bool apicv_only)  {
> -	__vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy_x2apic_apicv,
> +	__vmx_disable_intercept_for_msr(vmx_msr_bitmap_x2apic_apicv,
>  					msr, type);
> -	__vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode_x2apic_apicv,
> -					msr, type);
> -	if (!apicv_only) {
> -		__vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy_x2apic,
> -				msr, type);
> -		__vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode_x2apic,
> +	if (!apicv_only)
> +		__vmx_disable_intercept_for_msr(vmx_msr_bitmap_x2apic,
>  				msr, type);
> -	}
>  }
> 
>  static bool vmx_get_enable_apicv(struct kvm_vcpu *vcpu) @@ -5733,7 +5711,7 @@ static void vmx_vcpu_setup(struct
> vcpu_vmx *vmx)
>  		vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmwrite_bitmap));
>  	}
>  	if (cpu_has_vmx_msr_bitmap())
> -		vmcs_write64(MSR_BITMAP, __pa(vmx_msr_bitmap_legacy));
> +		vmcs_write64(MSR_BITMAP, __pa(vmx_msr_bitmap_nox2apic));
> 
>  	vmcs_write64(VMCS_LINK_POINTER, -1ull); /* 22.3.1.5 */
> 
> @@ -7047,8 +7025,9 @@ static __init int hardware_setup(void)
>  	memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
>  	memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
> 
> -	memset(vmx_msr_bitmap_legacy, 0xff, PAGE_SIZE);
> -	memset(vmx_msr_bitmap_longmode, 0xff, PAGE_SIZE);
> +	memset(vmx_msr_bitmap_nox2apic, 0xff, PAGE_SIZE);
> +	memset(vmx_msr_bitmap_x2apic, 0xff, PAGE_SIZE);
> +	memset(vmx_msr_bitmap_x2apic_apicv, 0xff, PAGE_SIZE);
> 
>  	if (setup_vmcs_config(&vmcs_config) < 0) {
>  		r = -EIO;
> @@ -7113,21 +7092,12 @@ static __init int hardware_setup(void)
>  		kvm_tsc_scaling_ratio_frac_bits = 48;
>  	}
> 
> -	vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
> -	vmx_disable_intercept_for_msr(MSR_GS_BASE, false);
> -	vmx_disable_intercept_for_msr(MSR_KERNEL_GS_BASE, true);
> -	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, false);
> -	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, false);
> -	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, false);
> -
> -	memcpy(vmx_msr_bitmap_legacy_x2apic_apicv,
> -			vmx_msr_bitmap_legacy, PAGE_SIZE);
> -	memcpy(vmx_msr_bitmap_longmode_x2apic_apicv,
> -			vmx_msr_bitmap_longmode, PAGE_SIZE);
> -	memcpy(vmx_msr_bitmap_legacy_x2apic,
> -			vmx_msr_bitmap_legacy, PAGE_SIZE);
> -	memcpy(vmx_msr_bitmap_longmode_x2apic,
> -			vmx_msr_bitmap_longmode, PAGE_SIZE);
> +	vmx_disable_intercept_for_msr(MSR_FS_BASE);
> +	vmx_disable_intercept_for_msr(MSR_GS_BASE);
> +	vmx_disable_intercept_for_msr(MSR_KERNEL_GS_BASE);
> +	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_CS);
> +	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP);
> +	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP);
> 
>  	set_bit(0, vmx_vpid_bitmap); /* 0 is reserved for host */
> 

Reviewed-by: Luwei Kang <luwei.kang@xxxxxxxxx>

Thanks,
Luwei Kang





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux