| Message ID | 1422339213-23426-1-git-send-email-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
On 01/27/2015 05:13 PM, Alexey Kardashevskiy wrote: > Instead of tweaking a TCE table device by adding there a bypass flag, > let's add an alias to RAM and IOMMU memory region, and enable/disable > those according to the selected bypass mode. > This way IOMMU memory region can have size of the actual window rather > than ram_size which is essential for upcoming DDW support. > > This moves bypass logic to VIO layer and keeps @bypass flag in TCE table > for migration compatibility only. This replaces spapr_tce_set_bypass() > calls with explicit assignment to avoid confusion as the function could > do something more that just syncing the @bypass flag. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > Changes: > v2: > * kept @bypass in sPAPRTCETable not to break migration > --- > hw/ppc/spapr_iommu.c | 13 +++---------- > hw/ppc/spapr_vio.c | 39 +++++++++++++++++++++++++++++++++++++-- > include/hw/ppc/spapr.h | 1 - > include/hw/ppc/spapr_vio.h | 2 ++ > 4 files changed, 42 insertions(+), 13 deletions(-) > > diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c > index da47474..a7b027a 100644 > --- a/hw/ppc/spapr_iommu.c > +++ b/hw/ppc/spapr_iommu.c > @@ -72,9 +72,7 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr, > .perm = IOMMU_NONE, > }; > > - if (tcet->bypass) { > - ret.perm = IOMMU_RW; > - } else if ((addr >> tcet->page_shift) < tcet->nb_table) { > + if ((addr >> tcet->page_shift) < tcet->nb_table) { > /* Check if we are in bound */ > hwaddr page_mask = IOMMU_PAGE_MASK(tcet->page_shift); > > @@ -131,7 +129,8 @@ static int spapr_tce_table_realize(DeviceState *dev) > trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd); > > memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops, > - "iommu-spapr", ram_size); > + "iommu-spapr", > + (uint64_t)tcet->nb_table << tcet->page_shift); > > QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list); > > @@ -191,17 +190,11 @@ MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet) > return &tcet->iommu; > } > > -void spapr_tce_set_bypass(sPAPRTCETable *tcet, bool bypass) > -{ > - tcet->bypass = bypass; > -} > - > static void spapr_tce_reset(DeviceState *dev) > { > sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev); > size_t table_size = tcet->nb_table * sizeof(uint64_t); > > - tcet->bypass = false; > memset(tcet->table, 0, table_size); > } > > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c > index dc9e46a..b65eb11 100644 > --- a/hw/ppc/spapr_vio.c > +++ b/hw/ppc/spapr_vio.c > @@ -322,6 +322,18 @@ static void spapr_vio_quiesce_one(VIOsPAPRDevice *dev) > free_crq(dev); > } > > +static void spapr_vio_set_bypass(VIOsPAPRDevice *dev, bool bypass) > +{ > + if (!dev->tcet) { > + return; > + } > + > + memory_region_set_enabled(&dev->mrbypass, bypass); > + memory_region_set_enabled(spapr_tce_get_iommu(dev->tcet), !bypass); > + > + dev->tcet->bypass = bypass; > +} > + > static void rtas_set_tce_bypass(PowerPCCPU *cpu, sPAPREnvironment *spapr, > uint32_t token, > uint32_t nargs, target_ulong args, > @@ -348,7 +360,7 @@ static void rtas_set_tce_bypass(PowerPCCPU *cpu, sPAPREnvironment *spapr, > return; > } > > - spapr_tce_set_bypass(dev->tcet, !!enable); > + spapr_vio_set_bypass(dev, !!enable); > > rtas_st(rets, 0, RTAS_OUT_SUCCESS); > } > @@ -407,6 +419,7 @@ static void spapr_vio_busdev_reset(DeviceState *qdev) > > dev->signal_state = 0; > > + spapr_vio_set_bypass(dev, false); > if (pc->reset) { > pc->reset(dev); > } > @@ -456,12 +469,22 @@ static int spapr_vio_busdev_init(DeviceState *qdev) > > if (pc->rtce_window_size) { > uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev->reg; > + > + memory_region_init(&dev->mrroot, OBJECT(dev), "iommu-spapr-root", > + ram_size); > + memory_region_init_alias(&dev->mrbypass, OBJECT(dev), > + "iommu-spapr-bypass", get_system_memory(), > + 0, ram_size); > + memory_region_add_subregion_overlap(&dev->mrroot, 0, &dev->mrbypass, 1); > + address_space_init(&dev->as, &dev->mrroot, qdev->id); > + > dev->tcet = spapr_tce_new_table(qdev, liobn, > 0, > SPAPR_TCE_PAGE_SHIFT, > pc->rtce_window_size >> > SPAPR_TCE_PAGE_SHIFT, false); > - address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet), qdev->id); > + memory_region_add_subregion_overlap(&dev->mrroot, 0, > + spapr_tce_get_iommu(dev->tcet), 2); > } > > return pc->init(dev); > @@ -541,6 +564,17 @@ static void spapr_vio_bridge_class_init(ObjectClass *klass, void *data) > k->init = spapr_vio_bridge_init; > } > > +static int spapr_vio_post_load(void *opaque, int version_id) > +{ > + VIOsPAPRDevice *dev = VIO_SPAPR_DEVICE(opaque); > + > + if (dev->tcet) { > + spapr_vio_set_bypass(dev, dev->tcet->bypass); Oh. Just realized that if VIOsPAPRDevice migrated before sPAPRTCETable, @bypass will always be false. So please ignore this patch for now :( > + } > + > + return 0; > +} > + > static const TypeInfo spapr_vio_bridge_info = { > .name = "spapr-vio-bridge", > .parent = TYPE_SYS_BUS_DEVICE, > @@ -552,6 +586,7 @@ const VMStateDescription vmstate_spapr_vio = { > .name = "spapr_vio", > .version_id = 1, > .minimum_version_id = 1, > + .post_load = spapr_vio_post_load, > .fields = (VMStateField[]) { > /* Sanity check */ > VMSTATE_UINT32_EQUAL(reg, VIOsPAPRDevice), > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 716bff4..ebc7e51 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -475,7 +475,6 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, > uint32_t nb_table, > bool vfio_accel); > MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet); > -void spapr_tce_set_bypass(sPAPRTCETable *tcet, bool bypass); > int spapr_dma_dt(void *fdt, int node_off, const char *propname, > uint32_t liobn, uint64_t window, uint32_t size); > int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname, > diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h > index 46edc2a..6ad55d1 100644 > --- a/include/hw/ppc/spapr_vio.h > +++ b/include/hw/ppc/spapr_vio.h > @@ -64,6 +64,8 @@ struct VIOsPAPRDevice { > target_ulong signal_state; > VIOsPAPR_CRQ crq; > AddressSpace as; > + MemoryRegion mrroot; > + MemoryRegion mrbypass; > sPAPRTCETable *tcet; > }; > >
On Tue, Jan 27, 2015 at 05:13:32PM +1100, Alexey Kardashevskiy wrote: > Instead of tweaking a TCE table device by adding there a bypass flag, > let's add an alias to RAM and IOMMU memory region, and enable/disable > those according to the selected bypass mode. > This way IOMMU memory region can have size of the actual window rather > than ram_size which is essential for upcoming DDW support. > > This moves bypass logic to VIO layer and keeps @bypass flag in TCE table > for migration compatibility only. This replaces spapr_tce_set_bypass() > calls with explicit assignment to avoid confusion as the function could > do something more that just syncing the @bypass flag. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > Changes: > v2: > * kept @bypass in sPAPRTCETable not to break migration So, the bypass field definition in the struct should probably get a comment explaining how it's only used for migration compatibility now. Apart from that: Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c index da47474..a7b027a 100644 --- a/hw/ppc/spapr_iommu.c +++ b/hw/ppc/spapr_iommu.c @@ -72,9 +72,7 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr, .perm = IOMMU_NONE, }; - if (tcet->bypass) { - ret.perm = IOMMU_RW; - } else if ((addr >> tcet->page_shift) < tcet->nb_table) { + if ((addr >> tcet->page_shift) < tcet->nb_table) { /* Check if we are in bound */ hwaddr page_mask = IOMMU_PAGE_MASK(tcet->page_shift); @@ -131,7 +129,8 @@ static int spapr_tce_table_realize(DeviceState *dev) trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd); memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops, - "iommu-spapr", ram_size); + "iommu-spapr", + (uint64_t)tcet->nb_table << tcet->page_shift); QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list); @@ -191,17 +190,11 @@ MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet) return &tcet->iommu; } -void spapr_tce_set_bypass(sPAPRTCETable *tcet, bool bypass) -{ - tcet->bypass = bypass; -} - static void spapr_tce_reset(DeviceState *dev) { sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev); size_t table_size = tcet->nb_table * sizeof(uint64_t); - tcet->bypass = false; memset(tcet->table, 0, table_size); } diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c index dc9e46a..b65eb11 100644 --- a/hw/ppc/spapr_vio.c +++ b/hw/ppc/spapr_vio.c @@ -322,6 +322,18 @@ static void spapr_vio_quiesce_one(VIOsPAPRDevice *dev) free_crq(dev); } +static void spapr_vio_set_bypass(VIOsPAPRDevice *dev, bool bypass) +{ + if (!dev->tcet) { + return; + } + + memory_region_set_enabled(&dev->mrbypass, bypass); + memory_region_set_enabled(spapr_tce_get_iommu(dev->tcet), !bypass); + + dev->tcet->bypass = bypass; +} + static void rtas_set_tce_bypass(PowerPCCPU *cpu, sPAPREnvironment *spapr, uint32_t token, uint32_t nargs, target_ulong args, @@ -348,7 +360,7 @@ static void rtas_set_tce_bypass(PowerPCCPU *cpu, sPAPREnvironment *spapr, return; } - spapr_tce_set_bypass(dev->tcet, !!enable); + spapr_vio_set_bypass(dev, !!enable); rtas_st(rets, 0, RTAS_OUT_SUCCESS); } @@ -407,6 +419,7 @@ static void spapr_vio_busdev_reset(DeviceState *qdev) dev->signal_state = 0; + spapr_vio_set_bypass(dev, false); if (pc->reset) { pc->reset(dev); } @@ -456,12 +469,22 @@ static int spapr_vio_busdev_init(DeviceState *qdev) if (pc->rtce_window_size) { uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev->reg; + + memory_region_init(&dev->mrroot, OBJECT(dev), "iommu-spapr-root", + ram_size); + memory_region_init_alias(&dev->mrbypass, OBJECT(dev), + "iommu-spapr-bypass", get_system_memory(), + 0, ram_size); + memory_region_add_subregion_overlap(&dev->mrroot, 0, &dev->mrbypass, 1); + address_space_init(&dev->as, &dev->mrroot, qdev->id); + dev->tcet = spapr_tce_new_table(qdev, liobn, 0, SPAPR_TCE_PAGE_SHIFT, pc->rtce_window_size >> SPAPR_TCE_PAGE_SHIFT, false); - address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet), qdev->id); + memory_region_add_subregion_overlap(&dev->mrroot, 0, + spapr_tce_get_iommu(dev->tcet), 2); } return pc->init(dev); @@ -541,6 +564,17 @@ static void spapr_vio_bridge_class_init(ObjectClass *klass, void *data) k->init = spapr_vio_bridge_init; } +static int spapr_vio_post_load(void *opaque, int version_id) +{ + VIOsPAPRDevice *dev = VIO_SPAPR_DEVICE(opaque); + + if (dev->tcet) { + spapr_vio_set_bypass(dev, dev->tcet->bypass); + } + + return 0; +} + static const TypeInfo spapr_vio_bridge_info = { .name = "spapr-vio-bridge", .parent = TYPE_SYS_BUS_DEVICE, @@ -552,6 +586,7 @@ const VMStateDescription vmstate_spapr_vio = { .name = "spapr_vio", .version_id = 1, .minimum_version_id = 1, + .post_load = spapr_vio_post_load, .fields = (VMStateField[]) { /* Sanity check */ VMSTATE_UINT32_EQUAL(reg, VIOsPAPRDevice), diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 716bff4..ebc7e51 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -475,7 +475,6 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, uint32_t nb_table, bool vfio_accel); MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet); -void spapr_tce_set_bypass(sPAPRTCETable *tcet, bool bypass); int spapr_dma_dt(void *fdt, int node_off, const char *propname, uint32_t liobn, uint64_t window, uint32_t size); int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname, diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h index 46edc2a..6ad55d1 100644 --- a/include/hw/ppc/spapr_vio.h +++ b/include/hw/ppc/spapr_vio.h @@ -64,6 +64,8 @@ struct VIOsPAPRDevice { target_ulong signal_state; VIOsPAPR_CRQ crq; AddressSpace as; + MemoryRegion mrroot; + MemoryRegion mrbypass; sPAPRTCETable *tcet; };
Instead of tweaking a TCE table device by adding there a bypass flag, let's add an alias to RAM and IOMMU memory region, and enable/disable those according to the selected bypass mode. This way IOMMU memory region can have size of the actual window rather than ram_size which is essential for upcoming DDW support. This moves bypass logic to VIO layer and keeps @bypass flag in TCE table for migration compatibility only. This replaces spapr_tce_set_bypass() calls with explicit assignment to avoid confusion as the function could do something more that just syncing the @bypass flag. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Changes: v2: * kept @bypass in sPAPRTCETable not to break migration --- hw/ppc/spapr_iommu.c | 13 +++---------- hw/ppc/spapr_vio.c | 39 +++++++++++++++++++++++++++++++++++++-- include/hw/ppc/spapr.h | 1 - include/hw/ppc/spapr_vio.h | 2 ++ 4 files changed, 42 insertions(+), 13 deletions(-)