diff mbox

[edk2,v3,1/5] ArmPlatformPkg: do not fulfil MemoryInitPeiLib dependency directly via .c file

Message ID 1428477061-1768-2-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel April 8, 2015, 7:10 a.m. UTC
MemoryInitPeim short-circuits its MemoryInitPeiLib dependency by including
the .c file directly. This prevents us from having a special implementation
for ArmVirtualizationPkg that performs additional cache maintenance before
enabling the MMU. So instead, make it depend on the library class.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc | 1 +
 ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf             | 2 +-
 ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf               | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

Comments

Ard Biesheuvel April 8, 2015, 10:54 a.m. UTC | #1
On 8 April 2015 at 12:46, Olivier Martin <Olivier.Martin@arm.com> wrote:
> This patch breaks the following builds:
> - ArmPlatformPkg/ArmPlatformPkg.dsc
> - ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc
> - ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc
> - (...)
>
> Always with the same issue:
> ArmPlatformPkg/ArmPlatformPkg.dsc(...): error 4000: Instance of library class [MemoryInitPeiLib] is not found
>         in [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf] [ARM]
>         consumed by module [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf]
>

Hmm, that is surprising:

$ git grep MemoryInitPeiLib
ArmPlatformPkg/ArmJunoPkg/ArmJuno.dsc:
MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc:
MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc:
MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc:
MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf

produces hits for the associated platforms, so I did not look at the
.DSCs in detail. ArmVExpress-FVP-AArch64 does build, btw

I will update the patch so that all .DSCs are updated with the missing
library reference.
Ard Biesheuvel April 8, 2015, 11:01 a.m. UTC | #2
On 8 April 2015 at 12:58, Olivier Martin <Olivier.Martin@arm.com> wrote:
> I have just had a look to understand the reason. And here is the reason... ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf is a PEIM module (MODULE_TYPE = PEIM); while all the declarations you quoted before are for the SEC module:
>
> [LibraryClasses.common.SEC]
>   MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>
> Moving `MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf`  from [LibraryClasses.common.SEC] to [LibraryClasses.common.SEC, LibraryClasses.common.PEIM] should fix the issue.
>

OK, but that also implies that MemoryInitPeiLib.inf's module type
should become 'SEC PEIM' not just 'SEC', right?

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 08 April 2015 11:55
> To: Olivier Martin
> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com; roy.franz@linaro.org; leif.lindholm@linaro.org
> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil MemoryInitPeiLib dependency directly via .c file
>
> On 8 April 2015 at 12:46, Olivier Martin <Olivier.Martin@arm.com> wrote:
>> This patch breaks the following builds:
>> - ArmPlatformPkg/ArmPlatformPkg.dsc
>> - ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc
>> - ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc
>> - (...)
>>
>> Always with the same issue:
>> ArmPlatformPkg/ArmPlatformPkg.dsc(...): error 4000: Instance of library class [MemoryInitPeiLib] is not found
>>         in [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf] [ARM]
>>         consumed by module
>> [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf]
>>
>
> Hmm, that is surprising:
>
> $ git grep MemoryInitPeiLib
> ArmPlatformPkg/ArmJunoPkg/ArmJuno.dsc:
> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
> ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc:
> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
> ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc:
> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
> ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc:
> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>
> produces hits for the associated platforms, so I did not look at the .DSCs in detail. ArmVExpress-FVP-AArch64 does build, btw
>
> I will update the patch so that all .DSCs are updated with the missing library reference.
>
> --
> Ard.
>
>
>
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: 08 April 2015 08:11
>> To: edk2-devel@lists.sourceforge.net; lersek@redhat.com; Olivier Martin
>> Cc: roy.franz@linaro.org; leif.lindholm@linaro.org; Ard Biesheuvel
>> Subject: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil MemoryInitPeiLib dependency directly via .c file
>>
>> MemoryInitPeim short-circuits its MemoryInitPeiLib dependency by including the .c file directly. This prevents us from having a special implementation for ArmVirtualizationPkg that performs additional cache maintenance before enabling the MMU. So instead, make it depend on the library class.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc | 1 +
>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf             | 2 +-
>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf               | 2 +-
>>  3 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>> index 51c163286d1f..b03616842f81 100644
>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>> @@ -77,6 +77,7 @@
>>    ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.inf
>>
>>    PlatformPeiLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
>> +  MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>    EfiResetSystemLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPsciResetSystemLib/ArmVirtualizationPsciResetSystemLib.inf
>>
>>    # ARM PL031 RTC Driver
>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>> index 1e5b93e8a507..7f1976d60c31 100755
>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>> @@ -17,7 +17,7 @@
>>    FILE_GUID                      = 55ddb6e0-70b5-11e0-b33e-0002a5d5c51b
>>    MODULE_TYPE                    = SEC
>>    VERSION_STRING                 = 1.0
>> -  LIBRARY_CLASS                  = PlatformPeiLib
>> +  LIBRARY_CLASS                  = MemoryInitPeiLib
>>
>>  [Sources]
>>    MemoryInitPeiLib.c
>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>> index 6374b63f8524..2c14a9c826ff 100755
>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>> @@ -28,7 +28,6 @@
>>
>>  [Sources]
>>    MemoryInitPeim.c
>> -  MemoryInitPeiLib.c
>>
>>  [Packages]
>>    MdePkg/MdePkg.dec
>> @@ -43,6 +42,7 @@
>>    HobLib
>>    ArmLib
>>    ArmPlatformLib
>> +  MemoryInitPeiLib
>>
>>  [Guids]
>>    gEfiMemoryTypeInformationGuid
>> --
>> 1.8.3.2
>>
>>
>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>
>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
>> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782
>>
>
>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
Ard Biesheuvel April 8, 2015, 11:07 a.m. UTC | #3
On 8 April 2015 at 13:03, Olivier Martin <Olivier.Martin@arm.com> wrote:
> That's correct!
>

Hmm, that doesn't seem to work.

Would there be any problem with making it a 'BASE' module type?
And making it a general dependency in [LibraryClasses]' rather than
for SEC or PEIM specifically?

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 08 April 2015 12:01
> To: Olivier Martin
> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com; roy.franz@linaro.org; leif.lindholm@linaro.org
> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil MemoryInitPeiLib dependency directly via .c file
>
> On 8 April 2015 at 12:58, Olivier Martin <Olivier.Martin@arm.com> wrote:
>> I have just had a look to understand the reason. And here is the reason... ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf is a PEIM module (MODULE_TYPE = PEIM); while all the declarations you quoted before are for the SEC module:
>>
>> [LibraryClasses.common.SEC]
>>   MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>
>> Moving `MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf`  from [LibraryClasses.common.SEC] to [LibraryClasses.common.SEC, LibraryClasses.common.PEIM] should fix the issue.
>>
>
> OK, but that also implies that MemoryInitPeiLib.inf's module type should become 'SEC PEIM' not just 'SEC', right?
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: 08 April 2015 11:55
>> To: Olivier Martin
>> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com;
>> roy.franz@linaro.org; leif.lindholm@linaro.org
>> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>> MemoryInitPeiLib dependency directly via .c file
>>
>> On 8 April 2015 at 12:46, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>> This patch breaks the following builds:
>>> - ArmPlatformPkg/ArmPlatformPkg.dsc
>>> - ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc
>>> - ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc
>>> - (...)
>>>
>>> Always with the same issue:
>>> ArmPlatformPkg/ArmPlatformPkg.dsc(...): error 4000: Instance of library class [MemoryInitPeiLib] is not found
>>>         in [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf] [ARM]
>>>         consumed by module
>>> [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf]
>>>
>>
>> Hmm, that is surprising:
>>
>> $ git grep MemoryInitPeiLib
>> ArmPlatformPkg/ArmJunoPkg/ArmJuno.dsc:
>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>> ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc:
>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>> ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc:
>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>> ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc:
>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>
>> produces hits for the associated platforms, so I did not look at the
>> .DSCs in detail. ArmVExpress-FVP-AArch64 does build, btw
>>
>> I will update the patch so that all .DSCs are updated with the missing library reference.
>>
>> --
>> Ard.
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>> Sent: 08 April 2015 08:11
>>> To: edk2-devel@lists.sourceforge.net; lersek@redhat.com; Olivier
>>> Martin
>>> Cc: roy.franz@linaro.org; leif.lindholm@linaro.org; Ard Biesheuvel
>>> Subject: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>>> MemoryInitPeiLib dependency directly via .c file
>>>
>>> MemoryInitPeim short-circuits its MemoryInitPeiLib dependency by including the .c file directly. This prevents us from having a special implementation for ArmVirtualizationPkg that performs additional cache maintenance before enabling the MMU. So instead, make it depend on the library class.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc | 1 +
>>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf             | 2 +-
>>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf               | 2 +-
>>>  3 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git
>>> a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>> index 51c163286d1f..b03616842f81 100644
>>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>> @@ -77,6 +77,7 @@
>>>
>>> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerVirtCounterLi
>>> b/ArmGenericTimerVirtCounterLib.inf
>>>
>>>
>>> PlatformPeiLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPe
>>> iLib/PlatformPeiLib.inf
>>> +  MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>
>>> EfiResetSystemLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirt
>>> ualizationPsciResetSystemLib/ArmVirtualizationPsciResetSystemLib.inf
>>>
>>>    # ARM PL031 RTC Driver
>>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>> index 1e5b93e8a507..7f1976d60c31 100755
>>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>> @@ -17,7 +17,7 @@
>>>    FILE_GUID                      = 55ddb6e0-70b5-11e0-b33e-0002a5d5c51b
>>>    MODULE_TYPE                    = SEC
>>>    VERSION_STRING                 = 1.0
>>> -  LIBRARY_CLASS                  = PlatformPeiLib
>>> +  LIBRARY_CLASS                  = MemoryInitPeiLib
>>>
>>>  [Sources]
>>>    MemoryInitPeiLib.c
>>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>> index 6374b63f8524..2c14a9c826ff 100755
>>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>> @@ -28,7 +28,6 @@
>>>
>>>  [Sources]
>>>    MemoryInitPeim.c
>>> -  MemoryInitPeiLib.c
>>>
>>>  [Packages]
>>>    MdePkg/MdePkg.dec
>>> @@ -43,6 +42,7 @@
>>>    HobLib
>>>    ArmLib
>>>    ArmPlatformLib
>>> +  MemoryInitPeiLib
>>>
>>>  [Guids]
>>>    gEfiMemoryTypeInformationGuid
>>> --
>>> 1.8.3.2
>>>
>>>
>>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>>
>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>> Registered in England & Wales, Company No:  2557590 ARM Holdings plc,
>>> Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in
>>> England & Wales, Company No:  2548782
>>>
>>
>>
>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>
>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>> Registered in England & Wales, Company No:  2557590 ARM Holdings plc,
>> Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in
>> England & Wales, Company No:  2548782
>
>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
Ard Biesheuvel April 8, 2015, 11:13 a.m. UTC | #4
On 8 April 2015 at 13:12, Olivier Martin <Olivier.Martin@arm.com> wrote:
> Is the reason why it does not work is dependency issue on other PEIM library?

