Re: [PATCH] KVM: Qemu: Flush i-cache after ide-dma operation in IA64

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2009-04-02 at 10:01 +0800, Zhang, Yang wrote:
> The data from dma will include instructions. In order to exeuting the right
> instruction, we should to flush the i-cache to ensure those data can be see 
> by cpu.
> 
> Signed-off-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
> Signed-off-by: Yang Zhang <yang.zhang@xxxxxxxxx>
> ---
> 
> 
> diff --git a/qemu/cache-utils.h b/qemu/cache-utils.h
> index b45fde4..5e11d12 100644
> --- a/qemu/cache-utils.h
> +++ b/qemu/cache-utils.h
> @@ -33,8 +33,22 @@ static inline void flush_icache_range(unsigned long start, unsigned long stop)
>      asm volatile ("sync" : : : "memory");
>      asm volatile ("isync" : : : "memory");
>  }
> +#define qemu_sync_idcache flush_icache_range
> +#else
>  
> +#ifdef __ia64__
> +static inline void qemu_sync_idcache(unsigned long start, unsigned long stop)
> +{
> +    while (start < stop) {
> +	    asm volatile ("fc %0" :: "r"(start));
> +	    start += 32;
> +    }
> +    asm volatile (";;sync.i;;srlz.i;;");
> +}
>  #else
> +static inline void qemu_sync_idcache(unsigned long start, unsigned long stop)
> +#endif
> +
>  #define qemu_cache_utils_init(envp) do { (void) (envp); } while (0)
>  #endif

You already have flush_icache_range() in qemu/target-ia64/fake-exec.c,
so this is redundant. Moving that to cache-utils.h might make sense, but
this should be discussed on qemu-devel.

Also, flush_icache_range() is already called from
cpu_physical_memory_rw(). It would be helpful to include a comment in
this commit explaining why this path is different. (I can see that it
is, but only because I went hunting myself.)

> diff --git a/qemu/cutils.c b/qemu/cutils.c
> index 5b36cc6..7b57173 100644
> --- a/qemu/cutils.c
> +++ b/qemu/cutils.c
> @@ -23,6 +23,7 @@
>   */
>  #include "qemu-common.h"
>  #include "host-utils.h"
> +#include "cache-utils.h"
>  #include <assert.h>
>  
>  void pstrcpy(char *buf, int buf_size, const char *str)
> @@ -215,6 +216,8 @@ 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);
> +        qemu_sync_idcache((unsigned long)qiov->iov[i].iov_base, 
> +                    (unsigned long)(qiov->iov[i].iov_base + copy));
>          p     += copy;
>          count -= copy;
>      }

This is way too generic a call for this location. Other architectures
also need to synchronize L1 caches sometimes, but they don't need to do
it here. You need to comment and guard this call better (probably using
some combination of kvm_enabled() and ifdefs).

-- 
Hollis Blanchard
IBM Linux Technology Center

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

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux