On Mon, Jan 11, 2021 at 11:20:11AM -0800, Sean Christopherson wrote:
> Well, mechanically, that would generate a build failure if the kernel does the
> obvious things and names the 'enum cpuid_leafs' entry CPUID_12_EAX. That would
> be an obvious clue that KVM should be updated.
Then we need to properly document that whenever someone does that
change, someone needs to touch the proper places.
> If the kernel named the enum entry something different, and we botched the code
> review, KVM would continue to work, but would unnecessarily copy the bits it
> cares about to its own word. E.g. the boot_cpu_has() checks and translation to
> __X86_FEATURE_* would still be valid. As far as failure modes go, that's not
> terrible.
Right, which reminds me: with your prototype patch, we would have:
static __always_inline void __kvm_cpu_cap_mask(enum cpuid_leafs leaf)
{
const struct cpuid_reg cpuid = x86_feature_cpuid(leaf * 32);
struct kvm_cpuid_entry2 entry;
reverse_cpuid_check(leaf);
cpuid_count(cpuid.function, cpuid.index,
&entry.eax, &entry.ebx, &entry.ecx, &entry.edx);
kvm_cpu_caps[leaf] &= *__cpuid_entry_get_reg(&entry, cpuid.reg);
}
which does read CPUID from the hw and kvm_cpu_caps[] has already the
copied bits from boot_cpu_data.x86_capability.
Now you said that reading the CPUID is mostly redundant but we're
paranoid so we do it anyway, just in case, so how about we remove the
copying of boot_cpu_data.x86_capability? That's one less dependency
on the baremetal implementation.
Practically, nothing changes for kvm because it will read CPUID which is
the canonical thing anyway. And this should simplify the deal more and
keep it simple(r).
Hmmm.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
