Re: [PATCH 5/5] hw/ppc/pnv: Set QDev properties using QDev API

[Daniel Henrique Barboza]

On 2/3/23 18:16, Philippe Mathieu-Daudé wrote:
> No need to use the low-level QOM API when an object
> inherits from QDev. Directly use the QDev API to set
> its properties.
>
> One call in pnv_psi_power8_realize() propagate the
> Error* argument:
>
>    if (!object_property_set_int(OBJECT(ics), "nr-irqs",
>                                 PSI_NUM_INTERRUPTS, errp)) {
>        return;
>    }
>
> TYPE_ICS "nr-irqs" is declared in ics_properties[],
> itself always registered in ics_class_init(); so converting
> this errp to &error_abort is safe.
>
> All other calls use either errp=&error_abort or &error_fatal,
> so converting to the QDev API is almost a no-op (QDev API
> always uses &error_abort).
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   hw/intc/pnv_xive.c         | 11 ++++------
>   hw/intc/pnv_xive2.c        | 15 +++++---------
>   hw/pci-host/pnv_phb3.c     |  9 +++------
>   hw/pci-host/pnv_phb4.c     |  4 ++--
>   hw/pci-host/pnv_phb4_pec.c | 10 +++-------
>   hw/ppc/pnv.c               | 41 ++++++++++++++++----------------------
>   hw/ppc/pnv_psi.c           | 10 +++-------
>   7 files changed, 37 insertions(+), 63 deletions(-)
>
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 622f9d28b7..ccc1ea5380 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -1857,17 +1857,14 @@ static void pnv_xive_realize(DeviceState *dev, Error 
> **errp)
>        * resized dynamically when the controller is configured by the FW
>        * to limit accesses to resources not provisioned.
>        */
> -    object_property_set_int(OBJECT(xsrc), "nr-irqs", PNV_XIVE_NR_IRQS,
> -                            &error_fatal);
> -    object_property_set_link(OBJECT(xsrc), "xive", OBJECT(xive), &error_abort);
> +    qdev_prop_set_uint32(DEVICE(xsrc), "nr-irqs", PNV_XIVE_NR_IRQS);
> +    qdev_prop_set_link(DEVICE(xsrc), "xive", OBJECT(xive));
>       if (!qdev_realize(DEVICE(xsrc), NULL, errp)) {
>           return;
>       }
>   
> -    object_property_set_int(OBJECT(end_xsrc), "nr-ends", PNV_XIVE_NR_ENDS,
> -                            &error_fatal);
> -    object_property_set_link(OBJECT(end_xsrc), "xive", OBJECT(xive),
> -                             &error_abort);
> +    qdev_prop_set_uint32(DEVICE(end_xsrc), "nr-ends", PNV_XIVE_NR_ENDS);
> +    qdev_prop_set_link(DEVICE(end_xsrc), "xive", OBJECT(xive));
>       if (!qdev_realize(DEVICE(end_xsrc), NULL, errp)) {
>           return;
>       }
> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> index 7176d70234..d7695f65e7 100644
> --- a/hw/intc/pnv_xive2.c
> +++ b/hw/intc/pnv_xive2.c
> @@ -1821,22 +1821,17 @@ static void pnv_xive2_realize(DeviceState *dev, Error 
> **errp)
>        * resized dynamically when the controller is configured by the FW
>        * to limit accesses to resources not provisioned.
>        */
> -    object_property_set_int(OBJECT(xsrc), "flags", XIVE_SRC_STORE_EOI,
> -                            &error_fatal);
> -    object_property_set_int(OBJECT(xsrc), "nr-irqs", PNV_XIVE2_NR_IRQS,
> -                            &error_fatal);
> -    object_property_set_link(OBJECT(xsrc), "xive", OBJECT(xive),
> -                             &error_fatal);
> +    qdev_prop_set_uint64(DEVICE(xsrc), "flags", XIVE_SRC_STORE_EOI);
> +    qdev_prop_set_uint32(DEVICE(xsrc), "nr-irqs", PNV_XIVE2_NR_IRQS);
> +    qdev_prop_set_link(DEVICE(xsrc), "xive", OBJECT(xive));
>       qdev_realize(DEVICE(xsrc), NULL, &local_err);
>       if (local_err) {
>           error_propagate(errp, local_err);
>           return;
>       }
>   
> -    object_property_set_int(OBJECT(end_xsrc), "nr-ends", PNV_XIVE2_NR_ENDS,
> -                            &error_fatal);
> -    object_property_set_link(OBJECT(end_xsrc), "xive", OBJECT(xive),
> -                             &error_abort);
> +    qdev_prop_set_uint32(DEVICE(end_xsrc), "nr-ends", PNV_XIVE2_NR_ENDS);
> +    qdev_prop_set_link(DEVICE(end_xsrc), "xive", OBJECT(xive));
>       qdev_realize(DEVICE(end_xsrc), NULL, &local_err);
>       if (local_err) {
>           error_propagate(errp, local_err);
> diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
> index 7a21497cf8..6da9053ffa 100644
> --- a/hw/pci-host/pnv_phb3.c
> +++ b/hw/pci-host/pnv_phb3.c
> @@ -1029,8 +1029,7 @@ static void pnv_phb3_realize(DeviceState *dev, Error 
> **errp)
>       /* LSI sources */
>       object_property_set_link(OBJECT(&phb->lsis), "xics", OBJECT(pnv),
>                                &error_abort);
> -    object_property_set_int(OBJECT(&phb->lsis), "nr-irqs", PNV_PHB3_NUM_LSI,
> -                            &error_abort);
> +    qdev_prop_set_uint32(DEVICE(&phb->lsis), "nr-irqs", PNV_PHB3_NUM_LSI);
>       if (!qdev_realize(DEVICE(&phb->lsis), NULL, errp)) {
>           return;
>       }
> @@ -1046,15 +1045,13 @@ static void pnv_phb3_realize(DeviceState *dev, Error 
> **errp)
>                                &error_abort);
>       object_property_set_link(OBJECT(&phb->msis), "xics", OBJECT(pnv),
>                                &error_abort);
> -    object_property_set_int(OBJECT(&phb->msis), "nr-irqs", PHB3_MAX_MSI,
> -                            &error_abort);
> +    qdev_prop_set_uint32(DEVICE(&phb->msis), "nr-irqs", PHB3_MAX_MSI);
>       if (!qdev_realize(DEVICE(&phb->msis), NULL, errp)) {
>           return;
>       }
>   
>       /* Power Bus Common Queue */
> -    object_property_set_link(OBJECT(&phb->pbcq), "phb", OBJECT(phb),
> -                             &error_abort);
> +    qdev_prop_set_link(DEVICE(&phb->pbcq), "phb", OBJECT(phb));
>       if (!qdev_realize(DEVICE(&phb->pbcq), NULL, errp)) {
>           return;
>       }
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index ccbde841fc..c4e7cb0efe 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1583,8 +1583,8 @@ static void pnv_phb4_realize(DeviceState *dev, Error 
> **errp)
>       } else {
>           nr_irqs = PNV_PHB4_MAX_INTs >> 1;
>       }
> -    object_property_set_int(OBJECT(xsrc), "nr-irqs", nr_irqs, &error_fatal);
> -    object_property_set_link(OBJECT(xsrc), "xive", OBJECT(phb), &error_fatal);
> +    qdev_prop_set_uint32(DEVICE(xsrc), "nr-irqs", nr_irqs);
> +    qdev_prop_set_link(DEVICE(xsrc), "xive", OBJECT(phb));
>       if (!qdev_realize(DEVICE(xsrc), NULL, errp)) {
>           return;
>       }
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index 43267a428f..9c21382330 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -120,13 +120,9 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState 
> *pec,
>       int phb_id = pnv_phb4_pec_get_phb_id(pec, stack_no);
>   
>       object_property_add_child(OBJECT(pec), "phb[*]", OBJECT(phb));
> -    object_property_set_link(OBJECT(phb), "pec", OBJECT(pec),
> -                             &error_abort);
> -    object_property_set_int(OBJECT(phb), "chip-id", pec->chip_id,
> -                            &error_fatal);
> -    object_property_set_int(OBJECT(phb), "index", phb_id,
> -                            &error_fatal);
> -
> +    qdev_prop_set_link(DEVICE(phb), "pec", OBJECT(pec));
> +    qdev_prop_set_uint32(DEVICE(phb), "chip-id", pec->chip_id);
> +    qdev_prop_set_uint32(DEVICE(phb), "index", phb_id);
>       if (!sysbus_realize(SYS_BUS_DEVICE(phb), errp)) {
>           return;
>       }
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 44b1fbbc93..7c6d5e4320 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -954,35 +954,31 @@ static void pnv_init(MachineState *machine)
>       pnv->chips = g_new0(PnvChip *, pnv->num_chips);
>       for (i = 0; i < pnv->num_chips; i++) {
>           char chip_name[32];
> -        Object *chip = OBJECT(qdev_new(chip_typename));
> +        DeviceState *chip = qdev_new(chip_typename);
>           uint64_t chip_ram_size =  pnv_chip_get_ram_size(pnv, i);
>   
>           pnv->chips[i] = PNV_CHIP(chip);
>   
> +        snprintf(chip_name, sizeof(chip_name), "chip[%d]", i);
> +        object_property_add_child(OBJECT(pnv), chip_name, OBJECT(chip));
> +
>           /* Distribute RAM among the chips  */
> -        object_property_set_int(chip, "ram-start", chip_ram_start,
> -                                &error_fatal);
> -        object_property_set_int(chip, "ram-size", chip_ram_size,
> -                                &error_fatal);
> +        qdev_prop_set_uint64(chip, "ram-start", chip_ram_start);
> +        qdev_prop_set_uint64(chip, "ram-size", chip_ram_size);
>           chip_ram_start += chip_ram_size;
>   
> -        snprintf(chip_name, sizeof(chip_name), "chip[%d]", i);
> -        object_property_add_child(OBJECT(pnv), chip_name, chip);
> -        object_property_set_int(chip, "chip-id", i, &error_fatal);
> -        object_property_set_int(chip, "nr-cores", machine->smp.cores,
> -                                &error_fatal);
> -        object_property_set_int(chip, "nr-threads", machine->smp.threads,
> -                                &error_fatal);
> +        qdev_prop_set_uint32(chip, "chip-id", i);
> +        qdev_prop_set_uint32(chip, "nr-cores", machine->smp.cores);
> +        qdev_prop_set_uint32(chip, "nr-threads", machine->smp.threads);
>           /*
>            * The POWER8 machine use the XICS interrupt interface.
>            * Propagate the XICS fabric to the chip and its controllers.
>            */
>           if (object_dynamic_cast(OBJECT(pnv), TYPE_XICS_FABRIC)) {
> -            object_property_set_link(chip, "xics", OBJECT(pnv), &error_abort);
> +            qdev_prop_set_link(chip, "xics", OBJECT(pnv));
>           }
>           if (object_dynamic_cast(OBJECT(pnv), TYPE_XIVE_FABRIC)) {
> -            object_property_set_link(chip, "xive-fabric", OBJECT(pnv),
> -                                     &error_abort);
> +            qdev_prop_set_link(chip, "xive-fabric", OBJECT(pnv));
>           }
>           sysbus_realize_and_unref(SYS_BUS_DEVICE(chip), &error_fatal);
>       }
> @@ -1492,7 +1488,7 @@ static void pnv_chip_quad_realize_one(PnvChip *chip, 
> PnvQuad *eq,
>                                          sizeof(*eq), TYPE_PNV_QUAD,
>                                          &error_fatal, NULL);
>   
> -    object_property_set_int(OBJECT(eq), "quad-id", core_id, &error_fatal);
> +    qdev_prop_set_uint32(DEVICE(eq), "quad-id", core_id);
>       qdev_realize(DEVICE(eq), NULL, &error_fatal);
>   }
>   
> @@ -1969,16 +1965,13 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
>           snprintf(core_name, sizeof(core_name), "core[%d]", core_hwid);
>           object_property_add_child(OBJECT(chip), core_name, OBJECT(pnv_core));
>           chip->cores[i] = pnv_core;
> -        object_property_set_int(OBJECT(pnv_core), "nr-threads",
> -                                chip->nr_threads, &error_fatal);
> +        qdev_prop_set_uint32(DEVICE(pnv_core), "nr-threads", chip->nr_threads);
>           object_property_set_int(OBJECT(pnv_core), CPU_CORE_PROP_CORE_ID,
>                                   core_hwid, &error_fatal);
> -        object_property_set_int(OBJECT(pnv_core), "pir",
> -                                pcc->core_pir(chip, core_hwid), &error_fatal);
> -        object_property_set_int(OBJECT(pnv_core), "hrmor", pnv->fw_load_addr,
> -                                &error_fatal);
> -        object_property_set_link(OBJECT(pnv_core), "chip", OBJECT(chip),
> -                                 &error_abort);
> +        qdev_prop_set_uint32(DEVICE(pnv_core), "pir",
> +                             pcc->core_pir(chip, core_hwid));
> +        qdev_prop_set_uint64(DEVICE(pnv_core), "hrmor", pnv->fw_load_addr);
> +        qdev_prop_set_link(DEVICE(pnv_core), "chip", OBJECT(chip));
>           qdev_realize(DEVICE(pnv_core), NULL, &error_fatal);
>   
>           /* Each core has an XSCOM MMIO region */
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index 8aa09ab26b..fb90d47138 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -492,10 +492,7 @@ static void pnv_psi_power8_realize(DeviceState *dev, Error 
> **errp)
>       unsigned int i;
>   
>       /* Create PSI interrupt control source */
> -    if (!object_property_set_int(OBJECT(ics), "nr-irqs", PSI_NUM_INTERRUPTS,
> -                                 errp)) {
> -        return;
> -    }
> +    qdev_prop_set_uint32(DEVICE(ics), "nr-irqs", PSI_NUM_INTERRUPTS);
>       if (!qdev_realize(DEVICE(ics), NULL, errp)) {
>           return;
>       }
> @@ -849,9 +846,8 @@ static void pnv_psi_power9_realize(DeviceState *dev, Error 
> **errp)
>       XiveSource *xsrc = &PNV9_PSI(psi)->source;
>       int i;
>   
> -    object_property_set_int(OBJECT(xsrc), "nr-irqs", PSIHB9_NUM_IRQS,
> -                            &error_fatal);
> -    object_property_set_link(OBJECT(xsrc), "xive", OBJECT(psi), &error_abort);
> +    qdev_prop_set_uint32(DEVICE(xsrc), "nr-irqs", PSIHB9_NUM_IRQS);
> +    qdev_prop_set_link(DEVICE(xsrc), "xive", OBJECT(psi));
>       if (!qdev_realize(DEVICE(xsrc), NULL, errp)) {
>           return;
>       }