Re: [RFC PATCH 13/40] hw/arm/bcm2836: Set mp-affinity property in realize

[Philippe Mathieu-Daudé]

On 3/1/23 19:16, Richard Henderson wrote:
> There was even a TODO comment that we ought to be using a cpu
> property, but we failed to update when the property was added.
> Use ARM_AFF1_SHIFT instead of the bare constant 8.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   hw/arm/bcm2836.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 24354338ca..abbb3689d0 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -130,8 +130,11 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
>           qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-fiq", 0));
>   
>       for (n = 0; n < BCM283X_NCPUS; n++) {
> -        /* TODO: this should be converted to a property of ARM_CPU */
> -        s->cpu[n].core.mp_affinity = (bc->clusterid << 8) | n;
> +        if (!object_property_set_int(OBJECT(&s->cpu[n].core), "mp-affinity",
> +                                     (bc->clusterid << ARM_AFF1_SHIFT) | n,
> +                                     errp)) {
> +            return;
> +        }
>   
>           /* set periphbase/CBAR value for CPU-local registers */
>           if (!object_property_set_int(OBJECT(&s->cpu[n].core), "reset-cbar",

Eh I have almost the same patch locally:

-- >8 --
$ git show 5f675655c844154f3760967296e82adf5d8d7c24
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 24354338ca..6f964a3b31 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -130,8 +130,8 @@ static void bcm2836_realize(DeviceState *dev, Error 
**errp)
         qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-fiq", 0));

     for (n = 0; n < BCM283X_NCPUS; n++) {
-        /* TODO: this should be converted to a property of ARM_CPU */
-        s->cpu[n].core.mp_affinity = (bc->clusterid << 8) | n;
+        object_property_set_int(OBJECT(&s->cpu[n].core), "mp-affinity",
+                                (bc->clusterid << 8) | n, &error_abort);

         /* set periphbase/CBAR value for CPU-local registers */
         if (!object_property_set_int(OBJECT(&s->cpu[n].core), 
"reset-cbar",
---

Yours is better (ARM_AFF1_SHIFT & checks return value).

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>