On Mon, Sep 27, 2021 at 12:49 PM Michael S. Tsirkin <
mst@redhat.com> wrote:
On Mon, Sep 27, 2021 at 10:33:42AM +0100, Daniel P. Berrangé wrote:
> On Tue, Aug 03, 2021 at 04:52:03PM -0400, Michael S. Tsirkin wrote:
> > From: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> >
> > Q35 has now ACPI hotplug enabled by default for PCI(e) devices.
> > As opposed to native PCIe hotplug, guests like Fedora 34
> > will not assign IO range to pcie-root-ports not supporting
> > native hotplug, resulting into a regression.
> >
> > Reproduce by:
> > qemu-bin -M q35 -device pcie-root-port,id=p1 -monitor stdio
> > device_add e1000,bus=p1
> > In the Guest OS the respective pcie-root-port will have the IO range
> > disabled.
> >
> > Fix it by setting the "reserve-io" hint capability of the
> > pcie-root-ports so the firmware will allocate the IO range instead.
> >
> > Acked-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> > Message-Id: <20210802090057.1709775-1-marcel@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > hw/pci-bridge/gen_pcie_root_port.c | 5 +++++
> > 1 file changed, 5 insertions(+)
>
> This change, when combined with the switch to ACPI based hotplug by
> default, is responsible for a significant regression in QEMU 6.1.0
>
> It is no longer possible to have more than 15 pcie-root-port devices
> added to a q35 VM in 6.1.0. Before this I've had as many as 80+ devices
> present before I stopped trying to add more.
>
> https://gitlab.com/qemu-project/qemu/-/issues/641
>
> This regression is significant, because it has broken the out of the
> box default configuration that OpenStack uses for booting all VMs.
> They add 16 pcie-root-ports by defalt to allow empty slots for device
> hotplug under q35 [1].
Indeed, oops. Thanks for the report!
Going back and looking at seabios code, didn't we get confused?
Shouldn't we have reserved memory and not IO?
We need the IO space for the legacy PCI bridges, otherwise an empty PCI bridge will become unusable.
I see:
int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO);
if (!sum && hotplug_support && !resource_optional)
sum = align; /* reserve min size for hot-plug */
generally maybe we should just add an ACPI-hotplug capability and
teach seabios about it?
I suppose it is possible.
Thanks,
Marcel
Marcel?
> > diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> > index ec9907917e..20099a8ae3 100644
> > --- a/hw/pci-bridge/gen_pcie_root_port.c
> > +++ b/hw/pci-bridge/gen_pcie_root_port.c
> > @@ -28,6 +28,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(GenPCIERootPort, GEN_PCIE_ROOT_PORT)
> > (GEN_PCIE_ROOT_PORT_AER_OFFSET + PCI_ERR_SIZEOF)
> >
> > #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR 1
> > +#define GEN_PCIE_ROOT_DEFAULT_IO_RANGE 4096
> >
> > struct GenPCIERootPort {
> > /*< private >*/
> > @@ -75,6 +76,7 @@ static bool gen_rp_test_migrate_msix(void *opaque, int version_id)
> > static void gen_rp_realize(DeviceState *dev, Error **errp)
> > {
> > PCIDevice *d = PCI_DEVICE(dev);
> > + PCIESlot *s = PCIE_SLOT(d);
> > GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d);
> > PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
> > Error *local_err = NULL;
> > @@ -85,6 +87,9 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
> > return;
> > }
> >
> > + if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hotplug) {
> > + grp->res_reserve.io = GEN_PCIE_ROOT_DEFAULT_IO_RANGE;
> > + }
> > int rc = pci_bridge_qemu_reserve_cap_init(d, 0,
> > grp->res_reserve, errp);
> >
> > --
> > MST
> >
> >
>
> Regards,
> Daniel
>
> [1] https://github.com/openstack/tripleo-heat-templates/blob/7a6cd0640ec390a330f5699d8ed60f71b2a9f514/deployment/nova/nova-compute-container-puppet.yaml#L462-L472
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|