| Message ID | 20240128120405.25876-1-alexghiti@rivosinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [-fixes] riscv: Flush the tlb when a page directory is freed | expand |
Hi Alexandre, On Sun, Jan 28, 2024 at 8:04 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote: > > The riscv privileged specification mandates to flush the TLB whenever a > page directory is modified, so add that to tlb_flush(). > > Fixes: c5e9b2c2ae82 ("riscv: Improve tlb_flush()") > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/include/asm/tlb.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/tlb.h b/arch/riscv/include/asm/tlb.h > index 1eb5682b2af6..50b63b5c15bd 100644 > --- a/arch/riscv/include/asm/tlb.h > +++ b/arch/riscv/include/asm/tlb.h > @@ -16,7 +16,7 @@ static void tlb_flush(struct mmu_gather *tlb); > static inline void tlb_flush(struct mmu_gather *tlb) > { > #ifdef CONFIG_MMU > - if (tlb->fullmm || tlb->need_flush_all) > + if (tlb->fullmm || tlb->need_flush_all || tlb->freed_tables) > flush_tlb_mm(tlb->mm); Why is it necessary to flush all TLB entries of the process? Thanks, Yunhui
Hi Yunhui, On Mon, Jan 29, 2024 at 4:07 AM yunhui cui <cuiyunhui@bytedance.com> wrote: > > Hi Alexandre, > > On Sun, Jan 28, 2024 at 8:04 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote: > > > > The riscv privileged specification mandates to flush the TLB whenever a > > page directory is modified, so add that to tlb_flush(). > > > > Fixes: c5e9b2c2ae82 ("riscv: Improve tlb_flush()") > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > > --- > > arch/riscv/include/asm/tlb.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/riscv/include/asm/tlb.h b/arch/riscv/include/asm/tlb.h > > index 1eb5682b2af6..50b63b5c15bd 100644 > > --- a/arch/riscv/include/asm/tlb.h > > +++ b/arch/riscv/include/asm/tlb.h > > @@ -16,7 +16,7 @@ static void tlb_flush(struct mmu_gather *tlb); > > static inline void tlb_flush(struct mmu_gather *tlb) > > { > > #ifdef CONFIG_MMU > > - if (tlb->fullmm || tlb->need_flush_all) > > + if (tlb->fullmm || tlb->need_flush_all || tlb->freed_tables) > > flush_tlb_mm(tlb->mm); > > Why is it necessary to flush all TLB entries of the process? That's quite heavy but that's the only to flush any "intermediate" cache that would cache non-leaf PTEs, the specification mentions this: "If software modifies a non-leaf PTE, it should execute SFENCE.VMA with rs1=x0. If any PTE along the traversal path had its G bit set, rs2 must be x0; otherwise, rs2 should be set to the ASID for which the translation is being modified." That was the behaviour before commit c5e9b2c2ae82 ("riscv: Improve tlb_flush()"). > > Thanks, > Yunhui
On Sun, Jan 28, 2024 at 01:04:05PM +0100, Alexandre Ghiti wrote: > The riscv privileged specification mandates to flush the TLB whenever a > page directory is modified, so add that to tlb_flush(). > > Fixes: c5e9b2c2ae82 ("riscv: Improve tlb_flush()") > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/include/asm/tlb.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/tlb.h b/arch/riscv/include/asm/tlb.h > index 1eb5682b2af6..50b63b5c15bd 100644 > --- a/arch/riscv/include/asm/tlb.h > +++ b/arch/riscv/include/asm/tlb.h > @@ -16,7 +16,7 @@ static void tlb_flush(struct mmu_gather *tlb); > static inline void tlb_flush(struct mmu_gather *tlb) > { > #ifdef CONFIG_MMU > - if (tlb->fullmm || tlb->need_flush_all) > + if (tlb->fullmm || tlb->need_flush_all || tlb->freed_tables) > flush_tlb_mm(tlb->mm); > else > flush_tlb_mm_range(tlb->mm, tlb->start, tlb->end, > -- > 2.39.2 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
On 2024-01-28 6:04 AM, Alexandre Ghiti wrote: > The riscv privileged specification mandates to flush the TLB whenever a > page directory is modified, so add that to tlb_flush(). > > Fixes: c5e9b2c2ae82 ("riscv: Improve tlb_flush()") > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/include/asm/tlb.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Samuel Holland <samuel.holland@sifive.com> Tested-by: Samuel Holland <samuel.holland@sifive.com>
Hello: This patch was applied to riscv/linux.git (fixes) by Palmer Dabbelt <palmer@rivosinc.com>: On Sun, 28 Jan 2024 13:04:05 +0100 you wrote: > The riscv privileged specification mandates to flush the TLB whenever a > page directory is modified, so add that to tlb_flush(). > > Fixes: c5e9b2c2ae82 ("riscv: Improve tlb_flush()") > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/include/asm/tlb.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Here is the summary with links: - [-fixes] riscv: Flush the tlb when a page directory is freed https://git.kernel.org/riscv/c/97cf301fa42e You are awesome, thank you!
diff --git a/arch/riscv/include/asm/tlb.h b/arch/riscv/include/asm/tlb.h index 1eb5682b2af6..50b63b5c15bd 100644 --- a/arch/riscv/include/asm/tlb.h +++ b/arch/riscv/include/asm/tlb.h @@ -16,7 +16,7 @@ static void tlb_flush(struct mmu_gather *tlb); static inline void tlb_flush(struct mmu_gather *tlb) { #ifdef CONFIG_MMU - if (tlb->fullmm || tlb->need_flush_all) + if (tlb->fullmm || tlb->need_flush_all || tlb->freed_tables) flush_tlb_mm(tlb->mm); else flush_tlb_mm_range(tlb->mm, tlb->start, tlb->end,
The riscv privileged specification mandates to flush the TLB whenever a page directory is modified, so add that to tlb_flush(). Fixes: c5e9b2c2ae82 ("riscv: Improve tlb_flush()") Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- arch/riscv/include/asm/tlb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)