linux-kernel - Re: [PATCH v6 06/20] KVM: VMX: Set FRED MSR intercepts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aLDm9YID-r5WWcD9@google.com>
Date: Thu, 28 Aug 2025 16:32:05 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Xin Li <xin@...or.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org, 
	linux-doc@...r.kernel.org, pbonzini@...hat.com, corbet@....net, 
	tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, luto@...nel.org, 
	peterz@...radead.org, andrew.cooper3@...rix.com, chao.gao@...el.com, 
	hch@...radead.org
Subject: Re: [PATCH v6 06/20] KVM: VMX: Set FRED MSR intercepts

On Wed, Aug 27, 2025, Xin Li wrote:
> On 8/27/2025 3:24 PM, Xin Li wrote:
> > On 8/26/2025 3:17 PM, Sean Christopherson wrote:
> > > > +        if (!kvm_cpu_cap_has(X86_FEATURE_SHSTK))
> > > > +            wrmsrns(MSR_IA32_FRED_SSP0, vmx->msr_guest_fred_ssp0);
> > > FWIW, if we can't get an SDM change, don't bother with RDMSR/WRMSRNS, just
> > > configure KVM to intercept accesses.  Then in kvm_set_msr_common(), pivot on
> > > X86_FEATURE_SHSTK, e.g.
> > 
> > 
> > Intercepting is a solid approach: it ensures the guest value is fully
> > virtual and does not affect the hardware FRED SSP0 MSR.  Of course the code
> > is also simplified.
> > 
> > 
> > > 
> > >     case MSR_IA32_U_CET:
> > >     case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
> > >         if (!kvm_cpu_cap_has(X86_FEATURE_SHSTK)) {
> > >             WARN_ON_ONCE(msr != MSR_IA32_FRED_SSP0);
> > >             vcpu->arch.fred_rsp0_fallback = data;
> 
> Putting fred_rsp0_fallback in struct kvm_vcpu_arch reminds me one thing:
> 
> We know AMD will do FRED and follow the FRED spec for bare metal, but
> regarding virtualization of FRED, I have no idea how it will be done on
> AMD, so I keep the KVM FRED code in VMX files, e.g., msr_guest_fred_rsp0 is
> defined in struct vcpu_vmx, and saved/restored in vmx.c.

The problem is that if you do that, then the handling of MSR_IA32_PL0_SSP takes
completely different paths depending on vendor, theoretically on hardware, and
on guest CPUID model.  That makes it _really_ difficult to understand how PL0_SSP
is emulated by KVM.

And I actually think that's moot anyways.  KVM _always_ needs to emulated MSR
accesses in software, and the whole goofy PL0_SSP behavior is a bare metal quirk,
not a virtualization quirk.  So unless AMD defines different architecture (which
is certainly possible), AMD will also need arch.fred_rsp0_fallback.

> It is a future task to make common KVM FRED code for Intel and AMD.

No, this is not how I want to approach hardware enabling.  KVM needs to guard
against false advertising, e.g. ensure likely-to-be-common CPUID features are
explicitly cleared in the other vendor.  But deliberately burying code that's
vendor agnostic in whatever vendor support happens to come along first isn't
necessary by any means, and is usually a net negative in the grand scheme, and
often in a big way.

E.g. in this case, if arch.fred_rsp0_fallback ends up being unnecessary for AMD,
we probably don't even need to do anything, KVM will just have a field that's
only used on Intel because the quirky scenario can't be reached on AMD.

But if we bury the code in VMX, then the _best_ case scenario is that KVM carries
a weird split of responsibility in perpetuity (happy path handled in x86.c, rare
sad path handled in vmx.c).  And the worst case scenario is that we carry the
weird split for some time, and then have to undo all of it when AMD support comes
along.  Actually, the worst case scenario is that we forget about the VMX code
and re-implement the same thing in svm.c.