Re: [Qemu-devel] q35 and sysbus devices

[Eduardo Habkost]

On Fri, Mar 24, 2017 at 01:49:16PM +0300, Marcel Apfelbaum wrote:
> On 03/22/2017 10:46 PM, Laszlo Ersek wrote:
> > On 03/22/17 21:31, Eduardo Habkost wrote:
> > > Hi,
> > > 
> > > I am investigating the current status of has_dynamic_sysbus and
> > > sysbus device support on each of QEMU's machine types. The good
> > > news is that almost all has_dynamic_sysbus=1 machines have their
> > > own internal (often short) whitelist of supported sysbus device
> > > types, and automatically reject unsupported devices.
> > > 
> > > ...except for q35.
> > > 
> > > q35 currently accepts all sys-bus-device subtypes on "-device",
> > > and today this includes the following 23 devices:
> > > 
> > > * allwinner-ahci
> > > * amd-iommu
> > > * cfi.pflash01
> > > * esp
> > > * fw_cfg_io
> > > * fw_cfg_mem
> > > * generic-sdhci
> > > * hpet
> > > * intel-iommu
> > > * ioapic
> > > * isabus-bridge
> > > * kvmclock
> > > * kvm-ioapic
> > > * kvmvapic
> > > * SUNW,fdtwo
> > > * sysbus-ahci
> > > * sysbus-fdc
> > > * sysbus-ohci
> > > * unimplemented-device
> > > * virtio-mmio
> > > * xen-backend
> > > * xen-sysdev
> > > 
> > > My question is: do all those devices really make sense to be used
> > > with "-device" on q35?
> > 
> > I think fw_cfg_io and fw_cfg_mem should be board-only devices (no
> > -device switch).
> > 
> > Regarding cfi.pflash01, I think originally it would have been nice to
> > specify pflash chips with the modern (non-legacy) syntax, that is,
> > separate -drive if=none,file=... backend options combined with -device
> > cfi.pflash01,drive=... frontend options. However, that ship has sailed,
> > even libvirt uses -drive if=pflash for these, and given the purpose we
> > use pflash chips for, on Q35, I don't see much benefit in exposing
> > cfi.pflash01 with a naked -device *now*.
> > 
> > Re: virtio-mmio, I don't think that should be available on Q35 at all.
> > 
> > I can't comment on the rest.
> > 
> 
> Hi Eduardo,
> Thanks for finding these problems.
> 
> We should ping all maintainers of the above devices, the best way to do it
> is to add the "cannot_instantiate_with_device_add_yet = true" and ask 
> maintainers
> to agree (or not) on that.

If I understand it correctly,
cannot_instantiate_with_device_add_yet is supposed to be
temporary. And it applies to all machines, with no exceptions.

The problem with today's mechanism is that we have no way to make
a machine accept one type of sysbus device without making it
start accepting every other sysbus devices. If we thought all
!cannot_instantiate_with_device_add_yet sysbus devices were
already safe, we wouldn't have has_dynamic_sysbus in the first
place, would we?

-- 
Eduardo