回复: [edk2-devel] [PATCH v1 0/9] Add ImagePropertiesRecordLib and Fix MAT Bugs

[gaoliming via groups.io]

Ard:
  Now, there is no compatible way to introduce new library class for the basic module. One possible solution is to define the default library instance in package dec file together with the library class declaration. 

Beebe:
  This patch set looks good. But, the last patch 9/9 will require every platform to be updated. I may suggest to merge the first 8 patches for this stable tag 202308. The last one 9/9 can be merged for next stable tag 202311. Then, the platform owner have time to update their DSC first. 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel at edk2.groups.io <devel at edk2.groups.io> 代表 Ard
> Biesheuvel
> 发送时间: 2023年7月19日 4:18
> 收件人: Taylor Beebe <t at taylorbeebe.com>
> 抄送: devel at edk2.groups.io; Andrew Fish <afish at apple.com>; 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 v1 0/9] Add ImagePropertiesRecordLib and Fix
> MAT Bugs
> 
> On Tue, 18 Jul 2023 at 20:40, Taylor Beebe <t at taylorbeebe.com> wrote:
> >
> > 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.
> >
> 
> This all looks fine to me, but this is another series of the pattern
> - break out some functionality into a new lib class
> - provide a single implementation of that lib class
> - track down every DSC in existence and add the same library class
> resolution to each.
> 
> Could we *please* have a way for library classes to be declared with a
> default resolution? That way, series such as this one will be a lot
> leaner, and as a bonus, I am sure that there is a lot of boilerplate
> that can be removed from existing DSCs for library classes that only
> have a single implementation.
> 
> For this series,
> 
> Acked-by: Ard Biesheuvel <ardb at kernel.org>
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107036): https://edk2.groups.io/g/devel/message/107036
Mute This Topic: https://groups.io/mt/100230477/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-