[PATCH v8 0/8] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 1/3: iommu changes

[PATCH v8 0/8] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 1/3: iommu changes

[Eric Auger]

This series introduces the msi-iommu api used to:

- allocate/free resources for MSI IOMMU mapping
- set the MSI iova window aperture
- map/unmap physical addresses onto MSI IOVAs.
- determine whether an msi needs to be iommu mapped
- overwrite an msi_msg PA address with its pre-allocated/mapped IOVA

Also a new iommu domain attribute, DOMAIN_ATTR_MSI_GEOMETRY is introduced
to report the MSI iova window geometry (aperture and programmability).

Currently:
- iommu driver is supposed to allocate/free MSI mapping resources
- VFIO subsystem is supposed to set the MSI IOVA aperture.
- The MSI layer is supposed to allocate/free iova mappings and overwrite
  msi_msg with IOVA at composition time

More details & context can be found at:
http://www.linaro.org/blog/core-dump/kvm-pciemsi-passthrough-armarm64/

Best Regards

Eric

Git: complete series available at
https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/v4.6-rc5-pcie-passthrough-v8

History:

v7 -> v8:
- The API is retargetted for MSI: renamed msi-iommu
  all "dma-reserved" namings removed
- now implemented upon dma-iommu (get, put, init), ie. reuse iova_cookie,
  and iova API
- msi mapping resources now are guaranteed to exist during the whole iommu
  domain's lifetime. No need to lock to garantee the cookie integrity
- removed alloc/free_reserved_reserved_iova_domain. We now have a single
  function that sets the aperture, looking like iommu_dma_init_domain.
- we now use a list instead of an RB-tree
- prot is not propagated anymore at domain creation due to the retargetting
  for MSI
- iommu_domain pointer removed from doorbell_mapping struct
- replaced DOMAIN_ATTR_MSI_MAPPING by DOMAIN_ATTR_MSI_GEOMETRY

v6 -> v7:
- fixed known lock bugs and multiple page sized slots matching
  (I only have a single MSI frame made of a single page)
- reserved_iova_cookie now pointing to a struct that encapsulates the
  iova domain handle + protection attribute passed from VFIO (Alex' req)
- 2 new functions exposed: iommu_msi_mapping_translate_msg,
  iommu_msi_mapping_desc_to_domain: not sure this is the right location/proto
  though
- iommu_put_reserved_iova now takes a phys_addr_t
- everything now is cleanup on iommu_domain destruction

RFC v5 -> patch v6:
- split to ease the review process
- in dma-reserved-api use a spin lock instead of a mutex (reported by
  Jean-Philippe)
- revisit iommu_get_reserved_iova API to pass a size parameter upon
  Marc's request
- Consistently use the page order passed when creating the iova domain.
- init reserved_binding_list (reported by Julien)

RFC v4 -> RFC v5:
- take into account Thomas' comments on MSI related patches
  - split "msi: IOMMU map the doorbell address when needed"
  - increase readability and add comments
  - fix style issues
 - split "iommu: Add DOMAIN_ATTR_MSI_MAPPING attribute"
 - platform ITS now advertises IOMMU_CAP_INTR_REMAP
 - fix compilation issue with CONFIG_IOMMU API unset
 - arm-smmu-v3 now advertises DOMAIN_ATTR_MSI_MAPPING

RFC v3 -> v4:
- Move doorbell mapping/unmapping in msi.c
- fix ref count issue on set_affinity: in case of a change in the address
  the previous address is decremented
- doorbell map/unmap now is done on msi composition. Should allow the use
  case for platform MSI controllers
- create dma-reserved-iommu.h/c exposing/implementing a new API dedicated
  to reserved IOVA management (looking like dma-iommu glue)
- series reordering to ease the review:
  - first part is related to IOMMU
  - second related to MSI sub-system
  - third related to VFIO (except arm-smmu IOMMU_CAP_INTR_REMAP removal)
- expose the number of requested IOVA pages through VFIO_IOMMU_GET_INFO
  [this partially addresses Marc's comments on iommu_get/put_single_reserved
   size/alignment problematic - which I did not ignore - but I don't know
   how much I can do at the moment]

RFC v2 -> RFC v3:
- should fix wrong handling of some CONFIG combinations:
  CONFIG_IOVA, CONFIG_IOMMU_API, CONFIG_PCI_MSI_IRQ_DOMAIN
- fix MSI_FLAG_IRQ_REMAPPING setting in GICv3 ITS (although not tested)

PATCH v1 -> RFC v2:
- reverted to RFC since it looks more reasonable ;-) the code is split
  between VFIO, IOMMU, MSI controller and I am not sure I did the right
  choices. Also API need to be further discussed.
- iova API usage in arm-smmu.c.
- MSI controller natively programs the MSI addr with either the PA or IOVA.
  This is not done anymore in vfio-pci driver as suggested by Alex.
- check irq remapping capability of the group

RFC v1 [2] -> PATCH v1:
- use the existing dma map/unmap ioctl interface with a flag to register a
  reserved IOVA range. Use the legacy Rb to store this special vfio_dma.
- a single reserved IOVA contiguous region now is allowed
- use of an RB tree indexed by PA to store allocated reserved slots
- use of a vfio_domain iova_domain to manage iova allocation within the
  window provided by the userspace
- vfio alloc_map/unmap_free take a vfio_group handle
- vfio_group handle is cached in vfio_pci_device
- add ref counting to bindings
- user modality enabled at the end of the series


Eric Auger (8):
  iommu: Add iommu_domain_msi_geometry and DOMAIN_ATTR_MSI_GEOMETRY
  iommu/arm-smmu: sets the MSI geometry to programmable
  iommu: introduce an msi cookie
  iommu/msi-iommu: initialization
  iommu/msi-iommu: iommu_msi_[get,put]_doorbell_iova
  iommu/msi-iommu: iommu_msi_domain
  iommu/msi-iommu: iommu_msi_msg_pa_to_va
  iommu/arm-smmu: get/put the msi cookie

 drivers/iommu/Kconfig       |   7 +
 drivers/iommu/Makefile      |   1 +
 drivers/iommu/arm-smmu-v3.c |  18 ++-
 drivers/iommu/arm-smmu.c    |  18 ++-
 drivers/iommu/iommu.c       |   5 +
 drivers/iommu/msi-iommu.c   | 317 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iommu.h       |  10 ++
 include/linux/msi-iommu.h   | 144 ++++++++++++++++++++
 8 files changed, 512 insertions(+), 8 deletions(-)
 create mode 100644 drivers/iommu/msi-iommu.c
 create mode 100644 include/linux/msi-iommu.h

-- 
1.9.1

[PATCH v8 0/8] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 1/3: iommu changes

[Eric Auger]

This series introduces the msi-iommu api used to:

- allocate/free resources for MSI IOMMU mapping
- set the MSI iova window aperture
- map/unmap physical addresses onto MSI IOVAs.
- determine whether an msi needs to be iommu mapped
- overwrite an msi_msg PA address with its pre-allocated/mapped IOVA

Also a new iommu domain attribute, DOMAIN_ATTR_MSI_GEOMETRY is introduced
to report the MSI iova window geometry (aperture and programmability).

Currently:
- iommu driver is supposed to allocate/free MSI mapping resources
- VFIO subsystem is supposed to set the MSI IOVA aperture.
- The MSI layer is supposed to allocate/free iova mappings and overwrite
  msi_msg with IOVA at composition time

More details & context can be found at:
http://www.linaro.org/blog/core-dump/kvm-pciemsi-passthrough-armarm64/

Best Regards

Eric

Git: complete series available at
https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/v4.6-rc5-pcie-passthrough-v8

History:

v7 -> v8:
- The API is retargetted for MSI: renamed msi-iommu
  all "dma-reserved" namings removed
- now implemented upon dma-iommu (get, put, init), ie. reuse iova_cookie,
  and iova API
- msi mapping resources now are guaranteed to exist during the whole iommu
  domain's lifetime. No need to lock to garantee the cookie integrity
- removed alloc/free_reserved_reserved_iova_domain. We now have a single
  function that sets the aperture, looking like iommu_dma_init_domain.
- we now use a list instead of an RB-tree
- prot is not propagated anymore at domain creation due to the retargetting
  for MSI
- iommu_domain pointer removed from doorbell_mapping struct
- replaced DOMAIN_ATTR_MSI_MAPPING by DOMAIN_ATTR_MSI_GEOMETRY

v6 -> v7:
- fixed known lock bugs and multiple page sized slots matching
  (I only have a single MSI frame made of a single page)
- reserved_iova_cookie now pointing to a struct that encapsulates the
  iova domain handle + protection attribute passed from VFIO (Alex' req)
- 2 new functions exposed: iommu_msi_mapping_translate_msg,
  iommu_msi_mapping_desc_to_domain: not sure this is the right location/proto
  though
- iommu_put_reserved_iova now takes a phys_addr_t
- everything now is cleanup on iommu_domain destruction

RFC v5 -> patch v6:
- split to ease the review process
- in dma-reserved-api use a spin lock instead of a mutex (reported by
  Jean-Philippe)
- revisit iommu_get_reserved_iova API to pass a size parameter upon
  Marc's request
- Consistently use the page order passed when creating the iova domain.
- init reserved_binding_list (reported by Julien)

RFC v4 -> RFC v5:
- take into account Thomas' comments on MSI related patches
  - split "msi: IOMMU map the doorbell address when needed"
  - increase readability and add comments
  - fix style issues
 - split "iommu: Add DOMAIN_ATTR_MSI_MAPPING attribute"
 - platform ITS now advertises IOMMU_CAP_INTR_REMAP
 - fix compilation issue with CONFIG_IOMMU API unset
 - arm-smmu-v3 now advertises DOMAIN_ATTR_MSI_MAPPING

RFC v3 -> v4:
- Move doorbell mapping/unmapping in msi.c
- fix ref count issue on set_affinity: in case of a change in the address
  the previous address is decremented
- doorbell map/unmap now is done on msi composition. Should allow the use
  case for platform MSI controllers
- create dma-reserved-iommu.h/c exposing/implementing a new API dedicated
  to reserved IOVA management (looking like dma-iommu glue)
- series reordering to ease the review:
  - first part is related to IOMMU
  - second related to MSI sub-system
  - third related to VFIO (except arm-smmu IOMMU_CAP_INTR_REMAP removal)
- expose the number of requested IOVA pages through VFIO_IOMMU_GET_INFO
  [this partially addresses Marc's comments on iommu_get/put_single_reserved
   size/alignment problematic - which I did not ignore - but I don't know
   how much I can do at the moment]

RFC v2 -> RFC v3:
- should fix wrong handling of some CONFIG combinations:
  CONFIG_IOVA, CONFIG_IOMMU_API, CONFIG_PCI_MSI_IRQ_DOMAIN
- fix MSI_FLAG_IRQ_REMAPPING setting in GICv3 ITS (although not tested)

PATCH v1 -> RFC v2:
- reverted to RFC since it looks more reasonable ;-) the code is split
  between VFIO, IOMMU, MSI controller and I am not sure I did the right
  choices. Also API need to be further discussed.
- iova API usage in arm-smmu.c.
- MSI controller natively programs the MSI addr with either the PA or IOVA.
  This is not done anymore in vfio-pci driver as suggested by Alex.
- check irq remapping capability of the group

RFC v1 [2] -> PATCH v1:
- use the existing dma map/unmap ioctl interface with a flag to register a
  reserved IOVA range. Use the legacy Rb to store this special vfio_dma.
- a single reserved IOVA contiguous region now is allowed
- use of an RB tree indexed by PA to store allocated reserved slots
- use of a vfio_domain iova_domain to manage iova allocation within the
  window provided by the userspace
- vfio alloc_map/unmap_free take a vfio_group handle
- vfio_group handle is cached in vfio_pci_device
- add ref counting to bindings
- user modality enabled at the end of the series


Eric Auger (8):
  iommu: Add iommu_domain_msi_geometry and DOMAIN_ATTR_MSI_GEOMETRY
  iommu/arm-smmu: sets the MSI geometry to programmable
  iommu: introduce an msi cookie
  iommu/msi-iommu: initialization
  iommu/msi-iommu: iommu_msi_[get,put]_doorbell_iova
  iommu/msi-iommu: iommu_msi_domain
  iommu/msi-iommu: iommu_msi_msg_pa_to_va
  iommu/arm-smmu: get/put the msi cookie

 drivers/iommu/Kconfig       |   7 +
 drivers/iommu/Makefile      |   1 +
 drivers/iommu/arm-smmu-v3.c |  18 ++-
 drivers/iommu/arm-smmu.c    |  18 ++-
 drivers/iommu/iommu.c       |   5 +
 drivers/iommu/msi-iommu.c   | 317 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iommu.h       |  10 ++
 include/linux/msi-iommu.h   | 144 ++++++++++++++++++++
 8 files changed, 512 insertions(+), 8 deletions(-)
 create mode 100644 drivers/iommu/msi-iommu.c
 create mode 100644 include/linux/msi-iommu.h

-- 
1.9.1

[PATCH v8 0/8] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 1/3: iommu changes

[Eric Auger]

This series introduces the msi-iommu api used to:

- allocate/free resources for MSI IOMMU mapping
- set the MSI iova window aperture
- map/unmap physical addresses onto MSI IOVAs.
- determine whether an msi needs to be iommu mapped
- overwrite an msi_msg PA address with its pre-allocated/mapped IOVA

Also a new iommu domain attribute, DOMAIN_ATTR_MSI_GEOMETRY is introduced
to report the MSI iova window geometry (aperture and programmability).

Currently:
- iommu driver is supposed to allocate/free MSI mapping resources
- VFIO subsystem is supposed to set the MSI IOVA aperture.
- The MSI layer is supposed to allocate/free iova mappings and overwrite
  msi_msg with IOVA at composition time

More details & context can be found at:
http://www.linaro.org/blog/core-dump/kvm-pciemsi-passthrough-armarm64/

Best Regards

Eric

Git: complete series available at
https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/v4.6-rc5-pcie-passthrough-v8

History:

v7 -> v8:
- The API is retargetted for MSI: renamed msi-iommu
  all "dma-reserved" namings removed
- now implemented upon dma-iommu (get, put, init), ie. reuse iova_cookie,
  and iova API
- msi mapping resources now are guaranteed to exist during the whole iommu
  domain's lifetime. No need to lock to garantee the cookie integrity
- removed alloc/free_reserved_reserved_iova_domain. We now have a single
  function that sets the aperture, looking like iommu_dma_init_domain.
- we now use a list instead of an RB-tree
- prot is not propagated anymore at domain creation due to the retargetting
  for MSI
- iommu_domain pointer removed from doorbell_mapping struct
- replaced DOMAIN_ATTR_MSI_MAPPING by DOMAIN_ATTR_MSI_GEOMETRY

v6 -> v7:
- fixed known lock bugs and multiple page sized slots matching
  (I only have a single MSI frame made of a single page)
- reserved_iova_cookie now pointing to a struct that encapsulates the
  iova domain handle + protection attribute passed from VFIO (Alex' req)
- 2 new functions exposed: iommu_msi_mapping_translate_msg,
  iommu_msi_mapping_desc_to_domain: not sure this is the right location/proto
  though
- iommu_put_reserved_iova now takes a phys_addr_t
- everything now is cleanup on iommu_domain destruction

RFC v5 -> patch v6:
- split to ease the review process
- in dma-reserved-api use a spin lock instead of a mutex (reported by
  Jean-Philippe)
- revisit iommu_get_reserved_iova API to pass a size parameter upon
  Marc's request
- Consistently use the page order passed when creating the iova domain.
- init reserved_binding_list (reported by Julien)

RFC v4 -> RFC v5:
- take into account Thomas' comments on MSI related patches
  - split "msi: IOMMU map the doorbell address when needed"
  - increase readability and add comments
  - fix style issues
 - split "iommu: Add DOMAIN_ATTR_MSI_MAPPING attribute"
 - platform ITS now advertises IOMMU_CAP_INTR_REMAP
 - fix compilation issue with CONFIG_IOMMU API unset
 - arm-smmu-v3 now advertises DOMAIN_ATTR_MSI_MAPPING

RFC v3 -> v4:
- Move doorbell mapping/unmapping in msi.c
- fix ref count issue on set_affinity: in case of a change in the address
  the previous address is decremented
- doorbell map/unmap now is done on msi composition. Should allow the use
  case for platform MSI controllers
- create dma-reserved-iommu.h/c exposing/implementing a new API dedicated
  to reserved IOVA management (looking like dma-iommu glue)
- series reordering to ease the review:
  - first part is related to IOMMU
  - second related to MSI sub-system
  - third related to VFIO (except arm-smmu IOMMU_CAP_INTR_REMAP removal)
- expose the number of requested IOVA pages through VFIO_IOMMU_GET_INFO
  [this partially addresses Marc's comments on iommu_get/put_single_reserved
   size/alignment problematic - which I did not ignore - but I don't know
   how much I can do at the moment]

RFC v2 -> RFC v3:
- should fix wrong handling of some CONFIG combinations:
  CONFIG_IOVA, CONFIG_IOMMU_API, CONFIG_PCI_MSI_IRQ_DOMAIN
- fix MSI_FLAG_IRQ_REMAPPING setting in GICv3 ITS (although not tested)

PATCH v1 -> RFC v2:
- reverted to RFC since it looks more reasonable ;-) the code is split
  between VFIO, IOMMU, MSI controller and I am not sure I did the right
  choices. Also API need to be further discussed.
- iova API usage in arm-smmu.c.
- MSI controller natively programs the MSI addr with either the PA or IOVA.
  This is not done anymore in vfio-pci driver as suggested by Alex.
- check irq remapping capability of the group

RFC v1 [2] -> PATCH v1:
- use the existing dma map/unmap ioctl interface with a flag to register a
  reserved IOVA range. Use the legacy Rb to store this special vfio_dma.
- a single reserved IOVA contiguous region now is allowed
- use of an RB tree indexed by PA to store allocated reserved slots
- use of a vfio_domain iova_domain to manage iova allocation within the
  window provided by the userspace
- vfio alloc_map/unmap_free take a vfio_group handle
- vfio_group handle is cached in vfio_pci_device
- add ref counting to bindings
- user modality enabled at the end of the series


Eric Auger (8):
  iommu: Add iommu_domain_msi_geometry and DOMAIN_ATTR_MSI_GEOMETRY
  iommu/arm-smmu: sets the MSI geometry to programmable
  iommu: introduce an msi cookie
  iommu/msi-iommu: initialization
  iommu/msi-iommu: iommu_msi_[get,put]_doorbell_iova
  iommu/msi-iommu: iommu_msi_domain
  iommu/msi-iommu: iommu_msi_msg_pa_to_va
  iommu/arm-smmu: get/put the msi cookie

 drivers/iommu/Kconfig       |   7 +
 drivers/iommu/Makefile      |   1 +
 drivers/iommu/arm-smmu-v3.c |  18 ++-
 drivers/iommu/arm-smmu.c    |  18 ++-
 drivers/iommu/iommu.c       |   5 +
 drivers/iommu/msi-iommu.c   | 317 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iommu.h       |  10 ++
 include/linux/msi-iommu.h   | 144 ++++++++++++++++++++
 8 files changed, 512 insertions(+), 8 deletions(-)
 create mode 100644 drivers/iommu/msi-iommu.c
 create mode 100644 include/linux/msi-iommu.h

-- 
1.9.1

[PATCH v8 1/8] iommu: Add iommu_domain_msi_geometry and DOMAIN_ATTR_MSI_GEOMETRY

[Eric Auger]

Introduce a new DOMAIN_ATTR_MSI_GEOMETRY domain attribute. It enables
to query the IOVA window dedicated to MSIs and whether this window is
programmable or fixed/reserved.

x86 IOMMUs will typically expose an MSI aperture matching the 1MB
region [FEE0_0000h - FEF0_000h] corresponding to the the APIC
configuration space and not programmable. On ARM we will report a
programmable region and start/end will be set to 0.

In case the MSI IOVA aperture is programmable, the msi-iommu API needs
to be used to set it.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
Suggested-by: Alex Williamson <alex.williamson@redhat.com>

---

v8: creation
- deprecates DOMAIN_ATTR_MSI_MAPPING flag
---
 drivers/iommu/iommu.c | 5 +++++
 include/linux/iommu.h | 9 +++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b9df141..71ed58f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1498,6 +1498,7 @@ int iommu_domain_get_attr(struct iommu_domain *domain,
 			  enum iommu_attr attr, void *data)
 {
 	struct iommu_domain_geometry *geometry;
+	struct iommu_domain_msi_geometry *msi_geometry;
 	bool *paging;
 	int ret = 0;
 	u32 *count;
@@ -1508,6 +1509,10 @@ int iommu_domain_get_attr(struct iommu_domain *domain,
 		*geometry = domain->geometry;
 
 		break;
+	case DOMAIN_ATTR_MSI_GEOMETRY:
+		msi_geometry  = data;
+		*msi_geometry = domain->msi_geometry;
+		break;
 	case DOMAIN_ATTR_PAGING:
 		paging  = data;
 		*paging = (domain->ops->pgsize_bitmap != 0UL);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 62a5eae..ea5d288 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -52,6 +52,13 @@ struct iommu_domain_geometry {
 	bool force_aperture;       /* DMA only allowed in mappable range? */
 };
 
+struct iommu_domain_msi_geometry {
+	dma_addr_t aperture_start; /* First address used for MSI IOVA    */
+	dma_addr_t aperture_end;   /* Last address used for MSI IOVA     */
+	bool programmable;         /* Is the aperture programmable?	 */
+	/* In case the aperture is programmable, start/end are set to 0 */
+};
+
 /* Domain feature flags */
 #define __IOMMU_DOMAIN_PAGING	(1U << 0)  /* Support for iommu_map/unmap */
 #define __IOMMU_DOMAIN_DMA_API	(1U << 1)  /* Domain for use in DMA-API
@@ -82,6 +89,7 @@ struct iommu_domain {
 	iommu_fault_handler_t handler;
 	void *handler_token;
 	struct iommu_domain_geometry geometry;
+	struct iommu_domain_msi_geometry msi_geometry;
 	void *iova_cookie;
 };
 
@@ -107,6 +115,7 @@ enum iommu_cap {
 
 enum iommu_attr {
 	DOMAIN_ATTR_GEOMETRY,
+	DOMAIN_ATTR_MSI_GEOMETRY,
 	DOMAIN_ATTR_PAGING,
 	DOMAIN_ATTR_WINDOWS,
 	DOMAIN_ATTR_FSL_PAMU_STASH,
-- 
1.9.1

[PATCH v8 1/8] iommu: Add iommu_domain_msi_geometry and DOMAIN_ATTR_MSI_GEOMETRY

[Eric Auger]

Introduce a new DOMAIN_ATTR_MSI_GEOMETRY domain attribute. It enables
to query the IOVA window dedicated to MSIs and whether this window is
programmable or fixed/reserved.

x86 IOMMUs will typically expose an MSI aperture matching the 1MB
region [FEE0_0000h - FEF0_000h] corresponding to the the APIC
configuration space and not programmable. On ARM we will report a
programmable region and start/end will be set to 0.

In case the MSI IOVA aperture is programmable, the msi-iommu API needs
to be used to set it.

Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Suggested-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

---

v8: creation
- deprecates DOMAIN_ATTR_MSI_MAPPING flag
---
 drivers/iommu/iommu.c | 5 +++++
 include/linux/iommu.h | 9 +++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b9df141..71ed58f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1498,6 +1498,7 @@ int iommu_domain_get_attr(struct iommu_domain *domain,
 			  enum iommu_attr attr, void *data)
 {
 	struct iommu_domain_geometry *geometry;
+	struct iommu_domain_msi_geometry *msi_geometry;
 	bool *paging;
 	int ret = 0;
 	u32 *count;
@@ -1508,6 +1509,10 @@ int iommu_domain_get_attr(struct iommu_domain *domain,
 		*geometry = domain->geometry;
 
 		break;
+	case DOMAIN_ATTR_MSI_GEOMETRY:
+		msi_geometry  = data;
+		*msi_geometry = domain->msi_geometry;
+		break;
 	case DOMAIN_ATTR_PAGING:
 		paging  = data;
 		*paging = (domain->ops->pgsize_bitmap != 0UL);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 62a5eae..ea5d288 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -52,6 +52,13 @@ struct iommu_domain_geometry {
 	bool force_aperture;       /* DMA only allowed in mappable range? */
 };
 
+struct iommu_domain_msi_geometry {
+	dma_addr_t aperture_start; /* First address used for MSI IOVA    */
+	dma_addr_t aperture_end;   /* Last address used for MSI IOVA     */
+	bool programmable;         /* Is the aperture programmable?	 */
+	/* In case the aperture is programmable, start/end are set to 0 */
+};
+
 /* Domain feature flags */
 #define __IOMMU_DOMAIN_PAGING	(1U << 0)  /* Support for iommu_map/unmap */
 #define __IOMMU_DOMAIN_DMA_API	(1U << 1)  /* Domain for use in DMA-API
@@ -82,6 +89,7 @@ struct iommu_domain {
 	iommu_fault_handler_t handler;
 	void *handler_token;
 	struct iommu_domain_geometry geometry;
+	struct iommu_domain_msi_geometry msi_geometry;
 	void *iova_cookie;
 };
 
@@ -107,6 +115,7 @@ enum iommu_cap {
 
 enum iommu_attr {
 	DOMAIN_ATTR_GEOMETRY,
+	DOMAIN_ATTR_MSI_GEOMETRY,
 	DOMAIN_ATTR_PAGING,
 	DOMAIN_ATTR_WINDOWS,
 	DOMAIN_ATTR_FSL_PAMU_STASH,
-- 
1.9.1

[PATCH v8 1/8] iommu: Add iommu_domain_msi_geometry and DOMAIN_ATTR_MSI_GEOMETRY

[Eric Auger]

Introduce a new DOMAIN_ATTR_MSI_GEOMETRY domain attribute. It enables
to query the IOVA window dedicated to MSIs and whether this window is
programmable or fixed/reserved.

x86 IOMMUs will typically expose an MSI aperture matching the 1MB
region [FEE0_0000h - FEF0_000h] corresponding to the the APIC
configuration space and not programmable. On ARM we will report a
programmable region and start/end will be set to 0.

In case the MSI IOVA aperture is programmable, the msi-iommu API needs
to be used to set it.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
Suggested-by: Alex Williamson <alex.williamson@redhat.com>

---

v8: creation
- deprecates DOMAIN_ATTR_MSI_MAPPING flag
---
 drivers/iommu/iommu.c | 5 +++++
 include/linux/iommu.h | 9 +++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b9df141..71ed58f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1498,6 +1498,7 @@ int iommu_domain_get_attr(struct iommu_domain *domain,
 			  enum iommu_attr attr, void *data)
 {
 	struct iommu_domain_geometry *geometry;
+	struct iommu_domain_msi_geometry *msi_geometry;
 	bool *paging;
 	int ret = 0;
 	u32 *count;
@@ -1508,6 +1509,10 @@ int iommu_domain_get_attr(struct iommu_domain *domain,
 		*geometry = domain->geometry;
 
 		break;
+	case DOMAIN_ATTR_MSI_GEOMETRY:
+		msi_geometry  = data;
+		*msi_geometry = domain->msi_geometry;
+		break;
 	case DOMAIN_ATTR_PAGING:
 		paging  = data;
 		*paging = (domain->ops->pgsize_bitmap != 0UL);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 62a5eae..ea5d288 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -52,6 +52,13 @@ struct iommu_domain_geometry {
 	bool force_aperture;       /* DMA only allowed in mappable range? */
 };
 
+struct iommu_domain_msi_geometry {
+	dma_addr_t aperture_start; /* First address used for MSI IOVA    */
+	dma_addr_t aperture_end;   /* Last address used for MSI IOVA     */
+	bool programmable;         /* Is the aperture programmable?	 */
+	/* In case the aperture is programmable, start/end are set to 0 */
+};
+
 /* Domain feature flags */
 #define __IOMMU_DOMAIN_PAGING	(1U << 0)  /* Support for iommu_map/unmap */
 #define __IOMMU_DOMAIN_DMA_API	(1U << 1)  /* Domain for use in DMA-API
@@ -82,6 +89,7 @@ struct iommu_domain {
 	iommu_fault_handler_t handler;
 	void *handler_token;
 	struct iommu_domain_geometry geometry;
+	struct iommu_domain_msi_geometry msi_geometry;
 	void *iova_cookie;
 };
 
@@ -107,6 +115,7 @@ enum iommu_cap {
 
 enum iommu_attr {
 	DOMAIN_ATTR_GEOMETRY,
+	DOMAIN_ATTR_MSI_GEOMETRY,
 	DOMAIN_ATTR_PAGING,
 	DOMAIN_ATTR_WINDOWS,
 	DOMAIN_ATTR_FSL_PAMU_STASH,
-- 
1.9.1

[PATCH v8 2/8] iommu/arm-smmu: sets the MSI geometry to programmable

[Eric Auger]

On ARM, MSI write transactions from device upstream to the smmu
are conveyed through the iommu. Therefore target physical addresses
must be mapped and DOMAIN_ATTR_MSI_GEOMETRY advertises a programmable
MSI IOVA region.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v7 -> v8:
- use DOMAIN_ATTR_MSI_GEOMETRY

v4 -> v5:
- don't handle fsl_pamu_domain anymore
- handle arm-smmu-v3
---
 drivers/iommu/arm-smmu-v3.c | 2 ++
 drivers/iommu/arm-smmu.c    | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4ff73ff..bf222b5 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1396,6 +1396,7 @@ static bool arm_smmu_capable(enum iommu_cap cap)
 static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
+	struct iommu_domain_msi_geometry msi_geometry = {0, 0, true};
 
 	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
 		return NULL;
@@ -1414,6 +1415,7 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		kfree(smmu_domain);
 		return NULL;
 	}
+	smmu_domain->domain.msi_geometry = msi_geometry;
 
 	mutex_init(&smmu_domain->init_mutex);
 	spin_lock_init(&smmu_domain->pgtbl_lock);
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 7c39ac4..55f429d 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -976,6 +976,7 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
 static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
+	struct iommu_domain_msi_geometry msi_geometry = {0, 0, true};
 
 	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
 		return NULL;
@@ -994,6 +995,8 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return NULL;
 	}
 
+	smmu_domain->domain.msi_geometry = msi_geometry;
+
 	mutex_init(&smmu_domain->init_mutex);
 	spin_lock_init(&smmu_domain->pgtbl_lock);
 
-- 
1.9.1

[PATCH v8 2/8] iommu/arm-smmu: sets the MSI geometry to programmable

[Eric Auger]

On ARM, MSI write transactions from device upstream to the smmu
are conveyed through the iommu. Therefore target physical addresses
must be mapped and DOMAIN_ATTR_MSI_GEOMETRY advertises a programmable
MSI IOVA region.

Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

---
v7 -> v8:
- use DOMAIN_ATTR_MSI_GEOMETRY

v4 -> v5:
- don't handle fsl_pamu_domain anymore
- handle arm-smmu-v3
---
 drivers/iommu/arm-smmu-v3.c | 2 ++
 drivers/iommu/arm-smmu.c    | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4ff73ff..bf222b5 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1396,6 +1396,7 @@ static bool arm_smmu_capable(enum iommu_cap cap)
 static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
+	struct iommu_domain_msi_geometry msi_geometry = {0, 0, true};
 
 	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
 		return NULL;
@@ -1414,6 +1415,7 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		kfree(smmu_domain);
 		return NULL;
 	}
+	smmu_domain->domain.msi_geometry = msi_geometry;
 
 	mutex_init(&smmu_domain->init_mutex);
 	spin_lock_init(&smmu_domain->pgtbl_lock);
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 7c39ac4..55f429d 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -976,6 +976,7 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
 static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
+	struct iommu_domain_msi_geometry msi_geometry = {0, 0, true};
 
 	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
 		return NULL;
@@ -994,6 +995,8 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return NULL;
 	}
 
+	smmu_domain->domain.msi_geometry = msi_geometry;
+
 	mutex_init(&smmu_domain->init_mutex);
 	spin_lock_init(&smmu_domain->pgtbl_lock);
 
-- 
1.9.1

[PATCH v8 2/8] iommu/arm-smmu: sets the MSI geometry to programmable

[Eric Auger]

On ARM, MSI write transactions from device upstream to the smmu
are conveyed through the iommu. Therefore target physical addresses
must be mapped and DOMAIN_ATTR_MSI_GEOMETRY advertises a programmable
MSI IOVA region.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v7 -> v8:
- use DOMAIN_ATTR_MSI_GEOMETRY

v4 -> v5:
- don't handle fsl_pamu_domain anymore
- handle arm-smmu-v3
---
 drivers/iommu/arm-smmu-v3.c | 2 ++
 drivers/iommu/arm-smmu.c    | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4ff73ff..bf222b5 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1396,6 +1396,7 @@ static bool arm_smmu_capable(enum iommu_cap cap)
 static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
+	struct iommu_domain_msi_geometry msi_geometry = {0, 0, true};
 
 	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
 		return NULL;
@@ -1414,6 +1415,7 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		kfree(smmu_domain);
 		return NULL;
 	}
+	smmu_domain->domain.msi_geometry = msi_geometry;
 
 	mutex_init(&smmu_domain->init_mutex);
 	spin_lock_init(&smmu_domain->pgtbl_lock);
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 7c39ac4..55f429d 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -976,6 +976,7 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
 static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
+	struct iommu_domain_msi_geometry msi_geometry = {0, 0, true};
 
 	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
 		return NULL;
@@ -994,6 +995,8 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return NULL;
 	}
 
+	smmu_domain->domain.msi_geometry = msi_geometry;
+
 	mutex_init(&smmu_domain->init_mutex);
 	spin_lock_init(&smmu_domain->pgtbl_lock);
 
-- 
1.9.1

[PATCH v8 3/8] iommu: introduce an msi cookie

[Eric Auger]

This opaque pointer will enable to store information about msi
iommu mappings.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v7 -> v8:
remove spinlock and RB tree

v5 -> v6:
- initialize reserved_binding_list
- use a spinlock instead of a mutex
---
 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ea5d288..cb8d30a 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -91,6 +91,7 @@ struct iommu_domain {
 	struct iommu_domain_geometry geometry;
 	struct iommu_domain_msi_geometry msi_geometry;
 	void *iova_cookie;
+	void *msi_cookie;
 };
 
 enum iommu_cap {
-- 
1.9.1

[PATCH v8 3/8] iommu: introduce an msi cookie

[Eric Auger]

This opaque pointer will enable to store information about msi
iommu mappings.

Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

---
v7 -> v8:
remove spinlock and RB tree

v5 -> v6:
- initialize reserved_binding_list
- use a spinlock instead of a mutex
---
 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ea5d288..cb8d30a 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -91,6 +91,7 @@ struct iommu_domain {
 	struct iommu_domain_geometry geometry;
 	struct iommu_domain_msi_geometry msi_geometry;
 	void *iova_cookie;
+	void *msi_cookie;
 };
 
 enum iommu_cap {
-- 
1.9.1

[PATCH v8 3/8] iommu: introduce an msi cookie

[Eric Auger]

This opaque pointer will enable to store information about msi
iommu mappings.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v7 -> v8:
remove spinlock and RB tree

v5 -> v6:
- initialize reserved_binding_list
- use a spinlock instead of a mutex
---
 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ea5d288..cb8d30a 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -91,6 +91,7 @@ struct iommu_domain {
 	struct iommu_domain_geometry geometry;
 	struct iommu_domain_msi_geometry msi_geometry;
 	void *iova_cookie;
+	void *msi_cookie;
 };
 
 enum iommu_cap {
-- 
1.9.1

[PATCH v8 4/8] iommu/msi-iommu: initialization

[Eric Auger]

iommu_get/put_msi_cookie allocates/frees the resource used to store
and ref count the MSI doorbell mappings. iommu_msi_set_aperture
initializes the iova domain used for MSI IOVA allocation.

The implementation relies on dma-iommu API and iova API.

New msi functions are fully implemented if CONFIG_IOMMU_MSI is set.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v8:
- new design where msi-iommu relies on dma-iommu
- remove the iommu_domain * from the doorbell_mapping struct
- added is_aperture_set

v7:
- fix locking
- add iova_cache_get/put
- static inline functions when CONFIG_IOMMU_DMA_RESERVED is not set
- introduce struct reserved_iova_domain to encapsulate prot info &
  add prot parameter in alloc_reserved_iova_domain

v5 -> v6:
- use spin lock instead of mutex

v3 -> v4:
- formerly in "iommu/arm-smmu: implement alloc/free_reserved_iova_domain" &
  "iommu: add alloc/free_reserved_iova_domain"

v2 -> v3:
- remove iommu_alloc_reserved_iova_domain & iommu_free_reserved_iova_domain
  static implementation in case CONFIG_IOMMU_API is not set

v1 -> v2:
- moved from vfio API to IOMMU API
---
 drivers/iommu/Kconfig     |  7 ++++
 drivers/iommu/Makefile    |  1 +
 drivers/iommu/msi-iommu.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msi-iommu.h | 65 ++++++++++++++++++++++++++++++++
 4 files changed, 168 insertions(+)
 create mode 100644 drivers/iommu/msi-iommu.c
 create mode 100644 include/linux/msi-iommu.h

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index dd1dc39..0078b72 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -74,6 +74,11 @@ config IOMMU_DMA
 	select IOMMU_IOVA
 	select NEED_SG_DMA_LENGTH
 
+# IOMMU MSI mapping
+config IOMMU_MSI
+	bool
+	select IOMMU_DMA
+
 config FSL_PAMU
 	bool "Freescale IOMMU support"
 	depends on PPC32
@@ -307,6 +312,7 @@ config SPAPR_TCE_IOMMU
 config ARM_SMMU
 	bool "ARM Ltd. System MMU (SMMU) Support"
 	depends on (ARM64 || ARM) && MMU
+	select IOMMU_MSI
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select ARM_DMA_USE_IOMMU if ARM
@@ -320,6 +326,7 @@ config ARM_SMMU
 config ARM_SMMU_V3
 	bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
 	depends on ARM64 && PCI
+	select IOMMU_MSI
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select GENERIC_MSI_IRQ_DOMAIN
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index c6edb31..a381e66 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_IOMMU_API) += iommu.o
 obj-$(CONFIG_IOMMU_API) += iommu-traces.o
 obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
 obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
+obj-$(CONFIG_IOMMU_MSI) += msi-iommu.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
new file mode 100644
index 0000000..d38725e
--- /dev/null
+++ b/drivers/iommu/msi-iommu.c
@@ -0,0 +1,95 @@
+/*
+ * Reserved IOVA Management
+ *
+ * Copyright (c) 2015 Linaro Ltd.
+ *              www.linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/iommu.h>
+#include <linux/dma-iommu.h>
+#include <linux/msi-iommu.h>
+#include <linux/spinlock.h>
+#include <linux/iova.h>
+
+struct doorbell_mapping {
+	struct kref		kref;
+	struct list_head	next;
+	phys_addr_t		addr;
+	dma_addr_t		iova;
+	size_t			size;
+};
+
+struct doorbell_mapping_info {
+	struct list_head list; /* list of doorbell mapping entries */
+	bool is_aperture_set; /* Is the MSI IOVA aperture set? */
+	spinlock_t lock;
+};
+
+int iommu_get_msi_cookie(struct iommu_domain *domain)
+{
+	struct doorbell_mapping_info *dmi;
+	int ret;
+
+	if (domain->msi_cookie || domain->iova_cookie)
+		return -EINVAL;
+
+	ret = iommu_get_dma_cookie(domain);
+	if (ret)
+		return ret;
+
+	dmi = kzalloc(sizeof(*dmi), GFP_KERNEL);
+
+	INIT_LIST_HEAD(&dmi->list);
+	spin_lock_init(&dmi->lock);
+	iova_cache_get();
+
+	domain->msi_cookie = dmi;
+
+	return dmi ? 0 : -ENOMEM;
+}
+EXPORT_SYMBOL(iommu_get_msi_cookie);
+
+void iommu_put_msi_cookie(struct iommu_domain *domain)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+
+	if (!dmi)
+		return;
+
+	domain->msi_cookie = NULL;
+
+	WARN_ON(!list_empty(&dmi->list));
+
+	kfree(dmi);
+	iommu_put_dma_cookie(domain);
+	iova_cache_put();
+}
+EXPORT_SYMBOL(iommu_put_msi_cookie);
+
+int iommu_msi_set_aperture(struct iommu_domain *domain,
+			   dma_addr_t start, dma_addr_t end)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	int ret;
+
+	if (!dmi)
+		return -ENODEV;
+
+	ret = iommu_dma_init_domain(domain, start, end - start + 1);
+	if (!ret)
+		dmi->is_aperture_set = true;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_set_aperture);
+
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
new file mode 100644
index 0000000..392aa6f
--- /dev/null
+++ b/include/linux/msi-iommu.h
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2015 Linaro Ltd.
+ *              www.linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef __MSI_IOMMU_H
+#define __MSI_IOMMU_H
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+
+struct iommu_domain;
+
+#ifdef CONFIG_IOMMU_MSI
+
+/**
+ * iommu_get_msi_cookie - Acquire MSI mapping resources for a domain
+ * @domain: IOMMU domain to prepare for MSI mapping
+ *
+ * IOMMU drivers which require MSI mapping should normally call this
+ * from their domain_alloc callback when domain->type ==
+ * IOMMU_DOMAIN_UNMANAGED.
+ */
+int iommu_get_msi_cookie(struct iommu_domain *domain);
+
+/**
+ * iommu_put_msi_cookie - Release a domain's MSI mapping resources
+ * @domain: IOMMU domain previously prepared by iommu_get_msi_cookie()
+ *
+ * IOMMU drivers requesting MSI mapping should normally call this from
+ * their domain_free callback.
+ */
+void iommu_put_msi_cookie(struct iommu_domain *domain);
+
+/**
+ * iommu_msi_set_aperture: allocate the msi iova domain
+ * according to the specified start/end IOVAs
+ *
+ * @domain: iommu domain handle
+ * @start: MSI iova start address
+ * @end: MSI iova end address
+ */
+int iommu_msi_set_aperture(struct iommu_domain *domain,
+			   dma_addr_t start, dma_addr_t end);
+
+#else
+
+static inline int
+iommu_msi_set_aperture(struct iommu_domain *domain,
+		       dma_addr_t start, dma_addr_t end)
+{
+	return -ENOENT;
+}
+
+#endif	/* CONFIG_IOMMU_MSI */
+#endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 4/8] iommu/msi-iommu: initialization

[Eric Auger]

iommu_get/put_msi_cookie allocates/frees the resource used to store
and ref count the MSI doorbell mappings. iommu_msi_set_aperture
initializes the iova domain used for MSI IOVA allocation.

The implementation relies on dma-iommu API and iova API.

New msi functions are fully implemented if CONFIG_IOMMU_MSI is set.

Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

---
v8:
- new design where msi-iommu relies on dma-iommu
- remove the iommu_domain * from the doorbell_mapping struct
- added is_aperture_set

v7:
- fix locking
- add iova_cache_get/put
- static inline functions when CONFIG_IOMMU_DMA_RESERVED is not set
- introduce struct reserved_iova_domain to encapsulate prot info &
  add prot parameter in alloc_reserved_iova_domain

v5 -> v6:
- use spin lock instead of mutex

v3 -> v4:
- formerly in "iommu/arm-smmu: implement alloc/free_reserved_iova_domain" &
  "iommu: add alloc/free_reserved_iova_domain"

v2 -> v3:
- remove iommu_alloc_reserved_iova_domain & iommu_free_reserved_iova_domain
  static implementation in case CONFIG_IOMMU_API is not set

v1 -> v2:
- moved from vfio API to IOMMU API
---
 drivers/iommu/Kconfig     |  7 ++++
 drivers/iommu/Makefile    |  1 +
 drivers/iommu/msi-iommu.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msi-iommu.h | 65 ++++++++++++++++++++++++++++++++
 4 files changed, 168 insertions(+)
 create mode 100644 drivers/iommu/msi-iommu.c
 create mode 100644 include/linux/msi-iommu.h

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index dd1dc39..0078b72 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -74,6 +74,11 @@ config IOMMU_DMA
 	select IOMMU_IOVA
 	select NEED_SG_DMA_LENGTH
 
+# IOMMU MSI mapping
+config IOMMU_MSI
+	bool
+	select IOMMU_DMA
+
 config FSL_PAMU
 	bool "Freescale IOMMU support"
 	depends on PPC32
@@ -307,6 +312,7 @@ config SPAPR_TCE_IOMMU
 config ARM_SMMU
 	bool "ARM Ltd. System MMU (SMMU) Support"
 	depends on (ARM64 || ARM) && MMU
+	select IOMMU_MSI
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select ARM_DMA_USE_IOMMU if ARM
@@ -320,6 +326,7 @@ config ARM_SMMU
 config ARM_SMMU_V3
 	bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
 	depends on ARM64 && PCI
+	select IOMMU_MSI
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select GENERIC_MSI_IRQ_DOMAIN
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index c6edb31..a381e66 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_IOMMU_API) += iommu.o
 obj-$(CONFIG_IOMMU_API) += iommu-traces.o
 obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
 obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
+obj-$(CONFIG_IOMMU_MSI) += msi-iommu.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
new file mode 100644
index 0000000..d38725e
--- /dev/null
+++ b/drivers/iommu/msi-iommu.c
@@ -0,0 +1,95 @@
+/*
+ * Reserved IOVA Management
+ *
+ * Copyright (c) 2015 Linaro Ltd.
+ *              www.linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/iommu.h>
+#include <linux/dma-iommu.h>
+#include <linux/msi-iommu.h>
+#include <linux/spinlock.h>
+#include <linux/iova.h>
+
+struct doorbell_mapping {
+	struct kref		kref;
+	struct list_head	next;
+	phys_addr_t		addr;
+	dma_addr_t		iova;
+	size_t			size;
+};
+
+struct doorbell_mapping_info {
+	struct list_head list; /* list of doorbell mapping entries */
+	bool is_aperture_set; /* Is the MSI IOVA aperture set? */
+	spinlock_t lock;
+};
+
+int iommu_get_msi_cookie(struct iommu_domain *domain)
+{
+	struct doorbell_mapping_info *dmi;
+	int ret;
+
+	if (domain->msi_cookie || domain->iova_cookie)
+		return -EINVAL;
+
+	ret = iommu_get_dma_cookie(domain);
+	if (ret)
+		return ret;
+
+	dmi = kzalloc(sizeof(*dmi), GFP_KERNEL);
+
+	INIT_LIST_HEAD(&dmi->list);
+	spin_lock_init(&dmi->lock);
+	iova_cache_get();
+
+	domain->msi_cookie = dmi;
+
+	return dmi ? 0 : -ENOMEM;
+}
+EXPORT_SYMBOL(iommu_get_msi_cookie);
+
+void iommu_put_msi_cookie(struct iommu_domain *domain)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+
+	if (!dmi)
+		return;
+
+	domain->msi_cookie = NULL;
+
+	WARN_ON(!list_empty(&dmi->list));
+
+	kfree(dmi);
+	iommu_put_dma_cookie(domain);
+	iova_cache_put();
+}
+EXPORT_SYMBOL(iommu_put_msi_cookie);
+
+int iommu_msi_set_aperture(struct iommu_domain *domain,
+			   dma_addr_t start, dma_addr_t end)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	int ret;
+
+	if (!dmi)
+		return -ENODEV;
+
+	ret = iommu_dma_init_domain(domain, start, end - start + 1);
+	if (!ret)
+		dmi->is_aperture_set = true;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_set_aperture);
+
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
new file mode 100644
index 0000000..392aa6f
--- /dev/null
+++ b/include/linux/msi-iommu.h
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2015 Linaro Ltd.
+ *              www.linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef __MSI_IOMMU_H
+#define __MSI_IOMMU_H
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+
+struct iommu_domain;
+
+#ifdef CONFIG_IOMMU_MSI
+
+/**
+ * iommu_get_msi_cookie - Acquire MSI mapping resources for a domain
+ * @domain: IOMMU domain to prepare for MSI mapping
+ *
+ * IOMMU drivers which require MSI mapping should normally call this
+ * from their domain_alloc callback when domain->type ==
+ * IOMMU_DOMAIN_UNMANAGED.
+ */
+int iommu_get_msi_cookie(struct iommu_domain *domain);
+
+/**
+ * iommu_put_msi_cookie - Release a domain's MSI mapping resources
+ * @domain: IOMMU domain previously prepared by iommu_get_msi_cookie()
+ *
+ * IOMMU drivers requesting MSI mapping should normally call this from
+ * their domain_free callback.
+ */
+void iommu_put_msi_cookie(struct iommu_domain *domain);
+
+/**
+ * iommu_msi_set_aperture: allocate the msi iova domain
+ * according to the specified start/end IOVAs
+ *
+ * @domain: iommu domain handle
+ * @start: MSI iova start address
+ * @end: MSI iova end address
+ */
+int iommu_msi_set_aperture(struct iommu_domain *domain,
+			   dma_addr_t start, dma_addr_t end);
+
+#else
+
+static inline int
+iommu_msi_set_aperture(struct iommu_domain *domain,
+		       dma_addr_t start, dma_addr_t end)
+{
+	return -ENOENT;
+}
+
+#endif	/* CONFIG_IOMMU_MSI */
+#endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 4/8] iommu/msi-iommu: initialization

[Eric Auger]

iommu_get/put_msi_cookie allocates/frees the resource used to store
and ref count the MSI doorbell mappings. iommu_msi_set_aperture
initializes the iova domain used for MSI IOVA allocation.

The implementation relies on dma-iommu API and iova API.

New msi functions are fully implemented if CONFIG_IOMMU_MSI is set.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v8:
- new design where msi-iommu relies on dma-iommu
- remove the iommu_domain * from the doorbell_mapping struct
- added is_aperture_set

v7:
- fix locking
- add iova_cache_get/put
- static inline functions when CONFIG_IOMMU_DMA_RESERVED is not set
- introduce struct reserved_iova_domain to encapsulate prot info &
  add prot parameter in alloc_reserved_iova_domain

v5 -> v6:
- use spin lock instead of mutex

v3 -> v4:
- formerly in "iommu/arm-smmu: implement alloc/free_reserved_iova_domain" &
  "iommu: add alloc/free_reserved_iova_domain"

v2 -> v3:
- remove iommu_alloc_reserved_iova_domain & iommu_free_reserved_iova_domain
  static implementation in case CONFIG_IOMMU_API is not set

v1 -> v2:
- moved from vfio API to IOMMU API
---
 drivers/iommu/Kconfig     |  7 ++++
 drivers/iommu/Makefile    |  1 +
 drivers/iommu/msi-iommu.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msi-iommu.h | 65 ++++++++++++++++++++++++++++++++
 4 files changed, 168 insertions(+)
 create mode 100644 drivers/iommu/msi-iommu.c
 create mode 100644 include/linux/msi-iommu.h

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index dd1dc39..0078b72 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -74,6 +74,11 @@ config IOMMU_DMA
 	select IOMMU_IOVA
 	select NEED_SG_DMA_LENGTH
 
+# IOMMU MSI mapping
+config IOMMU_MSI
+	bool
+	select IOMMU_DMA
+
 config FSL_PAMU
 	bool "Freescale IOMMU support"
 	depends on PPC32
@@ -307,6 +312,7 @@ config SPAPR_TCE_IOMMU
 config ARM_SMMU
 	bool "ARM Ltd. System MMU (SMMU) Support"
 	depends on (ARM64 || ARM) && MMU
+	select IOMMU_MSI
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select ARM_DMA_USE_IOMMU if ARM
@@ -320,6 +326,7 @@ config ARM_SMMU
 config ARM_SMMU_V3
 	bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
 	depends on ARM64 && PCI
+	select IOMMU_MSI
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select GENERIC_MSI_IRQ_DOMAIN
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index c6edb31..a381e66 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_IOMMU_API) += iommu.o
 obj-$(CONFIG_IOMMU_API) += iommu-traces.o
 obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
 obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
