From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38711) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMTpW-0002ir-LI for qemu-devel@nongnu.org; Mon, 11 Jul 2016 01:32:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bMTpS-0008Cm-FJ for qemu-devel@nongnu.org; Mon, 11 Jul 2016 01:32:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48823) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMTpS-0008CY-A5 for qemu-devel@nongnu.org; Mon, 11 Jul 2016 01:32:26 -0400 Date: Mon, 11 Jul 2016 13:32:19 +0800 From: Peter Xu Message-ID: <20160711053219.GB3204@pxdev.xzpeter.org> References: <1467706769-12505-1-git-send-email-peterx@redhat.com> <1467706769-12505-5-git-send-email-peterx@redhat.com> <5780B278.7020001@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <5780B278.7020001@web.de> Subject: Re: [Qemu-devel] [PATCH v11 04/28] x86-iommu: q35: generalize find_add_as() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: qemu-devel@nongnu.org, imammedo@redhat.com, rth@twiddle.net, ehabkost@redhat.com, jasowang@redhat.com, marcel@redhat.com, mst@redhat.com, pbonzini@redhat.com, rkrcmar@redhat.com, alex.williamson@redhat.com, wexu@redhat.com, davidkiarie4@gmail.com On Sat, Jul 09, 2016 at 10:14:48AM +0200, Jan Kiszka wrote: > On 2016-07-05 10:19, Peter Xu wrote: > > Remove VT-d calls in common q35 codes. Instead, we provide a general > > find_add_as() for x86-iommu type. > > > > Signed-off-by: Peter Xu > > --- > > hw/i386/intel_iommu.c | 15 ++++++++------- > > include/hw/i386/intel_iommu.h | 5 ----- > > include/hw/i386/x86-iommu.h | 3 +++ > > 3 files changed, 11 insertions(+), 12 deletions(-) > > You claim to remove something from "common q35 code", but I don't see > changes to it. Instead, the patch introduces a method that seems to > remain unused outside the implementing class (I just grep'ed your tree). > Anything missing? Right. The commit message lost its point after I did the rebase to Marcel's "-device intel_iommu" patches... Thanks for pointing it out. Before the rebase, there is one q35_host_dma_iommu() in pc_q35.c, and originally this patch did remove something from q35. While in Marcel's commit (621d983a1f), q35_host_dma_iommu() is renamed to vtd_host_dma_iommu(), and it's put inside intel_iommu.c. After that, this commit message stopped making sense. So I think at least the commit message of this patch could be fixed into something like: "Introduce common find_add_as() interface for x86-iommu." And if I now see this... A better solution is to provide a more common interface directly in x86-iommu.c to find address spaces, and let Intel/AMD IOMMUs share this functionality. After all, we are doing merely the same thing to maintain namespaces in both Intel/AMD IOMMUs (vtd_find_add_as() and bridge_host_amdvi()). So, do you (and mst?) think I should respin to a v12, or we can first fix commit message of this patch, then I post another patch basd on this series for a better cleanup? Thanks, -- peterx