[RFC,v6,3/5] block: add a helper function to merge the segments by an IOMMU
diff mbox series

Message ID 1560421215-10750-4-git-send-email-yoshihiro.shimoda.uh@renesas.com
State New
Headers show
Series
  • treewide: improve R-Car SDHI performance
Related show

Commit Message

Yoshihiro Shimoda June 13, 2019, 10:20 a.m. UTC
This patch adds a helper function whether a queue can merge
the segments by an IOMMU.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 block/blk-settings.c   | 28 ++++++++++++++++++++++++++++
 include/linux/blkdev.h |  2 ++
 2 files changed, 30 insertions(+)

Comments

Christoph Hellwig June 14, 2019, 7:22 a.m. UTC | #1
I'm a little worried about this directly calling into the iommu
API instead of going through the DMA mapping code.  We still have
plenty of iommus not using the iommu layer for DMA mapping.  But at
least this code is in the block layer and not the driver, so maybe
we can live with it.
Robin Murphy June 14, 2019, 9:54 a.m. UTC | #2
On 13/06/2019 11:20, Yoshihiro Shimoda wrote:
> This patch adds a helper function whether a queue can merge
> the segments by an IOMMU.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>   block/blk-settings.c   | 28 ++++++++++++++++++++++++++++
>   include/linux/blkdev.h |  2 ++
>   2 files changed, 30 insertions(+)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 45f2c52..4e4e13e 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -4,9 +4,11 @@
>    */
>   #include <linux/bio.h>
>   #include <linux/blkdev.h>
> +#include <linux/device.h>
>   #include <linux/gcd.h>
>   #include <linux/gfp.h>
>   #include <linux/init.h>
> +#include <linux/iommu.h>
>   #include <linux/jiffies.h>
>   #include <linux/kernel.h>
>   #include <linux/lcm.h>
> @@ -831,6 +833,32 @@ void blk_queue_write_cache(struct request_queue *q, bool wc, bool fua)
>   }
>   EXPORT_SYMBOL_GPL(blk_queue_write_cache);
>   
> +/**
> + * blk_queue_can_use_iommu_merging - configure queue for merging segments.
> + * @q:		the request queue for the device
> + * @dev:	the device pointer for dma
> + *
> + * Tell the block layer about the iommu merging of @q.
> + */
> +bool blk_queue_can_use_iommu_merging(struct request_queue *q,
> +				     struct device *dev)
> +{
> +	struct iommu_domain *domain;
> +
> +	/*
> +	 * If the device DMA is translated by an IOMMU, we can assume
> +	 * the device can merge the segments.
> +	 */
> +	if (!device_iommu_mapped(dev))

Careful here - I think this validates the comment I made when this 
function was introduced, in that that name doesn't necesarily mean what 
it sounds like it might mean - "iommu_mapped" was as close as we managed 
to get to a convenient shorthand for "performs DMA through an 
IOMMU-API-enabled IOMMU". Specifically, it does not imply that 
translation is *currently* active; if you boot with "iommu=pt" or 
equivalent this will still return true even though the device will be 
using direct/SWIOTLB DMA ops without any IOMMU translation.

Robin.

> +		return false;
> +
> +	domain = iommu_get_domain_for_dev(dev);
> +	/* No need to update max_segment_size. see blk_queue_virt_boundary() */
> +	blk_queue_virt_boundary(q, iommu_get_minimum_page_size(domain) - 1);
> +
> +	return true;
> +}
> +
>   static int __init blk_settings_init(void)
>   {
>   	blk_max_low_pfn = max_low_pfn - 1;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 592669b..4d1f7dc 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1091,6 +1091,8 @@ extern void blk_queue_dma_alignment(struct request_queue *, int);
>   extern void blk_queue_update_dma_alignment(struct request_queue *, int);
>   extern void blk_queue_rq_timeout(struct request_queue *, unsigned int);
>   extern void blk_queue_write_cache(struct request_queue *q, bool enabled, bool fua);
> +extern bool blk_queue_can_use_iommu_merging(struct request_queue *q,
> +					    struct device *dev);
>   
>   /*
>    * Number of physical segments as sent to the device.
>
Yoshihiro Shimoda June 17, 2019, 6:29 a.m. UTC | #3
Hi Robin,

> From: Robin Murphy, Sent: Friday, June 14, 2019 6:55 PM
> 
> On 13/06/2019 11:20, Yoshihiro Shimoda wrote:
<snip>
> > +bool blk_queue_can_use_iommu_merging(struct request_queue *q,
> > +				     struct device *dev)
> > +{
> > +	struct iommu_domain *domain;
> > +
> > +	/*
> > +	 * If the device DMA is translated by an IOMMU, we can assume
> > +	 * the device can merge the segments.
> > +	 */
> > +	if (!device_iommu_mapped(dev))
> 
> Careful here - I think this validates the comment I made when this
> function was introduced, in that that name doesn't necesarily mean what
> it sounds like it might mean - "iommu_mapped" was as close as we managed
> to get to a convenient shorthand for "performs DMA through an
> IOMMU-API-enabled IOMMU". Specifically, it does not imply that
> translation is *currently* active; if you boot with "iommu=pt" or
> equivalent this will still return true even though the device will be
> using direct/SWIOTLB DMA ops without any IOMMU translation.

Thank you for your comments. I understood the mean of "iommu_mapped" and
this patch's condition causes a problem on iommu=pt.
So, I'll add and additional condition like
"domain->type == IOMMU_DOMAIN_DMA" to check whether the translation is
currently active on the domain or not.

Best regards,
Yoshihiro Shimoda

Patch
diff mbox series

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 45f2c52..4e4e13e 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -4,9 +4,11 @@ 
  */
 #include <linux/bio.h>
 #include <linux/blkdev.h>
+#include <linux/device.h>
 #include <linux/gcd.h>
 #include <linux/gfp.h>
 #include <linux/init.h>
+#include <linux/iommu.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/lcm.h>
@@ -831,6 +833,32 @@  void blk_queue_write_cache(struct request_queue *q, bool wc, bool fua)
 }
 EXPORT_SYMBOL_GPL(blk_queue_write_cache);
 
+/**
+ * blk_queue_can_use_iommu_merging - configure queue for merging segments.
+ * @q:		the request queue for the device
+ * @dev:	the device pointer for dma
+ *
+ * Tell the block layer about the iommu merging of @q.
+ */
+bool blk_queue_can_use_iommu_merging(struct request_queue *q,
+				     struct device *dev)
+{
+	struct iommu_domain *domain;
+
+	/*
+	 * If the device DMA is translated by an IOMMU, we can assume
+	 * the device can merge the segments.
+	 */
+	if (!device_iommu_mapped(dev))
+		return false;
+
+	domain = iommu_get_domain_for_dev(dev);
+	/* No need to update max_segment_size. see blk_queue_virt_boundary() */
+	blk_queue_virt_boundary(q, iommu_get_minimum_page_size(domain) - 1);
+
+	return true;
+}
+
 static int __init blk_settings_init(void)
 {
 	blk_max_low_pfn = max_low_pfn - 1;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 592669b..4d1f7dc 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1091,6 +1091,8 @@  extern void blk_queue_dma_alignment(struct request_queue *, int);
 extern void blk_queue_update_dma_alignment(struct request_queue *, int);
 extern void blk_queue_rq_timeout(struct request_queue *, unsigned int);
 extern void blk_queue_write_cache(struct request_queue *q, bool enabled, bool fua);
+extern bool blk_queue_can_use_iommu_merging(struct request_queue *q,
+					    struct device *dev);
 
 /*
  * Number of physical segments as sent to the device.