2019-01-31 16:35:23

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB

Hi,

here is the next 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]/

V4: https://lore.kernel.org/lkml/[email protected]/

V5: 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.

Changes to v5 are:

- Changed patch 3 to uninline dma_max_mapping_size()

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 | 11 +++++++++++
include/linux/dma-mapping.h | 8 ++++++++
include/linux/swiotlb.h | 11 +++++++++++
include/linux/virtio.h | 2 ++
kernel/dma/direct.c | 11 +++++++++++
kernel/dma/mapping.c | 14 ++++++++++++++
kernel/dma/swiotlb.c | 14 ++++++++++++++
9 files changed, 85 insertions(+), 4 deletions(-)

--
2.17.1



2019-01-31 16:34:32

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 3/5] dma: Introduce dma_max_mapping_size()

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]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
Documentation/DMA-API.txt | 8 ++++++++
include/linux/dma-mapping.h | 8 ++++++++
kernel/dma/direct.c | 11 +++++++++++
kernel/dma/mapping.c | 14 ++++++++++++++
4 files changed, 41 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..5b21f14802e1 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>

@@ -460,6 +463,7 @@ int dma_supported(struct device *dev, u64 mask);
int dma_set_mask(struct device *dev, u64 mask);
int dma_set_coherent_mask(struct device *dev, u64 mask);
u64 dma_get_required_mask(struct device *dev);
+size_t dma_max_mapping_size(struct device *dev);
#else /* CONFIG_HAS_DMA */
static inline dma_addr_t dma_map_page_attrs(struct device *dev,
struct page *page, size_t offset, size_t size,
@@ -561,6 +565,10 @@ static inline u64 dma_get_required_mask(struct device *dev)
{
return 0;
}
+static inline size_t dma_max_mapping_size(struct device *dev)
+{
+ return 0;
+}
#endif /* CONFIG_HAS_DMA */

static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
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;
+}
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index a11006b6d8e8..5753008ab286 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -357,3 +357,17 @@ void dma_cache_sync(struct device *dev, void *vaddr, size_t size,
ops->cache_sync(dev, vaddr, size, dir);
}
EXPORT_SYMBOL(dma_cache_sync);
+
+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;
+}
+EXPORT_SYMBOL_GPL(dma_max_mapping_size);
--
2.17.1


2019-01-31 16:34:36

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 4/5] virtio: Introduce virtio_max_dma_size()

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]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/virtio/virtio_ring.c | 11 +++++++++++
include/linux/virtio.h | 2 ++
2 files changed, 13 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd7e755484e3..8a31c6862b2b 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -266,6 +266,17 @@ 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;
+}
+EXPORT_SYMBOL_GPL(virtio_max_dma_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


2019-01-31 16:34:43

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size

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]>
Reviewed-by: Christoph Hellwig <[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


2019-01-31 16:35:02

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 2/5] swiotlb: Add is_swiotlb_active() function

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]>
Reviewed-by: Christoph Hellwig <[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


2019-01-31 16:35:14

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 1/5] swiotlb: Introduce swiotlb_max_mapping_size()

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]>
Reviewed-by: Christoph Hellwig <[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


2019-02-01 08:11:31

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB

For some reason patch 5 didn't make it to my inbox, but assuming
nothing has changed this whole series looks good to me now.

2019-02-05 21:03:02

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB

On Thu, Jan 31, 2019 at 05:33:58PM +0100, Joerg Roedel wrote:
> Hi,
>
> here is the next 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]/
>
> V4: https://lore.kernel.org/lkml/[email protected]/
>
> V5: 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.
>
> Changes to v5 are:
>
> - Changed patch 3 to uninline dma_max_mapping_size()

And this lead to problems reported by kbuild :(

BTW when you repost, can I ask you to pls include
the version in all patches? Both --subject-prefix
and -v flags to git format-patch will do this for you.


> 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 | 11 +++++++++++
> include/linux/dma-mapping.h | 8 ++++++++
> include/linux/swiotlb.h | 11 +++++++++++
> include/linux/virtio.h | 2 ++
> kernel/dma/direct.c | 11 +++++++++++
> kernel/dma/mapping.c | 14 ++++++++++++++
> kernel/dma/swiotlb.c | 14 ++++++++++++++
> 9 files changed, 85 insertions(+), 4 deletions(-)
>
> --
> 2.17.1

2019-02-05 21:03:14

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB

On Fri, Feb 01, 2019 at 09:09:46AM +0100, Christoph Hellwig wrote:
> For some reason patch 5 didn't make it to my inbox, but assuming
> nothing has changed this whole series looks good to me now.

Could you send a formal series ack pls?


2019-02-07 08:48:33

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB

Hi Michael,

On Tue, Feb 05, 2019 at 03:52:38PM -0500, Michael S. Tsirkin wrote:
> On Thu, Jan 31, 2019 at 05:33:58PM +0100, Joerg Roedel wrote:
> > Changes to v5 are:
> >
> > - Changed patch 3 to uninline dma_max_mapping_size()
>
> And this lead to problems reported by kbuild :(

Hmm, I didn't get any kbuild emails for this series. Can you please
forward it me so that I can look into it?

> BTW when you repost, can I ask you to pls include
> the version in all patches? Both --subject-prefix
> and -v flags to git format-patch will do this for you.

Will do, thanks.


Regards,

Joerg

2019-02-07 08:50:12

by Jörg Rödel

[permalink] [raw]
Subject: Re: [PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB

On Thu, Feb 07, 2019 at 09:46:13AM +0100, Joerg Roedel wrote:
> Hmm, I didn't get any kbuild emails for this series. Can you please
> forward it me so that I can look into it?

Nevermind, just found them in another inbox.


Joerg