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

[Andrew Jones]

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,
"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