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

["Michael D Kinney" <michael.d.kinney@intel.com>]

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