+obj-$(CONFIG_IOMMU_MSI) += msi-iommu.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
new file mode 100644
index 0000000..d38725e
--- /dev/null
+++ b/drivers/iommu/msi-iommu.c
@@ -0,0 +1,95 @@
+/*
+ * Reserved IOVA Management
+ *
+ * Copyright (c) 2015 Linaro Ltd.
+ *              www.linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/iommu.h>
+#include <linux/dma-iommu.h>
+#include <linux/msi-iommu.h>
+#include <linux/spinlock.h>
+#include <linux/iova.h>
+
+struct doorbell_mapping {
+	struct kref		kref;
+	struct list_head	next;
+	phys_addr_t		addr;
+	dma_addr_t		iova;
+	size_t			size;
+};
+
+struct doorbell_mapping_info {
+	struct list_head list; /* list of doorbell mapping entries */
+	bool is_aperture_set; /* Is the MSI IOVA aperture set? */
+	spinlock_t lock;
+};
+
+int iommu_get_msi_cookie(struct iommu_domain *domain)
+{
+	struct doorbell_mapping_info *dmi;
+	int ret;
+
+	if (domain->msi_cookie || domain->iova_cookie)
+		return -EINVAL;
+
+	ret = iommu_get_dma_cookie(domain);
+	if (ret)
+		return ret;
+
+	dmi = kzalloc(sizeof(*dmi), GFP_KERNEL);
+
+	INIT_LIST_HEAD(&dmi->list);
+	spin_lock_init(&dmi->lock);
+	iova_cache_get();
+
+	domain->msi_cookie = dmi;
+
+	return dmi ? 0 : -ENOMEM;
+}
+EXPORT_SYMBOL(iommu_get_msi_cookie);
+
+void iommu_put_msi_cookie(struct iommu_domain *domain)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+
+	if (!dmi)
+		return;
+
+	domain->msi_cookie = NULL;
+
+	WARN_ON(!list_empty(&dmi->list));
+
+	kfree(dmi);
+	iommu_put_dma_cookie(domain);
+	iova_cache_put();
+}
+EXPORT_SYMBOL(iommu_put_msi_cookie);
+
+int iommu_msi_set_aperture(struct iommu_domain *domain,
+			   dma_addr_t start, dma_addr_t end)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	int ret;
+
+	if (!dmi)
+		return -ENODEV;
+
+	ret = iommu_dma_init_domain(domain, start, end - start + 1);
+	if (!ret)
+		dmi->is_aperture_set = true;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_set_aperture);
+
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
new file mode 100644
index 0000000..392aa6f
--- /dev/null
+++ b/include/linux/msi-iommu.h
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2015 Linaro Ltd.
+ *              www.linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef __MSI_IOMMU_H
+#define __MSI_IOMMU_H
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+
+struct iommu_domain;
+
+#ifdef CONFIG_IOMMU_MSI
+
+/**
+ * iommu_get_msi_cookie - Acquire MSI mapping resources for a domain
+ * @domain: IOMMU domain to prepare for MSI mapping
+ *
+ * IOMMU drivers which require MSI mapping should normally call this
+ * from their domain_alloc callback when domain->type ==
+ * IOMMU_DOMAIN_UNMANAGED.
+ */
+int iommu_get_msi_cookie(struct iommu_domain *domain);
+
+/**
+ * iommu_put_msi_cookie - Release a domain's MSI mapping resources
+ * @domain: IOMMU domain previously prepared by iommu_get_msi_cookie()
+ *
+ * IOMMU drivers requesting MSI mapping should normally call this from
+ * their domain_free callback.
+ */
+void iommu_put_msi_cookie(struct iommu_domain *domain);
+
+/**
+ * iommu_msi_set_aperture: allocate the msi iova domain
+ * according to the specified start/end IOVAs
+ *
+ * @domain: iommu domain handle
+ * @start: MSI iova start address
+ * @end: MSI iova end address
+ */
+int iommu_msi_set_aperture(struct iommu_domain *domain,
+			   dma_addr_t start, dma_addr_t end);
+
+#else
+
+static inline int
+iommu_msi_set_aperture(struct iommu_domain *domain,
+		       dma_addr_t start, dma_addr_t end)
+{
+	return -ENOENT;
+}
+
+#endif	/* CONFIG_IOMMU_MSI */
+#endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 5/8] iommu/msi-iommu: iommu_msi_[get,put]_doorbell_iova

[Eric Auger]

iommu_msi_get_doorbell_iova allows to iommu map an MSI doorbell contiguous
physical region onto a reserved contiguous IOVA region. The physical
region base address does not need to be iommu page size aligned. iova
pages are allocated and mapped so that they cover all the physical region.
This mapping is tracked as a whole (and cannot be split).

In case a mapping already exists for the physical pages, the IOVA mapped
to the PA base is directly returned.

Each time the get succeeds a binding ref count is incremented.

iommu_put_reserved_iova decrements the ref count and when this latter
is null, the mapping is destroyed and the IOVAs are released.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v8:
- function renaming
- new design based on the assumption the iova domain cannot disappear
- free_iova outside of doorbell_mapping_release (iommu_domain * was
  removed from doorbell_mapping)

v7:
- change title and rework commit message with new name of the functions
  and size parameter
- fix locking
- rework header doc comments
- put now takes a phys_addr_t
- check prot argument against reserved_iova_domain prot flags

v5 -> v6:
- revisit locking with spin_lock instead of mutex
- do not kref_get on 1st get
- add size parameter to the get function following Marc's request
- use the iova domain shift instead of using the smallest supported page siz

v3 -> v4:
- formerly in iommu: iommu_get/put_single_reserved &
  iommu/arm-smmu: implement iommu_get/put_single_reserved
- Attempted to address Marc's doubts about missing size/alignment
  at VFIO level (user-space knows the IOMMU page size and the number
  of IOVA pages to provision)

v2 -> v3:
- remove static implementation of iommu_get_single_reserved &
  iommu_put_single_reserved when CONFIG_IOMMU_API is not set

v1 -> v2:
- previously a VFIO API, named vfio_alloc_map/unmap_free_reserved_iova
---
 drivers/iommu/msi-iommu.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msi-iommu.h |  39 ++++++++++++
 2 files changed, 189 insertions(+)

diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
index d38725e..203e86e 100644
--- a/drivers/iommu/msi-iommu.c
+++ b/drivers/iommu/msi-iommu.c
@@ -93,3 +93,153 @@ int iommu_msi_set_aperture(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_msi_set_aperture);
 
+/* called with info->lock held */
+static struct doorbell_mapping *
+search_msi_doorbell_mapping(struct doorbell_mapping_info *info,
+			    phys_addr_t addr, size_t size)
+{
+	struct doorbell_mapping *mapping;
+
+	list_for_each_entry(mapping, &info->list, next) {
+		if ((addr >= mapping->addr) &&
+		    (addr + size <= mapping->addr + mapping->size))
+			return mapping;
+	}
+	return NULL;
+}
+
+int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
+				phys_addr_t addr, size_t size, int prot,
+				dma_addr_t *iova)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	struct iova_domain *iovad = domain->iova_cookie;
+	struct doorbell_mapping *new_mapping, *mapping;
+	phys_addr_t aligned_base, offset;
+	size_t binding_size;
+	struct iova *p_iova;
+	dma_addr_t new_iova;
+	int ret = -EINVAL;
+	bool unmap = false;
+
+	if (!dmi)
+		return -ENODEV;
+
+	if (!dmi->is_aperture_set)
+		return -EINVAL;
+
+	offset = iova_offset(iovad, addr);
+	aligned_base = addr - offset;
+	binding_size = iova_align(iovad, size + offset);
+
+	spin_lock(&dmi->lock);
+
+	mapping = search_msi_doorbell_mapping(dmi, aligned_base, binding_size);
+	if (mapping) {
+		*iova = mapping->iova + offset + aligned_base - mapping->addr;
+		kref_get(&mapping->kref);
+		ret = 0;
+		goto unlock;
+	}
+
+	spin_unlock(&dmi->lock);
+
+	new_mapping = kzalloc(sizeof(*new_mapping), GFP_KERNEL);
+	if (!new_mapping)
+		return -ENOMEM;
+
+	p_iova = alloc_iova(iovad, binding_size >> iova_shift(iovad),
+			    iovad->dma_32bit_pfn, true);
+	if (!p_iova) {
+		kfree(new_mapping);
+		return -ENOMEM;
+	}
+
+	new_iova = iova_dma_addr(iovad, p_iova);
+	*iova = new_iova;
+
+	/* iommu_map is not supposed to be atomic */
+	ret = iommu_map(domain, *iova, aligned_base, binding_size, prot);
+
+	spin_lock(&dmi->lock);
+
+	if (ret)
+		goto free_iova;
+	/*
+	 * check again the doorbell mapping was not added while the lock
+	 * was released
+	 */
+	mapping = search_msi_doorbell_mapping(dmi, aligned_base, binding_size);
+	if (mapping) {
+		*iova = mapping->iova + offset + aligned_base - mapping->addr;
+		kref_get(&mapping->kref);
+		ret = 0;
+		unmap = true;
+		goto free_iova;
+	}
+
+	kref_init(&new_mapping->kref);
+	new_mapping->addr = aligned_base;
+	new_mapping->iova = *iova;
+	new_mapping->size = binding_size;
+
+	list_add(&new_mapping->next, &dmi->list);
+
+	*iova += offset;
+	goto unlock;
+free_iova:
+	free_iova(iovad, p_iova->pfn_lo);
+	kfree(new_mapping);
+unlock:
+	spin_unlock(&dmi->lock);
+	if (unmap)
+		iommu_unmap(domain, new_iova, binding_size);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_get_doorbell_iova);
+
+static void doorbell_mapping_release(struct kref *kref)
+{
+	struct doorbell_mapping *mapping =
+		container_of(kref, struct doorbell_mapping, kref);
+
+	list_del(&mapping->next);
+	kfree(mapping);
+}
+
+void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	struct iova_domain *iovad = domain->iova_cookie;
+	phys_addr_t aligned_addr, page_size, offset;
+	struct doorbell_mapping *mapping;
+	dma_addr_t iova;
+	size_t size;
+	int ret = 0;
+
+	if (!dmi)
+		return;
+
+	page_size = (uint64_t)1 << iova_shift(iovad);
+	offset = iova_offset(iovad, addr);
+	aligned_addr = addr - offset;
+
+	spin_lock(&dmi->lock);
+
+	mapping = search_msi_doorbell_mapping(dmi, aligned_addr, page_size);
+	if (!mapping)
+		goto unlock;
+
+	iova = mapping->iova;
+	size = mapping->size;
+
+	ret = kref_put(&mapping->kref, doorbell_mapping_release);
+
+unlock:
+	spin_unlock(&dmi->lock);
+	if (ret) {
+		iommu_unmap(domain, iova, size);
+		free_iova(iovad, iova_pfn(iovad, iova));
+	}
+}
+EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
index 392aa6f..1cd115f 100644
--- a/include/linux/msi-iommu.h
+++ b/include/linux/msi-iommu.h
@@ -52,6 +52,35 @@ void iommu_put_msi_cookie(struct iommu_domain *domain);
 int iommu_msi_set_aperture(struct iommu_domain *domain,
 			   dma_addr_t start, dma_addr_t end);
 
+/**
+ * iommu_msi_get_doorbell_iova: allocate a contiguous set of iova pages and
+ * map them to the MSI doorbell's physical range defined by @addr and @size.
+ *
+ * @domain: iommu domain handle
+ * @addr: physical address to bind
+ * @size: size of the binding
+ * @prot: mapping protection attribute
+ * @iova: returned iova
+ *
+ * Mapped physical pfns are within [@addr >> order, (@addr + size -1) >> order]
+ * where order corresponds to the iova domain order.
+ * This mapping is tracked and reference counted with the minimal granularity
+ * of @size.
+ */
+int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
+				phys_addr_t addr, size_t size, int prot,
+				dma_addr_t *iova);
+
+/**
+ * iommu_msi_put_doorbell_iova: decrement a ref count of the doorbell's mapping
+ *
+ * @domain: iommu domain handle
+ * @addr: physical address whose binding ref count is decremented
+ *
+ * if the binding ref count is null, destroy the MSI doorbell's mapping
+ */
+void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
+
 #else
 
 static inline int
@@ -61,5 +90,15 @@ iommu_msi_set_aperture(struct iommu_domain *domain,
 	return -ENOENT;
 }
 
+static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
+					      phys_addr_t addr, size_t size,
+					      int prot, dma_addr_t *iova)
+{
+	return -ENOENT;
+}
+
+static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
+					       phys_addr_t addr) {}
+
 #endif	/* CONFIG_IOMMU_MSI */
 #endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 5/8] iommu/msi-iommu: iommu_msi_[get,put]_doorbell_iova

[Eric Auger]

iommu_msi_get_doorbell_iova allows to iommu map an MSI doorbell contiguous
physical region onto a reserved contiguous IOVA region. The physical
region base address does not need to be iommu page size aligned. iova
pages are allocated and mapped so that they cover all the physical region.
This mapping is tracked as a whole (and cannot be split).

In case a mapping already exists for the physical pages, the IOVA mapped
to the PA base is directly returned.

Each time the get succeeds a binding ref count is incremented.

iommu_put_reserved_iova decrements the ref count and when this latter
is null, the mapping is destroyed and the IOVAs are released.

Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

---
v8:
- function renaming
- new design based on the assumption the iova domain cannot disappear
- free_iova outside of doorbell_mapping_release (iommu_domain * was
  removed from doorbell_mapping)

v7:
- change title and rework commit message with new name of the functions
  and size parameter
- fix locking
- rework header doc comments
- put now takes a phys_addr_t
- check prot argument against reserved_iova_domain prot flags

v5 -> v6:
- revisit locking with spin_lock instead of mutex
- do not kref_get on 1st get
- add size parameter to the get function following Marc's request
- use the iova domain shift instead of using the smallest supported page siz

v3 -> v4:
- formerly in iommu: iommu_get/put_single_reserved &
  iommu/arm-smmu: implement iommu_get/put_single_reserved
- Attempted to address Marc's doubts about missing size/alignment
  at VFIO level (user-space knows the IOMMU page size and the number
  of IOVA pages to provision)

v2 -> v3:
- remove static implementation of iommu_get_single_reserved &
  iommu_put_single_reserved when CONFIG_IOMMU_API is not set

v1 -> v2:
- previously a VFIO API, named vfio_alloc_map/unmap_free_reserved_iova
---
 drivers/iommu/msi-iommu.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msi-iommu.h |  39 ++++++++++++
 2 files changed, 189 insertions(+)

diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
index d38725e..203e86e 100644
--- a/drivers/iommu/msi-iommu.c
+++ b/drivers/iommu/msi-iommu.c
@@ -93,3 +93,153 @@ int iommu_msi_set_aperture(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_msi_set_aperture);
 
+/* called with info->lock held */
+static struct doorbell_mapping *
+search_msi_doorbell_mapping(struct doorbell_mapping_info *info,
+			    phys_addr_t addr, size_t size)
+{
+	struct doorbell_mapping *mapping;
+
+	list_for_each_entry(mapping, &info->list, next) {
+		if ((addr >= mapping->addr) &&
+		    (addr + size <= mapping->addr + mapping->size))
+			return mapping;
+	}
+	return NULL;
+}
+
+int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
+				phys_addr_t addr, size_t size, int prot,
+				dma_addr_t *iova)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	struct iova_domain *iovad = domain->iova_cookie;
+	struct doorbell_mapping *new_mapping, *mapping;
+	phys_addr_t aligned_base, offset;
+	size_t binding_size;
+	struct iova *p_iova;
+	dma_addr_t new_iova;
+	int ret = -EINVAL;
+	bool unmap = false;
+
+	if (!dmi)
+		return -ENODEV;
+
+	if (!dmi->is_aperture_set)
+		return -EINVAL;
+
+	offset = iova_offset(iovad, addr);
+	aligned_base = addr - offset;
+	binding_size = iova_align(iovad, size + offset);
+
+	spin_lock(&dmi->lock);
+
+	mapping = search_msi_doorbell_mapping(dmi, aligned_base, binding_size);
+	if (mapping) {
+		*iova = mapping->iova + offset + aligned_base - mapping->addr;
+		kref_get(&mapping->kref);
+		ret = 0;
+		goto unlock;
+	}
+
+	spin_unlock(&dmi->lock);
+
+	new_mapping = kzalloc(sizeof(*new_mapping), GFP_KERNEL);
+	if (!new_mapping)
+		return -ENOMEM;
+
+	p_iova = alloc_iova(iovad, binding_size >> iova_shift(iovad),
+			    iovad->dma_32bit_pfn, true);
+	if (!p_iova) {
+		kfree(new_mapping);
+		return -ENOMEM;
+	}
+
+	new_iova = iova_dma_addr(iovad, p_iova);
+	*iova = new_iova;
+
+	/* iommu_map is not supposed to be atomic */
+	ret = iommu_map(domain, *iova, aligned_base, binding_size, prot);
+
+	spin_lock(&dmi->lock);
+
+	if (ret)
+		goto free_iova;
+	/*
+	 * check again the doorbell mapping was not added while the lock
+	 * was released
+	 */
+	mapping = search_msi_doorbell_mapping(dmi, aligned_base, binding_size);
+	if (mapping) {
+		*iova = mapping->iova + offset + aligned_base - mapping->addr;
+		kref_get(&mapping->kref);
+		ret = 0;
+		unmap = true;
+		goto free_iova;
+	}
+
+	kref_init(&new_mapping->kref);
+	new_mapping->addr = aligned_base;
+	new_mapping->iova = *iova;
+	new_mapping->size = binding_size;
+
+	list_add(&new_mapping->next, &dmi->list);
+
+	*iova += offset;
+	goto unlock;
+free_iova:
+	free_iova(iovad, p_iova->pfn_lo);
+	kfree(new_mapping);
+unlock:
+	spin_unlock(&dmi->lock);
+	if (unmap)
+		iommu_unmap(domain, new_iova, binding_size);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_get_doorbell_iova);
+
+static void doorbell_mapping_release(struct kref *kref)
+{
+	struct doorbell_mapping *mapping =
+		container_of(kref, struct doorbell_mapping, kref);
+
+	list_del(&mapping->next);
+	kfree(mapping);
+}
+
+void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	struct iova_domain *iovad = domain->iova_cookie;
+	phys_addr_t aligned_addr, page_size, offset;
+	struct doorbell_mapping *mapping;
+	dma_addr_t iova;
+	size_t size;
+	int ret = 0;
+
+	if (!dmi)
+		return;
+
+	page_size = (uint64_t)1 << iova_shift(iovad);
+	offset = iova_offset(iovad, addr);
+	aligned_addr = addr - offset;
+
+	spin_lock(&dmi->lock);
+
+	mapping = search_msi_doorbell_mapping(dmi, aligned_addr, page_size);
+	if (!mapping)
+		goto unlock;
+
+	iova = mapping->iova;
+	size = mapping->size;
+
+	ret = kref_put(&mapping->kref, doorbell_mapping_release);
+
+unlock:
+	spin_unlock(&dmi->lock);
+	if (ret) {
+		iommu_unmap(domain, iova, size);
+		free_iova(iovad, iova_pfn(iovad, iova));
+	}
+}
+EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
index 392aa6f..1cd115f 100644
--- a/include/linux/msi-iommu.h
+++ b/include/linux/msi-iommu.h
@@ -52,6 +52,35 @@ void iommu_put_msi_cookie(struct iommu_domain *domain);
 int iommu_msi_set_aperture(struct iommu_domain *domain,
 			   dma_addr_t start, dma_addr_t end);
 
+/**
+ * iommu_msi_get_doorbell_iova: allocate a contiguous set of iova pages and
+ * map them to the MSI doorbell's physical range defined by @addr and @size.
+ *
+ * @domain: iommu domain handle
+ * @addr: physical address to bind
+ * @size: size of the binding
+ * @prot: mapping protection attribute
+ * @iova: returned iova
+ *
+ * Mapped physical pfns are within [@addr >> order, (@addr + size -1) >> order]
+ * where order corresponds to the iova domain order.
+ * This mapping is tracked and reference counted with the minimal granularity
+ * of @size.
+ */
+int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
+				phys_addr_t addr, size_t size, int prot,
+				dma_addr_t *iova);
+
+/**
+ * iommu_msi_put_doorbell_iova: decrement a ref count of the doorbell's mapping
+ *
+ * @domain: iommu domain handle
+ * @addr: physical address whose binding ref count is decremented
+ *
+ * if the binding ref count is null, destroy the MSI doorbell's mapping
+ */
+void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
+
 #else
 
 static inline int
@@ -61,5 +90,15 @@ iommu_msi_set_aperture(struct iommu_domain *domain,
 	return -ENOENT;
 }
 
+static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
+					      phys_addr_t addr, size_t size,
+					      int prot, dma_addr_t *iova)
+{
+	return -ENOENT;
+}
+
+static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
+					       phys_addr_t addr) {}
+
 #endif	/* CONFIG_IOMMU_MSI */
 #endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 5/8] iommu/msi-iommu: iommu_msi_[get,put]_doorbell_iova

[Eric Auger]

iommu_msi_get_doorbell_iova allows to iommu map an MSI doorbell contiguous
physical region onto a reserved contiguous IOVA region. The physical
region base address does not need to be iommu page size aligned. iova
pages are allocated and mapped so that they cover all the physical region.
This mapping is tracked as a whole (and cannot be split).

In case a mapping already exists for the physical pages, the IOVA mapped
to the PA base is directly returned.

Each time the get succeeds a binding ref count is incremented.

iommu_put_reserved_iova decrements the ref count and when this latter
is null, the mapping is destroyed and the IOVAs are released.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v8:
- function renaming
- new design based on the assumption the iova domain cannot disappear
- free_iova outside of doorbell_mapping_release (iommu_domain * was
  removed from doorbell_mapping)

v7:
- change title and rework commit message with new name of the functions
  and size parameter
- fix locking
- rework header doc comments
- put now takes a phys_addr_t
- check prot argument against reserved_iova_domain prot flags

v5 -> v6:
- revisit locking with spin_lock instead of mutex
- do not kref_get on 1st get
- add size parameter to the get function following Marc's request
- use the iova domain shift instead of using the smallest supported page siz

v3 -> v4:
- formerly in iommu: iommu_get/put_single_reserved &
  iommu/arm-smmu: implement iommu_get/put_single_reserved
- Attempted to address Marc's doubts about missing size/alignment
  at VFIO level (user-space knows the IOMMU page size and the number
  of IOVA pages to provision)

v2 -> v3:
- remove static implementation of iommu_get_single_reserved &
  iommu_put_single_reserved when CONFIG_IOMMU_API is not set

v1 -> v2:
- previously a VFIO API, named vfio_alloc_map/unmap_free_reserved_iova
---
 drivers/iommu/msi-iommu.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msi-iommu.h |  39 ++++++++++++
 2 files changed, 189 insertions(+)

diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
index d38725e..203e86e 100644
--- a/drivers/iommu/msi-iommu.c
+++ b/drivers/iommu/msi-iommu.c
@@ -93,3 +93,153 @@ int iommu_msi_set_aperture(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_msi_set_aperture);
 
+/* called with info->lock held */
+static struct doorbell_mapping *
+search_msi_doorbell_mapping(struct doorbell_mapping_info *info,
+			    phys_addr_t addr, size_t size)
+{
+	struct doorbell_mapping *mapping;
+
+	list_for_each_entry(mapping, &info->list, next) {
+		if ((addr >= mapping->addr) &&
+		    (addr + size <= mapping->addr + mapping->size))
+			return mapping;
+	}
+	return NULL;
+}
+
+int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
+				phys_addr_t addr, size_t size, int prot,
+				dma_addr_t *iova)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	struct iova_domain *iovad = domain->iova_cookie;
+	struct doorbell_mapping *new_mapping, *mapping;
+	phys_addr_t aligned_base, offset;
+	size_t binding_size;
+	struct iova *p_iova;
+	dma_addr_t new_iova;
+	int ret = -EINVAL;
+	bool unmap = false;
+
+	if (!dmi)
+		return -ENODEV;
+
+	if (!dmi->is_aperture_set)
+		return -EINVAL;
+
+	offset = iova_offset(iovad, addr);
+	aligned_base = addr - offset;
+	binding_size = iova_align(iovad, size + offset);
+
+	spin_lock(&dmi->lock);
+
+	mapping = search_msi_doorbell_mapping(dmi, aligned_base, binding_size);
+	if (mapping) {
+		*iova = mapping->iova + offset + aligned_base - mapping->addr;
+		kref_get(&mapping->kref);
+		ret = 0;
+		goto unlock;
+	}
+
+	spin_unlock(&dmi->lock);
+
+	new_mapping = kzalloc(sizeof(*new_mapping), GFP_KERNEL);
+	if (!new_mapping)
+		return -ENOMEM;
+
+	p_iova = alloc_iova(iovad, binding_size >> iova_shift(iovad),
+			    iovad->dma_32bit_pfn, true);
+	if (!p_iova) {
+		kfree(new_mapping);
+		return -ENOMEM;
+	}
+
+	new_iova = iova_dma_addr(iovad, p_iova);
+	*iova = new_iova;
+
+	/* iommu_map is not supposed to be atomic */
+	ret = iommu_map(domain, *iova, aligned_base, binding_size, prot);
+
+	spin_lock(&dmi->lock);
+
+	if (ret)
+		goto free_iova;
+	/*
+	 * check again the doorbell mapping was not added while the lock
+	 * was released
+	 */
+	mapping = search_msi_doorbell_mapping(dmi, aligned_base, binding_size);
+	if (mapping) {
+		*iova = mapping->iova + offset + aligned_base - mapping->addr;
+		kref_get(&mapping->kref);
+		ret = 0;
+		unmap = true;
+		goto free_iova;
+	}
+
+	kref_init(&new_mapping->kref);
+	new_mapping->addr = aligned_base;
+	new_mapping->iova = *iova;
+	new_mapping->size = binding_size;
+
+	list_add(&new_mapping->next, &dmi->list);
+
+	*iova += offset;
+	goto unlock;
+free_iova:
+	free_iova(iovad, p_iova->pfn_lo);
+	kfree(new_mapping);
+unlock:
+	spin_unlock(&dmi->lock);
+	if (unmap)
+		iommu_unmap(domain, new_iova, binding_size);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_get_doorbell_iova);
+
+static void doorbell_mapping_release(struct kref *kref)
+{
+	struct doorbell_mapping *mapping =
+		container_of(kref, struct doorbell_mapping, kref);
+
+	list_del(&mapping->next);
+	kfree(mapping);
+}
+
+void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	struct iova_domain *iovad = domain->iova_cookie;
+	phys_addr_t aligned_addr, page_size, offset;
+	struct doorbell_mapping *mapping;
+	dma_addr_t iova;
+	size_t size;
+	int ret = 0;
+
+	if (!dmi)
+		return;
+
+	page_size = (uint64_t)1 << iova_shift(iovad);
+	offset = iova_offset(iovad, addr);
+	aligned_addr = addr - offset;
+
+	spin_lock(&dmi->lock);
+
+	mapping = search_msi_doorbell_mapping(dmi, aligned_addr, page_size);
+	if (!mapping)
+		goto unlock;
+
+	iova = mapping->iova;
+	size = mapping->size;
+
+	ret = kref_put(&mapping->kref, doorbell_mapping_release);
+
+unlock:
+	spin_unlock(&dmi->lock);
+	if (ret) {
+		iommu_unmap(domain, iova, size);
+		free_iova(iovad, iova_pfn(iovad, iova));
+	}
+}
+EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
index 392aa6f..1cd115f 100644
--- a/include/linux/msi-iommu.h
+++ b/include/linux/msi-iommu.h
@@ -52,6 +52,35 @@ void iommu_put_msi_cookie(struct iommu_domain *domain);
 int iommu_msi_set_aperture(struct iommu_domain *domain,
 			   dma_addr_t start, dma_addr_t end);
 
+/**
+ * iommu_msi_get_doorbell_iova: allocate a contiguous set of iova pages and
+ * map them to the MSI doorbell's physical range defined by @addr and @size.
+ *
+ * @domain: iommu domain handle
+ * @addr: physical address to bind
+ * @size: size of the binding
+ * @prot: mapping protection attribute
+ * @iova: returned iova
+ *
+ * Mapped physical pfns are within [@addr >> order, (@addr + size -1) >> order]
+ * where order corresponds to the iova domain order.
+ * This mapping is tracked and reference counted with the minimal granularity
+ * of @size.
+ */
+int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
+				phys_addr_t addr, size_t size, int prot,
+				dma_addr_t *iova);
+
+/**
+ * iommu_msi_put_doorbell_iova: decrement a ref count of the doorbell's mapping
+ *
+ * @domain: iommu domain handle
+ * @addr: physical address whose binding ref count is decremented
+ *
+ * if the binding ref count is null, destroy the MSI doorbell's mapping
+ */
+void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
+
 #else
 
 static inline int
@@ -61,5 +90,15 @@ iommu_msi_set_aperture(struct iommu_domain *domain,
 	return -ENOENT;
 }
 
+static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
+					      phys_addr_t addr, size_t size,
+					      int prot, dma_addr_t *iova)
+{
+	return -ENOENT;
+}
+
+static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
+					       phys_addr_t addr) {}
+
 #endif	/* CONFIG_IOMMU_MSI */
 #endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Eric Auger]

This function checks whether
- the device belongs to a non default iommu domain
- this iommu domain requires the MSI address to be mapped.

If those conditions are met, the function returns the iommu domain
to be used for mapping the MSI doorbell; else it returns NULL.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v7 -> v8:
- renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
- the function now takes a struct device *
- use DOMAIN_ATTR_MSI_GEOMETRY attribute
---
 drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
 include/linux/msi-iommu.h | 14 ++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
index 203e86e..023ff17 100644
--- a/drivers/iommu/msi-iommu.c
+++ b/drivers/iommu/msi-iommu.c
@@ -243,3 +243,20 @@ unlock:
 	}
 }
 EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
+
+struct iommu_domain *iommu_msi_domain(struct device *dev)
+{
+	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
+	struct iommu_domain_msi_geometry msi_geometry;
+
+	if (!d || (d->type == IOMMU_DOMAIN_DMA))
+		return NULL;
+
+	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
+	if (!msi_geometry.programmable)
+		return NULL;
+
+	return d;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_domain);
+
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
index 1cd115f..114bd69 100644
--- a/include/linux/msi-iommu.h
+++ b/include/linux/msi-iommu.h
@@ -81,6 +81,15 @@ int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
  */
 void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
 
+/**
+ * iommu_msi_domain: in case the device is upstream to an IOMMU and this IOMMU
+ * translates the MSI transaction, returns the iommu domain the MSI doorbell
+ * address must be mapped in; else returns NULL.
+ *
+ * @dev: device handle
+ */
+struct iommu_domain *iommu_msi_domain(struct device *dev);
+
 #else
 
 static inline int
@@ -100,5 +109,10 @@ static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
 static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
 					       phys_addr_t addr) {}
 
+static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
+{
+	return NULL;
+}
+
 #endif	/* CONFIG_IOMMU_MSI */
 #endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Eric Auger]

This function checks whether
- the device belongs to a non default iommu domain
- this iommu domain requires the MSI address to be mapped.

If those conditions are met, the function returns the iommu domain
to be used for mapping the MSI doorbell; else it returns NULL.

Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

---

v7 -> v8:
- renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
- the function now takes a struct device *
- use DOMAIN_ATTR_MSI_GEOMETRY attribute
---
 drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
 include/linux/msi-iommu.h | 14 ++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
index 203e86e..023ff17 100644
--- a/drivers/iommu/msi-iommu.c
+++ b/drivers/iommu/msi-iommu.c
@@ -243,3 +243,20 @@ unlock:
 	}
 }
 EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
+
+struct iommu_domain *iommu_msi_domain(struct device *dev)
+{
+	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
+	struct iommu_domain_msi_geometry msi_geometry;
+
+	if (!d || (d->type == IOMMU_DOMAIN_DMA))
+		return NULL;
+
+	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
+	if (!msi_geometry.programmable)
+		return NULL;
+
+	return d;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_domain);
+
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
index 1cd115f..114bd69 100644
--- a/include/linux/msi-iommu.h
+++ b/include/linux/msi-iommu.h
@@ -81,6 +81,15 @@ int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
  */
 void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
 
+/**
+ * iommu_msi_domain: in case the device is upstream to an IOMMU and this IOMMU
+ * translates the MSI transaction, returns the iommu domain the MSI doorbell
+ * address must be mapped in; else returns NULL.
+ *
+ * @dev: device handle
+ */
+struct iommu_domain *iommu_msi_domain(struct device *dev);
+
 #else
 
 static inline int
@@ -100,5 +109,10 @@ static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
 static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
 					       phys_addr_t addr) {}
 
+static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
+{
+	return NULL;
+}
+
 #endif	/* CONFIG_IOMMU_MSI */
 #endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Eric Auger]

This function checks whether
- the device belongs to a non default iommu domain
- this iommu domain requires the MSI address to be mapped.

If those conditions are met, the function returns the iommu domain
to be used for mapping the MSI doorbell; else it returns NULL.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v7 -> v8:
- renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
- the function now takes a struct device *
- use DOMAIN_ATTR_MSI_GEOMETRY attribute
---
 drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
 include/linux/msi-iommu.h | 14 ++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
index 203e86e..023ff17 100644
--- a/drivers/iommu/msi-iommu.c
+++ b/drivers/iommu/msi-iommu.c
@@ -243,3 +243,20 @@ unlock:
 	}
 }
 EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
+
+struct iommu_domain *iommu_msi_domain(struct device *dev)
+{
+	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
+	struct iommu_domain_msi_geometry msi_geometry;
+
+	if (!d || (d->type == IOMMU_DOMAIN_DMA))
+		return NULL;
+
+	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
+	if (!msi_geometry.programmable)
+		return NULL;
+
+	return d;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_domain);
+
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
index 1cd115f..114bd69 100644
--- a/include/linux/msi-iommu.h
+++ b/include/linux/msi-iommu.h
@@ -81,6 +81,15 @@ int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
  */
 void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
 
+/**
+ * iommu_msi_domain: in case the device is upstream to an IOMMU and this IOMMU
+ * translates the MSI transaction, returns the iommu domain the MSI doorbell
+ * address must be mapped in; else returns NULL.
+ *
+ * @dev: device handle
+ */
+struct iommu_domain *iommu_msi_domain(struct device *dev);
+
 #else
 
 static inline int
@@ -100,5 +109,10 @@ static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
 static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
 					       phys_addr_t addr) {}
 
+static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
+{
+	return NULL;
+}
+
 #endif	/* CONFIG_IOMMU_MSI */
 #endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 7/8] iommu/msi-iommu: iommu_msi_msg_pa_to_va

[Eric Auger]

Introduce iommu_msi_msg_pa_to_va whose role consists in
detecting whether the device's MSIs must to be mapped into an IOMMU.
It case it must, the function overrides the MSI msg originally composed
and replaces the doorbell's PA by a pre-allocated and pre-mapped reserved
IOVA. In case the corresponding PA region is not found, the function
returns an error.

This function is likely to be called in some code that cannot sleep. This
is the reason why the allocation/mapping is done separately.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v7 -> v8:
- renamed from iommu_msi_mapping_translate_msg to iommu_msi_msg_pa_to_va
  and now takes a struct device * as parameter

v7: creation
---
 drivers/iommu/msi-iommu.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msi-iommu.h | 26 ++++++++++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
index 023ff17..fb3007f 100644
--- a/drivers/iommu/msi-iommu.c
+++ b/drivers/iommu/msi-iommu.c
@@ -20,6 +20,14 @@
 #include <linux/msi-iommu.h>
 #include <linux/spinlock.h>
 #include <linux/iova.h>
+#include <linux/msi.h>
+
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
+#define msg_to_phys_addr(msg) \
+	(((phys_addr_t)((msg)->address_hi) << 32) | (msg)->address_lo)
+#else
+#define msg_to_phys_addr(msg)	((msg)->address_lo)
+#endif
 
 struct doorbell_mapping {
 	struct kref		kref;
@@ -260,3 +268,50 @@ struct iommu_domain *iommu_msi_domain(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_msi_domain);
 
+static dma_addr_t iommu_msi_find_iova(struct iommu_domain *domain,
+				      phys_addr_t addr, size_t size)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	struct iova_domain *iovad = domain->iova_cookie;
+	struct doorbell_mapping *mapping;
+	dma_addr_t iova = DMA_ERROR_CODE;
+	phys_addr_t aligned_base, offset;
+	size_t binding_size;
+
+	if (!iovad || !dmi)
+		return iova;
+
+	offset = iova_offset(iovad, addr);
+	aligned_base = addr - offset;
+	binding_size = iova_align(iovad, size + offset);
+
+	spin_lock(&dmi->lock);
+
+	mapping = search_msi_doorbell_mapping(dmi, addr, size);
+	if (mapping)
+		iova = mapping->iova + offset + aligned_base - mapping->addr;
+
+	spin_unlock(&dmi->lock);
+	return iova;
+}
+
+int iommu_msi_msg_pa_to_va(struct device *dev, struct msi_msg *msg)
+{
+	struct iommu_domain *d = iommu_msi_domain(dev);
+	dma_addr_t iova;
+
+	if (!d)
+		return 0;
+
+	iova = iommu_msi_find_iova(d, msg_to_phys_addr(msg),
+				   sizeof(phys_addr_t));
+
+	if (iova == DMA_ERROR_CODE)
+		return -EINVAL;
+
+	msg->address_lo = lower_32_bits(iova);
+	msg->address_hi = upper_32_bits(iova);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_msg_pa_to_va);
+
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
index 114bd69..c1c9bd5 100644
--- a/include/linux/msi-iommu.h
+++ b/include/linux/msi-iommu.h
@@ -19,6 +19,7 @@
 #include <linux/kernel.h>
 
 struct iommu_domain;
+struct msi_msg;
 
 #ifdef CONFIG_IOMMU_MSI
 
@@ -90,6 +91,25 @@ void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
  */
 struct iommu_domain *iommu_msi_domain(struct device *dev);
 
+/**
+ * iommu_msi_msg_pa_to_va: in case a device's MSI transaction is translated
+ * by an IOMMU, the msg address must be an IOVA instead of a physical address.
+ * This function overwrites the original MSI message containing the doorbell's
+ * physical address with the doorbell's pre-allocated IOVA, if any.
+ *
+ * The doorbell physical address must be bound previously to an IOVA using
+ * iommu_msi_get_doorbell_iova
+ *
+ * @dev: device emitting the MSI
+ * @msg: original MSI message containing the PA to be overwritten with the IOVA
+ *
+ * return 0 if the MSI does not need to be mapped or when the PA/IOVA
+ * were successfully swapped; return -EINVAL if the addresses need
+ * to be swapped but not IOMMU binding is found
+ */
+int iommu_msi_msg_pa_to_va(struct device *dev, struct msi_msg *msg);
+
+
 #else
 
 static inline int
@@ -114,5 +134,11 @@ static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
 	return NULL;
 }
 
+static inline int iommu_msi_msg_pa_to_va(struct device *dev,
+					 struct msi_msg *msg)
+{
+	return 0;
+}
+
 #endif	/* CONFIG_IOMMU_MSI */
 #endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 7/8] iommu/msi-iommu: iommu_msi_msg_pa_to_va

[Eric Auger]

Introduce iommu_msi_msg_pa_to_va whose role consists in
detecting whether the device's MSIs must to be mapped into an IOMMU.
It case it must, the function overrides the MSI msg originally composed
and replaces the doorbell's PA by a pre-allocated and pre-mapped reserved
IOVA. In case the corresponding PA region is not found, the function
returns an error.

This function is likely to be called in some code that cannot sleep. This
is the reason why the allocation/mapping is done separately.

Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

---
v7 -> v8:
- renamed from iommu_msi_mapping_translate_msg to iommu_msi_msg_pa_to_va
  and now takes a struct device * as parameter

v7: creation
---
 drivers/iommu/msi-iommu.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msi-iommu.h | 26 ++++++++++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
index 023ff17..fb3007f 100644
--- a/drivers/iommu/msi-iommu.c
+++ b/drivers/iommu/msi-iommu.c
@@ -20,6 +20,14 @@
 #include <linux/msi-iommu.h>
 #include <linux/spinlock.h>
 #include <linux/iova.h>
+#include <linux/msi.h>
+
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
+#define msg_to_phys_addr(msg) \
+	(((phys_addr_t)((msg)->address_hi) << 32) | (msg)->address_lo)
+#else
+#define msg_to_phys_addr(msg)	((msg)->address_lo)
+#endif
 
 struct doorbell_mapping {
 	struct kref		kref;
@@ -260,3 +268,50 @@ struct iommu_domain *iommu_msi_domain(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_msi_domain);
 
+static dma_addr_t iommu_msi_find_iova(struct iommu_domain *domain,
+				      phys_addr_t addr, size_t size)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	struct iova_domain *iovad = domain->iova_cookie;
+	struct doorbell_mapping *mapping;
+	dma_addr_t iova = DMA_ERROR_CODE;
+	phys_addr_t aligned_base, offset;
+	size_t binding_size;
+
+	if (!iovad || !dmi)
+		return iova;
+
+	offset = iova_offset(iovad, addr);
+	aligned_base = addr - offset;
+	binding_size = iova_align(iovad, size + offset);
+
+	spin_lock(&dmi->lock);
+
+	mapping = search_msi_doorbell_mapping(dmi, addr, size);
+	if (mapping)
+		iova = mapping->iova + offset + aligned_base - mapping->addr;
+
+	spin_unlock(&dmi->lock);
+	return iova;
+}
+
+int iommu_msi_msg_pa_to_va(struct device *dev, struct msi_msg *msg)
+{
+	struct iommu_domain *d = iommu_msi_domain(dev);
+	dma_addr_t iova;
+
+	if (!d)
+		return 0;
+
+	iova = iommu_msi_find_iova(d, msg_to_phys_addr(msg),
+				   sizeof(phys_addr_t));
+
+	if (iova == DMA_ERROR_CODE)
+		return -EINVAL;
+
+	msg->address_lo = lower_32_bits(iova);
+	msg->address_hi = upper_32_bits(iova);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_msg_pa_to_va);
+
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
index 114bd69..c1c9bd5 100644
--- a/include/linux/msi-iommu.h
+++ b/include/linux/msi-iommu.h
@@ -19,6 +19,7 @@
 #include <linux/kernel.h>
 
 struct iommu_domain;
+struct msi_msg;
 
 #ifdef CONFIG_IOMMU_MSI
 
@@ -90,6 +91,25 @@ void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
  */
 struct iommu_domain *iommu_msi_domain(struct device *dev);
 
+/**
+ * iommu_msi_msg_pa_to_va: in case a device's MSI transaction is translated
+ * by an IOMMU, the msg address must be an IOVA instead of a physical address.
+ * This function overwrites the original MSI message containing the doorbell's
+ * physical address with the doorbell's pre-allocated IOVA, if any.
+ *
+ * The doorbell physical address must be bound previously to an IOVA using
+ * iommu_msi_get_doorbell_iova
+ *
+ * @dev: device emitting the MSI
+ * @msg: original MSI message containing the PA to be overwritten with the IOVA
+ *
+ * return 0 if the MSI does not need to be mapped or when the PA/IOVA
+ * were successfully swapped; return -EINVAL if the addresses need
+ * to be swapped but not IOMMU binding is found
+ */
+int iommu_msi_msg_pa_to_va(struct device *dev, struct msi_msg *msg);
+
+
 #else
 
 static inline int
@@ -114,5 +134,11 @@ static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
 	return NULL;
 }
 
+static inline int iommu_msi_msg_pa_to_va(struct device *dev,
+					 struct msi_msg *msg)
+{
+	return 0;
+}
+
 #endif	/* CONFIG_IOMMU_MSI */
 #endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 7/8] iommu/msi-iommu: iommu_msi_msg_pa_to_va

[Eric Auger]

Introduce iommu_msi_msg_pa_to_va whose role consists in
detecting whether the device's MSIs must to be mapped into an IOMMU.
It case it must, the function overrides the MSI msg originally composed
and replaces the doorbell's PA by a pre-allocated and pre-mapped reserved
IOVA. In case the corresponding PA region is not found, the function
returns an error.

This function is likely to be called in some code that cannot sleep. This
is the reason why the allocation/mapping is done separately.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v7 -> v8:
- renamed from iommu_msi_mapping_translate_msg to iommu_msi_msg_pa_to_va
  and now takes a struct device * as parameter

v7: creation
---
 drivers/iommu/msi-iommu.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msi-iommu.h | 26 ++++++++++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
index 023ff17..fb3007f 100644
--- a/drivers/iommu/msi-iommu.c
+++ b/drivers/iommu/msi-iommu.c
@@ -20,6 +20,14 @@
 #include <linux/msi-iommu.h>
 #include <linux/spinlock.h>
 #include <linux/iova.h>
+#include <linux/msi.h>
+
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
+#define msg_to_phys_addr(msg) \
+	(((phys_addr_t)((msg)->address_hi) << 32) | (msg)->address_lo)
+#else
+#define msg_to_phys_addr(msg)	((msg)->address_lo)
+#endif
 
 struct doorbell_mapping {
 	struct kref		kref;
@@ -260,3 +268,50 @@ struct iommu_domain *iommu_msi_domain(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_msi_domain);
 
+static dma_addr_t iommu_msi_find_iova(struct iommu_domain *domain,
+				      phys_addr_t addr, size_t size)
+{
+	struct doorbell_mapping_info *dmi = domain->msi_cookie;
+	struct iova_domain *iovad = domain->iova_cookie;
+	struct doorbell_mapping *mapping;
+	dma_addr_t iova = DMA_ERROR_CODE;
+	phys_addr_t aligned_base, offset;
+	size_t binding_size;
+
+	if (!iovad || !dmi)
+		return iova;
+
+	offset = iova_offset(iovad, addr);
+	aligned_base = addr - offset;
+	binding_size = iova_align(iovad, size + offset);
+
+	spin_lock(&dmi->lock);
+
+	mapping = search_msi_doorbell_mapping(dmi, addr, size);
+	if (mapping)
+		iova = mapping->iova + offset + aligned_base - mapping->addr;
+
+	spin_unlock(&dmi->lock);
+	return iova;
+}
+
+int iommu_msi_msg_pa_to_va(struct device *dev, struct msi_msg *msg)
+{
+	struct iommu_domain *d = iommu_msi_domain(dev);
+	dma_addr_t iova;
+
+	if (!d)
+		return 0;
+
+	iova = iommu_msi_find_iova(d, msg_to_phys_addr(msg),
+				   sizeof(phys_addr_t));
+
+	if (iova == DMA_ERROR_CODE)
+		return -EINVAL;
+
+	msg->address_lo = lower_32_bits(iova);
+	msg->address_hi = upper_32_bits(iova);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_msg_pa_to_va);
+
diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
index 114bd69..c1c9bd5 100644
--- a/include/linux/msi-iommu.h
+++ b/include/linux/msi-iommu.h
@@ -19,6 +19,7 @@
 #include <linux/kernel.h>
 
 struct iommu_domain;
+struct msi_msg;
 
 #ifdef CONFIG_IOMMU_MSI
 
@@ -90,6 +91,25 @@ void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
  */
 struct iommu_domain *iommu_msi_domain(struct device *dev);
 
+/**
+ * iommu_msi_msg_pa_to_va: in case a device's MSI transaction is translated
+ * by an IOMMU, the msg address must be an IOVA instead of a physical address.
+ * This function overwrites the original MSI message containing the doorbell's
+ * physical address with the doorbell's pre-allocated IOVA, if any.
+ *
+ * The doorbell physical address must be bound previously to an IOVA using
+ * iommu_msi_get_doorbell_iova
+ *
+ * @dev: device emitting the MSI
+ * @msg: original MSI message containing the PA to be overwritten with the IOVA
+ *
+ * return 0 if the MSI does not need to be mapped or when the PA/IOVA
+ * were successfully swapped; return -EINVAL if the addresses need
+ * to be swapped but not IOMMU binding is found
+ */
+int iommu_msi_msg_pa_to_va(struct device *dev, struct msi_msg *msg);
+
+
 #else
 
 static inline int
@@ -114,5 +134,11 @@ static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
 	return NULL;
 }
 
+static inline int iommu_msi_msg_pa_to_va(struct device *dev,
+					 struct msi_msg *msg)
+{
+	return 0;
+}
+
 #endif	/* CONFIG_IOMMU_MSI */
 #endif	/* __MSI_IOMMU_H */
-- 
1.9.1

[PATCH v8 8/8] iommu/arm-smmu: get/put the msi cookie

[Eric Auger]

For IOMMU_DOMAIN_UNMANAGED type we now also get the msi
cookie in both arm-smmu and arm-smmu-v3. This initializes
resources for MSI doorbell mapping.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 drivers/iommu/arm-smmu-v3.c | 16 ++++++++++++----
 drivers/iommu/arm-smmu.c    | 15 +++++++++++----
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index bf222b5..b5d9826 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -22,6 +22,7 @@
 
 #include <linux/delay.h>
 #include <linux/dma-iommu.h>
+#include <linux/msi-iommu.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
 #include <linux/iommu.h>
@@ -1411,15 +1412,21 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return NULL;
 
 	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&smmu_domain->domain)) {
-		kfree(smmu_domain);
-		return NULL;
-	}
+	    iommu_get_dma_cookie(&smmu_domain->domain))
+		goto err;
+
+	if (type == IOMMU_DOMAIN_UNMANAGED &&
+		iommu_get_msi_cookie(&smmu_domain->domain))
+		goto err;
+
 	smmu_domain->domain.msi_geometry = msi_geometry;
 
 	mutex_init(&smmu_domain->init_mutex);
 	spin_lock_init(&smmu_domain->pgtbl_lock);
 	return &smmu_domain->domain;
+err:
+	kfree(smmu_domain);
+	return NULL;
 }
 
 static int arm_smmu_bitmap_alloc(unsigned long *map, int span)
@@ -1445,6 +1452,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 
+	iommu_put_msi_cookie(domain);
 	iommu_put_dma_cookie(domain);
 	free_io_pgtable_ops(smmu_domain->pgtbl_ops);
 
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 55f429d..0908985 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -30,6 +30,7 @@
 
 #include <linux/delay.h>
 #include <linux/dma-iommu.h>
+#include <linux/msi-iommu.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
@@ -990,10 +991,12 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return NULL;
 
 	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&smmu_domain->domain)) {
-		kfree(smmu_domain);
-		return NULL;
-	}
+	    iommu_get_dma_cookie(&smmu_domain->domain))
+		goto err;
+
+	if (type == IOMMU_DOMAIN_UNMANAGED &&
+		iommu_get_msi_cookie(&smmu_domain->domain))
+		goto err;
 
 	smmu_domain->domain.msi_geometry = msi_geometry;
 
@@ -1001,6 +1004,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 	spin_lock_init(&smmu_domain->pgtbl_lock);
 
 	return &smmu_domain->domain;
+err:
+	kfree(smmu_domain);
+	return NULL;
 }
 
 static void arm_smmu_domain_free(struct iommu_domain *domain)
@@ -1011,6 +1017,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 	 * Free the domain resources. We assume that all devices have
 	 * already been detached.
 	 */
+	iommu_put_msi_cookie(domain);
 	iommu_put_dma_cookie(domain);
 	arm_smmu_destroy_domain_context(domain);
 	kfree(smmu_domain);
-- 
1.9.1

[PATCH v8 8/8] iommu/arm-smmu: get/put the msi cookie

[Eric Auger]

For IOMMU_DOMAIN_UNMANAGED type we now also get the msi
cookie in both arm-smmu and arm-smmu-v3. This initializes
resources for MSI doorbell mapping.

Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 16 ++++++++++++----
 drivers/iommu/arm-smmu.c    | 15 +++++++++++----
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index bf222b5..b5d9826 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -22,6 +22,7 @@
 
 #include <linux/delay.h>
 #include <linux/dma-iommu.h>
+#include <linux/msi-iommu.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
 #include <linux/iommu.h>
@@ -1411,15 +1412,21 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return NULL;
 
 	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&smmu_domain->domain)) {
-		kfree(smmu_domain);
-		return NULL;
-	}
+	    iommu_get_dma_cookie(&smmu_domain->domain))
+		goto err;
+
+	if (type == IOMMU_DOMAIN_UNMANAGED &&
+		iommu_get_msi_cookie(&smmu_domain->domain))
+		goto err;
+
 	smmu_domain->domain.msi_geometry = msi_geometry;
 
 	mutex_init(&smmu_domain->init_mutex);
 	spin_lock_init(&smmu_domain->pgtbl_lock);
 	return &smmu_domain->domain;
+err:
+	kfree(smmu_domain);
+	return NULL;
 }
 
 static int arm_smmu_bitmap_alloc(unsigned long *map, int span)
@@ -1445,6 +1452,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 
+	iommu_put_msi_cookie(domain);
 	iommu_put_dma_cookie(domain);
 	free_io_pgtable_ops(smmu_domain->pgtbl_ops);
 
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 55f429d..0908985 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -30,6 +30,7 @@
 
 #include <linux/delay.h>
 #include <linux/dma-iommu.h>
+#include <linux/msi-iommu.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
@@ -990,10 +991,12 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return NULL;
 
 	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&smmu_domain->domain)) {
-		kfree(smmu_domain);
-		return NULL;
-	}
+	    iommu_get_dma_cookie(&smmu_domain->domain))
+		goto err;
+
+	if (type == IOMMU_DOMAIN_UNMANAGED &&
+		iommu_get_msi_cookie(&smmu_domain->domain))
+		goto err;
 
 	smmu_domain->domain.msi_geometry = msi_geometry;
 
@@ -1001,6 +1004,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 	spin_lock_init(&smmu_domain->pgtbl_lock);
 
 	return &smmu_domain->domain;
+err:
+	kfree(smmu_domain);
+	return NULL;
 }
 
 static void arm_smmu_domain_free(struct iommu_domain *domain)
@@ -1011,6 +1017,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 	 * Free the domain resources. We assume that all devices have
 	 * already been detached.
 	 */
+	iommu_put_msi_cookie(domain);
 	iommu_put_dma_cookie(domain);
 	arm_smmu_destroy_domain_context(domain);
 	kfree(smmu_domain);
-- 
1.9.1

[PATCH v8 8/8] iommu/arm-smmu: get/put the msi cookie

[Eric Auger]

For IOMMU_DOMAIN_UNMANAGED type we now also get the msi
cookie in both arm-smmu and arm-smmu-v3. This initializes
resources for MSI doorbell mapping.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 drivers/iommu/arm-smmu-v3.c | 16 ++++++++++++----
 drivers/iommu/arm-smmu.c    | 15 +++++++++++----
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index bf222b5..b5d9826 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -22,6 +22,7 @@
 
 #include <linux/delay.h>
 #include <linux/dma-iommu.h>
+#include <linux/msi-iommu.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
 #include <linux/iommu.h>
@@ -1411,15 +1412,21 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return NULL;
 
 	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&smmu_domain->domain)) {
-		kfree(smmu_domain);
-		return NULL;
-	}
+	    iommu_get_dma_cookie(&smmu_domain->domain))
+		goto err;
+
+	if (type == IOMMU_DOMAIN_UNMANAGED &&
+		iommu_get_msi_cookie(&smmu_domain->domain))
+		goto err;
+
 	smmu_domain->domain.msi_geometry = msi_geometry;
 
 	mutex_init(&smmu_domain->init_mutex);
 	spin_lock_init(&smmu_domain->pgtbl_lock);
 	return &smmu_domain->domain;
+err:
+	kfree(smmu_domain);
+	return NULL;
 }
 
 static int arm_smmu_bitmap_alloc(unsigned long *map, int span)
@@ -1445,6 +1452,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 
+	iommu_put_msi_cookie(domain);
 	iommu_put_dma_cookie(domain);
 	free_io_pgtable_ops(smmu_domain->pgtbl_ops);
 
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 55f429d..0908985 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -30,6 +30,7 @@
 
 #include <linux/delay.h>
 #include <linux/dma-iommu.h>
+#include <linux/msi-iommu.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
@@ -990,10 +991,12 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return NULL;
 
 	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&smmu_domain->domain)) {
-		kfree(smmu_domain);
-		return NULL;
-	}
+	    iommu_get_dma_cookie(&smmu_domain->domain))
+		goto err;
+
+	if (type == IOMMU_DOMAIN_UNMANAGED &&
+		iommu_get_msi_cookie(&smmu_domain->domain))
+		goto err;
 
 	smmu_domain->domain.msi_geometry = msi_geometry;
 
@@ -1001,6 +1004,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 	spin_lock_init(&smmu_domain->pgtbl_lock);
 
 	return &smmu_domain->domain;
+err:
+	kfree(smmu_domain);
+	return NULL;
 }
 
 static void arm_smmu_domain_free(struct iommu_domain *domain)
@@ -1011,6 +1017,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 	 * Free the domain resources. We assume that all devices have
 	 * already been detached.
 	 */
+	iommu_put_msi_cookie(domain);
 	iommu_put_dma_cookie(domain);
 	arm_smmu_destroy_domain_context(domain);
 	kfree(smmu_domain);
-- 
1.9.1

Re: [PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Alex Williamson]

On Thu, 28 Apr 2016 08:15:21 +0000
Eric Auger <eric.auger@linaro.org> wrote:

> This function checks whether
> - the device belongs to a non default iommu domain
> - this iommu domain requires the MSI address to be mapped.
> 
> If those conditions are met, the function returns the iommu domain
> to be used for mapping the MSI doorbell; else it returns NULL.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> v7 -> v8:
> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
> - the function now takes a struct device *
> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
> ---
>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
>  include/linux/msi-iommu.h | 14 ++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
> index 203e86e..023ff17 100644
> --- a/drivers/iommu/msi-iommu.c
> +++ b/drivers/iommu/msi-iommu.c
> @@ -243,3 +243,20 @@ unlock:
>  	}
>  }
>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
> +
> +struct iommu_domain *iommu_msi_domain(struct device *dev)
> +{
> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
> +	struct iommu_domain_msi_geometry msi_geometry;
> +
> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
> +		return NULL;
> +
> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
> +	if (!msi_geometry.programmable)

It feels like we're conflating ideas with using the "programmable" flag
in this way.  AIUI the IOMMU API consumer is supposed to see the
invalid MSI geometry with the programmable feature set and know to call
iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
that had been done, but that doesn't tell us if it should have been
done.  iommu_msi_msg_pa_to_va() handles this later, if we return
NULL here that function returns success otherwise it goes on to fail if
the iova or msi cookie is not set.  So really what we're trying to flag
is that the MSI geometry participates in the IOMMU-MSI API you've
created and we should pick a feature name that says that rather than
something as vague a "programmable".  Perhaps simply iommu_msi_api
rather than programmable.

BTW, I don't see that you ever set aperture_start/end once
iommu_msi_set_aperture() has been called.  It seems like doing so would
add some consistency to that MSI geometry attribute.

Nice series overall.  Thanks,

Alex

> +		return NULL;
> +
> +	return d;
> +}
> +EXPORT_SYMBOL_GPL(iommu_msi_domain);
> +
> diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
> index 1cd115f..114bd69 100644
> --- a/include/linux/msi-iommu.h
> +++ b/include/linux/msi-iommu.h
> @@ -81,6 +81,15 @@ int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>   */
>  void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
>  
> +/**
> + * iommu_msi_domain: in case the device is upstream to an IOMMU and this IOMMU
> + * translates the MSI transaction, returns the iommu domain the MSI doorbell
> + * address must be mapped in; else returns NULL.
> + *
> + * @dev: device handle
> + */
> +struct iommu_domain *iommu_msi_domain(struct device *dev);
> +
>  #else
>  
>  static inline int
> @@ -100,5 +109,10 @@ static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>  static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
>  					       phys_addr_t addr) {}
>  
> +static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
> +{
> +	return NULL;
> +}
> +
>  #endif	/* CONFIG_IOMMU_MSI */
>  #endif	/* __MSI_IOMMU_H */

Re: [PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Alex Williamson]

On Thu, 28 Apr 2016 08:15:21 +0000
Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> This function checks whether
> - the device belongs to a non default iommu domain
> - this iommu domain requires the MSI address to be mapped.
> 
> If those conditions are met, the function returns the iommu domain
> to be used for mapping the MSI doorbell; else it returns NULL.
> 
> Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> ---
> 
> v7 -> v8:
> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
> - the function now takes a struct device *
> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
> ---
>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
>  include/linux/msi-iommu.h | 14 ++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
> index 203e86e..023ff17 100644
> --- a/drivers/iommu/msi-iommu.c
> +++ b/drivers/iommu/msi-iommu.c
> @@ -243,3 +243,20 @@ unlock:
>  	}
>  }
>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
> +
> +struct iommu_domain *iommu_msi_domain(struct device *dev)
> +{
> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
> +	struct iommu_domain_msi_geometry msi_geometry;
> +
> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
> +		return NULL;
> +
> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
> +	if (!msi_geometry.programmable)

It feels like we're conflating ideas with using the "programmable" flag
in this way.  AIUI the IOMMU API consumer is supposed to see the
invalid MSI geometry with the programmable feature set and know to call
iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
that had been done, but that doesn't tell us if it should have been
done.  iommu_msi_msg_pa_to_va() handles this later, if we return
NULL here that function returns success otherwise it goes on to fail if
the iova or msi cookie is not set.  So really what we're trying to flag
is that the MSI geometry participates in the IOMMU-MSI API you've
created and we should pick a feature name that says that rather than
something as vague a "programmable".  Perhaps simply iommu_msi_api
rather than programmable.

BTW, I don't see that you ever set aperture_start/end once
iommu_msi_set_aperture() has been called.  It seems like doing so would
add some consistency to that MSI geometry attribute.

Nice series overall.  Thanks,

Alex

> +		return NULL;
> +
> +	return d;
> +}
> +EXPORT_SYMBOL_GPL(iommu_msi_domain);
> +
> diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
> index 1cd115f..114bd69 100644
> --- a/include/linux/msi-iommu.h
> +++ b/include/linux/msi-iommu.h
> @@ -81,6 +81,15 @@ int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>   */
>  void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
>  
> +/**
> + * iommu_msi_domain: in case the device is upstream to an IOMMU and this IOMMU
> + * translates the MSI transaction, returns the iommu domain the MSI doorbell
> + * address must be mapped in; else returns NULL.
> + *
> + * @dev: device handle
> + */
> +struct iommu_domain *iommu_msi_domain(struct device *dev);
> +
>  #else
>  
>  static inline int
> @@ -100,5 +109,10 @@ static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>  static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
>  					       phys_addr_t addr) {}
>  
> +static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
> +{
> +	return NULL;
> +}
> +
>  #endif	/* CONFIG_IOMMU_MSI */
>  #endif	/* __MSI_IOMMU_H */

[PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Alex Williamson]

On Thu, 28 Apr 2016 08:15:21 +0000
Eric Auger <eric.auger@linaro.org> wrote:

> This function checks whether
> - the device belongs to a non default iommu domain
> - this iommu domain requires the MSI address to be mapped.
> 
> If those conditions are met, the function returns the iommu domain
> to be used for mapping the MSI doorbell; else it returns NULL.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> v7 -> v8:
> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
> - the function now takes a struct device *
> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
> ---
>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
>  include/linux/msi-iommu.h | 14 ++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
> index 203e86e..023ff17 100644
> --- a/drivers/iommu/msi-iommu.c
> +++ b/drivers/iommu/msi-iommu.c
> @@ -243,3 +243,20 @@ unlock:
>  	}
>  }
>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
> +
> +struct iommu_domain *iommu_msi_domain(struct device *dev)
> +{
> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
> +	struct iommu_domain_msi_geometry msi_geometry;
> +
> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
> +		return NULL;
> +
> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
> +	if (!msi_geometry.programmable)

It feels like we're conflating ideas with using the "programmable" flag
in this way.  AIUI the IOMMU API consumer is supposed to see the
invalid MSI geometry with the programmable feature set and know to call
iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
that had been done, but that doesn't tell us if it should have been
done.  iommu_msi_msg_pa_to_va() handles this later, if we return
NULL here that function returns success otherwise it goes on to fail if
the iova or msi cookie is not set.  So really what we're trying to flag
is that the MSI geometry participates in the IOMMU-MSI API you've
created and we should pick a feature name that says that rather than
something as vague a "programmable".  Perhaps simply iommu_msi_api
rather than programmable.

BTW, I don't see that you ever set aperture_start/end once
iommu_msi_set_aperture() has been called.  It seems like doing so would
add some consistency to that MSI geometry attribute.

Nice series overall.  Thanks,

Alex

> +		return NULL;
> +
> +	return d;
> +}
> +EXPORT_SYMBOL_GPL(iommu_msi_domain);
> +
> diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
> index 1cd115f..114bd69 100644
> --- a/include/linux/msi-iommu.h
> +++ b/include/linux/msi-iommu.h
> @@ -81,6 +81,15 @@ int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>   */
>  void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
>  
> +/**
> + * iommu_msi_domain: in case the device is upstream to an IOMMU and this IOMMU
> + * translates the MSI transaction, returns the iommu domain the MSI doorbell
> + * address must be mapped in; else returns NULL.
> + *
> + * @dev: device handle
> + */
> +struct iommu_domain *iommu_msi_domain(struct device *dev);
> +
>  #else
>  
>  static inline int
> @@ -100,5 +109,10 @@ static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>  static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
>  					       phys_addr_t addr) {}
>  
> +static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
> +{
> +	return NULL;
> +}
> +
>  #endif	/* CONFIG_IOMMU_MSI */
>  #endif	/* __MSI_IOMMU_H */

Re: [PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Eric Auger]

Hi Alex,
On 04/29/2016 12:27 AM, Alex Williamson wrote:
> On Thu, 28 Apr 2016 08:15:21 +0000
> Eric Auger <eric.auger@linaro.org> wrote:
> 
>> This function checks whether
>> - the device belongs to a non default iommu domain
>> - this iommu domain requires the MSI address to be mapped.
>>
>> If those conditions are met, the function returns the iommu domain
>> to be used for mapping the MSI doorbell; else it returns NULL.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>
>> ---
>>
>> v7 -> v8:
>> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
>> - the function now takes a struct device *
>> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
>> ---
>>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
>>  include/linux/msi-iommu.h | 14 ++++++++++++++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
>> index 203e86e..023ff17 100644
>> --- a/drivers/iommu/msi-iommu.c
>> +++ b/drivers/iommu/msi-iommu.c
>> @@ -243,3 +243,20 @@ unlock:
>>  	}
>>  }
>>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
>> +
>> +struct iommu_domain *iommu_msi_domain(struct device *dev)
>> +{
>> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
>> +	struct iommu_domain_msi_geometry msi_geometry;
>> +
>> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
>> +		return NULL;
>> +
>> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
>> +	if (!msi_geometry.programmable)
> 
> It feels like we're conflating ideas with using the "programmable" flag
> in this way.  AIUI the IOMMU API consumer is supposed to see the
> invalid MSI geometry with the programmable feature set and know to call
> iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
> that had been done, but that doesn't tell us if it should have been
> done.  iommu_msi_msg_pa_to_va() handles this later, if we return
> NULL here that function returns success otherwise it goes on to fail if
> the iova or msi cookie is not set.  So really what we're trying to flag
> is that the MSI geometry participates in the IOMMU-MSI API you've
> created and we should pick a feature name that says that rather than
> something as vague a "programmable".  Perhaps simply iommu_msi_api
> rather than programmable.
Yes I had the same questioning too when I wrote this code. So if my
understanding is correct we would have
- programmable: tells whether the MSI range is fixed or pogrammable and,
- mapping_required (new boolean field): indicating whether MSIs need to
be mapped in the IOMMU
> 
> BTW, I don't see that you ever set aperture_start/end once
> iommu_msi_set_aperture() has been called.  It seems like doing so would
> add some consistency to that MSI geometry attribute.
Indeed currently I mentionned:
/* In case the aperture is programmable, start/end are set to 0 */

If I set start/end in iommu_msi_set_aperture then I think I should also
return the actual values in iommu_domain_get_attr. I thought the extra
care to handle the possible race between the set_aperture (msi_iommu)
and the get_attr (iommu) was maybe not worth the benefits:
is_aperture_set is not visible to get_attr so I would be forced to
introduce a mutex at iommu_domain level or call an msi-iommu function
from iommu implementation. So I preferred to keep start/end as read-only
info initialized by the iommu driver.

> 
> Nice series overall.  Thanks,

Thanks

Eric
> 
> Alex
> 
>> +		return NULL;
>> +
>> +	return d;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_msi_domain);
>> +
>> diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
>> index 1cd115f..114bd69 100644
>> --- a/include/linux/msi-iommu.h
>> +++ b/include/linux/msi-iommu.h
>> @@ -81,6 +81,15 @@ int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>>   */
>>  void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
>>  
>> +/**
>> + * iommu_msi_domain: in case the device is upstream to an IOMMU and this IOMMU
>> + * translates the MSI transaction, returns the iommu domain the MSI doorbell
>> + * address must be mapped in; else returns NULL.
>> + *
>> + * @dev: device handle
>> + */
>> +struct iommu_domain *iommu_msi_domain(struct device *dev);
>> +
>>  #else
>>  
>>  static inline int
>> @@ -100,5 +109,10 @@ static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>>  static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
>>  					       phys_addr_t addr) {}
>>  
>> +static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
>> +{
>> +	return NULL;
>> +}
>> +
>>  #endif	/* CONFIG_IOMMU_MSI */
>>  #endif	/* __MSI_IOMMU_H */
> 

Re: [PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Eric Auger]

Hi Alex,
On 04/29/2016 12:27 AM, Alex Williamson wrote:
> On Thu, 28 Apr 2016 08:15:21 +0000
> Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> 
>> This function checks whether
>> - the device belongs to a non default iommu domain
>> - this iommu domain requires the MSI address to be mapped.
>>
>> If those conditions are met, the function returns the iommu domain
>> to be used for mapping the MSI doorbell; else it returns NULL.
>>
>> Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> ---
>>
>> v7 -> v8:
>> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
>> - the function now takes a struct device *
>> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
>> ---
>>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
>>  include/linux/msi-iommu.h | 14 ++++++++++++++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
>> index 203e86e..023ff17 100644
>> --- a/drivers/iommu/msi-iommu.c
>> +++ b/drivers/iommu/msi-iommu.c
>> @@ -243,3 +243,20 @@ unlock:
>>  	}
>>  }
>>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
>> +
>> +struct iommu_domain *iommu_msi_domain(struct device *dev)
>> +{
>> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
>> +	struct iommu_domain_msi_geometry msi_geometry;
>> +
>> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
>> +		return NULL;
>> +
>> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
>> +	if (!msi_geometry.programmable)
> 
> It feels like we're conflating ideas with using the "programmable" flag
> in this way.  AIUI the IOMMU API consumer is supposed to see the
> invalid MSI geometry with the programmable feature set and know to call
> iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
> that had been done, but that doesn't tell us if it should have been
> done.  iommu_msi_msg_pa_to_va() handles this later, if we return
> NULL here that function returns success otherwise it goes on to fail if
> the iova or msi cookie is not set.  So really what we're trying to flag
> is that the MSI geometry participates in the IOMMU-MSI API you've
> created and we should pick a feature name that says that rather than
> something as vague a "programmable".  Perhaps simply iommu_msi_api
> rather than programmable.
Yes I had the same questioning too when I wrote this code. So if my
understanding is correct we would have
- programmable: tells whether the MSI range is fixed or pogrammable and,
- mapping_required (new boolean field): indicating whether MSIs need to
be mapped in the IOMMU
> 
> BTW, I don't see that you ever set aperture_start/end once
> iommu_msi_set_aperture() has been called.  It seems like doing so would
> add some consistency to that MSI geometry attribute.
Indeed currently I mentionned:
/* In case the aperture is programmable, start/end are set to 0 */

If I set start/end in iommu_msi_set_aperture then I think I should also
return the actual values in iommu_domain_get_attr. I thought the extra
care to handle the possible race between the set_aperture (msi_iommu)
and the get_attr (iommu) was maybe not worth the benefits:
is_aperture_set is not visible to get_attr so I would be forced to
introduce a mutex at iommu_domain level or call an msi-iommu function
from iommu implementation. So I preferred to keep start/end as read-only
info initialized by the iommu driver.

> 
> Nice series overall.  Thanks,

Thanks

Eric
> 
> Alex
> 
>> +		return NULL;
>> +
>> +	return d;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_msi_domain);
>> +
>> diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
>> index 1cd115f..114bd69 100644
>> --- a/include/linux/msi-iommu.h
>> +++ b/include/linux/msi-iommu.h
>> @@ -81,6 +81,15 @@ int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>>   */
>>  void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
>>  
>> +/**
>> + * iommu_msi_domain: in case the device is upstream to an IOMMU and this IOMMU
>> + * translates the MSI transaction, returns the iommu domain the MSI doorbell
>> + * address must be mapped in; else returns NULL.
>> + *
>> + * @dev: device handle
>> + */
>> +struct iommu_domain *iommu_msi_domain(struct device *dev);
>> +
>>  #else
>>  
>>  static inline int
>> @@ -100,5 +109,10 @@ static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>>  static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
>>  					       phys_addr_t addr) {}
>>  
>> +static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
>> +{
>> +	return NULL;
>> +}
>> +
>>  #endif	/* CONFIG_IOMMU_MSI */
>>  #endif	/* __MSI_IOMMU_H */
> 

[PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Eric Auger]

Hi Alex,
On 04/29/2016 12:27 AM, Alex Williamson wrote:
> On Thu, 28 Apr 2016 08:15:21 +0000
> Eric Auger <eric.auger@linaro.org> wrote:
> 
>> This function checks whether
>> - the device belongs to a non default iommu domain
>> - this iommu domain requires the MSI address to be mapped.
>>
>> If those conditions are met, the function returns the iommu domain
>> to be used for mapping the MSI doorbell; else it returns NULL.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>
>> ---
>>
>> v7 -> v8:
>> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
>> - the function now takes a struct device *
>> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
>> ---
>>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
>>  include/linux/msi-iommu.h | 14 ++++++++++++++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
>> index 203e86e..023ff17 100644
>> --- a/drivers/iommu/msi-iommu.c
>> +++ b/drivers/iommu/msi-iommu.c
>> @@ -243,3 +243,20 @@ unlock:
>>  	}
>>  }
>>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
>> +
>> +struct iommu_domain *iommu_msi_domain(struct device *dev)
>> +{
>> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
>> +	struct iommu_domain_msi_geometry msi_geometry;
>> +
>> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
>> +		return NULL;
>> +
>> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
>> +	if (!msi_geometry.programmable)
> 
> It feels like we're conflating ideas with using the "programmable" flag
> in this way.  AIUI the IOMMU API consumer is supposed to see the
> invalid MSI geometry with the programmable feature set and know to call
> iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
> that had been done, but that doesn't tell us if it should have been
> done.  iommu_msi_msg_pa_to_va() handles this later, if we return
> NULL here that function returns success otherwise it goes on to fail if
> the iova or msi cookie is not set.  So really what we're trying to flag
> is that the MSI geometry participates in the IOMMU-MSI API you've
> created and we should pick a feature name that says that rather than
> something as vague a "programmable".  Perhaps simply iommu_msi_api
> rather than programmable.
Yes I had the same questioning too when I wrote this code. So if my
understanding is correct we would have
- programmable: tells whether the MSI range is fixed or pogrammable and,
- mapping_required (new boolean field): indicating whether MSIs need to
be mapped in the IOMMU
> 
> BTW, I don't see that you ever set aperture_start/end once
> iommu_msi_set_aperture() has been called.  It seems like doing so would
> add some consistency to that MSI geometry attribute.
Indeed currently I mentionned:
/* In case the aperture is programmable, start/end are set to 0 */

If I set start/end in iommu_msi_set_aperture then I think I should also
return the actual values in iommu_domain_get_attr. I thought the extra
care to handle the possible race between the set_aperture (msi_iommu)
and the get_attr (iommu) was maybe not worth the benefits:
is_aperture_set is not visible to get_attr so I would be forced to
introduce a mutex at iommu_domain level or call an msi-iommu function
from iommu implementation. So I preferred to keep start/end as read-only
info initialized by the iommu driver.

> 
> Nice series overall.  Thanks,

Thanks

Eric
> 
> Alex
> 
>> +		return NULL;
>> +
>> +	return d;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_msi_domain);
>> +
>> diff --git a/include/linux/msi-iommu.h b/include/linux/msi-iommu.h
>> index 1cd115f..114bd69 100644
>> --- a/include/linux/msi-iommu.h
>> +++ b/include/linux/msi-iommu.h
>> @@ -81,6 +81,15 @@ int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>>   */
>>  void iommu_msi_put_doorbell_iova(struct iommu_domain *domain, phys_addr_t addr);
>>  
>> +/**
>> + * iommu_msi_domain: in case the device is upstream to an IOMMU and this IOMMU
>> + * translates the MSI transaction, returns the iommu domain the MSI doorbell
>> + * address must be mapped in; else returns NULL.
>> + *
>> + * @dev: device handle
>> + */
>> +struct iommu_domain *iommu_msi_domain(struct device *dev);
>> +
>>  #else
>>  
>>  static inline int
>> @@ -100,5 +109,10 @@ static inline int iommu_msi_get_doorbell_iova(struct iommu_domain *domain,
>>  static inline void iommu_msi_put_doorbell_iova(struct iommu_domain *domain,
>>  					       phys_addr_t addr) {}
>>  
>> +static inline struct iommu_domain *iommu_msi_domain(struct device *dev)
>> +{
>> +	return NULL;
>> +}
>> +
>>  #endif	/* CONFIG_IOMMU_MSI */
>>  #endif	/* __MSI_IOMMU_H */
> 

Re: [PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Alex Williamson]

Hi Eric,

On Mon, 2 May 2016 17:48:13 +0200
Eric Auger <eric.auger@linaro.org> wrote:

> Hi Alex,
> On 04/29/2016 12:27 AM, Alex Williamson wrote:
> > On Thu, 28 Apr 2016 08:15:21 +0000
> > Eric Auger <eric.auger@linaro.org> wrote:
> >   
> >> This function checks whether
> >> - the device belongs to a non default iommu domain
> >> - this iommu domain requires the MSI address to be mapped.
> >>
> >> If those conditions are met, the function returns the iommu domain
> >> to be used for mapping the MSI doorbell; else it returns NULL.
> >>
> >> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> >>
> >> ---
> >>
> >> v7 -> v8:
> >> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
> >> - the function now takes a struct device *
> >> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
> >> ---
> >>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
> >>  include/linux/msi-iommu.h | 14 ++++++++++++++
> >>  2 files changed, 31 insertions(+)
> >>
> >> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
> >> index 203e86e..023ff17 100644
> >> --- a/drivers/iommu/msi-iommu.c
> >> +++ b/drivers/iommu/msi-iommu.c
> >> @@ -243,3 +243,20 @@ unlock:
> >>  	}
> >>  }
> >>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
> >> +
> >> +struct iommu_domain *iommu_msi_domain(struct device *dev)
> >> +{
> >> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
> >> +	struct iommu_domain_msi_geometry msi_geometry;
> >> +
> >> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
> >> +		return NULL;
> >> +
> >> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
> >> +	if (!msi_geometry.programmable)  
> > 
> > It feels like we're conflating ideas with using the "programmable" flag
> > in this way.  AIUI the IOMMU API consumer is supposed to see the
> > invalid MSI geometry with the programmable feature set and know to call
> > iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
> > that had been done, but that doesn't tell us if it should have been
> > done.  iommu_msi_msg_pa_to_va() handles this later, if we return
> > NULL here that function returns success otherwise it goes on to fail if
> > the iova or msi cookie is not set.  So really what we're trying to flag
> > is that the MSI geometry participates in the IOMMU-MSI API you've
> > created and we should pick a feature name that says that rather than
> > something as vague a "programmable".  Perhaps simply iommu_msi_api
> > rather than programmable.  
> Yes I had the same questioning too when I wrote this code. So if my
> understanding is correct we would have
> - programmable: tells whether the MSI range is fixed or pogrammable and,
> - mapping_required (new boolean field): indicating whether MSIs need to
> be mapped in the IOMMU

Let's say we had a flag "iommu_msi_supported", doesn't that already
tell us whether the MSI range is programmable and the API to use to do
it?  Can't we figure out if mapping is required based on whether
iommu_msi_supported is set and aperture_start and aperture_end indicate
a valid range?  It seems like what we want on this code path is to
simply know whether the IOMMU domain is relevant to the IOMMU MSI API,
which would be abundantly clear with such a flag.

> > 
> > BTW, I don't see that you ever set aperture_start/end once
> > iommu_msi_set_aperture() has been called.  It seems like doing so would
> > add some consistency to that MSI geometry attribute.  
> Indeed currently I mentionned:
> /* In case the aperture is programmable, start/end are set to 0 */

Which is confusing, if a user sets an aperture, I would think they'd
like to see the MSI geometry updated to reflect that.

> If I set start/end in iommu_msi_set_aperture then I think I should also
> return the actual values in iommu_domain_get_attr. I thought the extra
> care to handle the possible race between the set_aperture (msi_iommu)
> and the get_attr (iommu) was maybe not worth the benefits:
> is_aperture_set is not visible to get_attr so I would be forced to
> introduce a mutex at iommu_domain level or call an msi-iommu function
> from iommu implementation. So I preferred to keep start/end as read-only
> info initialized by the iommu driver.

Seems like a race between iommu_msi_set_aperture() and
iommu_domain_get_attr() is the caller's problem.  We can always define
that start >= end is invalid and set them in the right order that
iommu_domain_get_attr() may get different versions of invalid, but it
will always be invalid or valid.  A helper function could tell us if we
have a valid range rather than using is_aperture_set.  Thanks,

Alex

Re: [PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Alex Williamson]

Hi Eric,

On Mon, 2 May 2016 17:48:13 +0200
Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> Hi Alex,
> On 04/29/2016 12:27 AM, Alex Williamson wrote:
> > On Thu, 28 Apr 2016 08:15:21 +0000
> > Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >   
> >> This function checks whether
> >> - the device belongs to a non default iommu domain
> >> - this iommu domain requires the MSI address to be mapped.
> >>
> >> If those conditions are met, the function returns the iommu domain
> >> to be used for mapping the MSI doorbell; else it returns NULL.
> >>
> >> Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >>
> >> ---
> >>
> >> v7 -> v8:
> >> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
> >> - the function now takes a struct device *
> >> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
> >> ---
> >>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
> >>  include/linux/msi-iommu.h | 14 ++++++++++++++
> >>  2 files changed, 31 insertions(+)
> >>
> >> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
> >> index 203e86e..023ff17 100644
> >> --- a/drivers/iommu/msi-iommu.c
> >> +++ b/drivers/iommu/msi-iommu.c
> >> @@ -243,3 +243,20 @@ unlock:
> >>  	}
> >>  }
> >>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
> >> +
> >> +struct iommu_domain *iommu_msi_domain(struct device *dev)
> >> +{
> >> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
> >> +	struct iommu_domain_msi_geometry msi_geometry;
> >> +
> >> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
> >> +		return NULL;
> >> +
> >> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
> >> +	if (!msi_geometry.programmable)  
> > 
> > It feels like we're conflating ideas with using the "programmable" flag
> > in this way.  AIUI the IOMMU API consumer is supposed to see the
> > invalid MSI geometry with the programmable feature set and know to call
> > iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
> > that had been done, but that doesn't tell us if it should have been
> > done.  iommu_msi_msg_pa_to_va() handles this later, if we return
> > NULL here that function returns success otherwise it goes on to fail if
> > the iova or msi cookie is not set.  So really what we're trying to flag
> > is that the MSI geometry participates in the IOMMU-MSI API you've
> > created and we should pick a feature name that says that rather than
> > something as vague a "programmable".  Perhaps simply iommu_msi_api
> > rather than programmable.  
> Yes I had the same questioning too when I wrote this code. So if my
> understanding is correct we would have
> - programmable: tells whether the MSI range is fixed or pogrammable and,
> - mapping_required (new boolean field): indicating whether MSIs need to
> be mapped in the IOMMU

Let's say we had a flag "iommu_msi_supported", doesn't that already
tell us whether the MSI range is programmable and the API to use to do
it?  Can't we figure out if mapping is required based on whether
iommu_msi_supported is set and aperture_start and aperture_end indicate
a valid range?  It seems like what we want on this code path is to
simply know whether the IOMMU domain is relevant to the IOMMU MSI API,
which would be abundantly clear with such a flag.

> > 
> > BTW, I don't see that you ever set aperture_start/end once
> > iommu_msi_set_aperture() has been called.  It seems like doing so would
> > add some consistency to that MSI geometry attribute.  
> Indeed currently I mentionned:
> /* In case the aperture is programmable, start/end are set to 0 */

Which is confusing, if a user sets an aperture, I would think they'd
like to see the MSI geometry updated to reflect that.

> If I set start/end in iommu_msi_set_aperture then I think I should also
> return the actual values in iommu_domain_get_attr. I thought the extra
> care to handle the possible race between the set_aperture (msi_iommu)
> and the get_attr (iommu) was maybe not worth the benefits:
> is_aperture_set is not visible to get_attr so I would be forced to
> introduce a mutex at iommu_domain level or call an msi-iommu function
> from iommu implementation. So I preferred to keep start/end as read-only
> info initialized by the iommu driver.

Seems like a race between iommu_msi_set_aperture() and
iommu_domain_get_attr() is the caller's problem.  We can always define
that start >= end is invalid and set them in the right order that
iommu_domain_get_attr() may get different versions of invalid, but it
will always be invalid or valid.  A helper function could tell us if we
have a valid range rather than using is_aperture_set.  Thanks,

Alex

[PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Alex Williamson]

Hi Eric,

On Mon, 2 May 2016 17:48:13 +0200
Eric Auger <eric.auger@linaro.org> wrote:

> Hi Alex,
> On 04/29/2016 12:27 AM, Alex Williamson wrote:
> > On Thu, 28 Apr 2016 08:15:21 +0000
> > Eric Auger <eric.auger@linaro.org> wrote:
> >   
> >> This function checks whether
> >> - the device belongs to a non default iommu domain
> >> - this iommu domain requires the MSI address to be mapped.
> >>
> >> If those conditions are met, the function returns the iommu domain
> >> to be used for mapping the MSI doorbell; else it returns NULL.
> >>
> >> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> >>
> >> ---
> >>
> >> v7 -> v8:
> >> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
> >> - the function now takes a struct device *
> >> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
> >> ---
> >>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
> >>  include/linux/msi-iommu.h | 14 ++++++++++++++
> >>  2 files changed, 31 insertions(+)
> >>
> >> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
> >> index 203e86e..023ff17 100644
> >> --- a/drivers/iommu/msi-iommu.c
> >> +++ b/drivers/iommu/msi-iommu.c
> >> @@ -243,3 +243,20 @@ unlock:
> >>  	}
> >>  }
> >>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
> >> +
> >> +struct iommu_domain *iommu_msi_domain(struct device *dev)
> >> +{
> >> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
> >> +	struct iommu_domain_msi_geometry msi_geometry;
> >> +
> >> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
> >> +		return NULL;
> >> +
> >> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
> >> +	if (!msi_geometry.programmable)  
> > 
> > It feels like we're conflating ideas with using the "programmable" flag
> > in this way.  AIUI the IOMMU API consumer is supposed to see the
> > invalid MSI geometry with the programmable feature set and know to call
> > iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
> > that had been done, but that doesn't tell us if it should have been
> > done.  iommu_msi_msg_pa_to_va() handles this later, if we return
> > NULL here that function returns success otherwise it goes on to fail if
> > the iova or msi cookie is not set.  So really what we're trying to flag
> > is that the MSI geometry participates in the IOMMU-MSI API you've
> > created and we should pick a feature name that says that rather than
> > something as vague a "programmable".  Perhaps simply iommu_msi_api
> > rather than programmable.  
> Yes I had the same questioning too when I wrote this code. So if my
> understanding is correct we would have
> - programmable: tells whether the MSI range is fixed or pogrammable and,
> - mapping_required (new boolean field): indicating whether MSIs need to
> be mapped in the IOMMU

Let's say we had a flag "iommu_msi_supported", doesn't that already
tell us whether the MSI range is programmable and the API to use to do
it?  Can't we figure out if mapping is required based on whether
iommu_msi_supported is set and aperture_start and aperture_end indicate
a valid range?  It seems like what we want on this code path is to
simply know whether the IOMMU domain is relevant to the IOMMU MSI API,
which would be abundantly clear with such a flag.

> > 
> > BTW, I don't see that you ever set aperture_start/end once
> > iommu_msi_set_aperture() has been called.  It seems like doing so would
> > add some consistency to that MSI geometry attribute.  
> Indeed currently I mentionned:
> /* In case the aperture is programmable, start/end are set to 0 */

Which is confusing, if a user sets an aperture, I would think they'd
like to see the MSI geometry updated to reflect that.

> If I set start/end in iommu_msi_set_aperture then I think I should also
> return the actual values in iommu_domain_get_attr. I thought the extra
> care to handle the possible race between the set_aperture (msi_iommu)
> and the get_attr (iommu) was maybe not worth the benefits:
> is_aperture_set is not visible to get_attr so I would be forced to
> introduce a mutex at iommu_domain level or call an msi-iommu function
> from iommu implementation. So I preferred to keep start/end as read-only
> info initialized by the iommu driver.

Seems like a race between iommu_msi_set_aperture() and
iommu_domain_get_attr() is the caller's problem.  We can always define
that start >= end is invalid and set them in the right order that
iommu_domain_get_attr() may get different versions of invalid, but it
will always be invalid or valid.  A helper function could tell us if we
have a valid range rather than using is_aperture_set.  Thanks,

Alex

Re: [PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Eric Auger]

Hi Alex,
On 05/02/2016 10:23 PM, Alex Williamson wrote:
> Hi Eric,
> 
> On Mon, 2 May 2016 17:48:13 +0200
> Eric Auger <eric.auger@linaro.org> wrote:
> 
>> Hi Alex,
>> On 04/29/2016 12:27 AM, Alex Williamson wrote:
>>> On Thu, 28 Apr 2016 08:15:21 +0000
>>> Eric Auger <eric.auger@linaro.org> wrote:
>>>   
>>>> This function checks whether
>>>> - the device belongs to a non default iommu domain
>>>> - this iommu domain requires the MSI address to be mapped.
>>>>
>>>> If those conditions are met, the function returns the iommu domain
>>>> to be used for mapping the MSI doorbell; else it returns NULL.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>>
>>>> ---
>>>>
>>>> v7 -> v8:
>>>> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
>>>> - the function now takes a struct device *
>>>> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
>>>> ---
>>>>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
>>>>  include/linux/msi-iommu.h | 14 ++++++++++++++
>>>>  2 files changed, 31 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
>>>> index 203e86e..023ff17 100644
>>>> --- a/drivers/iommu/msi-iommu.c
>>>> +++ b/drivers/iommu/msi-iommu.c
>>>> @@ -243,3 +243,20 @@ unlock:
>>>>  	}
>>>>  }
>>>>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
>>>> +
>>>> +struct iommu_domain *iommu_msi_domain(struct device *dev)
>>>> +{
>>>> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
>>>> +	struct iommu_domain_msi_geometry msi_geometry;
>>>> +
>>>> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
>>>> +		return NULL;
>>>> +
>>>> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
>>>> +	if (!msi_geometry.programmable)  
>>>
>>> It feels like we're conflating ideas with using the "programmable" flag
>>> in this way.  AIUI the IOMMU API consumer is supposed to see the
>>> invalid MSI geometry with the programmable feature set and know to call
>>> iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
>>> that had been done, but that doesn't tell us if it should have been
>>> done.  iommu_msi_msg_pa_to_va() handles this later, if we return
>>> NULL here that function returns success otherwise it goes on to fail if
>>> the iova or msi cookie is not set.  So really what we're trying to flag
>>> is that the MSI geometry participates in the IOMMU-MSI API you've
>>> created and we should pick a feature name that says that rather than
>>> something as vague a "programmable".  Perhaps simply iommu_msi_api
>>> rather than programmable.  
>> Yes I had the same questioning too when I wrote this code. So if my
>> understanding is correct we would have
>> - programmable: tells whether the MSI range is fixed or pogrammable and,
>> - mapping_required (new boolean field): indicating whether MSIs need to
>> be mapped in the IOMMU
> 
> Let's say we had a flag "iommu_msi_supported", doesn't that already
> tell us whether the MSI range is programmable and the API to use to do
> it?  Can't we figure out if mapping is required based on whether
> iommu_msi_supported is set and aperture_start and aperture_end indicate
> a valid range?  It seems like what we want on this code path is to
> simply know whether the IOMMU domain is relevant to the IOMMU MSI API,
> which would be abundantly clear with such a flag.
OK I now get your point. Thanks for the clarification. I renamed
programmable into iommu_msi_supported.
> 
>>>
>>> BTW, I don't see that you ever set aperture_start/end once
>>> iommu_msi_set_aperture() has been called.  It seems like doing so would
>>> add some consistency to that MSI geometry attribute.  
>> Indeed currently I mentionned:
>> /* In case the aperture is programmable, start/end are set to 0 */
> 
> Which is confusing, if a user sets an aperture, I would think they'd
> like to see the MSI geometry updated to reflect that.
> 
>> If I set start/end in iommu_msi_set_aperture then I think I should also
>> return the actual values in iommu_domain_get_attr. I thought the extra
>> care to handle the possible race between the set_aperture (msi_iommu)
>> and the get_attr (iommu) was maybe not worth the benefits:
>> is_aperture_set is not visible to get_attr so I would be forced to
>> introduce a mutex at iommu_domain level or call an msi-iommu function
>> from iommu implementation. So I preferred to keep start/end as read-only
>> info initialized by the iommu driver.
> 
> Seems like a race between iommu_msi_set_aperture() and
> iommu_domain_get_attr() is the caller's problem.  We can always define
> that start >= end is invalid and set them in the right order that
> iommu_domain_get_attr() may get different versions of invalid, but it
> will always be invalid or valid.  A helper function could tell us if we
> have a valid range rather than using is_aperture_set.  Thanks,


Agreed; I added an iommu_domain_msi_aperture_valid helper function.

Thanks!

Eric
> 
> Alex
> 

Re: [PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Eric Auger]

Hi Alex,
On 05/02/2016 10:23 PM, Alex Williamson wrote:
> Hi Eric,
> 
> On Mon, 2 May 2016 17:48:13 +0200
> Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> 
>> Hi Alex,
>> On 04/29/2016 12:27 AM, Alex Williamson wrote:
>>> On Thu, 28 Apr 2016 08:15:21 +0000
>>> Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>   
>>>> This function checks whether
>>>> - the device belongs to a non default iommu domain
>>>> - this iommu domain requires the MSI address to be mapped.
>>>>
>>>> If those conditions are met, the function returns the iommu domain
>>>> to be used for mapping the MSI doorbell; else it returns NULL.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>
>>>> ---
>>>>
>>>> v7 -> v8:
>>>> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
>>>> - the function now takes a struct device *
>>>> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
>>>> ---
>>>>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
>>>>  include/linux/msi-iommu.h | 14 ++++++++++++++
>>>>  2 files changed, 31 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
>>>> index 203e86e..023ff17 100644
>>>> --- a/drivers/iommu/msi-iommu.c
>>>> +++ b/drivers/iommu/msi-iommu.c
>>>> @@ -243,3 +243,20 @@ unlock:
>>>>  	}
>>>>  }
>>>>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
>>>> +
>>>> +struct iommu_domain *iommu_msi_domain(struct device *dev)
>>>> +{
>>>> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
>>>> +	struct iommu_domain_msi_geometry msi_geometry;
>>>> +
>>>> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
>>>> +		return NULL;
>>>> +
>>>> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
>>>> +	if (!msi_geometry.programmable)  
>>>
>>> It feels like we're conflating ideas with using the "programmable" flag
>>> in this way.  AIUI the IOMMU API consumer is supposed to see the
>>> invalid MSI geometry with the programmable feature set and know to call
>>> iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
>>> that had been done, but that doesn't tell us if it should have been
>>> done.  iommu_msi_msg_pa_to_va() handles this later, if we return
>>> NULL here that function returns success otherwise it goes on to fail if
>>> the iova or msi cookie is not set.  So really what we're trying to flag
>>> is that the MSI geometry participates in the IOMMU-MSI API you've
>>> created and we should pick a feature name that says that rather than
>>> something as vague a "programmable".  Perhaps simply iommu_msi_api
>>> rather than programmable.  
>> Yes I had the same questioning too when I wrote this code. So if my
>> understanding is correct we would have
>> - programmable: tells whether the MSI range is fixed or pogrammable and,
>> - mapping_required (new boolean field): indicating whether MSIs need to
>> be mapped in the IOMMU
> 
> Let's say we had a flag "iommu_msi_supported", doesn't that already
> tell us whether the MSI range is programmable and the API to use to do
> it?  Can't we figure out if mapping is required based on whether
> iommu_msi_supported is set and aperture_start and aperture_end indicate
> a valid range?  It seems like what we want on this code path is to
> simply know whether the IOMMU domain is relevant to the IOMMU MSI API,
> which would be abundantly clear with such a flag.
OK I now get your point. Thanks for the clarification. I renamed
programmable into iommu_msi_supported.
> 
>>>
>>> BTW, I don't see that you ever set aperture_start/end once
>>> iommu_msi_set_aperture() has been called.  It seems like doing so would
>>> add some consistency to that MSI geometry attribute.  
>> Indeed currently I mentionned:
>> /* In case the aperture is programmable, start/end are set to 0 */
> 
> Which is confusing, if a user sets an aperture, I would think they'd
> like to see the MSI geometry updated to reflect that.
> 
>> If I set start/end in iommu_msi_set_aperture then I think I should also
>> return the actual values in iommu_domain_get_attr. I thought the extra
>> care to handle the possible race between the set_aperture (msi_iommu)
>> and the get_attr (iommu) was maybe not worth the benefits:
>> is_aperture_set is not visible to get_attr so I would be forced to
>> introduce a mutex at iommu_domain level or call an msi-iommu function
>> from iommu implementation. So I preferred to keep start/end as read-only
>> info initialized by the iommu driver.
> 
> Seems like a race between iommu_msi_set_aperture() and
> iommu_domain_get_attr() is the caller's problem.  We can always define
> that start >= end is invalid and set them in the right order that
> iommu_domain_get_attr() may get different versions of invalid, but it
> will always be invalid or valid.  A helper function could tell us if we
> have a valid range rather than using is_aperture_set.  Thanks,


Agreed; I added an iommu_domain_msi_aperture_valid helper function.

Thanks!

Eric
> 
> Alex
> 

[PATCH v8 6/8] iommu/msi-iommu: iommu_msi_domain

[Eric Auger]

Hi Alex,
On 05/02/2016 10:23 PM, Alex Williamson wrote:
> Hi Eric,
> 
> On Mon, 2 May 2016 17:48:13 +0200
> Eric Auger <eric.auger@linaro.org> wrote:
> 
>> Hi Alex,
>> On 04/29/2016 12:27 AM, Alex Williamson wrote:
>>> On Thu, 28 Apr 2016 08:15:21 +0000
>>> Eric Auger <eric.auger@linaro.org> wrote:
>>>   
>>>> This function checks whether
>>>> - the device belongs to a non default iommu domain
>>>> - this iommu domain requires the MSI address to be mapped.
>>>>
>>>> If those conditions are met, the function returns the iommu domain
>>>> to be used for mapping the MSI doorbell; else it returns NULL.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>>
>>>> ---
>>>>
>>>> v7 -> v8:
>>>> - renamed  iommu_msi_mapping_desc_to_domain to iommu_msi_domain
>>>> - the function now takes a struct device *
>>>> - use DOMAIN_ATTR_MSI_GEOMETRY attribute
>>>> ---
>>>>  drivers/iommu/msi-iommu.c | 17 +++++++++++++++++
>>>>  include/linux/msi-iommu.h | 14 ++++++++++++++
>>>>  2 files changed, 31 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/msi-iommu.c b/drivers/iommu/msi-iommu.c
>>>> index 203e86e..023ff17 100644
>>>> --- a/drivers/iommu/msi-iommu.c
>>>> +++ b/drivers/iommu/msi-iommu.c
>>>> @@ -243,3 +243,20 @@ unlock:
>>>>  	}
>>>>  }
>>>>  EXPORT_SYMBOL_GPL(iommu_msi_put_doorbell_iova);
>>>> +
>>>> +struct iommu_domain *iommu_msi_domain(struct device *dev)
>>>> +{
>>>> +	struct iommu_domain *d = iommu_get_domain_for_dev(dev);
>>>> +	struct iommu_domain_msi_geometry msi_geometry;
>>>> +
>>>> +	if (!d || (d->type == IOMMU_DOMAIN_DMA))
>>>> +		return NULL;
>>>> +
>>>> +	iommu_domain_get_attr(d, DOMAIN_ATTR_MSI_GEOMETRY, &msi_geometry);
>>>> +	if (!msi_geometry.programmable)  
>>>
>>> It feels like we're conflating ideas with using the "programmable" flag
>>> in this way.  AIUI the IOMMU API consumer is supposed to see the
>>> invalid MSI geometry with the programmable feature set and know to call
>>> iommu_msi_set_aperture().  Looking for the msi_cookie would tell us if
>>> that had been done, but that doesn't tell us if it should have been
>>> done.  iommu_msi_msg_pa_to_va() handles this later, if we return
>>> NULL here that function returns success otherwise it goes on to fail if
>>> the iova or msi cookie is not set.  So really what we're trying to flag
>>> is that the MSI geometry participates in the IOMMU-MSI API you've
>>> created and we should pick a feature name that says that rather than
>>> something as vague a "programmable".  Perhaps simply iommu_msi_api
>>> rather than programmable.  
>> Yes I had the same questioning too when I wrote this code. So if my
>> understanding is correct we would have
>> - programmable: tells whether the MSI range is fixed or pogrammable and,
>> - mapping_required (new boolean field): indicating whether MSIs need to
>> be mapped in the IOMMU
> 
> Let's say we had a flag "iommu_msi_supported", doesn't that already
> tell us whether the MSI range is programmable and the API to use to do
> it?  Can't we figure out if mapping is required based on whether
> iommu_msi_supported is set and aperture_start and aperture_end indicate
> a valid range?  It seems like what we want on this code path is to
> simply know whether the IOMMU domain is relevant to the IOMMU MSI API,
> which would be abundantly clear with such a flag.
OK I now get your point. Thanks for the clarification. I renamed
programmable into iommu_msi_supported.
> 
>>>
>>> BTW, I don't see that you ever set aperture_start/end once
>>> iommu_msi_set_aperture() has been called.  It seems like doing so would
>>> add some consistency to that MSI geometry attribute.  
>> Indeed currently I mentionned:
>> /* In case the aperture is programmable, start/end are set to 0 */
> 
> Which is confusing, if a user sets an aperture, I would think they'd
> like to see the MSI geometry updated to reflect that.
> 
>> If I set start/end in iommu_msi_set_aperture then I think I should also
>> return the actual values in iommu_domain_get_attr. I thought the extra
>> care to handle the possible race between the set_aperture (msi_iommu)
>> and the get_attr (iommu) was maybe not worth the benefits:
>> is_aperture_set is not visible to get_attr so I would be forced to
>> introduce a mutex at iommu_domain level or call an msi-iommu function
>> from iommu implementation. So I preferred to keep start/end as read-only
>> info initialized by the iommu driver.
> 
> Seems like a race between iommu_msi_set_aperture() and
> iommu_domain_get_attr() is the caller's problem.  We can always define
> that start >= end is invalid and set them in the right order that
> iommu_domain_get_attr() may get different versions of invalid, but it
> will always be invalid or valid.  A helper function could tell us if we
> have a valid range rather than using is_aperture_set.  Thanks,


Agreed; I added an iommu_domain_msi_aperture_valid helper function.

Thanks!

Eric
> 
> Alex
>