[edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[Leif Lindholm]

Related to https://bugzilla.tianocore.org/show_bug.cgi?id=4121, but not
resolving it. (Nearly?) all of ArmPkg describes industry standard
behaviour, and hence according to general rules, ought to live in MdePkg.

Addressing this will however be a substantial task.
Take a first step by moving the ArmLib interface definition to MdePkg,
as discussed in
https://edk2.groups.io/g/devel/topic/patch_v5_2_6/102725178

Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Leif Lindholm <quic_llindhol@quicinc.com>
---

This should have no functional differences (and the set of
platforms I have test built didn't find any problems).
This may result in some modules depending on ArmPkg only
for ArmLib now being able to drop ArmPkg dependency.

 ArmPkg/ArmPkg.dec                           | 4 ----
 MdePkg/MdePkg.dec                           | 5 +++++
 {ArmPkg => MdePkg}/Include/Library/ArmLib.h | 0
 Maintainers.txt                             | 6 ++++++
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 7fe2b9bca43b..4ce59f3e1fbd 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -50,10 +50,6 @@ [LibraryClasses.common]
   #
   ArmHvcLib|Include/Library/ArmHvcLib.h
 
-  ##  @libraryclass  Provides an interface to Arm registers.
-  #
-  ArmLib|Include/Library/ArmLib.h
-
   ##  @libraryclass  Provides a Mmu interface.
   #
   ArmMmuLib|Include/Library/ArmMmuLib.h
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index ac54338089e8..78e18ee444cd 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -339,6 +339,11 @@ [LibraryClasses.RISCV64]
   ##  @libraryclass  Provides function to make ecalls to SBI
   BaseRiscVSbiLib|Include/Library/BaseRiscVSbiLib.h
 
+[LibraryClasses.ARM, LibraryClasses.AARCH64]
+  ##  @libraryclass  Provides an interface to Arm registers.
+  #
+  ArmLib|Include/Library/ArmLib.h
+
 [Guids]
   #
   # GUID defined in UEFI2.1/UEFI2.0/EFI1.1
diff --git a/ArmPkg/Include/Library/ArmLib.h b/MdePkg/Include/Library/ArmLib.h
similarity index 100%
rename from ArmPkg/Include/Library/ArmLib.h
rename to MdePkg/Include/Library/ArmLib.h
diff --git a/Maintainers.txt b/Maintainers.txt
index 7c0b4cb58cfd..0315fa23dfce 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -450,6 +450,12 @@ M: Abner Chang <abner.chang@amd.com> [changab]
 R: Abdul Lateef Attar <AbdulLateef.Attar@amd.com> [abdattar]
 R: Nickle Wang <nicklew@nvidia.com> [nicklela]
 
+MdePkg: ARM/AARCH64 standard interfaces
+F: MdePkg/Include/Library/ArmLib.h
+M: Leif Lindholm <quic_llindhol@quicinc.com> [leiflindholm]
+M: Ard Biesheuvel <ardb+tianocore@kernel.org> [ardbiesheuvel]
+R: Sami Mujawar <sami.mujawar@arm.com> [samimujawar]
+
 NetworkPkg
 F: NetworkPkg/
 W: https://github.com/tianocore/tianocore.github.io/wiki/NetworkPkg
-- 
2.30.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111566): https://edk2.groups.io/g/devel/message/111566
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[PierreGondois]

Hello Leif,

I think the following files:
- ArmPkg/Include/Chipset/AArch64.h
- ArmPkg/Include/Chipset/AArch64Mmu.h
- ArmPkg/Include/Chipset/ArmV7.h
- ArmPkg/Include/Chipset/ArmV7Mmu.h
also need to be moved to the MdePkg. Otherwise the MdePkg
would depend on the ArmPkg, cf. ArmLib.h:

#ifdef MDE_CPU_ARM
   #include <Chipset/ArmV7.h>
#elif defined (MDE_CPU_AARCH64)
   #include <Chipset/AArch64.h>

Depending on where these files are placed in the MdePkg, this
might imply some renaming to include them. Meaning that all
   #include <Chipset/AArch64.h>
might need to be renamed
   #include <[New path]/AArch64.h>
but it should be ok.

A ArmLibNull.inf library might also need to be created. If the
OpensslLibFullAccel.inf module will depend on the ArmLib library,
a Null implementation will be necessary for non-Arm architectures.

Otherwise I could apply and run the CryptoPkg/Arm native instructions
patchset on top of this patch.

---

As a side note, it also seems that:
- ArmPkg/Include/Chipset/ArmCortexA5x.h
   isn't used anymore in edk2/edk2-platorms
- ArmPkg/Include/Chipset/ArmCortexA9.h
   is barely used in edk2-platforms.
Maybe the files should have been removed/simplified as part of
- cffa7925a293 ("ArmPkg: remove ArmCpuLib header and implementations")
- a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")

Regards,
Pierre

On 11/21/23 17:42, Leif Lindholm wrote:
> Related to https://bugzilla.tianocore.org/show_bug.cgi?id=4121, but not
> resolving it. (Nearly?) all of ArmPkg describes industry standard
> behaviour, and hence according to general rules, ought to live in MdePkg.
> 
> Addressing this will however be a substantial task.
> Take a first step by moving the ArmLib interface definition to MdePkg,
> as discussed in
> https://edk2.groups.io/g/devel/topic/patch_v5_2_6/102725178
> 
> Cc: Pierre Gondois <pierre.gondois@arm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Leif Lindholm <quic_llindhol@quicinc.com>
> ---
> 
> This should have no functional differences (and the set of
> platforms I have test built didn't find any problems).
> This may result in some modules depending on ArmPkg only
> for ArmLib now being able to drop ArmPkg dependency.
> 
>   ArmPkg/ArmPkg.dec                           | 4 ----
>   MdePkg/MdePkg.dec                           | 5 +++++
>   {ArmPkg => MdePkg}/Include/Library/ArmLib.h | 0
>   Maintainers.txt                             | 6 ++++++
>   4 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index 7fe2b9bca43b..4ce59f3e1fbd 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -50,10 +50,6 @@ [LibraryClasses.common]
>     #
>     ArmHvcLib|Include/Library/ArmHvcLib.h
>   
> -  ##  @libraryclass  Provides an interface to Arm registers.
> -  #
> -  ArmLib|Include/Library/ArmLib.h
> -
>     ##  @libraryclass  Provides a Mmu interface.
>     #
>     ArmMmuLib|Include/Library/ArmMmuLib.h
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index ac54338089e8..78e18ee444cd 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -339,6 +339,11 @@ [LibraryClasses.RISCV64]
>     ##  @libraryclass  Provides function to make ecalls to SBI
>     BaseRiscVSbiLib|Include/Library/BaseRiscVSbiLib.h
>   
> +[LibraryClasses.ARM, LibraryClasses.AARCH64]
> +  ##  @libraryclass  Provides an interface to Arm registers.
> +  #
> +  ArmLib|Include/Library/ArmLib.h
> +
>   [Guids]
>     #
>     # GUID defined in UEFI2.1/UEFI2.0/EFI1.1
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/MdePkg/Include/Library/ArmLib.h
> similarity index 100%
> rename from ArmPkg/Include/Library/ArmLib.h
> rename to MdePkg/Include/Library/ArmLib.h
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 7c0b4cb58cfd..0315fa23dfce 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -450,6 +450,12 @@ M: Abner Chang <abner.chang@amd.com> [changab]
>   R: Abdul Lateef Attar <AbdulLateef.Attar@amd.com> [abdattar]
>   R: Nickle Wang <nicklew@nvidia.com> [nicklela]
>   
> +MdePkg: ARM/AARCH64 standard interfaces
> +F: MdePkg/Include/Library/ArmLib.h
> +M: Leif Lindholm <quic_llindhol@quicinc.com> [leiflindholm]
> +M: Ard Biesheuvel <ardb+tianocore@kernel.org> [ardbiesheuvel]
> +R: Sami Mujawar <sami.mujawar@arm.com> [samimujawar]
> +
>   NetworkPkg
>   F: NetworkPkg/
>   W: https://github.com/tianocore/tianocore.github.io/wiki/NetworkPkg


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112809): https://edk2.groups.io/g/devel/message/112809
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[PierreGondois]

Hello,
I was wondering if moving ArmLib.h to the MdePkg was still considered ?

Regards,
Pierre

On 12/21/23 12:13, Pierre Gondois wrote:
> Hello Leif,
> 
> I think the following files:
> - ArmPkg/Include/Chipset/AArch64.h
> - ArmPkg/Include/Chipset/AArch64Mmu.h
> - ArmPkg/Include/Chipset/ArmV7.h
> - ArmPkg/Include/Chipset/ArmV7Mmu.h
> also need to be moved to the MdePkg. Otherwise the MdePkg
> would depend on the ArmPkg, cf. ArmLib.h:
> 
> #ifdef MDE_CPU_ARM
>     #include <Chipset/ArmV7.h>
> #elif defined (MDE_CPU_AARCH64)
>     #include <Chipset/AArch64.h>
> 
> Depending on where these files are placed in the MdePkg, this
> might imply some renaming to include them. Meaning that all
>     #include <Chipset/AArch64.h>
> might need to be renamed
>     #include <[New path]/AArch64.h>
> but it should be ok.
> 
> A ArmLibNull.inf library might also need to be created. If the
> OpensslLibFullAccel.inf module will depend on the ArmLib library,
> a Null implementation will be necessary for non-Arm architectures.
> 
> Otherwise I could apply and run the CryptoPkg/Arm native instructions
> patchset on top of this patch.
> 
> ---
> 
> As a side note, it also seems that:
> - ArmPkg/Include/Chipset/ArmCortexA5x.h
>     isn't used anymore in edk2/edk2-platorms
> - ArmPkg/Include/Chipset/ArmCortexA9.h
>     is barely used in edk2-platforms.
> Maybe the files should have been removed/simplified as part of
> - cffa7925a293 ("ArmPkg: remove ArmCpuLib header and implementations")
> - a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")
> 
> Regards,
> Pierre
> 
> On 11/21/23 17:42, Leif Lindholm wrote:
>> Related to https://bugzilla.tianocore.org/show_bug.cgi?id=4121, but not
>> resolving it. (Nearly?) all of ArmPkg describes industry standard
>> behaviour, and hence according to general rules, ought to live in MdePkg.
>>
>> Addressing this will however be a substantial task.
>> Take a first step by moving the ArmLib interface definition to MdePkg,
>> as discussed in
>> https://edk2.groups.io/g/devel/topic/patch_v5_2_6/102725178
>>
>> Cc: Pierre Gondois <pierre.gondois@arm.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>> Signed-off-by: Leif Lindholm <quic_llindhol@quicinc.com>
>> ---
>>
>> This should have no functional differences (and the set of
>> platforms I have test built didn't find any problems).
>> This may result in some modules depending on ArmPkg only
>> for ArmLib now being able to drop ArmPkg dependency.
>>
>>    ArmPkg/ArmPkg.dec                           | 4 ----
>>    MdePkg/MdePkg.dec                           | 5 +++++
>>    {ArmPkg => MdePkg}/Include/Library/ArmLib.h | 0
>>    Maintainers.txt                             | 6 ++++++
>>    4 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
>> index 7fe2b9bca43b..4ce59f3e1fbd 100644
>> --- a/ArmPkg/ArmPkg.dec
>> +++ b/ArmPkg/ArmPkg.dec
>> @@ -50,10 +50,6 @@ [LibraryClasses.common]
>>      #
>>      ArmHvcLib|Include/Library/ArmHvcLib.h
>>    
>> -  ##  @libraryclass  Provides an interface to Arm registers.
>> -  #
>> -  ArmLib|Include/Library/ArmLib.h
>> -
>>      ##  @libraryclass  Provides a Mmu interface.
>>      #
>>      ArmMmuLib|Include/Library/ArmMmuLib.h
>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>> index ac54338089e8..78e18ee444cd 100644
>> --- a/MdePkg/MdePkg.dec
>> +++ b/MdePkg/MdePkg.dec
>> @@ -339,6 +339,11 @@ [LibraryClasses.RISCV64]
>>      ##  @libraryclass  Provides function to make ecalls to SBI
>>      BaseRiscVSbiLib|Include/Library/BaseRiscVSbiLib.h
>>    
>> +[LibraryClasses.ARM, LibraryClasses.AARCH64]
>> +  ##  @libraryclass  Provides an interface to Arm registers.
>> +  #
>> +  ArmLib|Include/Library/ArmLib.h
>> +
>>    [Guids]
>>      #
>>      # GUID defined in UEFI2.1/UEFI2.0/EFI1.1
>> diff --git a/ArmPkg/Include/Library/ArmLib.h b/MdePkg/Include/Library/ArmLib.h
>> similarity index 100%
>> rename from ArmPkg/Include/Library/ArmLib.h
>> rename to MdePkg/Include/Library/ArmLib.h
>> diff --git a/Maintainers.txt b/Maintainers.txt
>> index 7c0b4cb58cfd..0315fa23dfce 100644
>> --- a/Maintainers.txt
>> +++ b/Maintainers.txt
>> @@ -450,6 +450,12 @@ M: Abner Chang <abner.chang@amd.com> [changab]
>>    R: Abdul Lateef Attar <AbdulLateef.Attar@amd.com> [abdattar]
>>    R: Nickle Wang <nicklew@nvidia.com> [nicklela]
>>    
>> +MdePkg: ARM/AARCH64 standard interfaces
>> +F: MdePkg/Include/Library/ArmLib.h
>> +M: Leif Lindholm <quic_llindhol@quicinc.com> [leiflindholm]
>> +M: Ard Biesheuvel <ardb+tianocore@kernel.org> [ardbiesheuvel]
>> +R: Sami Mujawar <sami.mujawar@arm.com> [samimujawar]
>> +
>>    NetworkPkg
>>    F: NetworkPkg/
>>    W: https://github.com/tianocore/tianocore.github.io/wiki/NetworkPkg


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115303): https://edk2.groups.io/g/devel/message/115303
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[Leif Lindholm]

Apologies, I think your original email got lost in my "just after going 
on christmas holidays dead zone" which I managed to mess up my read 
staus on...

Let me dig up that one and respond there.

/
     Leif

On 2024-02-09 13:25, Pierre Gondois wrote:
> Hello,
> I was wondering if moving ArmLib.h to the MdePkg was still considered ?
> 
> Regards,
> Pierre
> 
> On 12/21/23 12:13, Pierre Gondois wrote:
>> Hello Leif,
>>
>> I think the following files:
>> - ArmPkg/Include/Chipset/AArch64.h
>> - ArmPkg/Include/Chipset/AArch64Mmu.h
>> - ArmPkg/Include/Chipset/ArmV7.h
>> - ArmPkg/Include/Chipset/ArmV7Mmu.h
>> also need to be moved to the MdePkg. Otherwise the MdePkg
>> would depend on the ArmPkg, cf. ArmLib.h:
>>
>> #ifdef MDE_CPU_ARM
>>     #include <Chipset/ArmV7.h>
>> #elif defined (MDE_CPU_AARCH64)
>>     #include <Chipset/AArch64.h>
>>
>> Depending on where these files are placed in the MdePkg, this
>> might imply some renaming to include them. Meaning that all
>>     #include <Chipset/AArch64.h>
>> might need to be renamed
>>     #include <[New path]/AArch64.h>
>> but it should be ok.
>>
>> A ArmLibNull.inf library might also need to be created. If the
>> OpensslLibFullAccel.inf module will depend on the ArmLib library,
>> a Null implementation will be necessary for non-Arm architectures.
>>
>> Otherwise I could apply and run the CryptoPkg/Arm native instructions
>> patchset on top of this patch.
>>
>> ---
>>
>> As a side note, it also seems that:
>> - ArmPkg/Include/Chipset/ArmCortexA5x.h
>>     isn't used anymore in edk2/edk2-platorms
>> - ArmPkg/Include/Chipset/ArmCortexA9.h
>>     is barely used in edk2-platforms.
>> Maybe the files should have been removed/simplified as part of
>> - cffa7925a293 ("ArmPkg: remove ArmCpuLib header and implementations")
>> - a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")
>>
>> Regards,
>> Pierre
>>
>> On 11/21/23 17:42, Leif Lindholm wrote:
>>> Related to https://bugzilla.tianocore.org/show_bug.cgi?id=4121, but not
>>> resolving it. (Nearly?) all of ArmPkg describes industry standard
>>> behaviour, and hence according to general rules, ought to live in 
>>> MdePkg.
>>>
>>> Addressing this will however be a substantial task.
>>> Take a first step by moving the ArmLib interface definition to MdePkg,
>>> as discussed in
>>> https://edk2.groups.io/g/devel/topic/patch_v5_2_6/102725178
>>>
>>> Cc: Pierre Gondois <pierre.gondois@arm.com>
>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>>> Signed-off-by: Leif Lindholm <quic_llindhol@quicinc.com>
>>> ---
>>>
>>> This should have no functional differences (and the set of
>>> platforms I have test built didn't find any problems).
>>> This may result in some modules depending on ArmPkg only
>>> for ArmLib now being able to drop ArmPkg dependency.
>>>
>>>    ArmPkg/ArmPkg.dec                           | 4 ----
>>>    MdePkg/MdePkg.dec                           | 5 +++++
>>>    {ArmPkg => MdePkg}/Include/Library/ArmLib.h | 0
>>>    Maintainers.txt                             | 6 ++++++
>>>    4 files changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
>>> index 7fe2b9bca43b..4ce59f3e1fbd 100644
>>> --- a/ArmPkg/ArmPkg.dec
>>> +++ b/ArmPkg/ArmPkg.dec
>>> @@ -50,10 +50,6 @@ [LibraryClasses.common]
>>>      #
>>>      ArmHvcLib|Include/Library/ArmHvcLib.h
>>> -  ##  @libraryclass  Provides an interface to Arm registers.
>>> -  #
>>> -  ArmLib|Include/Library/ArmLib.h
>>> -
>>>      ##  @libraryclass  Provides a Mmu interface.
>>>      #
>>>      ArmMmuLib|Include/Library/ArmMmuLib.h
>>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>>> index ac54338089e8..78e18ee444cd 100644
>>> --- a/MdePkg/MdePkg.dec
>>> +++ b/MdePkg/MdePkg.dec
>>> @@ -339,6 +339,11 @@ [LibraryClasses.RISCV64]
>>>      ##  @libraryclass  Provides function to make ecalls to SBI
>>>      BaseRiscVSbiLib|Include/Library/BaseRiscVSbiLib.h
>>> +[LibraryClasses.ARM, LibraryClasses.AARCH64]
>>> +  ##  @libraryclass  Provides an interface to Arm registers.
>>> +  #
>>> +  ArmLib|Include/Library/ArmLib.h
>>> +
>>>    [Guids]
>>>      #
>>>      # GUID defined in UEFI2.1/UEFI2.0/EFI1.1
>>> diff --git a/ArmPkg/Include/Library/ArmLib.h 
>>> b/MdePkg/Include/Library/ArmLib.h
>>> similarity index 100%
>>> rename from ArmPkg/Include/Library/ArmLib.h
>>> rename to MdePkg/Include/Library/ArmLib.h
>>> diff --git a/Maintainers.txt b/Maintainers.txt
>>> index 7c0b4cb58cfd..0315fa23dfce 100644
>>> --- a/Maintainers.txt
>>> +++ b/Maintainers.txt
>>> @@ -450,6 +450,12 @@ M: Abner Chang <abner.chang@amd.com> [changab]
>>>    R: Abdul Lateef Attar <AbdulLateef.Attar@amd.com> [abdattar]
>>>    R: Nickle Wang <nicklew@nvidia.com> [nicklela]
>>> +MdePkg: ARM/AARCH64 standard interfaces
>>> +F: MdePkg/Include/Library/ArmLib.h
>>> +M: Leif Lindholm <quic_llindhol@quicinc.com> [leiflindholm]
>>> +M: Ard Biesheuvel <ardb+tianocore@kernel.org> [ardbiesheuvel]
>>> +R: Sami Mujawar <sami.mujawar@arm.com> [samimujawar]
>>> +
>>>    NetworkPkg
>>>    F: NetworkPkg/
>>>    W: https://github.com/tianocore/tianocore.github.io/wiki/NetworkPkg



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115306): https://edk2.groups.io/g/devel/message/115306
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[Leif Lindholm]

Hi Pierre,

You appear to have stumbled onto one of those areas we haven't gotten 
around to cleaning up yet...

On 2023-12-21 11:13, Pierre Gondois wrote:
> Hello Leif,
> 
> I think the following files:
> - ArmPkg/Include/Chipset/AArch64.h
> - ArmPkg/Include/Chipset/AArch64Mmu.h
> - ArmPkg/Include/Chipset/ArmV7.h
> - ArmPkg/Include/Chipset/ArmV7Mmu.h

If we moved these, I think we should also rename the latter two 
AArch32.h/AArch32Mmu.h.

> also need to be moved to the MdePkg. Otherwise the MdePkg
> would depend on the ArmPkg, cf. ArmLib.h:
 >> #ifdef MDE_CPU_ARM
>    #include <Chipset/ArmV7.h>
> #elif defined (MDE_CPU_AARCH64)
>    #include <Chipset/AArch64.h>

Sure.

> Depending on where these files are placed in the MdePkg, this
> might imply some renaming to include them. Meaning that all
>    #include <Chipset/AArch64.h>
> might need to be renamed
>    #include <[New path]/AArch64.h>
> but it should be ok.

An annoyance, but an improvement.

> A ArmLibNull.inf library might also need to be created. If the
> OpensslLibFullAccel.inf module will depend on the ArmLib library,
> a Null implementation will be necessary for non-Arm architectures.

Can ArmLib be declared under a [LibraryClasses.AArch64, 
LibraryClasses.ARM]? Have I forgotten something that we discussed back 
in ... November?

> Otherwise I could apply and run the CryptoPkg/Arm native instructions
> patchset on top of this patch.
> 
> ---
> 
> As a side note, it also seems that:
> - ArmPkg/Include/Chipset/ArmCortexA5x.h
>    isn't used anymore in edk2/edk2-platorms
> - ArmPkg/Include/Chipset/ArmCortexA9.h
>    is barely used in edk2-platforms.
> Maybe the files should have been removed/simplified as part of
> - cffa7925a293 ("ArmPkg: remove ArmCpuLib header and implementations")
> - a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")

I think you're right.
Well, ArmCortexA9.h is still *used*, but I can't imagine the Arm branch 
of ArmVExpressLib has been build by anyone for some time. And surely the 
inclusion of ArmVExpressLibSec in ArmVExpress-FVP-AArch64.dsc is 
superfluous (such that that .inf can be deleted)?

Regards,

Leif

> Regards,
> Pierre
> 
> On 11/21/23 17:42, Leif Lindholm wrote:
>> Related to https://bugzilla.tianocore.org/show_bug.cgi?id=4121, but not
>> resolving it. (Nearly?) all of ArmPkg describes industry standard
>> behaviour, and hence according to general rules, ought to live in MdePkg.
>>
>> Addressing this will however be a substantial task.
>> Take a first step by moving the ArmLib interface definition to MdePkg,
>> as discussed in
>> https://edk2.groups.io/g/devel/topic/patch_v5_2_6/102725178
>>
>> Cc: Pierre Gondois <pierre.gondois@arm.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>> Signed-off-by: Leif Lindholm <quic_llindhol@quicinc.com>
>> ---
>>
>> This should have no functional differences (and the set of
>> platforms I have test built didn't find any problems).
>> This may result in some modules depending on ArmPkg only
>> for ArmLib now being able to drop ArmPkg dependency.
>>
>>   ArmPkg/ArmPkg.dec                           | 4 ----
>>   MdePkg/MdePkg.dec                           | 5 +++++
>>   {ArmPkg => MdePkg}/Include/Library/ArmLib.h | 0
>>   Maintainers.txt                             | 6 ++++++
>>   4 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
>> index 7fe2b9bca43b..4ce59f3e1fbd 100644
>> --- a/ArmPkg/ArmPkg.dec
>> +++ b/ArmPkg/ArmPkg.dec
>> @@ -50,10 +50,6 @@ [LibraryClasses.common]
>>     #
>>     ArmHvcLib|Include/Library/ArmHvcLib.h
>> -  ##  @libraryclass  Provides an interface to Arm registers.
>> -  #
>> -  ArmLib|Include/Library/ArmLib.h
>> -
>>     ##  @libraryclass  Provides a Mmu interface.
>>     #
>>     ArmMmuLib|Include/Library/ArmMmuLib.h
>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>> index ac54338089e8..78e18ee444cd 100644
>> --- a/MdePkg/MdePkg.dec
>> +++ b/MdePkg/MdePkg.dec
>> @@ -339,6 +339,11 @@ [LibraryClasses.RISCV64]
>>     ##  @libraryclass  Provides function to make ecalls to SBI
>>     BaseRiscVSbiLib|Include/Library/BaseRiscVSbiLib.h
>> +[LibraryClasses.ARM, LibraryClasses.AARCH64]
>> +  ##  @libraryclass  Provides an interface to Arm registers.
>> +  #
>> +  ArmLib|Include/Library/ArmLib.h
>> +
>>   [Guids]
>>     #
>>     # GUID defined in UEFI2.1/UEFI2.0/EFI1.1
>> diff --git a/ArmPkg/Include/Library/ArmLib.h 
>> b/MdePkg/Include/Library/ArmLib.h
>> similarity index 100%
>> rename from ArmPkg/Include/Library/ArmLib.h
>> rename to MdePkg/Include/Library/ArmLib.h
>> diff --git a/Maintainers.txt b/Maintainers.txt
>> index 7c0b4cb58cfd..0315fa23dfce 100644
>> --- a/Maintainers.txt
>> +++ b/Maintainers.txt
>> @@ -450,6 +450,12 @@ M: Abner Chang <abner.chang@amd.com> [changab]
>>   R: Abdul Lateef Attar <AbdulLateef.Attar@amd.com> [abdattar]
>>   R: Nickle Wang <nicklew@nvidia.com> [nicklela]
>> +MdePkg: ARM/AARCH64 standard interfaces
>> +F: MdePkg/Include/Library/ArmLib.h
>> +M: Leif Lindholm <quic_llindhol@quicinc.com> [leiflindholm]
>> +M: Ard Biesheuvel <ardb+tianocore@kernel.org> [ardbiesheuvel]
>> +R: Sami Mujawar <sami.mujawar@arm.com> [samimujawar]
>> +
>>   NetworkPkg
>>   F: NetworkPkg/
>>   W: https://github.com/tianocore/tianocore.github.io/wiki/NetworkPkg



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115307): https://edk2.groups.io/g/devel/message/115307
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[PierreGondois]

Hello Leif,

On 2/9/24 16:29, Leif Lindholm wrote:
> Hi Pierre,
> 
> You appear to have stumbled onto one of those areas we haven't gotten
> around to cleaning up yet...
> 
> On 2023-12-21 11:13, Pierre Gondois wrote:
>> Hello Leif,
>>
>> I think the following files:
>> - ArmPkg/Include/Chipset/AArch64.h
>> - ArmPkg/Include/Chipset/AArch64Mmu.h
>> - ArmPkg/Include/Chipset/ArmV7.h
>> - ArmPkg/Include/Chipset/ArmV7Mmu.h
> 
> If we moved these, I think we should also rename the latter two
> AArch32.h/AArch32Mmu.h.

Yes ok.

> 
>> also need to be moved to the MdePkg. Otherwise the MdePkg
>> would depend on the ArmPkg, cf. ArmLib.h:
>   >> #ifdef MDE_CPU_ARM
>>     #include <Chipset/ArmV7.h>
>> #elif defined (MDE_CPU_AARCH64)
>>     #include <Chipset/AArch64.h>
> 
> Sure.
> 
>> Depending on where these files are placed in the MdePkg, this
>> might imply some renaming to include them. Meaning that all
>>     #include <Chipset/AArch64.h>
>> might need to be renamed
>>     #include <[New path]/AArch64.h>
>> but it should be ok.
> 
> An annoyance, but an improvement.
> 
>> A ArmLibNull.inf library might also need to be created. If the
>> OpensslLibFullAccel.inf module will depend on the ArmLib library,
>> a Null implementation will be necessary for non-Arm architectures.
> 
> Can ArmLib be declared under a [LibraryClasses.AArch64,
> LibraryClasses.ARM]? Have I forgotten something that we discussed back
> in ... November?

 From [1], it seems the MdePkg/CryptoPkg should build without a dependency
on the ArmPkg. This is currently not really the case. cf. [2].

However, having a ArmLibNull implementation in the MdePkg would allow to
avoid going in this direction when providing libraries to CryptoPkg.dsc.

(Just in case, I think this ArmLibNull is a minor point.)

[1] https://edk2.groups.io/g/devel/message/111545
[2] https://github.com/tianocore/edk2/blob/8801c75b4d77c2e6e06b3ddc8560e0db590f6342/CryptoPkg/CryptoPkg.dsc#L117

> 
>> Otherwise I could apply and run the CryptoPkg/Arm native instructions
>> patchset on top of this patch.
>>
>> ---
>>
>> As a side note, it also seems that:
>> - ArmPkg/Include/Chipset/ArmCortexA5x.h
>>     isn't used anymore in edk2/edk2-platorms
>> - ArmPkg/Include/Chipset/ArmCortexA9.h
>>     is barely used in edk2-platforms.
>> Maybe the files should have been removed/simplified as part of
>> - cffa7925a293 ("ArmPkg: remove ArmCpuLib header and implementations")
>> - a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")
> 
> I think you're right.
> Well, ArmCortexA9.h is still *used*, but I can't imagine the Arm branch
> of ArmVExpressLib has been build by anyone for some time. And surely the
> inclusion of ArmVExpressLibSec in ArmVExpress-FVP-AArch64.dsc is
> superfluous (such that that .inf can be deleted)?

The file could just be moved in the Library. I assume you/Sami/Ard
will know more on the usage of the library itself,

Regards,
Pierre


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115367): https://edk2.groups.io/g/devel/message/115367
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[Leif Lindholm]

Jiewen, can you clarify what you said back in
https://edk2.groups.io/g/devel/message/111551
?

On 2024-02-12 17:24, Pierre Gondois wrote:
>>> A ArmLibNull.inf library might also need to be created. If the
>>> OpensslLibFullAccel.inf module will depend on the ArmLib library,
>>> a Null implementation will be necessary for non-Arm architectures.
>>
>> Can ArmLib be declared under a [LibraryClasses.AArch64,
>> LibraryClasses.ARM]? Have I forgotten something that we discussed back
>> in ... November?
> 
>  From [1], it seems the MdePkg/CryptoPkg should build without a dependency
> on the ArmPkg. This is currently not really the case. cf. [2].
> 
> However, having a ArmLibNull implementation in the MdePkg would allow to
> avoid going in this direction when providing libraries to CryptoPkg.dsc.
>
> (Just in case, I think this ArmLibNull is a minor point.)

Well, sure, it is now.
Until we need a RiscV64LibNull, LoongarchLibNull, ...

> [1] https://edk2.groups.io/g/devel/message/111545
> [2] 
> https://github.com/tianocore/edk2/blob/8801c75b4d77c2e6e06b3ddc8560e0db590f6342/CryptoPkg/CryptoPkg.dsc#L117
> 
>>
>>> Otherwise I could apply and run the CryptoPkg/Arm native instructions
>>> patchset on top of this patch.
>>>
>>> ---
>>>
>>> As a side note, it also seems that:
>>> - ArmPkg/Include/Chipset/ArmCortexA5x.h
>>>     isn't used anymore in edk2/edk2-platorms
>>> - ArmPkg/Include/Chipset/ArmCortexA9.h
>>>     is barely used in edk2-platforms.
>>> Maybe the files should have been removed/simplified as part of
>>> - cffa7925a293 ("ArmPkg: remove ArmCpuLib header and implementations")
>>> - a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")
>>
>> I think you're right.
>> Well, ArmCortexA9.h is still *used*, but I can't imagine the Arm branch
>> of ArmVExpressLib has been build by anyone for some time. And surely the
>> inclusion of ArmVExpressLibSec in ArmVExpress-FVP-AArch64.dsc is
>> superfluous (such that that .inf can be deleted)?
> 
> The file could just be moved in the Library. I assume you/Sami/Ard
> will know more on the usage of the library itself,

Sami?

/
     Leif




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115368): https://edk2.groups.io/g/devel/message/115368
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[Sami Mujawar]

Hi Leif,

Please see my reply inline marked [SAMI].

Regards,

Sami Mujawar

On 12/02/2024, 17:38, "Leif Lindholm" <quic_llindhol@quicinc.com <mailto:quic_llindhol@quicinc.com>> wrote:


Jiewen, can you clarify what you said back in
https://edk2.groups.io/g/devel/message/111551 <https://edk2.groups.io/g/devel/message/111551>
?


On 2024-02-12 17:24, Pierre Gondois wrote:
>>> A ArmLibNull.inf library might also need to be created. If the
>>> OpensslLibFullAccel.inf module will depend on the ArmLib library,
>>> a Null implementation will be necessary for non-Arm architectures.
>>
>> Can ArmLib be declared under a [LibraryClasses.AArch64,
>> LibraryClasses.ARM]? Have I forgotten something that we discussed back
>> in ... November?
> 
> From [1], it seems the MdePkg/CryptoPkg should build without a dependency
> on the ArmPkg. This is currently not really the case. cf. [2].
> 
> However, having a ArmLibNull implementation in the MdePkg would allow to
> avoid going in this direction when providing libraries to CryptoPkg.dsc.
>
> (Just in case, I think this ArmLibNull is a minor point.)


Well, sure, it is now.
Until we need a RiscV64LibNull, LoongarchLibNull, ...


> [1] https://edk2.groups.io/g/devel/message/111545 <https://edk2.groups.io/g/devel/message/111545>
> [2] 
> https://github.com/tianocore/edk2/blob/8801c75b4d77c2e6e06b3ddc8560e0db590f6342/CryptoPkg/CryptoPkg.dsc#L117 <https://github.com/tianocore/edk2/blob/8801c75b4d77c2e6e06b3ddc8560e0db590f6342/CryptoPkg/CryptoPkg.dsc#L117>
> 
>>
>>> Otherwise I could apply and run the CryptoPkg/Arm native instructions
>>> patchset on top of this patch.
>>>
>>> ---
>>>
>>> As a side note, it also seems that:
>>> - ArmPkg/Include/Chipset/ArmCortexA5x.h
>>> isn't used anymore in edk2/edk2-platorms
>>> - ArmPkg/Include/Chipset/ArmCortexA9.h
>>> is barely used in edk2-platforms.
>>> Maybe the files should have been removed/simplified as part of
>>> - cffa7925a293 ("ArmPkg: remove ArmCpuLib header and implementations")
>>> - a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")
>>
>> I think you're right.
>> Well, ArmCortexA9.h is still *used*, but I can't imagine the Arm branch
>> of ArmVExpressLib has been build by anyone for some time. And surely the
>> inclusion of ArmVExpressLibSec in ArmVExpress-FVP-AArch64.dsc is
>> superfluous (such that that .inf can be deleted)?
> 
> The file could just be moved in the Library. I assume you/Sami/Ard
> will know more on the usage of the library itself,


