2024-03-18 10:42:08

by Andy Chiu

[permalink] [raw]
Subject: [PATCH v3 0/7] Support Zve32[xf] and Zve64[xfd] Vector subextensions

The series composes of two parts. The first part provides a quick fix for
the issue on a recent thread[1]. The issue happens when a platform has
ununified vector register length across multiple cores. Specifically,
patch 1 adds a comment at a callsite of riscv_setup_vsize to clarify how
vlenb is observed by the system. Patch 2 fixes the issue by failing the
boot process of a secondary core if vlenb mismatches.

The second part of the series provide a finer grain view of the Vector
extension. Patch 3 give the obsolete ISA parser the ability to expand
ISA extensions for sigle letter extensions. Patch 3, 4 introduces Zve32x,
Zve32f, Zve64x, Zve64f, Zve64d for isa parsing and hwprobe. Patch 5
updates all callsites such that Vector subextensions are maximumly
supported by the kernel.

Two parts of the series are sent together to ease the effort of picking
dependency patches. The first part can be merged independent of the
second one if necessary.

The series is tested on a QEMU and verified that booting, Vector
programs context-switch, signal, ptrace, prctl(sysctl knob) interfaces
works when we only report partial V from the ISA.

This patch should be able to apply on risc-v for-next branch on top of
the commit 099dbac6e90c ("Merge patch series "riscv: Use Kconfig to set unaligned access speed"")

[1]: https://lore.kernel.org/all/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/T/#u

v2 of this series can be found at: https://lore.kernel.org/all/[email protected]/

Changelog v3:
- Include correct maintainers and mailing list into CC.
- Cleanup isa string parser code (3)
- Adjust extensions order and name (4, 5)
- Refine commit message (6)

Changelog v2:
- Update comments and commit messages (1, 2, 7)
- Refine isa_exts[] lists for zve extensions (4)
- Add a patch for dt-binding (5)
- Make ZVE* extensions depend on has_vector(ZVE32X) (6, 7)

---
Andy Chiu (7):
riscv: vector: add a comment when calling riscv_setup_vsize()
riscv: smp: fail booting up smp if inconsistent vlen is detected
riscv: cpufeature: call match_isa_ext() for single-letter extensions
riscv: cpufeature: add zve32[xf] and zve64[xfd] isa detection
dt-bindings: riscv: add Zve32[xf] Zve64[xfd] ISA extension description
riscv: hwprobe: add zve Vector subextensions into hwprobe interface
riscv: vector: adjust minimum Vector requirement to ZVE32X

Documentation/arch/riscv/hwprobe.rst | 15 ++++++
.../devicetree/bindings/riscv/extensions.yaml | 30 ++++++++++++
arch/riscv/include/asm/hwcap.h | 5 ++
arch/riscv/include/asm/switch_to.h | 2 +-
arch/riscv/include/asm/vector.h | 21 +++++---
arch/riscv/include/asm/xor.h | 2 +-
arch/riscv/include/uapi/asm/hwprobe.h | 5 ++
arch/riscv/kernel/cpufeature.c | 56 ++++++++++++++++++----
arch/riscv/kernel/head.S | 14 +++---
arch/riscv/kernel/kernel_mode_vector.c | 4 +-
arch/riscv/kernel/process.c | 4 +-
arch/riscv/kernel/signal.c | 6 +--
arch/riscv/kernel/smpboot.c | 14 ++++--
arch/riscv/kernel/sys_hwprobe.c | 13 ++++-
arch/riscv/kernel/vector.c | 15 +++---
arch/riscv/lib/uaccess.S | 2 +-
16 files changed, 163 insertions(+), 45 deletions(-)
---
base-commit: 099dbac6e90c620d8ce0bbf75bbdc94da1feb4fb
change-id: 20240318-zve-detection-50106d2da527

Best regards,
--
Andy Chiu <[email protected]>



2024-03-18 10:44:14

by Andy Chiu

[permalink] [raw]
Subject: [PATCH v3 7/7] riscv: vector: adjust minimum Vector requirement to ZVE32X

Make has_vector take one argument. This argument represents the minimum
Vector subextension that the following Vector actions assume.

Also, change riscv_v_first_use_handler(), and boot code that calls
riscv_v_setup_vsize() to accept the minimum Vector sub-extension,
ZVE32X.

Most kernel/user interfaces requires minimum of ZVE32X. Thus, programs
compiled and run with ZVE32X should be supported by the kernel on most
aspects. This includes context-switch, signal, ptrace, prctl, and
hwprobe.

One exception is that ELF_HWCAP returns 'V' only if full V is supported
on the platform. This means that the system without a full V must not
rely on ELF_HWCAP to tell whether it is allowable to execute Vector
without first invoking a prctl() check.

Signed-off-by: Andy Chiu <[email protected]>
Acked-by: Joel Granados <[email protected]>
---
Changelog v2:
- update the comment in hwprobe.
---
arch/riscv/include/asm/switch_to.h | 2 +-
arch/riscv/include/asm/vector.h | 21 ++++++++++++++-------
arch/riscv/include/asm/xor.h | 2 +-
arch/riscv/kernel/cpufeature.c | 5 ++++-
arch/riscv/kernel/kernel_mode_vector.c | 4 ++--
arch/riscv/kernel/process.c | 4 ++--
arch/riscv/kernel/signal.c | 6 +++---
arch/riscv/kernel/smpboot.c | 2 +-
arch/riscv/kernel/sys_hwprobe.c | 8 ++++++--
arch/riscv/kernel/vector.c | 15 +++++++++------
arch/riscv/lib/uaccess.S | 2 +-
11 files changed, 44 insertions(+), 27 deletions(-)

diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index 7efdb0584d47..df1adf196c4f 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -78,7 +78,7 @@ do { \
struct task_struct *__next = (next); \
if (has_fpu()) \
__switch_to_fpu(__prev, __next); \
- if (has_vector()) \
+ if (has_vector(ZVE32X)) \
__switch_to_vector(__prev, __next); \
((last) = __switch_to(__prev, __next)); \
} while (0)
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index 731dcd0ed4de..b96750493dfb 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -18,6 +18,7 @@
#include <asm/cpufeature.h>
#include <asm/csr.h>
#include <asm/asm.h>
+#include <asm/bug.h>

extern unsigned long riscv_v_vsize;
int riscv_v_setup_vsize(void);
@@ -35,10 +36,16 @@ static inline u32 riscv_v_flags(void)
return READ_ONCE(current->thread.riscv_v_flags);
}

