Re: [edk2-devel] [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType

["Michael D Kinney" <michael.d.kinney@intel.com>]

Laszlo,

I tried to design this PCD so it could be used for other
architectures as needed in the future by expanding the enum.
I marked enum values 0x01(LFENCE) and 0x02(CPUID) for
IA32/X64.  Value 0x00 (NOP) is for all archs.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io
> [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
> Sent: Tuesday, April 30, 2019 4:47 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [Patch V2 1/6] MdePkg: Add
> PcdSpeculationBarrierType
> 
> On 04/30/19 03:30, Michael D Kinney wrote:
> > Add
> gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType that
> > uses the PCD type FixedAtBuild.  This performs a
> build time
> > selection for the type of speculation barrier to use
> in the
> > BaseLib function SpeculationBarrier().  The
> recommended
> > speculation barrier for x86 is LFENCE and this is the
> default
> > value for this PCD.  x86 CPUs that do not support
> LFENCE must
> > select one of the other supported values which
> includes CPUID
> > and nothing.
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> > ---
> >  MdePkg/MdePkg.dec | 9 +++++++++
> >  MdePkg/MdePkg.uni | 8 ++++++++
> >  2 files changed, 17 insertions(+)
> >
> > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> > index e2ea8fff66..28d4a966c2 100644
> > --- a/MdePkg/MdePkg.dec
> > +++ b/MdePkg/MdePkg.dec
> > @@ -2062,6 +2062,15 @@ [PcdsFixedAtBuild]
> >    # @Prompt Enable control flow enforcement.
> >
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPrope
> rtyMask|0x0|UINT32|0x30001017
> >
> > +  ## Indicates the type of instruction sequence to
> use for a speculation
> > +  #  barrier.  The default instruction sequence is
> LFENCE.<BR><BR>
> > +  #   0x00 - No operation.<BR>
> > +  #   0x01 - LFENCE (IA32/X64).<BR>
> > +  #   0x02 - CPUID  (IA32/X64).<BR>
> > +  #   Other - reserved
> > +  # @Prompt Speculation Barrier Type.
> > +
> gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType|0x01
> |UINT8|0x30001018
> > +
> >  [PcdsFixedAtBuild,PcdsPatchableInModule]
> >    ## Indicates the maximum length of unicode string
> used in the following
> >    #  BaseLib functions: StrLen(), StrSize(),
> StrCmp(), StrnCmp(), StrCpy(), StrnCpy()<BR><BR>
> 
> In MdePkg.dec, we have:
> - [Includes.X64]
> - [LibraryClasses.X64]
> - [Guids.X64]
> 
> but no PCD declarations that are architecture-specific.
> Is that
> intentional? Because, this PCD could be a good
> candidate for "IA32/X64
> only". (Looking at the next patch too.)
> 
> But, that's just my curiosity.
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> Thanks
> Laszlo
> 
> 
> > diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
> > index c359bb4b5b..5c1fa24065 100644
> > --- a/MdePkg/MdePkg.uni
> > +++ b/MdePkg/MdePkg.uni
> > @@ -149,6 +149,14 @@
> >
> " BIT0 - SMM CET Shadow Stack is enabled.<BR>\n"
> >
> " Other - reserved"
> >
> > +#string
> STR_gEfiMdePkgTokenSpaceGuid_PcdSpeculationBarrierType_
> PROMPT  #language en-US "Speculation Barrier Type."
> > +
> > +#string
> STR_gEfiMdePkgTokenSpaceGuid_PcdSpeculationBarrierType_
> HELP  #language en-US  "Indicates the type of
> instruction sequence to use for a speculation.barrier.
> The default instruction sequence is LFENCE.<BR><BR>\n"
> > +
> "0x00 - No operation.<BR>\n"
> > +
> "0x01 - LFENCE (IA32/X64).<BR>\n"
> > +
> "0x02 - CPUID  (IA32/X64).<BR>\n"
> > +
> "Other - reserved"
> > +
> >  #string
> STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLengt
> h_PROMPT  #language en-US "Maximum Length of Ascii
> String"
> >
> >  #string
> STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLengt
> h_HELP  #language en-US "Sets the maximum number of
> ASCII characters used for string functions.  This
> affects the following BaseLib functions: AsciiStrLen(),
> AsciiStrSize(), AsciiStrCmp(), AsciiStrnCmp(),
> AsciiStrCpy(), AsciiStrnCpy(). <BR><BR>\n"
> >
> 
> 
>