MODULE_TYPE must be either 'SEC' or 'PEIM' and not both. If we are
going to use the library in modules of both flavors, it would be
cleaner just to use BASE


> Yes, using 'BASE' might fix the issue.
>
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 08 April 2015 12:07
> To: Olivier Martin
> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com; roy.franz@linaro.org; leif.lindholm@linaro.org
> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil MemoryInitPeiLib dependency directly via .c file
>
> On 8 April 2015 at 13:03, Olivier Martin <Olivier.Martin@arm.com> wrote:
>> That's correct!
>>
>
> Hmm, that doesn't seem to work.
>
> Would there be any problem with making it a 'BASE' module type?
> And making it a general dependency in [LibraryClasses]' rather than for SEC or PEIM specifically?
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: 08 April 2015 12:01
>> To: Olivier Martin
>> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com;
>> roy.franz@linaro.org; leif.lindholm@linaro.org
>> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>> MemoryInitPeiLib dependency directly via .c file
>>
>> On 8 April 2015 at 12:58, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>> I have just had a look to understand the reason. And here is the reason... ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf is a PEIM module (MODULE_TYPE = PEIM); while all the declarations you quoted before are for the SEC module:
>>>
>>> [LibraryClasses.common.SEC]
>>>   MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>
>>> Moving `MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf`  from [LibraryClasses.common.SEC] to [LibraryClasses.common.SEC, LibraryClasses.common.PEIM] should fix the issue.
>>>
>>
>> OK, but that also implies that MemoryInitPeiLib.inf's module type should become 'SEC PEIM' not just 'SEC', right?
>>
>>> -----Original Message-----
>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>> Sent: 08 April 2015 11:55
>>> To: Olivier Martin
>>> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com;
>>> roy.franz@linaro.org; leif.lindholm@linaro.org
>>> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>>> MemoryInitPeiLib dependency directly via .c file
>>>
>>> On 8 April 2015 at 12:46, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>>> This patch breaks the following builds:
>>>> - ArmPlatformPkg/ArmPlatformPkg.dsc
>>>> - ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc
>>>> - ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc
>>>> - (...)
>>>>
>>>> Always with the same issue:
>>>> ArmPlatformPkg/ArmPlatformPkg.dsc(...): error 4000: Instance of library class [MemoryInitPeiLib] is not found
>>>>         in [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf] [ARM]
>>>>         consumed by module
>>>> [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf]
>>>>
>>>
>>> Hmm, that is surprising:
>>>
>>> $ git grep MemoryInitPeiLib
>>> ArmPlatformPkg/ArmJunoPkg/ArmJuno.dsc:
>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>> ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc:
>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>> ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc:
>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>> ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc:
>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>
>>> produces hits for the associated platforms, so I did not look at the
>>> .DSCs in detail. ArmVExpress-FVP-AArch64 does build, btw
>>>
>>> I will update the patch so that all .DSCs are updated with the missing library reference.
>>>
>>> --
>>> Ard.
>>>
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>>> Sent: 08 April 2015 08:11
>>>> To: edk2-devel@lists.sourceforge.net; lersek@redhat.com; Olivier
>>>> Martin
>>>> Cc: roy.franz@linaro.org; leif.lindholm@linaro.org; Ard Biesheuvel
>>>> Subject: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>>>> MemoryInitPeiLib dependency directly via .c file
>>>>
>>>> MemoryInitPeim short-circuits its MemoryInitPeiLib dependency by including the .c file directly. This prevents us from having a special implementation for ArmVirtualizationPkg that performs additional cache maintenance before enabling the MMU. So instead, make it depend on the library class.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>>  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc | 1 +
>>>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf             | 2 +-
>>>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf               | 2 +-
>>>>  3 files changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git
>>>> a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>> index 51c163286d1f..b03616842f81 100644
>>>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>> @@ -77,6 +77,7 @@
>>>>
>>>> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerVirtCounterL
>>>> ArmGenericTimerCounterLib|i
>>>> b/ArmGenericTimerVirtCounterLib.inf
>>>>
>>>>
>>>> PlatformPeiLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformP
>>>> PlatformPeiLib|e
>>>> iLib/PlatformPeiLib.inf
>>>> +
>>>> + MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>
>>>> EfiResetSystemLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVir
>>>> EfiResetSystemLib|t
>>>> ualizationPsciResetSystemLib/ArmVirtualizationPsciResetSystemLib.inf
>>>>
>>>>    # ARM PL031 RTC Driver
>>>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>> index 1e5b93e8a507..7f1976d60c31 100755
>>>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>> @@ -17,7 +17,7 @@
>>>>    FILE_GUID                      = 55ddb6e0-70b5-11e0-b33e-0002a5d5c51b
>>>>    MODULE_TYPE                    = SEC
>>>>    VERSION_STRING                 = 1.0
>>>> -  LIBRARY_CLASS                  = PlatformPeiLib
>>>> +  LIBRARY_CLASS                  = MemoryInitPeiLib
>>>>
>>>>  [Sources]
>>>>    MemoryInitPeiLib.c
>>>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>> index 6374b63f8524..2c14a9c826ff 100755
>>>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>> @@ -28,7 +28,6 @@
>>>>
>>>>  [Sources]
>>>>    MemoryInitPeim.c
>>>> -  MemoryInitPeiLib.c
>>>>
>>>>  [Packages]
>>>>    MdePkg/MdePkg.dec
>>>> @@ -43,6 +42,7 @@
>>>>    HobLib
>>>>    ArmLib
>>>>    ArmPlatformLib
>>>> +  MemoryInitPeiLib
>>>>
>>>>  [Guids]
>>>>    gEfiMemoryTypeInformationGuid
>>>> --
>>>> 1.8.3.2
>>>>
>>>>
>>>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>>>
>>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>>> Registered in England & Wales, Company No:  2557590 ARM Holdings
>>>> plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>>> Registered in England & Wales, Company No:  2548782
>>>>
>>>
>>>
>>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>>
>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>> Registered in England & Wales, Company No:  2557590 ARM Holdings plc,
>>> Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in
>>> England & Wales, Company No:  2548782
>>
>>
>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>
>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>> Registered in England & Wales, Company No:  2557590 ARM Holdings plc,
>> Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in
>> England & Wales, Company No:  2548782
>
>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
Ard Biesheuvel April 8, 2015, 11:33 a.m. UTC | #5
On 8 April 2015 at 13:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 8 April 2015 at 13:12, Olivier Martin <Olivier.Martin@arm.com> wrote:
>> Is the reason why it does not work is dependency issue on other PEIM library?
>
> MODULE_TYPE must be either 'SEC' or 'PEIM' and not both. If we are
> going to use the library in modules of both flavors, it would be
> cleaner just to use BASE
>

