Skip to contentSkip to conversation

riscv: mm: Add support for Svinval extension

PATCH In review Mayuresh Chitale 4 participants 8 messages Jul 2, 2024 – Jun 13, 2025 RISC-V

v8 of 11 In review Latest is v11 →

lore
Share & tools
lore
#0 lore raw

PATCH v8 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.

---
Changes in v8:
- Fix line wrap
- Add RB tag

Changes in v7:
- Use existing svinval macros in the insn-def.h
- Rename local_sinval_vma_asid to local_sinval_vma

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

 arch/riscv/mm/tlbflush.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
arch/riscv/mm/tlbflush.c +31 -0
index 9b6e86ce3867..782147a63f3b 100644
@@ -6,6 +6,27 @@
 #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)
+{
+	asm volatile(SFENCE_INVAL_IR() ::: "memory");
+}
+
+static inline void local_sfence_w_inval(void)
+{
+	asm volatile(SFENCE_W_INVAL() ::: "memory");
+}
+
+static inline void local_sinval_vma(unsigned long vma, unsigned long asid)
+{
+	if (asid != FLUSH_TLB_NO_ASID)
+		asm volatile(SINVAL_VMA(%0, %1) : : "r" (vma), "r" (asid) : "memory");
+	else
+		asm volatile(SINVAL_VMA(%0, zero) : : "r" (vma) : "memory");
+}
 
 /*
  * Flush entire TLB if number of entries to be flushed is greater
@@ -26,6 +47,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(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
Reviewed-by on v8 1/1
#1 lore raw

Hi Mayuresh,

Quoted 103 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_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.

---
Changes in v8:
- Fix line wrap
- Add RB tag

Changes in v7:
- Use existing svinval macros in the insn-def.h
- Rename local_sinval_vma_asid to local_sinval_vma

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

 arch/riscv/mm/tlbflush.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
arch/riscv/mm/tlbflush.c +31 -1
index 9b6e86ce3867..782147a63f3b 100644
@@ -6,6 +6,27 @@
 #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)
+{
+       asm volatile(SFENCE_INVAL_IR() ::: "memory");
+}
+
+static inline void local_sfence_w_inval(void)
+{
+       asm volatile(SFENCE_W_INVAL() ::: "memory");
+}
+
+static inline void local_sinval_vma(unsigned long vma, unsigned long asid)
+{
+       if (asid != FLUSH_TLB_NO_ASID)
+               asm volatile(SINVAL_VMA(%0, %1) : : "r" (vma), "r" (asid) : "memory");
+       else
+               asm volatile(SINVAL_VMA(%0, zero) : : "r" (vma) : "memory");
+}
 
 /*
  * Flush entire TLB if number of entries to be flushed is greater
@@ -26,6 +47,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(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;
--
Quoted 2 lines

2.34.1

Great, thanks again for reworking this patchset!

Thanks,

Alex

maintainer
#2 lore raw
In reply to Mayuresh Chitale
Quoted 4 lines · On Tue, 02 Jul 2024 03:26:37 PDT (-0700), [email protected] wrote:

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

Do you have any performance numbers for the optimization? As per here <https://lore.kernel.org/all/mhng-f799bd2b-7f22-4c03-bdb2-903fa3b5d508@palmer-ri-x1c9a/>.

Quoted 97 lines

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.

---
Changes in v8:
- Fix line wrap
- Add RB tag

Changes in v7:
- Use existing svinval macros in the insn-def.h
- Rename local_sinval_vma_asid to local_sinval_vma

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

 arch/riscv/mm/tlbflush.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
arch/riscv/mm/tlbflush.c +31 -0
index 9b6e86ce3867..782147a63f3b 100644
@@ -6,6 +6,27 @@
 #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)
+{
+	asm volatile(SFENCE_INVAL_IR() ::: "memory");
+}
+
+static inline void local_sfence_w_inval(void)
+{
+	asm volatile(SFENCE_W_INVAL() ::: "memory");
+}
+
+static inline void local_sinval_vma(unsigned long vma, unsigned long asid)
+{
+	if (asid != FLUSH_TLB_NO_ASID)
+		asm volatile(SINVAL_VMA(%0, %1) : : "r" (vma), "r" (asid) : "memory");
+	else
+		asm volatile(SINVAL_VMA(%0, zero) : : "r" (vma) : "memory");
+}
 
 /*
  * Flush entire TLB if number of entries to be flushed is greater
@@ -26,6 +47,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(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;
#3 lore raw
Quoted 9 lines · Palmer Dabbelt
Quoted 4 lines · On Tue, 02 Jul 2024 03:26:37 PDT (-0700), [email protected] wrote:

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

Do you have any performance numbers for the optimization? As per here
<https://lore.kernel.org/all/mhng-f799bd2b-7f22-4c03-bdb2-903fa3b5d508@palmer-ri-x1c9a/>.

No, currently there are no numbers available for comparison but the rationale for the optimization is described in the spec. The extension is mandatory for the RVA23S64 profile but any platform that doesn't support this extension will not be impacted as the code executes only if the svinval extension is enabled at the boot up.

Quoted 98 lines
Quoted 98 lines

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.

---
Changes in v8:
- Fix line wrap
- Add RB tag

Changes in v7:
- Use existing svinval macros in the insn-def.h
- Rename local_sinval_vma_asid to local_sinval_vma

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

 arch/riscv/mm/tlbflush.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
arch/riscv/mm/tlbflush.c +31 -0
index 9b6e86ce3867..782147a63f3b 100644
@@ -6,6 +6,27 @@
 #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)
+{
+     asm volatile(SFENCE_INVAL_IR() ::: "memory");
+}
+
+static inline void local_sfence_w_inval(void)
+{
+     asm volatile(SFENCE_W_INVAL() ::: "memory");
+}
+
+static inline void local_sinval_vma(unsigned long vma, unsigned long asid)
+{
+     if (asid != FLUSH_TLB_NO_ASID)
+             asm volatile(SINVAL_VMA(%0, %1) : : "r" (vma), "r" (asid) : "memory");
+     else
+             asm volatile(SINVAL_VMA(%0, zero) : : "r" (vma) : "memory");
+}
 
 /*
  * Flush entire TLB if number of entries to be flushed is greater
@@ -26,6 +47,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(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;
#4 lore raw

Hi Mayuresh!

Quoted 6 lines · Mayuresh Chitale
Quoted 5 lines · Palmer Dabbelt
Quoted 4 lines · On Tue, 02 Jul 2024 03:26:37 PDT (-0700), [email protected] wrote:

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

Quoted 7 lines
Quoted 2 lines

Do you have any performance numbers for the optimization? As per here
<https://lore.kernel.org/all/mhng-f799bd2b-7f22-4c03-bdb2-903fa3b5d508@palmer-ri-x1c9a/>.

No, currently there are no numbers available for comparison but the rationale for the optimization is described in the spec. The extension is mandatory for the RVA23S64 profile but any platform that doesn't support this extension will not be impacted as the code executes only if the svinval extension is enabled at the boot up.

So I finally have some numbers! I tested this patchset on the BananaPi: I measured the number of cycles when flushing 64 entries (which is our current threshold) and I made sure to touch the entries beforehand.

Here they are:

* svinval:

#cycles: 364920
#cycles: 365856
#cycles: 367993

* !svinval:

#cycles: 663585
#cycles: 663105
#cycles: 664073

So that's roughly /2 using svinval. To me that's good enough to merge that for 6.16 :)

Sorry for the very very long delay and thanks again for the multiple revisions!

Alex

Quoted 101 lines
Quoted 84 lines
Quoted 84 lines

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.

---
Changes in v8:
- Fix line wrap
- Add RB tag

Changes in v7:
- Use existing svinval macros in the insn-def.h
- Rename local_sinval_vma_asid to local_sinval_vma

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

  arch/riscv/mm/tlbflush.c | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)
arch/riscv/mm/tlbflush.c +21 -0
index 9b6e86ce3867..782147a63f3b 100644
@@ -6,6 +6,27 @@
  #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)
+{
+     asm volatile(SFENCE_INVAL_IR() ::: "memory");
+}
+
+static inline void local_sfence_w_inval(void)
+{
+     asm volatile(SFENCE_W_INVAL() ::: "memory");
+}
+
+static inline void local_sinval_vma(unsigned long vma, unsigned long asid)
+{
+     if (asid != FLUSH_TLB_NO_ASID)
+             asm volatile(SINVAL_VMA(%0, %1) : : "r" (vma), "r" (asid) : "memory");
+     else
+             asm volatile(SINVAL_VMA(%0, zero) : : "r" (vma) : "memory");
+}
 
  /*
   * Flush entire TLB if number of entries to be flushed is greater
@@ -26,6 +47,16 @@ static void local_flush_tlb_range_threshold_asid(unsigned long start,
               return;
       }
 
Quoted 13 lines

arch/riscv/mm/tlbflush.c static void local_flush_tlb_range_threshold_asid(unsigned long start, · line 62

Quoted 13 lines
arch/riscv/mm/tlbflush.c +10 -0
+     if (has_svinval()) {
+             local_sfence_w_inval();
+             for (i = 0; i < nr_ptes_in_range; ++i) {
+                     local_sinval_vma(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;

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

#5 lore raw

Hi Alex,

Quoted 31 lines · Alexandre Ghiti

Hi Mayuresh!

Quoted 6 lines · Mayuresh Chitale
Quoted 5 lines · Palmer Dabbelt
Quoted 4 lines · On Tue, 02 Jul 2024 03:26:37 PDT (-0700), [email protected] wrote:

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

Quoted 7 lines
Quoted 2 lines

Do you have any performance numbers for the optimization? As per here
<https://lore.kernel.org/all/mhng-f799bd2b-7f22-4c03-bdb2-903fa3b5d508@palmer-ri-x1c9a/>.

No, currently there are no numbers available for comparison but the rationale for the optimization is described in the spec. The extension is mandatory for the RVA23S64 profile but any platform that doesn't support this extension will not be impacted as the code executes only if the svinval extension is enabled at the boot up.

So I finally have some numbers! I tested this patchset on the BananaPi:
I measured the number of cycles when flushing 64 entries (which is our current threshold) and I made sure to touch the entries beforehand.

Here they are:

* svinval:

#cycles: 364920
#cycles: 365856
#cycles: 367993

Quoted 6 lines

* !svinval:

#cycles: 663585
#cycles: 663105
#cycles: 664073

That's awesome !! Thank you so much for getting the data.

Quoted 110 lines

So that's roughly /2 using svinval. To me that's good enough to merge that for 6.16 :)

Sorry for the very very long delay and thanks again for the multiple revisions!

Alex

Quoted 101 lines
Quoted 84 lines
Quoted 84 lines

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.

---
Changes in v8:
- Fix line wrap
- Add RB tag

Changes in v7:
- Use existing svinval macros in the insn-def.h
- Rename local_sinval_vma_asid to local_sinval_vma

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

  arch/riscv/mm/tlbflush.c | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)
arch/riscv/mm/tlbflush.c +21 -0
index 9b6e86ce3867..782147a63f3b 100644
@@ -6,6 +6,27 @@
  #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)
+{
+     asm volatile(SFENCE_INVAL_IR() ::: "memory");
+}
+
+static inline void local_sfence_w_inval(void)
+{
+     asm volatile(SFENCE_W_INVAL() ::: "memory");
+}
+
+static inline void local_sinval_vma(unsigned long vma, unsigned long asid)
+{
+     if (asid != FLUSH_TLB_NO_ASID)
+             asm volatile(SINVAL_VMA(%0, %1) : : "r" (vma), "r" (asid) : "memory");
+     else
+             asm volatile(SINVAL_VMA(%0, zero) : : "r" (vma) : "memory");
+}
 
  /*
   * Flush entire TLB if number of entries to be flushed is greater
@@ -26,6 +47,16 @@ static void local_flush_tlb_range_threshold_asid(unsigned long start,
               return;
       }
 
Quoted 13 lines

arch/riscv/mm/tlbflush.c static void local_flush_tlb_range_threshold_asid(unsigned long start, · line 62

Quoted 13 lines
arch/riscv/mm/tlbflush.c +10 -0
+     if (has_svinval()) {
+             local_sfence_w_inval();
+             for (i = 0; i < nr_ptes_in_range; ++i) {
+                     local_sinval_vma(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;

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

maintainer
#6 lore raw
Quoted 20 lines · On Thu, 22 May 2025 08:11:09 PDT (-0700), [email protected] wrote:

Hi Alex,

Quoted 17 lines · Alexandre Ghiti

Hi Mayuresh!

Quoted 13 lines · Mayuresh Chitale
Quoted 7 lines · Palmer Dabbelt
Quoted 4 lines · On Tue, 02 Jul 2024 03:26:37 PDT (-0700), [email protected] wrote:

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

Do you have any performance numbers for the optimization? As per here
<https://lore.kernel.org/all/mhng-f799bd2b-7f22-4c03-bdb2-903fa3b5d508@palmer-ri-x1c9a/>.

No, currently there are no numbers available for comparison but the rationale for the optimization is described in the spec. The extension is mandatory for the RVA23S64 profile but any platform that doesn't support this extension will not be impacted as the code executes only if the svinval extension is enabled at the boot up.

I seem to remember writing something like this up at some point, but I didn't see it so I figured I'd just write it up again in case anyone's looking in the future...

My core worry here is that there's two ways to tuse the Svinval extension:

* This, which just replaces the sfence.vma with a loop of Svinval instructions. This keeps the Svinval instructions close to each other.
* Some more extensive modifications to move the Svinval instructions farther apart from each other.

Which approach is better is going to depend on what them microarchicture implements: does it want the Svinval instructions close to each other so it can pattern match them, or does it want them far apart so the ordering instructions stall less. It's entirely possible that the wrong flavor of Svinval will actually decrease performance on systems that were built around the other flavor, and we don't know which flavor is getting implemented.

So it's not one of these extensions where we can just say "these are only used on systems that implement the extensions, so it's obviously faster". We really need some numbers to demonstrate this flavor is actually faster...

Quoted 12 lines
Quoted 12 lines

So I finally have some numbers! I tested this patchset on the BananaPi:
I measured the number of cycles when flushing 64 entries (which is our current threshold) and I made sure to touch the entries beforehand.

Here they are:

* svinval:

#cycles: 364920
#cycles: 365856
#cycles: 367993

Quoted 8 lines
Quoted 6 lines

* !svinval:

#cycles: 663585
#cycles: 663105
#cycles: 664073

That's awesome !! Thank you so much for getting the data.

So I think that's good enough to start, so it's going up to Linus and hopefully it lands (there's been a bit of chaos, but hopefully that's all sorted out).

Quoted 2 lines
Quoted 2 lines

So that's roughly /2 using svinval. To me that's good enough to merge that for 6.16 :)

Ya, but probably low enough that we're going to want a lower threshold too -- if the svinval loop is only a factor of 2 faster for 64 entries, it's probably going to end up slower at some point...

On the flip side, it's a big enough change that we probably want to re-evaluate tlb_flush_all_threshold, too.

Quoted 107 lines
Quoted 107 lines

Sorry for the very very long delay and thanks again for the multiple revisions!

Alex

Quoted 101 lines
Quoted 84 lines
Quoted 84 lines

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.

---
Changes in v8:
- Fix line wrap
- Add RB tag

Changes in v7:
- Use existing svinval macros in the insn-def.h
- Rename local_sinval_vma_asid to local_sinval_vma

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

  arch/riscv/mm/tlbflush.c | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)
arch/riscv/mm/tlbflush.c +21 -0
index 9b6e86ce3867..782147a63f3b 100644
@@ -6,6 +6,27 @@
  #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)
+{
+     asm volatile(SFENCE_INVAL_IR() ::: "memory");
+}
+
+static inline void local_sfence_w_inval(void)
+{
+     asm volatile(SFENCE_W_INVAL() ::: "memory");
+}
+
+static inline void local_sinval_vma(unsigned long vma, unsigned long asid)
+{
+     if (asid != FLUSH_TLB_NO_ASID)
+             asm volatile(SINVAL_VMA(%0, %1) : : "r" (vma), "r" (asid) : "memory");
+     else
+             asm volatile(SINVAL_VMA(%0, zero) : : "r" (vma) : "memory");
+}
 
  /*
   * Flush entire TLB if number of entries to be flushed is greater
@@ -26,6 +47,16 @@ static void local_flush_tlb_range_threshold_asid(unsigned long start,
               return;
       }
 
Quoted 13 lines

arch/riscv/mm/tlbflush.c static void local_flush_tlb_range_threshold_asid(unsigned long start, · line 62

Quoted 13 lines
arch/riscv/mm/tlbflush.c +10 -0
+     if (has_svinval()) {
+             local_sfence_w_inval();
+             for (i = 0; i < nr_ptes_in_range; ++i) {
+                     local_sinval_vma(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;

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

#7 lore raw
Quoted 31 lines · Palmer Dabbelt
Quoted 29 lines · On Thu, 22 May 2025 08:11:09 PDT (-0700), [email protected] wrote:

Hi Alex,

Quoted 26 lines · Alexandre Ghiti

Hi Mayuresh!

Quoted 1 line · Mayuresh Chitale

On Wed, Jul 24, 2024 at 8:20 PM Palmer Dabbelt

Quoted 1 line · <[email protected]> wrote:
Quoted 1 line

On Tue, 02 Jul 2024 03:26:37 PDT (-0700),

Quoted 1 line · [email protected] wrote:
Quoted 1 line
Quoted 1 line

The Svinval extension splits SFENCE.VMA instruction into

finer-grained

Quoted 1 line
Quoted 1 line
Quoted 1 line

invalidation and ordering operations and is mandatory for

RVA23S64 profile.

Quoted 1 line
Quoted 1 line
Quoted 1 line

When Svinval is enabled the local_flush_tlb_range_threshold_asid

function

Quoted 1 line
Quoted 1 line
Quoted 1 line

should use the following sequence to optimize the tlb flushes

instead of

Quoted 1 line
Quoted 1 line

Do you have any performance numbers for the optimization?  As per

here

Quoted 1 line
Quoted 1 line

<https://lore.kernel.org/all/mhng-f799bd2b-7f22-4c03-bdb2-903fa3b5d508@palmer-ri-x1c9a/>.

Quoted 2 lines

No, currently there are no numbers available for comparison but the rationale for the optimization is described in the spec. The

extension

Quoted 3 lines

is mandatory for the RVA23S64 profile but any platform that doesn't support this extension will not be impacted as the code executes only if the svinval extension is enabled at the boot up.

Quoted 6 lines

I seem to remember writing something like this up at some point, but I didn't see it so I figured I'd just write it up again in case anyone's looking in the future...

My core worry here  is that there's two ways to tuse the Svinval extension:

* This, which just replaces the sfence.vma with a loop of Svinval  instructions.  This keeps the Svinval instructions close to each  other.
* Some more extensive modifications to move the Svinval instructions  farther apart from each other.

Yes and those 2 ways are complementary, the next step is to implement the second one.

Quoted 4 lines

Which approach is better is going to depend on what them microarchicture implements: does it want the Svinval instructions close to each other so it can pattern match them, or does it want them far apart so the ordering instructions stall less.  It's entirely possible that the wrong flavor of Svinval will actually decrease performance on systems that were built around the other flavor, and we don't know which flavor is getting implemented.

So it's not one of these extensions where we can just say "these are only used on systems that implement the extensions, so it's obviously faster".  We really need some numbers to demonstrate this flavor is actually faster...

I don't see how an svinval implementation would lead in that case to poorer performance, unless the platform implements a "dummy" svinval to conform to rva23, in that case, I think we should not worry about this case in the long run.

Quoted 148 lines
Quoted 13 lines
Quoted 13 lines

So I finally have some numbers! I tested this patchset on the BananaPi:
I measured the number of cycles when flushing 64 entries (which is our current threshold) and I made sure to touch the entries beforehand.

Here they are:

* svinval:

#cycles: 364920
#cycles: 365856
#cycles: 367993

Quoted 8 lines
Quoted 6 lines

* !svinval:

#cycles: 663585
#cycles: 663105
#cycles: 664073

That's awesome !! Thank you so much for getting the data.

So I think that's good enough to start, so it's going up to Linus and hopefully it lands (there's been a bit of chaos, but hopefully that's all sorted out).

Quoted 2 lines
Quoted 2 lines

So that's roughly /2 using svinval. To me that's good enough to merge that for 6.16 :)

Ya, but probably low enough that we're going to want a lower threshold too -- if the svinval loop is only a factor of 2 faster for 64 entries, it's probably going to end up slower at some point...

On the flip side, it's a big enough change that we probably want to re-evaluate tlb_flush_all_threshold, too.

Quoted 108 lines
Quoted 108 lines

Sorry for the very very long delay and thanks again for the multiple revisions!

Alex

Quoted 48 lines
Quoted 48 lines
Quoted 48 lines

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.

---
Changes in v8:
- Fix line wrap
- Add RB tag

Changes in v7:
- Use existing svinval macros in the insn-def.h
- Rename local_sinval_vma_asid to local_sinval_vma

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

   arch/riscv/mm/tlbflush.c | 32 ++++++++++++++++++++++++++++++++

   1 file changed, 32 insertions(+)
Quoted 11 lines
Quoted 11 lines

arch/riscv/mm/tlbflush.c line 11

Quoted 11 lines
arch/riscv/mm/tlbflush.c +3 -0
index 9b6e86ce3867..782147a63f3b 100644
@@ -6,6 +6,27 @@
    #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)

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
arch/riscv/mm/tlbflush.c +4 -0
+
+static inline void local_sfence_inval_ir(void)
+{
+     asm volatile(SFENCE_INVAL_IR() ::: "memory");
Quoted 8 lines
Quoted 8 lines

arch/riscv/mm/tlbflush.c

Quoted 8 lines
arch/riscv/mm/tlbflush.c +8 -0
+}
+
+static inline void local_sfence_w_inval(void)
+{
+     asm volatile(SFENCE_W_INVAL() ::: "memory");
+}
+
+static inline void local_sinval_vma(unsigned long vma, unsigned 

long asid)

Quoted 3 lines
Quoted 3 lines

arch/riscv/mm/tlbflush.c

Quoted 3 lines
arch/riscv/mm/tlbflush.c +3 -0
+{
+     if (asid != FLUSH_TLB_NO_ASID)
+             asm volatile(SINVAL_VMA(%0, %1) : : "r" (vma), "r" 

(asid) : "memory");

Quoted 2 lines
Quoted 2 lines

arch/riscv/mm/tlbflush.c line 28

Quoted 2 lines
arch/riscv/mm/tlbflush.c +2 -0
+     else
+             asm volatile(SINVAL_VMA(%0, zero) : : "r" (vma) : 

"memory");

Quoted 5 lines
Quoted 5 lines

arch/riscv/mm/tlbflush.c static void

Quoted 5 lines
arch/riscv/mm/tlbflush.c +1 -0
+}
 
    /*
     * Flush entire TLB if number of entries to be flushed is greater
@@ -26,6 +47,16 @@ static void 

local_flush_tlb_range_threshold_asid(unsigned long start,

Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
arch/riscv/mm/tlbflush.c +10 -0
                 return;
         }
 
+     if (has_svinval()) {
+             local_sfence_w_inval();
+             for (i = 0; i < nr_ptes_in_range; ++i) {
+                     local_sinval_vma(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;
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

_______________________________________________
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