Hi Peter,
On 6/14/19 3:45 PM, Peter Maydell wrote:
> On Fri, 14 Jun 2019 at 14:40, Auger Eric <eric.au...@redhat.com> wrote:
>>
>> Hi Peter,
>>
>> On 6/14/19 3:26 PM, Peter Maydell wrote:
>>> On Tue, 11 Jun 2019 at 15:29, Eric Auger <eric.au...@redhat.com> 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...
fair enough
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.
OK I need to think about it. Maybe an alternative is to call
memory_region_iommu_replay() only when it makes sense.
>
> PS: we have a memory_region_iommu_replay_all() which currently
> appears to be not used by anybody, could we get rid of it?
I will do that
Thanks
Eric
>
> thanks
> -- PMM
>
