On Tue, Apr 29, 2014 at 2:57 AM, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 15 April 2014 04:18, Peter Crosthwaite <peter.crosthwa...@xilinx.com> > wrote: >> Add support for 16, 32 and 64 bit width FIFOs. The push and pop >> functions are replicated to accept all four different integer types. >> The element width of the FIFO is set at creation time. >> >> The backing storage for all element types is still uint8_t regardless of >> element width so some save-load logic is needed to handle endianness >> issue WRT VMSD. > >> +/* Always store buffer data in little (arbitrarily chosen) endian format to >> + * allow for migration to/from BE <-> LE hosts. >> + */ >> + >> +static inline void fifo_save_load_swap(Fifo *fifo) >> +{ >> +#ifdef HOST_WORDS_BIGENDIAN >> + int i; >> + uint16_t *d16 = (uint16_t *)fifo->data; >> + uint32_t *d32 = (uint32_t *)fifo->data; >> + uint64_t *d64 = (uint64_t *)fifo->data; >> + >> + for (i = 0; i < fifo->capacity; ++i) { >> + switch (fifo->width) { >> + case 1: >> + return; >> + case 2: >> + d16[i] = bswap16(d16[i]); >> + break; >> + case 4: >> + d32[i] = bswap32(d32[i]); >> + break; >> + case 8: >> + d64[i] = bswap64(d64[i]); >> + break; >> + default: >> + abort(); >> + } >> + } >> +#endif >> +} >> + >> +static void fifo_pre_save(void *opaque) >> +{ >> + fifo_save_load_swap((Fifo *)opaque); >> +} >> + >> +static int fifo_post_load(void *opaque, int version_id) >> +{ >> + fifo_save_load_swap((Fifo *)opaque); >> + return 0; >> +} >> + >> const VMStateDescription vmstate_fifo = { >> .name = "Fifo8", >> .version_id = 1, >> .minimum_version_id = 1, >> .minimum_version_id_old = 1, >> + .pre_save = fifo_pre_save, >> + .post_load = fifo_post_load, >> .fields = (VMStateField[]) { >> - VMSTATE_VBUFFER_UINT32(data, Fifo, 1, NULL, 0, capacity), >> + VMSTATE_VBUFFER_UINT32(data, Fifo, 1, NULL, 0, buffer_size), >> VMSTATE_UINT32(head, Fifo), >> VMSTATE_UINT32(num, Fifo), >> VMSTATE_END_OF_LIST() > > This doesn't look right -- when the VM continues after > a save on a bigendian system all its FIFO data will > have been byteswapped and it'll fall over. >
Yep. My bad. > I think you need to implement this by adding get/put > functions which byteswap as they put the data onto > or take it off the wire. Check the use and definition > of vmstate_info_bitmap for an example of handling a > data structure where the on-the-wire and in-memory > formats differ. > So I am starting to think there a better way. Ultimately I want this API to work for random structs too, not just integer types (E.G. PL330 would be a nice client). So I'm thinking this problem is outsourced to the client who is responsible from bringing a VMStateInfo to the table (input to fifo_create). We then have some some wrappers for the simple integer types that trivally take the global vmstate_info_uintxx structs as appropriate: void fifo_create8(Fifo *fifo, uint32_t capacity) { fifo_create(fifo, capacity, sizeof(uint8_t), vmstate_info_uint8); } Most users will just just user createxx for an int fifo. But if you want a migratable struct Fifo you can do that too. From device land: fifo_create(&s->fifo, s->fifo_capacity, sizeof(MySpecialStruct), vmstate_my_special_struct); Work for you? Regards, Peter > thanks > -- PMM >