Re: [PATCH 3/3] riscv: introduce cap framework and use it to optimize pgtable_l4|l5_enabled

[Conor Dooley]

On Tue, Sep 01, 2026 at 12:55:56PM +0800, Jisheng Zhang wrote:
> On Tue, Sep 01, 2026 at 08:21:16AM +0800, Jisheng Zhang wrote:
> > On Mon, Aug 31, 2026 at 11:04:12PM +0100, Conor Dooley wrote:
> > > On Mon, Aug 31, 2026 at 01:04:01AM +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, we can use alternative mechanism to optimize them.
> > > > 
> > > > So the question is whether we can add RISCV_ISA_EXT_SV48/SV5 and use
> > > > riscv_has_extension_*() or not. Although, per [1] and [2], SV48 and
> > > > SV57 are ISA exensions too, RISCV_ISA_EXT_SV48/SV57 are to describe hw
> > > > supported extensions, while this doesn't mean the pgtable_l4|l5 is
> > > > enabled, for example, we may pass no5lvl/no4lvl kernel boot args or
> > > > explicitly ask for SV39 by setting dt mmu-type property as
> > > > "riscv,sv39". If we clear RISCV_ISA_EXT_SV48|SV57, then internal
> > > > extension queries and potentially userspace reporting can no longer
> > > > distinguish “unsupported” from “supported but disabled.”
> > > 
> > > That's the case for all extensions that the kernel does not support,
> > > like if the fpu is disabled because the kernel doesn't have the
> > > configuration option set. In fact, I think the CFI extensions are very
> > 
> > To be honest, before introduce the cap framework, I did think about FPU,
> > ZACAS, ZABHA etc. ISA extensions' cases. Finally I came to a conclusion
> > that the SV48/57 is a different case: Let me take FPU for an example.
> > FPU once probed, the ISA support is never disabled, IOW, the
> > riscv_has_extension_likely(FPU) is always true no matter the CONFIG_FPU
> > is enabled or not. There's "no supported but disabled" case for FPU.
> > But for SV48 case, that's different. The underlying HW may support SV48
> > or SV57, but the "no5lvl and no4lvl" may explicitly force SV39, thus
> > if we go with exisiting riscv_has_extension_*() API, we need to
> > explicitly disable it by clear the isa bitmap. IOW, riscv_has_extension_likely(SV48)
> > is false even the underlying HW supports SV48, thus the user can't
> > distinguish between “unsupported” from “supported but disabled.”
> > What do you think?

(moving things around a little bit)

> and change its meaning a
> bit from "the underlying HW supports the feature" to "the underlying HW
> enables the feature".
> 
> or keep it only for ISA extensions only.
> 
> X86's X86_FEATURE_* take the first path, I.E it only means the feature
> is enabled or disabled on current platform, but doesn't mean the current
> platform HW supports the feature or not.
> 
> Any comments is appreciated.

On this particular point, this is already how it works.
riscv_has_extension_[un]likely() and other related functions report
whether the extension is supported and enabled on the platform, not just
whether the underlying hardware can support it. As I mentioned
yesterday, the F and D extensions depend on CONFIG_FPU being enabled. If
CONFIG_FPU is disabled, but F and D appear in the DT or ACPI, we clear
the relevant bits in the bitmap. In turn, if there are extensions that
depend on F and D being enabled, they'll get turned off too.

> 
> Other thoughts:
> 
> performance PoV: usually, an ISA extension improves the performance, so
> there's no case where we clear the ISA bit map. But SV48/SV57 is
> different, e.g on a board w/ only 4GB DDR but the cpu supports SV57,
> there's no reason to enable SV57 which impacts performance a bit. This
> is the reason why no4lvl/no5lvl are introduced, AIUI.

Again, I think this applies to CFI, I don't think what's being done here
is fundamentally different to that.

> > > similar here, because they also have riscv_nousercfi command line
> > 
> > This is another story, neither CPU ISA extension nor CPU capbility.

They are extensions though, riscv_nousercfi disables the cfilp and cfiss
extensions. You can see that in cpufeature.c. Maybe there's some
semantic argument about whether they're /CPU/ extensions, but there's
other stuff that aren't really CPU extensions and we treat them all the
same. I guess the difference for sv48/sv57 is that we'd just be setting
the bitmap bits rather than parsing it from DT/ACPI - but there's prior art
for that already.

> > 
> > > options that can disable them.
> > > If that's the reason for making a new-but-similar mechanism, I think you
> > > should just introduce RISCV_ISA_EXT_SV{48,57} and use that.

> other cap usage PoV: the cap can be used to replace various static
> branch usage in arch/riscv, for example the fast_unaligned_access_speed_key
> As is known, the static branch has some drawbacks.
> 
> So no matter which direction this series goes to, we need similar cap
> framework. The key question is whether we can extend the
> riscv_has_extension_*() to cover cpu/HW cap,

It'd be nice to see if we could reuse the functions, if we don't
want to invent a bunch of XLINUXFOO for them. The vendor extension stuff
that Charlie added reuses it, but since there's no Linux Foundation
JEDEC ID (AFAIK anyway) perhaps there's just no easy way to do it and
duplicating the functions with a different name is required.

This particular case is an extension, so I think we should try to make
it work using that framework though.

Attachment: signature.asc
Description: PGP signature