| Message ID | 20220802160756.339464-23-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: VMX: Support updated eVMCSv1 revision + use vmcs_config for L1 VMX MSRs | expand |
On Tue, Aug 02, 2022, Vitaly Kuznetsov wrote: > While it seems reasonable to not expose LOAD_IA32_PERF_GLOBAL_CTRL controls > to L1 hypervisor on buggy CPUs, such change would inevitably break live > migration from older KVMs where the controls are exposed. Keep the status quo > for now, L1 hypervisor itself is supposed to take care of the errata. As noted before, this statement is wrong as it requires guest FMS == host FMS, but it's irrelevant because KVM can emulate the control unconditionally. I'll test and fold in my suggested patch[*] (assuming it works) and reword this part of the changelog. Ah, and I'll also need to fold in a patch to actually emulate the controls without hardware support. [*] https://lore.kernel.org/all/YtnZmCutdd5tpUmz@google.com > Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/kvm/vmx/vmx.c | 59 +++++++++++++++++++++++++----------------- > 1 file changed, 35 insertions(+), 24 deletions(-) > ... > @@ -8192,6 +8199,10 @@ static __init int hardware_setup(void) > if (setup_vmcs_config(&vmcs_config, &vmx_capability) < 0) > return -EIO; > > + if (cpu_has_perf_global_ctrl_bug()) > + pr_warn_once("kvm: VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL " > + "does not work properly. Using workaround\n"); Any objections to opportunistically tweaking this? pr_warn_once("kvm: CPU has VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL erratum," "using MSR load/store lists for PERF_GLOBAL_CTRL\n"); > + > if (boot_cpu_has(X86_FEATURE_NX)) > kvm_enable_efer_bits(EFER_NX); > > -- > 2.35.3 >
Sean Christopherson <seanjc@google.com> writes: > On Tue, Aug 02, 2022, Vitaly Kuznetsov wrote: >> While it seems reasonable to not expose LOAD_IA32_PERF_GLOBAL_CTRL controls >> to L1 hypervisor on buggy CPUs, such change would inevitably break live >> migration from older KVMs where the controls are exposed. Keep the status quo >> for now, L1 hypervisor itself is supposed to take care of the errata. > > As noted before, this statement is wrong as it requires guest FMS == host FMS, > but it's irrelevant because KVM can emulate the control unconditionally. I'll > test and fold in my suggested patch[*] (assuming it works) and reword this part > of the changelog. Ah, and I'll also need to fold in a patch to actually emulate > the controls without hardware support. > > [*] https://lore.kernel.org/all/YtnZmCutdd5tpUmz@google.com Oh, I missed the part that my changelog is actually wrong when Paolo said "Can you send this as a separate patch", no objections to re-wording! > >> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> --- >> arch/x86/kvm/vmx/vmx.c | 59 +++++++++++++++++++++++++----------------- >> 1 file changed, 35 insertions(+), 24 deletions(-) >> > > ... > >> @@ -8192,6 +8199,10 @@ static __init int hardware_setup(void) >> if (setup_vmcs_config(&vmcs_config, &vmx_capability) < 0) >> return -EIO; >> >> + if (cpu_has_perf_global_ctrl_bug()) >> + pr_warn_once("kvm: VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL " >> + "does not work properly. Using workaround\n"); > > Any objections to opportunistically tweaking this? > > pr_warn_once("kvm: CPU has VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL erratum," > "using MSR load/store lists for PERF_GLOBAL_CTRL\n"); > Personally I'd say just pr_warn_once("kvm: CPU has VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL erratum\n"); leaving aside the workaround KVM uses. This is merely an implementation detail which KVM users most likely don't really need. I have no strong opinion though, feel free to adjust. >> + >> if (boot_cpu_has(X86_FEATURE_NX)) >> kvm_enable_efer_bits(EFER_NX); >> >> -- >> 2.35.3 >> >
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index baf1054765a7..ab5d16691c5e 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -2495,6 +2495,30 @@ static bool cpu_has_sgx(void) return cpuid_eax(0) >= 0x12 && (cpuid_eax(0x12) & BIT(0)); } +/* + * Some cpus support VM_{ENTRY,EXIT}_IA32_PERF_GLOBAL_CTRL but they + * can't be used due to errata where VM Exit may incorrectly clear + * IA32_PERF_GLOBAL_CTRL[34:32]. Work around the errata by using the + * MSR load mechanism to switch IA32_PERF_GLOBAL_CTRL. + */ +static bool cpu_has_perf_global_ctrl_bug(void) +{ + if (boot_cpu_data.x86 == 0x6) { + switch (boot_cpu_data.x86_model) { + case INTEL_FAM6_NEHALEM_EP: /* AAK155 */ + case INTEL_FAM6_NEHALEM: /* AAP115 */ + case INTEL_FAM6_WESTMERE: /* AAT100 */ + case INTEL_FAM6_WESTMERE_EP: /* BC86,AAY89,BD102 */ + case INTEL_FAM6_NEHALEM_EX: /* BA97 */ + return true; + default: + break; + } + } + + return false; +} + static __init int adjust_vmx_controls(u32 ctl_min, u32 ctl_opt, u32 msr, u32 *result) { @@ -2650,30 +2674,6 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf, _vmexit_control &= ~x_ctrl; } - /* - * Some cpus support VM_{ENTRY,EXIT}_IA32_PERF_GLOBAL_CTRL but they - * can't be used due to an errata where VM Exit may incorrectly clear - * IA32_PERF_GLOBAL_CTRL[34:32]. Workaround the errata by using the - * MSR load mechanism to switch IA32_PERF_GLOBAL_CTRL. - */ - if (boot_cpu_data.x86 == 0x6) { - switch (boot_cpu_data.x86_model) { - case INTEL_FAM6_NEHALEM_EP: /* AAK155 */ - case INTEL_FAM6_NEHALEM: /* AAP115 */ - case INTEL_FAM6_WESTMERE: /* AAT100 */ - case INTEL_FAM6_WESTMERE_EP: /* BC86,AAY89,BD102 */ - case INTEL_FAM6_NEHALEM_EX: /* BA97 */ - _vmentry_control &= ~VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL; - _vmexit_control &= ~VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL; - pr_warn_once("kvm: VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL " - "does not work properly. Using workaround\n"); - break; - default: - break; - } - } - - rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high); /* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */ @@ -4269,6 +4269,9 @@ static u32 vmx_vmentry_ctrl(void) VM_ENTRY_LOAD_IA32_EFER | VM_ENTRY_IA32E_MODE); + if (cpu_has_perf_global_ctrl_bug()) + vmentry_ctrl &= ~VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL; + return vmentry_ctrl; } @@ -4286,6 +4289,10 @@ static u32 vmx_vmexit_ctrl(void) if (vmx_pt_mode_is_system()) vmexit_ctrl &= ~(VM_EXIT_PT_CONCEAL_PIP | VM_EXIT_CLEAR_IA32_RTIT_CTL); + + if (cpu_has_perf_global_ctrl_bug()) + vmexit_ctrl &= ~VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL; + /* Loading of EFER and PERF_GLOBAL_CTRL are toggled dynamically */ return vmexit_ctrl & ~(VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL | VM_EXIT_LOAD_IA32_EFER); @@ -8192,6 +8199,10 @@ static __init int hardware_setup(void) if (setup_vmcs_config(&vmcs_config, &vmx_capability) < 0) return -EIO; + if (cpu_has_perf_global_ctrl_bug()) + pr_warn_once("kvm: VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL " + "does not work properly. Using workaround\n"); + if (boot_cpu_has(X86_FEATURE_NX)) kvm_enable_efer_bits(EFER_NX);