While reviewing the AVX512_BF16, Jing Liu and I noted that the handling of
CPUID leaf 7 is quite messy, and in general the handling of multi-index
CPUID leaves is confusing. These patches clean the code to prepare
for adding CPUID leaf 7 subleaf 1.
Paolo Bonzini (5):
KVM: cpuid: do_cpuid_ent works on a whole CPUID function
KVM: cpuid: extract do_cpuid_7_mask and support multiple subleafs
KVM: cpuid: set struct kvm_cpuid_entry2 flags in do_cpuid_1_ent
KVM: cpuid: rename do_cpuid_1_ent
KVM: cpuid: remove has_leaf_count from struct kvm_cpuid_param
arch/x86/kvm/cpuid.c | 222 ++++++++++++++++++++++++-------------------
1 file changed, 122 insertions(+), 100 deletions(-)
--
2.21.0
do_cpuid_1_ent is typically called in two places by __do_cpuid_func
for CPUID functions that have subleafs. Both places have to set
the KVM_CPUID_FLAG_SIGNIFCANT_INDEX. Set that flag, and
KVM_CPUID_FLAG_STATEFUL_FUNC as well, directly in do_cpuid_1_ent.
Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/x86/kvm/cpuid.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 1c6b9a4a74de..9ebc5ae7fa0e 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -298,6 +298,20 @@ static void do_cpuid_1_ent(struct kvm_cpuid_entry2 *entry, u32 function,
cpuid_count(entry->function, entry->index,
&entry->eax, &entry->ebx, &entry->ecx, &entry->edx);
+
+ switch (function) {
+ case 2:
+ entry->flags |= KVM_CPUID_FLAG_STATEFUL_FUNC;
+ break;
+ case 4:
+ case 7:
+ case 0xb:
+ case 0xd:
+ case 0x14:
+ case 0x8000001d:
+ entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ break;
+ }
}
static int __do_cpuid_func_emulated(struct kvm_cpuid_entry2 *entry,
@@ -497,14 +511,12 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
case 2: {
int t, times = entry->eax & 0xff;
- entry->flags |= KVM_CPUID_FLAG_STATEFUL_FUNC;
entry->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT;
for (t = 1; t < times; ++t) {
if (*nent >= maxnent)
goto out;
do_cpuid_1_ent(&entry[t], function, 0);
- entry[t].flags |= KVM_CPUID_FLAG_STATEFUL_FUNC;
++*nent;
}
break;
@@ -514,7 +526,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
case 0x8000001d: {
int i, cache_type;
- entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
/* read more entries until cache_type is zero */
for (i = 1; ; ++i) {
if (*nent >= maxnent)
@@ -524,8 +535,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
if (!cache_type)
break;
do_cpuid_1_ent(&entry[i], function, i);
- entry[i].flags |=
- KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
++*nent;
}
break;
@@ -540,7 +549,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
case 7: {
int i;
- entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
for (i = 0; ; ) {
do_cpuid_7_mask(&entry[i], i);
if (i == entry->eax)
@@ -550,8 +558,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
++i;
do_cpuid_1_ent(&entry[i], function, i);
- entry[i].flags |=
- KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
++*nent;
}
break;
@@ -595,7 +601,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
case 0xb: {
int i, level_type;
- entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
/* read more entries until level_type is zero */
for (i = 1; ; ++i) {
if (*nent >= maxnent)
@@ -605,8 +610,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
if (!level_type)
break;
do_cpuid_1_ent(&entry[i], function, i);
- entry[i].flags |=
- KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
++*nent;
}
break;
@@ -619,7 +622,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
entry->ebx = xstate_required_size(supported, false);
entry->ecx = entry->ebx;
entry->edx &= supported >> 32;
- entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
if (!supported)
break;
@@ -645,8 +647,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
}
entry[i].ecx = 0;
entry[i].edx = 0;
- entry[i].flags |=
- KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
++*nent;
++i;
}
@@ -659,12 +659,10 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
if (!f_intel_pt)
break;
- entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
for (t = 1; t <= times; ++t) {
if (*nent >= maxnent)
goto out;
do_cpuid_1_ent(&entry[t], function, t);
- entry[t].flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
++*nent;
}
break;
--
2.21.0
do_cpuid_1_ent does not do the entire processing for a CPUID entry, it
only retrieves the host's values. Rename it to match reality.
Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/x86/kvm/cpuid.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 9ebc5ae7fa0e..d403695f2f3b 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -289,7 +289,7 @@ static void cpuid_mask(u32 *word, int wordnum)
*word &= boot_cpu_data.x86_capability[wordnum];
}
-static void do_cpuid_1_ent(struct kvm_cpuid_entry2 *entry, u32 function,
+static void do_host_cpuid(struct kvm_cpuid_entry2 *entry, u32 function,
u32 index)
{
entry->function = function;
@@ -487,7 +487,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
if (*nent >= maxnent)
goto out;
- do_cpuid_1_ent(entry, function, 0);
+ do_host_cpuid(entry, function, 0);
++*nent;
switch (function) {
@@ -516,7 +516,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
if (*nent >= maxnent)
goto out;
- do_cpuid_1_ent(&entry[t], function, 0);
+ do_host_cpuid(&entry[t], function, 0);
++*nent;
}
break;
@@ -534,7 +534,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
cache_type = entry[i - 1].eax & 0x1f;
if (!cache_type)
break;
- do_cpuid_1_ent(&entry[i], function, i);
+ do_host_cpuid(&entry[i], function, i);
++*nent;
}
break;
@@ -557,7 +557,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
goto out;
++i;
- do_cpuid_1_ent(&entry[i], function, i);
+ do_host_cpuid(&entry[i], function, i);
++*nent;
}
break;
@@ -609,7 +609,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
level_type = entry[i - 1].ecx & 0xff00;
if (!level_type)
break;
- do_cpuid_1_ent(&entry[i], function, i);
+ do_host_cpuid(&entry[i], function, i);
++*nent;
}
break;
@@ -630,7 +630,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
if (*nent >= maxnent)
goto out;
- do_cpuid_1_ent(&entry[i], function, idx);
+ do_host_cpuid(&entry[i], function, idx);
if (idx == 1) {
entry[i].eax &= kvm_cpuid_D_1_eax_x86_features;
cpuid_mask(&entry[i].eax, CPUID_D_1_EAX);
@@ -662,7 +662,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
for (t = 1; t <= times; ++t) {
if (*nent >= maxnent)
goto out;
- do_cpuid_1_ent(&entry[t], function, t);
+ do_host_cpuid(&entry[t], function, t);
++*nent;
}
break;
--
2.21.0
Rename it as well as __do_cpuid_ent and __do_cpuid_ent_emulated to have
"func" in its name, and drop the index parameter which is always 0.
Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/x86/kvm/cpuid.c | 89 +++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 47 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 004cbd84c351..ddffc56c39b4 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -294,14 +294,19 @@ static void do_cpuid_1_ent(struct kvm_cpuid_entry2 *entry, u32 function,
{
entry->function = function;
entry->index = index;
+ entry->flags = 0;
+
cpuid_count(entry->function, entry->index,
&entry->eax, &entry->ebx, &entry->ecx, &entry->edx);
- entry->flags = 0;
}
-static int __do_cpuid_ent_emulated(struct kvm_cpuid_entry2 *entry,
- u32 func, u32 index, int *nent, int maxnent)
+static int __do_cpuid_func_emulated(struct kvm_cpuid_entry2 *entry,
+ u32 func, int *nent, int maxnent)
{
+ entry->function = func;
+ entry->index = 0;
+ entry->flags = 0;
+
switch (func) {
case 0:
entry->eax = 7;
@@ -313,21 +318,18 @@ static int __do_cpuid_ent_emulated(struct kvm_cpuid_entry2 *entry,
break;
case 7:
entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
- if (index == 0)
- entry->ecx = F(RDPID);
+ entry->eax = 0;
+ entry->ecx = F(RDPID);
++*nent;
default:
break;
}
- entry->function = func;
- entry->index = index;
-
return 0;
}
-static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
- u32 index, int *nent, int maxnent)
+static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
+ int *nent, int maxnent)
{
int r;
unsigned f_nx = is_efer_nx() ? F(NX) : 0;
@@ -431,7 +433,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
if (*nent >= maxnent)
goto out;
- do_cpuid_1_ent(entry, function, index);
+ do_cpuid_1_ent(entry, function, 0);
++*nent;
switch (function) {
@@ -496,34 +498,28 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
break;
case 7: {
entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
- /* Mask ebx against host capability word 9 */
- if (index == 0) {
- entry->ebx &= kvm_cpuid_7_0_ebx_x86_features;
- cpuid_mask(&entry->ebx, CPUID_7_0_EBX);
- // TSC_ADJUST is emulated
- entry->ebx |= F(TSC_ADJUST);
- entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
- f_la57 = entry->ecx & F(LA57);
- cpuid_mask(&entry->ecx, CPUID_7_ECX);
- /* Set LA57 based on hardware capability. */
- entry->ecx |= f_la57;
- entry->ecx |= f_umip;
- /* PKU is not yet implemented for shadow paging. */
- if (!tdp_enabled || !boot_cpu_has(X86_FEATURE_OSPKE))
- entry->ecx &= ~F(PKU);
- entry->edx &= kvm_cpuid_7_0_edx_x86_features;
- cpuid_mask(&entry->edx, CPUID_7_EDX);
- /*
- * We emulate ARCH_CAPABILITIES in software even
- * if the host doesn't support it.
- */
- entry->edx |= F(ARCH_CAPABILITIES);
- } else {
- entry->ebx = 0;
- entry->ecx = 0;
- entry->edx = 0;
- }
entry->eax = 0;
+ /* Mask ebx against host capability word 9 */
+ entry->ebx &= kvm_cpuid_7_0_ebx_x86_features;
+ cpuid_mask(&entry->ebx, CPUID_7_0_EBX);
+ // TSC_ADJUST is emulated
+ entry->ebx |= F(TSC_ADJUST);
+ entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
+ f_la57 = entry->ecx & F(LA57);
+ cpuid_mask(&entry->ecx, CPUID_7_ECX);
+ /* Set LA57 based on hardware capability. */
+ entry->ecx |= f_la57;
+ entry->ecx |= f_umip;
+ /* PKU is not yet implemented for shadow paging. */
+ if (!tdp_enabled || !boot_cpu_has(X86_FEATURE_OSPKE))
+ entry->ecx &= ~F(PKU);
+ entry->edx &= kvm_cpuid_7_0_edx_x86_features;
+ cpuid_mask(&entry->edx, CPUID_7_EDX);
+ /*
+ * We emulate ARCH_CAPABILITIES in software even
+ * if the host doesn't support it.
+ */
+ entry->edx |= F(ARCH_CAPABILITIES);
break;
}
case 9:
@@ -750,20 +746,19 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
return r;
}
-static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 func,
- u32 idx, int *nent, int maxnent, unsigned int type)
+static int do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 func,
+ int *nent, int maxnent, unsigned int type)
{
if (type == KVM_GET_EMULATED_CPUID)
- return __do_cpuid_ent_emulated(entry, func, idx, nent, maxnent);
+ return __do_cpuid_func_emulated(entry, func, nent, maxnent);
- return __do_cpuid_ent(entry, func, idx, nent, maxnent);
+ return __do_cpuid_func(entry, func, nent, maxnent);
}
#undef F
struct kvm_cpuid_param {
u32 func;
- u32 idx;
bool has_leaf_count;
bool (*qualifier)(const struct kvm_cpuid_param *param);
};
@@ -836,8 +831,8 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
if (ent->qualifier && !ent->qualifier(ent))
continue;
- r = do_cpuid_ent(&cpuid_entries[nent], ent->func, ent->idx,
- &nent, cpuid->nent, type);
+ r = do_cpuid_func(&cpuid_entries[nent], ent->func,
+ &nent, cpuid->nent, type);
if (r)
goto out_free;
@@ -847,8 +842,8 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
limit = cpuid_entries[nent - 1].eax;
for (func = ent->func + 1; func <= limit && nent < cpuid->nent && r == 0; ++func)
- r = do_cpuid_ent(&cpuid_entries[nent], func, ent->idx,
- &nent, cpuid->nent, type);
+ r = do_cpuid_func(&cpuid_entries[nent], func,
+ &nent, cpuid->nent, type);
if (r)
goto out_free;
--
2.21.0
CPUID function 7 has multiple subleafs. Instead of having nested
switch statements, move the logic to filter supported features to
a separate function, and call it for each subleaf.
Signed-off-by: Paolo Bonzini <[email protected]>
---
Here you would have something like entry->eax = min(entry->eax, 1)
when adding subleaf 1.
arch/x86/kvm/cpuid.c | 128 +++++++++++++++++++++++++++----------------
1 file changed, 81 insertions(+), 47 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index ddffc56c39b4..1c6b9a4a74de 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -328,6 +328,71 @@ static int __do_cpuid_func_emulated(struct kvm_cpuid_entry2 *entry,
return 0;
}
+static inline void do_cpuid_7_mask(struct kvm_cpuid_entry2 *entry, int index)
+{
+ unsigned f_invpcid = kvm_x86_ops->invpcid_supported() ? F(INVPCID) : 0;
+ unsigned f_mpx = kvm_mpx_supported() ? F(MPX) : 0;
+ unsigned f_umip = kvm_x86_ops->umip_emulated() ? F(UMIP) : 0;
+ unsigned f_intel_pt = kvm_x86_ops->pt_supported() ? F(INTEL_PT) : 0;
+ unsigned f_la57;
+
+ /* cpuid 7.0.ebx */
+ const u32 kvm_cpuid_7_0_ebx_x86_features =
+ F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) |
+ F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
+ F(ADX) | F(SMAP) | F(AVX512IFMA) | F(AVX512F) | F(AVX512PF) |
+ F(AVX512ER) | F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(AVX512DQ) |
+ F(SHA_NI) | F(AVX512BW) | F(AVX512VL) | f_intel_pt;
+
+ /* cpuid 7.0.ecx*/
+ const u32 kvm_cpuid_7_0_ecx_x86_features =
+ F(AVX512VBMI) | F(LA57) | F(PKU) | 0 /*OSPKE*/ |
+ F(AVX512_VPOPCNTDQ) | F(UMIP) | F(AVX512_VBMI2) | F(GFNI) |
+ F(VAES) | F(VPCLMULQDQ) | F(AVX512_VNNI) | F(AVX512_BITALG) |
+ F(CLDEMOTE) | F(MOVDIRI) | F(MOVDIR64B);
+
+ /* cpuid 7.0.edx*/
+ const u32 kvm_cpuid_7_0_edx_x86_features =
+ F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
+ F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
+ F(MD_CLEAR);
+
+ switch (index) {
+ case 0:
+ entry->eax = 0;
+ entry->ebx &= kvm_cpuid_7_0_ebx_x86_features;
+ cpuid_mask(&entry->ebx, CPUID_7_0_EBX);
+ /* TSC_ADJUST is emulated */
+ entry->ebx |= F(TSC_ADJUST);
+
+ entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
+ f_la57 = entry->ecx & F(LA57);
+ cpuid_mask(&entry->ecx, CPUID_7_ECX);
+ /* Set LA57 based on hardware capability. */
+ entry->ecx |= f_la57;
+ entry->ecx |= f_umip;
+ /* PKU is not yet implemented for shadow paging. */
+ if (!tdp_enabled || !boot_cpu_has(X86_FEATURE_OSPKE))
+ entry->ecx &= ~F(PKU);
+
+ entry->edx &= kvm_cpuid_7_0_edx_x86_features;
+ cpuid_mask(&entry->edx, CPUID_7_EDX);
+ /*
+ * We emulate ARCH_CAPABILITIES in software even
+ * if the host doesn't support it.
+ */
+ entry->edx |= F(ARCH_CAPABILITIES);
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ entry->eax = 0;
+ entry->ebx = 0;
+ entry->ecx = 0;
+ entry->edx = 0;
+ break;
+ }
+}
+
static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
int *nent, int maxnent)
{
@@ -342,12 +407,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
unsigned f_lm = 0;
#endif
unsigned f_rdtscp = kvm_x86_ops->rdtscp_supported() ? F(RDTSCP) : 0;
- unsigned f_invpcid = kvm_x86_ops->invpcid_supported() ? F(INVPCID) : 0;
- unsigned f_mpx = kvm_mpx_supported() ? F(MPX) : 0;
unsigned f_xsaves = kvm_x86_ops->xsaves_supported() ? F(XSAVES) : 0;
- unsigned f_umip = kvm_x86_ops->umip_emulated() ? F(UMIP) : 0;
unsigned f_intel_pt = kvm_x86_ops->pt_supported() ? F(INTEL_PT) : 0;
- unsigned f_la57 = 0;
/* cpuid 1.edx */
const u32 kvm_cpuid_1_edx_x86_features =
@@ -400,31 +461,10 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
F(ACE2) | F(ACE2_EN) | F(PHE) | F(PHE_EN) |
F(PMM) | F(PMM_EN);
- /* cpuid 7.0.ebx */
- const u32 kvm_cpuid_7_0_ebx_x86_features =
- F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) |
- F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
- F(ADX) | F(SMAP) | F(AVX512IFMA) | F(AVX512F) | F(AVX512PF) |
- F(AVX512ER) | F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(AVX512DQ) |
- F(SHA_NI) | F(AVX512BW) | F(AVX512VL) | f_intel_pt;
-
/* cpuid 0xD.1.eax */
const u32 kvm_cpuid_D_1_eax_x86_features =
F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | f_xsaves;
- /* cpuid 7.0.ecx*/
- const u32 kvm_cpuid_7_0_ecx_x86_features =
- F(AVX512VBMI) | F(LA57) | F(PKU) | 0 /*OSPKE*/ |
- F(AVX512_VPOPCNTDQ) | F(UMIP) | F(AVX512_VBMI2) | F(GFNI) |
- F(VAES) | F(VPCLMULQDQ) | F(AVX512_VNNI) | F(AVX512_BITALG) |
- F(CLDEMOTE) | F(MOVDIRI) | F(MOVDIR64B);
-
- /* cpuid 7.0.edx*/
- const u32 kvm_cpuid_7_0_edx_x86_features =
- F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
- F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
- F(MD_CLEAR);
-
/* all calls to cpuid_count() should be made on the same cpu */
get_cpu();
@@ -496,30 +536,24 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
entry->ecx = 0;
entry->edx = 0;
break;
+ /* function 7 has additional index. */
case 7: {
+ int i;
+
entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
- entry->eax = 0;
- /* Mask ebx against host capability word 9 */
- entry->ebx &= kvm_cpuid_7_0_ebx_x86_features;
- cpuid_mask(&entry->ebx, CPUID_7_0_EBX);
- // TSC_ADJUST is emulated
- entry->ebx |= F(TSC_ADJUST);
- entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
- f_la57 = entry->ecx & F(LA57);
- cpuid_mask(&entry->ecx, CPUID_7_ECX);
- /* Set LA57 based on hardware capability. */
- entry->ecx |= f_la57;
- entry->ecx |= f_umip;
- /* PKU is not yet implemented for shadow paging. */
- if (!tdp_enabled || !boot_cpu_has(X86_FEATURE_OSPKE))
- entry->ecx &= ~F(PKU);
- entry->edx &= kvm_cpuid_7_0_edx_x86_features;
- cpuid_mask(&entry->edx, CPUID_7_EDX);
- /*
- * We emulate ARCH_CAPABILITIES in software even
- * if the host doesn't support it.
- */
- entry->edx |= F(ARCH_CAPABILITIES);
+ for (i = 0; ; ) {
+ do_cpuid_7_mask(&entry[i], i);
+ if (i == entry->eax)
+ break;
+ if (*nent >= maxnent)
+ goto out;
+
+ ++i;
+ do_cpuid_1_ent(&entry[i], function, i);
+ entry[i].flags |=
+ KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ ++*nent;
+ }
break;
}
case 9:
--
2.21.0
The has_leaf_count member was originally added for KVM's paravirtualization
CPUID leaves. However, since then the leaf count _has_ been added to those
leaves as well, so we can drop that special case.
Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/x86/kvm/cpuid.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index d403695f2f3b..243613bf5978 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -791,7 +791,6 @@ static int do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 func,
struct kvm_cpuid_param {
u32 func;
- bool has_leaf_count;
bool (*qualifier)(const struct kvm_cpuid_param *param);
};
@@ -835,11 +834,10 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
int limit, nent = 0, r = -E2BIG, i;
u32 func;
static const struct kvm_cpuid_param param[] = {
- { .func = 0, .has_leaf_count = true },
- { .func = 0x80000000, .has_leaf_count = true },
- { .func = 0xC0000000, .qualifier = is_centaur_cpu, .has_leaf_count = true },
+ { .func = 0 },
+ { .func = 0x80000000 },
+ { .func = 0xC0000000, .qualifier = is_centaur_cpu },
{ .func = KVM_CPUID_SIGNATURE },
- { .func = KVM_CPUID_FEATURES },
};
if (cpuid->nent < 1)
@@ -869,9 +867,6 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
if (r)
goto out_free;
- if (!ent->has_leaf_count)
- continue;
-
limit = cpuid_entries[nent - 1].eax;
for (func = ent->func + 1; func <= limit && nent < cpuid->nent && r == 0; ++func)
r = do_cpuid_func(&cpuid_entries[nent], func,
--
2.21.0
Hi Paolo,
On 7/4/2019 10:07 PM, Paolo Bonzini wrote:
> Rename it as well as __do_cpuid_ent and __do_cpuid_ent_emulated to have
> "func" in its name, and drop the index parameter which is always 0.
>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> arch/x86/kvm/cpuid.c | 89 +++++++++++++++++++++-----------------------
> 1 file changed, 42 insertions(+), 47 deletions(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 004cbd84c351..ddffc56c39b4 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -294,14 +294,19 @@ static void do_cpuid_1_ent(struct kvm_cpuid_entry2 *entry, u32 function,
> {
> entry->function = function;
> entry->index = index;
> + entry->flags = 0;
> +
I'm wondering if we need set entry->flags = 0 here?
entry->flags was initialized as zero when vzalloc.
> cpuid_count(entry->function, entry->index,
> &entry->eax, &entry->ebx, &entry->ecx, &entry->edx);
> - entry->flags = 0;
> }
>
> -static int __do_cpuid_ent_emulated(struct kvm_cpuid_entry2 *entry,
> - u32 func, u32 index, int *nent, int maxnent)
> +static int __do_cpuid_func_emulated(struct kvm_cpuid_entry2 *entry,
> + u32 func, int *nent, int maxnent)
> {
> + entry->function = func;
> + entry->index = 0;
> + entry->flags = 0;
> +
The same question for flags and index, because entry is allocated
by vzalloc.
> switch (func) {
> case 0:
> entry->eax = 7;
> @@ -313,21 +318,18 @@ static int __do_cpuid_ent_emulated(struct kvm_cpuid_entry2 *entry,
> break;
> case 7:
> entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> - if (index == 0)
> - entry->ecx = F(RDPID);
> + entry->eax = 0;
> + entry->ecx = F(RDPID);
> ++*nent;
> default:
> break;
> }
>
> - entry->function = func;
> - entry->index = index;
> -
> return 0;
> }
>
Thanks,
Jing
Hi Paolo,
Thank you for refining the cpuid codes especially for case 7! It looks
much clear now!
On 7/4/2019 10:07 PM, Paolo Bonzini wrote:
> CPUID function 7 has multiple subleafs. Instead of having nested
> switch statements, move the logic to filter supported features to
> a separate function, and call it for each subleaf.
>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> Here you would have something like entry->eax = min(entry->eax, 1)
> when adding subleaf 1.
>
> arch/x86/kvm/cpuid.c | 128 +++++++++++++++++++++++++++----------------
> 1 file changed, 81 insertions(+), 47 deletions(-)
>
[...]
> +
> + switch (index) {
> + case 0:
> + entry->eax = 0;
Here, mark: when adding subleaf 1, change to
entry->eax = min(entry->eax, 1).
> + entry->ebx &= kvm_cpuid_7_0_ebx_x86_features;
> + cpuid_mask(&entry->ebx, CPUID_7_0_EBX);
> + /* TSC_ADJUST is emulated */
> + entry->ebx |= F(TSC_ADJUST);
> +
> + entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
> + f_la57 = entry->ecx & F(LA57);
> + cpuid_mask(&entry->ecx, CPUID_7_ECX);
> + /* Set LA57 based on hardware capability. */
> + entry->ecx |= f_la57;
> + entry->ecx |= f_umip;
> + /* PKU is not yet implemented for shadow paging. */
> + if (!tdp_enabled || !boot_cpu_has(X86_FEATURE_OSPKE))
> + entry->ecx &= ~F(PKU);
> +
> + entry->edx &= kvm_cpuid_7_0_edx_x86_features;
> + cpuid_mask(&entry->edx, CPUID_7_EDX);
> + /*
> + * We emulate ARCH_CAPABILITIES in software even
> + * if the host doesn't support it.
> + */
> + entry->edx |= F(ARCH_CAPABILITIES);
> + break;
And when adding subleaf 1, plan to add codes,
case 1:
entry->eax |= kvm_cpuid_7_1_eax_x86_features;
entry->ebx = entry->ecx = entry->edx =0;
break;
What do you think?
> + default:
> + WARN_ON_ONCE(1);
> + entry->eax = 0;
> + entry->ebx = 0;
> + entry->ecx = 0;
> + entry->edx = 0;
> + break;
> + }
> +}
> +
> static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
> int *nent, int maxnent)
[...]
> + /* function 7 has additional index. */
> case 7: {
> + int i;
> +
> entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> - entry->eax = 0;
> - /* Mask ebx against host capability word 9 */
> - entry->ebx &= kvm_cpuid_7_0_ebx_x86_features;
> - cpuid_mask(&entry->ebx, CPUID_7_0_EBX);
> - // TSC_ADJUST is emulated
> - entry->ebx |= F(TSC_ADJUST);
> - entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
> - f_la57 = entry->ecx & F(LA57);
> - cpuid_mask(&entry->ecx, CPUID_7_ECX);
> - /* Set LA57 based on hardware capability. */
> - entry->ecx |= f_la57;
> - entry->ecx |= f_umip;
> - /* PKU is not yet implemented for shadow paging. */
> - if (!tdp_enabled || !boot_cpu_has(X86_FEATURE_OSPKE))
> - entry->ecx &= ~F(PKU);
> - entry->edx &= kvm_cpuid_7_0_edx_x86_features;
> - cpuid_mask(&entry->edx, CPUID_7_EDX);
> - /*
> - * We emulate ARCH_CAPABILITIES in software even
> - * if the host doesn't support it.
> - */
> - entry->edx |= F(ARCH_CAPABILITIES);
> + for (i = 0; ; ) {
> + do_cpuid_7_mask(&entry[i], i);
> + if (i == entry->eax)
> + break;
> + if (*nent >= maxnent)
> + goto out;
> +
> + ++i;
> + do_cpuid_1_ent(&entry[i], function, i);
> + entry[i].flags |=
> + KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + ++*nent;
> + }
The new logic is great and adding subleaf support would be much easier!
> break;
> }
> case 9:
>
Thanks,
Jing
Hi Paolo,
On 7/4/2019 10:07 PM, Paolo Bonzini wrote:
> The has_leaf_count member was originally added for KVM's paravirtualization
> CPUID leaves. However, since then the leaf count _has_ been added to those
> leaves as well, so we can drop that special case.
>
> Signed-off-by: Paolo Bonzini <[email protected]>
[...]
> @@ -835,11 +834,10 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
> int limit, nent = 0, r = -E2BIG, i;
> u32 func;
> static const struct kvm_cpuid_param param[] = {
> - { .func = 0, .has_leaf_count = true },
> - { .func = 0x80000000, .has_leaf_count = true },
> - { .func = 0xC0000000, .qualifier = is_centaur_cpu, .has_leaf_count = true },
> + { .func = 0 },
> + { .func = 0x80000000 },
> + { .func = 0xC0000000, .qualifier = is_centaur_cpu },
> { .func = KVM_CPUID_SIGNATURE },
> - { .func = KVM_CPUID_FEATURES },
It seems the two func are introduced by 2b5e97e, as paravirtual cpuid.
But when searching KVM_CPUID_SIGNATURE, there seems no caller requesting
this cpuid. Meanwhile, I felt curious if KVM_CPUID_FEATURES is still in
use but it seems kvm_update_cpuid() uses that. Not sure which spec
introduces the latest pv cpuid.
Thanks,
Jing
[...]
On 08/07/19 09:07, Jing Liu wrote:
>>
> And when adding subleaf 1, plan to add codes,
>
> case 1:
> entry->eax |= kvm_cpuid_7_1_eax_x86_features;
> entry->ebx = entry->ecx = entry->edx =0;
> break;
>
> What do you think?
This should be "&=", not "|=". Otherwise yes, that's the idea.
Paolo
On 08/07/19 09:09, Jing Liu wrote:
> It seems the two func are introduced by 2b5e97e, as paravirtual cpuid.
> But when searching KVM_CPUID_SIGNATURE, there seems no caller requesting
> this cpuid. Meanwhile, I felt curious if KVM_CPUID_FEATURES is still in
> use but it seems kvm_update_cpuid() uses that. Not sure which spec
> introduces the latest pv cpuid.
Yes, KVM_CPUID_SIGNATURE is generally not very interesting for
userspace. But KVM_CPUID_FEATURES is called here:
for (w = 0; w < FEATURE_WORDS; w++) {
/* Override only features that weren't set explicitly
* by the user.
*/
env->features[w] |=
x86_cpu_get_supported_feature_word(w, cpu->migratable) &
~env->user_features[w] & \
~feature_word_info[w].no_autoenable_flags;
}
Paolo
On 7/10/2019 2:30 PM, Paolo Bonzini wrote:
> On 08/07/19 09:07, Jing Liu wrote:
>>>
>> And when adding subleaf 1, plan to add codes,
>>
>> case 1:
>> entry->eax |= kvm_cpuid_7_1_eax_x86_features;
>> entry->ebx = entry->ecx = entry->edx =0;
>> break;
>>
>> What do you think?
>
> This should be "&=", not "|=". Otherwise yes, that's the idea.
>
Yes! So let me send out the BFloat16 patch based on your patch set now
or you have merge plan soon?
Thanks,
Jing
> Paolo
>
On 10/07/19 09:32, Jing Liu wrote:
>
>
> On 7/10/2019 2:30 PM, Paolo Bonzini wrote:
>> On 08/07/19 09:07, Jing Liu wrote:
>>>>
>>> And when adding subleaf 1, plan to add codes,
>>>
>>> case 1:
>>> entry->eax |= kvm_cpuid_7_1_eax_x86_features;
>>> entry->ebx = entry->ecx = entry->edx =0;
>>> break;
>>>
>>> What do you think?
>>
>> This should be "&=", not "|=". Otherwise yes, that's the idea.
>>
>
> Yes! So let me send out the BFloat16 patch based on your patch set now
> or you have merge plan soon?
Just send it . :)
Paolo