Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1552260 > unrolled thread

[PATCH v6 01/18] iommu/dma: Allow MSI-only cookies

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v6 01/18] iommu/dma: Allow MSI-only cookies Eric Auger <eric.auger@redhat.com> - 2017-01-05 20:20 +0100
    Re: [PATCH v6 01/18] iommu/dma: Allow MSI-only cookies Joerg Roedel <joro@8bytes.org> - 2017-01-06 12:10 +0100
      Re: [PATCH v6 01/18] iommu/dma: Allow MSI-only cookies Robin Murphy <robin.murphy@arm.com> - 2017-01-06 13:20 +0100
        Re: [PATCH v6 01/18] iommu/dma: Allow MSI-only cookies Auger Eric <eric.auger@redhat.com> - 2017-01-06 14:10 +0100
      Re: [PATCH v6 01/18] iommu/dma: Allow MSI-only cookies Auger Eric <eric.auger@redhat.com> - 2017-01-06 13:20 +0100

#1552260 — [PATCH v6 01/18] iommu/dma: Allow MSI-only cookies

IOMMU domain users such as VFIO face a similar problem to DMA API ops
with regard to mapping MSI messages in systems where the MSI write is
subject to IOMMU translation. With the relevant infrastructure now in
place for managed DMA domains, it's actually really simple for other
users to piggyback off that and reap the benefits without giving up
their own IOVA management, and without having to reinvent their own
wheel in the MSI layer.

Allow such users to opt into automatic MSI remapping by dedicating a
region of their IOVA space to a managed cookie, and extend the mapping
routine to implement a trivial linear allocator in such cases, to avoid
the needless overhead of a full-blown IOVA domain.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
[Eric] fixed 2 issues on MSI path
---
 drivers/iommu/dma-iommu.c | 116 +++++++++++++++++++++++++++++++++++++---------
 include/linux/dma-iommu.h |   7 +++
 2 files changed, 101 insertions(+), 22 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 2db0d64..f2c47ad 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -37,10 +37,19 @@ struct iommu_dma_msi_page {
 	phys_addr_t		phys;
 };
 
+enum iommu_dma_cookie_type {
+	IOMMU_DMA_IOVA_COOKIE,
+	IOMMU_DMA_MSI_COOKIE,
+};
+
 struct iommu_dma_cookie {
-	struct iova_domain	iovad;
-	struct list_head	msi_page_list;
-	spinlock_t		msi_lock;
+	union {
+		struct iova_domain	iovad;
+		dma_addr_t		msi_iova;
+	};
+	struct list_head		msi_page_list;
+	spinlock_t			msi_lock;
+	enum iommu_dma_cookie_type	type;
 };
 
 static inline struct iova_domain *cookie_iovad(struct iommu_domain *domain)
@@ -53,6 +62,19 @@ int iommu_dma_init(void)
 	return iova_cache_get();
 }
 
+static struct iommu_dma_cookie *__cookie_alloc(enum iommu_dma_cookie_type type)
+{
+	struct iommu_dma_cookie *cookie;
+
+	cookie = kzalloc(sizeof(*cookie), GFP_KERNEL);
+	if (cookie) {
+		spin_lock_init(&cookie->msi_lock);
+		INIT_LIST_HEAD(&cookie->msi_page_list);
+		cookie->type = type;
+	}
+	return cookie;
+}
+
 /**
  * iommu_get_dma_cookie - Acquire DMA-API resources for a domain
  * @domain: IOMMU domain to prepare for DMA-API usage
@@ -62,25 +84,53 @@ int iommu_dma_init(void)
  */
 int iommu_get_dma_cookie(struct iommu_domain *domain)
 {
+	if (domain->iova_cookie)
+		return -EEXIST;
+
+	domain->iova_cookie = __cookie_alloc(IOMMU_DMA_IOVA_COOKIE);
+	if (!domain->iova_cookie)
+		return -ENOMEM;
+
+	return 0;
+}
+EXPORT_SYMBOL(iommu_get_dma_cookie);
+
+/**
+ * iommu_get_msi_cookie - Acquire just MSI remapping resources
+ * @domain: IOMMU domain to prepare
+ * @base: Start address of IOVA region for MSI mappings
+ *
+ * Users who manage their own IOVA allocation and do not want DMA API support,
+ * but would still like to take advantage of automatic MSI remapping, can use
+ * this to initialise their own domain appropriately. Users should reserve a
+ * contiguous IOVA region, starting at @base, large enough to accommodate the
+ * number of PAGE_SIZE mappings necessary to cover every MSI doorbell address
+ * used by the devices attached to @domain.
+ */
+int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
+{
 	struct iommu_dma_cookie *cookie;
 
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -EINVAL;
+
 	if (domain->iova_cookie)
 		return -EEXIST;
 
-	cookie = kzalloc(sizeof(*cookie), GFP_KERNEL);
+	cookie = __cookie_alloc(IOMMU_DMA_MSI_COOKIE);
 	if (!cookie)
 		return -ENOMEM;
 
-	spin_lock_init(&cookie->msi_lock);
-	INIT_LIST_HEAD(&cookie->msi_page_list);
+	cookie->msi_iova = base;
 	domain->iova_cookie = cookie;
 	return 0;
 }
