Re: [edk2-devel] [PATCH v4 5/6] CryptoPkg/OpensslLib: Add AArch64Cap for arch specific hooks

["PierreGondois" <pierre.gondois@arm.com>]

On 11/20/23 19:39, Leif Lindholm wrote:
> On Mon, Nov 20, 2023 at 14:48:25 +0100, Pierre Gondois wrote:
>> Add AARCH64 specific implementations of:
>> - OPENSSL_cpuid_setup(), probing hardware capabilitie
>>    (presence of FEAT_AES, etc.)
>> - OPENSSL_rdtsc(), returning non-trusted entropy by accessing
>>    system counter.
>>
>> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   .../Library/OpensslLib/OpensslLibAccel.inf    |  7 ++
>>   .../OpensslLib/OpensslLibFullAccel.inf        |  7 ++
>>   .../OpensslLib/OpensslStub/AArch64Cap.c       | 84 +++++++++++++++++++
>>   3 files changed, 98 insertions(+)
>>   create mode 100644 CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
>>
>> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
>> index 3d1a9638b1c1..08e8be6ea9e1 100644
>> --- a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
>> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
>> @@ -1329,6 +1329,7 @@ [Sources.X64]
>>   # Autogenerated files list ends here
>>   
>>   [Sources.AARCH64]
>> +  OpensslStub/AArch64Cap.c
>>   # Autogenerated files list starts here
>>     $(OPENSSL_PATH)/crypto/aes/aes_cbc.c
>>     $(OPENSSL_PATH)/crypto/aes/aes_cfb.c
>> @@ -1955,11 +1956,17 @@ [Packages]
>>     MdePkg/MdePkg.dec
>>     CryptoPkg/CryptoPkg.dec
>>   
>> +[Packages.AARCH64]
>> +  ArmPkg/ArmPkg.dec
>> +
>>   [LibraryClasses]
>>     BaseLib
>>     DebugLib
>>     RngLib
>>   
>> +[LibraryClasses.AARCH64]
>> +  ArmLib
>> +
>>   [BuildOptions]
>>     #
>>     # Disables the following Visual Studio compiler warnings brought by openssl source,
>> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
>> index e7e83d419f4b..2a01ffe06bd7 100644
>> --- a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
>> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
>> @@ -1432,6 +1432,7 @@ [Sources.X64]
>>   # Autogenerated files list ends here
>>   
>>   [Sources.AARCH64]
>> +  OpensslStub/AArch64Cap.c
>>   # Autogenerated files list starts here
>>     $(OPENSSL_PATH)/crypto/aes/aes_cbc.c
>>     $(OPENSSL_PATH)/crypto/aes/aes_cfb.c
>> @@ -2107,11 +2108,17 @@ [Packages]
>>     MdePkg/MdePkg.dec
>>     CryptoPkg/CryptoPkg.dec
>>   
>> +[Packages.AARCH64]
>> +  ArmPkg/ArmPkg.dec
>> +
>>   [LibraryClasses]
>>     BaseLib
>>     DebugLib
>>     RngLib
>>   
>> +[LibraryClasses.AARCH64]
>> +  ArmLib
>> +
>>   [BuildOptions]
>>     #
>>     # Disables the following Visual Studio compiler warnings brought by openssl source,
>> diff --git a/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
>> new file mode 100644
>> index 000000000000..7468ef3ab54e
>> --- /dev/null
>> +++ b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
>> @@ -0,0 +1,84 @@
>> +/** @file
>> +  Arm capabilities probing.
>> +
>> +  Copyright (c) 2023, Arm Limited. All rights reserved.<BR>
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +**/
>> +
>> +#include <openssl/types.h>
>> +#include "crypto/arm_arch.h"
>> +
>> +#include <Library/ArmLib.h>
>> +
>> +/** Get bits from a value.
>> +
>> +  Shift the input value from 'shift' bits and apply 'mask'.
>> +
>> +  @param   value    The value to get the bits from.
>> +  @param   shift    Index of the bits to read.
>> +  @param   mask     Mask to apply to the value once shifted.
>> +
>> +  @return  The desired bitfield from the value.
>> +**/
>> +#define GET_BITFIELD(value, shift, mask)    \
>> +  ((value >> shift) & mask)
> 
> (This macro appears unused here now.)
> 
>> +
>> +UINT32  OPENSSL_armcap_P = 0;
>> +
>> +void
>> +OPENSSL_cpuid_setup (
>> +  void
>> +  )
>> +{
>> +  OPENSSL_armcap_P = 0;
>> +
>> +  /* Access to EL0 registers is possible from higher ELx. */
>> +  OPENSSL_armcap_P |= ARMV8_CPUID;
>> +  /* Access to Physical timer is possible. */
>> +  OPENSSL_armcap_P |= ARMV7_TICK;
>> +
>> +  /* Neon support is not guaranteed, but it is assumed to be present.
>> +     Arm ARM for Armv8, sA1.5 Advanced SIMD and floating-point support
>> +  */
>> +  OPENSSL_armcap_P |= ARMV7_NEON;
>> +
>> +  if (ArmHasAes ())
>> +  {
> 
> And I think that curly bracket is supposed to be on the previous line
> (and similarly below), but this may be intional to align with nearby
> code?

I think it was changed by uncrustify. I changed it back.
Thanks for the review,

Regards,
Pierre

> 
> Anyway, this is a big readability improvement, thank you very much!
> Acked-by: Leif Lindholm <quic_llindhol@quicinc.com>
> 
> /
>      Leif
> 
>> +    OPENSSL_armcap_P |= ARMV8_AES;
>> +  }
>> +
>> +  if (ArmHasSha1 ())
>> +  {
>> +    OPENSSL_armcap_P |= ARMV8_SHA1;
>> +  }
>> +
>> +  if (ArmHasSha256 ())
>> +  {
>> +    OPENSSL_armcap_P |= ARMV8_SHA256;
>> +  }
>> +
>> +  if (ArmHasPmull ())
>> +  {
>> +    OPENSSL_armcap_P |= ARMV8_PMULL;
>> +  }
>> +
>> +  if (ArmHasSha512 ())
>> +  {
>> +    OPENSSL_armcap_P |= ARMV8_SHA512;
>> +  }
>> +}
>> +
>> +/** Read system counter value.
>> +
>> +  Used to get some non-trusted entropy.
>> +
>> +  @return Lower bits of the physical counter.
>> +**/
>> +uint32_t
>> +OPENSSL_rdtsc (
>> +  void
>> +  )
>> +{
>> +  return (UINT32)ArmReadCntPct ();
>> +}
>> -- 
>> 2.25.1
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111540): https://edk2.groups.io/g/devel/message/111540
Mute This Topic: https://groups.io/mt/102707267/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-