[v1] hw: Strengthen SysBus & QBus API

[PATCH 08/12] hw/isa: Realize ISA BUS sysbus device before accessing it

Philippe Mathieu-Daudé posted 12 patches 1 year, 1 month ago

Diff against v2 v3 v4
Download mbox

Maintainers: Peter Maydell <peter.maydell@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Richard Henderson <richard.henderson@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Peter Xu <peterx@redhat.com>, Jason Wang <jasowang@redhat.com>, Sergio Lopez <slp@redhat.com>, Song Gao <gaosong@loongson.cn>, Beniamino Galvani <b.galvani@gmail.com>, Strahinja Jankovic <strahinja.p.jankovic@gmail.com>, Huacai Chen <chenhuacai@kernel.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>

There is a newer version of this series

Expand all Fold all

[PATCH 08/12] hw/isa: Realize ISA BUS sysbus device before accessing it

Posted by Philippe Mathieu-Daudé 1 year, 1 month ago

sysbus_mmio_map() should not be called on unrealized device.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/isa/isa-bus.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index a289eccfb1..f1e0f14007 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -52,18 +52,25 @@ static const TypeInfo isa_bus_info = {
 ISABus *isa_bus_new(DeviceState *dev, MemoryRegion* address_space,
                     MemoryRegion *address_space_io, Error **errp)
 {
+    DeviceState *bridge = NULL;
+
     if (isabus) {
         error_setg(errp, "Can't create a second ISA bus");
         return NULL;
     }
     if (!dev) {
-        dev = qdev_new("isabus-bridge");
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+        bridge = qdev_new("isabus-bridge");
+        dev = bridge;
     }
 
     isabus = ISA_BUS(qbus_new(TYPE_ISA_BUS, dev, NULL));
     isabus->address_space = address_space;
     isabus->address_space_io = address_space_io;
+
+    if (bridge) {
+        sysbus_realize_and_unref(SYS_BUS_DEVICE(bridge), &error_fatal);
+    }
+
     return isabus;
 }
 
-- 
2.41.0

Re: [PATCH 08/12] hw/isa: Realize ISA BUS sysbus device before accessing it

Posted by Thomas Huth 1 year, 1 month ago

On 18/10/2023 16.11, Philippe Mathieu-Daudé wrote:
> sysbus_mmio_map() should not be called on unrealized device.

I also cannot spot a sysbus_mmio_map() here ... do you mean qdev_new() instead?

  Thomas

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/isa/isa-bus.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> index a289eccfb1..f1e0f14007 100644
> --- a/hw/isa/isa-bus.c
> +++ b/hw/isa/isa-bus.c
> @@ -52,18 +52,25 @@ static const TypeInfo isa_bus_info = {
>   ISABus *isa_bus_new(DeviceState *dev, MemoryRegion* address_space,
>                       MemoryRegion *address_space_io, Error **errp)
>   {
> +    DeviceState *bridge = NULL;
> +
>       if (isabus) {
>           error_setg(errp, "Can't create a second ISA bus");
>           return NULL;
>       }
>       if (!dev) {
> -        dev = qdev_new("isabus-bridge");
> -        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> +        bridge = qdev_new("isabus-bridge");
> +        dev = bridge;
>       }
>   
>       isabus = ISA_BUS(qbus_new(TYPE_ISA_BUS, dev, NULL));
>       isabus->address_space = address_space;
>       isabus->address_space_io = address_space_io;
> +
> +    if (bridge) {
> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(bridge), &error_fatal);
> +    }
> +
>       return isabus;
>   }
>

Re: [PATCH 08/12] hw/isa: Realize ISA BUS sysbus device before accessing it

Posted by Philippe Mathieu-Daudé 1 year, 1 month ago

On 18/10/23 17:57, Thomas Huth wrote:
> On 18/10/2023 16.11, Philippe Mathieu-Daudé wrote:
>> sysbus_mmio_map() should not be called on unrealized device.
> 
> I also cannot spot a sysbus_mmio_map() here ... do you mean qdev_new() 
> instead?

Yeah, bad copy/paste :/

> 
>   Thomas
> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/isa/isa-bus.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
>> index a289eccfb1..f1e0f14007 100644
>> --- a/hw/isa/isa-bus.c
>> +++ b/hw/isa/isa-bus.c
>> @@ -52,18 +52,25 @@ static const TypeInfo isa_bus_info = {
>>   ISABus *isa_bus_new(DeviceState *dev, MemoryRegion* address_space,
>>                       MemoryRegion *address_space_io, Error **errp)
>>   {
>> +    DeviceState *bridge = NULL;
>> +
>>       if (isabus) {
>>           error_setg(errp, "Can't create a second ISA bus");
>>           return NULL;
>>       }
>>       if (!dev) {
>> -        dev = qdev_new("isabus-bridge");
>> -        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>> +        bridge = qdev_new("isabus-bridge");
>> +        dev = bridge;
>>       }
>>       isabus = ISA_BUS(qbus_new(TYPE_ISA_BUS, dev, NULL));
>>       isabus->address_space = address_space;
>>       isabus->address_space_io = address_space_io;
>> +
>> +    if (bridge) {
>> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(bridge), &error_fatal);
>> +    }
>> +
>>       return isabus;
>>   }
>