[AMD Official Use Only - General]
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray via
groups.io
Sent: Wednesday, September 28, 2022 11:34 AM
To: devel@edk2.groups.io; Chang, Abner <abner.ch...@amd.com>
Cc: Kinney, Michael D <michael.d.kin...@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: [edk2-devel] The principles of EDK2 module reconstruction for archs
Caution: This message originated from an External Source. Use proper caution
when opening attachments, clicking links, or responding.
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
[Ray] Looks good.
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
[Ray] "CpuDxe.inf" and "CpuDxeArm.inf"? is that your intention? New developers
may be confused that CpuDxe.inf supports only X86 arch.
Do you have an example that one module supporting multiple archs using
different INF files? MdeModulePkg/DxeIpl is a module supporting different archs
with the same INF file.
Or shall we leave it to be decided between the patch contributor and
package/module maintainer?
[Chang, Abner] Here I mean the library module, for example
SmmCpuFeatureLib.inf and AmdSmmCpuFeatureLib.inf. Will make the statement
clear. The file naming above would be changed to <arch><OriginalFileName>.inf
as Mike suggested.
I am not sure if there is another example having arch-specific INF file.
Usually the driver module has the abstract interface and the implementation in
the library module. A newly introduced processor architecture driver may create
it's own module such as ArmCpuDxe if the delta between implementations is
huge. However, I would prefer to have arch-specific INF for the module if the
module implementation can be leveraged. And yes, this requires the discussion
between contributor and module maintainer if the principles can not perfectly
identify the case.
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
[Ray] If you check the MpInitLib, most of SEV logic are moved to AmdSev.c. But
MpLib.c still contains logic to call functions from AmdSev.c based on some AMD
specific check. In my opinion, that's already good enough.
[Chang, Abner] I am not quite lean to the If-AMD-else in the source file
solution. I prefer to separate AMD stuff or vendor feature to a separated file.
So we can have the reviewer or maintainer for *Amd* files/module/packages
specifically. This makes the review responsibility clear and efficient.
However, if you check MdeModulePkg/DxeIpl, implementations for different archs
are in different *folders*.
Can we leave this to the module owner to decide how to place the
implementations?
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
[Ray] Do you move existing arch implementation to that arch folder? It will
break existing platforms a lot.
[Chang, Abner] We will move the arch implementation to the arch folder without
moving INF. This wont impact the platform DSC and FDF, however this has the
impact to the existing overwrite.
* Create the arch folder for the new introduced arch
[Ray] I agree. But if we don't create arch folder for existing arch
implementation, the pkg layout will be a mess.
[Chang, Abner] right, so the first bullet is important.
[Ray] Hard for me to understand all the principles here. Maybe we review
existing code including to-be-upstreamed code and decide how to go.
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 (#94450): https://edk2.groups.io/g/devel/message/94450
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]
-=-=-=-=-=-=-=-=-=-=-=-
