From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-DM6-obe.outbound.protection.outlook.com (NAM04-DM6-obe.outbound.protection.outlook.com [40.107.102.58]) by mx.groups.io with SMTP id smtpd.web12.4108.1664339460670665419 for ; Tue, 27 Sep 2022 21:31:01 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amd.com header.s=selector1 header.b=v/OQfY+Y; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: amd.com, ip: 40.107.102.58, mailfrom: abner.chang@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Nb1Ho9nZDu9Y8UF0QabhghviFh180g8yPH+oj8t716PPeyhzsB8gxDbPRgzGSd0qirjhk3emB+Oh89T4hKxAqynffmRRsEoDpYfGkAPteVe1fUY7m9JNjLSc0QgM64NaUCOpVABHn0SA9e2K6/vnsWiFdxabMjKE9hesHkvTLPSAO0SwmBlgMBDaFiHiiAk6q63S0x1UseDLojtF5EChXPJFbQ+arbI8WlDEWAoHCnkfTA7pLp3bWIZPpLfvji2aZ0wGcXKD9S3/CCB4kHrVOJgytmdTresxarsElR7fDrt0FlOQLRpGmylmp4TwtGNM51165uI5bkj3hfK2Cu5rZw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=F9tMlb5taKvb9dWyDd8osARmBzrsu+S43WlN+/uIsRc=; b=KOZLa8yD8YVxWNKpiaK9olVthyQ1oMWr5NrIz8U7WTq6w6ga1KsmA/FxoSwtak6aERWsStUmiFdqttlBnFDAT9OWUN4xGOCeGcBJcMAXv+A0Qui9msivVowxnoFSkrefIh1LOQh/vdppdtlQwrf+qHu8sZBf+3YcWlTaEwYAkzk+G8UbNmNYfTE7ICYRcrdj1vlo3Cf4oJmAIAZN+4hL+5kXMglAHsyLJ25KW5JkiShMlhTBzNY4ZP9Rqsiim3Zi9zD18WAANVxRfRsEwQTPW6KDnytyFHKb3/WdkyNU+U+T7G/02/i+i+qyR35/EGutCx0+bjI4HIlJz3i4Upge5A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=F9tMlb5taKvb9dWyDd8osARmBzrsu+S43WlN+/uIsRc=; b=v/OQfY+Y8+F1R+GcnmFVd4351jCo+Qgo2Jm7wirW42RLsvEb2bgaydlh+YdCDJ4FWrT1Z9hBin1yZlgxV5NQaEFqtlqr/RGdQI59ecF+E0bmKynRReZDpe5jDWRJ9QsZZHPmnkKd4jc36G2SN0lTHGD64ZaZ+IdzEY0TrFurWYs= Received: from CH2PR12MB3957.namprd12.prod.outlook.com (2603:10b6:610:2c::17) by MN2PR12MB4455.namprd12.prod.outlook.com (2603:10b6:208:265::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5676.17; Wed, 28 Sep 2022 04:30:49 +0000 Received: from CH2PR12MB3957.namprd12.prod.outlook.com ([fe80::950d:72ec:6906:7f19]) by CH2PR12MB3957.namprd12.prod.outlook.com ([fe80::950d:72ec:6906:7f19%6]) with mapi id 15.20.5676.015; Wed, 28 Sep 2022 04:30:49 +0000 From: "Chang, Abner" To: "devel@edk2.groups.io" , "ray.ni@intel.com" CC: "Kinney, Michael D" , Sunil V L , lichao , "Kirkendall, Garrett" , "Grimes, Paul" , "He, Jiangang" , "Attar, AbdulLateef (Abdul Lateef)" , Leif Lindholm , Andrew Fish Subject: Re: [edk2-devel] The principles of EDK2 module reconstruction for archs Thread-Topic: [edk2-devel] The principles of EDK2 module reconstruction for archs Thread-Index: AdjPX4ZZG0rDXS4WTdOIWvoiYOLYcgC9rttQACWvEcA= Date: Wed, 28 Sep 2022 04:30:49 +0000 Message-ID: References: In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_Enabled=true; MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_SetDate=2022-09-28T04:30:46Z; MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_Method=Standard; MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_Name=General; MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_SiteId=3dd8961f-e488-4e60-8e11-a82d994e183d; MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_ActionId=d8d00239-48b2-4a9b-b873-513b943ebc05; MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_ContentBits=1 authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: CH2PR12MB3957:EE_|MN2PR12MB4455:EE_ x-ms-office365-filtering-correlation-id: 8255331a-c982-4565-cb67-08daa10a3873 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: gTcgacxH3EyXrM/msvazGH2BFzi0DlbhW7c/rdUDbBgrYTJgu64BHAbOGMFJ0KJr0giOkorZxl7jmHNLyTRCoO+WHbgHsAKoJw0/4At5gnRi9SajkukJ0KXMPQxwSEwZIu4xJS4l0egpBeMk5GSTFLu4RgaCLyP0tOXA048ZALsyB0MdtiEE4amGlFf3+uRDhpSxQ7wq1PxNBvOJmXtx1Mxcvsn4weqNgwX4T3bO9pB5ZP0Pq51jKYfnxj6mvCXE326UtL2tNvvHv7t3EuTvJcf6pk+21a+4HVB1xNJsmY0kf1TaqEo/u433IwY1BLxVjaM2QkBPB5yyGozW0OdjA3JjQzYVUrzgUTiVE1RC/0gUX8KN0YIEuvhFMurhAzEEpzOL1ytMOooQ51+Hko4B4sqTSEEeOwu9g13rCgoJ7XIYz+pjc3M+qnzIkHpmMaODXpzpTZ0VrcuBgrrHOwKP+UnZaeamis3pK7aR6wnqiKL8DszK56wRvyjeW+0EFKQJUMpCCb/qshISxyXdVvQESyJlP7qwDLwNjihiyT6ElUdr5pBopnYqPLLRUWFtq6dkNzb6YejP8ZiCHJ0tnukluZzZmIZNnCJNjiXdDsIKTrS7y3FHmCCkrY5lDhwPTuCJW3imUi+VFk2OdXw3+S/Z1Q2NQbmGxgXVzqf1UGN/5hIuOEYT8n5FgCN5nE3UOfPUlHaUv5Qr9T6LEm6bolYumfSjfPrrGf8u7NznA+g+1PV2FdKRdQsbg5T5ut/dRZtCF+y6XHLy+6RE0ITYcM4SH84ugiNrqAqs0+rVl30JeMQ= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CH2PR12MB3957.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(4636009)(396003)(39860400002)(376002)(136003)(366004)(346002)(451199015)(83380400001)(186003)(166002)(38100700002)(38070700005)(53546011)(122000001)(2906002)(41300700001)(5660300002)(52536014)(76236004)(55016003)(9686003)(478600001)(71200400001)(76116006)(26005)(6506007)(8936002)(7696005)(66446008)(8676002)(64756008)(66476007)(66556008)(66946007)(4326008)(316002)(54906003)(110136005)(33656002)(86362001)(66899015);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?N79akFSLgw3AceKmdbExEzGdcp9heeQYaLGj2cYcfZcQ3rhtamVaQ7HnM4xG?= =?us-ascii?Q?R+hl7FKoupUZ9aIG1aloSjxMfDbQbyBQN+i3Xy1WF9/q9c4FY6wLA4NLuvHO?= =?us-ascii?Q?+4rqWsrmUdRAwBa6XlIvkfuYBMqL+c+QvtQL0dbeX8tjG275/NGXp0EMQmtL?= =?us-ascii?Q?4pd1i1Oo3cI59JzNkwTD2L9lcrEXtVShqR8m5qZLyTJwgcYtipJc/yE9ONJo?= =?us-ascii?Q?AR/9Ga8k08nZZITyX1uk3PeNhtP9dpjD5pyn5MWM51MHFYTe6G2LvvvKHyKh?= =?us-ascii?Q?oo3zTtLtWcz7WYQhAyahGWC83DccgUWnl4n74BWt0oUARpmLZERJi9mhwQlM?= =?us-ascii?Q?0qYTaW0bLX64U/ZXBO5Ud8bZQUUrTSO0CTvon9uklKqahpU69zo5COWgO9h7?= =?us-ascii?Q?kk7QIpUi/lijq6tv2tH+yZlV5DPuXdslTuHMpG70TKU3fWGewsuYbv7ZbJVO?= =?us-ascii?Q?aqm5QzIsBoE7pGAoyIEvC6FPRrFEG1X3jR0Jdx/tzGzhPmlwcjOBofMgsR8t?= =?us-ascii?Q?n/rzBPXzsdjEPvkPZFM+PLQP0X/kBgauC7uGxzHpU/bgzrgNKhgXoiZ3AIlh?= =?us-ascii?Q?xI9XMvYJ294c29l1tECd1gzxi4ez1/lyU5oyI5BypXulpfoH6vPzx844aI5c?= =?us-ascii?Q?2mLfaancLdUzZcZc+WKTTFXa2NYTCVpJHzM7pHKDG2Lx+KdbEbK6B87G5aWM?= =?us-ascii?Q?4wo1TJhvvnFnUgYVrHdYdyZ9zob5k3Uwmq97W/X94H7nGnFc2P3RYttPx6hE?= =?us-ascii?Q?EINUKQgoCio2KHTbZ85Fd1mlFoSS6C7desSJULQM/1IqBnncJYo7YWERXQcC?= =?us-ascii?Q?eJwN4QlS/vC09qizo8PiC5Pe3Lgbmk5BvtawZ10oEPDejNCG2O8ux2jaEmAE?= =?us-ascii?Q?eMhdBd3LG8wL2nNkiWXca5AaZRYIqNJOMii7ejZhgm1Hdx9hbmXK/tqWbOJA?= =?us-ascii?Q?MPKwAoAtA++UvWrELpIq7SZkLc/wI4L+Znq97IGd8XyJg/uf7oVn8GHMr5tm?= =?us-ascii?Q?VM2dhOiBeGfoO2V+jylU2Y3dca8i1oz2/ek4BgndoPDfCXsfr3cfQsffemJ8?= =?us-ascii?Q?tNsH6ANPUhEmjhcmbItDUrY1lc2bHdtwEQS10EX4Q5qyUa5Te9m8v3TSnutZ?= =?us-ascii?Q?+AoCDZqmjKqrjAIKQCl73XRFTfh69GkQMTeBij8k+g93xNDp+uAspa85irH/?= =?us-ascii?Q?Asp/7WDqiATNUW/liWlC9cO++edxZ0LbE5nLX8TGLh3nDKNReoZ97cxRhgtx?= =?us-ascii?Q?CzNT8a9ocbLhlbzxQzVn14EcJtmN8v/TAehrb7gSsaWZcJ3S5Hjr7Zu2g7uY?= =?us-ascii?Q?WJwYi9KLX8VJ1jfHsKBag/7O3RuL5fYLJuV3aLdyKdOiIEAD1V/5ik+sNzy4?= =?us-ascii?Q?/2wZ9dGb65ZxdIG8xTetIv8jLxleVbl86t6uNJcqJn9sg48ue0/MRuW4L+sv?= =?us-ascii?Q?a0p3TzkK1JYitn5XUMWEdBkUDHoQL9MT5YXZAp68aw69an/sCusxHAD63cYP?= =?us-ascii?Q?O8y5X8dUjqhoXmj9MQGETQYK/0Zr4ZKOtF4zYNZ4ZOHNkl9e8gZoyMSEmphv?= =?us-ascii?Q?wvrzU8AP3uRRujx3ilpHM6qK66HDCr89/wqWcR/G?= MIME-Version: 1.0 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB3957.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8255331a-c982-4565-cb67-08daa10a3873 X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Sep 2022 04:30:49.4735 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: nIsGuL5FNmPxPug8V0Rh/Gv2OMWb294rEFZr9462lYEbVpXytBGe9jdULydiKzKKWQzqHz1BsFtSM5RTyS54DA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4455 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CH2PR12MB395707957F140E403D604524EA549CH2PR12MB3957namp_" --_000_CH2PR12MB395707957F140E403D604524EA549CH2PR12MB3957namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable [AMD Official Use Only - General] From: devel@edk2.groups.io On Behalf Of Ni, Ray via = groups.io Sent: Wednesday, September 28, 2022 11:34 AM To: devel@edk2.groups.io; Chang, Abner Cc: Kinney, Michael D ; Sunil V L ; lichao ; Kirkendall, Garrett ; Grimes, Paul ; He, Jiangang ; Attar, AbdulLateef (Abdul Lateef) ; Leif Lindholm ; Andrew Fish Subject: Re: [edk2-devel] The principles of EDK2 module reconstruction for = archs Caution: This message originated from an External Source. Use proper cautio= n when opening attachments, clicking links, or responding. The [Arch] refers to the Processor Architecture. The [Module] refer to the EDK2 module. The [X86] refers to both IA32 and X64. The principles to create the X86 folder in the module: 1. When X86-vendor's implementation is introduced to the existing module= : 1. The folder reconstruction: A-1. If the module is obviously used by IA32/X64 only * No need to create X86 folder * Create X86-vendor's stuff under the root directory of module A-2. If the existing module is expected to accommodate the different archs = or the module already has multiple archs: * Create X86 folder * Create X86-vendor's stuff under X86 folder [Ray] Looks good. 1. The files reconstruction: B-1. The module INF metafile * The existing INF metafile should be kept without relocation. Should n= ot have the impacts to the existing DSC/FDF file. * The new introduced INF metafile should be located under the root dire= ctory of module with the file naming format as below. This keeps the consis= tent module file structure. * .inf [Ray] "CpuDxe.inf" and "CpuDxeArm.inf"? is that your intention? New develo= pers may be confused that CpuDxe.inf supports only X86 arch. Do you have an example that one module supporting multiple archs using diff= erent INF files? MdeModulePkg/DxeIpl is a module supporting different archs= with the same INF file. Or shall we leave it to be decided between the patch contributor and packag= e/module maintainer? [Chang, Abner] Here I mean the library module, for example SmmCpuFeatureLi= b.inf and AmdSmmCpuFeatureLib.inf. Will make the statement clear. The file = naming above would be changed to .inf as Mike sugge= sted. I am not sure if there is another example having arch-specific INF file. Us= ually the driver module has the abstract interface and the implementation i= n the library module. A newly introduced processor architecture driver may = create it's own module such as ArmCpuDxe if the delta between implementatio= ns is huge. However, I would prefer to have arch-specific INF for the modu= le if the module implementation can be leveraged. And yes, this requires th= e discussion between contributor and module maintainer if the principles ca= n not perfectly identify the case. B-2. Source files The new arch implementation is introduced to = the module in order to leverage the source code and the module design archi= tecture, so * That is contributor's responsibility to review the source code and st= rip the arch-dependent code away and put it into the arch-specific file. Le= ave the common code in the original file if there is no arch-specific and a= rch-specific-feature wordings in the file name. Create a common file for th= e common implementation otherwise. * The file naming for the arch-specific file .* * The file naming for the common implementation < OriginalFileNaming >Common.* * That is contributor's responsibility to relocate the arch-specific so= urce files to the arch-specific folder. * That is contributor's responsibility to make sure the original INF me= tafile can properly pull-in the source file from arch-specific folder after= the source file reconstruction. * The common source files should be located under the root directory of= module [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 s= ome AMD specific check. In my opinion, that's already good enough. [Chang, Abner] I am not quite lean to the If-AMD-else in the source file s= olution. I prefer to separate AMD stuff or vendor feature to a separated fi= le. So we can have the reviewer or maintainer for *Amd* files/module/packag= es specifically. This makes the review responsibility clear and efficient. However, if you check MdeModulePkg/DxeIpl, implementations for different ar= chs are in different *folders*. Can we leave this to the module owner to decide how to place the implementa= tions? 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. [Chang, Abner] We will move the arch implementation to the arch folder with= out moving INF. This wont impact the platform DSC and FDF, however this has= the impact to the existing overwrite. * Create the arch folder for the new introduced arch [Ray] I agree. But if we don't create arch folder for existing arch impleme= ntation, the pkg layout will be a mess. [Chang, Abner] right, so the first bullet is important. [Ray] Hard for me to understand all the principles here. Maybe we review ex= isting code including to-be-upstreamed code and decide how to go. 1. The files reconstruction: B-1. The module INF metafile * The existing INF file should be kept without the relocation. Should n= ot have the impacts to the existing DSC/FDF file. * The new introduced INF metafile should be located under the root dire= ctory of module with the file naming format as below. This keeps the consis= tent module file structure. * < OriginalFileNaming>.inf B-2. Source files The new arch implementation is introduced to t= his module in order to leverage the source code and the module design archi= tecture, so * That is contributor's responsibility to review the source code and st= rip the arch-dependent code away and put it into the arch-specific file. Le= ave the common code in the original file if there is no arch-specific wordi= ng in the file name. Create a common file for the common implementation oth= erwise. * The file naming for the arch-specific source file < OriginalFileNaming >.* * The file naming for the common implementation Common.* * That is contributor's responsibility to relocate the arch-specific so= urce files to the arch-specific folder. * That is contributor's responsibility to make sure the original INF me= tafile can properly pull-in the source file from arch-specific folder after= the source file reconstruction. * The common source files should be located under the root directory of= module 1. When a new arch implementation has a huge delta with the original imp= lementation Create a separate module instance. The naming of the module should follow b= elow format, * Add the arch prefix with the original module name: < OriginalModuleNaming> --_000_CH2PR12MB395707957F140E403D604524EA549CH2PR12MB3957namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

[AMD Officia= l Use Only - General]

 

 

 

From: devel@edk2.groups.io <devel@edk2.gro= ups.io> On Behalf Of Ni, Ray via groups.io
Sent: Wednesday, September 28, 2022 11:34 AM
To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Sunil V L = <sunilvl@ventanamicro.com>; lichao <lichao@loongson.cn>; Kirken= dall, Garrett <Garrett.Kirkendall@amd.com>; Grimes, Paul <Paul.Gri= mes@amd.com>; He, Jiangang <Jiangang.He@amd.com>; Attar, AbdulLate= ef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Leif Lindholm <quic_l= lindhol@quicinc.com>; Andrew Fish <afish@apple.com>
Subject: Re: [edk2-devel] The principles of EDK2 module reconstructi= on for archs

 

Caution: This message originated from an External Source. Use proper caution= when opening attachments, clicking links, or responding.

 

The [Arch] refers to the Processor Architecture.

The [Module] refer to the EDK2 module.

The [X86] refers to both IA32 and X64.

The principles to create the X86 folder in the mo= dule:

  1. When X86-vendor’s implementation is introduced to the existi= ng module:
  1. The folder reconstruction:

A-1.= If the module is obviously used by IA32/X64 only

  • No need to create X86 folder
  • Create X86-vend= or’s stuff under the root directory of module

A-2.= If the existing module is expected to accommodate the different archs or t= he module already has multiple archs:

  • Create X86 folder
  • Create X86-vendor’s<= b> stuff under X86 folder

[Ray] Looks good.=

 

  1. The files reconstruction:

B-1. The module INF m= etafile

  • The existing INF metafile should be kept without relocation. Should= not have the impacts to the existing DSC/FDF file.
  • = 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 m= odule file structure.
    • <OriginalFileName><arch>.inf

[Ray]  “Cpu= Dxe.inf” and “CpuDxeArm.inf”? is that your intention? New= developers may be confused that CpuDxe.inf supports only X86 arch.

Do you have an example= that one module supporting multiple archs using different INF files? MdeMo= dulePkg/DxeIpl is a module supporting different archs with the same INF fil= e.

Or shall we leave it t= o be decided between the patch contributor and package/module maintainer?

[Chang, Abner]  Here I mean the library m= odule, for example SmmCpuFeatureLib.inf and AmdSmmCpuFeatureLib.inf. Will m= ake the statement clear. The file naming above would be changed to <arch><OriginalFileName>.inf as Mike suggested.

 

I am not sure if there is another example havi= ng arch-specific INF file. Usually the driver module has the abstract inter= face and the implementation in the library module. A newly introduced proce= ssor architecture driver may create it’s own module such as ArmCpuDxe if the delta between implementatio= ns  is huge. However, I would prefer to have arch-specific INF for the= module if the module implementation can be leveraged. And yes, this requir= es the discussion between contributor and module maintainer if the principles can not perfectly identify the case.<= o:p>

 

        &= nbsp;           &nbs= p;B-2. Source files

        &= nbsp;           &nbs= p;         The new arch impleme= ntation is introduced to the module in order to leverage the source code an= d the module design architecture, so

  • That is contributor’s responsibility to review the source cod= e 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 th= e file name. Create a common file for the common implementation otherwise.<= b>
    • The file naming for the arch-specific file

<OriginalFile= Name ><arch>.*

    • The file naming for the common implementation

< OriginalFil= eNaming >Common.*

  • That is contributor’s responsibility to relocate the arch-spe= cific source files to the arch-specific folder.
  • That= is contributor’s responsibility to make sure the original INF metafi= le can properly pull-in the source file from arch-specific folder after the= source file reconstruction.
  • The common source files= should be located under the root directory of module

[Ray] If you check the= MpInitLib, most of SEV logic are moved to AmdSev.c. But MpLib.c still cont= ains logic to call functions from AmdSev.c based on some AMD specific check= . In my opinion, that’s already good enough.

[Chang, Abner]  I am not quite lean to th= e If-AMD-else in the source file solution. I prefer to separate AMD stuff o= r vendor feature to a separated file. So we can have the reviewer or mainta= iner for *Amd* files/module/packages specifically. This makes the review responsibility clear and efficient.

 

However, if you check = MdeModulePkg/DxeIpl, implementations for different archs are in different *= folders*.

Can we leave this to t= he module owner to decide how to place the implementations?

 

 

 

  1. When a new arch’s implementation is introduced to the existi= ng module which was developed for the specific arch:
    1. The folder reconstruction:
    • Create arch folder for the existing arch implementation<= /li>

    [Ray] Do you move exis= ting arch implementation to that arch folder? It will break existing platfo= rms a lot.

    [Chang, Abner] We will move the arch implement= ation to the arch folder without moving INF. This wont impact the platform = DSC and FDF, however this has the impact to the existing overwrite.=

    • Create the arch folder for the new introduced arch
      • <= /ul>

      [Ray] I agree. But if = we don’t create arch folder for existing arch implementation, the pkg= layout will be a mess.

      [Chang, Abner] right, so the first bullet is i= mportant.

       

      [Ray] Hard for me to u= nderstand all the principles here. Maybe we review existing code including = to-be-upstreamed code and decide how to go.

       

      1. The files reconstruction:

      B-1. The modul= e INF metafile

      • The existing INF file should be kept without the relocation. Should= not have the impacts to the existing DSC/FDF file.
      • = 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 m= odule file structure.
        • < OriginalFileNaming><arch>.inf

       

      B-2. Source fi= les

              &nbs= p;            &= nbsp;       The new arch implementation is in= troduced to this module in order to leverage the source code and the module= design architecture, so

      • That is contributor’s responsibility to review the source cod= e 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 commo= n file for the common implementation otherwise.
        • The file naming for the arch-specific source file

        < OriginalFil= eNaming ><arch>.*

          • The file naming for the common implementation

        <OriginalFile= Naming>Common.*

        • That is contributor’s responsibility to relocate the arch-spe= cific source files to the arch-specific folder.
        • That= is contributor’s responsibility to make sure the original INF metafi= le can properly pull-in the source file from arch-specific folder after the= source file reconstruction.
        • The common source files= should be located under the root directory of module

         

        1. When a new arch implementation has a huge delta with the original = implementation

        Create a separate module instance. The naming= of the module should follow below format,

        • Add the arch prefix with the original module name:
          • <= /ul>

          < OriginalMod= uleNaming><arch>

           

--_000_CH2PR12MB395707957F140E403D604524EA549CH2PR12MB3957namp_--