> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Abner Chang
> Sent: Thursday, December 19, 2019 10:48 PM
> To: Leif Lindholm <leif.lindh...@linaro.org>; devel@edk2.groups.io
> Cc: Chen, Gilbert <gilbert.c...@hpe.com>; Kinney, Michael D
> <michael.d.kin...@intel.com>
> Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v3 03/39]
> RiscVPkg/opensbi: EDK2 RISC-V OpenSBI support
>
>
>
> > -----Original Message-----
> > From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > Sent: Thursday, December 19, 2019 9:22 PM
> > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> > <abner.ch...@hpe.com>
> > Cc: Chen, Gilbert <gilbert.c...@hpe.com>; Kinney, Michael D
> > <michael.d.kin...@intel.com>
> > Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v3 03/39]
> > RiscVPkg/opensbi: EDK2 RISC-V OpenSBI support
> >
> > On Thu, Dec 19, 2019 at 04:09:22 +0000, Abner Chang wrote:
> > > > > diff --git a/RiscVPkg/Include/sbi/sbi.h
> > > > > b/RiscVPkg/Include/sbi/sbi.h new file mode 100644 index
> > > > > 0000000..04e7f18
> > > > > --- /dev/null
> > > > > +++ b/RiscVPkg/Include/sbi/sbi.h
> > > > > @@ -0,0 +1,52 @@
> > > > > +/** @file
> > > > > + SBI inline function calls.
> > > > > +
> > > > > + Copyright (c) 2019, Hewlett Packard Enterprise Development LP.
> > > > > + All rights reserved.<BR>
> > > > > +
> > > > > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > > +
> > > > > +**/
> > > > > +
> > > > > +#ifndef EDK2_SBI_H_
> > > > > +#define EDK2_SBI_H_
> > > > > +
> > > > > +#include <sbi/sbi_types.h> // Reference to header file wrapper
> > > > > +#include <include/sbi/riscv_asm.h> // Reference to header file
> > > > > +in opensbi
> > > >
> > > > This whole sbi situation is too much of a hornets' nest, and
> > > > really does need to get sorted before we even push this to -staging.
> > > > (But I do think is is one of the last things we need to address.)
> > > >
> > > > And I think the starting point to untangle it is to rename the
> > > > wrapper include files in RiscVPkg/Include/sbi (and indeed the
> > > > directory
> > > > itself) to conform with the TianoCore standard.
> > > >
> > > > That means:
> > > >
> > > > RiscVPkg/Include/IndustryStandard/Sbi/Sbi.h
> > > > RiscVPkg/Include/IndustryStandard/Sbi/SbiBits.h
> > > > RiscVPkg/Include/IndustryStandard/Sbi/SbiTypes.h
> > > >
> > > > Unless all of the wrappers can be combined into a single
> > > > RiscVPkg/Include/IndustryStandard/Sbi.h
> > > >
> > > > ...and then tested on a case-sensitive filesystem.
> > > >
> > > > For the record, doing that immediately results in the non-wrapper
> > > > versions being pulled in and then the build failing.
> > >
> > > Leif,
> > > We can't change the naming of those header files under
> > > RiscVPkg/Include/sbi. We build opensbi source code in edk2 and those
> > > source files refer to those header file. And we are not going to
> > > change opensbi code as we discussed before.
> > >
> > > The only file we can change is sbi.h -> Sbi.h because this header
> > > file is created for edk2 RISC-V related drivers.
> > >
> > > I think we can make it this way,
> > > Rename sbi.h to RiscVOpensbi.h and move it to under
> > > RiscVPkg/Include/IndustryStandard, no need "/Sbi" subdirectory.
> >
> > That sounds good to me.
> >
> > > Keep the naming of sbi_bits.h and sbi_types.h and still leave those
> > > under RiscVPkg/Include/sbi.
> > >
> > > No naming changes in order to build opensbi source file. Can't even
> > > change "/sbi" subdirectory to "/Sbi". sbi_bits.h and sbi_types.h are
> > > the binding files for edk2 framework BTW.
> >
> > Sure, if we separate interface for EDK2 apps from binding, it becomes
> > less bad. Plus sbi_bits.h needs to just go, but we'll fix that in
> > opensbi before merging Risc-V to edk2 master.
> I am not sure this one. That may take some time to fix that in opensbi. I
> would rather merge our code to edk2 master first and deal with opensbi in
> parallel. Some people they approach to me and ask for edk2 RISC-V port.
> Currently they can just go to our private Github to retrieve code. More
> people will join edk2 RISC-V port if we can merge it to edk2 master earlier.
>
>
> > I think the proper fix for sbi_types.h also involves a change to opensbi.
> This has to ask opensbi to have binging mechanism for different frameworks,
> however I agree with you in this part.
>
> >
> > But if you can
> > - do those changes
> > - move/rename SbiFirmwareContext.h to live with RiscVOpensbi.h
> Hehe already. I know you :). I already moved this file to under
> RiscVPkg/Include/IndustryStandard just a separate file.
Just merged SbiFirmwareContext.h to RiscVOpensbi.h.
Now we only have two header files under Include/sbi/. I will start the
conversation with opensbi people to fix the duplicate definitions issue when
other firmware frameworks leverage opensbi. This helps to remove sbi_bits.h
from edk2 RISC-V port.
Also consult with them regard with the binding mechanism in opensbi for
different firmware frameworks.
Lefi, most of comments for RISC-V edk2 port are done and I will clean up the
commits on the private github repo we are working on. The next will be
edk2-platforms.
Abner
>
> > - add a (short) Readme.md to RiscVPkg/Include/sbi explaining what
> > these files are and why
> > I think that resolves my final reservations against bringing this set
> > onto a staging branch.
> sure
>
> >
> > Regards,
> >
> > Leif
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52438): https://edk2.groups.io/g/devel/message/52438
Mute This Topic: https://groups.io/mt/38757517/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
