* [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine
@ 2021-07-26 16:33 Peter Maydell
2021-07-26 23:08 ` Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Peter Maydell @ 2021-07-26 16:33 UTC (permalink / raw)
To: qemu-arm, qemu-devel
Cc: Leif Lindholm, Radoslaw Biernacki, Philippe Mathieu-Daudé,
Andrew Baumann
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(-)
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine
2021-07-26 16:33 [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine Peter Maydell
@ 2021-07-26 23:08 ` Richard Henderson
2021-07-27 7:58 ` Philippe Mathieu-Daudé
2021-08-03 14:29 ` Leif Lindholm
2 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2021-07-26 23:08 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel
Cc: Leif Lindholm, Philippe Mathieu-Daudé,
Andrew Baumann, Radoslaw Biernacki
On 7/26/21 6:33 AM, 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: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine
2021-07-26 16:33 [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine Peter Maydell
2021-07-26 23:08 ` Richard Henderson
@ 2021-07-27 7:58 ` Philippe Mathieu-Daudé
2021-08-03 14:29 ` Leif Lindholm
2 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-27 7:58 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel
Cc: Leif Lindholm, Andrew Baumann, Radoslaw Biernacki
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine
2021-07-26 16:33 [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine Peter Maydell
2021-07-26 23:08 ` Richard Henderson
2021-07-27 7:58 ` Philippe Mathieu-Daudé
@ 2021-08-03 14:29 ` Leif Lindholm
2021-08-03 15:33 ` Peter Maydell
2 siblings, 1 reply; 5+ messages in thread
From: Leif Lindholm @ 2021-08-03 14:29 UTC (permalink / raw)
To: Peter Maydell
Cc: Philippe Mathieu-Daudé,
qemu-arm, qemu-devel, Andrew Baumann, Radoslaw Biernacki
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"
> ---
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine
2021-08-03 14:29 ` Leif Lindholm
@ 2021-08-03 15:33 ` Peter Maydell
0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2021-08-03 15:33 UTC (permalink / raw)
To: Leif Lindholm
Cc: Philippe Mathieu-Daudé,
qemu-arm, QEMU Developers, Andrew Baumann, Radoslaw Biernacki
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-03 15:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-26 16:33 [PATCH for-6.1] hw/arm/boot: Report error if there is no fw_cfg device in the machine Peter Maydell
2021-07-26 23:08 ` Richard Henderson
2021-07-27 7:58 ` Philippe Mathieu-Daudé
2021-08-03 14:29 ` Leif Lindholm
2021-08-03 15:33 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).