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. > -----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 (#100737): https://edk2.groups.io/g/devel/message/100737 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] -=-=-=-=-=-=-=-=-=-=-=-
