[RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[Leif Lindholm]

Commit 05fd2a926833
("MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList LockBox")
added a dependency on LockBoxLib to NvmExpressPei, causing builds using
MdeModulePkg.dsc to fail on architectures other than IA32/X64 with
missing reference to
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode.

Add a resolution for LockBoxNullLib for ARM/AARCH64 to restore builds.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---

Note: this patch hides the symptom, but this isn't really the fix I
would like to see.

The build error is caused by the chain of:
1) NvmExpressPei depending on LockBoxLib
2) LockBoxLib being mapped to SmmLockBoxPeiLib in [LibraryClasses.common.PEIM]
3) SmmLockBoxPeiLib depending on PcdDxeIplSwitchToLongMode
4) PcdDxeIplSwitchToLongMode being declared in
   [PcdsFeatureFlag.IA32, PcdsFeatureFlag.X64] in MdeModulePkg.dsc

Now, an alternative quick-fix would be to move the PEIM LockBoxLib mapping
into a [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM]
section. But that would leave NvmExpressPei unbuildable on anything not
IA32/X64.

Another option would be to add default declaration (for all other
architectures) of FALSE for PcdDxeIplSwitchToLongMode in MdeModulePkg.dec,
but the current way this is expressed seems to treat this as an
architecture-specific feature (which it is).

What I believe would be the cleanest solution would be to abstract
NvmExpressPei to the point where it can function without the LockBoxLib.
But regardless, it does not look valid to me for something as
architecture-specific as MdeModulePkg/Library/SmmLockBoxLib/ to live under
.common sections in the .dsc. (And if this changes at some point, because we implement an ARM/AARCH64 equivalent based on StandaloneMmPkg, we will need
a major refactoring of that library anyway.)

/
    Leif

MdeModulePkg/MdeModulePkg.dsc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 6cd1727a0d..6e27e9cb68 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -178,6 +178,7 @@ [LibraryClasses.common.MM_STANDALONE]
 [LibraryClasses.ARM, LibraryClasses.AARCH64]
   ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
   ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+  LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
 
   #
   # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
-- 
2.11.0

Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[Laszlo Ersek]

Hi Leif,

On 03/18/19 15:56, Leif Lindholm wrote:
> Commit 05fd2a926833
> ("MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList LockBox")
> added a dependency on LockBoxLib to NvmExpressPei, causing builds using
> MdeModulePkg.dsc to fail on architectures other than IA32/X64 with
> missing reference to
> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode.
> 
> Add a resolution for LockBoxNullLib for ARM/AARCH64 to restore builds.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> 
> Note: this patch hides the symptom, but this isn't really the fix I
> would like to see.
> 
> The build error is caused by the chain of:
> 1) NvmExpressPei depending on LockBoxLib
> 2) LockBoxLib being mapped to SmmLockBoxPeiLib in [LibraryClasses.common.PEIM]
> 3) SmmLockBoxPeiLib depending on PcdDxeIplSwitchToLongMode
> 4) PcdDxeIplSwitchToLongMode being declared in
>    [PcdsFeatureFlag.IA32, PcdsFeatureFlag.X64] in MdeModulePkg.dsc
> 
> Now, an alternative quick-fix would be to move the PEIM LockBoxLib mapping
> into a [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM]
> section. But that would leave NvmExpressPei unbuildable on anything not
> IA32/X64.
> 
> Another option would be to add default declaration (for all other
> architectures) of FALSE for PcdDxeIplSwitchToLongMode in MdeModulePkg.dec,
> but the current way this is expressed seems to treat this as an
> architecture-specific feature (which it is).
> 
> What I believe would be the cleanest solution would be to abstract
> NvmExpressPei to the point where it can function without the LockBoxLib.
> But regardless, it does not look valid to me for something as
> architecture-specific as MdeModulePkg/Library/SmmLockBoxLib/ to live under
> .common sections in the .dsc. (And if this changes at some point, because we implement an ARM/AARCH64 equivalent based on StandaloneMmPkg, we will need
> a major refactoring of that library anyway.)
> 
> /
>     Leif
> 
> MdeModulePkg/MdeModulePkg.dsc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
> index 6cd1727a0d..6e27e9cb68 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -178,6 +178,7 @@ [LibraryClasses.common.MM_STANDALONE]
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
>    ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +  LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
>  
>    #
>    # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
> 

I think this patch is exactly the right solution.

The code added in commit 05fd2a926833 is gated by (BootMode ==
BOOT_ON_S3_RESUME). That condition can never evaluate to TRUE on
ARM/AARCH64, presently. Accordingly, the stated goal of the commit
doesn't apply to ARM/AARCH64:

    The purpose is to perform an on-demand (partial) NVM Express device
    enumeration/initialization to benefit the S3 resume performance.

