Re: [edk2-devel] [PATCH] UefiCpuPkg/Library/MpInitLib: Remove BSP index == 0 Assumption.

[Dong, Eric]

Ray,

I applied this change to an internal desktop machine and did below tests:

1.   boot it to shell

2.   reboot the system to shell.

Thanks,
Eric
From: Ni, Ray
Sent: Wednesday, January 15, 2020 3:43 PM
To: devel@edk2.groups.io; Dong, Eric 
Cc: Laszlo Ersek 
Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/Library/MpInitLib: Remove BSP 
index == 0 Assumption.

Eric,
What unit test was done for this patch?

> -Original Message-
> From: devel@edk2.groups.io 
> mailto:devel@edk2.groups.io>> On Behalf Of Dong, Eric
> Sent: Wednesday, January 15, 2020 2:07 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray mailto:ray...@intel.com>>; Laszlo Ersek 
> mailto:ler...@redhat.com>>
> Subject: [edk2-devel] [PATCH] UefiCpuPkg/Library/MpInitLib: Remove BSP index 
> == 0 Assumption.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2392
>
>
> Current code implementation assumes BSP index is 0 at the begin.
> This code change removes this assumption. It get BSP index from
> the saved data structure if it existed.
>
> Cc: Ray Ni mailto:ray...@intel.com>>
> Cc: Laszlo Ersek mailto:ler...@redhat.com>>
> Signed-off-by: Eric Dong mailto:eric.d...@intel.com>>
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 6ec9b172b8..922c87b766 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -636,7 +636,7 @@ ApWakeupFunction (
>//   to initialize AP in InitConfig path.
>
>// NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
> points to a different IDT shared by all APs.
>
>//
>
> -  RestoreVolatileRegisters (>CpuData[0].VolatileRegisters, 
> FALSE);
>
> +  RestoreVolatileRegisters 
> (>CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
>
>InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);
>
>ApStartupSignalBuffer = 
> CpuMpData->CpuData[ProcessorNumber].StartupApSignal;
>
>
>
> @@ -1615,6 +1615,7 @@ MpInitLibInitialize (
>UINTNApResetVectorSize;
>
>UINTNBackupBufferAddr;
>
>UINTNApIdtBase;
>
> +  UINT64   BspTopOfStack;
>
>
>
>OldCpuMpData = GetCpuMpDataFromGuidedHob ();
>
>if (OldCpuMpData == NULL) {
>
> @@ -1677,7 +1678,7 @@ MpInitLibInitialize (
>CpuMpData->BackupBufferSize = ApResetVectorSize;
>
>CpuMpData->WakeupBuffer = (UINTN) -1;
>
>CpuMpData->CpuCount = 1;
>
> -  CpuMpData->BspNumber= 0;
>
> +  CpuMpData->BspNumber= OldCpuMpData != NULL ? 
> OldCpuMpData->BspNumber : 0;
>
>CpuMpData->WaitEvent= NULL;
>
>CpuMpData->SwitchBspFlag= FALSE;
>
>CpuMpData->CpuData  = (CPU_AP_DATA *) (CpuMpData + 1);
>
> @@ -1704,11 +1705,12 @@ MpInitLibInitialize (
>// Don't pass BSP's TR to APs to avoid AP init failure.
>
>//
>
>VolatileRegisters.Tr = 0;
>
> -  CopyMem (>CpuData[0].VolatileRegisters, , 
> sizeof (VolatileRegisters));
>
> +  CopyMem (>CpuData[CpuMpData->BspNumber].VolatileRegisters, 
> , sizeof
> (VolatileRegisters));
>
>//
>
>// Set BSP basic information
>
>//
>
> -  InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize);
>
> +  BspTopOfStack = CpuMpData->Buffer + (CpuMpData->BspNumber + 1) * 
> CpuMpData->CpuApStackSize;
>
> +  InitializeApData (CpuMpData, CpuMpData->BspNumber, 0, BspTopOfStack);
>
>//
>
>// Save assembly code information
>
>//
>
> --
> 2.23.0.windows.1
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
>
> View/Reply Online (#53263): https://edk2.groups.io/g/devel/message/53263
> Mute This Topic: https://groups.io/mt/69712223/1712937
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [ray...@intel.com]
> -=-=-=-=-=-=

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

View/Reply Online (#53265): https://edk2.groups.io/g/devel/message/53265
Mute This Topic: https://groups.io/mt/69712223/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/Library/MpInitLib: Remove BSP index == 0 Assumption.

[Ni, Ray]

Eric,
What unit test was done for this patch?

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Dong, Eric
> Sent: Wednesday, January 15, 2020 2:07 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Laszlo Ersek 
> Subject: [edk2-devel] [PATCH] UefiCpuPkg/Library/MpInitLib: Remove BSP index 
> == 0 Assumption.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2392
> 
> 
> Current code implementation assumes BSP index is 0 at the begin.
> This code change removes this assumption. It get BSP index from
> the saved data structure if it existed.
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 6ec9b172b8..922c87b766 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -636,7 +636,7 @@ ApWakeupFunction (
>//   to initialize AP in InitConfig path.
> 
>// NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
> points to a different IDT shared by all APs.
> 
>//
> 
> -  RestoreVolatileRegisters (>CpuData[0].VolatileRegisters, 
> FALSE);
> 
> +  RestoreVolatileRegisters 
> (>CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
> 
>InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);
> 
>ApStartupSignalBuffer = 
> CpuMpData->CpuData[ProcessorNumber].StartupApSignal;
> 
> 
> 
> @@ -1615,6 +1615,7 @@ MpInitLibInitialize (
>UINTNApResetVectorSize;
> 
>UINTNBackupBufferAddr;
> 
>UINTNApIdtBase;
> 
> +  UINT64   BspTopOfStack;
> 
> 
> 
>OldCpuMpData = GetCpuMpDataFromGuidedHob ();
> 
>if (OldCpuMpData == NULL) {
> 
> @@ -1677,7 +1678,7 @@ MpInitLibInitialize (
>CpuMpData->BackupBufferSize = ApResetVectorSize;
> 
>CpuMpData->WakeupBuffer = (UINTN) -1;
> 
>CpuMpData->CpuCount = 1;
> 
> -  CpuMpData->BspNumber= 0;
> 
> +  CpuMpData->BspNumber= OldCpuMpData != NULL ? 
> OldCpuMpData->BspNumber : 0;
> 
>CpuMpData->WaitEvent= NULL;
> 
>CpuMpData->SwitchBspFlag= FALSE;
> 
>CpuMpData->CpuData  = (CPU_AP_DATA *) (CpuMpData + 1);
> 
> @@ -1704,11 +1705,12 @@ MpInitLibInitialize (
>// Don't pass BSP's TR to APs to avoid AP init failure.
> 
>//
> 
>VolatileRegisters.Tr = 0;
> 
> -  CopyMem (>CpuData[0].VolatileRegisters, , 
> sizeof (VolatileRegisters));
> 
> +  CopyMem (>CpuData[CpuMpData->BspNumber].VolatileRegisters, 
> , sizeof
> (VolatileRegisters));
> 
>//
> 
>// Set BSP basic information
> 
>//
> 
> -  InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize);
> 
> +  BspTopOfStack = CpuMpData->Buffer + (CpuMpData->BspNumber + 1) * 
> CpuMpData->CpuApStackSize;
> 
> +  InitializeApData (CpuMpData, CpuMpData->BspNumber, 0, BspTopOfStack);
> 
>//
> 
>// Save assembly code information
> 
>//
> 
> --
> 2.23.0.windows.1
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#53263): https://edk2.groups.io/g/devel/message/53263
> Mute This Topic: https://groups.io/mt/69712223/1712937
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [ray...@intel.com]
> -=-=-=-=-=-=


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

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

[edk2-devel] [PATCH] UefiCpuPkg/Library/MpInitLib: Remove BSP index == 0 Assumption.

[Dong, Eric]

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

Current code implementation assumes BSP index is 0 at the begin.
This code change removes this assumption. It get BSP index from
the saved data structure if it existed.

Cc: Ray Ni 
Cc: Laszlo Ersek 
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 6ec9b172b8..922c87b766 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -636,7 +636,7 @@ ApWakeupFunction (
   //   to initialize AP in InitConfig path.
   // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters 
points to a different IDT shared by all APs.
   //
-  RestoreVolatileRegisters (>CpuData[0].VolatileRegisters, 
FALSE);
+  RestoreVolatileRegisters 
(>CpuData[CpuMpData->BspNumber].VolatileRegisters, FALSE);
   InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);
   ApStartupSignalBuffer = 
CpuMpData->CpuData[ProcessorNumber].StartupApSignal;
 
@@ -1615,6 +1615,7 @@ MpInitLibInitialize (
   UINTNApResetVectorSize;
   UINTNBackupBufferAddr;
   UINTNApIdtBase;
+  UINT64   BspTopOfStack;
 
   OldCpuMpData = GetCpuMpDataFromGuidedHob ();
   if (OldCpuMpData == NULL) {
@@ -1677,7 +1678,7 @@ MpInitLibInitialize (
   CpuMpData->BackupBufferSize = ApResetVectorSize;
   CpuMpData->WakeupBuffer = (UINTN) -1;
   CpuMpData->CpuCount = 1;
-  CpuMpData->BspNumber= 0;
+  CpuMpData->BspNumber= OldCpuMpData != NULL ? OldCpuMpData->BspNumber 
: 0;
   CpuMpData->WaitEvent= NULL;
   CpuMpData->SwitchBspFlag= FALSE;
   CpuMpData->CpuData  = (CPU_AP_DATA *) (CpuMpData + 1);
@@ -1704,11 +1705,12 @@ MpInitLibInitialize (
   // Don't pass BSP's TR to APs to avoid AP init failure.
   //
   VolatileRegisters.Tr = 0;
-  CopyMem (>CpuData[0].VolatileRegisters, , 
sizeof (VolatileRegisters));
+  CopyMem (>CpuData[CpuMpData->BspNumber].VolatileRegisters, 
, sizeof (VolatileRegisters));
   //
   // Set BSP basic information
   //
-  InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize);
+  BspTopOfStack = CpuMpData->Buffer + (CpuMpData->BspNumber + 1) * 
CpuMpData->CpuApStackSize;
+  InitializeApData (CpuMpData, CpuMpData->BspNumber, 0, BspTopOfStack);
   //
   // Save assembly code information
   //
-- 
2.23.0.windows.1


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

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

Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix VarErrorFlag RT cache offset calculation

[Kubacki, Michael A]

Since I don't have a strong opinion either, I won't make any changes to V1 at 
this time.

Thanks,
Michael

> -Original Message-
> From: Wang, Jian J 
> Sent: Tuesday, January 14, 2020 8:09 PM
> To: Kubacki, Michael A ;
> devel@edk2.groups.io
> Cc: Gao, Liming ; Kinney, Michael D
> ; Michael Turner
> ; Wu, Hao A 
> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
> VarErrorFlag RT cache offset calculation
> 
> Mike,
> 
> Thanks for explaining. You're right that the error is rare case and it won't
> cause big problem, and NonVolatileLastVariableOffset will be approaching
> the whole FV size after some time. I don't have strong opinion. Both work for
> me.
> 
> Regards,
> Jian
> 
> > -Original Message-
> > From: Kubacki, Michael A 
> > Sent: Wednesday, January 15, 2020 11:53 AM
> > To: Wang, Jian J ; devel@edk2.groups.io
> > Cc: Gao, Liming ; Kinney, Michael D
> > ; Michael Turner
> > ; Wu, Hao A 
> > Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
> > VarErrorFlag RT cache offset calculation
> >
> > Hi Jian,
> >
> > I considered that but these are the reasons I settled on the approach in
> patch V1.
> >
> > 1. With the variable store filled, the length of
> > mVariableModuleGlobal-
> > >NonVolatileLastVariableOffset will only marginally be a smaller value
> > >than
> > mNvVariableCache->Size (since variable writes grow the store for SPI
> > mNvVariableCache->flash wear
> > leveling). In this case, it will be ~CommonRuntimeVariableSpace which
> > is usually a major portion of the variable store size anyway.
> > 2. Since mVariableModuleGlobal->NonVolatileLastVariableOffset is a
> > global moving value that is more frequently manipulated than the fixed
> > variable store size, depending upon it increases the likelihood it
> > will be set to an invalid value somewhere else.
> > 3. This is a relatively rare case (an error condition) and the memory
> > copy is within DRAM for variable stores that are typically ~128KB - ~512KB.
> >
> > To reduce the copy size, the Offset parameter can be "(UINTN)
> > VarErrFlag -
> > (UINTN) mNvVariableCache" (just remove the unnecessary addition of
> > (UINTN)
> > mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase) with
> > mVariableModuleGlobal->size
> > "sizeof (TempFlag)". How about this in a V2?
> >
> > Thanks,
> > Michael
> >
> > > -Original Message-
> > > From: Wang, Jian J 
> > > Sent: Monday, January 13, 2020 10:43 PM
> > > To: devel@edk2.groups.io; Kubacki, Michael A
> > > 
> > > Cc: Gao, Liming ; Kinney, Michael D
> > > ; Michael Turner
> > > ; Wu, Hao A 
> > > Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
> > > VarErrorFlag RT cache offset calculation
> > >
> > > Michael,
> > >
> > > I'm not sure sync-ing whole variable cache memory is an efficient
> operation.
> > > What about using
> > > mVariableModuleGlobal->NonVolatileLastVariableOffset
> > > as Length parameter?
> > >
> > >Status =  SynchronizeRuntimeVariableCache (
> > >
> > >
> >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache,
> > >0,
> > >mVariableModuleGlobal->NonVolatileLastVariableOffset
> > >);
> > >
> > > Regards,
> > > Jian
> > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io  On Behalf Of
> > > > Kubacki, Michael A
> > > > Sent: Tuesday, January 14, 2020 7:19 AM
> > > > To: devel@edk2.groups.io
> > > > Cc: Gao, Liming ; Kinney, Michael D
> > > > ; Michael Turner
> > > > ; Wang, Jian J
> > > > ; Wu, Hao A 
> > > > Subject: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
> > > > VarErrorFlag RT cache offset calculation
> > > >
> > > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2457
> > > >
> > > > This commit fixes an offset calculation that is used to write the
> > > > VarErrorFlag UEFI variable to the UEFI variable runtime cache.
> > > >
> > > > Currently a physical address is used instead of an offset. This
> > > > commit changes the offset to zero with a length of the entire
> > > > non-volatile variable store so the entire non-volatile variable
> > > > store buffer in SMRAM (with the variable update modification) is
> > > > copied to the runtime variable cache. This follows the same
> > > > pattern used in other SynchronizeRuntimeVariableCache () calls for
> consistency.
> > > >
> > > > * Observable symptom: An exception in SMM will most likely occur
> > > >   due to the invalid memory reference when the VarErrorFlag variable
> > > >   is written. The variable is most commonly written when the UEFI
> > > >   variable store is full.
> > > >
> > > > * The issue only occurs when the variable runtime cache is enabled
> > > >   by the following PCD being set to TRUE:
> > > >   gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache
> > > >
> > > > Fixes: aab3b9b9a1e5e1f3fa966fb1667fc3e6c47e7706
> > > >
> > > > Cc: Liming Gao 
> > > > Cc: Michael D Kinney 
> > > > 

Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix VarErrorFlag RT cache offset calculation

[Wang, Jian J]

Mike,

Thanks for explaining. You're right that the error is rare case and it won't
cause big problem, and NonVolatileLastVariableOffset will be approaching
the whole FV size after some time. I don't have strong opinion. Both work
for me.

Regards,
Jian

> -Original Message-
> From: Kubacki, Michael A 
> Sent: Wednesday, January 15, 2020 11:53 AM
> To: Wang, Jian J ; devel@edk2.groups.io
> Cc: Gao, Liming ; Kinney, Michael D
> ; Michael Turner
> ; Wu, Hao A 
> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
> VarErrorFlag RT cache offset calculation
> 
> Hi Jian,
> 
> I considered that but these are the reasons I settled on the approach in 
> patch V1.
> 
> 1. With the variable store filled, the length of mVariableModuleGlobal-
> >NonVolatileLastVariableOffset will only marginally be a smaller value than
> mNvVariableCache->Size (since variable writes grow the store for SPI flash 
> wear
> leveling). In this case, it will be ~CommonRuntimeVariableSpace which is 
> usually
> a major portion of the variable store size anyway.
> 2. Since mVariableModuleGlobal->NonVolatileLastVariableOffset is a global
> moving value that is more frequently manipulated than the fixed variable store
> size, depending upon it increases the likelihood it will be set to an invalid 
> value
> somewhere else.
> 3. This is a relatively rare case (an error condition) and the memory copy is
> within DRAM for variable stores that are typically ~128KB - ~512KB.
> 
> To reduce the copy size, the Offset parameter can be "(UINTN) VarErrFlag -
> (UINTN) mNvVariableCache" (just remove the unnecessary addition of (UINTN)
> mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase) with size
> "sizeof (TempFlag)". How about this in a V2?
> 
> Thanks,
> Michael
> 
> > -Original Message-
> > From: Wang, Jian J 
> > Sent: Monday, January 13, 2020 10:43 PM
> > To: devel@edk2.groups.io; Kubacki, Michael A
> > 
> > Cc: Gao, Liming ; Kinney, Michael D
> > ; Michael Turner
> > ; Wu, Hao A 
> > Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
> > VarErrorFlag RT cache offset calculation
> >
> > Michael,
> >
> > I'm not sure sync-ing whole variable cache memory is an efficient operation.
> > What about using mVariableModuleGlobal->NonVolatileLastVariableOffset
> > as Length parameter?
> >
> >Status =  SynchronizeRuntimeVariableCache (
> >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache,
> >0,
> >mVariableModuleGlobal->NonVolatileLastVariableOffset
> >);
> >
> > Regards,
> > Jian
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of
> > > Kubacki, Michael A
> > > Sent: Tuesday, January 14, 2020 7:19 AM
> > > To: devel@edk2.groups.io
> > > Cc: Gao, Liming ; Kinney, Michael D
> > > ; Michael Turner
> > > ; Wang, Jian J ;
> > > Wu, Hao A 
> > > Subject: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
> > > VarErrorFlag RT cache offset calculation
> > >
> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2457
> > >
> > > This commit fixes an offset calculation that is used to write the
> > > VarErrorFlag UEFI variable to the UEFI variable runtime cache.
> > >
> > > Currently a physical address is used instead of an offset. This commit
> > > changes the offset to zero with a length of the entire non-volatile
> > > variable store so the entire non-volatile variable store buffer in
> > > SMRAM (with the variable update modification) is copied to the runtime
> > > variable cache. This follows the same pattern used in other
> > > SynchronizeRuntimeVariableCache () calls for consistency.
> > >
> > > * Observable symptom: An exception in SMM will most likely occur
> > >   due to the invalid memory reference when the VarErrorFlag variable
> > >   is written. The variable is most commonly written when the UEFI
> > >   variable store is full.
> > >
> > > * The issue only occurs when the variable runtime cache is enabled
> > >   by the following PCD being set to TRUE:
> > >   gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache
> > >
> > > Fixes: aab3b9b9a1e5e1f3fa966fb1667fc3e6c47e7706
> > >
> > > Cc: Liming Gao 
> > > Cc: Michael D Kinney 
> > > Cc: Michael Turner 
> > > Cc: Jian J Wang 
> > > Cc: Hao A Wu 
> > > Signed-off-by: Michael Kubacki 
> > > ---
> > >  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > > index b0ee5e50d0..d23aea4bc7 100644
> > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > > @@ -16,7 +16,7 @@
> > >VariableServiceSetVariable() should also check authenticate data to
> > > avoid buffer overflow,
> > >integer overflow. It should 

Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix VarErrorFlag RT cache offset calculation

[Kubacki, Michael A]

Hi Jian,

I considered that but these are the reasons I settled on the approach in patch 
V1.

1. With the variable store filled, the length of 
mVariableModuleGlobal->NonVolatileLastVariableOffset will only marginally be a 
smaller value than mNvVariableCache->Size (since variable writes grow the store 
for SPI flash wear leveling). In this case, it will be 
~CommonRuntimeVariableSpace which is usually a major portion of the variable 
store size anyway.
2. Since mVariableModuleGlobal->NonVolatileLastVariableOffset is a global 
moving value that is more frequently manipulated than the fixed variable store 
size, depending upon it increases the likelihood it will be set to an invalid 
value somewhere else.
3. This is a relatively rare case (an error condition) and the memory copy is 
within DRAM for variable stores that are typically ~128KB - ~512KB.

To reduce the copy size, the Offset parameter can be "(UINTN) VarErrFlag - 
(UINTN) mNvVariableCache" (just remove the unnecessary addition of (UINTN) 
mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase) with size 
"sizeof (TempFlag)". How about this in a V2?

Thanks,
Michael

> -Original Message-
> From: Wang, Jian J 
> Sent: Monday, January 13, 2020 10:43 PM
> To: devel@edk2.groups.io; Kubacki, Michael A
> 
> Cc: Gao, Liming ; Kinney, Michael D
> ; Michael Turner
> ; Wu, Hao A 
> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
> VarErrorFlag RT cache offset calculation
> 
> Michael,
> 
> I'm not sure sync-ing whole variable cache memory is an efficient operation.
> What about using mVariableModuleGlobal->NonVolatileLastVariableOffset
> as Length parameter?
> 
>Status =  SynchronizeRuntimeVariableCache (
>
> >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache,
>0,
>mVariableModuleGlobal->NonVolatileLastVariableOffset
>);
> 
> Regards,
> Jian
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> > Kubacki, Michael A
> > Sent: Tuesday, January 14, 2020 7:19 AM
> > To: devel@edk2.groups.io
> > Cc: Gao, Liming ; Kinney, Michael D
> > ; Michael Turner
> > ; Wang, Jian J ;
> > Wu, Hao A 
> > Subject: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
> > VarErrorFlag RT cache offset calculation
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2457
> >
> > This commit fixes an offset calculation that is used to write the
> > VarErrorFlag UEFI variable to the UEFI variable runtime cache.
> >
> > Currently a physical address is used instead of an offset. This commit
> > changes the offset to zero with a length of the entire non-volatile
> > variable store so the entire non-volatile variable store buffer in
> > SMRAM (with the variable update modification) is copied to the runtime
> > variable cache. This follows the same pattern used in other
> > SynchronizeRuntimeVariableCache () calls for consistency.
> >
> > * Observable symptom: An exception in SMM will most likely occur
> >   due to the invalid memory reference when the VarErrorFlag variable
> >   is written. The variable is most commonly written when the UEFI
> >   variable store is full.
> >
> > * The issue only occurs when the variable runtime cache is enabled
> >   by the following PCD being set to TRUE:
> >   gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache
> >
> > Fixes: aab3b9b9a1e5e1f3fa966fb1667fc3e6c47e7706
> >
> > Cc: Liming Gao 
> > Cc: Michael D Kinney 
> > Cc: Michael Turner 
> > Cc: Jian J Wang 
> > Cc: Hao A Wu 
> > Signed-off-by: Michael Kubacki 
> > ---
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > index b0ee5e50d0..d23aea4bc7 100644
> > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > @@ -16,7 +16,7 @@
> >VariableServiceSetVariable() should also check authenticate data to
> > avoid buffer overflow,
> >integer overflow. It should also check attribute to avoid authentication
> bypass.
> >
> > -Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > reserved.
> > +Copyright (c) 2006 - 2020, Intel Corporation. All rights
> > +reserved.
> >  (C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -335,8 +335,8 @@ RecordVarErrorFlag (
> >*VarErrFlag = TempFlag;
> >Status =  SynchronizeRuntimeVariableCache (
> >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache,
> > -  (UINTN) VarErrFlag - (UINTN) mNvVariableCache + (UINTN)
> > mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase,
> > -  sizeof (TempFlag)
> > +  0,
> > +  

Re: [edk2-devel] [PATCH v2 1/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API

[Liming Gao]

Amol:
  This is new feature. Please submit BZ (https://bugzilla.tianocore.org/) to 
track it. If this feature catches edk2 2020 Q1 stable tag, I will add it into 
edk2 feature planning. 

Thanks
Liming
-Original Message-
From: devel@edk2.groups.io  On Behalf Of Wang, Jian J
Sent: 2020年1月15日 11:14
To: Sukerkar, Amol N ; devel@edk2.groups.io
Cc: Kinney, Michael D ; Yao, Jiewen 
; Agrawal, Sachin ; Musti, 
Srinivas 
Subject: Re: [edk2-devel] [PATCH v2 1/1] SecurityPkg/BaseHashLib: Implement 
Unified Hash Calculation API

Amol,

1. Your patch doesn't support hashing more than one algorithm at the same time.
   Is this on purpose? Sorry I don't remember the conclusion in last discussion.
2. There're trailing spaces in BaseHashLibCommon.c and BashHashLibCommon.h.
   You can use BaseTools\Scripts\PatchCheck.py to check it before sending patch.

See my other comments below.

> -Original Message-
> From: Sukerkar, Amol N 
> Sent: Tuesday, January 14, 2020 11:41 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Yao, Jiewen 
> ; Wang, Jian J ; Agrawal, 
> Sachin ; Musti, Srinivas 
> 
> Subject: [PATCH v2 1/1] SecurityPkg/BaseHashLib: Implement Unified 
> Hash Calculation API
> 
> This commit introduces a Unified Hash API to calculate hash using a 
> hashing algorithm specified by the PCD, PcdSystemHashPolicy. This 
> library interfaces with the various hashing API, such as, MD4, MD5, 
> SHA1, SHA256,
> SHA512 and SM3_256 implemented in CryptoPkg. The user can calculate 
> the desired hash by setting PcdSystemHashPolicy to appropriate value.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Michael D Kinney 
> Signed-off-by: Sukerkar, Amol N 
> ---
> 
> Notes:
> v2:
> - Fixed the commit message format
> 
>  SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c | 252
> 
>  SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c| 122 ++
>  SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c| 125 ++
>  SecurityPkg/Include/Library/BaseHashLib.h   |  84 +++
>  SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h |  71 ++
> SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf  |  47 
> SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni  |  18 ++
> SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf  |  52 
> SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni  |  17 ++
>  SecurityPkg/SecurityPkg.dec |  23 +-
>  SecurityPkg/SecurityPkg.dsc |  10 +-
>  SecurityPkg/SecurityPkg.uni |  15 +-
>  12 files changed, 833 insertions(+), 3 deletions(-)
> 
> diff --git a/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
> b/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
> new file mode 100644
> index ..f8742e55b5f7
> --- /dev/null
> +++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
> @@ -0,0 +1,252 @@
> +/** @file
> 
> +  Implement image verification services for secure boot service
> 
> +
> 
> +  Caution: This file requires additional review when modified.
> 
> +  This library will have external input - PE/COFF image.
> 
> +  This external input must be validated carefully to avoid security 
> + issue like
> 
> +  buffer overflow, integer overflow.
> 
> +
> 
> +  DxeImageVerificationLibImageRead() function will make sure the 
> + PE/COFF
> image content
> 
> +  read is within the image buffer.
> 
> +
> 
> +  DxeImageVerificationHandler(), HashPeImageByType(), HashPeImage()
> function will accept
> 
> +  untrusted PE/COFF image and validate its data structure within this 
> + image
> buffer before use.
> 
> +
> 
> +Copyright (c) 2009 - 2020, Intel Corporation. All rights 
> +reserved.
> 
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
> 
> +This program and the accompanying materials
> 
> +are licensed and made available under the terms and conditions of the 
> +BSD
> License
> 
> +which accompanies this distribution.  The full text of the license 
> +may be found
> at
> 
> +http://opensource.org/licenses/bsd-license.php
> 
> +
> 
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> 
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> 
> +
> 
> +**/
> 
> +
> 
> +#include 
> 
> +#include 
> 
> +#include 
> 
> +#include 
> 
> +#include 
> 
> +#include 
> 
> +#include 
> 
> +
> 
> +/**
> 
> +  Init hash sequence with Hash Algorithm specified by HashPolicy.
> 
> +
> 
> +  @param HashPolicy  Hash Algorithm Policy.
> 
> +  @param HashHandle  Hash handle.
> 
> +
> 
> +  @retval TRUE   Hash start and HashHandle returned.
> 
> +  @retval FALSE  Hash Init unsuccessful.
> 
> +**/
> 
> +BOOLEAN
> 
> +EFIAPI
> 
> +HashInitInternal (
> 
> +  IN UINT8  HashPolicy,
> 
> +  OUT HASH_HANDLE   *HashHandle
> 
> +  )
> 
> +{
> 
> +  BOOLEAN  Status;
> 
> +  VOID *HashCtx;
> 
> +  UINTNCtxSize;
> 
> +
> 
> +  switch (HashPolicy) {
> 
> +case HASH_MD4:
> 
> +  CtxSize = 

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

[Ni, Ray]

Laszlo,
I tend to make it a platform local implementation first.
Your proposals impact a broader scope which we could
consider later if it's hard for a platform to handle the issue.

Thanks,
Ray

> -Original Message-
> From: Laszlo Ersek 
> Sent: Saturday, January 11, 2020 12:24 AM
> To: Ni, Ray ; 'devel@edk2.groups.io'
> ; 'ard.biesheu...@linaro.org'
> 
> Cc: 'Leif Lindholm' ; Gao, Zhichao
> ; Ma, Maurice ; Dong,
> Guo ; You, Benjamin 
> Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow
> consoles on drivers loaded via Driver
> 
> On 01/10/20 15:37, Ni, Ray wrote:
> > Ard,
> > I understand now that: BeforeConsole() needs to connect Gfx to get the
> GOP device path
> > for ConOut updating. But the GOP driver is specified in Driver and it
> will be loaded after
> > BeforeConsole() in today's code. This order makes the Gfx connection fails
> to start the GOP driver
> > resulting ConOut not updated.
> >
> > Moving Driver processing before BeforeConsole() helps in your case.
> But it may impact
> > other platforms: There might be platforms that update Driver
> variables in BeforeConsole()
> > and expect these drivers are processed in the same boot (not the next
> boot). I don't have the real
> > examples. But today's flow allows this. With your change, Driver will
> not be processed in the first
> > boot. The change impacts these platforms.
> >
> > One backward compatible approach can be: processing Driver before
> BeforeConsole() and processing the new Driver (added by
> BeforeConsole()) after BeforeConsole().
> >
> > But another problem arises: If BeforeConsole() removes a Driver,
> platform's expectation is that deleted Driver doesn't run. But that driver
> already runs.
> >
> > So actually the question is: when BDS core can consume the Driver
> and process them?
> > Right now, it’s the time after BeforeConsole(). Just as right now
> BeforeConsole() updates ConOut
> > in your case, BeforeConsole() is the place where platform updates all UEFI
> defined boot related
> > variables. Processing Driver before BeforeConsole() requires platform
> updates Driver
> > in other places. It's a new requirement to the platform.
> >
> > With all what I explained in above, I cannot agree with the changes.
> >
> > Another approach is:
> > Platform could connect the GFX in AfterConsole() and update the ConOut.
> Then platform could manually call EfiBootManagerConnectConsoleVariable
> (ConOut) to re-connect ConOut.
> > It's a bit ugly I agree.
> 
> Let me raise three other ideas (alternatives to each other, and to the above),
> with varying levels of annoyance. :)
> 
> 
> (1) Keep the following logic (i.e. the subject of this patch):
> 
>   //
>   // Execute Driver Options
>   //
>   LoadOptions = EfiBootManagerGetLoadOptions (,
> LoadOptionTypeDriver);
>   ProcessLoadOptions (LoadOptions, LoadOptionCount);
>   EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
> 
> in *both* places, but gate each one with a bit in a new bitmask PCD.
> 
> (Note: it's probably not the best for any platform to permit both branches, as
> driver images would be loaded twice.)
> 
> 
> (2) EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL has recently been
> added to edk2. It looks like a well-designed (extensible) protocol, for two
> reasons:
> 
> - the protocol structure has a Revision field,
> 
> - the only current member function, RefreshAllBootOptions(), is permitted to
> return EFI_UNSUPPORTED -- and the single call site, in the
> EfiBootManagerRefreshAllBootOption() function, handles that return value
> well (it does nothing).
> 
> The idea would be to bump the Revision field, and add a new member
> function. Then call this member function (if it exists) in the spot where the
> current patch proposes to move the Driver dispatch logic to.
> 
> This is almost like a new PlatformBootManagerLib interface, except it does
> not require existent lib instances to be updated.
> 
> And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL
> implementation side, RefreshAllBootOptions() would return
> EFI_UNSUPPORTED. (Because that is irrelevant to Ard's use case.)
> 
> Perhaps add yet another member function that can disable the Driver
> option processing in the current location.
> 
> 
> (3) Extend the UEFI specification, section "3.1.3 Load Options".
> 
> The LOAD_OPTION_CATEGORY bit-field is almost what we want, except it's
> specified to be ignored for Driver options. So,
> 
> - either invent the standardese to let us use LOAD_OPTION_CATEGORY for
> Driver options too, without breaking existing platforms, *or*
> - introduce a new (not too wide) distinct bitfield called
> LOAD_OPTION_DRIVER_CATEGORY.
> 
> Whichever category bitfield proves acceptable, introduce a new "driver
> category bit" in that bitfield, called
> LOAD_OPTION_DRIVER_CATEGORY_CONSOLE.
> 
> Specify that, if any Driver option has the
> LOAD_OPTION_DRIVER_CATEGORY_CONSOLE attribute set, then
> 

Re: [edk2-devel] [PATCH v2 1/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API

[Wang, Jian J]

Amol,

1. Your patch doesn't support hashing more than one algorithm at the same time.
   Is this on purpose? Sorry I don't remember the conclusion in last discussion.
2. There're trailing spaces in BaseHashLibCommon.c and BashHashLibCommon.h.
   You can use BaseTools\Scripts\PatchCheck.py to check it before sending patch.

See my other comments below.

> -Original Message-
> From: Sukerkar, Amol N 
> Sent: Tuesday, January 14, 2020 11:41 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Yao, Jiewen
> ; Wang, Jian J ; Agrawal,
> Sachin ; Musti, Srinivas 
> Subject: [PATCH v2 1/1] SecurityPkg/BaseHashLib: Implement Unified Hash
> Calculation API
> 
> This commit introduces a Unified Hash API to calculate hash using a
> hashing algorithm specified by the PCD, PcdSystemHashPolicy. This library
> interfaces with the various hashing API, such as, MD4, MD5, SHA1, SHA256,
> SHA512 and SM3_256 implemented in CryptoPkg. The user can calculate the
> desired hash by setting PcdSystemHashPolicy to appropriate value.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Michael D Kinney 
> Signed-off-by: Sukerkar, Amol N 
> ---
> 
> Notes:
> v2:
> - Fixed the commit message format
> 
>  SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c | 252
> 
>  SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c| 122 ++
>  SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c| 125 ++
>  SecurityPkg/Include/Library/BaseHashLib.h   |  84 +++
>  SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h |  71 ++
>  SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf  |  47 
>  SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni  |  18 ++
>  SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf  |  52 
>  SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni  |  17 ++
>  SecurityPkg/SecurityPkg.dec |  23 +-
>  SecurityPkg/SecurityPkg.dsc |  10 +-
>  SecurityPkg/SecurityPkg.uni |  15 +-
>  12 files changed, 833 insertions(+), 3 deletions(-)
> 
> diff --git a/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
> b/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
> new file mode 100644
> index ..f8742e55b5f7
> --- /dev/null
> +++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
> @@ -0,0 +1,252 @@
> +/** @file
> 
> +  Implement image verification services for secure boot service
> 
> +
> 
> +  Caution: This file requires additional review when modified.
> 
> +  This library will have external input - PE/COFF image.
> 
> +  This external input must be validated carefully to avoid security issue 
> like
> 
> +  buffer overflow, integer overflow.
> 
> +
> 
> +  DxeImageVerificationLibImageRead() function will make sure the PE/COFF
> image content
> 
> +  read is within the image buffer.
> 
> +
> 
> +  DxeImageVerificationHandler(), HashPeImageByType(), HashPeImage()
> function will accept
> 
> +  untrusted PE/COFF image and validate its data structure within this image
> buffer before use.
> 
> +
> 
> +Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.
> 
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
> 
> +This program and the accompanying materials
> 
> +are licensed and made available under the terms and conditions of the BSD
> License
> 
> +which accompanies this distribution.  The full text of the license may be 
> found
> at
> 
> +http://opensource.org/licenses/bsd-license.php
> 
> +
> 
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> 
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> 
> +
> 
> +**/
> 
> +
> 
> +#include 
> 
> +#include 
> 
> +#include 
> 
> +#include 
> 
> +#include 
> 
> +#include 
> 
> +#include 
> 
> +
> 
> +/**
> 
> +  Init hash sequence with Hash Algorithm specified by HashPolicy.
> 
> +
> 
> +  @param HashPolicy  Hash Algorithm Policy.
> 
> +  @param HashHandle  Hash handle.
> 
> +
> 
> +  @retval TRUE   Hash start and HashHandle returned.
> 
> +  @retval FALSE  Hash Init unsuccessful.
> 
> +**/
> 
> +BOOLEAN
> 
> +EFIAPI
> 
> +HashInitInternal (
> 
> +  IN UINT8  HashPolicy,
> 
> +  OUT HASH_HANDLE   *HashHandle
> 
> +  )
> 
> +{
> 
> +  BOOLEAN  Status;
> 
> +  VOID *HashCtx;
> 
> +  UINTNCtxSize;
> 
> +
> 
> +  switch (HashPolicy) {
> 
> +case HASH_MD4:
> 
> +  CtxSize = Md4GetContextSize ();
> 
> +  HashCtx = AllocatePool (CtxSize);
> 
> +  ASSERT (HashCtx != NULL);
> 
> +
> 
> +  Status = Md4Init (HashCtx);
> 
> +  break;
> 
> +
> 
> +case HASH_MD5:
> 
> +  CtxSize = Md5GetContextSize ();
> 
> +  HashCtx = AllocatePool (CtxSize);
> 
> +  ASSERT (HashCtx != NULL);
> 
> +
> 
> + Status = Md5Init (HashCtx);
> 
> +  break;
> 
> +
> 
> +case HASH_SHA1:
> 
> +  CtxSize = Sha1GetContextSize ();
> 
> +  HashCtx = AllocatePool (CtxSize);
> 
> +  ASSERT (HashCtx != NULL);
> 
> +
> 
> +

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

[Ni, Ray]

> -Original Message-
> From: af...@apple.com 
> Sent: Wednesday, January 15, 2020 1:45 AM
> To: devel@edk2.groups.io; Ard Biesheuvel 
> Cc: Laszlo Ersek ; Ni, Ray ; Leif
> Lindholm ; Gao, Zhichao ;
> Ma, Maurice ; Dong, Guo ;
> You, Benjamin 
> Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow
> consoles on drivers loaded via Driver
> 
> 
> 
> > On Jan 14, 2020, at 8:46 AM, Ard Biesheuvel 
> wrote:
> >
> > On Mon, 13 Jan 2020 at 18:57, Andrew Fish via Groups.Io
> >  wrote:
> >>
> >> Ard,
> >>
> >> Is the problem GFX console? Would it be possible to have a PCD to
> assume graphics console, and if non was found on the boot connect those
> PCI devices and update the NVRAM to cause a console to connect. You might
> have to do a 2nd connect on the GOP handle after the nvram variable was
> written to make the ConSpliter see it?
> >>
> >
> > I'm not sure I follow. Do you mean update the console variable if it
> > doesn't contain the GOP produced by the Driver option and reboot?
> >
> 
> Ard,
> 
> I was thinking this specific case was no active GOP driver in the system due 
> to
> the emulation of the ROM. I was saying there could be a platform policy to
> require GOP, and there could be a  platform hook point to check for no GOP
> and take action. This is not really a Driver path, but a fallback path 
> for no
> GOP. So that is kind of cheating :).
> 
> Also I was thinking the ConSpliter would activate the new GOP if you do a
> 2nd gBS->ConnectController on the Graphics PCI or GOP handle after
> updating the console nvram variable.
> 
> Sorry I'm not really familiar with the modern TianoCore BDS, as we have a
> custom BDS on Macs, so I'm talking more hypothetically about how BDS could
> work.

Andrew,
TianoCore BDS in this part is very simple. The BDS core expects a platform
hook (PlatformBootManagerBeforeConsole()) to initialize every BDS related UEFI
variables like ConIn, ConOut, Driver, Boot, DriverOrder, BootOrder, etc.
Then BDS core consumes these variables and acts accordingly.
I think the "platform hook" you suggested is similar to what I suggested 
earlier,
except that you think that putting GOP in Driver is unnecessary.

Ard,
Assuming you've put GOP driver path to Driver,
you could put below code in the beginning of PlatformBootManagerAfterConsole():
1. Find the GFX controller handle
2. EfiBootManagerConnectVideoController (GFXHandle)

Thanks,
Ray


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

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

Re: [edk2-devel] UnitTests + Rust = <3

[Yao, Jiewen]

HI Bret
That is awesome.
I am also excited to see that.

I really like the rust unit test idea because the framework is embedded in the 
language and compiler.
That is perfect example to demonstrate the value of rust for EDKII. ☺

I have a quick look at the code. Here is my thought:
1) about the "win64" usage.
I think this may break the compilation with other architecture such as IA32, 
and Linux environment.
I am not sure which target you use. If you use x86_64-unknown-uefi, you may 
remove this flag.

2) about integration test.
Besides unit test, we can also create the “tests” dir for integration test for 
more complicated function call.
That is also supported by rust language.

Thank you
Yao Jiewen

From: devel@edk2.groups.io  On Behalf Of Bret Barkelew 
via Groups.Io
Sent: Wednesday, January 15, 2020 9:56 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] UnitTests + Rust = <3

What’s better than UnitTests almost being ready for deployment in TianoCore?
How about using those UnitTests to validate a native Rust port of one of our 
VarCheck libs?

Building upon Jiewen’s work (among others) I’ve finally managed to prototype a 
port of the UefiVariablePolicyLib to Rust, and have shown that it can build as 
part of our normal CI process and run the same UnitTests that are used for the 
C version…
https://github.com/corthon/edk2-staging/tree/rust_and_tests

There is a Readme.md in the root directory with a “Notes for this branch” 
section. All of those steps are important to get Rust set up correctly.
You can run a CI build of MdeModulePkg with the “-t NOOPT” target and it will 
build the UnitTests. It will build one version against the C library, and 
another against the Rust library.

You can also run native Rust test cases by going to 
https://github.com/corthon/edk2-staging/tree/rust_and_tests/MdeModulePkg/Library/UefiVariablePolicyLibRust
And running “cargo test”.

I acknowledge that this code is a little rough. It’s definitely a prototype, 
but one that I’m excited about and thing that it can be used as a pattern for 
interop with other core libraries.
I even found a couple small bugs in the original library through the process of 
porting to Rust, since Rust is so pedantic about covering all possible cases.

Anyway, super interested in any feedback or advice for improvement. 

- Bret

PS. Thanks again, Jiewen, for laying the groundwork. I’ve seen several of your 
contributions to Rust and the UEFI prototype while working on this.


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

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

Re: [edk2-devel] [PATCH v1] BaseTools/Capsule: Add capsule dependency support

[Bob Feng]

Reviewed-by: Bob Feng 

-Original Message-
From: Li, Aaron 
Sent: Friday, January 10, 2020 9:58 AM
To: devel@edk2.groups.io
Cc: Feng, Bob C ; Gao, Liming 
Subject: [PATCH v1] BaseTools/Capsule: Add capsule dependency support

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

Capsule generate tool support encode capsule dependencies through '-j'
command with a JSON file. To enable dependency feature, "Dependencies"
field for each payload in JSON file is required.
The value of "Dependencies" field is C style infix notation expression.
For example:
  "Dependencies":"72E2945A-00DA-448E-9AA7-075AD840F9D4 > 0x0001"

The relation of Dependency Expression Opcode in UEFI2.8 chap 23.2 and infix 
notation expression value is as follows:
+-+--+
| OPCODE  | INFIX EXPRESSION VALUE   |
+-+--+
| 0x00 (PUSH_GUID)| {GUID}   |
| 0x01 (PUSH_VERSION) | {UINT32} |
| 0x02 (DECLEAR_VERSION_NAME} | DECLEAR "{VERSION_NAME}" |
| 0x03 (AND)  | &&   |
| 0x04 (OR)   | ||   |
| 0x05 (NOT)  | ~|
| 0x06 (TRUE) | TRUE |
| 0x07 (FALSE)| FALSE|
| 0x08 (EQ)   | ==   |
| 0x09 (GT)   | >|
| 0x0A (GTE)  | >=   |
| 0x0B (LT)   | <|
| 0x0C (LTE)  | <=   |
+-+--+

Cc: Bob Feng 
Cc: Liming Gao 
Signed-off-by: Aaron Li 
---
 BaseTools/Source/Python/Capsule/GenerateCapsule.py   |  62 ++-
 BaseTools/Source/Python/Common/Uefi/Capsule/CapsuleDependency.py | 409 

 2 files changed, 464 insertions(+), 7 deletions(-)

diff --git a/BaseTools/Source/Python/Capsule/GenerateCapsule.py 
b/BaseTools/Source/Python/Capsule/GenerateCapsule.py
index 6838beb682..a8de988253 100644
--- a/BaseTools/Source/Python/Capsule/GenerateCapsule.py
+++ b/BaseTools/Source/Python/Capsule/GenerateCapsule.py
@@ -31,6 +31,7 @@ import json
 from Common.Uefi.Capsule.UefiCapsuleHeader import UefiCapsuleHeaderClass  from 
Common.Uefi.Capsule.FmpCapsuleHeader  import FmpCapsuleHeaderClass
 from Common.Uefi.Capsule.FmpAuthHeader import FmpAuthHeaderClass
+from Common.Uefi.Capsule.CapsuleDependency import 
+CapsuleDependencyClass
 from Common.Edk2.Capsule.FmpPayloadHeader  import FmpPayloadHeaderClass
 
 #
@@ -306,6 +307,7 @@ if __name__ == '__main__':
 OpenSslOtherPublicCertFile   = ConvertJsonValue (Config, 
'OpenSslOtherPublicCertFile', os.path.expandvars, Required = False, Default = 
None, Open = True)
 OpenSslTrustedPublicCertFile = ConvertJsonValue (Config, 
'OpenSslTrustedPublicCertFile', os.path.expandvars, Required = False, Default = 
None, Open = True)
 SigningToolPath  = ConvertJsonValue (Config, 
'SigningToolPath', os.path.expandvars, Required = False, Default = None)
+DepexExp = ConvertJsonValue (Config, 
'Dependencies', str, Required = False, Default = None)
 
 #
 # Read binary input file
@@ -330,7 +332,8 @@ if __name__ == '__main__':
 OpenSslSignerPrivateCertFile,
 OpenSslOtherPublicCertFile,
 OpenSslTrustedPublicCertFile,
-SigningToolPath
+SigningToolPath,
+DepexExp
 ))
 
 def GenerateOutputJson (PayloadJsonDescriptorList):
@@ -348,7 +351,8 @@ if __name__ == '__main__':
   "OpenSslSignerPrivateCertFile": 
str(PayloadDescriptor.OpenSslSignerPrivateCertFile),
   "OpenSslOtherPublicCertFile": 
str(PayloadDescriptor.OpenSslOtherPublicCertFile),
   "OpenSslTrustedPublicCertFile": 
str(PayloadDescriptor.OpenSslTrustedPublicCertFile),
-  "SigningToolPath": 
str(PayloadDescriptor.SigningToolPath)
+  "SigningToolPath": 
str(PayloadDescriptor.SigningToolPath),
+  "Dependencies" : 
+ str(PayloadDescriptor.DepexExp)
   }for PayloadDescriptor in 
PayloadJsonDescriptorList
   ]
   }
@@ -424,7 +428,8 @@ if __name__ == '__main__':
  OpenSslSignerPrivateCertFile = None,
  OpenSslOtherPublicCertFile   = None,
  

[edk2-devel] UnitTests + Rust = <3

[Bret Barkelew via Groups.Io]

What’s better than UnitTests almost being ready for deployment in TianoCore?
How about using those UnitTests to validate a native Rust port of one of our 
VarCheck libs?

Building upon Jiewen’s work (among others) I’ve finally managed to prototype a 
port of the UefiVariablePolicyLib to Rust, and have shown that it can build as 
part of our normal CI process and run the same UnitTests that are used for the 
C version…
https://github.com/corthon/edk2-staging/tree/rust_and_tests

There is a Readme.md in the root directory with a “Notes for this branch” 
section. All of those steps are important to get Rust set up correctly.
You can run a CI build of MdeModulePkg with the “-t NOOPT” target and it will 
build the UnitTests. It will build one version against the C library, and 
another against the Rust library.

You can also run native Rust test cases by going to 
https://github.com/corthon/edk2-staging/tree/rust_and_tests/MdeModulePkg/Library/UefiVariablePolicyLibRust
And running “cargo test”.

I acknowledge that this code is a little rough. It’s definitely a prototype, 
but one that I’m excited about and thing that it can be used as a pattern for 
interop with other core libraries.
I even found a couple small bugs in the original library through the process of 
porting to Rust, since Rust is so pedantic about covering all possible cases.

Anyway, super interested in any feedback or advice for improvement. 

- Bret

PS. Thanks again, Jiewen, for laying the groundwork. I’ve seen several of your 
contributions to Rust and the UEFI prototype while working on this.

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

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

Re: [edk2-devel] [PATCHv2 0/4] MdeModulePkg/SdMmcPciHcDxe: Retry the commands that failed due to CRC error

[Wu, Hao A]

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Albecki, Mateusz
> Sent: Tuesday, January 14, 2020 8:05 PM
> To: devel@edk2.groups.io
> Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao, Liming
> Subject: [edk2-devel] [PATCHv2 0/4] MdeModulePkg/SdMmcPciHcDxe:
> Retry the commands that failed due to CRC error
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1140
> 
> Some of the boards report that just after we change the clock frequency to
> 200MHz link is unable to stabilize fast enough and when driver sends the
> CMD13 it will often fail randomly with CRC error. To protect against this kind
> of random failures this patch series will make the driver retry the commands
> that failed due to random CRC errors.
> 
> Since async code has not yet been tested it has been put into separate patch.
> That patch is not needed to solve most pressing CMD13 issues.
> 
> changes in v2:
> -Split first patch into bugfix and refactor
> -Normal interrupt status register will now only be read once during
> SdMmcCheckTrbResult
> -We will no longer clear the data transfer timeout error in
> SdMmcCheckAndRecoverErrors. Instead we will immedieatly return for such
> case and register reset will be done in next SdMmcExecTrb
> 
> Tets performed:
> -Boot eMMC in HS400
> -Boot eMMC in HS400 with simulated CRC error on every first CMD13


For the series,
Reviewed-by: Hao A Wu 

I will wait a couple of days to see if there are comments from other reviewers
before pushing the series.

Best Regards,
Hao Wu


> 
> Cc: Hao A Wu 
> Cc: Marcin Wojtas 
> Cc: Zhichao Gao 
> Cc: Liming Gao 
> 
> Signed-off-by: Mateusz Albecki 
> 
> 
> Mateusz Albecki (4):
>   MdeModulePkg/SdMmcPciHcDxe: Fix DAT lane SW reset
>   MdeModulePkg/SdMmcPciHcDxe: Refactor command error detection
>   MdeModulePkg/SdMmcPciHcDxe: Add retries for sync commands
>   MdeModulePkg/SdMmcPciHcDxe: Add retries for async commands
> 
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  89 ++--
> -
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   5 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 218
> ++---
>  3 files changed, 204 insertions(+), 108 deletions(-)
> 
> --
> 2.14.1.windows.1
> 
> 
> 
> Intel Technology Poland sp. z o.o.
> ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII
> Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-
> 07-52-316 | Kapital zakladowy 200.000 PLN.
> 
> Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego
> adresata i moze zawierac informacje poufne. W razie przypadkowego
> otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale
> jej usuniecie; jakiekolwiek
> przegladanie lub rozpowszechnianie jest zabronione.
> This e-mail and any attachments may contain confidential material for the
> sole use of the intended recipient(s). If you are not the intended recipient,
> please contact the sender and delete all copies; any review or distribution by
> others is strictly prohibited.
> 
> 
> 


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

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

Re: [edk2-devel] [patch] MdeModulePkg/Setup: Update opcode number variable type to UINTN

[Dong, Eric]

Reviewed-by: Eric Dong 

> -Original Message-
> From: Bi, Dandan
> Sent: Tuesday, January 14, 2020 4:57 PM
> To: devel@edk2.groups.io
> Cc: Haug, Brian R ; Gao, Liming
> ; Dong, Eric 
> Subject: [patch] MdeModulePkg/Setup: Update opcode number variable
> type to UINTN
> 
> From: Brian R Haug 
> 
> Update data type of variables which save the opcode numbers to UINTN, in
> case some configuration module has lots of configuration items.
> 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Signed-off-by: Brian R Haug 
> Reviewed-by: Dandan Bi 
> ---
>  .../Universal/SetupBrowserDxe/IfrParse.c   | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> index 891b95cf9f..edb6a0fc4c 100644
> --- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> @@ -1,17 +1,17 @@
>  /** @file
>  Parser for IFR binary encoding.
> 
> -Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
> +Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
>  #include "Setup.h"
> 
> -UINT16   mStatementIndex;
> -UINT16   mExpressionOpCodeIndex;
> +UINTNmStatementIndex;
> +UINTNmExpressionOpCodeIndex;
>  EFI_QUESTION_ID  mUsedQuestionId;
>  extern LIST_ENTRY  gBrowserStorageList;
>  /**
>Initialize Statement header members.
> 
> @@ -1104,16 +1104,16 @@ IsUnKnownOpCode (
> 
>  **/
>  VOID
>  CountOpCodes (
>IN  FORM_BROWSER_FORMSET  *FormSet,
> -  IN OUT  UINT16*NumberOfStatement,
> -  IN OUT  UINT16*NumberOfExpression
> +  OUT  UINTN *NumberOfStatement,
> +  OUT  UINTN *NumberOfExpression
>)
>  {
> -  UINT16  StatementCount;
> -  UINT16  ExpressionCount;
> +  UINTN   StatementCount;
> +  UINTN   ExpressionCount;
>UINT8   *OpCodeData;
>UINTN   Offset;
>UINTN   OpCodeLen;
> 
>Offset = 0;
> @@ -1167,12 +1167,12 @@ ParseOpCodes (
>FORMSET_STORAGE *Storage;
>FORMSET_DEFAULTSTORE*DefaultStore;
>QUESTION_DEFAULT*CurrentDefault;
>QUESTION_OPTION *CurrentOption;
>UINT8   Width;
> -  UINT16  NumberOfStatement;
> -  UINT16  NumberOfExpression;
> +  UINTN   NumberOfStatement;
> +  UINTN   NumberOfExpression;
>EFI_IMAGE_ID*ImageId;
>BOOLEAN SuppressForQuestion;
>BOOLEAN SuppressForOption;
>UINT16  DepthOfDisable;
>BOOLEAN OpCodeDisabled;
> --
> 2.18.0.windows.1


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

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

Re: [edk2-devel] [PATCH edk2/eck2-platforms 0/2] Update email address for Leif Lindholm

[Leif Lindholm]

On Tue, Jan 14, 2020 at 16:32:53 +, Leif Lindholm wrote:
> I changed jobs at the turn of the year and now work for NUVIA
> (https://nuviainc.com/). My employer would like me to continue working
> on TianoCore, so let's just update the address to my current one.
> 
> I have Cc:d anyone I share a section in a Maintainers.txt with, but I
> don't think I need Reviewed-by:s from everyone - only one of the stewards
> for edk2 and I guess Mike for edk2-platforms.
> 
> Many thanks to Linaro, who are letting me keep that address through the
> transition phase. But that address will become non-functional in a couple
> of weeks or so.
> 
> Leif Lindholm (2):
>   Maintainers.txt: update email address for Leif Lindholm
>   Maintainers.txt: update email address for Leif Lindholm
> 
>  Maintainers.txt | 14 +++---
>  Maintainers.txt | 24 -
>  2 files changed, 19 insertions(+), 19 deletions(-)

For the series:
Reviewed-by: Leif Lindholm 

> Cc: Andrew Fish 
> Cc: Andy Hayes 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Cc: Marcin Wojtas 
> Cc: Michael D Kinney 
> Cc: Pete Batard 
> Cc: Ray Ni 
> Cc: Zhichao Gao 
> 
> -- 
> 2.20.1
> 

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

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

Re: [edk2-devel] [PATCH 0/2] some ARM fixes

[Ard Biesheuvel]

On Tue, 14 Jan 2020 at 17:43, Ard Biesheuvel  wrote:
>
> On Tue, 7 Jan 2020 at 10:22, Ard Biesheuvel  wrote:
> >
> > Some preparatory fixes for TPM measured boot on ARM systems.
> >
> > Patch #1 removes EnterS3WithImmediateWake() with the associated support
> > code from ArmSmcPsciResetSystemLib. EnterS3WithImmediateWake () is no
> > longer being called anywhere, and will be removed from ResetSystemLib,
> > and the support code dependencies on DXE facilities are preventing this
> > library from being used in the PEI phase, which is needed for TPM support.
> >
> > Patch #2 adds the missing call to enable VFP on PrePeiCore based platforms.
> > This is causing TPM crypto code to blow up.
> >
> > Ard Biesheuvel (2):
> >   ArmPkg/ArmSmcPsciResetSystemLib: remove EnterS3WithImmediateWake ()
> >   ArmPlatformPkg/PrePeiCore: enable VFP at startup
> >
>
> Ping?
>

Merged, thanks.

>
> >  ArmPkg/ArmPkg.dec|  4 
> > --
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 17 
> > -
> >  ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf   |  1 +
> >  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf  |  1 +
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c   | 66 
> > +---
> >  ArmPlatformPkg/PrePeiCore/PrePeiCore.c   |  5 
> > ++
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.S  | 24 
> > ---
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.asm| 29 
> > -
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.S  | 23 
> > ---
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.asm| 28 
> > -
> >  10 files changed, 9 insertions(+), 189 deletions(-)
> >  delete mode 100644 ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.S
> >  delete mode 100644 
> > ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.asm
> >  delete mode 100644 ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.S
> >  delete mode 100644 ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.asm
> >
> > --
> > 2.20.1
> >

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

View/Reply Online (#53248): https://edk2.groups.io/g/devel/message/53248
Mute This Topic: https://groups.io/mt/69498789/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/2] Maintainers.txt: update email address for Leif Lindholm

[Laszlo Ersek]

CC Phil

On 01/14/20 17:32, Leif Lindholm wrote:
> Leif now works at NUVIA Inc, update email address accordingly.
> 
> Cc: Andrew Fish 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Cc: Michael D Kinney 
> Cc: Ray Ni 
> Cc: Zhichao Gao 
> Cc: Leif Lindholm 
> Signed-off-by: Leif Lindholm 
> ---
>  Maintainers.txt | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)

For this patch:

Reviewed-by: Laszlo Ersek 

Phil, would it make sense for us to ask Leif to post an update to
".mailmap"?

For example, what happens if we run "git shortlog" over a period that
contains patches authored by *both* of Leif's email addresses? Would
those entries be merged into a single block? Would such a merging be
desirable (that's for Leif to decide / propose)?

Right now, Leif is not listed in ".mailmap" -- probably because there is
exactly one email address associated with his authorship, all across the
project history. But that's what's changing now, isn't it?

Thanks,
Laszlo



> diff --git a/Maintainers.txt b/Maintainers.txt
> index 146d8aca93f0..ca9da2892534 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -70,7 +70,7 @@ Tianocore Stewards
>  F: *
>  M: Andrew Fish 
>  M: Laszlo Ersek 
> -M: Leif Lindholm 
> +M: Leif Lindholm 
>  M: Michael D Kinney 
>  
>  Responsible Disclosure, Reporting Security Issues
> @@ -87,7 +87,7 @@ UEFI Shell Binaries (ShellBinPkg.zip) from EDK II Releases:
>  W: https://github.com/tianocore/edk2/releases/
>  M: Ray Ni   (Ia32/X64)
>  M: Zhichao Gao (Ia32/X64)
> -M: Leif Lindholm(ARM/AArch64)
> +M: Leif Lindholm   (ARM/AArch64)
>  M: Ard Biesheuvel  (ARM/AArch64)
>  
>  EDK II Architectures:
> @@ -95,7 +95,7 @@ EDK II Architectures:
>  ARM, AARCH64
>  F: */AArch64/
>  F: */Arm/
> -M: Leif Lindholm 
> +M: Leif Lindholm 
>  M: Ard Biesheuvel 
>  
>  EDK II Continuous Integration:
> @@ -126,13 +126,13 @@ EDK II Packages:
>  ArmPkg
>  F: ArmPkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/ArmPkg
> -M: Leif Lindholm 
> +M: Leif Lindholm 
>  M: Ard Biesheuvel 
>  
>  ArmPlatformPkg
>  F: ArmPlatformPkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/ArmPlatformPkg
> -M: Leif Lindholm 
> +M: Leif Lindholm 
>  M: Ard Biesheuvel 
>  
>  ArmVirtPkg
> @@ -140,7 +140,7 @@ F: ArmVirtPkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/ArmVirtPkg
>  M: Laszlo Ersek 
>  M: Ard Biesheuvel 
> -R: Leif Lindholm 
> +R: Leif Lindholm 
>  
>  ArmVirtPkg: modules used on Xen
>  F: ArmVirtPkg/ArmVirtXen.*
> @@ -173,7 +173,7 @@ M: Alexei Fedorov 
>  EmbeddedPkg
>  F: EmbeddedPkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/EmbeddedPkg
> -M: Leif Lindholm 
> +M: Leif Lindholm 
>  M: Ard Biesheuvel 
>  
>  EmulatorPkg
> 


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

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

Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH V2] EdkRepo: build_linux_installer.py path fixes

[Desimone, Ashley E]

Reviewed-by: Ashley DeSimone 

-Original Message-
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Desimone, 
Nathaniel L
Sent: Thursday, January 9, 2020 5:04 PM
To: devel@edk2.groups.io
Cc: Desimone, Ashley E ; Pandya, Puja 
; Bjorge, Erik C ; Bret 
Barkelew ; Philippe Mathieu-Daudé 

Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH V2] EdkRepo: 
build_linux_installer.py path fixes

Make path handling in build_linux_installer.py more platform agnostic

Cc: Ashley DeSimone 
Cc: Puja Pandya 
Cc: Erik Bjorge 
Cc: Bret Barkelew 
Cc: Philippe Mathieu-Daudé 
Signed-off-by: Nate DeSimone 
---
 build-scripts/build_linux_installer.py | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/build-scripts/build_linux_installer.py 
b/build-scripts/build_linux_installer.py
index 84ccf79..4501dec 100755
--- a/build-scripts/build_linux_installer.py
+++ b/build-scripts/build_linux_installer.py
@@ -3,7 +3,7 @@
 ## @file
 # build_linux_installer.py
 #
-# Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
+# Copyright (c) 2018 - 2020, Intel Corporation. All rights 
+reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent  #

@@ -21,7 +21,7 @@ def main():
 os.environ['BUILD_NUMBER'] = args.build

 # Step 1: Create required directory structure
-dist_root = os.path.abspath('../dist/self_extract')
+dist_root = os.path.abspath(os.path.join('..', 'dist', 
+ 'self_extract'))
 if not os.path.isdir(os.path.join(dist_root, 'wheels')):
 os.makedirs(os.path.join(dist_root, 'wheels'))
 if not os.path.isdir(os.path.join(dist_root, 'config')):
@@ -35,7 +35,7 @@ def main():
 return 1

 # Step 3: Copy required files
-inst_root = os.path.abspath('../edkrepo_installer')
+inst_root = os.path.abspath(os.path.join('..', 
+ 'edkrepo_installer'))
 ven_root = os.path.join(inst_root, 'Vendor')
 linux_root = os.path.join(inst_root, 'linux-scripts')
 try:
@@ -55,7 +55,7 @@ def main():

 # Step 4: Package installer files
 try:
-subprocess.run('./final_copy.py', check=True)
+subprocess.run('final_copy.py', check=True)
 except:
 print('Failed to generate installer package')
 return 1
@@ -65,7 +65,7 @@ def main():
 shutil.rmtree(dist_root, ignore_errors=True)
 except:
 print('Failed to remove temporary files')
-os.unlink('./final_copy.py')
+os.unlink('final_copy.py')

 return 0

--
2.20.1





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

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

Re: [edk2-devel] [PATCH edk2/eck2-platforms 0/2] Update email address for Leif Lindholm

[Philippe Mathieu-Daudé]

On 1/14/20 5:37 PM, Ard Biesheuvel wrote:

On Tue, 14 Jan 2020 at 17:32, Leif Lindholm  wrote:


I changed jobs at the turn of the year and now work for NUVIA
(https://nuviainc.com/). My employer would like me to continue working
on TianoCore, so let's just update the address to my current one.

I have Cc:d anyone I share a section in a Maintainers.txt with, but I
don't think I need Reviewed-by:s from everyone - only one of the stewards
for edk2 and I guess Mike for edk2-platforms.

Many thanks to Linaro, who are letting me keep that address through the
transition phase. But that address will become non-functional in a couple
of weeks or so.

Leif Lindholm (2):
   Maintainers.txt: update email address for Leif Lindholm
   Maintainers.txt: update email address for Leif Lindholm



Reviewed-by: Ard Biesheuvel 


Reviewed-by: Philippe Mathieu-Daude 
Tested-by: Philippe Mathieu-Daude 




  Maintainers.txt | 14 +++---
  Maintainers.txt | 24 -
  2 files changed, 19 insertions(+), 19 deletions(-)

Cc: Andrew Fish 
Cc: Andy Hayes 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Marcin Wojtas 
Cc: Michael D Kinney 
Cc: Pete Batard 
Cc: Ray Ni 
Cc: Zhichao Gao 

--
2.20.1








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

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

[edk2-devel] [PATCH v2 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

From: Pierre Gondois 

The dependencies for C files are satisfied by the build system.
However, there are use cases where source files with different
languages are inter-dependent. The EDKII build framework
currently doesn't have options to specify such dependencies.
E.g. It may be necessary to specify the build order between
 ASL files and C files. The use case being that the AML
 blob generated by compiling the ASL files is loaded and
 parsed by some C code.

This patch allows to describe dependencies between files listed
in the '[Sources]' section of '.inf' files. The list of source
files to build prior are colon separated (':'), e.g.:
[Sources]
  FileName1.X
  FileName2.Y : FileName1.X
  FileName3.Z : FileName1.X : FileName3.Z

In the example above:
  * FileName1.X will be built prior to FileName2.Y.
  * FileName1.X and FileName2.Y will be built prior to
FileName3.Z.

This does not affect the file specific build options,
which are pipe separated, e.g.:
  FileName2.Y | GCC : FileName1.X

The list of colon separated files must be part of the module,
i.e. they must be present in the [Sources] section.

Their file extension must be described in the
BaseTools/Conf/build_rule.template file, and generate
an output file, i.e. have a non-empty '' section.

Declaring a dependency in the [Sources] sections leads to
the creation of makefile rules between these files in the
build. The makefile rule is between the generated files.
E.g.:
  FileName1.X generates FileName1.objx
  FileName2.Y generates FileName2.objy
  Then
FileName2.Y : FileName1.X
  will create the following makefile rule:
FileName2.objy : FileName1.objx

INF Specification updates:
s3.2.1, "Common Definitions":
   ::= ":"
 ::=   

s2.5, "[Sources] Section":
  To describe a build order dependency between source
  files, specify the source files to build prior by
  listing them, colon separated.
   ::=  [] []*
   ::=  [FileNameDependency]

Signed-off-by: Pierre Gondois 
---

The changes can be seen at 
https://github.com/PierreARM/edk2/tree/676_build_asl_first_v2

Notes:
v2:
  - Enable file dependencies in .if files [Pierre]

 BaseTools/Source/Python/AutoGen/BuildEngine.py  |  6 
 BaseTools/Source/Python/AutoGen/GenMake.py  |  7 +
 BaseTools/Source/Python/AutoGen/ModuleAutoGen.py| 17 +++
 BaseTools/Source/Python/Common/DataType.py  |  3 +-
 BaseTools/Source/Python/Common/Misc.py  |  3 ++
 BaseTools/Source/Python/Workspace/InfBuildData.py   | 32 ++--
 BaseTools/Source/Python/Workspace/MetaFileParser.py | 18 +--
 7 files changed, 81 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/BuildEngine.py 
b/BaseTools/Source/Python/AutoGen/BuildEngine.py
index 
d602414ca41f37155c9c6d00eec54ea3918840c3..687617756619a5b547f18c99c4773842a8328ae8
 100644
--- a/BaseTools/Source/Python/AutoGen/BuildEngine.py
+++ b/BaseTools/Source/Python/AutoGen/BuildEngine.py
@@ -2,6 +2,8 @@
 # The engine for building files
 #
 # Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2020, ARM Limited. All rights reserved.
+#
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 
@@ -53,6 +55,7 @@ class TargetDescBlock(object):
 self.Outputs = Outputs
 self.Commands = Commands
 self.Dependencies = Dependencies
+self.SourceFileDependencies = None
 if self.Outputs:
 self.Target = self.Outputs[0]
 else:
@@ -277,6 +280,9 @@ class FileBuildRule:
 TargetDesc.GenListFile = self.GenListFile
 TargetDesc.GenIncListFile = self.GenIncListFile
 self.BuildTargets[DstFile[0]] = TargetDesc
+
+TargetDesc.SourceFileDependencies = SourceFile.SourceFileDependencies
+
 return TargetDesc
 
 def _BuildCommand(self, Macros):
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 
fe94f9a4c232bb599a59563444c3985700c78ec6..5d932b66fb68baa06570a2eb421d717f0dab8187
 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -2,6 +2,8 @@
 # Create makefile for MS nmake and GNU make
 #
 # Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2020, ARM Limited. All rights reserved.
+#
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 
@@ -997,6 +999,11 @@ cleanlib:
 
 Deps = []
 CCodeDeps = []
+
+if T.SourceFileDependencies:
+for File in T.SourceFileDependencies:
+Deps.append(self.PlaceMacro(str(File), self.Macros))
+
 # Add force-dependencies
 for Dep in T.Dependencies:
 Deps.append(self.PlaceMacro(str(Dep), self.Macros))
diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py 
b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
index 

[edk2-devel] [PATCH v1 1/1] BaseTools: Script for converting AML to .hex

[PierreGondois]

From: Pierre Gondois 

The "-tc" option of the iasl compiler allows to generate a
.hex file containing a C array storing AML bytecode.

An online discussion suggested that this "-tc" option
was specific to the iasl compiler and it shouldn't be relied
on. This conversation is available at:
https://edk2.groups.io/g/devel/topic/39786201#49659

A way to address this issue is to implement a compiler
independent script that takes an AML file as input, and
generates a .hex file.

This patch implements a Python script that converts an AML
file to a .hex file, containing a C array storing AML bytecode.
This scipt has been tested with the AML output from the
following compilers supported by the EDKII implementation:
  * Intel ASL compiler
  * Microsoft ASL compiler

Signed-off-by: Pierre Gondois 
---

The changes can be seen at 
https://github.com/PierreARM/edk2/commits/718_asl_to_hex_script_converter_1

Notes:
v1:
  Script converting AML to .hex file [Pierre]

 BaseTools/BinWrappers/PosixLike/AmlToHex   |  14 ++
 BaseTools/BinWrappers/WindowsLike/AmlToHex.BAT |   3 +
 BaseTools/Conf/build_rule.template |   3 +
 BaseTools/Source/Python/AmlToHex/AmlToHex.py   | 155 
 4 files changed, 175 insertions(+)

diff --git a/BaseTools/BinWrappers/PosixLike/AmlToHex 
b/BaseTools/BinWrappers/PosixLike/AmlToHex
new file mode 100755
index 
..1dd28e966288f6ea4fc52d42e2dc7b1f74226c23
--- /dev/null
+++ b/BaseTools/BinWrappers/PosixLike/AmlToHex
@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+#python `dirname $0`/RunToolFromSource.py `basename $0` $*
+
+# If a ${PYTHON_COMMAND} command is available, use it in preference to python
+if command -v ${PYTHON_COMMAND} >/dev/null 2>&1; then
+python_exe=${PYTHON_COMMAND}
+fi
+
+full_cmd=${BASH_SOURCE:-$0} # see http://mywiki.wooledge.org/BashFAQ/028 for a 
discussion of why $0 is not a good choice here
+dir=$(dirname "$full_cmd")
+exe=$(basename "$full_cmd")
+
+export PYTHONPATH="$dir/../../Source/Python${PYTHONPATH:+:"$PYTHONPATH"}"
+exec "${python_exe:-python}" "$dir/../../Source/Python/$exe/$exe.py" "$@"
diff --git a/BaseTools/BinWrappers/WindowsLike/AmlToHex.BAT 
b/BaseTools/BinWrappers/WindowsLike/AmlToHex.BAT
new file mode 100644
index 
..9616cd893bec9902451e6d8591f537cc408bd5e5
--- /dev/null
+++ b/BaseTools/BinWrappers/WindowsLike/AmlToHex.BAT
@@ -0,0 +1,3 @@
+@setlocal
+@set ToolName=%~n0%
+@%PYTHON_COMMAND% %BASE_TOOLS_PATH%\Source\Python\%ToolName%\%ToolName%.py %*
diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index 
51748bc0655a5c656258a3007b4db6b2dc941ea0..765cbb3881c59715a1bac539a27846e16abcbb49
 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -1,6 +1,7 @@
 #
 #  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
 #  Portions copyright (c) 2008 - 2010, Apple Inc. All rights reserved.
+#  Copyright (c) 2020, ARM Ltd. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 
@@ -427,12 +428,14 @@
 "$(ASLPP)" $(DEPS_FLAGS) $(ASLPP_FLAGS) $(INC) /I${s_path} 
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i > 
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
 Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}. 
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii 
 "$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} 
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.
+AmlToHex $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
 
 
 Trim --asl-file --asl-deps -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i 
-i $(INC_LIST) ${src}
 "$(ASLPP)" $(DEPS_FLAGS) $(ASLPP_FLAGS) $(INC) -I${s_path} 
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i > 
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
 Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}. 
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii 
 "$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} 
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.
+AmlToHex $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
 
 [C-Code-File.AcpiTable]
 
diff --git a/BaseTools/Source/Python/AmlToHex/AmlToHex.py 
b/BaseTools/Source/Python/AmlToHex/AmlToHex.py
new file mode 100644
index 
..e8e7ace3a68532bc625afb1e74404c4e4b0205dd
--- /dev/null
+++ b/BaseTools/Source/Python/AmlToHex/AmlToHex.py
@@ -0,0 +1,155 @@
+## @file
+#
+# Convert an AML file to a .hex file containing the AML bytecode stored in a
+# C array.
+# By default, "Tables\Dsdt.aml" will generate "Tables\Dsdt.hex".
+# "Tables\Dsdt.hex" will contain a C array named "dsdt_aml_code" that contains
+# the AML bytecode.
+#
+# Copyright (c) 2020, ARM Limited. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+import argparse
+import Common.EdkLogger as EdkLogger
+from Common.BuildToolError import *
+import sys
+import os
+
+## Parse the command line arguments.
+#
+# @retval A argparse.NameSpace instance, 

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

[Andrew Fish via Groups.Io]

> On Jan 14, 2020, at 8:46 AM, Ard Biesheuvel  wrote:
> 
> On Mon, 13 Jan 2020 at 18:57, Andrew Fish via Groups.Io
>  wrote:
>> 
>> Ard,
>> 
>> Is the problem GFX console? Would it be possible to have a PCD to assume 
>> graphics console, and if non was found on the boot connect those PCI devices 
>> and update the NVRAM to cause a console to connect. You might have to do a 
>> 2nd connect on the GOP handle after the nvram variable was written to make 
>> the ConSpliter see it?
>> 
> 
> I'm not sure I follow. Do you mean update the console variable if it
> doesn't contain the GOP produced by the Driver option and reboot?
> 

Ard,

I was thinking this specific case was no active GOP driver in the system due to 
the emulation of the ROM. I was saying there could be a platform policy to 
require GOP, and there could be a  platform hook point to check for no GOP and 
take action. This is not really a Driver path, but a fallback path for no 
GOP. So that is kind of cheating :). 

Also I was thinking the ConSpliter would activate the new GOP if you do a 2nd 
gBS->ConnectController on the Graphics PCI or GOP handle after updating the 
console nvram variable. 

Sorry I'm not really familiar with the modern TianoCore BDS, as we have a 
custom BDS on Macs, so I'm talking more hypothetically about how BDS could work.

Thanks,

Andrew Fish

> 
> 
>> 
>> On Jan 13, 2020, at 9:28 AM, Ard Biesheuvel  
>> wrote:
>> 
>> On Fri, 10 Jan 2020 at 17:23, Laszlo Ersek  wrote:
>> 
>> 
>> On 01/10/20 15:37, Ni, Ray wrote:
>> 
>> Ard,
>> I understand now that: BeforeConsole() needs to connect Gfx to get the GOP 
>> device path
>> for ConOut updating. But the GOP driver is specified in Driver and it 
>> will be loaded after
>> BeforeConsole() in today's code. This order makes the Gfx connection fails 
>> to start the GOP driver
>> resulting ConOut not updated.
>> 
>> Moving Driver processing before BeforeConsole() helps in your case. But 
>> it may impact
>> other platforms: There might be platforms that update Driver variables 
>> in BeforeConsole()
>> and expect these drivers are processed in the same boot (not the next boot). 
>> I don't have the real
>> examples. But today's flow allows this. With your change, Driver will 
>> not be processed in the first
>> boot. The change impacts these platforms.
>> 
>> One backward compatible approach can be: processing Driver before 
>> BeforeConsole() and processing the new Driver (added by BeforeConsole()) 
>> after BeforeConsole().
>> 
>> But another problem arises: If BeforeConsole() removes a Driver, 
>> platform's expectation is that deleted Driver doesn't run. But that 
>> driver already runs.
>> 
>> So actually the question is: when BDS core can consume the Driver and 
>> process them?
>> Right now, it’s the time after BeforeConsole(). Just as right now 
>> BeforeConsole() updates ConOut
>> in your case, BeforeConsole() is the place where platform updates all UEFI 
>> defined boot related
>> variables. Processing Driver before BeforeConsole() requires platform 
>> updates Driver
>> in other places. It's a new requirement to the platform.
>> 
>> With all what I explained in above, I cannot agree with the changes.
>> 
>> Another approach is:
>> Platform could connect the GFX in AfterConsole() and update the ConOut. Then 
>> platform could manually call EfiBootManagerConnectConsoleVariable (ConOut) 
>> to re-connect ConOut.
>> It's a bit ugly I agree.
>> 
>> 
>> Let me raise three other ideas (alternatives to each other, and to the 
>> above), with varying levels of annoyance. :)
>> 
>> 
>> Thanks Laszlo
>> 
>> Ray, given your objection to my approach, could you please consider
>> the below and give feedback on which approach is suitable to address
>> the issue I am trying to fix?
>> 
>> 
>> (1) Keep the following logic (i.e. the subject of this patch):
>> 
>> //
>> // Execute Driver Options
>> //
>> LoadOptions = EfiBootManagerGetLoadOptions (, 
>> LoadOptionTypeDriver);
>> ProcessLoadOptions (LoadOptions, LoadOptionCount);
>> EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
>> 
>> in *both* places, but gate each one with a bit in a new bitmask PCD.
>> 
>> (Note: it's probably not the best for any platform to permit both branches, 
>> as driver images would be loaded twice.)
>> 
>> 
>> (2) EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL has recently been added to edk2. It 
>> looks like a well-designed (extensible) protocol, for two reasons:
>> 
>> - the protocol structure has a Revision field,
>> 
>> - the only current member function, RefreshAllBootOptions(), is permitted to 
>> return EFI_UNSUPPORTED -- and the single call site, in the 
>> EfiBootManagerRefreshAllBootOption() function, handles that return value 
>> well (it does nothing).
>> 
>> The idea would be to bump the Revision field, and add a new member function. 
>> Then call this member function (if it exists) in the spot where the current 
>> patch proposes to 

Re: [edk2-devel] [PATCH 1/2] ArmPkg/ArmSmcPsciResetSystemLib: remove EnterS3WithImmediateWake ()

[Ard Biesheuvel]

On Tue, 14 Jan 2020 at 18:18, Leif Lindholm  wrote:
>
> Apologies for late response - only just got email fully back up.
> Minor comment below.
>
> On Tue, Jan 07, 2020 at 10:22:14 +0100, Ard Biesheuvel wrote:
> > EnterS3WithImmediateWake () no longer has any callers, so remove it
> > from ResetSystemLib. Note that this means the hack to support warm
> > reboot by jumping to the SEC entry point with the MMU and caches off
> > is also no longer used, and can be removed as well, along with the PCD
> > PcdArmReenterPeiForCapsuleWarmReboot that was introduced for this
> > purpose.
> >
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  ArmPkg/ArmPkg.dec|  4 
> > --
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 17 
> > -
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c   | 66 
> > +---
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.S  | 24 
> > ---
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.asm| 29 
> > -
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.S  | 23 
> > ---
> >  ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.asm| 28 
> > -
> >  7 files changed, 2 insertions(+), 189 deletions(-)
> >
>
> > diff --git 
> > a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c 
> > b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> > index b2dde9bfc13a..22fcf989e903 100644
> > --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> > +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> > @@ -10,13 +10,10 @@
> >
> >  #include 
> >
> > -#include 
> > -#include 
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> > -#include 
> > +#include 
>
> Why does ArmSmcLib.h move?
> If it is functionally required, then is there a bug in the file?
> If not, can it go back to its original spot?
> (If it can, and does, Reviewed-by: Leif Lindholm )
>

I think that was a side effect of the revert. I'll put it back where it was.

Thanks,

> ((I will keep R-b:ing with that address until Maintainers.txt is
> updated.))
>
> /
> Leif
>
> >
> >  #include 
> >
> > @@ -76,8 +73,6 @@ ResetShutdown (
> >ArmCallSmc ();
> >  }
> >
> > -VOID DisableMmuAndReenterPei (VOID);
> > -
> >  /**
> >This function causes the system to enter S3 and then wake up immediately.
> >

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

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

Re: [edk2-devel] [PATCH edk2-platforms] Platform/DeveloperBox: drop PcdArmReenterPeiForCapsuleWarmReboot

[Ard Biesheuvel]

On Tue, 14 Jan 2020 at 18:26, Leif Lindholm  wrote:
>
> On Tue, Jan 07, 2020 at 10:27:39 +0100, Ard Biesheuvel wrote:
> > The PCD PcdArmReenterPeiForCapsuleWarmReboot is going away so drop the
> > definition for DeveloperBox.
> >
> > Signed-off-by: Ard Biesheuvel 
>
> Reviewed-by: Leif Lindholm 
>

Pushed as df7fcd923a75..f69e04d25b20

Thanks,


> > ---
> >  Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc 
> > b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > index 084b4c994b97..838c55e1d1a8 100644
> > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > @@ -124,7 +124,6 @@
> >gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
> >gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport|FALSE
> >
> > -  gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot|TRUE
> >gEfiMdeModulePkgTokenSpaceGuid.PcdSupportUpdateCapsuleReset|TRUE
> >
> ># needed for NFIT tables installed by RamDiskDxe
> > --
> > 2.20.1
> >

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

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

Re: [edk2-devel] [PATCH edk2-platforms] Platform/DeveloperBox: drop PcdArmReenterPeiForCapsuleWarmReboot

[Leif Lindholm]

On Tue, Jan 07, 2020 at 10:27:39 +0100, Ard Biesheuvel wrote:
> The PCD PcdArmReenterPeiForCapsuleWarmReboot is going away so drop the
> definition for DeveloperBox.
> 
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc 
> b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> index 084b4c994b97..838c55e1d1a8 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> @@ -124,7 +124,6 @@
>gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport|FALSE
>  
> -  gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot|TRUE
>gEfiMdeModulePkgTokenSpaceGuid.PcdSupportUpdateCapsuleReset|TRUE
>  
># needed for NFIT tables installed by RamDiskDxe
> -- 
> 2.20.1
> 

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

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

Re: [edk2-devel] [PATCH 2/2] ArmPlatformPkg/PrePeiCore: enable VFP at startup

[Leif Lindholm]

On Tue, Jan 07, 2020 at 10:22:15 +0100, Ard Biesheuvel wrote:
> While the alternative PEI-less SEC implementation in PrePi already
> takes the EnableVFP PCD into account, the PrePeiCore code does not,
> and so we may end up triggering synchronous exception when code
> attempts to use FP or SIMD registers, which is permitted by the spec.

If you insert "on AARCH64" in an appropriate place in the above line:
Reviewed-by: Leif Lindholm 

> 
> So enable the VFP as early as feasible.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf  | 1 +
>  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 1 +
>  ArmPlatformPkg/PrePeiCore/PrePeiCore.c  | 5 +
>  3 files changed, 7 insertions(+)
> 
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf 
> b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> index f2ac45d171bc..104c7da53317 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> @@ -62,6 +62,7 @@ [FeaturePcd]
>  [FixedPcd]
>gArmTokenSpaceGuid.PcdFvBaseAddress
>gArmTokenSpaceGuid.PcdFvSize
> +  gArmTokenSpaceGuid.PcdVFPEnabled
>  
>gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase
>gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf 
> b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> index 84c319c3679b..ceb173d34f5d 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> @@ -60,6 +60,7 @@ [FeaturePcd]
>  [FixedPcd]
>gArmTokenSpaceGuid.PcdFvBaseAddress
>gArmTokenSpaceGuid.PcdFvSize
> +  gArmTokenSpaceGuid.PcdVFPEnabled
>  
>gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase
>gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c 
> b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> index 4911f67577a2..4f691d62cf3b 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> @@ -77,6 +77,11 @@ CEntryPoint (
>ASSERT (((UINTN)PeiVectorTable & ARM_VECTOR_TABLE_ALIGNMENT) == 0);
>ArmWriteVBar ((UINTN)PeiVectorTable);
>  
> +  // Enable Floating Point
> +  if (FixedPcdGet32 (PcdVFPEnabled)) {
> +ArmEnableVFP ();
> +  }
> +
>//Note: The MMU will be enabled by MemoryPeim. Only the primary core will 
> have the MMU on.
>  
>// If not primary Jump to Secondary Main
> -- 
> 2.20.1
> 

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

View/Reply Online (#53239): https://edk2.groups.io/g/devel/message/53239
Mute This Topic: https://groups.io/mt/69498791/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] ShellPkg/Application: Support nested variables substitution from cmd line

[Vladimir Olovyannikov via Groups.Io]

Hi Gao,

> Hi Vladimir,
>
> > -Original Message-
> > From: Vladimir Olovyannikov 
> > Sent: Saturday, January 11, 2020 12:33 AM
> > To: Ni, Ray ; devel@edk2.groups.io; Gao, Zhichao
> > 
> > Subject: RE: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support
> > nested variables substitution from cmd line
> >
> > Hi Ray,
> >
> > As an example, let's say that you have multiple variants of serverips
> > - each interface has its own, and each interface has a property as say
> > interface no (eth_no).
> > It is logical to address a serverip for an interface as
> > %serverip%eth_no%%, and in the end get the contents of variable
> > serverip0 (say for eth_no containing value 0).
> > In fact, Linux bash allows doing that easily.
> > We are doing most of our upgrade/configuration stuff via Shell scripts
> > which is very flexible, easy and efficient.
> > To support boot redundancy, we support multiple slots for kernel/dtbs,
> > etc, which are addressed as %update_type_%upd_type%% where
> upd_type
> > can be dtb, kernel, etc.
> > The spec says " Environment variables can be used on the command-line
> > by using %variable-name% where variable-name is the environment
> > variable's name. Variable substitution is not recursive."
> > I treat it as an extension to %var% case; %var case is not touched.
> > What do you think?
>
> "Variable substitution is not recursive" means the enviornment variable
> doesn't support nested. If you want to push the change, you should change
> the spec first. And there is one variable substitution flow figure, it
> should be
> update as well.
OK, thank you, I understand.
There is actually an ugly workaround. Vars substitution depends on the var
order. Thus, there is a way to
redeclare a variable (after deletion), so it changes the order in a way that
in %serverip%eth_no%% eth_no comes first,
and %serverip...% comes next. Then this nesting sort of working.
Another ugly (but possible) way would be do something like this:
set serverip%eth_no% >v tmp
This gives " serverip0 = nnn.nnn.nnn.nnn", and then have a string
find/supporting app which would give anything after "= ".
Still not clear why this limitation in the specs It is easy to do with
Bash script, and for a Shell script it is just a small change...
May I suggest to review the spec regarding Shell variables substitution?

Thank you,
Vladimir
>
> Thanks,
> Zhichao
>
> >
> > Regarding unit tests.
> > I verified variables substitutions using %var% and %var%var1%% and
> > %%var1%var% to make sure there would be no regression.
> > Our scripts use variables extensively, and no issues were encountered.
> > Please let me know if you need the scripts we are using, and I will
> > send them to you.
> >
> > Which unit tests are available?
> >
> > Thank you,
> > Vladimir
> >
> > -Original Message-
> > From: Ni, Ray 
> > Sent: Thursday, January 9, 2020 10:03 PM
> > To: devel@edk2.groups.io; vladimir.olovyanni...@broadcom.com; Gao,
> > Zhichao 
> > Subject: RE: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support
> > nested variables substitution from cmd line
> >
> > Vladimir,
> > Is this enhancement an extension to Shell spec or violation of Shell
> > spec?
> > If it's an extension, I am happy to have such feature implemented.
> > Otherwise, we may need to discuss in uefi.org.
> >
> > Can you list the unit tests you have done?
> >
> > Thanks,
> > Ray
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of
> > > Vladimir Olovyannikov via Groups.Io
> > > Sent: Friday, January 10, 2020 6:07 AM
> > > To: devel@edk2.groups.io; Ni, Ray ; Gao, Zhichao
> > > 
> > > Cc: Vladimir Olovyannikov 
> > > Subject: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support
> > > nested variables substitution from cmd line
> > >
> > > The current implementation of ShellSubstituteVariables() does not
> > > allow substitution of variables names containing another variable
> > > name(s) between %...%
> > >
> > > Example: %text%var_name%% where var_name variable contains 0.
> > > Here the variable value which should be returned on substitution
> > > would be contents of text0 variable.
> > > The current implementation would return nothing as %text0% would be
> > > eliminated by StripUnreplacedEnvironmentVariables().
> > >
> > > Modify the code so that StripUnreplacedEnvironmentVariables checks
> > > if variable between %...% really exists. If it does not, delete %...%.
> > > If it exists, preserve it and tell the caller that another run of
> > > ShellConvertVariables() is needed. This way, any nested variable
> > > between %...% can be easily retrieved.
> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2452
> > >
> > > Signed-off-by: Vladimir Olovyannikov
> > > 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > ---
> > >  ShellPkg/Application/Shell/Shell.c | 71
> > > --
> > >  1 file changed, 57 insertions(+), 14 deletions(-)
> > >
> > > diff --git 

[edk2-devel] [edk2-platforms] [PATCH] BoardModulePkg: Fix ECC Coding Style Issues

[Agyeman, Prince]

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

Fixed coding style issues as reported by the ECC tool

Cc: Shenglei Zhang 
Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Michael Kubacki 

Signed-off-by: Prince Agyeman 
---
 .../BoardModulePkg/LegacySioDxe/ComponentName.h   |  5 +
 .../BoardModulePkg/LegacySioDxe/LegacySioDxe.inf  |  2 ++
 .../Intel/BoardModulePkg/LegacySioDxe/SioChip.c   |  5 +++--
 .../Intel/BoardModulePkg/LegacySioDxe/SioDriver.c | 15 ---
 .../Library/BdsPs2KbcLib/BdsPs2KbcLib.c   |  6 +++---
 .../Library/BdsPs2KbcLib/BdsPs2KbcLib.h   |  4 ++--
 6 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/Platform/Intel/BoardModulePkg/LegacySioDxe/ComponentName.h 
b/Platform/Intel/BoardModulePkg/LegacySioDxe/ComponentName.h
index 85ca348701..91bc245f3a 100644
--- a/Platform/Intel/BoardModulePkg/LegacySioDxe/ComponentName.h
+++ b/Platform/Intel/BoardModulePkg/LegacySioDxe/ComponentName.h
@@ -6,6 +6,9 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
+#ifndef _LEGACY_SIO_DXE_COMPONENT_NAME_H_
+#define _LEGACY_SIO_DXE_COMPONENT_NAME_H_
+
 /**
   Retrieves a Unicode string that is the user-readable name of the EFI Driver.
 
@@ -85,3 +88,5 @@ SioComponentNameGetControllerName (
   IN  CHAR8 *Language,
   OUT CHAR16**ControllerName
   );
+
+#endif
diff --git a/Platform/Intel/BoardModulePkg/LegacySioDxe/LegacySioDxe.inf 
b/Platform/Intel/BoardModulePkg/LegacySioDxe/LegacySioDxe.inf
index 1d7cd92604..698d406450 100644
--- a/Platform/Intel/BoardModulePkg/LegacySioDxe/LegacySioDxe.inf
+++ b/Platform/Intel/BoardModulePkg/LegacySioDxe/LegacySioDxe.inf
@@ -44,6 +44,8 @@
   SioDriver.c
   SioDriver.h
   ComponentName.c
+  ComponentName.h
+  Register.h
 
 [Pcd]
   gBoardModulePkgTokenSpaceGuid.PcdPs2KbMsEnable
diff --git a/Platform/Intel/BoardModulePkg/LegacySioDxe/SioChip.c 
b/Platform/Intel/BoardModulePkg/LegacySioDxe/SioChip.c
index b9a84ca51c..846dddf739 100644
--- a/Platform/Intel/BoardModulePkg/LegacySioDxe/SioChip.c
+++ b/Platform/Intel/BoardModulePkg/LegacySioDxe/SioChip.c
@@ -145,7 +145,7 @@ DEVICE_INFOmDeviceInfo[] = {
 
 
 /**
-  Gets the number of devices in Table of SIO Controllers mDeviceInfo
+  Gets the number of devices in Table of SIO Controllers mDeviceInfo.
 
   @retval Number of enabled devices in Table of SIO Controllers.
 **/
@@ -153,7 +153,8 @@ UINTN
 EFIAPI
 GetDeviceCount (
   VOID
-){
+  )
+{
UINTNCount;
// Get mDeviceInfo item count
// -1 to account for for the end device info
diff --git a/Platform/Intel/BoardModulePkg/LegacySioDxe/SioDriver.c 
b/Platform/Intel/BoardModulePkg/LegacySioDxe/SioDriver.c
index 5bfdc94681..484c53e953 100644
--- a/Platform/Intel/BoardModulePkg/LegacySioDxe/SioDriver.c
+++ b/Platform/Intel/BoardModulePkg/LegacySioDxe/SioDriver.c
@@ -118,13 +118,14 @@ BOOLEAN
 EFIAPI
 SioDeviceEnabled (
   IN SIO_PCI_ISA_BRIDGE_DEVICE_INFO *CurrentDevice
-){
-SIO_PCI_ISA_BRIDGE_DEVICE_INFO *Device = \
-  (SIO_PCI_ISA_BRIDGE_DEVICE_INFO *) FixedPcdGetPtr 
(PcdSuperIoPciIsaBridgeDevice);
-if(CompareMem (Device, CurrentDevice, sizeof 
(SIO_PCI_ISA_BRIDGE_DEVICE_INFO)) == 0) {
-  return TRUE;
-}
-return FALSE;
+  )
+{
+  SIO_PCI_ISA_BRIDGE_DEVICE_INFO *Device;
+  Device = (SIO_PCI_ISA_BRIDGE_DEVICE_INFO *) FixedPcdGetPtr 
(PcdSuperIoPciIsaBridgeDevice);
+  if(CompareMem (Device, CurrentDevice, sizeof 
(SIO_PCI_ISA_BRIDGE_DEVICE_INFO)) == 0) {
+return TRUE;
+  }
+  return FALSE;
 }
 
 /**
diff --git a/Platform/Intel/BoardModulePkg/Library/BdsPs2KbcLib/BdsPs2KbcLib.c 
b/Platform/Intel/BoardModulePkg/Library/BdsPs2KbcLib/BdsPs2KbcLib.c
index eff10bddb4..0cf4c4d262 100644
--- a/Platform/Intel/BoardModulePkg/Library/BdsPs2KbcLib/BdsPs2KbcLib.c
+++ b/Platform/Intel/BoardModulePkg/Library/BdsPs2KbcLib/BdsPs2KbcLib.c
@@ -65,7 +65,7 @@ DetectPs2Keyboard (
 MicroSecondDelay (30);
   }
 
-  if (FoundPs2Kbc == FALSE) {
+  if (!FoundPs2Kbc) {
 return FALSE;
   }
 
@@ -126,7 +126,7 @@ IsPs2KeyboardConnected (
   BOOLEAN Result;
   Result = DetectPs2Keyboard ();
 
-  if (Result == FALSE) {
+  if (!Result) {
 //
 // If there is no ps2 keyboard detected for the 1st time, retry again.
 //
@@ -138,7 +138,7 @@ IsPs2KeyboardConnected (
 
 /**
   Updates the ConIn variable with Ps2 Keyboard device path,
-  if it doesn't already exists in ConIn and ConInDev
+  if it doesn't already exists in ConIn and ConInDev.
 **/
 VOID
 AddPs2Keyboard (
diff --git a/Platform/Intel/BoardModulePkg/Library/BdsPs2KbcLib/BdsPs2KbcLib.h 
b/Platform/Intel/BoardModulePkg/Library/BdsPs2KbcLib/BdsPs2KbcLib.h
index d9a27e6681..a96d53b98d 100644
--- a/Platform/Intel/BoardModulePkg/Library/BdsPs2KbcLib/BdsPs2KbcLib.h
+++ b/Platform/Intel/BoardModulePkg/Library/BdsPs2KbcLib/BdsPs2KbcLib.h
@@ -6,8 +6,8 @@
 
 **/
 
-#ifndef _PS2_KBC_LIB_H
-#define _PS2_KBC_LIB_H
+#ifndef _PS2_KBC_LIB_H_
+#define _PS2_KBC_LIB_H_
 
 #include 
 #include 
-- 

Re: [edk2-devel] [PATCH 1/2] ArmPkg/ArmSmcPsciResetSystemLib: remove EnterS3WithImmediateWake ()

[Leif Lindholm]

Apologies for late response - only just got email fully back up.
Minor comment below.

On Tue, Jan 07, 2020 at 10:22:14 +0100, Ard Biesheuvel wrote:
> EnterS3WithImmediateWake () no longer has any callers, so remove it
> from ResetSystemLib. Note that this means the hack to support warm
> reboot by jumping to the SEC entry point with the MMU and caches off
> is also no longer used, and can be removed as well, along with the PCD
> PcdArmReenterPeiForCapsuleWarmReboot that was introduced for this
> purpose.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/ArmPkg.dec|  4 --
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 17 
> -
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c   | 66 
> +---
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.S  | 24 
> ---
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.asm| 29 
> -
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.S  | 23 
> ---
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.asm| 28 
> -
>  7 files changed, 2 insertions(+), 189 deletions(-)
> 

> diff --git 
> a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c 
> b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> index b2dde9bfc13a..22fcf989e903 100644
> --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> @@ -10,13 +10,10 @@
>  
>  #include 
>  
> -#include 
> -#include 
>  #include 
>  #include 
>  #include 
> -#include 
> -#include 
> +#include 

Why does ArmSmcLib.h move?
If it is functionally required, then is there a bug in the file?
If not, can it go back to its original spot?
(If it can, and does, Reviewed-by: Leif Lindholm )

((I will keep R-b:ing with that address until Maintainers.txt is
updated.))

/
Leif

>  
>  #include 
>  
> @@ -76,8 +73,6 @@ ResetShutdown (
>ArmCallSmc ();
>  }
>  
> -VOID DisableMmuAndReenterPei (VOID);
> -
>  /**
>This function causes the system to enter S3 and then wake up immediately.
>  

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

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

[edk2-devel] [PATCH edk2-platforms 2/2] Maintainers.txt: update email address for Leif Lindholm

[Leif Lindholm]

Leif now works at NUVIA Inc, update email address accordingly.

Cc: Andy Hayes 
Cc: Ard Biesheuvel 
Cc: Marcin Wojtas 
Cc: Michael D Kinney 
Cc: Pete Batard 
Cc: Leif Lindholm 
Signed-off-by: Leif Lindholm 
---
 Maintainers.txt | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index dfecd3021441..8211d5a632a7 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -68,7 +68,7 @@ F: */
 EDK II Platforms maintainers
 
 F: *
-M: Leif Lindholm 
+M: Leif Lindholm 
 M: Michael D Kinney 
 
 Responsible Disclosure, Reporting Security Issues
@@ -81,7 +81,7 @@ EDK II Platforms Packages:
 96Boards
 F: Platform/96Boards/
 M: Ard Biesheuvel 
-M: Leif Lindholm 
+M: Leif Lindholm 
 
 AMD Seattle
 F: Platform/AMD/OverdriveBoard/
@@ -89,23 +89,23 @@ F: Platform/LeMaker/CelloBoard/
 F: Platform/SoftIron/
 F: Silicon/AMD/Styx/
 M: Ard Biesheuvel 
-M: Leif Lindholm 
+M: Leif Lindholm 
 
 ARM
 F: Platform/ARM/
 R: Ard Biesheuvel 
-M: Leif Lindholm 
+M: Leif Lindholm 
 
 BeagleBoard:
 F: Platform/BeagleBoard/
 F: Silicon/TexasInstruments/
 R: Ard Biesheuvel 
-M: Leif Lindholm 
+M: Leif Lindholm 
 
 Comcast
 F: Platform/Comcast/
 M: Ard Biesheuvel 
-M: Leif Lindholm 
+M: Leif Lindholm 
 
 OptionRomPkg
 F: Drivers/OptionRomPkg/
@@ -114,14 +114,14 @@ M: Ray Ni 
 
 DisplayLink
 F: Drivers/DisplayLink/
-M: Leif Lindholm 
+M: Leif Lindholm 
 M: Ard Biesheuvel 
 R: Andy Hayes 
 
 HiSilicon
 F: Platform/Hisilicon/
 F: Silicon/Hisilicon/
-M: Leif Lindholm 
+M: Leif Lindholm 
 R: Ard Biesheuvel 
 
 Features/Intel
@@ -232,7 +232,7 @@ F: Platform/Marvell/
 F: Platform/SolidRun/Armada80x0McBin/
 F: Silicon/Marvell/
 R: Marcin Wojtas 
-M: Leif Lindholm 
+M: Leif Lindholm 
 
 Miscellaneous drivers
 F: Silicon/Atmel/
@@ -240,17 +240,17 @@ F: Silicon/NXP/
 F: Silicon/Openmoko/
 F: Silicon/Synopsys/DesignWare/
 R: Ard Biesheuvel 
-M: Leif Lindholm 
+M: Leif Lindholm 
 
 Raspberry Pi platforms and silicon
 F: Platform/RaspberryPi/
 F: Silicon/Broadcom/
 M: Ard Biesheuvel 
-M: Leif Lindholm 
+M: Leif Lindholm 
 R: Pete Batard 
 
 Socionext platforms and silicon
 F: Platform/Socionext/
 F: Silicon/Socionext/
 M: Ard Biesheuvel 
-M: Leif Lindholm 
+M: Leif Lindholm 
-- 
2.20.1


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

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

[edk2-devel] [PATCH edk2/eck2-platforms 0/2] Update email address for Leif Lindholm

[Leif Lindholm]

I changed jobs at the turn of the year and now work for NUVIA
(https://nuviainc.com/). My employer would like me to continue working
on TianoCore, so let's just update the address to my current one.

I have Cc:d anyone I share a section in a Maintainers.txt with, but I
don't think I need Reviewed-by:s from everyone - only one of the stewards
for edk2 and I guess Mike for edk2-platforms.

Many thanks to Linaro, who are letting me keep that address through the
transition phase. But that address will become non-functional in a couple
of weeks or so.

Leif Lindholm (2):
  Maintainers.txt: update email address for Leif Lindholm
  Maintainers.txt: update email address for Leif Lindholm

 Maintainers.txt | 14 +++---
 Maintainers.txt | 24 -
 2 files changed, 19 insertions(+), 19 deletions(-)

Cc: Andrew Fish 
Cc: Andy Hayes 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Marcin Wojtas 
Cc: Michael D Kinney 
Cc: Pete Batard 
Cc: Ray Ni 
Cc: Zhichao Gao 

-- 
2.20.1


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

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

[edk2-devel] [PATCH 1/2] Maintainers.txt: update email address for Leif Lindholm

[Leif Lindholm]

Leif now works at NUVIA Inc, update email address accordingly.

Cc: Andrew Fish 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Michael D Kinney 
Cc: Ray Ni 
Cc: Zhichao Gao 
Cc: Leif Lindholm 
Signed-off-by: Leif Lindholm 
---
 Maintainers.txt | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 146d8aca93f0..ca9da2892534 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -70,7 +70,7 @@ Tianocore Stewards
 F: *
 M: Andrew Fish 
 M: Laszlo Ersek 
-M: Leif Lindholm 
+M: Leif Lindholm 
 M: Michael D Kinney 
 
 Responsible Disclosure, Reporting Security Issues
@@ -87,7 +87,7 @@ UEFI Shell Binaries (ShellBinPkg.zip) from EDK II Releases:
 W: https://github.com/tianocore/edk2/releases/
 M: Ray Ni   (Ia32/X64)
 M: Zhichao Gao (Ia32/X64)
-M: Leif Lindholm(ARM/AArch64)
+M: Leif Lindholm   (ARM/AArch64)
 M: Ard Biesheuvel  (ARM/AArch64)
 
 EDK II Architectures:
@@ -95,7 +95,7 @@ EDK II Architectures:
 ARM, AARCH64
 F: */AArch64/
 F: */Arm/
-M: Leif Lindholm 
+M: Leif Lindholm 
 M: Ard Biesheuvel 
 
 EDK II Continuous Integration:
@@ -126,13 +126,13 @@ EDK II Packages:
 ArmPkg
 F: ArmPkg/
 W: https://github.com/tianocore/tianocore.github.io/wiki/ArmPkg
-M: Leif Lindholm 
+M: Leif Lindholm 
 M: Ard Biesheuvel 
 
 ArmPlatformPkg
 F: ArmPlatformPkg/
 W: https://github.com/tianocore/tianocore.github.io/wiki/ArmPlatformPkg
-M: Leif Lindholm 
+M: Leif Lindholm 
 M: Ard Biesheuvel 
 
 ArmVirtPkg
@@ -140,7 +140,7 @@ F: ArmVirtPkg/
 W: https://github.com/tianocore/tianocore.github.io/wiki/ArmVirtPkg
 M: Laszlo Ersek 
 M: Ard Biesheuvel 
-R: Leif Lindholm 
+R: Leif Lindholm 
 
 ArmVirtPkg: modules used on Xen
 F: ArmVirtPkg/ArmVirtXen.*
@@ -173,7 +173,7 @@ M: Alexei Fedorov 
 EmbeddedPkg
 F: EmbeddedPkg/
 W: https://github.com/tianocore/tianocore.github.io/wiki/EmbeddedPkg
-M: Leif Lindholm 
+M: Leif Lindholm 
 M: Ard Biesheuvel 
 
 EmulatorPkg
-- 
2.20.1


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

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

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

[Ard Biesheuvel]

On Mon, 13 Jan 2020 at 18:57, Andrew Fish via Groups.Io
 wrote:
>
> Ard,
>
> Is the problem GFX console? Would it be possible to have a PCD to assume 
> graphics console, and if non was found on the boot connect those PCI devices 
> and update the NVRAM to cause a console to connect. You might have to do a 
> 2nd connect on the GOP handle after the nvram variable was written to make 
> the ConSpliter see it?
>

I'm not sure I follow. Do you mean update the console variable if it
doesn't contain the GOP produced by the Driver option and reboot?



>
> On Jan 13, 2020, at 9:28 AM, Ard Biesheuvel  wrote:
>
> On Fri, 10 Jan 2020 at 17:23, Laszlo Ersek  wrote:
>
>
> On 01/10/20 15:37, Ni, Ray wrote:
>
> Ard,
> I understand now that: BeforeConsole() needs to connect Gfx to get the GOP 
> device path
> for ConOut updating. But the GOP driver is specified in Driver and it 
> will be loaded after
> BeforeConsole() in today's code. This order makes the Gfx connection fails to 
> start the GOP driver
> resulting ConOut not updated.
>
> Moving Driver processing before BeforeConsole() helps in your case. But 
> it may impact
> other platforms: There might be platforms that update Driver variables in 
> BeforeConsole()
> and expect these drivers are processed in the same boot (not the next boot). 
> I don't have the real
> examples. But today's flow allows this. With your change, Driver will not 
> be processed in the first
> boot. The change impacts these platforms.
>
> One backward compatible approach can be: processing Driver before 
> BeforeConsole() and processing the new Driver (added by BeforeConsole()) 
> after BeforeConsole().
>
> But another problem arises: If BeforeConsole() removes a Driver, 
> platform's expectation is that deleted Driver doesn't run. But that 
> driver already runs.
>
> So actually the question is: when BDS core can consume the Driver and 
> process them?
> Right now, it’s the time after BeforeConsole(). Just as right now 
> BeforeConsole() updates ConOut
> in your case, BeforeConsole() is the place where platform updates all UEFI 
> defined boot related
> variables. Processing Driver before BeforeConsole() requires platform 
> updates Driver
> in other places. It's a new requirement to the platform.
>
> With all what I explained in above, I cannot agree with the changes.
>
> Another approach is:
> Platform could connect the GFX in AfterConsole() and update the ConOut. Then 
> platform could manually call EfiBootManagerConnectConsoleVariable (ConOut) to 
> re-connect ConOut.
> It's a bit ugly I agree.
>
>
> Let me raise three other ideas (alternatives to each other, and to the 
> above), with varying levels of annoyance. :)
>
>
> Thanks Laszlo
>
> Ray, given your objection to my approach, could you please consider
> the below and give feedback on which approach is suitable to address
> the issue I am trying to fix?
>
>
> (1) Keep the following logic (i.e. the subject of this patch):
>
>  //
>  // Execute Driver Options
>  //
>  LoadOptions = EfiBootManagerGetLoadOptions (, 
> LoadOptionTypeDriver);
>  ProcessLoadOptions (LoadOptions, LoadOptionCount);
>  EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
>
> in *both* places, but gate each one with a bit in a new bitmask PCD.
>
> (Note: it's probably not the best for any platform to permit both branches, 
> as driver images would be loaded twice.)
>
>
> (2) EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL has recently been added to edk2. It 
> looks like a well-designed (extensible) protocol, for two reasons:
>
> - the protocol structure has a Revision field,
>
> - the only current member function, RefreshAllBootOptions(), is permitted to 
> return EFI_UNSUPPORTED -- and the single call site, in the 
> EfiBootManagerRefreshAllBootOption() function, handles that return value well 
> (it does nothing).
>
> The idea would be to bump the Revision field, and add a new member function. 
> Then call this member function (if it exists) in the spot where the current 
> patch proposes to move the Driver dispatch logic to.
>
> This is almost like a new PlatformBootManagerLib interface, except it does 
> not require existent lib instances to be updated.
>
> And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL implementation side, 
> RefreshAllBootOptions() would return EFI_UNSUPPORTED. (Because that is 
> irrelevant to Ard's use case.)
>
> Perhaps add yet another member function that can disable the Driver 
> option processing in the current location.
>
>
> (3) Extend the UEFI specification, section "3.1.3 Load Options".
>
> The LOAD_OPTION_CATEGORY bit-field is almost what we want, except it's 
> specified to be ignored for Driver options. So,
>
> - either invent the standardese to let us use LOAD_OPTION_CATEGORY for 
> Driver options too, without breaking existing platforms, *or*
> - introduce a new (not too wide) distinct bitfield called 
> LOAD_OPTION_DRIVER_CATEGORY.
>
> Whichever 

Re: [edk2-devel] [PATCH 0/2] some ARM fixes

[Ard Biesheuvel]

On Tue, 7 Jan 2020 at 10:22, Ard Biesheuvel  wrote:
>
> Some preparatory fixes for TPM measured boot on ARM systems.
>
> Patch #1 removes EnterS3WithImmediateWake() with the associated support
> code from ArmSmcPsciResetSystemLib. EnterS3WithImmediateWake () is no
> longer being called anywhere, and will be removed from ResetSystemLib,
> and the support code dependencies on DXE facilities are preventing this
> library from being used in the PEI phase, which is needed for TPM support.
>
> Patch #2 adds the missing call to enable VFP on PrePeiCore based platforms.
> This is causing TPM crypto code to blow up.
>
> Ard Biesheuvel (2):
>   ArmPkg/ArmSmcPsciResetSystemLib: remove EnterS3WithImmediateWake ()
>   ArmPlatformPkg/PrePeiCore: enable VFP at startup
>

Ping?


>  ArmPkg/ArmPkg.dec|  4 --
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 17 
> -
>  ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf   |  1 +
>  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf  |  1 +
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c   | 66 
> +---
>  ArmPlatformPkg/PrePeiCore/PrePeiCore.c   |  5 ++
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.S  | 24 
> ---
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.asm| 29 
> -
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.S  | 23 
> ---
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.asm| 28 
> -
>  10 files changed, 9 insertions(+), 189 deletions(-)
>  delete mode 100644 ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.S
>  delete mode 100644 ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.asm
>  delete mode 100644 ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.S
>  delete mode 100644 ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.asm
>
> --
> 2.20.1
>

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

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

Re: [edk2-devel] [PATCH edk2/eck2-platforms 0/2] Update email address for Leif Lindholm

[Ard Biesheuvel]

On Tue, 14 Jan 2020 at 17:32, Leif Lindholm  wrote:
>
> I changed jobs at the turn of the year and now work for NUVIA
> (https://nuviainc.com/). My employer would like me to continue working
> on TianoCore, so let's just update the address to my current one.
>
> I have Cc:d anyone I share a section in a Maintainers.txt with, but I
> don't think I need Reviewed-by:s from everyone - only one of the stewards
> for edk2 and I guess Mike for edk2-platforms.
>
> Many thanks to Linaro, who are letting me keep that address through the
> transition phase. But that address will become non-functional in a couple
> of weeks or so.
>
> Leif Lindholm (2):
>   Maintainers.txt: update email address for Leif Lindholm
>   Maintainers.txt: update email address for Leif Lindholm
>

Reviewed-by: Ard Biesheuvel 

>  Maintainers.txt | 14 +++---
>  Maintainers.txt | 24 -
>  2 files changed, 19 insertions(+), 19 deletions(-)
>
> Cc: Andrew Fish 
> Cc: Andy Hayes 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Cc: Marcin Wojtas 
> Cc: Michael D Kinney 
> Cc: Pete Batard 
> Cc: Ray Ni 
> Cc: Zhichao Gao 
>
> --
> 2.20.1
>

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

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

[edk2-devel] [PATCH v2 1/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API

[Sukerkar, Amol N]

This commit introduces a Unified Hash API to calculate hash using a
hashing algorithm specified by the PCD, PcdSystemHashPolicy. This library
interfaces with the various hashing API, such as, MD4, MD5, SHA1, SHA256,
SHA512 and SM3_256 implemented in CryptoPkg. The user can calculate the
desired hash by setting PcdSystemHashPolicy to appropriate value.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Michael D Kinney 
Signed-off-by: Sukerkar, Amol N 
---

Notes:
v2:
- Fixed the commit message format

 SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c | 252 
 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c| 122 ++
 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c| 125 ++
 SecurityPkg/Include/Library/BaseHashLib.h   |  84 +++
 SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h |  71 ++
 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf  |  47 
 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni  |  18 ++
 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf  |  52 
 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni  |  17 ++
 SecurityPkg/SecurityPkg.dec |  23 +-
 SecurityPkg/SecurityPkg.dsc |  10 +-
 SecurityPkg/SecurityPkg.uni |  15 +-
 12 files changed, 833 insertions(+), 3 deletions(-)

diff --git a/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c 
b/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
new file mode 100644
index ..f8742e55b5f7
--- /dev/null
+++ b/SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
@@ -0,0 +1,252 @@
+/** @file
+  Implement image verification services for secure boot service
+
+  Caution: This file requires additional review when modified.
+  This library will have external input - PE/COFF image.
+  This external input must be validated carefully to avoid security issue like
+  buffer overflow, integer overflow.
+
+  DxeImageVerificationLibImageRead() function will make sure the PE/COFF image 
content
+  read is within the image buffer.
+
+  DxeImageVerificationHandler(), HashPeImageByType(), HashPeImage() function 
will accept
+  untrusted PE/COFF image and validate its data structure within this image 
buffer before use.
+
+Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD 
License
+which accompanies this distribution.  The full text of the license may be 
found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+  Init hash sequence with Hash Algorithm specified by HashPolicy.
+
+  @param HashPolicy  Hash Algorithm Policy.
+  @param HashHandle  Hash handle.
+
+  @retval TRUE   Hash start and HashHandle returned.
+  @retval FALSE  Hash Init unsuccessful.
+**/
+BOOLEAN
+EFIAPI
+HashInitInternal (
+  IN UINT8  HashPolicy, 
+  OUT HASH_HANDLE   *HashHandle
+  )
+{
+  BOOLEAN  Status;
+  VOID *HashCtx;
+  UINTNCtxSize;
+
+  switch (HashPolicy) {
+case HASH_MD4:
+  CtxSize = Md4GetContextSize ();
+  HashCtx = AllocatePool (CtxSize);
+  ASSERT (HashCtx != NULL);
+
+  Status = Md4Init (HashCtx);
+  break;
+
+case HASH_MD5:
+  CtxSize = Md5GetContextSize ();
+  HashCtx = AllocatePool (CtxSize);
+  ASSERT (HashCtx != NULL);
+
+ Status = Md5Init (HashCtx);
+  break;
+
+case HASH_SHA1:
+  CtxSize = Sha1GetContextSize ();
+  HashCtx = AllocatePool (CtxSize);
+  ASSERT (HashCtx != NULL);
+
+  Status = Sha1Init (HashCtx);
+  break;
+
+case HASH_SHA256:
+  CtxSize = Sha256GetContextSize ();
+  HashCtx = AllocatePool (CtxSize);
+  ASSERT (HashCtx != NULL);
+
+  Status = Sha256Init (HashCtx);
+  break;
+
+case HASH_SHA384:
+  CtxSize = Sha384GetContextSize ();
+  HashCtx = AllocatePool (CtxSize);
+  ASSERT (HashCtx != NULL);
+
+  Status = Sha384Init (HashCtx);
+  break;
+
+case HASH_SHA512:
+  CtxSize = Sha512GetContextSize ();
+  HashCtx = AllocatePool (CtxSize);
+  ASSERT (HashCtx != NULL);
+
+  Status = Sha512Init (HashCtx);
+  break;
+
+case HASH_SM3_256:
+  CtxSize = Sm3GetContextSize ();
+  HashCtx = AllocatePool (CtxSize);
+  ASSERT (HashCtx != NULL);
+
+  Status = Sm3Init (HashCtx);
+  break;
+
+default:
+  ASSERT (FALSE);
+  break;
+  }
+
+  *HashHandle = (HASH_HANDLE)HashCtx;
+
+  return Status;
+}
+
+/**
+  Update hash data with Hash Algorithm specified by HashPolicy.
+
+  @param HashPolicyHash Algorithm Policy.
+  @param HashHandleHash handle.
+  @param DataToHashData to be 

[edk2-devel] [PATCH v2 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API

[Sukerkar, Amol N]

Currently, the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded
API to calculate the hash, for instance, sha_256(...), etc. Since SHA384 and/or
SM3_256 are being increasingly adopted for robustness, it becomes cumbersome to
modify each driver that calls into hash calculating API.

To better achieve this, we are proposing a Unified API, which can be used by 
UEFI
drivers, that provides the drivers with flexibility to use the desired hashing
algorithm based on the required robnustness.

Alternatively, the design document is also attached to Bugzilla,
https://bugzilla.tianocore.org/show_bug.cgi?id=2151.

Sukerkar, Amol N (1):
  SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API

 SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c | 252 
 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c| 122 ++
 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c| 125 ++
 SecurityPkg/Include/Library/BaseHashLib.h   |  84 +++
 SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h |  71 ++
 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf  |  47 
 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni  |  18 ++
 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf  |  52 
 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni  |  17 ++
 SecurityPkg/SecurityPkg.dec |  23 +-
 SecurityPkg/SecurityPkg.dsc |  10 +-
 SecurityPkg/SecurityPkg.uni |  15 +-
 12 files changed, 833 insertions(+), 3 deletions(-)
 create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.c
 create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.c
 create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.c
 create mode 100644 SecurityPkg/Include/Library/BaseHashLib.h
 create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibCommon.h
 create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.inf
 create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibDxe.uni
 create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.inf
 create mode 100644 SecurityPkg/Library/BaseHashLib/BaseHashLibPei.uni

-- 
2.16.2.windows.1


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

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

Re: [edk2-devel] [patch] MdeModulePkg/Setup: Update opcode number variable type to UINTN

[Laszlo Ersek]

On 01/14/20 14:08, Laszlo Ersek wrote:
> Hi Dandan, Brian,
> 
> On 01/14/20 09:56, Dandan Bi wrote:
>> From: Brian R Haug 
>>
>> Update data type of variables which save the opcode numbers
>> to UINTN, in case some configuration module has lots of
>> configuration items.
>>
>> Cc: Liming Gao 
>> Cc: Eric Dong 
>> Signed-off-by: Brian R Haug 
>> Reviewed-by: Dandan Bi 
>> ---
>>  .../Universal/SetupBrowserDxe/IfrParse.c   | 18 +-
>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c 
>> b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
>> index 891b95cf9f..edb6a0fc4c 100644
>> --- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
>> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
>> @@ -1,17 +1,17 @@
>>  /** @file
>>  Parser for IFR binary encoding.
>>
>> -Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
>> +Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
>>  SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>>  **/
>>
>>  #include "Setup.h"
>>
>> -UINT16   mStatementIndex;
>> -UINT16   mExpressionOpCodeIndex;
>> +UINTNmStatementIndex;
>> +UINTNmExpressionOpCodeIndex;
>>  EFI_QUESTION_ID  mUsedQuestionId;
>>  extern LIST_ENTRY  gBrowserStorageList;
>>  /**
>>Initialize Statement header members.
>>
>> @@ -1104,16 +1104,16 @@ IsUnKnownOpCode (
>>
>>  **/
>>  VOID
>>  CountOpCodes (
>>IN  FORM_BROWSER_FORMSET  *FormSet,
>> -  IN OUT  UINT16*NumberOfStatement,
>> -  IN OUT  UINT16*NumberOfExpression
>> +  OUT  UINTN *NumberOfStatement,
>> +  OUT  UINTN *NumberOfExpression
>>)
>>  {
>> -  UINT16  StatementCount;
>> -  UINT16  ExpressionCount;
>> +  UINTN   StatementCount;
>> +  UINTN   ExpressionCount;
>>UINT8   *OpCodeData;
>>UINTN   Offset;
>>UINTN   OpCodeLen;
>>
>>Offset = 0;
>> @@ -1167,12 +1167,12 @@ ParseOpCodes (
>>FORMSET_STORAGE *Storage;
>>FORMSET_DEFAULTSTORE*DefaultStore;
>>QUESTION_DEFAULT*CurrentDefault;
>>QUESTION_OPTION *CurrentOption;
>>UINT8   Width;
>> -  UINT16  NumberOfStatement;
>> -  UINT16  NumberOfExpression;
>> +  UINTN   NumberOfStatement;
>> +  UINTN   NumberOfExpression;
>>EFI_IMAGE_ID*ImageId;
>>BOOLEAN SuppressForQuestion;
>>BOOLEAN SuppressForOption;
>>UINT16  DepthOfDisable;
>>BOOLEAN OpCodeDisabled;
>>
> 
> I think this patch is incomplete. While the following statements will
> adapt automatically:
> 
>   FormSet->StatementBuffer = AllocateZeroPool (NumberOfStatement * sizeof 
> (FORM_BROWSER_STATEMENT));
> 
>   FormSet->ExpressionBuffer = AllocateZeroPool (NumberOfExpression * sizeof 
> (EXPRESSION_OPCODE));
> 
> I think we'll need to update the indexing into those arrays explicitly.
> Namely,
> 
>> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c 
>> b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
>> index 891b95cf9fb8..9b241ded8cdc 100644
>> --- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
>> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
>> @@ -8,8 +8,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>>  #include "Setup.h"
>>
>> -UINT16   mStatementIndex;
>> -UINT16   mExpressionOpCodeIndex;
>> +UINTNmStatementIndex;
>> +UINTNmExpressionOpCodeIndex;
>>  EFI_QUESTION_ID  mUsedQuestionId;
>>  extern LIST_ENTRY  gBrowserStorageList;
>>  /**
> 
> Do you agree?

Aargh, I missed that that's exactly how the patch starts! Sorry about
the noise :)

Reviewed-by: Laszlo Ersek 

Thanks
Laszlo


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

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

Re: [edk2-devel] [patch] MdeModulePkg/Setup: Update opcode number variable type to UINTN

[Laszlo Ersek]

Hi Dandan, Brian,

On 01/14/20 09:56, Dandan Bi wrote:
> From: Brian R Haug 
>
> Update data type of variables which save the opcode numbers
> to UINTN, in case some configuration module has lots of
> configuration items.
>
> Cc: Liming Gao 
> Cc: Eric Dong 
> Signed-off-by: Brian R Haug 
> Reviewed-by: Dandan Bi 
> ---
>  .../Universal/SetupBrowserDxe/IfrParse.c   | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c 
> b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> index 891b95cf9f..edb6a0fc4c 100644
> --- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> @@ -1,17 +1,17 @@
>  /** @file
>  Parser for IFR binary encoding.
>
> -Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
> +Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  **/
>
>  #include "Setup.h"
>
> -UINT16   mStatementIndex;
> -UINT16   mExpressionOpCodeIndex;
> +UINTNmStatementIndex;
> +UINTNmExpressionOpCodeIndex;
>  EFI_QUESTION_ID  mUsedQuestionId;
>  extern LIST_ENTRY  gBrowserStorageList;
>  /**
>Initialize Statement header members.
>
> @@ -1104,16 +1104,16 @@ IsUnKnownOpCode (
>
>  **/
>  VOID
>  CountOpCodes (
>IN  FORM_BROWSER_FORMSET  *FormSet,
> -  IN OUT  UINT16*NumberOfStatement,
> -  IN OUT  UINT16*NumberOfExpression
> +  OUT  UINTN *NumberOfStatement,
> +  OUT  UINTN *NumberOfExpression
>)
>  {
> -  UINT16  StatementCount;
> -  UINT16  ExpressionCount;
> +  UINTN   StatementCount;
> +  UINTN   ExpressionCount;
>UINT8   *OpCodeData;
>UINTN   Offset;
>UINTN   OpCodeLen;
>
>Offset = 0;
> @@ -1167,12 +1167,12 @@ ParseOpCodes (
>FORMSET_STORAGE *Storage;
>FORMSET_DEFAULTSTORE*DefaultStore;
>QUESTION_DEFAULT*CurrentDefault;
>QUESTION_OPTION *CurrentOption;
>UINT8   Width;
> -  UINT16  NumberOfStatement;
> -  UINT16  NumberOfExpression;
> +  UINTN   NumberOfStatement;
> +  UINTN   NumberOfExpression;
>EFI_IMAGE_ID*ImageId;
>BOOLEAN SuppressForQuestion;
>BOOLEAN SuppressForOption;
>UINT16  DepthOfDisable;
>BOOLEAN OpCodeDisabled;
>

I think this patch is incomplete. While the following statements will
adapt automatically:

  FormSet->StatementBuffer = AllocateZeroPool (NumberOfStatement * sizeof 
(FORM_BROWSER_STATEMENT));

  FormSet->ExpressionBuffer = AllocateZeroPool (NumberOfExpression * sizeof 
(EXPRESSION_OPCODE));

I think we'll need to update the indexing into those arrays explicitly.
Namely,

> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c 
> b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> index 891b95cf9fb8..9b241ded8cdc 100644
> --- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> @@ -8,8 +8,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  #include "Setup.h"
>
> -UINT16   mStatementIndex;
> -UINT16   mExpressionOpCodeIndex;
> +UINTNmStatementIndex;
> +UINTNmExpressionOpCodeIndex;
>  EFI_QUESTION_ID  mUsedQuestionId;
>  extern LIST_ENTRY  gBrowserStorageList;
>  /**

Do you agree?

Thanks
Laszlo


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

View/Reply Online (#53225): https://edk2.groups.io/g/devel/message/53225
Mute This Topic: https://groups.io/mt/69690491/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] ShellPkg: Do not connect handles without device paths

[Laszlo Ersek]

On 01/14/20 11:34, Vitaly Cheptsov via Groups.Io wrote:

> - ConnectControllers
> ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
> 
> This one is more complex, as it supports explicitly connecting specified 
> controllers, however, for connecting all controllers it locates handles with 
> gEfiDevicePathProtocolGuid. I.e. exactly what we ask.

This surprised me, but I think you are right.

Interestingly, the logic goes back to historical commit 4ba49616416a
("Adding Driver1 profile commands to the UEFI Shell 2.0.", 2010-11-12).

Laszlo


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

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

[edk2-devel] [PATCH] MdeModulePkg: Perform test only if not ignore memory test

[Heng Luo]

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

Perform Data and Address line test only if not ignore memory test.

Signed-off-by: Heng Luo 
---
 .../MemoryTest/GenericMemoryTestDxe/LightMemoryTest.c  | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git 
a/MdeModulePkg/Universal/MemoryTest/GenericMemoryTestDxe/LightMemoryTest.c 
b/MdeModulePkg/Universal/MemoryTest/GenericMemoryTestDxe/LightMemoryTest.c
index ce9e5e659b..fe24e490d4 100644
--- a/MdeModulePkg/Universal/MemoryTest/GenericMemoryTestDxe/LightMemoryTest.c
+++ b/MdeModulePkg/Universal/MemoryTest/GenericMemoryTestDxe/LightMemoryTest.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -677,10 +677,12 @@ GenMemoryTestFinished (
   Private = GENERIC_MEMORY_TEST_PRIVATE_FROM_THIS (This);
 
   //
-  // Perform Data and Address line test
+  // Perform Data and Address line test only if not ignore memory test
   //
-  Status = PerformAddressDataLineTest (Private);
-  ASSERT_EFI_ERROR (Status);
+  if (Private->CoverLevel != IGNORE) {
+Status = PerformAddressDataLineTest (Private);
+ASSERT_EFI_ERROR (Status);
+  }
 
   //
   // Add the non tested memory range to system memory map through GCD service
-- 
2.24.0.windows.2


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

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

Re: [edk2-devel] reg: [edk2] [PATCH v1] NetworkPkg/SNPDxe: Validate the Memory BAR as per UEFI Spec

[Maciej Rabeda]

Hello Siva,

Which tools did you use to generate this patch? I cannot apply it via 
'git am' command.
Did you follow the edk2 git guide (using git format-patch && git 
send-email commands)?

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers

Thanks,
Maciej

On 04-Dec-19 11:06, Sivaraman Nainar wrote:


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



This patch is to check the PCI Memory Base Address Register is valid 
or not.


*** a\NetworkPkg\SnpDxe\Snp.c 2019-11-11 16:53:40.773300500 +0530

--- b\NetworkPkg\SnpDxe\Snp.c 2019-11-03 18:46:02.0 +0530

*** SimpleNetworkDriverStart (

*** 266,272 

UINT8 BarIndex;

PXE_STATFLAGS InitStatFlags;

EFI_PCI_IO_PROTOCOL   *PciIo;

! EFI_ACPI_QWORD_ADDRESS_SPACE_DESCRIPTOR   *BarDesc = NULL;

BOOLEAN   FoundIoBar;

BOOLEAN   FoundMemoryBar;

--- 266,272 

UINT8 BarIndex;

   PXE_STATFLAGS InitStatFlags;

EFI_PCI_IO_PROTOCOL   *PciIo;

! EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *BarDesc;

BOOLEAN   FoundIoBar;

    BOOLEAN   FoundMemoryBar;

*** SimpleNetworkDriverStart (

*** 481,494 

  } else if (EFI_ERROR (Status)) {

    goto Error_DeleteSNP;

  }

!  //PXE boot fails with cards having non-continuous BAR.

!      // From UEFI Spec, GetBarAttributes can have only 
descriptors of type ACPI_QWORD_ADDRESS_SPACE_DESCRIPTOR


! // and ACPI_END_TAG_DESCRIPTOR.

! if( BarDesc->Header.Header.Byte != 
ACPI_QWORD_ADDRESS_SPACE_DESCRIPTOR )  {


!   FreePool (BarDesc);

!   continue;

!     }

!

  if ((!FoundMemoryBar) && (BarDesc->ResType == 
ACPI_ADDRESS_SPACE_TYPE_MEM)) {


    Snp->MemoryBarIndex = BarIndex;

    FoundMemoryBar  = TRUE;

--- 481,487 

  } else if (EFI_ERROR (Status)) {

    goto Error_DeleteSNP;

  }

!

  if ((!FoundMemoryBar) && (BarDesc->ResType == 
ACPI_ADDRESS_SPACE_TYPE_MEM)) {


    Snp->MemoryBarIndex = BarIndex;

    FoundMemoryBar  = TRUE;




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

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

[edk2-devel] [PATCHv2 4/4] MdeModulePkg/SdMmcPciHcDxe: Add retries for async commands

[Albecki, Mateusz]

This patch adds retries for async execution for commands that
failed due to the CRC errors.

Cc: Hao A Wu 
Cc: Marcin Wojtas 
Cc: Zhichao Gao 
Cc: Liming Gao 

Signed-off-by: Mateusz Albecki 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index 193b0f24e2..b18ff3e972 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -211,8 +211,10 @@ Done:
   gBS->SignalEvent (TrbEvent);
   return;
 }
-  }
-  if ((Trb != NULL) && (Status != EFI_NOT_READY)) {
+  } else if ((Trb != NULL) && (Status == EFI_CRC_ERROR) && (Trb->Retries > 0)) 
{
+Trb->Retries--;
+Trb->Started = FALSE;
+  } else if ((Trb != NULL)) {
 RemoveEntryList (Link);
 Trb->Packet->TransactionStatus = Status;
 TrbEvent = Trb->Event;
-- 
2.14.1.windows.1



Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


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

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

[edk2-devel] [PATCHv2 0/4] MdeModulePkg/SdMmcPciHcDxe: Retry the commands that failed due to CRC error

[Albecki, Mateusz]

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

Some of the boards report that just after we change the clock frequency to 
200MHz link is unable to stabilize fast enough and when driver sends the CMD13 
it will often fail randomly with CRC error. To protect against this kind of 
random failures this patch series will make the driver retry the commands that 
failed due to random CRC errors.

Since async code has not yet been tested it has been put into separate patch. 
That patch is not needed to solve most pressing CMD13 issues.

changes in v2:
-Split first patch into bugfix and refactor
-Normal interrupt status register will now only be read once during 
SdMmcCheckTrbResult
-We will no longer clear the data transfer timeout error in 
SdMmcCheckAndRecoverErrors. Instead we will immedieatly return for such case 
and register reset will be done in next SdMmcExecTrb

Tets performed:
-Boot eMMC in HS400
-Boot eMMC in HS400 with simulated CRC error on every first CMD13

Cc: Hao A Wu 
Cc: Marcin Wojtas 
Cc: Zhichao Gao 
Cc: Liming Gao 

Signed-off-by: Mateusz Albecki 


Mateusz Albecki (4):
  MdeModulePkg/SdMmcPciHcDxe: Fix DAT lane SW reset
  MdeModulePkg/SdMmcPciHcDxe: Refactor command error detection
  MdeModulePkg/SdMmcPciHcDxe: Add retries for sync commands
  MdeModulePkg/SdMmcPciHcDxe: Add retries for async commands

 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  89 ++---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   5 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 218 ++---
 3 files changed, 204 insertions(+), 108 deletions(-)

-- 
2.14.1.windows.1



Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


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

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

[edk2-devel] [PATCHv2 2/4] MdeModulePkg/SdMmcPciHcDxe: Refactor command error detection

[Albecki, Mateusz]

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

Error detection function will now check if the command
failure has been caused by one of the errors that can
appear randomly on link(CRC error + end bit error). If
such an error has been a cause of failure, function will
return EFI_CRC_ERROR instead of EFI_DEVICE_ERROR to indicate
to the higher level that command has a chance of succeeding if
resent.

Cc: Hao A Wu 
Cc: Marcin Wojtas 
Cc: Zhichao Gao 
Cc: Liming Gao 

Signed-off-by: Mateusz Albecki 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 215 +++
 1 file changed, 140 insertions(+), 75 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index b1f316d444..637455b400 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -2137,6 +2137,137 @@ SdMmcExecTrb (
   return Status;
 }
 
+/**
+  Performs SW reset based on passed error status mask.
+
+  @param[in]  Private   Pointer to driver private data.
+  @param[in]  Slot  Index of the slot to reset.
+  @param[in]  ErrIntStatus  Error interrupt status mask.
+
+  @retval EFI_SUCCESS  Software reset performed successfully.
+  @retval OtherSoftware reset failed.
+**/
+EFI_STATUS
+SdMmcSoftwareReset (
+  IN SD_MMC_HC_PRIVATE_DATA  *Private,
+  IN UINT8   Slot,
+  IN UINT16  ErrIntStatus
+  )
+{
+  UINT8   SwReset;
+  EFI_STATUS  Status;
+
+  SwReset = 0;
+  if ((ErrIntStatus & 0x0F) != 0) {
+SwReset |= BIT1;
+  }
+  if ((ErrIntStatus & 0x70) != 0) {
+SwReset |= BIT2;
+  }
+
+  Status  = SdMmcHcRwMmio (
+  Private->PciIo,
+  Slot,
+  SD_MMC_HC_SW_RST,
+  FALSE,
+  sizeof (SwReset),
+  
+  );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = SdMmcHcWaitMmioSet (
+ Private->PciIo,
+ Slot,
+ SD_MMC_HC_SW_RST,
+ sizeof (SwReset),
+ 0xFF,
+ 0,
+ SD_MMC_HC_GENERIC_TIMEOUT
+ );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Checks the error status in error status register
+  and issues appropriate software reset as described in
+  SD specification section 3.10.
+
+  @param[in] PrivatePointer to driver private data.
+  @param[in] TrbPointer to currently executing TRB.
+  @param[in] IntStatus  Normal interrupt status mask.
+
+  @retval EFI_CRC_ERROR  CRC error happened during CMD execution.
+  @retval EFI_SUCCESSNo error reported.
+  @retval Others Some other error happened.
+
+**/
+EFI_STATUS
+SdMmcCheckAndRecoverErrors (
+  IN SD_MMC_HC_PRIVATE_DATA  *Private,
+  IN UINT8   Slot,
+  IN UINT16  IntStatus
+  )
+{
+  UINT16  ErrIntStatus;
+  EFI_STATUS  Status;
+  EFI_STATUS  ErrorStatus;
+
+  if ((IntStatus & BIT15) == 0) {
+return EFI_SUCCESS;
+  }
+
+  Status = SdMmcHcRwMmio (
+ Private->PciIo,
+ Slot,
+ SD_MMC_HC_ERR_INT_STS,
+ TRUE,
+ sizeof (ErrIntStatus),
+ 
+ );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  //
+  // If the data timeout error is reported
+  // but data transfer is signaled as completed we
+  // have to ignore data timeout. We also assume that no
+  // other error is present on the link since data transfer
+  // completed successfully. Error interrupt status
+  // register is going to be reset when the next command
+  // is started.
+  //
+  if (((ErrIntStatus & BIT4) != 0) && ((IntStatus & BIT1) != 0)) {
+return EFI_SUCCESS;
+  }
+
+  //
+  // We treat both CMD and DAT CRC errors and
+  // end bits errors as EFI_CRC_ERROR. This will
+  // let higher layer know that the error possibly
+  // happened due to random bus condition and the
+  // command can be retried.
+  //
+  if ((ErrIntStatus & (BIT1 | BIT2 | BIT5 | BIT6)) != 0) {
+ErrorStatus = EFI_CRC_ERROR;
+  } else {
+ErrorStatus = EFI_DEVICE_ERROR;
+  }
+
+  Status = SdMmcSoftwareReset (Private, Slot, ErrIntStatus);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  return ErrorStatus;
+}
+
 /**
   Check the TRB execution result.
 
@@ -2160,10 +2291,8 @@ SdMmcCheckTrbResult (
   UINT32  Response[4];
   UINT64  SdmaAddr;
   UINT8   Index;
-  UINT8   SwReset;
   UINT32  PioLength;
 
-  SwReset = 0;
   Packet  = Trb->Packet;
   //
   // Check Trb execution result by reading Normal Interrupt Status register.
@@ -2179,87 +2308,23 @@ SdMmcCheckTrbResult (
   if (EFI_ERROR (Status)) {
 goto Done;
   }
+
   //
-  // Check Transfer Complete bit is set or not.
+  // Check if there are any errors reported by host 

[edk2-devel] [PATCHv2 3/4] MdeModulePkg/SdMmcPciHcDxe: Add retries for sync commands

[Albecki, Mateusz]

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

To increase the resiliency driver will now attempt to
retry the commands that failed due to the CRC error up
to 5 times. This should address the problems with the commands
that fail due to random condition on links. This should also
help the boards on which CMD13 is particularly unstable after
switching the link frequency.

Cc: Hao A Wu 
Cc: Marcin Wojtas 
Cc: Zhichao Gao 
Cc: Liming Gao 

Signed-off-by: Mateusz Albecki 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 83 ++
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |  5 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |  1 +
 3 files changed, 59 insertions(+), 30 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index 373f1bed45..193b0f24e2 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -7,7 +7,7 @@
   It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
 
   Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
-  Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
+  Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -974,6 +974,58 @@ SdMmcPciHcDriverBindingStop (
   return Status;
 }
 
+/**
+  Execute TRB synchronously.
+
+  @param[in] Private  Pointer to driver private data.
+  @param[in] Trb  Pointer to TRB to execute.
+
+  @retval EFI_SUCCESS  TRB executed successfully.
+  @retval OtherTRB failed.
+**/
+EFI_STATUS
+SdMmcPassThruExecSyncTrb (
+  IN SD_MMC_HC_PRIVATE_DATA  *Private,
+  IN SD_MMC_HC_TRB   *Trb
+  )
+{
+  EFI_STATUS  Status;
+  EFI_TPL OldTpl;
+
+  //
+  // Wait async I/O list is empty before execute sync I/O operation.
+  //
+  while (TRUE) {
+OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
+if (IsListEmpty (>Queue)) {
+  gBS->RestoreTPL (OldTpl);
+  break;
+}
+gBS->RestoreTPL (OldTpl);
+  }
+
+  while (Trb->Retries) {
+Status = SdMmcWaitTrbEnv (Private, Trb);
+if (EFI_ERROR (Status)) {
+  return Status;
+}
+
+Status = SdMmcExecTrb (Private, Trb);
+if (EFI_ERROR (Status)) {
+  return Status;
+}
+
+Status = SdMmcWaitTrbResult (Private, Trb);
+if (Status == EFI_CRC_ERROR) {
+  Trb->Retries--;
+} else {
+  return Status;
+}
+  }
+
+  return Status;
+}
+
 /**
   Sends SD command to an SD card that is attached to the SD controller.
 
@@ -1023,7 +1075,6 @@ SdMmcPassThruPassThru (
   EFI_STATUS  Status;
   SD_MMC_HC_PRIVATE_DATA  *Private;
   SD_MMC_HC_TRB   *Trb;
-  EFI_TPL OldTpl;
 
   if ((This == NULL) || (Packet == NULL)) {
 return EFI_INVALID_PARAMETER;
@@ -1066,34 +1117,8 @@ SdMmcPassThruPassThru (
 return EFI_SUCCESS;
   }
 
-  //
-  // Wait async I/O list is empty before execute sync I/O operation.
-  //
-  while (TRUE) {
-OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
-if (IsListEmpty (>Queue)) {
-  gBS->RestoreTPL (OldTpl);
-  break;
-}
-gBS->RestoreTPL (OldTpl);
-  }
-
-  Status = SdMmcWaitTrbEnv (Private, Trb);
-  if (EFI_ERROR (Status)) {
-goto Done;
-  }
-
-  Status = SdMmcExecTrb (Private, Trb);
-  if (EFI_ERROR (Status)) {
-goto Done;
-  }
+  Status = SdMmcPassThruExecSyncTrb (Private, Trb);
 
-  Status = SdMmcWaitTrbResult (Private, Trb);
-  if (EFI_ERROR (Status)) {
-goto Done;
-  }
-
-Done:
   SdMmcFreeTrb (Trb);
 
   return Status;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 0304960132..5bc3577ba2 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -3,7 +3,7 @@
   Provides some data structure definitions used by the SD/MMC host controller 
driver.
 
 Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
-Copyright (c) 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -130,6 +130,8 @@ typedef struct {
 
 #define SD_MMC_HC_TRB_SIG SIGNATURE_32 ('T', 'R', 'B', 'T')
 
+#define SD_MMC_TRB_RETRIES5
+
 //
 // TRB (Transfer Request Block) contains information for the cmd request.
 //
@@ -152,6 +154,7 @@ typedef struct {
   EFI_EVENT   Event;
   BOOLEAN Started;
   UINT64  Timeout;
+  UINT32  Retries;
 
   SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
   SD_MMC_HC_ADMA_64_V3_DESC_LINE  *Adma64V3Desc;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 637455b400..b0384a507a 100644
--- 

[edk2-devel] [PATCHv2 1/4] MdeModulePkg/SdMmcPciHcDxe: Fix DAT lane SW reset

[Albecki, Mateusz]

Driver used to reset the DAT lane on a current error which
is not required according to SD specification(it's not going
to help). This patch will reset the DAT lane only on DAT
lane specific errors.

Cc: Hao A Wu 
Cc: Marcin Wojtas 
Cc: Zhichao Gao 
Cc: Liming Gao 

Signed-off-by: Mateusz Albecki 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index e7f2fac69b..b1f316d444 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -7,7 +7,7 @@
   It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
 
   Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
-  Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
+  Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -2229,7 +2229,7 @@ SdMmcCheckTrbResult (
 if ((IntStatus & 0x0F) != 0) {
   SwReset |= BIT1;
 }
-if ((IntStatus & 0xF0) != 0) {
+if ((IntStatus & 0x70) != 0) {
   SwReset |= BIT2;
 }
 
-- 
2.14.1.windows.1



Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


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

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

Re: [edk2-devel] [PATCH 0/2] UefiBootManagerLib, HttpDxe: tweaks for large HTTP(S) downloads

[Laszlo Ersek]

On 01/09/20 00:43, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: tweaks_for_large_http
> 
> This series aims to improve HTTP(S) Boot experience with large (4GiB+)
> files.
> 
> Cc: Hao A Wu 
> Cc: Jian J Wang 
> Cc: Jiaxin Wu 
> Cc: Maciej Rabeda 
> Cc: Ray Ni 
> Cc: Siyuan Fu 
> Cc: Zhichao Gao 
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (2):
>   MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure
>   NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download
> 
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 31 
>  NetworkPkg/HttpDxe/HttpImpl.c|  2 +-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 

Merged: commit range b112ec225f1c..4cca7923992a.

Thanks!
Laszlo


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

View/Reply Online (#53215): https://edk2.groups.io/g/devel/message/53215
Mute This Topic: https://groups.io/mt/69550071/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] ShellPkg: Do not connect handles without device paths

[Vitaly Cheptsov via Groups.Io]

In addition to my previous letter I have to mention a couple more newly 
discovered details.

1. UEFI Shell (ShellPkg) actually has 3 functions dedicated to connecting 
controllers and essentially doing the same thing:

- ConnectAllEfi
ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c

This one locates all handles and runs connect on them. It is the one we 
mentioned in the bug.

- LoadPciRomConnectAllDriversToAllControllers
ShellPkg/Library/UefiShellDebug1CommandsLib/LoadPciRom.c

This one is similar to ConnectAllEfi. The only difference is that it 
additionally checks for Ctrl+C via ShellGetExecutionBreakFlag.

- ConnectControllers
ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c

This one is more complex, as it supports explicitly connecting specified 
controllers, however, for connecting all controllers it locates handles with 
gEfiDevicePathProtocolGuid. I.e. exactly what we ask. I believe that all these 
functions should behave the same way in correspondence to 
gBS->ConnectController at the very least, and most likely there should be a 
library responsible for connection and disconnection.

2. We also checked EFI 1.1 Shell, and confirmed that it consistently has checks 
for device path presence before running connect on the handle[1][2]. This makes 
us believe that our proposal is not really a firmware bug, but rather a 
limitation of the legacy specification.

Considering all these discoveries, we believe our suggested change is legit 
depending on the minimum supported UEFI version. Therefore we propose 
submitting a ControllerConnection library with 5 functions:

- ConnectController
- DisconnectController
- GetAllControllers
- ConnectAllControllers
- DisconnectAllControllers

Adopting it throughout the code will let the distribitor to be responsible for 
choosing what is right for his applications (or firmwares).

Best wishes,
Vitaly

[1] 
https://sourceforge.net/p/efi-shell/code/64/tree/trunk/Shell/LoadPciRom/LoadPciRom.c#l528
[2] 
https://sourceforge.net/p/efi-shell/code/64/tree/trunk/Shell/load/load.c#l312


> 14 янв. 2020 г., в 11:36, vit9696  написал(а):
> 
> Ray,
> 
> We are quite reluctant to have patches in EDK II for a large amount of widely 
> adopted firmwares. Patches eventually break and require maintenance cost, and 
> currently we are trying to get rid of them all. We believe that EDK II Shell 
> is supposed to work on real world platforms and not only the ones that 
> theoretically support the specification. It is always hard to adopt changes 
> based on third-party bugs, and we very well understand your concern, yet it 
> is something we have to do to stay beneficial to the end user.
> 
> Best wishes,
> Vitaly
> 
> On Tue, Jan 14, 2020 at 05:53, Ni, Ray  > wrote:
>> 
>> Vitaly,
>> 
>> I still have concern to modify the EDKII code to workaround a firmware bug.
>> 
>> Can you just change in your local version?
>> 
>>
>> Thanks,
>> 
>> Ray
>> 
>>
>> From: devel@edk2.groups.io  On Behalf Of Vitaly 
>> Cheptsov via Groups.Io
>> Sent: Tuesday, January 14, 2020 4:47 AM
>> To: Laszlo Ersek ; devel@edk2.groups.io; Ni, Ray 
>> ; Gao, Zhichao 
>> Subject: Re: [edk2-devel] [PATCH 1/1] ShellPkg: Do not connect handles 
>> without device paths
>> 
>>
>> Thanks all for your input,
>>
>> These explanations seem sufficient to us that it is not a good idea to 
>> change the behaviour for everyone. Even so, we still need this to be 
>> configurable in some way, as having to patch EDK II is impracticable.
>>
>> We believe there are three possible routes to approach this problem:
>>
>> Introduce a separate ControllerConnectionLib library for this function. 
>> While it is small, we found several places in our code that need to call it 
>> beyond UEFI Shell. This way different implementations could be used 
>> depending on the chosen library.
>> Introduce a ConnectRequiresDevicePath PCD, which will choose the preferred 
>> logic.
>> Introduce a -dp Shell argument for affected commands the way Lazslo 
>> suggested.
>>
>> We believe either route or a combination of multiple routes have their own 
>> benefits, and would suggest either going with 1+2 or with 3. Any approach is 
>> fine for us.
>>
>> We will submit V2 of the patch after hearing the opinions.
>>
>> Best wishes,
>> Vitaly
>>
>> On Mon, Jan 13, 2020 at 20:55, Laszlo Ersek > > wrote:
>> 
>> On 01/13/20 12:56, Ni, Ray wrote:
>> > We shouldn't assume that a DriverBindingStart() can only start on a handle 
>> > with device path installed. DevicePath protocol is just a special protocol.
>> > It's possible that a bus driver starts on a host controller handle and 
>> > creates multiple children, each with only a Specific_IO protocol installed.
>> > Certain device driver can start on the children handle and open the 
>> > Specific_IO protocol BY_DRIVER.
>> > I am not sure if certain today's network drivers may work like this. It's 
>> > allowed per UEFI spec.
>> 
>> I agree.
>> 
>> Under 

Re: [edk2-devel] [PATCH v2] CryptoPkg/BaseCryptLib: remove HmacXxxGetContextSize interface

[Laszlo Ersek]

On 01/14/20 07:26, Jian J Wang wrote:
>> v2 changes:
>> - change HmacXxxInit to HmacXxxSetKey
>> - remove calling to HMAC_CTX_reset() since HmacXxxNew() has done it
>>   already and user-supplied context is not supported any longer.
>> - update comments affected by above change
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1792
> 
> Hmac(Md5|Sha1|Sha256)GetContextSize() use a deprecated macro
> HMAC_MAX_MD_CBLOCK defined in openssl. They should be dropped to
> avoid misuses in the future. For context allocation and release,
> use HmacXxxNew() and HmacXxxFree() instead.
> 
> Considering that HmacXxxInit() has no use cases, remove the calling
> to memset() and HMAC_CTX_reset() to drop the support of user supplied
> context buffer. The only functionality left is to set user supplied
> key for HMAC. To avoid confusion, change the the its name to
> HmacXxxSetKey.
> 
> Cc: Xiaoyu Lu 
> Cc: Laszlo Ersek 
> Signed-off-by: Jian J Wang 
> ---
>  CryptoPkg/Include/Library/BaseCryptLib.h  | 111 +-
>  .../Library/BaseCryptLib/Hmac/CryptHmacMd5.c  |  58 ++---
>  .../BaseCryptLib/Hmac/CryptHmacMd5Null.c  |  28 +
>  .../Library/BaseCryptLib/Hmac/CryptHmacSha1.c |  59 ++
>  .../BaseCryptLib/Hmac/CryptHmacSha1Null.c |  28 +
>  .../BaseCryptLib/Hmac/CryptHmacSha256.c   |  58 ++---
>  .../BaseCryptLib/Hmac/CryptHmacSha256Null.c   |  28 +
>  .../BaseCryptLibNull/Hmac/CryptHmacMd5Null.c  |  20 
>  .../BaseCryptLibNull/Hmac/CryptHmacSha1Null.c |  20 
>  .../Hmac/CryptHmacSha256Null.c|  20 
>  10 files changed, 72 insertions(+), 358 deletions(-)
> 
> diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h 
> b/CryptoPkg/Include/Library/BaseCryptLib.h
> index 8fe303a0b3..b93f1b9009 100644
> --- a/CryptoPkg/Include/Library/BaseCryptLib.h
> +++ b/CryptoPkg/Include/Library/BaseCryptLib.h
> @@ -1025,23 +1025,6 @@ Sm3HashAll (
>  //MAC (Message Authentication Code) Primitive
>  
> //=
>  
> -/**
> -  Retrieves the size, in bytes, of the context buffer required for HMAC-MD5 
> operations.
> -  (NOTE: This API is deprecated.
> - Use HmacMd5New() / HmacMd5Free() for HMAC-MD5 Context operations.)
> -
> -  If this interface is not supported, then return zero.
> -
> -  @return  The size, in bytes, of the context buffer required for HMAC-MD5 
> operations.
> -  @retval  0   This interface is not supported.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacMd5GetContextSize (
> -  VOID
> -  );
> -
>  /**
>Allocates and initializes one HMAC_CTX context for subsequent HMAC-MD5 use.
>  
> @@ -1073,24 +1056,24 @@ HmacMd5Free (
>);
>  
>  /**
> -  Initializes user-supplied memory pointed by HmacMd5Context as HMAC-MD5 
> context for
> -  subsequent use.
> +  Set user-supplied key for subsequent use. It must be done before any
> +  calling to HmacMd5Update().
>  
>If HmacMd5Context is NULL, then return FALSE.
>If this interface is not supported, then return FALSE.
>  
> -  @param[out]  HmacMd5Context  Pointer to HMAC-MD5 context being initialized.
> +  @param[out]  HmacMd5Context  Pointer to HMAC-MD5 context.
>@param[in]   Key Pointer to the user-supplied key.
>@param[in]   KeySize Key size in bytes.
>  
> -  @retval TRUE   HMAC-MD5 context initialization succeeded.
> -  @retval FALSE  HMAC-MD5 context initialization failed.
> +  @retval TRUE   Key is set succeefully.
> +  @retval FALSE  Key is set unsucceefully.

(1) Please fix the typos: it should be "successfully" and "unsuccessfully".

>@retval FALSE  This interface is not supported.
>  
>  **/
>  BOOLEAN
>  EFIAPI
> -HmacMd5Init (
> +HmacMd5SetKey (
>OUT  VOID *HmacMd5Context,
>IN   CONST UINT8  *Key,
>IN   UINTNKeySize
> @@ -1123,8 +1106,8 @@ HmacMd5Duplicate (
>  
>This function performs HMAC-MD5 digest on a data buffer of the specified 
> size.
>It can be called multiple times to compute the digest of long or 
> discontinuous data streams.
> -  HMAC-MD5 context should be already correctly initialized by HmacMd5Init(), 
> and should not be
> -  finalized by HmacMd5Final(). Behavior with invalid context is undefined.
> +  HMAC-MD5 context should be initialized by HmacMd5New(), and should not be 
> finalized by
> +  HmacMd5Final(). Behavior with invalid context is undefined.
>  
>If HmacMd5Context is NULL, then return FALSE.
>If this interface is not supported, then return FALSE.
> @@ -1152,8 +1135,8 @@ HmacMd5Update (
>This function completes HMAC-MD5 hash computation and retrieves the digest 
> value into
>the specified memory. After this function has been called, the HMAC-MD5 
> context cannot
>be used again.
> -  HMAC-MD5 context should be already correctly initialized by HmacMd5Init(), 
> and should not be
> -  finalized by HmacMd5Final(). Behavior with invalid HMAC-MD5 context is 
> undefined.
> +  HMAC-MD5 

[edk2-devel] [patch] MdeModulePkg/Setup: Update opcode number variable type to UINTN

[Dandan Bi]

From: Brian R Haug 

Update data type of variables which save the opcode numbers
to UINTN, in case some configuration module has lots of
configuration items.

Cc: Liming Gao 
Cc: Eric Dong 
Signed-off-by: Brian R Haug 
Reviewed-by: Dandan Bi 
---
 .../Universal/SetupBrowserDxe/IfrParse.c   | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c 
b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
index 891b95cf9f..edb6a0fc4c 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
@@ -1,17 +1,17 @@
 /** @file
 Parser for IFR binary encoding.
 
-Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
+Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include "Setup.h"
 
-UINT16   mStatementIndex;
-UINT16   mExpressionOpCodeIndex;
+UINTNmStatementIndex;
+UINTNmExpressionOpCodeIndex;
 EFI_QUESTION_ID  mUsedQuestionId;
 extern LIST_ENTRY  gBrowserStorageList;
 /**
   Initialize Statement header members.
 
@@ -1104,16 +1104,16 @@ IsUnKnownOpCode (
 
 **/
 VOID
 CountOpCodes (
   IN  FORM_BROWSER_FORMSET  *FormSet,
-  IN OUT  UINT16*NumberOfStatement,
-  IN OUT  UINT16*NumberOfExpression
+  OUT  UINTN *NumberOfStatement,
+  OUT  UINTN *NumberOfExpression
   )
 {
-  UINT16  StatementCount;
-  UINT16  ExpressionCount;
+  UINTN   StatementCount;
+  UINTN   ExpressionCount;
   UINT8   *OpCodeData;
   UINTN   Offset;
   UINTN   OpCodeLen;
 
   Offset = 0;
@@ -1167,12 +1167,12 @@ ParseOpCodes (
   FORMSET_STORAGE *Storage;
   FORMSET_DEFAULTSTORE*DefaultStore;
   QUESTION_DEFAULT*CurrentDefault;
   QUESTION_OPTION *CurrentOption;
   UINT8   Width;
-  UINT16  NumberOfStatement;
-  UINT16  NumberOfExpression;
+  UINTN   NumberOfStatement;
+  UINTN   NumberOfExpression;
   EFI_IMAGE_ID*ImageId;
   BOOLEAN SuppressForQuestion;
   BOOLEAN SuppressForOption;
   UINT16  DepthOfDisable;
   BOOLEAN OpCodeDisabled;
-- 
2.18.0.windows.1


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

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

Re: [edk2-devel] [Patch] BaseTools/Scripts/PatchCheck: Address false error conditions

[Laszlo Ersek]

On 01/13/20 19:38, Kinney, Michael D wrote:
> Hi Laszlo,
> 
> This patch has been pushed.
> 
> https://github.com/tianocore/edk2/pull/297
> 
> I also entered a new BZ to relax CVE pattern check.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2462

Thank you for both!
Laszlo


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

View/Reply Online (#53211): https://edk2.groups.io/g/devel/message/53211
Mute This Topic: https://groups.io/mt/69610631/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] ShellPkg: Do not connect handles without device paths

[Vitaly Cheptsov via Groups.Io]

Ray,

We are quite reluctant to have patches in EDK II for a large amount of widely 
adopted firmwares. Patches eventually break and require maintenance cost, and 
currently we are trying to get rid of them all. We believe that EDK II Shell is 
supposed to work on real world platforms and not only the ones that 
theoretically support the specification. It is always hard to adopt changes 
based on third-party bugs, and we very well understand your concern, yet it is 
something we have to do to stay beneficial to the end user.

Best wishes,
Vitaly

On Tue, Jan 14, 2020 at 05:53, Ni, Ray  wrote:

> Vitaly,
>
> I still have concern to modify the EDKII code to workaround a firmware bug.
>
> Can you just change in your local version?
>
> Thanks,
>
> Ray
>
> From: devel@edk2.groups.io   On Behalf Of Vitaly 
> Cheptsov via Groups.Io
> Sent: Tuesday, January 14, 2020 4:47 AM
> To: Laszlo Ersek ; devel@edk2.groups.io; Ni, Ray 
> ; Gao, Zhichao 
> Subject: Re: [edk2-devel] [PATCH 1/1] ShellPkg: Do not connect handles 
> without device paths
>
> Thanks all for your input,
>
> These explanations seem sufficient to us that it is not a good idea to change 
> the behaviour for everyone. Even so, we still need this to be configurable in 
> some way, as having to patch EDK II is impracticable.
>
> We believe there are three possible routes to approach this problem:
>
> -  Introduce a separate ControllerConnectionLib library for this function. 
> While it is small, we found several places in our code that need to call it 
> beyond UEFI Shell. This way different implementations could be used depending 
> on the chosen library.
> -  Introduce a ConnectRequiresDevicePath PCD, which will choose the preferred 
> logic.
> -  Introduce a -dp Shell argument for affected commands the way Lazslo 
> suggested.
>
> We believe either route or a combination of multiple routes have their own 
> benefits, and would suggest either going with 1+2 or with 3. Any approach is 
> fine for us.
>
> We will submit V2 of the patch after hearing the opinions.
>
> Best wishes,
>
> Vitaly
>
> On Mon, Jan 13, 2020 at 20:55, Laszlo Ersek  wrote:
>
>> On 01/13/20 12:56, Ni, Ray wrote:
>>> We shouldn't assume that a DriverBindingStart() can only start on a handle 
>>> with device path installed. DevicePath protocol is just a special protocol.
>>> It's possible that a bus driver starts on a host controller handle and 
>>> creates multiple children, each with only a Specific_IO protocol installed.
>>> Certain device driver can start on the children handle and open the 
>>> Specific_IO protocol BY_DRIVER.
>>> I am not sure if certain today's network drivers may work like this. It's 
>>> allowed per UEFI spec.
>>
>> I agree.
>>
>> Under "10.2 EFI Device Path Protocol", the spec says, "If the handle
>> does not logically map to a physical device, the handle may not
>> necessarily support the device path protocol."
>>
>> I think gBS->ConnectController() and
>> EFI_DRIVER_BINDING_PROTOCOL.Supported() should work on such handles.
>>
>> If we'd like to work around related issues in drivers, then I'd suggest
>> new command line options for the "load", "connect", "reconnect" shell
>> commands (maybe more), for filtering out handles that do not carry
>> device paths. Such command line options could be added as an extension,
>> IIUC, such as "-_option". I.e., I believe it's not necessary to start
>> with UEFI Shell Spec updates.
>>
>> Thanks
>> Laszlo
>
> 
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

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