Jeff, I think I forgot to ask a very basic question on the new variable RuntimeServicesSupported.
What if the variable claims SetVariable() is not supported but OS still calls SetVariable()? I think to behave in a consistent way, SetVariable() should reject to service. But I cannot find it in your patch. The similar thing was done for OsIndications variable. When firmware claims BOOT_TO_SETUP is not supported, the request is ignored even OS requests to boot to setup,. I suggest we change all runtime services implementation to return unsupported when the accordingly bit in the PCD is not set. Thanks, Ray > -----Original Message----- > From: Jeff Brasen <jbra...@nvidia.com> > Sent: Thursday, November 28, 2019 7:25 AM > To: devel@edk2.groups.io > Cc: Gao, Liming <liming....@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Wu, Hao A <hao.a...@intel.com>; Ni, Ray > <ray...@intel.com>; Gao, Zhichao <zhichao....@intel.com>; > jbra...@nvidia.com > Subject: [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set > RuntimeServicesSupported variable > > Add support for initializing and setting the UEFI 2.8 global variable > RuntimeServicesSupported based on the value of a PCD. > > Change-Id: I8fbd404d492ff8278466edde8aa37d203537318c > Signed-off-by: Jeff Brasen <jbra...@nvidia.com> > --- > MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 1 + > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 > +++++++++++++++++++++++++++++++- > MdePkg/MdePkg.dec | 18 ++++++++++++++++ > MdePkg/MdePkg.uni | 17 ++++++++++++++++ > 4 files changed, 70 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > index 9310b4d..52ec04f 100644 > --- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > +++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > @@ -90,6 +90,7 @@ > gEfiMdePkgTokenSpaceGuid.PcdUefiVariableDefaultPlatformLang ## > CONSUMES > gEfiMdePkgTokenSpaceGuid.PcdHardwareErrorRecordLevel ## > CONSUMES > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut ## > CONSUMES > + gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupported ## > CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor ## > CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision ## > CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand > ## CONSUMES > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > index d387dbe..16bc593 100644 > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > @@ -40,7 +40,8 @@ CHAR16 *mReadOnlyVariables[] = { > EFI_LANG_CODES_VARIABLE_NAME, > EFI_BOOT_OPTION_SUPPORT_VARIABLE_NAME, > EFI_HW_ERR_REC_SUPPORT_VARIABLE_NAME, > - EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME > + EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME, > + EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME > }; > > CHAR16 *mBdsLoadOptionName[] = { > @@ -626,6 +627,33 @@ BdsFormalizeOSIndicationVariable ( > > /** > > + Formalize RuntimeServicesSupported variable. > + > +**/ > +VOID > +BdsFormalizeRuntimeServicesSupportedVariable ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + UINT16 RuntimeServicesSupported; > + > + RuntimeServicesSupported = PcdGet16 (PcdRuntimeServicesSupported); > + Status = gRT->SetVariable ( > + EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME, > + &gEfiGlobalVariableGuid, > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > EFI_VARIABLE_RUNTIME_ACCESS, > + sizeof(RuntimeServicesSupported), > + &RuntimeServicesSupported > + ); > + // > + // Platform needs to make sure setting volatile variable before calling 3rd > party code shouldn't fail. > + // > + ASSERT_EFI_ERROR (Status); > +} > + > +/** > + > Validate variables. > > **/ > @@ -645,6 +673,11 @@ BdsFormalizeEfiGlobalVariable ( > // Validate OSIndication related variable. > // > BdsFormalizeOSIndicationVariable (); > + > + // > + // Validate Runtime Services Supported variable. > + // > + BdsFormalizeRuntimeServicesSupportedVariable (); > } > > /** > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec > index d022cc5..cdcb2f9 100644 > --- a/MdePkg/MdePkg.dec > +++ b/MdePkg/MdePkg.dec > @@ -2297,5 +2297,23 @@ > # @Prompt Boot Timeout (s) > > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x00 > 00002c > > + ## Bitmask of supported runtime services<BR> > + # BIT0 - GetTime > + # BIT1 - SetTime > + # BIT2 - GetWakeupTime > + # BIT3 - SetWakeupTime > + # BIT4 - GetVariable > + # BIT5 - GetNextVariableName > + # BIT6 - SetVariable > + # BIT7 - SetVirtualAddressMap > + # BIT8 - ConvertPointer > + # BIT9 - GetNextHighMonotonicCount > + # BIT10 - ResetSystem > + # BIT11 - UpdateCapsule > + # BIT12 - QueryCapsuleCapabilites > + # BIT13 - QueryVariableInfo > + # @Prompt Supported Runtime services bitmask. > + > gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupported|0x3FFF|UINT16 > |0x0000002e > + > [UserExtensions.TianoCore."ExtraFiles"] > MdePkgExtra.uni > diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni > index 5c1fa24..1edf681 100644 > --- a/MdePkg/MdePkg.uni > +++ b/MdePkg/MdePkg.uni > @@ -413,3 +413,20 @@ > > #string > STR_gEfiMdePkgTokenSpaceGuid_PcdUartDefaultReceiveFifoDepth_HELP > #language en-US "Indicates the receive FIFO depth of UART > controller.<BR><BR>" > > +#string > STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupported_PROMPT > #language en-US "Supported Runtime Services bitmask" > + > +#string > STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupported_HELP > #language en-US "Bitmask of supported runtime services<BR><BR>\n" > + > "BIT0 - GetTime<BR>\n" > + > "BIT1 - SetTime<BR>\n" > + > "BIT2 - GetWakeupTime<BR>\n" > + > "BIT3 - SetWakeupTime<BR>\n" > + > "BIT4 - GetVariable<BR>\n" > + > "BIT5 - > GetNextVariableName<BR>\n" > + > "BIT6 - SetVariable<BR>\n" > + > "BIT7 - > SetVirtualAddressMap<BR>\n" > + > "BIT8 - ConvertPointer<BR>\n" > + > "BIT9 - > GetNextHighMonotonicCount<BR>\n" > + > "BIT10 - ResetSystem<BR>\n" > + > "BIT11 - UpdateCapsule<BR>\n" > + > "BIT12 - > QueryCapsuleCapabilities<BR>\n" > + > "BIT13 - QueryVariableInfo<BR>" > -- > 2.7.4 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#51440): https://edk2.groups.io/g/devel/message/51440 Mute This Topic: https://groups.io/mt/63268513/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-