From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.5308.1658403435104923937 for ; Thu, 21 Jul 2022 04:37:15 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 248311042; Thu, 21 Jul 2022 04:37:15 -0700 (PDT) Received: from [10.34.100.102] (pierre123.nice.arm.com [10.34.100.102]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B8C313F70D; Thu, 21 Jul 2022 04:37:12 -0700 (PDT) Message-ID: Date: Thu, 21 Jul 2022 13:36:45 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [edk2-devel] [PATCH v3 00/22] Add Raw algorithm support using Arm FW-TRNG interface To: "Yao, Jiewen" , "devel@edk2.groups.io" Cc: Sami Mujawar , Leif Lindholm , Ard Biesheuvel , Rebecca Cran , "Kinney, Michael D" , "Gao, Liming" , "Wang, Jian J" References: <16FD1FA5CFF9CFC7.2437@groups.io> <0b1b12a0-70d1-d7e7-004b-5e5dcbe60b97@arm.com> <1b34c7fd-df17-6471-8e5d-f0c4af67640a@arm.com> <1703D1A6CBF30B16.27668@groups.io> From: "PierreGondois" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable On 7/21/22 12:42, Yao, Jiewen wrote: > Hi Pierre > Sorry, I should have given the specific comment. >=20 > [PATCH v3 12/22] SecurityPkg: Update Securitypkg.ci.yaml >=20 > Why we need ArmPkg as new dependency? I don=E2=80=99t think this is good = idea. > Can we have a way to remove that dependency? >=20 > Thank you > Yao, Jiewen >=20 >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Yao, Jiew= en >> Sent: Thursday, July 21, 2022 6:33 PM >> To: Pierre Gondois ; devel@edk2.groups.io >> Cc: Sami Mujawar ; Leif Lindholm >> ; Ard Biesheuvel ; >> Rebecca Cran ; Kinney, Michael D >> ; Gao, Liming ; >> Wang, Jian J >> Subject: Re: [edk2-devel] [PATCH v3 00/22] Add Raw algorithm support usi= ng >> Arm FW-TRNG interface >> >> Hi Pierre >> I give the feedback on interface design and dependency, esp SecurityPkg = and >> MdePkg. >> 1) Please don=E2=80=99t change package dependency policy. Yes right, with the changes suggested below: - in SecurityPkg.dsc, have TrngLib|MdePkg/Library/BaseTrngLibNull/BaseTrn= gLibNull.inf - change ArmHasRngExt() will allow to drop the following patch: [PATCH v3 12/22] SecurityPkg: Update Securitypkg.ci.yaml and maybe some others, so no more dependency over the ArmPkg (from the Secu= rityPkg). >> 2) For AesLib/DrbgLib/TrbgLib, I already expressed my concern on adding = those >> new APIs in MdePkg. Please try to avoid that. >> >> Basically, since this is ARM FW-TRNG feature, you can consider just add = this to >> ArmPkg without impacting other package. That would be a clean isolation. I noted your concerns. The TrngLib, AesLib and DrbgLib were split as 3 patc= h-sets. The TrngLib should come first in the 3 sets. I still need to work on the Ae= sLib and DrbgLib. I will send a new version of the TrngLib which will address your concerns a= nd have a clean isolation. Thanks for the comments, Pierre >> >> Thank you >> Yao, Jiewen >> >> >>> -----Original Message----- >>> From: Pierre Gondois >>> Sent: Thursday, July 21, 2022 4:55 PM >>> To: Yao, Jiewen ; devel@edk2.groups.io >>> Cc: Sami Mujawar ; Leif Lindholm >>> ; Ard Biesheuvel = ; >>> Rebecca Cran ; Kinney, Michael D >>> ; Gao, Liming ; >>> Wang, Jian J >>> Subject: Re: [edk2-devel] [PATCH v3 00/22] Add Raw algorithm support us= ing >>> Arm FW-TRNG interface >>> >>> Hi Jiewen, >>> >>> On 7/20/22 18:44, Yao, Jiewen wrote: >>>> Hey >>>> This patch add dependency that SecurityPkg will depend on ArmPkg. >>>> >>>> I am not sure it is good idea. As alternative, why not put all those t= o ArmPkg? >>>> Then you don=E2=80=99t need change the package dependency. >>> >>> I am not sure I understood what you want to move to the ArmPkg. >>> Nonetheless, doing the following removes the dependency over the ArmPkg >>> (and it seems to be the normal way way of removing dependencies): >>> + TrngLib|MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf >>> - TrngLib|ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf >>> - ArmMonitorLib|ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.inf >>> - ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf >>> - ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf >>> >>> The only other dependecy over the ArmPkg is the ArmHasRngExt() call in >>> [PATCH v3 20/21] SecurityPkg/RngDxe: Add Arm support of RngDxe >>> but this can be easily replaced. >>> >>> Would it be ok ? >>> >>> Regards, >>> Pierre >>> >>> >>>> >>>> Thank you >>>> Yao Jiewen >>>> >>>>> -----Original Message----- >>>>> From: devel@edk2.groups.io On Behalf Of >>>>> PierreGondois >>>>> Sent: Wednesday, July 20, 2022 10:59 PM >>>>> To: devel@edk2.groups.io >>>>> Cc: Sami Mujawar ; Leif Lindholm >>>>> ; Ard Biesheuvel >> ; >>>>> Rebecca Cran ; Kinney, Michael D >>>>> ; Gao, Liming ; >>> Yao, >>>>> Jiewen ; Wang, Jian J >>>>> Subject: Re: [edk2-devel] [PATCH v3 00/22] Add Raw algorithm support >> using >>>>> Arm FW-TRNG interface >>>>> >>>>> Hi, >>>>> I know this patch-set might be long to review, but I wanted to ask >>>>> if somebody already had some comments, >>>>> >>>>> Note the following patch-sets are depending on this patch-set: >>>>> - [PATCH v1 0/7] Add AesLib and ArmAesLib >>>>> https://edk2.groups.io/g/devel/message/90878 >>>>> - [PATCH RESEND v1 0/9] Add DrbgLib >>>>> https://edk2.groups.io/g/devel/message/90898 >>>>> >>>>> Regards, >>>>> Pierre >>>>> >>>>> On 6/29/22 17:02, PierreGondois via groups.io wrote: >>>>>> From: Pierre Gondois >>>>>> >>>>>> Bugzilla: Bug 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id= =3D3668) >>>>>> >>>>>> The Arm True Random Number Generator Firmware, Interface 1.0, >>>>> specification >>>>>> defines an interface between an Operating System (OS) executing at E= L1 >> and >>>>>> Firmware (FW) exposing a conditioned entropy source that is provided= by >> a >>>>>> TRNG back end. >>>>>> This patch-set: >>>>>> - defines a TRNG library class that provides an interface to access = the >>>>>> entropy source on a platform. >>>>>> - implements a TRNG library instance that uses the Arm FW-TRNG >> interface. >>>>>> - Adds RawAlgorithm support to RngDxe for Arm architecture using the >> Arm >>>>>> FW-TRNG interface. >>>>>> - Enables RNG support using FW-TRNG interface for Kvmtool Guest/Virt= ual >>>>>> firmware. >>>>>> >>>>>> This patch-set is based on the v2 from Sami Mujawar: >>>>>> [PATCH v2 0/8] Add Raw algorithm support using Arm FW-TRNG interface >>>>>> https://edk2.groups.io/g/devel/message/83775 >>>>>> >>>>>> This patch-set can seen at: >>>>>> https://github.com/PierreARM/edk2/tree/Arm_Trng_v3 >>>>>> >>>>>> V3: >>>>>> - Address Leif's comment (moving definitions, optimizations, ...= ) >>>>>> - Add ArmMonitorLib to choose Hvc/Smc conduit depending on a Pcd= . >>>>>> - Re-factor some parts of SecurityPkg/RngDxe/ to ease the additi= on >>>>>> of new algorithms. >>>>>> - Add ArmHasRngExt() function to check Arm's FEAT_RNG extension. >>>>>> V2: >>>>>> - Updates TrngLib definitions to use RETURN_STATUS as the return= type >>>>>> from the interface functions as TrngLib is base type library. >>>>>> - Drops the patch "MdePkg: Add definition for NULL GUID" as ther= e is >>>>>> already an equivalent definition provided by gZeroGuid. Thus, = the >>>>>> use of gNullGuid has been replaced with gZeroGuid. >>>>>> >>>>>> Pierre Gondois (14): >>>>>> ArmPkg/ArmMonitorLib: Definition for ArmMonitorLib library clas= s >>>>>> ArmPkg/ArmMonitorLib: Add ArmMonitorLib >>>>>> ArmPkg/ArmHvcNullLib: Add NULL instance of ArmHvcLib >>>>>> MdePkg/BaseRngLib: Rename ArmReadIdIsar0() to ArmGetFeatRng() >>>>>> ArmPkg/ArmLib: Add ArmReadIdIsar0() helper >>>>>> ArmPkg/ArmLib: Add ArmHasRngExt() >>>>>> SecurityPkg: Update Securitypkg.ci.yaml >>>>>> SecurityPkg/RngDxe: Replace Pcd with Sp80090Ctr256Guid >>>>>> SecurityPkg/RngDxe: Remove ArchGetSupportedRngAlgorithms() >>>>>> SecurityPkg/RngDxe: Documentation/include/parameter cleanup >>>>>> SecurityPkg/RngDxe: Check before advertising Cpu Rng algo >>>>>> SecurityPkg/RngDxe: Add debug warning for NULL >>>>>> PcdCpuRngSupportedAlgorithm >>>>>> SecurityPkg/RngDxe: Rename AArch64/RngDxe.c >>>>>> SecurityPkg/RngDxe: Add Arm support of RngDxe >>>>>> >>>>>> Sami Mujawar (8): >>>>>> ArmPkg: PCD to select conduit for monitor calls >>>>>> MdePkg/TrngLib: Definition for TRNG library class interface >>>>>> MdePkg/TrngLib: Add NULL instance of TRNG Library >>>>>> ArmPkg: Add FID definitions for Firmware TRNG >>>>>> ArmPkg/TrngLib: Add Arm Firmware TRNG library >>>>>> SecurityPkg/RngDxe: Rename RdRandGenerateEntropy to generic nam= e >>>>>> SecurityPkg/RngDxe: Add AArch64 RawAlgorithm support through >> TrngLib >>>>>> ArmVirtPkg: Kvmtool: Add RNG support using FW-TRNG interface >>>>>> >>>>>> ArmPkg/ArmPkg.dec | 12 +- >>>>>> ArmPkg/ArmPkg.dsc | 3 + >>>>>> ArmPkg/Include/IndustryStandard/ArmStdSmc.h | 109 ++++- >>>>>> ArmPkg/Include/Library/ArmLib.h | 12 +- >>>>>> ArmPkg/Include/Library/ArmMonitorLib.h | 42 ++ >>>>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h | 50 +++ >>>>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c | 403 >>>>> ++++++++++++++++++ >>>>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf | 29 ++ >>>>>> ArmPkg/Library/ArmHvcNullLib/ArmHvcNullLib.c | 29 ++ >>>>>> .../Library/ArmHvcNullLib/ArmHvcNullLib.inf | 22 + >>>>>> ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 15 +- >>>>>> ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h | 14 +- >>>>>> .../Library/ArmLib/AArch64/AArch64Support.S | 7 +- >>>>>> ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c | 16 +- >>>>>> ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.c | 34 ++ >>>>>> .../Library/ArmMonitorLib/ArmMonitorLib.inf | 29 ++ >>>>>> ArmVirtPkg/ArmVirtKvmTool.dsc | 10 + >>>>>> ArmVirtPkg/ArmVirtKvmTool.fdf | 5 + >>>>>> MdePkg/Include/Library/TrngLib.h | 121 ++++++ >>>>>> .../{ArmReadIdIsar0.S =3D> ArmGetFeatRng.S} | 8 +- >>>>>> .../{ArmReadIdIsar0.asm =3D> ArmGetFeatRng.asm} | 8 +- >>>>>> MdePkg/Library/BaseRngLib/AArch64/ArmRng.h | 2 +- >>>>>> MdePkg/Library/BaseRngLib/AArch64/Rndr.c | 2 +- >>>>>> MdePkg/Library/BaseRngLib/BaseRngLib.inf | 4 +- >>>>>> .../Library/BaseTrngLibNull/BaseTrngLibNull.c | 135 ++++++ >>>>>> .../BaseTrngLibNull/BaseTrngLibNull.inf | 30 ++ >>>>>> .../BaseTrngLibNull/BaseTrngLibNull.uni | 12 + >>>>>> MdePkg/MdePkg.dec | 5 + >>>>>> MdePkg/MdePkg.dsc | 1 + >>>>>> .../RngDxe/{AArch64/RngDxe.c =3D> ArmRngDxe.c} | 137 +++++- >>>>>> .../RandomNumberGenerator/RngDxe/ArmTrng.c | 71 +++ >>>>>> .../RngDxe/Rand/RdRand.c | 14 +- >>>>>> .../RngDxe/Rand/RdRand.h | 43 -- >>>>>> .../RngDxe/Rand/RngDxe.c | 36 +- >>>>>> .../RandomNumberGenerator/RngDxe/RngDxe.c | 52 +-- >>>>>> .../RandomNumberGenerator/RngDxe/RngDxe.inf | 17 +- >>>>>> .../RngDxe/RngDxeInternals.h | 44 +- >>>>>> SecurityPkg/SecurityPkg.ci.yaml | 1 + >>>>>> SecurityPkg/SecurityPkg.dsc | 12 +- >>>>>> 39 files changed, 1423 insertions(+), 173 deletions(-) >>>>>> create mode 100644 ArmPkg/Include/Library/ArmMonitorLib.h >>>>>> create mode 100644 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h >>>>>> create mode 100644 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c >>>>>> create mode 100644 ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf >>>>>> create mode 100644 ArmPkg/Library/ArmHvcNullLib/ArmHvcNullLib.c >>>>>> create mode 100644 ArmPkg/Library/ArmHvcNullLib/ArmHvcNullLib.in= f >>>>>> create mode 100644 ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.c >>>>>> create mode 100644 ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.in= f >>>>>> create mode 100644 MdePkg/Include/Library/TrngLib.h >>>>>> rename MdePkg/Library/BaseRngLib/AArch64/{ArmReadIdIsar0.S =3D> >>>>> ArmGetFeatRng.S} (78%) >>>>>> rename MdePkg/Library/BaseRngLib/AArch64/{ArmReadIdIsar0.asm =3D= > >>>>> ArmGetFeatRng.asm} (81%) >>>>>> create mode 100644 >> MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.c >>>>>> create mode 100644 >> MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf >>>>>> create mode 100644 >>> MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.uni >>>>>> rename >>> SecurityPkg/RandomNumberGenerator/RngDxe/{AArch64/RngDxe.c >>>>> =3D> ArmRngDxe.c} (51%) >>>>>> create mode 100644 >>>>> SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c >>>>>> delete mode 100644 >>>>> SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h >>>>>> >>>>> >>>>> >>>>> >>>>> >>>> >> >> >>=20 >> >=20