回复: [edk2-devel] [PATCH v4 00/14] Add ImagePropertiesRecordLib and Fix MAT Bugs
gaoliming via groups.io
gaoliming=byosoft.com.cn at groups.io
Sat Oct 7 05:57:32 UTC 2023
Taylor:
I agree to add new ImagePropertiesRecordLib library for DxeCore and SmmCore. The impact is that platform needs to update their DSC with new library.
Frankly, I have not understood MAT code in detail. So, I have no comments on this part.
Last, what test have been done to verify the current functionality?
Thanks
Liming
> -----邮件原件-----
> 发件人: devel at edk2.groups.io <devel at edk2.groups.io> 代表 Taylor Beebe
> 发送时间: 2023年10月4日 3:05
> 收件人: Ard Biesheuvel <ardb at kernel.org>
> 抄送: devel at edk2.groups.io; Andrew Fish <afish at apple.com>; Ard
> Biesheuvel <ardb+tianocore at kernel.org>; Dandan Bi <dandan.bi at intel.com>;
> Eric Dong <eric.dong at intel.com>; Gerd Hoffmann <kraxel at redhat.com>; Guo
> Dong <guo.dong at intel.com>; Gua Guo <gua.guo at intel.com>; James Lu
> <james.lu at intel.com>; Jian J Wang <jian.j.wang at intel.com>; Jiewen Yao
> <jiewen.yao at intel.com>; Jordan Justen <jordan.l.justen at intel.com>; Leif
> Lindholm <quic_llindhol at quicinc.com>; Liming Gao
> <gaoliming at byosoft.com.cn>; Rahul Kumar <rahul1.kumar at intel.com>; Ray
> Ni <ray.ni at intel.com>; Sami Mujawar <sami.mujawar at arm.com>; Sean
> Rhodes <sean at starlabs.systems>
> 主题: Re: [edk2-devel] [PATCH v4 00/14] Add ImagePropertiesRecordLib and
> Fix MAT Bugs
>
> Thank you , Ard. I very much appreciate your responsiveness.
>
> The majority of these patches fall under MdeModulePkg maintainers so
> I'll also need help from them to drive this forward.
>
> On 10/3/2023 11:57 AM, Ard Biesheuvel wrote:
> > On Tue, 3 Oct 2023 at 17:56, Taylor Beebe <taylor.d.beebe at gmail.com>
> wrote:
> >> Have we given up on this patch series and Bug 4492?
> >>
> > I haven't, I promise. I will get to this on Thursday or Friday.
> >
> >
> >> On 9/26/2023 9:02 AM, Taylor Beebe via groups.io wrote:
> >>> Reviews on this patch series would be much appreciated :)
> >>>
> >>> On 8/28/23 9:38 AM, Ard Biesheuvel wrote:
> >>>> I was hoping to get around to this before the end of the week (but I
> >>>> am not a MdeModulePkg maintainer)
> >>>>
> >>>>
> >>>> On Mon, 28 Aug 2023 at 18:27, Taylor Beebe
> <taylor.d.beebe at gmail.com>
> >>>> wrote:
> >>>>> Can I please get reviews/feedback on this patch series?
> >>>>>
> >>>>> On 8/16/23 11:14 AM, Taylor Beebe via groups.io wrote:
> >>>>>> Can I please get reviews/feedback on this patch series?
> >>>>>>
> >>>>>> On 8/4/2023 12:46 PM, Taylor Beebe via groups.io wrote:
> >>>>>>> From: Taylor Beebe <taylor.d.beebe at gmail.com>
> >>>>>>>
> >>>>>>> v4:
> >>>>>>> - Expose additional functions in the Library API
> >>>>>>> - Add NULL checks to library functions and return a
> >>>>>>> status where applicable.
> >>>>>>>
> >>>>>>> v3:
> >>>>>>> - Refactor patch series so the transition of logic from the DXE
> >>>>>>> MAT logic to the new library is more clear.
> >>>>>>> - Update function headers to improve clarity and follow EDK2
> >>>>>>> standards.
> >>>>>>> - Add Create and Delete functions for Image Properties Records
> >>>>>>> and redirect some of the SMM and DXE MAT code to use
> these
> >>>>>>> functions.
> >>>>>>> - Update/Add DumpImageRecords() to print the image name and
> code
> >>>>>>> sections of each runtime image which will be put in the MAT.
> >>>>>>> The DXE and SMM MAT logic will now invoke the
> DumpImageRecords()
> >>>>>>> on DEBUG builds at the EndOfDxe event to install the MAT.
> >>>>>>>
> >>>>>>> v2:
> >>>>>>> - A one-line change in patch 3 was moved to patch 9 for correctness.
> >>>>>>>
> >>>>>>> Reference: https://github.com/tianocore/edk2/pull/4590
> >>>>>>> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=4492
> >>>>>>>
> >>>>>>> The UEFI and SMM MAT logic contains duplicate logic for
> manipulating
> >>>>>>> image
> >>>>>>> properties records which is used to track runtime images.
> >>>>>>> This patch series adds a new library, ImagePropertiesRecordLib,
> >>>>>>> which consolidates this logic and fixes the bugs which currently
> >>>>>>> exist in
> >>>>>>> the MAT logic.
> >>>>>>>
> >>>>>>> The first patch adds the ImagePropertiesRecordLib implementation
> >>>>>>> which
> >>>>>>> is a copy of the UEFI MAT logic with minor modifications to remove
> >>>>>>> the
> >>>>>>> reliance on globabl variables and make the code unit testable.
> >>>>>>>
> >>>>>>> The second patch adds a unit test for the
> >>>>>>> ImagePropertiesRecordLib. The
> >>>>>>> logic tests various potential layouts of the EFI memory map and
> >>>>>>> runtime
> >>>>>>> images. 3/4 of these tests will fail which demonstrates the MAT logic
> >>>>>>> bugs.
> >>>>>>>
> >>>>>>> The third patch fixes the logic in the ImagePropertiesRecordLib so
> >>>>>>> that all of the unit tests pass and the MAT logic can be fixed by
> >>>>>>> using the library.
> >>>>>>>
> >>>>>>> The remaining patches add library instances to DSC files and remove
> >>>>>>> the image properties record logic from the SMM and UEFI MAT logic.
> >>>>>>>
> >>>>>>> Cc: Andrew Fish <afish at apple.com>
> >>>>>>> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> >>>>>>> Cc: Dandan Bi <dandan.bi at intel.com>
> >>>>>>> Cc: Eric Dong <eric.dong at intel.com>
> >>>>>>> Cc: Gerd Hoffmann <kraxel at redhat.com>
> >>>>>>> Cc: Guo Dong <guo.dong at intel.com>
> >>>>>>> Cc: Gua Guo <gua.guo at intel.com>
> >>>>>>> Cc: James Lu <james.lu at intel.com>
> >>>>>>> Cc: Jian J Wang <jian.j.wang at intel.com>
> >>>>>>> Cc: Jiewen Yao <jiewen.yao at intel.com>
> >>>>>>> Cc: Jordan Justen <jordan.l.justen at intel.com>
> >>>>>>> Cc: Leif Lindholm <quic_llindhol at quicinc.com>
> >>>>>>> Cc: Liming Gao <gaoliming at byosoft.com.cn>
> >>>>>>> Cc: Rahul Kumar <rahul1.kumar at intel.com>
> >>>>>>> Cc: Ray Ni <ray.ni at intel.com>
> >>>>>>> Cc: Sami Mujawar <sami.mujawar at arm.com>
> >>>>>>> Cc: Sean Rhodes <sean at starlabs.systems>
> >>>>>>>
> >>>>>>> Taylor Beebe (14):
> >>>>>>> MdeModulePkg: Add ImagePropertiesRecordLib
> >>>>>>> ArmVirtPkg: Add ImagePropertiesRecordLib Instance
> >>>>>>> EmulatorPkg: Add ImagePropertiesRecordLib Instance
> >>>>>>> OvmfPkg: Add ImagePropertiesRecordLib Instance
> >>>>>>> UefiPayloadPkg: Add ImagePropertiesRecordLib Instance
> >>>>>>> MdeModulePkg: Update MemoryAttributesTable.c to Reduce
> Global
> >>>>>>> Variable
> >>>>>>> Use
> >>>>>>> MdeModulePkg: Move Some DXE MAT Logic to
> ImagePropertiesRecordLib
> >>>>>>> MdeModulePkg: Add ImagePropertiesRecordLib Host-Based
> Unit Test
> >>>>>>> MdeModulePkg: Fix Bugs in MAT Logic
> >>>>>>> MdeModulePkg: Add NULL checks and Return Status to
> >>>>>>> ImagePropertiesRecordLib
> >>>>>>> UefiCpuPkg: Use Attribute From SMM
> MemoryAttributesTable if
> >>>>>>> Nonzero
> >>>>>>> MdeModulePkg: Transition SMM MAT Logic to Use
> >>>>>>> ImagePropertiesRecordLib
> >>>>>>> MdeModulePkg: Add Logic to Create/Delete Image Properties
> Records
> >>>>>>> MdeModulePkg: Update DumpImageRecord() in
> >>>>>>> ImagePropertiesRecordLib
> >>>>>>>
> >>>>>>> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 967
> >>>>>>> +----------------
> >>>>>>> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 24 +-
> >>>>>>> MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 958
> >>>>>>> +---------------
> >>>>>>>
> MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLi
> b.c
> >>>>>>>
> >>>>>>> | 1144 ++++++++++++++++++++
> >>>>>>>
> MdeModulePkg/Library/ImagePropertiesRecordLib/UnitTest/ImageProperties
> RecordLibUnitTestHost.c
> >>>>>>>
> >>>>>>> | 938 ++++++++++++++++
> >>>>>>> UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |
> 19 +-
> >>>>>>> ArmVirtPkg/ArmVirt.dsc.inc | 1 +
> >>>>>>> EmulatorPkg/EmulatorPkg.dsc | 1 +
> >>>>>>> MdeModulePkg/Core/Dxe/DxeMain.h | 20 -
> >>>>>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> >>>>>>> MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 1 +
> >>>>>>> MdeModulePkg/Include/Library/ImagePropertiesRecordLib.h | 234
> ++++
> >>>>>>>
> MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLi
> b.inf
> >>>>>>>
> >>>>>>> | 31 +
> >>>>>>>
> MdeModulePkg/Library/ImagePropertiesRecordLib/UnitTest/ImageProperties
> RecordLibUnitTestHost.inf
> >>>>>>>
> >>>>>>> | 35 +
> >>>>>>> MdeModulePkg/MdeModulePkg.dec | 5 +
> >>>>>>> MdeModulePkg/MdeModulePkg.dsc | 2 +
> >>>>>>> MdeModulePkg/Test/MdeModulePkgHostTest.dsc | 6 +
> >>>>>>> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> >>>>>>> OvmfPkg/Bhyve/BhyveX64.dsc | 1 +
> >>>>>>> OvmfPkg/CloudHv/CloudHvX64.dsc | 1 +
> >>>>>>> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
> >>>>>>> OvmfPkg/Microvm/MicrovmX64.dsc | 1 +
> >>>>>>> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> >>>>>>> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> >>>>>>> OvmfPkg/OvmfPkgX64.dsc | 1 +
> >>>>>>> OvmfPkg/OvmfXen.dsc | 1 +
> >>>>>>> OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc | 1 +
> >>>>>>> UefiPayloadPkg/UefiPayloadPkg.dsc | 1 +
> >>>>>>> 28 files changed, 2524 insertions(+), 1874 deletions(-)
> >>>>>>> create mode 100644
> >>>>>>>
> MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLi
> b.c
> >>>>>>>
> >>>>>>> create mode 100644
> >>>>>>>
> MdeModulePkg/Library/ImagePropertiesRecordLib/UnitTest/ImageProperties
> RecordLibUnitTestHost.c
> >>>>>>>
> >>>>>>> create mode 100644
> >>>>>>> MdeModulePkg/Include/Library/ImagePropertiesRecordLib.h
> >>>>>>> create mode 100644
> >>>>>>>
> MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLi
> b.inf
> >>>>>>>
> >>>>>>> create mode 100644
> >>>>>>>
> MdeModulePkg/Library/ImagePropertiesRecordLib/UnitTest/ImageProperties
> RecordLibUnitTestHost.inf
> >>>>>>>
> >>>>>>>
> >>>
> >>>
> >>>
> >>>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109381): https://edk2.groups.io/g/devel/message/109381
Mute This Topic: https://groups.io/mt/101812772/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list