[PATCH v2 0/8] aspeed: Add multi-SoC machine

[PATCH v2 0/8] aspeed: Add multi-SoC machine

[Peter Delevoryas]

v2:
- Rebased on upstream/master + v2 of Cedric's DRAM mapping change:
  https://patchwork.ozlabs.org/project/qemu-devel/patch/20220623202123.3972977-1-clg@kaod.org/
- Got rid of all the sysbus patches, not needed (I can send the dead code
  cleanup separately).
- Changed "system-memory" to "memory".
- Moved SoC unimplemented devices into AspeedSoCState, initialized and
  mapped them like normal peripheral devices.
- Added one more patch that removes 1 qemu_get_cpu call in aspeed_ast2600.c
- Kept the aspeed_board_init_flashes change: if you'd like, feel free
  to drop that patch and the one after it, I can work on refactoring
  them more to avoid that.

A few problems I still have before I can add the additional SoC and fix the
boot rom initialization:

1.
    We want to use '-device loader' to load firmware into 0x0000_0000, but
    that's not really how the SoC works. The SoC reads from SPI flash. It
    would be great if I could do '-device loader,file=firmware,addr=0x2000_0000',
    and 0x2000_0000 is the firmware controller's mmio region and it would
    store the file into the SPI flash device and the boot ROM (0x0000_0000)
    automatically, but I don't think that's possible right?

    It seems like what I'll need to do is make '-drive' the source of truth,
    and initialize the boot rom from that, similar to the existing code in
    aspeed.c with "write_boot_rom".

2.
    Right now the SoC doesn't initialize the boot rom memory region, the
    machine does it. Perhaps I should fix that, like the DRAM change Cedric
    made?

3.
    Each SoC creates its own SRAM memory with "memory_region_init_ram", so
    the machine doesn't know the name of it.

    Memory regions like that need to have a unique name for vm migration,
    but each SoC just gives it a generic one, "aspeed.sram". If you try to
    run multiple SoC's, the names collide.

    How is an SoC supposed to produce a unique name for each memory region
    it has, so that it doesn't collide with other SoC's?

    Should the machine provide the SRAM region and select a name that
    doesn't collide with other SoC's in the machine?

    That seems weird, because the SRAM is internal to the SoC. It has a size
    and mmio address that's internal to the SoC.

4.
    Lastly, I don't know a good way to allocate serial devices to the SoC
    UART's.

    Perhaps I can make each serial device specify the UART it should connect
    to? Like "-serial stdio,uart=bmc.uart5".

    Maybe I can create a subclass of the normal serial devices for this?

    I'm also not sure how the naming scheme would work. Probably just
    decided by the machine?

Note: I've reduced the number of emails cc'd in this series because I was
having trouble with the mail filter at work. Hopefully this goes through
properly.

Thanks,
Peter

Peter Delevoryas (8):
  aspeed: Set CPU memory property explicitly
  aspeed: Add memory property to Aspeed SoC
  aspeed: Remove usage of sysbus_mmio_map
  aspeed: Map unimplemented devices in SoC memory
  aspeed: Remove use of qemu_get_cpu
  aspeed: Add fby35 skeleton
  aspeed: Make aspeed_board_init_flashes public
  aspeed: Add AST2600 (BMC) to fby35

 MAINTAINERS                 |   1 +
 hw/arm/aspeed.c             |  29 ++--------
 hw/arm/aspeed_ast10x0.c     |  46 ++++++++-------
 hw/arm/aspeed_ast2600.c     |  84 +++++++++++++++------------
 hw/arm/aspeed_soc.c         | 110 +++++++++++++++++++++++++++---------
 hw/arm/fby35.c              |  93 ++++++++++++++++++++++++++++++
 hw/arm/meson.build          |   3 +-
 include/hw/arm/aspeed_soc.h |  13 +++++
 8 files changed, 269 insertions(+), 110 deletions(-)
 create mode 100644 hw/arm/fby35.c

-- 
2.30.2

[PATCH v2 1/8] aspeed: Set CPU memory property explicitly

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 hw/arm/aspeed_ast2600.c | 2 ++
 hw/arm/aspeed_soc.c     | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index f70b17d3f9..f950fff070 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -294,6 +294,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
 
         object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 1125000000,
                                 &error_abort);
+        object_property_set_link(OBJECT(&s->cpu[i]), "memory",
+                                 OBJECT(get_system_memory()), &error_abort);
 
         if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
             return;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index f530028874..06e5629800 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -247,6 +247,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 
     /* CPU */
     for (i = 0; i < sc->num_cpus; i++) {
+        object_property_set_link(OBJECT(&s->cpu[i]), "memory",
+                                 OBJECT(get_system_memory()), &error_abort);
         if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
             return;
         }
-- 
2.30.2

Re: [PATCH v2 1/8] aspeed: Set CPU memory property explicitly

[Cédric Le Goater]

On 6/24/22 02:36, Peter Delevoryas wrote:
> Signed-off-by: Peter Delevoryas <pdel@fb.com>

Please merge this patch with patch 2 in which the "memory" property
is defined.

Thanks,

C.


> ---
>   hw/arm/aspeed_ast2600.c | 2 ++
>   hw/arm/aspeed_soc.c     | 2 ++
>   2 files changed, 4 insertions(+)
> 
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index f70b17d3f9..f950fff070 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -294,6 +294,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>   
>           object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 1125000000,
>                                   &error_abort);
> +        object_property_set_link(OBJECT(&s->cpu[i]), "memory",
> +                                 OBJECT(get_system_memory()), &error_abort);
>   
>           if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
>               return;
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index f530028874..06e5629800 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -247,6 +247,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>   
>       /* CPU */
>       for (i = 0; i < sc->num_cpus; i++) {
> +        object_property_set_link(OBJECT(&s->cpu[i]), "memory",
> +                                 OBJECT(get_system_memory()), &error_abort);
>           if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
>               return;
>           }

Re: [PATCH v2 1/8] aspeed: Set CPU memory property explicitly

[Cédric Le Goater]

On 6/24/22 08:36, Cédric Le Goater wrote:
> On 6/24/22 02:36, Peter Delevoryas wrote:
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> 
> Please merge this patch with patch 2 in which the "memory" property
> is defined.

Ah no. That's another link. I don't understand where that was done
before.

C.


> 
> Thanks,
> 
> C.
> 
> 
>> ---
>>   hw/arm/aspeed_ast2600.c | 2 ++
>>   hw/arm/aspeed_soc.c     | 2 ++
>>   2 files changed, 4 insertions(+)
>>
>> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
>> index f70b17d3f9..f950fff070 100644
>> --- a/hw/arm/aspeed_ast2600.c
>> +++ b/hw/arm/aspeed_ast2600.c
>> @@ -294,6 +294,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>>           object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 1125000000,
>>                                   &error_abort);
>> +        object_property_set_link(OBJECT(&s->cpu[i]), "memory",
>> +                                 OBJECT(get_system_memory()), &error_abort);
>>           if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
>>               return;
>> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
>> index f530028874..06e5629800 100644
>> --- a/hw/arm/aspeed_soc.c
>> +++ b/hw/arm/aspeed_soc.c
>> @@ -247,6 +247,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>>       /* CPU */
>>       for (i = 0; i < sc->num_cpus; i++) {
>> +        object_property_set_link(OBJECT(&s->cpu[i]), "memory",
>> +                                 OBJECT(get_system_memory()), &error_abort);
>>           if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
>>               return;
>>           }
> 
> 

Re: [PATCH v2 1/8] aspeed: Set CPU memory property explicitly

[Peter Delevoryas]

> On Jun 23, 2022, at 11:55 PM, Cédric Le Goater <clg@kaod.org> wrote:
> 
> On 6/24/22 08:36, Cédric Le Goater wrote:
>> On 6/24/22 02:36, Peter Delevoryas wrote:
>>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>> Please merge this patch with patch 2 in which the "memory" property
>> is defined.
> 
> Ah no. That's another link. I don't understand where that was done
> before.

Yeah, sorry if this was confusing:

In the AST1030, we set the memory property on the armv7m core explicitly.

In the AST2400, AST2500, and AST2600, we have been relying on the
CPU init code implicitly setting it to get_system_memory().

The goal of this patch was just to make it consistent across all of the
Aspeed SoC’s, so that the next patch (2/8) can introduce an SoC
memory property at the machine:SoC level without changing anything
at the SoC:CPU level.

> 
> C.
> 
> 
>> Thanks,
>> C.
>>> ---
>>>   hw/arm/aspeed_ast2600.c | 2 ++
>>>   hw/arm/aspeed_soc.c     | 2 ++
>>>   2 files changed, 4 insertions(+)
>>> 
>>> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
>>> index f70b17d3f9..f950fff070 100644
>>> --- a/hw/arm/aspeed_ast2600.c
>>> +++ b/hw/arm/aspeed_ast2600.c
>>> @@ -294,6 +294,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>>>           object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 1125000000,
>>>                                   &error_abort);
>>> +        object_property_set_link(OBJECT(&s->cpu[i]), "memory",
>>> +                                 OBJECT(get_system_memory()), &error_abort);
>>>           if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
>>>               return;
>>> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
>>> index f530028874..06e5629800 100644
>>> --- a/hw/arm/aspeed_soc.c
>>> +++ b/hw/arm/aspeed_soc.c
>>> @@ -247,6 +247,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>>>       /* CPU */
>>>       for (i = 0; i < sc->num_cpus; i++) {
>>> +        object_property_set_link(OBJECT(&s->cpu[i]), "memory",
>>> +                                 OBJECT(get_system_memory()), &error_abort);
>>>           if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
>>>               return;
>>>           }
> 

[PATCH v2 2/8] aspeed: Add memory property to Aspeed SoC

[Peter Delevoryas]

Multi-SoC machines can use this property to specify a memory container
for each SoC. Single SoC machines will just specify get_system_memory().

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 hw/arm/aspeed.c             |  4 ++++
 hw/arm/aspeed_ast10x0.c     |  5 ++---
 hw/arm/aspeed_ast2600.c     |  4 ++--
 hw/arm/aspeed_soc.c         | 14 ++++++++------
 include/hw/arm/aspeed_soc.h |  1 +
 5 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index dc09773b0b..b43dc0fda8 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -329,6 +329,8 @@ static void aspeed_machine_init(MachineState *machine)
                             &error_abort);
     object_property_set_int(OBJECT(&bmc->soc), "hw-strap2", amc->hw_strap2,
                             &error_abort);
