Re: [PATCH] KVM: VMX: Enable MSR-BASED TPR shadow even if w/o APICv
From: Paolo Bonzini
Date: Wed Sep 14 2016 - 05:41:00 EST
On 14/09/2016 09:58, Wanpeng Li wrote:
> 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.
Ok, now I see what you meant; this actually makes sense. I don't expect
much speedup though, because Linux doesn't touch the TPR and Windows is
likely going to use the Hyper-V APIC MSRs when APICv is disabled. For
this reason I'm not sure if the patch is useful in practice.
To test this patch, you have to run kvm-unit-tests with Hyper-V
synthetic interrupt enabled. Did you do this?
Paolo
> 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>
> ---
> arch/x86/kvm/vmx.c | 41 ++++++++++++++++++++++-------------------
> 1 file changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 5cede40..e703129 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6336,7 +6336,7 @@ static void wakeup_handler(void)
>
> static __init int hardware_setup(void)
> {
> - int r = -ENOMEM, i, msr;
> + int r = -ENOMEM, i;
>
> rdmsrl_safe(MSR_EFER, &host_efer);
>
> @@ -6464,18 +6464,6 @@ 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_ept) {
> kvm_mmu_set_mask_ptes(VMX_EPT_READABLE_MASK,
> (enable_ept_ad_bits) ? VMX_EPT_ACCESS_BIT : 0ull,
> @@ -8435,12 +8423,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))
> @@ -8449,8 +8432,28 @@ static void vmx_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
> sec_exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
>
> if (set) {
> + int msr;
> +
> sec_exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
> sec_exec_control |= SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
> +
> + if (kvm_vcpu_apicv_active(vcpu)) {
> + 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 (vmx_exec_control(to_vmx(vcpu)) & CPU_BASED_TPR_SHADOW) {
> + /* TPR */
> + vmx_disable_intercept_msr_read_x2apic(0x808);
> + vmx_disable_intercept_msr_write_x2apic(0x808);
> + }
> } else {
> sec_exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
> sec_exec_control |= SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
>