pnv_parent_qom_fixup() and pnv_parent_bus_fixup() are versions of the
helpers that were reverted by commit 9c10d86fee "ppc/pnv: Remove
user-created PHB{3,4,5} devices". They are needed to amend the QOM and
bus hierarchies of user created pnv-phbs, matching them with default
pnv-phbs.
A new helper pnv_phb_user_device_init() is created to handle
user-created devices setup. We're going to call it inside
pnv_phb_realize() in case we're realizing an user created device. This
will centralize all user device realated in a single spot, leaving the
realize functions of the phb3/phb4 backends untouched.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
hw/pci-host/pnv_phb.c | 69 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
index 826c0c144e..da779dc298 100644
--- a/hw/pci-host/pnv_phb.c
+++ b/hw/pci-host/pnv_phb.c
@@ -18,6 +18,37 @@
#include "hw/qdev-properties.h"
#include "qom/object.h"
+
+/*
+ * Set the QOM parent of an object child. If the device state
+ * associated with the child has an id, use it as QOM id. Otherwise
+ * use object_typename[index] as QOM id.
+ */
+static void pnv_parent_qom_fixup(Object *parent, Object *child, int index)
+{
+ g_autofree char *default_id =
+ g_strdup_printf("%s[%d]", object_get_typename(child), index);
+ const char *dev_id = DEVICE(child)->id;
+
+ if (child->parent == parent) {
+ return;
+ }
+
+ object_ref(child);
+ object_unparent(child);
+ object_property_add_child(parent, dev_id ? dev_id : default_id, child);
+ object_unref(child);
+}
+
+static void pnv_parent_bus_fixup(DeviceState *parent, DeviceState *child)
+{
+ BusState *parent_bus = qdev_get_parent_bus(parent);
+
+ if (!qdev_set_parent_bus(child, parent_bus, &error_fatal)) {
+ return;
+ }
+}
+
/*
* Attach a root port device.
*
@@ -41,6 +72,36 @@ static void pnv_phb_attach_root_port(PCIHostState *pci)
pci_realize_and_unref(root, pci->bus, &error_fatal);
}
+/*
+ * User created devices won't have the initial setup that default
+ * devices have. This setup consists of assigning a parent device
+ * (chip for PHB3, PEC for PHB4/5) that will be the QOM/bus parent
+ * of the PHB.
+ */
+static void pnv_phb_user_device_init(PnvPHB *phb)
+{
+ PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+ PnvChip *chip = pnv_get_chip(pnv, phb->chip_id);
+ Object *parent = NULL;
+
+ if (!chip) {
+ error_setg(&error_fatal, "invalid chip id: %d", phb->chip_id);
+ return;
+ }
+
+ /*
+ * Reparent user created devices to the chip to build
+ * correctly the device tree. pnv_xscom_dt() needs every
+ * PHB to be a child of the chip to build the DT correctly.
+ *
+ * TODO: for version 3 we're still parenting the PHB with the
+ * chip. We should parent with a (so far not implemented)
+ * PHB3 PEC device.
+ */
+ pnv_parent_qom_fixup(parent, OBJECT(phb), phb->phb_id);
+ pnv_parent_bus_fixup(DEVICE(chip), DEVICE(phb));
+}
+
static void pnv_phb_realize(DeviceState *dev, Error **errp)
{
PnvPHB *phb = PNV_PHB(dev);
@@ -74,6 +135,14 @@ static void pnv_phb_realize(DeviceState *dev, Error **errp)
object_property_set_uint(phb->backend, "chip-id", phb->chip_id, errp);
object_property_set_link(phb->backend, "phb-base", OBJECT(phb), errp);
+ /*
+ * Handle user created devices. User devices will not have a
+ * pointer to a chip (PHB3) and a PEC (PHB4/5).
+ */
+ if (!phb->chip && !phb->pec) {
+ pnv_phb_user_device_init(phb);
+ }
+
if (phb->version == 3) {
object_property_set_link(phb->backend, "chip",
OBJECT(phb->chip), errp);
--
2.36.1
On 03/08/2022 15:44, Daniel Henrique Barboza wrote:
> pnv_parent_qom_fixup() and pnv_parent_bus_fixup() are versions of the
> helpers that were reverted by commit 9c10d86fee "ppc/pnv: Remove
> user-created PHB{3,4,5} devices". They are needed to amend the QOM and
> bus hierarchies of user created pnv-phbs, matching them with default
> pnv-phbs.
>
> A new helper pnv_phb_user_device_init() is created to handle
> user-created devices setup. We're going to call it inside
> pnv_phb_realize() in case we're realizing an user created device. This
> will centralize all user device realated in a single spot, leaving the
> realize functions of the phb3/phb4 backends untouched.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> hw/pci-host/pnv_phb.c | 69 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
> index 826c0c144e..da779dc298 100644
> --- a/hw/pci-host/pnv_phb.c
> +++ b/hw/pci-host/pnv_phb.c
> @@ -18,6 +18,37 @@
> #include "hw/qdev-properties.h"
> #include "qom/object.h"
>
> +
> +/*
> + * Set the QOM parent of an object child. If the device state
> + * associated with the child has an id, use it as QOM id. Otherwise
> + * use object_typename[index] as QOM id.
> + */
> +static void pnv_parent_qom_fixup(Object *parent, Object *child, int index)
> +{
> + g_autofree char *default_id =
> + g_strdup_printf("%s[%d]", object_get_typename(child), index);
> + const char *dev_id = DEVICE(child)->id;
> +
> + if (child->parent == parent) {
> + return;
> + }
> +
> + object_ref(child);
> + object_unparent(child);
> + object_property_add_child(parent, dev_id ? dev_id : default_id, child);
> + object_unref(child);
> +}
> +
> +static void pnv_parent_bus_fixup(DeviceState *parent, DeviceState *child)
> +{
> + BusState *parent_bus = qdev_get_parent_bus(parent);
> +
> + if (!qdev_set_parent_bus(child, parent_bus, &error_fatal)) {
> + return;
> + }
> +}
> +
> /*
> * Attach a root port device.
> *
> @@ -41,6 +72,36 @@ static void pnv_phb_attach_root_port(PCIHostState *pci)
> pci_realize_and_unref(root, pci->bus, &error_fatal);
> }
>
> +/*
> + * User created devices won't have the initial setup that default
> + * devices have. This setup consists of assigning a parent device
> + * (chip for PHB3, PEC for PHB4/5) that will be the QOM/bus parent
> + * of the PHB.
> + */
> +static void pnv_phb_user_device_init(PnvPHB *phb)
> +{
> + PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> + PnvChip *chip = pnv_get_chip(pnv, phb->chip_id);
> + Object *parent = NULL;
> +
> + if (!chip) {
> + error_setg(&error_fatal, "invalid chip id: %d", phb->chip_id);
> + return;
> + }
> +
> + /*
> + * Reparent user created devices to the chip to build
> + * correctly the device tree. pnv_xscom_dt() needs every
> + * PHB to be a child of the chip to build the DT correctly.
> + *
> + * TODO: for version 3 we're still parenting the PHB with the
> + * chip. We should parent with a (so far not implemented)
> + * PHB3 PEC device.
> + */
> + pnv_parent_qom_fixup(parent, OBJECT(phb), phb->phb_id);
Here we reparent the phb to NULL. I can see in the following patches why
it's done this way and the end result is ok. After this patch, we can't
create a user device yet, so no harm can be done. But a (temporary)
comment could save the reviewer a heart attack :-)
Fred
> + pnv_parent_bus_fixup(DEVICE(chip), DEVICE(phb));
> +}
> +
> static void pnv_phb_realize(DeviceState *dev, Error **errp)
> {
> PnvPHB *phb = PNV_PHB(dev);
> @@ -74,6 +135,14 @@ static void pnv_phb_realize(DeviceState *dev, Error **errp)
> object_property_set_uint(phb->backend, "chip-id", phb->chip_id, errp);
> object_property_set_link(phb->backend, "phb-base", OBJECT(phb), errp);
>
> + /*
> + * Handle user created devices. User devices will not have a
> + * pointer to a chip (PHB3) and a PEC (PHB4/5).
> + */
> + if (!phb->chip && !phb->pec) {
> + pnv_phb_user_device_init(phb);
> + }
> +
> if (phb->version == 3) {
> object_property_set_link(phb->backend, "chip",
> OBJECT(phb->chip), errp);
On 8/5/22 07:41, Frederic Barrat wrote:
>
>
> On 03/08/2022 15:44, Daniel Henrique Barboza wrote:
>> pnv_parent_qom_fixup() and pnv_parent_bus_fixup() are versions of the
>> helpers that were reverted by commit 9c10d86fee "ppc/pnv: Remove
>> user-created PHB{3,4,5} devices". They are needed to amend the QOM and
>> bus hierarchies of user created pnv-phbs, matching them with default
>> pnv-phbs.
>>
>> A new helper pnv_phb_user_device_init() is created to handle
>> user-created devices setup. We're going to call it inside
>> pnv_phb_realize() in case we're realizing an user created device. This
>> will centralize all user device realated in a single spot, leaving the
>> realize functions of the phb3/phb4 backends untouched.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>> hw/pci-host/pnv_phb.c | 69 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 69 insertions(+)
>>
>> diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
>> index 826c0c144e..da779dc298 100644
>> --- a/hw/pci-host/pnv_phb.c
>> +++ b/hw/pci-host/pnv_phb.c
>> @@ -18,6 +18,37 @@
>> #include "hw/qdev-properties.h"
>> #include "qom/object.h"
>> +
>> +/*
>> + * Set the QOM parent of an object child. If the device state
>> + * associated with the child has an id, use it as QOM id. Otherwise
>> + * use object_typename[index] as QOM id.
>> + */
>> +static void pnv_parent_qom_fixup(Object *parent, Object *child, int index)
>> +{
>> + g_autofree char *default_id =
>> + g_strdup_printf("%s[%d]", object_get_typename(child), index);
>> + const char *dev_id = DEVICE(child)->id;
>> +
>> + if (child->parent == parent) {
>> + return;
>> + }
>> +
>> + object_ref(child);
>> + object_unparent(child);
>> + object_property_add_child(parent, dev_id ? dev_id : default_id, child);
>> + object_unref(child);
>> +}
>> +
>> +static void pnv_parent_bus_fixup(DeviceState *parent, DeviceState *child)
>> +{
>> + BusState *parent_bus = qdev_get_parent_bus(parent);
>> +
>> + if (!qdev_set_parent_bus(child, parent_bus, &error_fatal)) {
>> + return;
>> + }
>> +}
>> +
>> /*
>> * Attach a root port device.
>> *
>> @@ -41,6 +72,36 @@ static void pnv_phb_attach_root_port(PCIHostState *pci)
>> pci_realize_and_unref(root, pci->bus, &error_fatal);
>> }
>> +/*
>> + * User created devices won't have the initial setup that default
>> + * devices have. This setup consists of assigning a parent device
>> + * (chip for PHB3, PEC for PHB4/5) that will be the QOM/bus parent
>> + * of the PHB.
>> + */
>> +static void pnv_phb_user_device_init(PnvPHB *phb)
>> +{
>> + PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>> + PnvChip *chip = pnv_get_chip(pnv, phb->chip_id);
>> + Object *parent = NULL;
>> +
>> + if (!chip) {
>> + error_setg(&error_fatal, "invalid chip id: %d", phb->chip_id);
>> + return;
>> + }
>> +
>> + /*
>> + * Reparent user created devices to the chip to build
>> + * correctly the device tree. pnv_xscom_dt() needs every
>> + * PHB to be a child of the chip to build the DT correctly.
>> + *
>> + * TODO: for version 3 we're still parenting the PHB with the
>> + * chip. We should parent with a (so far not implemented)
>> + * PHB3 PEC device.
>> + */
>> + pnv_parent_qom_fixup(parent, OBJECT(phb), phb->phb_id);
>
>
> Here we reparent the phb to NULL. I can see in the following patches why it's done this way and the end result is ok. After this patch, we can't create a user device yet, so no harm can be done. But a (temporary) comment could save the reviewer a heart attack :-)
Nah that was a mistake, I didn't intend to leave the code to be reparenting
stuff to NULL, even if it's not possible to break stuff at this moment.
I'll amend it like this:
+ if (phb->version == 3) {
+ parent = OBJECT(chip);
+ } else {
+ /* phb4 support will be added later */
+ return;
+ }
+
Thanks,
Daniel
>
> Fred
>
>
>> + pnv_parent_bus_fixup(DEVICE(chip), DEVICE(phb));
>> +}
>> +
>> static void pnv_phb_realize(DeviceState *dev, Error **errp)
>> {
>> PnvPHB *phb = PNV_PHB(dev);
>> @@ -74,6 +135,14 @@ static void pnv_phb_realize(DeviceState *dev, Error **errp)
>> object_property_set_uint(phb->backend, "chip-id", phb->chip_id, errp);
>> object_property_set_link(phb->backend, "phb-base", OBJECT(phb), errp);
>> + /*
>> + * Handle user created devices. User devices will not have a
>> + * pointer to a chip (PHB3) and a PEC (PHB4/5).
>> + */
>> + if (!phb->chip && !phb->pec) {
>> + pnv_phb_user_device_init(phb);
>> + }
>> +
>> if (phb->version == 3) {
>> object_property_set_link(phb->backend, "chip",
>> OBJECT(phb->chip), errp);
© 2016 - 2026 Red Hat, Inc.