Re: [PATCH v2 02/12] aspeed: Introduce a boot_rom region at the machine level

[Joel Stanley]

On Wed, 7 Jun 2023 at 04:40, Cédric Le Goater <clg@kaod.org> wrote:
>
> This should also avoid Coverity to report a memory leak warning when
> the QEMU process exits. See CID 1508061.

Ok, so now our layout is this if booted with a mtd device (rainier-bmc):

address-space: memory
  0000000000000000-ffffffffffffffff (prio 0, i/o): system
    0000000000000000-000000000fffffff (prio 0, i/o): aspeed.spi_boot_container
      0000000000000000-0000000007ffffff (prio 1, rom): aspeed.boot_rom
      0000000000000000-0000000007ffffff (prio 0, i/o): alias
aspeed.spi_boot @aspeed.smc.flash.0 0000000000000000-0000000007ffffff


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


>
> Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/arm/aspeed.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index bfc2070bd2ed..76a1e7303de1 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -40,6 +40,7 @@ struct AspeedMachineState {
>      /* Public */
>
>      AspeedSoCState soc;
> +    MemoryRegion boot_rom;
>      bool mmio_exec;
>      char *fmc_model;
>      char *spi_model;
> @@ -275,15 +276,15 @@ static void write_boot_rom(BlockBackend *blk, hwaddr 
> addr, size_t rom_size,
>   * 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(AspeedSoCState *soc, BlockBackend *blk,
> +static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend 
> *blk,
>                                      uint64_t rom_size)
>  {
> -    MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
> +    AspeedSoCState *soc = &bmc->soc;
>
> -    memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom", rom_size,
> +    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,
> -                                        boot_rom, 1);
> +                                        &bmc->boot_rom, 1);
>      write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, &error_abort);
>  }
>
> @@ -431,8 +432,7 @@ static void aspeed_machine_init(MachineState *machine)
>
>          if (mtd0) {
>              uint64_t rom_size = memory_region_size(&bmc->soc.spi_boot);
> -            aspeed_install_boot_rom(&bmc->soc, blk_by_legacy_dinfo(mtd0),
> -                                    rom_size);
> +            aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(mtd0), 
> rom_size);
>          }
>      }
>
> --
> 2.40.1
>