On 9/11/23 16:54, Michael Tokarev wrote:
11.09.2023 09:43, Alistair Francis:From: Daniel Henrique Barboza <dbarb...@ventanamicro.com> A build with --enable-debug and without KVM will fail as follows: /usr/bin/ld: libqemu-riscv64-softmmu.fa.p/hw_riscv_virt.c.o: in function `virt_machine_init': ./qemu/build/../hw/riscv/virt.c:1465: undefined reference to `kvm_riscv_aia_create' This happens because the code block with "if virt_use_kvm_aia(s)" isn't being ignored by the debug build, resulting in an undefined reference to a KVM only function. Add a 'kvm_enabled()' conditional together with virt_use_kvm_aia() will make the compiler crop the kvm_riscv_aia_create() call entirely from a non-KVM build. Note that adding the 'kvm_enabled()' conditional inside virt_use_kvm_aia() won't fix the build because this function would need to be inlined multiple times to make the compiler zero out the entire block. While we're at it, use kvm_enabled() in all instances where virt_use_kvm_aia() is checked to allow the compiler to elide these other kvm-only instances as well. Suggested-by: Richard Henderson <richard.hender...@linaro.org> Fixes: dbdb99948e ("target/riscv: select KVM AIA in riscv virt machine")This is probably commit 48c2c33c52, not dbdb99948e. ../* KVM AIA only has one APLIC instance */ - if (virt_use_kvm_aia(s)) { + if (kvm_enabled() && virt_use_kvm_aia(s)) { create_fdt_socket_aplic(s, memmap, 0,... As has been discovered earlier (see "target/i386: Restrict system-specific features from user emulation" threads), this is not enough, - compiler does not always eliminate if (0 && foo) { bar; } construct, it's too fragile and does not actually work. Either some #ifdef'fery is needed here or some other, more explicit, way to eliminate such code, like introducing stub functions. I know it's too late and this change is already in, but unfortunately that's when I noticed this. While the "Fixes:" tag can't be changed anymore, a more proper fix for the actual problem (with the proper Fixes tag now) can still be applied on top of this fix.
This works fine on current master on my machine: $ ../configure --cc=clang --target-list=riscv64-softmmu,riscv64-linux-user,riscv32-softmmu,riscv32-linux-user --enable-debug $ make -j So I'm not sure what do you mean by 'actual problem'. If you refer to the non-existence of stub functions, earlier today we had a review by Phil in this patch here where I was adding stubs for all KVM functions: https://lore.kernel.org/qemu-riscv/f30d8589-8b59-2fd7-c38c-3f79508a4...@linaro.org/ Phil mentioned that we don't need a function stub if the KVM only function is protected by "kvm_enabled()". And this is fine, but then, from what you're saying, we can't rely on (kvm_enabled() && foo) working all the time, so we're one conditional away from breaking things it seems. I think we need to make up our minds and standardize. Do we rely on the compiler to optimize stuff out, or do we use stubs/ifdefs? I'm fine with both options, as long as we pick one and stick with it. Thanks, Daniel
/mjt
