Hi Peter,
Comments below...
On 25/09/2018 16:43, Peter Jones wrote:
On Mon, Sep 24, 2018 at 03:54:03PM +0200, Grant Likely wrote:
After face to face meeting at Linaro Connect YVR18, the decision was made to keep variable services very simple. Either fully provide SetVariable/GetVariable during runtime services, or don't provide them at all.
This is an RFC patch. In the process of writing it, and after looking at the ECR submitted by Peter Jones[1], I started having doubts. I no longer think this is the right decision. Since the ECR now provides a mechanism for reporting which runtime services can be called, it should be just fine to provide GetVariable() even if SetVariable() returns EFI_UNSUPPORTED. There is a note added to the document to this effect. I will remove the note after committing the final version of the patch, and depending on mailing list discussion.
[1] https://pjones.fedorapeople.org/rt-unsupported-ecr-0.txt
Resolves: #22 Signed-off-by: Grant Likely grant.likely@secretlab.ca Cc: Peter Jones pjones@redhat.com
source/chapter2-uefi.rst | 113 ++++++++++++++++++++++++----------------------- 1 file changed, 58 insertions(+), 55 deletions(-)
diff --git a/source/chapter2-uefi.rst b/source/chapter2-uefi.rst index cbf1529..72b3d59 100644 --- a/source/chapter2-uefi.rst +++ b/source/chapter2-uefi.rst @@ -197,66 +197,69 @@ unless a specific reset is required after a call to UpdateCapsule(). Runtime Variable Access
[old bits skipped]
+There are many platforms where it is difficult to implement SetVariable() for +non-volatile variables during runtime services because the firmware cannot +access storage after ExitBootServices() is called.
+e.g., If firmware accesses an eMMC device directly at runtime, it will +collide with transactions initiated by the OS. +Neither U-Boot nor Tianocore have a generic solution for accessing or updating +variables stored on shared media. [#OPTEESupplicant]_
Well, that's not *quite* true. OvmfPkg in edk2 has EmuVariableFvbRuntimeDxe, which might be fairly close to what we want.
+If a platform does not implement modifying non-volatile variables with +SetVariable() after ExitBootServices(), +then it must not provide any variable operations after ExitBootServices(). +Firmware shall return EFI_UNSUPPORTED for any call to GetVariable(), +GetNextVariableName() and SetVariable(). +Firmware shall not emulated non-volatile variables using volatile RAM cache.
+.. note:: I'm still not sold on this. Can we simply advertise that SetVariable()
- doesn't work via the `RuntimeServicesSupported` variable?
- RuntimeServicesSupported is a proposed ECR to the UEFI spec.
I think we can do that, but only if we *don't* support using some on-disk storage during Boot Services and then handing that storage off to a linux driver, which I see you've mentioned below. I'll comment about that down there.
Took me a few reads to parse your meaning. You're arguing against the linux-driver-back-channel method of updating variables, correct? If so, then we're in violent agreement.
- If we say here that GetVariable() must be disabled if SetVariable() doesn't
- work, then that precludes all the RT but !NV global varibles, including all
- the secure boot variables.
- It also potentially means distros will depend on the
- "no SetVariable == no GetVariable" behaviour, meaning it will be difficult
- re-enable GetVariable() without SetVariable().
- Looking at section 3.3, the folloing EFI_GLOBAL_VARIABLEs are RT but not NV:
- AuditMode BS, RT
- BootCurrent BS, RT
- BootOptionSupport BS,RT,
- ConInDev BS, RT
- ConOutDev BS, RT
- dbDefault BS, RT
- dbrDefault BS, RT
- dbtDefault BS, RT
- dbxDefault BS, RT
- DeployedMode BS, RT
- ErrOutDev BS, RT
- KEKDefault BS, RT
- LangCodes BS, RT
- OsIndicationsSupported BS, RT
- PKDefault BS, RT
- PlatformLangCodes BS, RT
- PlatformRecovery#### BS, RT
- SignatureSupport BS, RT
- SecureBoot BS, RT
- SetupMode BS, RT
- VendorKeys BS, RT
Worth noting that all of these are expected to be generated during boot by the system firmware.
Yup, boot time generated.
It would be fairly reasonable to dump them into a UEFI config table and let us populate efivarfs from that.
Doesn't even need to be a config table. Just needs to be a page or 3 that is EfiRuntimeServicesData allocated and that GetVariable() can read when called.
I don't think it is necessary to expose the variable store with a mechanism other than GetVariable().
Which actually means I'm violating the convention to use NV for RuntimeServicesSupported, and I should probably mark it as optionally "BS" due to its definition, as well. I'll fix that, with something along the lines of:
--- rt-unsupported-ecr-0.txt 2018-09-19 19:15:26.467010512 -0400 +++ rt-unsupported-ecr-1.txt 2018-09-25 10:23:26.470218080 -0400 @@ -78,9 +78,11 @@ Add the following entry to table 10 ("Global Variables") +--------------------------+-------+----------------------------------------+
- | RuntimeServicesSupported | NV,BS | Bitmask of which calls are implemented |
- | RuntimeServicesSupported | BS,RT | Bitmask of which calls are implemented | | | | by the firmware during runtime |
- | | | services. See Section 8.5 . |
- | | | services. RT access is required only |
- | | | if GetVariable() is implemented by |
- | | | runtime services. See Section 8.5 . | +--------------------------+-------+----------------------------------------+
Yes, that looks better.
In section 8.5, add the following text before the "Related Definitions"
I'll update the UEFI mantis ticket as well.
- In related news, I'm considering proposing a "SetVariable on Disk" analogous
- to the current Capsule on Disk [UEFI]_ 8.5.5.
- That would allow the OS to write a standard format variable update file to
- the ESP that firmware can read, update, and clear at boot time.
- However, this could also be implemented using an OS provided variable-update
- UEFI application.
It doesn't need to be *analogous* to Capsule on Disk - that's already exactly the mechanism we're looking for. We just have to define a data structure to go in the capsule and a GUID to identify it. If we're careful we could probably use the same GUID and the same format for the config table to hand off boot-time generated variables to the OS.
That is a really good idea. Keeps it nice and simple.
Just brainstorming here, but I think when we have shared storage like eMMC, there's a fairly compelling story along these lines:
- UEFI writes a capsule to an EFI Config Table during boot with any generated variables as well as any persistent variables stored on e.g. an eMMC partition it sets as write protected during ExitBootServices()
- efivarfs picks up that capsule during boot and uses that as its seed data (maybe it uses it as is, maybe it translates it to some internal format and frees the ram back to the allocator. Doesn't make much difference.)
Wait, you've lost me. Are you suggesting that the OS read the capsule to get variables instead of using GetVariable()/GetNextVariableName()? I do not like the idea of having two different mechanisms for retrieving variables.
- for runtime updates, we do "best effort" at having the same security model as UEFI, and represent the changes in ram both in /sys/firmware/efi/efivars , as well as a capsule file in e.g. /sys/firmware/efi/efivars-updates.cap
Ah, clever. So that way the normal efibootmgr tool works, but userspace will still need to copy the new .cap from sysfs to the EFI, correct?
- during shutdown, we copy that .cap to the ESP as a Capsule on Disk
- during boot up, UEFI reads that .cap and applies the update to the storage. Any cases the OS was wrong about its "best effort" security model simply fail
Yes, completly agree here.
That gets us more or less the same behavior normal UEFI machines have, with the exception of the "do both pieces of code know about all variable update rules" issue - but since UEFI gets to be the final arbiter there, that failure can generally be safe.