Benjamin Herrenschmidt <b...@kernel.crashing.org> writes:
> On Mon, 2013-07-08 at 13:39 -0500, Anthony Liguori wrote:
>> > + .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.
>
> What do you mean by not endian safe ? The TCE table is a well defined format,
> it's always big endian regardless of the endianness of either host or
> guest.
VMSTATE_VBUFFER is essentially:
write(fd, s->table, byte_size_of_table);
It treats whatever is given it as a sized data blob.
table is an array of sPAPRTCE which is just a struct wrapper around a
uint64_t value (the tce entry).
Those entries are set via the h_put_tce hcall through a simple
assignment:
> static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
> target_ulong tce)
> {
> ...
>
> tcep = tcet->table + (ioba >> SPAPR_TCE_PAGE_SHIFT);
> tcep->tce = tce;
> ...
>
> static target_ulong h_put_tce(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> target_ulong opcode, target_ulong *args)
> {
> ...
> target_ulong tce = args[2];
> sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
>
> ...
>
> if (tcet) {
> return put_tce_emu(tcet, ioba, tce);
> }
Hypercall arguments are passed in CPU endianness so what's being stored
in the tce table is CPU endianness.
Since VBUFFER just does a blind write() of the full array of uint64s,
what goes on the wire will be CPU endianness.
So if you do a savevm on a little endian host and loadvm on a big endian
host, badness ensues.
The proper thing to do is use a VARRAY instead of a VBUFFER. VARRAY
will handle endian because it treats the data as an array, not as an
opaque buffer.
Regards,
Anthony Liguori
>
> Cheers,
> Ben.