Actually, looking at it again, shouldn't it just be a PEIM module type?
I don't think any modules depend on MemoryInitPeiLib at all, let alone
any SEC modules.

>
>> Yes, using 'BASE' might fix the issue.
>>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: 08 April 2015 12:07
>> To: Olivier Martin
>> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com; roy.franz@linaro.org; leif.lindholm@linaro.org
>> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil MemoryInitPeiLib dependency directly via .c file
>>
>> On 8 April 2015 at 13:03, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>> That's correct!
>>>
>>
>> Hmm, that doesn't seem to work.
>>
>> Would there be any problem with making it a 'BASE' module type?
>> And making it a general dependency in [LibraryClasses]' rather than for SEC or PEIM specifically?
>>
>>> -----Original Message-----
>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>> Sent: 08 April 2015 12:01
>>> To: Olivier Martin
>>> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com;
>>> roy.franz@linaro.org; leif.lindholm@linaro.org
>>> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>>> MemoryInitPeiLib dependency directly via .c file
>>>
>>> On 8 April 2015 at 12:58, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>>> I have just had a look to understand the reason. And here is the reason... ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf is a PEIM module (MODULE_TYPE = PEIM); while all the declarations you quoted before are for the SEC module:
>>>>
>>>> [LibraryClasses.common.SEC]
>>>>   MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>
>>>> Moving `MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf`  from [LibraryClasses.common.SEC] to [LibraryClasses.common.SEC, LibraryClasses.common.PEIM] should fix the issue.
>>>>
>>>
>>> OK, but that also implies that MemoryInitPeiLib.inf's module type should become 'SEC PEIM' not just 'SEC', right?
>>>
>>>> -----Original Message-----
>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>>> Sent: 08 April 2015 11:55
>>>> To: Olivier Martin
>>>> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com;
>>>> roy.franz@linaro.org; leif.lindholm@linaro.org
>>>> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>>>> MemoryInitPeiLib dependency directly via .c file
>>>>
>>>> On 8 April 2015 at 12:46, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>>>> This patch breaks the following builds:
>>>>> - ArmPlatformPkg/ArmPlatformPkg.dsc
>>>>> - ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc
>>>>> - ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc
>>>>> - (...)
>>>>>
>>>>> Always with the same issue:
>>>>> ArmPlatformPkg/ArmPlatformPkg.dsc(...): error 4000: Instance of library class [MemoryInitPeiLib] is not found
>>>>>         in [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf] [ARM]
>>>>>         consumed by module
>>>>> [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf]
>>>>>
>>>>
>>>> Hmm, that is surprising:
>>>>
>>>> $ git grep MemoryInitPeiLib
>>>> ArmPlatformPkg/ArmJunoPkg/ArmJuno.dsc:
>>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>> ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc:
>>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>> ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc:
>>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>> ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc:
>>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>
>>>> produces hits for the associated platforms, so I did not look at the
>>>> .DSCs in detail. ArmVExpress-FVP-AArch64 does build, btw
>>>>
>>>> I will update the patch so that all .DSCs are updated with the missing library reference.
>>>>
>>>> --
>>>> Ard.
>>>>
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>>>> Sent: 08 April 2015 08:11
>>>>> To: edk2-devel@lists.sourceforge.net; lersek@redhat.com; Olivier
>>>>> Martin
>>>>> Cc: roy.franz@linaro.org; leif.lindholm@linaro.org; Ard Biesheuvel
>>>>> Subject: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>>>>> MemoryInitPeiLib dependency directly via .c file
>>>>>
>>>>> MemoryInitPeim short-circuits its MemoryInitPeiLib dependency by including the .c file directly. This prevents us from having a special implementation for ArmVirtualizationPkg that performs additional cache maintenance before enabling the MMU. So instead, make it depend on the library class.
>>>>>
>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>> ---
>>>>>  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc | 1 +
>>>>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf             | 2 +-
>>>>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf               | 2 +-
>>>>>  3 files changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git
>>>>> a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>>> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>>> index 51c163286d1f..b03616842f81 100644
>>>>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>>> @@ -77,6 +77,7 @@
>>>>>
>>>>> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerVirtCounterL
>>>>> ArmGenericTimerCounterLib|i
>>>>> b/ArmGenericTimerVirtCounterLib.inf
>>>>>
>>>>>
>>>>> PlatformPeiLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformP
>>>>> PlatformPeiLib|e
>>>>> iLib/PlatformPeiLib.inf
>>>>> +
>>>>> + MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>>
>>>>> EfiResetSystemLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVir
>>>>> EfiResetSystemLib|t
>>>>> ualizationPsciResetSystemLib/ArmVirtualizationPsciResetSystemLib.inf
>>>>>
>>>>>    # ARM PL031 RTC Driver
>>>>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>> index 1e5b93e8a507..7f1976d60c31 100755
>>>>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>> @@ -17,7 +17,7 @@
>>>>>    FILE_GUID                      = 55ddb6e0-70b5-11e0-b33e-0002a5d5c51b
>>>>>    MODULE_TYPE                    = SEC
>>>>>    VERSION_STRING                 = 1.0
>>>>> -  LIBRARY_CLASS                  = PlatformPeiLib
>>>>> +  LIBRARY_CLASS                  = MemoryInitPeiLib
>>>>>
>>>>>  [Sources]
>>>>>    MemoryInitPeiLib.c
>>>>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>>> index 6374b63f8524..2c14a9c826ff 100755
>>>>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>>> @@ -28,7 +28,6 @@
>>>>>
>>>>>  [Sources]
>>>>>    MemoryInitPeim.c
>>>>> -  MemoryInitPeiLib.c
>>>>>
>>>>>  [Packages]
>>>>>    MdePkg/MdePkg.dec
>>>>> @@ -43,6 +42,7 @@
>>>>>    HobLib
>>>>>    ArmLib
>>>>>    ArmPlatformLib
>>>>> +  MemoryInitPeiLib
>>>>>
>>>>>  [Guids]
>>>>>    gEfiMemoryTypeInformationGuid
>>>>> --
>>>>> 1.8.3.2
>>>>>
>>>>>
>>>>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>>>>
>>>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>>>> Registered in England & Wales, Company No:  2557590 ARM Holdings
>>>>> plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>>>> Registered in England & Wales, Company No:  2548782
>>>>>
>>>>
>>>>
>>>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>>>
>>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>>> Registered in England & Wales, Company No:  2557590 ARM Holdings plc,
>>>> Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in
>>>> England & Wales, Company No:  2548782
>>>
>>>
>>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>>
>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>> Registered in England & Wales, Company No:  2557590 ARM Holdings plc,
>>> Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in
>>> England & Wales, Company No:  2548782
>>
>>
>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>
>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
>> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
Ard Biesheuvel April 8, 2015, 11:35 a.m. UTC | #6
On 8 April 2015 at 13:33, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 8 April 2015 at 13:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 8 April 2015 at 13:12, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>> Is the reason why it does not work is dependency issue on other PEIM library?
>>
>> MODULE_TYPE must be either 'SEC' or 'PEIM' and not both. If we are
>> going to use the library in modules of both flavors, it would be
>> cleaner just to use BASE
>>
>
> Actually, looking at it again, shouldn't it just be a PEIM module type?
> I don't think any modules depend on MemoryInitPeiLib at all, let alone
> any SEC modules.
>

