[edk2-devel] The principles of EDK2 module reconstruction for archs
Michael D Kinney
michael.d.kinney at intel.com
Fri Sep 30 15:28:49 UTC 2022
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 at amd.com>
> Sent: Friday, September 30, 2022 12:11 AM
> To: Ni, Ray <ray.ni at intel.com>; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar at amd.com>; Sunil V L
> <sunilvl at ventanamicro.com>; devel at edk2.groups.io; Kinney, Michael D <michael.d.kinney at intel.com>
> Cc: 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
>
> [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 (#94575): https://edk2.groups.io/g/devel/message/94575
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]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list