KVM: x86: Allow XSAVES on CPUs where host doesn't use it due to an errata

  From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>

Since commit b0563468eeac ("x86/CPU/AMD: Disable XSAVES on AMD family 0x17")
kernel unconditionally clears the XSAVES CPU feature bit on Zen1/2 CPUs.

Since KVM CPU caps are initialized from the kernel boot CPU features this
makes the XSAVES feature also unavailable for KVM guests in this case, even
though they might want to decide on their own whether they are affected by
this errata.

Allow KVM guests to make such decision by setting the XSAVES KVM CPU
capability bit based on the actual CPU capability.

This fixes booting Hyper-V enabled Windows Server 2016 VMs with more than
one vCPU on Zen1/2 CPUs.

Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
 arch/x86/kvm/cpuid.c   | 16 ++++++++++++++++
 arch/x86/kvm/svm/svm.c |  5 ++++-
 2 files changed, 20 insertions(+), 1 deletion(-)
  

On Thu, Nov 23, 2023, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> 
> Since commit b0563468eeac ("x86/CPU/AMD: Disable XSAVES on AMD family 0x17")
> kernel unconditionally clears the XSAVES CPU feature bit on Zen1/2 CPUs.
> 
> Since KVM CPU caps are initialized from the kernel boot CPU features this
> makes the XSAVES feature also unavailable for KVM guests in this case, even
> though they might want to decide on their own whether they are affected by
> this errata.
> 
> Allow KVM guests to make such decision by setting the XSAVES KVM CPU
> capability bit based on the actual CPU capability

This is not generally safe, as the guest can make such a decision if and only if
the Family/Model/Stepping information is reasonably accurate.

> This fixes booting Hyper-V enabled Windows Server 2016 VMs with more than
> one vCPU on Zen1/2 CPUs.

How/why does lack of XSAVES break a multi-vCPU setup?  Is Windows blindly doing
XSAVES based on FMS?
  

On 27.11.2023 18:24, Sean Christopherson wrote:
> On Thu, Nov 23, 2023, Maciej S. Szmigiero wrote:
>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>>
>> Since commit b0563468eeac ("x86/CPU/AMD: Disable XSAVES on AMD family 0x17")
>> kernel unconditionally clears the XSAVES CPU feature bit on Zen1/2 CPUs.
>>
>> Since KVM CPU caps are initialized from the kernel boot CPU features this
>> makes the XSAVES feature also unavailable for KVM guests in this case, even
>> though they might want to decide on their own whether they are affected by
>> this errata.
>>
>> Allow KVM guests to make such decision by setting the XSAVES KVM CPU
>> capability bit based on the actual CPU capability
> 
> This is not generally safe, as the guest can make such a decision if and only if
> the Family/Model/Stepping information is reasonably accurate.

If one lies to the guest about the CPU it is running on then obviously
things may work non-optimally.

>> This fixes booting Hyper-V enabled Windows Server 2016 VMs with more than
>> one vCPU on Zen1/2 CPUs.
> 
> How/why does lack of XSAVES break a multi-vCPU setup?  Is Windows blindly doing
> XSAVES based on FMS?

The hypercall from L2 Windows to L1 Hyper-V asking to boot the first AP
returns HV_STATUS_CPUID_XSAVE_FEATURE_VALIDATION_ERROR.

It's apparently a "should never happen" scenario for Windows since it
crashes soon after.

That's why uniprocessor configurations aren't broken - the BSP
doesn't need to be specifically booted by the L2 guest.

Unfortunately, Windows Server 2016 mainstream support has ended in
Jan 2022 so it is only getting security updates.
And you can't really break into an OS that you can't even start.

Thanks,
Maciej
  

On Mon, Nov 27, 2023, Maciej S. Szmigiero wrote:
> On 27.11.2023 18:24, Sean Christopherson wrote:
> > On Thu, Nov 23, 2023, Maciej S. Szmigiero wrote:
> > > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> > > 
> > > Since commit b0563468eeac ("x86/CPU/AMD: Disable XSAVES on AMD family 0x17")
> > > kernel unconditionally clears the XSAVES CPU feature bit on Zen1/2 CPUs.
> > > 
> > > Since KVM CPU caps are initialized from the kernel boot CPU features this
> > > makes the XSAVES feature also unavailable for KVM guests in this case, even
> > > though they might want to decide on their own whether they are affected by
> > > this errata.
> > > 
> > > Allow KVM guests to make such decision by setting the XSAVES KVM CPU
> > > capability bit based on the actual CPU capability
> > 
> > This is not generally safe, as the guest can make such a decision if and only if
> > the Family/Model/Stepping information is reasonably accurate.
> 
> If one lies to the guest about the CPU it is running on then obviously
> things may work non-optimally.

But this isn't about running optimally, it's about functional correctness.  And
"lying" to the guest about F/M/S is extremely common.

> > > This fixes booting Hyper-V enabled Windows Server 2016 VMs with more than
> > > one vCPU on Zen1/2 CPUs.
> > 
> > How/why does lack of XSAVES break a multi-vCPU setup?  Is Windows blindly doing
> > XSAVES based on FMS?
> 
> The hypercall from L2 Windows to L1 Hyper-V asking to boot the first AP
> returns HV_STATUS_CPUID_XSAVE_FEATURE_VALIDATION_ERROR.

If it's just about CPUID enumeration, then userspace can simply stuff the XSAVES
feature flag.  This is not something that belongs in KVM, because this is safe if
and only if F/M/S is accurate and the guest is actually aware of the erratum (or
will not actually use XSAVES for other reasons), neither of which KVM can guarantee.
  

On 28.11.2023 17:48, Sean Christopherson wrote:
> On Mon, Nov 27, 2023, Maciej S. Szmigiero wrote:
>> On 27.11.2023 18:24, Sean Christopherson wrote:
>>> On Thu, Nov 23, 2023, Maciej S. Szmigiero wrote:
>>>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>>>>
>>>> Since commit b0563468eeac ("x86/CPU/AMD: Disable XSAVES on AMD family 0x17")
>>>> kernel unconditionally clears the XSAVES CPU feature bit on Zen1/2 CPUs.
>>>>
>>>> Since KVM CPU caps are initialized from the kernel boot CPU features this
>>>> makes the XSAVES feature also unavailable for KVM guests in this case, even
>>>> though they might want to decide on their own whether they are affected by
>>>> this errata.
>>>>
>>>> Allow KVM guests to make such decision by setting the XSAVES KVM CPU
>>>> capability bit based on the actual CPU capability
>>>
>>> This is not generally safe, as the guest can make such a decision if and only if
>>> the Family/Model/Stepping information is reasonably accurate.
>>
>> If one lies to the guest about the CPU it is running on then obviously
>> things may work non-optimally.
> 
> But this isn't about running optimally, it's about functional correctness.  And
> "lying" to the guest about F/M/S is extremely common.
> 
>>>> This fixes booting Hyper-V enabled Windows Server 2016 VMs with more than
>>>> one vCPU on Zen1/2 CPUs.
>>>
>>> How/why does lack of XSAVES break a multi-vCPU setup?  Is Windows blindly doing
>>> XSAVES based on FMS?
>>
>> The hypercall from L2 Windows to L1 Hyper-V asking to boot the first AP
>> returns HV_STATUS_CPUID_XSAVE_FEATURE_VALIDATION_ERROR.
> 
> If it's just about CPUID enumeration, then userspace can simply stuff the XSAVES
> feature flag.  This is not something that belongs in KVM, because this is safe if
> and only if F/M/S is accurate and the guest is actually aware of the erratum (or
> will not actually use XSAVES for other reasons), neither of which KVM can guarantee.

In other words, your suggestion is that QEMU (or other VMM) not KVM
should be the one setting the XSAVES CPUID bit back, correct?

I don't think this would work with the current KVM code since it seems
to make various decisions depending on presence of XSAVES bit in KVM
caps rather than the guest CPUID and on boot_cpu_has(XSAVES) - one of
such code blocks was even modified by this patch.

