Re: [PATCH v3 1/2] RISC-V: clarify what some RISCV_ISA* config options do

[Conor Dooley]

On Wed, May 29, 2024 at 10:47:40AM +0200, Alexandre Ghiti wrote:
> Hi Conor,
> 
> On 28/05/2024 13:11, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > 
> > During some discussion on IRC yesterday and on Pu's bpf patch [1]
> > I noticed that these RISCV_ISA* Kconfig options are not really clear
> > about their implications. Many of these options have no impact on what
> > userspace is allowed to do, for example an application can use Zbb
> > regardless of whether or not the kernel does. Change the help text to
> > try and clarify whether or not an option affects just the kernel, or
> > also userspace. None of these options actually control whether or not an
> > extension is detected dynamically as that's done regardless of Kconfig
> > options, so drop any text that implies the option is required for
> > dynamic detection, rewording them as "do x when y is detected".
> > 
> > Link: https://lore.kernel.org/linux-riscv/20240328-ferocity-repose-c554f75a676c@spud/ [1]
> > Reviewed-by: Andrew Jones <ajones@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > ---
> >   arch/riscv/Kconfig | 36 +++++++++++++++++++-----------------
> >   1 file changed, 19 insertions(+), 17 deletions(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index b94176e25be1..3b702e6cc051 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -501,7 +501,8 @@ config RISCV_ISA_C
> >   	help
> >   	  Adds "C" to the ISA subsets that the toolchain is allowed to emit
> >   	  when building Linux, which results in compressed instructions in the
> > -	  Linux binary.
> > +	  Linux binary. This option produces a kernel that will not run on
> > +	  systems that do not support compressed instructions.
> >   	  If you don't know what to do here, say Y.
> > @@ -511,8 +512,8 @@ config RISCV_ISA_SVNAPOT
> >   	depends on RISCV_ALTERNATIVE
> >   	default y
> >   	help
> > -	  Allow kernel to detect the Svnapot ISA-extension dynamically at boot
> > -	  time and enable its usage.
> > +	  Add support for the Svnapot ISA-extension in the kernel when it
> > +	  is detected at boot.
> 
> 
> To me, the new version makes things even more confusing: svnapot mappings
> will indeed be handled by the kernel (since only the kernel sets up the page
> tables) but it will only be used (for now) for HugeTLB mappings in
> userspace.

How would you suggest that I word it? "Enable the use of the Svnapot
ISA-extension when it is detected at boot"? The current text implies that
these options control detection of extensions (which they do not) and
that is what I am looking to remove as it has caused confusion.

Attachment: signature.asc
Description: PGP signature