On Wed, 2009-04-22 at 09:59 +0800, Zhang, Xiantao wrote: > Hollis Blanchard wrote: > > On Tue, 2009-04-21 at 20:21 +0300, Avi Kivity wrote: > >> Hollis Blanchard wrote: > >>> On Tue, 2009-04-21 at 10:08 +0000, Avi Kivity wrote: > >>> > >>>> From: Xiantao Zhang <xiantao.zhang@xxxxxxxxx> > >>>> > >>>> ia64 depends on platform provides synced idcache after DMA > >>>> operation. For virtual dma operations in qemu, it also need to > >>>> provide similar machanism. > >>>> > >>>> Signed-off-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx> > >>>> Signed-off-by: Avi Kivity <avi@xxxxxxxxxx> > >>>> > >>> > >>> I pointed out some problems with this patch a couple weeks ago: > >>> http://article.gmane.org/gmane.comp.emulators.kvm.devel/30475 > >>> > >>> Why was it still applied? > >>> > >> > >> So I can release kvm-85. I don't like it either. > > Hi, Hollis > > > You don't need to commit it: > > 1. KVM on ia64 has made it for this long without that patch. > > No, I don't know you had been tracing the upstream code, but I had to > say this API is introduced recently, and it breaks kvm/ia64 since DMA > adopts it to access guest memory. > > > 2. The ia64 guys didn't even reply, much less revise the patch. > > Sorry to forget to reply your mail! > > > 3. The patch quality is clearly not high enough to be committed > > to qemu (at least, one would hope), so you're just going to > > have to revert it to apply a better fix anyways. > > Could you provide a better fix ? :) Sorry, even if I wanted to, I couldn't test it. > > I don't see why all ia64 patches had to be applied for kvm-85, > > regardless of merit. > > We are working on kvm-85 relase from rc1, but a lot of changes related > to DMA emulation in upstream Qemu introduced some problems for ia64. I > don't see any side-effect about other archs with this patch, because > this fucntion is defined as a blank function. > As you suggested, ifdef or kvm_enabled wrapper maybe better, but you > may need to check the declaration of this function has been wrapped by > "ifdef __ia64__ ", so I don't think it is a must to add the wrapper > again. As I said before, "synchronize instruction and data caches" is a meaningful and important operation on architectures other than IA64. *However*, that operation is not relevant on those architectures in this particular location. As far as I know, on architectures with non-coherent i/d caches, invalidating the icache during DMA writes is unique to IA64. Please re-read my original mail on this subject and let me know if my objections are still unclear. -- 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
