public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds
@ 2018-02-06 17:12 Leif Lindholm
  2018-02-06 17:12 ` [PATCH edk2-platforms 2/3] <Platform|Silicon>/Hisilicon: " Leif Lindholm
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Leif Lindholm @ 2018-02-06 17:12 UTC (permalink / raw)
  To: edk2-devel; +Cc: ard.biesheuvel

A set of mostly Ebl-related Pcds are about to be deleted from
edk2. Delete references to them here to keep platforms building.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
index 717f7332c9..b026ce3a42 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
@@ -257,20 +257,6 @@ [PcdsFeatureFlag.common]
   gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable|TRUE
   gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnostics2Disable|TRUE
 
-  #
-  # Control what commands are supported from the UI
-  # Turn these on and off to add features or save size
-  #
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE
-
-  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
-
   # Use the Vector Table location in CpuDxe. We will not copy the Vector Table at PcdCpuVectorBaseAddress
   gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE
 
@@ -333,10 +319,6 @@ [PcdsFixedAtBuild.common]
 
   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
 
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
-
   #
   # Optional feature to help prevent EFI memory map fragments
   # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH edk2-platforms 2/3] <Platform|Silicon>/Hisilicon: drop unused EmbeddedPkg Pcds
  2018-02-06 17:12 [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds Leif Lindholm
@ 2018-02-06 17:12 ` Leif Lindholm
  2018-02-06 17:12 ` [PATCH edk2-platforms 3/3] Silicon/Marvell: " Leif Lindholm
  2018-02-08 15:08 ` [PATCH edk2-platforms 1/3] Platform/ARM: " Alexei Fedorov
  2 siblings, 0 replies; 8+ messages in thread
From: Leif Lindholm @ 2018-02-06 17:12 UTC (permalink / raw)
  To: edk2-devel; +Cc: ard.biesheuvel

A set of mostly Ebl-related Pcds are about to be deleted from
edk2. Delete references to them here to keep platforms building.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/Hisilicon/HiKey/HiKey.dsc  | 18 ------------------
 Silicon/Hisilicon/Hisilicon.dsc.inc | 19 -------------------
 2 files changed, 37 deletions(-)

