This series adds the support to boot S-mode FW like EDK2 from the flash. The
S-mode firmware should be kept in pflash unit 1.

When -kernel (and -initrd) option is also provided along with the flash,
the kernel (and initrd) will be loaded into fw_cfg table and opensbi will
branch to the flash address which will be the entry point of the S-mode
firmware. The S-mode FW then loads and launches the kernel.

When only -pflash option is provided in the command line, the kernel
will be located and loaded in the usual way by the S-mode firmware.

These patches are available in below branch.
https://github.com/vlsunil/qemu/tree/pflash_v2

The first two patches in this series are refactor patches.

These changes are tested with a WIP EDK2 port for virt machine. Below
are the instructions to build and test this feature.

1) Get EDK2 sources from below branches.
https://github.com/vlsunil/edk2/tree/virt_refactor_smode_v1
https://github.com/vlsunil/edk2-platforms/tree/virt_refactor_smode_v1

2) Build EDK2 for RISC-V
	export WORKSPACE=`pwd`
        export GCC5_RISCV64_PREFIX=riscv64-linux-gnu-
        export PACKAGES_PATH=$WORKSPACE/edk2:$WORKSPACE/edk2-platforms
        export EDK_TOOLS_PATH=$WORKSPACE/edk2/BaseTools
        source edk2/edksetup.sh
        make -C edk2/BaseTools clean
        make -C edk2/BaseTools
        make -C edk2/BaseTools/Source/C
        source edk2/edksetup.sh BaseTools
        build -a RISCV64  -p Platform/Qemu/RiscVVirt/RiscVVirt.dsc -t GCC5

3)Make the EDK2 image size to match with what qemu flash expects
truncate -s 32M Build/RiscVVirt/DEBUG_GCC5/FV/RISCV_VIRT.fd

4) Run
a) Boot to EFI shell (no -kernel / -initrd option)
qemu-system-riscv64  -nographic   -drive file=Build/RiscVVirt/DEBUG_GCC5/FV/RISCV_VIRT.fd,if=pflash,format=raw,unit=1  -machine virt -M 2G

b) With -kernel, -initrd and -pflash
qemu-system-riscv64  -nographic   -drive file=Build/RiscVVirt/DEBUG_GCC5/FV/RISCV_VIRT.fd,if=pflash,format=raw,unit=1  -machine virt -M 2G -kernel arch/riscv/boot/Image.gz -initrd rootfs.cpio 


Changes since V3:
	1) White space and comment edits
	2) Added RB tag

Changes since V2:
	1) Moved the doc comment to .h file

Changes since V1:
	1) Modified code to support the use case when both -kernel and -pflash are configured.
	2) Refactor patches added to help (1) above.
	3) Cover letter added with test instructions.


Sunil V L (3):
  hw/arm,loongarch: Move load_image_to_fw_cfg() to common location
  hw/riscv: virt: Move create_fw_cfg() prior to loading kernel
  hw/riscv: virt: Enable booting S-mode firmware from pflash

 hw/arm/boot.c             | 49 ---------------------------------------
 hw/loongarch/virt.c       | 33 --------------------------
 hw/nvram/fw_cfg.c         | 32 +++++++++++++++++++++++++
 hw/riscv/boot.c           | 29 +++++++++++++++++++++++
 hw/riscv/virt.c           | 32 ++++++++++++++++++-------
 include/hw/nvram/fw_cfg.h | 21 +++++++++++++++++
 include/hw/riscv/boot.h   |  1 +
 7 files changed, 107 insertions(+), 90 deletions(-)

-- 
2.25.1

  Hi,

> 3)Make the EDK2 image size to match with what qemu flash expects
> truncate -s 32M Build/RiscVVirt/DEBUG_GCC5/FV/RISCV_VIRT.fd

Hmm, we have that kind of padding on arm too (64M for code and 64M for
vars) and only a fraction of the space is actually used, which isn't
exactly ideal.  So not sure it is a good plan to repeat that on riscv.

Also: Do you have support for persistent efi variables?  If that is the
case then it makes sense to have separate pflash devices for code and
variable store.  First because you can map the code part read-only then,
and second because decoupling code + vars to separate files allows easy
firmware code updates without loosing the variable store.

take care,
  Gerd

Hi Gerd,

On Tue, Sep 06, 2022 at 12:41:28PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > 3)Make the EDK2 image size to match with what qemu flash expects
> > truncate -s 32M Build/RiscVVirt/DEBUG_GCC5/FV/RISCV_VIRT.fd
> 
> Hmm, we have that kind of padding on arm too (64M for code and 64M for
> vars) and only a fraction of the space is actually used, which isn't
> exactly ideal.  So not sure it is a good plan to repeat that on riscv.

Yeah.. but it looks like limitation from qemu flash emulation. Do you mean
this limitation exists for arm in general on real flash also?

> 
> Also: Do you have support for persistent efi variables?  If that is the
> case then it makes sense to have separate pflash devices for code and
> variable store.  First because you can map the code part read-only then,
> and second because decoupling code + vars to separate files allows easy
> firmware code updates without loosing the variable store.

Yes, we have persistent variables in my WIP branch. We can easily make it
to create variables as separate file in EDK2. But we will need to
enhance qemu virt machine to create more than 2 flash since the first
one is currently reserved for machine mode firmware. This is a
good input to enhance it in future.

Thanks!
Sunil

> 
> take care,
>   Gerd
>

On Tue, Sep 06, 2022 at 06:02:00PM +0530, Sunil V L wrote:
> Hi Gerd,
> 
> On Tue, Sep 06, 2022 at 12:41:28PM +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > 3)Make the EDK2 image size to match with what qemu flash expects
> > > truncate -s 32M Build/RiscVVirt/DEBUG_GCC5/FV/RISCV_VIRT.fd
> > 
> > Hmm, we have that kind of padding on arm too (64M for code and 64M for
> > vars) and only a fraction of the space is actually used, which isn't
> > exactly ideal.  So not sure it is a good plan to repeat that on riscv.
> 
> Yeah.. but it looks like limitation from qemu flash emulation. Do you mean
> this limitation exists for arm in general on real flash also?

Well, at least on x86 flash devices can have odd sizes.  I don't think
the qemu pflash emulation dictates anything here.

I think the underlying problem we actually have in qemu is that the
flash size indirectly dictates the memory layout.  We pack the flash
devices next to each other, on x86 downwards from 4G, on arm upwards
from zero, not sure what risc-v is dong here.

edk2 arm code expects the variable store being mapped at 64m.  So
QEMU_EFI.fd (which is actually 2M in size) gets padded to 64m, which
has the desired effect that the next flash device (the varstore) is
mapped at 64m.  But also has the side effect that we map 62m of zeros
into the guest address space ...

The vars file is padded to 64m for consistency with the code.  Not
padding the vars file should have no bad side effects I think, except
for live migration where the flash size change might cause
compatibility problems.

Not padding the code file needs some alternative way to specify the
memory layout, to make sure the vars flash continues to be mapped at
64m even when the code flash is smaller.  Cc'ed Pawel who investigates
that right now.

One possible option is to just hard-code the flash memory layout per
machine type or per architecture.  Another option would be to add
some way to configure that on the command line.

take care,
  Gerd

