On 1/20/2025 6:02 PM, Srinivasan Shanmugam wrote:
> This commit adds an early return if no upstream bridge is found, setting
> the speed and width to PCI_SPEED_UNKNOWN and PCIE_LNK_WIDTH_UNKNOWN,
> respectively. This ensures that the function handles the absence of an
> upstream bridge gracefully.
>
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:6193 amdgpu_device_gpu_bandwidth()
> error: we previously assumed 'parent' could be null (see line 6180)
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> 6170 static void amdgpu_device_gpu_bandwidth(struct amdgpu_device *adev,
> 6171 enum pci_bus_speed *speed,
> 6172 enum pcie_link_width *width)
> 6173 {
> 6174 struct pci_dev *parent = adev->pdev;
> 6175
> 6176 if (!speed || !width)
> 6177 return;
> 6178
> 6179 parent = pci_upstream_bridge(parent);
> 6180 if (parent && parent->vendor == PCI_VENDOR_ID_ATI) {
> ^^^^^^
> If parent is NULL
>
> 6181 /* use the upstream/downstream switches internal to dGPU */
> 6182 *speed = pcie_get_speed_cap(parent);
> 6183 *width = pcie_get_width_cap(parent);
> 6184 while ((parent = pci_upstream_bridge(parent))) {
> 6185 if (parent->vendor == PCI_VENDOR_ID_ATI) {
> 6186 /* use the upstream/downstream switches internal to dGPU */
> 6187 *speed = pcie_get_speed_cap(parent);
> 6188 *width = pcie_get_width_cap(parent);
> 6189 }
> 6190 }
> 6191 } else {
> 6192 /* use the device itself */
> --> 6193 *speed = pcie_get_speed_cap(parent);
> ^^^^^^ Then we are toasted here.
I think the correct fix is to use adev->pdev here.
Thanks,
Lijo
>
> 6194 *width = pcie_get_width_cap(parent);
> 6195 }
> 6196 }
>
> Fixes: 9e424a5d9087 ("drm/amdgpu: cache gpu pcie link width")
> Cc: Christian König <christian.koenig@xxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 46af07faf8c8..749791a486bd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -6174,6 +6174,14 @@ static void amdgpu_device_gpu_bandwidth(struct amdgpu_device *adev,
> return;
>
> parent = pci_upstream_bridge(parent);
> +
> + if (!parent) {
> + /*Return early if no upstream bridge is found */
> + *speed = PCI_SPEED_UNKNOWN;
> + *width = PCIE_LNK_WIDTH_UNKNOWN;
> + return;
> + }
> +
> if (parent && parent->vendor == PCI_VENDOR_ID_ATI) {
> /* use the upstream/downstream switches internal to dGPU */
> *speed = pcie_get_speed_cap(parent);
