Re: [PATCH 3/6] target/i386: Add CPUID.24 leaf for AVX10

[Paolo Bonzini]

On 10/28/24 03:45, Tao Su wrote:

@@ -6835,6 +6850,26 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
          }
          break;
      }
+    case 0x24: {
+        *eax = 0;
+        *ebx = 0;
+        *ecx = 0;
+        *edx = 0;
+        if (!(env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10)) {
+            break;
+        }
+
+        if (count == 0) {
+            uint8_t v = kvm_arch_get_supported_cpuid(cs->kvm_state, 0x24,
+                                                     0, R_EBX);
+            if (env->avx10_version && env->avx10_version < v) {
+                v = env->avx10_version;
+            }
+
+            *ebx = env->features[FEAT_24_0_EBX] | v;
+        }
+        break;
+    }
      case 0x40000000:
          /*
           * CPUID code in kvm_arch_init_vcpu() ignores stuff

This check should be done elsewhere (called by x86_cpu_realizefn());
cpu_x86_cpuid() should only report the value:

         if ((env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) && count == 0) 
{
             *ebx = env->features[FEAT_24_0_EBX] | env->avx10_version;
         }

Also, the check should use x86_cpu_get_supported_cpuid() because KVM is not the
only accelerator.

+    if (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) {
+        uint8_t version =
+            kvm_arch_get_supported_cpuid(cs->kvm_state, 0x24, 0, R_EBX);
+
+        if (!env->avx10_version) {
+            env->avx10_version = version;
+        }
+

This should not be done here, but in max_x86_cpu_realize().  It should also
use x86_cpu_get_supported_cpuid().

For Granite Rapids you're only setting the AVX10 version in v2 and therefore
you don't need it, but there should also be (for the future) an avx10_version
field in X86CPUDefinition, which is set into the avx10-version property at
x86_cpu_load_model().

index d845384dcd..5566a13f4f 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -662,6 +662,7 @@ typedef enum FeatureWord {
      FEAT_XSAVE_XSS_HI,     /* CPUID[EAX=0xd,ECX=1].EDX */
      FEAT_7_1_EDX,       /* CPUID[EAX=7,ECX=1].EDX */
      FEAT_7_2_EDX,       /* CPUID[EAX=7,ECX=2].EDX */
+    FEAT_24_0_EBX,      /* CPUID[EAX=0x24,ECX=0].EBX */

Adding FEAT_24_0_EBX should be a separate patch.

Paolo

      FEATURE_WORDS,
  } FeatureWord;

@@ -990,6 +991,13 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w);

  /* Packets which contain IP payload have LIP values */
  #define CPUID_14_0_ECX_LIP              (1U << 31)

+/* AVX10 128-bit vector support is present */

+#define CPUID_24_0_EBX_AVX10_128        (1U << 16)
+/* AVX10 256-bit vector support is present */
+#define CPUID_24_0_EBX_AVX10_256        (1U << 17)
+/* AVX10 512-bit vector support is present */
+#define CPUID_24_0_EBX_AVX10_512        (1U << 18)
+
  /* RAS Features */
  #define CPUID_8000_0007_EBX_OVERFLOW_RECOV    (1U << 0)
  #define CPUID_8000_0007_EBX_SUCCOR      (1U << 1)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index fd9f198892..8e17942c3b 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1923,7 +1923,8 @@ static uint32_t kvm_x86_build_cpuid(CPUX86State *env,
          case 0x7:
          case 0x14:
          case 0x1d:
-        case 0x1e: {
+        case 0x1e:
+        case 0x24: {
              uint32_t times;

c->function = i;