| Message ID | 1374404241-19600-1-git-send-email-sw@weilnetz.de |
|---|---|
| State | Under Review |
| Headers | show |
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 <sw@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
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 <sw@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
diff --git a/Makefile.target b/Makefile.target index 9a49852..21a817f 100644 --- a/Makefile.target +++ b/Makefile.target @@ -84,7 +84,6 @@ obj-y += fpu/softfloat.o obj-y += target-$(TARGET_BASE_ARCH)/ obj-y += disas.o obj-$(CONFIG_GDBSTUB_XML) += gdbstub-xml.o -obj-$(CONFIG_NO_KVM) += kvm-stub.o ######################################################### # Linux user emulator target diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 9b701b4..44bf573 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -10,6 +10,7 @@ stub-obj-y += fdset-remove-fd.o stub-obj-y += get-fd.o stub-obj-y += get-vm-name.o stub-obj-y += iothread-lock.o +stub-obj-y += kvm-stub.o stub-obj-y += migr-blocker.o stub-obj-y += mon-is-qmp.o stub-obj-y += mon-printf.o diff --git a/kvm-stub.c b/stubs/kvm-stub.c similarity index 63% rename from kvm-stub.c rename to stubs/kvm-stub.c index 370c837..cc0fd06 100644 --- a/kvm-stub.c +++ b/stubs/kvm-stub.c @@ -10,14 +10,33 @@ * */ +/* We don't want to include exec/poison.h here. */ +#define HW_POISON_H + +/* Several data types are only used for unused function arguments + or in files which are included. + Define them here to avoid include dependencies. */ + +typedef struct CPUArchState CPUArchState; +typedef unsigned target_ulong; + +typedef struct ARMCPU ARMCPU; + +typedef struct CPUPPCState CPUPPCState; +typedef struct PowerPCCPU PowerPCCPU; + +extern unsigned ram_size; + #include "qemu-common.h" #include "hw/hw.h" -#include "cpu.h" -#include "sysemu/kvm.h" -#ifndef CONFIG_USER_ONLY +/* Pretend that cpu.h was included to get all prototypes. */ +#define NEED_CPU_H +#include "sysemu/kvm.h" +#include "hw/ppc/openpic.h" #include "hw/pci/msi.h" -#endif +#include "target-arm/kvm_arm.h" +#include "target-ppc/kvm_ppc.h" KVMState *kvm_state; bool kvm_kernel_irqchip; @@ -74,6 +93,11 @@ int kvm_has_pit_state2(void) return 0; } +void *kvm_ram_alloc(ram_addr_t size) +{ + g_assert_not_reached(); +} + void kvm_setup_guest_memory(void *start, size_t size) { } @@ -102,7 +126,7 @@ void kvm_remove_all_breakpoints(CPUState *cpu) #ifndef _WIN32 int kvm_set_signal_mask(CPUState *cpu, const sigset_t *sigset) { - abort(); + g_assert_not_reached(); } #endif @@ -116,7 +140,8 @@ int kvm_on_sigbus(int code, void *addr) return 1; } -#ifndef CONFIG_USER_ONLY +/* Stubs for system emulation. */ + int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg) { return -ENOSYS; @@ -144,4 +169,35 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq) { return -ENOSYS; } -#endif + +/* Stubs for target-arm. */ + +bool write_kvmstate_to_list(ARMCPU *cpu) +{ + g_assert_not_reached(); +} + +bool write_list_to_kvmstate(ARMCPU *cpu) +{ + g_assert_not_reached(); +} + +/* Stubs for target-i386. */ + +uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, + uint32_t index, int reg) +{ + g_assert_not_reached(); +} + +/* Stubs for target-ppc. */ + +int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs) +{ + return -EINVAL; +} + +void kvmppc_init(void) +{ + g_assert_not_reached(); +} diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs index 4a6e52e..d89b57c 100644 --- a/target-arm/Makefile.objs +++ b/target-arm/Makefile.objs @@ -1,6 +1,5 @@ obj-y += arm-semi.o obj-$(CONFIG_SOFTMMU) += machine.o obj-$(CONFIG_KVM) += kvm.o -obj-$(CONFIG_NO_KVM) += kvm-stub.o obj-y += translate.o op_helper.o helper.o cpu.o obj-y += neon_helper.o iwmmxt_helper.o diff --git a/target-arm/kvm-stub.c b/target-arm/kvm-stub.c deleted file mode 100644 index cd1849f..0000000 --- a/target-arm/kvm-stub.c +++ /dev/null @@ -1,23 +0,0 @@ -/* - * QEMU KVM ARM specific function stubs - * - * Copyright Linaro Limited 2013 - * - * Author: Peter Maydell <peter.maydell@linaro.org> - * - * This work is licensed under the terms of the GNU GPL, version 2 or later. - * See the COPYING file in the top-level directory. - * - */ -#include "qemu-common.h" -#include "kvm_arm.h" - -bool write_kvmstate_to_list(ARMCPU *cpu) -{ - abort(); -} - -bool write_list_to_kvmstate(ARMCPU *cpu) -{ - abort(); -} diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs index c1d4f05..963698a 100644 --- a/target-i386/Makefile.objs +++ b/target-i386/Makefile.objs @@ -3,6 +3,5 @@ obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o svm_helper.o obj-y += smm_helper.o misc_helper.o mem_helper.o seg_helper.o obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o obj-$(CONFIG_KVM) += kvm.o hyperv.o -obj-$(CONFIG_NO_KVM) += kvm-stub.o obj-$(CONFIG_LINUX_USER) += ioport-user.o obj-$(CONFIG_BSD_USER) += ioport-user.o diff --git a/target-i386/kvm-stub.c b/target-i386/kvm-stub.c deleted file mode 100644 index 11429c4..0000000 --- a/target-i386/kvm-stub.c +++ /dev/null @@ -1,18 +0,0 @@ -/* - * QEMU KVM x86 specific function stubs - * - * Copyright Linaro Limited 2012 - * - * Author: Peter Maydell <peter.maydell@linaro.org> - * - * This work is licensed under the terms of the GNU GPL, version 2 or later. - * See the COPYING file in the top-level directory. - * - */ -#include "qemu-common.h" -#include "kvm_i386.h" - -bool kvm_allows_irq0_override(void) -{ - return 1; -} diff --git a/target-ppc/Makefile.objs b/target-ppc/Makefile.objs index 6e78cb3..2c43c34 100644 --- a/target-ppc/Makefile.objs +++ b/target-ppc/Makefile.objs @@ -5,7 +5,6 @@ obj-y += machine.o mmu_helper.o mmu-hash32.o obj-$(TARGET_PPC64) += mmu-hash64.o endif obj-$(CONFIG_KVM) += kvm.o kvm_ppc.o -obj-$(CONFIG_NO_KVM) += kvm-stub.o obj-y += excp_helper.o obj-y += fpu_helper.o obj-y += int_helper.o diff --git a/target-ppc/kvm-stub.c b/target-ppc/kvm-stub.c deleted file mode 100644 index ee3f5d2..0000000 --- a/target-ppc/kvm-stub.c +++ /dev/null @@ -1,18 +0,0 @@ -/* - * QEMU KVM PPC specific function stubs - * - * Copyright Freescale Inc. 2013 - * - * Author: Alexander Graf <agraf@suse.de> - * - * This work is licensed under the terms of the GNU GPL, version 2 or later. - * See the COPYING file in the top-level directory. - * - */ -#include "qemu-common.h" -#include "hw/ppc/openpic.h" - -int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs) -{ - return -EINVAL; -}
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 <sw@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. Stefan Makefile.target | 1 - stubs/Makefile.objs | 1 + kvm-stub.c => stubs/kvm-stub.c | 70 ++++++++++++++++++++++++++++++++++++---- target-arm/Makefile.objs | 1 - target-arm/kvm-stub.c | 23 ------------- target-i386/Makefile.objs | 1 - target-i386/kvm-stub.c | 18 ----------- target-ppc/Makefile.objs | 1 - target-ppc/kvm-stub.c | 18 ----------- 9 files changed, 64 insertions(+), 70 deletions(-) rename kvm-stub.c => stubs/kvm-stub.c (63%) delete mode 100644 target-arm/kvm-stub.c delete mode 100644 target-i386/kvm-stub.c delete mode 100644 target-ppc/kvm-stub.c