[edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL
       [not found] <cover.1703466776.git.guox.xu@intel.com>
@ 2023-12-25  1:21 ` GuoX Xu
       [not found]   ` <CO1PR11MB49167DF996E427FCE9B823D6966A2@CO1PR11MB4916.namprd11.prod.outlook.com>
  0 siblings, 1 reply; 8+ messages in thread
From: GuoX Xu @ 2023-12-25  1:21 UTC (permalink / raw)
  To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Yi Li

1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
Add the following sentence at the end of the Image parameter
description. "May be NULL with a zero ImageSize in order to
determine the size of the buffer needed".

Modify the description of "EFI_INVALID_PARAMETER" return code as
"The ImageSize is not too small and Image is NULL."

2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
Add the following sentence at the end of the ImageInfo parameter
description."May be NULL with a zero ImageInfoSize in order to
determine the size of the buffer needed".

Modify the description of "EFI_INVALID_PARAMETER" return code as
"The ImageInfoSize is not too small and Image is NULL." and add new
descriptions for "EFI_INVALID_PARAMETER" return code.

REF: UEFI Spec 2.7B Chapter 23.1.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: GuoX Xu <guox.xu@intel.com>
---
 MdePkg/Include/Protocol/FirmwareManagement.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h b/MdePkg/Include/Protocol/FirmwareManagement.h
index f37067df3455..93c8b7658e1a 100644
--- a/MdePkg/Include/Protocol/FirmwareManagement.h
+++ b/MdePkg/Include/Protocol/FirmwareManagement.h
@@ -293,6 +293,8 @@ EFI_STATUS
                                      to contain the image(s) information if the buffer was too small.
   @param[in, out] ImageInfo          A pointer to the buffer in which firmware places the current image(s)
                                      information. The information is an array of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
+                                     May be NULL with a zero ImageInfoSize in order to determine the size of the
+                                     buffer needed.
   @param[out]     DescriptorVersion  A pointer to the location in which firmware returns the version number
                                      associated with the EFI_FIRMWARE_IMAGE_DESCRIPTOR.
   @param[out]     DescriptorCount    A pointer to the location in which firmware returns the number of
@@ -313,7 +315,12 @@ EFI_STATUS
   @retval EFI_SUCCESS                The device was successfully updated with the new image.
   @retval EFI_BUFFER_TOO_SMALL       The ImageInfo buffer was too small. The current buffer size
                                      needed to hold the image(s) information is returned in ImageInfoSize.
-  @retval EFI_INVALID_PARAMETER      ImageInfoSize is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is not too small and ImageInfo is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and DescriptorVersion is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and DescriptorCount is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and DescriptorSize is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and PackageVersion is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and PackageVersionName is NULL.
   @retval EFI_DEVICE_ERROR           Valid information could not be returned. Possible corrupted image.
 
 **/
@@ -340,6 +347,8 @@ EFI_STATUS
   @param[in]      ImageIndex     A unique number identifying the firmware image(s) within the device.
                                  The number is between 1 and DescriptorCount.
   @param[out]     Image          Points to the buffer where the current image is copied to.
+                                 May be NULL with a zero ImageSize in order to determine the size of the
+                                 buffer needed.
   @param[in, out] ImageSize      On entry, points to the size of the buffer pointed to by Image, in bytes.
                                  On return, points to the length of the image, in bytes.
 
@@ -347,7 +356,7 @@ EFI_STATUS
   @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is too small to hold the
                                  image. The current buffer size needed to hold the image is returned
                                  in ImageSize.
-  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_INVALID_PARAMETER  The ImageSize is not too small and Image is NULL.
   @retval EFI_NOT_FOUND          The current image is not copied to the buffer.
   @retval EFI_UNSUPPORTED        The operation is not supported.
   @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure.
-- 
2.29.2.windows.3



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112920): https://edk2.groups.io/g/devel/message/112920
Mute This Topic: https://groups.io/mt/103369617/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 8+ messages in thread

[parent not found: <CO1PR11MB49167DF996E427FCE9B823D6966A2@CO1PR11MB4916.namprd11.prod.outlook.com>]

* Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL
       [not found]   ` <CO1PR11MB49167DF996E427FCE9B823D6966A2@CO1PR11MB4916.namprd11.prod.outlook.com>
@ 2024-01-09  9:24     ` Xu, GuoX
  2024-01-16 14:20       ` 回复: " gaoliming via groups.io
  0 siblings, 1 reply; 8+ messages in thread
From: Xu, GuoX @ 2024-01-09  9:24 UTC (permalink / raw)
  To: devel@edk2.groups.io; +Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang

Hi all, any comments about this patch?

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of GuoX Xu
Sent: Monday, December 25, 2023 9:21 AM
To: devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Li, Yi1 <yi1.li@intel.com>
Subject: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL

1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
Add the following sentence at the end of the Image parameter description. "May be NULL with a zero ImageSize in order to determine the size of the buffer needed".

Modify the description of "EFI_INVALID_PARAMETER" return code as "The ImageSize is not too small and Image is NULL."

2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
Add the following sentence at the end of the ImageInfo parameter description."May be NULL with a zero ImageInfoSize in order to determine the size of the buffer needed".

Modify the description of "EFI_INVALID_PARAMETER" return code as "The ImageInfoSize is not too small and Image is NULL." and add new descriptions for "EFI_INVALID_PARAMETER" return code.

REF: UEFI Spec 2.7B Chapter 23.1.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: GuoX Xu <guox.xu@intel.com>
---
 MdePkg/Include/Protocol/FirmwareManagement.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h b/MdePkg/Include/Protocol/FirmwareManagement.h
index f37067df3455..93c8b7658e1a 100644
--- a/MdePkg/Include/Protocol/FirmwareManagement.h
+++ b/MdePkg/Include/Protocol/FirmwareManagement.h
@@ -293,6 +293,8 @@ EFI_STATUS
                                      to contain the image(s) information if the buffer was too small.
   @param[in, out] ImageInfo          A pointer to the buffer in which firmware places the current image(s)
                                      information. The information is an array of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
+                                     May be NULL with a zero ImageInfoSize in order to determine the size of the
+                                     buffer needed.
   @param[out]     DescriptorVersion  A pointer to the location in which firmware returns the version number
                                      associated with the EFI_FIRMWARE_IMAGE_DESCRIPTOR.
   @param[out]     DescriptorCount    A pointer to the location in which firmware returns the number of
@@ -313,7 +315,12 @@ EFI_STATUS
   @retval EFI_SUCCESS                The device was successfully updated with the new image.
   @retval EFI_BUFFER_TOO_SMALL       The ImageInfo buffer was too small. The current buffer size
                                      needed to hold the image(s) information is returned in ImageInfoSize.
-  @retval EFI_INVALID_PARAMETER      ImageInfoSize is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is not too small and ImageInfo is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and DescriptorVersion is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and DescriptorCount is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and DescriptorSize is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and PackageVersion is NULL.
+  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and PackageVersionName is NULL.
   @retval EFI_DEVICE_ERROR           Valid information could not be returned. Possible corrupted image.
 
 **/
@@ -340,6 +347,8 @@ EFI_STATUS
   @param[in]      ImageIndex     A unique number identifying the firmware image(s) within the device.
                                  The number is between 1 and DescriptorCount.
   @param[out]     Image          Points to the buffer where the current image is copied to.
+                                 May be NULL with a zero ImageSize in order to determine the size of the
+                                 buffer needed.
   @param[in, out] ImageSize      On entry, points to the size of the buffer pointed to by Image, in bytes.
                                  On return, points to the length of the image, in bytes.
 
@@ -347,7 +356,7 @@ EFI_STATUS
   @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is too small to hold the
                                  image. The current buffer size needed to hold the image is returned
                                  in ImageSize.
-  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_INVALID_PARAMETER  The ImageSize is not too small and Image is NULL.
   @retval EFI_NOT_FOUND          The current image is not copied to the buffer.
   @retval EFI_UNSUPPORTED        The operation is not supported.
   @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure.
--
2.29.2.windows.3








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113454): https://edk2.groups.io/g/devel/message/113454
Mute This Topic: https://groups.io/mt/103369617/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* 回复: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL
  2024-01-09  9:24     ` Xu, GuoX
