From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
Date: Wed, 20 Jun 2018 08:10:55 +0200
Subject: [U-Boot] [PATCH v8 02/30] efi: sandbox: Adjust memory usage for
	sandbox
In-Reply-To: <20180618140835.195901-3-sjg@chromium.org>
References: <20180618140835.195901-1-sjg@chromium.org>
 <20180618140835.195901-3-sjg@chromium.org>
Message-ID: <fd82eff9-ece7-42c2-e034-fb2e9db38bb7@gmx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 06/18/2018 04:08 PM, Simon Glass wrote:
> With sandbox the U-Boot code is not mapped into the sandbox memory range
> so does not need to be excluded when allocating EFI memory. Update the EFI
> memory init code to take account of that.
> 
> Also use mapmem instead of a cast to convert a memory address to a
> pointer.

This is not reflected in the patch.

> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v8: None
> Changes in v7:
> - Move some of the code from efi_memory_init() into a separate function
> 
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> - Update to use mapmem instead of a cast
> 
>  lib/efi_loader/efi_memory.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index ec66af98ea..c6410613c7 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -9,6 +9,7 @@
>  #include <efi_loader.h>
>  #include <inttypes.h>
>  #include <malloc.h>
> +#include <mapmem.h>

I cannot see any use of this include in the patch.

>  #include <watchdog.h>
>  #include <linux/list_sort.h>
>  
> @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
>  			       &t);
>  
>  	if (r == EFI_SUCCESS) {
> -		struct efi_pool_allocation *alloc = (void *)(uintptr_t)t;
> +		struct efi_pool_allocation *alloc = map_sysmem(t, size);
>  		alloc->num_pages = num_pages;
>  		*buffer = alloc->data;
>  	}
> @@ -496,14 +497,13 @@ __weak void efi_add_known_memory(void)
>  	}
>  }
>  
> -int efi_memory_init(void)
> +/* Add memory regions for U-Boot's memory and for the runtime services code */
> +static void add_u_boot_and_runtime(void)
>  {
>  	unsigned long runtime_start, runtime_end, runtime_pages;
>  	unsigned long uboot_start, uboot_pages;
>  	unsigned long uboot_stack_size = 16 * 1024 * 1024;
>  
> -	efi_add_known_memory();
> -
>  	/* Add U-Boot */
>  	uboot_start = (gd->start_addr_sp - uboot_stack_size) & ~EFI_PAGE_MASK;
>  	uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
> @@ -516,6 +516,14 @@ int efi_memory_init(void)
>  	runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>  	efi_add_memory_map(runtime_start, runtime_pages,
>  			   EFI_RUNTIME_SERVICES_CODE, false);
> +}
> +
> +int efi_memory_init(void)
> +{
> +	efi_add_known_memory();
> +
> +	if (!IS_ENABLED(CONFIG_SANDBOX))
> +		add_u_boot_and_runtime();

Is the sandbox not using relocation? A comment is missing in the code
here to explain why add_u_boot_and_runtime() should not be called for
the sandbox.

Best regards

Heinrich

>  
>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>  	/* Request a 32bit 64MB bounce buffer region */
>