Re: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add PCD for shadowing all microcode.

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

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Fu, Siyuan <siyuan.fu@intel.com>
> Sent: Tuesday, October 13, 2020 10:06 AM
> To: Ni, Ray <ray.ni@intel.com>; Li, Aaron <aaron.li@intel.com>; devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
> Subject: RE: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add PCD for shadowing all microcode.
> 
> > -----Original Message-----
> > From: Ni, Ray <ray.ni@intel.com>
> > Sent: 2020年9月24日 16:56
> > To: Li, Aaron <aaron.li@intel.com>; devel@edk2.groups.io
> > Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Fu, Siyuan
> > <siyuan.fu@intel.com>
> > Subject: RE: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add PCD for
> > shadowing all microcode.
> >
> > Aaron,
> > I understand the requirement now.
> > Can we avoid adding new PCD but re-interpret the ShadowMicrocode()
> > parameter to achieve the same result?
> >
> > For example, we can say when CpuIdCount is 0 and MicrocodeCpuId == NULL, it
> > means all microcode need to be shadowed.
> >
> > The benefit is: platform can use the single interface to control the behavior.
> 
> Hi, Ray
> 
> It's the platform PEIM (the ShadowMicrocode() function itself) to decide which
> Microcode it should shadow to memory, not the caller. So we can't use the input
> Parameter to control the behavior.
> 
> Thanks.
> Siyuan
> 
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: Li, Aaron <aaron.li@intel.com>
> > > Sent: Thursday, September 24, 2020 9:38 AM
> > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > > Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Fu, Siyuan
> > > <siyuan.fu@intel.com>
> > > Subject: RE: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add PCD for
> > > shadowing all microcode.
> > >
> > > Hi Ray,
> > >
> > > Accroding to
> > >
> > https://edk2.groups.io/g/devel/files/Designs/2020/0214/Support%20the%202n
> > d
> > > %20Microcode%20FV%20Flash%20Region.pdf
> > > The ShadowMicrocodePei provide a FIT based shadow microcode ppi to
> > > MpInitLib. It's needed.
> > >
> > >
> > > Best,
> > > Aaron
> > >
> > > > -----Original Message-----
> > > > From: Ni, Ray <ray.ni@intel.com>
> > > > Sent: Wednesday, September 23, 2020 2:25 PM
> > > > To: Li, Aaron <aaron.li@intel.com>; devel@edk2.groups.io
> > > > Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Fu, Siyuan
> > > > <siyuan.fu@intel.com>
> > > > Subject: RE: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add PCD
> > > > for shadowing all microcode.
> > > >
> > > > MpInitLib already contains logic to shadow microcode to memory.
> > > > Is this still needed?
> > > >
> > > > > -----Original Message-----
> > > > > From: Li, Aaron <aaron.li@intel.com>
> > > > > Sent: Wednesday, August 12, 2020 3:55 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > > > > <rangasai.v.chaganty@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > > > > Subject: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add PCD for
> > > > > shadowing all microcode.
> > > > >
> > > > > This patch is to add a PCD PcdShadowAllMicrocode to support shadowing
> > > > > all microcode patch to memory.
> > > > >
> > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2891
> > > > >
> > > > > Signed-off-by: Aaron Li <aaron.li@intel.com>
> > > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> > > > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > > > ---
> > > > >
> > > > >
> > > > Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodeP
> > > > ei.c
> > > > > | 4 ++++
> > > > >
> > > > >
> > > > Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodeP
> > > > ei.i
> > > > > nf | 3 +++
> > > > >  Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec                            | 7
> > > +++++++
> > > > >  3 files changed, 14 insertions(+)
> > > > >
> > > > > diff --git
> > > > >
> > > > a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > > > ePei
> > > > > .c
> > > > >
> > > > b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > > > ePei
> > > > > .c
> > > > > index 8d6574f66794..5c7ee6910c8e 100644
> > > > > ---
> > > > >
> > > > a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > > > ePei
> > > > > .c
> > > > > +++
> > > > >
> > > > b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > > > ePei
> > > > > .c
> > > > > @@ -132,6 +132,10 @@ IsMicrocodePatchNeedLoad (
> > > > >    CPU_MICROCODE_EXTENDED_TABLE           *ExtendedTable;
> > > > >
> > > > >    UINTN                                  Index;
> > > > >
> > > > >
> > > > >
> > > > > +  if (FeaturePcdGet (PcdShadowAllMicrocode)) {
> > > > >
> > > > > +    return TRUE;
> > > > >
> > > > > +  }
> > > > >
> > > > > +
> > > > >
> > > > >    //
> > > > >
> > > > >    // Check the 'ProcessorSignature' and 'ProcessorFlags' in microcode
> > patch
> > > > > header.
> > > > >
> > > > >    //
> > > > >
> > > > > diff --git
> > > > >
> > > > a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > > > ePei
> > > > > .inf
> > > > >
> > > > b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > > > ePei
> > > > > .inf
> > > > > index 019400ab31da..581780add891 100644
> > > > > ---
> > > > >
> > > > a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > > > ePei
> > > > > .inf
> > > > > +++
> > > > >
> > > > b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > > > ePei
> > > > > .inf
> > > > > @@ -39,5 +39,8 @@ [Guids]
> > > > >    gEdkiiMicrocodeShadowInfoHobGuid
> > > > >
> > > > >    gEdkiiMicrocodeStorageTypeFlashGuid
> > > > >
> > > > >
> > > > >
> > > > > +[Pcd]
> > > > >
> > > > > +  gIntelSiliconPkgTokenSpaceGuid.PcdShadowAllMicrocode
> > > > >
> > > > > +
> > > > >
> > > > >  [Depex]
> > > > >
> > > > >    TRUE
> > > > >
> > > > > diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> > > > > b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> > > > > index e4a7fec3a3ea..3a12fe99fac6 100644
> > > > > --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> > > > > +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> > > > > @@ -76,6 +76,13 @@ [Protocols]
> > > > >    # Include/Protocol/PlatformDeviceSecurityPolicy.h
> > > > >
> > > > >    gEdkiiDeviceSecurityPolicyProtocolGuid = {0x7ea41a99, 0x5e32, 0x4c97,
> > > > > {0x88, 0xc4, 0xd6, 0xe7, 0x46, 0x84, 0x9, 0xd4}}
> > > > >
> > > > >
> > > > >
> > > > > +[PcdsFeatureFlag]
> > > > >
> > > > > +  ## Indicates if all microcode update patches shall be shadowed to
> > > > memory.
> > > > >
> > > > > +  #   TRUE  - All microcode patches will be shadowed.<BR>
> > > > >
> > > > > +  #   FALSE - Only the microcode for current present processors will be
> > > > > shadowed.<BR>
> > > > >
> > > > > +  # @Prompt Shadow all microcode update patches.
> > > > >
> > > > > +
> > > > >
> > > > gIntelSiliconPkgTokenSpaceGuid.PcdShadowAllMicrocode|FALSE|BOOLEAN|
> > > > 0x
> > > > > 00000006
> > > > >
> > > > > +
> > > > >
> > > > >  [PcdsFixedAtBuild, PcdsPatchableInModule]
> > > > >
> > > > >    ## Error code for VTd error.<BR><BR>
> > > > >
> > > > >    #  EDKII_ERROR_CODE_VTD_ERROR = (EFI_IO_BUS_UNSPECIFIED |
> > > > > (EFI_OEM_SPECIFIC | 0x00000000)) = 0x02008000<BR>
> > > > >
> > > > > --
> > > > > 2.23.0.windows.1