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

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

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.
> 
>>
>>> 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