[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/pci/pci_bridge: ensure PCIe slots have only one slot
From: |
Roman Kagan |
Subject: |
Re: [PATCH] hw/pci/pci_bridge: ensure PCIe slots have only one slot |
Date: |
Wed, 6 Jul 2022 22:43:12 +0300 |
On Wed, Jul 06, 2022 at 09:38:39PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 7/4/22 13:25, Roman Kagan wrote:
> > It's possible to create non-working configurations by attaching a device
> > to a derivative of PCIe slot (pcie-root-port, ioh3420, etc) and
> > specifying a slot number other that zero, e.g.:
> >
> > -device pcie-root-port,id=s0,... \
> > -device virtio-blk-pci,bus=s0,addr=4,...
> >
> > Make QEMU reject such configurations and only allow addr=0 on the
> > secondary bus of a PCIe slot.
> >
> > Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
> > ---
> > hw/pci/pci_bridge.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> > index da34c8ebcd..8b38d5ad3d 100644
> > --- a/hw/pci/pci_bridge.c
> > +++ b/hw/pci/pci_bridge.c
> > @@ -33,6 +33,7 @@
> > #include "qemu/units.h"
> > #include "hw/pci/pci_bridge.h"
> > #include "hw/pci/pci_bus.h"
> > +#include "hw/pci/pcie_port.h"
> > #include "qemu/module.h"
> > #include "qemu/range.h"
> > #include "qapi/error.h"
> > @@ -386,6 +387,10 @@ void pci_bridge_initfn(PCIDevice *dev, const char
> > *typename)
> > br->windows = pci_bridge_region_init(br);
> > QLIST_INIT(&sec_bus->child);
> > QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling);
> > +
> > + if (PCIE_SLOT(dev)) {
>
> Hmm, wouldn't PCIE_SLOT just crash if dev is not pcie slot? As I understand,
> PCIE_SLOT is finally an OBJECT_CHECK(), which say:
>
> * If an invalid object is passed to this function, a run time assert will be
> * generated.
Well, the assertion is there only if configured with
--enable-qom-cast-debug which is off by default, that's why it even
passed make check. As it stands, it's just a typecast which is a no-op
here, and basically it makes every bridge have only a single slot, which
is wrong of course.
Will rework, thanks!
Roman.