Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 10/14] U500Pkg/Library: Library instances of U500 platform library

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

> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Friday, October 18, 2019 12:38 AM
> To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> <abner.chang@hpe.com>
> Cc: Chen, Gilbert <gilbert.chen@hpe.com>; Palmer Dabbelt
> <palmer@sifive.com>
> Subject: Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 10/14]
> U500Pkg/Library: Library instances of U500 platform library
> 
> On Thu, Oct 17, 2019 at 04:09:44PM +0000, Abner Chang wrote:
> > > -----Original Message-----
> > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> > > Of Leif Lindholm
> > > Sent: Thursday, October 17, 2019 7:19 PM
> > > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> > > <abner.chang@hpe.com>
> > > Cc: Chen, Gilbert <gilbert.chen@hpe.com>; Palmer Dabbelt
> > > <palmer@sifive.com>
> > > Subject: Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 10/14]
> > > U500Pkg/Library: Library instances of U500 platform library
> > >
> > > On Thu, Oct 17, 2019 at 07:44:47AM +0000, Abner Chang wrote:
> > > > > -----Original Message-----
> > > > > From: Chang, Abner (HPS SW/FW Technologist)
> > > > > Sent: Thursday, October 17, 2019 10:21 AM
> > > > > To: devel@edk2.groups.io; leif.lindholm@linaro.org; Chen,
> > > > > Gilbert <gilbert.chen@hpe.com>
> > > > > Cc: Palmer Dabbelt <palmer@sifive.com>
> > > > > Subject: RE: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2
> > > > > 10/14]
> > > > > U500Pkg/Library: Library instances of U500 platform library
> > >
> > > > > > Seeing this, I have a bit of a feeling that this Opensbi isn't
> > > > > > being consistently
> > > > > > treated/named:
> > > > > > - In edk2, we have RiscVPkg/Library/RiscVOpensbiLib which
> implements
> > > > > >   class RiscVOpensbiLib.
> > > > > > - Here, we have U500Pkg/Library/OpenSbiPlatformLib
> > > > > > implementing
> > > the
> > > > > >   class OpenSbiPlatformLib.
> > > > > > - *Logically*, what we have is
> > > > > >   Platform/RiscV/Universal/Sec/SecMain.inf which depends on the
> > > > > >   OpensbiLib which depends on the OpenSbiPlatformLib.
> > > > > >   - However, there is no OpenSbiPlatformLib in edk2, which is very
> > > > > >     unfortunate because that does not let us build edk2/RiscVPkg in
> > > > > >     isolation (like we can with all the other packages in edk2).
> > > > > Will name it as Opensbi.
> > > > > >
> > > > > > Here is my preferred solution for untangling this:
> > > > > > 1) Standardise on OpenSbi or Opensbi. Opensbi follows the pattern
> we
> > > > > >    already use for Openssl, so would be my preference.
> > > > > > 2) Standardise on RiscVOpensbi or Opensbi - both for filenames and
> > > > > >    LibararyClasses. Either is fine, the former is less likely to clash
> > > > > >    with other imported projects in the future (but I would estimate
> > > > > >    this risk as *very* low to begin with).
> > > > > > 3) Implement a (RiscV)OpensbiPlatformLibNull in
> > > > > > edk2/RiscVPkg/Library,
> > > > > I will have OpensbiPlatformLibNull  in RiscVPlatformPkg/Library
> > > > >
> > > > > >    containing only what is required to build/link
> > > > > >    RiscVPkg/Library/(RiscV)OpensbiLib/.
> > > > > >    3.5) Map that to OpensbiPlatformLib in RiscVPkg/RiscVPkg.dsc.
> > > > > >    3.6) Add a depedency on OpensbiPlatformLib to
> > > > > >         (RiscV)OpensbiLib.inf
> > > > > >    3.7) Drop the dependency on OpensbiPlatformLib from
> > > > > >         Platform/RiscV/Universal/Sec/SecMain.inf
> > > > >
> > > > > Above work for me
> > > >
> > > > I changed my mind. Due to SecMain is moved to RiscVPlatformPkg in
> > > > edk2 and requires both RiscVOpensbiPlatformLib and
> > > > RiscVOpensbLiib, SecMain.inf still has dependency with these two libs.
> > >
> > > I am not sure I understand.
> > >
> > > What I am saying is that OpensbiLib already has a dependency on
> > > OpensbiPlatformLib. So there is no benefit to adding the
> > > OpensbiPlatformLib dependency to SecMain. Or does that not work for
> some reason?
> >
> > In order to make those dependencies correct, below is the new changes,
> >
> > RiscVOpensbiPlatformLib has NO dependency with RiscVOpensbiLib.
> > RiscVOpensbiLib has NO dependency with RiscVOpensbiPlatformLib.
> > SecMain.inf has dependencies with both RiscVOpensbiPlatformLib and
> > RiscVOpensbiLib because SecMain.inf needs the functionalities of those
> > libs.
> 
> Ah, OK - this is new for v3 then?
> In that case, sure, the above sounds fine.
Yes, this will be in the next set of patches.
> 
> Regards,
> 
> Leif