[2/8] target/i386: do not rely on ExtSaveArea for accelerator-supported XCR0 bits

Commit Message

Paolo Bonzini Oct. 29, 2024, 3:18 p.m. UTC

Right now, QEMU is using the "feature" and "bits" fields of ExtSaveArea
to query the accelerator for the support status of extended save areas.
This is a problem for AVX10, which attaches two feature bits (AVX512F
and AVX10) to the same extended save states.

To keep the AVX10 hacks to the minimum, limit usage of esa->features
and esa->bits.  Instead, just query the accelerator for the 0xD leaf.
Do it in common code and clear esa->size if an extended save state is
unsupported.

Signed-off-by: Paolo Bonzini <[e-mail]>
---
 target/i386/cpu.c         | 33 +++++++++++++++++++++++++++++++--
 target/i386/kvm/kvm-cpu.c |  4 ----
 2 files changed, 31 insertions(+), 6 deletions(-)

Comments

Zhao Liu Oct. 30, 2024, 3:50 a.m. UTC | #1

On Tue, Oct 29, 2024 at 04:18:52PM +0100, Paolo Bonzini wrote:
> Date: Tue, 29 Oct 2024 16:18:52 +0100
> From: Paolo Bonzini <[e-mail]>
> Subject: [PATCH 2/8] target/i386: do not rely on ExtSaveArea for
>  accelerator-supported XCR0 bits
> X-Mailer: git-send-email 2.47.0
> 
> Right now, QEMU is using the "feature" and "bits" fields of ExtSaveArea
> to query the accelerator for the support status of extended save areas.
> This is a problem for AVX10, which attaches two feature bits (AVX512F
> and AVX10) to the same extended save states.
> 
> To keep the AVX10 hacks to the minimum, limit usage of esa->features
> and esa->bits.  Instead, just query the accelerator for the 0xD leaf.
> Do it in common code and clear esa->size if an extended save state is
> unsupported.
> 
> Signed-off-by: Paolo Bonzini <[e-mail]>
> ---
>  target/i386/cpu.c         | 33 +++++++++++++++++++++++++++++++--
>  target/i386/kvm/kvm-cpu.c |  4 ----
>  2 files changed, 31 insertions(+), 6 deletions(-)

Reviewed-by: Zhao Liu <[e-mail]>

Patch

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f08e9b8f1bc..1ee4d988caf 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7102,6 +7102,15 @@  static void x86_cpu_set_sgxlepubkeyhash(CPUX86State *env)
 #endif
 }
 
+static bool cpuid_has_xsave_feature(CPUX86State *env, const ExtSaveArea *esa)
+{
+    if (!esa->size) {
+        return false;
+    }
+
+    return (env->features[esa->feature] & esa->bits);
+}
+
 static void x86_cpu_reset_hold(Object *obj, ResetType type)
 {
     CPUState *cs = CPU(obj);
@@ -7210,7 +7219,7 @@  static void x86_cpu_reset_hold(Object *obj, ResetType type)
         if (!((1 << i) & CPUID_XSTATE_XCR0_MASK)) {
             continue;
         }
-        if (env->features[esa->feature] & esa->bits) {
+        if (cpuid_has_xsave_feature(env, esa)) {
             xcr0 |= 1ull << i;
         }
     }
@@ -7348,7 +7357,7 @@  static void x86_cpu_enable_xsave_components(X86CPU *cpu)
     mask = 0;
     for (i = 0; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
         const ExtSaveArea *esa = &x86_ext_save_areas[i];
-        if (env->features[esa->feature] & esa->bits) {
+        if (cpuid_has_xsave_feature(env, esa)) {
             mask |= (1ULL << i);
         }
     }
@@ -8020,6 +8029,26 @@  static void x86_cpu_register_feature_bit_props(X86CPUClass *xcc,
 
 static void x86_cpu_post_initfn(Object *obj)
 {
+    static bool first = true;
+    uint64_t supported_xcr0;
+    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_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))) {
+                esa->size = 0;
+            }
+        }
+    }
+
     accel_cpu_instance_init(CPU(obj));
 }
 
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 6bf8dcfc607..d9306418490 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -133,10 +133,6 @@  static void kvm_cpu_xsave_init(void)
         if (!esa->size) {
             continue;
         }
-        if ((x86_cpu_get_supported_feature_word(NULL, esa->feature) & esa->bits)
-            != esa->bits) {
-            continue;
-        }
         host_cpuid(0xd, i, &eax, &ebx, &ecx, &edx);
         if (eax != 0) {
             assert(esa->size == eax);