linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
@ 2024-01-23 13:27 Alexandre Ghiti
  2024-01-24  2:44 ` [External] " yunhui cui
  2024-01-29  9:01 ` Dennis Zhou
  0 siblings, 2 replies; 11+ messages in thread
From: Alexandre Ghiti @ 2024-01-23 13:27 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Geert Uytterhoeven,
	Alexandre Ghiti, Dennis Zhou, linux-riscv, linux-kernel

local_flush_tlb_range_asid() takes the size as argument, not the end of
the range to flush, so fix this by computing the size from the end and
the start of the range.

Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 arch/riscv/mm/tlbflush.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index 8d12b26f5ac3..9619965f6501 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -68,7 +68,7 @@ static inline void local_flush_tlb_range_asid(unsigned long start,
 
 void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
 {
-	local_flush_tlb_range_asid(start, end, PAGE_SIZE, FLUSH_TLB_NO_ASID);
+	local_flush_tlb_range_asid(start, end - start, PAGE_SIZE, FLUSH_TLB_NO_ASID);
 }
 
 static void __ipi_flush_tlb_all(void *info)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [External] [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
  2024-01-23 13:27 [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid() Alexandre Ghiti
@ 2024-01-24  2:44 ` yunhui cui
  2024-01-24  8:19   ` Dennis Zhou
  2024-01-29  9:01 ` Dennis Zhou
  1 sibling, 1 reply; 11+ messages in thread
From: yunhui cui @ 2024-01-24  2:44 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Geert Uytterhoeven,
	Dennis Zhou, linux-riscv, linux-kernel

Hi Alexandre,

On Tue, Jan 23, 2024 at 9:31 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> local_flush_tlb_range_asid() takes the size as argument, not the end of
> the range to flush, so fix this by computing the size from the end and
> the start of the range.
>
> Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  arch/riscv/mm/tlbflush.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index 8d12b26f5ac3..9619965f6501 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -68,7 +68,7 @@ static inline void local_flush_tlb_range_asid(unsigned long start,
>
>  void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
>  {
> -       local_flush_tlb_range_asid(start, end, PAGE_SIZE, FLUSH_TLB_NO_ASID);
> +       local_flush_tlb_range_asid(start, end - start, PAGE_SIZE, FLUSH_TLB_NO_ASID);
>  }

What makes me curious is that this patch has not been tested?
BTW, It is best to keep the parameter order of all functions in
tlbflush.c consistent: cpumask, start, size, stride, asid.

Thanks,
Yunhui

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [External] [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
  2024-01-24  2:44 ` [External] " yunhui cui
@ 2024-01-24  8:19   ` Dennis Zhou
  2024-01-24  8:38     ` Alexandre Ghiti
  0 siblings, 1 reply; 11+ messages in thread
From: Dennis Zhou @ 2024-01-24  8:19 UTC (permalink / raw)
  To: yunhui cui, Alexandre Ghiti
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Geert Uytterhoeven,
	linux-riscv, linux-kernel

Hello,

On Wed, Jan 24, 2024 at 10:44:12AM +0800, yunhui cui wrote:
> Hi Alexandre,
> 
> On Tue, Jan 23, 2024 at 9:31 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> >
> > local_flush_tlb_range_asid() takes the size as argument, not the end of
> > the range to flush, so fix this by computing the size from the end and
> > the start of the range.
> >
> > Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > ---
> >  arch/riscv/mm/tlbflush.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> > index 8d12b26f5ac3..9619965f6501 100644
> > --- a/arch/riscv/mm/tlbflush.c
> > +++ b/arch/riscv/mm/tlbflush.c
> > @@ -68,7 +68,7 @@ static inline void local_flush_tlb_range_asid(unsigned long start,
> >
> >  void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
> >  {
> > -       local_flush_tlb_range_asid(start, end, PAGE_SIZE, FLUSH_TLB_NO_ASID);
> > +       local_flush_tlb_range_asid(start, end - start, PAGE_SIZE, FLUSH_TLB_NO_ASID);
> >  }
> 

Well this was a miss during code review.. I'm going to take another look
tomorrow and then likely pull this into a fixes branch.

> What makes me curious is that this patch has not been tested?
> BTW, It is best to keep the parameter order of all functions in
> tlbflush.c consistent: cpumask, start, size, stride, asid.
> 

I can't speak to the riscv communities testing/regression suites, but
this would only be caught in a performance regression test.

That being said, Alexandre, can you please lmk what level of testing
this has gone through?

Thanks,
Dennis

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [External] [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
  2024-01-24  8:19   ` Dennis Zhou
@ 2024-01-24  8:38     ` Alexandre Ghiti
  2024-01-24  8:41       ` Alexandre Ghiti
  0 siblings, 1 reply; 11+ messages in thread
From: Alexandre Ghiti @ 2024-01-24  8:38 UTC (permalink / raw)
  To: Dennis Zhou, yunhui cui, Alexandre Ghiti
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Geert Uytterhoeven,
	linux-riscv, linux-kernel

Hi Dennis, Yunhui,

On 24/01/2024 09:19, Dennis Zhou wrote:
> Hello,
>
> On Wed, Jan 24, 2024 at 10:44:12AM +0800, yunhui cui wrote:
>> Hi Alexandre,
>>
>> On Tue, Jan 23, 2024 at 9:31 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>>> local_flush_tlb_range_asid() takes the size as argument, not the end of
>>> the range to flush, so fix this by computing the size from the end and
>>> the start of the range.
>>>
>>> Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
>>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>>> ---
>>>   arch/riscv/mm/tlbflush.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
>>> index 8d12b26f5ac3..9619965f6501 100644
>>> --- a/arch/riscv/mm/tlbflush.c
>>> +++ b/arch/riscv/mm/tlbflush.c
>>> @@ -68,7 +68,7 @@ static inline void local_flush_tlb_range_asid(unsigned long start,
>>>
>>>   void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
>>>   {
>>> -       local_flush_tlb_range_asid(start, end, PAGE_SIZE, FLUSH_TLB_NO_ASID);
>>> +       local_flush_tlb_range_asid(start, end - start, PAGE_SIZE, FLUSH_TLB_NO_ASID);
>>>   }
> Well this was a miss during code review.. I'm going to take another look
> tomorrow and then likely pull this into a fixes branch.
>
>> What makes me curious is that this patch has not been tested?
>> BTW, It is best to keep the parameter order of all functions in
>> tlbflush.c consistent: cpumask, start, size, stride, asid.
>>
> I can't speak to the riscv communities testing/regression suites, but
> this would only be caught in a performance regression test.
>
> That being said, Alexandre, can you please lmk what level of testing
> this has gone through?


All my patches go through the same level of testing:

* Build/boot an Ubuntu kernel with and without KASAN + a few simple 
testsuites (libhugetlbfs, riscv kselftests and other)
* Build/boot a simple rootfs on ~40 different rv64 configs
* Build/boot a simple rootfs on ~30 different rv32 configs

And I run LTP/full kselftests/perf testsuite on a weekly basis on every 
rc. All this validation is done on qemu.

The patch is functional, it "simply" flushes the whole TLB instead of a 
few entries, so the only way to catch that would have been a performance 
regression. But given it only runs on qemu, it would have been hard to 
catch any performance regression since that involves the TLB.

@Yunhui: Please let me know how I should validate my patches better.

Thanks,

Alex


> Thanks,
> Dennis
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [External] [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
  2024-01-24  8:38     ` Alexandre Ghiti
@ 2024-01-24  8:41       ` Alexandre Ghiti
  2024-01-29  1:43         ` yunhui cui
  0 siblings, 1 reply; 11+ messages in thread
From: Alexandre Ghiti @ 2024-01-24  8:41 UTC (permalink / raw)
  To: Dennis Zhou, yunhui cui, Alexandre Ghiti
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Geert Uytterhoeven,
	linux-riscv, linux-kernel

On 24/01/2024 09:38, Alexandre Ghiti wrote:
> Hi Dennis, Yunhui,
>
> On 24/01/2024 09:19, Dennis Zhou wrote:
>> Hello,
>>
>> On Wed, Jan 24, 2024 at 10:44:12AM +0800, yunhui cui wrote:
>>> Hi Alexandre,
>>>
>>> On Tue, Jan 23, 2024 at 9:31 PM Alexandre Ghiti 
>>> <alexghiti@rivosinc.com> wrote:
>>>> local_flush_tlb_range_asid() takes the size as argument, not the 
>>>> end of
>>>> the range to flush, so fix this by computing the size from the end and
>>>> the start of the range.
>>>>
>>>> Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
>>>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>>>> ---
>>>>   arch/riscv/mm/tlbflush.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
>>>> index 8d12b26f5ac3..9619965f6501 100644
>>>> --- a/arch/riscv/mm/tlbflush.c
>>>> +++ b/arch/riscv/mm/tlbflush.c
>>>> @@ -68,7 +68,7 @@ static inline void 
>>>> local_flush_tlb_range_asid(unsigned long start,
>>>>
>>>>   void local_flush_tlb_kernel_range(unsigned long start, unsigned 
>>>> long end)
>>>>   {
>>>> -       local_flush_tlb_range_asid(start, end, PAGE_SIZE, 
>>>> FLUSH_TLB_NO_ASID);
>>>> +       local_flush_tlb_range_asid(start, end - start, PAGE_SIZE, 
>>>> FLUSH_TLB_NO_ASID);
>>>>   }
>> Well this was a miss during code review.. I'm going to take another look
>> tomorrow and then likely pull this into a fixes branch.
>>
>>> What makes me curious is that this patch has not been tested?
>>> BTW, It is best to keep the parameter order of all functions in
>>> tlbflush.c consistent: cpumask, start, size, stride, asid.
>>>
>> I can't speak to the riscv communities testing/regression suites, but
>> this would only be caught in a performance regression test.
>>
>> That being said, Alexandre, can you please lmk what level of testing
>> this has gone through?
>
>
> All my patches go through the same level of testing:
>
> * Build/boot an Ubuntu kernel with and without KASAN + a few simple 
> testsuites (libhugetlbfs, riscv kselftests and other)
> * Build/boot a simple rootfs on ~40 different rv64 configs
> * Build/boot a simple rootfs on ~30 different rv32 configs
>
> And I run LTP/full kselftests/perf testsuite on a weekly basis on 
> every rc. All this validation is done on qemu.
>
> The patch is functional, it "simply" flushes the whole TLB instead of 
> a few entries, so the only way to catch that would have been a 
> performance regression. But given it only runs on qemu, it would have 
> been hard to catch any performance regression since that involves the 
> TLB.
>
> @Yunhui: Please let me know how I should validate my patches better.


@Yunhui: And BTW, we lack reviewers, so feel free to help ;)


>
> Thanks,
>
> Alex
>
>
>> Thanks,
>> Dennis
>>
>> _______________________________________________
>> 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] 11+ messages in thread

* Re: [External] [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
  2024-01-24  8:41       ` Alexandre Ghiti
@ 2024-01-29  1:43         ` yunhui cui
  0 siblings, 0 replies; 11+ messages in thread
From: yunhui cui @ 2024-01-29  1:43 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: Dennis Zhou, Alexandre Ghiti, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Geert Uytterhoeven, linux-riscv, linux-kernel

Hi Alexandre,

On Wed, Jan 24, 2024 at 4:41 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> On 24/01/2024 09:38, Alexandre Ghiti wrote:
> > Hi Dennis, Yunhui,
> >
> > On 24/01/2024 09:19, Dennis Zhou wrote:
> >> Hello,
> >>
> >> On Wed, Jan 24, 2024 at 10:44:12AM +0800, yunhui cui wrote:
> >>> Hi Alexandre,
> >>>
> >>> On Tue, Jan 23, 2024 at 9:31 PM Alexandre Ghiti
> >>> <alexghiti@rivosinc.com> wrote:
> >>>> local_flush_tlb_range_asid() takes the size as argument, not the
> >>>> end of
> >>>> the range to flush, so fix this by computing the size from the end and
> >>>> the start of the range.
> >>>>
> >>>> Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
> >>>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> >>>> ---
> >>>>   arch/riscv/mm/tlbflush.c | 2 +-
> >>>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> >>>> index 8d12b26f5ac3..9619965f6501 100644
> >>>> --- a/arch/riscv/mm/tlbflush.c
> >>>> +++ b/arch/riscv/mm/tlbflush.c
> >>>> @@ -68,7 +68,7 @@ static inline void
> >>>> local_flush_tlb_range_asid(unsigned long start,
> >>>>
> >>>>   void local_flush_tlb_kernel_range(unsigned long start, unsigned
> >>>> long end)
> >>>>   {
> >>>> -       local_flush_tlb_range_asid(start, end, PAGE_SIZE,
> >>>> FLUSH_TLB_NO_ASID);
> >>>> +       local_flush_tlb_range_asid(start, end - start, PAGE_SIZE,
> >>>> FLUSH_TLB_NO_ASID);
> >>>>   }
> >> Well this was a miss during code review.. I'm going to take another look
> >> tomorrow and then likely pull this into a fixes branch.
> >>
> >>> What makes me curious is that this patch has not been tested?
> >>> BTW, It is best to keep the parameter order of all functions in
> >>> tlbflush.c consistent: cpumask, start, size, stride, asid.
> >>>
> >> I can't speak to the riscv communities testing/regression suites, but
> >> this would only be caught in a performance regression test.
> >>
> >> That being said, Alexandre, can you please lmk what level of testing
> >> this has gone through?
> >
> >
> > All my patches go through the same level of testing:
> >
> > * Build/boot an Ubuntu kernel with and without KASAN + a few simple
> > testsuites (libhugetlbfs, riscv kselftests and other)
> > * Build/boot a simple rootfs on ~40 different rv64 configs
> > * Build/boot a simple rootfs on ~30 different rv32 configs
> >
> > And I run LTP/full kselftests/perf testsuite on a weekly basis on
> > every rc. All this validation is done on qemu.
> >
> > The patch is functional, it "simply" flushes the whole TLB instead of
> > a few entries, so the only way to catch that would have been a
> > performance regression. But given it only runs on qemu, it would have
> > been hard to catch any performance regression since that involves the
> > TLB.
> >
> > @Yunhui: Please let me know how I should validate my patches better.
>
>
> @Yunhui: And BTW, we lack reviewers, so feel free to help ;)

Okay, if you don’t mind, I will also review the RISC-V TLB related
patches later.
BTW, I mailed a patch "RISC-V: add uniprocessor flush_tlb_range()
support", and please help me review it, thank you ~

Thanks,
Yunhui

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
  2024-01-23 13:27 [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid() Alexandre Ghiti
  2024-01-24  2:44 ` [External] " yunhui cui
@ 2024-01-29  9:01 ` Dennis Zhou
  2024-01-29  9:04   ` Alexandre Ghiti
  2024-01-31 20:34   ` Palmer Dabbelt
  1 sibling, 2 replies; 11+ messages in thread
From: Dennis Zhou @ 2024-01-29  9:01 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Geert Uytterhoeven,
	linux-riscv, linux-kernel

Hi Alexandre,

On Tue, Jan 23, 2024 at 02:27:30PM +0100, Alexandre Ghiti wrote:
> local_flush_tlb_range_asid() takes the size as argument, not the end of
> the range to flush, so fix this by computing the size from the end and
> the start of the range.
> 
> Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  arch/riscv/mm/tlbflush.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index 8d12b26f5ac3..9619965f6501 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -68,7 +68,7 @@ static inline void local_flush_tlb_range_asid(unsigned long start,
>  
>  void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
>  {
> -	local_flush_tlb_range_asid(start, end, PAGE_SIZE, FLUSH_TLB_NO_ASID);
> +	local_flush_tlb_range_asid(start, end - start, PAGE_SIZE, FLUSH_TLB_NO_ASID);
>  }
>  
>  static void __ipi_flush_tlb_all(void *info)
> -- 
> 2.39.2
> 

Sorry for the delay, I just pulled this into percpu#for-6.8-fixes. I'll
send it to Linus this week.

Thanks,
Dennis

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
  2024-01-29  9:01 ` Dennis Zhou
@ 2024-01-29  9:04   ` Alexandre Ghiti
  2024-01-31 20:34   ` Palmer Dabbelt
  1 sibling, 0 replies; 11+ messages in thread
From: Alexandre Ghiti @ 2024-01-29  9:04 UTC (permalink / raw)
  To: Dennis Zhou
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Geert Uytterhoeven,
	linux-riscv, linux-kernel

On Mon, Jan 29, 2024 at 10:01 AM Dennis Zhou <dennis@kernel.org> wrote:
>
> Hi Alexandre,
>
> On Tue, Jan 23, 2024 at 02:27:30PM +0100, Alexandre Ghiti wrote:
> > local_flush_tlb_range_asid() takes the size as argument, not the end of
> > the range to flush, so fix this by computing the size from the end and
> > the start of the range.
> >
> > Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > ---
> >  arch/riscv/mm/tlbflush.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> > index 8d12b26f5ac3..9619965f6501 100644
> > --- a/arch/riscv/mm/tlbflush.c
> > +++ b/arch/riscv/mm/tlbflush.c
> > @@ -68,7 +68,7 @@ static inline void local_flush_tlb_range_asid(unsigned long start,
> >
> >  void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
> >  {
> > -     local_flush_tlb_range_asid(start, end, PAGE_SIZE, FLUSH_TLB_NO_ASID);
> > +     local_flush_tlb_range_asid(start, end - start, PAGE_SIZE, FLUSH_TLB_NO_ASID);
> >  }
> >
> >  static void __ipi_flush_tlb_all(void *info)
> > --
> > 2.39.2
> >
>
> Sorry for the delay, I just pulled this into percpu#for-6.8-fixes. I'll
> send it to Linus this week.
>
> Thanks,
> Dennis

No problem, thanks!

Alex

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
  2024-01-29  9:01 ` Dennis Zhou
  2024-01-29  9:04   ` Alexandre Ghiti
@ 2024-01-31 20:34   ` Palmer Dabbelt
  2024-02-01  3:45     ` Dennis Zhou
  1 sibling, 1 reply; 11+ messages in thread
From: Palmer Dabbelt @ 2024-01-31 20:34 UTC (permalink / raw)
  To: dennis; +Cc: alexghiti, Paul Walmsley, aou, geert, linux-riscv, linux-kernel

On Mon, 29 Jan 2024 01:01:00 PST (-0800), dennis@kernel.org wrote:
> Hi Alexandre,
>
> On Tue, Jan 23, 2024 at 02:27:30PM +0100, Alexandre Ghiti wrote:
>> local_flush_tlb_range_asid() takes the size as argument, not the end of
>> the range to flush, so fix this by computing the size from the end and
>> the start of the range.
>>
>> Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>> ---
>>  arch/riscv/mm/tlbflush.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
>> index 8d12b26f5ac3..9619965f6501 100644
>> --- a/arch/riscv/mm/tlbflush.c
>> +++ b/arch/riscv/mm/tlbflush.c
>> @@ -68,7 +68,7 @@ static inline void local_flush_tlb_range_asid(unsigned long start,
>>
>>  void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
>>  {
>> -	local_flush_tlb_range_asid(start, end, PAGE_SIZE, FLUSH_TLB_NO_ASID);
>> +	local_flush_tlb_range_asid(start, end - start, PAGE_SIZE, FLUSH_TLB_NO_ASID);
>>  }
>>
>>  static void __ipi_flush_tlb_all(void *info)
>> --
>> 2.39.2
>>
>
> Sorry for the delay, I just pulled this into percpu#for-6.8-fixes. I'll
> send it to Linus this week.

Do you mind if we do a shared tag or something?  It's going to conflict 
with 
https://lore.kernel.org/all/20240117140333.2479667-1-vincent.chen@sifive.com/ 
.  No big deal as it's a pretty trivial conflict, but they'll both need 
stable backports.

>
> Thanks,
> Dennis

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
  2024-01-31 20:34   ` Palmer Dabbelt
@ 2024-02-01  3:45     ` Dennis Zhou
  2024-02-09 16:41       ` Palmer Dabbelt
  0 siblings, 1 reply; 11+ messages in thread
From: Dennis Zhou @ 2024-02-01  3:45 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: alexghiti, Paul Walmsley, aou, geert, linux-riscv, linux-kernel

Hi Palmer,

On Wed, Jan 31, 2024 at 12:34:40PM -0800, Palmer Dabbelt wrote:
> On Mon, 29 Jan 2024 01:01:00 PST (-0800), dennis@kernel.org wrote:
> > Hi Alexandre,
> > 
> > On Tue, Jan 23, 2024 at 02:27:30PM +0100, Alexandre Ghiti wrote:
> > > local_flush_tlb_range_asid() takes the size as argument, not the end of
> > > the range to flush, so fix this by computing the size from the end and
> > > the start of the range.
> > > 
> > > Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
> > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > > ---
> > >  arch/riscv/mm/tlbflush.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> > > index 8d12b26f5ac3..9619965f6501 100644
> > > --- a/arch/riscv/mm/tlbflush.c
> > > +++ b/arch/riscv/mm/tlbflush.c
> > > @@ -68,7 +68,7 @@ static inline void local_flush_tlb_range_asid(unsigned long start,
> > > 
> > >  void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
> > >  {
> > > -	local_flush_tlb_range_asid(start, end, PAGE_SIZE, FLUSH_TLB_NO_ASID);
> > > +	local_flush_tlb_range_asid(start, end - start, PAGE_SIZE, FLUSH_TLB_NO_ASID);
> > >  }
> > > 
> > >  static void __ipi_flush_tlb_all(void *info)
> > > --
> > > 2.39.2
> > > 
> > 
> > Sorry for the delay, I just pulled this into percpu#for-6.8-fixes. I'll
> > send it to Linus this week.
> 
> Do you mind if we do a shared tag or something?  It's going to conflict with
> https://lore.kernel.org/all/20240117140333.2479667-1-vincent.chen@sifive.com/
> .  No big deal as it's a pretty trivial conflict, but they'll both need
> stable backports.

This alone won't need a stable backport, I merged the bug as part of
enabling the percpu page allocator in the recent 6.8 merge window.

That being said, this is the only patch I'm carrying for v6.8. I'm happy
to drop it and have you pick it up instead. Saves me a tag and a PR.
Lmk if that works for you.

Thanks,
Dennis

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid()
  2024-02-01  3:45     ` Dennis Zhou
@ 2024-02-09 16:41       ` Palmer Dabbelt
  0 siblings, 0 replies; 11+ messages in thread
From: Palmer Dabbelt @ 2024-02-09 16:41 UTC (permalink / raw)
  To: dennis; +Cc: alexghiti, Paul Walmsley, aou, geert, linux-riscv, linux-kernel

On Wed, 31 Jan 2024 19:45:27 PST (-0800), dennis@kernel.org wrote:
> Hi Palmer,
>
> On Wed, Jan 31, 2024 at 12:34:40PM -0800, Palmer Dabbelt wrote:
>> On Mon, 29 Jan 2024 01:01:00 PST (-0800), dennis@kernel.org wrote:
>> > Hi Alexandre,
>> >
>> > On Tue, Jan 23, 2024 at 02:27:30PM +0100, Alexandre Ghiti wrote:
>> > > local_flush_tlb_range_asid() takes the size as argument, not the end of
>> > > the range to flush, so fix this by computing the size from the end and
>> > > the start of the range.
>> > >
>> > > Fixes: 7a92fc8b4d20 ("mm: Introduce flush_cache_vmap_early()")
>> > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>> > > ---
>> > >  arch/riscv/mm/tlbflush.c | 2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
>> > > index 8d12b26f5ac3..9619965f6501 100644
>> > > --- a/arch/riscv/mm/tlbflush.c
>> > > +++ b/arch/riscv/mm/tlbflush.c
>> > > @@ -68,7 +68,7 @@ static inline void local_flush_tlb_range_asid(unsigned long start,
>> > >
>> > >  void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
>> > >  {
>> > > -	local_flush_tlb_range_asid(start, end, PAGE_SIZE, FLUSH_TLB_NO_ASID);
>> > > +	local_flush_tlb_range_asid(start, end - start, PAGE_SIZE, FLUSH_TLB_NO_ASID);
>> > >  }
>> > >
>> > >  static void __ipi_flush_tlb_all(void *info)
>> > > --
>> > > 2.39.2
>> > >
>> >
>> > Sorry for the delay, I just pulled this into percpu#for-6.8-fixes. I'll
>> > send it to Linus this week.
>>
>> Do you mind if we do a shared tag or something?  It's going to conflict with
>> https://lore.kernel.org/all/20240117140333.2479667-1-vincent.chen@sifive.com/
>> .  No big deal as it's a pretty trivial conflict, but they'll both need
>> stable backports.
>
> This alone won't need a stable backport, I merged the bug as part of
> enabling the percpu page allocator in the recent 6.8 merge window.
>
> That being said, this is the only patch I'm carrying for v6.8. I'm happy
> to drop it and have you pick it up instead. Saves me a tag and a PR.
> Lmk if that works for you.

Sorry I missed this, but that seems like the easiest way to go here -- 
the other patch fixes the same bug (and another one), so I think we're 
safe with just that (which I'm sending to Linus as part of my rc4 fixes 
now-ish, as I'd need to anyway).  It's d9807d60c145 ("riscv: mm: execute 
local TLB flush after populating vmemmap"), in case anyone's looking 
later...

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-02-09 16:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-23 13:27 [PATCH] riscv: Fix wrong size passed to local_flush_tlb_range_asid() Alexandre Ghiti
2024-01-24  2:44 ` [External] " yunhui cui
2024-01-24  8:19   ` Dennis Zhou
2024-01-24  8:38     ` Alexandre Ghiti
2024-01-24  8:41       ` Alexandre Ghiti
2024-01-29  1:43         ` yunhui cui
2024-01-29  9:01 ` Dennis Zhou
2024-01-29  9:04   ` Alexandre Ghiti
2024-01-31 20:34   ` Palmer Dabbelt
2024-02-01  3:45     ` Dennis Zhou
2024-02-09 16:41       ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).