[PATCH 0/2] selftests: KVM: Fix some compiler warnings

[PATCH 0/2] selftests: KVM: Fix some compiler warnings

[Oliver Upton]

Building KVM selftests for arm64 using clang throws a couple compiler
warnings. This series addresses the warnings found insofar that
selftests can be built quietly for arm64 with clang.

Series applies cleanly to 5.15-rc2.

Oliver Upton (2):
  selftests: KVM: Fix compiler warning in demand_paging_test
  selftests: KVM: Fix 'asm-operand-width' warnings in steal_time.c

 tools/testing/selftests/kvm/demand_paging_test.c | 2 +-
 tools/testing/selftests/kvm/steal_time.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.33.0.464.g1972c5931b-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 1/2] selftests: KVM: Fix compiler warning in demand_paging_test

[Oliver Upton]

Building demand_paging_test.c with clang throws the following warning:

>> demand_paging_test.c:182:7: error: logical not is only applied to the left hand side of this bitwise operator [-Werror,-Wlogical-not-parentheses]
                  if (!pollfd[0].revents & POLLIN)

Silence the warning by placing the bitwise operation within parentheses.

Signed-off-by: Oliver Upton <oupton@google.com>
---
 tools/testing/selftests/kvm/demand_paging_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index e79c1b64977f..10edae425ab3 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -179,7 +179,7 @@ static void *uffd_handler_thread_fn(void *arg)
 			return NULL;
 		}
 
-		if (!pollfd[0].revents & POLLIN)
+		if (!(pollfd[0].revents & POLLIN))
 			continue;
 
 		r = read(uffd, &msg, sizeof(msg));
-- 
2.33.0.464.g1972c5931b-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 2/2] selftests: KVM: Fix 'asm-operand-width' warnings in steal_time.c

[Oliver Upton]

Building steal_time.c for arm64 with clang throws the following:

>> steal_time.c:130:22: error: value size does not match register size specified by the constraint and modifier [-Werror,-Wasm-operand-widths]
          : "=r" (ret) : "r" (func), "r" (arg) :
                              ^
>> steal_time.c:130:34: error: value size does not match register size specified by the constraint and modifier [-Werror,-Wasm-operand-widths]
          : "=r" (ret) : "r" (func), "r" (arg) :
                                          ^

Silence by casting operands to 64 bits.

Signed-off-by: Oliver Upton <oupton@google.com>
---
 tools/testing/selftests/kvm/steal_time.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
index ecec30865a74..eb75b31122c5 100644
--- a/tools/testing/selftests/kvm/steal_time.c
+++ b/tools/testing/selftests/kvm/steal_time.c
@@ -127,7 +127,7 @@ static int64_t smccc(uint32_t func, uint32_t arg)
 		"mov	x1, %2\n"
 		"hvc	#0\n"
 		"mov	%0, x0\n"
-	: "=r" (ret) : "r" (func), "r" (arg) :
+	: "=r" (ret) : "r" ((uint64_t)func), "r" ((uint64_t)arg) :
 	  "x0", "x1", "x2", "x3");
 
 	return ret;
-- 
2.33.0.464.g1972c5931b-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/2] selftests: KVM: Fix compiler warning in demand_paging_test

[Andrew Jones]

On Tue, Sep 21, 2021 at 01:01:19AM +0000, Oliver Upton wrote:
> Building demand_paging_test.c with clang throws the following warning:
> 
> >> demand_paging_test.c:182:7: error: logical not is only applied to the left hand side of this bitwise operator [-Werror,-Wlogical-not-parentheses]
>                   if (!pollfd[0].revents & POLLIN)
> 
> Silence the warning by placing the bitwise operation within parentheses.
> 
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
>  tools/testing/selftests/kvm/demand_paging_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index e79c1b64977f..10edae425ab3 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -179,7 +179,7 @@ static void *uffd_handler_thread_fn(void *arg)
>  			return NULL;
>  		}
>  
> -		if (!pollfd[0].revents & POLLIN)
> +		if (!(pollfd[0].revents & POLLIN))

