Re: [edk2-devel] [PATCH 0/4] UefiCpuPkg: Add MicrocodeLib for loading microcode

["Ni, Ray" <ray.ni@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]

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.

[-- Attachment #2: Type: text/html, Size: 1956 bytes --]