On Tue, Jul 09, 2013 at 10:22:39AM -0500, Anthony Liguori wrote:
> 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.
>
> Why? There are no guest visible byte accesses done to the table
> AFAICT. Everything is done as words and there's quite a lot of math
> done to the entries.
>
> It seems like native endian is the right internal representation.
Hrm. I suppose it could be fixed at either end. The idea was that
the table array would contain exactly the same bytes as would be
present in physical memory on a real bare-metal system, which seems
like a generally nice property.
--
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
pgpqnm_1xpK4y.pgp
Description: PGP signature
