Re: [PATCH v2] KVM: VMX: Enable MSR-BASED TPR shadow even if w/o APICv

[Radim KrÄmÃÅ]

2016-09-20 08:35+0800, Wanpeng Li:
> From: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
> 
> I observed that kvmvapic(to optimize flexpriority=N or AMD) is used 
> to boost TPR access when testing kvm-unit-test/eventinj.flat tpr case
> on my haswell desktop (w/ flexpriority, w/o APICv). Commit (8d14695f9542 
> x86, apicv: add virtual x2apic support) disable virtual x2apic mode 
> completely if w/o APICv, and the author also told me that windows guest
> can't enter into x2apic mode when he developed the APICv feature several 
> years ago. However, it is not truth currently, Interrupt Remapping and 
> vIOMMU is added to qemu and the developers from Intel test windows 8 can 
> work in x2apic mode w/ Interrupt Remapping enabled recently. 
> 
> This patch enables TPR shadow for virtual x2apic mode to boost 
> windows guest in x2apic mode even if w/o APICv.
> 
> Can pass the kvm-unit-test.
> 
> Suggested-by: Wincy Van <fanwenyi0529@xxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
> Cc: Wincy Van <fanwenyi0529@xxxxxxxxx>
> Cc: Yang Zhang <yang.zhang.wz@xxxxxxxxx>
> Signed-off-by: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
> ---
> v1 -> v2: 
>  * leverage the cached msr bitmap

V2 has a similar bug as V1 ... we have to intercept most MSRs when the
APICv is disabled for selected VCPU, but globally enabled.

We need at least 2 sets of bitmaps for x2apic: one for
  "enable_apicv && kvm_vcpu_apicv_active()"

the other for
  "enable_apicv && !kvm_vcpu_apicv_active()"
  and "!enable_apicv"

and use the correct one in vmx_set_msr_bitmap().

>  arch/x86/kvm/vmx.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 5cede40..7d884e1 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6464,17 +6464,23 @@ static __init int hardware_setup(void)
>  
>  	set_bit(0, vmx_vpid_bitmap); /* 0 is reserved for host */
>  
> -	for (msr = 0x800; msr <= 0x8ff; msr++)
> -		vmx_disable_intercept_msr_read_x2apic(msr);
> -
> -	/* TMCCT */
> -	vmx_enable_intercept_msr_read_x2apic(0x839);
> -	/* TPR */
> -	vmx_disable_intercept_msr_write_x2apic(0x808);
> -	/* EOI */
> -	vmx_disable_intercept_msr_write_x2apic(0x80b);
> -	/* SELF-IPI */
> -	vmx_disable_intercept_msr_write_x2apic(0x83f);
> +	if (enable_apicv) {
> +		for (msr = 0x800; msr <= 0x8ff; msr++)
> +			vmx_disable_intercept_msr_read_x2apic(msr);
> +
> +		/* TMCCT */
> +		vmx_enable_intercept_msr_read_x2apic(0x839);
> +		/* TPR */
> +		vmx_disable_intercept_msr_write_x2apic(0x808);
> +		/* EOI */
> +		vmx_disable_intercept_msr_write_x2apic(0x80b);
> +		/* SELF-IPI */
> +		vmx_disable_intercept_msr_write_x2apic(0x83f);
> +	} else if (cpu_has_vmx_tpr_shadow()) {
> +		/* TPR */
> +		vmx_disable_intercept_msr_read_x2apic(0x808);
> +		vmx_disable_intercept_msr_write_x2apic(0x808);
> +	}
>  
>  	if (enable_ept) {
>  		kvm_mmu_set_mask_ptes(VMX_EPT_READABLE_MASK,
> @@ -8435,12 +8441,7 @@ static void vmx_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
>  		return;
>  	}
>  
> -	/*
> -	 * There is not point to enable virtualize x2apic without enable
> -	 * apicv
> -	 */
> -	if (!cpu_has_vmx_virtualize_x2apic_mode() ||
> -				!kvm_vcpu_apicv_active(vcpu))
> +	if (!cpu_has_vmx_virtualize_x2apic_mode())
>  		return;
>  
>  	if (!cpu_need_tpr_shadow(vcpu))
> -- 
> 1.9.1
>