Sami?
[SAMI] I think we can remove the ArmVExpressLibSec as well as drop Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.[dsc-fdf]


/
Leif









-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115378): https://edk2.groups.io/g/devel/message/115378
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[Yao, Jiewen]

Sure.

When we say "dependency", what we really mean is the dependency in INF file, not "dependency" in DSC file.

From package release perspective, only INF is the interface to other package. 
The DSC is only the package internal stuff, you can create multiple DSCs or add/remove DSC freely.

Having "dependency" in DSC does not matter.
Having dependency in INF is something we should care about.

Thank you
Yao, Jiewen


> -----Original Message-----
> From: Leif Lindholm <quic_llindhol@quicinc.com>
> Sent: Tuesday, February 13, 2024 1:38 AM
> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io; Yao,
> Jiewen <jiewen.yao@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Liming Gao
> <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Sami Mujawar <sami.mujawar@arm.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> Subject: Re: [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg
> 
> Jiewen, can you clarify what you said back in
> https://edk2.groups.io/g/devel/message/111551
> ?
> 
> On 2024-02-12 17:24, Pierre Gondois wrote:
> >>> A ArmLibNull.inf library might also need to be created. If the
> >>> OpensslLibFullAccel.inf module will depend on the ArmLib library,
> >>> a Null implementation will be necessary for non-Arm architectures.
> >>
> >> Can ArmLib be declared under a [LibraryClasses.AArch64,
> >> LibraryClasses.ARM]? Have I forgotten something that we discussed back
> >> in ... November?
> >
> >  From [1], it seems the MdePkg/CryptoPkg should build without a dependency
> > on the ArmPkg. This is currently not really the case. cf. [2].
> >
> > However, having a ArmLibNull implementation in the MdePkg would allow to
> > avoid going in this direction when providing libraries to CryptoPkg.dsc.
> >
> > (Just in case, I think this ArmLibNull is a minor point.)
> 
> Well, sure, it is now.
> Until we need a RiscV64LibNull, LoongarchLibNull, ...
> 
> > [1] https://edk2.groups.io/g/devel/message/111545
> > [2]
> >
> https://github.com/tianocore/edk2/blob/8801c75b4d77c2e6e06b3ddc8560e0db
> 590f6342/CryptoPkg/CryptoPkg.dsc#L117
> >
> >>
> >>> Otherwise I could apply and run the CryptoPkg/Arm native instructions
> >>> patchset on top of this patch.
> >>>
> >>> ---
> >>>
> >>> As a side note, it also seems that:
> >>> - ArmPkg/Include/Chipset/ArmCortexA5x.h
> >>>     isn't used anymore in edk2/edk2-platorms
> >>> - ArmPkg/Include/Chipset/ArmCortexA9.h
> >>>     is barely used in edk2-platforms.
> >>> Maybe the files should have been removed/simplified as part of
> >>> - cffa7925a293 ("ArmPkg: remove ArmCpuLib header and implementations")
> >>> - a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")
> >>
> >> I think you're right.
> >> Well, ArmCortexA9.h is still *used*, but I can't imagine the Arm branch
> >> of ArmVExpressLib has been build by anyone for some time. And surely the
> >> inclusion of ArmVExpressLibSec in ArmVExpress-FVP-AArch64.dsc is
> >> superfluous (such that that .inf can be deleted)?
> >
> > The file could just be moved in the Library. I assume you/Sami/Ard
> > will know more on the usage of the library itself,
> 
> Sami?
> 
> /
>      Leif
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116193): https://edk2.groups.io/g/devel/message/116193
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[Leif Lindholm]

Thank you.

OK, that's logically consistent.
So we'd need an ArmLibNull in MdePkg until ArmLib itself migrates there 
(ideally subsumed into BaseLib).

But the dependency in .inf should still be able to be declared under
[LibraryClasses.AArch64, LibraryClasses.ARM]?

Regards,

Leif

On 2024-03-01 01:00, Yao, Jiewen wrote:
> Sure.
> 
> When we say "dependency", what we really mean is the dependency in INF file, not "dependency" in DSC file.
> 
>  From package release perspective, only INF is the interface to other package.
> The DSC is only the package internal stuff, you can create multiple DSCs or add/remove DSC freely.
> 
> Having "dependency" in DSC does not matter.
> Having dependency in INF is something we should care about.
> 
> Thank you
> Yao, Jiewen
> 
> 
>> -----Original Message-----
>> From: Leif Lindholm <quic_llindhol@quicinc.com>
>> Sent: Tuesday, February 13, 2024 1:38 AM
>> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io; Yao,
>> Jiewen <jiewen.yao@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Liming Gao
>> <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>;
>> Sami Mujawar <sami.mujawar@arm.com>; Liu, Zhiguang
>> <zhiguang.liu@intel.com>
>> Subject: Re: [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg
>>
>> Jiewen, can you clarify what you said back in
>> https://edk2.groups.io/g/devel/message/111551
>> ?
>>
>> On 2024-02-12 17:24, Pierre Gondois wrote:
>>>>> A ArmLibNull.inf library might also need to be created. If the
>>>>> OpensslLibFullAccel.inf module will depend on the ArmLib library,
>>>>> a Null implementation will be necessary for non-Arm architectures.
>>>>
>>>> Can ArmLib be declared under a [LibraryClasses.AArch64,
>>>> LibraryClasses.ARM]? Have I forgotten something that we discussed back
>>>> in ... November?
>>>
>>>   From [1], it seems the MdePkg/CryptoPkg should build without a dependency
>>> on the ArmPkg. This is currently not really the case. cf. [2].
>>>
>>> However, having a ArmLibNull implementation in the MdePkg would allow to
>>> avoid going in this direction when providing libraries to CryptoPkg.dsc.
>>>
>>> (Just in case, I think this ArmLibNull is a minor point.)
>>
>> Well, sure, it is now.
>> Until we need a RiscV64LibNull, LoongarchLibNull, ...
>>
>>> [1] https://edk2.groups.io/g/devel/message/111545
>>> [2]
>>>
>> https://github.com/tianocore/edk2/blob/8801c75b4d77c2e6e06b3ddc8560e0db
>> 590f6342/CryptoPkg/CryptoPkg.dsc#L117
>>>
>>>>
>>>>> Otherwise I could apply and run the CryptoPkg/Arm native instructions
>>>>> patchset on top of this patch.
>>>>>
>>>>> ---
>>>>>
>>>>> As a side note, it also seems that:
>>>>> - ArmPkg/Include/Chipset/ArmCortexA5x.h
>>>>>      isn't used anymore in edk2/edk2-platorms
>>>>> - ArmPkg/Include/Chipset/ArmCortexA9.h
>>>>>      is barely used in edk2-platforms.
>>>>> Maybe the files should have been removed/simplified as part of
>>>>> - cffa7925a293 ("ArmPkg: remove ArmCpuLib header and implementations")
>>>>> - a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")
>>>>
>>>> I think you're right.
>>>> Well, ArmCortexA9.h is still *used*, but I can't imagine the Arm branch
>>>> of ArmVExpressLib has been build by anyone for some time. And surely the
>>>> inclusion of ArmVExpressLibSec in ArmVExpress-FVP-AArch64.dsc is
>>>> superfluous (such that that .inf can be deleted)?
>>>
>>> The file could just be moved in the Library. I assume you/Sami/Ard
>>> will know more on the usage of the library itself,
>>
>> Sami?
>>
>> /
>>       Leif
>>
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116235): https://edk2.groups.io/g/devel/message/116235
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[Yao, Jiewen]

Right, if it is only required by ARM, then it should under ARM section.

Thank you
Yao, Jiewen

> -----Original Message-----
> From: Leif Lindholm <quic_llindhol@quicinc.com>
> Sent: Friday, March 1, 2024 7:45 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Pierre Gondois
> <pierre.gondois@arm.com>; devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Liming Gao
> <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Sami Mujawar <sami.mujawar@arm.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> Subject: Re: [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg
> 
> Thank you.
> 
> OK, that's logically consistent.
> So we'd need an ArmLibNull in MdePkg until ArmLib itself migrates there
> (ideally subsumed into BaseLib).
> 
> But the dependency in .inf should still be able to be declared under
> [LibraryClasses.AArch64, LibraryClasses.ARM]?
> 
> Regards,
> 
> Leif
> 
> On 2024-03-01 01:00, Yao, Jiewen wrote:
> > Sure.
> >
> > When we say "dependency", what we really mean is the dependency in INF file,
> not "dependency" in DSC file.
> >
> >  From package release perspective, only INF is the interface to other package.
> > The DSC is only the package internal stuff, you can create multiple DSCs or
> add/remove DSC freely.
> >
> > Having "dependency" in DSC does not matter.
> > Having dependency in INF is something we should care about.
> >
> > Thank you
> > Yao, Jiewen
> >
> >
> >> -----Original Message-----
> >> From: Leif Lindholm <quic_llindhol@quicinc.com>
> >> Sent: Tuesday, February 13, 2024 1:38 AM
> >> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io; Yao,
> >> Jiewen <jiewen.yao@intel.com>
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Liming Gao
> >> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>;
> >> Sami Mujawar <sami.mujawar@arm.com>; Liu, Zhiguang
> >> <zhiguang.liu@intel.com>
> >> Subject: Re: [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg
> >>
> >> Jiewen, can you clarify what you said back in
> >> https://edk2.groups.io/g/devel/message/111551
> >> ?
> >>
> >> On 2024-02-12 17:24, Pierre Gondois wrote:
> >>>>> A ArmLibNull.inf library might also need to be created. If the
> >>>>> OpensslLibFullAccel.inf module will depend on the ArmLib library,
> >>>>> a Null implementation will be necessary for non-Arm architectures.
> >>>>
> >>>> Can ArmLib be declared under a [LibraryClasses.AArch64,
> >>>> LibraryClasses.ARM]? Have I forgotten something that we discussed back
> >>>> in ... November?
> >>>
> >>>   From [1], it seems the MdePkg/CryptoPkg should build without a
> dependency
> >>> on the ArmPkg. This is currently not really the case. cf. [2].
> >>>
> >>> However, having a ArmLibNull implementation in the MdePkg would allow to
> >>> avoid going in this direction when providing libraries to CryptoPkg.dsc.
> >>>
> >>> (Just in case, I think this ArmLibNull is a minor point.)
> >>
> >> Well, sure, it is now.
> >> Until we need a RiscV64LibNull, LoongarchLibNull, ...
> >>
> >>> [1] https://edk2.groups.io/g/devel/message/111545
> >>> [2]
> >>>
> >>
> https://github.com/tianocore/edk2/blob/8801c75b4d77c2e6e06b3ddc8560e0db
> >> 590f6342/CryptoPkg/CryptoPkg.dsc#L117
> >>>
> >>>>
> >>>>> Otherwise I could apply and run the CryptoPkg/Arm native instructions
> >>>>> patchset on top of this patch.
> >>>>>
> >>>>> ---
> >>>>>
> >>>>> As a side note, it also seems that:
> >>>>> - ArmPkg/Include/Chipset/ArmCortexA5x.h
> >>>>>      isn't used anymore in edk2/edk2-platorms
> >>>>> - ArmPkg/Include/Chipset/ArmCortexA9.h
> >>>>>      is barely used in edk2-platforms.
> >>>>> Maybe the files should have been removed/simplified as part of
> >>>>> - cffa7925a293 ("ArmPkg: remove ArmCpuLib header and
> implementations")
> >>>>> - a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")
> >>>>
> >>>> I think you're right.
> >>>> Well, ArmCortexA9.h is still *used*, but I can't imagine the Arm branch
> >>>> of ArmVExpressLib has been build by anyone for some time. And surely the
> >>>> inclusion of ArmVExpressLibSec in ArmVExpress-FVP-AArch64.dsc is
> >>>> superfluous (such that that .inf can be deleted)?
> >>>
> >>> The file could just be moved in the Library. I assume you/Sami/Ard
> >>> will know more on the usage of the library itself,
> >>
> >> Sami?
> >>
> >> /
> >>       Leif
> >>
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116261): https://edk2.groups.io/g/devel/message/116261
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[PierreGondois]

On 3/1/24 12:45, Leif Lindholm wrote:
> Thank you.
> 
> OK, that's logically consistent.
> So we'd need an ArmLibNull in MdePkg until ArmLib itself migrates there
> (ideally subsumed into BaseLib).

 From what Jiewen said, it doesn't seem that creating an ArmLibNull
in the MdePkg is necessary (unless I misunderstood).

I will send a follow-up patch to the RFC tomorrow. Along the RFC patch,
it should be sufficient to move ArmLib to the MdePkg,

Regards,
Pierre

> 
> But the dependency in .inf should still be able to be declared under
> [LibraryClasses.AArch64, LibraryClasses.ARM]?
> 
> Regards,
> 
> Leif
> 
> On 2024-03-01 01:00, Yao, Jiewen wrote:
>> Sure.
>>
>> When we say "dependency", what we really mean is the dependency in INF file, not "dependency" in DSC file.
>>
>>   From package release perspective, only INF is the interface to other package.
>> The DSC is only the package internal stuff, you can create multiple DSCs or add/remove DSC freely.
>>
>> Having "dependency" in DSC does not matter.
>> Having dependency in INF is something we should care about.
>>
>> Thank you
>> Yao, Jiewen
>>
>>
>>> -----Original Message-----
>>> From: Leif Lindholm <quic_llindhol@quicinc.com>
>>> Sent: Tuesday, February 13, 2024 1:38 AM
>>> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io; Yao,
>>> Jiewen <jiewen.yao@intel.com>
>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Liming Gao
>>> <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>;
>>> Sami Mujawar <sami.mujawar@arm.com>; Liu, Zhiguang
>>> <zhiguang.liu@intel.com>
>>> Subject: Re: [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg
>>>
>>> Jiewen, can you clarify what you said back in
>>> https://edk2.groups.io/g/devel/message/111551
>>> ?
>>>
>>> On 2024-02-12 17:24, Pierre Gondois wrote:
>>>>>> A ArmLibNull.inf library might also need to be created. If the
>>>>>> OpensslLibFullAccel.inf module will depend on the ArmLib library,
>>>>>> a Null implementation will be necessary for non-Arm architectures.
>>>>>
>>>>> Can ArmLib be declared under a [LibraryClasses.AArch64,
>>>>> LibraryClasses.ARM]? Have I forgotten something that we discussed back
>>>>> in ... November?
>>>>
>>>>    From [1], it seems the MdePkg/CryptoPkg should build without a dependency
>>>> on the ArmPkg. This is currently not really the case. cf. [2].
>>>>
>>>> However, having a ArmLibNull implementation in the MdePkg would allow to
>>>> avoid going in this direction when providing libraries to CryptoPkg.dsc.
>>>>
>>>> (Just in case, I think this ArmLibNull is a minor point.)
>>>
>>> Well, sure, it is now.
>>> Until we need a RiscV64LibNull, LoongarchLibNull, ...
>>>
>>>> [1] https://edk2.groups.io/g/devel/message/111545
>>>> [2]
>>>>
>>> https://github.com/tianocore/edk2/blob/8801c75b4d77c2e6e06b3ddc8560e0db
>>> 590f6342/CryptoPkg/CryptoPkg.dsc#L117
>>>>
>>>>>
>>>>>> Otherwise I could apply and run the CryptoPkg/Arm native instructions
>>>>>> patchset on top of this patch.
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> As a side note, it also seems that:
>>>>>> - ArmPkg/Include/Chipset/ArmCortexA5x.h
>>>>>>       isn't used anymore in edk2/edk2-platorms
>>>>>> - ArmPkg/Include/Chipset/ArmCortexA9.h
>>>>>>       is barely used in edk2-platforms.
>>>>>> Maybe the files should have been removed/simplified as part of
>>>>>> - cffa7925a293 ("ArmPkg: remove ArmCpuLib header and implementations")
>>>>>> - a913ad02479d ("ArmPlatformPkg: remove ArmVExpressPkg")
>>>>>
>>>>> I think you're right.
>>>>> Well, ArmCortexA9.h is still *used*, but I can't imagine the Arm branch
>>>>> of ArmVExpressLib has been build by anyone for some time. And surely the
>>>>> inclusion of ArmVExpressLibSec in ArmVExpress-FVP-AArch64.dsc is
>>>>> superfluous (such that that .inf can be deleted)?
>>>>
>>>> The file could just be moved in the Library. I assume you/Sami/Ard
>>>> will know more on the usage of the library itself,
>>>
>>> Sami?
>>>
>>> /
>>>        Leif
>>>
>>
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116649): https://edk2.groups.io/g/devel/message/116649
Mute This Topic: https://groups.io/mt/102731845/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-