| Message ID | 1462344751-28281-11-git-send-email-aik@ozlabs.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 04, 2016 at 04:52:22PM +1000, Alexey Kardashevskiy wrote: > The source guest could have reallocated the default TCE table and > migrate bigger/smaller table. This adds reallocation in post_load() > if the default table size is different on source and destination. > > This adds @bus_offset, @page_shift, @enabled to the migration stream. > These cannot change without dynamic DMA windows so no change in > behavior is expected now. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > David Gibson <david@gibson.dropbear.id.au> > --- > Changes: > v15: > * squashed "migrate full state" into this > * added missing tcet->mig_nb_table initialization in spapr_tce_table_pre_save() > * instead of bumping the version, moved extra parameters to subsection > > v14: > * new to the series > --- > hw/ppc/spapr_iommu.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-- > include/hw/ppc/spapr.h | 2 ++ > trace-events | 2 ++ > 3 files changed, 69 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c > index 9bcd3f6..52b1e0d 100644 > --- a/hw/ppc/spapr_iommu.c > +++ b/hw/ppc/spapr_iommu.c > @@ -137,33 +137,96 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr, > return ret; > } > > +static void spapr_tce_table_pre_save(void *opaque) > +{ > + sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); > + > + tcet->mig_table = tcet->table; > + tcet->mig_nb_table = tcet->nb_table; > + > + trace_spapr_iommu_pre_save(tcet->liobn, tcet->mig_nb_table, > + tcet->bus_offset, tcet->page_shift); > +} > + > +static void spapr_tce_table_do_enable(sPAPRTCETable *tcet); > +static void spapr_tce_table_do_disable(sPAPRTCETable *tcet); > + > static int spapr_tce_table_post_load(void *opaque, int version_id) > { > sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); > + uint32_t old_nb_table = tcet->nb_table; > > if (tcet->vdev) { > spapr_vio_set_bypass(tcet->vdev, tcet->bypass); > } > > + if (tcet->enabled) { > + if (tcet->nb_table != tcet->mig_nb_table) { > + if (tcet->nb_table) { > + spapr_tce_table_do_disable(tcet); > + } > + tcet->nb_table = tcet->mig_nb_table; > + spapr_tce_table_do_enable(tcet); > + } > + > + memcpy(tcet->table, tcet->mig_table, > + tcet->nb_table * sizeof(tcet->table[0])); > + > + free(tcet->mig_table); > + tcet->mig_table = NULL; > + } else if (tcet->table) { > + /* Destination guest has a default table but source does not -> free */ > + spapr_tce_table_do_disable(tcet); > + } > + > + trace_spapr_iommu_post_load(tcet->liobn, old_nb_table, tcet->nb_table, > + tcet->bus_offset, tcet->page_shift); > + > return 0; > } > > +static bool spapr_tce_table_ex_needed(void *opaque) > +{ > + sPAPRTCETable *tcet = opaque; > + > + return tcet->bus_offset || tcet->page_shift != 0xC; || !tcet->enabled ?? AFAICT you're assuming that the existing tcet on the destination will be enabled prior to an incoming migration. > +} > + > +static const VMStateDescription vmstate_spapr_tce_table_ex = { > + .name = "spapr_iommu_ex", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = spapr_tce_table_ex_needed, > + .fields = (VMStateField[]) { > + VMSTATE_BOOL(enabled, sPAPRTCETable), ..or could you encode enabled as !!mig_nb_table? > + VMSTATE_UINT64(bus_offset, sPAPRTCETable), > + VMSTATE_UINT32(page_shift, sPAPRTCETable), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > static const VMStateDescription vmstate_spapr_tce_table = { > .name = "spapr_iommu", > .version_id = 2, > .minimum_version_id = 2, > + .pre_save = spapr_tce_table_pre_save, > .post_load = spapr_tce_table_post_load, > .fields = (VMStateField []) { > /* Sanity check */ > VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable), > - VMSTATE_UINT32_EQUAL(nb_table, sPAPRTCETable), > > /* IOMMU state */ > + VMSTATE_UINT32(mig_nb_table, sPAPRTCETable), > VMSTATE_BOOL(bypass, sPAPRTCETable), > - VMSTATE_VARRAY_UINT32(table, sPAPRTCETable, nb_table, 0, vmstate_info_uint64, uint64_t), > + VMSTATE_VARRAY_UINT32_ALLOC(mig_table, sPAPRTCETable, mig_nb_table, 0, > + vmstate_info_uint64, uint64_t), > > VMSTATE_END_OF_LIST() > }, > + .subsections = (const VMStateDescription*[]) { > + &vmstate_spapr_tce_table_ex, > + NULL > + } > }; > > static MemoryRegionIOMMUOps spapr_iommu_ops = { > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 0140810..d36dda2 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -540,6 +540,8 @@ struct sPAPRTCETable { > uint64_t bus_offset; > uint32_t page_shift; > uint64_t *table; > + uint32_t mig_nb_table; > + uint64_t *mig_table; > bool bypass; > bool need_vfio; > int fd; > diff --git a/trace-events b/trace-events > index d96d344..dd50005 100644 > --- a/trace-events > +++ b/trace-events > @@ -1432,6 +1432,8 @@ spapr_iommu_pci_indirect(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t i > spapr_iommu_pci_stuff(uint64_t liobn, uint64_t ioba, uint64_t tce_value, uint64_t npages, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tcevalue=0x%"PRIx64" npages=%"PRId64" ret=%"PRId64 > spapr_iommu_xlate(uint64_t liobn, uint64_t ioba, uint64_t tce, unsigned perm, unsigned pgsize) "liobn=%"PRIx64" 0x%"PRIx64" -> 0x%"PRIx64" perm=%u mask=%x" > spapr_iommu_new_table(uint64_t liobn, void *table, int fd) "liobn=%"PRIx64" table=%p fd=%d" > +spapr_iommu_pre_save(uint64_t liobn, uint32_t nb, uint64_t offs, uint32_t ps) "liobn=%"PRIx64" %"PRIx32" bus_offset=%"PRIx64" ps=%"PRIu32 > +spapr_iommu_post_load(uint64_t liobn, uint32_t pre_nb, uint32_t post_nb, uint64_t offs, uint32_t ps) "liobn=%"PRIx64" %"PRIx32" => %"PRIx32" bus_offset=%"PRIx64" ps=%"PRIu32 > > # hw/ppc/ppc.c > ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
On 26/05/16 14:01, David Gibson wrote: > On Wed, May 04, 2016 at 04:52:22PM +1000, Alexey Kardashevskiy wrote: >> The source guest could have reallocated the default TCE table and >> migrate bigger/smaller table. This adds reallocation in post_load() >> if the default table size is different on source and destination. >> >> This adds @bus_offset, @page_shift, @enabled to the migration stream. >> These cannot change without dynamic DMA windows so no change in >> behavior is expected now. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> David Gibson <david@gibson.dropbear.id.au> >> --- >> Changes: >> v15: >> * squashed "migrate full state" into this >> * added missing tcet->mig_nb_table initialization in spapr_tce_table_pre_save() >> * instead of bumping the version, moved extra parameters to subsection >> >> v14: >> * new to the series >> --- >> hw/ppc/spapr_iommu.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-- >> include/hw/ppc/spapr.h | 2 ++ >> trace-events | 2 ++ >> 3 files changed, 69 insertions(+), 2 deletions(-) >> >> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c >> index 9bcd3f6..52b1e0d 100644 >> --- a/hw/ppc/spapr_iommu.c >> +++ b/hw/ppc/spapr_iommu.c >> @@ -137,33 +137,96 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr, >> return ret; >> } >> >> +static void spapr_tce_table_pre_save(void *opaque) >> +{ >> + sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); >> + >> + tcet->mig_table = tcet->table; >> + tcet->mig_nb_table = tcet->nb_table; >> + >> + trace_spapr_iommu_pre_save(tcet->liobn, tcet->mig_nb_table, >> + tcet->bus_offset, tcet->page_shift); >> +} >> + >> +static void spapr_tce_table_do_enable(sPAPRTCETable *tcet); >> +static void spapr_tce_table_do_disable(sPAPRTCETable *tcet); >> + >> static int spapr_tce_table_post_load(void *opaque, int version_id) >> { >> sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); >> + uint32_t old_nb_table = tcet->nb_table; >> >> if (tcet->vdev) { >> spapr_vio_set_bypass(tcet->vdev, tcet->bypass); >> } >> >> + if (tcet->enabled) { >> + if (tcet->nb_table != tcet->mig_nb_table) { >> + if (tcet->nb_table) { >> + spapr_tce_table_do_disable(tcet); >> + } >> + tcet->nb_table = tcet->mig_nb_table; >> + spapr_tce_table_do_enable(tcet); >> + } >> + >> + memcpy(tcet->table, tcet->mig_table, >> + tcet->nb_table * sizeof(tcet->table[0])); >> + >> + free(tcet->mig_table); >> + tcet->mig_table = NULL; >> + } else if (tcet->table) { >> + /* Destination guest has a default table but source does not -> free */ >> + spapr_tce_table_do_disable(tcet); >> + } >> + >> + trace_spapr_iommu_post_load(tcet->liobn, old_nb_table, tcet->nb_table, >> + tcet->bus_offset, tcet->page_shift); >> + >> return 0; >> } >> >> +static bool spapr_tce_table_ex_needed(void *opaque) >> +{ >> + sPAPRTCETable *tcet = opaque; >> + >> + return tcet->bus_offset || tcet->page_shift != 0xC; > > || !tcet->enabled ?? > > AFAICT you're assuming that the existing tcet on the destination will > be enabled prior to an incoming migration. > >> +} >> + >> +static const VMStateDescription vmstate_spapr_tce_table_ex = { >> + .name = "spapr_iommu_ex", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .needed = spapr_tce_table_ex_needed, >> + .fields = (VMStateField[]) { >> + VMSTATE_BOOL(enabled, sPAPRTCETable), > > ..or could you encode enabled as !!mig_nb_table? Sure. After a closer look, I can get rid of "enabled" field at all and use nb_table instead.
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c index 9bcd3f6..52b1e0d 100644 --- a/hw/ppc/spapr_iommu.c +++ b/hw/ppc/spapr_iommu.c @@ -137,33 +137,96 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr, return ret; } +static void spapr_tce_table_pre_save(void *opaque) +{ + sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); + + tcet->mig_table = tcet->table; + tcet->mig_nb_table = tcet->nb_table; + + trace_spapr_iommu_pre_save(tcet->liobn, tcet->mig_nb_table, + tcet->bus_offset, tcet->page_shift); +} + +static void spapr_tce_table_do_enable(sPAPRTCETable *tcet); +static void spapr_tce_table_do_disable(sPAPRTCETable *tcet); + static int spapr_tce_table_post_load(void *opaque, int version_id) { sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); + uint32_t old_nb_table = tcet->nb_table; if (tcet->vdev) { spapr_vio_set_bypass(tcet->vdev, tcet->bypass); } + if (tcet->enabled) { + if (tcet->nb_table != tcet->mig_nb_table) { + if (tcet->nb_table) { + spapr_tce_table_do_disable(tcet); + } + tcet->nb_table = tcet->mig_nb_table; + spapr_tce_table_do_enable(tcet); + } + + memcpy(tcet->table, tcet->mig_table, + tcet->nb_table * sizeof(tcet->table[0])); + + free(tcet->mig_table); + tcet->mig_table = NULL; + } else if (tcet->table) { + /* Destination guest has a default table but source does not -> free */ + spapr_tce_table_do_disable(tcet); + } + + trace_spapr_iommu_post_load(tcet->liobn, old_nb_table, tcet->nb_table, + tcet->bus_offset, tcet->page_shift); + return 0; } +static bool spapr_tce_table_ex_needed(void *opaque) +{ + sPAPRTCETable *tcet = opaque; + + return tcet->bus_offset || tcet->page_shift != 0xC; +} + +static const VMStateDescription vmstate_spapr_tce_table_ex = { + .name = "spapr_iommu_ex", + .version_id = 1, + .minimum_version_id = 1, + .needed = spapr_tce_table_ex_needed, + .fields = (VMStateField[]) { + VMSTATE_BOOL(enabled, sPAPRTCETable), + VMSTATE_UINT64(bus_offset, sPAPRTCETable), + VMSTATE_UINT32(page_shift, sPAPRTCETable), + VMSTATE_END_OF_LIST() + }, +}; + static const VMStateDescription vmstate_spapr_tce_table = { .name = "spapr_iommu", .version_id = 2, .minimum_version_id = 2, + .pre_save = spapr_tce_table_pre_save, .post_load = spapr_tce_table_post_load, .fields = (VMStateField []) { /* Sanity check */ VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable), - VMSTATE_UINT32_EQUAL(nb_table, sPAPRTCETable), /* IOMMU state */ + VMSTATE_UINT32(mig_nb_table, sPAPRTCETable), VMSTATE_BOOL(bypass, sPAPRTCETable), - VMSTATE_VARRAY_UINT32(table, sPAPRTCETable, nb_table, 0, vmstate_info_uint64, uint64_t), + VMSTATE_VARRAY_UINT32_ALLOC(mig_table, sPAPRTCETable, mig_nb_table, 0, + vmstate_info_uint64, uint64_t), VMSTATE_END_OF_LIST() }, + .subsections = (const VMStateDescription*[]) { + &vmstate_spapr_tce_table_ex, + NULL + } }; static MemoryRegionIOMMUOps spapr_iommu_ops = { diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 0140810..d36dda2 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -540,6 +540,8 @@ struct sPAPRTCETable { uint64_t bus_offset; uint32_t page_shift; uint64_t *table; + uint32_t mig_nb_table; + uint64_t *mig_table; bool bypass; bool need_vfio; int fd; diff --git a/trace-events b/trace-events index d96d344..dd50005 100644 --- a/trace-events +++ b/trace-events @@ -1432,6 +1432,8 @@ spapr_iommu_pci_indirect(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t i spapr_iommu_pci_stuff(uint64_t liobn, uint64_t ioba, uint64_t tce_value, uint64_t npages, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tcevalue=0x%"PRIx64" npages=%"PRId64" ret=%"PRId64 spapr_iommu_xlate(uint64_t liobn, uint64_t ioba, uint64_t tce, unsigned perm, unsigned pgsize) "liobn=%"PRIx64" 0x%"PRIx64" -> 0x%"PRIx64" perm=%u mask=%x" spapr_iommu_new_table(uint64_t liobn, void *table, int fd) "liobn=%"PRIx64" table=%p fd=%d" +spapr_iommu_pre_save(uint64_t liobn, uint32_t nb, uint64_t offs, uint32_t ps) "liobn=%"PRIx64" %"PRIx32" bus_offset=%"PRIx64" ps=%"PRIu32 +spapr_iommu_post_load(uint64_t liobn, uint32_t pre_nb, uint32_t post_nb, uint64_t offs, uint32_t ps) "liobn=%"PRIx64" %"PRIx32" => %"PRIx32" bus_offset=%"PRIx64" ps=%"PRIu32 # hw/ppc/ppc.c ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
The source guest could have reallocated the default TCE table and migrate bigger/smaller table. This adds reallocation in post_load() if the default table size is different on source and destination. This adds @bus_offset, @page_shift, @enabled to the migration stream. These cannot change without dynamic DMA windows so no change in behavior is expected now. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> David Gibson <david@gibson.dropbear.id.au> --- Changes: v15: * squashed "migrate full state" into this * added missing tcet->mig_nb_table initialization in spapr_tce_table_pre_save() * instead of bumping the version, moved extra parameters to subsection v14: * new to the series --- hw/ppc/spapr_iommu.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-- include/hw/ppc/spapr.h | 2 ++ trace-events | 2 ++ 3 files changed, 69 insertions(+), 2 deletions(-)