[PATCH v5 1/4] mm: modify pte format for Svnapot
Andrew Jones
ajones at ventanamicro.com
Tue Oct 4 10:00:27 PDT 2022
On Mon, Oct 03, 2022 at 09:47:18PM +0800, panqinglin2020 at iscas.ac.cn wrote:
> From: Qinglin Pan <panqinglin2020 at iscas.ac.cn>
>
> This commit adds 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 commit 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.
>
> Signed-off-by: Qinglin Pan <panqinglin2020 at iscas.ac.cn>
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index d557cc50295d..4354024aae21 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -383,6 +383,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 n
What's the reason for this to be default no? If there's a trade-off to be
made which requires opt-in, then it should be described in the text below.
> + help
> + Say Y if you want to 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 19a771085781..f3aff5ef52e4 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -22,7 +22,8 @@
>
> #define CPUFEATURE_SVPBMT 0
> #define CPUFEATURE_ZICBOM 1
> -#define CPUFEATURE_NUMBER 2
> +#define CPUFEATURE_SVNAPOT 2
> +#define CPUFEATURE_NUMBER 3
nit: Maybe take the opportunity to tab out the numbers to get them all
lined up. And tab enough for future longer names too.
>
> #ifdef __ASSEMBLY__
>
> @@ -142,6 +143,26 @@ asm volatile(ALTERNATIVE_2( \
> "r"((unsigned long)(_start) + (_size)) \
> : "a0")
>
> +#define ALT_SVNAPOT(_val) \
> +asm(ALTERNATIVE("li %0, 0", "li %0, 1", 0, \
> + CPUFEATURE_SVNAPOT, CONFIG_RISCV_ISA_SVNAPOT) \
> + : "=r"(_val) :)
> +
> +#define ALT_SVNAPOT_PTE_PFN(_val, _napot_shift, _pfn_mask, _pfn_shift) \
> +asm(ALTERNATIVE("and %0, %0, %1\n\t" \
> + "srli %0, %0, %2\n\t" \
> + __nops(3), \
> + "srli t3, %0, %3\n\t" \
> + "and %0, %0, %1\n\t" \
> + "srli %0, %0, %2\n\t" \
> + "sub t4, %0, t3\n\t" \
> + "and %0, %0, t4", \
This implements
temp = ((pte & _PAGE_PFN_MASK) >> _PAGE_PFN_SHIFT);
pfn = temp & (temp - (pte >> _PAGE_NAPOT_SHIFT));
which for a non-napot pte just returns the same as the non-napot
case would, but for a napot pte we return the same as the non-napot
case but with its first set bit cleared, wherever that first set
bit was. Can you explain to me why that's what we want to do?
> + 0, CPUFEATURE_SVNAPOT, CONFIG_RISCV_ISA_SVNAPOT) \
> + : "+r"(_val) \
> + : "r"(_pfn_mask), \
> + "i"(_pfn_shift), \
> + "i"(_napot_shift))
Should add t3 and t4 to clobbers list.
> +
> #endif /* __ASSEMBLY__ */
>
> #endif
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 6f59ec64175e..83b8e21a3573 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -54,10 +54,11 @@ extern unsigned long elf_hwcap;
> */
> enum riscv_isa_ext_id {
> RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> + RISCV_ISA_EXT_SSTC,
> + RISCV_ISA_EXT_SVNAPOT,
> RISCV_ISA_EXT_SVPBMT,
> RISCV_ISA_EXT_ZICBOM,
> RISCV_ISA_EXT_ZIHINTPAUSE,
> - RISCV_ISA_EXT_SSTC,
Opportunistic alphabetizing an enum is a bit risky. It'd be better if this
was a separate patch, because, even though nothing should care about the
order, if something does care about the order, then it'd be easier to
debug when this when changed alone.
> 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 dc42375c2357..25ec541192e5 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -74,6 +74,19 @@ 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)
> +#define NAPOT_CONT64KB_ORDER 4UL
> +#define NAPOT_CONT64KB_SHIFT (NAPOT_CONT64KB_ORDER + PAGE_SHIFT)
> +#define NAPOT_CONT64KB_SIZE BIT(NAPOT_CONT64KB_SHIFT)
> +#define NAPOT_CONT64KB_MASK (NAPOT_CONT64KB_SIZE - 1UL)
> +#define NAPOT_64KB_PTE_NUM BIT(NAPOT_CONT64KB_ORDER)
Please make three lined up columns out of these defines.
#define DEF VAL
> +
> /*
> * [62:61] Svpbmt Memory Type definitions:
> *
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 7ec936910a96..c3fc3c661699 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -264,10 +264,39 @@ 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);
Why aren't we using the isa extension static key framework for this?
> + 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);
> +}
> +#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 _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 0be8a2403212..d2a61122c595 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -96,6 +96,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> + __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
If we want to do a separate isa extension alphabetizing patch, then this
array would be a good one to do since it's the output order.
> __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> };
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 553d755483ed..3557c5cc6f04 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -204,6 +204,7 @@ void __init riscv_fill_hwcap(void)
> SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
> SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> + SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
Could alphabetize this too, even though it doesn't matter.
> }
> #undef SET_ISA_EXT_MAP
> }
> @@ -284,6 +285,20 @@ static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
> return false;
> }
>
> +static bool __init_or_module cpufeature_probe_svnapot(unsigned int stage)
> +{
> +#ifdef CONFIG_RISCV_ISA_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.
> *
> @@ -301,6 +316,9 @@ static u32 __init_or_module cpufeature_probe(unsigned int stage)
> if (cpufeature_probe_zicbom(stage))
> cpu_req_feature |= (1U << CPUFEATURE_ZICBOM);
>
> + if (cpufeature_probe_svnapot(stage))
> + cpu_req_feature |= BIT(CPUFEATURE_SVNAPOT);
> +
> return cpu_req_feature;
> }
>
> --
> 2.35.1
>
Thanks,
drew
More information about the linux-riscv
mailing list