[PATCH v2 0/7] Support VBOOTROM to ast2700fc machine

[PATCH v2 0/7] Support VBOOTROM to ast2700fc machine

[Jamin Lin via]

v1
 1. Added support for Vboot ROM.
 2. Moved coprocessor initialization from machine level to SoC level
 3. Unified SCU controllers between PSP and coprocessors
 4. Shared the same SRAM between PSP and coprocessors
 5. Support PSP DRAM remaps coprocessor SDRAM
 6. Added support for controlling coprocessor reset via SCU registers.

v2
Split the original patch set into smaller sub-patches for review.
 This patch focuses on:
  1. Adding support for Vboot ROM
  2. Moving common APIs to SoC-level code for reuse in different
     platforms and reducing duplication.

Dependencies

Based on https://github.com/legoater/qemu at the aspeed-next branch.

Jamin Lin (7):
  hw/arm/aspeed: Move aspeed_board_init_flashes() to common SoC code
  hw/arm/aspeed: Move write_boot_rom to common SoC code
  hw/arm/aspeed: Move aspeed_install_boot_rom to common SoC code
  hw/arm/aspeed: Move aspeed_load_vbootrom to common SoC code
  hw/arm/aspeed_ast27x0-fc: Replace error_abort with local errp
  hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM
  hw/arm/aspeed_ast27x0-fc: Add VBOOTROM support

 include/hw/arm/aspeed_soc.h |   8 +++
 hw/arm/aspeed.c             | 105 ++----------------------------------
 hw/arm/aspeed_ast27x0-fc.c  |  46 ++++++++++++----
 hw/arm/aspeed_soc_common.c  |  96 +++++++++++++++++++++++++++++++++
 4 files changed, 143 insertions(+), 112 deletions(-)

-- 
2.43.0

[PATCH v2 0/7] Support VBOOTROM to ast2700fc machine

[Jamin Lin via]

v1
 1. Added support for Vboot ROM.
 2. Moved coprocessor initialization from machine level to SoC level
 3. Unified SCU controllers between PSP and coprocessors
 4. Shared the same SRAM between PSP and coprocessors
 5. Support PSP DRAM remaps coprocessor SDRAM
 6. Added support for controlling coprocessor reset via SCU registers.

v2
Split the original patch set into smaller sub-patches for review.
 This patch focuses on:
  1. Adding support for Vboot ROM
  2. Moving common APIs to SoC-level code for reuse in different
     platforms and reducing duplication.

Dependencies

Based on https://github.com/legoater/qemu at the aspeed-next branch.

Jamin Lin (7):
  hw/arm/aspeed: Move aspeed_board_init_flashes() to common SoC code
  hw/arm/aspeed: Move write_boot_rom to common SoC code
  hw/arm/aspeed: Move aspeed_install_boot_rom to common SoC code
  hw/arm/aspeed: Move aspeed_load_vbootrom to common SoC code
  hw/arm/aspeed_ast27x0-fc: Replace error_abort with local errp
  hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM
  hw/arm/aspeed_ast27x0-fc: Add VBOOTROM support

 include/hw/arm/aspeed_soc.h |   8 +++
 hw/arm/aspeed.c             | 105 ++----------------------------------
 hw/arm/aspeed_ast27x0-fc.c  |  46 ++++++++++++----
 hw/arm/aspeed_soc_common.c  |  96 +++++++++++++++++++++++++++++++++
 4 files changed, 143 insertions(+), 112 deletions(-)

-- 
2.43.0

[PATCH v2 1/7] hw/arm/aspeed: Move aspeed_board_init_flashes() to common SoC code

[Jamin Lin via]

Relocate aspeed_board_init_flashes() from hw/arm/aspeed.c into
hw/arm/aspeed_soc_common.c so the helper can be reused by all
ASPEED machines. The API was already declared in
include/hw/arm/aspeed_soc.h; this change moves its
implementation out of the machine file to keep aspeed.c cleaner.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed.c            | 22 ----------------------
 hw/arm/aspeed_soc_common.c | 23 +++++++++++++++++++++++
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index d21b21965a..55f0afe0a4 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -337,28 +337,6 @@ static void aspeed_load_vbootrom(AspeedMachineState *bmc, const char *bios_name,
     }
 }
 
-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);
-        DeviceState *dev;
-
-        dev = qdev_new(flashtype);
-        if (dinfo) {
-            qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
-        }
-        qdev_prop_set_uint8(dev, "cs", i);
-        qdev_realize_and_unref(dev, BUS(s->spi), &error_fatal);
-    }
-}
-
 static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
                                bool boot_emmc)
 {
diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
index 1c4ac93a0f..31b1e683c3 100644
--- a/hw/arm/aspeed_soc_common.c
+++ b/hw/arm/aspeed_soc_common.c
@@ -16,6 +16,7 @@
 #include "hw/misc/unimp.h"
 #include "hw/arm/aspeed_soc.h"
 #include "hw/char/serial-mm.h"
+#include "system/blockdev.h"
 
 
 const char *aspeed_soc_cpu_type(AspeedSoCClass *sc)
@@ -124,6 +125,28 @@ void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
                                         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);
+        DeviceState *dev;
+
+        dev = qdev_new(flashtype);
+        if (dinfo) {
+            qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
+        }
+        qdev_prop_set_uint8(dev, "cs", i);
+        qdev_realize_and_unref(dev, BUS(s->spi), &error_fatal);
+    }
+}
+
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 {
     AspeedSoCState *s = ASPEED_SOC(dev);
-- 
2.43.0

[PATCH v2 1/7] hw/arm/aspeed: Move aspeed_board_init_flashes() to common SoC code

[Jamin Lin via]

Relocate aspeed_board_init_flashes() from hw/arm/aspeed.c into
hw/arm/aspeed_soc_common.c so the helper can be reused by all
ASPEED machines. The API was already declared in
include/hw/arm/aspeed_soc.h; this change moves its
implementation out of the machine file to keep aspeed.c cleaner.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed.c            | 22 ----------------------
 hw/arm/aspeed_soc_common.c | 23 +++++++++++++++++++++++
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index d21b21965a..55f0afe0a4 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -337,28 +337,6 @@ static void aspeed_load_vbootrom(AspeedMachineState *bmc, const char *bios_name,
     }
 }
 
-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);
-        DeviceState *dev;
-
-        dev = qdev_new(flashtype);
-        if (dinfo) {
-            qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
-        }
-        qdev_prop_set_uint8(dev, "cs", i);
-        qdev_realize_and_unref(dev, BUS(s->spi), &error_fatal);
-    }
-}
-
 static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
                                bool boot_emmc)
 {
diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
index 1c4ac93a0f..31b1e683c3 100644
--- a/hw/arm/aspeed_soc_common.c
+++ b/hw/arm/aspeed_soc_common.c
@@ -16,6 +16,7 @@
 #include "hw/misc/unimp.h"
 #include "hw/arm/aspeed_soc.h"
 #include "hw/char/serial-mm.h"
+#include "system/blockdev.h"
 
 
 const char *aspeed_soc_cpu_type(AspeedSoCClass *sc)
@@ -124,6 +125,28 @@ void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
                                         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);
+        DeviceState *dev;
+
+        dev = qdev_new(flashtype);
+        if (dinfo) {
+            qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
+        }
+        qdev_prop_set_uint8(dev, "cs", i);
+        qdev_realize_and_unref(dev, BUS(s->spi), &error_fatal);
+    }
+}
+
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 {
     AspeedSoCState *s = ASPEED_SOC(dev);
-- 
2.43.0

Re: [PATCH v2 1/7] hw/arm/aspeed: Move aspeed_board_init_flashes() to common SoC code

[Cédric Le Goater]

On 9/24/25 07:55, Jamin Lin wrote:
> Relocate aspeed_board_init_flashes() from hw/arm/aspeed.c into
> hw/arm/aspeed_soc_common.c so the helper can be reused by all
> ASPEED machines. The API was already declared in
> include/hw/arm/aspeed_soc.h; this change moves its
> implementation out of the machine file to keep aspeed.c cleaner.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/arm/aspeed.c            | 22 ----------------------
>   hw/arm/aspeed_soc_common.c | 23 +++++++++++++++++++++++
>   2 files changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index d21b21965a..55f0afe0a4 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -337,28 +337,6 @@ static void aspeed_load_vbootrom(AspeedMachineState *bmc, const char *bios_name,
>       }
>   }
>   
> -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);
> -        DeviceState *dev;
> -
> -        dev = qdev_new(flashtype);
> -        if (dinfo) {
> -            qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> -        }
> -        qdev_prop_set_uint8(dev, "cs", i);
> -        qdev_realize_and_unref(dev, BUS(s->spi), &error_fatal);
> -    }
> -}
> -
>   static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
>                                  bool boot_emmc)
>   {
> diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
> index 1c4ac93a0f..31b1e683c3 100644
> --- a/hw/arm/aspeed_soc_common.c
> +++ b/hw/arm/aspeed_soc_common.c
> @@ -16,6 +16,7 @@
>   #include "hw/misc/unimp.h"
>   #include "hw/arm/aspeed_soc.h"
>   #include "hw/char/serial-mm.h"
> +#include "system/blockdev.h"
>   
>   
>   const char *aspeed_soc_cpu_type(AspeedSoCClass *sc)
> @@ -124,6 +125,28 @@ void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
>                                           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);
> +        DeviceState *dev;
> +
> +        dev = qdev_new(flashtype);
> +        if (dinfo) {
> +            qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> +        }
> +        qdev_prop_set_uint8(dev, "cs", i);
> +        qdev_realize_and_unref(dev, BUS(s->spi), &error_fatal);
> +    }
> +}
> +
>   static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>   {
>       AspeedSoCState *s = ASPEED_SOC(dev);

[PATCH v2 2/7] hw/arm/aspeed: Move write_boot_rom to common SoC code

[Jamin Lin via]

Move the write_boot_rom helper from hw/arm/aspeed.c into
hw/arm/aspeed_soc_common.c so it can be reused by all ASPEED
machines. Export the API as aspeed_write_boot_rom() in
include/hw/arm/aspeed_soc.h and update the existing call site
to use the new helper.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h |  2 ++
 hw/arm/aspeed.c             | 33 ++-------------------------------
 hw/arm/aspeed_soc_common.c  | 31 +++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index aaf518d179..5567bdcb69 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -312,6 +312,8 @@ void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
                                    uint64_t size);
 void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
                                unsigned int count, int unit0);
