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

From: Andrea Bolognani <abologna@redhat.com>
To: Sunil V L <sunilvl@ventanamicro.com>
Cc: qemu-riscv@nongnu.org, qemu-devel@nongnu.org,
	 Palmer Dabbelt <palmer@dabbelt.com>,
	Alistair Francis <alistair.francis@wdc.com>,
	 Bin Meng <bin.meng@windriver.com>,
	Weiwei Li <liweiwei@iscas.ac.cn>,
	 Daniel Henrique Barboza <dbarboza@ventanamicro.com>,
	Liu Zhiwei <zhiwei_liu@linux.alibaba.com>,
	 Heinrich Schuchardt <xypron.glpk@gmx.de>
Subject: Re: [PATCH v3] hw/riscv: virt: Assume M-mode FW in pflash0 only when "-bios none"
Date: Wed, 24 May 2023 15:50:34 +0000	[thread overview]
Message-ID: <CABJz62MQ5uRHUP4LLKt=AxTf-Sz0aTEOLWDz=9ftK=H3tZ9JUg@mail.gmail.com> (raw)
In-Reply-To: <20230523102805.100160-1-sunilvl@ventanamicro.com>

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,
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?

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!

-- 
Andrea Bolognani / Red Hat / Virtualization