From patchwork Mon May 25 13:12:49 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jes Sorensen X-Patchwork-Id: 25838 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n4PDD39U010331 for ; Mon, 25 May 2009 13:13:04 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753382AbZEYNMz (ORCPT ); Mon, 25 May 2009 09:12:55 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752571AbZEYNMy (ORCPT ); Mon, 25 May 2009 09:12:54 -0400 Received: from relay2.sgi.com ([192.48.179.30]:53526 "EHLO relay.sgi.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753216AbZEYNMw (ORCPT ); Mon, 25 May 2009 09:12:52 -0400 Received: from eye3.emea.sgi.com (eye3.emea.sgi.com [144.253.156.24]) by relay2.corp.sgi.com (Postfix) with ESMTP id C778030405F; Mon, 25 May 2009 06:12:52 -0700 (PDT) Message-ID: <4A1A9951.5030408@sgi.com> Date: Mon, 25 May 2009 15:12:49 +0200 From: Jes Sorensen User-Agent: Thunderbird 2.0.0.21 (X11/20090320) MIME-Version: 1.0 To: Avi Kivity CC: "Zhang, Xiantao" , "kvm@vger.kernel.org" , "kvm-ia64@vger.kernel.org" , Hollis Blanchard Subject: Re: [PATCH] qemu-kvm: Flush icache after dma operations for ia64 References: <706158FABBBA044BAD4FE898A02E4BC236B1935F@pdsmsx503.ccr.corp.intel.com> <4A0807DC.6080109@redhat.com> <4A1A8015.8080908@redhat.com> In-Reply-To: <4A1A8015.8080908@redhat.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org 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 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 Signed-off-by: Jes Sorensen --- 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)