Re: [PATCH v3] hw/riscv: virt: Assume M-mode FW in pflash0 only when "-bios none"

[Sunil V L <sunilvl@ventanamicro.com>]

Hi Andrea,

On Wed, May 24, 2023 at 03:50:34PM +0000, Andrea Bolognani wrote:
> On Tue, May 23, 2023 at 03:58:05PM +0530, Sunil V L 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
> 
> I wanted to test this, both directly with QEMU and through libvirt,

Thank you very much for helping with this!

> but I'm hitting two roadblocks.
> 
> First off, the only RISC-V edk2 build readily accessible to me (from
> the edk2-riscv64 Fedora package) is configured to work off a R/W
> pflash1. You said that you have edk2 patches making R/O CODE pflash0
> and R/W VARS pflash1 ready. Any chance you could make either the
> build output, or the patches and some hints on how to build edk2
> after applying them, somewhere?
> 
Please build EDK2 using the branch
https://github.com/vlsunil/edk2/tree/separate_code_vars.

The instructions to build is in
https://github.com/vlsunil/riscv-uefi-edk2-docs/wiki/RISC-V-Qemu-Virt-support#build-edk2

However, now it will create two images for code and vars.

> Going further and testing libvirt integration. After hacking around
> other issues, I finally stumbled upon this error:
> 
>   qemu-system-riscv64: Property 'virt-machine.pflash0' not found
> 
> This is because a few years back libvirt has stopped using -drive
> if=pflash for configuring pflash devices, and these days it generates
> something along the lines of
> 
>   -blockdev '{"driver":"file","filename":"...","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
>   -blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
>   -machine virt,pflash0=libvirt-pflash0-format
> 
> instead, which both aarch64/virt and x86_64/q35 machine types are
> perfectly happy with.
> 
> I've tried to patch hw/riscv/virt.c to get the above working, and
> almost managed to succeed :) but eventually my unfamiliarity with the
> internals of QEMU caught up with me. Can you please look into making
> this work?
> 
Thanks!. This needs some investigation. Let me look into supporting
this.

Thanks!
Sunil