Re: [edk2-devel] The principles of EDK2 module reconstruction for archs

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Hi Mike and Leif,
First of all, we don’t use arch folder if the module is mainly for a specific 
arch in obviously. So we will  have both common and arch-specific files 
constructed under module/library root.
https://edk2.groups.io/g/devel/message/94567
SmmCpuFeatureLib\Ia32
SmmCpuFeatureLib\X64
SmmCpuFeatureLib\SmmCpuFeatureLib.c
SmmCpuFeatureLib\SmmCpuFeatureLibCommon.c
SmmCpuFeatureLib\IntelSmmCPuFeaturesLib
SmmCpuFeatureLib\AmdlSmmCPuFeaturesLib


> > Could we concatenate architectures?
> > I.e. AARCH64_ARM, IA32_X64, AARCH64_RISCV64... ?
Looks like below?

CpuDxe\IA32_X64\IA32\abc.nasm
CpuDxe\IA32_X64\X64\abc.nasm
CpuDxe\IA32_X64\CpuDxe.c
CpuDxe\IA32_X64\AmdCpuDxe.c
CpuDxe\IA32_X64\IntelCpuDxe.c
CpuDxe\RiscV64\CpuDxe.c
CpuDxe\ARM\CpuDxe.c
CpuDxe\
               CpuDxeCommon.c  // If required.
                CpuDxe.inf               // Use INF section arch modifier for 
X86, RISC-V and ARM.
                CpuDxeAmd.inf        // Separate INF for AMD.

Seems ok, but is AARCH64_RISCV64 a real case?  Or it means we can create a 
folder "AARCH64_RISCV64" when there are some common files shared by AARCH64 and 
RISCV64?

For the 32/64 bit support, it can still stay under module root and don’t need 
to assign a folder for them unless the arch has the different implementation.
Regards,
Abner



> -----Original Message-----
> From: Kinney, Michael D <michael.d.kin...@intel.com>
> Sent: Saturday, October 1, 2022 2:52 AM
> To: devel@edk2.groups.io; quic_llind...@quicinc.com; Chang, Abner
> <abner.ch...@amd.com>; Ni, Ray <ray...@intel.com>; Attar, AbdulLateef
> (Abdul Lateef) <abdullateef.at...@amd.com>; Sunil V L
> <suni...@ventanamicro.com>; Kinney, Michael D <michael.d.kin...@intel.com>
> Cc: lichao <lic...@loongson.cn>; Kirkendall, Garrett
> <garrett.kirkend...@amd.com>; Grimes, Paul <paul.gri...@amd.com>; He,
> Jiangang <jiangang...@amd.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.
> 
> 
> Hi Leif,
> 
> Concatenation is a good idea.  Makes it more obvious and can be easily adopted
> for new CPU archs.
> 
> There is an additional case where an implementation does not have differences
> based on CPU Arch or Vendor, but it does have differences based on the bit-
> width of the CPU.  Take BaseSafeIntLib as an example.
> It has source files for 32-bit CPUs, 64-bit CPUs, and CPU arch specific file 
> for Ebc
> because Ebc adapts to 32-bit or 64-bit depending on the CPU type the EBC
> Interpreter is running.
> 
> MdePkg/Library/BaseSafeIntLib/
>   BaseSafeIntLib.inf
>   SafeIntLib.c
>   SafeIntLib32.c
>   SafeIntLib64.c
>   SafeIntLibEbc.c
> 
> Should we add "32" and "64" as supported suffices in file names?
> 
> If we wanted to put type content into a subdirectory, what would be the
> suggested directory name for "32" and "64".  Or do we want to require this 
> type
> of difference to always be files in top level directory of the 
> modules/library?
> 
> Best regards,
> 
> Mike
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif
> > Lindholm
> > Sent: Friday, September 30, 2022 9:09 AM
> > To: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kin...@intel.com>; Chang, Abner <abner.ch...@amd.com>; Ni,
> > Ray <ray...@intel.com>; Attar, AbdulLateef (Abdul Lateef)
> > <abdullateef.at...@amd.com>; Sunil V L <suni...@ventanamicro.com>
> > Cc: lichao <lic...@loongson.cn>; Kirkendall, Garrett
> > <garrett.kirkend...@amd.com>; Grimes, Paul <paul.gri...@amd.com>; He,
> > Jiangang <jiangang...@amd.com>; Andrew Fish <af...@apple.com>
> > Subject: Re: [edk2-devel] The principles of EDK2 module reconstruction
> > for archs
> >
> > I agree similar things will certainly happen for ARM/AARCH64, which
> > will probably be noticeable when I start eradicating ArmPkg and
> > putting the arch-standard bits into (mostly) MdePkg.
> >
> > But I like the ability to see already at the filesystem level which
> > files belong to the architecture I'm currently working on and which don't.
> >
> > Could we concatenate architectures?
> > I.e. AARCH64_ARM, IA32_X64, AARCH64_RISCV64... ?
> >
> > /
> >      Leif
> >
> > On 2022-09-30 08:28, Michael D Kinney wrote:
> > > Hi Abner,
> > >
> > > One comment is on adding a new CPU type dir name of 'X86' for
> > > content that is common for IA32/X64.
> > >
> > > I can imagine a similar case for ARM/AARCH64 and for the RISCV (32,
> > > 64, 128) cases.
> > >
> > > I think I would prefer to drop X86 and if there are files that are
> > > common to multiple CPU architectures then they are considered common
> > > and are in top directory of module and the file header and INF file
> > > can clearly document which CPU archs the file applies.
> > >
> > > Mike
> > >
> > >> -----Original Message-----
> > >> From: Chang, Abner <abner.ch...@amd.com>
> > >> Sent: Friday, September 30, 2022 12:11 AM
> > >> To: Ni, Ray <ray...@intel.com>; Attar, AbdulLateef (Abdul Lateef)
> > >> <abdullateef.at...@amd.com>; Sunil V L <suni...@ventanamicro.com>;
> > >> devel@edk2.groups.io; Kinney, Michael D
> > >> <michael.d.kin...@intel.com>
> > >> Cc: lichao <lic...@loongson.cn>; Kirkendall, Garrett
> > >> <garrett.kirkend...@amd.com>; Grimes, Paul <paul.gri...@amd.com>;
> > >> He, Jiangang <jiangang...@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
> > >>
> > >> [AMD Official Use Only - General]
> > >>
> > >> Thanks Ray, here are my responses.
> > >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> > >> ithub.com%2Ftianocore-docs%2Fedk2-CCodingStandardsSpecification%2Fp
> > >>
> ull%2F2&amp;data=05%7C01%7CAbner.Chang%40amd.com%7C7c3292c8bd2f4
> 86f
> > >>
> 920908daa314e8e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6
> 3800
> > >>
> 1607445876768%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
> IjoiV
> > >>
> 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=
> HAq
> > >> ou4JyY1yxDthuQ1dEKcF7Q3o4W77Oo9rOOvkXNWU%3D&amp;reserved=0
> > >>
> > >> @Kinney, Michael D we may also need your clarification on the comments.
> > >>
> > >>
> > >>> -----Original Message-----
> > >>> From: Ni, Ray <ray...@intel.com>
> > >>> Sent: Thursday, September 29, 2022 3:42 PM
> > >>> To: Attar, AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>;
> > >>> Chang, Abner <abner.ch...@amd.com>; Sunil V L
> > >>> <suni...@ventanamicro.com>; devel@edk2.groups.io
> > >>> Cc: Kinney, Michael D <michael.d.kin...@intel.com>; lichao
> > >>> <lic...@loongson.cn>; Kirkendall, Garrett
> > >>> <garrett.kirkend...@amd.com>; Grimes, Paul <paul.gri...@amd.com>;
> > >>> He, Jiangang <jiangang...@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.
> > >>>
> > >>>
> > >>> Abner,
> > >>> Comments in
> > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > >>> gith
> > >>> ub.com%2Ftianocore-docs%2Fedk2-
> > >>> CCodingStandardsSpecification%2Fpull%2F2%23pullrequestreview-
> > >>>
> 1124763311&amp;data=05%7C01%7CAbner.Chang%40amd.com%7Cd825e24
> > >>>
> 8625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82d994e183d%7C0
> > >>>
> %7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
> > >>>
> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
> > >>>
> 7C%7C%7C&amp;sdata=RXxgpbEr6ivbqP1R6%2B3Rxl%2ByJAnaUJuaYYKdfCH
> > >>> 8jo8%3D&amp;reserved=0
> > >>>
> > >>> We can discuss more in tomorrow's meeting.
> > >>>
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Attar, AbdulLateef (Abdul Lateef)
> > >>>> <abdullateef.at...@amd.com>
> > >>>> Sent: Thursday, September 29, 2022 3:11 PM
> > >>>> To: Chang, Abner <abner.ch...@amd.com>; Sunil V L
> > >>>> <suni...@ventanamicro.com>; devel@edk2.groups.io; Ni, Ray
> > >>>> <ray...@intel.com>
> > >>>> Cc: Kinney, Michael D <michael.d.kin...@intel.com>; lichao
> > >>>> <lic...@loongson.cn>; Kirkendall, Garrett
> > >>>> <garrett.kirkend...@amd.com>; Grimes, Paul <paul.gri...@amd.com>;
> > >>> He,
> > >>>> Jiangang <jiangang...@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
> > >>>>
> > >>>> Hi Abner,
> > >>>>      Looks good to me.
> > >>>> Reviewed-by:  Abdul Lateef Attar <abdat...@amd.com>
> > >>>>
> > >>>> Thanks
> > >>>> AbduL
> > >>>>
> > >>>> -----Original Message-----
> > >>>> From: Chang, Abner <abner.ch...@amd.com>
> > >>>> Sent: 28 September 2022 20:31
> > >>>> To: Sunil V L <suni...@ventanamicro.com>; devel@edk2.groups.io;
> > >>>> ray...@intel.com
> > >>>> Cc: Kinney, Michael D <michael.d.kin...@intel.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
> > >>>>
> > >>>> [AMD Official Use Only - General]
> > >>>>
> > >>>> I just had created PR to update edkII C coding standard spec for
> > >>>> the file and directory naming. We can review and confirm this
> > >>>> update first and then go back to the principles of EDK2 module
> reconstruction for archs.
> > >>>> Here is the PR:
> > >>>>
> > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > >>> gith
> > >>>> ub.com%2Ftianocore-docs%2Fedk2-
> > >>> &amp;data=05%7C01%7CAbner.Chang%40amd.c
> > >>>>
> > >>>
> om%7Cd825e248625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82
> > >>> d994e18
> > >>>>
> > >>>
> 3d%7C0%7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJ
> > >>> WIjoiMC4wLj
> > >>>>
> > >>>
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%
> > >>> 7C%7C&a
> > >>>>
> > >>>
> mp;sdata=X4z9puj81nIGTqtMxC9igDZyBPOT6OTWSU%2BjoIowo%2BE%3D&a
> > >>> mp;reserv
> > >>>> ed=0
> > >>>> CCodingStandardsSpecification/pull/2
> > >>>>
> > >>>> The naming rule is mainly for the new module or new file IMO.
> > >>>> Some existing module may not meet the guidelines mentioned in this
> spec.
> > >>>> Thus we need the principles of EDK2 module reconstruction on the
> > >>>> existing module to support other processor archs and not
> > >>>> impacting the
> > >>> existing platforms (e.g.
> > >>>> rename the INF file or directory to meet the guidelines).
> > >>>>
> > >>>> Sunil, seems RISC-V CpuDxe meet the guideline. Please check it.
> > >>>> Just feel that having  CpuDxe.c to Riscv64 folder is not quite a
> > >>>> best solution. I think at least we can abstract the protocol
> > >>>> structure and protocol installation under CpuDxe\ and have the
> > >>>> arch implementation under arch folder. We can discuss this later
> > >>>> after we confirming the
> > >>> guideline and principles.
> > >>>>
> > >>>> Thanks
> > >>>> Abner
> > >>>>
> > >>>>> -----Original Message-----
> > >>>>> From: Sunil V L <suni...@ventanamicro.com>
> > >>>>> Sent: Wednesday, September 28, 2022 3:34 PM
> > >>>>> To: devel@edk2.groups.io; ray...@intel.com
> > >>>>> Cc: Chang, Abner <abner.ch...@amd.com>; Kinney, Michael D
> > >>>>> <michael.d.kin...@intel.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.
> > >>>>>
> > >>>>>
> > >>>>> On Wed, Sep 28, 2022 at 03:33:45AM +0000, Ni, Ray wrote:
> > >>>>> Hi Ray,
> > >>>>>>
> > >>>>>>    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.
> > >>>>>>
> > >>>>>>    *   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.
> > >>>>>>
> > >>>>>> [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.
> > >>>>>>
> > >>>>>
> > >>>>> Could you please take a look below changes which is trying to
> > >>>>> add RISC-V support for CpuDxe?
> > >>>>>
> > >>>>
> > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > >>> gith
> > >>>>> ub.com%2Ftianocore%2Fedk2-
> > >>>>>
> > >>>>
> > >>>
> staging%2Fcommit%2Fbba1a11be47dd091734e185afbed73ea75708749&amp;
> > >>>>>
> > >>>>
> > >>>
> data=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08d
> > >>>>>
> > >>>>
> > >>>
> aa123e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947
> > >>>>>
> > >>>>
> > >>>
> 2732494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> > >>>>>
> > >>>>
> > >>>
> oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sd
> > >>>>>
> > >>>>
> > >>>
> ata=Vq6pJLnn8yJrJhFZn7LfLbZzrtpG4n1VLWgAil6J38U%3D&amp;reserved=0
> > >>>>>
> > >>>>
> > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > >>> gith
> > >>>>> ub.com%2Ftianocore%2Fedk2-
> > >>>>>
> > >>>>
> > >>>
> staging%2Fcommit%2F7fccf92a97a6d0618a20f10622220e78b3687906&amp;da
> > >>>>>
> > >>>>
> > >>>
> ta=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08daa1
> > >>>>>
> > >>>>
> > >>>
> 23e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947273
> > >>>>>
> > >>>>
> > >>>
> 2494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV
> > >>>>>
> > >>>>
> > >>>
> 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata
> > >>>>>
> > >>>>
> > >>>
> =xFmvUv58vh4AUAM17Qy9G5jZWFZlK2Ozt3njpG1e8%2BY%3D&amp;reserv
> > >>>>> ed=0
> > >>>>>
> > >>>>> What do you suggest with above example?
> > >>>>>
> > >>>>> 1) Common INF for all architectures - but modify INF alone, no
> > >>>>> X86 folder creation.
> > >>>>>
> > >>>>> This is what I have done in the commit above. May be of least
> > >>>>> impact to existing code since it is only INF change. But like
> > >>>>> you mentioned this is bit weird that X86 files will remain in
> > >>>>> root folder directly along with some common files.
> > >>>>>
> > >>>>> 2) Common INF (CpuDxe.inf) + create arch folders X86, X64, IA32,
> > >>>>> RiscV64 etc
> > >>>>>
> > >>>>> IMO, this is probably the best approach. What would be the
> > >>>>> challenges with this?
> > >>>>>
> > >>>>> 3) Separate INF for arch like CpuDxe.inf for x86,
> > >>>>> CpuDxeRiscV64.inf for
> > >>>> RISC-V.
> > >>>>>
> > >>>>> This again probably is not a good idea.
> > >>>>>
> > >>>>> 4) If the module/library is specific to one arch (ex: SMM(X86),
> > >>>>> SBI(RISC-V)), then create separate INF.
> > >>>>>
> > >>>>> Thanks!
> > >>>>> Sunil
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> > 
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94631): https://edk2.groups.io/g/devel/message/94631
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]
-=-=-=-=-=-=-=-=-=-=-=-