Re: [PATCH v2 03/12] aspeed: Use the boot_rom region of the fby35 machine

[Joel Stanley]

On Wed, 7 Jun 2023 at 04:40, Cédric Le Goater <clg@kaod.org> wrote:
>
> This change completes commits 5aa281d757 ("aspeed: Introduce a
> spi_boot region under the SoC") and 8b744a6a47 ("aspeed: Add a
> boot_rom overlap region in the SoC spi_boot container") which
> introduced a spi_boot container at the SoC level to map the boot rom
> region as an overlap.
>
> It also fixes a Coverity report (CID 1508061) for a memory leak
> warning when the QEMU process exits by using an bmc_boot_rom
> MemoryRegion available at the machine level.
>
> Cc: Peter Delevoryas <peter@pjd.dev>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Joel Stanley <joel@jms.id.au>

> ---
>  hw/arm/fby35.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
> index f4600c290b62..f2ff6c1abfd9 100644
> --- a/hw/arm/fby35.c
> +++ b/hw/arm/fby35.c
> @@ -70,8 +70,6 @@ static void fby35_bmc_write_boot_rom(DriveInfo *dinfo, 
> MemoryRegion *mr,
>
>  static void fby35_bmc_init(Fby35State *s)
>  {
> -    DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
> -
>      object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3");
>
>      memory_region_init(&s->bmc_memory, OBJECT(&s->bmc), "bmc-memory",
> @@ -95,18 +93,21 @@ static void fby35_bmc_init(Fby35State *s)
>      aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0);
>
>      /* Install first FMC flash content as a boot rom. */
> -    if (drive0) {
> -        AspeedSMCFlash *fl = &s->bmc.fmc.flashes[0];
> -        MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
> -        uint64_t size = memory_region_size(&fl->mmio);
> -
> -        if (!s->mmio_exec) {
> -            memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom",
> -                                   size, &error_abort);
> -            memory_region_add_subregion(&s->bmc_memory, 
> FBY35_BMC_FIRMWARE_ADDR,
> -                                        boot_rom);
> -            fby35_bmc_write_boot_rom(drive0, boot_rom, 
> FBY35_BMC_FIRMWARE_ADDR,
> -                                     size, &error_abort);
> +    if (!s->mmio_exec) {
> +        DriveInfo *mtd0 = drive_get(IF_MTD, 0, 0);
> +
> +        if (mtd0) {
> +            AspeedSoCState *bmc = &s->bmc;
> +            uint64_t rom_size = memory_region_size(&bmc->spi_boot);
> +
> +            memory_region_init_rom(&s->bmc_boot_rom, NULL, "aspeed.boot_rom",
> +                                   rom_size, &error_abort);
> +            memory_region_add_subregion_overlap(&bmc->spi_boot_container, 0,
> +                                                &s->bmc_boot_rom, 1);
> +
> +            fby35_bmc_write_boot_rom(mtd0, &s->bmc_boot_rom,
> +                                     FBY35_BMC_FIRMWARE_ADDR,
> +                                     rom_size, &error_abort);
>          }
>      }
>  }
> --
> 2.40.1
>