[edk2,edk2-platforms,v4,27/34] Silicon/SynQuacerMemoryInitPeiLib: ignore capsules when clearing NVRAM

Message ID 20171110142127.12018-28-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • add support for Socionext SynQuacer
Related show

Commit Message

Ard Biesheuvel Nov. 10, 2017, 2:21 p.m.
In preparation of adding support for setting a DIP switch to clear the
EFI variable store, update the early capsule handling logic to take the
boot mode into account.

This is necessary for two reasons:
- we override the boot mode when a capsule is detected,
- the capsule detection itself involves reading a EFI variable, which we
  shouldn't be doing if the varstore may be in a bad state.

So factor out the initial capsule check (to keep the code understandable)
and only perform it if we are not booting in 'clear NVRAM' mode.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c | 68 +++++++++++++-------
 1 file changed, 45 insertions(+), 23 deletions(-)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Nov. 17, 2017, 3:23 p.m. | #1
On Fri, Nov 10, 2017 at 02:21:20PM +0000, Ard Biesheuvel wrote:
> In preparation of adding support for setting a DIP switch to clear the

> EFI variable store, update the early capsule handling logic to take the

> boot mode into account.

> 

> This is necessary for two reasons:

> - we override the boot mode when a capsule is detected,

> - the capsule detection itself involves reading a EFI variable, which we

>   shouldn't be doing if the varstore may be in a bad state.

> 

> So factor out the initial capsule check (to keep the code understandable)

> and only perform it if we are not booting in 'clear NVRAM' mode.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


Minor suggestion below, fold in if you like it. Regardless:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


> ---

>  Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c | 68 +++++++++++++-------

>  1 file changed, 45 insertions(+), 23 deletions(-)

> 

> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c

> index b44c58d61062..63c441872da7 100644

> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c

> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c

> @@ -170,6 +170,44 @@ DeclareDram (

>    return EFI_SUCCESS;

>  }

>  

> +STATIC

> +BOOLEAN

> +CheckCapsule (

> +  IN  EFI_PEI_SERVICES              **PeiServices,

> +  IN  PEI_CAPSULE_PPI               *Capsule,

> +  IN  EFI_PHYSICAL_ADDRESS          UefiMemoryBase,

> +  OUT VOID                          **CapsuleBuffer,

> +  OUT UINTN                         *CapsuleBufferLength

> +  )

> +{

> +  EFI_STATUS        Status;

> +

> +  Status = Capsule->CheckCapsuleUpdate (PeiServices);

> +  if (!EFI_ERROR (Status)) {

> +

> +    //

> +    // Coalesce the capsule into unused memory. CreateState() below will copy

> +    // it to a properly allocated buffer.

> +    //

> +    *CapsuleBuffer = (VOID *)PcdGet64 (PcdSystemMemoryBase);

> +    *CapsuleBufferLength = UefiMemoryBase - PcdGet64 (PcdSystemMemoryBase);

> +

> +    PeiServicesSetBootMode (BOOT_ON_FLASH_UPDATE);

> +

> +    Status = Capsule->Coalesce (PeiServices, CapsuleBuffer,

> +                        CapsuleBufferLength);

> +    if (!EFI_ERROR (Status)) {

> +      DEBUG ((DEBUG_INFO, "%a: Coalesced capsule @ %p (0x%lx)\n",

> +        __FUNCTION__, *CapsuleBuffer, *CapsuleBufferLength));

> +      return TRUE;

> +    } else {

> +      DEBUG ((DEBUG_WARN, "%a: failed to coalesce() capsule (Status == %r)\n",

> +        __FUNCTION__, Status));

> +    }

> +  }

> +  return FALSE;

> +}

> +

>  EFI_STATUS

>  EFIAPI

>  MemoryPeim (

> @@ -184,6 +222,7 @@ MemoryPeim (

>    VOID                          *CapsuleBuffer;

>    UINTN                         CapsuleBufferLength;

>    BOOLEAN                       HaveCapsule;

> +  EFI_BOOT_MODE                 BootMode;

>  

>    Status = DeclareDram (&VirtualMemoryTable);

>    ASSERT_EFI_ERROR (Status);

> @@ -199,31 +238,14 @@ MemoryPeim (

>    ASSERT_EFI_ERROR (Status);

>  

>    //

> -  // Check for persistent capsules

> +  // Check for persistent capsules, unless we are booting with default

> +  // settings.

>    //

>    HaveCapsule = FALSE;

> -  Status = Capsule->CheckCapsuleUpdate (PeiServices);

> -  if (!EFI_ERROR (Status)) {

> -

> -    //

> -    // Coalesce the capsule into unused memory. CreateState() below will copy

> -    // it to a properly allocated buffer.

> -    //

> -    CapsuleBuffer = (VOID *)PcdGet64 (PcdSystemMemoryBase);

> -    CapsuleBufferLength = UefiMemoryBase - PcdGet64 (PcdSystemMemoryBase);

> -

> -    PeiServicesSetBootMode (BOOT_ON_FLASH_UPDATE);

> -

> -    Status = Capsule->Coalesce (PeiServices, &CapsuleBuffer,

> -                           &CapsuleBufferLength);

> -    if (!EFI_ERROR (Status)) {

> -      DEBUG ((DEBUG_INFO, "%a: Coalesced capsule @ %p (0x%lx)\n",

> -        __FUNCTION__, CapsuleBuffer, CapsuleBufferLength));

> -      HaveCapsule = TRUE;

> -    } else {

> -      DEBUG ((DEBUG_WARN, "%a: failed to coalesce() capsule (Status == %r)\n",

> -        __FUNCTION__, Status));

> -    }

> +  Status = PeiServicesGetBootMode (&BootMode);

> +  if (!EFI_ERROR (Status) && BootMode != BOOT_WITH_DEFAULT_SETTINGS) {

> +    HaveCapsule = CheckCapsule (PeiServices, Capsule, UefiMemoryBase,

> +                    &CapsuleBuffer, &CapsuleBufferLength);


You could do
     } else {
       HaveCapsule = FALSE;
here instead of unconditionally before the test.

/
    Leif

>    }

>  

>    Status = ArmConfigureMmu (VirtualMemoryTable, NULL, NULL);

> -- 

> 2.11.0

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c
index b44c58d61062..63c441872da7 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c
@@ -170,6 +170,44 @@  DeclareDram (
   return EFI_SUCCESS;
 }
 
+STATIC
+BOOLEAN
+CheckCapsule (
+  IN  EFI_PEI_SERVICES              **PeiServices,
+  IN  PEI_CAPSULE_PPI               *Capsule,
+  IN  EFI_PHYSICAL_ADDRESS          UefiMemoryBase,
+  OUT VOID                          **CapsuleBuffer,
+  OUT UINTN                         *CapsuleBufferLength
+  )
+{
+  EFI_STATUS        Status;
+
+  Status = Capsule->CheckCapsuleUpdate (PeiServices);
+  if (!EFI_ERROR (Status)) {
+
+    //
+    // Coalesce the capsule into unused memory. CreateState() below will copy
+    // it to a properly allocated buffer.
+    //
+    *CapsuleBuffer = (VOID *)PcdGet64 (PcdSystemMemoryBase);
+    *CapsuleBufferLength = UefiMemoryBase - PcdGet64 (PcdSystemMemoryBase);
+
+    PeiServicesSetBootMode (BOOT_ON_FLASH_UPDATE);
+
+    Status = Capsule->Coalesce (PeiServices, CapsuleBuffer,
+                        CapsuleBufferLength);
+    if (!EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_INFO, "%a: Coalesced capsule @ %p (0x%lx)\n",
+        __FUNCTION__, *CapsuleBuffer, *CapsuleBufferLength));
+      return TRUE;
+    } else {
+      DEBUG ((DEBUG_WARN, "%a: failed to coalesce() capsule (Status == %r)\n",
+        __FUNCTION__, Status));
+    }
+  }
+  return FALSE;
+}
+
 EFI_STATUS
 EFIAPI
 MemoryPeim (
@@ -184,6 +222,7 @@  MemoryPeim (
   VOID                          *CapsuleBuffer;
   UINTN                         CapsuleBufferLength;
   BOOLEAN                       HaveCapsule;
+  EFI_BOOT_MODE                 BootMode;
 
   Status = DeclareDram (&VirtualMemoryTable);
   ASSERT_EFI_ERROR (Status);
@@ -199,31 +238,14 @@  MemoryPeim (
   ASSERT_EFI_ERROR (Status);
 
   //
-  // Check for persistent capsules
+  // Check for persistent capsules, unless we are booting with default
+  // settings.
   //
   HaveCapsule = FALSE;
-  Status = Capsule->CheckCapsuleUpdate (PeiServices);
-  if (!EFI_ERROR (Status)) {
-
-    //
-    // Coalesce the capsule into unused memory. CreateState() below will copy
-    // it to a properly allocated buffer.
-    //
-    CapsuleBuffer = (VOID *)PcdGet64 (PcdSystemMemoryBase);
-    CapsuleBufferLength = UefiMemoryBase - PcdGet64 (PcdSystemMemoryBase);
-
-    PeiServicesSetBootMode (BOOT_ON_FLASH_UPDATE);
-
-    Status = Capsule->Coalesce (PeiServices, &CapsuleBuffer,
-                           &CapsuleBufferLength);
-    if (!EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_INFO, "%a: Coalesced capsule @ %p (0x%lx)\n",
-        __FUNCTION__, CapsuleBuffer, CapsuleBufferLength));
-      HaveCapsule = TRUE;
-    } else {
-      DEBUG ((DEBUG_WARN, "%a: failed to coalesce() capsule (Status == %r)\n",
-        __FUNCTION__, Status));
-    }
+  Status = PeiServicesGetBootMode (&BootMode);
+  if (!EFI_ERROR (Status) && BootMode != BOOT_WITH_DEFAULT_SETTINGS) {
+    HaveCapsule = CheckCapsule (PeiServices, Capsule, UefiMemoryBase,
+                    &CapsuleBuffer, &CapsuleBufferLength);
   }
 
   Status = ArmConfigureMmu (VirtualMemoryTable, NULL, NULL);