Re: [Qemu-arm] [PATCH 2/2] hw/arm/smmuv3: Implement dummy replay

[Peter Maydell]

On Fri, 14 Jun 2019 at 14:40, Auger Eric <address@hidden> wrote:
>
> Hi Peter,
>
> On 6/14/19 3:26 PM, Peter Maydell wrote:
> > On Tue, 11 Jun 2019 at 15:29, Eric Auger <address@hidden> wrote:
> >>
> >> On ARM we currently do not support VFIO-PCI devices protected
> >> by the IOMMU. Any attempt to run such use case results in this
> >> kind of warning:
> >>
> >> "-device vfio-pci,host=0004:01:00.0,id=hostdev0,bus=pci.1,addr=0x0:
> >> warning: SMMUv3 does not support notification on MAP: device vfio-pci
> >> will not function properly".

> >> +static inline void
> >> +smmuv3_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n)
> >> +{
> >> +}
> >
> > This doesn't seem like a valid implementation of the replay
> > method to me. The API doc comment says
> >      * The default implementation of memory_region_iommu_replay() is to
> >      * call the IOMMU translate method for every page in the address space
> >      * with flag == IOMMU_NONE and then call the notifier if translate
> >      * returns a valid mapping. If this method is implemented then it
> >      * overrides the default behaviour, and must provide the full semantics
> >      * of memory_region_iommu_replay(), by calling @notifier for every
> >      * translation present in the IOMMU.
> >
> > This empty function is definitely not going to call the notifier
> > for every IOMMU translation...
> The situation is a bit odd. SMMUv3 is not integrated with VFIO so VFIO
> devices will not work anyway (we are not able to notify on MAP). There
> is a warning already reporting the issue. However the default
> implementation of memory_region_iommu_replay() prevents the guest from
> booting. So what would you advise?

I dunno, but if the API isn't supposed to behave the way we've
documented it to, we should fix the documentation... Since
the only user of memory_region_iommu_replay() is the vfio code
I guess we can define it however is most convenient for vfio,
but we should document what the method has to do to make things
work.

PS: we have a memory_region_iommu_replay_all() which currently
appears to be not used by anybody, could we get rid of it?

thanks
-- PMM