Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] amdgpu: KFD queue permanently hung when RAS is disabled due to bypassed GPU reset --
commit 8cb95b8549bf147ff94f08bcdd4e5a3aaa6812ae Author: Haotian Zhang <[email protected]> drm/amdgpu: fix NULL RAS context dereference in UMC page retirement amdgpu_umc_handle_bad_pages() and amdgpu_umc_do_page_retirement() take the RAS context from amdgpu_ras_get_context() and dereference it without checking for NULL. That context is NULL whenever RAS is disabled. > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c > index 34702e5cb1b6c..283180e3f2db2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c [ ... ] > @@ -217,6 +221,9 @@ static int amdgpu_umc_do_page_retirement(struct > amdgpu_device *adev, > kgd2kfd_set_sram_ecc_flag(adev->kfd.dev); > amdgpu_umc_handle_bad_pages(adev, ras_error_status); > > + if (!con) > + return AMDGPU_RAS_SUCCESS; > + [Severity: High] Does this early return ignore the reset parameter requested by the caller? When KFD encounters a poison consumption error and requests a fallback GPU reset by passing a non-zero reset flag, returning AMDGPU_RAS_SUCCESS here without handling the reset leaves the KFD queue permanently hung. This leaves the hardware in an unrecoverable state until the next system reboot. Should this trigger a generic GPU reset via amdgpu_device_gpu_recover() when the context is NULL? Also, is there a similar regression in amdgpu_umc_pasid_poison_handler()? if (adev->gmc.xgmi.connected_to_cpu || adev->gmc.is_app_apu) { if (reset) { /* MCA poison handler is only responsible for GPU reset, * let MCA notifier do page retirement. */ kgd2kfd_set_sram_ecc_flag(adev->kfd.dev); amdgpu_ras_reset_gpu(adev); } return ret; } If reset is requested and xgmi.connected_to_cpu is true, this unconditionally calls amdgpu_ras_reset_gpu(). Can this still trigger a NULL pointer dereference inside amdgpu_ras_reset_gpu() when RAS is disabled? > if ((err_data->ue_count || err_data->de_count) && > (reset || amdgpu_ras_is_rma(adev))) { > con->gpu_reset_flags |= reset; > amdgpu_ras_reset_gpu(adev); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
