> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Leif Lindholm
> Sent: Friday, November 22, 2019 12:24 AM
> To: Chang, Abner (HPS SW/FW Technologist) <abner.ch...@hpe.com>
> Cc: devel@edk2.groups.io; Chen, Gilbert <gilbert.c...@hpe.com>; Palmer
> Dabbelt <pal...@sifive.com>; Kinney, Michael D
> <michael.d.kin...@intel.com>
> Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v3 03/39]
> RiscVPkg/opensbi: EDK2 RISC-V OpenSBI support
> 
> On Mon, Oct 28, 2019 at 09:58:41 +0800, Abner Chang wrote:
> > Add EDK2 RISC-V OpenSBI header files.
> >
> > Signed-off-by: Abner Chang <abner.ch...@hpe.com>
> >
> > Cc: Leif Lindholm <leif.lindh...@linaro.org>
> > Cc: Gilbert Chen <gilbert.c...@hpe.com>
> > ---
> >  RiscVPkg/Include/sbi/SbiFirmwareContext.h | 33
> ++++++++++++++++++++
> >  RiscVPkg/Include/sbi/sbi.h                | 52
> +++++++++++++++++++++++++++++++
> >  RiscVPkg/Include/sbi/sbi_bits.h           | 17 ++++++++++
> >  RiscVPkg/Include/sbi/sbi_types.h          | 45
> ++++++++++++++++++++++++++
> >  4 files changed, 147 insertions(+)
> >  create mode 100644 RiscVPkg/Include/sbi/SbiFirmwareContext.h
> >  create mode 100644 RiscVPkg/Include/sbi/sbi.h  create mode 100644
> > RiscVPkg/Include/sbi/sbi_bits.h  create mode 100644
> > RiscVPkg/Include/sbi/sbi_types.h
> >
> > diff --git a/RiscVPkg/Include/sbi/SbiFirmwareContext.h
> > b/RiscVPkg/Include/sbi/SbiFirmwareContext.h
> > new file mode 100644
> > index 0000000..c3d3489
> > --- /dev/null
> > +++ b/RiscVPkg/Include/sbi/SbiFirmwareContext.h
> > @@ -0,0 +1,33 @@
> > +/** @file
> > +  RISC-V OpesbSBI Platform Firmware context definition
> > +
> > +  Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All
> > + rights reserved.<BR>
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +#ifndef SBI_FIRMWARE_CONTEXT_H_
> > +#define SBI_FIRMWARE_CONTEXT_H_
> > +
> > +#include <RiscVImpl.h>
> > +
> > +#define RISC_V_MAX_HART_SUPPORTED 16
> > +
> > +//
> > +// 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
> > +} EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC;
> > +
> > +#define FIRMWARE_CONTEXT_HART_SPECIFIC_SIZE  (64 * 7)
> > +
> > +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/RiscVPkg/Include/sbi/sbi.h b/RiscVPkg/Include/sbi/sbi.h
> > new file mode 100644 index 0000000..04e7f18
> > --- /dev/null
> > +++ b/RiscVPkg/Include/sbi/sbi.h
> > @@ -0,0 +1,52 @@
> > +/** @file
> > +  SBI inline function calls.
> > +
> > +  Copyright (c) 2019, 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 <sbi/sbi_types.h>  // Reference to header file wrapper
> > +#include <include/sbi/riscv_asm.h> // Reference to header file in
> > +opensbi
> 
> This whole sbi situation is too much of a hornets' nest, and really does need
> to get sorted before we even push this to -staging.
> (But I do think is is one of the last things we need to address.)
> 
> And I think the starting point to untangle it is to rename the wrapper include
> files in RiscVPkg/Include/sbi (and indeed the directory
> itself) to conform with the TianoCore standard.
> 
> That means:
> 
> RiscVPkg/Include/IndustryStandard/Sbi/Sbi.h
> RiscVPkg/Include/IndustryStandard/Sbi/SbiBits.h
> RiscVPkg/Include/IndustryStandard/Sbi/SbiTypes.h
> 
> Unless all of the wrappers can be combined into a single
> RiscVPkg/Include/IndustryStandard/Sbi.h
> 
> ...and then tested on a case-sensitive filesystem.
> 
> For the record, doing that immediately results in the non-wrapper versions
> being pulled in and then the build failing.

Leif,
We can't change the naming of those header files under RiscVPkg/Include/sbi. We 
build opensbi source code in edk2 and those source files refer to those header 
file. And we are not going to change opensbi code as we discussed before.
The only file we can change is sbi.h -> Sbi.h because this header file is 
created for edk2 RISC-V related drivers.

I think we can make it this way,
Rename sbi.h to RiscVOpensbi.h and move it to under 
RiscVPkg/Include/IndustryStandard, no need "/Sbi" subdirectory.
Keep the naming  of sbi_bits.h and sbi_types.h and still leave those under 
RiscVPkg/Include/sbi. No naming changes in order to build opensbi source file. 
Can't even change "/sbi" subdirectory to "/Sbi". sbi_bits.h and sbi_types.h are 
the binding files for edk2 framework BTW.

> 
> /
>     Leif
> 
> > +
> > +#define SBI_SET_TIMER 0
> > +#define SBI_CONSOLE_PUTCHAR 1
> > +#define SBI_CONSOLE_GETCHAR 2
> > +#define SBI_CLEAR_IPI 3
> > +#define SBI_SEND_IPI 4
> > +#define SBI_REMOTE_FENCE_I 5
> > +#define SBI_REMOTE_SFENCE_VMA 6
> > +#define SBI_REMOTE_SFENCE_VMA_ASID 7
> > +#define SBI_SHUTDOWN 8
> > +
> > +#define SBI_CALL(which, arg0, arg1, arg2) ({ \
> > +    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 a7 asm ("a7") = (uintptr_t)(which); \
> > +    asm volatile ("ecall" \
> > +         : "+r" (a0) \
> > +         : "r" (a1), "r" (a2), "r" (a7) \
> > +         : "memory"); \
> > +        a0; \
> > +})
> > +
> > +#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0) #define
> > +SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0) #define
> > +SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, 0)
> > +
> > +#define sbi_console_putchar(ch)    SBI_CALL_1(SBI_CONSOLE_PUTCHAR,
> ch)
> > +#define sbi_console_getchar()      SBI_CALL_0(SBI_CONSOLE_GETCHAR)
> > +#define sbi_set_timer(stime_value) SBI_CALL_1(SBI_SET_TIMER,
> stime_value)
> > +#define sbi_shutdown()             SBI_CALL_0(SBI_SHUTDOWN)
> > +#define sbi_clear_ipi()            SBI_CALL_0(SBI_CLEAR_IPI)
> > +#define sbi_send_ipi(hart_mask)       SBI_CALL_1(SBI_SEND_IPI,
> hart_mask)
> > +#define sbi_remote_fence_i(hart_mask)
> SBI_CALL_1(SBI_REMOTE_FENCE_I,
> > +hart_mask) #define sbi_remote_sfence_vma(hart_mask, start, size)
> > +SBI_CALL_1(SBI_REMOTE_SFENCE_VMA, hart_mask) #define
> > +sbi_remote_sfence_vma_asid(hart_mask, start, size, asid)
> > +SBI_CALL_1(SBI_REMOTE_SFENCE_VMA_ASID, hart_mask)
> > +
> > +#endif
> > diff --git a/RiscVPkg/Include/sbi/sbi_bits.h
> > b/RiscVPkg/Include/sbi/sbi_bits.h new file mode 100644 index
> > 0000000..c935547
> > --- /dev/null
> > +++ b/RiscVPkg/Include/sbi/sbi_bits.h
> > @@ -0,0 +1,17 @@
> > +/** @file
> > +  RISC-V OpesbSBI header file reference.
> > +
> > +  Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All
> > + rights reserved.<BR>
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +#ifndef EDK2_SBI_BITS_H_
> > +#define EDK2_SBI_BITS_H_
> > +
> > +#undef MAX
> > +#undef MIN
> > +
> > +#include "include/sbi/sbi_bits.h" // Reference to header file in
> > +opensbi
> > +
> > +#endif
> > diff --git a/RiscVPkg/Include/sbi/sbi_types.h
> > b/RiscVPkg/Include/sbi/sbi_types.h
> > new file mode 100644
> > index 0000000..95ee213
> > --- /dev/null
> > +++ b/RiscVPkg/Include/sbi/sbi_types.h
> > @@ -0,0 +1,45 @@
> > +/** @file
> > +  RISC-V OpesbSBI header file reference.
> > +
> > +  Copyright (c) 2019, 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_
> > +
> > +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"
> > +
> > +typedef INT32    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))
> > +#endif
> > --
> > 2.7.4
> >
> 
> 


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

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

Reply via email to