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

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

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