Hmm, or maybe they do. Apologies for thinking/typing out loud like this btw :-)

What confused me is that the name was wrong in the MemoryInitPeiLib.inf file.
But PrePi does depend on it, and is a SEC module

>>
>>> Yes, using 'BASE' might fix the issue.
>>>
>>> -----Original Message-----
>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>> Sent: 08 April 2015 12:07
>>> To: Olivier Martin
>>> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com; roy.franz@linaro.org; leif.lindholm@linaro.org
>>> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil MemoryInitPeiLib dependency directly via .c file
>>>
>>> On 8 April 2015 at 13:03, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>>> That's correct!
>>>>
>>>
>>> Hmm, that doesn't seem to work.
>>>
>>> Would there be any problem with making it a 'BASE' module type?
>>> And making it a general dependency in [LibraryClasses]' rather than for SEC or PEIM specifically?
>>>
>>>> -----Original Message-----
>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>>> Sent: 08 April 2015 12:01
>>>> To: Olivier Martin
>>>> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com;
>>>> roy.franz@linaro.org; leif.lindholm@linaro.org
>>>> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>>>> MemoryInitPeiLib dependency directly via .c file
>>>>
>>>> On 8 April 2015 at 12:58, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>>>> I have just had a look to understand the reason. And here is the reason... ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf is a PEIM module (MODULE_TYPE = PEIM); while all the declarations you quoted before are for the SEC module:
>>>>>
>>>>> [LibraryClasses.common.SEC]
>>>>>   MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>>
>>>>> Moving `MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf`  from [LibraryClasses.common.SEC] to [LibraryClasses.common.SEC, LibraryClasses.common.PEIM] should fix the issue.
>>>>>
>>>>
>>>> OK, but that also implies that MemoryInitPeiLib.inf's module type should become 'SEC PEIM' not just 'SEC', right?
>>>>
>>>>> -----Original Message-----
>>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>>>> Sent: 08 April 2015 11:55
>>>>> To: Olivier Martin
>>>>> Cc: edk2-devel@lists.sourceforge.net; lersek@redhat.com;
>>>>> roy.franz@linaro.org; leif.lindholm@linaro.org
>>>>> Subject: Re: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>>>>> MemoryInitPeiLib dependency directly via .c file
>>>>>
>>>>> On 8 April 2015 at 12:46, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>>>>> This patch breaks the following builds:
>>>>>> - ArmPlatformPkg/ArmPlatformPkg.dsc
>>>>>> - ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc
>>>>>> - ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc
>>>>>> - (...)
>>>>>>
>>>>>> Always with the same issue:
>>>>>> ArmPlatformPkg/ArmPlatformPkg.dsc(...): error 4000: Instance of library class [MemoryInitPeiLib] is not found
>>>>>>         in [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf] [ARM]
>>>>>>         consumed by module
>>>>>> [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf]
>>>>>>
>>>>>
>>>>> Hmm, that is surprising:
>>>>>
>>>>> $ git grep MemoryInitPeiLib
>>>>> ArmPlatformPkg/ArmJunoPkg/ArmJuno.dsc:
>>>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>> ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc:
>>>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>> ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc:
>>>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>> ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc:
>>>>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>>
>>>>> produces hits for the associated platforms, so I did not look at the
>>>>> .DSCs in detail. ArmVExpress-FVP-AArch64 does build, btw
>>>>>
>>>>> I will update the patch so that all .DSCs are updated with the missing library reference.
>>>>>
>>>>> --
>>>>> Ard.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>>>>> Sent: 08 April 2015 08:11
>>>>>> To: edk2-devel@lists.sourceforge.net; lersek@redhat.com; Olivier
>>>>>> Martin
>>>>>> Cc: roy.franz@linaro.org; leif.lindholm@linaro.org; Ard Biesheuvel
>>>>>> Subject: [PATCH v3 1/5] ArmPlatformPkg: do not fulfil
>>>>>> MemoryInitPeiLib dependency directly via .c file
>>>>>>
>>>>>> MemoryInitPeim short-circuits its MemoryInitPeiLib dependency by including the .c file directly. This prevents us from having a special implementation for ArmVirtualizationPkg that performs additional cache maintenance before enabling the MMU. So instead, make it depend on the library class.
>>>>>>
>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>> ---
>>>>>>  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc | 1 +
>>>>>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf             | 2 +-
>>>>>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf               | 2 +-
>>>>>>  3 files changed, 3 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git
>>>>>> a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>>>> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>>>> index 51c163286d1f..b03616842f81 100644
>>>>>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>>>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
>>>>>> @@ -77,6 +77,7 @@
>>>>>>
>>>>>> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerVirtCounterL
>>>>>> ArmGenericTimerCounterLib|i
>>>>>> b/ArmGenericTimerVirtCounterLib.inf
>>>>>>
>>>>>>
>>>>>> PlatformPeiLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformP
>>>>>> PlatformPeiLib|e
>>>>>> iLib/PlatformPeiLib.inf
>>>>>> +
>>>>>> + MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>>>
>>>>>> EfiResetSystemLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVir
>>>>>> EfiResetSystemLib|t
>>>>>> ualizationPsciResetSystemLib/ArmVirtualizationPsciResetSystemLib.inf
>>>>>>
>>>>>>    # ARM PL031 RTC Driver
>>>>>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>>> index 1e5b93e8a507..7f1976d60c31 100755
>>>>>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>>>>> @@ -17,7 +17,7 @@
>>>>>>    FILE_GUID                      = 55ddb6e0-70b5-11e0-b33e-0002a5d5c51b
>>>>>>    MODULE_TYPE                    = SEC
>>>>>>    VERSION_STRING                 = 1.0
>>>>>> -  LIBRARY_CLASS                  = PlatformPeiLib
>>>>>> +  LIBRARY_CLASS                  = MemoryInitPeiLib
>>>>>>
>>>>>>  [Sources]
>>>>>>    MemoryInitPeiLib.c
>>>>>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>>>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>>>> index 6374b63f8524..2c14a9c826ff 100755
>>>>>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>>>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>>>> @@ -28,7 +28,6 @@
>>>>>>
>>>>>>  [Sources]
>>>>>>    MemoryInitPeim.c
>>>>>> -  MemoryInitPeiLib.c
>>>>>>
>>>>>>  [Packages]
>>>>>>    MdePkg/MdePkg.dec
>>>>>> @@ -43,6 +42,7 @@
>>>>>>    HobLib
>>>>>>    ArmLib
>>>>>>    ArmPlatformLib
>>>>>> +  MemoryInitPeiLib
>>>>>>
>>>>>>  [Guids]
>>>>>>    gEfiMemoryTypeInformationGuid
>>>>>> --
>>>>>> 1.8.3.2
>>>>>>
>>>>>>
>>>>>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>>>>>
>>>>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>>>>> Registered in England & Wales, Company No:  2557590 ARM Holdings
>>>>>> plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>>>>> Registered in England & Wales, Company No:  2548782
>>>>>>
>>>>>
>>>>>
>>>>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>>>>
>>>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>>>> Registered in England & Wales, Company No:  2557590 ARM Holdings plc,
>>>>> Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in
>>>>> England & Wales, Company No:  2548782
>>>>
>>>>
>>>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>>>
>>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>>>> Registered in England & Wales, Company No:  2557590 ARM Holdings plc,
>>>> Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in
>>>> England & Wales, Company No:  2548782
>>>
>>>
>>> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
>>>
>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
>>> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
Ard Biesheuvel April 8, 2015, 11:43 a.m. UTC | #7
On 8 April 2015 at 13:38, Laszlo Ersek <lersek@redhat.com> wrote:
> On 04/08/15 13:33, Ard Biesheuvel wrote:
>> On 8 April 2015 at 13:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> On 8 April 2015 at 13:12, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>>> Is the reason why it does not work is dependency issue on other PEIM library?
>>>
>>> MODULE_TYPE must be either 'SEC' or 'PEIM' and not both. If we are
>>> going to use the library in modules of both flavors, it would be
>>> cleaner just to use BASE
>>>
>>
>> Actually, looking at it again, shouldn't it just be a PEIM module type?
>> I don't think any modules depend on MemoryInitPeiLib at all, let alone
>> any SEC modules.
>
> In a library instance INF file, there are two things to consider,
> MODULE_TYPE and the client module type list in LIBRARY_CLASS.
>
> I understand the restriction list in LIBRARY_CLASS. I never understood
> what MODULE_TYPE meant for a library instance. The INF file
> specification doesn't document it either -- if I remember correctly, I
> checked, and came up empty.
>

Actually, it appears to be optional, or at least, everything still
builds fine after I remove it.

I will change it to 'BASE' instead, which also builds fine. Should I
add the 'SEC PEIM' restriction? It is responsible for setting up the
page tables and enabling the MMU etc so you wouldn't be able to use it
elsewhere anyway.

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
Ard Biesheuvel April 8, 2015, 11:44 a.m. UTC | #8
On 8 April 2015 at 13:43, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 8 April 2015 at 13:38, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 04/08/15 13:33, Ard Biesheuvel wrote:
>>> On 8 April 2015 at 13:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>>> On 8 April 2015 at 13:12, Olivier Martin <Olivier.Martin@arm.com> wrote:
>>>>> Is the reason why it does not work is dependency issue on other PEIM library?
>>>>
>>>> MODULE_TYPE must be either 'SEC' or 'PEIM' and not both. If we are
>>>> going to use the library in modules of both flavors, it would be
>>>> cleaner just to use BASE
>>>>
>>>
>>> Actually, looking at it again, shouldn't it just be a PEIM module type?
>>> I don't think any modules depend on MemoryInitPeiLib at all, let alone
>>> any SEC modules.
>>
>> In a library instance INF file, there are two things to consider,
>> MODULE_TYPE and the client module type list in LIBRARY_CLASS.
>>
>> I understand the restriction list in LIBRARY_CLASS. I never understood
>> what MODULE_TYPE meant for a library instance. The INF file
>> specification doesn't document it either -- if I remember correctly, I
>> checked, and came up empty.
>>
>
> Actually, it appears to be optional, or at least, everything still
> builds fine after I remove it.
>

