[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] GCC build script change.

2017-05-15 Thread zwei4 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: zwei4 
---
 BuildBIOS.sh |  8 ++-
 Platform/BroxtonPlatformPkg/BuildBios.sh | 86 
 Platform/BroxtonPlatformPkg/BuildIFWI.sh |  4 +-
 3 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/BuildBIOS.sh b/BuildBIOS.sh
index 49a9e1b12..0dece1f77 100755
--- a/BuildBIOS.sh
+++ b/BuildBIOS.sh
@@ -8,6 +8,12 @@
 # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
 # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #
+Target_Flag=Release
+if [ "$1" == "Debug" ]; then
+  Target_Flag=Debug
+fi
+
+echo $Target_Flag
 
 export WORKSPACE=`pwd`
 export 
PACKAGES_PATH=$WORKSPACE:$WORKSPACE/Core:$WORKSPACE/Silicon/:$WORKSPACE/Platform:$WORKSPACE/Platform/BroxtonPlatformPkg:$WORKSPACE/Silicon/BroxtonSoC:$WORKSPACE/Platform/BroxtonPlatformPkg/Common
@@ -16,5 +22,5 @@ export 
PACKAGES_PATH=$WORKSPACE:$WORKSPACE/Core:$WORKSPACE/Silicon/:$WORKSPACE/P
 
 make -C BaseTools
 
-./Platform/BroxtonPlatformPkg/BuildIFWI.sh APLI Release
+. ./Platform/BroxtonPlatformPkg/BuildIFWI.sh APLI $Target_Flag
 
diff --git a/Platform/BroxtonPlatformPkg/BuildBios.sh 
b/Platform/BroxtonPlatformPkg/BuildBios.sh
index 3963c887a..88ea12f8c 100644
--- a/Platform/BroxtonPlatformPkg/BuildBios.sh
+++ b/Platform/BroxtonPlatformPkg/BuildBios.sh
@@ -76,19 +76,9 @@ cp $WORKSPACE/BaseTools/Conf/tools_def.template 
$WORKSPACE/Conf/tools_def.txt
 cp $WORKSPACE/BaseTools/Conf/build_rule.template $WORKSPACE/Conf/build_rule.txt
 
 
-## Get gcc version to determine which tool_def.template to use.
-## If gcc version is 4.6 or before, use default. If not, use new one.
-GCCVERSION=$(gcc --version | grep 'gcc' | grep '[0-9]' | cut -d ' ' -f 4 | cut 
-d '.' -f 2)
-if (($GCCVERSION > 6)); then
-  echo "GCC version is 4.7 or after"
-  TOOL_CHAIN_TAG=GCC47
-else
-  echo "Type 'gcc --version' to check version"
-  echo "Please update GCC version to 4.7 or later"
-  ErrorExit
-fi
 
-#make -C BaseTools > /dev/null
+
+TOOL_CHAIN_TAG=GCC5
 
 ## Define platform specific environment variables.
 PLATFORM_NAME=BroxtonPlatformPkg
@@ -268,28 +258,32 @@ build $Build_Flags
 ##**
 ## Post Build processing and cleanup
 ##**
+
+#
+# FSP Rebase and Split
+#
+#   0xFEF7A000 = gIntelFsp2WrapperTokenSpaceGuid.PcdFlashFvFspBase = 
$(CAR_BASE_ADDRESS) + $(BLD_RAM_DATA_SIZE) + $(FSP_RAM_DATA_SIZE) + 
$(FSP_EMP_DATA_SIZE) + $(BLD_IBBM_SIZE)
+pushd  $WORKSPACE/Silicon/BroxtonSoC/BroxtonFspPkg/ApolloLakeFspBinPkg/FspBin
+python $WORKSPACE/Core/IntelFsp2Pkg/Tools/SplitFspBin.py rebase -f 
ApolloLakeFsp.fd -c m -b 0xFEF7A000 -o ./ -n FSP.fd
+python $WORKSPACE/Core/IntelFsp2Pkg/Tools/SplitFspBin.py split -f FSP.fd -o ./ 
-n FSP.Fv
+popd
+cp -f 
$WORKSPACE/Silicon/BroxtonSoC/BroxtonFspPkg/ApolloLakeFspBinPkg/FspBin/FSP_T.Fv 
$WORKSPACE/Platform/BroxtonPlatformPkg/Common/Tools/Stitch
+cp -f 
$WORKSPACE/Silicon/BroxtonSoC/BroxtonFspPkg/ApolloLakeFspBinPkg/FspBin/FSP_M.Fv 
$WORKSPACE/Platform/BroxtonPlatformPkg/Common/Tools/Stitch
+cp -f 
$WORKSPACE/Silicon/BroxtonSoC/BroxtonFspPkg/ApolloLakeFspBinPkg/FspBin/FSP_S.Fv 
$WORKSPACE/Platform/BroxtonPlatformPkg/Common/Tools/Stitch
+
+
 grep "_PCD_VALUE_" 
$BUILD_PATH/IA32/BroxtonPlatformPkg/PlatformPei/PlatformPei/DEBUG/AutoGen.h > 
FlashMap.h
 
+
 #echo "Running fce..."
 ## Extract Hii data from build and store in HiiDefaultData.txt
 #wine PlatformTools/FCE/FCE.exe read -i $BUILD_PATH/FV/SOC.fd > 
$BUILD_PATH/FV/HiiDefaultData.txt 1>>EDK2.log 2>&1
 
 ## copy the Setup variable to the SetupDefault variable and save changes to 
BxtXXX.fd
 #wine PlatformTools/FCE/FCE.exe mirror -i $BUILD_PATH/FV/SOC.fd -o 
$BUILD_PATH/FV/Bxt"$Arch".fd Setup SetupDefault 1>>EDK2.log 2>&1
-echo "Skip FCE tool..."
+#echo "Skip FCE tool..."
 cp $BUILD_PATH/FV/SOC.fd $BUILD_PATH/FV/Bxt"$Arch".fd
 
-##echo Running KeyEnroll...
-## RestrictedBegin
-##if /i not "$Platform_Type" == "$eNB_RVP" (
-##   call $PLATFORM_PACKAGE/Restricted/Internal/Tools/KeyEnroll/KeyEnroll.bat  
$BUILD_PATH  $BUILD_PATH/FV/Vlv"$Arch".fd 1>>EDK2.log 2>&1
-##) else if /i "$Platform_Type" == "$eNB_RVP" (
-##   call 
$PLATFORM_PACKAGE/Restricted/Internal/Tools/KeyEnroll/BBAY-KeyEnroll.bat  
$BUILD_PATH  $BUILD_PATH/FV/Vlv"$Arch".fd 1>>EDK2.log 2>&1
-##)
-##   if %ERRORLEVEL% NEQ 0 goto BldFail
-## RestrictedEnd
-echo Skip "KeyEnroll tool..."
-
 ## Set the Board_Id, Build_Type, Version_Major, and Version_Minor environment 
variables
 ##find /v "#" Conf\BiosId.env > ver_strings
 ##for /f "tokens=1,3" %%i in (ver_strings) do set %%i=%%j
@@ -298,28 +292,36 @@ echo Skip "KeyEnroll tool..."
 VERSION_MAJOR=$(grep '^VERSION_MAJOR' Conf/BiosId.env | cut -d ' ' -f 3 | cut 
-c 1-4)
 VERSION_MINOR=$(grep '^VERSION_MINOR' Conf/BiosId.env | cut -d ' ' -f 3 | cut 
-c 1-2)
 
BIOS_Name="$BOARD_ID""$SV_String

Re: [edk2] [PATCH] MdeModulePkg/UfsPassThruDxe: Fix typo in UfsPassThruGetTargetLun()

2017-05-15 Thread Zeng, Star 
Reviewed-by: Star Zeng 

Thanks,
Star
-Original Message-
From: Wu, Hao A 
Sent: Tuesday, May 16, 2017 1:35 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Zeng, Star 
Subject: [PATCH] MdeModulePkg/UfsPassThruDxe: Fix typo in 
UfsPassThruGetTargetLun()

For function UfsPassThruGetTargetLun(), the length of the input device node 
specified by 'DevicePath' should be compared with the size of 'UFS_DEVICE_PATH' 
rather than the size of 'SCSI_DEVICE_PATH'.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c 
b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index 68a44367b5..e27f4fbab1 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -479,10 +479,10 @@ UfsPassThruGetTargetLun (
   }
 
   //
-  // Check whether the DevicePath belongs to SCSI_DEVICE_PATH
+  // Check whether the DevicePath belongs to UFS_DEVICE_PATH
   //
   if ((DevicePath->Type != MESSAGING_DEVICE_PATH) || (DevicePath->SubType != 
MSG_UFS_DP) ||
-  (DevicePathNodeLength(DevicePath) != sizeof(SCSI_DEVICE_PATH))) {
+  (DevicePathNodeLength(DevicePath) != sizeof(UFS_DEVICE_PATH))) {
 return EFI_UNSUPPORTED;
   }
 
--
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] MdeModulePkg/UfsPassThruDxe: Fix typo in UfsPassThruGetTargetLun()

2017-05-15 Thread Hao Wu 
For function UfsPassThruGetTargetLun(), the length of the input device
node specified by 'DevicePath' should be compared with the size of
'UFS_DEVICE_PATH' rather than the size of 'SCSI_DEVICE_PATH'.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c 
b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index 68a44367b5..e27f4fbab1 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -479,10 +479,10 @@ UfsPassThruGetTargetLun (
   }
 
   //
-  // Check whether the DevicePath belongs to SCSI_DEVICE_PATH
+  // Check whether the DevicePath belongs to UFS_DEVICE_PATH
   //
   if ((DevicePath->Type != MESSAGING_DEVICE_PATH) || (DevicePath->SubType != 
MSG_UFS_DP) ||
-  (DevicePathNodeLength(DevicePath) != sizeof(SCSI_DEVICE_PATH))) {
+  (DevicePathNodeLength(DevicePath) != sizeof(UFS_DEVICE_PATH))) {
 return EFI_UNSUPPORTED;
   }
 
-- 
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdePkg: Fix undefined behavior on variadic parameters

2017-05-15 Thread Gao, Liming 
Sergey:
  Could you give more detail on the undefined behavior on variadic parameters?

  I see https://bugzilla.tianocore.org/show_bug.cgi?id=410 describe this issues 
found in the latest CLANG tool chain. Do you find other tool chain reports it?

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Sergey 
> Temerkhanov
> Sent: Tuesday, May 16, 2017 10:57 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] MdePkg: Fix undefined behavior on variadic parameters
> 
> Fix undefined behavior by avoiding parameter type promotion
> 
> Signed-off-by: Sergey Temerkhanov 
> ---
>  MdePkg/Include/Library/UefiLib.h | 2 +-
>  MdePkg/Library/UefiLib/UefiLib.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Library/UefiLib.h 
> b/MdePkg/Include/Library/UefiLib.h
> index 0b14792..4e4697c 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -818,7 +818,7 @@ CHAR8 *
>  EFIAPI
>  GetBestLanguage (
>IN CONST CHAR8  *SupportedLanguages,
> -  IN BOOLEAN  Iso639Language,
> +  IN UINTN  Iso639Language,
>...
>);
> 
> diff --git a/MdePkg/Library/UefiLib/UefiLib.c 
> b/MdePkg/Library/UefiLib/UefiLib.c
> index a7eee01..74528ec 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.c
> +++ b/MdePkg/Library/UefiLib/UefiLib.c
> @@ -1514,7 +1514,7 @@ CHAR8 *
>  EFIAPI
>  GetBestLanguage (
>IN CONST CHAR8  *SupportedLanguages,
> -  IN BOOLEAN  Iso639Language,
> +  IN UINTN  Iso639Language,
>...
>)
>  {
> --
> 2.7.4
> 
> ___
> 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

Re: [edk2] [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)

2017-05-15 Thread Gary Lin 
On Mon, May 15, 2017 at 05:40:59PM -0700, Jordan Justen wrote:
> On 2017-05-12 01:40:34, Laszlo Ersek wrote:
> > On 05/12/17 04:02, Gary Lin wrote:
> > > On Mon, May 08, 2017 at 12:27:59PM +0800, Gary Lin wrote:
> > >> On Sat, May 06, 2017 at 09:30:18PM +0200, Laszlo Ersek wrote:
> > >>> (All hail Saturday!)
> > >>>
> > >>> Gary, can you please fetch this from my repo (URL & branch name below)
> > >>> and test it with Xen? Please test both the 4MB and the 2MB build. (I
> > >>> also tested both, with qemu + "-bios".)
> > >> Hi Laszlo,
> > >>
> > >> I have done some simples test with xen, and the 2MB build seems fine.
> > >> It booted into grub2 menu successfully. However, the 4MB build never 
> > >> boots.
> > >> The QEMU window showed less than 1 sec and then disappeared.
> > >>
> > >> Here is the snippet from 'xl dmesg'
> > >>
> > >> (d15)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... 
> > >> done.
> > >> (d15) Testing HVM environment:
> > >> (d15)  - REP INSB across page boundaries ... passed
> > >> (d15)  - GS base MSRs and SWAPGS ... passed
> > >> (d15) Passed 2 of 2 tests
> > >> (d15) Writing SMBIOS tables ...
> > >> (d15) Loading OVMF ...
> > >> (d15) no BIOS ROM image found
> > >> (d15) *** HVMLoader bug at hvmloader.c:381
> > >> (d15) *** HVMLoader crashed.
> > >>
> > >> I'm pretty sure that the ovmf path is right, so it seems Xen just 
> > >> rejected
> > >> the 4MB build :-\
> > >>
> > >> I'll try to dig more information.
> > >>
> > > There is a function in the xen hvmloader clearing the memory from
> > > 0x40 to 0x80. Unfortunately, the hvm_start_info struct of the
> > > 4MB OVMF was loaded to 0x588000, so the struct was cleared mistakenly
> > > and hvmloader cannot find the firmware. Xen is not ready for the 4MB
> > > build yet :-\
> > > 
> > > The discussion in xen-devel:
> > > https://lists.xen.org/archives/html/xen-devel/2017-05/msg01053.html
> > 
> > Thank you for the feedback!
> > 
> > In this case, I think we should drop the last patch from this series.
> 
> Can we come up with a plan for trying to fix this? Gary, would it be
> okay if we opened a bug and assigned it to you? Or, do you have
> another suggestion for a possible Xen owner?
> 
Jan Beulich (also a SUSE employee) is working on the patch(*), and it
works for me.

Cheers,

Gary Lin

(*) https://lists.xen.org/archives/html/xen-devel/2017-05/msg01242.html
> Thanks,
> 
> -Jordan
> 
> > 
> > However, your test results also confirm that the 2MB build continues to
> > work with Xen, which means that the reworking of the
> > EmuVariableFvbRuntimeDxe driver in this series, and the underlying
> > tweaks+cleanups series, cause no regression.
> > 
> > Can you please respond, with your "Regression-tested-by", to:
> > 
> > (1) the full series
> > 
> >   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> > 
> > (2) and patches 1 through 3 in this series? (Patch #4 is just
> > documentation, for which Tested-by would be strange.)
> > 
> > Thank you!
> > Laszlo
> > 
> > 
> > >>> Note: this series depends on:
> > >>>
> > >>>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> > >>>   https://lists.01.org/pipermail/edk2-devel/2017-May/010527.html
> > >>>
> > >>> and it has been pushed to my github repo as such.
> > >>>
> > >>> Repo:   https://github.com/lersek/edk2.git
> > >>> Branch: emu4k
> > >>>
> > >>> Cc: Gary Ching-Pang Lin 
> > >>> Cc: Jordan Justen 
> > >>>
> > >>> Thanks,
> > >>> Laszlo
> > >>>
> > >>> Laszlo Ersek (5):
> > >>>   OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
> > >>>   OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
> > >>>   OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
> > >>>   OvmfPkg/README: document 4MB flash layout
> > >>>   OvmfPkg: make the 4MB flash size the default (again)
> > >>>
> > >>>  OvmfPkg/OvmfPkgIa32.dsc|   2 +-
> > >>>  OvmfPkg/OvmfPkgIa32X64.dsc |   2 +-
> > >>>  OvmfPkg/OvmfPkgX64.dsc |   2 +-
> > >>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  26 ++-
> > >>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 191 +---
> > >>>  OvmfPkg/PlatformPei/Platform.c |  20 +-
> > >>>  OvmfPkg/README |  39 +++-
> > >>>  7 files changed, 143 insertions(+), 139 deletions(-)
> > >>>
> > >>> -- 
> > >>> 2.9.3
> > >>>
> > >>>
> > >> ___
> > >> 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
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] MdeModulePkg: Fix undefined behavior on variadic parameters

2017-05-15 Thread Sergey Temerkhanov 
Fix undefined behavior by avoiding automatic type promotion

Signed-off-by: Sergey Temerkhanov 
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 0a325de..03605b9 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -1611,7 +1611,7 @@ CHAR8 *
 EFIAPI
 VariableGetBestLanguage (
   IN CONST CHAR8  *SupportedLanguages,
-  IN BOOLEAN  Iso639Language,
+  IN UINTN  Iso639Language,
   ...
   )
 {
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] MdePkg: Fix undefined behavior on variadic parameters

2017-05-15 Thread Sergey Temerkhanov 
Fix undefined behavior by avoiding parameter type promotion

Signed-off-by: Sergey Temerkhanov 
---
 MdePkg/Include/Library/UefiLib.h | 2 +-
 MdePkg/Library/UefiLib/UefiLib.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index 0b14792..4e4697c 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -818,7 +818,7 @@ CHAR8 *
 EFIAPI
 GetBestLanguage (
   IN CONST CHAR8  *SupportedLanguages, 
-  IN BOOLEAN  Iso639Language,
+  IN UINTN  Iso639Language,
   ...
   );
 
diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
index a7eee01..74528ec 100644
--- a/MdePkg/Library/UefiLib/UefiLib.c
+++ b/MdePkg/Library/UefiLib/UefiLib.c
@@ -1514,7 +1514,7 @@ CHAR8 *
 EFIAPI
 GetBestLanguage (
   IN CONST CHAR8  *SupportedLanguages, 
-  IN BOOLEAN  Iso639Language,
+  IN UINTN  Iso639Language,
   ...
   )
 {
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] Arm: GICv3: Don't access GIC_ICDIPR for interrupts 0..31

2017-05-15 Thread Sergey Temerkhanov 
These registers are reserved for PPIs and unimplemented on
some architectures

Signed-off-by: Sergey Temerkhanov 
---
 ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c 
b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
index 8af97a9..dc6b896 100644
--- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
+++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
@@ -257,7 +257,7 @@ GicV3DxeInitialize (
 MmioOr32 (mGicDistributorBase + ARM_GIC_ICDDCR, ARM_GIC_ICDDCR_ARE);
   }
 
-  for (Index = 0; Index < mGicNumInterrupts; Index++) {
+  for (Index = 32; Index < mGicNumInterrupts; Index++) {
 GicV3DisableInterruptSource (&gHardwareInterruptV3Protocol, Index);
 
 // Set Priority
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/5] OvmfPkg: complete the 4MB flash image support ("-bios" / emulated variables)

2017-05-15 Thread Jordan Justen 
On 2017-05-12 01:40:34, Laszlo Ersek wrote:
> On 05/12/17 04:02, Gary Lin wrote:
> > On Mon, May 08, 2017 at 12:27:59PM +0800, Gary Lin wrote:
> >> On Sat, May 06, 2017 at 09:30:18PM +0200, Laszlo Ersek wrote:
> >>> (All hail Saturday!)
> >>>
> >>> Gary, can you please fetch this from my repo (URL & branch name below)
> >>> and test it with Xen? Please test both the 4MB and the 2MB build. (I
> >>> also tested both, with qemu + "-bios".)
> >> Hi Laszlo,
> >>
> >> I have done some simples test with xen, and the 2MB build seems fine.
> >> It booted into grub2 menu successfully. However, the 4MB build never boots.
> >> The QEMU window showed less than 1 sec and then disappeared.
> >>
> >> Here is the snippet from 'xl dmesg'
> >>
> >> (d15)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... done.
> >> (d15) Testing HVM environment:
> >> (d15)  - REP INSB across page boundaries ... passed
> >> (d15)  - GS base MSRs and SWAPGS ... passed
> >> (d15) Passed 2 of 2 tests
> >> (d15) Writing SMBIOS tables ...
> >> (d15) Loading OVMF ...
> >> (d15) no BIOS ROM image found
> >> (d15) *** HVMLoader bug at hvmloader.c:381
> >> (d15) *** HVMLoader crashed.
> >>
> >> I'm pretty sure that the ovmf path is right, so it seems Xen just rejected
> >> the 4MB build :-\
> >>
> >> I'll try to dig more information.
> >>
> > There is a function in the xen hvmloader clearing the memory from
> > 0x40 to 0x80. Unfortunately, the hvm_start_info struct of the
> > 4MB OVMF was loaded to 0x588000, so the struct was cleared mistakenly
> > and hvmloader cannot find the firmware. Xen is not ready for the 4MB
> > build yet :-\
> > 
> > The discussion in xen-devel:
> > https://lists.xen.org/archives/html/xen-devel/2017-05/msg01053.html
> 
> Thank you for the feedback!
> 
> In this case, I think we should drop the last patch from this series.

Can we come up with a plan for trying to fix this? Gary, would it be
okay if we opened a bug and assigned it to you? Or, do you have
another suggestion for a possible Xen owner?

Thanks,

-Jordan

> 
> However, your test results also confirm that the 2MB build continues to
> work with Xen, which means that the reworking of the
> EmuVariableFvbRuntimeDxe driver in this series, and the underlying
> tweaks+cleanups series, cause no regression.
> 
> Can you please respond, with your "Regression-tested-by", to:
> 
> (1) the full series
> 
>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> 
> (2) and patches 1 through 3 in this series? (Patch #4 is just
> documentation, for which Tested-by would be strange.)
> 
> Thank you!
> Laszlo
> 
> 
> >>> Note: this series depends on:
> >>>
> >>>   [edk2] [PATCH 0/7] OvmfPkg: small cleanups and tweaks
> >>>   https://lists.01.org/pipermail/edk2-devel/2017-May/010527.html
> >>>
> >>> and it has been pushed to my github repo as such.
> >>>
> >>> Repo:   https://github.com/lersek/edk2.git
> >>> Branch: emu4k
> >>>
> >>> Cc: Gary Ching-Pang Lin 
> >>> Cc: Jordan Justen 
> >>>
> >>> Thanks,
> >>> Laszlo
> >>>
> >>> Laszlo Ersek (5):
> >>>   OvmfPkg/EmuVariableFvbRuntimeDxe: strip trailing whitespace
> >>>   OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB
> >>>   OvmfPkg/PlatformPei: align EmuVariableNvStore at any page boundary
> >>>   OvmfPkg/README: document 4MB flash layout
> >>>   OvmfPkg: make the 4MB flash size the default (again)
> >>>
> >>>  OvmfPkg/OvmfPkgIa32.dsc|   2 +-
> >>>  OvmfPkg/OvmfPkgIa32X64.dsc |   2 +-
> >>>  OvmfPkg/OvmfPkgX64.dsc |   2 +-
> >>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  26 ++-
> >>>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 191 +---
> >>>  OvmfPkg/PlatformPei/Platform.c |  20 +-
> >>>  OvmfPkg/README |  39 +++-
> >>>  7 files changed, 143 insertions(+), 139 deletions(-)
> >>>
> >>> -- 
> >>> 2.9.3
> >>>
> >>>
> >> ___
> >> 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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/5] OvmfPkg/EmuVariableFvbRuntimeDxe: change block size to 4KB

2017-05-15 Thread Jordan Justen 
On 2017-05-06 12:30:20, Laszlo Ersek wrote:
> EmuVariableFvbRuntimeDxe currently produces a Firmware Volume Block
> protocol that is based on a block map of two blocks, each block having
> PcdFlashNvStorageFtwSpareSize for size.
> 
> (The total size is 2 * PcdFlashNvStorageFtwSpareSize.)
> 
> FaultTolerantWriteDxe in turn expects the block size to be a power of two.
> 
> In the 4MB build of OVMF, PcdFlashNvStorageFtwSpareSize is 264KB, which is
> not a power of two. In order to equip EmuVariableFvbRuntimeDxe for this
> build, shrink the block size to 4KB (EFI_PAGE_SIZE), and grow the block
> count from 2 to EFI_SIZE_TO_PAGES(2 * PcdFlashNvStorageFtwSpareSize). The
> total size remains
> 
>   2 * PcdFlashNvStorageFtwSpareSize
>   - * EFI_PAGE_SIZE
> EFI_PAGE_SIZE
> 
> Right now EmuVariableFvbRuntimeDxe open-codes the block count of 2 in
> various limit checks, so introduce a few new macros:
> - EMU_FVB_NUM_TOTAL_BLOCKS, for the LHS of the above product,
> - EMU_FVB_NUM_SPARE_BLOCKS for the half of that.
> 
> Also rework the FVB protocol members to support an arbitrary count of
> blocks.
> 
> Keep the invariant intact that the first half of the firmware volume hosts
> the variable store and the FTW working block, and that the second half
> maps the FTW spare area.
> 
> Cc: Jordan Justen 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=527
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h |  10 +-
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 147 +---
>  2 files changed, 75 insertions(+), 82 deletions(-)
> 
> diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h 
> b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> index 4247d21d72f8..beb11e3f9a90 100644
> --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.h
> @@ -58,8 +58,14 @@ typedef struct {
>  //
>  // Constants
>  //
> -#define EMU_FVB_BLOCK_SIZE (FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
> -#define EMU_FVB_SIZE (2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
> +#define EMU_FVB_BLOCK_SIZE \
> +  EFI_PAGE_SIZE
> +#define EMU_FVB_NUM_SPARE_BLOCKS \
> +  EFI_SIZE_TO_PAGES ((UINTN)FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
> +#define EMU_FVB_NUM_TOTAL_BLOCKS \
> +  (2 * EMU_FVB_NUM_SPARE_BLOCKS)
> +#define EMU_FVB_SIZE \
> +  (EMU_FVB_NUM_TOTAL_BLOCKS * EMU_FVB_BLOCK_SIZE)
>  #define FTW_WRITE_QUEUE_SIZE \
>(FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) - \
> sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER))
> diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c 
> b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> index 2f89632e5d75..11c8b1b75cb8 100644
> --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> @@ -74,8 +74,8 @@ EFI_FW_VOL_BLOCK_DEVICE mEmuVarsFvb = {
>  }
>},
>NULL, // BufferPtr
> -  FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize), // BlockSize
> -  2 * FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize), // Size
> +  EMU_FVB_BLOCK_SIZE, // BlockSize
> +  EMU_FVB_SIZE, // Size
>{ // FwVolBlockInstance
>  FvbProtocolGetAttributes,
>  FvbProtocolSetAttributes,
> @@ -185,14 +185,14 @@ FvbProtocolGetBlockSize (
>  {
>EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
>  
> -  if (Lba > 1) {
> +  if (Lba >= EMU_FVB_NUM_TOTAL_BLOCKS) {
>  return EFI_INVALID_PARAMETER;
>}
>  
>FvbDevice = FVB_DEVICE_FROM_THIS (This);
>  
>*BlockSize = FvbDevice->BlockSize;
> -  *NumberOfBlocks = (UINTN) (2 - (UINTN) Lba);
> +  *NumberOfBlocks = (UINTN)(EMU_FVB_NUM_TOTAL_BLOCKS - Lba);
>  
>return EFI_SUCCESS;
>  }
> @@ -322,68 +322,58 @@ FvbProtocolEraseBlocks (
>)
>  {
>EFI_FW_VOL_BLOCK_DEVICE *FvbDevice;
> -  VA_LIST args;
> +  VA_LIST Args;
>EFI_LBA StartingLba;
>UINTN   NumOfLba;
> -  UINT8   Erase;
> -  VOID*ErasePtr;
> +  UINT8   *ErasePtr;
>UINTN   EraseSize;
>  
>FvbDevice = FVB_DEVICE_FROM_THIS (This);
> -  Erase = 0;
> -
> -  VA_START (args, This);
>  
> +  //
> +  // Check input parameters
> +  //
> +  VA_START (Args, This);
>do {
> -StartingLba = VA_ARG (args, EFI_LBA);
> +StartingLba = VA_ARG (Args, EFI_LBA);
>  if (StartingLba == EFI_LBA_LIST_TERMINATOR) {
>break;
>  }
> +NumOfLba = VA_ARG (Args, UINTN);
>  
> -NumOfLba = VA_ARG (args, UINT32);

This bug seems mildly concerning. I guess real users are not going to
specify an lba count > 4G. Still I think you should break this fix
out, and perhaps notify other package owners:

$ git grep -e NumOfLba --and -e VA_ARG --and -e UINT32
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c:NumOfLba = VA_ARG 
(Args, UINT32);
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c:NumOfLba = VA_ARG 
(Args, UINT32);
DuetPkg/FvbRuntimeService/FWBlockService.c:

Re: [edk2] [PATCH] ShellPkg/Touch: Create file if it doesn't exist

2017-05-15 Thread Jeff Westfahl 

Hi Tim,

I think those are reasonable arguments. It's probably best to just leave 
the touch command alone. It sounds like we've gotten along fine with the 
existing behavior for quite a while.


Regards,
Jeff

On Mon, 15 May 2017, Tim Lewis wrote:


I don't think it is good to change the behavior of the tool beyond what is in the 
specification. Further, this tool has existed for quite a long time in the EDK shell and 
now the UEFI shell without this behavior. So the de-facto standard of this environment is 
"don't create". Leaving behind stray 0 byte files would be an unwanted side 
effect for existing scripts that use this command.

Further, this doesn't add any corresponding command-line options to , and 
certainly not without corresponding command-line options like -c to disable 
this behavior.

Thanks,

Tim


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jeff 
Westfahl
Sent: Monday, May 15, 2017 2:02 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Jaben Carsey 
Subject: [edk2] [PATCH] ShellPkg/Touch: Create file if it doesn't exist

The UEFI Shell touch command returns an error if a file to be touched doesn't 
exist. In other command line operating environments, the touch command 
typically creates a file if it doesn't exist. This patch updates the UEFI Shell 
touch command to follow this convention.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl 
---
ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c 
b/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
index 639346f..921de5a 100644
--- a/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
+++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
@@ -175,11 +175,13 @@ ShellCommandRunTouch (
  UINTN   ParamCount;
  EFI_SHELL_FILE_INFO *FileList;
  EFI_SHELL_FILE_INFO *Node;
+  SHELL_FILE_HANDLE   FileHandle;

  ProblemParam= NULL;
  ShellStatus = SHELL_SUCCESS;
  ParamCount  = 0;
  FileList= NULL;
+  FileHandle  = NULL;

  //
  // initialize the shell lib (we must be in non-auto-init...) @@ -226,9 
+228,17 @@ ShellCommandRunTouch (
 ){
Status = ShellOpenFileMetaArg((CHAR16*)Param, 
EFI_FILE_MODE_READ|EFI_FILE_MODE_WRITE, &FileList);
if (EFI_ERROR(Status)) {
-  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), 
gShellLevel3HiiHandle, L"touch", (CHAR16*)Param);
-  ShellStatus = SHELL_NOT_FOUND;
-  break;
+  //
+  // try to create the file in case it doesn't exist
+  //
+  gEfiShellProtocol->CreateFile(Param, 0, &FileHandle);
+  gEfiShellProtocol->CloseFile(FileHandle);
+  Status = ShellOpenFileMetaArg((CHAR16*)Param, 
EFI_FILE_MODE_READ|EFI_FILE_MODE_WRITE, &FileList);
+  if (EFI_ERROR(Status)) {
+ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), 
gShellLevel3HiiHandle, L"touch", (CHAR16*)Param);
+ShellStatus = SHELL_NOT_FOUND;
+break;
+  }
}
//
// make sure we completed the param parsing sucessfully...
--
2.7.4

___
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

Re: [edk2] [PATCH] ShellPkg/Touch: Create file if it doesn't exist

2017-05-15 Thread Tim Lewis 
I don't think it is good to change the behavior of the tool beyond what is in 
the specification. Further, this tool has existed for quite a long time in the 
EDK shell and now the UEFI shell without this behavior. So the de-facto 
standard of this environment is "don't create". Leaving behind stray 0 byte 
files would be an unwanted side effect for existing scripts that use this 
command.

Further, this doesn't add any corresponding command-line options to , and 
certainly not without corresponding command-line options like -c to disable 
this behavior.

Thanks,

Tim


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jeff 
Westfahl
Sent: Monday, May 15, 2017 2:02 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Jaben Carsey 
Subject: [edk2] [PATCH] ShellPkg/Touch: Create file if it doesn't exist

The UEFI Shell touch command returns an error if a file to be touched doesn't 
exist. In other command line operating environments, the touch command 
typically creates a file if it doesn't exist. This patch updates the UEFI Shell 
touch command to follow this convention.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl 
---
 ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c 
b/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
index 639346f..921de5a 100644
--- a/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
+++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
@@ -175,11 +175,13 @@ ShellCommandRunTouch (
   UINTN   ParamCount;
   EFI_SHELL_FILE_INFO *FileList;
   EFI_SHELL_FILE_INFO *Node;
+  SHELL_FILE_HANDLE   FileHandle;
 
   ProblemParam= NULL;
   ShellStatus = SHELL_SUCCESS;
   ParamCount  = 0;
   FileList= NULL;
+  FileHandle  = NULL;
 
   //
   // initialize the shell lib (we must be in non-auto-init...) @@ -226,9 
+228,17 @@ ShellCommandRunTouch (
  ){
 Status = ShellOpenFileMetaArg((CHAR16*)Param, 
EFI_FILE_MODE_READ|EFI_FILE_MODE_WRITE, &FileList);
 if (EFI_ERROR(Status)) {
-  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), 
gShellLevel3HiiHandle, L"touch", (CHAR16*)Param);  
-  ShellStatus = SHELL_NOT_FOUND;
-  break;
+  //
+  // try to create the file in case it doesn't exist
+  //
+  gEfiShellProtocol->CreateFile(Param, 0, &FileHandle);
+  gEfiShellProtocol->CloseFile(FileHandle);
+  Status = ShellOpenFileMetaArg((CHAR16*)Param, 
EFI_FILE_MODE_READ|EFI_FILE_MODE_WRITE, &FileList);
+  if (EFI_ERROR(Status)) {
+ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), 
gShellLevel3HiiHandle, L"touch", (CHAR16*)Param);
+ShellStatus = SHELL_NOT_FOUND;
+break;
+  }
 }
 //
 // make sure we completed the param parsing sucessfully...
--
2.7.4

___
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

[edk2] [PATCH] ShellPkg/Touch: Create file if it doesn't exist

2017-05-15 Thread Jeff Westfahl 
The UEFI Shell touch command returns an error if a file to be touched
doesn't exist. In other command line operating environments, the touch
command typically creates a file if it doesn't exist. This patch updates
the UEFI Shell touch command to follow this convention.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl 
---
 ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c 
b/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
index 639346f..921de5a 100644
--- a/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
+++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
@@ -175,11 +175,13 @@ ShellCommandRunTouch (
   UINTN   ParamCount;
   EFI_SHELL_FILE_INFO *FileList;
   EFI_SHELL_FILE_INFO *Node;
+  SHELL_FILE_HANDLE   FileHandle;
 
   ProblemParam= NULL;
   ShellStatus = SHELL_SUCCESS;
   ParamCount  = 0;
   FileList= NULL;
+  FileHandle  = NULL;
 
   //
   // initialize the shell lib (we must be in non-auto-init...)
@@ -226,9 +228,17 @@ ShellCommandRunTouch (
  ){
 Status = ShellOpenFileMetaArg((CHAR16*)Param, 
EFI_FILE_MODE_READ|EFI_FILE_MODE_WRITE, &FileList);
 if (EFI_ERROR(Status)) {
-  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), 
gShellLevel3HiiHandle, L"touch", (CHAR16*)Param);  
-  ShellStatus = SHELL_NOT_FOUND;
-  break;
+  //
+  // try to create the file in case it doesn't exist
+  //
+  gEfiShellProtocol->CreateFile(Param, 0, &FileHandle);
+  gEfiShellProtocol->CloseFile(FileHandle);
+  Status = ShellOpenFileMetaArg((CHAR16*)Param, 
EFI_FILE_MODE_READ|EFI_FILE_MODE_WRITE, &FileList);
+  if (EFI_ERROR(Status)) {
+ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), 
gShellLevel3HiiHandle, L"touch", (CHAR16*)Param);
+ShellStatus = SHELL_NOT_FOUND;
+break;
+  }
 }
 //
 // make sure we completed the param parsing sucessfully...
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 5/7] OvmfPkg/PlatformPei: don't allocate reserved mem varstore if SMM_REQUIRE

2017-05-15 Thread Jordan Justen 
On 2017-05-05 14:02:56, Laszlo Ersek wrote:
> For the emulated variable store, PlatformPei allocates reserved memory (as
> early as possible, so that the address remains the same during reboot),
> and PcdEmuVariableNvStoreReserved carries the address to
> EmuVariableFvbRuntimeDxe.
> 
> However, EmuVariableFvbRuntimeDxe is excluded from the SMM_REQUIRE build,
> and then noone consumes PcdEmuVariableNvStoreReserved. Don't waste
> reserved memory whenever that's the case.
> 
> (Even a dynamic default for PcdEmuVariableNvStoreReserved would be
> unnecessary; but that way the PcdSet64S() call in the
> ReserveEmuVariableNvStore() function doesn't compile.)
> 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
>  OvmfPkg/OvmfPkgIa32.dsc| 3 +++
>  OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++
>  OvmfPkg/OvmfPkgX64.dsc | 3 +++
>  OvmfPkg/PlatformPei/Platform.c | 4 +++-
>  4 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 64427716c53c..b46eef6cabc3 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -495,7 +495,10 @@ [PcdsFixedAtBuild]
>  
> 
>  
>  [PcdsDynamicDefault]
> +  # only set when
> +  #   ($(SMM_REQUIRE) == FALSE)
>gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
> +
>gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 887964cd27c2..08f471fbc542 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -501,7 +501,10 @@ [PcdsFixedAtBuild.X64]
>  
> 
>  
>  [PcdsDynamicDefault]
> +  # only set when
> +  #   ($(SMM_REQUIRE) == FALSE)

I don't think we should bother adding these comments into the .dsc.

Ultimately, I would prefer to always allocate this, even when SMM is
set to be required. It'd be nice if we could always fallback to
EmuFvb, but I understand that this might not be possible given how
difficult it is to determine if QEMU actually has SMM enabled. Anyway,
I think this patch makes sense until we can potentially fix that.
(Which may or may not be worth fixing.)

Series Reviewed-by: Jordan Justen 

>gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
> +
>gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index dc5fea3577d4..24053e5ff82d 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -500,7 +500,10 @@ [PcdsFixedAtBuild]
>  
> 
>  
>  [PcdsDynamicDefault]
> +  # only set when
> +  #   ($(SMM_REQUIRE) == FALSE)
>gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
> +
>gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
>gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
>gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 5e983a8dcea9..1b4dc00b0180 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -672,7 +672,9 @@ InitializePlatform (
>mHostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
>  
>if (mBootMode != BOOT_ON_S3_RESUME) {
> -ReserveEmuVariableNvStore ();
> +if (!FeaturePcdGet (PcdSmmSmramRequire)) {
> +  ReserveEmuVariableNvStore ();
> +}
>  PeiFvInitialization ();
>  MemMapInitialization ();
>  NoexecDxeInitialization ();
> -- 
> 2.9.3
> 
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] ShellPkg/Ls: Handle path specified from root

2017-05-15 Thread Carsey, Jaben 
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Jeff Westfahl [mailto:jeff.westf...@ni.com]
> Sent: Monday, May 15, 2017 10:18 AM
> To: edk2-devel@lists.01.org
> Cc: Jeff Westfahl ; Ni, Ruiyu ;
> Carsey, Jaben 
> Subject: [edk2][PATCH v2] ShellPkg/Ls: Handle path specified from root
> Importance: High
> 
> This fixes 'ls' when specifying a path from the root, like "ls \" from
> within a subfolder. Currently, 'ls' will append the specified path to the
> current working directory. The correct behavior is to start from the root
> of the currently selected filesystem.
> 
> Cc: Ruiyu Ni 
> Cc: Jaben Carsey 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Westfahl 
> ---
>  ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> index 8d33392..8eeb2c0 100644
> --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> @@ -685,6 +685,9 @@ ShellCommandRunLs (
>  ShellCommandLineFreeVarList (Package);
>  return SHELL_OUT_OF_RESOURCES;
>}
> +  if (PathName[0] == L'\\') {
> +while (PathRemoveLastItem(FullPath)) ;
> +  }
>Size = FullPath != NULL? StrSize(FullPath) : 0;
>StrnCatGrow(&FullPath, &Size, L"\\", 0);
>  }
> --
> 2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC v4 06/13] OvmfPkg:AmdSevDxe: add AmdSevDxe driver

2017-05-15 Thread Jordan Justen 
On 2017-05-11 11:01:57, Brijesh Singh wrote:
> 
> 
> On 05/11/2017 12:43 PM, Jordan Justen wrote:
> > On 2017-05-11 08:53:39, Laszlo Ersek wrote:
> >> (5) Please mention that the driver is being added to the APRIORI DXE
> >> file for a separate reason as well (not just for the early clearing of
> >> the C bit on MMIO/NonExistent): OvmfPkg's DXE phase modules that tailor
> >> their behavior to SEV presence will assume that the IOMMU protocol
> >> exported by this driver is available *at once*.
> >
> > What other code depends on this being run apriori?
> >
> 
> We basically need some kind of guarantee that this driver is run before any 
> other
> drivers or libs access MMIO register/buffers. In additional to clearing 
> encryption
> bit from MMIO spaces, the driver also installs IOMMU protocol. So far, IOMMU 
> protocol
> is directly consumed by PciHostBridgeDxe driver and QemuFwCfgDxeLib.

What about adding a NULL protocol named
gOvmfIoMmuDetectionProtocolGuid? (Better name suggestions welcomed. :)
Then we can use this protocol in a depex where needed.

Maybe we should consider naming the driver IoMmuDxe instead?

I think the generic PciRoot bridge driver shouldn't need this in the
depex because it will not start until the BDS phase, and the IoMmuDxe
driver would have been dispatched by then.

-Jordan

> To answer your question, any code which uses MMIO or DMA in Dxe phase depends 
> on this
> driver run as APRIORI.
> 
> -Brijesh
> ___
> 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

[edk2] [PATCH v2] ShellPkg/Ls: Handle path specified from root

2017-05-15 Thread Jeff Westfahl 
This fixes 'ls' when specifying a path from the root, like "ls \" from
within a subfolder. Currently, 'ls' will append the specified path to the
current working directory. The correct behavior is to start from the root
of the currently selected filesystem.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl 
---
 ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c 
b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
index 8d33392..8eeb2c0 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
@@ -685,6 +685,9 @@ ShellCommandRunLs (
 ShellCommandLineFreeVarList (Package);
 return SHELL_OUT_OF_RESOURCES;
   }
+  if (PathName[0] == L'\\') {
+while (PathRemoveLastItem(FullPath)) ;
+  }
   Size = FullPath != NULL? StrSize(FullPath) : 0;
   StrnCatGrow(&FullPath, &Size, L"\\", 0);
 }
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] UEFI HTTPS Boot Whitepaper on gitbook

2017-05-15 Thread Richardson, Brian 
The UEFI HTTPS Boot whitepaper, originally published as an Intel document, has 
been migrated to the TianoCore gitbook account. We plan to use gitbook format 
for all future TianoCore whitepapers. Please review and let us know if you have 
any feedback.

https://github.com/tianocore/tianocore.github.io/wiki/EDK%20II%20White%20papers

Thanks ... br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- @intel_brian 
(Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [staging/cadence-aarch64] Add readme for cadence-aarch64 branch

2017-05-15 Thread Scott Telford 
Announcing the creation of a new edk2-staging branch for Cadence peripheral
hardware support in AArch64 platforms.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Telford 
---
 Readme.MD | 27 +++
 1 file changed, 27 insertions(+)
 create mode 100644 Readme.MD

diff --git a/Readme.MD b/Readme.MD
new file mode 100644
index 000..e2ec063
--- /dev/null
+++ b/Readme.MD
@@ -0,0 +1,27 @@
+# Cadence IP Support Branch
+
+This branch will be used to contribute code to support Cadence
+hardware IP for AArch64-based platforms.
+
+## edk2-staging branch owners
+* Scott Telford 
+
+## Introduction 
+
+Cadence develops a variety of peripheral IP modules suitable for ARM
+SoC architectures. This branch is intended to provide a staging area
+for drivers and libraries developed to support Cadence peripherals
+that may be incorporated into various SoC architectures from other
+vendors, for later integration into the appropriate EDK2
+platforms. Initially, the focus will be support for the Cadence PCIe
+4.0 Root Port. For internal development and testing purposes, an EDK2
+platform has been developed for a minimal AArch64 system
+configuration, comprising a single Cortex-A53 processor, GIC-500,
+NIC-400, Cadence PCIe Root Port, and Cadence UART. This will be
+included in the branch to provide an example of driver/platform
+integration.
+
+## Related Modules
+The following modules are related to this branch:
+* CadencePkg   - Package containing Cadence-specific drivers and libraries
+* MdeModulePkg - Package containing generic drivers relevant to Cadence 
peripheral IP
-- 
2.2.2

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC] PCD: Extended SKU support 1 - inheritance

2017-05-15 Thread Tim Lewis 
Star --

Thanks for your work on this. I have reviewed this and it looks good, 
addressing all of our concerns.

Tim

-Original Message-
From: Zeng, Star [mailto:star.z...@intel.com] 
Sent: Monday, May 15, 2017 2:46 AM
To: Tim Lewis ; Kinney, Michael D 
; edk2-devel@lists.01.org
Cc: Gao, Liming ; Zeng, Star 
Subject: RE: [RFC] PCD: Extended SKU support 1 - inheritance

Thanks all for the comments to this RFC.

I have sent out the V2 of this RFC at 
https://www.mail-archive.com/edk2-devel@lists.01.org/msg25687.html.

Tim,
Could you help review the V2 of this RFC and provide feedback?

With the V2 of this RFC, the actual code for the runtime usage you provided 
will be like below.

UINT64 SkuId;
VOID *Resource;

SkuId = LibPcdGetSku ();

Resource = NULL;
while (!GetResourceForSkuId (SkuId, &Resource) && SkuId != 0) {
  SkuId = PcdGetParentSkuId (SkuId);
}
If (Resource == NULL) {
  // error, no resource found.
}


Thanks,
Star
-Original Message-
From: Kinney, Michael D
Sent: Saturday, May 6, 2017 12:07 AM
To: Zeng, Star ; Tim Lewis ; 
edk2-devel@lists.01.org; Kinney, Michael D 
Cc: Gao, Liming 
Subject: RE: [RFC] PCD: Extended SKU support 1 - inheritance

Star,

I think Tim's feedback is a request for an additional feature on top of this 
RFC.  I do not see any requests for changes to the proposed syntax extensions.  
I agree that implementing these new feature in an edk2-staging branch makes a 
lot of sense.
 
Since Tim's request is about access to the SKU relationships from FW modules 
when the FW module executes, let's discuss this from an API perspective instead 
of detailed autogen.  We can figure out what autogen is required to support the 
API.

When a platform boots with multiple SKUs enabled in the FW image, typically a 
platform PEIM detects what SKU is present can calls the PcdLib function 
LibPcdSetSku() to set the active SKU.

Then, that same module or other modules may want to know how the current SKU is 
related to other SKUs.  Those other modules can start with LibPcdGetSku() to 
get the active SKU.

This RFC introduces the concept that one SKU may be a derivative of another 
SKU, and may be used to reduce the total number PCD statements in a DSC file.  
This derivative concept can be thought of as parent to child link between the 2 
SKUs.  We can draw this visually as a tree with parent child relationships 
between all the SKUs.

Now from a module that calls LibPcdGetSku(), the module may want to know who 
the parent SKU is.  The information we have about the parent SKU is the SKU ID 
value.

A new API that may be useful here is something like:

UINTN
GetParentSku (
  IN UINTN  SkuId
  );

The algorithm that Tim provided below uses autogen and a loop to retrieve the 
parent SKU.

So the request here is to extend the autogen to support a lookup of the parent 
SKU given any valid SKU ID value.

Thanks,

Mike
 
