Re: [PATCH 1/2] MdeModulePkg/CapsuleRuntimeDxe: Merge changes form arm to all ARCH

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Wed, 20 Mar 2019 at 02:43, Zhichao Gao <zhichao.gao@intel.com> wrote:
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1462
>
> The arm ARCH has already to add a function CapsuleCacheWriteBack to
> flush the cache data to DRAM. That is also required in IA32 ARCH. So
> merge the changes. And this function do not support in runtime phase.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>

After this patch, platforms may return TRUE from
QueryCapsuleCapabilities() while they returned FALSE before, i.e., at
runtime. This behavior change will surely break ARM, but I don't see
how it doesn't break IA32 as well if it now relies on this cache
maintenance to occur in the context of UpdateCapsule().



> ---
>  .../Universal/CapsuleRuntimeDxe/CapsuleReset.c     | 21 ++++++++++++++++
>  .../{Arm/CapsuleReset.c => CapsuleResetNull.c}     | 29 +++-------------------
>  .../CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf        | 14 +++++------
>  3 files changed, 30 insertions(+), 34 deletions(-)
>  rename MdeModulePkg/Universal/CapsuleRuntimeDxe/{Arm/CapsuleReset.c => CapsuleResetNull.c} (51%)
>
> diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleReset.c b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleReset.c
> index 353f6f2090..8c45f6665e 100644
> --- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleReset.c
> +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleReset.c
> @@ -16,6 +16,8 @@
>
>  #include "CapsuleService.h"
>
> +#include <Library/CacheMaintenanceLib.h>
> +
>  /**
>    Whether the platform supports capsules that persist across reset. Note that
>    some platforms only support such capsules at boot time.
> @@ -46,4 +48,23 @@ CapsuleCacheWriteBack (
>    IN  EFI_PHYSICAL_ADDRESS    ScatterGatherList
>    )
>  {
> +  EFI_CAPSULE_BLOCK_DESCRIPTOR    *Desc;
> +
> +  if (!EfiAtRuntime()) {
> +    Desc = (EFI_CAPSULE_BLOCK_DESCRIPTOR *)(UINTN)ScatterGatherList;
> +    do {
> +      WriteBackDataCacheRange (Desc, sizeof *Desc);
> +
> +      if (Desc->Length > 0) {
> +        WriteBackDataCacheRange ((VOID *)(UINTN)Desc->Union.DataBlock,
> +                                 Desc->Length
> +                                 );
> +        Desc++;
> +      } else if (Desc->Union.ContinuationPointer > 0) {
> +        Desc = (EFI_CAPSULE_BLOCK_DESCRIPTOR *)(UINTN)Desc->Union.ContinuationPointer;
> +      }
> +    } while (Desc->Length > 0 || Desc->Union.ContinuationPointer > 0);
> +
> +    WriteBackDataCacheRange (Desc, sizeof *Desc);
> +  }
>  }
> diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CapsuleReset.c b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleResetNull.c
> similarity index 51%
> rename from MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CapsuleReset.c
> rename to MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleResetNull.c
> index d79d2fc693..3c5cfc1a16 100644
> --- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CapsuleReset.c
> +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleResetNull.c
> @@ -1,8 +1,9 @@
>  /** @file
> -  ARM implementation of architecture specific routines related to
> +  Default implementation of architecture specific routines related to
>    PersistAcrossReset capsules
>
>    Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> +  Copyright (c) 2019, 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
> @@ -31,14 +32,7 @@ IsPersistAcrossResetCapsuleSupported (
>    VOID
>    )
>  {
> -  //
> -  // ARM requires the capsule payload to be cleaned to the point of coherency
> -  // (PoC), but only permits doing so using cache maintenance instructions that
> -  // operate on virtual addresses. Since at runtime, we don't know the virtual
> -  // addresses of the data structures that make up the scatter/gather list, we
> -  // cannot perform the maintenance, and all we can do is give up.
> -  //
> -  return FeaturePcdGet (PcdSupportUpdateCapsuleReset) && !EfiAtRuntime ();
> +  return FeaturePcdGet (PcdSupportUpdateCapsuleReset);
>  }
>
>  /**
> @@ -55,21 +49,4 @@ CapsuleCacheWriteBack (
>    IN  EFI_PHYSICAL_ADDRESS    ScatterGatherList
>    )
>  {
> -  EFI_CAPSULE_BLOCK_DESCRIPTOR    *Desc;
> -
> -  Desc = (EFI_CAPSULE_BLOCK_DESCRIPTOR *)(UINTN)ScatterGatherList;
> -  do {
> -    WriteBackDataCacheRange (Desc, sizeof *Desc);
> -
> -    if (Desc->Length > 0) {
> -      WriteBackDataCacheRange ((VOID *)(UINTN)Desc->Union.DataBlock,
> -                               Desc->Length
> -                               );
> -      Desc++;
> -    } else if (Desc->Union.ContinuationPointer > 0) {
> -      Desc = (EFI_CAPSULE_BLOCK_DESCRIPTOR *)(UINTN)Desc->Union.ContinuationPointer;
> -    }
> -  } while (Desc->Length > 0 || Desc->Union.ContinuationPointer > 0);
> -
> -  WriteBackDataCacheRange (Desc, sizeof *Desc);
>  }
> diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> index ad7af5fe62..c0bdf6db3d 100644
> --- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> @@ -36,15 +36,15 @@
>
>  [Sources.Ia32, Sources.EBC, Sources.ARM, Sources.AARCH64]
>    SaveLongModeContext.c
> +
> +[Sources.Ia32, Sources.X64, Sources.ARM, Sources.AARCH64]
>    CapsuleReset.c
>
> +[Sources.EBC]
> +  CapsuleResetNull.c
> +
>  [Sources.X64]
>    X64/SaveLongModeContext.c
> -  CapsuleReset.c
> -
> -[Sources.ARM, Sources.AARCH64]
> -  SaveLongModeContext.c
> -  Arm/CapsuleReset.c
>
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -61,14 +61,12 @@
>    BaseLib
>    PrintLib
>    BaseMemoryLib
> +  CacheMaintenanceLib
>
>  [LibraryClasses.X64]
>    UefiLib
>    BaseMemoryLib
>
> -[LibraryClasses.ARM, LibraryClasses.AARCH64]
> -  CacheMaintenanceLib
> -
>  [Guids]
>    ## SOMETIMES_PRODUCES   ## Variable:L"CapsuleUpdateData" # (Process across reset capsule image) for capsule updated data
>    ## SOMETIMES_PRODUCES   ## Variable:L"CapsuleLongModeBuffer" # The long mode buffer used by IA32 Capsule PEIM to call X64 CapsuleCoalesce code to handle >4GB capsule blocks
> --
> 2.16.2.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel