[PATCH v3 03/20] i386/cpu: Clean up arch lbr xsave struct and comment

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 03/20] i386/cpu: Clean up arch lbr xsave struct and comment
Posted by Zhao Liu 3 days, 4 hours ago
Arch LBR state is area 15, not 19. Fix this comment. And considerring
other areas don't mention user or supervisor state, for consistent
style, remove "Supervisor mode" from its comment.

Moreover, rename XSavesArchLBR to XSaveArchLBR since there's no need to
emphasize XSAVES in naming; the XSAVE related structure is mainly
used to represent memory layout.

In addition, arch lbr specifies its offset of xsave component as 0. But
this cannot help on anything. The offset of ExtSaveArea is initialized
by accelerators (e.g., hvf_cpu_xsave_init(), kvm_cpu_xsave_init() and
x86_tcg_cpu_xsave_init()), so explicitly setting the offset doesn't
work and CPUID 0xD encoding has already ensure supervisor states won't
have non-zero offsets. Drop the offset initialization and its comment
from the xsave area of arch lbr.

Tested-by: Farrah Chen <farrah.chen@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 3 +--
 target/i386/cpu.h | 8 ++++----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f0e179c2d235..b9a5a0400dea 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2058,8 +2058,7 @@ ExtSaveArea x86_ext_save_areas[XSAVE_STATE_AREA_COUNT] = {
     },
     [XSTATE_ARCH_LBR_BIT] = {
         .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_ARCH_LBR,
-        .offset = 0 /*supervisor mode component, offset = 0 */,
-        .size = sizeof(XSavesArchLBR),
+        .size = sizeof(XSaveArchLBR),
     },
     [XSTATE_XTILE_CFG_BIT] = {
         .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_AMX_TILE,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d0da9bfe58ce..886a941e481c 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1747,15 +1747,15 @@ typedef struct {
 
 #define ARCH_LBR_NR_ENTRIES            32
 
-/* Ext. save area 19: Supervisor mode Arch LBR state */
-typedef struct XSavesArchLBR {
+/* Ext. save area 15: Arch LBR state */
+typedef struct XSaveArchLBR {
     uint64_t lbr_ctl;
     uint64_t lbr_depth;
     uint64_t ler_from;
     uint64_t ler_to;
     uint64_t ler_info;
     LBREntry lbr_records[ARCH_LBR_NR_ENTRIES];
-} XSavesArchLBR;
+} XSaveArchLBR;
 
 QEMU_BUILD_BUG_ON(sizeof(XSaveAVX) != 0x100);
 QEMU_BUILD_BUG_ON(sizeof(XSaveBNDREG) != 0x40);
@@ -1766,7 +1766,7 @@ QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400);
 QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8);
 QEMU_BUILD_BUG_ON(sizeof(XSaveXTILECFG) != 0x40);
 QEMU_BUILD_BUG_ON(sizeof(XSaveXTILEDATA) != 0x2000);
-QEMU_BUILD_BUG_ON(sizeof(XSavesArchLBR) != 0x328);
+QEMU_BUILD_BUG_ON(sizeof(XSaveArchLBR) != 0x328);
 
 typedef struct ExtSaveArea {
     uint32_t feature, bits;
-- 
2.34.1
Re: [PATCH v3 03/20] i386/cpu: Clean up arch lbr xsave struct and comment
Posted by Xiaoyao Li 5 hours ago
On 10/24/2025 2:56 PM, Zhao Liu wrote:
> Arch LBR state is area 15, not 19. Fix this comment. And considerring
> other areas don't mention user or supervisor state, for consistent
> style, remove "Supervisor mode" from its comment.
> 
> Moreover, rename XSavesArchLBR to XSaveArchLBR since there's no need to
> emphasize XSAVES in naming; the XSAVE related structure is mainly
> used to represent memory layout.
> 
> In addition, arch lbr specifies its offset of xsave component as 0. But
> this cannot help on anything. The offset of ExtSaveArea is initialized
> by accelerators (e.g., hvf_cpu_xsave_init(), kvm_cpu_xsave_init() and
> x86_tcg_cpu_xsave_init()), so explicitly setting the offset doesn't
> work and CPUID 0xD encoding has already ensure supervisor states won't
> have non-zero offsets. Drop the offset initialization and its comment
> from the xsave area of arch lbr.
> 
> Tested-by: Farrah Chen <farrah.chen@intel.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

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

> ---
>   target/i386/cpu.c | 3 +--
>   target/i386/cpu.h | 8 ++++----
>   2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index f0e179c2d235..b9a5a0400dea 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2058,8 +2058,7 @@ ExtSaveArea x86_ext_save_areas[XSAVE_STATE_AREA_COUNT] = {
>       },
>       [XSTATE_ARCH_LBR_BIT] = {
>           .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_ARCH_LBR,
> -        .offset = 0 /*supervisor mode component, offset = 0 */,
> -        .size = sizeof(XSavesArchLBR),
> +        .size = sizeof(XSaveArchLBR),
>       },
>       [XSTATE_XTILE_CFG_BIT] = {
>           .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_AMX_TILE,
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index d0da9bfe58ce..886a941e481c 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1747,15 +1747,15 @@ typedef struct {
>   
>   #define ARCH_LBR_NR_ENTRIES            32
>   
> -/* Ext. save area 19: Supervisor mode Arch LBR state */
> -typedef struct XSavesArchLBR {
> +/* Ext. save area 15: Arch LBR state */
> +typedef struct XSaveArchLBR {
>       uint64_t lbr_ctl;
>       uint64_t lbr_depth;
>       uint64_t ler_from;
>       uint64_t ler_to;
>       uint64_t ler_info;
>       LBREntry lbr_records[ARCH_LBR_NR_ENTRIES];
> -} XSavesArchLBR;
> +} XSaveArchLBR;
>   
>   QEMU_BUILD_BUG_ON(sizeof(XSaveAVX) != 0x100);
>   QEMU_BUILD_BUG_ON(sizeof(XSaveBNDREG) != 0x40);
> @@ -1766,7 +1766,7 @@ QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400);
>   QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8);
>   QEMU_BUILD_BUG_ON(sizeof(XSaveXTILECFG) != 0x40);
>   QEMU_BUILD_BUG_ON(sizeof(XSaveXTILEDATA) != 0x2000);
> -QEMU_BUILD_BUG_ON(sizeof(XSavesArchLBR) != 0x328);
> +QEMU_BUILD_BUG_ON(sizeof(XSaveArchLBR) != 0x328);
>   
>   typedef struct ExtSaveArea {
>       uint32_t feature, bits;
Re: [PATCH v3 03/20] i386/cpu: Clean up arch lbr xsave struct and comment
Posted by Chen, Zide 2 days, 17 hours ago

On 10/23/2025 11:56 PM, Zhao Liu wrote:
> Arch LBR state is area 15, not 19. Fix this comment. And considerring
> other areas don't mention user or supervisor state, for consistent
> style, remove "Supervisor mode" from its comment.
> 
> Moreover, rename XSavesArchLBR to XSaveArchLBR since there's no need to
> emphasize XSAVES in naming; the XSAVE related structure is mainly
> used to represent memory layout.
> 
> In addition, arch lbr specifies its offset of xsave component as 0. But
> this cannot help on anything. The offset of ExtSaveArea is initialized
> by accelerators (e.g., hvf_cpu_xsave_init(), kvm_cpu_xsave_init() and
> x86_tcg_cpu_xsave_init()), so explicitly setting the offset doesn't
> work and CPUID 0xD encoding has already ensure supervisor states won't
> have non-zero offsets. Drop the offset initialization and its comment
> from the xsave area of arch lbr.
> 
> Tested-by: Farrah Chen <farrah.chen@intel.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Zide Chen <zide.chen@intel.com>


> ---
>  target/i386/cpu.c | 3 +--
>  target/i386/cpu.h | 8 ++++----
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index f0e179c2d235..b9a5a0400dea 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2058,8 +2058,7 @@ ExtSaveArea x86_ext_save_areas[XSAVE_STATE_AREA_COUNT] = {
>      },
>      [XSTATE_ARCH_LBR_BIT] = {
>          .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_ARCH_LBR,
> -        .offset = 0 /*supervisor mode component, offset = 0 */,
> -        .size = sizeof(XSavesArchLBR),
> +        .size = sizeof(XSaveArchLBR),
>      },
>      [XSTATE_XTILE_CFG_BIT] = {
>          .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_AMX_TILE,
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index d0da9bfe58ce..886a941e481c 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1747,15 +1747,15 @@ typedef struct {
>  
>  #define ARCH_LBR_NR_ENTRIES            32
>  
> -/* Ext. save area 19: Supervisor mode Arch LBR state */
> -typedef struct XSavesArchLBR {
> +/* Ext. save area 15: Arch LBR state */
> +typedef struct XSaveArchLBR {
>      uint64_t lbr_ctl;
>      uint64_t lbr_depth;
>      uint64_t ler_from;
>      uint64_t ler_to;
>      uint64_t ler_info;
>      LBREntry lbr_records[ARCH_LBR_NR_ENTRIES];
> -} XSavesArchLBR;
> +} XSaveArchLBR;
>  
>  QEMU_BUILD_BUG_ON(sizeof(XSaveAVX) != 0x100);
>  QEMU_BUILD_BUG_ON(sizeof(XSaveBNDREG) != 0x40);
> @@ -1766,7 +1766,7 @@ QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400);
>  QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8);
>  QEMU_BUILD_BUG_ON(sizeof(XSaveXTILECFG) != 0x40);
>  QEMU_BUILD_BUG_ON(sizeof(XSaveXTILEDATA) != 0x2000);
> -QEMU_BUILD_BUG_ON(sizeof(XSavesArchLBR) != 0x328);
> +QEMU_BUILD_BUG_ON(sizeof(XSaveArchLBR) != 0x328);
>  
>  typedef struct ExtSaveArea {
>      uint32_t feature, bits;