AMD General

The original intent of this function is to collect the real hardware PCIe state 
from the AMD GPU's internal upstream/downstream switch ports.

However, under QEMU/VFIO passthrough, the guest-visible PCIe topology and PCI 
config space is virtualized.
QEMU/VFIO can intercept, emulate, or selectively forward PCI config accesses, 
so the upstream bridge information seen by the guest does not necessarily 
represent the real hardware topology or state.

Therefore, in a VFIO guest this function no longer has practical meaning for 
retrieving real HW PCIe state.
At most, it reads the virtual/emulated PCIe state exposed by QEMU/VFIO, which 
may not reflect the physical device (which depend on the implementation of 
QEMU/VFIO & kernel VFIO Driver source code)

So, I suggest that it is better to hide these interfaces under PT or SRIOV mode.

Best Regards,
Kevin

> -----Original Message-----
> From: amd-gfx <[email protected]> On Behalf Of Lijo Lazar
> Sent: Wednesday, June 17, 2026 17:35
> To: [email protected]
> Cc: Zhang, Hawking <[email protected]>; Deucher, Alexander
> <[email protected]>; Kamal, Asad <[email protected]>; Li,
> Candice <[email protected]>
> Subject: [PATCH] drm/amdgpu: Guard reads in pcie state readout
>
> Internal US/DS switch may not be exposed in passthrough. Guard the upstream 
> port
> reads to avoid a NULL dereference.
>
> Signed-off-by: Lijo Lazar <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 59 +++++++++++++++-------
>  1 file changed, 42 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
> b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
> index 72ea37dbfea8..5f1389901504 100644
> --- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
> +++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
> @@ -589,6 +589,29 @@ static struct aqua_reg_list pcie_reg_addrs[] = {
>       { smreg_0x1A380088, 6, DW_ADDR_INCR },  };
>
> +/*
> + * Return the GPU's internal US switch port, or NULL if it is not
> +visible
> + * (e.g. passthrough) or the EP is parented under an unrelated bridge.
> + */
> +static struct pci_dev *aqua_vanjaram_get_us_pdev(struct amdgpu_device
> +*adev) {
> +     struct pci_dev *ds_pdev, *us_pdev;
> +
> +     ds_pdev = pci_upstream_bridge(adev->pdev);
> +     if (!ds_pdev || ds_pdev->vendor != PCI_VENDOR_ID_ATI ||
> +         pci_pcie_type(ds_pdev) != PCI_EXP_TYPE_DOWNSTREAM)
> +             return NULL;
> +
> +     us_pdev = pci_upstream_bridge(ds_pdev);
> +     if (!us_pdev ||
> +         (us_pdev->vendor != PCI_VENDOR_ID_ATI &&
> +          us_pdev->vendor != PCI_VENDOR_ID_AMD) ||
> +         pci_pcie_type(us_pdev) != PCI_EXP_TYPE_UPSTREAM)
> +             return NULL;
> +
> +     return us_pdev;
> +}
> +
>  static ssize_t aqua_vanjaram_read_pcie_state(struct amdgpu_device *adev,
>                                            void *buf, size_t max_size)
>  {
> @@ -596,7 +619,7 @@ static ssize_t aqua_vanjaram_read_pcie_state(struct
> amdgpu_device *adev,
>       uint32_t start_addr, incrx, num_regs, szbuf;
>       struct amdgpu_regs_pcie_v1_0 *pcie_regs;
>       struct amdgpu_smn_reg_data *reg_data;
> -     struct pci_dev *us_pdev, *ds_pdev;
> +     struct pci_dev *us_pdev;
>       int aer_cap, r, n;
>
>       if (!buf || !max_size)
> @@ -628,25 +651,27 @@ static ssize_t aqua_vanjaram_read_pcie_state(struct
> amdgpu_device *adev,
>               }
>       }
>
> -     ds_pdev = pci_upstream_bridge(adev->pdev);
> -     us_pdev = pci_upstream_bridge(ds_pdev);
> +     us_pdev = aqua_vanjaram_get_us_pdev(adev);
> +     if (us_pdev) {
> +             pcie_capability_read_word(us_pdev, PCI_EXP_DEVSTA,
> +                                       &pcie_regs->device_status);
> +             pcie_capability_read_word(us_pdev, PCI_EXP_LNKSTA,
> +                                       &pcie_regs->link_status);
> +
> +             aer_cap = pci_find_ext_capability(us_pdev,
> PCI_EXT_CAP_ID_ERR);
> +             if (aer_cap) {
> +                     pci_read_config_dword(us_pdev,
> +                                           aer_cap + PCI_ERR_COR_STATUS,
> +                                           &pcie_regs->pcie_corr_err_status);
> +                     pci_read_config_dword(us_pdev,
> +                                           aer_cap + PCI_ERR_UNCOR_STATUS,
> +                                           
> &pcie_regs->pcie_uncorr_err_status);
> +             }
>
> -     pcie_capability_read_word(us_pdev, PCI_EXP_DEVSTA,
> -                               &pcie_regs->device_status);
> -     pcie_capability_read_word(us_pdev, PCI_EXP_LNKSTA,
> -                               &pcie_regs->link_status);
> -
> -     aer_cap = pci_find_ext_capability(us_pdev, PCI_EXT_CAP_ID_ERR);
> -     if (aer_cap) {
> -             pci_read_config_dword(us_pdev, aer_cap +
> PCI_ERR_COR_STATUS,
> -                                   &pcie_regs->pcie_corr_err_status);
> -             pci_read_config_dword(us_pdev, aer_cap +
> PCI_ERR_UNCOR_STATUS,
> -                                   &pcie_regs->pcie_uncorr_err_status);
> +             pci_read_config_dword(us_pdev, PCI_PRIMARY_BUS,
> +                                   &pcie_regs->sub_bus_number_latency);
>       }
>
> -     pci_read_config_dword(us_pdev, PCI_PRIMARY_BUS,
> -                           &pcie_regs->sub_bus_number_latency);
> -
>       pcie_reg_state->common_header.structure_size = szbuf;
>       pcie_reg_state->common_header.format_revision = 1;
>       pcie_reg_state->common_header.content_revision = 0;
> --
> 2.49.0

Reply via email to