Given that the RestoreLockBox() calls are never reached (which is
correct, by design, at the present level of ACPI S3 enablement in edk2
for ARM/AARCH64), causing the lockbox APIs to "do nothing beyond
compile" is exactly right. IMO anyway.

Once ARM/AARCH64 grow S3 support, a functional and secure LockBox will
have to be part of that. Perhaps it will use "standalone MM"; I'm not
sure. The point is, once the goal of the commit starts applying to
ARM/AARCH64, a functional LockBox will have been implemented for
ARM/AARCH64; and that lib instance will certainly not depend on
PcdDxeIplSwitchToLongMode.

Until such time, this patch is fine.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[Zeng, Star]

Same case for AhciPei.

I am ok with the patch. Reviewed-by: Star Zeng <star.zeng@intel.com>

Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Wednesday, March 20, 2019 10:52 PM
To: Leif Lindholm <leif.lindholm@linaro.org>; edk2-devel@lists.01.org
Cc: ard.biesheuvel@linaro.org; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

Hi Leif,

On 03/18/19 15:56, Leif Lindholm wrote:
> Commit 05fd2a926833
> ("MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList 
> LockBox") added a dependency on LockBoxLib to NvmExpressPei, causing 
> builds using MdeModulePkg.dsc to fail on architectures other than 
> IA32/X64 with missing reference to 
> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode.
> 
> Add a resolution for LockBoxNullLib for ARM/AARCH64 to restore builds.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> 
> Note: this patch hides the symptom, but this isn't really the fix I 
> would like to see.
> 
> The build error is caused by the chain of:
> 1) NvmExpressPei depending on LockBoxLib
> 2) LockBoxLib being mapped to SmmLockBoxPeiLib in 
> [LibraryClasses.common.PEIM]
> 3) SmmLockBoxPeiLib depending on PcdDxeIplSwitchToLongMode
> 4) PcdDxeIplSwitchToLongMode being declared in
>    [PcdsFeatureFlag.IA32, PcdsFeatureFlag.X64] in MdeModulePkg.dsc
> 
> Now, an alternative quick-fix would be to move the PEIM LockBoxLib 
> mapping into a [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM] 
> section. But that would leave NvmExpressPei unbuildable on anything 
> not IA32/X64.
> 
> Another option would be to add default declaration (for all other
> architectures) of FALSE for PcdDxeIplSwitchToLongMode in 
> MdeModulePkg.dec, but the current way this is expressed seems to treat 
> this as an architecture-specific feature (which it is).
> 
> What I believe would be the cleanest solution would be to abstract 
> NvmExpressPei to the point where it can function without the LockBoxLib.
> But regardless, it does not look valid to me for something as 
> architecture-specific as MdeModulePkg/Library/SmmLockBoxLib/ to live 
> under .common sections in the .dsc. (And if this changes at some 
> point, because we implement an ARM/AARCH64 equivalent based on 
> StandaloneMmPkg, we will need a major refactoring of that library 
> anyway.)
> 
> /
>     Leif
> 
> MdeModulePkg/MdeModulePkg.dsc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dsc 
> b/MdeModulePkg/MdeModulePkg.dsc index 6cd1727a0d..6e27e9cb68 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -178,6 +178,7 @@ [LibraryClasses.common.MM_STANDALONE]
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
>    ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +  LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
>  
>    #
>    # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
> 

I think this patch is exactly the right solution.

The code added in commit 05fd2a926833 is gated by (BootMode == BOOT_ON_S3_RESUME). That condition can never evaluate to TRUE on ARM/AARCH64, presently. Accordingly, the stated goal of the commit doesn't apply to ARM/AARCH64:

    The purpose is to perform an on-demand (partial) NVM Express device
    enumeration/initialization to benefit the S3 resume performance.

Given that the RestoreLockBox() calls are never reached (which is correct, by design, at the present level of ACPI S3 enablement in edk2 for ARM/AARCH64), causing the lockbox APIs to "do nothing beyond compile" is exactly right. IMO anyway.

Once ARM/AARCH64 grow S3 support, a functional and secure LockBox will have to be part of that. Perhaps it will use "standalone MM"; I'm not sure. The point is, once the goal of the commit starts applying to ARM/AARCH64, a functional LockBox will have been implemented for ARM/AARCH64; and that lib instance will certainly not depend on PcdDxeIplSwitchToLongMode.

Until such time, this patch is fine.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[Leif Lindholm]

On Wed, Mar 20, 2019 at 03:51:39PM +0100, Laszlo Ersek wrote:
> Hi Leif,
> 
> On 03/18/19 15:56, Leif Lindholm wrote:
> > Commit 05fd2a926833
> > ("MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList LockBox")
> > added a dependency on LockBoxLib to NvmExpressPei, causing builds using
> > MdeModulePkg.dsc to fail on architectures other than IA32/X64 with
> > missing reference to
> > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode.
> > 
> > Add a resolution for LockBoxNullLib for ARM/AARCH64 to restore builds.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> > ---
> > 
> > Note: this patch hides the symptom, but this isn't really the fix I
> > would like to see.
> > 
> > The build error is caused by the chain of:
> > 1) NvmExpressPei depending on LockBoxLib
> > 2) LockBoxLib being mapped to SmmLockBoxPeiLib in [LibraryClasses.common.PEIM]
> > 3) SmmLockBoxPeiLib depending on PcdDxeIplSwitchToLongMode
> > 4) PcdDxeIplSwitchToLongMode being declared in
> >    [PcdsFeatureFlag.IA32, PcdsFeatureFlag.X64] in MdeModulePkg.dsc
> > 
> > Now, an alternative quick-fix would be to move the PEIM LockBoxLib mapping
> > into a [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM]
> > section. But that would leave NvmExpressPei unbuildable on anything not
> > IA32/X64.
> > 
> > Another option would be to add default declaration (for all other
> > architectures) of FALSE for PcdDxeIplSwitchToLongMode in MdeModulePkg.dec,
> > but the current way this is expressed seems to treat this as an
> > architecture-specific feature (which it is).
> > 
> > What I believe would be the cleanest solution would be to abstract
> > NvmExpressPei to the point where it can function without the LockBoxLib.
> > But regardless, it does not look valid to me for something as
> > architecture-specific as MdeModulePkg/Library/SmmLockBoxLib/ to live under
> > .common sections in the .dsc. (And if this changes at some point, because we implement an ARM/AARCH64 equivalent based on StandaloneMmPkg, we will need
> > a major refactoring of that library anyway.)
> > 
> > /
> >     Leif
> > 
> > MdeModulePkg/MdeModulePkg.dsc | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
> > index 6cd1727a0d..6e27e9cb68 100644
> > --- a/MdeModulePkg/MdeModulePkg.dsc
> > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > @@ -178,6 +178,7 @@ [LibraryClasses.common.MM_STANDALONE]
> >  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> >    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> >    ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> > +  LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> >  
> >    #
> >    # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
> > 
> 
> I think this patch is exactly the right solution.
> 
> The code added in commit 05fd2a926833 is gated by (BootMode ==
> BOOT_ON_S3_RESUME). That condition can never evaluate to TRUE on
> ARM/AARCH64, presently. Accordingly, the stated goal of the commit
> doesn't apply to ARM/AARCH64:
> 
>     The purpose is to perform an on-demand (partial) NVM Express device
>     enumeration/initialization to benefit the S3 resume performance.
> 
> Given that the RestoreLockBox() calls are never reached (which is
> correct, by design, at the present level of ACPI S3 enablement in edk2
> for ARM/AARCH64), causing the lockbox APIs to "do nothing beyond
> compile" is exactly right. IMO anyway.
> 
> Once ARM/AARCH64 grow S3 support, a functional and secure LockBox will
> have to be part of that. Perhaps it will use "standalone MM"; I'm not
> sure. The point is, once the goal of the commit starts applying to
> ARM/AARCH64, a functional LockBox will have been implemented for
> ARM/AARCH64; and that lib instance will certainly not depend on
> PcdDxeIplSwitchToLongMode.
> 
> Until such time, this patch is fine.

OK, I buy that argument.

*But* I still think the IA32/X64 specific library mappings should be
moved out of .common LibraryClass sections.

Regards,

Leif

> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> Thanks
> Laszlo

Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[Zeng, Star]

Another way to update the file is

[LibraryClasses.EBC]
  LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf

->

[LibraryClasses.EBC, LibraryClasses.ARM, LibraryClasses.AARCH64]
  LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf


Thanks,
Star
-----Original Message-----
From: Leif Lindholm [mailto:leif.lindholm@linaro.org] 
Sent: Thursday, March 21, 2019 1:43 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

On Wed, Mar 20, 2019 at 03:51:39PM +0100, Laszlo Ersek wrote:
> Hi Leif,
> 
> On 03/18/19 15:56, Leif Lindholm wrote:
> > Commit 05fd2a926833
> > ("MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList 
> > LockBox") added a dependency on LockBoxLib to NvmExpressPei, causing 
> > builds using MdeModulePkg.dsc to fail on architectures other than 
> > IA32/X64 with missing reference to 
> > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode.
> > 
> > Add a resolution for LockBoxNullLib for ARM/AARCH64 to restore builds.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> > ---
> > 
> > Note: this patch hides the symptom, but this isn't really the fix I 
> > would like to see.
> > 
> > The build error is caused by the chain of:
> > 1) NvmExpressPei depending on LockBoxLib
> > 2) LockBoxLib being mapped to SmmLockBoxPeiLib in 
> > [LibraryClasses.common.PEIM]
> > 3) SmmLockBoxPeiLib depending on PcdDxeIplSwitchToLongMode
> > 4) PcdDxeIplSwitchToLongMode being declared in
> >    [PcdsFeatureFlag.IA32, PcdsFeatureFlag.X64] in MdeModulePkg.dsc
> > 
> > Now, an alternative quick-fix would be to move the PEIM LockBoxLib 
> > mapping into a [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM] 
> > section. But that would leave NvmExpressPei unbuildable on anything 
> > not IA32/X64.
> > 
> > Another option would be to add default declaration (for all other
> > architectures) of FALSE for PcdDxeIplSwitchToLongMode in 
> > MdeModulePkg.dec, but the current way this is expressed seems to 
> > treat this as an architecture-specific feature (which it is).
> > 
> > What I believe would be the cleanest solution would be to abstract 
> > NvmExpressPei to the point where it can function without the LockBoxLib.
> > But regardless, it does not look valid to me for something as 
> > architecture-specific as MdeModulePkg/Library/SmmLockBoxLib/ to live 
> > under .common sections in the .dsc. (And if this changes at some 
> > point, because we implement an ARM/AARCH64 equivalent based on 
> > StandaloneMmPkg, we will need a major refactoring of that library 
> > anyway.)
> > 
> > /
> >     Leif
> > 
> > MdeModulePkg/MdeModulePkg.dsc | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/MdeModulePkg/MdeModulePkg.dsc 
> > b/MdeModulePkg/MdeModulePkg.dsc index 6cd1727a0d..6e27e9cb68 100644
> > --- a/MdeModulePkg/MdeModulePkg.dsc
> > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > @@ -178,6 +178,7 @@ [LibraryClasses.common.MM_STANDALONE]
> >  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> >    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> >    ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> > +  LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> >  
> >    #
> >    # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
> > 
> 
> I think this patch is exactly the right solution.
> 
> The code added in commit 05fd2a926833 is gated by (BootMode == 
> BOOT_ON_S3_RESUME). That condition can never evaluate to TRUE on 
> ARM/AARCH64, presently. Accordingly, the stated goal of the commit 
> doesn't apply to ARM/AARCH64:
> 
>     The purpose is to perform an on-demand (partial) NVM Express device
>     enumeration/initialization to benefit the S3 resume performance.
> 
> Given that the RestoreLockBox() calls are never reached (which is 
> correct, by design, at the present level of ACPI S3 enablement in edk2 
> for ARM/AARCH64), causing the lockbox APIs to "do nothing beyond 
> compile" is exactly right. IMO anyway.
> 
> Once ARM/AARCH64 grow S3 support, a functional and secure LockBox will 
> have to be part of that. Perhaps it will use "standalone MM"; I'm not 
> sure. The point is, once the goal of the commit starts applying to 
> ARM/AARCH64, a functional LockBox will have been implemented for 
> ARM/AARCH64; and that lib instance will certainly not depend on 
> PcdDxeIplSwitchToLongMode.
> 
> Until such time, this patch is fine.

OK, I buy that argument.

*But* I still think the IA32/X64 specific library mappings should be moved out of .common LibraryClass sections.

Regards,

Leif

> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> Thanks
> Laszlo

Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[Wu, Hao A]

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, March 21, 2019 9:03 AM
> To: Leif Lindholm; Laszlo Ersek
> Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org; Wang, Jian J; Wu,
> Hao A; Ni, Ray; Andrew Fish; Kinney, Michael D; Zeng, Star
> Subject: RE: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64
> in .dsc
> 
> Another way to update the file is
> 
> [LibraryClasses.EBC]
>   LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> 
> ->
> 
> [LibraryClasses.EBC, LibraryClasses.ARM, LibraryClasses.AARCH64]
>   LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf

Hello Leif,

The current proposed patch seems great to me.
Reviewed-by: Hao Wu <hao.a.wu@intel.com>

I am also fine with the above suggestion by Star. So if you prefer the
above approach, please feel free to propose another patch. Thanks in
advance.

Best Regards,
Hao Wu

> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Thursday, March 21, 2019 1:43 AM
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Andrew Fish
> <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64
> in .dsc
> 
> On Wed, Mar 20, 2019 at 03:51:39PM +0100, Laszlo Ersek wrote:
> > Hi Leif,
> >
> > On 03/18/19 15:56, Leif Lindholm wrote:
> > > Commit 05fd2a926833
> > > ("MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList
> > > LockBox") added a dependency on LockBoxLib to NvmExpressPei, causing
> > > builds using MdeModulePkg.dsc to fail on architectures other than
> > > IA32/X64 with missing reference to
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode.
> > >
> > > Add a resolution for LockBoxNullLib for ARM/AARCH64 to restore builds.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> > > ---
> > >
> > > Note: this patch hides the symptom, but this isn't really the fix I
> > > would like to see.
> > >
> > > The build error is caused by the chain of:
> > > 1) NvmExpressPei depending on LockBoxLib
> > > 2) LockBoxLib being mapped to SmmLockBoxPeiLib in
> > > [LibraryClasses.common.PEIM]
> > > 3) SmmLockBoxPeiLib depending on PcdDxeIplSwitchToLongMode
> > > 4) PcdDxeIplSwitchToLongMode being declared in
> > >    [PcdsFeatureFlag.IA32, PcdsFeatureFlag.X64] in MdeModulePkg.dsc
> > >
> > > Now, an alternative quick-fix would be to move the PEIM LockBoxLib
> > > mapping into a [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM]
> > > section. But that would leave NvmExpressPei unbuildable on anything
> > > not IA32/X64.
> > >
> > > Another option would be to add default declaration (for all other
> > > architectures) of FALSE for PcdDxeIplSwitchToLongMode in
> > > MdeModulePkg.dec, but the current way this is expressed seems to
> > > treat this as an architecture-specific feature (which it is).
> > >
> > > What I believe would be the cleanest solution would be to abstract
> > > NvmExpressPei to the point where it can function without the LockBoxLib.
> > > But regardless, it does not look valid to me for something as
> > > architecture-specific as MdeModulePkg/Library/SmmLockBoxLib/ to live
> > > under .common sections in the .dsc. (And if this changes at some
> > > point, because we implement an ARM/AARCH64 equivalent based on
> > > StandaloneMmPkg, we will need a major refactoring of that library
> > > anyway.)
> > >
> > > /
> > >     Leif
> > >
> > > MdeModulePkg/MdeModulePkg.dsc | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > > b/MdeModulePkg/MdeModulePkg.dsc index 6cd1727a0d..6e27e9cb68
> 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dsc
> > > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > > @@ -178,6 +178,7 @@ [LibraryClasses.common.MM_STANDALONE]
> > >  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > >    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > >    ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> > > +  LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> > >
> > >    #
> > >    # It is not possible to prevent ARM compiler calls to generic intrinsic
> functions.
> > >
> >
> > I think this patch is exactly the right solution.
> >
> > The code added in commit 05fd2a926833 is gated by (BootMode ==
> > BOOT_ON_S3_RESUME). That condition can never evaluate to TRUE on
> > ARM/AARCH64, presently. Accordingly, the stated goal of the commit
> > doesn't apply to ARM/AARCH64:
> >
> >     The purpose is to perform an on-demand (partial) NVM Express device
> >     enumeration/initialization to benefit the S3 resume performance.
> >
> > Given that the RestoreLockBox() calls are never reached (which is
> > correct, by design, at the present level of ACPI S3 enablement in edk2
> > for ARM/AARCH64), causing the lockbox APIs to "do nothing beyond
> > compile" is exactly right. IMO anyway.
> >
> > Once ARM/AARCH64 grow S3 support, a functional and secure LockBox will
> > have to be part of that. Perhaps it will use "standalone MM"; I'm not
> > sure. The point is, once the goal of the commit starts applying to
> > ARM/AARCH64, a functional LockBox will have been implemented for
> > ARM/AARCH64; and that lib instance will certainly not depend on
> > PcdDxeIplSwitchToLongMode.
> >
> > Until such time, this patch is fine.
> 
> OK, I buy that argument.
> 
> *But* I still think the IA32/X64 specific library mappings should be moved out
> of .common LibraryClass sections.
> 
> Regards,
> 
> Leif
> 
> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >
> > Thanks
> > Laszlo

Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[Leif Lindholm]

On Thu, Mar 21, 2019 at 03:27:45AM +0000, Wu, Hao A wrote:
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Thursday, March 21, 2019 9:03 AM
> > To: Leif Lindholm; Laszlo Ersek
> > Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org; Wang, Jian J; Wu,
> > Hao A; Ni, Ray; Andrew Fish; Kinney, Michael D; Zeng, Star
> > Subject: RE: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64
> > in .dsc
> > 
> > Another way to update the file is
> > 
> > [LibraryClasses.EBC]
> >   LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> > 
> > ->
> > 
> > [LibraryClasses.EBC, LibraryClasses.ARM, LibraryClasses.AARCH64]
> >   LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> 
> Hello Leif,
> 
> The current proposed patch seems great to me.
> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
> 
> I am also fine with the above suggestion by Star. So if you prefer the
> above approach, please feel free to propose another patch. Thanks in
> advance.

Laszlo convinced me that this change makes sense. But the argument for
that was that each architecture needs to decide itself how to
implement LockBoxLib (or not).

What does not make sense to me is that
MdeModulePkg/Library/SmmLockBoxLib/ is used as a global default, and
set as the resolution for LockBoxLib in common sections, when it is
only valid for 2 of the 6 architectures supported by the UEFI
specification.

My original version is my preferred way of addressing the immediate
problem though, mainly to keep the separate .EBC section.

Best Regards,

Leif

> Best Regards,
> Hao Wu
> 
> > 
> > 
> > Thanks,
> > Star
> > -----Original Message-----
> > From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> > Sent: Thursday, March 21, 2019 1:43 AM
> > To: Laszlo Ersek <lersek@redhat.com>
> > Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org; Wang, Jian J
> > <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Andrew Fish
> > <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64
> > in .dsc
> > 
> > On Wed, Mar 20, 2019 at 03:51:39PM +0100, Laszlo Ersek wrote:
> > > Hi Leif,
> > >
> > > On 03/18/19 15:56, Leif Lindholm wrote:
> > > > Commit 05fd2a926833
> > > > ("MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList
> > > > LockBox") added a dependency on LockBoxLib to NvmExpressPei, causing
> > > > builds using MdeModulePkg.dsc to fail on architectures other than
> > > > IA32/X64 with missing reference to
> > > > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode.
> > > >
> > > > Add a resolution for LockBoxNullLib for ARM/AARCH64 to restore builds.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> > > > ---
> > > >
> > > > Note: this patch hides the symptom, but this isn't really the fix I
> > > > would like to see.
> > > >
> > > > The build error is caused by the chain of:
> > > > 1) NvmExpressPei depending on LockBoxLib
> > > > 2) LockBoxLib being mapped to SmmLockBoxPeiLib in
> > > > [LibraryClasses.common.PEIM]
> > > > 3) SmmLockBoxPeiLib depending on PcdDxeIplSwitchToLongMode
> > > > 4) PcdDxeIplSwitchToLongMode being declared in
> > > >    [PcdsFeatureFlag.IA32, PcdsFeatureFlag.X64] in MdeModulePkg.dsc
> > > >
> > > > Now, an alternative quick-fix would be to move the PEIM LockBoxLib
> > > > mapping into a [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM]
> > > > section. But that would leave NvmExpressPei unbuildable on anything
> > > > not IA32/X64.
> > > >
> > > > Another option would be to add default declaration (for all other
> > > > architectures) of FALSE for PcdDxeIplSwitchToLongMode in
> > > > MdeModulePkg.dec, but the current way this is expressed seems to
> > > > treat this as an architecture-specific feature (which it is).
> > > >
> > > > What I believe would be the cleanest solution would be to abstract
> > > > NvmExpressPei to the point where it can function without the LockBoxLib.
> > > > But regardless, it does not look valid to me for something as
> > > > architecture-specific as MdeModulePkg/Library/SmmLockBoxLib/ to live
> > > > under .common sections in the .dsc. (And if this changes at some
> > > > point, because we implement an ARM/AARCH64 equivalent based on
> > > > StandaloneMmPkg, we will need a major refactoring of that library
> > > > anyway.)
> > > >
> > > > /
> > > >     Leif
> > > >
> > > > MdeModulePkg/MdeModulePkg.dsc | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > > > b/MdeModulePkg/MdeModulePkg.dsc index 6cd1727a0d..6e27e9cb68
> > 100644
> > > > --- a/MdeModulePkg/MdeModulePkg.dsc
> > > > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > > > @@ -178,6 +178,7 @@ [LibraryClasses.common.MM_STANDALONE]
> > > >  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > > >    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > > >    ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> > > > +  LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> > > >
> > > >    #
> > > >    # It is not possible to prevent ARM compiler calls to generic intrinsic
> > functions.
> > > >
> > >
> > > I think this patch is exactly the right solution.
> > >
> > > The code added in commit 05fd2a926833 is gated by (BootMode ==
> > > BOOT_ON_S3_RESUME). That condition can never evaluate to TRUE on
> > > ARM/AARCH64, presently. Accordingly, the stated goal of the commit
> > > doesn't apply to ARM/AARCH64:
> > >
> > >     The purpose is to perform an on-demand (partial) NVM Express device
> > >     enumeration/initialization to benefit the S3 resume performance.
> > >
> > > Given that the RestoreLockBox() calls are never reached (which is
> > > correct, by design, at the present level of ACPI S3 enablement in edk2
> > > for ARM/AARCH64), causing the lockbox APIs to "do nothing beyond
> > > compile" is exactly right. IMO anyway.
> > >
> > > Once ARM/AARCH64 grow S3 support, a functional and secure LockBox will
> > > have to be part of that. Perhaps it will use "standalone MM"; I'm not
> > > sure. The point is, once the goal of the commit starts applying to
> > > ARM/AARCH64, a functional LockBox will have been implemented for
> > > ARM/AARCH64; and that lib instance will certainly not depend on
> > > PcdDxeIplSwitchToLongMode.
> > >
> > > Until such time, this patch is fine.
> > 
> > OK, I buy that argument.
> > 
> > *But* I still think the IA32/X64 specific library mappings should be moved out
> > of .common LibraryClass sections.
> > 
> > Regards,
> > 
> > Leif
> > 
> > > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> > >
> > > Thanks
> > > Laszlo

Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[Wu, Hao A]

> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Saturday, March 23, 2019 2:13 AM
> To: Wu, Hao A
> Cc: Zeng, Star; Laszlo Ersek; edk2-devel@lists.01.org;
> ard.biesheuvel@linaro.org; Wang, Jian J; Ni, Ray; Andrew Fish; Kinney, Michael
> D
> Subject: Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64
> in .dsc
> 
> On Thu, Mar 21, 2019 at 03:27:45AM +0000, Wu, Hao A wrote:
> > > -----Original Message-----
> > > From: Zeng, Star
> > > Sent: Thursday, March 21, 2019 9:03 AM
> > > To: Leif Lindholm; Laszlo Ersek
> > > Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org; Wang, Jian J; Wu,
> > > Hao A; Ni, Ray; Andrew Fish; Kinney, Michael D; Zeng, Star
> > > Subject: RE: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64
> > > in .dsc
> > >
> > > Another way to update the file is
> > >
> > > [LibraryClasses.EBC]
> > >   LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> > >
> > > ->
> > >
> > > [LibraryClasses.EBC, LibraryClasses.ARM, LibraryClasses.AARCH64]
> > >   LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> >
> > Hello Leif,
> >
> > The current proposed patch seems great to me.
> > Reviewed-by: Hao Wu <hao.a.wu@intel.com>
> >
> > I am also fine with the above suggestion by Star. So if you prefer the
> > above approach, please feel free to propose another patch. Thanks in
> > advance.
> 
> Laszlo convinced me that this change makes sense. But the argument for
> that was that each architecture needs to decide itself how to
> implement LockBoxLib (or not).
> 
> What does not make sense to me is that
> MdeModulePkg/Library/SmmLockBoxLib/ is used as a global default, and
> set as the resolution for LockBoxLib in common sections, when it is
> only valid for 2 of the 6 architectures supported by the UEFI
> specification.

Hello Leif,

I filed a BZ tracker according to your above concern:
https://bugzilla.tianocore.org/show_bug.cgi?id=1660

We will find an approach to address it.

> 
> My original version is my preferred way of addressing the immediate
> problem though, mainly to keep the separate .EBC section.

Got it.
Do you want me to help to push the patch?

Best Regards,
Hao Wu

> 
> Best Regards,
> 
> Leif
> 
> > Best Regards,
> > Hao Wu
> >
> > >
> > >
> > > Thanks,
> > > Star
> > > -----Original Message-----
> > > From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> > > Sent: Thursday, March 21, 2019 1:43 AM
> > > To: Laszlo Ersek <lersek@redhat.com>
> > > Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org; Wang, Jian J
> > > <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Andrew Fish
> > > <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> > > Subject: Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64
> > > in .dsc
> > >
> > > On Wed, Mar 20, 2019 at 03:51:39PM +0100, Laszlo Ersek wrote:
> > > > Hi Leif,
> > > >
> > > > On 03/18/19 15:56, Leif Lindholm wrote:
> > > > > Commit 05fd2a926833
> > > > > ("MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList
> > > > > LockBox") added a dependency on LockBoxLib to NvmExpressPei,
> causing
> > > > > builds using MdeModulePkg.dsc to fail on architectures other than
> > > > > IA32/X64 with missing reference to
> > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode.
> > > > >
> > > > > Add a resolution for LockBoxNullLib for ARM/AARCH64 to restore builds.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> > > > > ---
> > > > >
> > > > > Note: this patch hides the symptom, but this isn't really the fix I
> > > > > would like to see.
> > > > >
> > > > > The build error is caused by the chain of:
> > > > > 1) NvmExpressPei depending on LockBoxLib
> > > > > 2) LockBoxLib being mapped to SmmLockBoxPeiLib in
> > > > > [LibraryClasses.common.PEIM]
> > > > > 3) SmmLockBoxPeiLib depending on PcdDxeIplSwitchToLongMode
> > > > > 4) PcdDxeIplSwitchToLongMode being declared in
> > > > >    [PcdsFeatureFlag.IA32, PcdsFeatureFlag.X64] in MdeModulePkg.dsc
> > > > >
> > > > > Now, an alternative quick-fix would be to move the PEIM LockBoxLib
> > > > > mapping into a [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM]
> > > > > section. But that would leave NvmExpressPei unbuildable on anything
> > > > > not IA32/X64.
> > > > >
> > > > > Another option would be to add default declaration (for all other
> > > > > architectures) of FALSE for PcdDxeIplSwitchToLongMode in
> > > > > MdeModulePkg.dec, but the current way this is expressed seems to
> > > > > treat this as an architecture-specific feature (which it is).
> > > > >
> > > > > What I believe would be the cleanest solution would be to abstract
> > > > > NvmExpressPei to the point where it can function without the
> LockBoxLib.
> > > > > But regardless, it does not look valid to me for something as
> > > > > architecture-specific as MdeModulePkg/Library/SmmLockBoxLib/ to live
> > > > > under .common sections in the .dsc. (And if this changes at some
> > > > > point, because we implement an ARM/AARCH64 equivalent based on
> > > > > StandaloneMmPkg, we will need a major refactoring of that library
> > > > > anyway.)
> > > > >
> > > > > /
> > > > >     Leif
> > > > >
> > > > > MdeModulePkg/MdeModulePkg.dsc | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > > > > b/MdeModulePkg/MdeModulePkg.dsc index 6cd1727a0d..6e27e9cb68
> > > 100644
> > > > > --- a/MdeModulePkg/MdeModulePkg.dsc
> > > > > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > > > > @@ -178,6 +178,7 @@ [LibraryClasses.common.MM_STANDALONE]
> > > > >  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > > > >    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > > > >    ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> > > > > +
> LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> > > > >
> > > > >    #
> > > > >    # It is not possible to prevent ARM compiler calls to generic intrinsic
> > > functions.
> > > > >
> > > >
> > > > I think this patch is exactly the right solution.
> > > >
> > > > The code added in commit 05fd2a926833 is gated by (BootMode ==
> > > > BOOT_ON_S3_RESUME). That condition can never evaluate to TRUE on
> > > > ARM/AARCH64, presently. Accordingly, the stated goal of the commit
> > > > doesn't apply to ARM/AARCH64:
> > > >
> > > >     The purpose is to perform an on-demand (partial) NVM Express device
> > > >     enumeration/initialization to benefit the S3 resume performance.
> > > >
> > > > Given that the RestoreLockBox() calls are never reached (which is
> > > > correct, by design, at the present level of ACPI S3 enablement in edk2
> > > > for ARM/AARCH64), causing the lockbox APIs to "do nothing beyond
> > > > compile" is exactly right. IMO anyway.
> > > >
> > > > Once ARM/AARCH64 grow S3 support, a functional and secure LockBox
> will
> > > > have to be part of that. Perhaps it will use "standalone MM"; I'm not
> > > > sure. The point is, once the goal of the commit starts applying to
> > > > ARM/AARCH64, a functional LockBox will have been implemented for
> > > > ARM/AARCH64; and that lib instance will certainly not depend on
> > > > PcdDxeIplSwitchToLongMode.
> > > >
> > > > Until such time, this patch is fine.
> > >
> > > OK, I buy that argument.
> > >
> > > *But* I still think the IA32/X64 specific library mappings should be moved
> out
> > > of .common LibraryClass sections.
> > >
> > > Regards,
> > >
> > > Leif
> > >
> > > > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> > > >
> > > > Thanks
> > > > Laszlo

Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[Leif Lindholm]

On Mon, Mar 25, 2019 at 02:17:05AM +0000, Wu, Hao A wrote:
> > My original version is my preferred way of addressing the immediate
> > problem though, mainly to keep the separate .EBC section.
> 
> Got it.
> Do you want me to help to push the patch?

If you could, that would be appreciated.

Best Regards,

Leif

Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64 in .dsc

[Wu, Hao A]

> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Wednesday, March 27, 2019 3:43 AM
> To: Wu, Hao A
> Cc: Zeng, Star; Laszlo Ersek; edk2-devel@lists.01.org;
> ard.biesheuvel@linaro.org; Wang, Jian J; Ni, Ray; Andrew Fish; Kinney, Michael
> D
> Subject: Re: [RFC PATCH] MdeModulePkg: add LockBoxNullLib for !IA32/X64
> in .dsc
> 
> On Mon, Mar 25, 2019 at 02:17:05AM +0000, Wu, Hao A wrote:
> > > My original version is my preferred way of addressing the immediate
> > > problem though, mainly to keep the separate .EBC section.
> >
> > Got it.
> > Do you want me to help to push the patch?
> 
> If you could, that would be appreciated.

Hello Leif,

Pushed at commit:
4a1f6b85c184eecab22df88312a207f5e0ea4264

Thanks for the patch.

Best Regards,
Hao Wu

> 
> Best Regards,
> 
> Leif