* [PATCH v2 1/8] MdePkg: Definition for TRNG library class interface
2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls Sami Mujawar
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd
Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
The NIST Special Publications 800-90A, 800-90B and 800-90C
provide recommendations for random number generation. The
NIST 800-90C, Recommendation for Random Bit Generator (RBG)
Constructions, defines the GetEntropy() interface that is
used to access the entropy source. The GetEntropy() interface
is further used by Deterministic Random Bit Generators (DRBG)
to generate random numbers.
The True Random Number Generator (TRNG) library defines an
interface to access the entropy source on a platform. Some
platforms/architectures may provide access to the entropy
using a firmware interface. In such cases the TRNG library
shall be used to provide an abstraction.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
v2:
- MdePkg\Include\Library\TrngLib.h is base type [LIMING]
library. It can use RETURN_STATUS instead of
EFI_STATUS. Then, it doesn't need to include
<Uefi/UefiBaseType.h>
- Replaced EFI_STATUS with RETURN_STATUS and [SAMI]
removed include of /UefiBaseType.h.
- MdePkg\Include\Library\TrngLib.h API parameter [LIMING]
doesn't require CONST. CONST means the value
specified by the input pointer will not be
changed in API implementation.
- Removed the use of constant pointers in the [SAMI]
TRNG API.
MdePkg/Include/Library/TrngLib.h | 121 ++++++++++++++++++++
MdePkg/MdePkg.dec | 7 +-
2 files changed, 127 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/Library/TrngLib.h b/MdePkg/Include/Library/TrngLib.h
new file mode 100644
index 0000000000000000000000000000000000000000..082c83d0a6c67aa88c789a35b8f2f73ba7cf46cd
--- /dev/null
+++ b/MdePkg/Include/Library/TrngLib.h
@@ -0,0 +1,121 @@
+/** @file
+ TRNG interface library definitions.
+
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Reference(s):
+ - [1] Arm True Random Number Generator Firmware, Interface 1.0,
+ Platform Design Document.
+ (https://developer.arm.com/documentation/den0098/latest/)
+ - [2] NIST Special Publication 800-90A Revision 1, June 2015, Recommendation
+ for Random Number Generation Using Deterministic Random Bit Generators.
+ (https://csrc.nist.gov/publications/detail/sp/800-90a/rev-1/final)
+ - [3] NIST Special Publication 800-90B, Recommendation for the Entropy
+ Sources Used for Random Bit Generation.
+ (https://csrc.nist.gov/publications/detail/sp/800-90b/final)
+ - [4] (Second Draft) NIST Special Publication 800-90C, Recommendation for
+ Random Bit Generator (RBG) Constructions.
+ (https://csrc.nist.gov/publications/detail/sp/800-90c/draft)
+
+ @par Glossary:
+ - TRNG - True Random Number Generator
+**/
+#ifndef TRNG_LIB_H_
+#define TRNG_LIB_H_
+
+/** Get the version of the TRNG backend.
+
+ A TRNG may be implemented by the system firmware, in which case this
+ function shall return the version of the TRNG backend.
+ The implementation must return NOT_SUPPORTED if a Back end is not present.
+
+ @param [out] MajorRevision Major revision.
+ @param [out] MinorRevision Minor revision.
+
+ @retval RETURN_SUCCESS The function completed successfully.
+ @retval RETURN_INVALID_PARAMETER Invalid parameter.
+ @retval RETURN_UNSUPPORTED Backend not present.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngVersion (
+ OUT UINT16 *MajorRevision,
+ OUT UINT16 *MinorRevision
+ );
+
+/** Get the UUID of the TRNG backend.
+
+ A TRNG may be implemented by the system firmware, in which case this
+ function shall return the UUID of the TRNG backend.
+ Returning the TRNG UUID is optional and if not implemented, RETURN_UNSUPPORTED
+ shall be returned.
+
+ Note: The caller must not rely on the returned UUID as a trustworthy TRNG
+ Back end identity
+
+ @param [out] Guid UUID of the TRNG backend.
+
+ @retval RETURN_SUCCESS The function completed successfully.
+ @retval RETURN_INVALID_PARAMETER Invalid parameter.
+ @retval RETURN_UNSUPPORTED Function not implemented.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngUuid (
+ OUT GUID *Guid
+ );
+
+/** Returns maximum number of entropy bits that can be returned in a single
+ call.
+
+ @return Returns the maximum number of Entropy bits that can be returned
+ in a single call to GetEntropy().
+ If this feature is not supported MAX_UINTN is returned.
+**/
+UINTN
+EFIAPI
+GetTrngMaxSupportedEntropyBits (
+ VOID
+ );
+
+/** Returns N bits of conditioned entropy.
+
+ See [3] Section 2.3.1 GetEntropy: An Interface to the Entropy Source
+ GetEntropy
+ Input:
+ bits_of_entropy: the requested amount of entropy
+ Output:
+ entropy_bitstring: The string that provides the requested entropy.
+ status: A Boolean value that is TRUE if the request has been satisfied,
+ and is FALSE otherwise.
+
+ Note: In this implementation this function returns a status code instead
+ of a boolean value.
+ This is also compatible with the definition of Get_Entropy, see [4]
+ Section 7.4 Entropy Source Calls.
+ (status, entropy_bitstring) = Get_Entropy (
+ requested_entropy,
+ max_length
+ )
+
+ @param [in] EntropyBits Number of entropy bits requested.
+ @param [out] Buffer Buffer to return the entropy bits.
+ @param [in] Buffersize Size of the Buffer in bytes.
+
+ @retval RETURN_SUCCESS The function completed successfully.
+ @retval RETURN_INVALID_PARAMETER Invalid parameter.
+ @retval RETURN_UNSUPPORTED Function not implemented.
+ @retval RETURN_BAD_BUFFER_SIZE Buffer size is too small.
+ @retval RETURN_NOT_READY No Entropy available.
+**/
+RETURN_STATUS
+EFIAPI
+GetEntropy (
+ IN CONST UINTN EntropyBits,
+ OUT UINT8 *Buffer,
+ IN CONST UINTN BufferSize
+ );
+
+#endif // TRNG_LIB_H_
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 8b18415b107a03b11dc984341bb788cf9cd6e0ea..e612b5c57429d1af591de8e111f328e19a030ca0 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -7,6 +7,7 @@
# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
# Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
# (C) Copyright 2016 - 2021 Hewlett Packard Enterprise Development LP<BR>
+# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -267,11 +268,15 @@ [LibraryClasses]
#
RegisterFilterLib|Include/Library/RegisterFilterLib.h
-[LibraryClasses.IA32, LibraryClasses.X64, LibraryClasses.AARCH64]
+[LibraryClasses.IA32, LibraryClasses.X64, LibraryClasses.AARCH64, LibraryClasses.ARM]
## @libraryclass Provides services to generate random number.
#
RngLib|Include/Library/RngLib.h
+ ## @libraryclass Provides services to generate Entropy using a TRNG.
+ #
+ TrngLib|Include/Library/TrngLib.h
+
[LibraryClasses.IA32, LibraryClasses.X64]
## @libraryclass Abstracts both S/W SMI generation and detection.
##
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 1/8] MdePkg: Definition for TRNG library class interface Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
2021-11-24 12:07 ` Leif Lindholm
2021-11-16 11:32 ` [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library Sami Mujawar
` (5 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd
Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
meaning the SMC conduit is enabled as default.
Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
by virtual firmware implementations.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
v2:
- No code change since v1. Re-sending with V2 series. [SAMI]
ArmPkg/ArmPkg.dec | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
# Define if the GICv3 controller should use the GICv2 legacy
gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
+ ## Define the conduit to use for monitor calls.
+ # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
+ # If PcdMonitorConduitHvc = TRUE, conduit = HVC
+ gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
+
[PcdsFeatureFlag.ARM]
# Whether to map normal memory as non-shareable. FALSE is the safe choice, but
# TRUE may be appropriate to fix performance problems if you don't care about
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
2021-11-16 11:32 ` [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls Sami Mujawar
@ 2021-11-24 12:07 ` Leif Lindholm
2021-11-24 13:03 ` Ard Biesheuvel
0 siblings, 1 reply; 19+ messages in thread
From: Leif Lindholm @ 2021-11-24 12:07 UTC (permalink / raw)
To: Sami Mujawar, ardb+tianocore
Cc: devel, rebecca, kraxel, michael.d.kinney, gaoliming, zhiguang.liu,
jiewen.yao, jian.j.wang, Matteo.Carlini, Akanksha.Jain2,
Ben.Adderson, nd
Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
which reads its conduit out of the DT passed to it by QEMU?
/
Leif
On Tue, Nov 16, 2021 at 11:32:54 +0000, Sami Mujawar wrote:
> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
>
> Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
> monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
> meaning the SMC conduit is enabled as default.
>
> Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
> by virtual firmware implementations.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> Notes:
> v2:
> - No code change since v1. Re-sending with V2 series. [SAMI]
>
> ArmPkg/ArmPkg.dec | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
> # Define if the GICv3 controller should use the GICv2 legacy
> gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
>
> + ## Define the conduit to use for monitor calls.
> + # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> + # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> + gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
> +
> [PcdsFeatureFlag.ARM]
> # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
> # TRUE may be appropriate to fix performance problems if you don't care about
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
2021-11-24 12:07 ` Leif Lindholm
@ 2021-11-24 13:03 ` Ard Biesheuvel
2021-11-24 13:05 ` Leif Lindholm
0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2021-11-24 13:03 UTC (permalink / raw)
To: Leif Lindholm
Cc: Sami Mujawar, Ard Biesheuvel, edk2-devel-groups-io, Rebecca Cran,
Gerd Hoffmann, Michael Kinney, Liming Gao (Byosoft address),
Zhiguang Liu, Jiewen Yao, Jian J Wang, Matteo.Carlini,
Akanksha.Jain2, Ben.Adderson, nd
On Wed, 24 Nov 2021 at 13:07, Leif Lindholm <leif@nuviainc.com> wrote:
>
> Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> which reads its conduit out of the DT passed to it by QEMU?
>
As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
as before, I don't think those libraries are affected.
>
> On Tue, Nov 16, 2021 at 11:32:54 +0000, Sami Mujawar wrote:
> > Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
> >
> > Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
> > monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
> > meaning the SMC conduit is enabled as default.
> >
> > Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
> > by virtual firmware implementations.
> >
> > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > ---
> >
> > Notes:
> > v2:
> > - No code change since v1. Re-sending with V2 series. [SAMI]
> >
> > ArmPkg/ArmPkg.dec | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> > index 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87 100644
> > --- a/ArmPkg/ArmPkg.dec
> > +++ b/ArmPkg/ArmPkg.dec
> > @@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
> > # Define if the GICv3 controller should use the GICv2 legacy
> > gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
> >
> > + ## Define the conduit to use for monitor calls.
> > + # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> > + # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> > + gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
> > +
> > [PcdsFeatureFlag.ARM]
> > # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
> > # TRUE may be appropriate to fix performance problems if you don't care about
> > --
> > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
2021-11-24 13:03 ` Ard Biesheuvel
@ 2021-11-24 13:05 ` Leif Lindholm
2021-11-24 13:07 ` Ard Biesheuvel
0 siblings, 1 reply; 19+ messages in thread
From: Leif Lindholm @ 2021-11-24 13:05 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Sami Mujawar, Ard Biesheuvel, edk2-devel-groups-io, Rebecca Cran,
Gerd Hoffmann, Michael Kinney, Liming Gao (Byosoft address),
Zhiguang Liu, Jiewen Yao, Jian J Wang, Matteo.Carlini,
Akanksha.Jain2, Ben.Adderson, nd
On Wed, Nov 24, 2021 at 14:03:51 +0100, Ard Biesheuvel wrote:
> On Wed, 24 Nov 2021 at 13:07, Leif Lindholm <leif@nuviainc.com> wrote:
> >
> > Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> > which reads its conduit out of the DT passed to it by QEMU?
> >
>
> As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
> as before, I don't think those libraries are affected.
Well, I was thinking more that you can now end up with a firmware
build where the conduit is sometimes dynamically determined, and
sometimes compile-time determined.
/
Leif
> > On Tue, Nov 16, 2021 at 11:32:54 +0000, Sami Mujawar wrote:
> > > Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
> > >
> > > Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
> > > monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
> > > meaning the SMC conduit is enabled as default.
> > >
> > > Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
> > > by virtual firmware implementations.
> > >
> > > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > > ---
> > >
> > > Notes:
> > > v2:
> > > - No code change since v1. Re-sending with V2 series. [SAMI]
> > >
> > > ArmPkg/ArmPkg.dec | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> > > index 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87 100644
> > > --- a/ArmPkg/ArmPkg.dec
> > > +++ b/ArmPkg/ArmPkg.dec
> > > @@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
> > > # Define if the GICv3 controller should use the GICv2 legacy
> > > gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
> > >
> > > + ## Define the conduit to use for monitor calls.
> > > + # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> > > + # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> > > + gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
> > > +
> > > [PcdsFeatureFlag.ARM]
> > > # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
> > > # TRUE may be appropriate to fix performance problems if you don't care about
> > > --
> > > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> > >
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
2021-11-24 13:05 ` Leif Lindholm
@ 2021-11-24 13:07 ` Ard Biesheuvel
2021-11-24 13:25 ` Leif Lindholm
0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2021-11-24 13:07 UTC (permalink / raw)
To: Leif Lindholm
Cc: Sami Mujawar, Ard Biesheuvel, edk2-devel-groups-io, Rebecca Cran,
Gerd Hoffmann, Michael Kinney, Liming Gao (Byosoft address),
Zhiguang Liu, Jiewen Yao, Jian J Wang, Matteo.Carlini,
Akanksha.Jain2, Ben.Adderson, nd
On Wed, 24 Nov 2021 at 14:05, Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Wed, Nov 24, 2021 at 14:03:51 +0100, Ard Biesheuvel wrote:
> > On Wed, 24 Nov 2021 at 13:07, Leif Lindholm <leif@nuviainc.com> wrote:
> > >
> > > Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> > > which reads its conduit out of the DT passed to it by QEMU?
> > >
> >
> > As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
> > as before, I don't think those libraries are affected.
>
> Well, I was thinking more that you can now end up with a firmware
> build where the conduit is sometimes dynamically determined, and
> sometimes compile-time determined.
>
There is nothing wrong with that by itself: if you have a DT that
describes this, it makes sense to carry support for both. If the bare
metal you are targetting only ever uses SMC, no need for the discovery
and a PCD is fine.
Question remains whether this PCD could/should be configurable as
dynamic, so we collapse the static/dynamic options into a single PCD
based choice.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls
2021-11-24 13:07 ` Ard Biesheuvel
@ 2021-11-24 13:25 ` Leif Lindholm
0 siblings, 0 replies; 19+ messages in thread
From: Leif Lindholm @ 2021-11-24 13:25 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Sami Mujawar, Ard Biesheuvel, edk2-devel-groups-io, Rebecca Cran,
Gerd Hoffmann, Michael Kinney, Liming Gao (Byosoft address),
Zhiguang Liu, Jiewen Yao, Jian J Wang, Matteo.Carlini,
Akanksha.Jain2, Ben.Adderson, nd
On Wed, Nov 24, 2021 at 14:07:22 +0100, Ard Biesheuvel wrote:
> On Wed, 24 Nov 2021 at 14:05, Leif Lindholm <leif@nuviainc.com> wrote:
> >
> > On Wed, Nov 24, 2021 at 14:03:51 +0100, Ard Biesheuvel wrote:
> > > On Wed, 24 Nov 2021 at 13:07, Leif Lindholm <leif@nuviainc.com> wrote:
> > > >
> > > > Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> > > > which reads its conduit out of the DT passed to it by QEMU?
> > > >
> > >
> > > As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
> > > as before, I don't think those libraries are affected.
> >
> > Well, I was thinking more that you can now end up with a firmware
> > build where the conduit is sometimes dynamically determined, and
> > sometimes compile-time determined.
>
> There is nothing wrong with that by itself: if you have a DT that
> describes this, it makes sense to carry support for both. If the bare
> metal you are targetting only ever uses SMC, no need for the discovery
> and a PCD is fine.
>
> Question remains whether this PCD could/should be configurable as
> dynamic, so we collapse the static/dynamic options into a single PCD
> based choice.
That would feel cleaner. Although not required for this set.
/
Leif
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 1/8] MdePkg: Definition for TRNG library class interface Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
2021-11-24 13:01 ` [edk2-devel] " Leif Lindholm
2021-11-16 11:32 ` [PATCH v2 4/8] MdePkg: Add NULL instance of TRNG Library Sami Mujawar
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd
Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
The Arm True Random Number Generator Firmware, Interface 1.0,
Platform Design Document
(https://developer.arm.com/documentation/den0098/latest/)
defines an interface between an Operating System (OS) executing
at EL1 and Firmware (FW) exposing a conditioned entropy source
that is provided by a TRNG back end.
The conditioned entropy, that is provided by the TRNG FW interface,
is commonly used to seed deterministic random number generators.
This patch adds a TrngLib library that implements the Arm TRNG
firmware interface.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
v2:
- MdePkg\Include\Library\TrngLib.h is base type [LIMING]
library. It can use RETURN_STATUS instead of
EFI_STATUS.
- Replaced EFI_STATUS with RETURN_STATUS. [SAMI]
- MdePkg\Include\Library\TrngLib.h API parameter [LIMING]
doesn't require CONST. CONST means the value
specified by the input pointer will not be
changed in API implementation.
- Removed the use of constant pointers in the [SAMI]
TRNG API.
ArmPkg/ArmPkg.dsc | 1 +
ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h | 64 +++
ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c | 483 ++++++++++++++++++++
ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf | 34 ++
4 files changed, 582 insertions(+)
diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 59fd8f295d4f614cc68ee1021e691f94e279ab81..23df68c5eb53df11de5d96bde4949f3c833c9b2c 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -156,6 +156,7 @@ [Components.common]
ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
+ ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
new file mode 100644
index 0000000000000000000000000000000000000000..42236e743d972df0df205b1565496afeff5785f3
--- /dev/null
+++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
@@ -0,0 +1,64 @@
+/** @file
+ Arm Firmware TRNG definitions.
+
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Reference(s):
+ - [1] Arm True Random Number Generator Firmware, Interface 1.0,
+ Platform Design Document.
+ (https://developer.arm.com/documentation/den0098/latest/)
+
+ @par Glossary:
+ - TRNG - True Random Number Generator
+ - FID - Function ID
+**/
+
+#ifndef ARM_FW_TRNG_DEFS_H_
+#define ARM_FW_TRNG_DEFS_H_
+
+// Firmware TRNG interface Function IDs
+#define FID_TRNG_VERSION 0x84000050
+#define FID_TRNG_FEATURES 0x84000051
+#define FID_TRNG_GET_UUID 0x84000052
+#define FID_TRNG_RND_AARCH32 0x84000053
+#define FID_TRNG_RND_AARCH64 0xC4000053
+
+// Firmware TRNG revision mask and shift
+#define TRNG_REV_MAJOR_MASK 0x7FFF
+#define TRNG_REV_MINOR_MASK 0xFFFF
+#define TRNG_REV_MAJOR_SHIFT 16
+#define TRNG_REV_MINOR_SHIFT 0
+
+// Firmware TRNG status codes
+#define TRNG_STATUS_SUCCESS (INT32)(0)
+#define TRNG_NOT_SUPPORTED (INT32)(-1)
+#define TRNG_INVALID_PARAMETER (INT32)(-2)
+#define TRNG_NO_ENTROPY (INT32)(-3)
+
+#if defined (MDE_CPU_ARM)
+/** FID to use on AArch32 platform to request entropy.
+*/
+#define FID_TRNG_RND FID_TRNG_RND_AARCH32
+
+/** Maximum bits of entropy supported on AArch32.
+*/
+#define MAX_ENTROPY_BITS 96
+#elif defined (MDE_CPU_AARCH64)
+/** FID to use on AArch64 platform to request entropy.
+*/
+#define FID_TRNG_RND FID_TRNG_RND_AARCH64
+
+/** Maximum bits of entropy supported on AArch64.
+*/
+#define MAX_ENTROPY_BITS 192
+#else
+#error "Firmware TRNG not supported. Unknown chipset."
+#endif
+
+/** Typedef for SMC or HVC arguments.
+*/
+typedef ARM_SMC_ARGS ARM_MONITOR_ARGS;
+
+#endif // ARM_FW_TRNG_DEFS_H_
diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..314e7ffbc232ae90bbb77306f9c7113ce63012c8
--- /dev/null
+++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
@@ -0,0 +1,483 @@
+/** @file
+ Arm Firmware TRNG interface library.
+
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Reference(s):
+ - [1] Arm True Random Number Generator Firmware, Interface 1.0,
+ Platform Design Document.
+ (https://developer.arm.com/documentation/den0098/latest/)
+ - [2] NIST Special Publication 800-90A Revision 1, June 2015, Recommendation
+ for Random Number Generation Using Deterministic Random Bit Generators.
+ (https://csrc.nist.gov/publications/detail/sp/800-90a/rev-1/final)
+ - [3] NIST Special Publication 800-90B, Recommendation for the Entropy
+ Sources Used for Random Bit Generation.
+ (https://csrc.nist.gov/publications/detail/sp/800-90b/final)
+ - [4] (Second Draft) NIST Special Publication 800-90C, Recommendation for
+ Random Bit Generator (RBG) Constructions.
+ (https://csrc.nist.gov/publications/detail/sp/800-90c/draft)
+
+ @par Glossary:
+ - TRNG - True Random Number Generator
+ - FID - Function ID
+**/
+
+#include <Base.h>
+#include <Library/ArmHvcLib.h>
+#include <Library/ArmLib.h>
+#include <Library/ArmSmcLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+
+#include "ArmFwTrngDefs.h"
+
+/** Convert TRNG status codes to EFI status codes.
+
+ @param [in] TrngStatus TRNG status code.
+
+ @retval RETURN_SUCCESS Success.
+ @retval RETURN_UNSUPPORTED Function not implemented.
+ @retval RETURN_INVALID_PARAMETER A parameter is invalid.
+ @retval RETURN_NOT_READY No Entropy available.
+**/
+STATIC
+RETURN_STATUS
+TrngStatusToEfiStatus (
+ IN INT32 TrngStatus
+ )
+{
+ switch (TrngStatus) {
+ case TRNG_NOT_SUPPORTED:
+ return RETURN_UNSUPPORTED;
+
+ case TRNG_INVALID_PARAMETER:
+ return RETURN_INVALID_PARAMETER;
+
+ case TRNG_NO_ENTROPY:
+ return RETURN_NOT_READY;
+
+ case TRNG_STATUS_SUCCESS:
+ default:
+ return RETURN_SUCCESS;
+ }
+}
+
+/** Invoke the monitor call using the appropriate conduit.
+ If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
+
+ @param [in, out] Args Arguments passed to and returned from the monitor.
+
+ @return VOID
+**/
+STATIC
+VOID
+ArmCallMonitor (
+ IN OUT ARM_MONITOR_ARGS *Args
+ )
+{
+ if (FeaturePcdGet (PcdMonitorConduitHvc)) {
+ ArmCallHvc ((ARM_HVC_ARGS*)Args);
+ } else {
+ ArmCallSmc ((ARM_SMC_ARGS*)Args);
+ }
+}
+
+/** Get the version of the TRNG backend.
+
+ A TRNG may be implemented by the system firmware, in which case this
+ function shall return the version of the TRNG backend.
+ The implementation must return NOT_SUPPORTED if a Back end is not present.
+
+ @param [out] MajorRevision Major revision.
+ @param [out] MinorRevision Minor revision.
+
+ @retval RETURN_SUCCESS The function completed successfully.
+ @retval RETURN_INVALID_PARAMETER Invalid parameter.
+ @retval RETURN_UNSUPPORTED Backend not present.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngVersion (
+ OUT UINT16 *MajorRevision,
+ OUT UINT16 *MinorRevision
+ )
+{
+ RETURN_STATUS Status;
+ ARM_MONITOR_ARGS Parameters;
+ INT32 Revision;
+
+ if ((MajorRevision == NULL) || (MinorRevision == NULL)) {
+ return RETURN_INVALID_PARAMETER;
+ }
+
+ ZeroMem (&Parameters, sizeof (Parameters));
+
+ /*
+ Cf. [1], 2.1 TRNG_VERSION
+ Function ID (W0) 0x8400_0050
+ Parameters
+ W1-W7 Reserved (MBZ)
+ Returns
+ Success (W0 > 0) W0[31] MBZ
+ W0[30:16] Major revision
+ W0[15:0] Minor revision
+ W1 - W3 Reserved (MBZ)
+ Error (W0 < 0)
+ NOT_SUPPORTED Function not implemented
+ */
+ Parameters.Arg0 = FID_TRNG_VERSION;
+ ArmCallMonitor (&Parameters);
+
+ Revision = (INT32)Parameters.Arg0;
+ // Convert status codes to EFI status codes.
+ Status = TrngStatusToEfiStatus (Revision);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ *MinorRevision = (Revision & TRNG_REV_MINOR_MASK);
+ *MajorRevision = ((Revision >> TRNG_REV_MAJOR_SHIFT) & TRNG_REV_MAJOR_MASK);
+ return RETURN_SUCCESS;
+}
+
+#ifndef MDEPKG_NDEBUG
+/** Get the features supported by the TRNG backend.
+
+ The caller can determine if functions defined in the TRNG ABI are
+ present in the ABI implementation.
+
+ @param [in] FunctionId Function Id.
+ @param [out] Capability Function specific capability if present
+ otherwise Zero is returned.
+
+ @retval RETURN_SUCCESS The function completed successfully.
+ @retval RETURN_INVALID_PARAMETER Invalid parameter.
+ @retval RETURN_UNSUPPORTED Function not implemented.
+**/
+STATIC
+RETURN_STATUS
+EFIAPI
+GetTrngFeatures (
+ IN CONST UINT32 FunctionId,
+ OUT UINT32 *Capability OPTIONAL
+ )
+{
+ ARM_MONITOR_ARGS Parameters;
+
+ ZeroMem (&Parameters, sizeof (Parameters));
+
+ /*
+ Cf. [1], Section 2.2 TRNG_FEATURES
+ Function ID (W0) 0x8400_0051
+ Parameters
+ W1 trng_func_id
+ W2-W7 Reserved (MBZ)
+ Returns
+ Success (W0 >= 0)
+ SUCCESS Function is implemented.
+ > 0 Function is implemented and
+ has specific capabilities,
+ see function definition.
+ Error (W0 < 0)
+ NOT_SUPPORTED Function with FID=trng_func_id
+ is not implemented
+ */
+ Parameters.Arg0 = FID_TRNG_FEATURES;
+ Parameters.Arg1 = FunctionId;
+ ArmCallMonitor (&Parameters);
+ if (Parameters.Arg0 < TRNG_STATUS_SUCCESS) {
+ return RETURN_UNSUPPORTED;
+ }
+
+ if (Capability != NULL) {
+ *Capability = Parameters.Arg0;
+ }
+
+ return RETURN_SUCCESS;
+}
+#endif //MDEPKG_NDEBUG
+
+/** Get the UUID of the TRNG backend.
+
+ A TRNG may be implemented by the system firmware, in which case this
+ function shall return the UUID of the TRNG backend.
+ Returning the TRNG UUID is optional and if not implemented, RETURN_UNSUPPORTED
+ shall be returned.
+
+ Note: The caller must not rely on the returned UUID as a trustworthy TRNG
+ Back end identity
+
+ @param [out] Guid UUID of the TRNG backend.
+
+ @retval RETURN_SUCCESS The function completed successfully.
+ @retval RETURN_INVALID_PARAMETER Invalid parameter.
+ @retval RETURN_UNSUPPORTED Function not implemented.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngUuid (
+ OUT GUID *Guid
+ )
+{
+ RETURN_STATUS Status;
+ ARM_MONITOR_ARGS Parameters;
+
+ ZeroMem (&Parameters, sizeof (Parameters));
+
+ /*
+ Cf. [1], Section 2.3 TRNG_GET_UUID
+ Function ID (W0) 0x8400_0052
+ Parameters
+ W1-W7 Reserved (MBZ)
+ Returns
+ Success (W0 != -1)
+ W0 UUID[31:0]
+ W1 UUID[63:32]
+ W2 UUID[95:64]
+ W3 UUID[127:96]
+ Error (W0 = -1)
+ W0 NOT_SUPPORTED
+ */
+ Parameters.Arg0 = FID_TRNG_GET_UUID;
+ ArmCallMonitor (&Parameters);
+
+ // Convert status codes to EFI status codes.
+ Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Guid->Data1 = (Parameters.Arg0 & MAX_UINT32);
+ Guid->Data2 = (Parameters.Arg1 & MAX_UINT16);
+ Guid->Data3 = ((Parameters.Arg1 >> 16) & MAX_UINT16);
+
+ Guid->Data4[0] = (Parameters.Arg2 & MAX_UINT8);
+ Guid->Data4[1] = ((Parameters.Arg2 >> 8) & MAX_UINT8);
+ Guid->Data4[2] = ((Parameters.Arg2 >> 16) & MAX_UINT8);
+ Guid->Data4[3] = ((Parameters.Arg2 >> 24) & MAX_UINT8);
+
+ Guid->Data4[4] = (Parameters.Arg3 & MAX_UINT8);
+ Guid->Data4[5] = ((Parameters.Arg3 >> 8) & MAX_UINT8);
+ Guid->Data4[6] = ((Parameters.Arg3 >> 16) & MAX_UINT8);
+ Guid->Data4[7] = ((Parameters.Arg3 >> 24) & MAX_UINT8);
+
+ DEBUG ((DEBUG_INFO, "FW-TRNG: UUID %g\n", Guid));
+
+ return RETURN_SUCCESS;
+}
+
+/** Returns maximum number of entropy bits that can be returned in a single
+ call.
+
+ @return Returns the maximum number of Entropy bits that can be returned
+ in a single call to GetEntropy().
+**/
+UINTN
+EFIAPI
+GetTrngMaxSupportedEntropyBits (
+ VOID
+ )
+{
+ return MAX_ENTROPY_BITS;
+}
+
+/** Returns N bits of conditioned entropy.
+
+ See [3] Section 2.3.1 GetEntropy: An Interface to the Entropy Source
+ GetEntropy
+ Input:
+ bits_of_entropy: the requested amount of entropy
+ Output:
+ entropy_bitstring: The string that provides the requested entropy.
+ status: A Boolean value that is TRUE if the request has been satisfied,
+ and is FALSE otherwise.
+
+ Note: In this implementation this function returns a status code instead
+ of a boolean value.
+ This is also compatible with the definition of Get_Entropy, see [2]
+ Section 7.4 Entropy Source Calls.
+ (status, entropy_bitstring) = Get_Entropy (
+ requested_entropy,
+ max_length
+ )
+
+ @param [in] EntropyBits Number of entropy bits requested.
+ @param [out] Buffer Buffer to return the entropy bits.
+ @param [in] BufferSize Size of the Buffer in bytes.
+
+ @retval RETURN_SUCCESS The function completed successfully.
+ @retval RETURN_INVALID_PARAMETER Invalid parameter.
+ @retval RETURN_UNSUPPORTED Function not implemented.
+ @retval RETURN_BAD_BUFFER_SIZE Buffer size is too small.
+ @retval RETURN_NOT_READY No Entropy available.
+**/
+RETURN_STATUS
+EFIAPI
+GetEntropy (
+ IN CONST UINTN EntropyBits,
+ OUT UINT8 *Buffer,
+ IN CONST UINTN BufferSize
+ )
+{
+ RETURN_STATUS Status;
+ ARM_MONITOR_ARGS Parameters;
+ UINTN EntropyBytes;
+ UINTN LastValidBits;
+ UINTN ArgSelector;
+ UINTN BytesToClear;
+
+ // [1] Section 2.4.3 Caller responsibilities.
+ // The caller cannot request more than MAX_BITS bits of conditioned
+ // entropy per call.
+ if ((EntropyBits == 0) || (EntropyBits > MAX_ENTROPY_BITS)) {
+ return RETURN_INVALID_PARAMETER;
+ }
+
+ EntropyBytes = (EntropyBits + 7) >> 3;
+ if (EntropyBytes > BufferSize) {
+ return RETURN_BAD_BUFFER_SIZE;
+ }
+
+ ZeroMem (Buffer, BufferSize);
+ ZeroMem (&Parameters, sizeof (Parameters));
+
+ /*
+ Cf. [1], Section 2.4 TRNG_RND
+ Function ID (W0) 0x8400_0053
+ 0xC400_0053
+ SMC32 Parameters
+ W1 N bits of entropy (1 6 N 6 96)
+ W2-W7 Reserved (MBZ)
+ SMC64 Parameters
+ X1 N bits of entropy (1 6 N 6 192)
+ X2-X7 Reserved (MBZ)
+ SMC32 Returns
+ Success (W0 = 0):
+ W0 MBZ
+ W1 Entropy[95:64]
+ W2 Entropy[63:32]
+ W3 Entropy[31:0]
+ Error (W0 < 0)
+ W0 NOT_SUPPORTED
+ NO_ENTROPY
+ INVALID_PARAMETERS
+ W1 - W3 Reserved (MBZ)
+ SMC64 Returns
+ Success (X0 = 0):
+ X0 MBZ
+ X1 Entropy[191:128]
+ X2 Entropy[127:64]
+ X3 Entropy[63:0]
+ Error (X0 < 0)
+ X0 NOT_SUPPORTED
+ NO_ENTROPY
+ INVALID_PARAMETERS
+ X1 - X3 Reserved (MBZ)
+ */
+ Parameters.Arg0 = FID_TRNG_RND;
+ Parameters.Arg1 = EntropyBits;
+ ArmCallMonitor (&Parameters);
+
+ // Convert status codes to EFI status codes.
+ Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ // Extract Data
+ // ArgSelector = ((EntropyBytes + 3) >> 2); for AArch32
+ // ArgSelector = ((EntropyBytes + 7) >> 3); for AArch64
+ // ((sizeof (UINTN) >> 2) + 1) is 3 or 2 depending on size of UINTN
+ ArgSelector = ((EntropyBytes + (sizeof (UINTN) - 1)) >>
+ ((sizeof (UINTN) >> 2) + 1));
+
+ switch (ArgSelector) {
+ case 3:
+ CopyMem (&Buffer[(sizeof (UINTN) * 2)], &Parameters.Arg1, sizeof (UINTN));
+
+ case 2:
+ CopyMem (&Buffer[sizeof (UINTN)], &Parameters.Arg2, sizeof (UINTN));
+
+ case 1:
+ CopyMem (&Buffer[0], &Parameters.Arg3, sizeof (UINTN));
+ break;
+
+ default:
+ ASSERT (0);
+ return RETURN_INVALID_PARAMETER;
+ } // switch
+
+
+ // [1] Section 2.4.3 Caller responsibilities.
+ // The caller must ensure that only the value in Entropy[N-1:0] is consumed
+ // and that the remaining bits in Entropy[MAX_BITS-1:N] are ignored.
+ // Therefore, Clear the unused upper bytes.
+ BytesToClear = (sizeof (UINTN) * ArgSelector) - EntropyBytes;
+ if (BytesToClear != 0) {
+ ZeroMem (&Buffer[EntropyBytes], BytesToClear);
+ }
+
+ // Clear the unused MSB bits of the last byte.
+ LastValidBits = EntropyBits & 0x7;
+ if (LastValidBits != 0) {
+ Buffer[EntropyBytes - 1] &= (0xFF >> (8 - LastValidBits));
+ }
+
+ return Status;
+}
+
+/** The constructor checks that the FW-TRNG interface is supported
+ by the host firmware.
+
+ It will ASSERT() if FW-TRNG is not supported.
+ It will always return RETURN_SUCCESS.
+
+ @retval RETURN_SUCCESS The constructor always returns RETURN_SUCCESS.
+**/
+RETURN_STATUS
+EFIAPI
+ArmFwTrngLibConstructor (
+ VOID
+ )
+{
+ RETURN_STATUS Status;
+ UINT16 MajorRev;
+ UINT16 MinorRev;
+ GUID Guid;
+
+ Status = GetTrngVersion (&MajorRev, &MinorRev);
+ if (EFI_ERROR (Status)) {
+ return RETURN_SUCCESS;
+ }
+
+#ifndef MDEPKG_NDEBUG
+ // Check that the required features are present.
+ Status = GetTrngFeatures (FID_TRNG_RND, NULL);
+ if (EFI_ERROR (Status)) {
+ return RETURN_SUCCESS;
+ }
+
+ // Check if TRNG UUID is supported and if so trace the GUID.
+ Status = GetTrngFeatures (FID_TRNG_GET_UUID, NULL);
+ if (EFI_ERROR (Status)) {
+ return RETURN_SUCCESS;
+ }
+#endif
+
+ Status = GetTrngUuid (&Guid);
+ if (EFI_ERROR (Status)) {
+ return RETURN_SUCCESS;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "FW-TRNG: Version %d.%d, GUID {%g}\n",
+ MajorRev,
+ MinorRev,
+ Guid
+ ));
+
+ return RETURN_SUCCESS;
+}
diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..4b2c58251fbe8fbcb5af308736db014e8d954720
--- /dev/null
+++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
@@ -0,0 +1,34 @@
+## @file
+# Arm Firmware TRNG interface library.
+#
+# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = ArmFwTrngLib
+ FILE_GUID = 10DE97C9-28E4-4C9B-A53E-8D7D1B0DD4E0
+ VERSION_STRING = 1.0
+ MODULE_TYPE = BASE
+ LIBRARY_CLASS = TrngLib
+ CONSTRUCTOR = ArmFwTrngLibConstructor
+
+[Sources]
+ ArmFwTrngDefs.h
+ ArmFwTrngLib.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ ArmSmcLib
+ ArmHvcLib
+ BaseLib
+ BaseMemoryLib
+
+[Pcd]
+ gArmTokenSpaceGuid.PcdMonitorConduitHvc
+
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
2021-11-16 11:32 ` [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library Sami Mujawar
@ 2021-11-24 13:01 ` Leif Lindholm
2021-11-25 15:23 ` Sami Mujawar
0 siblings, 1 reply; 19+ messages in thread
From: Leif Lindholm @ 2021-11-24 13:01 UTC (permalink / raw)
To: devel, sami.mujawar
Cc: ardb+tianocore, rebecca, kraxel, michael.d.kinney, gaoliming,
zhiguang.liu, jiewen.yao, jian.j.wang, Matteo.Carlini,
Akanksha.Jain2, Ben.Adderson, nd
Hi Sami,
On Tue, Nov 16, 2021 at 11:32:55 +0000, Sami Mujawar wrote:
> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
>
> The Arm True Random Number Generator Firmware, Interface 1.0,
> Platform Design Document
> (https://developer.arm.com/documentation/den0098/latest/)
> defines an interface between an Operating System (OS) executing
> at EL1 and Firmware (FW) exposing a conditioned entropy source
> that is provided by a TRNG back end.
>
> The conditioned entropy, that is provided by the TRNG FW interface,
> is commonly used to seed deterministic random number generators.
>
> This patch adds a TrngLib library that implements the Arm TRNG
> firmware interface.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> Notes:
> v2:
> - MdePkg\Include\Library\TrngLib.h is base type [LIMING]
> library. It can use RETURN_STATUS instead of
> EFI_STATUS.
> - Replaced EFI_STATUS with RETURN_STATUS. [SAMI]
> - MdePkg\Include\Library\TrngLib.h API parameter [LIMING]
> doesn't require CONST. CONST means the value
> specified by the input pointer will not be
> changed in API implementation.
> - Removed the use of constant pointers in the [SAMI]
> TRNG API.
>
> ArmPkg/ArmPkg.dsc | 1 +
> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h | 64 +++
> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c | 483 ++++++++++++++++++++
> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf | 34 ++
> 4 files changed, 582 insertions(+)
>
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index 59fd8f295d4f614cc68ee1021e691f94e279ab81..23df68c5eb53df11de5d96bde4949f3c833c9b2c 100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -156,6 +156,7 @@ [Components.common]
> ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
> ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
>
> + ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
> ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
> ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..42236e743d972df0df205b1565496afeff5785f3
> --- /dev/null
> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
> @@ -0,0 +1,64 @@
> +/** @file
> + Arm Firmware TRNG definitions.
> +
> + Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + @par Reference(s):
> + - [1] Arm True Random Number Generator Firmware, Interface 1.0,
> + Platform Design Document.
> + (https://developer.arm.com/documentation/den0098/latest/)
> +
> + @par Glossary:
> + - TRNG - True Random Number Generator
> + - FID - Function ID
> +**/
> +
> +#ifndef ARM_FW_TRNG_DEFS_H_
> +#define ARM_FW_TRNG_DEFS_H_
> +
> +// Firmware TRNG interface Function IDs
> +#define FID_TRNG_VERSION 0x84000050
> +#define FID_TRNG_FEATURES 0x84000051
> +#define FID_TRNG_GET_UUID 0x84000052
> +#define FID_TRNG_RND_AARCH32 0x84000053
> +#define FID_TRNG_RND_AARCH64 0xC4000053
Do these belong in ArmStdSmc.h?
> +
> +// Firmware TRNG revision mask and shift
> +#define TRNG_REV_MAJOR_MASK 0x7FFF
> +#define TRNG_REV_MINOR_MASK 0xFFFF
> +#define TRNG_REV_MAJOR_SHIFT 16
> +#define TRNG_REV_MINOR_SHIFT 0
> +
> +// Firmware TRNG status codes
> +#define TRNG_STATUS_SUCCESS (INT32)(0)
> +#define TRNG_NOT_SUPPORTED (INT32)(-1)
> +#define TRNG_INVALID_PARAMETER (INT32)(-2)
> +#define TRNG_NO_ENTROPY (INT32)(-3)
And the rest of the stuff to here, really?
> +#if defined (MDE_CPU_ARM)
> +/** FID to use on AArch32 platform to request entropy.
> +*/
> +#define FID_TRNG_RND FID_TRNG_RND_AARCH32
> +
> +/** Maximum bits of entropy supported on AArch32.
> +*/
> +#define MAX_ENTROPY_BITS 96
> +#elif defined (MDE_CPU_AARCH64)
> +/** FID to use on AArch64 platform to request entropy.
> +*/
> +#define FID_TRNG_RND FID_TRNG_RND_AARCH64
> +
> +/** Maximum bits of entropy supported on AArch64.
> +*/
> +#define MAX_ENTROPY_BITS 192
> +#else
> +#error "Firmware TRNG not supported. Unknown chipset."
> +#endif
> +
> +/** Typedef for SMC or HVC arguments.
> +*/
> +typedef ARM_SMC_ARGS ARM_MONITOR_ARGS;
> +
> +#endif // ARM_FW_TRNG_DEFS_H_
> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..314e7ffbc232ae90bbb77306f9c7113ce63012c8
> --- /dev/null
> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
> @@ -0,0 +1,483 @@
> +/** @file
> + Arm Firmware TRNG interface library.
> +
> + Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + @par Reference(s):
> + - [1] Arm True Random Number Generator Firmware, Interface 1.0,
> + Platform Design Document.
> + (https://developer.arm.com/documentation/den0098/latest/)
> + - [2] NIST Special Publication 800-90A Revision 1, June 2015, Recommendation
> + for Random Number Generation Using Deterministic Random Bit Generators.
> + (https://csrc.nist.gov/publications/detail/sp/800-90a/rev-1/final)
> + - [3] NIST Special Publication 800-90B, Recommendation for the Entropy
> + Sources Used for Random Bit Generation.
> + (https://csrc.nist.gov/publications/detail/sp/800-90b/final)
> + - [4] (Second Draft) NIST Special Publication 800-90C, Recommendation for
> + Random Bit Generator (RBG) Constructions.
> + (https://csrc.nist.gov/publications/detail/sp/800-90c/draft)
> +
> + @par Glossary:
> + - TRNG - True Random Number Generator
> + - FID - Function ID
> +**/
> +
> +#include <Base.h>
> +#include <Library/ArmHvcLib.h>
> +#include <Library/ArmLib.h>
> +#include <Library/ArmSmcLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +
> +#include "ArmFwTrngDefs.h"
> +
> +/** Convert TRNG status codes to EFI status codes.
> +
> + @param [in] TrngStatus TRNG status code.
> +
> + @retval RETURN_SUCCESS Success.
> + @retval RETURN_UNSUPPORTED Function not implemented.
> + @retval RETURN_INVALID_PARAMETER A parameter is invalid.
> + @retval RETURN_NOT_READY No Entropy available.
> +**/
> +STATIC
> +RETURN_STATUS
> +TrngStatusToEfiStatus (
> + IN INT32 TrngStatus
> + )
> +{
> + switch (TrngStatus) {
> + case TRNG_NOT_SUPPORTED:
> + return RETURN_UNSUPPORTED;
> +
> + case TRNG_INVALID_PARAMETER:
> + return RETURN_INVALID_PARAMETER;
> +
> + case TRNG_NO_ENTROPY:
> + return RETURN_NOT_READY;
> +
> + case TRNG_STATUS_SUCCESS:
> + default:
> + return RETURN_SUCCESS;
> + }
> +}
> +
> +/** Invoke the monitor call using the appropriate conduit.
> + If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
> +
> + @param [in, out] Args Arguments passed to and returned from the monitor.
> +
> + @return VOID
> +**/
> +STATIC
> +VOID
> +ArmCallMonitor (
> + IN OUT ARM_MONITOR_ARGS *Args
> + )
> +{
> + if (FeaturePcdGet (PcdMonitorConduitHvc)) {
> + ArmCallHvc ((ARM_HVC_ARGS*)Args);
> + } else {
> + ArmCallSmc ((ARM_SMC_ARGS*)Args);
> + }
> +}
Should this be in (a potentially renamed) ArmSmcLib?
> +
> +/** Get the version of the TRNG backend.
> +
> + A TRNG may be implemented by the system firmware, in which case this
> + function shall return the version of the TRNG backend.
> + The implementation must return NOT_SUPPORTED if a Back end is not present.
> +
> + @param [out] MajorRevision Major revision.
> + @param [out] MinorRevision Minor revision.
> +
> + @retval RETURN_SUCCESS The function completed successfully.
> + @retval RETURN_INVALID_PARAMETER Invalid parameter.
> + @retval RETURN_UNSUPPORTED Backend not present.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +GetTrngVersion (
> + OUT UINT16 *MajorRevision,
> + OUT UINT16 *MinorRevision
> + )
> +{
> + RETURN_STATUS Status;
> + ARM_MONITOR_ARGS Parameters;
> + INT32 Revision;
> +
> + if ((MajorRevision == NULL) || (MinorRevision == NULL)) {
> + return RETURN_INVALID_PARAMETER;
> + }
> +
> + ZeroMem (&Parameters, sizeof (Parameters));
> +
> + /*
> + Cf. [1], 2.1 TRNG_VERSION
> + Function ID (W0) 0x8400_0050
> + Parameters
> + W1-W7 Reserved (MBZ)
> + Returns
> + Success (W0 > 0) W0[31] MBZ
> + W0[30:16] Major revision
> + W0[15:0] Minor revision
> + W1 - W3 Reserved (MBZ)
> + Error (W0 < 0)
> + NOT_SUPPORTED Function not implemented
> + */
I have a comment on the placement of API descriptions further down.
> + Parameters.Arg0 = FID_TRNG_VERSION;
> + ArmCallMonitor (&Parameters);
> +
> + Revision = (INT32)Parameters.Arg0;
> + // Convert status codes to EFI status codes.
> + Status = TrngStatusToEfiStatus (Revision);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + *MinorRevision = (Revision & TRNG_REV_MINOR_MASK);
> + *MajorRevision = ((Revision >> TRNG_REV_MAJOR_SHIFT) & TRNG_REV_MAJOR_MASK);
> + return RETURN_SUCCESS;
> +}
> +
> +#ifndef MDEPKG_NDEBUG
> +/** Get the features supported by the TRNG backend.
> +
> + The caller can determine if functions defined in the TRNG ABI are
> + present in the ABI implementation.
> +
> + @param [in] FunctionId Function Id.
> + @param [out] Capability Function specific capability if present
> + otherwise Zero is returned.
> +
> + @retval RETURN_SUCCESS The function completed successfully.
> + @retval RETURN_INVALID_PARAMETER Invalid parameter.
> + @retval RETURN_UNSUPPORTED Function not implemented.
> +**/
> +STATIC
> +RETURN_STATUS
> +EFIAPI
> +GetTrngFeatures (
> + IN CONST UINT32 FunctionId,
> + OUT UINT32 *Capability OPTIONAL
> + )
> +{
> + ARM_MONITOR_ARGS Parameters;
> +
> + ZeroMem (&Parameters, sizeof (Parameters));
> +
> + /*
> + Cf. [1], Section 2.2 TRNG_FEATURES
> + Function ID (W0) 0x8400_0051
> + Parameters
> + W1 trng_func_id
> + W2-W7 Reserved (MBZ)
> + Returns
> + Success (W0 >= 0)
> + SUCCESS Function is implemented.
> + > 0 Function is implemented and
> + has specific capabilities,
> + see function definition.
> + Error (W0 < 0)
> + NOT_SUPPORTED Function with FID=trng_func_id
> + is not implemented
> + */
I have a comment on the placement of API descriptions further down.
> + Parameters.Arg0 = FID_TRNG_FEATURES;
> + Parameters.Arg1 = FunctionId;
> + ArmCallMonitor (&Parameters);
> + if (Parameters.Arg0 < TRNG_STATUS_SUCCESS) {
> + return RETURN_UNSUPPORTED;
> + }
> +
> + if (Capability != NULL) {
> + *Capability = Parameters.Arg0;
> + }
> +
> + return RETURN_SUCCESS;
> +}
> +#endif //MDEPKG_NDEBUG
> +
> +/** Get the UUID of the TRNG backend.
> +
> + A TRNG may be implemented by the system firmware, in which case this
> + function shall return the UUID of the TRNG backend.
> + Returning the TRNG UUID is optional and if not implemented, RETURN_UNSUPPORTED
> + shall be returned.
> +
> + Note: The caller must not rely on the returned UUID as a trustworthy TRNG
> + Back end identity
> +
> + @param [out] Guid UUID of the TRNG backend.
> +
> + @retval RETURN_SUCCESS The function completed successfully.
> + @retval RETURN_INVALID_PARAMETER Invalid parameter.
> + @retval RETURN_UNSUPPORTED Function not implemented.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +GetTrngUuid (
> + OUT GUID *Guid
> + )
> +{
> + RETURN_STATUS Status;
> + ARM_MONITOR_ARGS Parameters;
> +
> + ZeroMem (&Parameters, sizeof (Parameters));
> +
> + /*
> + Cf. [1], Section 2.3 TRNG_GET_UUID
> + Function ID (W0) 0x8400_0052
> + Parameters
> + W1-W7 Reserved (MBZ)
> + Returns
> + Success (W0 != -1)
> + W0 UUID[31:0]
> + W1 UUID[63:32]
> + W2 UUID[95:64]
> + W3 UUID[127:96]
> + Error (W0 = -1)
> + W0 NOT_SUPPORTED
> + */
> + Parameters.Arg0 = FID_TRNG_GET_UUID;
> + ArmCallMonitor (&Parameters);
> +
> + // Convert status codes to EFI status codes.
> + Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Guid->Data1 = (Parameters.Arg0 & MAX_UINT32);
> + Guid->Data2 = (Parameters.Arg1 & MAX_UINT16);
> + Guid->Data3 = ((Parameters.Arg1 >> 16) & MAX_UINT16);
> +
> + Guid->Data4[0] = (Parameters.Arg2 & MAX_UINT8);
> + Guid->Data4[1] = ((Parameters.Arg2 >> 8) & MAX_UINT8);
> + Guid->Data4[2] = ((Parameters.Arg2 >> 16) & MAX_UINT8);
> + Guid->Data4[3] = ((Parameters.Arg2 >> 24) & MAX_UINT8);
> +
> + Guid->Data4[4] = (Parameters.Arg3 & MAX_UINT8);
> + Guid->Data4[5] = ((Parameters.Arg3 >> 8) & MAX_UINT8);
> + Guid->Data4[6] = ((Parameters.Arg3 >> 16) & MAX_UINT8);
> + Guid->Data4[7] = ((Parameters.Arg3 >> 24) & MAX_UINT8);
> +
> + DEBUG ((DEBUG_INFO, "FW-TRNG: UUID %g\n", Guid));
> +
> + return RETURN_SUCCESS;
> +}
> +
> +/** Returns maximum number of entropy bits that can be returned in a single
> + call.
> +
> + @return Returns the maximum number of Entropy bits that can be returned
> + in a single call to GetEntropy().
> +**/
> +UINTN
> +EFIAPI
> +GetTrngMaxSupportedEntropyBits (
> + VOID
> + )
> +{
> + return MAX_ENTROPY_BITS;
> +}
> +
> +/** Returns N bits of conditioned entropy.
> +
> + See [3] Section 2.3.1 GetEntropy: An Interface to the Entropy Source
> + GetEntropy
> + Input:
> + bits_of_entropy: the requested amount of entropy
> + Output:
> + entropy_bitstring: The string that provides the requested entropy.
> + status: A Boolean value that is TRUE if the request has been satisfied,
> + and is FALSE otherwise.
> +
> + Note: In this implementation this function returns a status code instead
> + of a boolean value.
> + This is also compatible with the definition of Get_Entropy, see [2]
> + Section 7.4 Entropy Source Calls.
> + (status, entropy_bitstring) = Get_Entropy (
> + requested_entropy,
> + max_length
> + )
> +
> + @param [in] EntropyBits Number of entropy bits requested.
> + @param [out] Buffer Buffer to return the entropy bits.
> + @param [in] BufferSize Size of the Buffer in bytes.
> +
> + @retval RETURN_SUCCESS The function completed successfully.
> + @retval RETURN_INVALID_PARAMETER Invalid parameter.
> + @retval RETURN_UNSUPPORTED Function not implemented.
> + @retval RETURN_BAD_BUFFER_SIZE Buffer size is too small.
> + @retval RETURN_NOT_READY No Entropy available.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +GetEntropy (
> + IN CONST UINTN EntropyBits,
> + OUT UINT8 *Buffer,
> + IN CONST UINTN BufferSize
> + )
> +{
> + RETURN_STATUS Status;
> + ARM_MONITOR_ARGS Parameters;
> + UINTN EntropyBytes;
> + UINTN LastValidBits;
> + UINTN ArgSelector;
> + UINTN BytesToClear;
> +
> + // [1] Section 2.4.3 Caller responsibilities.
> + // The caller cannot request more than MAX_BITS bits of conditioned
> + // entropy per call.
Comment is redundant, code is clearer without it.
> + if ((EntropyBits == 0) || (EntropyBits > MAX_ENTROPY_BITS)) {
> + return RETURN_INVALID_PARAMETER;
> + }
> +
> + EntropyBytes = (EntropyBits + 7) >> 3;
> + if (EntropyBytes > BufferSize) {
Not for later: we're verifying the value of EntropyBytes here - if
there are more aspects of it that need verifying, that should also be
done here.
> + return RETURN_BAD_BUFFER_SIZE;
> + }
> +
> + ZeroMem (Buffer, BufferSize);
> + ZeroMem (&Parameters, sizeof (Parameters));
> +
> + /*
> + Cf. [1], Section 2.4 TRNG_RND
> + Function ID (W0) 0x8400_0053
> + 0xC400_0053
> + SMC32 Parameters
> + W1 N bits of entropy (1 6 N 6 96)
> + W2-W7 Reserved (MBZ)
> + SMC64 Parameters
> + X1 N bits of entropy (1 6 N 6 192)
> + X2-X7 Reserved (MBZ)
> + SMC32 Returns
> + Success (W0 = 0):
> + W0 MBZ
> + W1 Entropy[95:64]
> + W2 Entropy[63:32]
> + W3 Entropy[31:0]
> + Error (W0 < 0)
> + W0 NOT_SUPPORTED
> + NO_ENTROPY
> + INVALID_PARAMETERS
> + W1 - W3 Reserved (MBZ)
> + SMC64 Returns
> + Success (X0 = 0):
> + X0 MBZ
> + X1 Entropy[191:128]
> + X2 Entropy[127:64]
> + X3 Entropy[63:0]
> + Error (X0 < 0)
> + X0 NOT_SUPPORTED
> + NO_ENTROPY
> + INVALID_PARAMETERS
> + X1 - X3 Reserved (MBZ)
> + */
The above comment block completely wrecks the readability of the
function.
Would suggest putting it in the header file describing the monitor
call. For our SIP SVC calls, we've done this in the following form:
/*
* SMC call to retrieve number of CPUs present in the system.
* Input values:
* x0: NUVIA_SIP_GET_NUM_CPUS
* Return values:
* x0: SMC_OK
* x1: Number of CPUs present
*/
#define NUVIA_SIP_GET_NUM_CPUS SIP_FUNCTION_ID(0x20)
(Where SIP_FUNCTION_ID is one of a set of macros I should submit for
addition to ArmStdSmc.h)
> + Parameters.Arg0 = FID_TRNG_RND;
> + Parameters.Arg1 = EntropyBits;
> + ArmCallMonitor (&Parameters);
> +
> + // Convert status codes to EFI status codes.
Function name already says this, comment redundant.
> + Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
>From here
> + // Extract Data
> + // ArgSelector = ((EntropyBytes + 3) >> 2); for AArch32
> + // ArgSelector = ((EntropyBytes + 7) >> 3); for AArch64
> + // ((sizeof (UINTN) >> 2) + 1) is 3 or 2 depending on size of UINTN
> + ArgSelector = ((EntropyBytes + (sizeof (UINTN) - 1)) >>
> + ((sizeof (UINTN) >> 2) + 1));
> +
> + switch (ArgSelector) {
> + case 3:
> + CopyMem (&Buffer[(sizeof (UINTN) * 2)], &Parameters.Arg1, sizeof (UINTN));
> +
> + case 2:
> + CopyMem (&Buffer[sizeof (UINTN)], &Parameters.Arg2, sizeof (UINTN));
> +
> + case 1:
> + CopyMem (&Buffer[0], &Parameters.Arg3, sizeof (UINTN));
> + break;
> +
> + default:
> + ASSERT (0);
> + return RETURN_INVALID_PARAMETER;
> + } // switch
to here ... I'm not convinced you yourself would be able to read or
explain this code a few months down the line.
Is there a strong reason for why Buffer cannot be a UINTN *?
I think what this code is doing can equally be written as:
Buffer[0] = Parameters.Arg3;
if ((EntropyBytes / sizeof (UINTN)) > 1) {
Buffer[1] = Parameters.Arg2;
}
if ((EntropyBytes / sizeof (UINTN)) > 2) {
Buffer[2] = Parameters.Arg1;
}
> +
> +
> + // [1] Section 2.4.3 Caller responsibilities.
> + // The caller must ensure that only the value in Entropy[N-1:0] is consumed
> + // and that the remaining bits in Entropy[MAX_BITS-1:N] are ignored.
> + // Therefore, Clear the unused upper bytes.
This is source code, not the specification.
// Mask off any unused top bytes, in accordance with specification
is sufficient as a comment.
/
Leif
> + BytesToClear = (sizeof (UINTN) * ArgSelector) - EntropyBytes;
> + if (BytesToClear != 0) {
> + ZeroMem (&Buffer[EntropyBytes], BytesToClear);
> + }
> +
> + // Clear the unused MSB bits of the last byte.
> + LastValidBits = EntropyBits & 0x7;
> + if (LastValidBits != 0) {
> + Buffer[EntropyBytes - 1] &= (0xFF >> (8 - LastValidBits));
> + }
> +
> + return Status;
> +}
> +
> +/** The constructor checks that the FW-TRNG interface is supported
> + by the host firmware.
> +
> + It will ASSERT() if FW-TRNG is not supported.
> + It will always return RETURN_SUCCESS.
> +
> + @retval RETURN_SUCCESS The constructor always returns RETURN_SUCCESS.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +ArmFwTrngLibConstructor (
> + VOID
> + )
> +{
> + RETURN_STATUS Status;
> + UINT16 MajorRev;
> + UINT16 MinorRev;
> + GUID Guid;
> +
> + Status = GetTrngVersion (&MajorRev, &MinorRev);
> + if (EFI_ERROR (Status)) {
> + return RETURN_SUCCESS;
> + }
> +
> +#ifndef MDEPKG_NDEBUG
> + // Check that the required features are present.
> + Status = GetTrngFeatures (FID_TRNG_RND, NULL);
> + if (EFI_ERROR (Status)) {
> + return RETURN_SUCCESS;
> + }
> +
> + // Check if TRNG UUID is supported and if so trace the GUID.
> + Status = GetTrngFeatures (FID_TRNG_GET_UUID, NULL);
> + if (EFI_ERROR (Status)) {
> + return RETURN_SUCCESS;
> + }
> +#endif
> +
> + Status = GetTrngUuid (&Guid);
> + if (EFI_ERROR (Status)) {
> + return RETURN_SUCCESS;
> + }
> +
> + DEBUG ((
> + DEBUG_INFO,
> + "FW-TRNG: Version %d.%d, GUID {%g}\n",
> + MajorRev,
> + MinorRev,
> + Guid
> + ));
> +
> + return RETURN_SUCCESS;
> +}
> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
> new file mode 100644
> index 0000000000000000000000000000000000000000..4b2c58251fbe8fbcb5af308736db014e8d954720
> --- /dev/null
> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
> @@ -0,0 +1,34 @@
> +## @file
> +# Arm Firmware TRNG interface library.
> +#
> +# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> + INF_VERSION = 0x0001001B
> + BASE_NAME = ArmFwTrngLib
> + FILE_GUID = 10DE97C9-28E4-4C9B-A53E-8D7D1B0DD4E0
> + VERSION_STRING = 1.0
> + MODULE_TYPE = BASE
> + LIBRARY_CLASS = TrngLib
> + CONSTRUCTOR = ArmFwTrngLibConstructor
> +
> +[Sources]
> + ArmFwTrngDefs.h
> + ArmFwTrngLib.c
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> + ArmSmcLib
> + ArmHvcLib
> + BaseLib
> + BaseMemoryLib
> +
> +[Pcd]
> + gArmTokenSpaceGuid.PcdMonitorConduitHvc
> +
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
2021-11-24 13:01 ` [edk2-devel] " Leif Lindholm
@ 2021-11-25 15:23 ` Sami Mujawar
2022-03-24 9:46 ` PierreGondois
[not found] ` <80941d66-5d31-053f-388a-95efe5dbbfdf@arm.com>
0 siblings, 2 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-25 15:23 UTC (permalink / raw)
To: Leif Lindholm, devel
Cc: ardb+tianocore, rebecca, kraxel, michael.d.kinney, gaoliming,
zhiguang.liu, jiewen.yao, jian.j.wang, Matteo.Carlini,
Akanksha.Jain2, Ben.Adderson, nd
Hi Leif,
Thank you for the feedback.
Please find my response inline marked [SAMI].
Regards,
Sami Mujawar
On 24/11/2021 01:01 PM, Leif Lindholm wrote:
> Hi Sami,
>
> On Tue, Nov 16, 2021 at 11:32:55 +0000, Sami Mujawar wrote:
>> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
>>
>> The Arm True Random Number Generator Firmware, Interface 1.0,
>> Platform Design Document
>> (https://developer.arm.com/documentation/den0098/latest/)
>> defines an interface between an Operating System (OS) executing
>> at EL1 and Firmware (FW) exposing a conditioned entropy source
>> that is provided by a TRNG back end.
>>
>> The conditioned entropy, that is provided by the TRNG FW interface,
>> is commonly used to seed deterministic random number generators.
>>
>> This patch adds a TrngLib library that implements the Arm TRNG
>> firmware interface.
>>
>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>> ---
>>
>> Notes:
>> v2:
>> - MdePkg\Include\Library\TrngLib.h is base type [LIMING]
>> library. It can use RETURN_STATUS instead of
>> EFI_STATUS.
>> - Replaced EFI_STATUS with RETURN_STATUS. [SAMI]
>> - MdePkg\Include\Library\TrngLib.h API parameter [LIMING]
>> doesn't require CONST. CONST means the value
>> specified by the input pointer will not be
>> changed in API implementation.
>> - Removed the use of constant pointers in the [SAMI]
>> TRNG API.
>>
>> ArmPkg/ArmPkg.dsc | 1 +
>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h | 64 +++
>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c | 483 ++++++++++++++++++++
>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf | 34 ++
>> 4 files changed, 582 insertions(+)
>>
>> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
>> index 59fd8f295d4f614cc68ee1021e691f94e279ab81..23df68c5eb53df11de5d96bde4949f3c833c9b2c 100644
>> --- a/ArmPkg/ArmPkg.dsc
>> +++ b/ArmPkg/ArmPkg.dsc
>> @@ -156,6 +156,7 @@ [Components.common]
>> ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
>> ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
>>
>> + ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
>> ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
>> ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>> ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
>> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..42236e743d972df0df205b1565496afeff5785f3
>> --- /dev/null
>> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
>> @@ -0,0 +1,64 @@
>> +/** @file
>> + Arm Firmware TRNG definitions.
>> +
>> + Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> + @par Reference(s):
>> + - [1] Arm True Random Number Generator Firmware, Interface 1.0,
>> + Platform Design Document.
>> + (https://developer.arm.com/documentation/den0098/latest/)
>> +
>> + @par Glossary:
>> + - TRNG - True Random Number Generator
>> + - FID - Function ID
>> +**/
>> +
>> +#ifndef ARM_FW_TRNG_DEFS_H_
>> +#define ARM_FW_TRNG_DEFS_H_
>> +
>> +// Firmware TRNG interface Function IDs
>> +#define FID_TRNG_VERSION 0x84000050
>> +#define FID_TRNG_FEATURES 0x84000051
>> +#define FID_TRNG_GET_UUID 0x84000052
>> +#define FID_TRNG_RND_AARCH32 0x84000053
>> +#define FID_TRNG_RND_AARCH64 0xC4000053
> Do these belong in ArmStdSmc.h?
[SAMI] I will fix this in the next version.
>
>> +
>> +// Firmware TRNG revision mask and shift
>> +#define TRNG_REV_MAJOR_MASK 0x7FFF
>> +#define TRNG_REV_MINOR_MASK 0xFFFF
>> +#define TRNG_REV_MAJOR_SHIFT 16
>> +#define TRNG_REV_MINOR_SHIFT 0
>> +
>> +// Firmware TRNG status codes
>> +#define TRNG_STATUS_SUCCESS (INT32)(0)
>> +#define TRNG_NOT_SUPPORTED (INT32)(-1)
>> +#define TRNG_INVALID_PARAMETER (INT32)(-2)
>> +#define TRNG_NO_ENTROPY (INT32)(-3)
> And the rest of the stuff to here, really?
[SAMI] I will fix this in the next version.
>
>> +#if defined (MDE_CPU_ARM)
>> +/** FID to use on AArch32 platform to request entropy.
>> +*/
>> +#define FID_TRNG_RND FID_TRNG_RND_AARCH32
>> +
>> +/** Maximum bits of entropy supported on AArch32.
>> +*/
>> +#define MAX_ENTROPY_BITS 96
>> +#elif defined (MDE_CPU_AARCH64)
>> +/** FID to use on AArch64 platform to request entropy.
>> +*/
>> +#define FID_TRNG_RND FID_TRNG_RND_AARCH64
>> +
>> +/** Maximum bits of entropy supported on AArch64.
>> +*/
>> +#define MAX_ENTROPY_BITS 192
>> +#else
>> +#error "Firmware TRNG not supported. Unknown chipset."
>> +#endif
>> +
>> +/** Typedef for SMC or HVC arguments.
>> +*/
>> +typedef ARM_SMC_ARGS ARM_MONITOR_ARGS;
>> +
>> +#endif // ARM_FW_TRNG_DEFS_H_
>> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..314e7ffbc232ae90bbb77306f9c7113ce63012c8
>> --- /dev/null
>> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
>> @@ -0,0 +1,483 @@
>> +/** @file
>> + Arm Firmware TRNG interface library.
>> +
>> + Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> + @par Reference(s):
>> + - [1] Arm True Random Number Generator Firmware, Interface 1.0,
>> + Platform Design Document.
>> + (https://developer.arm.com/documentation/den0098/latest/)
>> + - [2] NIST Special Publication 800-90A Revision 1, June 2015, Recommendation
>> + for Random Number Generation Using Deterministic Random Bit Generators.
>> + (https://csrc.nist.gov/publications/detail/sp/800-90a/rev-1/final)
>> + - [3] NIST Special Publication 800-90B, Recommendation for the Entropy
>> + Sources Used for Random Bit Generation.
>> + (https://csrc.nist.gov/publications/detail/sp/800-90b/final)
>> + - [4] (Second Draft) NIST Special Publication 800-90C, Recommendation for
>> + Random Bit Generator (RBG) Constructions.
>> + (https://csrc.nist.gov/publications/detail/sp/800-90c/draft)
>> +
>> + @par Glossary:
>> + - TRNG - True Random Number Generator
>> + - FID - Function ID
>> +**/
>> +
>> +#include <Base.h>
>> +#include <Library/ArmHvcLib.h>
>> +#include <Library/ArmLib.h>
>> +#include <Library/ArmSmcLib.h>
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/DebugLib.h>
>> +
>> +#include "ArmFwTrngDefs.h"
>> +
>> +/** Convert TRNG status codes to EFI status codes.
>> +
>> + @param [in] TrngStatus TRNG status code.
>> +
>> + @retval RETURN_SUCCESS Success.
>> + @retval RETURN_UNSUPPORTED Function not implemented.
>> + @retval RETURN_INVALID_PARAMETER A parameter is invalid.
>> + @retval RETURN_NOT_READY No Entropy available.
>> +**/
>> +STATIC
>> +RETURN_STATUS
>> +TrngStatusToEfiStatus (
>> + IN INT32 TrngStatus
>> + )
>> +{
>> + switch (TrngStatus) {
>> + case TRNG_NOT_SUPPORTED:
>> + return RETURN_UNSUPPORTED;
>> +
>> + case TRNG_INVALID_PARAMETER:
>> + return RETURN_INVALID_PARAMETER;
>> +
>> + case TRNG_NO_ENTROPY:
>> + return RETURN_NOT_READY;
>> +
>> + case TRNG_STATUS_SUCCESS:
>> + default:
>> + return RETURN_SUCCESS;
>> + }
>> +}
>> +
>> +/** Invoke the monitor call using the appropriate conduit.
>> + If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
>> +
>> + @param [in, out] Args Arguments passed to and returned from the monitor.
>> +
>> + @return VOID
>> +**/
>> +STATIC
>> +VOID
>> +ArmCallMonitor (
>> + IN OUT ARM_MONITOR_ARGS *Args
>> + )
>> +{
>> + if (FeaturePcdGet (PcdMonitorConduitHvc)) {
>> + ArmCallHvc ((ARM_HVC_ARGS*)Args);
>> + } else {
>> + ArmCallSmc ((ARM_SMC_ARGS*)Args);
>> + }
>> +}
> Should this be in (a potentially renamed) ArmSmcLib?
[SAMI] Looking at ArmSmcLib and ArmHvcLib libraries there is not much
difference in the code other than the SMC/HVC call. Please let me know
if I should submit a patch to unify these in ArmMonitorLib?
The ArmCall<Smc|Hvc> APIs would still remain the same but moved to
ArmMonitorLib.
>
>> +
>> +/** Get the version of the TRNG backend.
>> +
>> + A TRNG may be implemented by the system firmware, in which case this
>> + function shall return the version of the TRNG backend.
>> + The implementation must return NOT_SUPPORTED if a Back end is not present.
>> +
>> + @param [out] MajorRevision Major revision.
>> + @param [out] MinorRevision Minor revision.
>> +
>> + @retval RETURN_SUCCESS The function completed successfully.
>> + @retval RETURN_INVALID_PARAMETER Invalid parameter.
>> + @retval RETURN_UNSUPPORTED Backend not present.
>> +**/
>> +RETURN_STATUS
>> +EFIAPI
>> +GetTrngVersion (
>> + OUT UINT16 *MajorRevision,
>> + OUT UINT16 *MinorRevision
>> + )
>> +{
>> + RETURN_STATUS Status;
>> + ARM_MONITOR_ARGS Parameters;
>> + INT32 Revision;
>> +
>> + if ((MajorRevision == NULL) || (MinorRevision == NULL)) {
>> + return RETURN_INVALID_PARAMETER;
>> + }
>> +
>> + ZeroMem (&Parameters, sizeof (Parameters));
>> +
>> + /*
>> + Cf. [1], 2.1 TRNG_VERSION
>> + Function ID (W0) 0x8400_0050
>> + Parameters
>> + W1-W7 Reserved (MBZ)
>> + Returns
>> + Success (W0 > 0) W0[31] MBZ
>> + W0[30:16] Major revision
>> + W0[15:0] Minor revision
>> + W1 - W3 Reserved (MBZ)
>> + Error (W0 < 0)
>> + NOT_SUPPORTED Function not implemented
>> + */
> I have a comment on the placement of API descriptions further down.
>
>> + Parameters.Arg0 = FID_TRNG_VERSION;
>> + ArmCallMonitor (&Parameters);
>> +
>> + Revision = (INT32)Parameters.Arg0;
>> + // Convert status codes to EFI status codes.
>> + Status = TrngStatusToEfiStatus (Revision);
>> + if (EFI_ERROR (Status)) {
>> + return Status;
>> + }
>> +
>> + *MinorRevision = (Revision & TRNG_REV_MINOR_MASK);
>> + *MajorRevision = ((Revision >> TRNG_REV_MAJOR_SHIFT) & TRNG_REV_MAJOR_MASK);
>> + return RETURN_SUCCESS;
>> +}
>> +
>> +#ifndef MDEPKG_NDEBUG
>> +/** Get the features supported by the TRNG backend.
>> +
>> + The caller can determine if functions defined in the TRNG ABI are
>> + present in the ABI implementation.
>> +
>> + @param [in] FunctionId Function Id.
>> + @param [out] Capability Function specific capability if present
>> + otherwise Zero is returned.
>> +
>> + @retval RETURN_SUCCESS The function completed successfully.
>> + @retval RETURN_INVALID_PARAMETER Invalid parameter.
>> + @retval RETURN_UNSUPPORTED Function not implemented.
>> +**/
>> +STATIC
>> +RETURN_STATUS
>> +EFIAPI
>> +GetTrngFeatures (
>> + IN CONST UINT32 FunctionId,
>> + OUT UINT32 *Capability OPTIONAL
>> + )
>> +{
>> + ARM_MONITOR_ARGS Parameters;
>> +
>> + ZeroMem (&Parameters, sizeof (Parameters));
>> +
>> + /*
>> + Cf. [1], Section 2.2 TRNG_FEATURES
>> + Function ID (W0) 0x8400_0051
>> + Parameters
>> + W1 trng_func_id
>> + W2-W7 Reserved (MBZ)
>> + Returns
>> + Success (W0 >= 0)
>> + SUCCESS Function is implemented.
>> + > 0 Function is implemented and
>> + has specific capabilities,
>> + see function definition.
>> + Error (W0 < 0)
>> + NOT_SUPPORTED Function with FID=trng_func_id
>> + is not implemented
>> + */
> I have a comment on the placement of API descriptions further down.
>
>> + Parameters.Arg0 = FID_TRNG_FEATURES;
>> + Parameters.Arg1 = FunctionId;
>> + ArmCallMonitor (&Parameters);
>> + if (Parameters.Arg0 < TRNG_STATUS_SUCCESS) {
>> + return RETURN_UNSUPPORTED;
>> + }
>> +
>> + if (Capability != NULL) {
>> + *Capability = Parameters.Arg0;
>> + }
>> +
>> + return RETURN_SUCCESS;
>> +}
>> +#endif //MDEPKG_NDEBUG
>> +
>> +/** Get the UUID of the TRNG backend.
>> +
>> + A TRNG may be implemented by the system firmware, in which case this
>> + function shall return the UUID of the TRNG backend.
>> + Returning the TRNG UUID is optional and if not implemented, RETURN_UNSUPPORTED
>> + shall be returned.
>> +
>> + Note: The caller must not rely on the returned UUID as a trustworthy TRNG
>> + Back end identity
>> +
>> + @param [out] Guid UUID of the TRNG backend.
>> +
>> + @retval RETURN_SUCCESS The function completed successfully.
>> + @retval RETURN_INVALID_PARAMETER Invalid parameter.
>> + @retval RETURN_UNSUPPORTED Function not implemented.
>> +**/
>> +RETURN_STATUS
>> +EFIAPI
>> +GetTrngUuid (
>> + OUT GUID *Guid
>> + )
>> +{
>> + RETURN_STATUS Status;
>> + ARM_MONITOR_ARGS Parameters;
>> +
>> + ZeroMem (&Parameters, sizeof (Parameters));
>> +
>> + /*
>> + Cf. [1], Section 2.3 TRNG_GET_UUID
>> + Function ID (W0) 0x8400_0052
>> + Parameters
>> + W1-W7 Reserved (MBZ)
>> + Returns
>> + Success (W0 != -1)
>> + W0 UUID[31:0]
>> + W1 UUID[63:32]
>> + W2 UUID[95:64]
>> + W3 UUID[127:96]
>> + Error (W0 = -1)
>> + W0 NOT_SUPPORTED
>> + */
>> + Parameters.Arg0 = FID_TRNG_GET_UUID;
>> + ArmCallMonitor (&Parameters);
>> +
>> + // Convert status codes to EFI status codes.
>> + Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
>> + if (EFI_ERROR (Status)) {
>> + return Status;
>> + }
>> +
>> + Guid->Data1 = (Parameters.Arg0 & MAX_UINT32);
>> + Guid->Data2 = (Parameters.Arg1 & MAX_UINT16);
>> + Guid->Data3 = ((Parameters.Arg1 >> 16) & MAX_UINT16);
>> +
>> + Guid->Data4[0] = (Parameters.Arg2 & MAX_UINT8);
>> + Guid->Data4[1] = ((Parameters.Arg2 >> 8) & MAX_UINT8);
>> + Guid->Data4[2] = ((Parameters.Arg2 >> 16) & MAX_UINT8);
>> + Guid->Data4[3] = ((Parameters.Arg2 >> 24) & MAX_UINT8);
>> +
>> + Guid->Data4[4] = (Parameters.Arg3 & MAX_UINT8);
>> + Guid->Data4[5] = ((Parameters.Arg3 >> 8) & MAX_UINT8);
>> + Guid->Data4[6] = ((Parameters.Arg3 >> 16) & MAX_UINT8);
>> + Guid->Data4[7] = ((Parameters.Arg3 >> 24) & MAX_UINT8);
>> +
>> + DEBUG ((DEBUG_INFO, "FW-TRNG: UUID %g\n", Guid));
>> +
>> + return RETURN_SUCCESS;
>> +}
>> +
>> +/** Returns maximum number of entropy bits that can be returned in a single
>> + call.
>> +
>> + @return Returns the maximum number of Entropy bits that can be returned
>> + in a single call to GetEntropy().
>> +**/
>> +UINTN
>> +EFIAPI
>> +GetTrngMaxSupportedEntropyBits (
>> + VOID
>> + )
>> +{
>> + return MAX_ENTROPY_BITS;
>> +}
>> +
>> +/** Returns N bits of conditioned entropy.
>> +
>> + See [3] Section 2.3.1 GetEntropy: An Interface to the Entropy Source
>> + GetEntropy
>> + Input:
>> + bits_of_entropy: the requested amount of entropy
>> + Output:
>> + entropy_bitstring: The string that provides the requested entropy.
>> + status: A Boolean value that is TRUE if the request has been satisfied,
>> + and is FALSE otherwise.
>> +
>> + Note: In this implementation this function returns a status code instead
>> + of a boolean value.
>> + This is also compatible with the definition of Get_Entropy, see [2]
>> + Section 7.4 Entropy Source Calls.
>> + (status, entropy_bitstring) = Get_Entropy (
>> + requested_entropy,
>> + max_length
>> + )
>> +
>> + @param [in] EntropyBits Number of entropy bits requested.
>> + @param [out] Buffer Buffer to return the entropy bits.
>> + @param [in] BufferSize Size of the Buffer in bytes.
>> +
>> + @retval RETURN_SUCCESS The function completed successfully.
>> + @retval RETURN_INVALID_PARAMETER Invalid parameter.
>> + @retval RETURN_UNSUPPORTED Function not implemented.
>> + @retval RETURN_BAD_BUFFER_SIZE Buffer size is too small.
>> + @retval RETURN_NOT_READY No Entropy available.
>> +**/
>> +RETURN_STATUS
>> +EFIAPI
>> +GetEntropy (
>> + IN CONST UINTN EntropyBits,
>> + OUT UINT8 *Buffer,
>> + IN CONST UINTN BufferSize
>> + )
>> +{
>> + RETURN_STATUS Status;
>> + ARM_MONITOR_ARGS Parameters;
>> + UINTN EntropyBytes;
>> + UINTN LastValidBits;
>> + UINTN ArgSelector;
>> + UINTN BytesToClear;
>> +
>> + // [1] Section 2.4.3 Caller responsibilities.
>> + // The caller cannot request more than MAX_BITS bits of conditioned
>> + // entropy per call.
> Comment is redundant, code is clearer without it.
>
>> + if ((EntropyBits == 0) || (EntropyBits > MAX_ENTROPY_BITS)) {
>> + return RETURN_INVALID_PARAMETER;
>> + }
>> +
>> + EntropyBytes = (EntropyBits + 7) >> 3;
>> + if (EntropyBytes > BufferSize) {
> Not for later: we're verifying the value of EntropyBytes here - if
> there are more aspects of it that need verifying, that should also be
> done here.
>
>> + return RETURN_BAD_BUFFER_SIZE;
>> + }
>> +
>> + ZeroMem (Buffer, BufferSize);
>> + ZeroMem (&Parameters, sizeof (Parameters));
>> +
>> + /*
>> + Cf. [1], Section 2.4 TRNG_RND
>> + Function ID (W0) 0x8400_0053
>> + 0xC400_0053
>> + SMC32 Parameters
>> + W1 N bits of entropy (1 6 N 6 96)
>> + W2-W7 Reserved (MBZ)
>> + SMC64 Parameters
>> + X1 N bits of entropy (1 6 N 6 192)
>> + X2-X7 Reserved (MBZ)
>> + SMC32 Returns
>> + Success (W0 = 0):
>> + W0 MBZ
>> + W1 Entropy[95:64]
>> + W2 Entropy[63:32]
>> + W3 Entropy[31:0]
>> + Error (W0 < 0)
>> + W0 NOT_SUPPORTED
>> + NO_ENTROPY
>> + INVALID_PARAMETERS
>> + W1 - W3 Reserved (MBZ)
>> + SMC64 Returns
>> + Success (X0 = 0):
>> + X0 MBZ
>> + X1 Entropy[191:128]
>> + X2 Entropy[127:64]
>> + X3 Entropy[63:0]
>> + Error (X0 < 0)
>> + X0 NOT_SUPPORTED
>> + NO_ENTROPY
>> + INVALID_PARAMETERS
>> + X1 - X3 Reserved (MBZ)
>> + */
> The above comment block completely wrecks the readability of the
> function.
>
> Would suggest putting it in the header file describing the monitor
> call. For our SIP SVC calls, we've done this in the following form:
>
> /*
> * SMC call to retrieve number of CPUs present in the system.
> * Input values:
> * x0: NUVIA_SIP_GET_NUM_CPUS
> * Return values:
> * x0: SMC_OK
> * x1: Number of CPUs present
> */
> #define NUVIA_SIP_GET_NUM_CPUS SIP_FUNCTION_ID(0x20)
>
> (Where SIP_FUNCTION_ID is one of a set of macros I should submit for
> addition to ArmStdSmc.h)
>
>> + Parameters.Arg0 = FID_TRNG_RND;
>> + Parameters.Arg1 = EntropyBits;
>> + ArmCallMonitor (&Parameters);
>> +
>> + // Convert status codes to EFI status codes.
> Function name already says this, comment redundant.
>
>> + Status = TrngStatusToEfiStatus ((INT32)Parameters.Arg0);
>> + if (EFI_ERROR (Status)) {
>> + return Status;
>> + }
>> +
> From here
>
>> + // Extract Data
>> + // ArgSelector = ((EntropyBytes + 3) >> 2); for AArch32
>> + // ArgSelector = ((EntropyBytes + 7) >> 3); for AArch64
>> + // ((sizeof (UINTN) >> 2) + 1) is 3 or 2 depending on size of UINTN
>> + ArgSelector = ((EntropyBytes + (sizeof (UINTN) - 1)) >>
>> + ((sizeof (UINTN) >> 2) + 1));
>> +
>> + switch (ArgSelector) {
>> + case 3:
>> + CopyMem (&Buffer[(sizeof (UINTN) * 2)], &Parameters.Arg1, sizeof (UINTN));
>> +
>> + case 2:
>> + CopyMem (&Buffer[sizeof (UINTN)], &Parameters.Arg2, sizeof (UINTN));
>> +
>> + case 1:
>> + CopyMem (&Buffer[0], &Parameters.Arg3, sizeof (UINTN));
>> + break;
>> +
>> + default:
>> + ASSERT (0);
>> + return RETURN_INVALID_PARAMETER;
>> + } // switch
> to here ... I'm not convinced you yourself would be able to read or
> explain this code a few months down the line.
>
> Is there a strong reason for why Buffer cannot be a UINTN *?
[SAMI] The specification allows to request minimum 1 bit of entropy
(although I don't think there would be a use case for this). Therefore,
I selected UINT8.
However, I agree the logic is complex. I will simplify this code.
>
> I think what this code is doing can equally be written as:
>
> Buffer[0] = Parameters.Arg3;
> if ((EntropyBytes / sizeof (UINTN)) > 1) {
> Buffer[1] = Parameters.Arg2;
> }
> if ((EntropyBytes / sizeof (UINTN)) > 2) {
> Buffer[2] = Parameters.Arg1;
> }
>
>> +
>> +
>> + // [1] Section 2.4.3 Caller responsibilities.
>> + // The caller must ensure that only the value in Entropy[N-1:0] is consumed
>> + // and that the remaining bits in Entropy[MAX_BITS-1:N] are ignored.
>> + // Therefore, Clear the unused upper bytes.
> This is source code, not the specification.
>
> // Mask off any unused top bytes, in accordance with specification
>
> is sufficient as a comment.
[SAMI] I will fix this and the other comments in the next revision.
>
> /
> Leif
>
>> + BytesToClear = (sizeof (UINTN) * ArgSelector) - EntropyBytes;
>> + if (BytesToClear != 0) {
>> + ZeroMem (&Buffer[EntropyBytes], BytesToClear);
>> + }
>> +
>> + // Clear the unused MSB bits of the last byte.
>> + LastValidBits = EntropyBits & 0x7;
>> + if (LastValidBits != 0) {
>> + Buffer[EntropyBytes - 1] &= (0xFF >> (8 - LastValidBits));
>> + }
>> +
>> + return Status;
>> +}
>> +
>> +/** The constructor checks that the FW-TRNG interface is supported
>> + by the host firmware.
>> +
>> + It will ASSERT() if FW-TRNG is not supported.
>> + It will always return RETURN_SUCCESS.
>> +
>> + @retval RETURN_SUCCESS The constructor always returns RETURN_SUCCESS.
>> +**/
>> +RETURN_STATUS
>> +EFIAPI
>> +ArmFwTrngLibConstructor (
>> + VOID
>> + )
>> +{
>> + RETURN_STATUS Status;
>> + UINT16 MajorRev;
>> + UINT16 MinorRev;
>> + GUID Guid;
>> +
>> + Status = GetTrngVersion (&MajorRev, &MinorRev);
>> + if (EFI_ERROR (Status)) {
>> + return RETURN_SUCCESS;
>> + }
>> +
>> +#ifndef MDEPKG_NDEBUG
>> + // Check that the required features are present.
>> + Status = GetTrngFeatures (FID_TRNG_RND, NULL);
>> + if (EFI_ERROR (Status)) {
>> + return RETURN_SUCCESS;
>> + }
>> +
>> + // Check if TRNG UUID is supported and if so trace the GUID.
>> + Status = GetTrngFeatures (FID_TRNG_GET_UUID, NULL);
>> + if (EFI_ERROR (Status)) {
>> + return RETURN_SUCCESS;
>> + }
>> +#endif
>> +
>> + Status = GetTrngUuid (&Guid);
>> + if (EFI_ERROR (Status)) {
>> + return RETURN_SUCCESS;
>> + }
>> +
>> + DEBUG ((
>> + DEBUG_INFO,
>> + "FW-TRNG: Version %d.%d, GUID {%g}\n",
>> + MajorRev,
>> + MinorRev,
>> + Guid
>> + ));
>> +
>> + return RETURN_SUCCESS;
>> +}
>> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..4b2c58251fbe8fbcb5af308736db014e8d954720
>> --- /dev/null
>> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
>> @@ -0,0 +1,34 @@
>> +## @file
>> +# Arm Firmware TRNG interface library.
>> +#
>> +# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
>> +#
>> +# SPDX-License-Identifier: BSD-2-Clause-Patent
>> +##
>> +
>> +[Defines]
>> + INF_VERSION = 0x0001001B
>> + BASE_NAME = ArmFwTrngLib
>> + FILE_GUID = 10DE97C9-28E4-4C9B-A53E-8D7D1B0DD4E0
>> + VERSION_STRING = 1.0
>> + MODULE_TYPE = BASE
>> + LIBRARY_CLASS = TrngLib
>> + CONSTRUCTOR = ArmFwTrngLibConstructor
>> +
>> +[Sources]
>> + ArmFwTrngDefs.h
>> + ArmFwTrngLib.c
>> +
>> +[Packages]
>> + ArmPkg/ArmPkg.dec
>> + MdePkg/MdePkg.dec
>> +
>> +[LibraryClasses]
>> + ArmSmcLib
>> + ArmHvcLib
>> + BaseLib
>> + BaseMemoryLib
>> +
>> +[Pcd]
>> + gArmTokenSpaceGuid.PcdMonitorConduitHvc
>> +
>> --
>> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>>
>>
>>
>>
>>
>>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
2021-11-25 15:23 ` Sami Mujawar
@ 2022-03-24 9:46 ` PierreGondois
[not found] ` <80941d66-5d31-053f-388a-95efe5dbbfdf@arm.com>
1 sibling, 0 replies; 19+ messages in thread
From: PierreGondois @ 2022-03-24 9:46 UTC (permalink / raw)
To: devel, sami.mujawar, Leif Lindholm
Cc: ardb+tianocore, rebecca, kraxel, michael.d.kinney, gaoliming,
zhiguang.liu, jiewen.yao, jian.j.wang, Matteo.Carlini,
Akanksha.Jain2, Ben.Adderson, nd
On 11/25/21 16:23, Sami Mujawar via groups.io wrote:
> Hi Leif,
>
> Thank you for the feedback.
>
> Please find my response inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
>
> On 24/11/2021 01:01 PM, Leif Lindholm wrote:
>> Hi Sami,
>>
>> On Tue, Nov 16, 2021 at 11:32:55 +0000, Sami Mujawar wrote:
>>> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
>>>
>>> The Arm True Random Number Generator Firmware, Interface 1.0,
>>> Platform Design Document
>>> (https://developer.arm.com/documentation/den0098/latest/)
>>> defines an interface between an Operating System (OS) executing
>>> at EL1 and Firmware (FW) exposing a conditioned entropy source
>>> that is provided by a TRNG back end.
>>>
>>> The conditioned entropy, that is provided by the TRNG FW interface,
>>> is commonly used to seed deterministic random number generators.
>>>
>>> This patch adds a TrngLib library that implements the Arm TRNG
>>> firmware interface.
>>>
>>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>>> ---
>>>
>>> Notes:
>>> v2:
>>> - MdePkg\Include\Library\TrngLib.h is base type [LIMING]
>>> library. It can use RETURN_STATUS instead of
>>> EFI_STATUS.
>>> - Replaced EFI_STATUS with RETURN_STATUS. [SAMI]
>>> - MdePkg\Include\Library\TrngLib.h API parameter [LIMING]
>>> doesn't require CONST. CONST means the value
>>> specified by the input pointer will not be
>>> changed in API implementation.
>>> - Removed the use of constant pointers in the [SAMI]
>>> TRNG API.
>>>
>>> ArmPkg/ArmPkg.dsc | 1 +
>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h | 64 +++
>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c | 483 ++++++++++++++++++++
>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf | 34 ++
>>> 4 files changed, 582 insertions(+)
>>>
[...]
>>> +
>>> +/** Invoke the monitor call using the appropriate conduit.
>>> + If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
>>> +
>>> + @param [in, out] Args Arguments passed to and returned from the monitor.
>>> +
>>> + @return VOID
>>> +**/
>>> +STATIC
>>> +VOID
>>> +ArmCallMonitor (
>>> + IN OUT ARM_MONITOR_ARGS *Args
>>> + )
>>> +{
>>> + if (FeaturePcdGet (PcdMonitorConduitHvc)) {
>>> + ArmCallHvc ((ARM_HVC_ARGS*)Args);
>>> + } else {
>>> + ArmCallSmc ((ARM_SMC_ARGS*)Args);
>>> + }
>>> +}
>> Should this be in (a potentially renamed) ArmSmcLib?
> [SAMI] Looking at ArmSmcLib and ArmHvcLib libraries there is not much
> difference in the code other than the SMC/HVC call. Please let me know
> if I should submit a patch to unify these in ArmMonitorLib?
> The ArmCall<Smc|Hvc> APIs would still remain the same but moved to
> ArmMonitorLib.
Hello Leif,
About your comment, I am not sure I understand it correctly. Assuming the
function allowing to choose the conduit looks like:
VOID
EFIAPI
ArmConduitCall (UINT8 Conduit) {
if (Conduit == 0) {
ArmCallHvc ((ARM_HVC_ARGS*)Args);
} else if (Conduit == 1) {
ArmCallSmc ((ARM_SMC_ARGS*)Args);
} else {
ASSERT (FALSE);
}
}
Do you suggest to:
1. Make ArmSmcLib dependent on ArmHvcLib and add ArmConduitCall()
in ArmSmcLib (or do the opposite with the ArmHvcLib)
2. Merge ArmSmcLib and ArmHvcLib in a new ArmConduitLibrary and add
ArmConduitCall() in this new library.
3. Add an ArmConduitLibrary, relying on ArmSmcLib and ArmHvcLib, and having
only one function: ArmConduitCall()
2. would make the Hvc and Smc calls really tied together.
3. would avoid creating new dependencies on existing libraries (i.e. a
platform only using the ArmSmcLib would not require to have a NULL instance of
ArmHvcLib). I assume you meant 3.
Regards,
Pierre
^ permalink raw reply [flat|nested] 19+ messages in thread[parent not found: <80941d66-5d31-053f-388a-95efe5dbbfdf@arm.com>]
* Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
[not found] ` <80941d66-5d31-053f-388a-95efe5dbbfdf@arm.com>
@ 2022-03-24 14:56 ` PierreGondois
2022-03-24 18:12 ` Leif Lindholm
0 siblings, 1 reply; 19+ messages in thread
From: PierreGondois @ 2022-03-24 14:56 UTC (permalink / raw)
To: devel, sami.mujawar, Leif Lindholm
Cc: ardb+tianocore, rebecca, kraxel, michael.d.kinney, gaoliming,
zhiguang.liu, jiewen.yao, jian.j.wang, Matteo.Carlini,
Akanksha.Jain2, Ben.Adderson, nd
(Replacing Leif's mail address to the @quicinc.com one)
On 3/24/22 10:46, Pierre Gondois wrote:
>
> On 11/25/21 16:23, Sami Mujawar via groups.io wrote:
>> Hi Leif,
>>
>> Thank you for the feedback.
>>
>> Please find my response inline marked [SAMI].
>>
>> Regards,
>>
>> Sami Mujawar
>>
>>
>> On 24/11/2021 01:01 PM, Leif Lindholm wrote:
>>> Hi Sami,
>>>
>>> On Tue, Nov 16, 2021 at 11:32:55 +0000, Sami Mujawar wrote:
>>>> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
>>>>
>>>> The Arm True Random Number Generator Firmware, Interface 1.0,
>>>> Platform Design Document
>>>> (https://developer.arm.com/documentation/den0098/latest/)
>>>> defines an interface between an Operating System (OS) executing
>>>> at EL1 and Firmware (FW) exposing a conditioned entropy source
>>>> that is provided by a TRNG back end.
>>>>
>>>> The conditioned entropy, that is provided by the TRNG FW interface,
>>>> is commonly used to seed deterministic random number generators.
>>>>
>>>> This patch adds a TrngLib library that implements the Arm TRNG
>>>> firmware interface.
>>>>
>>>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>>>> ---
>>>>
>>>> Notes:
>>>> v2:
>>>> - MdePkg\Include\Library\TrngLib.h is base type [LIMING]
>>>> library. It can use RETURN_STATUS instead of
>>>> EFI_STATUS.
>>>> - Replaced EFI_STATUS with RETURN_STATUS. [SAMI]
>>>> - MdePkg\Include\Library\TrngLib.h API parameter [LIMING]
>>>> doesn't require CONST. CONST means the value
>>>> specified by the input pointer will not be
>>>> changed in API implementation.
>>>> - Removed the use of constant pointers in the [SAMI]
>>>> TRNG API.
>>>>
>>>> ArmPkg/ArmPkg.dsc | 1 +
>>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h | 64 +++
>>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c | 483 ++++++++++++++++++++
>>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf | 34 ++
>>>> 4 files changed, 582 insertions(+)
>>>>
>
> [...]
>
>>>> +
>>>> +/** Invoke the monitor call using the appropriate conduit.
>>>> + If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
>>>> +
>>>> + @param [in, out] Args Arguments passed to and returned from the monitor.
>>>> +
>>>> + @return VOID
>>>> +**/
>>>> +STATIC
>>>> +VOID
>>>> +ArmCallMonitor (
>>>> + IN OUT ARM_MONITOR_ARGS *Args
>>>> + )
>>>> +{
>>>> + if (FeaturePcdGet (PcdMonitorConduitHvc)) {
>>>> + ArmCallHvc ((ARM_HVC_ARGS*)Args);
>>>> + } else {
>>>> + ArmCallSmc ((ARM_SMC_ARGS*)Args);
>>>> + }
>>>> +}
>>> Should this be in (a potentially renamed) ArmSmcLib?
>> [SAMI] Looking at ArmSmcLib and ArmHvcLib libraries there is not much
>> difference in the code other than the SMC/HVC call. Please let me know
>> if I should submit a patch to unify these in ArmMonitorLib?
>> The ArmCall<Smc|Hvc> APIs would still remain the same but moved to
>> ArmMonitorLib.
>
> Hello Leif,
> About your comment, I am not sure I understand it correctly. Assuming the
> function allowing to choose the conduit looks like:
>
> VOID
> EFIAPI
> ArmConduitCall (UINT8 Conduit) {
> if (Conduit == 0) {
> ArmCallHvc ((ARM_HVC_ARGS*)Args);
> } else if (Conduit == 1) {
> ArmCallSmc ((ARM_SMC_ARGS*)Args);
> } else {
> ASSERT (FALSE);
> }
> }
>
> Do you suggest to:
> 1. Make ArmSmcLib dependent on ArmHvcLib and add ArmConduitCall()
> in ArmSmcLib (or do the opposite with the ArmHvcLib)
> 2. Merge ArmSmcLib and ArmHvcLib in a new ArmConduitLibrary and add
> ArmConduitCall() in this new library.
> 3. Add an ArmConduitLibrary, relying on ArmSmcLib and ArmHvcLib, and having
> only one function: ArmConduitCall()
>
> 2. would make the Hvc and Smc calls really tied together.
> 3. would avoid creating new dependencies on existing libraries (i.e. a
> platform only using the ArmSmcLib would not require to have a NULL instance of
> ArmHvcLib). I assume you meant 3.
>
> Regards,
> Pierre
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library
2022-03-24 14:56 ` PierreGondois
@ 2022-03-24 18:12 ` Leif Lindholm
0 siblings, 0 replies; 19+ messages in thread
From: Leif Lindholm @ 2022-03-24 18:12 UTC (permalink / raw)
To: Pierre Gondois
Cc: devel, sami.mujawar, ardb+tianocore, rebecca, kraxel,
michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd
On Thu, Mar 24, 2022 at 15:56:32 +0100, Pierre Gondois wrote:
> > > > > +/** Invoke the monitor call using the appropriate conduit.
> > > > > + If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
> > > > > +
> > > > > + @param [in, out] Args Arguments passed to and returned from the monitor.
> > > > > +
> > > > > + @return VOID
> > > > > +**/
> > > > > +STATIC
> > > > > +VOID
> > > > > +ArmCallMonitor (
> > > > > + IN OUT ARM_MONITOR_ARGS *Args
> > > > > + )
> > > > > +{
> > > > > + if (FeaturePcdGet (PcdMonitorConduitHvc)) {
> > > > > + ArmCallHvc ((ARM_HVC_ARGS*)Args);
> > > > > + } else {
> > > > > + ArmCallSmc ((ARM_SMC_ARGS*)Args);
> > > > > + }
> > > > > +}
> > > > Should this be in (a potentially renamed) ArmSmcLib?
> > > [SAMI] Looking at ArmSmcLib and ArmHvcLib libraries there is not much
> > > difference in the code other than the SMC/HVC call. Please let me know
> > > if I should submit a patch to unify these in ArmMonitorLib?
> > > The ArmCall<Smc|Hvc> APIs would still remain the same but moved to
> > > ArmMonitorLib.
> >
> > Hello Leif,
> > About your comment, I am not sure I understand it correctly. Assuming the
> > function allowing to choose the conduit looks like:
> >
> > VOID
> > EFIAPI
> > ArmConduitCall (UINT8 Conduit) {
> > if (Conduit == 0) {
> > ArmCallHvc ((ARM_HVC_ARGS*)Args);
> > } else if (Conduit == 1) {
> > ArmCallSmc ((ARM_SMC_ARGS*)Args);
> > } else {
> > ASSERT (FALSE);
> > }
> > }
> >
> > Do you suggest to:
> > 1. Make ArmSmcLib dependent on ArmHvcLib and add ArmConduitCall()
> > in ArmSmcLib (or do the opposite with the ArmHvcLib)
> > 2. Merge ArmSmcLib and ArmHvcLib in a new ArmConduitLibrary and add
> > ArmConduitCall() in this new library.
> > 3. Add an ArmConduitLibrary, relying on ArmSmcLib and ArmHvcLib, and having
> > only one function: ArmConduitCall()
> >
> >
> > 2. would make the Hvc and Smc calls really tied together.
> > 3. would avoid creating new dependencies on existing libraries (i.e. a
> > platform only using the ArmSmcLib would not require to have a NULL instance of
> > ArmHvcLib). I assume you meant 3.
I think 2 and 3 both capture the spirit of my request.
2 could potentially be achieved with a FixedPcd and conditionals in
asm (less sure about the armasm/vs situation).
I guess the question is if we think it plausible that a platform might
both want to use this new ArmConduitCall *and* directly make use of
ArmHvcLib/ArmSmcLib.
Best Regards,
Leif
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/8] MdePkg: Add NULL instance of TRNG Library
2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
` (2 preceding siblings ...)
2021-11-16 11:32 ` [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 5/8] SecurityPkg: Rename RdRandGenerateEntropy to common name Sami Mujawar
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd
Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
The True Random Number Generator (TRNG) library defines an
interface to access the entropy source on a platform. On
platforms that do not have access to an entropy source, a
NULL instance of the TRNG library may be useful to satisfy
the build dependency.
Therefore, add a NULL instance of the TRNG library.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
v2:
- MdePkg\Include\Library\TrngLib.h is base type [LIMING]
library. It can use RETURN_STATUS instead of
EFI_STATUS.
- Replaced EFI_STATUS with RETURN_STATUS. [SAMI]
- MdePkg\Include\Library\TrngLib.h API parameter [LIMING]
doesn't require CONST. CONST means the value
specified by the input pointer will not be
changed in API implementation.
- Removed the use of constant pointers in the [SAMI]
TRNG API.
MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c | 111 ++++++++++++++++++++
MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf | 30 ++++++
MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni | 12 +++
MdePkg/MdePkg.dsc | 1 +
4 files changed, 154 insertions(+)
diff --git a/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c
new file mode 100644
index 0000000000000000000000000000000000000000..406a1e8587ccfb0cd903bf7a3794f16627eb0a84
--- /dev/null
+++ b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c
@@ -0,0 +1,111 @@
+/** @file
+ Null version of TRNG (True Random Number Generator) services.
+
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Glossary:
+ - TRNG - True Random Number Generator
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/TrngLib.h>
+
+/** Get the TRNG version.
+
+ A TRNG may be implemented by the system firmware, in which case this
+ function shall return the version information for the TRNG implementation.
+ Returning version information is optional and if not implemented,
+ RETURN_UNSUPPORTED shall be returned.
+
+ @param [out] MajorRevision Major revision.
+ @param [out] MinorRevision Minor revision.
+
+ @retval RETURN_SUCCESS The function completed successfully.
+ @retval RETURN_INVALID_PARAMETER Invalid parameter.
+ @retval RETURN_UNSUPPORTED Function not implemented.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngVersion (
+ OUT UINT16 *MajorRevision,
+ OUT UINT16 *MinorRevision
+ )
+{
+ ASSERT (FALSE);
+ return RETURN_UNSUPPORTED;
+}
+
+/** Get the UUID of the TRNG backend.
+
+ A TRNG may be implemented by the system firmware, in which case this
+ function shall return the UUID for the TRNG implementation.
+ Returning the TRNG UUID is optional and if not implemented, RETURN_UNSUPPORTED
+ shall be returned.
+
+ @param [out] Guid UUID of the TRNG backend.
+
+ @retval RETURN_SUCCESS The function completed successfully.
+ @retval RETURN_INVALID_PARAMETER Invalid parameter.
+ @retval RETURN_UNSUPPORTED Function not implemented.
+**/
+RETURN_STATUS
+EFIAPI
+GetTrngUuid (
+ OUT GUID *Guid
+ )
+{
+ ASSERT (FALSE);
+ return RETURN_UNSUPPORTED;
+}
+
+/** Returns maximum number of entropy bits that can be returned in a single
+ call.
+
+ @return Returns the maximum number of Entropy bits that can be returned
+ in a single call to GetEntropy().
+**/
+UINTN
+EFIAPI
+GetTrngMaxSupportedEntropyBits (
+ VOID
+ )
+{
+ ASSERT (FALSE);
+ return 0;
+}
+
+/** Returns N bits of conditioned entropy.
+
+ See [3] Section 2.3.1 GetEntropy: An Interface to the Entropy Source
+ GetEntropy
+ Input:
+ bits_of_entropy: the requested amount of entropy
+ Output:
+ entropy_bitstring: The string that provides the requested entropy.
+ status: A Boolean value that is TRUE if the request has been satisfied,
+ and is FALSE otherwise.
+ Note: In this implementation this function returns a status code instead
+ of a boolean value.
+
+ @param [in] EntropyBits Number of entropy bits requested.
+ @param [out] Buffer Buffer to return the entropy bits.
+ @param [in] BufferSize Size of the Buffer in bytes.
+
+ @retval RETURN_SUCCESS The function completed successfully.
+ @retval RETURN_INVALID_PARAMETER Invalid parameter.
+ @retval RETURN_UNSUPPORTED Function not implemented.
+ @retval RETURN_BAD_BUFFER_SIZE Buffer size is too small.
+ @retval RETURN_NOT_READY No Entropy available.
+**/
+RETURN_STATUS
+EFIAPI
+GetEntropy (
+ IN CONST UINTN EntropyBits,
+ OUT UINT8 *Buffer,
+ IN CONST UINTN BufferSize
+ )
+{
+ ASSERT (FALSE);
+ return RETURN_UNSUPPORTED;
+}
diff --git a/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf
new file mode 100644
index 0000000000000000000000000000000000000000..a700cf66f457f8898d5c51a7b9c0b3d7643ff7f9
--- /dev/null
+++ b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf
@@ -0,0 +1,30 @@
+## @file
+# Null instance of TRNG (True Random Number Generator) Library.
+#
+# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = BaseTrngLibNull
+ MODULE_UNI_FILE = BaseTrngLibNull.uni
+ FILE_GUID = ABDE1C87-4F50-4B82-9133-7A79E13F69AB
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = TrngLib
+
+#
+# VALID_ARCHITECTURES = IA32 X64 ARM AARCH64
+#
+
+[Sources]
+ BaseTrngLibNull.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ DebugLib
diff --git a/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni
new file mode 100644
index 0000000000000000000000000000000000000000..1ec7def522e5975e9621eb280776251b1e5502ca
--- /dev/null
+++ b/MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni
@@ -0,0 +1,12 @@
+// /** @file
+// Null Instance of TRNG (True Random Number Generator) Library.
+//
+// Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+#string STR_MODULE_ABSTRACT #language en-US "Null instance of TRNG Library"
+
+#string STR_MODULE_DESCRIPTION #language en-US "This library instance should be used with modules that inherit an (indirect) dependency on the TrngLib class, but never actually call TrngLib APIs for consuming Entropy."
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index a94959169b2fd9d4b5bf7ad903bf5ce06566c60e..f83753e132e9b9eb4152927fc182701fb1e70ca4 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -67,6 +67,7 @@ [Components]
MdePkg/Library/DxeRngLib/DxeRngLib.inf
MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf
MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
+ MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf
MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 5/8] SecurityPkg: Rename RdRandGenerateEntropy to common name
2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
` (3 preceding siblings ...)
2021-11-16 11:32 ` [PATCH v2 4/8] MdePkg: Add NULL instance of TRNG Library Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 6/8] SecurityPkg: Restructure checks in RngGetInfo Sami Mujawar
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd
Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
Rename RdRandGenerateEntropy() to GenerateEntropy() to provide a
common interface to generate entropy on other architectures.
Also move the definition to RngDxeInternals.h
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
v2:
- No code change since v1. Re-sending with V2 series. [SAMI]
SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c | 13 ++++--
SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h | 43 --------------------
SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c | 6 ++-
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf | 1 -
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h | 19 +++++++++
5 files changed, 33 insertions(+), 49 deletions(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c
index 83025a47d43d442bfe1c324eda2916e6b5599a7e..0ee99a8661fc20094daef019a2f8015597073be4 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c
@@ -1,15 +1,22 @@
/** @file
- Support routines for RDRAND instruction access.
+ Support routines for RDRAND instruction access, which will leverage
+ Intel Secure Key technology to provide high-quality random numbers for use
+ in applications, or entropy for seeding other random number generators.
+ Refer to http://software.intel.com/en-us/articles/intel-digital-random-number
+ -generator-drng-software-implementation-guide/ for more information about Intel
+ Secure Key technology.
Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
#include <Library/RngLib.h>
+#include <Library/TimerLib.h>
#include "AesCore.h"
-#include "RdRand.h"
#include "RngDxeInternals.h"
/**
@@ -87,7 +94,7 @@ RdRandGetSeed128 (
**/
EFI_STATUS
EFIAPI
-RdRandGenerateEntropy (
+GenerateEntropy (
IN UINTN Length,
OUT UINT8 *Entropy
)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h
deleted file mode 100644
index 072378e062e7bee81a7e763fe9b4ed4517e4d82c..0000000000000000000000000000000000000000
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h
+++ /dev/null
@@ -1,43 +0,0 @@
-/** @file
- Header for the RDRAND APIs used by RNG DXE driver.
-
- Support API definitions for RDRAND instruction access, which will leverage
- Intel Secure Key technology to provide high-quality random numbers for use
- in applications, or entropy for seeding other random number generators.
- Refer to http://software.intel.com/en-us/articles/intel-digital-random-number
- -generator-drng-software-implementation-guide/ for more information about Intel
- Secure Key technology.
-
-Copyright (c) 2013, Intel Corporation. All rights reserved.<BR>
-(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#ifndef __RD_RAND_H__
-#define __RD_RAND_H__
-
-#include <Library/BaseLib.h>
-#include <Library/BaseMemoryLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Library/TimerLib.h>
-#include <Protocol/Rng.h>
-
-/**
- Generate high-quality entropy source through RDRAND.
-
- @param[in] Length Size of the buffer, in bytes, to fill with.
- @param[out] Entropy Pointer to the buffer to store the entropy data.
-
- @retval EFI_SUCCESS Entropy generation succeeded.
- @retval EFI_NOT_READY Failed to request random data.
-
-**/
-EFI_STATUS
-EFIAPI
-RdRandGenerateEntropy (
- IN UINTN Length,
- OUT UINT8 *Entropy
- );
-
-#endif // __RD_RAND_H__
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
index d0e6b7de06352b6a92a823681eab92b7a4ca720f..2009f95b4cadb07fc9073c3c0660cf549965422a 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
@@ -20,7 +20,9 @@
**/
-#include "RdRand.h"
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+
#include "RngDxeInternals.h"
/**
@@ -88,7 +90,7 @@ RngGetRNG (
return EFI_INVALID_PARAMETER;
}
- Status = RdRandGenerateEntropy (RNGValueLength, RNGValue);
+ Status = GenerateEntropy (RNGValueLength, RNGValue);
return Status;
}
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
index f3300971993f7c6fcdca441858de4c2fb35912e8..ef5cd73273e68c67bec7411279bb8433c45ab2d4 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
@@ -36,7 +36,6 @@ [Sources.common]
[Sources.IA32, Sources.X64]
Rand/RngDxe.c
Rand/RdRand.c
- Rand/RdRand.h
Rand/AesCore.c
Rand/AesCore.h
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
index 2660ed5875e0d52a6b9d806341431859374b1047..34886adcf549efdedc1a7b8f16b81a5148531de2 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
@@ -10,6 +10,8 @@
#ifndef RNGDXE_INTERNALS_H_
#define RNGDXE_INTERNALS_H_
+#include <Protocol/Rng.h>
+
/**
Returns information about the random number generation implementation.
@@ -114,4 +116,21 @@ RngGetBytes (
OUT UINT8 *RandBuffer
);
+/**
+ Generate high-quality entropy source using a TRNG or through RDRAND.
+
+ @param[in] Length Size of the buffer, in bytes, to fill with.
+ @param[out] Entropy Pointer to the buffer to store the entropy data.
+
+ @retval EFI_SUCCESS Entropy generation succeeded.
+ @retval EFI_NOT_READY Failed to request random data.
+
+**/
+EFI_STATUS
+EFIAPI
+GenerateEntropy (
+ IN UINTN Length,
+ OUT UINT8 *Entropy
+ );
+
#endif // RNGDXE_INTERNALS_H_
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 6/8] SecurityPkg: Restructure checks in RngGetInfo
2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
` (4 preceding siblings ...)
2021-11-16 11:32 ` [PATCH v2 5/8] SecurityPkg: Rename RdRandGenerateEntropy to common name Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
2021-11-16 11:32 ` [PATCH v2 7/8] SecurityPkg: Add RawAlgorithm support using TRNG library Sami Mujawar
2021-11-16 11:33 ` [PATCH v2 8/8] ArmVirtPkg: Kvmtool: Add RNG support using FW-TRNG interface Sami Mujawar
7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd
Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
Move the check to see if the RNGAlgorithmList pointer is NULL to
ArchGetSupportedRngAlgorithms(). This allows the caller to obtain
the buffer size required to store the Algorithm List by passing
RNGAlgorithmListSize as zero and RNGAlgorithmList as NULL.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
v2:
- No code change since v1. Re-sending with V2 series. [SAMI]
SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c | 6 ++++++
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c | 11 ++---------
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h | 2 +-
3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
index 2009f95b4cadb07fc9073c3c0660cf549965422a..f1122a48102595506cc423c3ab501d9a72f50543 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
@@ -16,6 +16,7 @@
Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -116,6 +117,7 @@ RngGetRNG (
@retval EFI_SUCCESS The RNG algorithm list was returned successfully.
@retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too small to hold the result.
+ @retval EFI_INVALID_PARAMETER The pointer to the buffer RNGAlgorithmList is invalid.
**/
UINTN
@@ -135,6 +137,10 @@ ArchGetSupportedRngAlgorithms (
return EFI_BUFFER_TOO_SMALL;
}
+ if (RNGAlgorithmList == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
CpuRngSupportedAlgorithm = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
CopyMem(&RNGAlgorithmList[0], CpuRngSupportedAlgorithm, sizeof (EFI_RNG_ALGORITHM));
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
index b959c70536ea3b9049905bbfd3d973fc9b2f6dcf..2e3b714bc691e4e517866369c034b721fbccfa24 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
@@ -14,6 +14,7 @@
Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -59,8 +60,6 @@ RngGetInfo (
OUT EFI_RNG_ALGORITHM *RNGAlgorithmList
)
{
- EFI_STATUS Status;
-
if ((This == NULL) || (RNGAlgorithmListSize == NULL)) {
return EFI_INVALID_PARAMETER;
}
@@ -68,13 +67,7 @@ RngGetInfo (
//
// Return algorithm list supported by driver.
//
- if (RNGAlgorithmList != NULL) {
- Status = ArchGetSupportedRngAlgorithms (RNGAlgorithmListSize, RNGAlgorithmList);
- } else {
- Status = EFI_INVALID_PARAMETER;
- }
-
- return Status;
+ return ArchGetSupportedRngAlgorithms (RNGAlgorithmListSize, RNGAlgorithmList);
}
//
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
index 34886adcf549efdedc1a7b8f16b81a5148531de2..37c27c4094e5302dfe2e7d9bbeef33a24b0c73ea 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
@@ -90,7 +90,7 @@ RngGetRNG (
@retval EFI_SUCCESS The RNG algorithm list was returned successfully.
@retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too small to hold the result.
-
+ @retval EFI_INVALID_PARAMETER The pointer to the buffer RNGAlgorithmList is invalid.
**/
UINTN
EFIAPI
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 7/8] SecurityPkg: Add RawAlgorithm support using TRNG library
2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
` (5 preceding siblings ...)
2021-11-16 11:32 ` [PATCH v2 6/8] SecurityPkg: Restructure checks in RngGetInfo Sami Mujawar
@ 2021-11-16 11:32 ` Sami Mujawar
2021-11-16 11:33 ` [PATCH v2 8/8] ArmVirtPkg: Kvmtool: Add RNG support using FW-TRNG interface Sami Mujawar
7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:32 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd
Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
RawAlgorithm is used to provide access to entropy that is suitable
for cryptographic applications. Therefore, add RawAlgorithm support
that provides access to entropy using the TRNG library interface.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
v2:
- MdeModulePkg\Include\Guid\ZeroGuid.h has defined [LIMING]
gZeroGuid. You don't define it again.
- Replaced use of gNullGuid with gZeroGuid. [SAMI]
SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c | 79 ++++++++--
SecurityPkg/RandomNumberGenerator/RngDxe/Arm/RngDxe.c | 163 ++++++++++++++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c | 61 ++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c | 2 +-
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf | 13 +-
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h | 1 +
SecurityPkg/SecurityPkg.dsc | 8 +-
7 files changed, 314 insertions(+), 13 deletions(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c
index 282fdca9d334b77e02ca47734df08729e0f4fd31..d1c8f4c69b4d65c10141da320d44cd8f01bb0c74 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c
@@ -1,11 +1,12 @@
/** @file
RNG Driver to produce the UEFI Random Number Generator protocol.
- The driver will use the RNDR instruction to produce random numbers.
+ The driver will use the RNDR instruction to produce random numbers. It also
+ uses the Arm FW-TRNG interface to implement EFI_RNG_ALGORITHM_RAW.
RNG Algorithms defined in UEFI 2.4:
- EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID
- - EFI_RNG_ALGORITHM_RAW - Unsupported
+ - EFI_RNG_ALGORITHM_RAW
- EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID
- EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID
- EFI_RNG_ALGORITHM_X9_31_3DES_GUID - Unsupported
@@ -14,15 +15,17 @@
Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
+#include <Guid/ZeroGuid.h>
#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Library/TimerLib.h>
+#include <Library/DebugLib.h>
+#include <Library/TrngLib.h>
#include <Protocol/Rng.h>
#include "RngDxeInternals.h"
@@ -58,7 +61,9 @@ RngGetRNG (
OUT UINT8 *RNGValue
)
{
- EFI_STATUS Status;
+ EFI_STATUS Status;
+ UINT16 MajorRevision;
+ UINT16 MinorRevision;
if ((RNGValueLength == 0) || (RNGValue == NULL)) {
return EFI_INVALID_PARAMETER;
@@ -76,6 +81,17 @@ RngGetRNG (
return Status;
}
+ //
+ // The "raw" algorithm is intended to provide entropy directly
+ //
+ if (CompareGuid (RNGAlgorithm, &gEfiRngAlgorithmRaw)) {
+ Status = GetTrngVersion (&MajorRevision, &MinorRevision);
+ if (EFI_ERROR (Status)) {
+ return EFI_UNSUPPORTED;
+ }
+ return GenerateEntropy (RNGValueLength, RNGValue);
+ }
+
//
// Other algorithms are unsupported by this driver.
//
@@ -97,8 +113,9 @@ RngGetRNG (
is the default algorithm for the driver.
@retval EFI_SUCCESS The RNG algorithm list was returned successfully.
+ @retval EFI_UNSUPPORTED No supported algorithms found.
@retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too small to hold the result.
-
+ @retval EFI_INVALID_PARAMETER The pointer to the buffer RNGAlgorithmList is invalid.
**/
UINTN
EFIAPI
@@ -107,19 +124,61 @@ ArchGetSupportedRngAlgorithms (
OUT EFI_RNG_ALGORITHM *RNGAlgorithmList
)
{
- UINTN RequiredSize;
+ EFI_STATUS Status;
+ UINT16 MajorRevision;
+ UINT16 MinorRevision;
+ UINTN RequiredSize;
+ BOOLEAN CpuRngAlgorithmSupported;
+ BOOLEAN RawAlgorithmSupported;
+ UINTN Index;
EFI_RNG_ALGORITHM *CpuRngSupportedAlgorithm;
- RequiredSize = sizeof (EFI_RNG_ALGORITHM);
+ RequiredSize = 0;
+ CpuRngAlgorithmSupported = FALSE;
+ RawAlgorithmSupported = FALSE;
+
+ CpuRngSupportedAlgorithm = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
+ if (!CompareGuid (CpuRngSupportedAlgorithm, &gZeroGuid)) {
+ CpuRngAlgorithmSupported = TRUE;
+ RequiredSize += sizeof (EFI_RNG_ALGORITHM);
+ }
+
+ Status = GetTrngVersion (&MajorRevision, &MinorRevision);
+ if (!EFI_ERROR (Status)) {
+ RawAlgorithmSupported = TRUE;
+ RequiredSize += sizeof (EFI_RNG_ALGORITHM);
+ }
if (*RNGAlgorithmListSize < RequiredSize) {
*RNGAlgorithmListSize = RequiredSize;
return EFI_BUFFER_TOO_SMALL;
}
- CpuRngSupportedAlgorithm = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
+ if (RequiredSize == 0) {
+ // No supported algorithms found.
+ return EFI_UNSUPPORTED;
+ }
- CopyMem(&RNGAlgorithmList[0], CpuRngSupportedAlgorithm, sizeof (EFI_RNG_ALGORITHM));
+ if (RNGAlgorithmList == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Index = 0;
+ if (CpuRngAlgorithmSupported) {
+ CopyMem (
+ &RNGAlgorithmList[Index++],
+ CpuRngSupportedAlgorithm,
+ sizeof (EFI_RNG_ALGORITHM)
+ );
+ }
+
+ if (RawAlgorithmSupported) {
+ CopyMem (
+ &RNGAlgorithmList[Index++],
+ &gEfiRngAlgorithmRaw,
+ sizeof (EFI_RNG_ALGORITHM)
+ );
+ }
*RNGAlgorithmListSize = RequiredSize;
return EFI_SUCCESS;
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/RngDxe.c
new file mode 100644
index 0000000000000000000000000000000000000000..cba9883e50cefbb22495190d17de99bfeab33cf3
--- /dev/null
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/RngDxe.c
@@ -0,0 +1,163 @@
+/** @file
+ RNG Driver to produce the UEFI Random Number Generator protocol.
+
+ The driver implements the EFI_RNG_ALGORITHM_RAW using the FW-TRNG
+ interface to provide entropy.
+
+ RNG Algorithms defined in UEFI 2.4:
+ - EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID
+ - EFI_RNG_ALGORITHM_RAW
+ - EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID
+ - EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID
+ - EFI_RNG_ALGORITHM_X9_31_3DES_GUID - Unsupported
+ - EFI_RNG_ALGORITHM_X9_31_AES_GUID - Unsupported
+
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/TrngLib.h>
+#include <Protocol/Rng.h>
+
+#include "RngDxeInternals.h"
+
+/**
+ Produces and returns an RNG value using either the default or specified
+ RNG algorithm.
+
+ @param[in] This A pointer to the EFI_RNG_PROTOCOL instance.
+ @param[in] RNGAlgorithm A pointer to the EFI_RNG_ALGORITHM that
+ identifies the RNG algorithm to use. May be
+ NULL in which case the function will use its
+ default RNG algorithm.
+ @param[in] RNGValueLength The length in bytes of the memory buffer
+ pointed to by RNGValue. The driver shall
+ return exactly this numbers of bytes.
+ @param[out] RNGValue A caller-allocated memory buffer filled by
+ the driver with the resulting RNG value.
+
+ @retval EFI_SUCCESS The RNG value was returned successfully.
+ @retval EFI_UNSUPPORTED The algorithm specified by RNGAlgorithm is
+ not supported by this driver.
+ @retval EFI_DEVICE_ERROR An RNG value could not be retrieved due to
+ a hardware or firmware error.
+ @retval EFI_NOT_READY There is not enough random data available
+ to satisfy the length requested by
+ RNGValueLength.
+ @retval EFI_INVALID_PARAMETER RNGValue is NULL or RNGValueLength is zero.
+
+**/
+EFI_STATUS
+EFIAPI
+RngGetRNG (
+ IN EFI_RNG_PROTOCOL *This,
+ IN EFI_RNG_ALGORITHM *RNGAlgorithm, OPTIONAL
+ IN UINTN RNGValueLength,
+ OUT UINT8 *RNGValue
+ )
+{
+ EFI_STATUS Status;
+ UINT16 MajorRevision;
+ UINT16 MinorRevision;
+
+ if ((RNGValueLength == 0) || (RNGValue == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (RNGAlgorithm == NULL) {
+ //
+ // Use the default RNG algorithm if RNGAlgorithm is NULL.
+ //
+ RNGAlgorithm = &gEfiRngAlgorithmRaw;
+ }
+
+ //
+ // The "raw" algorithm is intended to provide entropy directly
+ //
+ if (CompareGuid (RNGAlgorithm, &gEfiRngAlgorithmRaw)) {
+ Status = GetTrngVersion (&MajorRevision, &MinorRevision);
+ if (EFI_ERROR (Status)) {
+ return EFI_UNSUPPORTED;
+ }
+ return GenerateEntropy (RNGValueLength, RNGValue);
+ }
+
+ //
+ // Other algorithms are unsupported by this driver.
+ //
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ Returns information about the random number generation implementation.
+
+ @param[in,out] RNGAlgorithmListSize On input, the size in bytes of
+ RNGAlgorithmList.
+ On output with a return code of
+ EFI_SUCCESS, the size in bytes of the
+ data returned in RNGAlgorithmList.
+ On output with a return code of
+ EFI_BUFFER_TOO_SMALL, the size of
+ RNGAlgorithmList required to obtain the
+ list.
+ @param[out] RNGAlgorithmList A caller-allocated memory buffer filled
+ by the driver with one EFI_RNG_ALGORITHM
+ element for each supported RNG algorithm.
+ The list must not change across multiple
+ calls to the same driver. The first
+ algorithm in the list is the default
+ algorithm for the driver.
+
+ @retval EFI_SUCCESS The RNG algorithm list was returned
+ successfully.
+ @retval EFI_UNSUPPORTED No supported algorithms found.
+ @retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too small
+ to hold the result.
+ @retval EFI_INVALID_PARAMETER The pointer to the buffer RNGAlgorithmList
+ is invalid.
+**/
+UINTN
+EFIAPI
+ArchGetSupportedRngAlgorithms (
+ IN OUT UINTN *RNGAlgorithmListSize,
+ OUT EFI_RNG_ALGORITHM *RNGAlgorithmList
+ )
+{
+ EFI_STATUS Status;
+ UINTN RequiredSize;
+ UINT16 MajorRevision;
+ UINT16 MinorRevision;
+
+ RequiredSize = 0;
+
+ Status = GetTrngVersion (&MajorRevision, &MinorRevision);
+ if (EFI_ERROR (Status)) {
+ // No supported algorithms found.
+ return EFI_UNSUPPORTED;
+ }
+
+ RequiredSize += sizeof (EFI_RNG_ALGORITHM);
+
+ if (*RNGAlgorithmListSize < RequiredSize) {
+ *RNGAlgorithmListSize = RequiredSize;
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ if (RNGAlgorithmList == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ CopyMem (
+ &RNGAlgorithmList[0],
+ &gEfiRngAlgorithmRaw,
+ sizeof (EFI_RNG_ALGORITHM)
+ );
+
+ *RNGAlgorithmListSize = RequiredSize;
+ return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c
new file mode 100644
index 0000000000000000000000000000000000000000..8df37d82e2051854f74816711a14ee23472f6b41
--- /dev/null
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c
@@ -0,0 +1,61 @@
+/** @file
+ Arm FW-TRNG interface helper common for AArch32 and AArch64.
+
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/TrngLib.h>
+
+/**
+ Generate high-quality entropy source using a TRNG.
+
+ @param[in] Length Size of the buffer, in bytes, to fill with.
+ @param[out] Entropy Pointer to the buffer to store the entropy data.
+
+ @retval EFI_SUCCESS Entropy generation succeeded.
+ @retval EFI_NOT_READY Failed to request random data.
+
+**/
+EFI_STATUS
+EFIAPI
+GenerateEntropy (
+ IN UINTN Length,
+ OUT UINT8 *Entropy
+ )
+{
+ EFI_STATUS Status;
+ UINTN CollectedEntropyBits;
+ UINTN RequiredEntropyBits;
+ UINTN EntropyBits;
+ UINTN Index;
+ UINTN MaxBits;
+
+ ZeroMem (Entropy, Length);
+
+ RequiredEntropyBits = (Length << 3);
+ Index = 0;
+ CollectedEntropyBits = 0;
+ MaxBits = GetTrngMaxSupportedEntropyBits ();
+ while (CollectedEntropyBits < RequiredEntropyBits) {
+ EntropyBits = MIN ((RequiredEntropyBits - CollectedEntropyBits), MaxBits);
+ Status = GetEntropy (
+ EntropyBits,
+ &Entropy[Index],
+ (Length - Index)
+ );
+ if (EFI_ERROR (Status)) {
+ // Discard the collected bits.
+ ZeroMem (Entropy, Length);
+ return Status;
+ }
+ CollectedEntropyBits += EntropyBits;
+ Index += (EntropyBits >> 3);
+ } // while
+
+ return Status;
+}
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
index 2e3b714bc691e4e517866369c034b721fbccfa24..b7ac0baf3f8216c9a86029b3037bfe4fd59269f6 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
@@ -45,7 +45,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
is the default algorithm for the driver.
@retval EFI_SUCCESS The RNG algorithm list was returned successfully.
- @retval EFI_UNSUPPORTED The services is not supported by this driver.
+ @retval EFI_UNSUPPORTED No supported algorithms found.
@retval EFI_DEVICE_ERROR The list of algorithms could not be retrieved due to a
hardware or firmware error.
@retval EFI_INVALID_PARAMETER One or more of the parameters are incorrect.
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
index ef5cd73273e68c67bec7411279bb8433c45ab2d4..9f2e92512bfa48bd772c7f887a23453756421b80 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
@@ -10,6 +10,7 @@
#
# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
# (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -26,7 +27,7 @@ [Defines]
#
# The following information is for reference only and not required by the build tools.
#
-# VALID_ARCHITECTURES = IA32 X64 AARCH64
+# VALID_ARCHITECTURES = IA32 X64 AARCH64 ARM
#
[Sources.common]
@@ -41,8 +42,14 @@ [Sources.IA32, Sources.X64]
[Sources.AARCH64]
AArch64/RngDxe.c
+ ArmTrng.c
+
+[Sources.ARM]
+ Arm/RngDxe.c
+ ArmTrng.c
[Packages]
+ MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
SecurityPkg/SecurityPkg.dec
@@ -55,6 +62,9 @@ [LibraryClasses]
TimerLib
RngLib
+[LibraryClasses.AARCH64, LibraryClasses.ARM]
+ TrngLib
+
[Guids]
gEfiRngAlgorithmSp80090Hash256Guid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
gEfiRngAlgorithmSp80090Hmac256Guid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
@@ -62,6 +72,7 @@ [Guids]
gEfiRngAlgorithmX9313DesGuid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
gEfiRngAlgorithmX931AesGuid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
gEfiRngAlgorithmRaw ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
+ gZeroGuid ## CONSUMES
[Protocols]
gEfiRngProtocolGuid ## PRODUCES
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
index 37c27c4094e5302dfe2e7d9bbeef33a24b0c73ea..8978d54f51d4e72ad881ee584e16dcdda72a66ae 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
@@ -89,6 +89,7 @@ RngGetRNG (
is the default algorithm for the driver.
@retval EFI_SUCCESS The RNG algorithm list was returned successfully.
+ @retval EFI_UNSUPPORTED No supported algorithms found.
@retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too small to hold the result.
@retval EFI_INVALID_PARAMETER The pointer to the buffer RNGAlgorithmList is invalid.
**/
diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
index 73a93c2285b13a2e0ce45b08a1230a766e0d759a..63da3d8c92e5a2c559b7731dd6dc0654caab30b8 100644
--- a/SecurityPkg/SecurityPkg.dsc
+++ b/SecurityPkg/SecurityPkg.dsc
@@ -3,6 +3,7 @@
#
# Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>
# (C) Copyright 2015-2020 Hewlett Packard Enterprise Development LP<BR>
+# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -86,6 +87,11 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
+ # Arm FW-TRNG interface library.
+ TrngLib|ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
+ ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+ ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
+
[LibraryClasses.ARM]
RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
@@ -277,7 +283,7 @@ [Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.inf
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.inf
-[Components.IA32, Components.X64, Components.AARCH64]
+[Components.IA32, Components.X64, Components.AARCH64, Components.ARM]
#
# Random Number Generator
#
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 8/8] ArmVirtPkg: Kvmtool: Add RNG support using FW-TRNG interface
2021-11-16 11:32 [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface Sami Mujawar
` (6 preceding siblings ...)
2021-11-16 11:32 ` [PATCH v2 7/8] SecurityPkg: Add RawAlgorithm support using TRNG library Sami Mujawar
@ 2021-11-16 11:33 ` Sami Mujawar
7 siblings, 0 replies; 19+ messages in thread
From: Sami Mujawar @ 2021-11-16 11:33 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ardb+tianocore, leif, rebecca, kraxel,
michael.d.kinney, gaoliming, zhiguang.liu, jiewen.yao,
jian.j.wang, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson, nd
Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
The EFI_RNG_PROTOCOL published by RngDxe has been updated to
implement the EFI_RNG_ALGORITHM_RAW using the Arm FW-TRNG
interface to provide access to entropy.
Therefore, enable EFI_RNG_PROTOCOL for the Kvmtool guest/virtual
firmware.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
v2:
- No code change since v1. Re-sending with V2 series. [SAMI]
ArmVirtPkg/ArmVirtKvmTool.dsc | 10 ++++++++++
ArmVirtPkg/ArmVirtKvmTool.fdf | 5 +++++
2 files changed, 15 insertions(+)
diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 9d23072d8fa893907848ef105b2c96953a68c56e..418ae894681d0390907ff25538b58bf0162018b0 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -71,6 +71,8 @@ [LibraryClasses.common]
PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+ TrngLib|ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
+
[LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
@@ -102,6 +104,8 @@ [PcdsFeatureFlag.common]
# Use MMIO for accessing RTC controller registers.
gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|TRUE
+ gArmTokenSpaceGuid.PcdMonitorConduitHvc|TRUE
+
[PcdsFixedAtBuild.common]
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
@@ -353,3 +357,9 @@ [Components.common]
}
OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
OvmfPkg/Virtio10Dxe/Virtio10.inf
+
+ #
+ # Rng Support
+ #
+ SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+
diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
index 14a5fce43a0928d6d78b1af5d7bc3a16b6f07918..ed7e5cfcfad8ef762921de52af66f76351736468 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.fdf
+++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
@@ -211,6 +211,11 @@ [FV.FvMain]
#
INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
+ #
+ # Rng Support
+ #
+ INF SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+
[FV.FVMAIN_COMPACT]
FvAlignment = 16
ERASE_POLARITY = 1
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 19+ messages in thread