BaseRngLib: Rename ArmReadIdIsar0() to ArmGetFeatRng()

Leif Lindholm Thu, 29 Sep 2022 10:09:36 -0700

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.gond...@arm.com wrote:

From: Pierre Gondois <pierre.gond...@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 thePcdCpuRngSupportedAlgorithm,

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 otherpackages,if I actually needed to add ArmReadIdIsar0() to library should it belongfor 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 aswell. 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.gond...@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.Sb/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.asmb/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.hb/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.cb/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 ISAR0register returned by // MSR. A non-zero value indicates that the processor supportsthe RNDR instruction.

     //
-  Isar0 = ArmReadIdIsar0 ();
+  Isar0 = ArmGetFeatRng ();
     ASSERT ((Isar0 & RNDR_MASK) != 0);
     mRndrSupported = ((Isar0 & RNDR_MASK) != 0);

diff --git a/MdePkg/Library/BaseRngLib/BaseRngLib.infb/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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 09/21] MdePkg/BaseRngLib: Rename ArmReadIdIsar0() to ArmGetFeatRng()

Reply via email to