Re: [Qemu-devel] [PATCH 08/17] pseries: savevm support for PAPR TCE tables

[Anthony Liguori <aliguori@us.ibm.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 08/07/2013 20:39, Anthony Liguori ha scritto:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>> 
>>> From: David Gibson <david@gibson.dropbear.id.au>
>>>
>>> This patch adds the necessary VMStateDescription information to save the
>>> state of PAPR TCE tables (that is, the PAPR specified IOMMU).
>>>
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>  hw/ppc/spapr_iommu.c |   25 +++++++++++++++++++++++++
>>>  1 file changed, 25 insertions(+)
>>>
>>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>>> index 91bc8e4..ba1f7b6 100644
>>> --- a/hw/ppc/spapr_iommu.c
>>> +++ b/hw/ppc/spapr_iommu.c
>>> @@ -112,6 +112,25 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr)
>>>      };
>>>  }
>>>  
>>> +static const VMStateDescription vmstate_spapr_tce_table = {
>>> +    .name = "spapr_iommu",
>>> +    .version_id = 1,
>>> +    .minimum_version_id = 1,
>>> +    .minimum_version_id_old = 1,
>>> +    .fields      = (VMStateField []) {
>>> +        /* Sanity check */
>>> +        VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable),
>>> +        VMSTATE_UINT32_EQUAL(window_size, sPAPRTCETable),
>>> +
>>> +        /* IOMMU state */
>>> +        VMSTATE_BOOL(bypass, sPAPRTCETable),
>>> +        VMSTATE_VBUFFER_DIVIDE(table, sPAPRTCETable, 0, NULL, 0, window_size,
>>> +                               SPAPR_TCE_PAGE_SIZE /
>>> sizeof(sPAPRTCE)),
>> 
>> Not endian safe.  I really don't get the divide bit at all either.
>> 
>>> +
>>> +        VMSTATE_END_OF_LIST()
>>> +    },
>>> +};
>>> +
>>>  static MemoryRegionIOMMUOps spapr_iommu_ops = {
>>>      .translate = spapr_tce_translate_iommu,
>>>  };
>>> @@ -156,6 +175,8 @@ sPAPRTCETable *spapr_tce_new_table(uint32_t liobn, size_t window_size)
>>>  
>>>      QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list);
>>>  
>>> +    vmstate_register(NULL, tcet->liobn, &vmstate_spapr_tce_table, tcet);
>>> +
>> 
>> If you need to add these, then you need to do more QOM conversion.
>
> No, this does not need QOM conversion.  It needs a sub-vmstate, that is
> then used by both the PCI and VIO bridges via VMSTATE_STRUCT.

I already QOM converted it and made it a sub-object.

I think that's better from a modeling point of view than using a
sub-vmstate.

Patches coming shortly.

Regards,

Anthony Liguori

>
> Paolo
>
>> Regards,
>> 
>> Anthony Liguori
>> 
>>>      return tcet;
>>>  }
>>>  
>>> @@ -163,6 +184,10 @@ void spapr_tce_free(sPAPRTCETable *tcet)
>>>  {
>>>      QLIST_REMOVE(tcet, list);
>>>  
>>> +    vmstate_unregister(NULL, &vmstate_spapr_tce_table, tcet);
>>> +
>>> +    QLIST_REMOVE(tcet, list);
>>> +
>>>      if (!kvm_enabled() ||
>>>          (kvmppc_remove_spapr_tce(tcet->table, tcet->fd,
>>>                                   tcet->window_size) != 0)) {
>>> -- 
>>> 1.7.10.4
>>