public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification
@ 2023-07-06  8:51 PierreGondois
  2023-07-06  8:51 ` [PATCH v3 1/6] SecurityPkg/SecurityPkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg PierreGondois
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: PierreGondois @ 2023-07-06  8:51 UTC (permalink / raw)
  To: devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

From: Pierre Gondois <pierre.gondois@arm.com>

v3:
- As the unsafe algorithm GUID will not be added to the UEFI
  specification, rename:
  - gEfiRngAlgorithmUnSafe to gEdkiiRngAlgorithmUnSafe
  - EFI_RNG_ALGORITHM_UNSAFE to EDKII_RNG_ALGORITHM_UNSAFE

v2:
[1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation
- Dropped
[2/8] MdePkg/MdePkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg
- Change gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm
  token number
- Rename to SecurityPkg/SecurityPkg.dec: Move
  PcdCpuRngSupportedAlgorithm to MdePkg
[5/8] MdePkg/Rng: Add GetRngGuid() to RngLib
- Remove gEfiRngAlgorithmUnSafe from inf file
- Split Guids definitions in arch specific sections
[6/8] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
- Remove RngFindDefaultAlgo() and change logic accordingly.
[7/8] SecurityPkg/RngDxe: Select safe default Rng algorithm
- Dropped due to changes in [6/8]

This patch also requires the following patch on top of the serie:
- https://edk2.groups.io/g/devel/message/106546

This patchset follows the 'code first' approach and relates to [1].
This patchset follows the thread at [3] that aims to solve [2].
[1] and [2] are bound and this patchset aims to solve both.

In this patchset:
a-
The RngDxe can rely on the RngLib. However the RngLib has no
interface allowing to describe which Rng algorithm is implemented.
The RngDxe must advertise the algorithm that are available through
the RngGetInfo() callback.
Add a GetRngGuid() for interface to the RngLib.

b-
The Arm Architecture states the RNDR that the DRBG algorithm should
be compliant with NIST SP800-90A, while not mandating a particular
algorithm, so as to be inclusive of different geographies.
The RngLib can rely on this Arm RNDR instruction. In order to
accurately describe the implementation using the RNDR instruction,
add a EFI_RNG_ALGORITHM_ARM_RNDR GUID [1].

c-
For the same reason as a/b, add a GUID describing unsafe RNG
algorithms, allowing to accurately describe the BaseRngLibTimerLib.

d-
Use a/b/c mechanisms/GUIDs to select a safe Rng algorithm in the
Arm implementation of the RngDxe.

[1] BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
[2] BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4151
[3] https://edk2.groups.io/g/devel/message/100806

Pierre Gondois (6):
  SecurityPkg/SecurityPkg.dec: Move PcdCpuRngSupportedAlgorithm to
    MdePkg
  MdePkg/DxeRngLib: Request raw algorithm instead of default
  MdePkg/Rng: Add GUIDs to describe Rng algorithms
  MdePkg/Rng: Add GetRngGuid() to RngLib
  SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
  SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm

 MdePkg/Include/Library/RngLib.h               | 17 ++++++
 MdePkg/Include/Protocol/Rng.h                 | 20 +++++++
 MdePkg/Library/BaseRngLib/AArch64/Rndr.c      | 42 +++++++++++++++
 MdePkg/Library/BaseRngLib/BaseRngLib.inf      | 10 ++++
 MdePkg/Library/BaseRngLib/Rand/RdRand.c       | 26 +++++++++
 .../Library/BaseRngLibNull/BaseRngLibNull.c   | 22 ++++++++
 .../BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  3 ++
 .../Library/BaseRngLibTimerLib/RngLibTimer.c  | 28 ++++++++++
 MdePkg/Library/DxeRngLib/DxeRngLib.c          | 36 ++++++++++++-
 MdePkg/MdePkg.dec                             |  7 +++
 .../RngDxe/AArch64/AArch64Algo.c              | 54 +++++++++++++------
 .../RandomNumberGenerator/RngDxe/ArmRngDxe.c  | 23 ++++----
 .../RandomNumberGenerator/RngDxe/RngDxe.inf   |  5 +-
 SecurityPkg/SecurityPkg.dec                   |  2 -
 14 files changed, 258 insertions(+), 37 deletions(-)

-- 
2.25.1


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

* [PATCH v3 1/6] SecurityPkg/SecurityPkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg
  2023-07-06  8:51 [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification PierreGondois
@ 2023-07-06  8:51 ` PierreGondois
  2023-07-06  8:51 ` [PATCH v3 2/6] MdePkg/DxeRngLib: Request raw algorithm instead of default PierreGondois
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: PierreGondois @ 2023-07-06  8:51 UTC (permalink / raw)
  To: devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

From: Pierre Gondois <pierre.gondois@arm.com>

In order to use PcdCpuRngSupportedAlgorithm in the MdePkg in a
following patch and to avoid making the MdePkg dependent on another
package, move PcdCpuRngSupportedAlgorithm to the MdePkg.

As the Pcd is only used for AARCH64, place it in an AARCH64
specific sections.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
    - Change gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm
      token number
    - Rename to 'SecurityPkg/SecurityPkg.dec: Move
      PcdCpuRngSupportedAlgorithm to MdePkg'

 MdePkg/MdePkg.dec                                   | 5 +++++
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf | 4 ++--
 SecurityPkg/SecurityPkg.dec                         | 2 --
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index b85614992b94..5b8477f4cb8f 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2393,6 +2393,11 @@ [PcdsFixedAtBuild,PcdsPatchableInModule]
   # @Prompt Time-out for a response, internal
   gEfiMdePkgTokenSpaceGuid.PcdIpmiSsifResponseRetryIntervalMicrosecond|60000|UINT32|0x00000036
 
+[PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.AARCH64]
+  ## GUID identifying the Rng algorithm implemented by CPU instruction.
+  # @Prompt CPU Rng algorithm's GUID.
+  gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x00000037
+
 [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## This value is used to set the base address of PCI express hierarchy.
   # @Prompt PCI Express Base Address.
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
index c8e0ee4ae5d9..d6c2d30195bf 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
@@ -79,8 +79,8 @@ [Guids]
 [Protocols]
   gEfiRngProtocolGuid                ## PRODUCES
 
-[Pcd]
-  gEfiSecurityPkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm      ## CONSUMES
+[Pcd.AARCH64]
+  gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm      ## CONSUMES
 
 [Depex]
   TRUE
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 53aa7ec43557..00c4ebdbed59 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -325,8 +325,6 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationPass|0x0303100A|UINT32|0x00010030
   gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationFail|0x0303100B|UINT32|0x00010031
 
-  gEfiSecurityPkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x00010032
-
 [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## Image verification policy for OptionRom. Only following values are valid:<BR><BR>
   #  NOTE: Do NOT use 0x5 and 0x2 since it violates the UEFI specification and has been removed.<BR>
-- 
2.25.1


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

* [PATCH v3 2/6] MdePkg/DxeRngLib: Request raw algorithm instead of default
  2023-07-06  8:51 [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification PierreGondois
  2023-07-06  8:51 ` [PATCH v3 1/6] SecurityPkg/SecurityPkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg PierreGondois
@ 2023-07-06  8:51 ` PierreGondois
  2023-07-06  8:51 ` [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms PierreGondois
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: PierreGondois @ 2023-07-06  8:51 UTC (permalink / raw)
  To: devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

From: Pierre Gondois <pierre.gondois@arm.com>

The DxeRngLib tries to generate a random number using the 3 NIST
SP 800-90 compliant DRBG algorithms, i.e. 256-bits CTR, HASH and HMAC.
If none of the call is successful, the fallback option is the default
RNG algorithm of the EFI_RNG_PROTOCOL. This default algorithm might
be an unsafe implementation.

Try requesting the Raw algorithm before requesting the default one.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
 MdePkg/Library/DxeRngLib/DxeRngLib.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.c b/MdePkg/Library/DxeRngLib/DxeRngLib.c
index 46aea515924f..a01b66ad7d20 100644
--- a/MdePkg/Library/DxeRngLib/DxeRngLib.c
+++ b/MdePkg/Library/DxeRngLib/DxeRngLib.c
@@ -65,9 +65,15 @@ GenerateRandomNumberViaNist800Algorithm (
     return Status;
   }
 
+  Status = RngProtocol->GetRNG (RngProtocol, &gEfiRngAlgorithmRaw, BufferSize, Buffer);
+  DEBUG ((DEBUG_INFO, "%a: GetRNG algorithm Raw - Status = %r\n", __func__, Status));
+  if (!EFI_ERROR (Status)) {
+    return Status;
+  }
+
   // If all the other methods have failed, use the default method from the RngProtocol
   Status = RngProtocol->GetRNG (RngProtocol, NULL, BufferSize, Buffer);
-  DEBUG ((DEBUG_INFO, "%a: GetRNG algorithm Hash-256 - Status = %r\n", __func__, Status));
+  DEBUG ((DEBUG_INFO, "%a: GetRNG algorithm default - Status = %r\n", __func__, Status));
   if (!EFI_ERROR (Status)) {
     return Status;
   }
-- 
2.25.1


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

* [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms
  2023-07-06  8:51 [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification PierreGondois
  2023-07-06  8:51 ` [PATCH v3 1/6] SecurityPkg/SecurityPkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg PierreGondois
  2023-07-06  8:51 ` [PATCH v3 2/6] MdePkg/DxeRngLib: Request raw algorithm instead of default PierreGondois
@ 2023-07-06  8:51 ` PierreGondois
  2023-07-07  9:14   ` [edk2-devel] " Yao, Jiewen
  2023-07-06  8:51 ` [PATCH v3 4/6] MdePkg/Rng: Add GetRngGuid() to RngLib PierreGondois
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: PierreGondois @ 2023-07-06  8:51 UTC (permalink / raw)
  To: devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

From: Pierre Gondois <pierre.gondois@arm.com>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441

The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
To allow the RngDxe to detect when such implementation is used,
a GetRngGuid() function is added in a following patch.

Prepare GetRngGuid() return values and add GUIDs describing
Rng algorithms:
- gEfiRngAlgorithmArmRndr
to describe a Rng algorithm accessed through Arm's RNDR instruction.
[1] states that the implementation of this algorithm should be
compliant to NIST SP900-80. The compliance is not guaranteed.
- gEdkiiRngAlgorithmUnSafe
to describe an unsafe implementation, cf. the BaseRngLibTimerLib.

[1] Arm Architecture Reference Manual Armv8, for A-profile architecture
sK12.1 'Properties of the generated random number'

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
 MdePkg/Include/Protocol/Rng.h | 20 ++++++++++++++++++++
 MdePkg/MdePkg.dec             |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/MdePkg/Include/Protocol/Rng.h b/MdePkg/Include/Protocol/Rng.h
index baf425587b3c..ceae77ba9c73 100644
--- a/MdePkg/Include/Protocol/Rng.h
+++ b/MdePkg/Include/Protocol/Rng.h
@@ -67,6 +67,24 @@ typedef EFI_GUID EFI_RNG_ALGORITHM;
   { \
     0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 } \
   }
+///
+/// The Arm Architecture states the RNDR that the DRBG algorithm should be compliant
+/// with NIST SP800-90A, while not mandating a particular algorithm, so as to be
+/// inclusive of different geographies.
+///
+#define EFI_RNG_ALGORITHM_ARM_RNDR \
+  { \
+    0x43d2fde3, 0x9d4e, 0x4d79,  {0x02, 0x96, 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41} \
+  }
+///
+/// The implementation of a Random Number Generator might be unsafe, when using
+/// a dummy implementation for instance. Allow identifying such implementation
+/// with this GUID.
+///
+#define EDKII_RNG_ALGORITHM_UNSAFE \
+  { \
+    0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 } \
+  }
 
 /**
   Returns information about the random number generation implementation.
@@ -146,5 +164,7 @@ extern EFI_GUID  gEfiRngAlgorithmSp80090Ctr256Guid;
 extern EFI_GUID  gEfiRngAlgorithmX9313DesGuid;
 extern EFI_GUID  gEfiRngAlgorithmX931AesGuid;
 extern EFI_GUID  gEfiRngAlgorithmRaw;
+extern EFI_GUID  gEfiRngAlgorithmArmRndr;
+extern EFI_GUID  gEdkiiRngAlgorithmUnSafe;
 
 #endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 5b8477f4cb8f..2c8f985f253e 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -643,6 +643,8 @@ [Guids]
   gEfiRngAlgorithmX9313DesGuid       = { 0x63c4785a, 0xca34, 0x4012, {0xa3, 0xc8, 0x0b, 0x6a, 0x32, 0x4f, 0x55, 0x46 }}
   gEfiRngAlgorithmX931AesGuid        = { 0xacd03321, 0x777e, 0x4d3d, {0xb1, 0xc8, 0x20, 0xcf, 0xd8, 0x88, 0x20, 0xc9 }}
   gEfiRngAlgorithmRaw                = { 0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }}
+  gEfiRngAlgorithmArmRndr            = { 0x43d2fde3, 0x9d4e, 0x4d79, {0x02, 0x96, 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41 }}
+  gEdkiiRngAlgorithmUnSafe           = { 0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 }}
 
   ## Include/Protocol/AdapterInformation.h
   gEfiAdapterInfoMediaStateGuid       = { 0xD7C74207, 0xA831, 0x4A26, {0xB1, 0xF5, 0xD1, 0x93, 0x06, 0x5C, 0xE8, 0xB6 }}
-- 
2.25.1


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

* [PATCH v3 4/6] MdePkg/Rng: Add GetRngGuid() to RngLib
  2023-07-06  8:51 [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification PierreGondois
                   ` (2 preceding siblings ...)
  2023-07-06  8:51 ` [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms PierreGondois
@ 2023-07-06  8:51 ` PierreGondois
  2023-07-06  8:51 ` [PATCH v3 5/6] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib PierreGondois
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: PierreGondois @ 2023-07-06  8:51 UTC (permalink / raw)
  To: devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

From: Pierre Gondois <pierre.gondois@arm.com>

The EFI_RNG_PROTOCOL can use the RngLib. The RngLib has multiple
implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
To allow the RngDxe to detect when such implementation is used,
add a GetRngGuid() function to the RngLib.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
 MdePkg/Include/Library/RngLib.h               | 17 ++++++++
 MdePkg/Library/BaseRngLib/AArch64/Rndr.c      | 42 +++++++++++++++++++
 MdePkg/Library/BaseRngLib/BaseRngLib.inf      | 10 +++++
 MdePkg/Library/BaseRngLib/Rand/RdRand.c       | 26 ++++++++++++
 .../Library/BaseRngLibNull/BaseRngLibNull.c   | 22 ++++++++++
 .../BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  3 ++
 .../Library/BaseRngLibTimerLib/RngLibTimer.c  | 28 +++++++++++++
 MdePkg/Library/DxeRngLib/DxeRngLib.c          | 28 +++++++++++++
 8 files changed, 176 insertions(+)

diff --git a/MdePkg/Include/Library/RngLib.h b/MdePkg/Include/Library/RngLib.h
index 429ed19e287e..945482cd5e56 100644
--- a/MdePkg/Include/Library/RngLib.h
+++ b/MdePkg/Include/Library/RngLib.h
@@ -1,6 +1,7 @@
 /** @file
   Provides random number generator services.
 
+Copyright (c) 2023, Arm Limited. All rights reserved.<BR>
 Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -77,4 +78,20 @@ GetRandomNumber128 (
   OUT     UINT64  *Rand
   );
 
+/**
+  Get a GUID identifying the RNG algorithm implementation.
+
+  @param [out] RngGuid  If success, contains the GUID identifying
+                        the RNG algorithm implementation.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_UNSUPPORTED         Not supported.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+GetRngGuid (
+  GUID  *RngGuid
+  );
+
 #endif // __RNG_LIB_H__
diff --git a/MdePkg/Library/BaseRngLib/AArch64/Rndr.c b/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
index 20811bf3ebf3..d39db62153ee 100644
--- a/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
+++ b/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
@@ -2,6 +2,7 @@
   Random number generator service that uses the RNDR instruction
   to provide pseudorandom numbers.
 
+  Copyright (c) 2023, Arm Limited. All rights reserved.<BR>
   Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
   Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
 
@@ -11,6 +12,7 @@
 
 #include <Uefi.h>
 #include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/RngLib.h>
 
@@ -138,3 +140,43 @@ ArchIsRngSupported (
 {
   return mRndrSupported;
 }
+
+/**
+  Get a GUID identifying the RNG algorithm implementation.
+
+  @param [out] RngGuid  If success, contains the GUID identifying
+                        the RNG algorithm implementation.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_UNSUPPORTED         Not supported.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+GetRngGuid (
+  GUID  *RngGuid
+  )
+{
+  GUID  *RngLibGuid;
+
+  if (RngGuid == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (!mRndrSupported) {
+    return EFI_UNSUPPORTED;
+  }
+
+  //
+  // If the platform advertises the algorithm behind RNDR instruction,
+  // use it. Otherwise use gEfiRngAlgorithmArmRndr.
+  //
+  RngLibGuid = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
+  if (!IsZeroGuid (RngLibGuid)) {
+    CopyMem (RngGuid, RngLibGuid, sizeof (*RngGuid));
+  } else {
+    CopyMem (RngGuid, &gEfiRngAlgorithmArmRndr, sizeof (*RngGuid));
+  }
+
+  return EFI_SUCCESS;
+}
diff --git a/MdePkg/Library/BaseRngLib/BaseRngLib.inf b/MdePkg/Library/BaseRngLib/BaseRngLib.inf
index 1fcceb941495..49503b139be9 100644
--- a/MdePkg/Library/BaseRngLib/BaseRngLib.inf
+++ b/MdePkg/Library/BaseRngLib/BaseRngLib.inf
@@ -4,6 +4,7 @@
 #  BaseRng Library that uses CPU RNG instructions (e.g. RdRand) to
 #  provide random numbers.
 #
+#  Copyright (c) 2023, Arm Limited. All rights reserved.<BR>
 #  Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
 #  Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
 #
@@ -43,9 +44,18 @@ [Sources.AARCH64]
   AArch64/ArmReadIdIsar0.asm | MSFT
   AArch64/ArmRng.asm         | MSFT
 
+[Guids.AARCH64]
+  gEfiRngAlgorithmArmRndr
+
+[Guids.Ia32, Guids.X64]
+  gEfiRngAlgorithmSp80090Ctr256Guid
+
 [Packages]
   MdePkg/MdePkg.dec
 
+[Pcd.AARCH64]
+  gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm
+
 [LibraryClasses]
   BaseLib
   DebugLib
diff --git a/MdePkg/Library/BaseRngLib/Rand/RdRand.c b/MdePkg/Library/BaseRngLib/Rand/RdRand.c
index 070d41e2555f..9bd68352f9f7 100644
--- a/MdePkg/Library/BaseRngLib/Rand/RdRand.c
+++ b/MdePkg/Library/BaseRngLib/Rand/RdRand.c
@@ -2,6 +2,7 @@
   Random number generator services that uses RdRand instruction access
   to provide high-quality random numbers.
 
+Copyright (c) 2023, Arm Limited. All rights reserved.<BR>
 Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
 Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
 
@@ -11,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <Uefi.h>
 #include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 
 #include "BaseRngLibInternals.h"
@@ -128,3 +130,27 @@ ArchIsRngSupported (
   */
   return TRUE;
 }
+
+/**
+  Get a GUID identifying the RNG algorithm implementation.
+
+  @param [out] RngGuid  If success, contains the GUID identifying
+                        the RNG algorithm implementation.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_UNSUPPORTED         Not supported.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+GetRngGuid (
+  GUID  *RngGuid
+  )
+{
+  if (RngGuid == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  CopyMem (RngGuid, &gEfiRngAlgorithmSp80090Ctr256Guid, sizeof (*RngGuid));
+  return EFI_SUCCESS;
+}
diff --git a/MdePkg/Library/BaseRngLibNull/BaseRngLibNull.c b/MdePkg/Library/BaseRngLibNull/BaseRngLibNull.c
index efba5c851ead..af5e8eb8f72a 100644
--- a/MdePkg/Library/BaseRngLibNull/BaseRngLibNull.c
+++ b/MdePkg/Library/BaseRngLibNull/BaseRngLibNull.c
@@ -1,13 +1,16 @@
 /** @file
   Null version of Random number generator services.
 
+Copyright (c) 2023, Arm Limited. All rights reserved.<BR>
 Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
+#include <Uefi.h>
 #include <Library/DebugLib.h>
 #include <Library/RngLib.h>
+#include <Protocol/Rng.h>
 
 /**
   Generates a 16-bit random number.
@@ -92,3 +95,22 @@ GetRandomNumber128 (
   ASSERT (FALSE);
   return FALSE;
 }
+
+/**
+  Get a GUID identifying the RNG algorithm implementation.
+
+  @param [out] RngGuid  If success, contains the GUID identifying
+                        the RNG algorithm implementation.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_UNSUPPORTED         Not supported.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+GetRngGuid (
+  GUID  *RngGuid
+  )
+{
+  return EFI_UNSUPPORTED;
+}
diff --git a/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf b/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
index f857290e823b..76279b844962 100644
--- a/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
+++ b/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
@@ -30,6 +30,9 @@ [Sources]
 [Packages]
   MdePkg/MdePkg.dec
 
+[Guids]
+  gEdkiiRngAlgorithmUnSafe
+
 [LibraryClasses]
   BaseLib
   DebugLib
diff --git a/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
index 980854d67b72..110fad7cc116 100644
--- a/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
+++ b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
@@ -2,14 +2,18 @@
   BaseRng Library that uses the TimerLib to provide reasonably random numbers.
   Do not use this on a production system.
 
+  Copyright (c) 2023, Arm Limited. All rights reserved.
   Copyright (c) Microsoft Corporation.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
 #include <Base.h>
+#include <Uefi.h>
 #include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/TimerLib.h>
+#include <Protocol/Rng.h>
 
 #define DEFAULT_DELAY_TIME_IN_MICROSECONDS  10
 
@@ -190,3 +194,27 @@ GetRandomNumber128 (
   // Read second 64 bits
   return GetRandomNumber64 (++Rand);
 }
+
+/**
+  Get a GUID identifying the RNG algorithm implementation.
+
+  @param [out] RngGuid  If success, contains the GUID identifying
+                        the RNG algorithm implementation.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_UNSUPPORTED         Not supported.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+GetRngGuid (
+  GUID  *RngGuid
+  )
+{
+  if (RngGuid == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  CopyMem (RngGuid, &gEdkiiRngAlgorithmUnSafe, sizeof (*RngGuid));
+  return EFI_SUCCESS;
+}
diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.c b/MdePkg/Library/DxeRngLib/DxeRngLib.c
index a01b66ad7d20..cd23859e3112 100644
--- a/MdePkg/Library/DxeRngLib/DxeRngLib.c
+++ b/MdePkg/Library/DxeRngLib/DxeRngLib.c
@@ -1,6 +1,7 @@
 /** @file
  Provides an implementation of the library class RngLib that uses the Rng protocol.
 
+ Copyright (c) 2023, Arm Limited. All rights reserved.
  Copyright (c) Microsoft Corporation. All rights reserved.
  SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -207,3 +208,30 @@ GetRandomNumber128 (
 
   return TRUE;
 }
+
+/**
+  Get a GUID identifying the RNG algorithm implementation.
+
+  @param [out] RngGuid  If success, contains the GUID identifying
+                        the RNG algorithm implementation.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_UNSUPPORTED         Not supported.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+GetRngGuid (
+  GUID  *RngGuid
+  )
+{
+  /* It is not possible to know beforehand which Rng algorithm will
+     be used by this library.
+     This API is mainly used by RngDxe. RngDxe relies on the RngLib.
+     The RngLib|DxeRngLib.inf implementation locates and uses an installed
+     EFI_RNG_PROTOCOL.
+     It is thus not possible to have both RngDxe and RngLib|DxeRngLib.inf.
+     and it is ok not to support this API.
+  */
+  return EFI_UNSUPPORTED;
+}
-- 
2.25.1


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

* [PATCH v3 5/6] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
  2023-07-06  8:51 [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification PierreGondois
                   ` (3 preceding siblings ...)
  2023-07-06  8:51 ` [PATCH v3 4/6] MdePkg/Rng: Add GetRngGuid() to RngLib PierreGondois
@ 2023-07-06  8:51 ` PierreGondois
  2023-07-07  8:07   ` Sami Mujawar
  2023-07-06  8:51 ` [PATCH v3 6/6] SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm PierreGondois
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: PierreGondois @ 2023-07-06  8:51 UTC (permalink / raw)
  To: devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

From: Pierre Gondois <pierre.gondois@arm.com>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4151

The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
To allow the RngDxe to detect when such implementation is used,
a GetRngGuid() function was added in a previous patch.

The EFI_RNG_PROTOCOL can advertise multiple algorithms through
Guids. The PcdCpuRngSupportedAlgorithm is currently used to
advertise the RngLib in the Arm implementation.

The issues of doing that are:
- the RngLib implementation might not use CPU instructions,
  cf. the BaseRngLibTimerLib
- most platforms don't set PcdCpuRngSupportedAlgorithm

A GetRngGuid() was added to the RngLib in a previous patch,
allowing to identify the algorithm implemented by the RngLib.
Make use of this function and place the unsage algorithm
at the last position in the mAvailableAlgoArray.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 .../RngDxe/AArch64/AArch64Algo.c              | 54 +++++++++++++------
 .../RandomNumberGenerator/RngDxe/ArmRngDxe.c  |  6 ++-
 .../RandomNumberGenerator/RngDxe/RngDxe.inf   |  5 +-
 3 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
index e8be217f8a8c..d355f575d5c8 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
@@ -10,6 +10,7 @@
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/ArmTrngLib.h>
+#include <Library/RngLib.h>
 
 #include "RngDxeInternals.h"
 
@@ -28,9 +29,13 @@ GetAvailableAlgorithms (
   VOID
   )
 {
-  UINT64  DummyRand;
-  UINT16  MajorRevision;
-  UINT16  MinorRevision;
+  EFI_STATUS  Status;
+  UINT16      MajorRevision;
+  UINT16      MinorRevision;
+  GUID        RngGuid;
+  BOOLEAN     UnSafeAlgo;
+
+  UnSafeAlgo = FALSE;
 
   // Rng algorithms 2 times, one for the allocation, one to populate.
   mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX);
@@ -38,24 +43,29 @@ GetAvailableAlgorithms (
     return EFI_OUT_OF_RESOURCES;
   }
 
-  // Check RngGetBytes() before advertising PcdCpuRngSupportedAlgorithm.
-  if (!EFI_ERROR (RngGetBytes (sizeof (DummyRand), (UINT8 *)&DummyRand))) {
-    CopyMem (
-      &mAvailableAlgoArray[mAvailableAlgoArrayCount],
-      PcdGetPtr (PcdCpuRngSupportedAlgorithm),
-      sizeof (EFI_RNG_ALGORITHM)
-      );
-    mAvailableAlgoArrayCount++;
-
-    DEBUG_CODE_BEGIN ();
-    if (IsZeroGuid (PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
+  // Identify RngLib algorithm.
+  Status = GetRngGuid (&RngGuid);
+  if (!EFI_ERROR (Status)) {
+    if (IsZeroGuid (&RngGuid) ||
+        CompareGuid (&RngGuid, &gEdkiiRngAlgorithmUnSafe))
+    {
+      // Treat zero GUID as an unsafe algorithm
       DEBUG ((
         DEBUG_WARN,
-        "PcdCpuRngSupportedAlgorithm should be a non-zero GUID\n"
+        "RngLib uses an Unsafe algorithm and "
+        "must not be used for production builds.\n"
         ));
+      // Set the UnSafeAlgo flag to indicate an unsafe algorithm was found
+      // so that it can be added at the end of the algorithm list.
+      UnSafeAlgo = TRUE;
+    } else {
+      CopyMem (
+        &mAvailableAlgoArray[mAvailableAlgoArrayCount],
+        &RngGuid,
+        sizeof (RngGuid)
+        );
+      mAvailableAlgoArrayCount++;
     }
-
-    DEBUG_CODE_END ();
   }
 
   // Raw algorithm (Trng)
@@ -68,5 +78,15 @@ GetAvailableAlgorithms (
     mAvailableAlgoArrayCount++;
   }
 
+  // Add unsafe algorithm at the end of the list.
+  if (UnSafeAlgo) {
+    CopyMem (
+      &mAvailableAlgoArray[mAvailableAlgoArrayCount],
+      &gEdkiiRngAlgorithmUnSafe,
+      sizeof (EFI_RNG_ALGORITHM)
+      );
+    mAvailableAlgoArrayCount++;
+  }
+
   return EFI_SUCCESS;
 }
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
index ce49ff7ae661..78a18c5e1177 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
@@ -78,6 +78,7 @@ RngGetRNG (
 {
   EFI_STATUS  Status;
   UINTN       Index;
+  GUID        RngGuid;
 
   if ((This == NULL) || (RNGValueLength == 0) || (RNGValue == NULL)) {
     return EFI_INVALID_PARAMETER;
@@ -102,7 +103,10 @@ RngGetRNG (
   }
 
 FoundAlgo:
-  if (CompareGuid (RNGAlgorithm, PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
+  Status = GetRngGuid (&RngGuid);
+  if (!EFI_ERROR (Status) &&
+      CompareGuid (RNGAlgorithm, &RngGuid))
+  {
     Status = RngGetBytes (RNGValueLength, RNGValue);
     return Status;
   }
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
index d6c2d30195bf..27d3e39a675b 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
@@ -75,13 +75,12 @@ [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
+  gEfiRngAlgorithmArmRndr             ## SOMETIMES_PRODUCES    ## GUID        # Unique ID of the algorithm for RNG
+  gEdkiiRngAlgorithmUnSafe            ## SOMETIMES_PRODUCES    ## GUID        # Unique ID of the algorithm for RNG
 
 [Protocols]
   gEfiRngProtocolGuid                ## PRODUCES
 
-[Pcd.AARCH64]
-  gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm      ## CONSUMES
-
 [Depex]
   TRUE
 
-- 
2.25.1


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

* [PATCH v3 6/6] SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm
  2023-07-06  8:51 [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification PierreGondois
                   ` (4 preceding siblings ...)
  2023-07-06  8:51 ` [PATCH v3 5/6] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib PierreGondois
@ 2023-07-06  8:51 ` PierreGondois
  2023-07-06 19:01 ` [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification Kun Qin
  2023-07-07  8:26 ` Sami Mujawar
  7 siblings, 0 replies; 20+ messages in thread
From: PierreGondois @ 2023-07-06  8:51 UTC (permalink / raw)
  To: devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

From: Pierre Gondois <pierre.gondois@arm.com>

The first element of mAvailableAlgoArray is defined as the default
Rng algorithm to use. Don't go through the array at each RngGetRNG()
call and just return the first element of the array.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
 .../RandomNumberGenerator/RngDxe/ArmRngDxe.c    | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
index 78a18c5e1177..7a42e3cbe3d2 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
@@ -77,7 +77,6 @@ RngGetRNG (
   )
 {
   EFI_STATUS  Status;
-  UINTN       Index;
   GUID        RngGuid;
 
   if ((This == NULL) || (RNGValueLength == 0) || (RNGValue == NULL)) {
@@ -88,21 +87,13 @@ RngGetRNG (
     //
     // Use the default RNG algorithm if RNGAlgorithm is NULL.
     //
-    for (Index = 0; Index < mAvailableAlgoArrayCount; Index++) {
-      if (!IsZeroGuid (&mAvailableAlgoArray[Index])) {
-        RNGAlgorithm = &mAvailableAlgoArray[Index];
-        goto FoundAlgo;
-      }
-    }
-
-    if (Index == mAvailableAlgoArrayCount) {
-      // No algorithm available.
-      ASSERT (Index != mAvailableAlgoArrayCount);
-      return EFI_DEVICE_ERROR;
+    if (mAvailableAlgoArrayCount != 0) {
+      RNGAlgorithm = &mAvailableAlgoArray[0];
+    } else {
+      return EFI_UNSUPPORTED;
     }
   }
 
-FoundAlgo:
   Status = GetRngGuid (&RngGuid);
   if (!EFI_ERROR (Status) &&
       CompareGuid (RNGAlgorithm, &RngGuid))
-- 
2.25.1


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

* Re: [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification
  2023-07-06  8:51 [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification PierreGondois
                   ` (5 preceding siblings ...)
  2023-07-06  8:51 ` [PATCH v3 6/6] SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm PierreGondois
@ 2023-07-06 19:01 ` Kun Qin
  2023-07-12 13:38   ` PierreGondois
  2023-07-07  8:26 ` Sami Mujawar
  7 siblings, 1 reply; 20+ messages in thread
From: Kun Qin @ 2023-07-06 19:01 UTC (permalink / raw)
  To: pierre.gondois, devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ard Biesheuvel, Sami Mujawar, Jose Marinho

[-- Attachment #1: Type: text/plain, Size: 4264 bytes --]

Hi Pierre,

Thanks for sending the update. I tested on QEMU with this change (no 
TRNG from TFA), it works for me.
Tested-by: Kun Qin <kun.qin@microsoft.com>

Please note that the change below is still needed to avoid data abortion 
exception. It will be helpful if one
of the maintainers can help merging it.
[PATCH v2 1/1] SecurityPkg: RngDxe: Fixing mAvailableAlgoArray allocator 
(groups.io) <https://edk2.groups.io/g/devel/message/106547>

Regards,
Kun

On 7/6/2023 1:51 AM, pierre.gondois@arm.com wrote:
> From: Pierre Gondois<pierre.gondois@arm.com>
>
> v3:
> - As the unsafe algorithm GUID will not be added to the UEFI
>    specification, rename:
>    - gEfiRngAlgorithmUnSafe to gEdkiiRngAlgorithmUnSafe
>    - EFI_RNG_ALGORITHM_UNSAFE to EDKII_RNG_ALGORITHM_UNSAFE
>
> v2:
> [1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation
> - Dropped
> [2/8] MdePkg/MdePkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg
> - Change gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm
>    token number
> - Rename to SecurityPkg/SecurityPkg.dec: Move
>    PcdCpuRngSupportedAlgorithm to MdePkg
> [5/8] MdePkg/Rng: Add GetRngGuid() to RngLib
> - Remove gEfiRngAlgorithmUnSafe from inf file
> - Split Guids definitions in arch specific sections
> [6/8] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
> - Remove RngFindDefaultAlgo() and change logic accordingly.
> [7/8] SecurityPkg/RngDxe: Select safe default Rng algorithm
> - Dropped due to changes in [6/8]
>
> This patch also requires the following patch on top of the serie:
> -https://edk2.groups.io/g/devel/message/106546
>
> This patchset follows the 'code first' approach and relates to [1].
> This patchset follows the thread at [3] that aims to solve [2].
> [1] and [2] are bound and this patchset aims to solve both.
>
> In this patchset:
> a-
> The RngDxe can rely on the RngLib. However the RngLib has no
> interface allowing to describe which Rng algorithm is implemented.
> The RngDxe must advertise the algorithm that are available through
> the RngGetInfo() callback.
> Add a GetRngGuid() for interface to the RngLib.
>
> b-
> The Arm Architecture states the RNDR that the DRBG algorithm should
> be compliant with NIST SP800-90A, while not mandating a particular
> algorithm, so as to be inclusive of different geographies.
> The RngLib can rely on this Arm RNDR instruction. In order to
> accurately describe the implementation using the RNDR instruction,
> add a EFI_RNG_ALGORITHM_ARM_RNDR GUID [1].
>
> c-
> For the same reason as a/b, add a GUID describing unsafe RNG
> algorithms, allowing to accurately describe the BaseRngLibTimerLib.
>
> d-
> Use a/b/c mechanisms/GUIDs to select a safe Rng algorithm in the
> Arm implementation of the RngDxe.
>
> [1] BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> [2] BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4151
> [3]https://edk2.groups.io/g/devel/message/100806
>
> Pierre Gondois (6):
>    SecurityPkg/SecurityPkg.dec: Move PcdCpuRngSupportedAlgorithm to
>      MdePkg
>    MdePkg/DxeRngLib: Request raw algorithm instead of default
>    MdePkg/Rng: Add GUIDs to describe Rng algorithms
>    MdePkg/Rng: Add GetRngGuid() to RngLib
>    SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
>    SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm
>
>   MdePkg/Include/Library/RngLib.h               | 17 ++++++
>   MdePkg/Include/Protocol/Rng.h                 | 20 +++++++
>   MdePkg/Library/BaseRngLib/AArch64/Rndr.c      | 42 +++++++++++++++
>   MdePkg/Library/BaseRngLib/BaseRngLib.inf      | 10 ++++
>   MdePkg/Library/BaseRngLib/Rand/RdRand.c       | 26 +++++++++
>   .../Library/BaseRngLibNull/BaseRngLibNull.c   | 22 ++++++++
>   .../BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  3 ++
>   .../Library/BaseRngLibTimerLib/RngLibTimer.c  | 28 ++++++++++
>   MdePkg/Library/DxeRngLib/DxeRngLib.c          | 36 ++++++++++++-
>   MdePkg/MdePkg.dec                             |  7 +++
>   .../RngDxe/AArch64/AArch64Algo.c              | 54 +++++++++++++------
>   .../RandomNumberGenerator/RngDxe/ArmRngDxe.c  | 23 ++++----
>   .../RandomNumberGenerator/RngDxe/RngDxe.inf   |  5 +-
>   SecurityPkg/SecurityPkg.dec                   |  2 -
>   14 files changed, 258 insertions(+), 37 deletions(-)
>

[-- Attachment #2: Type: text/html, Size: 5188 bytes --]

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

* Re: [PATCH v3 5/6] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
  2023-07-06  8:51 ` [PATCH v3 5/6] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib PierreGondois
@ 2023-07-07  8:07   ` Sami Mujawar
  0 siblings, 0 replies; 20+ messages in thread
From: Sami Mujawar @ 2023-07-07  8:07 UTC (permalink / raw)
  To: pierre.gondois, devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ard Biesheuvel, Jose Marinho, Kun Qin, nd@arm.com

Hi Pierre,

Thank you for this patch.

These changes look good to me.

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

Regards,

Sami Mujawar

On 06/07/2023 09:51 am, pierre.gondois@arm.com wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4151
>
> The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
> implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
> To allow the RngDxe to detect when such implementation is used,
> a GetRngGuid() function was added in a previous patch.
>
> The EFI_RNG_PROTOCOL can advertise multiple algorithms through
> Guids. The PcdCpuRngSupportedAlgorithm is currently used to
> advertise the RngLib in the Arm implementation.
>
> The issues of doing that are:
> - the RngLib implementation might not use CPU instructions,
>    cf. the BaseRngLibTimerLib
> - most platforms don't set PcdCpuRngSupportedAlgorithm
>
> A GetRngGuid() was added to the RngLib in a previous patch,
> allowing to identify the algorithm implemented by the RngLib.
> Make use of this function and place the unsage algorithm
> at the last position in the mAvailableAlgoArray.
>
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>   .../RngDxe/AArch64/AArch64Algo.c              | 54 +++++++++++++------
>   .../RandomNumberGenerator/RngDxe/ArmRngDxe.c  |  6 ++-
>   .../RandomNumberGenerator/RngDxe/RngDxe.inf   |  5 +-
>   3 files changed, 44 insertions(+), 21 deletions(-)
>
> diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
> index e8be217f8a8c..d355f575d5c8 100644
> --- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
> +++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
> @@ -10,6 +10,7 @@
>   #include <Library/DebugLib.h>
>   #include <Library/MemoryAllocationLib.h>
>   #include <Library/ArmTrngLib.h>
> +#include <Library/RngLib.h>
>   
>   #include "RngDxeInternals.h"
>   
> @@ -28,9 +29,13 @@ GetAvailableAlgorithms (
>     VOID
>     )
>   {
> -  UINT64  DummyRand;
> -  UINT16  MajorRevision;
> -  UINT16  MinorRevision;
> +  EFI_STATUS  Status;
> +  UINT16      MajorRevision;
> +  UINT16      MinorRevision;
> +  GUID        RngGuid;
> +  BOOLEAN     UnSafeAlgo;
> +
> +  UnSafeAlgo = FALSE;
>   
>     // Rng algorithms 2 times, one for the allocation, one to populate.
>     mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX);
> @@ -38,24 +43,29 @@ GetAvailableAlgorithms (
>       return EFI_OUT_OF_RESOURCES;
>     }
>   
> -  // Check RngGetBytes() before advertising PcdCpuRngSupportedAlgorithm.
> -  if (!EFI_ERROR (RngGetBytes (sizeof (DummyRand), (UINT8 *)&DummyRand))) {
> -    CopyMem (
> -      &mAvailableAlgoArray[mAvailableAlgoArrayCount],
> -      PcdGetPtr (PcdCpuRngSupportedAlgorithm),
> -      sizeof (EFI_RNG_ALGORITHM)
> -      );
> -    mAvailableAlgoArrayCount++;
> -
> -    DEBUG_CODE_BEGIN ();
> -    if (IsZeroGuid (PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
> +  // Identify RngLib algorithm.
> +  Status = GetRngGuid (&RngGuid);
> +  if (!EFI_ERROR (Status)) {
> +    if (IsZeroGuid (&RngGuid) ||
> +        CompareGuid (&RngGuid, &gEdkiiRngAlgorithmUnSafe))
> +    {
> +      // Treat zero GUID as an unsafe algorithm
>         DEBUG ((
>           DEBUG_WARN,
> -        "PcdCpuRngSupportedAlgorithm should be a non-zero GUID\n"
> +        "RngLib uses an Unsafe algorithm and "
> +        "must not be used for production builds.\n"
>           ));
> +      // Set the UnSafeAlgo flag to indicate an unsafe algorithm was found
> +      // so that it can be added at the end of the algorithm list.
> +      UnSafeAlgo = TRUE;
> +    } else {
> +      CopyMem (
> +        &mAvailableAlgoArray[mAvailableAlgoArrayCount],
> +        &RngGuid,
> +        sizeof (RngGuid)
> +        );
> +      mAvailableAlgoArrayCount++;
>       }
> -
> -    DEBUG_CODE_END ();
>     }
>   
>     // Raw algorithm (Trng)
> @@ -68,5 +78,15 @@ GetAvailableAlgorithms (
>       mAvailableAlgoArrayCount++;
>     }
>   
> +  // Add unsafe algorithm at the end of the list.
> +  if (UnSafeAlgo) {
> +    CopyMem (
> +      &mAvailableAlgoArray[mAvailableAlgoArrayCount],
> +      &gEdkiiRngAlgorithmUnSafe,
> +      sizeof (EFI_RNG_ALGORITHM)
> +      );
> +    mAvailableAlgoArrayCount++;
> +  }
> +
>     return EFI_SUCCESS;
>   }
> diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
> index ce49ff7ae661..78a18c5e1177 100644
> --- a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
> +++ b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
> @@ -78,6 +78,7 @@ RngGetRNG (
>   {
>     EFI_STATUS  Status;
>     UINTN       Index;
> +  GUID        RngGuid;
>   
>     if ((This == NULL) || (RNGValueLength == 0) || (RNGValue == NULL)) {
>       return EFI_INVALID_PARAMETER;
> @@ -102,7 +103,10 @@ RngGetRNG (
>     }
>   
>   FoundAlgo:
> -  if (CompareGuid (RNGAlgorithm, PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
> +  Status = GetRngGuid (&RngGuid);
> +  if (!EFI_ERROR (Status) &&
> +      CompareGuid (RNGAlgorithm, &RngGuid))
> +  {
>       Status = RngGetBytes (RNGValueLength, RNGValue);
>       return Status;
>     }
> diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
> index d6c2d30195bf..27d3e39a675b 100644
> --- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
> +++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
> @@ -75,13 +75,12 @@ [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
> +  gEfiRngAlgorithmArmRndr             ## SOMETIMES_PRODUCES    ## GUID        # Unique ID of the algorithm for RNG
> +  gEdkiiRngAlgorithmUnSafe            ## SOMETIMES_PRODUCES    ## GUID        # Unique ID of the algorithm for RNG
>   
>   [Protocols]
>     gEfiRngProtocolGuid                ## PRODUCES
>   
> -[Pcd.AARCH64]
> -  gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm      ## CONSUMES
> -
>   [Depex]
>     TRUE
>   

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

* Re: [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification
  2023-07-06  8:51 [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification PierreGondois
                   ` (6 preceding siblings ...)
  2023-07-06 19:01 ` [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification Kun Qin
@ 2023-07-07  8:26 ` Sami Mujawar
  7 siblings, 0 replies; 20+ messages in thread
From: Sami Mujawar @ 2023-07-07  8:26 UTC (permalink / raw)
  To: pierre.gondois, devel, Michael D Kinney, Liming Gao, Zhiguang Liu,
	Jiewen Yao, Jian J Wang, Ard Biesheuvel
  Cc: Jose Marinho, Kun Qin, nd@arm.com

Dear MdePkg & SecurityPkg maintainers,

This series and Kun's patch at 
https://edk2.groups.io/g/devel/message/106547 are both required to fix 
the RNG implementation for Arm.

Is it possible to provide feedback for this series and Kun's patch, please?

I plan to merge this series and Kun's patch, if there is no further 
feedback by end of next week.

Regards,

Sami Mujawar

On 06/07/2023 09:51 am, pierre.gondois@arm.com wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> v3:
> - As the unsafe algorithm GUID will not be added to the UEFI
>    specification, rename:
>    - gEfiRngAlgorithmUnSafe to gEdkiiRngAlgorithmUnSafe
>    - EFI_RNG_ALGORITHM_UNSAFE to EDKII_RNG_ALGORITHM_UNSAFE
>
> v2:
> [1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation
> - Dropped
> [2/8] MdePkg/MdePkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg
> - Change gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm
>    token number
> - Rename to SecurityPkg/SecurityPkg.dec: Move
>    PcdCpuRngSupportedAlgorithm to MdePkg
> [5/8] MdePkg/Rng: Add GetRngGuid() to RngLib
> - Remove gEfiRngAlgorithmUnSafe from inf file
> - Split Guids definitions in arch specific sections
> [6/8] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
> - Remove RngFindDefaultAlgo() and change logic accordingly.
> [7/8] SecurityPkg/RngDxe: Select safe default Rng algorithm
> - Dropped due to changes in [6/8]
>
> This patch also requires the following patch on top of the serie:
> - https://edk2.groups.io/g/devel/message/106546
>
> This patchset follows the 'code first' approach and relates to [1].
> This patchset follows the thread at [3] that aims to solve [2].
> [1] and [2] are bound and this patchset aims to solve both.
>
> In this patchset:
> a-
> The RngDxe can rely on the RngLib. However the RngLib has no
> interface allowing to describe which Rng algorithm is implemented.
> The RngDxe must advertise the algorithm that are available through
> the RngGetInfo() callback.
> Add a GetRngGuid() for interface to the RngLib.
>
> b-
> The Arm Architecture states the RNDR that the DRBG algorithm should
> be compliant with NIST SP800-90A, while not mandating a particular
> algorithm, so as to be inclusive of different geographies.
> The RngLib can rely on this Arm RNDR instruction. In order to
> accurately describe the implementation using the RNDR instruction,
> add a EFI_RNG_ALGORITHM_ARM_RNDR GUID [1].
>
> c-
> For the same reason as a/b, add a GUID describing unsafe RNG
> algorithms, allowing to accurately describe the BaseRngLibTimerLib.
>
> d-
> Use a/b/c mechanisms/GUIDs to select a safe Rng algorithm in the
> Arm implementation of the RngDxe.
>
> [1] BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> [2] BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4151
> [3] https://edk2.groups.io/g/devel/message/100806
>
> Pierre Gondois (6):
>    SecurityPkg/SecurityPkg.dec: Move PcdCpuRngSupportedAlgorithm to
>      MdePkg
>    MdePkg/DxeRngLib: Request raw algorithm instead of default
>    MdePkg/Rng: Add GUIDs to describe Rng algorithms
>    MdePkg/Rng: Add GetRngGuid() to RngLib
>    SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
>    SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm
>
>   MdePkg/Include/Library/RngLib.h               | 17 ++++++
>   MdePkg/Include/Protocol/Rng.h                 | 20 +++++++
>   MdePkg/Library/BaseRngLib/AArch64/Rndr.c      | 42 +++++++++++++++
>   MdePkg/Library/BaseRngLib/BaseRngLib.inf      | 10 ++++
>   MdePkg/Library/BaseRngLib/Rand/RdRand.c       | 26 +++++++++
>   .../Library/BaseRngLibNull/BaseRngLibNull.c   | 22 ++++++++
>   .../BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  3 ++
>   .../Library/BaseRngLibTimerLib/RngLibTimer.c  | 28 ++++++++++
>   MdePkg/Library/DxeRngLib/DxeRngLib.c          | 36 ++++++++++++-
>   MdePkg/MdePkg.dec                             |  7 +++
>   .../RngDxe/AArch64/AArch64Algo.c              | 54 +++++++++++++------
>   .../RandomNumberGenerator/RngDxe/ArmRngDxe.c  | 23 ++++----
>   .../RandomNumberGenerator/RngDxe/RngDxe.inf   |  5 +-
>   SecurityPkg/SecurityPkg.dec                   |  2 -
>   14 files changed, 258 insertions(+), 37 deletions(-)
>

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

* Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms
  2023-07-06  8:51 ` [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms PierreGondois
@ 2023-07-07  9:14   ` Yao, Jiewen
  2023-07-07 12:49     ` PierreGondois
  0 siblings, 1 reply; 20+ messages in thread
From: Yao, Jiewen @ 2023-07-07  9:14 UTC (permalink / raw)
  To: devel@edk2.groups.io, pierre.gondois@arm.com
  Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang, Wang, Jian J,
	Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

MdePkg can only add UEFI defined API.

Is below defined by UEFI?

Thank you
Yao, Jiewen



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> PierreGondois
> Sent: Thursday, July 6, 2023 4:52 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
> <sami.mujawar@arm.com>; Jose Marinho <Jose.Marinho@arm.com>; Kun Qin
> <kuqin12@gmail.com>
> Subject: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng
> algorithms
> 
> From: Pierre Gondois <pierre.gondois@arm.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> 
> The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
> implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
> To allow the RngDxe to detect when such implementation is used,
> a GetRngGuid() function is added in a following patch.
> 
> Prepare GetRngGuid() return values and add GUIDs describing
> Rng algorithms:
> - gEfiRngAlgorithmArmRndr
> to describe a Rng algorithm accessed through Arm's RNDR instruction.
> [1] states that the implementation of this algorithm should be
> compliant to NIST SP900-80. The compliance is not guaranteed.
> - gEdkiiRngAlgorithmUnSafe
> to describe an unsafe implementation, cf. the BaseRngLibTimerLib.
> 
> [1] Arm Architecture Reference Manual Armv8, for A-profile architecture
> sK12.1 'Properties of the generated random number'
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>  MdePkg/Include/Protocol/Rng.h | 20 ++++++++++++++++++++
>  MdePkg/MdePkg.dec             |  2 ++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/MdePkg/Include/Protocol/Rng.h b/MdePkg/Include/Protocol/Rng.h
> index baf425587b3c..ceae77ba9c73 100644
> --- a/MdePkg/Include/Protocol/Rng.h
> +++ b/MdePkg/Include/Protocol/Rng.h
> @@ -67,6 +67,24 @@ typedef EFI_GUID EFI_RNG_ALGORITHM;
>    { \
>      0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }
> \
>    }
> +///
> +/// The Arm Architecture states the RNDR that the DRBG algorithm should be
> compliant
> +/// with NIST SP800-90A, while not mandating a particular algorithm, so as to
> be
> +/// inclusive of different geographies.
> +///
> +#define EFI_RNG_ALGORITHM_ARM_RNDR \
> +  { \
> +    0x43d2fde3, 0x9d4e, 0x4d79,  {0x02, 0x96, 0xa8, 0x9b, 0xca, 0x78, 0x08,
> 0x41} \
> +  }
> +///
> +/// The implementation of a Random Number Generator might be unsafe,
> when using
> +/// a dummy implementation for instance. Allow identifying such
> implementation
> +/// with this GUID.
> +///
> +#define EDKII_RNG_ALGORITHM_UNSAFE \
> +  { \
> +    0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09, 0xc1, 0xb3,
> 0xf4 } \
> +  }
> 
>  /**
>    Returns information about the random number generation implementation.
> @@ -146,5 +164,7 @@ extern EFI_GUID  gEfiRngAlgorithmSp80090Ctr256Guid;
>  extern EFI_GUID  gEfiRngAlgorithmX9313DesGuid;
>  extern EFI_GUID  gEfiRngAlgorithmX931AesGuid;
>  extern EFI_GUID  gEfiRngAlgorithmRaw;
> +extern EFI_GUID  gEfiRngAlgorithmArmRndr;
> +extern EFI_GUID  gEdkiiRngAlgorithmUnSafe;
> 
>  #endif
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index 5b8477f4cb8f..2c8f985f253e 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -643,6 +643,8 @@ [Guids]
>    gEfiRngAlgorithmX9313DesGuid       = { 0x63c4785a, 0xca34, 0x4012, {0xa3,
> 0xc8, 0x0b, 0x6a, 0x32, 0x4f, 0x55, 0x46 }}
>    gEfiRngAlgorithmX931AesGuid        = { 0xacd03321, 0x777e, 0x4d3d, {0xb1,
> 0xc8, 0x20, 0xcf, 0xd8, 0x88, 0x20, 0xc9 }}
>    gEfiRngAlgorithmRaw                = { 0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84,
> 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }}
> +  gEfiRngAlgorithmArmRndr            = { 0x43d2fde3, 0x9d4e, 0x4d79, {0x02, 0x96,
> 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41 }}
> +  gEdkiiRngAlgorithmUnSafe           = { 0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03,
> 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 }}
> 
>    ## Include/Protocol/AdapterInformation.h
>    gEfiAdapterInfoMediaStateGuid       = { 0xD7C74207, 0xA831, 0x4A26, {0xB1,
> 0xF5, 0xD1, 0x93, 0x06, 0x5C, 0xE8, 0xB6 }}
> --
> 2.25.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#106688): https://edk2.groups.io/g/devel/message/106688
> Mute This Topic: https://groups.io/mt/99981855/1772286
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiewen.yao@intel.com]
> -=-=-=-=-=-=
> 


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

* Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms
  2023-07-07  9:14   ` [edk2-devel] " Yao, Jiewen
@ 2023-07-07 12:49     ` PierreGondois
  2023-07-07 12:56       ` Yao, Jiewen
       [not found]       ` <176F972B57840483.2683@groups.io>
  0 siblings, 2 replies; 20+ messages in thread
From: PierreGondois @ 2023-07-07 12:49 UTC (permalink / raw)
  To: devel, jiewen.yao
  Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang, Wang, Jian J,
	Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

Hello Jiewen,

The gEfiRngAlgorithmArmRndr GUID is to be added to the UEFI spec with:
- https://bugzilla.tianocore.org/show_bug.cgi?id=4441
- https://mantis.uefi.org/mantis/view.php?id=2386

the gEdkiiMemoryAcceptProtocolGuid GUID should not be in the UEFI spec,
so I used the 'gEdkii' prefix as already used in MdePkg.dec for:
- gEdkiiMemoryAcceptProtocolGuid

Regards,
Pierre

On 7/7/23 11:14, Yao, Jiewen via groups.io wrote:
> MdePkg can only add UEFI defined API.
> 
> Is below defined by UEFI?
> 
> Thank you
> Yao, Jiewen
> 
> 
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>> PierreGondois
>> Sent: Thursday, July 6, 2023 4:52 PM
>> To: devel@edk2.groups.io
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Yao,
>> Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Ard
>> Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
>> <sami.mujawar@arm.com>; Jose Marinho <Jose.Marinho@arm.com>; Kun Qin
>> <kuqin12@gmail.com>
>> Subject: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng
>> algorithms
>>
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
>>
>> The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
>> implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
>> To allow the RngDxe to detect when such implementation is used,
>> a GetRngGuid() function is added in a following patch.
>>
>> Prepare GetRngGuid() return values and add GUIDs describing
>> Rng algorithms:
>> - gEfiRngAlgorithmArmRndr
>> to describe a Rng algorithm accessed through Arm's RNDR instruction.
>> [1] states that the implementation of this algorithm should be
>> compliant to NIST SP900-80. The compliance is not guaranteed.
>> - gEdkiiRngAlgorithmUnSafe
>> to describe an unsafe implementation, cf. the BaseRngLibTimerLib.
>>
>> [1] Arm Architecture Reference Manual Armv8, for A-profile architecture
>> sK12.1 'Properties of the generated random number'
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>> ---
>>   MdePkg/Include/Protocol/Rng.h | 20 ++++++++++++++++++++
>>   MdePkg/MdePkg.dec             |  2 ++
>>   2 files changed, 22 insertions(+)
>>
>> diff --git a/MdePkg/Include/Protocol/Rng.h b/MdePkg/Include/Protocol/Rng.h
>> index baf425587b3c..ceae77ba9c73 100644
>> --- a/MdePkg/Include/Protocol/Rng.h
>> +++ b/MdePkg/Include/Protocol/Rng.h
>> @@ -67,6 +67,24 @@ typedef EFI_GUID EFI_RNG_ALGORITHM;
>>     { \
>>       0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }
>> \
>>     }
>> +///
>> +/// The Arm Architecture states the RNDR that the DRBG algorithm should be
>> compliant
>> +/// with NIST SP800-90A, while not mandating a particular algorithm, so as to
>> be
>> +/// inclusive of different geographies.
>> +///
>> +#define EFI_RNG_ALGORITHM_ARM_RNDR \
>> +  { \
>> +    0x43d2fde3, 0x9d4e, 0x4d79,  {0x02, 0x96, 0xa8, 0x9b, 0xca, 0x78, 0x08,
>> 0x41} \
>> +  }
>> +///
>> +/// The implementation of a Random Number Generator might be unsafe,
>> when using
>> +/// a dummy implementation for instance. Allow identifying such
>> implementation
>> +/// with this GUID.
>> +///
>> +#define EDKII_RNG_ALGORITHM_UNSAFE \
>> +  { \
>> +    0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09, 0xc1, 0xb3,
>> 0xf4 } \
>> +  }
>>
>>   /**
>>     Returns information about the random number generation implementation.
>> @@ -146,5 +164,7 @@ extern EFI_GUID  gEfiRngAlgorithmSp80090Ctr256Guid;
>>   extern EFI_GUID  gEfiRngAlgorithmX9313DesGuid;
>>   extern EFI_GUID  gEfiRngAlgorithmX931AesGuid;
>>   extern EFI_GUID  gEfiRngAlgorithmRaw;
>> +extern EFI_GUID  gEfiRngAlgorithmArmRndr;
>> +extern EFI_GUID  gEdkiiRngAlgorithmUnSafe;
>>
>>   #endif
>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>> index 5b8477f4cb8f..2c8f985f253e 100644
>> --- a/MdePkg/MdePkg.dec
>> +++ b/MdePkg/MdePkg.dec
>> @@ -643,6 +643,8 @@ [Guids]
>>     gEfiRngAlgorithmX9313DesGuid       = { 0x63c4785a, 0xca34, 0x4012, {0xa3,
>> 0xc8, 0x0b, 0x6a, 0x32, 0x4f, 0x55, 0x46 }}
>>     gEfiRngAlgorithmX931AesGuid        = { 0xacd03321, 0x777e, 0x4d3d, {0xb1,
>> 0xc8, 0x20, 0xcf, 0xd8, 0x88, 0x20, 0xc9 }}
>>     gEfiRngAlgorithmRaw                = { 0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84,
>> 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }}
>> +  gEfiRngAlgorithmArmRndr            = { 0x43d2fde3, 0x9d4e, 0x4d79, {0x02, 0x96,
>> 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41 }}
>> +  gEdkiiRngAlgorithmUnSafe           = { 0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03,
>> 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 }}
>>
>>     ## Include/Protocol/AdapterInformation.h
>>     gEfiAdapterInfoMediaStateGuid       = { 0xD7C74207, 0xA831, 0x4A26, {0xB1,
>> 0xF5, 0xD1, 0x93, 0x06, 0x5C, 0xE8, 0xB6 }}
>> --
>> 2.25.1
>>
>>
>>
>> -=-=-=-=-=-=
>> Groups.io Links: You receive all messages sent to this group.
>> View/Reply Online (#106688): https://edk2.groups.io/g/devel/message/106688
>> Mute This Topic: https://groups.io/mt/99981855/1772286
>> Group Owner: devel+owner@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiewen.yao@intel.com]
>> -=-=-=-=-=-=
>>
> 
> 
> 
> 
> 
> 

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

* Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms
  2023-07-07 12:49     ` PierreGondois
@ 2023-07-07 12:56       ` Yao, Jiewen
       [not found]       ` <176F972B57840483.2683@groups.io>
  1 sibling, 0 replies; 20+ messages in thread
From: Yao, Jiewen @ 2023-07-07 12:56 UTC (permalink / raw)
  To: Pierre Gondois, devel@edk2.groups.io
  Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang, Wang, Jian J,
	Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

I don’t think MdePkg should have Edkii- style protocol.

I am not sure why gEdkiiMemoryAcceptProtocolGuid is in MdePkg.
It should be in MdeModulePkg, IMHO.

Thank you
Yao, Jiewen

> -----Original Message-----
> From: Pierre Gondois <pierre.gondois@arm.com>
> Sent: Friday, July 7, 2023 8:49 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
> Rng algorithms
> 
> Hello Jiewen,
> 
> The gEfiRngAlgorithmArmRndr GUID is to be added to the UEFI spec with:
> - https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> - https://mantis.uefi.org/mantis/view.php?id=2386
> 
> the gEdkiiMemoryAcceptProtocolGuid GUID should not be in the UEFI spec,
> so I used the 'gEdkii' prefix as already used in MdePkg.dec for:
> - gEdkiiMemoryAcceptProtocolGuid
> 
> Regards,
> Pierre
> 
> On 7/7/23 11:14, Yao, Jiewen via groups.io wrote:
> > MdePkg can only add UEFI defined API.
> >
> > Is below defined by UEFI?
> >
> > Thank you
> > Yao, Jiewen
> >
> >
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> >> PierreGondois
> >> Sent: Thursday, July 6, 2023 4:52 PM
> >> To: devel@edk2.groups.io
> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Yao,
> >> Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Ard
> >> Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
> >> <sami.mujawar@arm.com>; Jose Marinho <Jose.Marinho@arm.com>; Kun
> Qin
> >> <kuqin12@gmail.com>
> >> Subject: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
> Rng
> >> algorithms
> >>
> >> From: Pierre Gondois <pierre.gondois@arm.com>
> >>
> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> >>
> >> The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
> >> implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
> >> To allow the RngDxe to detect when such implementation is used,
> >> a GetRngGuid() function is added in a following patch.
> >>
> >> Prepare GetRngGuid() return values and add GUIDs describing
> >> Rng algorithms:
> >> - gEfiRngAlgorithmArmRndr
> >> to describe a Rng algorithm accessed through Arm's RNDR instruction.
> >> [1] states that the implementation of this algorithm should be
> >> compliant to NIST SP900-80. The compliance is not guaranteed.
> >> - gEdkiiRngAlgorithmUnSafe
> >> to describe an unsafe implementation, cf. the BaseRngLibTimerLib.
> >>
> >> [1] Arm Architecture Reference Manual Armv8, for A-profile architecture
> >> sK12.1 'Properties of the generated random number'
> >>
> >> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> >> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> >> ---
> >>   MdePkg/Include/Protocol/Rng.h | 20 ++++++++++++++++++++
> >>   MdePkg/MdePkg.dec             |  2 ++
> >>   2 files changed, 22 insertions(+)
> >>
> >> diff --git a/MdePkg/Include/Protocol/Rng.h
> b/MdePkg/Include/Protocol/Rng.h
> >> index baf425587b3c..ceae77ba9c73 100644
> >> --- a/MdePkg/Include/Protocol/Rng.h
> >> +++ b/MdePkg/Include/Protocol/Rng.h
> >> @@ -67,6 +67,24 @@ typedef EFI_GUID EFI_RNG_ALGORITHM;
> >>     { \
> >>       0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85,
> 0x61 }
> >> \
> >>     }
> >> +///
> >> +/// The Arm Architecture states the RNDR that the DRBG algorithm should
> be
> >> compliant
> >> +/// with NIST SP800-90A, while not mandating a particular algorithm, so as
> to
> >> be
> >> +/// inclusive of different geographies.
> >> +///
> >> +#define EFI_RNG_ALGORITHM_ARM_RNDR \
> >> +  { \
> >> +    0x43d2fde3, 0x9d4e, 0x4d79,  {0x02, 0x96, 0xa8, 0x9b, 0xca, 0x78, 0x08,
> >> 0x41} \
> >> +  }
> >> +///
> >> +/// The implementation of a Random Number Generator might be unsafe,
> >> when using
> >> +/// a dummy implementation for instance. Allow identifying such
> >> implementation
> >> +/// with this GUID.
> >> +///
> >> +#define EDKII_RNG_ALGORITHM_UNSAFE \
> >> +  { \
> >> +    0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09, 0xc1, 0xb3,
> >> 0xf4 } \
> >> +  }
> >>
> >>   /**
> >>     Returns information about the random number generation implementation.
> >> @@ -146,5 +164,7 @@ extern EFI_GUID
> gEfiRngAlgorithmSp80090Ctr256Guid;
> >>   extern EFI_GUID  gEfiRngAlgorithmX9313DesGuid;
> >>   extern EFI_GUID  gEfiRngAlgorithmX931AesGuid;
> >>   extern EFI_GUID  gEfiRngAlgorithmRaw;
> >> +extern EFI_GUID  gEfiRngAlgorithmArmRndr;
> >> +extern EFI_GUID  gEdkiiRngAlgorithmUnSafe;
> >>
> >>   #endif
> >> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> >> index 5b8477f4cb8f..2c8f985f253e 100644
> >> --- a/MdePkg/MdePkg.dec
> >> +++ b/MdePkg/MdePkg.dec
> >> @@ -643,6 +643,8 @@ [Guids]
> >>     gEfiRngAlgorithmX9313DesGuid       = { 0x63c4785a, 0xca34, 0x4012, {0xa3,
> >> 0xc8, 0x0b, 0x6a, 0x32, 0x4f, 0x55, 0x46 }}
> >>     gEfiRngAlgorithmX931AesGuid        = { 0xacd03321, 0x777e, 0x4d3d, {0xb1,
> >> 0xc8, 0x20, 0xcf, 0xd8, 0x88, 0x20, 0xc9 }}
> >>     gEfiRngAlgorithmRaw                = { 0xe43176d7, 0xb6e8, 0x4827, {0xb7,
> 0x84,
> >> 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }}
> >> +  gEfiRngAlgorithmArmRndr            = { 0x43d2fde3, 0x9d4e, 0x4d79, {0x02,
> 0x96,
> >> 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41 }}
> >> +  gEdkiiRngAlgorithmUnSafe           = { 0x869f728c, 0x409d, 0x4ab4, {0xac,
> 0x03,
> >> 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 }}
> >>
> >>     ## Include/Protocol/AdapterInformation.h
> >>     gEfiAdapterInfoMediaStateGuid       = { 0xD7C74207, 0xA831, 0x4A26,
> {0xB1,
> >> 0xF5, 0xD1, 0x93, 0x06, 0x5C, 0xE8, 0xB6 }}
> >> --
> >> 2.25.1
> >>
> >>
> >>
> >> -=-=-=-=-=-=
> >> Groups.io Links: You receive all messages sent to this group.
> >> View/Reply Online (#106688):
> https://edk2.groups.io/g/devel/message/106688
> >> Mute This Topic: https://groups.io/mt/99981855/1772286
> >> Group Owner: devel+owner@edk2.groups.io
> >> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiewen.yao@intel.com]
> >> -=-=-=-=-=-=
> >>
> >
> >
> >
> > 
> >
> >

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

* Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms
       [not found]       ` <176F972B57840483.2683@groups.io>
@ 2023-07-07 13:05         ` Yao, Jiewen
  2023-07-07 14:25           ` PierreGondois
  0 siblings, 1 reply; 20+ messages in thread
From: Yao, Jiewen @ 2023-07-07 13:05 UTC (permalink / raw)
  To: devel@edk2.groups.io, Yao, Jiewen, Pierre Gondois
  Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang, Wang, Jian J,
	Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

FYI: I filed https://bugzilla.tianocore.org/show_bug.cgi?id=4497 to track the gEdkiiMemoryAcceptProtocolGuid issue.

Thank you
Yao, Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Friday, July 7, 2023 8:57 PM
> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
> Rng algorithms
> 
> I don’t think MdePkg should have Edkii- style protocol.
> 
> I am not sure why gEdkiiMemoryAcceptProtocolGuid is in MdePkg.
> It should be in MdeModulePkg, IMHO.
> 
> Thank you
> Yao, Jiewen
> 
> > -----Original Message-----
> > From: Pierre Gondois <pierre.gondois@arm.com>
> > Sent: Friday, July 7, 2023 8:49 PM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang,
> > Jian J <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> > Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> > <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> > Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
> > Rng algorithms
> >
> > Hello Jiewen,
> >
> > The gEfiRngAlgorithmArmRndr GUID is to be added to the UEFI spec with:
> > - https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> > - https://mantis.uefi.org/mantis/view.php?id=2386
> >
> > the gEdkiiMemoryAcceptProtocolGuid GUID should not be in the UEFI spec,
> > so I used the 'gEdkii' prefix as already used in MdePkg.dec for:
> > - gEdkiiMemoryAcceptProtocolGuid
> >
> > Regards,
> > Pierre
> >
> > On 7/7/23 11:14, Yao, Jiewen via groups.io wrote:
> > > MdePkg can only add UEFI defined API.
> > >
> > > Is below defined by UEFI?
> > >
> > > Thank you
> > > Yao, Jiewen
> > >
> > >
> > >
> > >> -----Original Message-----
> > >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > >> PierreGondois
> > >> Sent: Thursday, July 6, 2023 4:52 PM
> > >> To: devel@edk2.groups.io
> > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> Yao,
> > >> Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Ard
> > >> Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
> > >> <sami.mujawar@arm.com>; Jose Marinho <Jose.Marinho@arm.com>; Kun
> > Qin
> > >> <kuqin12@gmail.com>
> > >> Subject: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
> > Rng
> > >> algorithms
> > >>
> > >> From: Pierre Gondois <pierre.gondois@arm.com>
> > >>
> > >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> > >>
> > >> The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
> > >> implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
> > >> To allow the RngDxe to detect when such implementation is used,
> > >> a GetRngGuid() function is added in a following patch.
> > >>
> > >> Prepare GetRngGuid() return values and add GUIDs describing
> > >> Rng algorithms:
> > >> - gEfiRngAlgorithmArmRndr
> > >> to describe a Rng algorithm accessed through Arm's RNDR instruction.
> > >> [1] states that the implementation of this algorithm should be
> > >> compliant to NIST SP900-80. The compliance is not guaranteed.
> > >> - gEdkiiRngAlgorithmUnSafe
> > >> to describe an unsafe implementation, cf. the BaseRngLibTimerLib.
> > >>
> > >> [1] Arm Architecture Reference Manual Armv8, for A-profile architecture
> > >> sK12.1 'Properties of the generated random number'
> > >>
> > >> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> > >> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> > >> ---
> > >>   MdePkg/Include/Protocol/Rng.h | 20 ++++++++++++++++++++
> > >>   MdePkg/MdePkg.dec             |  2 ++
> > >>   2 files changed, 22 insertions(+)
> > >>
> > >> diff --git a/MdePkg/Include/Protocol/Rng.h
> > b/MdePkg/Include/Protocol/Rng.h
> > >> index baf425587b3c..ceae77ba9c73 100644
> > >> --- a/MdePkg/Include/Protocol/Rng.h
> > >> +++ b/MdePkg/Include/Protocol/Rng.h
> > >> @@ -67,6 +67,24 @@ typedef EFI_GUID EFI_RNG_ALGORITHM;
> > >>     { \
> > >>       0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85,
> > 0x61 }
> > >> \
> > >>     }
> > >> +///
> > >> +/// The Arm Architecture states the RNDR that the DRBG algorithm should
> > be
> > >> compliant
> > >> +/// with NIST SP800-90A, while not mandating a particular algorithm, so as
> > to
> > >> be
> > >> +/// inclusive of different geographies.
> > >> +///
> > >> +#define EFI_RNG_ALGORITHM_ARM_RNDR \
> > >> +  { \
> > >> +    0x43d2fde3, 0x9d4e, 0x4d79,  {0x02, 0x96, 0xa8, 0x9b, 0xca, 0x78, 0x08,
> > >> 0x41} \
> > >> +  }
> > >> +///
> > >> +/// The implementation of a Random Number Generator might be unsafe,
> > >> when using
> > >> +/// a dummy implementation for instance. Allow identifying such
> > >> implementation
> > >> +/// with this GUID.
> > >> +///
> > >> +#define EDKII_RNG_ALGORITHM_UNSAFE \
> > >> +  { \
> > >> +    0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09, 0xc1, 0xb3,
> > >> 0xf4 } \
> > >> +  }
> > >>
> > >>   /**
> > >>     Returns information about the random number generation
> implementation.
> > >> @@ -146,5 +164,7 @@ extern EFI_GUID
> > gEfiRngAlgorithmSp80090Ctr256Guid;
> > >>   extern EFI_GUID  gEfiRngAlgorithmX9313DesGuid;
> > >>   extern EFI_GUID  gEfiRngAlgorithmX931AesGuid;
> > >>   extern EFI_GUID  gEfiRngAlgorithmRaw;
> > >> +extern EFI_GUID  gEfiRngAlgorithmArmRndr;
> > >> +extern EFI_GUID  gEdkiiRngAlgorithmUnSafe;
> > >>
> > >>   #endif
> > >> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> > >> index 5b8477f4cb8f..2c8f985f253e 100644
> > >> --- a/MdePkg/MdePkg.dec
> > >> +++ b/MdePkg/MdePkg.dec
> > >> @@ -643,6 +643,8 @@ [Guids]
> > >>     gEfiRngAlgorithmX9313DesGuid       = { 0x63c4785a, 0xca34, 0x4012,
> {0xa3,
> > >> 0xc8, 0x0b, 0x6a, 0x32, 0x4f, 0x55, 0x46 }}
> > >>     gEfiRngAlgorithmX931AesGuid        = { 0xacd03321, 0x777e, 0x4d3d,
> {0xb1,
> > >> 0xc8, 0x20, 0xcf, 0xd8, 0x88, 0x20, 0xc9 }}
> > >>     gEfiRngAlgorithmRaw                = { 0xe43176d7, 0xb6e8, 0x4827, {0xb7,
> > 0x84,
> > >> 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }}
> > >> +  gEfiRngAlgorithmArmRndr            = { 0x43d2fde3, 0x9d4e, 0x4d79, {0x02,
> > 0x96,
> > >> 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41 }}
> > >> +  gEdkiiRngAlgorithmUnSafe           = { 0x869f728c, 0x409d, 0x4ab4, {0xac,
> > 0x03,
> > >> 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 }}
> > >>
> > >>     ## Include/Protocol/AdapterInformation.h
> > >>     gEfiAdapterInfoMediaStateGuid       = { 0xD7C74207, 0xA831, 0x4A26,
> > {0xB1,
> > >> 0xF5, 0xD1, 0x93, 0x06, 0x5C, 0xE8, 0xB6 }}
> > >> --
> > >> 2.25.1
> > >>
> > >>
> > >>
> > >> -=-=-=-=-=-=
> > >> Groups.io Links: You receive all messages sent to this group.
> > >> View/Reply Online (#106688):
> > https://edk2.groups.io/g/devel/message/106688
> > >> Mute This Topic: https://groups.io/mt/99981855/1772286
> > >> Group Owner: devel+owner@edk2.groups.io
> > >> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiewen.yao@intel.com]
> > >> -=-=-=-=-=-=
> > >>
> > >
> > >
> > >
> > >
> > >
> > >
> 
> 
> 
> 


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

* Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms
  2023-07-07 13:05         ` Yao, Jiewen
@ 2023-07-07 14:25           ` PierreGondois
  2023-07-07 14:28             ` Yao, Jiewen
       [not found]             ` <176F9C2F554052EE.2683@groups.io>
  0 siblings, 2 replies; 20+ messages in thread
From: PierreGondois @ 2023-07-07 14:25 UTC (permalink / raw)
  To: Yao, Jiewen, devel@edk2.groups.io
  Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang, Wang, Jian J,
	Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

Hello Jiewen,

We have the following dependency issue:
- the BaseRngTimerLib is in the MdePkg
- we need a GUID to describe the BaseRngTimerLib algorithm
- we cannot add the gEdkiiRngAlgorithmUnSafe in the MdePkg, and the gZeroGuid is also not in the MdePkg
- the MdePkg should not have dependencies over other packages

As the BaseRngTimerLib is not really standard and should not be used in production builds,
would you agree if it was moved to the MdeModulePkg or to the SecurityPkg (with the gEdkiiRngAlgorithmUnSafe definition) ?

Regards,
Pierre


The issue we have

On 7/7/23 15:05, Yao, Jiewen wrote:
> FYI: I filed https://bugzilla.tianocore.org/show_bug.cgi?id=4497 to track the gEdkiiMemoryAcceptProtocolGuid issue.
> 
> Thank you
> Yao, Jiewen
> 
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
>> Sent: Friday, July 7, 2023 8:57 PM
>> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang,
>> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
>> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
>> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
>> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
>> Rng algorithms
>>
>> I don’t think MdePkg should have Edkii- style protocol.
>>
>> I am not sure why gEdkiiMemoryAcceptProtocolGuid is in MdePkg.
>> It should be in MdeModulePkg, IMHO.
>>
>> Thank you
>> Yao, Jiewen
>>
>>> -----Original Message-----
>>> From: Pierre Gondois <pierre.gondois@arm.com>
>>> Sent: Friday, July 7, 2023 8:49 PM
>>> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang,
>>> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
>>> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
>>> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
>>> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
>>> Rng algorithms
>>>
>>> Hello Jiewen,
>>>
>>> The gEfiRngAlgorithmArmRndr GUID is to be added to the UEFI spec with:
>>> - https://bugzilla.tianocore.org/show_bug.cgi?id=4441
>>> - https://mantis.uefi.org/mantis/view.php?id=2386
>>>
>>> the gEdkiiMemoryAcceptProtocolGuid GUID should not be in the UEFI spec,
>>> so I used the 'gEdkii' prefix as already used in MdePkg.dec for:
>>> - gEdkiiMemoryAcceptProtocolGuid
>>>
>>> Regards,
>>> Pierre
>>>
>>> On 7/7/23 11:14, Yao, Jiewen via groups.io wrote:
>>>> MdePkg can only add UEFI defined API.
>>>>
>>>> Is below defined by UEFI?
>>>>
>>>> Thank you
>>>> Yao, Jiewen
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>>>>> PierreGondois
>>>>> Sent: Thursday, July 6, 2023 4:52 PM
>>>>> To: devel@edk2.groups.io
>>>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>>>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
>> Yao,
>>>>> Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
>> Ard
>>>>> Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
>>>>> <sami.mujawar@arm.com>; Jose Marinho <Jose.Marinho@arm.com>; Kun
>>> Qin
>>>>> <kuqin12@gmail.com>
>>>>> Subject: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
>>> Rng
>>>>> algorithms
>>>>>
>>>>> From: Pierre Gondois <pierre.gondois@arm.com>
>>>>>
>>>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
>>>>>
>>>>> The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
>>>>> implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
>>>>> To allow the RngDxe to detect when such implementation is used,
>>>>> a GetRngGuid() function is added in a following patch.
>>>>>
>>>>> Prepare GetRngGuid() return values and add GUIDs describing
>>>>> Rng algorithms:
>>>>> - gEfiRngAlgorithmArmRndr
>>>>> to describe a Rng algorithm accessed through Arm's RNDR instruction.
>>>>> [1] states that the implementation of this algorithm should be
>>>>> compliant to NIST SP900-80. The compliance is not guaranteed.
>>>>> - gEdkiiRngAlgorithmUnSafe
>>>>> to describe an unsafe implementation, cf. the BaseRngLibTimerLib.
>>>>>
>>>>> [1] Arm Architecture Reference Manual Armv8, for A-profile architecture
>>>>> sK12.1 'Properties of the generated random number'
>>>>>
>>>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>>>>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>>>>> ---
>>>>>    MdePkg/Include/Protocol/Rng.h | 20 ++++++++++++++++++++
>>>>>    MdePkg/MdePkg.dec             |  2 ++
>>>>>    2 files changed, 22 insertions(+)
>>>>>
>>>>> diff --git a/MdePkg/Include/Protocol/Rng.h
>>> b/MdePkg/Include/Protocol/Rng.h
>>>>> index baf425587b3c..ceae77ba9c73 100644
>>>>> --- a/MdePkg/Include/Protocol/Rng.h
>>>>> +++ b/MdePkg/Include/Protocol/Rng.h
>>>>> @@ -67,6 +67,24 @@ typedef EFI_GUID EFI_RNG_ALGORITHM;
>>>>>      { \
>>>>>        0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85,
>>> 0x61 }
>>>>> \
>>>>>      }
>>>>> +///
>>>>> +/// The Arm Architecture states the RNDR that the DRBG algorithm should
>>> be
>>>>> compliant
>>>>> +/// with NIST SP800-90A, while not mandating a particular algorithm, so as
>>> to
>>>>> be
>>>>> +/// inclusive of different geographies.
>>>>> +///
>>>>> +#define EFI_RNG_ALGORITHM_ARM_RNDR \
>>>>> +  { \
>>>>> +    0x43d2fde3, 0x9d4e, 0x4d79,  {0x02, 0x96, 0xa8, 0x9b, 0xca, 0x78, 0x08,
>>>>> 0x41} \
>>>>> +  }
>>>>> +///
>>>>> +/// The implementation of a Random Number Generator might be unsafe,
>>>>> when using
>>>>> +/// a dummy implementation for instance. Allow identifying such
>>>>> implementation
>>>>> +/// with this GUID.
>>>>> +///
>>>>> +#define EDKII_RNG_ALGORITHM_UNSAFE \
>>>>> +  { \
>>>>> +    0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09, 0xc1, 0xb3,
>>>>> 0xf4 } \
>>>>> +  }
>>>>>
>>>>>    /**
>>>>>      Returns information about the random number generation
>> implementation.
>>>>> @@ -146,5 +164,7 @@ extern EFI_GUID
>>> gEfiRngAlgorithmSp80090Ctr256Guid;
>>>>>    extern EFI_GUID  gEfiRngAlgorithmX9313DesGuid;
>>>>>    extern EFI_GUID  gEfiRngAlgorithmX931AesGuid;
>>>>>    extern EFI_GUID  gEfiRngAlgorithmRaw;
>>>>> +extern EFI_GUID  gEfiRngAlgorithmArmRndr;
>>>>> +extern EFI_GUID  gEdkiiRngAlgorithmUnSafe;
>>>>>
>>>>>    #endif
>>>>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>>>>> index 5b8477f4cb8f..2c8f985f253e 100644
>>>>> --- a/MdePkg/MdePkg.dec
>>>>> +++ b/MdePkg/MdePkg.dec
>>>>> @@ -643,6 +643,8 @@ [Guids]
>>>>>      gEfiRngAlgorithmX9313DesGuid       = { 0x63c4785a, 0xca34, 0x4012,
>> {0xa3,
>>>>> 0xc8, 0x0b, 0x6a, 0x32, 0x4f, 0x55, 0x46 }}
>>>>>      gEfiRngAlgorithmX931AesGuid        = { 0xacd03321, 0x777e, 0x4d3d,
>> {0xb1,
>>>>> 0xc8, 0x20, 0xcf, 0xd8, 0x88, 0x20, 0xc9 }}
>>>>>      gEfiRngAlgorithmRaw                = { 0xe43176d7, 0xb6e8, 0x4827, {0xb7,
>>> 0x84,
>>>>> 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }}
>>>>> +  gEfiRngAlgorithmArmRndr            = { 0x43d2fde3, 0x9d4e, 0x4d79, {0x02,
>>> 0x96,
>>>>> 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41 }}
>>>>> +  gEdkiiRngAlgorithmUnSafe           = { 0x869f728c, 0x409d, 0x4ab4, {0xac,
>>> 0x03,
>>>>> 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 }}
>>>>>
>>>>>      ## Include/Protocol/AdapterInformation.h
>>>>>      gEfiAdapterInfoMediaStateGuid       = { 0xD7C74207, 0xA831, 0x4A26,
>>> {0xB1,
>>>>> 0xF5, 0xD1, 0x93, 0x06, 0x5C, 0xE8, 0xB6 }}
>>>>> --
>>>>> 2.25.1
>>>>>
>>>>>
>>>>>
>>>>> -=-=-=-=-=-=
>>>>> Groups.io Links: You receive all messages sent to this group.
>>>>> View/Reply Online (#106688):
>>> https://edk2.groups.io/g/devel/message/106688
>>>>> Mute This Topic: https://groups.io/mt/99981855/1772286
>>>>> Group Owner: devel+owner@edk2.groups.io
>>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiewen.yao@intel.com]
>>>>> -=-=-=-=-=-=
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>>
>> 
>>
> 

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

* Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms
  2023-07-07 14:25           ` PierreGondois
@ 2023-07-07 14:28             ` Yao, Jiewen
       [not found]             ` <176F9C2F554052EE.2683@groups.io>
  1 sibling, 0 replies; 20+ messages in thread
From: Yao, Jiewen @ 2023-07-07 14:28 UTC (permalink / raw)
  To: Pierre Gondois, devel@edk2.groups.io
  Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang, Wang, Jian J,
	Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

Thanks Pierre.
Yes, I agree to move it to other package to resolve dependency issue.

> -----Original Message-----
> From: Pierre Gondois <pierre.gondois@arm.com>
> Sent: Friday, July 7, 2023 10:25 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
> Rng algorithms
> 
> Hello Jiewen,
> 
> We have the following dependency issue:
> - the BaseRngTimerLib is in the MdePkg
> - we need a GUID to describe the BaseRngTimerLib algorithm
> - we cannot add the gEdkiiRngAlgorithmUnSafe in the MdePkg, and the
> gZeroGuid is also not in the MdePkg
> - the MdePkg should not have dependencies over other packages
> 
> As the BaseRngTimerLib is not really standard and should not be used in
> production builds,
> would you agree if it was moved to the MdeModulePkg or to the SecurityPkg
> (with the gEdkiiRngAlgorithmUnSafe definition) ?
> 
> Regards,
> Pierre
> 
> 
> The issue we have
> 
> On 7/7/23 15:05, Yao, Jiewen wrote:
> > FYI: I filed https://bugzilla.tianocore.org/show_bug.cgi?id=4497 to track the
> gEdkiiMemoryAcceptProtocolGuid issue.
> >
> > Thank you
> > Yao, Jiewen
> >
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> Jiewen
> >> Sent: Friday, July 7, 2023 8:57 PM
> >> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io
> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> Wang,
> >> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>;
> >> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> >> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> >> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
> describe
> >> Rng algorithms
> >>
> >> I don’t think MdePkg should have Edkii- style protocol.
> >>
> >> I am not sure why gEdkiiMemoryAcceptProtocolGuid is in MdePkg.
> >> It should be in MdeModulePkg, IMHO.
> >>
> >> Thank you
> >> Yao, Jiewen
> >>
> >>> -----Original Message-----
> >>> From: Pierre Gondois <pierre.gondois@arm.com>
> >>> Sent: Friday, July 7, 2023 8:49 PM
> >>> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> >>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> >>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> Wang,
> >>> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>;
> >>> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> >>> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> >>> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
> describe
> >>> Rng algorithms
> >>>
> >>> Hello Jiewen,
> >>>
> >>> The gEfiRngAlgorithmArmRndr GUID is to be added to the UEFI spec with:
> >>> - https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> >>> - https://mantis.uefi.org/mantis/view.php?id=2386
> >>>
> >>> the gEdkiiMemoryAcceptProtocolGuid GUID should not be in the UEFI spec,
> >>> so I used the 'gEdkii' prefix as already used in MdePkg.dec for:
> >>> - gEdkiiMemoryAcceptProtocolGuid
> >>>
> >>> Regards,
> >>> Pierre
> >>>
> >>> On 7/7/23 11:14, Yao, Jiewen via groups.io wrote:
> >>>> MdePkg can only add UEFI defined API.
> >>>>
> >>>> Is below defined by UEFI?
> >>>>
> >>>> Thank you
> >>>> Yao, Jiewen
> >>>>
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> >>>>> PierreGondois
> >>>>> Sent: Thursday, July 6, 2023 4:52 PM
> >>>>> To: devel@edk2.groups.io
> >>>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> >>>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> >> Yao,
> >>>>> Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> >> Ard
> >>>>> Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
> >>>>> <sami.mujawar@arm.com>; Jose Marinho <Jose.Marinho@arm.com>;
> Kun
> >>> Qin
> >>>>> <kuqin12@gmail.com>
> >>>>> Subject: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
> >>> Rng
> >>>>> algorithms
> >>>>>
> >>>>> From: Pierre Gondois <pierre.gondois@arm.com>
> >>>>>
> >>>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> >>>>>
> >>>>> The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
> >>>>> implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
> >>>>> To allow the RngDxe to detect when such implementation is used,
> >>>>> a GetRngGuid() function is added in a following patch.
> >>>>>
> >>>>> Prepare GetRngGuid() return values and add GUIDs describing
> >>>>> Rng algorithms:
> >>>>> - gEfiRngAlgorithmArmRndr
> >>>>> to describe a Rng algorithm accessed through Arm's RNDR instruction.
> >>>>> [1] states that the implementation of this algorithm should be
> >>>>> compliant to NIST SP900-80. The compliance is not guaranteed.
> >>>>> - gEdkiiRngAlgorithmUnSafe
> >>>>> to describe an unsafe implementation, cf. the BaseRngLibTimerLib.
> >>>>>
> >>>>> [1] Arm Architecture Reference Manual Armv8, for A-profile architecture
> >>>>> sK12.1 'Properties of the generated random number'
> >>>>>
> >>>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> >>>>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> >>>>> ---
> >>>>>    MdePkg/Include/Protocol/Rng.h | 20 ++++++++++++++++++++
> >>>>>    MdePkg/MdePkg.dec             |  2 ++
> >>>>>    2 files changed, 22 insertions(+)
> >>>>>
> >>>>> diff --git a/MdePkg/Include/Protocol/Rng.h
> >>> b/MdePkg/Include/Protocol/Rng.h
> >>>>> index baf425587b3c..ceae77ba9c73 100644
> >>>>> --- a/MdePkg/Include/Protocol/Rng.h
> >>>>> +++ b/MdePkg/Include/Protocol/Rng.h
> >>>>> @@ -67,6 +67,24 @@ typedef EFI_GUID EFI_RNG_ALGORITHM;
> >>>>>      { \
> >>>>>        0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6,
> 0x85,
> >>> 0x61 }
> >>>>> \
> >>>>>      }
> >>>>> +///
> >>>>> +/// The Arm Architecture states the RNDR that the DRBG algorithm
> should
> >>> be
> >>>>> compliant
> >>>>> +/// with NIST SP800-90A, while not mandating a particular algorithm, so
> as
> >>> to
> >>>>> be
> >>>>> +/// inclusive of different geographies.
> >>>>> +///
> >>>>> +#define EFI_RNG_ALGORITHM_ARM_RNDR \
> >>>>> +  { \
> >>>>> +    0x43d2fde3, 0x9d4e, 0x4d79,  {0x02, 0x96, 0xa8, 0x9b, 0xca, 0x78,
> 0x08,
> >>>>> 0x41} \
> >>>>> +  }
> >>>>> +///
> >>>>> +/// The implementation of a Random Number Generator might be
> unsafe,
> >>>>> when using
> >>>>> +/// a dummy implementation for instance. Allow identifying such
> >>>>> implementation
> >>>>> +/// with this GUID.
> >>>>> +///
> >>>>> +#define EDKII_RNG_ALGORITHM_UNSAFE \
> >>>>> +  { \
> >>>>> +    0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09, 0xc1,
> 0xb3,
> >>>>> 0xf4 } \
> >>>>> +  }
> >>>>>
> >>>>>    /**
> >>>>>      Returns information about the random number generation
> >> implementation.
> >>>>> @@ -146,5 +164,7 @@ extern EFI_GUID
> >>> gEfiRngAlgorithmSp80090Ctr256Guid;
> >>>>>    extern EFI_GUID  gEfiRngAlgorithmX9313DesGuid;
> >>>>>    extern EFI_GUID  gEfiRngAlgorithmX931AesGuid;
> >>>>>    extern EFI_GUID  gEfiRngAlgorithmRaw;
> >>>>> +extern EFI_GUID  gEfiRngAlgorithmArmRndr;
> >>>>> +extern EFI_GUID  gEdkiiRngAlgorithmUnSafe;
> >>>>>
> >>>>>    #endif
> >>>>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> >>>>> index 5b8477f4cb8f..2c8f985f253e 100644
> >>>>> --- a/MdePkg/MdePkg.dec
> >>>>> +++ b/MdePkg/MdePkg.dec
> >>>>> @@ -643,6 +643,8 @@ [Guids]
> >>>>>      gEfiRngAlgorithmX9313DesGuid       = { 0x63c4785a, 0xca34, 0x4012,
> >> {0xa3,
> >>>>> 0xc8, 0x0b, 0x6a, 0x32, 0x4f, 0x55, 0x46 }}
> >>>>>      gEfiRngAlgorithmX931AesGuid        = { 0xacd03321, 0x777e, 0x4d3d,
> >> {0xb1,
> >>>>> 0xc8, 0x20, 0xcf, 0xd8, 0x88, 0x20, 0xc9 }}
> >>>>>      gEfiRngAlgorithmRaw                = { 0xe43176d7, 0xb6e8, 0x4827, {0xb7,
> >>> 0x84,
> >>>>> 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }}
> >>>>> +  gEfiRngAlgorithmArmRndr            = { 0x43d2fde3, 0x9d4e, 0x4d79,
> {0x02,
> >>> 0x96,
> >>>>> 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41 }}
> >>>>> +  gEdkiiRngAlgorithmUnSafe           = { 0x869f728c, 0x409d, 0x4ab4, {0xac,
> >>> 0x03,
> >>>>> 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 }}
> >>>>>
> >>>>>      ## Include/Protocol/AdapterInformation.h
> >>>>>      gEfiAdapterInfoMediaStateGuid       = { 0xD7C74207, 0xA831, 0x4A26,
> >>> {0xB1,
> >>>>> 0xF5, 0xD1, 0x93, 0x06, 0x5C, 0xE8, 0xB6 }}
> >>>>> --
> >>>>> 2.25.1
> >>>>>
> >>>>>
> >>>>>
> >>>>> -=-=-=-=-=-=
> >>>>> Groups.io Links: You receive all messages sent to this group.
> >>>>> View/Reply Online (#106688):
> >>> https://edk2.groups.io/g/devel/message/106688
> >>>>> Mute This Topic: https://groups.io/mt/99981855/1772286
> >>>>> Group Owner: devel+owner@edk2.groups.io
> >>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [jiewen.yao@intel.com]
> >>>>> -=-=-=-=-=-=
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>
> >>
> >> 
> >>
> >

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

* Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms
       [not found]             ` <176F9C2F554052EE.2683@groups.io>
@ 2023-07-07 14:34               ` Yao, Jiewen
  2023-07-10  1:26                 ` 回复: " gaoliming
  0 siblings, 1 reply; 20+ messages in thread
From: Yao, Jiewen @ 2023-07-07 14:34 UTC (permalink / raw)
  To: devel@edk2.groups.io, Yao, Jiewen, Pierre Gondois
  Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang, Wang, Jian J,
	Ard Biesheuvel, Sami Mujawar, Jose Marinho, Kun Qin

I think a better way is to define a new library instance in other package with the new ARM APIs.
The old one can be kept as is.

That will limit the impact to existing platform.



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Friday, July 7, 2023 10:28 PM
> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
> Rng algorithms
> 
> Thanks Pierre.
> Yes, I agree to move it to other package to resolve dependency issue.
> 
> > -----Original Message-----
> > From: Pierre Gondois <pierre.gondois@arm.com>
> > Sent: Friday, July 7, 2023 10:25 PM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang,
> > Jian J <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> > Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> > <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> > Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
> > Rng algorithms
> >
> > Hello Jiewen,
> >
> > We have the following dependency issue:
> > - the BaseRngTimerLib is in the MdePkg
> > - we need a GUID to describe the BaseRngTimerLib algorithm
> > - we cannot add the gEdkiiRngAlgorithmUnSafe in the MdePkg, and the
> > gZeroGuid is also not in the MdePkg
> > - the MdePkg should not have dependencies over other packages
> >
> > As the BaseRngTimerLib is not really standard and should not be used in
> > production builds,
> > would you agree if it was moved to the MdeModulePkg or to the SecurityPkg
> > (with the gEdkiiRngAlgorithmUnSafe definition) ?
> >
> > Regards,
> > Pierre
> >
> >
> > The issue we have
> >
> > On 7/7/23 15:05, Yao, Jiewen wrote:
> > > FYI: I filed https://bugzilla.tianocore.org/show_bug.cgi?id=4497 to track the
> > gEdkiiMemoryAcceptProtocolGuid issue.
> > >
> > > Thank you
> > > Yao, Jiewen
> > >
> > >
> > >> -----Original Message-----
> > >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> > Jiewen
> > >> Sent: Friday, July 7, 2023 8:57 PM
> > >> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io
> > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> > Wang,
> > >> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
> > <ardb+tianocore@kernel.org>;
> > >> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> > >> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> > >> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
> > describe
> > >> Rng algorithms
> > >>
> > >> I don’t think MdePkg should have Edkii- style protocol.
> > >>
> > >> I am not sure why gEdkiiMemoryAcceptProtocolGuid is in MdePkg.
> > >> It should be in MdeModulePkg, IMHO.
> > >>
> > >> Thank you
> > >> Yao, Jiewen
> > >>
> > >>> -----Original Message-----
> > >>> From: Pierre Gondois <pierre.gondois@arm.com>
> > >>> Sent: Friday, July 7, 2023 8:49 PM
> > >>> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > >>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > >>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> > Wang,
> > >>> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
> > <ardb+tianocore@kernel.org>;
> > >>> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> > >>> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> > >>> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
> > describe
> > >>> Rng algorithms
> > >>>
> > >>> Hello Jiewen,
> > >>>
> > >>> The gEfiRngAlgorithmArmRndr GUID is to be added to the UEFI spec with:
> > >>> - https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> > >>> - https://mantis.uefi.org/mantis/view.php?id=2386
> > >>>
> > >>> the gEdkiiMemoryAcceptProtocolGuid GUID should not be in the UEFI
> spec,
> > >>> so I used the 'gEdkii' prefix as already used in MdePkg.dec for:
> > >>> - gEdkiiMemoryAcceptProtocolGuid
> > >>>
> > >>> Regards,
> > >>> Pierre
> > >>>
> > >>> On 7/7/23 11:14, Yao, Jiewen via groups.io wrote:
> > >>>> MdePkg can only add UEFI defined API.
> > >>>>
> > >>>> Is below defined by UEFI?
> > >>>>
> > >>>> Thank you
> > >>>> Yao, Jiewen
> > >>>>
> > >>>>
> > >>>>
> > >>>>> -----Original Message-----
> > >>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > >>>>> PierreGondois
> > >>>>> Sent: Thursday, July 6, 2023 4:52 PM
> > >>>>> To: devel@edk2.groups.io
> > >>>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > >>>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> > >> Yao,
> > >>>>> Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> > >> Ard
> > >>>>> Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
> > >>>>> <sami.mujawar@arm.com>; Jose Marinho <Jose.Marinho@arm.com>;
> > Kun
> > >>> Qin
> > >>>>> <kuqin12@gmail.com>
> > >>>>> Subject: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
> describe
> > >>> Rng
> > >>>>> algorithms
> > >>>>>
> > >>>>> From: Pierre Gondois <pierre.gondois@arm.com>
> > >>>>>
> > >>>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> > >>>>>
> > >>>>> The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has
> multiple
> > >>>>> implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
> > >>>>> To allow the RngDxe to detect when such implementation is used,
> > >>>>> a GetRngGuid() function is added in a following patch.
> > >>>>>
> > >>>>> Prepare GetRngGuid() return values and add GUIDs describing
> > >>>>> Rng algorithms:
> > >>>>> - gEfiRngAlgorithmArmRndr
> > >>>>> to describe a Rng algorithm accessed through Arm's RNDR instruction.
> > >>>>> [1] states that the implementation of this algorithm should be
> > >>>>> compliant to NIST SP900-80. The compliance is not guaranteed.
> > >>>>> - gEdkiiRngAlgorithmUnSafe
> > >>>>> to describe an unsafe implementation, cf. the BaseRngLibTimerLib.
> > >>>>>
> > >>>>> [1] Arm Architecture Reference Manual Armv8, for A-profile
> architecture
> > >>>>> sK12.1 'Properties of the generated random number'
> > >>>>>
> > >>>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> > >>>>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> > >>>>> ---
> > >>>>>    MdePkg/Include/Protocol/Rng.h | 20 ++++++++++++++++++++
> > >>>>>    MdePkg/MdePkg.dec             |  2 ++
> > >>>>>    2 files changed, 22 insertions(+)
> > >>>>>
> > >>>>> diff --git a/MdePkg/Include/Protocol/Rng.h
> > >>> b/MdePkg/Include/Protocol/Rng.h
> > >>>>> index baf425587b3c..ceae77ba9c73 100644
> > >>>>> --- a/MdePkg/Include/Protocol/Rng.h
> > >>>>> +++ b/MdePkg/Include/Protocol/Rng.h
> > >>>>> @@ -67,6 +67,24 @@ typedef EFI_GUID EFI_RNG_ALGORITHM;
> > >>>>>      { \
> > >>>>>        0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6,
> > 0x85,
> > >>> 0x61 }
> > >>>>> \
> > >>>>>      }
> > >>>>> +///
> > >>>>> +/// The Arm Architecture states the RNDR that the DRBG algorithm
> > should
> > >>> be
> > >>>>> compliant
> > >>>>> +/// with NIST SP800-90A, while not mandating a particular algorithm,
> so
> > as
> > >>> to
> > >>>>> be
> > >>>>> +/// inclusive of different geographies.
> > >>>>> +///
> > >>>>> +#define EFI_RNG_ALGORITHM_ARM_RNDR \
> > >>>>> +  { \
> > >>>>> +    0x43d2fde3, 0x9d4e, 0x4d79,  {0x02, 0x96, 0xa8, 0x9b, 0xca, 0x78,
> > 0x08,
> > >>>>> 0x41} \
> > >>>>> +  }
> > >>>>> +///
> > >>>>> +/// The implementation of a Random Number Generator might be
> > unsafe,
> > >>>>> when using
> > >>>>> +/// a dummy implementation for instance. Allow identifying such
> > >>>>> implementation
> > >>>>> +/// with this GUID.
> > >>>>> +///
> > >>>>> +#define EDKII_RNG_ALGORITHM_UNSAFE \
> > >>>>> +  { \
> > >>>>> +    0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09, 0xc1,
> > 0xb3,
> > >>>>> 0xf4 } \
> > >>>>> +  }
> > >>>>>
> > >>>>>    /**
> > >>>>>      Returns information about the random number generation
> > >> implementation.
> > >>>>> @@ -146,5 +164,7 @@ extern EFI_GUID
> > >>> gEfiRngAlgorithmSp80090Ctr256Guid;
> > >>>>>    extern EFI_GUID  gEfiRngAlgorithmX9313DesGuid;
> > >>>>>    extern EFI_GUID  gEfiRngAlgorithmX931AesGuid;
> > >>>>>    extern EFI_GUID  gEfiRngAlgorithmRaw;
> > >>>>> +extern EFI_GUID  gEfiRngAlgorithmArmRndr;
> > >>>>> +extern EFI_GUID  gEdkiiRngAlgorithmUnSafe;
> > >>>>>
> > >>>>>    #endif
> > >>>>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> > >>>>> index 5b8477f4cb8f..2c8f985f253e 100644
> > >>>>> --- a/MdePkg/MdePkg.dec
> > >>>>> +++ b/MdePkg/MdePkg.dec
> > >>>>> @@ -643,6 +643,8 @@ [Guids]
> > >>>>>      gEfiRngAlgorithmX9313DesGuid       = { 0x63c4785a, 0xca34, 0x4012,
> > >> {0xa3,
> > >>>>> 0xc8, 0x0b, 0x6a, 0x32, 0x4f, 0x55, 0x46 }}
> > >>>>>      gEfiRngAlgorithmX931AesGuid        = { 0xacd03321, 0x777e, 0x4d3d,
> > >> {0xb1,
> > >>>>> 0xc8, 0x20, 0xcf, 0xd8, 0x88, 0x20, 0xc9 }}
> > >>>>>      gEfiRngAlgorithmRaw                = { 0xe43176d7, 0xb6e8, 0x4827,
> {0xb7,
> > >>> 0x84,
> > >>>>> 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }}
> > >>>>> +  gEfiRngAlgorithmArmRndr            = { 0x43d2fde3, 0x9d4e, 0x4d79,
> > {0x02,
> > >>> 0x96,
> > >>>>> 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41 }}
> > >>>>> +  gEdkiiRngAlgorithmUnSafe           = { 0x869f728c, 0x409d, 0x4ab4,
> {0xac,
> > >>> 0x03,
> > >>>>> 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 }}
> > >>>>>
> > >>>>>      ## Include/Protocol/AdapterInformation.h
> > >>>>>      gEfiAdapterInfoMediaStateGuid       = { 0xD7C74207, 0xA831, 0x4A26,
> > >>> {0xB1,
> > >>>>> 0xF5, 0xD1, 0x93, 0x06, 0x5C, 0xE8, 0xB6 }}
> > >>>>> --
> > >>>>> 2.25.1
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> -=-=-=-=-=-=
> > >>>>> Groups.io Links: You receive all messages sent to this group.
> > >>>>> View/Reply Online (#106688):
> > >>> https://edk2.groups.io/g/devel/message/106688
> > >>>>> Mute This Topic: https://groups.io/mt/99981855/1772286
> > >>>>> Group Owner: devel+owner@edk2.groups.io
> > >>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [jiewen.yao@intel.com]
> > >>>>> -=-=-=-=-=-=
> > >>>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>
> > >>
> > >>
> > >>
> > >
> 
> 
> 
> 


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

* 回复: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms
  2023-07-07 14:34               ` Yao, Jiewen
@ 2023-07-10  1:26                 ` gaoliming
  2023-07-11 12:23                   ` PierreGondois
  0 siblings, 1 reply; 20+ messages in thread
From: gaoliming @ 2023-07-10  1:26 UTC (permalink / raw)
  To: devel, jiewen.yao, 'Pierre Gondois'
  Cc: 'Kinney, Michael D', 'Liu, Zhiguang',
	'Wang, Jian J', 'Ard Biesheuvel',
	'Sami Mujawar', 'Jose Marinho', 'Kun Qin'

Pierre:
  Another option is to define two PCD for Rng algorithm in MdePkg. One PCD value is ArmRndr GUID, another is UnSafe GUID. This way can also resolve the package dependency.

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Yao, Jiewen
> 发送时间: 2023年7月7日 22:34
> 收件人: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Pierre
> Gondois <pierre.gondois@arm.com>
> 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> 主题: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
> Rng algorithms
> 
> I think a better way is to define a new library instance in other package with
> the new ARM APIs.
> The old one can be kept as is.
> 
> That will limit the impact to existing platform.
> 
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> Jiewen
> > Sent: Friday, July 7, 2023 10:28 PM
> > To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> Wang,
> > Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>;
> > Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> > <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> > Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
> describe
> > Rng algorithms
> >
> > Thanks Pierre.
> > Yes, I agree to move it to other package to resolve dependency issue.
> >
> > > -----Original Message-----
> > > From: Pierre Gondois <pierre.gondois@arm.com>
> > > Sent: Friday, July 7, 2023 10:25 PM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> Wang,
> > > Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>;
> > > Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> > > <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> > > Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
> describe
> > > Rng algorithms
> > >
> > > Hello Jiewen,
> > >
> > > We have the following dependency issue:
> > > - the BaseRngTimerLib is in the MdePkg
> > > - we need a GUID to describe the BaseRngTimerLib algorithm
> > > - we cannot add the gEdkiiRngAlgorithmUnSafe in the MdePkg, and the
> > > gZeroGuid is also not in the MdePkg
> > > - the MdePkg should not have dependencies over other packages
> > >
> > > As the BaseRngTimerLib is not really standard and should not be used in
> > > production builds,
> > > would you agree if it was moved to the MdeModulePkg or to the
> SecurityPkg
> > > (with the gEdkiiRngAlgorithmUnSafe definition) ?
> > >
> > > Regards,
> > > Pierre
> > >
> > >
> > > The issue we have
> > >
> > > On 7/7/23 15:05, Yao, Jiewen wrote:
> > > > FYI: I filed https://bugzilla.tianocore.org/show_bug.cgi?id=4497 to track
> the
> > > gEdkiiMemoryAcceptProtocolGuid issue.
> > > >
> > > > Thank you
> > > > Yao, Jiewen
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Yao,
> > > Jiewen
> > > >> Sent: Friday, July 7, 2023 8:57 PM
> > > >> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io
> > > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> > > Wang,
> > > >> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
> > > <ardb+tianocore@kernel.org>;
> > > >> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> > > >> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> > > >> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
> > > describe
> > > >> Rng algorithms
> > > >>
> > > >> I don’t think MdePkg should have Edkii- style protocol.
> > > >>
> > > >> I am not sure why gEdkiiMemoryAcceptProtocolGuid is in MdePkg.
> > > >> It should be in MdeModulePkg, IMHO.
> > > >>
> > > >> Thank you
> > > >> Yao, Jiewen
> > > >>
> > > >>> -----Original Message-----
> > > >>> From: Pierre Gondois <pierre.gondois@arm.com>
> > > >>> Sent: Friday, July 7, 2023 8:49 PM
> > > >>> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > >>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > >>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>;
> > > Wang,
> > > >>> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
> > > <ardb+tianocore@kernel.org>;
> > > >>> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
> > > >>> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
> > > >>> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
> > > describe
> > > >>> Rng algorithms
> > > >>>
> > > >>> Hello Jiewen,
> > > >>>
> > > >>> The gEfiRngAlgorithmArmRndr GUID is to be added to the UEFI spec
> with:
> > > >>> - https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> > > >>> - https://mantis.uefi.org/mantis/view.php?id=2386
> > > >>>
> > > >>> the gEdkiiMemoryAcceptProtocolGuid GUID should not be in the UEFI
> > spec,
> > > >>> so I used the 'gEdkii' prefix as already used in MdePkg.dec for:
> > > >>> - gEdkiiMemoryAcceptProtocolGuid
> > > >>>
> > > >>> Regards,
> > > >>> Pierre
> > > >>>
> > > >>> On 7/7/23 11:14, Yao, Jiewen via groups.io wrote:
> > > >>>> MdePkg can only add UEFI defined API.
> > > >>>>
> > > >>>> Is below defined by UEFI?
> > > >>>>
> > > >>>> Thank you
> > > >>>> Yao, Jiewen
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>> -----Original Message-----
> > > >>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > >>>>> PierreGondois
> > > >>>>> Sent: Thursday, July 6, 2023 4:52 PM
> > > >>>>> To: devel@edk2.groups.io
> > > >>>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > >>>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>;
> > > >> Yao,
> > > >>>>> Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>;
> > > >> Ard
> > > >>>>> Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
> > > >>>>> <sami.mujawar@arm.com>; Jose Marinho
> <Jose.Marinho@arm.com>;
> > > Kun
> > > >>> Qin
> > > >>>>> <kuqin12@gmail.com>
> > > >>>>> Subject: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
> > describe
> > > >>> Rng
> > > >>>>> algorithms
> > > >>>>>
> > > >>>>> From: Pierre Gondois <pierre.gondois@arm.com>
> > > >>>>>
> > > >>>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> > > >>>>>
> > > >>>>> The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has
> > multiple
> > > >>>>> implementations, some of them are unsafe (e.g.
> BaseRngLibTimerLib).
> > > >>>>> To allow the RngDxe to detect when such implementation is used,
> > > >>>>> a GetRngGuid() function is added in a following patch.
> > > >>>>>
> > > >>>>> Prepare GetRngGuid() return values and add GUIDs describing
> > > >>>>> Rng algorithms:
> > > >>>>> - gEfiRngAlgorithmArmRndr
> > > >>>>> to describe a Rng algorithm accessed through Arm's RNDR
> instruction.
> > > >>>>> [1] states that the implementation of this algorithm should be
> > > >>>>> compliant to NIST SP900-80. The compliance is not guaranteed.
> > > >>>>> - gEdkiiRngAlgorithmUnSafe
> > > >>>>> to describe an unsafe implementation, cf. the BaseRngLibTimerLib.
> > > >>>>>
> > > >>>>> [1] Arm Architecture Reference Manual Armv8, for A-profile
> > architecture
> > > >>>>> sK12.1 'Properties of the generated random number'
> > > >>>>>
> > > >>>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> > > >>>>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> > > >>>>> ---
> > > >>>>>    MdePkg/Include/Protocol/Rng.h | 20 ++++++++++++++++++++
> > > >>>>>    MdePkg/MdePkg.dec             |  2 ++
> > > >>>>>    2 files changed, 22 insertions(+)
> > > >>>>>
> > > >>>>> diff --git a/MdePkg/Include/Protocol/Rng.h
> > > >>> b/MdePkg/Include/Protocol/Rng.h
> > > >>>>> index baf425587b3c..ceae77ba9c73 100644
> > > >>>>> --- a/MdePkg/Include/Protocol/Rng.h
> > > >>>>> +++ b/MdePkg/Include/Protocol/Rng.h
> > > >>>>> @@ -67,6 +67,24 @@ typedef EFI_GUID EFI_RNG_ALGORITHM;
> > > >>>>>      { \
> > > >>>>>        0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd,
> 0xc4, 0xb6,
> > > 0x85,
> > > >>> 0x61 }
> > > >>>>> \
> > > >>>>>      }
> > > >>>>> +///
> > > >>>>> +/// The Arm Architecture states the RNDR that the DRBG
> algorithm
> > > should
> > > >>> be
> > > >>>>> compliant
> > > >>>>> +/// with NIST SP800-90A, while not mandating a particular
> algorithm,
> > so
> > > as
> > > >>> to
> > > >>>>> be
> > > >>>>> +/// inclusive of different geographies.
> > > >>>>> +///
> > > >>>>> +#define EFI_RNG_ALGORITHM_ARM_RNDR \
> > > >>>>> +  { \
> > > >>>>> +    0x43d2fde3, 0x9d4e, 0x4d79,  {0x02, 0x96, 0xa8, 0x9b, 0xca,
> 0x78,
> > > 0x08,
> > > >>>>> 0x41} \
> > > >>>>> +  }
> > > >>>>> +///
> > > >>>>> +/// The implementation of a Random Number Generator might be
> > > unsafe,
> > > >>>>> when using
> > > >>>>> +/// a dummy implementation for instance. Allow identifying such
> > > >>>>> implementation
> > > >>>>> +/// with this GUID.
> > > >>>>> +///
> > > >>>>> +#define EDKII_RNG_ALGORITHM_UNSAFE \
> > > >>>>> +  { \
> > > >>>>> +    0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09,
> 0xc1,
> > > 0xb3,
> > > >>>>> 0xf4 } \
> > > >>>>> +  }
> > > >>>>>
> > > >>>>>    /**
> > > >>>>>      Returns information about the random number generation
> > > >> implementation.
> > > >>>>> @@ -146,5 +164,7 @@ extern EFI_GUID
> > > >>> gEfiRngAlgorithmSp80090Ctr256Guid;
> > > >>>>>    extern EFI_GUID  gEfiRngAlgorithmX9313DesGuid;
> > > >>>>>    extern EFI_GUID  gEfiRngAlgorithmX931AesGuid;
> > > >>>>>    extern EFI_GUID  gEfiRngAlgorithmRaw;
> > > >>>>> +extern EFI_GUID  gEfiRngAlgorithmArmRndr;
> > > >>>>> +extern EFI_GUID  gEdkiiRngAlgorithmUnSafe;
> > > >>>>>
> > > >>>>>    #endif
> > > >>>>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> > > >>>>> index 5b8477f4cb8f..2c8f985f253e 100644
> > > >>>>> --- a/MdePkg/MdePkg.dec
> > > >>>>> +++ b/MdePkg/MdePkg.dec
> > > >>>>> @@ -643,6 +643,8 @@ [Guids]
> > > >>>>>      gEfiRngAlgorithmX9313DesGuid       = { 0x63c4785a,
> 0xca34, 0x4012,
> > > >> {0xa3,
> > > >>>>> 0xc8, 0x0b, 0x6a, 0x32, 0x4f, 0x55, 0x46 }}
> > > >>>>>      gEfiRngAlgorithmX931AesGuid        = { 0xacd03321,
> 0x777e, 0x4d3d,
> > > >> {0xb1,
> > > >>>>> 0xc8, 0x20, 0xcf, 0xd8, 0x88, 0x20, 0xc9 }}
> > > >>>>>      gEfiRngAlgorithmRaw                = { 0xe43176d7,
> 0xb6e8, 0x4827,
> > {0xb7,
> > > >>> 0x84,
> > > >>>>> 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }}
> > > >>>>> +  gEfiRngAlgorithmArmRndr            = { 0x43d2fde3,
> 0x9d4e, 0x4d79,
> > > {0x02,
> > > >>> 0x96,
> > > >>>>> 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41 }}
> > > >>>>> +  gEdkiiRngAlgorithmUnSafe           = { 0x869f728c, 0x409d,
> 0x4ab4,
> > {0xac,
> > > >>> 0x03,
> > > >>>>> 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 }}
> > > >>>>>
> > > >>>>>      ## Include/Protocol/AdapterInformation.h
> > > >>>>>      gEfiAdapterInfoMediaStateGuid       = { 0xD7C74207,
> 0xA831, 0x4A26,
> > > >>> {0xB1,
> > > >>>>> 0xF5, 0xD1, 0x93, 0x06, 0x5C, 0xE8, 0xB6 }}
> > > >>>>> --
> > > >>>>> 2.25.1
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>> -=-=-=-=-=-=
> > > >>>>> Groups.io Links: You receive all messages sent to this group.
> > > >>>>> View/Reply Online (#106688):
> > > >>> https://edk2.groups.io/g/devel/message/106688
> > > >>>>> Mute This Topic: https://groups.io/mt/99981855/1772286
> > > >>>>> Group Owner: devel+owner@edk2.groups.io
> > > >>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > > [jiewen.yao@intel.com]
> > > >>>>> -=-=-=-=-=-=
> > > >>>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>
> > > >>
> > > >>
> > > >>
> > > >
> >
> >
> >
> >
> 
> 
> 
> 
> 




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

* Re: 回复: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms
  2023-07-10  1:26                 ` 回复: " gaoliming
@ 2023-07-11 12:23                   ` PierreGondois
  0 siblings, 0 replies; 20+ messages in thread
From: PierreGondois @ 2023-07-11 12:23 UTC (permalink / raw)
  To: gaoliming, devel, jiewen.yao
  Cc: 'Kinney, Michael D', 'Liu, Zhiguang',
	'Wang, Jian J', 'Ard Biesheuvel',
	'Sami Mujawar', 'Jose Marinho', 'Kun Qin'

Hello Liming, Jiewen,


On 7/10/23 03:26, gaoliming wrote:
> Pierre:
>    Another option is to define two PCD for Rng algorithm in MdePkg. One PCD value is ArmRndr GUID, another is UnSafe GUID. This way can also resolve the package dependency.

So there would be a Pcd for the unsafe algorithm so that the platform can
choose a custom GUID to advertise the unsafe algorithm ? If so, I think
it would be better to have only one common GUID,

> 
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Yao, Jiewen
>> 发送时间: 2023年7月7日 22:34
>> 收件人: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Pierre
>> Gondois <pierre.gondois@arm.com>
>> 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang,
>> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
>> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
>> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
>> 主题: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe
>> Rng algorithms
>>
>> I think a better way is to define a new library instance in other package with
>> the new ARM APIs.
>> The old one can be kept as is.
>>
>> That will limit the impact to existing platform.

Yes right,
However this would mean having 2 identical implementations of the BaseRngTimerLib,
I ll try to move the library instead, if it's ok

Regards,
Pierre

>>
>>
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
>> Jiewen
>>> Sent: Friday, July 7, 2023 10:28 PM
>>> To: Pierre Gondois <pierre.gondois@arm.com>; devel@edk2.groups.io
>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
>> Wang,
>>> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
>> <ardb+tianocore@kernel.org>;
>>> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
>>> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
>>> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
>> describe
>>> Rng algorithms
>>>
>>> Thanks Pierre.
>>> Yes, I agree to move it to other package to resolve dependency issue.
>>>
>>>> -----Original Message-----
>>>> From: Pierre Gondois <pierre.gondois@arm.com>
>>>> Sent: Friday, July 7, 2023 10:25 PM
>>>> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
>>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
>> Wang,
>>>> Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
>> <ardb+tianocore@kernel.org>;
>>>> Sami Mujawar <sami.mujawar@arm.com>; Jose Marinho
>>>> <Jose.Marinho@arm.com>; Kun Qin <kuqin12@gmail.com>
>>>> Subject: Re: [edk2-devel] [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to
>> describe
>>>> Rng algorithms
>>>>
>>>> Hello Jiewen,
>>>>
>>>> We have the following dependency issue:
>>>> - the BaseRngTimerLib is in the MdePkg
>>>> - we need a GUID to describe the BaseRngTimerLib algorithm
>>>> - we cannot add the gEdkiiRngAlgorithmUnSafe in the MdePkg, and the
>>>> gZeroGuid is also not in the MdePkg
>>>> - the MdePkg should not have dependencies over other packages
>>>>
>>>> As the BaseRngTimerLib is not really standard and should not be used in
>>>> production builds,
>>>> would you agree if it was moved to the MdeModulePkg or to the
>> SecurityPkg
>>>> (with the gEdkiiRngAlgorithmUnSafe definition) ?
>>>>
>>>> Regards,
>>>> Pierre
>>>>

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

* Re: [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification
  2023-07-06 19:01 ` [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification Kun Qin
@ 2023-07-12 13:38   ` PierreGondois
  0 siblings, 0 replies; 20+ messages in thread
From: PierreGondois @ 2023-07-12 13:38 UTC (permalink / raw)
  To: Kun Qin, devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ard Biesheuvel, Sami Mujawar, Jose Marinho

Hello Kun,
As I made some small changes to the patch-set, I didn't include your
tested-by tag, but the changes should be quite small,

The v4 is available at:
- https://edk2.groups.io/g/devel/message/106856

Regards,
Pierre

On 7/6/23 21:01, Kun Qin wrote:
> Hi Pierre,
> 
> Thanks for sending the update. I tested on QEMU with this change (no TRNG from TFA), it works for me.
> Tested-by: Kun Qin <kun.qin@microsoft.com>
> 
> Please note that the change below is still needed to avoid data abortion exception. It will be helpful if one
> of the maintainers can help merging it.
> [PATCH v2 1/1] SecurityPkg: RngDxe: Fixing mAvailableAlgoArray allocator (groups.io) <https://edk2.groups.io/g/devel/message/106547>
> 
> Regards,
> Kun
> 
> On 7/6/2023 1:51 AM, pierre.gondois@arm.com wrote:
>> From: Pierre Gondois<pierre.gondois@arm.com>
>>
>> v3:
>> - As the unsafe algorithm GUID will not be added to the UEFI
>>    specification, rename:
>>    - gEfiRngAlgorithmUnSafe to gEdkiiRngAlgorithmUnSafe
>>    - EFI_RNG_ALGORITHM_UNSAFE to EDKII_RNG_ALGORITHM_UNSAFE
>>
>> v2:
>> [1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation
>> - Dropped
>> [2/8] MdePkg/MdePkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg
>> - Change gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm
>>    token number
>> - Rename to SecurityPkg/SecurityPkg.dec: Move
>>    PcdCpuRngSupportedAlgorithm to MdePkg
>> [5/8] MdePkg/Rng: Add GetRngGuid() to RngLib
>> - Remove gEfiRngAlgorithmUnSafe from inf file
>> - Split Guids definitions in arch specific sections
>> [6/8] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
>> - Remove RngFindDefaultAlgo() and change logic accordingly.
>> [7/8] SecurityPkg/RngDxe: Select safe default Rng algorithm
>> - Dropped due to changes in [6/8]
>>
>> This patch also requires the following patch on top of the serie:
>> -https://edk2.groups.io/g/devel/message/106546
>>
>> This patchset follows the 'code first' approach and relates to [1].
>> This patchset follows the thread at [3] that aims to solve [2].
>> [1] and [2] are bound and this patchset aims to solve both.
>>
>> In this patchset:
>> a-
>> The RngDxe can rely on the RngLib. However the RngLib has no
>> interface allowing to describe which Rng algorithm is implemented.
>> The RngDxe must advertise the algorithm that are available through
>> the RngGetInfo() callback.
>> Add a GetRngGuid() for interface to the RngLib.
>>
>> b-
>> The Arm Architecture states the RNDR that the DRBG algorithm should
>> be compliant with NIST SP800-90A, while not mandating a particular
>> algorithm, so as to be inclusive of different geographies.
>> The RngLib can rely on this Arm RNDR instruction. In order to
>> accurately describe the implementation using the RNDR instruction,
>> add a EFI_RNG_ALGORITHM_ARM_RNDR GUID [1].
>>
>> c-
>> For the same reason as a/b, add a GUID describing unsafe RNG
>> algorithms, allowing to accurately describe the BaseRngLibTimerLib.
>>
>> d-
>> Use a/b/c mechanisms/GUIDs to select a safe Rng algorithm in the
>> Arm implementation of the RngDxe.
>>
>> [1] BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4441
>> [2] BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4151
>> [3]https://edk2.groups.io/g/devel/message/100806
>>
>> Pierre Gondois (6):
>>    SecurityPkg/SecurityPkg.dec: Move PcdCpuRngSupportedAlgorithm to
>>      MdePkg
>>    MdePkg/DxeRngLib: Request raw algorithm instead of default
>>    MdePkg/Rng: Add GUIDs to describe Rng algorithms
>>    MdePkg/Rng: Add GetRngGuid() to RngLib
>>    SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
>>    SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm
>>
>>   MdePkg/Include/Library/RngLib.h               | 17 ++++++
>>   MdePkg/Include/Protocol/Rng.h                 | 20 +++++++
>>   MdePkg/Library/BaseRngLib/AArch64/Rndr.c      | 42 +++++++++++++++
>>   MdePkg/Library/BaseRngLib/BaseRngLib.inf      | 10 ++++
>>   MdePkg/Library/BaseRngLib/Rand/RdRand.c       | 26 +++++++++
>>   .../Library/BaseRngLibNull/BaseRngLibNull.c   | 22 ++++++++
>>   .../BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  3 ++
>>   .../Library/BaseRngLibTimerLib/RngLibTimer.c  | 28 ++++++++++
>>   MdePkg/Library/DxeRngLib/DxeRngLib.c          | 36 ++++++++++++-
>>   MdePkg/MdePkg.dec                             |  7 +++
>>   .../RngDxe/AArch64/AArch64Algo.c              | 54 +++++++++++++------
>>   .../RandomNumberGenerator/RngDxe/ArmRngDxe.c  | 23 ++++----
>>   .../RandomNumberGenerator/RngDxe/RngDxe.inf   |  5 +-
>>   SecurityPkg/SecurityPkg.dec                   |  2 -
>>   14 files changed, 258 insertions(+), 37 deletions(-)
>>

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

end of thread, other threads:[~2023-07-12 13:38 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-06  8:51 [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification PierreGondois
2023-07-06  8:51 ` [PATCH v3 1/6] SecurityPkg/SecurityPkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg PierreGondois
2023-07-06  8:51 ` [PATCH v3 2/6] MdePkg/DxeRngLib: Request raw algorithm instead of default PierreGondois
2023-07-06  8:51 ` [PATCH v3 3/6] MdePkg/Rng: Add GUIDs to describe Rng algorithms PierreGondois
2023-07-07  9:14   ` [edk2-devel] " Yao, Jiewen
2023-07-07 12:49     ` PierreGondois
2023-07-07 12:56       ` Yao, Jiewen
     [not found]       ` <176F972B57840483.2683@groups.io>
2023-07-07 13:05         ` Yao, Jiewen
2023-07-07 14:25           ` PierreGondois
2023-07-07 14:28             ` Yao, Jiewen
     [not found]             ` <176F9C2F554052EE.2683@groups.io>
2023-07-07 14:34               ` Yao, Jiewen
2023-07-10  1:26                 ` 回复: " gaoliming
2023-07-11 12:23                   ` PierreGondois
2023-07-06  8:51 ` [PATCH v3 4/6] MdePkg/Rng: Add GetRngGuid() to RngLib PierreGondois
2023-07-06  8:51 ` [PATCH v3 5/6] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib PierreGondois
2023-07-07  8:07   ` Sami Mujawar
2023-07-06  8:51 ` [PATCH v3 6/6] SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm PierreGondois
2023-07-06 19:01 ` [PATCH v3 0/6] SecurityPkg/MdePkg: Update RngLib GUID identification Kun Qin
2023-07-12 13:38   ` PierreGondois
2023-07-07  8:26 ` Sami Mujawar

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