Using ptep_get() and its counterparts in common code is suboptimal on
kernel configurations with generic compile-time folded page tables.
By default, ptep_get() and its friends expands to READ_ONCE(),
forcing the compiler to emit a load even when the value is not used afterwards.

This issue was recently reported by Christophe Leroy [1] for ppc32
preventing futher code conversion to ptep_get()/pmdp_get()/... helper
and the same behavior can also be observed on arm64 when built with
2- or 3-level page tables

e.g) perf_get_page_size() in arm64 with CONFIG_PGTABLE_LEVEL=3:

00000000000052a0 <perf_get_page_size>:
    ...
    52dc: d53b4234      mrs     x20, DAIF
    52e0: d50343df      msr     DAIFSet, #0x3
    ...
    52fc: d35e9a69      ubfx    x9, x19, #30, #9        /* 
pud_offset_lockless() */
    5300: f9403508      ldr     x8, [x8, #0x68]
    5304: f869790a      ldr     x10, [x8, x9, lsl #3]   /* pudp_get() */
    5308: f90007ea      str     x10, [sp, #0x8]
    530c: f8697908      ldr     x8, [x8, x9, lsl #3]    /* pudp_get() */
    ...
    5360: 90000009      adrp    x9, 0x5000 <perf_prepare_sample+0x548>
    5364: 92746908      and     x8, x8, #0x7ffffff000
    5368: d3557675      ubfx    x21, x19, #21, #9       /* 
pmd_offset_lockless() */
    ...
    5394: f8757ac8      ldr     x8, [x22, x21, lsl #3]  /* pmdp_get() */

Though PGTABLE_LEVEL=3, since the pudp_get() still remain with
READ_ONCE(), there's redundant load for the pud which is folded.

To prevent generating suboptimal code, make pXdp_get() return a dummy
entry for compile-time folded page tables and prohibit calls to
pXd_page() in pgtable-nopXd.h.

Also make the helpers in pgtable-nopXd.h, such as pXd_offset_lockless(),
validate dummy entries at compile time to catch the wrong usage.

As the pXdp_get() can return *dummy* entry, some of code using
the stack value where saves the pXdp_get() could be a problematic:

  1. Passing address of stack value where saves the pXdp_get() result
     to pXd_offset() for example:

       pud_t *pudp, pud;
       pmd_t *pmdp;

       pud = pudp_get(pudp, address);
       pmdp = pmd_offset(&pud, pud, address);

     (e.g. host_pfn_mapping_level() in loongarch).

  2. Using the pXdp_get() result to use as argument of pXd_val() and
     to check prot without checking pgtable is folded.
     for example, x86's effective_prot().

  3. Using set_pXd() with pXdp_get() will set problematic dummy entry
     in folded page table like:

       set_pXd(pxdp, pXdp_get(pxdp_k));

  4. Using pgd_page_vaddr() to get the first-level pgtable.
     passing dummy pxdp_get() for pgd_page_vaddr() will return wrong
     address. Therefore, make pgd_page_vaddr() and pXd_pgtable() to
     trigger the error for improper usage with folded dummy entry in the
     generic compile-time folded pgtable.

Thanksfully, above cases are rare since (1) most of usage using
pXd_offset() with result of upper pXd_offset(), (2) it's extreamely
rare to use pXd_val() for non-leaf entry in the kernel,
(3) is to handle the vmalloc_fault or set the first level of page table
and (4) to setup early page table and etc.

Therefore, convert this kind of problematic rare pattern properly.

Furthermore, passing the ptep_get() (or its counterparts) as argument
directly to pte_present() and related helpers can generate suboptimal code,
in arm64 as the current macro implementation may evaluate its argument
more than once like:

  !pte_val(READ_ONCE(*pte) || pte_present_invalid(READ_ONCE(*pte))

resulting in redundant loads.

A typical example is pud_free_pmd_page(), where the expansion of
pmd_present() generates:
    ...
    /* pmd_present() (x20 = pmdp) */
    1b88: f9400288     ldr      x8, [x20]        // read pmdp.
    1b8c: f9000fa8     str      x8, [x29, #0x18]
    1b90: 3707fec8     tbnz     w8, #0x0, 0x1b68 <pud_free_pmd_page+0xd0>
    1b94: f9400288     ldr      x8, [x20]        // redundant read of pmdp.
    1b98: 8a170109     and      x9, x8, x23
    1b9c: f9000fa8     str      x8, [x29, #0x18]
    1ba0: f120013f     cmp      x9, #0x800
    1ba4: 54fffe20     b.eq     0x1b68 <pud_free_pmd_page+0xd0>
    1ba8: 17fffff4     b        0x1b78 <pud_free_pmd_page+0xe0>
    ...

To address this in arm64, convert pte_present() macro and its friends
to static inline function.

This patch is based on mm-unstable.

Future work
===========
 - print_bad_page_map() and show_pte() still prints dummy values
   instead of printing the same content for all generic compile-time
   folded page tables. We might want to skip printing dummy values later.

 - We currently catch abuse of dummy values on the stack at compile-time by
   relying on constant propagation by the compiler. Usama's work [3] on using
   distinct types for sw vs. hw PTEs could help here as well."

Patch History
=============
from v2 to v3:
  - repasre commit message
  - move ptdump_pgtable_first_level() into pgalloc.h
  - skip the huge pud operation when CONFIG_X86_DIRECT_GBPAGES is
    disabled
  - https://lore.kernel.org/all/[email protected]/

from v1 to v2:
  - Restore slient fallback to next pXd in set_pXd() and pXd_pgtable()
    and add check whether they're called with dummy entry.
  - Add some comment for returning first entry of pgd in arm with
    2 pgtable-level
  - https://lore.kernel.org/all/[email protected]/

To: Russell King <[email protected]>
To: Huacai Chen <[email protected]>
To: WANG Xuerui <[email protected]>
To: Thomas Bogendoerfer <[email protected]>
To: Catalin Marinas <[email protected]>
To: Will Deacon <[email protected]>
To: Arnd Bergmann <[email protected]>
To: Andrew Morton <[email protected]>
To: Kairui Song <[email protected]>
To: Qi Zheng <[email protected]>
To: Shakeel Butt <[email protected]>
To: Barry Song <[email protected]>
To: Axel Rasmussen <[email protected]>
To: Yuanchu Xie <[email protected]>
To: Wei Xu <[email protected]>
To: Johannes Weiner <[email protected]>
To: David Hildenbrand <[email protected]>
To: Michal Hocko <[email protected]>
To: Lorenzo Stoakes <[email protected]>
To: Tianrui Zhao <[email protected]>
To: Bibo Mao <[email protected]>
To: Anup Patel <[email protected]>
To: Atish Patra <[email protected]>
To: Paul Walmsley <[email protected]>
To: Palmer Dabbelt <[email protected]>
To: Albert Ou <[email protected]>
To: Alexandre Ghiti <[email protected]>
To: Dave Hansen <[email protected]>
To: Andy Lutomirski <[email protected]>
To: Peter Zijlstra <[email protected]>
To: Thomas Gleixner <[email protected]>
To: Ingo Molnar <[email protected]>
To: Borislav Petkov <[email protected]>
To: [email protected]
To: H. Peter Anvin <[email protected]>
To: Liam R. Howlett <[email protected]>
To: Vlastimil Babka <[email protected]>
To: Mike Rapoport <[email protected]>
To: Suren Baghdasaryan <[email protected]>
To: Michal Hocko <[email protected]>
To: Jonas Bonn <[email protected]>
To: Stefan Kristiansson <[email protected]>
To: Stafford Horne <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: [1] 
https://lore.kernel.org/all/[email protected]/
Link: [2] 
https://lore.kernel.org/all/[email protected]/
Link: [3] https://lore.kernel.org/r/[email protected]

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 3,
    "change-id": "20260722-dummy_ptxp3-3741d78cc70f",
    "prefixes": [
      "RFC"
    ],
    "history": {
      "v2": [
        "[email protected]"
      ]
    }
  }
}

Signed-off-by: Yeoreum Yun <[email protected]>
-- 
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}


Reply via email to