* [PATCH] implement flush_cache_vmap for RISC-V
@ 2021-04-12 0:05 ` Jiuyang Liu
0 siblings, 0 replies; 10+ messages in thread
From: Jiuyang Liu @ 2021-04-12 0:05 UTC (permalink / raw)
To: Alex Ghiti, Palmer Dabbelt
Cc: Jiuyang Liu, Andrew Waterman, Paul Walmsley, Albert Ou,
Andrew Morton, Geert Uytterhoeven, linux-riscv, linux-kernel
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 <liu@jiuyang.me>
Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
---
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.
+ */
+#define flush_cache_vmap(start, end) flush_tlb_all()
+
#ifndef CONFIG_SMP
#define flush_icache_all() local_flush_icache_all()
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] implement flush_cache_vmap for RISC-V
@ 2021-04-12 0:05 ` Jiuyang Liu
0 siblings, 0 replies; 10+ messages in thread
From: Jiuyang Liu @ 2021-04-12 0:05 UTC (permalink / raw)
To: Alex Ghiti, Palmer Dabbelt
Cc: Jiuyang Liu, Andrew Waterman, Paul Walmsley, Albert Ou,
Andrew Morton, Geert Uytterhoeven, linux-riscv, linux-kernel
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 <liu@jiuyang.me>
Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
---
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.
+ */
+#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-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] implement flush_cache_vmap for RISC-V
2021-04-12 0:05 ` Jiuyang Liu
@ 2021-04-12 7:08 ` Jisheng Zhang
-1 siblings, 0 replies; 10+ messages in thread
From: Jisheng Zhang @ 2021-04-12 7:08 UTC (permalink / raw)
To: Jiuyang Liu
Cc: Alex Ghiti, Palmer Dabbelt, Andrew Waterman, Paul Walmsley,
Albert Ou, Andrew Morton, Geert Uytterhoeven, linux-riscv,
linux-kernel
Hi Jiuyang,
On Mon, 12 Apr 2021 00:05:30 +0000 Jiuyang Liu <liu@jiuyang.me> 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 <liu@jiuyang.me>
> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
> Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] implement flush_cache_vmap for RISC-V
@ 2021-04-12 7:08 ` Jisheng Zhang
0 siblings, 0 replies; 10+ messages in thread
From: Jisheng Zhang @ 2021-04-12 7:08 UTC (permalink / raw)
To: Jiuyang Liu
Cc: Alex Ghiti, Palmer Dabbelt, Andrew Waterman, Paul Walmsley,
Albert Ou, Andrew Morton, Geert Uytterhoeven, linux-riscv,
linux-kernel
Hi Jiuyang,
On Mon, 12 Apr 2021 00:05:30 +0000 Jiuyang Liu <liu@jiuyang.me> 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 <liu@jiuyang.me>
> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
> Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] implement flush_cache_vmap for RISC-V
2021-04-12 7:08 ` Jisheng Zhang
@ 2021-04-14 7:03 ` Alex Ghiti
-1 siblings, 0 replies; 10+ messages in thread
From: Alex Ghiti @ 2021-04-14 7:03 UTC (permalink / raw)
To: Jisheng Zhang, Jiuyang Liu
Cc: Palmer Dabbelt, Andrew Waterman, Paul Walmsley, Albert Ou,
Andrew Morton, Geert Uytterhoeven, linux-riscv, linux-kernel
Hi,
Le 4/12/21 à 3:08 AM, Jisheng Zhang a écrit :
> Hi Jiuyang,
>
> On Mon, 12 Apr 2021 00:05:30 +0000 Jiuyang Liu <liu@jiuyang.me> 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 <liu@jiuyang.me>
>> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
>> Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
>
> 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-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] implement flush_cache_vmap for RISC-V
@ 2021-04-14 7:03 ` Alex Ghiti
0 siblings, 0 replies; 10+ messages in thread
From: Alex Ghiti @ 2021-04-14 7:03 UTC (permalink / raw)
To: Jisheng Zhang, Jiuyang Liu
Cc: Palmer Dabbelt, Andrew Waterman, Paul Walmsley, Albert Ou,
Andrew Morton, Geert Uytterhoeven, linux-riscv, linux-kernel
Hi,
Le 4/12/21 à 3:08 AM, Jisheng Zhang a écrit :
> Hi Jiuyang,
>
> On Mon, 12 Apr 2021 00:05:30 +0000 Jiuyang Liu <liu@jiuyang.me> 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 <liu@jiuyang.me>
>> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
>> Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
>
> 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-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] implement flush_cache_vmap for RISC-V
2021-04-14 7:03 ` Alex Ghiti
@ 2021-04-14 9:16 ` Jiuyang Liu
-1 siblings, 0 replies; 10+ messages in thread
From: Jiuyang Liu @ 2021-04-14 9:16 UTC (permalink / raw)
To: Jisheng Zhang, Alex Ghiti
Cc: Palmer Dabbelt, Andrew Waterman, Paul Walmsley, Albert Ou,
Andrew Morton, Geert Uytterhoeven, linux-riscv, linux-kernel
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 +0000 Jiuyang Liu <liu@jiuyang.me> 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 <liu@jiuyang.me>
> >> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
> >> Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
> >
> > 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-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
Thanks,
Jiuyang
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] implement flush_cache_vmap for RISC-V
@ 2021-04-14 9:16 ` Jiuyang Liu
0 siblings, 0 replies; 10+ messages in thread
From: Jiuyang Liu @ 2021-04-14 9:16 UTC (permalink / raw)
To: Jisheng Zhang, Alex Ghiti
Cc: Palmer Dabbelt, Andrew Waterman, Paul Walmsley, Albert Ou,
Andrew Morton, Geert Uytterhoeven, linux-riscv, linux-kernel
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 +0000 Jiuyang Liu <liu@jiuyang.me> 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 <liu@jiuyang.me>
> >> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
> >> Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
> >
> > 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-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
Thanks,
Jiuyang
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] implement flush_cache_vmap for RISC-V
2021-04-14 7:03 ` Alex Ghiti
@ 2021-04-23 4:39 ` Palmer Dabbelt
-1 siblings, 0 replies; 10+ messages in thread
From: Palmer Dabbelt @ 2021-04-23 4:39 UTC (permalink / raw)
To: alex
Cc: Jisheng.Zhang, liu, waterman, Paul Walmsley, aou, akpm, geert,
linux-riscv, linux-kernel
On Wed, 14 Apr 2021 00:03:13 PDT (-0700), alex@ghiti.fr wrote:
> Hi,
>
> Le 4/12/21 à 3:08 AM, Jisheng Zhang a écrit :
>> Hi Jiuyang,
>>
>> On Mon, 12 Apr 2021 00:05:30 +0000 Jiuyang Liu <liu@jiuyang.me> 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 <liu@jiuyang.me>
>>> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
>>> Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
>>
>> IIRC, Palmer hasn't given this Reviewed-by tag.
Yes. In fact, I gave the opposite of a RB: we shouldn't have this, at
least without some demonstration of a meaningful performance improvement
and likely with a host of other changes to change the whole port over to
avoid relying on traps to handle new mappings. I really don't think
that's a sane way to go, as the theory is that reasonable
microarchitectures won't have big windows over which these faults can
occur so there won't be that many of them. If it ends up being an issue
on real hardware we can try and sort something out, but it's going to be
a lot of work as we'll need to avoid hurting performance on
implementations that don't make invalid mappings visible often.
>>
>>> ---
>>
>> 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.
FWIW: I still think that flush_cache_vunmap() is necessary -- we don't
have any other way to guarantee that mapping isn't visible. Implementing
flush_cache_vmap() could work around the real bug of lacking
flush_cache_vunmap(), as we'd see stale mappings.
That said, it could just be covering up some other bug. Wouldn't be
surprised if it's a bug in our port, but this is the sort of thing that
could also be a hardware bug of some sort.
>
> @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-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] implement flush_cache_vmap for RISC-V
@ 2021-04-23 4:39 ` Palmer Dabbelt
0 siblings, 0 replies; 10+ messages in thread
From: Palmer Dabbelt @ 2021-04-23 4:39 UTC (permalink / raw)
To: alex
Cc: Jisheng.Zhang, liu, waterman, Paul Walmsley, aou, akpm, geert,
linux-riscv, linux-kernel
On Wed, 14 Apr 2021 00:03:13 PDT (-0700), alex@ghiti.fr wrote:
> Hi,
>
> Le 4/12/21 à 3:08 AM, Jisheng Zhang a écrit :
>> Hi Jiuyang,
>>
>> On Mon, 12 Apr 2021 00:05:30 +0000 Jiuyang Liu <liu@jiuyang.me> 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 <liu@jiuyang.me>
>>> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
>>> Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
>>
>> IIRC, Palmer hasn't given this Reviewed-by tag.
Yes. In fact, I gave the opposite of a RB: we shouldn't have this, at
least without some demonstration of a meaningful performance improvement
and likely with a host of other changes to change the whole port over to
avoid relying on traps to handle new mappings. I really don't think
that's a sane way to go, as the theory is that reasonable
microarchitectures won't have big windows over which these faults can
occur so there won't be that many of them. If it ends up being an issue
on real hardware we can try and sort something out, but it's going to be
a lot of work as we'll need to avoid hurting performance on
implementations that don't make invalid mappings visible often.
>>
>>> ---
>>
>> 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.
FWIW: I still think that flush_cache_vunmap() is necessary -- we don't
have any other way to guarantee that mapping isn't visible. Implementing
flush_cache_vmap() could work around the real bug of lacking
flush_cache_vunmap(), as we'd see stale mappings.
That said, it could just be covering up some other bug. Wouldn't be
surprised if it's a bug in our port, but this is the sort of thing that
could also be a hardware bug of some sort.
>
> @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-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-04-23 4:39 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 0:05 [PATCH] implement flush_cache_vmap for RISC-V Jiuyang Liu
2021-04-12 0:05 ` Jiuyang Liu
2021-04-12 7:08 ` Jisheng Zhang
2021-04-12 7:08 ` Jisheng Zhang
2021-04-14 7:03 ` Alex Ghiti
2021-04-14 7:03 ` Alex Ghiti
2021-04-14 9:16 ` Jiuyang Liu
2021-04-14 9:16 ` Jiuyang Liu
2021-04-23 4:39 ` Palmer Dabbelt
2021-04-23 4:39 ` Palmer Dabbelt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.