riscv: Fixup asid_allocator remaining issues
v3 cover riscv: Fixup asid_allocator remaining issues
From: Guo Ren <[email protected]>
The patchset fixes the remaining problems of asid_allocator.
- Fixup _PAGE_GLOBAL for kernel virtual address mapping
- Optimize tlb_flush with asid & range
Changes since v2:
- Remove PAGE_UP/DOWN usage in tlbflush.h
- Optimize variable name
Changes since v1:
- Drop PAGE_UP wrong fixup
- Rebase on clean linux-5.13-rc2
- Add Reviewed-by
Guo Ren (2):
riscv: Fixup _PAGE_GLOBAL in _PAGE_KERNEL
riscv: Use use_asid_allocator flush TLB
arch/riscv/include/asm/mmu_context.h | 2 ++ arch/riscv/include/asm/pgtable.h | 3 ++- arch/riscv/include/asm/tlbflush.h | 21 +++++++++++++++++++ arch/riscv/mm/context.c | 2 +- arch/riscv/mm/tlbflush.c | 40 +++++++++++++++++++++++++++++++++--- 5 files changed, 63 insertions(+), 5 deletions(-)
Signature · 1 line
2.7.4
PATCH v3 1/2 riscv: Fixup _PAGE_GLOBAL in _PAGE_KERNEL
From: Guo Ren <[email protected]>
Kernel virtual address translation should avoid care asid or it'll cause more TLB-miss and TLB-refill. Because the current asid in satp belongs to the current process, but the target kernel va TLB entry's asid still belongs to the previous process.
--- arch/riscv/include/asm/pgtable.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/riscv/include/asm/pgtable.h +2 -1
@@ -134,7 +134,8 @@ | _PAGE_WRITE \ | _PAGE_PRESENT \ | _PAGE_ACCESSED \ - | _PAGE_DIRTY) + | _PAGE_DIRTY \ + | _PAGE_GLOBAL)#define PAGE_KERNEL __pgprot(_PAGE_KERNEL)#define PAGE_KERNEL_READ __pgprot(_PAGE_KERNEL & ~_PAGE_WRITE)
Signature · 1 line
2.7.4
Re: [PATCH V3 1/2] riscv: Fixup _PAGE_GLOBAL in _PAGE_KERNEL
Quoted 3 lines · On Tue, May 25, 2021 at 12:24:06PM +0000, [email protected] wrote:
From: Guo Ren <[email protected]>
Kernel virtual address translation should avoid care asid or it'll
s/care aisd/to use ASIDs/ ?
Quoted 1 line
cause more TLB-miss and TLB-refill. Because the current asid in satp
s/asid/ASID/ ?
Quoted 6 lines
belongs to the current process, but the target kernel va TLB entry's asid still belongs to the previous process.
Signed-off-by Guo Ren <[email protected]>Reviewed-by Anup Patel <[email protected]>Cc Palmer Dabbelt <[email protected]>
Otherwise looks good,
PATCH v3 2/2 riscv: Use use_asid_allocator flush TLB
In reply to [email protected]From: Guo Ren <[email protected]>
Use static_branch_unlikely(&use_asid_allocator) to keep the origin tlb flush style, so it's no effect on the existing machine. Here are the optimized functions:
- flush_tlb_mm
- flush_tlb_page
- flush_tlb_range
All above are based on the below new implement functions:
- __sbi_tlb_flush_range_asid
- local_flush_tlb_range_asid
--- arch/riscv/include/asm/mmu_context.h | 2 ++ arch/riscv/include/asm/tlbflush.h | 21 +++++++++++++++++++ arch/riscv/mm/context.c | 2 +- arch/riscv/mm/tlbflush.c | 40 +++++++++++++++++++++++++++++++++--- 4 files changed, 61 insertions(+), 4 deletions(-)
arch/riscv/include/asm/mmu_context.h +2 -0
@@ -33,6 +33,8 @@ 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 */
arch/riscv/include/asm/tlbflush.h +21 -0
@@ -22,9 +22,30 @@ static inline void local_flush_tlb_page(unsigned long addr) {ALT_FLUSH_TLB_PAGE (__asm__ __volatile__ ("sfence.vma %0" : :"r" (addr) :"memory" )); } + +static inline void local_flush_tlb_range_asid (unsigned long start,unsigned long size, +unsigned long asid) +{ +unsigned long tmp = start &PAGE_MASK ; +unsigned long end =ALIGN (start + size,PAGE_SIZE ); + +if (size ==-1 ) { + __asm__ __volatile__ ("sfence.vma x0, %0" : :"r" (asid) :"memory" ); +return ; + } + +while (tmp < end) { + __asm__ __volatile__ ("sfence.vma %0, %1" + : + :"r" (tmp),"r" (asid) + :"memory" ); + tmp +=PAGE_SIZE ; + } +}#else /* CONFIG_MMU */ #define local_flush_tlb_all () do { } while (0)#define local_flush_tlb_page (addr) do { } while (0) +#define local_flush_tlb_range_asid (addr) do { } while (0)#endif /* CONFIG_MMU */ #if defined(CONFIG_SMP ) && defined(CONFIG_MMU )
arch/riscv/mm/context.c +1 -1
@@ -18,7 +18,7 @@#ifdef CONFIG_MMU -static DEFINE_STATIC_KEY_FALSE (use_asid_allocator); +DEFINE_STATIC_KEY_FALSE (use_asid_allocator);static unsigned long asid_bits;static unsigned long num_asids;
arch/riscv/mm/tlbflush.c +37 -3
@@ -4,6 +4,7 @@#include <linux/smp.h> #include <linux/sched.h> #include <asm/sbi.h> +#include <asm/mmu_context.h> void flush_tlb_all (void ) {
@@ -39,18 +40,51 @@ static void __sbi_tlb_flush_range(struct cpumask *cmask, unsigned long start,put_cpu (); } +static void __sbi_tlb_flush_range_asid (struct cpumask *cmask,unsigned long start, +unsigned long size,unsigned long asid) +{ +struct cpumask hmask; +unsigned int cpuid; + +if (cpumask_empty (cmask)) +return ; + + cpuid =get_cpu (); + +if (cpumask_any_but (cmask, cpuid) >= nr_cpu_ids) { +local_flush_tlb_range_asid (start, size, asid); + }else { +riscv_cpuid_to_hartid_mask (cmask, &hmask); +sbi_remote_sfence_vma_asid (cpumask_bits (&hmask), start, size, asid); + } + +put_cpu (); +} +void flush_tlb_mm (struct mm_struct *mm) { -__sbi_tlb_flush_range (mm_cpumask (mm),0 ,-1 ); +if (static_branch_unlikely (&use_asid_allocator)) +__sbi_tlb_flush_range_asid (mm_cpumask (mm),0 ,-1 , +atomic_long_read (&mm->context.id)); +else +__sbi_tlb_flush_range (mm_cpumask (mm),0 ,-1 ); }void flush_tlb_page (struct vm_area_struct *vma,unsigned long addr) { -__sbi_tlb_flush_range (mm_cpumask (vma->vm_mm), addr,PAGE_SIZE ); +if (static_branch_unlikely (&use_asid_allocator)) +__sbi_tlb_flush_range_asid (mm_cpumask (vma->vm_mm), addr,PAGE_SIZE , +atomic_long_read (&vma->vm_mm->context.id)); +else +__sbi_tlb_flush_range (mm_cpumask (vma->vm_mm), addr,PAGE_SIZE ); }void flush_tlb_range (struct vm_area_struct *vma,unsigned long start,unsigned long end) { -__sbi_tlb_flush_range (mm_cpumask (vma->vm_mm), start, end - start); +if (static_branch_unlikely (&use_asid_allocator)) +__sbi_tlb_flush_range_asid (mm_cpumask (vma->vm_mm), start, end - start, +atomic_long_read (&vma->vm_mm->context.id)); +else +__sbi_tlb_flush_range (mm_cpumask (vma->vm_mm), start, end - start); }
Signature · 1 line
2.7.4
Re: [PATCH V3 2/2] riscv: Use use_asid_allocator flush TLB
Quoted 9 lines · On Tue, May 25, 2021 at 12:24:07PM +0000, [email protected] wrote:
From: Guo Ren <[email protected]>
Use
static_branch_unlikely(&use_asid_allocator)to keep the origin tlb flush style, so it's no effect on the existing machine. Here are the optimized functions:
-flush_tlb_mm
-flush_tlb_page
-flush_tlb_range
Quoted 3 lines
All above are based on the below new implement functions:
-__sbi_tlb_flush_range_asid
-local_flush_tlb_range_asid
This mentiones what functions you're changing, but not what the substantial change is, and more importantly why you change it.
arch/riscv/include/asm/tlbflush.h static inline void local_flush_tlb_page(unsigned long addr) · line 27
Quoted 2 lines
arch/riscv/include/asm/tlbflush.h +2
-0+static inline void local_flush_tlb_range_asid (unsigned long start,unsigned long size, +unsigned long asid)
Crazy long line. Should be:
static inline void local_flush_tlb_range_asid(unsigned long start,
unsigned long size, unsigned long asid)
arch/riscv/include/asm/tlbflush.h static inline void local_flush_tlb_page(unsigned long addr) · line 34
Quoted 7 lines
arch/riscv/include/asm/tlbflush.h +7
-0+{ +unsigned long tmp = start &PAGE_MASK ; +unsigned long end =ALIGN (start + size,PAGE_SIZE ); + +if (size ==-1 ) { + __asm__ __volatile__ ("sfence.vma x0, %0" : :"r" (asid) :"memory" ); +return ;
Please split the global (size == -1) case into separate helpers.
arch/riscv/include/asm/tlbflush.h static inline void local_flush_tlb_page(unsigned long addr) · line 37
Quoted 1 line
arch/riscv/include/asm/tlbflush.h +1
-0+while (tmp < end) {
Missing whitespace befre the (.
Also I think this would read nicer as:
for (tmp = start & PAGE_MASK; tmp < end; tmp += PAGE_SIZE)
arch/riscv/mm/tlbflush.c static void __sbi_tlb_flush_range(struct cpumask *cmask, unsigned long start, · line 44
Quoted 2 lines
arch/riscv/mm/tlbflush.c +2
-0+static void __sbi_tlb_flush_range_asid (struct cpumask *cmask,unsigned long start, +unsigned long size,unsigned long asid)
Another overly long line.
Also for all thee __sbi_* functions, why the __ prefix?
arch/riscv/mm/tlbflush.c static void __sbi_tlb_flush_range(struct cpumask *cmask, unsigned long start, · line 58
Quoted 5 lines
arch/riscv/mm/tlbflush.c +5
-0+if (cpumask_any_but (cmask, cpuid) >= nr_cpu_ids) { +local_flush_tlb_range_asid (start, size, asid); + }else { +riscv_cpuid_to_hartid_mask (cmask, &hmask); +sbi_remote_sfence_vma_asid (cpumask_bits (&hmask), start, size, asid);
Another long line (and a few more later).
Re: [PATCH V3 2/2] riscv: Use use_asid_allocator flush TLB
Quoted 27 lines · Christoph Hellwig
Quoted 9 lines · On Tue, May 25, 2021 at 12:24:07PM +0000, [email protected] wrote:
From: Guo Ren <[email protected]>
Use
static_branch_unlikely(&use_asid_allocator)to keep the origin tlb flush style, so it's no effect on the existing machine. Here are the optimized functions:
-flush_tlb_mm
-flush_tlb_page
-flush_tlb_rangeQuoted 3 lines
All above are based on the below new implement functions:
-__sbi_tlb_flush_range_asid
-local_flush_tlb_range_asidThis mentiones what functions you're changing, but not what the substantial change is, and more importantly why you change it.
arch/riscv/include/asm/tlbflush.h static inline void local_flush_tlb_page(unsigned long addr) · line 27
Quoted 2 lines
arch/riscv/include/asm/tlbflush.h +2
-0+static inline void local_flush_tlb_range_asid (unsigned long start,unsigned long size, +unsigned long asid)Crazy long line. Should be:
static inline void local_flush_tlb_range_asid(unsigned long start,
unsigned long size, unsigned long asid)
Quoted 9 lines
arch/riscv/include/asm/tlbflush.h static inline void local_flush_tlb_page(unsigned long addr) · line 34
Quoted 7 lines
arch/riscv/include/asm/tlbflush.h +7
-0+{ +unsigned long tmp = start &PAGE_MASK ; +unsigned long end =ALIGN (start + size,PAGE_SIZE ); + +if (size ==-1 ) { + __asm__ __volatile__ ("sfence.vma x0, %0" : :"r" (asid) :"memory" ); +return ;Please split the global (size == -1) case into separate helpers.
Do you mean:
if (size == -1) {
__asm__ __volatile__ ("sfence.vma x0, %0"
:
: "r" (asid)
: "memory");
} else {
for (tmp = start & PAGE_MASK; tmp < end; tmp += PAGE_SIZE) {
__asm__ __volatile__ ("sfence.vma %0, %1"
:
: "r" (tmp), "r" (asid)
: "memory");
tmp += PAGE_SIZE;
}
}Quoted 7 lines
Quoted 2 lines
+ while(tmp < end) {
Missing whitespace befre the (.
Also I think this would read nicer as:
Quoted 8 lines
for (tmp = start & PAGE_MASK; tmp < end; tmp += PAGE_SIZE)arch/riscv/mm/tlbflush.c static void __sbi_tlb_flush_range(struct cpumask *cmask, unsigned long start, · line 44
Quoted 2 lines
arch/riscv/mm/tlbflush.c +2
-0+static void __sbi_tlb_flush_range_asid (struct cpumask *cmask,unsigned long start, +unsigned long size,unsigned long asid)Another overly long line.
Also for all thee __sbi_* functions, why the __ prefix?
I just follow the previous coding convention by __sbi_tlb_flush_range. If you don't like it, I think it should be another coding convention patchset.
This patchset is only to add the functions of tlb_flush_with_asid.
Quoted 8 lines
arch/riscv/mm/tlbflush.c static void __sbi_tlb_flush_range(struct cpumask *cmask, unsigned long start, · line 58
Quoted 6 lines
arch/riscv/mm/tlbflush.c +5
-0+if (cpumask_any_but (cmask, cpuid) >= nr_cpu_ids) { +local_flush_tlb_range_asid (start, size, asid); + }else { +riscv_cpuid_to_hartid_mask (cmask, &hmask); +sbi_remote_sfence_vma_asid (cpumask_bits (&hmask), start, size, asid);Another long line (and a few more later).
Signature · 4 lines
Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/
Re: [PATCH V3 2/2] riscv: Use use_asid_allocator flush TLB
Quoted 12 lines · Guo Ren
Quoted 12 lines
static inline void local_flush_tlb_range_asid(unsigned long start,
unsigned long size, unsigned long asid)arch/riscv/include/asm/tlbflush.h static inline void local_flush_tlb_page(unsigned long addr) · line 34
Quoted 7 lines
arch/riscv/include/asm/tlbflush.h +7
-0+{ +unsigned long tmp = start &PAGE_MASK ; +unsigned long end =ALIGN (start + size,PAGE_SIZE ); + +if (size ==-1 ) { + __asm__ __volatile__ ("sfence.vma x0, %0" : :"r" (asid) :"memory" ); +return ;Please split the global (size == -1) case into separate helpers.
Quoted 1 line
Do you mean:
No. Basically a
static inline void local_flush_tlb_ll_asid(unsigned long asid)
{
__asm__ __volatile__ ("sfence.vma x0, %0"
:
: "r" (asid)
: "memory");
}and
static inline void local_flush_tlb_range_asid(unsigned long start,
unsigned long size, unsigned long asid)
{
unsigned long end = ALIGN(start + size, PAGE_SIZE), tmp;
for (tmp = start & PAGE_MASK; tmp < end; tmp += PAGE_SIZE) {
__asm__ __volatile__ ("sfence.vma %0, %1"
:
: "r" (tmp), "r" (asid)
: "memory");
}
}