[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 2/4] util/fifo: Generalise for common integer
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH v4 2/4] util/fifo: Generalise for common integer widths |
Date: |
Tue, 6 May 2014 11:41:15 +1000 |
On Tue, Apr 29, 2014 at 2:57 AM, Peter Maydell <address@hidden> wrote:
> On 15 April 2014 04:18, Peter Crosthwaite <address@hidden> 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
>
- Re: [Qemu-devel] [PATCH v4 2/4] util/fifo: Generalise for common integer widths,
Peter Crosthwaite <=