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.web12.11923.1648133775544224337 for ; Thu, 24 Mar 2022 07:56:16 -0700 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 5A9021515; Thu, 24 Mar 2022 07:56:14 -0700 (PDT) Received: from [10.57.41.44] (unknown [10.57.41.44]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 610743F73B; Thu, 24 Mar 2022 07:56:11 -0700 (PDT) Message-ID: <384ff0c1-374c-09c4-3bcb-8110dfda22e2@arm.com> Date: Thu, 24 Mar 2022 15:56:32 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library From: "PierreGondois" To: devel@edk2.groups.io, sami.mujawar@arm.com, Leif Lindholm Cc: ardb+tianocore@kernel.org, rebecca@bsdio.com, kraxel@redhat.com, michael.d.kinney@intel.com, gaoliming@byosoft.com.cn, zhiguang.liu@intel.com, jiewen.yao@intel.com, jian.j.wang@intel.com, Matteo.Carlini@arm.com, Akanksha.Jain2@arm.com, Ben.Adderson@arm.com, nd@arm.com References: <20211116113301.31088-1-sami.mujawar@arm.com> <20211116113301.31088-4-sami.mujawar@arm.com> <15621f6b-8df4-a65b-9996-92b68c1ae3c1@arm.com> <80941d66-5d31-053f-388a-95efe5dbbfdf@arm.com> In-Reply-To: <80941d66-5d31-053f-388a-95efe5dbbfdf@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit (Replacing Leif's mail address to the @quicinc.com one) On 3/24/22 10:46, Pierre Gondois wrote: > > On 11/25/21 16:23, Sami Mujawar via groups.io wrote: >> Hi Leif, >> >> Thank you for the feedback. >> >> Please find my response inline marked [SAMI]. >> >> Regards, >> >> Sami Mujawar >> >> >> On 24/11/2021 01:01 PM, Leif Lindholm wrote: >>> Hi Sami, >>> >>> On Tue, Nov 16, 2021 at 11:32:55 +0000, Sami Mujawar wrote: >>>> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668) >>>> >>>> The Arm True Random Number Generator Firmware, Interface 1.0, >>>> Platform Design Document >>>> (https://developer.arm.com/documentation/den0098/latest/) >>>> defines an interface between an Operating System (OS) executing >>>> at EL1 and Firmware (FW) exposing a conditioned entropy source >>>> that is provided by a TRNG back end. >>>> >>>> The conditioned entropy, that is provided by the TRNG FW interface, >>>> is commonly used to seed deterministic random number generators. >>>> >>>> This patch adds a TrngLib library that implements the Arm TRNG >>>> firmware interface. >>>> >>>> Signed-off-by: Sami Mujawar >>>> --- >>>> >>>> Notes: >>>> v2: >>>> - MdePkg\Include\Library\TrngLib.h is base type [LIMING] >>>> library. It can use RETURN_STATUS instead of >>>> EFI_STATUS. >>>> - Replaced EFI_STATUS with RETURN_STATUS. [SAMI] >>>> - MdePkg\Include\Library\TrngLib.h API parameter [LIMING] >>>> doesn't require CONST. CONST means the value >>>> specified by the input pointer will not be >>>> changed in API implementation. >>>> - Removed the use of constant pointers in the [SAMI] >>>> TRNG API. >>>> >>>> ArmPkg/ArmPkg.dsc | 1 + >>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h | 64 +++ >>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c | 483 ++++++++++++++++++++ >>>> ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf | 34 ++ >>>> 4 files changed, 582 insertions(+) >>>> > > [...] > >>>> + >>>> +/** Invoke the monitor call using the appropriate conduit. >>>> + If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit. >>>> + >>>> + @param [in, out] Args Arguments passed to and returned from the monitor. >>>> + >>>> + @return VOID >>>> +**/ >>>> +STATIC >>>> +VOID >>>> +ArmCallMonitor ( >>>> + IN OUT ARM_MONITOR_ARGS *Args >>>> + ) >>>> +{ >>>> + if (FeaturePcdGet (PcdMonitorConduitHvc)) { >>>> + ArmCallHvc ((ARM_HVC_ARGS*)Args); >>>> + } else { >>>> + ArmCallSmc ((ARM_SMC_ARGS*)Args); >>>> + } >>>> +} >>> Should this be in (a potentially renamed) ArmSmcLib? >> [SAMI] Looking at ArmSmcLib and ArmHvcLib libraries there is not much >> difference in the code other than the SMC/HVC call. Please let me know >> if I should submit a patch to unify these in ArmMonitorLib? >> The ArmCall APIs would still remain the same but moved to >> ArmMonitorLib. > > Hello Leif, > About your comment, I am not sure I understand it correctly. Assuming the > function allowing to choose the conduit looks like: > > VOID > EFIAPI > ArmConduitCall (UINT8 Conduit) { > if (Conduit == 0) { > ArmCallHvc ((ARM_HVC_ARGS*)Args); > } else if (Conduit == 1) { > ArmCallSmc ((ARM_SMC_ARGS*)Args); > } else { > ASSERT (FALSE); > } > } > > Do you suggest to: > 1. Make ArmSmcLib dependent on ArmHvcLib and add ArmConduitCall() > in ArmSmcLib (or do the opposite with the ArmHvcLib) > 2. Merge ArmSmcLib and ArmHvcLib in a new ArmConduitLibrary and add > ArmConduitCall() in this new library. > 3. Add an ArmConduitLibrary, relying on ArmSmcLib and ArmHvcLib, and having > only one function: ArmConduitCall() > > 2. would make the Hvc and Smc calls really tied together. > 3. would avoid creating new dependencies on existing libraries (i.e. a > platform only using the ArmSmcLib would not require to have a NULL instance of > ArmHvcLib). I assume you meant 3. > > Regards, > Pierre >