David Gibson <da...@gibson.dropbear.id.au> writes:
> On Mon, Jul 08, 2013 at 01:39:26PM -0500, Anthony Liguori wrote:
>> Alexey Kardashevskiy <a...@ozlabs.ru> writes:
>>
>> > From: David Gibson <da...@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 <da...@gibson.dropbear.id.au>
>> > Signed-off-by: Alexey Kardashevskiy <a...@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.
>
> So, the actual bug is that we're currently storing the TCE table
> native endian, whereas it should be stored big endan always.
>
>> > +
>> > + 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.
>
> Again, it's not clear how this should be QOMed. Child of the device
> constructing the TCE table? But since that can often be a bus bridge,
> wouldn't the TCE table instances get confused with the real bus
> devices.
Only build tested.
https://github.com/aliguori/qemu/commit/a47a391c875a69f203110811c730877da12f5b14
I'll put together a patch series once I have a chance to test properly.
Regards,
Anthony Liguori
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_
> _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
