Re: [PATCH v4 0/6] Extend Last Attempt Status Usage

["Michael Kubacki" <michael.kubacki@outlook.com>]

It is going on a week now and I haven't seen a response to this patch 
series yet. Please review it when possible.

On a somewhat related note, I made the changes that should be necessary 
in edk2-platforms for backward compatibility in this patch:
https://edk2.groups.io/g/devel/message/65821

Thanks,
Michael

On 9/25/2020 7:19 PM, michael.kubacki@outlook.com wrote:
> 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.
> 
> 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/FmpDependencyCheckLibNull.c       |  14 +-
>   FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c                         |  93 +++++++++++--
>   FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c                             | 132 +++++++++++++++++-
>   FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.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, 718 insertions(+), 65 deletions(-)
>   create mode 100644 FmpDevicePkg/Include/LastAttemptStatus.h
>   create mode 100644 FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h
>