Risc-V Svinval support
v6 cover Risc-V Svinval support
This patch adds support for the Svinval extension as defined in the Risc V Privileged specification.
Changes in v6:
- Rebase on latest torvalds/master
Changes in v5:
- Reduce tlb flush threshold to 64
- Improve implementation of local_flush_tlb* functions
Changes in v4:
- Rebase and refactor as per latest changes on torvalds/master
- Drop patch 1 in the series
Changes in v3:
- Fix incorrect vma used for sinval instructions
- Use unified static key mechanism for svinval
- Rebased on torvalds/master
Changes in v2:
- Rebased on 5.18-rc3
- update riscv_fill_hwcap to probe Svinval extension
Mayuresh Chitale (1):
riscv: mm: Add support for Svinval extension
arch/riscv/mm/tlbflush.c | 58 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
Signature · 1 line
2.34.1
PATCH v6 1/1 riscv: mm: Add support for Svinval extension
The Svinval extension splits SFENCE.VMA instruction into finer-grained invalidation and ordering operations and is mandatory for RVA23S64 profile. When Svinval is enabled the local_flush_tlb_range_threshold_asid function should use the following sequence to optimize the tlb flushes instead of a simple sfence.vma:
sfence.w.inval
svinval.vma
.
.
svinval.vma
sfence.inval.ir
The maximum number of consecutive svinval.vma instructions that can be executed in local_flush_tlb_range_threshold_asid function is limited to 64. This is required to avoid soft lockups and the approach is similar to that used in arm64.
--- arch/riscv/mm/tlbflush.c | 58 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
arch/riscv/mm/tlbflush.c +58 -0
@@ -6,6 +6,54 @@#include <linux/hugetlb.h> #include <asm/sbi.h> #include <asm/mmu_context.h> +#include <asm/cpufeature.h> + +#define has_svinval () riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL) + +static inline void local_sfence_inval_ir (void ) +{ +/* +* SFENCE.INVAL.IR +* 0001100 00001 00000 000 00000 1110011 +*/ + __asm__ __volatile__ (".word 0x18100073" :::"memory" ); +} + +static inline void local_sfence_w_inval (void ) +{ +/* +* SFENCE.W.INVAL +* 0001100 00000 00000 000 00000 1110011 +*/ + __asm__ __volatile__ (".word 0x18000073" :::"memory" ); +} + +static inline void local_sinval_vma_asid (unsigned long vma,unsigned long asid) +{ +if (asid !=FLUSH_TLB_NO_ASID ) { +/* +* rs1 = a0 (VMA) +* rs2 = a1 (asid) +* SINVAL.VMA a0, a1 +* 0001011 01011 01010 000 00000 1110011 +*/ + __asm__ __volatile__ ("add a0, %0, zero\n" +"add a1, %1, zero\n" +".word 0x16B50073\n" + ::"r" (vma),"r" (asid) + :"a0" ,"a1" ,"memory" ); + }else { +/* +* rs1 = a0 (VMA) +* rs2 = 0 +* SINVAL.VMA a0 +* 0001011 00000 01010 000 00000 1110011 +*/ + __asm__ __volatile__ ("add a0, %0, zero\n" +".word 0x16050073\n" + ::"r" (vma) :"a0" ,"memory" ); + } +}/* * Flush entire TLB if number of entries to be flushed is greater
@@ -26,6 +74,16 @@ static void local_flush_tlb_range_threshold_asid(unsigned long start,return ; } +if (has_svinval ()) { +local_sfence_w_inval (); +for (i =0 ; i < nr_ptes_in_range; ++i) { +local_sinval_vma_asid (start, asid); + start += stride; + } +local_sfence_inval_ir (); +return ; + } +for (i =0 ; i < nr_ptes_in_range; ++i) {local_flush_tlb_page_asid (start, asid); start += stride;
Signature · 1 line
2.34.1
Re: [PATCH v6 1/1] riscv: mm: Add support for Svinval extension
Quoted 44 lines · Mayuresh Chitale
The Svinval extension splits SFENCE.VMA instruction into finer-grained invalidation and ordering operations and is mandatory for RVA23S64 profile. When Svinval is enabled the
local_flush_tlb_range_threshold_asidfunction should use the following sequence to optimize the tlb flushes instead of a simple sfence.vma:sfence.w.inval
svinval.vma
.
.
svinval.vma
sfence.inval.irThe maximum number of consecutive svinval.vma instructions that can be executed in
local_flush_tlb_range_threshold_asidfunction is limited to 64. This is required to avoid soft lockups and the approach is similar to that used in arm64.Signed-off-by Mayuresh Chitale <[email protected]>--- arch/riscv/mm/tlbflush.c | 58 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)arch/riscv/mm/tlbflush.c +13
-0@@ -6,6 +6,54 @@#include <linux/hugetlb.h> #include <asm/sbi.h> #include <asm/mmu_context.h> +#include <asm/cpufeature.h> + +#define has_svinval () riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL) + +static inline void local_sfence_inval_ir (void ) +{ +/* +* SFENCE.INVAL.IR +* 0001100 00001 00000 000 00000 1110011 +*/ + __asm__ __volatile__ (".word 0x18100073" :::"memory" ); +} +
arch/riscv/mm/tlbflush.c line 31
Quoted 10 lines
arch/riscv/mm/tlbflush.c +10
-0+static inline void local_sfence_w_inval (void ) +{ +/* +* SFENCE.W.INVAL +* 0001100 00000 00000 000 00000 1110011 +*/ + __asm__ __volatile__ (".word 0x18000073" :::"memory" ); +} + +static inline void local_sinval_vma_asid (unsigned long vma,unsigned long asid)
Please name this to
void local_sinval_vma(unsigned long vaddr, unsigned long asid)
to match the spec's naming. But, if we want the arguments in the name, then it should be something like
void local_sinval_vma_addr_asid(unsigned long vaddr, unsigned long asid)
but I think that's unnecessary.
Quoted 14 lines
arch/riscv/mm/tlbflush.c +14
-0+{ +if (asid !=FLUSH_TLB_NO_ASID ) { +/* +* rs1 = a0 (VMA) +* rs2 = a1 (asid) +* SINVAL.VMA a0, a1 +* 0001011 01011 01010 000 00000 1110011 +*/ + __asm__ __volatile__ ("add a0, %0, zero\n" +"add a1, %1, zero\n" +".word 0x16B50073\n" + ::"r" (vma),"r" (asid) + :"a0" ,"a1" ,"memory" ); + }else {
arch/riscv/mm/tlbflush.c line 55
Quoted 10 lines
arch/riscv/mm/tlbflush.c +10
-0+/* +* rs1 = a0 (VMA) +* rs2 = 0 +* SINVAL.VMA a0 +* 0001011 00000 01010 000 00000 1110011 +*/ + __asm__ __volatile__ ("add a0, %0, zero\n" +".word 0x16050073\n" + ::"r" (vma) :"a0" ,"memory" ); + }
Please create an SINVAL_VMA instruction in insn-def.h to allow the compiler to choose which registers will be used for asid and vaddr. And, since SINVAL_VMA will be in insn-def.h, then we might as well add SFENCE_INVAL_IR and SFENCE_W_INVAL there too for consistency, even though they don't have operands. We should still create the three static inline wrapper functions here though.
arch/riscv/mm/tlbflush.c static void local_flush_tlb_range_threshold_asid(unsigned long start, · line 79
Quoted 11 lines
arch/riscv/mm/tlbflush.c +4
-0+}/* * Flush entire TLB if number of entries to be flushed is greater @@ -26,6 +74,16 @@ static void local_flush_tlb_range_threshold_asid(unsigned long start,return ; } +if (has_svinval ()) { +local_sfence_w_inval (); +for (i =0 ; i < nr_ptes_in_range; ++i) {
Where do we limit this to 64 as the commit message states it does?
Quoted 13 lines
arch/riscv/mm/tlbflush.c +7
-0+local_sinval_vma_asid (start, asid); + start += stride; + } +local_sfence_inval_ir (); +return ; + } +for (i =0 ; i < nr_ptes_in_range; ++i) {local_flush_tlb_page_asid (start, asid); start += stride;Signature · 1 line
2.34.1
Thanks,
drew
Alexandre Ghiti
reviewerRe: [PATCH v6 1/1] riscv: mm: Add support for Svinval extension
Hi Mayuresh, Andrew,
Quoted 105 lines · Andrew Jones
Quoted 44 lines · Mayuresh Chitale
The Svinval extension splits SFENCE.VMA instruction into finer-grained invalidation and ordering operations and is mandatory for RVA23S64 profile. When Svinval is enabled the
local_flush_tlb_range_threshold_asidfunction should use the following sequence to optimize the tlb flushes instead of a simple sfence.vma:sfence.w.inval
svinval.vma
.
.
svinval.vma
sfence.inval.irThe maximum number of consecutive svinval.vma instructions that can be executed in
local_flush_tlb_range_threshold_asidfunction is limited to 64. This is required to avoid soft lockups and the approach is similar to that used in arm64.Signed-off-by Mayuresh Chitale <[email protected]>--- arch/riscv/mm/tlbflush.c | 58 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)arch/riscv/mm/tlbflush.c +13
-0@@ -6,6 +6,54 @@#include <linux/hugetlb.h> #include <asm/sbi.h> #include <asm/mmu_context.h> +#include <asm/cpufeature.h> + +#define has_svinval () riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL) + +static inline void local_sfence_inval_ir (void ) +{ +/* +* SFENCE.INVAL.IR +* 0001100 00001 00000 000 00000 1110011 +*/ + __asm__ __volatile__ (".word 0x18100073" :::"memory" ); +} +arch/riscv/mm/tlbflush.c line 31
Quoted 10 lines
arch/riscv/mm/tlbflush.c +10
-0+static inline void local_sfence_w_inval (void ) +{ +/* +* SFENCE.W.INVAL +* 0001100 00000 00000 000 00000 1110011 +*/ + __asm__ __volatile__ (".word 0x18000073" :::"memory" ); +} + +static inline void local_sinval_vma_asid (unsigned long vma,unsigned long asid)Please name this to
void
local_sinval_vma(unsigned long vaddr, unsigned long asid)to match the spec's naming. But, if we want the arguments in the name, then it should be something like
void
local_sinval_vma_addr_asid(unsigned long vaddr, unsigned long asid)but I think that's unnecessary.
Quoted 14 lines
arch/riscv/mm/tlbflush.c +14
-0+{ +if (asid !=FLUSH_TLB_NO_ASID ) { +/* +* rs1 = a0 (VMA) +* rs2 = a1 (asid) +* SINVAL.VMA a0, a1 +* 0001011 01011 01010 000 00000 1110011 +*/ + __asm__ __volatile__ ("add a0, %0, zero\n" +"add a1, %1, zero\n" +".word 0x16B50073\n" + ::"r" (vma),"r" (asid) + :"a0" ,"a1" ,"memory" ); + }else {arch/riscv/mm/tlbflush.c line 55
Quoted 10 lines
arch/riscv/mm/tlbflush.c +10
-0+/* +* rs1 = a0 (VMA) +* rs2 = 0 +* SINVAL.VMA a0 +* 0001011 00000 01010 000 00000 1110011 +*/ + __asm__ __volatile__ ("add a0, %0, zero\n" +".word 0x16050073\n" + ::"r" (vma) :"a0" ,"memory" ); + }Please create an
SINVAL_VMAinstruction ininsn-def.hto allow the compiler to choose which registers will be used for asid and vaddr. And, sinceSINVAL_VMAwill be ininsn-def.h, then we might as well addSFENCE_INVAL_IRandSFENCE_W_INVALthere too for consistency, even though they don't have operands. We should still create the three static inline wrapper functions here though.Quoted 8 lines
arch/riscv/mm/tlbflush.c +1
-0+}/* * Flush entire TLB if number of entries to be flushed is greater @@ -26,6 +74,16 @@ static void local_flush_tlb_range_threshold_asid(unsigned long start,return ; }
Quoted 4 lines
arch/riscv/mm/tlbflush.c static void local_flush_tlb_range_threshold_asid(unsigned long start, · line 79
Quoted 3 lines
arch/riscv/mm/tlbflush.c +3
-0+if (has_svinval ()) { +local_sfence_w_inval (); +for (i =0 ; i < nr_ptes_in_range; ++i) {Where do we limit this to 64 as the commit message states it does?
If the number of ptes to flush is greater than tlb_flush_all_threshold (= 64), we don't get there so this is indeed limited to 64 entries max.
I think this limit should be different when using svinval, but we can do that later and the goal is to allow the vendors to come with their own threshold anyway.
Thanks for reviving this Mayuresh, I'll add my RB in the next version when you fix Andrew's comments.
Alex
Quoted 21 lines
Quoted 14 lines
arch/riscv/mm/tlbflush.c +7
-0+local_sinval_vma_asid (start, asid); + start += stride; + } +local_sfence_inval_ir (); +return ; + } +for (i =0 ; i < nr_ptes_in_range; ++i) {local_flush_tlb_page_asid (start, asid); start += stride;Signature · 1 line
2.34.1Thanks,
drew_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv