[PATCH] qemu: add pcie-to-pci-bridge for q35 machines

[Laine Stump]

On 11/4/22 10:01 AM, Denis V. Lunev wrote:
> On 11/4/22 14:41, Daniel P. Berrangé wrote:
>> On Fri, Nov 04, 2022 at 01:03:52PM +0100, Peter Krempa wrote:
>>> On Fri, Nov 04, 2022 at 16:43:00 +0600, Oleg Vasilev wrote:
>>>> Hotplugging PCI devices on pc-i440fx machines is supported without
>>>> additional configuration. On q35, pcie-to-pci-bridge needs to be added
>>>> prior to the machine startup in order to support hotplug [1].
>>>>
>>>> The idea is to support the same workflow for creating VMs in q35, as 
>>>> was
>>>> in pc. Namely, do not require additional configuration when hotplugging
>>>> is needed. Otherwise, all libvirt clients need to be updated which 
>>>> there
>>>> are a lot and they are maintained by different parties.
>>>>
>>>> Instead, a pcie-to-pci-bridge better be created by default, so that PCI
>>>> slots are readily available. Might be a good idea to make it 
>>>> configurable
>>>> in the future.
>> The goal of q35 is to provide a PCI-e topology though, and we want
>> devices to be exposed as PCI-e endpoints, with no legacy PCI usage
>> by default.
>>
>> When a guest XML is provided for cold boot, libvirt will ensure
>> that all devices are placed onto the PCI-e bus via pcie-root-ports
>>
>> With this proposal hotplugging a device will cause it to be placed
>> on a PCI bus instead, if there are no spare pcie-root-port available.

Actually if the device being hotplugged is a PCIe device (e.g. a 
virtio-net device, or e1000e) what will instead happen is that libvirt 
will attempt to find an empty pcie-root-port and place the device there 
if one is found. If it can't find any empty pcie-root-ports (or 
pcie-downstream-ports) the hotplug will fail because there are no 
available PCIe slots - libvirt will never auto-assign a PCIe device to a 
conventional PCI slot, or vice-versa - it will only auto-assign a PCIe 
device to a PCIe slot (e.g. a a pcie-root-port), and will only 
auto-assign a conventional PCI device to a conventional PCI address 
(e.g. a slot on a pcie-to-pci-bridge). If you want it to do anything 
else, you need to supply explicit PCI addresses.



>>
>> This behavioural difference between cold plug and hot plug is very
>> undesirable IMHO, as it is counter to our goal of avoiding legacy
>> PCI on PCI-e machines.
> 
> that is an interesting point which I have initially missed. I have
> checked our investigation log and found that we have
> potentially made a mistake with making compatibility
> check with e1000 device instead of virtio, which is
> our default. This could be the reason.

Yes, with the default PCI bus topology created by libvirt, you would be 
unable to hotplug even one e1000 device, because the e1000 is a 
conventional PCI device, *not* PCIe, and by default libvirt doesn't add 
*any* conventional PCI slots to a domain based on Q35. If you must use 
an emulated device based on real hardware instead of virtio, always use 
the e1000e device (which is PCIe) for Q35 machinetypes, not e1000 (which 
is conventional PCI). If your test had specified e1000e, then you should 
have been able to hotplug one device, as libvirt makes sure there is a 
single empty pcie-root-port in any domain definition where it needs to 
auto-construct the PCI topology.

You really should avoid *any* emulated device that is conventional PCI 
only in a Q35 vm; fortunately I think the only type of device that 
doesn't have a PCIe flavor is the watchdog timers (I could be 
remembering incorrectly, but I think that was the one).

Also, I don't think anyone has mentioned it so far anywhere in this 
thread, but I just wanted to point out that if you plug a PCIe device 
into a conventional PCI slot (such as those provided on a 
pci-to-pcie-bridge) then the extended PCIe config/capabilities will be 
inaccessible for that device, which could impact performance - I recall 
one issue where the bus width/speed/something-like-that of a device was 
inaccessible, leading to problems because either the guest couldn't 
determine the value of the setting, or because it couldn't change it (I 
forget which, and the exact answer isn't important enough to spend an 
hour on mailing-list forensics to find it :-))

> this potentially an option, but I think that these port should be
> created automatically by libvirt to avoid major client rewrites.

As a sort of compromise, libvirt will always attempt to have one empty 
pcie-root-port when it's building the PCI controller topology. Anything 
beyond this requires intervention by the user/management application. If 
you want "lots" of open ports for hotplug, the simplest remedy is to 
include several <controller type='pci' model='pcie-root-port'/> in your 
initial definition.