Re: [PATCH -next v21 09/27] riscv: Introduce struct/helpers to save/restore per-task Vector state

Rémi Denis-Courmont
2023-06-12T14:36:18Z

Le maanantaina 5. kesäkuuta 2023, 14.07.06 EEST Andy Chiu a écrit :
> @@ -32,13 +54,86 @@ static __always_inline void riscv_v_disable(void)
> csr_clear(CSR_SSTATUS, SR_VS);
> }
>
> +static __always_inline void __vstate_csr_save(struct __riscv_v_ext_state
> *dest) +{
> + asm volatile (
> + "csrr %0, " __stringify(CSR_VSTART) "\n\t"
> + "csrr %1, " __stringify(CSR_VTYPE) "\n\t"
> + "csrr %2, " __stringify(CSR_VL) "\n\t"
> + "csrr %3, " __stringify(CSR_VCSR) "\n\t"
> + : "=r" (dest->vstart), "=r" (dest->vtype), "=r" (dest-
>vl),
> + "=r" (dest->vcsr) : :);
> +}
> +
> +static __always_inline void __vstate_csr_restore(struct __riscv_v_ext_state
> *src) +{
> + asm volatile (
> + ".option push\n\t"
> + ".option arch, +v\n\t"
> + "vsetvl x0, %2, %1\n\t"
> + ".option pop\n\t"
> + "csrw " __stringify(CSR_VSTART) ", %0\n\t"
> + "csrw " __stringify(CSR_VCSR) ", %3\n\t"
> + : : "r" (src->vstart), "r" (src->vtype), "r" (src->vl),
> + "r" (src->vcsr) :);
> +}
> +
> +static inline void __riscv_v_vstate_save(struct __riscv_v_ext_state
> *save_to, + void *datap)
> +{
> + unsigned long vl;
> +
> + riscv_v_enable();
> + __vstate_csr_save(save_to);
> + asm volatile (
> + ".option push\n\t"
> + ".option arch, +v\n\t"
> + "vsetvli %0, x0, e8, m8, ta, ma\n\t"
> + "vse8.v v0, (%1)\n\t"
> + "add %1, %1, %0\n\t"
> + "vse8.v v8, (%1)\n\t"
> + "add %1, %1, %0\n\t"
> + "vse8.v v16, (%1)\n\t"
> + "add %1, %1, %0\n\t"
> + "vse8.v v24, (%1)\n\t"
> + ".option pop\n\t"
> + : "=&r" (vl) : "r" (datap) : "memory");
> + riscv_v_disable();
> +}

Shouldn't this use `vs8r.v` rather than `vse8.v`, and do away with `vsetvli`?
This seems like a textbook use case for the whole-register store instruction,
no?

> +
> +static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state
> *restore_from, + void
*datap)
> +{
> + unsigned long vl;
> +
> + riscv_v_enable();
> + asm volatile (
> + ".option push\n\t"
> + ".option arch, +v\n\t"
> + "vsetvli %0, x0, e8, m8, ta, ma\n\t"
> + "vle8.v v0, (%1)\n\t"
> + "add %1, %1, %0\n\t"
> + "vle8.v v8, (%1)\n\t"
> + "add %1, %1, %0\n\t"
> + "vle8.v v16, (%1)\n\t"
> + "add %1, %1, %0\n\t"
> + "vle8.v v24, (%1)\n\t"
> + ".option pop\n\t"
> + : "=&r" (vl) : "r" (datap) : "memory");
> + __vstate_csr_restore(restore_from);
> + riscv_v_disable();
> +}
> +

Ditto but `vl8r.v`.

> #else /* ! CONFIG_RISCV_ISA_V */
>
> struct pt_regs;
>
> static inline int riscv_v_setup_vsize(void) { return -EOPNOTSUPP; }
> static __always_inline bool has_vector(void) { return false; }
> +static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return
> false; } #define riscv_v_vsize (0)
> +#define riscv_v_vstate_off(regs) do {} while (0)
> +#define riscv_v_vstate_on(regs) do {} while (0)
>
> #endif /* CONFIG_RISCV_ISA_V */
>

--
Реми Дёни-Курмон