Re: [edk2-devel] [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand polling

[Loh, Tien Hock]

Leif,
Some comments inlined.

> -Original Message-
> From: Loh, Tien Hock
> Sent: Tuesday, June 11, 2019 5:12 PM
> To: Leif Lindholm 
> Cc: 'Haojian Zhuang' ; 'devel@edk2.groups.io'
> ; 'thlo...@gmail.com' ; 'Ard
> Biesheuvel' 
> Subject: RE: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand
> polling
> 
> > -Original Message-
> > From: Leif Lindholm 
> > Sent: Tuesday, June 11, 2019 5:09 PM
> > To: Loh, Tien Hock 
> > Cc: 'Haojian Zhuang' ; 'devel@edk2.groups.io'
> > ; 'thlo...@gmail.com' ;
> 'Ard
> > Biesheuvel' 
> > Subject: Re: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand
> > polling
> >
> > On Tue, Jun 11, 2019 at 02:40:51AM +, Loh, Tien Hock wrote:
> > > Leif,
> > >
> > > Since Haojian have a newer driver model that uses the
> > > NonDiscoverableDeviceDxe, I believe we should be moving to use the
> > > new driver.
> > >
> > > I'll try to test out the patches submitted by Haojian in the mean time.
> > > Can you help review the patch? Thanks!
> >
> > I can have another look at the patch, but I would really appreciate if
> > you could also review it please?
> >
> > My problem is that I really don't have much understanding of SD/MMC
> > protocols.
> 
> Sure. I'll test it out on the SoCFPGA platform first. It is quite a long 
> patch, so I
> might take a bit of time to review.
> Thanks Leif!

I've reviewed and tested the driver. For SD portion the patch works correctly 
on the SoCFPGA platform. How should I ACK the patch?

> 
> >
> > /
> > Leif
> >
> > > > -Original Message-
> > > > From: Loh, Tien Hock
> > > > Sent: Thursday, May 30, 2019 3:56 PM
> > > > To: Haojian Zhuang ; Leif Lindholm
> > > > 
> > > > Cc: devel@edk2.groups.io; thlo...@gmail.com; Ard Biesheuvel
> > > > 
> > > > Subject: RE: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand
> > > > polling
> > > >
> > > > > -Original Message-
> > > > > From: Haojian Zhuang 
> > > > > Sent: Thursday, May 30, 2019 3:06 PM
> > > > > To: Leif Lindholm 
> > > > > Cc: Loh, Tien Hock ;
> > > > > devel@edk2.groups.io; thlo...@gmail.com; Ard Biesheuvel
> > > > > 
> > > > > Subject: Re: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc
> SendCommand
> > > > > polling
> > > > >
> > > > > On Tue, May 28, 2019 at 07:04:09PM +0100, Leif Lindholm wrote:
> > > > > > +Haojian,
> > > > > >
> > > > > > Haojian - since you are the original author, can you comment
> > > > > > on the delays? Are these silicon bug workarounds (so we need
> > > > > > to add a Pcd), or does these changes work on your platforms too?
> > > > >
> > > > > I'm not in the loop, so I missed the patch series.
> > > > >
> > > > > The patch series can't work on my platform for the eMMC.
> > > > > Although a variable is created to identify whether it's a SD or
> > > > > eMMC device, it doesn't identify the eMMC device by the right
> > > > > way. So the eMMC device isn't initialized successfully on my platform.
> > > > >
> > > > > 1. Since MMC framework could identify whether it's eMMC device
> > > > > or SD device, we need to make device driver gets this kind of
> > > > > information from the MMC framework. And we need to support
> > > > > multiple eMMC/SD instances in MMC framework.
> > > >
> > > > Yeah my bad I didn't read through the SD/MMC specs on that. Now I
> > > > check the specs and you're right, the information should be read
> > > > from MMC framework.
> > > >
> > > > >
> > > > > 2. I sent a patch series to support both eMMC device and SD
> > > > > device
> > before.
> > > > > https://edk2.groups.io/g/devel/message/28572
> > > > > &&
> > > > > https://edk2.groups.io/g/devel/message/28615
> > > > > Maybe it's missed. Could you help to review that patch series?
> > >
> > > Leif, can you help review the patch series? Since Haojian have moved
> > > the
> > driver to NonDiscoverableDeviceDxe, I think that would be a more
> > appropriate driver to be used going forward. Thanks!
> > >
> > > >
> > > > >
> > > > > Best Regards
> > > > > Haojian
> > > > >
> > > > > >
> > > > > > Regards,
> > > > > >
> > > > > > Leif
> > > > > >
> > > > > > On Mon, May 27, 2019 at 05:30:27PM +0800,
> > > > > > tien.hock@intel.com
> > > > > wrote:
> > > > > > > From: "Tien Hock, Loh" 
> > > > > > >
> > > > > > > Change SendCommand polling mode to remove unnecessary
> delay,
> > > > > > > and check for transfer done only when block data is to be
> > read/write.
> > > > > > > This would also increase performance slightly.
> > > > > > >
> > > > > > > Signed-off-by: "Tien Hock, Loh" 
> > > > > > > Cc: Leif Lindholm 
> > > > > > > Cc: Ard Biesheuvel 
> > > > > > > ---
> > > > > > >  EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 43
> > > > > +++-
> > > > > > >  1 file changed, 33 insertions(+), 10 deletions(-)
> > > > > > >
> > > > > > > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> > > > > > > b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> > > > > > > index c6c8e04917..b57833458f 100644
> > > > > > > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> > > > > > > 

Re: [edk2-devel] [edk2-platforms Patch 26/28] Vlv2TbltDevicePkg: Clean up all build scripts

[rebecca]

bld_vlv.sh should probably have "set -e" at the top, like the change to 
Build_IFWI.sh


See https://bugzilla.tianocore.org/show_bug.cgi?id=1599 ("bld_vlv.sh 
isn't `set -e` and doesn't check if commands completed successfully").



--
Rebecca Cran


On 7/10/19 1:05 PM, Michael D Kinney wrote:

* Remove cln.sh
* Remove unused PlatformDefintions.h file
* Remove unused defines from DSC files
* Make Windows and Linux script more consistent
* Remove unused options from Windows/Linux scripts
* Remove unused defines from Windows/Linux scripts
* Do not modify Target.txt in Conf directory.  Instead,
   pass all build flags on the command line
* Generate build report in the build output directory
* Generate build logs in the build output directory
* Do not delete files from Conf directory
* Update clean operation to only clean one platform target
* Do not generate AutoPlatformCFG.txt file anymore

Cc: Zailiang Sun 
Cc: Yi Qian 
Cc: Gary Lin 
Signed-off-by: Michael D Kinney 
---
  Platform/Intel/Vlv2TbltDevicePkg/.gitignore   |   5 -
  .../Intel/Vlv2TbltDevicePkg/Build_IFWI.bat|  75 ++---
  .../Intel/Vlv2TbltDevicePkg/Build_IFWI.sh |  57 +++
  .../Include/PlatformDefinitions.h |  43 -
  .../Vlv2TbltDevicePkg/PlatformPkgConfig.dsc   |   2 -
  .../Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc  |  55 +-
  .../Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc   |  53 +-
  .../Vlv2TbltDevicePkg/PlatformPkgIA32.dsc |  53 +-
  .../Vlv2TbltDevicePkg/PlatformPkgX64.dsc  |  55 +-
  Platform/Intel/Vlv2TbltDevicePkg/bld_vlv.bat  | 120 ++---
  Platform/Intel/Vlv2TbltDevicePkg/bld_vlv.sh   | 159 --
  Platform/Intel/Vlv2TbltDevicePkg/cln.sh   |  62 ---
  12 files changed, 156 insertions(+), 583 deletions(-)
  delete mode 100644 Platform/Intel/Vlv2TbltDevicePkg/.gitignore
  delete mode 100644 
Platform/Intel/Vlv2TbltDevicePkg/Include/PlatformDefinitions.h
  delete mode 100644 Platform/Intel/Vlv2TbltDevicePkg/cln.sh

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/.gitignore 
b/Platform/Intel/Vlv2TbltDevicePkg/.gitignore
deleted file mode 100644
index c7698262ad..00
--- a/Platform/Intel/Vlv2TbltDevicePkg/.gitignore
+++ /dev/null
@@ -1,5 +0,0 @@
-AutoPlatformCFG.txt
-Stitch/Stitching.log
-Stitch/MNW*.bin
-Stitch/MNW*.rom
-Stitch/MNW*.rom.orig
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/Build_IFWI.bat 
b/Platform/Intel/Vlv2TbltDevicePkg/Build_IFWI.bat
index 44759c617f..f65aa61f4a 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/Build_IFWI.bat
+++ b/Platform/Intel/Vlv2TbltDevicePkg/Build_IFWI.bat
@@ -9,15 +9,9 @@
  
  SetLocal EnableDelayedExpansion EnableExtensions
  
-@REM Go to work space directory.

-cd ..
-cd ..
-
  :: Assign initial values
  set exitCode=0
  set "Build_Flags= "
-set "Stitch_Flags= "
-set Arch=X64
  set PLATFORM_PACKAGE=Vlv2TbltDevicePkg
  
  set PLATFORM_PATH=%WORKSPACE%

@@ -42,11 +36,6 @@ if not exist %PLATFORM_PATH%\%PLATFORM_PACKAGE% (
  :OptLoop
  if /i "%~1"=="/?" goto Usage
  
-if /i "%~1"=="/q" (

-set Build_Flags=%Build_Flags% /q
-shift
-goto OptLoop
-)
  if /i "%~1"=="/l" (
  set Build_Flags=%Build_Flags% /l
  shift
@@ -67,53 +56,17 @@ if /i "%~1" == "/c" (
  shift
  goto OptLoop
  )
-if /i "%~1" == "/ECP" (
-set Build_Flags=%Build_Flags% /ecp
-shift
-goto OptLoop
-)
-
-if /i "%~1"=="/s" (
-set Build_Flags=%Build_Flags% /s
-shift
-goto OptLoop
-)
-
  if /i "%~1"=="/x64" (
-set Arch=X64
  set Build_Flags=%Build_Flags% /x64
  shift
  goto OptLoop
  )
-
  if /i "%~1"=="/IA32" (
-set Arch=IA32
  set Build_Flags=%Build_Flags% /IA32
  shift
  goto OptLoop
  )
  
-if /i "%~1"=="/nG" (

-set Stitch_Flags=%Stitch_Flags% /nG
-shift
-goto OptLoop
-)
-if /i "%~1"=="/nM" (
-set Stitch_Flags=%Stitch_Flags% /nM
-shift
-goto OptLoop
-)
-if /i "%~1"=="/nB" (
-set Stitch_Flags=%Stitch_Flags% /nB
-shift
-goto OptLoop
-)
-if /i "%~1"=="/yL" (
-set Stitch_Flags=%Stitch_Flags% /yL
-shift
-goto OptLoop
-)
-
  :: Require 2 input parameters
  if "%~2"=="" goto Usage
  
@@ -125,7 +78,7 @@ set Build_Target=%~2

  echo ==
  echo Build_IFWI:  Calling BIOS build Script...
  
-call %PLATFORM_PATH%\%PLATFORM_PACKAGE%\bld_vlv.bat %Build_Flags%  %Platform_Type% %Build_Target%

+call %PLATFORM_PATH%\%PLATFORM_PACKAGE%\bld_vlv.bat %Build_Flags% 
%Platform_Type% %Build_Target%
  
  if %ERRORLEVEL% NEQ 0 (

  echo echo  -- Error Building BIOS  & echo.
@@ -139,23 +92,21 @@ goto Exit
  :Usage
  echo Script to build BIOS firmware and stitch the entire IFWI.
  echo.
-echo Usage: Build_IFWI.bat [options]  PlatformType  BuildTarget  [IFWI Suffix]
+echo Usage: Build_IFWI.bat [options]  PlatformType  BuildTarget
  echo.
-echo/c CleanAll before building
-echo/x64   Set Arch to X64  (default: X64)
-echo/IA32  Set Arch to IA32 (default: X64)

Re: [edk2-devel] [edk2-platforms Patch 23/28] Vlv2TbltDevicePkg: Import TlsLib for HTTPS Boot

[rebecca]

Reviewed-by: Rebecca Cran 


On 7/10/19 1:05 PM, Michael D Kinney wrote:

From: Gary Lin 

When setting NETWORK_TLS_ENABLE to TRUE, we need TlsLib.

Cc: Zailiang Sun 
Cc: Yi Qian 
Cc: Michael D Kinney 
Signed-off-by: Gary Lin 
---
  Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc | 3 +++
  Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc  | 3 +++
  Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc| 3 +++
  Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 3 +++
  4 files changed, 12 insertions(+)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc
index 7993ee5758..e4e86cbb07 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc
@@ -242,6 +242,9 @@ [LibraryClasses.common]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!endif
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
  !endif

TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf

Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index ee0d055d64..bc986eae78 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -244,6 +244,9 @@ [LibraryClasses.common]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!endif
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
  !endif

TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf

Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index 62ca4f67cc..f2f02e5f76 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -242,6 +242,9 @@ [LibraryClasses.common]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!endif
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
  !endif

TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf

Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 282ff3c2e2..4184c946a6 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -244,6 +244,9 @@ [LibraryClasses.common]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!endif
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
  !endif

TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf

Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf


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

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

Re: [edk2-devel] [edk2-platforms Patch 02/28] Vlv2TbltDevicePkg/Build_IFWI.sh: Change the root directory path

[rebecca]

On 2019-07-10 13:04, Michael D Kinney wrote:
>  echo "Build_IFWI:  Calling BIOS build Script..."
> -./$PLATFORM_PACKAGE/bld_vlv.sh $Build_Flags $Platform_Type $Build_Target
> +./Platform/Intel/$PLATFORM_PACKAGE/bld_vlv.sh $Build_Flags $Platform_Type 
> $Build_Target


It looks like these changes are for getting ready to move the code into
master.

I've just started working on porting the BroxtonPlatformPkg over to
master too, and just wanted to check if you know if anyone's already
working on it? If so, I'll stop my work to avoid the duplication of effort.


-- 
Rebecca Cran


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

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

Re: [edk2-devel] [edk2-platforms Patch 01/28] Vlv2TbltDevicePkg/Build_IFWI.sh: Add "set -e" to exit on error

[rebecca]

On 2019-07-10 13:04, Michael D Kinney wrote:
> +set -e
> +

Reviewed-by: Rebecca Cran 


-- 
Rebecca Cran


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

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

Re: [edk2-devel] [edk2-platforms Patch 18/28] Vlv2TbltDevicePkg/Build_IFWI.sh: Check the WORKSPACE env variable

[Sun, Zailiang]

Reviewed-By: Zailiang Sun 

-Original Message-
From: Kinney, Michael D 
Sent: Thursday, July 11, 2019 3:05 AM
To: devel@edk2.groups.io
Cc: Gary Lin ; Sun, Zailiang ; Qian, Yi 

Subject: [edk2-platforms Patch 18/28] Vlv2TbltDevicePkg/Build_IFWI.sh: Check 
the WORKSPACE env variable

From: Gary Lin 

Check WORKSPACE before running the script and notify the user to export the 
variable if the variable doesn't exist

Cc: Zailiang Sun 
Cc: Yi Qian 
Cc: Michael D Kinney 
Signed-off-by: Gary Lin 
---
 Platform/Intel/Vlv2TbltDevicePkg/Build_IFWI.sh | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/Build_IFWI.sh 
b/Platform/Intel/Vlv2TbltDevicePkg/Build_IFWI.sh
index f8954cc9cd..422e90ca6c 100755
--- a/Platform/Intel/Vlv2TbltDevicePkg/Build_IFWI.sh
+++ b/Platform/Intel/Vlv2TbltDevicePkg/Build_IFWI.sh
@@ -6,7 +6,7 @@ function Usage ( ) {
   echo
   echo "Script to build BIOS firmware and stitch the entire IFWI."
   echo
-  echo "Usage: Build_IFWI.bat [options]  PlatformType  BuildTarget  "
+  echo "Usage: Build_IFWI.sh [options]  PlatformType  BuildTarget  "
   echo
   echo 
   echo "   /yL [option]  :   Enable SPI lock"
@@ -28,6 +28,18 @@ Stitch_Flags=
 Arch=X64
 PLATFORM_PACKAGE=Vlv2TbltDevicePkg
 
+## Check whether WORKSPACE is set or not if [[ -z "$WORKSPACE" ]]; then
+  echo "Please export WORKSPACE before running Build_IFWI.sh"
+  echo "See the details in Readme.md"
+  exit 1
+fi
+
+## Create $WORKSPACE/Conf if necessary
+if [ ! -d $WORKSPACE/Conf ]; then
+  mkdir $WORKSPACE/Conf
+fi
+
 ## Parse Optional arguments
 if [ "$1" == "/?" ]; then
   Usage
--
2.21.0.windows.1


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

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

Re: [edk2-devel] [edk2-platforms Patch 23/28] Vlv2TbltDevicePkg: Import TlsLib for HTTPS Boot

[Sun, Zailiang]

Reviewed-By: Zailiang Sun 

-Original Message-
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Michael D 
Kinney
Sent: Thursday, July 11, 2019 3:05 AM
To: devel@edk2.groups.io
Cc: Gary Lin ; Sun, Zailiang ; Qian, Yi 

Subject: [edk2-devel] [edk2-platforms Patch 23/28] Vlv2TbltDevicePkg: Import 
TlsLib for HTTPS Boot

From: Gary Lin 

When setting NETWORK_TLS_ENABLE to TRUE, we need TlsLib.

Cc: Zailiang Sun 
Cc: Yi Qian 
Cc: Michael D Kinney 
Signed-off-by: Gary Lin 
---
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc | 3 +++  
Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc  | 3 +++
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc| 3 +++
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 3 +++
 4 files changed, 12 insertions(+)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc
index 7993ee5758..e4e86cbb07 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc
@@ -242,6 +242,9 @@ [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!endif
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
   
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
   
Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index ee0d055d64..bc986eae78 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -244,6 +244,9 @@ [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!endif
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
   
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
   
Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index 62ca4f67cc..f2f02e5f76 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -242,6 +242,9 @@ [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!endif
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
   
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
   
Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 282ff3c2e2..4184c946a6 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -244,6 +244,9 @@ [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!endif
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
   
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
   
Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
--
2.21.0.windows.1





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

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

Re: [edk2-devel] [edk2-platforms Patch 00/28] Vlv2TbltDevicePkg: Fix Linux build issues

[Sun, Zailiang]

Reviewed-By: Zailiang Sun 

-Original Message-
From: Kinney, Michael D 
Sent: Thursday, July 11, 2019 3:05 AM
To: devel@edk2.groups.io
Cc: Sun, Zailiang ; Qian, Yi ; Gary 
Lin 
Subject: [edk2-platforms Patch 00/28] Vlv2TbltDevicePkg: Fix Linux build issues

* Update Readme.md for multiple repos
* Clean up all build scripts and make Linux and Windows support consistent
* Add IA32 GCC build support and only build capsules for X64
* Clean up library mappings, add TlsLib for HTTPS Boot, and add SD/MMC drivers.
* Re-enable BIOS ID feature using new GenBiosId.py tool
* Remove redundant declarations from DEC file and remove unused content
* Add missing header files in [Sources] sections of INF files.
* Vlv2DeviceRefCodePlg/AcpiTablesPCAT: Fix ASL build error using newer iasl.
* Vlvt2TbltDevicePkg: Fix NULL pointer access in SmbiosMiscDxe
* Vlv2TbltDevicePkg/AcpiPlatform: Fix the error from InstallAcpiTable and
  remove unused local variable.

Cc: Zailiang Sun 
Cc: Yi Qian 
Signed-off-by: Michael D Kinney 
Signed-off-by: Gary Lin 

Gary Lin (18):
  Vlv2TbltDevicePkg/Build_IFWI.sh: Add "set -e" to exit on error
  Vlv2TbltDevicePkg/Build_IFWI.sh: Change the root directory path
  Vlv2TbltDevicePkg/bld_vlv.sh: Fix the log name
  Vlv2TbltDevicePkg/bld_vlv.sh: Update the gcc version detection
  Vlv2TbltDevicePkg/bld_vlv.sh: Remove ECP completely
  Vlv2TbltDevicePkg/bld_vlv.sh: Update the script to fit edk2-platforms
  Vlv2TbltDevicePkg: Add the missing headers to inf files
  Vlv2TbltDevicePkg/bld_vlv.sh: Remove BiosId.env completely
  Vlv2TbltDevicePkg/bld_vlv.sh: Correct the path to
PlatformCapsuleGcc.dsc
  Vlv2TbltDevicePkg/bld_vlv.sh: Create Vlv.ROM
  Vlv2TbltDevicePkg/GenerateCapsule: Fix the bash scripts
  Vlv2TbltDevicePkg/AcpiPlatform: Remove a unused variable
  Vlv2TbltDevicePkg/AcpiPlatform: Fix the error from InstallAcpiTable
  Vlv2TbltDevicePkg/SmBiosMiscDxe: Fix the indentation
  Vlv2TbltDevicePkg: Import SD/MMC drivers
  Vlv2TbltDevicePkg/Build_IFWI.sh: Check the WORKSPACE env variable
  Vlv2TbltDevicePkg: Import TlsLib for HTTPS Boot
  Vlv2TbltDevicePkg: Reorganize the libraries

Michael D Kinney (10):
  Vlv2TbltDevicePkg: Update Linux build scripts
  Vlvt2TbltDevicePkg: Fix NULL pointer access in SmbiosMiscDxe
  Vlv2DeviceRefCodePkg: Remove redundant gEfiSpiProtocolGuid
  Vlv2TbltDevicePkg: Remove redundant gEfiSpiProtocolGuid
  Vlv2TbltDevicePkg: Re-enable BIOS ID feature
  Vlv2TbltDevicePkg: Add GCC IA32 build support
  Vlv2DeviceRefCodePlg/AcpiTablesPCAT: Fix ASL error
  Vlv2TbltDevicePkg: Clean up all build scripts
  Vlv2TbltDevicePkg: Only build capsules for X64
  Vlv2TbltDevicePkg: Update Readme.md for multiple repos

 Platform/Intel/Vlv2TbltDevicePkg/.gitignore   |   5 -
 .../AcpiPlatform/AcpiPlatform.c   |   5 +-
 .../Intel/Vlv2TbltDevicePkg/Build_IFWI.bat|  75 +---
 .../Intel/Vlv2TbltDevicePkg/Build_IFWI.sh |  76 ++--
 .../Capsule/GenerateCapsule/GenCapsuleAll.sh  |  37 +-
 .../GenerateCapsule/GenCapsuleMinnowMax.sh|  24 +-
 .../GenCapsuleMinnowMaxRelease.sh |  19 +-
 .../Include/Guid/PlatformCpuInfo.h|  16 +-
 .../Include/Library/BiosIdLib.h   |  86 -
 .../Include/Library/CpuIA32.h | 345 --
 .../Include/PlatformDefinitions.h |  43 ---
 .../Library/BiosIdLib/BiosIdLib.c |  98 -
 .../Library/BiosIdLib/BiosIdLib.inf   |  33 --
 .../Library/CpuIA32Lib/CpuIA32Lib.inf |  41 ---
 .../Library/CpuIA32Lib/EfiCpuVersion.c|  70 
 .../Library/CpuIA32Lib/IA32/CpuIA32.S | 223 ---
 .../Library/CpuIA32Lib/IA32/CpuIA32.asm   | 206 ---
 .../Library/CpuIA32Lib/IA32/CpuIA32.c | 177 -
 .../Library/CpuIA32Lib/X64/Cpu.S  | 207 ---
 .../Library/CpuIA32Lib/X64/Cpu.asm| 222 ---
 .../Library/FlashDeviceLib/FlashDeviceLib.inf |   2 +-
 .../Library/PchSmmLib/PchSmmLib.inf   |   8 +-
 .../MonoStatusCode/MonoStatusCode.inf |   2 +-
 .../PlatformDxe/PlatformDxe.inf   |   5 +-
 .../PlatformInitPei/MemoryCallback.c  |  11 +-
 .../PlatformInitPei/PlatformEarlyInit.h   |   4 +-
 .../PlatformInitPei/PlatformInitPei.inf   |   1 -
 .../PlatformPei/PlatformPei.inf   |   1 -
 .../Intel/Vlv2TbltDevicePkg/PlatformPkg.dec   |   1 -
 .../Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf   |  14 +
 .../Vlv2TbltDevicePkg/PlatformPkgConfig.dsc   |   2 -
 .../Vlv2TbltDevicePkg/PlatformPkgGcc.fdf  |  14 +
 ...formPkgIA32.dsc => PlatformPkgGccIA32.dsc} | 122 ++-
 .../Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc   | 112 ++
 .../Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 109 ++
 .../Vlv2TbltDevicePkg/PlatformPkgX64.dsc  | 113 ++
 .../PlatformSetupDxe/PlatformSetupDxe.h   |   1 -
 .../PlatformSetupDxe/PlatformSetupDxe.inf |   5 +-
 .../PlatformSetupDxe/SetupInfoRecords.c   |  42 ++-
 .../Vlv2TbltDevicePkg/PpmPolicy/PpmPolicy.c   

Re: [edk2-devel] [edk2-platforms Patch 24/28] Vlv2TbltDevicePkg: Reorganize the libraries

[Sun, Zailiang]

Reviewed-By: Zailiang Sun 

-Original Message-
From: Kinney, Michael D 
Sent: Thursday, July 11, 2019 3:05 AM
To: devel@edk2.groups.io
Cc: Gary Lin ; Sun, Zailiang ; Qian, Yi 

Subject: [edk2-platforms Patch 24/28] Vlv2TbltDevicePkg: Reorganize the 
libraries

From: Gary Lin 

* Simplify the logic of importing TpmMeasurementLib
* Import BaseCryptLib, OpensslLib, and IntrinsicLib unconditionally
  since FmpDxe needs them
* Import FileExplorerLib unconditionally since UiApp needs it
* Update the import of TPM/TCG libraries

Cc: Zailiang Sun 
Cc: Yi Qian 
Cc: Michael D Kinney 
Signed-off-by: Gary Lin 
---
 .../Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc  | 45 +++--
 .../Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc   | 48 +++
 .../Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 45 +++--
 .../Vlv2TbltDevicePkg/PlatformPkgX64.dsc  | 47 +++---
 4 files changed, 66 insertions(+), 119 deletions(-)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc
index e4e86cbb07..eb7ae46505 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccIA32.dsc
@@ -197,7 +197,6 @@ [LibraryClasses.common]
   TpmCommLib|SecurityPkg/Library/TpmCommLib/TpmCommLib.inf
   Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf
   Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
-
 !endif
 
   
PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
@@ -215,8 +214,9 @@ [LibraryClasses.common]
   # CryptLib
   #
 !if $(TPM_ENABLED) == TRUE
-  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
-  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+
+TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasur
+ementLib.inf
+!else
+
+TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasure
+mentLibNull.inf
 !endif
 
  BiosIdLib|BoardModulePkg/Library/BiosIdLib/DxeBiosIdLib.inf
@@ -224,34 +224,28 @@ [LibraryClasses.common]
   StallSmmLib|Vlv2TbltDevicePkg/Library/StallSmmLib/StallSmmLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
-  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
-  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
   
PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
-  
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
-  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
 !else
-  
TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
 !endif
+
+
+ FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.i
+ nf
+
   VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
   ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
   SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
-!if $(NETWORK_ISCSI_ENABLE) == TRUE
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
-!endif
 !if $(NETWORK_TLS_ENABLE) == TRUE
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
-  
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
+  Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
   
Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
   
Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
 
-
-  Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
 
 [LibraryClasses.IA32.SEC]
@@ -305,18 +299,13 @@ [LibraryClasses.IA32]
   
ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
   
ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
 
+
+Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.i
+nf
+!if $(TPM_ENABLED) == TRUE
   
TcgPhysicalPresenceLib|SecurityPkg/Library/DxeTcgPhysicalPresenceLib/DxeTcgPhysicalPresenceLib.inf
-!if $(TPM_ENABLED) == TRUE
-  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
 !endif
 
   LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
   EfiRegTableLib|Vlv2TbltDevicePkg/Library/EfiRegTableLib/EfiRegTableLib.inf
-
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
-!endif
-
   
HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
 
 [LibraryClasses.IA32.DXE_DRIVER]
@@ -370,6 

Re: [edk2-devel] [edk2-platforms Patch 13/28] Vlv2TbltDevicePkg/AcpiPlatform: Fix the error from InstallAcpiTable

[Sun, Zailiang]

Reviewed-By: Zailiang Sun 

-Original Message-
From: Kinney, Michael D 
Sent: Thursday, July 11, 2019 3:05 AM
To: devel@edk2.groups.io
Cc: Gary Lin ; Sun, Zailiang ; Qian, Yi 

Subject: [edk2-platforms Patch 13/28] Vlv2TbltDevicePkg/AcpiPlatform: Fix the 
error from InstallAcpiTable

From: Gary Lin 

The firmware crashed when installing ACPI tables:

ASSERT_EFI_ERROR (Status = Invalid Parameter) ASSERT [AcpiPlatform] 
edk2-platforms/Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c(1162):
 !EFI_ERROR (AcpiStatus)

The 'Size' from ReadSection() is not the size of the ACPI table so
InstallAcpiTable() returned EFI_INVALID_PARAMETER.

Use the 'Length' from the header to avoid the crash.

Cc: Zailiang Sun 
Cc: Yi Qian 
Cc: Michael D Kinney 
Signed-off-by: Gary Lin 
---
 Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c 
b/Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c
index f3efc41e31..72edc1bc1e 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c
+++ b/Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c
@@ -1152,7 +1152,7 @@ AcpiPlatformEntryPoint (
   AcpiStatus = AcpiTable->InstallAcpiTable (
   AcpiTable,
   CurrentTable,
-  Size,
+  CurrentTable->Length,
   
   );
   ASSERT_EFI_ERROR (AcpiStatus);
--
2.21.0.windows.1


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

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

Re: [edk2-devel] [edk2-platforms Patch 15/28] Vlv2TbltDevicePkg: Import SD/MMC drivers

[Sun, Zailiang]

Reviewed-By: Zailiang Sun 

-Original Message-
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Michael D 
Kinney
Sent: Thursday, July 11, 2019 3:05 AM
To: devel@edk2.groups.io
Cc: Gary Lin ; Sun, Zailiang ; Qian, Yi 

Subject: [edk2-devel] [edk2-platforms Patch 15/28] Vlv2TbltDevicePkg: Import 
SD/MMC drivers

From: Gary Lin 

Import the SD card/MMC drivers from MdeModulePkg

Cc: Zailiang Sun 
Cc: Yi Qian 
Cc: Michael D Kinney 
Signed-off-by: Gary Lin 
---
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf   | 7 +++
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf| 7 +++
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 7 +++
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc   | 7 +++
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc| 7 +++
 5 files changed, 35 insertions(+)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf
index 0661c778c1..c538fe4a06 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf
@@ -529,6 +529,13 @@ [FV.FVMAIN]
 INF  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
 !endif
 
+#
+# eMMC/SD Card
+#
+INF MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+INF MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
+
 #
 # IDE/SCSI/AHCI
 #
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
index 06bdebb10b..bd9d415939 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
@@ -478,6 +478,13 @@ [FV.FVMAIN]
 INF  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
 !endif
 
+#
+# eMMC/SD Card
+#
+INF MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+INF MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
+
 #
 # IDE/SCSI/AHCI
 #
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index b558caa61f..6317bc1342 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -1143,6 +1143,13 @@ [Components.X64]
   }
 !endif
 
+#
+# eMMC/SD Card
+#
+  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+  MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
+
 #
 # IDE/SCSI/AHCI
 #
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index 99ada0ef0a..8f69bb47dd 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -1127,6 +1127,13 @@ [Components.IA32]
   }
 !endif
 
+#
+# eMMC/SD Card
+#
+  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+  MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
+
 #
 # IDE/SCSI/AHCI
 #
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 87404d2d82..d146321750 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -1142,6 +1142,13 @@ [Components.X64]
   }
 !endif
 
+#
+# eMMC/SD Card
+#
+  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+  MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
+
 #
 # IDE/SCSI/AHCI
 #
-- 
2.21.0.windows.1





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

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

Re: [edk2-devel] [edk2-platforms Patch 14/28] Vlv2TbltDevicePkg/SmBiosMiscDxe: Fix the indentation

[Sun, Zailiang]

Reviewed-By: Zailiang Sun 

-Original Message-
From: Kinney, Michael D 
Sent: Thursday, July 11, 2019 3:05 AM
To: devel@edk2.groups.io
Cc: Gary Lin ; Sun, Zailiang ; Qian, Yi 

Subject: [edk2-platforms Patch 14/28] Vlv2TbltDevicePkg/SmBiosMiscDxe: Fix the 
indentation

From: Gary Lin 

Fix the indentation of MiscSystemManufacturerFunction.c to improve the 
readability.

Cc: Zailiang Sun 
Cc: Yi Qian 
Cc: Michael D Kinney 
Signed-off-by: Gary Lin 
---
 .../MiscSystemManufacturerFunction.c  | 51 +--
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git 
a/Platform/Intel/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
 
b/Platform/Intel/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
index f537e0db76..da660cb3a8 100644
--- 
a/Platform/Intel/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
+++ b/Platform/Intel/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufactu
+++ rerFunction.c
@@ -52,14 +52,14 @@ AddSmbiosManuCallback (
   UINTN PdNameStrLen;
   UINTN SerialNumStrLen;
   UINTN SkuNumberStrLen;
-  UINTNFamilyNameStrLen;
+  UINTN FamilyNameStrLen;
   EFI_STATUSStatus;
   EFI_STRINGManufacturer;
   EFI_STRINGProductName;
   EFI_STRINGVersion;
   EFI_STRINGSerialNumber;
   EFI_STRINGSkuNumber;
-  EFI_STRING   FamilyName;
+  EFI_STRINGFamilyName;
   STRING_REFTokenToGet;
   EFI_SMBIOS_HANDLE SmbiosHandle;
   SMBIOS_TABLE_TYPE1*SmbiosRecord;
@@ -323,40 +323,37 @@ MISC_SMBIOS_TABLE_FUNCTION(MiscSystemManufacturer)
 {
   EFI_STATUSStatus;
   static BOOLEANCallbackIsInstalledManu = FALSE;
-  VOID   *AddSmbiosManuCallbackNotifyReg;
-  EFI_EVENT  AddSmbiosManuCallbackEvent;
+  VOID  *AddSmbiosManuCallbackNotifyReg;
+  EFI_EVENT AddSmbiosManuCallbackEvent;
 
 
   if (CallbackIsInstalledManu == FALSE) {
 CallbackIsInstalledManu = TRUE;// Prevent more than 1 callback.
 DEBUG ((EFI_D_INFO, "Create Smbios Manu callback.\n"));
 
-  //
-  // gEfiDxeSmmReadyToLockProtocolGuid is ready
-  //
-  Status = gBS->CreateEvent (
-  EVT_NOTIFY_SIGNAL,
-  TPL_CALLBACK,
-  (EFI_EVENT_NOTIFY)AddSmbiosManuCallback,
-  RecordData,
-  
-  );
+//
+// gEfiDxeSmmReadyToLockProtocolGuid is ready
+//
+Status = gBS->CreateEvent (
+EVT_NOTIFY_SIGNAL,
+TPL_CALLBACK,
+(EFI_EVENT_NOTIFY)AddSmbiosManuCallback,
+RecordData,
+
+);
 
-  ASSERT_EFI_ERROR (Status);
-  if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+if (EFI_ERROR (Status)) {
+  return Status;
+}
+
+Status = gBS->RegisterProtocolNotify (
+,
+AddSmbiosManuCallbackEvent,
+
+);
 return Status;
-
-  }
-
-  Status = gBS->RegisterProtocolNotify (
-  ,
-  AddSmbiosManuCallbackEvent,
-  
-  );
-
-  return Status;
   }
 
   return EFI_SUCCESS;
-
 }
--
2.21.0.windows.1


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

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

Re: [edk2-devel] [edk2-platforms Patch 12/28] Vlv2TbltDevicePkg/AcpiPlatform: Remove a unused variable

[Sun, Zailiang]

Reviewed-By: Zailiang Sun 

-Original Message-
From: Kinney, Michael D 
Sent: Thursday, July 11, 2019 3:05 AM
To: devel@edk2.groups.io
Cc: Gary Lin ; Sun, Zailiang ; Qian, Yi 

Subject: [edk2-platforms Patch 12/28] Vlv2TbltDevicePkg/AcpiPlatform: Remove a 
unused variable

From: Gary Lin 

TableVersion in AcpiPlatformEntryPoint() is not used anymore.

Cc: Zailiang Sun 
Cc: Yi Qian 
Cc: Michael D Kinney 
Signed-off-by: Gary Lin 
---
 Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c 
b/Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c
index 962baf561d..f3efc41e31 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c
+++ b/Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c
@@ -747,7 +747,6 @@ AcpiPlatformEntryPoint (
   UINT32FvStatus;
   UINTN Size;
   EFI_EVENT Event;
-  EFI_ACPI_TABLE_VERSIONTableVersion;
   UINTN VarSize;
   UINTN SysCfgSize;
   EFI_HANDLEHandle;
@@ -759,7 +758,6 @@ AcpiPlatformEntryPoint (
 
   mFirstNotify  = FALSE;
 
-  TableVersion  = EFI_ACPI_TABLE_VERSION_2_0;
   Instance  = 0;
   CurrentTable  = NULL;
   TableHandle   = 0;
-- 
2.21.0.windows.1


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

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

Re: [edk2-devel] [edk2-platforms Patch 11/28] Vlv2TbltDevicePkg/GenerateCapsule: Fix the bash scripts

[Sun, Zailiang]

Reviewed-By: Zailiang Sun 

-Original Message-
From: Kinney, Michael D 
Sent: Thursday, July 11, 2019 3:05 AM
To: devel@edk2.groups.io
Cc: Gary Lin ; Sun, Zailiang ; Qian, Yi 

Subject: [edk2-platforms Patch 11/28] Vlv2TbltDevicePkg/GenerateCapsule: Fix 
the bash scripts

From: Gary Lin 

Adjust the paths to fit the current directory structure

Cc: Zailiang Sun 
Cc: Yi Qian 
Cc: Michael D Kinney 
Signed-off-by: Gary Lin 
---
 .../Capsule/GenerateCapsule/GenCapsuleAll.sh  | 37 +++
 .../GenerateCapsule/GenCapsuleMinnowMax.sh| 24 +---
 .../GenCapsuleMinnowMaxRelease.sh | 19 +-
 3 files changed, 39 insertions(+), 41 deletions(-)  mode change 100644 => 
100755 
Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/GenerateCapsule/GenCapsuleAll.sh

diff --git 
a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/GenerateCapsule/GenCapsuleAll.sh
 
b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/GenerateCapsule/GenCapsuleAll.sh
old mode 100644
new mode 100755
index 040024553a..7b77b50c3f
--- 
a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/GenerateCapsule/GenCapsuleAll.sh
+++ b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/GenerateCapsule/G
+++ enCapsuleAll.sh
@@ -9,20 +9,25 @@
 
 cd $(dirname $0)
 
-rm -R $WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules
-mkdir $WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules
-mkdir $WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules/SampleDevelopment
-mkdir $WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules/NewCert
-mkdir $WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules/TestCert
-cp $WORKSPACE/Build/Vlv2TbltDevicePkg/DEBUG_GCC49/X64/CapsuleApp.efi 
$WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules/SampleDevelopment/CapsuleApp.efi
-cp $WORKSPACE/Build/Vlv2TbltDevicePkg/RELEASE_GCC49/X64/CapsuleApp.efi 
$WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules/SampleDevelopment/CapsuleAppRelease.efi
-cp $WORKSPACE/Build/Vlv2TbltDevicePkg/DEBUG_GCC49/X64/CapsuleApp.efi 
$WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules/NewCert/CapsuleApp.efi
-cp $WORKSPACE/Build/Vlv2TbltDevicePkg/RELEASE_GCC49/X64/CapsuleApp.efi 
$WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules/NewCert/CapsuleAppRelease.efi
-cp $WORKSPACE/Build/Vlv2TbltDevicePkg/DEBUG_GCC49/X64/CapsuleApp.efi 
$WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules/TestCert/CapsuleApp.efi
-cp $WORKSPACE/Build/Vlv2TbltDevicePkg/RELEASE_GCC49/X64/CapsuleApp.efi 
$WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules/TestCert/CapsuleAppRelease.efi
+EFI_DIR=$WORKSPACE/Build/Vlv2TbltDevicePkg/${TARGET}_${TOOL_CHAIN_TAG}/
+X64/ CAP_DIR=$WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules
+SCRIPT_DIR=$WORKSPACE/edk2-platforms/Platform/Intel/Vlv2TbltDevicePkg/F
+eature/Capsule/GenerateCapsule
 
-. GenCapsuleMinnowMax.sh
-. GenCapsuleMinnowMaxRelease.sh
-. GenCapsuleSampleColor.sh Blue  149DA854-7D19-4FAA-A91E-862EA1324BE6
-. GenCapsuleSampleColor.sh Green 79179BFD-704D-4C90-9E02-0AB8D968C18A
-. GenCapsuleSampleColor.sh Red   72E2945A-00DA-448E-9AA7-075AD840F9D4
+rm -rf $CAP_DIR
+mkdir -p $CAP_DIR/SampleDevelopment
+mkdir -p $CAP_DIR/NewCert
+mkdir -p $CAP_DIR/TestCert
+if [ "$TARGET" == "DEBUG" ]; then
+cp $EFI_DIR/CapsuleApp.efi $CAP_DIR/SampleDevelopment/CapsuleApp.efi
+cp $EFI_DIR/CapsuleApp.efi $CAP_DIR/NewCert/CapsuleApp.efi
+cp $EFI_DIR/CapsuleApp.efi $CAP_DIR/TestCert/CapsuleApp.efi
+. $SCRIPT_DIR/GenCapsuleMinnowMax.sh
+else
+cp $EFI_DIR/CapsuleApp.efi $CAP_DIR/SampleDevelopment/CapsuleAppRelease.efi
+cp $EFI_DIR/CapsuleApp.efi $CAP_DIR/NewCert/CapsuleAppRelease.efi
+cp $EFI_DIR/CapsuleApp.efi $CAP_DIR/TestCert/CapsuleAppRelease.efi
+. $SCRIPT_DIR/GenCapsuleMinnowMaxRelease.sh
+fi
+. $SCRIPT_DIR/GenCapsuleSampleColor.sh Blue
+149DA854-7D19-4FAA-A91E-862EA1324BE6
+. $SCRIPT_DIR/GenCapsuleSampleColor.sh Green 
79179BFD-704D-4C90-9E02-0AB8D968C18A
+. $SCRIPT_DIR/GenCapsuleSampleColor.sh Red   
72E2945A-00DA-448E-9AA7-075AD840F9D4
diff --git 
a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/GenerateCapsule/GenCapsuleMinnowMax.sh
 
b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/GenerateCapsule/GenCapsuleMinnowMax.sh
index 4fb963c93c..114c4a3477 100644
--- 
a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/GenerateCapsule/GenCapsuleMinnowMax.sh
+++ b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/GenerateCapsule/G
+++ enCapsuleMinnowMax.sh
@@ -13,7 +13,10 @@ FMP_CAPSULE_VERSION=0x000C
 FMP_CAPSULE_STRING=0.0.0.12
 FMP_CAPSULE_NAME="Intel MinnowMax DEBUG UEFI $FMP_CAPSULE_STRING"
 FMP_CAPSULE_LSV=0x
-FMP_CAPSULE_PAYLOAD=$WORKSPACE/Build/Vlv2TbltDevicePkg/DEBUG_GCC49/FV/Vlv.ROM
+FMP_CAPSULE_PAYLOAD=$WORKSPACE/Build/Vlv2TbltDevicePkg/DEBUG_${TOOL_CHA
+IN_TAG}/FV/Vlv.ROM 
+FMP_CAPSULE_DIR=$WORKSPACE/Build/Vlv2TbltDevicePkg/Capsules/
+
+PKCS7SIGN_DIR=$WORKSPACE/edk2/BaseTools/Source/Python/Pkcs7Sign/
 
 if [ ! -e "$FMP_CAPSULE_PAYLOAD" ] ; then
   return
@@ -34,12 +37,8 @@ if [ -e NewCert.pem ]; then
 --signer-private-cert=NewCert.pem \
 --other-public-cert=NewSub.pub.pem \
 --trusted-public-cert=NewRoot.pub.pem 

Re: [edk2-devel] [PATCH 2/3] MdePkg/BaseLib: rewrite Base64Decode()

[Gao, Zhichao]

Sorry for late respond.
The whole code is OK for me. And I write a tiny test for it without the memory 
address check. See 
https://github.com/ZhichaoGao/edk2/commit/615356ba32d3147957d215debd844e7709f06849
 . It is tested in Emulator environment. If it is OK, I think you can take my 
Tested-by for this patch. If there are some missing, please let me know.

Base64Decode parameter Source is indicate as OPTIONAL. Although it is OK to be 
NULL, and return DestinationSize to be zero with success status to indicate no 
decode occurred . I don't know if it is meaningful to report the result as 
that. In my opinion, NULL Source is an invalid input. Just my opinion, if the 
maintainer is OK with that, I am OK too.

Some other minor comment about the block scope variable blow.

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Tuesday, July 2, 2019 6:29 PM
> To: edk2-devel-groups-io 
> Cc: Gao, Liming ; Marvin Häuser
> ; Kinney, Michael D ;
> Philippe Mathieu-Daudé ; Gao, Zhichao
> 
> Subject: [edk2-devel] [PATCH 2/3] MdePkg/BaseLib: rewrite Base64Decode()
>
> Rewrite Base64Decode() from scratch, due to reasons listed in the second
> reference below.
>
> Implement Base64Decode() according to the specification added in the
> previous patch. The decoder scans the input buffer once, it has no inner
> loop(s), and it spills each output byte as soon as the output byte is 
> complete.
>
> Cc: Liming Gao 
> Cc: Marvin Häuser 
> Cc: Michael D Kinney 
> Cc: Philippe Mathieu-Daudé 
> Cc: Zhichao Gao 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1891
> Ref: http://mid.mail-archive.com/c495bd0b-ea4d-7206-8a4f-
> a7149760d...@redhat.com
> Signed-off-by: Laszlo Ersek 
> ---
>  MdePkg/Library/BaseLib/String.c | 249 +++-
>  1 file changed, 247 insertions(+), 2 deletions(-)
>
> diff --git a/MdePkg/Library/BaseLib/String.c
> b/MdePkg/Library/BaseLib/String.c index f8397035c32a..6198ccbc9672
> 100644
> --- a/MdePkg/Library/BaseLib/String.c
> +++ b/MdePkg/Library/BaseLib/String.c
> @@ -1973,8 +1973,253 @@ Base64Decode (
>IN OUT UINTN   *DestinationSize
>)
>  {
> -  ASSERT (FALSE);
> -  return RETURN_INVALID_PARAMETER;
> +  BOOLEAN PaddingMode;
> +  UINTN   SixBitGroupsConsumed;
> +  UINT32  Accumulator;
> +  UINTN   OriginalDestinationSize;
> +  UINTN   SourceIndex;
> +
> +  if (DestinationSize == NULL) {
> +return RETURN_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Check Source array validity.
> +  //
> +  if (Source == NULL) {
> +if (SourceSize > 0) {
> +  //
> +  // At least one CHAR8 element at NULL Source.
> +  //
> +  return RETURN_INVALID_PARAMETER;
> +}
> +  } else if (SourceSize > MAX_ADDRESS - (UINTN)Source) {
> +//
> +// Non-NULL Source, but it wraps around.
> +//
> +return RETURN_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Check Destination array validity.
> +  //
> +  if (Destination == NULL) {
> +if (*DestinationSize > 0) {
> +  //
> +  // At least one UINT8 element at NULL Destination.
> +  //
> +  return RETURN_INVALID_PARAMETER;
> +}
> +  } else if (*DestinationSize > MAX_ADDRESS - (UINTN)Destination) {
> +//
> +// Non-NULL Destination, but it wraps around.
> +//
> +return RETURN_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Check for overlap.
> +  //
> +  if (Source != NULL && Destination != NULL) {
> +//
> +// Both arrays have been provided, and we know from earlier that each
> array
> +// is valid in itself.
> +//
> +if ((UINTN)Source + SourceSize <= (UINTN)Destination) {
> +  //
> +  // Source array precedes Destination array, OK.
> +  //
> +} else if ((UINTN)Destination + *DestinationSize <= (UINTN)Source) {
> +  //
> +  // Destination array precedes Source array, OK.
> +  //
> +} else {
> +  //
> +  // Overlap.
> +  //
> +  return RETURN_INVALID_PARAMETER;
> +}
> +  }
> +
> +  //
> +  // Decoding loop setup.
> +  //
> +  PaddingMode = FALSE;
> +  SixBitGroupsConsumed= 0;
> +  Accumulator = 0;
> +  OriginalDestinationSize = *DestinationSize;
> +  *DestinationSize= 0;
> +
> +  //
> +  // Decoding loop.
> +  //
> +  for (SourceIndex = 0; SourceIndex < SourceSize; SourceIndex++) {
> +CHAR8  SourceChar;
> +UINT32 Base64Value;
> +UINT8  DestinationOctet;

The local variable define in a block scope. For CSS_2_1 Section 5.4.1.1 "Block 
(local) Scope". It is strongly discouraged. Maybe we should move them to the 
beginning of the function.

Thanks,
Zhichao

> +
> +SourceChar = Source[SourceIndex];
> +
> +//
> +// Whitespace is ignored at all positions (regardless of padding mode).
> +//
> +if (SourceChar == '\t' || SourceChar == '\n' || SourceChar == '\v' ||
> +SourceChar == '\f' || SourceChar == '\r' || SourceChar == ' ') {
> +  continue;
> +}
> +

[edk2-devel] Upcoming Event: TianoCore Community Meeting - APAC/NAMO - Thu, 07/11/2019 7:30pm-8:00pm #cal-reminder

[devel@edk2.groups.io Calendar]

*Reminder:* TianoCore Community Meeting - APAC/NAMO

*When:* Thursday, 11 July 2019, 7:30pm to 8:00pm, (GMT-07:00) America/Los 
Angeles

*Where:* https://zoom.us/j/769108409

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=458030 )

*Organizer:* Stephano Cetola stephano.cet...@intel.com ( 
stephano.cet...@intel.com?subject=Re:%20Event:%20TianoCore%20Community%20Meeting%20-%20APAC%2FNAMO
 )

*Description:*

Join Zoom Meeting

https://zoom.us/j/769108409 ( https://zoom.us/j/769108409 )

One tap mobile

+17207072699,,769108409# US

+16465588656,,769108409# US (New York)

Dial by your location

+1 720 707 2699 US

+1 646 558 8656 US (New York)

Meeting ID: 769 108 409

Find your local number: https://zoom.us/u/abOtdJckxL

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

View/Reply Online (#43624): https://edk2.groups.io/g/devel/message/43624
Mute This Topic: https://groups.io/mt/32437753/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Patch 0/2] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls gBS service.

[Dong, Eric]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972

AP calls CollectProcessorData() to collect processor info.
CollectProcessorData function finally calls PcdGetSize function to
get DynamicPCD PcdCpuFeaturesSetting value. PcdGetSize will use gBS
which caused ASSERT. 
This patch serial fixes the issue and enhances the related code to avoid
later report this issue again.

Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Chandana Kumar 
Cc: Star Zeng 

Eric Dong (2):
  UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls gBS service.
  UefiCpuPkg/Library/RegisterCpuFeaturesLib: avoid use dynamic PCD.

 .../CpuFeaturesInitialize.c   |  77 ++---
 .../RegisterCpuFeatures.h |  10 +-
 .../RegisterCpuFeaturesLib.c  | 109 +++---
 3 files changed, 85 insertions(+), 111 deletions(-)

-- 
2.21.0.windows.1


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

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

[edk2-devel] [Patch 2/2] UefiCpuPkg/Library/RegisterCpuFeaturesLib: avoid use dynamic PCD.

[Dong, Eric]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972

Function in this library may be used by APs. Assert will be trig if AP uses
dynamic pcd.
This patch enhance the current code, remove the unnecessary usage of dynamic
PCD. This change try to avoid report this issue again later.

Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Chandana Kumar 
Cc: Star Zeng 
Signed-off-by: Eric Dong 
---
 .../CpuFeaturesInitialize.c   |  64 +-
 .../RegisterCpuFeatures.h |  10 +-
 .../RegisterCpuFeaturesLib.c  | 114 ++
 3 files changed, 77 insertions(+), 111 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 87bfc64250..16b99c0c27 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -21,16 +21,12 @@ CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR", L"MMIO", 
L"CACHE", L"SEMAP", L"INVA
 VOID
 SetCapabilityPcd (
   IN UINT8   *SupportedFeatureMask,
-  IN UINT32  FeatureMaskSize
+  IN UINTN   FeatureMaskSize
   )
 {
   EFI_STATUS Status;
-  UINTN  BitMaskSize;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
-  ASSERT (FeatureMaskSize == BitMaskSize);
-
-  Status = PcdSetPtrS (PcdCpuFeaturesCapability, , 
SupportedFeatureMask);
+  Status = PcdSetPtrS (PcdCpuFeaturesCapability, , 
SupportedFeatureMask);
   ASSERT_EFI_ERROR (Status);
 }
 
@@ -38,16 +34,16 @@ SetCapabilityPcd (
   Worker function to save PcdCpuFeaturesSetting.
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
+  @param[in]  FeatureMaskSize   CPU feature bits mask buffer size.
 **/
 VOID
 SetSettingPcd (
-  IN UINT8   *SupportedFeatureMask
+  IN UINT8   *SupportedFeatureMask,
+  IN UINTN   BitMaskSize
   )
 {
   EFI_STATUS Status;
-  UINTN  BitMaskSize;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Status = PcdSetPtrS (PcdCpuFeaturesSetting, , 
SupportedFeatureMask);
   ASSERT_EFI_ERROR (Status);
 }
@@ -272,19 +268,20 @@ SupportedMaskOr (
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
   @param[in]  AndFeatureBitMask The feature bit mask to do AND operation
+  @param[in]  BitMaskSize   CPU feature bits mask buffer size.
+
 **/
 VOID
 SupportedMaskAnd (
   IN   UINT8   *SupportedFeatureMask,
-  IN CONST UINT8   *AndFeatureBitMask
+  IN CONST UINT8   *AndFeatureBitMask,
+  IN   UINT32  BitMaskSize
   )
 {
   UINTN  Index;
-  UINTN  BitMaskSize;
   UINT8  *Data1;
   CONST UINT8*Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Data1 = SupportedFeatureMask;
   Data2 = AndFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -297,19 +294,19 @@ SupportedMaskAnd (
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
   @param[in]  AndFeatureBitMask The feature bit mask to do XOR operation
+  @param[in]  BitMaskSize   CPU feature bits mask buffer size.
 **/
 VOID
 SupportedMaskCleanBit (
   IN UINT8   *SupportedFeatureMask,
-  IN UINT8   *AndFeatureBitMask
+  IN UINT8   *AndFeatureBitMask,
+  IN UINT32  BitMaskSize
   )
 {
   UINTN  Index;
-  UINTN  BitMaskSize;
   UINT8  *Data1;
   UINT8  *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Data1 = SupportedFeatureMask;
   Data2 = AndFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -323,6 +320,7 @@ SupportedMaskCleanBit (
 
   @param[in]  SupportedFeatureMask   The pointer to CPU feature bits mask 
buffer
   @param[in]  ComparedFeatureBitMask The feature bit mask to be compared
+  @param[in]  BitMaskSizeCPU feature bits mask buffer size.
 
   @retval TRUE   The ComparedFeatureBitMask is set in CPU feature supported 
bits
  mask buffer.
@@ -332,16 +330,14 @@ SupportedMaskCleanBit (
 BOOLEAN
 IsBitMaskMatch (
   IN UINT8   *SupportedFeatureMask,
-  IN UINT8   *ComparedFeatureBitMask
+  IN UINT8   *ComparedFeatureBitMask,
+  IN UINT32  BitMaskSize
   )
 {
   UINTN  Index;
-  UINTN  BitMaskSize;
   UINT8  *Data1;
   UINT8  *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
-
   Data1 = SupportedFeatureMask;
   Data2 = ComparedFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -557,14 +553,14 @@ AnalysisProcessorFeatures (
 //
 // Calculate the last capability on all processors
 //
-SupportedMaskAnd 

[edk2-devel] [Patch 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls gBS service.

[Dong, Eric]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972

AP calls CollectProcessorData() to collect processor info.
CollectProcessorData function finally calls PcdGetSize function to
get DynamicPCD PcdCpuFeaturesSetting value. PcdGetSize will use gBS
which caused below assert info:
Processor Info: Package: 1, MaxCore : 4, MaxThread: 1
Package: 0, Valid Core : 4
ASSERT [CpuFeaturesPei] c:\projects\jsl\jsl_v1193\Edk2\MdePkg\Library
\PeiServicesTablePointerLibIdt\PeiServicesTablePointer.c(48):
PeiServices != ((void *) 0)

This change uses saved global pcd size instead of calls PcdGetSize to
fix this issue.

Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Chandana Kumar 
Cc: Star Zeng 
Signed-off-by: Eric Dong 
---
 .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c  | 13 -
 .../RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c |  5 +
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index aff7ad600c..87bfc64250 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -246,19 +246,20 @@ CpuInitDataInitialize (
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
   @param[in]  OrFeatureBitMask  The feature bit mask to do OR operation
+  @param[in]  BitMaskSize   The CPU feature bits mask buffer size.
+
 **/
 VOID
 SupportedMaskOr (
   IN UINT8   *SupportedFeatureMask,
-  IN UINT8   *OrFeatureBitMask
+  IN UINT8   *OrFeatureBitMask,
+  IN UINT32  BitMaskSize
   )
 {
   UINTN  Index;
-  UINTN  BitMaskSize;
   UINT8  *Data1;
   UINT8  *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Data1 = SupportedFeatureMask;
   Data2 = OrFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -384,12 +385,14 @@ CollectProcessorData (
   //
   SupportedMaskOr (
 CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
-CpuFeature->FeatureMask
+CpuFeature->FeatureMask,
+CpuFeaturesData->BitMaskSize
 );
 } else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo, 
CpuFeature->ConfigData)) {
   SupportedMaskOr (
 CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
-CpuFeature->FeatureMask
+CpuFeature->FeatureMask,
+CpuFeaturesData->BitMaskSize
 );
 }
 Entry = Entry->ForwardLink;
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index fa0f0b41e2..36aabd7267 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -658,6 +658,11 @@ RegisterCpuFeatureWorker (
 InitializeListHead (>FeatureList);
 InitializeSpinLock (>CpuFlags.MemoryMappedLock);
 InitializeSpinLock (>CpuFlags.ConsoleLogLock);
+//
+// Driver has assumption that these three PCD should has same buffer size.
+//
+ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize 
(PcdCpuFeaturesCapability));
+ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize 
(PcdCpuFeaturesSupport));
 CpuFeaturesData->BitMaskSize = (UINT32) BitMaskSize;
   }
   ASSERT (CpuFeaturesData->BitMaskSize == BitMaskSize);
-- 
2.21.0.windows.1


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

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

[edk2-devel] Upcoming Event: TianoCore Design Meeting - APAC/NAMO - Thu, 07/11/2019 6:30pm-7:30pm #cal-reminder

[devel@edk2.groups.io Calendar]

*Reminder:* TianoCore Design Meeting - APAC/NAMO

*When:* Thursday, 11 July 2019, 6:30pm to 7:30pm, (GMT-07:00) America/Los 
Angeles

*Where:* https://zoom.us/j/969264410

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=460286 )

*Organizer:* Stephano Cetola stephano.cet...@intel.com ( 
stephano.cet...@intel.com?subject=Re:%20Event:%20TianoCore%20Design%20Meeting%20-%20APAC%2FNAMO
 )

*Description:*

*Topic:
1. OBB Verification Introduction (Jian Wang)
* 
https://edk2.groups.io/g/devel/files/Designs/2019/0712/OBB%20Verification%20Feature.pdf

*2. Enhance CPU Feature Initialization to Handle Disabled Core#0 (Ray Ni)*
https://edk2.groups.io/g/devel/files/Designs/2019/0712/Enhance%20CPU%20Feature%20Initialization%20to%20Handle%20Disabled%20Core.pdf

Join Zoom Meeting

https://zoom.us/j/969264410 ( https://zoom.us/j/969264410 )

One tap mobile

+16465588656,,969264410# US (New York)

+17207072699,,969264410# US

Dial by your location

+1 646 558 8656 US (New York)

+1 720 707 2699 US

Meeting ID: 969 264 410

Find your local number: https://zoom.us/u/abOtdJckxL

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

View/Reply Online (#43619): https://edk2.groups.io/g/devel/message/43619
Mute This Topic: https://groups.io/mt/32437309/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v5 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Enable MM MP Protocol

[Dong, Eric]

Hi Laszlo,

Thanks for your patch and regression test. I will include your change when I 
push the patches.

Thanks,
Eric

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, July 12, 2019 1:54 AM
> To: devel@edk2.groups.io; Dong, Eric 
> Cc: Ni, Ray 
> Subject: Re: [edk2-devel] [Patch v5 2/2] UefiCpuPkg/PiSmmCpuDxeSmm:
> Enable MM MP Protocol
> 
> Hello Eric,
> 
> On 07/10/19 09:56, Dong, Eric wrote:
> > V5 changes:
> > 1. some small enhancement.
> >
> > v4 changes:
> > 1. Use link list to save the token info.
> >
> > v3 changes:
> > 1. Fix Token clean up too early caused CheckProcedure return error.
> >
> > v2 changes:
> > 1. Remove some duplicated global variables.
> > 2. Enhance token design to support multiple task trig for different APs at
> the same time.
> >
> > V1 changes:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1937
> >
> > Add MM Mp Protocol in PiSmmCpuDxeSmm driver.
> >
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Signed-off-by: Eric Dong 
> > ---
> >  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c| 570
> ++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   |  18 +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   | 193 ++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |   3 +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.c| 344 +++
> >  UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.h| 286 ++
> >  6 files changed, 1391 insertions(+), 23 deletions(-)  create mode
> > 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.c  create mode 100644
> > UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.h
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > index 64fb4d6344..f09e2738c3 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > @@ -140,7 +140,7 @@ ReleaseAllAPs (
> >
> >BspIndex = mSmmMpSyncData->BspIndex;
> >for (Index = mMaxNumberOfCpus; Index-- > 0;) {
> > -if (Index != BspIndex && *(mSmmMpSyncData-
> >CpuData[Index].Present)) {
> > +if (IsPresentAp (Index)) {
> >ReleaseSemaphore (mSmmMpSyncData->CpuData[Index].Run);
> >  }
> >}
> 
> version 5 again fails to build for me, with the following error message:
> 
> > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c: In function 'ReleaseAllAPs':
> > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c:139:37: error: variable
> > 'BspIndex' set but not used [-Werror=unused-but-set-variable]
> 
> With the following incremental patch:
> 
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > index f09e2738c30d..ef16997547b8 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > @@ -136,9 +136,7 @@ ReleaseAllAPs (
> >)
> >  {
> >UINTN Index;
> > -  UINTN BspIndex;
> >
> > -  BspIndex = mSmmMpSyncData->BspIndex;
> >for (Index = mMaxNumberOfCpus; Index-- > 0;) {
> >  if (IsPresentAp (Index)) {
> >ReleaseSemaphore (mSmmMpSyncData->CpuData[Index].Run);
> 
> the build completes fine (using GCC48).
> 
> If you change nothing on the series other than squashing the above fix, you
> can add my:
> 
> Regression-tested-by: Laszlo Ersek 
> 
> to both patches in the series.
> 
> -*-
> 
> Important: please do not push the series until the 5-level paging commits are
> reverted, and reapplied (with Mike's R-b on the MdePkg patch).
> 
> Thanks!
> Laszlo

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

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

[edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Thu, 07/11/2019 5:00pm-5:30pm #cal-reminder

[devel@edk2.groups.io Calendar]

*Reminder:* TianoCore Bug Triage - APAC / NAMO

*When:* Thursday, 11 July 2019, 5:00pm to 5:30pm, (GMT-07:00) America/Los 
Angeles

*Where:* https://zoom.us/j/251103409

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=460283 )

*Organizer:* Stephano Cetola stephano.cet...@intel.com ( 
stephano.cet...@intel.com?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

*Description:*

Join Zoom Meeting

https://zoom.us/j/251103409 ( https://zoom.us/j/251103409 )

One tap mobile

+17207072699,,251103409# US

+16465588656,,251103409# US (New York)

Dial by your location

+1 720 707 2699 US

+1 646 558 8656 US (New York)

Meeting ID: 251 103 409

Find your local number: https://zoom.us/u/acEIbOAUyA ( 
https://zoom.us/u/acEIbOAUyA )

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

View/Reply Online (#43617): https://edk2.groups.io/g/devel/message/43617
Mute This Topic: https://groups.io/mt/32436564/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] PL031: Actually disable interrupts

[Alexander Graf via Groups.Io]

> Am 11.07.2019 um 19:07 schrieb Laszlo Ersek :
> 
>> On 07/10/19 19:13, Leif Lindholm wrote:
>>> On Wed, Jul 10, 2019 at 04:53:11PM +0200, Alexander Graf via Groups.Io 
>>> wrote:
>>> The PL031 interrupt mask register (IMSC) is not very clearly documented
>>> in the PL031 specification. However, bit 0 (RTCIMSC) indicates whether
>>> interrupts are enabled, not disabled.
>> 
>> 3.3.5. Interrupt Mask Set or Clear register, RTCIMSC
>> ... Writing 1 sets the mask. ...
>> 
>> 3.6. Interrupts
>> ... This interrupt is enabled or disabled by changing the mask bit in
>> RTCIMSC. To enable the interrupt, set bit[0] HIGH. ...
>> 
>> *boggle*
> 
> Heh :)
> 
> Alex, out of interest, what were the symptoms of this issue on your end?
> Spurious interrupt confusing the firmware's exception handler, perhaps?

No symptoms that I've encountered. I just stumbled over it while studying the 
device and its respective UEFI code :).

But yes, you would see a spurious interrupt once the RTC wraps around to 0, so 
in 2038. Not that that would matter, as by that time you lost the only wall 
clock reference available on your ARM system anyway :).


Alex


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

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

Re: [edk2-devel] [Patch v5 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Enable MM MP Protocol

[Laszlo Ersek]

Hello Eric,

On 07/10/19 09:56, Dong, Eric wrote:
> V5 changes:
> 1. some small enhancement.
>
> v4 changes:
> 1. Use link list to save the token info.
>
> v3 changes:
> 1. Fix Token clean up too early caused CheckProcedure return error.
>
> v2 changes:
> 1. Remove some duplicated global variables.
> 2. Enhance token design to support multiple task trig for different APs at 
> the same time.
>
> V1 changes:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1937
>
> Add MM Mp Protocol in PiSmmCpuDxeSmm driver.
>
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c| 570 ++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   |  18 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   | 193 ++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |   3 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.c| 344 +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.h| 286 ++
>  6 files changed, 1391 insertions(+), 23 deletions(-)
>  create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.c
>  create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.h
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index 64fb4d6344..f09e2738c3 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -140,7 +140,7 @@ ReleaseAllAPs (
>
>BspIndex = mSmmMpSyncData->BspIndex;
>for (Index = mMaxNumberOfCpus; Index-- > 0;) {
> -if (Index != BspIndex && *(mSmmMpSyncData->CpuData[Index].Present)) {
> +if (IsPresentAp (Index)) {
>ReleaseSemaphore (mSmmMpSyncData->CpuData[Index].Run);
>  }
>}

version 5 again fails to build for me, with the following error message:

> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c: In function 'ReleaseAllAPs':
> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c:139:37: error: variable 'BspIndex' set 
> but not used [-Werror=unused-but-set-variable]

With the following incremental patch:

> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index f09e2738c30d..ef16997547b8 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -136,9 +136,7 @@ ReleaseAllAPs (
>)
>  {
>UINTN Index;
> -  UINTN BspIndex;
>
> -  BspIndex = mSmmMpSyncData->BspIndex;
>for (Index = mMaxNumberOfCpus; Index-- > 0;) {
>  if (IsPresentAp (Index)) {
>ReleaseSemaphore (mSmmMpSyncData->CpuData[Index].Run);

the build completes fine (using GCC48).

If you change nothing on the series other than squashing the above fix,
you can add my:

Regression-tested-by: Laszlo Ersek 

to both patches in the series.

-*-

Important: please do not push the series until the 5-level paging
commits are reverted, and reapplied (with Mike's R-b on the MdePkg
patch).

Thanks!
Laszlo

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

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

Re: [edk2-devel] [PATCH] PL031: Actually disable interrupts

[Laszlo Ersek]

On 07/10/19 19:13, Leif Lindholm wrote:
> On Wed, Jul 10, 2019 at 04:53:11PM +0200, Alexander Graf via Groups.Io wrote:
>> The PL031 interrupt mask register (IMSC) is not very clearly documented
>> in the PL031 specification. However, bit 0 (RTCIMSC) indicates whether
>> interrupts are enabled, not disabled.
> 
> 3.3.5. Interrupt Mask Set or Clear register, RTCIMSC
> ... Writing 1 sets the mask. ...
> 
> 3.6. Interrupts
> ... This interrupt is enabled or disabled by changing the mask bit in
> RTCIMSC. To enable the interrupt, set bit[0] HIGH. ...
> 
> *boggle*

Heh :)

Alex, out of interest, what were the symptoms of this issue on your end?
Spurious interrupt confusing the firmware's exception handler, perhaps?

Thanks!
Laszlo

> 
>> So before this commit, we were actually *enabling* interrupts for the RTC.
>>
>> This patch changes the logic to instead disable interrupts when they
>> are not disabled already.
> 
> Thanks for finding/fixing this.
> 
>> Signed-off-by: Alexander Graf 
> 
> Reviewed-by: Leif Lindholm 
> 
> I took the liberty to change the subject line to
> "ArmPlatformPkg: Actually disable PL031 interrupts"
> (We tend to start with the top-level directory.)
> 
> Pushed as 8df52631e53c.
> 
> /
> Leif
> 
>> ---
>>  .../Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git 
>> a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c 
>> b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
>> index b630a5cfbf..75c95985d4 100644
>> --- a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
>> +++ b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
>> @@ -80,8 +80,8 @@ InitializePL031 (
>>}
>>  
>>// Ensure interrupts are masked. We do not want RTC interrupts in UEFI
>> -  if ((MmioRead32 (mPL031RtcBase + 
>> PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER) & PL031_SET_IRQ_MASK) != 
>> PL031_SET_IRQ_MASK) {
>> -MmioOr32 (mPL031RtcBase + PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER, 
>> PL031_SET_IRQ_MASK);
>> +  if ((MmioRead32 (mPL031RtcBase + 
>> PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER) & PL031_SET_IRQ_MASK) != 0) {
>> +MmioWrite32 (mPL031RtcBase + 
>> PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER, 0);
>>}
>>  
>>// Clear any existing interrupts
>> -- 
>> 2.17.1
>>
>>
>>
>>
> 
> 
> 


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

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

Re: [edk2-devel] GCC compiler version

[Laszlo Ersek]

Hello Liming,

On 07/11/19 07:07, Liming Gao wrote:
> Hi, all
>   Recently, I find some failure with the different GCC compiler version. I 
> want to collect which GCC version are used by you. Now, I use GCC 5.5 for the 
> verification. It may be a little old.

I use multiple GCC* toolchains, with different frequencies. My primary
work laptop runs RHEL7, so I most frequently build (for IA32/X64) with
GCC48:

> gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)

Whenever I build on RHEL8, I use gcc-8.

On occasion, I might build on recent Fedora (gcc-9 I guess), but that's
quite exceptional for me.

Thanks,
Laszlo

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

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

[edk2-devel] [off-topic] Readme.md: add submodule policy and clone commands

[Laszlo Ersek]

(off-topic, as marked in the subject)

On 07/10/19 19:17, Leif Lindholm wrote:
> On Wed, Jul 10, 2019 at 05:08:32PM +, Kinney, Michael D wrote:
>> Leif,
>>
>> I think the following command has to be run from the git
>> dir that the edk2 repo was cloned into.
>>  
>> $ git submodule update --init
>>
>> So I think the correct instructions would be:
>>
>> $ git clone https://github.com/tianocore/edk2.git
>> $ cd edk2
>> $ git submodule update --init
> 
> I only said I reviewed it, not that the review was correct ;)
> 
> Yeah, good catch. I just mentally filtered out the clone bit.
> 
>> $ cd ..
> 
> Why would you want to leave? 

Heh, that's a good one :) Reminds me of a blog post or similar where the
question was posed (tongue-in-cheek of course), "why boot at all? UEFI
gives you everything!" :)

Thanks!
Laszlo

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

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

Re: [edk2-devel] [PATCH v6 edk2-platforms 1/1] Silicon/Synopsys/Designware/Driver: DwEmacSnpDxe: Add DesignWare EMAC driver

[Leif Lindholm]

Hi Tzy Way,

Nearly there, but please make sure BaseTools/Scripts/PatchCheck.py
runs without warnings or errors (there are quite a few still in this
version).

Building the .dsc using current edk2 fails with
'MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf is not found in packages path'
The path needs to be updated to NetworkPkg/Library/DxeNetLib/DxeNetLib.inf.

Secondly, could you add NOOPT as a BUILD_TARGET, in addition to DEBUG
and RELEASE?

Also, Debian's gcc 8.3 throws a set of compilation errors (both DEBUG
and RELEASE) when building for AARCH64:

/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c:
 In function <80><98>EmacSetupTxdesc<80><99>:
/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c:236:81:
 error: iteration 9 invokes undefined behavior 
[-Werror=aggressive-loop-optimizations]
 TxDescriptor->AddrNext = (UINT32)(UINTN)EmacDriver->TxdescRingMap[Index + 
1].AddrMap;
 
^~~~
/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c:233:3:
 note: within this loop
   for (Index = 0; Index < CONFIG_TX_DESCR_NUM; Index++) {
   ^~~
/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c:
 In function <80><98>EmacSetupRxdesc<80><99>:
/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c:270:81:
 error: iteration 9 invokes undefined behavior 
[-Werror=aggressive-loop-optimizations]
 RxDescriptor->AddrNext = (UINT32)(UINTN)EmacDriver->RxdescRingMap[Index + 
1].AddrMap;
 
^~~~
/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c:267:3:
 note: within this loop
   for (Index = 0; Index < CONFIG_RX_DESCR_NUM; Index++) {
   ^~~
cc1: all warnings being treated as errors


It also throws the following errors when building for ARM:
/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c:
 In function <80><98>SnpTransmit<80><99>:
/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c:962:21:
 error: cast to pointer from integer of different size 
[-Werror=int-to-pointer-cast]
   TxDescriptorMap = (VOID *)Snp->MacDriver.TxdescRingMap[DescNum].AddrMap;
 ^
/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c:1026:51:
 error: cast from pointer to integer of different size 
[-Werror=pointer-to-int-cast]
 Snp->RecycledTxBuf[Snp->RecycledTxBufCount] = (UINT64) Data;
   ^
/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c:
 In function <80><98>SnpReceive<80><99>:
/work/git/edk2-platforms/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c:1143:21:
 error: cast to pointer from integer of different size 
[-Werror=int-to-pointer-cast]
   RxDescriptorMap = (VOID *)Snp->MacDriver.RxdescRingMap[DescNum].AddrMap;
 ^
cc1: all warnings being treated as errors


Could you please look into and address these issues for the next version?

Best Regards,

Leif

On Mon, Jul 08, 2019 at 03:05:07PM +0800, tzy.way@intel.com wrote:
> From: Ooi Tzy Way 
> 
> Add driver support for the Ethernet MAC based on Synopsys DesignWare
> 3504-0 Universal 10/100/1000 Ethernet MAC and KSZ9031 PHY
> 
> Cc: Ard BieSheuvel 
> Cc: Leif Lindholm 
> Cc: Michael D Kinney 
> Cc: Loh Tien Hock 
> 
> Contributed-under: Tianocore Contribution Agreement 1.1
> Signed-off-by: Ooi Tzy Way 
> 
> ---
> v6:
> - Update to recent version for EDK2 specific file formats
> - Update the directory layout to Silicon/Synopsys/DesignWare
> - Add a DesignWare.dsc for building this driver
> - Update the license
> - Update the .c file to declare its own include file
> - Remove __ in defining the header file
> - Fix indentation
> - Delete commented-out code

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

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

Re: [edk2-devel] [Patch v5 1/2] MdePkg: Add new MM MP Protocol definition.

[Laszlo Ersek]

Hi Ray,

On 07/11/19 08:42, Ni, Ray wrote:
> + MdePkg maintainers
> 
> We need to follow the open source community process to include MdePkg
> maintainers for this change review.
> 
> Laszlo,
> You are in the Cc list and had given Regression-Tested-By for V3 patch series.
> It would be great if you could give patch owner a hint to avoid breaking the 
> process
> though I understand it's not responsibility of the people in the CC list. 

I'm unaware of any changes to the process.

In the commit message of the patch, Cc: tags should be included for all
"M" folks under XxxPkg, and also for all "R" folks who are listed with
interest in the given area (possibly the entirety of XxxPkg). The
contributor's git config should not include a
"sendemail.suppresscc=bodycc" setting; otherwise those Cc: tags will not
actually copy the intended reviewers.

For a given XxxPkg patch, at least one Acked-by or Required-by is needed
from an "M" person listed under XxxPkg in "Maintainers.txt", on the
mailing list, before the patch can be pushed.

Reviews and comments from others are welcome for the patch. The patch
should not be pushed as long as reasonable questions are still open for
it (regardless of origin). A patch should generally spend at least 24
hours on the list before it is pushed, even if an "M" approval arrives
earlier than that (so that others have a chance at noticing the patch
and at commenting). Once no questions seem to remain open, the patch has
spent the minimum time on the list, and there is public "M" approval,
the patch can be pushed.

An "M" person can defer to another "M" or "R" person, if he/she chooses
so. This is best done formally, i.e., wait until the deferred-to person
approves the patch, and then the first "M" person can give their own
Acked-by. (Standing for "I haven't reviewed in detail, but given the
circumstances / other reviews, I'm OK with this patch".)

When the patch is pushed, all feedback tags, given on the list for the
patch (regardless of origin), should be picked up, by whoever is pushing
the patch.

Let's consider the current state of this series, as an example.

- The first patch (for MdePkg) has an "M" review, from Liming.
- The second patch (for UefiCpuPkg) has an "M" review, from you.
- The series has an open question (regarding regression testing), from
myself.
- The series was posted more than 24 hours ago (as far as I can see).

So once I (hopefully) report back with an R-t-b for the series, the
series becomes pushable (assuming noone opens another question
meanwhile). In that case, any "M" person is welcome to push the patch
series -- in the current case, it will likely be Eric. When Eric
prepares for pushing the series, he's expected to pick up
- Liming's R-b for the first patch,
- my (to be posted) R-t-b for the first patch,
- your R-b for the second patch,
- my (to be posted) R-t-b for the second patch.

(I will likely send my R-t-b in response to the blurb (the "v5 0/2"
email), which means that the feedback tag applies to every patch in the
series.)

This "tag pickup" procedure is difficult to get right when using a MUA
that does not support a "threaded" view, and it is easy to get right
with a MUA that does. When I'm about to push a series, I tend to perform
one full scan over the thread:

- Whenever I reach patch-level feedback, I run "git rebase -i" for the
full series, just to pick up that one tag. (A single "reword" action
among the "pick"s.)

- Whenever I reach series-level feedback (grouped under the blurb), I
run "git rebase -i" for the full series again, and I apply the tag to
every patch in the series (all actions are set to "reword").

Thanks
Laszlo

>> -Original Message-
>> From: Dong, Eric
>> Sent: Wednesday, July 10, 2019 3:56 PM
>> To: devel@edk2.groups.io
>> Cc: Ni, Ray ; Laszlo Ersek 
>> Subject: [Patch v5 1/2] MdePkg: Add new MM MP Protocol definition.
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1937
>>
>> EFI MM MP Protocol is defined in the PI 1.5 specification.
>>
>> The MM MP protocol provides a set of functions to allow execution of
>> procedures on processors that have entered MM. This protocol has the
>> following properties:
>> 1. The caller can invoke execution of a procedure on a processor, other than
>> the caller, that has also entered MM. Supports blocking and non-blocking
>> modes of operation.
>> 2. The caller can invoke a procedure on multiple processors. Supports
>> blocking and non-blocking modes of operation.
>>
>> Cc: Ray Ni 
>> Cc: Laszlo Ersek 
>> Signed-off-by: Eric Dong 
>> Reviewed-by: Ray Ni 
>> ---
>>  MdePkg/Include/Pi/PiMultiPhase.h |  16 ++
>>  MdePkg/Include/Protocol/MmMp.h   | 333
>> +++
>>  MdePkg/MdePkg.dec|   3 +
>>  3 files changed, 352 insertions(+)
>>  create mode 100644 MdePkg/Include/Protocol/MmMp.h
>>
>> diff --git a/MdePkg/Include/Pi/PiMultiPhase.h
>> b/MdePkg/Include/Pi/PiMultiPhase.h
>> index eb12527767..a5056799e1 100644
>> --- 

[edk2-devel] Upcoming Event: TianoCore Community Meeting - EMEA/NAMO - Thu, 07/11/2019 9:00am-10:00am #cal-reminder

[devel@edk2.groups.io Calendar]

*Reminder:* TianoCore Community Meeting - EMEA/NAMO

*When:* Thursday, 11 July 2019, 9:00am to 10:00am, (GMT-07:00) America/Los 
Angeles

*Where:* https://zoom.us/j/188375690

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=458033 )

*Organizer:* Stephano Cetola stephano.cet...@intel.com ( 
stephano.cet...@intel.com?subject=Re:%20Event:%20TianoCore%20Community%20Meeting%20-%20EMEA%2FNAMO
 )

*Description:*

Join Zoom Meeting

https://zoom.us/j/188375690 ( https://zoom.us/j/188375690 )

One tap mobile

+16465588656,,188375690# US (New York)

+17207072699,,188375690# US

Dial by your location

+1 646 558 8656 US (New York)

+1 720 707 2699 US

Meeting ID: 188 375 690

Find your local number: https://zoom.us/u/abOtdJckxL

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

View/Reply Online (#43609): https://edk2.groups.io/g/devel/message/43609
Mute This Topic: https://groups.io/mt/32431457/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object to handle build conf file

[Bob Feng]

Yes.  it will work after I fix this bug in WorkspaceAutoGen._GetMetaFiles()

Here, the code assumes the tools_def.txt and build_rule.txt are always in Conf 
directory.

   #
# add build_rule.txt & tools_def.txt
#
AllWorkSpaceMetaFiles.add(os.path.join(GlobalData.gConfDirectory, 
gDefaultBuildRuleFile))
AllWorkSpaceMetaFiles.add(os.path.join(GlobalData.gConfDirectory, 
gDefaultToolsDefFile))

Thanks,
Bob

-Original Message-
From: Gao, Liming 
Sent: Thursday, July 11, 2019 10:31 PM
To: Feng, Bob C ; devel@edk2.groups.io
Subject: RE: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object to handle 
build conf file

Bob:


> -Original Message-
> From: Feng, Bob C
> Sent: Thursday, July 11, 2019 8:13 PM
> To: devel@edk2.groups.io; Feng, Bob C ; Gao, 
> Liming 
> Subject: RE: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object 
> to handle build conf file
> 
> Liming,
> 
> I tested the cases:
> 1. move tool_def.txt and build_rule.txt from Conf to another folder 
> and update target.txt to set BUILD_RULE_CONF and TOOL_CHAIN_CONF to point to 
> the new folder.

If their file name is also changed in target.txt, does it work in current 
BaseTools? I just want to make sure keep the same support scope with this 
change. 

Thanks
Liming
> 2. Move Conf folder to a new place and Set WORKSPACE to point the 
> parent folder of Conf. Append original WORKSPACE folder to 
> PACKAGES_PATH
> 
> My patch works fine.
> 
> But there is a bug in original code for case 1#, I'll send out another patch 
> to fix that.
> 
> Thanks,
> Bob
> 
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of 
> Bob Feng
> Sent: Thursday, July 11, 2019 5:27 PM
> To: Gao, Liming ; devel@edk2.groups.io
> Subject: Re: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object 
> to handle build conf file
> 
> Liming,
> 
> Thanks for your comments. I'll  test the cases you mentioned.
> 
> Thanks,
> Bob
> 
> -Original Message-
> From: Gao, Liming
> Sent: Thursday, July 11, 2019 5:11 PM
> To: Feng, Bob C ; devel@edk2.groups.io
> Subject: RE: [Patch 1/1] BaseTools: Singleton the object to handle 
> build conf file
> 
> Bob:
>   target.txt is from Conf directory. build_rule.txt and tools_def.txt 
> are specified in target.txt. Please take care this case that they may have 
> the different file name.
> 
>   And, Conf directory is relative to WORKSPACE and PACKAGES_PATH. Does this 
> patch support it?
> 
> Thanks
> Liming
> >-Original Message-
> >From: Feng, Bob C
> >Sent: Friday, June 28, 2019 3:07 PM
> >To: devel@edk2.groups.io
> >Cc: Gao, Liming ; Feng, Bob C 
> >
> >Subject: [Patch 1/1] BaseTools: Singleton the object to handle build 
> >conf file
> >
> >BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875
> >
> >The build config files are target.txt, build rule, tooldef During a 
> >build, the config is not changed, so the object to handle them need 
> >to be singleton.
> >
> >Cc: Liming Gao 
> >Signed-off-by: Bob Feng 
> >---
> > BaseTools/Source/Python/AutoGen/AutoGen.py| 33 ++--
> > .../Source/Python/AutoGen/BuildEngine.py  | 22 
> > .../Python/Common/TargetTxtClassObject.py |  2 +
> > .../Python/Common/ToolDefClassObject.py   |  6 ++-
> > BaseTools/Source/Python/GenFds/GenFds.py  |  4 +-
> > .../Python/GenFds/GenFdsGlobalVariable.py | 54 ---
> > .../Source/Python/Workspace/DscBuildData.py   |  8 +--
> > BaseTools/Source/Python/build/build.py| 29 +++---
> > 8 files changed, 62 insertions(+), 96 deletions(-)
> >
> >diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> >b/BaseTools/Source/Python/AutoGen/AutoGen.py
> >index e8e09dc8a366..a1f7f5641e09 100644
> >--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> >+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> >@@ -22,11 +22,12 @@ from . import GenC  from . import GenMake  from . 
> >import GenDepex  from io import BytesIO
> >
> > from .StrGather import *
> >-from .BuildEngine import BuildRule
> >+from .BuildEngine import BuildRuleObj as BuildRule from .BuildEngine 
> >+import
> >gDefaultBuildRuleFile,AutoGenReqBuildRuleVerNum
> > import shutil
> > from Common.LongFilePathSupport import CopyLongFilePath  from 
> >Common.BuildToolError import *  from Common.DataType import *  from 
> >Common.Misc import * @@ -76,16 +77,10 @@ gEfiVarStoreGuidPattern =
> >re.compile("\s*guid\s*=\s*({.*?{.*?}\s*})")
> >
> > ## Mapping Makefile type
> > gMakeTypeMap = {TAB_COMPILER_MSFT:"nmake", "GCC":"gmake"}
> >
> >
> >-## Build rule configuration file
> >-gDefaultBuildRuleFile = 'build_rule.txt'
> >-
> >-## Build rule default version
> >-AutoGenReqBuildRuleVerNum = "0.1"
> >-
> > ## default file name for AutoGen
> > gAutoGenCodeFileName = "AutoGen.c"
> > gAutoGenHeaderFileName = "AutoGen.h"
> > gAutoGenStringFileName = "%(module_name)sStrDefs.h"
> > gAutoGenStringFormFileName = "%(module_name)sStrDefs.hpk"
> >@@ -1964,32 +1959,10 

Re: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object to handle build conf file

[Liming Gao]

Bob:


> -Original Message-
> From: Feng, Bob C
> Sent: Thursday, July 11, 2019 8:13 PM
> To: devel@edk2.groups.io; Feng, Bob C ; Gao, Liming 
> 
> Subject: RE: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object to 
> handle build conf file
> 
> Liming,
> 
> I tested the cases:
> 1. move tool_def.txt and build_rule.txt from Conf to another folder and 
> update target.txt to set BUILD_RULE_CONF and
> TOOL_CHAIN_CONF to point to the new folder.

If their file name is also changed in target.txt, does it work in current 
BaseTools? I just want to make sure keep the same support scope with this 
change. 

Thanks
Liming
> 2. Move Conf folder to a new place and Set WORKSPACE to point the parent 
> folder of Conf. Append original WORKSPACE folder to
> PACKAGES_PATH
> 
> My patch works fine.
> 
> But there is a bug in original code for case 1#, I'll send out another patch 
> to fix that.
> 
> Thanks,
> Bob
> 
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Bob Feng
> Sent: Thursday, July 11, 2019 5:27 PM
> To: Gao, Liming ; devel@edk2.groups.io
> Subject: Re: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object to 
> handle build conf file
> 
> Liming,
> 
> Thanks for your comments. I'll  test the cases you mentioned.
> 
> Thanks,
> Bob
> 
> -Original Message-
> From: Gao, Liming
> Sent: Thursday, July 11, 2019 5:11 PM
> To: Feng, Bob C ; devel@edk2.groups.io
> Subject: RE: [Patch 1/1] BaseTools: Singleton the object to handle build conf 
> file
> 
> Bob:
>   target.txt is from Conf directory. build_rule.txt and tools_def.txt are 
> specified in target.txt. Please take care this case that they may have
> the different file name.
> 
>   And, Conf directory is relative to WORKSPACE and PACKAGES_PATH. Does this 
> patch support it?
> 
> Thanks
> Liming
> >-Original Message-
> >From: Feng, Bob C
> >Sent: Friday, June 28, 2019 3:07 PM
> >To: devel@edk2.groups.io
> >Cc: Gao, Liming ; Feng, Bob C
> >
> >Subject: [Patch 1/1] BaseTools: Singleton the object to handle build
> >conf file
> >
> >BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875
> >
> >The build config files are target.txt, build rule, tooldef During a
> >build, the config is not changed, so the object to handle them need to
> >be singleton.
> >
> >Cc: Liming Gao 
> >Signed-off-by: Bob Feng 
> >---
> > BaseTools/Source/Python/AutoGen/AutoGen.py| 33 ++--
> > .../Source/Python/AutoGen/BuildEngine.py  | 22 
> > .../Python/Common/TargetTxtClassObject.py |  2 +
> > .../Python/Common/ToolDefClassObject.py   |  6 ++-
> > BaseTools/Source/Python/GenFds/GenFds.py  |  4 +-
> > .../Python/GenFds/GenFdsGlobalVariable.py | 54 ---
> > .../Source/Python/Workspace/DscBuildData.py   |  8 +--
> > BaseTools/Source/Python/build/build.py| 29 +++---
> > 8 files changed, 62 insertions(+), 96 deletions(-)
> >
> >diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> >b/BaseTools/Source/Python/AutoGen/AutoGen.py
> >index e8e09dc8a366..a1f7f5641e09 100644
> >--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> >+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> >@@ -22,11 +22,12 @@ from . import GenC
> > from . import GenMake
> > from . import GenDepex
> > from io import BytesIO
> >
> > from .StrGather import *
> >-from .BuildEngine import BuildRule
> >+from .BuildEngine import BuildRuleObj as BuildRule from .BuildEngine
> >+import
> >gDefaultBuildRuleFile,AutoGenReqBuildRuleVerNum
> > import shutil
> > from Common.LongFilePathSupport import CopyLongFilePath  from
> >Common.BuildToolError import *  from Common.DataType import *  from
> >Common.Misc import * @@ -76,16 +77,10 @@ gEfiVarStoreGuidPattern =
> >re.compile("\s*guid\s*=\s*({.*?{.*?}\s*})")
> >
> > ## Mapping Makefile type
> > gMakeTypeMap = {TAB_COMPILER_MSFT:"nmake", "GCC":"gmake"}
> >
> >
> >-## Build rule configuration file
> >-gDefaultBuildRuleFile = 'build_rule.txt'
> >-
> >-## Build rule default version
> >-AutoGenReqBuildRuleVerNum = "0.1"
> >-
> > ## default file name for AutoGen
> > gAutoGenCodeFileName = "AutoGen.c"
> > gAutoGenHeaderFileName = "AutoGen.h"
> > gAutoGenStringFileName = "%(module_name)sStrDefs.h"
> > gAutoGenStringFormFileName = "%(module_name)sStrDefs.hpk"
> >@@ -1964,32 +1959,10 @@ class PlatformAutoGen(AutoGen):
> > ## Return the build options specific for EDKII modules in this platform
> > @cached_property
> > def EdkIIBuildOption(self):
> > return self._ExpandBuildOption(self.Platform.BuildOptions,
> >EDKII_NAME)
> >
> >-## Parse build_rule.txt in Conf Directory.
> >-#
> >-#   @retval BuildRule object
> >-#
> >-@cached_property
> >-def BuildRule(self):
> >-BuildRuleFile = None
> >-if TAB_TAT_DEFINES_BUILD_RULE_CONF in
> >self.Workspace.TargetTxt.TargetTxtDictionary:
> >-BuildRuleFile =
> >self.Workspace.TargetTxt.TargetTxtDictionary[TAB_TAT_DEFINES_BUILD_RUL
> 

Re: [edk2-devel] [PATCH] Revert "FmpDevicePkg: Fix various typos"

[Chiu, Chasel]

Revert patch submitted: efa12a3f029bd6ff4d2ada406c285f001b252907
Reapply patch submitted: 91cc60bafc7d6e49b7bc85990f895d6228f51364 

Thanks!
Chasel

> -Original Message-
> From: Antoine Cœur [mailto:co...@gmx.fr]
> Sent: Thursday, July 11, 2019 5:37 PM
> To: devel@edk2.groups.io
> Cc: Antoine Cœur ; Zeng, Star ; Chiu,
> Chasel 
> Subject: [PATCH] Revert "FmpDevicePkg: Fix various typos"
> 
> This reverts commit f527942e6bdd9f198db90f2de99a0482e9be5b1b.
> Commit message was incorrect.
> 
> Signed-off-by: Coeur 
> ---
>  .../FspSecCore/Ia32/FspApiEntryM.nasm |  4 +--
>  .../FspSecCore/Ia32/InitializeFpu.nasm|  4 +--
>  .../FspSecCore/Ia32/SaveRestoreSseNasm.inc|  4 +--
>  IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm   |  4 +--
>  IntelFsp2Pkg/FspSecCore/SecFsp.c  |  4 +--
>  IntelFsp2Pkg/FspSecCore/SecMain.c |  2 +-
>  .../FspSecCore/Vtf0/Ia16/ResetVec.asm16   |  4 +--
>  IntelFsp2Pkg/Include/FspEas/FspApi.h  |  8 ++---
>  .../Include/Library/FspSecPlatformLib.h   |  4 +--
>  IntelFsp2Pkg/Library/BaseCacheLib/CacheLib.c  | 12 +++
>  .../BaseFspDebugLibSerialPort/DebugLib.c  | 34 +--
>  .../BaseFspSwitchStackLib/Ia32/Stack.nasm |  4 +--
>  .../SecFspSecPlatformLibNull/Ia32/Flat32.nasm |  4 +--
>  .../PlatformSecLibNull.c  |  4 +--
>  IntelFsp2Pkg/Tools/GenCfgOpt.py   |  2 +-
>  IntelFsp2Pkg/Tools/PatchFv.py |  2 +-
>  .../Tools/UserManuals/GenCfgOptUserManual.md  |  2 +-
>  .../Tools/UserManuals/PatchFvUserManual.md|  2 +-
>  18 files changed, 52 insertions(+), 52 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> index e7261b41cd..f14c18c7b9 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> @@ -194,9 +194,9 @@ StackSetupDone:
> 
>;
>; Pass BFV into the PEI Core
> -  ; It uses relative address to calculate the actual boot FV base
> +  ; It uses relative address to calucate the actual boot FV base
>; For FSP implementation with single FV, PcdFspBootFirmwareVolumeBase
> and
> -  ; PcdFspAreaBaseAddress are the same. For FSP with multiple FVs,
> +  ; PcdFspAreaBaseAddress are the same. For FSP with mulitple FVs,
>; they are different. The code below can handle both cases.
>;
>callASM_PFX(AsmGetFspBaseAddress)
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
> index ebc91c41e4..e1886ea11b 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
> @@ -1,6 +1,6 @@
>  
> ;--
>  ;
> -; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
> +; Copyright (c) 2015, Intel Corporation. All rights reserved.
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent  ;  ; Abstract:
> @@ -46,7 +46,7 @@ ASM_PFX(InitializeFloatingPointUnits):
>  fldcw[ASM_PFX(mFpuControlWord)]
> 
>  ;
> -; Use CpuId instruction (CPUID.01H:EDX.SSE[bit 25] = 1) to test
> +; Use CpuId instructuion (CPUID.01H:EDX.SSE[bit 25] = 1) to test
>  ; whether the processor supports SSE instruction.
>  ;
>  mov eax, 1
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> b/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> index 4c321cbece..b257deb76c 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> @@ -1,6 +1,6 @@
>  
> ;--
>  ;
> -; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
> +; Copyright (c) 2015, Intel Corporation. All rights reserved.
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent  ;  ; Abstract:
> @@ -150,7 +150,7 @@ NextAddress:
>  fldcw   [FpuControlWord]
> 
>  ;
> -; Use CpuId instruction (CPUID.01H:EDX.SSE[bit 25] = 1) to test
> +; Use CpuId instructuion (CPUID.01H:EDX.SSE[bit 25] = 1) to
> + test
>  ; whether the processor supports SSE instruction.
>  ;
>  mov eax, 1
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm
> index 5a7e27c240..d72212ed45 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm
> @@ -1,6 +1,6 @@
>  
> ;--
>  ;
> -; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
> +; Copyright (c) 2015, Intel Corporation. All rights reserved.
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent  ;  ; Abstract:
> @@ -58,7 +58,7 @@ ASM_PFX(SecSwitchStack):
>  mov   esp, eax ; From now, 

Re: [edk2-devel] ExtScsiPassThru support for Logical SCSI devices

[Wu, Hao A]

Hello Ravi Kumar,

For: Now the issue is, The same GUID cannot be installed more than once onto 
the same handle (Controller handle).
So to support 2nd instance we need to create new device path (virtual device 
path) for that.
Yes. A new handled is needed for the ExtScsiPassThru protocol logical instance.

For: Once more issue is, how do we associate this newly created device path and 
SCSI PassThru instance with actual controller . ?
I think we can either:
Use OpenProtocol() on behalf of the new handle with an attribute of 
BY_CHILD_CONTROLLER to create a parent-child relationship.
Or use device path (append nodes after the actual controller’s device path when 
constructing the device path of the new handle)
to associate them.

For: I dont think, because if any driver supports only Instance with 
EFI_EXT_SCSI_PASS_THRU_ATTRIBUTES_PHYSICAL bit set, can't they create child 
handles ??
As far as i know, Logical or Physical Instances are no where related to media 
access protocols.
Per my understanding, the media device drivers in edk2 are written in a manner 
to only produce media access protocols upon logical instance of the PassThru 
protocols.
Some examples are:
MdeModulePkg\Bus\Ata\AtaBusDxe\AtaBus.c - Function 
AtaBusDriverBindingSupported()
MdeModulePkg\Bus\Scsi\ScsiDiskDxe\ScsiDisk.c - Function 
DetermineInstallBlockIo()

For: Yes user can get the list of EFI_EXT_SCSI_PASS_THRU_PROTOCOL instances by 
querying, but how the user can relate a particular instance with a controller. 
? Because the 2nd of SCSI PassThru is created on new device path.(which will 
not have any other protocols, other than device path and ext scsi pass thru).
In my opinion, the device path together with the attribute of the PassThru 
protocol instance will be sufficient for consumers.
The physical one will be used for device management, while the logical one will 
be used to further produce upper-layer protocols.


Best Regards,
Hao Wu

From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ravi 
Kumar Siadri
Sent: Monday, July 08, 2019 4:36 PM
To: Wu, Hao A
Cc: devel@edk2.groups.io
Subject: Re: [edk2-devel] ExtScsiPassThru support for Logical SCSI devices

Hi Hao Wu,

If a Controller supports both Logical and Physical Devices, as per spec we need 
to create 2 instances.
1. Instance with EFI_EXT_SCSI_PASS_THRU_ATTRIBUTES_LOGICAL bit set.
2.  Instance with EFI_EXT_SCSI_PASS_THRU_ATTRIBUTES_PHYSICAL bit set.

And Each EFI_EXT_SCSI_PASS_THRU_PROTOCOL instance must have an associated 
device path.

Now the issue is, The same GUID cannot be installed more than once onto the 
same handle (Controller handle).
So to support 2nd instance we need to create new device path (virtual device 
path) for that.

Once more issue is, how do we associate this newly created device path and SCSI 
PassThru instance with actual controller . ?


The logical protocol instance is used to produce media access protocols (like 
BlockIO protocol). Some physical HW may not be visible/accessible (e.g. HW used
for data backup, depending on the RAID configuration) by this logical protocol 
instance. The physical one is used to directly send commands to all the 
managing devices.

I dont think, because if any driver supports only Instance with 
EFI_EXT_SCSI_PASS_THRU_ATTRIBUTES_PHYSICAL bit set, can't they create child 
handles ??

As far as i know, Logical or Physical Instances are no where related to media 
access protocols. Even without SCSI passthru protocol on controller handle 
also, Block Io protocol can be installed on Controller Child devices. (Its the 
uefi driver wish, how it passes commands to the Child's of controller). Logical 
Instance is to interact with Logical Device and Physical Instance is to 
interact with Physical devices as per spec.

Users can always use the EFI Boot Service 'LocateHandleBuffer' to get all the 
EFI_EXT_SCSI_PASS_THRU_PROTOCOL instances. Then, users can evaluate each 
instance to see if it is the one they needed.

Yes user can get the list of EFI_EXT_SCSI_PASS_THRU_PROTOCOL instances by 
querying, but how the user can relate a particular instance with a controller. 
? Because the 2nd of SCSI PassThru is created on new device path.(which will 
not have any other protocols, other than device path and ext scsi pass thru).

Thanks,
Ravi Kumar


On Mon, Jul 8, 2019 at 8:39 AM Wu, Hao A 
mailto:hao.a...@intel.com>> wrote:
Hello Ravi Kumar,

May I know the specific issue you met when 2 ExtScsiPassThru instances are
provided for the device?

For some of your previous questions:

• “Any background why EFI_EXT_SCSI_PASS_THRU_PROTOCOL needed 2 
instances ?”
The logical protocol instance is used to produce media access protocols (like
BlockIO protocol). Some physical HW may not be visible/accessible (e.g. HW used
for data backup, depending on the RAID configuration) by this logical protocol
instance.

The physical one is used to directly send commands to all the managing devices.


• “How User locates 

Re: [edk2-devel] [PATCH v2 2/3] MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level paging

[Laszlo Ersek]

On 07/11/19 05:25, Ni, Ray wrote:
> Laszlo, Mike,
> Sorry I did violate the process.
> I had two assumptions which led me violate the process:
> 1. Reviewed-by from UefiCpuPkg maintainers on this IA32_CR4 change is more 
> important
> than that from MdePkg maintainers. In another word, I thought if 
> UefiCpuPkg maintainers
> agree with this change, MdePkg maintainers should have no concerns.
> (It's a wrong assumption. MdePkg maintainers may have some general 
> suggestions, e.g.:
> name, location, comments and etc..)
> 2. This change is directly from the published white paper and there is no 
> other option regarding
>  this IA32_CR4 change.
> (It's a wrong assumption. MdePkg maintainers may have some general 
> suggestions, e.g.:
> name, location, comments and etc..)

Both of these assumptions could make perfect sense *to you*, but the
rules exist to uphold a single standard for everyone. I didn't expect
Mike or Liming to find an issue in the "MdePkg/BaseLib.h" patch. I did
expect the process to be followed.

I fully support if you and Eric are added to MdePkg as co-maintainers,
for content that is closely related to UefiCpuPkg. Then, the assumptions
will be codified, and they will be clear to everyone.

To explain "single standard" a bit more: sometimes I too submit code for
MdePkg. (Right now, I happen to have a series pending review.) If I were
cynical, my thinking could go, "well, if other folks can push to MdePkg
without MdePkg maintainer approval, so can I".

Do you see my problem? Where does it end?

A maintainer A-b or R-b on the list is objective; it's a fact.

> I agree I should get Reviewed-by tag from MdePkg maintainers. My assumptions 
> are wrong.
> 
> To strictly follow the process, I will:
> 1. Post a patch series to revert the 3 patches.
>  Since this change doesn't break any functionality (does break the 
> process), I will wait for
>  Reviewed-by from each package maintainer and make sure push the patches 
> after 24h.
> 2. After step #1, post a new patch series to add the 3 patches back with 
> Reviewed-by tag from
> Eric and Mike, Regression-Tested-by from you.
> 
> Do you think it follows the existing process?

Yes, this can work, but I also don't mind if you use git-revert +
git-cherry-pick + git-push in this instance (as explained elsewhere), in
this particular instance. This is because no further review on the list
is necessary (in this particular case), the reverts and reapplications
are fully mechanical, you just need to make sure to pick up Mike's R-b
for the reapplied MdePkg patch (and to keep the tree buildable at every
stage). See again my other email with the detailed steps.

> Sorry again for this violation.

To emphasize -- I wasn't worried that the patch would cause breakage. My
point is that all contributors should be held to the same standard.

Thanks
Laszlo


>> -Original Message-
>> From: Kinney, Michael D
>> Sent: Thursday, July 11, 2019 3:39 AM
>> To: Laszlo Ersek ; devel@edk2.groups.io; Dong, Eric 
>> ; Ni, Ray ;
>> Kinney, Michael D 
>> Cc: Leif Lindholm ; Gao, Liming 
>> 
>> Subject: RE: [edk2-devel] [PATCH v2 2/3] MdePkg/BaseLib.h: Update IA32_CR4 
>> structure for 5-level paging
>>
>> Laszlo,
>>
>> I agree with your feedback.  Process must be followed.
>>
>> I also agree that it may make sense to add some more maintainers
>> to the MdePkg, especially for some of the content in MdePkg that
>> is closely related to the UefiCpuPkg content.
>>
>> I have reviewed this patch to the BaseLib.h.  The new LA57 bit
>> added to IA32_CR4 matches the documentation in the white paper
>> referenced in the series.
>>
>> Reviewed-by: Michael D Kinney 
>>
>> Thanks,
>>
>> Mike
>>
>>
>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Wednesday, July 10, 2019 10:17 AM
>>> To: devel@edk2.groups.io; Dong, Eric
>>> ; Ni, Ray 
>>> Cc: Leif Lindholm ; Gao, Liming
>>> ; Kinney, Michael D
>>> 
>>> Subject: Re: [edk2-devel] [PATCH v2 2/3]
>>> MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level
>>> paging
>>>
>>> Ray, Eric,
>>>
>>> (+Liming, +Mike, +Leif)
>>>
>>> On 07/09/19 03:04, Dong, Eric wrote:
 Reviewed-by: Eric Dong 

> -Original Message-
> From: Ni, Ray
> Sent: Wednesday, July 3, 2019 2:54 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Laszlo Ersek
> 
> Subject: [PATCH v2 2/3] MdePkg/BaseLib.h: Update
>>> IA32_CR4 structure
> for 5-level paging
>
> 5-level paging is documented in white paper:
>
>>> https://software.intel.com/sites/default/files/managed/2b
>>> /80/5-
> level_paging_white_paper.pdf
>
> Commit f8113e25001e715390127f23e2197252cbd6d1a2
> changed Cpuid.h already.
>
> This patch updates IA32_CR4 structure to include LA57
>>> field.
>
> Signed-off-by: Ray Ni 
> Cc: Eric Dong 
> Regression-tested-by: Laszlo Ersek 
> ---
>  MdePkg/Include/Library/BaseLib.h | 3 ++-
>  1 

Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supports

[Laszlo Ersek]

On 07/11/19 03:19, Ni, Ray wrote:
> Mike,
> Thanks for raising this build failure.
> I just tried in my Ubuntu 18 in Win10. Even GCC7 complains about this. My bad!
> I just posted a fix.

Thanks -- as I requested there, please do not push this new patch until
the revert+reapply completes.

Laszlo

>> -Original Message-
>> From: Kinney, Michael D
>> Sent: Thursday, July 11, 2019 4:06 AM
>> To: devel@edk2.groups.io; Ni, Ray ; Kinney, Michael D 
>> 
>> Cc: Dong, Eric ; Laszlo Ersek 
>> Subject: RE: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level 
>> paging when CPU supports
>>
>> Hi Ray,
>>
>> I noticed a Linux/GCC build issue with this patch when using GCC version:
>>
>> gcc version 8.2.1 20181215 (Red Hat 8.2.1-6) (GCC)
>>
>> edk2/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c: In function 'ReclaimPages':
>> edk2/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c:574:89: error: ?: using integer 
>> constants in boolean context, the
>> expression will always evaluate to 'true' [-Werror=int-in-bool-context]
>>for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / 
>> sizeof (*Pml4)) : 1; Pml5Index++) {
>>
>> I was able to get the build to pass if I added ().
>>
>> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c 
>> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
>> index c31160735a..a3b62f7787 100644
>> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
>> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
>> @@ -571,7 +571,7 @@ ReclaimPages (
>>//
>>// First, find the leaf entry has the smallest access record value
>>//
>> -  for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / 
>> sizeof (*Pml4)) : 1; Pml5Index++) {
>> +  for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / 
>> sizeof (*Pml4)) : 1); Pml5Index++) {^M
>>  if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & 
>> IA32_PG_PMNT) != 0) {
>>//
>>// If the PML5 entry is not present or is masked, skip it
>>
>> Best regards,
>>
>> Mike
>>
>>> -Original Message-
>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
>>> On Behalf Of Ni, Ray
>>> Sent: Tuesday, July 2, 2019 11:54 PM
>>> To: devel@edk2.groups.io
>>> Cc: Dong, Eric ; Laszlo Ersek
>>> 
>>> Subject: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu:
>>> Enable 5 level paging when CPU supports
>>>
>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1946
>>>
>>> The patch changes SMM environment to use 5 level paging
>>> when CPU
>>> supports it.
>>>
>>> Signed-off-by: Ray Ni 
>>> Cc: Eric Dong 
>>> Regression-tested-by: Laszlo Ersek 
>>> ---
>>>  .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   |  20 +-
>>>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c| 272
>>> ++
>>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c   | 485
>>> --
>>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm   |  12 +
>>>  .../PiSmmCpuDxeSmm/X64/SmmProfileArch.c   |  72 ++-
>>>  5 files changed, 561 insertions(+), 300 deletions(-)
>>>
>>> diff --git
>>> a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
>>> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
>>> index 069be3aaa5..55090e9c3e 100644
>>> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
>>> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
>>> @@ -125,18 +125,36 @@ GetPageTableEntry (
>>>UINTN Index2;
>>>UINTN Index3;
>>>UINTN Index4;
>>> +  UINTN Index5;
>>>UINT64*L1PageTable;
>>>UINT64*L2PageTable;
>>>UINT64*L3PageTable;
>>>UINT64*L4PageTable;
>>> +  UINT64*L5PageTable;
>>> +  IA32_CR4  Cr4;
>>> +  BOOLEAN   Enable5LevelPaging;
>>>
>>> +  Index5 = ((UINTN)RShiftU64 (Address, 48)) &
>>> PAGING_PAE_INDEX_MASK;
>>>Index4 = ((UINTN)RShiftU64 (Address, 39)) &
>>> PAGING_PAE_INDEX_MASK;
>>>Index3 = ((UINTN)Address >> 30) &
>>> PAGING_PAE_INDEX_MASK;
>>>Index2 = ((UINTN)Address >> 21) &
>>> PAGING_PAE_INDEX_MASK;
>>>Index1 = ((UINTN)Address >> 12) &
>>> PAGING_PAE_INDEX_MASK;
>>>
>>> +  Cr4.UintN = AsmReadCr4 ();
>>> +  Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
>>> +
>>>if (sizeof(UINTN) == sizeof(UINT64)) {
>>> -L4PageTable = (UINT64 *)GetPageTableBase ();
>>> +if (Enable5LevelPaging) {
>>> +  L5PageTable = (UINT64 *)GetPageTableBase ();
>>> +  if (L5PageTable[Index5] == 0) {
>>> +*PageAttribute = PageNone;
>>> +return NULL;
>>> +  }
>>> +
>>> +  L4PageTable = (UINT64
>>> *)(UINTN)(L5PageTable[Index5] & ~mAddressEncMask &
>>> PAGING_4K_ADDRESS_MASK_64);
>>> +} else {
>>> +  L4PageTable = (UINT64 *)GetPageTableBase ();
>>> +}
>>>  if (L4PageTable[Index4] == 0) {
>>>*PageAttribute = PageNone;
>>>return NULL;
>>> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
>>> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
>>> index 

Re: [edk2-devel] [PATCH v2 2/3] MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level paging

[Laszlo Ersek]

On 07/10/19 21:38, Kinney, Michael D wrote:
> Laszlo,
> 
> I agree with your feedback.  Process must be followed.
> 
> I also agree that it may make sense to add some more maintainers
> to the MdePkg, especially for some of the content in MdePkg that
> is closely related to the UefiCpuPkg content.

Should we ask Ray & Eric to submit a suitable patch for Maintainers.txt
(MdePkg) right now, or should we wait until Leif's & Hao's work on the
"wildcard path association" feature (for Maintainers.txt) completes?

> I have reviewed this patch to the BaseLib.h.  The new LA57 bit
> added to IA32_CR4 matches the documentation in the white paper
> referenced in the series.
> 
> Reviewed-by: Michael D Kinney 

Thanks!
Laszlo

> 
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Wednesday, July 10, 2019 10:17 AM
>> To: devel@edk2.groups.io; Dong, Eric
>> ; Ni, Ray 
>> Cc: Leif Lindholm ; Gao, Liming
>> ; Kinney, Michael D
>> 
>> Subject: Re: [edk2-devel] [PATCH v2 2/3]
>> MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level
>> paging
>>
>> Ray, Eric,
>>
>> (+Liming, +Mike, +Leif)
>>
>> On 07/09/19 03:04, Dong, Eric wrote:
>>> Reviewed-by: Eric Dong 
>>>
 -Original Message-
 From: Ni, Ray
 Sent: Wednesday, July 3, 2019 2:54 PM
 To: devel@edk2.groups.io
 Cc: Dong, Eric ; Laszlo Ersek
 
 Subject: [PATCH v2 2/3] MdePkg/BaseLib.h: Update
>> IA32_CR4 structure
 for 5-level paging

 5-level paging is documented in white paper:

>> https://software.intel.com/sites/default/files/managed/2b
>> /80/5-
 level_paging_white_paper.pdf

 Commit f8113e25001e715390127f23e2197252cbd6d1a2
 changed Cpuid.h already.

 This patch updates IA32_CR4 structure to include LA57
>> field.

 Signed-off-by: Ray Ni 
 Cc: Eric Dong 
 Regression-tested-by: Laszlo Ersek 
 ---
  MdePkg/Include/Library/BaseLib.h | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/MdePkg/Include/Library/BaseLib.h
 b/MdePkg/Include/Library/BaseLib.h
 index ebd7dd274c..a22bfc9fad 100644
 --- a/MdePkg/Include/Library/BaseLib.h
 +++ b/MdePkg/Include/Library/BaseLib.h
 @@ -5324,7 +5324,8 @@ typedef union {
  UINT32  OSXMMEXCPT:1;   ///< Operating System
>> Support for
  ///< Unmasked SIMD
>> Floating Point
  ///< Exceptions.
 -UINT32  Reserved_0:2;   ///< Reserved.
 +UINT32  Reserved_2:1;   ///< Reserved.
 +UINT32  LA57:1; ///< Linear Address
>> 57bit.
  UINT32  VMXE:1; ///< VMX Enable
  UINT32  Reserved_1:18;  ///< Reserved.
} Bits;
>>
>> I'm sorry but you will have to revert this patch series
>> immediately.
>> None of the MdePkg maintainers have approved this patch -
>> - commit 7c5010c7f88b.
>>
>> In the first place, Mike and Liming were never CC'd on
>> the patch, so they may not have noticed it, even.
>>
>> The situation is very similar to the recent SM3 crypto
>> series that I had to revert myself. An MdePkg patch was
>> pushed without package owner review.
>>
>> Can you guys please revert this series immediately,
>> without me having to do it?
>>
>>
>> If we think that MdePkg should have more "M" folks, in
>> order to distribute the review load better, then we
>> should address that problem first. Ignoring rules just
>> because that's more convenient is not acceptable.
>>
>> Thanks,
>> Laszlo


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

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

Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpu: Fix GCC7/GCC8 build failure

[Laszlo Ersek]

On 07/11/19 12:02, Laszlo Ersek wrote:
> Hello Ray,
> 
> On 07/11/19 03:16, Ray Ni wrote:
>> Signed-off-by: Ray Ni 
>> Cc: Michael D Kinney 
>> Cc: Eric Dong 
>> Cc: Laszlo Ersek 
>> ---
>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c 
>> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
>> index c31160735a..a3b62f7787 100644
>> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
>> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
>> @@ -571,7 +571,7 @@ ReclaimPages (
>>//
>>// First, find the leaf entry has the smallest access record value
>>//
>> -  for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / 
>> sizeof (*Pml4)) : 1; Pml5Index++) {
>> +  for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / 
>> sizeof (*Pml4)) : 1); Pml5Index++) {
>>  if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & 
>> IA32_PG_PMNT) != 0) {
>>//
>>// If the PML5 entry is not present or is masked, skip it
>>
> 
> This is not a GCC7/GCC8 build failure, but a genuine bug in the code
> that GCC7/GCC8 helpfully reported.
> 
> The conditional operator ?: has weaker precedence than the relational
> operator <, and so the patch incurs a behavioral change -- thus, it is a
> bugfix.
> 
> If we were only adding the parentheses to reinforce the operator
> bingings that are already in place, i.e., preserving the behavior,
> *then* we could call this a "build failure".
> 
> 
> (1) Please update the subject accordingly, for example:
> 
> UefiCpuPkg/PiSmmCpuDxeSmm: ReclaimPages: fix incorrect operator binding
> 
> (71 characters)
> 
> 
> (2) I'd suggest also adding, to the commit message body:
> 
> Fixes: 7365eb2c8cf1d7112330d09918c0c67e8d0b827a
> 
> With those:
> 
> Reviewed-by: Laszlo Ersek 

IMPORTANT: please do not push this patch before you revert and reapply
7365eb2c8cf1; otherwise we'll get into a huge mess. Please see here:

https://edk2.groups.io/g/devel/message/43599

(Namely, if you apply this patch on top of 7365eb2c8cf1, then you won't
be able to revert 7365eb2c8cf1, which is a pre-requisite for reverting
7c5010c7f88b.)

Thanks
Laszlo

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

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

Re: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object to handle build conf file

[Bob Feng]

Liming,

I tested the cases:
1. move tool_def.txt and build_rule.txt from Conf to another folder and update 
target.txt to set BUILD_RULE_CONF and TOOL_CHAIN_CONF to point to the new 
folder.
2. Move Conf folder to a new place and Set WORKSPACE to point the parent folder 
of Conf. Append original WORKSPACE folder to PACKAGES_PATH

My patch works fine.

But there is a bug in original code for case 1#, I'll send out another patch to 
fix that.

Thanks,
Bob

-Original Message-
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Bob Feng
Sent: Thursday, July 11, 2019 5:27 PM
To: Gao, Liming ; devel@edk2.groups.io
Subject: Re: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object to handle 
build conf file

Liming,

Thanks for your comments. I'll  test the cases you mentioned.

Thanks,
Bob

-Original Message-
From: Gao, Liming
Sent: Thursday, July 11, 2019 5:11 PM
To: Feng, Bob C ; devel@edk2.groups.io
Subject: RE: [Patch 1/1] BaseTools: Singleton the object to handle build conf 
file

Bob:
  target.txt is from Conf directory. build_rule.txt and tools_def.txt are 
specified in target.txt. Please take care this case that they may have the 
different file name. 

  And, Conf directory is relative to WORKSPACE and PACKAGES_PATH. Does this 
patch support it?

Thanks
Liming
>-Original Message-
>From: Feng, Bob C
>Sent: Friday, June 28, 2019 3:07 PM
>To: devel@edk2.groups.io
>Cc: Gao, Liming ; Feng, Bob C 
>
>Subject: [Patch 1/1] BaseTools: Singleton the object to handle build 
>conf file
>
>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875
>
>The build config files are target.txt, build rule, tooldef During a 
>build, the config is not changed, so the object to handle them need to 
>be singleton.
>
>Cc: Liming Gao 
>Signed-off-by: Bob Feng 
>---
> BaseTools/Source/Python/AutoGen/AutoGen.py| 33 ++--
> .../Source/Python/AutoGen/BuildEngine.py  | 22 
> .../Python/Common/TargetTxtClassObject.py |  2 +
> .../Python/Common/ToolDefClassObject.py   |  6 ++-
> BaseTools/Source/Python/GenFds/GenFds.py  |  4 +-
> .../Python/GenFds/GenFdsGlobalVariable.py | 54 ---
> .../Source/Python/Workspace/DscBuildData.py   |  8 +--
> BaseTools/Source/Python/build/build.py| 29 +++---
> 8 files changed, 62 insertions(+), 96 deletions(-)
>
>diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>b/BaseTools/Source/Python/AutoGen/AutoGen.py
>index e8e09dc8a366..a1f7f5641e09 100644
>--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>@@ -22,11 +22,12 @@ from . import GenC
> from . import GenMake
> from . import GenDepex
> from io import BytesIO
>
> from .StrGather import *
>-from .BuildEngine import BuildRule
>+from .BuildEngine import BuildRuleObj as BuildRule from .BuildEngine 
>+import
>gDefaultBuildRuleFile,AutoGenReqBuildRuleVerNum
> import shutil
> from Common.LongFilePathSupport import CopyLongFilePath  from 
>Common.BuildToolError import *  from Common.DataType import *  from 
>Common.Misc import * @@ -76,16 +77,10 @@ gEfiVarStoreGuidPattern =
>re.compile("\s*guid\s*=\s*({.*?{.*?}\s*})")
>
> ## Mapping Makefile type
> gMakeTypeMap = {TAB_COMPILER_MSFT:"nmake", "GCC":"gmake"}
>
>
>-## Build rule configuration file
>-gDefaultBuildRuleFile = 'build_rule.txt'
>-
>-## Build rule default version
>-AutoGenReqBuildRuleVerNum = "0.1"
>-
> ## default file name for AutoGen
> gAutoGenCodeFileName = "AutoGen.c"
> gAutoGenHeaderFileName = "AutoGen.h"
> gAutoGenStringFileName = "%(module_name)sStrDefs.h"
> gAutoGenStringFormFileName = "%(module_name)sStrDefs.hpk"
>@@ -1964,32 +1959,10 @@ class PlatformAutoGen(AutoGen):
> ## Return the build options specific for EDKII modules in this platform
> @cached_property
> def EdkIIBuildOption(self):
> return self._ExpandBuildOption(self.Platform.BuildOptions,
>EDKII_NAME)
>
>-## Parse build_rule.txt in Conf Directory.
>-#
>-#   @retval BuildRule object
>-#
>-@cached_property
>-def BuildRule(self):
>-BuildRuleFile = None
>-if TAB_TAT_DEFINES_BUILD_RULE_CONF in
>self.Workspace.TargetTxt.TargetTxtDictionary:
>-BuildRuleFile =
>self.Workspace.TargetTxt.TargetTxtDictionary[TAB_TAT_DEFINES_BUILD_RUL
>E_CONF]
>-if not BuildRuleFile:
>-BuildRuleFile = gDefaultBuildRuleFile
>-RetVal = BuildRule(BuildRuleFile)
>-if RetVal._FileVersion == "":
>-RetVal._FileVersion = AutoGenReqBuildRuleVerNum
>-else:
>-if RetVal._FileVersion < AutoGenReqBuildRuleVerNum :
>-# If Build Rule's version is less than the version number 
>required by
>the tools, halting the build.
>-EdkLogger.error("build", AUTOGEN_ERROR,
>-ExtraData="The version number [%s] of 
>build_rule.txt is less
>than the version number required by the AutoGen.(the minimum required 
>version number is 

Re: [edk2-devel] [PATCH v2 2/3] MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level paging

[Laszlo Ersek]

On 07/11/19 06:05, Ni, Ray wrote:
> Laszlo, Mike,
> 
> An update to my revert proposal, I will only revert the below 2 patches:
> 
> 7c5010c7f88b790f4524c4a5311819e3af5e2752
> * MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level paging
> 
> 7365eb2c8cf1d7112330d09918c0c67e8d0b827a
> * UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supports
> 
> 
> Will keep the below patch in github.
> 7e56f8928d8461d820a81a50908adf648279f1dc
> * UefiCpuPkg/PiSmmCpu: Change variable names and comments to follow SDM
> 
> Is it good to you?
> Or the revert process needs to revert whole patch series?
> 
> Can you or someone else guide me where the revert process is documented?

(1) In general, reverts are always applied in the reverse order of the
original series.

(2) It's fine to revert a subset of the patches if, and only if:

- at any stage during the revert series, the tree builds and works

- at the end of the revert series, all previous commits that have *not*
been reverted contain the required maintainer approval tags.

(3) So, in the present case, you should first revert 7365eb2c8cf1
("UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supports",
2019-07-10). This is because the code introduced in said commit would
not build without the MdePkg/BaseLib.h changes.

Then, second, you should revert 7c5010c7f88b ("MdePkg/BaseLib.h: Update
IA32_CR4 structure for 5-level paging", 2019-07-10).

Please use the "git revert" command. When reverting 7365eb2c8cf1, state
that the reason for the revert is maintaining proper dependencies
(buildability). When reverting 7c5010c7f88b, please state in the commit
message that the reason for the revert is incorrect patch review workflow.

Finally (now with Mike's R-b available on the list), you can reapply the
MdePkg and UefiCpuPkg patches (in that order); just make sure that you
add Mike's R-b to the reapplied MdePkg patch.

(4) Summary:

$ git revert 7365eb2c8cf1
--> reason to state in the commit message: maintain buildability
--> add your Signed-off-by

$ git revert 7c5010c7f88b
--> reason to state in the commit message: lacking patch review process

$ git cherry-pick -x -e 7c5010c7f88b
--> append Mike's R-b to the commit message

$ git cherry-pick -x 7365eb2c8cf1
--> commit message will not be edited

$ git push

Thanks,
Laszlo



>> -Original Message-
>> From: Ni, Ray
>> Sent: Thursday, July 11, 2019 11:25 AM
>> To: Kinney, Michael D ; Laszlo Ersek
>> ; devel@edk2.groups.io; Dong, Eric
>> 
>> Cc: Leif Lindholm ; Gao, Liming
>> 
>> Subject: RE: [edk2-devel] [PATCH v2 2/3] MdePkg/BaseLib.h: Update
>> IA32_CR4 structure for 5-level paging
>>
>> Laszlo, Mike,
>> Sorry I did violate the process.
>> I had two assumptions which led me violate the process:
>> 1. Reviewed-by from UefiCpuPkg maintainers on this IA32_CR4 change is
>> more important
>> than that from MdePkg maintainers. In another word, I thought if
>> UefiCpuPkg maintainers
>> agree with this change, MdePkg maintainers should have no concerns.
>> (It's a wrong assumption. MdePkg maintainers may have some general
>> suggestions, e.g.:
>> name, location, comments and etc..)
>> 2. This change is directly from the published white paper and there is no
>> other option regarding
>>  this IA32_CR4 change.
>> (It's a wrong assumption. MdePkg maintainers may have some general
>> suggestions, e.g.:
>> name, location, comments and etc..)
>>
>> I agree I should get Reviewed-by tag from MdePkg maintainers. My
>> assumptions are wrong.
>>
>> To strictly follow the process, I will:
>> 1. Post a patch series to revert the 3 patches.
>>  Since this change doesn't break any functionality (does break the 
>> process),
>> I will wait for
>>  Reviewed-by from each package maintainer and make sure push the
>> patches after 24h.
>> 2. After step #1, post a new patch series to add the 3 patches back with
>> Reviewed-by tag from
>> Eric and Mike, Regression-Tested-by from you.
>>
>> Do you think it follows the existing process?
>>
>> Sorry again for this violation.
>>
>> Thanks,
>> Ray
>>
>>
>>> -Original Message-
>>> From: Kinney, Michael D
>>> Sent: Thursday, July 11, 2019 3:39 AM
>>> To: Laszlo Ersek ; devel@edk2.groups.io; Dong, Eric
>>> ; Ni, Ray ; Kinney, Michael D
>>> 
>>> Cc: Leif Lindholm ; Gao, Liming
>>> 
>>> Subject: RE: [edk2-devel] [PATCH v2 2/3] MdePkg/BaseLib.h: Update
>>> IA32_CR4 structure for 5-level paging
>>>
>>> Laszlo,
>>>
>>> I agree with your feedback.  Process must be followed.
>>>
>>> I also agree that it may make sense to add some more maintainers to
>>> the MdePkg, especially for some of the content in MdePkg that is
>>> closely related to the UefiCpuPkg content.
>>>
>>> I have reviewed this patch to the BaseLib.h.  The new LA57 bit added
>>> to IA32_CR4 matches the documentation in the white paper referenced in
>>> the series.
>>>
>>> Reviewed-by: Michael D Kinney 
>>>
>>> Thanks,
>>>
>>> Mike
>>>
>>>
 -Original Message-
 From: Laszlo Ersek 

Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[Laszlo Ersek]

On 07/11/19 05:20, Zhang, Chao B wrote:
> HI Laszlo:
>There is a discussion over this issue in UEFI Manits 
> https://mantis.uefi.org/mantis/view.php?id=1983
> The justification lies here:
>   Spec perspective:
> Section 8.2.2  : In SetupMode Secure Boot Policy variables shall consider 
> step 3 and 4 check to be successful.
> Section 32.3.1 : If in the platform is in stepup mode, then the new PKpub 
> may be signed with PKpriv
> Customer needs:
>  1) PK update process is complex based on current implementation – self 
> signed PK is required. 2 PK images are required
> - new PK signed with the old PKprivate, to be used if system is in user mode
> - new self-sighed PK (new PK signed with new PKprivate) to be used if system 
> is in setup mode
>2) PKpub Default can be easily updated to PK
> 
> Back to Laszlo’s question
>   (1) What is the exact failure (or spec non-conformance) scenario?
>  A:Please see above explanation
>  (2a) whether skipping step#4 in SetupMode is a bug in the spec -- 
> because, I think it is;
>  A:   Please see customer needs part in above explanation
>  (2b) whether the edk2 code would continue enforcing self-signedness on
>   X509 certificates, if the proposed patch were applied.
>   A:  After this patch, there is no self-encrypted PK check. Per 
> discussion, we believe that is not a new security hole
>  Even with self-proofed PK check, attacker can easily spoof a faked 
> PK attack in setup mode.   Generally speaking,
>  PK provision should happen in Build phase or with Physical Presence 
> Asserted.

Thank you for the explanation.

The discussion in Mantis#1983 makes the situation clear.

In Mantis#1983, it was requested that self-signedness be strictly
enforced when a PK is enrolled in Setup Mode. That matches my opinion fully.

Mantis#1983 was ultimately revoked because "it would complicate the PK
update process". Namely, platform vendors would have to provide
different blobs, for enrollment in setup mode (self-signed) vs.
enrollment in user mode (signed with the currently in-place PK's private
half).

Well, I think that said process would be exactly right. The
counter-argument:

  it would complicate the PK update process

is poor, in my opinion, when it comes to the root of trust in Secure
Boot. There is no failure scenario in fact, it's just that vendors would
have to provide multiple update images, and users (or the update tools)
would have to download the image that would be accepted by the current
state. In my opinion, that should be fine.

Multiple PK update images cannot be avoided anyway, if a platform vendor
releases at least two PK updates, over time. (So that we have the
initial PK0, and two updates, PK1, PK2.) When PK2 is released, it must
be made available to systems running in User Mode
- both as signed with PK0,
- and as signed with PK1,
anyway.

So I don't see why it's a burden to release PK2 as signed with PK2 --
i.e., with itself -- as well.

Mantis#1983 also highlights that it should always be possible to revert
PK to PKDefault. IMO that should not be a problem if PKDefault is
self-signed, and the physically present user switches the SB mode to
Setup Mode first. In that case, PKDefault can be re-enrolled -- in the
firmware setup utility, at boot time only -- like any other brand new
self-signed PK.

The point of requiring a self-signed certificate, and not just a public
key, is *not* to increase security. The point is to perform a sanity
check. If the certificate is correctly self-signed, it proves that,
whoever generated the certificate, had access to the counterpart private
key at least at that time. Conversely, if it's just a standalone pubkey,
or the signature on the cert is from some other entity, then the
end-user has no evidence that a matching private key was *ever* saved by
the creator. Again, the point is not to prevent spoofing, but to
sanity-check that the creator of the pubkey was capable of signing *at
all* with the matching private key.

And so I disagree with the rejection / revocation of Mantis#1983.

Regarding the edk2 patch:

If the patch indeed matches the UEFI-2.8 spec, then I don't have a valid
case against the proposed patch.

(Again, I think it's the spec that is problematic, and that Mantis#1983
had merit.)

However, can we please introduce a FeaturePCD for sticking with the
present in-tree behavior?

I'm OK if the default value for the new FeaturePCD is such that the new
(proposed, spec-conformant) behavior becomes the default. I'd just like
if platforms could opt out (i.e. if they could enforce self-signedness
in SetupMode).

Thanks
Laszlo

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

View/Reply Online (#43598): https://edk2.groups.io/g/devel/message/43598
Mute This Topic: https://groups.io/mt/32283314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  

Re: [edk2-devel] [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol

[Leif Lindholm]

On Thu, Jul 11, 2019 at 12:24:46PM +0200, Laszlo Ersek wrote:
> Every single source file must have both copyright notice(s) and
> licensing information.
> 
> - Licensing information without copyright holder(s) makes zero sense --
> the licensing terms are offered / dictated *by* the copyright holders.
> 
> - A copyright notice in itself, without a suitable license (including
> the "no license" case) does not provide us with the necessary rights to
> carry the source file in the edk2 project.

Agree with all of the above.

> Regarding "EnableInterrupts.nasm". This looks like a tricky situation.
> Under the above link, the file is currently missing a Microsoft (C)
> notice (the last commit to modify the file is apparently from Microsoft,
> b621971). So that should be addressed in Project Mu first, in my opinion.
> 
> Then, Microsoft should please relicense the file under BSD+Patent, from
> pure 2-BSD. If they disagree, then the 2-BSD is still acceptable for edk2.
> 
> Then, the file could be imported into edk2, carrying
> - the MS (C) notice,
> - the Intel (C) notice (extended to 2019),
> - the license -- BSD+Patent, or else 2-BSD --, expressed with an SPDX ID
> (not open-coded license text).

While I agree it would be better if Microsoft relicensed it as
BSD+Patent, BSD+Patent is a strict superset of 2-clause BSD - so there
is no point in TianoCore to carry 2-Clause BSD code.

We can just add the explicit patent grant on contribution (i.e. change
the license header for a BSD+Patent SPDX tag). And we should, because
otherwise without the tianocore contribution agreement, we open
ourselves up to submarine contributions.

The downside to doing that would be if we wanted the code to be able
to flow in both directions, which is why it would be good if
Microsoft relicensed (and also because then they would be making an
explicit statement that they weren't trying to submarine TianoCore).

Best Regards,

Leif

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

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

Re: [edk2-devel] [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol

[Laszlo Ersek]

On 07/11/19 04:36, Gao, Zhichao wrote:
> 
> 
>> -Original Message-
>> From: Dong, Eric
>> Sent: Thursday, July 11, 2019 10:22 AM
>> To: Gao, Zhichao ; devel@edk2.groups.io
>> Cc: Sean Brogan ; Ni, Ray ;
>> Laszlo Ersek ; Gao, Liming ;
>> Michael Turner ; Bret Barkelew
>> 
>> Subject: RE: [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
>>
>> Hi Zhizhao,
>>
>> The new add files don't have copyright info, is it ok?

Good catch!

> I forgot add file comment for Ia32/EnableInterruptsAndSleep.c. Also lose the 
> BSD+Patent License. I would add them in the future.
> For the copyright:
> The code is contributed by Microsoft at 
> https://github.com/Microsoft/mu_basecore/blob/release/201808/MdePkg/Library/BaseLib/X64/EnableInterrupts.nasm#L48
> I move the code from MdePkg to UefiCpuPkg to a single file. It is 
> inappropriate to add both intel (not contributor) and Microsoft (they didn't 
> add in the above link).
> So I keep the copyright blank. I don't know whether it is ok without 
> copyright.

Every single source file must have both copyright notice(s) and
licensing information.

- Licensing information without copyright holder(s) makes zero sense --
the licensing terms are offered / dictated *by* the copyright holders.

- A copyright notice in itself, without a suitable license (including
the "no license" case) does not provide us with the necessary rights to
carry the source file in the edk2 project.

Regarding "EnableInterrupts.nasm". This looks like a tricky situation.
Under the above link, the file is currently missing a Microsoft (C)
notice (the last commit to modify the file is apparently from Microsoft,
b621971). So that should be addressed in Project Mu first, in my opinion.

Then, Microsoft should please relicense the file under BSD+Patent, from
pure 2-BSD. If they disagree, then the 2-BSD is still acceptable for edk2.

Then, the file could be imported into edk2, carrying
- the MS (C) notice,
- the Intel (C) notice (extended to 2019),
- the license -- BSD+Patent, or else 2-BSD --, expressed with an SPDX ID
(not open-coded license text).

Note: I'm not a lawyer; the above is just my opinion. CC'ing the other
stewards.

Thanks,
Laszlo

> 
> Hi Liming,
> 
> What's your opinion?
> 
> Thanks,
> Zhichao
> 
>>
>> Thanks,
>> Eric
>>
>>> -Original Message-
>>> From: Gao, Zhichao
>>> Sent: Tuesday, July 9, 2019 4:40 PM
>>> To: devel@edk2.groups.io
>>> Cc: Sean Brogan ; Dong, Eric
>>> ; Ni, Ray ; Laszlo Ersek
>>> ; Gao, Liming ; Michael
>>> Turner ; Bret Barkelew
>>> 
>>> Subject: [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
>>>
>>> From: Sean Brogan 
>>>
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1400
>>>
>>> Implement Cp2 protocol: it has one interface to enable the interrupt
>>> and put cpu to sleep and wait for an interrupt.
>>>
>>> Cc: Eric Dong 
>>> Cc: Ray Ni 
>>> Cc: Laszlo Ersek 
>>> Cc: Liming Gao 
>>> Cc: Sean Brogan 
>>> Cc: Michael Turner 
>>> Cc: Bret Barkelew 
>>> Signed-off-by: Zhichao Gao 
>>> ---
>>>  UefiCpuPkg/CpuDxe/CpuDxe.c| 38 +++
>>>  UefiCpuPkg/CpuDxe/CpuDxe.h| 25 
>>>  UefiCpuPkg/CpuDxe/CpuDxe.inf  |  3 ++
>>>  .../CpuDxe/Ia32/EnableInterruptsAndSleep.c| 24 
>>>  .../CpuDxe/X64/EnableInterruptsAndSleep.nasm  | 31 +++
>>>  5 files changed, 121 insertions(+)
>>>  create mode 100644
>> UefiCpuPkg/CpuDxe/Ia32/EnableInterruptsAndSleep.c
>>>  create mode 100644
>>> UefiCpuPkg/CpuDxe/X64/EnableInterruptsAndSleep.nasm
>>>


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

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

Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpu: Fix GCC7/GCC8 build failure

[Laszlo Ersek]

Hello Ray,

On 07/11/19 03:16, Ray Ni wrote:
> Signed-off-by: Ray Ni 
> Cc: Michael D Kinney 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index c31160735a..a3b62f7787 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -571,7 +571,7 @@ ReclaimPages (
>//
>// First, find the leaf entry has the smallest access record value
>//
> -  for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / 
> sizeof (*Pml4)) : 1; Pml5Index++) {
> +  for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / 
> sizeof (*Pml4)) : 1); Pml5Index++) {
>  if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & 
> IA32_PG_PMNT) != 0) {
>//
>// If the PML5 entry is not present or is masked, skip it
> 

This is not a GCC7/GCC8 build failure, but a genuine bug in the code
that GCC7/GCC8 helpfully reported.

The conditional operator ?: has weaker precedence than the relational
operator <, and so the patch incurs a behavioral change -- thus, it is a
bugfix.

If we were only adding the parentheses to reinforce the operator
bingings that are already in place, i.e., preserving the behavior,
*then* we could call this a "build failure".


(1) Please update the subject accordingly, for example:

UefiCpuPkg/PiSmmCpuDxeSmm: ReclaimPages: fix incorrect operator binding

(71 characters)


(2) I'd suggest also adding, to the commit message body:

Fixes: 7365eb2c8cf1d7112330d09918c0c67e8d0b827a

With those:

Reviewed-by: Laszlo Ersek 

Thanks
Laszlo

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

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

[edk2-devel] [PATCH] Revert "FmpDevicePkg: Fix various typos"

[Cœur]

This reverts commit f527942e6bdd9f198db90f2de99a0482e9be5b1b.
Commit message was incorrect.

Signed-off-by: Coeur 
---
 .../FspSecCore/Ia32/FspApiEntryM.nasm |  4 +--
 .../FspSecCore/Ia32/InitializeFpu.nasm|  4 +--
 .../FspSecCore/Ia32/SaveRestoreSseNasm.inc|  4 +--
 IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm   |  4 +--
 IntelFsp2Pkg/FspSecCore/SecFsp.c  |  4 +--
 IntelFsp2Pkg/FspSecCore/SecMain.c |  2 +-
 .../FspSecCore/Vtf0/Ia16/ResetVec.asm16   |  4 +--
 IntelFsp2Pkg/Include/FspEas/FspApi.h  |  8 ++---
 .../Include/Library/FspSecPlatformLib.h   |  4 +--
 IntelFsp2Pkg/Library/BaseCacheLib/CacheLib.c  | 12 +++
 .../BaseFspDebugLibSerialPort/DebugLib.c  | 34 +--
 .../BaseFspSwitchStackLib/Ia32/Stack.nasm |  4 +--
 .../SecFspSecPlatformLibNull/Ia32/Flat32.nasm |  4 +--
 .../PlatformSecLibNull.c  |  4 +--
 IntelFsp2Pkg/Tools/GenCfgOpt.py   |  2 +-
 IntelFsp2Pkg/Tools/PatchFv.py |  2 +-
 .../Tools/UserManuals/GenCfgOptUserManual.md  |  2 +-
 .../Tools/UserManuals/PatchFvUserManual.md|  2 +-
 18 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
index e7261b41cd..f14c18c7b9 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
@@ -194,9 +194,9 @@ StackSetupDone:

   ;
   ; Pass BFV into the PEI Core
-  ; It uses relative address to calculate the actual boot FV base
+  ; It uses relative address to calucate the actual boot FV base
   ; For FSP implementation with single FV, PcdFspBootFirmwareVolumeBase and
-  ; PcdFspAreaBaseAddress are the same. For FSP with multiple FVs,
+  ; PcdFspAreaBaseAddress are the same. For FSP with mulitple FVs,
   ; they are different. The code below can handle both cases.
   ;
   callASM_PFX(AsmGetFspBaseAddress)
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
index ebc91c41e4..e1886ea11b 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
@@ -1,6 +1,6 @@
 ;--
 ;
-; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
+; Copyright (c) 2015, Intel Corporation. All rights reserved.
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ; Abstract:
@@ -46,7 +46,7 @@ ASM_PFX(InitializeFloatingPointUnits):
 fldcw[ASM_PFX(mFpuControlWord)]

 ;
-; Use CpuId instruction (CPUID.01H:EDX.SSE[bit 25] = 1) to test
+; Use CpuId instructuion (CPUID.01H:EDX.SSE[bit 25] = 1) to test
 ; whether the processor supports SSE instruction.
 ;
 mov eax, 1
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc 
b/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
index 4c321cbece..b257deb76c 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
@@ -1,6 +1,6 @@
 ;--
 ;
-; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
+; Copyright (c) 2015, Intel Corporation. All rights reserved.
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ; Abstract:
@@ -150,7 +150,7 @@ NextAddress:
 fldcw   [FpuControlWord]

 ;
-; Use CpuId instruction (CPUID.01H:EDX.SSE[bit 25] = 1) to test
+; Use CpuId instructuion (CPUID.01H:EDX.SSE[bit 25] = 1) to test
 ; whether the processor supports SSE instruction.
 ;
 mov eax, 1
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm
index 5a7e27c240..d72212ed45 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/Stack.nasm
@@ -1,6 +1,6 @@
 ;--
 ;
-; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
+; Copyright (c) 2015, Intel Corporation. All rights reserved.
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ; Abstract:
@@ -58,7 +58,7 @@ ASM_PFX(SecSwitchStack):
 mov   esp, eax ; From now, esp is pointed to permanent 
memory

 ;
-; Fixup the ebp point to permanent memory
+; Fixup the ebp point to permenent memory
 ;
 mov   eax, ebp
 sub   eax, ebx
diff --git a/IntelFsp2Pkg/FspSecCore/SecFsp.c b/IntelFsp2Pkg/FspSecCore/SecFsp.c
index 446d1730e9..6497c88ebe 100644
--- a/IntelFsp2Pkg/FspSecCore/SecFsp.c
+++ b/IntelFsp2Pkg/FspSecCore/SecFsp.c
@@ -1,6 +1,6 @@
 /** @file

-  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.
+  Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent

 **/
@@ 

Re: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object to handle build conf file

[Bob Feng]

Liming,

Thanks for your comments. I'll  test the cases you mentioned.

Thanks,
Bob

-Original Message-
From: Gao, Liming 
Sent: Thursday, July 11, 2019 5:11 PM
To: Feng, Bob C ; devel@edk2.groups.io
Subject: RE: [Patch 1/1] BaseTools: Singleton the object to handle build conf 
file

Bob:
  target.txt is from Conf directory. build_rule.txt and tools_def.txt are 
specified in target.txt. Please take care this case that they may have the 
different file name. 

  And, Conf directory is relative to WORKSPACE and PACKAGES_PATH. Does this 
patch support it?

Thanks
Liming
>-Original Message-
>From: Feng, Bob C
>Sent: Friday, June 28, 2019 3:07 PM
>To: devel@edk2.groups.io
>Cc: Gao, Liming ; Feng, Bob C 
>
>Subject: [Patch 1/1] BaseTools: Singleton the object to handle build 
>conf file
>
>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875
>
>The build config files are target.txt, build rule, tooldef During a 
>build, the config is not changed, so the object to handle them need to 
>be singleton.
>
>Cc: Liming Gao 
>Signed-off-by: Bob Feng 
>---
> BaseTools/Source/Python/AutoGen/AutoGen.py| 33 ++--
> .../Source/Python/AutoGen/BuildEngine.py  | 22 
> .../Python/Common/TargetTxtClassObject.py |  2 +
> .../Python/Common/ToolDefClassObject.py   |  6 ++-
> BaseTools/Source/Python/GenFds/GenFds.py  |  4 +-
> .../Python/GenFds/GenFdsGlobalVariable.py | 54 ---
> .../Source/Python/Workspace/DscBuildData.py   |  8 +--
> BaseTools/Source/Python/build/build.py| 29 +++---
> 8 files changed, 62 insertions(+), 96 deletions(-)
>
>diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>b/BaseTools/Source/Python/AutoGen/AutoGen.py
>index e8e09dc8a366..a1f7f5641e09 100644
>--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>@@ -22,11 +22,12 @@ from . import GenC
> from . import GenMake
> from . import GenDepex
> from io import BytesIO
>
> from .StrGather import *
>-from .BuildEngine import BuildRule
>+from .BuildEngine import BuildRuleObj as BuildRule from .BuildEngine 
>+import
>gDefaultBuildRuleFile,AutoGenReqBuildRuleVerNum
> import shutil
> from Common.LongFilePathSupport import CopyLongFilePath  from 
>Common.BuildToolError import *  from Common.DataType import *  from 
>Common.Misc import * @@ -76,16 +77,10 @@ gEfiVarStoreGuidPattern =
>re.compile("\s*guid\s*=\s*({.*?{.*?}\s*})")
>
> ## Mapping Makefile type
> gMakeTypeMap = {TAB_COMPILER_MSFT:"nmake", "GCC":"gmake"}
>
>
>-## Build rule configuration file
>-gDefaultBuildRuleFile = 'build_rule.txt'
>-
>-## Build rule default version
>-AutoGenReqBuildRuleVerNum = "0.1"
>-
> ## default file name for AutoGen
> gAutoGenCodeFileName = "AutoGen.c"
> gAutoGenHeaderFileName = "AutoGen.h"
> gAutoGenStringFileName = "%(module_name)sStrDefs.h"
> gAutoGenStringFormFileName = "%(module_name)sStrDefs.hpk"
>@@ -1964,32 +1959,10 @@ class PlatformAutoGen(AutoGen):
> ## Return the build options specific for EDKII modules in this platform
> @cached_property
> def EdkIIBuildOption(self):
> return self._ExpandBuildOption(self.Platform.BuildOptions, 
>EDKII_NAME)
>
>-## Parse build_rule.txt in Conf Directory.
>-#
>-#   @retval BuildRule object
>-#
>-@cached_property
>-def BuildRule(self):
>-BuildRuleFile = None
>-if TAB_TAT_DEFINES_BUILD_RULE_CONF in
>self.Workspace.TargetTxt.TargetTxtDictionary:
>-BuildRuleFile =
>self.Workspace.TargetTxt.TargetTxtDictionary[TAB_TAT_DEFINES_BUILD_RUL
>E_CONF]
>-if not BuildRuleFile:
>-BuildRuleFile = gDefaultBuildRuleFile
>-RetVal = BuildRule(BuildRuleFile)
>-if RetVal._FileVersion == "":
>-RetVal._FileVersion = AutoGenReqBuildRuleVerNum
>-else:
>-if RetVal._FileVersion < AutoGenReqBuildRuleVerNum :
>-# If Build Rule's version is less than the version number 
>required by
>the tools, halting the build.
>-EdkLogger.error("build", AUTOGEN_ERROR,
>-ExtraData="The version number [%s] of 
>build_rule.txt is less
>than the version number required by the AutoGen.(the minimum required 
>version number is [%s])"\
>- % (RetVal._FileVersion, 
>AutoGenReqBuildRuleVerNum))
>-return RetVal
>-
> ## Summarize the packages used by modules in this platform
> @cached_property
> def PackageList(self):
> RetVal = set()
> for La in self.LibraryAutoGenList:
>@@ -3143,11 +3116,11 @@ class ModuleAutoGen(AutoGen):
> return RetVal
>
> @cached_property
> def BuildRules(self):
> RetVal = {}
>-BuildRuleDatabase = self.PlatformInfo.BuildRule
>+BuildRuleDatabase = BuildRule
> for Type in BuildRuleDatabase.FileTypeList:
> #first try getting build rule by BuildRuleFamily
> RuleObject = BuildRuleDatabase[Type, 

[edk2-devel] [Patch 1/1] BaseTools: Split WorkspaceAutoGen._InitWorker into multiple functions

[Bob Feng]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875

The WorkspaceAutoGen.__InitWorker function is too long, it's hard
to read and understand.
This patch is to separate the __InitWorker into multiple small ones.

Cc: Liming Gao 
Signed-off-by: Bob Feng 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 247 +
 1 file changed, 152 insertions(+), 95 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 8bf71d770e2c..088cad9f145a 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -333,13 +333,58 @@ class WorkspaceAutoGen(AutoGen):
 self._GuidDict = {}
 
 # there's many relative directory operations, so ...
 os.chdir(self.WorkspaceDir)
 
+self.MergeArch()
+self.ValidateBuildTarget()
+
+EdkLogger.info("")
+if self.ArchList:
+EdkLogger.info('%-16s = %s' % ("Architecture(s)", ' 
'.join(self.ArchList)))
+EdkLogger.info('%-16s = %s' % ("Build target", self.BuildTarget))
+EdkLogger.info('%-16s = %s' % ("Toolchain", self.ToolChain))
+
+EdkLogger.info('\n%-24s = %s' % ("Active Platform", self.Platform))
+if BuildModule:
+EdkLogger.info('%-24s = %s' % ("Active Module", BuildModule))
+
+if self.FdfFile:
+EdkLogger.info('%-24s = %s' % ("Flash Image Definition", 
self.FdfFile))
+
+EdkLogger.verbose("\nFLASH_DEFINITION = %s" % self.FdfFile)
+
+if Progress:
+Progress.Start("\nProcessing meta-data")
 #
-# Merge Arch
+# Mark now build in AutoGen Phase
 #
+GlobalData.gAutoGenPhase = True
+self.ProcessModuleFromPdf()
+self.ProcessPcdType()
+self.ProcessMixedPcd()
+self.GetPcdsFromFDF()
+self.CollectAllPcds()
+self.GeneratePkgLevelHash()
+#
+# Check PCDs token value conflict in each DEC file.
+#
+self._CheckAllPcdsTokenValueConflict()
+#
+# Check PCD type and definition between DSC and DEC
+#
+self._CheckPcdDefineAndType()
+
+self.CreateBuildOptionsFile()
+self.CreatePcdTokenNumberFile()
+self.CreateModuleHashInfo()
+GlobalData.gAutoGenPhase = False
+
+#
+# Merge Arch
+#
+def MergeArch(self):
 if not self.ArchList:
 ArchList = set(self.Platform.SupArchList)
 else:
 ArchList = set(self.ArchList) & set(self.Platform.SupArchList)
 if not ArchList:
@@ -349,57 +394,49 @@ class WorkspaceAutoGen(AutoGen):
 SkippedArchList = 
set(self.ArchList).symmetric_difference(set(self.Platform.SupArchList))
 EdkLogger.verbose("\nArch [%s] is ignored because the platform 
supports [%s] only!"
   % (" ".join(SkippedArchList), " 
".join(self.Platform.SupArchList)))
 self.ArchList = tuple(ArchList)
 
-# Validate build target
+# Validate build target
+def ValidateBuildTarget(self):
 if self.BuildTarget not in self.Platform.BuildTargets:
 EdkLogger.error("build", PARAMETER_INVALID,
 ExtraData="Build target [%s] is not supported by 
the platform. [Valid target: %s]"
   % (self.BuildTarget, " 
".join(self.Platform.BuildTargets)))
-
-
-# parse FDF file to get PCDs in it, if any
+@cached_property
+def FdfProfile(self):
 if not self.FdfFile:
 self.FdfFile = self.Platform.FlashDefinition
 
-EdkLogger.info("")
-if self.ArchList:
-EdkLogger.info('%-16s = %s' % ("Architecture(s)", ' 
'.join(self.ArchList)))
-EdkLogger.info('%-16s = %s' % ("Build target", self.BuildTarget))
-EdkLogger.info('%-16s = %s' % ("Toolchain", self.ToolChain))
-
-EdkLogger.info('\n%-24s = %s' % ("Active Platform", self.Platform))
-if BuildModule:
-EdkLogger.info('%-24s = %s' % ("Active Module", BuildModule))
-
+FdfProfile = None
 if self.FdfFile:
-EdkLogger.info('%-24s = %s' % ("Flash Image Definition", 
self.FdfFile))
-
-EdkLogger.verbose("\nFLASH_DEFINITION = %s" % self.FdfFile)
-
-if Progress:
-Progress.Start("\nProcessing meta-data")
-
-if self.FdfFile:
-#
-# Mark now build in AutoGen Phase
-#
-GlobalData.gAutoGenPhase = True
 Fdf = FdfParser(self.FdfFile.Path)
 Fdf.ParseFile()
 GlobalData.gFdfParser = Fdf
-GlobalData.gAutoGenPhase = False
-PcdSet = Fdf.Profile.PcdDict
 if Fdf.CurrentFdName and Fdf.CurrentFdName in Fdf.Profile.FdDict:
 FdDict = Fdf.Profile.FdDict[Fdf.CurrentFdName]
 for FdRegion in FdDict.RegionList:
 if 

Re: [edk2-devel] [PATCH] Revert "FmpDevicePkg: Fix various typos" + Reapply as "IntelFsp2Pkg: Fix various typos"

[Liming Gao]

Coeur:
  Please use git send-email to send those patches.

  And, Signed-off-by: Cœur . Can you change 
it Signed-off-by: Coeur  with Ascii char 
only?

Thanks
Liming
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of C?ur
Sent: Thursday, July 11, 2019 5:01 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH] Revert "FmpDevicePkg: Fix various typos" + 
Reapply as "IntelFsp2Pkg: Fix various typos"


Two patches attached:

1. revert f527942e6bdd9f198db90f2de99a0482e9be5b1b, because the commit message 
was incorrect.

2. reapply it with a correct commit message.

If you apply both patches, the balance of changes should be zero (except for 
the commit messages).

Sorry for the confusion.
Coeur


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

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

Re: [edk2-devel] [Patch 1/1] BaseTools: Singleton the object to handle build conf file

[Liming Gao]

Bob:
  target.txt is from Conf directory. build_rule.txt and tools_def.txt are 
specified in target.txt. Please take care this case that they may have the 
different file name. 

  And, Conf directory is relative to WORKSPACE and PACKAGES_PATH. Does this 
patch support it?

Thanks
Liming
>-Original Message-
>From: Feng, Bob C
>Sent: Friday, June 28, 2019 3:07 PM
>To: devel@edk2.groups.io
>Cc: Gao, Liming ; Feng, Bob C 
>Subject: [Patch 1/1] BaseTools: Singleton the object to handle build conf file
>
>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875
>
>The build config files are target.txt, build rule, tooldef
>During a build, the config is not changed, so the object to
>handle them need to be singleton.
>
>Cc: Liming Gao 
>Signed-off-by: Bob Feng 
>---
> BaseTools/Source/Python/AutoGen/AutoGen.py| 33 ++--
> .../Source/Python/AutoGen/BuildEngine.py  | 22 
> .../Python/Common/TargetTxtClassObject.py |  2 +
> .../Python/Common/ToolDefClassObject.py   |  6 ++-
> BaseTools/Source/Python/GenFds/GenFds.py  |  4 +-
> .../Python/GenFds/GenFdsGlobalVariable.py | 54 ---
> .../Source/Python/Workspace/DscBuildData.py   |  8 +--
> BaseTools/Source/Python/build/build.py| 29 +++---
> 8 files changed, 62 insertions(+), 96 deletions(-)
>
>diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>b/BaseTools/Source/Python/AutoGen/AutoGen.py
>index e8e09dc8a366..a1f7f5641e09 100644
>--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>@@ -22,11 +22,12 @@ from . import GenC
> from . import GenMake
> from . import GenDepex
> from io import BytesIO
>
> from .StrGather import *
>-from .BuildEngine import BuildRule
>+from .BuildEngine import BuildRuleObj as BuildRule
>+from .BuildEngine import
>gDefaultBuildRuleFile,AutoGenReqBuildRuleVerNum
> import shutil
> from Common.LongFilePathSupport import CopyLongFilePath
> from Common.BuildToolError import *
> from Common.DataType import *
> from Common.Misc import *
>@@ -76,16 +77,10 @@ gEfiVarStoreGuidPattern =
>re.compile("\s*guid\s*=\s*({.*?{.*?}\s*})")
>
> ## Mapping Makefile type
> gMakeTypeMap = {TAB_COMPILER_MSFT:"nmake", "GCC":"gmake"}
>
>
>-## Build rule configuration file
>-gDefaultBuildRuleFile = 'build_rule.txt'
>-
>-## Build rule default version
>-AutoGenReqBuildRuleVerNum = "0.1"
>-
> ## default file name for AutoGen
> gAutoGenCodeFileName = "AutoGen.c"
> gAutoGenHeaderFileName = "AutoGen.h"
> gAutoGenStringFileName = "%(module_name)sStrDefs.h"
> gAutoGenStringFormFileName = "%(module_name)sStrDefs.hpk"
>@@ -1964,32 +1959,10 @@ class PlatformAutoGen(AutoGen):
> ## Return the build options specific for EDKII modules in this platform
> @cached_property
> def EdkIIBuildOption(self):
> return self._ExpandBuildOption(self.Platform.BuildOptions, EDKII_NAME)
>
>-## Parse build_rule.txt in Conf Directory.
>-#
>-#   @retval BuildRule object
>-#
>-@cached_property
>-def BuildRule(self):
>-BuildRuleFile = None
>-if TAB_TAT_DEFINES_BUILD_RULE_CONF in
>self.Workspace.TargetTxt.TargetTxtDictionary:
>-BuildRuleFile =
>self.Workspace.TargetTxt.TargetTxtDictionary[TAB_TAT_DEFINES_BUILD_RUL
>E_CONF]
>-if not BuildRuleFile:
>-BuildRuleFile = gDefaultBuildRuleFile
>-RetVal = BuildRule(BuildRuleFile)
>-if RetVal._FileVersion == "":
>-RetVal._FileVersion = AutoGenReqBuildRuleVerNum
>-else:
>-if RetVal._FileVersion < AutoGenReqBuildRuleVerNum :
>-# If Build Rule's version is less than the version number 
>required by
>the tools, halting the build.
>-EdkLogger.error("build", AUTOGEN_ERROR,
>-ExtraData="The version number [%s] of 
>build_rule.txt is less
>than the version number required by the AutoGen.(the minimum required
>version number is [%s])"\
>- % (RetVal._FileVersion, 
>AutoGenReqBuildRuleVerNum))
>-return RetVal
>-
> ## Summarize the packages used by modules in this platform
> @cached_property
> def PackageList(self):
> RetVal = set()
> for La in self.LibraryAutoGenList:
>@@ -3143,11 +3116,11 @@ class ModuleAutoGen(AutoGen):
> return RetVal
>
> @cached_property
> def BuildRules(self):
> RetVal = {}
>-BuildRuleDatabase = self.PlatformInfo.BuildRule
>+BuildRuleDatabase = BuildRule
> for Type in BuildRuleDatabase.FileTypeList:
> #first try getting build rule by BuildRuleFamily
> RuleObject = BuildRuleDatabase[Type, self.BuildType, self.Arch,
>self.BuildRuleFamily]
> if not RuleObject:
> # build type is always module type, but ...
>diff --git a/BaseTools/Source/Python/AutoGen/BuildEngine.py
>b/BaseTools/Source/Python/AutoGen/BuildEngine.py
>index 14e61140e7ba..bb9153447793 100644
>--- 

[edk2-devel] [PATCH] Revert "FmpDevicePkg: Fix various typos" + Reapply as "IntelFsp2Pkg: Fix various typos"

[Cœur]

Two patches attached:

1. revert f527942e6bdd9f198db90f2de99a0482e9be5b1b, because the commit message 
was incorrect.

2. reapply it with a correct commit message.

If you apply both patches, the balance of changes should be zero (except for 
the commit messages).

Sorry for the confusion.
Coeur

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

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



patch1.diff
Description: Binary data


patch2.diff
Description: Binary data

Re: [edk2-devel] [Patch 0/7] Revert new added BfmLib/FCE/FMMT

[Leif Lindholm]

On Thu, Jul 11, 2019 at 09:44:33AM +0800, Liming Gao wrote:
> Rewrite those tools for review again.
> Now, move them into edk2-staging/FceFmmt branch.
> 
> Cc: Leif Lindholm 
> Cc: Feng Bob C 

Thanks for this. For the series:
Reviewed-by: Leif Lindholm 

> Liming Gao (7):
>   Revert "BaseTools/FCE: Change FCE script type in PosixLike"
>   Revert "BaseTools/BfmLib: Change BfmLib script type in PosixLike"
>   Revert "BaseTools/FMMT: Change FMMT script type in PosixLike"
>   Revert "BaseTools: fix FCE build when edksetup not executed"
>   Revert "BaseTools/FMMT: Add a tool FMMT"
>   Revert "BaseTools/FCE: Add a tool FCE"
>   Revert "BaseTools/BfmLib: Add a tool BfmLib"
> 
>  BaseTools/Source/C/BfmLib/BfmLib.c | 4355 -
>  BaseTools/Source/C/BfmLib/BinFileManager.c | 1024 
>  BaseTools/Source/C/FCE/BinaryCreate.c  |  216 -
>  BaseTools/Source/C/FCE/BinaryParse.c   | 1326 
>  BaseTools/Source/C/FCE/Common.c| 2183 ---
>  BaseTools/Source/C/FCE/Expression.c| 2367 ---
>  BaseTools/Source/C/FCE/Fce.c   | 6449 
> 
>  BaseTools/Source/C/FCE/IfrParse.c  | 4836 ---
>  BaseTools/Source/C/FCE/MonotonicBasedVariable.c|  874 ---
>  BaseTools/Source/C/FCE/TimeBasedVariable.c |  878 ---
>  BaseTools/Source/C/FCE/Variable.c  | 1091 
>  BaseTools/Source/C/FMMT/FirmwareModuleManagement.c | 2559 
>  BaseTools/Source/C/FMMT/FmmtLib.c  | 5051 ---
>  BaseTools/Source/C/FMMT/Rebase.c   |  846 ---
>  BaseTools/BinWrappers/PosixLike/BfmLib |   29 -
>  BaseTools/BinWrappers/PosixLike/FCE|   29 -
>  BaseTools/BinWrappers/PosixLike/FMMT   |   29 -
>  BaseTools/Source/C/BfmLib/BinFileManager.h |  439 --
>  BaseTools/Source/C/BfmLib/GNUmakefile  |   15 -
>  BaseTools/Source/C/BfmLib/Makefile |   17 -
>  BaseTools/Source/C/FCE/BinaryCreate.h  |  157 -
>  BaseTools/Source/C/FCE/BinaryParse.h   |  187 -
>  BaseTools/Source/C/FCE/Common.h|  999 ---
>  BaseTools/Source/C/FCE/Fce.h   |  447 --
>  BaseTools/Source/C/FCE/GNUmakefile |   55 -
>  BaseTools/Source/C/FCE/IfrParse.h  |  789 ---
>  BaseTools/Source/C/FCE/Makefile|   19 -
>  BaseTools/Source/C/FCE/MonotonicBasedVariable.h|  162 -
>  BaseTools/Source/C/FCE/TimeBasedVariable.h |  166 -
>  BaseTools/Source/C/FCE/Variable.h  |  154 -
>  BaseTools/Source/C/FCE/VariableCommon.h|   55 -
>  BaseTools/Source/C/FMMT/FirmwareModuleManagement.h |  479 --
>  BaseTools/Source/C/FMMT/FmmtConf.ini   |6 -
>  BaseTools/Source/C/FMMT/GNUmakefile|   16 -
>  BaseTools/Source/C/FMMT/Makefile   |   17 -
>  BaseTools/Source/C/FMMT/Rebase.h   |   31 -
>  BaseTools/Source/C/GNUmakefile |5 +-
>  BaseTools/Source/C/Makefile|5 +-
>  38 files changed, 2 insertions(+), 38360 deletions(-)
>  delete mode 100644 BaseTools/Source/C/BfmLib/BfmLib.c
>  delete mode 100644 BaseTools/Source/C/BfmLib/BinFileManager.c
>  delete mode 100644 BaseTools/Source/C/FCE/BinaryCreate.c
>  delete mode 100644 BaseTools/Source/C/FCE/BinaryParse.c
>  delete mode 100644 BaseTools/Source/C/FCE/Common.c
>  delete mode 100644 BaseTools/Source/C/FCE/Expression.c
>  delete mode 100644 BaseTools/Source/C/FCE/Fce.c
>  delete mode 100644 BaseTools/Source/C/FCE/IfrParse.c
>  delete mode 100644 BaseTools/Source/C/FCE/MonotonicBasedVariable.c
>  delete mode 100644 BaseTools/Source/C/FCE/TimeBasedVariable.c
>  delete mode 100644 BaseTools/Source/C/FCE/Variable.c
>  delete mode 100644 BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
>  delete mode 100644 BaseTools/Source/C/FMMT/FmmtLib.c
>  delete mode 100644 BaseTools/Source/C/FMMT/Rebase.c
>  delete mode 100755 BaseTools/BinWrappers/PosixLike/BfmLib
>  delete mode 100755 BaseTools/BinWrappers/PosixLike/FCE
>  delete mode 100755 BaseTools/BinWrappers/PosixLike/FMMT
>  delete mode 100644 BaseTools/Source/C/BfmLib/BinFileManager.h
>  delete mode 100644 BaseTools/Source/C/BfmLib/GNUmakefile
>  delete mode 100644 BaseTools/Source/C/BfmLib/Makefile
>  delete mode 100644 BaseTools/Source/C/FCE/BinaryCreate.h
>  delete mode 100644 BaseTools/Source/C/FCE/BinaryParse.h
>  delete mode 100644 BaseTools/Source/C/FCE/Common.h
>  delete mode 100644 BaseTools/Source/C/FCE/Fce.h
>  delete mode 100644 BaseTools/Source/C/FCE/GNUmakefile
>  delete mode 100644 BaseTools/Source/C/FCE/IfrParse.h
>  delete mode 100644 BaseTools/Source/C/FCE/Makefile
>  delete mode 100644 BaseTools/Source/C/FCE/MonotonicBasedVariable.h
>  delete mode 100644 BaseTools/Source/C/FCE/TimeBasedVariable.h
>  delete mode 100644 

Re: [edk2-devel] [edk2-platforms: PATCH] Marvell/Drivers: XenonDxe: Explicitly disable HS400

[Leif Lindholm]

On Thu, Jul 11, 2019 at 10:07:27AM +0200, Marcin Wojtas wrote:
> Hi Leif,
> 
> śr., 26 cze 2019 o 11:58 Marcin Wojtas  napisał(a):
> >
> > Hi Leif,
> >
> > śr., 26 cze 2019 o 11:31 Leif Lindholm  
> > napisał(a):
> > >
> > > On Wed, Jun 26, 2019 at 09:04:14AM +0200, Marcin Wojtas wrote:
> > > > Ensure that in case of SlowMode or 3.3V operation,
> > > > also the HS400 capability will be disabled in the
> > > > SdMmc driver.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > >
> > > Well done on keeping this tag. But I'm thinking we need to do that
> > > relicensing sooner rather than later, and drop the tag.
> >
> > I left it, as this file is still not 2-clause SPDX tagged.
> >
> > >
> > >
> > > However, can you clarify what problem this solves?
> > >
> >
> > On another SoC revision, the capability register marks HS400 support
> > as enabled. However the interface itself is powered with 3.3V and it
> > turned out that my flags in SdMmcOverride driver did not cover this
> > case, which resulted in an unsuccessful EmmcSwitchToHS400 () execution
> > - it shouldn't be done at all.
> >
> 
> Did you have a chance to see my explanation? Should I repost?

Sorry, yes. Explanation is fine. If you can update the commit message
and drop the Contributed-under, I will push this once we get the
licenses updated.

Best Regards,

Leif

> Best regards,
> Marcin
> 
> > > /
> > > Leif
> > >
> > > > Signed-off-by: Marcin Wojtas 
> > > > ---
> > > >  Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h | 1 +
> > > >  Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c | 5 +++--
> > > >  2 files changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h 
> > > > b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h
> > > > index 8bf1835..2d7c7f0 100644
> > > > --- a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h
> > > > +++ b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h
> > > > @@ -82,6 +82,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH 
> > > > DAMAGE.
> > > >  #define SDHC_CAP_SDR50BIT32
> > > >  #define SDHC_CAP_SDR104   BIT33
> > > >  #define SDHC_CAP_DDR50BIT34
> > > > +#define SDHC_CAP_HS400BIT63
> > > >  #define SDHC_MAX_CURRENT_CAP  0x0048
> > > >  #define SDHC_FORCE_EVT_AUTO_CMD   0x0050
> > > >  #define SDHC_FORCE_EVT_ERR_INT0x0052
> > > > diff --git 
> > > > a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c 
> > > > b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c
> > > > index 7a9266e..55ebcf8 100644
> > > > --- a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c
> > > > +++ b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c
> > > > @@ -357,7 +357,8 @@ XenonSdMmcCapability (
> > > >  Capability &= ~(UINT64)(SDHC_CAP_VOLTAGE_33 | SDHC_CAP_VOLTAGE_30);
> > > >} else {
> > > >  Capability &= ~(UINT64)(SDHC_CAP_SDR104 | SDHC_CAP_DDR50 |
> > > > -SDHC_CAP_SDR50 | SDHC_CAP_VOLTAGE_18);
> > > > +SDHC_CAP_SDR50 | SDHC_CAP_HS400 |
> > > > +SDHC_CAP_VOLTAGE_18);
> > > >}
> > > >
> > > >if (!SdMmcDesc.Xenon8BitBusEnabled) {
> > > > @@ -365,7 +366,7 @@ XenonSdMmcCapability (
> > > >}
> > > >
> > > >if (SdMmcDesc.XenonSlowModeEnabled) {
> > > > -Capability &= ~(UINT64)(SDHC_CAP_SDR104 | SDHC_CAP_DDR50);
> > > > +Capability &= ~(UINT64)(SDHC_CAP_SDR104 | SDHC_CAP_DDR50 | 
> > > > SDHC_CAP_HS400);
> > > >}
> > > >
> > > >Capability &= ~(UINT64)(SDHC_CAP_SLOT_TYPE_MASK);
> > > > --
> > > > 2.7.4
> > > >

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

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

Re: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

[Sami Mujawar]

Hi Mike,

Since LocalityCount is 64-bit wide the SLIT validation code could possibly end 
up in an infinite loop. I am not aware of a platform that has a large enough 
LocalityCount to hit this condition. However, would it be good to have a check 
that limits the validation to MAX_UINT32?

e.g. Something like
if  (LocalityCount < MAX_UINT32) {
  // Validate
  for (Count = 0; Count < LocalityCount; Count++) {
for (Index = 0; Index < LocalityCount; Index++) {
  ...
} else {
  Print (L"INFO: Skipping validation of System Localities as locality count is 
> MAX_UINT32\n");
}

Regards,

Sami Mujawar

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Michael D Kinney 
via Groups.Io
Sent: 10 July 2019 11:35 PM
To: devel@edk2.groups.io
Cc: Jaben Carsey ; Ray Ni ; Zhichao 
Gao 
Subject: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

https://bugzilla.tianocore.org/show_bug.cgi?id=1970

Update local variable in ParseAcpiSlot() to be UINT32 instead of UINT64 to 
avoid 64-bit multiply operation in the SLIT_ELEMENT() macro.

Cc: Jaben Carsey 
Cc: Ray Ni 
Cc: Zhichao Gao 
Signed-off-by: Michael D Kinney 
---
 .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
index 1f9dac66ee..af85c9aa1c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitPars
+++ er.c
@@ -57,8 +57,8 @@ ParseAcpiSlit (
   )
 {
   UINT32 Offset;
-  UINT64 Count;
-  UINT64 Index;
+  UINT32 Count;
+  UINT32 Index;
   UINT64 LocalityCount;
   UINT8* LocalityPtr;
   CHAR16 Buffer[80];  // Used for AsciiName param of ParseAcpi
--
2.21.0.windows.1




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.

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

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

Re: [edk2-devel] [edk2-platforms: PATCH] Marvell/Drivers: XenonDxe: Explicitly disable HS400

[Marcin Wojtas]

Hi Leif,

śr., 26 cze 2019 o 11:58 Marcin Wojtas  napisał(a):
>
> Hi Leif,
>
> śr., 26 cze 2019 o 11:31 Leif Lindholm  napisał(a):
> >
> > On Wed, Jun 26, 2019 at 09:04:14AM +0200, Marcin Wojtas wrote:
> > > Ensure that in case of SlowMode or 3.3V operation,
> > > also the HS400 capability will be disabled in the
> > > SdMmc driver.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> >
> > Well done on keeping this tag. But I'm thinking we need to do that
> > relicensing sooner rather than later, and drop the tag.
>
> I left it, as this file is still not 2-clause SPDX tagged.
>
> >
> >
> > However, can you clarify what problem this solves?
> >
>
> On another SoC revision, the capability register marks HS400 support
> as enabled. However the interface itself is powered with 3.3V and it
> turned out that my flags in SdMmcOverride driver did not cover this
> case, which resulted in an unsuccessful EmmcSwitchToHS400 () execution
> - it shouldn't be done at all.
>

Did you have a chance to see my explanation? Should I repost?

Best regards,
Marcin

> > /
> > Leif
> >
> > > Signed-off-by: Marcin Wojtas 
> > > ---
> > >  Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h | 1 +
> > >  Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c | 5 +++--
> > >  2 files changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h 
> > > b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h
> > > index 8bf1835..2d7c7f0 100644
> > > --- a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h
> > > +++ b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h
> > > @@ -82,6 +82,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH 
> > > DAMAGE.
> > >  #define SDHC_CAP_SDR50BIT32
> > >  #define SDHC_CAP_SDR104   BIT33
> > >  #define SDHC_CAP_DDR50BIT34
> > > +#define SDHC_CAP_HS400BIT63
> > >  #define SDHC_MAX_CURRENT_CAP  0x0048
> > >  #define SDHC_FORCE_EVT_AUTO_CMD   0x0050
> > >  #define SDHC_FORCE_EVT_ERR_INT0x0052
> > > diff --git a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c 
> > > b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c
> > > index 7a9266e..55ebcf8 100644
> > > --- a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c
> > > +++ b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c
> > > @@ -357,7 +357,8 @@ XenonSdMmcCapability (
> > >  Capability &= ~(UINT64)(SDHC_CAP_VOLTAGE_33 | SDHC_CAP_VOLTAGE_30);
> > >} else {
> > >  Capability &= ~(UINT64)(SDHC_CAP_SDR104 | SDHC_CAP_DDR50 |
> > > -SDHC_CAP_SDR50 | SDHC_CAP_VOLTAGE_18);
> > > +SDHC_CAP_SDR50 | SDHC_CAP_HS400 |
> > > +SDHC_CAP_VOLTAGE_18);
> > >}
> > >
> > >if (!SdMmcDesc.Xenon8BitBusEnabled) {
> > > @@ -365,7 +366,7 @@ XenonSdMmcCapability (
> > >}
> > >
> > >if (SdMmcDesc.XenonSlowModeEnabled) {
> > > -Capability &= ~(UINT64)(SDHC_CAP_SDR104 | SDHC_CAP_DDR50);
> > > +Capability &= ~(UINT64)(SDHC_CAP_SDR104 | SDHC_CAP_DDR50 | 
> > > SDHC_CAP_HS400);
> > >}
> > >
> > >Capability &= ~(UINT64)(SDHC_CAP_SLOT_TYPE_MASK);
> > > --
> > > 2.7.4
> > >

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

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

Re: [edk2-devel] [Patch v5 1/2] MdePkg: Add new MM MP Protocol definition.

[Liming Gao]

Thanks for your CC.

The change is OK. Reviewed-by: Liming Gao 

>-Original Message-
>From: Dong, Eric
>Sent: Thursday, July 11, 2019 2:40 PM
>To: devel@edk2.groups.io; Dong, Eric ; Gao, Liming
>; Kinney, Michael D 
>Cc: Ni, Ray ; Laszlo Ersek 
>Subject: RE: [edk2-devel] [Patch v5 1/2] MdePkg: Add new MM MP Protocol
>definition.
>
>Hi Liming,  Mike,
>
>Can you help to review this patch?
>
>Thanks,
>Eric
>
>> -Original Message-
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Dong, Eric
>> Sent: Wednesday, July 10, 2019 3:56 PM
>> To: devel@edk2.groups.io
>> Cc: Ni, Ray ; Laszlo Ersek 
>> Subject: [edk2-devel] [Patch v5 1/2] MdePkg: Add new MM MP Protocol
>> definition.
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1937
>>
>> EFI MM MP Protocol is defined in the PI 1.5 specification.
>>
>> The MM MP protocol provides a set of functions to allow execution of
>> procedures on processors that have entered MM. This protocol has the
>> following properties:
>> 1. The caller can invoke execution of a procedure on a processor, other than
>> the caller, that has also entered MM. Supports blocking and non-blocking
>> modes of operation.
>> 2. The caller can invoke a procedure on multiple processors. Supports
>> blocking and non-blocking modes of operation.
>>
>> Cc: Ray Ni 
>> Cc: Laszlo Ersek 
>> Signed-off-by: Eric Dong 
>> Reviewed-by: Ray Ni 
>> ---
>>  MdePkg/Include/Pi/PiMultiPhase.h |  16 ++
>>  MdePkg/Include/Protocol/MmMp.h   | 333
>> +++
>>  MdePkg/MdePkg.dec|   3 +
>>  3 files changed, 352 insertions(+)
>>  create mode 100644 MdePkg/Include/Protocol/MmMp.h
>>
>> diff --git a/MdePkg/Include/Pi/PiMultiPhase.h
>> b/MdePkg/Include/Pi/PiMultiPhase.h
>> index eb12527767..a5056799e1 100644
>> --- a/MdePkg/Include/Pi/PiMultiPhase.h
>> +++ b/MdePkg/Include/Pi/PiMultiPhase.h
>> @@ -176,4 +176,20 @@ VOID
>>IN OUT VOID  *Buffer
>>);
>>
>> +/**
>> +  The function prototype for invoking a function on an Application
>Processor.
>> +
>> +  This definition is used by the UEFI MM MP Serices Protocol.
>> +
>> +  @param[in] ProcedureArgumentThe pointer to private data buffer.
>> +
>> +  @retval EFI_SUCCESS Excutive the procedure successfully
>> +
>> +**/
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *EFI_AP_PROCEDURE2)(
>> +  IN VOID  *ProcedureArgument
>> +);
>> +
>>  #endif
>> diff --git a/MdePkg/Include/Protocol/MmMp.h
>> b/MdePkg/Include/Protocol/MmMp.h new file mode 100644 index
>> 00..beace1386c
>> --- /dev/null
>> +++ b/MdePkg/Include/Protocol/MmMp.h
>> @@ -0,0 +1,333 @@
>> +/** @file
>> +  EFI MM MP Protocol is defined in the PI 1.5 specification.
>> +
>> +  The MM MP protocol provides a set of functions to allow execution of
>> + procedures on processors that  have entered MM. This protocol has the
>> following properties:
>> +  1. The caller can only invoke execution of a procedure on a processor,
>> other than the caller, that
>> + has also entered MM.
>> +  2. It is possible to invoke a procedure on multiple processors. Supports
>> blocking and non-blocking
>> + modes of operation.
>> +
>> +  Copyright (c) 2019, Intel Corporation. All rights reserved.
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#ifndef _MM_MP_H_
>> +#define _MM_MP_H_
>> +
>> +#include 
>> +
>> +#define EFI_MM_MP_PROTOCOL_GUID \
>> +  { \
>> +0x5d5450d7, 0x990c, 0x4180, {0xa8, 0x3, 0x8e, 0x63, 0xf0, 0x60,
>> +0x83, 0x7  }  \
>> +  }
>> +
>> +//
>> +// Revision definition.
>> +//
>> +#define EFI_MM_MP_PROTOCOL_REVISION0x00
>> +
>> +//
>> +// Attribute flags
>> +//
>> +#define EFI_MM_MP_TIMEOUT_SUPPORTED0x01
>> +
>> +//
>> +// Completion token
>> +//
>> +typedef VOID* MM_COMPLETION;
>> +
>> +typedef struct {
>> +  MM_COMPLETION  Completion;
>> +  EFI_STATUS Status;
>> +} MM_DISPATCH_COMPLETION_TOKEN;
>> +
>> +typedef struct _EFI_MM_MP_PROTOCOL  EFI_MM_MP_PROTOCOL;
>> +
>> +/**
>> +  Service to retrieves the number of logical processor in the platform.
>> +
>> +  @param[in]  ThisThe EFI_MM_MP_PROTOCOL instance.
>> +  @param[out] NumberOfProcessors  Pointer to the total number of logical
>> processors in the system,
>> +  including the BSP and all APs.
>> +
>> +  @retval EFI_SUCCESS The number of processors was retrieved
>> successfully
>> +  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL
>> +**/
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *EFI_MM_GET_NUMBER_OF_PROCESSORS) (
>> +  IN CONST EFI_MM_MP_PROTOCOL  *This,
>> +  OUT  UINTN   *NumberOfProcessors
>> +);
>> +
>> +
>> +/**
>> +  This service allows the caller to invoke a procedure one of the
>> +application processors (AP). This
>> +  function uses an optional token parameter to support blocking and
>> +non-blocking modes. If the token
>> +  is passed into the call, the function will operate in a non-blocking
>> +fashion and the caller can

Re: [edk2-devel] [Patch 1/1] BaseTools: Fixed the issue when ToolDefinitionFile is not generated

[Liming Gao]

Reviewed-by: Liming Gao 

>-Original Message-
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Bob Feng
>Sent: Thursday, July 11, 2019 8:58 AM
>To: devel@edk2.groups.io
>Cc: Gao, Liming ; Feng, Bob C 
>Subject: [edk2-devel] [Patch 1/1] BaseTools: Fixed the issue when
>ToolDefinitionFile is not generated
>
>ToolDefinitionFile is generated by PlatformAutoGen.ToolDefinition()
>Code assume ToolDefinition is always called before using
>ToolDefinitionFile, but in some cases, it's not true.
>
>This patch is to fix this issue.
>
>Cc: Liming Gao 
>Signed-off-by: Bob Feng 
>---
> BaseTools/Source/Python/AutoGen/AutoGen.py | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>b/BaseTools/Source/Python/AutoGen/AutoGen.py
>index c0d0ca15867b..5cfaf2141af0 100644
>--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>@@ -1910,22 +1910,25 @@ class PlatformAutoGen(AutoGen):
> if Attr == "FLAGS":
> MakeFlags = Value
> else:
> ToolsDef += "%s_%s = %s\n" % (Tool, Attr, Value)
> ToolsDef += "\n"
>-
>-SaveFileOnChange(self.ToolDefinitionFile, ToolsDef, False)
>+tool_def_file = os.path.join(self.MakeFileDir, "TOOLS_DEF." + 
>self.Arch)
>+SaveFileOnChange(tool_def_file, ToolsDef, False)
> for DllPath in DllPathList:
> os.environ["PATH"] = DllPath + os.pathsep + os.environ["PATH"]
> os.environ["MAKE_FLAGS"] = MakeFlags
>
> return RetVal
>
> ## Return the paths of tools
> @cached_property
> def ToolDefinitionFile(self):
>-return os.path.join(self.MakeFileDir, "TOOLS_DEF." + self.Arch)
>+tool_def_file = os.path.join(self.MakeFileDir, "TOOLS_DEF." + 
>self.Arch)
>+if not os.path.exists(tool_def_file):
>+self.ToolDefinition
>+return tool_def_file
>
> ## Retrieve the toolchain family of given toolchain tag. Default to 
> 'MSFT'.
> @cached_property
> def ToolChainFamily(self):
> ToolDefinition = self.Workspace.ToolDef.ToolsDefTxtDatabase
>--
>2.20.1.windows.1
>
>
>


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

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

Re: [edk2-devel] [ PATCH v2 1/2] Readme.md: add submodule policy and clone commands

[Wang, Jian J]

Liming,

> -Original Message-
> From: Gao, Liming
> Sent: Wednesday, July 10, 2019 3:44 PM
> To: devel@edk2.groups.io; Wang, Jian J 
> Cc: Leif Lindholm ; Kinney, Michael D
> 
> Subject: RE: [edk2-devel] [ PATCH v2 1/2] Readme.md: add submodule
> policy and clone commands
> 
> Jian:
> 
> >-Original Message-
> >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> >Wang, Jian J
> >Sent: Wednesday, July 10, 2019 2:10 PM
> >To: devel@edk2.groups.io
> >Cc: Leif Lindholm ; Kinney, Michael D
> >; Gao, Liming 
> >Subject: [edk2-devel] [ PATCH v2 1/2] Readme.md: add submodule policy
> and
> >clone commands
> >
> >> v2: update wording per Leif's and others' comments
> >
> >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1910
> >
> >A section 'Submodules' is added to clarify the submodule policy
> >in edk2 repo. Git commands are also added to show the correct
> >way to clone submodule repos, in which '--recursive' is removed
> >because it's not needed but recommended in other document.
> >
> >Related commits:
> >Openssl-1.1.1b upgrade: acfb90911840c38a0beb9bcfe0065668244d2b4d
> >berkeley-softfloat-3:   3cc57695df5a6e8c65fb46b993836c315cabf49d
> >
> >Cc: Leif Lindholm 
> >Cc: Michael D Kinney 
> >Cc: Liming Gao 
> >Signed-off-by: Jian J Wang 
> >---
> > Readme.md | 28 
> > 1 file changed, 28 insertions(+)
> >
> >diff --git a/Readme.md b/Readme.md
> >index e564c6c09b..c475468655 100644
> >--- a/Readme.md
> >+++ b/Readme.md
> >@@ -143,3 +143,31 @@ Signed-off-by: Contributor Name
> >
> >   the change.  Each line should be less than ~70 characters.
> > * `Signed-off-by` is the contributor's signature identifying them
> >   by their real/legal name and their email address.
> >+
> >+# Submodules
> >+
> >+As a general policy, submodule chain should be avoided in EDK II repo
> >+as possible as we can. Currently EDK II  contains two submodules
> Here, I understand to avoid the child submodule, and the root submodule is
> allowed.
> Besides, there is two spaces between EDK II  contains. Please take care.
> 

No exactly. I think it's dependency chain, i.e. one submodule depends on
another submodule. It's out of our control that one submodule imports
other submodules in its repo. What about changing the first statement to
following:

Submodule is allowed to import 3rd party code into EDK II repo. But
submodules that require further submodules should be avoided as possible
as we can.

And I'll remove the extra space here.

> >+
> >+- CryptoPkg/Library/OpensslLib/openssl
> >+- ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3
> >+
> >+The latter one is actually required by previous one. It's inevitable
> >+in openssl-1.1.1 (since stable201905) for floating point parameter
> >+conversion, but should be dropped once there's no such need in future
> >+release of openssl.
> >+
> >+To get a full, buildable EDK II repository, use following two steps
> >+of git command
> >+
> >+```
> >+$ git clone https://github.com/tianocore/edk2.git
> >+$ git submodule update --init
> >+```
> >+
> Please also give the update command for submodule.
> 

I'll add it.

> Thanks
> Liming
> >+Note: When cloning submodule repos, '--recursive' option is not
> >+recommended. EDK II itself will not use any code/feature from
> >+submodules in above submodules. So using '--recursive' adds a
> >+dependency on being able to reach servers we do not actually want
> >+any code from, as well as needlessly downloading code we will not
> >+use.
> >--
> >2.17.1.windows.2
> >
> >
> >


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

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

Re: [edk2-devel] GCC compiler version

[Gary Lin]

On Thu, Jul 11, 2019 at 05:07:49AM +,  Liming Gao  wrote:
> Hi, all
>   Recently, I find some failure with the different GCC compiler version. I 
> want to collect which GCC version are used by you. Now, I use GCC 5.5 for the 
> verification. It may be a little old.
> 
Hi Liming,

I'm using openSUSE Tumbleweed which basically uses the latest stable
version of everything. The current gcc version is 9.1.1.

Thanks,

Gary Lin

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

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

Re: [edk2-devel] [Patch v5 1/2] MdePkg: Add new MM MP Protocol definition.

[Ni, Ray]

+ MdePkg maintainers

We need to follow the open source community process to include MdePkg
maintainers for this change review.

Laszlo,
You are in the Cc list and had given Regression-Tested-By for V3 patch series.
It would be great if you could give patch owner a hint to avoid breaking the 
process
though I understand it's not responsibility of the people in the CC list. 

Thanks,
Ray


> -Original Message-
> From: Dong, Eric
> Sent: Wednesday, July 10, 2019 3:56 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Laszlo Ersek 
> Subject: [Patch v5 1/2] MdePkg: Add new MM MP Protocol definition.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1937
> 
> EFI MM MP Protocol is defined in the PI 1.5 specification.
> 
> The MM MP protocol provides a set of functions to allow execution of
> procedures on processors that have entered MM. This protocol has the
> following properties:
> 1. The caller can invoke execution of a procedure on a processor, other than
> the caller, that has also entered MM. Supports blocking and non-blocking
> modes of operation.
> 2. The caller can invoke a procedure on multiple processors. Supports
> blocking and non-blocking modes of operation.
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Eric Dong 
> Reviewed-by: Ray Ni 
> ---
>  MdePkg/Include/Pi/PiMultiPhase.h |  16 ++
>  MdePkg/Include/Protocol/MmMp.h   | 333
> +++
>  MdePkg/MdePkg.dec|   3 +
>  3 files changed, 352 insertions(+)
>  create mode 100644 MdePkg/Include/Protocol/MmMp.h
> 
> diff --git a/MdePkg/Include/Pi/PiMultiPhase.h
> b/MdePkg/Include/Pi/PiMultiPhase.h
> index eb12527767..a5056799e1 100644
> --- a/MdePkg/Include/Pi/PiMultiPhase.h
> +++ b/MdePkg/Include/Pi/PiMultiPhase.h
> @@ -176,4 +176,20 @@ VOID
>IN OUT VOID  *Buffer
>);
> 
> +/**
> +  The function prototype for invoking a function on an Application Processor.
> +
> +  This definition is used by the UEFI MM MP Serices Protocol.
> +
> +  @param[in] ProcedureArgumentThe pointer to private data buffer.
> +
> +  @retval EFI_SUCCESS Excutive the procedure successfully
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_AP_PROCEDURE2)(
> +  IN VOID  *ProcedureArgument
> +);
> +
>  #endif
> diff --git a/MdePkg/Include/Protocol/MmMp.h
> b/MdePkg/Include/Protocol/MmMp.h new file mode 100644 index
> 00..beace1386c
> --- /dev/null
> +++ b/MdePkg/Include/Protocol/MmMp.h
> @@ -0,0 +1,333 @@
> +/** @file
> +  EFI MM MP Protocol is defined in the PI 1.5 specification.
> +
> +  The MM MP protocol provides a set of functions to allow execution of
> + procedures on processors that  have entered MM. This protocol has the
> following properties:
> +  1. The caller can only invoke execution of a procedure on a processor,
> other than the caller, that
> + has also entered MM.
> +  2. It is possible to invoke a procedure on multiple processors. Supports
> blocking and non-blocking
> + modes of operation.
> +
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef _MM_MP_H_
> +#define _MM_MP_H_
> +
> +#include 
> +
> +#define EFI_MM_MP_PROTOCOL_GUID \
> +  { \
> +0x5d5450d7, 0x990c, 0x4180, {0xa8, 0x3, 0x8e, 0x63, 0xf0, 0x60,
> +0x83, 0x7  }  \
> +  }
> +
> +//
> +// Revision definition.
> +//
> +#define EFI_MM_MP_PROTOCOL_REVISION0x00
> +
> +//
> +// Attribute flags
> +//
> +#define EFI_MM_MP_TIMEOUT_SUPPORTED0x01
> +
> +//
> +// Completion token
> +//
> +typedef VOID* MM_COMPLETION;
> +
> +typedef struct {
> +  MM_COMPLETION  Completion;
> +  EFI_STATUS Status;
> +} MM_DISPATCH_COMPLETION_TOKEN;
> +
> +typedef struct _EFI_MM_MP_PROTOCOL  EFI_MM_MP_PROTOCOL;
> +
> +/**
> +  Service to retrieves the number of logical processor in the platform.
> +
> +  @param[in]  ThisThe EFI_MM_MP_PROTOCOL instance.
> +  @param[out] NumberOfProcessors  Pointer to the total number of logical
> processors in the system,
> +  including the BSP and all APs.
> +
> +  @retval EFI_SUCCESS The number of processors was retrieved
> successfully
> +  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_MM_GET_NUMBER_OF_PROCESSORS) (
> +  IN CONST EFI_MM_MP_PROTOCOL  *This,
> +  OUT  UINTN   *NumberOfProcessors
> +);
> +
> +
> +/**
> +  This service allows the caller to invoke a procedure one of the
> +application processors (AP). This
> +  function uses an optional token parameter to support blocking and
> +non-blocking modes. If the token
> +  is passed into the call, the function will operate in a non-blocking
> +fashion and the caller can
> +  check for completion with CheckOnProcedure or WaitForProcedure.
> +
> +  @param[in] This   The EFI_MM_MP_PROTOCOL instance.
> +  @param[in] Procedure  A pointer to the procedure to be run 
> on
> the 

Re: [edk2-devel] [Patch v5 1/2] MdePkg: Add new MM MP Protocol definition.

[Dong, Eric]

Hi Liming,  Mike,

Can you help to review this patch? 

Thanks,
Eric

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Dong, Eric
> Sent: Wednesday, July 10, 2019 3:56 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Laszlo Ersek 
> Subject: [edk2-devel] [Patch v5 1/2] MdePkg: Add new MM MP Protocol
> definition.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1937
> 
> EFI MM MP Protocol is defined in the PI 1.5 specification.
> 
> The MM MP protocol provides a set of functions to allow execution of
> procedures on processors that have entered MM. This protocol has the
> following properties:
> 1. The caller can invoke execution of a procedure on a processor, other than
> the caller, that has also entered MM. Supports blocking and non-blocking
> modes of operation.
> 2. The caller can invoke a procedure on multiple processors. Supports
> blocking and non-blocking modes of operation.
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Eric Dong 
> Reviewed-by: Ray Ni 
> ---
>  MdePkg/Include/Pi/PiMultiPhase.h |  16 ++
>  MdePkg/Include/Protocol/MmMp.h   | 333
> +++
>  MdePkg/MdePkg.dec|   3 +
>  3 files changed, 352 insertions(+)
>  create mode 100644 MdePkg/Include/Protocol/MmMp.h
> 
> diff --git a/MdePkg/Include/Pi/PiMultiPhase.h
> b/MdePkg/Include/Pi/PiMultiPhase.h
> index eb12527767..a5056799e1 100644
> --- a/MdePkg/Include/Pi/PiMultiPhase.h
> +++ b/MdePkg/Include/Pi/PiMultiPhase.h
> @@ -176,4 +176,20 @@ VOID
>IN OUT VOID  *Buffer
>);
> 
> +/**
> +  The function prototype for invoking a function on an Application Processor.
> +
> +  This definition is used by the UEFI MM MP Serices Protocol.
> +
> +  @param[in] ProcedureArgumentThe pointer to private data buffer.
> +
> +  @retval EFI_SUCCESS Excutive the procedure successfully
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_AP_PROCEDURE2)(
> +  IN VOID  *ProcedureArgument
> +);
> +
>  #endif
> diff --git a/MdePkg/Include/Protocol/MmMp.h
> b/MdePkg/Include/Protocol/MmMp.h new file mode 100644 index
> 00..beace1386c
> --- /dev/null
> +++ b/MdePkg/Include/Protocol/MmMp.h
> @@ -0,0 +1,333 @@
> +/** @file
> +  EFI MM MP Protocol is defined in the PI 1.5 specification.
> +
> +  The MM MP protocol provides a set of functions to allow execution of
> + procedures on processors that  have entered MM. This protocol has the
> following properties:
> +  1. The caller can only invoke execution of a procedure on a processor,
> other than the caller, that
> + has also entered MM.
> +  2. It is possible to invoke a procedure on multiple processors. Supports
> blocking and non-blocking
> + modes of operation.
> +
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef _MM_MP_H_
> +#define _MM_MP_H_
> +
> +#include 
> +
> +#define EFI_MM_MP_PROTOCOL_GUID \
> +  { \
> +0x5d5450d7, 0x990c, 0x4180, {0xa8, 0x3, 0x8e, 0x63, 0xf0, 0x60,
> +0x83, 0x7  }  \
> +  }
> +
> +//
> +// Revision definition.
> +//
> +#define EFI_MM_MP_PROTOCOL_REVISION0x00
> +
> +//
> +// Attribute flags
> +//
> +#define EFI_MM_MP_TIMEOUT_SUPPORTED0x01
> +
> +//
> +// Completion token
> +//
> +typedef VOID* MM_COMPLETION;
> +
> +typedef struct {
> +  MM_COMPLETION  Completion;
> +  EFI_STATUS Status;
> +} MM_DISPATCH_COMPLETION_TOKEN;
> +
> +typedef struct _EFI_MM_MP_PROTOCOL  EFI_MM_MP_PROTOCOL;
> +
> +/**
> +  Service to retrieves the number of logical processor in the platform.
> +
> +  @param[in]  ThisThe EFI_MM_MP_PROTOCOL instance.
> +  @param[out] NumberOfProcessors  Pointer to the total number of logical
> processors in the system,
> +  including the BSP and all APs.
> +
> +  @retval EFI_SUCCESS The number of processors was retrieved
> successfully
> +  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_MM_GET_NUMBER_OF_PROCESSORS) (
> +  IN CONST EFI_MM_MP_PROTOCOL  *This,
> +  OUT  UINTN   *NumberOfProcessors
> +);
> +
> +
> +/**
> +  This service allows the caller to invoke a procedure one of the
> +application processors (AP). This
> +  function uses an optional token parameter to support blocking and
> +non-blocking modes. If the token
> +  is passed into the call, the function will operate in a non-blocking
> +fashion and the caller can
> +  check for completion with CheckOnProcedure or WaitForProcedure.
> +
> +  @param[in] This   The EFI_MM_MP_PROTOCOL instance.
> +  @param[in] Procedure  A pointer to the procedure to be run 
> on
> the designated target
> +AP of the system. Type 
> EFI_AP_PROCEDURE2 is defined
> below in
> +related definitions.
> +  @param[in] CpuNumber  The 

Re: [edk2-devel] [edk2-platforms Patch 10/28] Vlv2TbltDevicePkg/bld_vlv.sh: Create Vlv.ROM

[Gary Lin]

On Thu, Jul 11, 2019 at 04:52:10AM +,  Sun, Zailiang  wrote:
> Gary,
> 
> I suggest remove the first line "From: Gary Lin " from the 
> description section since you have appended the "signed-off-by" declaration.
> 
Hi Zailiang,

That is added by Mike. I'm totally fine if you remove it when pushing
the commits.

Thanks,

Gary Lin

> > -Original Message-
> > From: Kinney, Michael D
> > Sent: Thursday, July 11, 2019 3:05 AM
> > To: devel@edk2.groups.io
> > Cc: Gary Lin ; Sun, Zailiang ; Qian,
> > Yi 
> > Subject: [edk2-platforms Patch 10/28] Vlv2TbltDevicePkg/bld_vlv.sh: Create
> > Vlv.ROM
> > 
> > From: Gary Lin 
> > 
> > The scripts for PlatformCapsuleGcc.dsc need Vlv.ROM.
> > 
> > Cc: Zailiang Sun 
> > Cc: Yi Qian 
> > Cc: Michael D Kinney 
> > Signed-off-by: Gary Lin 
> > ---
> >  Platform/Intel/Vlv2TbltDevicePkg/bld_vlv.sh | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Platform/Intel/Vlv2TbltDevicePkg/bld_vlv.sh
> > b/Platform/Intel/Vlv2TbltDevicePkg/bld_vlv.sh
> > index 7fd5f44264..c68e59398a 100755
> > --- a/Platform/Intel/Vlv2TbltDevicePkg/bld_vlv.sh
> > +++ b/Platform/Intel/Vlv2TbltDevicePkg/bld_vlv.sh
> > @@ -199,6 +199,8 @@ echo Skip "Running fce..."
> > 
> > ##
> > **
> >  ## Build Capsules
> > 
> > ##
> > **
> > +cp -f
> > $WORKSPACE/Build/Vlv2TbltDevicePkg/${TARGET}_${TOOL_CHAIN_TAG}/F
> > V/VLV.fd \
> > +
> > $WORKSPACE/Build/Vlv2TbltDevicePkg/${TARGET}_${TOOL_CHAIN_TAG}/F
> > V/Vlv.ROM
> >  build -p $PLATFORM_PKG_PATH/PlatformCapsuleGcc.dsc
> > 
> >  echo
> > --
> > 2.21.0.windows.1
> 
> 
> 
> 
> 

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

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