[Qemu-devel] [PATCH] i386: make 'hv-spinlocks' a regular uint32 property

[Qemu-devel] [PATCH] i386: make 'hv-spinlocks' a regular uint32 property

[Roman Kagan]

X86CPU.hv-spinlocks is a uint32 property that has a special setter
validating the value to be no less than 0xFFF and no bigger than
UINT_MAX.  The latter check is redundant; as for the former, there
appears to be no reason to prohibit the user from setting it to a lower
value.

So nuke the dedicated getter/setter pair and convert 'hv-spinlocks' to a
regular uint32 property.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
Based-on: <20190615200505.31348-1-ehabkost@redhat.com>
([PATCH] i386: Fix signedness of hyperv_spinlock_attempts)

 target/i386/cpu.c | 45 ++-------------------------------------------
 1 file changed, 2 insertions(+), 43 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index fbed2eb804..843d45262e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3497,46 +3497,6 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
     visit_type_X86CPUFeatureWordInfoList(v, "feature-words", &list, errp);
 }
 
-static void x86_get_hv_spinlocks(Object *obj, Visitor *v, const char *name,
-                                 void *opaque, Error **errp)
-{
-    X86CPU *cpu = X86_CPU(obj);
-    int64_t value = cpu->hyperv_spinlock_attempts;
-
-    visit_type_int(v, name, &value, errp);
-}
-
-static void x86_set_hv_spinlocks(Object *obj, Visitor *v, const char *name,
-                                 void *opaque, Error **errp)
-{
-    const int64_t min = 0xFFF;
-    const int64_t max = UINT_MAX;
-    X86CPU *cpu = X86_CPU(obj);
-    Error *err = NULL;
-    int64_t value;
-
-    visit_type_int(v, name, &value, &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
-
-    if (value < min || value > max) {
-        error_setg(errp, "Property %s.%s doesn't take value %" PRId64
-                   " (minimum: %" PRId64 ", maximum: %" PRId64 ")",
-                   object_get_typename(obj), name ? name : "null",
-                   value, min, max);
-        return;
-    }
-    cpu->hyperv_spinlock_attempts = value;
-}
-
-static const PropertyInfo qdev_prop_spinlocks = {
-    .name  = "int",
-    .get   = x86_get_hv_spinlocks,
-    .set   = x86_set_hv_spinlocks,
-};
-
 /* Convert all '_' in a feature string option name to '-', to make feature
  * name conform to QOM property naming rule, which uses '-' instead of '_'.
  */
@@ -5658,8 +5618,6 @@ static void x86_cpu_initfn(Object *obj)
     object_property_add(obj, "crash-information", "GuestPanicInformation",
                         x86_cpu_get_crash_info_qom, NULL, NULL, NULL, NULL);
 
-    cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
-
     for (w = 0; w < FEATURE_WORDS; w++) {
         int bitnr;
 
@@ -5853,7 +5811,8 @@ static Property x86_cpu_properties[] = {
 #endif
     DEFINE_PROP_INT32("node-id", X86CPU, node_id, CPU_UNSET_NUMA_NODE_ID),
     DEFINE_PROP_BOOL("pmu", X86CPU, enable_pmu, false),
-    { .name  = "hv-spinlocks", .info  = &qdev_prop_spinlocks },
+    DEFINE_PROP_UINT32("hv-spinlocks", X86CPU, hyperv_spinlock_attempts,
+                       HYPERV_SPINLOCK_NEVER_RETRY),
     DEFINE_PROP_BOOL("hv-relaxed", X86CPU, hyperv_relaxed_timing, false),
     DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false),
     DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
-- 
2.21.0

Re: [Qemu-devel] [PATCH] i386: make 'hv-spinlocks' a regular uint32 property

[Eduardo Habkost]

On Tue, Jun 18, 2019 at 11:07:06AM +0000, Roman Kagan wrote:
> X86CPU.hv-spinlocks is a uint32 property that has a special setter
> validating the value to be no less than 0xFFF and no bigger than
> UINT_MAX.  The latter check is redundant; as for the former, there
> appears to be no reason to prohibit the user from setting it to a lower
> value.
> 
> So nuke the dedicated getter/setter pair and convert 'hv-spinlocks' to a
> regular uint32 property.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

Queued on x86-next.  Thanks!

-- 
Eduardo