Hi Mike,
In the v2, I moved two of OpenSBI header files to under 
MdePkg/Include/IndustryStadard (patch 5/8). However, it looks to me not quite 
proper to have those files there. Seems to keeping those files under UefiCpuPkg 
makes more sense.
What do you think?

Abner
________________________________
From: Chang, Abner (HPS SW/FW Technologist)
Sent: Saturday, March 19, 2022 10:05 AM
To: Kinney, Michael D <michael.d.kin...@intel.com>; devel@edk2.groups.io 
<devel@edk2.groups.io>; Ni, Ray <ray...@intel.com>
Cc: Schaefer, Daniel (ROM Janitor) <daniel.schae...@hpe.com>; 
eric.d...@intel.com <eric.d...@intel.com>; rahul1.ku...@intel.com 
<rahul1.ku...@intel.com>; Sunil V L <suni...@ventanamicro.com>; Andrew Fish 
<af...@apple.com>; quic_llind...@quicinc.com <quic_llind...@quicinc.com>; Chao 
Li <lic...@loongson.cn>
Subject: RE: [edk2-devel] [PATCH 0/6] [RFC] Rework UefiCpuPkg


Just aware that I didn’t Cc stakeholders to the cover letter, add those people 
in CC.

-----Original Message-----
> From: Kinney, Michael D <michael.d.kin...@intel.com>
> Sent: Saturday, March 19, 2022 12:47 AM
> To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> <abner.ch...@hpe.com>; Kinney, Michael D <michael.d.kin...@intel.com>;
> Ni, Ray <ray...@intel.com>
> Subject: RE: [edk2-devel] [PATCH 0/6] [RFC] Rework UefiCpuPkg
>
> Hi Abner,
>
> Will OpenSBI content be needed by libs/modules outside of UefiCpuPkg?
Edk2OpenSBILib is the wrapper library of OpenSBI, so there is module neither 
inside nor outside UefiCpuPkg uses OpenSBI directly. Edk2OpenSbiLib is 
currently used by SecCore only as it describes here:
https://github.com/tianocore/edk2-platforms/tree/master/Platform/RISC-V/PlatformPkg.
We had designed to expose OepnSBI API in both edk2 version OpenSBI PPI and 
Protocol, so the answer to your question is: No, OpenSBI/Edk2OpenSbiLib is not 
needed by modules outside UefiCpuPkg as the design we have so far.

>
> Should OpenSBI includes be promoted to MdePkg?
That is possible and seems reasonable to move the entire 
OpenSBI(Edk2OpenSbiLib) source/include to under MdePkg because there is no 
dependency with edk2 RISC-V header files under UefiCpuPkg.
But one question: Shall we have OpenSBI lib under MdePkg if it is only used by 
UefiCpuPkg?
>
> I do not think the dir name "RISC-V" follows the file/dir name requirements.
> The '-' should not be used.
We had the same discussion before and '-' is valid in the file name as it 
defined in the coding standard. I remember we also agreed or accepted having 
"RISC-V' as the directory name for the modules on edk2-platforms repo. Same 
scenario can applied on edk2 repo.

>
> I think there is a discussion about moving UefiCpuLib to MdePkg.
Is that a serious discussion or just a verbal discussion? 😊 Any conclusion we 
had from the discussion?
Move UefiCpuLib to MdePkg leads the dependency with UefiCpuPkg for those 
architecture header files. I consider this doesn't make sense to MdePkg, right?
I would suggest still having UefiCPuPkg under UefiCpuPkg for now. Move it 
around one day when there is a clear decision for UefiCpuPkg comes out.

Thanks for feedbacks
Abner

