[U-Boot] [PATCH v8 22/30] efi: sandbox: Tidy up copy_fdt() to work with sandbox

[Alexander Graf]

On 06/21/2018 09:45 PM, Simon Glass wrote:
> Hi Alex,
>
> On 21 June 2018 at 04:13, Alexander Graf <agraf at suse.de> wrote:
>> On 06/21/2018 04:01 AM, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On 18 June 2018 at 09:00, Alexander Graf <agraf at suse.de> wrote:
>>>> On 06/18/2018 04:08 PM, Simon Glass wrote:
>>>>> At present this function takes a pointer as its argument, then passes
>>>>> this
>>>>> to efi_allocate_pages(), which actually takes an address. It uses casts,
>>>>> which are not supported on sandbox.
>>>>
>>>> I think this is the big API misunderstanding that caused our
>>>> disagreements:
>>>> efi_allocate_pages(uint64_t *memory) gets a uint64_t *address as input
>>>> parameter, but uses the same field to actually return a void * pointer.
>>>> This
>>>> is the function that really converts between virtual and physical address
>>>> space.
>>>>
>>>> This is the explicit wording of the spec[1] page 168:
>>>>
>>>>     The AllocatePages() function allocates the requested number of pages
>>>> and
>>>> returns a pointer to the base address of the page range in the location
>>>> referenced by Memory.
>>> The code at present uses *Memory as the address on both input and
>>> output. The spec is confusing, but I suspect that is what it meant.
>>
>> The spec means *Memory on IN is an address, on OUT is a pointer. It's quite
>> clear on that. Since the functions is available to payloads, we should
>> follow that semantic.
>>
>>>> So yes, we have to cast. There is no other way around it without
>>>> completely
>>>> creating a trainwreck of the API.
>>> efi_allocate_pages_ext() can do this though. We don't need to copy the
>>> API to efi_allocate_pages().
>>
>> Yikes. There's no way we'll create a frankenstein not-really-almost EFI API
>> inside U-Boot. Either we stick to the standard or we don't.
> The API is the _ext() function, right? We can rename the internal
> function if you like.
>
> In any case I think you have this confused. From the spec:
>
>       "Pointer to a physical address. On input, the way in which the
> address is used depends on the value of Type. See “Description” for
> more information. On output the address is set to the base of the page
> range that was allocated. See “Related Definitions.”"
>
> The parameter does not turn into a pointer on exit. It is an address,
> just as it is on input. What am I missing?

Just keep reading. A few lines further down:

   The AllocatePages() function allocates the requested number of pages 
and returns a pointer to the base address of the page range in the 
location referenced by Memory.

the spec explicitly says the function returns *a pointer to the base 
address*. It doesn't return an address. It returns a pointer.

Either way, I've applied the patch that calls map_sysmem() inside 
efi_allocate_pages() to efi-next.


Alex