[AMD Official Use Only - General] Thanks Ray, here are my responses. https://github.com/tianocore-docs/edk2-CCodingStandardsSpecification/pull/2
@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%2Fgith > 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%2Fgith > > 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%2Fgith > > > 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%2Fgith > > > 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 (#94563): https://edk2.groups.io/g/devel/message/94563 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] -=-=-=-=-=-=-=-=-=-=-=-
