Re: [PATCH 2/2] arm64/efi: Don't pad between EFI_MEMORY_RUNTIME regions

[Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]

On 25 September 2015 at 23:01, Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> * Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
>
>> From: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> The new Properties Table feature introduced in UEFIv2.5 may split
>> memory regions that cover PE/COFF memory images into separate code
>> and data regions. Since these regions only differ in the type (runtime
>> code vs runtime data) and the permission bits, but not in the memory
>> type attributes (UC/WC/WT/WB), the spec does not require them to be
>> aligned to 64 KB.
>>
>> Since the relative offset of PE/COFF .text and .data segments cannot
>> be changed on the fly, this means that we can no longer pad out those
>> regions to be mappable using 64 KB pages.
>> Unfortunately, there is no annotation in the UEFI memory map that
>> identifies data regions that were split off from a code region, so we
>> must apply this logic to all adjacent runtime regions whose attributes
>> only differ in the permission bits.
>>
>> So instead of rounding each memory region to 64 KB alignment at both
>> ends, only round down regions that are not directly preceded by another
>> runtime region with the same type attributes. Since the UEFI spec does
>> not mandate that the memory map be sorted, this means we also need to
>> sort it first.
>
> So I think this is fundamentally wrong as well, similarly to the related x86 fix.
>
> I think for compatibility reasons the whole 'EFI runtime image' should be mapped
> in a single go, as closely matching the EFI layouts and offsets as possible. We
> are not talking about gigabytes here, right?
>

As I explained in the other thread, this is really not necessary, and
never has been until the firmware started splitting up PE/COFF images
into several sections each. As long as we keep those PE/COFF images
together, everything will work as before, and the only complication is
that the memory map does not contain any clues about which regions
belong to a single PE/COFF image, so we need to keep all adjacent
EFI_MEMORY_RUNTIME regions adjacent in the VA mapping.

> Even if technically they are 'separate sections', the x86 bug shows that they
> aren't. So we should not pretend so on the Linux side either and we should not
> tear them apart (and then work hard to preserve the interdependencies, some
> declared, some hidden!).
>

This is about relocations and interdependencies at the symbol level,
and such interdependencies only exist internally inside PE/COFF
images.

> If we allocate the EFI runtime as a single virtual memory block then issues like
> rounding between sections does not even come up as a problem: we map the original
> offsets and sizes byte by byte.
>

Well, by that reasoning, we should not call SetVirtualAddressMap() in
the first place, and just use the 1:1 mapping UEFI uses natively. This
is more than feasible on arm64, and I actually fought hard against
using SetVirtualAddressMap() at all, but I was overruled by others. I
think this is also trivially possible on X64, since the 1:1 mapping is
already active alongside the VA mapping.

-- 
Ard.