[v1] hw: Strengthen SysBus & QBus API

[PATCH 12/12] hw/sysbus: Ensure device is realized before mapping 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 12/12] hw/sysbus: Ensure device is realized before mapping 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/core/sysbus.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index ce54e2c416..a46828a808 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
+#include "qemu/error-report.h"
 #include "hw/sysbus.h"
 #include "monitor/monitor.h"
 #include "exec/address-spaces.h"
@@ -132,6 +133,13 @@ static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
 {
     assert(n >= 0 && n < dev->num_mmio);
 
+    if (!DEVICE(dev)->realized) {
+        error_report("sysbus_mmio_map(type:%s, index:%d, addr:0x%"HWADDR_PRIx","
+                     " prio:%d) but object is not realized",
+                     object_get_typename(OBJECT(dev)), n, addr, priority);
+        abort();
+    }
+
     if (dev->mmio[n].addr == addr) {
         /* ??? region already mapped here.  */
         return;
-- 
2.41.0

Re: [PATCH 12/12] hw/sysbus: Ensure device is realized before mapping 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/core/sysbus.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index ce54e2c416..a46828a808 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -20,6 +20,7 @@
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
>   #include "qemu/module.h"
> +#include "qemu/error-report.h"

I assume this hunk should go into the previous patch?

>   #include "hw/sysbus.h"
>   #include "monitor/monitor.h"
>   #include "exec/address-spaces.h"
> @@ -132,6 +133,13 @@ static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
>   {
>       assert(n >= 0 && n < dev->num_mmio);
>   
> +    if (!DEVICE(dev)->realized) {
> +        error_report("sysbus_mmio_map(type:%s, index:%d, addr:0x%"HWADDR_PRIx","
> +                     " prio:%d) but object is not realized",
> +                     object_get_typename(OBJECT(dev)), n, addr, priority);
> +        abort();
> +    }

With the hunk moved (or the order of the patches reversed):

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 12/12] hw/sysbus: Ensure device is realized before mapping it

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

On 18/10/23 18:13, Thomas Huth wrote:
> On 18/10/2023 16.11, Philippe Mathieu-Daudé wrote:
>> sysbus_mmio_map() should not be called on unrealized device.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/core/sysbus.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
>> index ce54e2c416..a46828a808 100644
>> --- a/hw/core/sysbus.c
>> +++ b/hw/core/sysbus.c
>> @@ -20,6 +20,7 @@
>>   #include "qemu/osdep.h"
>>   #include "qapi/error.h"
>>   #include "qemu/module.h"
>> +#include "qemu/error-report.h"
> 
> I assume this hunk should go into the previous patch?

Oops indeed...


> With the hunk moved (or the order of the patches reversed):
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks!