[edk2,02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable

Message ID 20190305133248.4828-3-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • StandaloneMmPkg, ArmPkg: cleanups and improvements
Related show

Commit Message

Ard Biesheuvel March 5, 2019, 1:32 p.m.
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

Comments

Yao, Jiewen March 5, 2019, 1:55 p.m. | #1
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
Achin Gupta March 6, 2019, 3:16 p.m. | #2
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
Ard Biesheuvel March 6, 2019, 3:17 p.m. | #3
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
Achin Gupta March 6, 2019, 3:37 p.m. | #4
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
Ard Biesheuvel March 7, 2019, 10:09 a.m. | #5
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
Achin Gupta March 7, 2019, 11:14 a.m. | #6
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

Patch

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