Re: [PATCH 1/3] MdePkg/BaseLib: Add support for ARMv8.5 RNG instructions


Rebecca Cran
 

On 5/4/21 3:06 PM, Sami Mujawar wrote:

+#if defined (MDE_CPU_AARCH64)
+
+/**
+  Reads the ID_AA64ISAR0 Register.
+
+  @return The contents of the ID_AA64ISAR0 Register
+
+**/
+UINT64
+EFIAPI
+ArmReadIdIsar0 (
+  VOID
+  );
+
[SAMI] Should this function be part of ArmLib?
[/SAMI]
It's currently used in BaseRngLib, and since there are other Aarch64 functions in BaseLib I thought it should belong here - especially since I don't think MdePkg can depend on ArmPkg?

diff --git a/MdePkg/Library/BaseLib/BaseLibInternals.h b/MdePkg/Library/BaseLib/BaseLibInternals.h
index 6837d67d90cf..4ae79a4e7ab4 100644
--- a/MdePkg/Library/BaseLib/BaseLibInternals.h
+++ b/MdePkg/Library/BaseLib/BaseLibInternals.h
@@ -862,6 +862,12 @@ InternalX86RdRand64  (
    OUT     UINT64                    *Rand
    );
[SAMI] I can see that the X86RdRand64 functions are implemented in this library. However, I am not sure we want ArmRndr() and ArmRndrrs() in BaseLib.
I think these functions should be in BaseRngLib and should not be available publicly. The RngLib interface should be used by edk2 modules. What do you think?
[/SAMI]
That makes sense - I'll move them.

--
Rebecca Cran

Join devel@edk2.groups.io to automatically receive all group messages.