Am 18.03.24 um 02:43 schrieb Ma, Jun:
Hi Christian,
On 3/15/2024 3:16 PM, Christian König wrote:
Am 15.03.24 um 06:17 schrieb Ma Jun:
Setting the rmmio pointer to NULL to fix the following
iounmap error and calltrace.
iounmap: bad address 00000000d0b3631f
Fixes: 923f7a82d2e1 ("drm/amd/amdgpu: Fix potential ioremap() memory leaks in amdgpu_device_init()")
Signed-off-by: Ma Jun <Jun.Ma2@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 39dd76e57154..d65a6aabefbb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4383,6 +4383,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
unmap_memory:
iounmap(adev->rmmio);
+ adev->rmmio = NULL;
Well that doesn't looks correct to me. You seem to be working around
broken initialisation code here.
I got this error when I tried to rescan the gpu device after removing it.
Yeah, exactly that's the point. The whole smatch warning was just bogous
since amdgpu_device_fini_sw() will always cleanup the mapping.
So the whole patch 923f7a82d2e1 should be reverted instead.
Regards,
Christian.
Regards,
Ma Jun
return r;
}
@@ -4514,9 +4515,11 @@ void amdgpu_device_fini_sw(struct amdgpu_device *adev)
#endif
if (drm_dev_enter(adev_to_drm(adev), &idx)) {
Ok, well that alone doesn't look correct to me. The MMIO regions needs
to be unmapped independent if the driver is disconnected or not.
+ if (adev->rmmio) {
That looks just like a hack to me.
Regards,
Christian.
+ iounmap(adev->rmmio);
+ adev->rmmio = NULL;
+ }
- iounmap(adev->rmmio);
- adev->rmmio = NULL;
amdgpu_doorbell_fini(adev);
drm_dev_exit(idx);
}
