Re: [PATCH] implement flush_cache_vmap for RISC-V
On Wednesday, April 14, 2021 7:03:13 AM UTC you wrote: > Hi, > > Le 4/12/21 à 3:08 AM, Jisheng Zhang a écrit : > > Hi Jiuyang, > > > > On Mon, 12 Apr 2021 00:05:30 + Jiuyang Liu wrote: > >> This patch implements flush_cache_vmap for RISC-V, since it modifies PTE. > >> Without this patch, SFENCE.VMA won't be added to related codes, which > >> might introduce a bug in the out-of-order micro-architecture > >> implementations. > >> > >> Signed-off-by: Jiuyang Liu > >> Reviewed-by: Alexandre Ghiti > >> Reviewed-by: Palmer Dabbelt > > > > IIRC, Palmer hasn't given this Reviewed-by tag. > > > >> --- > > > > Could you plz add version and changes? IIRC, this is the v3. > > > >> arch/riscv/include/asm/cacheflush.h | 6 ++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/arch/riscv/include/asm/cacheflush.h > >> b/arch/riscv/include/asm/cacheflush.h index 23ff70350992..3fd528badc35 > >> 100644 > >> --- a/arch/riscv/include/asm/cacheflush.h > >> +++ b/arch/riscv/include/asm/cacheflush.h > >> @@ -30,6 +30,12 @@ static inline void flush_dcache_page(struct page > >> *page) > >> > >> #define flush_icache_user_page(vma, pg, addr, len) \ > >> > >> flush_icache_mm(vma->vm_mm, 0) > >> > >> +/* > >> + * flush_cache_vmap is invoked after map_kernel_range() has installed > >> the page + * table entries, which modifies PTE, SFENCE.VMA should be > >> inserted.> > > Just my humble opinion, flush_cache_vmap() may not be necessary. > > vmalloc_fault can take care of this, and finally sfence.vma is inserted > > in related path. > I believe Palmer and Jisheng are right, my initial proposal to implement > flush_cache_vmap is wrong. > > But then, Jiuyang should not have noticed any problem here, so what's > wrong? @Jiuyang: Does implementing flush_cache_vmap fix your issue? Yes, I just checked this today, it did fix our issue. But I'm happy to rework on this, thanks for your help! This week I will try to trace wave of our core, and give a more detail email then. > And regarding flush_cache_vunmap, from Jisheng call stack, it seems also > not necessary. > > @Jiuyang: Can you tell us more about what you noticed? > > > Regards > > > >> + */ > >> +#define flush_cache_vmap(start, end) flush_tlb_all() > >> + > >> > >> #ifndef CONFIG_SMP > >> > >> #define flush_icache_all() local_flush_icache_all() > >> > >> -- > >> 2.31.1 > > > > ___ > > linux-riscv mailing list > > linux-ri...@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv Thanks, Jiuyang
Re: [PATCH] implement flush_cache_vmap for RISC-V
Hi, Le 4/12/21 à 3:08 AM, Jisheng Zhang a écrit : Hi Jiuyang, On Mon, 12 Apr 2021 00:05:30 + Jiuyang Liu wrote: This patch implements flush_cache_vmap for RISC-V, since it modifies PTE. Without this patch, SFENCE.VMA won't be added to related codes, which might introduce a bug in the out-of-order micro-architecture implementations. Signed-off-by: Jiuyang Liu Reviewed-by: Alexandre Ghiti Reviewed-by: Palmer Dabbelt IIRC, Palmer hasn't given this Reviewed-by tag. --- Could you plz add version and changes? IIRC, this is the v3. arch/riscv/include/asm/cacheflush.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h index 23ff70350992..3fd528badc35 100644 --- a/arch/riscv/include/asm/cacheflush.h +++ b/arch/riscv/include/asm/cacheflush.h @@ -30,6 +30,12 @@ static inline void flush_dcache_page(struct page *page) #define flush_icache_user_page(vma, pg, addr, len) \ flush_icache_mm(vma->vm_mm, 0) +/* + * flush_cache_vmap is invoked after map_kernel_range() has installed the page + * table entries, which modifies PTE, SFENCE.VMA should be inserted. Just my humble opinion, flush_cache_vmap() may not be necessary. vmalloc_fault can take care of this, and finally sfence.vma is inserted in related path. I believe Palmer and Jisheng are right, my initial proposal to implement flush_cache_vmap is wrong. But then, Jiuyang should not have noticed any problem here, so what's wrong? @Jiuyang: Does implementing flush_cache_vmap fix your issue? And regarding flush_cache_vunmap, from Jisheng call stack, it seems also not necessary. @Jiuyang: Can you tell us more about what you noticed? Regards + */ +#define flush_cache_vmap(start, end) flush_tlb_all() + #ifndef CONFIG_SMP #define flush_icache_all() local_flush_icache_all() -- 2.31.1 ___ linux-riscv mailing list linux-ri...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH] implement flush_cache_vmap for RISC-V
Hi Jiuyang, On Mon, 12 Apr 2021 00:05:30 + Jiuyang Liu wrote: > > This patch implements flush_cache_vmap for RISC-V, since it modifies PTE. > Without this patch, SFENCE.VMA won't be added to related codes, which > might introduce a bug in the out-of-order micro-architecture > implementations. > > Signed-off-by: Jiuyang Liu > Reviewed-by: Alexandre Ghiti > Reviewed-by: Palmer Dabbelt IIRC, Palmer hasn't given this Reviewed-by tag. > --- Could you plz add version and changes? IIRC, this is the v3. > arch/riscv/include/asm/cacheflush.h | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/riscv/include/asm/cacheflush.h > b/arch/riscv/include/asm/cacheflush.h > index 23ff70350992..3fd528badc35 100644 > --- a/arch/riscv/include/asm/cacheflush.h > +++ b/arch/riscv/include/asm/cacheflush.h > @@ -30,6 +30,12 @@ static inline void flush_dcache_page(struct page *page) > #define flush_icache_user_page(vma, pg, addr, len) \ > flush_icache_mm(vma->vm_mm, 0) > > +/* > + * flush_cache_vmap is invoked after map_kernel_range() has installed the > page > + * table entries, which modifies PTE, SFENCE.VMA should be inserted. Just my humble opinion, flush_cache_vmap() may not be necessary. vmalloc_fault can take care of this, and finally sfence.vma is inserted in related path. Regards > + */ > +#define flush_cache_vmap(start, end) flush_tlb_all() > + > #ifndef CONFIG_SMP > > #define flush_icache_all() local_flush_icache_all() > -- > 2.31.1