>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abner
> Chang
> > Sent: Thursday, March 17, 2022 10:43 PM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner <abner.ch...@hpe.com>
> > Subject: [edk2-devel] [PATCH 0/6] [RFC] Rework UefiCpuPkg
> >
> >
> INVALID URI REMOVED
> d=3860__;!!NpxR!ymTSNAU_VEYCUYtX3YUSGsB0Ma3GzipVS95V5WEZxBeO
> QvdiEx6MgV61kZMs6TM$
> >
> > This is the project having rework on UefiCpuPkg in order to support a
> variety
> > of processor architectures. Some modules under UefiCpuPkg are required
> to be
> > abstract for the different archs.
> >
> > The first step is to classify UefiCpuPkg modules to IA32 and X64 sections in
> > DSC file (Patch 1/6). Move the module to Common section later if more
> than one
> > archs can leverage the same module (such as Patch 3/6 for BaseUefiCpuLib).
> >
> > Abner Chang (6):
> >   [RFC] UefiCpuPkg: Classify IA32/X64 modules in DSC file
> >   [RFC] UefiCpuPkg/Include: Add header files of RISC-V processor
> >     architecture
> >   [RFC] UefiCpuPkg/BaseUefiCpuLib: Add RISC-V RISCV64 instace
> >   [RFC] UefiCpuPkg/RiscVOpensbLib: Add opensbi submodule
> >   [RFC] UefiCpuPkg/Library: Add RiscVOpensbiLib
> >   [RFC] UefiCpuPkg: Update YAML file for RISC-V arch
> >
> >  UefiCpuPkg/UefiCpuPkg.dec                     |  12 +-
> >  UefiCpuPkg/UefiCpuPkg.dsc                     |  45 +++--
> >  .../Library/BaseUefiCpuLib/BaseUefiCpuLib.inf |   8 +-
> >  .../RiscVOpensbiLib/RiscVOpensbiLib.inf       |  89 ++++++++++
> >  .../Include/IndustryStandard/RISC-V/RiscV.h   | 162
> ++++++++++++++++++
> >  .../IndustryStandard/RISC-V/RiscVOpensbi.h    |  62 +++++++
> >  .../Include/Library/RISC-V/RiscVCpuLib.h      | 118 +++++++++++++
> >  UefiCpuPkg/Include/RISC-V/OpensbiTypes.h      |  82 +++++++++
> >  UefiCpuPkg/Include/RISC-V/RiscVImpl.h         |  87 ++++++++++
> >  .gitmodules                                   |  45 ++---
> >  BaseTools/Conf/tools_def.template             |   2 +-
> >  .../Library/BaseUefiCpuLib/BaseUefiCpuLib.uni |   5 +-
> >  .../Library/BaseUefiCpuLib/RISCV64/Cpu.S      | 143 ++++++++++++++++
> >  .../Library/RISC-V/RiscVOpensbiLib/opensbi    |   1 +
> >  UefiCpuPkg/UefiCpuPkg.ci.yaml                 |  61 ++++++-
> >  15 files changed, 877 insertions(+), 45 deletions(-)
> >  create mode 100644 UefiCpuPkg/Library/RISC-
> V/RiscVOpensbiLib/RiscVOpensbiLib.inf
> >  create mode 100644 UefiCpuPkg/Include/IndustryStandard/RISC-
> V/RiscV.h
> >  create mode 100644 UefiCpuPkg/Include/IndustryStandard/RISC-
> V/RiscVOpensbi.h
> >  create mode 100644 UefiCpuPkg/Include/Library/RISC-V/RiscVCpuLib.h
> >  create mode 100644 UefiCpuPkg/Include/RISC-V/OpensbiTypes.h
> >  create mode 100644 UefiCpuPkg/Include/RISC-V/RiscVImpl.h
> >  create mode 100644 UefiCpuPkg/Library/BaseUefiCpuLib/RISCV64/Cpu.S
> >  create mode 160000 UefiCpuPkg/Library/RISC-V/RiscVOpensbiLib/opensbi
> >
> > --
> > 2.31.1
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88022): https://edk2.groups.io/g/devel/message/88022
Mute This Topic: https://groups.io/mt/89863231/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to