Re: [PATCH 2/3] drm/amdgpu: Fix fault shader cleaner on MI3

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/amdgpu: physical XCC bit index used in
  gfx_v9_clean_shaders()
--

commit d2d8466691d7c4222ecf6651152915dffb5f88c8
Author: Kent Russell <kent.russell@amd.com>

drm/amdgpu: Fix fault shader cleaner on MI3

This commit adds MI300/MI350 register initialization definitions for the
fault shader cleaner and updates the dispatch dimension in
gfx_v9_clean_shaders() to cover all wave slots per SIMD.

> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_2.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_2.c
> index dc46133ed5068..95789f8d7018a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_2.c

[ ... ]

> @@ -731,6 +735,20 @@ static int gfx_v9_clean_shaders(struct amdgpu_device *adev)
>  		return 0;
>  	}
>  
> +	/* gfx943+ has different register layout compared to gfx942.*/
> +	switch (amdgpu_ip_version(adev, GC_HWIP, 0)) {
> +	case IP_VERSION(9, 4, 3):
> +	case IP_VERSION(9, 4, 4):
> +	case IP_VERSION(9, 5, 0):
> +		init_regs = gfx_v9_4_3_fault_cleaner_init_regs;
> +		init_regs_size = GFX_V9_4_3_FAULT_CLEANER_INIT_REGS_SIZE;
> +		break;
> +	default:
> +		init_regs = fault_cleaner_init_regs_aldebaran;
> +		init_regs_size = ARRAY_SIZE(fault_cleaner_init_regs_aldebaran);
> +		break;
> +	}
> +
>  	for_each_inst(xcc_id, adev->gfx.xcc_mask) {
>  		ring = &adev->gfx.compute_ring[xcc_id * adev->gfx.num_compute_rings];

[Severity: Medium]
Does iterating over adev->gfx.xcc_mask with for_each_inst() index
adev->gfx.compute_ring[] with a physical XCC bit position instead of a
logical XCC index?

In amdgpu_discovery_harvest_ip(), harvested GC instances are cleared from
adev->gfx.xcc_mask with ~BIT(inst), whereas gfx_v9_4_3_sw_init() and
amdgpu_gfx_run_cleaner_shader() populate and index adev->gfx.compute_ring[]
contiguously for logical XCCs 0 .. NUM_XCC(adev->gfx.xcc_mask) - 1.  On
MI300/MI350 parts with harvested lower-numbered XCCs, this loop skips the
logical compute rings at the harvested bit positions (leaving those active
XCCs uncleaned after a VM fault) and reads uninitialized compute_ring
entries at physical bit positions >= NUM_XCC(adev->gfx.xcc_mask).

Iterating from 0 to NUM_XCC(adev->gfx.xcc_mask) - 1 would match how
adev->gfx.compute_ring[] is indexed elsewhere.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112134340.4122401-1-kent.russell@amd.com?part=2