Re: [Qemu-devel] [PATCH v2 1/1] memory.h: Improve IOMMU related documentation

[Auger Eric]

Hi Peter,

On 05/08/2018 06:25 PM, Peter Maydell wrote:
> On 1 May 2018 at 16:53, Auger Eric <address@hidden> wrote:
>> Hi Peter,
>>
>> On 05/01/2018 05:00 PM, Peter Maydell wrote:
>>> On 1 May 2018 at 15:32, Auger Eric <address@hidden> wrote:
>>>> Hi Peter,
>>>>
>>>> On 05/01/2018 12:19 PM, Peter Maydell wrote:
>>>>> +     * Once the IOMMU has returned a TLB entry, it must notify
>>>>> +     * the IOMMU's users if that TLB entry changes, using
>>>>> +     * memory_region_notify_iommu() (or, if necessary, by calling
>>>>> +     * memory_region_notify_one() for each registered notifier).
>>>> if caching mode is set (VFIO integration), On the first guest map
>>>> (invalidation on map), the IOMMU must replay the mapping (notify VFIO)
>>>> to get the 1st stage of the physical IOMMU programmed. At that moment
>>>> the IOMMU may not have returned any TLB entry.
>>>
>>> I'm afraid you've lost me. What is caching mode, how do you set it,
>>> how does the IOMMU replay the mapping? More concretely, what change
>>> needs to be made to this text?
>>
>> np. The caching mode relates to Intel IOMMU. When this bit is advertised
>> in the Intel IOMMU, it forces the IOMMU driver to invalidate TLB entries
>> also on map and not only on unmap. So when any iova is mapped, an
>> invalidation command is sent and this allows to trap when entries are
>> created. Then the IOMMU device can notify "users" that a mapping
>> happened and this mapping can be forwarded to the physical IOMMU.
>> Caching mode is set when a VFIO device is behind the IOMMU and both
>> stage 1 and stage 2 are combined in the physical IOMMU.
>>
>> what about: the IOMMU is responsible to call the registered user
>> notifiers when the associated event occurs on an IOTLB entry (map/unmap).
> 
> This runs into something I found when I was implementing the Arm
> Memory Protection Controller -- at the point when the guest changes
> the config,
do you mean the config structures (STE, CD) or the page table update?

 it doesn't have enough information to be able to call a
> "map" notifier, because the mapping depends on the memory transaction
> attributes, it's not fixed and dependent only on the address.
I am not sure I understand what you mean here. When the notifier get's
called, aren't we supposed to look for the info in the actual page table
(where memory access control attributes and others can be found at that
time, ie. ARM AP[] for instance) and send those through the notifier (as
stored in the IOTLBEnry)?

For instance in the intel iommu code, the whole table is parsed and the
replay hook is called for all valid entries.

vtd_iommu_replay/ptw/vtd_replay_hook/memory_region_notify_one

Thanks

Eric
 We can
> say "unmap" to indicate that the old information is not usable any
> more, but that's all. (Similarly if the mapping depends on the
> read/write permission then the IOMMU isn't going to be able to
> provide a new mapping to the notifier, because it doesn't have the
> information to do that.)

> 
> My use-case for notifiers (getting the TCG code to throw away its
> TLB entries for the now-stale information) doesn't require anything
> more from the notifier, but it's not clear to me what the exact
> notifier-calling requirements are.
> 
> If you can't actually use the memory transaction attributes info
> to inform your translation, then there's not much point plumbing
> them into the IOMMU.
> 
> thanks
> -- PMM
>