hw/pci/pcie.c | 7 +++++++ 1 file changed, 7 insertions(+)
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>
---
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
---
hw/pci/pcie.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 4b2f0805c6e0..aa154ec4b054 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 (pci_is_express_downstream_port(dev) &&
+ pcie_find_port_by_pn(pci_get_bus(dev), port)) {
+ pos = -EBUSY;
+ error_setg(errp, "Can't link port, error %d", pos);
+ return pos;
+ }
+
pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset,
PCI_EXP_VER2_SIZEOF, errp);
if (pos < 0) {
--
2.41.0
On Wed, 24 Jul 2024 23:27:31 -0400 Yao Xingtao via <qemu-devel@nongnu.org> 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> > > --- > 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 > --- > hw/pci/pcie.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 4b2f0805c6e0..aa154ec4b054 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 (pci_is_express_downstream_port(dev) && > + pcie_find_port_by_pn(pci_get_bus(dev), port)) { > + pos = -EBUSY; > + error_setg(errp, "Can't link port, error %d", pos); perhaps, also mention what's wrong and suggest user how to fix mis-configuration > + return pos; > + } > + > pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset, > PCI_EXP_VER2_SIZEOF, errp); > if (pos < 0) {
> -----Original Message----- > From: Igor Mammedov <imammedo@redhat.com> > Sent: Thursday, July 25, 2024 4:36 PM > To: Yao Xingtao via <qemu-devel@nongnu.org> > Cc: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; mst@redhat.com; > marcel.apfelbaum@gmail.com > Subject: Re: [PATCH v3] pci-bridge: avoid linking a single downstream port more > than once > > On Wed, 24 Jul 2024 23:27:31 -0400 > Yao Xingtao via <qemu-devel@nongnu.org> 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> > > > > --- > > 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 > > --- > > hw/pci/pcie.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > > index 4b2f0805c6e0..aa154ec4b054 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 (pci_is_express_downstream_port(dev) && sorry, this check will not be valid since the dev->exp.exp_cap was not set. I will fix this bug in next revision. > > + pcie_find_port_by_pn(pci_get_bus(dev), port)) { > > + pos = -EBUSY; > > + error_setg(errp, "Can't link port, error %d", pos); > perhaps, also mention what's wrong and suggest user how to fix mis-configuration Thanks, will append in next revision. > > > + return pos; > > + } > > + > > pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset, > > PCI_EXP_VER2_SIZEOF, errp); > > if (pos < 0) {
© 2016 - 2024 Red Hat, Inc.