diff mbox series

[14/24] i386: Register feature bit properties as class properties

Message ID 20200921221045.699690-15-ehabkost@redhat.com (mailing list archive)
State New, archived
Headers show
Series qom: Convert some properties to class properties | expand

Commit Message

Eduardo Habkost Sept. 21, 2020, 10:10 p.m. UTC
Class properties make QOM introspection simpler and easier, as
they don't require an object to be instantiated.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 target/i386/cpu.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

Comments

Igor Mammedov Sept. 22, 2020, 6:47 a.m. UTC | #1
On Mon, 21 Sep 2020 18:10:35 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Class properties make QOM introspection simpler and easier, as
> they don't require an object to be instantiated.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: qemu-devel@nongnu.org

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  target/i386/cpu.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 66792f28ba7..1aba87e0a5b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6819,16 +6819,17 @@ static void x86_cpu_release_bit_prop(Object *obj, const char *name,
>   * multiple bits in the same FeatureWord. In that case, the getter will return
>   * true only if all bits are set.
>   */
> -static void x86_cpu_register_bit_prop(X86CPU *cpu,
> +static void x86_cpu_register_bit_prop(X86CPUClass *xcc,
>                                        const char *prop_name,
>                                        FeatureWord w,
>                                        int bitnr)
>  {
> +    ObjectClass *oc = OBJECT_CLASS(xcc);
>      BitProperty *fp;
>      ObjectProperty *op;
>      uint64_t mask = (1ULL << bitnr);
>  
> -    op = object_property_find(OBJECT(cpu), prop_name, NULL);
> +    op = object_class_property_find(oc, prop_name, NULL);
>      if (op) {
>          fp = op->opaque;
>          assert(fp->w == w);
> @@ -6837,14 +6838,14 @@ static void x86_cpu_register_bit_prop(X86CPU *cpu,
>          fp = g_new0(BitProperty, 1);
>          fp->w = w;
>          fp->mask = mask;
> -        object_property_add(OBJECT(cpu), prop_name, "bool",
> -                            x86_cpu_get_bit_prop,
> -                            x86_cpu_set_bit_prop,
> -                            x86_cpu_release_bit_prop, fp);
> +        object_class_property_add(oc, prop_name, "bool",
> +                                  x86_cpu_get_bit_prop,
> +                                  x86_cpu_set_bit_prop,
> +                                  x86_cpu_release_bit_prop, fp);
>      }
>  }
>  
> -static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
> +static void x86_cpu_register_feature_bit_props(X86CPUClass *xcc,
>                                                 FeatureWord w,
>                                                 int bitnr)
>  {
> @@ -6863,7 +6864,7 @@ static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
>      /* aliases don't use "|" delimiters anymore, they are registered
>       * manually using object_property_add_alias() */
>      assert(!strchr(name, '|'));
> -    x86_cpu_register_bit_prop(cpu, name, w, bitnr);
> +    x86_cpu_register_bit_prop(xcc, name, w, bitnr);
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> @@ -6917,7 +6918,6 @@ static void x86_cpu_initfn(Object *obj)
>      X86CPU *cpu = X86_CPU(obj);
>      X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
>      CPUX86State *env = &cpu->env;
> -    FeatureWord w;
>  
>      env->nr_dies = 1;
>      cpu_set_cpustate_pointers(cpu);
> @@ -6929,14 +6929,6 @@ static void x86_cpu_initfn(Object *obj)
>                          x86_cpu_get_feature_words,
>                          NULL, NULL, (void *)cpu->filtered_features);
>  
> -    for (w = 0; w < FEATURE_WORDS; w++) {
> -        int bitnr;
> -
> -        for (bitnr = 0; bitnr < 64; bitnr++) {
> -            x86_cpu_register_feature_bit_props(cpu, w, bitnr);
> -        }
> -    }
> -
>      object_property_add_alias(obj, "sse3", obj, "pni");
>      object_property_add_alias(obj, "pclmuldq", obj, "pclmulqdq");
>      object_property_add_alias(obj, "sse4-1", obj, "sse4.1");
> @@ -7221,6 +7213,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>      X86CPUClass *xcc = X86_CPU_CLASS(oc);
>      CPUClass *cc = CPU_CLASS(oc);
>      DeviceClass *dc = DEVICE_CLASS(oc);
> +    FeatureWord w;
>  
>      device_class_set_parent_realize(dc, x86_cpu_realizefn,
>                                      &xcc->parent_realize);
> @@ -7310,6 +7303,12 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>                                x86_cpu_get_crash_info_qom, NULL, NULL, NULL);
>  #endif
>  
> +    for (w = 0; w < FEATURE_WORDS; w++) {
> +        int bitnr;
> +        for (bitnr = 0; bitnr < 64; bitnr++) {
> +            x86_cpu_register_feature_bit_props(xcc, w, bitnr);
> +        }
> +    }
>  }
>  
>  static const TypeInfo x86_cpu_type_info = {
Eduardo Habkost Sept. 22, 2020, 12:47 p.m. UTC | #2
On Mon, Sep 21, 2020 at 06:10:35PM -0400, Eduardo Habkost wrote:
> Class properties make QOM introspection simpler and easier, as
> they don't require an object to be instantiated.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
>  target/i386/cpu.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 66792f28ba7..1aba87e0a5b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6819,16 +6819,17 @@ static void x86_cpu_release_bit_prop(Object *obj, const char *name,
>   * multiple bits in the same FeatureWord. In that case, the getter will return
>   * true only if all bits are set.
>   */
> -static void x86_cpu_register_bit_prop(X86CPU *cpu,
> +static void x86_cpu_register_bit_prop(X86CPUClass *xcc,
>                                        const char *prop_name,
>                                        FeatureWord w,
>                                        int bitnr)
>  {
> +    ObjectClass *oc = OBJECT_CLASS(xcc);
>      BitProperty *fp;
>      ObjectProperty *op;
>      uint64_t mask = (1ULL << bitnr);
>  
> -    op = object_property_find(OBJECT(cpu), prop_name, NULL);
> +    op = object_class_property_find(oc, prop_name, NULL);
>      if (op) {
>          fp = op->opaque;
>          assert(fp->w == w);
> @@ -6837,14 +6838,14 @@ static void x86_cpu_register_bit_prop(X86CPU *cpu,
>          fp = g_new0(BitProperty, 1);
>          fp->w = w;
>          fp->mask = mask;
> -        object_property_add(OBJECT(cpu), prop_name, "bool",
> -                            x86_cpu_get_bit_prop,
> -                            x86_cpu_set_bit_prop,
> -                            x86_cpu_release_bit_prop, fp);
> +        object_class_property_add(oc, prop_name, "bool",
> +                                  x86_cpu_get_bit_prop,
> +                                  x86_cpu_set_bit_prop,
> +                                  x86_cpu_release_bit_prop, fp);

This one is broken.  x86_cpu_release_bit_prop() frees fp, and
now fp should never be freed.  The release function shouldn't be
set.

I will send a new version of this patch later.

> [...]
diff mbox series

Patch

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 66792f28ba7..1aba87e0a5b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6819,16 +6819,17 @@  static void x86_cpu_release_bit_prop(Object *obj, const char *name,
  * multiple bits in the same FeatureWord. In that case, the getter will return
  * true only if all bits are set.
  */
-static void x86_cpu_register_bit_prop(X86CPU *cpu,
+static void x86_cpu_register_bit_prop(X86CPUClass *xcc,
                                       const char *prop_name,
                                       FeatureWord w,
                                       int bitnr)
 {
+    ObjectClass *oc = OBJECT_CLASS(xcc);
     BitProperty *fp;
     ObjectProperty *op;
     uint64_t mask = (1ULL << bitnr);
 
-    op = object_property_find(OBJECT(cpu), prop_name, NULL);
+    op = object_class_property_find(oc, prop_name, NULL);
     if (op) {
         fp = op->opaque;
         assert(fp->w == w);
@@ -6837,14 +6838,14 @@  static void x86_cpu_register_bit_prop(X86CPU *cpu,
         fp = g_new0(BitProperty, 1);
         fp->w = w;
         fp->mask = mask;
-        object_property_add(OBJECT(cpu), prop_name, "bool",
-                            x86_cpu_get_bit_prop,
-                            x86_cpu_set_bit_prop,
-                            x86_cpu_release_bit_prop, fp);
+        object_class_property_add(oc, prop_name, "bool",
+                                  x86_cpu_get_bit_prop,
+                                  x86_cpu_set_bit_prop,
+                                  x86_cpu_release_bit_prop, fp);
     }
 }
 
-static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
+static void x86_cpu_register_feature_bit_props(X86CPUClass *xcc,
                                                FeatureWord w,
                                                int bitnr)
 {
@@ -6863,7 +6864,7 @@  static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
     /* aliases don't use "|" delimiters anymore, they are registered
      * manually using object_property_add_alias() */
     assert(!strchr(name, '|'));
-    x86_cpu_register_bit_prop(cpu, name, w, bitnr);
+    x86_cpu_register_bit_prop(xcc, name, w, bitnr);
 }
 
 #if !defined(CONFIG_USER_ONLY)
@@ -6917,7 +6918,6 @@  static void x86_cpu_initfn(Object *obj)
     X86CPU *cpu = X86_CPU(obj);
     X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
     CPUX86State *env = &cpu->env;
-    FeatureWord w;
 
     env->nr_dies = 1;
     cpu_set_cpustate_pointers(cpu);
@@ -6929,14 +6929,6 @@  static void x86_cpu_initfn(Object *obj)
                         x86_cpu_get_feature_words,
                         NULL, NULL, (void *)cpu->filtered_features);
 
-    for (w = 0; w < FEATURE_WORDS; w++) {
-        int bitnr;
-
-        for (bitnr = 0; bitnr < 64; bitnr++) {
-            x86_cpu_register_feature_bit_props(cpu, w, bitnr);
-        }
-    }
-
     object_property_add_alias(obj, "sse3", obj, "pni");
     object_property_add_alias(obj, "pclmuldq", obj, "pclmulqdq");
     object_property_add_alias(obj, "sse4-1", obj, "sse4.1");
@@ -7221,6 +7213,7 @@  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
     CPUClass *cc = CPU_CLASS(oc);
     DeviceClass *dc = DEVICE_CLASS(oc);
+    FeatureWord w;
 
     device_class_set_parent_realize(dc, x86_cpu_realizefn,
                                     &xcc->parent_realize);
@@ -7310,6 +7303,12 @@  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
                               x86_cpu_get_crash_info_qom, NULL, NULL, NULL);
 #endif
 
+    for (w = 0; w < FEATURE_WORDS; w++) {
+        int bitnr;
+        for (bitnr = 0; bitnr < 64; bitnr++) {
+            x86_cpu_register_feature_bit_props(xcc, w, bitnr);
+        }
+    }
 }
 
 static const TypeInfo x86_cpu_type_info = {