Re: [Qemu-devel] [PATCH qemu v16 10/19] spapr_iommu: Migrate full state

[Alexey Kardashevskiy]

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 <address@hidden>
>> David Gibson <address@hidden>
>> ---
>> 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.



-- 
Alexey

signature.asc
Description: OpenPGP digital signature