Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy

From: Ben Dooks
Date: Fri May 03 2024 - 10:30:41 EST


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@xxxxxxxxx>
---
  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@xxxxxxxxxxxx>

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@xxxxxxxxxxxxxxx/

https://lore.kernel.org/linux-riscv/20210329095749.998940-1-ben.dooks@xxxxxxxxxxxxxxx/
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html