Re: [edk2-devel] [PATCH v2 1/7] MdePkg/BaseLib: AARCH64: Add ArmReadCntPctReg()

["Leif Lindholm" <quic_llindhol@quicinc.com>]

On Thu, Nov 09, 2023 at 10:23:01 +0100, Pierre Gondois wrote:
> To enable AARCH64 native instruction support for Openssl,
> some interfaces must be implemented. OPENSSL_rdtsc() requests
> an access to a counter to get some non-trusted entropy.
> 
> Add ArmReadCntPctReg() to read system count.
> A similar ArmReadCntPct() function is available in the ArmPkg,
> but the CryptoPkg where OPENSSL_rdtsc will reside cannot rely
> on the ArmPkg.

This is patently untrue, as can be discovered by grepping for ArmPkg
under CryptoPkg already.

Yes, we have a problematic history around how architectures that
weren't already in tree when edk2 was first published got
introduced at a later date. But this bit of contortionism helps no one.
Please move this to ArmPkg, which is effectively an exclave of MdePkg
anyway.

(Yes, there is an argument for moving ArmLib into MdePkg, but that
quickly escalates through dependencies to moving all of ArmPkg into
MdePkg, and that's a fairly big task.)

/
    Leif

> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>  MdePkg/Include/Library/BaseLib.h              | 14 +++++++++
>  .../BaseLib/AArch64/ArmReadCntPctReg.S        | 30 +++++++++++++++++++
>  .../BaseLib/AArch64/ArmReadCntPctReg.asm      | 30 +++++++++++++++++++
>  MdePkg/Library/BaseLib/BaseLib.inf            |  4 ++-
>  4 files changed, 77 insertions(+), 1 deletion(-)
>  create mode 100644 MdePkg/Library/BaseLib/AArch64/ArmReadCntPctReg.S
>  create mode 100644 MdePkg/Library/BaseLib/AArch64/ArmReadCntPctReg.asm
> 
> diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
> index 5d7067ee854e..b81c9dd83508 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -126,6 +126,20 @@ typedef struct {
>  
>  #define BASE_LIBRARY_JUMP_BUFFER_ALIGNMENT  8
>  
> +/**
> +  Reads the current value of CNTPCT_EL0 register.
> +
> +  Reads and returns the current value of CNTPCT_EL0.
> +  This function is only available on AARCH64.
> +
> +  @return The current value of CNTPCT_EL0
> +**/
> +UINT64
> +EFIAPI
> +ArmReadCntPctReg (
> +  VOID
> +  );
> +
>  #endif // defined (MDE_CPU_AARCH64)
>  
>  #if defined (MDE_CPU_RISCV64)
> diff --git a/MdePkg/Library/BaseLib/AArch64/ArmReadCntPctReg.S b/MdePkg/Library/BaseLib/AArch64/ArmReadCntPctReg.S
> new file mode 100644
> index 000000000000..d5f3a0082a99
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/AArch64/ArmReadCntPctReg.S
> @@ -0,0 +1,30 @@
> +#------------------------------------------------------------------------------
> +#
> +# ArmReadCntPctReg() for AArch64
> +#
> +# Copyright (c) 2023, Arm Limited. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#------------------------------------------------------------------------------
> +
> +.text
> +.p2align 2
> +GCC_ASM_EXPORT(ArmReadCntPctReg)
> +
> +#/**
> +#  Reads the CNTPCT_EL0 Register.
> +#
> +#  @return The contents of the CNTPCT_EL0 register.
> +#
> +#**/
> +#UINT64
> +#EFIAPI
> +#ArmReadCntPctReg (
> +#  VOID
> +#  );
> +#
> +ASM_PFX(ArmReadCntPctReg):
> +  AARCH64_BTI(c)
> +  mrs   x0, cntpct_el0
> +  ret
> diff --git a/MdePkg/Library/BaseLib/AArch64/ArmReadCntPctReg.asm b/MdePkg/Library/BaseLib/AArch64/ArmReadCntPctReg.asm
> new file mode 100644
> index 000000000000..cfdfe4cea4eb
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/AArch64/ArmReadCntPctReg.asm
> @@ -0,0 +1,30 @@
> +;------------------------------------------------------------------------------
> +;
> +; ArmReadCntPctReg() for AArch64
> +;
> +; Copyright (c) 2023, Arm Limited. All rights reserved.<BR>
> +;
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +;------------------------------------------------------------------------------
> +
> +  EXPORT ArmReadCntPctReg
> +  AREA BaseLib_LowLevel, CODE, READONLY
> +
> +;/**
> +;  Reads the CNTPCT_EL0 Register.
> +;
> +; @return The contents of the CNTPCT_EL0 register.
> +;
> +;**/
> +;UINT64
> +;EFIAPI
> +;ArmReadCntPctReg (
> +;  VOID
> +;  );
> +;
> +ArmReadCntPctReg
> +  mrs   x0, cntpct_el0
> +  ret
> +
> +  END
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
> index 03c7b02e828b..24e5e6c3ecb5 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -3,7 +3,7 @@
>  #
>  #  Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
>  #  Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
> -#  Portions copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
> +#  Portions copyright (c) 2011 - 2023, Arm Limited. All rights reserved.<BR>
>  #  Copyright (c) 2020 - 2021, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -376,6 +376,7 @@ [Sources.AARCH64]
>    AArch64/SetJumpLongJump.S         | GCC
>    AArch64/CpuBreakpoint.S           | GCC
>    AArch64/SpeculationBarrier.S      | GCC
> +  AArch64/ArmReadCntPctReg.S        | GCC
>  
>    AArch64/MemoryFence.asm           | MSFT
>    AArch64/SwitchStack.asm           | MSFT
> @@ -385,6 +386,7 @@ [Sources.AARCH64]
>    AArch64/SetJumpLongJump.asm       | MSFT
>    AArch64/CpuBreakpoint.asm         | MSFT
>    AArch64/SpeculationBarrier.asm    | MSFT
> +  AArch64/ArmReadCntPctReg.asm      | MSFT
>  
>  [Sources.RISCV64]
>    Math64.c
> -- 
> 2.25.1
> 


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