That's a bug fix. If revents was e.g. POLLPRI then this logic
wouldn't have done what it's supposed to do. Maybe we should
better call out that this is a fix in the summary and add a
fixes tag?

Anyway,

Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew

>  			continue;
>  
>  		r = read(uffd, &msg, sizeof(msg));
> -- 
> 2.33.0.464.g1972c5931b-goog
> 

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] selftests: KVM: Fix 'asm-operand-width' warnings in steal_time.c

[Andrew Jones]

On Tue, Sep 21, 2021 at 01:01:20AM +0000, Oliver Upton wrote:
> Building steal_time.c for arm64 with clang throws the following:
> 
> >> steal_time.c:130:22: error: value size does not match register size specified by the constraint and modifier [-Werror,-Wasm-operand-widths]
>           : "=r" (ret) : "r" (func), "r" (arg) :
>                               ^
> >> steal_time.c:130:34: error: value size does not match register size specified by the constraint and modifier [-Werror,-Wasm-operand-widths]
>           : "=r" (ret) : "r" (func), "r" (arg) :
>                                           ^
> 
> Silence by casting operands to 64 bits.
> 
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
>  tools/testing/selftests/kvm/steal_time.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
> index ecec30865a74..eb75b31122c5 100644
> --- a/tools/testing/selftests/kvm/steal_time.c
> +++ b/tools/testing/selftests/kvm/steal_time.c
> @@ -127,7 +127,7 @@ static int64_t smccc(uint32_t func, uint32_t arg)
>  		"mov	x1, %2\n"
>  		"hvc	#0\n"
>  		"mov	%0, x0\n"
> -	: "=r" (ret) : "r" (func), "r" (arg) :
> +	: "=r" (ret) : "r" ((uint64_t)func), "r" ((uint64_t)arg) :

Actually, I think I'd rather fix this smccc implementation to match the
spec, which I think should be done like this

diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
index ecec30865a74..7da957259ce4 100644
--- a/tools/testing/selftests/kvm/steal_time.c
+++ b/tools/testing/selftests/kvm/steal_time.c
@@ -118,12 +118,12 @@ struct st_time {
        uint64_t st_time;
 };
 
-static int64_t smccc(uint32_t func, uint32_t arg)
+static int64_t smccc(uint32_t func, uint64_t arg)
 {
        unsigned long ret;
 
        asm volatile(
-               "mov    x0, %1\n"
+               "mov    w0, %w1\n"
                "mov    x1, %2\n"
                "hvc    #0\n"
                "mov    %0, x0\n"


Thanks,
drew

>  	  "x0", "x1", "x2", "x3");
>  
>  	return ret;
> -- 
> 2.33.0.464.g1972c5931b-goog
> 

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] selftests: KVM: Fix 'asm-operand-width' warnings in steal_time.c

[Paolo Bonzini]

