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

Fri Sep 30 07:10:44 UTC 2022

[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 at intel.com>
> Sent: Thursday, September 29, 2022 3:42 PM
> To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar at amd.com>; Chang,
> Abner <Abner.Chang at amd.com>; Sunil V L <sunilvl at ventanamicro.com>;
> devel at edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney at intel.com>; lichao
> <lichao at loongson.cn>; Kirkendall, Garrett <Garrett.Kirkendall at amd.com>;
> Grimes, Paul <Paul.Grimes at amd.com>; He, Jiangang
> <Jiangang.He at amd.com>; Leif Lindholm <quic_llindhol at quicinc.com>;
> Andrew Fish <afish at 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.Attar at amd.com>
> > Sent: Thursday, September 29, 2022 3:11 PM
> > To: Chang, Abner <Abner.Chang at amd.com>; Sunil V L
> > <sunilvl at ventanamicro.com>; devel at edk2.groups.io; Ni, Ray
> > <ray.ni at intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney at intel.com>; lichao
> > <lichao at loongson.cn>; Kirkendall, Garrett
> > <Garrett.Kirkendall at amd.com>; Grimes, Paul <Paul.Grimes at amd.com>;
> He,
> > Jiangang <Jiangang.He at amd.com>; Leif Lindholm
> > <quic_llindhol at quicinc.com>; Andrew Fish <afish at 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 at amd.com>
> >
> > Thanks
> > AbduL
> >
> > -----Original Message-----
> > From: Chang, Abner <Abner.Chang at amd.com>
> > Sent: 28 September 2022 20:31
> > To: Sunil V L <sunilvl at ventanamicro.com>; devel at edk2.groups.io;
> > ray.ni at intel.com
> > Cc: Kinney, Michael D <michael.d.kinney at intel.com>; lichao
> > <lichao at loongson.cn>; Kirkendall, Garrett
> > <Garrett.Kirkendall at amd.com>; Grimes, Paul <Paul.Grimes at amd.com>;
> He,
> > Jiangang <Jiangang.He at amd.com>; Attar, AbdulLateef (Abdul Lateef)
> > <AbdulLateef.Attar at amd.com>; Leif Lindholm
> > <quic_llindhol at quicinc.com>; Andrew Fish <afish at 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 <sunilvl at ventanamicro.com>
> > > Sent: Wednesday, September 28, 2022 3:34 PM
> > > To: devel at edk2.groups.io; ray.ni at intel.com
> > > Cc: Chang, Abner <Abner.Chang at amd.com>; Kinney, Michael D
> > > <michael.d.kinney at intel.com>; lichao <lichao at loongson.cn>;
> > > Kirkendall, Garrett <Garrett.Kirkendall at amd.com>; Grimes, Paul
> > > <Paul.Grimes at amd.com>; He, Jiangang <Jiangang.He at amd.com>; Attar,
> > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar at amd.com>; Leif
> > > Lindholm <quic_llindhol at quicinc.com>; Andrew Fish <afish at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-