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.10479.1676021200408400347 for ; Fri, 10 Feb 2023 01:26:40 -0800 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 EAB302F4; Fri, 10 Feb 2023 01:27:21 -0800 (PST) Received: from [10.57.50.132] (unknown [10.57.50.132]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B4AC43F71E; Fri, 10 Feb 2023 01:26:37 -0800 (PST) Message-ID: <28d536b5-ef46-807c-bcec-a00ecffb7fe2@arm.com> Date: Fri, 10 Feb 2023 10:26:31 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [edk2-devel] [PATCH v3 0/4] ArmPkg/SecurityPkg: Fixes for ArmTrngLib/RngDxe From: "PierreGondois" To: devel@edk2.groups.io Cc: Leif Lindholm , Sami Mujawar , Jiewen Yao , Jian J Wang , Ard Biesheuvel References: <20221124161756.216996-1-Pierre.Gondois@arm.com> <172BC2FE233A5592.368@groups.io> <19cafc02-6589-462c-e200-00018486bb1f@arm.com> In-Reply-To: <19cafc02-6589-462c-e200-00018486bb1f@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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, 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 them. >>>> >>>> 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_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 >>>> >> >> >> >> >>