[edk2-devel] [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType

Michael D Kinney posted 6 patches 6 years, 9 months ago
There is a newer version of this series
[edk2-devel] [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType
Posted by Michael D Kinney 6 years, 9 months ago
Use PcdSpeculationBarrierType in the x86 implementation
of SpeculationBarrier() to select between AsmLfence(),
AsmCpuid(), and no operation.

Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 MdePkg/Library/BaseLib/BaseLib.inf             | 1 +
 MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
index 533e83e0b2..3586beb0ab 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -394,6 +394,7 @@ [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength     ## SOMETIMES_CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength   ## SOMETIMES_CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask   ## SOMETIMES_CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType       ## SOMETIMES_CONSUMES
 
 [FeaturePcd]
   gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList  ## CONSUMES
diff --git a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
index 8e5f983bb8..b28fd8de9b 100644
--- a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
+++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
@@ -1,7 +1,7 @@
 /** @file
   SpeculationBarrier() function for IA32 and x64.
 
-  Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (C) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -22,5 +22,9 @@ SpeculationBarrier (
   VOID
   )
 {
-  AsmLfence ();
+  if (PcdGet8 (PcdSpeculationBarrierType) == 0x01) {
+    AsmLfence ();
+  } else if (PcdGet8 (PcdSpeculationBarrierType) == 0x02) {
+    AsmCpuid (0x01, NULL, NULL, NULL, NULL);
+  }
 }
-- 
2.21.0.windows.1


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

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

Re: [edk2-devel] [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType
Posted by Brian J. Johnson 6 years, 9 months ago
On 4/29/19 8:30 PM, Michael D Kinney wrote:
> Use PcdSpeculationBarrierType in the x86 implementation
> of SpeculationBarrier() to select between AsmLfence(),
> AsmCpuid(), and no operation.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>   MdePkg/Library/BaseLib/BaseLib.inf             | 1 +
>   MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 8 ++++++--
>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
> index 533e83e0b2..3586beb0ab 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -394,6 +394,7 @@ [Pcd]
>     gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength     ## SOMETIMES_CONSUMES
>     gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength   ## SOMETIMES_CONSUMES
>     gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask   ## SOMETIMES_CONSUMES
> +  gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType       ## SOMETIMES_CONSUMES
>   
>   [FeaturePcd]
>     gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList  ## CONSUMES
> diff --git a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> index 8e5f983bb8..b28fd8de9b 100644
> --- a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> +++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> @@ -1,7 +1,7 @@
>   /** @file
>     SpeculationBarrier() function for IA32 and x64.
>   
> -  Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (C) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
>   
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>   
> @@ -22,5 +22,9 @@ SpeculationBarrier (
>     VOID
>     )
>   {
> -  AsmLfence ();
> +  if (PcdGet8 (PcdSpeculationBarrierType) == 0x01) {
> +    AsmLfence ();
> +  } else if (PcdGet8 (PcdSpeculationBarrierType) == 0x02) {
> +    AsmCpuid (0x01, NULL, NULL, NULL, NULL);
> +  }
>   }
> 

Looks good.  I'm not a maintainer, but FWIW:

Reviewed-by: Brian J. Johnson <brian.johnson@hpe.com>

-- 
Brian J. Johnson
Enterprise X86 Lab

Hewlett Packard Enterprise

brian.johnson@hpe.com
+1 651 683 7521  Office

Eagan, MN
hpe.com

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

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

Re: [edk2-devel] [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType
Posted by Laszlo Ersek 6 years, 9 months ago
On 04/30/19 03:30, Michael D Kinney wrote:
> Use PcdSpeculationBarrierType in the x86 implementation
> of SpeculationBarrier() to select between AsmLfence(),
> AsmCpuid(), and no operation.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  MdePkg/Library/BaseLib/BaseLib.inf             | 1 +
>  MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 8 ++++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
> index 533e83e0b2..3586beb0ab 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -394,6 +394,7 @@ [Pcd]
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength     ## SOMETIMES_CONSUMES
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength   ## SOMETIMES_CONSUMES
>    gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask   ## SOMETIMES_CONSUMES
> +  gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType       ## SOMETIMES_CONSUMES
>  
>  [FeaturePcd]
>    gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList  ## CONSUMES

So, based on the example of

  MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf

and a few other INF files, I think we could use

  [Pcd.IA32, Pcd.X64]

here as well.

Just an idea.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> diff --git a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> index 8e5f983bb8..b28fd8de9b 100644
> --- a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> +++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> @@ -1,7 +1,7 @@
>  /** @file
>    SpeculationBarrier() function for IA32 and x64.
>  
> -  Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (C) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -22,5 +22,9 @@ SpeculationBarrier (
>    VOID
>    )
>  {
> -  AsmLfence ();
> +  if (PcdGet8 (PcdSpeculationBarrierType) == 0x01) {
> +    AsmLfence ();
> +  } else if (PcdGet8 (PcdSpeculationBarrierType) == 0x02) {
> +    AsmCpuid (0x01, NULL, NULL, NULL, NULL);
> +  }
>  }
> 


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

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

Re: [edk2-devel] [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType
Posted by Michael D Kinney 6 years, 9 months ago
Laszlo,

I agree the current usage is only for IA32/X64 source
files so your suggestion make sense.  If there is
a use case for other archs in the future for this PCD,
then the INF would have to be updated.

Mike

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, April 30, 2019 4:50 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 2/6]
> MdePkg/BaseLib: Use PcdSpeculationBarrierType
> 
> On 04/30/19 03:30, Michael D Kinney wrote:
> > Use PcdSpeculationBarrierType in the x86
> implementation
> > of SpeculationBarrier() to select between
> AsmLfence(),
> > AsmCpuid(), and no operation.
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> > ---
> >  MdePkg/Library/BaseLib/BaseLib.inf             | 1 +
> >  MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 8
> ++++++--
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> b/MdePkg/Library/BaseLib/BaseLib.inf
> > index 533e83e0b2..3586beb0ab 100644
> > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > @@ -394,6 +394,7 @@ [Pcd]
> >
> gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength
> ## SOMETIMES_CONSUMES
> >
> gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength
> ## SOMETIMES_CONSUMES
> >
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPrope
> rtyMask   ## SOMETIMES_CONSUMES
> > +  gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType
> ## SOMETIMES_CONSUMES
> >
> >  [FeaturePcd]
> >    gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList  ##
> CONSUMES
> 
> So, based on the example of
> 
> 
> MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCp
> u.inf
> 
> and a few other INF files, I think we could use
> 
>   [Pcd.IA32, Pcd.X64]
> 
> here as well.
> 
> Just an idea.
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> Thanks
> Laszlo
> 
> > diff --git
> a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> > index 8e5f983bb8..b28fd8de9b 100644
> > --- a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> > +++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    SpeculationBarrier() function for IA32 and x64.
> >
> > -  Copyright (C) 2018, Intel Corporation. All rights
> reserved.<BR>
> > +  Copyright (C) 2018 - 2019, Intel Corporation. All
> rights reserved.<BR>
> >
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -22,5 +22,9 @@ SpeculationBarrier (
> >    VOID
> >    )
> >  {
> > -  AsmLfence ();
> > +  if (PcdGet8 (PcdSpeculationBarrierType) == 0x01) {
> > +    AsmLfence ();
> > +  } else if (PcdGet8 (PcdSpeculationBarrierType) ==
> 0x02) {
> > +    AsmCpuid (0x01, NULL, NULL, NULL, NULL);
> > +  }
> >  }
> >


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

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