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

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

[Yuchen]

[-- Attachment #1: Type: text/plain, Size: 2221 bytes --]

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!

[-- Attachment #2: Type: text/html, Size: 12205 bytes --]

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

[Paolo Bonzini]

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

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

[Dongli Zhang]

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

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

[Yuchen]

Paolo, thanks, 
The kernel patch can solve this problem. But it is difficult to upgrade the kernel in some production environments, 
and upgrading qemu is easy. This patch is just to sync qemu with kvm XSAVE features, no negative impact. 
At the same time, it increases the compatibility of qemu to kernel and improves the robustness of qemu.

> -----邮件原件-----
> 发件人: Paolo Bonzini <pbonzini@redhat.com>
> 发送时间: 2023年5月20日 1:37
> 收件人: yuchen (Cloud) <yu.chen@h3c.com>
> 主题: Re: 回复: [PATCH] target/i386: Clear xsave pkru bit when KVM
> XCR0 not support
> 
> On 5/18/23 14:37, Yuchen wrote:
> > Yes, because x86_cpu_xsave_all_areas() can get the correct XSAVE
> features.
> 
> If you tested it, I can post the patch as a workaround.  However, the
> kernel bug has been fixed too.
> 
> Paolo
> 
> >> -----邮件原件-----
> >> 发件人: Paolo Bonzini <pbonzini@redhat.com>
> >> 发送时间: 2023年5月18日 1:08
> >> 收件人: yuchen (Cloud) <yu.chen@h3c.com>;
> qemu-devel@nongnu.org
> >> 抄送: rth@twiddle.net; ehabkost@redhat.com; chengchiwen (Cloud)
> >> <chengchiwen@h3c.com>
> >> 主题: Re: [PATCH] target/i386: Clear xsave pkru bit when KVM XCR0
> not
> >> support
> >>
> >> 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
> >
> > ----------------------------------------------------------------------
> > ---------------------------------------------------------------
> > 本邮件及其附件含有新华三集团的保密信息，仅限于发送给上面地址
> 中列出
> > 的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部
> 或部分地泄露、复制、
> > 或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮
> 件通知发件人并删除本
> > 邮件！
> > 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!

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

[Yuchen]

Paolo,
I debugged and found, x86_cpu_xsave_xcr0_components() retuen 0, although it can solve problem, but XCR0 bit 0 also is 0, 
this is not correct. Because Intel manual Requirements : " XCR0.X87 (bit 0): This bit 0 must be 1. An attempt to write 0 to this bit causes a #GP exception. " (2.6 EXTENDED CONTROL REGISTERS (INCLUDING XCR0)).

> -----邮件原件-----
> 发件人: yuchen (Cloud)
> 发送时间: 2023年5月20日 9:58
> 收件人: 'Paolo Bonzini' <pbonzini@redhat.com>;
> 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>
> 抄送: 'rth@twiddle.net' <rth@twiddle.net>; 'ehabkost@redhat.com'
> <ehabkost@redhat.com>; 'Dongli Zhang' <dongli.zhang@oracle.com>
> 主题: 回复: 回复: [PATCH] target/i386: Clear xsave pkru bit when KVM
> XCR0 not support
> 
> Paolo, thanks,
> The kernel patch can solve this problem. But it is difficult to upgrade the
> kernel in some production environments, and upgrading qemu is easy.
> This patch is just to sync qemu with kvm XSAVE features, no negative
> impact.
> At the same time, it increases the compatibility of qemu to kernel and
> improves the robustness of qemu.
> 
> > -----邮件原件-----
> > 发件人: Paolo Bonzini <pbonzini@redhat.com>
> > 发送时间: 2023年5月20日 1:37
> > 收件人: yuchen (Cloud) <yu.chen@h3c.com>
> > 主题: Re: 回复: [PATCH] target/i386: Clear xsave pkru bit when KVM
> > XCR0 not support
> >
> > On 5/18/23 14:37, Yuchen wrote:
> > > Yes, because x86_cpu_xsave_all_areas() can get the correct XSAVE
> > features.
> >
> > If you tested it, I can post the patch as a workaround.  However, the
> > kernel bug has been fixed too.
> >
> > Paolo
> >
> > >> -----邮件原件-----
> > >> 发件人: Paolo Bonzini <pbonzini@redhat.com>
> > >> 发送时间: 2023年5月18日 1:08
> > >> 收件人: yuchen (Cloud) <yu.chen@h3c.com>;
> > qemu-devel@nongnu.org
> > >> 抄送: rth@twiddle.net; ehabkost@redhat.com; chengchiwen
> (Cloud)
> > >> <chengchiwen@h3c.com>
> > >> 主题: Re: [PATCH] target/i386: Clear xsave pkru bit when KVM XCR0
> > not
> > >> support
> > >>
> > >> 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
> > >
> > > --------------------------------------------------------------------
> > > --
> > > ---------------------------------------------------------------
> > > 本邮件及其附件含有新华三集团的保密信息，仅限于发送给上面地
> 址
> > 中列出
> > > 的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全
> 部
> > 或部分地泄露、复制、
> > > 或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或
> 邮
> > 件通知发件人并删除本
> > > 邮件！
> > > 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!