+void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
+                           Error **errp);
 
 static inline int aspeed_uart_index(int uart_dev)
 {
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 55f0afe0a4..4d0d935836 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -263,35 +263,6 @@ static void aspeed_reset_secondary(ARMCPU *cpu,
     cpu_set_pc(cs, info->smp_loader_start);
 }
 
-static void write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
-                           Error **errp)
-{
-    g_autofree void *storage = NULL;
-    int64_t size;
-
-    /*
-     * The block backend size should have already been 'validated' by
-     * the creation of the m25p80 object.
-     */
-    size = blk_getlength(blk);
-    if (size <= 0) {
-        error_setg(errp, "failed to get flash size");
-        return;
-    }
-
-    if (rom_size > size) {
-        rom_size = size;
-    }
-
-    storage = g_malloc0(rom_size);
-    if (blk_pread(blk, 0, rom_size, storage, 0) < 0) {
-        error_setg(errp, "failed to read the initial flash content");
-        return;
-    }
-
-    rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
-}
-
 /*
  * Create a ROM and copy the flash contents at the expected address
  * (0x0). Boots faster than execute-in-place.
@@ -306,8 +277,8 @@ static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
                            &error_abort);
     memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
                                         &bmc->boot_rom, 1);
-    write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
-                   rom_size, &error_abort);
+    aspeed_write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], rom_size,
+                          &error_abort);
 }
 
 #define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
index 31b1e683c3..d0a400725f 100644
--- a/hw/arm/aspeed_soc_common.c
+++ b/hw/arm/aspeed_soc_common.c
@@ -17,6 +17,8 @@
 #include "hw/arm/aspeed_soc.h"
 #include "hw/char/serial-mm.h"
 #include "system/blockdev.h"
+#include "system/block-backend.h"
+#include "hw/loader.h"
 
 
 const char *aspeed_soc_cpu_type(AspeedSoCClass *sc)
@@ -147,6 +149,35 @@ void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
     }
 }
 
+void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
+                           Error **errp)
+{
+    g_autofree void *storage = NULL;
+    int64_t size;
+
+    /*
+     * The block backend size should have already been 'validated' by
+     * the creation of the m25p80 object.
+     */
+    size = blk_getlength(blk);
+    if (size <= 0) {
+        error_setg(errp, "failed to get flash size");
+        return;
+    }
+
+    if (rom_size > size) {
+        rom_size = size;
+    }
+
+    storage = g_malloc0(rom_size);
+    if (blk_pread(blk, 0, rom_size, storage, 0) < 0) {
+        error_setg(errp, "failed to read the initial flash content");
+        return;
+    }
+
+    rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
+}
+
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 {
     AspeedSoCState *s = ASPEED_SOC(dev);
-- 
2.43.0

[PATCH v2 2/7] hw/arm/aspeed: Move write_boot_rom to common SoC code

[Jamin Lin via]

Move the write_boot_rom helper from hw/arm/aspeed.c into
hw/arm/aspeed_soc_common.c so it can be reused by all ASPEED
machines. Export the API as aspeed_write_boot_rom() in
include/hw/arm/aspeed_soc.h and update the existing call site
to use the new helper.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h |  2 ++
 hw/arm/aspeed.c             | 33 ++-------------------------------
 hw/arm/aspeed_soc_common.c  | 31 +++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index aaf518d179..5567bdcb69 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -312,6 +312,8 @@ void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
                                    uint64_t size);
 void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
                                unsigned int count, int unit0);
+void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
+                           Error **errp);
 
 static inline int aspeed_uart_index(int uart_dev)
 {
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 55f0afe0a4..4d0d935836 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -263,35 +263,6 @@ static void aspeed_reset_secondary(ARMCPU *cpu,
     cpu_set_pc(cs, info->smp_loader_start);
 }
 
-static void write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
-                           Error **errp)
-{
-    g_autofree void *storage = NULL;
-    int64_t size;
-
-    /*
-     * The block backend size should have already been 'validated' by
-     * the creation of the m25p80 object.
-     */
-    size = blk_getlength(blk);
-    if (size <= 0) {
-        error_setg(errp, "failed to get flash size");
-        return;
-    }
-
-    if (rom_size > size) {
-        rom_size = size;
-    }
-
-    storage = g_malloc0(rom_size);
-    if (blk_pread(blk, 0, rom_size, storage, 0) < 0) {
-        error_setg(errp, "failed to read the initial flash content");
-        return;
-    }
-
-    rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
-}
-
 /*
  * Create a ROM and copy the flash contents at the expected address
  * (0x0). Boots faster than execute-in-place.
@@ -306,8 +277,8 @@ static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
                            &error_abort);
     memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
                                         &bmc->boot_rom, 1);
-    write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
-                   rom_size, &error_abort);
+    aspeed_write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], rom_size,
+                          &error_abort);
 }
 
 #define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
index 31b1e683c3..d0a400725f 100644
--- a/hw/arm/aspeed_soc_common.c
+++ b/hw/arm/aspeed_soc_common.c
@@ -17,6 +17,8 @@
 #include "hw/arm/aspeed_soc.h"
 #include "hw/char/serial-mm.h"
 #include "system/blockdev.h"
+#include "system/block-backend.h"
+#include "hw/loader.h"
 
 
 const char *aspeed_soc_cpu_type(AspeedSoCClass *sc)
@@ -147,6 +149,35 @@ void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
     }
 }
 
+void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
+                           Error **errp)
+{
+    g_autofree void *storage = NULL;
+    int64_t size;
+
+    /*
+     * The block backend size should have already been 'validated' by
+     * the creation of the m25p80 object.
+     */
+    size = blk_getlength(blk);
+    if (size <= 0) {
+        error_setg(errp, "failed to get flash size");
+        return;
+    }
+
+    if (rom_size > size) {
+        rom_size = size;
+    }
+
+    storage = g_malloc0(rom_size);
+    if (blk_pread(blk, 0, rom_size, storage, 0) < 0) {
+        error_setg(errp, "failed to read the initial flash content");
+        return;
+    }
+
+    rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
+}
+
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 {
     AspeedSoCState *s = ASPEED_SOC(dev);
-- 
2.43.0

Re: [PATCH v2 2/7] hw/arm/aspeed: Move write_boot_rom to common SoC code

[Cédric Le Goater]

On 9/24/25 07:55, Jamin Lin wrote:
> Move the write_boot_rom helper from hw/arm/aspeed.c into
> hw/arm/aspeed_soc_common.c so it can be reused by all ASPEED
> machines. Export the API as aspeed_write_boot_rom() in
> include/hw/arm/aspeed_soc.h and update the existing call site
> to use the new helper.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/hw/arm/aspeed_soc.h |  2 ++
>   hw/arm/aspeed.c             | 33 ++-------------------------------
>   hw/arm/aspeed_soc_common.c  | 31 +++++++++++++++++++++++++++++++
>   3 files changed, 35 insertions(+), 31 deletions(-)
> 
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index aaf518d179..5567bdcb69 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -312,6 +312,8 @@ void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
>                                      uint64_t size);
>   void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
>                                  unsigned int count, int unit0);
> +void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
> +                           Error **errp);
>   
>   static inline int aspeed_uart_index(int uart_dev)
>   {
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 55f0afe0a4..4d0d935836 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -263,35 +263,6 @@ static void aspeed_reset_secondary(ARMCPU *cpu,
>       cpu_set_pc(cs, info->smp_loader_start);
>   }
>   
> -static void write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
> -                           Error **errp)
> -{
> -    g_autofree void *storage = NULL;
> -    int64_t size;
> -
> -    /*
> -     * The block backend size should have already been 'validated' by
> -     * the creation of the m25p80 object.
> -     */
> -    size = blk_getlength(blk);
> -    if (size <= 0) {
> -        error_setg(errp, "failed to get flash size");
> -        return;
> -    }
> -
> -    if (rom_size > size) {
> -        rom_size = size;
> -    }
> -
> -    storage = g_malloc0(rom_size);
> -    if (blk_pread(blk, 0, rom_size, storage, 0) < 0) {
> -        error_setg(errp, "failed to read the initial flash content");
> -        return;
> -    }
> -
> -    rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
> -}
> -
>   /*
>    * Create a ROM and copy the flash contents at the expected address
>    * (0x0). Boots faster than execute-in-place.
> @@ -306,8 +277,8 @@ static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
>                              &error_abort);
>       memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
>                                           &bmc->boot_rom, 1);
> -    write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
> -                   rom_size, &error_abort);
> +    aspeed_write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], rom_size,
> +                          &error_abort);
>   }
>   
>   #define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
> diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
> index 31b1e683c3..d0a400725f 100644
> --- a/hw/arm/aspeed_soc_common.c
> +++ b/hw/arm/aspeed_soc_common.c
> @@ -17,6 +17,8 @@
>   #include "hw/arm/aspeed_soc.h"
>   #include "hw/char/serial-mm.h"
>   #include "system/blockdev.h"
> +#include "system/block-backend.h"
> +#include "hw/loader.h"
>   
>   
>   const char *aspeed_soc_cpu_type(AspeedSoCClass *sc)
> @@ -147,6 +149,35 @@ void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
>       }
>   }
>   
> +void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
> +                           Error **errp)
> +{
> +    g_autofree void *storage = NULL;
> +    int64_t size;
> +
> +    /*
> +     * The block backend size should have already been 'validated' by
> +     * the creation of the m25p80 object.
> +     */
> +    size = blk_getlength(blk);
> +    if (size <= 0) {
> +        error_setg(errp, "failed to get flash size");
> +        return;
> +    }
> +
> +    if (rom_size > size) {
> +        rom_size = size;
> +    }
> +
> +    storage = g_malloc0(rom_size);
> +    if (blk_pread(blk, 0, rom_size, storage, 0) < 0) {
> +        error_setg(errp, "failed to read the initial flash content");
> +        return;
> +    }
> +
> +    rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
> +}
> +
>   static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>   {
>       AspeedSoCState *s = ASPEED_SOC(dev);

[PATCH v2 3/7] hw/arm/aspeed: Move aspeed_install_boot_rom to common SoC code

[Jamin Lin via]

Move the boot ROM install helper into common SoC code so it can be reused
by all ASPEED boards, and decouple the API from AspeedMachineState.

Specifically:
 - Move aspeed_install_boot_rom() to hw/arm/aspeed_soc_common.c and
   declare it in include/hw/arm/aspeed_soc.h.
 - Change the helper’s signature to take AspeedSoCState * and a
   MemoryRegion * provided by the caller, instead of AspeedMachineState *.
 - Update aspeed_machine_init() call sites accordingly.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h |  2 ++
 hw/arm/aspeed.c             | 23 +++--------------------
 hw/arm/aspeed_soc_common.c  | 17 +++++++++++++++++
 3 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 5567bdcb69..aea210a8e2 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -314,6 +314,8 @@ void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
                                unsigned int count, int unit0);
 void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
                            Error **errp);
