diff mbox series

selftests: KVM: Explicitly use movq to read xmm registers

Message ID 20210924005147.1122357-1-oupton@google.com (mailing list archive)
State New, archived
Headers show
Series selftests: KVM: Explicitly use movq to read xmm registers | expand

Commit Message

Oliver Upton Sept. 24, 2021, 12:51 a.m. UTC
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(-)

Comments

Ricardo Koller Sept. 24, 2021, 4:56 a.m. UTC | #1
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>
Paolo Bonzini Sept. 24, 2021, 6:33 a.m. UTC | #2
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
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 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;
 }