[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg devi
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine |
Date: |
Tue, 27 Jul 2021 09:58:43 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
On 7/26/21 6:33 PM, Peter Maydell wrote:
> If the user provides both a BIOS/firmware image and also a guest
> kernel filename, arm_setup_firmware_boot() will pass the
> kernel image to the firmware via the fw_cfg device. However we
> weren't checking whether there really was a fw_cfg device present,
> and if there wasn't we would crash.
>
> This crash can be provoked with a command line such as
> qemu-system-aarch64 -M raspi3 -kernel /dev/null -bios /dev/null -display none
>
> It is currently only possible on the raspi3 machine, because unless
> the machine sets info->firmware_loaded we won't call
> arm_setup_firmware_boot(), and the only machines which set that are:
> * virt (has a fw-cfg device)
> * sbsa-ref (checks itself for kernel_filename && firmware_loaded)
> * raspi3 (crashes)
>
> But this is an unfortunate beartrap to leave for future machine
> model implementors, so we should handle this situation in boot.c.
>
> Check in arm_setup_firmware_boot() whether the fw-cfg device exists
> before trying to load files into it, and if it doesn't exist then
> exit with a hopefully helpful error message.
>
> Because we now handle this check in a machine-agnostic way, we
> can remove the check from sbsa-ref.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/503
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I didn't reuse exactly the same wording sbsa-ref used to have,
> because the bit about loading an OS from hard disk might not
> apply to all machine types.
> ---
> hw/arm/boot.c | 9 +++++++++
> hw/arm/sbsa-ref.c | 7 -------
> 2 files changed, 9 insertions(+), 7 deletions(-)
Thanks for fixing this.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>