Re: [PATCH v2 04/10] hw/arm: Use nodefault version of qdev props when not needed

Mark Cave-Ayland <[email protected]> Wed, 10 Jun 2026 16:31:49 +0100
Newsgroups org.nongnu.qemu-rust,org.nongnu.qemu-arm,org.nongnu.qemu-devel
Message-ID <[email protected]>
On 09/06/2026 18:25, Peter Xu wrote:

> We're going to introduce defvars for object properties just like qdev-props
> defvars.  It work slightly differently in that props can be added in
> instance_init() with default values, then the default values can only apply
> after instance_init() completes.
> 
> It means any explicit set of property values within instance_init() when
> used together with object property's default value (or qdev-prop, which
> builts the default value framework on top) will stop working.
> 
> This patch touches two of such use cases where the device may add qdev
> properties with a default value first, then quickly set another value
> within instance_init().
> 
> These are the only two outliers I found across the QEMU tree that do it
> this way.  Switch them to use qdev-prop's NODEFAULT APIs, and always
> initialize the values manually.
> 
> Example 1:
> 
> bcm283x_base_init() adds qdev prop bcm2836_enabled_cores_property with a
> default value 0, then it immediately set another value inside the same
> instance_init() hook.
> 
> Example 2:
> 
> aarch64_add_pauth_properties() adds qdev prop arm_cpu_pauth_property, but
> then it will immediately update the same field (cpu->prop_pauth)
> conditionally when hwaccel_enabled()==true.
> 
> Without the change, some arm test may fail (boot-serial-test) after
> introducing object property's default value framework, because devices'
> values will be overwritten by the properties' later.
> 
> OTOH, this patch itself should introduce no functional change.
> 
> Cc: Marc Zyngier <[email protected]>
> Cc: Philippe Mathieu-Daudé <[email protected]>
> Signed-off-by: Peter Xu <[email protected]>
> ---
>   hw/arm/bcm2836.c   | 3 ++-
>   target/arm/cpu64.c | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index f4ecea908b..9e4d30c99c 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -19,7 +19,8 @@
>   #include "target/arm/gtimer.h"
>   
>   static const Property bcm2836_enabled_cores_property =
> -    DEFINE_PROP_UINT32("enabled-cpus", BCM283XBaseState, enabled_cpus, 0);
> +    DEFINE_PROP_UINT32_NODEFAULT(
> +        "enabled-cpus", BCM283XBaseState, enabled_cpus);
>   
>   static void bcm283x_base_init(Object *obj)
>   {
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 2816735577..a3a014a994 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -635,7 +635,7 @@ void aarch64_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
>   }
>   
>   static const Property arm_cpu_pauth_property =
> -    DEFINE_PROP_BOOL("pauth", ARMCPU, prop_pauth, true);
> +    DEFINE_PROP_BOOL_NODEFAULT("pauth", ARMCPU, prop_pauth);
>   static const Property arm_cpu_pauth_impdef_property =
>       DEFINE_PROP_BOOL("pauth-impdef", ARMCPU, prop_pauth_impdef, false);
>   static const Property arm_cpu_pauth_qarma3_property =
> @@ -660,6 +660,7 @@ void aarch64_add_pauth_properties(Object *obj)
>            */
>           cpu->prop_pauth = cpu_isar_feature(aa64_pauth, cpu);
>       } else {
> +        cpu->prop_pauth = true;
>           qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
>           qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_qarma3_property);
>           qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_qarma5_property);

I checked against the commit message for 5cc56cc687 ("qdev: support 
properties which don't set a default value") and this is exactly what 
_NODEFAULT() is intended for, so:

Reviewed-by: Mark Cave-Ayland <[email protected]>


ATB,

Mark.