KVM: x86: Pass additional FPU bits to the guest

KVM: x86: Pass additional FPU bits to the guest

[Sebastian Andrzej Siewior]

While testing my FPU patches on AMD's ZEN platform I noticed that the
XSAVES feature flag was never used in the guest while it was present in
the host. Patch #1 seemed to work but I holded on to it because I could
explain why the guest did report ´fxsave_leak' while the host did not.
It turns out that I need #2 _and_ tell qemu not to mask this information
away.

Sebastian

[PATCH 1/2] KVM: x86: svm: Pass XSAVES to guest if available on host

[Sebastian Andrzej Siewior]

In commit
   55412b2eda2b7 ("kvm: x86: Add kvm_x86_ops hook that enables XSAVES for guest")

XSAVES was enabled on VMX with a few additional tweaks and was always
disabled on SVM. Before ZEN XSAVES was not available so it made no
difference. With Zen it is possible to expose it to the guest if it is
available on the host.
I didn't find anything close to VMX's "VM-Execution Controls" and
exposing this flag based on the CPUID flags cause no harm so far.

Expose the XSAVES flag to the guest if the host supports it.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/kvm/svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index e0368076a1ef9..3878eb766fa39 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -5992,7 +5992,7 @@ static bool svm_mpx_supported(void)
 
 static bool svm_xsaves_supported(void)
 {
-	return false;
+	return boot_cpu_has(X86_FEATURE_XSAVES);
 }
 
 static bool svm_umip_emulated(void)
-- 
2.23.0

Re: [PATCH 1/2] KVM: x86: svm: Pass XSAVES to guest if available on host

[Jim Mattson]

On Wed, Sep 25, 2019 at 2:37 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> In commit
>    55412b2eda2b7 ("kvm: x86: Add kvm_x86_ops hook that enables XSAVES for guest")
>
> XSAVES was enabled on VMX with a few additional tweaks and was always
> disabled on SVM. Before ZEN XSAVES was not available so it made no
> difference. With Zen it is possible to expose it to the guest if it is
> available on the host.
> I didn't find anything close to VMX's "VM-Execution Controls" and
> exposing this flag based on the CPUID flags cause no harm so far.
>
> Expose the XSAVES flag to the guest if the host supports it.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/x86/kvm/svm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index e0368076a1ef9..3878eb766fa39 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -5992,7 +5992,7 @@ static bool svm_mpx_supported(void)
>
>  static bool svm_xsaves_supported(void)
>  {
> -       return false;
> +       return boot_cpu_has(X86_FEATURE_XSAVES);
>  }
>
>  static bool svm_umip_emulated(void)
> --
> 2.23.0

This is inadequate. Please read the existing thread, "[Patch] KVM:
SVM: Fix svm_xsaves_supported." Aaron Lewis is working on completing
this as we speak.

[PATCH 2/2] KVM: x86: Expose CLZERO and XSAVEERPTR to the guest

[Sebastian Andrzej Siewior]

I was surprised to see that the guest reported `fxsave_leak' while the
host did not. After digging deeper I noticed that the bits are simply
masked out during enumeration.
The XSAVEERPTR feature is actually a bug fix on AMD which means the
kernel can disable a workaround.
While here, I've seen that CLZERO is also masked out. This opcode is
unprivilged so exposing it to the guest should not make any difference.

Pass CLZERO and XSAVEERPTR to the guest if available on the host.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/kvm/cpuid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 22c2720cd948e..0ae9194d0f4d2 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -473,6 +473,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
 
 	/* cpuid 0x80000008.ebx */
 	const u32 kvm_cpuid_8000_0008_ebx_x86_features =
+		F(CLZERO) | F(XSAVEERPTR) |
 		F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
 		F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON);
 
-- 
2.23.0

Re: [PATCH 2/2] KVM: x86: Expose CLZERO and XSAVEERPTR to the guest

[Jim Mattson]

On Wed, Sep 25, 2019 at 2:37 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> I was surprised to see that the guest reported `fxsave_leak' while the
> host did not. After digging deeper I noticed that the bits are simply
> masked out during enumeration.
> The XSAVEERPTR feature is actually a bug fix on AMD which means the
> kernel can disable a workaround.
> While here, I've seen that CLZERO is also masked out. This opcode is
> unprivilged so exposing it to the guest should not make any difference.
>
> Pass CLZERO and XSAVEERPTR to the guest if available on the host.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/x86/kvm/cpuid.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 22c2720cd948e..0ae9194d0f4d2 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -473,6 +473,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
>
>         /* cpuid 0x80000008.ebx */
>         const u32 kvm_cpuid_8000_0008_ebx_x86_features =
> +               F(CLZERO) | F(XSAVEERPTR) |
>                 F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
>                 F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON);
>
> --
> 2.23.0

Didn't someone just post "[PATCH] kvm: x86: Enumerate support for
CLZERO instruction" yesterday? :-)

Re: [PATCH 2/2] KVM: x86: Expose CLZERO and XSAVEERPTR to the guest

[Paolo Bonzini]

On 25/09/19 23:37, Sebastian Andrzej Siewior wrote:
> I was surprised to see that the guest reported `fxsave_leak' while the
> host did not. After digging deeper I noticed that the bits are simply
> masked out during enumeration.
> The XSAVEERPTR feature is actually a bug fix on AMD which means the
> kernel can disable a workaround.
> While here, I've seen that CLZERO is also masked out. This opcode is
> unprivilged so exposing it to the guest should not make any difference.
> 
> Pass CLZERO and XSAVEERPTR to the guest if available on the host.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/x86/kvm/cpuid.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 22c2720cd948e..0ae9194d0f4d2 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -473,6 +473,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
>  
>  	/* cpuid 0x80000008.ebx */
>  	const u32 kvm_cpuid_8000_0008_ebx_x86_features =
> +		F(CLZERO) | F(XSAVEERPTR) |
>  		F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
>  		F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON);
>  
> 

Applied (on top of Jim's CLZERO patch, so removing the CLZERO reference
in the commit message).

Paolo