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

From: Marc Zyngier <maz@kernel.org>
To: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	linux-arm-kernel@lists.infradead.org, mark.rutland@arm.com
Subject: Re: [PATCH] arm64: mm: Align PGDs to at least 64 bytes
Date: Tue, 29 Nov 2022 12:54:14 +0000	[thread overview]
Message-ID: <86fse1ncp5.wl-maz@kernel.org> (raw)
In-Reply-To: <20221129122259.GB25960@willie-the-truck>

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