[Spice-devel,Xspice,09/11] qxl_ring: replace memcpy with a simple copy_to/from_qxl_ring

Submitted by Uri Lublin on Jan. 26, 2015, 10:35 a.m.

Details

Message ID 7d4203db1305a3987894098bc629ae8a33b8a78d.1422267879.git.uril@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Uri Lublin Jan. 26, 2015, 10:35 a.m.
cc: Dave Airlie <airlied@redhat.com>

Since memcpy is not supposed to be used on volatile memory, use
a simple loop that copies one byte at a time.

This fixes compiler warnings:
qxl_ring.c: In function ‘qxl_ring_push’:
qxl_ring.c:96: warning: cast discards qualifiers from pointer target type
qxl_ring.c: In function ‘qxl_ring_pop’:
qxl_ring.c:121: warning: cast discards qualifiers from pointer target type

Related to commit f7ba4bae

This commit may hurt performance a bit, but users of these functions use
n=8 or n=16, so it should not be too bad.

To boost performance, we can copy 8 bytes (uint64_t) at a time
instead of one, as users of this function always have n=8 or n=16.
---
 src/qxl_ring.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/qxl_ring.c b/src/qxl_ring.c
index 0dcacf7..3928d51 100644
--- a/src/qxl_ring.c
+++ b/src/qxl_ring.c
@@ -69,6 +69,24 @@  qxl_ring_create (struct qxl_ring_header *header,
     return ring;
 }
 
+static void
+copy_to_qxl_ring(volatile uint8_t *ring_dst, const uint8_t *src, int n)
+{
+    int i;
+
+    for (i=0; i<n; i++)
+        ring_dst[i] = src[i];
+}
+
+static void
+copy_from_qxl_ring(uint8_t *dst, volatile uint8_t *ring_src, int n)
+{
+    int i;
+
+    for (i=0; i<n; i++)
+        dst[i] = ring_src[i];
+}
+
 void
 qxl_ring_push (struct qxl_ring *ring,
 	       const void      *new_elt)
@@ -91,9 +109,7 @@  qxl_ring_push (struct qxl_ring *ring,
     idx = header->prod & (ring->n_elements - 1);
     elt = ring->ring->elements + idx * ring->element_size;
 
-    /* TODO:  We should use proper MMIO accessors; the use of
-             volatile leads to a gcc warning.  See commit f7ba4bae */
-    memcpy((void *)elt, new_elt, ring->element_size);
+   copy_to_qxl_ring(elt, new_elt, ring->element_size);
 
     header->prod++;
 
@@ -118,7 +134,7 @@  qxl_ring_pop (struct qxl_ring *ring,
     idx = header->cons & (ring->n_elements - 1);
     ring_elt = ring->ring->elements + idx * ring->element_size;
 
-    memcpy (element, (void *)ring_elt, ring->element_size);
+    copy_from_qxl_ring(element, ring_elt, ring->element_size);
 
     header->cons++;