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.web11.70340.1673267190473297366 for ; Mon, 09 Jan 2023 04:26:30 -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 24E701042; Mon, 9 Jan 2023 04:27:11 -0800 (PST) Received: from [10.34.100.128] (pierre123.nice.arm.com [10.34.100.128]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 450EC3F67D; Mon, 9 Jan 2023 04:26:28 -0800 (PST) Message-ID: <19cafc02-6589-462c-e200-00018486bb1f@arm.com> Date: Mon, 9 Jan 2023 13:26:23 +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 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> From: "PierreGondois" In-Reply-To: <172BC2FE233A5592.368@groups.io> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 >>> > > > > >