Messages in this thread | | | Date | Mon, 13 Nov 2023 16:03:43 +0800 | Subject | Re: [PATCH 6/9] KVM: x86: Update guest cpu_caps at runtime for dynamic CPUID-based features | From | Robert Hoo <> |
| |
On 11/11/2023 7:55 AM, Sean Christopherson wrote: > When updating guest CPUID entries to emulate runtime behavior, e.g. when > the guest enables a CR4-based feature that is tied to a CPUID flag, also > update the vCPU's cpu_caps accordingly. This will allow replacing all > usage of guest_cpuid_has() with guest_cpu_cap_has(). > > Take care not to update guest capabilities when KVM is updating CPUID > entries that *may* become the vCPU's CPUID, e.g. if userspace tries to set > bogus CPUID information. No extra call to update cpu_caps is needed as > the cpu_caps are initialized from the incoming guest CPUID, i.e. will > automatically get the updated values. > > Note, none of the features in question use guest_cpu_cap_has() at this > time, i.e. aside from settings bits in cpu_caps, this is a glorified nop. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/cpuid.c | 48 +++++++++++++++++++++++++++++++------------- > 1 file changed, 34 insertions(+), 14 deletions(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 36bd04030989..37a991439fe6 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -262,31 +262,48 @@ static u64 cpuid_get_supported_xcr0(struct kvm_cpuid_entry2 *entries, int nent) > return (best->eax | ((u64)best->edx << 32)) & kvm_caps.supported_xcr0; > } > > +static __always_inline void kvm_update_feature_runtime(struct kvm_vcpu *vcpu, > + struct kvm_cpuid_entry2 *entry, > + unsigned int x86_feature, > + bool has_feature) > +{ > + if (entry) > + cpuid_entry_change(entry, x86_feature, has_feature); > + > + if (vcpu) > + guest_cpu_cap_change(vcpu, x86_feature, has_feature); > +} > + > static void __kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *entries, > int nent) > { > struct kvm_cpuid_entry2 *best; > + struct kvm_vcpu *caps = vcpu;
u32 *caps = vcpu->arch.cpu_caps; and update guest_cpu_cap_set(), guest_cpu_cap_clear(), guest_cpu_cap_change() and guest_cpu_cap_restrict() to pass in vcpu->arch.cpu_caps instead of vcpu, since all of them merely refer to vcpu cap, rather than whole vcpu info.
Or, for simple change, here rename variable name "caps" --> "vcpu", to less reading confusion.
> + > + /* > + * Don't update vCPU capabilities if KVM is updating CPUID entries that > + * are coming in from userspace! > + */ > + if (entries != vcpu->arch.cpuid_entries) > + caps = NULL; > > best = cpuid_entry2_find(entries, nent, 1, KVM_CPUID_INDEX_NOT_SIGNIFICANT); > - if (best) { > - /* Update OSXSAVE bit */ > - if (boot_cpu_has(X86_FEATURE_XSAVE)) > - cpuid_entry_change(best, X86_FEATURE_OSXSAVE, > + > + if (boot_cpu_has(X86_FEATURE_XSAVE)) > + kvm_update_feature_runtime(caps, best, X86_FEATURE_OSXSAVE, > kvm_is_cr4_bit_set(vcpu, X86_CR4_OSXSAVE)); > > - cpuid_entry_change(best, X86_FEATURE_APIC, > - vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE); > + kvm_update_feature_runtime(caps, best, X86_FEATURE_APIC, > + vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE); > > - if (!kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_MISC_ENABLE_NO_MWAIT)) > - cpuid_entry_change(best, X86_FEATURE_MWAIT, > - vcpu->arch.ia32_misc_enable_msr & > - MSR_IA32_MISC_ENABLE_MWAIT); > - } > + if (!kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_MISC_ENABLE_NO_MWAIT)) > + kvm_update_feature_runtime(caps, best, X86_FEATURE_MWAIT, > + vcpu->arch.ia32_misc_enable_msr & MSR_IA32_MISC_ENABLE_MWAIT); > > best = cpuid_entry2_find(entries, nent, 7, 0); > - if (best && boot_cpu_has(X86_FEATURE_PKU)) > - cpuid_entry_change(best, X86_FEATURE_OSPKE, > - kvm_is_cr4_bit_set(vcpu, X86_CR4_PKE)); > + if (boot_cpu_has(X86_FEATURE_PKU)) > + kvm_update_feature_runtime(caps, best, X86_FEATURE_OSPKE, > + kvm_is_cr4_bit_set(vcpu, X86_CR4_PKE)); > > best = cpuid_entry2_find(entries, nent, 0xD, 0); > if (best) > @@ -353,6 +370,9 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > * Reset guest capabilities to userspace's guest CPUID definition, i.e. > * honor userspace's definition for features that don't require KVM or > * hardware management/support (or that KVM simply doesn't care about). > + * > + * Note, KVM has already done runtime updates on guest CPUID, i.e. this > + * will also correctly set runtime features in guest CPU capabilities. > */ > for (i = 0; i < NR_KVM_CPU_CAPS; i++) { > const struct cpuid_reg cpuid = reverse_cpuid[i];
| |