Re: [PATCH] qemu-kvm: Flush icache after dma operations for ia64

[Jes Sorensen <jes@sgi.com>]

[-- Attachment #1: Type: text/plain, Size: 407 bytes --]

Ok,

Trying once more. After spending a couple of hours trying to follow
the QEMU dma codeflow, I have convinced myself Avi is right and those
two functions don't need to do the flushing as they all end up calling
dma_bdrv_cb() which calls dma_brdv_unmap(). I have added a couple
comments to the code, which will hopefully save the next person the
'pleasure' of trying to figure out this too.

Cheers,
Jes


[-- Attachment #2: 0001-qemu-kvm-Flush-icache-after-dma-operations-for-ia64-v3.patch --]
[-- Type: text/x-patch, Size: 4678 bytes --]

ia64 system depends on that platform issues snoop cycle to flush
icache for memory touched by DMA write operations, but virtual DMA
operations is emulated by memcpy, so use explict instrustions to flush
the related icache, otherwise, guest may use obsolete icache.

Slightly modified version of Xiantao's patch, which avoids the #ifdef's
for ia64 by introducing a dma_flush_range() function defined as a noop
on architectures which do not need it.

Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
Signed-off-by: Jes Sorensen <jes@sgi.com>

---
 cache-utils.h           |   21 +++++++++++++++++++++
 cutils.c                |    5 +++++
 dma-helpers.c           |    4 ++++
 exec.c                  |    7 ++++++-
 target-ia64/cpu.h       |    1 -
 target-ia64/fake-exec.c |    9 ---------
 6 files changed, 36 insertions(+), 11 deletions(-)

Index: qemu-kvm/cache-utils.h
===================================================================
--- qemu-kvm.orig/cache-utils.h
+++ qemu-kvm/cache-utils.h
@@ -34,7 +34,28 @@
     asm volatile ("isync" : : : "memory");
 }
 
+/*
+ * Is this correct for PPC?
+ */
+static inline void dma_flush_range(unsigned long start, unsigned long stop)
+{
+}
+
+#elif defined(__ia64__)
+static inline void flush_icache_range(unsigned long start, unsigned long stop)
+{
+    while (start < stop) {
+	asm volatile ("fc %0" :: "r"(start));
+	start += 32;
+    }
+    asm volatile (";;sync.i;;srlz.i;;");
+}
+#define dma_flush_range(start, end) flush_icache_range(start, end)
+#define qemu_cache_utils_init(envp) do { (void) (envp); } while (0)
 #else
+static inline void dma_flush_range(unsigned long start, unsigned long stop)
+{
+}
 #define qemu_cache_utils_init(envp) do { (void) (envp); } while (0)
 #endif
 
Index: qemu-kvm/cutils.c
===================================================================
--- qemu-kvm.orig/cutils.c
+++ qemu-kvm/cutils.c
@@ -165,6 +165,11 @@
     }
 }
 
+/*
+ * No dma flushing needed here, as the aio code will call dma_bdrv_cb()
+ * on completion as well, which will result in a call to
+ * dma_bdrv_unmap() which will do the flushing ....
+ */
 void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void *buf, size_t count)
 {
     const uint8_t *p = (const uint8_t *)buf;
Index: qemu-kvm/dma-helpers.c
===================================================================
--- qemu-kvm.orig/dma-helpers.c
+++ qemu-kvm/dma-helpers.c
@@ -148,6 +148,10 @@
     dbs->is_write = is_write;
     dbs->bh = NULL;
     qemu_iovec_init(&dbs->iov, sg->nsg);
+    /*
+     * DMA flushing is handled in dma_bdrv_cb() calling dma_bdrv_unmap()
+     * so we don't need to do that here.
+     */
     dma_bdrv_cb(dbs, 0);
     if (!dbs->acb) {
         qemu_aio_release(dbs);
Index: qemu-kvm/exec.c
===================================================================
--- qemu-kvm.orig/exec.c
+++ qemu-kvm/exec.c
@@ -35,6 +35,7 @@
 #include "cpu.h"
 #include "exec-all.h"
 #include "qemu-common.h"
+#include "cache-utils.h"
 
 #if !defined(TARGET_IA64)
 #include "tcg.h"
@@ -3385,6 +3386,8 @@
 void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
                                int is_write, target_phys_addr_t access_len)
 {
+    unsigned long flush_len = (unsigned long)access_len;
+
     if (buffer != bounce.buffer) {
         if (is_write) {
             ram_addr_t addr1 = qemu_ram_addr_from_host(buffer);
@@ -3402,7 +3405,9 @@
                 }
                 addr1 += l;
                 access_len -= l;
-            }
+	    }
+	    dma_flush_range((unsigned long)buffer,
+			    (unsigned long)buffer + flush_len);
         }
         return;
     }
Index: qemu-kvm/target-ia64/cpu.h
===================================================================
--- qemu-kvm.orig/target-ia64/cpu.h
+++ qemu-kvm/target-ia64/cpu.h
@@ -73,7 +73,6 @@
  * These ones really should go to the appropriate tcg header file, if/when
  * tcg support is added for ia64.
  */
-void flush_icache_range(unsigned long start, unsigned long stop);
 void tcg_dump_info(FILE *f,
                    int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
 
Index: qemu-kvm/target-ia64/fake-exec.c
===================================================================
--- qemu-kvm.orig/target-ia64/fake-exec.c
+++ qemu-kvm/target-ia64/fake-exec.c
@@ -41,15 +41,6 @@
     return;
 }
 
-void flush_icache_range(unsigned long start, unsigned long stop)
-{
-    while (start < stop) {
-	asm volatile ("fc %0" :: "r"(start));
-	start += 32;
-    }
-    asm volatile (";;sync.i;;srlz.i;;");
-}
-
 int cpu_restore_state(TranslationBlock *tb,
                       CPUState *env, unsigned long searched_pc,
                       void *puc)