* [PATCH 0/2] Change reset logic related on capsule
@ 2019-03-20 1:42 Zhichao Gao
2019-03-20 1:42 ` [PATCH 1/2] MdeModulePkg/CapsuleRuntimeDxe: Merge changes form arm to all ARCH Zhichao Gao
2019-03-20 1:42 ` [PATCH 2/2] MdeModulePkg/ResetSystemRuntimeDxe: Remove DoS3 in warm reset Zhichao Gao
0 siblings, 2 replies; 5+ messages in thread
From: Zhichao Gao @ 2019-03-20 1:42 UTC (permalink / raw)
To: edk2-devel
Add CapsuleCacheWriteBack for IA ARCH.
Remove DoS3 in ResetSystemRuntimeDxe.
Zhichao Gao (2):
MdeModulePkg/CapsuleRuntimeDxe: Merge changes form arm to all ARCH
MdeModulePkg/ResetSystemRuntimeDxe: Remove DoS3 in warm reset
.../Universal/CapsuleRuntimeDxe/CapsuleReset.c | 21 ++++++++++++
.../{Arm/CapsuleReset.c => CapsuleResetNull.c} | 29 ++---------------
.../CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf | 14 ++++----
.../Universal/ResetSystemRuntimeDxe/ResetSystem.c | 38 ----------------------
4 files changed, 30 insertions(+), 72 deletions(-)
rename MdeModulePkg/Universal/CapsuleRuntimeDxe/{Arm/CapsuleReset.c => CapsuleResetNull.c} (51%)
--
2.16.2.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] MdeModulePkg/CapsuleRuntimeDxe: Merge changes form arm to all ARCH
2019-03-20 1:42 [PATCH 0/2] Change reset logic related on capsule Zhichao Gao
@ 2019-03-20 1:42 ` Zhichao Gao
2019-03-20 9:48 ` Ard Biesheuvel
2019-03-20 1:42 ` [PATCH 2/2] MdeModulePkg/ResetSystemRuntimeDxe: Remove DoS3 in warm reset Zhichao Gao
1 sibling, 1 reply; 5+ messages in thread
From: Zhichao Gao @ 2019-03-20 1:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao
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>
---
.../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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] MdeModulePkg/ResetSystemRuntimeDxe: Remove DoS3 in warm reset
2019-03-20 1:42 [PATCH 0/2] Change reset logic related on capsule Zhichao Gao
2019-03-20 1:42 ` [PATCH 1/2] MdeModulePkg/CapsuleRuntimeDxe: Merge changes form arm to all ARCH Zhichao Gao
@ 2019-03-20 1:42 ` Zhichao Gao
1 sibling, 0 replies; 5+ messages in thread
From: Zhichao Gao @ 2019-03-20 1:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1462
Original logic is that checking the CapsuleUpdate variable
and do the EnterS3WithImmediateWake if the system require a
capsule update. The EnterS3WithImmediateWake is usually
implemented in Platform ResetSystemLib instance and it may
do some operation for capsule update. For now, thess preparations
of capsule are platform reset notify functions' duty. Most
platforms need flush cache to memory before warm reset during
capsule update and this operation is added to capsule flow.
So it is safe to remove it and do not affect the capsule update
function.
Change-Id: I7af8221528c7baee6d16daee79837c7e9584b232
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>
---
.../Universal/ResetSystemRuntimeDxe/ResetSystem.c | 38 ----------------------
1 file changed, 38 deletions(-)
diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
index 4c7107faea..36234f4d5b 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
@@ -206,22 +206,6 @@ InitializeResetSystem (
return Status;
}
-/**
- Put the system into S3 power state.
-**/
-VOID
-DoS3 (
- VOID
- )
-{
- EnterS3WithImmediateWake ();
-
- //
- // Should not return
- //
- CpuDeadLoop ();
-}
-
/**
Resets the entire platform.
@@ -249,9 +233,6 @@ RuntimeServiceResetSystem (
IN VOID *ResetData OPTIONAL
)
{
- EFI_STATUS Status;
- UINTN Size;
- UINTN CapsuleDataPtr;
LIST_ENTRY *Link;
RESET_NOTIFY_ENTRY *Entry;
@@ -315,25 +296,6 @@ RuntimeServiceResetSystem (
switch (ResetType) {
case EfiResetWarm:
- //
- //Check if there are pending capsules to process
- //
- Size = sizeof (CapsuleDataPtr);
- Status = EfiGetVariable (
- EFI_CAPSULE_VARIABLE_NAME,
- &gEfiCapsuleVendorGuid,
- NULL,
- &Size,
- (VOID *) &CapsuleDataPtr
- );
-
- if (Status == EFI_SUCCESS) {
- //
- //Process capsules across a system reset.
- //
- DoS3();
- }
-
ResetWarm ();
break;
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] MdeModulePkg/CapsuleRuntimeDxe: Merge changes form arm to all ARCH
2019-03-20 1:42 ` [PATCH 1/2] MdeModulePkg/CapsuleRuntimeDxe: Merge changes form arm to all ARCH Zhichao Gao
@ 2019-03-20 9:48 ` Ard Biesheuvel
2019-03-21 7:41 ` Gao, Zhichao
0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2019-03-20 9:48 UTC (permalink / raw)
To: Zhichao Gao; +Cc: edk2-devel@lists.01.org, Hao Wu, Liming Gao, Star Zeng
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] MdeModulePkg/CapsuleRuntimeDxe: Merge changes form arm to all ARCH
2019-03-20 9:48 ` Ard Biesheuvel
@ 2019-03-21 7:41 ` Gao, Zhichao
0 siblings, 0 replies; 5+ messages in thread
From: Gao, Zhichao @ 2019-03-21 7:41 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Wu, Hao A, Gao, Liming, Zeng, Star
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, March 20, 2019 5:49 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>
> Cc: edk2-devel@lists.01.org; Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH 1/2] MdeModulePkg/CapsuleRuntimeDxe:
> Merge changes form arm to all ARCH
>
> 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().
>
You are right. I forgot to consider the function QueryCapsuleCapabilities.
At runtime, maybe IA32 would catch the same issue on arm ARCH. That should be concerned.
For this patch set, we only want to add cache function before runtime. So it is better to keep its original logic and add this support.
I would fix the issue you mentioned to make sure it would not affect arm ARCH. But disable some function for IA32 ARCH should be another case.
Thanks,
Zhichao
>
>
> > ---
> > .../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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-21 7:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-20 1:42 [PATCH 0/2] Change reset logic related on capsule Zhichao Gao
2019-03-20 1:42 ` [PATCH 1/2] MdeModulePkg/CapsuleRuntimeDxe: Merge changes form arm to all ARCH Zhichao Gao
2019-03-20 9:48 ` Ard Biesheuvel
2019-03-21 7:41 ` Gao, Zhichao
2019-03-20 1:42 ` [PATCH 2/2] MdeModulePkg/ResetSystemRuntimeDxe: Remove DoS3 in warm reset Zhichao Gao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox