Re: [tip:x86/alternatives] [x86/alternatives] ee8962082a: WARNING:at_arch/x86/kernel/cpu/cpuid-deps.c:#do_clear_cpu_cap

[Sean Christopherson <seanjc@google.com>]

On Tue, Apr 30, 2024, Borislav Petkov wrote:
> + Sean.
> > [ 0.055225][ T0] ? __warn (kernel/panic.c:694) 
> > [ 0.055225][ T0] ? do_clear_cpu_cap (arch/x86/kernel/cpu/cpuid-deps.c:118 (discriminator 1)) 
> > [ 0.055225][ T0] ? report_bug (lib/bug.c:180 lib/bug.c:219) 
> > [ 0.055225][ T0] ? handle_bug (arch/x86/kernel/traps.c:239 (discriminator 1)) 
> > [ 0.055225][ T0] ? exc_invalid_op (arch/x86/kernel/traps.c:260 (discriminator 1)) 
> > [ 0.055225][ T0] ? asm_exc_invalid_op (arch/x86/include/asm/idtentry.h:621) 
> > [ 0.055225][ T0] ? do_clear_cpu_cap (arch/x86/kernel/cpu/cpuid-deps.c:118 (discriminator 1)) 
> > [ 0.055225][ T0] ? __pfx_do_clear_cpu_cap (arch/x86/kernel/cpu/cpuid-deps.c:109) 
> > [ 0.055225][ T0] init_ia32_feat_ctl (arch/x86/kernel/cpu/feat_ctl.c:181)
> 
> Yap, works as designed:

...

> won't work as expected because the patching has already happened.
> 
> And I'm not sure even the dynamic testing *cpu_has() does will always
> work as we do this dance in get_cpu_cap() with forced flags.
> 
> So, I'm thinking init_ia32_feat_ctl() should run in early_init_intel()
> which is before alternatives.
> 
> And looking at init_ia32_feat_ctl(), all it does is set and clear
> a bunch of bits so I think it should be ok.

Hmm, I don't think the problem is that init_ia32_feat_ctl() is called too late.
It too is called from the BSP prior to alternative_instructions():

  arch_cpu_finalize_init()
  |
  -> identify_boot_cpu()
     |
     -> identify_cpu()
        |
        -> .c_init() => init_intel()

Ah, and the WARN even specifically checks for the case where there's divergence
from the boot CPU:

	if (boot_cpu_has(feature))
		WARN_ON(alternatives_patched);

So I think this is a "real" warning about a misconfigured system, where VMX is
fully configured in MSR_IA32_FEAT_CTL on the boot CPU, but is disabled on a
secondary CPU.

FWIW, KVM will play nice with such a setup, as KVM specifically checks that VMX
is fully enabled all CPUs, but I would still consider this a misconfigured system.

> > [ 0.055225][ T0] init_intel (arch/x86/include/asm/msr.h:146 arch/x86/include/asm/msr.h:300 arch/x86/kernel/cpu/intel.c:583 arch/x86/kernel/cpu/intel.c:687) 
> > [ 0.055225][ T0] identify_cpu (arch/x86/kernel/cpu/common.c:1824) 
> > [ 0.055225][ T0] identify_secondary_cpu (arch/x86/kernel/cpu/common.c:1949) 
> > [ 0.055225][ T0] smp_store_cpu_info (arch/x86/kernel/smpboot.c:333) 
> > [ 0.055225][ T0] start_secondary (arch/x86/kernel/smpboot.c:197 arch/x86/kernel/smpboot.c:281) 
> > [ 0.055225][ T0] ? __pfx_start_secondary (arch/x86/kernel/smpboot.c:231) 
> > [ 0.055225][ T0] common_startup_64 (arch/x86/kernel/head_64.S:421) 
> > [    0.055225][    T0]  </TASK>
> > [    0.055225][    T0] ---[ end trace 0000000000000000 ]---