Message ID | 1458546426-26222-6-git-send-email-aik@ozlabs.ru (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Mon, Mar 21, 2016 at 06:46:53PM +1100, Alexey Kardashevskiy wrote: > Currently TCE tables are created once at start and their sizes never > change. We are going to change that by introducing a Dynamic DMA windows > support where DMA configuration may change during the guest execution. > > This changes spapr_tce_new_table() to create an empty zero-size IOMMU > memory region (IOMMU MR). Only LIOBN is assigned by the time of creation. > It still will be called once at the owner object (VIO or PHB) creation. > > This introduces an "enabled" state for TCE table objects with two > helper functions - spapr_tce_table_enable()/spapr_tce_table_disable(). > - spapr_tce_table_enable() receives TCE table parameters, allocates > a guest view of the TCE table (in the user space or KVM) and > sets the correct size on the IOMMU MR. > - spapr_tce_table_disable() disposes the table and resets the IOMMU MR > size. > > This changes the PHB reset handler to do the default DMA initialization > instead of spapr_phb_realize(). This does not make differenct now but > later with more than just one DMA window, we will have to remove them all > and create the default one on a system reset. > > No visible change in behaviour is expected except the actual table > will be reallocated every reset. We might optimize this later. > > The other way to implement this would be dynamically create/remove > the TCE table QOM objects but this would make migration impossible > as the migration code expects all QOM objects to exist at the receiver > so we have to have TCE table objects created when migration begins. > > spapr_tce_table_do_enable() is separated from from spapr_tce_table_enable() > as later it will be called at the sPAPRTCETable post-migration stage when > it already has all the properties set after the migration; the same is > done for spapr_tce_table_disable(). > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> R-b stands, but I noticed one nit: > --- > Changes: > v14: > * added spapr_tce_table_do_disable(), will make difference in following > patch with fully dynamic table migration > --- > hw/ppc/spapr_iommu.c | 86 ++++++++++++++++++++++++++++++++++++-------------- > hw/ppc/spapr_pci.c | 13 ++++++-- > hw/ppc/spapr_vio.c | 8 ++--- > include/hw/ppc/spapr.h | 10 +++--- > 4 files changed, 81 insertions(+), 36 deletions(-) > > diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c > index 8132f64..9bcd3f6 100644 > --- a/hw/ppc/spapr_iommu.c > +++ b/hw/ppc/spapr_iommu.c > @@ -17,6 +17,7 @@ > * License along with this library; if not, see <http://www.gnu.org/licenses/>. > */ > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "hw/hw.h" > #include "sysemu/kvm.h" > #include "hw/qdev.h" > @@ -174,15 +175,9 @@ static int spapr_tce_table_realize(DeviceState *dev) > sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev); > > tcet->fd = -1; > - tcet->table = spapr_tce_alloc_table(tcet->liobn, > - tcet->page_shift, > - tcet->nb_table, > - &tcet->fd, > - tcet->need_vfio); > - > + tcet->need_vfio = false; > memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops, > - "iommu-spapr", > - (uint64_t)tcet->nb_table << tcet->page_shift); > + "iommu-spapr", 0); > > QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list); > > @@ -224,14 +219,10 @@ void spapr_tce_set_need_vfio(sPAPRTCETable *tcet, bool need_vfio) > tcet->table = newtable; > } > > -sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, > - uint64_t bus_offset, > - uint32_t page_shift, > - uint32_t nb_table, > - bool need_vfio) > +sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn) > { > sPAPRTCETable *tcet; > - char tmp[64]; > + char tmp[32]; > > if (spapr_tce_find_by_liobn(liobn)) { > fprintf(stderr, "Attempted to create TCE table with duplicate" > @@ -239,16 +230,8 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, > return NULL; > } > > - if (!nb_table) { > - return NULL; > - } > - > tcet = SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE)); > tcet->liobn = liobn; > - tcet->bus_offset = bus_offset; > - tcet->page_shift = page_shift; > - tcet->nb_table = nb_table; > - tcet->need_vfio = need_vfio; > > snprintf(tmp, sizeof(tmp), "tce-table-%x", liobn); > object_property_add_child(OBJECT(owner), tmp, OBJECT(tcet), NULL); > @@ -258,14 +241,69 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, > return tcet; > } > > +static void spapr_tce_table_do_enable(sPAPRTCETable *tcet) > +{ > + if (!tcet->nb_table) { > + return; > + } > + > + tcet->table = spapr_tce_alloc_table(tcet->liobn, > + tcet->page_shift, > + tcet->nb_table, > + &tcet->fd, > + tcet->need_vfio); > + > + memory_region_set_size(&tcet->iommu, > + (uint64_t)tcet->nb_table << tcet->page_shift); > + > + tcet->enabled = true; > +} > + > +void spapr_tce_table_enable(sPAPRTCETable *tcet, > + uint32_t page_shift, uint64_t bus_offset, > + uint32_t nb_table) > +{ > + if (tcet->enabled) { > + error_report("Warning: trying to enable already enabled TCE table"); > + return; > + } > + > + tcet->bus_offset = bus_offset; > + tcet->page_shift = page_shift; > + tcet->nb_table = nb_table; > + > + spapr_tce_table_do_enable(tcet); > +} > + > +static void spapr_tce_table_do_disable(sPAPRTCETable *tcet) > +{ > + memory_region_set_size(&tcet->iommu, 0); > + > + spapr_tce_free_table(tcet->table, tcet->fd, tcet->nb_table); > + tcet->fd = -1; > + tcet->table = NULL; > + tcet->enabled = false; > + tcet->bus_offset = 0; > + tcet->page_shift = 0; > + tcet->nb_table = 0; > +} > + > +static void spapr_tce_table_disable(sPAPRTCETable *tcet) > +{ > + if (!tcet->enabled) { > + error_report("Warning: trying to disable already disabled TCE table"); > + return; > + } > + spapr_tce_table_do_disable(tcet); > +} > + > static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp) > { > sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev); > > QLIST_REMOVE(tcet, list); > > - spapr_tce_free_table(tcet->table, tcet->fd, tcet->nb_table); > - tcet->fd = -1; > + spapr_tce_table_disable(tcet); > } > > MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet) > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 18332bf..df5f7b9 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -818,14 +818,15 @@ static void spapr_phb_dma_window_enable(sPAPRPHBState *sphb, > return; > } > > - tcet = spapr_tce_new_table(DEVICE(sphb), liobn, window_addr, > - page_shift, nb_table, false); > + tcet = spapr_tce_find_by_liobn(liobn); > if (!tcet) { > error_setg(errp, "Unable to create TCE table liobn %x for %s", > liobn, sphb->dtbusname); > return; > } > > + spapr_tce_table_enable(tcet, page_shift, window_addr, nb_table); > + > memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset, > spapr_tce_get_iommu(tcet)); > } > @@ -1335,6 +1336,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > PCIBus *bus; > uint64_t msi_window_size = 4096; > Error *local_err = NULL; > + sPAPRTCETable *tcet; > > if (sphb->index != (uint32_t)-1) { > hwaddr windows_base; > @@ -1486,6 +1488,13 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > } > } > > + /* DMA setup */ > + tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn); > + if (!tcet) { > + error_report("No default TCE table for %s", sphb->dtbusname); > + return; You have an errp in this function so you should error_setg(), rather than just error_report(). > + } > + > /* Register default 32bit DMA window */ > spapr_phb_dma_window_enable(sphb, sphb->dma_liobn, SPAPR_TCE_PAGE_SHIFT, > sphb->dma_win_addr, sphb->dma_win_size, > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c > index 0f61a55..7f57290 100644 > --- a/hw/ppc/spapr_vio.c > +++ b/hw/ppc/spapr_vio.c > @@ -481,11 +481,9 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp) > 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); > + dev->tcet = spapr_tce_new_table(qdev, liobn); > + spapr_tce_table_enable(dev->tcet, SPAPR_TCE_PAGE_SHIFT, 0, > + pc->rtce_window_size >> SPAPR_TCE_PAGE_SHIFT); > dev->tcet->vdev = dev; > memory_region_add_subregion_overlap(&dev->mrroot, 0, > spapr_tce_get_iommu(dev->tcet), 2); > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 098d85d..75b0b55 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -539,6 +539,7 @@ typedef struct sPAPRTCETable sPAPRTCETable; > > struct sPAPRTCETable { > DeviceState parent; > + bool enabled; > uint32_t liobn; > uint32_t nb_table; > uint64_t bus_offset; > @@ -566,11 +567,10 @@ void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq); > int spapr_h_cas_compose_response(sPAPRMachineState *sm, > target_ulong addr, target_ulong size, > bool cpu_update, bool memory_update); > -sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, > - uint64_t bus_offset, > - uint32_t page_shift, > - uint32_t nb_table, > - bool need_vfio); > +sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn); > +void spapr_tce_table_enable(sPAPRTCETable *tcet, > + uint32_t page_shift, uint64_t bus_offset, > + uint32_t nb_table); > void spapr_tce_set_need_vfio(sPAPRTCETable *tcet, bool need_vfio); > > MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet);
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c index 8132f64..9bcd3f6 100644 --- a/hw/ppc/spapr_iommu.c +++ b/hw/ppc/spapr_iommu.c @@ -17,6 +17,7 @@ * License along with this library; if not, see <http://www.gnu.org/licenses/>. */ #include "qemu/osdep.h" +#include "qemu/error-report.h" #include "hw/hw.h" #include "sysemu/kvm.h" #include "hw/qdev.h" @@ -174,15 +175,9 @@ static int spapr_tce_table_realize(DeviceState *dev) sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev); tcet->fd = -1; - tcet->table = spapr_tce_alloc_table(tcet->liobn, - tcet->page_shift, - tcet->nb_table, - &tcet->fd, - tcet->need_vfio); - + tcet->need_vfio = false; memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops, - "iommu-spapr", - (uint64_t)tcet->nb_table << tcet->page_shift); + "iommu-spapr", 0); QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list); @@ -224,14 +219,10 @@ void spapr_tce_set_need_vfio(sPAPRTCETable *tcet, bool need_vfio) tcet->table = newtable; } -sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, - uint64_t bus_offset, - uint32_t page_shift, - uint32_t nb_table, - bool need_vfio) +sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn) { sPAPRTCETable *tcet; - char tmp[64]; + char tmp[32]; if (spapr_tce_find_by_liobn(liobn)) { fprintf(stderr, "Attempted to create TCE table with duplicate" @@ -239,16 +230,8 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, return NULL; } - if (!nb_table) { - return NULL; - } - tcet = SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE)); tcet->liobn = liobn; - tcet->bus_offset = bus_offset; - tcet->page_shift = page_shift; - tcet->nb_table = nb_table; - tcet->need_vfio = need_vfio; snprintf(tmp, sizeof(tmp), "tce-table-%x", liobn); object_property_add_child(OBJECT(owner), tmp, OBJECT(tcet), NULL); @@ -258,14 +241,69 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, return tcet; } +static void spapr_tce_table_do_enable(sPAPRTCETable *tcet) +{ + if (!tcet->nb_table) { + return; + } + + tcet->table = spapr_tce_alloc_table(tcet->liobn, + tcet->page_shift, + tcet->nb_table, + &tcet->fd, + tcet->need_vfio); + + memory_region_set_size(&tcet->iommu, + (uint64_t)tcet->nb_table << tcet->page_shift); + + tcet->enabled = true; +} + +void spapr_tce_table_enable(sPAPRTCETable *tcet, + uint32_t page_shift, uint64_t bus_offset, + uint32_t nb_table) +{ + if (tcet->enabled) { + error_report("Warning: trying to enable already enabled TCE table"); + return; + } + + tcet->bus_offset = bus_offset; + tcet->page_shift = page_shift; + tcet->nb_table = nb_table; + + spapr_tce_table_do_enable(tcet); +} + +static void spapr_tce_table_do_disable(sPAPRTCETable *tcet) +{ + memory_region_set_size(&tcet->iommu, 0); + + spapr_tce_free_table(tcet->table, tcet->fd, tcet->nb_table); + tcet->fd = -1; + tcet->table = NULL; + tcet->enabled = false; + tcet->bus_offset = 0; + tcet->page_shift = 0; + tcet->nb_table = 0; +} + +static void spapr_tce_table_disable(sPAPRTCETable *tcet) +{ + if (!tcet->enabled) { + error_report("Warning: trying to disable already disabled TCE table"); + return; + } + spapr_tce_table_do_disable(tcet); +} + static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp) { sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev); QLIST_REMOVE(tcet, list); - spapr_tce_free_table(tcet->table, tcet->fd, tcet->nb_table); - tcet->fd = -1; + spapr_tce_table_disable(tcet); } MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 18332bf..df5f7b9 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -818,14 +818,15 @@ static void spapr_phb_dma_window_enable(sPAPRPHBState *sphb, return; } - tcet = spapr_tce_new_table(DEVICE(sphb), liobn, window_addr, - page_shift, nb_table, false); + tcet = spapr_tce_find_by_liobn(liobn); if (!tcet) { error_setg(errp, "Unable to create TCE table liobn %x for %s", liobn, sphb->dtbusname); return; } + spapr_tce_table_enable(tcet, page_shift, window_addr, nb_table); + memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset, spapr_tce_get_iommu(tcet)); } @@ -1335,6 +1336,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) PCIBus *bus; uint64_t msi_window_size = 4096; Error *local_err = NULL; + sPAPRTCETable *tcet; if (sphb->index != (uint32_t)-1) { hwaddr windows_base; @@ -1486,6 +1488,13 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) } } + /* DMA setup */ + tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn); + if (!tcet) { + error_report("No default TCE table for %s", sphb->dtbusname); + return; + } + /* Register default 32bit DMA window */ spapr_phb_dma_window_enable(sphb, sphb->dma_liobn, SPAPR_TCE_PAGE_SHIFT, sphb->dma_win_addr, sphb->dma_win_size, diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c index 0f61a55..7f57290 100644 --- a/hw/ppc/spapr_vio.c +++ b/hw/ppc/spapr_vio.c @@ -481,11 +481,9 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp) 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); + dev->tcet = spapr_tce_new_table(qdev, liobn); + spapr_tce_table_enable(dev->tcet, SPAPR_TCE_PAGE_SHIFT, 0, + pc->rtce_window_size >> SPAPR_TCE_PAGE_SHIFT); dev->tcet->vdev = dev; memory_region_add_subregion_overlap(&dev->mrroot, 0, spapr_tce_get_iommu(dev->tcet), 2); diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 098d85d..75b0b55 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -539,6 +539,7 @@ typedef struct sPAPRTCETable sPAPRTCETable; struct sPAPRTCETable { DeviceState parent; + bool enabled; uint32_t liobn; uint32_t nb_table; uint64_t bus_offset; @@ -566,11 +567,10 @@ void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq); int spapr_h_cas_compose_response(sPAPRMachineState *sm, target_ulong addr, target_ulong size, bool cpu_update, bool memory_update); -sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, - uint64_t bus_offset, - uint32_t page_shift, - uint32_t nb_table, - bool need_vfio); +sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn); +void spapr_tce_table_enable(sPAPRTCETable *tcet, + uint32_t page_shift, uint64_t bus_offset, + uint32_t nb_table); void spapr_tce_set_need_vfio(sPAPRTCETable *tcet, bool need_vfio); MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet);