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.12673.1688133396193368586
 for <devel@edk2.groups.io>;
 Fri, 30 Jun 2023 06:56:36 -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 603FFD75;
	Fri, 30 Jun 2023 06:57:19 -0700 (PDT)
Received: from [10.57.86.110] (unknown [10.57.86.110])
	by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A14C83F73F;
	Fri, 30 Jun 2023 06:56:33 -0700 (PDT)
Message-ID: <d430cd5a-895d-a56d-5425-9ffb14ebc780@arm.com>
Date: Fri, 30 Jun 2023 15:56:27 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [edk2-devel] [PATCH v1 1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation
To: Kun Qin <kuqin12@gmail.com>, devel@edk2.groups.io
Cc: Michael D Kinney <michael.d.kinney@intel.com>,
 Liming Gao <gaoliming@byosoft.com.cn>, Zhiguang Liu
 <zhiguang.liu@intel.com>, Jiewen Yao <jiewen.yao@intel.com>,
 Jian J Wang <jian.j.wang@intel.com>,
 Ard Biesheuvel <ardb+tianocore@kernel.org>,
 Sami Mujawar <sami.mujawar@arm.com>, Jose Marinho <Jose.Marinho@arm.com>,
 Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
References: <20230509074042.1523428-1-pierre.gondois@arm.com>
 <20230509074042.1523428-2-pierre.gondois@arm.com>
 <b998fc94-a7c9-6fff-be57-317e09d7dfce@gmail.com>
From: "PierreGondois" <pierre.gondois@arm.com>
In-Reply-To: <b998fc94-a7c9-6fff-be57-317e09d7dfce@gmail.com>
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hello Kun,
The reason was that:
- KvmTool uses the SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf module
- the RngDxe.inf modules requires an ArmTrng implementation
- GetArmTrngVersion() is called to check whether there is a Trng backend
- GetArmTrngVersion() asserts as it is a NULL implementation

but it seems the actual implementation of the ArmTrngLib could be used instead.
The returned error code is actually the same (UNSUPPORTED). I will check with
Sami if this is ok to do this instead.

Regards,
Pierre

On 6/29/23 22:34, Kun Qin wrote:
> Hi Pierre,
> 
> Do we really need this removal of ASSERT? I tried to use the real
> ArmTrngLib with this patch
> and it seems to work fine with a TFA that does not support TRNG interfaces.
> 
> I think it would be valuable to keep the ASSERT to indicate there might
> be an integration error?
> 
> Please let me know if I missed anything.
> 
> Regards,
> Kun
> 
> On 5/9/2023 12:40 AM, PierreGondois wrote:
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> Remove ASSERTs to allow RngDxe probing the Null implementation
>> of the TrngLib.
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>    MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c | 4 ----
>>    1 file changed, 4 deletions(-)
>>
>> diff --git a/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c b/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
>> index 316d78bf5e83..0ea9aafa59f1 100644
>> --- a/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
>> +++ b/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
>> @@ -41,7 +41,6 @@ GetArmTrngVersion (
>>      OUT UINT16  *MinorRevision
>>      )
>>    {
>> -  ASSERT (FALSE);
>>      return RETURN_UNSUPPORTED;
>>    }
>>    
>> @@ -67,7 +66,6 @@ GetArmTrngUuid (
>>      OUT GUID  *Guid
>>      )
>>    {
>> -  ASSERT (FALSE);
>>      return RETURN_UNSUPPORTED;
>>    }
>>    
>> @@ -83,7 +81,6 @@ GetArmTrngMaxSupportedEntropyBits (
>>      VOID
>>      )
>>    {
>> -  ASSERT (FALSE);
>>      return 0;
>>    }
>>    
>> @@ -116,6 +113,5 @@ GetArmTrngEntropy (
>>      OUT UINT8  *Buffer
>>      )
>>    {
>> -  ASSERT (FALSE);
>>      return RETURN_UNSUPPORTED;
>>    }