+    object_property_set_link(OBJECT(&bmc->soc), "memory",
+                             OBJECT(get_system_memory()), &error_abort);
     object_property_set_link(OBJECT(&bmc->soc), "dram",
                              OBJECT(machine->ram), &error_abort);
     if (machine->kernel_filename) {
@@ -1336,6 +1338,8 @@ static void aspeed_minibmc_machine_init(MachineState *machine)
     object_initialize_child(OBJECT(machine), "soc", &bmc->soc, amc->soc_name);
     qdev_connect_clock_in(DEVICE(&bmc->soc), "sysclk", sysclk);
 
+    object_property_set_link(OBJECT(&bmc->soc), "memory",
+                             OBJECT(get_system_memory()), &error_abort);
     qdev_prop_set_uint32(DEVICE(&bmc->soc), "uart-default",
                          amc->uart_default);
     qdev_realize(DEVICE(&bmc->soc), NULL, &error_abort);
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 5df480a21f..e074f80cc7 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -148,7 +148,6 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
 {
     AspeedSoCState *s = ASPEED_SOC(dev_soc);
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
-    MemoryRegion *system_memory = get_system_memory();
     DeviceState *armv7m;
     Error *err = NULL;
     int i;
@@ -172,7 +171,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     qdev_prop_set_string(armv7m, "cpu-type", sc->cpu_type);
     qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
     object_property_set_link(OBJECT(&s->armv7m), "memory",
-                             OBJECT(system_memory), &error_abort);
+                             OBJECT(s->memory), &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(&s->armv7m), &error_abort);
 
     /* Internal SRAM */
@@ -181,7 +180,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
         error_propagate(errp, err);
         return;
     }
-    memory_region_add_subregion(system_memory,
+    memory_region_add_subregion(s->memory,
                                 sc->memmap[ASPEED_DEV_SRAM],
                                 &s->sram);
 
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index f950fff070..d8e5b607e9 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -295,7 +295,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
         object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 1125000000,
                                 &error_abort);
         object_property_set_link(OBJECT(&s->cpu[i]), "memory",
-                                 OBJECT(get_system_memory()), &error_abort);
+                                 OBJECT(s->memory), &error_abort);
 
         if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
             return;
@@ -333,7 +333,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
         error_propagate(errp, err);
         return;
     }
-    memory_region_add_subregion(get_system_memory(),
+    memory_region_add_subregion(s->memory,
                                 sc->memmap[ASPEED_DEV_SRAM], &s->sram);
 
     /* DPMCU */
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 06e5629800..3b531519e9 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -248,7 +248,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     /* CPU */
     for (i = 0; i < sc->num_cpus; i++) {
         object_property_set_link(OBJECT(&s->cpu[i]), "memory",
-                                 OBJECT(get_system_memory()), &error_abort);
+                                 OBJECT(s->memory), &error_abort);
         if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
             return;
         }
@@ -261,7 +261,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
         error_propagate(errp, err);
         return;
     }
-    memory_region_add_subregion(get_system_memory(),
+    memory_region_add_subregion(s->memory,
                                 sc->memmap[ASPEED_DEV_SRAM], &s->sram);
 
     /* SCU */
@@ -456,6 +456,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
                        aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
 }
 static Property aspeed_soc_properties[] = {
+    DEFINE_PROP_LINK("memory", AspeedSoCState, memory, TYPE_MEMORY_REGION,
+                     MemoryRegion *),
     DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
                      MemoryRegion *),
     DEFINE_PROP_UINT32("uart-default", AspeedSoCState, uart_default,
@@ -555,14 +557,14 @@ void aspeed_soc_uart_init(AspeedSoCState *s)
     int i, uart;
 
     /* Attach an 8250 to the IO space as our UART */
-    serial_mm_init(get_system_memory(), sc->memmap[s->uart_default], 2,
+    serial_mm_init(s->memory, sc->memmap[s->uart_default], 2,
                    aspeed_soc_get_irq(s, s->uart_default), 38400,
                    serial_hd(0), DEVICE_LITTLE_ENDIAN);
     for (i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) {
         if (uart == s->uart_default) {
             uart++;
         }
-        serial_mm_init(get_system_memory(), sc->memmap[uart], 2,
+        serial_mm_init(s->memory, sc->memmap[uart], 2,
                        aspeed_soc_get_irq(s, uart), 38400,
                        serial_hd(i), DEVICE_LITTLE_ENDIAN);
     }
@@ -596,7 +598,7 @@ bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp)
     memory_region_add_subregion_overlap(&s->dram_container, ram_size,
                       sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0), -1000);
 
-    memory_region_add_subregion(get_system_memory(),
-                      sc->memmap[ASPEED_DEV_SDRAM], &s->dram_container);
+    memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SDRAM],
+                                &s->dram_container);
     return true;
 }
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index e8a104823d..c8e903b821 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -49,6 +49,7 @@ struct AspeedSoCState {
     ARMCPU cpu[ASPEED_CPUS_NUM];
     A15MPPrivState     a7mpcore;
     ARMv7MState        armv7m;
+    MemoryRegion *memory;
     MemoryRegion *dram_mr;
     MemoryRegion dram_container;
     MemoryRegion sram;
-- 
2.30.2

Re: [PATCH v2 2/8] aspeed: Add memory property to Aspeed SoC

[Cédric Le Goater]

On 6/24/22 02:36, Peter Delevoryas wrote:
> Multi-SoC machines can use this property to specify a memory container
> for each SoC. Single SoC machines will just specify get_system_memory().
> 
> Signed-off-by: Peter Delevoryas <pdel@fb.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   hw/arm/aspeed.c             |  4 ++++
>   hw/arm/aspeed_ast10x0.c     |  5 ++---
>   hw/arm/aspeed_ast2600.c     |  4 ++--
>   hw/arm/aspeed_soc.c         | 14 ++++++++------
>   include/hw/arm/aspeed_soc.h |  1 +
>   5 files changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index dc09773b0b..b43dc0fda8 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -329,6 +329,8 @@ static void aspeed_machine_init(MachineState *machine)
>                               &error_abort);
>       object_property_set_int(OBJECT(&bmc->soc), "hw-strap2", amc->hw_strap2,
>                               &error_abort);
> +    object_property_set_link(OBJECT(&bmc->soc), "memory",
> +                             OBJECT(get_system_memory()), &error_abort);
>       object_property_set_link(OBJECT(&bmc->soc), "dram",
>                                OBJECT(machine->ram), &error_abort);
>       if (machine->kernel_filename) {
> @@ -1336,6 +1338,8 @@ static void aspeed_minibmc_machine_init(MachineState *machine)
>       object_initialize_child(OBJECT(machine), "soc", &bmc->soc, amc->soc_name);
>       qdev_connect_clock_in(DEVICE(&bmc->soc), "sysclk", sysclk);
>   
> +    object_property_set_link(OBJECT(&bmc->soc), "memory",
> +                             OBJECT(get_system_memory()), &error_abort);
>       qdev_prop_set_uint32(DEVICE(&bmc->soc), "uart-default",
>                            amc->uart_default);
>       qdev_realize(DEVICE(&bmc->soc), NULL, &error_abort);
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 5df480a21f..e074f80cc7 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -148,7 +148,6 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>   {
>       AspeedSoCState *s = ASPEED_SOC(dev_soc);
>       AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> -    MemoryRegion *system_memory = get_system_memory();
>       DeviceState *armv7m;
>       Error *err = NULL;
>       int i;
> @@ -172,7 +171,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>       qdev_prop_set_string(armv7m, "cpu-type", sc->cpu_type);
>       qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
>       object_property_set_link(OBJECT(&s->armv7m), "memory",
> -                             OBJECT(system_memory), &error_abort);
> +                             OBJECT(s->memory), &error_abort);
>       sysbus_realize(SYS_BUS_DEVICE(&s->armv7m), &error_abort);
>   
>       /* Internal SRAM */
> @@ -181,7 +180,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>           error_propagate(errp, err);
>           return;
>       }
> -    memory_region_add_subregion(system_memory,
> +    memory_region_add_subregion(s->memory,
>                                   sc->memmap[ASPEED_DEV_SRAM],
>                                   &s->sram);
>   
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index f950fff070..d8e5b607e9 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -295,7 +295,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>           object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 1125000000,
>                                   &error_abort);
>           object_property_set_link(OBJECT(&s->cpu[i]), "memory",
> -                                 OBJECT(get_system_memory()), &error_abort);
> +                                 OBJECT(s->memory), &error_abort);
>   
>           if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
>               return;
> @@ -333,7 +333,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>           error_propagate(errp, err);
>           return;
>       }
> -    memory_region_add_subregion(get_system_memory(),
> +    memory_region_add_subregion(s->memory,
>                                   sc->memmap[ASPEED_DEV_SRAM], &s->sram);
>   
>       /* DPMCU */
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 06e5629800..3b531519e9 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -248,7 +248,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       /* CPU */
>       for (i = 0; i < sc->num_cpus; i++) {
>           object_property_set_link(OBJECT(&s->cpu[i]), "memory",
> -                                 OBJECT(get_system_memory()), &error_abort);
> +                                 OBJECT(s->memory), &error_abort);
>           if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
>               return;
>           }
> @@ -261,7 +261,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>           error_propagate(errp, err);
>           return;
>       }
> -    memory_region_add_subregion(get_system_memory(),
> +    memory_region_add_subregion(s->memory,
>                                   sc->memmap[ASPEED_DEV_SRAM], &s->sram);
>   
>       /* SCU */
> @@ -456,6 +456,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>                          aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
>   }
>   static Property aspeed_soc_properties[] = {
> +    DEFINE_PROP_LINK("memory", AspeedSoCState, memory, TYPE_MEMORY_REGION,
> +                     MemoryRegion *),
>       DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
>                        MemoryRegion *),
>       DEFINE_PROP_UINT32("uart-default", AspeedSoCState, uart_default,
> @@ -555,14 +557,14 @@ void aspeed_soc_uart_init(AspeedSoCState *s)
>       int i, uart;
>   
>       /* Attach an 8250 to the IO space as our UART */
> -    serial_mm_init(get_system_memory(), sc->memmap[s->uart_default], 2,
> +    serial_mm_init(s->memory, sc->memmap[s->uart_default], 2,
>                      aspeed_soc_get_irq(s, s->uart_default), 38400,
>                      serial_hd(0), DEVICE_LITTLE_ENDIAN);
>       for (i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) {
>           if (uart == s->uart_default) {
>               uart++;
>           }
> -        serial_mm_init(get_system_memory(), sc->memmap[uart], 2,
> +        serial_mm_init(s->memory, sc->memmap[uart], 2,
>                          aspeed_soc_get_irq(s, uart), 38400,
>                          serial_hd(i), DEVICE_LITTLE_ENDIAN);
>       }
> @@ -596,7 +598,7 @@ bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp)
>       memory_region_add_subregion_overlap(&s->dram_container, ram_size,
>                         sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0), -1000);
>   
> -    memory_region_add_subregion(get_system_memory(),
> -                      sc->memmap[ASPEED_DEV_SDRAM], &s->dram_container);
> +    memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SDRAM],
> +                                &s->dram_container);
>       return true;
>   }
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index e8a104823d..c8e903b821 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -49,6 +49,7 @@ struct AspeedSoCState {
>       ARMCPU cpu[ASPEED_CPUS_NUM];
>       A15MPPrivState     a7mpcore;
>       ARMv7MState        armv7m;
> +    MemoryRegion *memory;
>       MemoryRegion *dram_mr;
>       MemoryRegion dram_container;
>       MemoryRegion sram;

[PATCH v2 3/8] aspeed: Remove usage of sysbus_mmio_map

[Peter Delevoryas]

sysbus_mmio_map maps devices into "get_system_memory()".

With the new SoC memory attribute, we want to make sure that each device is
mapped into the SoC memory.

In single SoC machines, the SoC memory is the same as "get_system_memory()",
but in multi SoC machines it will be different.

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 hw/arm/aspeed_ast10x0.c     | 25 +++++++++---------
 hw/arm/aspeed_ast2600.c     | 51 ++++++++++++++++++++-----------------
 hw/arm/aspeed_soc.c         | 47 ++++++++++++++++++++--------------
 include/hw/arm/aspeed_soc.h |  1 +
 4 files changed, 69 insertions(+), 55 deletions(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index e074f80cc7..f8f321374a 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -188,7 +188,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
 
     /* I2C */
 
@@ -197,7 +197,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
     for (i = 0; i < ASPEED_I2C_GET_CLASS(&s->i2c)->num_busses; i++) {
         qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->armv7m),
                                         sc->irqmap[ASPEED_DEV_I2C] + i);
@@ -209,7 +209,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->lpc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
 
     /* Connect the LPC IRQ to the GIC. It is otherwise unused. */
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 0,
@@ -243,7 +243,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->timerctrl), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->timerctrl), 0,
                     sc->memmap[ASPEED_DEV_TIMER1]);
     for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
         qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
@@ -254,7 +254,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->adc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->adc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_ADC));
 
@@ -264,8 +264,8 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->fmc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 1,
                     ASPEED_SMC_GET_CLASS(&s->fmc)->flash_window_base);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_FMC));
@@ -277,9 +277,9 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
             return;
         }
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 0,
                         sc->memmap[ASPEED_DEV_SPI1 + i]);
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 1,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 1,
                         ASPEED_SMC_GET_CLASS(&s->spi[i])->flash_window_base);
     }
 
@@ -287,7 +287,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->sbc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
 
     /* Watch dog */
     for (i = 0; i < sc->wdts_num; i++) {
@@ -298,7 +298,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->wdt[i]), errp)) {
             return;
         }
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->wdt[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
                         sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
     }
 
@@ -306,7 +306,8 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, sc->memmap[ASPEED_DEV_GPIO]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->gpio), 0,
+                    sc->memmap[ASPEED_DEV_GPIO]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_GPIO));
 }
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index d8e5b607e9..477dd36042 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -310,7 +310,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
                             &error_abort);
 
     sysbus_realize(SYS_BUS_DEVICE(&s->a7mpcore), &error_abort);
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->a7mpcore), 0, ASPEED_A7MPCORE_ADDR);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->a7mpcore), 0, ASPEED_A7MPCORE_ADDR);
 
     for (i = 0; i < sc->num_cpus; i++) {
         SysBusDevice *sbd = SYS_BUS_DEVICE(&s->a7mpcore);
@@ -344,13 +344,13 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
 
     /* RTC */
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->rtc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->rtc), 0, sc->memmap[ASPEED_DEV_RTC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->rtc), 0, sc->memmap[ASPEED_DEV_RTC]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->rtc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_RTC));
 
@@ -360,7 +360,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->timerctrl), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->timerctrl), 0,
                     sc->memmap[ASPEED_DEV_TIMER1]);
     for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
         qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
@@ -371,7 +371,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->adc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->adc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_ADC));
 
@@ -384,7 +384,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
     for (i = 0; i < ASPEED_I2C_GET_CLASS(&s->i2c)->num_busses; i++) {
         qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
                                         sc->irqmap[ASPEED_DEV_I2C] + i);
@@ -398,8 +398,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->fmc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 1,
                     ASPEED_SMC_GET_CLASS(&s->fmc)->flash_window_base);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_FMC));
@@ -411,9 +411,9 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
             return;
         }
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 0,
                         sc->memmap[ASPEED_DEV_SPI1 + i]);
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 1,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 1,
                         ASPEED_SMC_GET_CLASS(&s->spi[i])->flash_window_base);
     }
 
@@ -422,7 +422,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->ehci[i]), errp)) {
             return;
         }
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->ehci[i]), 0,
                         sc->memmap[ASPEED_DEV_EHCI1 + i]);
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                            aspeed_soc_get_irq(s, ASPEED_DEV_EHCI1 + i));
@@ -432,7 +432,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->sdmc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdmc), 0, sc->memmap[ASPEED_DEV_SDMC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sdmc), 0,
+                    sc->memmap[ASPEED_DEV_SDMC]);
 
     /* Watch dog */
     for (i = 0; i < sc->wdts_num; i++) {
@@ -443,7 +444,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->wdt[i]), errp)) {
             return;
         }
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->wdt[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
                         sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
     }
 
@@ -454,7 +455,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), errp)) {
             return;
         }
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
                         sc->memmap[ASPEED_DEV_ETH1 + i]);
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
                            aspeed_soc_get_irq(s, ASPEED_DEV_ETH1 + i));
@@ -465,7 +466,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
             return;
         }
 
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->mii[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->mii[i]), 0,
                         sc->memmap[ASPEED_DEV_MII1 + i]);
     }
 
@@ -473,7 +474,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->xdma), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->xdma), 0,
                     sc->memmap[ASPEED_DEV_XDMA]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_XDMA));
@@ -482,14 +483,14 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, sc->memmap[ASPEED_DEV_GPIO]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->gpio), 0, sc->memmap[ASPEED_DEV_GPIO]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_GPIO));
 
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio_1_8v), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio_1_8v), 0,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->gpio_1_8v), 0,
                     sc->memmap[ASPEED_DEV_GPIO_1_8V]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio_1_8v), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_GPIO_1_8V));
@@ -498,7 +499,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->sdhci), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdhci), 0,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sdhci), 0,
                     sc->memmap[ASPEED_DEV_SDHCI]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_SDHCI));
@@ -507,7 +508,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->emmc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->emmc), 0, sc->memmap[ASPEED_DEV_EMMC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->emmc), 0,
+                    sc->memmap[ASPEED_DEV_EMMC]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_EMMC));
 
@@ -515,7 +517,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->lpc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
 
     /* Connect the LPC IRQ to the GIC. It is otherwise unused. */
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 0,
@@ -551,7 +553,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->hace), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->hace), 0,
+                    sc->memmap[ASPEED_DEV_HACE]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
 
@@ -559,7 +562,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->i3c), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
     for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
         qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
                                         sc->irqmap[ASPEED_DEV_I3C] + i);
@@ -571,7 +574,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->sbc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
 }
 
 static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 3b531519e9..46cbc63644 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -268,13 +268,13 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
 
     /* VIC */
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->vic), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->vic), 0, sc->memmap[ASPEED_DEV_VIC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->vic), 0, sc->memmap[ASPEED_DEV_VIC]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->vic), 0,
                        qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ));
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->vic), 1,
@@ -284,7 +284,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->rtc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->rtc), 0, sc->memmap[ASPEED_DEV_RTC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->rtc), 0, sc->memmap[ASPEED_DEV_RTC]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->rtc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_RTC));
 
@@ -294,7 +294,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->timerctrl), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->timerctrl), 0,
                     sc->memmap[ASPEED_DEV_TIMER1]);
     for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
         qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
@@ -305,7 +305,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->adc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->adc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_ADC));
 
@@ -318,7 +318,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_I2C));
 
@@ -328,8 +328,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->fmc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 1,
                     ASPEED_SMC_GET_CLASS(&s->fmc)->flash_window_base);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_FMC));
@@ -339,9 +339,9 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
             return;
         }
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 0,
                         sc->memmap[ASPEED_DEV_SPI1 + i]);
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 1,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 1,
                         ASPEED_SMC_GET_CLASS(&s->spi[i])->flash_window_base);
     }
 
@@ -350,7 +350,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->ehci[i]), errp)) {
             return;
         }
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->ehci[i]), 0,
                         sc->memmap[ASPEED_DEV_EHCI1 + i]);
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                            aspeed_soc_get_irq(s, ASPEED_DEV_EHCI1 + i));
@@ -360,7 +360,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->sdmc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdmc), 0, sc->memmap[ASPEED_DEV_SDMC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sdmc), 0,
+                    sc->memmap[ASPEED_DEV_SDMC]);
 
     /* Watch dog */
     for (i = 0; i < sc->wdts_num; i++) {
@@ -371,7 +372,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->wdt[i]), errp)) {
             return;
         }
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->wdt[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
                         sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
     }
 
@@ -382,7 +383,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), errp)) {
             return;
         }
-        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
                         sc->memmap[ASPEED_DEV_ETH1 + i]);
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
                            aspeed_soc_get_irq(s, ASPEED_DEV_ETH1 + i));
@@ -392,7 +393,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->xdma), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->xdma), 0,
                     sc->memmap[ASPEED_DEV_XDMA]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_XDMA));
@@ -401,7 +402,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, sc->memmap[ASPEED_DEV_GPIO]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->gpio), 0,
+                    sc->memmap[ASPEED_DEV_GPIO]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_GPIO));
 
@@ -409,7 +411,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->sdhci), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdhci), 0,
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sdhci), 0,
                     sc->memmap[ASPEED_DEV_SDHCI]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_SDHCI));
