Re: [edk2-devel] [PATCH v2 3/3] ProcessorPkg/Library: Add RiscVEdk2SbiLib

["Daniel Schaefer" <daniel.schaefer@hpe.com>]

Hi Leif,

thanks for this super careful review!
Comments and one question inline.

- Daniel

On 5/20/20 8:27 PM, Leif Lindholm wrote:
> On Fri, May 15, 2020 at 15:39:37 +0200, Daniel Schaefer wrote:
>> Library provides interfaces to invoke SBI extensions.
>>
>> Signed-off-by: Daniel Schaefer <daniel.schaefer@hpe.com>
>>
>> Cc: Abner Chang <abner.chang@hpe.com>
>> Cc: Gilbert Chen <gilbert.chen@hpe.com>
>> Cc: Michael D Kinney <michael.k.kinney@intel.com>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>> ---
>>   Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.inf |  28 +
>>   Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h     |  43 +-
>>   Silicon/RISC-V/ProcessorPkg/Include/Library/RiscVEdk2SbiLib.h           | 631 ++++++++++++++++
>>   Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.c   | 789 ++++++++++++++++++++
>>   4 files changed, 1466 insertions(+), 25 deletions(-)
>>
>> diff --git a/Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.inf b/Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.inf
>> new file mode 100644
>> index 000000000000..665dcbf40e01
>> --- /dev/null
>> +++ b/Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.inf
>> @@ -0,0 +1,28 @@
>> +## @file
>> +# RISC-V Library to call SBI ecalls
>> +#
>> +#  Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>> +#
>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION    = 0x0001001b
>> +  BASE_NAME      = RiscVEdk2SbiLib
>> +  FILE_GUID      = 0DF1BBBD-F7E5-4E8A-BCF1-9D63D2DD9FDD
>> +  MODULE_TYPE    = BASE
>> +  VERSION_STRING = 1.0
>> +  LIBRARY_CLASS  = RiscVEdk2SbiLib
>> +
>> +[Sources]
>> +  RiscVEdk2SbiLib.c
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +  Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dec
>> +  Platform/RISC-V/PlatformPkg/RiscVPlatformPkg.dec
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  RiscVOpensbiLib
>> diff --git a/Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h b/Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h
>> index c5c0bd6d9b01..18a85e2238d2 100644
>> --- a/Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h
>> +++ b/Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h
>> @@ -10,42 +10,35 @@
>>   #ifndef EDK2_SBI_H_
>>   #define EDK2_SBI_H_
>>   
>> -#include <include/sbi/riscv_asm.h> // Reference to header file in opensbi
>>   #include <RiscVImpl.h>
>> +#include <sbi/riscv_asm.h> // Reference to header file in opensbi
> 
> I don't see anything in *this* patch requiring this change - does this
> belong squashed into 2/3?

Yes... I'll fix it in the next patchset.

> 
>> +#include <sbi/sbi_ecall_interface.h>
>> +#include <sbi/sbi_error.h>
>>   #include <sbi/sbi_types.h>  // Reference to header file wrapper
>>   
>> -#define SBI_SUCCESS                    0
>> -#define SBI_ERR_FAILED                -1
>> -#define SBI_ERR_NOT_SUPPORTED         -2
>> -#define SBI_ERR_INVALID_PARAM         -3
>> -#define SBI_ERR_DENIED                -4
>> -#define SBI_ERR_INVALID_ADDRESS       -5
>> -#define SBI_ERR_ALREADY_AVAILABLE     -6
>> +// Translation from OpenSBI constants to SBI names
>> +#define SBI_SUCCESS               SBI_OK
>> +#define SBI_ERR_FAILED            SBI_EFAIL
>> +#define SBI_ERR_NOT_SUPPORTED     SBI_ENOTSUPP
>> +#define SBI_ERR_INVALID_PARAM     SBI_EINVAL
>> +#define SBI_ERR_DENIED            SBI_DENIED
>> +#define SBI_ERR_INVALID_ADDRESS   SBI_INVALID_ADDR
>> +#define SBI_ERR_ALREADY_AVAILABLE -6
> 
> Ah, right, see my confusion from reviewing 2/3.
> Please move this to 2/3, adding a comment on why
> SBI_ERR_ALREADY_AVAILABLE needs to be locally defined here.

See above, comment added.

> 
>>   
>> -#define SBI_BASE_EXT                   0x10
>> -#define SBI_HSM_EXT                    0x48534D
>> -#define SBI_TIME_EXT                   0x54494D45
>> -#define SBI_IPI_EXT                    0x735049
>> -#define SBI_RFNC_EXT                   0x52464E43
> 
> Why do we add these in 2/3 only to delete them again here?

See above.

> 
>> +// Included in OpenSBI 0.7
>> +// Can be removed, once we upgrade
>> +#define SBI_EXT_HSM               0x48534D
>> +#define SBI_EXT_HSM_HART_START    0x0
>> +#define SBI_EXT_HSM_HART_STOP     0x1
>> +#define SBI_EXT_HSM_HART_GET_STATUS 0x2
>>   
>>   //
>>   // Below two definitions should be defined in OpenSBI.
>> +// Submitted to upstream, waiting for merge and release.
> 
> Good call out. This isn't pretty, but it is the right thing to do.
> 
>>   //
>>   #define SBI_EXT_FIRMWARE_CODE_BASE_START 0x0A000000
>>   #define SBI_EXT_FIRMWARE_CODE_BASE_END   0x0AFFFFFF
>>   
>> -#define SBI_GET_SPEC_VERSION_FUNC      0
>> -#define SBI_GET_IMPL_ID_FUNC           1
>> -#define SBI_GET_IMPL_VERSION_FUNC      2
>> -#define SBI_PROBE_EXTENSION_FUNC       3
>> -#define SBI_GET_MVENDORID_FUNC         4
>> -#define SBI_GET_MARCHID_FUNC           5
>> -#define SBI_GET_MIMPID_FUNC            6
>> -
>> -#define SBI_HART_START_FUNC            0
>> -#define SBI_HART_STOP_FUNC             1
>> -#define SBI_HART_GET_STATUS_FUNC       2
>> -
> 
> Why do we add these in 2/3 only to delete them again here?

See above.

> 
>>   #define RISC_V_MAX_HART_SUPPORTED 16
>>   
>>   typedef
>> diff --git a/Silicon/RISC-V/ProcessorPkg/Include/Library/RiscVEdk2SbiLib.h b/Silicon/RISC-V/ProcessorPkg/Include/Library/RiscVEdk2SbiLib.h
>> new file mode 100644
>> index 000000000000..cf77814e3bbc
>> --- /dev/null
>> +++ b/Silicon/RISC-V/ProcessorPkg/Include/Library/RiscVEdk2SbiLib.h
>> @@ -0,0 +1,631 @@
>> +/** @file Defines the PPIs to let PEIMs call SBI
>> +
>> +Copyright (c) 2020, Hewlett Packard Development LP. All rights reserved.<BR>
>> +
>> +SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#ifndef RISCV_SBI_LIB_H_
>> +#define RISCV_SBI_LIB_H_
>> +
>> +#include <Uefi.h>
>> +#include <IndustryStandard/RiscVOpensbi.h>
>> +#include <sbi/sbi_scratch.h>
>> +#include <sbi/sbi_platform.h>
>> +
>> +//
>> +// EDK2 OpenSBI Firmware extension.
>> +//
>> +#define SBI_EDK2_FW_EXT (SBI_EXT_FIRMWARE_CODE_BASE_START | SBI_OPENSBI_IMPID)
>> +//
>> +// EDK2 OpenSBI Firmware extension functions.
>> +//
>> +#define SBI_EXT_FW_MSCRATCH_FUNC        0
>> +#define SBI_EXT_FW_MSCRATCH_HARTID_FUNC 1
>> +
>> +//
>> +// EDK2 OpenSBI firmware extension return status.
>> +//
>> +struct sbiret {
> 
> This struct appears only to be referenceed outside the opensbi
> submodule, so name should conform to EDK2 coding style (and preferably
> with a typedef).

Okidoki.

> 
>> +  long error;   ///< SBI status code
>> +  long value;   ///< Value returned
> 
> This looks like a maintenance hazard (means different things to GCC
> and VS for example). Can we use UINT32?

I'll use UINTN because it's bigger than 32bits on RISCV64

> 
>> +};
>> +
>> +#define SbiCall0(ext_id, func_id) \
>> +  SbiCall(ext_id, func_id, 0, 0, 0, 0, 0, 0)
>> +#define SbiCall1(ext_id, func_id, arg0) \
>> +  SbiCall(ext_id, func_id, arg0, 0, 0, 0, 0, 0)
>> +#define SbiCall2(ext_id, func_id, arg0, arg1) \
>> +  SbiCall(ext_id, func_id, arg0, arg1, 0, 0, 0, 0)
>> +#define SbiCall3(ext_id, func_id, arg0, arg1, arg2) \
>> +  SbiCall(ext_id, func_id, arg0, arg1, arg2, 0, 0, 0)
>> +#define SbiCall4(ext_id, func_id, arg0, arg1, arg2, arg3) \
>> +  SbiCall(ext_id, func_id, arg0, arg1, arg2, arg3, 0, 0)
>> +#define SbiCall5(ext_id, func_id, arg0, arg1, arg2, arg3, arg4) \
>> +  SbiCall(ext_id, func_id, arg0, arg1, arg2, arg3, arg4, 0)
>> +#define SbiCall6(ext_id, func_id, arg0, arg1, arg2, arg3, arg4, arg5) \
>> +  SbiCall(ext_id, func_id, arg0, arg1, arg2, arg3, arg4, arg5)
> 
> Ugh. This looks way too much like pre-EFIAPI x86 code.
> 
> Is this a pattern used across multiple codebases?
> If not, could we make this a simple argc/argv instead and do the
> unpacking inside SbiCall()? Hmm, maybe that would make the call sites
> even worse.
> 
> If we need to keep these, coding style says all macros should use
> UPPERCASE_AND_UNDERSCORES.
> 

Yeah, I think argc/argv is going to make the callsites worse. What about 
vararg, like I used in SbiVendorCall in
Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.c
? Or does that have a performance impact? Maybe an architecture specific 
one?

>> +
>> +/**
>> +  EDK2 SbiCall to invoke SBI extensions.
>> +
>> +  @param[in] ext_id   Sbi extension ID.
>> +  @param[in] func_id  Sbi functions ID.
>> +  @param[in] arg0     Arg0 to function.
>> +  @param[in] arg1     Arg1 to function.
>> +  @param[in] arg2     Arg2 to function.
>> +  @param[in] arg3     Arg3 to function.
>> +  @param[in] arg4     Arg4 to function.
>> +  @param[in] arg5     Arg5 to function.
>> +
>> +  @retval  Returns sbiret structure.
>> +
>> +**/
>> +inline
>> +EFIAPI
>> +struct sbiret SbiCall(UINTN ext_id, UINTN func_id, UINTN arg0, UINTN arg1,
> 
> Function name starts in the first column of a new line.
> But please drop the entire forward-declaration.
> 
>> +                           UINTN arg2, UINTN arg3, UINTN arg4, UINTN arg5)
>> +__attribute__((always_inline));
>> +
>> +/**
>> +  EDK2 SbiCall to invoke SBI extensions.
>> +
>> +  @param[in] ext_id   Sbi extension ID.
>> +  @param[in] func_id  Sbi functions ID.
>> +  @param[in] arg0     Arg0 to function.
>> +  @param[in] arg1     Arg1 to function.
>> +  @param[in] arg2     Arg2 to function.
>> +  @param[in] arg3     Arg3 to function.
>> +  @param[in] arg4     Arg4 to function.
>> +  @param[in] arg5     Arg5 to function.
>> +
>> +  @retval  Returns sbiret structure.
>> +
>> +**/
>> +inline
> 
> Technically, the coding standard bans function definitions in header
> files[1]. If you can give me a good reason for why this function
> should be here, I may consider to consider making an exception.
> If I do, make it just STATIC (let the compiler worry about the
> inlining).

When I first wrote this library, it was necessary but it's not anymore.
I moved it to the .c file, because it's not used anywhere else.

> 
> If it is just by habit from other projects of putting helper functions
> into headers, please move them to a .c file.
> 
> [1] https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/53_include_files#5-3-7-include-files-shall-not-generate-code-or-define-data-variables
> 
>> +EFIAPI
>> +struct sbiret SbiCall(UINTN ext_id, UINTN func_id, UINTN arg0, UINTN arg1,
>> +                           UINTN arg2, UINTN arg3, UINTN arg4, UINTN arg5) {
>> +    register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0);
>> +    register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1);
>> +    register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2);
>> +    register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3);
>> +    register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4);
>> +    register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5);
>> +    register uintptr_t a6 asm ("a6") = (uintptr_t)(func_id);
>> +    register uintptr_t a7 asm ("a7") = (uintptr_t)(ext_id);
> 
> I would *prefer* UINTN over uintptr_t here.
> 
>> +    asm volatile ("ecall" \
>> +         : "+r" (a0) \
> 
> Isn't a1 also an input/output operand here?

Yes, you're right.

> 
>> +         : "r" (a1), "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) \
>> +         : "memory"); \
>> +    struct sbiret ret = { a0, a1 };
>> +    return ret;
> 
> CamelCase naming.

Will change it everywhere.

> 
>> +}
>> +
>> +/**
>> +  Get the implemented SBI specification version
>> +
>> +  The minor number of the SBI specification is encoded in the low 24 bits,
>> +  with the major number encoded in the next 7 bits.  Bit 32 must be 0 and is
>> +  reserved for future expansion.
>> +
>> +  @param[out] SpecVersion          The Version of the SBI specification.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetSpecVersion (
>> +  OUT UINTN                       *SpecVersion
>> +  );
>> +
>> +/**
>> +  Get the SBI implementation ID
>> +
>> +  This ID is used to idenetify a specific SBI implementation in order to work
>> +  around any quirks it might have.
>> +
>> +  @param[out] ImplId               The ID of the SBI implementation.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetImplId (
>> +  OUT UINTN                      *ImplId
>> +  );
>> +
>> +/**
>> +  Get the SBI implementation version
>> +
>> +  The version of this SBI implementation.
>> +  The encoding of this number is determined by the specific SBI implementation.
>> +
>> +  @param[out] ImplVersion          The version of the SBI implementation.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetImplversion (
> 
> Uppercase V for CamelCase (and matching the argument name below).

Oops.

> 
>> +  OUT UINTN                       *ImplVersion
>> +  );
>> +
>> +/**
>> +  Probe whether an SBI extension is available
>> +
>> +  ProbeResult is set to 0 if the extension is not available or to an extension
>> +  specified value if it is available.
>> +
>> +  @param[in]  ExtensionId          The extension ID.
>> +  @param[out] ProbeResult          The return value of the probe.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiProbeExtension (
>> +  IN  INTN                         ExtensionId,
>> +  OUT INTN                        *ProbeResult
>> +  );
>> +
>> +/**
>> +  Get the CPU's vendor ID
>> +
>> +  Reads the mvendorid CSR.
> 
> What is an MvendorId? MachineVendorId?
> Even if an official register name, I would prefer function and
> arguments to be given proper descriptive CamelCase names.

Yes, it's the official register name. Alright, will change it.

> 
>> +
>> +  @param[out] MvendorId            The CPU's vendor ID.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetMvendorId (
>> +  OUT UINTN                       *MvendorId
>> +  );
>> +
>> +/**
>> +  Get the CPU's architecture ID
>> +
>> +  Reads the marchid CSR.
>> +
>> +  @param[out] MarchId              The CPU's architecture ID.
> 
> This should probebly be MArchId (or MachineArchId?)?

Yes, changed it to MachineArchId.

> 
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetMarchId (
>> +  OUT UINTN                       *MarchId
>> +  );
>> +
>> +/**
>> +  Get the CPU's implementation ID
>> +
>> +  Reads the mimpid CSR.
>> +
>> +  @param[out] MimpId               The CPU's implementation ID.
> 
> Above "Impl" is used for "Impelentation". *Strictly* speaking, "Impl"
> doesn't fall in the pretty small group of abbreviations permitted
> without a glossary section in the source file, but it's clear enough
> to me I'll let it slip.
> The same cannot be said for "Imp".
> MachineImplId?

Sounds good. Should it be added to the permitted abbreviations?
If I spell it out fully here, it becomes quite long.

> 
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetMimpId (
>> +  OUT UINTN                       *Mimpid
>> +  );
>> +
>> +/**
>> +  Politely ask the SBI to start a given hart.
> 
> I know hart is a pretty fundamental concept in RISC-V.
> Still, I would request to have it added in a glossary section in the
> top-of-file comment header.

Cool, didn't know that existed! Will do.

> 
>> +
>> +  This call may return before the hart has actually started executing, if the
>> +  SBI implementation can guarantee that the hart is actually going to start.
>> +
>> +  Before the hart jumps to StartAddr, the hart MUST configure PMP if present
>> +  and switch to S-mode.
>> +
>> +  @param[in]  HartId               The id of the hart to start.
>> +  @param[in]  StartAddr            The physical address, where the hart starts
>> +                                   executing from.
>> +  @param[in]  Priv                 An XLEN-bit value, which will be in register
>> +                                   a1 when the hart starts.
>> +  @retval EFI_SUCCESS              Hart was stopped and will start executing from StartAddr.
>> +  @retval EFI_LOAD_ERROR           StartAddr is not valid, possibly due to following reasons:
>> +                                    - It is not a valid physical address.
>> +                                    - The address is prohibited by PMP to run in
>> +                                      supervisor mode.
>> +  @retval EFI_INVALID_PARAMETER    HartId is not a valid hart id
>> +  @retval EFI_ALREADY_STARTED      The hart is already running.
>> +  @retval other                    The start request failed for unknown reasons.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiHartStart (
> 
> (With great effort, I suppress a Mötley Crüe joke.)
> 
>> +  IN  UINTN                          HartId,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Priv
>> +  );
>> +
>> +/**
>> +  Return execution of the calling hart to SBI.
>> +
>> +  MUST be called in S-Mode with user interrupts disabled.
>> +  This call is not expected to return, unless a failure occurs.
>> +
>> +  @retval     EFI_SUCCESS          Never occurs. When successful, the call does not return.
>> +  @retval     other                Failed to stop hard for an unknown reason.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiHartStop (
>> +  );
>> +
>> +/**
>> +  Get the current status of a hart.
>> +
>> +  Since harts can transition between states at any time, the status retrieved
>> +  by this function may already be out of date, once it returns.
>> +
>> +  Possible values for HartStatus are:
>> +  0: STARTED
>> +  1: STOPPED
>> +  2: START_REQUEST_PENDING
>> +  3: STOP_REQUEST_PENDING
>> +
>> +  @param[out] HartStatus           The pointer in which the hart's status is
>> +                                   stored.
>> +  @retval EFI_SUCCESS              The operation succeeds.
>> +  @retval EFI_INVALID_PARAMETER    A parameter is invalid.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiHartGetStatus (
>> +  IN  UINTN                          HartId,
>> +  OUT UINTN                         *HartStatus
>> +  );
>> +
>> +///
>> +/// Timer extension
>> +///
>> +
>> +/**
>> +  Clear pending timer interrupt bit and set timer for next event after StimeValue.
> 
> What does the S stand for in Stime?

That's what they call it in the spec: stime_value.
I guess it stands for supervisor. Should we change it to just `Time`?

> 
>> +
>> +  To clear the timer without scheduling a timer event, set StimeValue to a
>> +  practically infinite value or mask the timer interrupt by clearing sie.STIE.
>> +
>> +  @param[in]  StimeValue           The time offset to the next scheduled timer interrupt.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiSetTimer (
>> +  IN  UINT64                         StimeValue
>> +  );
>> +
>> +///
>> +/// IPI extension
>> +///
>> +
>> +/**
>> +  Send IPI to all harts specified in the mask.
>> +
>> +  The interrupts are registered as supervisor software interrupts at the
>> +  receiving hart.
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiSendIpi (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase
>> +  );
>> +
>> +///
>> +/// Remote fence extension
>> +///
>> +
>> +/**
>> +  Instructs remote harts to execute a FENCE.I instruction.
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteFenceI (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase
>> +  );
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.VMA instructions.
>> +
>> +  The SFENCE.VMA covers the range of virtual addresses between StartAaddr and Size.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteSfenceVma (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size
>> +  );
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.VMA instructions.
>> +
>> +  The SFENCE.VMA covers the range of virtual addresses between StartAaddr and Size.
>> +  Covers only the given ASID.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteSfenceVmaAsid (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size,
>> +  IN  UINTN                          Asid
>> +  );
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.GVMA instructions.
>> +
>> +  The SFENCE.GVMA covers the range of virtual addresses between StartAaddr and Size.
>> +  Covers only the given VMID.
>> +  This function call is only valid for harts implementing the hypervisor extension.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_UNSUPPORTED          SBI does not implement this function or one
>> +                                   of the target harts does not support the
>> +                                   hypervisor extension.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteHfenceGvmaVmid (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size,
>> +  IN  UINTN                          Vmid
>> +  );
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.GVMA instructions.
>> +
>> +  The SFENCE.GVMA covers the range of virtual addresses between StartAaddr and Size.
>> +  This function call is only valid for harts implementing the hypervisor extension.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_UNSUPPORTED          SBI does not implement this function or one
>> +                                   of the target harts does not support the
>> +                                   hypervisor extension.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteHfenceGvma (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size
>> +  );
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.VVMA instructions.
>> +
>> +  The SFENCE.GVMA covers the range of virtual addresses between StartAaddr and Size.
>> +  Covers only the given ASID.
>> +  This function call is only valid for harts implementing the hypervisor extension.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_UNSUPPORTED          SBI does not implement this function or one
>> +                                   of the target harts does not support the
>> +                                   hypervisor extension.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteHfenceVvmaAsid (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size,
>> +  IN  UINTN                          Asid
>> +  );
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.VVMA instructions.
>> +
>> +  The SFENCE.GVMA covers the range of virtual addresses between StartAaddr and Size.
>> +  This function call is only valid for harts implementing the hypervisor extension.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_UNSUPPORTED          SBI does not implement this function or one
>> +                                   of the target harts does not support the
>> +                                   hypervisor extension.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteHfenceVvma (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size
>> +  );
>> +
>> +///
>> +/// Vendor Specific extension space: Extension Ids 0x09000000 through 0x09FFFFFF
>> +///
>> +
>> +/**
>> +  Call a function in a vendor defined SBI extension
>> +
>> +  ASSERT() if the ExtensionId is not in the designated SBI Vendor Extension
>> +  Space.
>> +
>> +  @param[in]  ExtensionId          The SBI vendor extension ID.
>> +  @param[in]  FunctionId           The function ID to call in this extension.
>> +  @param[in]  NumArgs              How many arguments are passed.
>> +  @param[in]  ...                  Actual Arguments to the function.
>> +  @retval EFI_SUCCESS if the SBI function was called and it was successful
>> +  @retval EFI_INVALID_PARAMETER if NumArgs exceeds 6
>> +  @retval others if the called SBI function returns an error
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiVendorCall (
>> +  IN  UINTN                          ExtensionId,
>> +  IN  UINTN                          FunctionId,
>> +  IN  UINTN                          NumArgs,
>> +  ...
>> +  );
>> +
>> +///
>> +/// Firmware SBI Extension
>> +///
>> +/// This SBI Extension is defined and used by EDK2 only in order to be able to
>> +/// run PI and DXE phase in S-Mode.
>> +///
>> +
>> +/**
>> +  Get scratch space of the current hart.
>> +
>> +  Please consider using the wrapper SbiGetFirmwareContext if you only need to
>> +  access the firmware context.
>> +
>> +  @param[out] ScratchSpace         The scratch space pointer.
>> +  @retval EFI_SUCCESS              The operation succeeds.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiGetMscratch (
>> +  OUT struct sbi_scratch             **ScratchSpace
> 
> Could we add a typedef for "struct sbi_scratch" to make the code more
> style compliant?

Yeah, will do. Then I'll also do one for `struct sbi_platform`, which is 
used in the .c file.

> 
>> +  );
>> +
>> +/**
>> +  Get scratch space of the given hart id.
>> +
>> +  @param[in]  HartId               The hart id.
>> +  @param[out] ScratchSpace         The scratch space pointer.
>> +  @retval EFI_SUCCESS              The operation succeeds.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiGetMscratchHartid (
>> +  IN  UINTN                            HartId,
>> +  OUT struct sbi_scratch             **ScratchSpace
>> +  );
>> +
>> +/**
>> +  Get firmware context of the calling hart.
>> +
>> +  @param[out] FirmwareContext      The firmware context pointer.
>> +  @retval EFI_SUCCESS              The operation succeeds.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiGetFirmwareContext (
>> +  OUT EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT **FirmwareContext
>> +  );
>> +
>> +/**
>> +  Set firmware context of the calling hart.
>> +
>> +  @param[in] FirmwareContext       The firmware context pointer.
>> +  @retval EFI_SUCCESS              The operation succeeds.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiSetFirmwareContext (
>> +  IN EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *FirmwareContext
>> +  );
>> +
>> +#endif
>> diff --git a/Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.c b/Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.c
>> new file mode 100644
>> index 000000000000..bbe006a78af8
>> --- /dev/null
>> +++ b/Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.c
>> @@ -0,0 +1,789 @@
>> +/** @file
>> +  Instance of the SBI ecall library.
>> +
>> +  It allows calling an SBI function via an ecall from S-Mode.
>> +
>> +  The legacy extensions are not included because they are not necessary.
>> +  They would be:
>> +  - SbiLegacySetTimer            -> Use SbiSetTimer
>> +  - SbiLegacyConsolePutChar      -> No replacement - Use regular UEFI functions
>> +  - SbiLegacyConsoleGetChar      -> No replacement - Use regular UEFI functions
>> +  - SbiLegacyClearIpi            -> Write 0 to SSIP
>> +  - SbiLegacySendIpi             -> Use SbiSendIpi
>> +  - SbiLegacyRemoteFenceI        -> Use SbiRemoteFenceI
>> +  - SbiLegacyRemoteSfenceVma     -> Use SbiRemoteSfenceVma
>> +  - SbiLegacyRemoteSfenceVmaAsid -> Use SbiRemoteSfenceVmaAsid
>> +  - SbiLegacyShutdown            -> Wait for new System Reset extension
>> +
>> +  Copyright (c) 2020, Hewlett Packard Development LP. All rights reserved.<BR>
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +**/
>> +
>> +#include <IndustryStandard/RiscVOpensbi.h>
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/RiscVEdk2SbiLib.h>
>> +#include <sbi/riscv_asm.h>
>> +#include <sbi/sbi_hart.h>
>> +#include <sbi/sbi_types.h>
>> +#include <sbi/sbi_platform.h>
>> +#include <sbi/sbi_init.h>
>> +
>> +/**
>> +  Translate SBI error code to EFI status.
>> +
>> +  @param[in] SbiError   SBI error code
>> +  @retval EFI_STATUS
>> +**/
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +TranslateError(
>> +  IN UINTN SbiError
>> +  ) {
>> +  switch (SbiError) {
>> +    case SBI_SUCCESS:
>> +      return EFI_SUCCESS;
>> +    case SBI_ERR_FAILED:
>> +      return EFI_DEVICE_ERROR;
>> +      break;
>> +    case SBI_ERR_NOT_SUPPORTED:
>> +      return EFI_UNSUPPORTED;
>> +      break;
>> +    case SBI_ERR_INVALID_PARAM:
>> +      return EFI_INVALID_PARAMETER;
>> +      break;
>> +    case SBI_ERR_DENIED:
>> +      return EFI_ACCESS_DENIED;
>> +      break;
>> +    case SBI_ERR_INVALID_ADDRESS:
>> +      return EFI_LOAD_ERROR;
>> +      break;
>> +    case SBI_ERR_ALREADY_AVAILABLE:
>> +      return EFI_ALREADY_STARTED;
>> +      break;
>> +    default:
>> +      //
>> +      // Reaches here only if SBI has defined a new error type
>> +      //
>> +      ASSERT (FALSE);
>> +      return EFI_UNSUPPORTED;
>> +      break;
>> +  }
>> +}
>> +
>> +//
>> +// OpenSBI libraary interface function for the base extension
>> +//
>> +
>> +/**
>> +  Get the implemented SBI specification version
>> +
>> +  The minor number of the SBI specification is encoded in the low 24 bits,
>> +  with the major number encoded in the next 7 bits.  Bit 32 must be 0 and is
>> +  reserved for future expansion.
>> +
>> +  @param[out] SpecVersion          The Version of the SBI specification.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetSpecVersion (
>> +  OUT UINTN                       *SpecVersion
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall0 (SBI_EXT_BASE, SBI_EXT_BASE_GET_SPEC_VERSION);
>> +
>> +  if (!ret.error) {
>> +    *SpecVersion = (UINTN) ret.value;
>> +  }
>> +
>> +  //return TranslateError(ret.error);
>> +}
>> +
>> +/**
>> +  Get the SBI implementation ID
>> +
>> +  This ID is used to idenetify a specific SBI implementation in order to work
>> +  around any quirks it might have.
>> +
>> +  @param[out] ImplId               The ID of the SBI implementation.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetImplId (
>> +  OUT UINTN                       *ImplId
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall0 (SBI_EXT_BASE, SBI_EXT_BASE_GET_IMP_ID);
>> +  *ImplId = (UINTN) ret.value;
>> +}
>> +
>> +/**
>> +  Get the SBI implementation version
>> +
>> +  The version of this SBI implementation.
>> +  The encoding of this number is determined by the specific SBI implementation.
>> +
>> +  @param[out] ImplVersion          The version of the SBI implementation.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetImplVersion (
>> +  OUT UINTN                       *ImplVersion
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall0 (SBI_EXT_BASE, SBI_EXT_BASE_GET_IMP_VERSION);
>> +  *ImplVersion = (UINTN) ret.value;
>> +}
>> +
>> +/**
>> +  Probe whether an SBI extension is available
>> +
>> +  ProbeResult is set to 0 if the extension is not available or to an extension
>> +  specified value if it is available.
>> +
>> +  @param[in]  ExtensionId          The extension ID.
>> +  @param[out] ProbeResult          The return value of the probe.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiProbeExtension (
>> +  IN  INTN                         ExtensionId,
>> +  OUT INTN                        *ProbeResult
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall0 (SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT);
>> +  *ProbeResult = (UINTN) ret.value;
>> +}
>> +
>> +/**
>> +  Get the CPU's vendor ID
>> +
>> +  Reads the mvendorid CSR.
>> +
>> +  @param[out] MvendorId            The CPU's vendor ID.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetMvendorId (
>> +  OUT UINTN                       *MvendorId
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall0 (SBI_EXT_BASE, SBI_EXT_BASE_GET_MVENDORID);
>> +  *MvendorId = (UINTN) ret.value;
>> +}
>> +
>> +/**
>> +  Get the CPU's vendor ID
>> +
>> +  Reads the mvendorid CSR.
>> +
>> +  @param[out] MvendorId            The CPU's vendor ID.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetMarchId (
>> +  OUT UINTN                       *MarchId
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall0 (SBI_EXT_BASE, SBI_EXT_BASE_GET_MARCHID);
>> +  *MarchId = (UINTN) ret.value;
>> +}
>> +
>> +/**
>> +  Get the CPU's architecture ID
>> +
>> +  Reads the marchid CSR.
>> +
>> +  @param[out] MarchId              The CPU's architecture ID.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiGetMimpId (
>> +  OUT UINTN                       *MimpId
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall0 (SBI_EXT_BASE, SBI_EXT_BASE_GET_MIMPID);
>> +  *MimpId = (UINTN) ret.value;
>> +}
>> +
>> +//
>> +// SBI interface function for the hart state management extension
>> +//
>> +
>> +/**
>> +  Politely ask the SBI to start a given hart.
>> +
>> +  This call may return before the hart has actually started executing, if the
>> +  SBI implementation can guarantee that the hart is actually going to start.
>> +
>> +  Before the hart jumps to StartAddr, the hart MUST configure PMP if present
>> +  and switch to S-mode.
>> +
>> +  @param[in]  HartId               The id of the hart to start.
>> +  @param[in]  StartAddr            The physical address, where the hart starts
>> +                                   executing from.
>> +  @param[in]  Priv                 An XLEN-bit value, which will be in register
>> +                                   a1 when the hart starts.
>> +  @retval EFI_SUCCESS              Hart was stopped and will start executing from StartAddr.
>> +  @retval EFI_LOAD_ERROR           StartAddr is not valid, possibly due to following reasons:
>> +                                     - It is not a valid physical address.
>> +                                     - The address is prohibited by PMP to run in
>> +                                       supervisor mode.
>> +  @retval EFI_INVALID_PARAMETER    HartId is not a valid hart id
>> +  @retval EFI_ALREADY_STARTED      The hart is already running.
>> +  @retval other                    The start request failed for unknown reasons.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiHartStart (
>> +  IN  UINTN                          HartId,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Priv
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall3 (SBI_EXT_HSM,
>> +                                SBI_EXT_HSM_HART_START,
>> +                                HartId,
>> +                                StartAddr,
>> +                                Priv);
>> +  return TranslateError(ret.error);
>> +}
>> +
>> +/**
>> +  Return execution of the calling hart to SBI.
>> +
>> +  MUST be called in S-Mode with user interrupts disabled.
>> +  This call is not expected to return, unless a failure occurs.
>> +
>> +  @retval     EFI_SUCCESS          Never occurs. When successful, the call does not return.
>> +  @retval     other                Failed to stop hard for an unknown reason.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiHartStop (
>> +  )
>> +{
>> +  struct sbiret Ret = SbiCall0 (SBI_EXT_HSM, SBI_EXT_HSM_HART_STOP);
>> +  return TranslateError(Ret.error);
>> +}
>> +
>> +/**
>> +  Get the current status of a hart.
>> +
>> +  Since harts can transition between states at any time, the status retrieved
>> +  by this function may already be out of date, once it returns.
>> +
>> +  Possible values for HartStatus are:
>> +  0: STARTED
>> +  1: STOPPED
>> +  2: START_REQUEST_PENDING
>> +  3: STOP_REQUEST_PENDING
>> +
>> +  @param[out] HartStatus           The pointer in which the hart's status is
>> +                                   stored.
>> +  @retval EFI_SUCCESS              The operation succeeds.
>> +  @retval EFI_INVALID_PARAMETER    A parameter is invalid.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiHartGetStatus (
>> +  IN  UINTN                          HartId,
>> +  OUT UINTN                         *HartStatus
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall1 (SBI_EXT_HSM, SBI_EXT_HSM_HART_GET_STATUS, HartId);
>> +
>> +  if (!ret.error) {
>> +    *HartStatus = (UINTN) ret.value;
>> +  }
>> +
>> +  return TranslateError(ret.error);
>> +}
>> +
>> +/**
>> +  Clear pending timer interrupt bit and set timer for next event after StimeValue.
>> +
>> +  To clear the timer without scheduling a timer event, set StimeValue to a
>> +  practically infinite value or mask the timer interrupt by clearing sie.STIE.
>> +
>> +  @param[in]  StimeValue           The time offset to the next scheduled timer interrupt.
>> +**/
>> +VOID
>> +EFIAPI
>> +SbiSetTimer (
>> +  IN  UINT64                         StimeValue
>> +  )
>> +{
>> +  SbiCall1 (SBI_EXT_TIME, SBI_EXT_TIME_SET_TIMER, StimeValue);
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +SbiSendIpi (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall2 (SBI_EXT_IPI,
>> +                                SBI_EXT_IPI_SEND_IPI,
>> +                                (UINTN) HartMask,
>> +                                HartMaskBase);
>> +  return TranslateError(ret.error);
>> +}
>> +
>> +/**
>> +  Instructs remote harts to execute a FENCE.I instruction.
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteFenceI (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall2 (SBI_EXT_RFENCE,
>> +                                SBI_EXT_RFENCE_REMOTE_FENCE_I,
>> +                                (UINTN) HartMask,
>> +                                HartMaskBase);
>> +  return TranslateError(ret.error);
>> +}
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.VMA instructions.
>> +
>> +  The SFENCE.VMA covers the range of virtual addresses between StartAaddr and Size.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteSfenceVma (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall4 (SBI_EXT_RFENCE,
>> +                                SBI_EXT_RFENCE_REMOTE_SFENCE_VMA,
>> +                                (UINTN) HartMask,
>> +                                HartMaskBase,
>> +                                StartAddr,
>> +                                Size);
>> +  return TranslateError(ret.error);
>> +}
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.VMA instructions.
>> +
>> +  The SFENCE.VMA covers the range of virtual addresses between StartAaddr and Size.
>> +  Covers only the given ASID.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteSfenceVmaAsid (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size,
>> +  IN  UINTN                          Asid
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall5 (SBI_EXT_RFENCE,
>> +                                SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID,
>> +                                (UINTN) HartMask,
>> +                                HartMaskBase,
>> +                                StartAddr,
>> +                                Size,
>> +                                Asid);
>> +  return TranslateError(ret.error);
>> +}
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.GVMA instructions.
>> +
>> +  The SFENCE.GVMA covers the range of virtual addresses between StartAaddr and Size.
>> +  Covers only the given VMID.
>> +  This function call is only valid for harts implementing the hypervisor extension.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_UNSUPPORTED          SBI does not implement this function or one
>> +                                   of the target harts does not support the
>> +                                   hypervisor extension.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteHFenceGvmaVmid (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size,
>> +  IN  UINTN                          Vmid
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall5 (SBI_EXT_RFENCE,
>> +                                SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA,
>> +                                (UINTN) HartMask,
>> +                                HartMaskBase,
>> +                                StartAddr,
>> +                                Size,
>> +                                Vmid);
>> +  return TranslateError(ret.error);
>> +}
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.GVMA instructions.
>> +
>> +  The SFENCE.GVMA covers the range of virtual addresses between StartAaddr and Size.
>> +  This function call is only valid for harts implementing the hypervisor extension.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_UNSUPPORTED          SBI does not implement this function or one
>> +                                   of the target harts does not support the
>> +                                   hypervisor extension.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteHFenceGvma (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall4 (SBI_EXT_RFENCE,
>> +                                SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID,
>> +                                (UINTN) HartMask,
>> +                                HartMaskBase,
>> +                                StartAddr,
>> +                                Size);
>> +  return TranslateError(ret.error);
>> +}
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.VVMA instructions.
>> +
>> +  The SFENCE.GVMA covers the range of virtual addresses between StartAaddr and Size.
>> +  Covers only the given ASID.
>> +  This function call is only valid for harts implementing the hypervisor extension.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_UNSUPPORTED          SBI does not implement this function or one
>> +                                   of the target harts does not support the
>> +                                   hypervisor extension.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteHFenceVvmaAsid (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size,
>> +  IN  UINTN                          Asid
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall5 (SBI_EXT_RFENCE,
>> +                                SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA,
>> +                                (UINTN) HartMask,
>> +                                HartMaskBase,
>> +                                StartAddr,
>> +                                Size,
>> +                                Asid);
>> +  return TranslateError(ret.error);
>> +}
>> +
>> +/**
>> +  Instructs the remote harts to execute one or more SFENCE.VVMA instructions.
>> +
>> +  The SFENCE.GVMA covers the range of virtual addresses between StartAaddr and Size.
>> +  This function call is only valid for harts implementing the hypervisor extension.
>> +
>> +  The remote fence function acts as a full tlb flush if * StartAddr and size
>> +  are both 0 * size is equal to 2^XLEN-1
>> +
>> +  @param[in]  HartMask             Scalar bit-vector containing hart ids
>> +  @param[in]  HartMaskBase         The starting hartid from which the bit-vector
>> +                                   must be computed. If set to -1, HartMask is
>> +                                   ignored and all harts are considered.
>> +  @param[in]  StartAddr            The first address of the affected range.
>> +  @param[in]  Size                 How many addresses are affected.
>> +  @retval EFI_SUCCESS              IPI was sent to all the targeted harts.
>> +  @retval EFI_LOAD_ERROR           StartAddr or Size is not valid.
>> +  @retval EFI_UNSUPPORTED          SBI does not implement this function or one
>> +                                   of the target harts does not support the
>> +                                   hypervisor extension.
>> +  @retval EFI_INVALID_PARAMETER    Either hart_mask_base or any of the hartid
>> +                                   from hart_mask is not valid i.e. either the
>> +                                   hartid is not enabled by the platform or is
>> +                                   not available to the supervisor.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiRemoteHFenceVvma (
>> +  IN  UINTN                         *HartMask,
>> +  IN  UINTN                          HartMaskBase,
>> +  IN  UINTN                          StartAddr,
>> +  IN  UINTN                          Size
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall4 (SBI_EXT_RFENCE,
>> +                                SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID,
>> +                                (UINTN) HartMask,
>> +                                HartMaskBase,
>> +                                StartAddr,
>> +                                Size);
>> +  return TranslateError(ret.error);
>> +}
>> +
>> +//
>> +// SBI interface function for the vendor extension
>> +//
>> +
>> +/**
>> +  Call a function in a vendor defined SBI extension
>> +
>> +  ASSERT() if the ExtensionId is not in the designated SBI Vendor Extension
>> +  Space.
>> +
>> +  @param[in]  ExtensionId          The SBI vendor extension ID.
>> +  @param[in]  FunctionId           The function ID to call in this extension.
>> +  @param[in]  NumArgs              How many arguments are passed.
>> +  @param[in]  ...                  Actual Arguments to the function.
>> +  @retval EFI_SUCCESS if the SBI function was called and it was successful
>> +  @retval EFI_INVALID_PARAMETER if NumArgs exceeds 6
>> +  @retval others if the called SBI function returns an error
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiVendorCall (
>> +  IN  UINTN                          ExtensionId,
>> +  IN  UINTN                          FunctionId,
>> +  IN  UINTN                          NumArgs,
>> +  ...
>> +  )
>> +{
>> +    struct sbiret ret;
>> +    VA_LIST Args;
>> +    VA_START(Args, NumArgs);
>> +
>> +    ASSERT (ExtensionId >= 0x09000000 && ExtensionId <= 0x09FFFFFF);
>> +
>> +    switch (NumArgs) {
>> +      case 0:
>> +        ret = SbiCall0 (ExtensionId, FunctionId);
>> +        break;
>> +      case 1:
>> +        ret = SbiCall1 (ExtensionId, FunctionId, VA_ARG(Args, UINTN));
>> +        break;
>> +      case 2:
>> +        ret = SbiCall2 (ExtensionId, FunctionId, VA_ARG(Args, UINTN), VA_ARG(Args, UINTN));
>> +        break;
>> +      case 3:
>> +        ret = SbiCall3 (ExtensionId, FunctionId, VA_ARG(Args, UINTN), VA_ARG(Args, UINTN), VA_ARG(Args, UINTN));
>> +        break;
>> +      case 4:
>> +        ret = SbiCall4 (ExtensionId, FunctionId, VA_ARG(Args, UINTN), VA_ARG(Args, UINTN), VA_ARG(Args, UINTN), VA_ARG(Args, UINTN));
>> +        break;
>> +      case 5:
>> +        ret = SbiCall5 (ExtensionId, FunctionId, VA_ARG(Args, UINTN), VA_ARG(Args, UINTN), VA_ARG(Args, UINTN), VA_ARG(Args, UINTN), VA_ARG(Args, UINTN));
>> +        break;
>> +      case 6:
>> +        ret = SbiCall6 (ExtensionId, FunctionId, VA_ARG(Args, UINTN), VA_ARG(Args, UINTN), VA_ARG(Args, UINTN), VA_ARG(Args, UINTN), VA_ARG(Args, UINTN), VA_ARG(Args, UINTN));
>> +        break;
>> +      default:
>> +        // Too many args. In theory SBI can handle more arguments when they are
>> +        // passed on the stack but no SBI extension uses this, therefore it's
>> +        // not yet implemented here.
>> +        return EFI_INVALID_PARAMETER;
>> +     }
>> +
>> +    VA_END(Args);
>> +    return TranslateError(ret.error);
>> +}
>> +
>> +//
>> +// SBI Firmware extension
>> +//
>> +
>> +/**
>> +  Get scratch space of the current hart.
>> +
>> +  Please consider using the wrapper SbiGetFirmwareContext if you only need to
>> +  access the firmware context.
>> +
>> +  @param[out] ScratchSpace         The scratch space pointer.
>> +  @retval EFI_SUCCESS              The operation succeeds.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiGetMscratch (
>> +  OUT struct sbi_scratch             **ScratchSpace
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall0 (SBI_EDK2_FW_EXT, SBI_EXT_FW_MSCRATCH_FUNC);
>> +
>> +  if (!ret.error) {
>> +    *ScratchSpace = (struct sbi_scratch *) ret.value;
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> +  Get scratch space of the given hart id.
>> +
>> +  @param[in]  HartId               The hart id.
>> +  @param[out] ScratchSpace         The scratch space pointer.
>> +  @retval EFI_SUCCESS              The operation succeeds.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiGetMscratchHartid (
>> +  IN  UINTN                            HartId,
>> +  OUT struct sbi_scratch             **ScratchSpace
>> +  )
>> +{
>> +  struct sbiret ret = SbiCall1 (SBI_EDK2_FW_EXT,
>> +                                SBI_EXT_FW_MSCRATCH_HARTID_FUNC,
>> +                                HartId);
>> +
>> +  if (!ret.error) {
>> +    *ScratchSpace = (struct sbi_scratch *) ret.value;
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> +  Get firmware context of the calling hart.
>> +
>> +  @param[out] FirmwareContext      The firmware context pointer.
>> +  @retval EFI_SUCCESS              The operation succeeds.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiGetFirmwareContext (
>> +  OUT EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT **FirmwareContext
>> +  )
>> +{
>> +  struct sbi_scratch *ScratchSpace;
>> +  struct sbi_platform *SbiPlatform;
>> +  struct sbiret ret = SbiCall0 (SBI_EDK2_FW_EXT, SBI_EXT_FW_MSCRATCH_FUNC);
>> +
>> +  if (!ret.error) {
>> +    ScratchSpace = (struct sbi_scratch *) ret.value;
>> +    SbiPlatform = (struct sbi_platform *) sbi_platform_ptr(ScratchSpace);
>> +    *FirmwareContext = (EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *) SbiPlatform->firmware_context;
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> +  Set firmware context of the calling hart.
>> +
>> +  @param[in] FirmwareContext       The firmware context pointer.
>> +  @retval EFI_SUCCESS              The operation succeeds.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SbiSetFirmwareContext (
>> +  IN EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *FirmwareContext
>> +  )
>> +{
>> +  struct sbi_scratch *ScratchSpace;
>> +  struct sbi_platform *SbiPlatform;
>> +  struct sbiret ret = SbiCall0 (SBI_EDK2_FW_EXT, SBI_EXT_FW_MSCRATCH_FUNC);
>> +
>> +  if (!ret.error) {
>> +    ScratchSpace = (struct sbi_scratch *) ret.value;
>> +    SbiPlatform = (struct sbi_platform *) sbi_platform_ptr(ScratchSpace);
>> +    SbiPlatform->firmware_context = (long unsigned int) FirmwareContext;
> 
> UINT64?

UINTN for compatibility with 32bits.

> 
> /
>      Leif
> 
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +}
>> -- 
>> 2.26.1
>>
> 
> 
>