Message-ID: <e34f73ce-bca8-4d18-84cd-9b8d0a6130b9@ghiti.fr>
Date: Wed, 29 May 2024 11:08:15 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: Conor Dooley <conor.dooley@...rochip.com>
Cc: Conor Dooley <conor@...nel.org>, linux-riscv@...ts.infradead.org,
 xiao.w.wang@...el.com, Andrew Jones <ajones@...tanamicro.com>,
 pulehui@...wei.com, Charlie Jenkins <charlie@...osinc.com>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, linux-kernel@...r.kernel.org,
 Samuel Holland <samuel.holland@...ive.com>,
 Pu Lehui <pulehui@...weicloud.com>, Björn Töpel
 <bjorn@...nel.org>
Subject: Re: [PATCH v3 1/2] RISC-V: clarify what some RISCV_ISA* config
 options do


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@...rochip.com>
>>>
>>> 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@...tanamicro.com>
>>> Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
>>> ---
>>>    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@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Powered by blists - more mailing lists