From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web09.5813.1642641571995417129 for ; Wed, 19 Jan 2022 17:19:33 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([101.224.116.119]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Thu, 20 Jan 2022 09:19:26 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 101.224.116.119 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Michael Kubacki'" , , Cc: "'Kinney, Michael D'" , "'Xu, Wei6'" References: <20220104203824.2047-1-mikuback@linux.microsoft.com> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHYxIDEvMV0gRm1wRGV2aWNlUGtnL0ZtcER4ZTogVXBkYXRlIEZtcERldmljZUNoZWNrSW1hZ2VXaXRoU3RhdHVzKCkgaGFuZGxpbmc=?= Date: Thu, 20 Jan 2022 09:19:29 +0800 Message-ID: <022301d80d9b$c6009ad0$5201d070$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQI69COTWIZJcXCgfjnXKjr2qz76bQIvTSRvAXx+Jxqrh/DaAA== Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Create PR https://github.com/tianocore/edk2/pull/2427 for it.=20 Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Michael Kubacki > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2022=E5=B9=B41=E6=9C=8820=E6=97=A5 = 3:46 > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; guomin.jiang@intel.com > =E6=8A=84=E9=80=81: Gao, Liming ; Kinney, Micha= el D > ; Xu, Wei6 > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH v1 1/1] FmpDevicePkg/FmpDxe: = Update > FmpDeviceCheckImageWithStatus() handling >=20 > Hi Guomin, >=20 > It has been a couple of weeks since your review and I have not seen the > patch merged yet. >=20 > Do you know when it will be merged? >=20 > Thanks, > Michael >=20 > On 1/6/2022 10:45 PM, Guomin Jiang wrote: > > Reviewed-by: Guomin Jiang > > > > Guomin > > > >> -----Original Message----- > >> From: mikuback@linux.microsoft.com > >> Sent: Wednesday, January 5, 2022 4:38 AM > >> To: devel@edk2.groups.io > >> Cc: Gao, Liming ; Kinney, Michael D > >> ; Jiang, Guomin > ; > >> Xu, Wei6 > >> Subject: [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update > >> FmpDeviceCheckImageWithStatus() handling > >> > >> From: Michael Kubacki > >> > >> 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 w= as > >> checked successfully) but the ImageUpdatable value is not valid. > >> > >> In addition the following sentence is removed from the LastAttemptStat= us > >> parameter definition for > >> FmpDeviceCheckImageWithStatus() since it can lead to confusion. > >> The expected status code value range is sufficient to implement the li= brary > >> API. > >> > >> "This value will only be checked when this > >> function returns an error." > >> > >> Cc: Liming Gao > >> Cc: Michael D Kinney > >> Cc: Guomin Jiang > >> Cc: Wei6 Xu > >> Signed-off-by: Michael Kubacki > >> --- > >> 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 =3D FmpDeviceCheckImageWithStatus ((((UINT8 *)Image) + > >> AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus); > >> if (EFI_ERROR (Status)) { > >> + // The image cannot be valid if an error occurred checking the im= age > >> + if (*ImageUpdatable =3D=3D IMAGE_UPDATABLE_VALID) { > >> + *ImageUpdatable =3D IMAGE_UPDATABLE_INVALID; > >> + } > >> + > >> DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - > FmpDeviceLib > >> CheckImage failed. Status =3D %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 !=3D IMAGE_UPDATABLE_VALID) { > >> // > >> // LastAttemptStatus returned from the device library should fal= l > 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 =3D > 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 > > > > > > > >=20 > >