[PATCH v3 0/2] use static key to optimize pgtable_l4_enabled

8 views
Skip to first unread message

Jisheng Zhang

unread,
May 19, 2022, 12:07:58 PM5/19/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Anup Patel, Atish Patra, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
The pgtable_l4|[l5]_enabled check sits at hot code path, performance
is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
boot, so static key can be used to solve the issue[1].

An unified way static key was introduced in [2], but it's only targets
riscv isa extension. We dunno whether SV48 and SV57 will be considered
as isa extension, so the unified solution isn't used for
pgtable_l4[l5]_enabled.

patch1 fix a NULL pointer deference if static key is used a bit earlier.
patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.

[1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
[2] https://lore.kernel.org/linux-riscv/20220517184453...@kernel.org/T/#t

Since v2:
- move the W=1 warning fix to a separate patch
- move the unified way to use static key to a new patch series.

Since v1:
- Add a W=1 warning fix
- Fix W=1 error
- Based on v5.18-rcN, since SV57 support is added, so convert
pgtable_l5_enabled as well.


Jisheng Zhang (2):
riscv: move sbi_init() earlier before jump_label_init()
riscv: turn pgtable_l4|[l5]_enabled to static key for RV64

arch/riscv/include/asm/pgalloc.h | 16 ++++----
arch/riscv/include/asm/pgtable-32.h | 3 ++
arch/riscv/include/asm/pgtable-64.h | 59 +++++++++++++++++---------
arch/riscv/include/asm/pgtable.h | 5 +--
arch/riscv/kernel/cpu.c | 4 +-
arch/riscv/kernel/setup.c | 2 +-
arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
arch/riscv/mm/kasan_init.c | 16 ++++----
8 files changed, 103 insertions(+), 66 deletions(-)

--
2.34.1

Jisheng Zhang

unread,
May 19, 2022, 12:08:05 PM5/19/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Anup Patel, Atish Patra, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
We call jump_label_init() in setup_arch() is to use static key
mechanism earlier, but riscv jump label relies on the sbi functions,
If we enable static key before sbi_init(), the code path looks like:
static_branch_enable()
..
arch_jump_label_transform()
patch_text_nosync()
flush_icache_range()
flush_icache_all()
sbi_remote_fence_i() for CONFIG_RISCV_SBI case
__sbi_rfence()

Since sbi isn't initialized, so NULL deference! Here is a typical
panic log:

[ 0.000000] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[ 0.000000] Oops [#1]
[ 0.000000] Modules linked in:
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.18.0-rc7+ #79
[ 0.000000] Hardware name: riscv-virtio,qemu (DT)
[ 0.000000] epc : 0x0
[ 0.000000] ra : sbi_remote_fence_i+0x1e/0x26
[ 0.000000] epc : 0000000000000000 ra : ffffffff80005826 sp : ffffffff80c03d50
[ 0.000000] gp : ffffffff80ca6178 tp : ffffffff80c0ad80 t0 : 6200000000000000
[ 0.000000] t1 : 0000000000000000 t2 : 62203a6b746e6972 s0 : ffffffff80c03d60
[ 0.000000] s1 : ffffffff80001af6 a0 : 0000000000000000 a1 : 0000000000000000
[ 0.000000] a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
[ 0.000000] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000080200
[ 0.000000] s2 : ffffffff808b3e48 s3 : ffffffff808bf698 s4 : ffffffff80cb2818
[ 0.000000] s5 : 0000000000000001 s6 : ffffffff80c9c345 s7 : ffffffff80895aa0
[ 0.000000] s8 : 0000000000000001 s9 : 000000000000007f s10: 0000000000000000
[ 0.000000] s11: 0000000000000000 t3 : ffffffff80824d08 t4 : 0000000000000022
[ 0.000000] t5 : 000000000000003d t6 : 0000000000000000
[ 0.000000] status: 0000000000000100 badaddr: 0000000000000000 cause: 000000000000000c
[ 0.000000] ---[ end trace 0000000000000000 ]---
[ 0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
[ 0.000000] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---

Fix this issue by moving sbi_init() earlier before jump_label_init()

Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
---
arch/riscv/kernel/setup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 834eb652a7b9..d150cedeb7e0 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -268,6 +268,7 @@ void __init setup_arch(char **cmdline_p)
*cmdline_p = boot_command_line;

early_ioremap_setup();
+ sbi_init();
jump_label_init();
parse_early_param();

@@ -284,7 +285,6 @@ void __init setup_arch(char **cmdline_p)
misc_mem_init();

init_resources();
- sbi_init();

#ifdef CONFIG_KASAN
kasan_init();
--
2.34.1

Jisheng Zhang

unread,
May 19, 2022, 12:08:13 PM5/19/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Anup Patel, Atish Patra, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
On a specific HW platform, pgtable_l4|[l5]_enabled won't change after
boot, and the check sits at hot code path, this characteristic makes it
suitable for optimization with static key.

_pgtable_l4|[l5]_enabled is used very early during boot, even is used
with MMU off, so the static key mechanism isn't ready. For this case,
we use another static key _pgtable_lx_ready to indicate whether we
have finalised pgtable_l4|[l5]_enabled or not, then fall back to
_pgtable_l4|[l5]_enabled_early bool.

Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
---
arch/riscv/include/asm/pgalloc.h | 16 ++++----
arch/riscv/include/asm/pgtable-32.h | 3 ++
arch/riscv/include/asm/pgtable-64.h | 59 +++++++++++++++++---------
arch/riscv/include/asm/pgtable.h | 5 +--
arch/riscv/kernel/cpu.c | 4 +-
arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
arch/riscv/mm/kasan_init.c | 16 ++++----
7 files changed, 102 insertions(+), 65 deletions(-)

diff --git a/arch/riscv/include/asm/pgalloc.h b/arch/riscv/include/asm/pgalloc.h
index 947f23d7b6af..0280eeb4756f 100644
--- a/arch/riscv/include/asm/pgalloc.h
+++ b/arch/riscv/include/asm/pgalloc.h
@@ -41,7 +41,7 @@ static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)

static inline void p4d_populate(struct mm_struct *mm, p4d_t *p4d, pud_t *pud)
{
- if (pgtable_l4_enabled) {
+ if (pgtable_l4_enabled()) {
unsigned long pfn = virt_to_pfn(pud);

set_p4d(p4d, __p4d((pfn << _PAGE_PFN_SHIFT) | _PAGE_TABLE));
@@ -51,7 +51,7 @@ static inline void p4d_populate(struct mm_struct *mm, p4d_t *p4d, pud_t *pud)
static inline void p4d_populate_safe(struct mm_struct *mm, p4d_t *p4d,
pud_t *pud)
{
- if (pgtable_l4_enabled) {
+ if (pgtable_l4_enabled()) {
unsigned long pfn = virt_to_pfn(pud);

set_p4d_safe(p4d,
@@ -61,7 +61,7 @@ static inline void p4d_populate_safe(struct mm_struct *mm, p4d_t *p4d,

static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, p4d_t *p4d)
{
- if (pgtable_l5_enabled) {
+ if (pgtable_l5_enabled()) {
unsigned long pfn = virt_to_pfn(p4d);

set_pgd(pgd, __pgd((pfn << _PAGE_PFN_SHIFT) | _PAGE_TABLE));
@@ -71,7 +71,7 @@ static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, p4d_t *p4d)
static inline void pgd_populate_safe(struct mm_struct *mm, pgd_t *pgd,
p4d_t *p4d)
{
- if (pgtable_l5_enabled) {
+ if (pgtable_l5_enabled()) {
unsigned long pfn = virt_to_pfn(p4d);

set_pgd_safe(pgd,
@@ -82,7 +82,7 @@ static inline void pgd_populate_safe(struct mm_struct *mm, pgd_t *pgd,
#define pud_alloc_one pud_alloc_one
static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return __pud_alloc_one(mm, addr);

return NULL;
@@ -91,7 +91,7 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
#define pud_free pud_free
static inline void pud_free(struct mm_struct *mm, pud_t *pud)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
__pud_free(mm, pud);
}

@@ -100,7 +100,7 @@ static inline void pud_free(struct mm_struct *mm, pud_t *pud)
#define p4d_alloc_one p4d_alloc_one
static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
{
- if (pgtable_l5_enabled) {
+ if (pgtable_l5_enabled()) {
gfp_t gfp = GFP_PGTABLE_USER;

if (mm == &init_mm)
@@ -120,7 +120,7 @@ static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
#define p4d_free p4d_free
static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
__p4d_free(mm, p4d);
}

diff --git a/arch/riscv/include/asm/pgtable-32.h b/arch/riscv/include/asm/pgtable-32.h
index 5b2e79e5bfa5..8af36d76b70d 100644
--- a/arch/riscv/include/asm/pgtable-32.h
+++ b/arch/riscv/include/asm/pgtable-32.h
@@ -16,4 +16,7 @@

#define MAX_POSSIBLE_PHYSMEM_BITS 34

+#define pgtable_l5_enabled() 0
+#define pgtable_l4_enabled() 0
+
#endif /* _ASM_RISCV_PGTABLE_32_H */
diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 7e246e9f8d70..c56bbeacd369 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -8,16 +8,35 @@

#include <linux/const.h>

-extern bool pgtable_l4_enabled;
-extern bool pgtable_l5_enabled;
+extern bool _pgtable_l5_enabled_early;
+extern bool _pgtable_l4_enabled_early;
+extern struct static_key_false _pgtable_l5_enabled;
+extern struct static_key_false _pgtable_l4_enabled;
+extern struct static_key_false _pgtable_lx_ready;
+
+static __always_inline bool pgtable_l5_enabled(void)
+{
+ if (static_branch_likely(&_pgtable_lx_ready))
+ return static_branch_likely(&_pgtable_l5_enabled);
+ else
+ return _pgtable_l5_enabled_early;
+}
+
+static __always_inline bool pgtable_l4_enabled(void)
+{
+ if (static_branch_likely(&_pgtable_lx_ready))
+ return static_branch_likely(&_pgtable_l4_enabled);
+ else
+ return _pgtable_l4_enabled_early;
+}

#define PGDIR_SHIFT_L3 30
#define PGDIR_SHIFT_L4 39
#define PGDIR_SHIFT_L5 48
#define PGDIR_SIZE_L3 (_AC(1, UL) << PGDIR_SHIFT_L3)

-#define PGDIR_SHIFT (pgtable_l5_enabled ? PGDIR_SHIFT_L5 : \
- (pgtable_l4_enabled ? PGDIR_SHIFT_L4 : PGDIR_SHIFT_L3))
+#define PGDIR_SHIFT (pgtable_l5_enabled() ? PGDIR_SHIFT_L5 : \
+ (pgtable_l4_enabled() ? PGDIR_SHIFT_L4 : PGDIR_SHIFT_L3))
/* Size of region mapped by a page global directory */
#define PGDIR_SIZE (_AC(1, UL) << PGDIR_SHIFT)
#define PGDIR_MASK (~(PGDIR_SIZE - 1))
@@ -119,7 +138,7 @@ static inline struct page *pud_page(pud_t pud)
#define mm_p4d_folded mm_p4d_folded
static inline bool mm_p4d_folded(struct mm_struct *mm)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return false;

return true;
@@ -128,7 +147,7 @@ static inline bool mm_p4d_folded(struct mm_struct *mm)
#define mm_pud_folded mm_pud_folded
static inline bool mm_pud_folded(struct mm_struct *mm)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return false;

return true;
@@ -159,7 +178,7 @@ static inline unsigned long _pmd_pfn(pmd_t pmd)

static inline void set_p4d(p4d_t *p4dp, p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
*p4dp = p4d;
else
set_pud((pud_t *)p4dp, (pud_t){ p4d_val(p4d) });
@@ -167,7 +186,7 @@ static inline void set_p4d(p4d_t *p4dp, p4d_t p4d)

static inline int p4d_none(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (p4d_val(p4d) == 0);

return 0;
@@ -175,7 +194,7 @@ static inline int p4d_none(p4d_t p4d)

static inline int p4d_present(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (p4d_val(p4d) & _PAGE_PRESENT);

return 1;
@@ -183,7 +202,7 @@ static inline int p4d_present(p4d_t p4d)

static inline int p4d_bad(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return !p4d_present(p4d);

return 0;
@@ -191,7 +210,7 @@ static inline int p4d_bad(p4d_t p4d)

static inline void p4d_clear(p4d_t *p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
set_p4d(p4d, __p4d(0));
}

@@ -207,7 +226,7 @@ static inline unsigned long _p4d_pfn(p4d_t p4d)

static inline pud_t *p4d_pgtable(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (pud_t *)pfn_to_virt(p4d_val(p4d) >> _PAGE_PFN_SHIFT);

return (pud_t *)pud_pgtable((pud_t) { p4d_val(p4d) });
@@ -224,7 +243,7 @@ static inline struct page *p4d_page(p4d_t p4d)
#define pud_offset pud_offset
static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return p4d_pgtable(*p4d) + pud_index(address);

return (pud_t *)p4d;
@@ -232,7 +251,7 @@ static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)

static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
*pgdp = pgd;
else
set_p4d((p4d_t *)pgdp, (p4d_t){ pgd_val(pgd) });
@@ -240,7 +259,7 @@ static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)

static inline int pgd_none(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (pgd_val(pgd) == 0);

return 0;
@@ -248,7 +267,7 @@ static inline int pgd_none(pgd_t pgd)

static inline int pgd_present(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (pgd_val(pgd) & _PAGE_PRESENT);

return 1;
@@ -256,7 +275,7 @@ static inline int pgd_present(pgd_t pgd)

static inline int pgd_bad(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return !pgd_present(pgd);

return 0;
@@ -264,13 +283,13 @@ static inline int pgd_bad(pgd_t pgd)

static inline void pgd_clear(pgd_t *pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
set_pgd(pgd, __pgd(0));
}

static inline p4d_t *pgd_pgtable(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (p4d_t *)pfn_to_virt(pgd_val(pgd) >> _PAGE_PFN_SHIFT);

return (p4d_t *)p4d_pgtable((p4d_t) { pgd_val(pgd) });
@@ -288,7 +307,7 @@ static inline struct page *pgd_page(pgd_t pgd)
#define p4d_offset p4d_offset
static inline p4d_t *p4d_offset(pgd_t *pgd, unsigned long address)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return pgd_pgtable(*pgd) + p4d_index(address);

return (p4d_t *)pgd;
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 046b44225623..ae01a9b83ac4 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -63,8 +63,8 @@
* position vmemmap directly below the VMALLOC region.
*/
#ifdef CONFIG_64BIT
-#define VA_BITS (pgtable_l5_enabled ? \
- 57 : (pgtable_l4_enabled ? 48 : 39))
+#define VA_BITS (pgtable_l5_enabled() ? \
+ 57 : (pgtable_l4_enabled() ? 48 : 39))
#else
#define VA_BITS 32
#endif
@@ -738,7 +738,6 @@ extern uintptr_t _dtb_early_pa;
#define dtb_early_pa _dtb_early_pa
#endif /* CONFIG_XIP_KERNEL */
extern u64 satp_mode;
-extern bool pgtable_l4_enabled;

void paging_init(void);
void misc_mem_init(void);
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index ccb617791e56..29bb0ef75248 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -141,9 +141,9 @@ static void print_mmu(struct seq_file *f)
#if defined(CONFIG_32BIT)
strncpy(sv_type, "sv32", 5);
#elif defined(CONFIG_64BIT)
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
strncpy(sv_type, "sv57", 5);
- else if (pgtable_l4_enabled)
+ else if (pgtable_l4_enabled())
strncpy(sv_type, "sv48", 5);
else
strncpy(sv_type, "sv39", 5);
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 05ed641a1134..42c79388e6fd 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -44,10 +44,16 @@ u64 satp_mode __ro_after_init = SATP_MODE_32;
#endif
EXPORT_SYMBOL(satp_mode);

-bool pgtable_l4_enabled = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
-bool pgtable_l5_enabled = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
-EXPORT_SYMBOL(pgtable_l4_enabled);
-EXPORT_SYMBOL(pgtable_l5_enabled);
+DEFINE_STATIC_KEY_FALSE(_pgtable_l4_enabled);
+DEFINE_STATIC_KEY_FALSE(_pgtable_l5_enabled);
+DEFINE_STATIC_KEY_FALSE(_pgtable_lx_ready);
+EXPORT_SYMBOL(_pgtable_l4_enabled);
+EXPORT_SYMBOL(_pgtable_l5_enabled);
+EXPORT_SYMBOL(_pgtable_lx_ready);
+bool _pgtable_l4_enabled_early = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
+bool _pgtable_l5_enabled_early = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
+EXPORT_SYMBOL(_pgtable_l4_enabled_early);
+EXPORT_SYMBOL(_pgtable_l5_enabled_early);

phys_addr_t phys_ram_base __ro_after_init;
EXPORT_SYMBOL(phys_ram_base);
@@ -555,26 +561,26 @@ static void __init create_p4d_mapping(p4d_t *p4dp,
}

#define pgd_next_t p4d_t
-#define alloc_pgd_next(__va) (pgtable_l5_enabled ? \
- pt_ops.alloc_p4d(__va) : (pgtable_l4_enabled ? \
+#define alloc_pgd_next(__va) (pgtable_l5_enabled() ? \
+ pt_ops.alloc_p4d(__va) : (pgtable_l4_enabled() ? \
pt_ops.alloc_pud(__va) : pt_ops.alloc_pmd(__va)))
-#define get_pgd_next_virt(__pa) (pgtable_l5_enabled ? \
- pt_ops.get_p4d_virt(__pa) : (pgd_next_t *)(pgtable_l4_enabled ? \
+#define get_pgd_next_virt(__pa) (pgtable_l5_enabled() ? \
+ pt_ops.get_p4d_virt(__pa) : (pgd_next_t *)(pgtable_l4_enabled() ? \
pt_ops.get_pud_virt(__pa) : (pud_t *)pt_ops.get_pmd_virt(__pa)))
#define create_pgd_next_mapping(__nextp, __va, __pa, __sz, __prot) \
- (pgtable_l5_enabled ? \
+ (pgtable_l5_enabled() ? \
create_p4d_mapping(__nextp, __va, __pa, __sz, __prot) : \
- (pgtable_l4_enabled ? \
+ (pgtable_l4_enabled() ? \
create_pud_mapping((pud_t *)__nextp, __va, __pa, __sz, __prot) : \
create_pmd_mapping((pmd_t *)__nextp, __va, __pa, __sz, __prot)))
-#define fixmap_pgd_next (pgtable_l5_enabled ? \
- (uintptr_t)fixmap_p4d : (pgtable_l4_enabled ? \
+#define fixmap_pgd_next (pgtable_l5_enabled() ? \
+ (uintptr_t)fixmap_p4d : (pgtable_l4_enabled() ? \
(uintptr_t)fixmap_pud : (uintptr_t)fixmap_pmd))
-#define trampoline_pgd_next (pgtable_l5_enabled ? \
- (uintptr_t)trampoline_p4d : (pgtable_l4_enabled ? \
+#define trampoline_pgd_next (pgtable_l5_enabled() ? \
+ (uintptr_t)trampoline_p4d : (pgtable_l4_enabled() ? \
(uintptr_t)trampoline_pud : (uintptr_t)trampoline_pmd))
-#define early_dtb_pgd_next (pgtable_l5_enabled ? \
- (uintptr_t)early_dtb_p4d : (pgtable_l4_enabled ? \
+#define early_dtb_pgd_next (pgtable_l5_enabled() ? \
+ (uintptr_t)early_dtb_p4d : (pgtable_l4_enabled() ? \
(uintptr_t)early_dtb_pud : (uintptr_t)early_dtb_pmd))
#else
#define pgd_next_t pte_t
@@ -680,14 +686,14 @@ static __init pgprot_t pgprot_from_va(uintptr_t va)
#ifdef CONFIG_64BIT
static void __init disable_pgtable_l5(void)
{
- pgtable_l5_enabled = false;
+ _pgtable_l5_enabled_early = false;
kernel_map.page_offset = PAGE_OFFSET_L4;
satp_mode = SATP_MODE_48;
}

static void __init disable_pgtable_l4(void)
{
- pgtable_l4_enabled = false;
+ _pgtable_l4_enabled_early = false;
kernel_map.page_offset = PAGE_OFFSET_L3;
satp_mode = SATP_MODE_39;
}
@@ -816,11 +822,11 @@ static void __init create_fdt_early_page_table(pgd_t *pgdir, uintptr_t dtb_pa)
PGDIR_SIZE,
IS_ENABLED(CONFIG_64BIT) ? PAGE_TABLE : PAGE_KERNEL);

- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(early_dtb_p4d, DTB_EARLY_BASE_VA,
(uintptr_t)early_dtb_pud, P4D_SIZE, PAGE_TABLE);

- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(early_dtb_pud, DTB_EARLY_BASE_VA,
(uintptr_t)early_dtb_pmd, PUD_SIZE, PAGE_TABLE);

@@ -961,11 +967,11 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)

#ifndef __PAGETABLE_PMD_FOLDED
/* Setup fixmap P4D and PUD */
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(fixmap_p4d, FIXADDR_START,
(uintptr_t)fixmap_pud, P4D_SIZE, PAGE_TABLE);
/* Setup fixmap PUD and PMD */
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(fixmap_pud, FIXADDR_START,
(uintptr_t)fixmap_pmd, PUD_SIZE, PAGE_TABLE);
create_pmd_mapping(fixmap_pmd, FIXADDR_START,
@@ -973,10 +979,10 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
/* Setup trampoline PGD and PMD */
create_pgd_mapping(trampoline_pg_dir, kernel_map.virt_addr,
trampoline_pgd_next, PGDIR_SIZE, PAGE_TABLE);
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(trampoline_p4d, kernel_map.virt_addr,
(uintptr_t)trampoline_pud, P4D_SIZE, PAGE_TABLE);
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(trampoline_pud, kernel_map.virt_addr,
(uintptr_t)trampoline_pmd, PUD_SIZE, PAGE_TABLE);
#ifdef CONFIG_XIP_KERNEL
@@ -1165,8 +1171,18 @@ static void __init reserve_crashkernel(void)
crashk_res.end = crash_base + crash_size - 1;
}

+static void __init riscv_finalise_pgtable_lx(void)
+{
+ if (_pgtable_l5_enabled_early)
+ static_branch_enable(&_pgtable_l5_enabled);
+ if (_pgtable_l4_enabled_early)
+ static_branch_enable(&_pgtable_l4_enabled);
+ static_branch_enable(&_pgtable_lx_ready);
+}
+
void __init paging_init(void)
{
+ riscv_finalise_pgtable_lx();
setup_bootmem();
setup_vm_final();
}
diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c
index a22e418dbd82..356044498e8a 100644
--- a/arch/riscv/mm/kasan_init.c
+++ b/arch/riscv/mm/kasan_init.c
@@ -209,15 +209,15 @@ static void __init kasan_populate_p4d(pgd_t *pgd,
set_pgd(pgd, pfn_pgd(PFN_DOWN(__pa(base_p4d)), PAGE_TABLE));
}

-#define kasan_early_shadow_pgd_next (pgtable_l5_enabled ? \
+#define kasan_early_shadow_pgd_next (pgtable_l5_enabled() ? \
(uintptr_t)kasan_early_shadow_p4d : \
- (pgtable_l4_enabled ? \
+ (pgtable_l4_enabled() ? \
(uintptr_t)kasan_early_shadow_pud : \
(uintptr_t)kasan_early_shadow_pmd))
#define kasan_populate_pgd_next(pgdp, vaddr, next, early) \
- (pgtable_l5_enabled ? \
+ (pgtable_l5_enabled() ? \
kasan_populate_p4d(pgdp, vaddr, next, early) : \
- (pgtable_l4_enabled ? \
+ (pgtable_l4_enabled() ? \
kasan_populate_pud(pgdp, vaddr, next, early) : \
kasan_populate_pmd((pud_t *)pgdp, vaddr, next)))

@@ -274,7 +274,7 @@ asmlinkage void __init kasan_early_init(void)
(__pa((uintptr_t)kasan_early_shadow_pte)),
PAGE_TABLE));

- if (pgtable_l4_enabled) {
+ if (pgtable_l4_enabled()) {
for (i = 0; i < PTRS_PER_PUD; ++i)
set_pud(kasan_early_shadow_pud + i,
pfn_pud(PFN_DOWN
@@ -282,7 +282,7 @@ asmlinkage void __init kasan_early_init(void)
PAGE_TABLE));
}

- if (pgtable_l5_enabled) {
+ if (pgtable_l5_enabled()) {
for (i = 0; i < PTRS_PER_P4D; ++i)
set_p4d(kasan_early_shadow_p4d + i,
pfn_p4d(PFN_DOWN
@@ -393,9 +393,9 @@ static void __init kasan_shallow_populate_p4d(pgd_t *pgdp,
}

#define kasan_shallow_populate_pgd_next(pgdp, vaddr, next) \
- (pgtable_l5_enabled ? \
+ (pgtable_l5_enabled() ? \
kasan_shallow_populate_p4d(pgdp, vaddr, next) : \
- (pgtable_l4_enabled ? \
+ (pgtable_l4_enabled() ? \
kasan_shallow_populate_pud(pgdp, vaddr, next) : \
kasan_shallow_populate_pmd(pgdp, vaddr, next)))

--
2.34.1

kernel test robot

unread,
May 19, 2022, 7:57:53 PM5/19/22
to Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Anup Patel, Atish Patra, ll...@lists.linux.dev, kbuil...@lists.01.org, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
Hi Jisheng,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.18-rc7]
[cannot apply to next-20220519]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Jisheng-Zhang/use-static-key-to-optimize-pgtable_l4_enabled/20220520-001459
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git f993aed406eaf968ba3867a76bb46c95336a33d0
config: riscv-buildonly-randconfig-r003-20220519 (https://download.01.org/0day-ci/archive/20220520/202205200730...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project e00cbbec06c08dc616a0d52a20f678b8fbd4e304)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/d052c69ebaf48ac2925d6f9fa033d9e394da1074
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jisheng-Zhang/use-static-key-to-optimize-pgtable_l4_enabled/20220520-001459
git checkout d052c69ebaf48ac2925d6f9fa033d9e394da1074
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/tty/ net/ceph/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <l...@intel.com>

All error/warnings (new ones prefixed by >>):

In file included from drivers/tty/tty_io.c:73:
In file included from include/linux/sched/signal.h:9:
In file included from include/linux/sched/task.h:11:
In file included from include/linux/uaccess.h:11:
In file included from arch/riscv/include/asm/uaccess.h:12:
In file included from arch/riscv/include/asm/pgtable.h:112:
>> arch/riscv/include/asm/pgtable-64.h:19:6: error: call to undeclared function 'static_branch_likely'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (static_branch_likely(&_pgtable_lx_ready))
^
arch/riscv/include/asm/pgtable-64.h:27:6: error: call to undeclared function 'static_branch_likely'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (static_branch_likely(&_pgtable_lx_ready))
^
In file included from drivers/tty/tty_io.c:75:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/tty/tty_io.c:75:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/tty/tty_io.c:75:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
~~~~~~~~~~ ^
7 warnings and 2 errors generated.
--
In file included from drivers/tty/tty_ioctl.c:11:
In file included from include/uapi/linux/termios.h:6:
In file included from ./arch/riscv/include/generated/uapi/asm/termios.h:1:
In file included from include/asm-generic/termios.h:6:
In file included from include/linux/uaccess.h:11:
In file included from arch/riscv/include/asm/uaccess.h:12:
In file included from arch/riscv/include/asm/pgtable.h:112:
>> arch/riscv/include/asm/pgtable-64.h:19:6: error: call to undeclared function 'static_branch_likely'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (static_branch_likely(&_pgtable_lx_ready))
^
arch/riscv/include/asm/pgtable-64.h:27:6: error: call to undeclared function 'static_branch_likely'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (static_branch_likely(&_pgtable_lx_ready))
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:97:11: warning: array index 3 is past the end of the array (which contains 1 element) [-Warray-bounds]
return (set->sig[3] | set->sig[2] |
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:97:25: warning: array index 2 is past the end of the array (which contains 1 element) [-Warray-bounds]
return (set->sig[3] | set->sig[2] |
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:98:4: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
set->sig[1] | set->sig[0]) == 0;
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:100:11: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
return (set->sig[1] | set->sig[0]) == 0;
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:113:11: warning: array index 3 is past the end of the array (which contains 1 element) [-Warray-bounds]
return (set1->sig[3] == set2->sig[3]) &&
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:113:27: warning: array index 3 is past the end of the array (which contains 1 element) [-Warray-bounds]
return (set1->sig[3] == set2->sig[3]) &&
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:114:5: warning: array index 2 is past the end of the array (which contains 1 element) [-Warray-bounds]
(set1->sig[2] == set2->sig[2]) &&
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:114:21: warning: array index 2 is past the end of the array (which contains 1 element) [-Warray-bounds]
(set1->sig[2] == set2->sig[2]) &&
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:115:5: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
(set1->sig[1] == set2->sig[1]) &&
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:115:21: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
(set1->sig[1] == set2->sig[1]) &&
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:118:11: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
return (set1->sig[1] == set2->sig[1]) &&
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/tty_ioctl.c:13:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:118:27: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
return (set1->sig[1] == set2->sig[1]) &&
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
--
In file included from drivers/tty/tty_port.c:8:
In file included from include/linux/tty.h:5:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:9:
In file included from include/linux/sched/task.h:11:
In file included from include/linux/uaccess.h:11:
In file included from arch/riscv/include/asm/uaccess.h:12:
In file included from arch/riscv/include/asm/pgtable.h:112:
>> arch/riscv/include/asm/pgtable-64.h:19:6: error: call to undeclared function 'static_branch_likely'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (static_branch_likely(&_pgtable_lx_ready))
^
arch/riscv/include/asm/pgtable-64.h:27:6: error: call to undeclared function 'static_branch_likely'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (static_branch_likely(&_pgtable_lx_ready))
^
In file included from drivers/tty/tty_port.c:8:
In file included from include/linux/tty.h:12:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:42:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/tty/tty_port.c:8:
In file included from include/linux/tty.h:12:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:42:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/tty/tty_port.c:8:
In file included from include/linux/tty.h:12:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:42:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
~~~~~~~~~~ ^
>> drivers/tty/tty_port.c:245:2: warning: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
INIT_KFIFO(port->xmit_fifo);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/kfifo.h:130:69: note: expanded from macro 'INIT_KFIFO'
__kfifo->mask = __is_kfifo_ptr(__tmp) ? 0 : ARRAY_SIZE(__tmp->buf) - 1;\
~ ~~~~~~~~~~~~~~~~~~~~~~~^~~
8 warnings and 2 errors generated.
--
In file included from drivers/tty/serial/earlycon.c:16:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:15:
In file included from include/linux/pgtable.h:6:
In file included from arch/riscv/include/asm/pgtable.h:112:
>> arch/riscv/include/asm/pgtable-64.h:19:6: error: call to undeclared function 'static_branch_likely'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (static_branch_likely(&_pgtable_lx_ready))
^
arch/riscv/include/asm/pgtable-64.h:27:6: error: call to undeclared function 'static_branch_likely'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (static_branch_likely(&_pgtable_lx_ready))
^
In file included from drivers/tty/serial/earlycon.c:16:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/tty/serial/earlycon.c:16:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/tty/serial/earlycon.c:16:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
~~~~~~~~~~ ^
In file included from drivers/tty/serial/earlycon.c:17:
In file included from include/linux/serial_core.h:13:
In file included from include/linux/interrupt.h:21:
In file included from arch/riscv/include/asm/sections.h:9:
In file included from include/linux/mm.h:700:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:97:11: warning: array index 3 is past the end of the array (which contains 1 element) [-Warray-bounds]
return (set->sig[3] | set->sig[2] |
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/serial/earlycon.c:17:
In file included from include/linux/serial_core.h:13:
In file included from include/linux/interrupt.h:21:
In file included from arch/riscv/include/asm/sections.h:9:
In file included from include/linux/mm.h:700:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:97:25: warning: array index 2 is past the end of the array (which contains 1 element) [-Warray-bounds]
return (set->sig[3] | set->sig[2] |
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/serial/earlycon.c:17:
In file included from include/linux/serial_core.h:13:
In file included from include/linux/interrupt.h:21:
In file included from arch/riscv/include/asm/sections.h:9:
In file included from include/linux/mm.h:700:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:98:4: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
set->sig[1] | set->sig[0]) == 0;
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from drivers/tty/serial/earlycon.c:17:
In file included from include/linux/serial_core.h:13:
In file included from include/linux/interrupt.h:21:
In file included from arch/riscv/include/asm/sections.h:9:
In file included from include/linux/mm.h:700:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:100:11: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
..


vim +/static_branch_likely +19 arch/riscv/include/asm/pgtable-64.h

16
17 static __always_inline bool pgtable_l5_enabled(void)
18 {
> 19 if (static_branch_likely(&_pgtable_lx_ready))
20 return static_branch_likely(&_pgtable_l5_enabled);
21 else
22 return _pgtable_l5_enabled_early;
23 }
24

--
0-DAY CI Kernel Test Service
https://01.org/lkp

kernel test robot

unread,
May 19, 2022, 8:08:54 PM5/19/22
to Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Anup Patel, Atish Patra, kbuil...@lists.01.org, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
Hi Jisheng,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.18-rc7]
[cannot apply to next-20220519]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Jisheng-Zhang/use-static-key-to-optimize-pgtable_l4_enabled/20220520-001459
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git f993aed406eaf968ba3867a76bb46c95336a33d0
config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20220520/202205200851...@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/d052c69ebaf48ac2925d6f9fa033d9e394da1074
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jisheng-Zhang/use-static-key-to-optimize-pgtable_l4_enabled/20220520-001459
git checkout d052c69ebaf48ac2925d6f9fa033d9e394da1074
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <l...@intel.com>

All errors (new ones prefixed by >>):

In file included from arch/riscv/include/asm/pgtable.h:112,
from arch/riscv/include/asm/uaccess.h:12,
from include/linux/uaccess.h:11,
from include/linux/sched/task.h:11,
from include/linux/sched/signal.h:9,
from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from fs/char_dev.c:9:
arch/riscv/include/asm/pgtable-64.h: In function 'pgtable_l5_enabled':
>> arch/riscv/include/asm/pgtable-64.h:19:13: error: implicit declaration of function 'static_branch_likely' [-Werror=implicit-function-declaration]
19 | if (static_branch_likely(&_pgtable_lx_ready))
| ^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from arch/riscv/include/asm/pgtable.h:112,
from arch/riscv/include/asm/uaccess.h:12,
from include/linux/uaccess.h:11,
from include/linux/sched/task.h:11,
from include/linux/sched/signal.h:9,
from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from include/uapi/linux/aio_abi.h:31,
from include/linux/syscalls.h:77,
from fs/d_path.c:2:
arch/riscv/include/asm/pgtable-64.h: In function 'pgtable_l5_enabled':
>> arch/riscv/include/asm/pgtable-64.h:19:13: error: implicit declaration of function 'static_branch_likely' [-Werror=implicit-function-declaration]
19 | if (static_branch_likely(&_pgtable_lx_ready))
| ^~~~~~~~~~~~~~~~~~~~
fs/d_path.c: At top level:
fs/d_path.c:318:7: warning: no previous prototype for 'simple_dname' [-Wmissing-prototypes]
318 | char *simple_dname(struct dentry *dentry, char *buffer, int buflen)
| ^~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from arch/riscv/include/asm/pgtable.h:112,
from arch/riscv/include/asm/uaccess.h:12,
from include/linux/uaccess.h:11,
from include/linux/sched/task.h:11,
from include/linux/sched/signal.h:9,
from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from include/uapi/linux/aio_abi.h:31,
from include/linux/syscalls.h:77,
from fs/io_uring.c:45:
arch/riscv/include/asm/pgtable-64.h: In function 'pgtable_l5_enabled':
>> arch/riscv/include/asm/pgtable-64.h:19:13: error: implicit declaration of function 'static_branch_likely' [-Werror=implicit-function-declaration]
19 | if (static_branch_likely(&_pgtable_lx_ready))
| ^~~~~~~~~~~~~~~~~~~~
fs/io_uring.c: In function '__io_submit_flush_completions':
fs/io_uring.c:2660:40: warning: variable 'prev' set but not used [-Wunused-but-set-variable]
2660 | struct io_wq_work_node *node, *prev;
| ^~~~
cc1: some warnings being treated as errors
--
In file included from arch/riscv/include/asm/pgtable.h:112,
from arch/riscv/include/asm/uaccess.h:12,
from include/linux/uaccess.h:11,
from include/linux/sched/task.h:11,
from include/linux/sched/signal.h:9,
from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from fs/proc/meminfo.c:2:
arch/riscv/include/asm/pgtable-64.h: In function 'pgtable_l5_enabled':
>> arch/riscv/include/asm/pgtable-64.h:19:13: error: implicit declaration of function 'static_branch_likely' [-Werror=implicit-function-declaration]
19 | if (static_branch_likely(&_pgtable_lx_ready))
| ^~~~~~~~~~~~~~~~~~~~
fs/proc/meminfo.c: At top level:
fs/proc/meminfo.c:22:28: warning: no previous prototype for 'arch_report_meminfo' [-Wmissing-prototypes]
22 | void __attribute__((weak)) arch_report_meminfo(struct seq_file *m)
| ^~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors

Jisheng Zhang

unread,
May 21, 2022, 10:43:37 AM5/21/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Anup Patel, Atish Patra, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
The pgtable_l4|[l5]_enabled check sits at hot code path, performance
is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
boot, so static key can be used to solve the performance issue[1].

An unified way static key was introduced in [2], but it only targets
riscv isa extension. We dunno whether SV48 and SV57 will be considered
as isa extension, so the unified solution isn't used for
pgtable_l4[l5]_enabled now.

patch1 fixes a NULL pointer deference if static key is used a bit earlier.
Since v3:
- fix W=1 call to undeclared function 'static_branch_likely' error

Since v2:
- move the W=1 warning fix to a separate patch
- move the unified way to use static key to a new patch series.

Since v1:
- Add a W=1 warning fix
- Fix W=1 error
- Based on v5.18-rcN, since SV57 support is added, so convert
pgtable_l5_enabled as well.



Jisheng Zhang (2):
riscv: move sbi_init() earlier before jump_label_init()
riscv: turn pgtable_l4|[l5]_enabled to static key for RV64

arch/riscv/include/asm/pgalloc.h | 16 ++++----
arch/riscv/include/asm/pgtable-32.h | 3 ++
arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
arch/riscv/include/asm/pgtable.h | 5 +--
arch/riscv/kernel/cpu.c | 4 +-
arch/riscv/kernel/setup.c | 2 +-
arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
arch/riscv/mm/kasan_init.c | 16 ++++----
8 files changed, 104 insertions(+), 66 deletions(-)

--
2.34.1

Jisheng Zhang

unread,
May 21, 2022, 10:43:46 AM5/21/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Anup Patel, Atish Patra, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
---

Jisheng Zhang

unread,
May 21, 2022, 10:43:52 AM5/21/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Anup Patel, Atish Patra, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
On a specific HW platform, pgtable_l4|[l5]_enabled won't change after
boot, and the check sits at hot code path, this characteristic makes it
suitable for optimization with static key.

_pgtable_l4|[l5]_enabled is used very early during boot, even is used
with MMU off, so the static key mechanism isn't ready. For this case,
we use another static key _pgtable_lx_ready to indicate whether we
have finalised pgtable_l4|[l5]_enabled or not, then fall back to
_pgtable_l4|[l5]_enabled_early bool.

Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
---
arch/riscv/include/asm/pgalloc.h | 16 ++++----
arch/riscv/include/asm/pgtable-32.h | 3 ++
arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
arch/riscv/include/asm/pgtable.h | 5 +--
arch/riscv/kernel/cpu.c | 4 +-
arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
arch/riscv/mm/kasan_init.c | 16 ++++----
7 files changed, 103 insertions(+), 65 deletions(-)
index 7e246e9f8d70..d14a3a8f1f4b 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -7,17 +7,37 @@
#define _ASM_RISCV_PGTABLE_64_H

#include <linux/const.h>
+#include <linux/jump_label.h>
@@ -119,7 +139,7 @@ static inline struct page *pud_page(pud_t pud)
#define mm_p4d_folded mm_p4d_folded
static inline bool mm_p4d_folded(struct mm_struct *mm)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return false;

return true;
@@ -128,7 +148,7 @@ static inline bool mm_p4d_folded(struct mm_struct *mm)
#define mm_pud_folded mm_pud_folded
static inline bool mm_pud_folded(struct mm_struct *mm)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return false;

return true;
@@ -159,7 +179,7 @@ static inline unsigned long _pmd_pfn(pmd_t pmd)

static inline void set_p4d(p4d_t *p4dp, p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
*p4dp = p4d;
else
set_pud((pud_t *)p4dp, (pud_t){ p4d_val(p4d) });
@@ -167,7 +187,7 @@ static inline void set_p4d(p4d_t *p4dp, p4d_t p4d)

static inline int p4d_none(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (p4d_val(p4d) == 0);

return 0;
@@ -175,7 +195,7 @@ static inline int p4d_none(p4d_t p4d)

static inline int p4d_present(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (p4d_val(p4d) & _PAGE_PRESENT);

return 1;
@@ -183,7 +203,7 @@ static inline int p4d_present(p4d_t p4d)

static inline int p4d_bad(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return !p4d_present(p4d);

return 0;
@@ -191,7 +211,7 @@ static inline int p4d_bad(p4d_t p4d)

static inline void p4d_clear(p4d_t *p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
set_p4d(p4d, __p4d(0));
}

@@ -207,7 +227,7 @@ static inline unsigned long _p4d_pfn(p4d_t p4d)

static inline pud_t *p4d_pgtable(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (pud_t *)pfn_to_virt(p4d_val(p4d) >> _PAGE_PFN_SHIFT);

return (pud_t *)pud_pgtable((pud_t) { p4d_val(p4d) });
@@ -224,7 +244,7 @@ static inline struct page *p4d_page(p4d_t p4d)
#define pud_offset pud_offset
static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return p4d_pgtable(*p4d) + pud_index(address);

return (pud_t *)p4d;
@@ -232,7 +252,7 @@ static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)

static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
*pgdp = pgd;
else
set_p4d((p4d_t *)pgdp, (p4d_t){ pgd_val(pgd) });
@@ -240,7 +260,7 @@ static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)

static inline int pgd_none(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (pgd_val(pgd) == 0);

return 0;
@@ -248,7 +268,7 @@ static inline int pgd_none(pgd_t pgd)

static inline int pgd_present(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (pgd_val(pgd) & _PAGE_PRESENT);

return 1;
@@ -256,7 +276,7 @@ static inline int pgd_present(pgd_t pgd)

static inline int pgd_bad(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return !pgd_present(pgd);

return 0;
@@ -264,13 +284,13 @@ static inline int pgd_bad(pgd_t pgd)

static inline void pgd_clear(pgd_t *pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
set_pgd(pgd, __pgd(0));
}

static inline p4d_t *pgd_pgtable(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (p4d_t *)pfn_to_virt(pgd_val(pgd) >> _PAGE_PFN_SHIFT);

return (p4d_t *)p4d_pgtable((p4d_t) { pgd_val(pgd) });
@@ -288,7 +308,7 @@ static inline struct page *pgd_page(pgd_t pgd)

Atish Patra

unread,
May 22, 2022, 4:01:58 AM5/22/22
to Jisheng Zhang, Alexandre Ghiti, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Anup Patel, Atish Patra, linux-riscv, linux-kernel@vger.kernel.org List, kasa...@googlegroups.com
On Sat, May 21, 2022 at 7:44 AM Jisheng Zhang <jsz...@kernel.org> wrote:
>
> We call jump_label_init() in setup_arch() is to use static key
> mechanism earlier, but riscv jump label relies on the sbi functions,
> If we enable static key before sbi_init(), the code path looks like:
> static_branch_enable()
> ..
> arch_jump_label_transform()
> patch_text_nosync()
> flush_icache_range()
> flush_icache_all()
> sbi_remote_fence_i() for CONFIG_RISCV_SBI case
> __sbi_rfence()
>

@Alexandre Ghiti : Is this the root cause of the panic you were seeing ?

IIRC, you mentioned in your last email that you don't see the issue
anymore. May be you avoided the issue because alternatives usage
was moved but root case remains as it is ?
> _______________________________________________
> linux-riscv mailing list
> linux...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



--
Regards,
Atish

Anup Patel

unread,
May 23, 2022, 12:02:59 PM5/23/22
to Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Atish Patra, linux-riscv, linux-kernel@vger.kernel.org List, kasa...@googlegroups.com
On Thu, May 19, 2022 at 9:38 PM Jisheng Zhang <jsz...@kernel.org> wrote:
>
> On a specific HW platform, pgtable_l4|[l5]_enabled won't change after
> boot, and the check sits at hot code path, this characteristic makes it
> suitable for optimization with static key.
>
> _pgtable_l4|[l5]_enabled is used very early during boot, even is used
> with MMU off, so the static key mechanism isn't ready. For this case,
> we use another static key _pgtable_lx_ready to indicate whether we
> have finalised pgtable_l4|[l5]_enabled or not, then fall back to
> _pgtable_l4|[l5]_enabled_early bool.
>
> Signed-off-by: Jisheng Zhang <jsz...@kernel.org>

Overall, this patch looks good to me. Please fix the errors reported by
autobuilders.

Reviewed-by: Anup Patel <an...@brainfault.org>

Regards,
Anup

Jisheng Zhang

unread,
May 24, 2022, 11:36:55 AM5/24/22
to Anup Patel, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Atish Patra, linux-riscv, linux-kernel@vger.kernel.org List, kasa...@googlegroups.com
On Mon, May 23, 2022 at 09:32:46PM +0530, Anup Patel wrote:
> On Thu, May 19, 2022 at 9:38 PM Jisheng Zhang <jsz...@kernel.org> wrote:
> >
> > On a specific HW platform, pgtable_l4|[l5]_enabled won't change after
> > boot, and the check sits at hot code path, this characteristic makes it
> > suitable for optimization with static key.
> >
> > _pgtable_l4|[l5]_enabled is used very early during boot, even is used
> > with MMU off, so the static key mechanism isn't ready. For this case,
> > we use another static key _pgtable_lx_ready to indicate whether we
> > have finalised pgtable_l4|[l5]_enabled or not, then fall back to
> > _pgtable_l4|[l5]_enabled_early bool.
> >
> > Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
>
> Overall, this patch looks good to me. Please fix the errors reported by
> autobuilders.
>
> Reviewed-by: Anup Patel <an...@brainfault.org>

Thank Anup. I sent the v4 two days ago
https://lore.kernel.org/linux-riscv/20220521143456...@kernel.org/T/#t

Anup Patel

unread,
May 24, 2022, 11:59:17 AM5/24/22
to Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Atish Patra, linux-riscv, linux-kernel@vger.kernel.org List, kasa...@googlegroups.com
On Sat, May 21, 2022 at 8:13 PM Jisheng Zhang <jsz...@kernel.org> wrote:
>
> On a specific HW platform, pgtable_l4|[l5]_enabled won't change after
> boot, and the check sits at hot code path, this characteristic makes it
> suitable for optimization with static key.
>
> _pgtable_l4|[l5]_enabled is used very early during boot, even is used
> with MMU off, so the static key mechanism isn't ready. For this case,
> we use another static key _pgtable_lx_ready to indicate whether we
> have finalised pgtable_l4|[l5]_enabled or not, then fall back to
> _pgtable_l4|[l5]_enabled_early bool.
>
> Signed-off-by: Jisheng Zhang <jsz...@kernel.org>

Looks good to me.

Reviewed-by: Anup Patel <an...@brainfault.org>

Regards,
Anup

Anup Patel

unread,
May 24, 2022, 12:00:47 PM5/24/22
to Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Atish Patra, linux-riscv, linux-kernel@vger.kernel.org List, kasa...@googlegroups.com
On Tue, May 24, 2022 at 9:06 PM Jisheng Zhang <jsz...@kernel.org> wrote:
>
> On Mon, May 23, 2022 at 09:32:46PM +0530, Anup Patel wrote:
> > On Thu, May 19, 2022 at 9:38 PM Jisheng Zhang <jsz...@kernel.org> wrote:
> > >
> > > On a specific HW platform, pgtable_l4|[l5]_enabled won't change after
> > > boot, and the check sits at hot code path, this characteristic makes it
> > > suitable for optimization with static key.
> > >
> > > _pgtable_l4|[l5]_enabled is used very early during boot, even is used
> > > with MMU off, so the static key mechanism isn't ready. For this case,
> > > we use another static key _pgtable_lx_ready to indicate whether we
> > > have finalised pgtable_l4|[l5]_enabled or not, then fall back to
> > > _pgtable_l4|[l5]_enabled_early bool.
> > >
> > > Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
> >
> > Overall, this patch looks good to me. Please fix the errors reported by
> > autobuilders.
> >
> > Reviewed-by: Anup Patel <an...@brainfault.org>
>
> Thank Anup. I sent the v4 two days ago
> https://lore.kernel.org/linux-riscv/20220521143456...@kernel.org/T/#t

Sorry, I missed your v4.

I have provided Reviewed-by to your v4 as well.

Regards,
Anup

Jisheng Zhang

unread,
Jun 5, 2022, 2:20:52 AM6/5/22
to Anup Patel, Palmer Dabbelt, Paul Walmsley, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Atish Patra, linux-riscv, linux-kernel@vger.kernel.org List, kasa...@googlegroups.com
On Tue, May 24, 2022 at 09:29:04PM +0530, Anup Patel wrote:
> On Sat, May 21, 2022 at 8:13 PM Jisheng Zhang <jsz...@kernel.org> wrote:
> >
> > On a specific HW platform, pgtable_l4|[l5]_enabled won't change after
> > boot, and the check sits at hot code path, this characteristic makes it
> > suitable for optimization with static key.
> >
> > _pgtable_l4|[l5]_enabled is used very early during boot, even is used
> > with MMU off, so the static key mechanism isn't ready. For this case,
> > we use another static key _pgtable_lx_ready to indicate whether we
> > have finalised pgtable_l4|[l5]_enabled or not, then fall back to
> > _pgtable_l4|[l5]_enabled_early bool.
> >
> > Signed-off-by: Jisheng Zhang <jsz...@kernel.org>

Hi Palmer,

This series is missing for riscv v5.19 part2. Or Is there anything I
can do to improve the series?

Thanks in advance

Anup Patel

unread,
Jun 26, 2022, 12:32:30 AM6/26/22
to Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Atish Patra, linux-riscv, linux-kernel@vger.kernel.org List, kasa...@googlegroups.com, Sunil V L
On Sat, May 21, 2022 at 8:13 PM Jisheng Zhang <jsz...@kernel.org> wrote:
>
We are seeing a similar crash when booting kernel via EDK2 with RNG enabled.

Shell> fs0:\Image root=/dev/vda2 rootwait console=ttyS0
earlycon=uart8250,mmio,0x10000000 initrd=\initramfs.cp
EFI stub: Booting Linux Kernel...
EFI stub: Using DTB from configuration table
EFI stub: Exiting boot services...
[ 0.000000] Linux version 5.19.0-rc3 (oe-user@oe-host)
(riscv64-unknown-linux-gnu-gcc (Ventana-2022.05.16) 12.1.0, GNU ld
(Ventana-2022.05.16) 2.37.90.20220201) #1 SMP Thu Jun 23 05:33:13 UTC
2022
[ 0.000000] OF: fdt: Ignoring memory range 0x80000000 - 0x81200000
[ 0.000000] earlycon: uart8250 at MMIO 0x0000000010000000 (options '')
[ 0.000000] printk: bootconsole [uart8250] enabled
[ 0.000000] efi: EFI v2.70 by EDK II
[ 0.000000] efi: RNG=0xff94fd98 MEMRESERVE=0xfe658f18
[ 0.000000] efi: seeding entropy pool
[ 0.000000] Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000000
[ 0.000000] Oops [#1]
[ 0.000000] Modules linked in:
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc3 #1
[ 0.000000] epc : 0x0
[ 0.000000] ra : sbi_remote_fence_i+0x1e/0x26
[ 0.000000] epc : 0000000000000000 ra : ffffffff800080f8 sp :
ffffffff81203cd0
[ 0.000000] gp : ffffffff812f1d40 tp : ffffffff8120da80 t0 :
0000000000cb8266
[ 0.000000] t1 : 000000006d5e5146 t2 : 0000000058000000 s0 :
ffffffff81203ce0
[ 0.000000] s1 : ffffffff8047586a a0 : 0000000000000000 a1 :
0000000000000000
[ 0.000000] a2 : 0000000000000000 a3 : 0000000000000000 a4 :
0000000000000000
[ 0.000000] a5 : 0000000000000000 a6 : 0000000000000000 a7 :
0000000000000000
[ 0.000000] s2 : ffffffff80dea320 s3 : ffffffff80deabb0 s4 :
ffffffff81353d48
[ 0.000000] s5 : 0000000000000001 s6 : 00000000fffde848 s7 :
0000000000000004
[ 0.000000] s8 : 0000000081021714 s9 : 000000008101e6f0 s10:
00000000fffde780
[ 0.000000] s11: 0000000000000004 t3 : 000000001467a415 t4 :
0000000000000000
[ 0.000000] t5 : 00000000007627e0 t6 : ffffffffbc865574
[ 0.000000] status: 0000000200000100 badaddr: 0000000000000000
cause: 000000000000000c
[ 0.000000] ---[ end trace 0000000000000000 ]---
[ 0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
[ 0.000000] ---[ end Kernel panic - not syncing: Attempted to kill
the idle task! ]---

This patch fixes the above crash as well.

Reviewed-by: Anup Patel <an...@brainfault.org>

Thanks,
Anup

Anup Patel

unread,
Jun 26, 2022, 12:33:21 AM6/26/22
to Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Atish Patra, linux-riscv, linux-kernel@vger.kernel.org List, kasa...@googlegroups.com
On Sat, May 21, 2022 at 8:13 PM Jisheng Zhang <jsz...@kernel.org> wrote:
>
I have tested both these patches on QEMU RV64 and RV32.

Tested-by: Anup Patel <an...@brainfault.org>

Thanks,
Anup

>

Atish Patra

unread,
Jun 27, 2022, 2:47:46 AM6/27/22
to Anup Patel, Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Atish Patra, linux-riscv, linux-kernel@vger.kernel.org List, kasan-dev, Sunil V L
Thanks for the confirmation.

> Reviewed-by: Anup Patel <an...@brainfault.org>
>
> Thanks,
> Anup
>
> > ---
> > arch/riscv/kernel/setup.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > index 834eb652a7b9..d150cedeb7e0 100644
> > --- a/arch/riscv/kernel/setup.c
> > +++ b/arch/riscv/kernel/setup.c
> > @@ -268,6 +268,7 @@ void __init setup_arch(char **cmdline_p)
> > *cmdline_p = boot_command_line;
> >
> > early_ioremap_setup();
> > + sbi_init();
> > jump_label_init();
> > parse_early_param();
> >
> > @@ -284,7 +285,6 @@ void __init setup_arch(char **cmdline_p)
> > misc_mem_init();
> >
> > init_resources();
> > - sbi_init();
> >
> > #ifdef CONFIG_KASAN
> > kasan_init();
> > --
> > 2.34.1
> >
>
> _______________________________________________
> linux-riscv mailing list
> linux...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


Reviewed-by: Atish Patra <ati...@rivosinc.com>

--
Regards,
Atish

Palmer Dabbelt

unread,
Jul 1, 2022, 11:48:27 PM7/1/22
to an...@brainfault.org, jsz...@kernel.org, Paul Walmsley, a...@eecs.berkeley.edu, ryabin...@gmail.com, gli...@google.com, andre...@gmail.com, dvy...@google.com, vincenzo...@arm.com, alexand...@canonical.com, Atish Patra, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
Thanks for testing these. Unfortunatly they're failing for me under my
kasan+sparsemem-vmemmap config, which looks like a defconfig with

CONFIG_KASAN=y
# CONFIG_FLATMEM_MANUAL is not set
CONFIG_SPARSEMEM_MANUAL=y
CONFIG_SPARSEMEM=y
# CONFIG_SPARSEMEM_VMEMMAP is not set

Nothing's really jumping out and I'm not sure that's a super compelling
configuration, but IIRC it's found a handful of issues before so I'm not
sure it's sane to just toss it.

I've put this all on the riscv-pgtable_static_key branch of
kernel.org/palmer/linux . If nobody has the time to look then I'll try
and give it another shot, but I'm pretty buried right now so happy to
have the help.

Jisheng Zhang

unread,
Jul 15, 2022, 9:44:44 AM7/15/22
to Palmer Dabbelt, an...@brainfault.org, Paul Walmsley, a...@eecs.berkeley.edu, ryabin...@gmail.com, gli...@google.com, andre...@gmail.com, dvy...@google.com, vincenzo...@arm.com, alexand...@canonical.com, Atish Patra, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
Hi Palmer,

Thank you for the hint, I find the reason: SPARSEMEM is the key, KASAN
doesn't matter. To fix this issue, we need to move
riscv_finalise_pgtable_lx() after sparse_init(). I will send out a
newer version soon.

>
> Nothing's really jumping out and I'm not sure that's a super compelling
> configuration, but IIRC it's found a handful of issues before so I'm not
> sure it's sane to just toss it.
>
> I've put this all on the riscv-pgtable_static_key branch of
> kernel.org/palmer/linux . If nobody has the time to look then I'll try and
> give it another shot, but I'm pretty buried right now so happy to have the
> help.

Let me know if you want a seperate patch against
riscv-pgtable_static_key branch.

Thanks

Jisheng Zhang

unread,
Jul 15, 2022, 9:57:53 AM7/15/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
The pgtable_l4|[l5]_enabled check sits at hot code path, performance
is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
boot, so static key can be used to solve the performance issue[1].

An unified way static key was introduced in [2], but it only targets
riscv isa extension. We dunno whether SV48 and SV57 will be considered
as isa extension, so the unified solution isn't used for
pgtable_l4[l5]_enabled now.

patch1 fixes a NULL pointer deference if static key is used a bit earlier.
patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.

[1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
[2] https://lore.kernel.org/linux-riscv/20220517184453...@kernel.org/T/#t

Since v4:
- rebased on v5.19-rcN
- collect Reviewed-by tags
- Fix kernel panic issue if SPARSEMEM is enabled by moving the
riscv_finalise_pgtable_lx() after sparse_init()

Since v3:
- fix W=1 call to undeclared function 'static_branch_likely' error

Since v2:
- move the W=1 warning fix to a separate patch
- move the unified way to use static key to a new patch series.

Since v1:
- Add a W=1 warning fix
- Fix W=1 error
- Based on v5.18-rcN, since SV57 support is added, so convert
pgtable_l5_enabled as well.


Jisheng Zhang (2):
riscv: move sbi_init() earlier before jump_label_init()
riscv: turn pgtable_l4|[l5]_enabled to static key for RV64

Jisheng Zhang

unread,
Jul 15, 2022, 9:57:57 AM7/15/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Anup Patel, Atish Patra
Reviewed-by: Anup Patel <an...@brainfault.org>
Reviewed-by: Atish Patra <ati...@rivosinc.com>
---
arch/riscv/kernel/setup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index f0f36a4a0e9b..f5762f7b982d 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -269,6 +269,7 @@ void __init setup_arch(char **cmdline_p)
*cmdline_p = boot_command_line;

early_ioremap_setup();
+ sbi_init();
jump_label_init();
parse_early_param();

@@ -285,7 +286,6 @@ void __init setup_arch(char **cmdline_p)

Jisheng Zhang

unread,
Jul 15, 2022, 9:58:01 AM7/15/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Anup Patel
On a specific HW platform, pgtable_l4|[l5]_enabled won't change after
boot, and the check sits at hot code path, this characteristic makes it
suitable for optimization with static key.

_pgtable_l4|[l5]_enabled is used very early during boot, even is used
with MMU off, so the static key mechanism isn't ready. For this case,
we use another static key _pgtable_lx_ready to indicate whether we
have finalised pgtable_l4|[l5]_enabled or not, then fall back to
_pgtable_l4|[l5]_enabled_early bool.

Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
Reviewed-by: Anup Patel <an...@brainfault.org>
---
arch/riscv/include/asm/pgalloc.h | 16 ++++----
arch/riscv/include/asm/pgtable-32.h | 3 ++
arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
arch/riscv/include/asm/pgtable.h | 5 +--
arch/riscv/kernel/cpu.c | 4 +-
arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
arch/riscv/mm/kasan_init.c | 16 ++++----
index 59ba1fbaf784..1ef52079179a 100644
--- a/arch/riscv/include/asm/pgtable-32.h
+++ b/arch/riscv/include/asm/pgtable-32.h
@@ -17,6 +17,9 @@

#define MAX_POSSIBLE_PHYSMEM_BITS 34

+#define pgtable_l5_enabled() 0
+#define pgtable_l4_enabled() 0
+
/*
* rv32 PTE format:
* | XLEN-1 10 | 9 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 5c2aba5efbd0..edfff00d8ca3 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -8,18 +8,38 @@

#include <linux/bits.h>
#include <linux/const.h>
+#include <linux/jump_label.h>
#include <asm/errata_list.h>
@@ -191,7 +211,7 @@ static inline struct page *pud_page(pud_t pud)
#define mm_p4d_folded mm_p4d_folded
static inline bool mm_p4d_folded(struct mm_struct *mm)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return false;

return true;
@@ -200,7 +220,7 @@ static inline bool mm_p4d_folded(struct mm_struct *mm)
#define mm_pud_folded mm_pud_folded
static inline bool mm_pud_folded(struct mm_struct *mm)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return false;

return true;
@@ -235,7 +255,7 @@ static inline unsigned long _pmd_pfn(pmd_t pmd)

static inline void set_p4d(p4d_t *p4dp, p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
*p4dp = p4d;
else
set_pud((pud_t *)p4dp, (pud_t){ p4d_val(p4d) });
@@ -243,7 +263,7 @@ static inline void set_p4d(p4d_t *p4dp, p4d_t p4d)

static inline int p4d_none(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (p4d_val(p4d) == 0);

return 0;
@@ -251,7 +271,7 @@ static inline int p4d_none(p4d_t p4d)

static inline int p4d_present(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (p4d_val(p4d) & _PAGE_PRESENT);

return 1;
@@ -259,7 +279,7 @@ static inline int p4d_present(p4d_t p4d)

static inline int p4d_bad(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return !p4d_present(p4d);

return 0;
@@ -267,7 +287,7 @@ static inline int p4d_bad(p4d_t p4d)

static inline void p4d_clear(p4d_t *p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
set_p4d(p4d, __p4d(0));
}

@@ -283,7 +303,7 @@ static inline unsigned long _p4d_pfn(p4d_t p4d)

static inline pud_t *p4d_pgtable(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (pud_t *)pfn_to_virt(p4d_val(p4d) >> _PAGE_PFN_SHIFT);

return (pud_t *)pud_pgtable((pud_t) { p4d_val(p4d) });
@@ -300,7 +320,7 @@ static inline struct page *p4d_page(p4d_t p4d)
#define pud_offset pud_offset
static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return p4d_pgtable(*p4d) + pud_index(address);

return (pud_t *)p4d;
@@ -308,7 +328,7 @@ static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)

static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
*pgdp = pgd;
else
set_p4d((p4d_t *)pgdp, (p4d_t){ pgd_val(pgd) });
@@ -316,7 +336,7 @@ static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)

static inline int pgd_none(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (pgd_val(pgd) == 0);

return 0;
@@ -324,7 +344,7 @@ static inline int pgd_none(pgd_t pgd)

static inline int pgd_present(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (pgd_val(pgd) & _PAGE_PRESENT);

return 1;
@@ -332,7 +352,7 @@ static inline int pgd_present(pgd_t pgd)

static inline int pgd_bad(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return !pgd_present(pgd);

return 0;
@@ -340,13 +360,13 @@ static inline int pgd_bad(pgd_t pgd)

static inline void pgd_clear(pgd_t *pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
set_pgd(pgd, __pgd(0));
}

static inline p4d_t *pgd_pgtable(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (p4d_t *)pfn_to_virt(pgd_val(pgd) >> _PAGE_PFN_SHIFT);

return (p4d_t *)p4d_pgtable((p4d_t) { pgd_val(pgd) });
@@ -364,7 +384,7 @@ static inline struct page *pgd_page(pgd_t pgd)
#define p4d_offset p4d_offset
static inline p4d_t *p4d_offset(pgd_t *pgd, unsigned long address)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return pgd_pgtable(*pgd) + p4d_index(address);

return (p4d_t *)pgd;
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 1d1be9d9419c..3eaa01d880b9 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -63,8 +63,8 @@
* position vmemmap directly below the VMALLOC region.
*/
#ifdef CONFIG_64BIT
-#define VA_BITS (pgtable_l5_enabled ? \
- 57 : (pgtable_l4_enabled ? 48 : 39))
+#define VA_BITS (pgtable_l5_enabled() ? \
+ 57 : (pgtable_l4_enabled() ? 48 : 39))
#else
#define VA_BITS 32
#endif
@@ -834,7 +834,6 @@ extern uintptr_t _dtb_early_pa;
#define dtb_early_pa _dtb_early_pa
#endif /* CONFIG_XIP_KERNEL */
extern u64 satp_mode;
-extern bool pgtable_l4_enabled;

void paging_init(void);
void misc_mem_init(void);
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index fba9e9f46a8c..9b3697a97e41 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -143,9 +143,9 @@ static void print_mmu(struct seq_file *f)
#if defined(CONFIG_32BIT)
strncpy(sv_type, "sv32", 5);
#elif defined(CONFIG_64BIT)
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
strncpy(sv_type, "sv57", 5);
- else if (pgtable_l4_enabled)
+ else if (pgtable_l4_enabled())
strncpy(sv_type, "sv48", 5);
else
strncpy(sv_type, "sv39", 5);
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index d466ec670e1f..11708cdb7094 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -44,10 +44,16 @@ u64 satp_mode __ro_after_init = SATP_MODE_32;
#endif
EXPORT_SYMBOL(satp_mode);

-bool pgtable_l4_enabled = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
-bool pgtable_l5_enabled = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
-EXPORT_SYMBOL(pgtable_l4_enabled);
-EXPORT_SYMBOL(pgtable_l5_enabled);
+DEFINE_STATIC_KEY_FALSE(_pgtable_l4_enabled);
+DEFINE_STATIC_KEY_FALSE(_pgtable_l5_enabled);
+DEFINE_STATIC_KEY_FALSE(_pgtable_lx_ready);
+EXPORT_SYMBOL(_pgtable_l4_enabled);
+EXPORT_SYMBOL(_pgtable_l5_enabled);
+EXPORT_SYMBOL(_pgtable_lx_ready);
+bool _pgtable_l4_enabled_early = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
+bool _pgtable_l5_enabled_early = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
+EXPORT_SYMBOL(_pgtable_l4_enabled_early);
+EXPORT_SYMBOL(_pgtable_l5_enabled_early);

phys_addr_t phys_ram_base __ro_after_init;
EXPORT_SYMBOL(phys_ram_base);
@@ -585,26 +591,26 @@ static void __init create_p4d_mapping(p4d_t *p4dp,
@@ -710,14 +716,14 @@ static __init pgprot_t pgprot_from_va(uintptr_t va)
#if defined(CONFIG_64BIT) && !defined(CONFIG_XIP_KERNEL)
static void __init disable_pgtable_l5(void)
{
- pgtable_l5_enabled = false;
+ _pgtable_l5_enabled_early = false;
kernel_map.page_offset = PAGE_OFFSET_L4;
satp_mode = SATP_MODE_48;
}

static void __init disable_pgtable_l4(void)
{
- pgtable_l4_enabled = false;
+ _pgtable_l4_enabled_early = false;
kernel_map.page_offset = PAGE_OFFSET_L3;
satp_mode = SATP_MODE_39;
}
@@ -846,11 +852,11 @@ static void __init create_fdt_early_page_table(pgd_t *pgdir, uintptr_t dtb_pa)
PGDIR_SIZE,
IS_ENABLED(CONFIG_64BIT) ? PAGE_TABLE : PAGE_KERNEL);

- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(early_dtb_p4d, DTB_EARLY_BASE_VA,
(uintptr_t)early_dtb_pud, P4D_SIZE, PAGE_TABLE);

- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(early_dtb_pud, DTB_EARLY_BASE_VA,
(uintptr_t)early_dtb_pmd, PUD_SIZE, PAGE_TABLE);

@@ -992,11 +998,11 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)

#ifndef __PAGETABLE_PMD_FOLDED
/* Setup fixmap P4D and PUD */
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(fixmap_p4d, FIXADDR_START,
(uintptr_t)fixmap_pud, P4D_SIZE, PAGE_TABLE);
/* Setup fixmap PUD and PMD */
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(fixmap_pud, FIXADDR_START,
(uintptr_t)fixmap_pmd, PUD_SIZE, PAGE_TABLE);
create_pmd_mapping(fixmap_pmd, FIXADDR_START,
@@ -1004,10 +1010,10 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
/* Setup trampoline PGD and PMD */
create_pgd_mapping(trampoline_pg_dir, kernel_map.virt_addr,
trampoline_pgd_next, PGDIR_SIZE, PAGE_TABLE);
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(trampoline_p4d, kernel_map.virt_addr,
(uintptr_t)trampoline_pud, P4D_SIZE, PAGE_TABLE);
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(trampoline_pud, kernel_map.virt_addr,
(uintptr_t)trampoline_pmd, PUD_SIZE, PAGE_TABLE);
#ifdef CONFIG_XIP_KERNEL
@@ -1196,6 +1202,15 @@ static void __init reserve_crashkernel(void)
crashk_res.end = crash_base + crash_size - 1;
}

+static void __init riscv_finalise_pgtable_lx(void)
+{
+ if (_pgtable_l5_enabled_early)
+ static_branch_enable(&_pgtable_l5_enabled);
+ if (_pgtable_l4_enabled_early)
+ static_branch_enable(&_pgtable_l4_enabled);
+ static_branch_enable(&_pgtable_lx_ready);
+}
+
void __init paging_init(void)
{
setup_bootmem();
@@ -1207,6 +1222,7 @@ void __init misc_mem_init(void)
early_memtest(min_low_pfn << PAGE_SHIFT, max_low_pfn << PAGE_SHIFT);
arch_numa_init();
sparse_init();
+ riscv_finalise_pgtable_lx();
zone_sizes_init();
reserve_crashkernel();
memblock_dump_all();

Emil Renner Berthing

unread,
Jul 15, 2022, 11:05:07 AM7/15/22
to Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Anup Patel
It amounts to the same, but I wonder if we ought to use the
DECLARE_STATIC_KEY_FALSE macro here.

Jisheng Zhang

unread,
Jul 15, 2022, 1:02:18 PM7/15/22
to Emil Renner Berthing, Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Anup Patel
Thanks for the hint, will send out a newer version soon. Before
that, I will wait a bit for other review feedbacks.

Thanks

Jisheng Zhang

unread,
Jul 16, 2022, 8:00:01 AM7/16/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Emil Renner Berthing, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
The pgtable_l4|[l5]_enabled check sits at hot code path, performance
is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
boot, so static key can be used to solve the performance issue[1].

An unified way static key was introduced in [2], but it only targets
riscv isa extension. We dunno whether SV48 and SV57 will be considered
as isa extension, so the unified solution isn't used for
pgtable_l4[l5]_enabled now.

patch1 fixes a NULL pointer deference if static key is used a bit earlier.
patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.

[1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
[2] https://lore.kernel.org/linux-riscv/20220517184453...@kernel.org/T/#t

Since v5:
- Use DECLARE_STATIC_KEY_FALSE

Since v4:
- rebased on v5.19-rcN
- collect Reviewed-by tags
- Fix kernel panic issue if SPARSEMEM is enabled by moving the
riscv_finalise_pgtable_lx() after sparse_init()

Since v3:
- fix W=1 call to undeclared function 'static_branch_likely' error

Since v2:
- move the W=1 warning fix to a separate patch
- move the unified way to use static key to a new patch series.

Since v1:
- Add a W=1 warning fix
- Fix W=1 error
- Based on v5.18-rcN, since SV57 support is added, so convert
pgtable_l5_enabled as well.

Jisheng Zhang (2):
riscv: move sbi_init() earlier before jump_label_init()
riscv: turn pgtable_l4|[l5]_enabled to static key for RV64

arch/riscv/include/asm/pgalloc.h | 16 ++++----
arch/riscv/include/asm/pgtable-32.h | 3 ++
arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
arch/riscv/include/asm/pgtable.h | 5 +--
arch/riscv/kernel/cpu.c | 4 +-
arch/riscv/kernel/setup.c | 2 +-
arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
arch/riscv/mm/kasan_init.c | 16 ++++----

Jisheng Zhang

unread,
Jul 16, 2022, 8:00:04 AM7/16/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Emil Renner Berthing, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Anup Patel, Atish Patra
Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
Reviewed-by: Anup Patel <an...@brainfault.org>

Jisheng Zhang

unread,
Jul 16, 2022, 8:00:07 AM7/16/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Emil Renner Berthing, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Anup Patel
On a specific HW platform, pgtable_l4|[l5]_enabled won't change after
boot, and the check sits at hot code path, this characteristic makes it
suitable for optimization with static key.

_pgtable_l4|[l5]_enabled is used very early during boot, even is used
with MMU off, so the static key mechanism isn't ready. For this case,
we use another static key _pgtable_lx_ready to indicate whether we
have finalised pgtable_l4|[l5]_enabled or not, then fall back to
_pgtable_l4|[l5]_enabled_early bool.

Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
Reviewed-by: Anup Patel <an...@brainfault.org>
---
arch/riscv/include/asm/pgalloc.h | 16 ++++----
arch/riscv/include/asm/pgtable-32.h | 3 ++
arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
arch/riscv/include/asm/pgtable.h | 5 +--
arch/riscv/kernel/cpu.c | 4 +-
arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
arch/riscv/mm/kasan_init.c | 16 ++++----
index 5c2aba5efbd0..baab8e6bec01 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -8,18 +8,38 @@

#include <linux/bits.h>
#include <linux/const.h>
+#include <linux/jump_label.h>
#include <asm/errata_list.h>

-extern bool pgtable_l4_enabled;
-extern bool pgtable_l5_enabled;
+extern bool _pgtable_l5_enabled_early;
+extern bool _pgtable_l4_enabled_early;
+DECLARE_STATIC_KEY_FALSE(_pgtable_l5_enabled);
+DECLARE_STATIC_KEY_FALSE(_pgtable_l4_enabled);
+DECLARE_STATIC_KEY_FALSE(_pgtable_lx_ready);
+
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return false;

return true;
@@ -200,7 +220,7 @@ static inline bool mm_p4d_folded(struct mm_struct *mm)
#define mm_pud_folded mm_pud_folded
static inline bool mm_pud_folded(struct mm_struct *mm)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return false;

return true;
@@ -235,7 +255,7 @@ static inline unsigned long _pmd_pfn(pmd_t pmd)

static inline void set_p4d(p4d_t *p4dp, p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
*p4dp = p4d;
else
set_pud((pud_t *)p4dp, (pud_t){ p4d_val(p4d) });
@@ -243,7 +263,7 @@ static inline void set_p4d(p4d_t *p4dp, p4d_t p4d)

static inline int p4d_none(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (p4d_val(p4d) == 0);

return 0;
@@ -251,7 +271,7 @@ static inline int p4d_none(p4d_t p4d)

static inline int p4d_present(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (p4d_val(p4d) & _PAGE_PRESENT);

return 1;
@@ -259,7 +279,7 @@ static inline int p4d_present(p4d_t p4d)

static inline int p4d_bad(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return !p4d_present(p4d);

return 0;
@@ -267,7 +287,7 @@ static inline int p4d_bad(p4d_t p4d)

static inline void p4d_clear(p4d_t *p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
set_p4d(p4d, __p4d(0));
}

@@ -283,7 +303,7 @@ static inline unsigned long _p4d_pfn(p4d_t p4d)

static inline pud_t *p4d_pgtable(p4d_t p4d)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return (pud_t *)pfn_to_virt(p4d_val(p4d) >> _PAGE_PFN_SHIFT);

return (pud_t *)pud_pgtable((pud_t) { p4d_val(p4d) });
@@ -300,7 +320,7 @@ static inline struct page *p4d_page(p4d_t p4d)
#define pud_offset pud_offset
static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)
{
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
return p4d_pgtable(*p4d) + pud_index(address);

return (pud_t *)p4d;
@@ -308,7 +328,7 @@ static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)

static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
*pgdp = pgd;
else
set_p4d((p4d_t *)pgdp, (p4d_t){ pgd_val(pgd) });
@@ -316,7 +336,7 @@ static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)

static inline int pgd_none(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (pgd_val(pgd) == 0);

return 0;
@@ -324,7 +344,7 @@ static inline int pgd_none(pgd_t pgd)

static inline int pgd_present(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (pgd_val(pgd) & _PAGE_PRESENT);

return 1;
@@ -332,7 +352,7 @@ static inline int pgd_present(pgd_t pgd)

static inline int pgd_bad(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return !pgd_present(pgd);

return 0;
@@ -340,13 +360,13 @@ static inline int pgd_bad(pgd_t pgd)

static inline void pgd_clear(pgd_t *pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
set_pgd(pgd, __pgd(0));
}

static inline p4d_t *pgd_pgtable(pgd_t pgd)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return (p4d_t *)pfn_to_virt(pgd_val(pgd) >> _PAGE_PFN_SHIFT);

return (p4d_t *)p4d_pgtable((p4d_t) { pgd_val(pgd) });
@@ -364,7 +384,7 @@ static inline struct page *pgd_page(pgd_t pgd)
#define p4d_offset p4d_offset
static inline p4d_t *p4d_offset(pgd_t *pgd, unsigned long address)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(early_dtb_p4d, DTB_EARLY_BASE_VA,
(uintptr_t)early_dtb_pud, P4D_SIZE, PAGE_TABLE);

- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(early_dtb_pud, DTB_EARLY_BASE_VA,
(uintptr_t)early_dtb_pmd, PUD_SIZE, PAGE_TABLE);

@@ -992,11 +998,11 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)

#ifndef __PAGETABLE_PMD_FOLDED
/* Setup fixmap P4D and PUD */
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(fixmap_p4d, FIXADDR_START,
(uintptr_t)fixmap_pud, P4D_SIZE, PAGE_TABLE);
/* Setup fixmap PUD and PMD */
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(fixmap_pud, FIXADDR_START,
(uintptr_t)fixmap_pmd, PUD_SIZE, PAGE_TABLE);
create_pmd_mapping(fixmap_pmd, FIXADDR_START,
@@ -1004,10 +1010,10 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
/* Setup trampoline PGD and PMD */
create_pgd_mapping(trampoline_pg_dir, kernel_map.virt_addr,
trampoline_pgd_next, PGDIR_SIZE, PAGE_TABLE);
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(trampoline_p4d, kernel_map.virt_addr,
(uintptr_t)trampoline_pud, P4D_SIZE, PAGE_TABLE);
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
- if (pgtable_l4_enabled) {
+ if (pgtable_l4_enabled()) {
for (i = 0; i < PTRS_PER_PUD; ++i)
set_pud(kasan_early_shadow_pud + i,
pfn_pud(PFN_DOWN
@@ -282,7 +282,7 @@ asmlinkage void __init kasan_early_init(void)
PAGE_TABLE));
}

- if (pgtable_l5_enabled) {
+ if (pgtable_l5_enabled()) {

Jisheng Zhang

unread,
Aug 13, 2022, 10:02:46 PM8/13/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, Emil Renner Berthing, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
On Sat, Jul 16, 2022 at 07:50:57PM +0800, Jisheng Zhang wrote:
> The pgtable_l4|[l5]_enabled check sits at hot code path, performance
> is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
> boot, so static key can be used to solve the performance issue[1].
>
> An unified way static key was introduced in [2], but it only targets
> riscv isa extension. We dunno whether SV48 and SV57 will be considered
> as isa extension, so the unified solution isn't used for
> pgtable_l4[l5]_enabled now.
>
> patch1 fixes a NULL pointer deference if static key is used a bit earlier.
> patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.
>
> [1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
> [2] https://lore.kernel.org/linux-riscv/20220517184453...@kernel.org/T/#t

Hi Palmer,

I see part1 and part2 were sent out...
What I can do to make this series merged for 6.0-rc1? I'm afraid this series
may miss anothe round of merge window again.

Thanks in advance

Jisheng Zhang

unread,
Aug 21, 2022, 10:18:33 AM8/21/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
The pgtable_l4|[l5]_enabled check sits at hot code path, performance
is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
boot, so static key can be used to solve the performance issue[1].

An unified way static key was introduced in [2], but it only targets
riscv isa extension. We dunno whether SV48 and SV57 will be considered
as isa extension, so the unified solution isn't used for
pgtable_l4[l5]_enabled now.

patch1 fixes a NULL pointer deference if static key is used a bit earlier.
patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.

[1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
[2] https://lore.kernel.org/linux-riscv/20220517184453...@kernel.org/T/#t

Jisheng Zhang

unread,
Aug 21, 2022, 10:18:36 AM8/21/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Anup Patel, Atish Patra
Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
Reviewed-by: Anup Patel <an...@brainfault.org>
Reviewed-by: Atish Patra <ati...@rivosinc.com>
---
arch/riscv/kernel/setup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 95ef6e2bf45c..19ead6877c16 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -270,6 +270,7 @@ void __init setup_arch(char **cmdline_p)
*cmdline_p = boot_command_line;

early_ioremap_setup();
+ sbi_init();
jump_label_init();
parse_early_param();

@@ -286,7 +287,6 @@ void __init setup_arch(char **cmdline_p)

Jisheng Zhang

unread,
Aug 21, 2022, 10:18:39 AM8/21/22
to Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Alexandre Ghiti, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Anup Patel
On a specific HW platform, pgtable_l4|[l5]_enabled won't change after
boot, and the check sits at hot code path, this characteristic makes it
suitable for optimization with static key.

_pgtable_l4|[l5]_enabled is used very early during boot, even is used
with MMU off, so the static key mechanism isn't ready. For this case,
we use another static key _pgtable_lx_ready to indicate whether we
have finalised pgtable_l4|[l5]_enabled or not, then fall back to
_pgtable_l4|[l5]_enabled_early bool.

Signed-off-by: Jisheng Zhang <jsz...@kernel.org>
Reviewed-by: Anup Patel <an...@brainfault.org>
---
arch/riscv/include/asm/pgalloc.h | 16 ++++----
arch/riscv/include/asm/pgtable-32.h | 3 ++
arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
arch/riscv/include/asm/pgtable.h | 5 +--
arch/riscv/kernel/cpu.c | 4 +-
arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
arch/riscv/mm/kasan_init.c | 16 ++++----
index dc42375c2357..ef182aa785d5 100644
return (pud_t *)pfn_to_virt(__page_val_to_pfn(p4d_val(p4d)));
return (p4d_t *)pfn_to_virt(__page_val_to_pfn(pgd_val(pgd)));

return (p4d_t *)p4d_pgtable((p4d_t) { pgd_val(pgd) });
@@ -364,7 +384,7 @@ static inline struct page *pgd_page(pgd_t pgd)
#define p4d_offset p4d_offset
static inline p4d_t *p4d_offset(pgd_t *pgd, unsigned long address)
{
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
return pgd_pgtable(*pgd) + p4d_index(address);

return (p4d_t *)pgd;
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 7ec936910a96..daf2475b6f43 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -63,8 +63,8 @@
* position vmemmap directly below the VMALLOC region.
*/
#ifdef CONFIG_64BIT
-#define VA_BITS (pgtable_l5_enabled ? \
- 57 : (pgtable_l4_enabled ? 48 : 39))
+#define VA_BITS (pgtable_l5_enabled() ? \
+ 57 : (pgtable_l4_enabled() ? 48 : 39))
#else
#define VA_BITS 32
#endif
@@ -814,7 +814,6 @@ extern uintptr_t _dtb_early_pa;
#define dtb_early_pa _dtb_early_pa
#endif /* CONFIG_XIP_KERNEL */
extern u64 satp_mode;
-extern bool pgtable_l4_enabled;

void paging_init(void);
void misc_mem_init(void);
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 0be8a2403212..8e2fae6cad34 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -150,9 +150,9 @@ static void print_mmu(struct seq_file *f)
#if defined(CONFIG_32BIT)
strncpy(sv_type, "sv32", 5);
#elif defined(CONFIG_64BIT)
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
strncpy(sv_type, "sv57", 5);
- else if (pgtable_l4_enabled)
+ else if (pgtable_l4_enabled())
strncpy(sv_type, "sv48", 5);
else
strncpy(sv_type, "sv39", 5);
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index b56a0a75533f..713831f12fe2 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -44,10 +44,16 @@ u64 satp_mode __ro_after_init = SATP_MODE_32;
#endif
EXPORT_SYMBOL(satp_mode);

-bool pgtable_l4_enabled = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
-bool pgtable_l5_enabled = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
-EXPORT_SYMBOL(pgtable_l4_enabled);
-EXPORT_SYMBOL(pgtable_l5_enabled);
+DEFINE_STATIC_KEY_FALSE(_pgtable_l4_enabled);
+DEFINE_STATIC_KEY_FALSE(_pgtable_l5_enabled);
+DEFINE_STATIC_KEY_FALSE(_pgtable_lx_ready);
+EXPORT_SYMBOL(_pgtable_l4_enabled);
+EXPORT_SYMBOL(_pgtable_l5_enabled);
+EXPORT_SYMBOL(_pgtable_lx_ready);
+bool _pgtable_l4_enabled_early = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
+bool _pgtable_l5_enabled_early = IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_XIP_KERNEL);
+EXPORT_SYMBOL(_pgtable_l4_enabled_early);
+EXPORT_SYMBOL(_pgtable_l5_enabled_early);

phys_addr_t phys_ram_base __ro_after_init;
EXPORT_SYMBOL(phys_ram_base);
@@ -609,26 +615,26 @@ static void __init create_p4d_mapping(p4d_t *p4dp,
@@ -734,14 +740,14 @@ static __init pgprot_t pgprot_from_va(uintptr_t va)
#if defined(CONFIG_64BIT) && !defined(CONFIG_XIP_KERNEL)
static void __init disable_pgtable_l5(void)
{
- pgtable_l5_enabled = false;
+ _pgtable_l5_enabled_early = false;
kernel_map.page_offset = PAGE_OFFSET_L4;
satp_mode = SATP_MODE_48;
}

static void __init disable_pgtable_l4(void)
{
- pgtable_l4_enabled = false;
+ _pgtable_l4_enabled_early = false;
kernel_map.page_offset = PAGE_OFFSET_L3;
satp_mode = SATP_MODE_39;
}
@@ -870,11 +876,11 @@ static void __init create_fdt_early_page_table(pgd_t *pgdir, uintptr_t dtb_pa)
PGDIR_SIZE,
IS_ENABLED(CONFIG_64BIT) ? PAGE_TABLE : PAGE_KERNEL);

- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(early_dtb_p4d, DTB_EARLY_BASE_VA,
(uintptr_t)early_dtb_pud, P4D_SIZE, PAGE_TABLE);

- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(early_dtb_pud, DTB_EARLY_BASE_VA,
(uintptr_t)early_dtb_pmd, PUD_SIZE, PAGE_TABLE);

@@ -1016,11 +1022,11 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)

#ifndef __PAGETABLE_PMD_FOLDED
/* Setup fixmap P4D and PUD */
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(fixmap_p4d, FIXADDR_START,
(uintptr_t)fixmap_pud, P4D_SIZE, PAGE_TABLE);
/* Setup fixmap PUD and PMD */
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(fixmap_pud, FIXADDR_START,
(uintptr_t)fixmap_pmd, PUD_SIZE, PAGE_TABLE);
create_pmd_mapping(fixmap_pmd, FIXADDR_START,
@@ -1028,10 +1034,10 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
/* Setup trampoline PGD and PMD */
create_pgd_mapping(trampoline_pg_dir, kernel_map.virt_addr,
trampoline_pgd_next, PGDIR_SIZE, PAGE_TABLE);
- if (pgtable_l5_enabled)
+ if (pgtable_l5_enabled())
create_p4d_mapping(trampoline_p4d, kernel_map.virt_addr,
(uintptr_t)trampoline_pud, P4D_SIZE, PAGE_TABLE);
- if (pgtable_l4_enabled)
+ if (pgtable_l4_enabled())
create_pud_mapping(trampoline_pud, kernel_map.virt_addr,
(uintptr_t)trampoline_pmd, PUD_SIZE, PAGE_TABLE);
#ifdef CONFIG_XIP_KERNEL
@@ -1220,6 +1226,15 @@ static void __init reserve_crashkernel(void)
crashk_res.end = crash_base + crash_size - 1;
}

+static void __init riscv_finalise_pgtable_lx(void)
+{
+ if (_pgtable_l5_enabled_early)
+ static_branch_enable(&_pgtable_l5_enabled);
+ if (_pgtable_l4_enabled_early)
+ static_branch_enable(&_pgtable_l4_enabled);
+ static_branch_enable(&_pgtable_lx_ready);
+}
+
void __init paging_init(void)
{
setup_bootmem();
@@ -1231,6 +1246,7 @@ void __init misc_mem_init(void)

Palmer Dabbelt

unread,
Oct 5, 2022, 9:05:33 PM10/5/22
to jsz...@kernel.org, Paul Walmsley, a...@eecs.berkeley.edu, ryabin...@gmail.com, gli...@google.com, andre...@gmail.com, dvy...@google.com, vincenzo...@arm.com, alexand...@canonical.com, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
Sorry for being slow here, but it looks like this still causes some
early boot hangs. Specifically kasan+sparsemem is failing. As you can
probably see from the latency I'm still a bit buried right now so I'm
not sure when I'll have a chance to take more of a look.

Jisheng Zhang

unread,
Oct 6, 2022, 4:47:28 AM10/6/22
to Palmer Dabbelt, Paul Walmsley, a...@eecs.berkeley.edu, ryabin...@gmail.com, gli...@google.com, andre...@gmail.com, dvy...@google.com, vincenzo...@arm.com, alexand...@canonical.com, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
Hi Palmer,

Before V4, there is a bug which can cause kernel panic when SPARSEMEM
is enabled, V4 have fixed it by moving the riscv_finalise_pgtable_lx()
after sparse_init(). And I just tested the riscv-pgtable_static_key
branch in your tree, enabling KASAN and SPARSEMEM, system booted fine.
I'm not sure what happened. Could you please send me your kernel
config file? I want to fix any issue which can block this series being
merged in 6.1-rc1.

Thanks in advance

Jisheng Zhang

unread,
Oct 8, 2022, 10:38:07 AM10/8/22
to Palmer Dabbelt, Paul Walmsley, a...@eecs.berkeley.edu, ryabin...@gmail.com, gli...@google.com, andre...@gmail.com, dvy...@google.com, vincenzo...@arm.com, alexand...@canonical.com, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
Hi Palmer,

I know you are busy ;) Do you have time to send me your test kernel
config file so that I can reproduce the "early boot hang"?

Thanks

Jisheng Zhang

unread,
Dec 11, 2022, 5:54:01 AM12/11/22
to Palmer Dabbelt, Paul Walmsley, a...@eecs.berkeley.edu, ryabin...@gmail.com, gli...@google.com, andre...@gmail.com, dvy...@google.com, vincenzo...@arm.com, alexand...@canonical.com, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
Hi Palmer,

I think the early boot hangs maybe the same as the one which has been
fixed by commit 9f2ac64d6ca6 ("riscv: mm: add missing memcpy in
kasan_init"). Will you give this series another try for v6.2-rc1? If
the boot hang can still be reproduced, could you please send me your
.config file?

Thanks in advance

Jisheng Zhang

unread,
Jan 11, 2023, 12:39:01 PM1/11/23
to Palmer Dabbelt, Paul Walmsley, a...@eecs.berkeley.edu, ryabin...@gmail.com, gli...@google.com, andre...@gmail.com, dvy...@google.com, vincenzo...@arm.com, alexand...@canonical.com, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
Hi all,

Just request to comment what to do with this patch, I think there
are two independent points to consult:

1. IIRC, Palmer gave this patch two chances to merge in early versions
but he found boot hangs if enable KASAN and SPARSEMEM, while I can't
reproduce the boot hang. And I also expect the hang should be fixed by
commit 9f2ac64d6ca6 ("riscv: mm: add missing memcpy in kasan_init")

2. Now we know alternative is preferred than static branch for ISA
extensions dynamic code patching. So we also need to switch static
branch usage here to alternative mechanism, but the problem is
SV48 and SV57 are not ISA extensions, so we can't directly make use
of the recently introduced riscv_has_extension_likely|unlikely()[1]
which is based on alternative mechanism.

Any comments are appreciated.

Thanks in advance

[1] https://lore.kernel.org/linux-riscv/20230111171027...@kernel.org/T/#t

Andrew Jones

unread,
Jan 11, 2023, 2:00:32 PM1/11/23
to Jisheng Zhang, Palmer Dabbelt, Paul Walmsley, a...@eecs.berkeley.edu, ryabin...@gmail.com, gli...@google.com, andre...@gmail.com, dvy...@google.com, vincenzo...@arm.com, alexand...@canonical.com, linux...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com
We could rename the "has_extension" framework to "has_cpufeature" and
then lump extensions and features such as sv48 and sv57 together. Or,
if it's best to keep extensions separate, then duplicate the framework
to create a "has_non_extension_feature" version where features like
sv48 and sv57 live.

Thanks,
drew

>
> Any comments are appreciated.
>
> Thanks in advance
>
> [1] https://lore.kernel.org/linux-riscv/20230111171027...@kernel.org/T/#t
>

Jessica Clarke

unread,
Jan 11, 2023, 3:51:57 PM1/11/23
to Andrew Jones, Jisheng Zhang, Palmer Dabbelt, Paul Walmsley, Albert Ou, ryabin...@gmail.com, gli...@google.com, andre...@gmail.com, dvy...@google.com, vincenzo...@arm.com, Alexandre Ghiti, linux-riscv, Linux Kernel Mailing List, kasa...@googlegroups.com
Sv39, Sv48 and Sv57 are extensions these days (see the draft profiles
spec[1] and [2]).

Jess

[1] https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc#522-rva20s64-mandatory-extensions
[2] https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions
Reply all
Reply to author
Forward
0 new messages