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.36860.1678117080614454479 for ; Mon, 06 Mar 2023 07:38:00 -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 70FFA12FC; Mon, 6 Mar 2023 07:38:43 -0800 (PST) Received: from [10.57.52.218] (unknown [10.57.52.218]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 02CC13F71A; Mon, 6 Mar 2023 07:37:58 -0800 (PST) Message-ID: <9404e123-ff27-f184-a63b-a587f842f428@arm.com> Date: Mon, 6 Mar 2023 16:37:56 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 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> <19cafc02-6589-462c-e200-00018486bb1f@arm.com> <17426C66FE22D73E.5513@groups.io> From: "PierreGondois" In-Reply-To: <17426C66FE22D73E.5513@groups.io> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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, 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 >>>>> >>> >>> >>> >>> >>> > > > > >