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

["Cédric Le Goater" <clg@kaod.org>]

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


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

Thanks,

C.


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