UEFI SCT: Improve runtime services support detection #316

sathishas89 · 2026-02-06T04:18:22Z

Update RuntimePropertiesTable test to validate runtime service support based on actual return status, treating EFI_UNSUPPORTED as not supported.

vstehle · 2026-02-06T08:09:23Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+STATIC
+EFI_STATUS
+CallRuntimeService (
+  IN  UINT32   RTServiceFlag


I find the "Flag" in the name misleading as this looks more like a "choice" or a "type" or a "selection".

vstehle · 2026-02-06T08:12:36Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+      return gtRT->QueryCapsuleCapabilities (NULL, 0, &MaxCapsuleSize, &ResetType);
+    }
+
+    case EFI_RT_SUPPORTED_UPDATE_CAPSULE: {


Nit-picking: you do not need the braces "{ ... }" here.
(And below.)

vstehle · 2026-02-06T08:14:54Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+      // ResetSystem is platform-handled and would reset the system, so we don't
+      // actually call it here. Treat this test as UNSUPPORTED.
+      //
+      return EFI_UNSUPPORTED;


Should we not rather return another error code instead, different from EFI_UNSUPPORTED?
EFI_INVALID_PARAMETER maybe?

vstehle · 2026-02-06T08:27:04Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+      VOID *Function = RuntimeServices[Index].Function;
+      if (Function != NULL) {
+        if ((RtPropertiesTable->RuntimeServicesSupported & Flag) == 0) { // marked as unsupported
+          Status = CallRuntimeService (Flag);


I think this is where we should avoid calling ResetSystem().

vstehle · 2026-02-06T08:28:22Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

@@ -733,17 +732,186 @@ CheckRuntimeServices (
  return EFI_SUCCESS;
 }



We should document that CallRuntimeService() will refuse to call ResetSystem() for now.

sathishas89

Fixed the given comments by @vstehle .

vstehle · 2026-02-09T17:03:26Z

Thanks @sathishas89.

sunnywang-arm

Thanks for addressing Ehdaya and my offline comments, @sathishas89.
Just few more minor comments. Otherwise, it looks good to me.

sunnywang-arm · 2026-02-09T22:32:24Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+        if (!EFI_ERROR(Status)) {
+          // supported
+        } else {
+          bUnsupportedRuntimeServiceFound = TRUE;


Can we print message to indicate which runtime service unexpectly return unsupported?

I don't see the fix in this commit. Could you check?

sunnywang-arm · 2026-02-09T22:33:15Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+          bUnsupportedRuntimeServiceFound = TRUE;
+        }
+      } else {
+        bUnsupportedRuntimeServiceFound = TRUE;


Can we print message to indicate which runtime service pointer is unexpectly NULL?

I don't see the fix in this commit. Could you check?

sunnywang-arm · 2026-02-09T22:34:11Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+{
+  switch (RTServiceType) {
+    case EFI_RT_SUPPORTED_GET_TIME: {
+      EFI_TIME Time;


Per edk2 coding formatting and its based C coding standard C95, could you move all variable declarations after code statements to the top of function (before code statements)?

sathishas89

Fixed the given comments @sunnywang-arm .

sunnywang-arm · 2026-02-10T16:02:55Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+      // SetVirtualAddressMap should not be called during boot services.
+      // Return the status as-is to detect if the service is present.
+      //
+      return gtRT->SetVirtualAddressMap (0, 0, 0, NULL);


The comment is confusing. We should not call SetVirtualAddressMap here, right? Move this to the block of line 1002?

sunnywang-arm · 2026-02-10T16:03:10Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+      // Return the status as-is to detect if the service is present.
+      //
+      Pointer = NULL;
+      return gtRT->ConvertPointer (0, &Pointer);


The comment is confusing. We should not call ConvertPointer here, right? Move this check to the block of line 1002?

sunnywang-arm · 2026-02-10T20:25:48Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+        if (!EFI_ERROR(Status)) {
+          // supported
+        } else {
+          bUnsupportedRuntimeServiceFound = TRUE;


I don't see the fix in this commit. Could you check?

sunnywang-arm · 2026-02-10T20:25:51Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+          bUnsupportedRuntimeServiceFound = TRUE;
+        }
+      } else {
+        bUnsupportedRuntimeServiceFound = TRUE;


I don't see the fix in this commit. Could you check?

cgjertsen · 2026-02-10T21:15:06Z

...SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestRequired_uefi.c

+    case EFI_RT_SUPPORTED_GET_TIME:
+      return gtRT->GetTime (&Time, &Cap);
+
+    case EFI_RT_SUPPORTED_SET_TIME: {


I've never seen the use of braces like this in a case statement and I don't see an EDKII Coding Standard rule about using it in this kind of context.

Removed braces as per EDKII guidelines.

Update RuntimePropertiesTable test to validate runtime service support based on actual return status, treating EFI_UNSUPPORTED as not supported. Signed-off-by: Sathisha S <[email protected]>

sathishas89

Fixed the comments, Please check.

tianocore-assign-reviewers bot requested review from anncheng-arm, cgjertsen, edhay, shellyki-intel and sunnywang-arm February 6, 2026 04:18

vstehle reviewed Feb 6, 2026

View reviewed changes

sathishas89 force-pushed the EFI_Runtime_Properties_Table_Issue branch from e71ac43 to 5fd6ce5 Compare February 9, 2026 04:16

sathishas89 commented Feb 9, 2026

View reviewed changes

sunnywang-arm suggested changes Feb 9, 2026

View reviewed changes

sathishas89 force-pushed the EFI_Runtime_Properties_Table_Issue branch from 5fd6ce5 to 04f1f5f Compare February 10, 2026 10:34

sathishas89 commented Feb 10, 2026

View reviewed changes

sunnywang-arm suggested changes Feb 10, 2026

View reviewed changes

cgjertsen reviewed Feb 10, 2026

View reviewed changes

UEFI SCT: Improve runtime services support detection

65187c9

Update RuntimePropertiesTable test to validate runtime service support based on actual return status, treating EFI_UNSUPPORTED as not supported. Signed-off-by: Sathisha S <[email protected]>

sathishas89 force-pushed the EFI_Runtime_Properties_Table_Issue branch from 04f1f5f to 65187c9 Compare February 12, 2026 10:48

sathishas89 commented Feb 12, 2026

View reviewed changes

		@@ -733,17 +732,186 @@ CheckRuntimeServices (
		return EFI_SUCCESS;
		}

UEFI SCT: Improve runtime services support detection #316

Are you sure you want to change the base?

UEFI SCT: Improve runtime services support detection #316

Uh oh!

Conversation

sathishas89 commented Feb 6, 2026

Uh oh!

vstehle Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstehle Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstehle Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sathishas89 left a comment

Choose a reason for hiding this comment

Uh oh!

vstehle commented Feb 9, 2026

Uh oh!

sunnywang-arm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sathishas89 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sathishas89 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

vstehle Feb 6, 2026 •

edited

Loading

vstehle Feb 6, 2026 •

edited

Loading

vstehle Feb 6, 2026 •

edited

Loading