[edk2-devel] [PATCH v5 09/21] MdePkg/BaseRngLib: Rename ArmReadIdIsar0() to ArmGetFeatRng()
Leif Lindholm
quic_llindhol at quicinc.com
Thu Sep 29 17:09:12 UTC 2022
On 2022-09-29 09:21, Pierre Gondois wrote:
>
>
> On 9/28/22 19:10, Leif Lindholm wrote:
>> On 2022-09-19 12:21, Pierre.Gondois at arm.com wrote:
>>> From: Pierre Gondois <pierre.gondois at arm.com>
>>>
>>> The MdePkg must be self contained and not have external dependencies.
>>> ArmReadIdIsar0() is defined in MdePkg/Library/BaseRngLib and is
>>> limited to the scope of this library.
>> >
>>> The same function will be required to check the FEAT_AES and FEAT_RNG
>>> extensions in other libraries. As this function is Arm specific, it
>>> cannot be added to a library interface in MdePkg. It should be part of
>>> ArmPkg/ArmLib.
>>
>> Ultimately, ArmPkg has no justification for existing. It is a legacy of
>> ARM support being added as a prototype, bolted onto the side of the rest
>> of EDK2. Over time, it needs to be integrated into MdePkg, UefiCpuPkg,
>> and possibly MdeModulePkg.
>>
>> It makes sense to break the ID_ISAR0 read out into a separate library,
>> but we're not making the world a better place if we then hide random
>> direct-asm-accesses in various libraries. There are similar things in
>> other architectures - i.e. CPUID.
>>
>> But I don't like this patch. I'd prefer to keep this code as-is until
>> the separate library has been added - in MdePkg - and we can switch
>> straight to that.
>
> Ok, it is also possible to use RngGetBytes() to enable the
> PcdCpuRngSupportedAlgorithm,
> so I'll do that and drop:
> - ArmPkg/ArmLib: Add ArmHasRngExt()
> - ArmPkg/ArmLib: Add ArmReadIdIsar0() helper
That works for me - thanks!
> Just to understand how the ArmPkg should ideally be melded in other
> packages,
> if I actually needed to add ArmReadIdIsar0() to library should it belong
> for instance
> to:
> MdePkg/Include/Library/BaseLib.h
> as some of the functions there allow to read intel extensions ?
I think it would make sense to create a separate ArchitectureFeatureLib
(or somesuch) and move the x86 extension detection bits into that as
well. But then I'm not an MdePkg maintainer. :)
Regards,
Leif
> Regards,
> Pierre
>
>>
>> /
>> Leif
>>
>>> To avoid having mutiple definitions/prototypes of ArmReadIdIsar0(),
>>> and as BaseRngLib only requires to check the RNG capability bits,
>>> rename the MdePkg/Library/BaseRngLib implementation to ArmGetFeatRng().
>>>
>>> Signed-off-by: Pierre Gondois <Pierre.Gondois at arm.com>
>>> ---
>>> .../AArch64/{ArmReadIdIsar0.S => ArmGetFeatRng.S} | 8
>>> ++++----
>>> .../AArch64/{ArmReadIdIsar0.asm => ArmGetFeatRng.asm} | 8
>>> ++++----
>>> MdePkg/Library/BaseRngLib/AArch64/ArmRng.h | 2 +-
>>> MdePkg/Library/BaseRngLib/AArch64/Rndr.c | 2 +-
>>> MdePkg/Library/BaseRngLib/BaseRngLib.inf | 4 ++--
>>> 5 files changed, 12 insertions(+), 12 deletions(-)
>>> rename MdePkg/Library/BaseRngLib/AArch64/{ArmReadIdIsar0.S =>
>>> ArmGetFeatRng.S} (78%)
>>> rename MdePkg/Library/BaseRngLib/AArch64/{ArmReadIdIsar0.asm =>
>>> ArmGetFeatRng.asm} (81%)
>>>
>>> diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S
>>> b/MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.S
>>> similarity index 78%
>>> rename from MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S
>>> rename to MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.S
>>> index 82a00d362212..c42d60513077 100644
>>> --- a/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S
>>> +++ b/MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.S
>>> @@ -1,6 +1,6 @@
>>>
>>> #------------------------------------------------------------------------------
>>>
>>> #
>>> -# ArmReadIdIsar0() for AArch64
>>> +# ArmGetFeatRng() for AArch64
>>> #
>>> # Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
>>> #
>>> @@ -10,7 +10,7 @@
>>> .text
>>> .p2align 2
>>> -GCC_ASM_EXPORT(ArmReadIdIsar0)
>>> +GCC_ASM_EXPORT(ArmGetFeatRng)
>>> #/**
>>> # Reads the ID_AA64ISAR0 Register.
>>> @@ -20,11 +20,11 @@ GCC_ASM_EXPORT(ArmReadIdIsar0)
>>> #**/
>>> #UINT64
>>> #EFIAPI
>>> -#ArmReadIdIsar0 (
>>> +#ArmGetFeatRng (
>>> # VOID
>>> # );
>>> #
>>> -ASM_PFX(ArmReadIdIsar0):
>>> +ASM_PFX(ArmGetFeatRng):
>>> mrs x0, id_aa64isar0_el1 // Read ID_AA64ISAR0 Register
>>> ret
>>> diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm
>>> b/MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.asm
>>> similarity index 81%
>>> rename from MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm
>>> rename to MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.asm
>>> index 1d9f9a808c0c..947adfcd2749 100644
>>> --- a/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm
>>> +++ b/MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.asm
>>> @@ -1,6 +1,6 @@
>>>
>>> ;------------------------------------------------------------------------------
>>>
>>> ;
>>> -; ArmReadIdIsar0() for AArch64
>>> +; ArmGetFeatRng() for AArch64
>>> ;
>>> ; Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
>>> ;
>>> @@ -8,7 +8,7 @@
>>> ;
>>>
>>> ;------------------------------------------------------------------------------
>>>
>>> - EXPORT ArmReadIdIsar0
>>> + EXPORT ArmGetFeatRng
>>> AREA BaseLib_LowLevel, CODE, READONLY
>>> ;/**
>>> @@ -19,11 +19,11 @@
>>> ;**/
>>> ;UINT64
>>> ;EFIAPI
>>> -;ArmReadIdIsar0 (
>>> +;ArmGetFeatRng (
>>> ; VOID
>>> ; );
>>> ;
>>> -ArmReadIdIsar0
>>> +ArmGetFeatRng
>>> mrs x0, id_aa64isar0_el1 // Read ID_AA64ISAR0 Register
>>> ret
>>> diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h
>>> b/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h
>>> index 2d6ef48ab941..b35cba3c063a 100644
>>> --- a/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h
>>> +++ b/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h
>>> @@ -35,7 +35,7 @@ ArmRndr (
>>> **/
>>> UINT64
>>> EFIAPI
>>> -ArmReadIdIsar0 (
>>> +ArmGetFeatRng (
>>> VOID
>>> );
>>> diff --git a/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
>>> b/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
>>> index 20811bf3ebf3..0cfdf4c37149 100644
>>> --- a/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
>>> +++ b/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
>>> @@ -47,7 +47,7 @@ BaseRngLibConstructor (
>>> // Determine RNDR support by examining bits 63:60 of the ISAR0
>>> register returned by
>>> // MSR. A non-zero value indicates that the processor supports
>>> the RNDR instruction.
>>> //
>>> - Isar0 = ArmReadIdIsar0 ();
>>> + Isar0 = ArmGetFeatRng ();
>>> ASSERT ((Isar0 & RNDR_MASK) != 0);
>>> mRndrSupported = ((Isar0 & RNDR_MASK) != 0);
>>> diff --git a/MdePkg/Library/BaseRngLib/BaseRngLib.inf
>>> b/MdePkg/Library/BaseRngLib/BaseRngLib.inf
>>> index 1fcceb941495..d6eccb07d469 100644
>>> --- a/MdePkg/Library/BaseRngLib/BaseRngLib.inf
>>> +++ b/MdePkg/Library/BaseRngLib/BaseRngLib.inf
>>> @@ -37,10 +37,10 @@ [Sources.AARCH64]
>>> AArch64/Rndr.c
>>> AArch64/ArmRng.h
>>> - AArch64/ArmReadIdIsar0.S | GCC
>>> + AArch64/ArmGetFeatRng.S | GCC
>>> AArch64/ArmRng.S | GCC
>>> - AArch64/ArmReadIdIsar0.asm | MSFT
>>> + AArch64/ArmGetFeatRng.asm | MSFT
>>> AArch64/ArmRng.asm | MSFT
>>> [Packages]
>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94526): https://edk2.groups.io/g/devel/message/94526
Mute This Topic: https://groups.io/mt/93788880/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list