Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH] selftests: KVM: Explicitly use movq to read xmm registers
@ 2021-09-24  0:51 Oliver Upton
  2021-09-24  4:56 ` Ricardo Koller
  0 siblings, 1 reply; 3+ messages in thread
From: Oliver Upton @ 2021-09-24  0:51 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, Jim Mattson, Ricardo Koller, Sean Christopherson,
	Oliver Upton

Compiling the KVM selftests with clang emits the following warning:

>> include/x86_64/processor.h:297:25: error: variable 'xmm0' is uninitialized when used here [-Werror,-Wuninitialized]
>>                return (unsigned long)xmm0;

where xmm0 is accessed via an uninitialized register variable.

Indeed, this is a misuse of register variables, which really should only
be used for specifying register constraints on variables passed to
inline assembly. Rather than attempting to read xmm registers via
register variables, just explicitly perform the movq from the desired
xmm register.

Fixes: 783e9e51266e ("kvm: selftests: add API testing infrastructure")
Signed-off-by: Oliver Upton <oupton@google.com>
---
 .../selftests/kvm/include/x86_64/processor.h  | 34 +++++++++----------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 242ae8e09a65..eba8bd08293e 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -312,37 +312,37 @@ static inline void set_xmm(int n, unsigned long val)
 	}
 }
 
-typedef unsigned long v1di __attribute__ ((vector_size (8)));
+#define GET_XMM(__xmm)							\
+({									\
+	unsigned long __val;						\
+	asm volatile("movq %%"#__xmm", %0" : "=r"(__val) : : #__xmm);	\
+	__val;								\
+})
+
 static inline unsigned long get_xmm(int n)
 {
 	assert(n >= 0 && n <= 7);
 
-	register v1di xmm0 __asm__("%xmm0");
-	register v1di xmm1 __asm__("%xmm1");
-	register v1di xmm2 __asm__("%xmm2");
-	register v1di xmm3 __asm__("%xmm3");
-	register v1di xmm4 __asm__("%xmm4");
-	register v1di xmm5 __asm__("%xmm5");
-	register v1di xmm6 __asm__("%xmm6");
-	register v1di xmm7 __asm__("%xmm7");
 	switch (n) {
 	case 0:
-		return (unsigned long)xmm0;
+		return GET_XMM(xmm0);
 	case 1:
-		return (unsigned long)xmm1;
+		return GET_XMM(xmm1);
 	case 2:
-		return (unsigned long)xmm2;
+		return GET_XMM(xmm2);
 	case 3:
-		return (unsigned long)xmm3;
+		return GET_XMM(xmm3);
 	case 4:
-		return (unsigned long)xmm4;
+		return GET_XMM(xmm4);
 	case 5:
-		return (unsigned long)xmm5;
+		return GET_XMM(xmm5);
 	case 6:
-		return (unsigned long)xmm6;
+		return GET_XMM(xmm6);
 	case 7:
-		return (unsigned long)xmm7;
+		return GET_XMM(xmm7);
 	}
+
+	/* never reached */
 	return 0;
 }
 
-- 
2.33.0.685.g46640cef36-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] selftests: KVM: Explicitly use movq to read xmm registers
  2021-09-24  0:51 [PATCH] selftests: KVM: Explicitly use movq to read xmm registers Oliver Upton
@ 2021-09-24  4:56 ` Ricardo Koller
  2021-09-24  6:33   ` Paolo Bonzini
  0 siblings, 1 reply; 3+ messages in thread
From: Ricardo Koller @ 2021-09-24  4:56 UTC (permalink / raw)
  To: Oliver Upton; +Cc: kvm, Paolo Bonzini, Jim Mattson, Sean Christopherson

On Fri, Sep 24, 2021 at 12:51:47AM +0000, Oliver Upton wrote:
> Compiling the KVM selftests with clang emits the following warning:
> 
> >> include/x86_64/processor.h:297:25: error: variable 'xmm0' is uninitialized when used here [-Werror,-Wuninitialized]
> >>                return (unsigned long)xmm0;
> 
> where xmm0 is accessed via an uninitialized register variable.
> 
> Indeed, this is a misuse of register variables, which really should only
> be used for specifying register constraints on variables passed to
> inline assembly. Rather than attempting to read xmm registers via
> register variables, just explicitly perform the movq from the desired
> xmm register.
> 
> Fixes: 783e9e51266e ("kvm: selftests: add API testing infrastructure")
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
>  .../selftests/kvm/include/x86_64/processor.h  | 34 +++++++++----------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index 242ae8e09a65..eba8bd08293e 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -312,37 +312,37 @@ static inline void set_xmm(int n, unsigned long val)
>  	}
>  }
>  
> -typedef unsigned long v1di __attribute__ ((vector_size (8)));
> +#define GET_XMM(__xmm)							\
> +({									\
> +	unsigned long __val;						\
> +	asm volatile("movq %%"#__xmm", %0" : "=r"(__val) : : #__xmm);	\
> +	__val;								\
> +})
> +
>  static inline unsigned long get_xmm(int n)
>  {
>  	assert(n >= 0 && n <= 7);
>  
> -	register v1di xmm0 __asm__("%xmm0");
> -	register v1di xmm1 __asm__("%xmm1");
> -	register v1di xmm2 __asm__("%xmm2");
> -	register v1di xmm3 __asm__("%xmm3");
> -	register v1di xmm4 __asm__("%xmm4");
> -	register v1di xmm5 __asm__("%xmm5");
> -	register v1di xmm6 __asm__("%xmm6");
> -	register v1di xmm7 __asm__("%xmm7");
>  	switch (n) {
>  	case 0:
> -		return (unsigned long)xmm0;
> +		return GET_XMM(xmm0);
>  	case 1:
> -		return (unsigned long)xmm1;
> +		return GET_XMM(xmm1);
>  	case 2:
> -		return (unsigned long)xmm2;
> +		return GET_XMM(xmm2);
>  	case 3:
> -		return (unsigned long)xmm3;
> +		return GET_XMM(xmm3);
>  	case 4:
> -		return (unsigned long)xmm4;
> +		return GET_XMM(xmm4);
>  	case 5:
> -		return (unsigned long)xmm5;
> +		return GET_XMM(xmm5);
>  	case 6:
> -		return (unsigned long)xmm6;
> +		return GET_XMM(xmm6);
>  	case 7:
> -		return (unsigned long)xmm7;
> +		return GET_XMM(xmm7);
>  	}
> +
> +	/* never reached */
>  	return 0;
>  }
>  
> -- 
> 2.33.0.685.g46640cef36-goog
> 

Reviewed-by: Ricardo Koller <ricarkol@google.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] selftests: KVM: Explicitly use movq to read xmm registers
  2021-09-24  4:56 ` Ricardo Koller
@ 2021-09-24  6:33   ` Paolo Bonzini
  0 siblings, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2021-09-24  6:33 UTC (permalink / raw)
  To: Ricardo Koller, Oliver Upton; +Cc: kvm, Jim Mattson, Sean Christopherson

On 24/09/21 06:56, Ricardo Koller wrote:
> Reviewed-by: Ricardo Koller<ricarkol@google.com>

Queued, thanks for the patch and the review!

Paolo


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-09-24  6:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-24  0:51 [PATCH] selftests: KVM: Explicitly use movq to read xmm registers Oliver Upton
2021-09-24  4:56 ` Ricardo Koller
2021-09-24  6:33   ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox