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

[Xingtao Yao (Fujitsu)]

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Wednesday, July 17, 2024 6:18 PM
> To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; mst@redhat.com;
> marcel.apfelbaum@gmail.com
> Cc: qemu-devel@nongnu.org; jonathan.cameron@huawei.com
> Subject: Re: [PATCH v2] pci-bridge: avoid linking a single downstream port 
> more
> than once
> 
> 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:
> https://lore.kernel.org/qemu-devel/OSZPR01MB6453BC61D2FF4035F18084EF8D
> DC2@OSZPR01MB6453.jpnprd01.prod.outlook.com
> > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> >
> > ---
> > V1[1] -> V2:
> >   - Move downstream port check forward
> >
> > [1]
> https://lore.kernel.org/qemu-devel/20240704033834.3362-1-yaoxt.fnst@fujitsu.co
> m
> > ---
> >   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.
Yes, thanks for your comment.
I think pcie_cap_init() may be better.
> 
> > +    }
> > +
> >       pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >       pcie_port_init_reg(d);