Re: [Qemu-devel] spapr-pci: introduce a finish_realize() callback

[Mike Day]

On 21/11/13 15:08 +1100, Alexey Kardashevskiy wrote:
> The spapr-pci PHB initializes IOMMU for emulataed devices only.
> The upcoming VFIO support will do it different. However both emulated
> and VFIO PHB types share most of the initialization code.
> For the type specific things a new finish_realize() callback is
> introduced.
> 
> This introduces sPAPRPHBClass derived from PCIHostBridgeClass and
> adds the callback pointer.
> 
> This implements finish_realize() for emulated devices.
> 
> This changes initialization steps order to have the finish_realize()
> call at the end of the spapr_finalize().
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>

Reviewed-by: Mike Day <address@hidden>

> ---
> Changes:
> v5:
> * this is a new patch in the series, it was a part of a previous patch
> ---
>  hw/ppc/spapr_pci.c          | 46 
> +++++++++++++++++++++++++++++----------------
>  include/hw/pci-host/spapr.h | 18 ++++++++++++++++--
>  2 files changed, 46 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index aeb012d..963841c 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -500,6 +500,7 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>      SysBusDevice *s = SYS_BUS_DEVICE(dev);
>      sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s);
>      PCIHostState *phb = PCI_HOST_BRIDGE(s);
> +    sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(s);
>      const char *busname;
>      char *namebuf;
>      int i;
> @@ -609,22 +610,6 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>                             PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
>      phb->bus = bus;
>  
> -    sphb->dma_window_start = 0;
> -    sphb->dma_window_size = 0x40000000;
> -    sphb->tcet = spapr_tce_new_table(dev, sphb->dma_liobn,
> -                                     sphb->dma_window_size);
> -    if (!sphb->tcet) {
> -        error_setg(errp, "Unable to create TCE table for %s",
> -                   sphb->dtbusname);
> -        return;
> -    }
> -    address_space_init(&sphb->iommu_as, spapr_tce_get_iommu(sphb->tcet),
> -                       sphb->dtbusname);
> -
> -    pci_setup_iommu(bus, spapr_pci_dma_iommu, sphb);
> -
> -    pci_bus_set_route_irq_fn(bus, spapr_route_intx_pin_to_irq);
> -
>      QLIST_INSERT_HEAD(&spapr->phbs, sphb, list);
>  
>      /* Initialize the LSI table */
> @@ -639,6 +624,32 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  
>          sphb->lsi_table[i].irq = irq;
>      }
> +
> +    pci_setup_iommu(sphb->parent_obj.bus, spapr_pci_dma_iommu, sphb);
> +
> +    pci_bus_set_route_irq_fn(bus, spapr_route_intx_pin_to_irq);
> +
> +    if (!info->finish_realize) {
> +        error_setg(errp, "finish_realize not defined");
> +        return;
> +    }
> +
> +    info->finish_realize(sphb, errp);
> +}
> +
> +static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
> +{
> +    sphb->dma_window_start = 0;
> +    sphb->dma_window_size = 0x40000000;
> +    sphb->tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
> +                                     sphb->dma_window_size);
> +    if (!sphb->tcet) {
> +        error_setg(errp, "Unable to create TCE table for %s",
> +                   sphb->dtbusname);
> +        return ;
> +    }
> +    address_space_init(&sphb->iommu_as, spapr_tce_get_iommu(sphb->tcet),
> +                       sphb->dtbusname);
>  }
>  
>  static void spapr_phb_reset(DeviceState *qdev)
> @@ -722,12 +733,14 @@ static void spapr_phb_class_init(ObjectClass *klass, 
> void *data)
>  {
>      PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_CLASS(klass);
>  
>      hc->root_bus_path = spapr_phb_root_bus_path;
>      dc->realize = spapr_phb_realize;
>      dc->props = spapr_phb_properties;
>      dc->reset = spapr_phb_reset;
>      dc->vmsd = &vmstate_spapr_pci;
> +    spc->finish_realize = spapr_phb_finish_realize;
>  }
>  
>  static const TypeInfo spapr_phb_info = {
> @@ -735,6 +748,7 @@ static const TypeInfo spapr_phb_info = {
>      .parent        = TYPE_PCI_HOST_BRIDGE,
>      .instance_size = sizeof(sPAPRPHBState),
>      .class_init    = spapr_phb_class_init,
> +    .class_size    = sizeof(sPAPRPHBClass),
>  };
>  
>  PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 970b4a9..0f428a1 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -34,7 +34,21 @@
>  #define SPAPR_PCI_HOST_BRIDGE(obj) \
>      OBJECT_CHECK(sPAPRPHBState, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
>  
> -typedef struct sPAPRPHBState {
> +#define SPAPR_PCI_HOST_BRIDGE_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(sPAPRPHBClass, (klass), TYPE_SPAPR_PCI_HOST_BRIDGE)
> +#define SPAPR_PCI_HOST_BRIDGE_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(sPAPRPHBClass, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
> +
> +typedef struct sPAPRPHBClass sPAPRPHBClass;
> +typedef struct sPAPRPHBState sPAPRPHBState;
> +
> +struct sPAPRPHBClass {
> +    PCIHostBridgeClass parent_class;
> +
> +    void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
> +};
> +
> +struct sPAPRPHBState {
>      PCIHostState parent_obj;
>  
>      int32_t index;
> @@ -62,7 +76,7 @@ typedef struct sPAPRPHBState {
>      } msi_table[SPAPR_MSIX_MAX_DEVS];
>  
>      QLIST_ENTRY(sPAPRPHBState) list;
> -} sPAPRPHBState;
> +};
>  
>  #define SPAPR_PCI_BASE_BUID          0x800000020000000ULL
>  
> -- 
> 1.8.4.rc4
> 
> 
> 
>