[U-Boot] Crash in U-Boot

[Heinrich Schuchardt]

On 6/24/19 7:56 AM, Alexander Graf wrote:
>
>
>> Am 24.06.2019 um 00:01 schrieb Ard Biesheuvel <ard.biesheuvel at linaro.org>:
>>
>>> On Sun, 23 Jun 2019 at 23:47, Alexander Graf <agraf at csgraf.de> wrote:
>>>
>>>
>>>
>>>> Am 23.06.2019 um 17:08 schrieb Heinrich Schuchardt <xypron.glpk at gmx.de>:
>>>>
>>>> Hello Alex,
>>>>
>>>> on the i.MX6 Wandboard the Kernel crashes when booting via GRUB at least
>>>> since U-Boot v2019.01. See below.
>>>>
>>>> In the code for SetVirtualAddressMap I saw the that the pointer to the
>>>> list of configuration tables is adjusted:
>>>>
>>>>         if ((map_start <= (uintptr_t)systab.tables) &&
>>>>             (map_end >= (uintptr_t)systab.tables)) {
>>>>                 char *ptr = (char *)systab.tables;
>>>>                 ptr += off;
>>>>                 systab.tables = (struct efi_configuration_table *)ptr;
>>>>         }
>>>>
>>>> Shouldn't the pointers to the individual configuration tables be
>>>> adjusted too? I found no such code.
>>>
>>> We have to adapt the systable, because RT code may use it. However, I thought tables are not guaranteed to be around after SVAM? Ard?
>>>
>>
>> Only the system table pointers are updated for virtual remapping. I'm
>> not entirely sure why, but this permits the runtime firmware
>> components themselves to access the array. However, runtime firmware
>> typically has no way to translate a physical address into a virtual
>> address, so if they need to access such tables, they have to grab the
>> address *before* SVAM(), which means it doesn't matter whether the
>> config table array point is translated as well.
>>
>>> I tend to agree that in that case, adjusting the table pointer does not sound very useful either. Can we legally just set the table count to 0 there?
>>>
>>
>> No. The OS expects to be able to locate config tables, and the OS
>> itself does not care about runtime remapping.
>>
>>> If however they are indeed legal to access via virtual addresses afterwards, I do agree that we should patch the individual table pointers too, yes.
>>>
>>>
>>
>> Please don't do the sane thing. Where's the fun in that? :-)
>
> I keep forgetting that we talk UEFI here ;). So the status quo is correct? Or do we have to drop the relocation part for the table array pointer?

EDK2 is doing it just like U-Boot. Change the pointer to the array. Do
not change the array.

Regards

Heinrich