From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web11.18400.1678203502943108861 for ; Tue, 07 Mar 2023 07:38:23 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=DlfIAQaq; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 071FEB81902 for ; Tue, 7 Mar 2023 15:38:21 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C63A1C433A0 for ; Tue, 7 Mar 2023 15:38:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1678203499; bh=FZL2NH1FjG+G4EY0HPisHKD6Zq3hS1EVdddXDIlW81E=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=DlfIAQaqYufwm+zdiff+pM/9JFmf3q8z/9XoAfhtx90ebEQoA2lO+y7XLI3isfK1v TFJtcoeZOra42Ims+aES+eriI4oi7IZ3Qnf/HCVlXqOo3Q/uoNtgvHts6erEradKkH jPPeNeGTJ/vKXpywISKr/XwNI9iSbdljYayQbEW2tKYwkecNoAC41DMoEE9V+34glq CeH22ZGgXm6cG04YYyNHeuof6SyBvZJlRT9GhQ/ENDpqGt7Peqc3n4B81zrr4yDrtw cScirVYWhAaS8nS6arnIa60pCAHZ5wXGo8H+GUZ5I0RojGTo2JbMIea6O0tIvAK6VD eKCsHftaT3Mrw== Received: by mail-lf1-f46.google.com with SMTP id g17so17624482lfv.4 for ; Tue, 07 Mar 2023 07:38:19 -0800 (PST) X-Gm-Message-State: AO0yUKU5Yq33vH4gn0g8Q2ULf46JgOawaO0XkpUK1FqNySHGlwlHhUIn u6OIzjiqKNWYrN6upuqIAJgj/pKIWSMee8X7B7o= X-Google-Smtp-Source: AK7set+GfVrlTIkGNf0UsAI9kbCCESS8w5emBkTPN4hoURJIwrr+nMbc/OMzw6sPQpE5oClemlcSQzYEJN/Ce4Gbp/c= X-Received: by 2002:ac2:5923:0:b0:4d5:ca42:e43e with SMTP id v3-20020ac25923000000b004d5ca42e43emr4649797lfi.7.1678203497737; Tue, 07 Mar 2023 07:38:17 -0800 (PST) MIME-Version: 1.0 References: <20221124161756.216996-1-Pierre.Gondois@arm.com> <172BC2FE233A5592.368@groups.io> <19cafc02-6589-462c-e200-00018486bb1f@arm.com> <17426C66FE22D73E.5513@groups.io> <9404e123-ff27-f184-a63b-a587f842f428@arm.com> In-Reply-To: From: "Ard Biesheuvel" Date: Tue, 7 Mar 2023 16:38:06 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v3 0/4] ArmPkg/SecurityPkg: Fixes for ArmTrngLib/RngDxe To: "Yao, Jiewen" Cc: "devel@edk2.groups.io" , Pierre Gondois , Leif Lindholm , Sami Mujawar , "Wang, Jian J" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 7 Mar 2023 at 08:53, Yao, Jiewen wrote: > > I don=E2=80=99t mind, please go ahead. > Thanks. Merged as #4109 Note that I replaced the 'return EFI_UNSUPPORTED' with 'return EFI_REQUEST_UNLOAD_IMAGE' so that RngDxe is simply unloaded again without an error if no RNG sources are available to it. > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Ard > > Biesheuvel > > Sent: Tuesday, March 7, 2023 12:22 AM > > To: Yao, Jiewen > > Cc: Pierre Gondois ; devel@edk2.groups.io; Leif > > Lindholm ; Sami Mujawar ; > > Wang, Jian J > > Subject: Re: [edk2-devel] [PATCH v3 0/4] ArmPkg/SecurityPkg: Fixes for > > ArmTrngLib/RngDxe > > > > On Mon, 6 Mar 2023 at 16:42, Yao, Jiewen wrote: > > > > > > Hi Pierre > > > I don=E2=80=99t 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 > > > > 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 > > > > Sent: Monday, March 6, 2023 11:38 PM > > > > To: devel@edk2.groups.io > > > > Cc: Leif Lindholm ; Sami Mujawar > > > > ; Yao, Jiewen ; > > Wang, > > > > Jian J ; Ard Biesheuvel > > > > 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/ther= e 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, wrote: > > > > >>>>> > > > > >>>>> From: Pierre Gondois > > > > >>>>> > > > > >>>>> 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 t= hem. > > > > >>>>> > > > > >>>>> Pierre Gondois (4): > > > > >>>>> ArmPkg/ArmTrngLib: Remove ASSERTs in > > ArmTrngLibConstructor() > > > > >>>> > > > > >>>> Thanks for the fixed > > > > >>>> > > > > >>>> Reviewed-by: Ard Biesheuvel > > > > >>>> > > > > >>>> I pushed this one as #3663 (pending CI verification atm) > > > > >>>> > > > > >>>>> SecurityPkg/RngDxe: Correctly update > > mAvailableAlgoArrayCount > > > > >>>>> SecurityPkg/RngDxe: Conditionally install EFI_RNG_PROTO= COL > > > > >>>>> SecurityPkg/RngDxe: Fix Rng algo selection for Arm > > > > >>>>> > > > > >>>> > > > > >>>> The remaining code still looks a bit clunky to me. Can't we ju= st > > > > >>>> return an error from the library constructor of the library ca= nnot > > > > >>>> initialize due to a missing prerequisite? > > > > >>> > > > > >>> In RngDriverEntry(), GetAvailableAlgorithms() probe the availab= le > > > > >>> 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 i= f > > > > >>> 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 incorrectl= y ? > > > > >>> > > > > >>> 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 > > > > >>>>> > > > > >>> > > > > >>> > > > > >>> > > > > >>> > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >=20 > > >