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.web08.4446.1603863424861097212 for ; Tue, 27 Oct 2020 22:37:06 -0700 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 ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Wed, 28 Oct 2020 13:36:54 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Kinney, Michael D'" , , Cc: "'Jiang, Guomin'" , "'Xu, Wei6'" , "'Liu, Zhiguang'" References: In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHY2IDAvNl0gRXh0ZW5kIExhc3QgQXR0ZW1wdCBTdGF0dXMgVXNhZ2U=?= Date: Wed, 28 Oct 2020 13:37:01 +0800 Message-ID: <002f01d6acec$5c6aa2c0$153fe840$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQICAXFYo6MYjqDbZKe36xhlOMydlAGBKSz2qUnMf6A= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Create PR https://github.com/tianocore/edk2/pull/1054 for this patch set.= =20 > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Kinney, Michael D > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2020=E5=B9=B410=E6=9C=8820=E6=97= =A5 8:16 > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; michael.kubacki@outlo= ok.com; Kinney, > Michael D > =E6=8A=84=E9=80=81: Liming Gao ; Jiang, Guomin > ; Xu, Wei6 ; Liu, Zhiguang > > =E4=B8=BB=E9=A2=98: RE: [edk2-devel] [PATCH v6 0/6] Extend Last Attempt = Status Usage >=20 > Series Reviewed-by: Michael D Kinney >=20 >=20 > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Michael > Kubacki > > Sent: Monday, October 19, 2020 5:00 PM > > To: devel@edk2.groups.io > > Cc: Liming Gao ; Kinney, Michael D > ; Jiang, Guomin > > ; Xu, Wei6 ; Liu, Zhiguang > > > Subject: [edk2-devel] [PATCH v6 0/6] Extend Last Attempt Status Usage > > > > From: Michael Kubacki > > > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2802 > > > > This patch series adds more granularity to Last Attempt Status > > codes reported during FMP check image and set image operations > > that greatly improve precision of the status codes. > > > > The unsuccessful vendor range (0x1000 - 0x4000) was introduced > > in UEFI Specification 2.8. At a high-level, two subranges are > > defined within that range in this patch series: > > 1. The FMP Reserved range - reserved for components implemented > > in FmpDevicePkg. > > 2. The FMP Device Library Reserved range - reserved for > > FmpDeviceLib instance-specific usage. > > > > The ranges are described in a public header file LastAttemptStatus.h > > while the specific codes used within FmpDevicePkg implementation > > are defined in a private header file FmpLastAttemptStatus.h. > > > > FmpDeviceLib instances should use the range definition from the > > public header file to define Last Attempt Status codes local to > > their library instance. > > > > Of note, there's multiple approaches to assigning private status > > codes in the FMP Reserved range. For example, individual components > > could define their last attempt status codes locally with the > > range allocated to the component defined in a package-wide private > > header file. However, one goal of the granularity being introduced > > is to provide straightforward traceability to an error source. > > > > For that reason, it was chosen to define a constant set of codes at > > the package level in FmpLastAttemptStatus.h. For example, if a new > > FmpDependencyLib instance is added, it would not be able to reassign > > status code values in the pre-existing FMP Dependency range; it > > would reuse codes for the same error source and be able to add new > > codes onto the range for its usage. > > > > V6 changes: > > 1. FmpDevicePkg/Library/FmpDeviceLibNull > > * Updated FmpDeviceCheckImage() and FmpDeviceSetImage() to call > > FmpDeviceCheckImageWithStatus() and > > FmpDeviceSetImageWithStatus() respectively. > > > > This is to clearly demonstrate to FmpDeviceLib implementors > > that only the *WithStatus() versions need to be implemented. > > > > 2. FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h > > * Updated the block comment above the > > FmpDependencyCheckLib definitions to use /// instead of // > > to be consistent with the rest of the file. > > > > V5 changes: > > 1. Fixed an issue where > > LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_CERTIFICATE is > changed > > to LAST_ATTEMPT_STATUS_DRIVER_ERROR_IMAGE_AUTH_FAILURE > in the > > logic to return the last attempt status code in > > CheckTheImageInternal(). > > > > V4 changes: > > 1. Simplified range value definitions in LastAttemptStatus.h. > > Directly assign the values in the macro definition instead > > of using calculations. > > 2. Adjusted range sizes to leave more room for future expansion. > > > > OLD: > > START | END | Usage > > ------------------------------------------------| > > 0x1000 | 0x1FFF | FmpDevicePkg | > > 0x1000 | 0x107F | FmpDxe driver | > > 0x1080 | 0x109F | FMP dependency Libs | > > 0x2000 | 0x3FFF | FmpDeviceLib instances | > > > > NEW: > > START | END | Usage > > ----------------------------------------------------------------| > > 0x1000 | 0x17FF | FmpDevicePkg > | > > 0x1000 | 0x107F | FmpDxe driver > | > > 0x1080 | 0x109F | FmpDependencyLib > | > > 0x10A0 | 0x10BF | FmpDependencyCheckLib > | > > 0x10C0 | 0x17FF | Unused. Available for future expansion. | > > 0x1800 | 0x1FFF | FmpDeviceLib instances implementation > | > > 0x2000 | 0x3FFF | Unused. Available for future expansion. | > > > > 3. Broke the single range in v3 for FMP Dependency libraries into > > separate ranges. > > 4. Clarified LastAttemptStatus return values in each function > > description. > > 5. Returned an expected LastAttemptStatus value for some functions > > that previously did not return a value. > > 6. Reverted changes in FmpDxe to call the new FmpDeviceLib APIs > > for FmpDeviceCheckImage () and FmpDeviceSetImage (). These will > > be added in a future series after impacted platforms in > > edk2-platforms are updated to use the new APIs. > > 7. Instead of directly changing the pre-existing APIs in > > FmpDeviceLib to add a LastAttemptStatus parameter, the new > > functions were added to the library interface: > > * FmpDeviceCheckImageWithStatus () > > * FmpDeviceSetImageWithStatus () > > > > V3 changes: > > 1. Enhanced range definitions in LastAttemptStatus.h with more > > completeness providing length, min, and max values. > > 2. Moved the actual Last Attempt Status code assignments to a > > private header file PrivateInclude/FmpLastAttemptStatus.h. > > 3. Changed the value of > > > LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX > > to 0x3FFF instead of 0x4000 even though 0x4000 is defined in > > the UEFI specification. The length is 0x4000 but the max > > allowed value should be 0x3FFF. This change was made now to > > prevent implementation compatibility issues in the future. > > 4. Included "DEVICE" in the following macro name to clearly > > associate it with the FmpDeviceLib library class: > > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_ERROR_xxx > > 5. Included a map to help the reader better visualize the range > > definitions in LastAttemptStatus.h. > > 6. Included additional documentation describing the enum in > > FmpLastAttemptStatus.h. An explicit statement stating that new > > codes should be added onto the end of ranges to preserve the > > values was added. > > 7. Simplified error handling logic in FmpDxe for FmpDeviceLib > > calls that return Last Attempt Status. > > 8. V2 had a single memory allocation failure code used for > > different memory allocations in CheckFmpDependency () in > > FmpDependencyLib. Each potential allocation failure was > > assigned a unique code. > > > > V2 changes: > > 1. Consolidate all previous incremental updates to > > LastAttemptStatus.h into one patch (patch 2) > > 2. Move LastAttemptStatus.h from Include to PrivateInclude > > 3. Correct patch 1 subject from "FmpDevicePkg" to "MdePkg" > > > > Cc: Liming Gao > > Cc: Michael D Kinney > > Cc: Guomin Jiang > > Cc: Wei6 Xu > > Cc: Zhiguang Liu > > Signed-off-by: Michael Kubacki > > > > Michael Kubacki (6): > > MdePkg/SystemResourceTable.h: Add vendor range values > > FmpDevicePkg: Add Last Attempt Status header files > > FmpDevicePkg/FmpDxe: Add check image path Last Attempt Status > > capability > > FmpDevicePkg/FmpDxe: Improve set image path Last Attempt Status > > granularity > > FmpDevicePkg: Add Last Attempt Status support to dependency libs > > FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to Check/Set API > > > > FmpDevicePkg/FmpDxe/FmpDxe.c > | 146 +++++++++++++++++--- > > > FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib. > c | 39 ++++-- > > > FmpDevicePkg/Library/FmpDependencyCheckLibNull/FmpDependencyCheck > LibNull.c | 14 +- > > FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c > | 93 +++++++++++-- > > FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c > | 144 ++++++++++++++++++- > > > FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependen > cyUnitTest.c | 7 +- > > FmpDevicePkg/FmpDxe/FmpDxe.h > | 4 +- > > FmpDevicePkg/Include/LastAttemptStatus.h > | 81 +++++++++++ > > FmpDevicePkg/Include/Library/FmpDependencyCheckLib.h > | 8 +- > > FmpDevicePkg/Include/Library/FmpDependencyLib.h > | 44 ++++-- > > FmpDevicePkg/Include/Library/FmpDeviceLib.h > | 121 +++++++++++++++- > > FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h > | 81 +++++++++++ > > MdePkg/Include/Guid/SystemResourceTable.h > | 13 ++ > > 13 files changed, 730 insertions(+), 65 deletions(-) > > create mode 100644 FmpDevicePkg/Include/LastAttemptStatus.h > > create mode 100644 > FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h > > > > -- > > 2.28.0.windows.1 > > > > > > > >=20 > >