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

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZyuVHJ9K51tOkOMM@google.com>
Date: Wed, 6 Nov 2024 08:11:08 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Mario Limonciello <superm1@...nel.org>
Cc: Maxim Levitsky <mlevitsk@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, 
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>, "H . Peter Anvin" <hpa@...or.com>, Nikolay Borisov <nik.borisov@...e.com>, 
	Tom Lendacky <thomas.lendacky@....com>, Brijesh Singh <brijesh.singh@....com>, 
	"open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <linux-kernel@...r.kernel.org>, 
	Mario Limonciello <mario.limonciello@....com>, kvm@...r.kernel.org
Subject: Re: [PATCH] x86/CPU/AMD: Clear virtualized VMLOAD/VMSAVE on Zen4 client

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.