On 1/5/22 22:23, Daniel Henrique Barboza wrote:
> We want to be able to support user creatable pnv-phb4 objects to allow
> users to instantiate a powernv9 machine similar to what it is done with
> powernv8.
>
> The main difference is that pnv-phb3 devs are attached directly to the
> system bus and can be created in the command line. PCI devices such as
> root-ports can be explictly connected to them. This allows users to
> create the phbs, assign a bus name if desired, then connect devices onto
> them.
>
> pnv-phb4 devices on the other hand are created by adding PCI Express
> Controllers (PEC) that will create a certain amount of pnv-phb4 buses
> depending on the PEC index used. Index 0 will create 1 phb, index 1
> creates 2 phbs, index 2 creates 3 phbs. Creating all PECs from the same
> chip will create 6 PHBs. This doesn't users to rename the buses, like it
> is done with pnv-phb3, because there's no user control over how the
> pnv-phb4 are being created - aside from the amount of phbs and in which
> chips they'll reside.
>
> This implicit relationship between PEC devices and available buses can
> be tolerable for users that knows how the hardware works, but it's
> annoying for Libvirt to deal with. Since there's no explicit
> relationship, in the command line, between the created buses and the PCI
> devices that will connect to them, the domain XML needs to make a lot of
> extra assumptions regarding the relationship between regular PCI devices
> and the existing PECs.
>
> The first step to allow for user creatable pnv-phb4 devices is to
> decouple the pvn-phb logic from the pnv-phb4-pec code. This patch adds a
> helper called pnv_phb4_set_stack_phb_props() to remove the code from
> pnv_phb4_pec.c that initiates the object properties of pnv-phb4 devices.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> hw/pci-host/pnv_phb4.c | 25 +++++++++++++++++++++++++
> hw/pci-host/pnv_phb4_pec.c | 12 +-----------
> include/hw/pci-host/pnv_phb4.h | 1 +
> 3 files changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 83dedc878a..6c1a33bc66 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1158,6 +1158,31 @@ static AddressSpace *pnv_phb4_dma_iommu(PCIBus *bus, void *opaque, int devfn)
> return &ds->dma_as;
> }
>
> +/*
> + * Set the object properties of a phb in relation with its stack.
> + *
> + * Note: stack->pec must not be NULL.
> + */
> +void pnv_phb4_set_stack_phb_props(PnvPhb4PecStack *stack,
> + PnvPHB4 *phb)
> +{
> + PnvPhb4PecState *pec = stack->pec;
> + PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
> + char name[64];
> +
> + snprintf(name, sizeof(name), "xscom-pec-%d.%d-pci-stack-%d-phb",
> + pec->chip_id, pec->index, stack->stack_no);
> + pnv_xscom_region_init(&stack->phb_regs_mr, OBJECT(phb),
> + &pnv_phb4_xscom_ops, phb, name, 0x40);
> +
> + object_property_set_int(OBJECT(phb), "chip-id", pec->chip_id,
> + &error_fatal);
> + object_property_set_int(OBJECT(phb), "version", pecc->version,
> + &error_fatal);
> + object_property_set_link(OBJECT(phb), "stack", OBJECT(stack),
> + &error_abort);
> +}
> +
> static void pnv_phb4_instance_init(Object *obj)
> {
> PnvPHB4 *phb = PNV_PHB4(obj);
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index f3e4fa0c82..057d4b07fb 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -577,17 +577,7 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error **errp)
> PHB4_PEC_PCI_STK_REGS_COUNT);
>
> /* PHB pass-through */
> - snprintf(name, sizeof(name), "xscom-pec-%d.%d-pci-stack-%d-phb",
> - pec->chip_id, pec->index, stack->stack_no);
> - pnv_xscom_region_init(&stack->phb_regs_mr, OBJECT(&stack->phb),
> - &pnv_phb4_xscom_ops, &stack->phb, name, 0x40);
> -
> - object_property_set_int(OBJECT(&stack->phb), "chip-id", pec->chip_id,
> - &error_fatal);
> - object_property_set_int(OBJECT(&stack->phb), "version", pecc->version,
> - &error_fatal);
> - object_property_set_link(OBJECT(&stack->phb), "stack", OBJECT(stack),
> - &error_abort);
> + pnv_phb4_set_stack_phb_props(stack, &stack->phb);
> if (!sysbus_realize(SYS_BUS_DEVICE(&stack->phb), errp)) {
> return;
> }
> diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
> index ea63df9676..7f5b9cc0ac 100644
> --- a/include/hw/pci-host/pnv_phb4.h
> +++ b/include/hw/pci-host/pnv_phb4.h
> @@ -131,6 +131,7 @@ struct PnvPHB4 {
>
> void pnv_phb4_pic_print_info(PnvPHB4 *phb, Monitor *mon);
> void pnv_phb4_update_regions(PnvPhb4PecStack *stack);
> +void pnv_phb4_set_stack_phb_props(PnvPhb4PecStack *stack, PnvPHB4 *phb);
> extern const MemoryRegionOps pnv_phb4_xscom_ops;
>
> /*
>