On Thu, Feb 13, 2025 at 12:57 PM Srinivasan Shanmugam
<srinivasan.shanmugam@xxxxxxx> wrote:
>
> By adding these NULL pointer checks and improving error handling, we can
> prevent crashes when the enforce_isolation sysfs file is accessed on
> non-supported systems.
Can you clarify what the issue is? The code seems correct as is.
With this change the driver will start logging errors for all parts
that don't support enforce isolation yet which is misleading.
Alex
>
> Cc: Christian König <christian.koenig@xxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 27f5318c3a26..bf0bf6382b65 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -1777,20 +1777,27 @@ static int amdgpu_gfx_sysfs_isolation_shader_init(struct amdgpu_device *adev)
> {
> int r;
>
> + if (!adev->gfx.enable_cleaner_shader)
> + return -EINVAL;
> +
> r = device_create_file(adev->dev, &dev_attr_enforce_isolation);
> if (r)
> return r;
> - if (adev->gfx.enable_cleaner_shader)
> - r = device_create_file(adev->dev, &dev_attr_run_cleaner_shader);
>
> - return r;
> + r = device_create_file(adev->dev, &dev_attr_run_cleaner_shader);
> + if (r)
> + return r;
> +
> + return 0;
> }
>
> static void amdgpu_gfx_sysfs_isolation_shader_fini(struct amdgpu_device *adev)
> {
> + if (!adev->gfx.enable_cleaner_shader)
> + return;
> +
> device_remove_file(adev->dev, &dev_attr_enforce_isolation);
> - if (adev->gfx.enable_cleaner_shader)
> - device_remove_file(adev->dev, &dev_attr_run_cleaner_shader);
> + device_remove_file(adev->dev, &dev_attr_run_cleaner_shader);
> }
>
> static int amdgpu_gfx_sysfs_reset_mask_init(struct amdgpu_device *adev)
> --
> 2.34.1
>
