Re: [PATCH 10/13] drm/amdgpu: Catch register access while GMUX keeps the GPU off

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/amdgpu: missing skip_hw_access check in amdgpu_device_indirect_rreg_ext()
--

commit 68a505d63bba52b15e530c1a5a5c68524afaf59d
Author: Andre Eikmeyer <dev@deq.rocks>

drm/amdgpu: Catch register access while GMUX keeps the GPU off

This commit tracks when the GPU is powered off via GMUX during runtime
suspend by setting adev->gmux_off, and skips and reports register accesses in
amdgpu_device_skip_hw_access() and indirect PCIe register access helpers to
prevent hanging the PEG link.

[ ... ]

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
> index 7468855c16a25..bb5dbeb7fa721 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c
> @@ -594,6 +594,9 @@ u32 amdgpu_device_indirect_rreg(struct amdgpu_device *adev, u32 reg_addr)
>  	void __iomem *pcie_data_offset;
>  	u32 r;
>  
> +	if (amdgpu_device_skip_hw_access(adev))
> +		return 0;
> +
>  	pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
>  	pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
>  
> @@ -677,6 +680,12 @@ u64 amdgpu_device_indirect_rreg64(struct amdgpu_device *adev, u32 reg_addr)
>  	void __iomem *pcie_data_offset;
>  	u64 r;
>  
> +	if (amdgpu_device_skip_hw_access(adev))
> +		return 0;
> +
> +	if (amdgpu_device_skip_hw_access(adev))
> +		return 0;

[Severity: High]
Was one of these duplicate amdgpu_device_skip_hw_access(adev) checks in
amdgpu_device_indirect_rreg64() intended for amdgpu_device_indirect_rreg_ext()
just above it?

Currently amdgpu_device_indirect_rreg_ext() in
drivers/gpu/drm/amd/amdgpu/amdgpu_reg_access.c is the only indirect PCIe
register accessor without this guard:

u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device *adev, u64 reg_addr)
{
	unsigned long flags, pcie_index, pcie_index_hi, pcie_data;
	u32 r;
	void __iomem *pcie_index_offset;
	void __iomem *pcie_index_hi_offset;
	void __iomem *pcie_data_offset;

	if (unlikely(!adev->nbio.funcs)) {
		...
	}
	...
	spin_lock_irqsave(&adev->reg.pcie.lock, flags);
	pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
	pcie_data_offset = (void __iomem *)adev->rmmio + pcie_data * 4;
	...
	writel(reg_addr, pcie_index_offset);
	readl(pcie_index_offset);

If RREG32_PCIE_EXT() -> amdgpu_reg_pcie_ext_rd32() ->
amdgpu_device_indirect_rreg_ext() is called while adev->gmux_off or
adev->no_hw_access is true, won't it still perform MMIO accesses on
adev->rmmio?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025040046.1602-1-atharvatiwarilinuxdev@gmail.com?part=10