public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call FmpDeviceLib WithStatus() functions
@ 2020-10-30 21:12 Michael Kubacki
  2020-11-06  3:16 ` Michael Kubacki
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Kubacki @ 2020-10-30 21:12 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Michael D Kinney, Guomin Jiang, Wei6 Xu

From: Michael Kubacki <michael.kubacki@microsoft.com>

Commit 6ad819c introduced two new functions in FmpDeviceLib:
1. FmpDeviceCheckImageWithStatus ()
2. FmpDeviceSetImageWithStatus ()

These functions allow an FmpDeviceLib implementation to return a
Last Attempt Status code value within the Device Library range from
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.

To maintain backward compatibility, commit 6ad819c did not update
the FmpDxe driver to invoke these functions. FmpDeviceLib instances
should update their FmpDeviceCheckImage () function to simply call
FmpDeviceCheckImageWithStatus (). Similarly, FmpDeviceSetImage ()
should simply call FmpDeviceSetImageWithStatus (). This is
demonstrated in the implementation of these functions in
FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c. By doing so,
the library can remain compatible with FmpDxe implementations before
and after this transition.

This commit updates FmpDxe to call the WithStatus () version of
these functions enabling the Last Attempt Status code returned to
be accessible to FmpDxe.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 FmpDevicePkg/FmpDxe/FmpDxe.c | 40 ++++++++++++++++++--
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index de7f1fe53e32..6b0675ea38f8 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -1025,9 +1025,24 @@ CheckTheImageInternal (
   //
   // FmpDeviceLib CheckImage function to do any specific checks
   //
-  Status = FmpDeviceCheckImage ((((UINT8 *)Image) + AllHeaderSize), RawSize, ImageUpdatable);
+  Status = FmpDeviceCheckImageWithStatus ((((UINT8 *) Image) + AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - FmpDeviceLib CheckImage failed. Status = %r\n", mImageIdName, Status));
+
+    //
+    // LastAttemptStatus returned from the device library should fall within the designated error range
+    // [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE, LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
+    //
+    if ((*LastAttemptStatus < LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
+        (*LastAttemptStatus > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) {
+      DEBUG (
+        (DEBUG_ERROR,
+        "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from FmpDeviceCheckImageWithStatus() is invalid.\n",
+        mImageIdName,
+        *LastAttemptStatus)
+        );
+      *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
+    }
   }
 
 cleanup:
@@ -1353,16 +1368,33 @@ SetTheImage (
   //
   //Copy the requested image to the firmware using the FmpDeviceLib
   //
-  Status = FmpDeviceSetImage (
-             (((UINT8 *)Image) + AllHeaderSize),
+  Status = FmpDeviceSetImageWithStatus (
+             (((UINT8 *) Image) + AllHeaderSize),
              ImageSize - AllHeaderSize,
              VendorCode,
              FmpDxeProgress,
              IncomingFwVersion,
-             AbortReason
+             AbortReason,
+             &LastAttemptStatus
              );
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() SetImage from FmpDeviceLib failed. Status =  %r.\n", mImageIdName, Status));
+
+    //
+    // LastAttemptStatus returned from the device library should fall within the designated error range
+    // [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE, LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
+    //
+    if ((LastAttemptStatus < LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
+        (LastAttemptStatus > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) {
+      DEBUG (
+        (DEBUG_ERROR,
+        "FmpDxe(%s): SetTheImage() - LastAttemptStatus %d from FmpDeviceSetImageWithStatus() is invalid.\n",
+        mImageIdName,
+        LastAttemptStatus)
+        );
+      LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
+    }
+
     goto cleanup;
   }
 
-- 
2.28.0.windows.1


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

* Re: [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call FmpDeviceLib WithStatus() functions
  2020-10-30 21:12 [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call FmpDeviceLib WithStatus() functions Michael Kubacki
@ 2020-11-06  3:16 ` Michael Kubacki
  2020-11-06  5:15   ` [edk2-devel] " Xu, Wei6
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Kubacki @ 2020-11-06  3:16 UTC (permalink / raw)
  To: devel, Liming Gao, Michael D Kinney, Guomin Jiang, Wei6 Xu

Hi FmpDevicePkg maintainers,

Please let me know your feedback on this patch. Note that it is required 
to complete the Last Attempt Status support already merged in the 
following patch series:
https://edk2.groups.io/g/devel/message/66418

Thanks,
Michael

On 10/30/2020 2:12 PM, michael.kubacki@outlook.com wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Commit 6ad819c introduced two new functions in FmpDeviceLib:
> 1. FmpDeviceCheckImageWithStatus ()
> 2. FmpDeviceSetImageWithStatus ()
> 
> These functions allow an FmpDeviceLib implementation to return a
> Last Attempt Status code value within the Device Library range from
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
> 
> To maintain backward compatibility, commit 6ad819c did not update
> the FmpDxe driver to invoke these functions. FmpDeviceLib instances
> should update their FmpDeviceCheckImage () function to simply call
> FmpDeviceCheckImageWithStatus (). Similarly, FmpDeviceSetImage ()
> should simply call FmpDeviceSetImageWithStatus (). This is
> demonstrated in the implementation of these functions in
> FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c. By doing so,
> the library can remain compatible with FmpDxe implementations before
> and after this transition.
> 
> This commit updates FmpDxe to call the WithStatus () version of
> these functions enabling the Last Attempt Status code returned to
> be accessible to FmpDxe.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Guomin Jiang <guomin.jiang@intel.com>
> Cc: Wei6 Xu <wei6.xu@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>   FmpDevicePkg/FmpDxe/FmpDxe.c | 40 ++++++++++++++++++--
>   1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
> index de7f1fe53e32..6b0675ea38f8 100644
> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> @@ -1025,9 +1025,24 @@ CheckTheImageInternal (
>     //
>     // FmpDeviceLib CheckImage function to do any specific checks
>     //
> -  Status = FmpDeviceCheckImage ((((UINT8 *)Image) + AllHeaderSize), RawSize, ImageUpdatable);
> +  Status = FmpDeviceCheckImageWithStatus ((((UINT8 *) Image) + AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus);
>     if (EFI_ERROR (Status)) {
>       DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - FmpDeviceLib CheckImage failed. Status = %r\n", mImageIdName, Status));
> +
> +    //
> +    // LastAttemptStatus returned from the device library should fall within the designated error range
> +    // [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE, LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
> +    //
> +    if ((*LastAttemptStatus < LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
> +        (*LastAttemptStatus > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) {
> +      DEBUG (
> +        (DEBUG_ERROR,
> +        "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from FmpDeviceCheckImageWithStatus() is invalid.\n",
> +        mImageIdName,
> +        *LastAttemptStatus)
> +        );
> +      *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
> +    }
>     }
>   
>   cleanup:
> @@ -1353,16 +1368,33 @@ SetTheImage (
>     //
>     //Copy the requested image to the firmware using the FmpDeviceLib
>     //
> -  Status = FmpDeviceSetImage (
> -             (((UINT8 *)Image) + AllHeaderSize),
> +  Status = FmpDeviceSetImageWithStatus (
> +             (((UINT8 *) Image) + AllHeaderSize),
>                ImageSize - AllHeaderSize,
>                VendorCode,
>                FmpDxeProgress,
>                IncomingFwVersion,
> -             AbortReason
> +             AbortReason,
> +             &LastAttemptStatus
>                );
>     if (EFI_ERROR (Status)) {
>       DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() SetImage from FmpDeviceLib failed. Status =  %r.\n", mImageIdName, Status));
> +
> +    //
> +    // LastAttemptStatus returned from the device library should fall within the designated error range
> +    // [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE, LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
> +    //
> +    if ((LastAttemptStatus < LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
> +        (LastAttemptStatus > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) {
> +      DEBUG (
> +        (DEBUG_ERROR,
> +        "FmpDxe(%s): SetTheImage() - LastAttemptStatus %d from FmpDeviceSetImageWithStatus() is invalid.\n",
> +        mImageIdName,
> +        LastAttemptStatus)
> +        );
> +      LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
> +    }
> +
>       goto cleanup;
>     }
>   
> 

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

* Re: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call FmpDeviceLib WithStatus() functions
  2020-11-06  3:16 ` Michael Kubacki
