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

[Alexandre Ghiti]

On 29/05/2024 10:54, Conor Dooley wrote:
> 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.


Ok, I see what you mean. Your above suggestion looks great then :)

Thanks,

Alex


> _______________________________________________
> linux-riscv mailing list
> linux-riscv@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv