* [PATCH] arm64: mm: Align PGDs to at least 64 bytes @ 2022-11-22 16:56 Ard Biesheuvel 2022-11-24 4:34 ` Anshuman Khandual 2022-11-28 17:50 ` Catalin Marinas 0 siblings, 2 replies; 10+ messages in thread From: Ard Biesheuvel @ 2022-11-22 16:56 UTC (permalink / raw) To: linux-arm-kernel; +Cc: will, catalin.marinas, Ard Biesheuvel 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). 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. Signed-off-by: Ard Biesheuvel <ardb@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); } -- 2.35.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: Align PGDs to at least 64 bytes 2022-11-22 16:56 [PATCH] arm64: mm: Align PGDs to at least 64 bytes Ard Biesheuvel @ 2022-11-24 4:34 ` Anshuman Khandual 2022-11-24 7:42 ` Ard Biesheuvel 2022-11-28 17:50 ` Catalin Marinas 1 sibling, 1 reply; 10+ messages in thread From: Anshuman Khandual @ 2022-11-24 4:34 UTC (permalink / raw) To: Ard Biesheuvel, linux-arm-kernel; +Cc: will, catalin.marinas 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. > > 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. > > Signed-off-by: Ard Biesheuvel <ardb@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. 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. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: Align PGDs to at least 64 bytes 2022-11-24 4:34 ` Anshuman Khandual @ 2022-11-24 7:42 ` Ard Biesheuvel 2022-11-24 11:56 ` Anshuman Khandual 0 siblings, 1 reply; 10+ messages in thread From: Ard Biesheuvel @ 2022-11-24 7:42 UTC (permalink / raw) To: Anshuman Khandual; +Cc: linux-arm-kernel, will, catalin.marinas On Thu, 24 Nov 2022 at 05:34, Anshuman Khandual <anshuman.khandual@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@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! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: Align PGDs to at least 64 bytes 2022-11-24 7:42 ` Ard Biesheuvel @ 2022-11-24 11:56 ` Anshuman Khandual 0 siblings, 0 replies; 10+ messages in thread From: Anshuman Khandual @ 2022-11-24 11:56 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-arm-kernel, will, catalin.marinas On 11/24/22 13:12, Ard Biesheuvel wrote: > On Thu, 24 Nov 2022 at 05:34, Anshuman Khandual > <anshuman.khandual@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. Okay. > >>> >>> 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. > """ Sounds good. > >>> >>> Signed-off-by: Ard Biesheuvel <ardb@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? Right, makes sense. Please add this macro. > >> 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! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: Align PGDs to at least 64 bytes 2022-11-22 16:56 [PATCH] arm64: mm: Align PGDs to at least 64 bytes Ard Biesheuvel 2022-11-24 4:34 ` Anshuman Khandual @ 2022-11-28 17:50 ` Catalin Marinas 2022-11-28 17:54 ` Ard Biesheuvel 2022-11-29 9:51 ` Will Deacon 1 sibling, 2 replies; 10+ messages in thread From: Catalin Marinas @ 2022-11-28 17:50 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-arm-kernel, will On Tue, Nov 22, 2022 at 05:56:18PM +0100, Ard Biesheuvel wrote: > My copy of the ARM ARM (DDI 0487G.a) no longer describes the 64 byte G.a is nearly two years old. You may want to upgrade to H.a ;). > 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). The wording in the ARM ARM implies that it's only needed if we go beyond 48 bits for the base address: A translation table must be aligned to the size of the table, except that when using a translation table base address larger than 48 bits the minimum alignment of a table containing fewer than eight entries is 64 bytes. But I'm fine with the patch, always forcing the 64 byte alignment. With the 'max_t' instead of 'max' (or whatever solves Anshuman's error): Acked-by: Catalin Marinas <catalin.marinas@arm.com> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: Align PGDs to at least 64 bytes 2022-11-28 17:50 ` Catalin Marinas @ 2022-11-28 17:54 ` Ard Biesheuvel 2022-11-29 9:51 ` Will Deacon 1 sibling, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2022-11-28 17:54 UTC (permalink / raw) To: Catalin Marinas; +Cc: linux-arm-kernel, will On Mon, 28 Nov 2022 at 18:50, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Tue, Nov 22, 2022 at 05:56:18PM +0100, Ard Biesheuvel wrote: > > My copy of the ARM ARM (DDI 0487G.a) no longer describes the 64 byte > > G.a is nearly two years old. You may want to upgrade to H.a ;). > > > 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). > > The wording in the ARM ARM implies that it's only needed if we go beyond > 48 bits for the base address: > > A translation table must be aligned to the size of the table, except > that when using a translation table base address larger than 48 bits > the minimum alignment of a table containing fewer than eight entries > is 64 bytes. > > But I'm fine with the patch, always forcing the 64 byte alignment. With > the 'max_t' instead of 'max' (or whatever solves Anshuman's error): > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> Right, so it appears they backpedaled on that. The distinction is kind of important depending on whether we want to fall back to 47 or 48 bits of VA space on on 16k granule configs with LPA2 running on systems that lack LPA2 support. If we want to fall back to 48 bits, TTBR1 must be incremented to point to the entries describing the 48-bit VA space inside a table dimensioned for 52 bits, and in this case, those entries can simply not appear on a 64 byte aligned boundary. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: Align PGDs to at least 64 bytes 2022-11-28 17:50 ` Catalin Marinas 2022-11-28 17:54 ` Ard Biesheuvel @ 2022-11-29 9:51 ` Will Deacon 2022-11-29 11:18 ` Ard Biesheuvel 1 sibling, 1 reply; 10+ messages in thread From: Will Deacon @ 2022-11-29 9:51 UTC (permalink / raw) To: Catalin Marinas; +Cc: Ard Biesheuvel, linux-arm-kernel On Mon, Nov 28, 2022 at 05:50:48PM +0000, Catalin Marinas wrote: > On Tue, Nov 22, 2022 at 05:56:18PM +0100, Ard Biesheuvel wrote: > > My copy of the ARM ARM (DDI 0487G.a) no longer describes the 64 byte > > G.a is nearly two years old. You may want to upgrade to H.a ;). H.a is over eight months old. You may want to upgrade to I.a :p (Actually, don't bother -- it's written using these unreadable rule things. H.a, all is forgiven). > > 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). > > The wording in the ARM ARM implies that it's only needed if we go beyond > 48 bits for the base address: > > A translation table must be aligned to the size of the table, except > that when using a translation table base address larger than 48 bits > the minimum alignment of a table containing fewer than eight entries > is 64 bytes. FWIW, this wording is the same in I.a. > But I'm fine with the patch, always forcing the 64 byte alignment. With > the 'max_t' instead of 'max' (or whatever solves Anshuman's error): > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> Happy to take a v2 or add the max_t() to this version. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: Align PGDs to at least 64 bytes 2022-11-29 9:51 ` Will Deacon @ 2022-11-29 11:18 ` Ard Biesheuvel 2022-11-29 12:23 ` Will Deacon 0 siblings, 1 reply; 10+ messages in thread From: Ard Biesheuvel @ 2022-11-29 11:18 UTC (permalink / raw) To: Will Deacon; +Cc: Catalin Marinas, linux-arm-kernel On Tue, 29 Nov 2022 at 10:51, Will Deacon <will@kernel.org> wrote: > > On Mon, Nov 28, 2022 at 05:50:48PM +0000, Catalin Marinas wrote: > > On Tue, Nov 22, 2022 at 05:56:18PM +0100, Ard Biesheuvel wrote: > > > My copy of the ARM ARM (DDI 0487G.a) no longer describes the 64 byte > > > > G.a is nearly two years old. You may want to upgrade to H.a ;). > > H.a is over eight months old. You may want to upgrade to I.a :p > > (Actually, don't bother -- it's written using these unreadable rule things. > H.a, all is forgiven). > > > > 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). > > > > The wording in the ARM ARM implies that it's only needed if we go beyond > > 48 bits for the base address: > > > > A translation table must be aligned to the size of the table, except > > that when using a translation table base address larger than 48 bits > > the minimum alignment of a table containing fewer than eight entries > > is 64 bytes. > > FWIW, this wording is the same in I.a. > > > But I'm fine with the patch, always forcing the 64 byte alignment. With > > the 'max_t' instead of 'max' (or whatever solves Anshuman's error): > > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > > Happy to take a v2 or add the max_t() to this version. > In spite of the off-list discussion we just had where we concluded that this patch is not necessary, I think we still do: in revision I.a, I still see the following wording D17.2.147 TTBR1_EL1, Translation Table Base Register 1 (EL1) ------- Note -------- A translation table is required to be aligned to the size of the table. If a table contains fewer than eight entries, it must be aligned on a 64 byte address boundary. with no mention whatsoever regarding this requirement being conditional on the configured PA range. I'll respin and resend. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: Align PGDs to at least 64 bytes 2022-11-29 11:18 ` Ard Biesheuvel @ 2022-11-29 12:23 ` Will Deacon 2022-11-29 12:54 ` Marc Zyngier 0 siblings, 1 reply; 10+ messages in thread From: Will Deacon @ 2022-11-29 12:23 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: Catalin Marinas, linux-arm-kernel, mark.rutland, maz On Tue, Nov 29, 2022 at 12:18:20PM +0100, Ard Biesheuvel wrote: > On Tue, 29 Nov 2022 at 10:51, Will Deacon <will@kernel.org> wrote: > > > > On Mon, Nov 28, 2022 at 05:50:48PM +0000, Catalin Marinas wrote: > > > On Tue, Nov 22, 2022 at 05:56:18PM +0100, Ard Biesheuvel wrote: > > > > My copy of the ARM ARM (DDI 0487G.a) no longer describes the 64 byte > > > > > > G.a is nearly two years old. You may want to upgrade to H.a ;). > > > > H.a is over eight months old. You may want to upgrade to I.a :p > > > > (Actually, don't bother -- it's written using these unreadable rule things. > > H.a, all is forgiven). > > > > > > 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). > > > > > > The wording in the ARM ARM implies that it's only needed if we go beyond > > > 48 bits for the base address: > > > > > > A translation table must be aligned to the size of the table, except > > > that when using a translation table base address larger than 48 bits > > > the minimum alignment of a table containing fewer than eight entries > > > is 64 bytes. > > > > FWIW, this wording is the same in I.a. > > > > > But I'm fine with the patch, always forcing the 64 byte alignment. With > > > the 'max_t' instead of 'max' (or whatever solves Anshuman's error): > > > > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > > > > Happy to take a v2 or add the max_t() to this version. > > > > In spite of the off-list discussion we just had where we concluded > that this patch is not necessary, I think we still do: > in revision I.a, I still see the following wording > > D17.2.147 TTBR1_EL1, Translation Table Base Register 1 (EL1) > > ------- Note -------- > A translation table is required to be aligned to the size of the > table. If a table contains fewer than > eight entries, it must be aligned on a 64 byte address boundary. > > > with no mention whatsoever regarding this requirement being > conditional on the configured PA range. Ha, so the text is different between stage-1 (e.g. TTBRx_EL1) and stage-2 (e.g. VTTBR_EL2)! I wonder if that's deliberate? Maybe something to do with coalescing? :/ Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: mm: Align PGDs to at least 64 bytes 2022-11-29 12:23 ` Will Deacon @ 2022-11-29 12:54 ` Marc Zyngier 0 siblings, 0 replies; 10+ messages in thread From: Marc Zyngier @ 2022-11-29 12:54 UTC (permalink / raw) To: Will Deacon Cc: Ard Biesheuvel, Catalin Marinas, linux-arm-kernel, mark.rutland On Tue, 29 Nov 2022 12:23:00 +0000, Will Deacon <will@kernel.org> wrote: > > On Tue, Nov 29, 2022 at 12:18:20PM +0100, Ard Biesheuvel wrote: > > On Tue, 29 Nov 2022 at 10:51, Will Deacon <will@kernel.org> wrote: > > > > > > On Mon, Nov 28, 2022 at 05:50:48PM +0000, Catalin Marinas wrote: > > > > On Tue, Nov 22, 2022 at 05:56:18PM +0100, Ard Biesheuvel wrote: > > > > > My copy of the ARM ARM (DDI 0487G.a) no longer describes the 64 byte > > > > > > > > G.a is nearly two years old. You may want to upgrade to H.a ;). > > > > > > H.a is over eight months old. You may want to upgrade to I.a :p > > > > > > (Actually, don't bother -- it's written using these unreadable rule things. > > > H.a, all is forgiven). > > > > > > > > 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). > > > > > > > > The wording in the ARM ARM implies that it's only needed if we go beyond > > > > 48 bits for the base address: > > > > > > > > A translation table must be aligned to the size of the table, except > > > > that when using a translation table base address larger than 48 bits > > > > the minimum alignment of a table containing fewer than eight entries > > > > is 64 bytes. > > > > > > FWIW, this wording is the same in I.a. > > > > > > > But I'm fine with the patch, always forcing the 64 byte alignment. With > > > > the 'max_t' instead of 'max' (or whatever solves Anshuman's error): > > > > > > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > > > > > > Happy to take a v2 or add the max_t() to this version. > > > > > > > In spite of the off-list discussion we just had where we concluded > > that this patch is not necessary, I think we still do: > > in revision I.a, I still see the following wording > > > > D17.2.147 TTBR1_EL1, Translation Table Base Register 1 (EL1) > > > > ------- Note -------- > > A translation table is required to be aligned to the size of the > > table. If a table contains fewer than > > eight entries, it must be aligned on a 64 byte address boundary. > > > > > > with no mention whatsoever regarding this requirement being > > conditional on the configured PA range. > > Ha, so the text is different between stage-1 (e.g. TTBRx_EL1) and stage-2 > (e.g. VTTBR_EL2)! I wonder if that's deliberate? Maybe something to do with > coalescing? :/ I think the whole VTTBR_EL2.BADDR section is full of crap, and has been for a long time (since 0487B.a). It talks about S1 translation all over the shop, and feels like a copy-paste gone horribly wrong... Coalescing actually forces a stronger alignment, as you have to align on the full size of the top-level table. M. /puzzled -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-11-29 12:55 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-22 16:56 [PATCH] arm64: mm: Align PGDs to at least 64 bytes Ard Biesheuvel 2022-11-24 4:34 ` Anshuman Khandual 2022-11-24 7:42 ` Ard Biesheuvel 2022-11-24 11:56 ` Anshuman Khandual 2022-11-28 17:50 ` Catalin Marinas 2022-11-28 17:54 ` Ard Biesheuvel 2022-11-29 9:51 ` Will Deacon 2022-11-29 11:18 ` Ard Biesheuvel 2022-11-29 12:23 ` Will Deacon 2022-11-29 12:54 ` Marc Zyngier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).