Re: [PATCH] drm/[amdgpu|radeon]: fix memset on io mem

[Chen Li <chenli@uniontech.com>]

On Wed, 16 Dec 2020 15:59:37 +0800,
Christian König wrote:
> 
> Am 16.12.20 um 06:41 schrieb Chen Li:
> > When using e8860(gcn1) on arm64, the kernel crashed on drm/radeon:
> > 
> > [   11.240414] pc : __memset+0x4c/0x188
> > [   11.244101] lr : radeon_uvd_get_create_msg+0x114/0x1d0 [radeon]
> > [   11.249995] sp : ffff00000d7eb700
> > [   11.253295] x29: ffff00000d7eb700 x28: ffff8001f632a868
> > [   11.258585] x27: 0000000000040000 x26: ffff00000de00000
> > [   11.263875] x25: 0000000000000125 x24: 0000000000000001
> > [   11.269168] x23: 0000000000000000 x22: 0000000000000005
> > [   11.274459] x21: ffff00000df24000 x20: ffff8001f74b4000
> > [   11.279753] x19: 0000000000124000 x18: 0000000000000020
> > [   11.285043] x17: 0000000000000000 x16: 0000000000000000
> > [   11.290336] x15: ffff000009309000 x14: ffffffffffffffff
> > [   11.290340] x13: ffff0000094b6f88 x12: ffff0000094b6bd2
> > [   11.290343] x11: ffff00000d7eb700 x10: ffff00000d7eb700
> > [   11.306246] x9 : ffff00000d7eb700 x8 : ffff00000df2402c
> > [   11.306254] x7 : 0000000000000000 x6 : ffff0000094b626a
> > [   11.306257] x5 : 0000000000000000 x4 : 0000000000000004
> > [   11.306262] x3 : ffffffffffffffff x2 : 0000000000000fd4
> > [   11.306265] x1 : 0000000000000000 x0 : ffff00000df2402c
> > [   11.306272] Call trace:
> > [   11.306316]  __memset+0x4c/0x188
> > [   11.306638]  uvd_v1_0_ib_test+0x70/0x1c0 [radeon]
> > [   11.306758]  radeon_ib_ring_tests+0x54/0xe0 [radeon]
> > [   11.309961] IPv6: ADDRCONF(NETDEV_UP): enp5s0f0: link is not ready
> > [   11.354628]  radeon_device_init+0x53c/0xbdc [radeon]
> > [   11.354693]  radeon_driver_load_kms+0x6c/0x1b0 [radeon]
> > [   11.364788]  drm_dev_register+0x130/0x1c0
> > [   11.364794]  drm_get_pci_dev+0x8c/0x14c
> > [   11.372704]  radeon_pci_probe+0xb0/0x110 [radeon]
> > [   11.372715]  local_pci_probe+0x3c/0xb0
> > [   11.381129]  pci_device_probe+0x114/0x1b0
> > [   11.385121]  really_probe+0x23c/0x400
> > [   11.388757]  driver_probe_device+0xdc/0x130
> > [   11.392921]  __driver_attach+0x128/0x150
> > [   11.396826]  bus_for_each_dev+0x70/0xbc
> > [   11.400643]  driver_attach+0x20/0x2c
> > [   11.404201]  bus_add_driver+0x160/0x260
> > [   11.408019]  driver_register+0x74/0x120
> > [   11.411837]  __pci_register_driver+0x40/0x50
> > [   11.416149]  radeon_init+0x78/0x1000 [radeon]
> > [   11.420489]  do_one_initcall+0x54/0x154
> > [   11.424310]  do_init_module+0x54/0x260
> > [   11.428041]  load_module+0x1ccc/0x20b0
> > [   11.431773]  __se_sys_finit_module+0xac/0x10c
> > [   11.436109]  __arm64_sys_finit_module+0x18/0x20
> > [   11.440622]  el0_svc_common+0x70/0x164
> > [   11.444353]  el0_svc_handler+0x2c/0x80
> > [   11.448084]  el0_svc+0x8/0xc
> > [   11.450954] Code: d65f03c0 cb0803e4 f2400c84 54000080 (a9001d07)
> > 
> > Obviously, the __memset call is generated by gcc(8.3.1). It optimizes
> > this for loop into memset. But this may break, because dest here is
> > cpu_addr mapped to io mem. So, just invoke `memset_io` directly, which
> > do solve the problem here.
> 
> Well interesting problem you stumbled over here, but the solution is quite a
> hack.