On 21/09/21 09:19, Andrew Jones wrote:
> On Tue, Sep 21, 2021 at 01:01:20AM +0000, Oliver Upton wrote:
>> Building steal_time.c for arm64 with clang throws the following:
>>
>>>> steal_time.c:130:22: error: value size does not match register size specified by the constraint and modifier [-Werror,-Wasm-operand-widths]
>>            : "=r" (ret) : "r" (func), "r" (arg) :
>>                                ^
>>>> steal_time.c:130:34: error: value size does not match register size specified by the constraint and modifier [-Werror,-Wasm-operand-widths]
>>            : "=r" (ret) : "r" (func), "r" (arg) :
>>                                            ^
>>
>> Silence by casting operands to 64 bits.
>>
>> Signed-off-by: Oliver Upton <oupton@google.com>
>> ---
>>   tools/testing/selftests/kvm/steal_time.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
>> index ecec30865a74..eb75b31122c5 100644
>> --- a/tools/testing/selftests/kvm/steal_time.c
>> +++ b/tools/testing/selftests/kvm/steal_time.c
>> @@ -127,7 +127,7 @@ static int64_t smccc(uint32_t func, uint32_t arg)
>>   		"mov	x1, %2\n"
>>   		"hvc	#0\n"
>>   		"mov	%0, x0\n"
>> -	: "=r" (ret) : "r" (func), "r" (arg) :
>> +	: "=r" (ret) : "r" ((uint64_t)func), "r" ((uint64_t)arg) :
> 
> Actually, I think I'd rather fix this smccc implementation to match the
> spec, which I think should be done like this
> 
> diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
> index ecec30865a74..7da957259ce4 100644
> --- a/tools/testing/selftests/kvm/steal_time.c
> +++ b/tools/testing/selftests/kvm/steal_time.c
> @@ -118,12 +118,12 @@ struct st_time {
>          uint64_t st_time;
>   };
>   
> -static int64_t smccc(uint32_t func, uint32_t arg)
> +static int64_t smccc(uint32_t func, uint64_t arg)
>   {
>          unsigned long ret;
>   
>          asm volatile(
> -               "mov    x0, %1\n"
> +               "mov    w0, %w1\n"
>                  "mov    x1, %2\n"
>                  "hvc    #0\n"
>                  "mov    %0, x0\n"
> 

Agreed, can you send out a patch?  Thanks,

Paolo

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/2] selftests: KVM: Fix compiler warning in demand_paging_test

[Paolo Bonzini]

On 21/09/21 03:01, Oliver Upton wrote:
> Building demand_paging_test.c with clang throws the following warning:
> 
>>> demand_paging_test.c:182:7: error: logical not is only applied to the left hand side of this bitwise operator [-Werror,-Wlogical-not-parentheses]
>                    if (!pollfd[0].revents & POLLIN)
> 
> Silence the warning by placing the bitwise operation within parentheses.
> 
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
>   tools/testing/selftests/kvm/demand_paging_test.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index e79c1b64977f..10edae425ab3 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -179,7 +179,7 @@ static void *uffd_handler_thread_fn(void *arg)
>   			return NULL;
>   		}
>   
> -		if (!pollfd[0].revents & POLLIN)
> +		if (!(pollfd[0].revents & POLLIN))
>   			continue;
>   
>   		r = read(uffd, &msg, sizeof(msg));
> 

Queued (with small adjustments to the commit message and Cc: stable), 
thanks.

Paolo

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/2] selftests: KVM: Fix compiler warning in demand_paging_test

[Oliver Upton]

Hi Paolo,

On Tue, Sep 21, 2021 at 10:38 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 21/09/21 03:01, Oliver Upton wrote:
> > Building demand_paging_test.c with clang throws the following warning:
> >
> >>> demand_paging_test.c:182:7: error: logical not is only applied to the left hand side of this bitwise operator [-Werror,-Wlogical-not-parentheses]
> >                    if (!pollfd[0].revents & POLLIN)
> >
> > Silence the warning by placing the bitwise operation within parentheses.
> >
> > Signed-off-by: Oliver Upton <oupton@google.com>
> > ---
> >   tools/testing/selftests/kvm/demand_paging_test.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> > index e79c1b64977f..10edae425ab3 100644
> > --- a/tools/testing/selftests/kvm/demand_paging_test.c
> > +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> > @@ -179,7 +179,7 @@ static void *uffd_handler_thread_fn(void *arg)
> >                       return NULL;
> >               }
> >
> > -             if (!pollfd[0].revents & POLLIN)
> > +             if (!(pollfd[0].revents & POLLIN))
> >                       continue;
> >
> >               r = read(uffd, &msg, sizeof(msg));
> >
>
> Queued (with small adjustments to the commit message and Cc: stable),
> thanks.

I sent out a v2 of this series that addressed Drew's comments here and
picked up his suggested fix for 2/2. Would you like to queue that
version instead?

http://lore.kernel.org/kvm/20210921171121.2148982-1-oupton@google.com

--
Thanks,
Oliver
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm