Re: [PATCH v2 4/7] KVM: x86: Add a per-vendor callback to setup EFER caps

[Yosry Ahmed]

On Wed, Jul 8, 2026 at 7:01 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Tue, Jul 07, 2026, Yosry Ahmed wrote:
> > On Tue, Jul 7, 2026 at 2:56 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > >
> > > On Mon, Jul 06, 2026, Yosry Ahmed wrote:
> > > > Move handling EFER.SVME and EFER.LMSLE from hardware setup to a new
> > > > optional per-vendor callback invoked from kvm_setup_efer_caps(). This
> > > > centralizes allowed EFER bits handling to kvm_setup_efer_caps(),
> > > > facilitating following changes to move efer_reserved_bits into kvm_caps.
> > > >
> > > > Move the call to kvm_setup_efer_caps() after per-vendor ops are
> > > > initialized.
> > >
> > > Why?
> > >
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > index a0b2c40d93c21..a297a77469b38 100644
> > > > --- a/arch/x86/kvm/x86.c
> > > > +++ b/arch/x86/kvm/x86.c
> > > > @@ -6903,6 +6903,8 @@ static void kvm_setup_efer_caps(void)
> > > >
> > > >       if (kvm_cpu_cap_has(X86_FEATURE_AUTOIBRS))
> > > >               kvm_enable_efer_bits(EFER_AUTOIBRS);
> > > > +
> > > > +     kvm_x86_call(setup_efer_caps)();
> > >
> > > I would rather move the togging to kvm_setup_efer_caps(), e.g.
> >
> > I didn't do it this way because it creates a dependency on SVM setting
> > the X86_FEATURE_SVM cap before kvm_setup_efer_caps() is called.
>
> For all intents and purposes, that dependency already exists due to the
> X86_FEATURE_{NX,FXSR_OPT,AUTOIBRS} checks.  And thanks to kvm_is_configuring_cpu_caps,
> it's "impossible" for those caps to be toggled outside of svm_set_cpu_caps().

Right, I missed this. kvm_is_configuring_cpu_caps is neat. I wonder if
we can make that an enum with values {UNINIT, CONFIGURING,
INITIALIZED}, then we can be more paranoid and WARN if the the state
isn't INITIALIZED in kvm_setup_efer_caps(). That might be too paranoid
though.

>
> > e.g. it would break if the call to kvm_setup_efer_caps() is moved
> > before the vendor-specific hardware_setup().
>
> As above, that would break for other reasons.
>
> > Maybe that's fine, but it just seemed like the dependency can be easily
> > avoided here by adding a new vendor-specific callback.
>
> No, all it does is change what can go wrong.   E.g. if KVM cleared "nested" in
> svm_hardware_setup(), which is *very* realistic given that we carry an internal
> patch to disable nested if TDP is disabled, then hoisting kvm_setup_efer_caps()
> above ops->hardware_setup() would still break (even ignoring the above issues).

Yeah, thanks for explaining that. I will drop the per-vendor callback
and just do it directly in kvm_setup_efer_caps().