<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="#954F72" style="word-wrap:break-word"> <div class="WordSection1"> Abner,<o:p></o:p> I think we Mike’s first email regarding the package structure is a good guideline regarding how to place the modules.<o:p></o:p> <o:p> </o:p> What we are discussing is how to organize the module internal contents for different archs. Do you agree?<o:p></o:p> I want to clarify this so we have a clear boundary between the two topics.<o:p></o:p> <o:p> </o:p> However, Mike’s proposal also defines the <arch> folder inside a module directory.<o:p></o:p> Maybe we just work together to refine Mike’s proposal and document it in EDKII coding standard document.<o:p></o:p> <o:p> </o:p> Chao, can you check Mike’s proposal and raise if there is anything not captured?<o:p></o:p> <o:p> </o:p> Thanks,<o:p></o:p> Ray<o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, Abner via groups.io Sent: Wednesday, September 28, 2022 12:31 PM To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Sunil V L <sunilvl@ventanamicro.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<o:p></o:p> </div> </div> <o:p> </o:p> [AMD Official Use Only - General]<o:p></o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> <div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a> <<a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a>> On Behalf Of Ni, Ray via groups.io Sent: Wednesday, September 28, 2022 11:34 AM To: <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a>; Chang, Abner <<a href="mailto:Abner.Chang@amd.com">Abner.Chang@amd.com</a>> Cc: Kinney, Michael D <<a href="mailto:michael.d.kinney@intel.com">michael.d.kinney@intel.com</a>>; Sunil V L <<a href="mailto:sunilvl@ventanamicro.com">sunilvl@ventanamicro.com</a>>; lichao <<a href="mailto:lichao@loongson.cn">lichao@loongson.cn</a>>; Kirkendall, Garrett <<a href="mailto:Garrett.Kirkendall@amd.com">Garrett.Kirkendall@amd.com</a>>; Grimes, Paul <<a href="mailto:Paul.Grimes@amd.com">Paul.Grimes@amd.com</a>>; He, Jiangang <<a href="mailto:Jiangang.He@amd.com">Jiangang.He@amd.com</a>>; Attar, AbdulLateef (Abdul Lateef) <<a href="mailto:AbdulLateef.Attar@amd.com">AbdulLateef.Attar@amd.com</a>>; Leif Lindholm <<a href="mailto:quic_llindhol@quicinc.com">quic_llindhol@quicinc.com</a>>; Andrew Fish <<a href="mailto:afish@apple.com">afish@apple.com</a>> Subject: Re: [edk2-devel] The principles of EDK2 module reconstruction for archs<o:p></o:p> </div> </div> <o:p> </o:p> <table class="MsoNormalTable" border="0" cellspacing="0" cellpadding="0" align="left" width="100%" style="width:100.0%"> <tbody> <tr> <td style="background:#FFB900;padding:5.0pt 2.0pt 5.0pt 2.0pt"></td> <td width="100%" style="width:100.0%;background:#FFF8E5;padding:5.0pt 4.0pt 5.0pt 12.0pt"> <div> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. <o:p></o:p> </div> </td> </tr> </tbody> </table> <o:p> </o:p> <div> The [Arch] refers to the Processor Architecture.<o:p></o:p> The [Module] refer to the EDK2 module.<o:p></o:p> The [X86] refers to both IA32 and X64.<o:p></o:p> The principles to create the X86 folder in the module:<o:p></o:p> <ol style="margin-top:0in" start="1" type="1"> <li class="MsoListParagraph" style="margin-left:0in;mso-list:l1 level1 lfo1">When X86-vendor’s implementation is introduced to the existing module:<o:p></o:p></li></ol> <ol style="margin-top:0in" start="1" type="A"> <li class="MsoListParagraph" style="margin-left:.25in;mso-list:l2 level1 lfo2">The folder reconstruction:<o:p></o:p></li></ol> A-1. If the module is obviously used by IA32/X64 only<o:p></o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">No need to create X86 folder<o:p></o:p></li><li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">Create X86-vendor’s stuff under the root directory of module<o:p></o:p></li></ul> A-2. If the existing module is expected to accommodate the different archs or the module already has multiple archs:<o:p></o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">Create X86 folder<o:p></o:p></li><li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">Create X86-vendor’s stuff under X86 folder<o:p></o:p></li></ul> [Ray] Looks good.<o:p></o:p> <o:p> </o:p> <ol style="margin-top:0in" start="2" type="A"> <li class="MsoListParagraph" style="margin-left:.25in;mso-list:l2 level1 lfo2">The files reconstruction:<o:p></o:p></li></ol> B-1. The module INF metafile<o:p></o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">The existing INF metafile should be kept without relocation. Should not have the impacts to the existing DSC/FDF file.<o:p></o:p></li><li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">The new introduced INF metafile should be located under the root directory of module with the file naming format as below. This keeps the consistent module file structure.<o:p></o:p></li></ul> <ul style="margin-top:0in" type="disc"> <ul style="margin-top:0in" type="circle"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level2 lfo3"><OriginalFileName><arch>.inf<o:p></o:p></li></ul> </ul> [Ray] “CpuDxe.inf” and “CpuDxeArm.inf”? is that your intention? New developers may be confused that CpuDxe.inf supports only X86 arch.<o:p></o:p> Do you have an example that one module supporting multiple archs using different INF files? MdeModulePkg/DxeIpl is a module supporting different archs with the same INF file.<o:p></o:p> Or shall we leave it to be decided between the patch contributor and package/module maintainer?<o:p></o:p> [Chang, Abner] Here I mean the library module, for example SmmCpuFeatureLib.inf and AmdSmmCpuFeatureLib.inf. Will make the statement clear. The file naming above would be changed to <arch><OriginalFileName>.inf as Mike suggested.<o:p></o:p> <o:p> </o:p> I am not sure if there is another example having arch-specific INF file. Usually the driver module has the abstract interface and the implementation in the library module. A newly introduced processor architecture driver may create it’s own module such as ArmCpuDxe if the delta between implementations is huge. However, I would prefer to have arch-specific INF for the module if the module implementation can be leveraged. And yes, this requires the discussion between contributor and module maintainer if the principles can not perfectly identify the case.<o:p></o:p> <o:p> </o:p> B-2. Source files<o:p></o:p> The new arch implementation is introduced to the module in order to leverage the source code and the module design architecture, so<o:p></o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">That is contributor’s responsibility to review the source code and strip the arch-dependent code away and put it into the arch-specific file. Leave the common code in the original file if there is no arch-specific and arch-specific-feature wordings in the file name. Create a common file for the common implementation otherwise.<o:p></o:p></li></ul> <ul style="margin-top:0in" type="disc"> <ul style="margin-top:0in" type="circle"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level2 lfo3">The file naming for the arch-specific file<o:p></o:p></li></ul> </ul> <OriginalFileName ><arch>.*<o:p></o:p> <ul style="margin-top:0in" type="disc"> <ul style="margin-top:0in" type="circle"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level2 lfo3">The file naming for the common implementation<o:p></o:p></li></ul> </ul> < OriginalFileNaming >Common.*<o:p></o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">That is contributor’s responsibility to relocate the arch-specific source files to the arch-specific folder.<o:p></o:p></li><li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">That is contributor’s responsibility to make sure the original INF metafile can properly pull-in the source file from arch-specific folder after the source file reconstruction.<o:p></o:p></li><li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">The common source files should be located under the root directory of module<o:p></o:p></li></ul> [Ray] If you check the MpInitLib, most of SEV logic are moved to AmdSev.c. But MpLib.c still contains logic to call functions from AmdSev.c based on some AMD specific check. In my opinion, that’s already good enough.<o:p></o:p> [Chang, Abner] I am not quite lean to the If-AMD-else in the source file solution. I prefer to separate AMD stuff or vendor feature to a separated file. So we can have the reviewer or maintainer for *Amd* files/module/packages specifically. This makes the review responsibility clear and efficient.<o:p></o:p> [Ray] You can take a look at the MpInitLib. There are some code paths that are AMD only. I agree to have dedicated reviewers from different vendors if the implementations are different.<o:p></o:p> <o:p> </o:p> However, if you check MdeModulePkg/DxeIpl, implementations for different archs are in different *folders*.<o:p></o:p> Can we leave this to the module owner to decide how to place the implementations?<o:p></o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> <ol style="margin-top:0in" start="2" type="1"> <li class="MsoListParagraph" style="margin-left:0in;mso-list:l1 level1 lfo1">When a new arch’s implementation is introduced to the existing module which was developed for the specific arch:<o:p></o:p></li></ol> <ol style="margin-top:0in" start="1" type="A"> <li class="MsoListParagraph" style="margin-left:.25in;mso-list:l3 level1 lfo4">The folder reconstruction:<o:p></o:p></li></ol> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">Create arch folder for the existing arch implementation<o:p></o:p></li></ul> [Ray] Do you move existing arch implementation to that arch folder? It will break existing platforms a lot.<o:p></o:p> [Chang, Abner] We will move the arch implementation to the arch folder without moving INF. This wont impact the platform DSC and FDF, however this has the impact to the existing overwrite.<o:p></o:p> [Ray] I might misunderstand your idea as “UefiCpuPkg/CpuDxe -> UefiCpuPkg/X86/CpuDxe”.<o:p></o:p> Now I guess you tries to say: “UefiCpuPkg/CpuDxe -> UefiCpuPkg/CpuDxe/X86”. That looks good to me.<o:p></o:p> We need a more clear statement here😊<o:p></o:p> <o:p> </o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">Create the arch folder for the new introduced arch<o:p></o:p></li></ul> [Ray] I agree. But if we don’t create arch folder for existing arch implementation, the pkg layout will be a mess.<o:p></o:p> [Chang, Abner] right, so the first bullet is important.<o:p></o:p> <o:p> </o:p> [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.<o:p></o:p> <o:p> </o:p> <ol style="margin-top:0in" start="2" type="A"> <li class="MsoListParagraph" style="margin-left:.25in;mso-list:l3 level1 lfo4">The files reconstruction:<o:p></o:p></li></ol> B-1. The module INF metafile<o:p></o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">The existing INF file should be kept without the relocation. Should not have the impacts to the existing DSC/FDF file.<o:p></o:p></li><li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">The new introduced INF metafile should be located under the root directory of module with the file naming format as below. This keeps the consistent module file structure.<o:p></o:p></li></ul> <ul style="margin-top:0in" type="disc"> <ul style="margin-top:0in" type="circle"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level2 lfo3">< OriginalFileNaming><arch>.inf<o:p></o:p></li></ul> </ul> <o:p> </o:p> B-2. Source files<o:p></o:p> The new arch implementation is introduced to this module in order to leverage the source code and the module design architecture, so<o:p></o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">That is contributor’s responsibility to review the source code and strip the arch-dependent code away and put it into the arch-specific file. Leave the common code in the original file if there is no arch-specific wording in the file name. Create a common file for the common implementation otherwise.<o:p></o:p></li></ul> <ul style="margin-top:0in" type="disc"> <ul style="margin-top:0in" type="circle"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level2 lfo3">The file naming for the arch-specific source file<o:p></o:p></li></ul> </ul> < OriginalFileNaming ><arch>.*<o:p></o:p> <ul style="margin-top:0in" type="disc"> <ul style="margin-top:0in" type="circle"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level2 lfo3">The file naming for the common implementation<o:p></o:p></li></ul> </ul> <OriginalFileNaming>Common.*<o:p></o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">That is contributor’s responsibility to relocate the arch-specific source files to the arch-specific folder.<o:p></o:p></li><li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">That is contributor’s responsibility to make sure the original INF metafile can properly pull-in the source file from arch-specific folder after the source file reconstruction.<o:p></o:p></li><li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">The common source files should be located under the root directory of module<o:p></o:p></li></ul> <o:p> </o:p> <ol style="margin-top:0in" start="3" type="1"> <li class="MsoListParagraph" style="margin-left:0in;mso-list:l1 level1 lfo1">When a new arch implementation has a huge delta with the original implementation<o:p></o:p></li></ol> Create a separate module instance. The naming of the module should follow below format,<o:p></o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:.75in;mso-list:l0 level1 lfo3">Add the arch prefix with the original module name:<o:p></o:p></li></ul> < OriginalModuleNaming><arch><o:p></o:p> <o:p> </o:p> </div> </div> <div> </o:p> </div> </div> </body> </html> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links: You receive all messages sent to this group. <a target="_blank" href="https://edk2.groups.io/g/devel/message/94452">View/Reply Online (#94452)</a> | | <a target="_blank" href="https://groups.io/mt/93872791/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com] <div width="1" style="color:white;clear:both">_._,_._,_</div>