From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36676) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T1ZOv-00073J-MV for qemu-devel@nongnu.org; Wed, 15 Aug 2012 04:56:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T1ZOu-0003bb-KN for qemu-devel@nongnu.org; Wed, 15 Aug 2012 04:56:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13682) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T1ZOu-0003bW-B3 for qemu-devel@nongnu.org; Wed, 15 Aug 2012 04:56:28 -0400 Message-ID: <502B6434.1080604@redhat.com> Date: Wed, 15 Aug 2012 11:56:20 +0300 From: Avi Kivity MIME-Version: 1.0 References: <20120801050241.22163.78549.stgit@bling.home> <20120801051814.22163.66621.stgit@bling.home> <502A7495.2020501@redhat.com> <1344964988.4683.276.camel@ul30vt.home> In-Reply-To: <1344964988.4683.276.camel@ul30vt.home> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] vfio: vfio-pci device assignment driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: aik@ozlabs.ru, aliguori@us.ibm.com, qemu-devel@nongnu.org, kvm@vger.kernel.org On 08/14/2012 08:23 PM, Alex Williamson wrote: > >> Unrelated nit: memcmp() doesn't return a boolean or a count, so >> !memcmp() is really unintuitive, at least to me. > > I figure we're all pretty used to it growing up on !strcmp though. I hate that one too. >> > + >> > +/* XXX This should move to msi.c */ >> >> Well? > > Just marking a todo item. I'll change it formally to TODO. I think > there are a few interfaces to msi.c that probably needs some rethinking > for device assignment. When they're small like this it seems easier to > have the user in tree first. I prefer them in the right place but I don't insist. >> > + >> > + if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) != >> > + (section->offset_within_region & ~TARGET_PAGE_MASK))) { >> > + error_report("%s received unaligned region\n", __func__); >> >> Is it really an error? I think you can just add the condition to >> skipped_section. > > I had left this in as paranoia for myself that I wanted to see if this > actually happens. I want to assume that our TARGET_PAGE_ALIGNED > offset_within_address_space results in an aligned ram pointer. If one > is aligned different from the other we're kinda screwed trying to map it > into the iommu. So far I haven't seen it. Thanks for the feedback, We could have a sub-page RAM region (perhaps inserted as a mapped BAR from some emulated device, or from vfio if/when it grows that capability). But you're right, it really is an error, we can't just ignore it. So the current code is right. -- error compiling committee.c: too many arguments to function