[PATCH 07/40] efi: Correct call to write_acpi_tables()

[Heinrich Schuchardt]

On 12/1/21 20:32, Simon Glass wrote:
> Hi Heinrich,
>
> On Wed, 1 Dec 2021 at 11:08, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 12/1/21 17:02, Simon Glass wrote:
>>> This must be passed a ulong, not a u64. Fix it to avoid LTO warnings on
>>> sandbox.
>>>
>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>> ---
>>>
>>>    lib/efi_loader/efi_acpi.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/lib/efi_loader/efi_acpi.c b/lib/efi_loader/efi_acpi.c
>>> index a62c34009cc..016bbf6db33 100644
>>> --- a/lib/efi_loader/efi_acpi.c
>>> +++ b/lib/efi_loader/efi_acpi.c
>>> @@ -34,7 +34,7 @@ efi_status_t efi_acpi_register(void)
>>>         * a 4k-aligned address, so it is safe to assume that
>>>         * write_acpi_tables() will write the table at that address.
>>>         */
>>> -     write_acpi_tables(acpi);
>>> +     write_acpi_tables((ulong)acpi);
>>
>> This is wrong: acpi is not an address in the virtual address space of
>> the sandbox. You must not pass it to use map_sysmem() to convert it to a
>> pointer.
>
> I think you are referring to the other patch.
>
> Here the problem is that a u64 is being passed to a ulong. As you say,
> this should be a global prototype and I will fix that in v2.
>
>>
>> Please change the parameter of write_acpi_tables() to be a pointer and
>> move all sandbox specific conversions to the sandbox code.
>
> Sorry, no, I don't want to do that as it would be a significant
> departure from how U-Boot currently works. We use ulong everywhere for
> addresses.

We typically use pointers to pass references to memory and not ulong.
Relative addresses in the virtual address space of the sandbox are only
relevant for testing on and user interaction with the sandbox.

The location of the memory location for the ACPI table is not determined
by the user.

Please, have a look at the called function where the sandbox address is
immediately converted back to a pointer. The parameter value is only
used in a debug statement. This double conversion makes is a waste of
memory and CPU cycles. Even for the debug output on the sandbox I would
go for a pointer because that is the value that I will need to
understand what is happening in the booted image.

Best regards

Heinrich

>
> Regards,
> Simon
>