Mike,
Has following content already been documented somewhere?
It looks good to me. Very good abstraction of existing cases. Maybe there are
some lib/modules that don’t follow this rule. But the number should be very
small.
But I didn’t check how ARM constructs the pkg. So it’s very welcomed to see
non-X86 people for review.
Thanks,
Ray
From: Kinney, Michael D <michael.d.kin...@intel.com>
Sent: Saturday, September 24, 2022 2:01 AM
To: devel@edk2.groups.io; abner.ch...@amd.com; Kinney, Michael D
<michael.d.kin...@intel.com>
Cc: Ni, Ray <ray...@intel.com>; Sunil V L <suni...@ventanamicro.com>; lichao
<lic...@loongson.cn>; Kirkendall, Garrett <garrett.kirkend...@amd.com>; Grimes,
Paul <paul.gri...@amd.com>; He, Jiangang <jiangang...@amd.com>; Attar,
AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>; Leif Lindholm
<quic_llind...@quicinc.com>; Andrew Fish <af...@apple.com>
Subject: RE: The principles of EDK2 module reconstruction for archs
Hi Abner,
I think it would be good to clarify when a difference in implementation is due
to a CPU Arch difference or a Vendor implementation difference.
I would also be good to provide guidelines for directory names and file names
for all EDK II meta data file types. Here are some examples to get started:
Package Directory Name: <PackageName>Pkg
Package DEC File Name:
<PackageName>Pkg.dec
<PackageName> REQUIRED *
Module Directory Name: <ModuleName><Phase>
<
Feature >/<Phase>
Module INF File Name: <ModuleName><Phase>.inf
<
Feature>/<Phase>/<ModuleName>.inf
<Feature> OPTIONAL Only used
if implementation does not have any shared code between phases (e.g.
MdeModulePkg/Universal/PCD)
<Phase> REQUIRED Base,
Sec, Pei, Dxe, DxeRuntime, Mm, StandaloneMm, Smm, Uefi
<ModuleName> REQUIRED *
[Ray] Looks good to me. Good abstraction of existing cases.
Library Instance Directory Name:
<Phase><CpuArch><Vendor><LibraryClassName><Dependency>
Library Instance INF File Name:
<Phase><CpuArch><Vendor><LibraryClassName><Dependency>.inf
<Phase> REQUIRED Base,
Sec, Pei, Dxe, DxeRuntime, Mm, StandaloneMm, Smm, Uefi
<CpuArch> OPTIONAL Ia32, X64,
Arm, AArch64, RiscV64, LoongArch64, Ebc If
not provided, then component supports >=2 or all CPU archs
<Vendor> OPTIONAL *
<LibraryClassName> REQUIRED *
<Dependency> OPTIONAL *
Typically name of PPI, Protocol, LibraryClass that the implementation is
layered on top of.
Source File Paths within a Library/Module instance
<FileName>.c
<FileName>.h
<CpuArch>/<FileName>.c
<CpuArch>/<FileName>.h
<CpuArch>/<FileName>.nasm
<CpuArch>/<FileName>.S
<CpuArch> OPTIONAL Ia32, X64,
Arm, AArch64, RiscV64, LoongArch64, Ebc
[Ray] Looks good to me. Good abstraction of existing cases.
I think the point you are raising in the discussion is that sometimes there may
be shared content between a small subset of CPU archs (e.g. IA32/X64 or
Arm/AArch64 or RiscV32/RiscV64/RiscV128) and that you are proposing a new
standard directory name for these combinations. Your proposal is X86 for a
directory that contains content for both IA32 and X64. You are also wanting to
support vendor specific content in the naming convention. An example where it
is already being done is in MdePkg/Include/Registers/<VendorName>. So an
enhancement to the above Source File Paths would be:
Source File Paths within a Library/Module instance
<FileName>.c
<FileName>.h
<CpuArch>/<FileName>.c
<CpuArch>/<FileName>.h
<CpuArch>/<FileName>.nasm
<CpuArch>/<FileName>.S
<CpuArch>/<Vendor>/<FileName>.c
<CpuArch>/<Vendor>/<FileName>.h
<CpuArch>/<Vendor>/<FileName>.nasm
<CpuArch>/<Vendor>/<FileName>.S
<CpuArch> OPTIONAL Ia32, X64,
Arm, AArch64, RiscV64, LoongArch64, Ebc, X86
<Vendor> OPTIONAL *
I am not sure if we should use “Common” in the naming conventions. I think by
default, any content that is not CpuArch or Vendor specific could be considered
common content.
[Ray] Good to me.
Thanks,
Mike
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
<devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chang, Abner
via groups.io
Sent: Friday, September 23, 2022 8:39 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Ni, Ray <ray...@intel.com<mailto:ray...@intel.com>>; Kinney, Michael D
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>; Sunil V L
<suni...@ventanamicro.com<mailto:suni...@ventanamicro.com>>; lichao
<lic...@loongson.cn<mailto:lic...@loongson.cn>>; Kirkendall, Garrett
<garrett.kirkend...@amd.com<mailto:garrett.kirkend...@amd.com>>; Grimes, Paul
<paul.gri...@amd.com<mailto:paul.gri...@amd.com>>; He, Jiangang
<jiangang...@amd.com<mailto:jiangang...@amd.com>>; Attar, AbdulLateef (Abdul
Lateef) <abdullateef.at...@amd.com<mailto:abdullateef.at...@amd.com>>; Leif
Lindholm <quic_llind...@quicinc.com<mailto:quic_llind...@quicinc.com>>; Andrew
Fish <af...@apple.com<mailto:af...@apple.com>>
Subject: [edk2-devel] The principles of EDK2 module reconstruction for archs
[AMD Official Use Only - General]
All,
Today in edk2 open design meeting, we went through the draft of principles of
the EDK2 module reconstruction for accommodating different archs (IA32, X64,
Arm, AArch64, RiscV64, Loongson64 and etc.) or different vendors of the same
arch (AMD/Intel to IA32 and X64).
@Ray Ni<mailto:ray...@intel.com> and
@michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>
1. We may need somewhere on edk2 repo or another place that can have PR for
the easy review, please let me know where I can create the PR instead of
reviewing this through dev mailing list.
2. I didn’t mention using CPUID, Family ID or PCD to have the different code
path for AMD and Intel. The reason is,
* This decreases the readability of code.
* That makes a confusing to the review process
i. Says
the maintainer/reviewer owns the package, however the patch is specific to AMD
implementation but the change is in the file that mixes up Intel and AMD code.
Then who is supposed the right person to give the reviewed-by? Perhaps the AMD
edk2 module maintainers or reviewers is the proper person to give the
reviewed-by for this case. Of course, other maintainers still can join the
review process and give the comments. So to separate the arch-specific code in
a arch-specific source file simplifies the review, even that is just a small
delta between two implementations.
ii. We can
have the maintainers or reviewers for the entire module or *Amd* files only. So
the maintainers/reviewers do not have to review the changes that only made for
other archs. But they have to help reviewing the common code if that gets
impact.
1. I didn’t mention to have <phase><arch><basename> for the new module. I
prefer we just inherit the original module name or file name so we can know the
module or the source file has the different implementation for archs in the
file browser (when the files are sorted in alphabet).
Lets discuss this using PR if possible.
Thanks
Abner
Below is the draft of principles:
Preface:
The principle is mainly for UefiCpuPKg, but the same principle maybe applied to
the EDK2 module that has the processor architecture dependence (such as the
BaseLib under MdePkg/Library). Most of the EDK2 modules under UefiCpuPkg were
developed specifically to IA32/X64 architecture, that is necessary to
reconstruct the folder or revise the source files to accommodate other
processor architectures. The EDK2 module reconstruction is also required for
accommodating the same-arch-but-different-vendor’s implementation (e.g., Intel
and AMD for the X86 based processors). The EDK2 module may be strictly
developed based on the specific processor architecture. The new introduced
implementation for other processor architectures may consider to have a
separate EDK2 module instance. Not all of the EDK2 modules revising can exactly
meet the principles listed below, that depends on the delta between the
original EDK2 module and the implementation for the new introduced processor
architecture. It may require the further discussions with EDK2 module
maintainers.
The [Arch] refers to the Processor Architecture.
The [Module] refer to the EDK2 module.
The [X86] refers to both IA32 and X64.
The principles to create the X86 folder in the module:
1. When X86-vendor’s implementation is introduced to the existing module:
1. The folder reconstruction:
A-1. If the module is obviously used by IA32/X64 only
* No need to create X86 folder
* Create X86-vendor’s stuff under the root directory of module
A-2. If the existing module is expected to accommodate the different archs or
the module already has multiple archs:
* Create X86 folder
* Create X86-vendor’s stuff under X86 folder
1. The files reconstruction:
B-1. The module INF metafile
* The existing INF metafile should be kept without relocation. Should not
have the impacts to the existing DSC/FDF file.
* The new introduced INF metafile should be located under the root
directory of module with the file naming format as below. This keeps the
consistent module file structure.
* <OriginalFileName><arch>.inf
B-2. Source files
The new arch implementation is introduced to the
module in order to leverage the source code and the module design architecture,
so
* That is contributor’s responsibility to review the source code and strip
the arch-dependent code away and put it into the arch-specific file. Leave the
common code in the original file if there is no arch-specific and
arch-specific-feature wordings in the file name. Create a common file for the
common implementation otherwise.
* The file naming for the arch-specific file
<OriginalFileName ><arch>.*
* The file naming for the common implementation
< OriginalFileNaming >Common.*
* That is contributor’s responsibility to relocate the arch-specific source
files to the arch-specific folder.
* That is contributor’s responsibility to make sure the original INF
metafile can properly pull-in the source file from arch-specific folder after
the source file reconstruction.
* The common source files should be located under the root directory of
module
1. When a new arch’s implementation is introduced to the existing module
which was developed for the specific arch:
1. The folder reconstruction:
* Create arch folder for the existing arch implementation
* Create the arch folder for the new introduced arch
1. The files reconstruction:
B-1. The module INF metafile
* The existing INF file should be kept without the relocation. Should not
have the impacts to the existing DSC/FDF file.
* The new introduced INF metafile should be located under the root
directory of module with the file naming format as below. This keeps the
consistent module file structure.
* < OriginalFileNaming><arch>.inf
B-2. Source files
The new arch implementation is introduced to this
module in order to leverage the source code and the module design architecture,
so
* That is contributor’s responsibility to review the source code and strip
the arch-dependent code away and put it into the arch-specific file. Leave the
common code in the original file if there is no arch-specific wording in the
file name. Create a common file for the common implementation otherwise.
* The file naming for the arch-specific source file
< OriginalFileNaming ><arch>.*
* The file naming for the common implementation
<OriginalFileNaming>Common.*
* That is contributor’s responsibility to relocate the arch-specific source
files to the arch-specific folder.
* That is contributor’s responsibility to make sure the original INF
metafile can properly pull-in the source file from arch-specific folder after
the source file reconstruction.
* The common source files should be located under the root directory of
module
1. When a new arch implementation has a huge delta with the original
implementation
Create a separate module instance. The naming of the module should follow below
format,
* Add the arch prefix with the original module name:
< OriginalModuleNaming><arch>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94449): https://edk2.groups.io/g/devel/message/94449
Mute This Topic: https://groups.io/mt/93872791/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
