On 05/29/15 14:33, Ard Biesheuvel wrote:
> Clone ArmGicArchLib into a SEC phase specific ArmGicArchSecLib
> so that we can modify the former in a subsequent patch to cache
> the GIC revision in a global variable.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
>  ArmPkg/Library/{ArmGicArchLib => ArmGicArchSecLib}/AArch64/ArmGicArchLib.c   
>              | 0
>  ArmPkg/Library/{ArmGicArchLib => ArmGicArchSecLib}/Arm/ArmGicArchLib.c       
>              | 0
>  ArmPkg/Library/{ArmGicArchLib/ArmGicArchLib.inf => 
> ArmGicArchSecLib/ArmGicArchSecLib.inf} | 6 +++---
>  ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc                                   
>              | 1 +
>  ArmPlatformPkg/ArmPlatformPkg.dsc                                            
>              | 2 ++
>  ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc                        
>              | 2 ++
>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc                            
>              | 2 ++
>  7 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c 
> b/ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
> similarity index 100%
> copy from ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
> copy to ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
> diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c 
> b/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
> similarity index 100%
> copy from ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> copy to ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
> diff --git a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf 
> b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> similarity index 79%
> copy from ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> copy to ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> index d71b2adc3027..a2fb623a8537 100644
> --- a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> +++ b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> @@ -13,11 +13,11 @@
>  
>  [Defines]
>    INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = ArmGicArchLib
> -  FILE_GUID                      = cd67f41a-26e9-4482-90c9-a9aff803382a
> +  BASE_NAME                      = ArmGicArchSecLib
> +  FILE_GUID                      = c1dd9745-9459-4e9a-9f5b-99cbd233c27d
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
> -  LIBRARY_CLASS                  = ArmGicArchLib
> +  LIBRARY_CLASS                  = ArmGicArchLib|SEC

* Okay, so this patch does the clone for SEC modules and points the SEC
library class resolutions in the DSC files to the clone, overriding the
default resolutions in them.

ArmPkg/ArmPkg.dsc and ArmVirtPkg/ArmVirt.dsc.inc are not updated. For
ArmVirtPkg, the default resolution stays intact in this patch, which
makes sense, because patch #7 handles it separately.

But, is there any particular reason to leave ArmPkg/ArmPkg.dsc unchanged
in this patch? Hm... I guess it should be safe, because patch #5 is
explicit about DXE_DRIVER, UEFI_DRIVER, UEFI_APPLICATION; so if there's
a problem later, it will show up as a build time failure. Okay.

* Anyway, my main concern was PEIMs. Especially because in patch #3 you
wrote,

> this statelessness is only needed for SEC type modules

This is in general not true, because PEIMs also may execute-in-place
from flash. However, from patch #5 I can see that the caching version
will be available only to DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION,
leaving PEIMs uncovered -- which is safe, and also good enough since
(apparently) no PEIMs depend on this library class.

If you have to respin the series for any reason, please consider
mentioning PEIMs in the commit message of patch #3 and patch #4, and
making ArmGicArchSecLib available to PEIMs too. However, as I said
above, that would be quite speculative, and this could be addressed any
time later, if a new PEIM actually needed the library. So feel free to
ignore this note.

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

Thanks
Laszlo

>  
>  [Sources.ARM]
>    Arm/ArmGicArchLib.c
> diff --git a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc 
> b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> index 0859bc31ff00..6e6687c8a735 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> +++ b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> @@ -138,6 +138,7 @@
>    
> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>    PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
>    PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>  
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>    MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc 
> b/ArmPlatformPkg/ArmPlatformPkg.dsc
> index be31025d9bd0..14d82f61ec76 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dsc
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc
> @@ -134,6 +134,8 @@
>    
> DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
>    
> DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
>  
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>    MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>  
> diff --git a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc 
> b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> index 19de3816ae5c..4b4867f9926a 100644
> --- a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> +++ b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> @@ -128,6 +128,8 @@
>    
> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>  !endif
>  
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>    MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>  
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc 
> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
> index 84e2a9987f7d..8f7b5f1644de 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
> @@ -144,6 +144,8 @@
>    # Trustzone Support
>    
> ArmTrustedMonitorLib|ArmPlatformPkg/Library/ArmTrustedMonitorLibNull/ArmTrustedMonitorLibNull.inf
>  
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +
>  [LibraryClasses.common.PEI_CORE]
>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>    PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> 


------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to