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

["Ni, Ray" <ray.ni@intel.com>]

Abner,
Comments in https://github.com/tianocore-docs/edk2-CCodingStandardsSpecification/pull/2#pullrequestreview-1124763311

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://github.com/tianocore-docs/edk2-
> 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