Further customize the -bios and -kernel options behaviour for the
microchip-icicle-kit machine. If "-bios none -kernel filename" is
specified, then do not load a firmware and instead only load and start
the kernel image.
Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
hw/riscv/microchip_pfsoc.c | 57 ++++++++++++++++++++++++++------------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 844dc0545c..df902c8667 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -578,29 +578,45 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
}
/*
- * We follow the following table to select which payload we execute.
+ * We follow the following table to select which firmware we use.
*
- * -bios | -kernel | payload
- * -------+------------+--------
- * N | N | HSS
- * Y | don't care | HSS
- * N | Y | kernel
- *
- * This ensures backwards compatibility with how we used to expose -bios
- * to users but allows them to run through direct kernel booting as well.
+ * -bios | -kernel | firmware
+ * --------------+------------+--------
+ * none | N | error
+ * none | Y | kernel
+ * NULL, default | N | BIOS_FILENAME
+ * NULL, default | Y | RISCV64_BIOS_BIN
+ * other | don't care | other
*/
+ if (machine->firmware && !strcmp(machine->firmware, "none")) {
+ if (!machine->kernel_filename) {
+ error_report("for -bios none, a kernel is required");
+ exit(1);
+ }
- if (machine->kernel_filename) {
- firmware_name = RISCV64_BIOS_BIN;
- firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
+ firmware_name = NULL;
+ firmware_load_addr = RESET_VECTOR;
+ } else if (!machine->firmware || !strcmp(machine->firmware, "default")) {
+ if (machine->kernel_filename) {
+ firmware_name = RISCV64_BIOS_BIN;
+ firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
+ } else {
+ firmware_name = BIOS_FILENAME;
+ firmware_load_addr = RESET_VECTOR;
+ }
} else {
- firmware_name = BIOS_FILENAME;
+ firmware_name = machine->firmware;
firmware_load_addr = RESET_VECTOR;
}
- /* Load the firmware */
- firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name,
- &firmware_load_addr, NULL);
+ /* Load the firmware if necessary */
+ if (firmware_name) {
+ const char *filename = riscv_find_firmware(firmware_name, NULL);
+ firmware_end_addr = riscv_load_firmware(filename, &firmware_load_addr,
+ NULL);
+ } else {
+ firmware_end_addr = firmware_load_addr;
+ }
riscv_boot_info_init(&boot_info, &s->soc.u_cpus);
if (machine->kernel_filename) {
@@ -638,8 +654,15 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
fdt_load_addr = 0;
}
+ hwaddr start_addr;
+ if (firmware_name) {
+ start_addr = firmware_load_addr;
+ } else {
+ start_addr = kernel_entry;
+ }
+
/* Load the reset vector */
- riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, firmware_load_addr,
+ riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, start_addr,
memmap[MICROCHIP_PFSOC_ENVM_DATA].base,
memmap[MICROCHIP_PFSOC_ENVM_DATA].size,
kernel_entry, fdt_load_addr);
--
2.43.0Hi,
On 2/24/25 9:54 PM, Sebastian Huber wrote:
> Further customize the -bios and -kernel options behaviour for the
> microchip-icicle-kit machine. If "-bios none -kernel filename" is
> specified, then do not load a firmware and instead only load and start
> the kernel image.
>
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
> ---
> hw/riscv/microchip_pfsoc.c | 57 ++++++++++++++++++++++++++------------
> 1 file changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 844dc0545c..df902c8667 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -578,29 +578,45 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> }
>
> /*
> - * We follow the following table to select which payload we execute.
> + * We follow the following table to select which firmware we use.
> *
> - * -bios | -kernel | payload
> - * -------+------------+--------
> - * N | N | HSS
> - * Y | don't care | HSS
> - * N | Y | kernel
> - *
> - * This ensures backwards compatibility with how we used to expose -bios
> - * to users but allows them to run through direct kernel booting as well.
> + * -bios | -kernel | firmware
> + * --------------+------------+--------
> + * none | N | error
> + * none | Y | kernel
> + * NULL, default | N | BIOS_FILENAME
This change is breaking the following test:
---------
$ QTEST_QEMU_BINARY=./build/qemu-system-riscv64 ./build/tests/qtest/qom-test
(...)
# slow test /riscv64/qom/amd-microblaze-v-generic executed in 2.28 secs
# starting QEMU: exec ./build/qemu-system-riscv64 -qtest unix:/tmp/qtest-1361875.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-1361875.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine microchip-icicle-kit -accel qtest
**
ERROR:../hw/riscv/boot.c:164:riscv_load_firmware: assertion failed: (firmware_filename != NULL)
Bail out! ERROR:../hw/riscv/boot.c:164:riscv_load_firmware: assertion failed: (firmware_filename != NULL)
Broken pipe
---------
The reason is that, with the default machine settings (no -bios and no -kernel
options), firmware_name is now defaulted to BIOS_FILENAME (hss.bin). But we're not
distributing 'hss.bin' in pc-bios:
$ ls pc-bios/ | grep hss
$
Then, in the following code, 'filename' will be NULL and riscv_load_firmware() will
g_assert():
> + if (firmware_name) {
> + const char *filename = riscv_find_firmware(firmware_name, NULL);
> + firmware_end_addr = riscv_load_firmware(filename, &firmware_load_addr,
> + NULL);
> + }
Possible solutions:
- package hss.bin in QEMU so it can be used as a default firmware;
- redo the logic to allow the board to run (even if inactive) with absent
-bios and -kernel to allow QEMU model tests to run.
Thanks,
Daniel
> + * NULL, default | Y | RISCV64_BIOS_BIN
> + * other | don't care | other
> */
> + if (machine->firmware && !strcmp(machine->firmware, "none")) {
> + if (!machine->kernel_filename) {
> + error_report("for -bios none, a kernel is required");
> + exit(1);
> + }
>
> - if (machine->kernel_filename) {
> - firmware_name = RISCV64_BIOS_BIN;
> - firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> + firmware_name = NULL;
> + firmware_load_addr = RESET_VECTOR;
> + } else if (!machine->firmware || !strcmp(machine->firmware, "default")) {
> + if (machine->kernel_filename) {
> + firmware_name = RISCV64_BIOS_BIN;
> + firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> + } else {
> + firmware_name = BIOS_FILENAME;
> + firmware_load_addr = RESET_VECTOR;
> + }
> } else {
> - firmware_name = BIOS_FILENAME;
> + firmware_name = machine->firmware;
> firmware_load_addr = RESET_VECTOR;
> }
>
> - /* Load the firmware */
> - firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name,
> - &firmware_load_addr, NULL);
> + /* Load the firmware if necessary */
> + if (firmware_name) {
> + const char *filename = riscv_find_firmware(firmware_name, NULL);
> + firmware_end_addr = riscv_load_firmware(filename, &firmware_load_addr,
> + NULL);
> + } else {
> + firmware_end_addr = firmware_load_addr;
> + }
>
> riscv_boot_info_init(&boot_info, &s->soc.u_cpus);
> if (machine->kernel_filename) {
> @@ -638,8 +654,15 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> fdt_load_addr = 0;
> }
>
> + hwaddr start_addr;
> + if (firmware_name) {
> + start_addr = firmware_load_addr;
> + } else {
> + start_addr = kernel_entry;
> + }
> +
> /* Load the reset vector */
> - riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, firmware_load_addr,
> + riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, start_addr,
> memmap[MICROCHIP_PFSOC_ENVM_DATA].base,
> memmap[MICROCHIP_PFSOC_ENVM_DATA].size,
> kernel_entry, fdt_load_addr);
On Tue, Feb 25, 2025 at 10:55 AM Sebastian Huber
<sebastian.huber@embedded-brains.de> wrote:
>
> Further customize the -bios and -kernel options behaviour for the
> microchip-icicle-kit machine. If "-bios none -kernel filename" is
> specified, then do not load a firmware and instead only load and start
> the kernel image.
>
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/riscv/microchip_pfsoc.c | 57 ++++++++++++++++++++++++++------------
> 1 file changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 844dc0545c..df902c8667 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -578,29 +578,45 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> }
>
> /*
> - * We follow the following table to select which payload we execute.
> + * We follow the following table to select which firmware we use.
> *
> - * -bios | -kernel | payload
> - * -------+------------+--------
> - * N | N | HSS
> - * Y | don't care | HSS
> - * N | Y | kernel
> - *
> - * This ensures backwards compatibility with how we used to expose -bios
> - * to users but allows them to run through direct kernel booting as well.
> + * -bios | -kernel | firmware
> + * --------------+------------+--------
> + * none | N | error
> + * none | Y | kernel
> + * NULL, default | N | BIOS_FILENAME
> + * NULL, default | Y | RISCV64_BIOS_BIN
> + * other | don't care | other
> */
> + if (machine->firmware && !strcmp(machine->firmware, "none")) {
> + if (!machine->kernel_filename) {
> + error_report("for -bios none, a kernel is required");
> + exit(1);
> + }
>
> - if (machine->kernel_filename) {
> - firmware_name = RISCV64_BIOS_BIN;
> - firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> + firmware_name = NULL;
> + firmware_load_addr = RESET_VECTOR;
> + } else if (!machine->firmware || !strcmp(machine->firmware, "default")) {
> + if (machine->kernel_filename) {
> + firmware_name = RISCV64_BIOS_BIN;
> + firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> + } else {
> + firmware_name = BIOS_FILENAME;
> + firmware_load_addr = RESET_VECTOR;
> + }
> } else {
> - firmware_name = BIOS_FILENAME;
> + firmware_name = machine->firmware;
> firmware_load_addr = RESET_VECTOR;
> }
>
> - /* Load the firmware */
> - firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name,
> - &firmware_load_addr, NULL);
> + /* Load the firmware if necessary */
> + if (firmware_name) {
> + const char *filename = riscv_find_firmware(firmware_name, NULL);
> + firmware_end_addr = riscv_load_firmware(filename, &firmware_load_addr,
> + NULL);
> + } else {
> + firmware_end_addr = firmware_load_addr;
> + }
>
> riscv_boot_info_init(&boot_info, &s->soc.u_cpus);
> if (machine->kernel_filename) {
> @@ -638,8 +654,15 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> fdt_load_addr = 0;
> }
>
> + hwaddr start_addr;
> + if (firmware_name) {
> + start_addr = firmware_load_addr;
> + } else {
> + start_addr = kernel_entry;
> + }
> +
> /* Load the reset vector */
> - riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, firmware_load_addr,
> + riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, start_addr,
> memmap[MICROCHIP_PFSOC_ENVM_DATA].base,
> memmap[MICROCHIP_PFSOC_ENVM_DATA].size,
> kernel_entry, fdt_load_addr);
> --
> 2.43.0
>
© 2016 - 2026 Red Hat, Inc.