On Mon, Jul 26, 2021 at 17:33:51 +0100, 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.mayd...@linaro.org>
Reviewed-by: Leif Lindholm <l...@nuviainc.com>
However, the subject line threw me slightly. How about:?
"Report error if trying to load kernel with no fw_cfg"
> ---
> 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(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index d7b059225e6..57efb61ee41 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -1243,6 +1243,15 @@ static void arm_setup_firmware_boot(ARMCPU *cpu,
> struct arm_boot_info *info)
> bool try_decompressing_kernel;
>
> fw_cfg = fw_cfg_find();
> +
> + if (!fw_cfg) {
> + error_report("This machine type does not support loading both "
> + "a guest firmware/BIOS image and a guest kernel at "
> + "the same time. You should change your QEMU command
> "
> + "line to specify one or the other, but not both.");
> + exit(1);
> + }
> +
> try_decompressing_kernel = arm_feature(&cpu->env,
> ARM_FEATURE_AARCH64);
>
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index 43c19b49234..c1629df6031 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -691,13 +691,6 @@ static void sbsa_ref_init(MachineState *machine)
>
> firmware_loaded = sbsa_firmware_init(sms, sysmem, secure_sysmem);
>
> - if (machine->kernel_filename && firmware_loaded) {
> - error_report("sbsa-ref: No fw_cfg device on this machine, "
> - "so -kernel option is not supported when firmware
> loaded, "
> - "please load OS from hard disk instead");
> - exit(1);
> - }
> -
> /*
> * This machine has EL3 enabled, external firmware should supply PSCI
> * implementation, so the QEMU's internal PSCI is disabled.
> --
> 2.20.1
>
