Re: [edk2-devel] [PATCH] UefiPayloadPkg: Add RNG support

["Yao, Jiewen" <jiewen.yao@intel.com>]

Duplicating code is not the best idea.

I recommend we just add support to MdePkg.

Thank you
Yao Jiewen

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ma,
> Maurice
> Sent: Thursday, January 21, 2021 11:39 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; patrick.rudolph@9elements.com
> Cc: Dong, Guo <guo.dong@intel.com>; You, Benjamin
> <benjamin.you@intel.com>; devel@edk2.groups.io; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> Subject: Re: [edk2-devel] [PATCH] UefiPayloadPkg: Add RNG support
> 
> Hi,  Patrick
> Let us add MdePkg maintainers into the discussion to see what could be the best
> approach for this.
> 
> Hi, Mike and Liming,
> Any recommendation from your side ?
> 
> Thanks
> Maurice
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Patrick
> > Rudolph
> > Sent: Thursday, January 21, 2021 1:05
> > To: Ma, Maurice <maurice.ma@intel.com>
> > Cc: devel@edk2.groups.io; Dong, Guo <guo.dong@intel.com>; You, Benjamin
> > <benjamin.you@intel.com>
> > Subject: Re: [edk2-devel] [PATCH] UefiPayloadPkg: Add RNG support
> >
> > Hi Maurice,
> > it's a copy of the MdeModulePkg's BaseRngLib with runtime detection support.
> >
> > Are the MdeModulePkg maintainers OK with adding runtime detection support?
> > I had the impression that it should only be used on platforms supporting rdrand
> > for sure, and therefore must not be used on platforms not supporting rdrand.
> >
> > Kind Regards,
> > Patrick Rudolph
> >
> > On Thu, Jan 21, 2021 at 1:42 AM Ma, Maurice <maurice.ma@intel.com> wrote:
> > >
> > > Hi,  Patrick
> > >
> > > There is a BaseRngLib in MdePkg package already.  I am wondering why a
> new
> > instance was created under UefiPayloadPkg in the patch.
> > > Could we just reuse the same library in MdePkg?
> > > If not, what is the reason?   Can we try to enhance the library in MdePkg to
> > address it if required?
> > >
> > > Thanks
> > > Maurice
> > > > -----Original Message-----
> > > > From: Patrick Rudolph <patrick.rudolph@9elements.com>
> > > > Sent: Wednesday, January 20, 2021 7:52
> > > > To: devel@edk2.groups.io
> > > > Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo
> > > > <guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
> > > > Subject: [PATCH] UefiPayloadPkg: Add RNG support
> > > >
> > > > Uses the RDRAND instruction if available and install EfiRngProtocol.
> > > > The protocol may be used by iPXE or the Linux kernel to gather entropy.
> > > >
> > > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > > > ---
> > > >  UefiPayloadPkg/Library/BaseRngLib/BaseRng.c      | 199
> > > > ++++++++++++++++++++
> > > >  UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf |  32 ++++
> > > > UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni |  17 ++
> > > >  UefiPayloadPkg/UefiPayloadPkg.dsc                |   8 +
> > > >  UefiPayloadPkg/UefiPayloadPkg.fdf                |   4 +
> > > >  5 files changed, 260 insertions(+)
> > > >
> > > > diff --git a/UefiPayloadPkg/Library/BaseRngLib/BaseRng.c
> > > > b/UefiPayloadPkg/Library/BaseRngLib/BaseRng.c
> > > > new file mode 100644
> > > > index 0000000000..1fe9e1dbe0
> > > > --- /dev/null
> > > > +++ b/UefiPayloadPkg/Library/BaseRngLib/BaseRng.c
> > > > @@ -0,0 +1,199 @@
> > > > +/** @file+  Random number generator services that uses RdRand
> > > > +instruction
> > > > access+  to provide high-quality random numbers.++Copyright (c)
> > > > access+ 2015, Intel
> > > > Corporation. All rights reserved.<BR>+SPDX-License-Identifier:
> > > > BSD-2-Clause-
> > > > Patent++**/++#include <Library/BaseLib.h>+#include
> > > > <Library/DebugLib.h>+#include <Register/Intel/Cpuid.h>++STATIC
> > > > BOOLEAN mHasRdRand;++//+// Bit mask used to determine if RdRand
> > instruction is
> > > > supported.+//+#define RDRAND_MASK                  BIT30++//+// Limited
> retry
> > > > number when valid random data is returned.+// Uses the recommended
> > > > value defined in Section 7.3.17 of "Intel 64 and IA-32+// Architectures
> > Software
> > > > Developer's Mannual".+//+#define RDRAND_RETRY_LIMIT           10++/**+
> > The
> > > > constructor function checks whether or not RDRAND instruction is
> > > > supported+ by the host hardware.++  The constructor function checks
> > > > whether or not RDRAND instruction is supported.+  It will always return
> > RETURN_SUCCESS.++
> > > > @retval RETURN_SUCCESS   The constructor always returns
> > > > EFI_SUCCESS.++**/+RETURN_STATUS+EFIAPI+BaseRngLibConstructor (+
> > > > VOID+  )+{+  UINT32  RegEax;+  UINT32  RegEcx;++  AsmCpuid
> > > > (CPUID_SIGNATURE, &RegEax, NULL, NULL, NULL);+  if (RegEax < 1) {+
> > > > mHasRdRand = FALSE;+    return RETURN_SUCCESS;+  }++  //+  //
> Determine
> > > > RDRAND support by examining bit 30 of the ECX register returned by+  //
> > CPUID.
> > > > A value of 1 indicates that processor support RDRAND instruction.+
> > > > //+ AsmCpuid (CPUID_VERSION_INFO, 0, 0, &RegEcx, 0);++  mHasRdRand
> =
> > > > ((RegEcx & RDRAND_MASK) == RDRAND_MASK);++  return
> > > > RETURN_SUCCESS;+}++/**+  Generates a 16-bit random number.++  if
> Rand
> > is
> > > > NULL, then ASSERT().++  @param[out] Rand     Buffer pointer to store the
> > 16-bit
> > > > random value.++  @retval TRUE         Random number generated
> > successfully.+
> > > > @retval FALSE        Failed to generate the random
> > > > number.++**/+BOOLEAN+EFIAPI+GetRandomNumber16 (+  OUT     UINT16
> > > > *Rand+  )+{+  UINT32  Index;++  ASSERT (Rand != NULL);++  if
> (mHasRdRand)
> > {+
> > > > //+    // A loop to fetch a 16 bit random value with a retry count limit.+
> //+
> > > > for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {+      if
> > (AsmRdRand16
> > > > (Rand)) {+        return TRUE;+      }+    }+  }++  return FALSE;+}++/**+
> > Generates a
> > > > 32-bit random number.++  if Rand is NULL, then ASSERT().++  @param[out]
> > > > Rand     Buffer pointer to store the 32-bit random value.++  @retval TRUE
> > > > Random number generated successfully.+  @retval FALSE        Failed to
> > generate
> > > > the random number.++**/+BOOLEAN+EFIAPI+GetRandomNumber32 (+
> > OUT
> > > > UINT32                    *Rand+  )+{+  UINT32  Index;++  ASSERT (Rand !=
> > NULL);++  if
> > > > (mHasRdRand) {+    //+    // A loop to fetch a 32 bit random value with a
> retry
> > > > count limit.+    //+    for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++)
> > {+
> > > > if (AsmRdRand32 (Rand)) {+        return TRUE;+      }+    }+  }++  return
> > > > FALSE;+}++/**+  Generates a 64-bit random number.++  if Rand is NULL,
> > then
> > > > ASSERT().++  @param[out] Rand     Buffer pointer to store the 64-bit
> > random
> > > > value.++  @retval TRUE         Random number generated successfully.+
> > @retval
> > > > FALSE        Failed to generate the random
> > > > number.++**/+BOOLEAN+EFIAPI+GetRandomNumber64 (+  OUT     UINT64
> > > > *Rand+  )+{+  UINT32  Index;++  ASSERT (Rand != NULL);++  if
> (mHasRdRand)
> > {+
> > > > //+    // A loop to fetch a 64 bit random value with a retry count limit.+
> //+
> > > > for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {+      if
> > (AsmRdRand64
> > > > (Rand)) {+        return TRUE;+      }+    }+  }++  return FALSE;+}++/**+
> > Generates a
> > > > 128-bit random number.++  if Rand is NULL, then ASSERT().++  @param[out]
> > > > Rand     Buffer pointer to store the 128-bit random value.++  @retval TRUE
> > > > Random number generated successfully.+  @retval FALSE        Failed to
> > generate
> > > > the random number.++**/+BOOLEAN+EFIAPI+GetRandomNumber128 (+
> > OUT
> > > > UINT64                    *Rand+  )+{+  ASSERT (Rand != NULL);++  //+  // Read
> first
> > 64
> > > > bits+  //+  if (!GetRandomNumber64 (Rand)) {+    return FALSE;+  }++  //+  //
> > > > Read second 64 bits+  //+  return GetRandomNumber64 (++Rand);+}diff
> > > > --git a/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf
> > > > b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf
> > > > new file mode 100644
> > > > index 0000000000..67a91ccfff
> > > > --- /dev/null
> > > > +++ b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf
> > > > @@ -0,0 +1,32 @@
> > > > +## @file+#  Instance of RNG (Random Number Generator) Library.+#+#
> > > > Copyright (c) 2020 9elements Agency GmbH.<BR>+#+#  SPDX-License-
> > Identifier:
> > > > BSD-2-Clause-Patent+#+##++[Defines]+  INF_VERSION                    =
> > > > 0x00010005+  BASE_NAME                      = BaseRngLib+  MODULE_UNI_FILE
> > > > = BaseRngLib.uni+  FILE_GUID                      = 05C48431-DE18-4550-931A-
> > > > 3350E8551498+  MODULE_TYPE                    = BASE+  VERSION_STRING
> > > > = 1.0+  LIBRARY_CLASS                  = RngLib+  CONSTRUCTOR                    =
> > > > BaseRngLibConstructor++#+#  VALID_ARCHITECTURES           = IA32
> > > > X64+#++[Sources.Ia32, Sources.X64]+  BaseRng.c++[Packages]+
> > > > MdePkg/MdePkg.dec++[LibraryClasses]+  BaseLib+  DebugLibdiff --git
> > > > a/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni
> > > > b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni
> > > > new file mode 100644
> > > > index 0000000000..f3ed954c52
> > > > --- /dev/null
> > > > +++ b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni
> > > > @@ -0,0 +1,17 @@
> > > > +// /** @file+// Instance of RNG (Random Number Generator)
> > > > +Library.+//+//
> > > > BaseRng Library that uses CPU RdRand instruction access to
> > > > provide+// high- quality random numbers.+//+// Copyright (c) 2015,
> > > > Intel Corporation. All rights reserved.<BR>+//+// SPDX-License-Identifier:
> > BSD-2-Clause-Patent+//+//
> > > > **/+++#string STR_MODULE_ABSTRACT             #language en-US "Instance
> > of
> > > > RNG Library"++#string STR_MODULE_DESCRIPTION          #language en-US
> > > > "BaseRng Library that uses CPU RdRand instruction access to provide
> > > > high- quality random numbers"+diff --git
> > > > a/UefiPayloadPkg/UefiPayloadPkg.dsc
> > > > b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > > > index ae62a9c4d6..78a475ea02 100644
> > > > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> > > > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > > > @@ -494,6 +494,14 @@
> > > >  !endif   UefiPayloadPkg/GraphicsOutputDxe/GraphicsOutputDxe.inf +  #+
> #
> > > > Random Number Generator+  #+
> > > > SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf {+
> > > > <LibraryClasses>+
> > > > RngLib|UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf+  }+   #----------
> --
> > -----
> > > > -------------   #  Build the shell   #------------------------------diff --git
> > > > a/UefiPayloadPkg/UefiPayloadPkg.fdf
> > > > b/UefiPayloadPkg/UefiPayloadPkg.fdf
> > > > index a97ace7395..57c06c8621 100644
> > > > --- a/UefiPayloadPkg/UefiPayloadPkg.fdf
> > > > +++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
> > > > @@ -169,6 +169,10 @@ INF
> > > > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
> > > >  INF MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf INF
> > > > MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
> > +#+#
> > > > Random Number Generator+#+INF
> > > > SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf  # # Shell--
> > > > 2.26.2
> > >
> >
> >
> >
> >
> 
> 
> 
> 
>