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

[Andrew Jones]

On Thu, May 25, 2023 at 11:03:52AM -0700, Andrea Bolognani wrote:
> On Thu, May 25, 2023 at 10:18:00PM +0530, Sunil V L 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 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.
> 
> Success! \o/
> 
> With these patches applied, libvirt built from the master branch,
> edk2 built from your branch and a JSON firmware descriptor for it
> installed (attached), it's finally possible to boot an unmodified
> openSUSE Tumbleweed RISC-V disk image by simply including
> 
>   <os firmware='efi'>

Hi Andrea,

I'm a bit concerned that we don't also need to add some XML in order to
disable ACPI right now. RISC-V guest kernels will support ACPI in the
near future. Ideally a default libvirt VM using edk2 will also use ACPI.
Will there be a problem with changing that default later? If so, then
I'd change it now and continue burdening developers a bit longer by
requiring them to explicitly disable it.

Thanks,
drew