On 03/13/2014 12:56 PM, Andreas Färber wrote: > Am 21.11.2013 05:08, schrieb Alexey Kardashevskiy: >> The spapr-pci PHB initializes IOMMU for emulataed devices only. > > "emulated" > >> 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 <a...@ozlabs.ru> >> --- >> 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); > > Accessing ->parent_obj anywhere but in VMState is very likely wrong, > here it is definitely.
This is just a victim of multiple cut-n-paste's. My bad, missed it... It should be: pci_setup_iommu(bus, spapr_pci_dma_iommu, sphb); > Also due to time pressure I'm not comfortable taking this into 2.0-rc0 > and would like to see how this works for the second implementation. Repost it? Or I better wait for something (what?)? Thanks. > > Regards, > Andreas > >> + >> + 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 >> >> > > -- Alexey