Re: [PATCH V2] arm64/mm: Create level specific section mappings in map_range()

[Ard Biesheuvel]

On Mon, 10 Mar 2025 at 15:42, Mark Rutland <mark.rutland@xxxxxxx> wrote:
>
> On Mon, Mar 10, 2025 at 01:48:53PM +0000, Ryan Roberts wrote:
> > On 10/03/2025 10:55, Mark Rutland wrote:
> > > On Mon, Mar 10, 2025 at 11:58:12AM +0530, Anshuman Khandual wrote:
> > >> Currently PMD section mapping mask i.e PMD_TYPE_SECT is used while creating
> > >> section mapping at all page table levels except the last level. This works
> > >> fine as the section mapping masks are exactly the same (0x1UL) for all page
> > >> table levels.
> > >>
> > >> This will change in the future with D128 page tables that have unique skip
> > >> level values (SKL) required for creating section mapping at different page
> > >> table levels. Hence use page table level specific section mapping macros
> > >> instead of the common PMD_TYPE_SECT. While here also ensure that a section
> > >> mapping is only created on page table levels which could support that on a
> > >> given page size configuration otherwise fall back to create table entries.
> > >>
> > >> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> > >> Cc: Will Deacon <will@xxxxxxxxxx>
> > >> Cc: Ryan Roberts <ryan.roberts@xxxxxxx>
> > >> Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > >> Cc: linux-kernel@xxxxxxxxxxxxxxx
> > >> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > >> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> > >> ---
> > >> This patch applies on v6.14-rc6
> > >>
> > >> Changes in V2:
> > >>
> > >> - Dropped PGD_TYPE_SECT macro and its instance from map_range()
> > >> - Create table entries on levels where section mapping is not possible
> > >
> > > On the last version, there was talk of an extant bug:
> > >
> > >   https://lore.kernel.org/all/5f1b36e2-6455-44d9-97b0-253aefd5024f@xxxxxxx/
> > >
> > > ... did that turn out to not be the case?
> >
> > In the absence of Ard (or anyone else) telling me it's not a bug, then I'll
> > continue to assert that it is a bug, and should have a Fixes tag, and Cc stable.
>
> Ok -- can we please have a description of the bug, and a fix
> specifically for that, before we try to reshape this for D128?
>
> IIUC the problem is that for a sufficiently large (and aligned) extent
> of physical memory, we might try to create a block mapping larger than
> the HW supports?
>

Yes. However, this code is only called to map the kernel, the DTB and
the ID map code and data pages, and so it seems unlikely that we'll
hit this bug in practice.

Nonetheless, it is a shortcoming that we should address to avoid
future surprises.

> With that in mind, my earlier comment about LPA2 and blocks at level-0
> is particularly relevant...
>

Indeed - at the higher levels, there is quite some variation between
page sizes, LPA2 vs !LPA2 etc in which level is the highest at which
block mappings are supported.

But given the context where this code is used, I'd prefer to simply
map everything down to level 2 or higher (for 64-bit descriptors)
rather than add elaborate rules for all these combinations that are
never exercised in practice. The net result should be identical.

...
>
> Though TBH, maybe it makes more sense to split this up into separate
> levels (as with the code in arch/arm64/mm/mmu.c) and handle each level
> explicitly with a separate function. That way each level's check can be
> handled within that level's function, and we can use the right types,
> etc. It feels like that might be better for D128 in future?
>

I have a slight inclination towards having a separate routine for
D128, but I can easily be persuaded otherwise.