Re: [PATCH v2] pci-bridge: avoid linking a single downstream port more than once

[Michael S. Tsirkin]

On Wed, Jul 17, 2024 at 04:56:21AM -0400, Yao Xingtao wrote:
> Since the downstream port is not checked, two slots can be linked to
> a single port. However, this can prevent the driver from detecting the
> device properly.
> 
> It is necessary to ensure that a downstream port is not linked more than
> once.
> 
> Links: 
> OSZPR01MB6453BC61D2FF4035F18084EF8DDC2@OSZPR01MB6453.jpnprd01.prod.outlook.com">https://lore.kernel.org/qemu-devel/OSZPR01MB6453BC61D2FF4035F18084EF8DDC2@OSZPR01MB6453.jpnprd01.prod.outlook.com
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>

You also need to take ARI into account.
That can look like slot != 0.

> ---
> V1[1] -> V2:
>  - Move downstream port check forward
> 
> [1] 
> 20240704033834.3362-1-yaoxt.fnst@fujitsu.com">https://lore.kernel.org/qemu-devel/20240704033834.3362-1-yaoxt.fnst@fujitsu.com
> ---
>  hw/pci-bridge/cxl_downstream.c     | 5 +++++
>  hw/pci-bridge/pcie_root_port.c     | 5 +++++
>  hw/pci-bridge/xio3130_downstream.c | 5 +++++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/hw/pci-bridge/cxl_downstream.c b/hw/pci-bridge/cxl_downstream.c
> index 742da07a015a..af81ddfeec13 100644
> --- a/hw/pci-bridge/cxl_downstream.c
> +++ b/hw/pci-bridge/cxl_downstream.c
> @@ -142,6 +142,11 @@ static void cxl_dsp_realize(PCIDevice *d, Error **errp)
>      MemoryRegion *component_bar = &cregs->component_registers;
>      int rc;
>  
> +    if (pcie_find_port_by_pn(pci_get_bus(d), p->port) != NULL) {
> +        error_setg(errp, "Can't link port, error %d", -EBUSY);
> +        return;
> +    }
> +
>      pci_bridge_initfn(d, TYPE_PCIE_BUS);
>      pcie_port_init_reg(d);
>  
> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> index 09a34786bc62..a540204bda27 100644
> --- a/hw/pci-bridge/pcie_root_port.c
> +++ b/hw/pci-bridge/pcie_root_port.c
> @@ -67,6 +67,11 @@ static void rp_realize(PCIDevice *d, Error **errp)
>      PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
>      int rc;
>  
> +    if (pcie_find_port_by_pn(pci_get_bus(d), p->port) != NULL) {
> +        error_setg(errp, "Can't link port, error %d", -EBUSY);
> +        return;
> +    }
> +
>      pci_config_set_interrupt_pin(d->config, 1);
>      if (d->cap_present & QEMU_PCIE_CAP_CXL) {
>          pci_bridge_initfn(d, TYPE_CXL_BUS);
> diff --git a/hw/pci-bridge/xio3130_downstream.c 
> b/hw/pci-bridge/xio3130_downstream.c
> index 907d5105b019..63f6baa615fd 100644
> --- a/hw/pci-bridge/xio3130_downstream.c
> +++ b/hw/pci-bridge/xio3130_downstream.c
> @@ -69,6 +69,11 @@ static void xio3130_downstream_realize(PCIDevice *d, Error 
> **errp)
>      PCIESlot *s = PCIE_SLOT(d);
>      int rc;
>  
> +    if (pcie_find_port_by_pn(pci_get_bus(d), p->port) != NULL) {
> +        error_setg(errp, "Can't link port, error %d", -EBUSY);
> +        return;
> +    }
> +
>      pci_bridge_initfn(d, TYPE_PCIE_BUS);
>      pcie_port_init_reg(d);
>  
> -- 
> 2.37.3