qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC] qmp, target-i386: device_list_properties for TYPE_CPU
@ 2016-02-12  9:13 Valentin Rakush
  2016-02-12 11:30 ` Andreas Färber
  0 siblings, 1 reply; 3+ messages in thread
From: Valentin Rakush @ 2016-02-12  9:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Markus Armbruster, Luiz Capitulino,
	Andreas Färber, asmetanin, den, Valentin Rakush

This is RFC because there is another implementation option: it is
possible to implement this functionality in the object_finalize for
all available targets. All targets change will require more testing.
Please let me know if all targets should be changed at once.

This patch changes qmp_device_list_properties and only target-i386
to allow TYPE_CPU class properties to be quered with QMP interface and
with -device core2duo-x86_64-cpu,help command line.

Signed-off-by: Valentin Rakush <valentin.rakush@gmail.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
---
 qmp.c             | 19 +++++++++++++++++++
 target-i386/cpu.c | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/qmp.c b/qmp.c
index 6ae7230..2721f16 100644
--- a/qmp.c
+++ b/qmp.c
@@ -516,6 +516,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
                                                    Error **errp)
 {
     ObjectClass *klass;
+    ObjectClass *cpu_klass;
     Object *obj;
     ObjectProperty *prop;
     ObjectPropertyIterator iter;
@@ -580,6 +581,24 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
         prop_list = entry;
     }
 
+    cpu_klass = object_class_dynamic_cast(klass, TYPE_CPU);
+    if (cpu_klass) {
+        CPUState *tmp_cpu = CPU(obj);
+        CPUState *cs = first_cpu;
+
+        CPU_FOREACH(cs) {
+            if (tmp_cpu->cpu_index == cs->cpu_index) {
+#if defined(CONFIG_USER_ONLY)
+                cpu_list_lock();
+#endif
+                QTAILQ_REMOVE(&cpus, cs, node);
+#if defined(CONFIG_USER_ONLY)
+                cpu_list_unlock();
+#endif
+            }
+        }
+    }
+
     object_unref(obj);
 
     return prop_list;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3fa14bf..8c32794 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1479,7 +1479,7 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
 
     dc->props = host_x86_cpu_properties;
     /* Reason: host_x86_cpu_initfn() dies when !kvm_enabled() */
-    dc->cannot_destroy_with_object_finalize_yet = true;
+    dc->cannot_destroy_with_object_finalize_yet = false;
 }
 
 static void host_x86_cpu_initfn(Object *obj)
@@ -3225,11 +3225,39 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     cc->cpu_exec_enter = x86_cpu_exec_enter;
     cc->cpu_exec_exit = x86_cpu_exec_exit;
 
+    object_class_property_add(oc, "family", "int",
+                        x86_cpuid_version_get_family,
+                        x86_cpuid_version_set_family, NULL, NULL, NULL);
+    object_class_property_add(oc, "model", "int",
+                        x86_cpuid_version_get_model,
+                        x86_cpuid_version_set_model, NULL, NULL, NULL);
+    object_class_property_add(oc, "stepping", "int",
+                        x86_cpuid_version_get_stepping,
+                        x86_cpuid_version_set_stepping, NULL, NULL, NULL);
+    object_class_property_add_str(oc, "vendor",
+                            x86_cpuid_get_vendor,
+                            x86_cpuid_set_vendor, NULL);
+    object_class_property_add_str(oc, "model-id",
+                            x86_cpuid_get_model_id,
+                            x86_cpuid_set_model_id, NULL);
+    object_class_property_add(oc, "tsc-frequency", "int",
+                        x86_cpuid_get_tsc_freq,
+                        x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
+    object_class_property_add(oc, "apic-id", "int",
+                        x86_cpuid_get_apic_id,
+                        x86_cpuid_set_apic_id, NULL, NULL, NULL);
+    object_class_property_add(oc, "feature-words", "X86CPUFeatureWordInfo",
+                        x86_cpu_get_feature_words,
+                        NULL, NULL, NULL, NULL);
+    object_class_property_add(oc, "filtered-features", "X86CPUFeatureWordInfo",
+                        x86_cpu_get_feature_words,
+                        NULL, NULL, NULL, NULL);
+
     /*
      * Reason: x86_cpu_initfn() calls cpu_exec_init(), which saves the
      * object in cpus -> dangling pointer after final object_unref().
      */
-    dc->cannot_destroy_with_object_finalize_yet = true;
+    dc->cannot_destroy_with_object_finalize_yet = false;
 }
 
 static const TypeInfo x86_cpu_type_info = {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] qmp, target-i386: device_list_properties for TYPE_CPU
  2016-02-12  9:13 [Qemu-devel] [PATCH RFC] qmp, target-i386: device_list_properties for TYPE_CPU Valentin Rakush
@ 2016-02-12 11:30 ` Andreas Färber
  2016-02-12 18:21   ` Eduardo Habkost
  0 siblings, 1 reply; 3+ messages in thread
From: Andreas Färber @ 2016-02-12 11:30 UTC (permalink / raw)
  To: Valentin Rakush, qemu-devel
  Cc: Eduardo Habkost, Markus Armbruster, Luiz Capitulino, asmetanin, den

Hi,

Am 12.02.2016 um 10:13 schrieb Valentin Rakush:
> This is RFC because there is another implementation option: it is
> possible to implement this functionality in the object_finalize for
> all available targets. All targets change will require more testing.
> Please let me know if all targets should be changed at once.

I thought I had already seen some Fujitsu/IBM patch to fix this...
(pointers appreciated) It should be fixed on the level where the problem
is introduced - i.e. if qom/cpu.c calls the cpu_exec_init() in
instance_init it needs to call the corresponding exit function in
instance_finalize; dito for target-i386/cpu.c or wherever. Or we try to
move it from instance_init to realize, avoiding it getting called in the
first place.

> 
> This patch changes qmp_device_list_properties and only target-i386
> to allow TYPE_CPU class properties to be quered with QMP interface and
> with -device core2duo-x86_64-cpu,help command line.
> 
> Signed-off-by: Valentin Rakush <valentin.rakush@gmail.com>
> Cc: Luiz Capitulino <lcapitulino@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Andreas Färber <afaerber@suse.de>
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qmp.c             | 19 +++++++++++++++++++
>  target-i386/cpu.c | 32 ++++++++++++++++++++++++++++++--
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/qmp.c b/qmp.c
> index 6ae7230..2721f16 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -516,6 +516,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>                                                     Error **errp)
>  {
>      ObjectClass *klass;
> +    ObjectClass *cpu_klass;

Please use cpu_class, only "class" is a reserved identifier.

>      Object *obj;
>      ObjectProperty *prop;
>      ObjectPropertyIterator iter;
> @@ -580,6 +581,24 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>          prop_list = entry;
>      }
>  
> +    cpu_klass = object_class_dynamic_cast(klass, TYPE_CPU);
> +    if (cpu_klass) {
> +        CPUState *tmp_cpu = CPU(obj);
> +        CPUState *cs = first_cpu;
> +
> +        CPU_FOREACH(cs) {
> +            if (tmp_cpu->cpu_index == cs->cpu_index) {
> +#if defined(CONFIG_USER_ONLY)
> +                cpu_list_lock();
> +#endif
> +                QTAILQ_REMOVE(&cpus, cs, node);
> +#if defined(CONFIG_USER_ONLY)
> +                cpu_list_unlock();
> +#endif
> +            }
> +        }
> +    }
> +
>      object_unref(obj);
>  
>      return prop_list;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3fa14bf..8c32794 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1479,7 +1479,7 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
>  
>      dc->props = host_x86_cpu_properties;
>      /* Reason: host_x86_cpu_initfn() dies when !kvm_enabled() */
> -    dc->cannot_destroy_with_object_finalize_yet = true;
> +    dc->cannot_destroy_with_object_finalize_yet = false;
>  }
>  
>  static void host_x86_cpu_initfn(Object *obj)
> @@ -3225,11 +3225,39 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>      cc->cpu_exec_enter = x86_cpu_exec_enter;
>      cc->cpu_exec_exit = x86_cpu_exec_exit;
>  
> +    object_class_property_add(oc, "family", "int",
> +                        x86_cpuid_version_get_family,
> +                        x86_cpuid_version_set_family, NULL, NULL, NULL);

You add class properties here but I don't see you deleting the previous
instance properties anywhere?

> +    object_class_property_add(oc, "model", "int",
> +                        x86_cpuid_version_get_model,
> +                        x86_cpuid_version_set_model, NULL, NULL, NULL);
> +    object_class_property_add(oc, "stepping", "int",
> +                        x86_cpuid_version_get_stepping,
> +                        x86_cpuid_version_set_stepping, NULL, NULL, NULL);
> +    object_class_property_add_str(oc, "vendor",
> +                            x86_cpuid_get_vendor,
> +                            x86_cpuid_set_vendor, NULL);
> +    object_class_property_add_str(oc, "model-id",
> +                            x86_cpuid_get_model_id,
> +                            x86_cpuid_set_model_id, NULL);
> +    object_class_property_add(oc, "tsc-frequency", "int",
> +                        x86_cpuid_get_tsc_freq,
> +                        x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> +    object_class_property_add(oc, "apic-id", "int",
> +                        x86_cpuid_get_apic_id,
> +                        x86_cpuid_set_apic_id, NULL, NULL, NULL);
> +    object_class_property_add(oc, "feature-words", "X86CPUFeatureWordInfo",
> +                        x86_cpu_get_feature_words,
> +                        NULL, NULL, NULL, NULL);
> +    object_class_property_add(oc, "filtered-features", "X86CPUFeatureWordInfo",
> +                        x86_cpu_get_feature_words,
> +                        NULL, NULL, NULL, NULL);
> +
>      /*
>       * Reason: x86_cpu_initfn() calls cpu_exec_init(), which saves the
>       * object in cpus -> dangling pointer after final object_unref().
>       */
> -    dc->cannot_destroy_with_object_finalize_yet = true;
> +    dc->cannot_destroy_with_object_finalize_yet = false;

This looks bogus, Markus will need to take a close look.

>  }
>  
>  static const TypeInfo x86_cpu_type_info = {
> 

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] qmp, target-i386: device_list_properties for TYPE_CPU
  2016-02-12 11:30 ` Andreas Färber
