Abner, I think another guideline is to minimize the number of subdirectories.
Only create them if it helps with the organization and long term maintenance of the component. Mike > -----Original Message----- > From: Chang, Abner <abner.ch...@amd.com> > Sent: Sunday, October 2, 2022 8:13 PM > To: Kinney, Michael D <michael.d.kin...@intel.com>; devel@edk2.groups.io; > quic_llind...@quicinc.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 > > [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&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&sdata= > > HAq > > > >> ou4JyY1yxDthuQ1dEKcF7Q3o4W77Oo9rOOvkXNWU%3D&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&data=05%7C01%7CAbner.Chang%40amd.com%7Cd825e24 > > > >>> > > 8625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82d994e183d%7C0 > > > >>> > > %7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC > > > >>> > > 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000% > > > >>> > > 7C%7C%7C&sdata=RXxgpbEr6ivbqP1R6%2B3Rxl%2ByJAnaUJuaYYKdfCH > > > >>> 8jo8%3D&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- > > > >>> &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& > > > >>>>> > > > >>>> > > > >>> > > data=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08d > > > >>>>> > > > >>>> > > > >>> > > aa123e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947 > > > >>>>> > > > >>>> > > > >>> > > 2732494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > > > >>>>> > > > >>>> > > > >>> > > oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd > > > >>>>> > > > >>>> > > > >>> > > ata=Vq6pJLnn8yJrJhFZn7LfLbZzrtpG4n1VLWgAil6J38U%3D&reserved=0 > > > >>>>> > > > >>>> > > > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > > >>> gith > > > >>>>> ub.com%2Ftianocore%2Fedk2- > > > >>>>> > > > >>>> > > > >>> > > staging%2Fcommit%2F7fccf92a97a6d0618a20f10622220e78b3687906&da > > > >>>>> > > > >>>> > > > >>> > > ta=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08daa1 > > > >>>>> > > > >>>> > > > >>> > > 23e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947273 > > > >>>>> > > > >>>> > > > >>> > > 2494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > > >>>>> > > > >>>> > > > >>> > > 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata > > > >>>>> > > > >>>> > > > >>> > > =xFmvUv58vh4AUAM17Qy9G5jZWFZlK2Ozt3njpG1e8%2BY%3D&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 (#94632): https://edk2.groups.io/g/devel/message/94632 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] -=-=-=-=-=-=-=-=-=-=-=-
