| Message ID | 20190202144531.5772-1-n54@gmx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target-i386: Enhance the stub for kvm_arch_get_supported_cpuid() | expand |
Patchew URL: https://patchew.org/QEMU/20190202144531.5772-1-n54@gmx.com/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash time make docker-test-mingw@fedora SHOW_ENV=1 J=14 === TEST SCRIPT END === Configure options: --enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install --python=/usr/bin/python3 --cross-prefix=x86_64-w64-mingw32- --enable-trace-backends=simple --enable-gnutls --enable-nettle --enable-curl --enable-vnc --enable-bzip2 --enable-guest-agent --with-sdlabi=2.0 ERROR: unknown option --with-sdlabi=2.0 Try '/tmp/qemu-test/src/configure --help' for more information # QEMU configure log Sun Feb 3 17:31:44 UTC 2019 # Configured with: '/tmp/qemu-test/src/configure' '--enable-werror' '--target-list=x86_64-softmmu,aarch64-softmmu' '--prefix=/tmp/qemu-test/install' '--python=/usr/bin/python3' '--cross-prefix=x86_64-w64-mingw32-' '--enable-trace-backends=simple' '--enable-gnutls' '--enable-nettle' '--enable-curl' '--enable-vnc' '--enable-bzip2' '--enable-guest-agent' '--with-sdlabi=2.0' --- funcs: do_compiler do_cc compile_object check_define main lines: 92 122 617 634 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c config-temp/qemu-conf.c:2:2: error: #error __linux__ not defined #error __linux__ not defined ^~~~~ --- funcs: do_compiler do_cc compile_object check_define main lines: 92 122 617 686 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c config-temp/qemu-conf.c:2:2: error: #error __i386__ not defined #error __i386__ not defined ^~~~~ --- funcs: do_compiler do_cc compile_object check_define main lines: 92 122 617 689 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c config-temp/qemu-conf.c:2:2: error: #error __ILP32__ not defined #error __ILP32__ not defined ^~~~~ --- lines: 92 128 920 0 x86_64-w64-mingw32-gcc -mthreads -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -o config-temp/qemu-conf.exe config-temp/qemu-conf.c -g -liberty /usr/lib/gcc/x86_64-w64-mingw32/8.2.0/../../../../x86_64-w64-mingw32/bin/ld: cannot find -liberty collect2: error: ld returned 1 exit status Failed to run 'configure' Traceback (most recent call last): File "./tests/docker/docker.py", line 563, in <module> The full log is available at http://patchew.org/logs/20190202144531.5772-1-n54@gmx.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
Ping? On 02.02.2019 15:45, Kamil Rytarowski wrote: > This improves the commit: > "target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()" > r. 2140cfa51d59177815f5b82e94ac48fb24909aba > > Clang/LLVM on NetBSD with enabled optimization cannot link > correct qemu program because of a missing symbol of > kvm_arch_get_supported_cpuid() in kvm-stubs.o used by executables. > > There are more than a single one kvm-stub.c and several types > of possible programs such as bsd-user ones. the previous workaround > does not work reliably for all use-cases. Instead of reworking > the stubs and linking rules, move the workaround from a code that > depends on the __OPTIMIZE__ builtin compiler flag, build option (KVM), > compiler and arrangement of linking rules to a simple macro in a > shared header with all the users that defines fallback dummy > implementation, ignoring whether it is optimized out or not. > > Signed-off-by: Kamil Rytarowski <n54@gmx.com> > --- > include/sysemu/kvm.h | 13 +++++++++++++ > target/i386/kvm-stub.c | 10 ---------- > 2 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index a6d1cd190f..93d3c0f0b3 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -459,8 +459,21 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension); > kvm_vcpu_ioctl(cpu, KVM_ENABLE_CAP, &cap); \ > }) > > +#ifdef CONFIG_KVM > uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, > uint32_t index, int reg); > +#else > +/* > + * This function is only called inside conditionals which we > + * rely on the compiler to optimize out when CONFIG_KVM is not > + * defined. > + */ > +#define kvm_arch_get_supported_cpuid(a, b, c, d) \ > + ({ \ > + abort(); \ > + 0; \ > + }) > +#endif > uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index); > > > diff --git a/target/i386/kvm-stub.c b/target/i386/kvm-stub.c > index e7a673e5db..9ce8566700 100644 > --- a/target/i386/kvm-stub.c > +++ b/target/i386/kvm-stub.c > @@ -29,16 +29,6 @@ bool kvm_enable_x2apic(void) > { > return false; > } > - > -/* This function is only called inside conditionals which we > - * rely on the compiler to optimize out when CONFIG_KVM is not > - * defined. > - */ > -uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, > - uint32_t index, int reg) > -{ > - abort(); > -} > #endif > > bool kvm_hv_vpindex_settable(void) >
On 02/02/19 15:45, Kamil Rytarowski wrote: > > Clang/LLVM on NetBSD with enabled optimization cannot link > correct qemu program because of a missing symbol of > kvm_arch_get_supported_cpuid() in kvm-stubs.o used by executables. Can you please include the full error message? Usually these things are a sign of a bug elsewhere. Paolo
On 14.02.2019 19:44, Paolo Bonzini wrote: > On 02/02/19 15:45, Kamil Rytarowski wrote: >> >> Clang/LLVM on NetBSD with enabled optimization cannot link >> correct qemu program because of a missing symbol of >> kvm_arch_get_supported_cpuid() in kvm-stubs.o used by executables. > > Can you please include the full error message? Usually these things are > a sign of a bug elsewhere. > > Paolo > Please do replace the current kludge that is sensitive to: - compiler behavior that can change with new versions - compiler gcc/clang - optimization options - linux(KVM) - non-linux (no-KVM) build - community not actively testing non-linux no-kvm build with optimization on clang My patch replaced it makes it work. Build error: LINK i386-bsd-user/qemu-i386 /usr/bin/ld: /usr/lib/libc.so and /usr/lib/crt0.o: warning: multiple common of `environ' /usr/bin/ld: target/i386/cpu.o: in function `x86_cpu_filter_features': /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5047: undefined reference to `kvm_arch_get_supported_cpuid' /usr/bin/ld: /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5048: undefined reference to `kvm_arch_get_supported_cpuid' /usr/bin/ld: /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5049: undefined reference to `kvm_arch_get_supported_cpuid' /usr/bin/ld: /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5050: undefined reference to `kvm_arch_get_supported_cpuid' /usr/bin/ld: /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5051: undefined reference to `kvm_arch_get_supported_cpuid' clang-9: error: linker command failed with exit code 1 (use -v to see invocation) make[1]: *** [Makefile:199: qemu-i386] Error 1 gmake: *** [Makefile:483: subdir-i386-bsd-user] Error 2 gmake: *** Waiting for unfinished jobs.... LINK x86_64-bsd-user/qemu-x86_64 /usr/bin/ld: /usr/lib/libc.so and /usr/lib/crt0.o: warning: multiple common of `environ' /usr/bin/ld: target/i386/cpu.o: in function `x86_cpu_filter_features': /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5047: undefined reference to `kvm_arch_get_supported_cpuid' /usr/bin/ld: /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5048: undefined reference to `kvm_arch_get_supported_cpuid' /usr/bin/ld: /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5049: undefined reference to `kvm_arch_get_supported_cpuid' /usr/bin/ld: /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5050: undefined reference to `kvm_arch_get_supported_cpuid' /usr/bin/ld: /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5051: undefined reference to `kvm_arch_get_supported_cpuid' clang-9: error: linker command failed with exit code 1 (use -v to see invocation) make[1]: *** [Makefile:199: qemu-x86_64] Error 1 gmake: *** [Makefile:483: subdir-x86_64-bsd-user] Error 2 *** Error code 2
On 14/02/19 20:41, Kamil Rytarowski wrote: > Please do replace the current kludge that is sensitive to: > - compiler behavior that can change with new versions > - compiler gcc/clang > - optimization options Not really, any half-decent compiler will optimize away "if (0)" and QEMU is far from being the only software that relies on that. GCC has been doing that even at -O0 for like 15 years, at some point it was basically the only optimization it did. Just try it for yourself: int f(void); int main() { if (0) return f(); else return 0; } Throw it at all compilers and optimization levels, and it *will* work. If it doesn't then I'll consider again your patch. > - linux(KVM) - non-linux (no-KVM) build That's the point. We want your non-Linux non-KVM build to be as lean as possible and not cause possible run-time failures due to people forgetting about them. > - community not actively testing non-linux no-kvm build with > optimization on clang False, we test OS X and there are VM builds for the BSDs. > My patch replaced it makes it work. > > Build error: > > LINK i386-bsd-user/qemu-i386 Ok, please use "make -C i386-bsd-user target/i386/cpu.o V=1" to get the command line, invoke it again with "-save-temps" at the end, and send me both the command line and the resulting "cpu.i" file. Paolo > /usr/bin/ld: /usr/lib/libc.so and /usr/lib/crt0.o: warning: multiple > common of `environ' > /usr/bin/ld: target/i386/cpu.o: in function `x86_cpu_filter_features': > /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5047: > undefined reference to `kvm_arch_get_supported_cpuid' > /usr/bin/ld: > /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5048: > undefined reference to `kvm_arch_get_supported_cpuid' > /usr/bin/ld: > /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5049: > undefined reference to `kvm_arch_get_supported_cpuid' > /usr/bin/ld: > /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5050: > undefined reference to `kvm_arch_get_supported_cpuid' > /usr/bin/ld: > /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5051: > undefined reference to `kvm_arch_get_supported_cpuid' > clang-9: error: linker command failed with exit code 1 (use -v to see > invocation) > make[1]: *** [Makefile:199: qemu-i386] Error 1 > gmake: *** [Makefile:483: subdir-i386-bsd-user] Error 2 > gmake: *** Waiting for unfinished jobs.... > LINK x86_64-bsd-user/qemu-x86_64 > /usr/bin/ld: /usr/lib/libc.so and /usr/lib/crt0.o: warning: multiple > common of `environ' > /usr/bin/ld: target/i386/cpu.o: in function `x86_cpu_filter_features': > /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5047: > undefined reference to `kvm_arch_get_supported_cpuid' > /usr/bin/ld: > /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5048: > undefined reference to `kvm_arch_get_supported_cpuid' > /usr/bin/ld: > /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5049: > undefined reference to `kvm_arch_get_supported_cpuid' > /usr/bin/ld: > /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5050: > undefined reference to `kvm_arch_get_supported_cpuid' > /usr/bin/ld: > /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5051: > undefined reference to `kvm_arch_get_supported_cpuid' > clang-9: error: linker command failed with exit code 1 (use -v to see > invocation) > make[1]: *** [Makefile:199: qemu-x86_64] Error 1 > gmake: *** [Makefile:483: subdir-x86_64-bsd-user] Error 2 > *** Error code 2 >
On 14.02.2019 21:51, Paolo Bonzini wrote: > On 14/02/19 20:41, Kamil Rytarowski wrote: >> Please do replace the current kludge that is sensitive to: >> - compiler behavior that can change with new versions >> - compiler gcc/clang >> - optimization options > > Not really, any half-decent compiler will optimize away "if (0)" and > QEMU is far from being the only software that relies on that. > > GCC has been doing that even at -O0 for like 15 years, at some point it > was basically the only optimization it did. Just try it for yourself: > > int f(void); > > int main() > { > if (0) > return f(); > else > return 0; > } > > Throw it at all compilers and optimization levels, and it *will* work. > If it doesn't then I'll consider again your patch. > I consider it as fragile hack and certainly not something to depend on. Also in some circumstances of such code, especially "if (zero0)" we want to enable disabled code under a debugger. There were also kernel backdoors due to this optimization. >> - linux(KVM) - non-linux (no-KVM) build > > That's the point. We want your non-Linux non-KVM build to be as lean as > possible and not cause possible run-time failures due to people > forgetting about them. > >> - community not actively testing non-linux no-kvm build with >> optimization on clang > > False, we test OS X and there are VM builds for the BSDs. Unfortunately not in the same combination of options as nobody caught it in years. (Probably not many people actually develop it on these OSes with debug flags). I was keeping this patch locally for some time now. This hack was introduced several years ago. >> My patch replaced it makes it work. >> >> Build error: >> >> LINK i386-bsd-user/qemu-i386 > > Ok, please use "make -C i386-bsd-user target/i386/cpu.o V=1" to get the > command line, invoke it again with "-save-temps" at the end, and send me > both the command line and the resulting "cpu.i" file. > I'm building qemu with pkgsrc that provides all the dependencies and compiler settings. It also uses wrappers to translate original compiler options with transformed ones. Log from pkgsrc with command lines: http://netbsd.org/~kamil/qemu/qemu-build-2019-02-14.txt.bz2 Requested cpu.i (hopefully correctly generated) http://netbsd.org/~kamil/qemu/cpu.i.bz2 I've generated it manually with this command. /usr/local/bin/clang -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/target/i386 -iquote target/i386 -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/tcg -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/tcg/i386 -iquote . -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0 -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/accel/tcg -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/include -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.x11-buildlink/include/pixman-1 -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/dtc/libfdt -pthread -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/include/glib/glib-2.0 -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/lib/glib-2.0/include -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/include -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wno-error=address-of-packed-member -Wno-string-plus-int -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/include/libpng16 -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/capstone/include -iquote .. -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/target/i386 -DNEED_CPU_H -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/include -MMD -MP -MT target/i386/cpu.o -MF target/i386/cpu.d -O2 -g -O2 -O0 -g -ggdb -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/include/SDL2 -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.x11-buildlink/include -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.x11-buildlink/include/libdrm -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/include/glib/gio-unix-2.0 -I/usr/include/krb5 -c -o target/i386/cpu.o /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/target/i386/cpu.c -Qunused-arguments -fstack-protector -save-temps > Paolo > >> /usr/bin/ld: /usr/lib/libc.so and /usr/lib/crt0.o: warning: multiple >> common of `environ' >> /usr/bin/ld: target/i386/cpu.o: in function `x86_cpu_filter_features': >> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5047: >> undefined reference to `kvm_arch_get_supported_cpuid' >> /usr/bin/ld: >> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5048: >> undefined reference to `kvm_arch_get_supported_cpuid' >> /usr/bin/ld: >> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5049: >> undefined reference to `kvm_arch_get_supported_cpuid' >> /usr/bin/ld: >> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5050: >> undefined reference to `kvm_arch_get_supported_cpuid' >> /usr/bin/ld: >> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5051: >> undefined reference to `kvm_arch_get_supported_cpuid' >> clang-9: error: linker command failed with exit code 1 (use -v to see >> invocation) >> make[1]: *** [Makefile:199: qemu-i386] Error 1 >> gmake: *** [Makefile:483: subdir-i386-bsd-user] Error 2 >> gmake: *** Waiting for unfinished jobs.... >> LINK x86_64-bsd-user/qemu-x86_64 >> /usr/bin/ld: /usr/lib/libc.so and /usr/lib/crt0.o: warning: multiple >> common of `environ' >> /usr/bin/ld: target/i386/cpu.o: in function `x86_cpu_filter_features': >> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5047: >> undefined reference to `kvm_arch_get_supported_cpuid' >> /usr/bin/ld: >> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5048: >> undefined reference to `kvm_arch_get_supported_cpuid' >> /usr/bin/ld: >> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5049: >> undefined reference to `kvm_arch_get_supported_cpuid' >> /usr/bin/ld: >> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5050: >> undefined reference to `kvm_arch_get_supported_cpuid' >> /usr/bin/ld: >> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5051: >> undefined reference to `kvm_arch_get_supported_cpuid' >> clang-9: error: linker command failed with exit code 1 (use -v to see >> invocation) >> make[1]: *** [Makefile:199: qemu-x86_64] Error 1 >> gmake: *** [Makefile:483: subdir-x86_64-bsd-user] Error 2 >> *** Error code 2 >> > >
Ping, still valid. On 15.02.2019 00:38, Kamil Rytarowski wrote: > On 14.02.2019 21:51, Paolo Bonzini wrote: >> On 14/02/19 20:41, Kamil Rytarowski wrote: >>> Please do replace the current kludge that is sensitive to: >>> - compiler behavior that can change with new versions >>> - compiler gcc/clang >>> - optimization options >> >> Not really, any half-decent compiler will optimize away "if (0)" and >> QEMU is far from being the only software that relies on that. >> >> GCC has been doing that even at -O0 for like 15 years, at some point it >> was basically the only optimization it did. Just try it for yourself: >> >> int f(void); >> >> int main() >> { >> if (0) >> return f(); >> else >> return 0; >> } >> >> Throw it at all compilers and optimization levels, and it *will* work. >> If it doesn't then I'll consider again your patch. >> > > I consider it as fragile hack and certainly not something to depend on. > Also in some circumstances of such code, especially "if (zero0)" we want > to enable disabled code under a debugger. > > There were also kernel backdoors due to this optimization. > >>> - linux(KVM) - non-linux (no-KVM) build >> >> That's the point. We want your non-Linux non-KVM build to be as lean as >> possible and not cause possible run-time failures due to people >> forgetting about them. >> >>> - community not actively testing non-linux no-kvm build with >>> optimization on clang >> >> False, we test OS X and there are VM builds for the BSDs. > > Unfortunately not in the same combination of options as nobody caught it > in years. (Probably not many people actually develop it on these OSes > with debug flags). I was keeping this patch locally for some time now. > This hack was introduced several years ago. > >>> My patch replaced it makes it work. >>> >>> Build error: >>> >>> LINK i386-bsd-user/qemu-i386 >> >> Ok, please use "make -C i386-bsd-user target/i386/cpu.o V=1" to get the >> command line, invoke it again with "-save-temps" at the end, and send me >> both the command line and the resulting "cpu.i" file. >> > > I'm building qemu with pkgsrc that provides all the dependencies and > compiler settings. It also uses wrappers to translate original compiler > options with transformed ones. > > Log from pkgsrc with command lines: > > http://netbsd.org/~kamil/qemu/qemu-build-2019-02-14.txt.bz2 > > Requested cpu.i (hopefully correctly generated) > > http://netbsd.org/~kamil/qemu/cpu.i.bz2 > > I've generated it manually with this command. > > /usr/local/bin/clang -iquote > /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/target/i386 -iquote > target/i386 -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/tcg > -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/tcg/i386 -iquote . > -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0 -iquote > /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/accel/tcg -iquote > /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/include > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.x11-buildlink/include/pixman-1 > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/dtc/libfdt -pthread > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/include/glib/glib-2.0 > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/lib/glib-2.0/include > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/include -m64 -mcx16 > -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE > -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings > -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv > -Wno-error=address-of-packed-member -Wno-string-plus-int > -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels > -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body > -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self > -Wignored-qualifiers -Wold-style-definition -Wtype-limits > -fstack-protector-strong > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/include/libpng16 > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/capstone/include -iquote > .. -iquote /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/target/i386 > -DNEED_CPU_H -iquote > /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/include -MMD -MP -MT > target/i386/cpu.o -MF target/i386/cpu.d -O2 -g -O2 -O0 -g -ggdb > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/include/SDL2 > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.x11-buildlink/include > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.x11-buildlink/include/libdrm > -I/tmp/pkgsrc-tmp/wip/qemu-haxm/work/.buildlink/include/glib/gio-unix-2.0 > -I/usr/include/krb5 -c -o target/i386/cpu.o > /tmp/pkgsrc-tmp/wip/qemu-haxm/work/qemu-3.0.0/target/i386/cpu.c > -Qunused-arguments -fstack-protector -save-temps > >> Paolo >> >>> /usr/bin/ld: /usr/lib/libc.so and /usr/lib/crt0.o: warning: multiple >>> common of `environ' >>> /usr/bin/ld: target/i386/cpu.o: in function `x86_cpu_filter_features': >>> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5047: >>> undefined reference to `kvm_arch_get_supported_cpuid' >>> /usr/bin/ld: >>> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5048: >>> undefined reference to `kvm_arch_get_supported_cpuid' >>> /usr/bin/ld: >>> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5049: >>> undefined reference to `kvm_arch_get_supported_cpuid' >>> /usr/bin/ld: >>> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5050: >>> undefined reference to `kvm_arch_get_supported_cpuid' >>> /usr/bin/ld: >>> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5051: >>> undefined reference to `kvm_arch_get_supported_cpuid' >>> clang-9: error: linker command failed with exit code 1 (use -v to see >>> invocation) >>> make[1]: *** [Makefile:199: qemu-i386] Error 1 >>> gmake: *** [Makefile:483: subdir-i386-bsd-user] Error 2 >>> gmake: *** Waiting for unfinished jobs.... >>> LINK x86_64-bsd-user/qemu-x86_64 >>> /usr/bin/ld: /usr/lib/libc.so and /usr/lib/crt0.o: warning: multiple >>> common of `environ' >>> /usr/bin/ld: target/i386/cpu.o: in function `x86_cpu_filter_features': >>> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5047: >>> undefined reference to `kvm_arch_get_supported_cpuid' >>> /usr/bin/ld: >>> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5048: >>> undefined reference to `kvm_arch_get_supported_cpuid' >>> /usr/bin/ld: >>> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5049: >>> undefined reference to `kvm_arch_get_supported_cpuid' >>> /usr/bin/ld: >>> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5050: >>> undefined reference to `kvm_arch_get_supported_cpuid' >>> /usr/bin/ld: >>> /tmp/pkgsrc-tmp/emulators/qemu/work/qemu-3.1.0/target/i386/cpu.c:5051: >>> undefined reference to `kvm_arch_get_supported_cpuid' >>> clang-9: error: linker command failed with exit code 1 (use -v to see >>> invocation) >>> make[1]: *** [Makefile:199: qemu-x86_64] Error 1 >>> gmake: *** [Makefile:483: subdir-x86_64-bsd-user] Error 2 >>> *** Error code 2 >>> >> >> > >
On 20/02/19 12:59, Kamil Rytarowski wrote: > Ping, still valid. Sorry, I missed your email. > On 15.02.2019 00:38, Kamil Rytarowski wrote: >> I consider it as fragile hack and certainly not something to depend on. >> Also in some circumstances of such code, especially "if (zero0)" we want >> to enable disabled code under a debugger. That's a good objection, but certainly does not apply to KVM on NetBSD. >> There were also kernel backdoors due to this optimization. Citation please? >> Requested cpu.i (hopefully correctly generated) >> >> http://netbsd.org/~kamil/qemu/cpu.i.bz2 So, first thing first I can reproduce clang's behavior with this .i file and also with this reduced test case. extern void f(void); int i, j; int main() { if (0 && i) f(); if (j && 0) f(); } The first is eliminated but the second is not, just like in QEMU where this works: if (kvm_enabled() && cpu->enable_pmu) { KVMState *s = cs->kvm_state; *eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX); *ebx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EBX); *ecx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_ECX); *edx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EDX); } else if (hvf_enabled() && cpu->enable_pmu) { *eax = hvf_get_supported_cpuid(0xA, count, R_EAX); *ebx = hvf_get_supported_cpuid(0xA, count, R_EBX); *ecx = hvf_get_supported_cpuid(0xA, count, R_ECX); *edx = hvf_get_supported_cpuid(0xA, count, R_EDX); while this doesn't: if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) && kvm_enabled()) { KVMState *s = CPU(cpu)->kvm_state; uint32_t eax_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EAX); uint32_t ebx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EBX); uint32_t ecx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_ECX); uint32_t eax_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EAX); uint32_t ebx_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EBX); But, that's okay, it's -O0 so we give clang a pass for that Note that clang does do the optimization even in more complex cases like extern _Bool f(void); int main() { if (!0) return 0; if (!f()) return 0; } The problem is that there is a kvm-stub.c entry for that, and in fact my compilation passes and the symbol is resolved correctly: $ nm target/i386/cpu.o |grep kvm_.*get_sup U kvm_arch_get_supported_cpuid $ nm target/i386/kvm-stub.o|grep kvm_.*get_sup 0000000000000030 T kvm_arch_get_supported_cpuid $ nm qemu-system-x86_64 |grep kvm_.*get_sup 000000000046eab0 T kvm_arch_get_supported_cpuid As expected, something much less obvious is going on for you, in particular __OPTIMIZE__seems not to be working properly. However, that would also be very surprising. Please: 1) run the last two "nm" commands on your build (wthout grep). 2) do the same exercise to get a .i for target/i386/kvm-stub.c 3) try removing the "#ifndef __OPTIMIZE__" and leave everything else as is, see if it works. No need to play with macros, which also goes to show that you didn't really understand what's going on---that's fine, but then please refrain from making summary judgments which only lengthen the discussion. Thanks, Paolo
On 20.02.2019 18:29, Paolo Bonzini wrote: > On 20/02/19 12:59, Kamil Rytarowski wrote: >> Ping, still valid. > > Sorry, I missed your email. > >> On 15.02.2019 00:38, Kamil Rytarowski wrote: >>> I consider it as fragile hack and certainly not something to depend on. >>> Also in some circumstances of such code, especially "if (zero0)" we want >>> to enable disabled code under a debugger. > > That's a good objection, but certainly does not apply to KVM on NetBSD. > There is KVM for Darwin (experimental and rather toy project) and it might be ported to NetBSD (I have actually forked it on GitHub recently), but I doubt that someone would enable KVM on any platform under a debugger this way and expect something to work. >>> There were also kernel backdoors due to this optimization. > > Citation please? > I saw an exploit for such case with a .txt writeup on ftp of grsecurity but that service seems to be gone (probably long time ago), so please defer discussion on it. If someone is interested to find it out, there are enough pointers to dig it (assuming that this is still possible). >>> Requested cpu.i (hopefully correctly generated) >>> >>> http://netbsd.org/~kamil/qemu/cpu.i.bz2 > > So, first thing first I can reproduce clang's behavior with this .i file > and also with this reduced test case. > > extern void f(void); > int i, j; > int main() > { > if (0 && i) f(); > if (j && 0) f(); > } > > The first is eliminated but the second is not, just like in QEMU where > this works: > > if (kvm_enabled() && cpu->enable_pmu) { > KVMState *s = cs->kvm_state; > > *eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX); > *ebx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EBX); > *ecx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_ECX); > *edx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EDX); > } else if (hvf_enabled() && cpu->enable_pmu) { > *eax = hvf_get_supported_cpuid(0xA, count, R_EAX); > *ebx = hvf_get_supported_cpuid(0xA, count, R_EBX); > *ecx = hvf_get_supported_cpuid(0xA, count, R_ECX); > *edx = hvf_get_supported_cpuid(0xA, count, R_EDX); > > while this doesn't: > > if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) && > kvm_enabled()) { > KVMState *s = CPU(cpu)->kvm_state; > uint32_t eax_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EAX); > uint32_t ebx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EBX); > uint32_t ecx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_ECX); > uint32_t eax_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EAX); > uint32_t ebx_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EBX); > > But, that's okay, it's -O0 so we give clang a pass for that Note that > clang does do the optimization even in more complex cases like > > extern _Bool f(void); > int main() > { > if (!0) return 0; > if (!f()) return 0; > } > > The problem is that there is a kvm-stub.c entry for that, and in fact > my compilation passes and the symbol is resolved correctly: > > $ nm target/i386/cpu.o |grep kvm_.*get_sup > U kvm_arch_get_supported_cpuid > $ nm target/i386/kvm-stub.o|grep kvm_.*get_sup > 0000000000000030 T kvm_arch_get_supported_cpuid > $ nm qemu-system-x86_64 |grep kvm_.*get_sup > 000000000046eab0 T kvm_arch_get_supported_cpuid > > As expected, something much less obvious is going on for you, in > particular __OPTIMIZE__seems not to be working properly. However, > that would also be very surprising. > > Please: > > 1) run the last two "nm" commands on your build (wthout grep). > I cannot run nm(1) on qemu-system-x86_64 as it's not linkable. I'm getting the same result for target/i386/cpu.o and target/i386/kvm-stub.o. $ nm ./i386-softmmu/target/i386/kvm-stub.o U abort 0000000000000000 T kvm_allows_irq0_override 0000000000000030 T kvm_arch_get_supported_cpuid 0000000000000020 T kvm_enable_x2apic 0000000000000010 T kvm_has_smm 0000000000000050 T kvm_hv_vpindex_settable grep(1) used, but otherwise I would need to upload results somewhere else. $ nm ./i386-bsd-user/target/i386/cpu.o |grep kvm U kvm_arch_get_supported_cpuid 0000000000001290 d kvm_default_props U kvm_state 0000000000000240 T x86_cpu_change_kvm_default Please note that there are 4 types of x86 build: i386, x86_64 and two bsd-user (32-bit and 64-bit). According to my observations of repeated attempts both builds of bsd-user are affected. There is also a difference that kvm-stub is twice in !bsd-user directories and once in bsd-user ones. $ find . -name kvm-stub.o|grep x86_64 ./x86_64-softmmu/accel/stubs/kvm-stub.o ./x86_64-softmmu/target/i386/kvm-stub.o ./x86_64-bsd-user/accel/stubs/kvm-stub.o > 2) do the same exercise to get a .i for target/i386/kvm-stub.c > > 3) try removing the "#ifndef __OPTIMIZE__" and leave everything else as is, > see if it works. Same result as above, it seems that bsd-user ones are affected in the same way. > No need to play with macros, which also goes to show that > you didn't really understand what's going on---that's fine, but then please > refrain from making summary judgments which only lengthen the discussion. > I stand with my expression that I find playing with optimizer as too fragile to depend on it. Relying on a preprocessor to disable unused code is predictable. But if that is the style in qemu, I can just acknowledge it. > Thanks, > > Paolo >
On 21/02/19 03:08, Kamil Rytarowski wrote: > $ find . -name kvm-stub.o|grep x86_64 > ./x86_64-softmmu/accel/stubs/kvm-stub.o > ./x86_64-softmmu/target/i386/kvm-stub.o > ./x86_64-bsd-user/accel/stubs/kvm-stub.o > So here's the bug. The fix should be as simple as diff --git a/target/i386/Makefile.objs b/target/i386/Makefile.objs index cb9c265525..48e0c28434 100644 --- a/target/i386/Makefile.objs +++ b/target/i386/Makefile.objs @@ -3,10 +3,10 @@ obj-$(CONFIG_TCG) += translate.o obj-$(CONFIG_TCG) += bpt_helper.o cc_helper.o excp_helper.o fpu_helper.o obj-$(CONFIG_TCG) += int_helper.o mem_helper.o misc_helper.o mpx_helper.o obj-$(CONFIG_TCG) += seg_helper.o smm_helper.o svm_helper.o +obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o ifeq ($(CONFIG_SOFTMMU),y) obj-y += machine.o arch_memory_mapping.o arch_dump.o monitor.o obj-$(CONFIG_KVM) += kvm.o -obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o obj-$(CONFIG_HYPERV) += hyperv.o obj-$(call lnot,$(CONFIG_HYPERV)) += hyperv-stub.o ifeq ($(CONFIG_WIN32),y) Thanks, Paolo
Ping. On 21.02.2019 03:08, Kamil Rytarowski wrote: > On 20.02.2019 18:29, Paolo Bonzini wrote: >> On 20/02/19 12:59, Kamil Rytarowski wrote: >>> Ping, still valid. >> >> Sorry, I missed your email. >> >>> On 15.02.2019 00:38, Kamil Rytarowski wrote: >>>> I consider it as fragile hack and certainly not something to depend on. >>>> Also in some circumstances of such code, especially "if (zero0)" we want >>>> to enable disabled code under a debugger. >> >> That's a good objection, but certainly does not apply to KVM on NetBSD. >> > > There is KVM for Darwin (experimental and rather toy project) and it > might be ported to NetBSD (I have actually forked it on GitHub > recently), but I doubt that someone would enable KVM on any platform > under a debugger this way and expect something to work. > >>>> There were also kernel backdoors due to this optimization. >> >> Citation please? >> > > I saw an exploit for such case with a .txt writeup on ftp of grsecurity > but that service seems to be gone (probably long time ago), so please > defer discussion on it. If someone is interested to find it out, there > are enough pointers to dig it (assuming that this is still possible). > >>>> Requested cpu.i (hopefully correctly generated) >>>> >>>> http://netbsd.org/~kamil/qemu/cpu.i.bz2 >> >> So, first thing first I can reproduce clang's behavior with this .i file >> and also with this reduced test case. >> >> extern void f(void); >> int i, j; >> int main() >> { >> if (0 && i) f(); >> if (j && 0) f(); >> } >> >> The first is eliminated but the second is not, just like in QEMU where >> this works: >> >> if (kvm_enabled() && cpu->enable_pmu) { >> KVMState *s = cs->kvm_state; >> >> *eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX); >> *ebx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EBX); >> *ecx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_ECX); >> *edx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EDX); >> } else if (hvf_enabled() && cpu->enable_pmu) { >> *eax = hvf_get_supported_cpuid(0xA, count, R_EAX); >> *ebx = hvf_get_supported_cpuid(0xA, count, R_EBX); >> *ecx = hvf_get_supported_cpuid(0xA, count, R_ECX); >> *edx = hvf_get_supported_cpuid(0xA, count, R_EDX); >> >> while this doesn't: >> >> if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) && >> kvm_enabled()) { >> KVMState *s = CPU(cpu)->kvm_state; >> uint32_t eax_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EAX); >> uint32_t ebx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EBX); >> uint32_t ecx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_ECX); >> uint32_t eax_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EAX); >> uint32_t ebx_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EBX); >> >> But, that's okay, it's -O0 so we give clang a pass for that Note that >> clang does do the optimization even in more complex cases like >> >> extern _Bool f(void); >> int main() >> { >> if (!0) return 0; >> if (!f()) return 0; >> } >> >> The problem is that there is a kvm-stub.c entry for that, and in fact >> my compilation passes and the symbol is resolved correctly: >> >> $ nm target/i386/cpu.o |grep kvm_.*get_sup >> U kvm_arch_get_supported_cpuid >> $ nm target/i386/kvm-stub.o|grep kvm_.*get_sup >> 0000000000000030 T kvm_arch_get_supported_cpuid >> $ nm qemu-system-x86_64 |grep kvm_.*get_sup >> 000000000046eab0 T kvm_arch_get_supported_cpuid >> >> As expected, something much less obvious is going on for you, in >> particular __OPTIMIZE__seems not to be working properly. However, >> that would also be very surprising. >> >> Please: >> >> 1) run the last two "nm" commands on your build (wthout grep). >> > > I cannot run nm(1) on qemu-system-x86_64 as it's not linkable. > > I'm getting the same result for target/i386/cpu.o and > target/i386/kvm-stub.o. > > $ nm ./i386-softmmu/target/i386/kvm-stub.o > U abort > 0000000000000000 T kvm_allows_irq0_override > 0000000000000030 T kvm_arch_get_supported_cpuid > 0000000000000020 T kvm_enable_x2apic > 0000000000000010 T kvm_has_smm > 0000000000000050 T kvm_hv_vpindex_settable > > grep(1) used, but otherwise I would need to upload results somewhere else. > > $ nm ./i386-bsd-user/target/i386/cpu.o |grep kvm > U kvm_arch_get_supported_cpuid > 0000000000001290 d kvm_default_props > U kvm_state > 0000000000000240 T x86_cpu_change_kvm_default > > Please note that there are 4 types of x86 build: i386, x86_64 and two > bsd-user (32-bit and 64-bit). > > According to my observations of repeated attempts both builds of > bsd-user are affected. > > There is also a difference that kvm-stub is twice in !bsd-user > directories and once in bsd-user ones. > > $ find . -name kvm-stub.o|grep x86_64 > ./x86_64-softmmu/accel/stubs/kvm-stub.o > ./x86_64-softmmu/target/i386/kvm-stub.o > ./x86_64-bsd-user/accel/stubs/kvm-stub.o > >> 2) do the same exercise to get a .i for target/i386/kvm-stub.c >> >> 3) try removing the "#ifndef __OPTIMIZE__" and leave everything else as is, >> see if it works. > > Same result as above, it seems that bsd-user ones are affected in the > same way. > >> No need to play with macros, which also goes to show that >> you didn't really understand what's going on---that's fine, but then please >> refrain from making summary judgments which only lengthen the discussion. >> > > I stand with my expression that I find playing with optimizer as too > fragile to depend on it. Relying on a preprocessor to disable unused > code is predictable. But if that is the style in qemu, I can just > acknowledge it. > >> Thanks, >> >> Paolo >> > >
On 25.02.2019 08:10, Paolo Bonzini wrote: > I have replied already. > Sorry, I don't have this mail in my mailbox for some reason. The issue is gone with the following patch: https://www.mail-archive.com/qemu-devel@nongnu.org/msg598417.html Tested-by: Kamil Rytarowski <n54@gmx.com> Please merge with qemu. Thank you in advance!
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index a6d1cd190f..93d3c0f0b3 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -459,8 +459,21 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension); kvm_vcpu_ioctl(cpu, KVM_ENABLE_CAP, &cap); \ }) +#ifdef CONFIG_KVM uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, uint32_t index, int reg); +#else +/* + * This function is only called inside conditionals which we + * rely on the compiler to optimize out when CONFIG_KVM is not + * defined. + */ +#define kvm_arch_get_supported_cpuid(a, b, c, d) \ + ({ \ + abort(); \ + 0; \ + }) +#endif uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index); diff --git a/target/i386/kvm-stub.c b/target/i386/kvm-stub.c index e7a673e5db..9ce8566700 100644 --- a/target/i386/kvm-stub.c +++ b/target/i386/kvm-stub.c @@ -29,16 +29,6 @@ bool kvm_enable_x2apic(void) { return false; } - -/* This function is only called inside conditionals which we - * rely on the compiler to optimize out when CONFIG_KVM is not - * defined. - */ -uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, - uint32_t index, int reg) -{ - abort(); -} #endif bool kvm_hv_vpindex_settable(void)
This improves the commit: "target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()" r. 2140cfa51d59177815f5b82e94ac48fb24909aba Clang/LLVM on NetBSD with enabled optimization cannot link correct qemu program because of a missing symbol of kvm_arch_get_supported_cpuid() in kvm-stubs.o used by executables. There are more than a single one kvm-stub.c and several types of possible programs such as bsd-user ones. the previous workaround does not work reliably for all use-cases. Instead of reworking the stubs and linking rules, move the workaround from a code that depends on the __OPTIMIZE__ builtin compiler flag, build option (KVM), compiler and arrangement of linking rules to a simple macro in a shared header with all the users that defines fallback dummy implementation, ignoring whether it is optimized out or not. Signed-off-by: Kamil Rytarowski <n54@gmx.com> --- include/sysemu/kvm.h | 13 +++++++++++++ target/i386/kvm-stub.c | 10 ---------- 2 files changed, 13 insertions(+), 10 deletions(-)