[PATCH] target/i386/hax: Add XCR0 support

Wang, Wenchao posted 1 patch 1 year, 4 months ago
Failed in applying to current master (apply log)
1 file changed, 2 insertions(+)
[PATCH] target/i386/hax: Add XCR0 support
Posted by Wang, Wenchao 1 year, 4 months ago
Hi, Thomas,

As HAXM v7.8.0 is released and it added XCR0 support, it needs this patch to add corresponding support into HAX user space of QEMU. I have pushed this merge request before and Philippe has reviewed it and he thought the change is correct. If no one else raises any other opinion, could you help to merge this patch for HAX? We have verified the patched QEMU and it can launch all guest OSes. Thanks for your support.


Best Regards,
Wenchao


---------------------------------



From b1789f2523d06798b8883664bfa9a9df797bfccf Mon Sep 17 00:00:00 2001

From: Wenchao Wang <wenchao.wang@intel.com<mailto:wenchao.wang@intel.com>>

Date: Fri, 25 Nov 2022 18:37:34 +0800

Subject: [PATCH] target/i386/hax: Add XCR0 support



Introduce extended control register XCR0 to support XSAVE feature set.



Note: This change requires at least HAXM v7.8.0 to support.



Reviewed-by: Hang Yuan <hang.yuan@intel.com<mailto:hang.yuan@intel.com>>

Signed-off-by: Wenchao Wang <wenchao.wang@intel.com<mailto:wenchao.wang@intel.com>>

---

target/i386/hax/hax-interface.h | 2 ++

1 file changed, 2 insertions(+)



diff --git a/target/i386/hax/hax-interface.h b/target/i386/hax/hax-interface.h index 537ae084e9..1d13bb2380 100644

--- a/target/i386/hax/hax-interface.h

+++ b/target/i386/hax/hax-interface.h

