Re: [PATCH for-7.2 v4 01/11] ppc/pnv: add phb-id/chip-id PnvPHB3RootBus properties

[Frederic Barrat <fbarrat@linux.ibm.com>]

On 11/08/2022 18:39, Daniel Henrique Barboza wrote:
> We rely on the phb-id and chip-id, which are PHB properties, to assign
> chassis and slot to the root port. For default devices this is no big
> deal: the root port is being created under pnv_phb_realize() and the
> values are being passed on via the 'index' and 'chip-id' of the
> pnv_phb_attach_root_port() helper.
> 
> If we want to implement user created root ports we have a problem. The
> user created root port will not be aware of which PHB it belongs to,
> unless we're willing to violate QOM best practices and access the PHB
> via dev->parent_bus->parent. What we can do is to access the root bus
> parent bus.
> 
> Since we're already assigning the root port as QOM child of the bus, and
> the bus is initiated using PHB properties, let's add phb-id and chip-id
> as properties of the bus. This will allow us trivial access to them, for
> both user-created and default root ports, without doing anything too
> shady with QOM.
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---


Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

   Fred


>   hw/pci-host/pnv_phb3.c         | 50 ++++++++++++++++++++++++++++++++++
>   include/hw/pci-host/pnv_phb3.h |  9 +++++-
>   2 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
> index d4c04a281a..af8575c007 100644
> --- a/hw/pci-host/pnv_phb3.c
> +++ b/hw/pci-host/pnv_phb3.c
> @@ -1006,6 +1006,11 @@ void pnv_phb3_bus_init(DeviceState *dev, PnvPHB3 *phb)
>                                        &phb->pci_mmio, &phb->pci_io,
>                                        0, 4, TYPE_PNV_PHB3_ROOT_BUS);
>   
> +    object_property_set_int(OBJECT(pci->bus), "phb-id", phb->phb_id,
> +                            &error_abort);
> +    object_property_set_int(OBJECT(pci->bus), "chip-id", phb->chip_id,
> +                            &error_abort);
> +
>       pci_setup_iommu(pci->bus, pnv_phb3_dma_iommu, phb);
>   }
>   
> @@ -1105,10 +1110,55 @@ static const TypeInfo pnv_phb3_type_info = {
>       .instance_init = pnv_phb3_instance_init,
>   };
>   
> +static void pnv_phb3_root_bus_get_prop(Object *obj, Visitor *v,
> +                                       const char *name,
> +                                       void *opaque, Error **errp)
> +{
> +    PnvPHB3RootBus *bus = PNV_PHB3_ROOT_BUS(obj);
> +    uint64_t value = 0;
> +
> +    if (strcmp(name, "phb-id") == 0) {
> +        value = bus->phb_id;
> +    } else {
> +        value = bus->chip_id;
> +    }
> +
> +    visit_type_size(v, name, &value, errp);
> +}
> +
> +static void pnv_phb3_root_bus_set_prop(Object *obj, Visitor *v,
> +                                       const char *name,
> +                                       void *opaque, Error **errp)
> +
> +{
> +    PnvPHB3RootBus *bus = PNV_PHB3_ROOT_BUS(obj);
> +    uint64_t value;
> +
> +    if (!visit_type_size(v, name, &value, errp)) {
> +        return;
> +    }
> +
> +    if (strcmp(name, "phb-id") == 0) {
> +        bus->phb_id = value;
> +    } else {
> +        bus->chip_id = value;
> +    }
> +}
> +
>   static void pnv_phb3_root_bus_class_init(ObjectClass *klass, void *data)
>   {
>       BusClass *k = BUS_CLASS(klass);
>   
> +    object_class_property_add(klass, "phb-id", "int",
> +                              pnv_phb3_root_bus_get_prop,
> +                              pnv_phb3_root_bus_set_prop,
> +                              NULL, NULL);
> +
> +    object_class_property_add(klass, "chip-id", "int",
> +                              pnv_phb3_root_bus_get_prop,
> +                              pnv_phb3_root_bus_set_prop,
> +                              NULL, NULL);
> +
>       /*
>        * PHB3 has only a single root complex. Enforce the limit on the
>        * parent bus
> diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h
> index bff69201d9..4854f6d2f6 100644
> --- a/include/hw/pci-host/pnv_phb3.h
> +++ b/include/hw/pci-host/pnv_phb3.h
> @@ -104,9 +104,16 @@ struct PnvPBCQState {
>   };
>   
>   /*
> - * PHB3 PCIe Root port
> + * PHB3 PCIe Root Bus
>    */
>   #define TYPE_PNV_PHB3_ROOT_BUS "pnv-phb3-root"
> +struct PnvPHB3RootBus {
> +    PCIBus parent;
> +
> +    uint32_t chip_id;
> +    uint32_t phb_id;
> +};
> +OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB3RootBus, PNV_PHB3_ROOT_BUS)
>   
>   /*
>    * PHB3 PCIe Host Bridge for PowerNV machines (POWER8)