Re: [PATCH v4 06/23] i386/kvm: Initialize x86_ext_save_areas[] based on KVM support

[Paolo Bonzini]

On 11/18/25 04:42, Zhao Liu wrote:
At present, in KVM, x86_ext_save_areas[] is initialized based on Host
CPUIDs.
Except AMX xstates, both user & supervisor xstates have their
information exposed in KVM_GET_SUPPORTED_CPUID. Therefore, their entries
in x86_ext_save_areas[] should be filled based on KVM support.

For AMX xstates (XFEATURE_MASK_XTILE_DATA and XFEATURE_MASK_XTILE_CFG),
KVM doesn't report their details before they (mainly
XSTATE_XTILE_DATA_MASK) get permission on host. But this happens within
the function kvm_request_xsave_components(), after the current
initialization. So still fill AMX entries with Host CPUIDs.

In addition, drop a check: "if (eax != 0)" when assert the assert the
size of xstate. In fact, this check is incorrect, since any valid
xstate should have non-zero size of xstate area.

Tested-by: Farrah Chen <farrah.c...@intel.com>
Signed-off-by: Zhao Liu <zhao1....@intel.com>
---
Changes Since v3:
  - New commit.
---
  target/i386/cpu.h         |  3 +++
  target/i386/kvm/kvm-cpu.c | 23 +++++++++++++++++------
  2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 3d74afc5a8e7..f065527757c4 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -609,6 +609,9 @@ typedef enum X86Seg {
  
  #define XSTATE_DYNAMIC_MASK             (XSTATE_XTILE_DATA_MASK)
  
+#define XSTATE_XTILE_MASK               (XSTATE_XTILE_DATA_MASK | \
+                                         XSTATE_XTILE_CFG_MASK)
+
  #define ESA_FEATURE_ALIGN64_BIT         1
  #define ESA_FEATURE_XFD_BIT             2
  
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 9c25b5583955..2e2d47d2948a 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -136,7 +136,7 @@ static void kvm_cpu_max_instance_init(X86CPU *cpu)
  static void kvm_cpu_xsave_init(void)
  {
      static bool first = true;
-    uint32_t eax, ebx, ecx, edx;
+    uint32_t eax, ebx, ecx, unused;
      int i;
  
      if (!first) {
@@ -154,12 +154,23 @@ static void kvm_cpu_xsave_init(void)
          if (!esa->size) {
              continue;
          }
-        host_cpuid(0xd, i, &eax, &ebx, &ecx, &edx);
-        if (eax != 0) {
-            assert(esa->size == eax);
-            esa->offset = ebx;
-            esa->ecx = ecx;
+
+        /*
+         * AMX xstates are supported in KVM_GET_SUPPORTED_CPUID only when
+         * XSTATE_XTILE_DATA_MASK gets guest permission in
+         * kvm_request_xsave_components().
+         */
+        if (!((1 << i) & XSTATE_XTILE_MASK)) {

This should be XSTATE_DYNAMIC_MASK, but I don't like getting the 
information differently.  My understanding is that this is useful for 
the next patch, but I don't understand well why the next patch is 
needed. The commit message doesn't help.

Can you move the call to kvm_cpu_xsave_init() after 
x86_cpu_enable_xsave_components()?  Is it used anywhere before the CPU 
is running?

Paolo

+            eax = kvm_arch_get_supported_cpuid(kvm_state, 0xd, i, R_EAX);
+            ebx = kvm_arch_get_supported_cpuid(kvm_state, 0xd, i, R_EBX);
+            ecx = kvm_arch_get_supported_cpuid(kvm_state, 0xd, i, R_ECX);
+        } else {
+            host_cpuid(0xd, i, &eax, &ebx, &ecx, &unused);
          }
+
+        assert(esa->size == eax);
+        esa->offset = ebx;
+        esa->ecx = ecx;
      }
  }