SecurityPkg/MdePkg: RngLib GUID

[edk2-devel] [PATCH v1 1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation

Posted by PierreGondois 2 years, 9 months ago

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;
 }
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104342): https://edk2.groups.io/g/devel/message/104342
Mute This Topic: https://groups.io/mt/98779038/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation

Posted by Kun Qin 2 years, 7 months ago

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;
>   }


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106529): https://edk2.groups.io/g/devel/message/106529
Mute This Topic: https://groups.io/mt/98779038/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation

Posted by PierreGondois 2 years, 7 months ago

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;
>>    }


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106566): https://edk2.groups.io/g/devel/message/106566
Mute This Topic: https://groups.io/mt/98779038/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation

Posted by Sami Mujawar 2 years, 7 months ago

Hi Pierre,

Thank you for this patch.

This change looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 09/05/2023 08:40 am, pierre.gondois@arm.com 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;
>   }


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106506): https://edk2.groups.io/g/devel/message/106506
Mute This Topic: https://groups.io/mt/98779038/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-