-static __always_inline bool has_vector(void)
-{
- return riscv_has_extension_unlikely(RISCV_ISA_EXT_v);
-}
+#define has_vector(VEXT) \
+({ \
+ static_assert(RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE32X || \
+ RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE32F || \
+ RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE64X || \
+ RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE64F || \
+ RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE64D || \
+ RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_v); \
+ riscv_has_extension_unlikely(RISCV_ISA_EXT_##VEXT); \
+})

static inline void __riscv_v_vstate_clean(struct pt_regs *regs)
{
@@ -131,7 +138,7 @@ static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_
riscv_v_enable();
asm volatile (
".option push\n\t"
- ".option arch, +v\n\t"
+ ".option arch, +zve32x\n\t"
"vsetvli %0, x0, e8, m8, ta, ma\n\t"
"vle8.v v0, (%1)\n\t"
"add %1, %1, %0\n\t"
@@ -153,7 +160,7 @@ static inline void __riscv_v_vstate_discard(void)
riscv_v_enable();
asm volatile (
".option push\n\t"
- ".option arch, +v\n\t"
+ ".option arch, +zve32x\n\t"
"vsetvli %0, x0, e8, m8, ta, ma\n\t"
"vmv.v.i v0, -1\n\t"
"vmv.v.i v8, -1\n\t"
@@ -267,7 +274,7 @@ bool riscv_v_vstate_ctrl_user_allowed(void);
struct pt_regs;

static inline int riscv_v_setup_vsize(void) { return -EOPNOTSUPP; }
-static __always_inline bool has_vector(void) { return false; }
+static __always_inline bool has_vector(unsigned long min_sub_ext) { return false; }
static inline bool riscv_v_first_use_handler(struct pt_regs *regs) { return false; }
static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; }
static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; }
diff --git a/arch/riscv/include/asm/xor.h b/arch/riscv/include/asm/xor.h
index 96011861e46b..46042ef5a2f7 100644
--- a/arch/riscv/include/asm/xor.h
+++ b/arch/riscv/include/asm/xor.h
@@ -61,7 +61,7 @@ static struct xor_block_template xor_block_rvv = {
do { \
xor_speed(&xor_block_8regs); \
xor_speed(&xor_block_32regs); \
- if (has_vector()) { \
+ if (has_vector(ZVE32X)) { \
xor_speed(&xor_block_rvv);\
} \
} while (0)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 6e294a35a4b0..3e2a62873f55 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -690,12 +690,15 @@ void __init riscv_fill_hwcap(void)
elf_hwcap &= ~COMPAT_HWCAP_ISA_F;
}

- if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
+ if (__riscv_isa_extension_available(NULL, RISCV_ISA_EXT_ZVE32X)) {
/*
* This callsite can't fail here. It cannot fail when called on
* the boot hart.
*/
riscv_v_setup_vsize();
+ }
+
+ if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
/*
* ISA string in device tree might have 'v' flag, but
* CONFIG_RISCV_ISA_V is disabled in kernel.
diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
index 6afe80c7f03a..0d4d1a03d1c7 100644
--- a/arch/riscv/kernel/kernel_mode_vector.c
+++ b/arch/riscv/kernel/kernel_mode_vector.c
@@ -208,7 +208,7 @@ void kernel_vector_begin(void)
{
bool nested = false;

- if (WARN_ON(!has_vector()))
+ if (WARN_ON(!has_vector(ZVE32X)))
return;

BUG_ON(!may_use_simd());
@@ -236,7 +236,7 @@ EXPORT_SYMBOL_GPL(kernel_vector_begin);
*/
void kernel_vector_end(void)
{
- if (WARN_ON(!has_vector()))
+ if (WARN_ON(!has_vector(ZVE32X)))
return;

riscv_v_disable();
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 92922dbd5b5c..919e72f9fff6 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -178,7 +178,7 @@ void flush_thread(void)
void arch_release_task_struct(struct task_struct *tsk)
{
/* Free the vector context of datap. */
- if (has_vector())
+ if (has_vector(ZVE32X))
riscv_v_thread_free(tsk);
}

@@ -225,7 +225,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
p->thread.s[0] = 0;
}
p->thread.riscv_v_flags = 0;
- if (has_vector())
+ if (has_vector(ZVE32X))
riscv_v_thread_alloc(p);
p->thread.ra = (unsigned long)ret_from_fork;
p->thread.sp = (unsigned long)childregs; /* kernel sp */
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 501e66debf69..a96e6e969a3f 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -188,7 +188,7 @@ static long restore_sigcontext(struct pt_regs *regs,

return 0;
case RISCV_V_MAGIC:
- if (!has_vector() || !riscv_v_vstate_query(regs) ||
+ if (!has_vector(ZVE32X) || !riscv_v_vstate_query(regs) ||
size != riscv_v_sc_size)
return -EINVAL;

@@ -210,7 +210,7 @@ static size_t get_rt_frame_size(bool cal_all)

frame_size = sizeof(*frame);

- if (has_vector()) {
+ if (has_vector(ZVE32X)) {
if (cal_all || riscv_v_vstate_query(task_pt_regs(current)))
total_context_size += riscv_v_sc_size;
}
@@ -283,7 +283,7 @@ static long setup_sigcontext(struct rt_sigframe __user *frame,
if (has_fpu())
err |= save_fp_state(regs, &sc->sc_fpregs);
/* Save the vector state. */
- if (has_vector() && riscv_v_vstate_query(regs))
+ if (has_vector(ZVE32X) && riscv_v_vstate_query(regs))
err |= save_v_state(regs, (void __user **)&sc_ext_ptr);
/* Write zero to fp-reserved space and check it on restore_sigcontext */
err |= __put_user(0, &sc->sc_extdesc.reserved);
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 1f86ee10192f..4eb36d75f091 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -218,7 +218,7 @@ asmlinkage __visible void smp_callin(void)
struct mm_struct *mm = &init_mm;
unsigned int curr_cpuid = smp_processor_id();

- if (has_vector()) {
+ if (has_vector(ZVE32X)) {
/*
* Return as early as possible so the hart with a mismatching
* vlen won't boot.
diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index c8219b82fbfc..e7c3fcac62a1 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -69,7 +69,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
if (riscv_isa_extension_available(NULL, c))
pair->value |= RISCV_HWPROBE_IMA_C;

- if (has_vector())
+ if (has_vector(v))
pair->value |= RISCV_HWPROBE_IMA_V;

/*
@@ -112,7 +112,11 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
EXT_KEY(ZACAS);
EXT_KEY(ZICOND);

- if (has_vector()) {
+ /*
+ * Vector crypto and ZVE* extensions are supported only if
+ * kernel has minimum V support of ZVE32X.
+ */
+ if (has_vector(ZVE32X)) {
EXT_KEY(ZVE32X);
EXT_KEY(ZVE32F);
EXT_KEY(ZVE64X);
diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
index 6727d1d3b8f2..e8a47fa72351 100644
--- a/arch/riscv/kernel/vector.c
+++ b/arch/riscv/kernel/vector.c
@@ -53,7 +53,7 @@ int riscv_v_setup_vsize(void)

void __init riscv_v_setup_ctx_cache(void)
{
- if (!has_vector())
+ if (!has_vector(ZVE32X))
return;

riscv_v_user_cachep = kmem_cache_create_usercopy("riscv_vector_ctx",
@@ -173,8 +173,11 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
u32 __user *epc = (u32 __user *)regs->epc;
u32 insn = (u32)regs->badaddr;

+ if (!has_vector(ZVE32X))
+ return false;
+
/* Do not handle if V is not supported, or disabled */
- if (!(ELF_HWCAP & COMPAT_HWCAP_ISA_V))
+ if (!riscv_v_vstate_ctrl_user_allowed())
return false;

/* If V has been enabled then it is not the first-use trap */
@@ -213,7 +216,7 @@ void riscv_v_vstate_ctrl_init(struct task_struct *tsk)
bool inherit;
int cur, next;

- if (!has_vector())
+ if (!has_vector(ZVE32X))
return;

next = riscv_v_ctrl_get_next(tsk);
@@ -235,7 +238,7 @@ void riscv_v_vstate_ctrl_init(struct task_struct *tsk)

long riscv_v_vstate_ctrl_get_current(void)
{
- if (!has_vector())
+ if (!has_vector(ZVE32X))
return -EINVAL;

return current->thread.vstate_ctrl & PR_RISCV_V_VSTATE_CTRL_MASK;
@@ -246,7 +249,7 @@ long riscv_v_vstate_ctrl_set_current(unsigned long arg)
bool inherit;
int cur, next;

- if (!has_vector())
+ if (!has_vector(ZVE32X))
return -EINVAL;

if (arg & ~PR_RISCV_V_VSTATE_CTRL_MASK)
@@ -296,7 +299,7 @@ static struct ctl_table riscv_v_default_vstate_table[] = {

static int __init riscv_v_sysctl_init(void)
{
- if (has_vector())
+ if (has_vector(ZVE32X))
if (!register_sysctl("abi", riscv_v_default_vstate_table))
return -EINVAL;
return 0;
diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
index bc22c078aba8..bbe143bb32a0 100644
--- a/arch/riscv/lib/uaccess.S
+++ b/arch/riscv/lib/uaccess.S
@@ -14,7 +14,7 @@

SYM_FUNC_START(__asm_copy_to_user)
#ifdef CONFIG_RISCV_ISA_V
- ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_v, CONFIG_RISCV_ISA_V)
+ ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_ZVE32X, CONFIG_RISCV_ISA_V)
REG_L t0, riscv_v_usercopy_threshold
bltu a2, t0, fallback_scalar_usercopy
tail enter_vector_usercopy

--
2.44.0.rc2


2024-03-18 17:35:31

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 7/7] riscv: vector: adjust minimum Vector requirement to ZVE32X

Hi Andy,

kernel test robot noticed the following build errors:

[auto build test ERROR on 099dbac6e90c620d8ce0bbf75bbdc94da1feb4fb]

url: https://github.com/intel-lab-lkp/linux/commits/Andy-Chiu/riscv-vector-add-a-comment-when-calling-riscv_setup_vsize/20240318-184348
base: 099dbac6e90c620d8ce0bbf75bbdc94da1feb4fb
patch link: https://lore.kernel.org/r/20240318-zve-detection-v3-7-e12d42107fa8%40sifive.com
patch subject: [PATCH v3 7/7] riscv: vector: adjust minimum Vector requirement to ZVE32X
config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20240319/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240319/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

arch/riscv/kernel/process.c: In function 'arch_release_task_struct':
>> arch/riscv/kernel/process.c:181:24: error: 'ZVE32X' undeclared (first use in this function)
181 | if (has_vector(ZVE32X))
| ^~~~~~
arch/riscv/kernel/process.c:181:24: note: each undeclared identifier is reported only once for each function it appears in
arch/riscv/kernel/process.c: In function 'copy_thread':
arch/riscv/kernel/process.c:228:24: error: 'ZVE32X' undeclared (first use in this function)
228 | if (has_vector(ZVE32X))
| ^~~~~~
--
arch/riscv/kernel/signal.c: In function 'restore_sigcontext':
>> arch/riscv/kernel/signal.c:191:41: error: 'ZVE32X' undeclared (first use in this function)
191 | if (!has_vector(ZVE32X) || !riscv_v_vstate_query(regs) ||
| ^~~~~~
arch/riscv/kernel/signal.c:191:41: note: each undeclared identifier is reported only once for each function it appears in
arch/riscv/kernel/signal.c: In function 'get_rt_frame_size':
arch/riscv/kernel/signal.c:213:24: error: 'ZVE32X' undeclared (first use in this function)
213 | if (has_vector(ZVE32X)) {
| ^~~~~~
arch/riscv/kernel/signal.c: In function 'setup_sigcontext':
arch/riscv/kernel/signal.c:286:24: error: 'ZVE32X' undeclared (first use in this function)
286 | if (has_vector(ZVE32X) && riscv_v_vstate_query(regs))
| ^~~~~~
--
arch/riscv/kernel/sys_hwprobe.c: In function 'hwprobe_isa_ext0':
>> arch/riscv/kernel/sys_hwprobe.c:72:24: error: 'v' undeclared (first use in this function)
72 | if (has_vector(v))
| ^
arch/riscv/kernel/sys_hwprobe.c:72:24: note: each undeclared identifier is reported only once for each function it appears in
>> arch/riscv/kernel/sys_hwprobe.c:119:32: error: 'ZVE32X' undeclared (first use in this function)
119 | if (has_vector(ZVE32X)) {
| ^~~~~~
--
In file included from kernel/sched/core.c:78:
kernel/sched/core.c: In function 'context_switch':
>> arch/riscv/include/asm/switch_to.h:81:24: error: 'ZVE32X' undeclared (first use in this function)
81 | if (has_vector(ZVE32X)) \
| ^~~~~~
kernel/sched/core.c:5400:9: note: in expansion of macro 'switch_to'
5400 | switch_to(prev, next, prev);
| ^~~~~~~~~
arch/riscv/include/asm/switch_to.h:81:24: note: each undeclared identifier is reported only once for each function it appears in
81 | if (has_vector(ZVE32X)) \
| ^~~~~~
kernel/sched/core.c:5400:9: note: in expansion of macro 'switch_to'
5400 | switch_to(prev, next, prev);
| ^~~~~~~~~


vim +/ZVE32X +181 arch/riscv/kernel/process.c

177
178 void arch_release_task_struct(struct task_struct *tsk)
179 {
180 /* Free the vector context of datap. */
> 181 if (has_vector(ZVE32X))
182 riscv_v_thread_free(tsk);
183 }
184

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki