All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types
@ 2014-04-08  2:04 Peter Crosthwaite
  2014-04-08  2:04 ` [Qemu-devel] [PATCH for-2.1 v2 1/2] util/fifo: s/fifo8/fifo globally Peter Crosthwaite
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Peter Crosthwaite @ 2014-04-08  2:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: b.galvani, dslutz


There is a utility helper for dealing with 8 bit fifos. This should be
applicable to other integer widths as well. These two patches
generalise this FIFO to work for 16, 32 and 64 bit ints.

CC some recent contributors around this code.

changed since v1:
Rebased to include fifo buffer functionality.


Peter Crosthwaite (2):
  util/fifo: s/fifo8/fifo globally
  util/fifo: Generalise for common integer widths

 hw/char/serial.c                |  30 +++---
 hw/net/allwinner_emac.c         |  72 +++++++-------
 hw/ssi/xilinx_spi.c             |  42 ++++-----
 hw/ssi/xilinx_spips.c           |  70 +++++++-------
 include/hw/char/serial.h        |   6 +-
 include/hw/net/allwinner_emac.h |   6 +-
 include/qemu/fifo.h             | 166 ++++++++++++++++++++++++++++++++
 include/qemu/fifo8.h            | 160 -------------------------------
 util/Makefile.objs              |   2 +-
 util/fifo.c                     | 203 ++++++++++++++++++++++++++++++++++++++++
 util/fifo8.c                    | 126 -------------------------
 11 files changed, 483 insertions(+), 400 deletions(-)
 create mode 100644 include/qemu/fifo.h
 delete mode 100644 include/qemu/fifo8.h
 create mode 100644 util/fifo.c
 delete mode 100644 util/fifo8.c

-- 
1.9.1.1.gbb9f595

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH for-2.1 v2 1/2] util/fifo: s/fifo8/fifo globally
  2014-04-08  2:04 [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types Peter Crosthwaite
@ 2014-04-08  2:04 ` Peter Crosthwaite
  2014-04-08 18:48   ` Beniamino Galvani
  2014-04-08  2:05 ` [Qemu-devel] [PATCH for-2.1 v2 2/2] util/fifo: Generalise for common integer widths Peter Crosthwaite
  2014-04-08  7:14 ` [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types Markus Armbruster
  2 siblings, 1 reply; 12+ messages in thread
From: Peter Crosthwaite @ 2014-04-08  2:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: b.galvani, dslutz

This prepares support for generalising FIFO support to more integer
widths.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/char/serial.c                 | 30 ++++++-------
 hw/net/allwinner_emac.c          | 72 +++++++++++++++---------------
 hw/ssi/xilinx_spi.c              | 42 +++++++++---------
 hw/ssi/xilinx_spips.c            | 70 ++++++++++++++---------------
 include/hw/char/serial.h         |  6 +--
 include/hw/net/allwinner_emac.h  |  6 +--
 include/qemu/{fifo8.h => fifo.h} | 95 ++++++++++++++++++++--------------------
 util/Makefile.objs               |  2 +-
 util/{fifo8.c => fifo.c}         | 32 +++++++-------
 9 files changed, 178 insertions(+), 177 deletions(-)
 rename include/qemu/{fifo8.h => fifo.h} (53%)
 rename util/{fifo8.c => fifo.c} (75%)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index f4d167f..a0d8024 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -108,8 +108,8 @@ static void serial_receive1(void *opaque, const uint8_t *buf, int size);
 static inline void recv_fifo_put(SerialState *s, uint8_t chr)
 {
     /* Receive overruns do not overwrite FIFO contents. */
-    if (!fifo8_is_full(&s->recv_fifo)) {
-        fifo8_push(&s->recv_fifo, chr);
+    if (!fifo_is_full(&s->recv_fifo)) {
+        fifo_push(&s->recv_fifo, chr);
     } else {
         s->lsr |= UART_LSR_OE;
     }
@@ -225,10 +225,10 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
 
     if (s->tsr_retry <= 0) {
         if (s->fcr & UART_FCR_FE) {
-            if (fifo8_is_empty(&s->xmit_fifo)) {
+            if (fifo_is_empty(&s->xmit_fifo)) {
                 return FALSE;
             }
-            s->tsr = fifo8_pop(&s->xmit_fifo);
+            s->tsr = fifo_pop(&s->xmit_fifo);
             if (!s->xmit_fifo.num) {
                 s->lsr |= UART_LSR_THRE;
             }
@@ -284,10 +284,10 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
             s->thr = (uint8_t) val;
             if(s->fcr & UART_FCR_FE) {
                 /* xmit overruns overwrite data, so make space if needed */
-                if (fifo8_is_full(&s->xmit_fifo)) {
-                    fifo8_pop(&s->xmit_fifo);
+                if (fifo_is_full(&s->xmit_fifo)) {
+                    fifo_pop(&s->xmit_fifo);
                 }
-                fifo8_push(&s->xmit_fifo, s->thr);
+                fifo_push(&s->xmit_fifo, s->thr);
                 s->lsr &= ~UART_LSR_TEMT;
             }
             s->thr_ipending = 0;
@@ -334,11 +334,11 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
         if (val & UART_FCR_RFR) {
             timer_del(s->fifo_timeout_timer);
             s->timeout_ipending=0;
-            fifo8_reset(&s->recv_fifo);
+            fifo_reset(&s->recv_fifo);
         }
 
         if (val & UART_FCR_XFR) {
-            fifo8_reset(&s->xmit_fifo);
+            fifo_reset(&s->xmit_fifo);
         }
 
         if (val & UART_FCR_FE) {
@@ -427,8 +427,8 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
             ret = s->divider & 0xff;
         } else {
             if(s->fcr & UART_FCR_FE) {
-                ret = fifo8_is_empty(&s->recv_fifo) ?
-                            0 : fifo8_pop(&s->recv_fifo);
+                ret = fifo_is_empty(&s->recv_fifo) ?
+                            0 : fifo_pop(&s->recv_fifo);
                 if (s->recv_fifo.num == 0) {
                     s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
                 } else {
@@ -635,8 +635,8 @@ static void serial_reset(void *opaque)
     s->char_transmit_time = (get_ticks_per_sec() / 9600) * 10;
     s->poll_msl = 0;
 
-    fifo8_reset(&s->recv_fifo);
-    fifo8_reset(&s->xmit_fifo);
+    fifo_reset(&s->recv_fifo);
+    fifo_reset(&s->xmit_fifo);
 
     s->last_xmit_ts = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
@@ -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);
-    fifo8_create(&s->recv_fifo, UART_FIFO_LENGTH);
-    fifo8_create(&s->xmit_fifo, UART_FIFO_LENGTH);
+    fifo_create(&s->recv_fifo, UART_FIFO_LENGTH);
+    fifo_create(&s->xmit_fifo, UART_FIFO_LENGTH);
 }
 
 void serial_exit_core(SerialState *s)
diff --git a/hw/net/allwinner_emac.c b/hw/net/allwinner_emac.c
index 469f2f0..e804411 100644
--- a/hw/net/allwinner_emac.c
+++ b/hw/net/allwinner_emac.c
@@ -18,7 +18,7 @@
  */
 #include "hw/sysbus.h"
 #include "net/net.h"
-#include "qemu/fifo8.h"
+#include "qemu/fifo.h"
 #include "hw/net/allwinner_emac.h"
 #include <zlib.h>
 
@@ -139,34 +139,34 @@ static void aw_emac_update_irq(AwEmacState *s)
 
 static void aw_emac_tx_reset(AwEmacState *s, int chan)
 {
-    fifo8_reset(&s->tx_fifo[chan]);
+    fifo_reset(&s->tx_fifo[chan]);
     s->tx_length[chan] = 0;
 }
 
 static void aw_emac_rx_reset(AwEmacState *s)
 {
-    fifo8_reset(&s->rx_fifo);
+    fifo_reset(&s->rx_fifo);
     s->rx_num_packets = 0;
     s->rx_packet_size = 0;
     s->rx_packet_pos = 0;
 }
 
-static void fifo8_push_word(Fifo8 *fifo, uint32_t val)
+static void fifo_push_word(Fifo *fifo, uint32_t val)
 {
-    fifo8_push(fifo, val);
-    fifo8_push(fifo, val >> 8);
-    fifo8_push(fifo, val >> 16);
-    fifo8_push(fifo, val >> 24);
+    fifo_push(fifo, val);
+    fifo_push(fifo, val >> 8);
+    fifo_push(fifo, val >> 16);
+    fifo_push(fifo, val >> 24);
 }
 
-static uint32_t fifo8_pop_word(Fifo8 *fifo)
+static uint32_t fifo_pop_word(Fifo *fifo)
 {
     uint32_t ret;
 
-    ret = fifo8_pop(fifo);
-    ret |= fifo8_pop(fifo) << 8;
-    ret |= fifo8_pop(fifo) << 16;
-    ret |= fifo8_pop(fifo) << 24;
+    ret = fifo_pop(fifo);
+    ret |= fifo_pop(fifo) << 8;
+    ret |= fifo_pop(fifo) << 16;
+    ret |= fifo_pop(fifo) << 24;
 
     return ret;
 }
@@ -179,37 +179,37 @@ static int aw_emac_can_receive(NetClientState *nc)
      * To avoid packet drops, allow reception only when there is space
      * for a full frame: 1522 + 8 (rx headers) + 2 (padding).
      */
-    return (s->ctl & EMAC_CTL_RX_EN) && (fifo8_num_free(&s->rx_fifo) >= 1532);
+    return (s->ctl & EMAC_CTL_RX_EN) && (fifo_num_free(&s->rx_fifo) >= 1532);
 }
 
 static ssize_t aw_emac_receive(NetClientState *nc, const uint8_t *buf,
                                size_t size)
 {
     AwEmacState *s = qemu_get_nic_opaque(nc);
-    Fifo8 *fifo = &s->rx_fifo;
+    Fifo *fifo = &s->rx_fifo;
     size_t padded_size, total_size;
     uint32_t crc;
 
     padded_size = size > 60 ? size : 60;
     total_size = QEMU_ALIGN_UP(RX_HDR_SIZE + padded_size + CRC_SIZE, 4);
 
-    if (!(s->ctl & EMAC_CTL_RX_EN) || (fifo8_num_free(fifo) < total_size)) {
+    if (!(s->ctl & EMAC_CTL_RX_EN) || (fifo_num_free(fifo) < total_size)) {
         return -1;
     }
 
-    fifo8_push_word(fifo, EMAC_UNDOCUMENTED_MAGIC);
-    fifo8_push_word(fifo, EMAC_RX_HEADER(padded_size + CRC_SIZE,
-                                         EMAC_RX_IO_DATA_STATUS_OK));
-    fifo8_push_all(fifo, buf, size);
+    fifo_push_word(fifo, EMAC_UNDOCUMENTED_MAGIC);
+    fifo_push_word(fifo, EMAC_RX_HEADER(padded_size + CRC_SIZE,
+                                        EMAC_RX_IO_DATA_STATUS_OK));
+    fifo_push_all(fifo, buf, size);
     crc = crc32(~0, buf, size);
 
     if (padded_size != size) {
-        fifo8_push_all(fifo, padding, padded_size - size);
+        fifo_push_all(fifo, padding, padded_size - size);
         crc = crc32(crc, padding, padded_size - size);
     }
 
-    fifo8_push_word(fifo, crc);
-    fifo8_push_all(fifo, padding, QEMU_ALIGN_UP(padded_size, 4) - padded_size);
+    fifo_push_word(fifo, crc);
+    fifo_push_all(fifo, padding, QEMU_ALIGN_UP(padded_size, 4) - padded_size);
     s->rx_num_packets++;
 
     s->int_sta |= EMAC_INT_RX;
@@ -247,7 +247,7 @@ static void aw_emac_reset(DeviceState *dev)
 static uint64_t aw_emac_read(void *opaque, hwaddr offset, unsigned size)
 {
     AwEmacState *s = opaque;
-    Fifo8 *fifo = &s->rx_fifo;
+    Fifo *fifo = &s->rx_fifo;
     NetClientState *nc;
     uint64_t ret;
 
@@ -267,7 +267,7 @@ static uint64_t aw_emac_read(void *opaque, hwaddr offset, unsigned size)
             return 0;
         }
 
-        ret = fifo8_pop_word(fifo);
+        ret = fifo_pop_word(fifo);
 
         switch (s->rx_packet_pos) {
         case 0:     /* Word is magic header */
@@ -315,7 +315,7 @@ static void aw_emac_write(void *opaque, hwaddr offset, uint64_t value,
                           unsigned size)
 {
     AwEmacState *s = opaque;
-    Fifo8 *fifo;
+    Fifo *fifo;
     NetClientState *nc = qemu_get_queue(s->nic);
     int chan;
 
@@ -343,13 +343,13 @@ static void aw_emac_write(void *opaque, hwaddr offset, uint64_t value,
             fifo = &s->tx_fifo[chan];
             len = s->tx_length[chan];
 
-            if (len > fifo8_num_used(fifo)) {
-                len = fifo8_num_used(fifo);
+            if (len > fifo_num_used(fifo)) {
+                len = fifo_num_used(fifo);
                 qemu_log_mask(LOG_GUEST_ERROR,
                               "allwinner_emac: TX length > fifo data length\n");
             }
             if (len > 0) {
-                data = fifo8_pop_buf(fifo, len, &ret);
+                data = fifo_pop_buf(fifo, len, &ret);
                 qemu_send_packet(nc, data, ret);
                 aw_emac_tx_reset(s, chan);
                 /* Raise TX interrupt */
@@ -374,12 +374,12 @@ static void aw_emac_write(void *opaque, hwaddr offset, uint64_t value,
         break;
     case EMAC_TX_IO_DATA_REG:
         fifo = &s->tx_fifo[s->tx_channel];
-        if (fifo8_num_free(fifo) < 4) {
+        if (fifo_num_free(fifo) < 4) {
             qemu_log_mask(LOG_GUEST_ERROR,
                           "allwinner_emac: TX data overruns fifo\n");
             break;
         }
-        fifo8_push_word(fifo, value);
+        fifo_push_word(fifo, value);
         break;
     case EMAC_RX_CTL_REG:
         s->rx_ctl = value;
@@ -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);
 
-    fifo8_create(&s->rx_fifo, RX_FIFO_SIZE);
-    fifo8_create(&s->tx_fifo[0], TX_FIFO_SIZE);
-    fifo8_create(&s->tx_fifo[1], TX_FIFO_SIZE);
+    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);
 }
 
 static Property aw_emac_properties[] = {
@@ -501,12 +501,12 @@ static const VMStateDescription vmstate_aw_emac = {
         VMSTATE_UINT32(int_ctl, AwEmacState),
         VMSTATE_UINT32(int_sta, AwEmacState),
         VMSTATE_UINT32(phy_target, AwEmacState),
-        VMSTATE_FIFO8(rx_fifo, AwEmacState),
+        VMSTATE_FIFO(rx_fifo, AwEmacState),
         VMSTATE_UINT32(rx_num_packets, AwEmacState),
         VMSTATE_UINT32(rx_packet_size, AwEmacState),
         VMSTATE_UINT32(rx_packet_pos, AwEmacState),
         VMSTATE_STRUCT_ARRAY(tx_fifo, AwEmacState, NUM_TX_FIFOS, 1,
-                             vmstate_fifo8, Fifo8),
+                             vmstate_fifo, Fifo),
         VMSTATE_UINT32_ARRAY(tx_length, AwEmacState, NUM_TX_FIFOS),
         VMSTATE_UINT32(tx_channel, AwEmacState),
         VMSTATE_END_OF_LIST()
diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
index d44caae..8fe3072 100644
--- a/hw/ssi/xilinx_spi.c
+++ b/hw/ssi/xilinx_spi.c
@@ -27,7 +27,7 @@
 #include "hw/sysbus.h"
 #include "sysemu/sysemu.h"
 #include "qemu/log.h"
-#include "qemu/fifo8.h"
+#include "qemu/fifo.h"
 
 #include "hw/ssi.h"
 
@@ -89,15 +89,15 @@ typedef struct XilinxSPI {
 
     SSIBus *spi;
 
-    Fifo8 rx_fifo;
-    Fifo8 tx_fifo;
+    Fifo rx_fifo;
+    Fifo tx_fifo;
 
     uint32_t regs[R_MAX];
 } XilinxSPI;
 
 static void txfifo_reset(XilinxSPI *s)
 {
-    fifo8_reset(&s->tx_fifo);
+    fifo_reset(&s->tx_fifo);
 
     s->regs[R_SPISR] &= ~SR_TX_FULL;
     s->regs[R_SPISR] |= SR_TX_EMPTY;
@@ -105,7 +105,7 @@ static void txfifo_reset(XilinxSPI *s)
 
 static void rxfifo_reset(XilinxSPI *s)
 {
-    fifo8_reset(&s->rx_fifo);
+    fifo_reset(&s->rx_fifo);
 
     s->regs[R_SPISR] |= SR_RX_EMPTY;
     s->regs[R_SPISR] &= ~SR_RX_FULL;
@@ -125,8 +125,8 @@ static void xlx_spi_update_irq(XilinxSPI *s)
     uint32_t pending;
 
     s->regs[R_IPISR] |=
-            (!fifo8_is_empty(&s->rx_fifo) ? IRQ_DRR_NOT_EMPTY : 0) |
-            (fifo8_is_full(&s->rx_fifo) ? IRQ_DRR_FULL : 0);
+            (!fifo_is_empty(&s->rx_fifo) ? IRQ_DRR_NOT_EMPTY : 0) |
+            (fifo_is_full(&s->rx_fifo) ? IRQ_DRR_FULL : 0);
 
     pending = s->regs[R_IPISR] & s->regs[R_IPIER];
 
@@ -171,16 +171,16 @@ static void spi_flush_txfifo(XilinxSPI *s)
     uint32_t tx;
     uint32_t rx;
 
-    while (!fifo8_is_empty(&s->tx_fifo)) {
-        tx = (uint32_t)fifo8_pop(&s->tx_fifo);
+    while (!fifo_is_empty(&s->tx_fifo)) {
+        tx = (uint32_t)fifo_pop(&s->tx_fifo);
         DB_PRINT("data tx:%x\n", tx);
         rx = ssi_transfer(s->spi, tx);
         DB_PRINT("data rx:%x\n", rx);
-        if (fifo8_is_full(&s->rx_fifo)) {
+        if (fifo_is_full(&s->rx_fifo)) {
             s->regs[R_IPISR] |= IRQ_DRR_OVERRUN;
         } else {
-            fifo8_push(&s->rx_fifo, (uint8_t)rx);
-            if (fifo8_is_full(&s->rx_fifo)) {
+            fifo_push(&s->rx_fifo, (uint8_t)rx);
+            if (fifo_is_full(&s->rx_fifo)) {
                 s->regs[R_SPISR] |= SR_RX_FULL;
                 s->regs[R_IPISR] |= IRQ_DRR_FULL;
             }
@@ -205,14 +205,14 @@ spi_read(void *opaque, hwaddr addr, unsigned int size)
     addr >>= 2;
     switch (addr) {
     case R_SPIDRR:
-        if (fifo8_is_empty(&s->rx_fifo)) {
+        if (fifo_is_empty(&s->rx_fifo)) {
             DB_PRINT("Read from empty FIFO!\n");
             return 0xdeadbeef;
         }
 
         s->regs[R_SPISR] &= ~SR_RX_FULL;
-        r = fifo8_pop(&s->rx_fifo);
-        if (fifo8_is_empty(&s->rx_fifo)) {
+        r = fifo_pop(&s->rx_fifo);
+        if (fifo_is_empty(&s->rx_fifo)) {
             s->regs[R_SPISR] |= SR_RX_EMPTY;
         }
         break;
@@ -253,8 +253,8 @@ spi_write(void *opaque, hwaddr addr,
 
     case R_SPIDTR:
         s->regs[R_SPISR] &= ~SR_TX_EMPTY;
-        fifo8_push(&s->tx_fifo, (uint8_t)value);
-        if (fifo8_is_full(&s->tx_fifo)) {
+        fifo_push(&s->tx_fifo, (uint8_t)value);
+        if (fifo_is_full(&s->tx_fifo)) {
             s->regs[R_SPISR] |= SR_TX_FULL;
         }
         if (!spi_master_enabled(s)) {
@@ -341,8 +341,8 @@ static int xilinx_spi_init(SysBusDevice *sbd)
 
     s->irqline = -1;
 
-    fifo8_create(&s->tx_fifo, FIFO_CAPACITY);
-    fifo8_create(&s->rx_fifo, FIFO_CAPACITY);
+    fifo_create(&s->tx_fifo, FIFO_CAPACITY);
+    fifo_create(&s->rx_fifo, FIFO_CAPACITY);
 
     return 0;
 }
@@ -353,8 +353,8 @@ static const VMStateDescription vmstate_xilinx_spi = {
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
     .fields = (VMStateField[]) {
-        VMSTATE_FIFO8(tx_fifo, XilinxSPI),
-        VMSTATE_FIFO8(rx_fifo, XilinxSPI),
+        VMSTATE_FIFO(tx_fifo, XilinxSPI),
+        VMSTATE_FIFO(rx_fifo, XilinxSPI),
         VMSTATE_UINT32_ARRAY(regs, XilinxSPI, R_MAX),
         VMSTATE_END_OF_LIST()
     }
diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index 8977243..d7d4c41 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -26,7 +26,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/ptimer.h"
 #include "qemu/log.h"
-#include "qemu/fifo8.h"
+#include "qemu/fifo.h"
 #include "hw/ssi.h"
 #include "qemu/bitops.h"
 
@@ -150,8 +150,8 @@ typedef struct {
     qemu_irq *cs_lines;
     SSIBus **spi;
 
-    Fifo8 rx_fifo;
-    Fifo8 tx_fifo;
+    Fifo rx_fifo;
+    Fifo tx_fifo;
 
     uint8_t num_txrx_bytes;
 
@@ -196,7 +196,7 @@ static inline int num_effective_busses(XilinxSPIPS *s)
 static inline bool xilinx_spips_cs_is_set(XilinxSPIPS *s, int i, int field)
 {
     return ~field & (1 << i) && (s->regs[R_CONFIG] & MANUAL_CS
-                    || !fifo8_is_empty(&s->tx_fifo));
+                    || !fifo_is_empty(&s->tx_fifo));
 }
 
 static void xilinx_spips_update_cs_lines(XilinxSPIPS *s)
@@ -239,9 +239,9 @@ static void xilinx_spips_update_ixr(XilinxSPIPS *s)
                                 IXR_TX_FIFO_MODE_FAIL);
     /* these are pure functions of fifo state, set them here */
     s->regs[R_INTR_STATUS] |=
-        (fifo8_is_full(&s->rx_fifo) ? IXR_RX_FIFO_FULL : 0) |
+        (fifo_is_full(&s->rx_fifo) ? IXR_RX_FIFO_FULL : 0) |
         (s->rx_fifo.num >= s->regs[R_RX_THRES] ? IXR_RX_FIFO_NOT_EMPTY : 0) |
-        (fifo8_is_full(&s->tx_fifo) ? IXR_TX_FIFO_FULL : 0) |
+        (fifo_is_full(&s->tx_fifo) ? IXR_TX_FIFO_FULL : 0) |
         (s->tx_fifo.num < s->regs[R_TX_THRES] ? IXR_TX_FIFO_NOT_FULL : 0);
     /* drive external interrupt pin */
     int new_irqline = !!(s->regs[R_INTR_MASK] & s->regs[R_INTR_STATUS] &
@@ -261,8 +261,8 @@ static void xilinx_spips_reset(DeviceState *d)
         s->regs[i] = 0;
     }
 
-    fifo8_reset(&s->rx_fifo);
-    fifo8_reset(&s->rx_fifo);
+    fifo_reset(&s->rx_fifo);
+    fifo_reset(&s->rx_fifo);
     /* non zero resets */
     s->regs[R_CONFIG] |= MODEFAIL_GEN_EN;
     s->regs[R_SLAVE_IDLE_COUNT] = 0xFF;
@@ -315,7 +315,7 @@ static void xilinx_spips_flush_txfifo(XilinxSPIPS *s)
         uint8_t tx = 0;
         uint8_t tx_rx[num_effective_busses(s)];
 
-        if (fifo8_is_empty(&s->tx_fifo)) {
+        if (fifo_is_empty(&s->tx_fifo)) {
             if (!(s->regs[R_LQSPI_CFG] & LQSPI_CFG_LQ_MODE)) {
                 s->regs[R_INTR_STATUS] |= IXR_TX_FIFO_UNDERFLOW;
             }
@@ -323,11 +323,11 @@ static void xilinx_spips_flush_txfifo(XilinxSPIPS *s)
             return;
         } else if (s->snoop_state == SNOOP_STRIPING) {
             for (i = 0; i < num_effective_busses(s); ++i) {
-                tx_rx[i] = fifo8_pop(&s->tx_fifo);
+                tx_rx[i] = fifo_pop(&s->tx_fifo);
             }
             stripe8(tx_rx, num_effective_busses(s), false);
         } else {
-            tx = fifo8_pop(&s->tx_fifo);
+            tx = fifo_pop(&s->tx_fifo);
             for (i = 0; i < num_effective_busses(s); ++i) {
                 tx_rx[i] = tx;
             }
@@ -339,16 +339,16 @@ static void xilinx_spips_flush_txfifo(XilinxSPIPS *s)
             DB_PRINT_L(debug_level, "rx = %02x\n", tx_rx[i]);
         }
 
-        if (fifo8_is_full(&s->rx_fifo)) {
+        if (fifo_is_full(&s->rx_fifo)) {
             s->regs[R_INTR_STATUS] |= IXR_RX_FIFO_OVERFLOW;
             DB_PRINT_L(0, "rx FIFO overflow");
         } else if (s->snoop_state == SNOOP_STRIPING) {
             stripe8(tx_rx, num_effective_busses(s), true);
             for (i = 0; i < num_effective_busses(s); ++i) {
-                fifo8_push(&s->rx_fifo, (uint8_t)tx_rx[i]);
+                fifo_push(&s->rx_fifo, (uint8_t)tx_rx[i]);
             }
         } else {
-           fifo8_push(&s->rx_fifo, (uint8_t)tx_rx[0]);
+           fifo_push(&s->rx_fifo, (uint8_t)tx_rx[0]);
         }
 
         DB_PRINT_L(debug_level, "initial snoop state: %x\n",
@@ -395,8 +395,8 @@ static inline void rx_data_bytes(XilinxSPIPS *s, uint8_t *value, int max)
 {
     int i;
 
-    for (i = 0; i < max && !fifo8_is_empty(&s->rx_fifo); ++i) {
-        value[i] = fifo8_pop(&s->rx_fifo);
+    for (i = 0; i < max && !fifo_is_empty(&s->rx_fifo); ++i) {
+        value[i] = fifo_pop(&s->rx_fifo);
     }
 }
 
@@ -453,12 +453,12 @@ static uint64_t xilinx_spips_read(void *opaque, hwaddr addr,
 static inline void tx_data_bytes(XilinxSPIPS *s, uint32_t value, int num)
 {
     int i;
-    for (i = 0; i < num && !fifo8_is_full(&s->tx_fifo); ++i) {
+    for (i = 0; i < num && !fifo_is_full(&s->tx_fifo); ++i) {
         if (s->regs[R_CONFIG] & ENDIAN) {
-            fifo8_push(&s->tx_fifo, (uint8_t)(value >> 24));
+            fifo_push(&s->tx_fifo, (uint8_t)(value >> 24));
             value <<= 8;
         } else {
-            fifo8_push(&s->tx_fifo, (uint8_t)value);
+            fifo_push(&s->tx_fifo, (uint8_t)value);
             value >>= 8;
         }
     }
@@ -520,7 +520,7 @@ static void xilinx_spips_write(void *opaque, hwaddr addr,
 no_reg_update:
     xilinx_spips_update_cs_lines(s);
     if ((man_start_com && s->regs[R_CONFIG] & MAN_START_EN) ||
-            (fifo8_is_empty(&s->tx_fifo) && s->regs[R_CONFIG] & MAN_START_EN)) {
+            (fifo_is_empty(&s->tx_fifo) && s->regs[R_CONFIG] & MAN_START_EN)) {
         xilinx_spips_flush_txfifo(s);
     }
     xilinx_spips_update_cs_lines(s);
@@ -580,34 +580,34 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size)
 
         DB_PRINT_L(0, "config reg status: %08x\n", s->regs[R_LQSPI_CFG]);
 
-        fifo8_reset(&s->tx_fifo);
-        fifo8_reset(&s->rx_fifo);
+        fifo_reset(&s->tx_fifo);
+        fifo_reset(&s->rx_fifo);
 
         /* instruction */
         DB_PRINT_L(0, "pushing read instruction: %02x\n",
                    (unsigned)(uint8_t)(s->regs[R_LQSPI_CFG] &
                                        LQSPI_CFG_INST_CODE));
-        fifo8_push(&s->tx_fifo, s->regs[R_LQSPI_CFG] & LQSPI_CFG_INST_CODE);
+        fifo_push(&s->tx_fifo, s->regs[R_LQSPI_CFG] & LQSPI_CFG_INST_CODE);
         /* read address */
         DB_PRINT_L(0, "pushing read address %06x\n", flash_addr);
-        fifo8_push(&s->tx_fifo, (uint8_t)(flash_addr >> 16));
-        fifo8_push(&s->tx_fifo, (uint8_t)(flash_addr >> 8));
-        fifo8_push(&s->tx_fifo, (uint8_t)flash_addr);
+        fifo_push(&s->tx_fifo, (uint8_t)(flash_addr >> 16));
+        fifo_push(&s->tx_fifo, (uint8_t)(flash_addr >> 8));
+        fifo_push(&s->tx_fifo, (uint8_t)flash_addr);
         /* mode bits */
         if (s->regs[R_LQSPI_CFG] & LQSPI_CFG_MODE_EN) {
-            fifo8_push(&s->tx_fifo, extract32(s->regs[R_LQSPI_CFG],
-                                              LQSPI_CFG_MODE_SHIFT,
-                                              LQSPI_CFG_MODE_WIDTH));
+            fifo_push(&s->tx_fifo, extract32(s->regs[R_LQSPI_CFG],
+                                             LQSPI_CFG_MODE_SHIFT,
+                                             LQSPI_CFG_MODE_WIDTH));
         }
         /* dummy bytes */
         for (i = 0; i < (extract32(s->regs[R_LQSPI_CFG], LQSPI_CFG_DUMMY_SHIFT,
                                    LQSPI_CFG_DUMMY_WIDTH)); ++i) {
             DB_PRINT_L(0, "pushing dummy byte\n");
-            fifo8_push(&s->tx_fifo, 0);
+            fifo_push(&s->tx_fifo, 0);
         }
         xilinx_spips_update_cs_lines(s);
         xilinx_spips_flush_txfifo(s);
-        fifo8_reset(&s->rx_fifo);
+        fifo_reset(&s->rx_fifo);
 
         DB_PRINT_L(0, "starting QSPI data read\n");
 
@@ -669,8 +669,8 @@ static void xilinx_spips_realize(DeviceState *dev, Error **errp)
 
     s->irqline = -1;
 
-    fifo8_create(&s->rx_fifo, xsc->rx_fifo_size);
-    fifo8_create(&s->tx_fifo, xsc->tx_fifo_size);
+    fifo_create(&s->rx_fifo, xsc->rx_fifo_size);
+    fifo_create(&s->tx_fifo, xsc->tx_fifo_size);
 }
 
 static void xilinx_qspips_realize(DeviceState *dev, Error **errp)
@@ -707,8 +707,8 @@ static const VMStateDescription vmstate_xilinx_spips = {
     .minimum_version_id_old = 2,
     .post_load = xilinx_spips_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_FIFO8(tx_fifo, XilinxSPIPS),
-        VMSTATE_FIFO8(rx_fifo, XilinxSPIPS),
+        VMSTATE_FIFO(tx_fifo, XilinxSPIPS),
+        VMSTATE_FIFO(rx_fifo, XilinxSPIPS),
         VMSTATE_UINT32_ARRAY(regs, XilinxSPIPS, R_MAX),
         VMSTATE_UINT8(snoop_state, XilinxSPIPS),
         VMSTATE_END_OF_LIST()
diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
index f431764..021499b 100644
--- a/include/hw/char/serial.h
+++ b/include/hw/char/serial.h
@@ -28,7 +28,7 @@
 #include "hw/hw.h"
 #include "sysemu/sysemu.h"
 #include "exec/memory.h"
-#include "qemu/fifo8.h"
+#include "qemu/fifo.h"
 
 #define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
 
@@ -60,8 +60,8 @@ struct SerialState {
 
     /* Time when the last byte was successfully sent out of the tsr */
     uint64_t last_xmit_ts;
-    Fifo8 recv_fifo;
-    Fifo8 xmit_fifo;
+    Fifo recv_fifo;
+    Fifo xmit_fifo;
     /* Interrupt trigger level for recv_fifo */
     uint8_t recv_fifo_itl;
 
diff --git a/include/hw/net/allwinner_emac.h b/include/hw/net/allwinner_emac.h
index a5e944a..295b524 100644
--- a/include/hw/net/allwinner_emac.h
+++ b/include/hw/net/allwinner_emac.h
@@ -23,7 +23,7 @@
 #define AW_EMAC_H
 
 #include "net/net.h"
-#include "qemu/fifo8.h"
+#include "qemu/fifo.h"
 
 #define TYPE_AW_EMAC "allwinner-emac"
 #define AW_EMAC(obj) OBJECT_CHECK(AwEmacState, (obj), TYPE_AW_EMAC)
@@ -197,12 +197,12 @@ typedef struct AwEmacState {
     uint32_t       int_sta;
     uint32_t       phy_target;
 
-    Fifo8          rx_fifo;
+    Fifo           rx_fifo;
     uint32_t       rx_num_packets;
     uint32_t       rx_packet_size;
     uint32_t       rx_packet_pos;
 
-    Fifo8          tx_fifo[NUM_TX_FIFOS];
+    Fifo           tx_fifo[NUM_TX_FIFOS];
     uint32_t       tx_length[NUM_TX_FIFOS];
     uint32_t       tx_channel;
 } AwEmacState;
diff --git a/include/qemu/fifo8.h b/include/qemu/fifo.h
similarity index 53%
rename from include/qemu/fifo8.h
rename to include/qemu/fifo.h
index 8820780..44766b3 100644
--- a/include/qemu/fifo8.h
+++ b/include/qemu/fifo.h
@@ -9,102 +9,103 @@ typedef struct {
     uint32_t capacity;
     uint32_t head;
     uint32_t num;
-} Fifo8;
+} Fifo;
 
 /**
- * fifo8_create:
- * @fifo: struct Fifo8 to initialise with new FIFO
+ * fifo_create:
+ * @fifo: struct Fifo to initialise with new FIFO
  * @capacity: capacity of the newly created FIFO
  *
- * Create a FIFO of the specified size. Clients should call fifo8_destroy()
+ * Create a FIFO of the specified size. Clients should call fifo_destroy()
  * when finished using the fifo. The FIFO is initially empty.
  */
 
-void fifo8_create(Fifo8 *fifo, uint32_t capacity);
+void fifo_create(Fifo *fifo, uint32_t capacity);
 
 /**
- * fifo8_destroy:
+ * fifo_destroy:
  * @fifo: FIFO to cleanup
  *
- * Cleanup a FIFO created with fifo8_create(). Frees memory created for FIFO
+ * Cleanup a FIFO created with fifo_create(). Frees memory created for FIFO
   *storage. The FIFO is no longer usable after this has been called.
  */
 
-void fifo8_destroy(Fifo8 *fifo);
+void fifo_destroy(Fifo *fifo);
 
 /**
- * fifo8_push:
+ * fifo_push:
  * @fifo: FIFO to push to
- * @data: data byte to push
+ * @data: data value to push
  *
- * Push a data byte to the FIFO. Behaviour is undefined if the FIFO is full.
- * Clients are responsible for checking for fullness using fifo8_is_full().
+ * Push a data value to the FIFO. Behaviour is undefined if the FIFO is full.
+ * Clients are responsible for checking for fullness using fifo_is_full().
  */
 
-void fifo8_push(Fifo8 *fifo, uint8_t data);
+void fifo_push(Fifo *fifo, uint8_t data);
 
 /**
- * fifo8_push_all:
+ * fifo_push_all:
  * @fifo: FIFO to push to
  * @data: data to push
- * @size: number of bytes to push
+ * @size: number of entries to push
  *
- * Push a byte array to the FIFO. Behaviour is undefined if the FIFO is full.
+ * Push a buffer to the FIFO. Behaviour is undefined if the FIFO is full.
  * Clients are responsible for checking the space left in the FIFO using
- * fifo8_num_free().
+ * fifo_num_free().
  */
 
-void fifo8_push_all(Fifo8 *fifo, const uint8_t *data, uint32_t num);
+void fifo_push_all(Fifo *fifo, const uint8_t *data, uint32_t num);
 
 /**
- * fifo8_pop:
+ * fifo_pop:
  * @fifo: fifo to pop from
  *
- * Pop a data byte from the FIFO. Behaviour is undefined if the FIFO is empty.
- * Clients are responsible for checking for emptyness using fifo8_is_empty().
+ * Pop a data value from the FIFO. Behaviour is undefined if the FIFO is empty.
+ * Clients are responsible for checking for emptyness using fifo_is_empty().
  *
- * Returns: The popped data byte.
+ * Returns: The popped data value.
  */
 
-uint8_t fifo8_pop(Fifo8 *fifo);
+uint8_t fifo_pop(Fifo *fifo);
 
 /**
- * fifo8_pop_buf:
+ * fifo_pop_buf:
  * @fifo: FIFO to pop from
  * @max: maximum number of bytes to pop
  * @num: actual number of returned bytes
  *
  * Pop a number of elements from the FIFO up to a maximum of max. The buffer
  * containing the popped data is returned. This buffer points directly into
- * the FIFO backing store and data is invalidated once any of the fifo8_* APIs
+ * the FIFO backing store and data is invalidated once any of the fifo_* APIs
  * are called on the FIFO.
  *
- * The function may return fewer bytes than requested when the data wraps
+ * The function may return fewer elements than requested when the data wraps
  * around in the ring buffer; in this case only a contiguous part of the data
  * is returned.
  *
  * The number of valid bytes returned is populated in *num; will always return
- * at least 1 byte. max must not be 0 or greater than the number of bytes in
- * the FIFO.
+ * at least 1 element. max must not be 0 or greater than the number of elements
+ * in the FIFO.
  *
  * Clients are responsible for checking the availability of requested data
- * using fifo8_num_used().
+ * using fifo_num_used().
  *
  * Returns: A pointer to popped data.
  */
-const uint8_t *fifo8_pop_buf(Fifo8 *fifo, uint32_t max, uint32_t *num);
+
+const uint8_t *fifo_pop_buf(Fifo *fifo, uint32_t max, uint32_t *num);
 
 /**
- * fifo8_reset:
+ * fifo_reset:
  * @fifo: FIFO to reset
  *
  * Reset a FIFO. All data is discarded and the FIFO is emptied.
  */
 
-void fifo8_reset(Fifo8 *fifo);
+void fifo_reset(Fifo *fifo);
 
 /**
- * fifo8_is_empty:
+ * fifo_is_empty:
  * @fifo: FIFO to check
  *
  * Check if a FIFO is empty.
@@ -112,10 +113,10 @@ void fifo8_reset(Fifo8 *fifo);
  * Returns: True if the fifo is empty, false otherwise.
  */
 
-bool fifo8_is_empty(Fifo8 *fifo);
+bool fifo_is_empty(Fifo *fifo);
 
 /**
- * fifo8_is_full:
+ * fifo_is_full:
  * @fifo: FIFO to check
  *
  * Check if a FIFO is full.
@@ -123,38 +124,38 @@ bool fifo8_is_empty(Fifo8 *fifo);
  * Returns: True if the fifo is full, false otherwise.
  */
 
-bool fifo8_is_full(Fifo8 *fifo);
+bool fifo_is_full(Fifo *fifo);
 
 /**
- * fifo8_num_free:
+ * fifo_num_free:
  * @fifo: FIFO to check
  *
- * Return the number of free bytes in the FIFO.
+ * Return the number of free elements in the FIFO.
  *
  * Returns: Number of free bytes.
  */
 
-uint32_t fifo8_num_free(Fifo8 *fifo);
+uint32_t fifo_num_free(Fifo *fifo);
 
 /**
- * fifo8_num_used:
+ * fifo_num_used:
  * @fifo: FIFO to check
  *
- * Return the number of used bytes in the FIFO.
+ * Return the number of used elements in the FIFO.
  *
  * Returns: Number of used bytes.
  */
 
-uint32_t fifo8_num_used(Fifo8 *fifo);
+uint32_t fifo_num_used(Fifo *fifo);
 
-extern const VMStateDescription vmstate_fifo8;
+extern const VMStateDescription vmstate_fifo;
 
-#define VMSTATE_FIFO8(_field, _state) {                              \
+#define VMSTATE_FIFO(_field, _state) {                               \
     .name       = (stringify(_field)),                               \
-    .size       = sizeof(Fifo8),                                     \
-    .vmsd       = &vmstate_fifo8,                                    \
+    .size       = sizeof(Fifo),                                      \
+    .vmsd       = &vmstate_fifo,                                     \
     .flags      = VMS_STRUCT,                                        \
-    .offset     = vmstate_offset_value(_state, _field, Fifo8),       \
+    .offset     = vmstate_offset_value(_state, _field, Fifo),        \
 }
 
 #endif /* FIFO_H */
diff --git a/util/Makefile.objs b/util/Makefile.objs
index df83b62..7f1489c 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -3,7 +3,7 @@ util-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o event_notifier-win
 util-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o event_notifier-posix.o qemu-openpty.o
 util-obj-y += envlist.o path.o host-utils.o cache-utils.o module.o
 util-obj-y += bitmap.o bitops.o hbitmap.o
-util-obj-y += fifo8.o
+util-obj-y += fifo.o
 util-obj-y += acl.o
 util-obj-y += error.o qemu-error.o
 util-obj-$(CONFIG_POSIX) += compatfd.o
diff --git a/util/fifo8.c b/util/fifo.c
similarity index 75%
rename from util/fifo8.c
rename to util/fifo.c
index 6a43482..ffadf55 100644
--- a/util/fifo8.c
+++ b/util/fifo.c
@@ -13,9 +13,9 @@
  */
 
 #include "qemu-common.h"
-#include "qemu/fifo8.h"
+#include "qemu/fifo.h"
 
-void fifo8_create(Fifo8 *fifo, uint32_t capacity)
+void fifo_create(Fifo *fifo, uint32_t capacity)
 {
     fifo->data = g_new(uint8_t, capacity);
     fifo->capacity = capacity;
@@ -23,12 +23,12 @@ void fifo8_create(Fifo8 *fifo, uint32_t capacity)
     fifo->num = 0;
 }
 
-void fifo8_destroy(Fifo8 *fifo)
+void fifo_destroy(Fifo *fifo)
 {
     g_free(fifo->data);
 }
 
-void fifo8_push(Fifo8 *fifo, uint8_t data)
+void fifo_push(Fifo *fifo, uint8_t data)
 {
     if (fifo->num == fifo->capacity) {
         abort();
@@ -37,7 +37,7 @@ void fifo8_push(Fifo8 *fifo, uint8_t data)
     fifo->num++;
 }
 
-void fifo8_push_all(Fifo8 *fifo, const uint8_t *data, uint32_t num)
+void fifo_push_all(Fifo *fifo, const uint8_t *data, uint32_t num)
 {
     uint32_t start, avail;
 
@@ -58,7 +58,7 @@ void fifo8_push_all(Fifo8 *fifo, const uint8_t *data, uint32_t num)
     fifo->num += num;
 }
 
-uint8_t fifo8_pop(Fifo8 *fifo)
+uint8_t fifo_pop(Fifo *fifo)
 {
     uint8_t ret;
 
@@ -71,7 +71,7 @@ uint8_t fifo8_pop(Fifo8 *fifo)
     return ret;
 }
 
-const uint8_t *fifo8_pop_buf(Fifo8 *fifo, uint32_t max, uint32_t *num)
+const uint8_t *fifo_pop_buf(Fifo *fifo, uint32_t max, uint32_t *num)
 {
     uint8_t *ret;
 
@@ -86,41 +86,41 @@ const uint8_t *fifo8_pop_buf(Fifo8 *fifo, uint32_t max, uint32_t *num)
     return ret;
 }
 
-void fifo8_reset(Fifo8 *fifo)
+void fifo_reset(Fifo *fifo)
 {
     fifo->num = 0;
     fifo->head = 0;
 }
 
-bool fifo8_is_empty(Fifo8 *fifo)
+bool fifo_is_empty(Fifo *fifo)
 {
     return (fifo->num == 0);
 }
 
-bool fifo8_is_full(Fifo8 *fifo)
+bool fifo_is_full(Fifo *fifo)
 {
     return (fifo->num == fifo->capacity);
 }
 
-uint32_t fifo8_num_free(Fifo8 *fifo)
+uint32_t fifo_num_free(Fifo *fifo)
 {
     return fifo->capacity - fifo->num;
 }
 
-uint32_t fifo8_num_used(Fifo8 *fifo)
+uint32_t fifo_num_used(Fifo *fifo)
 {
     return fifo->num;
 }
 
-const VMStateDescription vmstate_fifo8 = {
+const VMStateDescription vmstate_fifo = {
     .name = "Fifo8",
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
     .fields      = (VMStateField[]) {
-        VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
-        VMSTATE_UINT32(head, Fifo8),
-        VMSTATE_UINT32(num, Fifo8),
+        VMSTATE_VBUFFER_UINT32(data, Fifo, 1, NULL, 0, capacity),
+        VMSTATE_UINT32(head, Fifo),
+        VMSTATE_UINT32(num, Fifo),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
1.9.1.1.gbb9f595

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH for-2.1 v2 2/2] util/fifo: Generalise for common integer widths
  2014-04-08  2:04 [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types Peter Crosthwaite
  2014-04-08  2:04 ` [Qemu-devel] [PATCH for-2.1 v2 1/2] util/fifo: s/fifo8/fifo globally Peter Crosthwaite
@ 2014-04-08  2:05 ` Peter Crosthwaite
  2014-04-08 21:42   ` Beniamino Galvani
  2014-04-08  7:14 ` [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types Markus Armbruster
  2 siblings, 1 reply; 12+ messages in thread
From: Peter Crosthwaite @ 2014-04-08  2:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: b.galvani, dslutz

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.crosthwaite@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 */
+    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):
+        ((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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types
  2014-04-08  2:04 [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types Peter Crosthwaite
  2014-04-08  2:04 ` [Qemu-devel] [PATCH for-2.1 v2 1/2] util/fifo: s/fifo8/fifo globally Peter Crosthwaite
  2014-04-08  2:05 ` [Qemu-devel] [PATCH for-2.1 v2 2/2] util/fifo: Generalise for common integer widths Peter Crosthwaite
@ 2014-04-08  7:14 ` Markus Armbruster
  2014-04-08  7:24   ` Peter Crosthwaite
  2 siblings, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2014-04-08  7:14 UTC (permalink / raw)
  To: Peter Crosthwaite; +Cc: b.galvani, qemu-devel, dslutz

Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:

> There is a utility helper for dealing with 8 bit fifos. This should be
> applicable to other integer widths as well. These two patches
> generalise this FIFO to work for 16, 32 and 64 bit ints.

Can you show us a user for the wider FIFOs?

[...]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types
  2014-04-08  7:14 ` [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types Markus Armbruster
@ 2014-04-08  7:24   ` Peter Crosthwaite
  2014-04-08  9:10     ` Markus Armbruster
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Crosthwaite @ 2014-04-08  7:24 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Beniamino Galvani, qemu-devel@nongnu.org Developers, Don Slutz

On Tue, Apr 8, 2014 at 5:14 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>
>> There is a utility helper for dealing with 8 bit fifos. This should be
>> applicable to other integer widths as well. These two patches
>> generalise this FIFO to work for 16, 32 and 64 bit ints.
>
> Can you show us a user for the wider FIFOs?
>

Hi Markus,

I have a couple out of tree that are incomplete and a bit out of scope
of this series. Rather not wait and send a complicated series spanning
multiple sub-systrems. I guess I could shop around for an easy lead
example to fix, but does the series stand in its own right?

Regards,
Peter

> [...]
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types
  2014-04-08  7:24   ` Peter Crosthwaite
@ 2014-04-08  9:10     ` Markus Armbruster
  2014-04-08 21:22       ` Don Slutz
  0 siblings, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2014-04-08  9:10 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: Beniamino Galvani, qemu-devel@nongnu.org Developers, Don Slutz

Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:

> On Tue, Apr 8, 2014 at 5:14 PM, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>>
>>> There is a utility helper for dealing with 8 bit fifos. This should be
>>> applicable to other integer widths as well. These two patches
>>> generalise this FIFO to work for 16, 32 and 64 bit ints.
>>
>> Can you show us a user for the wider FIFOs?
>>
>
> Hi Markus,
>
> I have a couple out of tree that are incomplete and a bit out of scope
> of this series. Rather not wait and send a complicated series spanning
> multiple sub-systrems. I guess I could shop around for an easy lead
> example to fix, but does the series stand in its own right?

I don't like infrastructure without an in-tree user.  Even if it's
"only" a sensible, straightforward generalization of existing
infrastructure.

Infrastructure without a user tends to rot.  Probably not a serious
issue in this particular case.

The cost of applying a change and maintaining the additional complexity
needs to be justified by some gain.  Even when the costs are small, like
they probably are in this particular case.  An in-tree user could
probably justify easily.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.1 v2 1/2] util/fifo: s/fifo8/fifo globally
  2014-04-08  2:04 ` [Qemu-devel] [PATCH for-2.1 v2 1/2] util/fifo: s/fifo8/fifo globally Peter Crosthwaite
@ 2014-04-08 18:48   ` Beniamino Galvani
  2014-04-09  1:50     ` Peter Crosthwaite
  0 siblings, 1 reply; 12+ messages in thread
From: Beniamino Galvani @ 2014-04-08 18:48 UTC (permalink / raw)
  To: Peter Crosthwaite; +Cc: qemu-devel, dslutz

On Mon, Apr 07, 2014 at 07:04:43PM -0700, Peter Crosthwaite wrote:
> This prepares support for generalising FIFO support to more integer
> widths.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> [...]
>
> --- a/include/qemu/fifo8.h
> +++ b/include/qemu/fifo.h
> @@ -9,102 +9,103 @@ typedef struct {
>      uint32_t capacity;
>      uint32_t head;
>      uint32_t num;
> -} Fifo8;
> +} Fifo;
>  
>  /**
> - * fifo8_create:
> - * @fifo: struct Fifo8 to initialise with new FIFO
> + * fifo_create:
> + * @fifo: struct Fifo to initialise with new FIFO
>   * @capacity: capacity of the newly created FIFO
>   *
> - * Create a FIFO of the specified size. Clients should call fifo8_destroy()
> + * Create a FIFO of the specified size. Clients should call fifo_destroy()
>   * when finished using the fifo. The FIFO is initially empty.
>   */
>  
> -void fifo8_create(Fifo8 *fifo, uint32_t capacity);
> +void fifo_create(Fifo *fifo, uint32_t capacity);
>  
>  /**
> - * fifo8_destroy:
> + * fifo_destroy:
>   * @fifo: FIFO to cleanup
>   *
> - * Cleanup a FIFO created with fifo8_create(). Frees memory created for FIFO
> + * Cleanup a FIFO created with fifo_create(). Frees memory created for FIFO
>    *storage. The FIFO is no longer usable after this has been called.
>   */
>  
> -void fifo8_destroy(Fifo8 *fifo);
> +void fifo_destroy(Fifo *fifo);
>  
>  /**
> - * fifo8_push:
> + * fifo_push:
>   * @fifo: FIFO to push to
> - * @data: data byte to push
> + * @data: data value to push
>   *
> - * Push a data byte to the FIFO. Behaviour is undefined if the FIFO is full.
> - * Clients are responsible for checking for fullness using fifo8_is_full().
> + * Push a data value to the FIFO. Behaviour is undefined if the FIFO is full.
> + * Clients are responsible for checking for fullness using fifo_is_full().
>   */
>  
> -void fifo8_push(Fifo8 *fifo, uint8_t data);
> +void fifo_push(Fifo *fifo, uint8_t data);
>  
>  /**
> - * fifo8_push_all:
> + * fifo_push_all:
>   * @fifo: FIFO to push to
>   * @data: data to push
> - * @size: number of bytes to push
> + * @size: number of entries to push
>   *
> - * Push a byte array to the FIFO. Behaviour is undefined if the FIFO is full.
> + * Push a buffer to the FIFO. Behaviour is undefined if the FIFO is full.
>   * Clients are responsible for checking the space left in the FIFO using
> - * fifo8_num_free().
> + * fifo_num_free().
>   */
>  
> -void fifo8_push_all(Fifo8 *fifo, const uint8_t *data, uint32_t num);
> +void fifo_push_all(Fifo *fifo, const uint8_t *data, uint32_t num);
>  
>  /**
> - * fifo8_pop:
> + * fifo_pop:
>   * @fifo: fifo to pop from
>   *
> - * Pop a data byte from the FIFO. Behaviour is undefined if the FIFO is empty.
> - * Clients are responsible for checking for emptyness using fifo8_is_empty().
> + * Pop a data value from the FIFO. Behaviour is undefined if the FIFO is empty.
> + * Clients are responsible for checking for emptyness using fifo_is_empty().
>   *
> - * Returns: The popped data byte.
> + * Returns: The popped data value.
>   */
>  
> -uint8_t fifo8_pop(Fifo8 *fifo);
> +uint8_t fifo_pop(Fifo *fifo);
>  
>  /**
> - * fifo8_pop_buf:
> + * fifo_pop_buf:
>   * @fifo: FIFO to pop from
>   * @max: maximum number of bytes to pop
>   * @num: actual number of returned bytes

Perhaps these and the remaining occurrences of 'bytes' should be
replaced as well.

Otherwise:

Reviewed-by: Beniamino Galvani <b.galvani@gmail.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types
  2014-04-08  9:10     ` Markus Armbruster
@ 2014-04-08 21:22       ` Don Slutz
  2014-04-09  0:03         ` Peter Crosthwaite
  0 siblings, 1 reply; 12+ messages in thread
From: Don Slutz @ 2014-04-08 21:22 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Beniamino Galvani, Peter Crosthwaite,
	qemu-devel@nongnu.org Developers, Don Slutz


On 04/08/14 05:10, Markus Armbruster wrote:
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>
>> On Tue, Apr 8, 2014 at 5:14 PM, Markus Armbruster <armbru@redhat.com> wrote:
>>> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>>>
>>>> There is a utility helper for dealing with 8 bit fifos. This should be
>>>> applicable to other integer widths as well. These two patches
>>>> generalise this FIFO to work for 16, 32 and 64 bit ints.
>>> Can you show us a user for the wider FIFOs?
>>>
>> Hi Markus,
>>
>> I have a couple out of tree that are incomplete and a bit out of scope
>> of this series. Rather not wait and send a complicated series spanning
>> multiple sub-systrems. I guess I could shop around for an easy lead
>> example to fix, but does the series stand in its own right?
> I don't like infrastructure without an in-tree user.  Even if it's
> "only" a sensible, straightforward generalization of existing
> infrastructure.
>
> Infrastructure without a user tends to rot.  Probably not a serious
> issue in this particular case.
>
> The cost of applying a change and maintaining the additional complexity
> needs to be justified by some gain.  Even when the costs are small, like
> they probably are in this particular case.  An in-tree user could
> probably justify easily.

I am not sure that the run time cost is the best.  Looks more like c++ 
templates should be done via macros.  Just like 
include/exec/softmmu_header.h:


void glue(glue(fifo, 8), _create)(Fifo *fifo, uint32_t capacity);
void glue(glue(fifo, 8), _push)(Fifo *fifo, glue(glue(uint, 8), _t) data);
...

Also that would mean not doing the patch #1 at all.

That way all the cost is at compile time, and only the versions 
currently used are generated.

    -Don Slutz

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.1 v2 2/2] util/fifo: Generalise for common integer widths
  2014-04-08  2:05 ` [Qemu-devel] [PATCH for-2.1 v2 2/2] util/fifo: Generalise for common integer widths Peter Crosthwaite
@ 2014-04-08 21:42   ` Beniamino Galvani
  2014-04-09  1:51     ` Peter Crosthwaite
  0 siblings, 1 reply; 12+ messages in thread
From: Beniamino Galvani @ 2014-04-08 21:42 UTC (permalink / raw)
  To: Peter Crosthwaite; +Cc: qemu-devel, dslutz

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.crosthwaite@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types
  2014-04-08 21:22       ` Don Slutz
@ 2014-04-09  0:03         ` Peter Crosthwaite
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Crosthwaite @ 2014-04-09  0:03 UTC (permalink / raw)
  To: Don Slutz
  Cc: Beniamino Galvani, Markus Armbruster, qemu-devel@nongnu.org Developers

On Wed, Apr 9, 2014 at 7:22 AM, Don Slutz <dslutz@verizon.com> wrote:
>
> On 04/08/14 05:10, Markus Armbruster wrote:
>>
>> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>>
>>> On Tue, Apr 8, 2014 at 5:14 PM, Markus Armbruster <armbru@redhat.com>
>>> wrote:
>>>>
>>>> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>>>>
>>>>> There is a utility helper for dealing with 8 bit fifos. This should be
>>>>> applicable to other integer widths as well. These two patches
>>>>> generalise this FIFO to work for 16, 32 and 64 bit ints.
>>>>
>>>> Can you show us a user for the wider FIFOs?
>>>>
>>> Hi Markus,
>>>
>>> I have a couple out of tree that are incomplete and a bit out of scope
>>> of this series. Rather not wait and send a complicated series spanning
>>> multiple sub-systrems. I guess I could shop around for an easy lead
>>> example to fix, but does the series stand in its own right?
>>
>> I don't like infrastructure without an in-tree user.  Even if it's
>> "only" a sensible, straightforward generalization of existing
>> infrastructure.
>>
>> Infrastructure without a user tends to rot.  Probably not a serious
>> issue in this particular case.
>>
>> The cost of applying a change and maintaining the additional complexity
>> needs to be justified by some gain.  Even when the costs are small, like
>> they probably are in this particular case.  An in-tree user could
>> probably justify easily.
>
>
> I am not sure that the run time cost is the best.  Looks more like c++
> templates should be done via macros.  Just like
> include/exec/softmmu_header.h:
>
>
> void glue(glue(fifo, 8), _create)(Fifo *fifo, uint32_t capacity);
> void glue(glue(fifo, 8), _push)(Fifo *fifo, glue(glue(uint, 8), _t) data);
> ...
>

So I did actually start out with a glue based implementation of this
but thought the better of it. It's reasonably ugly in general and the
VMSD stuff was trickiest. There are however some possible compromises
when we consider only using glue for the hottest paths. E.g. I think
push and pop are both red-hot as they tend to happen once per byte.
The buffer pushes/pops not so much - they are more a once-per-packet.
So if we glue just the single push/pop we might be in a good place.
reset/create etc is all pretty cold. occupancy checkers (is_full) etc
are unaffected perf wise by the common implementation.

> Also that would mean not doing the patch #1 at all.
>

If we went with my hot-path-only compromise I think we would alias the
create function as such to keep the namespaces consistent:

static inline void fifo_create8(Fifo *fifo, uint32_t cap) {
    fifo_create(fifo, cap, 8);
}

But I think I want to s/fifo8_pop/fifo_pop8 etc though as the fifo8
type wouldnt really exist anymore and this would be more consistent
with other api that have 8/16/32/64 bit variants of their functions.
So the P1 change pattern has a place. Reset/is_full/is_empty would all
remain as in this series.

Regards,
Peter

> That way all the cost is at compile time, and only the versions currently
> used are generated.
>
>    -Don Slutz
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.1 v2 1/2] util/fifo: s/fifo8/fifo globally
  2014-04-08 18:48   ` Beniamino Galvani
@ 2014-04-09  1:50     ` Peter Crosthwaite
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Crosthwaite @ 2014-04-09  1:50 UTC (permalink / raw)
  To: Beniamino Galvani; +Cc: qemu-devel@nongnu.org Developers, Don Slutz

On Wed, Apr 9, 2014 at 4:48 AM, Beniamino Galvani <b.galvani@gmail.com> wrote:
> On Mon, Apr 07, 2014 at 07:04:43PM -0700, Peter Crosthwaite wrote:
>> This prepares support for generalising FIFO support to more integer
>> widths.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>> [...]
>>
>> --- a/include/qemu/fifo8.h
>> +++ b/include/qemu/fifo.h
>> @@ -9,102 +9,103 @@ typedef struct {
>>      uint32_t capacity;
>>      uint32_t head;
>>      uint32_t num;
>> -} Fifo8;
>> +} Fifo;
>>
>>  /**
>> - * fifo8_create:
>> - * @fifo: struct Fifo8 to initialise with new FIFO
>> + * fifo_create:
>> + * @fifo: struct Fifo to initialise with new FIFO
>>   * @capacity: capacity of the newly created FIFO
>>   *
>> - * Create a FIFO of the specified size. Clients should call fifo8_destroy()
>> + * Create a FIFO of the specified size. Clients should call fifo_destroy()
>>   * when finished using the fifo. The FIFO is initially empty.
>>   */
>>
>> -void fifo8_create(Fifo8 *fifo, uint32_t capacity);
>> +void fifo_create(Fifo *fifo, uint32_t capacity);
>>
>>  /**
>> - * fifo8_destroy:
>> + * fifo_destroy:
>>   * @fifo: FIFO to cleanup
>>   *
>> - * Cleanup a FIFO created with fifo8_create(). Frees memory created for FIFO
>> + * Cleanup a FIFO created with fifo_create(). Frees memory created for FIFO
>>    *storage. The FIFO is no longer usable after this has been called.
>>   */
>>
>> -void fifo8_destroy(Fifo8 *fifo);
>> +void fifo_destroy(Fifo *fifo);
>>
>>  /**
>> - * fifo8_push:
>> + * fifo_push:
>>   * @fifo: FIFO to push to
>> - * @data: data byte to push
>> + * @data: data value to push
>>   *
>> - * Push a data byte to the FIFO. Behaviour is undefined if the FIFO is full.
>> - * Clients are responsible for checking for fullness using fifo8_is_full().
>> + * Push a data value to the FIFO. Behaviour is undefined if the FIFO is full.
>> + * Clients are responsible for checking for fullness using fifo_is_full().
>>   */
>>
>> -void fifo8_push(Fifo8 *fifo, uint8_t data);
>> +void fifo_push(Fifo *fifo, uint8_t data);
>>
>>  /**
>> - * fifo8_push_all:
>> + * fifo_push_all:
>>   * @fifo: FIFO to push to
>>   * @data: data to push
>> - * @size: number of bytes to push
>> + * @size: number of entries to push
>>   *
>> - * Push a byte array to the FIFO. Behaviour is undefined if the FIFO is full.
>> + * Push a buffer to the FIFO. Behaviour is undefined if the FIFO is full.
>>   * Clients are responsible for checking the space left in the FIFO using
>> - * fifo8_num_free().
>> + * fifo_num_free().
>>   */
>>
>> -void fifo8_push_all(Fifo8 *fifo, const uint8_t *data, uint32_t num);
>> +void fifo_push_all(Fifo *fifo, const uint8_t *data, uint32_t num);
>>
>>  /**
>> - * fifo8_pop:
>> + * fifo_pop:
>>   * @fifo: fifo to pop from
>>   *
>> - * Pop a data byte from the FIFO. Behaviour is undefined if the FIFO is empty.
>> - * Clients are responsible for checking for emptyness using fifo8_is_empty().
>> + * Pop a data value from the FIFO. Behaviour is undefined if the FIFO is empty.
>> + * Clients are responsible for checking for emptyness using fifo_is_empty().
>>   *
>> - * Returns: The popped data byte.
>> + * Returns: The popped data value.
>>   */
>>
>> -uint8_t fifo8_pop(Fifo8 *fifo);
>> +uint8_t fifo_pop(Fifo *fifo);
>>
>>  /**
>> - * fifo8_pop_buf:
>> + * fifo_pop_buf:
>>   * @fifo: FIFO to pop from
>>   * @max: maximum number of bytes to pop
>>   * @num: actual number of returned bytes
>
> Perhaps these and the remaining occurrences of 'bytes' should be
> replaced as well.
>

Will fix.

> Otherwise:
>
> Reviewed-by: Beniamino Galvani <b.galvani@gmail.com>
>

Thanks.

Regards,
Peter

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.1 v2 2/2] util/fifo: Generalise for common integer widths
  2014-04-08 21:42   ` Beniamino Galvani
@ 2014-04-09  1:51     ` Peter Crosthwaite
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Crosthwaite @ 2014-04-09  1:51 UTC (permalink / raw)
  To: Beniamino Galvani; +Cc: qemu-devel@nongnu.org Developers, Don Slutz

On Wed, Apr 9, 2014 at 7:42 AM, Beniamino Galvani <b.galvani@gmail.com> wrote:
> 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.crosthwaite@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
>

Will fix.

>> +    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 :)
>

Ok, will fix.

Regards,
Peter

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-04-09  1:51 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-08  2:04 [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types Peter Crosthwaite
2014-04-08  2:04 ` [Qemu-devel] [PATCH for-2.1 v2 1/2] util/fifo: s/fifo8/fifo globally Peter Crosthwaite
2014-04-08 18:48   ` Beniamino Galvani
2014-04-09  1:50     ` Peter Crosthwaite
2014-04-08  2:05 ` [Qemu-devel] [PATCH for-2.1 v2 2/2] util/fifo: Generalise for common integer widths Peter Crosthwaite
2014-04-08 21:42   ` Beniamino Galvani
2014-04-09  1:51     ` Peter Crosthwaite
2014-04-08  7:14 ` [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types Markus Armbruster
2014-04-08  7:24   ` Peter Crosthwaite
2014-04-08  9:10     ` Markus Armbruster
2014-04-08 21:22       ` Don Slutz
2014-04-09  0:03         ` Peter Crosthwaite

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.