Create PR https://github.com/tianocore/edk2/pull/2427 for it.
Thanks
Liming
> -----邮件原件-----
> 发件人: Michael Kubacki <mikub...@linux.microsoft.com>
> 发送时间: 2022年1月20日 3:46
> 收件人: devel@edk2.groups.io; guomin.ji...@intel.com
> 抄送: Gao, Liming <gaolim...@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kin...@intel.com>; Xu, Wei6 <wei6...@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.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 (#85858): https://edk2.groups.io/g/devel/message/85858
Mute This Topic: https://groups.io/mt/88549864/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
