Re: [Qemu-devel] [RFC 13/19] unimplemented-device: Remove user_creatable flag

[Alexander Graf]

> Am 04.04.2017 um 09:05 schrieb Thomas Huth <address@hidden>:
> 
>> On 03.04.2017 22:05, Eduardo Habkost wrote:
>>> On Mon, Apr 03, 2017 at 08:42:12PM +0100, Peter Maydell wrote:
>>>> On 3 April 2017 at 19:30, Eduardo Habkost <address@hidden> wrote:
>>>>> On Mon, Apr 03, 2017 at 03:08:06PM +0100, Peter Maydell wrote:
>>>>>> On 3 April 2017 at 14:54, Eduardo Habkost <address@hidden> wrote:
>>>>>> This, on the other hand, currently works:
>>>>>>  $ qemu-system-x86_64 -M q35 -device 
>>>>>> unimplemented-device,size=1024,name=foo
>>>>> 
>>>>> That's a bug in the q35 machine or the handling of -device
>>>>> on non-platform-bus systems, though, isn't it? The default
>>>>> for all sysbus devices has always been "you can't use
>>>>> -device with this", [...]
>>>> 
>>>> This was true until 2014, only. commit
>>>> 33cd52b5d7b9adfd009e95f07e6c64dd88ae2a31 changed sys-bus-device
>>>> to have cannot_instantiate_with_device_add_yet=false. See patch
>>>> 03/19 for more detailed info.
>>> 
>>> That commit shouldn't cause this problem -- unless the
>>> machine sets has_dynamic_sysbus then the machine_init_notify()
>>> that commit adds will cause it to complain.
>>> 
>>> I think the bug was introduced much later, in bf8d4924 in 2016,
>>> when q35 had the has_dynamic_sysbus flag added but without
>>> any code to restruct this to a whitelist of working
>>> devices. (In fact you can see in that commit a very
>>> minimal attempt to blacklist a few devices.)
>> 
>> It's true that the problem happened when q35 set
>> has_dynamic_sysbus=1 without a whitelist. The point of this
>> series is to make a per-machine-type whitelist unnecessary.
>> 
>>> 
>>> The commit message says only intel-iommu was supposed to
>>> be -device creatable, so it should have been the only
>>> thing on the whitelist.
>> 
>> There was a thread about it (Subject: "q35 and sysbus devices"),
>> and nobody knew for sure if the q35 whitelist was supposed to
>> have *-iommu only, or not.
>> 
>> The conclusion of that thread is that we can simply use
>> cannot_instantiate_with_device_add_yet to build the whitelist, if
>> the field was set consistently. This series renames
>> cannot_instantiate_with_device_add_yet to user_creatable, and
>> sets it consistently.
> 
> I don't think that this will always work. While you can clearly mark
> those devices that can never be added dynamically, there might also be
> some devices that only work on certain machines. For example, there
> might be devices that only work on the "ppce500" machine, but do not
> work on the "pseries" machine. So we always need some kind of
> whitelisting for the machines that have a dynamic sysbus.

It's even worse. Sysbus needs explicit logic to wire up memory regions and 
irqs. That logic is board specific (or at least was last time I checked).

So your board *needs* explicit logic for every sysbus device it spawns. And 
that's why you also get a board whitelist.


Alex