On 7/14/25 8:31 AM, Pierrick Bouvier wrote:
On 7/14/25 6:51 AM, Peter Maydell wrote:
If you try to build aarch64-linux-user with clang and --enable-debug then it
fails to compile:

   ld: libqemu-aarch64-linux-user.a.p/target_arm_cpu64.c.o: in function 
`cpu_arm_set_sve':
   ../../target/arm/cpu64.c:321:(.text+0x1254): undefined reference to 
`kvm_arm_sve_supported'

This is a regression introduced in commit f86d4220, which switched
the kvm-stub.c file away from being built for all arm targets to only
being built for system emulation binaries.  It doesn't affect gcc,
presumably because even at -O0 gcc folds away the always-false
kvm_enabled() condition but clang does not.

We would prefer not to build kvm-stub.c once for usermode and once
for system-emulation binaries, and we can't build it just once for
both because it includes cpu.h.  So instead provide always-false
versions of the five functions that are valid to call without KVM
support in kvm_arm.h.

Fixes: f86d42205c2eba ("target/arm/meson: accelerator files are not needed in user 
mode")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3033
Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
---
I'm never sure when we prefer to use stub-functions in separate C files
vs when we prefer to have ifdeffed stubs in headers. There are several
ways we could fix this compile error, so I just picked one...
---
   target/arm/kvm_arm.h | 35 +++++++++++++++++++++++++++++++++++
   1 file changed, 35 insertions(+)


Thanks Peter, clang with --enable-debug is indeed a combination I didn't
try. I'll test this too now. Going through this topic, yes I noticed
that gcc always folds the any if (0) condition, and, based on a Richard
comment (I can't find now) it seems that we used to rely on that for
other parts of the code.

The fix you propose works well (initial goal was just to remove
CONFIG_KVM, so having CONFIG_USER_ONLY is ok), but I wonder if there is
something specific affecting clang in this case and preventing the folding.


Indeed, clang does not fold the condition "value && kvm_enabled() && !kvm_arm_sve_supported()". Looks like a missing case.
This code compiles with gcc -O0, but not clang -O0.

extern int f(void);
int main(int argc) {
    if (argc && 0)
        f();
}

As folding is not guaranteed by C standard, I'm not sure it's really possible to file a bug. However, since we rely on this behaviour in other parts, maybe it would be better to rewrite the condition on our side.

By changing the code to this, the folding happens as expected.

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 26cf7e6dfa2..af5788dafab 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -318,9 +318,11 @@ static void cpu_arm_set_sve(Object *obj, bool value, Error **errp)
 {
     ARMCPU *cpu = ARM_CPU(obj);

-    if (value && kvm_enabled() && !kvm_arm_sve_supported()) {
- error_setg(errp, "'sve' feature not supported by KVM on this host");
-        return;
+    if (value) {
+        if (kvm_enabled() && !kvm_arm_sve_supported()) {
+ error_setg(errp, "'sve' feature not supported by KVM on this host");
+            return;
+        }
     }

     FIELD_DP64_IDREG(&cpu->isar, ID_AA64PFR0, SVE, value);

If you prefer keeping your patch, I'm ok, but fixing the condition looks better to me (as we already rely on constant folding in other places).

Reviewed-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>

Thanks,
Pierrick


Reply via email to