[PATCH] arm64: mm: Align PGDs to at least 64 bytes

[Ard Biesheuvel]

On Thu, 24 Nov 2022 at 05:34, Anshuman Khandual
<anshuman.khandual at arm.com> wrote:
>
>
>
> On 11/22/22 22:26, Ard Biesheuvel wrote:
> > My copy of the ARM ARM (DDI 0487G.a) no longer describes the 64 byte
> > minimum alignment of root page tables as being conditional on whether
> > 52-bit physical addressing is supported and enabled, even though I seem
> > to remember that this was the case formerly (and our code suggests the
> > same).
> ARM ARM (DDI 0487G.a) says,
>
> "The minimum alignment of a translation table containing fewer than eight
> entries is 64 bytes." But that does not seem to be conditional on 52-bit
> physical address support, unless only the 52 bit physical address space
> support could produce table configurations, which will have fewer than 8
> entries in the PGD level.
>

The architecture permits you to dimension the top level table freely,
so this could happen with smaller PA sizes too.

> >
> > So align pgd_t[] allocations to 64 bytes. Note that this only affects
> > 16k/4 levels configurations, which are unlikely to be in wide use.
>
> Instead of "16k/4 levels configurations", could we mention the in terms
> of [PG_SIZE/VA_BITS/PA_BITS] format which can be easily related with
> available config options.
>

"""
With 16k granule and 48 bits of VA space, the root level table is only
16 bytes in size (two entries), and so aligning to PGD_SIZE is
insufficient here.
"""

> >
> > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> > ---
> >  arch/arm64/mm/pgd.c | 13 +++----------
> >  1 file changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm64/mm/pgd.c b/arch/arm64/mm/pgd.c
> > index 4a64089e5771c1e2..8f01a75c35caaa9a 100644
> > --- a/arch/arm64/mm/pgd.c
> > +++ b/arch/arm64/mm/pgd.c
> > @@ -40,17 +40,10 @@ void __init pgtable_cache_init(void)
> >       if (PGD_SIZE == PAGE_SIZE)
> >               return;
> >
> > -#ifdef CONFIG_ARM64_PA_BITS_52
> >       /*
> > -      * With 52-bit physical addresses, the architecture requires the
> > -      * top-level table to be aligned to at least 64 bytes.
> > +      * Naturally aligned pgds required by the architecture, with a minimum
> > +      * alignment of 64 bytes.
> >        */
> > -     BUILD_BUG_ON(PGD_SIZE < 64);
> > -#endif
> > -
> > -     /*
> > -      * Naturally aligned pgds required by the architecture.
> > -      */
> > -     pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_SIZE,
> > +     pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, max(PGD_SIZE, 64),
> >                                     SLAB_PANIC, NULL);
>
> There is a build warning as follows, which can be fixed with typecasting
> constant 64 with (size_t). While here, it would also make sense to define
> a macro for PGD minimum alignment requirement i.e 64 bytes.
>

Hmm, I didn't spot this.

Maybe something like

#define TTBR_MIN_ALIGN 64UL

in the pgtable-hwdef header should do the trick?

> diff --git a/arch/arm64/mm/pgd.c b/arch/arm64/mm/pgd.c
> index 8f01a75c35ca..8d4b28d9590f 100644
> --- a/arch/arm64/mm/pgd.c
> +++ b/arch/arm64/mm/pgd.c
> @@ -44,6 +44,6 @@ void __init pgtable_cache_init(void)
>          * Naturally aligned pgds required by the architecture, with a minimum
>          * alignment of 64 bytes.
>          */
> -       pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, max(PGD_SIZE, 64),
> +       pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, max(PGD_SIZE, (size_t) 64),
>                                       SLAB_PANIC, NULL);
>  }
>
> In file included from ./include/linux/kernel.h:26,
>                  from ./arch/arm64/include/asm/cpufeature.h:22,
>                  from ./arch/arm64/include/asm/ptrace.h:11,
>                  from ./arch/arm64/include/asm/irqflags.h:10,
>                  from ./include/linux/irqflags.h:16,
>                  from ./include/linux/spinlock.h:59,
>                  from ./include/linux/mmzone.h:8,
>                  from ./include/linux/gfp.h:7,
>                  from ./include/linux/mm.h:7,
>                  from arch/arm64/mm/pgd.c:9:
> arch/arm64/mm/pgd.c: In function ‘pgtable_cache_init’:
> ./include/linux/minmax.h:20:28: warning: comparison of distinct pointer types lacks a cast
>    20 |  (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>       |                            ^~
> ./include/linux/minmax.h:26:4: note: in expansion of macro ‘__typecheck’
>    26 |   (__typecheck(x, y) && __no_side_effects(x, y))
>       |    ^~~~~~~~~~~
> ./include/linux/minmax.h:36:24: note: in expansion of macro ‘__safe_cmp’
>    36 |  __builtin_choose_expr(__safe_cmp(x, y), \
>       |                        ^~~~~~~~~~
> ./include/linux/minmax.h:52:19: note: in expansion of macro ‘__careful_cmp’
>    52 | #define max(x, y) __careful_cmp(x, y, >)
>       |                   ^~~~~~~~~~~~~
> arch/arm64/mm/pgd.c:47:55: note: in expansion of macro ‘max’
>    47 |  pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, max(PGD_SIZE, 64),
>
> >  }
>
> Besides, tested on config [16K|48VA|48PA] producing "CONFIG_PGTABLE_LEVELS=4"
> and with PGD_SIZE != PAGE_SIZE, which booted successfully.

Thanks!