hw/sysbus: Spring cleanups (part 1)

[PATCH 03/27] hw/sysbus: Use memory_region_name()

Philippe Mathieu-Daudé posted 27 patches 2 days, 16 hours ago

Download mbox

Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Alistair Francis <alistair@alistair23.me>, Peter Maydell <peter.maydell@linaro.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <richard.henderson@linaro.org>, Artyom Tarasenko <atar4qemu@gmail.com>, Jason Wang <jasowang@redhat.com>, Michael Rolnik <mrolnik@gmail.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Magnus Damm <magnus.damm@gmail.com>, Vijai Kumar K <vijai@behindbytes.com>, Palmer Dabbelt <palmer@dabbelt.com>, Helge Deller <deller@gmx.de>, Song Gao <gaosong@loongson.cn>, Bibo Mao <maobibo@loongson.cn>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Huacai Chen <chenhuacai@kernel.org>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Ran Wang <wangran@bosc.ac.cn>, Anup Patel <anup.patel@wdc.com>, Fam Zheng <fam@euphon.net>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Gerd Hoffmann <kraxel@redhat.com>, Jean-Christophe Dubois <jcd@tribudubois.net>

Expand all Fold all

[PATCH 03/27] hw/sysbus: Use memory_region_name()

Posted by Philippe Mathieu-Daudé 2 days, 16 hours ago

MemoryRegion::name is kind of internal. Use the proper
memory_region_name() accessor, which might return a different
name.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/core/sysbus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index ae447c1196a..d33be6b2b52 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -154,7 +154,7 @@ void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr)
 int sysbus_mmio_map_name(SysBusDevice *dev, const char *name, hwaddr addr)
 {
     for (int i = 0; i < dev->num_mmio; i++) {
-        if (!strcmp(dev->mmio[i].memory->name, name)) {
+        if (!strcmp(memory_region_name(dev->mmio[i].memory), name)) {
             sysbus_mmio_map(dev, i, addr);
             return i;
         }
-- 
2.51.0

Re: [PATCH 03/27] hw/sysbus: Use memory_region_name()

Posted by Mark Cave-Ayland 2 days, 16 hours ago

On 24/10/2025 20:03, Philippe Mathieu-Daudé wrote:

> MemoryRegion::name is kind of internal. Use the proper
> memory_region_name() accessor, which might return a different
> name.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/core/sysbus.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index ae447c1196a..d33be6b2b52 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -154,7 +154,7 @@ void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr)
>   int sysbus_mmio_map_name(SysBusDevice *dev, const char *name, hwaddr addr)
>   {
>       for (int i = 0; i < dev->num_mmio; i++) {
> -        if (!strcmp(dev->mmio[i].memory->name, name)) {
> +        if (!strcmp(memory_region_name(dev->mmio[i].memory), name)) {
>               sysbus_mmio_map(dev, i, addr);
>               return i;
>           }

Interesting. I'm mildly curious as to if you discovered anything that broke with this 
change (even though I agree it is correct)?


ATB,

Mark.

Re: [PATCH 03/27] hw/sysbus: Use memory_region_name()

Posted by Philippe Mathieu-Daudé 2 days, 14 hours ago

On 24/10/25 21:40, Mark Cave-Ayland wrote:
> On 24/10/2025 20:03, Philippe Mathieu-Daudé wrote:
> 
>> MemoryRegion::name is kind of internal. Use the proper
>> memory_region_name() accessor, which might return a different
>> name.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/core/sysbus.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
>> index ae447c1196a..d33be6b2b52 100644
>> --- a/hw/core/sysbus.c
>> +++ b/hw/core/sysbus.c
>> @@ -154,7 +154,7 @@ void sysbus_mmio_map(SysBusDevice *dev, int n, 
>> hwaddr addr)
>>   int sysbus_mmio_map_name(SysBusDevice *dev, const char *name, hwaddr 
>> addr)
>>   {
>>       for (int i = 0; i < dev->num_mmio; i++) {
>> -        if (!strcmp(dev->mmio[i].memory->name, name)) {
>> +        if (!strcmp(memory_region_name(dev->mmio[i].memory), name)) {
>>               sysbus_mmio_map(dev, i, addr);
>>               return i;
>>           }
> 
> Interesting. I'm mildly curious as to if you discovered anything that 
> broke with this change (even though I agree it is correct)?

I'm working on having qdev to absorb sysbus features (MMIO, PIO
and IRQ links); what is left is the qom parenting on the main
sysbus, only used during legacy reset; I hope to convert the
remaining as a cripple reset tree.