Updates for ARC feedback #182, #198 and #201 #208
Conversation
|
@andreiw @jhauser-us @avpatel @adurbin-rivos - please review. |
vlsunil
changed the title
There was a problem hiding this comment.
The intention of URT_040 was to avoid RPi-like UEFI implementations, where NV variables are in practice volatile after ExitBootServices. This leads to bad experiences with OS installers and probably other things.
Pi 4 had no non-volatile store beyond the SD card. Variables were persisted in RPI_EFI.FD during Boot Services, but he SD card could not be accessed after OS use, so - no non-volatile vars in the OS.
URT_040 refers to a behavior wrt ResetSystem, and that was intended - again, deficient designs like RPi4 could, during their ResetSystem path, flush out NV variables to persistent store (now that the OS is out of the way)
There was a problem hiding this comment.
I hate to be pendantic, but the UEFI spec already talks about identity mapping for RAM regions as a requirement.
Probably want an additional requirement that EFI_MEMORY_ATTRIBUTE_PROTOCOL and EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes() are implemented and non-dummy (really affects page attributes).
|
It seems that people feel that system designs should follow the already written requirements in the specs vs trying to re-describe or over-describe things to keep people from shooting themselves in the foot. I'm fine w/ these changes as I'm more on the side of "people should design things correctly" vs trying to prevent larger mistakes. |
|
If its a common mistake then its not a bad idea to call it out. The rule book is intended to be helpful. |
Ahh OK. Thanks!. I thought that was the intention. In that case, let's rephrase the requirement that "The UEFI variables MUST be persistent across ResetSystem() call". I hope that should be clear enough requirement. |
While I agree with that, I think it is better to mention again here since we are talking about MMU configuration. This should make it clear that we are still requiring identity map while configuring non-BARE mode.
Good point. However, it is better to avoid requiring EFI_CPU_ARCH_PROTOCOL which is part of PI spec. Better to leave to the FW implementation to provide EFI_MEMORY_ATTRIBUTE_PROTOCOL even when they are not PI complaint. WDYT? |
URT_040 merely states that non-volatile UEFI variables must be persistent across reboots. This is redundant statement and the requirement is not very clear. The intention of the requirement is to mandate non-volatile UEFI variables. So, just make it is clear that UEFI variables must be persistent. Fixes: riscv-non-isa#201 Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
The intention of UEFI_030 is to configure the MMU in UEFI firmware for memory protection use cases while keeping the identify mapping. Add bit more text around this to clarify the intention. Fixes: riscv-non-isa#182 Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
The RVI-specific ACPI Device Properties section exists as a place holder to define standard DSD properties in the "rscv-" namespace. While there are no properties currently defined, it is the place where one can add new properties approved by appropriate RVI forum. Try to reword to make the intention clear. Fixes: riscv-non-isa#198 Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
This get's it wrong. Volatile variables should not be persisted. Only variables with attribute EFI_VARIABLE_NON_VOLATILE must be persisted. |
My attempt to resolve ARC feedback regarding UEFI variable persistence, SATP and ACPI device property section. Please review and suggest better wordings or if I am missing any thing.