On 9 June 2015 at 18:33, Laszlo Ersek <ler...@redhat.com> wrote:
> 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?
Actually, that is an oversight. I should probably add it there for
completeness, although the exact purpose of these package .DSCs
escapes me, to be honest.
Anyway, ArmPkg.dsc still builds ok with these changes, so I assume
there is no SEC module being built that [transitively] relies on
ArmGicArchLib
> 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.
>
i have you on record (and I can dig up the email, if you like) saying
that you prefer the LIBRARY_CLASS phase modifiers to be minimal.
> 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,
Ard,
>>
>> [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
