On 7/9/2024 5:18 AM, Sean Christopherson wrote:
On Thu, Jul 04, 2024, Maxim Levitsky wrote:
On Fri, 2024-05-17 at 10:39 -0700, Sean Christopherson wrote:
Merge kvm_cpu_cap_init() and kvm_cpu_cap_init_kvm_defined() into a single
helper. The only advantage of separating the two was to make it somewhat
obvious that KVM directly initializes the KVM-defined words, whereas using
a common helper will allow for hardening both kernel- and KVM-defined
CPUID words without needing copy+paste.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
Reviewed-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
---
arch/x86/kvm/cpuid.c | 44 +++++++++++++++-----------------------------
1 file changed, 15 insertions(+), 29 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index f2bd2f5c4ea3..8efffd48cdf1 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -622,37 +622,23 @@ static __always_inline u32 raw_cpuid_get(struct cpuid_reg cpuid)
return *__cpuid_entry_get_reg(&entry, cpuid.reg);
}
-/* Mask kvm_cpu_caps for @leaf with the raw CPUID capabilities of this CPU. */
-static __always_inline void __kvm_cpu_cap_mask(unsigned int leaf)
+static __always_inline void kvm_cpu_cap_init(u32 leaf, u32 mask)
{
const struct cpuid_reg cpuid = x86_feature_cpuid(leaf * 32);
- reverse_cpuid_check(leaf);
+ /*
+ * For kernel-defined leafs, mask the boot CPU's pre-populated value.
+ * For KVM-defined leafs, explicitly set the leaf, as KVM is the one
+ * and only authority.
+ */
+ if (leaf < NCAPINTS)
+ kvm_cpu_caps[leaf] &= mask;
+ else
+ kvm_cpu_caps[leaf] = mask;
Hi,
I have an idea, how about we just initialize the kvm only leafs to 0xFFFFFFFF
and then treat them exactly in the same way as kernel regular leafs?
Then the user won't have to figure out (assuming that the user doesn't read
the comment, who does?) why we use mask as init value.
But if you prefer to leave it this way, I won't object either.
Huh, hadn't thought of that. It's a small code change, but I'm leaning towards
keeping the current code as we'd still need a comment to explain why KVM sets
all bits by default.
And in the unlikely case that we royally screw up and fail
to call kvm_cpu_cap_init() on a word, starting with 0xff would result in all
features in the uninitialized word being treated as supported.
+1
For posterity...
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 18ded0e682f2..6fcfb0fa4bd6 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -762,11 +762,7 @@ do { \
u32 kvm_cpu_cap_emulated = 0; \
u32 kvm_cpu_cap_synthesized = 0; \
\
- if (leaf < NCAPINTS) \
- kvm_cpu_caps[leaf] &= (mask); \
- else \
- kvm_cpu_caps[leaf] = (mask); \
- \
+ kvm_cpu_caps[leaf] &= (mask); \
kvm_cpu_caps[leaf] &= (raw_cpuid_get(cpuid) | \
kvm_cpu_cap_synthesized); \
kvm_cpu_caps[leaf] |= kvm_cpu_cap_emulated; \
@@ -780,7 +776,7 @@ do { \
void kvm_set_cpu_caps(void)
{
- memset(kvm_cpu_caps, 0, sizeof(kvm_cpu_caps));
+ memset(kvm_cpu_caps, 0xff, sizeof(kvm_cpu_caps));
BUILD_BUG_ON(sizeof(kvm_cpu_caps) - (NKVMCAPINTS * sizeof(*kvm_cpu_caps)) >
sizeof(boot_cpu_data.x86_capability));
