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

["Laszlo Ersek" <lersek@redhat.com>]

Hi Ray,

On 04/02/21 07:58, Ni, Ray wrote:
> The patch set creates a new MicrocodeLib for loading microcode.
> Then updates all platforms to include this lib in DSC.
> Then updates the MpInitLib to consume this lib.
> 
> Edk2-platforms change will be sent out in a separate patch set.
> 
> 
> Ray Ni (4):
>   UefiCpuPkg: Add MicrocodeLib for loading microcode
>   OvmfPkg: Add MicrocodeLib in DSC files.
>   UefiPayloadPkg/UefiPayloadPkg.dsc: Consume MicrocodeLib
>   UefiCpuPkg/MpInitLib: Consume MicrocodeLib to remove duplicated code
> 
>  OvmfPkg/AmdSev/AmdSevX64.dsc                  |   1 +
>  OvmfPkg/Bhyve/BhyveX64.dsc                    |   1 +
>  OvmfPkg/OvmfPkgIa32.dsc                       |   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |   1 +
>  OvmfPkg/OvmfPkgX64.dsc                        |   1 +
>  OvmfPkg/OvmfXen.dsc                           |   1 +
>  UefiCpuPkg/Include/Library/MicrocodeLib.h     | 120 +++++
>  .../Library/MicrocodeLib/MicrocodeLib.c       | 322 ++++++++++++
>  .../Library/MicrocodeLib/MicrocodeLib.inf     |  32 ++
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   1 +
>  UefiCpuPkg/Library/MpInitLib/Microcode.c      | 484 ++++--------------
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |   1 +
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   1 +
>  UefiCpuPkg/UefiCpuPkg.dec                     |   5 +-
>  UefiCpuPkg/UefiCpuPkg.dsc                     |   1 +
>  UefiPayloadPkg/UefiPayloadPkg.dsc             |   1 +
>  16 files changed, 582 insertions(+), 392 deletions(-)
>  create mode 100644 UefiCpuPkg/Include/Library/MicrocodeLib.h
>  create mode 100644 UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.c
>  create mode 100644 UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
> 

(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.)

(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?

Thanks,
Laszlo