Message ID | 20240517173926.965351-7-seanjc@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM: x86: CPUID overhaul, fixes, and caching | expand |
On Fri, 2024-05-17 at 10:38 -0700, Sean Christopherson wrote: > Refresh selftests' CPUID cache in the vCPU structure when querying a CPUID > entry so that tests don't consume stale data when KVM modifies CPUID as a > side effect to a completely unrelated change. E.g. KVM adjusts OSXSAVE in > response to CR4.OSXSAVE changes. > > Unnecessarily invoking KVM_GET_CPUID is suboptimal, but vcpu->cpuid exists > to simplify selftests development, not for performance reasons. And, > unfortunately, trying to handle the side effects in tests or other flows > is unpleasant, e.g. selftests could manually refresh if KVM_SET_SREGS is > successful, but that would still leave a gap with respect to guest CR4 > changes. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > .../testing/selftests/kvm/include/x86_64/processor.h | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h > index 8eb57de0b587..99aa3dfca16c 100644 > --- a/tools/testing/selftests/kvm/include/x86_64/processor.h > +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h > @@ -992,10 +992,17 @@ static inline struct kvm_cpuid2 *allocate_kvm_cpuid2(int nr_entries) > void vcpu_init_cpuid(struct kvm_vcpu *vcpu, const struct kvm_cpuid2 *cpuid); > void vcpu_set_hv_cpuid(struct kvm_vcpu *vcpu); > > +static inline void vcpu_get_cpuid(struct kvm_vcpu *vcpu) > +{ > + vcpu_ioctl(vcpu, KVM_GET_CPUID2, vcpu->cpuid); > +} > + > static inline struct kvm_cpuid_entry2 *__vcpu_get_cpuid_entry(struct kvm_vcpu *vcpu, > uint32_t function, > uint32_t index) > { > + vcpu_get_cpuid(vcpu); > + > return (struct kvm_cpuid_entry2 *)get_cpuid_entry(vcpu->cpuid, > function, index); > } > @@ -1016,7 +1023,7 @@ static inline int __vcpu_set_cpuid(struct kvm_vcpu *vcpu) > return r; > > /* On success, refresh the cache to pick up adjustments made by KVM. */ > - vcpu_ioctl(vcpu, KVM_GET_CPUID2, vcpu->cpuid); > + vcpu_get_cpuid(vcpu); > return 0; > } > > @@ -1026,7 +1033,7 @@ static inline void vcpu_set_cpuid(struct kvm_vcpu *vcpu) > vcpu_ioctl(vcpu, KVM_SET_CPUID2, vcpu->cpuid); > > /* Refresh the cache to pick up adjustments made by KVM. */ > - vcpu_ioctl(vcpu, KVM_GET_CPUID2, vcpu->cpuid); > + vcpu_get_cpuid(vcpu); > } > > void vcpu_set_cpuid_property(struct kvm_vcpu *vcpu, Hi, Indeed - fully agree with this, and again very sad that Intel made their CPUID dynamic - what a hack :( Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index 8eb57de0b587..99aa3dfca16c 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -992,10 +992,17 @@ static inline struct kvm_cpuid2 *allocate_kvm_cpuid2(int nr_entries) void vcpu_init_cpuid(struct kvm_vcpu *vcpu, const struct kvm_cpuid2 *cpuid); void vcpu_set_hv_cpuid(struct kvm_vcpu *vcpu); +static inline void vcpu_get_cpuid(struct kvm_vcpu *vcpu) +{ + vcpu_ioctl(vcpu, KVM_GET_CPUID2, vcpu->cpuid); +} + static inline struct kvm_cpuid_entry2 *__vcpu_get_cpuid_entry(struct kvm_vcpu *vcpu, uint32_t function, uint32_t index) { + vcpu_get_cpuid(vcpu); + return (struct kvm_cpuid_entry2 *)get_cpuid_entry(vcpu->cpuid, function, index); } @@ -1016,7 +1023,7 @@ static inline int __vcpu_set_cpuid(struct kvm_vcpu *vcpu) return r; /* On success, refresh the cache to pick up adjustments made by KVM. */ - vcpu_ioctl(vcpu, KVM_GET_CPUID2, vcpu->cpuid); + vcpu_get_cpuid(vcpu); return 0; } @@ -1026,7 +1033,7 @@ static inline void vcpu_set_cpuid(struct kvm_vcpu *vcpu) vcpu_ioctl(vcpu, KVM_SET_CPUID2, vcpu->cpuid); /* Refresh the cache to pick up adjustments made by KVM. */ - vcpu_ioctl(vcpu, KVM_GET_CPUID2, vcpu->cpuid); + vcpu_get_cpuid(vcpu); } void vcpu_set_cpuid_property(struct kvm_vcpu *vcpu,
Refresh selftests' CPUID cache in the vCPU structure when querying a CPUID entry so that tests don't consume stale data when KVM modifies CPUID as a side effect to a completely unrelated change. E.g. KVM adjusts OSXSAVE in response to CR4.OSXSAVE changes. Unnecessarily invoking KVM_GET_CPUID is suboptimal, but vcpu->cpuid exists to simplify selftests development, not for performance reasons. And, unfortunately, trying to handle the side effects in tests or other flows is unpleasant, e.g. selftests could manually refresh if KVM_SET_SREGS is successful, but that would still leave a gap with respect to guest CR4 changes. Signed-off-by: Sean Christopherson <seanjc@google.com> --- .../testing/selftests/kvm/include/x86_64/processor.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)