Re: [PATCH v5 0/3] hw/riscv/virt: pflash improvements

From: Anup Patel <apatel@ventanamicro.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>,
	 Andrea Bolognani <abologna@redhat.com>
Subject: Re: [PATCH v5 0/3] hw/riscv/virt: pflash improvements
Date: Wed, 31 May 2023 10:46:17 +0530	[thread overview]
Message-ID: <CAK9=C2VRF-QobT_-qObOAnjqk8haxOUkmnsPCtCKuRk3pL+jPw@mail.gmail.com> (raw)
In-Reply-To: <20230526121006.76388-1-sunilvl@ventanamicro.com>

On Fri, May 26, 2023 at 5:41 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> This series improves the pflash usage in RISC-V virt machine with solutions to
> below issues.
>
> 1) Currently the first pflash is reserved for ROM/M-mode firmware code. But S-mode
> payload firmware like EDK2 need both pflash devices to have separate code and variable
> store so that OS distros can keep the FW code as read-only.
>
> The issue is reported at
> https://salsa.debian.org/qemu-team/edk2/-/commit/c345655a0149f64c5020bfc1e53c619ce60587f6
>
> 2) The latest way of using pflash devices in other architectures and libvirt
> is by using -blockdev and machine options. However, currently this method is
> not working in RISC-V.
>
> With above issues fixed, added documentation on how to use pflash devices
> in RISC-V virt machine.
>
> This patch series is based on Alistair's riscv-to-apply.next branch.
>
> Changes since v4:
>         1) Updated patch 2 to avoid accessing private field as per feedback from Philippe.
>         2) Updated documentation patch to add read-only for ROM usage.
>         3) Rebased to latest riscv-to-apply.next branch and updated tags.
>
> Changes since v3:
>         1) Converted single patch to a series with a cover letter since there are
>            multiple patches now.
>         2) Added a new patch to enable pflash usage via -blockdev option.
>         3) Separated the documentation change into new patch and updated the
>            documentation to mention only -blockdev option which seems to be the
>            recommended way of using pflash.
>
> 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.
>
> Sunil V L (3):
>   hw/riscv: virt: Assume M-mode FW in pflash0 only when "-bios none"
>   riscv/virt: Support using pflash via -blockdev option
>   docs/system: riscv: Add pflash usage details

In case of KVM guests, there is no M-mode so pflash0 will always
contain S-mode FW.

I suggest improving this series to consider KVM guests as well
such that the same EDK2 S-mode works for KVM and TCG guests.

Regards,
Anup

>
>  docs/system/riscv/virt.rst | 29 ++++++++++++++++++++
>  hw/riscv/virt.c            | 56 +++++++++++++++-----------------------
>  2 files changed, 51 insertions(+), 34 deletions(-)
>
> --
> 2.34.1
>
>