[PATCH v9 1/3] riscv: mm: modify pte format for Svnapot
Qinglin Pan
panqinglin2020 at iscas.ac.cn
Wed Dec 7 20:51:03 PST 2022
Hey!
On 2022/12/8 02:46, Conor Dooley wrote:
> Hey!
>
> Couple small remarks and questions for you.
>
> On Sun, Dec 04, 2022 at 10:11:35PM +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>
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index ef8d66de5f38..1d8477c0af7c 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -395,6 +395,20 @@ 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 SVNAPOT ISA-extension dynamically in boot time
>> + and enable its usage.
>> +
>> + SVNAPOT extension helps to mark contiguous PTEs as a range
>> + of contiguous virtual-to-physical translations, with a naturally
>> + aligned power-of-2 (NAPOT) granularity larger than the base 4KB page
>> + size.
>> +
>> config RISCV_ISA_SVPBMT
>> bool "SVPBMT extension support"
>> depends on 64BIT && MMU
>> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
>> index 4180312d2a70..beadb1126ed9 100644
>> --- a/arch/riscv/include/asm/errata_list.h
>> +++ b/arch/riscv/include/asm/errata_list.h
>> @@ -22,9 +22,10 @@
>> #define ERRATA_THEAD_NUMBER 3
>> #endif
>>
>> -#define CPUFEATURE_SVPBMT 0
>> -#define CPUFEATURE_ZICBOM 1
>> -#define CPUFEATURE_NUMBER 2
>> +#define CPUFEATURE_SVPBMT 0
>> +#define CPUFEATURE_ZICBOM 1
>> +#define CPUFEATURE_SVNAPOT 2
>> +#define CPUFEATURE_NUMBER 3
>>
>> #ifdef __ASSEMBLY__
>>
>> @@ -156,6 +157,14 @@ asm volatile(ALTERNATIVE( \
>> : "=r" (__ovl) : \
>> : "memory")
>>
>> +#define ALT_SVNAPOT(_val) \
>> +asm(ALTERNATIVE( \
>> + "li %0, 0", \
>> + "li %0, 1", \
>> + 0, CPUFEATURE_SVNAPOT, CONFIG_RISCV_ISA_SVNAPOT) \
>> + : "=r" (_val) : \
>> + : "memory")
>> +
>> #endif /* __ASSEMBLY__ */
>>
>> #endif
>> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
>> index b22525290073..4cbc1f45ab26 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_SVNAPOT,
>> RISCV_ISA_EXT_SVPBMT,
>> RISCV_ISA_EXT_ZICBOM,
>> RISCV_ISA_EXT_ZIHINTPAUSE,
>> @@ -87,7 +88,6 @@ static __always_inline int riscv_isa_ext2key(int num)
>> {
>> switch (num) {
>> case RISCV_ISA_EXT_f:
>> - return RISCV_ISA_EXT_KEY_FPU;
>> case RISCV_ISA_EXT_d:
>> return RISCV_ISA_EXT_KEY_FPU;
>> case RISCV_ISA_EXT_ZIHINTPAUSE:
>> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
>> index ac70b0fd9a9a..349fad5e35de 100644
>> --- a/arch/riscv/include/asm/page.h
>> +++ b/arch/riscv/include/asm/page.h
>> @@ -16,11 +16,6 @@
>> #define PAGE_SIZE (_AC(1, UL) << PAGE_SHIFT)
>> #define PAGE_MASK (~(PAGE_SIZE - 1))
>>
>> -#ifdef CONFIG_64BIT
>> -#define HUGE_MAX_HSTATE 2
>> -#else
>> -#define HUGE_MAX_HSTATE 1
>> -#endif
>> #define HPAGE_SHIFT PMD_SHIFT
>> #define HPAGE_SIZE (_AC(1, UL) << HPAGE_SHIFT)
>> #define HPAGE_MASK (~(HPAGE_SIZE - 1))
>> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
>> index dc42375c2357..9611833907ec 100644
>> --- a/arch/riscv/include/asm/pgtable-64.h
>> +++ b/arch/riscv/include/asm/pgtable-64.h
>> @@ -74,6 +74,40 @@ 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 BIT(_PAGE_NAPOT_SHIFT)
>> +/*
>> + * Only 64KB (order 4) napot ptes supported.
>> + */
>> +#define NAPOT_CONT_ORDER_BASE 4
>> +enum napot_cont_order {
>> + NAPOT_CONT64KB_ORDER = NAPOT_CONT_ORDER_BASE,
>> + NAPOT_ORDER_MAX,
>> +};
>> +
>> +#define for_each_napot_order(order) \
>> + for (order = NAPOT_CONT_ORDER_BASE; order < NAPOT_ORDER_MAX; order++)
>> +#define for_each_napot_order_rev(order) \
>
> Would it be terrible to do s/rev/reverse to make things more obvious?
Maybe we should just keep it in the same style as existing macros like
for_each_mem_range/for_each_mem_range_rev :)
>
>> + for (order = NAPOT_ORDER_MAX - 1; \
>> + order >= NAPOT_CONT_ORDER_BASE; order--)
>> +#define napot_cont_order(val) (__builtin_ctzl((val.pte >> _PAGE_PFN_SHIFT) << 1))
>> +
>> +#define napot_cont_shift(order) ((order) + PAGE_SHIFT)
>> +#define napot_cont_size(order) BIT(napot_cont_shift(order))
>> +#define napot_cont_mask(order) (~(napot_cont_size(order) - 1UL))
>> +#define napot_pte_num(order) BIT(order)
>> +
>> +#ifdef CONFIG_RISCV_ISA_SVNAPOT
>> +#define HUGE_MAX_HSTATE (2 + (NAPOT_ORDER_MAX - NAPOT_CONT_ORDER_BASE))
>> +#else
>> +#define HUGE_MAX_HSTATE 2
>> +#endif
>
> This is coming from a place of *complete* ignorance:
> If CONFIG_RISCV_ISA_SVNAPOT is enabled in the kernel but there is no
> support for it on a platform is it okay to change the value of
> HUGE_MAX_HSTATE? Apologies if I've missed something obvious.
:(
You are right. If CONFIG_RISCV_ISA_SVNAPOT is enabled, HUGE_MAX_HSTATE
will always be 3 whether has_svnapot() is true or false. I will try to
fix this.
>
>> +
>> /*
>> * [62:61] Svpbmt Memory Type definitions:
>> *
>> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
>> index c61ae83aadee..99957b1270f2 100644
>> --- a/arch/riscv/include/asm/pgtable.h
>> +++ b/arch/riscv/include/asm/pgtable.h
>> @@ -6,10 +6,12 @@
>> #ifndef _ASM_RISCV_PGTABLE_H
>> #define _ASM_RISCV_PGTABLE_H
>>
>> +#include <linux/jump_label.h>
>> #include <linux/mmzone.h>
>> #include <linux/sizes.h>
>>
>> #include <asm/pgtable-bits.h>
>> +#include <asm/hwcap.h>
>>
>> #ifndef CONFIG_MMU
>> #define KERNEL_LINK_ADDR PAGE_OFFSET
>> @@ -264,10 +266,49 @@ static inline pte_t pud_pte(pud_t pud)
>> return __pte(pud_val(pud));
>> }
>>
>> +static __always_inline bool has_svnapot(void)
>> +{
>> + unsigned int _val;
>> +
>> + ALT_SVNAPOT(_val);
>> +
>> + return _val;
>> +}
>> +
>> +#ifdef CONFIG_RISCV_ISA_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)
>> +{
>> + int pos = order - 1 + _PAGE_PFN_SHIFT;
>> + unsigned long napot_bit = BIT(pos);
>> + unsigned long napot_mask = ~GENMASK(pos, _PAGE_PFN_SHIFT);
>> +
>> + return __pte((pte_val(pte) & napot_mask) | napot_bit | _PAGE_NAPOT);
>> +}
>> +
>> +#else
>> +
>> +static inline unsigned long pte_napot(pte_t pte)
>> +{
>> + return 0;
>> +}
>> +
>> +#endif /* CONFIG_RISCV_ISA_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 res = __page_val_to_pfn(pte_val(pte));
>
> nit: extra space before the =
Nice catch! Will remove this extra space :)
>
>> +
>> + if (has_svnapot() && pte_napot(pte))
>> + res = res & (res - 1UL);
>> +
>> + return res;
>> }
>>
>> #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 bf9dd6764bad..88495f5fcafd 100644
>> --- a/arch/riscv/kernel/cpu.c
>> +++ b/arch/riscv/kernel/cpu.c
>> @@ -165,6 +165,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
>> __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>> __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
>> __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
>> + __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
>> __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
>> __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
>> __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 694267d1fe81..eeed66c3d497 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -205,6 +205,7 @@ void __init riscv_fill_hwcap(void)
>> SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
>> SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
>> SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
>> + SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
>> }
>> #undef SET_ISA_EXT_MAP
>> }
>> @@ -252,6 +253,17 @@ void __init riscv_fill_hwcap(void)
>> }
>>
>> #ifdef CONFIG_RISCV_ALTERNATIVE
>> +static bool __init_or_module cpufeature_probe_svnapot(unsigned int stage)
>> +{
>> + if (!IS_ENABLED(CONFIG_RISCV_ISA_SVNAPOT))
>> + return false;
>> +
>> + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
>> + return false;
>> +
>> + return riscv_isa_extension_available(NULL, SVNAPOT);
>> +}
>> +
>> static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
>> {
>> if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
>> @@ -289,6 +301,9 @@ static u32 __init_or_module cpufeature_probe(unsigned int stage)
>> {
>> u32 cpu_req_feature = 0;
>>
>> + if (cpufeature_probe_svnapot(stage))
>> + cpu_req_feature |= BIT(CPUFEATURE_SVNAPOT);
>> +
>> if (cpufeature_probe_svpbmt(stage))
>> cpu_req_feature |= BIT(CPUFEATURE_SVPBMT);
>>
>
> There's a bunch of stuff in this patch that may go away, depending on
> sequencing just FYI. See [1] for more. I wouldn't rebase on top of that,
> but just so that you're aware.
Thanks for the information! If this patchset will be merged firstly,
Jisheng can easily replace implementation of has_svnapot with his
interface. Otherwise I will do this replacement :)
Thanks,
Qinglin
>
> 1 - https://patchwork.kernel.org/project/linux-riscv/cover/20221204174632.3677-1-jszhang@kernel.org/
More information about the linux-riscv
mailing list