Re: [edk2-devel] [PATCH v2 0/3] New RISC-V Patches

["Leif Lindholm" <leif@nuviainc.com>]

On Fri, May 15, 2020 at 15:39:34 +0200, Daniel Schaefer wrote:
> In this updated version I addressed Leif's comments and made the following
> changes:
> 
> - Refactor sbi_call* to SbiCall* (EDKII style)
> - Use OpenSBI constants if possible
> - Include Base.h in OpensbiTypes.h
> - Only use __builtin_expect with Clang and GCC (not MSVC)
> 
> I'm sorry, I hadn't explained the new branches properly.
> Previously we had all code going to EDK2 via the RISC-V-V2 branch.
> 
> Now we're only making the least amount of necessary changes in edk2 and
> everything else in edk2-platforms.
> Those changes to edk2 can be grouped into different categories:
> 
> - Patches for RISC-V EDK2 CI enablement
> - Patches for edk2 modules other than RISC-V ones, to allow building them with the RISC-V toolchain
> - Other RISC-V enablement like PE/COFF relocation
> 
> Those have all been reviewed and merged to edk2 master.
> 
> Previously we had two packages just for RISC-V on our edk2 branch:
>   RiscVPkg and RiscVPlatformPkg
> They are now under
>   Platform/RISC-V/PlatformPkg and Silicon/RISC-V/ProcessorPkg
> in edk2-platforms.

Understood. I took my eye off the ball there for a while, but I'm a
bit confused as to why RiscVPkg isn't going into EDK2. That is very
counterintuitive. And clearly it will need revisiting if we are to add
first-class CI checks like those we do with OvmfPkg/ArmVirtPkg.

> You, Leif, have previously reviewed those. In addition to this old code, which
> was moved, we need some more patches to allow running PEI in S-Mode and
> building in edk2-platforms. That's what this patch series is about.

I *did* have some outstanding comments specifically with regards to
large amounts of code duplication between the SMBIOS implementation of
some closely related RISC-V platforms. That now needs to be revisited.

> In the previous version of this patchseries I forgot to attach the biggest new
> commit, which adds RiscVEdk2SbiLib. It wraps the ecall interface for calling
> SBI in a C API and lets PEI and DXE call SBI interfaces. Because we need more
> M-Mode capabilities in PEI and DXE than SBI gives us, we register another SBI
> extension, that gives us access to the mscratch register.

Without looking at it yet, it sounds like that may resolve the only
remaining major issue I had with RiscVPkg.

> I hope now it makes more sense.

It is more clear, as per above I am not sure it makes more sense :)
Thanks!

Best Regards,

Leif

> - Daniel
> 
> 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>
> 
> Abner Chang (1):
>   ProcessorPkg/Library: Add RiscVOpensbiLib
> 
> Daniel Schaefer (2):
>   ProcessorPkg/RiscVOpensbLib: Add opensbi submodule
>   ProcessorPkg/Library: Add RiscVEdk2SbiLib
> 
>  Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.inf |  28 +
>  Silicon/RISC-V/ProcessorPkg/Library/RiscVOpensbiLib/RiscVOpensbiLib.inf |  60 ++
>  Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h     |  72 ++
>  Silicon/RISC-V/ProcessorPkg/Include/Library/RiscVEdk2SbiLib.h           | 631 ++++++++++++++++
>  Silicon/RISC-V/ProcessorPkg/Include/OpensbiTypes.h                      |  73 ++
>  Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.c   | 789 ++++++++++++++++++++
>  .gitmodules                                                             |   3 +
>  Readme.md                                                               |  36 +
>  Silicon/RISC-V/ProcessorPkg/Library/RiscVOpensbiLib/opensbi             |   1 +
>  9 files changed, 1693 insertions(+)
>  create mode 100644 Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.inf
>  create mode 100644 Silicon/RISC-V/ProcessorPkg/Library/RiscVOpensbiLib/RiscVOpensbiLib.inf
>  create mode 100644 Silicon/RISC-V/ProcessorPkg/Include/IndustryStandard/RiscVOpensbi.h
>  create mode 100644 Silicon/RISC-V/ProcessorPkg/Include/Library/RiscVEdk2SbiLib.h
>  create mode 100644 Silicon/RISC-V/ProcessorPkg/Include/OpensbiTypes.h
>  create mode 100644 Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.c
>  create mode 100644 .gitmodules
>  create mode 160000 Silicon/RISC-V/ProcessorPkg/Library/RiscVOpensbiLib/opensbi
> 
> -- 
> 2.26.1
> 
> 
> 
>