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 <[email protected]> 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/[email protected]?part=10
