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

["Leif Lindholm" <leif.lindholm@linaro.org>]

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.

Regards,

Leif