@ 2016-02-12 18:21   ` Eduardo Habkost
  0 siblings, 0 replies; 3+ messages in thread
From: Eduardo Habkost @ 2016-02-12 18:21 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Markus Armbruster, qemu-devel, Luiz Capitulino, asmetanin, den,
	Valentin Rakush

On Fri, Feb 12, 2016 at 12:30:34PM +0100, Andreas Färber wrote:
> Hi,
> 
> Am 12.02.2016 um 10:13 schrieb Valentin Rakush:
> > This is RFC because there is another implementation option: it is
> > possible to implement this functionality in the object_finalize for
> > all available targets. All targets change will require more testing.
> > Please let me know if all targets should be changed at once.
> 
> I thought I had already seen some Fujitsu/IBM patch to fix this...
> (pointers appreciated) It should be fixed on the level where the problem
> is introduced - i.e. if qom/cpu.c calls the cpu_exec_init() in
> instance_init it needs to call the corresponding exit function in
> instance_finalize; dito for target-i386/cpu.c or wherever. Or we try to
> move it from instance_init to realize, avoiding it getting called in the
> first place.

cpu_exec_init() calls must all be moved to realize.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-02-12 18:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-12  9:13 [Qemu-devel] [PATCH RFC] qmp, target-i386: device_list_properties for TYPE_CPU Valentin Rakush
2016-02-12 11:30 ` Andreas Färber
2016-02-12 18:21   ` Eduardo Habkost

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).