On 5/25/2025 8:47 PM, Xiaoyao Li wrote:
On 1/3/2025 4:48 PM, Xin Li (Intel) wrote:
@@ -1133,6 +1134,25 @@ FeatureWordInfo
feature_word_info[FEATURE_WORDS] = {
},
.tcg_features = TCG_7_1_EAX_FEATURES,
},
+ [FEAT_7_1_ECX] = {
+ .type = CPUID_FEATURE_WORD,
+ .feat_names = {
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
This looks silly, and the size of feat_names[] was changed from 32 to
64. Just explicitly assign the first 32 entries with NULL doesn't make
any sense after the size change.
We can just merge the next patch into this one and make it,
.feat_names = {
[5] = "msr-imm",
},
I appreciate this format that clearly indicates which bit corresponds to
which feature, and I'm inclined to proceed with the change.
On the flip side, I hope the new format won't be perceived as disrupting
the consistency of the existing codebase. If this form could get called
out by maintainers, there needs a cleanup patch to change all the
instances.
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index dbd8f1ffc7..d23fa96549 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -667,6 +667,7 @@ typedef enum FeatureWord {
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 */
+ FEAT_7_1_ECX, /* CPUID[EAX=7,ECX=1].ECX */
I would prefer the newly added leaf being ordered at least in the same
leaf. i.e.,
@@ -661,6 +661,7 @@ typedef enum FeatureWord {
FEAT_SGX_12_1_EAX, /* CPUID[EAX=0x12,ECX=1].EAX (SGX
ATTRIBUTES[31:0]) */
FEAT_XSAVE_XSS_LO, /* CPUID[EAX=0xd,ECX=1].ECX */
FEAT_XSAVE_XSS_HI, /* CPUID[EAX=0xd,ECX=1].EDX */
+ FEAT_7_1_ECX, /* CPUID[EAX=7,ECX=1].ECX */
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 */
They are QEMU internally data, injecting a new one instead of putting it
at the end is OK to me.
Makes sense to me. Will move FEAT_7_1_ECX, FEAT_7_1_EDX and
FEAT_7_2_EDX immediate after FEAT_7_1_EAX.
Thanks!
Xin