@@ -418,7 +420,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->lpc), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
 
     /* Connect the LPC IRQ to the VIC */
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 0,
@@ -451,7 +453,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->hace), errp)) {
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->hace), 0,
+                    sc->memmap[ASPEED_DEV_HACE]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
 }
@@ -602,3 +605,9 @@ bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp)
                                 &s->dram_container);
     return true;
 }
+
+void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr)
+{
+    memory_region_add_subregion(s->memory, addr,
+                                sysbus_mmio_get_region(dev, n));
+}
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index c8e903b821..1ab328d00c 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -168,5 +168,6 @@ enum {
 qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
 void aspeed_soc_uart_init(AspeedSoCState *s);
 bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp);
+void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr);
 
 #endif /* ASPEED_SOC_H */
-- 
2.30.2

Re: [PATCH v2 3/8] aspeed: Remove usage of sysbus_mmio_map

[Cédric Le Goater]

On 6/24/22 02:36, Peter Delevoryas wrote:
> sysbus_mmio_map maps devices into "get_system_memory()".
> 
> With the new SoC memory attribute, we want to make sure that each device is
> mapped into the SoC memory.
> 
> In single SoC machines, the SoC memory is the same as "get_system_memory()",
> but in multi SoC machines it will be different.
> 
> Signed-off-by: Peter Delevoryas <pdel@fb.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>


Thanks,

C.


> ---
>   hw/arm/aspeed_ast10x0.c     | 25 +++++++++---------
>   hw/arm/aspeed_ast2600.c     | 51 ++++++++++++++++++++-----------------
>   hw/arm/aspeed_soc.c         | 47 ++++++++++++++++++++--------------
>   include/hw/arm/aspeed_soc.h |  1 +
>   4 files changed, 69 insertions(+), 55 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index e074f80cc7..f8f321374a 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -188,7 +188,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
>   
>       /* I2C */
>   
> @@ -197,7 +197,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
>       for (i = 0; i < ASPEED_I2C_GET_CLASS(&s->i2c)->num_busses; i++) {
>           qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->armv7m),
>                                           sc->irqmap[ASPEED_DEV_I2C] + i);
> @@ -209,7 +209,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->lpc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
>   
>       /* Connect the LPC IRQ to the GIC. It is otherwise unused. */
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 0,
> @@ -243,7 +243,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->timerctrl), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->timerctrl), 0,
>                       sc->memmap[ASPEED_DEV_TIMER1]);
>       for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
>           qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
> @@ -254,7 +254,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->adc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->adc), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_ADC));
>   
> @@ -264,8 +264,8 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->fmc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 1,
>                       ASPEED_SMC_GET_CLASS(&s->fmc)->flash_window_base);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_FMC));
> @@ -277,9 +277,9 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>           if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
>               return;
>           }
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 0,
>                           sc->memmap[ASPEED_DEV_SPI1 + i]);
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 1,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 1,
>                           ASPEED_SMC_GET_CLASS(&s->spi[i])->flash_window_base);
>       }
>   
> @@ -287,7 +287,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->sbc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
>   
>       /* Watch dog */
>       for (i = 0; i < sc->wdts_num; i++) {
> @@ -298,7 +298,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>           if (!sysbus_realize(SYS_BUS_DEVICE(&s->wdt[i]), errp)) {
>               return;
>           }
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->wdt[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
>                           sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
>       }
>   
> @@ -306,7 +306,8 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, sc->memmap[ASPEED_DEV_GPIO]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->gpio), 0,
> +                    sc->memmap[ASPEED_DEV_GPIO]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_GPIO));
>   }
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index d8e5b607e9..477dd36042 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -310,7 +310,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>                               &error_abort);
>   
>       sysbus_realize(SYS_BUS_DEVICE(&s->a7mpcore), &error_abort);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->a7mpcore), 0, ASPEED_A7MPCORE_ADDR);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->a7mpcore), 0, ASPEED_A7MPCORE_ADDR);
>   
>       for (i = 0; i < sc->num_cpus; i++) {
>           SysBusDevice *sbd = SYS_BUS_DEVICE(&s->a7mpcore);
> @@ -344,13 +344,13 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
>   
>       /* RTC */
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->rtc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->rtc), 0, sc->memmap[ASPEED_DEV_RTC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->rtc), 0, sc->memmap[ASPEED_DEV_RTC]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->rtc), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_RTC));
>   
> @@ -360,7 +360,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->timerctrl), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->timerctrl), 0,
>                       sc->memmap[ASPEED_DEV_TIMER1]);
>       for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
>           qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
> @@ -371,7 +371,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->adc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->adc), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_ADC));
>   
> @@ -384,7 +384,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
>       for (i = 0; i < ASPEED_I2C_GET_CLASS(&s->i2c)->num_busses; i++) {
>           qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
>                                           sc->irqmap[ASPEED_DEV_I2C] + i);
> @@ -398,8 +398,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->fmc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 1,
>                       ASPEED_SMC_GET_CLASS(&s->fmc)->flash_window_base);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_FMC));
> @@ -411,9 +411,9 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>           if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
>               return;
>           }
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 0,
>                           sc->memmap[ASPEED_DEV_SPI1 + i]);
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 1,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 1,
>                           ASPEED_SMC_GET_CLASS(&s->spi[i])->flash_window_base);
>       }
>   
> @@ -422,7 +422,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>           if (!sysbus_realize(SYS_BUS_DEVICE(&s->ehci[i]), errp)) {
>               return;
>           }
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->ehci[i]), 0,
>                           sc->memmap[ASPEED_DEV_EHCI1 + i]);
>           sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
>                              aspeed_soc_get_irq(s, ASPEED_DEV_EHCI1 + i));
> @@ -432,7 +432,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->sdmc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdmc), 0, sc->memmap[ASPEED_DEV_SDMC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sdmc), 0,
> +                    sc->memmap[ASPEED_DEV_SDMC]);
>   
>       /* Watch dog */
>       for (i = 0; i < sc->wdts_num; i++) {
> @@ -443,7 +444,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>           if (!sysbus_realize(SYS_BUS_DEVICE(&s->wdt[i]), errp)) {
>               return;
>           }
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->wdt[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
>                           sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
>       }
>   
> @@ -454,7 +455,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>           if (!sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), errp)) {
>               return;
>           }
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
>                           sc->memmap[ASPEED_DEV_ETH1 + i]);
>           sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
>                              aspeed_soc_get_irq(s, ASPEED_DEV_ETH1 + i));
> @@ -465,7 +466,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>               return;
>           }
>   
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->mii[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->mii[i]), 0,
>                           sc->memmap[ASPEED_DEV_MII1 + i]);
>       }
>   
> @@ -473,7 +474,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->xdma), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->xdma), 0,
>                       sc->memmap[ASPEED_DEV_XDMA]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_XDMA));
> @@ -482,14 +483,14 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, sc->memmap[ASPEED_DEV_GPIO]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->gpio), 0, sc->memmap[ASPEED_DEV_GPIO]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_GPIO));
>   
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio_1_8v), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio_1_8v), 0,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->gpio_1_8v), 0,
>                       sc->memmap[ASPEED_DEV_GPIO_1_8V]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio_1_8v), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_GPIO_1_8V));
> @@ -498,7 +499,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->sdhci), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdhci), 0,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sdhci), 0,
>                       sc->memmap[ASPEED_DEV_SDHCI]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_SDHCI));
> @@ -507,7 +508,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->emmc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->emmc), 0, sc->memmap[ASPEED_DEV_EMMC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->emmc), 0,
> +                    sc->memmap[ASPEED_DEV_EMMC]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_EMMC));
>   
> @@ -515,7 +517,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->lpc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
>   
>       /* Connect the LPC IRQ to the GIC. It is otherwise unused. */
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 0,
> @@ -551,7 +553,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->hace), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->hace), 0,
> +                    sc->memmap[ASPEED_DEV_HACE]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
>   
> @@ -559,7 +562,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->i3c), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
>       for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
>           qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
>                                           sc->irqmap[ASPEED_DEV_I3C] + i);
> @@ -571,7 +574,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->sbc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
>   }
>   
>   static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 3b531519e9..46cbc63644 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -268,13 +268,13 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
>   
>       /* VIC */
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->vic), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->vic), 0, sc->memmap[ASPEED_DEV_VIC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->vic), 0, sc->memmap[ASPEED_DEV_VIC]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->vic), 0,
>                          qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ));
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->vic), 1,
> @@ -284,7 +284,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->rtc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->rtc), 0, sc->memmap[ASPEED_DEV_RTC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->rtc), 0, sc->memmap[ASPEED_DEV_RTC]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->rtc), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_RTC));
>   
> @@ -294,7 +294,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->timerctrl), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->timerctrl), 0,
>                       sc->memmap[ASPEED_DEV_TIMER1]);
>       for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
>           qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
> @@ -305,7 +305,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->adc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->adc), 0, sc->memmap[ASPEED_DEV_ADC]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->adc), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_ADC));
>   
> @@ -318,7 +318,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_I2C));
>   
> @@ -328,8 +328,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->fmc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 0, sc->memmap[ASPEED_DEV_FMC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fmc), 1,
>                       ASPEED_SMC_GET_CLASS(&s->fmc)->flash_window_base);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_FMC));
> @@ -339,9 +339,9 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>           if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
>               return;
>           }
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 0,
>                           sc->memmap[ASPEED_DEV_SPI1 + i]);
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 1,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->spi[i]), 1,
>                           ASPEED_SMC_GET_CLASS(&s->spi[i])->flash_window_base);
>       }
>   
> @@ -350,7 +350,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>           if (!sysbus_realize(SYS_BUS_DEVICE(&s->ehci[i]), errp)) {
>               return;
>           }
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->ehci[i]), 0,
>                           sc->memmap[ASPEED_DEV_EHCI1 + i]);
>           sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
>                              aspeed_soc_get_irq(s, ASPEED_DEV_EHCI1 + i));
> @@ -360,7 +360,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->sdmc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdmc), 0, sc->memmap[ASPEED_DEV_SDMC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sdmc), 0,
> +                    sc->memmap[ASPEED_DEV_SDMC]);
>   
>       /* Watch dog */
>       for (i = 0; i < sc->wdts_num; i++) {
> @@ -371,7 +372,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>           if (!sysbus_realize(SYS_BUS_DEVICE(&s->wdt[i]), errp)) {
>               return;
>           }
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->wdt[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0,
>                           sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
>       }
>   
> @@ -382,7 +383,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>           if (!sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), errp)) {
>               return;
>           }
> -        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
> +        aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
>                           sc->memmap[ASPEED_DEV_ETH1 + i]);
>           sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
>                              aspeed_soc_get_irq(s, ASPEED_DEV_ETH1 + i));
> @@ -392,7 +393,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->xdma), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->xdma), 0,
>                       sc->memmap[ASPEED_DEV_XDMA]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_XDMA));
> @@ -401,7 +402,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, sc->memmap[ASPEED_DEV_GPIO]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->gpio), 0,
> +                    sc->memmap[ASPEED_DEV_GPIO]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_GPIO));
>   
> @@ -409,7 +411,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->sdhci), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdhci), 0,
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sdhci), 0,
>                       sc->memmap[ASPEED_DEV_SDHCI]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_SDHCI));
> @@ -418,7 +420,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->lpc), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
>   
>       /* Connect the LPC IRQ to the VIC */
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 0,
> @@ -451,7 +453,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->hace), errp)) {
>           return;
>       }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
> +    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->hace), 0,
> +                    sc->memmap[ASPEED_DEV_HACE]);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
>                          aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
>   }
> @@ -602,3 +605,9 @@ bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp)
>                                   &s->dram_container);
>       return true;
>   }
> +
> +void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr)
> +{
> +    memory_region_add_subregion(s->memory, addr,
> +                                sysbus_mmio_get_region(dev, n));
> +}
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index c8e903b821..1ab328d00c 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -168,5 +168,6 @@ enum {
>   qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
>   void aspeed_soc_uart_init(AspeedSoCState *s);
>   bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp);
> +void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr);
>   
>   #endif /* ASPEED_SOC_H */

[PATCH v2 4/8] aspeed: Map unimplemented devices in SoC memory

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 hw/arm/aspeed_ast10x0.c     | 16 ++++++++++------
 hw/arm/aspeed_ast2600.c     | 27 ++++++++++++++++++---------
 hw/arm/aspeed_soc.c         | 23 +++++++++++++++++++----
 include/hw/arm/aspeed_soc.h |  9 +++++++++
 4 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index f8f321374a..d34c06db16 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -142,6 +142,10 @@ static void aspeed_soc_ast1030_init(Object *obj)
 
     snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
     object_initialize_child(obj, "gpio", &s->gpio, typename);
+
+    object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
+    object_initialize_child(obj, "sbc-unimplemented", &s->sbc_unimplemented,
+                            TYPE_UNIMPLEMENTED_DEVICE);
 }
 
 static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
@@ -158,12 +162,12 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     }
 
     /* General I/O memory space to catch all unimplemented device */
-    create_unimplemented_device("aspeed.sbc",
-                                sc->memmap[ASPEED_DEV_SBC],
-                                0x40000);
-    create_unimplemented_device("aspeed.io",
-                                sc->memmap[ASPEED_DEV_IOMEM],
-                                ASPEED_SOC_IOMEM_SIZE);
+    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem), "aspeed.io",
+                                  sc->memmap[ASPEED_DEV_IOMEM],
+                                  ASPEED_SOC_IOMEM_SIZE);
+    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->sbc_unimplemented),
+                                  "aspeed.sbc", sc->memmap[ASPEED_DEV_SBC],
+                                  0x40000);
 
     /* AST1030 CPU Core */
     armv7m = DEVICE(&s->armv7m);
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 477dd36042..8693660271 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -246,6 +246,13 @@ static void aspeed_soc_ast2600_init(Object *obj)
     object_initialize_child(obj, "i3c", &s->i3c, TYPE_ASPEED_I3C);
 
     object_initialize_child(obj, "sbc", &s->sbc, TYPE_ASPEED_SBC);
+
+    object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
+    object_initialize_child(obj, "video", &s->video, TYPE_UNIMPLEMENTED_DEVICE);
+    object_initialize_child(obj, "dpmcu", &s->dpmcu, TYPE_UNIMPLEMENTED_DEVICE);
+    object_initialize_child(obj, "emmc-boot-controller",
+                            &s->emmc_boot_controller,
+                            TYPE_UNIMPLEMENTED_DEVICE);
 }
 
 /*
@@ -267,21 +274,22 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     qemu_irq irq;
 
     /* IO space */
-    create_unimplemented_device("aspeed_soc.io", sc->memmap[ASPEED_DEV_IOMEM],
-                                ASPEED_SOC_IOMEM_SIZE);
+    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem), "aspeed.io",
+                                  sc->memmap[ASPEED_DEV_IOMEM],
+                                  ASPEED_SOC_IOMEM_SIZE);
     /* RAM */
     if (!aspeed_soc_dram_init(s, errp)) {
         return;
     }
 
     /* Video engine stub */
-    create_unimplemented_device("aspeed.video", sc->memmap[ASPEED_DEV_VIDEO],
-                                0x1000);
+    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->video), "aspeed.video",
+                                  sc->memmap[ASPEED_DEV_VIDEO], 0x1000);
 
     /* eMMC Boot Controller stub */
-    create_unimplemented_device("aspeed.emmc-boot-controller",
-                                sc->memmap[ASPEED_DEV_EMMC_BC],
-                                0x1000);
+    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->emmc_boot_controller),
+                                  "aspeed.emmc-boot-controller",
+                                  sc->memmap[ASPEED_DEV_EMMC_BC], 0x1000);
 
     /* CPU */
     for (i = 0; i < sc->num_cpus; i++) {
@@ -337,8 +345,9 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
                                 sc->memmap[ASPEED_DEV_SRAM], &s->sram);
 
     /* DPMCU */
-    create_unimplemented_device("aspeed.dpmcu", sc->memmap[ASPEED_DEV_DPMCU],
-                                ASPEED_SOC_DPMCU_SIZE);
+    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->dpmcu), "aspeed.dpmcu",
+                                  sc->memmap[ASPEED_DEV_DPMCU],
+                                  ASPEED_SOC_DPMCU_SIZE);
 
     /* SCU */
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 46cbc63644..16394c8e0b 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -223,6 +223,9 @@ static void aspeed_soc_init(Object *obj)
 
     snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
     object_initialize_child(obj, "hace", &s->hace, typename);
+
+    object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
+    object_initialize_child(obj, "video", &s->video, TYPE_UNIMPLEMENTED_DEVICE);
 }
 
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
@@ -233,8 +236,9 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     Error *err = NULL;
 
     /* IO space */
-    create_unimplemented_device("aspeed_soc.io", sc->memmap[ASPEED_DEV_IOMEM],
-                                ASPEED_SOC_IOMEM_SIZE);
+    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem), "aspeed.io",
+                                  sc->memmap[ASPEED_DEV_IOMEM],
+                                  ASPEED_SOC_IOMEM_SIZE);
 
     /* RAM */
     if (!aspeed_soc_dram_init(s, errp)) {
@@ -242,8 +246,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     }
 
     /* Video engine stub */
-    create_unimplemented_device("aspeed.video", sc->memmap[ASPEED_DEV_VIDEO],
-                                0x1000);
+    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->video), "aspeed.video",
+                                  sc->memmap[ASPEED_DEV_VIDEO], 0x1000);
 
     /* CPU */
     for (i = 0; i < sc->num_cpus; i++) {
@@ -611,3 +615,14 @@ void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr)
     memory_region_add_subregion(s->memory, addr,
                                 sysbus_mmio_get_region(dev, n));
 }
+
+void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
+                                   const char *name, hwaddr addr, uint64_t size)
+{
+    qdev_prop_set_string(DEVICE(dev), "name", name);
+    qdev_prop_set_uint64(DEVICE(dev), "size", size);
+    sysbus_realize(dev, &error_abort);
+
+    memory_region_add_subregion_overlap(s->memory, addr,
+                                        sysbus_mmio_get_region(dev, 0), -1000);
+}
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 1ab328d00c..6cfc063985 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -34,6 +34,7 @@
 #include "hw/usb/hcd-ehci.h"
 #include "qom/object.h"
 #include "hw/misc/aspeed_lpc.h"
+#include "hw/misc/unimp.h"
 
 #define ASPEED_SPIS_NUM  2
 #define ASPEED_EHCIS_NUM 2
@@ -66,6 +67,7 @@ struct AspeedSoCState {
     AspeedSMCState spi[ASPEED_SPIS_NUM];
     EHCISysBusState ehci[ASPEED_EHCIS_NUM];
     AspeedSBCState sbc;
+    UnimplementedDeviceState sbc_unimplemented;
     AspeedSDMCState sdmc;
     AspeedWDTState wdt[ASPEED_WDTS_NUM];
     FTGMAC100State ftgmac100[ASPEED_MACS_NUM];
@@ -77,6 +79,10 @@ struct AspeedSoCState {
     AspeedLPCState lpc;
     uint32_t uart_default;
     Clock *sysclk;
+    UnimplementedDeviceState iomem;
+    UnimplementedDeviceState video;
+    UnimplementedDeviceState emmc_boot_controller;
+    UnimplementedDeviceState dpmcu;
 };
 
 #define TYPE_ASPEED_SOC "aspeed-soc"
@@ -169,5 +175,8 @@ qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
 void aspeed_soc_uart_init(AspeedSoCState *s);
 bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp);
 void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr);
+void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
+                                   const char *name, hwaddr addr,
+                                   uint64_t size);
 
 #endif /* ASPEED_SOC_H */
-- 
2.30.2

Re: [PATCH v2 4/8] aspeed: Map unimplemented devices in SoC memory

[Cédric Le Goater]

On 6/24/22 02:36, Peter Delevoryas wrote:
> Signed-off-by: Peter Delevoryas <pdel@fb.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>


Thanks,

C.


> ---
>   hw/arm/aspeed_ast10x0.c     | 16 ++++++++++------
>   hw/arm/aspeed_ast2600.c     | 27 ++++++++++++++++++---------
>   hw/arm/aspeed_soc.c         | 23 +++++++++++++++++++----
>   include/hw/arm/aspeed_soc.h |  9 +++++++++
>   4 files changed, 56 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index f8f321374a..d34c06db16 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -142,6 +142,10 @@ static void aspeed_soc_ast1030_init(Object *obj)
>   
>       snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
>       object_initialize_child(obj, "gpio", &s->gpio, typename);
> +
> +    object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
> +    object_initialize_child(obj, "sbc-unimplemented", &s->sbc_unimplemented,
> +                            TYPE_UNIMPLEMENTED_DEVICE);
>   }
>   
>   static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> @@ -158,12 +162,12 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
>       }
>   
>       /* General I/O memory space to catch all unimplemented device */
> -    create_unimplemented_device("aspeed.sbc",
> -                                sc->memmap[ASPEED_DEV_SBC],
> -                                0x40000);
> -    create_unimplemented_device("aspeed.io",
> -                                sc->memmap[ASPEED_DEV_IOMEM],
> -                                ASPEED_SOC_IOMEM_SIZE);
> +    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem), "aspeed.io",
> +                                  sc->memmap[ASPEED_DEV_IOMEM],
> +                                  ASPEED_SOC_IOMEM_SIZE);
> +    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->sbc_unimplemented),
> +                                  "aspeed.sbc", sc->memmap[ASPEED_DEV_SBC],
> +                                  0x40000);
>   
>       /* AST1030 CPU Core */
>       armv7m = DEVICE(&s->armv7m);
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 477dd36042..8693660271 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -246,6 +246,13 @@ static void aspeed_soc_ast2600_init(Object *obj)
>       object_initialize_child(obj, "i3c", &s->i3c, TYPE_ASPEED_I3C);
>   
>       object_initialize_child(obj, "sbc", &s->sbc, TYPE_ASPEED_SBC);
> +
> +    object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
> +    object_initialize_child(obj, "video", &s->video, TYPE_UNIMPLEMENTED_DEVICE);
> +    object_initialize_child(obj, "dpmcu", &s->dpmcu, TYPE_UNIMPLEMENTED_DEVICE);
> +    object_initialize_child(obj, "emmc-boot-controller",
> +                            &s->emmc_boot_controller,
> +                            TYPE_UNIMPLEMENTED_DEVICE);
>   }
>   
>   /*
> @@ -267,21 +274,22 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       qemu_irq irq;
>   
>       /* IO space */
> -    create_unimplemented_device("aspeed_soc.io", sc->memmap[ASPEED_DEV_IOMEM],
> -                                ASPEED_SOC_IOMEM_SIZE);
> +    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem), "aspeed.io",
> +                                  sc->memmap[ASPEED_DEV_IOMEM],
> +                                  ASPEED_SOC_IOMEM_SIZE);
>       /* RAM */
>       if (!aspeed_soc_dram_init(s, errp)) {
>           return;
>       }
>   
>       /* Video engine stub */
> -    create_unimplemented_device("aspeed.video", sc->memmap[ASPEED_DEV_VIDEO],
> -                                0x1000);
> +    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->video), "aspeed.video",
> +                                  sc->memmap[ASPEED_DEV_VIDEO], 0x1000);
>   
>       /* eMMC Boot Controller stub */
> -    create_unimplemented_device("aspeed.emmc-boot-controller",
> -                                sc->memmap[ASPEED_DEV_EMMC_BC],
> -                                0x1000);
> +    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->emmc_boot_controller),
> +                                  "aspeed.emmc-boot-controller",
> +                                  sc->memmap[ASPEED_DEV_EMMC_BC], 0x1000);
>   
>       /* CPU */
>       for (i = 0; i < sc->num_cpus; i++) {
> @@ -337,8 +345,9 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>                                   sc->memmap[ASPEED_DEV_SRAM], &s->sram);
>   
>       /* DPMCU */
> -    create_unimplemented_device("aspeed.dpmcu", sc->memmap[ASPEED_DEV_DPMCU],
> -                                ASPEED_SOC_DPMCU_SIZE);
> +    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->dpmcu), "aspeed.dpmcu",
> +                                  sc->memmap[ASPEED_DEV_DPMCU],
> +                                  ASPEED_SOC_DPMCU_SIZE);
>   
>       /* SCU */
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 46cbc63644..16394c8e0b 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -223,6 +223,9 @@ static void aspeed_soc_init(Object *obj)
>   
>       snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
>       object_initialize_child(obj, "hace", &s->hace, typename);
> +
> +    object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
> +    object_initialize_child(obj, "video", &s->video, TYPE_UNIMPLEMENTED_DEVICE);
>   }
>   
>   static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> @@ -233,8 +236,9 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       Error *err = NULL;
>   
>       /* IO space */
> -    create_unimplemented_device("aspeed_soc.io", sc->memmap[ASPEED_DEV_IOMEM],
> -                                ASPEED_SOC_IOMEM_SIZE);
> +    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem), "aspeed.io",
> +                                  sc->memmap[ASPEED_DEV_IOMEM],
> +                                  ASPEED_SOC_IOMEM_SIZE);
>   
>       /* RAM */
>       if (!aspeed_soc_dram_init(s, errp)) {
> @@ -242,8 +246,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>       }
>   
>       /* Video engine stub */
> -    create_unimplemented_device("aspeed.video", sc->memmap[ASPEED_DEV_VIDEO],
> -                                0x1000);
> +    aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->video), "aspeed.video",
> +                                  sc->memmap[ASPEED_DEV_VIDEO], 0x1000);
>   
>       /* CPU */
>       for (i = 0; i < sc->num_cpus; i++) {
> @@ -611,3 +615,14 @@ void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr)
>       memory_region_add_subregion(s->memory, addr,
>                                   sysbus_mmio_get_region(dev, n));
>   }
> +
> +void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
> +                                   const char *name, hwaddr addr, uint64_t size)
> +{
> +    qdev_prop_set_string(DEVICE(dev), "name", name);
> +    qdev_prop_set_uint64(DEVICE(dev), "size", size);
> +    sysbus_realize(dev, &error_abort);
> +
> +    memory_region_add_subregion_overlap(s->memory, addr,
> +                                        sysbus_mmio_get_region(dev, 0), -1000);
> +}
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 1ab328d00c..6cfc063985 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -34,6 +34,7 @@
>   #include "hw/usb/hcd-ehci.h"
>   #include "qom/object.h"
>   #include "hw/misc/aspeed_lpc.h"
> +#include "hw/misc/unimp.h"
>   
>   #define ASPEED_SPIS_NUM  2
>   #define ASPEED_EHCIS_NUM 2
> @@ -66,6 +67,7 @@ struct AspeedSoCState {
>       AspeedSMCState spi[ASPEED_SPIS_NUM];
>       EHCISysBusState ehci[ASPEED_EHCIS_NUM];
>       AspeedSBCState sbc;
> +    UnimplementedDeviceState sbc_unimplemented;
>       AspeedSDMCState sdmc;
>       AspeedWDTState wdt[ASPEED_WDTS_NUM];
>       FTGMAC100State ftgmac100[ASPEED_MACS_NUM];
> @@ -77,6 +79,10 @@ struct AspeedSoCState {
>       AspeedLPCState lpc;
>       uint32_t uart_default;
>       Clock *sysclk;
> +    UnimplementedDeviceState iomem;
> +    UnimplementedDeviceState video;
> +    UnimplementedDeviceState emmc_boot_controller;
> +    UnimplementedDeviceState dpmcu;
>   };
>   
>   #define TYPE_ASPEED_SOC "aspeed-soc"
> @@ -169,5 +175,8 @@ qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
>   void aspeed_soc_uart_init(AspeedSoCState *s);
>   bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp);
>   void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr);
> +void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
> +                                   const char *name, hwaddr addr,
> +                                   uint64_t size);
>   
>   #endif /* ASPEED_SOC_H */

[PATCH v2 5/8] aspeed: Remove use of qemu_get_cpu

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 hw/arm/aspeed_ast2600.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 8693660271..0656c02889 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -322,7 +322,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
 
     for (i = 0; i < sc->num_cpus; i++) {
         SysBusDevice *sbd = SYS_BUS_DEVICE(&s->a7mpcore);
-        DeviceState  *d   = DEVICE(qemu_get_cpu(i));
+        DeviceState  *d   = DEVICE(&s->cpu[i]);
 
         irq = qdev_get_gpio_in(d, ARM_CPU_IRQ);
         sysbus_connect_irq(sbd, i, irq);
-- 
2.30.2

Re: [PATCH v2 5/8] aspeed: Remove use of qemu_get_cpu

[Cédric Le Goater]

On 6/24/22 02:36, Peter Delevoryas wrote:
> Signed-off-by: Peter Delevoryas <pdel@fb.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>


Thanks,

C.

> ---
>   hw/arm/aspeed_ast2600.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 8693660271..0656c02889 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -322,7 +322,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>   
>       for (i = 0; i < sc->num_cpus; i++) {
>           SysBusDevice *sbd = SYS_BUS_DEVICE(&s->a7mpcore);
> -        DeviceState  *d   = DEVICE(qemu_get_cpu(i));
> +        DeviceState  *d   = DEVICE(&s->cpu[i]);
>   
>           irq = qdev_get_gpio_in(d, ARM_CPU_IRQ);
>           sysbus_connect_irq(sbd, i, irq);

[PATCH v2 6/8] aspeed: Add fby35 skeleton

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 MAINTAINERS        |  1 +
 hw/arm/fby35.c     | 54 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/arm/meson.build |  3 ++-
 3 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/fby35.c

diff --git a/MAINTAINERS b/MAINTAINERS
index aaa649a50d..b43b428a30 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1067,6 +1067,7 @@ F: hw/net/ftgmac100.c
 F: include/hw/net/ftgmac100.h
 F: docs/system/arm/aspeed.rst
 F: tests/qtest/*aspeed*
+F: hw/arm/fby35.c
 
 NRF51
 M: Joel Stanley <joel@jms.id.au>
diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
new file mode 100644
index 0000000000..dc1ae8e62a
--- /dev/null
+++ b/hw/arm/fby35.c
@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/boards.h"
+
+#define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
+OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
+
+struct Fby35State {
+    MachineState parent_obj;
+};
+
+static void fby35_init(MachineState *machine)
+{
+}
+
+static void fby35_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->desc = "Meta Platforms fby35";
+    mc->init = fby35_init;
+}
+
+static const TypeInfo fby35_types[] = {
+    {
+        .name = MACHINE_TYPE_NAME("fby35"),
+        .parent = TYPE_MACHINE,
+        .class_init = fby35_class_init,
+        .instance_size = sizeof(Fby35State),
+    },
+};
+
+DEFINE_TYPES(fby35_types);
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 2d8381339c..92f9f6e000 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -51,7 +51,8 @@ arm_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
   'aspeed_soc.c',
   'aspeed.c',
   'aspeed_ast2600.c',
-  'aspeed_ast10x0.c'))
+  'aspeed_ast10x0.c',
+  'fby35.c'))
 arm_ss.add(when: 'CONFIG_MPS2', if_true: files('mps2.c'))
 arm_ss.add(when: 'CONFIG_MPS2', if_true: files('mps2-tz.c'))
 arm_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-soc.c'))
-- 
2.30.2

Re: [PATCH v2 6/8] aspeed: Add fby35 skeleton

[Cédric Le Goater]

On 6/24/22 02:36, Peter Delevoryas wrote:
> Signed-off-by: Peter Delevoryas <pdel@fb.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>


Thanks,

C.


> ---
>   MAINTAINERS        |  1 +
>   hw/arm/fby35.c     | 54 ++++++++++++++++++++++++++++++++++++++++++++++
>   hw/arm/meson.build |  3 ++-
>   3 files changed, 57 insertions(+), 1 deletion(-)
>   create mode 100644 hw/arm/fby35.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aaa649a50d..b43b428a30 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1067,6 +1067,7 @@ F: hw/net/ftgmac100.c
>   F: include/hw/net/ftgmac100.h
>   F: docs/system/arm/aspeed.rst
>   F: tests/qtest/*aspeed*
> +F: hw/arm/fby35.c
>   
>   NRF51
>   M: Joel Stanley <joel@jms.id.au>
> diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
> new file mode 100644
> index 0000000000..dc1ae8e62a
> --- /dev/null
> +++ b/hw/arm/fby35.c
> @@ -0,0 +1,54 @@
> +/*
> + * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/boards.h"
> +
> +#define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
> +OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
> +
> +struct Fby35State {
> +    MachineState parent_obj;
> +};
> +
> +static void fby35_init(MachineState *machine)
> +{
> +}
> +
> +static void fby35_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    mc->desc = "Meta Platforms fby35";
> +    mc->init = fby35_init;
> +}
> +
> +static const TypeInfo fby35_types[] = {
> +    {
> +        .name = MACHINE_TYPE_NAME("fby35"),
> +        .parent = TYPE_MACHINE,
> +        .class_init = fby35_class_init,
> +        .instance_size = sizeof(Fby35State),
> +    },
> +};
> +
> +DEFINE_TYPES(fby35_types);
> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> index 2d8381339c..92f9f6e000 100644
> --- a/hw/arm/meson.build
> +++ b/hw/arm/meson.build
> @@ -51,7 +51,8 @@ arm_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
>     'aspeed_soc.c',
>     'aspeed.c',
>     'aspeed_ast2600.c',
> -  'aspeed_ast10x0.c'))
> +  'aspeed_ast10x0.c',
> +  'fby35.c'))
>   arm_ss.add(when: 'CONFIG_MPS2', if_true: files('mps2.c'))
>   arm_ss.add(when: 'CONFIG_MPS2', if_true: files('mps2-tz.c'))
>   arm_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-soc.c'))

[PATCH v2 7/8] aspeed: Make aspeed_board_init_flashes public

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 hw/arm/aspeed.c             | 25 -------------------------
 hw/arm/aspeed_soc.c         | 26 ++++++++++++++++++++++++++
 include/hw/arm/aspeed_soc.h |  2 ++
 3 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index b43dc0fda8..a7352ca837 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -257,31 +257,6 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
     rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
 }
 
-static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
-                                      unsigned int count, int unit0)
-{
-    int i;
-
-    if (!flashtype) {
-        return;
-    }
-
-    for (i = 0; i < count; ++i) {
-        DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i);
-        qemu_irq cs_line;
-        DeviceState *dev;
-
-        dev = qdev_new(flashtype);
-        if (dinfo) {
-            qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
-        }
-        qdev_realize_and_unref(dev, BUS(s->spi), &error_fatal);
-
-        cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
-        sysbus_connect_irq(SYS_BUS_DEVICE(s), i + 1, cs_line);
-    }
-}
-
 static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo)
 {
         DeviceState *card;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 16394c8e0b..4183f5ffbf 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -21,6 +21,7 @@
 #include "hw/i2c/aspeed_i2c.h"
 #include "net/net.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/blockdev.h"
 
 #define ASPEED_SOC_IOMEM_SIZE       0x00200000
 
@@ -626,3 +627,28 @@ void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
     memory_region_add_subregion_overlap(s->memory, addr,
                                         sysbus_mmio_get_region(dev, 0), -1000);
 }
+
+void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
+                               unsigned int count, int unit0)
+{
+    int i;
+
+    if (!flashtype) {
+        return;
+    }
+
+    for (i = 0; i < count; ++i) {
+        DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i);
+        qemu_irq cs_line;
+        DeviceState *dev;
+
+        dev = qdev_new(flashtype);
+        if (dinfo) {
+            qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
+        }
+        qdev_realize_and_unref(dev, BUS(s->spi), &error_fatal);
+
+        cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
+        sysbus_connect_irq(SYS_BUS_DEVICE(s), i + 1, cs_line);
+    }
+}
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 6cfc063985..5cd31c1306 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -178,5 +178,7 @@ void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr);
 void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
                                    const char *name, hwaddr addr,
                                    uint64_t size);
+void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
+                               unsigned int count, int unit0);
 
 #endif /* ASPEED_SOC_H */
-- 
2.30.2

Re: [PATCH v2 7/8] aspeed: Make aspeed_board_init_flashes public

[Cédric Le Goater]

On 6/24/22 02:37, Peter Delevoryas wrote:
> Signed-off-by: Peter Delevoryas <pdel@fb.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/arm/aspeed.c             | 25 -------------------------
>   hw/arm/aspeed_soc.c         | 26 ++++++++++++++++++++++++++
>   include/hw/arm/aspeed_soc.h |  2 ++
>   3 files changed, 28 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index b43dc0fda8..a7352ca837 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -257,31 +257,6 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
>       rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
>   }
>   
> -static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
> -                                      unsigned int count, int unit0)
> -{
> -    int i;
> -
> -    if (!flashtype) {
> -        return;
> -    }
> -
> -    for (i = 0; i < count; ++i) {
> -        DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i);
> -        qemu_irq cs_line;
> -        DeviceState *dev;
> -
> -        dev = qdev_new(flashtype);
> -        if (dinfo) {
> -            qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> -        }
> -        qdev_realize_and_unref(dev, BUS(s->spi), &error_fatal);
> -
> -        cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
> -        sysbus_connect_irq(SYS_BUS_DEVICE(s), i + 1, cs_line);
> -    }
> -}
> -
>   static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo)
>   {
>           DeviceState *card;
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 16394c8e0b..4183f5ffbf 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -21,6 +21,7 @@
>   #include "hw/i2c/aspeed_i2c.h"
>   #include "net/net.h"
>   #include "sysemu/sysemu.h"
> +#include "sysemu/blockdev.h"
>   
>   #define ASPEED_SOC_IOMEM_SIZE       0x00200000
>   
> @@ -626,3 +627,28 @@ void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
>       memory_region_add_subregion_overlap(s->memory, addr,
>                                           sysbus_mmio_get_region(dev, 0), -1000);
>   }
> +
> +void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
> +                               unsigned int count, int unit0)
> +{
> +    int i;
> +
> +    if (!flashtype) {
> +        return;
> +    }
> +
> +    for (i = 0; i < count; ++i) {
> +        DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i);
> +        qemu_irq cs_line;
> +        DeviceState *dev;
> +
> +        dev = qdev_new(flashtype);
> +        if (dinfo) {
> +            qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> +        }
> +        qdev_realize_and_unref(dev, BUS(s->spi), &error_fatal);
> +
> +        cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(s), i + 1, cs_line);
> +    }
> +}
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 6cfc063985..5cd31c1306 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -178,5 +178,7 @@ void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr);
>   void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
>                                      const char *name, hwaddr addr,
>                                      uint64_t size);
> +void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
> +                               unsigned int count, int unit0);
>   
>   #endif /* ASPEED_SOC_H */

[PATCH v2 8/8] aspeed: Add AST2600 (BMC) to fby35

[Peter Delevoryas]

You can test booting the BMC with both '-device loader' and '-drive
file'. This is necessary because of how the fb-openbmc boot sequence
works (jump to 0x20000000 after U-Boot SPL).

    wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
    qemu-system-arm -machine fby35 -nographic \
        -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive file=fby35.mtd,format=raw,if=mtd

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 hw/arm/fby35.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index dc1ae8e62a..1e24cbf3f8 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -21,17 +21,53 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qapi/error.h"
 #include "hw/boards.h"
+#include "hw/arm/aspeed_soc.h"
 
 #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
 OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
 
 struct Fby35State {
     MachineState parent_obj;
+
+    MemoryRegion bmc_memory;
+    MemoryRegion bmc_dram;
+    MemoryRegion bmc_boot_rom;
+
+    AspeedSoCState bmc;
 };
 
+static void fby35_bmc_init(Fby35State *s)
+{
+    uint32_t boot_rom_size;
+
+    memory_region_init(&s->bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX);
+    memory_region_init_ram(&s->bmc_dram, OBJECT(s), "bmc-dram", 2 * GiB, &error_abort);
+
+    object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3");
+    object_property_set_int(OBJECT(&s->bmc), "ram-size", s->bmc_dram.size, &error_abort);
+    object_property_set_link(OBJECT(&s->bmc), "memory", OBJECT(&s->bmc_memory), &error_abort);
+    object_property_set_link(OBJECT(&s->bmc), "dram", OBJECT(&s->bmc_dram), &error_abort);
+    object_property_set_int(OBJECT(&s->bmc), "hw-strap1", 0x000000C0, &error_abort);
+    object_property_set_int(OBJECT(&s->bmc), "hw-strap2", 0x00000003, &error_abort);
+    object_property_set_int(OBJECT(&s->bmc), "uart-default", ASPEED_DEV_UART5, &error_abort);
+    qdev_realize(DEVICE(&s->bmc), NULL, &error_abort);
+
+    boot_rom_size = ASPEED_SMC_GET_CLASS(&s->bmc.fmc)->segments[0].size;
+
+    memory_region_init_rom(&s->bmc_boot_rom, OBJECT(s), "bmc-boot-rom", boot_rom_size, &error_abort);
+    memory_region_add_subregion(&s->bmc_memory, 0, &s->bmc_boot_rom);
+
+    aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0);
+}
+
 static void fby35_init(MachineState *machine)
 {
+    Fby35State *s = FBY35(machine);
+
+    fby35_bmc_init(s);
 }
 
 static void fby35_class_init(ObjectClass *oc, void *data)
@@ -40,6 +76,9 @@ static void fby35_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Meta Platforms fby35";
     mc->init = fby35_init;
+    mc->no_floppy = 1;
+    mc->no_cdrom = 1;
+    mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
 }
 
 static const TypeInfo fby35_types[] = {
-- 
2.30.2

Re: [PATCH v2 8/8] aspeed: Add AST2600 (BMC) to fby35

[Cédric Le Goater]

On 6/24/22 02:37, Peter Delevoryas wrote:
> You can test booting the BMC with both '-device loader' and '-drive
> file'. This is necessary because of how the fb-openbmc boot sequence
> works (jump to 0x20000000 after U-Boot SPL).
>
> 
>      wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
>      qemu-system-arm -machine fby35 -nographic \
>          -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive file=fby35.mtd,format=raw,if=mtd
> 
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> ---
>   hw/arm/fby35.c | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
> index dc1ae8e62a..1e24cbf3f8 100644
> --- a/hw/arm/fby35.c
> +++ b/hw/arm/fby35.c
> @@ -21,17 +21,53 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "qapi/error.h"
>   #include "hw/boards.h"
> +#include "hw/arm/aspeed_soc.h"
>   
>   #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
>   OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
>   
>   struct Fby35State {
>       MachineState parent_obj;
> +
> +    MemoryRegion bmc_memory;
> +    MemoryRegion bmc_dram;
> +    MemoryRegion bmc_boot_rom;
> +
> +    AspeedSoCState bmc;
>   };
>   
> +static void fby35_bmc_init(Fby35State *s)
> +{
> +    uint32_t boot_rom_size;
> +
> +    memory_region_init(&s->bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX);
> +    memory_region_init_ram(&s->bmc_dram, OBJECT(s), "bmc-dram", 2 * GiB, &error_abort);
> +
> +    object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3");
> +    object_property_set_int(OBJECT(&s->bmc), "ram-size", s->bmc_dram.size, &error_abort);

This fails to compile on some platforms :

./hw/arm/fby35.c: In function ‘fby35_bmc_init’:
../hw/arm/fby35.c:50:69: error: incompatible type for argument 3 of ‘object_property_set_int’
    50 |     object_property_set_int(OBJECT(&s->bmc), "ram-size", s->bmc_dram.size, &error_abort);
       |                                                          ~~~~~~~~~~~^~~~~
       |                                                                     |
       |                                                                     Int128
In file included from /builds/legoater/qemu/include/exec/memory.h:28,
                  from /builds/legoater/qemu/include/hw/boards.h:6,
                  from ../hw/arm/fby35.c:26:
/builds/legoater/qemu/include/qom/object.h:1342:38: note: expected ‘int64_t’ {aka ‘long long int’} but argument is of type ‘Int128’
  1342 |                              int64_t value, Error **errp);
       |                              ~~~~~~~~^~~~~


You don't need to resend the patches 1-7. I have pulled them in my branch
for the next PR.

> +    object_property_set_link(OBJECT(&s->bmc), "memory", OBJECT(&s->bmc_memory), &error_abort);
> +    object_property_set_link(OBJECT(&s->bmc), "dram", OBJECT(&s->bmc_dram), &error_abort);
> +    object_property_set_int(OBJECT(&s->bmc), "hw-strap1", 0x000000C0, &error_abort);
> +    object_property_set_int(OBJECT(&s->bmc), "hw-strap2", 0x00000003, &error_abort);

We could share common definitions with the BMC machine in .h file.

> +    object_property_set_int(OBJECT(&s->bmc), "uart-default", ASPEED_DEV_UART5, &error_abort);
> +    qdev_realize(DEVICE(&s->bmc), NULL, &error_abort);
> +
> +    boot_rom_size = ASPEED_SMC_GET_CLASS(&s->bmc.fmc)->segments[0].size;
> +
> +    memory_region_init_rom(&s->bmc_boot_rom, OBJECT(s), "bmc-boot-rom", boot_rom_size, &error_abort);
> +    memory_region_add_subregion(&s->bmc_memory, 0, &s->bmc_boot_rom);
> +
> +    aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0);

I am not totally convinced with the ROM because it complexifies how the
machine is started but I haven't tried the other way using the loader
either. It is my TODO list.

Thanks,

C.
  

> +}
> +
>   static void fby35_init(MachineState *machine)
>   {
> +    Fby35State *s = FBY35(machine);
> +
> +    fby35_bmc_init(s);
>   }
>   
>   static void fby35_class_init(ObjectClass *oc, void *data)
> @@ -40,6 +76,9 @@ static void fby35_class_init(ObjectClass *oc, void *data)
>   
>       mc->desc = "Meta Platforms fby35";
>       mc->init = fby35_init;
> +    mc->no_floppy = 1;
> +    mc->no_cdrom = 1;
> +    mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
>   }
>   
>   static const TypeInfo fby35_types[] = {

Re: [PATCH v2 8/8] aspeed: Add AST2600 (BMC) to fby35

[Peter Delevoryas]

> On Jun 27, 2022, at 10:01 PM, Cédric Le Goater <clg@kaod.org> wrote:
> 
> On 6/24/22 02:37, Peter Delevoryas wrote:
>> You can test booting the BMC with both '-device loader' and '-drive
>> file'. This is necessary because of how the fb-openbmc boot sequence
>> works (jump to 0x20000000 after U-Boot SPL).
>> 
>>     wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
>>     qemu-system-arm -machine fby35 -nographic \
>>         -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive file=fby35.mtd,format=raw,if=mtd
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>> ---
>>  hw/arm/fby35.c | 39 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>> diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
>> index dc1ae8e62a..1e24cbf3f8 100644
>> --- a/hw/arm/fby35.c
>> +++ b/hw/arm/fby35.c
>> @@ -21,17 +21,53 @@
>>   */
>>    #include "qemu/osdep.h"
>> +#include "qemu/units.h"
>> +#include "qapi/error.h"
>>  #include "hw/boards.h"
>> +#include "hw/arm/aspeed_soc.h"
>>    #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
>>  OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
>>    struct Fby35State {
>>      MachineState parent_obj;
>> +
>> +    MemoryRegion bmc_memory;
>> +    MemoryRegion bmc_dram;
>> +    MemoryRegion bmc_boot_rom;
>> +
>> +    AspeedSoCState bmc;
>>  };
>>  +static void fby35_bmc_init(Fby35State *s)
>> +{
>> +    uint32_t boot_rom_size;
>> +
>> +    memory_region_init(&s->bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX);
>> +    memory_region_init_ram(&s->bmc_dram, OBJECT(s), "bmc-dram", 2 * GiB, &error_abort);
>> +
>> +    object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3");
>> +    object_property_set_int(OBJECT(&s->bmc), "ram-size", s->bmc_dram.size, &error_abort);
> 
> This fails to compile on some platforms :
> 
> ./hw/arm/fby35.c: In function ‘fby35_bmc_init’:
> ../hw/arm/fby35.c:50:69: error: incompatible type for argument 3 of ‘object_property_set_int’
>   50 |     object_property_set_int(OBJECT(&s->bmc), "ram-size", s->bmc_dram.size, &error_abort);
>      |                                                          ~~~~~~~~~~~^~~~~
>      |                                                                     |
>      |                                                                     Int128
> In file included from /builds/legoater/qemu/include/exec/memory.h:28,
>                 from /builds/legoater/qemu/include/hw/boards.h:6,
>                 from ../hw/arm/fby35.c:26:
> /builds/legoater/qemu/include/qom/object.h:1342:38: note: expected ‘int64_t’ {aka ‘long long int’} but argument is of type ‘Int128’
> 1342 |                              int64_t value, Error **errp);
>      |                              ~~~~~~~~^~~~~
> 
> 

Oh yikes, I’ll fix this. Hopefully without casting from Int128 to int64_t or defining a new constant.

> You don't need to resend the patches 1-7. I have pulled them in my branch
> for the next PR.

Got it, I’ll just resend this one as v3 then.

> 
>> +    object_property_set_link(OBJECT(&s->bmc), "memory", OBJECT(&s->bmc_memory), &error_abort);
>> +    object_property_set_link(OBJECT(&s->bmc), "dram", OBJECT(&s->bmc_dram), &error_abort);
>> +    object_property_set_int(OBJECT(&s->bmc), "hw-strap1", 0x000000C0, &error_abort);
>> +    object_property_set_int(OBJECT(&s->bmc), "hw-strap2", 0x00000003, &error_abort);
> 
> We could share common definitions with the BMC machine in .h file.

Good point, I’ll put then in aspeed.h then.

> 
>> +    object_property_set_int(OBJECT(&s->bmc), "uart-default", ASPEED_DEV_UART5, &error_abort);
>> +    qdev_realize(DEVICE(&s->bmc), NULL, &error_abort);
>> +
>> +    boot_rom_size = ASPEED_SMC_GET_CLASS(&s->bmc.fmc)->segments[0].size;
>> +
>> +    memory_region_init_rom(&s->bmc_boot_rom, OBJECT(s), "bmc-boot-rom", boot_rom_size, &error_abort);
>> +    memory_region_add_subregion(&s->bmc_memory, 0, &s->bmc_boot_rom);
>> +
>> +    aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0);
> 
> I am not totally convinced with the ROM because it complexifies how the
> machine is started but I haven't tried the other way using the loader
> either. It is my TODO list.

Yep, totally agree: if I can fix this soon I’ll send out a fix or something, otherwise
feel free to submit a change of your own.

Thanks for the review!

> 
> Thanks,
> 
> C.
> 
>> +}
>> +
>>  static void fby35_init(MachineState *machine)
>>  {
>> +    Fby35State *s = FBY35(machine);
>> +
>> +    fby35_bmc_init(s);
>>  }
>>    static void fby35_class_init(ObjectClass *oc, void *data)
>> @@ -40,6 +76,9 @@ static void fby35_class_init(ObjectClass *oc, void *data)
>>        mc->desc = "Meta Platforms fby35";
>>      mc->init = fby35_init;
>> +    mc->no_floppy = 1;
>> +    mc->no_cdrom = 1;
>> +    mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
>>  }
>>    static const TypeInfo fby35_types[] = {
>