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

[Philippe Mathieu-Daudé]

Hi Yao,

On 17/7/24 10:56, Yao Xingtao via 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>
>
> ---
> 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;

Could pcie_cap_slot_init() be a good place to check for that?

Otherwise IMHO we should add a helper in "hw/pci/pcie.h" and
call it here, not duplicate this code in each model.

> +    }
> +
>       pci_bridge_initfn(d, TYPE_PCIE_BUS);
>       pcie_port_init_reg(d);