Hi, Christian. I'm not sure why this change is a hack here. I cannot see the problem and wll be grateful if you give more explainations.
> 
> For amdgpu I suggest that we allocate the UVD message in GTT instead of VRAM
> since we don't have the hardware restriction for that on the new generations.
> 
Thanks, I will try to dig into deeper. But what's the "hardware restriction" meaning here? I'm not familiar with video driver stack and amd gpu, sorry.
> For radeon I think the better approach would be to convert the direct memory
> writes into calls to writel().
Ok, so you mean the more proper way is to use writel instead of memset_io?

> BTW: How does userspace work on arm64 then? The driver stack usually only works
> if mmio can be mapped directly.
I also post two usespace issue on mesa, and you may be interested with them: 
 https://gitlab.freedesktop.org/mesa/mesa/-/issues/3954
 https://gitlab.freedesktop.org/mesa/mesa/-/issues/3951
I paste some virtual memory map in userspace there. (and the two problems do bother me quite a long time.)
> Regards,
> Christian.
> 
> > 
> > Signed-off-by: chenli <chenli@uniontech.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 6 ++----
> >   drivers/gpu/drm/radeon/radeon_uvd.c     | 6 ++----
> >   2 files changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> > index 7c5b60e53482..4dccde7a9e83 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> > @@ -1187,8 +1187,7 @@ int amdgpu_uvd_get_create_msg(struct amdgpu_ring *ring, uint32_t handle,
> >   	msg[8] = cpu_to_le32(0x00000440);
> >   	msg[9] = cpu_to_le32(0x00000000);
> >   	msg[10] = cpu_to_le32(0x01b37000);
> > -	for (i = 11; i < 1024; ++i)
> > -		msg[i] = cpu_to_le32(0x0);
> > +	memset_io(&msg[i], 0x0, 1013 * sizeof(uint32_t));
> >     	return amdgpu_uvd_send_msg(ring, bo, true, fence);
> >   }
> > @@ -1212,8 +1211,7 @@ int amdgpu_uvd_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
> >   	msg[1] = cpu_to_le32(0x00000002);
> >   	msg[2] = cpu_to_le32(handle);
> >   	msg[3] = cpu_to_le32(0x00000000);
> > -	for (i = 4; i < 1024; ++i)
> > -		msg[i] = cpu_to_le32(0x0);
> > +	memset_io(&msg[i], 0x0, 1020 * sizeof(uint32_t));
> >     	return amdgpu_uvd_send_msg(ring, bo, direct, fence);
> >   }
> > diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
> > index 57fb3eb3a4b4..2e2e737c4706 100644
> > --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> > +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> > @@ -802,8 +802,7 @@ int radeon_uvd_get_create_msg(struct radeon_device *rdev, int ring,
> >   	msg[8] = cpu_to_le32(0x00000440);
> >   	msg[9] = cpu_to_le32(0x00000000);
> >   	msg[10] = cpu_to_le32(0x01b37000);
> > -	for (i = 11; i < 1024; ++i)
> > -		msg[i] = cpu_to_le32(0x0);
> > +	memset_io(&msg[i], 0x0, 1013 * sizeof(uint32_t));
> >     	r = radeon_uvd_send_msg(rdev, ring, addr, fence);
> >   	radeon_bo_unreserve(rdev->uvd.vcpu_bo);
> > @@ -831,8 +830,7 @@ int radeon_uvd_get_destroy_msg(struct radeon_device *rdev, int ring,
> >   	msg[1] = cpu_to_le32(0x00000002);
> >   	msg[2] = cpu_to_le32(handle);
> >   	msg[3] = cpu_to_le32(0x00000000);
> > -	for (i = 4; i < 1024; ++i)
> > -		msg[i] = cpu_to_le32(0x0);
> > +	memset_io(&msg[i], 0x0, 1020 * sizeof(uint32_t));
> >     	r = radeon_uvd_send_msg(rdev, ring, addr, fence);
> >   	radeon_bo_unreserve(rdev->uvd.vcpu_bo);
> 
> 
> 


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel