[PATCH v2] arm64/kernel: Always use level 2 or higher for early mappings

[Ard Biesheuvel]

On Tue, 11 Mar 2025 at 12:31, Ryan Roberts <ryan.roberts at arm.com> wrote:
>
> On 11/03/2025 07:30, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb at kernel.org>
> >
> > The page table population code in map_range() uses a recursive algorithm
> > to create the early mappings of the kernel, the DTB and the ID mapped
> > text and data pages, and this fails to take into account that the way
> > these page tables may be constructed is not precisely the same at each
> > level. In particular, block mappings are not permitted at each level,
> > and the code as it exists today might inadvertently create such a
> > forbidden block mapping if it were used to map a region of the
> > appropriate size and alignment.
> >
> > This never happens in practice, given the limited size of the assets
> > being mapped by the early boot code. Nonetheless, it would be better if
> > this code would behave correctly in all circumstances.
> >
> > So only permit block mappings at level 2, and page mappings at level 3,
> > for any page size, and use table mappings exclusively at all other
> > levels. This change should have no impact in practice, but it makes the
> > code more robust.
> >
> > Cc: Anshuman Khandual <anshuman.khandual at arm.com>
> > Reported-by: Ryan Roberts <ryan.roberts at arm.com>
> > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> > ---
> > v2: take the assignment of protval out of the loop again, so that
> >     clearing a mapping works as expected wrt the PTE_CONT bit
>
> Ouch, totally missed that. Now that I've looked at the code properly, I wonder
> if it is worth supporting CONT_PMD mappings at level 2? That would be 32M for 4K
> pages, so is likely to actaually get used. I think it would just be something like:
>
> u64 cmask = (level == 3) ? CONT_PTE_SIZE - 1 :
>            ((level == 2) ? CONT_PMD_SIZE - 1 : U64_MAX) : U64_MAX;
>

With this patch applied, it is guaranteed that cmask is only
referenced if level >= 2, so we can simplify this as

  u64 cmask = (level < 3) ? CONT_PMD_SIZE - 1 : CONT_PTE_SIZE - 1;

(using the level < 3 for consistency with the protval assignment)

> Anyway, probably not worth the complexity for these boot-time tables, so regardless:
>

They are created at boot time but these are not 'boot-time tables' -
they are the ones that are used by the kernel at runtime. So I think
it makes sense to add this.

> Reviewed-by: Ryan Roberts <ryan.roberts at arm.com>
>

Thanks.