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

[Thomas Huth]

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.

 Thomas