+void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
+                             MemoryRegion *boot_rom, uint64_t rom_size);
 
 static inline int aspeed_uart_index(int uart_dev)
 {
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 4d0d935836..429f4c6d77 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -263,24 +263,6 @@ static void aspeed_reset_secondary(ARMCPU *cpu,
     cpu_set_pc(cs, info->smp_loader_start);
 }
 
-/*
- * Create a ROM and copy the flash contents at the expected address
- * (0x0). Boots faster than execute-in-place.
- */
-static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
-                                    uint64_t rom_size)
-{
-    AspeedSoCState *soc = bmc->soc;
-    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
-
-    memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size,
-                           &error_abort);
-    memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
-                                        &bmc->boot_rom, 1);
-    aspeed_write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], rom_size,
-                          &error_abort);
-}
-
 #define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
 
 /*
@@ -460,9 +442,10 @@ static void aspeed_machine_init(MachineState *machine)
 
         if (fmc0 && !boot_emmc) {
             uint64_t rom_size = memory_region_size(&bmc->soc->spi_boot);
-            aspeed_install_boot_rom(bmc, fmc0, rom_size);
+            aspeed_install_boot_rom(bmc->soc, fmc0, &bmc->boot_rom, rom_size);
         } else if (emmc0) {
-            aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0), 64 * KiB);
+            aspeed_install_boot_rom(bmc->soc, blk_by_legacy_dinfo(emmc0),
+                                    &bmc->boot_rom, 64 * KiB);
         }
     }
 
diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
index d0a400725f..7f104f8de5 100644
--- a/hw/arm/aspeed_soc_common.c
+++ b/hw/arm/aspeed_soc_common.c
@@ -178,6 +178,23 @@ void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
     rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
 }
 
+/*
+ * Create a ROM and copy the flash contents at the expected address
+ * (0x0). Boots faster than execute-in-place.
+ */
+void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
+                             MemoryRegion *boot_rom, uint64_t rom_size)
+{
+    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
+
+    memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom", rom_size,
+                           &error_abort);
+    memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
+                                        boot_rom, 1);
+    aspeed_write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], rom_size,
+                          &error_abort);
+}
+
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 {
     AspeedSoCState *s = ASPEED_SOC(dev);
-- 
2.43.0

[PATCH v2 3/7] hw/arm/aspeed: Move aspeed_install_boot_rom to common SoC code

[Jamin Lin via]

Move the boot ROM install helper into common SoC code so it can be reused
by all ASPEED boards, and decouple the API from AspeedMachineState.

Specifically:
 - Move aspeed_install_boot_rom() to hw/arm/aspeed_soc_common.c and
   declare it in include/hw/arm/aspeed_soc.h.
 - Change the helper’s signature to take AspeedSoCState * and a
   MemoryRegion * provided by the caller, instead of AspeedMachineState *.
 - Update aspeed_machine_init() call sites accordingly.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h |  2 ++
 hw/arm/aspeed.c             | 23 +++--------------------
 hw/arm/aspeed_soc_common.c  | 17 +++++++++++++++++
 3 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 5567bdcb69..aea210a8e2 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -314,6 +314,8 @@ void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
                                unsigned int count, int unit0);
 void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
                            Error **errp);
+void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
+                             MemoryRegion *boot_rom, uint64_t rom_size);
 
 static inline int aspeed_uart_index(int uart_dev)
 {
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 4d0d935836..429f4c6d77 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -263,24 +263,6 @@ static void aspeed_reset_secondary(ARMCPU *cpu,
     cpu_set_pc(cs, info->smp_loader_start);
 }
 
-/*
- * Create a ROM and copy the flash contents at the expected address
- * (0x0). Boots faster than execute-in-place.
- */
-static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
-                                    uint64_t rom_size)
-{
-    AspeedSoCState *soc = bmc->soc;
-    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
-
-    memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size,
-                           &error_abort);
-    memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
-                                        &bmc->boot_rom, 1);
-    aspeed_write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], rom_size,
-                          &error_abort);
-}
-
 #define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
 
 /*
@@ -460,9 +442,10 @@ static void aspeed_machine_init(MachineState *machine)
 
         if (fmc0 && !boot_emmc) {
             uint64_t rom_size = memory_region_size(&bmc->soc->spi_boot);
-            aspeed_install_boot_rom(bmc, fmc0, rom_size);
+            aspeed_install_boot_rom(bmc->soc, fmc0, &bmc->boot_rom, rom_size);
         } else if (emmc0) {
-            aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0), 64 * KiB);
+            aspeed_install_boot_rom(bmc->soc, blk_by_legacy_dinfo(emmc0),
+                                    &bmc->boot_rom, 64 * KiB);
         }
     }
 
diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
index d0a400725f..7f104f8de5 100644
--- a/hw/arm/aspeed_soc_common.c
+++ b/hw/arm/aspeed_soc_common.c
@@ -178,6 +178,23 @@ void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
     rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
 }
 
+/*
+ * Create a ROM and copy the flash contents at the expected address
+ * (0x0). Boots faster than execute-in-place.
+ */
+void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
+                             MemoryRegion *boot_rom, uint64_t rom_size)
+{
+    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
+
+    memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom", rom_size,
+                           &error_abort);
+    memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
+                                        boot_rom, 1);
+    aspeed_write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], rom_size,
+                          &error_abort);
+}
+
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 {
     AspeedSoCState *s = ASPEED_SOC(dev);
-- 
2.43.0

Re: [PATCH v2 3/7] hw/arm/aspeed: Move aspeed_install_boot_rom to common SoC code

[Cédric Le Goater]

On 9/24/25 07:55, Jamin Lin wrote:
> Move the boot ROM install helper into common SoC code so it can be reused
> by all ASPEED boards, and decouple the API from AspeedMachineState.
> 
> Specifically:
>   - Move aspeed_install_boot_rom() to hw/arm/aspeed_soc_common.c and
>     declare it in include/hw/arm/aspeed_soc.h.
>   - Change the helper’s signature to take AspeedSoCState * and a
>     MemoryRegion * provided by the caller, instead of AspeedMachineState *.
>   - Update aspeed_machine_init() call sites accordingly.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/hw/arm/aspeed_soc.h |  2 ++
>   hw/arm/aspeed.c             | 23 +++--------------------
>   hw/arm/aspeed_soc_common.c  | 17 +++++++++++++++++
>   3 files changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 5567bdcb69..aea210a8e2 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -314,6 +314,8 @@ void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
>                                  unsigned int count, int unit0);
>   void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
>                              Error **errp);
> +void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
> +                             MemoryRegion *boot_rom, uint64_t rom_size);
>   
>   static inline int aspeed_uart_index(int uart_dev)
>   {
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 4d0d935836..429f4c6d77 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -263,24 +263,6 @@ static void aspeed_reset_secondary(ARMCPU *cpu,
>       cpu_set_pc(cs, info->smp_loader_start);
>   }
>   
> -/*
> - * Create a ROM and copy the flash contents at the expected address
> - * (0x0). Boots faster than execute-in-place.
> - */
> -static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
> -                                    uint64_t rom_size)
> -{
> -    AspeedSoCState *soc = bmc->soc;
> -    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
> -
> -    memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size,
> -                           &error_abort);
> -    memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
> -                                        &bmc->boot_rom, 1);
> -    aspeed_write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], rom_size,
> -                          &error_abort);
> -}
> -
>   #define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
>   
>   /*
> @@ -460,9 +442,10 @@ static void aspeed_machine_init(MachineState *machine)
>   
>           if (fmc0 && !boot_emmc) {
>               uint64_t rom_size = memory_region_size(&bmc->soc->spi_boot);
> -            aspeed_install_boot_rom(bmc, fmc0, rom_size);
> +            aspeed_install_boot_rom(bmc->soc, fmc0, &bmc->boot_rom, rom_size);
>           } else if (emmc0) {
> -            aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0), 64 * KiB);
> +            aspeed_install_boot_rom(bmc->soc, blk_by_legacy_dinfo(emmc0),
> +                                    &bmc->boot_rom, 64 * KiB);
>           }
>       }
>   
> diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
> index d0a400725f..7f104f8de5 100644
> --- a/hw/arm/aspeed_soc_common.c
> +++ b/hw/arm/aspeed_soc_common.c
> @@ -178,6 +178,23 @@ void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
>       rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
>   }
>   
> +/*
> + * Create a ROM and copy the flash contents at the expected address
> + * (0x0). Boots faster than execute-in-place.
> + */
> +void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
> +                             MemoryRegion *boot_rom, uint64_t rom_size)
> +{
> +    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
> +
> +    memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom", rom_size,
> +                           &error_abort);
> +    memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
> +                                        boot_rom, 1);
> +    aspeed_write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], rom_size,
> +                          &error_abort);
> +}
> +
>   static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>   {
>       AspeedSoCState *s = ASPEED_SOC(dev);

[PATCH v2 4/7] hw/arm/aspeed: Move aspeed_load_vbootrom to common SoC code

[Jamin Lin via]

Move the vbootrom loader helper into common SoC code so it can be reused
by all ASPEED boards, and decouple the API from AspeedMachineState.

Specifically:
- Move aspeed_load_vbootrom() to hw/arm/aspeed_soc_common.c and
declare it in include/hw/arm/aspeed_soc.h.
- Change the helper’s signature to take AspeedSoCState * instead of
AspeedMachineState *.
- Update aspeed_machine_init() call sites accordingly.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h |  4 ++++
 hw/arm/aspeed.c             | 31 +------------------------------
 hw/arm/aspeed_soc_common.c  | 25 +++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index aea210a8e2..ed32efb543 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -43,6 +43,8 @@
 #include "hw/char/serial-mm.h"
 #include "hw/intc/arm_gicv3.h"
 
+#define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
+
 #define ASPEED_SPIS_NUM  3
 #define ASPEED_EHCIS_NUM 4
 #define ASPEED_WDTS_NUM  8
@@ -316,6 +318,8 @@ void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
                            Error **errp);
 void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
                              MemoryRegion *boot_rom, uint64_t rom_size);
