On 04/07/21 04:43, Ni, Ray wrote: > On Tue, Apr 6, 2021 at 08:03 PM, Laszlo Ersek wrote: > >> >> (1) I think we should use a new TianoCore feature request BZ for this >> feature, and the commit messages should link it. (I understand the >> library only factors out existent logic, but still.) > > sure. https://bugzilla.tianocore.org/show_bug.cgi?id=3303 > >> >> (2) As I understand it, a platform can provide microcode in three ways: >> - via the "microcode patch" GUIDed HOB (PEI and DXE phases both), >> - via the "shadow microcode" PPI (PEI phase only), >> - via the PcdCpuMicrocodePatch* PCDs (PEI and DXE phases both). >> >> If a platform uses none of these methods (for example, OVMF does not), >> then I think it would benefit from a Null instance of the new >> MicrocodeLib class. >> >> Would you consider introducing a Null instance too, and using that one >> in the OVMF DSC files? >> > > No. I don't think it's necessary for a NULL instance. > Because: > 1. MicrocodePatch GUIDed HOB is only produced when "shadow microcode" PPI or > "PcdCpuMicrocodePatch *" exists. > I will further simplify today's MpInitLib to skip loading microcode when > this HOB exists (because DXE re-load is unnecessary). > This is captured by https://bugzilla.tianocore.org/show_bug.cgi?id=3155. > > 2. Today's logic only calls the MicrocodeLib API when "shadow microcode" PPI > or "PcdCpuMicrocodePatch *" exists. > Even NULL instance is provided, it's not called. > > 3. MicrocodeLib calls ASSERT() when the supplied microcode binary is NULL. > If the logic in MpInitLib is changed by accident to allow the call to > MicrocodeLib even in OVMF, the assertion can catch this.
I was thinking that a Null instance would be useful for eliminating dead code from the binary (because: the PPI check is dynamic, unlike a compile-time PCD check, so it can not be optimized out). But it's not critical. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73775): https://edk2.groups.io/g/devel/message/73775 Mute This Topic: https://groups.io/mt/81796730/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
