Re: [PATCH] selftests: KVM: Explicitly use movq to read xmm registers

[Ricardo Koller <ricarkol@google.com>]

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>