[edk2,4/7] ArmPkg: copy ArmGicArchLib to ArmGicArchSecLib

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.biesheuvel@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(-)

On 9 June 2015 at 18:33, Laszlo Ersek <lersek@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.biesheuvel@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 <lersek@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
>>
>

------------------------------------------------------------------------------