On 19/5/23 18:34, Philippe Mathieu-Daudé wrote:
On 18/5/23 08:03, Sunil V L wrote:
On Thu, May 18, 2023 at 02:55:16PM +1000, Alistair Francis wrote:
On Wed, May 17, 2023 at 10:48 PM Philippe Mathieu-Daudé
<phi...@linaro.org> wrote:
On 8/5/23 12:00, Andrea Bolognani wrote:
On Mon, May 08, 2023 at 11:37:43AM +0530, Sunil V L wrote:
On Mon, May 08, 2023 at 07:37:23AM +0200, Heinrich Schuchardt wrote:
On 4/25/23 12:25, Sunil V L wrote:
Currently, virt machine supports two pflash instances each with
32MB size. However, the first pflash is always assumed to
contain M-mode firmware and reset vector is set to this if
enabled. Hence, for S-mode payloads like EDK2, only one pflash
instance is available for use. This means both code and NV
variables
of EDK2 will need to use the same pflash.
The OS distros keep the EDK2 FW code as readonly. When non-volatile
variables also need to share the same pflash, it is not possible
to keep it as readonly since variables need write access.
To resolve this issue, the code and NV variables need to be
separated.
But in that case we need an extra flash. Hence, modify the
convention
such that pflash0 will contain the M-mode FW only when "-bios none"
option is used. Otherwise, pflash0 will contain the S-mode
payload FW.
This enables both pflash instances available for EDK2 use.
Example usage:
1) pflash0 containing M-mode FW
qemu-system-riscv64 -bios none -pflash <mmode_fw> -machine virt
or
qemu-system-riscv64 -bios none \
-drive file=<mmode_fw>,if=pflash,format=raw,unit=0 -machine virt
2) pflash0 containing S-mode payload like EDK2
qemu-system-riscv64 -pflash <smode_fw_vars> -pflash
<smode_fw_code> -machine virt
or
qemu-system-riscv64 -bios <opensbi_fw> \
-pflash <smode_fw_vars> \
-pflash <smode_fw_code> \
On amd64 and arm64 unit=0 is used for code and unit=1 is used for
variables.
Shouldn't riscv64 do the same?
Good catch, I had missed that!
This is a design mistake spreading.
What EDK2 maintainers want is one Read-Only + Exec region for CODE and
one Read-Write + NoExec region for VARS.
QEMU never implemented correctly pflash bank (multiple sectors) write
protected.
When EDK2 (x86, OVMF) was tried on QEMU, QEMU was using a single
pflash.
To separate CODE/VARS, a second pflash was added, the first one being
"locked" into Read-Only mode. Using a pflash allowed the firmware to
identify device size using pflash CFI commands.
Then this design was copied to the ARM virt board for EDK2 needs.
In retrospective, this design was declared a mistake, since a simple
ROM region for the CODE is sufficient, and much simpler [*].
It seems like we are making changes to the whole flash setup. Is it
worth adding the ROM region now as well, so we can migrate to the
simpler approach.
ROM is used for FW image used for -bios option in RISC-V. It appears
that loongarch uses -bios to boot EDK2 also as per
docs/system/loongarch/virt.rst. But in RISC-V, EDK2 is S-mode payload
and hence -bios can not be used.
Personally I'd try to not use -bios at all, as I see it as a magic /
confusing machine-specific command line option, doing some open-coded
"load this blob somewhere in memory". I prefer using explicit hardware
device model.
-bios usually call the "hw/loader.h" API which eventually model a ROM,
but do a lot of file format parsing magic. If you pass a plain ROM file,
no need for all this magic.
If we have to create ROM for S-mode
payload, do we need to add a new qemu option? Also, I don't see
enough space in the memmap of RISC-V virt machine for PAYLOAD_ROM
region. So, I am not sure how to keep 2 flashes and add ROM region for
S-mode payload. Let me know if I am missing some thing.
We would keep two pflash regions, hopefully in a way that doesn't
break any existing users.
Agree. While I agree with Philippe, I think we better solve the current
problem by going back to v1 of the patch.
BTW clarifying, I'm not rejecting this particular patch; I was just
trying to correct the idea than "doing what other architectures do"
is always the right approach ;)
