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

["Michael D Kinney" <michael.d.kinney@intel.com>]

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.Chang@amd.com>
> Sent: Friday, September 30, 2022 12:11 AM
> To: Ni, Ray <ray.ni@intel.com>; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Sunil V L
> <sunilvl@ventanamicro.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: lichao <lichao@loongson.cn>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; Grimes, Paul <Paul.Grimes@amd.com>; He,
> Jiangang <Jiangang.He@amd.com>; Leif Lindholm <quic_llindhol@quicinc.com>; Andrew Fish <afish@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://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.ni@intel.com>
> > Sent: Thursday, September 29, 2022 3:42 PM
> > To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Chang,
> > Abner <Abner.Chang@amd.com>; Sunil V L <sunilvl@ventanamicro.com>;
> > devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; lichao
> > <lichao@loongson.cn>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>;
> > Grimes, Paul <Paul.Grimes@amd.com>; He, Jiangang
> > <Jiangang.He@amd.com>; Leif Lindholm <quic_llindhol@quicinc.com>;
> > Andrew Fish <afish@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&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.Attar@amd.com>
> > > Sent: Thursday, September 29, 2022 3:11 PM
> > > To: Chang, Abner <Abner.Chang@amd.com>; Sunil V L
> > > <sunilvl@ventanamicro.com>; devel@edk2.groups.io; Ni, Ray
> > > <ray.ni@intel.com>
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; lichao
> > > <lichao@loongson.cn>; Kirkendall, Garrett
> > > <Garrett.Kirkendall@amd.com>; Grimes, Paul <Paul.Grimes@amd.com>;
> > He,
> > > Jiangang <Jiangang.He@amd.com>; Leif Lindholm
> > > <quic_llindhol@quicinc.com>; Andrew Fish <afish@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 <abdattar@amd.com>
> > >
> > > Thanks
> > > AbduL
> > >
> > > -----Original Message-----
> > > From: Chang, Abner <Abner.Chang@amd.com>
> > > Sent: 28 September 2022 20:31
> > > To: Sunil V L <sunilvl@ventanamicro.com>; devel@edk2.groups.io;
> > > ray.ni@intel.com
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; lichao
> > > <lichao@loongson.cn>; Kirkendall, Garrett
> > > <Garrett.Kirkendall@amd.com>; Grimes, Paul <Paul.Grimes@amd.com>;
> > He,
> > > Jiangang <Jiangang.He@amd.com>; Attar, AbdulLateef (Abdul Lateef)
> > > <AbdulLateef.Attar@amd.com>; Leif Lindholm
> > > <quic_llindhol@quicinc.com>; Andrew Fish <afish@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-
> > &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 <sunilvl@ventanamicro.com>
> > > > Sent: Wednesday, September 28, 2022 3:34 PM
> > > > To: devel@edk2.groups.io; ray.ni@intel.com
> > > > Cc: Chang, Abner <Abner.Chang@amd.com>; Kinney, Michael D
> > > > <michael.d.kinney@intel.com>; lichao <lichao@loongson.cn>;
> > > > Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; Grimes, Paul
> > > > <Paul.Grimes@amd.com>; He, Jiangang <Jiangang.He@amd.com>; Attar,
> > > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Leif
> > > > Lindholm <quic_llindhol@quicinc.com>; Andrew Fish <afish@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&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%2Fgith
> > > > 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