Thanks for the clarification, Ray.

Regards,
Sunny Wang

-----Original Message-----
From: Ni, Ray [mailto:ray...@intel.com] 
Sent: Thursday, November 21, 2019 7:51 PM
To: devel@edk2.groups.io; Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; Gao, 
Zhichao <zhichao....@intel.com>; Jeff Brasen <jbra...@nvidia.com>; 
edk2-de...@lists.01.org
Cc: Gao, Liming <liming....@intel.com>; Kinney, Michael D 
<michael.d.kin...@intel.com>; Wu, Hao A <hao.a...@intel.com>; Spottswood, Jason 
<jason.spottsw...@hpe.com>
Subject: RE: [edk2-devel] [PATCH 3/3] MdeModulePkg/BdsDxe: Set 
RuntimeServicesSupported variable
Importance: High

Sunny,
I am fine to put it to dynamic section if there are real requirements.
I thought if the platform doesn't support runtime services, it always doesn't 
support.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wang, 
> Sunny (HPS SW)
> Sent: Thursday, November 21, 2019 7:27 PM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao....@intel.com>; Ni, 
> Ray <ray...@intel.com>; Jeff Brasen <jbra...@nvidia.com>; 
> edk2-de...@lists.01.org
> Cc: Gao, Liming <liming....@intel.com>; Kinney, Michael D 
> <michael.d.kin...@intel.com>; Wu, Hao A <hao.a...@intel.com>; 
> Spottswood, Jason <jason.spottsw...@hpe.com>; Wang, Sunny (HPS SW) 
> <sunnyw...@hpe.com>
> Subject: Re: [edk2-devel] [PATCH 3/3] MdeModulePkg/BdsDxe: Set 
> RuntimeServicesSupported variable
> 
> Hi Ray,
> 
> May I know why we need to put this PCD to [PcdsFixedAtBuild, 
> PcdsPatchableInModule] section only? If the reason is the security 
> concern, Locking the variable (value of PCD) at the EndOfDxe should be secure 
> enough. For the platforms that want to make it more secure (don't want the 
> PCD to be modified), they can override the PCD type in their .dsc file.
> I can imagine that there are still some use cases that need to modify 
> the PCD during boot. Can we put this PCD in [PcdsFixedAtBuild, 
> PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] to make it more flexible?
> 
> Regards,
> Sunny Wang
> 
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of 
> Gao, Zhichao
> Sent: Thursday, November 21, 2019 2:12 PM
> To: Ni, Ray <ray...@intel.com>; Jeff Brasen <jbra...@nvidia.com>; 
> edk2-de...@lists.01.org; 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>
> Subject: Re: [edk2-devel] [PATCH 3/3] MdeModulePkg/BdsDxe: Set 
> RuntimeServicesSupported variable
> 
> Agree with Ray, and we should update the uni file at the same time when add 
> the new pcd.
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Ni, Ray <ray...@intel.com>
> > Sent: Thursday, November 21, 2019 11:13 AM
> > To: Jeff Brasen <jbra...@nvidia.com>; edk2-de...@lists.01.org; 
> > 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>; Gao, 
> > Zhichao <zhichao....@intel.com>
> > Subject: RE: [PATCH 3/3] MdeModulePkg/BdsDxe: Set 
> > RuntimeServicesSupported variable
> >
> > Jeff,
> > I suggest you add the PCD definition to MdePkg.dec because this PCD 
> > just maps to the spec defined variable RuntimeServicesSupported.
> >
> > And can you put this PCD to [PcdsFixedAtBuild, 
> > PcdsPatchableInModule] section only?
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: Jeff Brasen <jbra...@nvidia.com>
> > > Sent: Saturday, November 16, 2019 1:43 AM
> > > To: edk2-de...@lists.01.org; devel@edk2.groups.io
> > > Cc: Jeff Brasen <jbra...@nvidia.com>; 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>
> > > Subject: [PATCH 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.
> > >
> > > Signed-off-by: Jeff Brasen <jbra...@nvidia.com>
> > > ---
> > >  MdeModulePkg/MdeModulePkg.dec            | 18 ++++++++++++++++
> > >  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf |  1 + 
> > > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35
> > > +++++++++++++++++++++++++++++++-
> > >  3 files changed, 53 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/MdeModulePkg/MdeModulePkg.dec 
> > > b/MdeModulePkg/MdeModulePkg.dec index 41b9e70..a1767e4 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > @@ -2003,6 +2003,24 @@
> > >    # @Prompt Capsule On Disk relocation device path.
> > >
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdCodRelocationDevPath|{0xFF}|VOI
> > > D*|0x0000002f
> > >
> > > +  ## 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.
> > > +
> > > +
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdRuntimeServicesSupported|0x3FFF
> > > |UINT
> > > + 16|0x00000030
> > > +
> > >  [PcdsPatchableInModule]
> > >    ## Specify memory size with page number for PEI code when
> > >    #  Loading Module at Fixed Address feature is enabled.
> > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > > b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > > index 9310b4d..e4ba9be 100644
> > > --- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > > +++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > > @@ -97,6 +97,7 @@
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed                       ##
> > > CONSUMES
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleOnDiskSupport              ##
> > > CONSUMES
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdPlatformRecoverySupport           ##
> > > CONSUMES
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdRuntimeServicesSupported
> > > ## CONSUMES
> > >
> > >  [Depex]
> > >    TRUE
> > > 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 ();
> > >  }
> > >
> > >  /**
> > > --
> > > 2.7.4
> 
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51168): https://edk2.groups.io/g/devel/message/51168
Mute This Topic: https://groups.io/mt/59298933/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to