Risc-V Svinval support
v4 cover Risc-V Svinval support
This patch adds support for the Svinval extension as defined in the Risc V Privileged specification.
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: use svinval instructions instead of sfence.vma
arch/riscv/mm/tlbflush.c | 62 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-)
Signature · 7 lines
2.34.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
PATCH v4 1/1 riscv: mm: use svinval instructions instead of sfence.vma
When svinval is supported the local_flush_tlb_page* functions would prefer to 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_page* functions is limited to PTRS_PER_PTE. This is required to avoid soft lockups and the approach is similar to that used in arm64.
--- arch/riscv/mm/tlbflush.c | 62 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-)
arch/riscv/mm/tlbflush.c +56 -6
@@ -5,6 +5,12 @@#include <linux/sched.h> #include <asm/sbi.h> #include <asm/mmu_context.h> +#include <asm/hwcap.h> +#include <asm/insn-def.h> + +#define has_svinval () riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL) + +static unsigned long tlb_flush_all_threshold__read_mostly =PTRS_PER_PTE ;static inline void local_flush_tlb_all_asid (unsigned long asid) {
@@ -26,19 +32,63 @@ static inline void local_flush_tlb_page_asid(unsigned long addr,static inline void local_flush_tlb_range (unsigned long start,unsigned long size,unsigned long stride) { -if (size <= stride) -local_flush_tlb_page (start); -else +if ((size / stride) <= tlb_flush_all_threshold) { +if (has_svinval ()) { + asm volatile(SFENCE_W_INVAL() ::: "memory"); +while (size) { + asm volatile(SINVAL_VMA(%0, zero) + : : "r" (start) : "memory"); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + asm volatile(SFENCE_INVAL_IR() ::: "memory"); + }else { +while (size) { +local_flush_tlb_page (start); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + } + }else {local_flush_tlb_all (); + } }static inline void local_flush_tlb_range_asid (unsigned long start,unsigned long size,unsigned long stride,unsigned long asid) { -if (size <= stride) -local_flush_tlb_page_asid (start, asid); -else +if ((size / stride) <= tlb_flush_all_threshold) { +if (has_svinval ()) { + asm volatile(SFENCE_W_INVAL() ::: "memory"); +while (size) { + asm volatile(SINVAL_VMA(%0, %1) : : "r" (start), + "r" (asid) : "memory"); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + asm volatile(SFENCE_INVAL_IR() ::: "memory"); + }else { +while (size) { +local_flush_tlb_page_asid (start, asid); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + } + }else {local_flush_tlb_all_asid (asid); + } }static void __ipi_flush_tlb_all (void *info)
Signature · 7 lines
2.34.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v4 1/1] riscv: mm: use svinval instructions instead of sfence.vma
Quoted 47 lines · Mayuresh Chitale
When svinval is supported the local_flush_tlb_page* functions would prefer to 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_page* functions is limited to
PTRS_PER_PTE. 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 | 62 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-)arch/riscv/mm/tlbflush.c +8
-3@@ -5,6 +5,12 @@#include <linux/sched.h> #include <asm/sbi.h> #include <asm/mmu_context.h> +#include <asm/hwcap.h> +#include <asm/insn-def.h> + +#define has_svinval () riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL) + +static unsigned long tlb_flush_all_threshold__read_mostly =PTRS_PER_PTE ;static inline void local_flush_tlb_all_asid (unsigned long asid) {@@ -26,19 +32,63 @@ static inline void local_flush_tlb_page_asid(unsigned long addr,static inline void local_flush_tlb_range (unsigned long start,unsigned long size,unsigned long stride) { -if (size <= stride) -local_flush_tlb_page (start); -else +if ((size / stride) <= tlb_flush_all_threshold) { +if (has_svinval ()) {
arch/riscv/mm/tlbflush.c static inline void local_flush_tlb_page_asid(unsigned long addr, · line 46
Quoted 10 lines
arch/riscv/mm/tlbflush.c +10
-0+ asm volatile(SFENCE_W_INVAL() ::: "memory"); +while (size) { + asm volatile(SINVAL_VMA(%0, zero) + : : "r" (start) : "memory"); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + }
nit: The four while loops added by this patch could all be written more concisely as
unsigned long end = start + size;
while (start < end) {
/* flush one */
start += stride;
}And we could shift everything one level of indentation left with
if ((size / stride) > tlb_flush_all_threshold) {
local_flush_tlb_all();
return;
}
if (has_svinval()) {
...Thanks,
drew
Quoted 59 lines
arch/riscv/mm/tlbflush.c +38
-3+ asm volatile(SFENCE_INVAL_IR() ::: "memory"); + }else { +while (size) { +local_flush_tlb_page (start); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + } + }else {local_flush_tlb_all (); + } }static inline void local_flush_tlb_range_asid (unsigned long start,unsigned long size,unsigned long stride,unsigned long asid) { -if (size <= stride) -local_flush_tlb_page_asid (start, asid); -else +if ((size / stride) <= tlb_flush_all_threshold) { +if (has_svinval ()) { + asm volatile(SFENCE_W_INVAL() ::: "memory"); +while (size) { + asm volatile(SINVAL_VMA(%0, %1) : : "r" (start), + "r" (asid) : "memory"); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + asm volatile(SFENCE_INVAL_IR() ::: "memory"); + }else { +while (size) { +local_flush_tlb_page_asid (start, asid); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + } + }else {local_flush_tlb_all_asid (asid); + } }static void __ipi_flush_tlb_all (void *info)Signature · 7 lines
2.34.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v4 1/1] riscv: mm: use svinval instructions instead of sfence.vma
Quoted 70 lines · Andrew Jones
Quoted 47 lines · Mayuresh Chitale
When svinval is supported the local_flush_tlb_page* functions would prefer to 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_page* functions is limited to
PTRS_PER_PTE. 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 | 62 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-)arch/riscv/mm/tlbflush.c +8
-3@@ -5,6 +5,12 @@#include <linux/sched.h> #include <asm/sbi.h> #include <asm/mmu_context.h> +#include <asm/hwcap.h> +#include <asm/insn-def.h> + +#define has_svinval () riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL) + +static unsigned long tlb_flush_all_threshold__read_mostly =PTRS_PER_PTE ;static inline void local_flush_tlb_all_asid (unsigned long asid) {@@ -26,19 +32,63 @@ static inline void local_flush_tlb_page_asid(unsigned long addr,static inline void local_flush_tlb_range (unsigned long start,unsigned long size,unsigned long stride) { -if (size <= stride) -local_flush_tlb_page (start); -else +if ((size / stride) <= tlb_flush_all_threshold) { +if (has_svinval ()) {arch/riscv/mm/tlbflush.c static inline void local_flush_tlb_page_asid(unsigned long addr, · line 46
Quoted 10 lines
arch/riscv/mm/tlbflush.c +10
-0+ asm volatile(SFENCE_W_INVAL() ::: "memory"); +while (size) { + asm volatile(SINVAL_VMA(%0, zero) + : : "r" (start) : "memory"); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + }nit: The four while loops added by this patch could all be written more concisely as
unsigned long end = start + size;
while (start < end) { /* flush one */ start += stride; }
Quoted 8 lines
And we could shift everything one level of indentation left with
if ((size / stride) > tlb_flush_all_threshold) { local_flush_tlb_all(); return; } if (has_svinval()) {
Thx Drew. I will update in the next revision
Quoted 64 lines
...
Thanks,
drewQuoted 59 lines
arch/riscv/mm/tlbflush.c +38
-4+ asm volatile(SFENCE_INVAL_IR() ::: "memory"); + }else { +while (size) { +local_flush_tlb_page (start); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + } + }else {local_flush_tlb_all (); + } }static inline void local_flush_tlb_range_asid (unsigned long start,unsigned long size,unsigned long stride,unsigned long asid) { -if (size <= stride) -local_flush_tlb_page_asid (start, asid); -else +if ((size / stride) <= tlb_flush_all_threshold) { +if (has_svinval ()) { + asm volatile(SFENCE_W_INVAL() ::: "memory"); +while (size) { + asm volatile(SINVAL_VMA(%0, %1) : : "r" (start), + "r" (asid) : "memory"); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + asm volatile(SFENCE_INVAL_IR() ::: "memory"); + }else { +while (size) { +local_flush_tlb_page_asid (start, asid); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + } + }else {local_flush_tlb_all_asid (asid); + } }static void __ipi_flush_tlb_all (void *info) --2.34.1
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Alexandre Ghiti
reviewerRe: [PATCH v4 1/1] riscv: mm: use svinval instructions instead of sfence.vma
In reply to Mayuresh ChitaleHi Mayuresh,
Quoted 26 lines · Mayuresh Chitale
When svinval is supported the local_flush_tlb_page* functions would prefer to 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_page* functions is limited to
PTRS_PER_PTE. 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 | 62 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-)arch/riscv/mm/tlbflush.c +0
-0
arch/riscv/mm/tlbflush.c line 13
Quoted 9 lines
arch/riscv/mm/tlbflush.c +6
-0@@ -5,6 +5,12 @@#include <linux/sched.h> #include <asm/sbi.h> #include <asm/mmu_context.h> +#include <asm/hwcap.h> +#include <asm/insn-def.h> + +#define has_svinval () riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL) + +static unsigned long tlb_flush_all_threshold__read_mostly =PTRS_PER_PTE ;
The threshold is quite high to me: internally, we computed that something like 50 would be a good bet, because to me, 512 * sfence.vma (or svinval equivalent) takes way more time than just flushing the whole tlb (even with the whole refill I'd say). How did you get this number? And this value is micro-architecture dependent, so we need to find a consensus or a mechanism to allow a vendor to change it.
FYI, x86 threshold is 33 (can't find right now the pointer sorry).
arch/riscv/mm/tlbflush.c static inline void local_flush_tlb_page_asid(unsigned long addr, · line 35
Quoted 11 lines
arch/riscv/mm/tlbflush.c +1
-3static inline void local_flush_tlb_all_asid (unsigned long asid) {@@ -26,19 +32,63 @@ static inline void local_flush_tlb_page_asid(unsigned long addr,static inline void local_flush_tlb_range (unsigned long start,unsigned long size,unsigned long stride) { -if (size <= stride) -local_flush_tlb_page (start); -else +if ((size / stride) <= tlb_flush_all_threshold) {
If size is not aligned on stride, you could get another page to flush so you do not respect the threshold. In my patchset, I used DIV_ROUND_UP.
Quoted 52 lines
arch/riscv/mm/tlbflush.c +43
-3+if (has_svinval ()) { + asm volatile(SFENCE_W_INVAL() ::: "memory"); +while (size) { + asm volatile(SINVAL_VMA(%0, zero) + : : "r" (start) : "memory"); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + asm volatile(SFENCE_INVAL_IR() ::: "memory"); + }else { +while (size) { +local_flush_tlb_page (start); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + } + }else {local_flush_tlb_all (); + } }static inline void local_flush_tlb_range_asid (unsigned long start,unsigned long size,unsigned long stride,unsigned long asid) { -if (size <= stride) -local_flush_tlb_page_asid (start, asid); -else +if ((size / stride) <= tlb_flush_all_threshold) { +if (has_svinval ()) { + asm volatile(SFENCE_W_INVAL() ::: "memory"); +while (size) { + asm volatile(SINVAL_VMA(%0, %1) : : "r" (start), + "r" (asid) : "memory"); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + asm volatile(SFENCE_INVAL_IR() ::: "memory"); + }else { +while (size) { +local_flush_tlb_page_asid (start, asid); + start += stride; +if (size > stride) + size -= stride;
arch/riscv/mm/tlbflush.c static inline void local_flush_tlb_page_asid(unsigned long addr, · line 94
Quoted 10 lines
arch/riscv/mm/tlbflush.c +6
-0+else + size =0 ; + } + } + }else {local_flush_tlb_all_asid (asid); + } }static void __ipi_flush_tlb_all (void *info)
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v4 1/1] riscv: mm: use svinval instructions instead of sfence.vma
Hi Alex,
Quoted 41 lines · Alexandre Ghiti
Hi Mayuresh,
Quoted 26 lines · Mayuresh Chitale
When svinval is supported the local_flush_tlb_page* functions would prefer to 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_page* functions is limited to
PTRS_PER_PTE. 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 | 62 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-)arch/riscv/mm/tlbflush.c +0
-0arch/riscv/mm/tlbflush.c line 13
Quoted 9 lines
arch/riscv/mm/tlbflush.c +6
-0@@ -5,6 +5,12 @@#include <linux/sched.h> #include <asm/sbi.h> #include <asm/mmu_context.h> +#include <asm/hwcap.h> +#include <asm/insn-def.h> + +#define has_svinval () riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL) + +static unsigned long tlb_flush_all_threshold__read_mostly =PTRS_PER_PTE ;
Quoted 6 lines
The threshold is quite high to me: internally, we computed that something like 50 would be a good bet, because to me, 512 * sfence.vma (or svinval equivalent) takes way more time than just flushing the whole tlb (even with the whole refill I'd say). How did you get this number? And this value is micro-architecture dependent, so we need to find a consensus or a mechanism to allow a vendor to change it.
I had borrowed this limit from ARM64 but I agree that this is micro-architecture specific.
So how about we make this a global variable and let the platform override it?
Quoted 5 lines
FYI, x86 threshold is 33 (can't find right now the pointer sorry).
Quoted 1 line
Quoted 14 lines
arch/riscv/mm/tlbflush.c static inline void local_flush_tlb_page_asid(unsigned long addr, · line 35
Quoted 10 lines
static inline void
local_flush_tlb_all_asid(unsigned long asid){arch/riscv/mm/tlbflush.c +1
-3@@ -26,19 +32,63 @@ static inline void local_flush_tlb_page_asid(unsigned long addr,static inline void local_flush_tlb_range (unsigned long start,unsigned long size,unsigned long stride) { -if (size <= stride) -local_flush_tlb_page (start); -else +if ((size / stride) <= tlb_flush_all_threshold) {If size is not aligned on stride, you could get another page to flush so you do not respect the threshold. In my patchset, I used
DIV_ROUND_UP.
Ok. Will change it in the next revision.
Quoted 63 lines
Quoted 63 lines
arch/riscv/mm/tlbflush.c +49
-3+if (has_svinval ()) { + asm volatile(SFENCE_W_INVAL() ::: "memory"); +while (size) { + asm volatile(SINVAL_VMA(%0, zero) + : : "r" (start) : "memory"); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + asm volatile(SFENCE_INVAL_IR() ::: "memory"); + }else { +while (size) { +local_flush_tlb_page (start); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + } + }else {local_flush_tlb_all (); + } }static inline void local_flush_tlb_range_asid (unsigned long start,unsigned long size,unsigned long stride,unsigned long asid) { -if (size <= stride) -local_flush_tlb_page_asid (start, asid); -else +if ((size / stride) <= tlb_flush_all_threshold) { +if (has_svinval ()) { + asm volatile(SFENCE_W_INVAL() ::: "memory"); +while (size) { + asm volatile(SINVAL_VMA(%0, %1) : : "r" (start), + "r" (asid) : "memory"); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + asm volatile(SFENCE_INVAL_IR() ::: "memory"); + }else { +while (size) { +local_flush_tlb_page_asid (start, asid); + start += stride; +if (size > stride) + size -= stride; +else + size =0 ; + } + } + }else {local_flush_tlb_all_asid (asid); + } }
Quoted 1 line
arch/riscv/mm/tlbflush.c static inline void local_flush_tlb_page_asid(unsigned long addr, · line 94
Quoted 1 line
arch/riscv/mm/tlbflush.c +0
-0static void __ipi_flush_tlb_all (void *info)
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv