Re: [Qemu-devel] [PATCH v2 10/11] aspeed: use first FMC flash as a boot ROM

[Peter Maydell]

On 9 January 2017 at 16:24, Cédric Le Goater <address@hidden> wrote:
> Create a ROM region, using the default size of the mapping window for
> the CE0 FMC flash module, and fill it with the flash content.
>
> This is a little hacky but until we can boot from a MMIO region, it
> seems difficult to do anything else.
>
> Signed-off-by: Cédric Le Goater <address@hidden>
> Reviewed-by: Joel Stanley <address@hidden>
> Reviewed-by: Andrew Jeffery <address@hidden>
> ---
>  hw/arm/aspeed.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 40c13838fb2d..a92c2f1c362b 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -20,6 +20,8 @@
>  #include "qemu/log.h"
>  #include "sysemu/block-backend.h"
>  #include "sysemu/blockdev.h"
> +#include "hw/loader.h"
> +#include "qemu/error-report.h"
>
>  static struct arm_boot_info aspeed_board_binfo = {
>      .board_id = -1, /* device-tree-only board */
> @@ -104,6 +106,28 @@ static const AspeedBoardConfig aspeed_boards[] = {
>      },
>  };
>
> +#define FIRMWARE_ADDR 0x0
> +
> +static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
> +                           Error **errp)
> +{
> +    BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> +    uint8_t *storage;
> +
> +    if (rom_size > blk_getlength(blk)) {
> +        rom_size = blk_getlength(blk);
> +    }
> +
> +    storage = g_new0(uint8_t, rom_size);

Hi -- coverity points out that you're not checking for the
case where blk_getlength() returns negative. (This can happen
for the case where the BlockBackend represents an insertable
device like a floppy or cdrom with no medium present.)

Since for aspeed this dinfo always represents a flash device,
we know this shouldn't happen, so you can just make it a fatal
error if blk_getlength() doesn't return what you're expecting.

> +    if (blk_pread(blk, 0, storage, rom_size) < 0) {
> +        error_setg(errp, "failed to read the initial flash content");
> +        return;
> +    }
> +
> +    rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
> +    g_free(storage);
> +}

thanks
-- PMM