Hi,
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.hu...@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);
