* [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing
@ 2018-01-05 15:43 Paolo Bonzini
2018-01-08 6:45 ` Kang, Luwei
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Paolo Bonzini @ 2018-01-05 15:43 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Luwei Kang
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@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
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 */
--
1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing
2018-01-05 15:43 [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing Paolo Bonzini
@ 2018-01-08 6:45 ` Kang, Luwei
2018-01-08 21:23 ` Jim Mattson
2018-01-09 9:35 ` Paolo Bonzini
2 siblings, 0 replies; 5+ messages in thread
From: Kang, Luwei @ 2018-01-08 6:45 UTC (permalink / raw)
To: Paolo Bonzini, linux-kernel, kvm
> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Friday, January 5, 2018 11:43 PM
> To: linux-kernel@vger.kernel.org; kvm@vger.kernel.org
> Cc: Kang, Luwei <luwei.kang@intel.com>
> 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@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 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@intel.com>
Thanks,
Luwei Kang
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing
2018-01-05 15:43 [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing Paolo Bonzini
2018-01-08 6:45 ` Kang, Luwei
@ 2018-01-08 21:23 ` Jim Mattson
2018-01-08 23:11 ` Paolo Bonzini
2018-01-09 9:35 ` Paolo Bonzini
2 siblings, 1 reply; 5+ messages in thread
From: Jim Mattson @ 2018-01-08 21:23 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: LKML, kvm list, Luwei Kang
I would like to campaign for removing all of the precomputed MSR
permission bitmaps, and using a separate bitmap for each vCPU, a la
svm.c.
On Fri, Jan 5, 2018 at 7:43 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 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@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 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 */
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing
2018-01-08 21:23 ` Jim Mattson
@ 2018-01-08 23:11 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2018-01-08 23:11 UTC (permalink / raw)
To: Jim Mattson; +Cc: LKML, kvm list, Luwei Kang
----- Original Message -----
> From: "Jim Mattson" <jmattson@google.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "LKML" <linux-kernel@vger.kernel.org>, "kvm list" <kvm@vger.kernel.org>, "Luwei Kang" <luwei.kang@intel.com>
> Sent: Monday, January 8, 2018 10:23:41 PM
> Subject: Re: [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing
>
> I would like to campaign for removing all of the precomputed MSR
> permission bitmaps, and using a separate bitmap for each vCPU, a la
> svm.c.
As usual, patches are welcome. :)
Paolo
> On Fri, Jan 5, 2018 at 7:43 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > 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@intel.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > 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 */
> >
> > --
> > 1.8.3.1
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing
2018-01-05 15:43 [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing Paolo Bonzini
2018-01-08 6:45 ` Kang, Luwei
2018-01-08 21:23 ` Jim Mattson
@ 2018-01-09 9:35 ` Paolo Bonzini
2 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2018-01-09 9:35 UTC (permalink / raw)
To: linux-kernel, kvm
On 05/01/2018 16:43, Paolo Bonzini wrote:
> 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@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/kvm/vmx.c | 126 ++++++++++++++++++++---------------------------------
> 1 file changed, 48 insertions(+), 78 deletions(-)
Scratch it, this patch is wrong. You cannot change the MSR bitmaps,
they are shared across all VCPUs.
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-09 9:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-05 15:43 [PATCH] KVM: VMX: use same MSR bitmaps for 32-/64-bit modes, fix MSR bitmaps for processor tracing Paolo Bonzini
2018-01-08 6:45 ` Kang, Luwei
2018-01-08 21:23 ` Jim Mattson
2018-01-08 23:11 ` Paolo Bonzini
2018-01-09 9:35 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®