Hi Pierre Sorry, I should have given the specific comment. [PATCH v3 12/22] SecurityPkg: Update Securitypkg.ci.yaml
Why we need ArmPkg as new dependency? I don’t think this is good idea. Can we have a way to remove that dependency? Thank you Yao, Jiewen > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen > Sent: Thursday, July 21, 2022 6:33 PM > To: Pierre Gondois <pierre.gond...@arm.com>; devel@edk2.groups.io > Cc: Sami Mujawar <sami.muja...@arm.com>; Leif Lindholm > <quic_llind...@quicinc.com>; Ard Biesheuvel <ardb+tianoc...@kernel.org>; > Rebecca Cran <rebe...@bsdio.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>; > Wang, Jian J <jian.j.w...@intel.com> > Subject: Re: [edk2-devel] [PATCH v3 00/22] Add Raw algorithm support using > Arm FW-TRNG interface > > Hi Pierre > I give the feedback on interface design and dependency, esp SecurityPkg and > MdePkg. > 1) Please don’t change package dependency policy. > 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. > > Thank you > Yao, Jiewen > > > > -----Original Message----- > > From: Pierre Gondois <pierre.gond...@arm.com> > > Sent: Thursday, July 21, 2022 4:55 PM > > To: Yao, Jiewen <jiewen....@intel.com>; devel@edk2.groups.io > > Cc: Sami Mujawar <sami.muja...@arm.com>; Leif Lindholm > > <quic_llind...@quicinc.com>; Ard Biesheuvel <ardb+tianoc...@kernel.org>; > > Rebecca Cran <rebe...@bsdio.com>; Kinney, Michael D > > <michael.d.kin...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>; > > Wang, Jian J <jian.j.w...@intel.com> > > Subject: Re: [edk2-devel] [PATCH v3 00/22] Add Raw algorithm support using > > 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 to > > > ArmPkg? > > > Then you don’t 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 <devel@edk2.groups.io> On Behalf Of > > >> PierreGondois > > >> Sent: Wednesday, July 20, 2022 10:59 PM > > >> To: devel@edk2.groups.io > > >> Cc: Sami Mujawar <sami.muja...@arm.com>; Leif Lindholm > > >> <quic_llind...@quicinc.com>; Ard Biesheuvel > <ardb+tianoc...@kernel.org>; > > >> Rebecca Cran <rebe...@bsdio.com>; Kinney, Michael D > > >> <michael.d.kin...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>; > > Yao, > > >> Jiewen <jiewen....@intel.com>; Wang, Jian J <jian.j.w...@intel.com> > > >> 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 <pierre.gond...@arm.com> > > >>> > > >>> Bugzilla: Bug 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668) > > >>> > > >>> The Arm True Random Number Generator Firmware, Interface 1.0, > > >> specification > > >>> 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. > > >>> 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/Virtual > > >>> 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 addition > > >>> 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 there 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 class > > >>> 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 name > > >>> 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 => ArmGetFeatRng.S} | 8 +- > > >>> .../{ArmReadIdIsar0.asm => 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 => 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.inf > > >>> create mode 100644 ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.c > > >>> create mode 100644 ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.inf > > >>> create mode 100644 MdePkg/Include/Library/TrngLib.h > > >>> rename MdePkg/Library/BaseRngLib/AArch64/{ArmReadIdIsar0.S => > > >> ArmGetFeatRng.S} (78%) > > >>> rename MdePkg/Library/BaseRngLib/AArch64/{ArmReadIdIsar0.asm => > > >> 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 > > >> => ArmRngDxe.c} (51%) > > >>> create mode 100644 > > >> SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c > > >>> delete mode 100644 > > >> SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h > > >>> > > >> > > >> > > >> > > >> > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91631): https://edk2.groups.io/g/devel/message/91631 Mute This Topic: https://groups.io/mt/92066719/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
