1. Patchew
  2. QEMU
  3. View series

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

Thomas Huth posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1503416789-32080-1-git-send-email-thuth@redhat.com
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
hw/arm/allwinner-a10.c    | 2 ++
scripts/device-crash-test | 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable = false
Posted by Thomas Huth 6 years, 8 months ago 
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 <thuth@redhat.com>
---
 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;
 }
 
 static const TypeInfo aw_a10_type_info = {
diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 8eb2d02..74aee68 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -187,7 +187,6 @@ ERROR_WHITELIST = [
     {'log':r"Device [\w.,-]+ can not be dynamically instantiated"},
     {'log':r"Platform Bus: Can not fit MMIO region of size "},
     # other more specific errors we will ignore:
-    {'device':'allwinner-a10', 'log':"Unsupported NIC model:"},
     {'device':'.*-spapr-cpu-core', 'log':r"CPU core type should be"},
     {'log':r"MSI(-X)? is not supported by interrupt controller"},
     {'log':r"pxb-pcie? devices cannot reside on a PCIe? bus"},
-- 
1.8.3.1
Re: [Qemu-devel] [PATCH] hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable = false
Posted by Eduardo Habkost 6 years, 8 months ago 
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 <thuth@redhat.com>
> ---
>  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?


>  }
>  
>  static const TypeInfo aw_a10_type_info = {
> diff --git a/scripts/device-crash-test b/scripts/device-crash-test
> index 8eb2d02..74aee68 100755
> --- a/scripts/device-crash-test
> +++ b/scripts/device-crash-test
> @@ -187,7 +187,6 @@ ERROR_WHITELIST = [
>      {'log':r"Device [\w.,-]+ can not be dynamically instantiated"},
>      {'log':r"Platform Bus: Can not fit MMIO region of size "},
>      # other more specific errors we will ignore:
> -    {'device':'allwinner-a10', 'log':"Unsupported NIC model:"},
>      {'device':'.*-spapr-cpu-core', 'log':r"CPU core type should be"},
>      {'log':r"MSI(-X)? is not supported by interrupt controller"},
>      {'log':r"pxb-pcie? devices cannot reside on a PCIe? bus"},
> -- 
> 1.8.3.1
> 

-- 
Eduardo
Re: [Qemu-devel] [PATCH] hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable = false
Posted by Peter Maydell 6 years, 8 months ago 
On 23 August 2017 at 18:21, Eduardo Habkost <ehabkost@redhat.com> 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 <thuth@redhat.com>
>> ---
>>  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.

thanks
-- PMM
Re: [Qemu-devel] [PATCH] hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable = false
Posted by Eduardo Habkost 6 years, 8 months ago 
On Wed, Aug 23, 2017 at 06:51:28PM +0100, Peter Maydell wrote:
> On 23 August 2017 at 18:21, Eduardo Habkost <ehabkost@redhat.com> 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 <thuth@redhat.com>
> >> ---
> >>  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 <ehabkost@redhat.com>

-- 
Eduardo
Re: [Qemu-devel] [PATCH] hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable = false
Posted by Thomas Huth 6 years, 7 months ago 
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 <ehabkost@redhat.com> 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 <thuth@redhat.com>
>>>> ---
>>>>  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 <ehabkost@redhat.com>

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?

 Thanks,
  Thomas
Re: [Qemu-devel] [PATCH] hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable = false
Posted by Peter Maydell 6 years, 7 months ago 
On 5 September 2017 at 14:57, Thomas Huth <thuth@redhat.com> 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 <ehabkost@redhat.com> 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 <thuth@redhat.com>
>>>>> ---
>>>>>  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 <ehabkost@redhat.com>
>
> 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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.