Re: [PATCH] hw/arm/nseries: Check return value from load_image_targphys()

[Philippe Mathieu-Daudé]

On 11/3/20 12:49 PM, Peter Maydell wrote:
> The nseries machines have a codepath that allows them to load a
> secondary bootloader.  This code wasn't checking that the
> load_image_targphys() succeeded.  Check the return value and report
> the error to the user.
> 
> While we're in the vicinity, fix the comment style of the
> comment documenting what this image load is doing.
> 
> Fixes: Coverity CID 1192904
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/arm/nseries.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
> index 76fd7fe9854..d49852c60d7 100644
> --- a/hw/arm/nseries.c
> +++ b/hw/arm/nseries.c
> @@ -1380,7 +1380,8 @@ static void n8x0_init(MachineState *machine,
>          /* No, wait, better start at the ROM.  */
>          s->mpu->cpu->env.regs[15] = OMAP2_Q2_BASE + 0x400000;
>  
> -        /* This is intended for loading the `secondary.bin' program from
> +        /*
> +         * This is intended for loading the `secondary.bin' program from
>           * Nokia images (the NOLO bootloader).  The entry point seems
>           * to be at OMAP2_Q2_BASE + 0x400000.
>           *
> @@ -1388,9 +1389,15 @@ static void n8x0_init(MachineState *machine,
>           * for them the entry point needs to be set to OMAP2_SRAM_BASE.
>           *
>           * The code above is for loading the `zImage' file from Nokia
> -         * images.  */
> -        load_image_targphys(option_rom[0].name, OMAP2_Q2_BASE + 0x400000,
> -                            machine->ram_size - 0x400000);
> +         * images.
> +         */
> +        if (load_image_targphys(option_rom[0].name,
> +                                OMAP2_Q2_BASE + 0x400000,
> +                                machine->ram_size - 0x400000) < 0) {
> +            error_report("Failed to load secondary bootloader %s",
> +                         option_rom[0].name);
> +            exit(EXIT_FAILURE);
> +        }
>  
>          n800_setup_nolo_tags(nolo_tags);
>          cpu_physical_memory_write(OMAP2_SRAM_BASE, nolo_tags, 0x10000);
> 

What about the other cases?

$ git grep -E ^\\s+load_image_targphys
hw/alpha/dp264.c:163:            load_image_targphys(initrd_filename,
initrd_base,
hw/hppa/machine.c:301:            load_image_targphys(initrd_filename,
initrd_base, initrd_size);
hw/m68k/q800.c:388:            load_image_targphys(initrd_filename,
initrd_base,

Thanks,

Phil.