Public
>________________________________________
>From: Deucher, Alexander <[email protected]>
>Sent: Saturday, 11 July 2026 5:52 AM
>To: McRae, Geoffrey
>Cc: Koenig, Christian; [email protected]
>Subject: RE: [PATCH v2] drm/amdgpu: fix sysfs duplicate errors and incomplete
>unwind in ip_init
>
>Public
>
>> -----Original Message-----
>> From: McRae, Geoffrey <[email protected]>
>> Sent: Thursday, July 9, 2026 12:53 AM
>> To: Deucher, Alexander <[email protected]>
>> Cc: Koenig, Christian <[email protected]>; amd-
>> [email protected]; McRae, Geoffrey <[email protected]>
>> Subject: [PATCH v2] drm/amdgpu: fix sysfs duplicate errors and incomplete
>> unwind in ip_init
>>
>> When amdgpu_device_ip_init() fails partway through, IP blocks that had
>> completed sw_init() were never unwound, leaving sysfs files registered.
>> On the next device init attempt this produced duplicate filename errors:
>>
>> sysfs: cannot create duplicate filename '.../enforce_isolation'
>> sysfs: cannot create duplicate filename '.../sdma_reset_mask'
>> sysfs: cannot create duplicate filename '.../vcn_reset_mask'
>> sysfs: cannot create duplicate filename '.../jpeg_reset_mask'
>> sysfs: cannot create duplicate filename '.../vpe_reset_mask'
>> sysfs: cannot create duplicate filename '.../current_memory_partition'
>> sysfs: cannot create duplicate filename '.../available_memory_partition'
>>
>> Fix the init_failed unwind path in amdgpu_device_ip_init() to properly
>> reverse
>> all initialisation performed by the function:
>>
>> - Call sw_fini() in reverse order for all IP blocks where status.sw is
>> true, mirroring amdgpu_device_ip_fini().
>>
>> - For the GMC block, free the resources allocated during its early
>> hw_init phase in the correct reverse order: seq64, static CSA,
>> writeback buffer, and mem scratch page.
>>
>> - Free resources initialised after the IP block loop: ucode BO, IB
>> pool, KFD device, TTM buffer funcs, and the SRIOV vf2pf data
>> exchange work item.
>>
>> Additionally, fix amdgpu_gfx_sysfs_init() to properly unwind partially
>> registered gfx sysfs files on registration failure, and drop the
>> now-unnecessary
>> kobj.sd liveness guard from amdgpu_gfx_sysfs_fini() since
>> device_remove_file() is safe to call during sysfs teardown.
>>
>> Signed-off-by: Geoffrey McRae <[email protected]>
>> Cc: Alex Deucher <[email protected]>
>> Cc: Christian König <[email protected]>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 21
>> ++++++++++++++++++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 23 +++++++++++++++-----
>> --
>> 2 files changed, 37 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 70d07ca187a3..9630ad5f3048 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2491,6 +2491,27 @@ static int amdgpu_device_ip_init(struct
>> amdgpu_device *adev)
>> r = amdgpu_cper_init(adev);
>>
>> init_failed:
>> + if (r) {
>> + amdgpu_amdkfd_device_fini_sw(adev);
>> + amdgpu_ttm_disable_buffer_funcs(adev);
>> + if (amdgpu_sriov_vf(adev))
>> + amdgpu_virt_fini_data_exchange(adev);
>> + amdgpu_ucode_free_bo(adev);
>> + amdgpu_ib_pool_fini(adev);
>> + for (i = adev->num_ip_blocks - 1; i >= 0; i--) {
>> + if (!adev->ip_blocks[i].status.sw)
>> + continue;
>> + if (adev->ip_blocks[i].version->type ==
>> AMD_IP_BLOCK_TYPE_GMC) {
>> + amdgpu_seq64_fini(adev);
>> + amdgpu_free_static_csa(&adev-
>> >virt.csa_obj);
>> + amdgpu_device_wb_fini(adev);
>> + amdgpu_device_mem_scratch_fini(adev);
>> + }
>> + if (adev->ip_blocks[i].version->funcs->sw_fini)
>> + adev->ip_blocks[i].version->funcs-
>> >sw_fini(&adev->ip_blocks[i]);
>> + adev->ip_blocks[i].status.sw = false;
>> + }
>
>Also need to unwind all of the other stuff in that function (hw_init(), ucode
>bo allocation, etc.). Might be easier to call amdgpu_device_ip_fini() instead
>to cover all of the unwinding properly.
>
>Alex
>
ip_fini_early() and ip_fini() make assumptions about device state that don't
hold for a partially initialised device, for example, ip_fini() calls
amdgpu_doorbell_fini() from the GMC block, but doorbells are owned by the outer
amdgpu_device_init() and would be double-freed. The init_failed unwind is the
correct approach, we already handle sw_fini for all IP blocks, and the
associated resources (ucode BO, IB pool, WB, mem scratch, seq64, CSA, KFD, TTM
buffer funcs) are all properly torn down.
Geoff
>
>> + }
>>
>> return r;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> index 982b41606d48..0ab4c73c102a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> @@ -2182,23 +2182,32 @@ int amdgpu_gfx_sysfs_init(struct
>> amdgpu_device *adev)
>> }
>>
>> r = amdgpu_gfx_sysfs_isolation_shader_init(adev);
>> - if (r)
>> + if (r) {
>> dev_err(adev->dev, "failed to create isolation sysfs files");
>> + goto err_isolation_shader;
>> + }
>>
>> r = amdgpu_gfx_sysfs_reset_mask_init(adev);
>> - if (r)
>> + if (r) {
>> dev_err(adev->dev, "failed to create reset mask sysfs files");
>> + goto err_reset_mask;
>> + }
>> +
>> + return 0;
>>
>> +err_reset_mask:
>> + amdgpu_gfx_sysfs_reset_mask_fini(adev);
>> + amdgpu_gfx_sysfs_isolation_shader_fini(adev);
>> +err_isolation_shader:
>> + amdgpu_gfx_sysfs_xcp_fini(adev);
>> return r;
>> }
>>
>> void amdgpu_gfx_sysfs_fini(struct amdgpu_device *adev) {
>> - if (adev->dev->kobj.sd) {
>> - amdgpu_gfx_sysfs_xcp_fini(adev);
>> - amdgpu_gfx_sysfs_isolation_shader_fini(adev);
>> - amdgpu_gfx_sysfs_reset_mask_fini(adev);
>> - }
>> + amdgpu_gfx_sysfs_xcp_fini(adev);
>> + amdgpu_gfx_sysfs_isolation_shader_fini(adev);
>> + amdgpu_gfx_sysfs_reset_mask_fini(adev);
>> }
>>
>> static void amdgpu_gfx_reset_start_compute_scheds(struct amdgpu_device
>> *adev,
>> --
>> 2.43.0
