From: Jisheng Zhang <jszhang@kernel.org>
To: Conor Dooley <conor@kernel.org>
Cc: Paul Walmsley <pjw@kernel.org>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Alexandre Ghiti <alex@ghiti.fr>,
Andrey Ryabinin <ryabinin.a.a@gmail.com>,
Alexander Potapenko <glider@google.com>,
Andrey Konovalov <andreyknvl@gmail.com>,
Dmitry Vyukov <dvyukov@google.com>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
kasan-dev@googlegroups.com
Subject: Re: [PATCH v2 4/5] riscv: RISCV_ISA_SV48 and RISCV_ISA_SV57 under NONPORTABLE
Date: Wed, 9 Sep 2026 07:48:57 +0800 [thread overview]
Message-ID: <aqCe6T8QKtJcLHiM@xhacker> (raw)
In-Reply-To: <20260908-deflector-degree-ceca18036811@spud>
On Tue, Sep 08, 2026 at 02:07:27PM +0100, Conor Dooley wrote:
> On Tue, Sep 08, 2026 at 11:49:10AM +0800, Jisheng Zhang wrote:
> > On Mon, Sep 07, 2026 at 05:21:11PM +0100, Conor Dooley wrote:
> > > On Mon, Sep 07, 2026 at 11:14:36PM +0800, Jisheng Zhang wrote:
> > > > In last commit, we have optimized the pgtable_l4|l5_enabled with isa
> > > > extension alternative mechanism, thus a typical access_ok(addr, 1);
> > > > is compiled as:
> > > >
> > > > ffffffff8001dd30: 1141 addi sp,sp,-16
> > > > ffffffff8001dd32: e022 sd s0,0(sp)
> > > > ffffffff8001dd34: e406 sd ra,8(sp)
> > > > ffffffff8001dd36: 0800 addi s0,sp,16
> > > > ffffffff8001dd38: 00000013 nop
> > > > ffffffff8001dd3c: 02a0006f j ffffffff8001dd66 <foo+0x3e>
> > > > ffffffff8001dd40: 60a2 ld ra,8(sp)
> > > > ffffffff8001dd42: 6402 ld s0,0(sp)
> > > > ffffffff8001dd44: 57fd li a5,-1
> > > > ffffffff8001dd46: 83a1 srli a5,a5,0x8
> > > > ffffffff8001dd48: 00a7b533 sltu a0,a5,a0
> > > > ffffffff8001dd4c: 00154513 xori a0,a0,1
> > > > ffffffff8001dd50: 0141 addi sp,sp,16
> > > > ffffffff8001dd52: 8082 ret
> > > > ...
> > > > ffffffff8001dd64: bfe1 j ffffffff8001dd3c <foo+0x14>
> > > > ffffffff8001dd66: 0180006f j ffffffff8001dd7e <foo+0x56>
> > > > ffffffff8001dd6a: 60a2 ld ra,8(sp)
> > > > ffffffff8001dd6c: 6402 ld s0,0(sp)
> > > > ffffffff8001dd6e: 57fd li a5,-1
> > > > ffffffff8001dd70: 83c5 srli a5,a5,0x11
> > > > ffffffff8001dd72: 00a7b533 sltu a0,a5,a0
> > > > ffffffff8001dd76: 00154513 xori a0,a0,1
> > > > ffffffff8001dd7a: 0141 addi sp,sp,16
> > > > ffffffff8001dd7c: 8082 ret
> > > > ffffffff8001dd7e: 60a2 ld ra,8(sp)
> > > > ffffffff8001dd80: 6402 ld s0,0(sp)
> > > > ffffffff8001dd82: 57fd li a5,-1
> > > > ffffffff8001dd84: 83e9 srli a5,a5,0x1a
> > > > ffffffff8001dd86: 00a7b533 sltu a0,a5,a0
> > > > ffffffff8001dd8a: 00154513 xori a0,a0,1
> > > > ffffffff8001dd8e: 0141 addi sp,sp,16
> > > > ffffffff8001dd90: 8082 ret
> > > >
> > > > As can be seen, different branches for SV39/SV48/SV57 are still there,
> > > > since access_ok() sits at hot code path, why not reduce the code size
> > > > to optimize to make the instruction cache happy?
> > > >
> > > > Introduce RISCV_ISA_SV48 and RISCV_ISA_SV57 under NONPORTABLE, so that
> > > > the embedded platforms can choose the best option themselves, while
> > > > still keep the feature of unified one kernel Image for all SV39, SV48
> > > > and SV57.
> > > >
> > > > Before the patch, vmlinux built with pure RISCV64 defconfig
> > > > text data bss dec hex filename
> > > > 13439437 6790446 483221 20713104 13c0e90 /tmp/old/vmlinux
> > > >
> > > > After the patch, vmlinux built with pure RISCV64 defconfig:
> > > > text data bss dec hex filename
> > > > 13314445 6787934 466837 20569216 139dc80 /tmp/new/vmlinux
> > > >
> > > > .text section is reduced by 122KB!
> > > >
> > > > And after the patch, typical access_ok(addr, 1) is compiled as:
> > > > ffffffff8001809a: 1141 addi sp,sp,-16
> > > > ffffffff8001809c: e022 sd s0,0(sp)
> > > > ffffffff8001809e: e406 sd ra,8(sp)
> > > > ffffffff800180a0: 0800 addi s0,sp,16
> > > > ffffffff800180a2: 00000013 nop
> > > > ffffffff800180a6: 60a2 ld ra,8(sp)
> > > > ffffffff800180a8: 6402 ld s0,0(sp)
> > > > ffffffff800180aa: 4785 li a5,1
> > > > ffffffff800180ac: 179a slli a5,a5,0x26
> > > > ffffffff800180ae: 00f53533 sltu a0,a0,a5
> > > > ffffffff800180b2: 0141 addi sp,sp,16
> > > > ffffffff800180b4: 8082 ret
> > > >
> > > > only 12 instruction!! For refernece, before the patch, the access(addr,
> > > > 1) needs 32 instructions.
> > > >
> > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > > > ---
> > > > arch/riscv/Kconfig | 24 ++++++++++++++++++++++++
> > > > arch/riscv/include/asm/pgtable-64.h | 12 ++++++++++++
> > > > arch/riscv/mm/init.c | 10 ++++++++++
> > > > 3 files changed, 46 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > index e9476b8cbeb0..4cdb1e3f34be 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > > > @@ -602,6 +602,30 @@ config RISCV_ISA_SSQOSID
> > > > cache partitioning scheme and use the MCID to track how much
> > > > cache a process, or a group of processes, is using.
> > > >
> > > > +config RISCV_ISA_SV48
> > > > + bool "Page-Based 48-bit Virtual-Memory System"
> > > > + depends on 64BIT && NONPORTABLE
> > > > + default y
> > >
> > > I don't understand how you've got this set up here.
> > > If this is genuinely non-portable, then you should not set the default to
> > > enabled. However, I'm not really sure that this is actually non-portable
> > > at all....
> > >
> > > > + help
> > > > + Add support for the Page-Based 48-bit Virtual-Memory System.
> > > > +
> > > > + Enable this option for systems for which a 39-bit virtual address
> > > > + space is insufficient. But if 39-bit is enough, disabling this
> > > > + option will result in smaller kernel size and slighly better
> > > > + performance.
> > > > +
> > > > +config RISCV_ISA_SV57
> > > > + bool "Page-Based 57-bit Virtual-Memory System"
> > > > + depends on 64BIT && NONPORTABLE
> > > > + default y
> > > > + help
> > > > + Add support for the Page-Based 57-bit Virtual-Memory System.
> > > > +
> > > > + Enable this option for systems for which a 48-bit virtual address
> > > > + space is insufficient. But if 48-bit is enough, disabling this
> > > > + option will result in smaller kernel size and slighly better
> > > > + performance.
> > > > +
> > > > config RISCV_ISA_SVPBMT
> > > > bool "Svpbmt extension support for supervisor mode page-based memory types"
> > > > depends on 64BIT && MMU
> > > > diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> > > > index 7e30ec0e9f6c..f2d29da93eea 100644
> > > > --- a/arch/riscv/include/asm/pgtable-64.h
> > > > +++ b/arch/riscv/include/asm/pgtable-64.h
> > > > @@ -16,21 +16,33 @@ extern bool _pgtable_l5_enabled;
> > > > #ifdef USE_EARLY_PGTABLE_LEVELS
> > > > static __always_inline bool pgtable_l5_enabled(void)
> > > > {
> > > > + if (IS_ENABLED(CONFIG_NONPORTABLE) && !IS_ENABLED(CONFIG_RISCV_ISA_SV57))
> > > > + return false;
> > > > +
> > > > return _pgtable_l5_enabled;
> > > > }
> > > >
> > > > static __always_inline bool pgtable_l4_enabled(void)
> > > > {
> > > > + if (IS_ENABLED(CONFIG_NONPORTABLE) && !IS_ENABLED(CONFIG_RISCV_ISA_SV48))
> > > > + return false;
> > > > +
> > > > return _pgtable_l4_enabled;
> > > > }
> > > > #else
> > > > static __always_inline bool pgtable_l4_enabled(void)
> > > > {
> > > > + if (IS_ENABLED(CONFIG_NONPORTABLE) && !IS_ENABLED(CONFIG_RISCV_ISA_SV48))
> > > > + return false;
> > > > +
> > > > return riscv_has_extension_likely(RISCV_ISA_EXT_SV48);
> > > > }
> > > >
> > > > static __always_inline bool pgtable_l5_enabled(void)
> > > > {
> > > > + if (IS_ENABLED(CONFIG_NONPORTABLE) && !IS_ENABLED(CONFIG_RISCV_ISA_SV57))
> > > > + return false;
> > > > +
> > > > return riscv_has_extension_likely(RISCV_ISA_EXT_SV57);
> > > > }
> > > > #endif
> > >
> > > ...because the code here won't enable sv57 etc if not supported...
> > >
> > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > index 2a2402d79bc4..90bb85c5f167 100644
> > > > --- a/arch/riscv/mm/init.c
> > > > +++ b/arch/riscv/mm/init.c
> > > > @@ -844,6 +844,16 @@ static __init void set_satp_mode(uintptr_t dtb_pa)
> > > >
> > > > kernel_map.page_offset = PAGE_OFFSET_L5;
> > > >
> > > > + /* Fall back to Sv48 if Sv57 is unsupported on non-portable systems */
> > > > + if (IS_ENABLED(CONFIG_NONPORTABLE) && !IS_ENABLED(CONFIG_RISCV_ISA_SV57) &&
> > > > + satp_mode_limit == 0)
> > > > + satp_mode_limit = SATP_MODE_48;
> > >
> > > ...and neither does the code here.
> > >
> > > > Introduce RISCV_ISA_SV48 and RISCV_ISA_SV57 under NONPORTABLE, so that
> > > > the embedded platforms can choose the best option themselves, while
> > > > still keep the feature of unified one kernel Image for all SV39, SV48
> > > > and SV57.
> > >
> > > NONPORTABLE is what you're meant to depend on when the kernel that gets
> > > built cannot run on platforms without the feature.
> >
> > The reason I put these two Kconfig options under NONPORTABLE is: all
> > distributions won't enable NONPORTABLE for their defconfig, and I guess
> > the distributions want to kept unified one kernel Image for all SV39,
> > SV48 and SV57.
> >
> > I have no preference whether move them out of NONPORTABLE or not.
>
> Oh right I think I made a mistake in my understanding of what's going on
> here.
>
> If you turn on NONPORTABLE (but not these new options) then you only get
> sv39 support. I think what you should do to make the behaviour of these
> options clearer is invert them.
hmm, other ISA ext Kconfig options are not inverted, so I prefer moving
SV48|57 out of NONPORTABLE and make them y by default.
>
> e.g. something like RISCV_ISA_NO_SV57 and RISCV_ISA_NO_SV48. These would
> then depend on NONPORTABLE and be default n.
>
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -16,21 +16,33 @@ extern bool _pgtable_l5_enabled;
> #ifdef USE_EARLY_PGTABLE_LEVELS
> static __always_inline bool pgtable_l5_enabled(void)
> {
> + if (IS_ENABLED(CONFIG_RISCV_ISA_NO_SV57))
> + return false;
> +
> return _pgtable_l5_enabled;
> }
>
> static __always_inline bool pgtable_l4_enabled(void)
> {
> + if (IS_ENABLED(CONFIG_RISCV_ISA_SV48))
> + return false;
> +
> return _pgtable_l4_enabled;
> }
> #else
> static __always_inline bool pgtable_l4_enabled(void)
> {
> + if (IS_ENABLED(CONFIG_RISCV_ISA_NO_SV48))
> + return false;
> +
> return riscv_has_extension_likely(RISCV_ISA_EXT_SV48);
> }
>
> static __always_inline bool pgtable_l5_enabled(void)
> {
> + if (IS_ENABLED(CONFIG_RISCV_ISA_NO_SV57))
> + return false;
> +
> return riscv_has_extension_likely(RISCV_ISA_EXT_SV57);
> }
> #endif
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 2a2402d79bc4..90bb85c5f167 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -844,6 +844,16 @@ static __init void set_satp_mode(uintptr_t dtb_pa)
>
> kernel_map.page_offset = PAGE_OFFSET_L5;
>
> + /* Fall back to Sv48 if Sv57 is unsupported on non-portable systems */
> + if (IS_ENABLED(CONFIG_RISCV_ISA_NO_SV57) &&
> + satp_mode_limit == 0)
> + satp_mode_limit = SATP_MODE_48;
> +
> + /* Fall back to Sv39 if Sv48 is unsupported on non-portable systems */
> + if (IS_ENABLED(CONFIG_RISCV_ISA_NO_SV48) &&
> + satp_mode_limit == SATP_MODE_48)
> + satp_mode_limit = SATP_MODE_39;
>
>
> I think that code's behaviour is a bit easier to understand and the
> options become more obvious in terms of what they do.
>
> Cheers,
> Conor.
>
> > >
> > > > +
> > > > + /* Fall back to Sv39 if Sv48 is unsupported on non-portable systems */
> > > > + if (IS_ENABLED(CONFIG_NONPORTABLE) && !IS_ENABLED(CONFIG_RISCV_ISA_SV48) &&
> > > > + satp_mode_limit == SATP_MODE_48)
> > > > + satp_mode_limit = SATP_MODE_39;
> > > > +
> > > > if (satp_mode_limit == SATP_MODE_48) {
> > > > disable_pgtable_l5();
> > > > } else if (satp_mode_limit == SATP_MODE_39) {
> > > > --
> > > > 2.53.0
> > > >
> >
> >
next prev parent reply other threads:[~2026-09-09 0:08 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-07 15:14 [PATCH v2 0/5] optimize pgtable_l4|l5_enabled Jisheng Zhang
2026-09-07 15:14 ` [PATCH v2 1/5] riscv: remove RISCV_ALTERNATIVE Kconfig option Jisheng Zhang
2026-09-07 15:14 ` [PATCH v2 2/5] riscv: convert pgtable_l4|l5_enabled to inline function Jisheng Zhang
2026-09-07 15:14 ` [PATCH v2 3/5] riscv: support early isa ext and use it to optimize pgtable_l4|l5_enabled Jisheng Zhang
2026-09-07 16:39 ` Conor Dooley
2026-09-08 3:43 ` Jisheng Zhang
2026-09-08 13:00 ` Conor Dooley
2026-09-08 23:46 ` Jisheng Zhang
2026-09-09 11:49 ` Conor Dooley
2026-09-09 15:01 ` Jisheng Zhang
2026-09-07 15:14 ` [PATCH v2 4/5] riscv: RISCV_ISA_SV48 and RISCV_ISA_SV57 under NONPORTABLE Jisheng Zhang
2026-09-07 16:21 ` Conor Dooley
2026-09-08 3:49 ` Jisheng Zhang
2026-09-08 13:07 ` Conor Dooley
2026-09-08 23:48 ` Jisheng Zhang [this message]
2026-09-09 11:08 ` Conor Dooley
2026-09-07 15:14 ` [PATCH v2 5/5] riscv: mm: unexport _pgtable_l4_enabled and _pgtable_l5_enabled Jisheng Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aqCe6T8QKtJcLHiM@xhacker \
--to=jszhang@kernel.org \
--cc=alex@ghiti.fr \
--cc=andreyknvl@gmail.com \
--cc=aou@eecs.berkeley.edu \
--cc=conor@kernel.org \
--cc=dvyukov@google.com \
--cc=glider@google.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=pjw@kernel.org \
--cc=ryabinin.a.a@gmail.com \
--cc=vincenzo.frascino@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®