Re: [Qemu-arm] [Qemu-devel] [PATCH] hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable = false

[Peter Maydell]

On 5 September 2017 at 14:57, Thomas Huth <address@hidden> wrote:
> On 23.08.2017 20:00, Eduardo Habkost wrote:
>> On Wed, Aug 23, 2017 at 06:51:28PM +0100, Peter Maydell wrote:
>>> On 23 August 2017 at 18:21, Eduardo Habkost <address@hidden> wrote:
>>>> On Tue, Aug 22, 2017 at 05:46:29PM +0200, Thomas Huth wrote:
>>>>> QEMU currently exits unexpectedly when the user accidentially
>>>>> tries to do something like this:
>>>>>
>>>>> $ aarch64-softmmu/qemu-system-aarch64 -S -M integratorcp -nographic
>>>>> QEMU 2.9.93 monitor - type 'help' for more information
>>>>> (qemu) device_add allwinner-a10
>>>>> Unsupported NIC model: smc91c111
>>>>>
>>>>> Exiting just due to a "device_add" should not happen. Looking closer
>>>>> at the the realize and instance_init function of this device also
>>>>> reveals that it is using serial_hds and nd_table directly there, so
>>>>> this device is clearly not creatable by the user and should be marked
>>>>> accordingly.
>>>>>
>>>>> Signed-off-by: Thomas Huth <address@hidden>
>>>>> ---
>>>>>  hw/arm/allwinner-a10.c    | 2 ++
>>>>>  scripts/device-crash-test | 1 -
>>>>>  2 files changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
>>>>> index f62a9a3..43a3f01 100644
>>>>> --- a/hw/arm/allwinner-a10.c
>>>>> +++ b/hw/arm/allwinner-a10.c
>>>>> @@ -118,6 +118,8 @@ static void aw_a10_class_init(ObjectClass *oc, void 
>>>>> *data)
>>>>>      DeviceClass *dc = DEVICE_CLASS(oc);
>>>>>
>>>>>      dc->realize = aw_a10_realize;
>>>>> +    /* Reason: Uses serial_hds in realize and nd_table in instance_init 
>>>>> */
>>>>> +    dc->user_creatable = false;
>>>>
>>>> I assume this patch will be replaced by one changing TYPE_DEVICE
>>>> to default to user_creatable=false, based on the replies to the
>>>> hw/misc/auxbus.c patch?
>>>
>>> user-creatable and hotplug are different things -- most
>>> devices are user-creatable, many fewer are hotpluggable.
>>> So the tradeoff for the default doesn't *necessarily* go
>>> the same way.
>>
>> You're right, I mixed things up when reading this patch.
>>
>> Reviewed-by: Eduardo Habkost <address@hidden>
>
> Peter, looking at your last PULL request, it looks like this felt
> through the cracks. Could you please pick it up for the next one?

Oops, yes it did. Applied to target-arm.next, thanks.

-- PMM