Hi A?drea, Kristina,
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
Eric
>
>
> (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.)
>
