* [PATCH v1 1/5] MdePkg/BaseLib: Introduce new SpeculationBarrier API
2018-12-21 3:11 [PATCH v1 0/5] Ues arch-generic API SpeculationBarrier() to replace AsmLfence() Hao Wu
@ 2018-12-21 3:11 ` Hao Wu
2018-12-24 3:15 ` Gao, Liming
2018-12-21 3:11 ` [PATCH v1 2/5] MdeModulePkg/FaultTolerantWrite: Update to consume SpeculationBarrier Hao Wu
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Hao Wu @ 2018-12-21 3:11 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Ard Biesheuvel, Leif Lindholm, Liming Gao,
Michael D Kinney, Jiewen Yao, Laszlo Ersek
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
X86 specific BaseLib API AsmLfence() was introduced to address the Spectre
Variant 1 (CVE-2017-5753) issue. The purpose of this API is to insert
barriers to stop speculative execution. However, the API is highly
architecture (X86) specific, and thus should be avoided using across
generic code.
To address this issue, this patch will add a new BaseLib API called
SpeculationBarrier(). Different architectures will have different
implementations for this API.
For IA32 and x64, the implementation of SpeculationBarrier() will
directly call AsmLfence().
For ARM and AARCH64, this patch will add a temporary empty implementation
as a placeholder. We hope experts in ARM can help to contribute the actual
implementation.
For EBC, similar to the ARM and AARCH64 cases, a temporary empty
implementation is added.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
MdePkg/Library/BaseLib/BaseLib.inf | 5 +++
MdePkg/Include/Library/BaseLib.h | 15 +++++++++
MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c | 30 ++++++++++++++++++
MdePkg/Library/BaseLib/Ebc/SpeculationBarrier.c | 30 ++++++++++++++++++
MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 32 ++++++++++++++++++++
5 files changed, 112 insertions(+)
diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
index b84e58324c..d195c5417b 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -336,6 +336,7 @@
X86DisablePaging32.c
X86RdRand.c
X86PatchInstruction.c
+ X86SpeculationBarrier.c
[Sources.X64]
X64/Thunk16.nasm
@@ -515,6 +516,7 @@
X86DisablePaging32.c
X86RdRand.c
X86PatchInstruction.c
+ X86SpeculationBarrier.c
X64/GccInline.c | GCC
X64/Thunk16.S | XCODE
X64/SwitchStack.nasm| GCC
@@ -543,12 +545,14 @@
Ebc/CpuBreakpoint.c
Ebc/SetJumpLongJump.c
Ebc/SwitchStack.c
+ Ebc/SpeculationBarrier.c
Unaligned.c
Math64.c
[Sources.ARM]
Arm/InternalSwitchStack.c
Arm/Unaligned.c
+ Arm/SpeculationBarrier.c
Math64.c | RVCT
Math64.c | MSFT
@@ -582,6 +586,7 @@
[Sources.AARCH64]
Arm/InternalSwitchStack.c
Arm/Unaligned.c
+ Arm/SpeculationBarrier.c
Math64.c
AArch64/MemoryFence.S | GCC
diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 8cc086983d..1eb842384e 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -5111,6 +5111,21 @@ CpuDeadLoop (
VOID
);
+
+/**
+ Uses as a barrier to stop speculative execution.
+
+ Ensures that no later instruction will execute speculatively, until all prior
+ instructions have completed.
+
+**/
+VOID
+EFIAPI
+SpeculationBarrier (
+ VOID
+ );
+
+
#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
///
/// IA32 and x64 Specific Functions.
diff --git a/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c b/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c
new file mode 100644
index 0000000000..8a6165a102
--- /dev/null
+++ b/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c
@@ -0,0 +1,30 @@
+/** @file
+ SpeculationBarrier() function for ARM.
+
+ Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
+
+ This program and the accompanying materials are licensed and made available
+ under the terms and conditions of the BSD License which accompanies this
+ distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php.
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+ WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+
+/**
+ Uses as a barrier to stop speculative execution.
+
+ Ensures that no later instruction will execute speculatively, until all prior
+ instructions have completed.
+
+**/
+VOID
+EFIAPI
+SpeculationBarrier (
+ VOID
+ )
+{
+}
diff --git a/MdePkg/Library/BaseLib/Ebc/SpeculationBarrier.c b/MdePkg/Library/BaseLib/Ebc/SpeculationBarrier.c
new file mode 100644
index 0000000000..8fa4c204f8
--- /dev/null
+++ b/MdePkg/Library/BaseLib/Ebc/SpeculationBarrier.c
@@ -0,0 +1,30 @@
+/** @file
+ SpeculationBarrier() function for EBC.
+
+ Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
+
+ This program and the accompanying materials are licensed and made available
+ under the terms and conditions of the BSD License which accompanies this
+ distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php.
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+ WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+
+/**
+ Uses as a barrier to stop speculative execution.
+
+ Ensures that no later instruction will execute speculatively, until all prior
+ instructions have completed.
+
+**/
+VOID
+EFIAPI
+SpeculationBarrier (
+ VOID
+ )
+{
+}
diff --git a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
new file mode 100644
index 0000000000..03deca8489
--- /dev/null
+++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
@@ -0,0 +1,32 @@
+/** @file
+ SpeculationBarrier() function for IA32 and x64.
+
+ Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
+
+ This program and the accompanying materials are licensed and made available
+ under the terms and conditions of the BSD License which accompanies this
+ distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php.
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+ WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Library/BaseLib.h>
+
+/**
+ Uses as a barrier to stop speculative execution.
+
+ Ensures that no later instruction will execute speculatively, until all prior
+ instructions have completed.
+
+**/
+VOID
+EFIAPI
+SpeculationBarrier (
+ VOID
+ )
+{
+ AsmLfence ();
+}
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v1 1/5] MdePkg/BaseLib: Introduce new SpeculationBarrier API
2018-12-21 3:11 ` [PATCH v1 1/5] MdePkg/BaseLib: Introduce new SpeculationBarrier API Hao Wu
@ 2018-12-24 3:15 ` Gao, Liming
0 siblings, 0 replies; 12+ messages in thread
From: Gao, Liming @ 2018-12-24 3:15 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
Cc: Ard Biesheuvel, Leif Lindholm, Kinney, Michael D, Yao, Jiewen,
Laszlo Ersek
Reviewed-by: Liming Gao <liming.gao@intel.com>
> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, December 21, 2018 11:11 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Leif Lindholm <leif.lindholm@linaro.org>; Gao,
> Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> Subject: [PATCH v1 1/5] MdePkg/BaseLib: Introduce new SpeculationBarrier API
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
>
> X86 specific BaseLib API AsmLfence() was introduced to address the Spectre
> Variant 1 (CVE-2017-5753) issue. The purpose of this API is to insert
> barriers to stop speculative execution. However, the API is highly
> architecture (X86) specific, and thus should be avoided using across
> generic code.
>
> To address this issue, this patch will add a new BaseLib API called
> SpeculationBarrier(). Different architectures will have different
> implementations for this API.
>
> For IA32 and x64, the implementation of SpeculationBarrier() will
> directly call AsmLfence().
>
> For ARM and AARCH64, this patch will add a temporary empty implementation
> as a placeholder. We hope experts in ARM can help to contribute the actual
> implementation.
>
> For EBC, similar to the ARM and AARCH64 cases, a temporary empty
> implementation is added.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
> MdePkg/Library/BaseLib/BaseLib.inf | 5 +++
> MdePkg/Include/Library/BaseLib.h | 15 +++++++++
> MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c | 30 ++++++++++++++++++
> MdePkg/Library/BaseLib/Ebc/SpeculationBarrier.c | 30 ++++++++++++++++++
> MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 32 ++++++++++++++++++++
> 5 files changed, 112 insertions(+)
>
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
> index b84e58324c..d195c5417b 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -336,6 +336,7 @@
> X86DisablePaging32.c
> X86RdRand.c
> X86PatchInstruction.c
> + X86SpeculationBarrier.c
>
> [Sources.X64]
> X64/Thunk16.nasm
> @@ -515,6 +516,7 @@
> X86DisablePaging32.c
> X86RdRand.c
> X86PatchInstruction.c
> + X86SpeculationBarrier.c
> X64/GccInline.c | GCC
> X64/Thunk16.S | XCODE
> X64/SwitchStack.nasm| GCC
> @@ -543,12 +545,14 @@
> Ebc/CpuBreakpoint.c
> Ebc/SetJumpLongJump.c
> Ebc/SwitchStack.c
> + Ebc/SpeculationBarrier.c
> Unaligned.c
> Math64.c
>
> [Sources.ARM]
> Arm/InternalSwitchStack.c
> Arm/Unaligned.c
> + Arm/SpeculationBarrier.c
> Math64.c | RVCT
> Math64.c | MSFT
>
> @@ -582,6 +586,7 @@
> [Sources.AARCH64]
> Arm/InternalSwitchStack.c
> Arm/Unaligned.c
> + Arm/SpeculationBarrier.c
> Math64.c
>
> AArch64/MemoryFence.S | GCC
> diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
> index 8cc086983d..1eb842384e 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -5111,6 +5111,21 @@ CpuDeadLoop (
> VOID
> );
>
> +
> +/**
> + Uses as a barrier to stop speculative execution.
> +
> + Ensures that no later instruction will execute speculatively, until all prior
> + instructions have completed.
> +
> +**/
> +VOID
> +EFIAPI
> +SpeculationBarrier (
> + VOID
> + );
> +
> +
> #if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
> ///
> /// IA32 and x64 Specific Functions.
> diff --git a/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c b/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c
> new file mode 100644
> index 0000000000..8a6165a102
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c
> @@ -0,0 +1,30 @@
> +/** @file
> + SpeculationBarrier() function for ARM.
> +
> + Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
> +
> + This program and the accompanying materials are licensed and made available
> + under the terms and conditions of the BSD License which accompanies this
> + distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php.
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +
> +/**
> + Uses as a barrier to stop speculative execution.
> +
> + Ensures that no later instruction will execute speculatively, until all prior
> + instructions have completed.
> +
> +**/
> +VOID
> +EFIAPI
> +SpeculationBarrier (
> + VOID
> + )
> +{
> +}
> diff --git a/MdePkg/Library/BaseLib/Ebc/SpeculationBarrier.c b/MdePkg/Library/BaseLib/Ebc/SpeculationBarrier.c
> new file mode 100644
> index 0000000000..8fa4c204f8
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/Ebc/SpeculationBarrier.c
> @@ -0,0 +1,30 @@
> +/** @file
> + SpeculationBarrier() function for EBC.
> +
> + Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
> +
> + This program and the accompanying materials are licensed and made available
> + under the terms and conditions of the BSD License which accompanies this
> + distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php.
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +
> +/**
> + Uses as a barrier to stop speculative execution.
> +
> + Ensures that no later instruction will execute speculatively, until all prior
> + instructions have completed.
> +
> +**/
> +VOID
> +EFIAPI
> +SpeculationBarrier (
> + VOID
> + )
> +{
> +}
> diff --git a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> new file mode 100644
> index 0000000000..03deca8489
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> @@ -0,0 +1,32 @@
> +/** @file
> + SpeculationBarrier() function for IA32 and x64.
> +
> + Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
> +
> + This program and the accompanying materials are licensed and made available
> + under the terms and conditions of the BSD License which accompanies this
> + distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php.
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Library/BaseLib.h>
> +
> +/**
> + Uses as a barrier to stop speculative execution.
> +
> + Ensures that no later instruction will execute speculatively, until all prior
> + instructions have completed.
> +
> +**/
> +VOID
> +EFIAPI
> +SpeculationBarrier (
> + VOID
> + )
> +{
> + AsmLfence ();
> +}
> --
> 2.12.0.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 2/5] MdeModulePkg/FaultTolerantWrite: Update to consume SpeculationBarrier
2018-12-21 3:11 [PATCH v1 0/5] Ues arch-generic API SpeculationBarrier() to replace AsmLfence() Hao Wu
2018-12-21 3:11 ` [PATCH v1 1/5] MdePkg/BaseLib: Introduce new SpeculationBarrier API Hao Wu
@ 2018-12-21 3:11 ` Hao Wu
2018-12-24 2:56 ` Wang, Jian J
2018-12-21 3:11 ` [PATCH v1 3/5] MdeModulePkg/SmmLockBox: " Hao Wu
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Hao Wu @ 2018-12-21 3:11 UTC (permalink / raw)
To: edk2-devel; +Cc: Hao Wu, Ard Biesheuvel, Jiewen Yao, Liming Gao, Jian J Wang
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
Since BaseLib API AsmLfence() is a x86 arch specific API and should be
avoided using in generic codes, this commit replaces the usage of
AsmLfence() with arch-generic API SpeculationBarrier().
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
index 27fcab19b6..481fea3f1f 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
@@ -419,11 +419,11 @@ SmmFaultTolerantWriteHandler (
);
if (!EFI_ERROR (Status)) {
//
- // The AsmLfence() call here is to ensure the previous range/content
- // checks for the CommBuffer have been completed before calling into
- // FtwWrite().
+ // The SpeculationBarrier() call here is to ensure the previous
+ // range/content checks for the CommBuffer have been completed before
+ // calling into FtwWrite().
//
- AsmLfence ();
+ SpeculationBarrier ();
Status = FtwWrite(
&mFtwDevice->FtwInstance,
SmmFtwWriteHeader->Lba,
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v1 2/5] MdeModulePkg/FaultTolerantWrite: Update to consume SpeculationBarrier
2018-12-21 3:11 ` [PATCH v1 2/5] MdeModulePkg/FaultTolerantWrite: Update to consume SpeculationBarrier Hao Wu
@ 2018-12-24 2:56 ` Wang, Jian J
0 siblings, 0 replies; 12+ messages in thread
From: Wang, Jian J @ 2018-12-24 2:56 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
Cc: Ard Biesheuvel, Yao, Jiewen, Gao, Liming
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, December 21, 2018 11:11 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Yao, Jiewen <jiewen.yao@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> Subject: [PATCH v1 2/5] MdeModulePkg/FaultTolerantWrite: Update to
> consume SpeculationBarrier
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
>
> Since BaseLib API AsmLfence() is a x86 arch specific API and should be
> avoided using in generic codes, this commit replaces the usage of
> AsmLfence() with arch-generic API SpeculationBarrier().
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c | 8
> ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
> index 27fcab19b6..481fea3f1f 100644
> ---
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
> +++
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
> @@ -419,11 +419,11 @@ SmmFaultTolerantWriteHandler (
> );
> if (!EFI_ERROR (Status)) {
> //
> - // The AsmLfence() call here is to ensure the previous range/content
> - // checks for the CommBuffer have been completed before calling into
> - // FtwWrite().
> + // The SpeculationBarrier() call here is to ensure the previous
> + // range/content checks for the CommBuffer have been completed before
> + // calling into FtwWrite().
> //
> - AsmLfence ();
> + SpeculationBarrier ();
> Status = FtwWrite(
> &mFtwDevice->FtwInstance,
> SmmFtwWriteHeader->Lba,
> --
> 2.12.0.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 3/5] MdeModulePkg/SmmLockBox: Update to consume SpeculationBarrier
2018-12-21 3:11 [PATCH v1 0/5] Ues arch-generic API SpeculationBarrier() to replace AsmLfence() Hao Wu
2018-12-21 3:11 ` [PATCH v1 1/5] MdePkg/BaseLib: Introduce new SpeculationBarrier API Hao Wu
2018-12-21 3:11 ` [PATCH v1 2/5] MdeModulePkg/FaultTolerantWrite: Update to consume SpeculationBarrier Hao Wu
@ 2018-12-21 3:11 ` Hao Wu
2018-12-24 2:56 ` Wang, Jian J
2018-12-21 3:11 ` [PATCH v1 4/5] MdeModulePkg/Variable: " Hao Wu
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Hao Wu @ 2018-12-21 3:11 UTC (permalink / raw)
To: edk2-devel; +Cc: Hao Wu, Ard Biesheuvel, Jiewen Yao, Liming Gao, Jian J Wang
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
Since BaseLib API AsmLfence() is a x86 arch specific API and should be
avoided using in generic codes, this commit replaces the usage of
AsmLfence() with arch-generic API SpeculationBarrier().
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
index c1c9aa5663..a743129539 100644
--- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
+++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
@@ -77,10 +77,10 @@ SmmLockBoxSave (
return ;
}
//
- // The AsmLfence() call here is to ensure the above range check for the
- // CommBuffer have been completed before calling into SaveLockBox().
+ // The SpeculationBarrier() call here is to ensure the above range check for
+ // the CommBuffer have been completed before calling into SaveLockBox().
//
- AsmLfence ();
+ SpeculationBarrier ();
//
// Save data
@@ -166,10 +166,10 @@ SmmLockBoxUpdate (
return ;
}
//
- // The AsmLfence() call here is to ensure the above range check for the
- // CommBuffer have been completed before calling into UpdateLockBox().
+ // The SpeculationBarrier() call here is to ensure the above range check for
+ // the CommBuffer have been completed before calling into UpdateLockBox().
//
- AsmLfence ();
+ SpeculationBarrier ();
//
// Update data
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v1 3/5] MdeModulePkg/SmmLockBox: Update to consume SpeculationBarrier
2018-12-21 3:11 ` [PATCH v1 3/5] MdeModulePkg/SmmLockBox: " Hao Wu
@ 2018-12-24 2:56 ` Wang, Jian J
0 siblings, 0 replies; 12+ messages in thread
From: Wang, Jian J @ 2018-12-24 2:56 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
Cc: Ard Biesheuvel, Yao, Jiewen, Gao, Liming
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, December 21, 2018 11:11 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Yao, Jiewen <jiewen.yao@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> Subject: [PATCH v1 3/5] MdeModulePkg/SmmLockBox: Update to consume
> SpeculationBarrier
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
>
> Since BaseLib API AsmLfence() is a x86 arch specific API and should be
> avoided using in generic codes, this commit replaces the usage of
> AsmLfence() with arch-generic API SpeculationBarrier().
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
> MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c | 12 ++++++-
> -----
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
> b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
> index c1c9aa5663..a743129539 100644
> --- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
> +++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
> @@ -77,10 +77,10 @@ SmmLockBoxSave (
> return ;
> }
> //
> - // The AsmLfence() call here is to ensure the above range check for the
> - // CommBuffer have been completed before calling into SaveLockBox().
> + // The SpeculationBarrier() call here is to ensure the above range check for
> + // the CommBuffer have been completed before calling into SaveLockBox().
> //
> - AsmLfence ();
> + SpeculationBarrier ();
>
> //
> // Save data
> @@ -166,10 +166,10 @@ SmmLockBoxUpdate (
> return ;
> }
> //
> - // The AsmLfence() call here is to ensure the above range check for the
> - // CommBuffer have been completed before calling into UpdateLockBox().
> + // The SpeculationBarrier() call here is to ensure the above range check for
> + // the CommBuffer have been completed before calling into UpdateLockBox().
> //
> - AsmLfence ();
> + SpeculationBarrier ();
>
> //
> // Update data
> --
> 2.12.0.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 4/5] MdeModulePkg/Variable: Update to consume SpeculationBarrier
2018-12-21 3:11 [PATCH v1 0/5] Ues arch-generic API SpeculationBarrier() to replace AsmLfence() Hao Wu
` (2 preceding siblings ...)
2018-12-21 3:11 ` [PATCH v1 3/5] MdeModulePkg/SmmLockBox: " Hao Wu
@ 2018-12-21 3:11 ` Hao Wu
2018-12-24 2:59 ` Wang, Jian J
2018-12-21 3:11 ` [PATCH v1 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: " Hao Wu
2018-12-21 11:22 ` [PATCH v1 0/5] Ues arch-generic API SpeculationBarrier() to replace AsmLfence() Ard Biesheuvel
5 siblings, 1 reply; 12+ messages in thread
From: Hao Wu @ 2018-12-21 3:11 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Ard Biesheuvel, Jiewen Yao, Liming Gao, Jian J Wang,
Star Zeng
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
Since BaseLib API AsmLfence() is a x86 arch specific API and should be
avoided using in generic codes, this commit replaces the usage of
AsmLfence() with arch-generic API SpeculationBarrier().
Please note that speculation execution barriers are intended to be
asserted for SMM codes, hence, this commit still preserve an empty
implementation of the speculation execution barrier for the DXE codes.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 2 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 2 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h | 10 ++++----
MdeModulePkg/Universal/Variable/RuntimeDxe/{LoadFenceDxe.c => SpeculationBarrierDxe.c} | 12 ++++++----
MdeModulePkg/Universal/Variable/RuntimeDxe/{LoadFenceSmm.c => SpeculationBarrierSmm.c} | 14 +++++++-----
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 ++---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 24 ++++++++++----------
7 files changed, 38 insertions(+), 32 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index 868981ccaf..7ef8a97f5d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -46,7 +46,7 @@
TcgMorLockDxe.c
VarCheck.c
VariableExLib.c
- LoadFenceDxe.c
+ SpeculationBarrierDxe.c
[Packages]
MdePkg/MdePkg.dec
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
index 2fe72ff8a4..db7d220e06 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
@@ -54,7 +54,7 @@
PrivilegePolymorphic.h
VariableExLib.c
TcgMorLockSmm.c
- LoadFenceSmm.c
+ SpeculationBarrierSmm.c
[Packages]
MdePkg/MdePkg.dec
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
index a324ad2365..7af22a4ad6 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
@@ -85,13 +85,15 @@ SetVariableCheckHandlerMor (
);
/**
- This service is consumed by the variable modules to perform a serializing
- operation on all load-from-memory instructions that were issued prior to the
- call of this function.
+ This service is consumed by the variable modules to place a barrier to stop
+ speculative execution.
+
+ Ensures that no later instruction will execute speculatively, until all prior
+ instructions have completed.
**/
VOID
-MemoryLoadFence (
+VariableSpeculationBarrier (
VOID
);
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierDxe.c
similarity index 62%
rename from MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceDxe.c
rename to MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierDxe.c
index 0f64ee093b..bc3f695335 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierDxe.c
@@ -1,5 +1,5 @@
/** @file
- Serialize operation on all load-from-memory instructions (DXE version).
+ Barrier to stop speculative execution (DXE version).
Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
@@ -15,13 +15,15 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include "Variable.h"
/**
- This service is consumed by the variable modules to perform a serializing
- operation on all load-from-memory instructions that were issued prior to the
- call of this function.
+ This service is consumed by the variable modules to place a barrier to stop
+ speculative execution.
+
+ Ensures that no later instruction will execute speculatively, until all prior
+ instructions have completed.
**/
VOID
-MemoryLoadFence (
+VariableSpeculationBarrier (
VOID
)
{
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierSmm.c
similarity index 61%
rename from MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceSmm.c
rename to MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierSmm.c
index 4b0d7e3e95..dbc20f6c4d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierSmm.c
@@ -1,5 +1,5 @@
/** @file
- Serialize operation on all load-from-memory instructions (SMM version).
+ Barrier to stop speculative execution (SMM version).
Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
@@ -16,15 +16,17 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include "Variable.h"
/**
- This service is consumed by the variable modules to perform a serializing
- operation on all load-from-memory instructions that were issued prior to the
- call of this function.
+ This service is consumed by the variable modules to place a barrier to stop
+ speculative execution.
+
+ Ensures that no later instruction will execute speculatively, until all prior
+ instructions have completed.
**/
VOID
-MemoryLoadFence (
+VariableSpeculationBarrier (
VOID
)
{
- AsmLfence ();
+ SpeculationBarrier ();
}
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index d100b1dcc5..443cf07144 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -3201,11 +3201,11 @@ VariableServiceSetVariable (
return EFI_SECURITY_VIOLATION;
}
//
- // The MemoryLoadFence() call here is to ensure the above sanity check
- // for the EFI_VARIABLE_AUTHENTICATION_2 descriptor has been completed
+ // The VariableSpeculationBarrier() call here is to ensure the above sanity
+ // check for the EFI_VARIABLE_AUTHENTICATION_2 descriptor has been completed
// before the execution of subsequent codes.
//
- MemoryLoadFence ();
+ VariableSpeculationBarrier ();
PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
} else {
PayloadSize = DataSize;
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index 6dc19c24db..8c53f84ff6 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -538,11 +538,11 @@ SmmVariableHandler (
}
//
- // The MemoryLoadFence() call here is to ensure the previous range/content
- // checks for the CommBuffer have been completed before the subsequent
- // consumption of the CommBuffer content.
+ // The VariableSpeculationBarrier() call here is to ensure the previous
+ // range/content checks for the CommBuffer have been completed before the
+ // subsequent consumption of the CommBuffer content.
//
- MemoryLoadFence ();
+ VariableSpeculationBarrier ();
if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {
//
// Make sure VariableName is A Null-terminated string.
@@ -638,11 +638,11 @@ SmmVariableHandler (
}
//
- // The MemoryLoadFence() call here is to ensure the previous range/content
- // checks for the CommBuffer have been completed before the subsequent
- // consumption of the CommBuffer content.
+ // The VariableSpeculationBarrier() call here is to ensure the previous
+ // range/content checks for the CommBuffer have been completed before the
+ // subsequent consumption of the CommBuffer content.
//
- MemoryLoadFence ();
+ VariableSpeculationBarrier ();
if (SmmVariableHeader->NameSize < sizeof (CHAR16) || SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != L'\0') {
//
// Make sure VariableName is A Null-terminated string.
@@ -779,11 +779,11 @@ SmmVariableHandler (
}
//
- // The MemoryLoadFence() call here is to ensure the previous range/content
- // checks for the CommBuffer have been completed before the subsequent
- // consumption of the CommBuffer content.
+ // The VariableSpeculationBarrier() call here is to ensure the previous
+ // range/content checks for the CommBuffer have been completed before the
+ // subsequent consumption of the CommBuffer content.
//
- MemoryLoadFence ();
+ VariableSpeculationBarrier ();
if (CommVariableProperty->NameSize < sizeof (CHAR16) || CommVariableProperty->Name[CommVariableProperty->NameSize/sizeof (CHAR16) - 1] != L'\0') {
//
// Make sure VariableName is A Null-terminated string.
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v1 4/5] MdeModulePkg/Variable: Update to consume SpeculationBarrier
2018-12-21 3:11 ` [PATCH v1 4/5] MdeModulePkg/Variable: " Hao Wu
@ 2018-12-24 2:59 ` Wang, Jian J
0 siblings, 0 replies; 12+ messages in thread
From: Wang, Jian J @ 2018-12-24 2:59 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
Cc: Ard Biesheuvel, Yao, Jiewen, Gao, Liming, Zeng, Star
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, December 21, 2018 11:11 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Yao, Jiewen <jiewen.yao@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [PATCH v1 4/5] MdeModulePkg/Variable: Update to consume
> SpeculationBarrier
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
>
> Since BaseLib API AsmLfence() is a x86 arch specific API and should be
> avoided using in generic codes, this commit replaces the usage of
> AsmLfence() with arch-generic API SpeculationBarrier().
>
> Please note that speculation execution barriers are intended to be
> asserted for SMM codes, hence, this commit still preserve an empty
> implementation of the speculation execution barrier for the DXE codes.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> | 2 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> | 2 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
> | 10 ++++----
> MdeModulePkg/Universal/Variable/RuntimeDxe/{LoadFenceDxe.c =>
> SpeculationBarrierDxe.c} | 12 ++++++----
> MdeModulePkg/Universal/Variable/RuntimeDxe/{LoadFenceSmm.c =>
> SpeculationBarrierSmm.c} | 14 +++++++-----
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c |
> 6 ++---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> | 24 ++++++++++----------
> 7 files changed, 38 insertions(+), 32 deletions(-)
>
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> index 868981ccaf..7ef8a97f5d 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> @@ -46,7 +46,7 @@
> TcgMorLockDxe.c
> VarCheck.c
> VariableExLib.c
> - LoadFenceDxe.c
> + SpeculationBarrierDxe.c
>
> [Packages]
> MdePkg/MdePkg.dec
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> index 2fe72ff8a4..db7d220e06 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> @@ -54,7 +54,7 @@
> PrivilegePolymorphic.h
> VariableExLib.c
> TcgMorLockSmm.c
> - LoadFenceSmm.c
> + SpeculationBarrierSmm.c
>
> [Packages]
> MdePkg/MdePkg.dec
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
> index a324ad2365..7af22a4ad6 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
> @@ -85,13 +85,15 @@ SetVariableCheckHandlerMor (
> );
>
> /**
> - This service is consumed by the variable modules to perform a serializing
> - operation on all load-from-memory instructions that were issued prior to the
> - call of this function.
> + This service is consumed by the variable modules to place a barrier to stop
> + speculative execution.
> +
> + Ensures that no later instruction will execute speculatively, until all prior
> + instructions have completed.
>
> **/
> VOID
> -MemoryLoadFence (
> +VariableSpeculationBarrier (
> VOID
> );
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierDxe.c
> similarity index 62%
> rename from MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceDxe.c
> rename to
> MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierDxe.c
> index 0f64ee093b..bc3f695335 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierDxe.c
> @@ -1,5 +1,5 @@
> /** @file
> - Serialize operation on all load-from-memory instructions (DXE version).
> + Barrier to stop speculative execution (DXE version).
>
> Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> @@ -15,13 +15,15 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> #include "Variable.h"
>
> /**
> - This service is consumed by the variable modules to perform a serializing
> - operation on all load-from-memory instructions that were issued prior to the
> - call of this function.
> + This service is consumed by the variable modules to place a barrier to stop
> + speculative execution.
> +
> + Ensures that no later instruction will execute speculatively, until all prior
> + instructions have completed.
>
> **/
> VOID
> -MemoryLoadFence (
> +VariableSpeculationBarrier (
> VOID
> )
> {
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceSmm.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierSmm.c
> similarity index 61%
> rename from MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceSmm.c
> rename to
> MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierSmm.c
> index 4b0d7e3e95..dbc20f6c4d 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceSmm.c
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/SpeculationBarrierSmm.c
> @@ -1,5 +1,5 @@
> /** @file
> - Serialize operation on all load-from-memory instructions (SMM version).
> + Barrier to stop speculative execution (SMM version).
>
> Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> @@ -16,15 +16,17 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> #include "Variable.h"
>
> /**
> - This service is consumed by the variable modules to perform a serializing
> - operation on all load-from-memory instructions that were issued prior to the
> - call of this function.
> + This service is consumed by the variable modules to place a barrier to stop
> + speculative execution.
> +
> + Ensures that no later instruction will execute speculatively, until all prior
> + instructions have completed.
>
> **/
> VOID
> -MemoryLoadFence (
> +VariableSpeculationBarrier (
> VOID
> )
> {
> - AsmLfence ();
> + SpeculationBarrier ();
> }
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index d100b1dcc5..443cf07144 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -3201,11 +3201,11 @@ VariableServiceSetVariable (
> return EFI_SECURITY_VIOLATION;
> }
> //
> - // The MemoryLoadFence() call here is to ensure the above sanity check
> - // for the EFI_VARIABLE_AUTHENTICATION_2 descriptor has been completed
> + // The VariableSpeculationBarrier() call here is to ensure the above sanity
> + // check for the EFI_VARIABLE_AUTHENTICATION_2 descriptor has been
> completed
> // before the execution of subsequent codes.
> //
> - MemoryLoadFence ();
> + VariableSpeculationBarrier ();
> PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
> } else {
> PayloadSize = DataSize;
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> index 6dc19c24db..8c53f84ff6 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> @@ -538,11 +538,11 @@ SmmVariableHandler (
> }
>
> //
> - // The MemoryLoadFence() call here is to ensure the previous range/content
> - // checks for the CommBuffer have been completed before the subsequent
> - // consumption of the CommBuffer content.
> + // The VariableSpeculationBarrier() call here is to ensure the previous
> + // range/content checks for the CommBuffer have been completed before
> the
> + // subsequent consumption of the CommBuffer content.
> //
> - MemoryLoadFence ();
> + VariableSpeculationBarrier ();
> if (SmmVariableHeader->NameSize < sizeof (CHAR16) ||
> SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) -
> 1] != L'\0') {
> //
> // Make sure VariableName is A Null-terminated string.
> @@ -638,11 +638,11 @@ SmmVariableHandler (
> }
>
> //
> - // The MemoryLoadFence() call here is to ensure the previous range/content
> - // checks for the CommBuffer have been completed before the subsequent
> - // consumption of the CommBuffer content.
> + // The VariableSpeculationBarrier() call here is to ensure the previous
> + // range/content checks for the CommBuffer have been completed before
> the
> + // subsequent consumption of the CommBuffer content.
> //
> - MemoryLoadFence ();
> + VariableSpeculationBarrier ();
> if (SmmVariableHeader->NameSize < sizeof (CHAR16) ||
> SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) -
> 1] != L'\0') {
> //
> // Make sure VariableName is A Null-terminated string.
> @@ -779,11 +779,11 @@ SmmVariableHandler (
> }
>
> //
> - // The MemoryLoadFence() call here is to ensure the previous range/content
> - // checks for the CommBuffer have been completed before the subsequent
> - // consumption of the CommBuffer content.
> + // The VariableSpeculationBarrier() call here is to ensure the previous
> + // range/content checks for the CommBuffer have been completed before
> the
> + // subsequent consumption of the CommBuffer content.
> //
> - MemoryLoadFence ();
> + VariableSpeculationBarrier ();
> if (CommVariableProperty->NameSize < sizeof (CHAR16) ||
> CommVariableProperty->Name[CommVariableProperty->NameSize/sizeof
> (CHAR16) - 1] != L'\0') {
> //
> // Make sure VariableName is A Null-terminated string.
> --
> 2.12.0.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Update to consume SpeculationBarrier
2018-12-21 3:11 [PATCH v1 0/5] Ues arch-generic API SpeculationBarrier() to replace AsmLfence() Hao Wu
` (3 preceding siblings ...)
2018-12-21 3:11 ` [PATCH v1 4/5] MdeModulePkg/Variable: " Hao Wu
@ 2018-12-21 3:11 ` Hao Wu
2018-12-24 0:54 ` Dong, Eric
2018-12-21 11:22 ` [PATCH v1 0/5] Ues arch-generic API SpeculationBarrier() to replace AsmLfence() Ard Biesheuvel
5 siblings, 1 reply; 12+ messages in thread
From: Hao Wu @ 2018-12-21 3:11 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Ard Biesheuvel, Jiewen Yao, Liming Gao, Eric Dong,
Ruiyu Ni, Laszlo Ersek
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
Since BaseLib API AsmLfence() is a x86 arch specific API and should be
avoided using in generic codes, this commit replaces the usage of
AsmLfence() with arch-generic API SpeculationBarrier().
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 19979d5418..8c9fa14b5b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -238,10 +238,10 @@ SmmReadSaveState (
return EFI_INVALID_PARAMETER;
}
//
- // The AsmLfence() call here is to ensure the above check for the CpuIndex
- // has been completed before the execution of subsequent codes.
+ // The SpeculationBarrier() call here is to ensure the above check for the
+ // CpuIndex has been completed before the execution of subsequent codes.
//
- AsmLfence ();
+ SpeculationBarrier ();
//
// Check for special EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v1 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Update to consume SpeculationBarrier
2018-12-21 3:11 ` [PATCH v1 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: " Hao Wu
@ 2018-12-24 0:54 ` Dong, Eric
0 siblings, 0 replies; 12+ messages in thread
From: Dong, Eric @ 2018-12-24 0:54 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
Cc: Ard Biesheuvel, Yao, Jiewen, Gao, Liming, Ni, Ruiyu, Laszlo Ersek
Reviewed-by: Eric Dong <eric.dong@intel.com>
> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, December 21, 2018 11:11 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Yao, Jiewen <jiewen.yao@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH v1 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Update to
> consume SpeculationBarrier
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
>
> Since BaseLib API AsmLfence() is a x86 arch specific API and should be
> avoided using in generic codes, this commit replaces the usage of
> AsmLfence() with arch-generic API SpeculationBarrier().
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index 19979d5418..8c9fa14b5b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -238,10 +238,10 @@ SmmReadSaveState (
> return EFI_INVALID_PARAMETER;
> }
> //
> - // The AsmLfence() call here is to ensure the above check for the CpuIndex
> - // has been completed before the execution of subsequent codes.
> + // The SpeculationBarrier() call here is to ensure the above check
> + for the // CpuIndex has been completed before the execution of
> subsequent codes.
> //
> - AsmLfence ();
> + SpeculationBarrier ();
>
> //
> // Check for special EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID
> --
> 2.12.0.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/5] Ues arch-generic API SpeculationBarrier() to replace AsmLfence()
2018-12-21 3:11 [PATCH v1 0/5] Ues arch-generic API SpeculationBarrier() to replace AsmLfence() Hao Wu
` (4 preceding siblings ...)
2018-12-21 3:11 ` [PATCH v1 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: " Hao Wu
@ 2018-12-21 11:22 ` Ard Biesheuvel
5 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2018-12-21 11:22 UTC (permalink / raw)
To: Hao Wu
Cc: edk2-devel@lists.01.org, Leif Lindholm, Liming Gao,
Michael D Kinney, Jiewen Yao, Laszlo Ersek, Jian J Wang,
Star Zeng, Eric Dong, Ruiyu Ni
On Fri, 21 Dec 2018 at 04:11, Hao Wu <hao.a.wu@intel.com> wrote:
>
> X86 specific BaseLib API AsmLfence() was introduced to address the Spectre
> Variant 1 (CVE-2017-5753) issue. The purpose of this API is to insert
> barriers to stop speculative execution. However, the API is highly
> architecture (X86) specific, and thus should be avoided using across
> generic code.
>
> To address this issue, this series will add a new BaseLib API called
> SpeculationBarrier(). Different architectures will have different
> implementations for this API. And the series will replace the usage of
> AsmLfence() in generic codes with this newly added SpeculationBarrier().
>
> For the implementations of API SpeculationBarrier() among different
> architectures, this series will:
>
> * For IA32 and x64, SpeculationBarrier() will directly call AsmLfence().
> * For ARM and EBC architectures, an empty implementation is temporarily
> added as a placeholder. We hope experts in those domains can help to
> contribute the actual implementation.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>
> Hao Wu (5):
> MdePkg/BaseLib: Introduce new SpeculationBarrier API
> MdeModulePkg/FaultTolerantWrite: Update to consume SpeculationBarrier
> MdeModulePkg/SmmLockBox: Update to consume SpeculationBarrier
> MdeModulePkg/Variable: Update to consume SpeculationBarrier
> UefiCpuPkg/PiSmmCpuDxeSmm: Update to consume SpeculationBarrier
>
Thanks Hao, this looks fine to me.
We've raised this with people in ARM, so we'll try and contribute the
missing pieces as soon as we can. In the mean time, please go ahead
and merge this as is.
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 2 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 2 +-
> MdePkg/Library/BaseLib/BaseLib.inf | 5 +++
> MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h | 10 +++---
> MdePkg/Include/Library/BaseLib.h | 15 +++++++++
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c | 8 ++---
> MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c | 12 ++++----
> MdeModulePkg/Universal/Variable/RuntimeDxe/{LoadFenceDxe.c => SpeculationBarrierDxe.c} | 12 +++++---
> MdeModulePkg/Universal/Variable/RuntimeDxe/{LoadFenceSmm.c => SpeculationBarrierSmm.c} | 14 +++++----
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 ++--
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 24 +++++++--------
> MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c | 30 ++++++++++++++++++
> MdePkg/Library/BaseLib/Ebc/SpeculationBarrier.c | 30 ++++++++++++++++++
> MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 32 ++++++++++++++++++++
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 ++--
> 15 files changed, 163 insertions(+), 45 deletions(-)
> rename MdeModulePkg/Universal/Variable/RuntimeDxe/{LoadFenceDxe.c => SpeculationBarrierDxe.c} (62%)
> rename MdeModulePkg/Universal/Variable/RuntimeDxe/{LoadFenceSmm.c => SpeculationBarrierSmm.c} (61%)
> create mode 100644 MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c
> create mode 100644 MdePkg/Library/BaseLib/Ebc/SpeculationBarrier.c
> create mode 100644 MdePkg/Library/BaseLib/X86SpeculationBarrier.c
>
> --
> 2.12.0.windows.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread