On Mon, Jul 11, 2016 at 10:41 AM, Peter Xu <pet...@redhat.com> wrote: > On Mon, Jul 11, 2016 at 10:16:11AM +0300, David Kiarie wrote: >> On Mon, Jul 11, 2016 at 9:49 AM, Peter Xu <pet...@redhat.com> wrote: >> > On Mon, Jul 11, 2016 at 08:46:12AM +0300, David Kiarie wrote: >> >> 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. >> >> Hi >> > >> > This patch can be dropped. I was just not sure whether it's the >> > correct time to do that. Anyway, we may still need one more patch to >> > cleanup this in the future, as I have mentioned in the previous email. >> >> I think there is a misunderstanding here. >> >> We (me and Jan) are basically asking did you plan to use "find_add_as" >> somewhere and may be missed it ? Why does x86-iommu class need >> "find_add_as" ? >> The reason is I'm not able to receive IOAPIC >> interrupts with AMD IOMMU basing my work on your code. We thought >> you'd clarify on where "find_add_as" is used or how you plan to use >> it. > > As mentioned in previous email, before Marcel's patches, > vtd_host_dma_iommu() was named q35_host_dma_iommu().
Okay, that solves it - _before_ the adoption of '-device iommu' so you're right, this is not needed anymore. At that time, I > need "find_add_as" to let Q35 codes get rid of direct calls to VT-d > (so that pc_q35.c will not need to include "intel_iommu.h" any more, > instead, it should include "x86-iommu.h"). Also, that interface is > prepared for future AMD as well. However, now AMD (you patches) are > directly calling pci_setup_iommu(). I am not sure whether you were > using it from the beginning, but IIUC as long as you are using > pci_setup_iommu() interface, we should be able to avoid providing > find_add_as any more. So I think this patch is indeed okay to be > dropped... Please kindly correct me if I missed anything. :) > > The only reason that we keep this patch (as far as I can think of..) > is that mst has done some testing on v11 and I'm not sure whether we'd > better keep it untouched if we are going to merge it (fixing commit > message does not count, right?). But I'd say I'm not familiar with how > maintainers manage codes to be merged... Maybe different maintainers > have their own flavor on this matter? I don't know. Anyway, these are > only my wild guess. > > For the problem you have encountered with IOAPIC, do you think it's > related to this patch? Have you tried to add some logs in e.g. > ioapic_service() to see what's wrong in there? > > (Maybe we need some general trace logs in IOAPIC codes?...) > > -- peterx