qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine
@ 2025-09-25  5:05 Jamin Lin via
  2025-09-25  5:05 ` [PATCH v3 1/8] hw/arm/aspeed: Move aspeed_board_init_flashes() to common SoC code Jamin Lin via
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Jamin Lin via @ 2025-09-25  5:05 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

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.

v3
  1. Drop dead return checks.
  2. Make sub-init functions return bool with errp.

Dependencies

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

Jamin Lin (8):
  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: Drop dead return checks
  hw/arm/aspeed_ast27x0-fc: Make sub-init functions return bool with
    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  |  96 +++++++++++++++++----------------
 hw/arm/aspeed_soc_common.c  |  96 +++++++++++++++++++++++++++++++++
 4 files changed, 159 insertions(+), 146 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/8] hw/arm/aspeed: Move aspeed_board_init_flashes() to common SoC code
  2025-09-25  5:05 [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Jamin Lin via
@ 2025-09-25  5:05 ` Jamin Lin via
  2025-09-25  5:05 ` [PATCH v3 2/8] hw/arm/aspeed: Move write_boot_rom " Jamin Lin via
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Jamin Lin via @ 2025-09-25  5:05 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

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>
---
 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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/8] hw/arm/aspeed: Move write_boot_rom to common SoC code
  2025-09-25  5:05 [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Jamin Lin via
  2025-09-25  5:05 ` [PATCH v3 1/8] hw/arm/aspeed: Move aspeed_board_init_flashes() to common SoC code Jamin Lin via
