| Message ID | 20220906090219.412517-1-sunilvl@ventanamicro.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | hw/riscv: virt: Enable booting S-mode firmware from pflash | expand |
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 >
> 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
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
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(-)