On Mon, Apr 07, 2014 at 07:05:18PM -0700, Peter Crosthwaite wrote:
> Add support for 16, 32 and 64 bit width FIFOs. The push and pop
> functions are patched to accept uint64_t always to support up to 64bit
> integer elements. The element width 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.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
> ---
>
> hw/char/serial.c | 4 +-
> hw/net/allwinner_emac.c | 6 +--
> hw/ssi/xilinx_spi.c | 4 +-
> hw/ssi/xilinx_spips.c | 4 +-
> include/qemu/fifo.h | 19 +++++----
> util/fifo.c | 109
> +++++++++++++++++++++++++++++++++++++++++-------
> 6 files changed, 114 insertions(+), 32 deletions(-)
>
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index a0d8024..3060769 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -659,8 +659,8 @@ void serial_realize_core(SerialState *s, Error **errp)
>
> qemu_chr_add_handlers(s->chr, serial_can_receive1, serial_receive1,
> serial_event, s);
> - fifo_create(&s->recv_fifo, UART_FIFO_LENGTH);
> - fifo_create(&s->xmit_fifo, UART_FIFO_LENGTH);
> + fifo_create(&s->recv_fifo, UART_FIFO_LENGTH, 8);
> + fifo_create(&s->xmit_fifo, UART_FIFO_LENGTH, 8);
> }
>
> void serial_exit_core(SerialState *s)
> diff --git a/hw/net/allwinner_emac.c b/hw/net/allwinner_emac.c
> index e804411..2a0d2ac 100644
> --- a/hw/net/allwinner_emac.c
> +++ b/hw/net/allwinner_emac.c
> @@ -455,9 +455,9 @@ static void aw_emac_realize(DeviceState *dev, Error
> **errp)
> object_get_typename(OBJECT(dev)), dev->id, s);
> qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
>
> - fifo_create(&s->rx_fifo, RX_FIFO_SIZE);
> - fifo_create(&s->tx_fifo[0], TX_FIFO_SIZE);
> - fifo_create(&s->tx_fifo[1], TX_FIFO_SIZE);
> + fifo_create(&s->rx_fifo, RX_FIFO_SIZE, 8);
> + fifo_create(&s->tx_fifo[0], TX_FIFO_SIZE, 8);
> + fifo_create(&s->tx_fifo[1], TX_FIFO_SIZE, 8);
> }
>
> static Property aw_emac_properties[] = {
> diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
> index 8fe3072..cac666b 100644
> --- a/hw/ssi/xilinx_spi.c
> +++ b/hw/ssi/xilinx_spi.c
> @@ -341,8 +341,8 @@ static int xilinx_spi_init(SysBusDevice *sbd)
>
> s->irqline = -1;
>
> - fifo_create(&s->tx_fifo, FIFO_CAPACITY);
> - fifo_create(&s->rx_fifo, FIFO_CAPACITY);
> + fifo_create(&s->tx_fifo, FIFO_CAPACITY, 8);
> + fifo_create(&s->rx_fifo, FIFO_CAPACITY, 8);
>
> return 0;
> }
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index d7d4c41..a7a639f 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -669,8 +669,8 @@ static void xilinx_spips_realize(DeviceState *dev, Error
> **errp)
>
> s->irqline = -1;
>
> - fifo_create(&s->rx_fifo, xsc->rx_fifo_size);
> - fifo_create(&s->tx_fifo, xsc->tx_fifo_size);
> + fifo_create(&s->rx_fifo, xsc->rx_fifo_size, 8);
> + fifo_create(&s->tx_fifo, xsc->tx_fifo_size, 8);
> }
>
> static void xilinx_qspips_realize(DeviceState *dev, Error **errp)
> diff --git a/include/qemu/fifo.h b/include/qemu/fifo.h
> index 44766b3..8738027 100644
> --- a/include/qemu/fifo.h
> +++ b/include/qemu/fifo.h
> @@ -5,8 +5,12 @@
>
> typedef struct {
> /* All fields are private */
> + int width; /* byte width each each element */
of each element
> + uint32_t capacity; /* number of elements */
> +
> uint8_t *data;
> - uint32_t capacity;
> + uint32_t buffer_size;
> +
> uint32_t head;
> uint32_t num;
> } Fifo;
> @@ -14,13 +18,14 @@ typedef struct {
> /**
> * fifo_create:
> * @fifo: struct Fifo to initialise with new FIFO
> - * @capacity: capacity of the newly created FIFO
> + * @capacity: capacity (number of elements) of the newly created FIFO
> + * @width: integer width of each element. Must be 8, 16, 32 or 64.
> *
> * Create a FIFO of the specified size. Clients should call fifo_destroy()
> * when finished using the fifo. The FIFO is initially empty.
> */
>
> -void fifo_create(Fifo *fifo, uint32_t capacity);
> +void fifo_create(Fifo *fifo, uint32_t capacity, int width);
>
> /**
> * fifo_destroy:
> @@ -41,7 +46,7 @@ void fifo_destroy(Fifo *fifo);
> * Clients are responsible for checking for fullness using fifo_is_full().
> */
>
> -void fifo_push(Fifo *fifo, uint8_t data);
> +void fifo_push(Fifo *fifo, uint64_t data);
>
> /**
> * fifo_push_all:
> @@ -54,7 +59,7 @@ void fifo_push(Fifo *fifo, uint8_t data);
> * fifo_num_free().
> */
>
> -void fifo_push_all(Fifo *fifo, const uint8_t *data, uint32_t num);
> +void fifo_push_all(Fifo *fifo, const void *data, uint32_t num);
>
> /**
> * fifo_pop:
> @@ -66,7 +71,7 @@ void fifo_push_all(Fifo *fifo, const uint8_t *data,
> uint32_t num);
> * Returns: The popped data value.
> */
>
> -uint8_t fifo_pop(Fifo *fifo);
> +uint64_t fifo_pop(Fifo *fifo);
>
> /**
> * fifo_pop_buf:
> @@ -93,7 +98,7 @@ uint8_t fifo_pop(Fifo *fifo);
> * Returns: A pointer to popped data.
> */
>
> -const uint8_t *fifo_pop_buf(Fifo *fifo, uint32_t max, uint32_t *num);
> +const void *fifo_pop_buf(Fifo *fifo, uint32_t max, uint32_t *num);
>
> /**
> * fifo_reset:
> diff --git a/util/fifo.c b/util/fifo.c
> index ffadf55..51318ad 100644
> --- a/util/fifo.c
> +++ b/util/fifo.c
> @@ -15,9 +15,11 @@
> #include "qemu-common.h"
> #include "qemu/fifo.h"
>
> -void fifo_create(Fifo *fifo, uint32_t capacity)
> +void fifo_create(Fifo *fifo, uint32_t capacity, int width)
> {
> - fifo->data = g_new(uint8_t, capacity);
> + assert(width == 8 || width == 16 || width == 32 || width == 64);
> + fifo->width = width / 8;
> + fifo->data = g_new(uint8_t, capacity * fifo->width);
> fifo->capacity = capacity;
> fifo->head = 0;
> fifo->num = 0;
> @@ -28,16 +30,33 @@ void fifo_destroy(Fifo *fifo)
> g_free(fifo->data);
> }
>
> -void fifo_push(Fifo *fifo, uint8_t data)
> +void fifo_push(Fifo *fifo, uint64_t data)
> {
> + uint32_t next_idx = (fifo->head + fifo->num) % fifo->capacity;
> +
> if (fifo->num == fifo->capacity) {
> abort();
> }
> - fifo->data[(fifo->head + fifo->num) % fifo->capacity] = data;
> + switch (fifo->width) {
> + case(1):
To me 'case 1:' looks better, but it's only a personal taste :)
> + ((uint8_t *)fifo->data)[next_idx] = data;
> + break;
> + case(2):
> + ((uint16_t *)fifo->data)[next_idx] = data;
> + break;
> + case(4):
> + ((uint32_t *)fifo->data)[next_idx] = data;
> + break;
> + case(8):
> + ((uint64_t *)fifo->data)[next_idx] = data;
> + break;
> + default:
> + abort();
> + }
> fifo->num++;
> }
>
> -void fifo_push_all(Fifo *fifo, const uint8_t *data, uint32_t num)
> +void fifo_push_all(Fifo *fifo, const void *data, uint32_t num)
> {
> uint32_t start, avail;
>
> @@ -48,38 +67,51 @@ void fifo_push_all(Fifo *fifo, const uint8_t *data,
> uint32_t num)
> start = (fifo->head + fifo->num) % fifo->capacity;
>
> if (start + num <= fifo->capacity) {
> - memcpy(&fifo->data[start], data, num);
> + memcpy(&fifo->data[start * fifo->width], data, num * fifo->width);
> } else {
> avail = fifo->capacity - start;
> - memcpy(&fifo->data[start], data, avail);
> - memcpy(&fifo->data[0], &data[avail], num - avail);
> + memcpy(&fifo->data[start * fifo->width], data, avail * fifo->width);
> + memcpy(&fifo->data[0], data + avail * fifo->width,
> + (num - avail) * fifo->width);
> }
>
> fifo->num += num;
> }
>
> -uint8_t fifo_pop(Fifo *fifo)
> +uint64_t fifo_pop(Fifo *fifo)
> {
> - uint8_t ret;
> + uint32_t next_idx;
>
> if (fifo->num == 0) {
> abort();
> }
> - ret = fifo->data[fifo->head++];
> + next_idx = fifo->head++;
> fifo->head %= fifo->capacity;
> fifo->num--;
> - return ret;
> + switch (fifo->width) {
> + case(1):
> + return ((uint8_t *)fifo->data)[next_idx];
> + case(2):
> + return ((uint16_t *)fifo->data)[next_idx];
> + case(4):
> + return ((uint32_t *)fifo->data)[next_idx];
> + case(8):
> + return ((uint64_t *)fifo->data)[next_idx];
> + default:
> + abort();
> + return 0; /* unreachable */
> + }
> }
>
> -const uint8_t *fifo_pop_buf(Fifo *fifo, uint32_t max, uint32_t *num)
> +const void *fifo_pop_buf(Fifo *fifo, uint32_t max, uint32_t *num)
> {
> - uint8_t *ret;
> + void *ret;
>
> if (max == 0 || max > fifo->num) {
> abort();
> }
> *num = MIN(fifo->capacity - fifo->head, max);
> - ret = &fifo->data[fifo->head];
> + ret = &fifo->data[fifo->head * fifo->width];
> fifo->head += *num;
> fifo->head %= fifo->capacity;
> fifo->num -= *num;
> @@ -112,13 +144,58 @@ uint32_t fifo_num_used(Fifo *fifo)
> return fifo->num;
> }
>
> +/* 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()
> --
> 1.9.1.1.gbb9f595
I'm not familiar with vmsd save/load code, but the rest of the patch
seems good to me.
Beniamino
