From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37590) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QSBng-00088N-0h for qemu-devel@nongnu.org; Thu, 02 Jun 2011 13:35:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QSBne-0004Jg-GV for qemu-devel@nongnu.org; Thu, 02 Jun 2011 13:35:15 -0400 Received: from mail-fx0-f45.google.com ([209.85.161.45]:62423) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QSBnd-0004Jb-UZ for qemu-devel@nongnu.org; Thu, 02 Jun 2011 13:35:14 -0400 Received: by fxm2 with SMTP id 2so945484fxm.4 for ; Thu, 02 Jun 2011 10:35:12 -0700 (PDT) Sender: Eduard - Gabriel Munteanu Date: Thu, 2 Jun 2011 20:35:07 +0300 From: Eduard - Gabriel Munteanu Message-ID: <20110602173507.GA4735@localhost> References: <1307027562-3460-1-git-send-email-david@gibson.dropbear.id.au> <1307027562-3460-2-git-send-email-david@gibson.dropbear.id.au> <4DE7BDB4.1060601@twiddle.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4DE7BDB4.1060601@twiddle.net> Subject: Re: [Qemu-devel] [PATCH 01/14] Generic DMA memory access interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-devel@nongnu.org, David Gibson On Thu, Jun 02, 2011 at 09:43:32AM -0700, Richard Henderson wrote: > On 06/02/2011 08:12 AM, David Gibson wrote: > > + err = iommu->translate(dev, addr, &paddr, &plen, is_write); > > + if (err) { > > + return NULL; > > + } > > + > > + /* > > + * If this is true, the virtual region is contiguous, > > + * but the translated physical region isn't. We just > > + * clamp *len, much like cpu_physical_memory_map() does. > > + */ > > + if (plen < *len) { > > + *len = plen; > > + } > > + > > + buf = cpu_physical_memory_map(paddr, &plen, is_write); > > + *len = plen; > > + > > + /* We treat maps as remote TLBs to cope with stuff like AIO. */ > > Oh, that reminds me. There's a bug here in Eduard's original: > > PLEN is set to the maximum length of the transfer by the > translate function. What we do *not* want is to pass a > very very large region to cpu_physical_memory_map. > > The effects of this are hard to see with the AMD IOMMU, since > it is entirely page based and thus PLEN will be increased by > no more than 4k, but Alpha IOMMUs have direct-mapped translation > windows that can be up to 128GB. > > I'm unsure whether I prefer to force the translator function > to never increase PLEN (which is what I implemented in my > own branch) or whether all callers of the translate function > must be aware that the returned PLEN can increase. My latest patches seem to have fixed that: + if (plen < *len) { + *len = plen; + } + + buf = cpu_physical_memory_map(paddr, len, is_write); I think the callers of translate() should take care of clamping the length. Note the 'len' passed into translate() is write-only, so there's nothing to increase in relation to, but I get your point. The reason is we have two different behaviors to cater for: maps need to be contiguous, while plain reads/writes try to resolve the whole range of DMA addresses. In order to do that, translate() tells the caller where the translation ceases to be valid so it can make informed choices. And anyway, translate() isn't supposed to be called from other places, just the DMA abstraction. > > r~