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

Michael D Kinney posted 6 patches 6 years, 9 months ago
There is a newer version of this series
[edk2-devel] [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType
Posted by Michael D Kinney 6 years, 9 months ago
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.PcdControlFlowEnforcementPropertyMask|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>
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_PcdMaximumAsciiStringLength_PROMPT  #language en-US "Maximum Length of Ascii String"
 
 #string STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLength_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"
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39805): https://edk2.groups.io/g/devel/message/39805
Mute This Topic: https://groups.io/mt/31415900/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType
Posted by Laszlo Ersek 6 years, 9 months ago
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.PcdControlFlowEnforcementPropertyMask|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_PcdMaximumAsciiStringLength_PROMPT  #language en-US "Maximum Length of Ascii String"
>  
>  #string STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLength_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"
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39845): https://edk2.groups.io/g/devel/message/39845
Mute This Topic: https://groups.io/mt/31415900/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType
Posted by Michael D Kinney 6 years, 9 months ago
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"
> >
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39853): https://edk2.groups.io/g/devel/message/39853
Mute This Topic: https://groups.io/mt/31415900/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType
Posted by Laszlo Ersek 6 years, 9 months ago
On 04/30/19 17:16, Kinney, Michael D wrote:
> 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.

Ah, good point. In fact, this has more or less crossed my mind, but I
ruled out the idea, as (I thought) a multi-arch PCD would have to be a
bitmap, not a simple enum.

Of course, I was wrong about that -- in any given platform build, the
PCD doesn't have to contain the right setting for every possible
architecture supported by edk2. It only need contain the right setting
for the arch of the current platform build.

So yes, this design is great; please apply my R-b.

Thanks
Laszlo



>> -----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"
>>>
>>
>>
>> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39857): https://edk2.groups.io/g/devel/message/39857
Mute This Topic: https://groups.io/mt/31415900/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-