RVV support enabled accidentally and leads to SIGILL on older kernels

[xctan]

PR https://github.com/zlib-ng/zlib-ng/pull/1585 has added runtime detection of RISC-V vector support for kernels newer than or equal to 6.5, and if the kernel is too old, zlib-ng would use compile time compiler support as fallback. However, this behavior is not safe for older kernels in conjunction with new compilers. In my case, I was using Linux 6.1.61 and GCC 13.2.1 when trying to compile starship 1.18.1 with default features enabled, including zlib-ng. Some git test cases of starship would fail because of the SIGILL signal coming from adler32_rvv.

I suggest using a more conservative assumption about runtime support of RVV when hwcap is not available. When the kernel is too old, or hwcap is disabled by the sandbox mechanism, RVV support should be disabled.

[mtl1979]

We enable everything by default if we can't detect availability of specific instructions... This is to encourage people to upgrade to at least oldest usable kernel and C library versions... Flipping the default to not enable everything would require the change for all architectures. We still allow disabling every optimisation to make it possible to compile for older kernels.

[ncopa]

I bumped into this on Alpine Linux with the Mlik-v pioneer machine, with sophgo 2042 and RVV 0.7.1.

In alpine we have previously bumped into same issue with ffmpeg and highway.

[ncopa]

We are using this workaround based on the fix from highway

diff --git a/arch/riscv/riscv_features.c b/arch/riscv/riscv_features.c
index b066f42..259a63a 100644
--- a/arch/riscv/riscv_features.c
+++ b/arch/riscv/riscv_features.c
@@ -42,4 +42,20 @@ void Z_INTERNAL riscv_check_features(struct riscv_cpu_features *features) {
         riscv_check_features_runtime(features);
     else
         riscv_check_features_compile_time(features);
+    if (features->has_rvv) {
+       size_t e8m1_vec_len;
+       int64_t vtype_reg_val;
+       // Check that a vuint8m1_t vector is at least 16 bytes and that tail
+       // agnostic and mask agnostic mode are supported
+       //
+       __asm__ volatile(
+               "vsetvli %0, zero, e8, m1, ta, ma\n\t"
+               "csrr %1, vtype"
+               : "=r"(e8m1_vec_len), "=r"(vtype_reg_val));
+
+       // The RVV target is supported if the VILL bit of VTYPE (the MSB bit of
+       // VTYPE) is not set and the length of a vuint8m1_t vector is at least 16
+       // bytes
+       features->has_rvv = (vtype_reg_val >= 0 && e8m1_vec_len >= 16);
+    }
 }

For 32 bit the vtype_reg_val needs to be int32_t, but I was lazy since Alpine only supports 64 bit riscv.

[mtl1979]

We are using this workaround based on the fix from highway

diff --git a/arch/riscv/riscv_features.c b/arch/riscv/riscv_features.c
index b066f42..259a63a 100644
--- a/arch/riscv/riscv_features.c
+++ b/arch/riscv/riscv_features.c
@@ -42,4 +42,20 @@ void Z_INTERNAL riscv_check_features(struct riscv_cpu_features *features) {
         riscv_check_features_runtime(features);
     else
         riscv_check_features_compile_time(features);
+    if (features->has_rvv) {
+       size_t e8m1_vec_len;
+       int64_t vtype_reg_val;
+       // Check that a vuint8m1_t vector is at least 16 bytes and that tail
+       // agnostic and mask agnostic mode are supported
+       //
+       __asm__ volatile(
+               "vsetvli %0, zero, e8, m1, ta, ma\n\t"
+               "csrr %1, vtype"
+               : "=r"(e8m1_vec_len), "=r"(vtype_reg_val));
+
+       // The RVV target is supported if the VILL bit of VTYPE (the MSB bit of
+       // VTYPE) is not set and the length of a vuint8m1_t vector is at least 16
+       // bytes
+       features->has_rvv = (vtype_reg_val >= 0 && e8m1_vec_len >= 16);
+    }
 }

For 32 bit the vtype_reg_val needs to be int32_t, but I was lazy since Alpine only supports 64 bit riscv.

When the type depends on if the target is 32-bit or 64-bit, the variable type should be intptr_t, which maps on modern compilers to correct type depending on the target.

You should make a pull request with the patch, so it can be reviewed by contributors who use RISC-V.

[mtl1979]

@alexsifivetw Can you check the patch above, if it can be cleaned up for zlib-ng?

[alexsifivetw]

Hi @xctan If you know that your env does not support RVV, you can try setting WITH_RVV=OFF. Please also check the RAEDME.

[alexsifivetw]

diff --git a/arch/riscv/riscv_features.c b/arch/riscv/riscv_features.c
index b066f42..259a63a 100644
--- a/arch/riscv/riscv_features.c
+++ b/arch/riscv/riscv_features.c
@@ -42,4 +42,20 @@ void Z_INTERNAL riscv_check_features(struct riscv_cpu_features *features) {
         riscv_check_features_runtime(features);
     else
         riscv_check_features_compile_time(features);
+    if (features->has_rvv) {
+       size_t e8m1_vec_len;
+       int64_t vtype_reg_val;
+       // Check that a vuint8m1_t vector is at least 16 bytes and that tail
+       // agnostic and mask agnostic mode are supported
+       //
+       __asm__ volatile(
+               "vsetvli %0, zero, e8, m1, ta, ma\n\t"
+               "csrr %1, vtype"
+               : "=r"(e8m1_vec_len), "=r"(vtype_reg_val));
+
+       // The RVV target is supported if the VILL bit of VTYPE (the MSB bit of
+       // VTYPE) is not set and the length of a vuint8m1_t vector is at least 16
+       // bytes
+       features->has_rvv = (vtype_reg_val >= 0 && e8m1_vec_len >= 16);
+    }
 }

@ncopa This patch looks useful. Could you make a PR with the patch? It's more convenient for discussion and allows RISC-V users to focus on it.

[alexsifivetw]

I'm not familiar with Rust and Cargo. It will install a prebuilt version, right? Could you specify the no-RVV version in Cargo? https://github.com/starship/starship/blob/37fba4cfb706533988404255e6226f37cdab13f9/Cargo.toml#L39