| Message ID | 20240725093819.15549-1-yaoxt.fnst@fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] pci-bridge: avoid linking a single downstream port more than once | expand |
ping. > -----Original Message----- > From: Yao Xingtao <yaoxt.fnst@fujitsu.com> > Sent: Thursday, July 25, 2024 5:38 PM > To: mst@redhat.com; marcel.apfelbaum@gmail.com > Cc: qemu-devel@nongnu.org; Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com> > Subject: [PATCH v4] pci-bridge: avoid linking a single downstream port more than > once > > 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> > > --- > V3[3] -> V4: > - make the error message more readable > - fix the downstream port check error > > V2[2] -> V3: > - Move this check into pcie_cap_init() > > V1[1] -> V2: > - Move downstream port check forward > > [1] > https://lore.kernel.org/qemu-devel/20240704033834.3362-1-yaoxt.fnst@fujitsu.co > m > [2] > https://lore.kernel.org/qemu-devel/20240717085621.55315-1-yaoxt.fnst@fujitsu.c > om > [3] > https://lore.kernel.org/qemu-devel/20240725032731.13032-1-yaoxt.fnst@fujitsu.c > om > --- > hw/pci/pcie.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 4b2f0805c6e0..1e53be1bc7c5 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -192,6 +192,13 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, > > assert(pci_is_express(dev)); > > + if ((type == PCI_EXP_TYPE_DOWNSTREAM || type == > PCI_EXP_TYPE_ROOT_PORT) && > + pcie_find_port_by_pn(pci_get_bus(dev), port)) { > + error_setg(errp, "The port %d is already in use, please select " > + "another port", port); > + return -EBUSY; > + } > + > pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset, > PCI_EXP_VER2_SIZEOF, errp); > if (pos < 0) { > -- > 2.41.0
On Thu, Jul 25, 2024 at 05:38:19AM -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: https://lore.kernel.org/qemu-devel/OSZPR01MB6453BC61D2FF4035F18084EF8DDC2@OSZPR01MB6453.jpnprd01.prod.outlook.com > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > --- > V3[3] -> V4: > - make the error message more readable > - fix the downstream port check error > > V2[2] -> V3: > - Move this check into pcie_cap_init() > > V1[1] -> V2: > - Move downstream port check forward > > [1] https://lore.kernel.org/qemu-devel/20240704033834.3362-1-yaoxt.fnst@fujitsu.com > [2] https://lore.kernel.org/qemu-devel/20240717085621.55315-1-yaoxt.fnst@fujitsu.com > [3] https://lore.kernel.org/qemu-devel/20240725032731.13032-1-yaoxt.fnst@fujitsu.com > --- > hw/pci/pcie.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 4b2f0805c6e0..1e53be1bc7c5 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -192,6 +192,13 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, > > assert(pci_is_express(dev)); > > + if ((type == PCI_EXP_TYPE_DOWNSTREAM || type == PCI_EXP_TYPE_ROOT_PORT) && > + pcie_find_port_by_pn(pci_get_bus(dev), port)) { > + error_setg(errp, "The port %d is already in use, please select " > + "another port", port); > + return -EBUSY; > + } > + > pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset, > PCI_EXP_VER2_SIZEOF, errp); > if (pos < 0) { But can't there be two functions of a multi-function device, sharing a port? > -- > 2.41.0
> -----Original Message----- > From: Michael S. Tsirkin <mst@redhat.com> > Sent: Tuesday, September 10, 2024 11:17 PM > To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com> > Cc: marcel.apfelbaum@gmail.com; qemu-devel@nongnu.org > Subject: Re: [PATCH v4] pci-bridge: avoid linking a single downstream port more > than once > > On Thu, Jul 25, 2024 at 05:38:19AM -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: > https://lore.kernel.org/qemu-devel/OSZPR01MB6453BC61D2FF4035F18084EF8D > DC2@OSZPR01MB6453.jpnprd01.prod.outlook.com > > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > > > --- > > V3[3] -> V4: > > - make the error message more readable > > - fix the downstream port check error > > > > V2[2] -> V3: > > - Move this check into pcie_cap_init() > > > > V1[1] -> V2: > > - Move downstream port check forward > > > > [1] > https://lore.kernel.org/qemu-devel/20240704033834.3362-1-yaoxt.fnst@fujitsu.co > m > > [2] > https://lore.kernel.org/qemu-devel/20240717085621.55315-1-yaoxt.fnst@fujitsu.c > om > > [3] > https://lore.kernel.org/qemu-devel/20240725032731.13032-1-yaoxt.fnst@fujitsu.c > om > > --- > > hw/pci/pcie.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > > index 4b2f0805c6e0..1e53be1bc7c5 100644 > > --- a/hw/pci/pcie.c > > +++ b/hw/pci/pcie.c > > @@ -192,6 +192,13 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, > > > > assert(pci_is_express(dev)); > > > > + if ((type == PCI_EXP_TYPE_DOWNSTREAM || type == > PCI_EXP_TYPE_ROOT_PORT) && > > + pcie_find_port_by_pn(pci_get_bus(dev), port)) { > > + error_setg(errp, "The port %d is already in use, please select " > > + "another port", port); > > + return -EBUSY; > > + } > > + > > pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset, > > PCI_EXP_VER2_SIZEOF, errp); > > if (pos < 0) { > > > But can't there be two functions of a multi-function device, > sharing a port? Good question. But I am not good at PCIe protocol, can anyone give me some advice when using the mulit-function feature? > > > -- > > 2.41.0
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 4b2f0805c6e0..1e53be1bc7c5 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -192,6 +192,13 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, assert(pci_is_express(dev)); + if ((type == PCI_EXP_TYPE_DOWNSTREAM || type == PCI_EXP_TYPE_ROOT_PORT) && + pcie_find_port_by_pn(pci_get_bus(dev), port)) { + error_setg(errp, "The port %d is already in use, please select " + "another port", port); + return -EBUSY; + } + pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset, PCI_EXP_VER2_SIZEOF, errp); if (pos < 0) {
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/OSZPR01MB6453BC61D2FF4035F18084EF8DDC2@OSZPR01MB6453.jpnprd01.prod.outlook.com Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> --- V3[3] -> V4: - make the error message more readable - fix the downstream port check error V2[2] -> V3: - Move this check into pcie_cap_init() V1[1] -> V2: - Move downstream port check forward [1] https://lore.kernel.org/qemu-devel/20240704033834.3362-1-yaoxt.fnst@fujitsu.com [2] https://lore.kernel.org/qemu-devel/20240717085621.55315-1-yaoxt.fnst@fujitsu.com [3] https://lore.kernel.org/qemu-devel/20240725032731.13032-1-yaoxt.fnst@fujitsu.com --- hw/pci/pcie.c | 7 +++++++ 1 file changed, 7 insertions(+)