Re: [edk2] [PATCH v3] Pkg-Module: Integrate new RngLib into RngDxe

[Kinney, Michael D]

LONG, Qin,

Do you know if the 10uS delay required in RdRandGetSeed128()?

Thanks,

Mike

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Long,
>Qin
>Sent: Wednesday, October 07, 2015 8:44 PM
>To: Justen, Jordan L; Thomas Palmer; edk2-devel@lists.01.org
>Subject: Re: [edk2] [PATCH v3] Pkg-Module: Integrate new RngLib into RngDxe
>
>Thomas,
>
>Thanks for doing this.
>I think it's better to separate your patch into two parts (one for MdePkg, and
>the other for SecurityPkg). Then you can CC me as Jordan's suggestion.
>
>
>Best Regards & Thanks,
>LONG, Qin
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Jordan Justen
>> Sent: Thursday, October 08, 2015 11:36 AM
>> To: Thomas Palmer; edk2-devel@lists.01.org
>> Subject: Re: [edk2] [PATCH v3] Pkg-Module: Integrate new RngLib into
>> RngDxe
>>
>> In the subject line, 'Pkg-Module' should be something like 'MdePkg:'
>> or 'SecurityPkg/RandomNumberGenerator:'.
>>
>> On 2015-10-07 18:55:42, Thomas Palmer wrote:
>> > Use the new RngLib to provide the IA32/X64 random data for RngDxe.
>> > Remove x86 specific functions from RdRand files.
>> > Clean up files in RngDxe/IA32 and RngDxe/X64 that are subsumed by files
>> in BaseRngLib.
>> > Simplify RngDxe by using WriteUnaligned64 for both IA32 and X64
>> platforms.
>> > Create and use GetRandomNumber128 in RngDxe to leverage 128 bit
>> > support found in some HW RNG devices
>>
>> It sounds like you are doing about 5 things here. How about 5 patches?
>>
>> Each patch should only touch one package whenever possiable.
>>
>> Also, consider adding 'Cc:' near your Signed-off-by for the maintainers of a
>> package. (see Maintainers.txt)
>>
>> -Jordan
>>
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Thomas Palmer <thomas.pal...@hpe.com>
>> > ---
>> >  MdePkg/Include/Library/RngLib.h|   17 ++
>> >  MdePkg/Library/BaseRngLib/BaseRng.c|   32 +++
>> >  .../RngDxe/IA32/AsmRdRand.asm  |   67 -
>> >  .../RandomNumberGenerator/RngDxe/IA32/GccRdRand.c  |   69 --
>> >  .../RandomNumberGenerator/RngDxe/IA32/RdRandWord.c |  104 
>> > SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.c  |  256
>> > ++--
>> SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.h  |  151 +---
>> >  SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c  |9 +-
>> >  .../RandomNumberGenerator/RngDxe/RngDxe.inf|   14 +-
>> >  .../RandomNumberGenerator/RngDxe/X64/AsmRdRand.asm |   83 ---
>> >  .../RandomNumberGenerator/RngDxe/X64/GccRdRand.c   |   95 
>> >  .../RandomNumberGenerator/RngDxe/X64/RdRandWord.c  |   70 --
>> >  SecurityPkg/SecurityPkg.dsc|3 +
>> >  13 files changed, 75 insertions(+), 895 deletions(-)  delete mode
>> > 100644
>> SecurityPkg/RandomNumberGenerator/RngDxe/IA32/AsmRdRand.asm
>> >  delete mode 100644
>> > SecurityPkg/RandomNumberGenerator/RngDxe/IA32/GccRdRand.c
>> >  delete mode 100644
>> > SecurityPkg/RandomNumberGenerator/RngDxe/IA32/RdRandWord.c
>> >  delete mode 100644
>> > SecurityPkg/RandomNumberGenerator/RngDxe/X64/AsmRdRand.asm
>> >  delete mode 100644
>> > SecurityPkg/RandomNumberGenerator/RngDxe/X64/GccRdRand.c
>> >  delete mode 100644
>> > SecurityPkg/RandomNumberGenerator/RngDxe/X64/RdRandWord.c
>> >
>> > diff --git a/MdePkg/Include/Library/RngLib.h
>> > b/MdePkg/Include/Library/RngLib.h index 157a931..ece4394 100644
>> > --- a/MdePkg/Include/Library/RngLib.h
>> > +++ b/MdePkg/Include/Library/RngLib.h
>> > @@ -66,4 +66,21 @@ GetRandomNumber64 (
>> >OUT UINT64*Rand
>> >);
>> >
>> > +/**
>> > +  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 FALSEFailed to generate the random number.
>> > +
>> > +**/
>> > +BOOLEAN
>> > +EFIAPI
>> > +GetRandomNumber128 (
>> > +  OUT UINT64*Rand
>> > +  );
>> > +
>> >  #endif  // __RNG_L

[edk2] [PATCH v3] Pkg-Module: Integrate new RngLib into RngDxe

[Thomas Palmer]

Use the new RngLib to provide the IA32/X64 random data for RngDxe.
Remove x86 specific functions from RdRand files.
Clean up files in RngDxe/IA32 and RngDxe/X64 that are subsumed by files in 
BaseRngLib.
Simplify RngDxe by using WriteUnaligned64 for both IA32 and X64 platforms.
Create and use GetRandomNumber128 in RngDxe to leverage 128 bit support found 
in some HW RNG devices

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Thomas Palmer 
---
 MdePkg/Include/Library/RngLib.h|   17 ++
 MdePkg/Library/BaseRngLib/BaseRng.c|   32 +++
 .../RngDxe/IA32/AsmRdRand.asm  |   67 -
 .../RandomNumberGenerator/RngDxe/IA32/GccRdRand.c  |   69 --
 .../RandomNumberGenerator/RngDxe/IA32/RdRandWord.c |  104 
 SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.c  |  256 ++--
 SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.h  |  151 +---
 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c  |9 +-
 .../RandomNumberGenerator/RngDxe/RngDxe.inf|   14 +-
 .../RandomNumberGenerator/RngDxe/X64/AsmRdRand.asm |   83 ---
 .../RandomNumberGenerator/RngDxe/X64/GccRdRand.c   |   95 
 .../RandomNumberGenerator/RngDxe/X64/RdRandWord.c  |   70 --
 SecurityPkg/SecurityPkg.dsc|3 +
 13 files changed, 75 insertions(+), 895 deletions(-)
 delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/IA32/AsmRdRand.asm
 delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/IA32/GccRdRand.c
 delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/IA32/RdRandWord.c
 delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/X64/AsmRdRand.asm
 delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/X64/GccRdRand.c
 delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/X64/RdRandWord.c

diff --git a/MdePkg/Include/Library/RngLib.h b/MdePkg/Include/Library/RngLib.h
index 157a931..ece4394 100644
--- a/MdePkg/Include/Library/RngLib.h
+++ b/MdePkg/Include/Library/RngLib.h
@@ -66,4 +66,21 @@ GetRandomNumber64 (
   OUT UINT64*Rand
   );
 
+/**
+  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 FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber128 (
+  OUT UINT64*Rand
+  );
+
 #endif  // __RNG_LIB_H__
diff --git a/MdePkg/Library/BaseRngLib/BaseRng.c 
b/MdePkg/Library/BaseRngLib/BaseRng.c
index 279df30..2c8df56 100644
--- a/MdePkg/Library/BaseRngLib/BaseRng.c
+++ b/MdePkg/Library/BaseRngLib/BaseRng.c
@@ -155,3 +155,35 @@ GetRandomNumber64 (
 
   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 FALSEFailed 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/SecurityPkg/RandomNumberGenerator/RngDxe/IA32/AsmRdRand.asm 
b/SecurityPkg/RandomNumberGenerator/RngDxe/IA32/AsmRdRand.asm
deleted file mode 100644
index 37b3830..000
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/IA32/AsmRdRand.asm
+++ /dev/null
@@ -1,67 +0,0 @@
-;--
-;
-; Copyright (c) 2013, Intel Corporation. All rights reserved.
-; This program and the accompanying materials
-; are licensed and made available under the terms and conditions of the BSD 
License
-; which accompanies this distribution.  The full text of the license may be 
found at
-; http://opensource.org/licenses/bsd-license.php.
-;
-; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-;
-; Module Name:
-;
-;   AsmRdRand.Asm
-;
-; Abstract:
-;
-;   Implementation for 16-, and 32- invocations of RDRAND instruction under 
32bit platform.
-;
-; Notes:
-;
-;   Visual Studio coding practices do not use inline asm since multiple 
compilers and 
-;   architectures are supported assembler not recognizing rdrand instruction 
so using DB's.
-;
-;--
-
-.586P
-.model flat, C
-.code
- 
-;--
-;  Generate a 16 bit random number
-;  Return TRUE if Rand generated successfully, or FALSE if not
-;
-;  BOOLEAN EFIAPI RdRand16Step (UINT16 *Rand);   ECX

Re: [edk2] [PATCH v3] Pkg-Module: Integrate new RngLib into RngDxe

[Jordan Justen]

In the subject line, 'Pkg-Module' should be something like 'MdePkg:'
or 'SecurityPkg/RandomNumberGenerator:'.

On 2015-10-07 18:55:42, Thomas Palmer wrote:
> Use the new RngLib to provide the IA32/X64 random data for RngDxe.
> Remove x86 specific functions from RdRand files.
> Clean up files in RngDxe/IA32 and RngDxe/X64 that are subsumed by files in 
> BaseRngLib.
> Simplify RngDxe by using WriteUnaligned64 for both IA32 and X64 platforms.
> Create and use GetRandomNumber128 in RngDxe to leverage 128 bit support found 
> in some HW RNG devices

It sounds like you are doing about 5 things here. How about 5 patches?

Each patch should only touch one package whenever possiable.

Also, consider adding 'Cc:' near your Signed-off-by for the
maintainers of a package. (see Maintainers.txt)

-Jordan

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Thomas Palmer 
> ---
>  MdePkg/Include/Library/RngLib.h|   17 ++
>  MdePkg/Library/BaseRngLib/BaseRng.c|   32 +++
>  .../RngDxe/IA32/AsmRdRand.asm  |   67 -
>  .../RandomNumberGenerator/RngDxe/IA32/GccRdRand.c  |   69 --
>  .../RandomNumberGenerator/RngDxe/IA32/RdRandWord.c |  104 
>  SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.c  |  256 
> ++--
>  SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.h  |  151 +---
>  SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c  |9 +-
>  .../RandomNumberGenerator/RngDxe/RngDxe.inf|   14 +-
>  .../RandomNumberGenerator/RngDxe/X64/AsmRdRand.asm |   83 ---
>  .../RandomNumberGenerator/RngDxe/X64/GccRdRand.c   |   95 
>  .../RandomNumberGenerator/RngDxe/X64/RdRandWord.c  |   70 --
>  SecurityPkg/SecurityPkg.dsc|3 +
>  13 files changed, 75 insertions(+), 895 deletions(-)
>  delete mode 100644 
> SecurityPkg/RandomNumberGenerator/RngDxe/IA32/AsmRdRand.asm
>  delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/IA32/GccRdRand.c
>  delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/IA32/RdRandWord.c
>  delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/X64/AsmRdRand.asm
>  delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/X64/GccRdRand.c
>  delete mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/X64/RdRandWord.c
> 
> diff --git a/MdePkg/Include/Library/RngLib.h b/MdePkg/Include/Library/RngLib.h
> index 157a931..ece4394 100644
> --- a/MdePkg/Include/Library/RngLib.h
> +++ b/MdePkg/Include/Library/RngLib.h
> @@ -66,4 +66,21 @@ GetRandomNumber64 (
>OUT UINT64*Rand
>);
>  
> +/**
> +  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 FALSEFailed to generate the random number.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +GetRandomNumber128 (
> +  OUT UINT64*Rand
> +  );
> +
>  #endif  // __RNG_LIB_H__
> diff --git a/MdePkg/Library/BaseRngLib/BaseRng.c 
> b/MdePkg/Library/BaseRngLib/BaseRng.c
> index 279df30..2c8df56 100644
> --- a/MdePkg/Library/BaseRngLib/BaseRng.c
> +++ b/MdePkg/Library/BaseRngLib/BaseRng.c
> @@ -155,3 +155,35 @@ GetRandomNumber64 (
>  
>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 FALSEFailed 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/SecurityPkg/RandomNumberGenerator/RngDxe/IA32/AsmRdRand.asm 
> b/SecurityPkg/RandomNumberGenerator/RngDxe/IA32/AsmRdRand.asm
> deleted file mode 100644
> index 37b3830..000
> --- a/SecurityPkg/RandomNumberGenerator/RngDxe/IA32/AsmRdRand.asm
> +++ /dev/null
> @@ -1,67 +0,0 @@
> -;--
> -;
> -; Copyright (c) 2013, Intel Corporation. All rights reserved.
> -; This program and the accompanying materials
> -; are licensed and made available under the terms and conditions of the BSD 
> License
> -; which accompanies this distribution.  The full text of the license may be 
> found at
> -; http://opensource.org/licenses/bsd-license.php.
> -;
> -; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> -;
> -; Module Name:
> -;
> -;   AsmRdRand.Asm
> -;
> -; Abstract:
> -;
> 

Re: [edk2] [PATCH v3] Pkg-Module: Integrate new RngLib into RngDxe

[Long, Qin]

Thomas,

Thanks for doing this.
I think it's better to separate your patch into two parts (one for MdePkg, and 
the other for SecurityPkg). Then you can CC me as Jordan's suggestion.


Best Regards & Thanks,
LONG, Qin

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jordan Justen
> Sent: Thursday, October 08, 2015 11:36 AM
> To: Thomas Palmer; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v3] Pkg-Module: Integrate new RngLib into
> RngDxe
> 
> In the subject line, 'Pkg-Module' should be something like 'MdePkg:'
> or 'SecurityPkg/RandomNumberGenerator:'.
> 
> On 2015-10-07 18:55:42, Thomas Palmer wrote:
> > Use the new RngLib to provide the IA32/X64 random data for RngDxe.
> > Remove x86 specific functions from RdRand files.
> > Clean up files in RngDxe/IA32 and RngDxe/X64 that are subsumed by files
> in BaseRngLib.
> > Simplify RngDxe by using WriteUnaligned64 for both IA32 and X64
> platforms.
> > Create and use GetRandomNumber128 in RngDxe to leverage 128 bit
> > support found in some HW RNG devices
> 
> It sounds like you are doing about 5 things here. How about 5 patches?
> 
> Each patch should only touch one package whenever possiable.
> 
> Also, consider adding 'Cc:' near your Signed-off-by for the maintainers of a
> package. (see Maintainers.txt)
> 
> -Jordan
> 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Thomas Palmer <thomas.pal...@hpe.com>
> > ---
> >  MdePkg/Include/Library/RngLib.h|   17 ++
> >  MdePkg/Library/BaseRngLib/BaseRng.c|   32 +++
> >  .../RngDxe/IA32/AsmRdRand.asm  |   67 -
> >  .../RandomNumberGenerator/RngDxe/IA32/GccRdRand.c  |   69 --
> >  .../RandomNumberGenerator/RngDxe/IA32/RdRandWord.c |  104 
> > SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.c  |  256
> > ++--
> SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.h  |  151 +---
> >  SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c  |9 +-
> >  .../RandomNumberGenerator/RngDxe/RngDxe.inf|   14 +-
> >  .../RandomNumberGenerator/RngDxe/X64/AsmRdRand.asm |   83 ---
> >  .../RandomNumberGenerator/RngDxe/X64/GccRdRand.c   |   95 
> >  .../RandomNumberGenerator/RngDxe/X64/RdRandWord.c  |   70 --
> >  SecurityPkg/SecurityPkg.dsc|3 +
> >  13 files changed, 75 insertions(+), 895 deletions(-)  delete mode
> > 100644
> SecurityPkg/RandomNumberGenerator/RngDxe/IA32/AsmRdRand.asm
> >  delete mode 100644
> > SecurityPkg/RandomNumberGenerator/RngDxe/IA32/GccRdRand.c
> >  delete mode 100644
> > SecurityPkg/RandomNumberGenerator/RngDxe/IA32/RdRandWord.c
> >  delete mode 100644
> > SecurityPkg/RandomNumberGenerator/RngDxe/X64/AsmRdRand.asm
> >  delete mode 100644
> > SecurityPkg/RandomNumberGenerator/RngDxe/X64/GccRdRand.c
> >  delete mode 100644
> > SecurityPkg/RandomNumberGenerator/RngDxe/X64/RdRandWord.c
> >
> > diff --git a/MdePkg/Include/Library/RngLib.h
> > b/MdePkg/Include/Library/RngLib.h index 157a931..ece4394 100644
> > --- a/MdePkg/Include/Library/RngLib.h
> > +++ b/MdePkg/Include/Library/RngLib.h
> > @@ -66,4 +66,21 @@ GetRandomNumber64 (
> >OUT UINT64*Rand
> >);
> >
> > +/**
> > +  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 FALSEFailed to generate the random number.
> > +
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +GetRandomNumber128 (
> > +  OUT UINT64*Rand
> > +  );
> > +
> >  #endif  // __RNG_LIB_H__
> > diff --git a/MdePkg/Library/BaseRngLib/BaseRng.c
> > b/MdePkg/Library/BaseRngLib/BaseRng.c
> > index 279df30..2c8df56 100644
> > --- a/MdePkg/Library/BaseRngLib/BaseRng.c
> > +++ b/MdePkg/Library/BaseRngLib/BaseRng.c
> > @@ -155,3 +155,35 @@ GetRandomNumber64 (
> >
> >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 FALSEFailed to generate the random number.
> > +
> > +**/
> > +BOOLEAN
>