[PATCH 0/3] MdePkg/MdeModulePkg: Introduce a pcd to control runtime services

[PATCH 0/3] MdePkg/MdeModulePkg: Introduce a pcd to control runtime services

[Gao, Zhichao]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1907

UEFI spec 2.8 introduce a new variable L"RuntimeServicesSupported".
If some runtime sevices is not supported at runtime phase, the variable
should present at boot services. It is a bitmask value, the bit value of
zero indicate the related runtime services is not supported at runtime
phase.
Add the difinition and use it to control Capsule runtime services.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>

Zhichao Gao (3):
  MdePkg/UefiSpec.h: Add define of runtime services support
  MdePkg: Indicate new pcd PcdRuntimeServicesSupport
  MdeModulePkg/CapsuleRuntimeDxe: Control runtime services supported

 .../CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf   |   3 +
 .../CapsuleRuntimeDxe/CapsuleService.c        | 129 ++++++++++++++++++
 MdePkg/Include/Uefi/UefiSpec.h                |  15 ++
 MdePkg/MdePkg.dec                             |  19 +++
 MdePkg/MdePkg.uni                             |  19 +++
 5 files changed, 185 insertions(+)

-- 
2.21.0.windows.1

[PATCH 1/3] MdePkg/UefiSpec.h: Add define of runtime services support

[Gao, Zhichao]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1907

UEFI spec 2.8, Section 8.1.1 define some MACROs for
RuntimeServicesSupported variable. Add them to UefiSpec.h.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdePkg/Include/Uefi/UefiSpec.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/MdePkg/Include/Uefi/UefiSpec.h b/MdePkg/Include/Uefi/UefiSpec.h
index 44a0a6a7fa..20ecbff26e 100644
--- a/MdePkg/Include/Uefi/UefiSpec.h
+++ b/MdePkg/Include/Uefi/UefiSpec.h
@@ -1783,6 +1783,21 @@ EFI_STATUS
 #define EFI_RUNTIME_SERVICES_SIGNATURE  SIGNATURE_64 ('R','U','N','T','S','E','R','V')
 #define EFI_RUNTIME_SERVICES_REVISION   EFI_SPECIFICATION_VERSION
 
+#define EFI_RT_SUPPORTED_GET_TIME                             0x0001
+#define EFI_RT_SUPPORTED_SET_TIME                             0x0002
+#define EFI_RT_SUPPORTED_GET_WAKEUP_TIME                      0x0004
+#define EFI_RT_SUPPORTED_SET_WAKEUP_TIME                      0x0008
+#define EFI_RT_SUPPORTED_GET_VARIABLE                         0x0010
+#define EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME               0x0020
+#define EFI_RT_SUPPORTED_SET_VARIABLE                         0x0040
+#define EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP              0x0080
+#define EFI_RT_SUPPORTED_CONVERT_POINTER                      0x0100
+#define EFI_RT_SUPPORTED_GET_NEXT_HIGH_MONOTONIC_COUNT        0x0200
+#define EFI_RT_SUPPORTED_RESET_SYSTEM                         0x0400
+#define EFI_RT_SUPPORTED_UPDATE_CAPSULE                       0x0800
+#define EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES           0x1000
+#define EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO                  0x2000
+
 ///
 /// EFI Runtime Services Table.
 ///
-- 
2.21.0.windows.1

[PATCH 2/3] MdePkg: Add new pcd PcdRuntimeServicesSupport

[Gao, Zhichao]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1907

Add pcd PcdRuntimeServicesSupport to control whether runtime
services is supported at runtime phase. It is a UINT16 type
bitmask value. Refer to Uefi Spec 2.8, Section 8.1.1.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdePkg/MdePkg.dec | 19 +++++++++++++++++++
 MdePkg/MdePkg.uni | 19 +++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 6c563375ee..5e9205dc10 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2076,6 +2076,25 @@
   # @Prompt Speculation Barrier Type.
   gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType|0x01|UINT8|0x30001018
 
+  ## Indicates the supported runtime services after exit boot services.
+  #  Uefi Spec 2.8, Section 8.1.1, RuntimeServicesSupported variable realted definitions.<BR><BR>
+  #   0x0001 (EFI_RT_SUPPORTED_GET_TIME)<BR>
+  #   0x0002 (EFI_RT_SUPPORTED_SET_TIME)<BR>
+  #   0x0004 (EFI_RT_SUPPORTED_GET_WAKEUP_TIME)<BR>
+  #   0x0008 (EFI_RT_SUPPORTED_SET_WAKEUP_TIME)<BR>
+  #   0x0010 (EFI_RT_SUPPORTED_GET_VARIABLE)<BR>
+  #   0x0020 (EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME)<BR>
+  #   0x0040 (EFI_RT_SUPPORTED_SET_VARIABLE)<BR>
+  #   0x0080 (EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP)<BR>
+  #   0x0100 (EFI_RT_SUPPORTED_CONVERT_POINTER)<BR>
+  #   0x0200 (EFI_RT_SUPPORTED_GET_NEXT_HIGH_MONOTONIC_COUNT)<BR>
+  #   0x0400 (EFI_RT_SUPPORTED_RESET_SYSTEM)<BR>
+  #   0x0800 (EFI_RT_SUPPORTED_UPDATE_CAPSULE)<BR>
+  #   0x1000 (EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES)<BR>
+  #   0x2000 (EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)
+  # @Prompt Runtime Services support.
+  gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupport|0x3FFF|UINT16|0x30001019
+
 [PcdsFixedAtBuild,PcdsPatchableInModule]
   ## Indicates the maximum length of unicode string used in the following
   #  BaseLib functions: StrLen(), StrSize(), StrCmp(), StrnCmp(), StrCpy(), StrnCpy()<BR><BR>
diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
index 5c1fa24065..09e0cf22f1 100644
--- a/MdePkg/MdePkg.uni
+++ b/MdePkg/MdePkg.uni
@@ -157,6 +157,25 @@
                                                                                       "0x02 - CPUID  (IA32/X64).<BR>\n"
                                                                                       "Other - reserved"
 
+#string STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupport_PROMPT  #language en-US "Runtime Services support."
+
+#string STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupport_HELP  #language en-US   "Indicates the supported runtime services after exit boot services."
+                                                                                             "Uefi Spec 2.8, Section 8.1.1, RuntimeServicesSupported variable realted definitions.<BR><BR>"
+                                                                                             " 0x0001 (EFI_RT_SUPPORTED_GET_TIME)<BR>"
+                                                                                             " 0x0002 (EFI_RT_SUPPORTED_SET_TIME)<BR>"
+                                                                                             " 0x0004 (EFI_RT_SUPPORTED_GET_WAKEUP_TIME)<BR>"
+                                                                                             " 0x0008 (EFI_RT_SUPPORTED_SET_WAKEUP_TIME)<BR>"
+                                                                                             " 0x0010 (EFI_RT_SUPPORTED_GET_VARIABLE)<BR>"
+                                                                                             " 0x0020 (EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME)<BR>"
+                                                                                             " 0x0040 (EFI_RT_SUPPORTED_SET_VARIABLE)<BR>"
+                                                                                             " 0x0080 (EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP)<BR>"
+                                                                                             " 0x0100 (EFI_RT_SUPPORTED_CONVERT_POINTER)<BR>"
+                                                                                             " 0x0200 (EFI_RT_SUPPORTED_GET_NEXT_HIGH_MONOTONIC_COUNT)<BR>"
+                                                                                             " 0x0400 (EFI_RT_SUPPORTED_RESET_SYSTEM)<BR>"
+                                                                                             " 0x0800 (EFI_RT_SUPPORTED_UPDATE_CAPSULE)<BR>"
+                                                                                             " 0x1000 (EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES)<BR>"
+                                                                                             " 0x2000 (EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)"
+
 #string STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLength_PROMPT  #language en-US "Maximum Length of Ascii String"
 
 #string STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLength_HELP  #language en-US "Sets the maximum number of ASCII characters used for string functions.  This affects the following BaseLib functions: AsciiStrLen(), AsciiStrSize(), AsciiStrCmp(), AsciiStrnCmp(), AsciiStrCpy(), AsciiStrnCpy(). <BR><BR>\n"
-- 
2.21.0.windows.1

[PATCH 3/3] MdeModulePkg/CapsuleRuntimeDxe: Control runtime services supported

[Gao, Zhichao]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1907

Add PcdRuntimeServicesSupport to control whether the capsule services
is supported during runtime phase. If the L"RuntimeServicesSupported"
variable is not set yet, it would set the variable base on the pcd.
If the pcd value is 0x3FFF that means all runtime services is supported
at runtime phase, L"RuntimeServicesSupported" would not be set.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 .../CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf   |   3 +
 .../CapsuleRuntimeDxe/CapsuleService.c        | 129 ++++++++++++++++++
 2 files changed, 132 insertions(+)

diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
index 9da450722b..95b760d94e 100644
--- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
@@ -72,6 +72,8 @@
   ## SOMETIMES_PRODUCES   ## Variable:L"CapsuleLongModeBuffer" # The long mode buffer used by IA32 Capsule PEIM to call X64 CapsuleCoalesce code to handle >4GB capsule blocks
   gEfiCapsuleVendorGuid
   gEfiFmpCapsuleGuid                            ## SOMETIMES_CONSUMES   ## GUID # FMP capsule GUID
+  gEfiGlobalVariableGuid                        ## SOMETIMES_CONSUMES   ## Variable L"RuntimeServicesSupported"
+  gEfiEventExitBootServicesGuid                 ## CONSUMES
 
 [Protocols]
   gEfiCapsuleArchProtocolGuid                   ## PRODUCES
@@ -91,6 +93,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsule   ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizePopulateCapsule      ## SOMETIMES_CONSUMES # Populate Image requires reset support.
   gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleInRamSupport         ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupport            ## SOME_TIMES_CONSUMES
 
 [Pcd.X64]
   gEfiMdeModulePkgTokenSpaceGuid.PcdCapsulePeiLongModeStackSize   ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
index 77b8f00062..8ab77361d8 100644
--- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
+++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
@@ -24,6 +24,12 @@ UINTN       mTimes      = 0;
 UINT32      mMaxSizePopulateCapsule     = 0;
 UINT32      mMaxSizeNonPopulateCapsule  = 0;
 
