[U-Boot] [PATCH v8 02/30] efi: sandbox: Adjust memory usage for sandbox

[Simon Glass]

Hi Alex,

On 21 June 2018 at 03:52, Alexander Graf <agraf at suse.de> wrote:
> On 06/21/2018 04:02 AM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 20 June 2018 at 02:54, Alexander Graf <agraf at suse.de> wrote:
>>>
>>> On 06/20/2018 08:10 AM, Heinrich Schuchardt wrote:
>>>>
>>>> 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 at 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);
>>>
>>>
>>> ^^^
>>>
>>> This is where mapmem.h gets used. And yes, it's the wrong place. So NAK
>>> on
>>> the patch as-is.
>>
>> What is wrong with it?
>
>
> Efi_allocate_pool calls efi_allocate_pages() which according to spec returns
> a pointer. So efi_allocate_pool() should not have to map anything, because
> it does not receive an addres.

You are referring to efi_allocate_pages_ext() I suspect. In my series
this does the mapping, so that efi_allocate_pages() uses addresses
only. We could rename it if you like.

Regards,
Simon