From: "David Hildenbrand (Arm)" <[email protected]>
Using pmdp_get() and friends in common code on a kernel config with
folded page tables is suboptimal: they default to a READ_ONCE(), forcing
the compiler to actually read that value even though it will not actually
be used afterwards.
This was recently reported by Christophe Leroy [1] and block conversion
of more common code to pmdp_get() and friends.
(using pgdp_get() as one example)
Most of the code ignores the result from pgdp_get() on configs with
folded page tables entirely, as we hardcode:
pgd_present()==1 && pgd_leaf()==false
Common code will just treat it as a "this is a page table" and call
p4d_offset() or p4d_offset_lockless() for the next lower level, where
we just ignore the obtained pgdp_get() result entirely.
So we can just return a dummy value and avoid any memory reads.
There is a catch, though:
1) If code calls pgd_val() and somehow relies on the data, it would now
see dummy values. The code really must be aware of folded page table
levels. Fortunately, code usually ignores pgd_val() completely for
page tables (with ptdump being one exception when calculating
effective permissions). We checked + fixed the x86 ptdump mechanism.
2) If code passes the pgd_t to a function that would work on the result,
it would now see dummy values. The only concern is really passing
the pgd_t on the stack as a pointer to p4d_offset(). Most code that
would do that, should actually use p4d_offset_lockless(), which
handles this properly. We checked + fixed problematic instances.
As an example, this is the generated code for perf_get_page_size() with
PGTABLE_LEVELS=3 on arm64:
Before:
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() */
After:
0000000000052a0 <perf_get_page_size>:
...
52dc: d53b4234 mrs x20, DAIF
52e0: d50343df msr DAIFSet, #0x3
... /* no
pud_offset_lockless() and pudp_get() */
5318: 90000009 adrp x9, 0x5000 <perf_prepare_sample+0x548>
531c: 92746908 and x8, x8, #0x7ffffff000
5320: d3557675 ubfx x21, x19, #21, #9 /*
pmd_offset_lockless() */
...
5334: f8757ac8 ldr x8, [x22, x21, lsl #3] /* pmdp_get() */
[1] https://lore.kernel.org/all/[email protected]/
Signed-off-by: David Hildenbrand (Arm) <[email protected]>
---
include/asm-generic/pgtable-nop4d.h | 8 ++++++++
include/asm-generic/pgtable-nopmd.h | 8 ++++++++
include/asm-generic/pgtable-nopud.h | 8 ++++++++
3 files changed, 24 insertions(+)
diff --git a/include/asm-generic/pgtable-nop4d.h
b/include/asm-generic/pgtable-nop4d.h
index 019c3f074b77..ab4a826b3404 100644
--- a/include/asm-generic/pgtable-nop4d.h
+++ b/include/asm-generic/pgtable-nop4d.h
@@ -34,6 +34,14 @@ static inline bool pgd_leaf(pgd_t pgd) {
return false; }
*/
#define set_pgd(pgdptr, pgdval) set_p4d((p4d_t *)(pgdptr), (p4d_t) {
pgdval })
+static inline pgd_t pgdp_get(pgd_t *p4dp)
+{
+ pgd_t dummy = { 0 };
+
+ return dummy;
+}
+#define pgdp_get pgdp_get
+
static inline p4d_t *p4d_offset(pgd_t *pgd, unsigned long address)
{
return (p4d_t *)pgd;
diff --git a/include/asm-generic/pgtable-nopmd.h
b/include/asm-generic/pgtable-nopmd.h
index ae2eff44889a..711e12e5d180 100644
--- a/include/asm-generic/pgtable-nopmd.h
+++ b/include/asm-generic/pgtable-nopmd.h
@@ -44,6 +44,14 @@ static inline void pud_clear(pud_t *pud) { }
*/
#define set_pud(pudptr, pudval) set_pmd((pmd_t
*)(pudptr), (pmd_t) { pudval })
+static inline pud_t pudp_get(pud_t *pudp)
+{
+ pud_t dummy = { 0 };
+
+ return dummy;
+}
+#define pudp_get pudp_get
+
static inline pmd_t * pmd_offset(pud_t * pud, unsigned long address)
{
return (pmd_t *)pud;
diff --git a/include/asm-generic/pgtable-nopud.h
b/include/asm-generic/pgtable-nopud.h
index 5a2b0a81ae19..8f01abbb0050 100644
--- a/include/asm-generic/pgtable-nopud.h
+++ b/include/asm-generic/pgtable-nopud.h
@@ -41,6 +41,14 @@ static inline bool p4d_leaf(p4d_t p4d) {
return false; }
*/
#define set_p4d(p4dptr, p4dval) set_pud((pud_t *)(p4dptr), (pud_t) {
p4dval })
+static inline p4d_t p4dp_get(p4d_t *p4dp)
+{
+ p4d_t dummy = { 0 };
+
+ return dummy;
+}
+#define p4dp_get p4dp_get
+
static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)
{
return (pud_t *)p4d;
--
2.43.0
