Re: [edk2-devel] [PATCH 0/6] [RFC] Rework UefiCpuPkg

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

[-- Attachment #1: Type: text/plain, Size: 6237 bytes --]

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.kinney@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>
Cc: Schaefer, Daniel (ROM Janitor) <daniel.schaefer@hpe.com>; eric.dong@intel.com <eric.dong@intel.com>; rahul1.kumar@intel.com <rahul1.kumar@intel.com>; Sunil V L <sunilvl@ventanamicro.com>; Andrew Fish <afish@apple.com>; quic_llindhol@quicinc.com <quic_llindhol@quicinc.com>; Chao Li <lichao@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.kinney@intel.com>
> Sent: Saturday, March 19, 2022 12:47 AM
> To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> <abner.chang@hpe.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Ni, Ray <ray.ni@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.chang@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
> >
> >
> >
> > 
> >


[-- Attachment #2: Type: text/html, Size: 9798 bytes --]