Am 21.07.2013 13:20, schrieb Andreas Färber: > Hi Stefan, > > Am 21.07.2013 12:57, schrieb Stefan Weil: >> The KVM stub variables and functions don't depend on target specific data >> types, so it is possible to compile kvm-stub.c only once. >> >> Integrating the target specific KVM stubs for ARM, I386 and PPC in the >> common kvm-stub.c further simplifies the build environment and allows >> removing CONFIG_NO_KVM. >> >> Instead of 53 kvm-stub.o files, there is now only one file. >> >> abort() is replaced by g_assert_not_reached() which gives better diagnostic >> messages when it is called. >> >> Signed-off-by: Stefan Weil <s...@weilnetz.de> >> --- >> >> The resulting binary is slightly larger than before because it >> includes more stub functions. It could be made smaller by adding >> separate stubs/kvm-stub-arm.c, stubs/kvm-stub-i386.c and >> stubs/kvm-stub-ppc.c files. Using alias symbols for the stub >> functions would reduce the size further, but I don't think >> the size is critical here. >> >> Should we call g_assert_not_reached() in more (all?) stub functions? >> >> If the patch is accepted, a similar modification could be done >> for xen-stub.c. > I had thought about turning kvm-stub.c into stubs/kvm.c but rejected > that idea at the time due to CPU dependencies beyond CPUState. > Also I was wondering whether all stubs are actually from kvm-all.c (fine > then) or whether we would be opening a door for silently forgetting to > implement some functions in a new KVM target such as MIPS. > > In the current form I think it's a gross hack wrt target_ulong and > HW_POISON_H. ;) > > And yes, I would prefer to keep target stubs in separate files. > > Regards, > Andreas
The hack could be removed if the include files were cleaned. Do we still have to poison "env", for example? It is no longer a global variable. The handling of cpu.h is also a mess. The hack for target_ulong could also be avoided by using modified kvm_insert_breakpoint, kvm_remove_breakpoint. Fixing the existing include files was beyond the scope of my patch. Mixing kvm-all.o and kvm-stub.o would be detected by the linker, so missing KVM interfaces for new KVM targets is not possible. Regards, Stefan
