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

From: "Zhang, Xiantao" <xiantao.zhang@intel.com>
To: Avi Kivity <avi@redhat.com>, "Zhang, Yang" <yang.zhang@intel.com>
Cc: "kvm-ia64@vger.kernel.org" <kvm-ia64@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: RE: [PATCH] KVM: Qemu: Flush i-cache after ide-dma operation in IA64
Date: Fri, 3 Apr 2009 09:31:48 +0800	[thread overview]
Message-ID: <706158FABBBA044BAD4FE898A02E4BC2326A0CC6@pdsmsx503.ccr.corp.intel.com> (raw)
In-Reply-To: <49D47D7F.2040904@redhat.com>

Avi Kivity wrote:
> 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.
>> 
>> 
>> 
>> 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;;");
>> +}
>> 
> 
> What about smp?
> 
> I'm surprised the guest doesn't do this by itself?
> 
>> 
>>  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 the wrong place to put this.  Once we stop bouncing
> scatter/gather DMA, we will no longer call this function.

This patch intends to fix the issue before adopting scatter/gather DMA mode. But if we want to keep this funtion, had better to pick it to avoid such issues in future. 

> The correct place is either in the device code itself, or in the dma
> api (dma-helpers.c).

Maybe dma-helpers.c 
Xiantao