Re: [PATCH v2 3/5] riscv: Vector checksum header

From: Charlie Jenkins
Date: Thu Sep 07 2023 - 13:42:23 EST


On Thu, Sep 07, 2023 at 10:58:33AM +0100, Conor Dooley wrote:
> On Tue, Sep 05, 2023 at 09:46:52PM -0700, Charlie Jenkins wrote:
> > Vector code is written in assembly rather than using the GCC vector
> > instrinsics because they did not provide optimal code. Vector
> > instrinsic types are still used so the inline assembly can
> > appropriately select vector registers. However, this code cannot be
> > merged yet because it is currently not possible to use vector
> > instrinsics in the kernel because vector support needs to be directly
> > enabled by assembly.
> >
> > Signed-off-by: Charlie Jenkins <charlie@xxxxxxxxxxxx>
> > ---
> > arch/riscv/include/asm/checksum.h | 87 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 87 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/checksum.h b/arch/riscv/include/asm/checksum.h
> > index 3f9d5a202e95..1d6c23cd1221 100644
> > --- a/arch/riscv/include/asm/checksum.h
> > +++ b/arch/riscv/include/asm/checksum.h
> > @@ -10,6 +10,10 @@
> > #include <linux/in6.h>
> > #include <linux/uaccess.h>
> >
> > +#ifdef CONFIG_RISCV_ISA_V
> > +#include <riscv_vector.h>
> > +#endif
> > +
> > #ifdef CONFIG_32BIT
> > typedef unsigned int csum_t;
> > #else
> > @@ -43,6 +47,89 @@ static inline __sum16 csum_fold(__wsum sum)
> > */
> > static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
> > {
> > +#ifdef CONFIG_RISCV_ISA_V
> > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) {
> > + /*
> > + * Vector is likely available when the kernel is compiled with
> > + * vector support, so nop when vector is available and jump when
> > + * vector is not available.
> > + */
> > + asm_volatile_goto(ALTERNATIVE("j %l[no_vector]", "nop", 0,
> > + RISCV_ISA_EXT_v, 1)
> > + :
> > + :
> > + :
> > + : no_vector);
> > + } else {
> > + if (!__riscv_isa_extension_available(NULL, RISCV_ISA_EXT_v))
> > + goto no_vector;
> > + }
> > +
> > + vuint64m1_t prev_buffer;
> > + vuint32m1_t curr_buffer;
> > + unsigned int vl;
> > +#ifdef CONFIG_32_BIT
> > + csum_t high_result, low_result;
> > +
> > + riscv_v_enable();
> > + asm(".option push \n\
> > + .option arch, +v \n\
> > + vsetivli x0, 1, e64, ta, ma \n\
>
> Also, I don't see that you have addressed previous review comments from
> Samuel:
> https://lore.kernel.org/linux-riscv/0a8c98bf-46da-e77a-0431-a6c1e224af2e@xxxxxxxxxx/
I added the check for vector as Samuel suggested, but then I
accidentally used riscv_v_enable() instead of kernel_vector_begin(), I
will make that change.

- Charlie