I don’t mind, please go ahead. > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard > Biesheuvel > Sent: Tuesday, March 7, 2023 12:22 AM > To: Yao, Jiewen <jiewen....@intel.com> > Cc: Pierre Gondois <pierre.gond...@arm.com>; devel@edk2.groups.io; Leif > Lindholm <l...@nuviainc.com>; Sami Mujawar <sami.muja...@arm.com>; > Wang, Jian J <jian.j.w...@intel.com> > Subject: Re: [edk2-devel] [PATCH v3 0/4] ArmPkg/SecurityPkg: Fixes for > ArmTrngLib/RngDxe > > On Mon, 6 Mar 2023 at 16:42, Yao, Jiewen <jiewen....@intel.com> wrote: > > > > Hi Pierre > > I don’t have strong opinion. > > > > For ARM specific patch, would you please get R-B from ARM expert? > > > > I think we need to wait for the response from Ard to confirm. > > > > These patches > > SecurityPkg/RngDxe: Correctly update mAvailableAlgoArrayCount > SecurityPkg/RngDxe: Conditionally install EFI_RNG_PROTOCOL > > Reviewed-by: Ard Biesheuvel <a...@kernel.org> > > Jiewen, if you don't mind, I will merge those right away. > > For the remaining patch, I am not sure I understand why the behavior > regarding the zero GUID is correct. Perhaps we could > revisit/resend/review that patch in isolation? > > Thanks, > Ard. > > > > > > > > > -----Original Message----- > > > From: Pierre Gondois <pierre.gond...@arm.com> > > > Sent: Monday, March 6, 2023 11:38 PM > > > To: devel@edk2.groups.io > > > Cc: Leif Lindholm <l...@nuviainc.com>; Sami Mujawar > > > <sami.muja...@arm.com>; Yao, Jiewen <jiewen....@intel.com>; > Wang, > > > Jian J <jian.j.w...@intel.com>; Ard Biesheuvel <a...@kernel.org> > > > Subject: Re: [edk2-devel] [PATCH v3 0/4] ArmPkg/SecurityPkg: Fixes for > > > ArmTrngLib/RngDxe > > > > > > Hello Jiewen, Jian, > > > Do these patches in this set look ok to you ? > > > - SecurityPkg/RngDxe: Correctly update mAvailableAlgoArrayCount > > > - SecurityPkg/RngDxe: Conditionally install EFI_RNG_PROTOCOL > > > - SecurityPkg/RngDxe: Fix Rng algo selection for Arm > > > > > > Regards, > > > Pierre > > > > > > On 2/10/23 10:26, PierreGondois via groups.io wrote: > > > > Hello Jiewen, Jian, > > > > Just a reminder in case this was forgotten, > > > > Regards, > > > > Pierre > > > > > > > > On 1/9/23 13:26, Pierre Gondois wrote: > > > >> Hello, > > > >> I was wondering if I should re-arrange the patch in any way/there was > any > > > >> concern with the patch set. The first patch of serie was taken, but the > > > other > > > >> ones are pending, > > > >> > > > >> Regards, > > > >> Pierre > > > >> > > > >> On 11/28/22 14:34, PierreGondois via groups.io wrote: > > > >>> Hello Ard, > > > >>> > > > >>> On 11/26/22 15:33, Ard Biesheuvel wrote: > > > >>>> On Thu, 24 Nov 2022 at 17:18, <pierre.gond...@arm.com> wrote: > > > >>>>> > > > >>>>> From: Pierre Gondois <pierre.gond...@arm.com> > > > >>>>> > > > >>>>> v1: > > > >>>>> - https://edk2.groups.io/g/devel/message/96356 > > > >>>>> v2: > > > >>>>> - https://edk2.groups.io/g/devel/message/96434 > > > >>>>> - Reformulate commit message. > > > >>>>> - Do not warn if no algorithm is found as the message > > > >>>>> would be printed on non-Arm platforms. > > > >>>>> v3: > > > >>>>> - Add the following patches: > > > >>>>> 1. ArmPkg/ArmTrngLib: Remove ASSERTs in > > > ArmTrngLibConstructor() > > > >>>>> Requested by Ard. > > > >>>>> Cf https://edk2.groups.io/g/devel/message/96495 > > > >>>>> 2. SecurityPkg/RngDxe: Conditionally install > EFI_RNG_PROTOCOL > > > >>>>> Do not install EFI_RNG_PROTOCOL if no RNG algorithm is > > > available. > > > >>>>> Cf. https://edk2.groups.io/g/devel/message/96494 > > > >>>>> 3. SecurityPkg/RngDxe: Fix Rng algo selection for Arm > > > >>>>> Coming from v2 patch being split. > > > >>>>> > > > >>>>> Some issues were found by Ard/Sami on the RngDxe/ArmTrngLib > after > > > >>>>> recent patches were merged. This patch serie intends to fix them. > > > >>>>> > > > >>>>> Pierre Gondois (4): > > > >>>>> ArmPkg/ArmTrngLib: Remove ASSERTs in > ArmTrngLibConstructor() > > > >>>> > > > >>>> Thanks for the fixed > > > >>>> > > > >>>> Reviewed-by: Ard Biesheuvel <a...@kernel.org> > > > >>>> > > > >>>> I pushed this one as #3663 (pending CI verification atm) > > > >>>> > > > >>>>> SecurityPkg/RngDxe: Correctly update > mAvailableAlgoArrayCount > > > >>>>> SecurityPkg/RngDxe: Conditionally install EFI_RNG_PROTOCOL > > > >>>>> SecurityPkg/RngDxe: Fix Rng algo selection for Arm > > > >>>>> > > > >>>> > > > >>>> The remaining code still looks a bit clunky to me. Can't we just > > > >>>> return an error from the library constructor of the library cannot > > > >>>> initialize due to a missing prerequisite? > > > >>> > > > >>> In RngDriverEntry(), GetAvailableAlgorithms() probe the available > > > >>> RNG algorithms. If there are none, then we return an error code. > > > >>> Otherwise we install EFI_RNG_PROTOCOL. > > > >>> > > > >>> I assume the prerequisite you a referring to is 'checking there is > > > >>> at least one available RNG algorithm that RngDxe can use', so if > > > >>> ArmTrngLib's constructor fails, we should not prevent RngDxe to be > > > >>> loaded (as the RngLib could be used for instance). > > > >>> > > > >>> Does it answer your question, or did I understand it incorrectly ? > > > >>> > > > >>> Regards, > > > >>> Pierre > > > >>> > > > >>>> > > > >>>> > > > >>>>> ArmPkg/Library/ArmTrngLib/ArmTrngLib.c | 5 ----- > > > >>>>> .../RandomNumberGenerator/RngDxe/ArmRngDxe.c | 18 > +++++--- > > > ---------- > > > >>>>> .../RngDxe/Rand/RngDxe.c | 9 ++++++++- > > > >>>>> .../RandomNumberGenerator/RngDxe/RngDxe.c | 19 > > > ++++++++++++++----- > > > >>>>> 4 files changed, 27 insertions(+), 24 deletions(-) > > > >>>>> > > > >>>>> -- > > > >>>>> 2.25.1 > > > >>>>> > > > >>> > > > >>> > > > >>> > > > >>> > > > >>> > > > > > > > > > > > > > > > > > > > > > > > >
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100790): https://edk2.groups.io/g/devel/message/100790 Mute This Topic: https://groups.io/mt/95240503/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-