回复: [edk2-devel] [PATCH v6 0/6] Extend Last Attempt Status Usage

["gaoliming" <gaoliming@byosoft.com.cn>]

Create PR https://github.com/tianocore/edk2/pull/1054 for this patch set. 

> -----邮件原件-----
> 发件人: Kinney, Michael D <michael.d.kinney@intel.com>
> 发送时间: 2020年10月20日 8:16
> 收件人: devel@edk2.groups.io; michael.kubacki@outlook.com; Kinney,
> Michael D <michael.d.kinney@intel.com>
> 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Jiang, Guomin
> <guomin.jiang@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> 主题: RE: [edk2-devel] [PATCH v6 0/6] Extend Last Attempt Status Usage
> 
> Series Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <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 <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Jiang, Guomin
> > <guomin.jiang@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> > Subject: [edk2-devel] [PATCH v6 0/6] Extend Last Attempt Status Usage
> >
> > From: Michael Kubacki <michael.kubacki@microsoft.com>
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2802
> >
> > 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 <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>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> >
> > 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
> >
> >
> >
> > 
> >