* [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update FmpDeviceCheckImageWithStatus() handling
@ 2022-01-04 20:38 Michael Kubacki
2022-01-07 3:45 ` Guomin Jiang
0 siblings, 1 reply; 4+ messages in thread
From: Michael Kubacki @ 2022-01-04 20:38 UTC (permalink / raw)
To: devel; +Cc: Liming Gao, Michael D Kinney, Guomin Jiang, Wei6 Xu
From: Michael Kubacki <michael.kubacki@microsoft.com>
Update the logic handling last attempt status codes from
FmpDeviceCheckImageWithStatus() implementations to account for
cases when the function return status code is EFI_SUCCESS
(since the image was checked successfully) but the ImageUpdatable
value is not valid.
In addition the following sentence is removed from the
LastAttemptStatus parameter definition for
FmpDeviceCheckImageWithStatus() since it can lead to confusion.
The expected status code value range is sufficient to implement
the library API.
"This value will only be checked when this
function returns an error."
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 | 23 +++++++++++++++-----
FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c | 3 +--
FmpDevicePkg/Include/Library/FmpDeviceLib.h | 3 +--
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index 197df28c8dd6..1e7ec4a09e16 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -1040,8 +1040,19 @@ CheckTheImageInternal (
//
Status = FmpDeviceCheckImageWithStatus ((((UINT8 *)Image) + AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus);
if (EFI_ERROR (Status)) {
+ // The image cannot be valid if an error occurred checking the image
+ if (*ImageUpdatable == IMAGE_UPDATABLE_VALID) {
+ *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
+ }
+
DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - FmpDeviceLib CheckImage failed. Status = %r\n", mImageIdName, Status));
+ }
+ //
+ // Only validate the library last attempt status code if the image is not updatable.
+ // This specifically avoids converting LAST_ATTEMPT_STATUS_SUCCESS if it set for an updatable image.
+ //
+ if (*ImageUpdatable != IMAGE_UPDATABLE_VALID) {
//
// 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]
@@ -1049,12 +1060,12 @@ CheckTheImageInternal (
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)
- );
+ DEBUG ((
+ DEBUG_ERROR,
+ "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from FmpDeviceCheckImageWithStatus() is invalid.\n",
+ mImageIdName,
+ *LastAttemptStatus
+ ));
*LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
}
}
diff --git a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
index 2e5c17b2b0f9..82219e87a430 100644
--- a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
+++ b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
@@ -434,8 +434,7 @@ FmpDeviceCheckImage (
IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
@param[out] LastAttemptStatus A pointer to a UINT32 that holds the last attempt
status to report back to the ESRT table in case
- of error. This value will only be checked when this
- function returns an error.
+ of error.
The return status code must fall in the range of
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
diff --git a/FmpDevicePkg/Include/Library/FmpDeviceLib.h b/FmpDevicePkg/Include/Library/FmpDeviceLib.h
index a14406abe8b5..f82ef64503fa 100644
--- a/FmpDevicePkg/Include/Library/FmpDeviceLib.h
+++ b/FmpDevicePkg/Include/Library/FmpDeviceLib.h
@@ -421,8 +421,7 @@ FmpDeviceCheckImage (
IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
@param[out] LastAttemptStatus A pointer to a UINT32 that holds the last attempt
status to report back to the ESRT table in case
- of error. This value will only be checked when this
- function returns an error.
+ of error.
The return status code must fall in the range of
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update FmpDeviceCheckImageWithStatus() handling
2022-01-04 20:38 [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update FmpDeviceCheckImageWithStatus() handling Michael Kubacki
@ 2022-01-07 3:45 ` Guomin Jiang
2022-01-19 19:46 ` [edk2-devel] " Michael Kubacki
0 siblings, 1 reply; 4+ messages in thread
From: Guomin Jiang @ 2022-01-07 3:45 UTC (permalink / raw)
To: mikuback@linux.microsoft.com, devel@edk2.groups.io
Cc: Gao, Liming, Kinney, Michael D, Xu, Wei6
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
Guomin
> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Wednesday, January 5, 2022 4:38 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <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: [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update
> FmpDeviceCheckImageWithStatus() handling
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> Update the logic handling last attempt status codes from
> FmpDeviceCheckImageWithStatus() implementations to account for cases
> when the function return status code is EFI_SUCCESS (since the image was
> checked successfully) but the ImageUpdatable value is not valid.
>
> In addition the following sentence is removed from the LastAttemptStatus
> parameter definition for
> FmpDeviceCheckImageWithStatus() since it can lead to confusion.
> The expected status code value range is sufficient to implement the library
> API.
>
> "This value will only be checked when this
> function returns an error."
>
> 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 | 23 +++++++++++++++-----
> FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c | 3 +--
> FmpDevicePkg/Include/Library/FmpDeviceLib.h | 3 +--
> 3 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> b/FmpDevicePkg/FmpDxe/FmpDxe.c index 197df28c8dd6..1e7ec4a09e16
> 100644
> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> @@ -1040,8 +1040,19 @@ CheckTheImageInternal (
> //
> Status = FmpDeviceCheckImageWithStatus ((((UINT8 *)Image) +
> AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus);
> if (EFI_ERROR (Status)) {
> + // The image cannot be valid if an error occurred checking the image
> + if (*ImageUpdatable == IMAGE_UPDATABLE_VALID) {
> + *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
> + }
> +
> DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - FmpDeviceLib
> CheckImage failed. Status = %r\n", mImageIdName, Status));
> + }
>
> + //
> + // Only validate the library last attempt status code if the image is not
> updatable.
> + // This specifically avoids converting LAST_ATTEMPT_STATUS_SUCCESS if it
> set for an updatable image.
> + //
> + if (*ImageUpdatable != IMAGE_UPDATABLE_VALID) {
> //
> // 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]
> @@ -1049,12 +1060,12 @@ CheckTheImageInternal (
> 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)
> - );
> + DEBUG ((
> + DEBUG_ERROR,
> + "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from
> FmpDeviceCheckImageWithStatus() is invalid.\n",
> + mImageIdName,
> + *LastAttemptStatus
> + ));
> *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
> }
> }
> diff --git a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
> b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
> index 2e5c17b2b0f9..82219e87a430 100644
> --- a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
> +++ b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
> @@ -434,8 +434,7 @@ FmpDeviceCheckImage (
> IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
> @param[out] LastAttemptStatus A pointer to a UINT32 that holds the last
> attempt
> status to report back to the ESRT table in case
> - of error. This value will only be checked when this
> - function returns an error.
> + of error.
>
> The return status code must fall in the range of
>
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> diff --git a/FmpDevicePkg/Include/Library/FmpDeviceLib.h
> b/FmpDevicePkg/Include/Library/FmpDeviceLib.h
> index a14406abe8b5..f82ef64503fa 100644
> --- a/FmpDevicePkg/Include/Library/FmpDeviceLib.h
> +++ b/FmpDevicePkg/Include/Library/FmpDeviceLib.h
> @@ -421,8 +421,7 @@ FmpDeviceCheckImage (
> IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
> @param[out] LastAttemptStatus A pointer to a UINT32 that holds the last
> attempt
> status to report back to the ESRT table in case
> - of error. This value will only be checked when this
> - function returns an error.
> + of error.
>
> The return status code must fall in the range of
>
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> --
> 2.28.0.windows.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update FmpDeviceCheckImageWithStatus() handling
2022-01-07 3:45 ` Guomin Jiang
@ 2022-01-19 19:46 ` Michael Kubacki
2022-01-20 1:19 ` 回复: " gaoliming
0 siblings, 1 reply; 4+ messages in thread
From: Michael Kubacki @ 2022-01-19 19:46 UTC (permalink / raw)
To: devel, guomin.jiang; +Cc: Gao, Liming, Kinney, Michael D, Xu, Wei6
Hi Guomin,
It has been a couple of weeks since your review and I have not seen the
patch merged yet.
Do you know when it will be merged?
Thanks,
Michael
On 1/6/2022 10:45 PM, Guomin Jiang wrote:
> Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
>
> Guomin
>
>> -----Original Message-----
>> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
>> Sent: Wednesday, January 5, 2022 4:38 AM
>> To: devel@edk2.groups.io
>> Cc: Gao, Liming <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: [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update
>> FmpDeviceCheckImageWithStatus() handling
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> Update the logic handling last attempt status codes from
>> FmpDeviceCheckImageWithStatus() implementations to account for cases
>> when the function return status code is EFI_SUCCESS (since the image was
>> checked successfully) but the ImageUpdatable value is not valid.
>>
>> In addition the following sentence is removed from the LastAttemptStatus
>> parameter definition for
>> FmpDeviceCheckImageWithStatus() since it can lead to confusion.
>> The expected status code value range is sufficient to implement the library
>> API.
>>
>> "This value will only be checked when this
>> function returns an error."
>>
>> 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 | 23 +++++++++++++++-----
>> FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c | 3 +--
>> FmpDevicePkg/Include/Library/FmpDeviceLib.h | 3 +--
>> 3 files changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
>> b/FmpDevicePkg/FmpDxe/FmpDxe.c index 197df28c8dd6..1e7ec4a09e16
>> 100644
>> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
>> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
>> @@ -1040,8 +1040,19 @@ CheckTheImageInternal (
>> //
>> Status = FmpDeviceCheckImageWithStatus ((((UINT8 *)Image) +
>> AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus);
>> if (EFI_ERROR (Status)) {
>> + // The image cannot be valid if an error occurred checking the image
>> + if (*ImageUpdatable == IMAGE_UPDATABLE_VALID) {
>> + *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
>> + }
>> +
>> DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - FmpDeviceLib
>> CheckImage failed. Status = %r\n", mImageIdName, Status));
>> + }
>>
>> + //
>> + // Only validate the library last attempt status code if the image is not
>> updatable.
>> + // This specifically avoids converting LAST_ATTEMPT_STATUS_SUCCESS if it
>> set for an updatable image.
>> + //
>> + if (*ImageUpdatable != IMAGE_UPDATABLE_VALID) {
>> //
>> // 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]
>> @@ -1049,12 +1060,12 @@ CheckTheImageInternal (
>> 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)
>> - );
>> + DEBUG ((
>> + DEBUG_ERROR,
>> + "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from
>> FmpDeviceCheckImageWithStatus() is invalid.\n",
>> + mImageIdName,
>> + *LastAttemptStatus
>> + ));
>> *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
>> }
>> }
>> diff --git a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
>> b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
>> index 2e5c17b2b0f9..82219e87a430 100644
>> --- a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
>> +++ b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
>> @@ -434,8 +434,7 @@ FmpDeviceCheckImage (
>> IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
>> @param[out] LastAttemptStatus A pointer to a UINT32 that holds the last
>> attempt
>> status to report back to the ESRT table in case
>> - of error. This value will only be checked when this
>> - function returns an error.
>> + of error.
>>
>> The return status code must fall in the range of
>>
>> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
>> diff --git a/FmpDevicePkg/Include/Library/FmpDeviceLib.h
>> b/FmpDevicePkg/Include/Library/FmpDeviceLib.h
>> index a14406abe8b5..f82ef64503fa 100644
>> --- a/FmpDevicePkg/Include/Library/FmpDeviceLib.h
>> +++ b/FmpDevicePkg/Include/Library/FmpDeviceLib.h
>> @@ -421,8 +421,7 @@ FmpDeviceCheckImage (
>> IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
>> @param[out] LastAttemptStatus A pointer to a UINT32 that holds the last
>> attempt
>> status to report back to the ESRT table in case
>> - of error. This value will only be checked when this
>> - function returns an error.
>> + of error.
>>
>> The return status code must fall in the range of
>>
>> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
>> --
>> 2.28.0.windows.1
>
>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* 回复: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update FmpDeviceCheckImageWithStatus() handling
2022-01-19 19:46 ` [edk2-devel] " Michael Kubacki
@ 2022-01-20 1:19 ` gaoliming
0 siblings, 0 replies; 4+ messages in thread
From: gaoliming @ 2022-01-20 1:19 UTC (permalink / raw)
To: 'Michael Kubacki', devel, guomin.jiang
Cc: 'Kinney, Michael D', 'Xu, Wei6'
Create PR https://github.com/tianocore/edk2/pull/2427 for it.
Thanks
Liming
> -----邮件原件-----
> 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
> 发送时间: 2022年1月20日 3:46
> 收件人: devel@edk2.groups.io; guomin.jiang@intel.com
> 抄送: Gao, Liming <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Xu, Wei6 <wei6.xu@intel.com>
> 主题: Re: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update
> FmpDeviceCheckImageWithStatus() handling
>
> Hi Guomin,
>
> It has been a couple of weeks since your review and I have not seen the
> patch merged yet.
>
> Do you know when it will be merged?
>
> Thanks,
> Michael
>
> On 1/6/2022 10:45 PM, Guomin Jiang wrote:
> > Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
> >
> > Guomin
> >
> >> -----Original Message-----
> >> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> >> Sent: Wednesday, January 5, 2022 4:38 AM
> >> To: devel@edk2.groups.io
> >> Cc: Gao, Liming <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: [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update
> >> FmpDeviceCheckImageWithStatus() handling
> >>
> >> From: Michael Kubacki <michael.kubacki@microsoft.com>
> >>
> >> Update the logic handling last attempt status codes from
> >> FmpDeviceCheckImageWithStatus() implementations to account for cases
> >> when the function return status code is EFI_SUCCESS (since the image was
> >> checked successfully) but the ImageUpdatable value is not valid.
> >>
> >> In addition the following sentence is removed from the LastAttemptStatus
> >> parameter definition for
> >> FmpDeviceCheckImageWithStatus() since it can lead to confusion.
> >> The expected status code value range is sufficient to implement the library
> >> API.
> >>
> >> "This value will only be checked when this
> >> function returns an error."
> >>
> >> 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 | 23
> +++++++++++++++-----
> >> FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c | 3 +--
> >> FmpDevicePkg/Include/Library/FmpDeviceLib.h | 3 +--
> >> 3 files changed, 19 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> >> b/FmpDevicePkg/FmpDxe/FmpDxe.c index 197df28c8dd6..1e7ec4a09e16
> >> 100644
> >> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> >> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> >> @@ -1040,8 +1040,19 @@ CheckTheImageInternal (
> >> //
> >> Status = FmpDeviceCheckImageWithStatus ((((UINT8 *)Image) +
> >> AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus);
> >> if (EFI_ERROR (Status)) {
> >> + // The image cannot be valid if an error occurred checking the image
> >> + if (*ImageUpdatable == IMAGE_UPDATABLE_VALID) {
> >> + *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
> >> + }
> >> +
> >> DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() -
> FmpDeviceLib
> >> CheckImage failed. Status = %r\n", mImageIdName, Status));
> >> + }
> >>
> >> + //
> >> + // Only validate the library last attempt status code if the image is not
> >> updatable.
> >> + // This specifically avoids converting LAST_ATTEMPT_STATUS_SUCCESS
> if it
> >> set for an updatable image.
> >> + //
> >> + if (*ImageUpdatable != IMAGE_UPDATABLE_VALID) {
> >> //
> >> // 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]
> >> @@ -1049,12 +1060,12 @@ CheckTheImageInternal (
> >> 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)
> >> - );
> >> + DEBUG ((
> >> + DEBUG_ERROR,
> >> + "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from
> >> FmpDeviceCheckImageWithStatus() is invalid.\n",
> >> + mImageIdName,
> >> + *LastAttemptStatus
> >> + ));
> >> *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
> >> }
> >> }
> >> diff --git a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
> >> b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
> >> index 2e5c17b2b0f9..82219e87a430 100644
> >> --- a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
> >> +++ b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
> >> @@ -434,8 +434,7 @@ FmpDeviceCheckImage (
> >>
> IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
> >> @param[out] LastAttemptStatus A pointer to a UINT32 that holds
> the last
> >> attempt
> >> status to report back to the
> ESRT table in case
> >> - of error. This value will only be
> checked when this
> >> - function returns an error.
> >> + of error.
> >>
> >> The return status code must
> fall in the range of
> >>
> >> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> >> diff --git a/FmpDevicePkg/Include/Library/FmpDeviceLib.h
> >> b/FmpDevicePkg/Include/Library/FmpDeviceLib.h
> >> index a14406abe8b5..f82ef64503fa 100644
> >> --- a/FmpDevicePkg/Include/Library/FmpDeviceLib.h
> >> +++ b/FmpDevicePkg/Include/Library/FmpDeviceLib.h
> >> @@ -421,8 +421,7 @@ FmpDeviceCheckImage (
> >>
> IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
> >> @param[out] LastAttemptStatus A pointer to a UINT32 that holds
> the last
> >> attempt
> >> status to report back to the
> ESRT table in case
> >> - of error. This value will only be
> checked when this
> >> - function returns an error.
> >> + of error.
> >>
> >> The return status code must
> fall in the range of
> >>
> >> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> >> --
> >> 2.28.0.windows.1
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-20 1:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-04 20:38 [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update FmpDeviceCheckImageWithStatus() handling Michael Kubacki
2022-01-07 3:45 ` Guomin Jiang
2022-01-19 19:46 ` [edk2-devel] " Michael Kubacki
2022-01-20 1:19 ` 回复: " gaoliming
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox