[PATCH v11 1/3] riscv: mm: modify pte format for Svnapot

[Qinglin Pan]

Hi Andrew and Conor,

On 2022/12/27 03:19, Andrew Jones wrote:
> On Mon, Dec 26, 2022 at 06:40:23PM +0000, Conor Dooley wrote:
>> On Mon, Dec 26, 2022 at 05:01:29PM +0100, Andrew Jones wrote:
>>> On Mon, Dec 19, 2022 at 09:51:29PM +0000, Conor Dooley wrote:
>>>> On Fri, Dec 16, 2022 at 02:21:07PM +0800, panqinglin2020 at iscas.ac.cn wrote:
>>>>> From: Qinglin Pan <panqinglin2020 at iscas.ac.cn>
>>>>>
>>>>> Add one alternative to enable/disable svnapot support, enable this static
>>>>> key when "svnapot" is in the "riscv,isa" field of fdt and SVNAPOT compile
>>>>> option is set. It will influence the behavior of has_svnapot. All code
>>>>> dependent on svnapot should make sure that has_svnapot return true firstly.
>>>>>
>>>>> Modify PTE definition for Svnapot, and creates some functions in pgtable.h
>>>>> to mark a PTE as napot and check if it is a Svnapot PTE. Until now, only
>>>>> 64KB napot size is supported in spec, so some macros has only 64KB version.
>>>>>
>>>>> Signed-off-by: Qinglin Pan <panqinglin2020 at iscas.ac.cn>
>>>>> Reviewed-by: Andrew Jones <ajones at ventanamicro.com>
>>>>>
>>>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>>>>> index e1a9fa47f012..25c230e3bf61 100644
>>>>> --- a/arch/riscv/Kconfig
>>>>> +++ b/arch/riscv/Kconfig
>>>>> @@ -397,6 +397,25 @@ config RISCV_ISA_C
>>>>>   
>>>>>   	  If you don't know what to do here, say Y.
>>>>>   
>>>>> +config RISCV_ISA_SVNAPOT
>>>>> +	bool "SVNAPOT extension support"
>>>>> +	depends on 64BIT && MMU
>>>>> +	select RISCV_ALTERNATIVE
>>>>> +	default y
>>>>> +	help
>>>>> +	  Allow kernel to detect the SVNAPOT ISA-extension dynamically at boot
>>>>> +	  time and enable its usage.
>>>>> +
>>>>> +	  The SVNAPOT extension is used to mark contiguous PTEs as a range
>>>>> +	  of contiguous virtual-to-physical translations for a naturally
>>>>> +	  aligned power-of-2 (NAPOT) granularity larger than the base 4KB page
>>>>> +	  size. When HUGETLBFS is also selected this option unconditionally
>>>>> +	  allocates some memory for each NAPOT page size supported by the kernel.
>>>>> +	  When optimizing for low memory consumption and for platforms without
>>>>
>>>> nit: Does this make more sense as "When optimising for low memory
>>>> consumption on platforms without the SVNAPOT extension"...?
>>>> Or does disabling Svnapot save memory on Svnapot capable systems too?
>>>
>>> Depends on how we define "save". Disabling RISCV_ISA_SVNAPOT on HUGETLBFS
>>> enabled systems will reduce memory consumption whether the svnapot
>>> extension is present or not. When the extension is present, the memory
>>> consumed may serve a purpose, whereas, on systems without svnapot, the
>>> memory consumed cannot serve a purpose.
>>
>> Right, thanks for the explanation. I'm sorry to be a bore here, but should
>> it be reworded then to "When optimising for low memory consumption on
>> platforms without SVNAPOT, or if HUGETLBFS is enabled, say N here"?
> 
> I'd prefer we keep the instructions as simple as possible. Basically,

Sorry for slow reply.

I agree with Andrew, we should keep it as simple as possible. :)
So I will keep it unchanged unless any new suggestions about this
patchset is given.

Thanks,
Qinglin

> "Turn this off if you don't plan to use it and if you care about memory
> consumption", which, IMHO, is how the text more or less already reads.
> But...
> 
>>
>> I don't really care either way to be perfectly honest, so as I said in
>> my other reply "don't bother resubmitting unless someone else asks for
>> meaningful changes" ;)
> 
> ...I'm not too worried about it either, so I won't complain if Qinglin
> respins with your suggestion.
> 
> Thanks,
> drew