riscv: features: Detect extensions always and only through AT_HWCAP#1996
riscv: features: Detect extensions always and only through AT_HWCAP#1996ziyao233 wants to merge 1 commit intozlib-ng:developfrom
Conversation
Previously, RVV/Zbc is used when - Linux kernel version (detected by uname(2)) is greater or equal to 6.5, and HWCAP claims support for such extension, or - Linux kernel version is less than 6.5, but compiler claims support for the extension and for RVV, we additionally check whether it's (the earlier, draft) v0.7.1 release or the (the later, ratified) v1.0 release. Zlib-ng only supports v1.0. This strategy is problematic in several aspects, - Fallback to compile-time constants with Linux kernel earlier than 6.5 makes it hard for distributions to ship the same binary for different machines with different kernel versions. - Detecting Linux kernel version through uname(2) could violate sandbox enforcement[1]. And since RVV introduces special vector registers, RVV usage in userspace relies on the kernel to implement proper context switching, which is only present since Linux kernel 6.5, which is also the version when COMPAT_HWCAP_ISA_V was introduced. This makes it reasonable to only do HWCAP-based extension detection for RVV. The additional check for RVV version is also unnecessary since in mainline Linux kernel, COMPAT_HWCAP_ISA_V is only set when a ratified v1.0 implementation of RVV is present. A downstream kernel claiming COMPAT_HWCAP_ISA_V with a v0.7.1 RVV implementation breaks the kABI, and userspace shouldn't pay for the mistake. This patch removes extra detection logic, now RVV is and only is used when the kernel claims COMPAT_HWCAP_ISA_V. The detection logic of Zbc checks an undefined HWCAP bit, thus doesn't detect Zbc correctly with any current Linux kernel, which should be fixed with another patch. Fixes: 6ff8b52 ("Support RVV hwcap detect at runtime") Link: openssh/openssh-portable#605 # [1] Closes: zlib-ng#1705 Closes: zlib-ng#1769 Signed-off-by: Yao Zi <ziyao@disroot.org>
WalkthroughConsolidated RISC-V CPU feature detection by removing separate compile-time and runtime detection paths, replacing them with a single unified function that detects CPU capabilities solely from hardware capabilities when available, eliminating unsafe fallbacks on older kernels. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant riscv_check_features
participant getauxval
Caller->>riscv_check_features: riscv_check_features(features)
rect rgb(220, 240, 250)
Note over riscv_check_features: Unified detection path
riscv_check_features->>getauxval: getauxval(AT_HWCAP)
end
alt AT_HWCAP available
getauxval-->>riscv_check_features: hw_cap
riscv_check_features->>riscv_check_features: Detect has_rvv, has_zbc
else AT_HWCAP unavailable
getauxval-->>riscv_check_features: 0
riscv_check_features->>riscv_check_features: Default: no RVV/ZBC support
end
riscv_check_features-->>Caller: features populated
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
arch/riscv/riscv_features.c (1)
11-19: Excellent simplification achieving the PR objectives.This unified detection function correctly implements the conservative approach described in the PR:
- Detects features exclusively via
AT_HWCAPwhen available- Defaults to 0 (no features) on older kernels or sandboxed environments, preventing SIGILL
- Eliminates kernel version checks that could violate sandbox enforcement
- Removes compile-time fallbacks that prevented cross-kernel binary portability
The implementation follows the same pattern as
s390_check_featuresand correctly avoids feature-specific compilation flags (per learnings).Optional: Consider adding a null pointer check for defensive coding:
void Z_INTERNAL riscv_check_features(struct riscv_cpu_features *features) { + if (features == NULL) return; + #if defined(__linux__) && defined(HAVE_SYS_AUXV_H) unsigned long hw_cap = getauxval(AT_HWCAP);Note: This is purely defensive since the existing s390 implementation also doesn't include this check.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
arch/riscv/riscv_features.c(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mtl1979
Repo: zlib-ng/zlib-ng PR: 1904
File: arch/riscv/Makefile.in:12-14
Timestamp: 2025-04-15T09:30:10.081Z
Learning: Feature detection modules like riscv_features.c should not be compiled with feature-specific flags (like RVVFLAG) because they need to be compilable on all systems regardless of feature support. These modules perform runtime detection and must initialize feature availability flags to zero on unsupported systems.
Learnt from: Ag-Cu
Repo: zlib-ng/zlib-ng PR: 1905
File: arch/riscv/crc32_riscv.c:0-0
Timestamp: 2025-04-22T07:05:36.825Z
Learning: On RISC-V architectures running Linux, misaligned memory accesses are guaranteed to work for userspace applications as documented in the kernel's UABI specifications. The kernel provides this guarantee by either enabling hardware support for misaligned accesses when available or by using trap handlers to emulate them in software when hardware doesn't support them natively.
Learnt from: Ag-Cu
Repo: zlib-ng/zlib-ng PR: 1905
File: arch/riscv/crc32_riscv.c:0-0
Timestamp: 2025-04-22T07:05:36.825Z
Learning: On RISC-V architectures running Linux, misaligned memory accesses are guaranteed to work for userspace applications as documented in the kernel's UABI specifications. While ISO C considers unaligned pointer casts as undefined behavior, the Linux kernel guarantees support for misaligned loads/stores either through hardware support or kernel emulation (with potential performance impact). This does not apply to atomic operations, which must remain aligned.
Learnt from: mtl1979
Repo: zlib-ng/zlib-ng PR: 1925
File: arch/loongarch/lasxintrin_ext.h:38-65
Timestamp: 2025-07-04T16:59:44.725Z
Learning: For new architectures like LoongArch64, inline assembly fallbacks are necessary because compilers don't yet have intrinsic functions for all common operations. This requires maintaining complex inline assembly implementations until the compiler ecosystem matures.
📚 Learning: 2025-04-15T09:30:10.081Z
Learnt from: mtl1979
Repo: zlib-ng/zlib-ng PR: 1904
File: arch/riscv/Makefile.in:12-14
Timestamp: 2025-04-15T09:30:10.081Z
Learning: Feature detection modules like riscv_features.c should not be compiled with feature-specific flags (like RVVFLAG) because they need to be compilable on all systems regardless of feature support. These modules perform runtime detection and must initialize feature availability flags to zero on unsupported systems.
Applied to files:
arch/riscv/riscv_features.c
📚 Learning: 2025-04-22T07:05:36.825Z
Learnt from: Ag-Cu
Repo: zlib-ng/zlib-ng PR: 1905
File: arch/riscv/crc32_riscv.c:0-0
Timestamp: 2025-04-22T07:05:36.825Z
Learning: On RISC-V architectures running Linux, misaligned memory accesses are guaranteed to work for userspace applications as documented in the kernel's UABI specifications. The kernel provides this guarantee by either enabling hardware support for misaligned accesses when available or by using trap handlers to emulate them in software when hardware doesn't support them natively.
Applied to files:
arch/riscv/riscv_features.c
📚 Learning: 2025-04-22T07:05:36.825Z
Learnt from: Ag-Cu
Repo: zlib-ng/zlib-ng PR: 1905
File: arch/riscv/crc32_riscv.c:0-0
Timestamp: 2025-04-22T07:05:36.825Z
Learning: On RISC-V architectures running Linux, misaligned memory accesses are guaranteed to work for userspace applications as documented in the kernel's UABI specifications. While ISO C considers unaligned pointer casts as undefined behavior, the Linux kernel guarantees support for misaligned loads/stores either through hardware support or kernel emulation (with potential performance impact). This does not apply to atomic operations, which must remain aligned.
Applied to files:
arch/riscv/riscv_features.c
🧬 Code graph analysis (1)
arch/riscv/riscv_features.c (2)
arch/s390/s390_features.c (1)
Z_INTERNAL(12-14)arch/riscv/riscv_features.h (2)
riscv_cpu_features(17-17)riscv_cpu_features(12-15)
🔇 Additional comments (1)
arch/riscv/riscv_features.c (1)
8-8: Based on my investigation, I found that web searches indicate V extension support was still being added to the RISC-V hwcap interface and the RISC-V kernel implementation for hwcap V detection was not yet finalized when that discussion occurred. The calculation(1 << ('v' - 'a'))produces bit 21 mathematically, but verification against the actual kernel implementation is inconclusive from available documentation.Verify ISA_V_HWCAP bit position against the kernel version being used.
Given that RISC-V hwcap V extension support has evolved, confirm that bit 21 matches the kernel's actual COMPAT_HWCAP_ISA_V or equivalent definition in your target kernel version. Verify the code works correctly on target platforms.
| @@ -13,39 +8,7 @@ | |||
| #define ISA_V_HWCAP (1 << ('v' - 'a')) | |||
| #define ISA_ZBC_HWCAP (1 << 29) | |||
There was a problem hiding this comment.
🧩 Analysis chain
ISA_ZBC_HWCAP uses an undefined bit (known issue).
The PR description notes: "Zbc detection currently checks an undefined HWCAP bit and thus does not detect correctly with current kernels; that requires a separate patch." This line will not correctly detect Zbc support until the kernel defines the appropriate hwcap bit and a follow-up patch updates this constant.
Please verify the correct hwcap bit for Zbc once kernel support is available:
🌐 Web query:
What is the correct HWCAP bit definition for RISC-V Zbc extension in Linux kernel?
💡 Result:
The kernel defines the Zbc hwprobe bit as
#define RISCV_HWPROBE_EXT_ZBC (1 << 5).
See the RISC‑V hwprobe docs and the patch adding Zbc support. [1][2]
Sources:
[1] RISC‑V Hardware Probing Interface — Linux kernel docs. [2] "RISC‑V: hwprobe: Expose Zbc ..." (kernel patch discussion).
Update ISA_ZBC_HWCAP to use the correct hwprobe bit value.
The kernel defines the Zbc hwprobe bit as #define RISCV_HWPROBE_EXT_ZBC (1 << 5), but the current code uses (1 << 29), which is undefined and will not detect Zbc support. Change line 9 to:
#define ISA_ZBC_HWCAP (1 << 5)
🤖 Prompt for AI Agents
In arch/riscv/riscv_features.c around line 9, the ISA_ZBC_HWCAP macro uses the
wrong hwprobe bit (1 << 29); change it to use the kernel-defined Zbc hwprobe bit
by setting the macro to (1 << 5) so Zbc support is detected correctly.
There was a problem hiding this comment.
Auxiliary vector HWCAP bits are currently undocumented on RISC-V and separate from newer riscv_hwprobe interface. zlib-ng doesn't have any support for riscv_hwprobe yet. It is likely that any ZBC support has required custom kernel and C library so run-time loader has had required information to fill auxiliary vector.
There was a problem hiding this comment.
Well C library never fills HWCAP, it should be custom kernels that fills it.
HWCAP seems to be not going to support any multi-letter extensions.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
I've tested this change on StarFive JH7110 (without RVV, 6.12 Linux kernel) and SpacemiT K1 (with RVV, 6.6 Linux kernel), and confirmed RVV is detected on K1. Will test it on QEMU with 6.1 kernel later. Cc @chLFF @Icenowy @MingcongBai could you please help test whether sandbox violation in OpenSSH is fixed with this? |
|
Personally I prefer hwprobe only instead of HWCAP only, and we can assume devices stuck to kernels before hwprobe has no fancy extensions at all. |
Agree in long-term, but this requires more code changes than sticking to HWCAP for now, and I'm willing to see the change land in zlib-ng 2.3.0. The runtime detection logic of Zbc has been broken for a long time, and this change doesn't make it worse. |
|
I would like to see the HWCAP feature bit defines pulled from toolchain headers, like we do on other architectures, instead of defining them unconditionally in zlib-ng. Obviously when we still supported older kernels, those defines didn't always exist. This means that in the rare case when the bit numbers are different in different kernels or changed for any reason, the detection still works reliably and correctly. |
|
Well in fact the kernel UAPI of RISC-V only defines hwcaps for the following single letter extensions: All other bits are not defined in UAPI (and no multi-letter extensions are going to enter HWCAP -- they should be probed via hwprobe instead). |
|
There is three layers that all need to support the feature detection:
As I see it, hwprobe falls in between 2 and 3. It's a chicken and egg type of issue where hwcap can't have something that hwprobe or the kernel doesn't have first. The two methods are not alternatives, they are the same, but in different layer. |
|
Well both libc and rtld is not necessary at all. For libc, if the C library is too old, syscall(2) could just be used instead for hwprobe. For rtld, there's nothing about it if you do not use IFUNC (the dispatcher facility of the runtime linker) -- AT_HWCAP is filled by the kernel instead. Both hwprobe and hwcap are kernel interfaces, the libc just wrapped it a bit to make them easy to use (for hwcap, make retriving it easier, for hwprobe, provide the interface definition and wrap it to a function). |
|
Glibc do have some support for dispatching to different .so file paths on some architectures according to HWCAP (e.g. on x86), however this isn't a thing on RISC-V yet; zlib-ng doesn't use IFUNC either, so runtime linker just needs no presence in this context. |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## develop #1996 +/- ##
============================================
- Coverage 83.31% 39.28% -44.03%
============================================
Files 161 73 -88
Lines 12960 7364 -5596
Branches 3149 1308 -1841
============================================
- Hits 10797 2893 -7904
- Misses 1133 4225 +3092
+ Partials 1030 246 -784 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
You are confusing runtime loader and runtime linker (also known as dynamic linker).... Runtime loader caches information about the environment the executable is running as user code doesn't have direct access to kernel structures, user code has to go through C library as that is what defines the user-side of syscalls and the interface to call them. Kernel, C library and runtime loader need to be compatible versions. Proper name for runtime linker is dynamic linker, it just loads required shared libraries and resolves any undefined symbols that each shared library provides. We don't assume that something doesn't have any support for anything, this is exactly why in configure step we test known headers and expected functions in them. Even if both the header exist and there is required function we need, it doesn't mean there is full support as we also need to know what to pass to each of the functions. |
|
Well I don't think a "runtime loader" that complies to your definition exists -- it's either a part of the libc (e.g. as crt*.o) or part of the kernel. |
AT_HWCAP entry, being part of aux vector, is prepared by Linux kernel and placed just beneath envp when a process is started, which makes it essential part of the kernel ABI, and I don't think it's supposed to change. I could understand the opinion to prefer toolchain-provided HWCAP definition, but IMHO it's out of this PR's scope. |
No, Both Anyway, |
Well does musl contains HWCAP definition? In glibc-based toolchains, it could be retrieved by part of the kernel UAPI headers; however I think musl does not like the practice of including kernel UAPI headers in toolchains. |
|
In addition, glibc seems not to suggest usage of HWCAP -- it's used nowhere inside glibc itself, and to get the definitions you need to directly include <asm/hwcap.h> (which is kernel UAPI header) instead of any glibc-provided headers. (On contrary, <sys/hwprobe.h> is provided by Glibc as either a wrapper for kernel UAPI header <asm/hwprobe.h> if the kernel UAPI headers are new enough or Glibc's own fallback) |
Musl doesn't require linux's UAPI header to function, it ships the definitions on its own. Sadly as of musl v1.2.5, no HWCAP bits are defined for RISC-V ports. However, I said
But this would require more conditional-compiled code added in (As a side note, I noted zlib-ng has been using linux uapi headers directly1 for other ports, so it seems okay to do similar things for RISC-V) |
The issue with auxiliary vector HWCAP bit defines is that they must be in sync with the latest supported kernel, so some distributions decided to separate them out of This is pretty much chicken and egg kind of dilemma... We wait and see each part of the operating system to implement something and then other parts need to sync with the changes. When all parts of the operating system have some feature implemented, people can start to standardise it. Auxiliary vector itself is part of the ELF specification, so a lot of it is already standardised in general level, but architecture-specific parts are not part of the ELF specification. Like with hwprobe, the architecture-specific standardisation comes from kernel team, which is pretty much lead by my cousin. |
Do your "distribution" here mean "architecture" instead? I don't see how these things could be customized by distributions w/o breaking API. I cannot find any source, but it seems that RISC-V Linux maintainers decided not to extend user-visible HWCAP bit vectors. (They have internal bit definitions for some further extensions, but these are implementation-detail and intentionally filtered out to prevent being seen by userspace, see https://github.com/torvalds/linux/blob/master/arch/riscv/kernel/cpufeature.c#L1113 ). So we don't need to sync with the kernel any more and it's already a dead end. |
|
FYI, here's a raw auxv dump on a Lichee Pi 4A with 6.17 kernel, along with the hwprobe value of RISCV_HWPROBE_KEY_IMA_EXT_0: |
|
I think that for 2.3.0, we should prioritize a simpler fix that fixes the worst problems, then we can work on and discuss the best way to improve on this later. Lets do that discussion in #1997. 2.3.0 is coming out real soon now, with or without a fix for RiscV, so please lets focus on whether this PR is good enough to improve the situation for the 2.3.0 release. |
No. You can't break something that is undefined in userspace and vendor-specific in kernel. A lot of the features in kernel can be toggled on and off as seen in the kernel sources just few lines later. |
That's exactly what we are doing... The consensus so far leans towards having at least support for hwprobe, but dropping support for auxiliary vector or fixing it in kernel instead remains under discussion. |
I see no PRs for hwprobe, only this one for relying on hwcap. |
This isn't how UAPI of Linux works -- UAPI cannot be toggled on or off, only kernel features can. In fact, when you install UAPI headers, no config file is generated -- see https://www.linuxfromscratch.org/lfs/view/systemd/chapter05/linux-headers.html , where no kernel configuration step is present. In addition, Linux do not provide any header files in /usr/include/sys/ ; these header files are provided by the libc instead. Architecture-specific HWCAP definitions may come in /usr/include/asm/ (this is where all architecture-specific UAPI headers lie) and then get included by something in /usr/include/sys/. |
|
#1999 was merged, closing this. |
Previously, RVV/Zbc is used when
and for RVV, we additionally check whether it's (the earlier, draft) v0.7.1 release or the (the later, ratified) v1.0 release. Zlib-ng only supports v1.0.
This strategy is problematic in several aspects,
And since RVV introduces special vector registers, RVV usage in userspace relies on the kernel to implement proper context switching, which is only present since Linux kernel 6.5, which is also the version when COMPAT_HWCAP_ISA_V was introduced. This makes it reasonable to only do HWCAP-based extension detection for RVV.
The additional check for RVV version is also unnecessary since in mainline Linux kernel, COMPAT_HWCAP_ISA_V is only set when a ratified v1.0 implementation of RVV is present. A downstream kernel claiming COMPAT_HWCAP_ISA_V with a v0.7.1 RVV implementation breaks the kABI, and userspace shouldn't pay for the mistake.
This patch removes extra detection logic, now RVV is and only is used when the kernel claims COMPAT_HWCAP_ISA_V. The detection logic of Zbc checks an undefined HWCAP bit, thus doesn't detect Zbc correctly with any current Linux kernel, which should be fixed with another patch.
Fixes: 6ff8b52 ("Support RVV hwcap detect at runtime")
Link: openssh/openssh-portable#605 # [1]
Closes: #1705
Closes: #1769