Re: [PATCH v2] hw/arm/boot: Report error msg if loading elf/dtb failed

[duchangbin]

On Mon, Sep 02, 2024 at 09:55:19PM +0200, Philippe Mathieu-Daudé wrote:
> Hi Changbin,
> 
> On 30/8/24 12:53, Changbin Du via wrote:
> > Print errors before exit. Do not exit silently.
> > 
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > 
> > ---
> > v2: remove msg for arm_load_dtb.
> > ---
> >   hw/arm/boot.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> > index d480a7da02cf..e15bf097a559 100644
> > --- a/hw/arm/boot.c
> > +++ b/hw/arm/boot.c
> > @@ -839,6 +839,7 @@ static ssize_t arm_load_elf(struct arm_boot_info *info, 
> > uint64_t *pentry,
> >                         1, data_swab, as);
> >       if (ret <= 0) {
> >           /* The header loaded but the image didn't */
> > +        error_report("could not load elf '%s'", info->kernel_filename);
> 
> "Could ..." (caps)
> 
> "hw/loader.h" is not well documented, but it seems load_elf*() returns:
> 
>   #define ELF_LOAD_FAILED       -1
>   #define ELF_LOAD_NOT_ELF      -2
>   #define ELF_LOAD_WRONG_ARCH   -3
>   #define ELF_LOAD_WRONG_ENDIAN -4
>   #define ELF_LOAD_TOO_BIG      -5
> 
> And we can display this error calling:
> 
>   const char *load_elf_strerror(ssize_t error);
> 
> So we can be more precise here using:
> 
>   error_report("Could not load elf '%s'", info->kernel_filename,
>                load_elf_strerror(ret));
> 
> >           exit(1);
> >       }
> 
> Better (but out of scope of this patch) could be to pass an Error *errp
> argument to the load_elf*() family of functions, and fill it with the
> appropriate error message.
>
Thanks for your suggestion. I changed it as below:
+        error_report("Couldn't load elf '%s': %s",
+                     info->kernel_filename, load_elf_strerror(ret));

$ qemu-system-aarch64 -M virt -kernel /work/linux/vmlinux
qemu-system-aarch64: Couldn't load elf '/work/linux/vmlinux': The image is from 
incompatible architecture

> Regards,
> 
> Phil.

-- 
Cheers,
Changbin Du