+void aspeed_load_vbootrom(AspeedSoCState *soc, const char *bios_name,
+                          Error **errp);
 
 static inline int aspeed_uart_index(int uart_dev)
 {
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 429f4c6d77..6046ec0bb2 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -26,9 +26,7 @@
 #include "hw/qdev-properties.h"
 #include "system/block-backend.h"
 #include "system/reset.h"
-#include "hw/loader.h"
 #include "qemu/error-report.h"
-#include "qemu/datadir.h"
 #include "qemu/units.h"
 #include "hw/qdev-clock.h"
 #include "system/system.h"
@@ -263,33 +261,6 @@ static void aspeed_reset_secondary(ARMCPU *cpu,
     cpu_set_pc(cs, info->smp_loader_start);
 }
 
-#define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
-
-/*
- * This function locates the vbootrom image file specified via the command line
- * using the -bios option. It loads the specified image into the vbootrom
- * memory region and handles errors if the file cannot be found or loaded.
- */
-static void aspeed_load_vbootrom(AspeedMachineState *bmc, const char *bios_name,
-                                 Error **errp)
-{
-    g_autofree char *filename = NULL;
-    AspeedSoCState *soc = bmc->soc;
-    int ret;
-
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-    if (!filename) {
-        error_setg(errp, "Could not find vbootrom image '%s'", bios_name);
-        return;
-    }
-
-    ret = load_image_mr(filename, &soc->vbootrom);
-    if (ret < 0) {
-        error_setg(errp, "Failed to load vbootrom image '%s'", bios_name);
-        return;
-    }
-}
-
 static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
                                bool boot_emmc)
 {
@@ -451,7 +422,7 @@ static void aspeed_machine_init(MachineState *machine)
 
     if (amc->vbootrom) {
         bios_name = machine->firmware ?: VBOOTROM_FILE_NAME;
-        aspeed_load_vbootrom(bmc, bios_name, &error_abort);
+        aspeed_load_vbootrom(bmc->soc, bios_name, &error_abort);
     }
 
     arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo);
diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
index 7f104f8de5..bc70e864fb 100644
--- a/hw/arm/aspeed_soc_common.c
+++ b/hw/arm/aspeed_soc_common.c
@@ -19,6 +19,7 @@
 #include "system/blockdev.h"
 #include "system/block-backend.h"
 #include "hw/loader.h"
+#include "qemu/datadir.h"
 
 
 const char *aspeed_soc_cpu_type(AspeedSoCClass *sc)
@@ -195,6 +196,30 @@ void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
                           &error_abort);
 }
 
+/*
+ * This function locates the vbootrom image file specified via the command line
+ * using the -bios option. It loads the specified image into the vbootrom
+ * memory region and handles errors if the file cannot be found or loaded.
+ */
+void aspeed_load_vbootrom(AspeedSoCState *soc, const char *bios_name,
+                          Error **errp)
+{
+    g_autofree char *filename = NULL;
+    int ret;
+
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    if (!filename) {
+        error_setg(errp, "Could not find vbootrom image '%s'", bios_name);
+        return;
+    }
+
+    ret = load_image_mr(filename, &soc->vbootrom);
+    if (ret < 0) {
+        error_setg(errp, "Failed to load vbootrom image '%s'", bios_name);
+        return;
+    }
+}
+
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 {
     AspeedSoCState *s = ASPEED_SOC(dev);
-- 
2.43.0

[PATCH v2 4/7] hw/arm/aspeed: Move aspeed_load_vbootrom to common SoC code

[Jamin Lin via]

Move the vbootrom loader helper into common SoC code so it can be reused
by all ASPEED boards, and decouple the API from AspeedMachineState.

Specifically:
- Move aspeed_load_vbootrom() to hw/arm/aspeed_soc_common.c and
declare it in include/hw/arm/aspeed_soc.h.
- Change the helper’s signature to take AspeedSoCState * instead of
AspeedMachineState *.
- Update aspeed_machine_init() call sites accordingly.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h |  4 ++++
 hw/arm/aspeed.c             | 31 +------------------------------
 hw/arm/aspeed_soc_common.c  | 25 +++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index aea210a8e2..ed32efb543 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -43,6 +43,8 @@
 #include "hw/char/serial-mm.h"
 #include "hw/intc/arm_gicv3.h"
 
+#define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
+
 #define ASPEED_SPIS_NUM  3
 #define ASPEED_EHCIS_NUM 4
 #define ASPEED_WDTS_NUM  8
@@ -316,6 +318,8 @@ void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
                            Error **errp);
 void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
                              MemoryRegion *boot_rom, uint64_t rom_size);
+void aspeed_load_vbootrom(AspeedSoCState *soc, const char *bios_name,
+                          Error **errp);
 
 static inline int aspeed_uart_index(int uart_dev)
 {
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 429f4c6d77..6046ec0bb2 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -26,9 +26,7 @@
 #include "hw/qdev-properties.h"
 #include "system/block-backend.h"
 #include "system/reset.h"
-#include "hw/loader.h"
 #include "qemu/error-report.h"
-#include "qemu/datadir.h"
 #include "qemu/units.h"
 #include "hw/qdev-clock.h"
 #include "system/system.h"
@@ -263,33 +261,6 @@ static void aspeed_reset_secondary(ARMCPU *cpu,
     cpu_set_pc(cs, info->smp_loader_start);
 }
 
-#define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
-
-/*
- * This function locates the vbootrom image file specified via the command line
- * using the -bios option. It loads the specified image into the vbootrom
- * memory region and handles errors if the file cannot be found or loaded.
- */
-static void aspeed_load_vbootrom(AspeedMachineState *bmc, const char *bios_name,
-                                 Error **errp)
-{
-    g_autofree char *filename = NULL;
-    AspeedSoCState *soc = bmc->soc;
-    int ret;
-
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-    if (!filename) {
-        error_setg(errp, "Could not find vbootrom image '%s'", bios_name);
-        return;
-    }
-
-    ret = load_image_mr(filename, &soc->vbootrom);
-    if (ret < 0) {
-        error_setg(errp, "Failed to load vbootrom image '%s'", bios_name);
-        return;
-    }
-}
-
 static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
                                bool boot_emmc)
 {
@@ -451,7 +422,7 @@ static void aspeed_machine_init(MachineState *machine)
 
     if (amc->vbootrom) {
         bios_name = machine->firmware ?: VBOOTROM_FILE_NAME;
-        aspeed_load_vbootrom(bmc, bios_name, &error_abort);
+        aspeed_load_vbootrom(bmc->soc, bios_name, &error_abort);
     }
 
     arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo);
diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
index 7f104f8de5..bc70e864fb 100644
--- a/hw/arm/aspeed_soc_common.c
+++ b/hw/arm/aspeed_soc_common.c
@@ -19,6 +19,7 @@
 #include "system/blockdev.h"
 #include "system/block-backend.h"
 #include "hw/loader.h"
+#include "qemu/datadir.h"
 
 
 const char *aspeed_soc_cpu_type(AspeedSoCClass *sc)
@@ -195,6 +196,30 @@ void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
                           &error_abort);
 }
 
+/*
+ * This function locates the vbootrom image file specified via the command line
+ * using the -bios option. It loads the specified image into the vbootrom
+ * memory region and handles errors if the file cannot be found or loaded.
+ */
+void aspeed_load_vbootrom(AspeedSoCState *soc, const char *bios_name,
+                          Error **errp)
+{
+    g_autofree char *filename = NULL;
+    int ret;
+
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    if (!filename) {
+        error_setg(errp, "Could not find vbootrom image '%s'", bios_name);
+        return;
+    }
+
+    ret = load_image_mr(filename, &soc->vbootrom);
+    if (ret < 0) {
+        error_setg(errp, "Failed to load vbootrom image '%s'", bios_name);
+        return;
+    }
+}
+
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 {
     AspeedSoCState *s = ASPEED_SOC(dev);
-- 
2.43.0

Re: [PATCH v2 4/7] hw/arm/aspeed: Move aspeed_load_vbootrom to common SoC code

[Cédric Le Goater]

On 9/24/25 07:55, Jamin Lin wrote:
> Move the vbootrom loader helper into common SoC code so it can be reused
> by all ASPEED boards, and decouple the API from AspeedMachineState.
> 
> Specifically:
> - Move aspeed_load_vbootrom() to hw/arm/aspeed_soc_common.c and
> declare it in include/hw/arm/aspeed_soc.h.
> - Change the helper’s signature to take AspeedSoCState * instead of
> AspeedMachineState *.
> - Update aspeed_machine_init() call sites accordingly.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/hw/arm/aspeed_soc.h |  4 ++++
>   hw/arm/aspeed.c             | 31 +------------------------------
>   hw/arm/aspeed_soc_common.c  | 25 +++++++++++++++++++++++++
>   3 files changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index aea210a8e2..ed32efb543 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -43,6 +43,8 @@
>   #include "hw/char/serial-mm.h"
>   #include "hw/intc/arm_gicv3.h"
>   
> +#define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
> +
>   #define ASPEED_SPIS_NUM  3
>   #define ASPEED_EHCIS_NUM 4
>   #define ASPEED_WDTS_NUM  8
> @@ -316,6 +318,8 @@ void aspeed_write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
>                              Error **errp);
>   void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
>                                MemoryRegion *boot_rom, uint64_t rom_size);
> +void aspeed_load_vbootrom(AspeedSoCState *soc, const char *bios_name,
> +                          Error **errp);
>   
>   static inline int aspeed_uart_index(int uart_dev)
>   {
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 429f4c6d77..6046ec0bb2 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -26,9 +26,7 @@
>   #include "hw/qdev-properties.h"
>   #include "system/block-backend.h"
>   #include "system/reset.h"
> -#include "hw/loader.h"
>   #include "qemu/error-report.h"
> -#include "qemu/datadir.h"
>   #include "qemu/units.h"
>   #include "hw/qdev-clock.h"
>   #include "system/system.h"
> @@ -263,33 +261,6 @@ static void aspeed_reset_secondary(ARMCPU *cpu,
>       cpu_set_pc(cs, info->smp_loader_start);
>   }
>   
> -#define VBOOTROM_FILE_NAME  "ast27x0_bootrom.bin"
> -
> -/*
> - * This function locates the vbootrom image file specified via the command line
> - * using the -bios option. It loads the specified image into the vbootrom
> - * memory region and handles errors if the file cannot be found or loaded.
> - */
> -static void aspeed_load_vbootrom(AspeedMachineState *bmc, const char *bios_name,
> -                                 Error **errp)
> -{
> -    g_autofree char *filename = NULL;
> -    AspeedSoCState *soc = bmc->soc;
> -    int ret;
> -
> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> -    if (!filename) {
> -        error_setg(errp, "Could not find vbootrom image '%s'", bios_name);
> -        return;
> -    }
> -
> -    ret = load_image_mr(filename, &soc->vbootrom);
> -    if (ret < 0) {
> -        error_setg(errp, "Failed to load vbootrom image '%s'", bios_name);
> -        return;
> -    }
> -}
> -
>   static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
>                                  bool boot_emmc)
>   {
> @@ -451,7 +422,7 @@ static void aspeed_machine_init(MachineState *machine)
>   
>       if (amc->vbootrom) {
>           bios_name = machine->firmware ?: VBOOTROM_FILE_NAME;
> -        aspeed_load_vbootrom(bmc, bios_name, &error_abort);
> +        aspeed_load_vbootrom(bmc->soc, bios_name, &error_abort);
>       }
>   
>       arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo);
> diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
> index 7f104f8de5..bc70e864fb 100644
> --- a/hw/arm/aspeed_soc_common.c
> +++ b/hw/arm/aspeed_soc_common.c
> @@ -19,6 +19,7 @@
>   #include "system/blockdev.h"
>   #include "system/block-backend.h"
>   #include "hw/loader.h"
> +#include "qemu/datadir.h"
>   
>   
>   const char *aspeed_soc_cpu_type(AspeedSoCClass *sc)
> @@ -195,6 +196,30 @@ void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk,
>                             &error_abort);
>   }
>   
> +/*
> + * This function locates the vbootrom image file specified via the command line
> + * using the -bios option. It loads the specified image into the vbootrom
> + * memory region and handles errors if the file cannot be found or loaded.
> + */
> +void aspeed_load_vbootrom(AspeedSoCState *soc, const char *bios_name,
> +                          Error **errp)
> +{
> +    g_autofree char *filename = NULL;
> +    int ret;
> +
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +    if (!filename) {
> +        error_setg(errp, "Could not find vbootrom image '%s'", bios_name);
> +        return;
> +    }
> +
> +    ret = load_image_mr(filename, &soc->vbootrom);
> +    if (ret < 0) {
> +        error_setg(errp, "Failed to load vbootrom image '%s'", bios_name);
> +        return;
> +    }
> +}
> +
>   static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>   {
>       AspeedSoCState *s = ASPEED_SOC(dev);

[PATCH v2 5/7] hw/arm/aspeed_ast27x0-fc: Replace error_abort with local errp

[Jamin Lin via]

This patch introduces a local Error **errp = NULL and
replaces error_abort with errp in these paths.

This makes error handling more consistent with QEMU guidelines and avoids
using error_abort in cases where failure should not be treated as a
programming error.

Discussion:
object_property_set_link() can return false only when it fails, and it
sets an error when it fails. Since passing &error_abort causes an abort,
the function never returns false, and the return statement is effectively
dead code. If failure is considered a programming error, using
&error_abort is correct. However, if failure is not a programming error,
passing &error_abort is wrong, and errp should be used instead. This
patch follows the latter approach by replacing error_abort with errp.
https://patchwork.kernel.org/project/qemu-devel/patch/20250717034054.1903991-3-jamin_lin@aspeedtech.com/#26540626

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index 7087be4288..b15cb94c39 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -61,6 +61,7 @@ static void ast2700fc_ca35_init(MachineState *machine)
     Ast2700FCState *s = AST2700A1FC(machine);
     AspeedSoCState *soc;
     AspeedSoCClass *sc;
+    Error **errp = NULL;
 
     object_initialize_child(OBJECT(s), "ca35", &s->ca35, "ast2700-a1");
     soc = ASPEED_SOC(&s->ca35);
@@ -71,20 +72,20 @@ static void ast2700fc_ca35_init(MachineState *machine)
     memory_region_add_subregion(get_system_memory(), 0, &s->ca35_memory);
 
     if (!memory_region_init_ram(&s->ca35_dram, OBJECT(&s->ca35), "ca35-dram",
-                                AST2700FC_BMC_RAM_SIZE, &error_abort)) {
+                                AST2700FC_BMC_RAM_SIZE, errp)) {
         return;
     }
     if (!object_property_set_link(OBJECT(&s->ca35), "memory",
                                   OBJECT(&s->ca35_memory),
-                                  &error_abort)) {
+                                  errp)) {
         return;
     };
     if (!object_property_set_link(OBJECT(&s->ca35), "dram",
-                                  OBJECT(&s->ca35_dram), &error_abort)) {
+                                  OBJECT(&s->ca35_dram), errp)) {
         return;
     }
     if (!object_property_set_int(OBJECT(&s->ca35), "ram-size",
-                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
+                                 AST2700FC_BMC_RAM_SIZE, errp)) {
         return;
     }
 
@@ -95,15 +96,15 @@ static void ast2700fc_ca35_init(MachineState *machine)
         }
     }
     if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
-                                 AST2700FC_HW_STRAP1, &error_abort)) {
+                                 AST2700FC_HW_STRAP1, errp)) {
         return;
     }
     if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
-                                 AST2700FC_HW_STRAP2, &error_abort)) {
+                                 AST2700FC_HW_STRAP2, errp)) {
         return;
     }
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART12, serial_hd(0));
-    if (!qdev_realize(DEVICE(&s->ca35), NULL, &error_abort)) {
+    if (!qdev_realize(DEVICE(&s->ca35), NULL, errp)) {
         return;
     }
 
@@ -125,6 +126,8 @@ static void ast2700fc_ssp_init(MachineState *machine)
 {
     AspeedSoCState *soc;
     Ast2700FCState *s = AST2700A1FC(machine);
+    Error **errp = NULL;
+
     s->ssp_sysclk = clock_new(OBJECT(s), "SSP_SYSCLK");
     clock_set_hz(s->ssp_sysclk, 200000000ULL);
 
@@ -134,13 +137,13 @@ static void ast2700fc_ssp_init(MachineState *machine)
 
     qdev_connect_clock_in(DEVICE(&s->ssp), "sysclk", s->ssp_sysclk);
     if (!object_property_set_link(OBJECT(&s->ssp), "memory",
-                                  OBJECT(&s->ssp_memory), &error_abort)) {
+                                  OBJECT(&s->ssp_memory), errp)) {
         return;
     }
 
     soc = ASPEED_SOC(&s->ssp);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART4, serial_hd(1));
-    if (!qdev_realize(DEVICE(&s->ssp), NULL, &error_abort)) {
+    if (!qdev_realize(DEVICE(&s->ssp), NULL, errp)) {
         return;
     }
 }
@@ -149,6 +152,8 @@ static void ast2700fc_tsp_init(MachineState *machine)
 {
     AspeedSoCState *soc;
     Ast2700FCState *s = AST2700A1FC(machine);
+    Error **errp = NULL;
+
     s->tsp_sysclk = clock_new(OBJECT(s), "TSP_SYSCLK");
     clock_set_hz(s->tsp_sysclk, 200000000ULL);
 
@@ -158,13 +163,13 @@ static void ast2700fc_tsp_init(MachineState *machine)
 
     qdev_connect_clock_in(DEVICE(&s->tsp), "sysclk", s->tsp_sysclk);
     if (!object_property_set_link(OBJECT(&s->tsp), "memory",
-                                  OBJECT(&s->tsp_memory), &error_abort)) {
+                                  OBJECT(&s->tsp_memory), errp)) {
         return;
     }
 
     soc = ASPEED_SOC(&s->tsp);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART7, serial_hd(2));
-    if (!qdev_realize(DEVICE(&s->tsp), NULL, &error_abort)) {
+    if (!qdev_realize(DEVICE(&s->tsp), NULL, errp)) {
         return;
     }
 }
-- 
2.43.0

[PATCH v2 5/7] hw/arm/aspeed_ast27x0-fc: Replace error_abort with local errp

[Jamin Lin via]

This patch introduces a local Error **errp = NULL and
replaces error_abort with errp in these paths.

This makes error handling more consistent with QEMU guidelines and avoids
using error_abort in cases where failure should not be treated as a
programming error.

Discussion:
object_property_set_link() can return false only when it fails, and it
sets an error when it fails. Since passing &error_abort causes an abort,
the function never returns false, and the return statement is effectively
dead code. If failure is considered a programming error, using
&error_abort is correct. However, if failure is not a programming error,
passing &error_abort is wrong, and errp should be used instead. This
patch follows the latter approach by replacing error_abort with errp.
https://patchwork.kernel.org/project/qemu-devel/patch/20250717034054.1903991-3-jamin_lin@aspeedtech.com/#26540626

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index 7087be4288..b15cb94c39 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -61,6 +61,7 @@ static void ast2700fc_ca35_init(MachineState *machine)
     Ast2700FCState *s = AST2700A1FC(machine);
     AspeedSoCState *soc;
     AspeedSoCClass *sc;
+    Error **errp = NULL;
 
     object_initialize_child(OBJECT(s), "ca35", &s->ca35, "ast2700-a1");
     soc = ASPEED_SOC(&s->ca35);
@@ -71,20 +72,20 @@ static void ast2700fc_ca35_init(MachineState *machine)
     memory_region_add_subregion(get_system_memory(), 0, &s->ca35_memory);
 
     if (!memory_region_init_ram(&s->ca35_dram, OBJECT(&s->ca35), "ca35-dram",
-                                AST2700FC_BMC_RAM_SIZE, &error_abort)) {
+                                AST2700FC_BMC_RAM_SIZE, errp)) {
         return;
     }
     if (!object_property_set_link(OBJECT(&s->ca35), "memory",
                                   OBJECT(&s->ca35_memory),
-                                  &error_abort)) {
+                                  errp)) {
         return;
     };
     if (!object_property_set_link(OBJECT(&s->ca35), "dram",
-                                  OBJECT(&s->ca35_dram), &error_abort)) {
+                                  OBJECT(&s->ca35_dram), errp)) {
         return;
     }
     if (!object_property_set_int(OBJECT(&s->ca35), "ram-size",
-                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
+                                 AST2700FC_BMC_RAM_SIZE, errp)) {
         return;
     }
 
@@ -95,15 +96,15 @@ static void ast2700fc_ca35_init(MachineState *machine)
         }
     }
     if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
-                                 AST2700FC_HW_STRAP1, &error_abort)) {
+                                 AST2700FC_HW_STRAP1, errp)) {
         return;
     }
     if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
-                                 AST2700FC_HW_STRAP2, &error_abort)) {
+                                 AST2700FC_HW_STRAP2, errp)) {
         return;
     }
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART12, serial_hd(0));
-    if (!qdev_realize(DEVICE(&s->ca35), NULL, &error_abort)) {
+    if (!qdev_realize(DEVICE(&s->ca35), NULL, errp)) {
         return;
     }
 
@@ -125,6 +126,8 @@ static void ast2700fc_ssp_init(MachineState *machine)
 {
     AspeedSoCState *soc;
     Ast2700FCState *s = AST2700A1FC(machine);
+    Error **errp = NULL;
+
     s->ssp_sysclk = clock_new(OBJECT(s), "SSP_SYSCLK");
     clock_set_hz(s->ssp_sysclk, 200000000ULL);
 
@@ -134,13 +137,13 @@ static void ast2700fc_ssp_init(MachineState *machine)
 
     qdev_connect_clock_in(DEVICE(&s->ssp), "sysclk", s->ssp_sysclk);
     if (!object_property_set_link(OBJECT(&s->ssp), "memory",
-                                  OBJECT(&s->ssp_memory), &error_abort)) {
+                                  OBJECT(&s->ssp_memory), errp)) {
         return;
     }
 
     soc = ASPEED_SOC(&s->ssp);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART4, serial_hd(1));
-    if (!qdev_realize(DEVICE(&s->ssp), NULL, &error_abort)) {
+    if (!qdev_realize(DEVICE(&s->ssp), NULL, errp)) {
         return;
     }
 }
@@ -149,6 +152,8 @@ static void ast2700fc_tsp_init(MachineState *machine)
 {
     AspeedSoCState *soc;
     Ast2700FCState *s = AST2700A1FC(machine);
+    Error **errp = NULL;
+
     s->tsp_sysclk = clock_new(OBJECT(s), "TSP_SYSCLK");
     clock_set_hz(s->tsp_sysclk, 200000000ULL);
 
@@ -158,13 +163,13 @@ static void ast2700fc_tsp_init(MachineState *machine)
 
     qdev_connect_clock_in(DEVICE(&s->tsp), "sysclk", s->tsp_sysclk);
     if (!object_property_set_link(OBJECT(&s->tsp), "memory",
-                                  OBJECT(&s->tsp_memory), &error_abort)) {
+                                  OBJECT(&s->tsp_memory), errp)) {
         return;
     }
 
     soc = ASPEED_SOC(&s->tsp);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART7, serial_hd(2));
-    if (!qdev_realize(DEVICE(&s->tsp), NULL, &error_abort)) {
+    if (!qdev_realize(DEVICE(&s->tsp), NULL, errp)) {
         return;
     }
 }
-- 
2.43.0

Re: [PATCH v2 5/7] hw/arm/aspeed_ast27x0-fc: Replace error_abort with local errp

[Cédric Le Goater]

On 9/24/25 07:55, Jamin Lin wrote:
> This patch introduces a local Error **errp = NULL and
> replaces error_abort with errp in these paths.
> 
> This makes error handling more consistent with QEMU guidelines and avoids
> using error_abort in cases where failure should not be treated as a
> programming error.
> 
> Discussion:
> object_property_set_link() can return false only when it fails, and it
> sets an error when it fails. Since passing &error_abort causes an abort,
> the function never returns false, and the return statement is effectively
> dead code. If failure is considered a programming error, using
> &error_abort is correct. However, if failure is not a programming error,
> passing &error_abort is wrong, and errp should be used instead. This
> patch follows the latter approach by replacing error_abort with errp.
> https://patchwork.kernel.org/project/qemu-devel/patch/20250717034054.1903991-3-jamin_lin@aspeedtech.com/#26540626
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast27x0-fc.c | 27 ++++++++++++++++-----------
>   1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index 7087be4288..b15cb94c39 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -61,6 +61,7 @@ static void ast2700fc_ca35_init(MachineState *machine)


ast2700fc_ca35_init(), and others below, should to take an 'Error **errp'
parameter and return a bool, which should be false in case of error. The
caller, ast2700fc_init() would then check the returned value and possibly
report the error.

A good reading on the error topic is in file "include/qapi/error.h".
  
>       Ast2700FCState *s = AST2700A1FC(machine);
>       AspeedSoCState *soc;
>       AspeedSoCClass *sc;
> +    Error **errp = NULL;
>   
>       object_initialize_child(OBJECT(s), "ca35", &s->ca35, "ast2700-a1");
>       soc = ASPEED_SOC(&s->ca35);
> @@ -71,20 +72,20 @@ static void ast2700fc_ca35_init(MachineState *machine)
>       memory_region_add_subregion(get_system_memory(), 0, &s->ca35_memory);
>   
>       if (!memory_region_init_ram(&s->ca35_dram, OBJECT(&s->ca35), "ca35-dram",
> -                                AST2700FC_BMC_RAM_SIZE, &error_abort)) {
> +                                AST2700FC_BMC_RAM_SIZE, errp)) {
>           return;
>       }
>       if (!object_property_set_link(OBJECT(&s->ca35), "memory",
>                                     OBJECT(&s->ca35_memory),
> -                                  &error_abort)) {
> +                                  errp)) {
>           return;
>       };
>       if (!object_property_set_link(OBJECT(&s->ca35), "dram",
> -                                  OBJECT(&s->ca35_dram), &error_abort)) {
> +                                  OBJECT(&s->ca35_dram), errp)) {
>           return;
>       }
>       if (!object_property_set_int(OBJECT(&s->ca35), "ram-size",
> -                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
> +                                 AST2700FC_BMC_RAM_SIZE, errp)) {

object_property_set_int() is considered as a routine which shouldn't fail.
So the common practice in models is to pass &error_abort and ignore the
returned value.


Thanks,

C.



>           return;
>       }
>   
> @@ -95,15 +96,15 @@ static void ast2700fc_ca35_init(MachineState *machine)
>           }
>       }
>       if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
> -                                 AST2700FC_HW_STRAP1, &error_abort)) {
> +                                 AST2700FC_HW_STRAP1, errp)) {
>           return;
>       }
>       if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
> -                                 AST2700FC_HW_STRAP2, &error_abort)) {
> +                                 AST2700FC_HW_STRAP2, errp)) {
>           return;
>       }
>       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART12, serial_hd(0));
> -    if (!qdev_realize(DEVICE(&s->ca35), NULL, &error_abort)) {
> +    if (!qdev_realize(DEVICE(&s->ca35), NULL, errp)) {
>           return;
>       }
>   
> @@ -125,6 +126,8 @@ static void ast2700fc_ssp_init(MachineState *machine)
>   {
>       AspeedSoCState *soc;
>       Ast2700FCState *s = AST2700A1FC(machine);
> +    Error **errp = NULL;
> +
>       s->ssp_sysclk = clock_new(OBJECT(s), "SSP_SYSCLK");
>       clock_set_hz(s->ssp_sysclk, 200000000ULL);
>   
> @@ -134,13 +137,13 @@ static void ast2700fc_ssp_init(MachineState *machine)
>   
>       qdev_connect_clock_in(DEVICE(&s->ssp), "sysclk", s->ssp_sysclk);
>       if (!object_property_set_link(OBJECT(&s->ssp), "memory",
> -                                  OBJECT(&s->ssp_memory), &error_abort)) {
> +                                  OBJECT(&s->ssp_memory), errp)) {
>           return;
>       }
>   
>       soc = ASPEED_SOC(&s->ssp);
>       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART4, serial_hd(1));
> -    if (!qdev_realize(DEVICE(&s->ssp), NULL, &error_abort)) {
> +    if (!qdev_realize(DEVICE(&s->ssp), NULL, errp)) {
>           return;
>       }
>   }
> @@ -149,6 +152,8 @@ static void ast2700fc_tsp_init(MachineState *machine)
>   {
>       AspeedSoCState *soc;
>       Ast2700FCState *s = AST2700A1FC(machine);
> +    Error **errp = NULL;
> +
>       s->tsp_sysclk = clock_new(OBJECT(s), "TSP_SYSCLK");
>       clock_set_hz(s->tsp_sysclk, 200000000ULL);
>   
> @@ -158,13 +163,13 @@ static void ast2700fc_tsp_init(MachineState *machine)
>   
>       qdev_connect_clock_in(DEVICE(&s->tsp), "sysclk", s->tsp_sysclk);
>       if (!object_property_set_link(OBJECT(&s->tsp), "memory",
> -                                  OBJECT(&s->tsp_memory), &error_abort)) {
> +                                  OBJECT(&s->tsp_memory), errp)) {
>           return;
>       }
>   
>       soc = ASPEED_SOC(&s->tsp);
>       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART7, serial_hd(2));
> -    if (!qdev_realize(DEVICE(&s->tsp), NULL, &error_abort)) {
> +    if (!qdev_realize(DEVICE(&s->tsp), NULL, errp)) {
>           return;
>       }
>   }

RE: [PATCH v2 5/7] hw/arm/aspeed_ast27x0-fc: Replace error_abort with local errp

[Jamin Lin]

Hi Cédric

> Subject: Re: [PATCH v2 5/7] hw/arm/aspeed_ast27x0-fc: Replace error_abort
> with local errp
> 
> On 9/24/25 07:55, Jamin Lin wrote:
> > This patch introduces a local Error **errp = NULL and replaces
> > error_abort with errp in these paths.
> >
> > This makes error handling more consistent with QEMU guidelines and
> > avoids using error_abort in cases where failure should not be treated
> > as a programming error.
> >
> > Discussion:
> > object_property_set_link() can return false only when it fails, and it
> > sets an error when it fails. Since passing &error_abort causes an
> > abort, the function never returns false, and the return statement is
> > effectively dead code. If failure is considered a programming error,
> > using &error_abort is correct. However, if failure is not a
> > programming error, passing &error_abort is wrong, and errp should be
> > used instead. This patch follows the latter approach by replacing error_abort
> with errp.
> > https://patchwork.kernel.org/project/qemu-devel/patch/20250717034054.1
> > 903991-3-jamin_lin@aspeedtech.com/#26540626
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >   hw/arm/aspeed_ast27x0-fc.c | 27 ++++++++++++++++-----------
> >   1 file changed, 16 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> > index 7087be4288..b15cb94c39 100644
> > --- a/hw/arm/aspeed_ast27x0-fc.c
> > +++ b/hw/arm/aspeed_ast27x0-fc.c
> > @@ -61,6 +61,7 @@ static void ast2700fc_ca35_init(MachineState
> > *machine)
> 
> 
> ast2700fc_ca35_init(), and others below, should to take an 'Error **errp'
> parameter and return a bool, which should be false in case of error. The caller,
> ast2700fc_init() would then check the returned value and possibly report the
> error.


Thanks for your review and suggestion.
Will fix it.

> 
> A good reading on the error topic is in file "include/qapi/error.h".
> 
> >       Ast2700FCState *s = AST2700A1FC(machine);
> >       AspeedSoCState *soc;
> >       AspeedSoCClass *sc;
> > +    Error **errp = NULL;
> >
> >       object_initialize_child(OBJECT(s), "ca35", &s->ca35, "ast2700-a1");
> >       soc = ASPEED_SOC(&s->ca35);
> > @@ -71,20 +72,20 @@ static void ast2700fc_ca35_init(MachineState
> *machine)
> >       memory_region_add_subregion(get_system_memory(), 0,
> > &s->ca35_memory);
> >
> >       if (!memory_region_init_ram(&s->ca35_dram, OBJECT(&s->ca35),
> "ca35-dram",
> > -                                AST2700FC_BMC_RAM_SIZE,
> &error_abort)) {
> > +                                AST2700FC_BMC_RAM_SIZE, errp))
> {
> >           return;
> >       }
> >       if (!object_property_set_link(OBJECT(&s->ca35), "memory",
> >                                     OBJECT(&s->ca35_memory),
> > -                                  &error_abort)) {
> > +                                  errp)) {
> >           return;
> >       };
> >       if (!object_property_set_link(OBJECT(&s->ca35), "dram",
> > -                                  OBJECT(&s->ca35_dram),
> &error_abort)) {
> > +                                  OBJECT(&s->ca35_dram), errp)) {
> >           return;
> >       }
> >       if (!object_property_set_int(OBJECT(&s->ca35), "ram-size",
> > -                                 AST2700FC_BMC_RAM_SIZE,
> &error_abort)) {
> > +                                 AST2700FC_BMC_RAM_SIZE, errp))
> {
> 
> object_property_set_int() is considered as a routine which shouldn't fail.
> So the common practice in models is to pass &error_abort and ignore the
> returned value.
>
Will fix it.

Thanks-Jamin
> 
> Thanks,
> 
> C.
> 
> 
> 
> >           return;
> >       }
> >
> > @@ -95,15 +96,15 @@ static void ast2700fc_ca35_init(MachineState
> *machine)
> >           }
> >       }
> >       if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
> > -                                 AST2700FC_HW_STRAP1,
> &error_abort)) {
> > +                                 AST2700FC_HW_STRAP1, errp)) {
> >           return;
> >       }
> >       if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
> > -                                 AST2700FC_HW_STRAP2,
> &error_abort)) {
> > +                                 AST2700FC_HW_STRAP2, errp)) {
> >           return;
> >       }
> >       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART12, serial_hd(0));
> > -    if (!qdev_realize(DEVICE(&s->ca35), NULL, &error_abort)) {
> > +    if (!qdev_realize(DEVICE(&s->ca35), NULL, errp)) {
> >           return;
> >       }
> >
> > @@ -125,6 +126,8 @@ static void ast2700fc_ssp_init(MachineState
> *machine)
> >   {
> >       AspeedSoCState *soc;
> >       Ast2700FCState *s = AST2700A1FC(machine);
> > +    Error **errp = NULL;
> > +
> >       s->ssp_sysclk = clock_new(OBJECT(s), "SSP_SYSCLK");
> >       clock_set_hz(s->ssp_sysclk, 200000000ULL);
> >
> > @@ -134,13 +137,13 @@ static void ast2700fc_ssp_init(MachineState
> > *machine)
> >
> >       qdev_connect_clock_in(DEVICE(&s->ssp), "sysclk", s->ssp_sysclk);
> >       if (!object_property_set_link(OBJECT(&s->ssp), "memory",
> > -                                  OBJECT(&s->ssp_memory),
> &error_abort)) {
> > +                                  OBJECT(&s->ssp_memory), errp))
> {
> >           return;
> >       }
> >
> >       soc = ASPEED_SOC(&s->ssp);
> >       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART4, serial_hd(1));
> > -    if (!qdev_realize(DEVICE(&s->ssp), NULL, &error_abort)) {
> > +    if (!qdev_realize(DEVICE(&s->ssp), NULL, errp)) {
> >           return;
> >       }
> >   }
> > @@ -149,6 +152,8 @@ static void ast2700fc_tsp_init(MachineState
> *machine)
> >   {
> >       AspeedSoCState *soc;
> >       Ast2700FCState *s = AST2700A1FC(machine);
> > +    Error **errp = NULL;
> > +
> >       s->tsp_sysclk = clock_new(OBJECT(s), "TSP_SYSCLK");
> >       clock_set_hz(s->tsp_sysclk, 200000000ULL);
> >
> > @@ -158,13 +163,13 @@ static void ast2700fc_tsp_init(MachineState
> > *machine)
> >
> >       qdev_connect_clock_in(DEVICE(&s->tsp), "sysclk", s->tsp_sysclk);
> >       if (!object_property_set_link(OBJECT(&s->tsp), "memory",
> > -                                  OBJECT(&s->tsp_memory),
> &error_abort)) {
> > +                                  OBJECT(&s->tsp_memory), errp))
> {
> >           return;
> >       }
> >
> >       soc = ASPEED_SOC(&s->tsp);
> >       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART7, serial_hd(2));
> > -    if (!qdev_realize(DEVICE(&s->tsp), NULL, &error_abort)) {
> > +    if (!qdev_realize(DEVICE(&s->tsp), NULL, errp)) {
> >           return;
> >       }
> >   }

[PATCH v2 6/7] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM

[Jamin Lin via]

This patch introduces a dedicated ca35_boot_rom memory region and
copies the FMC0 flash data into it when mmio_exec is disabled.

The main purpose of this change is to support the upcoming VBOOTROM
, which can directly fetch data from FMC0 flash at the SPI boot ROM
base address (0x100000000) without requiring any SPI controller
drivers.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index b15cb94c39..2e6036b192 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -35,6 +35,7 @@ struct Ast2700FCState {
 
     MemoryRegion ca35_memory;
     MemoryRegion ca35_dram;
+    MemoryRegion ca35_boot_rom;
     MemoryRegion ssp_memory;
     MemoryRegion tsp_memory;
 
@@ -61,7 +62,10 @@ static void ast2700fc_ca35_init(MachineState *machine)
     Ast2700FCState *s = AST2700A1FC(machine);
     AspeedSoCState *soc;
     AspeedSoCClass *sc;
+    BlockBackend *fmc0 = NULL;
+    DeviceState *dev = NULL;
     Error **errp = NULL;
+    uint64_t rom_size;
 
     object_initialize_child(OBJECT(s), "ca35", &s->ca35, "ast2700-a1");
     soc = ASPEED_SOC(&s->ca35);
@@ -119,6 +123,16 @@ static void ast2700fc_ca35_init(MachineState *machine)
     ast2700fc_board_info.ram_size = machine->ram_size;
     ast2700fc_board_info.loader_start = sc->memmap[ASPEED_DEV_SDRAM];
 
+    if (!s->mmio_exec) {
+        dev = ssi_get_cs(soc->fmc.spi, 0);
+        fmc0 = dev ? m25p80_get_blk(dev) : NULL;
+
+        if (fmc0) {
+            rom_size = memory_region_size(&soc->spi_boot);
+            aspeed_install_boot_rom(soc, fmc0, &s->ca35_boot_rom, rom_size);
+        }
+    }
+
     arm_load_kernel(ARM_CPU(first_cpu), machine, &ast2700fc_board_info);
 }
 
-- 
2.43.0

[PATCH v2 6/7] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM

[Jamin Lin via]

This patch introduces a dedicated ca35_boot_rom memory region and
copies the FMC0 flash data into it when mmio_exec is disabled.

The main purpose of this change is to support the upcoming VBOOTROM
, which can directly fetch data from FMC0 flash at the SPI boot ROM
base address (0x100000000) without requiring any SPI controller
drivers.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index b15cb94c39..2e6036b192 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -35,6 +35,7 @@ struct Ast2700FCState {
 
     MemoryRegion ca35_memory;
     MemoryRegion ca35_dram;
+    MemoryRegion ca35_boot_rom;
     MemoryRegion ssp_memory;
     MemoryRegion tsp_memory;
 
@@ -61,7 +62,10 @@ static void ast2700fc_ca35_init(MachineState *machine)
     Ast2700FCState *s = AST2700A1FC(machine);
     AspeedSoCState *soc;
     AspeedSoCClass *sc;
+    BlockBackend *fmc0 = NULL;
+    DeviceState *dev = NULL;
     Error **errp = NULL;
+    uint64_t rom_size;
 
     object_initialize_child(OBJECT(s), "ca35", &s->ca35, "ast2700-a1");
     soc = ASPEED_SOC(&s->ca35);
@@ -119,6 +123,16 @@ static void ast2700fc_ca35_init(MachineState *machine)
     ast2700fc_board_info.ram_size = machine->ram_size;
     ast2700fc_board_info.loader_start = sc->memmap[ASPEED_DEV_SDRAM];
 
+    if (!s->mmio_exec) {
+        dev = ssi_get_cs(soc->fmc.spi, 0);
+        fmc0 = dev ? m25p80_get_blk(dev) : NULL;
+
+        if (fmc0) {
+            rom_size = memory_region_size(&soc->spi_boot);
+            aspeed_install_boot_rom(soc, fmc0, &s->ca35_boot_rom, rom_size);
+        }
+    }
+
     arm_load_kernel(ARM_CPU(first_cpu), machine, &ast2700fc_board_info);
 }
 
-- 
2.43.0

Re: [PATCH v2 6/7] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM

[Cédric Le Goater]

On 9/24/25 07:56, Jamin Lin wrote:
> This patch introduces a dedicated ca35_boot_rom memory region and
> copies the FMC0 flash data into it when mmio_exec is disabled.
> 
> The main purpose of this change is to support the upcoming VBOOTROM
> , which can directly fetch data from FMC0 flash at the SPI boot ROM
> base address (0x100000000) without requiring any SPI controller
> drivers.

That's the 'execute-in-place' case but the property activating
'execute-in-place' is always false in this machine. Could you
explain why booting from vbootrom needs the fmc0 contents to be
mapped in memory too ?




> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast27x0-fc.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index b15cb94c39..2e6036b192 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -35,6 +35,7 @@ struct Ast2700FCState {
>   
>       MemoryRegion ca35_memory;
>       MemoryRegion ca35_dram;
> +    MemoryRegion ca35_boot_rom;
>       MemoryRegion ssp_memory;
>       MemoryRegion tsp_memory;
>   
> @@ -61,7 +62,10 @@ static void ast2700fc_ca35_init(MachineState *machine)
>       Ast2700FCState *s = AST2700A1FC(machine);
>       AspeedSoCState *soc;
>       AspeedSoCClass *sc;
> +    BlockBackend *fmc0 = NULL;
> +    DeviceState *dev = NULL;
>       Error **errp = NULL;
> +    uint64_t rom_size;
>   
>       object_initialize_child(OBJECT(s), "ca35", &s->ca35, "ast2700-a1");
>       soc = ASPEED_SOC(&s->ca35);
> @@ -119,6 +123,16 @@ static void ast2700fc_ca35_init(MachineState *machine)
>       ast2700fc_board_info.ram_size = machine->ram_size;
>       ast2700fc_board_info.loader_start = sc->memmap[ASPEED_DEV_SDRAM];
>   
> +    if (!s->mmio_exec) {

bool 'mmio_exec' is always false in this machine. Should we keep it ?

Thanks,

C.



> +        dev = ssi_get_cs(soc->fmc.spi, 0);
> +        fmc0 = dev ? m25p80_get_blk(dev) : NULL;
> +
> +        if (fmc0) {
> +            rom_size = memory_region_size(&soc->spi_boot);
> +            aspeed_install_boot_rom(soc, fmc0, &s->ca35_boot_rom, rom_size);
> +        }
> +    }
> +
>       arm_load_kernel(ARM_CPU(first_cpu), machine, &ast2700fc_board_info);
>   }
>   

RE: [PATCH v2 6/7] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM

[Jamin Lin]

Hi Cédric

> Subject: Re: [PATCH v2 6/7] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash
> contents into CA35 boot ROM
> 
> On 9/24/25 07:56, Jamin Lin wrote:
> > This patch introduces a dedicated ca35_boot_rom memory region and
> > copies the FMC0 flash data into it when mmio_exec is disabled.
> >
> > The main purpose of this change is to support the upcoming VBOOTROM ,
> > which can directly fetch data from FMC0 flash at the SPI boot ROM base
> > address (0x100000000) without requiring any SPI controller drivers.
> 
> That's the 'execute-in-place' case but the property activating
> 'execute-in-place' is always false in this machine. Could you explain why
> booting from vbootrom needs the fmc0 contents to be mapped in memory
> too ?
> 
The main goal of vbootrom is to replace bootmuc (RISC-V 32).
Our current bootmuc firmware is SPL.
The SPL reads flash data (image-bmc) via FMC_CS0 and loads the following components into different DRAM addresses:
Trusted Firmware-A
OP-TEE OS
u-boot-nodtb.bin
u-boot.dtb

After loading, it(BOOTMCU) releases the CA35(PSP) reset so that CA35 can run Trusted Firmware-A.

The vbootrom performs the same sequence: it reads flash data (image-bmc), parses the FIT image,
and loads each image into its designated DRAM address. Finally, it jumps to Trusted Firmware-A.

https://github.com/google/vbootrom/blob/master/ast27x0/include/image.h#L21
https://github.com/google/vbootrom/blob/master/ast27x0/image.c#L30 
https://github.com/google/vbootrom/blob/master/ast27x0/image.c#L457
https://github.com/google/vbootrom/blob/master/ast27x0/image.c#L488-L505

That is why I need to copy FMC0 data into the its memory region to make vbootrom work correctly.

> 
> 
> 
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >   hw/arm/aspeed_ast27x0-fc.c | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> >
> > diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> > index b15cb94c39..2e6036b192 100644
> > --- a/hw/arm/aspeed_ast27x0-fc.c
> > +++ b/hw/arm/aspeed_ast27x0-fc.c
> > @@ -35,6 +35,7 @@ struct Ast2700FCState {
> >
> >       MemoryRegion ca35_memory;
> >       MemoryRegion ca35_dram;
> > +    MemoryRegion ca35_boot_rom;
> >       MemoryRegion ssp_memory;
> >       MemoryRegion tsp_memory;
> >
> > @@ -61,7 +62,10 @@ static void ast2700fc_ca35_init(MachineState
> *machine)
> >       Ast2700FCState *s = AST2700A1FC(machine);
> >       AspeedSoCState *soc;
> >       AspeedSoCClass *sc;
> > +    BlockBackend *fmc0 = NULL;
> > +    DeviceState *dev = NULL;
> >       Error **errp = NULL;
> > +    uint64_t rom_size;
> >
> >       object_initialize_child(OBJECT(s), "ca35", &s->ca35, "ast2700-a1");
> >       soc = ASPEED_SOC(&s->ca35);
> > @@ -119,6 +123,16 @@ static void ast2700fc_ca35_init(MachineState
> *machine)
> >       ast2700fc_board_info.ram_size = machine->ram_size;
> >       ast2700fc_board_info.loader_start =
> > sc->memmap[ASPEED_DEV_SDRAM];
> >
> > +    if (!s->mmio_exec) {
> 
> bool 'mmio_exec' is always false in this machine. Should we keep it ?
> 

Will remove it.

Thanks-Jamin
> Thanks,
> 
> C.
> 
> 
> 
> > +        dev = ssi_get_cs(soc->fmc.spi, 0);
> > +        fmc0 = dev ? m25p80_get_blk(dev) : NULL;
> > +
> > +        if (fmc0) {
> > +            rom_size = memory_region_size(&soc->spi_boot);
> > +            aspeed_install_boot_rom(soc, fmc0, &s->ca35_boot_rom,
> rom_size);
> > +        }
> > +    }
> > +
> >       arm_load_kernel(ARM_CPU(first_cpu), machine,
> &ast2700fc_board_info);
> >   }
> >

[PATCH v2 7/7] hw/arm/aspeed_ast27x0-fc: Add VBOOTROM support

[Jamin Lin via]

Introduces support for loading a vbootrom image into the dedicated vbootrom
memory region in the AST2700 Full Core machine.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index 2e6036b192..b2d963e1fe 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -62,6 +62,7 @@ static void ast2700fc_ca35_init(MachineState *machine)
     Ast2700FCState *s = AST2700A1FC(machine);
     AspeedSoCState *soc;
     AspeedSoCClass *sc;
+    const char *bios_name = NULL;
     BlockBackend *fmc0 = NULL;
     DeviceState *dev = NULL;
     Error **errp = NULL;
@@ -133,6 +134,10 @@ static void ast2700fc_ca35_init(MachineState *machine)
         }
     }
 
+    /* VBOOTROM */
+    bios_name = machine->firmware ?: VBOOTROM_FILE_NAME;
+    aspeed_load_vbootrom(soc, bios_name, errp);
+
     arm_load_kernel(ARM_CPU(first_cpu), machine, &ast2700fc_board_info);
 }
 
-- 
2.43.0

[PATCH v2 7/7] hw/arm/aspeed_ast27x0-fc: Add VBOOTROM support

[Jamin Lin via]

Introduces support for loading a vbootrom image into the dedicated vbootrom
memory region in the AST2700 Full Core machine.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index 2e6036b192..b2d963e1fe 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -62,6 +62,7 @@ static void ast2700fc_ca35_init(MachineState *machine)
     Ast2700FCState *s = AST2700A1FC(machine);
     AspeedSoCState *soc;
     AspeedSoCClass *sc;
+    const char *bios_name = NULL;
     BlockBackend *fmc0 = NULL;
     DeviceState *dev = NULL;
     Error **errp = NULL;
@@ -133,6 +134,10 @@ static void ast2700fc_ca35_init(MachineState *machine)
         }
     }
 
+    /* VBOOTROM */
+    bios_name = machine->firmware ?: VBOOTROM_FILE_NAME;
+    aspeed_load_vbootrom(soc, bios_name, errp);
+
     arm_load_kernel(ARM_CPU(first_cpu), machine, &ast2700fc_board_info);
 }
 
-- 
2.43.0

Re: [PATCH v2 7/7] hw/arm/aspeed_ast27x0-fc: Add VBOOTROM support

[Cédric Le Goater]

On 9/24/25 07:56, Jamin Lin wrote:
> Introduces support for loading a vbootrom image into the dedicated vbootrom
> memory region in the AST2700 Full Core machine.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast27x0-fc.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index 2e6036b192..b2d963e1fe 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -62,6 +62,7 @@ static void ast2700fc_ca35_init(MachineState *machine)
>       Ast2700FCState *s = AST2700A1FC(machine);
>       AspeedSoCState *soc;
>       AspeedSoCClass *sc;
> +    const char *bios_name = NULL;
>       BlockBackend *fmc0 = NULL;
>       DeviceState *dev = NULL;
>       Error **errp = NULL;
> @@ -133,6 +134,10 @@ static void ast2700fc_ca35_init(MachineState *machine)
>           }
>       }
>   
> +    /* VBOOTROM */
> +    bios_name = machine->firmware ?: VBOOTROM_FILE_NAME;
> +    aspeed_load_vbootrom(soc, bios_name, errp);
> +
>       arm_load_kernel(ARM_CPU(first_cpu), machine, &ast2700fc_board_info);
>   }
>   


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.