On Tue, May 23, 2023 at 8:29 PM Sunil V L <suni...@ventanamicro.com> wrote: > > Currently, virt machine supports two pflash instances each with > 32MB size. However, the first pflash is always assumed to > contain M-mode firmware and reset vector is set to this if > enabled. Hence, for S-mode payloads like EDK2, only one pflash > instance is available for use. This means both code and NV variables > of EDK2 will need to use the same pflash. > > The OS distros keep the EDK2 FW code as readonly. When non-volatile > variables also need to share the same pflash, it is not possible > to keep it as readonly since variables need write access. > > To resolve this issue, the code and NV variables need to be separated. > But in that case we need an extra flash. Hence, modify the convention > such that pflash0 will contain the M-mode FW only when "-bios none" > option is used. Otherwise, pflash0 will contain the S-mode payload FW. > This enables both pflash instances available for EDK2 use. > > Example usage: > 1) pflash0 containing M-mode FW > qemu-system-riscv64 -bios none -pflash <mmode_fw> -machine virt > or > qemu-system-riscv64 -bios none \ > -drive file=<mmode_fw>,if=pflash,format=raw,unit=0 -machine virt > > 2) pflash0 containing S-mode payload like EDK2 > qemu-system-riscv64 -pflash <smode_fw_code> -pflash <smode_vars> -machine > virt > or > qemu-system-riscv64 -bios <opensbi_fw> \ > -pflash <smode_fw_code> \ > -pflash <smode_vars> \ > -machine virt > or > qemu-system-riscv64 -bios <opensbi_fw> \ > -drive file=<smode_fw_code>,if=pflash,format=raw,unit=0,readonly=on \ > -drive file=<smode_fw_vars>,if=pflash,format=raw,unit=1 \ > -machine virt > > Signed-off-by: Sunil V L <suni...@ventanamicro.com> > Reported-by: Heinrich Schuchardt <xypron.g...@gmx.de>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> Alistair > --- > > The issue is reported at > https://salsa.debian.org/qemu-team/edk2/-/commit/c345655a0149f64c5020bfc1e53c619ce60587f6 > > The patch is based on Alistair's riscv-to-apply.next branch. > > Changes since v2: > 1) Reverted v2 changes and used v1 approach so that pflash0 can be > used > for code and pflash1 for variable store. > 2) Rebased to latest riscv-to-apply.next branch. > 3) Added documentation for pflash usage. > > Changes since v1: > 1) Simplified the fix such that it doesn't break current EDK2. > > docs/system/riscv/virt.rst | 33 ++++++++++++++++++++++++ > hw/riscv/virt.c | 51 ++++++++++++++------------------------ > 2 files changed, 52 insertions(+), 32 deletions(-) > > diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst > index 4b16e41d7f..4ac0d38fdf 100644 > --- a/docs/system/riscv/virt.rst > +++ b/docs/system/riscv/virt.rst > @@ -53,6 +53,39 @@ with the default OpenSBI firmware image as the -bios. It > also supports > the recommended RISC-V bootflow: U-Boot SPL (M-mode) loads OpenSBI fw_dynamic > firmware and U-Boot proper (S-mode), using the standard -bios functionality. > > +Using flash devices > +------------------- > + > +The first flash device (pflash0) can contain either ROM code like Oreboot > +or S-mode payload firmware code like EDK2. If the pflash0 contains the > +ROM code, -bios should be set to none. Otherwise, pflash0 is assumed to > +contain S-mode payload code. > + > +Firmware images used for pflash should be of size 32M. > + > +To boot as ROM code like Oreboot: > + > +.. code-block:: bash > + > + $ qemu-system-riscv64 -bios none -pflash <rom_code_image> \ > + ... other args .... > + > +To boot as S-mode payload like EDK2: > + > +.. code-block:: bash > + > + $ qemu-system-riscv64 -pflash <s-mode_fw_code> -pflash <smode_fw_vars> \ > + ... other args .... > + > +To boot as read-only S-mode payload like EDK2: > + > +.. code-block:: bash > + > + $ qemu-system-riscv64 -bios <opensbi_fw> \ > + -drive file=<smode_fw_code>,if=pflash,format=raw,unit=0,readonly=on \ > + -drive file=<smode_fw_vars>,if=pflash,format=raw,unit=1 \ > + ... other args .... > + > Machine-specific options > ------------------------ > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index 4e3efbee16..1187a60d6e 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -1245,7 +1245,7 @@ static void virt_machine_done(Notifier *notifier, void > *data) > target_ulong firmware_end_addr, kernel_start_addr; > const char *firmware_name = riscv_default_firmware_name(&s->soc[0]); > uint32_t fdt_load_addr; > - uint64_t kernel_entry; > + uint64_t kernel_entry = 0; > > /* > * Only direct boot kernel is currently supported for KVM VM, > @@ -1266,42 +1266,29 @@ static void virt_machine_done(Notifier *notifier, > void *data) > firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name, > start_addr, NULL); > > - if (drive_get(IF_PFLASH, 0, 1)) { > - /* > - * S-mode FW like EDK2 will be kept in second plash (unit 1). > - * When both kernel, initrd and pflash options are provided in the > - * command line, the kernel and initrd will be copied to the fw_cfg > - * table and opensbi will jump to the flash address which is the > - * entry point of S-mode FW. It is the job of the S-mode FW to load > - * the kernel and initrd using fw_cfg table. > - * > - * If only pflash is given but not -kernel, then it is the job of > - * of the S-mode firmware to locate and load the kernel. > - * In either case, the next_addr for opensbi will be the flash > address. > - */ > - riscv_setup_firmware_boot(machine); > - kernel_entry = virt_memmap[VIRT_FLASH].base + > - virt_memmap[VIRT_FLASH].size / 2; > - } else if (machine->kernel_filename) { > + if (drive_get(IF_PFLASH, 0, 0)) { > + if (machine->firmware && !strcmp(machine->firmware, "none")) { > + /* > + * Pflash was supplied but bios is none, let's overwrite the > + * address we jump to after reset to the base of the flash. > + */ > + start_addr = virt_memmap[VIRT_FLASH].base; > + } else { > + /* > + * Pflash was supplied but bios is not none. In this case, > + * base of the flash would contain S-mode payload. > + */ > + riscv_setup_firmware_boot(machine); > + kernel_entry = virt_memmap[VIRT_FLASH].base; > + } > + } > + > + if (machine->kernel_filename && !kernel_entry) { > kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0], > firmware_end_addr); > > kernel_entry = riscv_load_kernel(machine, &s->soc[0], > kernel_start_addr, true, NULL); > - } else { > - /* > - * If dynamic firmware is used, it doesn't know where is the next mode > - * if kernel argument is not set. > - */ > - kernel_entry = 0; > - } > - > - if (drive_get(IF_PFLASH, 0, 0)) { > - /* > - * Pflash was supplied, let's overwrite the address we jump to after > - * reset to the base of the flash. > - */ > - start_addr = virt_memmap[VIRT_FLASH].base; > } > > fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base, > -- > 2.34.1 > >