@ 2025-09-25  5:05 ` Jamin Lin via
  2025-09-25  5:05 ` [PATCH v3 3/8] hw/arm/aspeed: Move aspeed_install_boot_rom " Jamin Lin via
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Jamin Lin via @ 2025-09-25  5:05 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

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>
---
 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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 3/8] hw/arm/aspeed: Move aspeed_install_boot_rom to common SoC code
  2025-09-25  5:05 [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Jamin Lin via
  2025-09-25  5:05 ` [PATCH v3 1/8] hw/arm/aspeed: Move aspeed_board_init_flashes() to common SoC code Jamin Lin via
  2025-09-25  5:05 ` [PATCH v3 2/8] hw/arm/aspeed: Move write_boot_rom " Jamin Lin via
@ 2025-09-25  5:05 ` Jamin Lin via
  2025-09-25  5:05 ` [PATCH v3 4/8] hw/arm/aspeed: Move aspeed_load_vbootrom " Jamin Lin via
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Jamin Lin via @ 2025-09-25  5:05 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

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>
---
 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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 4/8] hw/arm/aspeed: Move aspeed_load_vbootrom to common SoC code
  2025-09-25  5:05 [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Jamin Lin via
                   ` (2 preceding siblings ...)
  2025-09-25  5:05 ` [PATCH v3 3/8] hw/arm/aspeed: Move aspeed_install_boot_rom " Jamin Lin via
@ 2025-09-25  5:05 ` Jamin Lin via
  2025-09-25  5:05 ` [PATCH v3 5/8] hw/arm/aspeed_ast27x0-fc: Drop dead return checks Jamin Lin via
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Jamin Lin via @ 2025-09-25  5:05 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

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>
---
 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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 5/8] hw/arm/aspeed_ast27x0-fc: Drop dead return checks
  2025-09-25  5:05 [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Jamin Lin via
                   ` (3 preceding siblings ...)
  2025-09-25  5:05 ` [PATCH v3 4/8] hw/arm/aspeed: Move aspeed_load_vbootrom " Jamin Lin via
@ 2025-09-25  5:05 ` Jamin Lin via
  2025-09-25 16:10   ` Cédric Le Goater
  2025-09-25  5:05 ` [PATCH v3 6/8] hw/arm/aspeed_ast27x0-fc: Make sub-init functions return bool with errp Jamin Lin via
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Jamin Lin via @ 2025-09-25  5:05 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

1. 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.
2. 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.
https://patchwork.kernel.org/project/qemu-devel/patch/20250717034054.1903991-3-jamin_lin@aspeedtech.com/#26540626

No functional change.

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

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index 7087be4288..ebf3784df5 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -74,19 +74,12 @@ static void ast2700fc_ca35_init(MachineState *machine)
                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
         return;
     }
-    if (!object_property_set_link(OBJECT(&s->ca35), "memory",
-                                  OBJECT(&s->ca35_memory),
-                                  &error_abort)) {
-        return;
-    };
-    if (!object_property_set_link(OBJECT(&s->ca35), "dram",
-                                  OBJECT(&s->ca35_dram), &error_abort)) {
-        return;
-    }
-    if (!object_property_set_int(OBJECT(&s->ca35), "ram-size",
-                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
-        return;
-    }
+    object_property_set_link(OBJECT(&s->ca35), "memory",
+                             OBJECT(&s->ca35_memory), &error_abort);
+    object_property_set_link(OBJECT(&s->ca35), "dram", OBJECT(&s->ca35_dram),
+                             &error_abort);
+    object_property_set_int(OBJECT(&s->ca35), "ram-size",
+                            AST2700FC_BMC_RAM_SIZE, &error_abort);
 
     for (int i = 0; i < sc->macs_num; i++) {
         if (!qemu_configure_nic_device(DEVICE(&soc->ftgmac100[i]),
@@ -94,14 +87,10 @@ static void ast2700fc_ca35_init(MachineState *machine)
             break;
         }
     }
-    if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
-                                 AST2700FC_HW_STRAP1, &error_abort)) {
-        return;
-    }
-    if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
-                                 AST2700FC_HW_STRAP2, &error_abort)) {
-        return;
-    }
+    object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
+                            AST2700FC_HW_STRAP1, &error_abort);
+    object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
+                            AST2700FC_HW_STRAP2, &error_abort);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART12, serial_hd(0));
     if (!qdev_realize(DEVICE(&s->ca35), NULL, &error_abort)) {
         return;
@@ -133,10 +122,8 @@ static void ast2700fc_ssp_init(MachineState *machine)
                        UINT64_MAX);
 
     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)) {
-        return;
-    }
+    object_property_set_link(OBJECT(&s->ssp), "memory",
+                             OBJECT(&s->ssp_memory), &error_abort);
 
     soc = ASPEED_SOC(&s->ssp);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART4, serial_hd(1));
@@ -157,10 +144,8 @@ static void ast2700fc_tsp_init(MachineState *machine)
                        UINT64_MAX);
 
     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)) {
-        return;
-    }
+    object_property_set_link(OBJECT(&s->tsp), "memory",
+                             OBJECT(&s->tsp_memory), &error_abort);
 
     soc = ASPEED_SOC(&s->tsp);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART7, serial_hd(2));
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 6/8] hw/arm/aspeed_ast27x0-fc: Make sub-init functions return bool with errp
  2025-09-25  5:05 [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Jamin Lin via
                   ` (4 preceding siblings ...)
  2025-09-25  5:05 ` [PATCH v3 5/8] hw/arm/aspeed_ast27x0-fc: Drop dead return checks Jamin Lin via
@ 2025-09-25  5:05 ` Jamin Lin via
  2025-09-25 16:10   ` Cédric Le Goater
  2025-09-25  5:05 ` [PATCH v3 7/8] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM Jamin Lin via
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Jamin Lin via @ 2025-09-25  5:05 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Refactor ast2700fc_ca35_init(), ast2700fc_ssp_init(), and ast2700fc_tsp_init()
to take an Error **errp parameter and return a bool.
Each function now reports failure through the error object and returns false.

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

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index ebf3784df5..2e16a0340a 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -56,7 +56,7 @@ struct Ast2700FCState {
 #define AST2700FC_FMC_MODEL "w25q01jvq"
 #define AST2700FC_SPI_MODEL "w25q512jv"
 
-static void ast2700fc_ca35_init(MachineState *machine)
+static bool ast2700fc_ca35_init(MachineState *machine, Error **errp)
 {
     Ast2700FCState *s = AST2700A1FC(machine);
     AspeedSoCState *soc;
@@ -71,8 +71,8 @@ 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)) {
-        return;
+                                AST2700FC_BMC_RAM_SIZE, errp)) {
+        return false;
     }
     object_property_set_link(OBJECT(&s->ca35), "memory",
                              OBJECT(&s->ca35_memory), &error_abort);
@@ -92,8 +92,8 @@ static void ast2700fc_ca35_init(MachineState *machine)
     object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
                             AST2700FC_HW_STRAP2, &error_abort);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART12, serial_hd(0));
-    if (!qdev_realize(DEVICE(&s->ca35), NULL, &error_abort)) {
-        return;
+    if (!qdev_realize(DEVICE(&s->ca35), NULL, errp)) {
+        return false;
     }
 
     /*
@@ -108,9 +108,11 @@ static void ast2700fc_ca35_init(MachineState *machine)
     ast2700fc_board_info.loader_start = sc->memmap[ASPEED_DEV_SDRAM];
 
     arm_load_kernel(ARM_CPU(first_cpu), machine, &ast2700fc_board_info);
+
+    return true;
 }
 
-static void ast2700fc_ssp_init(MachineState *machine)
+static bool ast2700fc_ssp_init(MachineState *machine, Error **errp)
 {
     AspeedSoCState *soc;
     Ast2700FCState *s = AST2700A1FC(machine);
@@ -127,12 +129,14 @@ static void ast2700fc_ssp_init(MachineState *machine)
 
     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)) {
-        return;
+    if (!qdev_realize(DEVICE(&s->ssp), NULL, errp)) {
+        return false;
     }
+
+    return true;
 }
 
-static void ast2700fc_tsp_init(MachineState *machine)
+static bool ast2700fc_tsp_init(MachineState *machine, Error **errp)
 {
     AspeedSoCState *soc;
     Ast2700FCState *s = AST2700A1FC(machine);
@@ -149,16 +153,18 @@ static void ast2700fc_tsp_init(MachineState *machine)
 
     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)) {
-        return;
+    if (!qdev_realize(DEVICE(&s->tsp), NULL, errp)) {
+        return false;
     }
+
+    return true;
 }
 
 static void ast2700fc_init(MachineState *machine)
 {
-    ast2700fc_ca35_init(machine);
-    ast2700fc_ssp_init(machine);
-    ast2700fc_tsp_init(machine);
+    ast2700fc_ca35_init(machine, &error_abort);
+    ast2700fc_ssp_init(machine, &error_abort);
+    ast2700fc_tsp_init(machine, &error_abort);
 }
 
 static void ast2700fc_class_init(ObjectClass *oc, const void *data)
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 7/8] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM
  2025-09-25  5:05 [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Jamin Lin via
                   ` (5 preceding siblings ...)
  2025-09-25  5:05 ` [PATCH v3 6/8] hw/arm/aspeed_ast27x0-fc: Make sub-init functions return bool with errp Jamin Lin via
@ 2025-09-25  5:05 ` Jamin Lin via
  2025-09-25 16:16   ` Cédric Le Goater
  2025-09-25  5:05 ` [PATCH v3 8/8] hw/arm/aspeed_ast27x0-fc: Add VBOOTROM support Jamin Lin via
  2025-09-29  5:41 ` [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Cédric Le Goater
  8 siblings, 1 reply; 14+ messages in thread
From: Jamin Lin via @ 2025-09-25  5:05 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

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, 12 insertions(+), 2 deletions(-)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index 2e16a0340a..57964e336c 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;
 
@@ -44,8 +45,6 @@ struct Ast2700FCState {
     Aspeed27x0SoCState ca35;
     Aspeed27x0SSPSoCState ssp;
     Aspeed27x0TSPSoCState tsp;
-
-    bool mmio_exec;
 };
 
 #define AST2700FC_BMC_RAM_SIZE (1 * GiB)
@@ -61,6 +60,9 @@ static bool ast2700fc_ca35_init(MachineState *machine, Error **errp)
     Ast2700FCState *s = AST2700A1FC(machine);
     AspeedSoCState *soc;
     AspeedSoCClass *sc;
+    BlockBackend *fmc0 = NULL;
+    DeviceState *dev = NULL;
+    uint64_t rom_size;
 
     object_initialize_child(OBJECT(s), "ca35", &s->ca35, "ast2700-a1");
     soc = ASPEED_SOC(&s->ca35);
@@ -107,6 +109,14 @@ static bool ast2700fc_ca35_init(MachineState *machine, Error **errp)
     ast2700fc_board_info.ram_size = machine->ram_size;
     ast2700fc_board_info.loader_start = sc->memmap[ASPEED_DEV_SDRAM];
 
+    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);
 
     return true;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 8/8] hw/arm/aspeed_ast27x0-fc: Add VBOOTROM support
  2025-09-25  5:05 [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Jamin Lin via
                   ` (6 preceding siblings ...)
  2025-09-25  5:05 ` [PATCH v3 7/8] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM Jamin Lin via
@ 2025-09-25  5:05 ` Jamin Lin via
  2025-09-29  5:41 ` [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Cédric Le Goater
  8 siblings, 0 replies; 14+ messages in thread
From: Jamin Lin via @ 2025-09-25  5:05 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

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>
Reviewed-by: Cédric Le Goater <clg@redhat.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 57964e336c..d7f0f3325d 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -60,6 +60,7 @@ static bool ast2700fc_ca35_init(MachineState *machine, Error **errp)
     Ast2700FCState *s = AST2700A1FC(machine);
     AspeedSoCState *soc;
     AspeedSoCClass *sc;
+    const char *bios_name = NULL;
     BlockBackend *fmc0 = NULL;
     DeviceState *dev = NULL;
     uint64_t rom_size;
@@ -117,6 +118,10 @@ static bool ast2700fc_ca35_init(MachineState *machine, Error **errp)
         aspeed_install_boot_rom(soc, fmc0, &s->ca35_boot_rom, rom_size);
     }
 
+    /* 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);
 
     return true;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 5/8] hw/arm/aspeed_ast27x0-fc: Drop dead return checks
  2025-09-25  5:05 ` [PATCH v3 5/8] hw/arm/aspeed_ast27x0-fc: Drop dead return checks Jamin Lin via
@ 2025-09-25 16:10   ` Cédric Le Goater
  0 siblings, 0 replies; 14+ messages in thread
From: Cédric Le Goater @ 2025-09-25 16:10 UTC (permalink / raw)
  To: Jamin Lin, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery,
	Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee

On 9/25/25 07:05, Jamin Lin wrote:
> 1. 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.
> 2. 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.
> https://patchwork.kernel.org/project/qemu-devel/patch/20250717034054.1903991-3-jamin_lin@aspeedtech.com/#26540626
> 
> 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_ast27x0-fc.c | 43 +++++++++++++-------------------------
>   1 file changed, 14 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index 7087be4288..ebf3784df5 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -74,19 +74,12 @@ static void ast2700fc_ca35_init(MachineState *machine)
>                                   AST2700FC_BMC_RAM_SIZE, &error_abort)) {
>           return;
>       }
> -    if (!object_property_set_link(OBJECT(&s->ca35), "memory",
> -                                  OBJECT(&s->ca35_memory),
> -                                  &error_abort)) {
> -        return;
> -    };
> -    if (!object_property_set_link(OBJECT(&s->ca35), "dram",
> -                                  OBJECT(&s->ca35_dram), &error_abort)) {
> -        return;
> -    }
> -    if (!object_property_set_int(OBJECT(&s->ca35), "ram-size",
> -                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->ca35), "memory",
> +                             OBJECT(&s->ca35_memory), &error_abort);
> +    object_property_set_link(OBJECT(&s->ca35), "dram", OBJECT(&s->ca35_dram),
> +                             &error_abort);
> +    object_property_set_int(OBJECT(&s->ca35), "ram-size",
> +                            AST2700FC_BMC_RAM_SIZE, &error_abort);
>   
>       for (int i = 0; i < sc->macs_num; i++) {
>           if (!qemu_configure_nic_device(DEVICE(&soc->ftgmac100[i]),
> @@ -94,14 +87,10 @@ static void ast2700fc_ca35_init(MachineState *machine)
>               break;
>           }
>       }
> -    if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
> -                                 AST2700FC_HW_STRAP1, &error_abort)) {
> -        return;
> -    }
> -    if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
> -                                 AST2700FC_HW_STRAP2, &error_abort)) {
> -        return;
> -    }
> +    object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
> +                            AST2700FC_HW_STRAP1, &error_abort);
> +    object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
> +                            AST2700FC_HW_STRAP2, &error_abort);
>       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART12, serial_hd(0));
>       if (!qdev_realize(DEVICE(&s->ca35), NULL, &error_abort)) {
>           return;
> @@ -133,10 +122,8 @@ static void ast2700fc_ssp_init(MachineState *machine)
>                          UINT64_MAX);
>   
>       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)) {
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->ssp), "memory",
> +                             OBJECT(&s->ssp_memory), &error_abort);
>   
>       soc = ASPEED_SOC(&s->ssp);
>       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART4, serial_hd(1));
> @@ -157,10 +144,8 @@ static void ast2700fc_tsp_init(MachineState *machine)
>                          UINT64_MAX);
>   
>       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)) {
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->tsp), "memory",
> +                             OBJECT(&s->tsp_memory), &error_abort);
>   
>       soc = ASPEED_SOC(&s->tsp);
>       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART7, serial_hd(2));



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 6/8] hw/arm/aspeed_ast27x0-fc: Make sub-init functions return bool with errp
  2025-09-25  5:05 ` [PATCH v3 6/8] hw/arm/aspeed_ast27x0-fc: Make sub-init functions return bool with errp Jamin Lin via
@ 2025-09-25 16:10   ` Cédric Le Goater
  0 siblings, 0 replies; 14+ messages in thread
From: Cédric Le Goater @ 2025-09-25 16:10 UTC (permalink / raw)
  To: Jamin Lin, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery,
	Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee

On 9/25/25 07:05, Jamin Lin wrote:
> Refactor ast2700fc_ca35_init(), ast2700fc_ssp_init(), and ast2700fc_tsp_init()
> to take an Error **errp parameter and return a bool.
> Each function now reports failure through the error object and returns false.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>



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

Thanks,

C.


> ---
>   hw/arm/aspeed_ast27x0-fc.c | 34 ++++++++++++++++++++--------------
>   1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index ebf3784df5..2e16a0340a 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -56,7 +56,7 @@ struct Ast2700FCState {
>   #define AST2700FC_FMC_MODEL "w25q01jvq"
>   #define AST2700FC_SPI_MODEL "w25q512jv"
>   
> -static void ast2700fc_ca35_init(MachineState *machine)
> +static bool ast2700fc_ca35_init(MachineState *machine, Error **errp)
>   {
>       Ast2700FCState *s = AST2700A1FC(machine);
>       AspeedSoCState *soc;
> @@ -71,8 +71,8 @@ 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)) {
> -        return;
> +                                AST2700FC_BMC_RAM_SIZE, errp)) {
> +        return false;
>       }
>       object_property_set_link(OBJECT(&s->ca35), "memory",
>                                OBJECT(&s->ca35_memory), &error_abort);
> @@ -92,8 +92,8 @@ static void ast2700fc_ca35_init(MachineState *machine)
>       object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
>                               AST2700FC_HW_STRAP2, &error_abort);
>       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART12, serial_hd(0));
> -    if (!qdev_realize(DEVICE(&s->ca35), NULL, &error_abort)) {
> -        return;
> +    if (!qdev_realize(DEVICE(&s->ca35), NULL, errp)) {
> +        return false;
>       }
>   
>       /*
> @@ -108,9 +108,11 @@ static void ast2700fc_ca35_init(MachineState *machine)
>       ast2700fc_board_info.loader_start = sc->memmap[ASPEED_DEV_SDRAM];
>   
>       arm_load_kernel(ARM_CPU(first_cpu), machine, &ast2700fc_board_info);
> +
> +    return true;
>   }
>   
> -static void ast2700fc_ssp_init(MachineState *machine)
> +static bool ast2700fc_ssp_init(MachineState *machine, Error **errp)
>   {
>       AspeedSoCState *soc;
>       Ast2700FCState *s = AST2700A1FC(machine);
> @@ -127,12 +129,14 @@ static void ast2700fc_ssp_init(MachineState *machine)
>   
>       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)) {
> -        return;
> +    if (!qdev_realize(DEVICE(&s->ssp), NULL, errp)) {
> +        return false;
>       }
> +
> +    return true;
>   }
>   
> -static void ast2700fc_tsp_init(MachineState *machine)
> +static bool ast2700fc_tsp_init(MachineState *machine, Error **errp)
>   {
>       AspeedSoCState *soc;
>       Ast2700FCState *s = AST2700A1FC(machine);
> @@ -149,16 +153,18 @@ static void ast2700fc_tsp_init(MachineState *machine)
>   
>       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)) {
> -        return;
> +    if (!qdev_realize(DEVICE(&s->tsp), NULL, errp)) {
> +        return false;
>       }
> +
> +    return true;
>   }
>   
>   static void ast2700fc_init(MachineState *machine)
>   {
> -    ast2700fc_ca35_init(machine);
> -    ast2700fc_ssp_init(machine);
> -    ast2700fc_tsp_init(machine);
> +    ast2700fc_ca35_init(machine, &error_abort);
> +    ast2700fc_ssp_init(machine, &error_abort);
> +    ast2700fc_tsp_init(machine, &error_abort);
>   }
>   
>   static void ast2700fc_class_init(ObjectClass *oc, const void *data)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 7/8] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM
  2025-09-25  5:05 ` [PATCH v3 7/8] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM Jamin Lin via
@ 2025-09-25 16:16   ` Cédric Le Goater
  0 siblings, 0 replies; 14+ messages in thread
From: Cédric Le Goater @ 2025-09-25 16:16 UTC (permalink / raw)
  To: Jamin Lin, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery,
	Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee

Hello Jamin,

On 9/25/25 07:05, 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.

Could you please rephrase this commit log with the explanations
provided in :

   https://lore.kernel.org/qemu-devel/SI2PR06MB50417B26704FD2FAD0BB8105FC1FA@SI2PR06MB5041.apcprd06.prod.outlook.com/

(without the vbootrom URLs)

No need to resend. I will update the commit log of this patch.


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

Thanks,

C.


> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast27x0-fc.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index 2e16a0340a..57964e336c 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;
>   
> @@ -44,8 +45,6 @@ struct Ast2700FCState {
>       Aspeed27x0SoCState ca35;
>       Aspeed27x0SSPSoCState ssp;
>       Aspeed27x0TSPSoCState tsp;
> -
> -    bool mmio_exec;
>   };
>   
>   #define AST2700FC_BMC_RAM_SIZE (1 * GiB)
> @@ -61,6 +60,9 @@ static bool ast2700fc_ca35_init(MachineState *machine, Error **errp)
>       Ast2700FCState *s = AST2700A1FC(machine);
>       AspeedSoCState *soc;
>       AspeedSoCClass *sc;
> +    BlockBackend *fmc0 = NULL;
> +    DeviceState *dev = NULL;
> +    uint64_t rom_size;
>   
>       object_initialize_child(OBJECT(s), "ca35", &s->ca35, "ast2700-a1");
>       soc = ASPEED_SOC(&s->ca35);
> @@ -107,6 +109,14 @@ static bool ast2700fc_ca35_init(MachineState *machine, Error **errp)
>       ast2700fc_board_info.ram_size = machine->ram_size;
>       ast2700fc_board_info.loader_start = sc->memmap[ASPEED_DEV_SDRAM];
>   
> +    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);
>   
>       return true;



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine
  2025-09-25  5:05 [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Jamin Lin via
                   ` (7 preceding siblings ...)
  2025-09-25  5:05 ` [PATCH v3 8/8] hw/arm/aspeed_ast27x0-fc: Add VBOOTROM support Jamin Lin via
@ 2025-09-29  5:41 ` Cédric Le Goater
  2025-10-01  2:26   ` Jamin Lin
  8 siblings, 1 reply; 14+ messages in thread
From: Cédric Le Goater @ 2025-09-29  5:41 UTC (permalink / raw)
  To: Jamin Lin, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery,
	Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee

Jamin

On 9/25/25 07:05, Jamin Lin wrote:
> 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.
> 
> v3
>    1. Drop dead return checks.
>    2. Make sub-init functions return bool with errp.
> 
> Dependencies
> 
> Based on https://github.com/legoater/qemu at the aspeed-next branch.
> 
> Jamin Lin (8):
>    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: Drop dead return checks
>    hw/arm/aspeed_ast27x0-fc: Make sub-init functions return bool with
>      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  |  96 +++++++++++++++++----------------
>   hw/arm/aspeed_soc_common.c  |  96 +++++++++++++++++++++++++++++++++
>   4 files changed, 159 insertions(+), 146 deletions(-)
> 

Applied 1-6 to aspeed-next. Waiting for a commit log update of patch 7.

Also, why isn't there a vbootrom functional test for ast2700fc machine ?

Thanks,

C.




^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine
  2025-09-29  5:41 ` [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Cédric Le Goater
@ 2025-10-01  2:26   ` Jamin Lin
  0 siblings, 0 replies; 14+ messages in thread
From: Jamin Lin @ 2025-10-01  2:26 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: Troy Lee

Hi Cédric

> From: Cédric Le Goater <clg@kaod.org>
> Subject: Re: [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine
> 
> Jamin
> 
> On 9/25/25 07:05, Jamin Lin wrote:
> > 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.
> >
> > v3
> >    1. Drop dead return checks.
> >    2. Make sub-init functions return bool with errp.
> >
> > Dependencies
> >
> > Based on https://github.com/legoater/qemu at the aspeed-next branch.
> >
> > Jamin Lin (8):
> >    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: Drop dead return checks
> >    hw/arm/aspeed_ast27x0-fc: Make sub-init functions return bool with
> >      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  |  96 +++++++++++++++++----------------
> >   hw/arm/aspeed_soc_common.c  |  96
> +++++++++++++++++++++++++++++++++
> >   4 files changed, 159 insertions(+), 146 deletions(-)
> >
> 
> Applied 1-6 to aspeed-next. Waiting for a commit log update of patch 7.
> 


Sorry, I may have misunderstood your comments on this patch:
https://patchwork.kernel.org/project/qemu-devel/patch/20250925050535.2657256-8-jamin_lin@aspeedtech.com/ 
Will reset patch 7 and 8 and create a new patch 9 for functional testing (v4).

Thanks,
Jamin

> Also, why isn't there a vbootrom functional test for ast2700fc machine ?
> 
> Thanks,
> 
> C.
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-10-01  2:29 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-25  5:05 [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Jamin Lin via
2025-09-25  5:05 ` [PATCH v3 1/8] hw/arm/aspeed: Move aspeed_board_init_flashes() to common SoC code Jamin Lin via
2025-09-25  5:05 ` [PATCH v3 2/8] hw/arm/aspeed: Move write_boot_rom " Jamin Lin via
2025-09-25  5:05 ` [PATCH v3 3/8] hw/arm/aspeed: Move aspeed_install_boot_rom " Jamin Lin via
2025-09-25  5:05 ` [PATCH v3 4/8] hw/arm/aspeed: Move aspeed_load_vbootrom " Jamin Lin via
2025-09-25  5:05 ` [PATCH v3 5/8] hw/arm/aspeed_ast27x0-fc: Drop dead return checks Jamin Lin via
2025-09-25 16:10   ` Cédric Le Goater
2025-09-25  5:05 ` [PATCH v3 6/8] hw/arm/aspeed_ast27x0-fc: Make sub-init functions return bool with errp Jamin Lin via
2025-09-25 16:10   ` Cédric Le Goater
2025-09-25  5:05 ` [PATCH v3 7/8] hw/arm/aspeed_ast27x0-fc: Map FMC0 flash contents into CA35 boot ROM Jamin Lin via
2025-09-25 16:16   ` Cédric Le Goater
2025-09-25  5:05 ` [PATCH v3 8/8] hw/arm/aspeed_ast27x0-fc: Add VBOOTROM support Jamin Lin via
2025-09-29  5:41 ` [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine Cédric Le Goater
2025-10-01  2:26   ` Jamin Lin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).