On 5/12/20 1:17 AM, Leif Lindholm wrote:
On Mon, May 11, 2020 at 19:20:15 +0200, Daniel Schaefer wrote:
EDK2 RISC-V OpenSBI library which pull in external source files under
RISC-V/ProcessorPkg/Library/RiscVOpensbiLib/opensbi to the build process.

Signed-off-by: Daniel Schaefer <daniel.schae...@hpe.com>
Co-authored-by: Abner Chang <abner.ch...@hpe.com>
Co-authored-by: Gilbert Chen <gilbert.c...@hpe.com>
Reviewed-by: Leif Lindholm <l...@nuviainc.com>

Cc: Leif Lindholm <leif.lindh...@nuviainc.com>
Cc: Gilbert Chen <gilbert.c...@hpe.com>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
---
  Silicon/RISC-V/ProcessorPkg/Library/RiscVOpensbiLib/RiscVOpensbiLib.inf | 60 
+++++++++++++++
  Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h     | 79 
++++++++++++++++++++
  Silicon/RISC-V/ProcessorPkg/Include/OpensbiTypes.h                      | 66 
++++++++++++++++
  3 files changed, 205 insertions(+)

diff --git 
a/Silicon/RISC-V/ProcessorPkg/Library/RiscVOpensbiLib/RiscVOpensbiLib.inf 
b/Silicon/RISC-V/ProcessorPkg/Library/RiscVOpensbiLib/RiscVOpensbiLib.inf
new file mode 100644
index 000000000000..59dbd67d8e03
--- /dev/null
+++ b/Silicon/RISC-V/ProcessorPkg/Library/RiscVOpensbiLib/RiscVOpensbiLib.inf
@@ -0,0 +1,60 @@
+## @file
+# RISC-V Opensbi Library Instance.
+#
+#  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      = RiscVOpensbiLib
+  FILE_GUID      = 6EF0C812-66F6-11E9-93CE-3F5D5F0DF0A7
+  MODULE_TYPE    = BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = RiscVOpensbiLib
+
+[Sources]
+  opensbi/lib/sbi/riscv_asm.c
+  opensbi/lib/sbi/riscv_atomic.c
+  opensbi/lib/sbi/riscv_hardfp.S
+  opensbi/lib/sbi/riscv_locks.c
+  opensbi/lib/sbi/sbi_console.c
+  opensbi/lib/sbi/sbi_ecall.c
+  opensbi/lib/sbi/sbi_ecall_vendor.c
+  opensbi/lib/sbi/sbi_ecall_replace.c
+  opensbi/lib/sbi/sbi_ecall_legacy.c
+  opensbi/lib/sbi/sbi_ecall_base.c
+  opensbi/lib/sbi/sbi_emulate_csr.c
+  opensbi/lib/sbi/sbi_fifo.c
+  opensbi/lib/sbi/sbi_hart.c
+  opensbi/lib/sbi/sbi_hfence.S
+  opensbi/lib/sbi/sbi_illegal_insn.c
+  opensbi/lib/sbi/sbi_init.c
+  opensbi/lib/sbi/sbi_ipi.c
+  opensbi/lib/sbi/sbi_misaligned_ldst.c
+  opensbi/lib/sbi/sbi_scratch.c
+  opensbi/lib/sbi/sbi_string.c
+  opensbi/lib/sbi/sbi_system.c
+  opensbi/lib/sbi/sbi_timer.c
+  opensbi/lib/sbi/sbi_tlb.c
+  opensbi/lib/sbi/sbi_trap.c
+  opensbi/lib/sbi/sbi_unpriv.c
+  opensbi/lib/utils/sys/clint.c
+  opensbi/lib/utils/irqchip/plic.c
+  opensbi/lib/utils/serial/sifive-uart.c
+  opensbi/lib/utils/serial/uart8250.c
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec   # For libfdt.
+  MdePkg/MdePkg.dec
+  Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  PcdLib
   > +  RiscVCpuLib

Did you want to leave a comment here?
It does seem like this Lib is not needed. Why did you add it, Abner?
Is PcdLib necessary?

+
+
+
diff --git 
a/Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h 
b/Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h
new file mode 100644
index 000000000000..c5c0bd6d9b01
--- /dev/null
+++ b/Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h
@@ -0,0 +1,79 @@
+/** @file
+  SBI inline function calls.
+
+  Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights 
reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#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/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

These #defines are translations of what exists in
opensbi/include/sbi/sbi_error.h - can we use the definitions there
directly (redefining them here)?
Yes, you're right. One is improperly defined in OpenSBI but I'll use the other ones here.

+
+#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
+
+//
+// Below two definitions should be defined in OpenSBI.
+//
+#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
+
+#define RISC_V_MAX_HART_SUPPORTED 16
+
+typedef
+VOID
+(EFIAPI *RISCV_HART_SWITCH_MODE)(
+  IN  UINTN   FuncArg0,
+  IN  UINTN   FuncArg1,
+  IN  UINTN   NextAddr,
+  IN  UINTN   NextMode,
+  IN  BOOLEAN NextVirt
+  );
+
+//
+// Keep the structure member in 64-bit alignment.
+//
+typedef struct {
+    UINT64                 IsaExtensionSupported;  // The ISA extension this 
core supported.
+    RISCV_UINT128          MachineVendorId;        // Machine vendor ID
+    RISCV_UINT128          MachineArchId;          // Machine Architecture ID
+    RISCV_UINT128          MachineImplId;          // Machine Implementation ID
+    RISCV_HART_SWITCH_MODE HartSwitchMode;         // OpenSBI's function to 
switch the mode of a hart
+} EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC;
+#define FIRMWARE_CONTEXT_HART_SPECIFIC_SIZE  (64 * 8) // This is the size of 
EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC
+                                                      // structure. Referred 
by both C code and assembly code.
+
+typedef struct {
+  VOID            *PeiServiceTable;       // PEI Service table
+  EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC  
*HartSpecific[RISC_V_MAX_HART_SUPPORTED];
+} EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT;
+
+#endif
diff --git a/Silicon/RISC-V/ProcessorPkg/Include/OpensbiTypes.h 
b/Silicon/RISC-V/ProcessorPkg/Include/OpensbiTypes.h
new file mode 100644
index 000000000000..3964bf00ea88
--- /dev/null
+++ b/Silicon/RISC-V/ProcessorPkg/Include/OpensbiTypes.h
@@ -0,0 +1,66 @@
+/** @file
+  RISC-V OpesbSBI header file reference.
+
+  Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights 
reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#ifndef EDK2_SBI_TYPES_H_
+#define EDK2_SBI_TYPES_H_
+
I think this header needs to include either Base.h or ProcessorBind.h?

I am also somewhat confused by this file existing both here and in
edk2-staging RISC-V-V2. Am I on the wrong timeline?

Yes, that's a good idea. I'll include Base.h

We have abandoned the branch RISC-V-V2 on edk2-staging. Like mentioned in the cover letter, we only merged the bare minimum to edk2 and will include all of the remaining code in edk2-platforms.
Thus this file will only remain here.


+typedef INT8    s8;
+typedef UINT8   u8;
+typedef UINT8   uint8_t;
+
+typedef INT16   s16;
+typedef UINT16  u16;
+typedef INT16   int16_t;
+typedef UINT16  uint16_t;
+
+typedef INT32   s32;
+typedef UINT32  u32;
+typedef INT32   int32_t;
+typedef UINT32  uint32_t;
+
+typedef INT64   s64;
+typedef UINT64  u64;
+typedef INT64   int64_t;
+typedef UINT64  uint64_t;
+
+#define PRILX   "016lx"
Please add a comment about what PRILX is for, given this isn't
something I can even grep for in this repo. Maybe consider also moving
it after all the typedefs, so it isn't as undexpected?
Yes, we don't use it currently but we need to define it, because in the OpenSBI header it's guarded by #ifndef OPENSBI_EXTERNAL_SBI_TYPES, which we define to override
their types.

+
+typedef BOOLEAN  bool;
+typedef unsigned long   ulong;
+typedef UINT64   uintptr_t;
+typedef UINT64   size_t;
+typedef INT64    ssize_t;
+typedef UINT64   virtual_addr_t;
+typedef UINT64   virtual_size_t;
+typedef UINT64   physical_addr_t;
+typedef UINT64   physical_size_t;
+
+#define __packed        __attribute__((packed))
+#define __noreturn      __attribute__((noreturn))
+
+#define likely(x) __builtin_expect((x), 1)
+#define unlikely(x) __builtin_expect((x), 0)
It may be premature to start worrying about support in non-gcc-like
toolchains, but if that *does* happen, it might be nicer if it fails
with a clear error message about "unsupported toolchain" (Base.h uses
   #if defined(__GNUC__) || defined(__clang__)
).
Good catch!
However it seems that both GCC and Clang support this builtin

/
     Leif
Thanks for the review. I'll send an updated patch.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59681): https://edk2.groups.io/g/devel/message/59681
Mute This Topic: https://groups.io/mt/74140933/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to