Re: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches
On 10/05/18 18:49, Kinney, Michael D wrote:
> Reviewed-by: Michael D Kinney
Great, thank you :)
Patch pushed as commit d20ae95a13e8.
Thanks!
Laszlo
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-
>> boun...@lists.01.org] On Behalf Of
>> marcandre.lur...@redhat.com
>> Sent: Tuesday, October 2, 2018 5:17 AM
>> To: edk2-devel@lists.01.org
>> Cc: Justen, Jordan L ;
>> Anthony Perard ; Laszlo Ersek
>>
>> Subject: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei:
>> clear CPU caches
>>
>> From: Marc-André Lureau
>>
>> 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.
>>
>> Flush the cache on all logical processors, thanks to
>> EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.
>>
>> 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
>>
>> Message-Id: <20181002120730.13013-1-
>> marcandre.lur...@redhat.com>
>> ---
>>
>> v3:
>> - update top comment with notes about TCG spec
>> - sort headers inclusion
>>
>> OvmfPkg/PlatformPei/PlatformPei.inf | 2 +
>> OvmfPkg/PlatformPei/Platform.h | 5 +
>> OvmfPkg/PlatformPei/ClearCache.c| 117
>>
>> OvmfPkg/PlatformPei/Platform.c | 1 +
>> 4 files changed, 125 insertions(+)
>>
>> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf
>> b/OvmfPkg/PlatformPei/PlatformPei.inf
>> index 9c5ad9961c4a..5c8dd0fe6d72 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
>> @@ -54,6 +55,7 @@
>>
>> [LibraryClasses]
>>BaseLib
>> + CacheMaintenanceLib
>>DebugLib
>>HobLib
>>IoLib
>> 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 ..7d15fd925c3c
>> --- /dev/null
>> +++ b/OvmfPkg/PlatformPei/ClearCache.c
>> @@ -0,0 +1,117 @@
>> +/**@file
>> + Install a callback to clear cache on all processors.
>> + 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
>> +
>> +#include "Platform.h"
>> +
>> +/**
>> + Invalidate data & instruction caches.
>> + 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
>> + )
>> +{
>> + WriteBackInvalidateDataCache ();
>> + InvalidateInstructionCache ();
>> +}
>> +
>> +/**
>&
Re: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches
Reviewed-by: Michael D Kinney
Mike
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of
> marcandre.lur...@redhat.com
> Sent: Tuesday, October 2, 2018 5:17 AM
> To: edk2-devel@lists.01.org
> Cc: Justen, Jordan L ;
> Anthony Perard ; Laszlo Ersek
>
> Subject: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei:
> clear CPU caches
>
> From: Marc-André Lureau
>
> 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.
>
> Flush the cache on all logical processors, thanks to
> EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.
>
> 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
>
> Message-Id: <20181002120730.13013-1-
> marcandre.lur...@redhat.com>
> ---
>
> v3:
> - update top comment with notes about TCG spec
> - sort headers inclusion
>
> OvmfPkg/PlatformPei/PlatformPei.inf | 2 +
> OvmfPkg/PlatformPei/Platform.h | 5 +
> OvmfPkg/PlatformPei/ClearCache.c| 117
>
> OvmfPkg/PlatformPei/Platform.c | 1 +
> 4 files changed, 125 insertions(+)
>
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf
> b/OvmfPkg/PlatformPei/PlatformPei.inf
> index 9c5ad9961c4a..5c8dd0fe6d72 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
> @@ -54,6 +55,7 @@
>
> [LibraryClasses]
>BaseLib
> + CacheMaintenanceLib
>DebugLib
>HobLib
>IoLib
> 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 ..7d15fd925c3c
> --- /dev/null
> +++ b/OvmfPkg/PlatformPei/ClearCache.c
> @@ -0,0 +1,117 @@
> +/**@file
> + Install a callback to clear cache on all processors.
> + 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
> +
> +#include "Platform.h"
> +
> +/**
> + Invalidate data & instruction caches.
> + 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
> + )
> +{
> + WriteBackInvalidateDataCache ();
> + InvalidateInstructionCache ();
> +}
> +
> +/**
> + 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
> +ClearCacheOnMpServicesAvailable (
> + IN EFI_PEI_SERVICES **PeiServices,
> + IN EFI_P
Re: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches
On 10/02/18 14:17, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau > > 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. > > Flush the cache on all logical processors, thanks to > EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib. > > 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 > Message-Id: <20181002120730.13013-1-marcandre.lur...@redhat.com> I'm going to remove the Message-Id line on push. I don't know why it's there, but either way, it's not correct. (The msgid of your posting is <20181002121725.17178-1-marcandre.lur...@redhat.com>.) With that: Reviewed-by: Laszlo Ersek Also, Regression-tested-by: Laszlo Ersek I'm ready to push this, but I'd still like to get an A-b from Mike (CC'd). Thanks! Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches
On 10/02/18 15:27, Anthony PERARD wrote: > On Tue, Oct 02, 2018 at 04:17:25PM +0400, marcandre.lur...@redhat.com wrote: >> From: Marc-André Lureau >> >> 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. >> >> Flush the cache on all logical processors, thanks to >> EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib. >> >> 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 > > That patch works for me on Xen. I can still boot guests with the patch > applied, with either 1 or 4 vcpus assigned to the guest: > > Tested-by: Anthony PERARD Thanks a lot! Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches
On Tue, Oct 02, 2018 at 04:17:25PM +0400, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau > > 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. > > Flush the cache on all logical processors, thanks to > EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib. > > 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 That patch works for me on Xen. I can still boot guests with the patch applied, with either 1 or 4 vcpus assigned to the guest: Tested-by: Anthony PERARD Thanks, -- Anthony PERARD ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches
On 10/02/18 14:17, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau > > 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. > > Flush the cache on all logical processors, thanks to > EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib. > > 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 > Message-Id: <20181002120730.13013-1-marcandre.lur...@redhat.com> > --- > > v3: > - update top comment with notes about TCG spec > - sort headers inclusion So, meta comments first... I got this patch (v3) in three variants: (a) the one you sent me off-list, as quoted-printable: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable (b) this message, reaching me directly from you: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (c) this message, reflected by the edk2-devel list software, from you to me: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Results: - git-am fails with (a). - git-am *also* fails with (c). - git-am only works on (b). Which means that your setup is now correct, but some mail server on path (c) is broken, and corrupts your patch email when it transcodes the email to base64. Yay! I guess I'll work with copy (b), in my inbox. Sigh, is the mailing list workflow actually *more* broken than github pull requests? I thought that was impossible, but I guess I'm being proven wrong. Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches
From: Marc-André Lureau
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.
Flush the cache on all logical processors, thanks to
EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.
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
Message-Id: <20181002120730.13013-1-marcandre.lur...@redhat.com>
---
v3:
- update top comment with notes about TCG spec
- sort headers inclusion
OvmfPkg/PlatformPei/PlatformPei.inf | 2 +
OvmfPkg/PlatformPei/Platform.h | 5 +
OvmfPkg/PlatformPei/ClearCache.c| 117
OvmfPkg/PlatformPei/Platform.c | 1 +
4 files changed, 125 insertions(+)
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 9c5ad9961c4a..5c8dd0fe6d72 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
@@ -54,6 +55,7 @@
[LibraryClasses]
BaseLib
+ CacheMaintenanceLib
DebugLib
HobLib
IoLib
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 ..7d15fd925c3c
--- /dev/null
+++ b/OvmfPkg/PlatformPei/ClearCache.c
@@ -0,0 +1,117 @@
+/**@file
+ Install a callback to clear cache on all processors.
+ 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
+
+#include "Platform.h"
+
+/**
+ Invalidate data & instruction caches.
+ 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
+ )
+{
+ WriteBackInvalidateDataCache ();
+ InvalidateInstructionCache ();
+}
+
+/**
+ 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
+ClearCacheOnMpServicesAvailable (
+ 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
+
