From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (NAM11-DM6-obe.outbound.protection.outlook.com [40.92.19.55]) by mx.groups.io with SMTP id smtpd.web11.1098.1602889156116252674 for ; Fri, 16 Oct 2020 15:59:16 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@outlook.com header.s=selector1 header.b=Rvk2uorM; spf=pass (domain: outlook.com, ip: 40.92.19.55, mailfrom: michael.kubacki@outlook.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=X6OlrTFRtoyHwyRBA8b1Hu0+66wCeWnnh3swD24+BbhPk1IJWRFCbpW3s8PsayS4P7Jl0uIv+LRp80df9/45dOdhz8PnL/2xKKEGD8IXdESqt2moiuTvB0XNsJ7dqYexYeHPHpeP21WNAE7e7kCVgAYGhVUqWGpS8L/Z926aWO1UG4UVla4fP2a0CGkmux8fLloKlShcM9c+GlT3qKwDPQdDGvr2nbCG555t1bzpGLJMyEfrnLgdJdkQsA/q/3XzP2nWeJTaS22V06TgfJTMylF0G6VASwhrWShGFDpoz3cpZ/xE7I9OxbWvhn42B0Rdt6l686L6UehBhk1JMJXLsg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=niY0StOqILDPky4qQcCCsyIn8nA6h8nlQk5vqXnSZhg=; b=ATIMYBKw3yOvUkPkZEAa66Y6dKPKOheRy/GjskO8re9JTKRGPmGEZ2JpQwhiomoGHPfdY77oJ5nPssoCk5zef9mYNIWHORWxdBrPRTguk/qLSA+bqtifOJQO0u9GfWFl+7W8sVrkW2KWYGoYdGWT1zYGO7csTY3pn2tqz1dJetVzabvIBBUWXHD8y2kWij/jdjmVUheC5olRVTQFT9MGD0dayu6xHqqkpfFyoP4aoGIc3OS4ClOBpSAiUo1eL3bRrHwl40WDzYzUIAttNoe4uNQHcMd/8j+NvD4aNrynoECiyQDMSywJy9xr5rRA6uGvNIgfZuZ1su05QNfzz6OCxw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=niY0StOqILDPky4qQcCCsyIn8nA6h8nlQk5vqXnSZhg=; b=Rvk2uorMOJ05Ch22n5biKeHyK4gr1noouiQsGhQ+ZV20nOD1qsD6SVDM/imjv80bU1Nc9wLEWL8qdmzo5QqB2kidafFkkF/G9RplQ9Xczgb82NTDGR4Wal3nGPbgwml9QeQW1G+KbZ8joJGqsLgzW/JyHz2yEIbyV9PyYQZVEE4oBou8gSVxHEa7nADsUrwCqShYpckcze3GwbTGEVN85lCwTZ5CXQoIBhpuPujUED6y/8Y6EHtcs5SR7vpzh5Bq4pxu1ewRXFenIAADWYSD4atG7ahZ2sGrgo7BrLsUE0zI8I0a7nyIKcHqkE7juutzTFrE/rft5tlCztmfYjoI0g== Received: from DM6NAM11FT032.eop-nam11.prod.protection.outlook.com (2a01:111:e400:fc4d::48) by DM6NAM11HT177.eop-nam11.prod.protection.outlook.com (2a01:111:e400:fc4d::120) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.23; Fri, 16 Oct 2020 22:59:15 +0000 Received: from MWHPR07MB3440.namprd07.prod.outlook.com (2a01:111:e400:fc4d::4a) by DM6NAM11FT032.mail.protection.outlook.com (2a01:111:e400:fc4d::349) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.21 via Frontend Transport; Fri, 16 Oct 2020 22:59:15 +0000 X-IncomingTopHeaderMarker: OriginalChecksum:0D6290ECE7E8950C2962C1433F2F930B57641B2FBBAADBB46B468A6E7884DFC0;UpperCasedChecksum:B61165FF12E159F61BCD186480368664E50B3FBA06E5D537F3128B0178216D7D;SizeAsReceived:9113;Count:48 Received: from MWHPR07MB3440.namprd07.prod.outlook.com ([fe80::858f:bd50:1b65:e803]) by MWHPR07MB3440.namprd07.prod.outlook.com ([fe80::858f:bd50:1b65:e803%7]) with mapi id 15.20.3455.031; Fri, 16 Oct 2020 22:59:15 +0000 Subject: Re: [edk2-devel] [PATCH v5 0/6] Extend Last Attempt Status Usage To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: Liming Gao , "Jiang, Guomin" , "Xu, Wei6" , "Liu, Zhiguang" References: From: "Michael Kubacki" Message-ID: Date: Fri, 16 Oct 2020 15:59:14 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.1 In-Reply-To: X-TMN: [AiKHNwFry0OHaxCy0BvEwUkhH6mQJzzn6oUKEx40I0dQsISqI0f6YimUvVdAJR+R] X-ClientProxiedBy: MWHPR14CA0037.namprd14.prod.outlook.com (2603:10b6:300:12b::23) To MWHPR07MB3440.namprd07.prod.outlook.com (2603:10b6:301:69::28) Return-Path: michael.kubacki@outlook.com X-Microsoft-Original-Message-ID: <396334df-2a32-f4ac-6682-072b929c4cb8@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [IPv6:2001:4898:d8:39:ed38:9a1b:5c35:df84] (2001:4898:80e8:f:6d52:9a1b:5c35:df84) by MWHPR14CA0037.namprd14.prod.outlook.com (2603:10b6:300:12b::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.20 via Frontend Transport; Fri, 16 Oct 2020 22:59:14 +0000 X-MS-PublicTrafficType: Email X-IncomingHeaderCount: 48 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-Correlation-Id: cc15c61c-039a-4dc1-0f25-08d872271a8f X-MS-TrafficTypeDiagnostic: DM6NAM11HT177: X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: hXhrG/2OaTkMvSdHtdvtpFPPRoLneBPMYFpS3PjMOVGh6foONGJD6vPc5f3hxR8p/rHdxezNCVGSpOwOUpCPK6sHZZbguMpypgiCqhP6v0sPMAqiFKd9DkwSH7eAyLOcQAHHdWFxEkyytfVCC8lQhs0PG4Dkkc+pVEy8T4RL6L5d5Sj+ORneWx7KOEmW2OOWXNYql53oKeCwdM+cHrtFmuvt694ASPICUi1B9KkYvD8Wg1mVXt6RcPOAQyDnjtMi X-MS-Exchange-AntiSpam-MessageData: 7K1JAYAtk5ljPSNvxdBl1UexNb3tjj/dglMFe7aTj2RRNRUJHrpYAqzVSk4E3hvZR5WAS7qBAFbVwgMhp3s4SJf9TyEV9x746QoxKEad+SXFyS1niubdsG7D6BvrFustH7clGPurJV+Y0idVpa+42c43BMtfGJyGKHh8e8HZKZLEPIVUGgu4otz9ChmYjxOuCUP+lPqHqWfbVwglhmjL8A== X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: cc15c61c-039a-4dc1-0f25-08d872271a8f X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Oct 2020 22:59:14.9968 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-AuthSource: DM6NAM11FT032.eop-nam11.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6NAM11HT177 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Mike, These changes will be made on top of v5. Thanks, Michael On 10/16/2020 3:57 PM, Michael D Kinney wrote: > Hi Michael, > > Thank you for the updates. This feedback was meant for V5, and not V4. > > A couple minor comments: > > 1) FmpDevicePkg/Library/FmpDeviceLibNull > > In order to demonstrate the preferred implementation I think we should > have FmpDeviceCheckImage() call FmpDeviceCheckImageWithStatus() and > FmpDeviceSetImage() call FmpDeviceSetImageWithStatus(). This way, it > will be clear to FmpDeviceLib developers that they only need to > implement the *WithStatus() version. > > 2) FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h > > FmpDependencyCheckLib comment block should use /// instead of // to be > consistent with the rest of the include file. > > Thanks, > > Mike > >> -----Original Message----- >> From: michael.kubacki@outlook.com >> Sent: Thursday, October 15, 2020 2:11 PM >> To: devel@edk2.groups.io >> Cc: Liming Gao ; Kinney, Michael D ; Jiang, Guomin ; >> Xu, Wei6 ; Liu, Zhiguang >> Subject: [PATCH v5 0/6] Extend Last Attempt Status Usage >> >> From: Michael Kubacki >> >> 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. >> >> 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/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 >> >> -- >> 2.28.0.windows.1 > > > > > >