From 41c6bcd912d1a2461313040566077b86e48eea31 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 5 Dec 2012 14:49:07 -0200 Subject: [PATCH 01/17] libqemustub: Add qemu_[un]register_reset() stubs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will be useful for code that don't call qemu_devices_reset() (e.g. *-user). If qemu_devices_reset() is never called, it means we don't need to keep track of the reset handler list. Signed-off-by: Eduardo Habkost Signed-off-by: Andreas Färber --- stubs/Makefile.objs | 1 + stubs/reset.c | 13 +++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 stubs/reset.c diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 035b29a1f3..00f0b641d6 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -5,4 +5,5 @@ stub-obj-y += fdset-get-fd.o stub-obj-y += fdset-remove-fd.o stub-obj-y += get-fd.o stub-obj-y += set-fd-handler.o +stub-obj-y += reset.o stub-obj-$(CONFIG_WIN32) += fd-register.o diff --git a/stubs/reset.c b/stubs/reset.c new file mode 100644 index 0000000000..ad287251ed --- /dev/null +++ b/stubs/reset.c @@ -0,0 +1,13 @@ +#include "hw/hw.h" + +/* Stub functions for binaries that never call qemu_devices_reset(), + * and don't need to keep track of the reset handler list. + */ + +void qemu_register_reset(QEMUResetHandler *func, void *opaque) +{ +} + +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque) +{ +} From 083a5f8731bb3c7e0eae99dcdb1209027d770aaf Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 5 Dec 2012 14:49:08 -0200 Subject: [PATCH 02/17] libqemustub: vmstate register/unregister stubs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add vmstate stub functions, so that qdev.o can be used without savevm.o when vmstate support is not necessary (i.e. by *-user). Signed-off-by: Eduardo Habkost Signed-off-by: Andreas Färber --- stubs/Makefile.objs | 1 + stubs/vmstate.c | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 stubs/vmstate.c diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 00f0b641d6..ca2197e6fb 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -6,4 +6,5 @@ stub-obj-y += fdset-remove-fd.o stub-obj-y += get-fd.o stub-obj-y += set-fd-handler.o stub-obj-y += reset.o +stub-obj-y += vmstate.o stub-obj-$(CONFIG_WIN32) += fd-register.o diff --git a/stubs/vmstate.c b/stubs/vmstate.c new file mode 100644 index 0000000000..3682af599e --- /dev/null +++ b/stubs/vmstate.c @@ -0,0 +1,17 @@ +#include "qemu-common.h" +#include "migration/vmstate.h" + +int vmstate_register_with_alias_id(DeviceState *dev, + int instance_id, + const VMStateDescription *vmsd, + void *base, int alias_id, + int required_for_version) +{ + return 0; +} + +void vmstate_unregister(DeviceState *dev, + const VMStateDescription *vmsd, + void *opaque) +{ +} From 906709a151344805df4ff493a7d3a81fbce46fbe Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 5 Dec 2012 14:49:09 -0200 Subject: [PATCH 03/17] libqemustub: sysbus_get_default() stub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stub will be used on cases where sysbus.c is not compiled in (e.g. *-user). Note that code that uses NULL as the bus with qdev{_try,}_create() implicitly uses sysbus_get_default() as the bus, and will still require sysbus.c to be compiled in. Signed-off-by: Eduardo Habkost Signed-off-by: Andreas Färber --- stubs/Makefile.objs | 1 + stubs/sysbus.c | 6 ++++++ 2 files changed, 7 insertions(+) create mode 100644 stubs/sysbus.c diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index ca2197e6fb..7672c69a29 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -7,4 +7,5 @@ stub-obj-y += get-fd.o stub-obj-y += set-fd-handler.o stub-obj-y += reset.o stub-obj-y += vmstate.o +stub-obj-y += sysbus.o stub-obj-$(CONFIG_WIN32) += fd-register.o diff --git a/stubs/sysbus.c b/stubs/sysbus.c new file mode 100644 index 0000000000..e13496582b --- /dev/null +++ b/stubs/sysbus.c @@ -0,0 +1,6 @@ +#include "hw/qdev-core.h" + +BusState *sysbus_get_default(void) +{ + return NULL; +} From 507066f8a9610c0088df19ce7b3e436f43165ec1 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 5 Dec 2012 14:49:12 -0200 Subject: [PATCH 04/17] qdev: Include qdev code into *-user, too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code depends on some functions from qemu-option.o, so add qemu-option.o to universal-obj-y to make sure it's included. Signed-off-by: Eduardo Habkost Signed-off-by: Andreas Färber --- Makefile.objs | 8 ++++++++ hw/Makefile.objs | 9 +++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index a3eab4b410..12a314e3fb 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -21,6 +21,13 @@ qom-obj-y = qom/ universal-obj-y += $(qom-obj-y) +####################################################################### +# Core hw code (qdev core) +hw-core-obj-y += hw/ +hw-core-obj-y += qemu-option.o + +universal-obj-y += $(hw-core-obj-y) + ####################################################################### # oslib-obj-y is code depending on the OS (win32 vs posix) oslib-obj-y = osdep.o cutils.o qemu-timer-common.o @@ -182,6 +189,7 @@ nested-vars += \ user-obj-y \ common-obj-y \ universal-obj-y \ + hw-core-obj-y \ extra-obj-y \ trace-obj-y dummy := $(call unnest-vars) diff --git a/hw/Makefile.objs b/hw/Makefile.objs index b8bbed39c1..6b8a68c504 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -1,3 +1,9 @@ +# core qdev-related obj files, also used by *-user: +hw-core-obj-y += qdev.o qdev-properties.o +# irq.o needed for qdev GPIO handling: +hw-core-obj-y += irq.o + + common-obj-y = usb/ ide/ pci/ common-obj-y += loader.o common-obj-$(CONFIG_VIRTIO) += virtio-console.o @@ -154,7 +160,6 @@ common-obj-$(CONFIG_SOUND) += $(sound-obj-y) common-obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/ common-obj-y += usb/ -common-obj-y += irq.o common-obj-$(CONFIG_PTIMER) += ptimer.o common-obj-$(CONFIG_MAX7310) += max7310.o common-obj-$(CONFIG_WM8750) += wm8750.o @@ -180,7 +185,7 @@ common-obj-$(CONFIG_SD) += sd.o common-obj-y += bt.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o common-obj-y += bt-hci-csr.o common-obj-y += msmouse.o ps2.o -common-obj-y += qdev.o qdev-properties.o qdev-monitor.o +common-obj-y += qdev-monitor.o common-obj-y += qdev-properties-system.o common-obj-$(CONFIG_BRLAPI) += baum.o From 5d5b24d042072fb4d13e7027f6e52e44390a9896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Fri, 4 Jan 2013 18:13:00 +0100 Subject: [PATCH 05/17] qdev: Don't assume existence of parent bus on unparenting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 667d22d1ae59da46b4c1fbd094ca61145f19b8c3 (qdev: move bus removal to object_unparent) made the assumption that at unparenting time parent_bus is not NULL. This assumption is unjustified since object_unparent() may well be called directly after object_initialize(), without any qdev_set_parent_bus(). This did not cause any issues yet because qdev_[try_]create() does call qdev_set_parent_bus(), falling back to SysBus if unsupplied. While at it, ensure that this new function uses the device_ prefix and make the name more neutral in light of this semantic change. Reported-by: Eduardo Habkost Signed-off-by: Andreas Färber Tested-by: Igor Mammedov --- hw/qdev.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hw/qdev.c b/hw/qdev.c index f2c248451c..e2a5c5735b 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -698,16 +698,18 @@ static void device_class_base_init(ObjectClass *class, void *data) klass->props = NULL; } -static void qdev_remove_from_bus(Object *obj) +static void device_unparent(Object *obj) { DeviceState *dev = DEVICE(obj); - bus_remove_child(dev->parent_bus, dev); + if (dev->parent_bus != NULL) { + bus_remove_child(dev->parent_bus, dev); + } } static void device_class_init(ObjectClass *class, void *data) { - class->unparent = qdev_remove_from_bus; + class->unparent = device_unparent; } void device_reset(DeviceState *dev) From 961f839570f01d60a0b224248e6e56fc1d675793 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 5 Dec 2012 14:49:13 -0200 Subject: [PATCH 06/17] cpu: Change parent type to Device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This finally makes the CPU class a subclass of the Device class, allowing us to start using DeviceState properties on CPU subclasses. It has no_user=1, as creating CPUs using -device doesn't work yet. Signed-off-by: Igor Mammedov Signed-off-by: Eduardo Habkost Signed-off-by: Andreas Färber --- include/qom/cpu.h | 6 +++--- qom/cpu.c | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 3e9fc3aca5..fbacb2756b 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -20,7 +20,7 @@ #ifndef QEMU_CPU_H #define QEMU_CPU_H -#include "qom/object.h" +#include "hw/qdev-core.h" #include "qemu/thread.h" /** @@ -46,7 +46,7 @@ typedef struct CPUState CPUState; */ typedef struct CPUClass { /*< private >*/ - ObjectClass parent_class; + DeviceClass parent_class; /*< public >*/ void (*reset)(CPUState *cpu); @@ -66,7 +66,7 @@ struct kvm_run; */ struct CPUState { /*< private >*/ - Object parent_obj; + DeviceState parent_obj; /*< public >*/ struct QemuThread *thread; diff --git a/qom/cpu.c b/qom/cpu.c index d4d436f80a..49e5134ea1 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -36,14 +36,16 @@ static void cpu_common_reset(CPUState *cpu) static void cpu_class_init(ObjectClass *klass, void *data) { + DeviceClass *dc = DEVICE_CLASS(klass); CPUClass *k = CPU_CLASS(klass); k->reset = cpu_common_reset; + dc->no_user = 1; } -static TypeInfo cpu_type_info = { +static const TypeInfo cpu_type_info = { .name = TYPE_CPU, - .parent = TYPE_OBJECT, + .parent = TYPE_DEVICE, .instance_size = sizeof(CPUState), .abstract = true, .class_size = sizeof(CPUClass), From fcb93c036053ca8a5cfc02ca72b1b80dd2062423 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 4 Jan 2013 20:01:04 -0200 Subject: [PATCH 07/17] target-i386: kvm: -cpu host: Use GET_SUPPORTED_CPUID for SVM features MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing -cpu host code simply sets every bit inside svm_features (initializing it to -1), and that makes it impossible to make the enforce/check options work properly when the user asks for SVM features explicitly in the command-line. So, instead of initializing svm_features to -1, use GET_SUPPORTED_CPUID to fill only the bits that are supported by the host (just like we do for all other CPUID feature words inside kvm_cpu_fill_host()). This will keep the existing behavior (as filter_features_for_kvm() already uses GET_SUPPORTED_CPUID to filter svm_features), but will allow us to properly check for KVM features inside kvm_check_features_against_host() later. For example, we will be able to make this: $ qemu-system-x86_64 -cpu ...,+pfthreshold,enforce refuse to start if the SVM "pfthreshold" feature is not supported by the host (after we fix kvm_check_features_against_host() to check SVM flags as well). Signed-off-by: Eduardo Habkost Reviewed-by: Gleb Natapov Signed-off-by: Andreas Färber --- target-i386/cpu.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 82685dc3f5..1e30015be9 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -897,13 +897,10 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) } } - /* - * Every SVM feature requires emulation support in KVM - so we can't just - * read the host features here. KVM might even support SVM features not - * available on the host hardware. Just set all bits and mask out the - * unsupported ones later. - */ - x86_cpu_def->svm_features = -1; + /* Other KVM-specific feature fields: */ + x86_cpu_def->svm_features = + kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX); + #endif /* CONFIG_KVM */ } From bd004beff8db09b5790b1bb19fad3974e112f007 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 4 Jan 2013 20:01:05 -0200 Subject: [PATCH 08/17] target-i386: kvm: Enable all supported KVM features for -cpu host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using -cpu host, we don't need to use the kvm_default_features variable, as the user is explicitly asking QEMU to enable all feature supported by the host. This changes the kvm_cpu_fill_host() code to use GET_SUPPORTED_CPUID to initialize the kvm_features field, so we get all host KVM features enabled. This will also allow us to properly check/enforce KVM features inside kvm_check_features_against_host() later. For example, we will be able to make this: $ qemu-system-x86_64 -cpu ...,+kvm_pv_eoi,enforce refuse to start if kvm_pv_eoi is not supported by the host (after we fix kvm_check_features_against_host() to check KVM flags as well). Signed-off-by: Eduardo Habkost Reviewed-by: Gleb Natapov Signed-off-by: Andreas Färber --- target-i386/cpu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 1e30015be9..2547bfabf1 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -900,6 +900,8 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) /* Other KVM-specific feature fields: */ x86_cpu_def->svm_features = kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX); + x86_cpu_def->kvm_features = + kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX); #endif /* CONFIG_KVM */ } From 8b4beddc6bead9d7c85fe690b62f2621574eb195 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 4 Jan 2013 20:01:06 -0200 Subject: [PATCH 09/17] target-i386: check/enforce: Fix CPUID leaf numbers on error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The -cpu check/enforce warnings are printing incorrect information about the missing flags. There are no feature flags on CPUID leaves 0 and 0x80000000, but there were references to 0 and 0x80000000 in the table at kvm_check_features_against_host(). This changes the model_features_t struct to contain the register number as well, so the error messages print the correct CPUID leaf+register information, instead of wrong CPUID leaf numbers. This also changes the format of the error messages, so they follow the "CPUID... [bit ]" convention used in Intel documentation. Example output: $ qemu-system-x86_64 -machine pc-1.0,accel=kvm -cpu Opteron_G4,+ia64,enforce warning: host doesn't support requested feature: CPUID.01H:EDX.ia64 [bit 30] warning: host doesn't support requested feature: CPUID.01H:ECX.xsave [bit 26] warning: host doesn't support requested feature: CPUID.01H:ECX.avx [bit 28] warning: host doesn't support requested feature: CPUID.80000001H:ECX.abm [bit 5] warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit 6] warning: host doesn't support requested feature: CPUID.80000001H:ECX.misalignsse [bit 7] warning: host doesn't support requested feature: CPUID.80000001H:ECX.3dnowprefetch [bit 8] warning: host doesn't support requested feature: CPUID.80000001H:ECX.xop [bit 11] warning: host doesn't support requested feature: CPUID.80000001H:ECX.fma4 [bit 16] Unable to find x86 CPU definition $ Signed-off-by: Eduardo Habkost Reviewed-by: Gleb Natapov Signed-off-by: Andreas Färber --- target-i386/cpu.c | 42 +++++++++++++++++++++++++++++++++--------- target-i386/cpu.h | 3 +++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 2547bfabf1..ddf70248a0 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -124,6 +124,25 @@ static const char *cpuid_7_0_ebx_feature_name[] = { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, }; +const char *get_register_name_32(unsigned int reg) +{ + static const char *reg_names[CPU_NB_REGS32] = { + [R_EAX] = "EAX", + [R_ECX] = "ECX", + [R_EDX] = "EDX", + [R_EBX] = "EBX", + [R_ESP] = "ESP", + [R_EBP] = "EBP", + [R_ESI] = "ESI", + [R_EDI] = "EDI", + }; + + if (reg > CPU_NB_REGS32) { + return NULL; + } + return reg_names[reg]; +} + /* collects per-function cpuid data */ typedef struct model_features_t { @@ -132,7 +151,8 @@ typedef struct model_features_t { uint32_t check_feat; const char **flag_names; uint32_t cpuid; - } model_features_t; + int reg; +} model_features_t; int check_cpuid = 0; int enforce_cpuid = 0; @@ -912,10 +932,13 @@ static int unavailable_host_feature(struct model_features_t *f, uint32_t mask) for (i = 0; i < 32; ++i) if (1 << i & mask) { - fprintf(stderr, "warning: host cpuid %04x_%04x lacks requested" - " flag '%s' [0x%08x]\n", - f->cpuid >> 16, f->cpuid & 0xffff, - f->flag_names[i] ? f->flag_names[i] : "[reserved]", mask); + const char *reg = get_register_name_32(f->reg); + assert(reg); + fprintf(stderr, "warning: host doesn't support requested feature: " + "CPUID.%02XH:%s%s%s [bit %d]\n", + f->cpuid, reg, + f->flag_names[i] ? "." : "", + f->flag_names[i] ? f->flag_names[i] : "", i); break; } return 0; @@ -934,13 +957,14 @@ static int kvm_check_features_against_host(x86_def_t *guest_def) int rv, i; struct model_features_t ft[] = { {&guest_def->features, &host_def.features, - ~0, feature_name, 0x00000000}, + ~0, feature_name, 0x00000001, R_EDX}, {&guest_def->ext_features, &host_def.ext_features, - ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001}, + ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001, R_ECX}, {&guest_def->ext2_features, &host_def.ext2_features, - ~PPRO_FEATURES, ext2_feature_name, 0x80000000}, + ~PPRO_FEATURES, ext2_feature_name, 0x80000001, R_EDX}, {&guest_def->ext3_features, &host_def.ext3_features, - ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001}}; + ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001, R_ECX} + }; assert(kvm_enabled()); diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 1283537108..e56921bbe3 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -1220,4 +1220,7 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access); void enable_kvm_pv_eoi(void); +/* Return name of 32-bit register, from a R_* constant */ +const char *get_register_name_32(unsigned int reg); + #endif /* CPU_I386_H */ From 54830ff84df5d1fb182e91bf40e3d7c66c2559a4 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 4 Jan 2013 20:01:07 -0200 Subject: [PATCH 10/17] target-i386: check/enforce: Do not ignore "hypervisor" flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't need any hack to ignore CPUID_EXT_HYPERVISOR anymore, because kvm_arch_get_supported_cpuid() now sets CPUID_EXT_HYPERVISOR properly. So, this shouldn't introduce any behavior change, but it makes the code simpler. Signed-off-by: Eduardo Habkost Reviewed-by: Gleb Natapov Signed-off-by: Andreas Färber --- target-i386/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index ddf70248a0..a3d104d7dc 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -959,7 +959,7 @@ static int kvm_check_features_against_host(x86_def_t *guest_def) {&guest_def->features, &host_def.features, ~0, feature_name, 0x00000001, R_EDX}, {&guest_def->ext_features, &host_def.ext_features, - ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001, R_ECX}, + ~0, ext_feature_name, 0x00000001, R_ECX}, {&guest_def->ext2_features, &host_def.ext2_features, ~PPRO_FEATURES, ext2_feature_name, 0x80000001, R_EDX}, {&guest_def->ext3_features, &host_def.ext3_features, From 227146259e8deb14b7b30e7718e61512e0f524a9 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 4 Jan 2013 20:01:08 -0200 Subject: [PATCH 11/17] target-i386: check/enforce: Check all CPUID.80000001H.EDX bits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I have no idea why PPRO_FEATURES was being ignored on the check of the CPUID.80000001H.EDX bits. I believe it was a mistake, and it was supposed to be ~(PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) or just ~CPUID_EXT2_AMD_ALIASES, because some time ago kvm_cpu_fill_host() used the CPUID instruction directly (instead of kvm_arch_get_supported_cpuid()). But now kvm_cpu_fill_host() uses kvm_arch_get_supported_cpuid(), and kvm_arch_get_supported_cpuid() returns all supported bits for CPUID.80000001H.EDX, even the AMD aliases (that are explicitly copied from CPUID.01H.EDX), so we can make the code check/enforce all the CPUID.80000001H.EDX bits. Signed-off-by: Eduardo Habkost Reviewed-by: Gleb Natapov Signed-off-by: Andreas Färber --- target-i386/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index a3d104d7dc..a2971d27eb 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -961,7 +961,7 @@ static int kvm_check_features_against_host(x86_def_t *guest_def) {&guest_def->ext_features, &host_def.ext_features, ~0, ext_feature_name, 0x00000001, R_ECX}, {&guest_def->ext2_features, &host_def.ext2_features, - ~PPRO_FEATURES, ext2_feature_name, 0x80000001, R_EDX}, + ~0, ext2_feature_name, 0x80000001, R_EDX}, {&guest_def->ext3_features, &host_def.ext3_features, ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001, R_ECX} }; From 396d2cfccdc1a46a8c66d9d9baaa59071a553b1c Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 4 Jan 2013 20:01:09 -0200 Subject: [PATCH 12/17] target-i386: check/enforce: Check SVM flag support as well MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When nested SVM is supported, the kernel returns the SVM flag on GET_SUPPORTED_CPUID[1], so we can check the SVM flag safely in kvm_check_features_against_host(). I don't know why the original code ignored the SVM flag. Maybe it was because kvm_cpu_fill_host() used the CPUID instruction directly instead of GET_SUPPORTED_CPUID [1] Older kernels (before v2.6.37) returned the SVM flag even if nested SVM was _not_ supported. So the only cases where this patch should change behavior is when SVM is being requested by the user or the CPU model, but not supported by the host. And on these cases we really want QEMU to abort if the "enforce" option is set. Signed-off-by: Eduardo Habkost Reviewed-by: Gleb Natapov Signed-off-by: Andreas Färber --- target-i386/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index a2971d27eb..535cd52d17 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -963,7 +963,7 @@ static int kvm_check_features_against_host(x86_def_t *guest_def) {&guest_def->ext2_features, &host_def.ext2_features, ~0, ext2_feature_name, 0x80000001, R_EDX}, {&guest_def->ext3_features, &host_def.ext3_features, - ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001, R_ECX} + ~0, ext3_feature_name, 0x80000001, R_ECX} }; assert(kvm_enabled()); From e8beac00bd26a60e788ab336f38bc12a95b20f0d Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 4 Jan 2013 20:01:10 -0200 Subject: [PATCH 13/17] target-i386: check/enforce: Eliminate check_feat field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that all entries have check_feat=~0 in kvm_check_features_against_host(), we can eliminate check_feat entirely and make the code check all bits. This patch shouldn't introduce any behavior change, as check_feat is set to ~0 on all entries. Signed-off-by: Eduardo Habkost Reviewed-by: Gleb Natapov Signed-off-by: Andreas Färber --- target-i386/cpu.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 535cd52d17..951e206779 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -148,7 +148,6 @@ const char *get_register_name_32(unsigned int reg) typedef struct model_features_t { uint32_t *guest_feat; uint32_t *host_feat; - uint32_t check_feat; const char **flag_names; uint32_t cpuid; int reg; @@ -945,8 +944,7 @@ static int unavailable_host_feature(struct model_features_t *f, uint32_t mask) } /* best effort attempt to inform user requested cpu flags aren't making - * their way to the guest. Note: ft[].check_feat ideally should be - * specified via a guest_def field to suppress report of extraneous flags. + * their way to the guest. * * This function may be called only if KVM is enabled. */ @@ -957,13 +955,13 @@ static int kvm_check_features_against_host(x86_def_t *guest_def) int rv, i; struct model_features_t ft[] = { {&guest_def->features, &host_def.features, - ~0, feature_name, 0x00000001, R_EDX}, + feature_name, 0x00000001, R_EDX}, {&guest_def->ext_features, &host_def.ext_features, - ~0, ext_feature_name, 0x00000001, R_ECX}, + ext_feature_name, 0x00000001, R_ECX}, {&guest_def->ext2_features, &host_def.ext2_features, - ~0, ext2_feature_name, 0x80000001, R_EDX}, + ext2_feature_name, 0x80000001, R_EDX}, {&guest_def->ext3_features, &host_def.ext3_features, - ~0, ext3_feature_name, 0x80000001, R_ECX} + ext3_feature_name, 0x80000001, R_ECX} }; assert(kvm_enabled()); @@ -971,7 +969,7 @@ static int kvm_check_features_against_host(x86_def_t *guest_def) kvm_cpu_fill_host(&host_def); for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i) for (mask = 1; mask; mask <<= 1) - if (ft[i].check_feat & mask && *ft[i].guest_feat & mask && + if (*ft[i].guest_feat & mask && !(*ft[i].host_feat & mask)) { unavailable_host_feature(&ft[i], mask); rv = 1; From 75a192aa68e7801ab8465b3345ac74d6d3cdceca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Sat, 5 Jan 2013 14:44:08 +0100 Subject: [PATCH 14/17] qemu-common.h: Make qemu_init_vcpu() stub static inline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turn the *-user macro into a no-op inline function to avoid unused-variable warnings and band-aiding #ifdef'ery. This allows to drop an #ifdef for alpha and avoids more for unicore32 and other upcoming trivial realizefn implementations. Suggested-by: Lluís Vilanova Signed-off-by: Eduardo Habkost Signed-off-by: Andreas Färber Reviewed-by: Eduardo Habkost --- include/qemu-common.h | 4 +++- target-alpha/cpu.c | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/qemu-common.h b/include/qemu-common.h index 2b83de395c..ca464bb367 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -288,7 +288,9 @@ struct qemu_work_item { }; #ifdef CONFIG_USER_ONLY -#define qemu_init_vcpu(env) do { } while (0) +static inline void qemu_init_vcpu(void *env) +{ +} #else void qemu_init_vcpu(void *env); #endif diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c index 212a6250ba..40e980933f 100644 --- a/target-alpha/cpu.c +++ b/target-alpha/cpu.c @@ -26,11 +26,9 @@ static void alpha_cpu_realize(Object *obj, Error **errp) { -#ifndef CONFIG_USER_ONLY AlphaCPU *cpu = ALPHA_CPU(obj); qemu_init_vcpu(&cpu->env); -#endif } /* Sort alphabetically by type name. */ From 4586f157757acc5c8edcc954289c7aa51661235c Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Fri, 28 Dec 2012 21:01:16 +0100 Subject: [PATCH 15/17] target-i386: Filter out unsupported features at realize time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Eduardo Habkost Signed-off-by: Igor Mammedov Reviewed-by: Eduardo Habkost Signed-off-by: Andreas Färber --- target-i386/cpu.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 951e206779..a776e1118d 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1571,21 +1571,6 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) env->cpuid_ext2_features |= (def->features & CPUID_EXT2_AMD_ALIASES); } - if (!kvm_enabled()) { - env->cpuid_features &= TCG_FEATURES; - env->cpuid_ext_features &= TCG_EXT_FEATURES; - env->cpuid_ext2_features &= (TCG_EXT2_FEATURES -#ifdef TARGET_X86_64 - | CPUID_EXT2_SYSCALL | CPUID_EXT2_LM -#endif - ); - env->cpuid_ext3_features &= TCG_EXT3_FEATURES; - env->cpuid_svm_features &= TCG_SVM_FEATURES; - } else { -#ifdef CONFIG_KVM - filter_features_for_kvm(cpu); -#endif - } object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error); if (error) { fprintf(stderr, "%s\n", error_get_pretty(error)); @@ -2106,6 +2091,22 @@ void x86_cpu_realize(Object *obj, Error **errp) env->cpuid_level = 7; } + if (!kvm_enabled()) { + env->cpuid_features &= TCG_FEATURES; + env->cpuid_ext_features &= TCG_EXT_FEATURES; + env->cpuid_ext2_features &= (TCG_EXT2_FEATURES +#ifdef TARGET_X86_64 + | CPUID_EXT2_SYSCALL | CPUID_EXT2_LM +#endif + ); + env->cpuid_ext3_features &= TCG_EXT3_FEATURES; + env->cpuid_svm_features &= TCG_SVM_FEATURES; + } else { +#ifdef CONFIG_KVM + filter_features_for_kvm(cpu); +#endif + } + #ifndef CONFIG_USER_ONLY qemu_register_reset(x86_cpu_machine_reset_cb, cpu); From 9b15cd9e7a1ab0827f4d01c4be77eb41f195073f Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Fri, 28 Dec 2012 21:01:17 +0100 Subject: [PATCH 16/17] target-i386: Sanitize AMD's ext2_features at realize time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When CPU properties are implemented, ext2_features may change between object_new(CPU) and cpu_realize_fn(). Sanitizing ext2_features for AMD based CPU at realize() time will keep current behavior after CPU features are converted to properties. Signed-off-by: Igor Mammedov Reviewed-by: Eduardo Habkost Signed-off-by: Andreas Färber --- target-i386/cpu.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index a776e1118d..b40cc37bac 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1561,16 +1561,6 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000, "tsc-frequency", &error); - /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on - * CPUID[1].EDX. - */ - if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && - env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && - env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) { - env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES; - env->cpuid_ext2_features |= (def->features & CPUID_EXT2_AMD_ALIASES); - } - object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error); if (error) { fprintf(stderr, "%s\n", error_get_pretty(error)); @@ -2091,6 +2081,17 @@ void x86_cpu_realize(Object *obj, Error **errp) env->cpuid_level = 7; } + /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on + * CPUID[1].EDX. + */ + if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && + env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && + env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) { + env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES; + env->cpuid_ext2_features |= (env->cpuid_features + & CPUID_EXT2_AMD_ALIASES); + } + if (!kvm_enabled()) { env->cpuid_features &= TCG_FEATURES; env->cpuid_ext_features &= TCG_EXT_FEATURES; From ebe8b9c6eb6e425d44805288b6b5dabd69368f46 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Fri, 28 Dec 2012 21:01:18 +0100 Subject: [PATCH 17/17] target-i386: Explicitly set vendor for each built-in cpudef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since cpudef config is not supported anymore and all remaining sources now always set x86_def_t.vendor[123] fields, remove setting default vendor to simplify future re-factoring. Signed-off-by: Igor Mammedov Reviewed-by: Eduardo Habkost Signed-off-by: Andreas Färber --- target-i386/cpu.c | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index b40cc37bac..78bd61e18f 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -407,6 +407,9 @@ static x86_def_t builtin_x86_defs[] = { { .name = "core2duo", .level = 10, + .vendor1 = CPUID_VENDOR_INTEL_1, + .vendor2 = CPUID_VENDOR_INTEL_2, + .vendor3 = CPUID_VENDOR_INTEL_3, .family = 6, .model = 15, .stepping = 11, @@ -451,6 +454,9 @@ static x86_def_t builtin_x86_defs[] = { { .name = "qemu32", .level = 4, + .vendor1 = CPUID_VENDOR_INTEL_1, + .vendor2 = CPUID_VENDOR_INTEL_2, + .vendor3 = CPUID_VENDOR_INTEL_3, .family = 6, .model = 3, .stepping = 3, @@ -461,6 +467,9 @@ static x86_def_t builtin_x86_defs[] = { { .name = "kvm32", .level = 5, + .vendor1 = CPUID_VENDOR_INTEL_1, + .vendor2 = CPUID_VENDOR_INTEL_2, + .vendor3 = CPUID_VENDOR_INTEL_3, .family = 15, .model = 6, .stepping = 1, @@ -475,6 +484,9 @@ static x86_def_t builtin_x86_defs[] = { { .name = "coreduo", .level = 10, + .vendor1 = CPUID_VENDOR_INTEL_1, + .vendor2 = CPUID_VENDOR_INTEL_2, + .vendor3 = CPUID_VENDOR_INTEL_3, .family = 6, .model = 14, .stepping = 8, @@ -490,6 +502,9 @@ static x86_def_t builtin_x86_defs[] = { { .name = "486", .level = 1, + .vendor1 = CPUID_VENDOR_INTEL_1, + .vendor2 = CPUID_VENDOR_INTEL_2, + .vendor3 = CPUID_VENDOR_INTEL_3, .family = 4, .model = 0, .stepping = 0, @@ -499,6 +514,9 @@ static x86_def_t builtin_x86_defs[] = { { .name = "pentium", .level = 1, + .vendor1 = CPUID_VENDOR_INTEL_1, + .vendor2 = CPUID_VENDOR_INTEL_2, + .vendor3 = CPUID_VENDOR_INTEL_3, .family = 5, .model = 4, .stepping = 3, @@ -508,6 +526,9 @@ static x86_def_t builtin_x86_defs[] = { { .name = "pentium2", .level = 2, + .vendor1 = CPUID_VENDOR_INTEL_1, + .vendor2 = CPUID_VENDOR_INTEL_2, + .vendor3 = CPUID_VENDOR_INTEL_3, .family = 6, .model = 5, .stepping = 2, @@ -517,6 +538,9 @@ static x86_def_t builtin_x86_defs[] = { { .name = "pentium3", .level = 2, + .vendor1 = CPUID_VENDOR_INTEL_1, + .vendor2 = CPUID_VENDOR_INTEL_2, + .vendor3 = CPUID_VENDOR_INTEL_3, .family = 6, .model = 7, .stepping = 3, @@ -542,6 +566,9 @@ static x86_def_t builtin_x86_defs[] = { .name = "n270", /* original is on level 10 */ .level = 5, + .vendor1 = CPUID_VENDOR_INTEL_1, + .vendor2 = CPUID_VENDOR_INTEL_2, + .vendor3 = CPUID_VENDOR_INTEL_3, .family = 6, .model = 28, .stepping = 2, @@ -1534,15 +1561,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) if (cpu_x86_parse_featurestr(def, features) < 0) { goto error; } - if (def->vendor1) { - env->cpuid_vendor1 = def->vendor1; - env->cpuid_vendor2 = def->vendor2; - env->cpuid_vendor3 = def->vendor3; - } else { - env->cpuid_vendor1 = CPUID_VENDOR_INTEL_1; - env->cpuid_vendor2 = CPUID_VENDOR_INTEL_2; - env->cpuid_vendor3 = CPUID_VENDOR_INTEL_3; - } + assert(def->vendor1); + env->cpuid_vendor1 = def->vendor1; + env->cpuid_vendor2 = def->vendor2; + env->cpuid_vendor3 = def->vendor3; env->cpuid_vendor_override = def->vendor_override; object_property_set_int(OBJECT(cpu), def->level, "level", &error); object_property_set_int(OBJECT(cpu), def->family, "family", &error);