From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v3 2/2] efi: Avoid calling boot services
	after ExitBootServices()
Date: Mon, 1 Jun 2015 11:26:43 +0100
Message-ID: <556C3363.3020404@citrix.com>
References: <1433153878-32199-1-git-send-email-ross.lagerwall@citrix.com>
	<1433153878-32199-2-git-send-email-ross.lagerwall@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1433153878-32199-2-git-send-email-ross.lagerwall@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ross Lagerwall <ross.lagerwall@citrix.com>, xen-devel@lists.xen.org
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>, Keir Fraser <keir@xen.org>, Ian Campbell <ian.campbell@citrix.com>, Jan Beulich <jbeulich@suse.com>, Tim Deegan <tim@xen.org>
List-Id: xen-devel@lists.xenproject.org

On 01/06/15 11:17, Ross Lagerwall wrote:
> After the first call to ExitBootServices(), avoid calling any boot
> services by setting setting efi_bs to NULL and entering an infinite loop
> in blexit().
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  xen/common/efi/boot.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index 60c1b8d..5c5c158 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -216,6 +216,12 @@ static void __init noreturn blexit(const CHAR16 *str)
>          PrintStr((CHAR16 *)str);
>      PrintStr(newline);
>  
> +    if ( !efi_bs )
> +    {
> +        for ( ; ; )
> +            ;

At the very least this should be halt() to avoid spinning in a busy
loop, and probably with a local_irq_disable() ahead of the for.

~Andrew

> +    }
> +
>      if ( cfg.addr )
>          efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
>      if ( kernel.addr )
> @@ -1063,8 +1069,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>      for ( retry = 0; ; retry = 1 )
>      {
>          efi_memmap_size = map_alloc_size;
> -        status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key,
> -                                      &efi_mdesc_size, &mdesc_ver);
> +        status = SystemTable->BootServices->GetMemoryMap(&efi_memmap_size,
> +                                                         efi_memmap, &map_key,
> +                                                         &efi_mdesc_size,
> +                                                         &mdesc_ver);
>          if ( EFI_ERROR(status) )
>              PrintErrMesg(L"Cannot obtain memory map", status);
>  
> @@ -1073,7 +1081,9 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>  
>          efi_arch_pre_exit_boot();
>  
> -        status = efi_bs->ExitBootServices(ImageHandle, map_key);
> +        status = SystemTable->BootServices->ExitBootServices(ImageHandle,
> +                                                             map_key);
> +        efi_bs = NULL;
>          if ( status != EFI_INVALID_PARAMETER || retry )
>              break;
>      }