search

riscv-non-isa / riscv-brs

The Boot and Runtime Services (BRS) specification provides the software requirements for system vendors and Operating System Vendors (OSVs) to interoperate with one another by providing expectations for the Operating System (OS) to utilize in acts of device discovery, system management, and other rich operations provided in this specification.
https://jira.riscv.org/browse/RVG-48
Creative Commons Attribution 4.0 International
42 stars 13 forks source link

Updates for ARC feedback #182, #198 and #201 #208

Closed vlsunil closed 1 week ago

vlsunil commented 3 weeks ago

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.

vlsunil commented 3 weeks ago

@andreiw @jhauser-us @avpatel @adurbin-rivos - please review.

adurbin-rivos commented 3 weeks ago

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.

ved-rivos commented 3 weeks ago

If its a common mistake then its not a bad idea to call it out. The rule book is intended to be helpful.

vlsunil commented 2 weeks ago

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)

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.

vlsunil commented 2 weeks ago

I hate to be pendantic, but the UEFI spec already talks about identity mapping for RAM regions as a 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.

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).

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?