From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mx.groups.io with SMTP id smtpd.web11.1454.1578512953585524133 for ; Wed, 08 Jan 2020 11:49:13 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=MVxrmLyA; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: michael.a.kubacki@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Jan 2020 11:49:13 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,411,1571727600"; d="scan'208";a="211647983" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga007.jf.intel.com with ESMTP; 08 Jan 2020 11:49:12 -0800 Received: from fmsmsx120.amr.corp.intel.com (10.18.124.208) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 8 Jan 2020 11:49:12 -0800 Received: from FMSEDG002.ED.cps.intel.com (10.1.192.134) by fmsmsx120.amr.corp.intel.com (10.18.124.208) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 8 Jan 2020 11:49:11 -0800 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (104.47.55.169) by edgegateway.intel.com (192.55.55.69) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 8 Jan 2020 11:49:10 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bULw8sto2PHz5dxtJgNHswOmT5sC8coxDrt2xmXewdoP35apKDDY8uWe5bolgIKzPUWd1oJ29qc4+c4NodaRU1XfY5NwHc+HFhR3mFT6UO4l124CHJ7Euqll0dFQ2eZue4QFBiN7Lc+JT3hvWgeLRy6MFlvvB/vnaW8CPGJtIdPeIcbkrmSznO4TZSGLLqkUD26vcxiXr3Rrs5TMWNJFrPcQKuQX7/hqGQRhy4YXdhT4XKpjhjK6AkDbtQis8b/cWB+NfJEkhJPhjuLmDwha8x73E9hQVlopplG0038UYCvRIjyBg9CGsYm6lpqc3BtntYZBuyDSrbHGbzedf+iakQ== 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=BS5xdhaax4EpPuewn0xADiVoJ+YUWC//k52jlCvq1EM=; b=ZjlPR3MMC7i189Mstcc1Y8VmoWLmZbUSaom7X490FyQQI+ljWtuV3V2m7zD8RUtWmP/6tYzpykd+px4BX7/U6jTQXpc2UoTcpx+v1i7aievxqUIz7/VK0gMZGkwlTJEU+KSK0B2HsjR+ZG1Qp/mUH6VS0NUvQYiUA+7r/LUoovDf1y9obKcwI+GwdqVWqAp5VyYA+C1mK19OPEi5yTgBuh0d4B1qk5HuPzM0F5Y7NK49aUMpqi2rSQgfk0GzFfYWmEsLGcuO53WpCoxG480Epz/HjbbClkKr2qIwD2EVeXCBl7xu2CygiYGXLlmbyLQDqhMcbyqYHQn7E9dB8NwMEQ== 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=BS5xdhaax4EpPuewn0xADiVoJ+YUWC//k52jlCvq1EM=; b=MVxrmLyAk88BRNLfVkh6mJyl2FXUP+z9eWB1F9w9OAjpSe9n/m0sLXnApz7Kw8roWZyRtHs3/Yfxa5iD8fuk980ixQYyjY+5uH/M3DHt09vMvV0Khzy0mo9NteHk4SeyMylyeUtMp+hwyNv0jej0cSxgECG+VFpS+uzjreYQQh8= Received: from BY5PR11MB4484.namprd11.prod.outlook.com (52.132.254.155) by BY5PR11MB4183.namprd11.prod.outlook.com (10.255.162.161) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2623.9; Wed, 8 Jan 2020 19:49:05 +0000 Received: from BY5PR11MB4484.namprd11.prod.outlook.com ([fe80::b9de:29da:2b58:18b3]) by BY5PR11MB4484.namprd11.prod.outlook.com ([fe80::b9de:29da:2b58:18b3%7]) with mapi id 15.20.2623.008; Wed, 8 Jan 2020 19:49:04 +0000 From: "Kubacki, Michael A" To: "Chen, Marc W" , "devel@edk2.groups.io" CC: "Chaganty, Rangasai V" , "Gao, Liming" , "Zhang, Shenglei" Subject: Re: [edk2-devel][edk2-platforms][PATCH] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment Thread-Topic: [edk2-devel][edk2-platforms][PATCH] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment Thread-Index: AQHVu7lGwu4CgbpuV0WuOxONVc/gtKfhPm5A Date: Wed, 8 Jan 2020 19:49:04 +0000 Message-ID: References: <20191226065308.15572-1-marc.w.chen@intel.com> In-Reply-To: <20191226065308.15572-1-marc.w.chen@intel.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: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZjM2YjVjMWQtZTJiZi00NDAwLWIyYzctZTQ0ZDQ2MWQ2ZWFhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiclFHYkdtNXJKUTl5M0hqOFJveloyT2lWcG56Y3QyM1E4MEU1ZlZcL3ZlTlhWRE5uRlpqZ3JqcGxxM1g4clVaQlEifQ== 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: 95bd2797-ffb2-4d08-c781-08d79473d14c x-ms-traffictypediagnostic: BY5PR11MB4183: 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:9508; x-forefront-prvs: 02760F0D1C x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(136003)(376002)(366004)(346002)(39860400002)(396003)(199004)(189003)(13464003)(2906002)(66476007)(107886003)(66946007)(66556008)(33656002)(110136005)(86362001)(54906003)(966005)(9686003)(55016002)(4326008)(26005)(64756008)(66446008)(186003)(19273905006)(478600001)(316002)(7696005)(6506007)(81166006)(71200400001)(8936002)(53546011)(5660300002)(81156014)(76116006)(52536014)(8676002)(562404015)(563064011);DIR:OUT;SFP:1102;SCL:1;SRVR:BY5PR11MB4183;H:BY5PR11MB4484.namprd11.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: +9EaAF/ENeHZHSsqe9Lg+zRBI8WLZPmTcdZyxCPH65jIyuJJjJrXOw0peh+G5KAnjBB2/VW81ZUrDGqLNiN89kr+EHD8bX5kFE8pS8hSfAM2x6FM3sZHOWh/uc3RCfzu5Qtyx+hxzaesKoJPUE43xbVKRIrycsFiqBqMr8Ldn+51TFqXLv1eOFr8334ZsHD+2RWQoJAzuYLD8ib6r9Ds0GMLW6MmxiChClPCWQvNjjcKB33oNNtB07UY6ZMUujefmnBmcvyF+WfpAuMQV1x8UeCDMm/bMyj7B4FmkWeIw/t8vgXwK3SEycv3QiCBO/YXt+3xIrA9QuHydM52CPSSjfrI+fckcsZ6x/qjSHPDS2I+vkpB7xCnBR9dccNuUeQq0vm9DqQT6ZlQ0x5WC0gAqYnj5ndtIeLAQqeinb3j0KHXVQRdR4HgKOb9SaVViFq7O+ZhVjYX8SW5WcPQK1XyRqx2yXrZuyBUJDBRrsXzp8Vjcw7rJhMU4ZdOy7kX5b8pDCEHMr3IVtwnGtcn2QZkiVvUI95z/BlSCTN9AuhQM86DBkqskzDDq8xKPQItVGU89+ynNBjnMJ9dIqhR3Ul1oUbpeFuXU7AHyFV7p/fgeamUtdxkt7l19baJtcn7vT4J MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 95bd2797-ffb2-4d08-c781-08d79473d14c X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Jan 2020 19:49:04.7675 (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: Z55jYTcvAMfThiXsPqLxnz9jjShH0BCc6JGB6WUYfpYaZTVF0lDJyp1XuQQsKQbEDQv/BPVThB8Sr3j8tInespyY8EIeUgSN/8II1OdUKg8= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR11MB4183 Return-Path: michael.a.kubacki@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable edk2-platforms\Silicon\Intel\IntelSiliconPkg\Include\Library\SmmAccessLib.h * I think the description would be clearer by making the following change: "An API to install a PEI_MM_ACCESS_PPI PPI for controlling SMM mode memo= ry access basically for S3 resume usage." to "An API to install an instance of EFI_PEI_MM_ACCESS_PPI. This PPI is com= monly used to control SMM mode memory access for S3 resume." edk2-platforms\Silicon\Intel\IntelSiliconPkg\Feature\SmmAccess\Library\PeiS= mmAccessLib\PeiSmmAccessLib.c * I suggest aligning the " General purpose services available to every PEIM= ." on the same vertical column as the other parameter text in the function = description. * Same update to the function description for PeiInstallSmmAccessPei () tha= t was suggested in SmmAccessLib.h edk2-platforms\Silicon\Intel\IntelSiliconPkg\Feature\SmmAccess\SmmAccessDxe= \SmmAccessDriver.h edk2-platforms\Silicon\Intel\IntelSiliconPkg\Feature\SmmAccess\SmmAccessDxe= \SmmAccessDriver.c * The function description for SmmAccessDriverEntryPoint () is inconsistent= in these files. * Some return values in the description in SmmAccessDriver.c are missing= from the description in SmmAccessDriver.h With these updates: Reviewed-by: Michael Kubacki Thanks, Michael > -----Original Message----- > From: Chen, Marc W > Sent: Wednesday, December 25, 2019 10:53 PM > To: devel@edk2.groups.io > Cc: Kubacki, Michael A ; Chaganty, Rangasai = V > ; Gao, Liming ; > Zhang, Shenglei ; Chen, Marc W > > Subject: [edk2-devel][edk2-platforms][PATCH] > IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2436 >=20 > Cc: Michael Kubacki > Cc: Sai Chaganty > Cc: Liming Gao > Cc: Shenglei Zhang > Signed-off-by: Marc Chen > --- > .../Library/PeiSmmAccessLib/PeiSmmAccessLib.c | 19 ++++++++-----= ----- > - > .../Feature/SmmAccess/SmmAccessDxe/SmmAccessDriver.c | 6 +++--- > .../Feature/SmmAccess/SmmAccessDxe/SmmAccessDriver.h | 13 +++++---- > ---- > .../IntelSiliconPkg/Include/Library/SmmAccessLib.h | 5 +---- > 4 files changed, 17 insertions(+), 26 deletions(-) >=20 > diff --git > a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi > b/PeiSmmAccessLib.c > b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi > b/PeiSmmAccessLib.c > index da141cfa0e..61da7ea0bd 100644 > --- > a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi > b/PeiSmmAccessLib.c > +++ > b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAcce > +++ ssLib/PeiSmmAccessLib.c > @@ -46,9 +46,9 @@ typedef struct { > The use of "open" means that the memory is visible from all PEIM > and SMM agents. >=20 > + @param[in] PeiServices - General purpose services available to e= very > PEIM. > @param[in] This - Pointer to the SMM Access Interface. > @param[in] DescriptorIndex - Region of SMRAM to Open. > - @param[in] PeiServices - General purpose services available to e= very > PEIM. >=20 > @retval EFI_SUCCESS - The region was successfully opened. > @retval EFI_DEVICE_ERROR - The region could not be opened becau= se > locked by > @@ -193,12 +193,12 @@ Lock ( > ranges that are possible for SMRAM access, based upon the > memory controller capabilities. >=20 > - @param[in] PeiServices - General purpose services available to every > PEIM. > - @param[in] This - Pointer to the SMRAM Access Interface. > - @param[in] SmramMapSize - Pointer to the variable containing size of= the > - buffer to contain the description informat= ion. > - @param[in] SmramMap - Buffer containing the data describing the > Smram > - region descriptors. > + @param[in] PeiServices - General purpose services available to = every > PEIM. > + @param[in] This - Pointer to the SMRAM Access Interface= . > + @param[in, out] SmramMapSize - Pointer to the variable containing si= ze > of the > + buffer to contain the description inf= ormation. > + @param[in, out] SmramMap - Buffer containing the data describing= the > Smram > + region descriptors. >=20 > @retval EFI_BUFFER_TOO_SMALL - The user did not provide a sufficient > buffer. > @retval EFI_SUCCESS - The user provided a sufficiently-size= d buffer. > @@ -234,10 +234,7 @@ GetCapabilities ( > /** > This function is to install an SMM Access PPI > - Introduction \n > - A module to install a PPI for controlling SMM mode memory access > basically for S3 resume usage. > - > - - @result > - Publish _EFI_PEI_MM_ACCESS_PPI. > + An API to install a PEI_MM_ACCESS_PPI PPI for controlling SMM mode > memory access basically for S3 resume usage. >=20 > @retval EFI_SUCCESS - Ppi successfully started and install= ed. > @retval EFI_NOT_FOUND - Ppi can't be found. > diff --git > a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc > cessDriver.c > b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc > cessDriver.c > index 3d3c4ab206..9409345f6b 100644 > --- > a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc > cessDriver.c > +++ > b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc > +++ cessDriver.c > @@ -18,7 +18,7 @@ static SMM_ACCESS_PRIVATE_DATA mSmmAccess; > @param[in] SystemTable - Pointer to the EFI System Table >=20 > @retval EFI_SUCCESS - Protocol was installed successfully > - @exception EFI_UNSUPPORTED - Protocol was not installed > + @retval EFI_UNSUPPORTED - Protocol was not installed > @retval EFI_NOT_FOUND - Protocol can't be found. > @retval EFI_OUT_OF_RESOURCES - Protocol does not have enough > resources to initialize the driver. > **/ > @@ -229,9 +229,9 @@ Lock ( > memory controller capabilities. >=20 > @param[in] This - Pointer to the SMRAM Access Interfa= ce. > - @param[in] SmramMapSize - Pointer to the variable containing = size of > the > + @param[in, out] SmramMapSize - Pointer to the variable containing = size > of the > buffer to contain the description i= nformation. > - @param[in] SmramMap - Buffer containing the data describi= ng the > Smram > + @param[in, out] SmramMap - Buffer containing the data describi= ng > the Smram > region descriptors. >=20 > @retval EFI_BUFFER_TOO_SMALL - The user did not provide a sufficient > buffer. > diff --git > a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc > cessDriver.h > b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc > cessDriver.h > index c0ff3a250b..a2ea6332fa 100644 > --- > a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc > cessDriver.h > +++ > b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc > +++ cessDriver.h > @@ -59,9 +59,6 @@ typedef struct { > Please refer the SMM Protocols section in the attached SMM CIS > Specification version 0.9 for further details. > This driver is required if SMM is supported. Proper configuration of= SMM > registers is recommended even if SMM is not supported. >=20 > - - @result > - Publishes the _EFI_SMM_ACCESS_PROTOCOL: Documented in the > System Management Mode Core Interface Specification, available at the > URL: http://www.intel.com/technology/framework/spec.htm > - > - Porting Recommendations \n > No modification of this module is recommended. Any modification sho= uld > be done in compliance with the _EFI_SMM_ACCESS_PROTOCOL protocol > definition. >=20 > @@ -69,7 +66,7 @@ typedef struct { > @param[in] SystemTable - Pointer to the EFI System Table >=20 > @retval EFI_SUCCESS - Protocol was installed successfully > - @exception EFI_UNSUPPORTED - Protocol was not installed > + @retval EFI_UNSUPPORTED - Protocol was not installed > **/ > EFI_STATUS > EFIAPI > @@ -142,10 +139,10 @@ Lock ( > memory controller capabilities. >=20 > @param[in] This - Pointer to the SMRAM Access Interfa= ce. > - @param[in] SmramMapSize - Pointer to the variable containing = size of > the > - buffer to contain the description informatio= n. > - @param[in] SmramMap - Buffer containing the data describi= ng the > Smram > - region descriptors. > + @param[in, out] SmramMapSize - Pointer to the variable containing = size > of the > + buffer to contain the description i= nformation. > + @param[in, out] SmramMap - Buffer containing the data describi= ng > the Smram > + region descriptors. >=20 > @retval EFI_BUFFER_TOO_SMALL - The user did not provide a sufficient > buffer. > @retval EFI_SUCCESS - The user provided a sufficiently-sized= buffer. > diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h > b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h > index f658bac68c..0a287b75b2 100644 > --- a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h > +++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h > @@ -11,10 +11,7 @@ > /** > This function is to install an SMM Access PPI > - Introduction \n > - A module to install a PPI for controlling SMM mode memory access > basically for S3 resume usage. > - > - - @result > - Publish _PEI_MM_ACCESS_PPI. > + An API to install a PEI_MM_ACCESS_PPI PPI for controlling SMM mode > memory access basically for S3 resume usage. >=20 > @retval EFI_SUCCESS - Ppi successfully started and install= ed. > @retval EFI_NOT_FOUND - Ppi can't be found. > -- > 2.16.2.windows.1