Re: [PATCH v2 2/2] radeon: use memcpy_to/fromio for UVD fw upload

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 04.06.21 um 05:04 schrieb Chen Li:
> I met a gpu addr bug recently and the kernel log
> tells me the pc is memcpy/memset and link register is
> radeon_uvd_resume.
>
> As we know, in some architectures, optimized memcpy/memset
> may not work well on device memory. Trival memcpy_toio/memset_io
> can fix this problem.
>
> BTW, amdgpu has already done it in:
> commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
> that's why it has no this issue on the same gpu and platform.
>
> Signed-off-by: Chen Li <chenli@uniontech.com>
> ---
>   drivers/gpu/drm/radeon/radeon_uvd.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> index 85a1f2c31749..0d6a5cfa2abf 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -288,7 +288,9 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	if (rdev->uvd.vcpu_bo == NULL)
>   		return -EINVAL;
>   
> -	memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
> +	memcpy_toio((void __iomem *)rdev->uvd.cpu_addr,
> +				rdev->uvd_fw->data,
> +				le32_to_cpu((__force __le32)rdev->uvd_fw->size));

The le32_to_cpu and coding style looks wrong here. The uvd_fw->size is 
always in CPU byte order and is used as such.

And please keep in mind that architectures where memcpy() on MMIO 
doesn't work in the kernel usually doesn't work in userspace as well.

So this is actually now necessary a valid workaround.

Christian.

>   
>   	size = radeon_bo_size(rdev->uvd.vcpu_bo);
>   	size -= rdev->uvd_fw->size;
> @@ -296,7 +298,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
>   	ptr = rdev->uvd.cpu_addr;
>   	ptr += rdev->uvd_fw->size;
>   
> -	memset(ptr, 0, size);
> +	memset_io((void __iomem *)ptr, 0, size);
>   
>   	return 0;
>   }