[PATCH v3 09/20] i386/cpu: Fix supervisor xstate initialization

Zhao Liu posted 20 patches 3 days, 4 hours ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Zhao Liu <zhao1.liu@intel.com>, Marcelo Tosatti <mtosatti@redhat.com>
[PATCH v3 09/20] i386/cpu: Fix supervisor xstate initialization
Posted by Zhao Liu 3 days, 4 hours ago
From: Chao Gao <chao.gao@intel.com>

Arch lbr is a supervisor xstate, but its area is not covered in
x86_cpu_init_xsave().

Fix it by checking supported xss bitmap.

In addition, drop the (uint64_t) type casts for supported_xcr0 since
x86_cpu_get_supported_feature_word() returns uint64_t so that the cast
is not needed. Then ensure line length is within 90 characters.

Tested-by: Farrah Chen <farrah.chen@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Co-developed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 5cd335bb5574..1917376dbea9 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -9707,20 +9707,23 @@ static void x86_cpu_post_initfn(Object *obj)
 static void x86_cpu_init_xsave(void)
 {
     static bool first = true;
-    uint64_t supported_xcr0;
+    uint64_t supported_xcr0, supported_xss;
     int i;
 
     if (first) {
         first = false;
 
         supported_xcr0 =
-            ((uint64_t) x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_HI) << 32) |
+            x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_HI) |
             x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_LO);
+        supported_xss =
+            x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XSS_HI) << 32 |
+            x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XSS_LO);
 
         for (i = XSTATE_SSE_BIT + 1; i < XSAVE_STATE_AREA_COUNT; i++) {
             ExtSaveArea *esa = &x86_ext_save_areas[i];
 
-            if (!(supported_xcr0 & (1 << i))) {
+            if (!((supported_xcr0 | supported_xss) & (1 << i))) {
                 esa->size = 0;
             }
         }
-- 
2.34.1
Re: [PATCH v3 09/20] i386/cpu: Fix supervisor xstate initialization
Posted by Xiaoyao Li 3 hours ago
On 10/24/2025 2:56 PM, Zhao Liu wrote:
> From: Chao Gao <chao.gao@intel.com>
> 
> Arch lbr is a supervisor xstate, but its area is not covered in
> x86_cpu_init_xsave().
> 
> Fix it by checking supported xss bitmap.
> 
> In addition, drop the (uint64_t) type casts for supported_xcr0 since
> x86_cpu_get_supported_feature_word() returns uint64_t so that the cast
> is not needed. Then ensure line length is within 90 characters.
> 
> Tested-by: Farrah Chen <farrah.chen@intel.com>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Co-developed-by: Zhao Liu <zhao1.liu@intel.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   target/i386/cpu.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 5cd335bb5574..1917376dbea9 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -9707,20 +9707,23 @@ static void x86_cpu_post_initfn(Object *obj)
>   static void x86_cpu_init_xsave(void)
>   {
>       static bool first = true;
> -    uint64_t supported_xcr0;
> +    uint64_t supported_xcr0, supported_xss;
>       int i;
>   
>       if (first) {
>           first = false;
>   
>           supported_xcr0 =
> -            ((uint64_t) x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_HI) << 32) |
> +            x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_HI) |

missing the "<< 32" here,

with it fixed,

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

>               x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_LO);
> +        supported_xss =
> +            x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XSS_HI) << 32 |
> +            x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XSS_LO);
>   
>           for (i = XSTATE_SSE_BIT + 1; i < XSAVE_STATE_AREA_COUNT; i++) {
>               ExtSaveArea *esa = &x86_ext_save_areas[i];
>   
> -            if (!(supported_xcr0 & (1 << i))) {
> +            if (!((supported_xcr0 | supported_xss) & (1 << i))) {
>                   esa->size = 0;
>               }
>           }
Re: [PATCH v3 09/20] i386/cpu: Fix supervisor xstate initialization
Posted by Zhao Liu an hour ago
On Mon, Oct 27, 2025 at 03:55:30PM +0800, Xiaoyao Li wrote:
> Date: Mon, 27 Oct 2025 15:55:30 +0800
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: Re: [PATCH v3 09/20] i386/cpu: Fix supervisor xstate initialization
> 
> On 10/24/2025 2:56 PM, Zhao Liu wrote:
> > From: Chao Gao <chao.gao@intel.com>
> > 
> > Arch lbr is a supervisor xstate, but its area is not covered in
> > x86_cpu_init_xsave().
> > 
> > Fix it by checking supported xss bitmap.
> > 
> > In addition, drop the (uint64_t) type casts for supported_xcr0 since
> > x86_cpu_get_supported_feature_word() returns uint64_t so that the cast
> > is not needed. Then ensure line length is within 90 characters.
> > 
> > Tested-by: Farrah Chen <farrah.chen@intel.com>
> > Signed-off-by: Chao Gao <chao.gao@intel.com>
> > Co-developed-by: Zhao Liu <zhao1.liu@intel.com>
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> >   target/i386/cpu.c | 9 ++++++---
> >   1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 5cd335bb5574..1917376dbea9 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -9707,20 +9707,23 @@ static void x86_cpu_post_initfn(Object *obj)
> >   static void x86_cpu_init_xsave(void)
> >   {
> >       static bool first = true;
> > -    uint64_t supported_xcr0;
> > +    uint64_t supported_xcr0, supported_xss;
> >       int i;
> >       if (first) {
> >           first = false;
> >           supported_xcr0 =
> > -            ((uint64_t) x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_HI) << 32) |
> > +            x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_HI) |
> 
> missing the "<< 32" here,

Yes, good catch.