@ 2024-01-16 14:20       ` gaoliming via groups.io
  2024-01-22  5:37         ` Xu, GuoX
  0 siblings, 1 reply; 8+ messages in thread
From: gaoliming via groups.io @ 2024-01-16 14:20 UTC (permalink / raw)
  To: devel, guox.xu; +Cc: 'Kinney, Michael D', 'Liu, Zhiguang'

I am OK for this change. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Xu, GuoX
> 发送时间: 2024年1月9日 17:24
> 收件人: devel@edk2.groups.io
> 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> 主题: Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> Hi all, any comments about this patch?
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of GuoX Xu
> Sent: Monday, December 25, 2023 9:21 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Li, Yi1
> <yi1.li@intel.com>
> Subject: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> 1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
> Add the following sentence at the end of the Image parameter description.
> "May be NULL with a zero ImageSize in order to determine the size of the
> buffer needed".
> 
> Modify the description of "EFI_INVALID_PARAMETER" return code as "The
> ImageSize is not too small and Image is NULL."
> 
> 2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
> Add the following sentence at the end of the ImageInfo parameter
> description."May be NULL with a zero ImageInfoSize in order to determine
> the size of the buffer needed".
> 
> Modify the description of "EFI_INVALID_PARAMETER" return code as "The
> ImageInfoSize is not too small and Image is NULL." and add new descriptions
> for "EFI_INVALID_PARAMETER" return code.
> 
> REF: UEFI Spec 2.7B Chapter 23.1.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Yi Li <yi1.li@intel.com>
> Signed-off-by: GuoX Xu <guox.xu@intel.com>
> ---
>  MdePkg/Include/Protocol/FirmwareManagement.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h
> b/MdePkg/Include/Protocol/FirmwareManagement.h
> index f37067df3455..93c8b7658e1a 100644
> --- a/MdePkg/Include/Protocol/FirmwareManagement.h
> +++ b/MdePkg/Include/Protocol/FirmwareManagement.h
> @@ -293,6 +293,8 @@ EFI_STATUS
>                                       to contain the image(s)
> information if the buffer was too small.
>    @param[in, out] ImageInfo          A pointer to the buffer in which
> firmware places the current image(s)
>                                       information. The information
> is an array of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
> +                                     May be NULL with a zero
> ImageInfoSize in order to determine the size of the
> +                                     buffer needed.
>    @param[out]     DescriptorVersion  A pointer to the location in which
> firmware returns the version number
>                                       associated with the
> EFI_FIRMWARE_IMAGE_DESCRIPTOR.
>    @param[out]     DescriptorCount    A pointer to the location in
> which firmware returns the number of
> @@ -313,7 +315,12 @@ EFI_STATUS
>    @retval EFI_SUCCESS                The device was successfully
> updated with the new image.
>    @retval EFI_BUFFER_TOO_SMALL       The ImageInfo buffer was too
> small. The current buffer size
>                                       needed to hold the image(s)
> information is returned in ImageInfoSize.
> -  @retval EFI_INVALID_PARAMETER      ImageInfoSize is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is not too small
> and ImageInfo is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> DescriptorVersion is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> DescriptorCount is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> DescriptorSize is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> PackageVersion is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> PackageVersionName is NULL.
>    @retval EFI_DEVICE_ERROR           Valid information could not be
> returned. Possible corrupted image.
> 
>  **/
> @@ -340,6 +347,8 @@ EFI_STATUS
>    @param[in]      ImageIndex     A unique number identifying the
> firmware image(s) within the device.
>                                   The number is between 1 and
> DescriptorCount.
>    @param[out]     Image          Points to the buffer where the
> current image is copied to.
> +                                 May be NULL with a zero
> ImageSize in order to determine the size of the
> +                                 buffer needed.
>    @param[in, out] ImageSize      On entry, points to the size of the
> buffer pointed to by Image, in bytes.
>                                   On return, points to the length of
> the image, in bytes.
> 
> @@ -347,7 +356,7 @@ EFI_STATUS
>    @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is
> too small to hold the
>                                   image. The current buffer size
> needed to hold the image is returned
>                                   in ImageSize.
> -  @retval EFI_INVALID_PARAMETER  The Image was NULL.
> +  @retval EFI_INVALID_PARAMETER  The ImageSize is not too small and
> Image is NULL.
>    @retval EFI_NOT_FOUND          The current image is not copied to
> the buffer.
>    @retval EFI_UNSUPPORTED        The operation is not supported.
>    @retval EFI_SECURITY_VIOLATION The operation could not be performed
> due to an authentication failure.
> --
> 2.29.2.windows.3
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113901): https://edk2.groups.io/g/devel/message/113901
Mute This Topic: https://groups.io/mt/103762175/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL
  2024-01-16 14:20       ` 回复: " gaoliming via groups.io
@ 2024-01-22  5:37         ` Xu, GuoX
  2024-01-22 23:08           ` Michael D Kinney
  0 siblings, 1 reply; 8+ messages in thread
From: Xu, GuoX @ 2024-01-22  5:37 UTC (permalink / raw)
  To: gaoliming, devel@edk2.groups.io; +Cc: Kinney, Michael D, Liu, Zhiguang

Hi Liming,
  I created a PR: https://github.com/tianocore/edk2/pull/5182, could you help push it ?

Thanks
Xu Guo

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: Tuesday, January 16, 2024 10:20 PM
To: devel@edk2.groups.io; Xu, GuoX <guox.xu@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>
Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL

I am OK for this change. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Xu, GuoX
> 发送时间: 2024年1月9日 17:24
> 收件人: devel@edk2.groups.io
> 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming 
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> 主题: Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of 
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> Hi all, any comments about this patch?
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of GuoX Xu
> Sent: Monday, December 25, 2023 9:21 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming 
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; 
> Li, Yi1 <yi1.li@intel.com>
> Subject: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of 
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> 1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
> Add the following sentence at the end of the Image parameter description.
> "May be NULL with a zero ImageSize in order to determine the size of 
> the buffer needed".
> 
> Modify the description of "EFI_INVALID_PARAMETER" return code as "The 
> ImageSize is not too small and Image is NULL."
> 
> 2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
> Add the following sentence at the end of the ImageInfo parameter 
> description."May be NULL with a zero ImageInfoSize in order to 
> determine the size of the buffer needed".
> 
> Modify the description of "EFI_INVALID_PARAMETER" return code as "The 
> ImageInfoSize is not too small and Image is NULL." and add new 
> descriptions for "EFI_INVALID_PARAMETER" return code.
> 
> REF: UEFI Spec 2.7B Chapter 23.1.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Yi Li <yi1.li@intel.com>
> Signed-off-by: GuoX Xu <guox.xu@intel.com>
> ---
>  MdePkg/Include/Protocol/FirmwareManagement.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h
> b/MdePkg/Include/Protocol/FirmwareManagement.h
> index f37067df3455..93c8b7658e1a 100644
> --- a/MdePkg/Include/Protocol/FirmwareManagement.h
> +++ b/MdePkg/Include/Protocol/FirmwareManagement.h
> @@ -293,6 +293,8 @@ EFI_STATUS
>                                       to contain the image(s) 
> information if the buffer was too small.
>    @param[in, out] ImageInfo          A pointer to the buffer in which
> firmware places the current image(s)
>                                       information. The information is 
> an array of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
> +                                     May be NULL with a zero
> ImageInfoSize in order to determine the size of the
> +                                     buffer needed.
>    @param[out]     DescriptorVersion  A pointer to the location in which
> firmware returns the version number
>                                       associated with the 
> EFI_FIRMWARE_IMAGE_DESCRIPTOR.
>    @param[out]     DescriptorCount    A pointer to the location in
> which firmware returns the number of
> @@ -313,7 +315,12 @@ EFI_STATUS
>    @retval EFI_SUCCESS                The device was successfully
> updated with the new image.
>    @retval EFI_BUFFER_TOO_SMALL       The ImageInfo buffer was too
> small. The current buffer size
>                                       needed to hold the image(s) 
> information is returned in ImageInfoSize.
> -  @retval EFI_INVALID_PARAMETER      ImageInfoSize is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is not too small
> and ImageInfo is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> DescriptorVersion is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> DescriptorCount is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> DescriptorSize is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> PackageVersion is NULL.
> +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> PackageVersionName is NULL.
>    @retval EFI_DEVICE_ERROR           Valid information could not be
> returned. Possible corrupted image.
> 
>  **/
> @@ -340,6 +347,8 @@ EFI_STATUS
>    @param[in]      ImageIndex     A unique number identifying the
> firmware image(s) within the device.
>                                   The number is between 1 and 
> DescriptorCount.
>    @param[out]     Image          Points to the buffer where the
> current image is copied to.
> +                                 May be NULL with a zero
> ImageSize in order to determine the size of the
> +                                 buffer needed.
>    @param[in, out] ImageSize      On entry, points to the size of the
> buffer pointed to by Image, in bytes.
>                                   On return, points to the length of 
> the image, in bytes.
> 
> @@ -347,7 +356,7 @@ EFI_STATUS
>    @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is
> too small to hold the
>                                   image. The current buffer size 
> needed to hold the image is returned
>                                   in ImageSize.
> -  @retval EFI_INVALID_PARAMETER  The Image was NULL.
> +  @retval EFI_INVALID_PARAMETER  The ImageSize is not too small and
> Image is NULL.
>    @retval EFI_NOT_FOUND          The current image is not copied to
> the buffer.
>    @retval EFI_UNSUPPORTED        The operation is not supported.
>    @retval EFI_SECURITY_VIOLATION The operation could not be performed 
> due to an authentication failure.
> --
> 2.29.2.windows.3
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114175): https://edk2.groups.io/g/devel/message/114175
Mute This Topic: https://groups.io/mt/103904259/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL
  2024-01-22  5:37         ` Xu, GuoX
@ 2024-01-22 23:08           ` Michael D Kinney
  2024-01-30  7:17             ` Xu, GuoX
  0 siblings, 1 reply; 8+ messages in thread
From: Michael D Kinney @ 2024-01-22 23:08 UTC (permalink / raw)
  To: Xu, GuoX, gaoliming, devel@edk2.groups.io
  Cc: Liu, Zhiguang, Kinney, Michael D

Hi Guo,

Thank you for starting this task to update Firmware Management Protocol.

There are additional tasks to make sure all required code changes are
also implemented for a specification update like this.

* Create a TianoCore Bugzilla that describes the specification update
  needed with links to the public documents with the required
  specification update. 

* Review/update edk2 repo include files.

* Review/update function headers of the implementations of the
  Firmware Management Protocol in the edk2 repo.

* Review/update logic in implementations of the Firmware
  Management Protocol in the edk2 repo to make sure all error 
  return conditions are checked.

* Review/update function headers of the implementations of the
  Firmware Management Protocol in the edk2-platforms repo.

* Review/update logic in implementations of the Firmware
  Management Protocol in the edk2-platforms repo to make sure 
  all error return conditions are checked.

* Review/update test cases edk2-test repo for the UEFI SCTs for
  these error return conditions.


The "REF" in the commit message should be a link to the TianoCore
Bugzilla.  The reference to the specification is also required, but
should be in the text of the commit message.

Thanks,

Mike


> -----Original Message-----
> From: Xu, GuoX <guox.xu@intel.com>
> Sent: Sunday, January 21, 2024 9:37 PM
> To: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> Hi Liming,
>   I created a PR: https://github.com/tianocore/edk2/pull/5182, could you
> help push it ?
> 
> Thanks
> Xu Guo
> 
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Tuesday, January 16, 2024 10:20 PM
> To: devel@edk2.groups.io; Xu, GuoX <guox.xu@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> I am OK for this change. Reviewed-by: Liming Gao
> <gaoliming@byosoft.com.cn>
> 
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Xu, GuoX
> > 发送时间: 2024年1月9日 17:24
> > 收件人: devel@edk2.groups.io
> > 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> > 主题: Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > Hi all, any comments about this patch?
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of GuoX Xu
> > Sent: Monday, December 25, 2023 9:21 AM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> > Li, Yi1 <yi1.li@intel.com>
> > Subject: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > 1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
> > Add the following sentence at the end of the Image parameter
> description.
> > "May be NULL with a zero ImageSize in order to determine the size of
> > the buffer needed".
> >
> > Modify the description of "EFI_INVALID_PARAMETER" return code as "The
> > ImageSize is not too small and Image is NULL."
> >
> > 2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
> > Add the following sentence at the end of the ImageInfo parameter
> > description."May be NULL with a zero ImageInfoSize in order to
> > determine the size of the buffer needed".
> >
> > Modify the description of "EFI_INVALID_PARAMETER" return code as "The
> > ImageInfoSize is not too small and Image is NULL." and add new
> > descriptions for "EFI_INVALID_PARAMETER" return code.
> >
> > REF: UEFI Spec 2.7B Chapter 23.1.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Cc: Yi Li <yi1.li@intel.com>
> > Signed-off-by: GuoX Xu <guox.xu@intel.com>
> > ---
> >  MdePkg/Include/Protocol/FirmwareManagement.h | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h
> > b/MdePkg/Include/Protocol/FirmwareManagement.h
> > index f37067df3455..93c8b7658e1a 100644
> > --- a/MdePkg/Include/Protocol/FirmwareManagement.h
> > +++ b/MdePkg/Include/Protocol/FirmwareManagement.h
> > @@ -293,6 +293,8 @@ EFI_STATUS
> >                                       to contain the image(s)
> > information if the buffer was too small.
> >    @param[in, out] ImageInfo          A pointer to the buffer in which
> > firmware places the current image(s)
> >                                       information. The information is
> > an array of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
> > +                                     May be NULL with a zero
> > ImageInfoSize in order to determine the size of the
> > +                                     buffer needed.
> >    @param[out]     DescriptorVersion  A pointer to the location in
> which
> > firmware returns the version number
> >                                       associated with the
> > EFI_FIRMWARE_IMAGE_DESCRIPTOR.
> >    @param[out]     DescriptorCount    A pointer to the location in
> > which firmware returns the number of
> > @@ -313,7 +315,12 @@ EFI_STATUS
> >    @retval EFI_SUCCESS                The device was successfully
> > updated with the new image.
> >    @retval EFI_BUFFER_TOO_SMALL       The ImageInfo buffer was too
> > small. The current buffer size
> >                                       needed to hold the image(s)
> > information is returned in ImageInfoSize.
> > -  @retval EFI_INVALID_PARAMETER      ImageInfoSize is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is not too small
> > and ImageInfo is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > DescriptorVersion is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > DescriptorCount is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > DescriptorSize is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > PackageVersion is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > PackageVersionName is NULL.
> >    @retval EFI_DEVICE_ERROR           Valid information could not be
> > returned. Possible corrupted image.
> >
> >  **/
> > @@ -340,6 +347,8 @@ EFI_STATUS
> >    @param[in]      ImageIndex     A unique number identifying the
> > firmware image(s) within the device.
> >                                   The number is between 1 and
> > DescriptorCount.
> >    @param[out]     Image          Points to the buffer where the
> > current image is copied to.
> > +                                 May be NULL with a zero
> > ImageSize in order to determine the size of the
> > +                                 buffer needed.
> >    @param[in, out] ImageSize      On entry, points to the size of the
> > buffer pointed to by Image, in bytes.
> >                                   On return, points to the length of
> > the image, in bytes.
> >
> > @@ -347,7 +356,7 @@ EFI_STATUS
> >    @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is
> > too small to hold the
> >                                   image. The current buffer size
> > needed to hold the image is returned
> >                                   in ImageSize.
> > -  @retval EFI_INVALID_PARAMETER  The Image was NULL.
> > +  @retval EFI_INVALID_PARAMETER  The ImageSize is not too small and
> > Image is NULL.
> >    @retval EFI_NOT_FOUND          The current image is not copied to
> > the buffer.
> >    @retval EFI_UNSUPPORTED        The operation is not supported.
> >    @retval EFI_SECURITY_VIOLATION The operation could not be performed
> > due to an authentication failure.
> > --
> > 2.29.2.windows.3
> >
> >
> >
> >
> >
> >
> >
> >
> > 
> >
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114151): https://edk2.groups.io/g/devel/message/114151
Mute This Topic: https://groups.io/mt/103898765/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL
  2024-01-22 23:08           ` Michael D Kinney
@ 2024-01-30  7:17             ` Xu, GuoX
  2024-01-30 18:18               ` Michael D Kinney
  0 siblings, 1 reply; 8+ messages in thread
From: Xu, GuoX @ 2024-01-30  7:17 UTC (permalink / raw)
  To: Kinney, Michael D, gaoliming, devel@edk2.groups.io; +Cc: Liu, Zhiguang

Hi Michael,

Thanks for your suggestions.

1. In edk2 repo there are two implementations for Firmware Management Protocol.
  1.1 In edk2\FmpDevicePkg\FmpDxe\FmpDxe.c, function GetTheImageInfo():
    //
    // Check the buffer size
    // NOTE: Check this first so caller can get the necessary memory size it must allocate.
    //
    if (*ImageInfoSize < (sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR))) {
      *ImageInfoSize = sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR);
      DEBUG ((DEBUG_VERBOSE, "FmpDxe(%s): GetImageInfo() - ImageInfoSize is to small.\n", mImageIdName));
      Status = EFI_BUFFER_TOO_SMALL;
      goto cleanup;
    }

    //
    // Confirm that buffer isn't null
    //
    if (  (ImageInfo == NULL) || (DescriptorVersion == NULL) || (DescriptorCount == NULL) || (DescriptorSize == NULL)
       || (PackageVersion == NULL))
    {
      DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetImageInfo() - Pointer Parameter is NULL.\n", mImageIdName));
      Status = EFI_INVALID_PARAMETER;
      goto cleanup;
    }
    In the line 497 to 498, it is missed the judgment of PackageVersionName but there is a comment in line 529 " // Do not update PackageVersionName since it is not supported in this instance."
    So, do we need to add the judgment of PackageVersionName in this function?

  1.2 In edk2\SignedCapsulePkg\Universal\SystemFirmwareUpdate\SystemFirmwareCommonDxe.c, function FmpGetImageInfo():
         if (ImageInfoSize == NULL) {
           return EFI_INVALID_PARAMETER;
         }

         if (*ImageInfoSize < sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR) * SystemFmpPrivate->DescriptorCount) {
           *ImageInfoSize = sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR) * SystemFmpPrivate->DescriptorCount;
           return EFI_BUFFER_TOO_SMALL;
        }

        if ((ImageInfo == NULL) ||
          (DescriptorVersion == NULL) ||
          (DescriptorCount == NULL) ||
          (DescriptorSize == NULL) ||
          (PackageVersion == NULL) ||
          (PackageVersionName == NULL))
       {
         return EFI_INVALID_PARAMETER;
       }
    The implementation already included the required code changes.

 2. In edk2-platforms repo there is only one implementation for Firmware Management Protocol.
     In the file MicrocodeFmp.c, function FmpGetImageInfo():
     If (ImageInfoSize == NULL) {
       return EFI_INVALID_PARAMETER;
     }
     if (*ImageInfoSize < sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR) * MicrocodeFmpPrivate->DescriptorCount) {
       *ImageInfoSize = sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR) * MicrocodeFmpPrivate->DescriptorCount;
       return EFI_BUFFER_TOO_SMALL;
     }
     if (ImageInfo == NULL ||
        DescriptorVersion == NULL ||
        DescriptorCount == NULL ||
        DescriptorSize == NULL ||
        PackageVersion == NULL ||
        PackageVersionName == NULL) {
      return EFI_INVALID_PARAMETER;
    }
   The implementation already included the required code changes.    

Thanks
Xu Guo

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Tuesday, January 23, 2024 7:08 AM
To: Xu, GuoX <guox.xu@intel.com>; gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL

Hi Guo,

Thank you for starting this task to update Firmware Management Protocol.

There are additional tasks to make sure all required code changes are also implemented for a specification update like this.

* Create a TianoCore Bugzilla that describes the specification update
  needed with links to the public documents with the required
  specification update. 

* Review/update edk2 repo include files.

* Review/update function headers of the implementations of the
  Firmware Management Protocol in the edk2 repo.

* Review/update logic in implementations of the Firmware
  Management Protocol in the edk2 repo to make sure all error
  return conditions are checked.

* Review/update function headers of the implementations of the
  Firmware Management Protocol in the edk2-platforms repo.

* Review/update logic in implementations of the Firmware
  Management Protocol in the edk2-platforms repo to make sure
  all error return conditions are checked.

* Review/update test cases edk2-test repo for the UEFI SCTs for
  these error return conditions.


The "REF" in the commit message should be a link to the TianoCore Bugzilla.  The reference to the specification is also required, but should be in the text of the commit message.

Thanks,

Mike


> -----Original Message-----
> From: Xu, GuoX <guox.xu@intel.com>
> Sent: Sunday, January 21, 2024 9:37 PM
> To: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang 
> <zhiguang.liu@intel.com>
> Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of 
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> Hi Liming,
>   I created a PR: https://github.com/tianocore/edk2/pull/5182, could 
> you help push it ?
> 
> Thanks
> Xu Guo
> 
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Tuesday, January 16, 2024 10:20 PM
> To: devel@edk2.groups.io; Xu, GuoX <guox.xu@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang 
> <zhiguang.liu@intel.com>
> Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of 
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> I am OK for this change. Reviewed-by: Liming Gao 
> <gaoliming@byosoft.com.cn>
> 
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Xu, GuoX
> > 发送时间: 2024年1月9日 17:24
> > 收件人: devel@edk2.groups.io
> > 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming 
> > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> > 主题: Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of 
> > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > Hi all, any comments about this patch?
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of GuoX 
> > Xu
> > Sent: Monday, December 25, 2023 9:21 AM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming 
> > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; 
> > Li, Yi1 <yi1.li@intel.com>
> > Subject: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of 
> > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > 1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
> > Add the following sentence at the end of the Image parameter
> description.
> > "May be NULL with a zero ImageSize in order to determine the size of 
> > the buffer needed".
> >
> > Modify the description of "EFI_INVALID_PARAMETER" return code as 
> > "The ImageSize is not too small and Image is NULL."
> >
> > 2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
> > Add the following sentence at the end of the ImageInfo parameter 
> > description."May be NULL with a zero ImageInfoSize in order to 
> > determine the size of the buffer needed".
> >
> > Modify the description of "EFI_INVALID_PARAMETER" return code as 
> > "The ImageInfoSize is not too small and Image is NULL." and add new 
> > descriptions for "EFI_INVALID_PARAMETER" return code.
> >
> > REF: UEFI Spec 2.7B Chapter 23.1.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Cc: Yi Li <yi1.li@intel.com>
> > Signed-off-by: GuoX Xu <guox.xu@intel.com>
> > ---
> >  MdePkg/Include/Protocol/FirmwareManagement.h | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h
> > b/MdePkg/Include/Protocol/FirmwareManagement.h
> > index f37067df3455..93c8b7658e1a 100644
> > --- a/MdePkg/Include/Protocol/FirmwareManagement.h
> > +++ b/MdePkg/Include/Protocol/FirmwareManagement.h
> > @@ -293,6 +293,8 @@ EFI_STATUS
> >                                       to contain the image(s) 
> > information if the buffer was too small.
> >    @param[in, out] ImageInfo          A pointer to the buffer in which
> > firmware places the current image(s)
> >                                       information. The information 
> > is an array of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
> > +                                     May be NULL with a zero
> > ImageInfoSize in order to determine the size of the
> > +                                     buffer needed.
> >    @param[out]     DescriptorVersion  A pointer to the location in
> which
> > firmware returns the version number
> >                                       associated with the 
> > EFI_FIRMWARE_IMAGE_DESCRIPTOR.
> >    @param[out]     DescriptorCount    A pointer to the location in
> > which firmware returns the number of @@ -313,7 +315,12 @@ EFI_STATUS
> >    @retval EFI_SUCCESS                The device was successfully
> > updated with the new image.
> >    @retval EFI_BUFFER_TOO_SMALL       The ImageInfo buffer was too
> > small. The current buffer size
> >                                       needed to hold the image(s) 
> > information is returned in ImageInfoSize.
> > -  @retval EFI_INVALID_PARAMETER      ImageInfoSize is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is not too small
> > and ImageInfo is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > DescriptorVersion is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > DescriptorCount is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > DescriptorSize is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > PackageVersion is NULL.
> > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > PackageVersionName is NULL.
> >    @retval EFI_DEVICE_ERROR           Valid information could not be
> > returned. Possible corrupted image.
> >
> >  **/
> > @@ -340,6 +347,8 @@ EFI_STATUS
> >    @param[in]      ImageIndex     A unique number identifying the
> > firmware image(s) within the device.
> >                                   The number is between 1 and 
> > DescriptorCount.
> >    @param[out]     Image          Points to the buffer where the
> > current image is copied to.
> > +                                 May be NULL with a zero
> > ImageSize in order to determine the size of the
> > +                                 buffer needed.
> >    @param[in, out] ImageSize      On entry, points to the size of the
> > buffer pointed to by Image, in bytes.
> >                                   On return, points to the length of 
> > the image, in bytes.
> >
> > @@ -347,7 +356,7 @@ EFI_STATUS
> >    @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is
> > too small to hold the
> >                                   image. The current buffer size 
> > needed to hold the image is returned
> >                                   in ImageSize.
> > -  @retval EFI_INVALID_PARAMETER  The Image was NULL.
> > +  @retval EFI_INVALID_PARAMETER  The ImageSize is not too small and
> > Image is NULL.
> >    @retval EFI_NOT_FOUND          The current image is not copied to
> > the buffer.
> >    @retval EFI_UNSUPPORTED        The operation is not supported.
> >    @retval EFI_SECURITY_VIOLATION The operation could not be 
> > performed due to an authentication failure.
> > --
> > 2.29.2.windows.3
> >
> >
> >
> >
> >
> >
> >
> >
> > 
> >
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114798): https://edk2.groups.io/g/devel/message/114798
Mute This Topic: https://groups.io/mt/103898765/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL
  2024-01-30  7:17             ` Xu, GuoX
@ 2024-01-30 18:18               ` Michael D Kinney
  2024-01-30 19:41                 ` Michael D Kinney
  0 siblings, 1 reply; 8+ messages in thread
From: Michael D Kinney @ 2024-01-30 18:18 UTC (permalink / raw)
  To: Xu, GuoX, gaoliming, devel@edk2.groups.io
  Cc: Liu, Zhiguang, Kinney, Michael D

Hi Guo,

One comment below.

Mike


> -----Original Message-----
> From: Xu, GuoX <guox.xu@intel.com>
> Sent: Monday, January 29, 2024 11:18 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; gaoliming
> <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>
> Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> Hi Michael,
> 
> Thanks for your suggestions.
> 
> 1. In edk2 repo there are two implementations for Firmware Management
> Protocol.
>   1.1 In edk2\FmpDevicePkg\FmpDxe\FmpDxe.c, function GetTheImageInfo():
>     //
>     // Check the buffer size
>     // NOTE: Check this first so caller can get the necessary memory
> size it must allocate.
>     //
>     if (*ImageInfoSize < (sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR))) {
>       *ImageInfoSize = sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR);
>       DEBUG ((DEBUG_VERBOSE, "FmpDxe(%s): GetImageInfo() - ImageInfoSize
> is to small.\n", mImageIdName));
>       Status = EFI_BUFFER_TOO_SMALL;
>       goto cleanup;
>     }
> 
>     //
>     // Confirm that buffer isn't null
>     //
>     if (  (ImageInfo == NULL) || (DescriptorVersion == NULL) ||
> (DescriptorCount == NULL) || (DescriptorSize == NULL)
>        || (PackageVersion == NULL))
>     {
>       DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetImageInfo() - Pointer
> Parameter is NULL.\n", mImageIdName));
>       Status = EFI_INVALID_PARAMETER;
>       goto cleanup;
>     }
>     In the line 497 to 498, it is missed the judgment of
> PackageVersionName but there is a comment in line 529 " // Do not update
> PackageVersionName since it is not supported in this instance."
>     So, do we need to add the judgment of PackageVersionName in this
> function?

Yes.  The check needs to be added for PackageVersionName being NULL for
this protocol service to be conformant with the UEFI Specification.

The fact that this specific implementation does not update PackageVersionName
contents does not change what a conformance test that checks the behavior
of a NULL parameter value would conclude that the service does not follow
the spec.

> 
>   1.2 In
> edk2\SignedCapsulePkg\Universal\SystemFirmwareUpdate\SystemFirmwareCommo
> nDxe.c, function FmpGetImageInfo():
>          if (ImageInfoSize == NULL) {
>            return EFI_INVALID_PARAMETER;
>          }
> 
>          if (*ImageInfoSize < sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR) *
> SystemFmpPrivate->DescriptorCount) {
>            *ImageInfoSize = sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR) *
> SystemFmpPrivate->DescriptorCount;
>            return EFI_BUFFER_TOO_SMALL;
>         }
> 
>         if ((ImageInfo == NULL) ||
>           (DescriptorVersion == NULL) ||
>           (DescriptorCount == NULL) ||
>           (DescriptorSize == NULL) ||
>           (PackageVersion == NULL) ||
>           (PackageVersionName == NULL))
>        {
>          return EFI_INVALID_PARAMETER;
>        }
>     The implementation already included the required code changes.
> 
>  2. In edk2-platforms repo there is only one implementation for Firmware
> Management Protocol.
>      In the file MicrocodeFmp.c, function FmpGetImageInfo():
>      If (ImageInfoSize == NULL) {
>        return EFI_INVALID_PARAMETER;
>      }
>      if (*ImageInfoSize < sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR) *
> MicrocodeFmpPrivate->DescriptorCount) {
>        *ImageInfoSize = sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR) *
> MicrocodeFmpPrivate->DescriptorCount;
>        return EFI_BUFFER_TOO_SMALL;
>      }
>      if (ImageInfo == NULL ||
>         DescriptorVersion == NULL ||
>         DescriptorCount == NULL ||
>         DescriptorSize == NULL ||
>         PackageVersion == NULL ||
>         PackageVersionName == NULL) {
>       return EFI_INVALID_PARAMETER;
>     }
>    The implementation already included the required code changes.
> 
> Thanks
> Xu Guo
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, January 23, 2024 7:08 AM
> To: Xu, GuoX <guox.xu@intel.com>; gaoliming <gaoliming@byosoft.com.cn>;
> devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> Hi Guo,
> 
> Thank you for starting this task to update Firmware Management Protocol.
> 
> There are additional tasks to make sure all required code changes are
> also implemented for a specification update like this.
> 
> * Create a TianoCore Bugzilla that describes the specification update
>   needed with links to the public documents with the required
>   specification update.
> 
> * Review/update edk2 repo include files.
> 
> * Review/update function headers of the implementations of the
>   Firmware Management Protocol in the edk2 repo.
> 
> * Review/update logic in implementations of the Firmware
>   Management Protocol in the edk2 repo to make sure all error
>   return conditions are checked.
> 
> * Review/update function headers of the implementations of the
>   Firmware Management Protocol in the edk2-platforms repo.
> 
> * Review/update logic in implementations of the Firmware
>   Management Protocol in the edk2-platforms repo to make sure
>   all error return conditions are checked.
> 
> * Review/update test cases edk2-test repo for the UEFI SCTs for
>   these error return conditions.
> 
> 
> The "REF" in the commit message should be a link to the TianoCore
> Bugzilla.  The reference to the specification is also required, but
> should be in the text of the commit message.
> 
> Thanks,
> 
> Mike
> 
> 
> > -----Original Message-----
> > From: Xu, GuoX <guox.xu@intel.com>
> > Sent: Sunday, January 21, 2024 9:37 PM
> > To: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
> > <zhiguang.liu@intel.com>
> > Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > Hi Liming,
> >   I created a PR: https://github.com/tianocore/edk2/pull/5182, could
> > you help push it ?
> >
> > Thanks
> > Xu Guo
> >
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Tuesday, January 16, 2024 10:20 PM
> > To: devel@edk2.groups.io; Xu, GuoX <guox.xu@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
> > <zhiguang.liu@intel.com>
> > Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > I am OK for this change. Reviewed-by: Liming Gao
> > <gaoliming@byosoft.com.cn>
> >
> > > -----邮件原件-----
> > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Xu, GuoX
> > > 发送时间: 2024年1月9日 17:24
> > > 收件人: devel@edk2.groups.io
> > > 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> > > 主题: Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> > >
> > > Hi all, any comments about this patch?
> > >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of GuoX
> > > Xu
> > > Sent: Monday, December 25, 2023 9:21 AM
> > > To: devel@edk2.groups.io
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> > > Li, Yi1 <yi1.li@intel.com>
> > > Subject: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> > >
> > > 1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
> > > Add the following sentence at the end of the Image parameter
> > description.
> > > "May be NULL with a zero ImageSize in order to determine the size of
> > > the buffer needed".
> > >
> > > Modify the description of "EFI_INVALID_PARAMETER" return code as
> > > "The ImageSize is not too small and Image is NULL."
> > >
> > > 2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
> > > Add the following sentence at the end of the ImageInfo parameter
> > > description."May be NULL with a zero ImageInfoSize in order to
> > > determine the size of the buffer needed".
> > >
> > > Modify the description of "EFI_INVALID_PARAMETER" return code as
> > > "The ImageInfoSize is not too small and Image is NULL." and add new
> > > descriptions for "EFI_INVALID_PARAMETER" return code.
> > >
> > > REF: UEFI Spec 2.7B Chapter 23.1.
> > >
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > > Cc: Yi Li <yi1.li@intel.com>
> > > Signed-off-by: GuoX Xu <guox.xu@intel.com>
> > > ---
> > >  MdePkg/Include/Protocol/FirmwareManagement.h | 13 +++++++++++--
> > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h
> > > b/MdePkg/Include/Protocol/FirmwareManagement.h
> > > index f37067df3455..93c8b7658e1a 100644
> > > --- a/MdePkg/Include/Protocol/FirmwareManagement.h
> > > +++ b/MdePkg/Include/Protocol/FirmwareManagement.h
> > > @@ -293,6 +293,8 @@ EFI_STATUS
> > >                                       to contain the image(s)
> > > information if the buffer was too small.
> > >    @param[in, out] ImageInfo          A pointer to the buffer in
> which
> > > firmware places the current image(s)
> > >                                       information. The information
> > > is an array of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
> > > +                                     May be NULL with a zero
> > > ImageInfoSize in order to determine the size of the
> > > +                                     buffer needed.
> > >    @param[out]     DescriptorVersion  A pointer to the location in
> > which
> > > firmware returns the version number
> > >                                       associated with the
> > > EFI_FIRMWARE_IMAGE_DESCRIPTOR.
> > >    @param[out]     DescriptorCount    A pointer to the location in
> > > which firmware returns the number of @@ -313,7 +315,12 @@ EFI_STATUS
> > >    @retval EFI_SUCCESS                The device was successfully
> > > updated with the new image.
> > >    @retval EFI_BUFFER_TOO_SMALL       The ImageInfo buffer was too
> > > small. The current buffer size
> > >                                       needed to hold the image(s)
> > > information is returned in ImageInfoSize.
> > > -  @retval EFI_INVALID_PARAMETER      ImageInfoSize is NULL.
> > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is not too small
> > > and ImageInfo is NULL.
> > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > > DescriptorVersion is NULL.
> > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > > DescriptorCount is NULL.
> > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > > DescriptorSize is NULL.
> > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > > PackageVersion is NULL.
> > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero and
> > > PackageVersionName is NULL.
> > >    @retval EFI_DEVICE_ERROR           Valid information could not be
> > > returned. Possible corrupted image.
> > >
> > >  **/
> > > @@ -340,6 +347,8 @@ EFI_STATUS
> > >    @param[in]      ImageIndex     A unique number identifying the
> > > firmware image(s) within the device.
> > >                                   The number is between 1 and
> > > DescriptorCount.
> > >    @param[out]     Image          Points to the buffer where the
> > > current image is copied to.
> > > +                                 May be NULL with a zero
> > > ImageSize in order to determine the size of the
> > > +                                 buffer needed.
> > >    @param[in, out] ImageSize      On entry, points to the size of
> the
> > > buffer pointed to by Image, in bytes.
> > >                                   On return, points to the length of
> > > the image, in bytes.
> > >
> > > @@ -347,7 +356,7 @@ EFI_STATUS
> > >    @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize
> is
> > > too small to hold the
> > >                                   image. The current buffer size
> > > needed to hold the image is returned
> > >                                   in ImageSize.
> > > -  @retval EFI_INVALID_PARAMETER  The Image was NULL.
> > > +  @retval EFI_INVALID_PARAMETER  The ImageSize is not too small and
> > > Image is NULL.
> > >    @retval EFI_NOT_FOUND          The current image is not copied to
> > > the buffer.
> > >    @retval EFI_UNSUPPORTED        The operation is not supported.
> > >    @retval EFI_SECURITY_VIOLATION The operation could not be
> > > performed due to an authentication failure.
> > > --
> > > 2.29.2.windows.3
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > 
> > >
> >
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114812): https://edk2.groups.io/g/devel/message/114812
Mute This Topic: https://groups.io/mt/103898765/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL
  2024-01-30 18:18               ` Michael D Kinney
@ 2024-01-30 19:41                 ` Michael D Kinney
  0 siblings, 0 replies; 8+ messages in thread
From: Michael D Kinney @ 2024-01-30 19:41 UTC (permalink / raw)
  To: Xu, GuoX, gaoliming, devel@edk2.groups.io, Pethaiyan, Madhan
  Cc: Liu, Zhiguang, Kinney, Michael D

Hi Guo,

I also see Madhan has been working on this same issue.

https://edk2.groups.io/g/devel/topic/103620853#114085

Can you please coordinate your patches with Madhan so it is a single patch series.

Also, my feedback to enter a BZ and update REF: with the link to the BZ has 
not been done yet.  Please do that for the next update.

Thanks,

Mike


> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, January 30, 2024 10:18 AM
> To: Xu, GuoX <guox.xu@intel.com>; gaoliming <gaoliming@byosoft.com.cn>;
> devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> Hi Guo,
> 
> One comment below.
> 
> Mike
> 
> 
> > -----Original Message-----
> > From: Xu, GuoX <guox.xu@intel.com>
> > Sent: Monday, January 29, 2024 11:18 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>; gaoliming
> > <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> > Cc: Liu, Zhiguang <zhiguang.liu@intel.com>
> > Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > Hi Michael,
> >
> > Thanks for your suggestions.
> >
> > 1. In edk2 repo there are two implementations for Firmware Management
> > Protocol.
> >   1.1 In edk2\FmpDevicePkg\FmpDxe\FmpDxe.c, function
> GetTheImageInfo():
> >     //
> >     // Check the buffer size
> >     // NOTE: Check this first so caller can get the necessary memory
> > size it must allocate.
> >     //
> >     if (*ImageInfoSize < (sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR))) {
> >       *ImageInfoSize = sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR);
> >       DEBUG ((DEBUG_VERBOSE, "FmpDxe(%s): GetImageInfo() -
> ImageInfoSize
> > is to small.\n", mImageIdName));
> >       Status = EFI_BUFFER_TOO_SMALL;
> >       goto cleanup;
> >     }
> >
> >     //
> >     // Confirm that buffer isn't null
> >     //
> >     if (  (ImageInfo == NULL) || (DescriptorVersion == NULL) ||
> > (DescriptorCount == NULL) || (DescriptorSize == NULL)
> >        || (PackageVersion == NULL))
> >     {
> >       DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetImageInfo() - Pointer
> > Parameter is NULL.\n", mImageIdName));
> >       Status = EFI_INVALID_PARAMETER;
> >       goto cleanup;
> >     }
> >     In the line 497 to 498, it is missed the judgment of
> > PackageVersionName but there is a comment in line 529 " // Do not
> update
> > PackageVersionName since it is not supported in this instance."
> >     So, do we need to add the judgment of PackageVersionName in this
> > function?
> 
> Yes.  The check needs to be added for PackageVersionName being NULL for
> this protocol service to be conformant with the UEFI Specification.
> 
> The fact that this specific implementation does not update
> PackageVersionName
> contents does not change what a conformance test that checks the
> behavior
> of a NULL parameter value would conclude that the service does not
> follow
> the spec.
> 
> >
> >   1.2 In
> >
> edk2\SignedCapsulePkg\Universal\SystemFirmwareUpdate\SystemFirmwareCommo
> > nDxe.c, function FmpGetImageInfo():
> >          if (ImageInfoSize == NULL) {
> >            return EFI_INVALID_PARAMETER;
> >          }
> >
> >          if (*ImageInfoSize < sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR) *
> > SystemFmpPrivate->DescriptorCount) {
> >            *ImageInfoSize = sizeof (EFI_FIRMWARE_IMAGE_DESCRIPTOR) *
> > SystemFmpPrivate->DescriptorCount;
> >            return EFI_BUFFER_TOO_SMALL;
> >         }
> >
> >         if ((ImageInfo == NULL) ||
> >           (DescriptorVersion == NULL) ||
> >           (DescriptorCount == NULL) ||
> >           (DescriptorSize == NULL) ||
> >           (PackageVersion == NULL) ||
> >           (PackageVersionName == NULL))
> >        {
> >          return EFI_INVALID_PARAMETER;
> >        }
> >     The implementation already included the required code changes.
> >
> >  2. In edk2-platforms repo there is only one implementation for
> Firmware
> > Management Protocol.
> >      In the file MicrocodeFmp.c, function FmpGetImageInfo():
> >      If (ImageInfoSize == NULL) {
> >        return EFI_INVALID_PARAMETER;
> >      }
> >      if (*ImageInfoSize < sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR) *
> > MicrocodeFmpPrivate->DescriptorCount) {
> >        *ImageInfoSize = sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR) *
> > MicrocodeFmpPrivate->DescriptorCount;
> >        return EFI_BUFFER_TOO_SMALL;
> >      }
> >      if (ImageInfo == NULL ||
> >         DescriptorVersion == NULL ||
> >         DescriptorCount == NULL ||
> >         DescriptorSize == NULL ||
> >         PackageVersion == NULL ||
> >         PackageVersionName == NULL) {
> >       return EFI_INVALID_PARAMETER;
> >     }
> >    The implementation already included the required code changes.
> >
> > Thanks
> > Xu Guo
> >
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Tuesday, January 23, 2024 7:08 AM
> > To: Xu, GuoX <guox.xu@intel.com>; gaoliming
> <gaoliming@byosoft.com.cn>;
> > devel@edk2.groups.io
> > Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > Hi Guo,
> >
> > Thank you for starting this task to update Firmware Management
> Protocol.
> >
> > There are additional tasks to make sure all required code changes are
> > also implemented for a specification update like this.
> >
> > * Create a TianoCore Bugzilla that describes the specification update
> >   needed with links to the public documents with the required
> >   specification update.
> >
> > * Review/update edk2 repo include files.
> >
> > * Review/update function headers of the implementations of the
> >   Firmware Management Protocol in the edk2 repo.
> >
> > * Review/update logic in implementations of the Firmware
> >   Management Protocol in the edk2 repo to make sure all error
> >   return conditions are checked.
> >
> > * Review/update function headers of the implementations of the
> >   Firmware Management Protocol in the edk2-platforms repo.
> >
> > * Review/update logic in implementations of the Firmware
> >   Management Protocol in the edk2-platforms repo to make sure
> >   all error return conditions are checked.
> >
> > * Review/update test cases edk2-test repo for the UEFI SCTs for
> >   these error return conditions.
> >
> >
> > The "REF" in the commit message should be a link to the TianoCore
> > Bugzilla.  The reference to the specification is also required, but
> > should be in the text of the commit message.
> >
> > Thanks,
> >
> > Mike
> >
> >
> > > -----Original Message-----
> > > From: Xu, GuoX <guox.xu@intel.com>
> > > Sent: Sunday, January 21, 2024 9:37 PM
> > > To: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
> > > <zhiguang.liu@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> > >
> > > Hi Liming,
> > >   I created a PR: https://github.com/tianocore/edk2/pull/5182, could
> > > you help push it ?
> > >
> > > Thanks
> > > Xu Guo
> > >
> > > -----Original Message-----
> > > From: gaoliming <gaoliming@byosoft.com.cn>
> > > Sent: Tuesday, January 16, 2024 10:20 PM
> > > To: devel@edk2.groups.io; Xu, GuoX <guox.xu@intel.com>
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
> > > <zhiguang.liu@intel.com>
> > > Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments
> of
> > > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> > >
> > > I am OK for this change. Reviewed-by: Liming Gao
> > > <gaoliming@byosoft.com.cn>
> > >
> > > > -----邮件原件-----
> > > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Xu, GuoX
> > > > 发送时间: 2024年1月9日 17:24
> > > > 收件人: devel@edk2.groups.io
> > > > 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> > > > 主题: Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > > > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> > > >
> > > > Hi all, any comments about this patch?
> > > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> GuoX
> > > > Xu
> > > > Sent: Monday, December 25, 2023 9:21 AM
> > > > To: devel@edk2.groups.io
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>;
> > > > Li, Yi1 <yi1.li@intel.com>
> > > > Subject: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > > > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> > > >
> > > > 1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
> > > > Add the following sentence at the end of the Image parameter
> > > description.
> > > > "May be NULL with a zero ImageSize in order to determine the size
> of
> > > > the buffer needed".
> > > >
> > > > Modify the description of "EFI_INVALID_PARAMETER" return code as
> > > > "The ImageSize is not too small and Image is NULL."
> > > >
> > > > 2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
> > > > Add the following sentence at the end of the ImageInfo parameter
> > > > description."May be NULL with a zero ImageInfoSize in order to
> > > > determine the size of the buffer needed".
> > > >
> > > > Modify the description of "EFI_INVALID_PARAMETER" return code as
> > > > "The ImageInfoSize is not too small and Image is NULL." and add
> new
> > > > descriptions for "EFI_INVALID_PARAMETER" return code.
> > > >
> > > > REF: UEFI Spec 2.7B Chapter 23.1.
> > > >
> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > > > Cc: Yi Li <yi1.li@intel.com>
> > > > Signed-off-by: GuoX Xu <guox.xu@intel.com>
> > > > ---
> > > >  MdePkg/Include/Protocol/FirmwareManagement.h | 13 +++++++++++--
> > > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h
> > > > b/MdePkg/Include/Protocol/FirmwareManagement.h
> > > > index f37067df3455..93c8b7658e1a 100644
> > > > --- a/MdePkg/Include/Protocol/FirmwareManagement.h
> > > > +++ b/MdePkg/Include/Protocol/FirmwareManagement.h
> > > > @@ -293,6 +293,8 @@ EFI_STATUS
> > > >                                       to contain the image(s)
> > > > information if the buffer was too small.
> > > >    @param[in, out] ImageInfo          A pointer to the buffer in
> > which
> > > > firmware places the current image(s)
> > > >                                       information. The information
> > > > is an array of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
> > > > +                                     May be NULL with a zero
> > > > ImageInfoSize in order to determine the size of the
> > > > +                                     buffer needed.
> > > >    @param[out]     DescriptorVersion  A pointer to the location in
> > > which
> > > > firmware returns the version number
> > > >                                       associated with the
> > > > EFI_FIRMWARE_IMAGE_DESCRIPTOR.
> > > >    @param[out]     DescriptorCount    A pointer to the location in
> > > > which firmware returns the number of @@ -313,7 +315,12 @@
> EFI_STATUS
> > > >    @retval EFI_SUCCESS                The device was successfully
> > > > updated with the new image.
> > > >    @retval EFI_BUFFER_TOO_SMALL       The ImageInfo buffer was too
> > > > small. The current buffer size
> > > >                                       needed to hold the image(s)
> > > > information is returned in ImageInfoSize.
> > > > -  @retval EFI_INVALID_PARAMETER      ImageInfoSize is NULL.
> > > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is not too
> small
> > > > and ImageInfo is NULL.
> > > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero
> and
> > > > DescriptorVersion is NULL.
> > > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero
> and
> > > > DescriptorCount is NULL.
> > > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero
> and
> > > > DescriptorSize is NULL.
> > > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero
> and
> > > > PackageVersion is NULL.
> > > > +  @retval EFI_INVALID_PARAMETER      ImageInfoSize is non-zero
> and
> > > > PackageVersionName is NULL.
> > > >    @retval EFI_DEVICE_ERROR           Valid information could not
> be
> > > > returned. Possible corrupted image.
> > > >
> > > >  **/
> > > > @@ -340,6 +347,8 @@ EFI_STATUS
> > > >    @param[in]      ImageIndex     A unique number identifying the
> > > > firmware image(s) within the device.
> > > >                                   The number is between 1 and
> > > > DescriptorCount.
> > > >    @param[out]     Image          Points to the buffer where the
> > > > current image is copied to.
> > > > +                                 May be NULL with a zero
> > > > ImageSize in order to determine the size of the
> > > > +                                 buffer needed.
> > > >    @param[in, out] ImageSize      On entry, points to the size of
> > the
> > > > buffer pointed to by Image, in bytes.
> > > >                                   On return, points to the length
> of
> > > > the image, in bytes.
> > > >
> > > > @@ -347,7 +356,7 @@ EFI_STATUS
> > > >    @retval EFI_BUFFER_TOO_SMALL   The buffer specified by
> ImageSize
> > is
> > > > too small to hold the
> > > >                                   image. The current buffer size
> > > > needed to hold the image is returned
> > > >                                   in ImageSize.
> > > > -  @retval EFI_INVALID_PARAMETER  The Image was NULL.
> > > > +  @retval EFI_INVALID_PARAMETER  The ImageSize is not too small
> and
> > > > Image is NULL.
> > > >    @retval EFI_NOT_FOUND          The current image is not copied
> to
> > > > the buffer.
> > > >    @retval EFI_UNSUPPORTED        The operation is not supported.
> > > >    @retval EFI_SECURITY_VIOLATION The operation could not be
> > > > performed due to an authentication failure.
> > > > --
> > > > 2.29.2.windows.3
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > 
> > > >
> > >
> > >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114825): https://edk2.groups.io/g/devel/message/114825
Mute This Topic: https://groups.io/mt/103898765/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-01-30 19:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1703466776.git.guox.xu@intel.com>
2023-12-25  1:21 ` [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL GuoX Xu
     [not found]   ` <CO1PR11MB49167DF996E427FCE9B823D6966A2@CO1PR11MB4916.namprd11.prod.outlook.com>
2024-01-09  9:24     ` Xu, GuoX
2024-01-16 14:20       ` 回复: " gaoliming via groups.io
2024-01-22  5:37         ` Xu, GuoX
2024-01-22 23:08           ` Michael D Kinney
2024-01-30  7:17             ` Xu, GuoX
2024-01-30 18:18               ` Michael D Kinney
2024-01-30 19:41                 ` Michael D Kinney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox