search

pftf / RPi4

Raspberry Pi 4 UEFI Firmware Images
https://rpi4-uefi.dev
Other
1.16k stars 138 forks source link

ACS FWTS uefirttime - RT services failure with GetNextHighMonotonicCount #163

Open samerhaj opened 2 years ago

samerhaj commented 2 years ago

ACS 3.0 FWTS failure reported on the RPi 400:

Test 2 of 4: Stress test for UEFI miscellaneous runtime service interfaces.
Stress testing for UEFI runtime service GetNextHighMonotonicCount interface.
FAILED [HIGH] UEFIRuntimeGetNextHighMonotonicCount: Test 2, Failed to get high
monotonic count with UEFI runtime service.
Return status: EFI_OUT_OF_RESOURCES. A resource has run out.

he RPi does not support storing UEFI NV variables at runtime (as reported in #6)

GetNextMonotonicCount implementation on the RPi (using the MdeModulePkg driver version: https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounter.c#L131)  follows the UEFI spec, and calls SetVariable to store the higher monotonic count. The calls ends up failing with EFI_OUT_OF_RESOURCES, causing the test failure.

Note that other FWTS tests that do this (like uefirtvariable) see the same failure, but the tests are skipped rather than marked as failed:

uefirtvariable: UEFI Runtime service variable interface tests.
--------------------------------------------------------------------------------
Test 1 of 9: Test UEFI RT service get variable interface.
Return status: EFI_OUT_OF_RESOURCES. A resource has run out.
SKIPPED: Test 1, Run out of resources for SetVariable UEFI runtime interface:
cannot test.

Also Note that FWTS checks for EFI_RT_PROPERTIES table, and skips these tests if the table properly flags the service as un-implemented. However, the RPi (and EDK2 in general) does not implement this table yet, as reported in #112 . Once that is implemented, these FWTS failures will get resolved

sunnywang-arm commented 2 years ago

A fix is sent to the edk2 mailing list - https://edk2.groups.io/g/devel/topic/edk2_platform_patch_v1_1_1/84397460

sunnywang-arm commented 2 years ago

In short, I think using UNSUPPORTED to skip the FWTS test is not a good solution at this moment. After looking into this further , the better solution should be to expand RPi4's EFI storage space size. For the details, you can check the following:

Directly making gRT->SetVariable at runtime return EFI_UNSUPPORTED would cause some OSes' installation failure/error.

As for Using EFI_RT_PROPERTIES table, it would cause another FWTS failures:

Also, I found that RPi4's gRT->SetVariable() at runtime still partially works. The problem with RPi4's gRT->SetVariable at runtime is that RPi4 UEFI only writes UEFI variable to the cache data in preserved runtime memory that won’t be synced with non-volatile storage. Therefore, any UEFI variable modification at runtime won’t be preserved after reboot. In other words, the caller of gRT->SetVariable() would still get EFI_SUCCESS. Therefore, I think returning EFI_UNSUPPORTED is not a good solution at this moment.

The root cause of this test failure is that the FWTS Stress testing for the UEFI runtime service GetNextHighMonotonicCount interface would use up the UEFI storage space. Running the stress testing at least needs 36K. RPi4 UEFI variable storage size is only 56K, and the remaining space at runtime may be less than 30K.

Therefore, I think a better solution should be to expand the UEFI storage space size. Now I ran into some problem with expanding the space, so I'm still debugging it. It looks like only modifying the flash map in RPi4.fdf won't work, I got the messages and ASSERT below. @ardbiesheuvel @samerhaj @pbatard if you guys have any concerns about expanding the UEFI storage space or have any idea about fixing the ASSERT issue, please let me know. By the way, I will also create a bug in https://bugs.launchpad.net/fwts to check why FWTS needs this Stress testing.

FD Variables:
PhysicalBase: 0x1B0000
VirtualBase: 0x1B0000
Length: 0x40000
....
ValidateFvHeader: Start
Variable FV header is not valid. It will be reinitialized.
VarStoreErase Start Address = 0x1B0000
VarStoreWrite Address = 0x1B0000
....
ASSERT [VariableRuntimeDxe] /mnt/c/Src/RPi4/edk2/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c(229): VariableStore->Size == VariableStoreLength
ardbiesheuvel commented 2 years ago

Maybe this is not the right venue to discuss this, but no OS actually uses GetNextHighMonotonicCount() for anything, so it seems like a lot of wasted effort to implement this, and it would be better to enhance the spec and/or the test suite to permit GetNextHighMonotonicCount() to remain unimplemented.

paulwratt commented 2 years ago

Is this one of the variable that can be stored in the RPi4, the way the current "hack" is used for some other variables (or is the "hack" acceptable upstream at RPi Firmware) as long as its not writing every Monotonic Tick.