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
> Signed-off-by: Mukul Joshi
> ---
> 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;
> }
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
