Commit 2db76d7c3c6d ("lib/scatterlist: sg_page_iter: support sg lists w/o
backing pages") introduced the sg_page_iter_dma_address() function without
providing a way to use it in the general case. If the sg_dma_len() is not
equal to the sg length callers cannot safely use the
for_each_sg_page/sg_page_iter_dma_address combination.
Resolve this API mistake by providing a DMA specific iterator,
for_each_sg_dma_page(), that uses the right length so
sg_page_iter_dma_address() works as expected with all sglists.
A new iterator type is introduced to provide compile-time safety against
wrongly mixing accessors and iterators.
Acked-by: Christoph Hellwig (for scatterlist)
Signed-off-by: Jason Gunthorpe
---
.clang-format | 1 +
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 8 +++-
drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 +-
include/linux/scatterlist.h| 49 ++
lib/scatterlist.c | 26
5 files changed, 76 insertions(+), 12 deletions(-)
v2:
- Drop the vmwgfx fix in favor of keeping it unchanged as there is no
reviewer. Use an ugly case with a comment instead.
I'd like to apply this to the RDMA tree next week, a large series from
Shiraz is waiting on it:
https://patchwork.kernel.org/project/linux-rdma/list/?series=71841=*
Regards,
Jason
diff --git a/.clang-format b/.clang-format
index bc2ffb2a0b5366..335ce29ab8132c 100644
--- a/.clang-format
+++ b/.clang-format
@@ -240,6 +240,7 @@ ForEachMacros:
- 'for_each_set_bit'
- 'for_each_set_bit_from'
- 'for_each_sg'
+ - 'for_each_sg_dma_page'
- 'for_each_sg_page'
- 'for_each_sibling_event'
- '__for_each_thread'
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 31786b200afc47..e84f6aaee778f0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -311,7 +311,13 @@ static dma_addr_t __vmw_piter_dma_addr(struct vmw_piter
*viter)
static dma_addr_t __vmw_piter_sg_addr(struct vmw_piter *viter)
{
- return sg_page_iter_dma_address(>iter);
+ /*
+* FIXME: This driver wrongly mixes DMA and CPU SG list iteration and
+* needs revision. See
+* https://lore.kernel.org/lkml/20190104223531.ga1...@ziepe.ca/
+*/
+ return sg_page_iter_dma_address(
+ (struct sg_dma_page_iter *)>iter);
}
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index cdb79ae2d8dc72..c0a5ce1d13b0bc 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -846,7 +846,7 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb)
unsigned int pages = DIV_ROUND_UP(vb->planes[0].length, CIO2_PAGE_SIZE);
unsigned int lops = DIV_ROUND_UP(pages + 1, entries_per_page);
struct sg_table *sg;
- struct sg_page_iter sg_iter;
+ struct sg_dma_page_iter sg_iter;
int i, j;
if (lops <= 0 || lops > CIO2_MAX_LOPS) {
@@ -873,7 +873,7 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb)
b->offset = sg->sgl->offset;
i = j = 0;
- for_each_sg_page(sg->sgl, _iter, sg->nents, 0) {
+ for_each_sg_dma_page(sg->sgl, _iter, sg->nents, 0) {
if (!pages--)
break;
b->lop[i][j] = sg_page_iter_dma_address(_iter) >> PAGE_SHIFT;
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index b96f0d0b5b8f30..b4be960c7e5dba 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -339,12 +339,12 @@ int sg_alloc_table_chained(struct sg_table *table, int
nents,
/*
* sg page iterator
*
- * Iterates over sg entries page-by-page. On each successful iteration,
- * you can call sg_page_iter_page(@piter) and sg_page_iter_dma_address(@piter)
- * to get the current page and its dma address. @piter->sg will point to the
- * sg holding this page and @piter->sg_pgoffset to the page's page offset
- * within the sg. The iteration will stop either when a maximum number of sg
- * entries was reached or a terminating sg (sg_last(sg) == true) was reached.
+ * Iterates over sg entries page-by-page. On each successful iteration, you
+ * can call sg_page_iter_page(@piter) to get the current page and its dma
+ * address. @piter->sg will point to the sg holding this page and
+ * @piter->sg_pgoffset to the page's page offset within the sg. The iteration
+ * will stop either when a maximum number of sg entries was reached or a
+ * terminating sg (sg_last(sg) == true) was reached.
*/
struct sg_page_iter {
struct scatterlist *sg;/* sg holding the page */
@@ -356,7 +356,19 @@ struct sg_page_iter {
* next step */
};
+/*
+ * sg page iterator for DMA addresses
+ *
+ * This is the same as sg_page_iter however you can call
+ *
