12.09.2023 00:43, Daniel Henrique Barboza:
On 9/11/23 16:54, Michael Tokarev wrote:
...
/* 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/[email protected]/ 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.
Please see: https://lore.kernel.org/qemu-devel/[email protected]/T/#t (fix v4) https://lore.kernel.org/qemu-devel/[email protected]/T/#t (fix v3) https://lore.kernel.org/qemu-devel/[email protected]/T/#t (fix v2) https://lore.kernel.org/qemu-devel/zp74b%[email protected]/T/#t (fix v1) and the original issue at https://lore.kernel.org/qemu-devel/[email protected]/T/#m65801e9edf31688a45722271a97e628ec98a0c23 (this is an i386 pullreq which removed stub functions in presence of (!kvm_enabled() && foo)). It is exactly the same issue as this one, with exactly the same fix, which resulted in breakage. I dunno why your build succeeded, but the whole thing is.. not easy. /mjt
