Re: [edk2-staging/RiscV64QemuVirt PATCH V7 02/20] MdePkg/BaseLib: RISC-V: Add few more helper functions

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Sunil,

Just a formatting comment below.

Mike

> -----Original Message-----
> From: Sunil V L <sunilvl@ventanamicro.com>
> Sent: Saturday, January 28, 2023 11:18 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> Daniel Schaefer <git@danielschaefer.me>; Abner Chang <abner.chang@amd.com>
> Subject: [edk2-staging/RiscV64QemuVirt PATCH V7 02/20] MdePkg/BaseLib: RISC-V: Add few more helper functions
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4076
> 
> Few of the basic helper functions required for any
> RISC-V CPU were added in edk2-platforms. To support
> qemu virt, they need to be added in BaseLib.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Daniel Schaefer <git@danielschaefer.me>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Acked-by: Abner Chang <abner.chang@amd.com>
> ---
>  MdePkg/Library/BaseLib/BaseLib.inf              |  3 ++
>  MdePkg/Include/Library/BaseLib.h                | 50 ++++++++++++++++++
>  MdePkg/Library/BaseLib/RiscV64/CpuScratch.S     | 31 ++++++++++++
>  MdePkg/Library/BaseLib/RiscV64/ReadTimer.S      | 23 +++++++++
>  MdePkg/Library/BaseLib/RiscV64/RiscVInterrupt.S | 53 ++++++++++++++++++--
>  MdePkg/Library/BaseLib/RiscV64/RiscVMmu.S       | 23 +++++++++
>  6 files changed, 179 insertions(+), 4 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
> index 9ed46a584a14..3a48492b1a01 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -401,6 +401,9 @@ [Sources.RISCV64]
>    RiscV64/RiscVCpuPause.S           | GCC
>    RiscV64/RiscVInterrupt.S          | GCC
>    RiscV64/FlushCache.S              | GCC
> +  RiscV64/CpuScratch.S              | GCC
> +  RiscV64/ReadTimer.S               | GCC
> +  RiscV64/RiscVMmu.S                | GCC
> 
>  [Sources.LOONGARCH64]
>    Math64.c
> diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
> index f3f59f21c2ea..b4f4e45a1486 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -151,6 +151,56 @@ typedef struct {
> 
>  #define BASE_LIBRARY_JUMP_BUFFER_ALIGNMENT  8
> 
> +VOID
> +  RiscVSetSupervisorScratch (
> +                             UINT64
> +                             );
> +
> +UINT64
> +RiscVGetSupervisorScratch (
> +  VOID
> +  );
> +
> +VOID
> +  RiscVSetSupervisorStvec (
> +                           UINT64
> +                           );
> +
> +UINT64
> +RiscVGetSupervisorStvec (
> +  VOID
> +  );
> +
> +UINT64
> +RiscVGetSupervisorTrapCause (
> +  VOID
> +  );
> +
> +VOID
> +  RiscVSetSupervisorAddressTranslationRegister (
> +                                                UINT64
> +                                                );

Formatting does not look right.

Have you run EDK II uncrustify on this patch series.

> +
> +UINT64
> +RiscVReadTimer (
> +  VOID
> +  );
> +
> +VOID
> +RiscVEnableTimerInterrupt (
> +  VOID
> +  );
> +
> +VOID
> +RiscVDisableTimerInterrupt (
> +  VOID
> +  );
> +
> +VOID
> +RiscVClearPendingTimerInterrupt (
> +  VOID
> +  );
> +
>  #endif // defined (MDE_CPU_RISCV64)
> 
>  #if defined (MDE_CPU_LOONGARCH64)
> diff --git a/MdePkg/Library/BaseLib/RiscV64/CpuScratch.S b/MdePkg/Library/BaseLib/RiscV64/CpuScratch.S
> new file mode 100644
> index 000000000000..5492a500eb5e
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/RiscV64/CpuScratch.S
> @@ -0,0 +1,31 @@
> +//------------------------------------------------------------------------------
> +//
> +// CPU scratch register related functions for RISC-V
> +//
> +// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +//------------------------------------------------------------------------------
> +
> +#include <Register/RiscV64/RiscVImpl.h>
> +
> +.data
> +.align 3
> +.section .text
> +
> +//
> +// Set Supervisor mode scratch.
> +// @param a0 : Value set to Supervisor mode scratch
> +//
> +ASM_FUNC (RiscVSetSupervisorScratch)
> +    csrw CSR_SSCRATCH, a0
> +    ret
> +
> +//
> +// Get Supervisor mode scratch.
> +// @retval a0 : Value in Supervisor mode scratch
> +//
> +ASM_FUNC (RiscVGetSupervisorScratch)
> +    csrr a0, CSR_SSCRATCH
> +    ret
> diff --git a/MdePkg/Library/BaseLib/RiscV64/ReadTimer.S b/MdePkg/Library/BaseLib/RiscV64/ReadTimer.S
> new file mode 100644
> index 000000000000..39a06efa51ef
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/RiscV64/ReadTimer.S
> @@ -0,0 +1,23 @@
> +//------------------------------------------------------------------------------
> +//
> +// Read CPU timer
> +//
> +// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +//------------------------------------------------------------------------------
> +
> +#include <Register/RiscV64/RiscVImpl.h>
> +
> +.data
> +.align 3
> +.section .text
> +
> +//
> +// Read TIME CSR.
> +// @retval a0 : 64-bit timer.
> +//
> +ASM_FUNC (RiscVReadTimer)
> +    csrr a0, CSR_TIME
> +    ret
> diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVInterrupt.S b/MdePkg/Library/BaseLib/RiscV64/RiscVInterrupt.S
> index 87b3468fc7fd..6a1b90a7e45c 100644
> --- a/MdePkg/Library/BaseLib/RiscV64/RiscVInterrupt.S
> +++ b/MdePkg/Library/BaseLib/RiscV64/RiscVInterrupt.S
> @@ -8,13 +8,13 @@
>  //
>  //------------------------------------------------------------------------------
> 
> +#include <Register/RiscV64/RiscVImpl.h>
> +
>  ASM_GLOBAL ASM_PFX(RiscVDisableSupervisorModeInterrupts)
>  ASM_GLOBAL ASM_PFX(RiscVEnableSupervisorModeInterrupt)
>  ASM_GLOBAL ASM_PFX(RiscVGetSupervisorModeInterrupts)
> 
> -#define  SSTATUS_SIE                 0x00000002
> -#define  CSR_SSTATUS                 0x100
> -  #define  SSTATUS_SPP_BIT_POSITION  8
> +#define  SSTATUS_SPP_BIT_POSITION  8
> 
>  //
>  // This routine disables supervisor mode interrupt
> @@ -53,11 +53,56 @@ InTrap:
>    ret
> 
>  //
> +// Set Supervisor mode trap vector.
> +// @param a0 : Value set to Supervisor mode trap vector
> +//
> +ASM_FUNC (RiscVSetSupervisorStvec)
> +    csrrw a1, CSR_STVEC, a0
> +    ret
> +
> +//
> +// Get Supervisor mode trap vector.
> +// @retval a0 : Value in Supervisor mode trap vector
> +//
> +ASM_FUNC (RiscVGetSupervisorStvec)
> +    csrr a0, CSR_STVEC
> +    ret
> +
> +//
> +// Get Supervisor trap cause CSR.
> +//
> +ASM_FUNC (RiscVGetSupervisorTrapCause)
> +    csrrs a0, CSR_SCAUSE, 0
> +    ret
> +//
>  // This routine returns supervisor mode interrupt
>  // status.
>  //
> -ASM_PFX(RiscVGetSupervisorModeInterrupts):
> +ASM_FUNC (RiscVGetSupervisorModeInterrupts)
>    csrr a0, CSR_SSTATUS
>    andi a0, a0, SSTATUS_SIE
>    ret
> 
> +//
> +// This routine disables supervisor mode timer interrupt
> +//
> +ASM_FUNC (RiscVDisableTimerInterrupt)
> +    li   a0, SIP_STIP
> +    csrc CSR_SIE, a0
> +    ret
> +
> +//
> +// This routine enables supervisor mode timer interrupt
> +//
> +ASM_FUNC (RiscVEnableTimerInterrupt)
> +    li    a0, SIP_STIP
> +    csrs CSR_SIE, a0
> +    ret
> +
> +//
> +// This routine clears pending supervisor mode timer interrupt
> +//
> +ASM_FUNC (RiscVClearPendingTimerInterrupt)
> +    li   a0, SIP_STIP
> +    csrc CSR_SIP, a0
> +    ret
> diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVMmu.S b/MdePkg/Library/BaseLib/RiscV64/RiscVMmu.S
> new file mode 100644
> index 000000000000..ac8f92f38aed
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/RiscV64/RiscVMmu.S
> @@ -0,0 +1,23 @@
> +//------------------------------------------------------------------------------
> +//
> +// CPU scratch register related functions for RISC-V
> +//
> +// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +//------------------------------------------------------------------------------
> +
> +#include <Register/RiscV64/RiscVImpl.h>
> +
> +.data
> +.align 3
> +.section .text
> +
> +//
> +// Set Supervisor Address Translation and
> +// Protection Register.
> +//
> +ASM_FUNC (RiscVSetSupervisorAddressTranslationRegister)
> +    csrw  CSR_SATP, a0
> +    ret
> --
> 2.38.0