+//
+// Runtime Services Supported Flag
+// Initialize it to 0x3FFF to indicate it is all supported before runtime
+//
+static UINT16     mRuntimeServicesSupported = 0x3FFF;
+
 /**
   Passes capsules to the firmware with both virtual and physical mapping. Depending on the intended
   consumption, the firmware may process the capsule immediately. If the payload should persist
@@ -71,6 +77,10 @@ UpdateCapsule (
   CHAR16                    CapsuleVarName[30];
   CHAR16                    *TempVarName;
 
+  if (!(mRuntimeServicesSupported | EFI_RT_SUPPORTED_UPDATE_CAPSULE)) {
+    return EFI_UNSUPPORTED;
+  }
+
   //
   // Check if platform support Capsule In RAM or not.
   // Platform could choose to drop CapsulePei/CapsuleX64 and do not support Capsule In RAM.
@@ -259,6 +269,10 @@ QueryCapsuleCapabilities (
   EFI_CAPSULE_HEADER        *CapsuleHeader;
   BOOLEAN                   NeedReset;
 
+  if (!(mRuntimeServicesSupported | EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES)) {
+    return EFI_UNSUPPORTED;
+  }
+
   //
   // Capsule Count can't be less than one.
   //
@@ -343,6 +357,106 @@ QueryCapsuleCapabilities (
   return EFI_SUCCESS;
 }
 
+/**
+  Set the L"RuntimeServicesSupported" variable depend on the pcd PcdRuntimeServicesSupport.
+
+  Firstly try to get the variable, it may be set in other dxe driver. If it is set, then return
+  EFI_SUCCESS. If it isn't present, try to set it.
+
+  @retval EFI_SUCCESS       The variable is already set.
+          EFI_NOT_FOUND     All runtime services are supported at runtime. No variable is set.
+          Others            Error to get variable or set variable. Unexpected.
+**/
+static
+EFI_STATUS
+SetRuntimeServicesSupported (
+  VOID
+  )
+{
+  EFI_STATUS    Status;
+  UINT16        RuntimeServicesSupported;
+  UINT32        Attributes;
+  UINTN         DataSize;
+
+  //
+  // Firstly try to get L"RuntimeServicesSupported" variable
+  //
+  Attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS;
+  DataSize = sizeof (UINT16);
+  Status = gRT->GetVariable (
+                  L"RuntimeServicesSupported",
+                  &gEfiGlobalVariableGuid,
+                  &Attributes,
+                  &DataSize,
+                  &RuntimeServicesSupported
+                  );
+
+  if (Status == EFI_NOT_FOUND) {
+    //
+    // L"RuntimeServicesSupported" isn't set yet. Then set it if
+    // some of the RuntimeServices is unsupported.
+    //
+    RuntimeServicesSupported = PcdGet16 (PcdRuntimeServicesSupport);
+    if (RuntimeServicesSupported != 0x3FFF) {
+      Status = gRT->SetVariable (
+                      L"RuntimeServicesSupported",
+                      &gEfiGlobalVariableGuid,
+                      EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+                      sizeof (UINT16),
+                      &RuntimeServicesSupported
+                      );
+    } else {
+      //
+      // Set Status to EFI_NOT_FOUND to indicate not such variable
+      //
+      Status = EFI_NOT_FOUND;
+    }
+  }
+
+  return Status;
+}
+
+/**
+  ExitBootServices Event to update the mRuntimeServicesSupported to
+  affect the runtime services.
+
+  @param[in]  Event     Event whose notification function is being invoked
+  @param[in]  Context   Pointer to the notification function's context
+**/
+static
+VOID
+UpdateRuntimeServicesSupported (
+  IN      EFI_EVENT                 Event,
+  IN      VOID                      *Context
+  )
+{
+  EFI_STATUS    Status;
+  UINT16        RuntimeServicesSupported;
+  UINT32        Attributes;
+  UINTN         DataSize;
+
+  Attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS;
+  DataSize = sizeof (UINT16);
+  Status = gRT->GetVariable (
+                  L"RuntimeServicesSupported",
+                  &gEfiGlobalVariableGuid,
+                  &Attributes,
+                  &DataSize,
+                  &RuntimeServicesSupported
+                  );
+
+  if (!EFI_ERROR (Status)) {
+    mRuntimeServicesSupported = RuntimeServicesSupported;
+  } else if (Status == EFI_NOT_FOUND) {
+    mRuntimeServicesSupported = 0x3FFF;
+  } else {
+    //
+    // Should never arrive here.
+    //
+    ASSERT_EFI_ERROR (Status);
+  }
+}
+
 
 /**
 
@@ -362,6 +476,7 @@ CapsuleServiceInitialize (
   )
 {
   EFI_STATUS  Status;
+  EFI_EVENT   Event;
 
   mMaxSizePopulateCapsule = PcdGet32(PcdMaxSizePopulateCapsule);
   mMaxSizeNonPopulateCapsule = PcdGet32(PcdMaxSizeNonPopulateCapsule);
@@ -393,5 +508,19 @@ CapsuleServiceInitialize (
                   );
   ASSERT_EFI_ERROR (Status);
 
+  Status = SetRuntimeServicesSupported ();
+
+  ASSERT (Status == EFI_NOT_FOUND || Status == EFI_SUCCESS);
+
+  Status = gBS->CreateEventEx(
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_NOTIFY,
+                  UpdateRuntimeServicesSupported,
+                  NULL,
+                  &gEfiEventExitBootServicesGuid,
+                  &Event
+                  );
+  ASSERT_EFI_ERROR (Status);
+
   return Status;
 }
-- 
2.21.0.windows.1

Re: [edk2-devel] [PATCH 3/3] MdeModulePkg/CapsuleRuntimeDxe: Control runtime services supported

[Michael D Kinney]

Zhichao,

Why is CapsuleRuntimeDxe the component that is responsible for
setting the UEFI Variable for supported RT services?

I would think this should be in:

	MdeModulePkg\Core\RuntimeDxe

I agree that each RT driver that populates the RT Services 
Table with a RT services can consume the new bitmask PCD and
use the PCD to determine if the RT Service should return
EFI_UNSUPPORTED after ExitBootServices().

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
> On Behalf Of Gao, Zhichao
> Sent: Wednesday, July 17, 2019 12:37 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng,
> Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan
> <sean.brogan@microsoft.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: [edk2-devel] [PATCH 3/3]
> MdeModulePkg/CapsuleRuntimeDxe: Control runtime services
> supported
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1907
> 
> Add PcdRuntimeServicesSupport to control whether the
> capsule services is supported during runtime phase. If
> the L"RuntimeServicesSupported"
> variable is not set yet, it would set the variable base
> on the pcd.
> If the pcd value is 0x3FFF that means all runtime
> services is supported at runtime phase,
> L"RuntimeServicesSupported" would not be set.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>  .../CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf   |   3 +
>  .../CapsuleRuntimeDxe/CapsuleService.c        | 129
> ++++++++++++++++++
>  2 files changed, 132 insertions(+)
> 
> diff --git
> a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime
> Dxe.inf
> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime
> Dxe.inf
> index 9da450722b..95b760d94e 100644
> ---
> a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime
> Dxe.inf
> +++
> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime
> Dxe.inf
> @@ -72,6 +72,8 @@
>    ## SOMETIMES_PRODUCES   ##
> Variable:L"CapsuleLongModeBuffer" # The long mode buffer
> used by IA32 Capsule PEIM to call X64 CapsuleCoalesce
> code to handle >4GB capsule blocks
>    gEfiCapsuleVendorGuid
>    gEfiFmpCapsuleGuid                            ##
> SOMETIMES_CONSUMES   ## GUID # FMP capsule GUID
> +  gEfiGlobalVariableGuid                        ##
> SOMETIMES_CONSUMES   ## Variable
> L"RuntimeServicesSupported"
> +  gEfiEventExitBootServicesGuid                 ##
> CONSUMES
> 
>  [Protocols]
>    gEfiCapsuleArchProtocolGuid                   ##
> PRODUCES
> @@ -91,6 +93,7 @@
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsu
> le   ## SOMETIMES_CONSUMES
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizePopulateCapsule
> ## SOMETIMES_CONSUMES # Populate Image requires reset
> support.
>    gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleInRamSupport
> ## CONSUMES
> +  gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupport
> ## SOME_TIMES_CONSUMES
> 
>  [Pcd.X64]
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdCapsulePeiLongModeStack
> Size   ## SOMETIMES_CONSUMES
> diff --git
> a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService
> .c
> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService
> .c
> index 77b8f00062..8ab77361d8 100644
> ---
> a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService
> .c
> +++
> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService
> .c
> @@ -24,6 +24,12 @@ UINTN       mTimes      = 0;
>  UINT32      mMaxSizePopulateCapsule     = 0;
>  UINT32      mMaxSizeNonPopulateCapsule  = 0;
> 
> +//
> +// Runtime Services Supported Flag
> +// Initialize it to 0x3FFF to indicate it is all
> supported before
> +runtime //
> +static UINT16     mRuntimeServicesSupported = 0x3FFF;
> +
>  /**
>    Passes capsules to the firmware with both virtual and
> physical mapping. Depending on the intended
>    consumption, the firmware may process the capsule
> immediately. If the payload should persist @@ -71,6
> +77,10 @@ UpdateCapsule (
>    CHAR16                    CapsuleVarName[30];
>    CHAR16                    *TempVarName;
> 
> +  if (!(mRuntimeServicesSupported |
> EFI_RT_SUPPORTED_UPDATE_CAPSULE)) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
>    //
>    // Check if platform support Capsule In RAM or not.
>    // Platform could choose to drop CapsulePei/CapsuleX64
> and do not support Capsule In RAM.
> @@ -259,6 +269,10 @@ QueryCapsuleCapabilities (
>    EFI_CAPSULE_HEADER        *CapsuleHeader;
>    BOOLEAN                   NeedReset;
> 
> +  if (!(mRuntimeServicesSupported |
> EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES)) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
>    //
>    // Capsule Count can't be less than one.
>    //
> @@ -343,6 +357,106 @@ QueryCapsuleCapabilities (
>    return EFI_SUCCESS;
>  }
> 
> +/**
> +  Set the L"RuntimeServicesSupported" variable depend on
> the pcd PcdRuntimeServicesSupport.
> +
> +  Firstly try to get the variable, it may be set in
> other dxe driver.
> + If it is set, then return  EFI_SUCCESS. If it isn't
> present, try to set it.
> +
> +  @retval EFI_SUCCESS       The variable is already set.
> +          EFI_NOT_FOUND     All runtime services are
> supported at runtime. No variable is set.
> +          Others            Error to get variable or set
> variable. Unexpected.
> +**/
> +static
> +EFI_STATUS
> +SetRuntimeServicesSupported (
> +  VOID
> +  )
> +{
> +  EFI_STATUS    Status;
> +  UINT16        RuntimeServicesSupported;
> +  UINT32        Attributes;
> +  UINTN         DataSize;
> +
> +  //
> +  // Firstly try to get L"RuntimeServicesSupported"
> variable  //
> + Attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS;  DataSize = sizeof
> (UINT16);  Status =
> + gRT->GetVariable (
> +                  L"RuntimeServicesSupported",
> +                  &gEfiGlobalVariableGuid,
> +                  &Attributes,
> +                  &DataSize,
> +                  &RuntimeServicesSupported
> +                  );
> +
> +  if (Status == EFI_NOT_FOUND) {
> +    //
> +    // L"RuntimeServicesSupported" isn't set yet. Then
> set it if
> +    // some of the RuntimeServices is unsupported.
> +    //
> +    RuntimeServicesSupported = PcdGet16
> (PcdRuntimeServicesSupport);
> +    if (RuntimeServicesSupported != 0x3FFF) {
> +      Status = gRT->SetVariable (
> +                      L"RuntimeServicesSupported",
> +                      &gEfiGlobalVariableGuid,
> +                      EFI_VARIABLE_BOOTSERVICE_ACCESS |
> EFI_VARIABLE_RUNTIME_ACCESS,
> +                      sizeof (UINT16),
> +                      &RuntimeServicesSupported
> +                      );
> +    } else {
> +      //
> +      // Set Status to EFI_NOT_FOUND to indicate not
> such variable
> +      //
> +      Status = EFI_NOT_FOUND;
> +    }
> +  }
> +
> +  return Status;
> +}
> +
> +/**
> +  ExitBootServices Event to update the
> mRuntimeServicesSupported to
> +  affect the runtime services.
> +
> +  @param[in]  Event     Event whose notification
> function is being invoked
> +  @param[in]  Context   Pointer to the notification
> function's context
> +**/
> +static
> +VOID
> +UpdateRuntimeServicesSupported (
> +  IN      EFI_EVENT                 Event,
> +  IN      VOID                      *Context
> +  )
> +{
> +  EFI_STATUS    Status;
> +  UINT16        RuntimeServicesSupported;
> +  UINT32        Attributes;
> +  UINTN         DataSize;
> +
> +  Attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS;  DataSize = sizeof
> (UINT16);  Status =
> + gRT->GetVariable (
> +                  L"RuntimeServicesSupported",
> +                  &gEfiGlobalVariableGuid,
> +                  &Attributes,
> +                  &DataSize,
> +                  &RuntimeServicesSupported
> +                  );
> +
> +  if (!EFI_ERROR (Status)) {
> +    mRuntimeServicesSupported =
> RuntimeServicesSupported;
> +  } else if (Status == EFI_NOT_FOUND) {
> +    mRuntimeServicesSupported = 0x3FFF;
> +  } else {
> +    //
> +    // Should never arrive here.
> +    //
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +}
> +
> 
>  /**
> 
> @@ -362,6 +476,7 @@ CapsuleServiceInitialize (
>    )
>  {
>    EFI_STATUS  Status;
> +  EFI_EVENT   Event;
> 
>    mMaxSizePopulateCapsule =
> PcdGet32(PcdMaxSizePopulateCapsule);
>    mMaxSizeNonPopulateCapsule =
> PcdGet32(PcdMaxSizeNonPopulateCapsule);
> @@ -393,5 +508,19 @@ CapsuleServiceInitialize (
>                    );
>    ASSERT_EFI_ERROR (Status);
> 
> +  Status = SetRuntimeServicesSupported ();
> +
> +  ASSERT (Status == EFI_NOT_FOUND || Status ==
> EFI_SUCCESS);
> +
> +  Status = gBS->CreateEventEx(
> +                  EVT_NOTIFY_SIGNAL,
> +                  TPL_NOTIFY,
> +                  UpdateRuntimeServicesSupported,
> +                  NULL,
> +                  &gEfiEventExitBootServicesGuid,
> +                  &Event
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
>    return Status;
>  }
> --
> 2.21.0.windows.1
> 
> 
> 

Re: [edk2-devel] [PATCH 3/3] MdeModulePkg/CapsuleRuntimeDxe: Control runtime services supported

[Gao, Zhichao]

> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, July 17, 2019 11:41 PM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: RE: [edk2-devel] [PATCH 3/3] MdeModulePkg/CapsuleRuntimeDxe:
> Control runtime services supported
> 
> Zhichao,
> 
> Why is CapsuleRuntimeDxe the component that is responsible for setting the
> UEFI Variable for supported RT services?
> 
> I would think this should be in:
> 
> 	MdeModulePkg\Core\RuntimeDxe

Sorry. I didn't think of RuntimeDxe when I work on the patch. I assume the variable may be set in some dxe driver (may be a platform drive not in the edk repo) and capsule driver would check that and then set it if it isn't set yet.
Agree with you, RuntimeDxe is a better place to set the variable. Once the setting variable code is added to edk repo, there would be no responsibility for others to set the variable.

Thanks,
Zhichao

> 
> I agree that each RT driver that populates the RT Services Table with a RT
> services can consume the new bitmask PCD and use the PCD to determine if
> the RT Service should return EFI_UNSUPPORTED after ExitBootServices().
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Gao, Zhichao
> > Sent: Wednesday, July 17, 2019 12:37 AM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> > <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean Brogan
> > <sean.brogan@microsoft.com>; Michael Turner
> > <Michael.Turner@microsoft.com>; Bret Barkelew
> > <Bret.Barkelew@microsoft.com>
> > Subject: [edk2-devel] [PATCH 3/3]
> > MdeModulePkg/CapsuleRuntimeDxe: Control runtime services supported
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1907
> >
> > Add PcdRuntimeServicesSupport to control whether the capsule services
> > is supported during runtime phase. If the L"RuntimeServicesSupported"
> > variable is not set yet, it would set the variable base on the pcd.
> > If the pcd value is 0x3FFF that means all runtime services is
> > supported at runtime phase, L"RuntimeServicesSupported" would not be
> > set.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Liming gao <liming.gao@intel.com>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > ---
> >  .../CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf   |   3 +
> >  .../CapsuleRuntimeDxe/CapsuleService.c        | 129
> > ++++++++++++++++++
> >  2 files changed, 132 insertions(+)
> >
> > diff --git
> > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime
> > Dxe.inf
> > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime
> > Dxe.inf
> > index 9da450722b..95b760d94e 100644
> > ---
> > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime
> > Dxe.inf
> > +++
> > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime
> > Dxe.inf
> > @@ -72,6 +72,8 @@
> >    ## SOMETIMES_PRODUCES   ##
> > Variable:L"CapsuleLongModeBuffer" # The long mode buffer used by IA32
> > Capsule PEIM to call X64 CapsuleCoalesce code to handle >4GB capsule
> > blocks
> >    gEfiCapsuleVendorGuid
> >    gEfiFmpCapsuleGuid                            ##
> > SOMETIMES_CONSUMES   ## GUID # FMP capsule GUID
> > +  gEfiGlobalVariableGuid                        ##
> > SOMETIMES_CONSUMES   ## Variable
> > L"RuntimeServicesSupported"
> > +  gEfiEventExitBootServicesGuid                 ##
> > CONSUMES
> >
> >  [Protocols]
> >    gEfiCapsuleArchProtocolGuid                   ##
> > PRODUCES
> > @@ -91,6 +93,7 @@
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsu
> > le   ## SOMETIMES_CONSUMES
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizePopulateCapsule
> > ## SOMETIMES_CONSUMES # Populate Image requires reset support.
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleInRamSupport
> > ## CONSUMES
> > +  gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupport
> > ## SOME_TIMES_CONSUMES
> >
> >  [Pcd.X64]
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdCapsulePeiLongModeStack
> > Size   ## SOMETIMES_CONSUMES
> > diff --git
> > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService
> > .c
> > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService
> > .c
> > index 77b8f00062..8ab77361d8 100644
> > ---
> > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService
> > .c
> > +++
> > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService
> > .c
> > @@ -24,6 +24,12 @@ UINTN       mTimes      = 0;
> >  UINT32      mMaxSizePopulateCapsule     = 0;
> >  UINT32      mMaxSizeNonPopulateCapsule  = 0;
> >
> > +//
> > +// Runtime Services Supported Flag
> > +// Initialize it to 0x3FFF to indicate it is all
> > supported before
> > +runtime //
> > +static UINT16     mRuntimeServicesSupported = 0x3FFF;
> > +
> >  /**
> >    Passes capsules to the firmware with both virtual and physical
> > mapping. Depending on the intended
> >    consumption, the firmware may process the capsule immediately. If
> > the payload should persist @@ -71,6
> > +77,10 @@ UpdateCapsule (
> >    CHAR16                    CapsuleVarName[30];
> >    CHAR16                    *TempVarName;
> >
> > +  if (!(mRuntimeServicesSupported |
> > EFI_RT_SUPPORTED_UPDATE_CAPSULE)) {
> > +    return EFI_UNSUPPORTED;
> > +  }
> > +
> >    //
> >    // Check if platform support Capsule In RAM or not.
> >    // Platform could choose to drop CapsulePei/CapsuleX64 and do not
> > support Capsule In RAM.
> > @@ -259,6 +269,10 @@ QueryCapsuleCapabilities (
> >    EFI_CAPSULE_HEADER        *CapsuleHeader;
> >    BOOLEAN                   NeedReset;
> >
> > +  if (!(mRuntimeServicesSupported |
> > EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES)) {
> > +    return EFI_UNSUPPORTED;
> > +  }
> > +
> >    //
> >    // Capsule Count can't be less than one.
> >    //
> > @@ -343,6 +357,106 @@ QueryCapsuleCapabilities (
> >    return EFI_SUCCESS;
> >  }
> >
> > +/**
> > +  Set the L"RuntimeServicesSupported" variable depend on
> > the pcd PcdRuntimeServicesSupport.
> > +
> > +  Firstly try to get the variable, it may be set in
> > other dxe driver.
> > + If it is set, then return  EFI_SUCCESS. If it isn't
> > present, try to set it.
> > +
> > +  @retval EFI_SUCCESS       The variable is already set.
> > +          EFI_NOT_FOUND     All runtime services are
> > supported at runtime. No variable is set.
> > +          Others            Error to get variable or set
> > variable. Unexpected.
> > +**/
> > +static
> > +EFI_STATUS
> > +SetRuntimeServicesSupported (
> > +  VOID
> > +  )
> > +{
> > +  EFI_STATUS    Status;
> > +  UINT16        RuntimeServicesSupported;
> > +  UINT32        Attributes;
> > +  UINTN         DataSize;
> > +
> > +  //
> > +  // Firstly try to get L"RuntimeServicesSupported"
> > variable  //
> > + Attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS;  DataSize = sizeof
> > (UINT16);  Status =
> > + gRT->GetVariable (
> > +                  L"RuntimeServicesSupported",
> > +                  &gEfiGlobalVariableGuid,
> > +                  &Attributes,
> > +                  &DataSize,
> > +                  &RuntimeServicesSupported
> > +                  );
> > +
> > +  if (Status == EFI_NOT_FOUND) {
> > +    //
> > +    // L"RuntimeServicesSupported" isn't set yet. Then
> > set it if
> > +    // some of the RuntimeServices is unsupported.
> > +    //
> > +    RuntimeServicesSupported = PcdGet16
> > (PcdRuntimeServicesSupport);
> > +    if (RuntimeServicesSupported != 0x3FFF) {
> > +      Status = gRT->SetVariable (
> > +                      L"RuntimeServicesSupported",
> > +                      &gEfiGlobalVariableGuid,
> > +                      EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > EFI_VARIABLE_RUNTIME_ACCESS,
> > +                      sizeof (UINT16),
> > +                      &RuntimeServicesSupported
> > +                      );
> > +    } else {
> > +      //
> > +      // Set Status to EFI_NOT_FOUND to indicate not
> > such variable
> > +      //
> > +      Status = EFI_NOT_FOUND;
> > +    }
> > +  }
> > +
> > +  return Status;
> > +}
> > +
> > +/**
> > +  ExitBootServices Event to update the
> > mRuntimeServicesSupported to
> > +  affect the runtime services.
> > +
> > +  @param[in]  Event     Event whose notification
> > function is being invoked
> > +  @param[in]  Context   Pointer to the notification
> > function's context
> > +**/
> > +static
> > +VOID
> > +UpdateRuntimeServicesSupported (
> > +  IN      EFI_EVENT                 Event,
> > +  IN      VOID                      *Context
> > +  )
> > +{
> > +  EFI_STATUS    Status;
> > +  UINT16        RuntimeServicesSupported;
> > +  UINT32        Attributes;
> > +  UINTN         DataSize;
> > +
> > +  Attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS;  DataSize = sizeof
> > (UINT16);  Status =
> > + gRT->GetVariable (
> > +                  L"RuntimeServicesSupported",
> > +                  &gEfiGlobalVariableGuid,
> > +                  &Attributes,
> > +                  &DataSize,
> > +                  &RuntimeServicesSupported
> > +                  );
> > +
> > +  if (!EFI_ERROR (Status)) {
> > +    mRuntimeServicesSupported =
> > RuntimeServicesSupported;
> > +  } else if (Status == EFI_NOT_FOUND) {
> > +    mRuntimeServicesSupported = 0x3FFF;
> > +  } else {
> > +    //
> > +    // Should never arrive here.
> > +    //
> > +    ASSERT_EFI_ERROR (Status);
> > +  }
> > +}
> > +
> >
> >  /**
> >
> > @@ -362,6 +476,7 @@ CapsuleServiceInitialize (
> >    )
> >  {
> >    EFI_STATUS  Status;
> > +  EFI_EVENT   Event;
> >
> >    mMaxSizePopulateCapsule =
> > PcdGet32(PcdMaxSizePopulateCapsule);
> >    mMaxSizeNonPopulateCapsule =
> > PcdGet32(PcdMaxSizeNonPopulateCapsule);
> > @@ -393,5 +508,19 @@ CapsuleServiceInitialize (
> >                    );
> >    ASSERT_EFI_ERROR (Status);
> >
> > +  Status = SetRuntimeServicesSupported ();
> > +
> > +  ASSERT (Status == EFI_NOT_FOUND || Status ==
> > EFI_SUCCESS);
> > +
> > +  Status = gBS->CreateEventEx(
> > +                  EVT_NOTIFY_SIGNAL,
> > +                  TPL_NOTIFY,
> > +                  UpdateRuntimeServicesSupported,
> > +                  NULL,
> > +                  &gEfiEventExitBootServicesGuid,
> > +                  &Event
> > +                  );
> > +  ASSERT_EFI_ERROR (Status);
> > +
> >    return Status;
> >  }
> > --
> > 2.21.0.windows.1
> >
> >
> > 

Re: [edk2-devel] [PATCH 0/3] MdePkg/MdeModulePkg: Introduce a pcd to control runtime services

[Laszlo Ersek]

Hello Zhichao,

On 07/17/19 09:37, Gao, Zhichao wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1907
> 
> UEFI spec 2.8 introduce a new variable L"RuntimeServicesSupported".
> If some runtime sevices is not supported at runtime phase, the variable
> should present at boot services. It is a bitmask value, the bit value of
> zero indicate the related runtime services is not supported at runtime
> phase.
> Add the difinition and use it to control Capsule runtime services.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> 
> Zhichao Gao (3):
>   MdePkg/UefiSpec.h: Add define of runtime services support
>   MdePkg: Indicate new pcd PcdRuntimeServicesSupport
>   MdeModulePkg/CapsuleRuntimeDxe: Control runtime services supported

Is a given platform expected to set the new PCD in accordance with other
platform settings?

Here's why I'm asking. OVMF does not support capsule services, as shown
by the following library class resolution in the OVMF DSC files:

  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

OVMF will return EFI_UNSUPPORTED when the capsule runtime services are
invoked.

I'm not sure if the new PCD (and new UEFI variable,
"RuntimeServicesSupported") place new requirements on OVMF. It looks
like they do.

(1) If you agree, can you please include a new patch for OvmfPkg that
clears the EFI_RT_SUPPORTED_UPDATE_CAPSULE and
RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES bits in the PCD, in all three
DSC files?

(2) And, the same applies to all three ArmVirtPkg platforms:
ArmVirtQemu, ArmVirtQemuKernel, and ArmVirtXen.

(3) Furthermore, we already seem to have a related (generic) PCD, namely
PcdCapsuleInRamSupport. If that PCD is set to FALSE, then
UpdateCapsule() in
"MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c" will return
EFI_UNSUPPORTED.

Should we introduce a consistency check somewhere, between the
capsule-related bits of the PcdRuntimeServicesSupport bitmask, and
PcdCapsuleInRamSupport?


I'm getting confused by the possible interactions between:
- PcdRuntimeServicesSupport
- PcdCapsuleInRamSupport
- and the CapsuleLib class resolution.

Thanks!
Laszlo

Re: [edk2-devel] [PATCH 0/3] MdePkg/MdeModulePkg: Introduce a pcd to control runtime services

[Gao, Zhichao]

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Wednesday, July 17, 2019 9:15 PM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: Re: [edk2-devel] [PATCH 0/3] MdePkg/MdeModulePkg: Introduce a
> pcd to control runtime services
> 
> Hello Zhichao,
> 
> On 07/17/19 09:37, Gao, Zhichao wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1907
> >
> > UEFI spec 2.8 introduce a new variable L"RuntimeServicesSupported".
> > If some runtime sevices is not supported at runtime phase, the
> > variable should present at boot services. It is a bitmask value, the
> > bit value of zero indicate the related runtime services is not
> > supported at runtime phase.
> > Add the difinition and use it to control Capsule runtime services.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> >
> > Zhichao Gao (3):
> >   MdePkg/UefiSpec.h: Add define of runtime services support
> >   MdePkg: Indicate new pcd PcdRuntimeServicesSupport
> >   MdeModulePkg/CapsuleRuntimeDxe: Control runtime services supported
> 
> Is a given platform expected to set the new PCD in accordance with other
> platform settings?

No. I add the new pcd because some platforms want to disable the capsule services at runtime phase, but I don't know the specific platforms, and the pcd is consistence with the Uefi spec 2.8.

> 
> Here's why I'm asking. OVMF does not support capsule services, as shown by
> the following library class resolution in the OVMF DSC files:
> 
> 
> CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.i
> nf
> 
> OVMF will return EFI_UNSUPPORTED when the capsule runtime services are
> invoked.
> 
> I'm not sure if the new PCD (and new UEFI variable,
> "RuntimeServicesSupported") place new requirements on OVMF. It looks
> like they do.
> 
> (1) If you agree, can you please include a new patch for OvmfPkg that clears
> the EFI_RT_SUPPORTED_UPDATE_CAPSULE and
> RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES bits in the PCD, in all three
> DSC files?
> 
> (2) And, the same applies to all three ArmVirtPkg platforms:
> ArmVirtQemu, ArmVirtQemuKernel, and ArmVirtXen.

This pcd is introduced for the platform to have a convenience way to disable their unwanted runtime services at runtime.
I am not sure which runtime services is unwanted in the above platforms. The new pcd should be set to a right value although disabling other runtime services is not implemented yet. If only capsule services are unwanted, I am fine to change it for them.

> 
> (3) Furthermore, we already seem to have a related (generic) PCD, namely
> PcdCapsuleInRamSupport. If that PCD is set to FALSE, then
> UpdateCapsule() in
> "MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c" will
> return EFI_UNSUPPORTED.
> 
> Should we introduce a consistency check somewhere, between the capsule-
> related bits of the PcdRuntimeServicesSupport bitmask, and
> PcdCapsuleInRamSupport?
> 
> 
> I'm getting confused by the possible interactions between:
> - PcdRuntimeServicesSupport

The new pcd PcdRuntimeServicesSupport is required because of the spec. And it only disable the unwanted runtime services at runtime without any effect on boot phase.
See the description above Uefi spec 2.8, Section 8.1.

> - PcdCapsuleInRamSupport

I didn't know the background of PcdCapsuleInRamSupport. But it only affects capsule services->CapsuleUpdate. It is introduced on https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Capsule-on-Disk-Introducation. And it affects both boot and runtime phase.

> - and the CapsuleLib class resolution.

The null-version may be added for build pass. And it disable capsule service in both boot and runtime phase. Those are just my thoughts.

Thanks,
Zhichao

> 
> Thanks!
> Laszlo
> 
>