> -----Original Message-----
> From: Schaefer, Daniel (DualStudy)
> Sent: Wednesday, June 3, 2020 7:57 PM
> To: Sean Brogan <sean.bro...@microsoft.com>; Kinney, Michael D
> <michael.d.kin...@intel.com>; Chang, Abner (HPS SW/FW Technologist)
> <abner.ch...@hpe.com>; Bret Barkelew <bret.barke...@microsoft.com>;
> Ard Biesheuvel (ard.biesheu...@linaro.org) <ard.biesheu...@linaro.org>;
> Zimmer, Vincent <vincent.zim...@intel.com>; Leif Lindholm
> <l...@nuviainc.com>
> Cc: devel@edk2.groups.io
> Subject: [edk2-devel] Where to put RISC-V packages
>
> Hi all,
>
> (CC edk2 devel ML - shouldn't be a private discussion again)
>
> let me try to summarize the points of each:
>
> - edk2-platforms isn't a less stable edk2 per se
> - edk2-platforms can be used for architectural decisions that are not
> stable yet and therefore different platforms might implement them
> differently
> - Still we'd rather put unstable things in edk2-platforms for now
> - In the future CPU Init shouldn't be totally separate packages but the
> overlapping code should be combined in
> UefiCpuPkg/MdePkg/MdeModulePkg/...
> - This applies to RISC-V, as well as ARM
> - UefiCpuPkg is too x86 focused
> -> needs lots of work and RISC-V shouldn't wait for that
> - RISC-V changes should preferably be integrated into existing packages
> instead of building an entirely separate hierarchy
> -> We have done this. For example DxeIpl is now in edk2 in
> MdeModulePkg.
>
> I hope everyone can agree with this summary.
> I suggest that we do the following:
>
> - Merge all of the RISC-V code to edk2-platforms
Agree. This is the first step after those new changes get "Reviewed-by".
> - Evaluate the packages individually and move them to edk2 one by one,
> or to merge them into existing edk2 packages
> - In the end in edk2-platforms should only remain code for specific
> platforms, like the SiFive Unleashed board. No generic RISC-V code,
> right?
I agree with above. But those architecture-specific drivers may still stay in
their own package.
>
> I'm not experienced enough with EDK2 to evaluate whether packages might
> fit into EDK2 directly. Maybe we should have a call and have a look at each on
> individually? Below I looked into the libraries and drivers of every package
> that we want to upstream with our branch. I describe, what they do and if
> they are platform specific. Then I issue my guess for where it would belong.
> I hope that gives everyone a bit more insight into all of the code we'd like
> to
> merge.
>
> Again: Would it be okay to merge everything as is into edk2-platforms.
> (DxeIpl
> was previously merged into edk2). And then slowly migrate them over into
> edk2?
> Or should be evaluate each package individually now instead of merging
> everything at once.
>
> - Platform/RISC-V/PlatformPkg
> - FirmwareContextProcessorSpecificLib
> Take processors specific data from SMBIOS and put in HOB
> => edk2
> - OpensbiPlatformLibNull
> Null library for platform provided OpenSBI functions.
> Those functions are defined in the SiFive directories
> => edk2
> - PlatformBootManagerLib
> I'm not sure why this is there. It doesn't look RISC-V specific.
> cc: Abner
> => ???
> - PlatformMemoryTestLibNull
> Same as above
> => ???
> - PlatformUpdateProgressLibNull
> Same as above
> => ???
> - RiscVPlatformTempMemoryInitLibNull
> Initialize temporary RAM on stack. Same on every RISC-V CPU
> Depends on platform's PCD values
> => edk2
> - Sec
> SEC Phase, same for every RISC-V CPU
> => edk2
> - Silicon/RISC-V/ProcessorPkg
> - PeiServicesTablePointerLibOpenSbi
> Saving/loading PeiServicesTable in RISC-V scratch space with OpenSBI
> On every RISC-V CPU
> => edk2
> - RiscVCpuLib
> ASM functions to read RISC-V CSRs on every RISC-V CPU
> For UefiCpuPkg?
> => edk2
> - RiscVEdk2SbiLib
> Wrapper library to call the SBI ecalls on every RISC-V CPU
> => edk2
> - RiscVExceptionLib
> CpuExceptionHandlerLib implementation for RISC-V
> Currently only uses CSR accesses
> cc Abner: Will this be platform specific with the CLIC proposals?
> => edk2/edk2-platforms??
> - RiscVOpensbiLib
> OpenSBI library
> Needs to be called in early init on every RISC-V CPU
> => edk2?
> - RiscVPlatformTimerLibNull,RiscVTimerLib
> TimerLib implementation for RISC-V
> Depends on platform specific PCDs only
> => edk2?
> - CpuDxe
> Produces: gEfiCpuArchProtocolGuid
> Would go into UefiCpuPkg
> => edk2
> - SmbiosDxe
> Create SMBIOS entries type 4, type 7 and type 44
> Values need to be set by platform
> => edk2-platforms
Yes, we will have to review above modules and move those to
UefiCpuPkg/MdePkg/MdeModulePkg as many as we can. But I guess some maybe still
have to stay in RiscVPkg under edk2, unless we can move those RISC-V specific
definitions to MdePkg/Include. We can discuss this later and create another
branch as POC code based on edk2/edk2-platform RISC-V port.
>
> All packages under edk2-platforms/{Silicon,Platform}/SiFive/ are not
> included here, as they are obviously platform specific and need to go into
> edk2-platforms.
>
> Thanks,
> Daniel
>
>
>
> On 5/29/20 7:25 AM, Sean Brogan wrote:
> > Sorry was just getting to this. You guys are fast. This is mostly to
> > Daniel’s original email but I think it lines up with Abner’s questions
> > and Mike’s response.
> >
> > Daniel,
> >
> > I think both Mike and Bret said most everything needed but I'll
> > clarify just in case.
> >
> > I don't think the conversations below (Daniels) is accurate for my
> > concerns about RiscV support or my view of edk2-platforms.
> >
> > In my view Edk2 packages are best when they are functionality based.
> > This allows for proper code base layering and makes sure that
> > abstractions are created in a way that will facilitate cross platform
> > core development. For packages that are based on other
> > characteristics, like architecture, dependencies at the package level
> > usually become tangled and brittle which slowly leaks out into the
> > rest of the code base and then makes the entire repository harder to work
> with.
> >
> > For that reason I advocated that the RiscV support be integrated into
> > the appropriate packages and modules. Where there is unique
> > functionality that then needs to be evaluated to determine if its core
> > functionality or platform functionality. Depending on that it would
> > then land in the appropriate edk2 package or something in edk2-platforms.
> >
> > Thanks
> >
> > Sean
> >
> > *From:* Kinney, Michael D <michael.d.kin...@intel.com>
> > *Sent:* Thursday, May 28, 2020 10:18 PM
> > *To:* Chang, Abner (HPS SW/FW Technologist) <abner.ch...@hpe.com>;
> > Bret Barkelew <bret.barke...@microsoft.com>; Schaefer, Daniel
> > (DualStudy) <daniel.schae...@hpe.com>; Sean Brogan
> > <sean.bro...@microsoft.com>; Ard Biesheuvel
> > (ard.biesheu...@linaro.org) <ard.biesheu...@linaro.org>; Zimmer,
> > Vincent <vincent.zim...@intel.com>; Kinney, Michael D
> > <michael.d.kin...@intel.com>
> > *Cc:* Leif Lindholm <l...@nuviainc.com>
> > *Subject:* [EXTERNAL] RE: Re: [edk2-devel] [PATCH v2 0/3] New RISC-V
> > Patches - Why in edk2-platforms
> >
> > Abner,
> >
> > The architectural RISCV CPU content that does not change between RISCV
> > CPU implementations would be a candidate for MdePkg, MdeModulePkg,
> or
> > UefiCpuPkg. We would need to do another review of the content along
> > with the ARM/AARCH64 content to see how best to organize the CPU
> > related content for now and future.
> >
> > RISCV platform content would still need to go into edk2-platforms.
> > Non-architectural RISCV CPU content would also need to go into
> > edk2-platforms.
> >
> > Mike
> >
> > *From:* Chang, Abner (HPS SW/FW Technologist) <abner.ch...@hpe.com
> > <mailto:abner.ch...@hpe.com>>
> > *Sent:* Thursday, May 28, 2020 10:01 PM
> > *To:* Kinney, Michael D <michael.d.kin...@intel.com
> > <mailto:michael.d.kin...@intel.com>>; Bret Barkelew
> > <bret.barke...@microsoft.com <mailto:bret.barke...@microsoft.com>>;
> > Schaefer, Daniel (DualStudy) <daniel.schae...@hpe.com
> > <mailto:daniel.schae...@hpe.com>>; Sean Brogan
> > <sean.bro...@microsoft.com <mailto:sean.bro...@microsoft.com>>;
> Ard
> > Biesheuvel (ard.biesheu...@linaro.org
> > <mailto:ard.biesheu...@linaro.org>) <ard.biesheu...@linaro.org
> > <mailto:ard.biesheu...@linaro.org>>; Zimmer, Vincent
> > <vincent.zim...@intel.com <mailto:vincent.zim...@intel.com>>
> > *Cc:* Leif Lindholm <l...@nuviainc.com <mailto:l...@nuviainc.com>>
> > *Subject:* RE: Re: [edk2-devel] [PATCH v2 0/3] New RISC-V Patches -
> > Why in edk2-platforms
> >
> > *From:* Kinney, Michael D [mailto:michael.d.kin...@intel.com]
> > *Sent:* Friday, May 29, 2020 12:36 PM
> > *To:* Bret Barkelew <bret.barke...@microsoft.com
> > <mailto:bret.barke...@microsoft.com>>; Schaefer, Daniel (DualStudy)
> > <daniel.schae...@hpe.com <mailto:daniel.schae...@hpe.com>>; Sean
> > Brogan <sean.bro...@microsoft.com
> <mailto:sean.bro...@microsoft.com>>;
> > Ard Biesheuvel (ard.biesheu...@linaro.org
> > <mailto:ard.biesheu...@linaro.org>) <ard.biesheu...@linaro.org
> > <mailto:ard.biesheu...@linaro.org>>; Kinney, Michael D
> > <michael.d.kin...@intel.com <mailto:michael.d.kin...@intel.com>>;
> > Zimmer, Vincent <vincent.zim...@intel.com
> > <mailto:vincent.zim...@intel.com>>
> > *Cc:* Leif Lindholm <l...@nuviainc.com <mailto:l...@nuviainc.com>>;
> > Chang, Abner (HPS SW/FW Technologist) <abner.ch...@hpe.com
> > <mailto:abner.ch...@hpe.com>>
> > *Subject:* RE: [EXTERNAL] Re: [edk2-devel] [PATCH v2 0/3] New RISC-V
> > Patches - Why in edk2-platforms
> >
> > We did a community review meeting. I recall Ard and Vincent being
> present.
> >
> > We discuss the CPU execution mode for PEI and DXE and there was a
> > discussion that RISCV has 2 modes and they want flexibility to use
> > both. This choice is not defined in the PI spec yet. We suggested
> > that it could follow the ARM/Thumb model and RISCV could choose a
> > single mode for all PEIMs and DXE drivers and choose to go into the
> > other mode in the module implementation as needed.
> >
> > */[Abner] Yes. we done this as we discussed in the community meeting.
> > Those code are belong to RiscV*pkg and currently lay on
> > devel-riscvplatfoms which Leif is reviewing now./*
> >
> > Given that these fundamental CPU execution mode design choices were
> > still in flux, it did not seem like it was ready for edk2 yet.
> >
> > */[Abner] The current implementation is PEI/DXE are in the same
> > mode./*
> >
> > edk2-staging or an edk2-platforms branch seemed more appropriate until
> > all that was worked out.
> >
> > */[Abner] Seems like that work is done. Maybe I have to review PI/UEFI
> > spec for the necessary changes./*
> >
> > edk2-platforms/master seemed ok as well if different RISCV CPU modes
> > would be used for different platform solutions until a unified
> > approach by the RISCV vendors could be determined Once that was
> > solidified, promoting to edk2 would be possible.
> >
> > */[Abner] this means RicsVPkg and RicsVPlatformPkg eventually will be
> > located in edk2 but not edk2-platforms if above issues are addressed?
> > We don’t have to consider UefiCpuPkg for each arch now? We are good
> > with this though./*
> >
> > Hopefully this clarifies for Leif why there was some resistance to
> > edk2 repo right now. Still on deck for the future.
> >
> > Mike
> >
> > *From:* Bret Barkelew <bret.barke...@microsoft.com
> > <mailto:bret.barke...@microsoft.com>>
> > *Sent:* Thursday, May 28, 2020 8:11 PM
> > *To:* Daniel Schaefer <daniel.schae...@hpe.com
> > <mailto:daniel.schae...@hpe.com>>; Kinney, Michael D
> > <michael.d.kin...@intel.com <mailto:michael.d.kin...@intel.com>>;
> Sean
> > Brogan <sean.bro...@microsoft.com
> <mailto:sean.bro...@microsoft.com>>
> > *Cc:* Leif Lindholm <l...@nuviainc.com <mailto:l...@nuviainc.com>>;
> > Chang, Abner (HPS SW/FW Technologist) <abner.ch...@hpe.com
> > <mailto:abner.ch...@hpe.com>>
> > *Subject:* RE: [EXTERNAL] Re: [edk2-devel] [PATCH v2 0/3] New RISC-V
> > Patches - Why in edk2-platforms
> >
> > I think Sean has his own feedback, but my response to Abner was that
> > that I didn’t like the idea of continuing a pattern of separate
> > packages for significant portions of the CPU init. There have been a
> > few times where it has created unnecessary divergence that made it
> > very difficult to write cross-architecture code. I, personally, wasn’t
> > fighting the code landing in edk2 (except for RiscVPlatform, because
> > it had platform in the name), but I was inquiring to see whether it
> > made more sense to break things up among Mde, UefiCpu, and a couple
> > other packages to encourage adhering to similar patterns and
> > interfaces between all the different architectures. I’ve previously
> > wondered openly whether it made sense to do the same with the Arm
> packages.
> >
> > I’ll be honest about not being super familiar with the RISCV code and
> > it would be easy for me to just shrug and say put it wherever, but I
> > know that architecture mobility/agility is important for us and
> > wouldn’t be surprised if I had to use RISCV in the future, and so
> > wanted to make sure that it was as close to what I was familiar with as
> possible.
> >
> > - Bret
> >
> > *From: *Daniel Schaefer <mailto:daniel.schae...@hpe.com>
> > *Sent: *Thursday, May 28, 2020 8:44 AM
> > *To: *Kinney, Michael D <mailto:michael.d.kin...@intel.com>; Bret
> > Barkelew <mailto:bret.barke...@microsoft.com>; Sean Brogan
> > <mailto:sean.bro...@microsoft.com>
> > *Cc: *Leif Lindholm <mailto:l...@nuviainc.com>; Chang, Abner (HPS
> > SW/FW
> > Technologist) <mailto:abner.ch...@hpe.com>
> > *Subject: *[EXTERNAL] Re: [edk2-devel] [PATCH v2 0/3] New RISC-V
> > Patches
> > - Why in edk2-platforms
> >
> > Hi Mike, Bret and Sean,
> >
> > you have previously expressed concern about merging HPE's RISC-V code
> > into edk2 and suggested merging it into edk2-platforms. Apparently you
> > discussed this with Abner in a private email thread. According to that
> > information I tried to summarize your points in an email on the edk2 ML.
> > I hope I got it right.
> >
> > As you can see, we have followed that suggestion and sent the new
> > patches required for that to the ML.
> >
> > Leif, and I, are still not convinced it's the right choice to not
> > include it in edk2 proper.
> > We would appreciate if you could address the concerns Leif has
> > mentioned in the below email on the public ML.
> >
> > Thanks!
> > Daniel
> >
> > On 5/28/20 1:54 PM, Leif Lindholm wrote:
> > > Hi Abner,
> > >
> > > Sorry, I should have followed up on this sooner.
> > >
> > > On Thu, May 21, 2020 at 06:59:20 +0000, Chang, Abner (HPS SW/FW
> > Technologist) wrote:
> > >>> 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:
> > >
> > > Can you point me to the conversation I have missed?
> > >
> > >>>>> 1. RISC-V is very new and potentially unstable - it's quicker
> > to make >>>>> changes in edk2-platforms.
> > >
> > > I don't see this as a valid argument.
> > > It's not edk2-unstable, it is edk2-platforms.
> > >
> > > edk2-platforms exists because there used to be strong feelings
> > against > holding *real* platforms in edk2, with edk2 being
> > originally intended > only as a code library for IBV/ISV to cherry-pick
> > from.
> > >
> > > But fundamentally, if code is too immature to go into the master >
> > branch of edk2, it is too immature to go into the master branch of >
> > edk2-platforms. If we want edk2-might-be-a-bit-shaky-but-who-cares,
> > > then someone will have to create it.
> > >
> > >>>>> 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.
> > >
> > > Yes it is.
> > > The only thing making it less strict is its contents - platform
> > ports > and device drivers. The changes tend to be self-contained.
> > Where they > are not, they need to be carefully managed.
> > >
> > >>>>> 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.
> > >
> > > I don't think there is any need to tie the two together.
> > > Yes, UefiCpuPkg should be a generic place where not only x86
> > support > can be contained, but the paths for ARM* and RISC-V into
> > there do not > have any interdependencies.
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#60672): https://edk2.groups.io/g/devel/message/60672
Mute This Topic: https://groups.io/mt/74647810/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
