Re: [Qemu-devel] [PATCH v7 5/5] IOMMU: add specific null implementation of iommu_replay to intel_iommu

[Aviv B.D.]

On Mon, Nov 28, 2016 at 6:36 PM Alex Williamson <address@hidden>
wrote:

> On Mon, 28 Nov 2016 17:51:55 +0200
> "Aviv B.D" <address@hidden> wrote:
>
> > From: "Aviv Ben-David" <address@hidden>
> >
> > Currently the implementation preventing VFIO to work together with
> > intel_iommu.
> >
> > Signed-off-by: Aviv Ben-David <address@hidden>
> > ---
> >  hw/i386/intel_iommu.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index d872969..0787714 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -2453,6 +2453,12 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState
> *s, PCIBus *bus, int devfn)
> >      return vtd_dev_as;
> >  }
> >
> > +static void vtd_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n,
> > +                             bool is_write){
> > +    error_report("VFIO use with intel_iommu is currently not
> supported.");
>
> It doesn't make sense to call out vfio here, this is a deficiency of
> intel-iommu, not vfio.  vfio is just trying to use QEMU's iommu api.
> vfio is currently the only caller, but that could change and just adds
> maintenance effort to scrub this error message at a later time.  We
> could make the whole path support an error return to allow vfio to
> report the error, we could add a name field in the IOMMUNotifier, or we
> could just make the error more generic.  Thanks,
>
> Alex
>
> Sure, I'll change the message to something generic.

Aviv.


> > +    exit(1);
> > +}
> > +
> >  /* Do the initialization. It will also be called when reset, so pay
> >   * attention when adding new initialization stuff.
> >   */
> > @@ -2467,6 +2473,7 @@ static void vtd_init(IntelIOMMUState *s)
> >
> >      s->iommu_ops.translate = vtd_iommu_translate;
> >      s->iommu_ops.notify_flag_changed = vtd_iommu_notify_flag_changed;
> > +    s->iommu_ops.replay = vtd_iommu_replay;
> >      s->root = 0;
> >      s->root_extended = false;
> >      s->dmar_enabled = false;
>
>