| Message ID | 20190305133248.4828-3-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | StandaloneMmPkg, ArmPkg: cleanups and improvements | expand |
Reviewed-by: jiewen.yao@intel.com > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: Tuesday, March 5, 2019 5:33 AM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen <jiewen.yao@intel.com> > Subject: [edk2] [PATCH 02/10] StandaloneMmPkg: drop unused PCD > PcdStandaloneMmEnable > > The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the > first place since the value is implied by the context (it is never > valid to set it to FALSE for standalone MM or TRUE for traditional > MM). So drop it. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > StandaloneMmPkg/StandaloneMmPkg.dec > | 3 --- > StandaloneMmPkg/StandaloneMmPkg.dsc > | 3 --- > > StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalone > MmPeCoffExtraActionLib.inf | 3 --- > 3 files changed, 9 deletions(-) > > diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec > b/StandaloneMmPkg/StandaloneMmPkg.dec > index 0b5fbf9e1767..2d178c5e20a6 100644 > --- a/StandaloneMmPkg/StandaloneMmPkg.dec > +++ b/StandaloneMmPkg/StandaloneMmPkg.dec > @@ -39,6 +39,3 @@ [Guids] > gEfiStandaloneMmNonSecureBufferGuid = { 0xf00497e3, 0xbfa2, > 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }} > gEfiArmTfCpuDriverEpDescriptorGuid = { 0x6ecbd5a1, 0xc0f8, > 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }} > > -[PcdsFeatureFlag] > - > gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOL > EAN|0x00000001 > - > diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc > b/StandaloneMmPkg/StandaloneMmPkg.dsc > index e8d71ad56f52..f279d9e7e5c7 100644 > --- a/StandaloneMmPkg/StandaloneMmPkg.dsc > +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc > @@ -78,9 +78,6 @@ [LibraryClasses.AARCH64] > # Pcd Section - list of all EDK II PCD Entries defined by this Platform > # > > ############################################################## > ################## > -[PcdsFeatureFlag] > - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|TRUE > - > [PcdsFixedAtBuild] > gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x800000CF > gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff > diff --git > a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo > neMmPeCoffExtraActionLib.inf > b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo > neMmPeCoffExtraActionLib.inf > index eef3d7c6e253..181c15b9345a 100644 > --- > a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo > neMmPeCoffExtraActionLib.inf > +++ > b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo > neMmPeCoffExtraActionLib.inf > @@ -37,9 +37,6 @@ [Packages] > MdePkg/MdePkg.dec > StandaloneMmPkg/StandaloneMmPkg.dec > > -[FeaturePcd] > - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable > - > [LibraryClasses] > StandaloneMmMmuLib > PcdLib > -- > 2.20.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hi Ard, On Tue, Mar 05, 2019 at 02:32:40PM +0100, Ard Biesheuvel wrote: > The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the > first place since the value is implied by the context (it is never > valid to set it to FALSE for standalone MM or TRUE for traditional > MM). So drop it. This is being used to determine if the ArmVExpressPkg should include support for StMM comm. buffer or not [1] but it does look redundant now. cheers, Achin [1] https://lists.01.org/pipermail/edk2-devel/2018-December/034432.html > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > StandaloneMmPkg/StandaloneMmPkg.dec | 3 --- > StandaloneMmPkg/StandaloneMmPkg.dsc | 3 --- > StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf | 3 --- > 3 files changed, 9 deletions(-) > > diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec > index 0b5fbf9e1767..2d178c5e20a6 100644 > --- a/StandaloneMmPkg/StandaloneMmPkg.dec > +++ b/StandaloneMmPkg/StandaloneMmPkg.dec > @@ -39,6 +39,3 @@ [Guids] > gEfiStandaloneMmNonSecureBufferGuid = { 0xf00497e3, 0xbfa2, 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }} > gEfiArmTfCpuDriverEpDescriptorGuid = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }} > > -[PcdsFeatureFlag] > - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOLEAN|0x00000001 > - > diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc b/StandaloneMmPkg/StandaloneMmPkg.dsc > index e8d71ad56f52..f279d9e7e5c7 100644 > --- a/StandaloneMmPkg/StandaloneMmPkg.dsc > +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc > @@ -78,9 +78,6 @@ [LibraryClasses.AARCH64] > # Pcd Section - list of all EDK II PCD Entries defined by this Platform > # > ################################################################################ > -[PcdsFeatureFlag] > - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|TRUE > - > [PcdsFixedAtBuild] > gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x800000CF > gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff > diff --git a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf > index eef3d7c6e253..181c15b9345a 100644 > --- a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf > +++ b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf > @@ -37,9 +37,6 @@ [Packages] > MdePkg/MdePkg.dec > StandaloneMmPkg/StandaloneMmPkg.dec > > -[FeaturePcd] > - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable > - > [LibraryClasses] > StandaloneMmMmuLib > PcdLib > -- > 2.20.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, 6 Mar 2019 at 16:16, Achin Gupta <Achin.Gupta@arm.com> wrote: > > Hi Ard, > > On Tue, Mar 05, 2019 at 02:32:40PM +0100, Ard Biesheuvel wrote: > > The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the > > first place since the value is implied by the context (it is never > > valid to set it to FALSE for standalone MM or TRUE for traditional > > MM). So drop it. > > This is being used to determine if the ArmVExpressPkg should include support for > StMM comm. buffer or not [1] but it does look redundant now. > If that is the case, the PCD should be defined in that package. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Mar 06, 2019 at 04:17:51PM +0100, Ard Biesheuvel wrote: > On Wed, 6 Mar 2019 at 16:16, Achin Gupta <Achin.Gupta@arm.com> wrote: > > > > Hi Ard, > > > > On Tue, Mar 05, 2019 at 02:32:40PM +0100, Ard Biesheuvel wrote: > > > The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the > > > first place since the value is implied by the context (it is never > > > valid to set it to FALSE for standalone MM or TRUE for traditional > > > MM). So drop it. > > > > This is being used to determine if the ArmVExpressPkg should include support for > > StMM comm. buffer or not [1] but it does look redundant now. > > > > If that is the case, the PCD should be defined in that package. The Arm FVP port for StMM needs a rewrite on the lines of other platforms. This change is fine. Reviewed-by: achin.gupta@arm.com _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, 6 Mar 2019 at 16:37, Achin Gupta <Achin.Gupta@arm.com> wrote: > > On Wed, Mar 06, 2019 at 04:17:51PM +0100, Ard Biesheuvel wrote: > > On Wed, 6 Mar 2019 at 16:16, Achin Gupta <Achin.Gupta@arm.com> wrote: > > > > > > Hi Ard, > > > > > > On Tue, Mar 05, 2019 at 02:32:40PM +0100, Ard Biesheuvel wrote: > > > > The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the > > > > first place since the value is implied by the context (it is never > > > > valid to set it to FALSE for standalone MM or TRUE for traditional > > > > MM). So drop it. > > > > > > This is being used to determine if the ArmVExpressPkg should include support for > > > StMM comm. buffer or not [1] but it does look redundant now. > > > > > > > If that is the case, the PCD should be defined in that package. > > The Arm FVP port for StMM needs a rewrite on the lines of other platforms. This > change is fine. > Yes, you are right. SynQuacer also needs some tweaks to align with these changes, but I will post those separately. So with those changes merged, the only thing preventing us from building the SynQuacer + MM platform from upstream sources is the MmCommunicate VA vs PA issue. Is there any progress on that front? Thanks, Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Thu, Mar 07, 2019 at 11:09:35AM +0100, Ard Biesheuvel wrote: > On Wed, 6 Mar 2019 at 16:37, Achin Gupta <Achin.Gupta@arm.com> wrote: > > > > On Wed, Mar 06, 2019 at 04:17:51PM +0100, Ard Biesheuvel wrote: > > > On Wed, 6 Mar 2019 at 16:16, Achin Gupta <Achin.Gupta@arm.com> wrote: > > > > > > > > Hi Ard, > > > > > > > > On Tue, Mar 05, 2019 at 02:32:40PM +0100, Ard Biesheuvel wrote: > > > > > The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the > > > > > first place since the value is implied by the context (it is never > > > > > valid to set it to FALSE for standalone MM or TRUE for traditional > > > > > MM). So drop it. > > > > > > > > This is being used to determine if the ArmVExpressPkg should include support for > > > > StMM comm. buffer or not [1] but it does look redundant now. > > > > > > > > > > If that is the case, the PCD should be defined in that package. > > > > The Arm FVP port for StMM needs a rewrite on the lines of other platforms. This > > change is fine. > > > > Yes, you are right. SynQuacer also needs some tweaks to align with > these changes, but I will post those separately. > > So with those changes merged, the only thing preventing us from > building the SynQuacer + MM platform from upstream sources is the > MmCommunicate VA vs PA issue. Is there any progress on that front? I am looking at this now after my holiday. I have some questions that I will post separately. cheers, Achin > > Thanks, > Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec index 0b5fbf9e1767..2d178c5e20a6 100644 --- a/StandaloneMmPkg/StandaloneMmPkg.dec +++ b/StandaloneMmPkg/StandaloneMmPkg.dec @@ -39,6 +39,3 @@ [Guids] gEfiStandaloneMmNonSecureBufferGuid = { 0xf00497e3, 0xbfa2, 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }} gEfiArmTfCpuDriverEpDescriptorGuid = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }} -[PcdsFeatureFlag] - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOLEAN|0x00000001 - diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc b/StandaloneMmPkg/StandaloneMmPkg.dsc index e8d71ad56f52..f279d9e7e5c7 100644 --- a/StandaloneMmPkg/StandaloneMmPkg.dsc +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc @@ -78,9 +78,6 @@ [LibraryClasses.AARCH64] # Pcd Section - list of all EDK II PCD Entries defined by this Platform # ################################################################################ -[PcdsFeatureFlag] - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|TRUE - [PcdsFixedAtBuild] gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x800000CF gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff diff --git a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf index eef3d7c6e253..181c15b9345a 100644 --- a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf +++ b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf @@ -37,9 +37,6 @@ [Packages] MdePkg/MdePkg.dec StandaloneMmPkg/StandaloneMmPkg.dec -[FeaturePcd] - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable - [LibraryClasses] StandaloneMmMmuLib PcdLib
The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the first place since the value is implied by the context (it is never valid to set it to FALSE for standalone MM or TRUE for traditional MM). So drop it. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- StandaloneMmPkg/StandaloneMmPkg.dec | 3 --- StandaloneMmPkg/StandaloneMmPkg.dsc | 3 --- StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf | 3 --- 3 files changed, 9 deletions(-) -- 2.20.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel