[PATCH 4/6] qemu: assign PCI address to device pvpanic-pci

[Andrea Bolognani]

On Thu, Feb 09, 2023 at 06:26:14PM +0100, Eric Auger wrote:
> On 2/9/23 17:33, Andrea Bolognani wrote:
> > On Wed, Feb 08, 2023 at 12:49:03PM +0100, Kristina Hanicova wrote:
> >> +++ b/src/qemu/qemu_domain_address.c
> >> @@ -1062,10 +1062,24 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev,
> >>          }
> >>          break;
> >>
> >> +    case VIR_DOMAIN_DEVICE_PANIC:
> >> +        switch ((virDomainPanicModel) dev->data.panic->model) {
> >> +        case VIR_DOMAIN_PANIC_MODEL_PVPANIC:
> >> +            return pciFlags;
> >
> > So, this is correct, because pvpanic-pci is indeed a conventional PCI
> > device (as opposed to a PCI Express device).
> >
> > However, when used with a PCIe-based machine such as aarch64/virt,
> > these flags result in the device being plugged into a
> > pcie-pci-bridge, which in turn is plugged into a pcie-root-port,
> > itself plugged into pcie.0.
> >
> > While this seems to work fine, it's also kind of a waste of PCI
> > controllers. For a "system" device such as pvpanic-pci, I think
> > plugging directly into pcie.0 might be more appropriate. This is
> > particularly true of x86/q35, where with the current implementation
> > switching from ISA pvpanic to pvpanic-pci would result in adding two
> > extra PCI controllers.
> >
> > So I think this would be a good fit for the
> > VIR_PCI_CONNECT_INTEGRATED flag that I've added a while ago for use
> > with virtio-iommu. While this would technically result in libvirt
> > being more restrictive than QEMU in what PCI addresses it accepts for
> > pvpanic-pci, I don't think this would limit users in practice, and in
> > the default case (libvirt automatically picking the address) the
> > resulting configuration would be preferable.
> >
> > We can always decide to relax this restriction later down the line,
> > if it turns out to really be a limiting factor.
> >
> > Eric, what do you think about this idea?
>
> I do agree. If we can avoid putting that device downstream to a
> pcie-to-pci bridge that's much better. Putting it onto pcie.0 looks more
> appropriate to me indeed.

Thanks for the input! Let's go with this approach then, unless
someone raises concerns with it.

> > (There's one caveat: VIR_PCI_CONNECT_INTEGRATED doesn't seem to work
> >  with pciFlags at the moment O:-) It works fine with pcieFlags and
> >  virtioFlags. I'll try to figure out why that's the case.)

Fixed by this patch:

  https://listman.redhat.com/archives/libvir-list/2023-February/237688.html

-- 
Andrea Bolognani / Red Hat / Virtualization