* [PATCH] riscv: uaccess: Allow the last potential unrolled copy @ 2024-03-13 10:33 Xiao Wang 2024-05-03 12:16 ` Alexandre Ghiti 2024-05-22 23:51 ` patchwork-bot+linux-riscv 0 siblings, 2 replies; 7+ messages in thread From: Xiao Wang @ 2024-03-13 10:33 UTC (permalink / raw) To: paul.walmsley, palmer, aou Cc: jerry.shih, nick.knight, ajones, bjorn, andy.chiu, viro, cleger, alexghiti, haicheng.li, akira.tsukamoto, linux-riscv, linux-kernel, Xiao Wang When the dst buffer pointer points to the last accessible aligned addr, we could still run another iteration of unrolled copy. Signed-off-by: Xiao Wang <xiao.w.wang@intel.com> --- arch/riscv/lib/uaccess.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S index 2e665f8f8fcc..1399d797d81b 100644 --- a/arch/riscv/lib/uaccess.S +++ b/arch/riscv/lib/uaccess.S @@ -103,7 +103,7 @@ SYM_FUNC_START(fallback_scalar_usercopy) fixup REG_S t4, 7*SZREG(a0), 10f addi a0, a0, 8*SZREG addi a1, a1, 8*SZREG - bltu a0, t0, 2b + bleu a0, t0, 2b addi t0, t0, 8*SZREG /* revert to original value */ j .Lbyte_copy_tail -- 2.25.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy 2024-03-13 10:33 [PATCH] riscv: uaccess: Allow the last potential unrolled copy Xiao Wang @ 2024-05-03 12:16 ` Alexandre Ghiti 2024-05-03 12:19 ` Ben Dooks 2024-05-22 23:51 ` patchwork-bot+linux-riscv 1 sibling, 1 reply; 7+ messages in thread From: Alexandre Ghiti @ 2024-05-03 12:16 UTC (permalink / raw) To: Xiao Wang, paul.walmsley, palmer, aou Cc: jerry.shih, nick.knight, ajones, bjorn, andy.chiu, viro, cleger, alexghiti, haicheng.li, akira.tsukamoto, linux-riscv, linux-kernel Hi Xiao, On 13/03/2024 11:33, Xiao Wang wrote: > When the dst buffer pointer points to the last accessible aligned addr, we > could still run another iteration of unrolled copy. > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com> > --- > arch/riscv/lib/uaccess.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S > index 2e665f8f8fcc..1399d797d81b 100644 > --- a/arch/riscv/lib/uaccess.S > +++ b/arch/riscv/lib/uaccess.S > @@ -103,7 +103,7 @@ SYM_FUNC_START(fallback_scalar_usercopy) > fixup REG_S t4, 7*SZREG(a0), 10f > addi a0, a0, 8*SZREG > addi a1, a1, 8*SZREG > - bltu a0, t0, 2b > + bleu a0, t0, 2b > > addi t0, t0, 8*SZREG /* revert to original value */ > j .Lbyte_copy_tail I agree it is still safe to continue for another word_copy here. Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> Thanks, Alex _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy 2024-05-03 12:16 ` Alexandre Ghiti @ 2024-05-03 12:19 ` Ben Dooks 2024-05-03 13:02 ` Alexandre Ghiti 0 siblings, 1 reply; 7+ messages in thread From: Ben Dooks @ 2024-05-03 12:19 UTC (permalink / raw) To: Alexandre Ghiti, Xiao Wang, paul.walmsley, palmer, aou Cc: jerry.shih, nick.knight, ajones, bjorn, andy.chiu, viro, cleger, alexghiti, haicheng.li, akira.tsukamoto, linux-riscv, linux-kernel On 03/05/2024 13:16, Alexandre Ghiti wrote: > Hi Xiao, > > On 13/03/2024 11:33, Xiao Wang wrote: >> When the dst buffer pointer points to the last accessible aligned >> addr, we >> could still run another iteration of unrolled copy. >> >> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com> >> --- >> arch/riscv/lib/uaccess.S | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S >> index 2e665f8f8fcc..1399d797d81b 100644 >> --- a/arch/riscv/lib/uaccess.S >> +++ b/arch/riscv/lib/uaccess.S >> @@ -103,7 +103,7 @@ SYM_FUNC_START(fallback_scalar_usercopy) >> fixup REG_S t4, 7*SZREG(a0), 10f >> addi a0, a0, 8*SZREG >> addi a1, a1, 8*SZREG >> - bltu a0, t0, 2b >> + bleu a0, t0, 2b >> addi t0, t0, 8*SZREG /* revert to original value */ >> j .Lbyte_copy_tail > > > I agree it is still safe to continue for another word_copy here. > > Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> Out of interest, has anyone checked if causing a schedule event during this code breaks like the last time we had issues with the upstream testing? I did propose saving the state of the user-access flag in the task struct but we mostly solved it by making sleeping functions stay away from the address calculation. This of course may have been done already or need to be done if three's long areas where the user-access flags can be disabled (generally only a few drivers did this, so we may not have come across the problem) -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy 2024-05-03 12:19 ` Ben Dooks @ 2024-05-03 13:02 ` Alexandre Ghiti 2024-05-03 14:30 ` Ben Dooks 0 siblings, 1 reply; 7+ messages in thread From: Alexandre Ghiti @ 2024-05-03 13:02 UTC (permalink / raw) To: Ben Dooks, Xiao Wang, paul.walmsley, palmer, aou Cc: jerry.shih, nick.knight, ajones, bjorn, andy.chiu, viro, cleger, alexghiti, haicheng.li, akira.tsukamoto, linux-riscv, linux-kernel Hi Ben, On 03/05/2024 14:19, Ben Dooks wrote: > On 03/05/2024 13:16, Alexandre Ghiti wrote: >> Hi Xiao, >> >> On 13/03/2024 11:33, Xiao Wang wrote: >>> When the dst buffer pointer points to the last accessible aligned >>> addr, we >>> could still run another iteration of unrolled copy. >>> >>> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com> >>> --- >>> arch/riscv/lib/uaccess.S | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S >>> index 2e665f8f8fcc..1399d797d81b 100644 >>> --- a/arch/riscv/lib/uaccess.S >>> +++ b/arch/riscv/lib/uaccess.S >>> @@ -103,7 +103,7 @@ SYM_FUNC_START(fallback_scalar_usercopy) >>> fixup REG_S t4, 7*SZREG(a0), 10f >>> addi a0, a0, 8*SZREG >>> addi a1, a1, 8*SZREG >>> - bltu a0, t0, 2b >>> + bleu a0, t0, 2b >>> addi t0, t0, 8*SZREG /* revert to original value */ >>> j .Lbyte_copy_tail >> >> >> I agree it is still safe to continue for another word_copy here. >> >> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> > > Out of interest, has anyone checked if causing a schedule event during > this code breaks like the last time we had issues with the upstream > testing? I vaguely remember something, do you have a link to that discussion by chance? > > I did propose saving the state of the user-access flag in the task > struct Makes sense, I just took a quick look and SR_SUM is cleared as soon as we enter handle_exception() and it does not seem to be restored. Weird it works, unless I missed something! > but we mostly solved it by making sleeping functions stay > away from the address calculation. This of course may have been done > already or need to be done if three's long areas where the user-access > flags can be disabled (generally only a few drivers did this, so we > may not have come across the problem) > I don't understand what you mean here, would you mind expanding a bit? Thanks, Alex _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy 2024-05-03 13:02 ` Alexandre Ghiti @ 2024-05-03 14:30 ` Ben Dooks 2024-05-06 7:53 ` Wang, Xiao W 0 siblings, 1 reply; 7+ messages in thread From: Ben Dooks @ 2024-05-03 14:30 UTC (permalink / raw) To: Alexandre Ghiti, Xiao Wang, paul.walmsley, palmer, aou Cc: jerry.shih, nick.knight, ajones, bjorn, andy.chiu, viro, cleger, alexghiti, haicheng.li, akira.tsukamoto, linux-riscv, linux-kernel On 03/05/2024 14:02, Alexandre Ghiti wrote: > Hi Ben, > > On 03/05/2024 14:19, Ben Dooks wrote: >> On 03/05/2024 13:16, Alexandre Ghiti wrote: >>> Hi Xiao, >>> >>> On 13/03/2024 11:33, Xiao Wang wrote: >>>> When the dst buffer pointer points to the last accessible aligned >>>> addr, we >>>> could still run another iteration of unrolled copy. >>>> >>>> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com> >>>> --- >>>> arch/riscv/lib/uaccess.S | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S >>>> index 2e665f8f8fcc..1399d797d81b 100644 >>>> --- a/arch/riscv/lib/uaccess.S >>>> +++ b/arch/riscv/lib/uaccess.S >>>> @@ -103,7 +103,7 @@ SYM_FUNC_START(fallback_scalar_usercopy) >>>> fixup REG_S t4, 7*SZREG(a0), 10f >>>> addi a0, a0, 8*SZREG >>>> addi a1, a1, 8*SZREG >>>> - bltu a0, t0, 2b >>>> + bleu a0, t0, 2b >>>> addi t0, t0, 8*SZREG /* revert to original value */ >>>> j .Lbyte_copy_tail >>> >>> >>> I agree it is still safe to continue for another word_copy here. >>> >>> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> >> >> Out of interest, has anyone checked if causing a schedule event during >> this code breaks like the last time we had issues with the upstream >> testing? > > > I vaguely remember something, do you have a link to that discussion by > chance? > > >> >> I did propose saving the state of the user-access flag in the task >> struct > > > Makes sense, I just took a quick look and SR_SUM is cleared as soon as > we enter handle_exception() and it does not seem to be restored. Weird > it works, unless I missed something! > > >> but we mostly solved it by making sleeping functions stay >> away from the address calculation. This of course may have been done >> already or need to be done if three's long areas where the user-access >> flags can be disabled (generally only a few drivers did this, so we >> may not have come across the problem) >> > I don't understand what you mean here, would you mind expanding a bit? > I think this was all gone through in the original post where we initially suggested saving SR_SUM and then moved as much out of the critical SR_SUM area by changing how the macros worked https://lore.kernel.org/linux-riscv/20210318151010.100966-1-ben.dooks@codethink.co.uk/ https://lore.kernel.org/linux-riscv/20210329095749.998940-1-ben.dooks@codethink.co.uk/ -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] riscv: uaccess: Allow the last potential unrolled copy 2024-05-03 14:30 ` Ben Dooks @ 2024-05-06 7:53 ` Wang, Xiao W 0 siblings, 0 replies; 7+ messages in thread From: Wang, Xiao W @ 2024-05-06 7:53 UTC (permalink / raw) To: ben.dooks@codethink.co.uk, Alexandre Ghiti, paul.walmsley@sifive.com, palmer@dabbelt.com, aou@eecs.berkeley.edu Cc: jerry.shih@sifive.com, nick.knight@sifive.com, ajones@ventanamicro.com, bjorn@rivosinc.com, andy.chiu@sifive.com, viro@zeniv.linux.org.uk, cleger@rivosinc.com, alexghiti@rivosinc.com, Li, Haicheng, akira.tsukamoto@gmail.com, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Hi Alex, > -----Original Message----- > From: ben.dooks@codethink.co.uk <ben.dooks@codethink.co.uk> > Sent: Friday, May 3, 2024 10:30 PM > To: Alexandre Ghiti <alex@ghiti.fr>; Wang, Xiao W <xiao.w.wang@intel.com>; > paul.walmsley@sifive.com; palmer@dabbelt.com; aou@eecs.berkeley.edu > Cc: jerry.shih@sifive.com; nick.knight@sifive.com; > ajones@ventanamicro.com; bjorn@rivosinc.com; andy.chiu@sifive.com; > viro@zeniv.linux.org.uk; cleger@rivosinc.com; alexghiti@rivosinc.com; Li, > Haicheng <haicheng.li@intel.com>; akira.tsukamoto@gmail.com; linux- > riscv@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy > > On 03/05/2024 14:02, Alexandre Ghiti wrote: > > Hi Ben, > > > > On 03/05/2024 14:19, Ben Dooks wrote: > >> On 03/05/2024 13:16, Alexandre Ghiti wrote: > >>> Hi Xiao, > >>> > >>> On 13/03/2024 11:33, Xiao Wang wrote: > >>>> When the dst buffer pointer points to the last accessible aligned > >>>> addr, we > >>>> could still run another iteration of unrolled copy. > >>>> > >>>> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com> > >>>> --- > >>>> arch/riscv/lib/uaccess.S | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S > >>>> index 2e665f8f8fcc..1399d797d81b 100644 > >>>> --- a/arch/riscv/lib/uaccess.S > >>>> +++ b/arch/riscv/lib/uaccess.S > >>>> @@ -103,7 +103,7 @@ SYM_FUNC_START(fallback_scalar_usercopy) > >>>> fixup REG_S t4, 7*SZREG(a0), 10f > >>>> addi a0, a0, 8*SZREG > >>>> addi a1, a1, 8*SZREG > >>>> - bltu a0, t0, 2b > >>>> + bleu a0, t0, 2b > >>>> addi t0, t0, 8*SZREG /* revert to original value */ > >>>> j .Lbyte_copy_tail > >>> > >>> > >>> I agree it is still safe to continue for another word_copy here. > >>> > >>> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> > >> > >> Out of interest, has anyone checked if causing a schedule event during > >> this code breaks like the last time we had issues with the upstream > >> testing? > > > > > > I vaguely remember something, do you have a link to that discussion by > > chance? > > > > > >> > >> I did propose saving the state of the user-access flag in the task > >> struct > > > > > > Makes sense, I just took a quick look and SR_SUM is cleared as soon as > > we enter handle_exception() and it does not seem to be restored. Weird > > it works, unless I missed something! I think it's already saved/restored via pt_regs.status, using the PT_STATUS() macro in entry.S. BRs, Xiao > > > > > >> but we mostly solved it by making sleeping functions stay > >> away from the address calculation. This of course may have been done > >> already or need to be done if three's long areas where the user-access > >> flags can be disabled (generally only a few drivers did this, so we > >> may not have come across the problem) > >> > > I don't understand what you mean here, would you mind expanding a bit? > > > > I think this was all gone through in the original post where > we initially suggested saving SR_SUM and then moved as much out > of the critical SR_SUM area by changing how the macros worked > > https://lore.kernel.org/linux-riscv/20210318151010.100966-1- > ben.dooks@codethink.co.uk/ > > https://lore.kernel.org/linux-riscv/20210329095749.998940-1- > ben.dooks@codethink.co.uk/ > -- > Ben Dooks http://www.codethink.co.uk/ > Senior Engineer Codethink - Providing Genius > > https://www.codethink.co.uk/privacy.html _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy 2024-03-13 10:33 [PATCH] riscv: uaccess: Allow the last potential unrolled copy Xiao Wang 2024-05-03 12:16 ` Alexandre Ghiti @ 2024-05-22 23:51 ` patchwork-bot+linux-riscv 1 sibling, 0 replies; 7+ messages in thread From: patchwork-bot+linux-riscv @ 2024-05-22 23:51 UTC (permalink / raw) To: Xiao Wang Cc: linux-riscv, paul.walmsley, palmer, aou, jerry.shih, nick.knight, ajones, bjorn, andy.chiu, viro, cleger, alexghiti, haicheng.li, akira.tsukamoto, linux-kernel Hello: This patch was applied to riscv/linux.git (for-next) by Palmer Dabbelt <palmer@rivosinc.com>: On Wed, 13 Mar 2024 18:33:34 +0800 you wrote: > When the dst buffer pointer points to the last accessible aligned addr, we > could still run another iteration of unrolled copy. > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com> > --- > arch/riscv/lib/uaccess.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Here is the summary with links: - riscv: uaccess: Allow the last potential unrolled copy https://git.kernel.org/riscv/c/74362d66a416 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-05-22 23:52 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-13 10:33 [PATCH] riscv: uaccess: Allow the last potential unrolled copy Xiao Wang 2024-05-03 12:16 ` Alexandre Ghiti 2024-05-03 12:19 ` Ben Dooks 2024-05-03 13:02 ` Alexandre Ghiti 2024-05-03 14:30 ` Ben Dooks 2024-05-06 7:53 ` Wang, Xiao W 2024-05-22 23:51 ` patchwork-bot+linux-riscv
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox