Re: [PATCH for-7.2 2/2] ppc/pnv: fix QOM parenting of user creatable root ports

[Cédric Le Goater]

On 8/19/22 00:45, Daniel Henrique Barboza wrote:
> User creatable root ports are being parented by the 'peripheral' or the
> 'peripheral-anon' container. This happens because this is the regular
> QOM schema for sysbus devices that are added via the command line.
>
> Let's make this QOM hierarchy similar to what we have with default root
> ports, i.e. the root port must be parented by the pnv-root-bus. To do
> that we change the qom and bus parent of the root port during
> root_port_realize(). The realize() is shared by the default root port
> code path, so we can remove the code inside pnv_phb_attach_root_port()
> that was adding the root port as a child of the bus as well.
>
> While we're at it, change pnv_phb_attach_root_port() to receive a PCIBus
> instead of a PCIHostState to make it clear that the function does not
> make use of the PHB.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   hw/pci-host/pnv_phb.c | 38 +++++++++++++++++++-------------------
>   1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
> index 4ea33fb6ba..38ec8571b7 100644
> --- a/hw/pci-host/pnv_phb.c
> +++ b/hw/pci-host/pnv_phb.c
> @@ -62,27 +62,11 @@ static bool pnv_parent_fixup(Object *parent, BusState 
> *parent_bus,
>       return true;
>   }
>   
> -/*
> - * Attach a root port device.
> - *
> - * 'index' will be used both as a PCIE slot value and to calculate
> - * QOM id. 'chip_id' is going to be used as PCIE chassis for the
> - * root port.
> - */
> -static void pnv_phb_attach_root_port(PCIHostState *pci)
> +static void pnv_phb_attach_root_port(PCIBus *bus)

Do we still need this pnv_phb_attach_root_port routine ?

>   {
>       PCIDevice *root = pci_new(PCI_DEVFN(0, 0), TYPE_PNV_PHB_ROOT_PORT);
> -    const char *dev_id = DEVICE(root)->id;
> -    g_autofree char *default_id = NULL;
> -    int index;
>   
> -    index = object_property_get_int(OBJECT(pci->bus), "phb-id", &error_fatal);
> -    default_id = g_strdup_printf("%s[%d]", TYPE_PNV_PHB_ROOT_PORT, index);
> -
> -    object_property_add_child(OBJECT(pci->bus), dev_id ? dev_id : default_id,
> -                              OBJECT(root));
> -
> -    pci_realize_and_unref(root, pci->bus, &error_fatal);
> +    pci_realize_and_unref(root, bus, &error_fatal);
>   }
>   
>   /*
> @@ -184,7 +168,7 @@ static void pnv_phb_realize(DeviceState *dev, Error **errp)
>           return;
>       }
>   
> -    pnv_phb_attach_root_port(pci);
> +    pnv_phb_attach_root_port(pci->bus);
>   }
>   
>   static const char *pnv_phb_root_bus_path(PCIHostState *host_bridge,
> @@ -259,6 +243,11 @@ static void pnv_phb_root_port_realize(DeviceState *dev, 
> Error **errp)
>       Error *local_err = NULL;
>       int chip_id, index;
>   
> +    /*
> +     * 'index' will be used both as a PCIE slot value and to calculate
> +     * QOM id. 'chip_id' is going to be used as PCIE chassis for the
> +     * root port.
> +     */
>       chip_id = object_property_get_int(OBJECT(bus), "chip-id", &error_fatal);
>       index = object_property_get_int(OBJECT(bus), "phb-id", &error_fatal);
>   
> @@ -266,6 +255,17 @@ static void pnv_phb_root_port_realize(DeviceState *dev, Error **errp)
>       qdev_prop_set_uint8(dev, "chassis", chip_id);
>       qdev_prop_set_uint16(dev, "slot", index);
>   
> +    /*
> +     * User created root ports are QOM parented to one of
> +     * the peripheral containers but it's already at the right
> +     * parent bus. Change the QOM parent to be the same as the
> +     * parent bus it's already assigned to.
> +     */
> +    if (!pnv_parent_fixup(OBJECT(bus), BUS(bus), OBJECT(dev),
> +                          index, errp)) {
> +        return;
> +    }
> +
>       rpc->parent_realize(dev, &local_err);
>       if (local_err) {
>           error_propagate(errp, local_err);