diff --git a/Platform/Hisilicon/HiKey/HiKey.dsc b/Platform/Hisilicon/HiKey/HiKey.dsc
index d3fb0832c4..e8cb957cf0 100644
--- a/Platform/Hisilicon/HiKey/HiKey.dsc
+++ b/Platform/Hisilicon/HiKey/HiKey.dsc
@@ -173,20 +173,6 @@ [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
 ################################################################################
 
 [PcdsFeatureFlag.common]
-  #
-  # Control what commands are supported from the UI
-  # Turn these on and off to add features or save size
-  #
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE
-
-  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
-
   # Use the Vector Table location in CpuDxe. We will not copy the Vector Table at PcdCpuVectorBaseAddress
   gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE
 
@@ -264,10 +250,6 @@ [PcdsFixedAtBuild.common]
 
   gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
 
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
-
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Alpha"
 
   #
diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc b/Silicon/Hisilicon/Hisilicon.dsc.inc
index cc2367351f..f9d2ffa10e 100644
--- a/Silicon/Hisilicon/Hisilicon.dsc.inc
+++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
@@ -225,22 +225,6 @@ [PcdsFeatureFlag.common]
   gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable|TRUE
   gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnostics2Disable|TRUE
 
-  #
-  # Control what commands are supported from the UI
-  # Turn these on and off to add features or save size
-  #
-  ## Set PcdEmbeddedMacBoot to FALSE, or console mode will be changed when
-  ## entering EBL and not restored when exiting.
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|FALSE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE
-
-  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
-
   # Use the Vector Table location in CpuDxe. We will not copy the Vector Table at PcdCpuVectorBaseAddress
   gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE
 
@@ -295,9 +279,6 @@ [PcdsFixedAtBuild.common]
 
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80000004
   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x06
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
 
   #
   # Optional feature to help prevent EFI memory map fragments
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH edk2-platforms 3/3] Silicon/Marvell: drop unused EmbeddedPkg Pcds
  2018-02-06 17:12 [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds Leif Lindholm
  2018-02-06 17:12 ` [PATCH edk2-platforms 2/3] <Platform|Silicon>/Hisilicon: " Leif Lindholm
@ 2018-02-06 17:12 ` Leif Lindholm
  2018-02-06 17:24   ` Ard Biesheuvel
  2018-02-08 15:08 ` [PATCH edk2-platforms 1/3] Platform/ARM: " Alexei Fedorov
  2 siblings, 1 reply; 8+ messages in thread
From: Leif Lindholm @ 2018-02-06 17:12 UTC (permalink / raw)
  To: edk2-devel; +Cc: ard.biesheuvel

A set of mostly Ebl-related Pcds are about to be deleted from
edk2. Delete references to them here to keep platforms building.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index bf11e7c813..ef70b52167 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -203,21 +203,6 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
 ################################################################################
 
 [PcdsFeatureFlag.common]
-
-  #
-  # Control what commands are supported from the UI
-  # Turn these on and off to add features or save size
-  #
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE
-
-  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
-
   # Use the Vector Table location in CpuDxe.
   # We will not copy the Vector Table at PcdCpuVectorBaseAddress
   gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE
@@ -314,10 +299,6 @@ [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
 !endif
 
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
-
   #
   # Optional feature to help prevent EFI memory map fragments
   # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH edk2-platforms 3/3] Silicon/Marvell: drop unused EmbeddedPkg Pcds
  2018-02-06 17:12 ` [PATCH edk2-platforms 3/3] Silicon/Marvell: " Leif Lindholm
@ 2018-02-06 17:24   ` Ard Biesheuvel
  2018-02-06 18:05     ` Leif Lindholm
  0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2018-02-06 17:24 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org

On 6 February 2018 at 17:12, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> A set of mostly Ebl-related Pcds are about to be deleted from
> edk2. Delete references to them here to keep platforms building.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

For the series:
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 19 -------------------
>  1 file changed, 19 deletions(-)
>
> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> index bf11e7c813..ef70b52167 100644
> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> @@ -203,21 +203,6 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
>  ################################################################################
>
>  [PcdsFeatureFlag.common]
> -
> -  #
> -  # Control what commands are supported from the UI
> -  # Turn these on and off to add features or save size
> -  #
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|TRUE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE
> -
> -  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
> -
>    # Use the Vector Table location in CpuDxe.
>    # We will not copy the Vector Table at PcdCpuVectorBaseAddress
>    gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE
> @@ -314,10 +299,6 @@ [PcdsFixedAtBuild.common]
>    gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
>  !endif
>
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
> -
>    #
>    # Optional feature to help prevent EFI memory map fragments
>    # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
> --
> 2.11.0
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH edk2-platforms 3/3] Silicon/Marvell: drop unused EmbeddedPkg Pcds
  2018-02-06 17:24   ` Ard Biesheuvel
@ 2018-02-06 18:05     ` Leif Lindholm
  0 siblings, 0 replies; 8+ messages in thread
From: Leif Lindholm @ 2018-02-06 18:05 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org

On Tue, Feb 06, 2018 at 05:24:33PM +0000, Ard Biesheuvel wrote:
> On 6 February 2018 at 17:12, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > A set of mostly Ebl-related Pcds are about to be deleted from
> > edk2. Delete references to them here to keep platforms building.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> 
> For the series:
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Thanks! Pushed as 4297639904..b75684ef78.

> > ---
> >  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 19 -------------------
> >  1 file changed, 19 deletions(-)
> >
> > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > index bf11e7c813..ef70b52167 100644
> > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > @@ -203,21 +203,6 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
> >  ################################################################################
> >
> >  [PcdsFeatureFlag.common]
> > -
> > -  #
> > -  # Control what commands are supported from the UI
> > -  # Turn these on and off to add features or save size
> > -  #
> > -  gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|TRUE
> > -  gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE
> > -  gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE
> > -  gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE
> > -  gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE
> > -  gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE
> > -  gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE
> > -
> > -  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
> > -
> >    # Use the Vector Table location in CpuDxe.
> >    # We will not copy the Vector Table at PcdCpuVectorBaseAddress
> >    gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE
> > @@ -314,10 +299,6 @@ [PcdsFixedAtBuild.common]
> >    gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
> >  !endif
> >
> > -  gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
> > -  gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
> > -  gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
> > -
> >    #
> >    # Optional feature to help prevent EFI memory map fragments
> >    # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
> > --
> > 2.11.0
> >


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds
  2018-02-06 17:12 [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds Leif Lindholm
  2018-02-06 17:12 ` [PATCH edk2-platforms 2/3] <Platform|Silicon>/Hisilicon: " Leif Lindholm
  2018-02-06 17:12 ` [PATCH edk2-platforms 3/3] Silicon/Marvell: " Leif Lindholm
@ 2018-02-08 15:08 ` Alexei Fedorov
  2018-02-08 17:45   ` Leif Lindholm
  2 siblings, 1 reply; 8+ messages in thread
From: Alexei Fedorov @ 2018-02-08 15:08 UTC (permalink / raw)
  To: Leif Lindholm, edk2-devel@lists.01.org
  Cc: ard.biesheuvel@linaro.org, Evan Lloyd, Sami Mujawar,
	Girish Pathak, Mitch Ishihara, Matteo Carlini

This patch causes Juno and other ARM platforms to raise an assert during MMU initialisation in ArmConfigureMmu()

(edk2\ArmPkg\Library\ArmMmuLib\AArch64\ArmMmuLibCore.c):


  ASSERT (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK ||
          TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK);


see debug output:

UEFI firmware (version  built at 12:01:21 on Feb  8 2018)
add-symbol-file n:\edk2\Build\ArmJuno\DEBUG_GCC5\AARCH64\ArmPlatformPkg\PrePi\PeiUniCore\DEBUG\ArmPlatformPrePiUniCore.dll 0xE0000800
ASSERT [ArmPlatformPrePiUniCore] n:\edk2\ArmPkg\Library\ArmMmuLib\AArch64\ArmMmuLibCore.c(744): TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK || TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK

because removal of
-  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
from ArmVExpress.dsc.inc now makes ArmPlatformGetVirtualMemoryMap () function in
\edk2-platforms\Platform\ARM\JunoPkg\Library\ArmJunoLib\ArmJunoMem.c

use default FALSE value defined in EmbeddedPkg.dec:


[PcdsFeatureFlag.common]
  gEmbeddedTokenSpaceGuid.PcdCacheEnable|FALSE|BOOLEAN|0x00000042


& set CacheAttributes to DDR_ATTRIBUTES_UNCACHED:

if (FeaturePcdGet(PcdCacheEnable) == TRUE) {
    CacheAttributes = DDR_ATTRIBUTES_CACHED;
  } else {
    CacheAttributes = DDR_ATTRIBUTES_UNCACHED;
  }


Thank you for your cooperation.

Alexei.

________________________________
From: edk2-devel <edk2-devel-bounces@lists.01.org> on behalf of Leif Lindholm <leif.lindholm@linaro.org>
Sent: 06 February 2018 17:12
To: edk2-devel@lists.01.org
Cc: ard.biesheuvel@linaro.org
Subject: [edk2] [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds

A set of mostly Ebl-related Pcds are about to be deleted from
edk2. Delete references to them here to keep platforms building.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
index 717f7332c9..b026ce3a42 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
@@ -257,20 +257,6 @@ [PcdsFeatureFlag.common]
   gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable|TRUE
   gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnostics2Disable|TRUE

-  #
-  # Control what commands are supported from the UI
-  # Turn these on and off to add features or save size
-  #
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE
-
-  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
-
   # Use the Vector Table location in CpuDxe. We will not copy the Vector Table at PcdCpuVectorBaseAddress
   gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE

@@ -333,10 +319,6 @@ [PcdsFixedAtBuild.common]

   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07

-  gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
-
   #
   # Optional feature to help prevent EFI memory map fragments
   # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
--
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds
  2018-02-08 15:08 ` [PATCH edk2-platforms 1/3] Platform/ARM: " Alexei Fedorov
@ 2018-02-08 17:45   ` Leif Lindholm
  2018-02-08 17:57     ` Ard Biesheuvel
  0 siblings, 1 reply; 8+ messages in thread
From: Leif Lindholm @ 2018-02-08 17:45 UTC (permalink / raw)
  To: Alexei Fedorov
  Cc: edk2-devel@lists.01.org, ard.biesheuvel@linaro.org, Evan Lloyd,
	Sami Mujawar, Girish Pathak, Mitch Ishihara, Matteo Carlini

On Thu, Feb 08, 2018 at 03:08:19PM +0000, Alexei Fedorov wrote:
> This patch causes Juno and other ARM platforms to raise an assert during MMU initialisation in ArmConfigureMmu()

Huh...

> (edk2\ArmPkg\Library\ArmMmuLib\AArch64\ArmMmuLibCore.c):
> 
> 
>   ASSERT (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK ||
>           TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK);
> 
> 
> see debug output:
>
> UEFI firmware (version  built at 12:01:21 on Feb  8 2018)
> add-symbol-file n:\edk2\Build\ArmJuno\DEBUG_GCC5\AARCH64\ArmPlatformPkg\PrePi\PeiUniCore\DEBUG\ArmPlatformPrePiUniCore.dll 0xE0000800
> ASSERT [ArmPlatformPrePiUniCore] n:\edk2\ArmPkg\Library\ArmMmuLib\AArch64\ArmMmuLibCore.c(744): TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK || TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK
> 
> because removal of
> -  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
> from ArmVExpress.dsc.inc now makes ArmPlatformGetVirtualMemoryMap () function in
> \edk2-platforms\Platform\ARM\JunoPkg\Library\ArmJunoLib\ArmJunoMem.c
> 
> use default FALSE value defined in EmbeddedPkg.dec:
> 
> 
> [PcdsFeatureFlag.common]
>   gEmbeddedTokenSpaceGuid.PcdCacheEnable|FALSE|BOOLEAN|0x00000042
> 
> 
> & set CacheAttributes to DDR_ATTRIBUTES_UNCACHED:
> 
> if (FeaturePcdGet(PcdCacheEnable) == TRUE) {
>     CacheAttributes = DDR_ATTRIBUTES_CACHED;
>   } else {
>     CacheAttributes = DDR_ATTRIBUTES_UNCACHED;
>   }

So. I spent a while scratching my head here over how i didn't spot
this when I did my final test build before generating the patches.

Turns out, somewhere amongst my rebasing,
gEmbeddedTokenSpaceGuid.PcdCacheEnable managed to reintroduce itself
into EmbeddedPkg.dec (I explicitly deleted it).

Since I _knew_ I'd deleted it, and all the platforms still built, I
didn't bother grepping for it through all of edk2-platforms.

Ho hum.

Well, the fix is trivial - just excise it from all platforms, and turn
all conditional statements into the unconditional execution of the
TRUE path. There is no code in public trees that actually does
anything different initialisation-wise depending on the value of this
Pcd.

Scream if you disagree.

> Thank you for your cooperation.

Thanks for reporting!

/
    Leif

> Alexei.
> 
> ________________________________
> From: edk2-devel <edk2-devel-bounces@lists.01.org> on behalf of Leif Lindholm <leif.lindholm@linaro.org>
> Sent: 06 February 2018 17:12
> To: edk2-devel@lists.01.org
> Cc: ard.biesheuvel@linaro.org
> Subject: [edk2] [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds
> 
> A set of mostly Ebl-related Pcds are about to be deleted from
> edk2. Delete references to them here to keep platforms building.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 18 ------------------
>  1 file changed, 18 deletions(-)
> 
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> index 717f7332c9..b026ce3a42 100644
> --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> @@ -257,20 +257,6 @@ [PcdsFeatureFlag.common]
>    gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable|TRUE
>    gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnostics2Disable|TRUE
> 
> -  #
> -  # Control what commands are supported from the UI
> -  # Turn these on and off to add features or save size
> -  #
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|TRUE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE
> -
> -  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
> -
>    # Use the Vector Table location in CpuDxe. We will not copy the Vector Table at PcdCpuVectorBaseAddress
>    gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE
> 
> @@ -333,10 +319,6 @@ [PcdsFixedAtBuild.common]
> 
>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
> 
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
> -
>    #
>    # Optional feature to help prevent EFI memory map fragments
>    # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
> --
> 2.11.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds
  2018-02-08 17:45   ` Leif Lindholm
@ 2018-02-08 17:57     ` Ard Biesheuvel
  0 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2018-02-08 17:57 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: Alexei Fedorov, edk2-devel@lists.01.org, Evan Lloyd, Sami Mujawar,
	Girish Pathak, Mitch Ishihara, Matteo Carlini

On 8 February 2018 at 17:45, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Feb 08, 2018 at 03:08:19PM +0000, Alexei Fedorov wrote:
>> This patch causes Juno and other ARM platforms to raise an assert during MMU initialisation in ArmConfigureMmu()
>
> Huh...
>
>> (edk2\ArmPkg\Library\ArmMmuLib\AArch64\ArmMmuLibCore.c):
>>
>>
>>   ASSERT (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK ||
>>           TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK);
>>
>>
>> see debug output:
>>
>> UEFI firmware (version  built at 12:01:21 on Feb  8 2018)
>> add-symbol-file n:\edk2\Build\ArmJuno\DEBUG_GCC5\AARCH64\ArmPlatformPkg\PrePi\PeiUniCore\DEBUG\ArmPlatformPrePiUniCore.dll 0xE0000800
>> ASSERT [ArmPlatformPrePiUniCore] n:\edk2\ArmPkg\Library\ArmMmuLib\AArch64\ArmMmuLibCore.c(744): TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK || TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK
>>
>> because removal of
>> -  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
>> from ArmVExpress.dsc.inc now makes ArmPlatformGetVirtualMemoryMap () function in
>> \edk2-platforms\Platform\ARM\JunoPkg\Library\ArmJunoLib\ArmJunoMem.c
>>
>> use default FALSE value defined in EmbeddedPkg.dec:
>>
>>
>> [PcdsFeatureFlag.common]
>>   gEmbeddedTokenSpaceGuid.PcdCacheEnable|FALSE|BOOLEAN|0x00000042
>>
>>
>> & set CacheAttributes to DDR_ATTRIBUTES_UNCACHED:
>>
>> if (FeaturePcdGet(PcdCacheEnable) == TRUE) {
>>     CacheAttributes = DDR_ATTRIBUTES_CACHED;
>>   } else {
>>     CacheAttributes = DDR_ATTRIBUTES_UNCACHED;
>>   }
>
> So. I spent a while scratching my head here over how i didn't spot
> this when I did my final test build before generating the patches.
>
> Turns out, somewhere amongst my rebasing,
> gEmbeddedTokenSpaceGuid.PcdCacheEnable managed to reintroduce itself
> into EmbeddedPkg.dec (I explicitly deleted it).
>
> Since I _knew_ I'd deleted it, and all the platforms still built, I
> didn't bother grepping for it through all of edk2-platforms.
>
> Ho hum.
>
> Well, the fix is trivial - just excise it from all platforms, and turn
> all conditional statements into the unconditional execution of the
> TRUE path. There is no code in public trees that actually does
> anything different initialisation-wise depending on the value of this
> Pcd.
>
> Scream if you disagree.
>

My vote would be to get rid of it entirely. These days, uncached means
non-coherent, which breaks all sorts of assumptions in UEFI itself but
also in Trusted Firmware.


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-02-08 17:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-06 17:12 [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds Leif Lindholm
2018-02-06 17:12 ` [PATCH edk2-platforms 2/3] <Platform|Silicon>/Hisilicon: " Leif Lindholm
2018-02-06 17:12 ` [PATCH edk2-platforms 3/3] Silicon/Marvell: " Leif Lindholm
2018-02-06 17:24   ` Ard Biesheuvel
2018-02-06 18:05     ` Leif Lindholm
2018-02-08 15:08 ` [PATCH edk2-platforms 1/3] Platform/ARM: " Alexei Fedorov
2018-02-08 17:45   ` Leif Lindholm
2018-02-08 17:57     ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox