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

Jamin Lin via posted 7 patches 4 days, 10 hours ago
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>
There is a newer version of this series
[PATCH v2 4/7] hw/arm/aspeed: Move aspeed_load_vbootrom to common SoC code
Posted by Jamin Lin via 4 days, 10 hours ago
Move the vbootrom loader helper into common SoC code so it can be reused
by all ASPEED boards, and decouple the API from AspeedMachineState.

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

No functional change.

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

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


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


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

Thanks,

C.


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