@ 2020-11-06  5:15   ` Xu, Wei6
  2020-11-09  1:03     ` 回复: " gaoliming
       [not found]     ` <1645B1531363BCC0.9654@groups.io>
  0 siblings, 2 replies; 5+ messages in thread
From: Xu, Wei6 @ 2020-11-06  5:15 UTC (permalink / raw)
  To: devel@edk2.groups.io, michael.kubacki@outlook.com, Liming Gao,
	Kinney, Michael D, Jiang, Guomin

This patch is good to me.
Reviewed-by: Wei6 Xu <wei6.xu@intel.com>

BR,
Wei
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
Sent: Friday, November 6, 2020 11:16 AM
To: devel@edk2.groups.io; Liming Gao <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; Xu, Wei6 <wei6.xu@intel.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call FmpDeviceLib WithStatus() functions

Hi FmpDevicePkg maintainers,

Please let me know your feedback on this patch. Note that it is required to complete the Last Attempt Status support already merged in the following patch series:
https://edk2.groups.io/g/devel/message/66418

Thanks,
Michael

On 10/30/2020 2:12 PM, michael.kubacki@outlook.com wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Commit 6ad819c introduced two new functions in FmpDeviceLib:
> 1. FmpDeviceCheckImageWithStatus ()
> 2. FmpDeviceSetImageWithStatus ()
> 
> These functions allow an FmpDeviceLib implementation to return a Last 
> Attempt Status code value within the Device Library range from 
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to 
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
> 
> To maintain backward compatibility, commit 6ad819c did not update the 
> FmpDxe driver to invoke these functions. FmpDeviceLib instances should 
> update their FmpDeviceCheckImage () function to simply call 
> FmpDeviceCheckImageWithStatus (). Similarly, FmpDeviceSetImage () 
> should simply call FmpDeviceSetImageWithStatus (). This is 
> demonstrated in the implementation of these functions in 
> FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c. By doing so, the 
> library can remain compatible with FmpDxe implementations before and 
> after this transition.
> 
> This commit updates FmpDxe to call the WithStatus () version of these 
> functions enabling the Last Attempt Status code returned to be 
> accessible to FmpDxe.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Guomin Jiang <guomin.jiang@intel.com>
> Cc: Wei6 Xu <wei6.xu@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>   FmpDevicePkg/FmpDxe/FmpDxe.c | 40 ++++++++++++++++++--
>   1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c 
> b/FmpDevicePkg/FmpDxe/FmpDxe.c index de7f1fe53e32..6b0675ea38f8 100644
> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> @@ -1025,9 +1025,24 @@ CheckTheImageInternal (
>     //
>     // FmpDeviceLib CheckImage function to do any specific checks
>     //
> -  Status = FmpDeviceCheckImage ((((UINT8 *)Image) + AllHeaderSize), 
> RawSize, ImageUpdatable);
> +  Status = FmpDeviceCheckImageWithStatus ((((UINT8 *) Image) + 
> + AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus);
>     if (EFI_ERROR (Status)) {
>       DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - FmpDeviceLib 
> CheckImage failed. Status = %r\n", mImageIdName, Status));
> +
> +    //
> +    // LastAttemptStatus returned from the device library should fall within the designated error range
> +    // [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE, LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
> +    //
> +    if ((*LastAttemptStatus < LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
> +        (*LastAttemptStatus > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) {
> +      DEBUG (
> +        (DEBUG_ERROR,
> +        "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from FmpDeviceCheckImageWithStatus() is invalid.\n",
> +        mImageIdName,
> +        *LastAttemptStatus)
> +        );
> +      *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
> +    }
>     }
>   
>   cleanup:
> @@ -1353,16 +1368,33 @@ SetTheImage (
>     //
>     //Copy the requested image to the firmware using the FmpDeviceLib
>     //
> -  Status = FmpDeviceSetImage (
> -             (((UINT8 *)Image) + AllHeaderSize),
> +  Status = FmpDeviceSetImageWithStatus (
> +             (((UINT8 *) Image) + AllHeaderSize),
>                ImageSize - AllHeaderSize,
>                VendorCode,
>                FmpDxeProgress,
>                IncomingFwVersion,
> -             AbortReason
> +             AbortReason,
> +             &LastAttemptStatus
>                );
>     if (EFI_ERROR (Status)) {
>       DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() SetImage from 
> FmpDeviceLib failed. Status =  %r.\n", mImageIdName, Status));
> +
> +    //
> +    // LastAttemptStatus returned from the device library should fall within the designated error range
> +    // [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE, LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
> +    //
> +    if ((LastAttemptStatus < LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
> +        (LastAttemptStatus > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) {
> +      DEBUG (
> +        (DEBUG_ERROR,
> +        "FmpDxe(%s): SetTheImage() - LastAttemptStatus %d from FmpDeviceSetImageWithStatus() is invalid.\n",
> +        mImageIdName,
> +        LastAttemptStatus)
> +        );
> +      LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
> +    }
> +
>       goto cleanup;
>     }
>   
> 






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

* 回复: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call FmpDeviceLib WithStatus() functions
  2020-11-06  5:15   ` [edk2-devel] " Xu, Wei6
@ 2020-11-09  1:03     ` gaoliming
       [not found]     ` <1645B1531363BCC0.9654@groups.io>
  1 sibling, 0 replies; 5+ messages in thread
From: gaoliming @ 2020-11-09  1:03 UTC (permalink / raw)
  To: 'Xu, Wei6', devel, michael.kubacki,
	'Kinney, Michael D', 'Jiang, Guomin'

Acked-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: Xu, Wei6 <wei6.xu@intel.com>
> 发送时间: 2020年11月6日 13:15
> 收件人: devel@edk2.groups.io; michael.kubacki@outlook.com; Liming Gao
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> 主题: RE: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call
> FmpDeviceLib WithStatus() functions
> 
> This patch is good to me.
> Reviewed-by: Wei6 Xu <wei6.xu@intel.com>
> 
> BR,
> Wei
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Friday, November 6, 2020 11:16 AM
> To: devel@edk2.groups.io; Liming Gao <gaoliming@byosoft.com.cn>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Jiang, Guomin
> <guomin.jiang@intel.com>; Xu, Wei6 <wei6.xu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call
> FmpDeviceLib WithStatus() functions
> 
> Hi FmpDevicePkg maintainers,
> 
> Please let me know your feedback on this patch. Note that it is required to
> complete the Last Attempt Status support already merged in the following
> patch series:
> https://edk2.groups.io/g/devel/message/66418
> 
> Thanks,
> Michael
> 
> On 10/30/2020 2:12 PM, michael.kubacki@outlook.com wrote:
> > From: Michael Kubacki <michael.kubacki@microsoft.com>
> >
> > Commit 6ad819c introduced two new functions in FmpDeviceLib:
> > 1. FmpDeviceCheckImageWithStatus ()
> > 2. FmpDeviceSetImageWithStatus ()
> >
> > These functions allow an FmpDeviceLib implementation to return a Last
> > Attempt Status code value within the Device Library range from
> > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
> >
> > To maintain backward compatibility, commit 6ad819c did not update the
> > FmpDxe driver to invoke these functions. FmpDeviceLib instances should
> > update their FmpDeviceCheckImage () function to simply call
> > FmpDeviceCheckImageWithStatus (). Similarly, FmpDeviceSetImage ()
> > should simply call FmpDeviceSetImageWithStatus (). This is
> > demonstrated in the implementation of these functions in
> > FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c. By doing so, the
> > library can remain compatible with FmpDxe implementations before and
> > after this transition.
> >
> > This commit updates FmpDxe to call the WithStatus () version of these
> > functions enabling the Last Attempt Status code returned to be
> > accessible to FmpDxe.
> >
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > Cc: Wei6 Xu <wei6.xu@intel.com>
> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> > ---
> >   FmpDevicePkg/FmpDxe/FmpDxe.c | 40 ++++++++++++++++++--
> >   1 file changed, 36 insertions(+), 4 deletions(-)
> >
> > diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> > b/FmpDevicePkg/FmpDxe/FmpDxe.c index de7f1fe53e32..6b0675ea38f8
> 100644
> > --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> > +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> > @@ -1025,9 +1025,24 @@ CheckTheImageInternal (
> >     //
> >     // FmpDeviceLib CheckImage function to do any specific checks
> >     //
> > -  Status = FmpDeviceCheckImage ((((UINT8 *)Image) + AllHeaderSize),
> > RawSize, ImageUpdatable);
> > +  Status = FmpDeviceCheckImageWithStatus ((((UINT8 *) Image) +
> > + AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus);
> >     if (EFI_ERROR (Status)) {
> >       DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() -
> FmpDeviceLib
> > CheckImage failed. Status = %r\n", mImageIdName, Status));
> > +
> > +    //
> > +    // LastAttemptStatus returned from the device library should fall
> within the designated error range
> > +    //
> [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE,
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
> > +    //
> > +    if ((*LastAttemptStatus <
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
> > +        (*LastAttemptStatus >
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) {
> > +      DEBUG (
> > +        (DEBUG_ERROR,
> > +        "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from
> FmpDeviceCheckImageWithStatus() is invalid.\n",
> > +        mImageIdName,
> > +        *LastAttemptStatus)
> > +        );
> > +      *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
> > +    }
> >     }
> >
> >   cleanup:
> > @@ -1353,16 +1368,33 @@ SetTheImage (
> >     //
> >     //Copy the requested image to the firmware using the FmpDeviceLib
> >     //
> > -  Status = FmpDeviceSetImage (
> > -             (((UINT8 *)Image) + AllHeaderSize),
> > +  Status = FmpDeviceSetImageWithStatus (
> > +             (((UINT8 *) Image) + AllHeaderSize),
> >                ImageSize - AllHeaderSize,
> >                VendorCode,
> >                FmpDxeProgress,
> >                IncomingFwVersion,
> > -             AbortReason
> > +             AbortReason,
> > +             &LastAttemptStatus
> >                );
> >     if (EFI_ERROR (Status)) {
> >       DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() SetImage
> from
> > FmpDeviceLib failed. Status =  %r.\n", mImageIdName, Status));
> > +
> > +    //
> > +    // LastAttemptStatus returned from the device library should fall
> within the designated error range
> > +    //
> [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE,
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
> > +    //
> > +    if ((LastAttemptStatus <
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
> > +        (LastAttemptStatus >
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) {
> > +      DEBUG (
> > +        (DEBUG_ERROR,
> > +        "FmpDxe(%s): SetTheImage() - LastAttemptStatus %d from
> FmpDeviceSetImageWithStatus() is invalid.\n",
> > +        mImageIdName,
> > +        LastAttemptStatus)
> > +        );
> > +      LastAttemptStatus =
> LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
> > +    }
> > +
> >       goto cleanup;
> >     }
> >
> >
> 
> 
> 
> 




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

* 回复: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call FmpDeviceLib WithStatus() functions
       [not found]     ` <1645B1531363BCC0.9654@groups.io>
@ 2020-11-10  1:28       ` gaoliming
  0 siblings, 0 replies; 5+ messages in thread
From: gaoliming @ 2020-11-10  1:28 UTC (permalink / raw)
  To: devel, gaoliming, 'Xu, Wei6', michael.kubacki,
	'Kinney, Michael D', 'Jiang, Guomin'

Create PR https://github.com/tianocore/edk2/pull/1103 for this patch. 

> -----邮件原件-----
> 发件人: bounce+27952+67127+4905953+8761045@groups.io
> <bounce+27952+67127+4905953+8761045@groups.io> 代表 gaoliming
> 发送时间: 2020年11月9日 9:03
> 收件人: 'Xu, Wei6' <wei6.xu@intel.com>; devel@edk2.groups.io;
> michael.kubacki@outlook.com; 'Kinney, Michael D'
> <michael.d.kinney@intel.com>; 'Jiang, Guomin' <guomin.jiang@intel.com>
> 主题: 回复: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call
> FmpDeviceLib WithStatus() functions
> 
> Acked-by: Liming Gao <gaoliming@byosoft.com.cn>
> 
> > -----邮件原件-----
> > 发件人: Xu, Wei6 <wei6.xu@intel.com>
> > 发送时间: 2020年11月6日 13:15
> > 收件人: devel@edk2.groups.io; michael.kubacki@outlook.com; Liming
> Gao
> > <gaoliming@byosoft.com.cn>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > 主题: RE: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call
> > FmpDeviceLib WithStatus() functions
> >
> > This patch is good to me.
> > Reviewed-by: Wei6 Xu <wei6.xu@intel.com>
> >
> > BR,
> > Wei
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> > Kubacki
> > Sent: Friday, November 6, 2020 11:16 AM
> > To: devel@edk2.groups.io; Liming Gao <gaoliming@byosoft.com.cn>;
> Kinney,
> > Michael D <michael.d.kinney@intel.com>; Jiang, Guomin
> > <guomin.jiang@intel.com>; Xu, Wei6 <wei6.xu@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call
> > FmpDeviceLib WithStatus() functions
> >
> > Hi FmpDevicePkg maintainers,
> >
> > Please let me know your feedback on this patch. Note that it is required to
> > complete the Last Attempt Status support already merged in the following
> > patch series:
> > https://edk2.groups.io/g/devel/message/66418
> >
> > Thanks,
> > Michael
> >
> > On 10/30/2020 2:12 PM, michael.kubacki@outlook.com wrote:
> > > From: Michael Kubacki <michael.kubacki@microsoft.com>
> > >
> > > Commit 6ad819c introduced two new functions in FmpDeviceLib:
> > > 1. FmpDeviceCheckImageWithStatus ()
> > > 2. FmpDeviceSetImageWithStatus ()
> > >
> > > These functions allow an FmpDeviceLib implementation to return a Last
> > > Attempt Status code value within the Device Library range from
> > > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> > > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
> > >
> > > To maintain backward compatibility, commit 6ad819c did not update the
> > > FmpDxe driver to invoke these functions. FmpDeviceLib instances should
> > > update their FmpDeviceCheckImage () function to simply call
> > > FmpDeviceCheckImageWithStatus (). Similarly, FmpDeviceSetImage ()
> > > should simply call FmpDeviceSetImageWithStatus (). This is
> > > demonstrated in the implementation of these functions in
> > > FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c. By doing so,
> the
> > > library can remain compatible with FmpDxe implementations before and
> > > after this transition.
> > >
> > > This commit updates FmpDxe to call the WithStatus () version of these
> > > functions enabling the Last Attempt Status code returned to be
> > > accessible to FmpDxe.
> > >
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > > Cc: Wei6 Xu <wei6.xu@intel.com>
> > > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> > > ---
> > >   FmpDevicePkg/FmpDxe/FmpDxe.c | 40 ++++++++++++++++++--
> > >   1 file changed, 36 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> > > b/FmpDevicePkg/FmpDxe/FmpDxe.c index de7f1fe53e32..6b0675ea38f8
> > 100644
> > > --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> > > +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> > > @@ -1025,9 +1025,24 @@ CheckTheImageInternal (
> > >     //
> > >     // FmpDeviceLib CheckImage function to do any specific checks
> > >     //
> > > -  Status = FmpDeviceCheckImage ((((UINT8 *)Image) + AllHeaderSize),
> > > RawSize, ImageUpdatable);
> > > +  Status = FmpDeviceCheckImageWithStatus ((((UINT8 *) Image) +
> > > + AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus);
> > >     if (EFI_ERROR (Status)) {
> > >       DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() -
> > FmpDeviceLib
> > > CheckImage failed. Status = %r\n", mImageIdName, Status));
> > > +
> > > +    //
> > > +    // LastAttemptStatus returned from the device library should fall
> > within the designated error range
> > > +    //
> > [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE,
> > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
> > > +    //
> > > +    if ((*LastAttemptStatus <
> > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
> > > +        (*LastAttemptStatus >
> > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) {
> > > +      DEBUG (
> > > +        (DEBUG_ERROR,
> > > +        "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from
> > FmpDeviceCheckImageWithStatus() is invalid.\n",
> > > +        mImageIdName,
> > > +        *LastAttemptStatus)
> > > +        );
> > > +      *LastAttemptStatus =
> > LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
> > > +    }
> > >     }
> > >
> > >   cleanup:
> > > @@ -1353,16 +1368,33 @@ SetTheImage (
> > >     //
> > >     //Copy the requested image to the firmware using the FmpDeviceLib
> > >     //
> > > -  Status = FmpDeviceSetImage (
> > > -             (((UINT8 *)Image) + AllHeaderSize),
> > > +  Status = FmpDeviceSetImageWithStatus (
> > > +             (((UINT8 *) Image) + AllHeaderSize),
> > >                ImageSize - AllHeaderSize,
> > >                VendorCode,
> > >                FmpDxeProgress,
> > >                IncomingFwVersion,
> > > -             AbortReason
> > > +             AbortReason,
> > > +             &LastAttemptStatus
> > >                );
> > >     if (EFI_ERROR (Status)) {
> > >       DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() SetImage
> > from
> > > FmpDeviceLib failed. Status =  %r.\n", mImageIdName, Status));
> > > +
> > > +    //
> > > +    // LastAttemptStatus returned from the device library should fall
> > within the designated error range
> > > +    //
> > [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE,
> > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
> > > +    //
> > > +    if ((LastAttemptStatus <
> > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
> > > +        (LastAttemptStatus >
> > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) {
> > > +      DEBUG (
> > > +        (DEBUG_ERROR,
> > > +        "FmpDxe(%s): SetTheImage() - LastAttemptStatus %d from
> > FmpDeviceSetImageWithStatus() is invalid.\n",
> > > +        mImageIdName,
> > > +        LastAttemptStatus)
> > > +        );
> > > +      LastAttemptStatus =
> > LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
> > > +    }
> > > +
> > >       goto cleanup;
> > >     }
> > >
> > >
> >
> >
> >
> >
> 
> 
> 
> 
> 
> 
> 




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

end of thread, other threads:[~2020-11-10  1:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-30 21:12 [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Call FmpDeviceLib WithStatus() functions Michael Kubacki
2020-11-06  3:16 ` Michael Kubacki
2020-11-06  5:15   ` [edk2-devel] " Xu, Wei6
2020-11-09  1:03     ` 回复: " gaoliming
     [not found]     ` <1645B1531363BCC0.9654@groups.io>
2020-11-10  1:28       ` gaoliming

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