diff mbox

qemu-kvm: Flush icache after dma operations for ia64

Message ID 706158FABBBA044BAD4FE898A02E4BC236B1935F@pdsmsx503.ccr.corp.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang, Xiantao May 11, 2009, 10:20 a.m. UTC
Avi
     This is the new patch for icache flush after DMA emualtion for ia64, and it should address Hollis's comments. 
Xiantao

From 60a27e2ea9758c97e974aa5bb1925ad4ed045c5f Mon Sep 17 00:00:00 2001
From: Xiantao Zhang <xiantao.zhang@intel.com>
Date: Mon, 11 May 2009 18:04:15 +0800
Subject: [PATCH] qemu-kvm: Flush icache after dma operations for ia64
 
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.
 
Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
---
 cache-utils.h           |   10 ++++++++++
 cutils.c                |   14 ++++++++++++++
 dma-helpers.c           |   21 +++++++++++++++++++++
 exec.c                  |    9 +++++++--
 target-ia64/cpu.h       |    1 -
 target-ia64/fake-exec.c |    9 ---------
 6 files changed, 52 insertions(+), 12 deletions(-)

Comments

Avi Kivity May 11, 2009, 11:11 a.m. UTC | #1
Zhang, Xiantao wrote:
>  Avi
>      This is the new patch for icache flush after DMA emualtion for ia64, and it should address Hollis's comments. 
> Xiantao
>
> From 60a27e2ea9758c97e974aa5bb1925ad4ed045c5f Mon Sep 17 00:00:00 2001
> From: Xiantao Zhang <xiantao.zhang@intel.com>
> Date: Mon, 11 May 2009 18:04:15 +0800
> Subject: [PATCH] qemu-kvm: Flush icache after dma operations for ia64
>  
> 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.
>  
>  
> +#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 qemu_cache_utils_init(envp) do { (void) (envp); } while (0)
>  #else
>  #define qemu_cache_utils_init(envp) do { (void) (envp); } while (0)
>  #endif
> diff --git a/cutils.c b/cutils.c
> index a1652ab..6b7d506 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -25,6 +25,10 @@
>  #include "host-utils.h"
>  #include <assert.h>
>  
> +#ifdef __ia64__
> +#include "cache-utils.h"
> +#endif
>   

#includes should be unconditional.

> +
>  void pstrcpy(char *buf, int buf_size, const char *str)
>  {
>      int c;
> @@ -176,6 +180,16 @@ void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void *buf, size_t count)
>          if (copy > qiov->iov[i].iov_len)
>              copy = qiov->iov[i].iov_len;
>          memcpy(qiov->iov[i].iov_base, p, copy);
> +
> + /*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. */
> +#ifdef __ia64__
> + flush_icache_range((unsigned long)qiov->iov[i].iov_base,
> +                  (unsigned long)(qiov->iov[i].iov_base + copy));
> +#endif
>   

Instead of the #ifdef, please add a flush_icache_range_after_dma() 
function which does flush_icache_range() for ia64 and nothing for other 
architectures.  This avoids the #ifdef everywhere, and you will only 
need one copy of the comment.

I don't think you need to flush here.  Instead, put the flush in 
cpu_physical_memory_unmap().  Every dma access should invoke that.  If 
you find a path which doesn't, it should be fixed.

>  
> +#ifdef __ia64__
> +#include "cache-utils.h"
> +#endif
>   

Unconditional #include.

> +
>  static AIOPool dma_aio_pool;
>  
>  void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint)
> @@ -149,6 +153,23 @@ static BlockDriverAIOCB *dma_bdrv_io(
>      dbs->bh = NULL;
>      qemu_iovec_init(&dbs->iov, sg->nsg);
>      dma_bdrv_cb(dbs, 0);
> +
> +    /*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. */
> +#ifdef __ia64__
> +    int i;
> +    QEMUIOVector *qiov;
> +    if (!is_write) {
> +        qiov = &dbs->iov;
> +        for (i = 0; i < qiov->niov; ++i) {
> +           flush_icache_range((unsigned long)qiov->iov[i].iov_base,
> +                 (unsigned long)(qiov->iov[i].iov_base + qiov->iov[i].iov_len));
> + }
> +    }
> +#endif
>   

If you move the flush to cpu_physical_memory_unmap(), this can go away.

> +
>      if (!dbs->acb) {
>          qemu_aio_release(dbs);
>          return NULL;
> diff --git a/exec.c b/exec.c
> index 29c91fb..170ede1 100644
> --- a/exec.c
> +++ b/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"
> @@ -3402,8 +3403,12 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
>                  }
>                  addr1 += l;
>                  access_len -= l;
> -            }
> -        }
> +     }
> +#ifdef TARGET_IA64
> +     flush_icache_range((unsigned long)buffer,
> +       (unsigned long)buffer + access_len);
> +#endif
> +       }
>   

Whitespace damage.

access_len here is zero, so this does nothing.
Jes Sorensen May 25, 2009, 10:55 a.m. UTC | #2
Hi,

Looking at Avi's comments, I came up with a slightly modified version
of this patch, which avoids the #ifdef __ia64__ problem by introducing
dma_flush_range().

I have made it a noop on non ia64, but it may need to be changed for
PPC?  Hollis would you check if you guys need this too?

It also fixes the problem of trying to flush a zero sized block in
cpu_physical_memory_unmap().

Xiantao, are you ok with this version?

Cheers,
Jes

[...]

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/cache-utils.h b/cache-utils.h
index b45fde4..db850ba 100644
--- a/cache-utils.h
+++ b/cache-utils.h
@@ -34,6 +34,16 @@  static inline void flush_icache_range(unsigned long start, unsigned long stop)
     asm volatile ("isync" : : : "memory");
 }
 
+#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 qemu_cache_utils_init(envp) do { (void) (envp); } while (0)
 #else
 #define qemu_cache_utils_init(envp) do { (void) (envp); } while (0)
 #endif
diff --git a/cutils.c b/cutils.c
index a1652ab..6b7d506 100644
--- a/cutils.c
+++ b/cutils.c
@@ -25,6 +25,10 @@ 
 #include "host-utils.h"
 #include <assert.h>
 
+#ifdef __ia64__
+#include "cache-utils.h"
+#endif
+
 void pstrcpy(char *buf, int buf_size, const char *str)
 {
     int c;
@@ -176,6 +180,16 @@  void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void *buf, size_t count)
         if (copy > qiov->iov[i].iov_len)
             copy = qiov->iov[i].iov_len;
         memcpy(qiov->iov[i].iov_base, p, copy);
+
+ /*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. */
+#ifdef __ia64__
+ flush_icache_range((unsigned long)qiov->iov[i].iov_base,
+                  (unsigned long)(qiov->iov[i].iov_base + copy));
+#endif
+
         p     += copy;
         count -= copy;
     }
diff --git a/dma-helpers.c b/dma-helpers.c
index f9eb224..b895099 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -10,6 +10,10 @@ 
 #include "dma.h"
 #include "block_int.h"
 
+#ifdef __ia64__
+#include "cache-utils.h"
+#endif
+
 static AIOPool dma_aio_pool;
 
 void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint)
@@ -149,6 +153,23 @@  static BlockDriverAIOCB *dma_bdrv_io(
     dbs->bh = NULL;
     qemu_iovec_init(&dbs->iov, sg->nsg);
     dma_bdrv_cb(dbs, 0);
+
+    /*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. */
+#ifdef __ia64__
+    int i;
+    QEMUIOVector *qiov;
+    if (!is_write) {
+        qiov = &dbs->iov;
+        for (i = 0; i < qiov->niov; ++i) {
+           flush_icache_range((unsigned long)qiov->iov[i].iov_base,
+                 (unsigned long)(qiov->iov[i].iov_base + qiov->iov[i].iov_len));
+ }
+    }
+#endif
+
     if (!dbs->acb) {
         qemu_aio_release(dbs);
         return NULL;
diff --git a/exec.c b/exec.c
index 29c91fb..170ede1 100644
--- a/exec.c
+++ b/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"
@@ -3402,8 +3403,12 @@  void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
                 }
                 addr1 += l;
                 access_len -= l;
-            }
-        }
+     }
+#ifdef TARGET_IA64
+     flush_icache_range((unsigned long)buffer,
+       (unsigned long)buffer + access_len);
+#endif
+       }
         return;
     }
     if (is_write) {
diff --git a/target-ia64/cpu.h b/target-ia64/cpu.h
index e002d56..fb51463 100644
--- a/target-ia64/cpu.h
+++ b/target-ia64/cpu.h
@@ -71,7 +71,6 @@  static inline int cpu_mmu_index (CPUState *env)
  * 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, ...));
 
diff --git a/target-ia64/fake-exec.c b/target-ia64/fake-exec.c
index c11cc32..8d6ded0 100644
--- a/target-ia64/fake-exec.c
+++ b/target-ia64/fake-exec.c
@@ -41,15 +41,6 @@  void tcg_dump_info(FILE *f,
     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)