| Message ID | 20160609183430.GC17952@thinpad.lan.raisama.net |
|---|---|
| State | New |
| Headers | show |
On Thu, 9 Jun 2016 15:34:30 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Mon, Jun 06, 2016 at 05:16:45PM +0200, Igor Mammedov wrote: > > it will allow to drop custom cpu_x86_init() and use > > cpu_generic_init() insteadi, reducing cpu_x86_create() > > to a simple 3-liner. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Eduardo Habkost <ehabkost@redhat.com> > > This triggers an assert when trying to use -cpu host with TCG: > > # ./x86_64-softmmu/qemu-system-x86_64 -machine accel=tcg -cpu host -nographic > qemu-system-x86_64: /root/qemu/target-i386/cpu.c:1558: host_x86_cpu_initfn: Assertion `(kvm_allowed)' failed. > Aborted This is not related to this patch and it was there before. Maybe a separate patch saying that it replaces assert() in host_x86_cpu_initfn() with error check and user friendly error message at later stage. > > I will squash the following fix in it: > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 3f886a5..dcbfa0b 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1555,16 +1555,17 @@ static void host_x86_cpu_initfn(Object *obj) > CPUX86State *env = &cpu->env; > KVMState *s = kvm_state; > > - assert(kvm_enabled()); > - > /* We can't fill the features array here because we don't know yet if > * "migratable" is true or false. > */ > cpu->host_features = true; > > - env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX); > - env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX); > - env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > + /* If KVM is disabled, x86_cpu_realizefn() will report an error later */ > + if (kvm_enabled()) { > + env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX); > + env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX); > + env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > + } > > object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort); > } >
On Fri, Jun 10, 2016 at 09:15:44AM +0200, Igor Mammedov wrote: > On Thu, 9 Jun 2016 15:34:30 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Mon, Jun 06, 2016 at 05:16:45PM +0200, Igor Mammedov wrote: > > > it will allow to drop custom cpu_x86_init() and use > > > cpu_generic_init() insteadi, reducing cpu_x86_create() > > > to a simple 3-liner. > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > Eduardo Habkost <ehabkost@redhat.com> > > > > This triggers an assert when trying to use -cpu host with TCG: > > > > # ./x86_64-softmmu/qemu-system-x86_64 -machine accel=tcg -cpu host -nographic > > qemu-system-x86_64: /root/qemu/target-i386/cpu.c:1558: host_x86_cpu_initfn: Assertion `(kvm_allowed)' failed. > > Aborted > This is not related to this patch and it was there before. It is caused by this patch. The assert() was there, but you removed the check on cpu_x86_create() that prevented it from being triggered. > > Maybe a separate patch saying that it replaces assert() in > host_x86_cpu_initfn() with error check and user friendly > error message at later stage. We could apply the fix below as a separate patch before 03/10, or just squash the fix in it. What do you think? (We can't do it as a follow-up patch.) If you prefer to suggest another fix, I will remove the series from x86-next and wait for v3. > > > > > I will squash the following fix in it: > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 3f886a5..dcbfa0b 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -1555,16 +1555,17 @@ static void host_x86_cpu_initfn(Object *obj) > > CPUX86State *env = &cpu->env; > > KVMState *s = kvm_state; > > > > - assert(kvm_enabled()); > > - > > /* We can't fill the features array here because we don't know yet if > > * "migratable" is true or false. > > */ > > cpu->host_features = true; > > > > - env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX); > > - env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX); > > - env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > > + /* If KVM is disabled, x86_cpu_realizefn() will report an error later */ > > + if (kvm_enabled()) { > > + env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX); > > + env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX); > > + env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > > + } > > > > object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort); > > } > > >
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 3f886a5..dcbfa0b 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1555,16 +1555,17 @@ static void host_x86_cpu_initfn(Object *obj) CPUX86State *env = &cpu->env; KVMState *s = kvm_state; - assert(kvm_enabled()); - /* We can't fill the features array here because we don't know yet if * "migratable" is true or false. */ cpu->host_features = true; - env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX); - env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX); - env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); + /* If KVM is disabled, x86_cpu_realizefn() will report an error later */ + if (kvm_enabled()) { + env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX); + env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX); + env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); + } object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort); }