public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface
@ 2021-11-16 11:32 Sami Mujawar
  2021-11-16 11:32 ` [PATCH v2 1/8] MdePkg: Definition for TRNG library class interface Sami Mujawar
                   ` (7 more replies)
  0 siblings, 8 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
	michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
	jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd

Bugzilla: Bug 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

The Arm True Random Number Generator Firmware, Interface 1.0, specification
defines an interface between an Operating System (OS) executing at EL1 and
Firmware (FW) exposing a conditioned entropy source that is provided by a
TRNG back end.

This v2 patch series updates the following based on the feedback received
for the v1 series at https://edk2.groups.io/g/devel/message/81653:
 - Updates TrngLib definitions to use RETURN_STATUS as the return type
   from the interface functions as TrngLib is base type library.
 - Drops the patch "MdePkg: Add definition for NULL GUID" as there is
   already an equivalent definition provided by gZeroGuid. Thus, the
   use of gNullGuid has been replaced with gZeroGuid.

The V1 patch series:
 - defines a TRNG library class that provides an interface to access the
   entropy source on a platform.
 - implements a TRNG library instance that uses the Arm FW-TRNG interface.
 - Adds RawAlgorithm support to RngDxe for Arm architecture using the Arm
   FW-TRNG interface.
 - Enables RNG support using FW-TRNG interface for Kvmtool Guest/Virtual
   firmware.

The changes can be seen at:
https://github.com/samimujawar/edk2/tree/1829_arm_fw_trng_v2

Sami Mujawar (8):
  MdePkg: Definition for TRNG library class interface
  ArmPkg: PCD to select conduit for monitor calls
  ArmPkg: Add Arm Firmware TRNG library
  MdePkg: Add NULL instance of TRNG Library
  SecurityPkg: Rename RdRandGenerateEntropy to common name
  SecurityPkg: Restructure checks in RngGetInfo
  SecurityPkg: Add RawAlgorithm support using TRNG library
  ArmVirtPkg: Kvmtool: Add RNG support using FW-TRNG interface

 ArmPkg/ArmPkg.dec                                          |   5 +
 ArmPkg/ArmPkg.dsc                                          |   1 +
 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h                |  64 +++
 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c                 | 483 ++++++++++++++++++++
 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf               |  34 ++
 ArmVirtPkg/ArmVirtKvmTool.dsc                              |  10 +
 ArmVirtPkg/ArmVirtKvmTool.fdf                              |   5 +
 MdePkg/Include/Library/TrngLib.h                           | 121 +++++
 MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c           | 111 +++++
 MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf         |  30 ++
 MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni         |  12 +
 MdePkg/MdePkg.dec                                          |   7 +-
 MdePkg/MdePkg.dsc                                          |   1 +
 SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c  |  79 +++-
 SecurityPkg/RandomNumberGenerator/RngDxe/Arm/RngDxe.c      | 163 +++++++
 SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c         |  61 +++
 SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c     |  13 +-
 SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h     |  43 --
 SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c     |  12 +-
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c          |  13 +-
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf        |  14 +-
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h |  22 +-
 SecurityPkg/SecurityPkg.dsc                                |   8 +-
 23 files changed, 1239 insertions(+), 73 deletions(-)
 create mode 100644 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
 create mode 100644 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
 create mode 100644 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
 create mode 100644 MdePkg/Include/Library/TrngLib.h
 create mode 100644 MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c
 create mode 100644 MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf
 create mode 100644 MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni
 create mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/Arm/RngDxe.c
 create mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c
 delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h

-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/8] MdePkg: Definition for TRNG library class interface
  2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
  2021-11-16 11:32 ` [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls Sami Mujawar
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
	michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
	jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd

Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

The NIST Special Publications 800-90A, 800-90B and 800-90C
provide recommendations for random number generation. The
NIST 800-90C, Recommendation for Random Bit Generator (RBG)
Constructions, defines the GetEntropy() interface that is
used to access the entropy source. The GetEntropy() interface
is further used by Deterministic Random Bit Generators (DRBG)
to generate random numbers.

The True Random Number Generator (TRNG) library defines an
interface to access the entropy source on a platform. Some
platforms/architectures may provide access to the entropy
using a firmware interface. In such cases the TRNG library
shall be used to provide an abstraction.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
     - MdePkg\Include\Library\TrngLib.h is base type     [LIMING]
       library. It can use RETURN_STATUS instead of
       EFI_STATUS. Then, it doesn't need to include
       <Uefi/UefiBaseType.h>
     - Replaced EFI_STATUS with RETURN_STATUS and        [SAMI]
       removed include of /UefiBaseType.h.
     - MdePkg\Include\Library\TrngLib.h API parameter    [LIMING]
       doesn't require CONST. CONST means the value
       specified by the input pointer will not be
       changed in API implementation.
     - Removed the use of constant pointers in the       [SAMI]
       TRNG API.

 MdePkg/Include/Library/TrngLib.h | 121 ++++++++++++++++++++
 MdePkg/MdePkg.dec                |   7 +-
 2 files changed, 127 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/TrngLib.h b/MdePkg/Include/Library/TrngLib.h
new file mode 100644
index 0000000000000000000000000000000000000000..082c83d0a6c67aa88c789a35b8f2f73ba7cf46cd
--- /dev/null
+++ b/MdePkg/Include/Library/TrngLib.h
@@ -0,0 +1,121 @@
+/** @file
+  TRNG interface library definitions.
+
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+  - [1] Arm True Random Number Generator Firmware, Interface 1.0,
+        Platform Design Document.
+        (https://developer.arm.com/documentation/den0098/latest/)
+  - [2] NIST Special Publication 800-90A Revision 1, June 2015, Recommendation
+        for Random Number Generation Using Deterministic Random Bit Generators.
+        (https://csrc.nist.gov/publications/detail/sp/800-90a/rev-1/final)
+  - [3] NIST Special Publication 800-90B, Recommendation for the Entropy
+        Sources Used for Random Bit Generation.
+        (https://csrc.nist.gov/publications/detail/sp/800-90b/final)
+  - [4] (Second Draft) NIST Special Publication 800-90C, Recommendation for
+        Random Bit Generator (RBG) Constructions.
+        (https://csrc.nist.gov/publications/detail/sp/800-90c/draft)
+
+  @par Glossary:
+    - TRNG - True Random Number Generator
+**/
+#ifndef TRNG_LIB_H_
+#define TRNG_LIB_H_
+
+/** Get the version of the TRNG backend.
+
+  A TRNG may be implemented by the system firmware, in which case this
+  function shall return the version of the TRNG backend.
+  The implementation must return NOT_SUPPORTED if a Back end is not present.
+
+  @param [out]  MajorRevision     Major revision.
+  @param [out]  MinorRevision     Minor revision.
+
+  @retval  RETURN_SUCCESS            The function completed successfully.
+  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
+  @retval  RETURN_UNSUPPORTED        Backend not present.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngVersion (
+  OUT UINT16  *MajorRevision,
+  OUT UINT16  *MinorRevision
+  );
+
+/** Get the UUID of the TRNG backend.
+
+  A TRNG may be implemented by the system firmware, in which case this
+  function shall return the UUID of the TRNG backend.
+  Returning the TRNG UUID is optional and if not implemented, RETURN_UNSUPPORTED
+  shall be returned.
+
+  Note: The caller must not rely on the returned UUID as a trustworthy TRNG
+        Back end identity
+
+  @param [out]  Guid              UUID of the TRNG backend.
+
+  @retval  RETURN_SUCCESS            The function completed successfully.
+  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
+  @retval  RETURN_UNSUPPORTED        Function not implemented.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngUuid (
+  OUT GUID  *Guid
+  );
+
+/** Returns maximum number of entropy bits that can be returned in a single
+    call.
+
+  @return Returns the maximum number of Entropy bits that can be returned
+          in a single call to GetEntropy().
+          If this feature is not supported MAX_UINTN is returned.
+**/
+UINTN
+EFIAPI
+GetTrngMaxSupportedEntropyBits (
+  VOID
+  );
+
+/** Returns N bits of conditioned entropy.
+
+  See [3] Section 2.3.1 GetEntropy: An Interface to the Entropy Source
+    GetEntropy
+      Input:
+        bits_of_entropy: the requested amount of entropy
+      Output:
+        entropy_bitstring: The string that provides the requested entropy.
+      status: A Boolean value that is TRUE if the request has been satisfied,
+              and is FALSE otherwise.
+
+  Note: In this implementation this function returns a status code instead
+        of a boolean value.
+        This is also compatible with the definition of Get_Entropy, see [4]
+        Section 7.4 Entropy Source Calls.
+          (status, entropy_bitstring) = Get_Entropy (
+                                          requested_entropy,
+                                          max_length
+                                          )
+
+  @param  [in]   EntropyBits  Number of entropy bits requested.
+  @param  [out]  Buffer       Buffer to return the entropy bits.
+  @param  [in]   Buffersize   Size of the Buffer in bytes.
+
+  @retval  RETURN_SUCCESS            The function completed successfully.
+  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
+  @retval  RETURN_UNSUPPORTED        Function not implemented.
+  @retval  RETURN_BAD_BUFFER_SIZE    Buffer size is too small.
+  @retval  RETURN_NOT_READY          No Entropy available.
+**/
+RETURN_STATUS
+EFIAPI
+GetEntropy (
+  IN  CONST UINTN       EntropyBits,
+  OUT       UINT8       *Buffer,
+  IN  CONST UINTN       BufferSize
+  );
+
+#endif // TRNG_LIB_H_
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 8b18415b107a03b11dc984341bb788cf9cd6e0ea..e612b5c57429d1af591de8e111f328e19a030ca0 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -7,6 +7,7 @@
 # Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
 # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
 # (C) Copyright 2016 - 2021 Hewlett Packard Enterprise Development LP<BR>
+#  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -267,11 +268,15 @@ [LibraryClasses]
   #
   RegisterFilterLib|Include/Library/RegisterFilterLib.h
 
-[LibraryClasses.IA32, LibraryClasses.X64, LibraryClasses.AARCH64]
+[LibraryClasses.IA32, LibraryClasses.X64, LibraryClasses.AARCH64, LibraryClasses.ARM]
   ##  @libraryclass  Provides services to generate random number.
   #
   RngLib|Include/Library/RngLib.h
 
+  ##  @libraryclass  Provides services to generate Entropy using a TRNG.
+  #
+  TrngLib|Include/Library/TrngLib.h
+
 [LibraryClasses.IA32, LibraryClasses.X64]
   ##  @libraryclass  Abstracts both S/W SMI generation and detection.
   ##
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
  2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
  2021-11-16 11:32 ` [PATCH v2 1/8] MdePkg: Definition for TRNG library class interface Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
  2021-11-24 12:07   ` Leif Lindholm
  2021-11-16 11:32 ` [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library Sami Mujawar
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
	michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
	jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd

Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
meaning the SMC conduit is enabled as default.

Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
by virtual firmware implementations.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
     - No code change since v1. Re-sending with V2 series.   [SAMI]

 ArmPkg/ArmPkg.dec | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
   # Define if the GICv3 controller should use the GICv2 legacy
   gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
 
+  ## Define the conduit to use for monitor calls.
+  # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
+  # If PcdMonitorConduitHvc = TRUE, conduit = HVC
+  gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
+
 [PcdsFeatureFlag.ARM]
   # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
   # TRUE may be appropriate to fix performance problems if you don't care about
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
  2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
  2021-11-16 11:32 ` [PATCH v2 1/8] MdePkg: Definition for TRNG library class interface Sami Mujawar
  2021-11-16 11:32 ` [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
  2021-11-24 13:01   ` [edk2-devel] " Leif Lindholm
  2021-11-16 11:32 ` [PATCH v2 4/8] MdePkg: Add NULL instance of TRNG Library Sami Mujawar
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
	michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
	jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd

Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

The Arm True Random Number Generator Firmware, Interface 1.0,
Platform Design Document
(https://developer.arm.com/documentation/den0098/latest/)
defines an interface between an Operating System (OS) executing
at EL1 and Firmware (FW) exposing a conditioned entropy source
that is provided by a TRNG back end.

The conditioned entropy, that is provided by the TRNG FW interface,
is commonly used to seed deterministic random number generators.

This patch adds a TrngLib library that implements the Arm TRNG
firmware interface.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
     - MdePkg\Include\Library\TrngLib.h is base type     [LIMING]
       library. It can use RETURN_STATUS instead of
       EFI_STATUS.
     - Replaced EFI_STATUS with RETURN_STATUS.           [SAMI]
     - MdePkg\Include\Library\TrngLib.h API parameter    [LIMING]
       doesn't require CONST. CONST means the value
       specified by the input pointer will not be
       changed in API implementation.
     - Removed the use of constant pointers in the       [SAMI]
       TRNG API.

 ArmPkg/ArmPkg.dsc                            |   1 +
 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h  |  64 +++
 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c   | 483 ++++++++++++++++++++
 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf |  34 ++
 4 files changed, 582 insertions(+)

diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 59fd8f295d4f614cc68ee1021e691f94e279ab81..23df68c5eb53df11de5d96bde4949f3c833c9b2c 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -156,6 +156,7 @@ [Components.common]
   ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
   ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
 
+  ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
   ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
   ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
   ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
new file mode 100644
index 0000000000000000000000000000000000000000..42236e743d972df0df205b1565496afeff5785f3
--- /dev/null
+++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
@@ -0,0 +1,64 @@
+/** @file
+  Arm Firmware TRNG definitions.
+
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+  - [1] Arm True Random Number Generator Firmware, Interface 1.0,
+        Platform Design Document.
+        (https://developer.arm.com/documentation/den0098/latest/)
+
+  @par Glossary:
+    - TRNG - True Random Number Generator
+    - FID  - Function ID
+**/
+
+#ifndef ARM_FW_TRNG_DEFS_H_
+#define ARM_FW_TRNG_DEFS_H_
+
+// Firmware TRNG interface Function IDs
+#define FID_TRNG_VERSION      0x84000050
+#define FID_TRNG_FEATURES     0x84000051
+#define FID_TRNG_GET_UUID     0x84000052
+#define FID_TRNG_RND_AARCH32  0x84000053
+#define FID_TRNG_RND_AARCH64  0xC4000053
+
+// Firmware TRNG revision mask and shift
+#define TRNG_REV_MAJOR_MASK   0x7FFF
+#define TRNG_REV_MINOR_MASK   0xFFFF
+#define TRNG_REV_MAJOR_SHIFT  16
+#define TRNG_REV_MINOR_SHIFT  0
+
+// Firmware TRNG status codes
+#define TRNG_STATUS_SUCCESS     (INT32)(0)
+#define TRNG_NOT_SUPPORTED      (INT32)(-1)
+#define TRNG_INVALID_PARAMETER  (INT32)(-2)
+#define TRNG_NO_ENTROPY         (INT32)(-3)
+
+#if defined (MDE_CPU_ARM)
+/** FID to use on AArch32 platform to request entropy.
+*/
+#define FID_TRNG_RND        FID_TRNG_RND_AARCH32
+
+/** Maximum bits of entropy supported on AArch32.
+*/
+#define MAX_ENTROPY_BITS    96
+#elif defined (MDE_CPU_AARCH64)
+/** FID to use on AArch64 platform to request entropy.
+*/
+#define FID_TRNG_RND        FID_TRNG_RND_AARCH64
+
+/** Maximum bits of entropy supported on AArch64.
+*/
+#define MAX_ENTROPY_BITS    192
+#else
+#error "Firmware TRNG not supported. Unknown chipset."
+#endif
+
+/** Typedef for SMC or HVC arguments.
+*/
+typedef ARM_SMC_ARGS  ARM_MONITOR_ARGS;
+
+#endif // ARM_FW_TRNG_DEFS_H_
diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..314e7ffbc232ae90bbb77306f9c7113ce63012c8
--- /dev/null
+++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
@@ -0,0 +1,483 @@
+/** @file
+  Arm Firmware TRNG interface library.
+
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+  - [1] Arm True Random Number Generator Firmware, Interface 1.0,
+        Platform Design Document.
+        (https://developer.arm.com/documentation/den0098/latest/)
+  - [2] NIST Special Publication 800-90A Revision 1, June 2015, Recommendation
+        for Random Number Generation Using Deterministic Random Bit Generators.
+        (https://csrc.nist.gov/publications/detail/sp/800-90a/rev-1/final)
+  - [3] NIST Special Publication 800-90B, Recommendation for the Entropy
+        Sources Used for Random Bit Generation.
+        (https://csrc.nist.gov/publications/detail/sp/800-90b/final)
+  - [4] (Second Draft) NIST Special Publication 800-90C, Recommendation for
+        Random Bit Generator (RBG) Constructions.
+        (https://csrc.nist.gov/publications/detail/sp/800-90c/draft)
+
+  @par Glossary:
+    - TRNG - True Random Number Generator
+    - FID  - Function ID
+**/
+
+#include <Base.h>
+#include <Library/ArmHvcLib.h>
+#include <Library/ArmLib.h>
+#include <Library/ArmSmcLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+
+#include "ArmFwTrngDefs.h"
+
+/** Convert TRNG status codes to EFI status codes.
+
+  @param [in]  TrngStatus    TRNG status code.
+
+  @retval  RETURN_SUCCESS            Success.
+  @retval  RETURN_UNSUPPORTED        Function not implemented.
+  @retval  RETURN_INVALID_PARAMETER  A parameter is invalid.
+  @retval  RETURN_NOT_READY          No Entropy available.
+**/
+STATIC
+RETURN_STATUS
+TrngStatusToEfiStatus (
+  IN  INT32   TrngStatus
+  )
+{
+  switch (TrngStatus) {
+    case TRNG_NOT_SUPPORTED:
+      return RETURN_UNSUPPORTED;
+
+    case TRNG_INVALID_PARAMETER:
+      return RETURN_INVALID_PARAMETER;
+
+    case TRNG_NO_ENTROPY:
+      return RETURN_NOT_READY;
+
+    case TRNG_STATUS_SUCCESS:
+    default:
+      return RETURN_SUCCESS;
+  }
+}
+
+/** Invoke the monitor call using the appropriate conduit.
+    If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
+
+  @param [in, out]  Args    Arguments passed to and returned from the monitor.
+
+  @return  VOID
+**/
+STATIC
+VOID
+ArmCallMonitor (
+  IN OUT ARM_MONITOR_ARGS   *Args
+  )
+{
+  if (FeaturePcdGet (PcdMonitorConduitHvc)) {
+    ArmCallHvc ((ARM_HVC_ARGS*)Args);
+  } else {
+    ArmCallSmc ((ARM_SMC_ARGS*)Args);
+  }
+}
+
+/** Get the version of the TRNG backend.
+
+  A TRNG may be implemented by the system firmware, in which case this
+  function shall return the version of the TRNG backend.
+  The implementation must return NOT_SUPPORTED if a Back end is not present.
+
+  @param [out]  MajorRevision     Major revision.
+  @param [out]  MinorRevision     Minor revision.
+
+  @retval  RETURN_SUCCESS            The function completed successfully.
+  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
+  @retval  RETURN_UNSUPPORTED        Backend not present.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngVersion (
+  OUT UINT16  *MajorRevision,
+  OUT UINT16  *MinorRevision
+  )
+{
+  RETURN_STATUS     Status;
+  ARM_MONITOR_ARGS  Parameters;
+  INT32             Revision;
+
+  if ((MajorRevision == NULL) || (MinorRevision == NULL)) {
+    return RETURN_INVALID_PARAMETER;
+  }
+
+  ZeroMem (&Parameters, sizeof (Parameters));
+
+  /*
+    Cf. [1], 2.1 TRNG_VERSION
+    Function ID (W0) 0x8400_0050
+    Parameters
+        W1-W7 Reserved (MBZ)
+    Returns
+        Success (W0 > 0) W0[31] MBZ
+        W0[30:16] Major revision
+        W0[15:0] Minor revision
+        W1 - W3 Reserved (MBZ)
+    Error (W0 < 0)
+        NOT_SUPPORTED Function not implemented
+  */
+  Parameters.Arg0 = FID_TRNG_VERSION;
+  ArmCallMonitor (&Parameters);
+
+  Revision = (INT32)Parameters.Arg0;
+  // Convert status codes to EFI status codes.
+  Status = TrngStatusToEfiStatus (Revision);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  *MinorRevision = (Revision & TRNG_REV_MINOR_MASK);
+  *MajorRevision = ((Revision >> TRNG_REV_MAJOR_SHIFT) & TRNG_REV_MAJOR_MASK);
+  return RETURN_SUCCESS;
+}
+
+#ifndef MDEPKG_NDEBUG
+/** Get the features supported by the TRNG backend.
+
+  The caller can determine if functions defined in the TRNG ABI are
+  present in the ABI implementation.
+
+  @param [in]  FunctionId         Function Id.
+  @param [out] Capability         Function specific capability if present
+                                  otherwise Zero is returned.
+
+  @retval  RETURN_SUCCESS            The function completed successfully.
+  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
+  @retval  RETURN_UNSUPPORTED        Function not implemented.
+**/
+STATIC
+RETURN_STATUS
+EFIAPI
+GetTrngFeatures (
+  IN  CONST UINT32  FunctionId,
+  OUT       UINT32  *Capability      OPTIONAL
+  )
+{
+  ARM_MONITOR_ARGS  Parameters;
+
+  ZeroMem (&Parameters, sizeof (Parameters));
+
+  /*
+    Cf. [1], Section 2.2 TRNG_FEATURES
+    Function ID (W0) 0x8400_0051
+    Parameters
+        W1 trng_func_id
+        W2-W7 Reserved (MBZ)
+    Returns
+        Success (W0 >= 0)
+          SUCCESS Function is implemented.
+            > 0     Function is implemented and
+                    has specific capabilities,
+                    see function definition.
+          Error (W0 < 0)
+            NOT_SUPPORTED Function with FID=trng_func_id
+            is not implemented
+  */
+  Parameters.Arg0 = FID_TRNG_FEATURES;
+  Parameters.Arg1 = FunctionId;
+  ArmCallMonitor (&Parameters);
+  if (Parameters.Arg0 < TRNG_STATUS_SUCCESS) {
+    return RETURN_UNSUPPORTED;
+  }
+
+  if (Capability != NULL) {
+    *Capability = Parameters.Arg0;
+  }
+
+  return RETURN_SUCCESS;
+}
+#endif  //MDEPKG_NDEBUG
+
+/** Get the UUID of the TRNG backend.
+
+  A TRNG may be implemented by the system firmware, in which case this
+  function shall return the UUID of the TRNG backend.
+  Returning the TRNG UUID is optional and if not implemented, RETURN_UNSUPPORTED
+  shall be returned.
+
+  Note: The caller must not rely on the returned UUID as a trustworthy TRNG
+        Back end identity
+
+  @param [out]  Guid              UUID of the TRNG backend.
+
+  @retval  RETURN_SUCCESS            The function completed successfully.
+  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
+  @retval  RETURN_UNSUPPORTED        Function not implemented.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngUuid (
+  OUT GUID  *Guid
+  )
+{
+  RETURN_STATUS     Status;
+  ARM_MONITOR_ARGS  Parameters;
+
+  ZeroMem (&Parameters, sizeof (Parameters));
+
+  /*
+    Cf. [1], Section 2.3 TRNG_GET_UUID
+    Function ID (W0) 0x8400_0052
+    Parameters
+        W1-W7 Reserved (MBZ)
+    Returns
+        Success (W0 != -1)
+            W0 UUID[31:0]
+            W1 UUID[63:32]
+            W2 UUID[95:64]
+            W3 UUID[127:96]
+        Error (W0 = -1)
+            W0 NOT_SUPPORTED
+  */
+  Parameters.Arg0 = FID_TRNG_GET_UUID;
+  ArmCallMonitor (&Parameters);
+
+  // Convert status codes to EFI status codes.
+  Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Guid->Data1 = (Parameters.Arg0 & MAX_UINT32);
+  Guid->Data2 = (Parameters.Arg1 & MAX_UINT16);
+  Guid->Data3 = ((Parameters.Arg1 >> 16) & MAX_UINT16);
+
+  Guid->Data4[0] = (Parameters.Arg2 & MAX_UINT8);
+  Guid->Data4[1] = ((Parameters.Arg2 >> 8) & MAX_UINT8);
+  Guid->Data4[2] = ((Parameters.Arg2 >> 16) & MAX_UINT8);
+  Guid->Data4[3] = ((Parameters.Arg2 >> 24) & MAX_UINT8);
+
+  Guid->Data4[4] = (Parameters.Arg3 & MAX_UINT8);
+  Guid->Data4[5] = ((Parameters.Arg3 >> 8) & MAX_UINT8);
+  Guid->Data4[6] = ((Parameters.Arg3 >> 16) & MAX_UINT8);
+  Guid->Data4[7] = ((Parameters.Arg3 >> 24) & MAX_UINT8);
+
+  DEBUG ((DEBUG_INFO, "FW-TRNG: UUID %g\n", Guid));
+
+  return RETURN_SUCCESS;
+}
+
+/** Returns maximum number of entropy bits that can be returned in a single
+    call.
+
+  @return Returns the maximum number of Entropy bits that can be returned
+          in a single call to GetEntropy().
+**/
+UINTN
+EFIAPI
+GetTrngMaxSupportedEntropyBits (
+  VOID
+  )
+{
+  return MAX_ENTROPY_BITS;
+}
+
+/** Returns N bits of conditioned entropy.
+
+  See [3] Section 2.3.1 GetEntropy: An Interface to the Entropy Source
+    GetEntropy
+      Input:
+        bits_of_entropy: the requested amount of entropy
+      Output:
+        entropy_bitstring: The string that provides the requested entropy.
+      status: A Boolean value that is TRUE if the request has been satisfied,
+              and is FALSE otherwise.
+
+  Note: In this implementation this function returns a status code instead
+        of a boolean value.
+        This is also compatible with the definition of Get_Entropy, see [2]
+        Section 7.4 Entropy Source Calls.
+          (status, entropy_bitstring) = Get_Entropy (
+                                          requested_entropy,
+                                          max_length
+                                          )
+
+  @param  [in]   EntropyBits  Number of entropy bits requested.
+  @param  [out]  Buffer       Buffer to return the entropy bits.
+  @param  [in]   BufferSize   Size of the Buffer in bytes.
+
+  @retval  RETURN_SUCCESS            The function completed successfully.
+  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
+  @retval  RETURN_UNSUPPORTED        Function not implemented.
+  @retval  RETURN_BAD_BUFFER_SIZE    Buffer size is too small.
+  @retval  RETURN_NOT_READY          No Entropy available.
+**/
+RETURN_STATUS
+EFIAPI
+GetEntropy (
+  IN  CONST UINTN   EntropyBits,
+  OUT       UINT8   *Buffer,
+  IN  CONST UINTN   BufferSize
+  )
+{
+  RETURN_STATUS     Status;
+  ARM_MONITOR_ARGS  Parameters;
+  UINTN             EntropyBytes;
+  UINTN             LastValidBits;
+  UINTN             ArgSelector;
+  UINTN             BytesToClear;
+
+  // [1] Section 2.4.3 Caller responsibilities.
+  // The caller cannot request more than MAX_BITS bits of conditioned
+  // entropy per call.
+  if ((EntropyBits == 0) || (EntropyBits > MAX_ENTROPY_BITS)) {
+    return RETURN_INVALID_PARAMETER;
+  }
+
+  EntropyBytes = (EntropyBits + 7) >> 3;
+  if (EntropyBytes > BufferSize) {
+    return RETURN_BAD_BUFFER_SIZE;
+  }
+
+  ZeroMem (Buffer, BufferSize);
+  ZeroMem (&Parameters, sizeof (Parameters));
+
+  /*
+    Cf. [1], Section 2.4 TRNG_RND
+    Function ID (W0)  0x8400_0053
+                      0xC400_0053
+    SMC32 Parameters
+        W1    N bits of entropy (1 6 N 6 96)
+        W2-W7 Reserved (MBZ)
+    SMC64 Parameters
+        X1    N bits of entropy (1 6 N 6 192)
+        X2-X7 Reserved (MBZ)
+    SMC32 Returns
+        Success (W0 = 0):
+          W0 MBZ
+          W1 Entropy[95:64]
+          W2 Entropy[63:32]
+          W3 Entropy[31:0]
+    Error (W0 < 0)
+          W0 NOT_SUPPORTED
+          NO_ENTROPY
+          INVALID_PARAMETERS
+          W1 - W3 Reserved (MBZ)
+    SMC64 Returns
+          Success (X0 = 0):
+          X0 MBZ
+          X1 Entropy[191:128]
+          X2 Entropy[127:64]
+          X3 Entropy[63:0]
+    Error (X0 < 0)
+          X0 NOT_SUPPORTED
+          NO_ENTROPY
+          INVALID_PARAMETERS
+          X1 - X3 Reserved (MBZ)
+  */
+  Parameters.Arg0 = FID_TRNG_RND;
+  Parameters.Arg1 = EntropyBits;
+  ArmCallMonitor (&Parameters);
+
+  // Convert status codes to EFI status codes.
+  Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  // Extract Data
+  // ArgSelector = ((EntropyBytes + 3) >> 2); for AArch32
+  // ArgSelector = ((EntropyBytes + 7) >> 3); for AArch64
+  // ((sizeof (UINTN) >> 2) + 1) is 3 or 2 depending on size of UINTN
+  ArgSelector = ((EntropyBytes + (sizeof (UINTN) - 1)) >>
+                 ((sizeof (UINTN) >> 2) + 1));
+
+  switch (ArgSelector) {
+    case 3:
+      CopyMem (&Buffer[(sizeof (UINTN) * 2)], &Parameters.Arg1, sizeof (UINTN));
+
+    case 2:
+      CopyMem (&Buffer[sizeof (UINTN)], &Parameters.Arg2, sizeof (UINTN));
+
+    case 1:
+      CopyMem (&Buffer[0], &Parameters.Arg3, sizeof (UINTN));
+      break;
+
+    default:
+      ASSERT (0);
+      return RETURN_INVALID_PARAMETER;
+  } // switch
+
+
+  // [1] Section 2.4.3 Caller responsibilities.
+  // The caller must ensure that only the value in Entropy[N-1:0] is consumed
+  // and that the remaining bits in Entropy[MAX_BITS-1:N] are ignored.
+  // Therefore, Clear the unused upper bytes.
+  BytesToClear = (sizeof (UINTN) * ArgSelector) - EntropyBytes;
+  if (BytesToClear != 0) {
+    ZeroMem (&Buffer[EntropyBytes], BytesToClear);
+  }
+
+  // Clear the unused MSB bits of the last byte.
+  LastValidBits = EntropyBits & 0x7;
+  if (LastValidBits != 0) {
+    Buffer[EntropyBytes - 1] &= (0xFF >> (8 - LastValidBits));
+  }
+
+  return Status;
+}
+
+/** The constructor checks that the FW-TRNG interface is supported
+    by the host firmware.
+
+  It will ASSERT() if FW-TRNG is not supported.
+  It will always return RETURN_SUCCESS.
+
+  @retval RETURN_SUCCESS   The constructor always returns RETURN_SUCCESS.
+**/
+RETURN_STATUS
+EFIAPI
+ArmFwTrngLibConstructor (
+  VOID
+  )
+{
+  RETURN_STATUS Status;
+  UINT16        MajorRev;
+  UINT16        MinorRev;
+  GUID          Guid;
+
+  Status = GetTrngVersion (&MajorRev, &MinorRev);
+  if (EFI_ERROR (Status)) {
+    return RETURN_SUCCESS;
+  }
+
+#ifndef MDEPKG_NDEBUG
+  // Check that the required features are present.
+  Status = GetTrngFeatures (FID_TRNG_RND, NULL);
+  if (EFI_ERROR (Status)) {
+    return RETURN_SUCCESS;
+  }
+
+  // Check if TRNG UUID is supported and if so trace the GUID.
+  Status = GetTrngFeatures (FID_TRNG_GET_UUID, NULL);
+  if (EFI_ERROR (Status)) {
+    return RETURN_SUCCESS;
+  }
+#endif
+
+  Status = GetTrngUuid (&Guid);
+  if (EFI_ERROR (Status)) {
+    return RETURN_SUCCESS;
+  }
+
+  DEBUG ((
+    DEBUG_INFO,
+    "FW-TRNG: Version %d.%d, GUID {%g}\n",
+    MajorRev,
+    MinorRev,
+    Guid
+    ));
+
+  return RETURN_SUCCESS;
+}
diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..4b2c58251fbe8fbcb5af308736db014e8d954720
--- /dev/null
+++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
@@ -0,0 +1,34 @@
+## @file
+#  Arm Firmware TRNG interface library.
+#
+#  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION       = 0x0001001B
+  BASE_NAME         = ArmFwTrngLib
+  FILE_GUID         = 10DE97C9-28E4-4C9B-A53E-8D7D1B0DD4E0
+  VERSION_STRING    = 1.0
+  MODULE_TYPE       = BASE
+  LIBRARY_CLASS     = TrngLib
+  CONSTRUCTOR       = ArmFwTrngLibConstructor
+
+[Sources]
+  ArmFwTrngDefs.h
+  ArmFwTrngLib.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  ArmSmcLib
+  ArmHvcLib
+  BaseLib
+  BaseMemoryLib
+
+[Pcd]
+  gArmTokenSpaceGuid.PcdMonitorConduitHvc
+
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 4/8] MdePkg: Add NULL instance of TRNG Library
  2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
                   ` (2 preceding siblings ...)
  2021-11-16 11:32 ` [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
  2021-11-16 11:32 ` [PATCH v2 5/8] SecurityPkg: Rename RdRandGenerateEntropy to common name Sami Mujawar
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
	michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
	jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd

Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

The True Random Number Generator (TRNG) library defines an
interface to access the entropy source on a platform. On
platforms that do not have access to an entropy source, a
NULL instance of the TRNG library may be useful to satisfy
the build dependency.

Therefore, add a NULL instance of the TRNG library.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
     - MdePkg\Include\Library\TrngLib.h is base type     [LIMING]
       library. It can use RETURN_STATUS instead of
       EFI_STATUS.
     - Replaced EFI_STATUS with RETURN_STATUS.           [SAMI]
     - MdePkg\Include\Library\TrngLib.h API parameter    [LIMING]
       doesn't require CONST. CONST means the value
       specified by the input pointer will not be
       changed in API implementation.
     - Removed the use of constant pointers in the       [SAMI]
       TRNG API.

 MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c   | 111 ++++++++++++++++++++
 MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf |  30 ++++++
 MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni |  12 +++
 MdePkg/MdePkg.dsc                                  |   1 +
 4 files changed, 154 insertions(+)

diff --git a/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c
new file mode 100644
index 0000000000000000000000000000000000000000..406a1e8587ccfb0cd903bf7a3794f16627eb0a84
--- /dev/null
+++ b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c
@@ -0,0 +1,111 @@
+/** @file
+  Null version of TRNG (True Random Number Generator) services.
+
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Glossary:
+    - TRNG - True Random Number Generator
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/TrngLib.h>
+
+/** Get the TRNG version.
+
+  A TRNG may be implemented by the system firmware, in which case this
+  function shall return the version information for the TRNG implementation.
+  Returning version information is optional and if not implemented,
+  RETURN_UNSUPPORTED shall be returned.
+
+  @param [out]  MajorRevision     Major revision.
+  @param [out]  MinorRevision     Minor revision.
+
+  @retval  RETURN_SUCCESS            The function completed successfully.
+  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
+  @retval  RETURN_UNSUPPORTED        Function not implemented.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngVersion (
+  OUT UINT16  *MajorRevision,
+  OUT UINT16  *MinorRevision
+  )
+{
+  ASSERT (FALSE);
+  return RETURN_UNSUPPORTED;
+}
+
+/** Get the UUID of the TRNG backend.
+
+  A TRNG may be implemented by the system firmware, in which case this
+  function shall return the UUID for the TRNG implementation.
+  Returning the TRNG UUID is optional and if not implemented, RETURN_UNSUPPORTED
+  shall be returned.
+
+  @param [out]  Guid              UUID of the TRNG backend.
+
+  @retval  RETURN_SUCCESS            The function completed successfully.
+  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
+  @retval  RETURN_UNSUPPORTED        Function not implemented.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngUuid (
+  OUT GUID  *Guid
+  )
+{
+  ASSERT (FALSE);
+  return RETURN_UNSUPPORTED;
+}
+
+/** Returns maximum number of entropy bits that can be returned in a single
+    call.
+
+  @return Returns the maximum number of Entropy bits that can be returned
+          in a single call to GetEntropy().
+**/
+UINTN
+EFIAPI
+GetTrngMaxSupportedEntropyBits (
+  VOID
+  )
+{
+  ASSERT (FALSE);
+  return 0;
+}
+
+/** Returns N bits of conditioned entropy.
+
+  See [3] Section 2.3.1 GetEntropy: An Interface to the Entropy Source
+    GetEntropy
+      Input:
+        bits_of_entropy: the requested amount of entropy
+      Output:
+        entropy_bitstring: The string that provides the requested entropy.
+      status: A Boolean value that is TRUE if the request has been satisfied,
+              and is FALSE otherwise.
+    Note: In this implementation this function returns a status code instead
+          of a boolean value.
+
+  @param  [in]   EntropyBits  Number of entropy bits requested.
+  @param  [out]  Buffer       Buffer to return the entropy bits.
+  @param  [in]   BufferSize   Size of the Buffer in bytes.
+
+  @retval  RETURN_SUCCESS            The function completed successfully.
+  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
+  @retval  RETURN_UNSUPPORTED        Function not implemented.
+  @retval  RETURN_BAD_BUFFER_SIZE    Buffer size is too small.
+  @retval  RETURN_NOT_READY          No Entropy available.
+**/
+RETURN_STATUS
+EFIAPI
+GetEntropy (
+  IN  CONST UINTN   EntropyBits,
+  OUT       UINT8   *Buffer,
+  IN  CONST UINTN   BufferSize
+  )
+{
+  ASSERT (FALSE);
+  return RETURN_UNSUPPORTED;
+}
diff --git a/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf
new file mode 100644
index 0000000000000000000000000000000000000000..a700cf66f457f8898d5c51a7b9c0b3d7643ff7f9
--- /dev/null
+++ b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf
@@ -0,0 +1,30 @@
+## @file
+#  Null instance of TRNG (True Random Number Generator) Library.
+#
+#  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = BaseTrngLibNull
+  MODULE_UNI_FILE                = BaseTrngLibNull.uni
+  FILE_GUID                      = ABDE1C87-4F50-4B82-9133-7A79E13F69AB
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = TrngLib
+
+#
+#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
+#
+
+[Sources]
+  BaseTrngLibNull.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  DebugLib
diff --git a/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni
new file mode 100644
index 0000000000000000000000000000000000000000..1ec7def522e5975e9621eb280776251b1e5502ca
--- /dev/null
+++ b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni
@@ -0,0 +1,12 @@
+// /** @file
+// Null Instance of TRNG (True Random Number Generator) Library.
+//
+//  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+#string STR_MODULE_ABSTRACT             #language en-US "Null instance of TRNG Library"
+
+#string STR_MODULE_DESCRIPTION          #language en-US "This library instance should be used with modules that inherit an (indirect) dependency on the TrngLib class, but never actually call TrngLib APIs for consuming Entropy."
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index a94959169b2fd9d4b5bf7ad903bf5ce06566c60e..f83753e132e9b9eb4152927fc182701fb1e70ca4 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -67,6 +67,7 @@ [Components]
   MdePkg/Library/DxeRngLib/DxeRngLib.inf
   MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf
   MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
+  MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf
 
   MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
   MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 5/8] SecurityPkg: Rename RdRandGenerateEntropy to common name
  2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
                   ` (3 preceding siblings ...)
  2021-11-16 11:32 ` [PATCH v2 4/8] MdePkg: Add NULL instance of TRNG Library Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
  2021-11-16 11:32 ` [PATCH v2 6/8] SecurityPkg: Restructure checks in RngGetInfo Sami Mujawar
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
	michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
	jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd

Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

Rename RdRandGenerateEntropy() to GenerateEntropy() to provide a
common interface to generate entropy on other architectures.

Also move the definition to RngDxeInternals.h

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
     - No code change since v1. Re-sending with V2 series.   [SAMI]

 SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c     | 13 ++++--
 SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h     | 43 --------------------
 SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c     |  6 ++-
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf        |  1 -
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h | 19 +++++++++
 5 files changed, 33 insertions(+), 49 deletions(-)

diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c
index 83025a47d43d442bfe1c324eda2916e6b5599a7e..0ee99a8661fc20094daef019a2f8015597073be4 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c
@@ -1,15 +1,22 @@
 /** @file
-  Support routines for RDRAND instruction access.
+  Support routines for RDRAND instruction access, which will leverage
+  Intel Secure Key technology to provide high-quality random numbers for use
+  in applications, or entropy for seeding other random number generators.
+  Refer to http://software.intel.com/en-us/articles/intel-digital-random-number
+  -generator-drng-software-implementation-guide/ for more information about Intel
+  Secure Key technology.
 
 Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
 #include <Library/RngLib.h>
+#include <Library/TimerLib.h>
 
 #include "AesCore.h"
-#include "RdRand.h"
 #include "RngDxeInternals.h"
 
 /**
@@ -87,7 +94,7 @@ RdRandGetSeed128 (
 **/
 EFI_STATUS
 EFIAPI
-RdRandGenerateEntropy (
+GenerateEntropy (
   IN UINTN         Length,
   OUT UINT8        *Entropy
   )
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h
deleted file mode 100644
index 072378e062e7bee81a7e763fe9b4ed4517e4d82c..0000000000000000000000000000000000000000
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h
+++ /dev/null
@@ -1,43 +0,0 @@
-/** @file
-  Header for the RDRAND APIs used by RNG DXE driver.
-
-  Support API definitions for RDRAND instruction access, which will leverage
-  Intel Secure Key technology to provide high-quality random numbers for use
-  in applications, or entropy for seeding other random number generators.
-  Refer to http://software.intel.com/en-us/articles/intel-digital-random-number
-  -generator-drng-software-implementation-guide/ for more information about Intel
-  Secure Key technology.
-
-Copyright (c) 2013, Intel Corporation. All rights reserved.<BR>
-(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#ifndef __RD_RAND_H__
-#define __RD_RAND_H__
-
-#include <Library/BaseLib.h>
-#include <Library/BaseMemoryLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Library/TimerLib.h>
-#include <Protocol/Rng.h>
-
-/**
-  Generate high-quality entropy source through RDRAND.
-
-  @param[in]   Length        Size of the buffer, in bytes, to fill with.
-  @param[out]  Entropy       Pointer to the buffer to store the entropy data.
-
-  @retval EFI_SUCCESS        Entropy generation succeeded.
-  @retval EFI_NOT_READY      Failed to request random data.
-
-**/
-EFI_STATUS
-EFIAPI
-RdRandGenerateEntropy (
-  IN UINTN         Length,
-  OUT UINT8        *Entropy
-  );
-
-#endif  // __RD_RAND_H__
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
index d0e6b7de06352b6a92a823681eab92b7a4ca720f..2009f95b4cadb07fc9073c3c0660cf549965422a 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
@@ -20,7 +20,9 @@
 
 **/
 
-#include "RdRand.h"
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+
 #include "RngDxeInternals.h"
 
 /**
@@ -88,7 +90,7 @@ RngGetRNG (
       return EFI_INVALID_PARAMETER;
     }
 
-    Status = RdRandGenerateEntropy (RNGValueLength, RNGValue);
+    Status = GenerateEntropy (RNGValueLength, RNGValue);
     return Status;
   }
 
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
index f3300971993f7c6fcdca441858de4c2fb35912e8..ef5cd73273e68c67bec7411279bb8433c45ab2d4 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
@@ -36,7 +36,6 @@ [Sources.common]
 [Sources.IA32, Sources.X64]
   Rand/RngDxe.c
   Rand/RdRand.c
-  Rand/RdRand.h
   Rand/AesCore.c
   Rand/AesCore.h
 
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
index 2660ed5875e0d52a6b9d806341431859374b1047..34886adcf549efdedc1a7b8f16b81a5148531de2 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
@@ -10,6 +10,8 @@
 #ifndef RNGDXE_INTERNALS_H_
 #define RNGDXE_INTERNALS_H_
 
+#include <Protocol/Rng.h>
+
 /**
   Returns information about the random number generation implementation.
 
@@ -114,4 +116,21 @@ RngGetBytes (
   OUT UINT8        *RandBuffer
   );
 
+/**
+  Generate high-quality entropy source using a TRNG or through RDRAND.
+
+  @param[in]   Length        Size of the buffer, in bytes, to fill with.
+  @param[out]  Entropy       Pointer to the buffer to store the entropy data.
+
+  @retval EFI_SUCCESS        Entropy generation succeeded.
+  @retval EFI_NOT_READY      Failed to request random data.
+
+**/
+EFI_STATUS
+EFIAPI
+GenerateEntropy (
+  IN UINTN         Length,
+  OUT UINT8        *Entropy
+  );
+
 #endif  // RNGDXE_INTERNALS_H_
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 6/8] SecurityPkg: Restructure checks in RngGetInfo
  2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
                   ` (4 preceding siblings ...)
  2021-11-16 11:32 ` [PATCH v2 5/8] SecurityPkg: Rename RdRandGenerateEntropy to common name Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
  2021-11-16 11:32 ` [PATCH v2 7/8] SecurityPkg: Add RawAlgorithm support using TRNG library Sami Mujawar
  2021-11-16 11:33 ` [PATCH v2 8/8] ArmVirtPkg: Kvmtool: Add RNG support using FW-TRNG interface Sami Mujawar
  7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
	michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
	jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd

Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

Move the check to see if the RNGAlgorithmList pointer is NULL to
ArchGetSupportedRngAlgorithms(). This allows the caller to obtain
the buffer size required to store the Algorithm List by passing
RNGAlgorithmListSize as zero and RNGAlgorithmList as NULL.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
     - No code change since v1. Re-sending with V2 series.   [SAMI]

 SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c     |  6 ++++++
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c          | 11 ++---------
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h |  2 +-
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
index 2009f95b4cadb07fc9073c3c0660cf549965422a..f1122a48102595506cc423c3ab501d9a72f50543 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
@@ -16,6 +16,7 @@
 
   Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
   (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -116,6 +117,7 @@ RngGetRNG (
 
   @retval EFI_SUCCESS                 The RNG algorithm list was returned successfully.
   @retval EFI_BUFFER_TOO_SMALL        The buffer RNGAlgorithmList is too small to hold the result.
+  @retval EFI_INVALID_PARAMETER       The pointer to the buffer RNGAlgorithmList is invalid.
 
 **/
 UINTN
@@ -135,6 +137,10 @@ ArchGetSupportedRngAlgorithms (
     return EFI_BUFFER_TOO_SMALL;
   }
 
+  if (RNGAlgorithmList == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   CpuRngSupportedAlgorithm = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
 
   CopyMem(&RNGAlgorithmList[0], CpuRngSupportedAlgorithm, sizeof (EFI_RNG_ALGORITHM));
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
index b959c70536ea3b9049905bbfd3d973fc9b2f6dcf..2e3b714bc691e4e517866369c034b721fbccfa24 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
@@ -14,6 +14,7 @@
 
 Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -59,8 +60,6 @@ RngGetInfo (
   OUT EFI_RNG_ALGORITHM           *RNGAlgorithmList
   )
 {
-  EFI_STATUS    Status;
-
   if ((This == NULL) || (RNGAlgorithmListSize == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
@@ -68,13 +67,7 @@ RngGetInfo (
   //
   // Return algorithm list supported by driver.
   //
-  if (RNGAlgorithmList != NULL) {
-    Status = ArchGetSupportedRngAlgorithms (RNGAlgorithmListSize, RNGAlgorithmList);
-  } else {
-    Status = EFI_INVALID_PARAMETER;
-  }
-
-  return Status;
+  return ArchGetSupportedRngAlgorithms (RNGAlgorithmListSize, RNGAlgorithmList);
 }
 
 //
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
index 34886adcf549efdedc1a7b8f16b81a5148531de2..37c27c4094e5302dfe2e7d9bbeef33a24b0c73ea 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
@@ -90,7 +90,7 @@ RngGetRNG (
 
   @retval EFI_SUCCESS                 The RNG algorithm list was returned successfully.
   @retval EFI_BUFFER_TOO_SMALL        The buffer RNGAlgorithmList is too small to hold the result.
-
+  @retval EFI_INVALID_PARAMETER       The pointer to the buffer RNGAlgorithmList is invalid.
 **/
 UINTN
 EFIAPI
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 7/8] SecurityPkg: Add RawAlgorithm support using TRNG library
  2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
                   ` (5 preceding siblings ...)
  2021-11-16 11:32 ` [PATCH v2 6/8] SecurityPkg: Restructure checks in RngGetInfo Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
  2021-11-16 11:33 ` [PATCH v2 8/8] ArmVirtPkg: Kvmtool: Add RNG support using FW-TRNG interface Sami Mujawar
  7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
	michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
	jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd

Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

RawAlgorithm is used to provide access to entropy that is suitable
for cryptographic applications. Therefore, add RawAlgorithm support
that provides access to entropy using the TRNG library interface.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
     - MdeModulePkg\Include\Guid\ZeroGuid.h has defined     [LIMING]
       gZeroGuid. You don't define it again.
     - Replaced use of gNullGuid with gZeroGuid.            [SAMI]

 SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c  |  79 ++++++++--
 SecurityPkg/RandomNumberGenerator/RngDxe/Arm/RngDxe.c      | 163 ++++++++++++++++++++
 SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c         |  61 ++++++++
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c          |   2 +-
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf        |  13 +-
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h |   1 +
 SecurityPkg/SecurityPkg.dsc                                |   8 +-
 7 files changed, 314 insertions(+), 13 deletions(-)

diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c
index 282fdca9d334b77e02ca47734df08729e0f4fd31..d1c8f4c69b4d65c10141da320d44cd8f01bb0c74 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c
@@ -1,11 +1,12 @@
 /** @file
   RNG Driver to produce the UEFI Random Number Generator protocol.
 
-  The driver will use the RNDR instruction to produce random numbers.
+  The driver will use the RNDR instruction to produce random numbers. It also
+  uses the Arm FW-TRNG interface to implement EFI_RNG_ALGORITHM_RAW.
 
   RNG Algorithms defined in UEFI 2.4:
    - EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID
-   - EFI_RNG_ALGORITHM_RAW                    - Unsupported
+   - EFI_RNG_ALGORITHM_RAW
    - EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID
    - EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID
    - EFI_RNG_ALGORITHM_X9_31_3DES_GUID        - Unsupported
@@ -14,15 +15,17 @@
   Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
   Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
   (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
+#include <Guid/ZeroGuid.h>
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Library/TimerLib.h>
+#include <Library/DebugLib.h>
+#include <Library/TrngLib.h>
 #include <Protocol/Rng.h>
 
 #include "RngDxeInternals.h"
@@ -58,7 +61,9 @@ RngGetRNG (
   OUT UINT8                      *RNGValue
   )
 {
-  EFI_STATUS    Status;
+  EFI_STATUS  Status;
+  UINT16      MajorRevision;
+  UINT16      MinorRevision;
 
   if ((RNGValueLength == 0) || (RNGValue == NULL)) {
     return EFI_INVALID_PARAMETER;
@@ -76,6 +81,17 @@ RngGetRNG (
     return Status;
   }
 
+  //
+  // The "raw" algorithm is intended to provide entropy directly
+  //
+  if (CompareGuid (RNGAlgorithm, &gEfiRngAlgorithmRaw)) {
+    Status = GetTrngVersion (&MajorRevision, &MinorRevision);
+    if (EFI_ERROR (Status)) {
+      return EFI_UNSUPPORTED;
+    }
+    return GenerateEntropy (RNGValueLength, RNGValue);
+  }
+
   //
   // Other algorithms are unsupported by this driver.
   //
@@ -97,8 +113,9 @@ RngGetRNG (
                                       is the default algorithm for the driver.
 
   @retval EFI_SUCCESS                 The RNG algorithm list was returned successfully.
+  @retval EFI_UNSUPPORTED             No supported algorithms found.
   @retval EFI_BUFFER_TOO_SMALL        The buffer RNGAlgorithmList is too small to hold the result.
-
+  @retval EFI_INVALID_PARAMETER       The pointer to the buffer RNGAlgorithmList is invalid.
 **/
 UINTN
 EFIAPI
@@ -107,19 +124,61 @@ ArchGetSupportedRngAlgorithms (
   OUT    EFI_RNG_ALGORITHM         *RNGAlgorithmList
   )
 {
-  UINTN RequiredSize;
+  EFI_STATUS  Status;
+  UINT16      MajorRevision;
+  UINT16      MinorRevision;
+  UINTN       RequiredSize;
+  BOOLEAN     CpuRngAlgorithmSupported;
+  BOOLEAN     RawAlgorithmSupported;
+  UINTN       Index;
   EFI_RNG_ALGORITHM *CpuRngSupportedAlgorithm;
 
-  RequiredSize = sizeof (EFI_RNG_ALGORITHM);
+  RequiredSize = 0;
+  CpuRngAlgorithmSupported = FALSE;
+  RawAlgorithmSupported = FALSE;
+
+  CpuRngSupportedAlgorithm = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
+  if (!CompareGuid (CpuRngSupportedAlgorithm, &gZeroGuid)) {
+    CpuRngAlgorithmSupported = TRUE;
+    RequiredSize += sizeof (EFI_RNG_ALGORITHM);
+  }
+
+  Status = GetTrngVersion (&MajorRevision, &MinorRevision);
+  if (!EFI_ERROR (Status)) {
+    RawAlgorithmSupported = TRUE;
+    RequiredSize += sizeof (EFI_RNG_ALGORITHM);
+  }
 
   if (*RNGAlgorithmListSize < RequiredSize) {
     *RNGAlgorithmListSize = RequiredSize;
     return EFI_BUFFER_TOO_SMALL;
   }
 
-  CpuRngSupportedAlgorithm = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
+  if (RequiredSize == 0) {
+    // No supported algorithms found.
+    return EFI_UNSUPPORTED;
+  }
 
-  CopyMem(&RNGAlgorithmList[0], CpuRngSupportedAlgorithm, sizeof (EFI_RNG_ALGORITHM));
+  if (RNGAlgorithmList == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Index = 0;
+  if (CpuRngAlgorithmSupported) {
+    CopyMem (
+      &RNGAlgorithmList[Index++],
+      CpuRngSupportedAlgorithm,
+      sizeof (EFI_RNG_ALGORITHM)
+      );
+  }
+
+  if (RawAlgorithmSupported) {
+    CopyMem (
+      &RNGAlgorithmList[Index++],
+      &gEfiRngAlgorithmRaw,
+      sizeof (EFI_RNG_ALGORITHM)
+      );
+  }
 
   *RNGAlgorithmListSize = RequiredSize;
   return EFI_SUCCESS;
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/RngDxe.c
new file mode 100644
index 0000000000000000000000000000000000000000..cba9883e50cefbb22495190d17de99bfeab33cf3
--- /dev/null
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/RngDxe.c
@@ -0,0 +1,163 @@
+/** @file
+  RNG Driver to produce the UEFI Random Number Generator protocol.
+
+  The driver implements the EFI_RNG_ALGORITHM_RAW using the FW-TRNG
+  interface to provide entropy.
+
+  RNG Algorithms defined in UEFI 2.4:
+   - EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID
+   - EFI_RNG_ALGORITHM_RAW
+   - EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID
+   - EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID
+   - EFI_RNG_ALGORITHM_X9_31_3DES_GUID        - Unsupported
+   - EFI_RNG_ALGORITHM_X9_31_AES_GUID         - Unsupported
+
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/TrngLib.h>
+#include <Protocol/Rng.h>
+
+#include "RngDxeInternals.h"
+
+/**
+  Produces and returns an RNG value using either the default or specified
+  RNG algorithm.
+
+  @param[in]  This                  A pointer to the EFI_RNG_PROTOCOL instance.
+  @param[in]  RNGAlgorithm          A pointer to the EFI_RNG_ALGORITHM that
+                                    identifies the RNG algorithm to use. May be
+                                    NULL in which case the function will use its
+                                    default RNG algorithm.
+  @param[in]  RNGValueLength        The length in bytes of the memory buffer
+                                    pointed to by RNGValue. The driver shall
+                                    return exactly this numbers of bytes.
+  @param[out] RNGValue              A caller-allocated memory buffer filled by
+                                    the driver with the resulting RNG value.
+
+  @retval EFI_SUCCESS               The RNG value was returned successfully.
+  @retval EFI_UNSUPPORTED           The algorithm specified by RNGAlgorithm is
+                                    not supported by this driver.
+  @retval EFI_DEVICE_ERROR          An RNG value could not be retrieved due to
+                                    a hardware or firmware error.
+  @retval EFI_NOT_READY             There is not enough random data available
+                                    to satisfy the length requested by
+                                    RNGValueLength.
+  @retval EFI_INVALID_PARAMETER     RNGValue is NULL or RNGValueLength is zero.
+
+**/
+EFI_STATUS
+EFIAPI
+RngGetRNG (
+  IN EFI_RNG_PROTOCOL            *This,
+  IN EFI_RNG_ALGORITHM           *RNGAlgorithm, OPTIONAL
+  IN UINTN                       RNGValueLength,
+  OUT UINT8                      *RNGValue
+  )
+{
+  EFI_STATUS  Status;
+  UINT16      MajorRevision;
+  UINT16      MinorRevision;
+
+  if ((RNGValueLength == 0) || (RNGValue == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (RNGAlgorithm == NULL) {
+    //
+    // Use the default RNG algorithm if RNGAlgorithm is NULL.
+    //
+    RNGAlgorithm = &gEfiRngAlgorithmRaw;
+  }
+
+  //
+  // The "raw" algorithm is intended to provide entropy directly
+  //
+  if (CompareGuid (RNGAlgorithm, &gEfiRngAlgorithmRaw)) {
+    Status = GetTrngVersion (&MajorRevision, &MinorRevision);
+    if (EFI_ERROR (Status)) {
+      return EFI_UNSUPPORTED;
+    }
+    return GenerateEntropy (RNGValueLength, RNGValue);
+  }
+
+  //
+  // Other algorithms are unsupported by this driver.
+  //
+  return EFI_UNSUPPORTED;
+}
+
+/**
+  Returns information about the random number generation implementation.
+
+  @param[in,out] RNGAlgorithmListSize On input, the size in bytes of
+                                      RNGAlgorithmList.
+                                      On output with a return code of
+                                      EFI_SUCCESS, the size in bytes of the
+                                      data returned in RNGAlgorithmList.
+                                      On output with a return code of
+                                      EFI_BUFFER_TOO_SMALL, the size of
+                                      RNGAlgorithmList required to obtain the
+                                      list.
+  @param[out] RNGAlgorithmList        A caller-allocated memory buffer filled
+                                      by the driver with one EFI_RNG_ALGORITHM
+                                      element for each supported RNG algorithm.
+                                      The list must not change across multiple
+                                      calls to the same driver. The first
+                                      algorithm in the list is the default
+                                      algorithm for the driver.
+
+  @retval EFI_SUCCESS                 The RNG algorithm list was returned
+                                      successfully.
+  @retval EFI_UNSUPPORTED             No supported algorithms found.
+  @retval EFI_BUFFER_TOO_SMALL        The buffer RNGAlgorithmList is too small
+                                      to hold the result.
+  @retval EFI_INVALID_PARAMETER       The pointer to the buffer RNGAlgorithmList
+                                      is invalid.
+**/
+UINTN
+EFIAPI
+ArchGetSupportedRngAlgorithms (
+  IN OUT UINTN                     *RNGAlgorithmListSize,
+  OUT    EFI_RNG_ALGORITHM         *RNGAlgorithmList
+  )
+{
+  EFI_STATUS  Status;
+  UINTN       RequiredSize;
+  UINT16      MajorRevision;
+  UINT16      MinorRevision;
+
+  RequiredSize = 0;
+
+  Status = GetTrngVersion (&MajorRevision, &MinorRevision);
+  if (EFI_ERROR (Status)) {
+    // No supported algorithms found.
+    return EFI_UNSUPPORTED;
+  }
+
+  RequiredSize += sizeof (EFI_RNG_ALGORITHM);
+
+  if (*RNGAlgorithmListSize < RequiredSize) {
+    *RNGAlgorithmListSize = RequiredSize;
+    return EFI_BUFFER_TOO_SMALL;
+  }
+
+  if (RNGAlgorithmList == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  CopyMem (
+    &RNGAlgorithmList[0],
+    &gEfiRngAlgorithmRaw,
+    sizeof (EFI_RNG_ALGORITHM)
+    );
+
+  *RNGAlgorithmListSize = RequiredSize;
+  return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c
new file mode 100644
index 0000000000000000000000000000000000000000..8df37d82e2051854f74816711a14ee23472f6b41
--- /dev/null
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c
@@ -0,0 +1,61 @@
+/** @file
+  Arm FW-TRNG interface helper common for AArch32 and AArch64.
+
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/TrngLib.h>
+
+/**
+  Generate high-quality entropy source using a TRNG.
+
+  @param[in]   Length        Size of the buffer, in bytes, to fill with.
+  @param[out]  Entropy       Pointer to the buffer to store the entropy data.
+
+  @retval EFI_SUCCESS        Entropy generation succeeded.
+  @retval EFI_NOT_READY      Failed to request random data.
+
+**/
+EFI_STATUS
+EFIAPI
+GenerateEntropy (
+  IN  UINTN        Length,
+  OUT UINT8        *Entropy
+  )
+{
+  EFI_STATUS  Status;
+  UINTN       CollectedEntropyBits;
+  UINTN       RequiredEntropyBits;
+  UINTN       EntropyBits;
+  UINTN       Index;
+  UINTN       MaxBits;
+
+  ZeroMem (Entropy, Length);
+
+  RequiredEntropyBits = (Length << 3);
+  Index = 0;
+  CollectedEntropyBits = 0;
+  MaxBits = GetTrngMaxSupportedEntropyBits ();
+  while (CollectedEntropyBits < RequiredEntropyBits) {
+    EntropyBits = MIN ((RequiredEntropyBits - CollectedEntropyBits), MaxBits);
+    Status = GetEntropy (
+               EntropyBits,
+               &Entropy[Index],
+               (Length - Index)
+               );
+    if (EFI_ERROR (Status)) {
+      // Discard the collected bits.
+      ZeroMem (Entropy, Length);
+      return Status;
+    }
+    CollectedEntropyBits += EntropyBits;
+    Index += (EntropyBits >> 3);
+  } // while
+
+  return Status;
+}
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
index 2e3b714bc691e4e517866369c034b721fbccfa24..b7ac0baf3f8216c9a86029b3037bfe4fd59269f6 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
@@ -45,7 +45,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
                                       is the default algorithm for the driver.
 
   @retval EFI_SUCCESS                 The RNG algorithm list was returned successfully.
-  @retval EFI_UNSUPPORTED             The services is not supported by this driver.
+  @retval EFI_UNSUPPORTED             No supported algorithms found.
   @retval EFI_DEVICE_ERROR            The list of algorithms could not be retrieved due to a
                                       hardware or firmware error.
   @retval EFI_INVALID_PARAMETER       One or more of the parameters are incorrect.
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
index ef5cd73273e68c67bec7411279bb8433c45ab2d4..9f2e92512bfa48bd772c7f887a23453756421b80 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
@@ -10,6 +10,7 @@
 #
 #  Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+#  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -26,7 +27,7 @@ [Defines]
 #
 # The following information is for reference only and not required by the build tools.
 #
-#  VALID_ARCHITECTURES           = IA32 X64 AARCH64
+#  VALID_ARCHITECTURES           = IA32 X64 AARCH64 ARM
 #
 
 [Sources.common]
@@ -41,8 +42,14 @@ [Sources.IA32, Sources.X64]
 
 [Sources.AARCH64]
   AArch64/RngDxe.c
+  ArmTrng.c
+
+[Sources.ARM]
+  Arm/RngDxe.c
+  ArmTrng.c
 
 [Packages]
+  MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   SecurityPkg/SecurityPkg.dec
 
@@ -55,6 +62,9 @@ [LibraryClasses]
   TimerLib
   RngLib
 
+[LibraryClasses.AARCH64, LibraryClasses.ARM]
+  TrngLib
+
 [Guids]
   gEfiRngAlgorithmSp80090Hash256Guid  ## SOMETIMES_PRODUCES    ## GUID        # Unique ID of the algorithm for RNG
   gEfiRngAlgorithmSp80090Hmac256Guid  ## SOMETIMES_PRODUCES    ## GUID        # Unique ID of the algorithm for RNG
@@ -62,6 +72,7 @@ [Guids]
   gEfiRngAlgorithmX9313DesGuid        ## SOMETIMES_PRODUCES    ## GUID        # Unique ID of the algorithm for RNG
   gEfiRngAlgorithmX931AesGuid         ## SOMETIMES_PRODUCES    ## GUID        # Unique ID of the algorithm for RNG
   gEfiRngAlgorithmRaw                 ## SOMETIMES_PRODUCES    ## GUID        # Unique ID of the algorithm for RNG
+  gZeroGuid                           ## CONSUMES
 
 [Protocols]
   gEfiRngProtocolGuid                ## PRODUCES
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
index 37c27c4094e5302dfe2e7d9bbeef33a24b0c73ea..8978d54f51d4e72ad881ee584e16dcdda72a66ae 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
@@ -89,6 +89,7 @@ RngGetRNG (
                                       is the default algorithm for the driver.
 
   @retval EFI_SUCCESS                 The RNG algorithm list was returned successfully.
+  @retval EFI_UNSUPPORTED             No supported algorithms found.
   @retval EFI_BUFFER_TOO_SMALL        The buffer RNGAlgorithmList is too small to hold the result.
   @retval EFI_INVALID_PARAMETER       The pointer to the buffer RNGAlgorithmList is invalid.
 **/
diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
index 73a93c2285b13a2e0ce45b08a1230a766e0d759a..63da3d8c92e5a2c559b7731dd6dc0654caab30b8 100644
--- a/SecurityPkg/SecurityPkg.dsc
+++ b/SecurityPkg/SecurityPkg.dsc
@@ -3,6 +3,7 @@
 #
 # Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>
 # (C) Copyright 2015-2020 Hewlett Packard Enterprise Development LP<BR>
+# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -86,6 +87,11 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
 
   ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
 
+  # Arm FW-TRNG interface library.
+  TrngLib|ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
+  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+  ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
+
 [LibraryClasses.ARM]
   RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
@@ -277,7 +283,7 @@ [Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
   SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.inf
   SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.inf
 
-[Components.IA32, Components.X64, Components.AARCH64]
+[Components.IA32, Components.X64, Components.AARCH64, Components.ARM]
   #
   # Random Number Generator
   #
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 8/8] ArmVirtPkg: Kvmtool: Add RNG support using FW-TRNG interface
  2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
                   ` (6 preceding siblings ...)
  2021-11-16 11:32 ` [PATCH v2 7/8] SecurityPkg: Add RawAlgorithm support using TRNG library Sami Mujawar
@ 2021-11-16 11:33 ` Sami Mujawar
  7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:33 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
	michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
	jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd

Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

The EFI_RNG_PROTOCOL published by RngDxe has been updated to
implement the EFI_RNG_ALGORITHM_RAW using the Arm FW-TRNG
interface to provide access to entropy.

Therefore, enable EFI_RNG_PROTOCOL for the Kvmtool guest/virtual
firmware.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
     - No code change since v1. Re-sending with V2 series.   [SAMI]

 ArmVirtPkg/ArmVirtKvmTool.dsc | 10 ++++++++++
 ArmVirtPkg/ArmVirtKvmTool.fdf |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 9d23072d8fa893907848ef105b2c96953a68c56e..418ae894681d0390907ff25538b58bf0162018b0 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -71,6 +71,8 @@ [LibraryClasses.common]
   PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
   SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
 
+  TrngLib|ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
+
 [LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
   PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
   PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
@@ -102,6 +104,8 @@ [PcdsFeatureFlag.common]
   # Use MMIO for accessing RTC controller registers.
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|TRUE
 
+  gArmTokenSpaceGuid.PcdMonitorConduitHvc|TRUE
+
 [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
 
@@ -353,3 +357,9 @@ [Components.common]
   }
   OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
   OvmfPkg/Virtio10Dxe/Virtio10.inf
+
+  #
+  # Rng Support
+  #
+  SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+
diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
index 14a5fce43a0928d6d78b1af5d7bc3a16b6f07918..ed7e5cfcfad8ef762921de52af66f76351736468 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.fdf
+++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
@@ -211,6 +211,11 @@ [FV.FvMain]
   #
   INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
 
+  #
+  # Rng Support
+  #
+  INF SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+
 [FV.FVMAIN_COMPACT]
 FvAlignment        = 16
 ERASE_POLARITY     = 1
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
  2021-11-16 11:32 ` [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls Sami Mujawar
@ 2021-11-24 12:07   ` Leif Lindholm
  2021-11-24 13:03     ` Ard Biesheuvel
  0 siblings, 1 reply; 19+ messages in thread
From: Leif Lindholm @ 2021-11-24 12:07 UTC (permalink / raw)
  To: Sami Mujawar, ardb+tianocore
  Cc: devel, rebecca, kraxel, michael.d.kinney, gaoliming, zhiguang.liu,
	jiewen.yao, jian.j.wang, Matteo.Carlini, Akanksha.Jain2,
	Ben.Adderson, nd

Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
which reads its conduit out of the DT passed to it by QEMU?

/
    Leif

On Tue, Nov 16, 2021 at 11:32:54 +0000, Sami Mujawar wrote:
> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
> 
> Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
> monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
> meaning the SMC conduit is enabled as default.
> 
> Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
> by virtual firmware implementations.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> 
> Notes:
>     v2:
>      - No code change since v1. Re-sending with V2 series.   [SAMI]
> 
>  ArmPkg/ArmPkg.dec | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
>    # Define if the GICv3 controller should use the GICv2 legacy
>    gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
>  
> +  ## Define the conduit to use for monitor calls.
> +  # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> +  # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> +  gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
> +
>  [PcdsFeatureFlag.ARM]
>    # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
>    # TRUE may be appropriate to fix performance problems if you don't care about
> -- 
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
  2021-11-16 11:32 ` [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library Sami Mujawar
@ 2021-11-24 13:01   ` Leif Lindholm
  2021-11-25 15:23     ` Sami Mujawar
  0 siblings, 1 reply; 19+ messages in thread
From: Leif Lindholm @ 2021-11-24 13:01 UTC (permalink / raw)
  To: devel, sami.mujawar
  Cc: ardb+tianocore, rebecca, kraxel, michael.d.kinney, gaoliming,
	zhiguang.liu, jiewen.yao, jian.j.wang, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

Hi Sami,

On Tue, Nov 16, 2021 at 11:32:55 +0000, Sami Mujawar wrote:
> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
> 
> The Arm True Random Number Generator Firmware, Interface 1.0,
> Platform Design Document
> (https://developer.arm.com/documentation/den0098/latest/)
> defines an interface between an Operating System (OS) executing
> at EL1 and Firmware (FW) exposing a conditioned entropy source
> that is provided by a TRNG back end.
> 
> The conditioned entropy, that is provided by the TRNG FW interface,
> is commonly used to seed deterministic random number generators.
> 
> This patch adds a TrngLib library that implements the Arm TRNG
> firmware interface.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> 
> Notes:
>     v2:
>      - MdePkg\Include\Library\TrngLib.h is base type     [LIMING]
>        library. It can use RETURN_STATUS instead of
>        EFI_STATUS.
>      - Replaced EFI_STATUS with RETURN_STATUS.           [SAMI]
>      - MdePkg\Include\Library\TrngLib.h API parameter    [LIMING]
>        doesn't require CONST. CONST means the value
>        specified by the input pointer will not be
>        changed in API implementation.
>      - Removed the use of constant pointers in the       [SAMI]
>        TRNG API.
> 
>  ArmPkg/ArmPkg.dsc                            |   1 +
>  ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h  |  64 +++
>  ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c   | 483 ++++++++++++++++++++
>  ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf |  34 ++
>  4 files changed, 582 insertions(+)
> 
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index 59fd8f295d4f614cc68ee1021e691f94e279ab81..23df68c5eb53df11de5d96bde4949f3c833c9b2c 100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -156,6 +156,7 @@ [Components.common]
>    ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
>    ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
>  
> +  ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
>    ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
>    ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>    ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..42236e743d972df0df205b1565496afeff5785f3
> --- /dev/null
> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
> @@ -0,0 +1,64 @@
> +/** @file
> +  Arm Firmware TRNG definitions.
> +
> +  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Reference(s):
> +  - [1] Arm True Random Number Generator Firmware, Interface 1.0,
> +        Platform Design Document.
> +        (https://developer.arm.com/documentation/den0098/latest/)
> +
> +  @par Glossary:
> +    - TRNG - True Random Number Generator
> +    - FID  - Function ID
> +**/
> +
> +#ifndef ARM_FW_TRNG_DEFS_H_
> +#define ARM_FW_TRNG_DEFS_H_
> +
> +// Firmware TRNG interface Function IDs
> +#define FID_TRNG_VERSION      0x84000050
> +#define FID_TRNG_FEATURES     0x84000051
> +#define FID_TRNG_GET_UUID     0x84000052
> +#define FID_TRNG_RND_AARCH32  0x84000053
> +#define FID_TRNG_RND_AARCH64  0xC4000053

Do these belong in ArmStdSmc.h?

> +
> +// Firmware TRNG revision mask and shift
> +#define TRNG_REV_MAJOR_MASK   0x7FFF
> +#define TRNG_REV_MINOR_MASK   0xFFFF
> +#define TRNG_REV_MAJOR_SHIFT  16
> +#define TRNG_REV_MINOR_SHIFT  0
> +
> +// Firmware TRNG status codes
> +#define TRNG_STATUS_SUCCESS     (INT32)(0)
> +#define TRNG_NOT_SUPPORTED      (INT32)(-1)
> +#define TRNG_INVALID_PARAMETER  (INT32)(-2)
> +#define TRNG_NO_ENTROPY         (INT32)(-3)

And the rest of the stuff to here, really?

> +#if defined (MDE_CPU_ARM)
> +/** FID to use on AArch32 platform to request entropy.
> +*/
> +#define FID_TRNG_RND        FID_TRNG_RND_AARCH32
> +
> +/** Maximum bits of entropy supported on AArch32.
> +*/
> +#define MAX_ENTROPY_BITS    96
> +#elif defined (MDE_CPU_AARCH64)
> +/** FID to use on AArch64 platform to request entropy.
> +*/
> +#define FID_TRNG_RND        FID_TRNG_RND_AARCH64
> +
> +/** Maximum bits of entropy supported on AArch64.
> +*/
> +#define MAX_ENTROPY_BITS    192
> +#else
> +#error "Firmware TRNG not supported. Unknown chipset."
> +#endif
> +
> +/** Typedef for SMC or HVC arguments.
> +*/
> +typedef ARM_SMC_ARGS  ARM_MONITOR_ARGS;
> +
> +#endif // ARM_FW_TRNG_DEFS_H_
> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..314e7ffbc232ae90bbb77306f9c7113ce63012c8
> --- /dev/null
> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
> @@ -0,0 +1,483 @@
> +/** @file
> +  Arm Firmware TRNG interface library.
> +
> +  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Reference(s):
> +  - [1] Arm True Random Number Generator Firmware, Interface 1.0,
> +        Platform Design Document.
> +        (https://developer.arm.com/documentation/den0098/latest/)
> +  - [2] NIST Special Publication 800-90A Revision 1, June 2015, Recommendation
> +        for Random Number Generation Using Deterministic Random Bit Generators.
> +        (https://csrc.nist.gov/publications/detail/sp/800-90a/rev-1/final)
> +  - [3] NIST Special Publication 800-90B, Recommendation for the Entropy
> +        Sources Used for Random Bit Generation.
> +        (https://csrc.nist.gov/publications/detail/sp/800-90b/final)
> +  - [4] (Second Draft) NIST Special Publication 800-90C, Recommendation for
> +        Random Bit Generator (RBG) Constructions.
> +        (https://csrc.nist.gov/publications/detail/sp/800-90c/draft)
> +
> +  @par Glossary:
> +    - TRNG - True Random Number Generator
> +    - FID  - Function ID
> +**/
> +
> +#include <Base.h>
> +#include <Library/ArmHvcLib.h>
> +#include <Library/ArmLib.h>
> +#include <Library/ArmSmcLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +
> +#include "ArmFwTrngDefs.h"
> +
> +/** Convert TRNG status codes to EFI status codes.
> +
> +  @param [in]  TrngStatus    TRNG status code.
> +
> +  @retval  RETURN_SUCCESS            Success.
> +  @retval  RETURN_UNSUPPORTED        Function not implemented.
> +  @retval  RETURN_INVALID_PARAMETER  A parameter is invalid.
> +  @retval  RETURN_NOT_READY          No Entropy available.
> +**/
> +STATIC
> +RETURN_STATUS
> +TrngStatusToEfiStatus (
> +  IN  INT32   TrngStatus
> +  )
> +{
> +  switch (TrngStatus) {
> +    case TRNG_NOT_SUPPORTED:
> +      return RETURN_UNSUPPORTED;
> +
> +    case TRNG_INVALID_PARAMETER:
> +      return RETURN_INVALID_PARAMETER;
> +
> +    case TRNG_NO_ENTROPY:
> +      return RETURN_NOT_READY;
> +
> +    case TRNG_STATUS_SUCCESS:
> +    default:
> +      return RETURN_SUCCESS;
> +  }
> +}
> +
> +/** Invoke the monitor call using the appropriate conduit.
> +    If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
> +
> +  @param [in, out]  Args    Arguments passed to and returned from the monitor.
> +
> +  @return  VOID
> +**/
> +STATIC
> +VOID
> +ArmCallMonitor (
> +  IN OUT ARM_MONITOR_ARGS   *Args
> +  )
> +{
> +  if (FeaturePcdGet (PcdMonitorConduitHvc)) {
> +    ArmCallHvc ((ARM_HVC_ARGS*)Args);
> +  } else {
> +    ArmCallSmc ((ARM_SMC_ARGS*)Args);
> +  }
> +}

Should this be in (a potentially renamed) ArmSmcLib?

> +
> +/** Get the version of the TRNG backend.
> +
> +  A TRNG may be implemented by the system firmware, in which case this
> +  function shall return the version of the TRNG backend.
> +  The implementation must return NOT_SUPPORTED if a Back end is not present.
> +
> +  @param [out]  MajorRevision     Major revision.
> +  @param [out]  MinorRevision     Minor revision.
> +
> +  @retval  RETURN_SUCCESS            The function completed successfully.
> +  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
> +  @retval  RETURN_UNSUPPORTED        Backend not present.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +GetTrngVersion (
> +  OUT UINT16  *MajorRevision,
> +  OUT UINT16  *MinorRevision
> +  )
> +{
> +  RETURN_STATUS     Status;
> +  ARM_MONITOR_ARGS  Parameters;
> +  INT32             Revision;
> +
> +  if ((MajorRevision == NULL) || (MinorRevision == NULL)) {
> +    return RETURN_INVALID_PARAMETER;
> +  }
> +
> +  ZeroMem (&Parameters, sizeof (Parameters));
> +
> +  /*
> +    Cf. [1], 2.1 TRNG_VERSION
> +    Function ID (W0) 0x8400_0050
> +    Parameters
> +        W1-W7 Reserved (MBZ)
> +    Returns
> +        Success (W0 > 0) W0[31] MBZ
> +        W0[30:16] Major revision
> +        W0[15:0] Minor revision
> +        W1 - W3 Reserved (MBZ)
> +    Error (W0 < 0)
> +        NOT_SUPPORTED Function not implemented
> +  */

I have a comment on the placement of API descriptions further down.

> +  Parameters.Arg0 = FID_TRNG_VERSION;
> +  ArmCallMonitor (&Parameters);
> +
> +  Revision = (INT32)Parameters.Arg0;
> +  // Convert status codes to EFI status codes.
> +  Status = TrngStatusToEfiStatus (Revision);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  *MinorRevision = (Revision & TRNG_REV_MINOR_MASK);
> +  *MajorRevision = ((Revision >> TRNG_REV_MAJOR_SHIFT) & TRNG_REV_MAJOR_MASK);
> +  return RETURN_SUCCESS;
> +}
> +
> +#ifndef MDEPKG_NDEBUG
> +/** Get the features supported by the TRNG backend.
> +
> +  The caller can determine if functions defined in the TRNG ABI are
> +  present in the ABI implementation.
> +
> +  @param [in]  FunctionId         Function Id.
> +  @param [out] Capability         Function specific capability if present
> +                                  otherwise Zero is returned.
> +
> +  @retval  RETURN_SUCCESS            The function completed successfully.
> +  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
> +  @retval  RETURN_UNSUPPORTED        Function not implemented.
> +**/
> +STATIC
> +RETURN_STATUS
> +EFIAPI
> +GetTrngFeatures (
> +  IN  CONST UINT32  FunctionId,
> +  OUT       UINT32  *Capability      OPTIONAL
> +  )
> +{
> +  ARM_MONITOR_ARGS  Parameters;
> +
> +  ZeroMem (&Parameters, sizeof (Parameters));
> +
> +  /*
> +    Cf. [1], Section 2.2 TRNG_FEATURES
> +    Function ID (W0) 0x8400_0051
> +    Parameters
> +        W1 trng_func_id
> +        W2-W7 Reserved (MBZ)
> +    Returns
> +        Success (W0 >= 0)
> +          SUCCESS Function is implemented.
> +            > 0     Function is implemented and
> +                    has specific capabilities,
> +                    see function definition.
> +          Error (W0 < 0)
> +            NOT_SUPPORTED Function with FID=trng_func_id
> +            is not implemented
> +  */

I have a comment on the placement of API descriptions further down.

> +  Parameters.Arg0 = FID_TRNG_FEATURES;
> +  Parameters.Arg1 = FunctionId;
> +  ArmCallMonitor (&Parameters);
> +  if (Parameters.Arg0 < TRNG_STATUS_SUCCESS) {
> +    return RETURN_UNSUPPORTED;
> +  }
> +
> +  if (Capability != NULL) {
> +    *Capability = Parameters.Arg0;
> +  }
> +
> +  return RETURN_SUCCESS;
> +}
> +#endif  //MDEPKG_NDEBUG
> +
> +/** Get the UUID of the TRNG backend.
> +
> +  A TRNG may be implemented by the system firmware, in which case this
> +  function shall return the UUID of the TRNG backend.
> +  Returning the TRNG UUID is optional and if not implemented, RETURN_UNSUPPORTED
> +  shall be returned.
> +
> +  Note: The caller must not rely on the returned UUID as a trustworthy TRNG
> +        Back end identity
> +
> +  @param [out]  Guid              UUID of the TRNG backend.
> +
> +  @retval  RETURN_SUCCESS            The function completed successfully.
> +  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
> +  @retval  RETURN_UNSUPPORTED        Function not implemented.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +GetTrngUuid (
> +  OUT GUID  *Guid
> +  )
> +{
> +  RETURN_STATUS     Status;
> +  ARM_MONITOR_ARGS  Parameters;
> +
> +  ZeroMem (&Parameters, sizeof (Parameters));
> +
> +  /*
> +    Cf. [1], Section 2.3 TRNG_GET_UUID
> +    Function ID (W0) 0x8400_0052
> +    Parameters
> +        W1-W7 Reserved (MBZ)
> +    Returns
> +        Success (W0 != -1)
> +            W0 UUID[31:0]
> +            W1 UUID[63:32]
> +            W2 UUID[95:64]
> +            W3 UUID[127:96]
> +        Error (W0 = -1)
> +            W0 NOT_SUPPORTED
> +  */
> +  Parameters.Arg0 = FID_TRNG_GET_UUID;
> +  ArmCallMonitor (&Parameters);
> +
> +  // Convert status codes to EFI status codes.
> +  Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Guid->Data1 = (Parameters.Arg0 & MAX_UINT32);
> +  Guid->Data2 = (Parameters.Arg1 & MAX_UINT16);
> +  Guid->Data3 = ((Parameters.Arg1 >> 16) & MAX_UINT16);
> +
> +  Guid->Data4[0] = (Parameters.Arg2 & MAX_UINT8);
> +  Guid->Data4[1] = ((Parameters.Arg2 >> 8) & MAX_UINT8);
> +  Guid->Data4[2] = ((Parameters.Arg2 >> 16) & MAX_UINT8);
> +  Guid->Data4[3] = ((Parameters.Arg2 >> 24) & MAX_UINT8);
> +
> +  Guid->Data4[4] = (Parameters.Arg3 & MAX_UINT8);
> +  Guid->Data4[5] = ((Parameters.Arg3 >> 8) & MAX_UINT8);
> +  Guid->Data4[6] = ((Parameters.Arg3 >> 16) & MAX_UINT8);
> +  Guid->Data4[7] = ((Parameters.Arg3 >> 24) & MAX_UINT8);
> +
> +  DEBUG ((DEBUG_INFO, "FW-TRNG: UUID %g\n", Guid));
> +
> +  return RETURN_SUCCESS;
> +}
> +
> +/** Returns maximum number of entropy bits that can be returned in a single
> +    call.
> +
> +  @return Returns the maximum number of Entropy bits that can be returned
> +          in a single call to GetEntropy().
> +**/
> +UINTN
> +EFIAPI
> +GetTrngMaxSupportedEntropyBits (
> +  VOID
> +  )
> +{
> +  return MAX_ENTROPY_BITS;
> +}
> +
> +/** Returns N bits of conditioned entropy.
> +
> +  See [3] Section 2.3.1 GetEntropy: An Interface to the Entropy Source
> +    GetEntropy
> +      Input:
> +        bits_of_entropy: the requested amount of entropy
> +      Output:
> +        entropy_bitstring: The string that provides the requested entropy.
> +      status: A Boolean value that is TRUE if the request has been satisfied,
> +              and is FALSE otherwise.
> +
> +  Note: In this implementation this function returns a status code instead
> +        of a boolean value.
> +        This is also compatible with the definition of Get_Entropy, see [2]
> +        Section 7.4 Entropy Source Calls.
> +          (status, entropy_bitstring) = Get_Entropy (
> +                                          requested_entropy,
> +                                          max_length
> +                                          )
> +
> +  @param  [in]   EntropyBits  Number of entropy bits requested.
> +  @param  [out]  Buffer       Buffer to return the entropy bits.
> +  @param  [in]   BufferSize   Size of the Buffer in bytes.
> +
> +  @retval  RETURN_SUCCESS            The function completed successfully.
> +  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
> +  @retval  RETURN_UNSUPPORTED        Function not implemented.
> +  @retval  RETURN_BAD_BUFFER_SIZE    Buffer size is too small.
> +  @retval  RETURN_NOT_READY          No Entropy available.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +GetEntropy (
> +  IN  CONST UINTN   EntropyBits,
> +  OUT       UINT8   *Buffer,
> +  IN  CONST UINTN   BufferSize
> +  )
> +{
> +  RETURN_STATUS     Status;
> +  ARM_MONITOR_ARGS  Parameters;
> +  UINTN             EntropyBytes;
> +  UINTN             LastValidBits;
> +  UINTN             ArgSelector;
> +  UINTN             BytesToClear;
> +
> +  // [1] Section 2.4.3 Caller responsibilities.
> +  // The caller cannot request more than MAX_BITS bits of conditioned
> +  // entropy per call.

Comment is redundant, code is clearer without it.

> +  if ((EntropyBits == 0) || (EntropyBits > MAX_ENTROPY_BITS)) {
> +    return RETURN_INVALID_PARAMETER;
> +  }
> +
> +  EntropyBytes = (EntropyBits + 7) >> 3;
> +  if (EntropyBytes > BufferSize) {

Not for later: we're verifying the value of EntropyBytes here - if
there are more aspects of it that need verifying, that should also be
done here.

> +    return RETURN_BAD_BUFFER_SIZE;
> +  }
> +
> +  ZeroMem (Buffer, BufferSize);
> +  ZeroMem (&Parameters, sizeof (Parameters));
> +
> +  /*
> +    Cf. [1], Section 2.4 TRNG_RND
> +    Function ID (W0)  0x8400_0053
> +                      0xC400_0053
> +    SMC32 Parameters
> +        W1    N bits of entropy (1 6 N 6 96)
> +        W2-W7 Reserved (MBZ)
> +    SMC64 Parameters
> +        X1    N bits of entropy (1 6 N 6 192)
> +        X2-X7 Reserved (MBZ)
> +    SMC32 Returns
> +        Success (W0 = 0):
> +          W0 MBZ
> +          W1 Entropy[95:64]
> +          W2 Entropy[63:32]
> +          W3 Entropy[31:0]
> +    Error (W0 < 0)
> +          W0 NOT_SUPPORTED
> +          NO_ENTROPY
> +          INVALID_PARAMETERS
> +          W1 - W3 Reserved (MBZ)
> +    SMC64 Returns
> +          Success (X0 = 0):
> +          X0 MBZ
> +          X1 Entropy[191:128]
> +          X2 Entropy[127:64]
> +          X3 Entropy[63:0]
> +    Error (X0 < 0)
> +          X0 NOT_SUPPORTED
> +          NO_ENTROPY
> +          INVALID_PARAMETERS
> +          X1 - X3 Reserved (MBZ)
> +  */

The above comment block completely wrecks the readability of the
function.

Would suggest putting it in the header file describing the monitor
call. For our SIP SVC calls, we've done this in the following form:

/*
 * SMC call to retrieve number of CPUs present in the system.
 * Input values:
 *   x0: NUVIA_SIP_GET_NUM_CPUS
 * Return values:
 *   x0: SMC_OK
 *   x1: Number of CPUs present
 */
#define NUVIA_SIP_GET_NUM_CPUS   SIP_FUNCTION_ID(0x20)

(Where SIP_FUNCTION_ID is one of a set of macros I should submit for
addition to ArmStdSmc.h)

> +  Parameters.Arg0 = FID_TRNG_RND;
> +  Parameters.Arg1 = EntropyBits;
> +  ArmCallMonitor (&Parameters);
> +
> +  // Convert status codes to EFI status codes.

Function name already says this, comment redundant.

> +  Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +

>From here

> +  // Extract Data
> +  // ArgSelector = ((EntropyBytes + 3) >> 2); for AArch32
> +  // ArgSelector = ((EntropyBytes + 7) >> 3); for AArch64
> +  // ((sizeof (UINTN) >> 2) + 1) is 3 or 2 depending on size of UINTN
> +  ArgSelector = ((EntropyBytes + (sizeof (UINTN) - 1)) >>
> +                 ((sizeof (UINTN) >> 2) + 1));
> +
> +  switch (ArgSelector) {
> +    case 3:
> +      CopyMem (&Buffer[(sizeof (UINTN) * 2)], &Parameters.Arg1, sizeof (UINTN));
> +
> +    case 2:
> +      CopyMem (&Buffer[sizeof (UINTN)], &Parameters.Arg2, sizeof (UINTN));
> +
> +    case 1:
> +      CopyMem (&Buffer[0], &Parameters.Arg3, sizeof (UINTN));
> +      break;
> +
> +    default:
> +      ASSERT (0);
> +      return RETURN_INVALID_PARAMETER;
> +  } // switch

to here ... I'm not convinced you yourself would be able to read or
explain this code a few months down the line.

Is there a strong reason for why Buffer cannot be a UINTN *?

I think what this code is doing can equally be written as:

  Buffer[0] = Parameters.Arg3;
  if ((EntropyBytes / sizeof (UINTN)) > 1) {
    Buffer[1] = Parameters.Arg2;
  }
  if ((EntropyBytes / sizeof (UINTN)) > 2) {
    Buffer[2] = Parameters.Arg1;
  }

> +
> +
> +  // [1] Section 2.4.3 Caller responsibilities.
> +  // The caller must ensure that only the value in Entropy[N-1:0] is consumed
> +  // and that the remaining bits in Entropy[MAX_BITS-1:N] are ignored.
> +  // Therefore, Clear the unused upper bytes.

This is source code, not the specification.

  // Mask off any unused top bytes, in accordance with specification

is sufficient as a comment.

/
    Leif

> +  BytesToClear = (sizeof (UINTN) * ArgSelector) - EntropyBytes;
> +  if (BytesToClear != 0) {
> +    ZeroMem (&Buffer[EntropyBytes], BytesToClear);
> +  }
> +
> +  // Clear the unused MSB bits of the last byte.
> +  LastValidBits = EntropyBits & 0x7;
> +  if (LastValidBits != 0) {
> +    Buffer[EntropyBytes - 1] &= (0xFF >> (8 - LastValidBits));
> +  }
> +
> +  return Status;
> +}
> +
> +/** The constructor checks that the FW-TRNG interface is supported
> +    by the host firmware.
> +
> +  It will ASSERT() if FW-TRNG is not supported.
> +  It will always return RETURN_SUCCESS.
> +
> +  @retval RETURN_SUCCESS   The constructor always returns RETURN_SUCCESS.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +ArmFwTrngLibConstructor (
> +  VOID
> +  )
> +{
> +  RETURN_STATUS Status;
> +  UINT16        MajorRev;
> +  UINT16        MinorRev;
> +  GUID          Guid;
> +
> +  Status = GetTrngVersion (&MajorRev, &MinorRev);
> +  if (EFI_ERROR (Status)) {
> +    return RETURN_SUCCESS;
> +  }
> +
> +#ifndef MDEPKG_NDEBUG
> +  // Check that the required features are present.
> +  Status = GetTrngFeatures (FID_TRNG_RND, NULL);
> +  if (EFI_ERROR (Status)) {
> +    return RETURN_SUCCESS;
> +  }
> +
> +  // Check if TRNG UUID is supported and if so trace the GUID.
> +  Status = GetTrngFeatures (FID_TRNG_GET_UUID, NULL);
> +  if (EFI_ERROR (Status)) {
> +    return RETURN_SUCCESS;
> +  }
> +#endif
> +
> +  Status = GetTrngUuid (&Guid);
> +  if (EFI_ERROR (Status)) {
> +    return RETURN_SUCCESS;
> +  }
> +
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "FW-TRNG: Version %d.%d, GUID {%g}\n",
> +    MajorRev,
> +    MinorRev,
> +    Guid
> +    ));
> +
> +  return RETURN_SUCCESS;
> +}
> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
> new file mode 100644
> index 0000000000000000000000000000000000000000..4b2c58251fbe8fbcb5af308736db014e8d954720
> --- /dev/null
> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
> @@ -0,0 +1,34 @@
> +## @file
> +#  Arm Firmware TRNG interface library.
> +#
> +#  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION       = 0x0001001B
> +  BASE_NAME         = ArmFwTrngLib
> +  FILE_GUID         = 10DE97C9-28E4-4C9B-A53E-8D7D1B0DD4E0
> +  VERSION_STRING    = 1.0
> +  MODULE_TYPE       = BASE
> +  LIBRARY_CLASS     = TrngLib
> +  CONSTRUCTOR       = ArmFwTrngLibConstructor
> +
> +[Sources]
> +  ArmFwTrngDefs.h
> +  ArmFwTrngLib.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  ArmSmcLib
> +  ArmHvcLib
> +  BaseLib
> +  BaseMemoryLib
> +
> +[Pcd]
> +  gArmTokenSpaceGuid.PcdMonitorConduitHvc
> +
> -- 
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
  2021-11-24 12:07   ` Leif Lindholm
@ 2021-11-24 13:03     ` Ard Biesheuvel
  2021-11-24 13:05       ` Leif Lindholm
  0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2021-11-24 13:03 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: Sami Mujawar, Ard Biesheuvel, edk2-devel-groups-io, Rebecca Cran,
	Gerd Hoffmann, Michael Kinney, Liming Gao (Byosoft address),
	Zhiguang Liu, Jiewen Yao, Jian J Wang, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

On Wed, 24 Nov 2021 at 13:07, Leif Lindholm <leif@nuviainc.com> wrote:
>
> Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> which reads its conduit out of the DT passed to it by QEMU?
>

As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
as before, I don't think those libraries are affected.

>
> On Tue, Nov 16, 2021 at 11:32:54 +0000, Sami Mujawar wrote:
> > Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
> >
> > Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
> > monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
> > meaning the SMC conduit is enabled as default.
> >
> > Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
> > by virtual firmware implementations.
> >
> > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > ---
> >
> > Notes:
> >     v2:
> >      - No code change since v1. Re-sending with V2 series.   [SAMI]
> >
> >  ArmPkg/ArmPkg.dec | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> > index 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87 100644
> > --- a/ArmPkg/ArmPkg.dec
> > +++ b/ArmPkg/ArmPkg.dec
> > @@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
> >    # Define if the GICv3 controller should use the GICv2 legacy
> >    gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
> >
> > +  ## Define the conduit to use for monitor calls.
> > +  # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> > +  # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> > +  gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
> > +
> >  [PcdsFeatureFlag.ARM]
> >    # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
> >    # TRUE may be appropriate to fix performance problems if you don't care about
> > --
> > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
  2021-11-24 13:03     ` Ard Biesheuvel
@ 2021-11-24 13:05       ` Leif Lindholm
  2021-11-24 13:07         ` Ard Biesheuvel
  0 siblings, 1 reply; 19+ messages in thread
From: Leif Lindholm @ 2021-11-24 13:05 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Sami Mujawar, Ard Biesheuvel, edk2-devel-groups-io, Rebecca Cran,
	Gerd Hoffmann, Michael Kinney, Liming Gao (Byosoft address),
	Zhiguang Liu, Jiewen Yao, Jian J Wang, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

On Wed, Nov 24, 2021 at 14:03:51 +0100, Ard Biesheuvel wrote:
> On Wed, 24 Nov 2021 at 13:07, Leif Lindholm <leif@nuviainc.com> wrote:
> >
> > Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> > which reads its conduit out of the DT passed to it by QEMU?
> >
> 
> As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
> as before, I don't think those libraries are affected.

Well, I was thinking more that you can now end up with a firmware
build where the conduit is sometimes dynamically determined, and
sometimes compile-time determined.

/
    Leif

> > On Tue, Nov 16, 2021 at 11:32:54 +0000, Sami Mujawar wrote:
> > > Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
> > >
> > > Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
> > > monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
> > > meaning the SMC conduit is enabled as default.
> > >
> > > Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
> > > by virtual firmware implementations.
> > >
> > > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > > ---
> > >
> > > Notes:
> > >     v2:
> > >      - No code change since v1. Re-sending with V2 series.   [SAMI]
> > >
> > >  ArmPkg/ArmPkg.dec | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> > > index 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87 100644
> > > --- a/ArmPkg/ArmPkg.dec
> > > +++ b/ArmPkg/ArmPkg.dec
> > > @@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
> > >    # Define if the GICv3 controller should use the GICv2 legacy
> > >    gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
> > >
> > > +  ## Define the conduit to use for monitor calls.
> > > +  # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> > > +  # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> > > +  gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
> > > +
> > >  [PcdsFeatureFlag.ARM]
> > >    # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
> > >    # TRUE may be appropriate to fix performance problems if you don't care about
> > > --
> > > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> > >

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
  2021-11-24 13:05       ` Leif Lindholm
@ 2021-11-24 13:07         ` Ard Biesheuvel
  2021-11-24 13:25           ` Leif Lindholm
  0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2021-11-24 13:07 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: Sami Mujawar, Ard Biesheuvel, edk2-devel-groups-io, Rebecca Cran,
	Gerd Hoffmann, Michael Kinney, Liming Gao (Byosoft address),
	Zhiguang Liu, Jiewen Yao, Jian J Wang, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

On Wed, 24 Nov 2021 at 14:05, Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Wed, Nov 24, 2021 at 14:03:51 +0100, Ard Biesheuvel wrote:
> > On Wed, 24 Nov 2021 at 13:07, Leif Lindholm <leif@nuviainc.com> wrote:
> > >
> > > Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> > > which reads its conduit out of the DT passed to it by QEMU?
> > >
> >
> > As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
> > as before, I don't think those libraries are affected.
>
> Well, I was thinking more that you can now end up with a firmware
> build where the conduit is sometimes dynamically determined, and
> sometimes compile-time determined.
>

There is nothing wrong with that by itself: if you have a DT that
describes this, it makes sense to carry support for both. If the bare
metal you are targetting only ever uses SMC, no need for the discovery
and a PCD is fine.

Question remains whether this PCD could/should be configurable as
dynamic, so we collapse the static/dynamic options into a single PCD
based choice.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
  2021-11-24 13:07         ` Ard Biesheuvel
@ 2021-11-24 13:25           ` Leif Lindholm
  0 siblings, 0 replies; 19+ messages in thread
From: Leif Lindholm @ 2021-11-24 13:25 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Sami Mujawar, Ard Biesheuvel, edk2-devel-groups-io, Rebecca Cran,
	Gerd Hoffmann, Michael Kinney, Liming Gao (Byosoft address),
	Zhiguang Liu, Jiewen Yao, Jian J Wang, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

On Wed, Nov 24, 2021 at 14:07:22 +0100, Ard Biesheuvel wrote:
> On Wed, 24 Nov 2021 at 14:05, Leif Lindholm <leif@nuviainc.com> wrote:
> >
> > On Wed, Nov 24, 2021 at 14:03:51 +0100, Ard Biesheuvel wrote:
> > > On Wed, 24 Nov 2021 at 13:07, Leif Lindholm <leif@nuviainc.com> wrote:
> > > >
> > > > Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> > > > which reads its conduit out of the DT passed to it by QEMU?
> > > >
> > >
> > > As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
> > > as before, I don't think those libraries are affected.
> >
> > Well, I was thinking more that you can now end up with a firmware
> > build where the conduit is sometimes dynamically determined, and
> > sometimes compile-time determined.
> 
> There is nothing wrong with that by itself: if you have a DT that
> describes this, it makes sense to carry support for both. If the bare
> metal you are targetting only ever uses SMC, no need for the discovery
> and a PCD is fine.
> 
> Question remains whether this PCD could/should be configurable as
> dynamic, so we collapse the static/dynamic options into a single PCD
> based choice.

That would feel cleaner. Although not required for this set.

/
    Leif

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
  2021-11-24 13:01   ` [edk2-devel] " Leif Lindholm
@ 2021-11-25 15:23     ` Sami Mujawar
  2022-03-24  9:46       ` PierreGondois
       [not found]       ` <80941d66-5d31-053f-388a-95efe5dbbfdf@arm.com>
  0 siblings, 2 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-25 15:23 UTC (permalink / raw)
  To: Leif Lindholm, devel
  Cc: ardb+tianocore, rebecca, kraxel, michael.d.kinney, gaoliming,
	zhiguang.liu, jiewen.yao, jian.j.wang, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

Hi Leif,

Thank you for the feedback.

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar


On 24/11/2021 01:01 PM, Leif Lindholm wrote:
> Hi Sami,
>
> On Tue, Nov 16, 2021 at 11:32:55 +0000, Sami Mujawar wrote:
>> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
>>
>> The Arm True Random Number Generator Firmware, Interface 1.0,
>> Platform Design Document
>> (https://developer.arm.com/documentation/den0098/latest/)
>> defines an interface between an Operating System (OS) executing
>> at EL1 and Firmware (FW) exposing a conditioned entropy source
>> that is provided by a TRNG back end.
>>
>> The conditioned entropy, that is provided by the TRNG FW interface,
>> is commonly used to seed deterministic random number generators.
>>
>> This patch adds a TrngLib library that implements the Arm TRNG
>> firmware interface.
>>
>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>> ---
>>
>> Notes:
>>      v2:
>>       - MdePkg\Include\Library\TrngLib.h is base type     [LIMING]
>>         library. It can use RETURN_STATUS instead of
>>         EFI_STATUS.
>>       - Replaced EFI_STATUS with RETURN_STATUS.           [SAMI]
>>       - MdePkg\Include\Library\TrngLib.h API parameter    [LIMING]
>>         doesn't require CONST. CONST means the value
>>         specified by the input pointer will not be
>>         changed in API implementation.
>>       - Removed the use of constant pointers in the       [SAMI]
>>         TRNG API.
>>
>>   ArmPkg/ArmPkg.dsc                            |   1 +
>>   ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h  |  64 +++
>>   ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c   | 483 ++++++++++++++++++++
>>   ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf |  34 ++
>>   4 files changed, 582 insertions(+)
>>
>> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
>> index 59fd8f295d4f614cc68ee1021e691f94e279ab81..23df68c5eb53df11de5d96bde4949f3c833c9b2c 100644
>> --- a/ArmPkg/ArmPkg.dsc
>> +++ b/ArmPkg/ArmPkg.dsc
>> @@ -156,6 +156,7 @@ [Components.common]
>>     ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
>>     ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
>>   
>> +  ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
>>     ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
>>     ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>>     ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
>> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..42236e743d972df0df205b1565496afeff5785f3
>> --- /dev/null
>> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
>> @@ -0,0 +1,64 @@
>> +/** @file
>> +  Arm Firmware TRNG definitions.
>> +
>> +  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +  @par Reference(s):
>> +  - [1] Arm True Random Number Generator Firmware, Interface 1.0,
>> +        Platform Design Document.
>> +        (https://developer.arm.com/documentation/den0098/latest/)
>> +
>> +  @par Glossary:
>> +    - TRNG - True Random Number Generator
>> +    - FID  - Function ID
>> +**/
>> +
>> +#ifndef ARM_FW_TRNG_DEFS_H_
>> +#define ARM_FW_TRNG_DEFS_H_
>> +
>> +// Firmware TRNG interface Function IDs
>> +#define FID_TRNG_VERSION      0x84000050
>> +#define FID_TRNG_FEATURES     0x84000051
>> +#define FID_TRNG_GET_UUID     0x84000052
>> +#define FID_TRNG_RND_AARCH32  0x84000053
>> +#define FID_TRNG_RND_AARCH64  0xC4000053
> Do these belong in ArmStdSmc.h?
[SAMI] I will fix this in the next version.
>
>> +
>> +// Firmware TRNG revision mask and shift
>> +#define TRNG_REV_MAJOR_MASK   0x7FFF
>> +#define TRNG_REV_MINOR_MASK   0xFFFF
>> +#define TRNG_REV_MAJOR_SHIFT  16
>> +#define TRNG_REV_MINOR_SHIFT  0
>> +
>> +// Firmware TRNG status codes
>> +#define TRNG_STATUS_SUCCESS     (INT32)(0)
>> +#define TRNG_NOT_SUPPORTED      (INT32)(-1)
>> +#define TRNG_INVALID_PARAMETER  (INT32)(-2)
>> +#define TRNG_NO_ENTROPY         (INT32)(-3)
> And the rest of the stuff to here, really?
[SAMI] I will fix this in the next version.
>
>> +#if defined (MDE_CPU_ARM)
>> +/** FID to use on AArch32 platform to request entropy.
>> +*/
>> +#define FID_TRNG_RND        FID_TRNG_RND_AARCH32
>> +
>> +/** Maximum bits of entropy supported on AArch32.
>> +*/
>> +#define MAX_ENTROPY_BITS    96
>> +#elif defined (MDE_CPU_AARCH64)
>> +/** FID to use on AArch64 platform to request entropy.
>> +*/
>> +#define FID_TRNG_RND        FID_TRNG_RND_AARCH64
>> +
>> +/** Maximum bits of entropy supported on AArch64.
>> +*/
>> +#define MAX_ENTROPY_BITS    192
>> +#else
>> +#error "Firmware TRNG not supported. Unknown chipset."
>> +#endif
>> +
>> +/** Typedef for SMC or HVC arguments.
>> +*/
>> +typedef ARM_SMC_ARGS  ARM_MONITOR_ARGS;
>> +
>> +#endif // ARM_FW_TRNG_DEFS_H_
>> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..314e7ffbc232ae90bbb77306f9c7113ce63012c8
>> --- /dev/null
>> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
>> @@ -0,0 +1,483 @@
>> +/** @file
>> +  Arm Firmware TRNG interface library.
>> +
>> +  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +  @par Reference(s):
>> +  - [1] Arm True Random Number Generator Firmware, Interface 1.0,
>> +        Platform Design Document.
>> +        (https://developer.arm.com/documentation/den0098/latest/)
>> +  - [2] NIST Special Publication 800-90A Revision 1, June 2015, Recommendation
>> +        for Random Number Generation Using Deterministic Random Bit Generators.
>> +        (https://csrc.nist.gov/publications/detail/sp/800-90a/rev-1/final)
>> +  - [3] NIST Special Publication 800-90B, Recommendation for the Entropy
>> +        Sources Used for Random Bit Generation.
>> +        (https://csrc.nist.gov/publications/detail/sp/800-90b/final)
>> +  - [4] (Second Draft) NIST Special Publication 800-90C, Recommendation for
>> +        Random Bit Generator (RBG) Constructions.
>> +        (https://csrc.nist.gov/publications/detail/sp/800-90c/draft)
>> +
>> +  @par Glossary:
>> +    - TRNG - True Random Number Generator
>> +    - FID  - Function ID
>> +**/
>> +
>> +#include <Base.h>
>> +#include <Library/ArmHvcLib.h>
>> +#include <Library/ArmLib.h>
>> +#include <Library/ArmSmcLib.h>
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/DebugLib.h>
>> +
>> +#include "ArmFwTrngDefs.h"
>> +
>> +/** Convert TRNG status codes to EFI status codes.
>> +
>> +  @param [in]  TrngStatus    TRNG status code.
>> +
>> +  @retval  RETURN_SUCCESS            Success.
>> +  @retval  RETURN_UNSUPPORTED        Function not implemented.
>> +  @retval  RETURN_INVALID_PARAMETER  A parameter is invalid.
>> +  @retval  RETURN_NOT_READY          No Entropy available.
>> +**/
>> +STATIC
>> +RETURN_STATUS
>> +TrngStatusToEfiStatus (
>> +  IN  INT32   TrngStatus
>> +  )
>> +{
>> +  switch (TrngStatus) {
>> +    case TRNG_NOT_SUPPORTED:
>> +      return RETURN_UNSUPPORTED;
>> +
>> +    case TRNG_INVALID_PARAMETER:
>> +      return RETURN_INVALID_PARAMETER;
>> +
>> +    case TRNG_NO_ENTROPY:
>> +      return RETURN_NOT_READY;
>> +
>> +    case TRNG_STATUS_SUCCESS:
>> +    default:
>> +      return RETURN_SUCCESS;
>> +  }
>> +}
>> +
>> +/** Invoke the monitor call using the appropriate conduit.
>> +    If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
>> +
>> +  @param [in, out]  Args    Arguments passed to and returned from the monitor.
>> +
>> +  @return  VOID
>> +**/
>> +STATIC
>> +VOID
>> +ArmCallMonitor (
>> +  IN OUT ARM_MONITOR_ARGS   *Args
>> +  )
>> +{
>> +  if (FeaturePcdGet (PcdMonitorConduitHvc)) {
>> +    ArmCallHvc ((ARM_HVC_ARGS*)Args);
>> +  } else {
>> +    ArmCallSmc ((ARM_SMC_ARGS*)Args);
>> +  }
>> +}
> Should this be in (a potentially renamed) ArmSmcLib?
[SAMI] Looking at ArmSmcLib and ArmHvcLib libraries there is not much 
difference in the code other than the SMC/HVC call. Please let me know 
if I should submit a patch to unify these in ArmMonitorLib?
The ArmCall<Smc|Hvc> APIs would still remain the same but moved to 
ArmMonitorLib.
>
>> +
>> +/** Get the version of the TRNG backend.
>> +
>> +  A TRNG may be implemented by the system firmware, in which case this
>> +  function shall return the version of the TRNG backend.
>> +  The implementation must return NOT_SUPPORTED if a Back end is not present.
>> +
>> +  @param [out]  MajorRevision     Major revision.
>> +  @param [out]  MinorRevision     Minor revision.
>> +
>> +  @retval  RETURN_SUCCESS            The function completed successfully.
>> +  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
>> +  @retval  RETURN_UNSUPPORTED        Backend not present.
>> +**/
>> +RETURN_STATUS
>> +EFIAPI
>> +GetTrngVersion (
>> +  OUT UINT16  *MajorRevision,
>> +  OUT UINT16  *MinorRevision
>> +  )
>> +{
>> +  RETURN_STATUS     Status;
>> +  ARM_MONITOR_ARGS  Parameters;
>> +  INT32             Revision;
>> +
>> +  if ((MajorRevision == NULL) || (MinorRevision == NULL)) {
>> +    return RETURN_INVALID_PARAMETER;
>> +  }
>> +
>> +  ZeroMem (&Parameters, sizeof (Parameters));
>> +
>> +  /*
>> +    Cf. [1], 2.1 TRNG_VERSION
>> +    Function ID (W0) 0x8400_0050
>> +    Parameters
>> +        W1-W7 Reserved (MBZ)
>> +    Returns
>> +        Success (W0 > 0) W0[31] MBZ
>> +        W0[30:16] Major revision
>> +        W0[15:0] Minor revision
>> +        W1 - W3 Reserved (MBZ)
>> +    Error (W0 < 0)
>> +        NOT_SUPPORTED Function not implemented
>> +  */
> I have a comment on the placement of API descriptions further down.
>
>> +  Parameters.Arg0 = FID_TRNG_VERSION;
>> +  ArmCallMonitor (&Parameters);
>> +
>> +  Revision = (INT32)Parameters.Arg0;
>> +  // Convert status codes to EFI status codes.
>> +  Status = TrngStatusToEfiStatus (Revision);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  *MinorRevision = (Revision & TRNG_REV_MINOR_MASK);
>> +  *MajorRevision = ((Revision >> TRNG_REV_MAJOR_SHIFT) & TRNG_REV_MAJOR_MASK);
>> +  return RETURN_SUCCESS;
>> +}
>> +
>> +#ifndef MDEPKG_NDEBUG
>> +/** Get the features supported by the TRNG backend.
>> +
>> +  The caller can determine if functions defined in the TRNG ABI are
>> +  present in the ABI implementation.
>> +
>> +  @param [in]  FunctionId         Function Id.
>> +  @param [out] Capability         Function specific capability if present
>> +                                  otherwise Zero is returned.
>> +
>> +  @retval  RETURN_SUCCESS            The function completed successfully.
>> +  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
>> +  @retval  RETURN_UNSUPPORTED        Function not implemented.
>> +**/
>> +STATIC
>> +RETURN_STATUS
>> +EFIAPI
>> +GetTrngFeatures (
>> +  IN  CONST UINT32  FunctionId,
>> +  OUT       UINT32  *Capability      OPTIONAL
>> +  )
>> +{
>> +  ARM_MONITOR_ARGS  Parameters;
>> +
>> +  ZeroMem (&Parameters, sizeof (Parameters));
>> +
>> +  /*
>> +    Cf. [1], Section 2.2 TRNG_FEATURES
>> +    Function ID (W0) 0x8400_0051
>> +    Parameters
>> +        W1 trng_func_id
>> +        W2-W7 Reserved (MBZ)
>> +    Returns
>> +        Success (W0 >= 0)
>> +          SUCCESS Function is implemented.
>> +            > 0     Function is implemented and
>> +                    has specific capabilities,
>> +                    see function definition.
>> +          Error (W0 < 0)
>> +            NOT_SUPPORTED Function with FID=trng_func_id
>> +            is not implemented
>> +  */
> I have a comment on the placement of API descriptions further down.
>
>> +  Parameters.Arg0 = FID_TRNG_FEATURES;
>> +  Parameters.Arg1 = FunctionId;
>> +  ArmCallMonitor (&Parameters);
>> +  if (Parameters.Arg0 < TRNG_STATUS_SUCCESS) {
>> +    return RETURN_UNSUPPORTED;
>> +  }
>> +
>> +  if (Capability != NULL) {
>> +    *Capability = Parameters.Arg0;
>> +  }
>> +
>> +  return RETURN_SUCCESS;
>> +}
>> +#endif  //MDEPKG_NDEBUG
>> +
>> +/** Get the UUID of the TRNG backend.
>> +
>> +  A TRNG may be implemented by the system firmware, in which case this
>> +  function shall return the UUID of the TRNG backend.
>> +  Returning the TRNG UUID is optional and if not implemented, RETURN_UNSUPPORTED
>> +  shall be returned.
>> +
>> +  Note: The caller must not rely on the returned UUID as a trustworthy TRNG
>> +        Back end identity
>> +
>> +  @param [out]  Guid              UUID of the TRNG backend.
>> +
>> +  @retval  RETURN_SUCCESS            The function completed successfully.
>> +  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
>> +  @retval  RETURN_UNSUPPORTED        Function not implemented.
>> +**/
>> +RETURN_STATUS
>> +EFIAPI
>> +GetTrngUuid (
>> +  OUT GUID  *Guid
>> +  )
>> +{
>> +  RETURN_STATUS     Status;
>> +  ARM_MONITOR_ARGS  Parameters;
>> +
>> +  ZeroMem (&Parameters, sizeof (Parameters));
>> +
>> +  /*
>> +    Cf. [1], Section 2.3 TRNG_GET_UUID
>> +    Function ID (W0) 0x8400_0052
>> +    Parameters
>> +        W1-W7 Reserved (MBZ)
>> +    Returns
>> +        Success (W0 != -1)
>> +            W0 UUID[31:0]
>> +            W1 UUID[63:32]
>> +            W2 UUID[95:64]
>> +            W3 UUID[127:96]
>> +        Error (W0 = -1)
>> +            W0 NOT_SUPPORTED
>> +  */
>> +  Parameters.Arg0 = FID_TRNG_GET_UUID;
>> +  ArmCallMonitor (&Parameters);
>> +
>> +  // Convert status codes to EFI status codes.
>> +  Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  Guid->Data1 = (Parameters.Arg0 & MAX_UINT32);
>> +  Guid->Data2 = (Parameters.Arg1 & MAX_UINT16);
>> +  Guid->Data3 = ((Parameters.Arg1 >> 16) & MAX_UINT16);
>> +
>> +  Guid->Data4[0] = (Parameters.Arg2 & MAX_UINT8);
>> +  Guid->Data4[1] = ((Parameters.Arg2 >> 8) & MAX_UINT8);
>> +  Guid->Data4[2] = ((Parameters.Arg2 >> 16) & MAX_UINT8);
>> +  Guid->Data4[3] = ((Parameters.Arg2 >> 24) & MAX_UINT8);
>> +
>> +  Guid->Data4[4] = (Parameters.Arg3 & MAX_UINT8);
>> +  Guid->Data4[5] = ((Parameters.Arg3 >> 8) & MAX_UINT8);
>> +  Guid->Data4[6] = ((Parameters.Arg3 >> 16) & MAX_UINT8);
>> +  Guid->Data4[7] = ((Parameters.Arg3 >> 24) & MAX_UINT8);
>> +
>> +  DEBUG ((DEBUG_INFO, "FW-TRNG: UUID %g\n", Guid));
>> +
>> +  return RETURN_SUCCESS;
>> +}
>> +
>> +/** Returns maximum number of entropy bits that can be returned in a single
>> +    call.
>> +
>> +  @return Returns the maximum number of Entropy bits that can be returned
>> +          in a single call to GetEntropy().
>> +**/
>> +UINTN
>> +EFIAPI
>> +GetTrngMaxSupportedEntropyBits (
>> +  VOID
>> +  )
>> +{
>> +  return MAX_ENTROPY_BITS;
>> +}
>> +
>> +/** Returns N bits of conditioned entropy.
>> +
>> +  See [3] Section 2.3.1 GetEntropy: An Interface to the Entropy Source
>> +    GetEntropy
>> +      Input:
>> +        bits_of_entropy: the requested amount of entropy
>> +      Output:
>> +        entropy_bitstring: The string that provides the requested entropy.
>> +      status: A Boolean value that is TRUE if the request has been satisfied,
>> +              and is FALSE otherwise.
>> +
>> +  Note: In this implementation this function returns a status code instead
>> +        of a boolean value.
>> +        This is also compatible with the definition of Get_Entropy, see [2]
>> +        Section 7.4 Entropy Source Calls.
>> +          (status, entropy_bitstring) = Get_Entropy (
>> +                                          requested_entropy,
>> +                                          max_length
>> +                                          )
>> +
>> +  @param  [in]   EntropyBits  Number of entropy bits requested.
>> +  @param  [out]  Buffer       Buffer to return the entropy bits.
>> +  @param  [in]   BufferSize   Size of the Buffer in bytes.
>> +
>> +  @retval  RETURN_SUCCESS            The function completed successfully.
>> +  @retval  RETURN_INVALID_PARAMETER  Invalid parameter.
>> +  @retval  RETURN_UNSUPPORTED        Function not implemented.
>> +  @retval  RETURN_BAD_BUFFER_SIZE    Buffer size is too small.
>> +  @retval  RETURN_NOT_READY          No Entropy available.
>> +**/
>> +RETURN_STATUS
>> +EFIAPI
>> +GetEntropy (
>> +  IN  CONST UINTN   EntropyBits,
>> +  OUT       UINT8   *Buffer,
>> +  IN  CONST UINTN   BufferSize
>> +  )
>> +{
>> +  RETURN_STATUS     Status;
>> +  ARM_MONITOR_ARGS  Parameters;
>> +  UINTN             EntropyBytes;
>> +  UINTN             LastValidBits;
>> +  UINTN             ArgSelector;
>> +  UINTN             BytesToClear;
>> +
>> +  // [1] Section 2.4.3 Caller responsibilities.
>> +  // The caller cannot request more than MAX_BITS bits of conditioned
>> +  // entropy per call.
> Comment is redundant, code is clearer without it.
>
>> +  if ((EntropyBits == 0) || (EntropyBits > MAX_ENTROPY_BITS)) {
>> +    return RETURN_INVALID_PARAMETER;
>> +  }
>> +
>> +  EntropyBytes = (EntropyBits + 7) >> 3;
>> +  if (EntropyBytes > BufferSize) {
> Not for later: we're verifying the value of EntropyBytes here - if
> there are more aspects of it that need verifying, that should also be
> done here.
>
>> +    return RETURN_BAD_BUFFER_SIZE;
>> +  }
>> +
>> +  ZeroMem (Buffer, BufferSize);
>> +  ZeroMem (&Parameters, sizeof (Parameters));
>> +
>> +  /*
>> +    Cf. [1], Section 2.4 TRNG_RND
>> +    Function ID (W0)  0x8400_0053
>> +                      0xC400_0053
>> +    SMC32 Parameters
>> +        W1    N bits of entropy (1 6 N 6 96)
>> +        W2-W7 Reserved (MBZ)
>> +    SMC64 Parameters
>> +        X1    N bits of entropy (1 6 N 6 192)
>> +        X2-X7 Reserved (MBZ)
>> +    SMC32 Returns
>> +        Success (W0 = 0):
>> +          W0 MBZ
>> +          W1 Entropy[95:64]
>> +          W2 Entropy[63:32]
>> +          W3 Entropy[31:0]
>> +    Error (W0 < 0)
>> +          W0 NOT_SUPPORTED
>> +          NO_ENTROPY
>> +          INVALID_PARAMETERS
>> +          W1 - W3 Reserved (MBZ)
>> +    SMC64 Returns
>> +          Success (X0 = 0):
>> +          X0 MBZ
>> +          X1 Entropy[191:128]
>> +          X2 Entropy[127:64]
>> +          X3 Entropy[63:0]
>> +    Error (X0 < 0)
>> +          X0 NOT_SUPPORTED
>> +          NO_ENTROPY
>> +          INVALID_PARAMETERS
>> +          X1 - X3 Reserved (MBZ)
>> +  */
> The above comment block completely wrecks the readability of the
> function.
>
> Would suggest putting it in the header file describing the monitor
> call. For our SIP SVC calls, we've done this in the following form:
>
> /*
>   * SMC call to retrieve number of CPUs present in the system.
>   * Input values:
>   *   x0: NUVIA_SIP_GET_NUM_CPUS
>   * Return values:
>   *   x0: SMC_OK
>   *   x1: Number of CPUs present
>   */
> #define NUVIA_SIP_GET_NUM_CPUS   SIP_FUNCTION_ID(0x20)
>
> (Where SIP_FUNCTION_ID is one of a set of macros I should submit for
> addition to ArmStdSmc.h)
>
>> +  Parameters.Arg0 = FID_TRNG_RND;
>> +  Parameters.Arg1 = EntropyBits;
>> +  ArmCallMonitor (&Parameters);
>> +
>> +  // Convert status codes to EFI status codes.
> Function name already says this, comment redundant.
>
>> +  Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>  From here
>
>> +  // Extract Data
>> +  // ArgSelector = ((EntropyBytes + 3) >> 2); for AArch32
>> +  // ArgSelector = ((EntropyBytes + 7) >> 3); for AArch64
>> +  // ((sizeof (UINTN) >> 2) + 1) is 3 or 2 depending on size of UINTN
>> +  ArgSelector = ((EntropyBytes + (sizeof (UINTN) - 1)) >>
>> +                 ((sizeof (UINTN) >> 2) + 1));
>> +
>> +  switch (ArgSelector) {
>> +    case 3:
>> +      CopyMem (&Buffer[(sizeof (UINTN) * 2)], &Parameters.Arg1, sizeof (UINTN));
>> +
>> +    case 2:
>> +      CopyMem (&Buffer[sizeof (UINTN)], &Parameters.Arg2, sizeof (UINTN));
>> +
>> +    case 1:
>> +      CopyMem (&Buffer[0], &Parameters.Arg3, sizeof (UINTN));
>> +      break;
>> +
>> +    default:
>> +      ASSERT (0);
>> +      return RETURN_INVALID_PARAMETER;
>> +  } // switch
> to here ... I'm not convinced you yourself would be able to read or
> explain this code a few months down the line.
>
> Is there a strong reason for why Buffer cannot be a UINTN *?
[SAMI] The specification allows to request minimum 1 bit of entropy 
(although I don't think there would be a use case for this). Therefore, 
I selected UINT8.
However, I agree the logic is complex. I will simplify this code.
>
> I think what this code is doing can equally be written as:
>
>    Buffer[0] = Parameters.Arg3;
>    if ((EntropyBytes / sizeof (UINTN)) > 1) {
>      Buffer[1] = Parameters.Arg2;
>    }
>    if ((EntropyBytes / sizeof (UINTN)) > 2) {
>      Buffer[2] = Parameters.Arg1;
>    }
>
>> +
>> +
>> +  // [1] Section 2.4.3 Caller responsibilities.
>> +  // The caller must ensure that only the value in Entropy[N-1:0] is consumed
>> +  // and that the remaining bits in Entropy[MAX_BITS-1:N] are ignored.
>> +  // Therefore, Clear the unused upper bytes.
> This is source code, not the specification.
>
>    // Mask off any unused top bytes, in accordance with specification
>
> is sufficient as a comment.
[SAMI] I will fix this and the other comments in the next revision.
>
> /
>      Leif
>
>> +  BytesToClear = (sizeof (UINTN) * ArgSelector) - EntropyBytes;
>> +  if (BytesToClear != 0) {
>> +    ZeroMem (&Buffer[EntropyBytes], BytesToClear);
>> +  }
>> +
>> +  // Clear the unused MSB bits of the last byte.
>> +  LastValidBits = EntropyBits & 0x7;
>> +  if (LastValidBits != 0) {
>> +    Buffer[EntropyBytes - 1] &= (0xFF >> (8 - LastValidBits));
>> +  }
>> +
>> +  return Status;
>> +}
>> +
>> +/** The constructor checks that the FW-TRNG interface is supported
>> +    by the host firmware.
>> +
>> +  It will ASSERT() if FW-TRNG is not supported.
>> +  It will always return RETURN_SUCCESS.
>> +
>> +  @retval RETURN_SUCCESS   The constructor always returns RETURN_SUCCESS.
>> +**/
>> +RETURN_STATUS
>> +EFIAPI
>> +ArmFwTrngLibConstructor (
>> +  VOID
>> +  )
>> +{
>> +  RETURN_STATUS Status;
>> +  UINT16        MajorRev;
>> +  UINT16        MinorRev;
>> +  GUID          Guid;
>> +
>> +  Status = GetTrngVersion (&MajorRev, &MinorRev);
>> +  if (EFI_ERROR (Status)) {
>> +    return RETURN_SUCCESS;
>> +  }
>> +
>> +#ifndef MDEPKG_NDEBUG
>> +  // Check that the required features are present.
>> +  Status = GetTrngFeatures (FID_TRNG_RND, NULL);
>> +  if (EFI_ERROR (Status)) {
>> +    return RETURN_SUCCESS;
>> +  }
>> +
>> +  // Check if TRNG UUID is supported and if so trace the GUID.
>> +  Status = GetTrngFeatures (FID_TRNG_GET_UUID, NULL);
>> +  if (EFI_ERROR (Status)) {
>> +    return RETURN_SUCCESS;
>> +  }
>> +#endif
>> +
>> +  Status = GetTrngUuid (&Guid);
>> +  if (EFI_ERROR (Status)) {
>> +    return RETURN_SUCCESS;
>> +  }
>> +
>> +  DEBUG ((
>> +    DEBUG_INFO,
>> +    "FW-TRNG: Version %d.%d, GUID {%g}\n",
>> +    MajorRev,
>> +    MinorRev,
>> +    Guid
>> +    ));
>> +
>> +  return RETURN_SUCCESS;
>> +}
>> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..4b2c58251fbe8fbcb5af308736db014e8d954720
>> --- /dev/null
>> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
>> @@ -0,0 +1,34 @@
>> +## @file
>> +#  Arm Firmware TRNG interface library.
>> +#
>> +#  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
>> +#
>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION       = 0x0001001B
>> +  BASE_NAME         = ArmFwTrngLib
>> +  FILE_GUID         = 10DE97C9-28E4-4C9B-A53E-8D7D1B0DD4E0
>> +  VERSION_STRING    = 1.0
>> +  MODULE_TYPE       = BASE
>> +  LIBRARY_CLASS     = TrngLib
>> +  CONSTRUCTOR       = ArmFwTrngLibConstructor
>> +
>> +[Sources]
>> +  ArmFwTrngDefs.h
>> +  ArmFwTrngLib.c
>> +
>> +[Packages]
>> +  ArmPkg/ArmPkg.dec
>> +  MdePkg/MdePkg.dec
>> +
>> +[LibraryClasses]
>> +  ArmSmcLib
>> +  ArmHvcLib
>> +  BaseLib
>> +  BaseMemoryLib
>> +
>> +[Pcd]
>> +  gArmTokenSpaceGuid.PcdMonitorConduitHvc
>> +
>> -- 
>> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>>
>>
>>
>> 
>>
>>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
  2021-11-25 15:23     ` Sami Mujawar
@ 2022-03-24  9:46       ` PierreGondois
       [not found]       ` <80941d66-5d31-053f-388a-95efe5dbbfdf@arm.com>
  1 sibling, 0 replies; 19+ messages in thread
From: PierreGondois @ 2022-03-24  9:46 UTC (permalink / raw)
  To: devel, sami.mujawar, Leif Lindholm
  Cc: ardb+tianocore, rebecca, kraxel, michael.d.kinney, gaoliming,
	zhiguang.liu, jiewen.yao, jian.j.wang, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd


On 11/25/21 16:23, Sami Mujawar via groups.io wrote:
> Hi Leif,
> 
> Thank you for the feedback.
> 
> Please find my response inline marked [SAMI].
> 
> Regards,
> 
> Sami Mujawar
> 
> 
> On 24/11/2021 01:01 PM, Leif Lindholm wrote:
>> Hi Sami,
>>
>> On Tue, Nov 16, 2021 at 11:32:55 +0000, Sami Mujawar wrote:
>>> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
>>>
>>> The Arm True Random Number Generator Firmware, Interface 1.0,
>>> Platform Design Document
>>> (https://developer.arm.com/documentation/den0098/latest/)
>>> defines an interface between an Operating System (OS) executing
>>> at EL1 and Firmware (FW) exposing a conditioned entropy source
>>> that is provided by a TRNG back end.
>>>
>>> The conditioned entropy, that is provided by the TRNG FW interface,
>>> is commonly used to seed deterministic random number generators.
>>>
>>> This patch adds a TrngLib library that implements the Arm TRNG
>>> firmware interface.
>>>
>>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>>> ---
>>>
>>> Notes:
>>>       v2:
>>>        - MdePkg\Include\Library\TrngLib.h is base type     [LIMING]
>>>          library. It can use RETURN_STATUS instead of
>>>          EFI_STATUS.
>>>        - Replaced EFI_STATUS with RETURN_STATUS.           [SAMI]
>>>        - MdePkg\Include\Library\TrngLib.h API parameter    [LIMING]
>>>          doesn't require CONST. CONST means the value
>>>          specified by the input pointer will not be
>>>          changed in API implementation.
>>>        - Removed the use of constant pointers in the       [SAMI]
>>>          TRNG API.
>>>
>>>    ArmPkg/ArmPkg.dsc                            |   1 +
>>>    ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h  |  64 +++
>>>    ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c   | 483 ++++++++++++++++++++
>>>    ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf |  34 ++
>>>    4 files changed, 582 insertions(+)
>>>

[...]

>>> +
>>> +/** Invoke the monitor call using the appropriate conduit.
>>> +    If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
>>> +
>>> +  @param [in, out]  Args    Arguments passed to and returned from the monitor.
>>> +
>>> +  @return  VOID
>>> +**/
>>> +STATIC
>>> +VOID
>>> +ArmCallMonitor (
>>> +  IN OUT ARM_MONITOR_ARGS   *Args
>>> +  )
>>> +{
>>> +  if (FeaturePcdGet (PcdMonitorConduitHvc)) {
>>> +    ArmCallHvc ((ARM_HVC_ARGS*)Args);
>>> +  } else {
>>> +    ArmCallSmc ((ARM_SMC_ARGS*)Args);
>>> +  }
>>> +}
>> Should this be in (a potentially renamed) ArmSmcLib?
> [SAMI] Looking at ArmSmcLib and ArmHvcLib libraries there is not much
> difference in the code other than the SMC/HVC call. Please let me know
> if I should submit a patch to unify these in ArmMonitorLib?
> The ArmCall<Smc|Hvc> APIs would still remain the same but moved to
> ArmMonitorLib.

Hello Leif,
About your comment, I am not sure I understand it correctly. Assuming the
function allowing to choose the conduit looks like:

VOID
EFIAPI
ArmConduitCall (UINT8 Conduit) {
   if (Conduit == 0) {
     ArmCallHvc ((ARM_HVC_ARGS*)Args);
   } else if (Conduit == 1) {
     ArmCallSmc ((ARM_SMC_ARGS*)Args);
   } else {
    ASSERT (FALSE);
   }
}

Do you suggest to:
1. Make ArmSmcLib dependent on ArmHvcLib and add ArmConduitCall()
    in ArmSmcLib (or do the opposite with the ArmHvcLib)
2. Merge ArmSmcLib and ArmHvcLib in a new ArmConduitLibrary and add
    ArmConduitCall() in this new library.
3. Add an ArmConduitLibrary, relying on ArmSmcLib and ArmHvcLib, and having
    only one function: ArmConduitCall()

2. would make the Hvc and Smc calls really tied together.
3. would avoid creating new dependencies on existing libraries (i.e. a
platform only using the ArmSmcLib would not require to have a NULL instance of
ArmHvcLib). I assume you meant 3.

Regards,
Pierre


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
       [not found]       ` <80941d66-5d31-053f-388a-95efe5dbbfdf@arm.com>
@ 2022-03-24 14:56         ` PierreGondois
  2022-03-24 18:12           ` Leif Lindholm
  0 siblings, 1 reply; 19+ messages in thread
From: PierreGondois @ 2022-03-24 14:56 UTC (permalink / raw)
  To: devel, sami.mujawar, Leif Lindholm
  Cc: ardb+tianocore, rebecca, kraxel, michael.d.kinney, gaoliming,
	zhiguang.liu, jiewen.yao, jian.j.wang, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

(Replacing Leif's mail address to the @quicinc.com one)


On 3/24/22 10:46, Pierre Gondois wrote:
> 
> On 11/25/21 16:23, Sami Mujawar via groups.io wrote:
>> Hi Leif,
>>
>> Thank you for the feedback.
>>
>> Please find my response inline marked [SAMI].
>>
>> Regards,
>>
>> Sami Mujawar
>>
>>
>> On 24/11/2021 01:01 PM, Leif Lindholm wrote:
>>> Hi Sami,
>>>
>>> On Tue, Nov 16, 2021 at 11:32:55 +0000, Sami Mujawar wrote:
>>>> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
>>>>
>>>> The Arm True Random Number Generator Firmware, Interface 1.0,
>>>> Platform Design Document
>>>> (https://developer.arm.com/documentation/den0098/latest/)
>>>> defines an interface between an Operating System (OS) executing
>>>> at EL1 and Firmware (FW) exposing a conditioned entropy source
>>>> that is provided by a TRNG back end.
>>>>
>>>> The conditioned entropy, that is provided by the TRNG FW interface,
>>>> is commonly used to seed deterministic random number generators.
>>>>
>>>> This patch adds a TrngLib library that implements the Arm TRNG
>>>> firmware interface.
>>>>
>>>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>>>> ---
>>>>
>>>> Notes:
>>>>        v2:
>>>>         - MdePkg\Include\Library\TrngLib.h is base type     [LIMING]
>>>>           library. It can use RETURN_STATUS instead of
>>>>           EFI_STATUS.
>>>>         - Replaced EFI_STATUS with RETURN_STATUS.           [SAMI]
>>>>         - MdePkg\Include\Library\TrngLib.h API parameter    [LIMING]
>>>>           doesn't require CONST. CONST means the value
>>>>           specified by the input pointer will not be
>>>>           changed in API implementation.
>>>>         - Removed the use of constant pointers in the       [SAMI]
>>>>           TRNG API.
>>>>
>>>>     ArmPkg/ArmPkg.dsc                            |   1 +
>>>>     ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h  |  64 +++
>>>>     ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c   | 483 ++++++++++++++++++++
>>>>     ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf |  34 ++
>>>>     4 files changed, 582 insertions(+)
>>>>
> 
> [...]
> 
>>>> +
>>>> +/** Invoke the monitor call using the appropriate conduit.
>>>> +    If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
>>>> +
>>>> +  @param [in, out]  Args    Arguments passed to and returned from the monitor.
>>>> +
>>>> +  @return  VOID
>>>> +**/
>>>> +STATIC
>>>> +VOID
>>>> +ArmCallMonitor (
>>>> +  IN OUT ARM_MONITOR_ARGS   *Args
>>>> +  )
>>>> +{
>>>> +  if (FeaturePcdGet (PcdMonitorConduitHvc)) {
>>>> +    ArmCallHvc ((ARM_HVC_ARGS*)Args);
>>>> +  } else {
>>>> +    ArmCallSmc ((ARM_SMC_ARGS*)Args);
>>>> +  }
>>>> +}
>>> Should this be in (a potentially renamed) ArmSmcLib?
>> [SAMI] Looking at ArmSmcLib and ArmHvcLib libraries there is not much
>> difference in the code other than the SMC/HVC call. Please let me know
>> if I should submit a patch to unify these in ArmMonitorLib?
>> The ArmCall<Smc|Hvc> APIs would still remain the same but moved to
>> ArmMonitorLib.
> 
> Hello Leif,
> About your comment, I am not sure I understand it correctly. Assuming the
> function allowing to choose the conduit looks like:
> 
> VOID
> EFIAPI
> ArmConduitCall (UINT8 Conduit) {
>     if (Conduit == 0) {
>       ArmCallHvc ((ARM_HVC_ARGS*)Args);
>     } else if (Conduit == 1) {
>       ArmCallSmc ((ARM_SMC_ARGS*)Args);
>     } else {
>      ASSERT (FALSE);
>     }
> }
> 
> Do you suggest to:
> 1. Make ArmSmcLib dependent on ArmHvcLib and add ArmConduitCall()
>      in ArmSmcLib (or do the opposite with the ArmHvcLib)
> 2. Merge ArmSmcLib and ArmHvcLib in a new ArmConduitLibrary and add
>      ArmConduitCall() in this new library.
> 3. Add an ArmConduitLibrary, relying on ArmSmcLib and ArmHvcLib, and having
>      only one function: ArmConduitCall()
> 
> 2. would make the Hvc and Smc calls really tied together.
> 3. would avoid creating new dependencies on existing libraries (i.e. a
> platform only using the ArmSmcLib would not require to have a NULL instance of
> ArmHvcLib). I assume you meant 3.
> 
> Regards,
> Pierre
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
  2022-03-24 14:56         ` PierreGondois
@ 2022-03-24 18:12           ` Leif Lindholm
  0 siblings, 0 replies; 19+ messages in thread
From: Leif Lindholm @ 2022-03-24 18:12 UTC (permalink / raw)
  To: Pierre Gondois
  Cc: devel, sami.mujawar, ardb+tianocore, rebecca, kraxel,
	michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
	jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd

On Thu, Mar 24, 2022 at 15:56:32 +0100, Pierre Gondois wrote:
> > > > > +/** Invoke the monitor call using the appropriate conduit.
> > > > > +    If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
> > > > > +
> > > > > +  @param [in, out]  Args    Arguments passed to and returned from the monitor.
> > > > > +
> > > > > +  @return  VOID
> > > > > +**/
> > > > > +STATIC
> > > > > +VOID
> > > > > +ArmCallMonitor (
> > > > > +  IN OUT ARM_MONITOR_ARGS   *Args
> > > > > +  )
> > > > > +{
> > > > > +  if (FeaturePcdGet (PcdMonitorConduitHvc)) {
> > > > > +    ArmCallHvc ((ARM_HVC_ARGS*)Args);
> > > > > +  } else {
> > > > > +    ArmCallSmc ((ARM_SMC_ARGS*)Args);
> > > > > +  }
> > > > > +}
> > > > Should this be in (a potentially renamed) ArmSmcLib?
> > > [SAMI] Looking at ArmSmcLib and ArmHvcLib libraries there is not much
> > > difference in the code other than the SMC/HVC call. Please let me know
> > > if I should submit a patch to unify these in ArmMonitorLib?
> > > The ArmCall<Smc|Hvc> APIs would still remain the same but moved to
> > > ArmMonitorLib.
> > 
> > Hello Leif,
> > About your comment, I am not sure I understand it correctly. Assuming the
> > function allowing to choose the conduit looks like:
> > 
> > VOID
> > EFIAPI
> > ArmConduitCall (UINT8 Conduit) {
> >     if (Conduit == 0) {
> >       ArmCallHvc ((ARM_HVC_ARGS*)Args);
> >     } else if (Conduit == 1) {
> >       ArmCallSmc ((ARM_SMC_ARGS*)Args);
> >     } else {
> >      ASSERT (FALSE);
> >     }
> > }
> > 
> > Do you suggest to:
> > 1. Make ArmSmcLib dependent on ArmHvcLib and add ArmConduitCall()
> >      in ArmSmcLib (or do the opposite with the ArmHvcLib)
> > 2. Merge ArmSmcLib and ArmHvcLib in a new ArmConduitLibrary and add
> >      ArmConduitCall() in this new library.
> > 3. Add an ArmConduitLibrary, relying on ArmSmcLib and ArmHvcLib, and having
> >      only one function: ArmConduitCall()
> >
> > 
> > 2. would make the Hvc and Smc calls really tied together.
> > 3. would avoid creating new dependencies on existing libraries (i.e. a
> > platform only using the ArmSmcLib would not require to have a NULL instance of
> > ArmHvcLib). I assume you meant 3.

I think 2 and 3 both capture the spirit of my request.

2 could potentially be achieved with a FixedPcd and conditionals in
asm (less sure about the armasm/vs situation).

I guess the question is if we think it plausible that a platform might
both want to use this new ArmConduitCall *and* directly make use of
ArmHvcLib/ArmSmcLib.

Best Regards,

Leif

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-03-24 18:12 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 1/8] MdePkg: Definition for TRNG library class interface Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls Sami Mujawar
2021-11-24 12:07   ` Leif Lindholm
2021-11-24 13:03     ` Ard Biesheuvel
2021-11-24 13:05       ` Leif Lindholm
2021-11-24 13:07         ` Ard Biesheuvel
2021-11-24 13:25           ` Leif Lindholm
2021-11-16 11:32 ` [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library Sami Mujawar
2021-11-24 13:01   ` [edk2-devel] " Leif Lindholm
2021-11-25 15:23     ` Sami Mujawar
2022-03-24  9:46       ` PierreGondois
     [not found]       ` <80941d66-5d31-053f-388a-95efe5dbbfdf@arm.com>
2022-03-24 14:56         ` PierreGondois
2022-03-24 18:12           ` Leif Lindholm
2021-11-16 11:32 ` [PATCH v2 4/8] MdePkg: Add NULL instance of TRNG Library Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 5/8] SecurityPkg: Rename RdRandGenerateEntropy to common name Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 6/8] SecurityPkg: Restructure checks in RngGetInfo Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 7/8] SecurityPkg: Add RawAlgorithm support using TRNG library Sami Mujawar
2021-11-16 11:33 ` [PATCH v2 8/8] ArmVirtPkg: Kvmtool: Add RNG support using FW-TRNG interface Sami Mujawar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox