Re: [Qemu-devel] [PATCH 15/27] iommu: Add IOMMU index argument to notifier APIs

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> Add support for multiple IOMMU indexes to the IOMMU notifier APIs.
> When initializing a notifier with iommu_notifier_init(), the caller
> must pass the IOMMU index that it is interested in. When a change
> happens, the IOMMU implementation must pass
> memory_region_notify_iommu() the IOMMU index that has changed and
> that notifiers must be called for.
>
> IOMMUs which support only a single index don't need to change.
> Callers which only really support working with IOMMUs with a single
> index can use the result of passing MEMTXATTRS_UNSPECIFIED to
> memory_region_iommu_attrs_to_index().
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/exec/memory.h    | 11 ++++++++++-
>  hw/i386/intel_iommu.c    |  4 ++--
>  hw/ppc/spapr_iommu.c     |  2 +-
>  hw/s390x/s390-pci-inst.c |  4 ++--
>  hw/vfio/common.c         |  6 +++++-
>  hw/virtio/vhost.c        |  7 ++++++-
>  memory.c                 |  8 +++++++-
>  7 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index f6226fb263..4e6b125add 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -71,6 +71,7 @@ struct IOMMUTLBEntry {
>      hwaddr           iova;
>      hwaddr           translated_addr;
>      hwaddr           addr_mask;  /* 0xfff = 4k translation */
> +    int              iommu_idx;
>      IOMMUAccessFlags perm;
>  };
>
> @@ -98,18 +99,21 @@ struct IOMMUNotifier {
>      /* Notify for address space range start <= addr <= end */
>      hwaddr start;
>      hwaddr end;
> +    int iommu_idx;

Its a minor thing but are we ever expecting iommu_idx to ever be
negative?

> --- a/memory.c
> +++ b/memory.c
> @@ -1802,6 +1802,9 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr,
>      iommu_mr = IOMMU_MEMORY_REGION(mr);
>      assert(n->notifier_flags != IOMMU_NOTIFIER_NONE);
>      assert(n->start <= n->end);
> +    assert(n->iommu_idx >= 0 &&
> +           n->iommu_idx < memory_region_iommu_num_indexes(iommu_mr));
> +

And then this would only have to assert we haven't exceeded the upper bound.

--
Alex Bennée