RE: x86/bugs: KVM: Add support for SRSO_MSR_FIX, back for moar

Kaplan, David
2025-05-05T18:25:54Z

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Sean Christopherson <seanjc@google.com>
> Sent: Monday, May 5, 2025 1:04 PM
> To: Kaplan, David <David.Kaplan@amd.com>
> Cc: Borislav Petkov <bp@alien8.de>; Yosry Ahmed <yosry.ahmed@linux.dev>;
> Patrick Bellasi <derkling@google.com>; Paolo Bonzini <pbonzini@redhat.com>;
> Josh Poimboeuf <jpoimboe@redhat.com>; Pawan Gupta
> <pawan.kumar.gupta@linux.intel.com>; x86@kernel.org; kvm@vger.kernel.org;
> linux-kernel@vger.kernel.org; Patrick Bellasi <derkling@matbug.net>; Brendan
> Jackman <jackmanb@google.com>; Michael Larabel
> <Michael@michaellarabel.com>
> Subject: Re: x86/bugs: KVM: Add support for SRSO_MSR_FIX, back for moar
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Mon, May 05, 2025, David Kaplan wrote:
> > > > Almost. My thought was that kvm_run could do something like:
> > > >
> > > > If (!this_cpu_read(bp_spec_reduce_is_set)) {
> > > > wrmsrl to set BP_SEC_REDUCE
> > > > this_cpu_write(bp_spec_reduce_is_set, 1) }
> > > >
> > > > That ensures the bit is set for your core before VMRUN. And as
> > > > noted below, you can clear the bit when the count drops to 0 but
> > > > that one is safe from race conditions.
> > >
> > > /facepalm
> > >
> > > I keep inverting the scenario in my head. I'm so used to KVM
> > > needing to ensure it doesn't run with guest state that I keep
> > > forgetting that running with
> > > BP_SPEC_REDUCE=1 is fine, just a bit slower.
> > >
> > > With that in mind, the best blend of simplicity and performance is
> > > likely to hook
> > > svm_prepare_switch_to_guest() and svm_prepare_host_switch().
> > > switch_to_guest() is called when KVM is about to do VMRUN, and
> > > host_switch() is called when the vCPU is put, i.e. when the task is
> > > scheduled out or when KVM_RUN exits to userspace.
> > >
> > > The existing svm->guest_state_loaded guard avoids toggling the bit
> > > when KVM handles a VM-Exit and re-enters the guest. The kernel may
> > > run a non-trivial amount of code with BP_SPEC_REDUCE, e.g. if #NPF
> > > triggers swap-in, an IRQ arrives while handling the exit, etc., but that's all fine
> from a security perspective.
> > >
> > > IIUC, per Boris[*] an IBPB is needed when toggling BP_SPEC_REDUCE
> > > on-
> > > demand:
> > >
> > > : You want to IBPB before clearing the MSR as otherwise host kernel
> > > will be
> > > : running with the mistrained gunk from the guest.
> > >
> > > [*]
> > > https://lore.kernel.org/all/20250217160728.GFZ7NewJHpMaWdiX2M@fat_cr
> > > ate.loc
> > > al
> > >
> > > Assuming that's the case...
> > >
> > > Compile-tested only. If this looks/sounds sane, I'll test the
> > > mechanics and write a changelog.
> >
> > I'm having trouble following the patch...where do you clear the MSR bit?
>
> Gah, the rdmsrl() in svm_prepare_host_switch() should be a wrmsrl().
>
> > I thought a per-cpu "cache" of the MSR bit might be good to avoid
> > having to issue slow RDMSRs, if these paths are 'hot'. I don't know
> > if that's the case or not.
>
> Oh, you were only proposing deferring setting BP_SPEC_REDUCE. I like it. That
> avoids setting BP_SPEC_REDUCE on CPUs that aren't running VMs, e.g.
> housekeeping CPUs, and makes it a heck of a lot easier to elide the lock on 1<=>N
> transitions.
>
> I posted v2 using that approach (when lore catches up):
>
> https://lore.kernel.org/all/20250505180300.973137-1-seanjc@google.com
>

LGTM.

Thanks --David Kaplan