[PATCHv3 0/5] use __create_pgd_mapping() to implement idmap and unify codes

[Pingfan Liu]

Hi Ard,

Thanks for your reviewing.

On Tue, Jun 1, 2021 at 3:50 AM Ard Biesheuvel <ardb at kernel.org> wrote:
>
> On Mon, 31 May 2021 at 10:46, Pingfan Liu <kernelfans at gmail.com> wrote:
> >
> > v2 -> v3:
> >   -1. leave out the part of redefinition the CONFIG_PGTABLE_LEVEL,
> > concentrate on sharing __create_pgd_mapping() in head.S as the first
> > step.
> >   -2. make IDMAP_PGTABLE_LEVELS use the max value ([3/5])
> >
> > rfc -> v2:
> >   more debug and test
> >
> > *** Goal of this series ***
> >
> > __create_pgd_mapping() sets up the pgtable for mapping __va(paddr) ->
> > paddr under the MMU-on situation.  Since pgtable upper level holds the
> > paddr of the lower level, with a slight adaptation,
> > __create_pgd_mapping() can also set up the mapping under the MMU-off
> > situation. ([4/5])
> >
> > After that, both idmap_pg_dir and init_pg_dir can be created by
> > __create_pgd_mapping(). And the counterpart asm code can be simplified.
> >
>
> I understand the desire to simplify the page table construction code
> in head.S, but up until now, we have been very careful to avoid
> calling into C code with the MMU off. There are simply too many
> assumptions in the compiler about the context the generated code will
> execute in: one example is unaligned access, which must be disabled
> for source files that may be called with the MMU off, as otherwise,
> the compiler is permitted to emit loads and stores that are not
> allowed on device memory (which is the default memory type used for
> all memory with the MMU off)
>
You are right. These C routines happen to use "unsigned long", which
can exclude this unaligned case.
To make an guarantee, is "-mno-unaligned-access" good enough?

Besides unaligned-access, any further risk originating from compiler
assumption? (I think that the common optimization: reordering,
merging, reloading on this "device" memory has no bad effect)

> Do you have a killer use case for this feature? Or is it just a nice cleanup?
>
Yes, in the omitted part in v2, I had planned to provide an unified
page table manipulation routine, and provide an create_idmap() API. So
there can be an handy interface to create a whole RAM addressable
idmapX where needed.

And three place can share the API create_idmap(): head.S,
trans_pgd_idmap_page(), kexec boot with mmu-on. As a result, most of
mm/trans_pgd.c can be cleaned after hibernation records paddr.

Thanks,

Pingfan
> > This series can be applied against commit 4284bdf9405a ("Linux 5.13-rc2").
> >
> >
> > *** Plan for the next ***
> >
> > The omitted part in V2, which resorts to redefinition of
> > CONFIG_PGTABLE_LEVEL to provides two sets of routines. One set is proper
> > to set up a idmap which can adress total system RAM.  That can simplify
> > the asm code in head.S furtherly, also provide an create_idmap() API.
> >
> >
> > *** Test ***
> >
> > This series can success booting with the following configuration on either Cavium
> > ThunderX2 99xx or Qualcomm Falkor:
> >             PAGE_SIZE  VA  PA  PGTABLE_LEVEL
> > Qualcomm    4K         48  48  4
> >             4K         39  48  3
> >             16K        48  48  4
> >             16K        47  48  3
> > Cavium      64K        52  52  3
> >             64K        48  52  3
> >             64K        42  52  2
> >
> >
> >
> > *** History ***
> >
> > RFC:
> > https://lore.kernel.org/linux-arm-kernel/20210410095654.24102-1-kernelfans@gmail.com/
> > V2:
> > https://lore.kernel.org/linux-arm-kernel/20210425141304.32721-1-kernelfans@gmail.com/
> >
> >
> > Cc: Catalin Marinas <catalin.marinas at arm.com>
> > Cc: Will Deacon <will at kernel.org>
> > Cc: Ard Biesheuvel <ardb at kernel.org>
> > Cc: Marc Zyngier <maz at kernel.org>
> > Cc: Kristina Martsenko <kristina.martsenko at arm.com>
> > Cc: James Morse <james.morse at arm.com>
> > Cc: Steven Price <steven.price at arm.com>
> > Cc: Jonathan Cameron <Jonathan.Cameron at huawei.com>
> > Cc: Pavel Tatashin <pasha.tatashin at soleen.com>
> > Cc: Anshuman Khandual <anshuman.khandual at arm.com>
> > Cc: Atish Patra <atish.patra at wdc.com>
> > Cc: Mike Rapoport <rppt at kernel.org>
> > Cc: Logan Gunthorpe <logang at deltatee.com>
> > Cc: Mark Brown <broonie at kernel.org>
> > To: linux-arm-kernel at lists.infradead.org
> >
> > Pingfan Liu (5):
> >   arm64/mm: introduce pgtable allocator for idmap_pg_dir and init_pg_dir
> >   arm64/mm: disable WARN_ON() and BUG_ON() in __create_pgd_mapping() if
> >     too early
> >   arm64/mm: unconditionally set IDMAP_PGTABLE_LEVELS to max pgtable
> >     level
> >   arm64/mm: make __create_pgd_mapping() capable to handle pgtable's
> >     paddr
> >   arm64/mm: use __create_pgd_mapping() to create pgtable for
> >     idmap_pg_dir and init_pg_dir
> >
> >  arch/arm64/include/asm/kernel-pgtable.h |  33 +++--
> >  arch/arm64/include/asm/pgalloc.h        |   9 ++
> >  arch/arm64/kernel/head.S                | 164 +++++++-----------------
> >  arch/arm64/mm/mmu.c                     | 108 ++++++++++++----
> >  4 files changed, 153 insertions(+), 161 deletions(-)
> >
> > --
> > 2.29.2
> >