[v5, 5/6] riscv: lib: vectorize copy_to_user/copy_from_user

[Andy Chiu]

On Fri, Dec 15, 2023 at 2:25 PM Charlie Jenkins <charlie at rivosinc.com> wrote:
> On Thu, Dec 14, 2023 at 03:57:20PM +0000, Andy Chiu wrote:
> > +     la      t0, riscv_v_usercopy_thres
> > +     REG_L   t0, (t0)
>
> The assembler does something really silly here it seems. With both
> binutils 2.41 and clang 18 the following is generated:
>
> 6:   00000297                auipc   t0,0x0
> a:   00028293                mv      t0,t0
> e:   0002b283                ld      t0,0(t0) # 6 <__asm_copy_from_user+0x4>
>
> However, this la is not needed. You can replace the la + REG_L with just
> a REG_L as follows:
>
> REG_L   t0, riscv_v_usercopy_thres
>
> This then generates the following code:
>
> 6:   00000297                auipc   t0,0x0
> a:   0002b283                ld      t0,0(t0) # 6 <__asm_copy_from_user+0x4>
>

Thanks, this will be fixed in v5

> > +     bltu    a2, t0, fallback_scalar_usercopy
> > +     tail enter_vector_usercopy
> > +#endif
> > +SYM_FUNC_START(fallback_scalar_usercopy)
> >
> >       /* Enable access to user memory */
> >       li t6, SR_SUM
> > @@ -181,6 +191,7 @@ SYM_FUNC_START(__asm_copy_to_user)
> >       sub a0, t5, a0
> >       ret
> >  SYM_FUNC_END(__asm_copy_to_user)
> > +SYM_FUNC_END(fallback_scalar_usercopy)
> >  EXPORT_SYMBOL(__asm_copy_to_user)
> >  SYM_FUNC_ALIAS(__asm_copy_from_user, __asm_copy_to_user)
> >  EXPORT_SYMBOL(__asm_copy_from_user)
> > diff --git a/arch/riscv/lib/uaccess_vector.S b/arch/riscv/lib/uaccess_vector.S
> > new file mode 100644
> > index 000000000000..5bebcb1276a2
> > --- /dev/null
> > +++ b/arch/riscv/lib/uaccess_vector.S
> > @@ -0,0 +1,55 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#include <linux/linkage.h>
> > +#include <asm-generic/export.h>
> > +#include <asm/asm.h>
> > +#include <asm/asm-extable.h>
> > +#include <asm/csr.h>
> > +
> > +#define pDst a0
> > +#define pSrc a1
> > +#define iNum a2
> > +
> > +#define iVL a3
> > +#define pDstPtr a4
> > +
> > +#define ELEM_LMUL_SETTING m8
> > +#define vData v0
> > +
> > +     .macro fixup op reg addr lbl
> > +100:
> > +     \op \reg, \addr
> > +     _asm_extable    100b, \lbl
> > +     .endm
> > +
> > +SYM_FUNC_START(__asm_vector_usercopy)
> > +     /* Enable access to user memory */
> > +     li t6, SR_SUM
> > +     csrs CSR_STATUS, t6
> > +
> > +     /* Save for return value */
> > +     mv      t5, a2
>
> What's the point of this?

Oops, I will remove it

>
> > +
> > +     mv pDstPtr, pDst
>
> Why do this move? pDst isn't used anywhere else so you can safely
> continue to use pDst everywhere that pDstPtr is used.

Yes, it makes more sense to remove pDstPtr and use just pDst.

Thanks,
Andy