public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel@lists.01.org
Subject: Re: [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: add missing call to ExitBootServices()
Date: Tue, 20 Nov 2018 14:48:54 +0000	[thread overview]
Message-ID: <20181120144854.2zjhyr3qrcm6hzhb@bivouac.eciton.net> (raw)
In-Reply-To: <20181120143300.26751-1-ard.biesheuvel@linaro.org>

On Tue, Nov 20, 2018 at 03:33:00PM +0100, Ard Biesheuvel wrote:
> Our poor man's implementation of EnterS3WithImmediateWake () currently
> sets a high TPL level to disable interrupts, and simply calls the
> PEI entrypoint again after disabling the MMU.
> 
> Unfortunately, this is not sufficient: DMA capable devices such as
> network controllers or USB controllers may still be enabled and
> writing to memory, e.g., in response to incoming network packets.
> 
> So instead, do the full ExitBootServices() dance: allocate space and
> get the memory map, call ExitBootServices(), and in case it fails, get
> the memory map again and call ExitBootServices() again. This ensures
> that all cleanup related to DMA capable devices is performed before
> doing the warm reset.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Include/Library/ArmLib.h                                    |  1 +
>  ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c | 51 ++++++++++++++++++--
>  2 files changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index e46df447b33d..ffda50e9d767 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -59,6 +59,7 @@ typedef enum {
>  
>  typedef struct {
>    EFI_PHYSICAL_ADDRESS          PhysicalBase;
> +  EFI_VIRTUAL_ADDRESS           VirtualBase;
>    UINT64                        Length;
>    ARM_MEMORY_REGION_ATTRIBUTES  Attributes;
>  } ARM_MEMORY_REGION_DESCRIPTOR;

Without the above hunk:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> index 10ceafd14d5d..c9c42ab3b244 100644
> --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>    ResetSystemLib implementation using PSCI calls
>  
> -  Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2018, Linaro Ltd. All rights reserved.<BR>
>  
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD License
> @@ -92,7 +92,13 @@ EnterS3WithImmediateWake (
>    VOID
>    )
>  {
> -  VOID (*Reset)(VOID);
> +  VOID                        (*Reset)(VOID);
> +  EFI_PHYSICAL_ADDRESS        Alloc;
> +  EFI_MEMORY_DESCRIPTOR       *MemMap;
> +  UINTN                       MemMapSize;
> +  UINTN                       MapKey, DescriptorSize;
> +  UINT32                      DescriptorVersion;
> +  EFI_STATUS                  Status;
>  
>    if (FeaturePcdGet (PcdArmReenterPeiForCapsuleWarmReboot) &&
>        !EfiAtRuntime ()) {
> @@ -103,7 +109,46 @@ EnterS3WithImmediateWake (
>      //
>      Reset = (VOID (*)(VOID))(UINTN)FixedPcdGet64 (PcdFvBaseAddress);
>  
> -    gBS->RaiseTPL (TPL_HIGH_LEVEL);
> +    //
> +    // Obtain the size of the memory map
> +    //
> +    MemMapSize = 0;
> +    Status = gBS->GetMemoryMap (&MemMapSize, MemMap, &MapKey, &DescriptorSize,
> +                    &DescriptorVersion);
> +    ASSERT (Status == EFI_BUFFER_TOO_SMALL);
> +
> +    //
> +    // Add some slack to the allocation to cater for changes in the memory
> +    // map if ExitBootServices () fails the first time around.
> +    //
> +    MemMapSize += SIZE_4KB;
> +    Status = gBS->AllocatePages (AllocateAnyPages, EfiBootServicesData,
> +                    EFI_SIZE_TO_PAGES (MemMapSize), &Alloc);
> +    ASSERT_EFI_ERROR (Status);
> +
> +    MemMap = (EFI_MEMORY_DESCRIPTOR *)(UINTN)Alloc;
> +
> +    Status = gBS->GetMemoryMap (&MemMapSize, MemMap, &MapKey, &DescriptorSize,
> +                    &DescriptorVersion);
> +    ASSERT_EFI_ERROR (Status);
> +
> +    Status = gBS->ExitBootServices (gImageHandle, MapKey);
> +    if (EFI_ERROR (Status)) {
> +      //
> +      // ExitBootServices () may fail the first time around if an event fired
> +      // right after the call to GetMemoryMap() which allocated or freed memory.
> +      // Since that first call to ExitBootServices () will disarm the timer,
> +      // this is guaranteed not to happen again, so one additional attempt
> +      // should suffice.
> +      //
> +      Status = gBS->GetMemoryMap (&MemMapSize, MemMap, &MapKey, &DescriptorSize,
> +                      &DescriptorVersion);
> +      ASSERT_EFI_ERROR (Status);
> +
> +      Status = gBS->ExitBootServices (gImageHandle, MapKey);
> +      ASSERT_EFI_ERROR (Status);
> +    }
> +
>      ArmDisableMmu ();
>      Reset ();
>    }
> -- 
> 2.17.1
> 


      parent reply	other threads:[~2018-11-20 14:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-20 14:33 [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: add missing call to ExitBootServices() Ard Biesheuvel
2018-11-20 14:34 ` Ard Biesheuvel
2018-11-20 14:48 ` Leif Lindholm [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181120144854.2zjhyr3qrcm6hzhb@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox