[PATCH] drm/amdgpu: Fix the iounmap error of rmmio

[Christian König]

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 at amd.com>
>>> ---
>>>    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);
>>>    	}