subject:"\\\\\\\[edk2\\\\\\\] \\\\\\\[PATCH v1 1\\\\\\\/1\\\\\\\] OvmfPkg\\\\\\\/PlatformPei\\\\\\\: clear CPU caches"

Re: [edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-01 Thread Laszlo Ersek

On 10/01/18 13:45, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> The TCG "Platform Reset Attack Mitigation Specification" requires to
> clear the processor caches when the MOR bit is set at boot time.
> 
> According to Paolo Bonzini, clearing the CPU cache takes only a few
> hundred clock cycles, so it can be done unconditionally.
> 
> Flush the cache on all logical processors, thanks to
> EFI_PEI_MP_SERVICES_PPI, calling WBINVD "Write Back and Invalidate
> Cache" x86 instruction.

(1) Please update this paragraph (and the code, of course) as suggested
by Michael. (I guess I should have known better, and suggested
WriteBackInvalidateDataCache() myself. While in this particular case,
there's not a big difference, because this code is X86 only, I agree
it's better to call the more abstract interface.)

> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
>  OvmfPkg/PlatformPei/PlatformPei.inf |   1 +
>  OvmfPkg/PlatformPei/Platform.h  |   5 +
>  OvmfPkg/PlatformPei/ClearCache.c| 110 
>  OvmfPkg/PlatformPei/Platform.c  |   1 +
>  4 files changed, 117 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
> b/OvmfPkg/PlatformPei/PlatformPei.inf
> index 9c5ad9961c4a..9c9a95fb3fe5 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -30,6 +30,7 @@
>  
>  [Sources]
>AmdSev.c
> +  ClearCache.c
>Cmos.c
>Cmos.h
>FeatureControl.c
> diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
> index f942e61bb4f9..b12a5c1f5f78 100644
> --- a/OvmfPkg/PlatformPei/Platform.h
> +++ b/OvmfPkg/PlatformPei/Platform.h
> @@ -83,6 +83,11 @@ InstallFeatureControlCallback (
>VOID
>);
>  
> +VOID
> +InstallClearCacheCallback (
> +  VOID
> +  );
> +
>  EFI_STATUS
>  InitializeXen (
>VOID
> diff --git a/OvmfPkg/PlatformPei/ClearCache.c 
> b/OvmfPkg/PlatformPei/ClearCache.c
> new file mode 100644
> index ..a1fff8446d13
> --- /dev/null
> +++ b/OvmfPkg/PlatformPei/ClearCache.c
> @@ -0,0 +1,110 @@
> +/**@file
> +  Install a callback to clear cache on all processors.

(2) Please fuse the first two paragraphs of the commit message into a
short additional sentence here, such as:

  This is for conformance with the TCG "Platform Reset Attack Mitigation
  Specification". Because clearing the CPU caches at boot doesn't impact
  performance significantly, do it unconditionally, for simplicity's
  sake.

> +
> +  Copyright (C) 2018, Red Hat, Inc.
> +
> +  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 
> +#include 
> +#include 
> +
> +#include "Platform.h"
> +
> +/**
> +  All APs execute this function in parallel. The BSP executes the function
> +  separately.
> +
> +  @param[in,out] WorkSpace  Pointer to the input/output argument workspace
> +shared by all processors.
> +**/

(3) Please prepend a short sentence to the top of the comment block,
stating what the callback does.

> +STATIC
> +VOID
> +EFIAPI
> +ClearCache (
> +  IN OUT VOID *WorkSpace
> +  )
> +{
> +  AsmWbinvd ();

(4) This is where Mike's comment applies.

> +}
> +
> +/**
> +  Notification function called when EFI_PEI_MP_SERVICES_PPI becomes 
> available.
> +
> +  @param[in] PeiServices  Indirect reference to the PEI Services Table.
> +  @param[in] NotifyDescriptor Address of the notification descriptor data
> +  structure.
> +  @param[in] Ppi  Address of the PPI that was installed.
> +
> +  @return  Status of the notification. The status code returned from this
> +   function is ignored.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +OnMpServicesAvailable (

(5) Technically this is correct, but we should preferably avoid a
function name collision even for debug log purposes, between this file,
and "OvmfPkg/PlatformPei/FeatureControl.c".

We already disambiguate the log message quite well, because we log
gEfiCallerBaseName, not just __FUNCTION__; however, in this case,
gEfiCallerBaseName will no longer be unique. So please rename the
function (include ClearCache somehow in the name).

> +  IN EFI_PEI_SERVICES   **PeiServices,
> +  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
> +  IN VOID   *Ppi
> +  )
> +{
> +  EFI_PEI_MP_SERVICES_PPI *MpServices;
> +  EFI_STATUS  Status;
> +
> +  DEBUG ((DEBUG_INFO, "%a: %a\n", gEfiCallerBaseName,

Re: [edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-01 Thread Kinney, Michael D

Hi,

Could you use a service from the CacheMaintenanceLib instead?

/**
  Writes back and invalidates the entire data cache in cache coherency domain
  of the calling CPU.

  Writes back and invalidates the entire data cache in cache coherency domain
  of the calling CPU. This function guarantees that all dirty cache lines are
  written back to system memory, and also invalidates all the data cache lines
  in the cache coherency domain of the calling CPU.

**/
VOID
EFIAPI
WriteBackInvalidateDataCache (
  VOID
  )

Thanks,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of
> marcandre.lur...@redhat.com
> Sent: Monday, October 1, 2018 4:45 AM
> To: edk2-devel@lists.01.org
> Cc: Justen, Jordan L ;
> Anthony Perard ; Laszlo
> Ersek 
> Subject: [edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei:
> clear CPU caches
> 
> From: Marc-André Lureau 
> 
> The TCG "Platform Reset Attack Mitigation
> Specification" requires to
> clear the processor caches when the MOR bit is set at
> boot time.
> 
> According to Paolo Bonzini, clearing the CPU cache
> takes only a few
> hundred clock cycles, so it can be done
> unconditionally.
> 
> Flush the cache on all logical processors, thanks to
> EFI_PEI_MP_SERVICES_PPI, calling WBINVD "Write Back and
> Invalidate
> Cache" x86 instruction.
> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau
> 
> ---
>  OvmfPkg/PlatformPei/PlatformPei.inf |   1 +
>  OvmfPkg/PlatformPei/Platform.h  |   5 +
>  OvmfPkg/PlatformPei/ClearCache.c| 110
> 
>  OvmfPkg/PlatformPei/Platform.c  |   1 +
>  4 files changed, 117 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf
> b/OvmfPkg/PlatformPei/PlatformPei.inf
> index 9c5ad9961c4a..9c9a95fb3fe5 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -30,6 +30,7 @@
> 
>  [Sources]
>AmdSev.c
> +  ClearCache.c
>Cmos.c
>Cmos.h
>FeatureControl.c
> diff --git a/OvmfPkg/PlatformPei/Platform.h
> b/OvmfPkg/PlatformPei/Platform.h
> index f942e61bb4f9..b12a5c1f5f78 100644
> --- a/OvmfPkg/PlatformPei/Platform.h
> +++ b/OvmfPkg/PlatformPei/Platform.h
> @@ -83,6 +83,11 @@ InstallFeatureControlCallback (
>VOID
>);
> 
> +VOID
> +InstallClearCacheCallback (
> +  VOID
> +  );
> +
>  EFI_STATUS
>  InitializeXen (
>VOID
> diff --git a/OvmfPkg/PlatformPei/ClearCache.c
> b/OvmfPkg/PlatformPei/ClearCache.c
> new file mode 100644
> index ..a1fff8446d13
> --- /dev/null
> +++ b/OvmfPkg/PlatformPei/ClearCache.c
> @@ -0,0 +1,110 @@
> +/**@file
> +  Install a callback to clear cache on all processors.
> +
> +  Copyright (C) 2018, Red Hat, Inc.
> +
> +  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 
> +#include 
> +#include 
> +
> +#include "Platform.h"
> +
> +/**
> +  All APs execute this function in parallel. The BSP
> executes the function
> +  separately.
> +
> +  @param[in,out] WorkSpace  Pointer to the
> input/output argument workspace
> +shared by all processors.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ClearCache (
> +  IN OUT VOID *WorkSpace
> +  )
> +{
> +  AsmWbinvd ();
> +}
> +
> +/**
> +  Notification function called when
> EFI_PEI_MP_SERVICES_PPI becomes available.
> +
> +  @param[in] PeiServices  Indirect reference to
> the PEI Services Table.
> +  @param[in] NotifyDescriptor Address of the
> notification descriptor data
> +  structure.
> +  @param[in] Ppi  Address of the PPI that
> was installed.
> +
> +  @return  Status of the notification. The status code
> returned from this
> +   function is ignored.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +OnMpServicesAvailable (
> +  IN EFI_PEI_SERVICES   **PeiServices,
> +  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
> +  IN VOID   *Ppi
> +  )
> +{
> +  EFI_PEI_MP_SERVICES_PPI *MpSer

[edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-01 Thread marcandre . lureau

From: Marc-André Lureau 

The TCG "Platform Reset Attack Mitigation Specification" requires to
clear the processor caches when the MOR bit is set at boot time.

According to Paolo Bonzini, clearing the CPU cache takes only a few
hundred clock cycles, so it can be done unconditionally.

Flush the cache on all logical processors, thanks to
EFI_PEI_MP_SERVICES_PPI, calling WBINVD "Write Back and Invalidate
Cache" x86 instruction.

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Anthony Perard 
Cc: Julien Grall 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
---
 OvmfPkg/PlatformPei/PlatformPei.inf |   1 +
 OvmfPkg/PlatformPei/Platform.h  |   5 +
 OvmfPkg/PlatformPei/ClearCache.c| 110 
 OvmfPkg/PlatformPei/Platform.c  |   1 +
 4 files changed, 117 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 9c5ad9961c4a..9c9a95fb3fe5 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -30,6 +30,7 @@
 
 [Sources]
   AmdSev.c
+  ClearCache.c
   Cmos.c
   Cmos.h
   FeatureControl.c
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index f942e61bb4f9..b12a5c1f5f78 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -83,6 +83,11 @@ InstallFeatureControlCallback (
   VOID
   );
 
+VOID
+InstallClearCacheCallback (
+  VOID
+  );
+
 EFI_STATUS
 InitializeXen (
   VOID
diff --git a/OvmfPkg/PlatformPei/ClearCache.c b/OvmfPkg/PlatformPei/ClearCache.c
new file mode 100644
index ..a1fff8446d13
--- /dev/null
+++ b/OvmfPkg/PlatformPei/ClearCache.c
@@ -0,0 +1,110 @@
+/**@file
+  Install a callback to clear cache on all processors.
+
+  Copyright (C) 2018, Red Hat, Inc.
+
+  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 
+#include 
+#include 
+
+#include "Platform.h"
+
+/**
+  All APs execute this function in parallel. The BSP executes the function
+  separately.
+
+  @param[in,out] WorkSpace  Pointer to the input/output argument workspace
+shared by all processors.
+**/
+STATIC
+VOID
+EFIAPI
+ClearCache (
+  IN OUT VOID *WorkSpace
+  )
+{
+  AsmWbinvd ();
+}
+
+/**
+  Notification function called when EFI_PEI_MP_SERVICES_PPI becomes available.
+
+  @param[in] PeiServices  Indirect reference to the PEI Services Table.
+  @param[in] NotifyDescriptor Address of the notification descriptor data
+  structure.
+  @param[in] Ppi  Address of the PPI that was installed.
+
+  @return  Status of the notification. The status code returned from this
+   function is ignored.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+OnMpServicesAvailable (
+  IN EFI_PEI_SERVICES   **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID   *Ppi
+  )
+{
+  EFI_PEI_MP_SERVICES_PPI *MpServices;
+  EFI_STATUS  Status;
+
+  DEBUG ((DEBUG_INFO, "%a: %a\n", gEfiCallerBaseName, __FUNCTION__));
+
+  //
+  // Clear cache on all the APs in parallel.
+  //
+  MpServices = Ppi;
+  Status = MpServices->StartupAllAPs (
+ (CONST EFI_PEI_SERVICES **)PeiServices,
+ MpServices,
+ ClearCache,  // Procedure
+ FALSE,   // SingleThread
+ 0,   // TimeoutInMicroSeconds: inf.
+ NULL // ProcedureArgument
+ );
+  if (EFI_ERROR (Status) && Status != EFI_NOT_STARTED) {
+DEBUG ((DEBUG_ERROR, "%a: StartupAllAps(): %r\n", __FUNCTION__, Status));
+return Status;
+  }
+
+  //
+  // Now clear cache on the BSP too.
+  //
+  ClearCache (NULL);
+  return EFI_SUCCESS;
+}
+
+//
+// Notification object for registering the callback, for when
+// EFI_PEI_MP_SERVICES_PPI becomes available.
+//
+STATIC CONST EFI_PEI_NOTIFY_DESCRIPTOR mMpServicesNotify = {
+  EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | // Flags
+  EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
+  ,   // Guid
+  OnMpServicesAvailable// Notify
+};
+
+VOID
+InstallClearCacheCallback (
+  VOID
+  )
+{
+  EFI_STATUS   Status;
+
+  Status = PeiServicesNotifyPpi ();
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "%a: failed to set up MP Services callback: %r\n",
+  __FUNCTION__, Status));
+  }
+}
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 5a78668126b4..22139a64cbf4 100644
---

Re: [edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

Re: [edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

[edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

3 matches

Site Navigation

Mail list logo

Footer information