On 10/01/18 13:45, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau <marcandre.lur...@redhat.com> > > 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 <jordan.l.jus...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Cc: Anthony Perard <anthony.per...@citrix.com> > Cc: Julien Grall <julien.gr...@linaro.org> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > 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 000000000000..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 <Library/DebugLib.h> > +#include <Library/PeiServicesLib.h> > +#include <Ppi/MpServices.h> > + > +#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, __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; > + } (Once you rename this function, this message becomes unique too.) > + > + // > + // 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, > + &gEfiPeiMpServicesPpiGuid, // Guid > + OnMpServicesAvailable // Notify > +}; > + > +VOID > +InstallClearCacheCallback ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + > + Status = PeiServicesNotifyPpi (&mMpServicesNotify); > + 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 > --- a/OvmfPkg/PlatformPei/Platform.c > +++ b/OvmfPkg/PlatformPei/Platform.c > @@ -672,6 +672,7 @@ InitializePlatform ( > NoexecDxeInitialization (); > } > > + InstallClearCacheCallback (); > AmdSevInitialize (); > MiscInitialization (); > InstallFeatureControlCallback (); > Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
