[U-Boot,v4,03/16] efi: sandbox: Adjust memory usage for sandbox

Message ID 20180516154233.21457-4-sjg@chromium.org
State Superseded
Delegated to: Alexander Graf
Headers show
Series
  • efi: Enable basic sandbox support for EFI loader
Related show

Commit Message

Simon Glass May 16, 2018, 3:42 p.m.
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.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

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 | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

Comments

Heinrich Schuchardt May 16, 2018, 5:02 p.m. | #1
On 05/16/2018 05:42 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.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

This patch does not apply to Alex's efi-next repository due to
https://github.com/agraf/u-boot/commit/e62d2cd60480a867dd21a102238f5387b82140d7

Please, rebase.

Best regards

Heinrich

> ---
> 
> 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 | 31 ++++++++++++++++++-------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 664c651db56..3fbed63728b 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>
>  #include <watchdog.h>
>  #include <asm/global_data.h>
>  #include <linux/list_sort.h>
> @@ -388,7 +389,7 @@ efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
>  	r = efi_allocate_pages(0, pool_type, num_pages, &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;
>  	}
> @@ -499,18 +500,22 @@ int efi_memory_init(void)
>  
>  	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;
> -	efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);
> -
> -	/* Add Runtime Services */
> -	runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
> -	runtime_end = (ulong)&__efi_runtime_stop;
> -	runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
> -	runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
> -	efi_add_memory_map(runtime_start, runtime_pages,
> -			   EFI_RUNTIME_SERVICES_CODE, false);
> +	if (!IS_ENABLED(CONFIG_SANDBOX)) {
> +		/* 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;
> +		efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA,
> +				   false);
> +
> +		/* Add Runtime Services */
> +		runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
> +		runtime_end = (ulong)&__efi_runtime_stop;
> +		runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
> +		runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
> +		efi_add_memory_map(runtime_start, runtime_pages,
> +				   EFI_RUNTIME_SERVICES_CODE, false);
> +	}
>  
>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>  	/* Request a 32bit 64MB bounce buffer region */
>
Heinrich Schuchardt May 16, 2018, 5:15 p.m. | #2
On 05/16/2018 05:42 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.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

running ./u-boot

bootefi selftest
Found 0 disks
WARNING: booting without device tree

Testing EFI API implementation

Number of tests to execute: 17

Setting up 'block device'
Setting up 'block device' succeeded

Executing 'block device'
lib/efi_selftest/efi_selftest_block_device.c(378):
TODO: Wrong volume label 'xxa1', expected 'U-BOOT TEST'
FAT: Misaligned buffer address (00007ff70aafe658)
Segmentation fault

Please, fix the alignment fault. You have to ensure that the memory that
Sandbox has retrieved via malloc is reduced to 4k aligned pages before
being published to the EFI implementation in lib/efi_loader/efi_memory.c

Best regards

Heinrich

> ---
> 
> 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 | 31 ++++++++++++++++++-------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 664c651db56..3fbed63728b 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>
>  #include <watchdog.h>
>  #include <asm/global_data.h>
>  #include <linux/list_sort.h>
> @@ -388,7 +389,7 @@ efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
>  	r = efi_allocate_pages(0, pool_type, num_pages, &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;
>  	}
> @@ -499,18 +500,22 @@ int efi_memory_init(void)
>  
>  	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;
> -	efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);
> -
> -	/* Add Runtime Services */
> -	runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
> -	runtime_end = (ulong)&__efi_runtime_stop;
> -	runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
> -	runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
> -	efi_add_memory_map(runtime_start, runtime_pages,
> -			   EFI_RUNTIME_SERVICES_CODE, false);
> +	if (!IS_ENABLED(CONFIG_SANDBOX)) {
> +		/* 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;
> +		efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA,
> +				   false);
> +
> +		/* Add Runtime Services */
> +		runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
> +		runtime_end = (ulong)&__efi_runtime_stop;
> +		runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
> +		runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
> +		efi_add_memory_map(runtime_start, runtime_pages,
> +				   EFI_RUNTIME_SERVICES_CODE, false);
> +	}
>  
>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>  	/* Request a 32bit 64MB bounce buffer region */
>
Heinrich Schuchardt May 24, 2018, 7:16 p.m. | #3
On 05/16/2018 07:15 PM, Heinrich Schuchardt wrote:
> On 05/16/2018 05:42 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.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
> 
> running ./u-boot
> 
> bootefi selftest
> Found 0 disks
> WARNING: booting without device tree
> 
> Testing EFI API implementation
> 
> Number of tests to execute: 17
> 
> Setting up 'block device'
> Setting up 'block device' succeeded
> 
> Executing 'block device'
> lib/efi_selftest/efi_selftest_block_device.c(378):
> TODO: Wrong volume label 'xxa1', expected 'U-BOOT TEST'
> FAT: Misaligned buffer address (00007ff70aafe658)
> Segmentation fault
> 
> Please, fix the alignment fault. You have to ensure that the memory that
> Sandbox has retrieved via malloc is reduced to 4k aligned pages before
> being published to the EFI implementation in lib/efi_loader/efi_memory.c

Hello Simon,

couldn't we use mmap() instead of malloc() to allocate the memory used
by the Sandbox? This would guarantee page aligned memory. mmap() with
MAP_ANON is available both on POSIX and BSD systems.

Best regards

Heinrich
Simon Glass May 25, 2018, 2:42 a.m. | #4
Hi Heinrick,

On 24 May 2018 at 13:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 05/16/2018 07:15 PM, Heinrich Schuchardt wrote:
>> On 05/16/2018 05:42 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.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>
>> running ./u-boot
>>
>> bootefi selftest
>> Found 0 disks
>> WARNING: booting without device tree
>>
>> Testing EFI API implementation
>>
>> Number of tests to execute: 17
>>
>> Setting up 'block device'
>> Setting up 'block device' succeeded
>>
>> Executing 'block device'
>> lib/efi_selftest/efi_selftest_block_device.c(378):
>> TODO: Wrong volume label 'xxa1', expected 'U-BOOT TEST'
>> FAT: Misaligned buffer address (00007ff70aafe658)
>> Segmentation fault
>>
>> Please, fix the alignment fault. You have to ensure that the memory that
>> Sandbox has retrieved via malloc is reduced to 4k aligned pages before
>> being published to the EFI implementation in lib/efi_loader/efi_memory.c
>
> Hello Simon,
>
> couldn't we use mmap() instead of malloc() to allocate the memory used
> by the Sandbox? This would guarantee page aligned memory. mmap() with
> MAP_ANON is available both on POSIX and BSD systems.

We do use mmap() to allocate U-Boot's memory. I wonder why it is not
page-aligned?

See os_malloc() for the implementation. Perhaps it needs another arg added?

Regards,
Simon

Patch

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index 664c651db56..3fbed63728b 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>
 #include <watchdog.h>
 #include <asm/global_data.h>
 #include <linux/list_sort.h>
@@ -388,7 +389,7 @@  efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
 	r = efi_allocate_pages(0, pool_type, num_pages, &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;
 	}
@@ -499,18 +500,22 @@  int efi_memory_init(void)
 
 	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;
-	efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);
-
-	/* Add Runtime Services */
-	runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
-	runtime_end = (ulong)&__efi_runtime_stop;
-	runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
-	runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
-	efi_add_memory_map(runtime_start, runtime_pages,
-			   EFI_RUNTIME_SERVICES_CODE, false);
+	if (!IS_ENABLED(CONFIG_SANDBOX)) {
+		/* 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;
+		efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA,
+				   false);
+
+		/* Add Runtime Services */
+		runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
+		runtime_end = (ulong)&__efi_runtime_stop;
+		runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
+		runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
+		efi_add_memory_map(runtime_start, runtime_pages,
+				   EFI_RUNTIME_SERVICES_CODE, false);
+	}
 
 #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
 	/* Request a 32bit 64MB bounce buffer region */