* [Qemu-devel] [PULL for-1.6 1/3] cpu: Partially revert "cpu: Change qemu_init_vcpu() argument to CPUState"
2013-07-29 15:32 [Qemu-devel] [PULL 0/3] QOM CPUState patch queue 2013-07-29 Andreas Färber
@ 2013-07-29 15:32 ` Andreas Färber
2013-07-29 17:41 ` Richard Henderson
2013-07-29 15:32 ` [Qemu-devel] [PULL for-1.6 2/3] target-i386: Pass X86CPU object to cpu_x86_find_by_name() Andreas Färber
2013-07-29 15:32 ` [Qemu-devel] [PULL for-1.6 3/3] target-i386: Disable PMU CPUID leaf by default Andreas Färber
2 siblings, 1 reply; 5+ messages in thread
From: Andreas Färber @ 2013-07-29 15:32 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Jia Liu, Anthony Green, Alexander Graf, Blue Swirl,
Max Filippov, Michael Walle, open list:PowerPC, Paul Brook,
Edgar E. Iglesias, Guan Xuetao, Andreas Färber,
Aurelien Jarno, Richard Henderson
Commit c643bed99 moved qemu_init_vcpu() calls to common CPUState code.
This causes x86 cpu-add to fail with "KVM: setting VAPIC address failed".
The reason for the failure is that CPUClass::kvm_fd is not yet
initialized in the following call graph:
->x86_cpu_realizefn
->x86_cpu_apic_realize
->qdev_init
->device_set_realized
->device_reset (hotplugged == 1)
->apic_reset_common
->vapic_base_update
->kvm_apic_vapic_base_update
This causes attempted KVM vCPU ioctls to fail.
By contrast, in the non-hotplug case the APIC is reset much later, when
the vCPU is already initialized.
As a quick and safe solution, move the qemu_init_vcpu() call back into
the targets' realize functions.
Reported-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Acked-by: Igor Mammedov <imammedo@redhat.com> (for i386)
Tested-by: Jia Liu <proljc@gmail.com> (for openrisc)
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
qom/cpu.c | 2 --
target-alpha/cpu.c | 3 +++
target-arm/cpu.c | 4 +++-
target-cris/cpu.c | 5 +++--
target-i386/cpu.c | 4 +++-
target-lm32/cpu.c | 6 ++++--
target-m68k/cpu.c | 4 +++-
target-microblaze/cpu.c | 5 +++--
target-mips/cpu.c | 5 +++--
target-moxie/cpu.c | 5 +++--
target-openrisc/cpu.c | 5 +++--
target-ppc/translate_init.c | 2 ++
target-s390x/cpu.c | 5 +++--
target-sh4/cpu.c | 5 +++--
target-sparc/cpu.c | 2 ++
target-unicore32/cpu.c | 2 ++
target-xtensa/cpu.c | 2 ++
17 files changed, 45 insertions(+), 21 deletions(-)
diff --git a/qom/cpu.c b/qom/cpu.c
index dbc9fb6..aa95108 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -228,8 +228,6 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
{
CPUState *cpu = CPU(dev);
- qemu_init_vcpu(cpu);
-
if (dev->hotplugged) {
cpu_synchronize_post_init(cpu);
notifier_list_notify(&cpu_added_notifiers, dev);
diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index 64c70bc..cfad2ea 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -33,8 +33,11 @@ static void alpha_cpu_set_pc(CPUState *cs, vaddr value)
static void alpha_cpu_realizefn(DeviceState *dev, Error **errp)
{
+ CPUState *cs = CPU(dev);
AlphaCPUClass *acc = ALPHA_CPU_GET_CLASS(dev);
+ qemu_init_vcpu(cs);
+
acc->parent_realize(dev, errp);
}
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 87d35c6..5a7566b 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -159,6 +159,7 @@ static void arm_cpu_finalizefn(Object *obj)
static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
{
+ CPUState *cs = CPU(dev);
ARMCPU *cpu = ARM_CPU(dev);
ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
CPUARMState *env = &cpu->env;
@@ -214,7 +215,8 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
init_cpreg_list(cpu);
- cpu_reset(CPU(cpu));
+ cpu_reset(cs);
+ qemu_init_vcpu(cs);
acc->parent_realize(dev, errp);
}
diff --git a/target-cris/cpu.c b/target-cris/cpu.c
index 45f2d6b..44301a4 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -137,10 +137,11 @@ void cris_cpu_list(FILE *f, fprintf_function cpu_fprintf)
static void cris_cpu_realizefn(DeviceState *dev, Error **errp)
{
- CRISCPU *cpu = CRIS_CPU(dev);
+ CPUState *cs = CPU(dev);
CRISCPUClass *ccc = CRIS_CPU_GET_CLASS(dev);
- cpu_reset(CPU(cpu));
+ cpu_reset(cs);
+ qemu_init_vcpu(cs);
ccc->parent_realize(dev, errp);
}
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2b59b7d..df2fb1b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2333,6 +2333,7 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
{
+ CPUState *cs = CPU(dev);
X86CPU *cpu = X86_CPU(dev);
X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
CPUX86State *env = &cpu->env;
@@ -2387,12 +2388,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
#endif
mce_init(cpu);
+ qemu_init_vcpu(cs);
x86_cpu_apic_realize(cpu, &local_err);
if (local_err != NULL) {
goto out;
}
- cpu_reset(CPU(cpu));
+ cpu_reset(cs);
xcc->parent_realize(dev, &local_err);
out:
diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
index 962d553..869878c 100644
--- a/target-lm32/cpu.c
+++ b/target-lm32/cpu.c
@@ -46,10 +46,12 @@ static void lm32_cpu_reset(CPUState *s)
static void lm32_cpu_realizefn(DeviceState *dev, Error **errp)
{
- LM32CPU *cpu = LM32_CPU(dev);
+ CPUState *cs = CPU(dev);
LM32CPUClass *lcc = LM32_CPU_GET_CLASS(dev);
- cpu_reset(CPU(cpu));
+ cpu_reset(cs);
+
+ qemu_init_vcpu(cs);
lcc->parent_realize(dev, errp);
}
diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
index c0bcb0d..008d8db 100644
--- a/target-m68k/cpu.c
+++ b/target-m68k/cpu.c
@@ -143,12 +143,14 @@ static const M68kCPUInfo m68k_cpus[] = {
static void m68k_cpu_realizefn(DeviceState *dev, Error **errp)
{
+ CPUState *cs = CPU(dev);
M68kCPU *cpu = M68K_CPU(dev);
M68kCPUClass *mcc = M68K_CPU_GET_CLASS(dev);
m68k_cpu_init_gdb(cpu);
- cpu_reset(CPU(cpu));
+ cpu_reset(cs);
+ qemu_init_vcpu(cs);
mcc->parent_realize(dev, errp);
}
diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
index c75d1bd..0ef9aa4 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -90,10 +90,11 @@ static void mb_cpu_reset(CPUState *s)
static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
{
- MicroBlazeCPU *cpu = MICROBLAZE_CPU(dev);
+ CPUState *cs = CPU(dev);
MicroBlazeCPUClass *mcc = MICROBLAZE_CPU_GET_CLASS(dev);
- cpu_reset(CPU(cpu));
+ cpu_reset(cs);
+ qemu_init_vcpu(cs);
mcc->parent_realize(dev, errp);
}
diff --git a/target-mips/cpu.c b/target-mips/cpu.c
index f81f9e9..9dd47e8 100644
--- a/target-mips/cpu.c
+++ b/target-mips/cpu.c
@@ -62,10 +62,11 @@ static void mips_cpu_reset(CPUState *s)
static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
{
- MIPSCPU *cpu = MIPS_CPU(dev);
+ CPUState *cs = CPU(dev);
MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(dev);
- cpu_reset(CPU(cpu));
+ cpu_reset(cs);
+ qemu_init_vcpu(cs);
mcc->parent_realize(dev, errp);
}
diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
index 6550be5..d97a091 100644
--- a/target-moxie/cpu.c
+++ b/target-moxie/cpu.c
@@ -45,10 +45,11 @@ static void moxie_cpu_reset(CPUState *s)
static void moxie_cpu_realizefn(DeviceState *dev, Error **errp)
{
- MoxieCPU *cpu = MOXIE_CPU(dev);
+ CPUState *cs = CPU(dev);
MoxieCPUClass *mcc = MOXIE_CPU_GET_CLASS(dev);
- cpu_reset(CPU(cpu));
+ qemu_init_vcpu(cs);
+ cpu_reset(cs);
mcc->parent_realize(dev, errp);
}
diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index aa269fb..075f00a 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -66,10 +66,11 @@ static inline void set_feature(OpenRISCCPU *cpu, int feature)
static void openrisc_cpu_realizefn(DeviceState *dev, Error **errp)
{
- OpenRISCCPU *cpu = OPENRISC_CPU(dev);
+ CPUState *cs = CPU(dev);
OpenRISCCPUClass *occ = OPENRISC_CPU_GET_CLASS(dev);
- cpu_reset(CPU(cpu));
+ qemu_init_vcpu(cs);
+ cpu_reset(cs);
occ->parent_realize(dev, errp);
}
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 8215946..3c81798 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7861,6 +7861,8 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
34, "power-spe.xml", 0);
}
+ qemu_init_vcpu(cs);
+
pcc->parent_realize(dev, errp);
#if defined(PPC_DUMP_CPU)
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 1d16da3..9b82495 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -101,10 +101,11 @@ static void s390_cpu_machine_reset_cb(void *opaque)
static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
{
- S390CPU *cpu = S390_CPU(dev);
+ CPUState *cs = CPU(dev);
S390CPUClass *scc = S390_CPU_GET_CLASS(dev);
- cpu_reset(CPU(cpu));
+ qemu_init_vcpu(cs);
+ cpu_reset(cs);
scc->parent_realize(dev, errp);
}
diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
index bda3c51..34b2b57 100644
--- a/target-sh4/cpu.c
+++ b/target-sh4/cpu.c
@@ -240,10 +240,11 @@ static const TypeInfo sh7785_type_info = {
static void superh_cpu_realizefn(DeviceState *dev, Error **errp)
{
- SuperHCPU *cpu = SUPERH_CPU(dev);
+ CPUState *cs = CPU(dev);
SuperHCPUClass *scc = SUPERH_CPU_GET_CLASS(dev);
- cpu_reset(CPU(cpu));
+ cpu_reset(cs);
+ qemu_init_vcpu(cs);
scc->parent_realize(dev, errp);
}
diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
index c7b4a90..47ce60d 100644
--- a/target-sparc/cpu.c
+++ b/target-sparc/cpu.c
@@ -743,6 +743,8 @@ static void sparc_cpu_realizefn(DeviceState *dev, Error **errp)
{
SPARCCPUClass *scc = SPARC_CPU_GET_CLASS(dev);
+ qemu_init_vcpu(CPU(dev));
+
scc->parent_realize(dev, errp);
}
diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
index 46813e5..3f78208 100644
--- a/target-unicore32/cpu.c
+++ b/target-unicore32/cpu.c
@@ -92,6 +92,8 @@ static void uc32_cpu_realizefn(DeviceState *dev, Error **errp)
{
UniCore32CPUClass *ucc = UNICORE32_CPU_GET_CLASS(dev);
+ qemu_init_vcpu(CPU(dev));
+
ucc->parent_realize(dev, errp);
}
diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
index e966aa0..c19d17a 100644
--- a/target-xtensa/cpu.c
+++ b/target-xtensa/cpu.c
@@ -90,6 +90,8 @@ static void xtensa_cpu_realizefn(DeviceState *dev, Error **errp)
cs->gdb_num_regs = xcc->config->gdb_regmap.num_regs;
+ qemu_init_vcpu(cs);
+
xcc->parent_realize(dev, errp);
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PULL for-1.6 2/3] target-i386: Pass X86CPU object to cpu_x86_find_by_name()
2013-07-29 15:32 [Qemu-devel] [PULL 0/3] QOM CPUState patch queue 2013-07-29 Andreas Färber
2013-07-29 15:32 ` [Qemu-devel] [PULL for-1.6 1/3] cpu: Partially revert "cpu: Change qemu_init_vcpu() argument to CPUState" Andreas Färber
@ 2013-07-29 15:32 ` Andreas Färber
2013-07-29 15:32 ` [Qemu-devel] [PULL for-1.6 3/3] target-i386: Disable PMU CPUID leaf by default Andreas Färber
2 siblings, 0 replies; 5+ messages in thread
From: Andreas Färber @ 2013-07-29 15:32 UTC (permalink / raw)
To: qemu-devel; +Cc: Eduardo Habkost, Andreas Färber
From: Eduardo Habkost <ehabkost@redhat.com>
This will help us change the initialization code to not require carrying
some intermediate values in a x86_def_t struct (and eventually kill the
x86_def_t struct entirely).
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
target-i386/cpu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index df2fb1b..80143bf 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1475,7 +1475,8 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
error_propagate(errp, err);
}
-static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
+static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
+ const char *name)
{
x86_def_t *def;
int i;
@@ -1742,7 +1743,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
memset(def, 0, sizeof(*def));
- if (cpu_x86_find_by_name(def, name) < 0) {
+ if (cpu_x86_find_by_name(cpu, def, name) < 0) {
error_setg(errp, "Unable to find CPU definition: %s", name);
return;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PULL for-1.6 3/3] target-i386: Disable PMU CPUID leaf by default
2013-07-29 15:32 [Qemu-devel] [PULL 0/3] QOM CPUState patch queue 2013-07-29 Andreas Färber
2013-07-29 15:32 ` [Qemu-devel] [PULL for-1.6 1/3] cpu: Partially revert "cpu: Change qemu_init_vcpu() argument to CPUState" Andreas Färber
2013-07-29 15:32 ` [Qemu-devel] [PULL for-1.6 2/3] target-i386: Pass X86CPU object to cpu_x86_find_by_name() Andreas Färber
@ 2013-07-29 15:32 ` Andreas Färber
2 siblings, 0 replies; 5+ messages in thread
From: Andreas Färber @ 2013-07-29 15:32 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Eduardo Habkost, Andreas Färber
From: Eduardo Habkost <ehabkost@redhat.com>
Bug description: QEMU currently gets all bits from GET_SUPPORTED_CPUID
for CPUID leaf 0xA and passes them directly to the guest. This makes
the guest ABI depend on host kernel and host CPU capabilities, and
breaks live migration if we migrate between hosts with different
capabilities (e.g., different number of PMU counters).
Add a "pmu" property to X86CPU, and set it to true only on "-cpu host",
or on pc-*-1.5 and older machine-types.
For now, setting pmu=on will enable the current passthrough mode that
doesn't have any ABI stability guarantees, but in the future we may
implement a mode where the PMU CPUID bits are stable and configurable.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
include/hw/i386/pc.h | 4 ++++
target-i386/cpu-qom.h | 7 +++++++
target-i386/cpu.c | 11 ++++++++++-
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 7fb97b0..09c2dd4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -235,6 +235,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
.driver = "virtio-net-pci",\
.property = "any_layout",\
.value = "off",\
+ },{\
+ .driver = TYPE_X86_CPU,\
+ .property = "pmu",\
+ .value = "on",\
}
#define PC_COMPAT_1_4 \
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 60d2b5d..53b4c34 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -68,6 +68,13 @@ typedef struct X86CPU {
/* Features that were filtered out because of missing host capabilities */
uint32_t filtered_features[FEATURE_WORDS];
+
+ /* Enable PMU CPUID bits. This can't be enabled by default yet because
+ * it doesn't have ABI stability guarantees, as it passes all PMU CPUID
+ * bits returned by GET_SUPPORTED_CPUID (that depend on host CPU and kernel
+ * capabilities) directly to the guest.
+ */
+ bool enable_pmu;
} X86CPU;
static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 80143bf..71ab915 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1479,6 +1479,7 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
const char *name)
{
x86_def_t *def;
+ Error *err = NULL;
int i;
if (name == NULL) {
@@ -1486,6 +1487,8 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
}
if (kvm_enabled() && strcmp(name, "host") == 0) {
kvm_cpu_fill_host(x86_cpu_def);
+ object_property_set_bool(OBJECT(cpu), true, "pmu", &err);
+ assert_no_error(err);
return 0;
}
@@ -2017,7 +2020,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
break;
case 0xA:
/* Architectural Performance Monitoring Leaf */
- if (kvm_enabled()) {
+ if (kvm_enabled() && cpu->enable_pmu) {
KVMState *s = cs->kvm_state;
*eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX);
@@ -2523,6 +2526,11 @@ static void x86_cpu_synchronize_from_tb(CPUState *cs, TranslationBlock *tb)
cpu->env.eip = tb->pc - tb->cs_base;
}
+static Property x86_cpu_properties[] = {
+ DEFINE_PROP_BOOL("pmu", X86CPU, enable_pmu, false),
+ DEFINE_PROP_END_OF_LIST()
+};
+
static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
{
X86CPUClass *xcc = X86_CPU_CLASS(oc);
@@ -2532,6 +2540,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
xcc->parent_realize = dc->realize;
dc->realize = x86_cpu_realizefn;
dc->bus_type = TYPE_ICC_BUS;
+ dc->props = x86_cpu_properties;
xcc->parent_reset = cc->reset;
cc->reset = x86_cpu_reset;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread