[v2,1/4] riscv, mm: detect svnapot cpu support at runtime

Commit Message

Qinglin Pan July 16, 2022, 8:56 a.m. UTC

From: Qinglin Pan <[e-mail]>

This patch add two erratas to enable/disable svnapot support, patches code
dynamicly when "svnapot" is in the "riscv,isa" field of fdt and SVNAPOT compile
option is set. It will influence the behavior of has_svnapot function and
pte_pfn function. All code dependent on svnapot should make sure that has_svnapot
return true firstly.

Also, this patch modifies 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 draft spec, so some macros
has only 64KB version.

Yours,
Qinglin

Signed-off-by: Qinglin Pan <[e-mail]>

Comments

Jisheng Zhang July 16, 2022, 12:35 p.m. UTC | #1

On Sat, Jul 16, 2022 at 04:56:45PM +0800, [e-mail] wrote:
> From: Qinglin Pan <[e-mail]>
> 
> This patch add two erratas to enable/disable svnapot support, patches code
> dynamicly when "svnapot" is in the "riscv,isa" field of fdt and SVNAPOT compile
> option is set. It will influence the behavior of has_svnapot function and
> pte_pfn function. All code dependent on svnapot should make sure that has_svnapot
> return true firstly.
> 
> Also, this patch modifies 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 draft spec, so some macros
> has only 64KB version.
> 
> Yours,
> Qinglin

hmm, this "Yours ..." should be removed in commit msg
> 
> Signed-off-by: Qinglin Pan <[e-mail]>
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index b17fd4666b0c..c5e1629a6033 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -385,6 +385,13 @@ config FPU
>  
>  	  If you don't know what to do here, say Y.
>  
> +config SVNAPOT
> +	bool "Svnapot support"

Do we add an config opition for each isa extension? That's not
necessary. I think we'd better remove this CONFIG option and keep
unified Image in mind.

> +	default n
> +	help
> +	  Select if your CPU supports Svnapot and you want to enable it when
> +	  kernel is booting.
> +
>  endmenu # "Platform type"
>  menu "Kernel features"
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 9e2888dbb5b1..84ad32075637 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -20,7 +20,8 @@
>  #endif
>  
>  #define	CPUFEATURE_SVPBMT 0
> -#define	CPUFEATURE_NUMBER 1
> +#define	CPUFEATURE_SVNAPOT 1
> +#define	CPUFEATURE_NUMBER 2
>  
>  #ifdef __ASSEMBLY__
>  
> @@ -93,6 +94,27 @@ asm volatile(ALTERNATIVE(						\
>  #define ALT_THEAD_PMA(_val)
>  #endif
>  
> +#define ALT_SVNAPOT(_val)						\

I believe SVNAPOT can be supported w/o ALTERNATIVE, static key mechanism
would be much simpler.

> +asm(ALTERNATIVE("li %0, 0", "li %0, 1", 0,				\
> +		CPUFEATURE_SVNAPOT, CONFIG_SVNAPOT)			\
> +		: "=r"(_val) :)
> +
> +#define ALT_SVNAPOT_PTE_PFN(_val, _napot_shift, _pfn_mask, _pfn_shift)	\
> +asm(ALTERNATIVE("and %0, %1, %2\n\t"					\
> +		"srli %0, %0, %3\n\t"					\
> +		"nop\n\tnop\n\tnop",					\
> +		"srli t3, %1, %4\n\t"					\
> +		"and %0, %1, %2\n\t"					\
> +		"srli %0, %0, %3\n\t"					\
> +		"sub  t4, %0, t3\n\t"					\
> +		"and  %0, %0, t4",					\
> +		0, CPUFEATURE_SVNAPOT, CONFIG_SVNAPOT)			\
> +		: "+r"(_val)						\
> +		: "r"(_val),						\
> +		  "r"(_pfn_mask),					\
> +		  "i"(_pfn_shift),					\
> +		  "i"(_napot_shift))
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e48eebdd2631..292ab93321e3 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -54,6 +54,7 @@ extern unsigned long elf_hwcap;
>  enum riscv_isa_ext_id {
>  	RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
>  	RISCV_ISA_EXT_SVPBMT,
> +	RISCV_ISA_EXT_SVNAPOT,
>  	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
>  };
>  
> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> index 5c2aba5efbd0..e8463515a46c 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -74,6 +74,20 @@ typedef struct {
>   */
>  #define _PAGE_PFN_MASK  GENMASK(53, 10)
>  
> +/*
> + * [63] Svnapot definitions:
> + * 0 Svnapot disabled
> + * 1 Svnapot enabled
> + */
> +#define _PAGE_NAPOT_SHIFT 63
> +#define _PAGE_NAPOT      (1UL << _PAGE_NAPOT_SHIFT)
> +#define NAPOT_CONT64KB_ORDER 4UL
> +#define NAPOT_CONT64KB_SHIFT (NAPOT_CONT64KB_ORDER + PAGE_SHIFT)
> +#define NAPOT_CONT64KB_SIZE (1UL << NAPOT_CONT64KB_SHIFT)
> +#define NAPOT_CONT64KB_MASK (NAPOT_CONT64KB_SIZE - 1)
> +#define NAPOT_64KB_PTE_NUM (1UL << NAPOT_CONT64KB_ORDER)
> +#define NAPOT_64KB_MASK (7UL << _PAGE_PFN_SHIFT)
> +
>  /*
>   * [62:61] Svpbmt Memory Type definitions:
>   *
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 1d1be9d9419c..34c4be9de79e 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -284,10 +284,38 @@ static inline pte_t pud_pte(pud_t pud)
>  	return __pte(pud_val(pud));
>  }
>  
> +static inline bool has_svnapot(void) {
> +	u64 _val;
> +	ALT_SVNAPOT(_val);
> +	return _val;
> +}
> +
> +#ifdef CONFIG_SVNAPOT
> +
> +static inline unsigned long pte_napot(pte_t pte)
> +{
> +	return pte_val(pte) & _PAGE_NAPOT;
> +}
> +
> +static inline pte_t pte_mknapot(pte_t pte, unsigned int order)
> +{
> +	unsigned long napot_bits = (1UL << (order - 1)) << _PAGE_PFN_SHIFT;
> +	unsigned long lower_prot =
> +		pte_val(pte) & ((1UL << _PAGE_PFN_SHIFT) - 1UL);
> +	unsigned long upper_prot = (pte_val(pte) >> _PAGE_PFN_SHIFT)
> +				   << _PAGE_PFN_SHIFT;
> +
> +	return __pte(upper_prot | napot_bits | lower_prot | _PAGE_NAPOT);
> +}
> +#endif /* CONFIG_SVNAPOT */
> +
>  /* Yields the page frame number (PFN) of a page table entry */
>  static inline unsigned long pte_pfn(pte_t pte)
>  {
> -	return __page_val_to_pfn(pte_val(pte));
> +	unsigned long _val  = pte_val(pte);
> +	ALT_SVNAPOT_PTE_PFN(_val, _PAGE_NAPOT_SHIFT,
> +			_PAGE_PFN_MASK, _PAGE_PFN_SHIFT);
> +	return _val;
>  }
>  
>  #define pte_page(x)     pfn_to_page(pte_pfn(x))
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fba9e9f46a8c..9f1113fa2b96 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -89,6 +89,7 @@ int riscv_of_parent_hartid(struct device_node *node)
>  static struct riscv_isa_ext_data isa_ext_arr[] = {
>  	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>  	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> +	__RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
>  	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
>  };
>  
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1b3ec44e25f5..9f38b7d02f2a 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void)
>  			} else {
>  				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
>  				SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> +				SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
>  			}
>  #undef SET_ISA_EXT_MAP
>  		}
> @@ -259,6 +260,20 @@ static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
>  	return false;
>  }
>  
> +static bool __init_or_module cpufeature_probe_svnapot(unsigned int stage)
> +{
> +#ifdef CONFIG_SVNAPOT
> +	switch (stage) {
> +	case RISCV_ALTERNATIVES_EARLY_BOOT:
> +		return false;
> +	default:
> +		return riscv_isa_extension_available(NULL, SVNAPOT);
> +	}
> +#endif
> +
> +	return false;
> +}
> +
>  /*
>   * Probe presence of individual extensions.
>   *
> @@ -273,6 +288,9 @@ static u32 __init_or_module cpufeature_probe(unsigned int stage)
>  	if (cpufeature_probe_svpbmt(stage))
>  		cpu_req_feature |= (1U << CPUFEATURE_SVPBMT);
>  
> +	if (cpufeature_probe_svnapot(stage))
> +		cpu_req_feature |= (1U << CPUFEATURE_SVNAPOT);
> +
>  	return cpu_req_feature;
>  }
>  
> -- 
> 2.35.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> [e-mail]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Conor Dooley July 16, 2022, 1:33 p.m. UTC | #2

On 16/07/2022 13:35, Jisheng Zhang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Sat, Jul 16, 2022 at 04:56:45PM +0800, [e-mail] wrote:
>> From: Qinglin Pan <[e-mail]>
>>
>> This patch add two erratas to enable/disable svnapot support, patches code
>> dynamicly when "svnapot" is in the "riscv,isa" field of fdt and SVNAPOT compile
>> option is set. It will influence the behavior of has_svnapot function and
>> pte_pfn function. All code dependent on svnapot should make sure that has_svnapot
>> return true firstly.
>>
>> Also, this patch modifies 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 draft spec, so some macros
>> has only 64KB version.
>>
>> Yours,
>> Qinglin
> 
> hmm, this "Yours ..." should be removed in commit msg

And while they're at it, they can drop the "This patch".

I might've said this on a v1 (I know I said it on someone's
v1 recently) that having "Also, this patch" makes it sound
like this should actually be two patches and not one.

Thanks,
Conor.

>>
>> Signed-off-by: Qinglin Pan <[e-mail]>
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index b17fd4666b0c..c5e1629a6033 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -385,6 +385,13 @@ config FPU
>>
>>         If you don't know what to do here, say Y.
>>
>> +config SVNAPOT
>> +     bool "Svnapot support"
> 
> Do we add an config opition for each isa extension? That's not
> necessary. I think we'd better remove this CONFIG option and keep
> unified Image in mind.
> 
>> +     default n
>> +     help
>> +       Select if your CPU supports Svnapot and you want to enable it when
>> +       kernel is booting.
>> +
>>  endmenu # "Platform type"
>>  menu "Kernel features"
>> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
>> index 9e2888dbb5b1..84ad32075637 100644
>> --- a/arch/riscv/include/asm/errata_list.h
>> +++ b/arch/riscv/include/asm/errata_list.h
>> @@ -20,7 +20,8 @@
>>  #endif
>>
>>  #define      CPUFEATURE_SVPBMT 0
>> -#define      CPUFEATURE_NUMBER 1
>> +#define      CPUFEATURE_SVNAPOT 1
>> +#define      CPUFEATURE_NUMBER 2
>>
>>  #ifdef __ASSEMBLY__
>>
>> @@ -93,6 +94,27 @@ asm volatile(ALTERNATIVE(                                          \
>>  #define ALT_THEAD_PMA(_val)
>>  #endif
>>
>> +#define ALT_SVNAPOT(_val)                                            \
> 
> I believe SVNAPOT can be supported w/o ALTERNATIVE, static key mechanism
> would be much simpler.
> 
>> +asm(ALTERNATIVE("li %0, 0", "li %0, 1", 0,                           \
>> +             CPUFEATURE_SVNAPOT, CONFIG_SVNAPOT)                     \
>> +             : "=r"(_val) :)
>> +
>> +#define ALT_SVNAPOT_PTE_PFN(_val, _napot_shift, _pfn_mask, _pfn_shift)       \
>> +asm(ALTERNATIVE("and %0, %1, %2\n\t"                                 \
>> +             "srli %0, %0, %3\n\t"                                   \
>> +             "nop\n\tnop\n\tnop",                                    \
>> +             "srli t3, %1, %4\n\t"                                   \
>> +             "and %0, %1, %2\n\t"                                    \
>> +             "srli %0, %0, %3\n\t"                                   \
>> +             "sub  t4, %0, t3\n\t"                                   \
>> +             "and  %0, %0, t4",                                      \
>> +             0, CPUFEATURE_SVNAPOT, CONFIG_SVNAPOT)                  \
>> +             : "+r"(_val)                                            \
>> +             : "r"(_val),                                            \
>> +               "r"(_pfn_mask),                                       \
>> +               "i"(_pfn_shift),                                      \
>> +               "i"(_napot_shift))
>> +
>>  #endif /* __ASSEMBLY__ */
>>
>>  #endif
>> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
>> index e48eebdd2631..292ab93321e3 100644
>> --- a/arch/riscv/include/asm/hwcap.h
>> +++ b/arch/riscv/include/asm/hwcap.h
>> @@ -54,6 +54,7 @@ extern unsigned long elf_hwcap;
>>  enum riscv_isa_ext_id {
>>       RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
>>       RISCV_ISA_EXT_SVPBMT,
>> +     RISCV_ISA_EXT_SVNAPOT,
>>       RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
>>  };
>>
>> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
>> index 5c2aba5efbd0..e8463515a46c 100644
>> --- a/arch/riscv/include/asm/pgtable-64.h
>> +++ b/arch/riscv/include/asm/pgtable-64.h
>> @@ -74,6 +74,20 @@ typedef struct {
>>   */
>>  #define _PAGE_PFN_MASK  GENMASK(53, 10)
>>
>> +/*
>> + * [63] Svnapot definitions:
>> + * 0 Svnapot disabled
>> + * 1 Svnapot enabled
>> + */
>> +#define _PAGE_NAPOT_SHIFT 63
>> +#define _PAGE_NAPOT      (1UL << _PAGE_NAPOT_SHIFT)
>> +#define NAPOT_CONT64KB_ORDER 4UL
>> +#define NAPOT_CONT64KB_SHIFT (NAPOT_CONT64KB_ORDER + PAGE_SHIFT)
>> +#define NAPOT_CONT64KB_SIZE (1UL << NAPOT_CONT64KB_SHIFT)
>> +#define NAPOT_CONT64KB_MASK (NAPOT_CONT64KB_SIZE - 1)
>> +#define NAPOT_64KB_PTE_NUM (1UL << NAPOT_CONT64KB_ORDER)
>> +#define NAPOT_64KB_MASK (7UL << _PAGE_PFN_SHIFT)
>> +
>>  /*
>>   * [62:61] Svpbmt Memory Type definitions:
>>   *
>> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
>> index 1d1be9d9419c..34c4be9de79e 100644
>> --- a/arch/riscv/include/asm/pgtable.h
>> +++ b/arch/riscv/include/asm/pgtable.h
>> @@ -284,10 +284,38 @@ static inline pte_t pud_pte(pud_t pud)
>>       return __pte(pud_val(pud));
>>  }
>>
>> +static inline bool has_svnapot(void) {
>> +     u64 _val;
>> +     ALT_SVNAPOT(_val);
>> +     return _val;
>> +}
>> +
>> +#ifdef CONFIG_SVNAPOT
>> +
>> +static inline unsigned long pte_napot(pte_t pte)
>> +{
>> +     return pte_val(pte) & _PAGE_NAPOT;
>> +}
>> +
>> +static inline pte_t pte_mknapot(pte_t pte, unsigned int order)
>> +{
>> +     unsigned long napot_bits = (1UL << (order - 1)) << _PAGE_PFN_SHIFT;
>> +     unsigned long lower_prot =
>> +             pte_val(pte) & ((1UL << _PAGE_PFN_SHIFT) - 1UL);
>> +     unsigned long upper_prot = (pte_val(pte) >> _PAGE_PFN_SHIFT)
>> +                                << _PAGE_PFN_SHIFT;
>> +
>> +     return __pte(upper_prot | napot_bits | lower_prot | _PAGE_NAPOT);
>> +}
>> +#endif /* CONFIG_SVNAPOT */
>> +
>>  /* Yields the page frame number (PFN) of a page table entry */
>>  static inline unsigned long pte_pfn(pte_t pte)
>>  {
>> -     return __page_val_to_pfn(pte_val(pte));
>> +     unsigned long _val  = pte_val(pte);
>> +     ALT_SVNAPOT_PTE_PFN(_val, _PAGE_NAPOT_SHIFT,
>> +                     _PAGE_PFN_MASK, _PAGE_PFN_SHIFT);
>> +     return _val;
>>  }
>>
>>  #define pte_page(x)     pfn_to_page(pte_pfn(x))
>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
>> index fba9e9f46a8c..9f1113fa2b96 100644
>> --- a/arch/riscv/kernel/cpu.c
>> +++ b/arch/riscv/kernel/cpu.c
>> @@ -89,6 +89,7 @@ int riscv_of_parent_hartid(struct device_node *node)
>>  static struct riscv_isa_ext_data isa_ext_arr[] = {
>>       __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>>       __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
>> +     __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
>>       __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
>>  };
>>
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 1b3ec44e25f5..9f38b7d02f2a 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -198,6 +198,7 @@ void __init riscv_fill_hwcap(void)
>>                       } else {
>>                               SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
>>                               SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
>> +                             SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
>>                       }
>>  #undef SET_ISA_EXT_MAP
>>               }
>> @@ -259,6 +260,20 @@ static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
>>       return false;
>>  }
>>
>> +static bool __init_or_module cpufeature_probe_svnapot(unsigned int stage)
>> +{
>> +#ifdef CONFIG_SVNAPOT
>> +     switch (stage) {
>> +     case RISCV_ALTERNATIVES_EARLY_BOOT:
>> +             return false;
>> +     default:
>> +             return riscv_isa_extension_available(NULL, SVNAPOT);
>> +     }
>> +#endif
>> +
>> +     return false;
>> +}
>> +
>>  /*
>>   * Probe presence of individual extensions.
>>   *
>> @@ -273,6 +288,9 @@ static u32 __init_or_module cpufeature_probe(unsigned int stage)
>>       if (cpufeature_probe_svpbmt(stage))
>>               cpu_req_feature |= (1U << CPUFEATURE_SVPBMT);
>>
>> +     if (cpufeature_probe_svnapot(stage))
>> +             cpu_req_feature |= (1U << CPUFEATURE_SVNAPOT);
>> +
>>       return cpu_req_feature;
>>  }
>>
>> --
>> 2.35.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [e-mail]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
> _______________________________________________
> linux-riscv mailing list
> [e-mail]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Qinglin Pan July 16, 2022, 1:41 p.m. UTC | #3

> On 16/07/2022 13:35, Jisheng Zhang wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On Sat, Jul 16, 2022 at 04:56:45PM +0800, [e-mail] wrote:
&gt; &gt;&gt; From: Qinglin Pan <[e-mail]>
&gt; &gt;&gt;
&gt; &gt;&gt; This patch add two erratas to enable/disable svnapot support, patches code
&gt; &gt;&gt; dynamicly when "svnapot" is in the "riscv,isa" field of fdt and SVNAPOT compile
&gt; &gt;&gt; option is set. It will influence the behavior of has_svnapot function and
&gt; &gt;&gt; pte_pfn function. All code dependent on svnapot should make sure that has_svnapot
&gt; &gt;&gt; return true firstly.
&gt; &gt;&gt;
&gt; &gt;&gt; Also, this patch modifies PTE definition for Svnapot, and creates some functions in
&gt; &gt;&gt; pgtable.h to mark a PTE as napot and check if it is a Svnapot PTE.
&gt; &gt;&gt; Until now, only 64KB napot size is supported in draft spec, so some macros
&gt; &gt;&gt; has only 64KB version.
&gt; &gt;&gt;
&gt; &gt;&gt; Yours,
&gt; &gt;&gt; Qinglin
&gt; &gt; 
&gt; &gt; hmm, this "Yours ..." should be removed in commit msg
&gt; 
&gt; And while they're at it, they can drop the "This patch".
&gt; 
&gt; I might've said this on a v1 (I know I said it on someone's
&gt; v1 recently) that having "Also, this patch" makes it sound
&gt; like this should actually be two patches and not one.
&gt; 
&gt; Thanks,
&gt; Conor.
&gt; 

Thanks for the hints, will do in a newer version patch.


Thanks,
Qinglin.</[e-mail]>

Qinglin Pan July 16, 2022, 1:42 p.m. UTC | #4

> >  
> > +config SVNAPOT
> > + bool "Svnapot support"
> 
> Do we add an config opition for each isa extension? That's not
> necessary. I think we'd better remove this CONFIG option and keep
> unified Image in mind.
> 

I am also not very sure about necessity of this. Maybe we should allow user to disable
Svnapot support manually? As this support may introduce extra overhead in functions
like pte_pfn. So should such option be reserved for setting it off manually? If it should not,
I will remove it in a newer version.

> > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > index 9e2888dbb5b1..84ad32075637 100644
> > --- a/arch/riscv/include/asm/errata_list.h
> > +++ b/arch/riscv/include/asm/errata_list.h
> > @@ -20,7 +20,8 @@
> >  #endif
> >  
> >  #define CPUFEATURE_SVPBMT 0
> > -#define CPUFEATURE_NUMBER 1
> > +#define CPUFEATURE_SVNAPOT 1
> > +#define CPUFEATURE_NUMBER 2
> >  
> >  #ifdef __ASSEMBLY__
> >  
> > @@ -93,6 +94,27 @@ asm volatile(ALTERNATIVE(      \
> >  #define ALT_THEAD_PMA(_val)
> >  #endif
> >  
> > +#define ALT_SVNAPOT(_val)      \
> 
> I believe SVNAPOT can be supported w/o ALTERNATIVE, static key mechanism
> would be much simpler.
>

Thanks for the hint, will implement it with static key mechanism in a newer version.

Thanks,
Qinglin

> > +asm(ALTERNATIVE("li %0, 0", "li %0, 1", 0,    \
> > +  CPUFEATURE_SVNAPOT, CONFIG_SVNAPOT)   \
> > +  : "=r"(_val) :)
> > +
> > +#define ALT_SVNAPOT_PTE_PFN(_val, _napot_shift, _pfn_mask, _pfn_shift) \
> > +asm(ALTERNATIVE("and %0, %1, %2\n\t"     \
> > +  "srli %0, %0, %3\n\t"     \
> > +  "nop\n\tnop\n\tnop",     \
> > +  "srli t3, %1, %4\n\t"     \
> > +  "and %0, %1, %2\n\t"     \
> > +  "srli %0, %0, %3\n\t"     \
> > +  "sub  t4, %0, t3\n\t"     \
> > +  "and  %0, %0, t4",     \
> > +  0, CPUFEATURE_SVNAPOT, CONFIG_SVNAPOT)   \
> > +  : "+r"(_val)      \
> > +  : "r"(_val),      \
> > +    "r"(_pfn_mask),     \
> > +    "i"(_pfn_shift),     \
> > +    "i"(_napot_shift))
> > +
> >  #endif /* __ASSEMBLY__ */
> >  
> >  #endif

Conor Dooley July 16, 2022, 1:51 p.m. UTC | #5

On 16/07/2022 14:41, 潘庆霖 wrote:
> > On 16/07/2022 13:35, Jisheng Zhang wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > > 
> > > On Sat, Jul 16, 2022 at 04:56:45PM +0800, [e-mail] wrote:
> &gt; &gt;&gt; From: Qinglin Pan <[e-mail]>
> &gt; &gt;&gt;
> &gt; &gt;&gt; This patch add two erratas to enable/disable svnapot support, patches code
> &gt; &gt;&gt; dynamicly when "svnapot" is in the "riscv,isa" field of fdt and SVNAPOT compile
> &gt; &gt;&gt; option is set. It will influence the behavior of has_svnapot function and
> &gt; &gt;&gt; pte_pfn function. All code dependent on svnapot should make sure that has_svnapot
> &gt; &gt;&gt; return true firstly.
> &gt; &gt;&gt;
> &gt; &gt;&gt; Also, this patch modifies PTE definition for Svnapot, and creates some functions in
> &gt; &gt;&gt; pgtable.h to mark a PTE as napot and check if it is a Svnapot PTE.
> &gt; &gt;&gt; Until now, only 64KB napot size is supported in draft spec, so some macros
> &gt; &gt;&gt; has only 64KB version.
> &gt; &gt;&gt;
> &gt; &gt;&gt; Yours,
> &gt; &gt;&gt; Qinglin
> &gt; &gt; 
> &gt; &gt; hmm, this "Yours ..." should be removed in commit msg
> &gt; 
> &gt; And while they're at it, they can drop the "This patch".
> &gt; 
> &gt; I might've said this on a v1 (I know I said it on someone's
> &gt; v1 recently) that having "Also, this patch" makes it sound
> &gt; like this should actually be two patches and not one.
> &gt; 
> &gt; Thanks,
> &gt; Conor.
> &gt;

^ might want to fix your mail client to not do that too! :)
Thanks,
Conor

> 
> Thanks for the hints, will do in a newer version patch.
> 
> 
> Thanks,
> Qinglin.</[e-mail]>
> _______________________________________________
> linux-riscv mailing list
> [e-mail]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index b17fd4666b0c..c5e1629a6033 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -385,6 +385,13 @@  config FPU
 
 	  If you don't know what to do here, say Y.
 
+config SVNAPOT
+	bool "Svnapot support"
+	default n
+	help
+	  Select if your CPU supports Svnapot and you want to enable it when
+	  kernel is booting.
+
 endmenu # "Platform type"
 
 menu "Kernel features"
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 9e2888dbb5b1..84ad32075637 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -20,7 +20,8 @@ 
 #endif
 
 #define	CPUFEATURE_SVPBMT 0
-#define	CPUFEATURE_NUMBER 1
+#define	CPUFEATURE_SVNAPOT 1
+#define	CPUFEATURE_NUMBER 2
 
 #ifdef __ASSEMBLY__
 
@@ -93,6 +94,27 @@  asm volatile(ALTERNATIVE(						\
 #define ALT_THEAD_PMA(_val)
 #endif
 
+#define ALT_SVNAPOT(_val)						\
+asm(ALTERNATIVE("li %0, 0", "li %0, 1", 0,				\
+		CPUFEATURE_SVNAPOT, CONFIG_SVNAPOT)			\
+		: "=r"(_val) :)
+
+#define ALT_SVNAPOT_PTE_PFN(_val, _napot_shift, _pfn_mask, _pfn_shift)	\
+asm(ALTERNATIVE("and %0, %1, %2\n\t"					\
+		"srli %0, %0, %3\n\t"					\
+		"nop\n\tnop\n\tnop",					\
+		"srli t3, %1, %4\n\t"					\
+		"and %0, %1, %2\n\t"					\
+		"srli %0, %0, %3\n\t"					\
+		"sub  t4, %0, t3\n\t"					\
+		"and  %0, %0, t4",					\
+		0, CPUFEATURE_SVNAPOT, CONFIG_SVNAPOT)			\
+		: "+r"(_val)						\
+		: "r"(_val),						\
+		  "r"(_pfn_mask),					\
+		  "i"(_pfn_shift),					\
+		  "i"(_napot_shift))
+
 #endif /* __ASSEMBLY__ */
 
 #endif
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e48eebdd2631..292ab93321e3 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -54,6 +54,7 @@  extern unsigned long elf_hwcap;
 enum riscv_isa_ext_id {
 	RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
 	RISCV_ISA_EXT_SVPBMT,
+	RISCV_ISA_EXT_SVNAPOT,
 	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
 };
 
diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 5c2aba5efbd0..e8463515a46c 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -74,6 +74,20 @@  typedef struct {
  */
 #define _PAGE_PFN_MASK  GENMASK(53, 10)
 
+/*
+ * [63] Svnapot definitions:
+ * 0 Svnapot disabled
+ * 1 Svnapot enabled
+ */
+#define _PAGE_NAPOT_SHIFT 63
+#define _PAGE_NAPOT      (1UL << _PAGE_NAPOT_SHIFT)
+#define NAPOT_CONT64KB_ORDER 4UL
+#define NAPOT_CONT64KB_SHIFT (NAPOT_CONT64KB_ORDER + PAGE_SHIFT)
+#define NAPOT_CONT64KB_SIZE (1UL << NAPOT_CONT64KB_SHIFT)
+#define NAPOT_CONT64KB_MASK (NAPOT_CONT64KB_SIZE - 1)
+#define NAPOT_64KB_PTE_NUM (1UL << NAPOT_CONT64KB_ORDER)
+#define NAPOT_64KB_MASK (7UL << _PAGE_PFN_SHIFT)
+
 /*
  * [62:61] Svpbmt Memory Type definitions:
  *
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 1d1be9d9419c..34c4be9de79e 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -284,10 +284,38 @@  static inline pte_t pud_pte(pud_t pud)
 	return __pte(pud_val(pud));
 }
 
+static inline bool has_svnapot(void) {
+	u64 _val;
+	ALT_SVNAPOT(_val);
+	return _val;
+}
+
+#ifdef CONFIG_SVNAPOT
+
+static inline unsigned long pte_napot(pte_t pte)
+{
+	return pte_val(pte) & _PAGE_NAPOT;
+}
+
+static inline pte_t pte_mknapot(pte_t pte, unsigned int order)
+{
+	unsigned long napot_bits = (1UL << (order - 1)) << _PAGE_PFN_SHIFT;
+	unsigned long lower_prot =
+		pte_val(pte) & ((1UL << _PAGE_PFN_SHIFT) - 1UL);
+	unsigned long upper_prot = (pte_val(pte) >> _PAGE_PFN_SHIFT)
+				   << _PAGE_PFN_SHIFT;
+
+	return __pte(upper_prot | napot_bits | lower_prot | _PAGE_NAPOT);
+}
+#endif /* CONFIG_SVNAPOT */
+
 /* Yields the page frame number (PFN) of a page table entry */
 static inline unsigned long pte_pfn(pte_t pte)
 {
-	return __page_val_to_pfn(pte_val(pte));
+	unsigned long _val  = pte_val(pte);
+	ALT_SVNAPOT_PTE_PFN(_val, _PAGE_NAPOT_SHIFT,
+			_PAGE_PFN_MASK, _PAGE_PFN_SHIFT);
+	return _val;
 }
 
 #define pte_page(x)     pfn_to_page(pte_pfn(x))
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index fba9e9f46a8c..9f1113fa2b96 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -89,6 +89,7 @@  int riscv_of_parent_hartid(struct device_node *node)
 static struct riscv_isa_ext_data isa_ext_arr[] = {
 	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
 	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
+	__RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
 	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
 };
 
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1b3ec44e25f5..9f38b7d02f2a 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -198,6 +198,7 @@  void __init riscv_fill_hwcap(void)
 			} else {
 				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
 				SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
+				SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
 			}
 #undef SET_ISA_EXT_MAP
 		}
@@ -259,6 +260,20 @@  static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
 	return false;
 }
 
+static bool __init_or_module cpufeature_probe_svnapot(unsigned int stage)
+{
+#ifdef CONFIG_SVNAPOT
+	switch (stage) {
+	case RISCV_ALTERNATIVES_EARLY_BOOT:
+		return false;
+	default:
+		return riscv_isa_extension_available(NULL, SVNAPOT);
+	}
+#endif
+
+	return false;
+}
+
 /*
  * Probe presence of individual extensions.
  *
@@ -273,6 +288,9 @@  static u32 __init_or_module cpufeature_probe(unsigned int stage)
 	if (cpufeature_probe_svpbmt(stage))
 		cpu_req_feature |= (1U << CPUFEATURE_SVPBMT);
 
+	if (cpufeature_probe_svnapot(stage))
+		cpu_req_feature |= (1U << CPUFEATURE_SVNAPOT);
+
 	return cpu_req_feature;
 }