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.