While reviewing Alexandre Ghiti's "riscv: tlb flush improvements"
series[1], I noticed that most TLB flush functions end up as a call to
local_flush_tlb_all() when SMP is disabled. This series resolves that,
and also optimizes the scenario where SMP is enabled but only one CPU is
present or online. Along the way, I realized that we should be using
single-ASID flushes wherever possible, so I implemented that as well.
Here are some numbers from D1 (with SMP disabled) which show the
performance impact:
v6.7-rc8:
System Benchmarks Partial Index BASELINE RESULT INDEX
Execl Throughput 43.0 207.4 48.2
File Copy 1024 bufsize 2000 maxblocks 3960.0 52187.4 131.8
File Copy 256 bufsize 500 maxblocks 1655.0 14872.6 89.9
File Copy 4096 bufsize 8000 maxblocks 5800.0 146597.8 252.8
Pipe Throughput 12440.0 125318.4 100.7
Pipe-based Context Switching 4000.0 17804.2 44.5
Process Creation 126.0 479.2 38.0
Shell Scripts (1 concurrent) 42.4 564.5 133.1
Shell Scripts (16 concurrent) --- 36.8 ---
Shell Scripts (8 concurrent) 6.0 74.3 123.9
System Call Overhead 15000.0 182050.7 121.4
========
System Benchmarks Index Score (Partial Only) 93.2
v6.7-rc8 plus this patch series:
System Benchmarks Partial Index BASELINE RESULT INDEX
Execl Throughput 43.0 208.5 48.5
File Copy 1024 bufsize 2000 maxblocks 3960.0 56847.0 143.6
File Copy 256 bufsize 500 maxblocks 1655.0 17728.9 107.1
File Copy 4096 bufsize 8000 maxblocks 5800.0 168016.2 289.7
Pipe Throughput 12440.0 133376.2 107.2
Pipe-based Context Switching 4000.0 19736.3 49.3
Process Creation 126.0 484.5 38.4
Shell Scripts (1 concurrent) 42.4 564.1 133.0
Shell Scripts (16 concurrent) --- 36.6 ---
Shell Scripts (8 concurrent) 6.0 74.1 123.5
System Call Overhead 15000.0 210181.8 140.1
========
System Benchmarks Index Score (Partial Only) 100.1
[1]: https://lore.kernel.org/linux-riscv/[email protected]/
Changes in v4:
- Fix a possible race between flush_icache_*() and SMP bringup
- Refactor riscv_use_ipi_for_rfence() to make later changes cleaner
- Optimize kernel TLB flushes with only one CPU online
- Optimize global cache/TLB flushes with only one CPU online
- Merge the two copies of __flush_tlb_range() and rely on the compiler
to optimize out the broadcast path (both clang and gcc do this)
- Merge the two copies of flush_tlb_all() and rely on constant folding
- Only set tlb_flush_all_threshold when CONFIG_MMU=y.
Changes in v3:
- Fixed a performance regression caused by executing sfence.vma in a
loop on implementations affected by SiFive CIP-1200
- Rebased on v6.7-rc1
Changes in v2:
- Move the SMP/UP merge earlier in the series to avoid build issues
- Make a copy of __flush_tlb_range() instead of adding ifdefs inside
- local_flush_tlb_all() is the only function used on !MMU (smpboot.c)
Samuel Holland (12):
riscv: Flush the instruction cache during SMP bringup
riscv: Use IPIs for remote cache/TLB flushes by default
riscv: mm: Broadcast kernel TLB flushes only when needed
riscv: Only send remote fences when some other CPU is online
riscv: mm: Combine the SMP and UP TLB flush code
riscv: Apply SiFive CIP-1200 workaround to single-ASID sfence.vma
riscv: Avoid TLB flush loops when affected by SiFive CIP-1200
riscv: mm: Introduce cntx2asid/cntx2version helper macros
riscv: mm: Use a fixed layout for the MM context ID
riscv: mm: Make asid_bits a local variable
riscv: mm: Preserve global TLB entries when switching contexts
riscv: mm: Always use an ASID to flush mm contexts
arch/riscv/errata/sifive/errata.c | 5 ++
arch/riscv/include/asm/errata_list.h | 12 ++++-
arch/riscv/include/asm/mmu.h | 3 ++
arch/riscv/include/asm/mmu_context.h | 2 -
arch/riscv/include/asm/sbi.h | 4 ++
arch/riscv/include/asm/smp.h | 15 +-----
arch/riscv/include/asm/tlbflush.h | 50 ++++++++----------
arch/riscv/kernel/sbi-ipi.c | 11 +++-
arch/riscv/kernel/smp.c | 11 +---
arch/riscv/kernel/smpboot.c | 7 +--
arch/riscv/mm/Makefile | 5 +-
arch/riscv/mm/cacheflush.c | 7 +--
arch/riscv/mm/context.c | 26 ++++------
arch/riscv/mm/tlbflush.c | 76 +++++++++-------------------
drivers/clocksource/timer-clint.c | 2 +-
15 files changed, 102 insertions(+), 134 deletions(-)
--
2.42.0
Instruction cache flush IPIs are sent only to CPUs in cpu_online_mask,
so they will not target a CPU until it calls set_cpu_online() earlier in
smp_callin(). As a result, if instruction memory is modified between the
CPU coming out of reset and that point, then its instruction cache may
contain stale data. Therefore, the instruction cache must be flushed
after the set_cpu_online() synchronization point.
Fixes: 08f051eda33b ("RISC-V: Flush I$ when making a dirty page executable")
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v4:
- New patch for v4
arch/riscv/kernel/smpboot.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index d162bf339beb..48af5bd3ec30 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -26,7 +26,7 @@
#include <linux/sched/task_stack.h>
#include <linux/sched/mm.h>
-#include <asm/cpufeature.h>
+#include <asm/cacheflush.h>
#include <asm/cpu_ops.h>
#include <asm/cpufeature.h>
#include <asm/irq.h>
@@ -257,9 +257,10 @@ asmlinkage __visible void smp_callin(void)
riscv_user_isa_enable();
/*
- * Remote TLB flushes are ignored while the CPU is offline, so emit
- * a local TLB flush right now just in case.
+ * Remote cache and TLB flushes are ignored while the CPU is offline,
+ * so flush them both right now just in case.
*/
+ local_flush_icache_all();
local_flush_tlb_all();
complete(&cpu_running);
/*
--
2.42.0
__flush_tlb_range() avoids broadcasting TLB flushes when an mm context
is only active on the local CPU. Apply this same optimization to TLB
flushes of kernel memory when only one CPU is online. This check can be
constant-folded when SMP is disabled.
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v4:
- New patch for v4
arch/riscv/mm/tlbflush.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index 09b03bf71e6a..2f18fe6fc4f3 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -98,27 +98,23 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
{
const struct cpumask *cmask;
unsigned long asid = FLUSH_TLB_NO_ASID;
- bool broadcast;
+ unsigned int cpu;
if (mm) {
- unsigned int cpuid;
-
cmask = mm_cpumask(mm);
if (cpumask_empty(cmask))
return;
- cpuid = get_cpu();
- /* check if the tlbflush needs to be sent to other CPUs */
- broadcast = cpumask_any_but(cmask, cpuid) < nr_cpu_ids;
-
if (static_branch_unlikely(&use_asid_allocator))
asid = atomic_long_read(&mm->context.id) & asid_mask;
} else {
cmask = cpu_online_mask;
- broadcast = true;
}
- if (!broadcast) {
+ cpu = get_cpu();
+
+ /* Check if the TLB flush needs to be sent to other CPUs. */
+ if (cpumask_any_but(cmask, cpu) >= nr_cpu_ids) {
local_flush_tlb_range_asid(start, size, stride, asid);
} else if (riscv_use_sbi_for_rfence()) {
sbi_remote_sfence_vma_asid(cmask, start, size, asid);
@@ -132,8 +128,7 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
on_each_cpu_mask(cmask, __ipi_flush_tlb_range_asid, &ftd, 1);
}
- if (mm)
- put_cpu();
+ put_cpu();
}
void flush_tlb_mm(struct mm_struct *mm)
--
2.42.0
If no other CPU is online, a local cache or TLB flush is sufficient.
These checks can be constant-folded when SMP is disabled.
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v4:
- New patch for v4
arch/riscv/mm/cacheflush.c | 4 +++-
arch/riscv/mm/tlbflush.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
index 47c485bc7df0..f7933ae88a55 100644
--- a/arch/riscv/mm/cacheflush.c
+++ b/arch/riscv/mm/cacheflush.c
@@ -21,7 +21,9 @@ void flush_icache_all(void)
{
local_flush_icache_all();
- if (riscv_use_sbi_for_rfence())
+ if (num_online_cpus() < 2)
+ return;
+ else if (riscv_use_sbi_for_rfence())
sbi_remote_fence_i(NULL);
else
on_each_cpu(ipi_remote_fence_i, NULL, 1);
diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index 2f18fe6fc4f3..37b3c93e3c30 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -73,7 +73,9 @@ static void __ipi_flush_tlb_all(void *info)
void flush_tlb_all(void)
{
- if (riscv_use_sbi_for_rfence())
+ if (num_online_cpus() < 2)
+ local_flush_tlb_all();
+ else if (riscv_use_sbi_for_rfence())
sbi_remote_sfence_vma_asid(NULL, 0, FLUSH_TLB_MAX_SIZE, FLUSH_TLB_NO_ASID);
else
on_each_cpu(__ipi_flush_tlb_all, NULL, 1);
--
2.42.0
commit 3f1e782998cd ("riscv: add ASID-based tlbflushing methods") added
calls to the sfence.vma instruction with rs2 != x0. These single-ASID
instruction variants are also affected by SiFive errata CIP-1200.
Until now, the errata workaround was not needed for the single-ASID
sfence.vma variants, because they were only used when the ASID allocator
was enabled, and the affected SiFive platforms do not support multiple
ASIDs. However, we are going to start using those sfence.vma variants
regardless of ASID support, so now we need alternatives covering them.
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v2)
Changes in v2:
- Rebase on Alexandre's "riscv: tlb flush improvements" series v5
arch/riscv/include/asm/errata_list.h | 12 +++++++++++-
arch/riscv/include/asm/tlbflush.h | 19 ++++++++++++++++++-
arch/riscv/mm/tlbflush.c | 23 -----------------------
3 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 83ed25e43553..6781460ae564 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -44,11 +44,21 @@ ALTERNATIVE(__stringify(RISCV_PTR do_page_fault), \
CONFIG_ERRATA_SIFIVE_CIP_453)
#else /* !__ASSEMBLY__ */
-#define ALT_FLUSH_TLB_PAGE(x) \
+#define ALT_SFENCE_VMA_ASID(asid) \
+asm(ALTERNATIVE("sfence.vma x0, %0", "sfence.vma", SIFIVE_VENDOR_ID, \
+ ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200) \
+ : : "r" (asid) : "memory")
+
+#define ALT_SFENCE_VMA_ADDR(addr) \
asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID, \
ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200) \
: : "r" (addr) : "memory")
+#define ALT_SFENCE_VMA_ADDR_ASID(addr, asid) \
+asm(ALTERNATIVE("sfence.vma %0, %1", "sfence.vma", SIFIVE_VENDOR_ID, \
+ ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200) \
+ : : "r" (addr), "r" (asid) : "memory")
+
/*
* _val is marked as "will be overwritten", so need to set it to 0
* in the default case.
diff --git a/arch/riscv/include/asm/tlbflush.h b/arch/riscv/include/asm/tlbflush.h
index 7712ffe2f6c4..002c4c2620f3 100644
--- a/arch/riscv/include/asm/tlbflush.h
+++ b/arch/riscv/include/asm/tlbflush.h
@@ -22,10 +22,27 @@ static inline void local_flush_tlb_all(void)
__asm__ __volatile__ ("sfence.vma" : : : "memory");
}
+static inline void local_flush_tlb_all_asid(unsigned long asid)
+{
+ if (asid != FLUSH_TLB_NO_ASID)
+ ALT_SFENCE_VMA_ASID(asid);
+ else
+ local_flush_tlb_all();
+}
+
/* Flush one page from local TLB */
static inline void local_flush_tlb_page(unsigned long addr)
{
- ALT_FLUSH_TLB_PAGE(__asm__ __volatile__ ("sfence.vma %0" : : "r" (addr) : "memory"));
+ ALT_SFENCE_VMA_ADDR(addr);
+}
+
+static inline void local_flush_tlb_page_asid(unsigned long addr,
+ unsigned long asid)
+{
+ if (asid != FLUSH_TLB_NO_ASID)
+ ALT_SFENCE_VMA_ADDR_ASID(addr, asid);
+ else
+ local_flush_tlb_page(addr);
}
void flush_tlb_all(void);
diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index 37b3c93e3c30..292d7cf3c4f6 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -7,29 +7,6 @@
#include <asm/sbi.h>
#include <asm/mmu_context.h>
-static inline void local_flush_tlb_all_asid(unsigned long asid)
-{
- if (asid != FLUSH_TLB_NO_ASID)
- __asm__ __volatile__ ("sfence.vma x0, %0"
- :
- : "r" (asid)
- : "memory");
- else
- local_flush_tlb_all();
-}
-
-static inline void local_flush_tlb_page_asid(unsigned long addr,
- unsigned long asid)
-{
- if (asid != FLUSH_TLB_NO_ASID)
- __asm__ __volatile__ ("sfence.vma %0, %1"
- :
- : "r" (addr), "r" (asid)
- : "memory");
- else
- local_flush_tlb_page(addr);
-}
-
/*
* Flush entire TLB if number of entries to be flushed is greater
* than the threshold below.
--
2.42.0
When using the ASID allocator, the MM context ID contains two values:
the ASID in the lower bits, and the allocator version number in the
remaining bits. Use macros to make this separation more obvious.
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
arch/riscv/include/asm/mmu.h | 3 +++
arch/riscv/mm/context.c | 12 ++++++------
arch/riscv/mm/tlbflush.c | 2 +-
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/riscv/include/asm/mmu.h b/arch/riscv/include/asm/mmu.h
index 355504b37f8e..a550fbf770be 100644
--- a/arch/riscv/include/asm/mmu.h
+++ b/arch/riscv/include/asm/mmu.h
@@ -26,6 +26,9 @@ typedef struct {
#endif
} mm_context_t;
+#define cntx2asid(cntx) ((cntx) & asid_mask)
+#define cntx2version(cntx) ((cntx) & ~asid_mask)
+
void __init create_pgd_mapping(pgd_t *pgdp, uintptr_t va, phys_addr_t pa,
phys_addr_t sz, pgprot_t prot);
#endif /* __ASSEMBLY__ */
diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
index 217fd4de6134..43d005f63253 100644
--- a/arch/riscv/mm/context.c
+++ b/arch/riscv/mm/context.c
@@ -81,7 +81,7 @@ static void __flush_context(void)
if (cntx == 0)
cntx = per_cpu(reserved_context, i);
- __set_bit(cntx & asid_mask, context_asid_map);
+ __set_bit(cntx2asid(cntx), context_asid_map);
per_cpu(reserved_context, i) = cntx;
}
@@ -102,7 +102,7 @@ static unsigned long __new_context(struct mm_struct *mm)
lockdep_assert_held(&context_lock);
if (cntx != 0) {
- unsigned long newcntx = ver | (cntx & asid_mask);
+ unsigned long newcntx = ver | cntx2asid(cntx);
/*
* If our current CONTEXT was active during a rollover, we
@@ -115,7 +115,7 @@ static unsigned long __new_context(struct mm_struct *mm)
* We had a valid CONTEXT in a previous life, so try to
* re-use it if possible.
*/
- if (!__test_and_set_bit(cntx & asid_mask, context_asid_map))
+ if (!__test_and_set_bit(cntx2asid(cntx), context_asid_map))
return newcntx;
}
@@ -168,7 +168,7 @@ static void set_mm_asid(struct mm_struct *mm, unsigned int cpu)
*/
old_active_cntx = atomic_long_read(&per_cpu(active_context, cpu));
if (old_active_cntx &&
- ((cntx & ~asid_mask) == atomic_long_read(¤t_version)) &&
+ (cntx2version(cntx) == atomic_long_read(¤t_version)) &&
atomic_long_cmpxchg_relaxed(&per_cpu(active_context, cpu),
old_active_cntx, cntx))
goto switch_mm_fast;
@@ -177,7 +177,7 @@ static void set_mm_asid(struct mm_struct *mm, unsigned int cpu)
/* Check that our ASID belongs to the current_version. */
cntx = atomic_long_read(&mm->context.id);
- if ((cntx & ~asid_mask) != atomic_long_read(¤t_version)) {
+ if (cntx2version(cntx) != atomic_long_read(¤t_version)) {
cntx = __new_context(mm);
atomic_long_set(&mm->context.id, cntx);
}
@@ -191,7 +191,7 @@ static void set_mm_asid(struct mm_struct *mm, unsigned int cpu)
switch_mm_fast:
csr_write(CSR_SATP, virt_to_pfn(mm->pgd) |
- ((cntx & asid_mask) << SATP_ASID_SHIFT) |
+ (cntx2asid(cntx) << SATP_ASID_SHIFT) |
satp_mode);
if (need_flush_tlb)
diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index 76b24d4ed4ab..5ec621545c69 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -85,7 +85,7 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
return;
if (static_branch_unlikely(&use_asid_allocator))
- asid = atomic_long_read(&mm->context.id) & asid_mask;
+ asid = cntx2asid(atomic_long_read(&mm->context.id));
} else {
cmask = cpu_online_mask;
}
--
2.42.0
Currently, the size of the ASID field in the MM context ID dynamically
depends on the number of hardware-supported ASID bits. This requires
reading a global variable to extract either field from the context ID.
Instead, allocate the maximum possible number of bits to the ASID field,
so the layout of the context ID is known at compile-time.
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
arch/riscv/include/asm/mmu.h | 4 ++--
arch/riscv/include/asm/tlbflush.h | 2 --
arch/riscv/mm/context.c | 6 ++----
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/riscv/include/asm/mmu.h b/arch/riscv/include/asm/mmu.h
index a550fbf770be..dc0273f7905f 100644
--- a/arch/riscv/include/asm/mmu.h
+++ b/arch/riscv/include/asm/mmu.h
@@ -26,8 +26,8 @@ typedef struct {
#endif
} mm_context_t;
-#define cntx2asid(cntx) ((cntx) & asid_mask)
-#define cntx2version(cntx) ((cntx) & ~asid_mask)
+#define cntx2asid(cntx) ((cntx) & SATP_ASID_MASK)
+#define cntx2version(cntx) ((cntx) & ~SATP_ASID_MASK)
void __init create_pgd_mapping(pgd_t *pgdp, uintptr_t va, phys_addr_t pa,
phys_addr_t sz, pgprot_t prot);
diff --git a/arch/riscv/include/asm/tlbflush.h b/arch/riscv/include/asm/tlbflush.h
index d9913590f82e..5bfd37cfd8c3 100644
--- a/arch/riscv/include/asm/tlbflush.h
+++ b/arch/riscv/include/asm/tlbflush.h
@@ -15,8 +15,6 @@
#define FLUSH_TLB_NO_ASID ((unsigned long)-1)
#ifdef CONFIG_MMU
-extern unsigned long asid_mask;
-
static inline void local_flush_tlb_all(void)
{
__asm__ __volatile__ ("sfence.vma" : : : "memory");
diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
index 43d005f63253..b5170ac1b742 100644
--- a/arch/riscv/mm/context.c
+++ b/arch/riscv/mm/context.c
@@ -22,7 +22,6 @@ DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
static unsigned long asid_bits;
static unsigned long num_asids;
-unsigned long asid_mask;
static atomic_long_t current_version;
@@ -128,7 +127,7 @@ static unsigned long __new_context(struct mm_struct *mm)
goto set_asid;
/* We're out of ASIDs, so increment current_version */
- ver = atomic_long_add_return_relaxed(num_asids, ¤t_version);
+ ver = atomic_long_add_return_relaxed(BIT(SATP_ASID_BITS), ¤t_version);
/* Flush everything */
__flush_context();
@@ -247,7 +246,6 @@ static int __init asids_init(void)
/* Pre-compute ASID details */
if (asid_bits) {
num_asids = 1 << asid_bits;
- asid_mask = num_asids - 1;
}
/*
@@ -255,7 +253,7 @@ static int __init asids_init(void)
* at-least twice more than CPUs
*/
if (num_asids > (2 * num_possible_cpus())) {
- atomic_long_set(¤t_version, num_asids);
+ atomic_long_set(¤t_version, BIT(SATP_ASID_BITS));
context_asid_map = bitmap_zalloc(num_asids, GFP_KERNEL);
if (!context_asid_map)
--
2.42.0
This variable is only used inside asids_init().
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
arch/riscv/mm/context.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
index b5170ac1b742..43a8bc2d5af4 100644
--- a/arch/riscv/mm/context.c
+++ b/arch/riscv/mm/context.c
@@ -20,7 +20,6 @@
DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
-static unsigned long asid_bits;
static unsigned long num_asids;
static atomic_long_t current_version;
@@ -226,7 +225,7 @@ static inline void set_mm(struct mm_struct *prev,
static int __init asids_init(void)
{
- unsigned long old;
+ unsigned long asid_bits, old;
/* Figure-out number of ASID bits in HW */
old = csr_read(CSR_SATP);
--
2.42.0
An IPI backend is always required in an SMP configuration, but an SBI
implementation is not. For example, SBI will be unavailable when the
kernel runs in M mode.
Generally, IPIs are assumed to be faster than SBI calls due to the SBI
context switch overhead. However, when SBI is used as the IPI backend,
then the context switch cost must be paid anyway, and performing the
cache/TLB flush directly in the SBI implementation is more efficient
than inserting an interrupt to the kernel. This is the only scenario
where riscv_ipi_set_virq_range()'s use_for_rfence parameter is false.
Thus, it makes sense for remote fences to use IPIs by default, and make
the SBI remote fence extension the special case.
sbi_ipi_init() already checks riscv_ipi_have_virq_range(), so it only
calls riscv_ipi_set_virq_range() when no other IPI device is available.
So we can move the static key and drop the use_for_rfence parameter.
Furthermore, the static branch only makes sense when CONFIG_RISCV_SBI is
enabled. Optherwise, IPIs must be used. Add a fallback definition of
riscv_use_sbi_for_rfence() which handles this case and removes the need
to check CONFIG_RISCV_SBI elsewhere, such as in cacheflush.c.
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v4:
- New patch for v4
arch/riscv/include/asm/sbi.h | 4 ++++
arch/riscv/include/asm/smp.h | 15 ++-------------
arch/riscv/kernel/sbi-ipi.c | 11 ++++++++++-
arch/riscv/kernel/smp.c | 11 +----------
arch/riscv/mm/cacheflush.c | 5 ++---
arch/riscv/mm/tlbflush.c | 31 ++++++++++++++-----------------
drivers/clocksource/timer-clint.c | 2 +-
7 files changed, 34 insertions(+), 45 deletions(-)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 0892f4421bc4..aeee0127df76 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -339,8 +339,12 @@ unsigned long riscv_cached_marchid(unsigned int cpu_id);
unsigned long riscv_cached_mimpid(unsigned int cpu_id);
#if IS_ENABLED(CONFIG_SMP) && IS_ENABLED(CONFIG_RISCV_SBI)
+DECLARE_STATIC_KEY_FALSE(riscv_sbi_for_rfence);
+#define riscv_use_sbi_for_rfence() \
+ static_branch_unlikely(&riscv_sbi_for_rfence)
void sbi_ipi_init(void);
#else
+static inline bool riscv_use_sbi_for_rfence(void) { return false; }
static inline void sbi_ipi_init(void) { }
#endif
diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 0d555847cde6..7ac80e9f2288 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -49,12 +49,7 @@ void riscv_ipi_disable(void);
bool riscv_ipi_have_virq_range(void);
/* Set the IPI interrupt numbers for arch (called by irqchip drivers) */
-void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence);
-
-/* Check if we can use IPIs for remote FENCEs */
-DECLARE_STATIC_KEY_FALSE(riscv_ipi_for_rfence);
-#define riscv_use_ipi_for_rfence() \
- static_branch_unlikely(&riscv_ipi_for_rfence)
+void riscv_ipi_set_virq_range(int virq, int nr);
/* Check other CPUs stop or not */
bool smp_crash_stop_failed(void);
@@ -104,16 +99,10 @@ static inline bool riscv_ipi_have_virq_range(void)
return false;
}
-static inline void riscv_ipi_set_virq_range(int virq, int nr,
- bool use_for_rfence)
+static inline void riscv_ipi_set_virq_range(int virq, int nr)
{
}
-static inline bool riscv_use_ipi_for_rfence(void)
-{
- return false;
-}
-
#endif /* CONFIG_SMP */
#if defined(CONFIG_HOTPLUG_CPU) && (CONFIG_SMP)
diff --git a/arch/riscv/kernel/sbi-ipi.c b/arch/riscv/kernel/sbi-ipi.c
index a4559695ce62..1026e22955cc 100644
--- a/arch/riscv/kernel/sbi-ipi.c
+++ b/arch/riscv/kernel/sbi-ipi.c
@@ -13,6 +13,9 @@
#include <linux/irqdomain.h>
#include <asm/sbi.h>
+DEFINE_STATIC_KEY_FALSE(riscv_sbi_for_rfence);
+EXPORT_SYMBOL_GPL(riscv_sbi_for_rfence);
+
static int sbi_ipi_virq;
static void sbi_ipi_handle(struct irq_desc *desc)
@@ -72,6 +75,12 @@ void __init sbi_ipi_init(void)
"irqchip/sbi-ipi:starting",
sbi_ipi_starting_cpu, NULL);
- riscv_ipi_set_virq_range(virq, BITS_PER_BYTE, false);
+ riscv_ipi_set_virq_range(virq, BITS_PER_BYTE);
pr_info("providing IPIs using SBI IPI extension\n");
+
+ /*
+ * Use the SBI remote fence extension to avoid
+ * the extra context switch needed to handle IPIs.
+ */
+ static_branch_enable(&riscv_sbi_for_rfence);
}
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 40420afbb1a0..1d06df04eb71 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -171,10 +171,7 @@ bool riscv_ipi_have_virq_range(void)
return (ipi_virq_base) ? true : false;
}
-DEFINE_STATIC_KEY_FALSE(riscv_ipi_for_rfence);
-EXPORT_SYMBOL_GPL(riscv_ipi_for_rfence);
-
-void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence)
+void riscv_ipi_set_virq_range(int virq, int nr)
{
int i, err;
@@ -197,12 +194,6 @@ void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence)
/* Enabled IPIs for boot CPU immediately */
riscv_ipi_enable();
-
- /* Update RFENCE static key */
- if (use_for_rfence)
- static_branch_enable(&riscv_ipi_for_rfence);
- else
- static_branch_disable(&riscv_ipi_for_rfence);
}
static const char * const ipi_names[] = {
diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
index 55a34f2020a8..47c485bc7df0 100644
--- a/arch/riscv/mm/cacheflush.c
+++ b/arch/riscv/mm/cacheflush.c
@@ -21,7 +21,7 @@ void flush_icache_all(void)
{
local_flush_icache_all();
- if (IS_ENABLED(CONFIG_RISCV_SBI) && !riscv_use_ipi_for_rfence())
+ if (riscv_use_sbi_for_rfence())
sbi_remote_fence_i(NULL);
else
on_each_cpu(ipi_remote_fence_i, NULL, 1);
@@ -69,8 +69,7 @@ void flush_icache_mm(struct mm_struct *mm, bool local)
* with flush_icache_deferred().
*/
smp_mb();
- } else if (IS_ENABLED(CONFIG_RISCV_SBI) &&
- !riscv_use_ipi_for_rfence()) {
+ } else if (riscv_use_sbi_for_rfence()) {
sbi_remote_fence_i(&others);
} else {
on_each_cpu_mask(&others, ipi_remote_fence_i, NULL, 1);
diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index e6659d7368b3..09b03bf71e6a 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -73,10 +73,10 @@ static void __ipi_flush_tlb_all(void *info)
void flush_tlb_all(void)
{
- if (riscv_use_ipi_for_rfence())
- on_each_cpu(__ipi_flush_tlb_all, NULL, 1);
- else
+ if (riscv_use_sbi_for_rfence())
sbi_remote_sfence_vma_asid(NULL, 0, FLUSH_TLB_MAX_SIZE, FLUSH_TLB_NO_ASID);
+ else
+ on_each_cpu(__ipi_flush_tlb_all, NULL, 1);
}
struct flush_tlb_range_data {
@@ -96,7 +96,6 @@ static void __ipi_flush_tlb_range_asid(void *info)
static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
unsigned long size, unsigned long stride)
{
- struct flush_tlb_range_data ftd;
const struct cpumask *cmask;
unsigned long asid = FLUSH_TLB_NO_ASID;
bool broadcast;
@@ -119,20 +118,18 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
broadcast = true;
}
- if (broadcast) {
- if (riscv_use_ipi_for_rfence()) {
- ftd.asid = asid;
- ftd.start = start;
- ftd.size = size;
- ftd.stride = stride;
- on_each_cpu_mask(cmask,
- __ipi_flush_tlb_range_asid,
- &ftd, 1);
- } else
- sbi_remote_sfence_vma_asid(cmask,
- start, size, asid);
- } else {
+ if (!broadcast) {
local_flush_tlb_range_asid(start, size, stride, asid);
+ } else if (riscv_use_sbi_for_rfence()) {
+ sbi_remote_sfence_vma_asid(cmask, start, size, asid);
+ } else {
+ struct flush_tlb_range_data ftd;
+
+ ftd.asid = asid;
+ ftd.start = start;
+ ftd.size = size;
+ ftd.stride = stride;
+ on_each_cpu_mask(cmask, __ipi_flush_tlb_range_asid, &ftd, 1);
}
if (mm)
diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
index 9a55e733ae99..7ccc16dd6a76 100644
--- a/drivers/clocksource/timer-clint.c
+++ b/drivers/clocksource/timer-clint.c
@@ -251,7 +251,7 @@ static int __init clint_timer_init_dt(struct device_node *np)
}
irq_set_chained_handler(clint_ipi_irq, clint_ipi_interrupt);
- riscv_ipi_set_virq_range(rc, BITS_PER_BYTE, true);
+ riscv_ipi_set_virq_range(rc, BITS_PER_BYTE);
clint_clear_ipi();
#endif
--
2.42.0
In SMP configurations, all TLB flushing narrower than flush_tlb_all()
goes through __flush_tlb_range(). Do the same in UP configurations.
This allows UP configurations to take advantage of recent improvements
to the code in tlbflush.c, such as support for huge pages and flushing
multiple-page ranges.
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v4:
- Merge the two copies of __flush_tlb_range() and rely on the compiler
to optimize out the broadcast path (both clang and gcc do this)
- Merge the two copies of flush_tlb_all() and rely on constant folding
Changes in v2:
- Move the SMP/UP merge earlier in the series to avoid build issues
- Make a copy of __flush_tlb_range() instead of adding ifdefs inside
- local_flush_tlb_all() is the only function used on !MMU (smpboot.c)
arch/riscv/include/asm/tlbflush.h | 29 +++--------------------------
arch/riscv/mm/Makefile | 5 +----
2 files changed, 4 insertions(+), 30 deletions(-)
diff --git a/arch/riscv/include/asm/tlbflush.h b/arch/riscv/include/asm/tlbflush.h
index 8f3418c5f172..7712ffe2f6c4 100644
--- a/arch/riscv/include/asm/tlbflush.h
+++ b/arch/riscv/include/asm/tlbflush.h
@@ -27,12 +27,7 @@ static inline void local_flush_tlb_page(unsigned long addr)
{
ALT_FLUSH_TLB_PAGE(__asm__ __volatile__ ("sfence.vma %0" : : "r" (addr) : "memory"));
}
-#else /* CONFIG_MMU */
-#define local_flush_tlb_all() do { } while (0)
-#define local_flush_tlb_page(addr) do { } while (0)
-#endif /* CONFIG_MMU */
-#if defined(CONFIG_SMP) && defined(CONFIG_MMU)
void flush_tlb_all(void);
void flush_tlb_mm(struct mm_struct *mm);
void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
@@ -46,26 +41,8 @@ void flush_tlb_kernel_range(unsigned long start, unsigned long end);
void flush_pmd_tlb_range(struct vm_area_struct *vma, unsigned long start,
unsigned long end);
#endif
-#else /* CONFIG_SMP && CONFIG_MMU */
-
-#define flush_tlb_all() local_flush_tlb_all()
-#define flush_tlb_page(vma, addr) local_flush_tlb_page(addr)
-
-static inline void flush_tlb_range(struct vm_area_struct *vma,
- unsigned long start, unsigned long end)
-{
- local_flush_tlb_all();
-}
-
-/* Flush a range of kernel pages */
-static inline void flush_tlb_kernel_range(unsigned long start,
- unsigned long end)
-{
- local_flush_tlb_all();
-}
-
-#define flush_tlb_mm(mm) flush_tlb_all()
-#define flush_tlb_mm_range(mm, start, end, page_size) flush_tlb_all()
-#endif /* !CONFIG_SMP || !CONFIG_MMU */
+#else /* CONFIG_MMU */
+#define local_flush_tlb_all() do { } while (0)
+#endif /* CONFIG_MMU */
#endif /* _ASM_RISCV_TLBFLUSH_H */
diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
index 3a4dfc8babcf..96e65c571ce8 100644
--- a/arch/riscv/mm/Makefile
+++ b/arch/riscv/mm/Makefile
@@ -13,15 +13,12 @@ endif
KCOV_INSTRUMENT_init.o := n
obj-y += init.o
-obj-$(CONFIG_MMU) += extable.o fault.o pageattr.o
+obj-$(CONFIG_MMU) += extable.o fault.o pageattr.o tlbflush.o
obj-y += cacheflush.o
obj-y += context.o
obj-y += pgtable.o
obj-y += pmem.o
-ifeq ($(CONFIG_MMU),y)
-obj-$(CONFIG_SMP) += tlbflush.o
-endif
obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
obj-$(CONFIG_PTDUMP_CORE) += ptdump.o
obj-$(CONFIG_KASAN) += kasan_init.o
--
2.42.0
If the CPU does not support multiple ASIDs, all MM contexts use ASID 0.
In this case, it is still beneficial to flush the TLB by ASID, as the
single-ASID variant of the sfence.vma instruction preserves TLB entries
for global (kernel) pages.
This optimization is recommended by the RISC-V privileged specification:
If the implementation does not provide ASIDs, or software chooses
to always use ASID 0, then after every satp write, software should
execute SFENCE.VMA with rs1=x0. In the common case that no global
translations have been modified, rs2 should be set to a register
other than x0 but which contains the value zero, so that global
translations are not flushed.
It is not possible to apply this optimization when using the ASID
allocator, because that code must flush the TLB for all ASIDs at once
when incrementing the version number.
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
arch/riscv/mm/context.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
index 43a8bc2d5af4..3ca9b653df7d 100644
--- a/arch/riscv/mm/context.c
+++ b/arch/riscv/mm/context.c
@@ -200,7 +200,7 @@ static void set_mm_noasid(struct mm_struct *mm)
{
/* Switch the page table and blindly nuke entire local TLB */
csr_write(CSR_SATP, virt_to_pfn(mm->pgd) | satp_mode);
- local_flush_tlb_all();
+ local_flush_tlb_all_asid(0);
}
static inline void set_mm(struct mm_struct *prev,
--
2.42.0
Even if multiple ASIDs are not supported, using the single-ASID variant
of the sfence.vma instruction preserves TLB entries for global (kernel)
pages. So it is always more efficient to use the single-ASID code path.
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v4:
- There is now only one copy of __flush_tlb_range()
Changes in v2:
- Update both copies of __flush_tlb_range()
arch/riscv/include/asm/mmu_context.h | 2 --
arch/riscv/mm/context.c | 3 +--
arch/riscv/mm/tlbflush.c | 3 +--
3 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/include/asm/mmu_context.h b/arch/riscv/include/asm/mmu_context.h
index 7030837adc1a..b0659413a080 100644
--- a/arch/riscv/include/asm/mmu_context.h
+++ b/arch/riscv/include/asm/mmu_context.h
@@ -33,8 +33,6 @@ static inline int init_new_context(struct task_struct *tsk,
return 0;
}
-DECLARE_STATIC_KEY_FALSE(use_asid_allocator);
-
#include <asm-generic/mmu_context.h>
#endif /* _ASM_RISCV_MMU_CONTEXT_H */
diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
index 3ca9b653df7d..20057085ab8a 100644
--- a/arch/riscv/mm/context.c
+++ b/arch/riscv/mm/context.c
@@ -18,8 +18,7 @@
#ifdef CONFIG_MMU
-DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
-
+static DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
static unsigned long num_asids;
static atomic_long_t current_version;
diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index 5ec621545c69..39d80f56d292 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -84,8 +84,7 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
if (cpumask_empty(cmask))
return;
- if (static_branch_unlikely(&use_asid_allocator))
- asid = cntx2asid(atomic_long_read(&mm->context.id));
+ asid = cntx2asid(atomic_long_read(&mm->context.id));
} else {
cmask = cpu_online_mask;
}
--
2.42.0
Since implementations affected by SiFive errata CIP-1200 always use the
global variant of the sfence.vma instruction, they only need to execute
the instruction once. The range-based loop only hurts performance.
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v4:
- Only set tlb_flush_all_threshold when CONFIG_MMU=y.
Changes in v3:
- New patch for v3
arch/riscv/errata/sifive/errata.c | 5 +++++
arch/riscv/include/asm/tlbflush.h | 2 ++
arch/riscv/mm/tlbflush.c | 2 +-
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c
index 3d9a32d791f7..716cfedad3a2 100644
--- a/arch/riscv/errata/sifive/errata.c
+++ b/arch/riscv/errata/sifive/errata.c
@@ -42,6 +42,11 @@ static bool errata_cip_1200_check_func(unsigned long arch_id, unsigned long imp
return false;
if ((impid & 0xffffff) > 0x200630 || impid == 0x1200626)
return false;
+
+#ifdef CONFIG_MMU
+ tlb_flush_all_threshold = 0;
+#endif
+
return true;
}
diff --git a/arch/riscv/include/asm/tlbflush.h b/arch/riscv/include/asm/tlbflush.h
index 002c4c2620f3..d9913590f82e 100644
--- a/arch/riscv/include/asm/tlbflush.h
+++ b/arch/riscv/include/asm/tlbflush.h
@@ -58,6 +58,8 @@ void flush_tlb_kernel_range(unsigned long start, unsigned long end);
void flush_pmd_tlb_range(struct vm_area_struct *vma, unsigned long start,
unsigned long end);
#endif
+
+extern unsigned long tlb_flush_all_threshold;
#else /* CONFIG_MMU */
#define local_flush_tlb_all() do { } while (0)
#endif /* CONFIG_MMU */
diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index 292d7cf3c4f6..76b24d4ed4ab 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -11,7 +11,7 @@
* Flush entire TLB if number of entries to be flushed is greater
* than the threshold below.
*/
-static unsigned long tlb_flush_all_threshold __read_mostly = 64;
+unsigned long tlb_flush_all_threshold __read_mostly = 64;
static void local_flush_tlb_range_threshold_asid(unsigned long start,
unsigned long size,
--
2.42.0
On Tue, Jan 02, 2024 at 02:00:41PM -0800, Samuel Holland wrote:
> If no other CPU is online, a local cache or TLB flush is sufficient.
> These checks can be constant-folded when SMP is disabled.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v4:
> - New patch for v4
>
> arch/riscv/mm/cacheflush.c | 4 +++-
> arch/riscv/mm/tlbflush.c | 4 +++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index 47c485bc7df0..f7933ae88a55 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -21,7 +21,9 @@ void flush_icache_all(void)
> {
> local_flush_icache_all();
>
> - if (riscv_use_sbi_for_rfence())
> + if (num_online_cpus() < 2)
with patch5, I think it's better to short cut for !SMP, I.E
if (!IS_ENABLED(CONFIG_SMP) || num_online_cpus()) < 2)
so that the UP case can avoid a atomic read and check
> + return;
> + else if (riscv_use_sbi_for_rfence())
> sbi_remote_fence_i(NULL);
> else
> on_each_cpu(ipi_remote_fence_i, NULL, 1);
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index 2f18fe6fc4f3..37b3c93e3c30 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -73,7 +73,9 @@ static void __ipi_flush_tlb_all(void *info)
>
> void flush_tlb_all(void)
> {
> - if (riscv_use_sbi_for_rfence())
> + if (num_online_cpus() < 2)
ditto
> + local_flush_tlb_all();
> + else if (riscv_use_sbi_for_rfence())
> sbi_remote_sfence_vma_asid(NULL, 0, FLUSH_TLB_MAX_SIZE, FLUSH_TLB_NO_ASID);
> else
> on_each_cpu(__ipi_flush_tlb_all, NULL, 1);
> --
> 2.42.0
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Jan 02, 2024 at 02:00:47PM -0800, Samuel Holland wrote:
> This variable is only used inside asids_init().
This is due to patch9, so can be folded into patch9.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> (no changes since v1)
>
> arch/riscv/mm/context.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
> index b5170ac1b742..43a8bc2d5af4 100644
> --- a/arch/riscv/mm/context.c
> +++ b/arch/riscv/mm/context.c
> @@ -20,7 +20,6 @@
>
> DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
>
> -static unsigned long asid_bits;
> static unsigned long num_asids;
>
> static atomic_long_t current_version;
> @@ -226,7 +225,7 @@ static inline void set_mm(struct mm_struct *prev,
>
> static int __init asids_init(void)
> {
> - unsigned long old;
> + unsigned long asid_bits, old;
>
> /* Figure-out number of ASID bits in HW */
> old = csr_read(CSR_SATP);
> --
> 2.42.0
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Jan 02, 2024 at 02:00:49PM -0800, Samuel Holland wrote:
> Even if multiple ASIDs are not supported, using the single-ASID variant
> of the sfence.vma instruction preserves TLB entries for global (kernel)
> pages. So it is always more efficient to use the single-ASID code path.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v4:
> - There is now only one copy of __flush_tlb_range()
>
> Changes in v2:
> - Update both copies of __flush_tlb_range()
>
> arch/riscv/include/asm/mmu_context.h | 2 --
> arch/riscv/mm/context.c | 3 +--
> arch/riscv/mm/tlbflush.c | 3 +--
> 3 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/riscv/include/asm/mmu_context.h b/arch/riscv/include/asm/mmu_context.h
> index 7030837adc1a..b0659413a080 100644
> --- a/arch/riscv/include/asm/mmu_context.h
> +++ b/arch/riscv/include/asm/mmu_context.h
> @@ -33,8 +33,6 @@ static inline int init_new_context(struct task_struct *tsk,
> return 0;
> }
>
> -DECLARE_STATIC_KEY_FALSE(use_asid_allocator);
> -
> #include <asm-generic/mmu_context.h>
>
> #endif /* _ASM_RISCV_MMU_CONTEXT_H */
> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
> index 3ca9b653df7d..20057085ab8a 100644
> --- a/arch/riscv/mm/context.c
> +++ b/arch/riscv/mm/context.c
> @@ -18,8 +18,7 @@
>
> #ifdef CONFIG_MMU
>
> -DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
> -
> +static DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
One of my optimization "riscv: tlb: avoid tlb flushing if fullmm == 1"
will make use of use_asid_allocator, so could we remove this modification?
> static unsigned long num_asids;
>
> static atomic_long_t current_version;
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index 5ec621545c69..39d80f56d292 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -84,8 +84,7 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
> if (cpumask_empty(cmask))
> return;
>
> - if (static_branch_unlikely(&use_asid_allocator))
> - asid = cntx2asid(atomic_long_read(&mm->context.id));
> + asid = cntx2asid(atomic_long_read(&mm->context.id));
> } else {
> cmask = cpu_online_mask;
> }
> --
> 2.42.0
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, Jan 03, 2024 at 10:58:01PM +0800, Jisheng Zhang wrote:
> On Tue, Jan 02, 2024 at 02:00:41PM -0800, Samuel Holland wrote:
> > If no other CPU is online, a local cache or TLB flush is sufficient.
> > These checks can be constant-folded when SMP is disabled.
> >
> > Signed-off-by: Samuel Holland <[email protected]>
> > ---
> >
> > Changes in v4:
> > - New patch for v4
> >
> > arch/riscv/mm/cacheflush.c | 4 +++-
> > arch/riscv/mm/tlbflush.c | 4 +++-
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> > index 47c485bc7df0..f7933ae88a55 100644
> > --- a/arch/riscv/mm/cacheflush.c
> > +++ b/arch/riscv/mm/cacheflush.c
> > @@ -21,7 +21,9 @@ void flush_icache_all(void)
> > {
> > local_flush_icache_all();
> >
> > - if (riscv_use_sbi_for_rfence())
> > + if (num_online_cpus() < 2)
>
> with patch5, I think it's better to short cut for !SMP, I.E
> if (!IS_ENABLED(CONFIG_SMP) || num_online_cpus()) < 2)
aha, plz ignore this comment, I see the num_online_cpus() is defined as 1U for
UP.
>
> so that the UP case can avoid a atomic read and check
>
> > + return;
> > + else if (riscv_use_sbi_for_rfence())
> > sbi_remote_fence_i(NULL);
> > else
> > on_each_cpu(ipi_remote_fence_i, NULL, 1);
> > diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> > index 2f18fe6fc4f3..37b3c93e3c30 100644
> > --- a/arch/riscv/mm/tlbflush.c
> > +++ b/arch/riscv/mm/tlbflush.c
> > @@ -73,7 +73,9 @@ static void __ipi_flush_tlb_all(void *info)
> >
> > void flush_tlb_all(void)
> > {
> > - if (riscv_use_sbi_for_rfence())
> > + if (num_online_cpus() < 2)
>
> ditto
>
> > + local_flush_tlb_all();
> > + else if (riscv_use_sbi_for_rfence())
> > sbi_remote_sfence_vma_asid(NULL, 0, FLUSH_TLB_MAX_SIZE, FLUSH_TLB_NO_ASID);
> > else
> > on_each_cpu(__ipi_flush_tlb_all, NULL, 1);
> > --
> > 2.42.0
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Samuel,
On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
<[email protected]> wrote:
>
> Instruction cache flush IPIs are sent only to CPUs in cpu_online_mask,
> so they will not target a CPU until it calls set_cpu_online() earlier in
> smp_callin(). As a result, if instruction memory is modified between the
> CPU coming out of reset and that point, then its instruction cache may
> contain stale data. Therefore, the instruction cache must be flushed
> after the set_cpu_online() synchronization point.
>
> Fixes: 08f051eda33b ("RISC-V: Flush I$ when making a dirty page executable")
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v4:
> - New patch for v4
>
> arch/riscv/kernel/smpboot.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index d162bf339beb..48af5bd3ec30 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -26,7 +26,7 @@
> #include <linux/sched/task_stack.h>
> #include <linux/sched/mm.h>
>
> -#include <asm/cpufeature.h>
> +#include <asm/cacheflush.h>
> #include <asm/cpu_ops.h>
> #include <asm/cpufeature.h>
> #include <asm/irq.h>
> @@ -257,9 +257,10 @@ asmlinkage __visible void smp_callin(void)
> riscv_user_isa_enable();
>
> /*
> - * Remote TLB flushes are ignored while the CPU is offline, so emit
> - * a local TLB flush right now just in case.
> + * Remote cache and TLB flushes are ignored while the CPU is offline,
> + * so flush them both right now just in case.
> */
> + local_flush_icache_all();
> local_flush_tlb_all();
> complete(&cpu_running);
> /*
> --
> 2.42.0
>
This looks good to me, so you can add:
Reviewed-by: Alexandre Ghiti <[email protected]>
Thanks,
Alex
On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
<[email protected]> wrote:
>
> An IPI backend is always required in an SMP configuration, but an SBI
> implementation is not. For example, SBI will be unavailable when the
> kernel runs in M mode.
>
> Generally, IPIs are assumed to be faster than SBI calls due to the SBI
> context switch overhead. However, when SBI is used as the IPI backend,
> then the context switch cost must be paid anyway, and performing the
> cache/TLB flush directly in the SBI implementation is more efficient
> than inserting an interrupt to the kernel. This is the only scenario
> where riscv_ipi_set_virq_range()'s use_for_rfence parameter is false.
>
> Thus, it makes sense for remote fences to use IPIs by default, and make
> the SBI remote fence extension the special case.
>
> sbi_ipi_init() already checks riscv_ipi_have_virq_range(), so it only
> calls riscv_ipi_set_virq_range() when no other IPI device is available.
> So we can move the static key and drop the use_for_rfence parameter.
>
> Furthermore, the static branch only makes sense when CONFIG_RISCV_SBI is
> enabled. Optherwise, IPIs must be used. Add a fallback definition of
> riscv_use_sbi_for_rfence() which handles this case and removes the need
> to check CONFIG_RISCV_SBI elsewhere, such as in cacheflush.c.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v4:
> - New patch for v4
>
> arch/riscv/include/asm/sbi.h | 4 ++++
> arch/riscv/include/asm/smp.h | 15 ++-------------
> arch/riscv/kernel/sbi-ipi.c | 11 ++++++++++-
> arch/riscv/kernel/smp.c | 11 +----------
> arch/riscv/mm/cacheflush.c | 5 ++---
> arch/riscv/mm/tlbflush.c | 31 ++++++++++++++-----------------
> drivers/clocksource/timer-clint.c | 2 +-
> 7 files changed, 34 insertions(+), 45 deletions(-)
>
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index 0892f4421bc4..aeee0127df76 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -339,8 +339,12 @@ unsigned long riscv_cached_marchid(unsigned int cpu_id);
> unsigned long riscv_cached_mimpid(unsigned int cpu_id);
>
> #if IS_ENABLED(CONFIG_SMP) && IS_ENABLED(CONFIG_RISCV_SBI)
> +DECLARE_STATIC_KEY_FALSE(riscv_sbi_for_rfence);
> +#define riscv_use_sbi_for_rfence() \
> + static_branch_unlikely(&riscv_sbi_for_rfence)
> void sbi_ipi_init(void);
> #else
> +static inline bool riscv_use_sbi_for_rfence(void) { return false; }
> static inline void sbi_ipi_init(void) { }
> #endif
>
> diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
> index 0d555847cde6..7ac80e9f2288 100644
> --- a/arch/riscv/include/asm/smp.h
> +++ b/arch/riscv/include/asm/smp.h
> @@ -49,12 +49,7 @@ void riscv_ipi_disable(void);
> bool riscv_ipi_have_virq_range(void);
>
> /* Set the IPI interrupt numbers for arch (called by irqchip drivers) */
> -void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence);
> -
> -/* Check if we can use IPIs for remote FENCEs */
> -DECLARE_STATIC_KEY_FALSE(riscv_ipi_for_rfence);
> -#define riscv_use_ipi_for_rfence() \
> - static_branch_unlikely(&riscv_ipi_for_rfence)
> +void riscv_ipi_set_virq_range(int virq, int nr);
>
> /* Check other CPUs stop or not */
> bool smp_crash_stop_failed(void);
> @@ -104,16 +99,10 @@ static inline bool riscv_ipi_have_virq_range(void)
> return false;
> }
>
> -static inline void riscv_ipi_set_virq_range(int virq, int nr,
> - bool use_for_rfence)
> +static inline void riscv_ipi_set_virq_range(int virq, int nr)
> {
> }
>
> -static inline bool riscv_use_ipi_for_rfence(void)
> -{
> - return false;
> -}
> -
> #endif /* CONFIG_SMP */
>
> #if defined(CONFIG_HOTPLUG_CPU) && (CONFIG_SMP)
> diff --git a/arch/riscv/kernel/sbi-ipi.c b/arch/riscv/kernel/sbi-ipi.c
> index a4559695ce62..1026e22955cc 100644
> --- a/arch/riscv/kernel/sbi-ipi.c
> +++ b/arch/riscv/kernel/sbi-ipi.c
> @@ -13,6 +13,9 @@
> #include <linux/irqdomain.h>
> #include <asm/sbi.h>
>
> +DEFINE_STATIC_KEY_FALSE(riscv_sbi_for_rfence);
> +EXPORT_SYMBOL_GPL(riscv_sbi_for_rfence);
> +
> static int sbi_ipi_virq;
>
> static void sbi_ipi_handle(struct irq_desc *desc)
> @@ -72,6 +75,12 @@ void __init sbi_ipi_init(void)
> "irqchip/sbi-ipi:starting",
> sbi_ipi_starting_cpu, NULL);
>
> - riscv_ipi_set_virq_range(virq, BITS_PER_BYTE, false);
> + riscv_ipi_set_virq_range(virq, BITS_PER_BYTE);
> pr_info("providing IPIs using SBI IPI extension\n");
> +
> + /*
> + * Use the SBI remote fence extension to avoid
> + * the extra context switch needed to handle IPIs.
> + */
> + static_branch_enable(&riscv_sbi_for_rfence);
> }
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index 40420afbb1a0..1d06df04eb71 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -171,10 +171,7 @@ bool riscv_ipi_have_virq_range(void)
> return (ipi_virq_base) ? true : false;
> }
>
> -DEFINE_STATIC_KEY_FALSE(riscv_ipi_for_rfence);
> -EXPORT_SYMBOL_GPL(riscv_ipi_for_rfence);
> -
> -void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence)
> +void riscv_ipi_set_virq_range(int virq, int nr)
> {
> int i, err;
>
> @@ -197,12 +194,6 @@ void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence)
>
> /* Enabled IPIs for boot CPU immediately */
> riscv_ipi_enable();
> -
> - /* Update RFENCE static key */
> - if (use_for_rfence)
> - static_branch_enable(&riscv_ipi_for_rfence);
> - else
> - static_branch_disable(&riscv_ipi_for_rfence);
> }
>
> static const char * const ipi_names[] = {
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index 55a34f2020a8..47c485bc7df0 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -21,7 +21,7 @@ void flush_icache_all(void)
> {
> local_flush_icache_all();
>
> - if (IS_ENABLED(CONFIG_RISCV_SBI) && !riscv_use_ipi_for_rfence())
> + if (riscv_use_sbi_for_rfence())
> sbi_remote_fence_i(NULL);
> else
> on_each_cpu(ipi_remote_fence_i, NULL, 1);
> @@ -69,8 +69,7 @@ void flush_icache_mm(struct mm_struct *mm, bool local)
> * with flush_icache_deferred().
> */
> smp_mb();
> - } else if (IS_ENABLED(CONFIG_RISCV_SBI) &&
> - !riscv_use_ipi_for_rfence()) {
> + } else if (riscv_use_sbi_for_rfence()) {
> sbi_remote_fence_i(&others);
> } else {
> on_each_cpu_mask(&others, ipi_remote_fence_i, NULL, 1);
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index e6659d7368b3..09b03bf71e6a 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -73,10 +73,10 @@ static void __ipi_flush_tlb_all(void *info)
>
> void flush_tlb_all(void)
> {
> - if (riscv_use_ipi_for_rfence())
> - on_each_cpu(__ipi_flush_tlb_all, NULL, 1);
> - else
> + if (riscv_use_sbi_for_rfence())
> sbi_remote_sfence_vma_asid(NULL, 0, FLUSH_TLB_MAX_SIZE, FLUSH_TLB_NO_ASID);
> + else
> + on_each_cpu(__ipi_flush_tlb_all, NULL, 1);
> }
>
> struct flush_tlb_range_data {
> @@ -96,7 +96,6 @@ static void __ipi_flush_tlb_range_asid(void *info)
> static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
> unsigned long size, unsigned long stride)
> {
> - struct flush_tlb_range_data ftd;
> const struct cpumask *cmask;
> unsigned long asid = FLUSH_TLB_NO_ASID;
> bool broadcast;
> @@ -119,20 +118,18 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
> broadcast = true;
> }
>
> - if (broadcast) {
> - if (riscv_use_ipi_for_rfence()) {
> - ftd.asid = asid;
> - ftd.start = start;
> - ftd.size = size;
> - ftd.stride = stride;
> - on_each_cpu_mask(cmask,
> - __ipi_flush_tlb_range_asid,
> - &ftd, 1);
> - } else
> - sbi_remote_sfence_vma_asid(cmask,
> - start, size, asid);
> - } else {
> + if (!broadcast) {
> local_flush_tlb_range_asid(start, size, stride, asid);
> + } else if (riscv_use_sbi_for_rfence()) {
> + sbi_remote_sfence_vma_asid(cmask, start, size, asid);
> + } else {
> + struct flush_tlb_range_data ftd;
> +
> + ftd.asid = asid;
> + ftd.start = start;
> + ftd.size = size;
> + ftd.stride = stride;
> + on_each_cpu_mask(cmask, __ipi_flush_tlb_range_asid, &ftd, 1);
> }
>
> if (mm)
> diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
> index 9a55e733ae99..7ccc16dd6a76 100644
> --- a/drivers/clocksource/timer-clint.c
> +++ b/drivers/clocksource/timer-clint.c
> @@ -251,7 +251,7 @@ static int __init clint_timer_init_dt(struct device_node *np)
> }
>
> irq_set_chained_handler(clint_ipi_irq, clint_ipi_interrupt);
> - riscv_ipi_set_virq_range(rc, BITS_PER_BYTE, true);
> + riscv_ipi_set_virq_range(rc, BITS_PER_BYTE);
> clint_clear_ipi();
> #endif
>
> --
> 2.42.0
>
You can add:
Reviewed-by: Alexandre Ghiti <[email protected]>
Thanks,
Alex
On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
<[email protected]> wrote:
>
> __flush_tlb_range() avoids broadcasting TLB flushes when an mm context
> is only active on the local CPU. Apply this same optimization to TLB
> flushes of kernel memory when only one CPU is online. This check can be
> constant-folded when SMP is disabled.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v4:
> - New patch for v4
>
> arch/riscv/mm/tlbflush.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index 09b03bf71e6a..2f18fe6fc4f3 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -98,27 +98,23 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
> {
> const struct cpumask *cmask;
> unsigned long asid = FLUSH_TLB_NO_ASID;
> - bool broadcast;
> + unsigned int cpu;
>
> if (mm) {
> - unsigned int cpuid;
> -
> cmask = mm_cpumask(mm);
> if (cpumask_empty(cmask))
> return;
>
> - cpuid = get_cpu();
> - /* check if the tlbflush needs to be sent to other CPUs */
> - broadcast = cpumask_any_but(cmask, cpuid) < nr_cpu_ids;
> -
> if (static_branch_unlikely(&use_asid_allocator))
> asid = atomic_long_read(&mm->context.id) & asid_mask;
> } else {
> cmask = cpu_online_mask;
> - broadcast = true;
> }
>
> - if (!broadcast) {
> + cpu = get_cpu();
> +
> + /* Check if the TLB flush needs to be sent to other CPUs. */
> + if (cpumask_any_but(cmask, cpu) >= nr_cpu_ids) {
> local_flush_tlb_range_asid(start, size, stride, asid);
> } else if (riscv_use_sbi_for_rfence()) {
> sbi_remote_sfence_vma_asid(cmask, start, size, asid);
> @@ -132,8 +128,7 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
> on_each_cpu_mask(cmask, __ipi_flush_tlb_range_asid, &ftd, 1);
> }
>
> - if (mm)
> - put_cpu();
> + put_cpu();
> }
>
> void flush_tlb_mm(struct mm_struct *mm)
> --
> 2.42.0
>
You can add:
Reviewed-by: Alexandre Ghiti <[email protected]>
Thanks,
Alex
On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
<[email protected]> wrote:
>
> If no other CPU is online, a local cache or TLB flush is sufficient.
> These checks can be constant-folded when SMP is disabled.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v4:
> - New patch for v4
>
> arch/riscv/mm/cacheflush.c | 4 +++-
> arch/riscv/mm/tlbflush.c | 4 +++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index 47c485bc7df0..f7933ae88a55 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -21,7 +21,9 @@ void flush_icache_all(void)
> {
> local_flush_icache_all();
>
> - if (riscv_use_sbi_for_rfence())
> + if (num_online_cpus() < 2)
> + return;
> + else if (riscv_use_sbi_for_rfence())
> sbi_remote_fence_i(NULL);
> else
> on_each_cpu(ipi_remote_fence_i, NULL, 1);
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index 2f18fe6fc4f3..37b3c93e3c30 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -73,7 +73,9 @@ static void __ipi_flush_tlb_all(void *info)
>
> void flush_tlb_all(void)
> {
> - if (riscv_use_sbi_for_rfence())
> + if (num_online_cpus() < 2)
> + local_flush_tlb_all();
> + else if (riscv_use_sbi_for_rfence())
> sbi_remote_sfence_vma_asid(NULL, 0, FLUSH_TLB_MAX_SIZE, FLUSH_TLB_NO_ASID);
> else
> on_each_cpu(__ipi_flush_tlb_all, NULL, 1);
> --
> 2.42.0
>
on_each_cpu() already deals correctly with a single online cpu, the
only thing to optimize here is the SBI rfence. So I'd move this new
test in sbi_remote_sfence_vma_asid() and sbi_remote_fence_i() to avoid
the superfluous M-mode entry when only one cpu is online by checking
the cpumask. And since sbi_remote_fence_i() is used in another
function (flush_icache_mm()), we could also take advantage of this
optimization when only the local cpu must be flushed.
On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
<[email protected]> wrote:
>
> In SMP configurations, all TLB flushing narrower than flush_tlb_all()
> goes through __flush_tlb_range(). Do the same in UP configurations.
>
> This allows UP configurations to take advantage of recent improvements
> to the code in tlbflush.c, such as support for huge pages and flushing
> multiple-page ranges.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v4:
> - Merge the two copies of __flush_tlb_range() and rely on the compiler
> to optimize out the broadcast path (both clang and gcc do this)
> - Merge the two copies of flush_tlb_all() and rely on constant folding
>
> Changes in v2:
> - Move the SMP/UP merge earlier in the series to avoid build issues
> - Make a copy of __flush_tlb_range() instead of adding ifdefs inside
> - local_flush_tlb_all() is the only function used on !MMU (smpboot.c)
>
> arch/riscv/include/asm/tlbflush.h | 29 +++--------------------------
> arch/riscv/mm/Makefile | 5 +----
> 2 files changed, 4 insertions(+), 30 deletions(-)
>
> diff --git a/arch/riscv/include/asm/tlbflush.h b/arch/riscv/include/asm/tlbflush.h
> index 8f3418c5f172..7712ffe2f6c4 100644
> --- a/arch/riscv/include/asm/tlbflush.h
> +++ b/arch/riscv/include/asm/tlbflush.h
> @@ -27,12 +27,7 @@ static inline void local_flush_tlb_page(unsigned long addr)
> {
> ALT_FLUSH_TLB_PAGE(__asm__ __volatile__ ("sfence.vma %0" : : "r" (addr) : "memory"));
> }
> -#else /* CONFIG_MMU */
> -#define local_flush_tlb_all() do { } while (0)
> -#define local_flush_tlb_page(addr) do { } while (0)
> -#endif /* CONFIG_MMU */
>
> -#if defined(CONFIG_SMP) && defined(CONFIG_MMU)
> void flush_tlb_all(void);
> void flush_tlb_mm(struct mm_struct *mm);
> void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
> @@ -46,26 +41,8 @@ void flush_tlb_kernel_range(unsigned long start, unsigned long end);
> void flush_pmd_tlb_range(struct vm_area_struct *vma, unsigned long start,
> unsigned long end);
> #endif
> -#else /* CONFIG_SMP && CONFIG_MMU */
> -
> -#define flush_tlb_all() local_flush_tlb_all()
> -#define flush_tlb_page(vma, addr) local_flush_tlb_page(addr)
> -
> -static inline void flush_tlb_range(struct vm_area_struct *vma,
> - unsigned long start, unsigned long end)
> -{
> - local_flush_tlb_all();
> -}
> -
> -/* Flush a range of kernel pages */
> -static inline void flush_tlb_kernel_range(unsigned long start,
> - unsigned long end)
> -{
> - local_flush_tlb_all();
> -}
> -
> -#define flush_tlb_mm(mm) flush_tlb_all()
> -#define flush_tlb_mm_range(mm, start, end, page_size) flush_tlb_all()
> -#endif /* !CONFIG_SMP || !CONFIG_MMU */
> +#else /* CONFIG_MMU */
> +#define local_flush_tlb_all() do { } while (0)
> +#endif /* CONFIG_MMU */
>
> #endif /* _ASM_RISCV_TLBFLUSH_H */
> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> index 3a4dfc8babcf..96e65c571ce8 100644
> --- a/arch/riscv/mm/Makefile
> +++ b/arch/riscv/mm/Makefile
> @@ -13,15 +13,12 @@ endif
> KCOV_INSTRUMENT_init.o := n
>
> obj-y += init.o
> -obj-$(CONFIG_MMU) += extable.o fault.o pageattr.o
> +obj-$(CONFIG_MMU) += extable.o fault.o pageattr.o tlbflush.o
> obj-y += cacheflush.o
> obj-y += context.o
> obj-y += pgtable.o
> obj-y += pmem.o
>
> -ifeq ($(CONFIG_MMU),y)
> -obj-$(CONFIG_SMP) += tlbflush.o
> -endif
> obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
> obj-$(CONFIG_PTDUMP_CORE) += ptdump.o
> obj-$(CONFIG_KASAN) += kasan_init.o
> --
> 2.42.0
>
Nice one, you can add:
Reviewed-by: Alexandre Ghiti <[email protected]>
Thanks,
Alex
On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
<[email protected]> wrote:
>
> When using the ASID allocator, the MM context ID contains two values:
> the ASID in the lower bits, and the allocator version number in the
> remaining bits. Use macros to make this separation more obvious.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> (no changes since v1)
>
> arch/riscv/include/asm/mmu.h | 3 +++
> arch/riscv/mm/context.c | 12 ++++++------
> arch/riscv/mm/tlbflush.c | 2 +-
> 3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/arch/riscv/include/asm/mmu.h b/arch/riscv/include/asm/mmu.h
> index 355504b37f8e..a550fbf770be 100644
> --- a/arch/riscv/include/asm/mmu.h
> +++ b/arch/riscv/include/asm/mmu.h
> @@ -26,6 +26,9 @@ typedef struct {
> #endif
> } mm_context_t;
>
> +#define cntx2asid(cntx) ((cntx) & asid_mask)
> +#define cntx2version(cntx) ((cntx) & ~asid_mask)
Not a big fan of the naming, I would have something like
get_asid_from_context() and get_version_from_context() or something
like that, but up to you of course.
> +
> void __init create_pgd_mapping(pgd_t *pgdp, uintptr_t va, phys_addr_t pa,
> phys_addr_t sz, pgprot_t prot);
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
> index 217fd4de6134..43d005f63253 100644
> --- a/arch/riscv/mm/context.c
> +++ b/arch/riscv/mm/context.c
> @@ -81,7 +81,7 @@ static void __flush_context(void)
> if (cntx == 0)
> cntx = per_cpu(reserved_context, i);
>
> - __set_bit(cntx & asid_mask, context_asid_map);
> + __set_bit(cntx2asid(cntx), context_asid_map);
> per_cpu(reserved_context, i) = cntx;
> }
>
> @@ -102,7 +102,7 @@ static unsigned long __new_context(struct mm_struct *mm)
> lockdep_assert_held(&context_lock);
>
> if (cntx != 0) {
> - unsigned long newcntx = ver | (cntx & asid_mask);
> + unsigned long newcntx = ver | cntx2asid(cntx);
>
> /*
> * If our current CONTEXT was active during a rollover, we
> @@ -115,7 +115,7 @@ static unsigned long __new_context(struct mm_struct *mm)
> * We had a valid CONTEXT in a previous life, so try to
> * re-use it if possible.
> */
> - if (!__test_and_set_bit(cntx & asid_mask, context_asid_map))
> + if (!__test_and_set_bit(cntx2asid(cntx), context_asid_map))
> return newcntx;
> }
>
> @@ -168,7 +168,7 @@ static void set_mm_asid(struct mm_struct *mm, unsigned int cpu)
> */
> old_active_cntx = atomic_long_read(&per_cpu(active_context, cpu));
> if (old_active_cntx &&
> - ((cntx & ~asid_mask) == atomic_long_read(¤t_version)) &&
> + (cntx2version(cntx) == atomic_long_read(¤t_version)) &&
> atomic_long_cmpxchg_relaxed(&per_cpu(active_context, cpu),
> old_active_cntx, cntx))
> goto switch_mm_fast;
> @@ -177,7 +177,7 @@ static void set_mm_asid(struct mm_struct *mm, unsigned int cpu)
>
> /* Check that our ASID belongs to the current_version. */
> cntx = atomic_long_read(&mm->context.id);
> - if ((cntx & ~asid_mask) != atomic_long_read(¤t_version)) {
> + if (cntx2version(cntx) != atomic_long_read(¤t_version)) {
> cntx = __new_context(mm);
> atomic_long_set(&mm->context.id, cntx);
> }
> @@ -191,7 +191,7 @@ static void set_mm_asid(struct mm_struct *mm, unsigned int cpu)
>
> switch_mm_fast:
> csr_write(CSR_SATP, virt_to_pfn(mm->pgd) |
> - ((cntx & asid_mask) << SATP_ASID_SHIFT) |
> + (cntx2asid(cntx) << SATP_ASID_SHIFT) |
> satp_mode);
>
> if (need_flush_tlb)
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index 76b24d4ed4ab..5ec621545c69 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -85,7 +85,7 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
> return;
>
> if (static_branch_unlikely(&use_asid_allocator))
> - asid = atomic_long_read(&mm->context.id) & asid_mask;
> + asid = cntx2asid(atomic_long_read(&mm->context.id));
> } else {
> cmask = cpu_online_mask;
> }
> --
> 2.42.0
>
You can add:
Reviewed-by: Alexandre Ghiti <[email protected]>
Thanks,
Alex
On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
<[email protected]> wrote:
>
> Currently, the size of the ASID field in the MM context ID dynamically
> depends on the number of hardware-supported ASID bits. This requires
> reading a global variable to extract either field from the context ID.
> Instead, allocate the maximum possible number of bits to the ASID field,
> so the layout of the context ID is known at compile-time.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> (no changes since v1)
>
> arch/riscv/include/asm/mmu.h | 4 ++--
> arch/riscv/include/asm/tlbflush.h | 2 --
> arch/riscv/mm/context.c | 6 ++----
> 3 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/include/asm/mmu.h b/arch/riscv/include/asm/mmu.h
> index a550fbf770be..dc0273f7905f 100644
> --- a/arch/riscv/include/asm/mmu.h
> +++ b/arch/riscv/include/asm/mmu.h
> @@ -26,8 +26,8 @@ typedef struct {
> #endif
> } mm_context_t;
>
> -#define cntx2asid(cntx) ((cntx) & asid_mask)
> -#define cntx2version(cntx) ((cntx) & ~asid_mask)
> +#define cntx2asid(cntx) ((cntx) & SATP_ASID_MASK)
> +#define cntx2version(cntx) ((cntx) & ~SATP_ASID_MASK)
>
> void __init create_pgd_mapping(pgd_t *pgdp, uintptr_t va, phys_addr_t pa,
> phys_addr_t sz, pgprot_t prot);
> diff --git a/arch/riscv/include/asm/tlbflush.h b/arch/riscv/include/asm/tlbflush.h
> index d9913590f82e..5bfd37cfd8c3 100644
> --- a/arch/riscv/include/asm/tlbflush.h
> +++ b/arch/riscv/include/asm/tlbflush.h
> @@ -15,8 +15,6 @@
> #define FLUSH_TLB_NO_ASID ((unsigned long)-1)
>
> #ifdef CONFIG_MMU
> -extern unsigned long asid_mask;
> -
> static inline void local_flush_tlb_all(void)
> {
> __asm__ __volatile__ ("sfence.vma" : : : "memory");
> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
> index 43d005f63253..b5170ac1b742 100644
> --- a/arch/riscv/mm/context.c
> +++ b/arch/riscv/mm/context.c
> @@ -22,7 +22,6 @@ DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
>
> static unsigned long asid_bits;
> static unsigned long num_asids;
> -unsigned long asid_mask;
>
> static atomic_long_t current_version;
>
> @@ -128,7 +127,7 @@ static unsigned long __new_context(struct mm_struct *mm)
> goto set_asid;
>
> /* We're out of ASIDs, so increment current_version */
> - ver = atomic_long_add_return_relaxed(num_asids, ¤t_version);
> + ver = atomic_long_add_return_relaxed(BIT(SATP_ASID_BITS), ¤t_version);
>
> /* Flush everything */
> __flush_context();
> @@ -247,7 +246,6 @@ static int __init asids_init(void)
> /* Pre-compute ASID details */
> if (asid_bits) {
> num_asids = 1 << asid_bits;
> - asid_mask = num_asids - 1;
> }
>
> /*
> @@ -255,7 +253,7 @@ static int __init asids_init(void)
> * at-least twice more than CPUs
> */
> if (num_asids > (2 * num_possible_cpus())) {
> - atomic_long_set(¤t_version, num_asids);
> + atomic_long_set(¤t_version, BIT(SATP_ASID_BITS));
>
> context_asid_map = bitmap_zalloc(num_asids, GFP_KERNEL);
> if (!context_asid_map)
> --
> 2.42.0
>
You can add:
Reviewed-by: Alexandre Ghiti <[email protected]>
Thanks,
Alex
On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
<[email protected]> wrote:
>
> This variable is only used inside asids_init().
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> (no changes since v1)
>
> arch/riscv/mm/context.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
> index b5170ac1b742..43a8bc2d5af4 100644
> --- a/arch/riscv/mm/context.c
> +++ b/arch/riscv/mm/context.c
> @@ -20,7 +20,6 @@
>
> DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
>
> -static unsigned long asid_bits;
> static unsigned long num_asids;
>
> static atomic_long_t current_version;
> @@ -226,7 +225,7 @@ static inline void set_mm(struct mm_struct *prev,
>
> static int __init asids_init(void)
> {
> - unsigned long old;
> + unsigned long asid_bits, old;
>
> /* Figure-out number of ASID bits in HW */
> old = csr_read(CSR_SATP);
> --
> 2.42.0
>
You can add:
Reviewed-by: Alexandre Ghiti <[email protected]>
Thanks,
Alex
On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
<[email protected]> wrote:
>
> If the CPU does not support multiple ASIDs, all MM contexts use ASID 0.
> In this case, it is still beneficial to flush the TLB by ASID, as the
> single-ASID variant of the sfence.vma instruction preserves TLB entries
> for global (kernel) pages.
>
> This optimization is recommended by the RISC-V privileged specification:
>
> If the implementation does not provide ASIDs, or software chooses
> to always use ASID 0, then after every satp write, software should
> execute SFENCE.VMA with rs1=x0. In the common case that no global
> translations have been modified, rs2 should be set to a register
> other than x0 but which contains the value zero, so that global
> translations are not flushed.
>
> It is not possible to apply this optimization when using the ASID
> allocator, because that code must flush the TLB for all ASIDs at once
> when incrementing the version number.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> (no changes since v1)
>
> arch/riscv/mm/context.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
> index 43a8bc2d5af4..3ca9b653df7d 100644
> --- a/arch/riscv/mm/context.c
> +++ b/arch/riscv/mm/context.c
> @@ -200,7 +200,7 @@ static void set_mm_noasid(struct mm_struct *mm)
> {
> /* Switch the page table and blindly nuke entire local TLB */
> csr_write(CSR_SATP, virt_to_pfn(mm->pgd) | satp_mode);
> - local_flush_tlb_all();
> + local_flush_tlb_all_asid(0);
> }
>
> static inline void set_mm(struct mm_struct *prev,
> --
> 2.42.0
>
You can add:
Reviewed-by: Alexandre Ghiti <[email protected]>
Thanks,
Alex
On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
<[email protected]> wrote:
>
> Even if multiple ASIDs are not supported, using the single-ASID variant
> of the sfence.vma instruction preserves TLB entries for global (kernel)
> pages. So it is always more efficient to use the single-ASID code path.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v4:
> - There is now only one copy of __flush_tlb_range()
>
> Changes in v2:
> - Update both copies of __flush_tlb_range()
>
> arch/riscv/include/asm/mmu_context.h | 2 --
> arch/riscv/mm/context.c | 3 +--
> arch/riscv/mm/tlbflush.c | 3 +--
> 3 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/riscv/include/asm/mmu_context.h b/arch/riscv/include/asm/mmu_context.h
> index 7030837adc1a..b0659413a080 100644
> --- a/arch/riscv/include/asm/mmu_context.h
> +++ b/arch/riscv/include/asm/mmu_context.h
> @@ -33,8 +33,6 @@ static inline int init_new_context(struct task_struct *tsk,
> return 0;
> }
>
> -DECLARE_STATIC_KEY_FALSE(use_asid_allocator);
> -
> #include <asm-generic/mmu_context.h>
>
> #endif /* _ASM_RISCV_MMU_CONTEXT_H */
> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
> index 3ca9b653df7d..20057085ab8a 100644
> --- a/arch/riscv/mm/context.c
> +++ b/arch/riscv/mm/context.c
> @@ -18,8 +18,7 @@
>
> #ifdef CONFIG_MMU
>
> -DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
> -
> +static DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
> static unsigned long num_asids;
>
> static atomic_long_t current_version;
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index 5ec621545c69..39d80f56d292 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -84,8 +84,7 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
> if (cpumask_empty(cmask))
> return;
>
> - if (static_branch_unlikely(&use_asid_allocator))
> - asid = cntx2asid(atomic_long_read(&mm->context.id));
> + asid = cntx2asid(atomic_long_read(&mm->context.id));
> } else {
> cmask = cpu_online_mask;
> }
> --
> 2.42.0
>
You can add:
Reviewed-by: Alexandre Ghiti <[email protected]>
Thanks!
Alex
Hi Alex,
On 2024-01-04 6:33 AM, Alexandre Ghiti wrote:
> On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
> <[email protected]> wrote:
>>
>> If no other CPU is online, a local cache or TLB flush is sufficient.
>> These checks can be constant-folded when SMP is disabled.
>>
>> Signed-off-by: Samuel Holland <[email protected]>
>> ---
>>
>> Changes in v4:
>> - New patch for v4
>>
>> arch/riscv/mm/cacheflush.c | 4 +++-
>> arch/riscv/mm/tlbflush.c | 4 +++-
>> 2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
>> index 47c485bc7df0..f7933ae88a55 100644
>> --- a/arch/riscv/mm/cacheflush.c
>> +++ b/arch/riscv/mm/cacheflush.c
>> @@ -21,7 +21,9 @@ void flush_icache_all(void)
>> {
>> local_flush_icache_all();
>>
>> - if (riscv_use_sbi_for_rfence())
>> + if (num_online_cpus() < 2)
>> + return;
>> + else if (riscv_use_sbi_for_rfence())
>> sbi_remote_fence_i(NULL);
>> else
>> on_each_cpu(ipi_remote_fence_i, NULL, 1);
>> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
>> index 2f18fe6fc4f3..37b3c93e3c30 100644
>> --- a/arch/riscv/mm/tlbflush.c
>> +++ b/arch/riscv/mm/tlbflush.c
>> @@ -73,7 +73,9 @@ static void __ipi_flush_tlb_all(void *info)
>>
>> void flush_tlb_all(void)
>> {
>> - if (riscv_use_sbi_for_rfence())
>> + if (num_online_cpus() < 2)
>> + local_flush_tlb_all();
>> + else if (riscv_use_sbi_for_rfence())
>> sbi_remote_sfence_vma_asid(NULL, 0, FLUSH_TLB_MAX_SIZE, FLUSH_TLB_NO_ASID);
>> else
>> on_each_cpu(__ipi_flush_tlb_all, NULL, 1);
>> --
>> 2.42.0
>>
>
> on_each_cpu() already deals correctly with a single online cpu, the
> only thing to optimize here is the SBI rfence. So I'd move this new
> test in sbi_remote_sfence_vma_asid() and sbi_remote_fence_i() to avoid
> the superfluous M-mode entry when only one cpu is online by checking
> the cpumask. And since sbi_remote_fence_i() is used in another
What specific cpumask check are you suggesting? In sbi_remote_sfence_vma_asid()
I don't think we can assume the local cpu is always included in the cpumask
(which we _can_ assume here), so it would need to construct/compare the whole
bitmap. That's much more expensive than the atomic load here.
> function (flush_icache_mm()), we could also take advantage of this
> optimization when only the local cpu must be flushed.
flush_icache_mm() already has a "local" variable which it uses to skip the call
to sbi_remote_fence_i(). Same with the broadcast check in __flush_tlb_range().
So no additional check is needed there. Those two functions, plus the two
changed in this patch, are the only call sites of the sbi_*() functions. I think
it makes more sense to optimize the four call sites than adding an additional
check in the sbi_*() functions.
Regards,
Samuel
Hi Alex,
On 2024-01-04 6:39 AM, Alexandre Ghiti wrote:
> On Tue, Jan 2, 2024 at 11:01 PM Samuel Holland
> <[email protected]> wrote:
>>
>> When using the ASID allocator, the MM context ID contains two values:
>> the ASID in the lower bits, and the allocator version number in the
>> remaining bits. Use macros to make this separation more obvious.
>>
>> Signed-off-by: Samuel Holland <[email protected]>
>> ---
>>
>> (no changes since v1)
>>
>> arch/riscv/include/asm/mmu.h | 3 +++
>> arch/riscv/mm/context.c | 12 ++++++------
>> arch/riscv/mm/tlbflush.c | 2 +-
>> 3 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/mmu.h b/arch/riscv/include/asm/mmu.h
>> index 355504b37f8e..a550fbf770be 100644
>> --- a/arch/riscv/include/asm/mmu.h
>> +++ b/arch/riscv/include/asm/mmu.h
>> @@ -26,6 +26,9 @@ typedef struct {
>> #endif
>> } mm_context_t;
>>
>> +#define cntx2asid(cntx) ((cntx) & asid_mask)
>> +#define cntx2version(cntx) ((cntx) & ~asid_mask)
>
> Not a big fan of the naming, I would have something like
> get_asid_from_context() and get_version_from_context() or something
> like that, but up to you of course.
Thanks for the review. I'm not really a fan of it either, but I tried to match
precedent from other architectures. These loosely follow the naming of
ctxid2asid() from arm64 and idx2asid() from csky. arm uses ASID().
Regards,
Samuel
>> +
>> void __init create_pgd_mapping(pgd_t *pgdp, uintptr_t va, phys_addr_t pa,
>> phys_addr_t sz, pgprot_t prot);
>> #endif /* __ASSEMBLY__ */
>> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
>> index 217fd4de6134..43d005f63253 100644
>> --- a/arch/riscv/mm/context.c
>> +++ b/arch/riscv/mm/context.c
>> @@ -81,7 +81,7 @@ static void __flush_context(void)
>> if (cntx == 0)
>> cntx = per_cpu(reserved_context, i);
>>
>> - __set_bit(cntx & asid_mask, context_asid_map);
>> + __set_bit(cntx2asid(cntx), context_asid_map);
>> per_cpu(reserved_context, i) = cntx;
>> }
>>
>> @@ -102,7 +102,7 @@ static unsigned long __new_context(struct mm_struct *mm)
>> lockdep_assert_held(&context_lock);
>>
>> if (cntx != 0) {
>> - unsigned long newcntx = ver | (cntx & asid_mask);
>> + unsigned long newcntx = ver | cntx2asid(cntx);
>>
>> /*
>> * If our current CONTEXT was active during a rollover, we
>> @@ -115,7 +115,7 @@ static unsigned long __new_context(struct mm_struct *mm)
>> * We had a valid CONTEXT in a previous life, so try to
>> * re-use it if possible.
>> */
>> - if (!__test_and_set_bit(cntx & asid_mask, context_asid_map))
>> + if (!__test_and_set_bit(cntx2asid(cntx), context_asid_map))
>> return newcntx;
>> }
>>
>> @@ -168,7 +168,7 @@ static void set_mm_asid(struct mm_struct *mm, unsigned int cpu)
>> */
>> old_active_cntx = atomic_long_read(&per_cpu(active_context, cpu));
>> if (old_active_cntx &&
>> - ((cntx & ~asid_mask) == atomic_long_read(¤t_version)) &&
>> + (cntx2version(cntx) == atomic_long_read(¤t_version)) &&
>> atomic_long_cmpxchg_relaxed(&per_cpu(active_context, cpu),
>> old_active_cntx, cntx))
>> goto switch_mm_fast;
>> @@ -177,7 +177,7 @@ static void set_mm_asid(struct mm_struct *mm, unsigned int cpu)
>>
>> /* Check that our ASID belongs to the current_version. */
>> cntx = atomic_long_read(&mm->context.id);
>> - if ((cntx & ~asid_mask) != atomic_long_read(¤t_version)) {
>> + if (cntx2version(cntx) != atomic_long_read(¤t_version)) {
>> cntx = __new_context(mm);
>> atomic_long_set(&mm->context.id, cntx);
>> }
>> @@ -191,7 +191,7 @@ static void set_mm_asid(struct mm_struct *mm, unsigned int cpu)
>>
>> switch_mm_fast:
>> csr_write(CSR_SATP, virt_to_pfn(mm->pgd) |
>> - ((cntx & asid_mask) << SATP_ASID_SHIFT) |
>> + (cntx2asid(cntx) << SATP_ASID_SHIFT) |
>> satp_mode);
>>
>> if (need_flush_tlb)
>> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
>> index 76b24d4ed4ab..5ec621545c69 100644
>> --- a/arch/riscv/mm/tlbflush.c
>> +++ b/arch/riscv/mm/tlbflush.c
>> @@ -85,7 +85,7 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
>> return;
>>
>> if (static_branch_unlikely(&use_asid_allocator))
>> - asid = atomic_long_read(&mm->context.id) & asid_mask;
>> + asid = cntx2asid(atomic_long_read(&mm->context.id));
>> } else {
>> cmask = cpu_online_mask;
>> }
>> --
>> 2.42.0
>>
>
> You can add:
>
> Reviewed-by: Alexandre Ghiti <[email protected]>
>
> Thanks,
>
> Alex
On 2024-01-03 9:00 AM, Jisheng Zhang wrote:
> On Tue, Jan 02, 2024 at 02:00:47PM -0800, Samuel Holland wrote:
>> This variable is only used inside asids_init().
>
> This is due to patch9, so can be folded into patch9.
I'm not sure what you mean here. Patch 9 does not touch any references to the
asid_bits variable, though it does touch adjacent lines.
>>
>> Signed-off-by: Samuel Holland <[email protected]>
>> ---
>>
>> (no changes since v1)
>>
>> arch/riscv/mm/context.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
>> index b5170ac1b742..43a8bc2d5af4 100644
>> --- a/arch/riscv/mm/context.c
>> +++ b/arch/riscv/mm/context.c
>> @@ -20,7 +20,6 @@
>>
>> DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
>>
>> -static unsigned long asid_bits;
>> static unsigned long num_asids;
>>
>> static atomic_long_t current_version;
>> @@ -226,7 +225,7 @@ static inline void set_mm(struct mm_struct *prev,
>>
>> static int __init asids_init(void)
>> {
>> - unsigned long old;
>> + unsigned long asid_bits, old;
>>
>> /* Figure-out number of ASID bits in HW */
>> old = csr_read(CSR_SATP);
>> --
>> 2.42.0
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
On 2024-01-03 9:02 AM, Jisheng Zhang wrote:
> On Tue, Jan 02, 2024 at 02:00:49PM -0800, Samuel Holland wrote:
>> Even if multiple ASIDs are not supported, using the single-ASID variant
>> of the sfence.vma instruction preserves TLB entries for global (kernel)
>> pages. So it is always more efficient to use the single-ASID code path.
>>
>> Signed-off-by: Samuel Holland <[email protected]>
>> ---
>>
>> Changes in v4:
>> - There is now only one copy of __flush_tlb_range()
>>
>> Changes in v2:
>> - Update both copies of __flush_tlb_range()
>>
>> arch/riscv/include/asm/mmu_context.h | 2 --
>> arch/riscv/mm/context.c | 3 +--
>> arch/riscv/mm/tlbflush.c | 3 +--
>> 3 files changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/mmu_context.h b/arch/riscv/include/asm/mmu_context.h
>> index 7030837adc1a..b0659413a080 100644
>> --- a/arch/riscv/include/asm/mmu_context.h
>> +++ b/arch/riscv/include/asm/mmu_context.h
>> @@ -33,8 +33,6 @@ static inline int init_new_context(struct task_struct *tsk,
>> return 0;
>> }
>>
>> -DECLARE_STATIC_KEY_FALSE(use_asid_allocator);
>> -
>> #include <asm-generic/mmu_context.h>
>>
>> #endif /* _ASM_RISCV_MMU_CONTEXT_H */
>> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
>> index 3ca9b653df7d..20057085ab8a 100644
>> --- a/arch/riscv/mm/context.c
>> +++ b/arch/riscv/mm/context.c
>> @@ -18,8 +18,7 @@
>>
>> #ifdef CONFIG_MMU
>>
>> -DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
>> -
>> +static DEFINE_STATIC_KEY_FALSE(use_asid_allocator);
>
> One of my optimization "riscv: tlb: avoid tlb flushing if fullmm == 1"
> will make use of use_asid_allocator, so could we remove this modification?
Yes, I can leave the global declaration alone for now.
>> static unsigned long num_asids;
>>
>> static atomic_long_t current_version;
>> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
>> index 5ec621545c69..39d80f56d292 100644
>> --- a/arch/riscv/mm/tlbflush.c
>> +++ b/arch/riscv/mm/tlbflush.c
>> @@ -84,8 +84,7 @@ static void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
>> if (cpumask_empty(cmask))
>> return;
>>
>> - if (static_branch_unlikely(&use_asid_allocator))
>> - asid = cntx2asid(atomic_long_read(&mm->context.id));
>> + asid = cntx2asid(atomic_long_read(&mm->context.id));
>> } else {
>> cmask = cpu_online_mask;
>> }
>> --
>> 2.42.0
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv