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 <pbonzini@xxxxxxxxxx>
---
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
