Re: [RFC PATCH V2 06/19] MdePkg/BaseLib: RISC-V: Add generic CPU related functions

[Heinrich Schuchardt <heinrich.schuchardt@canonical.com>]

On 9/7/22 13:36, Sunil V L wrote:
> EDK2 in S-mode needs to use SSCRATCH register. Implement functions
> to set/get the SSCRATCH register.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---
>   MdePkg/Library/BaseLib/BaseLib.inf      |  1 +
>   MdePkg/Include/Library/BaseLib.h        | 10 ++++++
>   MdePkg/Library/BaseLib/RiscV64/CpuGen.S | 33 ++++++++++++++++++++
>   3 files changed, 44 insertions(+)
> 
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
> index 6be5be9428f2..5429329e39b0 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -401,6 +401,7 @@ [Sources.RISCV64]
>     RiscV64/RiscVCpuPause.S           | GCC
>     RiscV64/RiscVInterrupt.S          | GCC
>     RiscV64/FlushCache.S              | GCC
> +  RiscV64/CpuGen.S                  | GCC
>   
>   [Packages]
>     MdePkg/MdePkg.dec
> diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
> index a6f9a194ef1c..a742de61a442 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -150,6 +150,16 @@ typedef struct {
>   
>   #define BASE_LIBRARY_JUMP_BUFFER_ALIGNMENT  8
>   
> +VOID
> +RiscVSetSupervisorScratch (
> +  UINT64
> +  );
> +
> +UINT64
> +RiscVGetSupervisorScratch (
> +  VOID
> +  );
> +
>   #endif // defined (MDE_CPU_RISCV64)
>   
>   //
> diff --git a/MdePkg/Library/BaseLib/RiscV64/CpuGen.S b/MdePkg/Library/BaseLib/RiscV64/CpuGen.S
> new file mode 100644
> index 000000000000..d11929cf3233
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/RiscV64/CpuGen.S
> @@ -0,0 +1,33 @@
> +//------------------------------------------------------------------------------
> +//
> +// Generic CPU related functions for RISC-V
> +//
> +// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> +// Copyright (c) 2022, Ventana Micro Systems Inc. All rights reserved.<BR>
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +//------------------------------------------------------------------------------
> +
> +#include <Register/RiscV64/RiscVAsm.h>
> +#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)
> +    csrrw a1, CSR_SSCRATCH, a0
> +    ret

I am wondering why you are reading the old value into a1 while the 
function has a VOID return value.

If you are not interested in reading the old value, the "RISC-V 
Unprivileged ISA Specification" suggests to use the x0 (zero) register 
as read register or just use the CSRW pseudo code.

Best regards1.

Heinrich

> +
> +//
> +// Get Supervisor mode scratch.
> +// @retval a0 : Value in Supervisor mode scratch
> +//
> +ASM_FUNC (RiscVGetSupervisorScratch)
> +    csrr a0, CSR_SSCRATCH
> +    ret