diff mbox

[v11,04/28] x86-iommu: q35: generalize find_add_as()

Message ID 1467706769-12505-5-git-send-email-peterx@redhat.com
State New
Headers show

Commit Message

Peter Xu July 5, 2016, 8:19 a.m. UTC
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 <peterx@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(-)

Comments

Jan Kiszka July 9, 2016, 8:14 a.m. UTC | #1
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 <peterx@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?

Jan

> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 3ee5782..2ac79ab 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1910,8 +1910,10 @@ static Property vtd_properties[] = {
>  };
>  
>  
> -VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
> +static AddressSpace *vtd_find_add_as(X86IOMMUState *x86_iommu, PCIBus *bus,
> +                                     int devfn)
>  {
> +    IntelIOMMUState *s = (IntelIOMMUState *)x86_iommu;
>      uintptr_t key = (uintptr_t)bus;
>      VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key);
>      VTDAddressSpace *vtd_dev_as;
> @@ -1939,7 +1941,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
>          address_space_init(&vtd_dev_as->as,
>                             &vtd_dev_as->iommu, "intel_iommu");
>      }
> -    return vtd_dev_as;
> +    return &vtd_dev_as->as;
>  }
>  
>  /* Do the initialization. It will also be called when reset, so pay
> @@ -2032,13 +2034,11 @@ static void vtd_reset(DeviceState *dev)
>  
>  static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>  {
> -    IntelIOMMUState *s = opaque;
> -    VTDAddressSpace *vtd_as;
> +    X86IOMMUState *x86_iommu = opaque;
> +    X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(x86_iommu);
>  
>      assert(0 <= devfn && devfn <= X86_IOMMU_PCI_DEVFN_MAX);
> -
> -    vtd_as = vtd_find_add_as(s, bus, devfn);
> -    return &vtd_as->as;
> +    return x86_class->find_add_as(x86_iommu, bus, devfn);
>  }
>  
>  static void vtd_realize(DeviceState *dev, Error **errp)
> @@ -2071,6 +2071,7 @@ static void vtd_class_init(ObjectClass *klass, void *data)
>      dc->props = vtd_properties;
>      dc->hotpluggable = false;
>      x86_class->realize = vtd_realize;
> +    x86_class->find_add_as = vtd_find_add_as;
>  }
>  
>  static const TypeInfo vtd_info = {
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 0794309..e36b896 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -125,9 +125,4 @@ struct IntelIOMMUState {
>      VTDBus *vtd_as_by_bus_num[VTD_PCI_BUS_MAX]; /* VTDBus objects indexed by bus number */
>  };
>  
> -/* Find the VTD Address space associated with the given bus pointer,
> - * create a new one if none exists
> - */
> -VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn);
> -
>  #endif
> diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
> index b2401a6..581da16 100644
> --- a/include/hw/i386/x86-iommu.h
> +++ b/include/hw/i386/x86-iommu.h
> @@ -21,6 +21,7 @@
>  #define IOMMU_COMMON_H
>  
>  #include "hw/sysbus.h"
> +#include "exec/memory.h"
>  
>  #define  TYPE_X86_IOMMU_DEVICE  ("x86-iommu")
>  #define  X86_IOMMU_DEVICE(obj) \
> @@ -39,6 +40,8 @@ struct X86IOMMUClass {
>      SysBusDeviceClass parent;
>      /* Intel/AMD specific realize() hook */
>      DeviceRealize realize;
> +    /* Find/Add IOMMU address space for specific PCI device */
> +    AddressSpace *(*find_add_as)(X86IOMMUState *s, PCIBus *bus, int devfn);
>  };
>  
>  struct X86IOMMUState {
>
Peter Xu July 11, 2016, 5:32 a.m. UTC | #2
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 <peterx@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.

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
David Kiarie July 11, 2016, 5:46 a.m. UTC | #3
On Mon, Jul 11, 2016 at 8:32 AM, Peter Xu <peterx@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 <peterx@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
Peter Xu July 11, 2016, 6:49 a.m. UTC | #4
On Mon, Jul 11, 2016 at 08:46:12AM +0300, David Kiarie wrote:
> On Mon, Jul 11, 2016 at 8:32 AM, Peter Xu <peterx@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 <peterx@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.

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 see that mst is possibly not around these two days. Let me prepare a
v12 before he comes back. Thank you.

-- peterx
David Kiarie July 11, 2016, 7:16 a.m. UTC | #5
On Mon, Jul 11, 2016 at 9:49 AM, Peter Xu <peterx@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 <peterx@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 <peterx@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.

>
> I see that mst is possibly not around these two days. Let me prepare a
> v12 before he comes back. Thank you.
>
> -- peterx
Peter Xu July 11, 2016, 7:41 a.m. UTC | #6
On Mon, Jul 11, 2016 at 10:16:11AM +0300, David Kiarie wrote:
> On Mon, Jul 11, 2016 at 9:49 AM, Peter Xu <peterx@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 <peterx@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 <peterx@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(). 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
Paolo Bonzini July 11, 2016, 8:30 a.m. UTC | #7
On 11/07/2016 09:41, Peter Xu wrote:
> As mentioned in previous email, before Marcel's patches,
> vtd_host_dma_iommu() was named q35_host_dma_iommu(). 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?

You can send v12.  mst will choose whether to merge v11 or v12.

Paolo
Peter Xu July 11, 2016, 8:40 a.m. UTC | #8
On Mon, Jul 11, 2016 at 10:30:38AM +0200, Paolo Bonzini wrote:
> 
> 
> On 11/07/2016 09:41, Peter Xu wrote:
> > As mentioned in previous email, before Marcel's patches,
> > vtd_host_dma_iommu() was named q35_host_dma_iommu(). 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?
> 
> You can send v12.  mst will choose whether to merge v11 or v12.

Thanks Paolo. Will hold for one or two days and prepare v12 before the
weekend.

-- peterx
David Kiarie July 11, 2016, 9:11 a.m. UTC | #9
On Mon, Jul 11, 2016 at 10:41 AM, Peter Xu <peterx@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 <peterx@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 <peterx@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 <peterx@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
diff mbox

Patch

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3ee5782..2ac79ab 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -1910,8 +1910,10 @@  static Property vtd_properties[] = {
 };
 
 
-VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
+static AddressSpace *vtd_find_add_as(X86IOMMUState *x86_iommu, PCIBus *bus,
+                                     int devfn)
 {
+    IntelIOMMUState *s = (IntelIOMMUState *)x86_iommu;
     uintptr_t key = (uintptr_t)bus;
     VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key);
     VTDAddressSpace *vtd_dev_as;
@@ -1939,7 +1941,7 @@  VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
         address_space_init(&vtd_dev_as->as,
                            &vtd_dev_as->iommu, "intel_iommu");
     }
-    return vtd_dev_as;
+    return &vtd_dev_as->as;
 }
 
 /* Do the initialization. It will also be called when reset, so pay
@@ -2032,13 +2034,11 @@  static void vtd_reset(DeviceState *dev)
 
 static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 {
-    IntelIOMMUState *s = opaque;
-    VTDAddressSpace *vtd_as;
+    X86IOMMUState *x86_iommu = opaque;
+    X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(x86_iommu);
 
     assert(0 <= devfn && devfn <= X86_IOMMU_PCI_DEVFN_MAX);
-
-    vtd_as = vtd_find_add_as(s, bus, devfn);
-    return &vtd_as->as;
+    return x86_class->find_add_as(x86_iommu, bus, devfn);
 }
 
 static void vtd_realize(DeviceState *dev, Error **errp)
@@ -2071,6 +2071,7 @@  static void vtd_class_init(ObjectClass *klass, void *data)
     dc->props = vtd_properties;
     dc->hotpluggable = false;
     x86_class->realize = vtd_realize;
+    x86_class->find_add_as = vtd_find_add_as;
 }
 
 static const TypeInfo vtd_info = {
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 0794309..e36b896 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -125,9 +125,4 @@  struct IntelIOMMUState {
     VTDBus *vtd_as_by_bus_num[VTD_PCI_BUS_MAX]; /* VTDBus objects indexed by bus number */
 };
 
-/* Find the VTD Address space associated with the given bus pointer,
- * create a new one if none exists
- */
-VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn);
-
 #endif
diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
index b2401a6..581da16 100644
--- a/include/hw/i386/x86-iommu.h
+++ b/include/hw/i386/x86-iommu.h
@@ -21,6 +21,7 @@ 
 #define IOMMU_COMMON_H
 
 #include "hw/sysbus.h"
+#include "exec/memory.h"
 
 #define  TYPE_X86_IOMMU_DEVICE  ("x86-iommu")
 #define  X86_IOMMU_DEVICE(obj) \
@@ -39,6 +40,8 @@  struct X86IOMMUClass {
     SysBusDeviceClass parent;
     /* Intel/AMD specific realize() hook */
     DeviceRealize realize;
+    /* Find/Add IOMMU address space for specific PCI device */
+    AddressSpace *(*find_add_as)(X86IOMMUState *s, PCIBus *bus, int devfn);
 };
 
 struct X86IOMMUState {