Remove WARN_ONCE for unaligned UEFI region?

[Heinrich Schuchardt]

On 11/5/22 22:54, Ard Biesheuvel wrote:
> (cc Heinrich and Ilias)
> 
> On Sat, 5 Nov 2022 at 21:27, Linus Torvalds
> <torvalds at linux-foundation.org> wrote:
>>
>> On Sat, Nov 5, 2022 at 1:18 PM Ard Biesheuvel <ardb at kernel.org> wrote:
>>>
>>> Yeah just rip it out. In the beginning, we tended to make these
>>> warnings noisy so people will actually notice.
>>
>> Rip it out entirely, or replace ith pr_warn_once()?
>>
> 
> A warning that can only trigger on 16k or 64k page size kernels
> clearly doesn't have a lot of coverage, so either we just drop it, or
> we make the warning use SZ_64K and not PAGE_SIZE.
> 
> And if we keep the warning, it should be separate from the if(): when
> the regions are misaligned, we have to use RWX mappings because an
> adjacent region that gets covered by the same mapping might require
> it.
> 
> Maybe I'll just whip up a patch myself.
> 
>>> I'd still like to see a memory map (boot with efi=debug) so we can get
>>> this reported and fixed in uboot. We need that so 16k and 64k pages
>>> boot doesn't cause surprises with overlapping mappings.
>>
>> Here's the dmesg attached with efi=debug for your viewing pleasure.
>>
> 
> Thanks.
> 
> I've cc'ed the u-boot EFI maintainers, who take EFI spec compliance
> very seriously, so I'm sure we'll get this fixed quickly.

It seems the thread
https://lore.kernel.org/linux-efi/CAHk-=whPRmHQ=KV9B3_jeOG4ydj8gkMwQKnde7BJ4wJjveyMDQ@mail.gmail.com/
refers to:

arch/arm64/kernel/efi.c:28:
if (WARN_ONCE(!PAGE_ALIGNED(md->phys_addr),

Function create_mapping_protection() checks UEFI Permission Attributes 
of pages.

This is the related requirement in the UEFI 2.9 specification:

<cite>
The ARM architecture allows mapping pages at a variety of granularities, 
including 4KiB and 64KiB. If a 64KiB physical page contains any 4KiB 
page with any of the following types listed below, then all 4KiB pages 
in the 64KiB page must use identical ARM Memory Page Attributes (as 
described in Map EFI Cacheability Attributes to AArch64 Memory Types ):

- EfiRuntimeServicesCode
- EfiRuntimeServicesData
- EfiReserved
- EfiACPIMemoryNVS
</cite>

The specification only refers to EFI cacheability attributes but not to 
UEFI Permission Attributes. Should the specification be updated in this 
regard?

The next steps to take are:

1) clarify the ARM hardware requirements for UEFI Permission Attributes
2) update the UEFI specification to make it clear if UEFI Permission 
Attributes have to follow the 64 KiB rule.
3) update the UEFI Self-Certification Test (SCT) to check the rules.
4) update U-Boot to respect 64 KiB requirements when allocating new pages.

We already have commit 7a82c3051c8 ("efi_loader: Align runtime section 
to 64kb") in U-Boot but that neither covers runtime data nor allocation 
of memory (AllocatePages(), AllocatedPool()).

The warning in question indicates that Linux currently is not able to 
establish secure page attributes. I see no reason to remove it.

cc: U-Boot mailing list
cc: Edhaya Chandran (regarding UEFI SCT)

Best regards

Heinrich