On Mon, Jul 11, 2016 at 8:32 AM, Peter Xu <pet...@redhat.com> wrote: > 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 <pet...@redhat.com> >> > --- >> > 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.
I think Jan is mainly asking about where the method 'find_add_as()' is being used. Unless I'm too missing something It doesn't seem to be used anywhere outside the implementing class. > > 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