On Wed, Sep 07, 2022 at 09:10:37AM +0200, Gerd Hoffmann wrote:
> On Tue, Sep 06, 2022 at 06:02:00PM +0530, Sunil V L wrote:
> > Hi Gerd,
> > 
> > On Tue, Sep 06, 2022 at 12:41:28PM +0200, Gerd Hoffmann wrote:
> > >   Hi,
> > > 
> > > > 3)Make the EDK2 image size to match with what qemu flash expects
> > > > truncate -s 32M Build/RiscVVirt/DEBUG_GCC5/FV/RISCV_VIRT.fd
> > > 
> > > Hmm, we have that kind of padding on arm too (64M for code and 64M for
> > > vars) and only a fraction of the space is actually used, which isn't
> > > exactly ideal.  So not sure it is a good plan to repeat that on riscv.
> > 
> > Yeah.. but it looks like limitation from qemu flash emulation. Do you mean
> > this limitation exists for arm in general on real flash also?
> 
> Well, at least on x86 flash devices can have odd sizes.  I don't think
> the qemu pflash emulation dictates anything here.
> 
> I think the underlying problem we actually have in qemu is that the
> flash size indirectly dictates the memory layout.  We pack the flash
> devices next to each other, on x86 downwards from 4G, on arm upwards
> from zero, not sure what risc-v is dong here.
> 
> edk2 arm code expects the variable store being mapped at 64m.  So
> QEMU_EFI.fd (which is actually 2M in size) gets padded to 64m, which
> has the desired effect that the next flash device (the varstore) is
> mapped at 64m.  But also has the side effect that we map 62m of zeros
> into the guest address space ...
> 
> The vars file is padded to 64m for consistency with the code.  Not
> padding the vars file should have no bad side effects I think, except
> for live migration where the flash size change might cause
> compatibility problems.
> 
> Not padding the code file needs some alternative way to specify the
> memory layout, to make sure the vars flash continues to be mapped at
> 64m even when the code flash is smaller.  Cc'ed Pawel who investigates
> that right now.
> 
> One possible option is to just hard-code the flash memory layout per
> machine type or per architecture.  Another option would be to add
> some way to configure that on the command line.

Thanks Gerd. One question: Is it possible to have separate code + vars
even when there is TF-A? My understanding is, TF-A will take one drive
and will be hidden from the non-secure word. So, there is only one flash
left for edk2. Is that correct?

In RISC-V, I think we have the this situation always since M-mode is
mandatory. The first flash is always reserved for secure fw. So, we will
have to increase the number of flash supported to 3 to support edk2 use
case.

I have a fix RISC-V which resolves truncate issue leveraging logic from
x86. It also creates 2 flash drives within non-secure space.
EDK2 also needs to be modified to work with smaller code flash. But
because the patch takes care of the actual size, it allows to have
bigger code and smaller var images.
Same thing can be adopted to arm also since both seem to follow the same
logic. But I think that will be a separate patch than this series. I
will run that as a separate RFC patch. Is that fine?

Thanks!
Sunil
> 
> take care,
>   Gerd
>

  Hi,

> Thanks Gerd. One question: Is it possible to have separate code + vars
> even when there is TF-A? My understanding is, TF-A will take one drive
> and will be hidden from the non-secure word. So, there is only one flash
> left for edk2. Is that correct?

Yes.

> In RISC-V, I think we have the this situation always since M-mode is
> mandatory. The first flash is always reserved for secure fw. So, we will
> have to increase the number of flash supported to 3 to support edk2 use
> case.

Well.  Adding one more flash is certainly the easiest approach.  Another
possible option would be to have the secure world store the efi variables.
That might be needed anyway for secure boot support.

> I have a fix RISC-V which resolves truncate issue leveraging logic from
> x86. It also creates 2 flash drives within non-secure space.
> EDK2 also needs to be modified to work with smaller code flash. But
> because the patch takes care of the actual size, it allows to have
> bigger code and smaller var images.

The size of the code flash and thereby the address of the varstore is
known at compile time too, so yes, that approach should work fine.

Note that changing the flash size can lead to compatibility problems
(for example when live migrating), so I'd suggest to be generous with
code/vars sizes.  On x64 the upstream default flash size is 2M and when
all compile-time options are enabled things don't fit any more.

So, assuming size figures are roughly the same for risc-v, I'd suggest
to go for 4M or 8M code flash and 1M vars flash.

> Same thing can be adopted to arm also since both seem to follow the same
> logic.

Yes.  The tricky part here is dealing with backward compatibility and
the transition from padded to non-padded images.  I suspect at the end
of the day keeping the vars flash mapping fixed at 64M (and have a hole
between code and vars) will be easier.

> But I think that will be a separate patch than this series. I
> will run that as a separate RFC patch. Is that fine?

No objections.

take care,
  Gerd

> Same thing can be adopted to arm also since both seem to follow the same
> logic. But I think that will be a separate patch than this series. I
> will run that as a separate RFC patch. Is that fine?

Just to be clear, I meant RISC-V fix as separate RFC patch. Not the ARM.

Thanks
Sunil