@@ -201,6 +201,8 @@ struct vcpu_state_t {

     uint64_t _cr3;

     uint64_t _cr4;



+    uint64_t _xcr0;

+

     uint64_t _dr0;

     uint64_t _dr1;

     uint64_t _dr2;

--

2.17.1

Re: [PATCH] target/i386/hax: Add XCR0 support
Posted by Thomas Huth 1 year, 4 months ago
On 14/12/2022 10.15, Wang, Wenchao wrote:
> Hi, Thomas,
> 
> As HAXM v7.8.0 is released and it added XCR0 support, it needs this patch to 
> add corresponding support into HAX user space of QEMU. I have pushed this 
> merge request before and Philippe has reviewed it and he thought the change 
> is correct. If no one else raises any other opinion, could you help to merge 
> this patch for HAX? 

  Hi,

sorry, I don't have a stake in the target/i386 code ... but you're listed as 
maintainer for the hax/ folder, so if no other x86 maintainer picks this up, 
I think you could send a pull request for this patch on your own. See:

  https://www.qemu.org/docs/master/devel/submitting-a-pull-request.html

  HTH,
   Thomas


> 
> ---------------------------------
> 
>  From b1789f2523d06798b8883664bfa9a9df797bfccf Mon Sep 17 00:00:00 2001
> 
> From: Wenchao Wang <wenchao.wang@intel.com <mailto:wenchao.wang@intel.com>>
> 
> Date: Fri, 25 Nov 2022 18:37:34 +0800
> 
> Subject: [PATCH] target/i386/hax: Add XCR0 support
> 
> Introduce extended control register XCR0 to support XSAVE feature set.
> 
> Note: This change requires at least HAXM v7.8.0 to support.
> 
> Reviewed-by: Hang Yuan <hang.yuan@intel.com <mailto:hang.yuan@intel.com>>
> 
> Signed-off-by: Wenchao Wang <wenchao.wang@intel.com 
> <mailto:wenchao.wang@intel.com>>
> 
> ---
> 
> target/i386/hax/hax-interface.h | 2 ++
> 
> 1 file changed, 2 insertions(+)
> 
> diff --git a/target/i386/hax/hax-interface.h 
> b/target/i386/hax/hax-interface.h index 537ae084e9..1d13bb2380 100644
> 
> --- a/target/i386/hax/hax-interface.h
> 
> +++ b/target/i386/hax/hax-interface.h
> 
> @@ -201,6 +201,8 @@ struct vcpu_state_t {
> 
>       uint64_t _cr3;
> 
>       uint64_t _cr4;
> 
> +    uint64_t _xcr0;
> 
> +
> 
>       uint64_t _dr0;
> 
>       uint64_t _dr1;
> 
>       uint64_t _dr2;
> 
> --
> 
> 2.17.1
> 


RE: [PATCH] target/i386/hax: Add XCR0 support
Posted by Wang, Wenchao 1 year, 4 months ago
Hi, Thomas,

Thanks for your reply. I have attempted to follow you suggestions but it always failed on tagging a GPG-signed tag before submitting the pull request. I have used GPG 2.2.4 to generate a RSA4096 GPG secret key and pasted the public key on GitHub successfully.

$ git tag -s pull-request-hax -m 'target/i386/hax: Add XCR0 support'
error: gpg failed to sign the data
error: unable to sign the tag

Meanwhile, could @Paolo Bonzini or @Stefan Hajnoczi help to pick the patch up as there is only one-line change for HAX and we have verified it for all guest launching? Thanks a lot.


Best Regards,
Wenchao

-----Original Message-----
From: Thomas Huth <thuth@redhat.com> 
Sent: Wednesday, December 14, 2022 17:39
To: Wang, Wenchao <wenchao.wang@intel.com>
Cc: qemu-devel@nongnu.org; Paolo Bonzini <pbonzini@redhat.com>; Peter Maydell <peter.maydell@linaro.org>; Stefan Hajnoczi <stefanha@redhat.com>; Richard Henderson <richard.henderson@linaro.org>
Subject: Re: [PATCH] target/i386/hax: Add XCR0 support

On 14/12/2022 10.15, Wang, Wenchao wrote:
> Hi, Thomas,
> 
> As HAXM v7.8.0 is released and it added XCR0 support, it needs this 
> patch to add corresponding support into HAX user space of QEMU. I have 
> pushed this merge request before and Philippe has reviewed it and he 
> thought the change is correct. If no one else raises any other 
> opinion, could you help to merge this patch for HAX?

  Hi,

sorry, I don't have a stake in the target/i386 code ... but you're listed as maintainer for the hax/ folder, so if no other x86 maintainer picks this up, I think you could send a pull request for this patch on your own. See:

  https://www.qemu.org/docs/master/devel/submitting-a-pull-request.html

  HTH,
   Thomas


> 
> ---------------------------------
> 
>  From b1789f2523d06798b8883664bfa9a9df797bfccf Mon Sep 17 00:00:00 
> 2001
> 
> From: Wenchao Wang <wenchao.wang@intel.com 
> <mailto:wenchao.wang@intel.com>>
> 
> Date: Fri, 25 Nov 2022 18:37:34 +0800
> 
> Subject: [PATCH] target/i386/hax: Add XCR0 support
> 
> Introduce extended control register XCR0 to support XSAVE feature set.
> 
> Note: This change requires at least HAXM v7.8.0 to support.
> 
> Reviewed-by: Hang Yuan <hang.yuan@intel.com 
> <mailto:hang.yuan@intel.com>>
> 
> Signed-off-by: Wenchao Wang <wenchao.wang@intel.com 
> <mailto:wenchao.wang@intel.com>>
> 
> ---
> 
> target/i386/hax/hax-interface.h | 2 ++
> 
> 1 file changed, 2 insertions(+)
> 
> diff --git a/target/i386/hax/hax-interface.h 
> b/target/i386/hax/hax-interface.h index 537ae084e9..1d13bb2380 100644
> 
> --- a/target/i386/hax/hax-interface.h
> 
> +++ b/target/i386/hax/hax-interface.h
> 
> @@ -201,6 +201,8 @@ struct vcpu_state_t {
> 
>       uint64_t _cr3;
> 
>       uint64_t _cr4;
> 
> +    uint64_t _xcr0;
> 
> +
> 
>       uint64_t _dr0;
> 
>       uint64_t _dr1;
> 
>       uint64_t _dr2;
> 
> --
> 
> 2.17.1
> 

Re: [PATCH] target/i386/hax: Add XCR0 support
Posted by Peter Maydell 1 year, 4 months ago
On Thu, 15 Dec 2022 at 09:45, Wang, Wenchao <wenchao.wang@intel.com> wrote:
>
> Hi, Thomas,
>
> Thanks for your reply. I have attempted to follow you suggestions but it always failed on tagging a GPG-signed tag before submitting the pull request. I have used GPG 2.2.4 to generate a RSA4096 GPG secret key and pasted the public key on GitHub successfully.
>
> $ git tag -s pull-request-hax -m 'target/i386/hax: Add XCR0 support'
> error: gpg failed to sign the data
> error: unable to sign the tag
>
> Meanwhile, could @Paolo Bonzini or @Stefan Hajnoczi help to pick the patch up as there is only one-line change for HAX and we have verified it for all guest launching? Thanks a lot.

Yes, please. For a single trivial patch I strongly prefer
that some existing (in this case x86) maintainer takes it in
their pullreq, rather than my having to deal with a
pullreq submission from a new-to-the-process person.
(It's extra work to check submissions from new people,
which is fine if they're going to be doing them a lot
in future, but for a one-off it's a waste of their time
and mine.)

thanks
-- PMM
RE: [PATCH] target/i386/hax: Add XCR0 support
Posted by Wang, Wenchao 1 year, 4 months ago
Thanks for Peter's reply. Since it is better to pull the patch by x86 maintainers, could any maintainer help to merge it, @Paolo Bonzini or @Stefan Hajnoczi? The original patch is attached below. Thanks a lot.


Best Regards,
Wenchao

---------------------------------

From b1789f2523d06798b8883664bfa9a9df797bfccf Mon Sep 17 00:00:00 2001
From: Wenchao Wang <wenchao.wang@intel.com>
Date: Fri, 25 Nov 2022 18:37:34 +0800
Subject: [PATCH] target/i386/hax: Add XCR0 support

Introduce extended control register XCR0 to support XSAVE feature set.

Note: This change requires at least HAXM v7.8.0 to support.

Reviewed-by: Hang Yuan <hang.yuan@intel.com>
Signed-off-by: Wenchao Wang <wenchao.wang@intel.com>
---
target/i386/hax/hax-interface.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/target/i386/hax/hax-interface.h b/target/i386/hax/hax-interface.h index 537ae084e9..1d13bb2380 100644
--- a/target/i386/hax/hax-interface.h
+++ b/target/i386/hax/hax-interface.h
@@ -201,6 +201,8 @@ struct vcpu_state_t {
     uint64_t _cr3;
     uint64_t _cr4;

+    uint64_t _xcr0;
+
     uint64_t _dr0;
     uint64_t _dr1;
     uint64_t _dr2;
--
2.17.1


-----Original Message-----
From: Peter Maydell <peter.maydell@linaro.org> 
Sent: Thursday, December 15, 2022 18:27
To: Wang, Wenchao <wenchao.wang@intel.com>
Cc: Thomas Huth <thuth@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>; qemu-devel@nongnu.org; Richard Henderson <richard.henderson@linaro.org>
Subject: Re: [PATCH] target/i386/hax: Add XCR0 support

On Thu, 15 Dec 2022 at 09:45, Wang, Wenchao <wenchao.wang@intel.com> wrote:
>
> Hi, Thomas,
>
> Thanks for your reply. I have attempted to follow you suggestions but it always failed on tagging a GPG-signed tag before submitting the pull request. I have used GPG 2.2.4 to generate a RSA4096 GPG secret key and pasted the public key on GitHub successfully.
>
> $ git tag -s pull-request-hax -m 'target/i386/hax: Add XCR0 support'
> error: gpg failed to sign the data
> error: unable to sign the tag
>
> Meanwhile, could @Paolo Bonzini or @Stefan Hajnoczi help to pick the patch up as there is only one-line change for HAX and we have verified it for all guest launching? Thanks a lot.

Yes, please. For a single trivial patch I strongly prefer that some existing (in this case x86) maintainer takes it in their pullreq, rather than my having to deal with a pullreq submission from a new-to-the-process person.
(It's extra work to check submissions from new people, which is fine if they're going to be doing them a lot in future, but for a one-off it's a waste of their time and mine.)

thanks
-- PMM