On 9/21/22 14:43, Chris Stillson wrote:
From: Greentime Hu <greentime.hu@xxxxxxxxxx>
This patch is used to detect vector support status of CPU and use
riscv_vsize to save the size of all the vector registers. It assumes
all harts has the same capabilities in SMP system.
Patch title is horrible. The meat of patch is vector state save/restore,
but no users of it yet. And then there are random unrelated snippets
thrown in same patch.
+#ifdef CONFIG_FPU
+__ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_fpu);
+#endif
This needs to be broken out to a FPU patch which actually uses
cpu_hwcap_fpu.
+#ifdef CONFIG_VECTOR
+#include <asm/vector.h>
+__ro_after_init DEFINE_STATIC_KEY_FALSE(cpu_hwcap_vector);
+unsigned long riscv_vsize __read_mostly;
+#endif
I'd move this patch v2/17 as part of detection etc.
+#ifdef CONFIG_VECTOR
+ if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
+ static_branch_enable(&cpu_hwcap_vector);
Ditto.
+ /* There are 32 vector registers with vlenb length. */
+ rvv_enable();
+ riscv_vsize = csr_read(CSR_VLENB) * 32;
Ditto.
+ rvv_disable();
But guess these needs to be added first as well, see below.
+#ifdef CONFIG_VECTOR
+#include <asm/vector.h>
+EXPORT_SYMBOL(rvv_enable);
+EXPORT_SYMBOL(rvv_disable);
As suggested in prior review comment, we don't need to EXPORT these, for
now at least.
diff --git a/arch/riscv/kernel/vector.S b/arch/riscv/kernel/vector.S
new file mode 100644
index 000000000000..9f7dc70c4443
--- /dev/null
+++ b/arch/riscv/kernel/vector.S
@@ -0,0 +1,93 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2017 SiFive
+ * Copyright (C) 2019 Alibaba Group Holding Limited
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation, version 2.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
SPDX hdr ...
+
+#include <linux/linkage.h>
+
+#include <asm/asm.h>
+#include <asm/csr.h>
+#include <asm/asm-offsets.h>
+
+#define vstatep a0
+#define datap a1
+#define x_vstart t0
+#define x_vtype t1
+#define x_vl t2
+#define x_vcsr t3
+#define incr t4
+#define status t5
+
A few words here as to when is this save/restore done.
Perhaps best to add this code in the patch which actually uses these.
+ENTRY(__vstate_save)
+ li status, SR_VS
+ csrs CSR_STATUS, status
+
+ csrr x_vstart, CSR_VSTART
+ csrr x_vtype, CSR_VTYPE
+ csrr x_vl, CSR_VL
+ csrr x_vcsr, CSR_VCSR
+ vsetvli incr, x0, e8, m8, ta, ma
+ vse8.v v0, (datap)
+ add datap, datap, incr
+ vse8.v v8, (datap)
+ add datap, datap, incr
+ vse8.v v16, (datap)
+ add datap, datap, incr
+ vse8.v v24, (datap)
+
+ REG_S x_vstart, RISCV_V_STATE_VSTART(vstatep)
+ REG_S x_vtype, RISCV_V_STATE_VTYPE(vstatep)
+ REG_S x_vl, RISCV_V_STATE_VL(vstatep)
+ REG_S x_vcsr, RISCV_V_STATE_VCSR(vstatep)
+
+ csrc CSR_STATUS, status
+ ret
+ENDPROC(__vstate_save)
+
+ENTRY(__vstate_restore)
+ li status, SR_VS
+ csrs CSR_STATUS, status
This is rvv_enable code duplicated inline.
+
+ vsetvli incr, x0, e8, m8, ta, ma
+ vle8.v v0, (datap)
+ add datap, datap, incr
+ vle8.v v8, (datap)
+ add datap, datap, incr
+ vle8.v v16, (datap)
+ add datap, datap, incr
+ vle8.v v24, (datap)
+
+ REG_L x_vstart, RISCV_V_STATE_VSTART(vstatep)
+ REG_L x_vtype, RISCV_V_STATE_VTYPE(vstatep)
+ REG_L x_vl, RISCV_V_STATE_VL(vstatep)
+ REG_L x_vcsr, RISCV_V_STATE_VCSR(vstatep)
+ vsetvl x0, x_vl, x_vtype
+ csrw CSR_VSTART, x_vstart
+ csrw CSR_VCSR, x_vcsr
+
+ csrc CSR_STATUS, status
+ ret
+ENDPROC(__vstate_restore)
+
+ENTRY(rvv_enable)
+ li status, SR_VS
+ csrs CSR_STATUS, status
+ ret
+ENDPROC(rvv_enable)
+
+ENTRY(rvv_disable)
+ li status, SR_VS
+ csrc CSR_STATUS, status
+ ret
+ENDPROC(rvv_disable)
I'd suggest these be made asm macros, to avoid function call overhead
and duplication as in save/restore above.
