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.ji...@intel.com>
Guomin
-----Original Message-----
From: mikub...@linux.microsoft.com <mikub...@linux.microsoft.com>
Sent: Wednesday, January 5, 2022 4:38 AM
To: devel@edk2.groups.io
Cc: Gao, Liming <gaolim...@byosoft.com.cn>; Kinney, Michael D
<michael.d.kin...@intel.com>; Jiang, Guomin <guomin.ji...@intel.com>;
Xu, Wei6 <wei6...@intel.com>
Subject: [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update
FmpDeviceCheckImageWithStatus() handling
From: Michael Kubacki <michael.kuba...@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 <gaolim...@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Guomin Jiang <guomin.ji...@intel.com>
Cc: Wei6 Xu <wei6...@intel.com>
Signed-off-by: Michael Kubacki <michael.kuba...@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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85830): https://edk2.groups.io/g/devel/message/85830
Mute This Topic: https://groups.io/mt/88199686/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
