* Re: [edk2-devel][edk2-platforms][PATCH v4] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
2020-01-10 3:28 [edk2-devel][edk2-platforms][PATCH v4] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment Marc W Chen
@ 2020-01-10 3:36 ` Kubacki, Michael A
0 siblings, 0 replies; 2+ messages in thread
From: Kubacki, Michael A @ 2020-01-10 3:36 UTC (permalink / raw)
To: Chen, Marc W, devel@edk2.groups.io
Cc: Chaganty, Rangasai V, Ni, Ray, Gao, Liming, Zhang, Shenglei
Hi Marc,
I already added my R-b in V3, that can be used here. I just wanted to note that it would be nice in the future if you could include a Git note in the commit that summarizes the changes between patch versions.
Thanks,
Michael
> -----Original Message-----
> From: Chen, Marc W <marc.w.chen@intel.com>
> Sent: Thursday, January 9, 2020 7:29 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zhang, Shenglei <shenglei.zhang@intel.com>; Chen,
> Marc W <marc.w.chen@intel.com>
> Subject: [edk2-devel][edk2-platforms][PATCH v4]
> IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2436
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Shenglei Zhang <shenglei.zhang@intel.com>
> Signed-off-by: Marc Chen <marc.w.chen@intel.com>
> ---
> .../Library/PeiSmmAccessLib/PeiSmmAccessLib.c | 37 +++++++++----------
> .../SmmAccess/SmmAccessDxe/SmmAccessDriver.c | 35 +++++++++++------
> -
> .../SmmAccess/SmmAccessDxe/SmmAccessDriver.h | 41 ++++++++++--------
> ----
> .../IntelSiliconPkg/Include/Library/SmmAccessLib.h | 7 ++--
> 4 files changed, 60 insertions(+), 60 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/P
> eiSmmAccessLib.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/P
> eiSmmAccessLib.c
> index da141cfa0e..d9bf4fba98 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/P
> eiSmmAccessLib.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/P
> eiSmmAccessLib.c
> @@ -1,7 +1,7 @@
> /** @file
> This is to publish the SMM Access Ppi instance.
>
> - Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -46,9 +46,9 @@ typedef struct {
> The use of "open" means that the memory is visible from all PEIM
> and SMM agents.
>
> - @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 every
> PEIM.
> + @param[in] PeiServices - General purpose services available to every
> PEIM.
> + @param[in] This - Pointer to the SMM Access Interface.
> + @param[in] DescriptorIndex - Region of SMRAM to Open.
>
> @retval EFI_SUCCESS - The region was successfully opened.
> @retval EFI_DEVICE_ERROR - The region could not be opened because
> locked by
> @@ -89,9 +89,9 @@ Open (
> This routine accepts a request to "close" a region of SMRAM. This is valid for
> compatible SMRAM region.
>
> - @param[in] PeiServices - General purpose services available to every
> PEIM.
> - @param[in] This - Pointer to the SMM Access Interface.
> - @param[in] DescriptorIndex - Region of SMRAM to Close.
> + @param[in] PeiServices - General purpose services available to every
> PEIM.
> + @param[in] This - Pointer to the SMM Access Interface.
> + @param[in] DescriptorIndex - Region of SMRAM to Close.
>
> @retval EFI_SUCCESS - The region was successfully closed.
> @retval EFI_DEVICE_ERROR - The region could not be closed because
> locked by
> @@ -151,9 +151,9 @@ Close (
> The use of "lock" means that the memory can no longer be opened
> to PEIM.
>
> - @param[in] PeiServices - General purpose services available to every
> PEIM.
> - @param[in] This - Pointer to the SMM Access Interface.
> - @param[in] DescriptorIndex - Region of SMRAM to Lock.
> + @param[in] PeiServices - General purpose services available to every
> PEIM.
> + @param[in] This - Pointer to the SMM Access Interface.
> + @param[in] DescriptorIndex - Region of SMRAM to Lock.
>
> @retval EFI_SUCCESS - The region was successfully locked.
> @retval EFI_DEVICE_ERROR - The region could not be locked because at
> least
> @@ -193,12 +193,12 @@ Lock (
> ranges that are possible for SMRAM access, based upon the
> memory controller capabilities.
>
> - @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 information.
> - @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 size of
> the
> + buffer to contain the description information.
> + @param[in, out] SmramMap - Buffer containing the data describing the
> Smram
> + region descriptors.
>
> @retval EFI_BUFFER_TOO_SMALL - The user did not provide a sufficient
> buffer.
> @retval EFI_SUCCESS - The user provided a sufficiently-sized buffer.
> @@ -234,10 +234,7 @@ GetCapabilities (
> /**
> This function is to install an SMM Access PPI
> - <b>Introduction</b> \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 an instance of EFI_PEI_MM_ACCESS_PPI. This PPI is
> commonly used to control SMM mode memory access for S3 resume.
>
> @retval EFI_SUCCESS - Ppi successfully started and installed.
> @retval EFI_NOT_FOUND - Ppi can't be found.
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.c
> index 3d3c4ab206..1cbce1907c 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.c
> @@ -2,7 +2,7 @@
> This is the driver that publishes the SMM Access Protocol
> instance for System Agent.
>
> - Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -11,14 +11,24 @@
> static SMM_ACCESS_PRIVATE_DATA mSmmAccess;
>
> /**
> - This is the standard EFI driver point that
> - installs an SMM Access Protocol
> -
> - @param[in] ImageHandle - Handle for the image of this driver
> - @param[in] SystemTable - Pointer to the EFI System Table
> + <b>SMM Access Driver Entry Point</b>
> + This driver installs an SMM Access Protocol
> + - <b>Introduction</b> \n
> + This module publishes the SMM access protocol. The protocol is used by
> the SMM Base driver to access the SMRAM region when the processor is not in
> SMM.
> + The SMM Base driver uses the services provided by the SMM access
> protocol to open SMRAM during POST and copy the SMM handler.
> + SMM access protocol is also used to close the SMRAM region once the
> copying is done.
> + Finally, the SMM access protocol provides services to "Lock" the SMRAM
> region.
> + 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.
> +
> + - <b>Porting Recommendations</b> \n
> + No modification of this module is recommended. Any modification should
> be done in compliance with the _EFI_SMM_ACCESS_PROTOCOL protocol
> definition.
> +
> + @param[in] ImageHandle - Handle for the image of this driver
> + @param[in] SystemTable - Pointer to the EFI System Table
>
> @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.
> **/
> @@ -108,8 +118,7 @@ SmmAccessDriverEntryPoint (
> @param[in] This - Pointer to the SMM Access Interface.
>
> @retval EFI_SUCCESS - The region was successfully opened.
> - @retval EFI_DEVICE_ERROR - The region could not be opened because
> locked by
> - chipset.
> + @retval EFI_DEVICE_ERROR - The region could not be opened because
> locked by chipset.
> @retval EFI_INVALID_PARAMETER - The descriptor index was out of bounds.
> **/
> EFI_STATUS
> @@ -229,13 +238,13 @@ Lock (
> memory controller capabilities.
>
> @param[in] This - Pointer to the SMRAM Access Interface.
> - @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 information.
> - @param[in] SmramMap - Buffer containing the data describing the
> Smram
> + @param[in, out] SmramMap - Buffer containing the data describing the
> Smram
> region descriptors.
>
> - @retval EFI_BUFFER_TOO_SMALL - The user did not provide a sufficient
> buffer.
> - @retval EFI_SUCCESS - The user provided a sufficiently-sized buffer.
> + @retval EFI_BUFFER_TOO_SMALL - The user did not provide a sufficient
> buffer.
> + @retval EFI_SUCCESS - The user provided a sufficiently-sized buffer.
> **/
> EFI_STATUS
> EFIAPI
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.h
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.h
> index c0ff3a250b..bcdaef7ba6 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.h
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.h
> @@ -1,7 +1,7 @@
> /** @file
> Header file for SMM Access Driver.
>
> - Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -53,23 +53,22 @@ typedef struct {
> This driver installs an SMM Access Protocol
> - <b>Introduction</b> \n
> This module publishes the SMM access protocol. The protocol is used by the
> SMM Base driver to access the SMRAM region when the processor is not in
> SMM.
> - The SMM Base driver uses the services provided by the SMM access
> protocol to open SMRAM during post and copy the SMM handler.
> + The SMM Base driver uses the services provided by the SMM access
> protocol to open SMRAM during POST and copy the SMM handler.
> SMM access protocol is also used to close the SMRAM region once the
> copying is done.
> Finally, the SMM access protocol provides services to "Lock" the SMRAM
> region.
> 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.
>
> - - @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
> -
> - <b>Porting Recommendations</b> \n
> No modification of this module is recommended. Any modification should
> be done in compliance with the _EFI_SMM_ACCESS_PROTOCOL protocol
> definition.
>
> - @param[in] ImageHandle - Handle for the image of this driver
> - @param[in] SystemTable - Pointer to the EFI System Table
> + @param[in] ImageHandle - Handle for the image of this driver
> + @param[in] SystemTable - Pointer to the EFI System Table
>
> - @retval EFI_SUCCESS - Protocol was installed successfully
> - @exception EFI_UNSUPPORTED - Protocol was not installed
> + @retval EFI_SUCCESS - Protocol was installed successfully
> + @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.
> **/
> EFI_STATUS
> EFIAPI
> @@ -87,8 +86,7 @@ SmmAccessDriverEntryPoint (
> @param[in] This - Pointer to the SMM Access Interface.
>
> @retval EFI_SUCCESS - The region was successfully opened.
> - @retval EFI_DEVICE_ERROR - The region could not be opened because
> locked by
> - chipset.
> + @retval EFI_DEVICE_ERROR - The region could not be opened because
> locked by chipset.
> @retval EFI_INVALID_PARAMETER - The descriptor index was out of bounds.
> **/
> EFI_STATUS
> @@ -103,11 +101,10 @@ Open (
> The use of "close" means that the memory is only visible from SMM agents,
> not from BS or RT code.
>
> - @param[in] This - Pointer to the SMM Access Interface.
> + @param[in] This - Pointer to the SMM Access Interface.
>
> @retval EFI_SUCCESS - The region was successfully closed.
> - @retval EFI_DEVICE_ERROR - The region could not be closed because
> locked by
> - chipset.
> + @retval EFI_DEVICE_ERROR - The region could not be closed because
> locked by chipset.
> @retval EFI_INVALID_PARAMETER - The descriptor index was out of bounds.
> **/
> EFI_STATUS
> @@ -122,11 +119,11 @@ Close (
> The use of "lock" means that the memory can no longer be opened
> to BS state..
>
> - @param[in] This - Pointer to the SMM Access Interface.
> + @param[in] This - Pointer to the SMM Access Interface.
>
> @retval EFI_SUCCESS - The region was successfully locked.
> @retval EFI_DEVICE_ERROR - The region could not be locked because at
> least
> - one range is still open.
> + one range is still open.
> @retval EFI_INVALID_PARAMETER - The descriptor index was out of bounds.
> **/
> EFI_STATUS
> @@ -142,13 +139,13 @@ Lock (
> memory controller capabilities.
>
> @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 information.
> - @param[in] SmramMap - Buffer containing the data describing the
> Smram
> - region descriptors.
> + @param[in, out] SmramMapSize - Pointer to the variable containing size
> of the
> + buffer to contain the description information.
> + @param[in, out] SmramMap - Buffer containing the data describing the
> Smram
> + region descriptors.
>
> - @retval EFI_BUFFER_TOO_SMALL - The user did not provide a sufficient
> buffer.
> - @retval EFI_SUCCESS - The user provided a sufficiently-sized buffer.
> + @retval EFI_BUFFER_TOO_SMALL - The user did not provide a sufficient
> buffer.
> + @retval EFI_SUCCESS - The user provided a sufficiently-sized buffer.
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> index f658bac68c..9792bc4099 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> @@ -1,7 +1,7 @@
> /** @file
> Header file for SMM Access Driver.
>
> - Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -11,10 +11,7 @@
> /**
> This function is to install an SMM Access PPI
> - <b>Introduction</b> \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 an instance of EFI_PEI_MM_ACCESS_PPI. This PPI is
> commonly used to control SMM mode memory access for S3 resume.
>
> @retval EFI_SUCCESS - Ppi successfully started and installed.
> @retval EFI_NOT_FOUND - Ppi can't be found.
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 2+ messages in thread