-EXPORT_SYMBOL(iommu_get_dma_cookie);
+EXPORT_SYMBOL(iommu_get_msi_cookie);
 
 /**
  * iommu_put_dma_cookie - Release a domain's DMA mapping resources
- * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
+ * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie() or
+ *          iommu_get_msi_cookie()
  *
  * IOMMU drivers should normally call this from their domain_free callback.
  */
@@ -92,7 +142,7 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
 	if (!cookie)
 		return;
 
-	if (cookie->iovad.granule)
+	if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule)
 		put_iova_domain(&cookie->iovad);
 
 	list_for_each_entry_safe(msi, tmp, &cookie->msi_page_list, list) {
@@ -137,11 +187,12 @@ static void iova_reserve_pci_windows(struct pci_dev *dev,
 int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 		u64 size, struct device *dev)
 {
-	struct iova_domain *iovad = cookie_iovad(domain);
+	struct iommu_dma_cookie *cookie = domain->iova_cookie;
+	struct iova_domain *iovad = &cookie->iovad;
 	unsigned long order, base_pfn, end_pfn;
 
-	if (!iovad)
-		return -ENODEV;
+	if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE)
+		return -EINVAL;
 
 	/* Use the smallest supported page size for IOVA granularity */
 	order = __ffs(domain->pgsize_bitmap);
@@ -662,11 +713,21 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
 {
 	struct iommu_dma_cookie *cookie = domain->iova_cookie;
 	struct iommu_dma_msi_page *msi_page;
-	struct iova_domain *iovad = &cookie->iovad;
+	struct iova_domain *iovad;
 	struct iova *iova;
 	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+	size_t size;
+
+	if (cookie->type == IOMMU_DMA_IOVA_COOKIE) {
+		iovad = &cookie->iovad;
+		size = iovad->granule;
+	} else {
+		iovad = NULL;
+		size = PAGE_SIZE;
+	}
+
+	msi_addr &= ~(phys_addr_t)(size - 1);
 
-	msi_addr &= ~(phys_addr_t)iova_mask(iovad);
 	list_for_each_entry(msi_page, &cookie->msi_page_list, list)
 		if (msi_page->phys == msi_addr)
 			return msi_page;
@@ -675,13 +736,18 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
 	if (!msi_page)
 		return NULL;
 
-	iova = __alloc_iova(domain, iovad->granule, dma_get_mask(dev));
-	if (!iova)
-		goto out_free_page;
-
 	msi_page->phys = msi_addr;
-	msi_page->iova = iova_dma_addr(iovad, iova);
-	if (iommu_map(domain, msi_page->iova, msi_addr, iovad->granule, prot))
+	if (iovad) {
+		iova = __alloc_iova(domain, size, dma_get_mask(dev));
+		if (!iova)
+			goto out_free_page;
+		msi_page->iova = iova_dma_addr(iovad, iova);
+	} else {
+		msi_page->iova = cookie->msi_iova;
+		cookie->msi_iova += size;
+	}
+
+	if (iommu_map(domain, msi_page->iova, msi_addr, size, prot))
 		goto out_free_iova;
 
 	INIT_LIST_HEAD(&msi_page->list);
@@ -689,7 +755,10 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
 	return msi_page;
 
 out_free_iova:
-	__free_iova(iovad, iova);
+	if (iovad)
+		__free_iova(iovad, iova);
+	else
+		cookie->msi_iova -= size;
 out_free_page:
 	kfree(msi_page);
 	return NULL;
@@ -730,7 +799,10 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
 		msg->data = ~0U;
 	} else {
 		msg->address_hi = upper_32_bits(msi_page->iova);
-		msg->address_lo &= iova_mask(&cookie->iovad);
+		if (cookie->type == IOMMU_DMA_IOVA_COOKIE)
+			msg->address_lo &= iova_mask(&cookie->iovad);
+		else
+			msg->address_lo &= (PAGE_SIZE - 1);
 		msg->address_lo += lower_32_bits(msi_page->iova);
 	}
 }
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 7f7e9a7..c843461 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -27,6 +27,7 @@
 
 /* Domain management interface for IOMMU drivers */
 int iommu_get_dma_cookie(struct iommu_domain *domain);
