From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mx.groups.io with SMTP id smtpd.web12.2187.1574884992657645235 for ; Wed, 27 Nov 2019 12:03:12 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=corsOm+J; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: michael.a.kubacki@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Nov 2019 12:03:11 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,250,1571727600"; d="scan'208";a="217440592" Received: from orsmsx106.amr.corp.intel.com ([10.22.225.133]) by fmsmga001.fm.intel.com with ESMTP; 27 Nov 2019 12:03:10 -0800 Received: from orsmsx114.amr.corp.intel.com (10.22.240.10) by ORSMSX106.amr.corp.intel.com (10.22.225.133) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 27 Nov 2019 12:03:10 -0800 Received: from ORSEDG001.ED.cps.intel.com (10.7.248.4) by ORSMSX114.amr.corp.intel.com (10.22.240.10) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 27 Nov 2019 12:03:03 -0800 Received: from NAM05-CO1-obe.outbound.protection.outlook.com (104.47.48.54) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 27 Nov 2019 12:03:03 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ErG21l0TJKq6hNU2C8WFCEGGzbpKDSYrBKrtvz1L/GGvKwPpYw7sSHacbGeteXKzcOWWpDS0RVfq0QFv3f2lflGla9czHrgxojXjSEGt/iWZGgWi5YLJ2+JO6gX37QqKTYEsY9s1MLz/n4jTeUMsPGRJFFAYNFCEkl2poACu1GMs024opRL2X+znRMhWxJTtRDgoPo5JUPqgb7FyMjEUb/SAQAp3O2Lj2KArL6bgM05VWlYmxVXCnywInYaHJ8mES+sNS+nW7+WxtB/pLjBvw8/TKbSTRmk1sS61AmE6oWssv86N+FsdsoF2a0QHzIq2EoFTSSwc2IufMG1iyrlUxQ== 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-SenderADCheck; bh=cN+JZb63LKu45pNo9FpJfhFMGL7s6nQZd1TXBkHhqqI=; b=e/6VCq0u9IrLCiLWRwQs28PRDDKr8XMa5tOaoRNaZ+K66cpFmsJ5HHtNpZNGVH3LuJ2XTGQ19sASvCJJtwzSM5PQA6C9wef3wKsfxXuQQzUwBu5EAof+BFKGWGJ7Vz1zomJ6e2pm/RO5DNv3aKdC4HFGomRr7NCe1zAfC5anW1tDuBKvz8376GctIP8KiBIAJVXXU46WbSLBmJM7lHOtgpPCtXgAvX+WTVwS3FLq4BGiFU1VY9aruQjQxB0/7ghTQQOKCi8CEUVoq17xNgSNtZE2Xl75BaRd+370jPjSlpUMBDMcnzocWl5EfG3Uzj/k+/Mb9CWs27I1TLXBmkbTTg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=cN+JZb63LKu45pNo9FpJfhFMGL7s6nQZd1TXBkHhqqI=; b=corsOm+JI+6vroaDnKR0M5sjJxfYUfNjykExXjXWTLEflbQDWHGwC4C9wT7olmyaOjnaIVGroT6/3ah/D59SgIq23rJRGmh4O+/BR/L/Q2UAqPbwWXtQWIl1uIMXmr5Acfhy21iNkY7n1rmZnguQxy+qfAvqToGjK20r4n8K6gs= Received: from BY5PR11MB4484.namprd11.prod.outlook.com (52.132.254.155) by BY5PR11MB4039.namprd11.prod.outlook.com (10.255.161.212) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2474.21; Wed, 27 Nov 2019 20:03:02 +0000 Received: from BY5PR11MB4484.namprd11.prod.outlook.com ([fe80::a114:604b:7ca3:5420]) by BY5PR11MB4484.namprd11.prod.outlook.com ([fe80::a114:604b:7ca3:5420%7]) with mapi id 15.20.2474.023; Wed, 27 Nov 2019 20:03:02 +0000 From: "Kubacki, Michael A" To: "devel@edk2.groups.io" , "philmd@redhat.com" CC: "Bi, Dandan" , "Gao, Liming" , "Wang, Jian J" , "Wu, Hao A" Subject: Re: [edk2-devel] [PATCH V1 2/2] MdeModulePkg PeiCore: Improve comment semantics Thread-Topic: [edk2-devel] [PATCH V1 2/2] MdeModulePkg PeiCore: Improve comment semantics Thread-Index: AQHVpRiYDcVqTugczE2rvhUYe/j2uqefZW1g Date: Wed, 27 Nov 2019 20:03:01 +0000 Message-ID: References: <20191127040648.8656-1-michael.a.kubacki@intel.com> <20191127040648.8656-3-michael.a.kubacki@intel.com> <6dc9556d-8ef2-00d2-12cc-c2d8a84d9e06@redhat.com> In-Reply-To: <6dc9556d-8ef2-00d2-12cc-c2d8a84d9e06@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiN2E1ZjE4NGUtZWVjNy00NjFhLTg2ZjgtODc1NWQwOGE4N2VmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibXdtVFJtSTU5WmVoVnFNN0Y2Q2p2K3ZobVpwRndRRzZBSlZoMEdKT2c4UVFiYTFIZVRJZHpwWkpqK2FhWjZ2QiJ9 dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: spf=none (sender IP is ) smtp.mailfrom=michael.a.kubacki@intel.com; x-originating-ip: [134.134.136.217] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 2688d05d-2f7d-489f-b9d1-08d77374cefc x-ms-traffictypediagnostic: BY5PR11MB4039: x-ms-exchange-purlcount: 3 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:3968; x-forefront-prvs: 023495660C x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(136003)(39860400002)(376002)(346002)(396003)(366004)(189003)(199004)(13464003)(107886003)(478600001)(966005)(55016002)(26005)(7736002)(6506007)(53546011)(99286004)(81156014)(305945005)(446003)(2906002)(6246003)(6306002)(9686003)(66066001)(11346002)(25786009)(110136005)(54906003)(316002)(4326008)(186003)(6436002)(74316002)(3846002)(71190400001)(86362001)(71200400001)(102836004)(52536014)(5660300002)(8936002)(66946007)(66446008)(76176011)(66476007)(66556008)(6116002)(14444005)(2501003)(7696005)(81166006)(76116006)(229853002)(64756008)(8676002)(14454004)(256004)(33656002);DIR:OUT;SFP:1102;SCL:1;SRVR:BY5PR11MB4039;H:BY5PR11MB4484.namprd11.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: L2UIWiEPfW8tTIeVcJFJ9zZiQ5j/To5FsKNg3wAannvyiv1u78rpUvqCK97wBtawlOJJD4BqUdg71ghEdnbC+UIUmcw6DM0/FqbhqVRZSsmHHlhYN6/gSHSIYpW5ugpn5qs4C63U53uMYbu2IRa23BOfV3p+ks2MKCwqG5dXVhNM1S3zXWCSf+Wsl5tP9Raiil39glOeX2chYf4hwd5oQcYr+i0w3VdbMLdjAMZ9fVJRZEZJjnwuMmaoVsxQJk0gm1nxBUnfsKF2+BXFxtNcXR8OgNDfb2bDmJ4js7u6fu1f/JDmkzQTUiSl7eOCCtt/0/GNJkWdmDgq4GSAvQgWn5XP4aWJO28J34xktsq/lR0Hwczbsar0WK+C6Po/gMPi7yuTuOq/mEzqmyjPC2nvpGqsROe/ZYtMgmM6pPyZDgHC5jTDLUW2+wEVysnW5s/7tWU1iYgkP/KQWe2wLD3jRL+sMFhWVP8Gy3E2AuUQzxU= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 2688d05d-2f7d-489f-b9d1-08d77374cefc X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Nov 2019 20:03:01.8276 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: xJcAvy45dj0RpQgb0fZnRnJn6HQ6Ag5nflGa4NC4jMs96CjiD1WRG6iuqFDSmJFSNJ87rStEWOD3dwHSoy+iMphwSMRAiGtwm83p0r5fWsM= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR11MB4039 Return-Path: michael.a.kubacki@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Hi Philippe, Thanks for your suggestions. I incorporated some of the suggestions into t= he V2 series. Thanks, Michael > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Philippe > Mathieu-Daud=E9 > Sent: Wednesday, November 27, 2019 3:49 AM > To: devel@edk2.groups.io; Kubacki, Michael A > > Cc: Bi, Dandan ; Gao, Liming = ; > Wang, Jian J ; Wu, Hao A > Subject: Re: [edk2-devel] [PATCH V1 2/2] MdeModulePkg PeiCore: Improve > comment semantics >=20 > On 11/27/19 5:06 AM, Kubacki, Michael A via Groups.Io wrote: > > Clarifies wording in several PeiCore comments to improve >=20 > "Clarify"? > =20 I believe an implied subject noun led to some confusion so I just explicit= ly included that in the V2 message. > > reading comprehension. > > > > Cc: Dandan Bi > > Cc: Liming Gao > > Cc: Jian J Wang > > Cc: Hao A Wu > > Signed-off-by: Michael Kubacki > > --- > > MdeModulePkg/Core/Pei/FwVol/FwVol.h | 4 ++-- > > MdeModulePkg/Core/Pei/PeiMain.h | 11 +++++----- > > MdeModulePkg/Core/Pei/Dependency/Dependency.c | 4 ++-- > > MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 4 ++-- > > MdeModulePkg/Core/Pei/FwVol/FwVol.c | 23 ++++++++++-------= --- > > MdeModulePkg/Core/Pei/Memory/MemoryServices.c | 4 ++-- > > 6 files changed, 26 insertions(+), 24 deletions(-) > > > > diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.h > > b/MdeModulePkg/Core/Pei/FwVol/FwVol.h > > index 263f0d7a56..8aaf84870b 100644 > > --- a/MdeModulePkg/Core/Pei/FwVol/FwVol.h > > +++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.h > > @@ -303,9 +303,9 @@ FindFileEx ( > > ); > > > > /** > > - Report the information for a new discovered FV in unknown format. > > + Report the information for a newly discovered FV in an unknown form= at. > > > > - If the EFI_PEI_FIRMWARE_VOLUME_PPI has not been installed for > > specific FV format, but > > + If the EFI_PEI_FIRMWARE_VOLUME_PPI has not been installed for a > > + third-party FV format, but > > the FV in this FV format has been discovered, then the information= of > this FV > > will be cached into PEI_CORE_INSTANCE's UnknownFvInfo array. > > Also a notification would be installed for unknown FV format GUID, > > if EFI_PEI_FIRMWARE_VOLUME_PPI diff --git > > a/MdeModulePkg/Core/Pei/PeiMain.h > b/MdeModulePkg/Core/Pei/PeiMain.h > > index 3f61247a0f..96d6df0485 100644 > > --- a/MdeModulePkg/Core/Pei/PeiMain.h > > +++ b/MdeModulePkg/Core/Pei/PeiMain.h > > @@ -1217,8 +1217,8 @@ PeiFfsGetVolumeInfo ( > > ); > > > > /** > > - This routine enable a PEIM to register itself to shadow when PEI > > Foundation > > - discovery permanent memory. > > + This routine enables a PEIM to register itself for shadow when the > > + PEI Foundation discovers permanent memory. > > > > @param FileHandle File handle of a PEIM. > > > > @@ -1314,12 +1314,13 @@ ProcessFvFile ( > > ); > > > > /** > > - Get instance of PEI_CORE_FV_HANDLE for next volume according to > given index. > > + Gets a PEI_CORE_FV_HANDLE instance for the next volume according to > the given index. >=20 > "Get"? >=20 I left as-is in V2. You can see similar usage in the majority of function = descriptions in BaseLib.h for example. > > > > - This routine also will install FvInfo PPI for FV HOB in PI ways. > > + This routine also will install an instance of the FvInfo PPI for > > + the FV HOB as defined in the PI specification. > > > > @param Private Pointer of PEI_CORE_INSTANCE > > - @param Instance The index of FV want to be searched. > > + @param Instance The index of the FV to search. >=20 > Maybe without "The" and trailing dot? >=20 Done in V2. > > > > @return Instance of PEI_CORE_FV_HANDLE. > > **/ > > diff --git a/MdeModulePkg/Core/Pei/Dependency/Dependency.c > > b/MdeModulePkg/Core/Pei/Dependency/Dependency.c > > index 9a8353aef2..b53e5f2686 100644 > > --- a/MdeModulePkg/Core/Pei/Dependency/Dependency.c > > +++ b/MdeModulePkg/Core/Pei/Dependency/Dependency.c > > @@ -2,8 +2,8 @@ > > PEI Dispatcher Dependency Evaluator > > > > This routine evaluates a dependency expression > > (DEPENDENCY_EXPRESSION) to determine > > - if a driver can be scheduled for execution. The criteria for > > - schedulability is that the dependency expression is satisfied. > > + if a driver can be scheduled for execution. The criteria to be > > + scheduled is that the dependency expression is satisfied. > > > > Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. > > SPDX-License-Identifier: BSD-2-Clause-Patent diff --git > > a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > > b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > > index c9f2a91264..a18ac47f61 100644 > > --- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > > +++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > > @@ -1347,8 +1347,8 @@ DepexSatisfied ( > > } > > > > /** > > - This routine enable a PEIM to register itself to shadow when PEI > > Foundation > > - discovery permanent memory. > > + This routine enables a PEIM to register itself for shadow when the > > + PEI Foundation discovers permanent memory. > > > > @param FileHandle File handle of a PEIM. > > > > diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c > > b/MdeModulePkg/Core/Pei/FwVol/FwVol.c > > index c21eb9c039..a9fa476846 100644 > > --- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c > > +++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c > > @@ -593,7 +593,7 @@ FirmwareVolumeInfoPpiNotifyCallback ( > > } > > > > // > > - // Locate the corresponding FV_PPI according to founded FV's format > > GUID > > + // Locate the corresponding FV_PPI according to the format GUID of > > + the FV found > > // > > Status =3D PeiServicesLocatePpi ( > > &FvInfo2Ppi.FvFormat, > > @@ -1533,7 +1533,7 @@ ProcessFvFile ( > > ); > > > > // > > - // Inform the extracted FvImage to FV HOB consumer phase, i.e. DX= E > phase > > + // Expose the extracted FvImage to the FV HOB consumer phase, > > + i.e. DXE phase > > // > > BuildFvHob ( > > (EFI_PHYSICAL_ADDRESS) (UINTN) FvHeader, @@ -2087,12 +2087,13 > > @@ FvHandleToCoreHandle ( > > } > > > > /** > > - Get instance of PEI_CORE_FV_HANDLE for next volume according to > given index. > > + Gets a PEI_CORE_FV_HANDLE instance for the next volume according to > the given index. >=20 > "Get"? > As in FwVol.h, I left as-is in V2. > > > > - This routine also will install FvInfo PPI for FV HOB in PI ways. > > + This routine also will install an instance of the FvInfo PPI for > > + the FV HOB as defined in the PI specification. > > > > @param Private Pointer of PEI_CORE_INSTANCE > > - @param Instance The index of FV want to be searched. > > + @param Instance The index of the FV to search. >=20 > Without "The"/trailing dot? >=20 As in FwVol.h, this is done in V2. > > > > @return Instance of PEI_CORE_FV_HANDLE. > > **/ > > @@ -2185,13 +2186,13 @@ PeiReinitializeFv ( > > } > > > > /** > > - Report the information for a new discovered FV in unknown third-par= ty > format. > > + Report the information for a newly discovered FV in an unknown form= at. > > > > - If the EFI_PEI_FIRMWARE_VOLUME_PPI has not been installed for > > third-party FV format, but > > - the FV in this format has been discovered, then this FV's > > information will be cached into > > - PEI_CORE_INSTANCE's UnknownFvInfo array. > > - Also a notification would be installed for unknown third-party FV > > format guid, if EFI_PEI_FIRMWARE_VOLUME_PPI > > - is installed later by platform's PEIM, the original unknown > > third-party FV will be processed by > > + If the EFI_PEI_FIRMWARE_VOLUME_PPI has not been installed for a > > + third-party FV format, but the FV in this FV format has been > > + discovered, then the information of this FV >=20 > Maybe "in this FV format" is redundant? >=20 Removed "in this FV format" in V2. > > + will be cached into PEI_CORE_INSTANCE's UnknownFvInfo array. > > + Also a notification would be installed for unknown FV format GUID, > > + if EFI_PEI_FIRMWARE_VOLUME_PPI is installed later by platform's > > + PEIM, the original unknown FV will be processed by > > using new installed EFI_PEI_FIRMWARE_VOLUME_PPI. > > > > @param PrivateData Point to instance of PEI_CORE_INSTANCE diff > > --git a/MdeModulePkg/Core/Pei/Memory/MemoryServices.c > > b/MdeModulePkg/Core/Pei/Memory/MemoryServices.c > > index 838a003baa..e713e6811a 100644 > > --- a/MdeModulePkg/Core/Pei/Memory/MemoryServices.c > > +++ b/MdeModulePkg/Core/Pei/Memory/MemoryServices.c > > @@ -759,7 +759,7 @@ PeiFreePages ( > > /** > > > > Pool allocation service. Before permanent memory is discovered, > > the pool will > > - be allocated the heap in the temporary memory. Generally, the size > > of heap in temporary > > + be allocated in the heap in temporary memory. Generally, the size > > + of the heap in temporary > > memory does not exceed to 64K, so the biggest pool size could be > allocated is > > 64K. > > > > @@ -789,7 +789,7 @@ PeiAllocatePool ( > > // > > > > // > > - // Generally, the size of heap in temporary memory does not exceed > > to 64K, > > + // Generally, the size of heap in temporary memory does not exceed > > + 64K, > > // HobLength is multiples of 8 bytes, so the maximum size of pool = is > 0xFFF8 - sizeof (EFI_HOB_MEMORY_POOL) > > // > > if (Size > (0xFFF8 - sizeof (EFI_HOB_MEMORY_POOL))) { > > >=20 >=20 >=20