Re: [PATCH] x86/CPU/AMD: Clear virtualized VMLOAD/VMSAVE on Zen4 client

[Sean Christopherson <seanjc@google.com>]

On Wed, Nov 06, 2024, Mario Limonciello wrote:
> On 11/6/2024 09:48, Maxim Levitsky wrote:
> > On Wed, 2024-11-06 at 07:15 -0800, Sean Christopherson wrote:
> > > On Wed, Nov 06, 2024, Mario Limonciello wrote:
> > > > On 11/6/2024 09:03, Sean Christopherson wrote:

...

> > > > > > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> > > > > > index 015971adadfc7..ecd42c2b3242e 100644
> > > > > > --- a/arch/x86/kernel/cpu/amd.c
> > > > > > +++ b/arch/x86/kernel/cpu/amd.c
> > > > > > @@ -924,6 +924,17 @@ static void init_amd_zen4(struct cpuinfo_x86 *c)
> > > > > >    {
> > > > > >    	if (!cpu_has(c, X86_FEATURE_HYPERVISOR))
> > > > > >    		msr_set_bit(MSR_ZEN4_BP_CFG, MSR_ZEN4_BP_CFG_SHARED_BTB_FIX_BIT);
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * These Zen4 SoCs advertise support for virtualized VMLOAD/VMSAVE
> > > > > > +	 * in some BIOS versions but they can lead to random host reboots.
> > > > > 
> > > > > Uh, CPU bug?  Erratum?
> > > > 
> > > > BIOS bug.  Those shouldn't have been advertised.
> > 
> > Hi!
> > 
> > My question is, why would AMD drop support intentionally for VLS on client machines?
> > 
> > I understand that there might be a errata, and I don't object disabling the
> > feature because of this.
> > 
> > But hearing that 'These instructions aren't intended to be advertised' means that
> > AMD intends to stop supporting virtualization on client systems or at least partially
> > do so.
> 
> Don't read into it too far.  It's just a BIOS problem with those
> instructions "specifically" on the processors indicated here.  Other
> processors (for example Zen 5 client processors) do correctly advertise
> support where applicable.
> 
> When they launched those bits weren't supposed to be set to indicate
> support, but BIOS did set them.

As you quite clearly call out below, this isn't simply a BIOS problem.

> > That worries me. So far AMD was much better that Intel supporting most of the
> > features across all of the systems which is very helpful in various scenarios,
> > and this is very appreciated by the community.
> > 
> > Speaking strictly personally here, as a AMD fan.
> > 
> > Best regards,> 	Maxim Levitsky
> > 
> > 
> > > 
> > > Why not?  "but they can lead to random host reboots" is a description of the
> > > symptom, not an explanation for why KVM is unable to use a feature that is
> > > apparently support by the CPU.
> > > 
> > > And if the CPU doesn't actually support virtualized VMLOAD/VMSAVE, then this is
> > > a much bigger problem, because it means KVM is effectively giving the guest read
> > > and write access to all of host memory.
> > > 
> > 
> > 
> 
> I'm gathering that what supported means to you and what it means to me are
> different things.

Yes.  And the distinction matters greatly in this case, because "VMLOAD/VMSAVE
in the guest are broken" is *very* different than "VMLOAD/VMSAVE in the guest
actually operate on SPAs, not GPAs".

> "Architecturally" the instructions for virtualized VMLOAD/VMSAVE exist.

Which means they're supported, but broken.

> There are problems with them on these processors, and for that reason the
> BIOS was not supposed to set those bits but it did.

In other words, this a CPU bug.  The kernel comment absolutely needs to reflect
that.  Passing this off as BIOS going rogue is misleading and confusing.