[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: |
Peter Maydell |
Subject: |
Re: [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine |
Date: |
Tue, 3 Aug 2021 16:33:29 +0100 |
On Tue, 3 Aug 2021 at 15:29, Leif Lindholm <leif@nuviainc.com> wrote:
>
> 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.maydell@linaro.org>
>
> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
>
> However, the subject line threw me slightly. How about:?
> "Report error if trying to load kernel with no fw_cfg"
Yeah, in retrospect that would have been a better subject.
However, the commit is in master already (dae257394ae5) so
it is what it is :-/
thanks
-- PMM