[PATCH] drm/amdkfd: sparse: Fix warning in reading SDMA counters

[Felix Kuehling]

Am 2020-08-17 um 4:45 p.m. schrieb Mukul Joshi:
> Add __user annotation to fix related sparse warning while reading
> SDMA counters from userland.
>
> Reported-by: kernel test robot <lkp at intel.com>
> Signed-off-by: Mukul Joshi <mukul.joshi at amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index e0e60b0d0669..a6a4bbf99d9b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -157,19 +157,21 @@ int read_sdma_queue_counter(uint64_t q_rptr, uint64_t *val)
>  {
>  	int ret;
>  	uint64_t tmp = 0;
> +	uint64_t __user *sdma_rptr;

This name is misleading. You never point this to the actual rptr. Call
this sdma_usage_cntr or something similar.


>  
>  	if (!val)
>  		return -EINVAL;
>  	/*
>  	 * SDMA activity counter is stored at queue's RPTR + 0x8 location.
>  	 */
> -	if (!access_ok((const void __user *)(q_rptr +
> -					sizeof(uint64_t)), sizeof(uint64_t))) {
> +	sdma_rptr = (uint64_t *)(q_rptr + sizeof(uint64_t));

Should this cast to (uint64_t __user *)? A more elegant way to get the
offset would be:

    sdma_usage_cntr = (uint64_t __user *)q_rptr + 1;


> +
> +	if (!access_ok((const void __user *)sdma_rptr, sizeof(uint64_t))) {

Is the explicit cast really needed here? And as far as I can tell
get_user already checks access_ok. So this check is probably redundant.

Regards,
  Felix


>  		pr_err("Can't access sdma queue activity counter\n");
>  		return -EFAULT;
>  	}
>  
> -	ret = get_user(tmp, (uint64_t *)(q_rptr + sizeof(uint64_t)));
> +	ret = get_user(tmp, sdma_rptr);
>  	if (!ret) {
>  		*val = tmp;
>  	}