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

["Abner Chang" <abner.chang@hpe.com>]

> -----Original Message-----
> From: Schaefer, Daniel (DualStudy)
> Sent: Thursday, May 21, 2020 12:18 AM
> To: Leif Lindholm <leif@nuviainc.com>; devel@edk2.groups.io
> Cc: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>;
> Chen, Gilbert <gilbert.chen@hpe.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Bret.Barkelew@microsoft.com;
> sean.brogan@microsoft.com
> Subject: Re: [edk2-devel] [PATCH v2 0/3] New RISC-V Patches - Why in edk2-
> platforms
> 
> 
> 
> On 5/20/20 6:07 PM, Daniel Schaefer wrote:
> > please reply here, fixed Mike's email address, sorry...
> >
> > On 5/20/20 6:03 PM, Daniel Schaefer wrote:
> >> On 5/20/20 1:43 PM, Leif Lindholm wrote:
> >>> On Fri, May 15, 2020 at 15:39:34 +0200, Daniel Schaefer wrote:
> >>>> 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.
> >>
> >> Yes, I understand your concern. Personally I'd like it also to be in
> >> EDK2 straight away, however Mike, Bret and Sean have raised valid
> >> concerns:
> >>
> >> 1. RISC-V is very new and potentially unstable - it's quicker to make
> >> changes in edk2-platforms.
> >>
> >> 2. If we define new interfaces and libraries in edk2, we can't remove
> >> them easily because it would be a backwards-incompatible change.
> >> edk2-platforms isn't quite as strict.
> >>
> >> 3. Long-term, I think many agree, we should aim to move much of the
> >> RISC-V code into UefiCpuPkg and OvmfPkg. Mike mentioned that would
> >> need coordination with ARM maintainers because it might make sense to
> >> move their code there as well.
> >>
> >> Maybe Mike, Bret or Sean would like to provide some more comments?
> >>
> >>> 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.
> >>
> >> The SMBIOS code hasn't changed. It has moved to
> >>    Silicon/SiFive/{E51,U54,U54MCCoreplex}/Library/PeiCoreInfoHobLib
> >> You're referring to this library, right?
> >>
> >> They build the SMBIOS entries for a particular processor. Yes, the
> >> values do have a lot of overlap but these files are like
> >> configuration files. They don't do much, they only set the values of the
> properties.
> >>
> >> Currently it is not possible to let the UEFI firmware get this
> >> information from the hardware at runtime, especially now, since we're
> >> running in S-Mode.
> >> To allow that, we created a RISC-V working group to be able to
> >> retrieve all of this information dynamically from the processor
> >> (among other goals). Then the vendor will not have to modify these
> >> files and hardcode the values anymore. Which enables us to create a
> >> single library for all processors.
> >> See: https://github.com/riscv/configuration-structure
> >>
> >> I hope I described everything properly, please correct me otherwise,
> >> Abner.
[Abner]  Yes almost. Thanks Daniel.
One thing I would like to add,
If you take a look on SiFive Core IP https://www.sifive.com/risc-v-core-ip you can see there are different SKUs of RISC-V core. Just take some as example,
S51 - Single core
U54 - Single core
S76 - Single core
U74- single core
U54-MC - Multicore which is 4*U54 cores +1*S51 core
U74-MC - Multicore which is 4*U74 core + 1*S7 core

Those are the combinations of core IP. Silicon vendor can get those core IPs and combine them to the RISC-V processor. To have CoreInfoHobLib libraries for each different core (not multicore) to build up the core capability is reasonable and makes sense. For the multicore, it just pulling the single core CoreInfoHobLib to build up the SMBIOS table for the multicore processor. Those libraries look duplicate in logically, however only one instance of CoreInfoHobLib is built in for multiple identical cores in physically view. Maybe we still can move some identical core into the core-specific library but it is not worthwhile.

Abner

> >>
> >>>
> >>>> 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!
> >>
> >> Your concerns are very valid, however due to the mentioned trade-offs
> >> it might not make sense to address them.
> >>
> >> - Daniel