Re: [Qemu-devel] [RFC PATCH v2 00/12] iommu: add MemTxAttrs argument to IOMMU translate function

[Paolo Bonzini]

On 17/05/2018 12:48, Peter Maydell wrote:
> So I had an idea this morning for a slightly different way to approach
> this, which is that we add a concept of an iommu_idx, analogous to
> our existing TCG mmu_idx. Basically an iommu_idx represents a
> page table (kind of like what Arm calls a "translation regime"), so
> that for any particular iommu index and input address the output
> address and permissions are always the same. For the memory
> protection controller there would be two iommu_idxes, one for
> secure and one for non-secure.
> 
> The API changes would be something like:
>  * new method get_num_indexes() which returns the number of iommu indexes
>    this IOMMU implements (default implementation: return 1)
>  * translate method takes an iommu index (and not the txattrs)
>  * new method index_from_attrs() which takes a MemTxAttrs and
>    returns the iommu index that should be used (default implementation:
>    always return 0)
>  * memory_region_register_iommu_notifier() takes an iommu index
>  * the default 'replay' method does "for each supported index,
>    for each address, call @translate"
>  * vfio and vhost can register their notifiers using the index
>    returned by index_from_attrs(MEMTXATTRS_UNSPECIFIED)
>  * maybe they could call get_num_indexes() and bail out if the
>    IOMMU has support for multiple indexes?
>  * maybe they could be enhanced to support tracking multiple
>    page tables if necessary in future
> 
> I haven't worked through the details yet, but this seems to me
> more flexible than working directly with txattrs. It also means
> it's harder to accidentally write an iommu implementation that
> looks at more fields in the txattrs than its notifier interface
> claims are significant to it.

Yes, this also sounds good.  It does have the same issue for VFIO that
get_num_indexes() would be called too late to fail (and again, in a
place where it's hard to fail).

Maybe the index count and the index-from/to-attrs translation should be
static (index-to-attrs could use the same pair of MemTxAttrs for "which
bits matter" and "what value should they have"), so that VFIO can
inspect it and decide if it's okay to proceed with e.g. the first iommu_idx?

Thanks,

Paolo