+int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base);
 void iommu_put_dma_cookie(struct iommu_domain *domain);
 
 /* Setup call for arch DMA mapping code */
@@ -86,6 +87,12 @@ static inline int iommu_get_dma_cookie(struct iommu_domain *domain)
 	return -ENODEV;
 }
 
+static inline int iommu_get_msi_cookie(struct iommu_domain *domain,
+				       dma_addr_t base)
+{
+	return -ENODEV;
+}
+
 static inline void iommu_put_dma_cookie(struct iommu_domain *domain)
 {
 }
-- 
1.9.1

[toc] | [next] | [standalone]

#1552692

On Thu, Jan 05, 2017 at 07:04:29PM +0000, Eric Auger wrote:
>  struct iommu_dma_cookie {
> -	struct iova_domain	iovad;
> -	struct list_head	msi_page_list;
> -	spinlock_t		msi_lock;
> +	union {
> +		struct iova_domain	iovad;
> +		dma_addr_t		msi_iova;
> +	};
> +	struct list_head		msi_page_list;
> +	spinlock_t			msi_lock;
> +	enum iommu_dma_cookie_type	type;

Please move the type to the beginning of the struct and add a comment
how the type relates to the union.



	Joerg

[toc] | [prev] | [next] | [standalone]

#1552730

On 06/01/17 11:46, Auger Eric wrote:
> 
> 
> On 06/01/2017 11:59, Joerg Roedel wrote:
>> On Thu, Jan 05, 2017 at 07:04:29PM +0000, Eric Auger wrote:
>>>  struct iommu_dma_cookie {
>>> -	struct iova_domain	iovad;
>>> -	struct list_head	msi_page_list;
>>> -	spinlock_t		msi_lock;
>>> +	union {
>>> +		struct iova_domain	iovad;
>>> +		dma_addr_t		msi_iova;
>>> +	};
>>> +	struct list_head		msi_page_list;
>>> +	spinlock_t			msi_lock;
>>> +	enum iommu_dma_cookie_type	type;
>>
>> Please move the type to the beginning of the struct and add a comment
>> how the type relates to the union.
> 
> Sure
> 
> Thank you for the review.

FWIW I already had a cleaned up version of this patch, I just hadn't
mentioned it. I've pushed out an update with that change added too[1].

Robin.

[1]:http://linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/misc

> 
> Best regards
> 
> Eric
>>
>>
>>
>> 	Joerg
>>

[toc] | [prev] | [next] | [standalone]

#1552766

Hi Robin,
On 06/01/2017 13:12, Robin Murphy wrote:
> On 06/01/17 11:46, Auger Eric wrote:
>>
>>
>> On 06/01/2017 11:59, Joerg Roedel wrote:
>>> On Thu, Jan 05, 2017 at 07:04:29PM +0000, Eric Auger wrote:
>>>>  struct iommu_dma_cookie {
>>>> -	struct iova_domain	iovad;
>>>> -	struct list_head	msi_page_list;
>>>> -	spinlock_t		msi_lock;
>>>> +	union {
>>>> +		struct iova_domain	iovad;
>>>> +		dma_addr_t		msi_iova;
>>>> +	};
>>>> +	struct list_head		msi_page_list;
>>>> +	spinlock_t			msi_lock;
>>>> +	enum iommu_dma_cookie_type	type;
>>>
>>> Please move the type to the beginning of the struct and add a comment
>>> how the type relates to the union.
>>
>> Sure
>>
>> Thank you for the review.
> 
> FWIW I already had a cleaned up version of this patch, I just hadn't
> mentioned it. I've pushed out an update with that change added too[1].
> 
> Robin.
> 
> [1]:http://linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/misc

Great, thanks!

Eric
> 
>>
>> Best regards
>>
>> Eric
>>>
>>>
>>>
>>> 	Joerg
>>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[toc] | [prev] | [next] | [standalone]

#1552739

On 06/01/2017 11:59, Joerg Roedel wrote:
> On Thu, Jan 05, 2017 at 07:04:29PM +0000, Eric Auger wrote:
>>  struct iommu_dma_cookie {
>> -	struct iova_domain	iovad;
>> -	struct list_head	msi_page_list;
>> -	spinlock_t		msi_lock;
>> +	union {
>> +		struct iova_domain	iovad;
>> +		dma_addr_t		msi_iova;
>> +	};
>> +	struct list_head		msi_page_list;
>> +	spinlock_t			msi_lock;
>> +	enum iommu_dma_cookie_type	type;
> 
> Please move the type to the beginning of the struct and add a comment
> how the type relates to the union.

Sure

Thank you for the review.

Best regards

Eric
> 
> 
> 
> 	Joerg
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web