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

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

[Sean Christopherson]

+KVM, given that this quite obviously affects KVM...

On Tue, Nov 05, 2024, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> A number of Zen4 client SoCs advertise the ability to use virtualized
> VMLOAD/VMSAVE, but using these instructions is reported to be a cause
> of a random host reboot.
> 
> These instructions aren't intended to be advertised on Zen4 client
> so clear the capability.
> 
> Cc: stable@vger.kernel.org
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219009
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  arch/x86/kernel/cpu/amd.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> 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?

> +	 */
> +	switch (c->x86_model) {
> +	case 0x18 ... 0x1f:
> +	case 0x60 ... 0x7f:
> +		clear_cpu_cap(c, X86_FEATURE_V_VMSAVE_VMLOAD);
> +		break;
> +	}
>  }
>  
>  static void init_amd_zen5(struct cpuinfo_x86 *c)
> -- 
> 2.43.0
> 

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

[Mario Limonciello]

On 11/6/2024 09:03, Sean Christopherson wrote:
> +KVM, given that this quite obviously affects KVM...
> 
> On Tue, Nov 05, 2024, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> A number of Zen4 client SoCs advertise the ability to use virtualized
>> VMLOAD/VMSAVE, but using these instructions is reported to be a cause
>> of a random host reboot.
>>
>> These instructions aren't intended to be advertised on Zen4 client
>> so clear the capability.
>>
>> Cc: stable@vger.kernel.org
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219009
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   arch/x86/kernel/cpu/amd.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> 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.

> 
>> +	 */
>> +	switch (c->x86_model) {
>> +	case 0x18 ... 0x1f:
>> +	case 0x60 ... 0x7f:
>> +		clear_cpu_cap(c, X86_FEATURE_V_VMSAVE_VMLOAD);
>> +		break;
>> +	}
>>   }
>>   
>>   static void init_amd_zen5(struct cpuinfo_x86 *c)
>> -- 
>> 2.43.0
>>

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

[Sean Christopherson]

On Wed, Nov 06, 2024, Mario Limonciello wrote:
> On 11/6/2024 09:03, Sean Christopherson wrote:
> > +KVM, given that this quite obviously affects KVM...
> > 
> > On Tue, Nov 05, 2024, Mario Limonciello wrote:
> > > From: Mario Limonciello <mario.limonciello@amd.com>
> > > 
> > > A number of Zen4 client SoCs advertise the ability to use virtualized
> > > VMLOAD/VMSAVE, but using these instructions is reported to be a cause
> > > of a random host reboot.
> > > 
> > > These instructions aren't intended to be advertised on Zen4 client
> > > so clear the capability.
> > > 
> > > Cc: stable@vger.kernel.org
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=219009
> > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > ---
> > >   arch/x86/kernel/cpu/amd.c | 11 +++++++++++
> > >   1 file changed, 11 insertions(+)
> > > 
> > > 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.

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.

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

[Maxim Levitsky]

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:
> > > +KVM, given that this quite obviously affects KVM...
> > > 
> > > On Tue, Nov 05, 2024, Mario Limonciello wrote:
> > > > From: Mario Limonciello <mario.limonciello@amd.com>
> > > > 
> > > > A number of Zen4 client SoCs advertise the ability to use virtualized
> > > > VMLOAD/VMSAVE, but using these instructions is reported to be a cause
> > > > of a random host reboot.
> > > > 
> > > > These instructions aren't intended to be advertised on Zen4 client
> > > > so clear the capability.
> > > > 
> > > > Cc: stable@vger.kernel.org
> > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=219009
> > > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > > ---
> > > >   arch/x86/kernel/cpu/amd.c | 11 +++++++++++
> > > >   1 file changed, 11 insertions(+)
> > > > 
> > > > 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.

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.
> 

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

[Mario Limonciello]

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:
>>>> +KVM, given that this quite obviously affects KVM...
>>>>
>>>> On Tue, Nov 05, 2024, Mario Limonciello wrote:
>>>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>>>
>>>>> A number of Zen4 client SoCs advertise the ability to use virtualized
>>>>> VMLOAD/VMSAVE, but using these instructions is reported to be a cause
>>>>> of a random host reboot.
>>>>>
>>>>> These instructions aren't intended to be advertised on Zen4 client
>>>>> so clear the capability.
>>>>>
>>>>> Cc: stable@vger.kernel.org
>>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219009
>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>> ---
>>>>>    arch/x86/kernel/cpu/amd.c | 11 +++++++++++
>>>>>    1 file changed, 11 insertions(+)
>>>>>
>>>>> 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.

> 
> 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.  "Architecturally" the instructions for 
virtualized VMLOAD/VMSAVE exist.  There are problems with them on these 
processors, and for that reason the BIOS was not supposed to set those 
bits but it did.

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

[Sean Christopherson]

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.

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

[Maxim Levitsky]

On Wed, 2024-11-06 at 09:58 -0600, 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:
> > > > > +KVM, given that this quite obviously affects KVM...
> > > > > 
> > > > > On Tue, Nov 05, 2024, Mario Limonciello wrote:
> > > > > > From: Mario Limonciello <mario.limonciello@amd.com>
> > > > > > 
> > > > > > A number of Zen4 client SoCs advertise the ability to use virtualized
> > > > > > VMLOAD/VMSAVE, but using these instructions is reported to be a cause
> > > > > > of a random host reboot.
> > > > > > 
> > > > > > These instructions aren't intended to be advertised on Zen4 client
> > > > > > so clear the capability.
> > > > > > 
> > > > > > Cc: stable@vger.kernel.org
> > > > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=219009
> > > > > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > > > > ---
> > > > > >    arch/x86/kernel/cpu/amd.c | 11 +++++++++++
> > > > > >    1 file changed, 11 insertions(+)
> > > > > > 
> > > > > > 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.

I am very glad to hear that, thanks!

> 
> When they launched those bits weren't supposed to be set to indicate 
> support, but BIOS did set them.

In other words if I understand correctly, there was an errata and to work
it around on the affected CPUs, AMD decided to disable the feature in CPUID,
which is reasonable, but some BIOS vendors forgot to do this.

It all makes sense, thanks again!

Best regards,
	Maxim Levitsky

> 
> > 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.  "Architecturally" the instructions for 
> virtualized VMLOAD/VMSAVE exist.  There are problems with them on these 
> processors, and for that reason the BIOS was not supposed to set those 
> bits but it did.
>