On Fri, Sep 19, 2025 at 04:28:03PM +0200, Peter Krempa wrote:
> On Fri, Sep 19, 2025 at 08:28:38 -0500, Andrea Bolognani wrote:
> > On Thu, Sep 18, 2025 at 03:04:28PM +0200, Peter Krempa wrote:
> > > On Tue, Aug 19, 2025 at 18:22:22 +0200, Andrea Bolognani via Devel wrote:
> > > > + /* Sanity check. If the machine type supports PCI, we need to
> > > > reflect
> > > > + * that fact in the XML or other parts of the machine handling code
> > > > + * might misbehave */
> > >
> > > This one is a bit borderline. IMO you can have machine with no default
> > > PCI but the possibility to explicily add such a bus.
> >
> > Can you point to a specific example? I'm not aware of any.
>
> I don't have a specific example for this, but I have a example where the
> sanity check breaks loading config of one of VMs I had defined before:
>
> <domain type='kvm'>
> <name>microvm</name>
> <uuid>e739ac15-61b5-48c2-acc8-e7fb2b79774f</uuid>
> <memory unit='KiB'>1024000</memory>
> <currentMemory unit='KiB'>1024000</currentMemory>
> <vcpu placement='static'>1</vcpu>
> <os>
> <type arch='x86_64' machine='microvm'>hvm</type>
> <boot dev='hd'/>
> </os>
> <cpu mode='custom' match='exact' check='none'>
> <model fallback='forbid'>qemu64</model>
> </cpu>
> <clock offset='utc'/>
> <on_poweroff>destroy</on_poweroff>
> <on_reboot>restart</on_reboot>
> <on_crash>destroy</on_crash>
> <devices>
> <emulator>/usr/bin/qemu-system-x86_64</emulator>
> <disk type='file' device='disk'>
> <driver name='qemu' type='qcow2'/>
> <source file='/var/lib/libvirt/images/alpine.qcow2'/>
> <target dev='vda' bus='virtio'/>
> <address type='virtio-mmio'/>
> </disk>
> <controller type='usb' index='0' model='none'/>
> <audio id='1' type='none'/>
> <memballoon model='none'/>
> </devices>
> </domain>
>
> ... will after this patch be rejected at load time:
>
> 2025-09-19 14:16:28.132+0000: 214862: info :
> virDomainObjListLoadAllConfigs:601 : Loading config file 'microvm.xml'
> 2025-09-19 14:16:28.132+0000: 214862: debug :
> virDomainControllerDefParseXML:9008 : Ignoring device address for none model
> usb controller
> 2025-09-19 14:16:28.132+0000: 214862: debug :
> virDomainMemballoonDefParseXML:12948 : Ignoring device address for none model
> Memballoon
> 2025-09-19 14:16:28.132+0000: 214862: debug : virCPUDataIsIdentical:1285 :
> a=0x7fffac30c7f0, b=0x7fffac523430
> 2025-09-19 14:16:28.132+0000: 214862: debug : virArchFromHost:236 : Mapped
> x86_64 to 35 (x86_64)
> 2025-09-19 14:16:28.132+0000: 214862: debug : virQEMUCapsCacheLookup:5996 :
> Returning caps 0x7fffac022ad0 for /usr/bin/qemu-system-x86_64
> 2025-09-19 14:16:28.132+0000: 214862: error :
> qemuDomainDefAddDefaultDevices:1343 : internal error: Machine type 'microvm'
> supports PCI but no PCI controller added
> 2025-09-19 14:16:28.132+0000: 214862: error :
> virDomainObjListLoadAllConfigs:622 : Failed to load config for domain
> 'microvm'
>
> > To the best of my knowledge, all machines that are PCI-capable come
> > with a root PCI controller out of the gate.
> >
> > There is no QEMU device corresponding to the root PCI controller
> > either, so I don't know how you would even go about adding it if it
> > were opt-in. Perhaps a machine type flag but again, I'm not aware of
> > that actually being a thing.
>
> IIUC 'microvm' doesn't support PCI, which shows that this check itself
> is broken not that there isn't merit in the check itself.
Yup, qemuDomainSupportsPCI() needs to be tweaked further to account
for this.
> > > > + /* Sanity check. USB controllers are PCI devices, so asking for a
> > > > + * USB controller to be added but not for a PCI(e) root to be
> > > > + * added at the same time is likely an oversight */
> > >
> > > I'm fairly sure there are non-PCI USB controllers so this one is
> > > definitely false. It's just that libvirt supports only USB controllers
> > > which are on PCI.
> > >
> > > IMO this assumption should be validated with the USB controller itself.
> >
> > I'm not aware of any non-PCI USB controller out there, certainly not
> > in QEMU. Can you point to one?
>
> One example is the USB controller in older raspberry-pis, which is
> embedded in the SoC. I presume it's accessed via MMIO. QEMU claims
> support for rpis at least.
>
> As said this is something that ought to be validated when validating the
> USB controller device, rather than here.
Oh yeah, here it is:
dev: dwc2-usb, id ""
gpio-out "sysbus-irq" 1
usb_version = 2 (0x2)
mmio ffffffffffffffff/0000000000011000
bus: usb-bus.0
type usb-bus
Point taken, this check is incorrect.
--
Andrea Bolognani / Red Hat / Virtualization
