diff mbox series

[v2,06/49] KVM: selftests: Refresh vCPU CPUID cache in __vcpu_get_cpuid_entry()

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

Commit Message

Sean Christopherson May 17, 2024, 5:38 p.m. UTC
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(-)

Comments

Maxim Levitsky July 5, 2024, 12:59 a.m. UTC | #1
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 mbox series

Patch

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,