[PATCH] target/i386: Clear xsave pkru bit when KVM XCR0 not support

Yuchen posted 1 patch 11 months, 3 weeks ago
Failed in applying to current master (apply log)
target/i386/xsave_helper.c | 8 ++++++++
1 file changed, 8 insertions(+)
[PATCH] target/i386: Clear xsave pkru bit when KVM XCR0 not support
Posted by Yuchen 11 months, 3 weeks ago
Migrating guest from Intel new CPU (as Gold 6230) to old CPU (as
E5-2650 v4) will pause on the destination host. Because old CPU
not support xsave pkru feature, and KVM KVM_SET_XSAVE ioctl
return EINVAL.

This kernel commit introduces the problem:
ea4d6938d4c0 x86/fpu: Replace KVMs home brewed FPU copy from user

Signed-off-by: YuChen <yu.chen@h3c.com>
---
target/i386/xsave_helper.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/target/i386/xsave_helper.c b/target/i386/xsave_helper.c
index 996e9f3bfe..64e2b969fe 100644
--- a/target/i386/xsave_helper.c
+++ b/target/i386/xsave_helper.c
@@ -6,6 +6,8 @@
 #include "cpu.h"
+static bool has_xsave_pkru;
+
void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen)
{
     CPUX86State *env = &cpu->env;
@@ -47,6 +49,9 @@ void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen)
         stq_p(xmm + 8, env->xmm_regs[i].ZMM_Q(1));
     }
+    if (!has_xsave_pkru) {
+        env->xstate_bv &= ~XSTATE_PKRU_MASK;
+    }
     header->xstate_bv = env->xstate_bv;
     e = &x86_ext_save_areas[XSTATE_YMM_BIT];
@@ -181,6 +186,9 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu, const void *buf, uint32_t buflen)
         env->xmm_regs[i].ZMM_Q(1) = ldq_p(xmm + 8);
     }
+    if (xsave->header.xstate_bv & XSTATE_PKRU_MASK) {
+        has_xsave_pkru = true;
+    }
     env->xstate_bv = header->xstate_bv;
     e = &x86_ext_save_areas[XSTATE_YMM_BIT];
--
2.34.1
-------------------------------------------------------------------------------------------------------------------------------------
?????????????????????????????????
????????????????????????????????????????
????????????????????????????????????????
???
This e-mail and its attachments contain confidential information from New H3C, which is
intended only for the person or entity whose address is listed above. Any use of the
information contained herein in any way (including, but not limited to, total or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender
by phone or email immediately and delete it!
Re: [PATCH] target/i386: Clear xsave pkru bit when KVM XCR0 not support
Posted by Dongli Zhang 11 months, 3 weeks ago
Hi Yuchen,

On 5/17/23 03:55, Yuchen wrote:
> Migrating guest from Intel new CPU (as Gold 6230) to old CPU (as
> E5-2650 v4) will pause on the destination host. Because old CPU
> not support xsave pkru feature, and KVM KVM_SET_XSAVE ioctl
> return EINVAL.
> 
> This kernel commit introduces the problem:
> ea4d6938d4c0 x86/fpu: Replace KVMs home brewed FPU copy from user

This kernel commit issue should be resolved by the below kernel commit.

x86/kvm/fpu: Limit guest user_xfeatures to supported bits of XCR0

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ad856280ddea3401e1f5060ef20e6de9f6122c76

Since the old target server does not support pkru, I assume the VM's cpu type
should not support pkru. Therefore, the pkru should never be migrated away from
source server.

Dongli Zhang

> 
> Signed-off-by: YuChen <yu.chen@h3c.com>
> ---
> target/i386/xsave_helper.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
> 
> diff --git a/target/i386/xsave_helper.c b/target/i386/xsave_helper.c
> index 996e9f3bfe..64e2b969fe 100644
> --- a/target/i386/xsave_helper.c
> +++ b/target/i386/xsave_helper.c
> @@ -6,6 +6,8 @@
>  #include "cpu.h"
> +static bool has_xsave_pkru;
> +
> void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen)
> {
>      CPUX86State *env = &cpu->env;
> @@ -47,6 +49,9 @@ void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen)
>          stq_p(xmm + 8, env->xmm_regs[i].ZMM_Q(1));
>      }
> +    if (!has_xsave_pkru) {
> +        env->xstate_bv &= ~XSTATE_PKRU_MASK;
> +    }
>      header->xstate_bv = env->xstate_bv;
>      e = &x86_ext_save_areas[XSTATE_YMM_BIT];
> @@ -181,6 +186,9 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu, const void *buf, uint32_t buflen)
>          env->xmm_regs[i].ZMM_Q(1) = ldq_p(xmm + 8);
>      }
> +    if (xsave->header.xstate_bv & XSTATE_PKRU_MASK) {
> +        has_xsave_pkru = true;
> +    }
>      env->xstate_bv = header->xstate_bv;
>      e = &x86_ext_save_areas[XSTATE_YMM_BIT];
> --
> 2.34.1
> -------------------------------------------------------------------------------------------------------------------------------------
> ?????????????????????????????????
> ????????????????????????????????????????
> ????????????????????????????????????????
> ???
> This e-mail and its attachments contain confidential information from New H3C, which is
> intended only for the person or entity whose address is listed above. Any use of the
> information contained herein in any way (including, but not limited to, total or partial
> disclosure, reproduction, or dissemination) by persons other than the intended
> recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender
> by phone or email immediately and delete it!
>
Re: [PATCH] target/i386: Clear xsave pkru bit when KVM XCR0 not support
Posted by Paolo Bonzini 11 months, 3 weeks ago
On 5/17/23 12:55, Yuchen wrote:
> Migrating guest from Intel new CPU (as Gold 6230) to old CPU (as
> E5-2650 v4) will pause on the destination host. Because old CPU
> not support xsave pkru feature, and KVM KVM_SET_XSAVE ioctl
> return EINVAL.
> 
> This kernel commit introduces the problem:
> 
> ea4d6938d4c0 x86/fpu: Replace KVMs home brewed FPU copy from user
> 
> Signed-off-by: YuChen <yu.chen@h3c.com>

Would this work instead?

diff --git a/target/i386/xsave_helper.c b/target/i386/xsave_helper.c
index 996e9f3bfef5..d3e5edad2ecd 100644
--- a/target/i386/xsave_helper.c
+++ b/target/i386/xsave_helper.c
@@ -47,7 +47,7 @@ void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen)
          stq_p(xmm + 8, env->xmm_regs[i].ZMM_Q(1));
      }
  
-    header->xstate_bv = env->xstate_bv;
+    header->xstate_bv = env->xstate_bv & x86_cpu_xsave_xcr0_components(cpu);
  
      e = &x86_ext_save_areas[XSTATE_YMM_BIT];
      if (e->size && e->offset) {

Paolo