Hi,
here is the fourth version of this patch-set. Previous
versions can be found here:
V1: https://lore.kernel.org/lkml/[email protected]/
V2: https://lore.kernel.org/lkml/[email protected]/
V3: https://lore.kernel.org/lkml/[email protected]/
The problem solved here is a limitation of the SWIOTLB implementation,
which does not support allocations larger than 256kb. When the
virtio-blk driver tries to read/write a block larger than that, the
allocation of the dma-handle fails and an IO error is reported.
For all changes to v3, a diff to v3 of the patch-set is at
the end of this email.
Please review.
Thanks,
Joerg
Joerg Roedel (5):
swiotlb: Introduce swiotlb_max_mapping_size()
swiotlb: Add is_swiotlb_active() function
dma: Introduce dma_max_mapping_size()
virtio: Introduce virtio_max_dma_size()
virtio-blk: Consider virtio_max_dma_size() for maximum segment size
Documentation/DMA-API.txt | 8 ++++++++
drivers/block/virtio_blk.c | 10 ++++++----
drivers/virtio/virtio_ring.c | 10 ++++++++++
include/linux/dma-mapping.h | 16 ++++++++++++++++
include/linux/swiotlb.h | 11 +++++++++++
include/linux/virtio.h | 2 ++
kernel/dma/direct.c | 11 +++++++++++
kernel/dma/swiotlb.c | 14 ++++++++++++++
8 files changed, 78 insertions(+), 4 deletions(-)
--
2.17.1
diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index e133ccd60228..acfe3d0f78d1 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -195,6 +195,14 @@ Requesting the required mask does not alter the current mask. If you
wish to take advantage of it, you should issue a dma_set_mask()
call to set the mask to the value returned.
+::
+
+ size_t
+ dma_direct_max_mapping_size(struct device *dev);
+
+Returns the maximum size of a mapping for the device. The size parameter
+of the mapping functions like dma_map_single(), dma_map_page() and
+others should not be larger than the returned value.
Part Id - Streaming DMA mappings
--------------------------------
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 5c087d330b4b..e9e786b4b598 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -62,7 +62,7 @@ extern void swiotlb_tbl_sync_single(struct device *hwdev,
extern int
swiotlb_dma_supported(struct device *hwdev, u64 mask);
-extern size_t swiotlb_max_mapping_size(struct device *dev);
+size_t swiotlb_max_mapping_size(struct device *dev);
bool is_swiotlb_active(void);
#ifdef CONFIG_SWIOTLB
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 9fbd075081d9..c873f9cc2146 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -670,5 +670,9 @@ size_t swiotlb_max_mapping_size(struct device *dev)
bool is_swiotlb_active(void)
{
- return !no_iotlb_memory;
+ /*
+ * When SWIOTLB is initialized, even if io_tlb_start points to physical
+ * address zero, io_tlb_end surely doesn't.
+ */
+ return io_tlb_end != 0;
}
From: Joerg Roedel <[email protected]>
This function will be used from dma_direct code to determine
the maximum segment size of a dma mapping.
Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
include/linux/swiotlb.h | 6 ++++++
kernel/dma/swiotlb.c | 9 +++++++++
2 files changed, 15 insertions(+)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 1c22d96e1742..e9e786b4b598 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -63,6 +63,7 @@ extern void swiotlb_tbl_sync_single(struct device *hwdev,
extern int
swiotlb_dma_supported(struct device *hwdev, u64 mask);
size_t swiotlb_max_mapping_size(struct device *dev);
+bool is_swiotlb_active(void);
#ifdef CONFIG_SWIOTLB
extern enum swiotlb_force swiotlb_force;
@@ -100,6 +101,11 @@ static inline size_t swiotlb_max_mapping_size(struct device *dev)
{
return SIZE_MAX;
}
+
+static inline bool is_swiotlb_active(void)
+{
+ return false;
+}
#endif /* CONFIG_SWIOTLB */
extern void swiotlb_print_info(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 9cb21259cb0b..c873f9cc2146 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -667,3 +667,12 @@ size_t swiotlb_max_mapping_size(struct device *dev)
{
return ((size_t)1 << IO_TLB_SHIFT) * IO_TLB_SEGSIZE;
}
+
+bool is_swiotlb_active(void)
+{
+ /*
+ * When SWIOTLB is initialized, even if io_tlb_start points to physical
+ * address zero, io_tlb_end surely doesn't.
+ */
+ return io_tlb_end != 0;
+}
--
2.17.1
From: Joerg Roedel <[email protected]>
The function returns the maximum size that can be mapped
using DMA-API functions. The patch also adds the
implementation for direct DMA and a new dma_map_ops pointer
so that other implementations can expose their limit.
Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
Documentation/DMA-API.txt | 8 ++++++++
include/linux/dma-mapping.h | 16 ++++++++++++++++
kernel/dma/direct.c | 11 +++++++++++
3 files changed, 35 insertions(+)
diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index e133ccd60228..acfe3d0f78d1 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -195,6 +195,14 @@ Requesting the required mask does not alter the current mask. If you
wish to take advantage of it, you should issue a dma_set_mask()
call to set the mask to the value returned.
+::
+
+ size_t
+ dma_direct_max_mapping_size(struct device *dev);
+
+Returns the maximum size of a mapping for the device. The size parameter
+of the mapping functions like dma_map_single(), dma_map_page() and
+others should not be larger than the returned value.
Part Id - Streaming DMA mappings
--------------------------------
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f6ded992c183..a3ca8a71a704 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -130,6 +130,7 @@ struct dma_map_ops {
enum dma_data_direction direction);
int (*dma_supported)(struct device *dev, u64 mask);
u64 (*get_required_mask)(struct device *dev);
+ size_t (*max_mapping_size)(struct device *dev);
};
#define DMA_MAPPING_ERROR (~(dma_addr_t)0)
@@ -257,6 +258,8 @@ static inline void dma_direct_sync_sg_for_cpu(struct device *dev,
}
#endif
+size_t dma_direct_max_mapping_size(struct device *dev);
+
#ifdef CONFIG_HAS_DMA
#include <asm/dma-mapping.h>
@@ -440,6 +443,19 @@ static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
return 0;
}
+static inline size_t dma_max_mapping_size(struct device *dev)
+{
+ const struct dma_map_ops *ops = get_dma_ops(dev);
+ size_t size = SIZE_MAX;
+
+ if (dma_is_direct(ops))
+ size = dma_direct_max_mapping_size(dev);
+ else if (ops && ops->max_mapping_size)
+ size = ops->max_mapping_size(dev);
+
+ return size;
+}
+
void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
gfp_t flag, unsigned long attrs);
void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..6310ad01f915 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -380,3 +380,14 @@ int dma_direct_supported(struct device *dev, u64 mask)
*/
return mask >= __phys_to_dma(dev, min_mask);
}
+
+size_t dma_direct_max_mapping_size(struct device *dev)
+{
+ size_t size = SIZE_MAX;
+
+ /* If SWIOTLB is active, use its maximum mapping size */
+ if (is_swiotlb_active())
+ size = swiotlb_max_mapping_size(dev);
+
+ return size;
+}
--
2.17.1
From: Joerg Roedel <[email protected]>
Segments can't be larger than the maximum DMA mapping size
supported on the platform. Take that into account when
setting the maximum segment size for a block device.
Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/block/virtio_blk.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index b16a887bbd02..4bc083b7c9b5 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -723,7 +723,7 @@ static int virtblk_probe(struct virtio_device *vdev)
struct request_queue *q;
int err, index;
- u32 v, blk_size, sg_elems, opt_io_size;
+ u32 v, blk_size, max_size, sg_elems, opt_io_size;
u16 min_io_size;
u8 physical_block_exp, alignment_offset;
@@ -826,14 +826,16 @@ static int virtblk_probe(struct virtio_device *vdev)
/* No real sector limit. */
blk_queue_max_hw_sectors(q, -1U);
+ max_size = virtio_max_dma_size(vdev);
+
/* Host can optionally specify maximum segment size and number of
* segments. */
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
struct virtio_blk_config, size_max, &v);
if (!err)
- blk_queue_max_segment_size(q, v);
- else
- blk_queue_max_segment_size(q, -1U);
+ max_size = min(max_size, v);
+
+ blk_queue_max_segment_size(q, max_size);
/* Host can optionally specify the block size of the device */
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE,
--
2.17.1
From: Joerg Roedel <[email protected]>
This function returns the maximum segment size for a single
dma transaction of a virtio device. The possible limit comes
from the SWIOTLB implementation in the Linux kernel, that
has an upper limit of (currently) 256kb of contiguous
memory it can map. Other DMA-API implementations might also
have limits.
Use the new dma_max_mapping_size() function to determine the
maximum mapping size when DMA-API is in use for virtio.
Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/virtio/virtio_ring.c | 10 ++++++++++
include/linux/virtio.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd7e755484e3..9ca3fe6af9fa 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -266,6 +266,16 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
return false;
}
+size_t virtio_max_dma_size(struct virtio_device *vdev)
+{
+ size_t max_segment_size = SIZE_MAX;
+
+ if (vring_use_dma_api(vdev))
+ max_segment_size = dma_max_mapping_size(&vdev->dev);
+
+ return max_segment_size;
+}
+
static void *vring_alloc_queue(struct virtio_device *vdev, size_t size,
dma_addr_t *dma_handle, gfp_t flag)
{
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index fa1b5da2804e..673fe3ef3607 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -157,6 +157,8 @@ int virtio_device_freeze(struct virtio_device *dev);
int virtio_device_restore(struct virtio_device *dev);
#endif
+size_t virtio_max_dma_size(struct virtio_device *vdev);
+
#define virtio_device_for_each_vq(vdev, vq) \
list_for_each_entry(vq, &vdev->vqs, list)
--
2.17.1
From: Joerg Roedel <[email protected]>
The function returns the maximum size that can be remapped
by the SWIOTLB implementation. This function will be later
exposed to users through the DMA-API.
Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
include/linux/swiotlb.h | 5 +++++
kernel/dma/swiotlb.c | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 7c007ed7505f..1c22d96e1742 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -62,6 +62,7 @@ extern void swiotlb_tbl_sync_single(struct device *hwdev,
extern int
swiotlb_dma_supported(struct device *hwdev, u64 mask);
+size_t swiotlb_max_mapping_size(struct device *dev);
#ifdef CONFIG_SWIOTLB
extern enum swiotlb_force swiotlb_force;
@@ -95,6 +96,10 @@ static inline unsigned int swiotlb_max_segment(void)
{
return 0;
}
+static inline size_t swiotlb_max_mapping_size(struct device *dev)
+{
+ return SIZE_MAX;
+}
#endif /* CONFIG_SWIOTLB */
extern void swiotlb_print_info(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 1fb6fd68b9c7..9cb21259cb0b 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -662,3 +662,8 @@ swiotlb_dma_supported(struct device *hwdev, u64 mask)
{
return __phys_to_dma(hwdev, io_tlb_end - 1) <= mask;
}
+
+size_t swiotlb_max_mapping_size(struct device *dev)
+{
+ return ((size_t)1 << IO_TLB_SHIFT) * IO_TLB_SEGSIZE;
+}
--
2.17.1
On Tue, Jan 29, 2019 at 09:43:39AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> This function will be used from dma_direct code to determine
> the maximum segment size of a dma mapping.
>
> Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
> Signed-off-by: Joerg Roedel <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
On Tue, Jan 29, 2019 at 09:43:38AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> The function returns the maximum size that can be remapped
> by the SWIOTLB implementation. This function will be later
> exposed to users through the DMA-API.
>
> Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
> Signed-off-by: Joerg Roedel <[email protected]>
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
On Tue, Jan 29, 2019 at 09:43:40AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> The function returns the maximum size that can be mapped
> using DMA-API functions. The patch also adds the
> implementation for direct DMA and a new dma_map_ops pointer
> so that other implementations can expose their limit.
>
> Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
> Signed-off-by: Joerg Roedel <[email protected]>
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
On Tue, Jan 29, 2019 at 09:43:41AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> This function returns the maximum segment size for a single
> dma transaction of a virtio device. The possible limit comes
> from the SWIOTLB implementation in the Linux kernel, that
> has an upper limit of (currently) 256kb of contiguous
> memory it can map. Other DMA-API implementations might also
> have limits.
>
> Use the new dma_max_mapping_size() function to determine the
> maximum mapping size when DMA-API is in use for virtio.
>
> Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
> Signed-off-by: Joerg Roedel <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
On Tue, Jan 29, 2019 at 09:43:42AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> Segments can't be larger than the maximum DMA mapping size
> supported on the platform. Take that into account when
> setting the maximum segment size for a block device.
>
> Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
> Signed-off-by: Joerg Roedel <[email protected]>
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
On 1/29/19 2:43 AM, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> This function returns the maximum segment size for a single
> dma transaction of a virtio device. The possible limit comes
> from the SWIOTLB implementation in the Linux kernel, that
> has an upper limit of (currently) 256kb of contiguous
> memory it can map. Other DMA-API implementations might also
> have limits.
>
> Use the new dma_max_mapping_size() function to determine the
> maximum mapping size when DMA-API is in use for virtio.
>
> Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/virtio/virtio_ring.c | 10 ++++++++++
> include/linux/virtio.h | 2 ++
> 2 files changed, 12 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755484e3..9ca3fe6af9fa 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -266,6 +266,16 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> return false;
> }
>
> +size_t virtio_max_dma_size(struct virtio_device *vdev)
> +{
> + size_t max_segment_size = SIZE_MAX;
> +
> + if (vring_use_dma_api(vdev))
> + max_segment_size = dma_max_mapping_size(&vdev->dev);
> +
> + return max_segment_size;
> +}
When I build with these patches and with the virtio devices as modules I
get a build failure. Looks like this needs an EXPORT_SYMBOL_GPL().
Thanks,
Tom
> +
> static void *vring_alloc_queue(struct virtio_device *vdev, size_t size,
> dma_addr_t *dma_handle, gfp_t flag)
> {
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index fa1b5da2804e..673fe3ef3607 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -157,6 +157,8 @@ int virtio_device_freeze(struct virtio_device *dev);
> int virtio_device_restore(struct virtio_device *dev);
> #endif
>
> +size_t virtio_max_dma_size(struct virtio_device *vdev);
> +
> #define virtio_device_for_each_vq(vdev, vq) \
> list_for_each_entry(vq, &vdev->vqs, list)
>
>
On 1/29/19 2:43 AM, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> The function returns the maximum size that can be mapped
> using DMA-API functions. The patch also adds the
> implementation for direct DMA and a new dma_map_ops pointer
> so that other implementations can expose their limit.
>
> Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> Documentation/DMA-API.txt | 8 ++++++++
> include/linux/dma-mapping.h | 16 ++++++++++++++++
> kernel/dma/direct.c | 11 +++++++++++
> 3 files changed, 35 insertions(+)
>
> diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
> index e133ccd60228..acfe3d0f78d1 100644
> --- a/Documentation/DMA-API.txt
> +++ b/Documentation/DMA-API.txt
> @@ -195,6 +195,14 @@ Requesting the required mask does not alter the current mask. If you
> wish to take advantage of it, you should issue a dma_set_mask()
> call to set the mask to the value returned.
>
> +::
> +
> + size_t
> + dma_direct_max_mapping_size(struct device *dev);
> +
> +Returns the maximum size of a mapping for the device. The size parameter
> +of the mapping functions like dma_map_single(), dma_map_page() and
> +others should not be larger than the returned value.
>
> Part Id - Streaming DMA mappings
> --------------------------------
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index f6ded992c183..a3ca8a71a704 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -130,6 +130,7 @@ struct dma_map_ops {
> enum dma_data_direction direction);
> int (*dma_supported)(struct device *dev, u64 mask);
> u64 (*get_required_mask)(struct device *dev);
> + size_t (*max_mapping_size)(struct device *dev);
> };
>
> #define DMA_MAPPING_ERROR (~(dma_addr_t)0)
> @@ -257,6 +258,8 @@ static inline void dma_direct_sync_sg_for_cpu(struct device *dev,
> }
> #endif
>
> +size_t dma_direct_max_mapping_size(struct device *dev);
> +
> #ifdef CONFIG_HAS_DMA
> #include <asm/dma-mapping.h>
>
> @@ -440,6 +443,19 @@ static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
> return 0;
> }
>
> +static inline size_t dma_max_mapping_size(struct device *dev)
> +{
> + const struct dma_map_ops *ops = get_dma_ops(dev);
> + size_t size = SIZE_MAX;
> +
> + if (dma_is_direct(ops))
> + size = dma_direct_max_mapping_size(dev);
> + else if (ops && ops->max_mapping_size)
> + size = ops->max_mapping_size(dev);
> +
> + return size;
> +}
> +
> void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
> gfp_t flag, unsigned long attrs);
> void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 355d16acee6d..6310ad01f915 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -380,3 +380,14 @@ int dma_direct_supported(struct device *dev, u64 mask)
> */
> return mask >= __phys_to_dma(dev, min_mask);
> }
> +
> +size_t dma_direct_max_mapping_size(struct device *dev)
> +{
> + size_t size = SIZE_MAX;
> +
> + /* If SWIOTLB is active, use its maximum mapping size */
> + if (is_swiotlb_active())
> + size = swiotlb_max_mapping_size(dev);
> +
> + return size;
> +}
When I build with these patches and with the virtio devices as modules I
get a build failure. Looks like this needs an EXPORT_SYMBOL() (I'm
assuming EXPORT_SYMBOL for DMA functions and not EXPORT_SYMBOL_GPL?).
Thanks,
Tom
>
Hi Tom,
On Wed, Jan 30, 2019 at 03:10:29PM +0000, Lendacky, Thomas wrote:
> On 1/29/19 2:43 AM, Joerg Roedel wrote:
> > +size_t virtio_max_dma_size(struct virtio_device *vdev)
> > +{
> > + size_t max_segment_size = SIZE_MAX;
> > +
> > + if (vring_use_dma_api(vdev))
> > + max_segment_size = dma_max_mapping_size(&vdev->dev);
> > +
> > + return max_segment_size;
> > +}
>
> When I build with these patches and with the virtio devices as modules I
> get a build failure. Looks like this needs an EXPORT_SYMBOL_GPL().
Thanks for pointing that out, I added the missing EXPORTs and will send
a new version shortly.
Regards,
Joerg