It even says in the comment above that code that it is not possible to
actually disable XSAVES without disabling all other variants on SVM so
this has to be enabled if CPU supports it to switch the XSS MSR at
guest entry/exit (in this case it looks harmless since Zen1/2
supposedly don't support any supervisor extended states).

So it looks like we would need changes to *both* KVM and QEMU to
restore the XSAVES support this way.

Thanks,
Maciej
  

On Tue, Nov 28, 2023, Maciej S. Szmigiero wrote:
> On 28.11.2023 17:48, Sean Christopherson wrote:
> > On Mon, Nov 27, 2023, Maciej S. Szmigiero wrote:
> > > On 27.11.2023 18:24, Sean Christopherson wrote:
> > > > On Thu, Nov 23, 2023, Maciej S. Szmigiero wrote:
> > > > > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> > > > > 
> > > > > Since commit b0563468eeac ("x86/CPU/AMD: Disable XSAVES on AMD family 0x17")
> > > > > kernel unconditionally clears the XSAVES CPU feature bit on Zen1/2 CPUs.
> > > > > 
> > > > > Since KVM CPU caps are initialized from the kernel boot CPU features this
> > > > > makes the XSAVES feature also unavailable for KVM guests in this case, even
> > > > > though they might want to decide on their own whether they are affected by
> > > > > this errata.
> > > > > 
> > > > > Allow KVM guests to make such decision by setting the XSAVES KVM CPU
> > > > > capability bit based on the actual CPU capability
> > > > 
> > > > This is not generally safe, as the guest can make such a decision if and only if
> > > > the Family/Model/Stepping information is reasonably accurate.
> > > 
> > > If one lies to the guest about the CPU it is running on then obviously
> > > things may work non-optimally.
> > 
> > But this isn't about running optimally, it's about functional correctness.  And
> > "lying" to the guest about F/M/S is extremely common.
> > 
> > > > > This fixes booting Hyper-V enabled Windows Server 2016 VMs with more than
> > > > > one vCPU on Zen1/2 CPUs.
> > > > 
> > > > How/why does lack of XSAVES break a multi-vCPU setup?  Is Windows blindly doing
> > > > XSAVES based on FMS?
> > > 
> > > The hypercall from L2 Windows to L1 Hyper-V asking to boot the first AP
> > > returns HV_STATUS_CPUID_XSAVE_FEATURE_VALIDATION_ERROR.
> > 
> > If it's just about CPUID enumeration, then userspace can simply stuff the XSAVES
> > feature flag.  This is not something that belongs in KVM, because this is safe if
> > and only if F/M/S is accurate and the guest is actually aware of the erratum (or
> > will not actually use XSAVES for other reasons), neither of which KVM can guarantee.
> 
> In other words, your suggestion is that QEMU (or other VMM) not KVM
> should be the one setting the XSAVES CPUID bit back, correct?
> 
> I don't think this would work with the current KVM code since it seems
> to make various decisions depending on presence of XSAVES bit in KVM
> caps rather than the guest CPUID and on boot_cpu_has(XSAVES) - one of
> such code blocks was even modified by this patch.
> 
> It even says in the comment above that code that it is not possible to
> actually disable XSAVES without disabling all other variants on SVM so
> this has to be enabled if CPU supports it to switch the XSS MSR at
> guest entry/exit (in this case it looks harmless since Zen1/2
> supposedly don't support any supervisor extended states).
> 
> So it looks like we would need changes to *both* KVM and QEMU to
> restore the XSAVES support this way.

I'm not suggesting we restore XSAVES support, I'm suggesting that _if_ someone
wants to hack their setup to let the guest use broken hardware, then they should
do that in userspace or in an a private kernel, not in upstream KVM.
  

On Mon, 2023-11-27 at 09:24 -0800, Sean Christopherson wrote:
> On Thu, Nov 23, 2023, Maciej S. Szmigiero wrote:
> > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> > 
> > Since commit b0563468eeac ("x86/CPU/AMD: Disable XSAVES on AMD family 0x17")
> > kernel unconditionally clears the XSAVES CPU feature bit on Zen1/2 CPUs.
> > 
> > Since KVM CPU caps are initialized from the kernel boot CPU features this
> > makes the XSAVES feature also unavailable for KVM guests in this case, even
> > though they might want to decide on their own whether they are affected by
> > this errata.
> > 
> > Allow KVM guests to make such decision by setting the XSAVES KVM CPU
> > capability bit based on the actual CPU capability
> 
> This is not generally safe, as the guest can make such a decision if and only if
> the Family/Model/Stepping information is reasonably accurate.

Another thing that really worries me is that the XSAVES errata is really nasty one - 
AFAIK it silently corrupts some registers.

Is it better to let a broken CPU boot a broken OS (OS which demands XSAVES blindly),
and let a silent data corruption happen than refuse to boot it completely?

I mean I understand that it is technically OS fault in this case (assuming that we
do provide it the correct CPU family info), but still this seems like the wrong thing to do.

I guess this is one of those few cases when it makes sense for the userspace to
override KVM's CPUID caps and force a feature - in this case at least that
won't be KVM's fault.


Best regards,
	Maxim Levitsky

> 
> > This fixes booting Hyper-V enabled Windows Server 2016 VMs with more than
> > one vCPU on Zen1/2 CPUs.
> 
> How/why does lack of XSAVES break a multi-vCPU setup?  Is Windows blindly doing
> XSAVES based on FMS?
>
  

On 30.11.2023 18:24, Maxim Levitsky wrote:
> On Mon, 2023-11-27 at 09:24 -0800, Sean Christopherson wrote:
>> On Thu, Nov 23, 2023, Maciej S. Szmigiero wrote:
>>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>>>
>>> Since commit b0563468eeac ("x86/CPU/AMD: Disable XSAVES on AMD family 0x17")
>>> kernel unconditionally clears the XSAVES CPU feature bit on Zen1/2 CPUs.
>>>
>>> Since KVM CPU caps are initialized from the kernel boot CPU features this
>>> makes the XSAVES feature also unavailable for KVM guests in this case, even
>>> though they might want to decide on their own whether they are affected by
>>> this errata.
>>>
>>> Allow KVM guests to make such decision by setting the XSAVES KVM CPU
>>> capability bit based on the actual CPU capability
>>
>> This is not generally safe, as the guest can make such a decision if and only if
>> the Family/Model/Stepping information is reasonably accurate.
> 
> Another thing that really worries me is that the XSAVES errata is really nasty one -
> AFAIK it silently corrupts some registers.

It's not unconditional state corruption, but corruption in specific set
of conditions, all of which have to be true for it to occur [1]:
* All XMM registers were restored to the initialization value by the most
recent XRSTORS instruction because the XSTATE_BV[SSE] bit was clear.
* The state save area for the XMM registers does not contain the
initialization state.
* The value in the XMM registers match the initialization value when the
XSAVES instruction is executed.
* The MXCSR register has been modified to a value different from the
initialization value since the most recent XRSTORS instruction.

According to [2] this issue was fixed in the microcode update released on
2022-08-09.
[2] also says it is not present anymore in (at least) version 0x08301055.

> Is it better to let a broken CPU boot a broken OS (OS which demands XSAVES blindly),
> and let a silent data corruption happen than refuse to boot it completely?

It is possible that, for example, Windows only uses safe subset of this
instruction or just verifies its presence but doesn't actually use it -
it's Hyper-V (L1) that throws this HV_STATUS_CPUID_XSAVE_FEATURE_VALIDATION_ERROR
but I presume it's Windows (L2) kernel which chooses which XSAVE-family
variant to actually use.

At least in the Linux guest case the guest won't use XSAVES anyway due
to this errata.

For other guests we also don't make situation any worse than on bare metal
- if they would use XSAVES anyway in KVM they would do it when running on
bare metal too.

> I mean I understand that it is technically OS fault in this case (assuming that we
> do provide it the correct CPU family info), but still this seems like the wrong thing to do.
> 
> I guess this is one of those few cases when it makes sense for the userspace to
> override KVM's CPUID caps and force a feature - in this case at least that
> won't be KVM's fault.

I am not against making the decision in QEMU instead of doing this in KVM,
but as I said to Sean it looks like this will still require some KVM
changes since KVM seems to make various decisions depending on presence
of XSAVES bit in KVM caps and boot_cpu_has(XSAVES) rather that exclusively
based on what VMM has set in CPUID.

That's why some KVM changes will be necessary even if the actual decision
logic will be in QEMU.

My point is to make this work out-of-the box for QEMU "-cpu host" and
similar CPU models that have support for XSAVES.

The reason for this is simple:
It's not like such Windows guests throw a big error screen saying
"Please enable XSAVES or disable XSAVEC for successful boot".

Instead, they simply hang at boot leaving the user wondering what could
be wrong.

Users can get very frustrated with situation like this since they don't
know what to do - on Intel side of things look for example how people
are unable to boot recent Windows versions (both server and client) on
Intel Core 12th gen or later in KVM and how people still try random
things to fix it [3] (it's Hyper-V being picky about extended topology
information in CPUID btw).

Cloud providers often know which guest type is going to be launched in
a VM so there's no problem adding a extra QEMU "-cpu" flag for
particular cloud VM.

But for individual users having to guess which magic flags are necessary
to make particular KVM guest work makes for miserable user experience.

I think that if particular guest would work on bare metal it should
work on "-cpu host" too - no tinkering should be required for such
basic functionality as being able to successfully finish booting.

> 
> Best regards,
> 	Maxim Levitsky

Thanks,
Maciej

[1]: https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/revision-guides/56323-PUB_1_01.pdf
[2]: https://google.github.io/security-research/pocs/cpus/errata/amd/1386/
[3]: https://lore.kernel.org/kvm/MN2PR12MB3023F67FF37889AB3E8885F2A0849@MN2PR12MB3023.namprd12.prod.outlook.com/
https://bugzilla.kernel.org/show_bug.cgi?id=217307
https://forums.unraid.net/topic/131838-windows-11-virtual-machine-platform-wsl2-boot-loop/
  

On Thu, Nov 30, 2023 at 2:00 PM Maciej S. Szmigiero
<mail@maciej.szmigiero.name> wrote:
> I think that if particular guest would work on bare metal it should
> work on "-cpu host" too - no tinkering should be required for such
> basic functionality as being able to successfully finish booting.

I disagree. Let's not focus on one particular erratum. If, for
whatever reason, the host kernel is booted with "noxsaves," I don't
think KVM should allow a guest to bypass that directive.
  

On 1.12.2023 00:57, Jim Mattson wrote:
> On Thu, Nov 30, 2023 at 2:00 PM Maciej S. Szmigiero
> <mail@maciej.szmigiero.name> wrote:
>> I think that if particular guest would work on bare metal it should
>> work on "-cpu host" too - no tinkering should be required for such
>> basic functionality as being able to successfully finish booting.
> 
> I disagree. Let's not focus on one particular erratum. If, for
> whatever reason, the host kernel is booted with "noxsaves," I don't
> think KVM should allow a guest to bypass that directive.

Good point, I agree that if user explicitly disabled XSAVES on
the host via this parameter then it should remain disabled for
the whole host kernel, including KVM.

This could be achieved by either adding special "noxsaves" flag
or by setting X86_BUG_XSAVES_AVOID instead of clearing
X86_FEATURE_XSAVES on these CPUs.

Then the core kernel XSAVES code would check for lack of
X86_BUG_XSAVES_AVOID (in addition to checking for
presence of X86_FEATURE_XSAVES) while KVM would keep using
only X86_FEATURE_XSAVES.

Thanks,
Maciej
  

On Fri, Dec 1, 2023 at 5:05 PM Maciej S. Szmigiero
<mail@maciej.szmigiero.name> wrote:
>
> On 1.12.2023 00:57, Jim Mattson wrote:
> > On Thu, Nov 30, 2023 at 2:00 PM Maciej S. Szmigiero
> > <mail@maciej.szmigiero.name> wrote:
> >> I think that if particular guest would work on bare metal it should
> >> work on "-cpu host" too - no tinkering should be required for such
> >> basic functionality as being able to successfully finish booting.
> >
> > I disagree. Let's not focus on one particular erratum. If, for
> > whatever reason, the host kernel is booted with "noxsaves," I don't
> > think KVM should allow a guest to bypass that directive.
>
> This could be achieved by either adding special "noxsaves" flag
> or by setting X86_BUG_XSAVES_AVOID instead of clearing
> X86_FEATURE_XSAVES on these CPUs.
>
> Then the core kernel XSAVES code would check for lack of
> X86_BUG_XSAVES_AVOID (in addition to checking for
> presence of X86_FEATURE_XSAVES) while KVM would keep using
> only X86_FEATURE_XSAVES.

This is feasible, on the other hand the erratum is pretty bad. Since
the workaround is easy (just disable XSAVEC; and maybe XGETBV1 as
well?), you could just do a printk_ratelimited() on the write to
HV_X64_MSR_GUEST_OS_ID, in particular if:

1) guest CPUID has XSAVEC and SVM

2) host CPUID does not have XSAVES

3) guest OS id indicates Windows Server 10.x (which is 2016 to 2022),
which should be 0x0001040A00??????.

Paolo