> -Original Message-
> From: Zeng, Star
> Sent: Friday, May 5, 2017 3:08 AM
> To: Tim Lewis ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zeng, Star 
> 
> Subject: RE: [RFC] PCD: Extended SKU support 1 - inheritance
> 
> Tim,
> 
> In your code base, there are other tools beyond BaseTools to parse the 
> [SkuIds] section and SKUID_IDENTIFIER in DSC, right?
> 
> The boot time usage (use the value in DEFAULT SKU instead if the value 
> in specified SKU is not configured) for other resources in your code base is 
> similar to PCD, right?
> This RFC has no change to this boot time behavior that "use the value 
> in DEFAULT SKU instead if the value in specified SKU is not configured".
> So I am still not so clear about the benefit of the macro like #define 
> SKUID_IDENTIFIER_DEFAULT 2,1,0,1,0,0.
> 
> 
> The DSC syntax extension in this RFC is OPTIONAL, the backward 
> compatibility is expected to be kept, no current tools and code should be 
> impacted.
> As I know, there will be a staging branch proposed for the development 
> of PCD related RFCs (include Structure PCD, etc). Mike and Liming can help 
> confirm it.
> How about we have the development of this RFC to the staging branch 
> first, then you can help evaluate the syntax and provide feedback 
> further? :)
> 
> 
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Tim Lewis
> Sent: Friday, May 5, 2017 12:36 AM
> To: Zeng, Star ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: Re: [edk2] [RFC] PCD: Extended SKU support 1 - inheritance
> 
> Star --
> 
> I understand your use case. We use the same behavior for other resources 
> besides PCD.
> 
> All we are asking is that this data that is used by the build tools is 
> also available in some form at runtime.
> 
> Thanks,
> 
> Tim
> 
> 
> 
> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Thursday, May 04, 2017 6:42 AM
> To: Tim Lewis ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zeng, S

Re: [edk2] [RFC 0/2] PCD: Extended SKU support 2 - sub SKU

2017-05-15 Thread Zeng, Star 
Hi,

The development will be done on a staging branch if there is no any comments in 
the following week.
Anyway, feedback still can be provided during the development on the staging 
branch and merging to master finally. :)


Thanks,
Star
-Original Message-
From: Zeng, Star 
Sent: Tuesday, April 25, 2017 8:48 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Kinney, Michael D 
; Gao, Liming ; Zhu, Yonghong 

Subject: [RFC 0/2] PCD: Extended SKU support 2 - sub SKU

- Requirement
Simplify the PCDs configuring for multiple SKUs in DSC.


- Current limitation
There is no sub SKU support for now. The PCD statements for different platform 
components need to be centralized for different SKUs, and the relationship 
between different SKUs is hard to build, then it is also hard to maintain the 
PCD values(configuring new PCD value or updating PCD value) for different SKUs.


- Proposal: Support sub SKU set and combinations of them.
This proposal depends on the proposal “PCD: Extended SKU support 1 - 
inheritance” at 
https://lists.01.org/pipermail/edk2-devel/2017-April/010194.html.
This proposal only extends DSC [SkuIds] section syntax.
BaseTools update is needed to support the syntax extension, and no any change 
in PCD database and driver is required.

DSC syntax:
  [SkuIds]
SubSkuSetName|SubSkuName[|ParentSubSkuName]
  SubSkuSetName: string or 0, 0 is reserved for DEFAULT SKU, string means 
it is sub SKU set name.
  SubSkuName: string
  ParentSubSkuName: string, optional, defines which sub SKU the PCD value 
will derive from for this sub SKU. The PCD value will derive from DEFAULT SKU 
for this sub SKU if ParentSubSkuName is absent.
  There could be multiple SubSku entries with different SubSkuSetName.
  There could be multiple SubSku entries with same SubSkuSetName and 
different SubSkuName.

ComboName | (SubSkuSetName1 or SubSkuName1, SubSkuSetName2 or SubSkuName2, 
...)
  There could be multiple Combo entries for different boards.

  SKUID_IDENTIFIER in [Defines] section could be DEFAULT, ALL, or combo name if 
sub SKU and Combo are specified in [Skuids] section.

PcdLib.h:
  // Retrieves sub SKU value based on Combo name and sub SKU name.
  // This definition is for platform PEIM to easily get the hidden sub SKU 
values in the Combo.
  // BaseTools can make sure the unique of platform SKU value that can be 
calculated by adding up sub SKU values in the Combo.
  #define PcdSubSkuValueInCombo (ComboName, SubSkuName) 
_PCD_SUB_SKU_##ComboName_##SubSkuName

AutoGen.h:
  Macros for _PCD_SUB_SKU_##ComboName_##SubSkuName will be generated in 
AutoGen.h based on the statements in [SkuIds] section of dsc.


- Example:
Without sub SKU support: 
Check the example at [RFC 1/2] Example: The PCDs configuring for multiple SKUs 
with current SKU usage

With sub SKU support: 
Check the example at [RFC 2/2] Example: The PCDs configuring for multiple SKUs 
with sub SKU support

  Pseudo-code for platform to set SKU with sub SKU support:
  Note: The Combo and CPU/CS/FRU identifying should be hardware detection.
UINTN CpuSkuV;
UINTN CsSkuV;
UINTN FruSkuV;
UINTN PlatformSku;

if (Combo1) {
  if (CpuA) {
CpuSkuV = PcdSubSkuValueInCombo(Combo1, CpuSkuA);
  } else if (CpuB) {
CpuSkuV = PcdSubSkuValueInCombo(Combo1, CpuSkuB); 
  }
  // Similar logic to get CsSkuV and FruSkuV.
  // Calculate the platform SKU.
  PlatformSku = CpuSkuV + CsSkuV + FruSkuV
} else …

LibPcdSetSku (PlatformSku);


Cc: Michael Kinney 
Cc: Liming Gao 
Cc: Yonghong Zhu 

Star Zeng (2):
  Example: The PCDs configuring for multiple SKUs with current SKU usage
  Example: The PCDs configuring for multiple SKUs with sub SKU support

 Nt32Pkg/Nt32Pkg.dec |  9 +
 Nt32Pkg/Nt32Pkg.dsc | 41 +
 2 files changed, 50 insertions(+)

-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC] PCD: Extended SKU support 1 - inheritance

2017-05-15 Thread Zeng, Star 
Thanks all for the comments to this RFC.

I have sent out the V2 of this RFC at 
https://www.mail-archive.com/edk2-devel@lists.01.org/msg25687.html.

Tim,
Could you help review the V2 of this RFC and provide feedback?

With the V2 of this RFC, the actual code for the runtime usage you provided 
will be like below.

UINT64 SkuId;
VOID *Resource;

SkuId = LibPcdGetSku ();

Resource = NULL;
while (!GetResourceForSkuId (SkuId, &Resource) && SkuId != 0) {
  SkuId = PcdGetParentSkuId (SkuId);
}
If (Resource == NULL) {
  // error, no resource found.
}


Thanks,
Star
-Original Message-
From: Kinney, Michael D 
Sent: Saturday, May 6, 2017 12:07 AM
To: Zeng, Star ; Tim Lewis ; 
edk2-devel@lists.01.org; Kinney, Michael D 
Cc: Gao, Liming 
Subject: RE: [RFC] PCD: Extended SKU support 1 - inheritance

Star,

I think Tim's feedback is a request for an additional feature on top of this 
RFC.  I do not see any requests for changes to the proposed syntax extensions.  
I agree that implementing these new feature in an edk2-staging branch makes a 
lot of sense.
 
Since Tim's request is about access to the SKU relationships from FW modules 
when the FW module executes, let's discuss this from an API perspective instead 
of detailed autogen.  We can figure out what autogen is required to support the 
API.

When a platform boots with multiple SKUs enabled in the FW image, typically a 
platform PEIM detects what SKU is present can calls the PcdLib function 
LibPcdSetSku() to set the active SKU.

Then, that same module or other modules may want to know how the current SKU is 
related to other SKUs.  Those other modules can start with LibPcdGetSku() to 
get the active SKU.

This RFC introduces the concept that one SKU may be a derivative of another 
SKU, and may be used to reduce the total number PCD statements in a DSC file.  
This derivative concept can be thought of as parent to child link between the 2 
SKUs.  We can draw this visually as a tree with parent child relationships 
between all the SKUs.

Now from a module that calls LibPcdGetSku(), the module may want to know who 
the parent SKU is.  The information we have about the parent SKU is the SKU ID 
value.

A new API that may be useful here is something like:

UINTN
GetParentSku (
  IN UINTN  SkuId
  );

The algorithm that Tim provided below uses autogen and a loop to retrieve the 
parent SKU.

So the request here is to extend the autogen to support a lookup of the parent 
SKU given any valid SKU ID value.

Thanks,

Mike
 
> -Original Message-
> From: Zeng, Star
> Sent: Friday, May 5, 2017 3:08 AM
> To: Tim Lewis ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zeng, Star 
> 
> Subject: RE: [RFC] PCD: Extended SKU support 1 - inheritance
> 
> Tim,
> 
> In your code base, there are other tools beyond BaseTools to parse the 
> [SkuIds] section and SKUID_IDENTIFIER in DSC, right?
> 
> The boot time usage (use the value in DEFAULT SKU instead if the value 
> in specified SKU is not configured) for other resources in your code base is 
> similar to PCD, right?
> This RFC has no change to this boot time behavior that "use the value 
> in DEFAULT SKU instead if the value in specified SKU is not configured".
> So I am still not so clear about the benefit of the macro like #define 
> SKUID_IDENTIFIER_DEFAULT 2,1,0,1,0,0.
> 
> 
> The DSC syntax extension in this RFC is OPTIONAL, the backward 
> compatibility is expected to be kept, no current tools and code should be 
> impacted.
> As I know, there will be a staging branch proposed for the development 
> of PCD related RFCs (include Structure PCD, etc). Mike and Liming can help 
> confirm it.
> How about we have the development of this RFC to the staging branch 
> first, then you can help evaluate the syntax and provide feedback 
> further? :)
> 
> 
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Tim Lewis
> Sent: Friday, May 5, 2017 12:36 AM
> To: Zeng, Star ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: Re: [edk2] [RFC] PCD: Extended SKU support 1 - inheritance
> 
> Star --
> 
> I understand your use case. We use the same behavior for other resources 
> besides PCD.
> 
> All we are asking is that this data that is used by the build tools is 
> also available in some form at runtime.
> 
> Thanks,
> 
> Tim
> 
> 
> 
> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Thursday, May 04, 2017 6:42 AM
> To: Tim Lewis ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zeng, Star 
> 
> Subject: RE: [RFC] PCD: Extended SKU support 1 - inheritance
> 
> Tim,
> 
> To avoid misunderstanding, I think I need to clarify more about this 
> RFC. :)
> 
> This RFC is NOT to propose building real relationship (like 
> parent-child) between non- DEFAULT SKUs, it seems easy to bring confusion 
> from the proposed syntax "
> SkuValue|SkuName[|ParentSkuName]", espe

[edk2] [RFC V2] PCD: Extended SKU support 1 - inheritance

2017-05-15 Thread Star Zeng 
- Requirement
Simplify the PCDs configuring for multiple SKUs in DSC.
Provide interface to get the relationship between multiple SKUs for runtime 
usage.


- Current limitation
Non-DEFAULT SKU could only derive from DEFAULT SKU, but could not derive from 
another non-DEFAULT SKU.
For example below, SkuA and SkuB could only derive from DEFAULT, but SkuB could 
not derive from SkuA.

[SkuIds]
  0 | DEFAULT
  1 | SkuA
  2 | SkuB


- Proposal: One non-DEFAULT SKU could be a derivative of another non-DEFAULT 
SKU.
This proposal extends DSC [SkuIds] section syntax and the extension is optional.
This proposal keeps the backward compatibility with current SKU usage.
This proposal provides interface to get the relationship between multiple SKUs 
for runtime usage.
BaseTools update is needed to support the extension, and no any change in PCD 
database and driver is required.

DSC syntax:
  [SkuIds]
SkuValue|SkuName[|ParentSkuName]
  SkuValue: integer, 0 is reserved for DEFAULT SKU.
  SkuName: string
  ParentSkuName: string, optional, it is new introduced in this proposal 
and defines which SKU the PCD value will derive from for this SKU. The PCD 
value will derive from DEFAULT SKU for this SKU if the ParentSkuName is absent.

AutoGen.h:
  extern UINT64 *_gPcd_SkuId_Array;

AutoGen.c:
  // SkuId array with relationship information for SkuIds listed in 
SKUID_IDENTIFIER
  GLOBAL_REMOVE_IF_UNREFERENCED UINT64 *_gPcd_SkuId_Array = { SkuId, 
ParentSkuId, ParentParentSkuId, …, 0x0, SkuId, ParentSkuId, ParentSkuId, …, 
0x0, …, 0x0 };
For the example below, _gPcd_SkuId_Array = { 0x2, 0x1, 0x0, 0x1, 0x0, 0x0 }.

PcdLib.h:
  LibPcdGetParentSkuId function and PcdGetParentSkuId macro, the patch below 
shows their implementation.


- Example: SkuB is a derivative of SkuA, but not a derivative of DEFAULT.

  [SkuIds]
0 | DEFAULT
1 | SkuA
2 | SkuB | SkuA

  [PcdsDynamicDefault.Common.DEFAULT]
gXXXPkgTokenSpaceGuid.PcdXXXSignature|"DEFAULT”
gXXXPkgTokenSpaceGuid.PcdXXXConfig1|FALSE
gXXXPkgTokenSpaceGuid.PcdXXXConfig2|FALSE
gXXXPkgTokenSpaceGuid.PcdXXXConfig3|FALSE

  [PcdsDynamicDefault.Common.SkuA]
gXXXPkgTokenSpaceGuid.PcdXXXSignature|“SkuA”
gXXXPkgTokenSpaceGuid.PcdXXXConfig1|TRUE
gXXXPkgTokenSpaceGuid.PcdXXXConfig2|TRUE
# No need statement for PcdXXXConfig3 whose value will derive from DEFAULT 
SKU and be FLASE.

  [PcdsDynamicDefault.Common.SkuB]
gXXXPkgTokenSpaceGuid.PcdXXXSignature|“SkuB”
# No need statement for PcdXXXConfig1 and PcdXXXConfig2 whose values will 
derive from SkuA SKU and be TRUE.
gXXXPkgTokenSpaceGuid.PcdXXXConfig3|TRUE


Cc: Michael Kinney 
Cc: Liming Gao 
Cc: Tim Lewis 
Cc: Yonghong Zhu 

---
 MdePkg/Include/Library/PcdLib.h| 38 +-
 MdePkg/Library/BasePcdLibNull/PcdLib.c | 43 ++
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/PcdLib.h b/MdePkg/Include/Library/PcdLib.h
index 9e7e09f52cdc..409689156c39 100644
--- a/MdePkg/Include/Library/PcdLib.h
+++ b/MdePkg/Include/Library/PcdLib.h
@@ -346,6 +346,24 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
   (Size),  
\
   (Buffer) 
\
   )
+
+/**
+  Get the parent SkuId for the input SkuId.
+
+  If SkuId is 0, return 0.
+  If SkuId is invalid, return SkuId without change.
+
+  @param[in] SkuId  SKU ID.
+
+  @return Return the parent SkuId.
+
+**/
+#define PcdGetParentSkuId(SkuId) \
+LibPcdGetParentSkuId (  \
+_gPcd_SkuId_Array,  \
+(SkuId) \
+)
+
 /**
   Retrieves an 8-bit PCD token value based on a token name.
   
@@ -2039,7 +2057,6 @@ LibPcdGetNextTokenSpace (
   IN CONST GUID  *TokenSpaceGuid
   );
 
-
 /**
   Sets a value of a patchable PCD entry that is type pointer.
   
@@ -2174,6 +2191,25 @@ LibPatchPcdSetPtrAndSizeS (
   IN CONST VOID *Buffer
   );
 
+/**
+  Get the parent SkuId for the input SkuId.
+
+  If SkuId is 0, return 0.
+  If SkuId is invalid, return SkuId without change.
+
+  @param[in] SkuIdArray Pointer to SkuId array with relationship information.
+  @param[in] SkuId  SKU ID.
+
+  @return Return the parent SkuId.
+
+**/
+UINT64
+EFIAPI
+LibPcdGetParentSkuId (
+  IN UINT64 *SkuIdArray,
+  IN UINT64 SkuId
+  );
+
 typedef enum {
   PCD_TYPE_8,
   PCD_TYPE_16,
diff --git a/MdePkg/Library/BasePcdLibNull/PcdLib.c 
b/MdePkg/Library/BasePcdLibNull/PcdLib.c
index 4fc3672b7a36..434508cfc26c 100644
--- a/MdePkg/Library/BasePcdLibNull/PcdLib.c
+++ b/MdePkg/Library/BasePcdLibNull/PcdLib.c
@@ -1415,6 +1415,49 @@ LibPatchPcdSetPtrAndSizeS (
 }
 
 /**
+  Get the parent SkuId for the input S