Ehm, no it doesn't. Not sure what happened there, but it does insist
on a MODULE_TYPE entry for libraries


> I will change it to 'BASE' instead, which also builds fine. Should I
> add the 'SEC PEIM' restriction? It is responsible for setting up the
> page tables and enabling the MMU etc so you wouldn't be able to use it
> elsewhere anyway.

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
diff mbox

Patch

diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
index 51c163286d1f..b03616842f81 100644
--- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
+++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualization.dsc.inc
@@ -77,6 +77,7 @@ 
   ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.inf
 
   PlatformPeiLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
+  MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
   EfiResetSystemLib|ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPsciResetSystemLib/ArmVirtualizationPsciResetSystemLib.inf
 
   # ARM PL031 RTC Driver
diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
index 1e5b93e8a507..7f1976d60c31 100755
--- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
+++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
@@ -17,7 +17,7 @@ 
   FILE_GUID                      = 55ddb6e0-70b5-11e0-b33e-0002a5d5c51b
   MODULE_TYPE                    = SEC
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = PlatformPeiLib
+  LIBRARY_CLASS                  = MemoryInitPeiLib
 
 [Sources]
   MemoryInitPeiLib.c
diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
index 6374b63f8524..2c14a9c826ff 100755
--- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
+++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
@@ -28,7 +28,6 @@ 
 
 [Sources]
   MemoryInitPeim.c
-  MemoryInitPeiLib.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -43,6 +42,7 @@ 
   HobLib
   ArmLib
   ArmPlatformLib
+  MemoryInitPeiLib
 
 [Guids]
   gEfiMemoryTypeInformationGuid