[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] q35 and sysbus devices
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] q35 and sysbus devices |
Date: |
Mon, 27 Mar 2017 13:11:09 -0300 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Fri, Mar 24, 2017 at 05:58:43PM +0100, Markus Armbruster wrote:
> Eduardo Habkost <address@hidden> writes:
>
> > 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.
>
> A couple of years ago, we had a -device regression: devices marked
> no_user were no longer rejected. To get my fix for that in, I had to
> rename it to cannot_instantiate_with_device_add_yet and add some solemn
> protestations that it's temporary. It's been temporary ever since.
Interesting story. I will look for it in the qemu-devel archives.
>
> Without doubt getting rid of it would be nice. But I'm not holding my
> breath.
This sounds like a good demonstration that
cannot_instantiate_with_device_add_yet is not going to be
temporary. I suggest renaming it back to "no_user", or
"user_creatable", and living with the fact that the ability to
create a device using -device or device_add needs to be reported
by our APIs, instead of pretending otherwise.
>
> > And it applies to all machines, with no exceptions.
>
> Correct.
>
> > 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?
>
> In my relatively ignorant opinion, "dynamic sysbus" has to die even
> harder than "sysbus".
>
> "Sysbus" isn't a bus. In qdev's original design, every device had to
> plug into a bus, period. The ones that really didn't were made to plug
> into "sysbus".
>
> Pretty much the only thing "sysbus" devices had in common was that they
> couldn't be used with device_add and device_del.
>
> We fixed the design to permit bus-less devices, but we didn't get rid of
> "sysbus".
>
> We got a "platform bus", which is really not the same as "sysbus", but
> we shoehorned it into "sysbus" anyway.
>
> The result is a mess, and you're sitting right in it.
>
> One hack we could perhaps pile on top of the others: have sysbus devices
> again set cannot_instantiate_with_device_add_yet, then unset it for the
> ones in the whitelist.
This makes a lot of sense, to me. Maybe it would make the
existing per-machine-type whitelists unnecessary.
(And while doing that, let's rename
cannot_instantiate_with_device_add_yet to no_user or
user_creatable, and stop lying to ourselves.)
--
Eduardo
- Re: [Qemu-devel] q35 and sysbus devices, (continued)
- Re: [Qemu-devel] q35 and sysbus devices, Peter Maydell, 2017/03/24
- Re: [Qemu-devel] q35 and sysbus devices, Eduardo Habkost, 2017/03/24
- Re: [Qemu-devel] q35 and sysbus devices, Markus Armbruster, 2017/03/24
- Re: [Qemu-devel] q35 and sysbus devices, Peter Maydell, 2017/03/24
- Re: [Qemu-devel] q35 and sysbus devices, Markus Armbruster, 2017/03/24
- Re: [Qemu-devel] q35 and sysbus devices, Peter Maydell, 2017/03/24
- Re: [Qemu-devel] q35 and sysbus devices, Thomas Huth, 2017/03/27
- Re: [Qemu-devel] q35 and sysbus devices, Eduardo Habkost, 2017/03/24
- Re: [Qemu-devel] q35 and sysbus devices, Cornelia Huck, 2017/03/27
- Re: [Qemu-devel] q35 and sysbus devices, Peter Maydell, 2017/03/27
- Re: [Qemu-devel] q35 and sysbus devices,
Eduardo Habkost <=
- Re: [Qemu-devel] q35 and sysbus devices, Thomas Huth, 2017/03/24