| 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 | expand |
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 */ >
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 */ >
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
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
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 */
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(-)