* [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3)
@ 2023-02-03 21:16 Philippe Mathieu-Daudé
2023-02-03 21:16 ` [PATCH 1/5] hw/misc/macio: Set QDev properties using QDev API Philippe Mathieu-Daudé
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-03 21:16 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater,
Daniel Henrique Barboza, Markus Armbruster, David Gibson,
Greg Kurz, Philippe Mathieu-Daudé
part 1 [*] cover:
--
QEMU provides the QOM API for core objects.
Devices are modelled on top of QOM as QDev objects.
There is no point in using the lower level QOM API with
QDev; it makes the code more complex and harder to review.
I first converted all the calls using errp=&error_abort or
&errp=NULL, then noticed the other uses weren't really
consistent.
A QDev property defined with the DEFINE_PROP_xxx() macros
is always available, thus can't fail. When using hot-plug
devices, we only need to check for optional properties
registered at runtime with the object_property_add_XXX()
API. Some are even always registered in device instance_init.
--
In this series PPC hw is converted. Only one call site in PNV
forwards the Error* argument and its conversion is justified.
Based-on: <20230203180914.49112-1-philmd@linaro.org>
(in particular [PATCH 02/19] hw/qdev: Introduce qdev_prop_set_link():
https://lore.kernel.org/qemu-devel/20230203180914.49112-3-philmd@linaro.org/)
[*] https://lore.kernel.org/qemu-devel/20230203180914.49112-1-philmd@linaro.org/
Philippe Mathieu-Daudé (5):
hw/misc/macio: Set QDev properties using QDev API
hw/pci-host/raven: Set QDev properties using QDev API
hw/ppc/ppc4xx: Set QDev properties using QDev API
hw/ppc/spapr: Set QDev properties using QDev API
hw/ppc/pnv: Set QDev properties using QDev API
hw/intc/pnv_xive.c | 11 ++++------
hw/intc/pnv_xive2.c | 15 +++++---------
hw/intc/spapr_xive.c | 11 ++++------
hw/intc/xics.c | 4 ++--
hw/intc/xive.c | 4 ++--
hw/misc/macio/macio.c | 6 ++----
hw/pci-host/pnv_phb3.c | 9 +++------
hw/pci-host/pnv_phb4.c | 4 ++--
hw/pci-host/pnv_phb4_pec.c | 10 +++-------
hw/pci-host/raven.c | 6 ++----
hw/ppc/e500.c | 3 +--
hw/ppc/pnv.c | 41 ++++++++++++++++----------------------
hw/ppc/pnv_psi.c | 10 +++-------
hw/ppc/ppc405_boards.c | 6 ++----
hw/ppc/ppc405_uc.c | 6 +++---
hw/ppc/ppc440_bamboo.c | 3 +--
hw/ppc/ppc4xx_devs.c | 2 +-
hw/ppc/sam460ex.c | 5 ++---
hw/ppc/spapr_irq.c | 8 +++-----
19 files changed, 62 insertions(+), 102 deletions(-)
--
2.38.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/5] hw/misc/macio: Set QDev properties using QDev API
2023-02-03 21:16 [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Philippe Mathieu-Daudé
@ 2023-02-03 21:16 ` Philippe Mathieu-Daudé
2023-02-03 21:16 ` [PATCH 2/5] hw/pci-host/raven: " Philippe Mathieu-Daudé
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-03 21:16 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater,
Daniel Henrique Barboza, Markus Armbruster, David Gibson,
Greg Kurz, Philippe Mathieu-Daudé
No need to use the low-level QOM API when an object
inherits from QDev. Directly use the QDev API to set
its properties.
All calls use errp=&error_abort, so converting to the
QDev API is a no-op (QDev API always uses &error_abort).
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/misc/macio/macio.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 08dbdd7fc0..66393280f1 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -128,8 +128,7 @@ static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
sysbus_connect_irq(sysbus_dev, 0, irq0);
sysbus_connect_irq(sysbus_dev, 1, irq1);
qdev_prop_set_uint32(DEVICE(ide), "channel", dmaid);
- object_property_set_link(OBJECT(ide), "dbdma", OBJECT(&s->dbdma),
- &error_abort);
+ qdev_prop_set_link(DEVICE(ide), "dbdma", OBJECT(&s->dbdma));
macio_ide_register_dma(ide);
qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
@@ -336,8 +335,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
/* PMU */
object_initialize_child(OBJECT(s), "pmu", &s->pmu, TYPE_VIA_PMU);
- object_property_set_link(OBJECT(&s->pmu), "gpio", OBJECT(sysbus_dev),
- &error_abort);
+ qdev_prop_set_link(DEVICE(&s->pmu), "gpio", OBJECT(sysbus_dev));
qdev_prop_set_bit(DEVICE(&s->pmu), "has-adb", ns->has_adb);
if (!qdev_realize(DEVICE(&s->pmu), BUS(&s->macio_bus), errp)) {
return;
--
2.38.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/5] hw/pci-host/raven: Set QDev properties using QDev API
2023-02-03 21:16 [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Philippe Mathieu-Daudé
2023-02-03 21:16 ` [PATCH 1/5] hw/misc/macio: Set QDev properties using QDev API Philippe Mathieu-Daudé
@ 2023-02-03 21:16 ` Philippe Mathieu-Daudé
2023-02-05 11:03 ` Daniel Henrique Barboza
2023-02-03 21:16 ` [PATCH 3/5] hw/ppc/ppc4xx: " Philippe Mathieu-Daudé
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-03 21:16 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater,
Daniel Henrique Barboza, Markus Armbruster, David Gibson,
Greg Kurz, Philippe Mathieu-Daudé
No need to use the low-level QOM API when an object
inherits from QDev. Directly use the QDev API to set
its properties.
All calls use either errp=&error_fatal or NULL, 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>
---
hw/pci-host/raven.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index cdfb62ac2e..2c842d2146 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -246,8 +246,7 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
/* According to PReP specification section 6.1.6 "System Interrupt
* Assignments", all PCI interrupts are routed via IRQ 15 */
s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
- object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
- &error_fatal);
+ qdev_prop_set_uint16(DEVICE(s->or_irq), "num-lines", PCI_NUM_PINS);
qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
sysbus_init_irq(dev, &s->or_irq->out_irq);
@@ -319,8 +318,7 @@ static void raven_pcihost_initfn(Object *obj)
object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE);
pci_dev = DEVICE(&s->pci_dev);
- object_property_set_int(OBJECT(&s->pci_dev), "addr", PCI_DEVFN(0, 0),
- NULL);
+ qdev_prop_set_int32(pci_dev, "addr", PCI_DEVFN(0, 0));
qdev_prop_set_bit(pci_dev, "multifunction", false);
}
--
2.38.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/5] hw/ppc/ppc4xx: Set QDev properties using QDev API
2023-02-03 21:16 [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Philippe Mathieu-Daudé
2023-02-03 21:16 ` [PATCH 1/5] hw/misc/macio: Set QDev properties using QDev API Philippe Mathieu-Daudé
2023-02-03 21:16 ` [PATCH 2/5] hw/pci-host/raven: " Philippe Mathieu-Daudé
@ 2023-02-03 21:16 ` Philippe Mathieu-Daudé
2023-02-03 21:26 ` BALATON Zoltan
2023-02-05 10:59 ` Daniel Henrique Barboza
2023-02-03 21:16 ` [PATCH 4/5] hw/ppc/spapr: " Philippe Mathieu-Daudé
` (3 subsequent siblings)
6 siblings, 2 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-03 21:16 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater,
Daniel Henrique Barboza, Markus Armbruster, David Gibson,
Greg Kurz, Philippe Mathieu-Daudé
No need to use the low-level QOM API when an object
inherits from QDev. Directly use the QDev API to set
its properties.
All 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>
---
hw/ppc/e500.c | 3 +--
hw/ppc/ppc405_boards.c | 6 ++----
hw/ppc/ppc405_uc.c | 6 +++---
hw/ppc/ppc440_bamboo.c | 3 +--
hw/ppc/ppc4xx_devs.c | 2 +-
hw/ppc/sam460ex.c | 5 ++---
6 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 9fa1f8e6cf..083961cef5 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -943,8 +943,7 @@ void ppce500_init(MachineState *machine)
* Secondary CPU starts in halted state for now. Needs to change
* when implementing non-kernel boot.
*/
- object_property_set_bool(OBJECT(cs), "start-powered-off", i != 0,
- &error_fatal);
+ qdev_prop_set_bit(DEVICE(cs), "start-powered-off", i != 0);
qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal);
if (!firstenv) {
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 4092ebc1ab..67eb9ac139 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -276,10 +276,8 @@ static void ppc405_init(MachineState *machine)
object_initialize_child(OBJECT(machine), "soc", &ppc405->soc,
TYPE_PPC405_SOC);
- object_property_set_link(OBJECT(&ppc405->soc), "dram",
- OBJECT(machine->ram), &error_abort);
- object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333,
- &error_abort);
+ qdev_prop_set_link(DEVICE(&ppc405->soc), "dram", OBJECT(machine->ram));
+ qdev_prop_set_uint32(DEVICE(&ppc405->soc), "sys-clk", 33333333);
qdev_realize(DEVICE(&ppc405->soc), NULL, &error_fatal);
/* allocate and load BIOS */
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index c973cfb04e..b7d5cfc548 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1080,7 +1080,7 @@ static void ppc405_soc_realize(DeviceState *dev, Error **errp)
* We use the 440 DDR SDRAM controller which has more regs and features
* but it's compatible enough for now
*/
- object_property_set_int(OBJECT(&s->sdram), "nbanks", 2, &error_abort);
+ qdev_prop_set_uint32(DEVICE(&s->sdram), "nbanks", 2);
if (!ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(&s->sdram), &s->cpu, errp)) {
return;
}
@@ -1147,8 +1147,8 @@ static void ppc405_soc_realize(DeviceState *dev, Error **errp)
}
/* MAL */
- object_property_set_int(OBJECT(&s->mal), "txc-num", 4, &error_abort);
- object_property_set_int(OBJECT(&s->mal), "rxc-num", 2, &error_abort);
+ qdev_prop_set_uint8(DEVICE(&s->mal), "txc-num", 4);
+ qdev_prop_set_uint8(DEVICE(&s->mal), "rxc-num", 2);
if (!ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(&s->mal), &s->cpu, errp)) {
return;
}
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 81d71adf34..3612471990 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -200,8 +200,7 @@ static void bamboo_init(MachineState *machine)
/* SDRAM controller */
dev = qdev_new(TYPE_PPC4xx_SDRAM_DDR);
- object_property_set_link(OBJECT(dev), "dram", OBJECT(machine->ram),
- &error_abort);
+ qdev_prop_set_link(dev, "dram", OBJECT(machine->ram));
ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(dev), cpu, &error_fatal);
object_unref(OBJECT(dev));
/* XXX 440EP's ECC interrupts are on UIC1, but we've only created UIC0. */
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index c1d111465d..1848cf5d3c 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -535,7 +535,7 @@ void ppc4xx_dcr_register(Ppc4xxDcrDeviceState *dev, int dcrn, void *opaque,
bool ppc4xx_dcr_realize(Ppc4xxDcrDeviceState *dev, PowerPCCPU *cpu,
Error **errp)
{
- object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
+ qdev_prop_set_link(DEVICE(dev), "cpu", OBJECT(cpu));
return sysbus_realize(SYS_BUS_DEVICE(dev), errp);
}
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index cf065aae0e..cb828b6d4d 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -345,13 +345,12 @@ static void sam460ex_init(MachineState *machine)
exit(1);
}
dev = qdev_new(TYPE_PPC4xx_SDRAM_DDR2);
- object_property_set_link(OBJECT(dev), "dram", OBJECT(machine->ram),
- &error_abort);
+ qdev_prop_set_link(dev, "dram", OBJECT(machine->ram));
/*
* Put all RAM on first bank because board has one slot
* and firmware only checks that
*/
- object_property_set_int(OBJECT(dev), "nbanks", 1, &error_abort);
+ qdev_prop_set_uint32(dev, "nbanks", 1);
ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(dev), cpu, &error_fatal);
object_unref(OBJECT(dev));
/* FIXME: does 460EX have ECC interrupts? */
--
2.38.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/5] hw/ppc/spapr: Set QDev properties using QDev API
2023-02-03 21:16 [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2023-02-03 21:16 ` [PATCH 3/5] hw/ppc/ppc4xx: " Philippe Mathieu-Daudé
@ 2023-02-03 21:16 ` Philippe Mathieu-Daudé
2023-02-05 11:00 ` Daniel Henrique Barboza
2023-02-06 8:06 ` Cédric Le Goater
2023-02-03 21:16 ` [PATCH 5/5] hw/ppc/pnv: " Philippe Mathieu-Daudé
` (2 subsequent siblings)
6 siblings, 2 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-03 21:16 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater,
Daniel Henrique Barboza, Markus Armbruster, David Gibson,
Greg Kurz, Philippe Mathieu-Daudé
No need to use the low-level QOM API when an object
inherits from QDev. Directly use the QDev API to set
its properties.
All 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>
---
hw/intc/spapr_xive.c | 11 ++++-------
hw/intc/xics.c | 4 ++--
hw/intc/xive.c | 4 ++--
hw/ppc/spapr_irq.c | 8 +++-----
4 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index dc641cc604..213c4cac44 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -310,9 +310,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
/*
* Initialize the internal sources, for IPIs and virtual devices.
*/
- object_property_set_int(OBJECT(xsrc), "nr-irqs", xive->nr_irqs,
- &error_fatal);
- object_property_set_link(OBJECT(xsrc), "xive", OBJECT(xive), &error_abort);
+ qdev_prop_set_uint32(DEVICE(xsrc), "nr-irqs", xive->nr_irqs);
+ qdev_prop_set_link(DEVICE(xsrc), "xive", OBJECT(xive));
if (!qdev_realize(DEVICE(xsrc), NULL, errp)) {
return;
}
@@ -321,10 +320,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
/*
* Initialize the END ESB source
*/
- object_property_set_int(OBJECT(end_xsrc), "nr-ends", xive->nr_irqs,
- &error_fatal);
- object_property_set_link(OBJECT(end_xsrc), "xive", OBJECT(xive),
- &error_abort);
+ qdev_prop_set_uint32(DEVICE(end_xsrc), "nr-ends", xive->nr_irqs);
+ qdev_prop_set_link(DEVICE(end_xsrc), "xive", OBJECT(xive));
if (!qdev_realize(DEVICE(end_xsrc), NULL, errp)) {
return;
}
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index c7f8abd71e..2fd1a15153 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -382,8 +382,8 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, Error **errp)
obj = object_new(type);
object_property_add_child(cpu, type, obj);
object_unref(obj);
- object_property_set_link(obj, ICP_PROP_XICS, OBJECT(xi), &error_abort);
- object_property_set_link(obj, ICP_PROP_CPU, cpu, &error_abort);
+ qdev_prop_set_link(DEVICE(obj), ICP_PROP_XICS, OBJECT(xi));
+ qdev_prop_set_link(DEVICE(obj), ICP_PROP_CPU, cpu);
if (!qdev_realize(DEVICE(obj), NULL, errp)) {
object_unparent(obj);
obj = NULL;
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index a986b96843..0e34035bc6 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -799,8 +799,8 @@ Object *xive_tctx_create(Object *cpu, XivePresenter *xptr, Error **errp)
obj = object_new(TYPE_XIVE_TCTX);
object_property_add_child(cpu, TYPE_XIVE_TCTX, obj);
object_unref(obj);
- object_property_set_link(obj, "cpu", cpu, &error_abort);
- object_property_set_link(obj, "presenter", OBJECT(xptr), &error_abort);
+ qdev_prop_set_link(DEVICE(obj), "cpu", cpu);
+ qdev_prop_set_link(DEVICE(obj), "presenter", OBJECT(xptr));
if (!qdev_realize(DEVICE(obj), NULL, errp)) {
object_unparent(obj);
return NULL;
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index a0d1e1298e..283769c074 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -313,9 +313,8 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
obj = object_new(TYPE_ICS_SPAPR);
object_property_add_child(OBJECT(spapr), "ics", obj);
- object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr),
- &error_abort);
- object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, &error_abort);
+ qdev_prop_set_link(DEVICE(obj), ICS_PROP_XICS, OBJECT(spapr));
+ qdev_prop_set_uint32(DEVICE(obj), "nr-irqs", smc->nr_xirqs);
if (!qdev_realize(DEVICE(obj), NULL, errp)) {
return;
}
@@ -335,8 +334,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
* priority
*/
qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3);
- object_property_set_link(OBJECT(dev), "xive-fabric", OBJECT(spapr),
- &error_abort);
+ qdev_prop_set_link(dev, "xive-fabric", OBJECT(spapr));
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
spapr->xive = SPAPR_XIVE(dev);
--
2.38.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/5] hw/ppc/pnv: Set QDev properties using QDev API
2023-02-03 21:16 [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2023-02-03 21:16 ` [PATCH 4/5] hw/ppc/spapr: " Philippe Mathieu-Daudé
@ 2023-02-03 21:16 ` Philippe Mathieu-Daudé
2023-02-05 11:00 ` Daniel Henrique Barboza
2023-02-06 8:06 ` Cédric Le Goater
2023-02-05 11:05 ` [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Daniel Henrique Barboza
2023-02-06 8:00 ` Cédric Le Goater
6 siblings, 2 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-03 21:16 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater,
Daniel Henrique Barboza, Markus Armbruster, David Gibson,
Greg Kurz, Philippe Mathieu-Daudé
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>
---
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;
}
--
2.38.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] hw/ppc/ppc4xx: Set QDev properties using QDev API
2023-02-03 21:16 ` [PATCH 3/5] hw/ppc/ppc4xx: " Philippe Mathieu-Daudé
@ 2023-02-03 21:26 ` BALATON Zoltan
2023-02-05 10:59 ` Daniel Henrique Barboza
1 sibling, 0 replies; 17+ messages in thread
From: BALATON Zoltan @ 2023-02-03 21:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Mark Cave-Ayland, qemu-ppc, Hervé Poussineau,
Cédric Le Goater, Daniel Henrique Barboza,
Markus Armbruster, David Gibson, Greg Kurz
[-- Attachment #1: Type: text/plain, Size: 5388 bytes --]
On Fri, 3 Feb 2023, 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.
>
> All 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: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/e500.c | 3 +--
> hw/ppc/ppc405_boards.c | 6 ++----
> hw/ppc/ppc405_uc.c | 6 +++---
> hw/ppc/ppc440_bamboo.c | 3 +--
> hw/ppc/ppc4xx_devs.c | 2 +-
> hw/ppc/sam460ex.c | 5 ++---
> 6 files changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 9fa1f8e6cf..083961cef5 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -943,8 +943,7 @@ void ppce500_init(MachineState *machine)
> * Secondary CPU starts in halted state for now. Needs to change
> * when implementing non-kernel boot.
> */
> - object_property_set_bool(OBJECT(cs), "start-powered-off", i != 0,
> - &error_fatal);
> + qdev_prop_set_bit(DEVICE(cs), "start-powered-off", i != 0);
> qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal);
>
> if (!firstenv) {
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index 4092ebc1ab..67eb9ac139 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -276,10 +276,8 @@ static void ppc405_init(MachineState *machine)
>
> object_initialize_child(OBJECT(machine), "soc", &ppc405->soc,
> TYPE_PPC405_SOC);
> - object_property_set_link(OBJECT(&ppc405->soc), "dram",
> - OBJECT(machine->ram), &error_abort);
> - object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333,
> - &error_abort);
> + qdev_prop_set_link(DEVICE(&ppc405->soc), "dram", OBJECT(machine->ram));
> + qdev_prop_set_uint32(DEVICE(&ppc405->soc), "sys-clk", 33333333);
> qdev_realize(DEVICE(&ppc405->soc), NULL, &error_fatal);
>
> /* allocate and load BIOS */
> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
> index c973cfb04e..b7d5cfc548 100644
> --- a/hw/ppc/ppc405_uc.c
> +++ b/hw/ppc/ppc405_uc.c
> @@ -1080,7 +1080,7 @@ static void ppc405_soc_realize(DeviceState *dev, Error **errp)
> * We use the 440 DDR SDRAM controller which has more regs and features
> * but it's compatible enough for now
> */
> - object_property_set_int(OBJECT(&s->sdram), "nbanks", 2, &error_abort);
> + qdev_prop_set_uint32(DEVICE(&s->sdram), "nbanks", 2);
> if (!ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(&s->sdram), &s->cpu, errp)) {
> return;
> }
> @@ -1147,8 +1147,8 @@ static void ppc405_soc_realize(DeviceState *dev, Error **errp)
> }
>
> /* MAL */
> - object_property_set_int(OBJECT(&s->mal), "txc-num", 4, &error_abort);
> - object_property_set_int(OBJECT(&s->mal), "rxc-num", 2, &error_abort);
> + qdev_prop_set_uint8(DEVICE(&s->mal), "txc-num", 4);
> + qdev_prop_set_uint8(DEVICE(&s->mal), "rxc-num", 2);
> if (!ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(&s->mal), &s->cpu, errp)) {
> return;
> }
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index 81d71adf34..3612471990 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -200,8 +200,7 @@ static void bamboo_init(MachineState *machine)
>
> /* SDRAM controller */
> dev = qdev_new(TYPE_PPC4xx_SDRAM_DDR);
> - object_property_set_link(OBJECT(dev), "dram", OBJECT(machine->ram),
> - &error_abort);
> + qdev_prop_set_link(dev, "dram", OBJECT(machine->ram));
> ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(dev), cpu, &error_fatal);
> object_unref(OBJECT(dev));
> /* XXX 440EP's ECC interrupts are on UIC1, but we've only created UIC0. */
> diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
> index c1d111465d..1848cf5d3c 100644
> --- a/hw/ppc/ppc4xx_devs.c
> +++ b/hw/ppc/ppc4xx_devs.c
> @@ -535,7 +535,7 @@ void ppc4xx_dcr_register(Ppc4xxDcrDeviceState *dev, int dcrn, void *opaque,
> bool ppc4xx_dcr_realize(Ppc4xxDcrDeviceState *dev, PowerPCCPU *cpu,
> Error **errp)
> {
> - object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
> + qdev_prop_set_link(DEVICE(dev), "cpu", OBJECT(cpu));
> return sysbus_realize(SYS_BUS_DEVICE(dev), errp);
> }
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index cf065aae0e..cb828b6d4d 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -345,13 +345,12 @@ static void sam460ex_init(MachineState *machine)
> exit(1);
> }
> dev = qdev_new(TYPE_PPC4xx_SDRAM_DDR2);
> - object_property_set_link(OBJECT(dev), "dram", OBJECT(machine->ram),
> - &error_abort);
> + qdev_prop_set_link(dev, "dram", OBJECT(machine->ram));
> /*
> * Put all RAM on first bank because board has one slot
> * and firmware only checks that
> */
> - object_property_set_int(OBJECT(dev), "nbanks", 1, &error_abort);
> + qdev_prop_set_uint32(dev, "nbanks", 1);
> ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(dev), cpu, &error_fatal);
> object_unref(OBJECT(dev));
> /* FIXME: does 460EX have ECC interrupts? */
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] hw/ppc/ppc4xx: Set QDev properties using QDev API
2023-02-03 21:16 ` [PATCH 3/5] hw/ppc/ppc4xx: " Philippe Mathieu-Daudé
2023-02-03 21:26 ` BALATON Zoltan
@ 2023-02-05 10:59 ` Daniel Henrique Barboza
1 sibling, 0 replies; 17+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-05 10:59 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater, Markus Armbruster,
David Gibson, Greg Kurz
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.
>
> All 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/ppc/e500.c | 3 +--
> hw/ppc/ppc405_boards.c | 6 ++----
> hw/ppc/ppc405_uc.c | 6 +++---
> hw/ppc/ppc440_bamboo.c | 3 +--
> hw/ppc/ppc4xx_devs.c | 2 +-
> hw/ppc/sam460ex.c | 5 ++---
> 6 files changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 9fa1f8e6cf..083961cef5 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -943,8 +943,7 @@ void ppce500_init(MachineState *machine)
> * Secondary CPU starts in halted state for now. Needs to change
> * when implementing non-kernel boot.
> */
> - object_property_set_bool(OBJECT(cs), "start-powered-off", i != 0,
> - &error_fatal);
> + qdev_prop_set_bit(DEVICE(cs), "start-powered-off", i != 0);
> qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal);
>
> if (!firstenv) {
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index 4092ebc1ab..67eb9ac139 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -276,10 +276,8 @@ static void ppc405_init(MachineState *machine)
>
> object_initialize_child(OBJECT(machine), "soc", &ppc405->soc,
> TYPE_PPC405_SOC);
> - object_property_set_link(OBJECT(&ppc405->soc), "dram",
> - OBJECT(machine->ram), &error_abort);
> - object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333,
> - &error_abort);
> + qdev_prop_set_link(DEVICE(&ppc405->soc), "dram", OBJECT(machine->ram));
> + qdev_prop_set_uint32(DEVICE(&ppc405->soc), "sys-clk", 33333333);
> qdev_realize(DEVICE(&ppc405->soc), NULL, &error_fatal);
>
> /* allocate and load BIOS */
> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
> index c973cfb04e..b7d5cfc548 100644
> --- a/hw/ppc/ppc405_uc.c
> +++ b/hw/ppc/ppc405_uc.c
> @@ -1080,7 +1080,7 @@ static void ppc405_soc_realize(DeviceState *dev, Error **errp)
> * We use the 440 DDR SDRAM controller which has more regs and features
> * but it's compatible enough for now
> */
> - object_property_set_int(OBJECT(&s->sdram), "nbanks", 2, &error_abort);
> + qdev_prop_set_uint32(DEVICE(&s->sdram), "nbanks", 2);
> if (!ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(&s->sdram), &s->cpu, errp)) {
> return;
> }
> @@ -1147,8 +1147,8 @@ static void ppc405_soc_realize(DeviceState *dev, Error **errp)
> }
>
> /* MAL */
> - object_property_set_int(OBJECT(&s->mal), "txc-num", 4, &error_abort);
> - object_property_set_int(OBJECT(&s->mal), "rxc-num", 2, &error_abort);
> + qdev_prop_set_uint8(DEVICE(&s->mal), "txc-num", 4);
> + qdev_prop_set_uint8(DEVICE(&s->mal), "rxc-num", 2);
> if (!ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(&s->mal), &s->cpu, errp)) {
> return;
> }
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index 81d71adf34..3612471990 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -200,8 +200,7 @@ static void bamboo_init(MachineState *machine)
>
> /* SDRAM controller */
> dev = qdev_new(TYPE_PPC4xx_SDRAM_DDR);
> - object_property_set_link(OBJECT(dev), "dram", OBJECT(machine->ram),
> - &error_abort);
> + qdev_prop_set_link(dev, "dram", OBJECT(machine->ram));
> ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(dev), cpu, &error_fatal);
> object_unref(OBJECT(dev));
> /* XXX 440EP's ECC interrupts are on UIC1, but we've only created UIC0. */
> diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
> index c1d111465d..1848cf5d3c 100644
> --- a/hw/ppc/ppc4xx_devs.c
> +++ b/hw/ppc/ppc4xx_devs.c
> @@ -535,7 +535,7 @@ void ppc4xx_dcr_register(Ppc4xxDcrDeviceState *dev, int dcrn, void *opaque,
> bool ppc4xx_dcr_realize(Ppc4xxDcrDeviceState *dev, PowerPCCPU *cpu,
> Error **errp)
> {
> - object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
> + qdev_prop_set_link(DEVICE(dev), "cpu", OBJECT(cpu));
> return sysbus_realize(SYS_BUS_DEVICE(dev), errp);
> }
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index cf065aae0e..cb828b6d4d 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -345,13 +345,12 @@ static void sam460ex_init(MachineState *machine)
> exit(1);
> }
> dev = qdev_new(TYPE_PPC4xx_SDRAM_DDR2);
> - object_property_set_link(OBJECT(dev), "dram", OBJECT(machine->ram),
> - &error_abort);
> + qdev_prop_set_link(dev, "dram", OBJECT(machine->ram));
> /*
> * Put all RAM on first bank because board has one slot
> * and firmware only checks that
> */
> - object_property_set_int(OBJECT(dev), "nbanks", 1, &error_abort);
> + qdev_prop_set_uint32(dev, "nbanks", 1);
> ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(dev), cpu, &error_fatal);
> object_unref(OBJECT(dev));
> /* FIXME: does 460EX have ECC interrupts? */
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] hw/ppc/spapr: Set QDev properties using QDev API
2023-02-03 21:16 ` [PATCH 4/5] hw/ppc/spapr: " Philippe Mathieu-Daudé
@ 2023-02-05 11:00 ` Daniel Henrique Barboza
2023-02-06 8:06 ` Cédric Le Goater
1 sibling, 0 replies; 17+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-05 11:00 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater, Markus Armbruster,
David Gibson, Greg Kurz
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.
>
> All 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/spapr_xive.c | 11 ++++-------
> hw/intc/xics.c | 4 ++--
> hw/intc/xive.c | 4 ++--
> hw/ppc/spapr_irq.c | 8 +++-----
> 4 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index dc641cc604..213c4cac44 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -310,9 +310,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
> /*
> * Initialize the internal sources, for IPIs and virtual devices.
> */
> - object_property_set_int(OBJECT(xsrc), "nr-irqs", xive->nr_irqs,
> - &error_fatal);
> - object_property_set_link(OBJECT(xsrc), "xive", OBJECT(xive), &error_abort);
> + qdev_prop_set_uint32(DEVICE(xsrc), "nr-irqs", xive->nr_irqs);
> + qdev_prop_set_link(DEVICE(xsrc), "xive", OBJECT(xive));
> if (!qdev_realize(DEVICE(xsrc), NULL, errp)) {
> return;
> }
> @@ -321,10 +320,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
> /*
> * Initialize the END ESB source
> */
> - object_property_set_int(OBJECT(end_xsrc), "nr-ends", xive->nr_irqs,
> - &error_fatal);
> - object_property_set_link(OBJECT(end_xsrc), "xive", OBJECT(xive),
> - &error_abort);
> + qdev_prop_set_uint32(DEVICE(end_xsrc), "nr-ends", xive->nr_irqs);
> + qdev_prop_set_link(DEVICE(end_xsrc), "xive", OBJECT(xive));
> if (!qdev_realize(DEVICE(end_xsrc), NULL, errp)) {
> return;
> }
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index c7f8abd71e..2fd1a15153 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -382,8 +382,8 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, Error **errp)
> obj = object_new(type);
> object_property_add_child(cpu, type, obj);
> object_unref(obj);
> - object_property_set_link(obj, ICP_PROP_XICS, OBJECT(xi), &error_abort);
> - object_property_set_link(obj, ICP_PROP_CPU, cpu, &error_abort);
> + qdev_prop_set_link(DEVICE(obj), ICP_PROP_XICS, OBJECT(xi));
> + qdev_prop_set_link(DEVICE(obj), ICP_PROP_CPU, cpu);
> if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> object_unparent(obj);
> obj = NULL;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index a986b96843..0e34035bc6 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -799,8 +799,8 @@ Object *xive_tctx_create(Object *cpu, XivePresenter *xptr, Error **errp)
> obj = object_new(TYPE_XIVE_TCTX);
> object_property_add_child(cpu, TYPE_XIVE_TCTX, obj);
> object_unref(obj);
> - object_property_set_link(obj, "cpu", cpu, &error_abort);
> - object_property_set_link(obj, "presenter", OBJECT(xptr), &error_abort);
> + qdev_prop_set_link(DEVICE(obj), "cpu", cpu);
> + qdev_prop_set_link(DEVICE(obj), "presenter", OBJECT(xptr));
> if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> object_unparent(obj);
> return NULL;
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index a0d1e1298e..283769c074 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -313,9 +313,8 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
> obj = object_new(TYPE_ICS_SPAPR);
>
> object_property_add_child(OBJECT(spapr), "ics", obj);
> - object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr),
> - &error_abort);
> - object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, &error_abort);
> + qdev_prop_set_link(DEVICE(obj), ICS_PROP_XICS, OBJECT(spapr));
> + qdev_prop_set_uint32(DEVICE(obj), "nr-irqs", smc->nr_xirqs);
> if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> return;
> }
> @@ -335,8 +334,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
> * priority
> */
> qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3);
> - object_property_set_link(OBJECT(dev), "xive-fabric", OBJECT(spapr),
> - &error_abort);
> + qdev_prop_set_link(dev, "xive-fabric", OBJECT(spapr));
> sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>
> spapr->xive = SPAPR_XIVE(dev);
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] hw/ppc/pnv: Set QDev properties using QDev API
2023-02-03 21:16 ` [PATCH 5/5] hw/ppc/pnv: " Philippe Mathieu-Daudé
@ 2023-02-05 11:00 ` Daniel Henrique Barboza
2023-02-06 8:06 ` Cédric Le Goater
1 sibling, 0 replies; 17+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-05 11:00 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater, Markus Armbruster,
David Gibson, Greg Kurz
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;
> }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/5] hw/pci-host/raven: Set QDev properties using QDev API
2023-02-03 21:16 ` [PATCH 2/5] hw/pci-host/raven: " Philippe Mathieu-Daudé
@ 2023-02-05 11:03 ` Daniel Henrique Barboza
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-05 11:03 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater, Markus Armbruster,
David Gibson, Greg Kurz
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.
>
> All calls use either errp=&error_fatal or NULL, 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/pci-host/raven.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
> index cdfb62ac2e..2c842d2146 100644
> --- a/hw/pci-host/raven.c
> +++ b/hw/pci-host/raven.c
> @@ -246,8 +246,7 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
> /* According to PReP specification section 6.1.6 "System Interrupt
> * Assignments", all PCI interrupts are routed via IRQ 15 */
> s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> - object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> - &error_fatal);
> + qdev_prop_set_uint16(DEVICE(s->or_irq), "num-lines", PCI_NUM_PINS);
> qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> sysbus_init_irq(dev, &s->or_irq->out_irq);
>
> @@ -319,8 +318,7 @@ static void raven_pcihost_initfn(Object *obj)
>
> object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE);
> pci_dev = DEVICE(&s->pci_dev);
> - object_property_set_int(OBJECT(&s->pci_dev), "addr", PCI_DEVFN(0, 0),
> - NULL);
> + qdev_prop_set_int32(pci_dev, "addr", PCI_DEVFN(0, 0));
> qdev_prop_set_bit(pci_dev, "multifunction", false);
> }
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3)
2023-02-03 21:16 [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2023-02-03 21:16 ` [PATCH 5/5] hw/ppc/pnv: " Philippe Mathieu-Daudé
@ 2023-02-05 11:05 ` Daniel Henrique Barboza
2023-02-06 8:00 ` Cédric Le Goater
6 siblings, 0 replies; 17+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-05 11:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Cédric Le Goater, Markus Armbruster,
David Gibson, Greg Kurz
On 2/3/23 18:16, Philippe Mathieu-Daudé wrote:
> part 1 [*] cover:
> --
> QEMU provides the QOM API for core objects.
> Devices are modelled on top of QOM as QDev objects.
>
> There is no point in using the lower level QOM API with
> QDev; it makes the code more complex and harder to review.
>
> I first converted all the calls using errp=&error_abort or
> &errp=NULL, then noticed the other uses weren't really
> consistent.
>
> A QDev property defined with the DEFINE_PROP_xxx() macros
> is always available, thus can't fail. When using hot-plug
> devices, we only need to check for optional properties
> registered at runtime with the object_property_add_XXX()
> API. Some are even always registered in device instance_init.
> --
>
> In this series PPC hw is converted. Only one call site in PNV
> forwards the Error* argument and its conversion is justified.
Feel free to take the 4 patches I acked via your tree when pushing it together
with part 1/3.
I can't ack macio because that's Mark's turf.
Thanks,
Daniel
>
> Based-on: <20230203180914.49112-1-philmd@linaro.org>
> (in particular [PATCH 02/19] hw/qdev: Introduce qdev_prop_set_link():
> https://lore.kernel.org/qemu-devel/20230203180914.49112-3-philmd@linaro.org/)
>
> [*] https://lore.kernel.org/qemu-devel/20230203180914.49112-1-philmd@linaro.org/
>
> Philippe Mathieu-Daudé (5):
> hw/misc/macio: Set QDev properties using QDev API
> hw/pci-host/raven: Set QDev properties using QDev API
> hw/ppc/ppc4xx: Set QDev properties using QDev API
> hw/ppc/spapr: Set QDev properties using QDev API
> hw/ppc/pnv: Set QDev properties using QDev API
>
> hw/intc/pnv_xive.c | 11 ++++------
> hw/intc/pnv_xive2.c | 15 +++++---------
> hw/intc/spapr_xive.c | 11 ++++------
> hw/intc/xics.c | 4 ++--
> hw/intc/xive.c | 4 ++--
> hw/misc/macio/macio.c | 6 ++----
> hw/pci-host/pnv_phb3.c | 9 +++------
> hw/pci-host/pnv_phb4.c | 4 ++--
> hw/pci-host/pnv_phb4_pec.c | 10 +++-------
> hw/pci-host/raven.c | 6 ++----
> hw/ppc/e500.c | 3 +--
> hw/ppc/pnv.c | 41 ++++++++++++++++----------------------
> hw/ppc/pnv_psi.c | 10 +++-------
> hw/ppc/ppc405_boards.c | 6 ++----
> hw/ppc/ppc405_uc.c | 6 +++---
> hw/ppc/ppc440_bamboo.c | 3 +--
> hw/ppc/ppc4xx_devs.c | 2 +-
> hw/ppc/sam460ex.c | 5 ++---
> hw/ppc/spapr_irq.c | 8 +++-----
> 19 files changed, 62 insertions(+), 102 deletions(-)
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3)
2023-02-03 21:16 [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2023-02-05 11:05 ` [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Daniel Henrique Barboza
@ 2023-02-06 8:00 ` Cédric Le Goater
2023-02-06 9:09 ` Mark Cave-Ayland
2023-02-06 13:52 ` Daniel Henrique Barboza
6 siblings, 2 replies; 17+ messages in thread
From: Cédric Le Goater @ 2023-02-06 8:00 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Daniel Henrique Barboza,
Markus Armbruster, David Gibson, Greg Kurz
On 2/3/23 22:16, Philippe Mathieu-Daudé wrote:
> part 1 [*] cover:
> --
> QEMU provides the QOM API for core objects.
> Devices are modelled on top of QOM as QDev objects.
>
> There is no point in using the lower level QOM API with
> QDev; it makes the code more complex and harder to review.
>
> I first converted all the calls using errp=&error_abort or
> &errp=NULL, then noticed the other uses weren't really
> consistent.
6/8 years ago, we converted models to QOM, supposedly because the qdev
interface was legacy and QOM was the new way. That's not true anymore ?
That said, I am ok with changes, even for the best practices. I would
like to know how to keep track. Do we have a model skeleton/reference ?
Thanks,
C.
> A QDev property defined with the DEFINE_PROP_xxx() macros
> is always available, thus can't fail. When using hot-plug
> devices, we only need to check for optional properties
> registered at runtime with the object_property_add_XXX()
> API. Some are even always registered in device instance_init.
> --
>
> In this series PPC hw is converted. Only one call site in PNV
> forwards the Error* argument and its conversion is justified.
>
> Based-on: <20230203180914.49112-1-philmd@linaro.org>
> (in particular [PATCH 02/19] hw/qdev: Introduce qdev_prop_set_link():
> https://lore.kernel.org/qemu-devel/20230203180914.49112-3-philmd@linaro.org/)
>
> [*] https://lore.kernel.org/qemu-devel/20230203180914.49112-1-philmd@linaro.org/
>
> Philippe Mathieu-Daudé (5):
> hw/misc/macio: Set QDev properties using QDev API
> hw/pci-host/raven: Set QDev properties using QDev API
> hw/ppc/ppc4xx: Set QDev properties using QDev API
> hw/ppc/spapr: Set QDev properties using QDev API
> hw/ppc/pnv: Set QDev properties using QDev API
>
> hw/intc/pnv_xive.c | 11 ++++------
> hw/intc/pnv_xive2.c | 15 +++++---------
> hw/intc/spapr_xive.c | 11 ++++------
> hw/intc/xics.c | 4 ++--
> hw/intc/xive.c | 4 ++--
> hw/misc/macio/macio.c | 6 ++----
> hw/pci-host/pnv_phb3.c | 9 +++------
> hw/pci-host/pnv_phb4.c | 4 ++--
> hw/pci-host/pnv_phb4_pec.c | 10 +++-------
> hw/pci-host/raven.c | 6 ++----
> hw/ppc/e500.c | 3 +--
> hw/ppc/pnv.c | 41 ++++++++++++++++----------------------
> hw/ppc/pnv_psi.c | 10 +++-------
> hw/ppc/ppc405_boards.c | 6 ++----
> hw/ppc/ppc405_uc.c | 6 +++---
> hw/ppc/ppc440_bamboo.c | 3 +--
> hw/ppc/ppc4xx_devs.c | 2 +-
> hw/ppc/sam460ex.c | 5 ++---
> hw/ppc/spapr_irq.c | 8 +++-----
> 19 files changed, 62 insertions(+), 102 deletions(-)
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] hw/ppc/pnv: Set QDev properties using QDev API
2023-02-03 21:16 ` [PATCH 5/5] hw/ppc/pnv: " Philippe Mathieu-Daudé
2023-02-05 11:00 ` Daniel Henrique Barboza
@ 2023-02-06 8:06 ` Cédric Le Goater
1 sibling, 0 replies; 17+ messages in thread
From: Cédric Le Goater @ 2023-02-06 8:06 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Daniel Henrique Barboza,
Markus Armbruster, David Gibson, Greg Kurz
On 2/3/23 22: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.
yes. That's ok. I think this model was one of the first I converted to QOM.
>
> 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: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> 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;
> }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] hw/ppc/spapr: Set QDev properties using QDev API
2023-02-03 21:16 ` [PATCH 4/5] hw/ppc/spapr: " Philippe Mathieu-Daudé
2023-02-05 11:00 ` Daniel Henrique Barboza
@ 2023-02-06 8:06 ` Cédric Le Goater
1 sibling, 0 replies; 17+ messages in thread
From: Cédric Le Goater @ 2023-02-06 8:06 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Daniel Henrique Barboza,
Markus Armbruster, David Gibson, Greg Kurz
On 2/3/23 22: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.
>
> All 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: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> hw/intc/spapr_xive.c | 11 ++++-------
> hw/intc/xics.c | 4 ++--
> hw/intc/xive.c | 4 ++--
> hw/ppc/spapr_irq.c | 8 +++-----
> 4 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index dc641cc604..213c4cac44 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -310,9 +310,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
> /*
> * Initialize the internal sources, for IPIs and virtual devices.
> */
> - object_property_set_int(OBJECT(xsrc), "nr-irqs", xive->nr_irqs,
> - &error_fatal);
> - object_property_set_link(OBJECT(xsrc), "xive", OBJECT(xive), &error_abort);
> + qdev_prop_set_uint32(DEVICE(xsrc), "nr-irqs", xive->nr_irqs);
> + qdev_prop_set_link(DEVICE(xsrc), "xive", OBJECT(xive));
> if (!qdev_realize(DEVICE(xsrc), NULL, errp)) {
> return;
> }
> @@ -321,10 +320,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
> /*
> * Initialize the END ESB source
> */
> - object_property_set_int(OBJECT(end_xsrc), "nr-ends", xive->nr_irqs,
> - &error_fatal);
> - object_property_set_link(OBJECT(end_xsrc), "xive", OBJECT(xive),
> - &error_abort);
> + qdev_prop_set_uint32(DEVICE(end_xsrc), "nr-ends", xive->nr_irqs);
> + qdev_prop_set_link(DEVICE(end_xsrc), "xive", OBJECT(xive));
> if (!qdev_realize(DEVICE(end_xsrc), NULL, errp)) {
> return;
> }
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index c7f8abd71e..2fd1a15153 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -382,8 +382,8 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, Error **errp)
> obj = object_new(type);
> object_property_add_child(cpu, type, obj);
> object_unref(obj);
> - object_property_set_link(obj, ICP_PROP_XICS, OBJECT(xi), &error_abort);
> - object_property_set_link(obj, ICP_PROP_CPU, cpu, &error_abort);
> + qdev_prop_set_link(DEVICE(obj), ICP_PROP_XICS, OBJECT(xi));
> + qdev_prop_set_link(DEVICE(obj), ICP_PROP_CPU, cpu);
> if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> object_unparent(obj);
> obj = NULL;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index a986b96843..0e34035bc6 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -799,8 +799,8 @@ Object *xive_tctx_create(Object *cpu, XivePresenter *xptr, Error **errp)
> obj = object_new(TYPE_XIVE_TCTX);
> object_property_add_child(cpu, TYPE_XIVE_TCTX, obj);
> object_unref(obj);
> - object_property_set_link(obj, "cpu", cpu, &error_abort);
> - object_property_set_link(obj, "presenter", OBJECT(xptr), &error_abort);
> + qdev_prop_set_link(DEVICE(obj), "cpu", cpu);
> + qdev_prop_set_link(DEVICE(obj), "presenter", OBJECT(xptr));
> if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> object_unparent(obj);
> return NULL;
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index a0d1e1298e..283769c074 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -313,9 +313,8 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
> obj = object_new(TYPE_ICS_SPAPR);
>
> object_property_add_child(OBJECT(spapr), "ics", obj);
> - object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr),
> - &error_abort);
> - object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, &error_abort);
> + qdev_prop_set_link(DEVICE(obj), ICS_PROP_XICS, OBJECT(spapr));
> + qdev_prop_set_uint32(DEVICE(obj), "nr-irqs", smc->nr_xirqs);
> if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> return;
> }
> @@ -335,8 +334,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
> * priority
> */
> qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3);
> - object_property_set_link(OBJECT(dev), "xive-fabric", OBJECT(spapr),
> - &error_abort);
> + qdev_prop_set_link(dev, "xive-fabric", OBJECT(spapr));
> sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>
> spapr->xive = SPAPR_XIVE(dev);
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3)
2023-02-06 8:00 ` Cédric Le Goater
@ 2023-02-06 9:09 ` Mark Cave-Ayland
2023-02-06 13:52 ` Daniel Henrique Barboza
1 sibling, 0 replies; 17+ messages in thread
From: Mark Cave-Ayland @ 2023-02-06 9:09 UTC (permalink / raw)
To: Cédric Le Goater, Philippe Mathieu-Daudé, qemu-devel
Cc: BALATON Zoltan, qemu-ppc, Hervé Poussineau,
Daniel Henrique Barboza, Markus Armbruster, David Gibson,
Greg Kurz, Peter Maydell
On 06/02/2023 08:00, Cédric Le Goater wrote:
> On 2/3/23 22:16, Philippe Mathieu-Daudé wrote:
>> part 1 [*] cover:
>> --
>> QEMU provides the QOM API for core objects.
>> Devices are modelled on top of QOM as QDev objects.
>>
>> There is no point in using the lower level QOM API with
>> QDev; it makes the code more complex and harder to review.
>>
>> I first converted all the calls using errp=&error_abort or
>> &errp=NULL, then noticed the other uses weren't really
>> consistent.
>
> 6/8 years ago, we converted models to QOM, supposedly because the qdev
> interface was legacy and QOM was the new way. That's not true anymore ?
That is a good question, and something that we really should decide first before
going ahead with these changes. My understanding is that architectures with newer
machines (particularly ARM and PPC) use QOM APIs directly, however more recently
Markus did some improvements to qdev which largely eliminated the gap between the
two. Hence why these days the two are mostly interchangeable: the main difference is
that qdev has a notion of a parent which can be useful during device modelling.
> That said, I am ok with changes, even for the best practices. I would
> like to know how to keep track. Do we have a model skeleton/reference ?
Agreed. I've added Peter on CC as I know he has had some thoughts on QOM vs. qdev,
but certainly as a reviewer it would be great to know which way we should be heading
in the future.
ATB,
Mark.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3)
2023-02-06 8:00 ` Cédric Le Goater
2023-02-06 9:09 ` Mark Cave-Ayland
@ 2023-02-06 13:52 ` Daniel Henrique Barboza
1 sibling, 0 replies; 17+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-06 13:52 UTC (permalink / raw)
To: Cédric Le Goater, Philippe Mathieu-Daudé, qemu-devel
Cc: BALATON Zoltan, Mark Cave-Ayland, qemu-ppc,
Hervé Poussineau, Markus Armbruster, David Gibson,
Greg Kurz
On 2/6/23 05:00, Cédric Le Goater wrote:
> On 2/3/23 22:16, Philippe Mathieu-Daudé wrote:
>> part 1 [*] cover:
>> --
>> QEMU provides the QOM API for core objects.
>> Devices are modelled on top of QOM as QDev objects.
>>
>> There is no point in using the lower level QOM API with
>> QDev; it makes the code more complex and harder to review.
>>
>> I first converted all the calls using errp=&error_abort or
>> &errp=NULL, then noticed the other uses weren't really
>> consistent.
>
> 6/8 years ago, we converted models to QOM, supposedly because the qdev
> interface was legacy and QOM was the new way. That's not true anymore ?
>
> That said, I am ok with changes, even for the best practices. I would
> like to know how to keep track. Do we have a model skeleton/reference ?
I second all that.
Last year we spent a considerable amount of time figuring out how to properly use
QOM in the pnv-phb controller, with a lot of code juggling to avoid using qdev
directly. And it's not like we were doing something that was novel - the core
hw/pci/pci.c files are filled with examples such as:
host_bridge = PCI_HOST_BRIDGE(rootbus->qbus.parent);
And this particular example (not accessing qbus.parent to get the parent bus) led
to *a lot* of QOM code being added to allow the pnv_phb_root_port to access the
parent bus because "you shouldn't access qdev in that way".
After all that, reading "There is no point in using the lower level QOM API
with QDev; it makes the code more complex and harder to review." is funny. I
know that there might be some nuance that I'm not aware of, and in the end
what we did last year and what Phil is doing today are both steps in the
same direction, but ATM this is confusing to me.
As Cedric said, I believe we should had a document laying out in a clear way when
it is OK to use QDEV APIs and when it is OK to use QOM APIs. It would also be nice
to document these (apparently) deprecated uses of these APIs that the core classes
are doing.
Thanks,
Daniel
>
> Thanks,
>
> C.
>
>> A QDev property defined with the DEFINE_PROP_xxx() macros
>> is always available, thus can't fail. When using hot-plug
>> devices, we only need to check for optional properties
>> registered at runtime with the object_property_add_XXX()
>> API. Some are even always registered in device instance_init.
>> --
>>
>> In this series PPC hw is converted. Only one call site in PNV
>> forwards the Error* argument and its conversion is justified.
>>
>> Based-on: <20230203180914.49112-1-philmd@linaro.org>
>> (in particular [PATCH 02/19] hw/qdev: Introduce qdev_prop_set_link():
>> https://lore.kernel.org/qemu-devel/20230203180914.49112-3-philmd@linaro.org/)
>>
>> [*] https://lore.kernel.org/qemu-devel/20230203180914.49112-1-philmd@linaro.org/
>>
>> Philippe Mathieu-Daudé (5):
>> hw/misc/macio: Set QDev properties using QDev API
>> hw/pci-host/raven: Set QDev properties using QDev API
>> hw/ppc/ppc4xx: Set QDev properties using QDev API
>> hw/ppc/spapr: Set QDev properties using QDev API
>> hw/ppc/pnv: Set QDev properties using QDev API
>>
>> hw/intc/pnv_xive.c | 11 ++++------
>> hw/intc/pnv_xive2.c | 15 +++++---------
>> hw/intc/spapr_xive.c | 11 ++++------
>> hw/intc/xics.c | 4 ++--
>> hw/intc/xive.c | 4 ++--
>> hw/misc/macio/macio.c | 6 ++----
>> hw/pci-host/pnv_phb3.c | 9 +++------
>> hw/pci-host/pnv_phb4.c | 4 ++--
>> hw/pci-host/pnv_phb4_pec.c | 10 +++-------
>> hw/pci-host/raven.c | 6 ++----
>> hw/ppc/e500.c | 3 +--
>> hw/ppc/pnv.c | 41 ++++++++++++++++----------------------
>> hw/ppc/pnv_psi.c | 10 +++-------
>> hw/ppc/ppc405_boards.c | 6 ++----
>> hw/ppc/ppc405_uc.c | 6 +++---
>> hw/ppc/ppc440_bamboo.c | 3 +--
>> hw/ppc/ppc4xx_devs.c | 2 +-
>> hw/ppc/sam460ex.c | 5 ++---
>> hw/ppc/spapr_irq.c | 8 +++-----
>> 19 files changed, 62 insertions(+), 102 deletions(-)
>>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-02-06 13:53 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-03 21:16 [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Philippe Mathieu-Daudé
2023-02-03 21:16 ` [PATCH 1/5] hw/misc/macio: Set QDev properties using QDev API Philippe Mathieu-Daudé
2023-02-03 21:16 ` [PATCH 2/5] hw/pci-host/raven: " Philippe Mathieu-Daudé
2023-02-05 11:03 ` Daniel Henrique Barboza
2023-02-03 21:16 ` [PATCH 3/5] hw/ppc/ppc4xx: " Philippe Mathieu-Daudé
2023-02-03 21:26 ` BALATON Zoltan
2023-02-05 10:59 ` Daniel Henrique Barboza
2023-02-03 21:16 ` [PATCH 4/5] hw/ppc/spapr: " Philippe Mathieu-Daudé
2023-02-05 11:00 ` Daniel Henrique Barboza
2023-02-06 8:06 ` Cédric Le Goater
2023-02-03 21:16 ` [PATCH 5/5] hw/ppc/pnv: " Philippe Mathieu-Daudé
2023-02-05 11:00 ` Daniel Henrique Barboza
2023-02-06 8:06 ` Cédric Le Goater
2023-02-05 11:05 ` [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3) Daniel Henrique Barboza
2023-02-06 8:00 ` Cédric Le Goater
2023-02-06 9:09 ` Mark Cave-Ayland
2023-02-06 13:52 ` Daniel Henrique Barboza
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.