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