Re: [PATCH v2 3/3] target/arm: Use object_property_add_bool for "sve" property

[Andrew Jones]

On Thu, Aug 13, 2020 at 01:02:43PM -0700, Richard Henderson wrote:
> The interface for object_property_add_bool is simpler,
> making the code easier to understand.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/cpu64.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 0227862d39..cce0da0b90 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -488,6 +488,12 @@ static void cpu_max_set_sve_max_vq(Object *obj, Visitor 
> *v, const char *name,
>      cpu->sve_max_vq = max_vq;
>  }
>  
> +/*
> + * Note that cpu_arm_get/set_sve_vq cannot use the simpler
> + * object_property_add_bool interface because they make use
> + * of the contents of "name" to determine which bit on which
> + * to operate.
> + */
>  static void cpu_arm_get_sve_vq(Object *obj, Visitor *v, const char *name,
>                                 void *opaque, Error **errp)
>  {
> @@ -529,26 +535,17 @@ static void cpu_arm_set_sve_vq(Object *obj, Visitor *v, 
> const char *name,
>      set_bit(vq - 1, cpu->sve_vq_init);
>  }
>  
> -static void cpu_arm_get_sve(Object *obj, Visitor *v, const char *name,
> -                            void *opaque, Error **errp)
> +static bool cpu_arm_get_sve(Object *obj, Error **errp)
>  {
>      ARMCPU *cpu = ARM_CPU(obj);
> -    bool value = cpu_isar_feature(aa64_sve, cpu);
> -
> -    visit_type_bool(v, name, &value, errp);
> +    return cpu_isar_feature(aa64_sve, cpu);
>  }
>  
> -static void cpu_arm_set_sve(Object *obj, Visitor *v, const char *name,
> -                            void *opaque, Error **errp)
> +static void cpu_arm_set_sve(Object *obj, bool value, Error **errp)
>  {
>      ARMCPU *cpu = ARM_CPU(obj);
> -    bool value;
>      uint64_t t;
>  
> -    if (!visit_type_bool(v, name, &value, errp)) {
> -        return;
> -    }
> -
>      if (value && kvm_enabled() && !kvm_arm_sve_supported()) {
>          error_setg(errp, "'sve' feature not supported by KVM on this host");
>          return;
> @@ -563,8 +560,7 @@ void aarch64_add_sve_properties(Object *obj)
>  {
>      uint32_t vq;
>  
> -    object_property_add(obj, "sve", "bool", cpu_arm_get_sve,
> -                        cpu_arm_set_sve, NULL, NULL);
> +    object_property_add_bool(obj, "sve", cpu_arm_get_sve, cpu_arm_set_sve);
>  
>      for (vq = 1; vq <= ARM_MAX_VQ; ++vq) {
>          char name[8];
> -- 
> 2.25.1
> 
>

Reviewed-by: Andrew Jones <drjones@redhat.com>