On Wed, Jun 29, 2022 at 8:07 AM Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote:
>
> As a preparation to reusing the result of setup_vmcs_config() for setting
> up nested VMX control MSRs, move LOAD_IA32_PERF_GLOBAL_CTRL errata handling
> to vmx_vmexit_ctrl()/vmx_vmentry_ctrl() and print the warning from
> hardware_setup(). 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 know, L1 hypervisor
> itself is supposed to take care of the errata.
It can only do that if L1 doesn't lie about the model. This is why
F/M/S checks are, in general, evil.
> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> ---
> arch/x86/kvm/vmx/vmx.c | 62 ++++++++++++++++++++++++++----------------
> 1 file changed, 38 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index fb58b0be953d..5f7ef1f8d2c6 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2416,6 +2416,31 @@ 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 an errata where VM Exit may incorrectly clear
Nit: erratum (singular), or drop the 'an' to refer to errata (plural).
> + * IA32_PERF_GLOBAL_CTRL[34:32]. Workaround the errata by using the
Nit: workaround (one word) is a noun. The verb form is "work around."
> + * 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 26: /* AAK155 */
> + case 30: /* AAP115 */
> + case 37: /* AAT100 */
> + case 44: /* BC86,AAY89,BD102 */
> + case 46: /* BA97 */
Nit: Replace decimal model numbers with mnemonics. See
https://lore.kernel.org/kvm/20220629222221.986645-1-jmattson@xxxxxxxxxx/.
> + return true;
> + default:
> + break;
> + }
> + }
> +
> + return false;
> +}
Is it worth either (a) memoizing the result, or (b) toggling a static
branch? Or am I prematurely optimizing?
> +
> +
> static __init int adjust_vmx_controls(u32 ctl_min, u32 ctl_opt,
> u32 msr, u32 *result)
> {
> @@ -2572,30 +2597,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 26: /* AAK155 */
> - case 30: /* AAP115 */
> - case 37: /* AAT100 */
> - case 44: /* BC86,AAY89,BD102 */
> - case 46: /* 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. */
> @@ -4188,6 +4189,10 @@ 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;
> }
>
> @@ -4202,6 +4207,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);
> @@ -8117,6 +8126,11 @@ 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);
>
> --
> 2.35.3
>
