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

[Alistair Francis]

On Mon, May 8, 2023 at 9:45 PM Andrea Bolognani <abologna@redhat.com> wrote:
>
> On Mon, May 08, 2023 at 04:53:46PM +0530, Sunil V L wrote:
> > On Mon, May 08, 2023 at 03:00:02AM -0700, Andrea Bolognani wrote:
> > > I think that it's more important to align with other architectures.

That's true, ideally we want to match what people are already doing.

> > >
> > > The number of people currently running edk2 on RISC-V is probably
> > > vanishingly small, and in my opinion requiring them to tweak their
> > > command lines a bit is a fair price to pay to avoid having to carry a
> > > subtle difference between architectures for years to come.
> >
> > It is not just tweaking the command line. The current EDK2 will not work
> > anymore if code is moved to plfash 0 since EDK2 assumed its entry point
> > is in pflash1. I agree there may not be too many users but if we have
> > to align with other archs, there will be combinations of qemu and
> > edk2 versions which won't work.
>
> Right.
>
> > > With that in mind, my preference would be to go back to v1.
> >
> > Thanks!. If this is the preference,  we can request people to use proper
> > versions of EDK2 with different qemu versions.
>
> Yeah, in the (not so) long run this will just not matter, as the
> versions of edk2 and QEMU available to people will all implement the
> new behavior. Better to optimize for the long future ahead of us
> rather than causing ongoing pain for the sake of the few users of a
> work-in-progress board.
>
> > > Taking a step back, what is even the use case for having M-mode code
> > > in pflash0? If you want to use an M-mode firmware, can't you just use
> > > -bios instead? In other words, can we change the behavior so that
> > > pflash being present always mean loading S-mode firmware off it?

It was originally added to support Oreboot (the Rust version of
Coreboot). The idea was that Oreboot (ROM) would be in flash and then
go from there.

It also applies to other ROM code that a user might want to test that
runs before OpenSBI.

> >
> > TBH, I don't know. I am sure Alistair would know since it was added in
> > https://github.com/qemu/qemu/commit/1c20d3ff6004b600336c52cbef9f134fad3ccd94
> > I don't think opensbi can be launched from pflash. So, it may be some
> > other use case which I am now aware of.
> >
> > I will be happy if this can be avoided by using -bios.
>
> The actual commit would be [1], from late 2019. Things might have
> changed in the intervening ~3.5 years. Let's wait to hear from
> Alistair :)

Overall for this patch I don't feel strongly about following what ARM
does or continuing with what we already have. I would prefer to match
other archs if we can though.

Also, either way we should update the documentation in
docs/system/riscv/virt.rst to describe what happens.

Alistair

>
>
> [1] 
> https://github.com/qemu/qemu/commit/2738b3b555efaf206b814677966e8e3510c64a8a
> --
> Andrea Bolognani / Red Hat / Virtualization
>
>