Hi Alex, On 14 June 2018 at 08:22, Alexander Graf <ag...@suse.de> wrote: > > > > > Am 14.06.2018 um 16:12 schrieb Simon Glass <s...@chromium.org>: > > > > Hi Alex, > > > >> On 14 June 2018 at 07:41, Alexander Graf <ag...@suse.de> wrote: > >>> On 06/14/2018 02:58 PM, Simon Glass wrote: > >>> > >>> Hi Alex, > >>> > >>>> On 14 June 2018 at 04:12, Alexander Graf <ag...@suse.de> wrote: > >>>> > >>>>> On 06/13/2018 04:37 AM, 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 <s...@chromium.org> > >>>>> --- > >>>>> > >>>>> 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 | 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 ec66af98ea..210e49ee8b 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 <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); > >>>> > >>>> > >>>> This is still the wrong spot. We don't want the conversion to happen when > >>>> going from an EFI internal address to an allocation, but when determining > >>>> which addresses are usable in the first place. > >>> > >>> There seem to be two ways to do this: > >>> > >>> 1. Record addresses (ulong) in the EFI tables and use map_sysmem() > >>> before returning an address in the allocator > >>> 2. Store pointers in the EFI tables using map_sysmem(), then do no > >>> mapping in the allocator > >>> > >>> I've gone with option 1 since: > >>> > >>> - the EFI addresses are not pointers > >>> - it makes sandbox consistent with other architectures which use an > >>> address rather than a pointer in EFI tables > >>> - we don't normally do pointer arithmetic on the results of map_sysmem() > >>> - we normally map the memory when it is used rather than when it is set up > >>> > >>> I think you are asking for option 2. I think that would get very > >>> confusing. The addresses where things actually end up in sandbox are > >>> best kept to sandbox. > >>> > >>> Overall I feel that you are either missing the point of sandbox > >>> addressing, or don't agree with how it is done. But it does work > >>> pretty well and we don't get a lot of confusion with sandbox pointers > >>> since we typically use the address until the last moment. > >> > >> > >> I've assembled a quick tree for version 2. With that I'm able to run a > >> simple hello world efi application. Grub refuses to start because it wants > >> memory in the low 32bit and also emits random PIO accessing functions, > >> which > >> obviously don't work work from user space. > >> > >> But overall, I think this is the right path to tackle this: > >> > >> https://github.com/agraf/u-boot/tree/efi-sandbox > > > > What do you mean by version 2? > > Option 2 is what you called it. It's the only option we have to make efi > binaries work. > > > It looks like you've added one patch, > > so will you send that to the list? > > It's more than 1 patch. And yes, I'll send them. > > > > > Anyway, I hope I can convince you of the above, the way sandbox memory > > works. > > I still dislike option 1 :) > > The reason is simple: The efi memory map is available to efi payloads. It's > perfectly legal for them to do a static allocation at a particular address. > We also share a lot of (host) pointers for callbacks and structs already with > efi applications, so there is no real point to have a split brain situation > between u-boot and host pointers.
OK so you mean they don't have to allocate memory before using it? How then do you make sure that there is no conflict? I thought EFI was an API? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot