* [PATCH v4 02/38] scatterlist: add generic wrappers for iterating over sgtable objects
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 12:18 ` Christoph Hellwig
2020-05-13 13:24 ` Robin Murphy
2020-05-12 9:00 ` [PATCH v4 03/38] iommu: add generic helper for mapping " Marek Szyprowski
` (37 subsequent siblings)
38 siblings, 2 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Daniel Vetter,
Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
struct sg_table is a common structure used for describing a memory
buffer. It consists of a scatterlist with memory pages and DMA addresses
(sgl entry), as well as the number of scatterlist entries: CPU pages
(orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling the scatterlist iterating functions with a wrong number
of the entries.
To avoid such issues, lets introduce a common wrappers operating directly
on the struct sg_table objects, which take care of the proper use of
the nents and orig_nents entries.
While touching this, lets clarify some ambiguities in the comments for
the existing for_each helpers.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
include/linux/scatterlist.h | 50 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 6eec50f..4f922af 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -151,6 +151,20 @@ static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
#define for_each_sg(sglist, sg, nr, __i) \
for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
+/*
+ * Loop over each sg element in the given sg_table object.
+ */
+#define for_each_sgtable_sg(sgt, sg, i) \
+ for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
+
+/*
+ * Loop over each sg element in the given *DMA mapped* sg_table object.
+ * Please use sg_dma_address(sg) and sg_dma_len(sg) to extract DMA addresses
+ * of the each element.
+ */
+#define for_each_sgtable_dma_sg(sgt, sg, i) \
+ for_each_sg(sgt->sgl, sg, sgt->nents, i)
+
/**
* sg_chain - Chain two sglists together
* @prv: First scatterlist
@@ -401,9 +415,10 @@ static inline struct page *sg_page_iter_page(struct sg_page_iter *piter)
* @sglist: sglist to iterate over
* @piter: page iterator to hold current page, sg, sg_pgoffset
* @nents: maximum number of sg entries to iterate over
- * @pgoffset: starting page offset
+ * @pgoffset: starting page offset (in pages)
*
* Callers may use sg_page_iter_page() to get each page pointer.
+ * In each loop it operates on PAGE_SIZE unit.
*/
#define for_each_sg_page(sglist, piter, nents, pgoffset) \
for (__sg_page_iter_start((piter), (sglist), (nents), (pgoffset)); \
@@ -412,18 +427,47 @@ static inline struct page *sg_page_iter_page(struct sg_page_iter *piter)
/**
* for_each_sg_dma_page - iterate over the pages of the given sg list
* @sglist: sglist to iterate over
- * @dma_iter: page iterator to hold current page
+ * @dma_iter: DMA page iterator to hold current page
* @dma_nents: maximum number of sg entries to iterate over, this is the value
* returned from dma_map_sg
- * @pgoffset: starting page offset
+ * @pgoffset: starting page offset (in pages)
*
* Callers may use sg_page_iter_dma_address() to get each page's DMA address.
+ * In each loop it operates on PAGE_SIZE unit.
*/
#define for_each_sg_dma_page(sglist, dma_iter, dma_nents, pgoffset) \
for (__sg_page_iter_start(&(dma_iter)->base, sglist, dma_nents, \
pgoffset); \
__sg_page_iter_dma_next(dma_iter);)
+/**
+ * for_each_sgtable_page - iterate over all pages in the sg_table object
+ * @sgt: sg_table object to iterate over
+ * @piter: page iterator to hold current page
+ * @pgoffset: starting page offset (in pages)
+ *
+ * Iterates over the all memory pages in the buffer described by
+ * a scatterlist stored in the given sg_table object.
+ * See also for_each_sg_page(). In each loop it operates on PAGE_SIZE unit.
+ */
+#define for_each_sgtable_page(sgt, piter, pgoffset) \
+ for_each_sg_page(sgt->sgl, piter, sgt->orig_nents, pgoffset)
+
+/**
+ * for_each_sgtable_dma_page - iterate over the DMA mapped sg_table object
+ * @sgt: sg_table object to iterate over
+ * @dma_iter: DMA page iterator to hold current page
+ * @pgoffset: starting page offset (in pages)
+ *
+ * Iterates over the all DMA mapped pages in the buffer described by
+ * a scatterlist stored in the given sg_table object.
+ * See also for_each_sg_dma_page(). In each loop it operates on PAGE_SIZE
+ * unit.
+ */
+#define for_each_sgtable_dma_page(sgt, dma_iter, pgoffset) \
+ for_each_sg_dma_page(sgt->sgl, dma_iter, sgt->nents, pgoffset)
+
+
/*
* Mapping sg iterator
*
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH v4 02/38] scatterlist: add generic wrappers for iterating over sgtable objects
2020-05-12 9:00 ` [PATCH v4 02/38] scatterlist: add generic wrappers for iterating over " Marek Szyprowski
@ 2020-05-12 12:18 ` Christoph Hellwig
2020-05-13 13:24 ` Robin Murphy
1 sibling, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2020-05-12 12:18 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Bartlomiej Zolnierkiewicz, David Airlie, linux-kernel, dri-devel,
linaro-mm-sig, iommu, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4 02/38] scatterlist: add generic wrappers for iterating over sgtable objects
2020-05-12 9:00 ` [PATCH v4 02/38] scatterlist: add generic wrappers for iterating over " Marek Szyprowski
2020-05-12 12:18 ` Christoph Hellwig
@ 2020-05-13 13:24 ` Robin Murphy
1 sibling, 0 replies; 53+ messages in thread
From: Robin Murphy @ 2020-05-13 13:24 UTC (permalink / raw)
To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: David Airlie, Daniel Vetter, Christoph Hellwig, linux-arm-kernel,
Bartlomiej Zolnierkiewicz
On 2020-05-12 10:00 am, Marek Szyprowski wrote:
> struct sg_table is a common structure used for describing a memory
> buffer. It consists of a scatterlist with memory pages and DMA addresses
> (sgl entry), as well as the number of scatterlist entries: CPU pages
> (orig_nents entry) and DMA mapped pages (nents entry).
>
> It turned out that it was a common mistake to misuse nents and orig_nents
> entries, calling the scatterlist iterating functions with a wrong number
> of the entries.
>
> To avoid such issues, lets introduce a common wrappers operating directly
> on the struct sg_table objects, which take care of the proper use of
> the nents and orig_nents entries.
>
> While touching this, lets clarify some ambiguities in the comments for
> the existing for_each helpers.
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
> vs. orig_nents misuse' thread:
> https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
> ---
> include/linux/scatterlist.h | 50 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index 6eec50f..4f922af 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -151,6 +151,20 @@ static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
> #define for_each_sg(sglist, sg, nr, __i) \
> for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
>
> +/*
> + * Loop over each sg element in the given sg_table object.
> + */
> +#define for_each_sgtable_sg(sgt, sg, i) \
> + for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
> +
> +/*
> + * Loop over each sg element in the given *DMA mapped* sg_table object.
> + * Please use sg_dma_address(sg) and sg_dma_len(sg) to extract DMA addresses
> + * of the each element.
> + */
> +#define for_each_sgtable_dma_sg(sgt, sg, i) \
> + for_each_sg(sgt->sgl, sg, sgt->nents, i)
> +
> /**
> * sg_chain - Chain two sglists together
> * @prv: First scatterlist
> @@ -401,9 +415,10 @@ static inline struct page *sg_page_iter_page(struct sg_page_iter *piter)
> * @sglist: sglist to iterate over
> * @piter: page iterator to hold current page, sg, sg_pgoffset
> * @nents: maximum number of sg entries to iterate over
> - * @pgoffset: starting page offset
> + * @pgoffset: starting page offset (in pages)
> *
> * Callers may use sg_page_iter_page() to get each page pointer.
> + * In each loop it operates on PAGE_SIZE unit.
> */
> #define for_each_sg_page(sglist, piter, nents, pgoffset) \
> for (__sg_page_iter_start((piter), (sglist), (nents), (pgoffset)); \
> @@ -412,18 +427,47 @@ static inline struct page *sg_page_iter_page(struct sg_page_iter *piter)
> /**
> * for_each_sg_dma_page - iterate over the pages of the given sg list
> * @sglist: sglist to iterate over
> - * @dma_iter: page iterator to hold current page
> + * @dma_iter: DMA page iterator to hold current page
> * @dma_nents: maximum number of sg entries to iterate over, this is the value
> * returned from dma_map_sg
> - * @pgoffset: starting page offset
> + * @pgoffset: starting page offset (in pages)
> *
> * Callers may use sg_page_iter_dma_address() to get each page's DMA address.
> + * In each loop it operates on PAGE_SIZE unit.
> */
> #define for_each_sg_dma_page(sglist, dma_iter, dma_nents, pgoffset) \
> for (__sg_page_iter_start(&(dma_iter)->base, sglist, dma_nents, \
> pgoffset); \
> __sg_page_iter_dma_next(dma_iter);)
>
> +/**
> + * for_each_sgtable_page - iterate over all pages in the sg_table object
> + * @sgt: sg_table object to iterate over
> + * @piter: page iterator to hold current page
> + * @pgoffset: starting page offset (in pages)
> + *
> + * Iterates over the all memory pages in the buffer described by
> + * a scatterlist stored in the given sg_table object.
> + * See also for_each_sg_page(). In each loop it operates on PAGE_SIZE unit.
> + */
> +#define for_each_sgtable_page(sgt, piter, pgoffset) \
> + for_each_sg_page(sgt->sgl, piter, sgt->orig_nents, pgoffset)
> +
> +/**
> + * for_each_sgtable_dma_page - iterate over the DMA mapped sg_table object
> + * @sgt: sg_table object to iterate over
> + * @dma_iter: DMA page iterator to hold current page
> + * @pgoffset: starting page offset (in pages)
> + *
> + * Iterates over the all DMA mapped pages in the buffer described by
> + * a scatterlist stored in the given sg_table object.
> + * See also for_each_sg_dma_page(). In each loop it operates on PAGE_SIZE
> + * unit.
> + */
> +#define for_each_sgtable_dma_page(sgt, dma_iter, pgoffset) \
> + for_each_sg_dma_page(sgt->sgl, dma_iter, sgt->nents, pgoffset)
> +
> +
> /*
> * Mapping sg iterator
> *
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v4 03/38] iommu: add generic helper for mapping sgtable objects
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 02/38] scatterlist: add generic wrappers for iterating over " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 12:18 ` Christoph Hellwig
` (2 more replies)
2020-05-12 9:00 ` [PATCH v4 04/38] drm: prime: add common helper to check scatterlist contiguity Marek Szyprowski
` (36 subsequent siblings)
38 siblings, 3 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Joerg Roedel,
Daniel Vetter, Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
struct sg_table is a common structure used for describing a memory
buffer. It consists of a scatterlist with memory pages and DMA addresses
(sgl entry), as well as the number of scatterlist entries: CPU pages
(orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling mapping functions with a wrong number of entries.
To avoid such issues, lets introduce a common wrapper operating directly
on the struct sg_table objects, which take care of the proper use of
the nents and orig_nents entries.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
include/linux/iommu.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7cfd2dd..ba662ba 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -478,6 +478,22 @@ extern size_t iommu_map_sg_atomic(struct iommu_domain *domain,
extern void iommu_set_fault_handler(struct iommu_domain *domain,
iommu_fault_handler_t handler, void *token);
+/**
+ * iommu_map_sgtable - Map the given buffer to the IOMMU domain
+ * @domain: The IOMMU domain to perfor
+ * @iova: The start addrees to map the buffer
+ * @sgt: The sg_table object describing the buffer
+ * @prot: IOMMU protection bits
+ *
+ * Create a mapping at @iova for the buffer described by a scatterlist
+ * stored in the given sg_table object in the provided IOMMU domain.
+ */
+static inline size_t iommu_map_sgtable(struct iommu_domain *domain,
+ unsigned long iova, struct sg_table *sgt, int prot)
+{
+ return iommu_map_sg(domain, iova, sgt->sgl, sgt->orig_nents, prot);
+}
+
extern void iommu_get_resv_regions(struct device *dev, struct list_head *list);
extern void iommu_put_resv_regions(struct device *dev, struct list_head *list);
extern void generic_iommu_put_resv_regions(struct device *dev,
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH v4 03/38] iommu: add generic helper for mapping sgtable objects
2020-05-12 9:00 ` [PATCH v4 03/38] iommu: add generic helper for mapping " Marek Szyprowski
@ 2020-05-12 12:18 ` Christoph Hellwig
2020-05-13 9:03 ` Joerg Roedel
2020-05-13 13:27 ` Robin Murphy
2 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2020-05-12 12:18 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Joerg Roedel,
linux-kernel, dri-devel, linaro-mm-sig, iommu, Daniel Vetter,
Robin Murphy, Christoph Hellwig, linux-arm-kernel
On Tue, May 12, 2020 at 11:00:23AM +0200, Marek Szyprowski wrote:
> struct sg_table is a common structure used for describing a memory
> buffer. It consists of a scatterlist with memory pages and DMA addresses
> (sgl entry), as well as the number of scatterlist entries: CPU pages
> (orig_nents entry) and DMA mapped pages (nents entry).
>
> It turned out that it was a common mistake to misuse nents and orig_nents
> entries, calling mapping functions with a wrong number of entries.
>
> To avoid such issues, lets introduce a common wrapper operating directly
> on the struct sg_table objects, which take care of the proper use of
> the nents and orig_nents entries.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4 03/38] iommu: add generic helper for mapping sgtable objects
2020-05-12 9:00 ` [PATCH v4 03/38] iommu: add generic helper for mapping " Marek Szyprowski
2020-05-12 12:18 ` Christoph Hellwig
@ 2020-05-13 9:03 ` Joerg Roedel
2020-05-13 13:27 ` Robin Murphy
2 siblings, 0 replies; 53+ messages in thread
From: Joerg Roedel @ 2020-05-13 9:03 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Bartlomiej Zolnierkiewicz, David Airlie, linux-kernel, dri-devel,
linaro-mm-sig, iommu, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel
Hi Marek,
On Tue, May 12, 2020 at 11:00:23AM +0200, Marek Szyprowski wrote:
> ---
> include/linux/iommu.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
Some nits below, with those fixed:
Acked-by: Joerg Roedel <jroedel@suse.de>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 7cfd2dd..ba662ba 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -478,6 +478,22 @@ extern size_t iommu_map_sg_atomic(struct iommu_domain *domain,
> extern void iommu_set_fault_handler(struct iommu_domain *domain,
> iommu_fault_handler_t handler, void *token);
>
> +/**
> + * iommu_map_sgtable - Map the given buffer to the IOMMU domain
> + * @domain: The IOMMU domain to perfor
^^^^^^ Truncated comment?
> + * @iova: The start addrees to map the buffer
^^^^^^^ Typo
> + * @sgt: The sg_table object describing the buffer
> + * @prot: IOMMU protection bits
> + *
> + * Create a mapping at @iova for the buffer described by a scatterlist
> + * stored in the given sg_table object in the provided IOMMU domain.
> + */
> +static inline size_t iommu_map_sgtable(struct iommu_domain *domain,
> + unsigned long iova, struct sg_table *sgt, int prot)
> +{
> + return iommu_map_sg(domain, iova, sgt->sgl, sgt->orig_nents, prot);
> +}
> +
> extern void iommu_get_resv_regions(struct device *dev, struct list_head *list);
> extern void iommu_put_resv_regions(struct device *dev, struct list_head *list);
> extern void generic_iommu_put_resv_regions(struct device *dev,
> --
> 1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH v4 03/38] iommu: add generic helper for mapping sgtable objects
2020-05-12 9:00 ` [PATCH v4 03/38] iommu: add generic helper for mapping " Marek Szyprowski
2020-05-12 12:18 ` Christoph Hellwig
2020-05-13 9:03 ` Joerg Roedel
@ 2020-05-13 13:27 ` Robin Murphy
2 siblings, 0 replies; 53+ messages in thread
From: Robin Murphy @ 2020-05-13 13:27 UTC (permalink / raw)
To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Joerg Roedel,
Daniel Vetter, Christoph Hellwig, linux-arm-kernel
On 2020-05-12 10:00 am, Marek Szyprowski wrote:
> struct sg_table is a common structure used for describing a memory
> buffer. It consists of a scatterlist with memory pages and DMA addresses
> (sgl entry), as well as the number of scatterlist entries: CPU pages
> (orig_nents entry) and DMA mapped pages (nents entry).
>
> It turned out that it was a common mistake to misuse nents and orig_nents
> entries, calling mapping functions with a wrong number of entries.
>
> To avoid such issues, lets introduce a common wrapper operating directly
> on the struct sg_table objects, which take care of the proper use of
> the nents and orig_nents entries.
Modulo Joerg's comments,
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
> vs. orig_nents misuse' thread:
> https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
> ---
> include/linux/iommu.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 7cfd2dd..ba662ba 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -478,6 +478,22 @@ extern size_t iommu_map_sg_atomic(struct iommu_domain *domain,
> extern void iommu_set_fault_handler(struct iommu_domain *domain,
> iommu_fault_handler_t handler, void *token);
>
> +/**
> + * iommu_map_sgtable - Map the given buffer to the IOMMU domain
> + * @domain: The IOMMU domain to perfor
> + * @iova: The start addrees to map the buffer
> + * @sgt: The sg_table object describing the buffer
> + * @prot: IOMMU protection bits
> + *
> + * Create a mapping at @iova for the buffer described by a scatterlist
> + * stored in the given sg_table object in the provided IOMMU domain.
> + */
> +static inline size_t iommu_map_sgtable(struct iommu_domain *domain,
> + unsigned long iova, struct sg_table *sgt, int prot)
> +{
> + return iommu_map_sg(domain, iova, sgt->sgl, sgt->orig_nents, prot);
> +}
> +
> extern void iommu_get_resv_regions(struct device *dev, struct list_head *list);
> extern void iommu_put_resv_regions(struct device *dev, struct list_head *list);
> extern void generic_iommu_put_resv_regions(struct device *dev,
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v4 04/38] drm: prime: add common helper to check scatterlist contiguity
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 02/38] scatterlist: add generic wrappers for iterating over " Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 03/38] iommu: add generic helper for mapping " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 05/38] drm: prime: use sgtable iterators in drm_prime_sg_to_page_addr_arrays() Marek Szyprowski
` (35 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Thomas Zimmermann, Bartlomiej Zolnierkiewicz, David Airlie,
Maarten Lankhorst, Maxime Ripard, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
It is a common operation done by DRM drivers to check the contiguity
of the DMA-mapped buffer described by a scatterlist in the
sg_table object. Let's add a common helper for this operation.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/drm_gem_cma_helper.c | 23 +++--------------------
drivers/gpu/drm/drm_prime.c | 26 ++++++++++++++++++++++++++
include/drm/drm_prime.h | 2 ++
3 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 12e98fb..9f2d13e 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -471,26 +471,9 @@ struct drm_gem_object *
{
struct drm_gem_cma_object *cma_obj;
- if (sgt->nents != 1) {
- /* check if the entries in the sg_table are contiguous */
- dma_addr_t next_addr = sg_dma_address(sgt->sgl);
- struct scatterlist *s;
- unsigned int i;
-
- for_each_sg(sgt->sgl, s, sgt->nents, i) {
- /*
- * sg_dma_address(s) is only valid for entries
- * that have sg_dma_len(s) != 0
- */
- if (!sg_dma_len(s))
- continue;
-
- if (sg_dma_address(s) != next_addr)
- return ERR_PTR(-EINVAL);
-
- next_addr = sg_dma_address(s) + sg_dma_len(s);
- }
- }
+ /* check if the entries in the sg_table are contiguous */
+ if (drm_prime_get_contiguous_size(sgt) < attach->dmabuf->size)
+ return ERR_PTR(-EINVAL);
/* Create a CMA GEM buffer. */
cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 282774e..1d2e5fe 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -826,6 +826,32 @@ struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_page
EXPORT_SYMBOL(drm_prime_pages_to_sg);
/**
+ * drm_prime_get_contiguous_size - returns the contiguous size of the buffer
+ * @sgt: sg_table describing the buffer to check
+ *
+ * This helper calculates the contiguous size in the DMA address space
+ * of the the buffer described by the provided sg_table.
+ *
+ * This is useful for implementing
+ * &drm_gem_object_funcs.gem_prime_import_sg_table.
+ */
+unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt)
+{
+ dma_addr_t expected = sg_dma_address(sgt->sgl);
+ struct sg_dma_page_iter dma_iter;
+ unsigned long size = 0;
+
+ for_each_sgtable_dma_page(sgt, &dma_iter, 0) {
+ if (sg_page_iter_dma_address(&dma_iter) != expected)
+ break;
+ expected += PAGE_SIZE;
+ size += PAGE_SIZE;
+ }
+ return size;
+}
+EXPORT_SYMBOL(drm_prime_get_contiguous_size);
+
+/**
* drm_gem_prime_export - helper library implementation of the export callback
* @obj: GEM object to export
* @flags: flags like DRM_CLOEXEC and DRM_RDWR
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index 9af7422..47ef116 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -92,6 +92,8 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
int flags);
+unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt);
+
/* helper functions for importing */
struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
struct dma_buf *dma_buf,
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 05/38] drm: prime: use sgtable iterators in drm_prime_sg_to_page_addr_arrays()
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (2 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 04/38] drm: prime: add common helper to check scatterlist contiguity Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 06/38] drm: core: fix common struct sg_table related issues Marek Szyprowski
` (34 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Thomas Zimmermann, Bartlomiej Zolnierkiewicz, David Airlie,
Maarten Lankhorst, Maxime Ripard, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
Replace the current hand-crafted code for extracting pages and DMA
addresses from the given scatterlist by the much more robust
code based on the generic scatterlist iterators and recently
introduced sg_table-based wrappers. The resulting code is simple and
easy to understand, so the comment describing the old code is no
longer needed.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/drm_prime.c | 47 ++++++++++++++-------------------------------
1 file changed, 14 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 1d2e5fe..dfdf4d4 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -985,45 +985,26 @@ struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,
dma_addr_t *addrs, int max_entries)
{
- unsigned count;
- struct scatterlist *sg;
- struct page *page;
- u32 page_len, page_index;
- dma_addr_t addr;
- u32 dma_len, dma_index;
+ struct sg_dma_page_iter dma_iter;
+ struct sg_page_iter page_iter;
+ struct page **p = pages;
+ dma_addr_t *a = addrs;
- /*
- * Scatterlist elements contains both pages and DMA addresses, but
- * one shoud not assume 1:1 relation between them. The sg->length is
- * the size of the physical memory chunk described by the sg->page,
- * while sg_dma_len(sg) is the size of the DMA (IO virtual) chunk
- * described by the sg_dma_address(sg).
- */
- page_index = 0;
- dma_index = 0;
- for_each_sg(sgt->sgl, sg, sgt->nents, count) {
- page_len = sg->length;
- page = sg_page(sg);
- dma_len = sg_dma_len(sg);
- addr = sg_dma_address(sg);
-
- while (pages && page_len > 0) {
- if (WARN_ON(page_index >= max_entries))
+ if (pages) {
+ for_each_sgtable_page(sgt, &page_iter, 0) {
+ if (p - pages >= max_entries)
return -1;
- pages[page_index] = page;
- page++;
- page_len -= PAGE_SIZE;
- page_index++;
+ *p++ = sg_page_iter_page(&page_iter);
}
- while (addrs && dma_len > 0) {
- if (WARN_ON(dma_index >= max_entries))
+ }
+ if (addrs) {
+ for_each_sgtable_dma_page(sgt, &dma_iter, 0) {
+ if (a - addrs >= max_entries)
return -1;
- addrs[dma_index] = addr;
- addr += PAGE_SIZE;
- dma_len -= PAGE_SIZE;
- dma_index++;
+ *a++ = sg_page_iter_dma_address(&dma_iter);
}
}
+
return 0;
}
EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 06/38] drm: core: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (3 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 05/38] drm: prime: use sgtable iterators in drm_prime_sg_to_page_addr_arrays() Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 07/38] drm: amdgpu: " Marek Szyprowski
` (33 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Thomas Zimmermann, Bartlomiej Zolnierkiewicz, David Airlie,
Maarten Lankhorst, Maxime Ripard, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/drm_cache.c | 2 +-
drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++++++++-----
drivers/gpu/drm/drm_prime.c | 11 ++++++-----
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 03e01b0..0fe3c49 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -127,7 +127,7 @@ static void drm_cache_flush_clflush(struct page *pages[],
struct sg_page_iter sg_iter;
mb(); /*CLFLUSH is ordered only by using memory barriers*/
- for_each_sg_page(st->sgl, &sg_iter, st->nents, 0)
+ for_each_sgtable_page(st, &sg_iter, 0)
drm_clflush_page(sg_page_iter_page(&sg_iter));
mb(); /*Make sure that all cache line entry is flushed*/
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index df31e57..00a43e8 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -117,8 +117,8 @@ void drm_gem_shmem_free_object(struct drm_gem_object *obj)
kvfree(shmem->pages);
} else {
if (shmem->sgt) {
- dma_unmap_sg(obj->dev->dev, shmem->sgt->sgl,
- shmem->sgt->nents, DMA_BIDIRECTIONAL);
+ dma_unmap_sgtable(obj->dev->dev, shmem->sgt,
+ DMA_BIDIRECTIONAL, 0);
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
}
@@ -395,8 +395,7 @@ void drm_gem_shmem_purge_locked(struct drm_gem_object *obj)
WARN_ON(!drm_gem_shmem_is_purgeable(shmem));
- dma_unmap_sg(obj->dev->dev, shmem->sgt->sgl,
- shmem->sgt->nents, DMA_BIDIRECTIONAL);
+ dma_unmap_sgtable(obj->dev->dev, shmem->sgt, DMA_BIDIRECTIONAL, 0);
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
shmem->sgt = NULL;
@@ -623,12 +622,17 @@ struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_object *obj)
goto err_put_pages;
}
/* Map the pages for use by the h/w. */
- dma_map_sg(obj->dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
+ ret = dma_map_sgtable(obj->dev->dev, sgt, DMA_BIDIRECTIONAL, 0);
+ if (ret)
+ goto err_free_sgt;
shmem->sgt = sgt;
return sgt;
+err_free_sgt:
+ sg_free_table(sgt);
+ kfree(sgt);
err_put_pages:
drm_gem_shmem_put_pages(shmem);
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index dfdf4d4..5ed22dd 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -617,6 +617,7 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
{
struct drm_gem_object *obj = attach->dmabuf->priv;
struct sg_table *sgt;
+ int ret;
if (WARN_ON(dir == DMA_NONE))
return ERR_PTR(-EINVAL);
@@ -626,11 +627,12 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
else
sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
- if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC)) {
+ ret = dma_map_sgtable(attach->dev, sgt, dir,
+ DMA_ATTR_SKIP_CPU_SYNC);
+ if (ret) {
sg_free_table(sgt);
kfree(sgt);
- sgt = ERR_PTR(-ENOMEM);
+ sgt = ERR_PTR(ret);
}
return sgt;
@@ -652,8 +654,7 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
if (!sgt)
return;
- dma_unmap_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC);
+ dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sgt);
kfree(sgt);
}
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 07/38] drm: amdgpu: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (4 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 06/38] drm: core: fix common struct sg_table related issues Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 08/38] drm: armada: " Marek Szyprowski
` (32 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, amd-gfx,
Christian König, Daniel Vetter, Alex Deucher, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 6 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 9 +++------
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 8 ++++----
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 43d8ed7..519ce44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -307,8 +307,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach,
if (IS_ERR(sgt))
return sgt;
- if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC))
+ if (dma_map_sgtable(attach->dev, sgt, dir,
+ DMA_ATTR_SKIP_CPU_SYNC))
goto error_free;
break;
@@ -349,7 +349,7 @@ static void amdgpu_dma_buf_unmap(struct dma_buf_attachment *attach,
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
if (sgt->sgl->page_link) {
- dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+ dma_unmap_sgtable(attach->dev, sgt, dir, 0);
sg_free_table(sgt);
kfree(sgt);
} else {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 9cbecd5..57a5d56 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1024,7 +1024,6 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
struct amdgpu_ttm_tt *gtt = (void *)ttm;
- unsigned nents;
int r;
int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
@@ -1039,9 +1038,8 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
goto release_sg;
/* Map SG to device */
- r = -ENOMEM;
- nents = dma_map_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
- if (nents == 0)
+ r = dma_map_sgtable(adev->dev, ttm->sg, direction, 0);
+ if (r)
goto release_sg;
/* convert SG to linear array of pages and dma addresses */
@@ -1072,8 +1070,7 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt *ttm)
return;
/* unmap the pages mapped to the device */
- dma_unmap_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
-
+ dma_unmap_sgtable(adev->dev, ttm->sg, direction, 0);
sg_free_table(ttm->sg);
#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index d399e58..75495a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -477,11 +477,11 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
if (r)
goto error_free;
- for_each_sg((*sgt)->sgl, sg, num_entries, i)
+ for_each_sgtable_sg(*sgt, sg, i)
sg->length = 0;
node = mem->mm_node;
- for_each_sg((*sgt)->sgl, sg, num_entries, i) {
+ for_each_sgtable_sg(*sgt, sg, i) {
phys_addr_t phys = (node->start << PAGE_SHIFT) +
adev->gmc.aper_base;
size_t size = node->size << PAGE_SHIFT;
@@ -501,7 +501,7 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
return 0;
error_unmap:
- for_each_sg((*sgt)->sgl, sg, num_entries, i) {
+ for_each_sgtable_sg(*sgt, sg, i) {
if (!sg->length)
continue;
@@ -532,7 +532,7 @@ void amdgpu_vram_mgr_free_sgt(struct amdgpu_device *adev,
struct scatterlist *sg;
int i;
- for_each_sg(sgt->sgl, sg, sgt->nents, i)
+ for_each_sgtable_sg(sgt, sg, i)
dma_unmap_resource(dev, sg->dma_address,
sg->length, dir,
DMA_ATTR_SKIP_CPU_SYNC);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 08/38] drm: armada: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (5 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 07/38] drm: amdgpu: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 09/38] drm: etnaviv: " Marek Szyprowski
` (31 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Russell King,
Daniel Vetter, Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/armada/armada_gem.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
index 976685f..0d61da5 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -395,7 +395,7 @@ int armada_gem_pwrite_ioctl(struct drm_device *dev, void *data,
mapping = dobj->obj.filp->f_mapping;
- for_each_sg(sgt->sgl, sg, count, i) {
+ for_each_sgtable_sg(sgt, sg, i) {
struct page *page;
page = shmem_read_mapping_page(mapping, i);
@@ -407,8 +407,8 @@ int armada_gem_pwrite_ioctl(struct drm_device *dev, void *data,
sg_set_page(sg, page, PAGE_SIZE, 0);
}
- if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0) {
- num = sgt->nents;
+ if (dma_map_sgtable(attach->dev, sgt, dir, 0)) {
+ num = count;
goto release;
}
} else if (dobj->page) {
@@ -418,7 +418,7 @@ int armada_gem_pwrite_ioctl(struct drm_device *dev, void *data,
sg_set_page(sgt->sgl, dobj->page, dobj->obj.size, 0);
- if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0)
+ if (dma_map_sgtable(attach->dev, sgt, dir, 0))
goto free_table;
} else if (dobj->linear) {
/* Single contiguous physical region - no struct page */
@@ -449,11 +449,11 @@ static void armada_gem_prime_unmap_dma_buf(struct dma_buf_attachment *attach,
int i;
if (!dobj->linear)
- dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+ dma_unmap_sgtable(attach->dev, sgt, dir, 0);
if (dobj->obj.filp) {
struct scatterlist *sg;
- for_each_sg(sgt->sgl, sg, sgt->nents, i)
+ for_each_sgtable_sg(sgt, sg, i)
put_page(sg_page(sg));
}
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 09/38] drm: etnaviv: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (6 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 08/38] drm: armada: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 10/38] drm: exynos: use common helper for a scatterlist contiguity check Marek Szyprowski
` (30 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, etnaviv, Daniel Vetter,
Lucas Stach, Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 12 +++++-------
drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 13 +++----------
2 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index dc9ef30..cc50c7b 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -27,7 +27,7 @@ static void etnaviv_gem_scatter_map(struct etnaviv_gem_object *etnaviv_obj)
* because display controller, GPU, etc. are not coherent.
*/
if (etnaviv_obj->flags & ETNA_BO_CACHE_MASK)
- dma_map_sg(dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
+ dma_map_sgtable(dev->dev, sgt, DMA_BIDIRECTIONAL, 0);
}
static void etnaviv_gem_scatterlist_unmap(struct etnaviv_gem_object *etnaviv_obj)
@@ -51,7 +51,7 @@ static void etnaviv_gem_scatterlist_unmap(struct etnaviv_gem_object *etnaviv_obj
* discard those writes.
*/
if (etnaviv_obj->flags & ETNA_BO_CACHE_MASK)
- dma_unmap_sg(dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
+ dma_unmap_sgtable(dev->dev, sgt, DMA_BIDIRECTIONAL, 0);
}
/* called with etnaviv_obj->lock held */
@@ -404,9 +404,8 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
}
if (etnaviv_obj->flags & ETNA_BO_CACHED) {
- dma_sync_sg_for_cpu(dev->dev, etnaviv_obj->sgt->sgl,
- etnaviv_obj->sgt->nents,
- etnaviv_op_to_dma_dir(op));
+ dma_sync_sgtable_for_cpu(dev->dev, etnaviv_obj->sgt,
+ etnaviv_op_to_dma_dir(op));
etnaviv_obj->last_cpu_prep_op = op;
}
@@ -421,8 +420,7 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj)
if (etnaviv_obj->flags & ETNA_BO_CACHED) {
/* fini without a prep is almost certainly a userspace error */
WARN_ON(etnaviv_obj->last_cpu_prep_op == 0);
- dma_sync_sg_for_device(dev->dev, etnaviv_obj->sgt->sgl,
- etnaviv_obj->sgt->nents,
+ dma_sync_sgtable_for_device(dev->dev, etnaviv_obj->sgt,
etnaviv_op_to_dma_dir(etnaviv_obj->last_cpu_prep_op));
etnaviv_obj->last_cpu_prep_op = 0;
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
index 3607d34..13b1005 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
@@ -79,7 +79,7 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context *context, u32 iova,
if (!context || !sgt)
return -EINVAL;
- for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+ for_each_sgtable_dma_sg(sgt, sg, i) {
u32 pa = sg_dma_address(sg) - sg->offset;
size_t bytes = sg_dma_len(sg) + sg->offset;
@@ -95,14 +95,7 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context *context, u32 iova,
return 0;
fail:
- da = iova;
-
- for_each_sg(sgt->sgl, sg, i, j) {
- size_t bytes = sg_dma_len(sg) + sg->offset;
-
- etnaviv_context_unmap(context, da, bytes);
- da += bytes;
- }
+ etnaviv_context_unmap(context, iova, da - iova);
return ret;
}
@@ -113,7 +106,7 @@ static void etnaviv_iommu_unmap(struct etnaviv_iommu_context *context, u32 iova,
unsigned int da = iova;
int i;
- for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+ for_each_sgtable_dma_sg(sgt, sg, i) {
size_t bytes = sg_dma_len(sg) + sg->offset;
etnaviv_context_unmap(context, da, bytes);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 10/38] drm: exynos: use common helper for a scatterlist contiguity check
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (7 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 09/38] drm: etnaviv: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 11/38] drm: exynos: fix common struct sg_table related issues Marek Szyprowski
` (29 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, David Airlie,
Seung-Woo Kim, Inki Dae, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
Use common helper for checking the contiguity of the imported dma-buf.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/exynos/exynos_drm_gem.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 0df57ee..a49a8ea 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -431,27 +431,10 @@ struct drm_gem_object *
{
struct exynos_drm_gem *exynos_gem;
- if (sgt->nents < 1)
+ /* check if the entries in the sg_table are contiguous */
+ if (drm_prime_get_contiguous_size(sgt) < attach->dmabuf->size) {
+ DRM_ERROR("buffer chunks must be mapped contiguously");
return ERR_PTR(-EINVAL);
-
- /*
- * Check if the provided buffer has been mapped as contiguous
- * into DMA address space.
- */
- if (sgt->nents > 1) {
- dma_addr_t next_addr = sg_dma_address(sgt->sgl);
- struct scatterlist *s;
- unsigned int i;
-
- for_each_sg(sgt->sgl, s, sgt->nents, i) {
- if (!sg_dma_len(s))
- break;
- if (sg_dma_address(s) != next_addr) {
- DRM_ERROR("buffer chunks must be mapped contiguously");
- return ERR_PTR(-EINVAL);
- }
- next_addr = sg_dma_address(s) + sg_dma_len(s);
- }
}
exynos_gem = exynos_drm_gem_init(dev, attach->dmabuf->size);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 11/38] drm: exynos: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (8 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 10/38] drm: exynos: use common helper for a scatterlist contiguity check Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 12/38] drm: i915: " Marek Szyprowski
` (28 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, David Airlie,
Seung-Woo Kim, Inki Dae, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index fcee33a..7014a8c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -395,8 +395,8 @@ static void g2d_userptr_put_dma_addr(struct g2d_data *g2d,
return;
out:
- dma_unmap_sg(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt->sgl,
- g2d_userptr->sgt->nents, DMA_BIDIRECTIONAL);
+ dma_unmap_sgtable(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt,
+ DMA_BIDIRECTIONAL, 0);
pages = frame_vector_pages(g2d_userptr->vec);
if (!IS_ERR(pages)) {
@@ -511,10 +511,10 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct g2d_data *g2d,
g2d_userptr->sgt = sgt;
- if (!dma_map_sg(to_dma_dev(g2d->drm_dev), sgt->sgl, sgt->nents,
- DMA_BIDIRECTIONAL)) {
+ ret = dma_map_sgtable(to_dma_dev(g2d->drm_dev), sgt,
+ DMA_BIDIRECTIONAL, 0);
+ if (ret) {
DRM_DEV_ERROR(g2d->dev, "failed to map sgt with dma region.\n");
- ret = -ENOMEM;
goto err_sg_free_table;
}
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 12/38] drm: i915: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (9 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 11/38] drm: exynos: fix common struct sg_table related issues Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 13/38] drm: lima: " Marek Szyprowski
` (27 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, intel-gfx,
Joonas Lahtinen, Jani Nikula, Daniel Vetter, Rodrigo Vivi,
Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
This driver creatively uses sg_table->orig_nents to store the size of the
allocated scatterlist and ignores the number of the entries returned by
dma_map_sg function. The sg_table->orig_nents is (mis)used to properly
free the (over)allocated scatterlist.
This patch only introduces the common DMA-mapping wrappers operating
directly on the struct sg_table objects to the dmabuf related functions,
so the other drivers, which might share buffers with i915 could rely on
the properly set nents and orig_nents values.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 11 +++--------
drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c | 7 +++----
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index 7db5a79..6c67810 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -48,12 +48,9 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
src = sg_next(src);
}
- if (!dma_map_sg_attrs(attachment->dev,
- st->sgl, st->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC)) {
- ret = -ENOMEM;
+ ret = dma_map_sgtable(attachment->dev, st, dir, DMA_ATTR_SKIP_CPU_SYNC);
+ if (ret)
goto err_free_sg;
- }
return st;
@@ -73,9 +70,7 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
{
struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);
- dma_unmap_sg_attrs(attachment->dev,
- sg->sgl, sg->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC);
+ dma_unmap_sgtable(attachment->dev, sg, dir, DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sg);
kfree(sg);
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
index debaf7b..be30b27 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
@@ -28,10 +28,9 @@ static struct sg_table *mock_map_dma_buf(struct dma_buf_attachment *attachment,
sg = sg_next(sg);
}
- if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
- err = -ENOMEM;
+ err = dma_map_sgtable(attachment->dev, st, dir, 0);
+ if (err)
goto err_st;
- }
return st;
@@ -46,7 +45,7 @@ static void mock_unmap_dma_buf(struct dma_buf_attachment *attachment,
struct sg_table *st,
enum dma_data_direction dir)
{
- dma_unmap_sg(attachment->dev, st->sgl, st->nents, dir);
+ dma_unmap_sgtable(attachment->dev, st, dir, 0);
sg_free_table(st);
kfree(st);
}
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 13/38] drm: lima: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (10 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 12/38] drm: i915: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 14/38] drm: mediatek: use common helper for a scatterlist contiguity check Marek Szyprowski
` (26 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: lima, Bartlomiej Zolnierkiewicz, David Airlie, Qiang Yu,
Daniel Vetter, Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/lima/lima_gem.c | 11 ++++++++---
drivers/gpu/drm/lima/lima_vm.c | 5 ++---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index 5404e0d..cda43f6 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -69,8 +69,7 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
return ret;
if (bo->base.sgt) {
- dma_unmap_sg(dev, bo->base.sgt->sgl,
- bo->base.sgt->nents, DMA_BIDIRECTIONAL);
+ dma_unmap_sgtable(dev, bo->base.sgt, DMA_BIDIRECTIONAL, 0);
sg_free_table(bo->base.sgt);
} else {
bo->base.sgt = kmalloc(sizeof(*bo->base.sgt), GFP_KERNEL);
@@ -80,7 +79,13 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
}
}
- dma_map_sg(dev, sgt.sgl, sgt.nents, DMA_BIDIRECTIONAL);
+ ret = dma_map_sgtable(dev, &sgt, DMA_BIDIRECTIONAL, 0);
+ if (ret) {
+ sg_free_table(&sgt);
+ kfree(bo->base.sgt);
+ bo->base.sgt = NULL;
+ return ret;
+ }
*bo->base.sgt = sgt;
diff --git a/drivers/gpu/drm/lima/lima_vm.c b/drivers/gpu/drm/lima/lima_vm.c
index 5b92fb8..2b2739a 100644
--- a/drivers/gpu/drm/lima/lima_vm.c
+++ b/drivers/gpu/drm/lima/lima_vm.c
@@ -124,7 +124,7 @@ int lima_vm_bo_add(struct lima_vm *vm, struct lima_bo *bo, bool create)
if (err)
goto err_out1;
- for_each_sg_dma_page(bo->base.sgt->sgl, &sg_iter, bo->base.sgt->nents, 0) {
+ for_each_sgtable_dma_page(bo->base.sgt, &sg_iter, 0) {
err = lima_vm_map_page(vm, sg_page_iter_dma_address(&sg_iter),
bo_va->node.start + offset);
if (err)
@@ -298,8 +298,7 @@ int lima_vm_map_bo(struct lima_vm *vm, struct lima_bo *bo, int pageoff)
mutex_lock(&vm->lock);
base = bo_va->node.start + (pageoff << PAGE_SHIFT);
- for_each_sg_dma_page(bo->base.sgt->sgl, &sg_iter,
- bo->base.sgt->nents, pageoff) {
+ for_each_sgtable_dma_page(bo->base.sgt, &sg_iter, pageoff) {
err = lima_vm_map_page(vm, sg_page_iter_dma_address(&sg_iter),
base + offset);
if (err)
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 14/38] drm: mediatek: use common helper for a scatterlist contiguity check
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (11 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 13/38] drm: lima: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 15/38] drm: mediatek: use common helper for extracting pages array Marek Szyprowski
` (25 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Chun-Kuang Hu, Philipp Zabel, Bartlomiej Zolnierkiewicz,
David Airlie, linux-mediatek, Daniel Vetter, Matthias Brugger,
Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
Use common helper for checking the contiguity of the imported dma-buf and
do this check before allocating resources, so the error path is simpler.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/mediatek/mtk_drm_gem.c | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
index b04a3c2..6c34c06 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
@@ -212,34 +212,21 @@ struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment *attach, struct sg_table *sg)
{
struct mtk_drm_gem_obj *mtk_gem;
- int ret;
- struct scatterlist *s;
- unsigned int i;
- dma_addr_t expected;
- mtk_gem = mtk_drm_gem_init(dev, attach->dmabuf->size);
+ /* check if the entries in the sg_table are contiguous */
+ if (drm_prime_get_contiguous_size(sg) < attach->dmabuf->size) {
+ DRM_ERROR("sg_table is not contiguous");
+ return ERR_PTR(-EINVAL);
+ }
+ mtk_gem = mtk_drm_gem_init(dev, attach->dmabuf->size);
if (IS_ERR(mtk_gem))
return ERR_CAST(mtk_gem);
- expected = sg_dma_address(sg->sgl);
- for_each_sg(sg->sgl, s, sg->nents, i) {
- if (sg_dma_address(s) != expected) {
- DRM_ERROR("sg_table is not contiguous");
- ret = -EINVAL;
- goto err_gem_free;
- }
- expected = sg_dma_address(s) + sg_dma_len(s);
- }
-
mtk_gem->dma_addr = sg_dma_address(sg->sgl);
mtk_gem->sg = sg;
return &mtk_gem->base;
-
-err_gem_free:
- kfree(mtk_gem);
- return ERR_PTR(ret);
}
void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 15/38] drm: mediatek: use common helper for extracting pages array
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (12 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 14/38] drm: mediatek: use common helper for a scatterlist contiguity check Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 16/38] drm: msm: fix common struct sg_table related issues Marek Szyprowski
` (24 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Chun-Kuang Hu, Philipp Zabel, Bartlomiej Zolnierkiewicz,
David Airlie, linux-mediatek, Daniel Vetter, Matthias Brugger,
Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
Use common helper for converting a sg_table object into struct
page pointer array.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/mediatek/mtk_drm_gem.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
index 6c34c06..14fcd48 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
@@ -233,9 +233,7 @@ void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
{
struct mtk_drm_gem_obj *mtk_gem = to_mtk_gem_obj(obj);
struct sg_table *sgt;
- struct sg_page_iter iter;
unsigned int npages;
- unsigned int i = 0;
if (mtk_gem->kvaddr)
return mtk_gem->kvaddr;
@@ -249,11 +247,8 @@ void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
if (!mtk_gem->pages)
goto out;
- for_each_sg_page(sgt->sgl, &iter, sgt->orig_nents, 0) {
- mtk_gem->pages[i++] = sg_page_iter_page(&iter);
- if (i > npages)
- break;
- }
+ drm_prime_sg_to_page_addr_arrays(sgt, mtk_gem->pages, NULL, npages);
+
mtk_gem->kvaddr = vmap(mtk_gem->pages, npages, VM_MAP,
pgprot_writecombine(PAGE_KERNEL));
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 16/38] drm: msm: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (13 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 15/38] drm: mediatek: use common helper for extracting pages array Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 17/38] drm: omapdrm: use common helper for extracting pages array Marek Szyprowski
` (23 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: freedreno, Bartlomiej Zolnierkiewicz, David Airlie, Sean Paul,
Rob Clark, Daniel Vetter, linux-arm-msm, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/msm/msm_gem.c | 13 +++++--------
drivers/gpu/drm/msm/msm_gpummu.c | 14 ++++++--------
drivers/gpu/drm/msm/msm_iommu.c | 2 +-
3 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 5a6a79f..6318c20 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -53,11 +53,10 @@ static void sync_for_device(struct msm_gem_object *msm_obj)
struct device *dev = msm_obj->base.dev->dev;
if (get_dma_ops(dev) && IS_ENABLED(CONFIG_ARM64)) {
- dma_sync_sg_for_device(dev, msm_obj->sgt->sgl,
- msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+ dma_sync_sgtable_for_device(dev, msm_obj->sgt,
+ DMA_BIDIRECTIONAL);
} else {
- dma_map_sg(dev, msm_obj->sgt->sgl,
- msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+ dma_map_sgtable(dev, msm_obj->sgt, DMA_BIDIRECTIONAL, 0);
}
}
@@ -66,11 +65,9 @@ static void sync_for_cpu(struct msm_gem_object *msm_obj)
struct device *dev = msm_obj->base.dev->dev;
if (get_dma_ops(dev) && IS_ENABLED(CONFIG_ARM64)) {
- dma_sync_sg_for_cpu(dev, msm_obj->sgt->sgl,
- msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+ dma_sync_sgtable_for_cpu(dev, msm_obj->sgt, DMA_BIDIRECTIONAL);
} else {
- dma_unmap_sg(dev, msm_obj->sgt->sgl,
- msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+ dma_unmap_sgtable(dev, msm_obj->sgt, DMA_BIDIRECTIONAL, 0);
}
}
diff --git a/drivers/gpu/drm/msm/msm_gpummu.c b/drivers/gpu/drm/msm/msm_gpummu.c
index 34980d8..5d8da4d 100644
--- a/drivers/gpu/drm/msm/msm_gpummu.c
+++ b/drivers/gpu/drm/msm/msm_gpummu.c
@@ -35,21 +35,19 @@ static int msm_gpummu_map(struct msm_mmu *mmu, uint64_t iova,
{
struct msm_gpummu *gpummu = to_msm_gpummu(mmu);
unsigned idx = (iova - GPUMMU_VA_START) / GPUMMU_PAGE_SIZE;
- struct scatterlist *sg;
+ struct sg_dma_page_iter dma_iter;
unsigned prot_bits = 0;
- unsigned i, j;
if (prot & IOMMU_WRITE)
prot_bits |= 1;
if (prot & IOMMU_READ)
prot_bits |= 2;
- for_each_sg(sgt->sgl, sg, sgt->nents, i) {
- dma_addr_t addr = sg->dma_address;
- for (j = 0; j < sg->length / GPUMMU_PAGE_SIZE; j++, idx++) {
- gpummu->table[idx] = addr | prot_bits;
- addr += GPUMMU_PAGE_SIZE;
- }
+ for_each_sgtable_dma_page(sgt, &dma_iter, 0) {
+ dma_addr_t addr = sg_page_iter_dma_address(&dma_iter);
+
+ BUILD_BUG_ON(GPUMMU_PAGE_SIZE != PAGE_SIZE);
+ gpummu->table[idx++] = addr | prot_bits;
}
/* we can improve by deferring flush for multiple map() */
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index ad58cfe..d322b39 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -43,7 +43,7 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
struct msm_iommu *iommu = to_msm_iommu(mmu);
size_t ret;
- ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot);
+ ret = iommu_map_sgtable(iommu->domain, iova, sgt, prot);
WARN_ON(!ret);
return (ret == len) ? 0 : -EINVAL;
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 17/38] drm: omapdrm: use common helper for extracting pages array
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (14 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 16/38] drm: msm: fix common struct sg_table related issues Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 18/38] drm: omapdrm: fix common struct sg_table related issues Marek Szyprowski
` (22 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Tomi Valkeinen,
Daniel Vetter, Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
Use common helper for converting a sg_table object into struct
page pointer array.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/omapdrm/omap_gem.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index d08ae95..c259411 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1297,10 +1297,9 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
omap_obj->dma_addr = sg_dma_address(sgt->sgl);
} else {
/* Create pages list from sgt */
- struct sg_page_iter iter;
struct page **pages;
unsigned int npages;
- unsigned int i = 0;
+ unsigned int ret;
npages = DIV_ROUND_UP(size, PAGE_SIZE);
pages = kcalloc(npages, sizeof(*pages), GFP_KERNEL);
@@ -1311,14 +1310,9 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
}
omap_obj->pages = pages;
-
- for_each_sg_page(sgt->sgl, &iter, sgt->orig_nents, 0) {
- pages[i++] = sg_page_iter_page(&iter);
- if (i > npages)
- break;
- }
-
- if (WARN_ON(i != npages)) {
+ ret = drm_prime_sg_to_page_addr_arrays(sgt, pages, NULL,
+ npages);
+ if (WARN_ON(ret)) {
omap_gem_free_object(obj);
obj = ERR_PTR(-ENOMEM);
goto done;
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 18/38] drm: omapdrm: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (15 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 17/38] drm: omapdrm: use common helper for extracting pages array Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 19/38] drm: panfrost: " Marek Szyprowski
` (21 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Tomi Valkeinen,
Daniel Vetter, Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
Fix the code to refer to proper nents or orig_nents entries. This driver
checks for a buffer contiguity in DMA address space, so it should test
sg_table->nents entry.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/omapdrm/omap_gem.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index c259411..cbcb361 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -48,7 +48,7 @@ struct omap_gem_object {
* OMAP_BO_MEM_DMA_API flag set)
*
* - buffers imported from dmabuf (with the OMAP_BO_MEM_DMABUF flag set)
- * if they are physically contiguous (when sgt->orig_nents == 1)
+ * if they are physically contiguous (when sgt->nents == 1)
*
* - buffers mapped through the TILER when dma_addr_cnt is not zero, in
* which case the DMA address points to the TILER aperture
@@ -1279,7 +1279,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
union omap_gem_size gsize;
/* Without a DMM only physically contiguous buffers can be supported. */
- if (sgt->orig_nents != 1 && !priv->has_dmm)
+ if (sgt->nents != 1 && !priv->has_dmm)
return ERR_PTR(-EINVAL);
gsize.bytes = PAGE_ALIGN(size);
@@ -1293,7 +1293,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
omap_obj->sgt = sgt;
- if (sgt->orig_nents == 1) {
+ if (sgt->nents == 1) {
omap_obj->dma_addr = sg_dma_address(sgt->sgl);
} else {
/* Create pages list from sgt */
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 19/38] drm: panfrost: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (16 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 18/38] drm: omapdrm: fix common struct sg_table related issues Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 20/38] drm: radeon: " Marek Szyprowski
` (20 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Rob Herring, Tomeu Vizoso, Bartlomiej Zolnierkiewicz,
David Airlie, Daniel Vetter, Robin Murphy, Christoph Hellwig,
linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Steven Price <steven.price@arm.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/panfrost/panfrost_gem.c | 4 ++--
drivers/gpu/drm/panfrost/panfrost_mmu.c | 7 +++----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 17b654e..6696304 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -41,8 +41,8 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj)
for (i = 0; i < n_sgt; i++) {
if (bo->sgts[i].sgl) {
- dma_unmap_sg(pfdev->dev, bo->sgts[i].sgl,
- bo->sgts[i].nents, DMA_BIDIRECTIONAL);
+ dma_unmap_sgtable(pfdev->dev, &bo->sgts[i],
+ DMA_BIDIRECTIONAL, 0);
sg_free_table(&bo->sgts[i]);
}
}
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index ed28aeb..eb2550e 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -253,7 +253,7 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu,
struct io_pgtable_ops *ops = mmu->pgtbl_ops;
u64 start_iova = iova;
- for_each_sg(sgt->sgl, sgl, sgt->nents, count) {
+ for_each_sgtable_dma_sg(sgt, sgl, count) {
unsigned long paddr = sg_dma_address(sgl);
size_t len = sg_dma_len(sgl);
@@ -517,10 +517,9 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
if (ret)
goto err_pages;
- if (!dma_map_sg(pfdev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL)) {
- ret = -EINVAL;
+ ret = dma_map_sgtable(pfdev->dev, sgt, DMA_BIDIRECTIONAL, 0);
+ if (ret)
goto err_map;
- }
mmu_map_sg(pfdev, bomapping->mmu, addr,
IOMMU_WRITE | IOMMU_READ | IOMMU_NOEXEC, sgt);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 20/38] drm: radeon: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (17 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 19/38] drm: panfrost: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 21/38] drm: rockchip: use common helper for a scatterlist contiguity check Marek Szyprowski
` (19 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, amd-gfx,
Christian König, Daniel Vetter, Alex Deucher, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/radeon/radeon_ttm.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 5d50c9e..0e3eb0d 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -481,7 +481,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm)
{
struct radeon_device *rdev = radeon_get_rdev(ttm->bdev);
struct radeon_ttm_tt *gtt = (void *)ttm;
- unsigned pinned = 0, nents;
+ unsigned pinned = 0;
int r;
int write = !(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
@@ -521,9 +521,8 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm)
if (r)
goto release_sg;
- r = -ENOMEM;
- nents = dma_map_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
- if (nents == 0)
+ r = dma_map_sgtable(rdev->dev, ttm->sg, direction, 0);
+ if (r)
goto release_sg;
drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
@@ -554,9 +553,9 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_tt *ttm)
return;
/* free the sg table and pages again */
- dma_unmap_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
+ dma_unmap_sgtable(rdev->dev, ttm->sg, direction, 0);
- for_each_sg_page(ttm->sg->sgl, &sg_iter, ttm->sg->nents, 0) {
+ for_each_sgtable_page(ttm->sg, &sg_iter, 0) {
struct page *page = sg_page_iter_page(&sg_iter);
if (!(gtt->userflags & RADEON_GEM_USERPTR_READONLY))
set_page_dirty(page);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 21/38] drm: rockchip: use common helper for a scatterlist contiguity check
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (18 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 20/38] drm: radeon: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 22/38] drm: rockchip: fix common struct sg_table related issues Marek Szyprowski
` (18 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Heiko Stübner, Bartlomiej Zolnierkiewicz, David Airlie,
Sandy Huang, linux-rockchip, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
Use common helper for checking the contiguity of the imported dma-buf.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 0d18846..21f8cb2 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -460,23 +460,6 @@ struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj)
return sgt;
}
-static unsigned long rockchip_sg_get_contiguous_size(struct sg_table *sgt,
- int count)
-{
- struct scatterlist *s;
- dma_addr_t expected = sg_dma_address(sgt->sgl);
- unsigned int i;
- unsigned long size = 0;
-
- for_each_sg(sgt->sgl, s, count, i) {
- if (sg_dma_address(s) != expected)
- break;
- expected = sg_dma_address(s) + sg_dma_len(s);
- size += sg_dma_len(s);
- }
- return size;
-}
-
static int
rockchip_gem_iommu_map_sg(struct drm_device *drm,
struct dma_buf_attachment *attach,
@@ -498,7 +481,7 @@ static unsigned long rockchip_sg_get_contiguous_size(struct sg_table *sgt,
if (!count)
return -EINVAL;
- if (rockchip_sg_get_contiguous_size(sg, count) < attach->dmabuf->size) {
+ if (drm_prime_get_contiguous_size(sg) < attach->dmabuf->size) {
DRM_ERROR("failed to map sg_table to contiguous linear address.\n");
dma_unmap_sg(drm->dev, sg->sgl, sg->nents,
DMA_BIDIRECTIONAL);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 22/38] drm: rockchip: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (19 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 21/38] drm: rockchip: use common helper for a scatterlist contiguity check Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 23/38] drm: tegra: " Marek Szyprowski
` (17 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Heiko Stübner, Bartlomiej Zolnierkiewicz, David Airlie,
Sandy Huang, linux-rockchip, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 21f8cb2..566557b 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -36,8 +36,8 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
rk_obj->dma_addr = rk_obj->mm.start;
- ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
- rk_obj->sgt->nents, prot);
+ ret = iommu_map_sgtable(private->domain, rk_obj->dma_addr, rk_obj->sgt,
+ prot);
if (ret < rk_obj->base.size) {
DRM_ERROR("failed to map buffer: size=%zd request_size=%zd\n",
ret, rk_obj->base.size);
@@ -98,11 +98,10 @@ static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
* TODO: Replace this by drm_clflush_sg() once it can be implemented
* without relying on symbols that are not exported.
*/
- for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
+ for_each_sgtable_sg(rk_obj->sgt, s, i)
sg_dma_address(s) = sg_phys(s);
- dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
- DMA_TO_DEVICE);
+ dma_sync_sgtable_for_device(drm->dev, rk_obj->sgt, DMA_TO_DEVICE);
return 0;
@@ -350,8 +349,8 @@ void rockchip_gem_free_object(struct drm_gem_object *obj)
if (private->domain) {
rockchip_gem_iommu_unmap(rk_obj);
} else {
- dma_unmap_sg(drm->dev, rk_obj->sgt->sgl,
- rk_obj->sgt->nents, DMA_BIDIRECTIONAL);
+ dma_unmap_sgtable(drm->dev, rk_obj->sgt,
+ DMA_BIDIRECTIONAL, 0);
}
drm_prime_gem_destroy(obj, rk_obj->sgt);
} else {
@@ -476,15 +475,13 @@ struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj)
struct sg_table *sg,
struct rockchip_gem_object *rk_obj)
{
- int count = dma_map_sg(drm->dev, sg->sgl, sg->nents,
- DMA_BIDIRECTIONAL);
- if (!count)
- return -EINVAL;
+ int err = dma_map_sgtable(drm->dev, sg, DMA_BIDIRECTIONAL, 0);
+ if (err)
+ return err;
if (drm_prime_get_contiguous_size(sg) < attach->dmabuf->size) {
DRM_ERROR("failed to map sg_table to contiguous linear address.\n");
- dma_unmap_sg(drm->dev, sg->sgl, sg->nents,
- DMA_BIDIRECTIONAL);
+ dma_unmap_sgtable(drm->dev, sg, DMA_BIDIRECTIONAL, 0);
return -EINVAL;
}
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 23/38] drm: tegra: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (20 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 22/38] drm: rockchip: fix common struct sg_table related issues Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 24/38] drm: v3d: " Marek Szyprowski
` (16 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Jonathan Hunter,
Thierry Reding, Daniel Vetter, linux-tegra, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/tegra/gem.c | 27 ++++++++++-----------------
drivers/gpu/drm/tegra/plane.c | 15 +++++----------
2 files changed, 15 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 6237681..2169130 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -98,8 +98,8 @@ static struct sg_table *tegra_bo_pin(struct device *dev, struct host1x_bo *bo,
* the SG table needs to be copied to avoid overwriting any
* other potential users of the original SG table.
*/
- err = sg_alloc_table_from_sg(sgt, obj->sgt->sgl, obj->sgt->nents,
- GFP_KERNEL);
+ err = sg_alloc_table_from_sg(sgt, obj->sgt->sgl,
+ obj->sgt->orig_nents, GFP_KERNEL);
if (err < 0)
goto free;
} else {
@@ -196,8 +196,7 @@ static int tegra_bo_iommu_map(struct tegra_drm *tegra, struct tegra_bo *bo)
bo->iova = bo->mm->start;
- bo->size = iommu_map_sg(tegra->domain, bo->iova, bo->sgt->sgl,
- bo->sgt->nents, prot);
+ bo->size = iommu_map_sgtable(tegra->domain, bo->iova, bo->sgt, prot);
if (!bo->size) {
dev_err(tegra->drm->dev, "failed to map buffer\n");
err = -ENOMEM;
@@ -264,8 +263,7 @@ static struct tegra_bo *tegra_bo_alloc_object(struct drm_device *drm,
static void tegra_bo_free(struct drm_device *drm, struct tegra_bo *bo)
{
if (bo->pages) {
- dma_unmap_sg(drm->dev, bo->sgt->sgl, bo->sgt->nents,
- DMA_FROM_DEVICE);
+ dma_unmap_sgtable(drm->dev, bo->sgt, DMA_FROM_DEVICE, 0);
drm_gem_put_pages(&bo->gem, bo->pages, true, true);
sg_free_table(bo->sgt);
kfree(bo->sgt);
@@ -290,12 +288,9 @@ static int tegra_bo_get_pages(struct drm_device *drm, struct tegra_bo *bo)
goto put_pages;
}
- err = dma_map_sg(drm->dev, bo->sgt->sgl, bo->sgt->nents,
- DMA_FROM_DEVICE);
- if (err == 0) {
- err = -EFAULT;
+ err = dma_map_sgtable(drm->dev, bo->sgt, DMA_FROM_DEVICE, 0);
+ if (err)
goto free_sgt;
- }
return 0;
@@ -571,7 +566,7 @@ int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
goto free;
}
- if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0)
+ if (dma_map_sgtable(attach->dev, sgt, dir, 0))
goto free;
return sgt;
@@ -590,7 +585,7 @@ static void tegra_gem_prime_unmap_dma_buf(struct dma_buf_attachment *attach,
struct tegra_bo *bo = to_tegra_bo(gem);
if (bo->pages)
- dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+ dma_unmap_sgtable(attach->dev, sgt, dir, 0);
sg_free_table(sgt);
kfree(sgt);
@@ -609,8 +604,7 @@ static int tegra_gem_prime_begin_cpu_access(struct dma_buf *buf,
struct drm_device *drm = gem->dev;
if (bo->pages)
- dma_sync_sg_for_cpu(drm->dev, bo->sgt->sgl, bo->sgt->nents,
- DMA_FROM_DEVICE);
+ dma_sync_sgtable_for_cpu(drm->dev, bo->sgt, DMA_FROM_DEVICE);
return 0;
}
@@ -623,8 +617,7 @@ static int tegra_gem_prime_end_cpu_access(struct dma_buf *buf,
struct drm_device *drm = gem->dev;
if (bo->pages)
- dma_sync_sg_for_device(drm->dev, bo->sgt->sgl, bo->sgt->nents,
- DMA_TO_DEVICE);
+ dma_sync_sgtable_for_device(drm->dev, bo->sgt, DMA_TO_DEVICE);
return 0;
}
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 9ccfb56..0d2ef16 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -130,12 +130,9 @@ static int tegra_dc_pin(struct tegra_dc *dc, struct tegra_plane_state *state)
}
if (sgt) {
- err = dma_map_sg(dc->dev, sgt->sgl, sgt->nents,
- DMA_TO_DEVICE);
- if (err == 0) {
- err = -ENOMEM;
+ err = dma_map_sgtable(dc->dev, sgt, DMA_TO_DEVICE, 0);
+ if (err)
goto unpin;
- }
/*
* The display controller needs contiguous memory, so
@@ -143,7 +140,7 @@ static int tegra_dc_pin(struct tegra_dc *dc, struct tegra_plane_state *state)
* map its SG table to a single contiguous chunk of
* I/O virtual memory.
*/
- if (err > 1) {
+ if (sgt->nents > 1) {
err = -EINVAL;
goto unpin;
}
@@ -165,8 +162,7 @@ static int tegra_dc_pin(struct tegra_dc *dc, struct tegra_plane_state *state)
struct sg_table *sgt = state->sgt[i];
if (sgt)
- dma_unmap_sg(dc->dev, sgt->sgl, sgt->nents,
- DMA_TO_DEVICE);
+ dma_unmap_sgtable(dc->dev, sgt, DMA_TO_DEVICE, 0);
host1x_bo_unpin(dc->dev, &bo->base, sgt);
state->iova[i] = DMA_MAPPING_ERROR;
@@ -185,8 +181,7 @@ static void tegra_dc_unpin(struct tegra_dc *dc, struct tegra_plane_state *state)
struct sg_table *sgt = state->sgt[i];
if (sgt)
- dma_unmap_sg(dc->dev, sgt->sgl, sgt->nents,
- DMA_TO_DEVICE);
+ dma_unmap_sgtable(dc->dev, sgt, DMA_TO_DEVICE, 0);
host1x_bo_unpin(dc->dev, &bo->base, sgt);
state->iova[i] = DMA_MAPPING_ERROR;
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 24/38] drm: v3d: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (21 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 23/38] drm: tegra: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 25/38] drm: virtio: " Marek Szyprowski
` (15 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Eric Anholt,
Daniel Vetter, Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/v3d/v3d_mmu.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_mmu.c b/drivers/gpu/drm/v3d/v3d_mmu.c
index 3b81ea2..175c257 100644
--- a/drivers/gpu/drm/v3d/v3d_mmu.c
+++ b/drivers/gpu/drm/v3d/v3d_mmu.c
@@ -90,19 +90,16 @@ void v3d_mmu_insert_ptes(struct v3d_bo *bo)
struct v3d_dev *v3d = to_v3d_dev(shmem_obj->base.dev);
u32 page = bo->node.start;
u32 page_prot = V3D_PTE_WRITEABLE | V3D_PTE_VALID;
- unsigned int count;
- struct scatterlist *sgl;
+ struct sg_dma_page_iter dma_iter;
- for_each_sg(shmem_obj->sgt->sgl, sgl, shmem_obj->sgt->nents, count) {
- u32 page_address = sg_dma_address(sgl) >> V3D_MMU_PAGE_SHIFT;
+ for_each_sgtable_dma_page(shmem_obj->sgt, &dma_iter, 0) {
+ dma_addr_t dma_addr = sg_page_iter_dma_address(&dma_iter);
+ u32 page_address = dma_addr >> V3D_MMU_PAGE_SHIFT;
u32 pte = page_prot | page_address;
- u32 i;
- BUG_ON(page_address + (sg_dma_len(sgl) >> V3D_MMU_PAGE_SHIFT) >=
- BIT(24));
-
- for (i = 0; i < sg_dma_len(sgl) >> V3D_MMU_PAGE_SHIFT; i++)
- v3d->pt[page++] = pte + i;
+ BUILD_BUG_ON(V3D_MMU_PAGE_SHIFT != PAGE_SIZE);
+ BUG_ON(page_address + 1 >= BIT(24));
+ v3d->pt[page++] = pte;
}
WARN_ON_ONCE(page - bo->node.start !=
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 25/38] drm: virtio: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (22 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 24/38] drm: v3d: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 26/38] drm: vmwgfx: " Marek Szyprowski
` (14 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, virtualization,
Gerd Hoffmann, Daniel Vetter, Robin Murphy, Christoph Hellwig,
linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/virtio/virtgpu_object.c | 36 +++++++++++++++++++--------------
drivers/gpu/drm/virtio/virtgpu_vq.c | 12 +++++------
2 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 6ccbd01..399556f 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -72,9 +72,8 @@ void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
if (shmem->pages) {
if (shmem->mapped) {
- dma_unmap_sg(vgdev->vdev->dev.parent,
- shmem->pages->sgl, shmem->mapped,
- DMA_TO_DEVICE);
+ dma_unmap_sgtable(vgdev->vdev->dev.parent,
+ shmem->pages, DMA_TO_DEVICE, 0);
shmem->mapped = 0;
}
@@ -157,13 +156,13 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev,
}
if (use_dma_api) {
- shmem->mapped = dma_map_sg(vgdev->vdev->dev.parent,
- shmem->pages->sgl,
- shmem->pages->nents,
- DMA_TO_DEVICE);
- *nents = shmem->mapped;
+ ret = dma_map_sgtable(vgdev->vdev->dev.parent,
+ shmem->pages, DMA_TO_DEVICE, 0);
+ if (ret)
+ return ret;
+ *nents = shmem->mapped = shmem->pages->nents;
} else {
- *nents = shmem->pages->nents;
+ *nents = shmem->pages->orig_nents;
}
*ents = kmalloc_array(*nents, sizeof(struct virtio_gpu_mem_entry),
@@ -173,13 +172,20 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev,
return -ENOMEM;
}
- for_each_sg(shmem->pages->sgl, sg, *nents, si) {
- (*ents)[si].addr = cpu_to_le64(use_dma_api
- ? sg_dma_address(sg)
- : sg_phys(sg));
- (*ents)[si].length = cpu_to_le32(sg->length);
- (*ents)[si].padding = 0;
+ if (use_dma_api) {
+ for_each_sgtable_dma_sg(shmem->pages, sg, si) {
+ (*ents)[si].addr = cpu_to_le64(sg_dma_address(sg));
+ (*ents)[si].length = cpu_to_le32(sg_dma_len(sg));
+ (*ents)[si].padding = 0;
+ }
+ } else {
+ for_each_sgtable_sg(shmem->pages, sg, si) {
+ (*ents)[si].addr = cpu_to_le64(sg_phys(sg));
+ (*ents)[si].length = cpu_to_le32(sg->length);
+ (*ents)[si].padding = 0;
+ }
}
+
return 0;
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 9e663a5..e5765db 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -302,7 +302,7 @@ static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents)
return NULL;
}
- for_each_sg(sgt->sgl, sg, *sg_ents, i) {
+ for_each_sgtable_sg(sgt, sg, i) {
pg = vmalloc_to_page(data);
if (!pg) {
sg_free_table(sgt);
@@ -603,9 +603,8 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
if (use_dma_api)
- dma_sync_sg_for_device(vgdev->vdev->dev.parent,
- shmem->pages->sgl, shmem->pages->nents,
- DMA_TO_DEVICE);
+ dma_sync_sgtable_for_device(vgdev->vdev->dev.parent,
+ shmem->pages, DMA_TO_DEVICE);
cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
memset(cmd_p, 0, sizeof(*cmd_p));
@@ -1019,9 +1018,8 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
if (use_dma_api)
- dma_sync_sg_for_device(vgdev->vdev->dev.parent,
- shmem->pages->sgl, shmem->pages->nents,
- DMA_TO_DEVICE);
+ dma_sync_sgtable_for_device(vgdev->vdev->dev.parent,
+ shmem->pages, DMA_TO_DEVICE);
cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
memset(cmd_p, 0, sizeof(*cmd_p));
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 26/38] drm: vmwgfx: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (23 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 25/38] drm: virtio: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 27/38] xen: gntdev: " Marek Szyprowski
` (13 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Thomas Hellstrom, Bartlomiej Zolnierkiewicz, David Airlie,
VMware Graphics, Daniel Vetter, Robin Murphy, Christoph Hellwig,
linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Roland Scheidegger <sroland@vmware.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index bf0bc46..49ed6ad 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -362,8 +362,7 @@ static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt *vmw_tt)
{
struct device *dev = vmw_tt->dev_priv->dev->dev;
- dma_unmap_sg(dev, vmw_tt->sgt.sgl, vmw_tt->sgt.nents,
- DMA_BIDIRECTIONAL);
+ dma_unmap_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL, 0);
vmw_tt->sgt.nents = vmw_tt->sgt.orig_nents;
}
@@ -383,16 +382,8 @@ static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt *vmw_tt)
static int vmw_ttm_map_for_dma(struct vmw_ttm_tt *vmw_tt)
{
struct device *dev = vmw_tt->dev_priv->dev->dev;
- int ret;
-
- ret = dma_map_sg(dev, vmw_tt->sgt.sgl, vmw_tt->sgt.orig_nents,
- DMA_BIDIRECTIONAL);
- if (unlikely(ret == 0))
- return -ENOMEM;
- vmw_tt->sgt.nents = ret;
-
- return 0;
+ return dma_map_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL, 0);
}
/**
@@ -449,10 +440,10 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt)
if (unlikely(ret != 0))
goto out_sg_alloc_fail;
- if (vsgt->num_pages > vmw_tt->sgt.nents) {
+ if (vsgt->num_pages > vmw_tt->sgt.orig_nents) {
uint64_t over_alloc =
sgl_size * (vsgt->num_pages -
- vmw_tt->sgt.nents);
+ vmw_tt->sgt.orig_nents);
ttm_mem_global_free(glob, over_alloc);
vmw_tt->sg_alloc_size -= over_alloc;
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 27/38] xen: gntdev: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (24 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 26/38] drm: vmwgfx: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 28/38] drm: host1x: " Marek Szyprowski
` (12 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Juergen Gross, Bartlomiej Zolnierkiewicz, David Airlie,
Daniel Vetter, xen-devel, Boris Ostrovsky, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/xen/gntdev-dmabuf.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c
index 75d3bb9..ba6cad8 100644
--- a/drivers/xen/gntdev-dmabuf.c
+++ b/drivers/xen/gntdev-dmabuf.c
@@ -247,10 +247,9 @@ static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf,
if (sgt) {
if (gntdev_dmabuf_attach->dir != DMA_NONE)
- dma_unmap_sg_attrs(attach->dev, sgt->sgl,
- sgt->nents,
- gntdev_dmabuf_attach->dir,
- DMA_ATTR_SKIP_CPU_SYNC);
+ dma_unmap_sgtable(attach->dev, sgt,
+ gntdev_dmabuf_attach->dir,
+ DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sgt);
}
@@ -288,8 +287,8 @@ static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf,
sgt = dmabuf_pages_to_sgt(gntdev_dmabuf->pages,
gntdev_dmabuf->nr_pages);
if (!IS_ERR(sgt)) {
- if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC)) {
+ if (dma_map_sgtable(attach->dev, sgt, dir,
+ DMA_ATTR_SKIP_CPU_SYNC)) {
sg_free_table(sgt);
kfree(sgt);
sgt = ERR_PTR(-ENOMEM);
@@ -625,7 +624,7 @@ static struct gntdev_dmabuf *dmabuf_imp_alloc_storage(int count)
/* Now convert sgt to array of pages and check for page validity. */
i = 0;
- for_each_sg_page(sgt->sgl, &sg_iter, sgt->nents, 0) {
+ for_each_sgtable_page(sgt, &sg_iter, 0) {
struct page *page = sg_page_iter_page(&sg_iter);
/*
* Check if page is valid: this can happen if we are given
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 28/38] drm: host1x: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (25 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 27/38] xen: gntdev: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 29/38] drm: rcar-du: " Marek Szyprowski
` (11 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Thierry Reding,
Daniel Vetter, linux-tegra, Robin Murphy, Christoph Hellwig,
linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/host1x/job.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index a10643a..4832b57 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -166,11 +166,9 @@ static unsigned int pin_job(struct host1x *host, struct host1x_job *job)
goto unpin;
}
- err = dma_map_sg(dev, sgt->sgl, sgt->nents, dir);
- if (!err) {
- err = -ENOMEM;
+ err = dma_map_sgtable(dev, sgt, dir, 0);
+ if (err)
goto unpin;
- }
job->unpins[job->num_unpins].dev = dev;
job->unpins[job->num_unpins].dir = dir;
@@ -217,7 +215,7 @@ static unsigned int pin_job(struct host1x *host, struct host1x_job *job)
}
if (!IS_ENABLED(CONFIG_TEGRA_HOST1X_FIREWALL) && host->domain) {
- for_each_sg(sgt->sgl, sg, sgt->nents, j)
+ for_each_sgtable_sg(sgt, sg, j)
gather_size += sg->length;
gather_size = iova_align(&host->iova, gather_size);
@@ -229,9 +227,9 @@ static unsigned int pin_job(struct host1x *host, struct host1x_job *job)
goto unpin;
}
- err = iommu_map_sg(host->domain,
+ err = iommu_map_sgtable(host->domain,
iova_dma_addr(&host->iova, alloc),
- sgt->sgl, sgt->nents, IOMMU_READ);
+ sgt, IOMMU_READ);
if (err == 0) {
__free_iova(&host->iova, alloc);
err = -EINVAL;
@@ -241,12 +239,9 @@ static unsigned int pin_job(struct host1x *host, struct host1x_job *job)
job->unpins[job->num_unpins].size = gather_size;
phys_addr = iova_dma_addr(&host->iova, alloc);
} else if (sgt) {
- err = dma_map_sg(host->dev, sgt->sgl, sgt->nents,
- DMA_TO_DEVICE);
- if (!err) {
- err = -ENOMEM;
+ err = dma_map_sgtable(host->dev, sgt, DMA_TO_DEVICE, 0);
+ if (err)
goto unpin;
- }
job->unpins[job->num_unpins].dir = DMA_TO_DEVICE;
job->unpins[job->num_unpins].dev = host->dev;
@@ -647,8 +642,7 @@ void host1x_job_unpin(struct host1x_job *job)
}
if (unpin->dev && sgt)
- dma_unmap_sg(unpin->dev, sgt->sgl, sgt->nents,
- unpin->dir);
+ dma_unmap_sgtable(unpin->dev, sgt, unpin->dir, 0);
host1x_bo_unpin(dev, unpin->bo, sgt);
host1x_bo_put(unpin->bo);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 29/38] drm: rcar-du: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (26 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 28/38] drm: host1x: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 30/38] dmabuf: " Marek Szyprowski
` (10 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, linux-media,
linux-renesas-soc, Kieran Bingham, Laurent Pinchart,
Daniel Vetter, Mauro Carvalho Chehab, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
dma_map_sgtable() function returns zero or an error code, so adjust the
return value check for the vsp1_du_map_sg() function.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +--
drivers/media/platform/vsp1/vsp1_drm.c | 8 ++++----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 5e4faf2..2fc1816 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -197,9 +197,8 @@ int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb,
goto fail;
ret = vsp1_du_map_sg(vsp->vsp, sgt);
- if (!ret) {
+ if (ret) {
sg_free_table(sgt);
- ret = -ENOMEM;
goto fail;
}
}
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index a4a45d6..86d5e3f 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -912,8 +912,8 @@ int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt)
* skip cache sync. This will need to be revisited when support for
* non-coherent buffers will be added to the DU driver.
*/
- return dma_map_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents,
- DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+ return dma_map_sgtable(vsp1->bus_master, sgt, DMA_TO_DEVICE,
+ DMA_ATTR_SKIP_CPU_SYNC);
}
EXPORT_SYMBOL_GPL(vsp1_du_map_sg);
@@ -921,8 +921,8 @@ void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt)
{
struct vsp1_device *vsp1 = dev_get_drvdata(dev);
- dma_unmap_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents,
- DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+ dma_unmap_sgtable(vsp1->bus_master, sgt, DMA_TO_DEVICE,
+ DMA_ATTR_SKIP_CPU_SYNC);
}
EXPORT_SYMBOL_GPL(vsp1_du_unmap_sg);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 30/38] dmabuf: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (27 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 29/38] drm: rcar-du: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 31/38] staging: ion: remove dead code Marek Szyprowski
` (9 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Sumit Semwal,
Gerd Hoffmann, Daniel Vetter, linux-media, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/dma-buf/heaps/heap-helpers.c | 13 ++++++-------
drivers/dma-buf/udmabuf.c | 7 +++----
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c
index 9f964ca..be9523a 100644
--- a/drivers/dma-buf/heaps/heap-helpers.c
+++ b/drivers/dma-buf/heaps/heap-helpers.c
@@ -140,13 +140,12 @@ struct sg_table *dma_heap_map_dma_buf(struct dma_buf_attachment *attachment,
enum dma_data_direction direction)
{
struct dma_heaps_attachment *a = attachment->priv;
- struct sg_table *table;
-
- table = &a->table;
+ struct sg_table *table = &a->table;
+ int ret;
- if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
- direction))
- table = ERR_PTR(-ENOMEM);
+ ret = dma_map_sgtable(attachment->dev, table, direction);
+ if (ret)
+ table = ERR_PTR(ret);
return table;
}
@@ -154,7 +153,7 @@ static void dma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment,
struct sg_table *table,
enum dma_data_direction direction)
{
- dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
+ dma_unmap_sgtable(attachment->dev, table, direction);
}
static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index acb26c6..5bcbf7a 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -63,10 +63,9 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
GFP_KERNEL);
if (ret < 0)
goto err;
- if (!dma_map_sg(dev, sg->sgl, sg->nents, direction)) {
- ret = -EINVAL;
+ ret = dma_map_sgtable(dev, sg, direction);
+ if (ret < 0)
goto err;
- }
return sg;
err:
@@ -78,7 +77,7 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
static void put_sg_table(struct device *dev, struct sg_table *sg,
enum dma_data_direction direction)
{
- dma_unmap_sg(dev, sg->sgl, sg->nents, direction);
+ dma_unmap_sgtable(dev, sg, direction);
sg_free_table(sg);
kfree(sg);
}
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 31/38] staging: ion: remove dead code
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (28 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 30/38] dmabuf: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 32/38] staging: ion: fix common struct sg_table related issues Marek Szyprowski
` (8 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: devel, Todd Kjos, Bartlomiej Zolnierkiewicz, David Airlie,
Greg Kroah-Hartman, Sumit Semwal, Arve Hjønnevåg,
Martijn Coenen, Christian Brauner, Daniel Vetter, Joel Fernandes,
Laura Abbott, Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
ion_heap_pages_zero() function is not used at all, so remove it to
simplify the ion_heap_sglist_zero() function later.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/staging/android/ion/ion.h | 1 -
drivers/staging/android/ion/ion_heap.c | 9 ---------
2 files changed, 10 deletions(-)
diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
index 74914a2..c199e88 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -177,7 +177,6 @@ struct ion_heap {
int ion_heap_map_user(struct ion_heap *heap, struct ion_buffer *buffer,
struct vm_area_struct *vma);
int ion_heap_buffer_zero(struct ion_buffer *buffer);
-int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot);
/**
* ion_heap_init_shrinker
diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c
index 0755b11..9c23b23 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -145,15 +145,6 @@ int ion_heap_buffer_zero(struct ion_buffer *buffer)
return ion_heap_sglist_zero(table->sgl, table->nents, pgprot);
}
-int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot)
-{
- struct scatterlist sg;
-
- sg_init_table(&sg, 1);
- sg_set_page(&sg, page, size, 0);
- return ion_heap_sglist_zero(&sg, 1, pgprot);
-}
-
void ion_heap_freelist_add(struct ion_heap *heap, struct ion_buffer *buffer)
{
spin_lock(&heap->free_lock);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 32/38] staging: ion: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (29 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 31/38] staging: ion: remove dead code Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 33/38] staging: tegra-vde: " Marek Szyprowski
` (7 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: devel, Todd Kjos, Bartlomiej Zolnierkiewicz, David Airlie,
Greg Kroah-Hartman, Sumit Semwal, Arve Hjønnevåg,
Martijn Coenen, Christian Brauner, Daniel Vetter, Joel Fernandes,
Laura Abbott, Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/staging/android/ion/ion.c | 25 +++++++--------
drivers/staging/android/ion/ion_heap.c | 44 ++++++++-------------------
drivers/staging/android/ion/ion_system_heap.c | 2 +-
3 files changed, 25 insertions(+), 46 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 38b51ea..2db041d 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -147,14 +147,14 @@ static struct sg_table *dup_sg_table(struct sg_table *table)
if (!new_table)
return ERR_PTR(-ENOMEM);
- ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL);
+ ret = sg_alloc_table(new_table, table->orig_nents, GFP_KERNEL);
if (ret) {
kfree(new_table);
return ERR_PTR(-ENOMEM);
}
new_sg = new_table->sgl;
- for_each_sg(table->sgl, sg, table->nents, i) {
+ for_each_sgtable_sg(table, sg, i) {
memcpy(new_sg, sg, sizeof(*sg));
new_sg->dma_address = 0;
new_sg = sg_next(new_sg);
@@ -224,12 +224,13 @@ static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
{
struct ion_dma_buf_attachment *a = attachment->priv;
struct sg_table *table;
+ int ret;
table = a->table;
- if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
- direction))
- return ERR_PTR(-ENOMEM);
+ ret = dma_map_sgtable(attachment->dev, table, direction);
+ if (ret)
+ return ERR_PTR(ret);
return table;
}
@@ -238,7 +239,7 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
struct sg_table *table,
enum dma_data_direction direction)
{
- dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
+ dma_unmap_sgtable(attachment->dev, table, direction);
}
static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
@@ -296,10 +297,8 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
}
mutex_lock(&buffer->lock);
- list_for_each_entry(a, &buffer->attachments, list) {
- dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
- direction);
- }
+ list_for_each_entry(a, &buffer->attachments, list)
+ dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
unlock:
mutex_unlock(&buffer->lock);
@@ -319,10 +318,8 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
}
mutex_lock(&buffer->lock);
- list_for_each_entry(a, &buffer->attachments, list) {
- dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents,
- direction);
- }
+ list_for_each_entry(a, &buffer->attachments, list)
+ dma_sync_sgtable_for_device(a->dev, a->table, direction);
mutex_unlock(&buffer->lock);
return 0;
diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c
index 9c23b23..79f2794 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -20,8 +20,7 @@
void *ion_heap_map_kernel(struct ion_heap *heap,
struct ion_buffer *buffer)
{
- struct scatterlist *sg;
- int i, j;
+ struct sg_page_iter piter;
void *vaddr;
pgprot_t pgprot;
struct sg_table *table = buffer->sg_table;
@@ -38,14 +37,11 @@ void *ion_heap_map_kernel(struct ion_heap *heap,
else
pgprot = pgprot_writecombine(PAGE_KERNEL);
- for_each_sg(table->sgl, sg, table->nents, i) {
- int npages_this_entry = PAGE_ALIGN(sg->length) / PAGE_SIZE;
- struct page *page = sg_page(sg);
-
- BUG_ON(i >= npages);
- for (j = 0; j < npages_this_entry; j++)
- *(tmp++) = page++;
+ for_each_sgtable_page(table, &piter, 0) {
+ BUG_ON(tmp - pages >= npages);
+ *tmp++ = sg_page_iter_page(&piter);
}
+
vaddr = vmap(pages, npages, VM_MAP, pgprot);
vfree(pages);
@@ -64,32 +60,19 @@ void ion_heap_unmap_kernel(struct ion_heap *heap,
int ion_heap_map_user(struct ion_heap *heap, struct ion_buffer *buffer,
struct vm_area_struct *vma)
{
+ struct sg_page_iter piter;
struct sg_table *table = buffer->sg_table;
unsigned long addr = vma->vm_start;
- unsigned long offset = vma->vm_pgoff * PAGE_SIZE;
- struct scatterlist *sg;
- int i;
int ret;
- for_each_sg(table->sgl, sg, table->nents, i) {
- struct page *page = sg_page(sg);
- unsigned long remainder = vma->vm_end - addr;
- unsigned long len = sg->length;
+ for_each_sgtable_page(table, &piter, vma->vm_pgoff) {
+ struct page *page = sg_page_iter_page(&piter);
- if (offset >= sg->length) {
- offset -= sg->length;
- continue;
- } else if (offset) {
- page += offset / PAGE_SIZE;
- len = sg->length - offset;
- offset = 0;
- }
- len = min(len, remainder);
- ret = remap_pfn_range(vma, addr, page_to_pfn(page), len,
+ ret = remap_pfn_range(vma, addr, page_to_pfn(page), PAGE_SIZE,
vma->vm_page_prot);
if (ret)
return ret;
- addr += len;
+ addr += PAGE_SIZE;
if (addr >= vma->vm_end)
return 0;
}
@@ -109,15 +92,14 @@ static int ion_heap_clear_pages(struct page **pages, int num, pgprot_t pgprot)
return 0;
}
-static int ion_heap_sglist_zero(struct scatterlist *sgl, unsigned int nents,
- pgprot_t pgprot)
+static int ion_heap_sglist_zero(struct sg_table *sgt, pgprot_t pgprot)
{
int p = 0;
int ret = 0;
struct sg_page_iter piter;
struct page *pages[32];
- for_each_sg_page(sgl, &piter, nents, 0) {
+ for_each_sgtable_page(sgt, &piter, 0) {
pages[p++] = sg_page_iter_page(&piter);
if (p == ARRAY_SIZE(pages)) {
ret = ion_heap_clear_pages(pages, p, pgprot);
@@ -142,7 +124,7 @@ int ion_heap_buffer_zero(struct ion_buffer *buffer)
else
pgprot = pgprot_writecombine(PAGE_KERNEL);
- return ion_heap_sglist_zero(table->sgl, table->nents, pgprot);
+ return ion_heap_sglist_zero(table, pgprot);
}
void ion_heap_freelist_add(struct ion_heap *heap, struct ion_buffer *buffer)
diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index b83a1d1..eac0632 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -162,7 +162,7 @@ static void ion_system_heap_free(struct ion_buffer *buffer)
if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE))
ion_heap_buffer_zero(buffer);
- for_each_sg(table->sgl, sg, table->nents, i)
+ for_each_sgtable_sg(table, sg, i)
free_buffer_page(sys_heap, buffer, sg_page(sg));
sg_free_table(table);
kfree(table);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 33/38] staging: tegra-vde: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (30 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 32/38] staging: ion: fix common struct sg_table related issues Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 34/38] misc: fastrpc: " Marek Szyprowski
` (6 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: devel, Bartlomiej Zolnierkiewicz, David Airlie,
Greg Kroah-Hartman, linux-media, Jonathan Hunter, Thierry Reding,
Daniel Vetter, linux-tegra, Dmitry Osipenko,
Mauro Carvalho Chehab, Robin Murphy, Christoph Hellwig,
linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/staging/media/tegra-vde/iommu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c
index 6af863d..adf8dc7 100644
--- a/drivers/staging/media/tegra-vde/iommu.c
+++ b/drivers/staging/media/tegra-vde/iommu.c
@@ -36,8 +36,8 @@ int tegra_vde_iommu_map(struct tegra_vde *vde,
addr = iova_dma_addr(&vde->iova, iova);
- size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents,
- IOMMU_READ | IOMMU_WRITE);
+ size = iommu_map_sgtable(vde->domain, addr, sgt,
+ IOMMU_READ | IOMMU_WRITE);
if (!size) {
__free_iova(&vde->iova, iova);
return -ENXIO;
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 34/38] misc: fastrpc: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (31 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 33/38] staging: tegra-vde: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 35/38] rapidio: " Marek Szyprowski
` (5 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Arnd Bergmann, Bartlomiej Zolnierkiewicz, David Airlie,
Greg Kroah-Hartman, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/misc/fastrpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index e3e085e..0a3e02aa 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -518,7 +518,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
table = &a->sgt;
- if (!dma_map_sg(attachment->dev, table->sgl, table->nents, dir))
+ if (!dma_map_sgtable(attachment->dev, table, dir))
return ERR_PTR(-ENOMEM);
return table;
@@ -528,7 +528,7 @@ static void fastrpc_unmap_dma_buf(struct dma_buf_attachment *attach,
struct sg_table *table,
enum dma_data_direction dir)
{
- dma_unmap_sg(attach->dev, table->sgl, table->nents, dir);
+ dma_unmap_sgtable(attach->dev, table, dir);
}
static void fastrpc_release(struct dma_buf *dmabuf)
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 35/38] rapidio: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (32 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 34/38] misc: fastrpc: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 36/38] samples: vfio-mdev/mbochs: " Marek Szyprowski
` (4 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, Alexandre Bounine,
Daniel Vetter, Matt Porter, Robin Murphy, Christoph Hellwig,
linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/rapidio/devices/rio_mport_cdev.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
index 4029637..df7dba8 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -574,8 +574,7 @@ static void dma_req_free(struct kref *ref)
struct mport_cdev_priv *priv = req->priv;
unsigned int i;
- dma_unmap_sg(req->dmach->device->dev,
- req->sgt.sgl, req->sgt.nents, req->dir);
+ dma_unmap_sgtable(req->dmach->device->dev, req->sgt, req->dir);
sg_free_table(&req->sgt);
if (req->page_list) {
for (i = 0; i < req->nr_pages; i++)
@@ -927,9 +926,8 @@ static int do_dma_request(struct mport_dma_req *req,
xfer->offset, xfer->length);
}
- nents = dma_map_sg(chan->device->dev,
- req->sgt.sgl, req->sgt.nents, dir);
- if (nents == 0) {
+ ret = dma_map_sgtable(chan->device->dev, req->sgt, dir);
+ if (ret) {
rmcd_error("Failed to map SG list");
ret = -EFAULT;
goto err_pg;
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 36/38] samples: vfio-mdev/mbochs: fix common struct sg_table related issues
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (33 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 35/38] rapidio: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 37/38] media: pci: fix common ALSA DMA-mapping related codes Marek Szyprowski
` (3 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: kvm, Bartlomiej Zolnierkiewicz, David Airlie, Kirti Wankhede,
Daniel Vetter, Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
returns the number of the created entries in the DMA address space.
However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
dma_unmap_sg must be called with the original number of the entries
passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous
memory buffer, used commonly in the DRM and graphics subsystems. It
consists of a scatterlist with memory pages and DMA addresses (sgl entry),
as well as the number of scatterlist entries: CPU pages (orig_nents entry)
and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents
entries, calling DMA-mapping functions with a wrong number of entries or
ignoring the number of mapped entries returned by the dma_map_sg()
function.
To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.
While touching this code, also add missing call to dma_unmap_sgtable.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
samples/vfio-mdev/mbochs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
index 3cc5e59..f2c62e0 100644
--- a/samples/vfio-mdev/mbochs.c
+++ b/samples/vfio-mdev/mbochs.c
@@ -846,7 +846,7 @@ static struct sg_table *mbochs_map_dmabuf(struct dma_buf_attachment *at,
if (sg_alloc_table_from_pages(sg, dmabuf->pages, dmabuf->pagecount,
0, dmabuf->mode.size, GFP_KERNEL) < 0)
goto err2;
- if (!dma_map_sg(at->dev, sg->sgl, sg->nents, direction))
+ if (dma_map_sgtable(at->dev, sg, direction))
goto err3;
return sg;
@@ -868,6 +868,7 @@ static void mbochs_unmap_dmabuf(struct dma_buf_attachment *at,
dev_dbg(dev, "%s: %d\n", __func__, dmabuf->id);
+ dma_unmap_sgtable(at->dev, sg, direction);
sg_free_table(sg);
kfree(sg);
}
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 37/38] media: pci: fix common ALSA DMA-mapping related codes
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (34 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 36/38] samples: vfio-mdev/mbochs: " Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 9:00 ` [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist wrappers Marek Szyprowski
` (2 subsequent siblings)
38 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, David Airlie, linux-media,
Daniel Vetter, Hans Verkuil, Mauro Carvalho Chehab, Robin Murphy,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
drivers/media/pci/cx23885/cx23885-alsa.c | 2 +-
drivers/media/pci/cx25821/cx25821-alsa.c | 2 +-
drivers/media/pci/cx88/cx88-alsa.c | 2 +-
drivers/media/pci/saa7134/saa7134-alsa.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/pci/cx23885/cx23885-alsa.c b/drivers/media/pci/cx23885/cx23885-alsa.c
index df44ed7..3f366e4 100644
--- a/drivers/media/pci/cx23885/cx23885-alsa.c
+++ b/drivers/media/pci/cx23885/cx23885-alsa.c
@@ -129,7 +129,7 @@ static int cx23885_alsa_dma_unmap(struct cx23885_audio_dev *dev)
if (!buf->sglen)
return 0;
- dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen, PCI_DMA_FROMDEVICE);
+ dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, PCI_DMA_FROMDEVICE);
buf->sglen = 0;
return 0;
}
diff --git a/drivers/media/pci/cx25821/cx25821-alsa.c b/drivers/media/pci/cx25821/cx25821-alsa.c
index 3016164..c40304d 100644
--- a/drivers/media/pci/cx25821/cx25821-alsa.c
+++ b/drivers/media/pci/cx25821/cx25821-alsa.c
@@ -193,7 +193,7 @@ static int cx25821_alsa_dma_unmap(struct cx25821_audio_dev *dev)
if (!buf->sglen)
return 0;
- dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen, PCI_DMA_FROMDEVICE);
+ dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, PCI_DMA_FROMDEVICE);
buf->sglen = 0;
return 0;
}
diff --git a/drivers/media/pci/cx88/cx88-alsa.c b/drivers/media/pci/cx88/cx88-alsa.c
index 7d7acee..3c6fe6c 100644
--- a/drivers/media/pci/cx88/cx88-alsa.c
+++ b/drivers/media/pci/cx88/cx88-alsa.c
@@ -332,7 +332,7 @@ static int cx88_alsa_dma_unmap(struct cx88_audio_dev *dev)
if (!buf->sglen)
return 0;
- dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen,
+ dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages,
PCI_DMA_FROMDEVICE);
buf->sglen = 0;
return 0;
diff --git a/drivers/media/pci/saa7134/saa7134-alsa.c b/drivers/media/pci/saa7134/saa7134-alsa.c
index 544ca57..398c47f 100644
--- a/drivers/media/pci/saa7134/saa7134-alsa.c
+++ b/drivers/media/pci/saa7134/saa7134-alsa.c
@@ -313,7 +313,7 @@ static int saa7134_alsa_dma_unmap(struct saa7134_dev *dev)
if (!dma->sglen)
return 0;
- dma_unmap_sg(&dev->pci->dev, dma->sglist, dma->sglen, PCI_DMA_FROMDEVICE);
+ dma_unmap_sg(&dev->pci->dev, dma->sglist, dma->nr_pages, PCI_DMA_FROMDEVICE);
dma->sglen = 0;
return 0;
}
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist wrappers
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (35 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 37/38] media: pci: fix common ALSA DMA-mapping related codes Marek Szyprowski
@ 2020-05-12 9:00 ` Marek Szyprowski
2020-05-12 17:52 ` Ruhl, Michael J
2020-05-12 12:18 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Christoph Hellwig
2020-05-13 13:23 ` Robin Murphy
38 siblings, 1 reply; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 9:00 UTC (permalink / raw)
To: dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: Pawel Osciak, Bartlomiej Zolnierkiewicz, David Airlie,
linux-media, Daniel Vetter, Hans Verkuil, Mauro Carvalho Chehab,
Robin Murphy, Christoph Hellwig, linux-arm-kernel,
Marek Szyprowski
Use recently introduced common wrappers operating directly on the struct
sg_table objects and scatterlist page iterators to make the code a bit
more compact, robust, easier to follow and copy/paste safe.
No functional change, because the code already properly did all the
scaterlist related calls.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread:
https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
---
.../media/common/videobuf2/videobuf2-dma-contig.c | 41 ++++++++++------------
drivers/media/common/videobuf2/videobuf2-dma-sg.c | 32 +++++++----------
drivers/media/common/videobuf2/videobuf2-vmalloc.c | 12 +++----
3 files changed, 34 insertions(+), 51 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index d3a3ee5..bf31a9d 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
@@ -48,16 +48,15 @@ struct vb2_dc_buf {
static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt)
{
- struct scatterlist *s;
dma_addr_t expected = sg_dma_address(sgt->sgl);
- unsigned int i;
+ struct sg_dma_page_iter dma_iter;
unsigned long size = 0;
- for_each_sg(sgt->sgl, s, sgt->nents, i) {
- if (sg_dma_address(s) != expected)
+ for_each_sgtable_dma_page(sgt, &dma_iter, 0) {
+ if (sg_page_iter_dma_address(&dma_iter) != expected)
break;
- expected = sg_dma_address(s) + sg_dma_len(s);
- size += sg_dma_len(s);
+ expected += PAGE_SIZE;
+ size += PAGE_SIZE;
}
return size;
}
@@ -99,8 +98,7 @@ static void vb2_dc_prepare(void *buf_priv)
if (!sgt || buf->db_attach)
return;
- dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir);
+ dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir);
}
static void vb2_dc_finish(void *buf_priv)
@@ -112,7 +110,7 @@ static void vb2_dc_finish(void *buf_priv)
if (!sgt || buf->db_attach)
return;
- dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
+ dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir);
}
/*********************************************/
@@ -273,8 +271,8 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf,
* memory locations do not require any explicit cache
* maintenance prior or after being used by the device.
*/
- dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents,
- attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
+ DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sgt);
kfree(attach);
db_attach->priv = NULL;
@@ -299,8 +297,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
/* release any previous cache */
if (attach->dma_dir != DMA_NONE) {
- dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents,
- attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
+ DMA_ATTR_SKIP_CPU_SYNC);
attach->dma_dir = DMA_NONE;
}
@@ -308,9 +306,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
* mapping to the client with new direction, no cache sync
* required see comment in vb2_dc_dmabuf_ops_detach()
*/
- sgt->nents = dma_map_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents,
- dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
- if (!sgt->nents) {
+ if (dma_map_sgtable(db_attach->dev, sgt, dma_dir,
+ DMA_ATTR_SKIP_CPU_SYNC)) {
pr_err("failed to map scatterlist\n");
mutex_unlock(lock);
return ERR_PTR(-EIO);
@@ -423,8 +420,8 @@ static void vb2_dc_put_userptr(void *buf_priv)
* No need to sync to CPU, it's already synced to the CPU
* since the finish() memop will have been called before this.
*/
- dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
+ dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir,
+ DMA_ATTR_SKIP_CPU_SYNC);
pages = frame_vector_pages(buf->vec);
/* sgt should exist only if vector contains pages... */
BUG_ON(IS_ERR(pages));
@@ -521,9 +518,8 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr,
* No need to sync to the device, this will happen later when the
* prepare() memop is called.
*/
- sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
- if (sgt->nents <= 0) {
+ if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir,
+ DMA_ATTR_SKIP_CPU_SYNC)) {
pr_err("failed to map scatterlist\n");
ret = -EIO;
goto fail_sgt_init;
@@ -545,8 +541,7 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr,
return buf;
fail_map_sg:
- dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
+ dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
fail_sgt_init:
sg_free_table(sgt);
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index 92072a0..6ddf953 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -142,9 +142,8 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs,
* No need to sync to the device, this will happen later when the
* prepare() memop is called.
*/
- sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
- if (!sgt->nents)
+ if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir,
+ DMA_ATTR_SKIP_CPU_SYNC)) {
goto fail_map;
buf->handler.refcount = &buf->refcount;
@@ -180,8 +179,8 @@ static void vb2_dma_sg_put(void *buf_priv)
if (refcount_dec_and_test(&buf->refcount)) {
dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
buf->num_pages);
- dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
+ dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir,
+ DMA_ATTR_SKIP_CPU_SYNC);
if (buf->vaddr)
vm_unmap_ram(buf->vaddr, buf->num_pages);
sg_free_table(buf->dma_sgt);
@@ -202,8 +201,7 @@ static void vb2_dma_sg_prepare(void *buf_priv)
if (buf->db_attach)
return;
- dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir);
+ dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir);
}
static void vb2_dma_sg_finish(void *buf_priv)
@@ -215,7 +213,7 @@ static void vb2_dma_sg_finish(void *buf_priv)
if (buf->db_attach)
return;
- dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
+ dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir);
}
static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr,
@@ -258,9 +256,8 @@ static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr,
* No need to sync to the device, this will happen later when the
* prepare() memop is called.
*/
- sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
- if (!sgt->nents)
+ if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir,
+ DMA_ATTR_SKIP_CPU_SYNC)) {
goto userptr_fail_map;
return buf;
@@ -286,8 +283,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
__func__, buf->num_pages);
- dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir,
- DMA_ATTR_SKIP_CPU_SYNC);
+ dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
if (buf->vaddr)
vm_unmap_ram(buf->vaddr, buf->num_pages);
sg_free_table(buf->dma_sgt);
@@ -410,8 +406,7 @@ static void vb2_dma_sg_dmabuf_ops_detach(struct dma_buf *dbuf,
/* release the scatterlist cache */
if (attach->dma_dir != DMA_NONE)
- dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
- attach->dma_dir);
+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir);
sg_free_table(sgt);
kfree(attach);
db_attach->priv = NULL;
@@ -436,15 +431,12 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map(
/* release any previous cache */
if (attach->dma_dir != DMA_NONE) {
- dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
- attach->dma_dir);
+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir);
attach->dma_dir = DMA_NONE;
}
/* mapping to the client with new direction */
- sgt->nents = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
- dma_dir);
- if (!sgt->nents) {
+ if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) {
pr_err("failed to map scatterlist\n");
mutex_unlock(lock);
return ERR_PTR(-EIO);
diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
index c66fda4..bf5ac63 100644
--- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
+++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
@@ -229,7 +229,7 @@ static int vb2_vmalloc_dmabuf_ops_attach(struct dma_buf *dbuf,
kfree(attach);
return ret;
}
- for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+ for_each_sgtable_sg(sgt, sg, i) {
struct page *page = vmalloc_to_page(vaddr);
if (!page) {
@@ -259,8 +259,7 @@ static void vb2_vmalloc_dmabuf_ops_detach(struct dma_buf *dbuf,
/* release the scatterlist cache */
if (attach->dma_dir != DMA_NONE)
- dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
- attach->dma_dir);
+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, 0);
sg_free_table(sgt);
kfree(attach);
db_attach->priv = NULL;
@@ -285,15 +284,12 @@ static struct sg_table *vb2_vmalloc_dmabuf_ops_map(
/* release any previous cache */
if (attach->dma_dir != DMA_NONE) {
- dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
- attach->dma_dir);
+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, 0);
attach->dma_dir = DMA_NONE;
}
/* mapping to the client with new direction */
- sgt->nents = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
- dma_dir);
- if (!sgt->nents) {
+ if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) {
pr_err("failed to map scatterlist\n");
mutex_unlock(lock);
return ERR_PTR(-EIO);
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 53+ messages in thread* RE: [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist wrappers
2020-05-12 9:00 ` [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist wrappers Marek Szyprowski
@ 2020-05-12 17:52 ` Ruhl, Michael J
2020-05-12 20:33 ` Marek Szyprowski
0 siblings, 1 reply; 53+ messages in thread
From: Ruhl, Michael J @ 2020-05-12 17:52 UTC (permalink / raw)
To: Marek Szyprowski, dri-devel@lists.freedesktop.org,
iommu@lists.linux-foundation.org, linaro-mm-sig@lists.linaro.org,
linux-kernel@vger.kernel.org
Cc: Pawel Osciak, Bartlomiej Zolnierkiewicz, David Airlie,
Hans Verkuil, Mauro Carvalho Chehab, Robin Murphy,
Christoph Hellwig, linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>Marek Szyprowski
>Sent: Tuesday, May 12, 2020 5:01 AM
>To: dri-devel@lists.freedesktop.org; iommu@lists.linux-foundation.org;
>linaro-mm-sig@lists.linaro.org; linux-kernel@vger.kernel.org
>Cc: Pawel Osciak <pawel@osciak.com>; Bartlomiej Zolnierkiewicz
><b.zolnierkie@samsung.com>; David Airlie <airlied@linux.ie>; linux-
>media@vger.kernel.org; Hans Verkuil <hverkuil-cisco@xs4all.nl>; Mauro
>Carvalho Chehab <mchehab@kernel.org>; Robin Murphy
><robin.murphy@arm.com>; Christoph Hellwig <hch@lst.de>; linux-arm-
>kernel@lists.infradead.org; Marek Szyprowski
><m.szyprowski@samsung.com>
>Subject: [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist wrappers
>
>Use recently introduced common wrappers operating directly on the struct
>sg_table objects and scatterlist page iterators to make the code a bit
>more compact, robust, easier to follow and copy/paste safe.
>
>No functional change, because the code already properly did all the
>scaterlist related calls.
>
>Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>---
>For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
>vs. orig_nents misuse' thread:
>https://lore.kernel.org/dri-devel/20200512085710.14688-1-
>m.szyprowski@samsung.com/T/
>---
> .../media/common/videobuf2/videobuf2-dma-contig.c | 41 ++++++++++----
>--------
> drivers/media/common/videobuf2/videobuf2-dma-sg.c | 32 +++++++--------
>--
> drivers/media/common/videobuf2/videobuf2-vmalloc.c | 12 +++----
> 3 files changed, 34 insertions(+), 51 deletions(-)
>
>diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>index d3a3ee5..bf31a9d 100644
>--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>@@ -48,16 +48,15 @@ struct vb2_dc_buf {
>
> static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt)
> {
>- struct scatterlist *s;
> dma_addr_t expected = sg_dma_address(sgt->sgl);
>- unsigned int i;
>+ struct sg_dma_page_iter dma_iter;
> unsigned long size = 0;
>
>- for_each_sg(sgt->sgl, s, sgt->nents, i) {
>- if (sg_dma_address(s) != expected)
>+ for_each_sgtable_dma_page(sgt, &dma_iter, 0) {
>+ if (sg_page_iter_dma_address(&dma_iter) != expected)
> break;
>- expected = sg_dma_address(s) + sg_dma_len(s);
>- size += sg_dma_len(s);
>+ expected += PAGE_SIZE;
>+ size += PAGE_SIZE;
This code in drm_prime_t_contiguous_size and here. I seem to remember seeing
the same pattern in other drivers.
Would it worthwhile to make this a helper as well?
Also, isn't the sg_dma_len() the actual length of the chunk we are looking at?
If its I not PAGE_SIZE (ie. dma chunk is 4 * PAGE_SIZE?), does your loop/calculation still work?
Thanks,
Mike
> }
> return size;
> }
>@@ -99,8 +98,7 @@ static void vb2_dc_prepare(void *buf_priv)
> if (!sgt || buf->db_attach)
> return;
>
>- dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
>- buf->dma_dir);
>+ dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir);
> }
>
> static void vb2_dc_finish(void *buf_priv)
>@@ -112,7 +110,7 @@ static void vb2_dc_finish(void *buf_priv)
> if (!sgt || buf->db_attach)
> return;
>
>- dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf-
>>dma_dir);
>+ dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir);
> }
>
> /*********************************************/
>@@ -273,8 +271,8 @@ static void vb2_dc_dmabuf_ops_detach(struct
>dma_buf *dbuf,
> * memory locations do not require any explicit cache
> * maintenance prior or after being used by the device.
> */
>- dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt-
>>orig_nents,
>- attach->dma_dir,
>DMA_ATTR_SKIP_CPU_SYNC);
>+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
>+ DMA_ATTR_SKIP_CPU_SYNC);
> sg_free_table(sgt);
> kfree(attach);
> db_attach->priv = NULL;
>@@ -299,8 +297,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
>
> /* release any previous cache */
> if (attach->dma_dir != DMA_NONE) {
>- dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt-
>>orig_nents,
>- attach->dma_dir,
>DMA_ATTR_SKIP_CPU_SYNC);
>+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
>+ DMA_ATTR_SKIP_CPU_SYNC);
> attach->dma_dir = DMA_NONE;
> }
>
>@@ -308,9 +306,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
> * mapping to the client with new direction, no cache sync
> * required see comment in vb2_dc_dmabuf_ops_detach()
> */
>- sgt->nents = dma_map_sg_attrs(db_attach->dev, sgt->sgl, sgt-
>>orig_nents,
>- dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
>- if (!sgt->nents) {
>+ if (dma_map_sgtable(db_attach->dev, sgt, dma_dir,
>+ DMA_ATTR_SKIP_CPU_SYNC)) {
> pr_err("failed to map scatterlist\n");
> mutex_unlock(lock);
> return ERR_PTR(-EIO);
>@@ -423,8 +420,8 @@ static void vb2_dc_put_userptr(void *buf_priv)
> * No need to sync to CPU, it's already synced to the CPU
> * since the finish() memop will have been called before this.
> */
>- dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
>- buf->dma_dir,
>DMA_ATTR_SKIP_CPU_SYNC);
>+ dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir,
>+ DMA_ATTR_SKIP_CPU_SYNC);
> pages = frame_vector_pages(buf->vec);
> /* sgt should exist only if vector contains pages... */
> BUG_ON(IS_ERR(pages));
>@@ -521,9 +518,8 @@ static void *vb2_dc_get_userptr(struct device *dev,
>unsigned long vaddr,
> * No need to sync to the device, this will happen later when the
> * prepare() memop is called.
> */
>- sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
>- buf->dma_dir,
>DMA_ATTR_SKIP_CPU_SYNC);
>- if (sgt->nents <= 0) {
>+ if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir,
>+ DMA_ATTR_SKIP_CPU_SYNC)) {
> pr_err("failed to map scatterlist\n");
> ret = -EIO;
> goto fail_sgt_init;
>@@ -545,8 +541,7 @@ static void *vb2_dc_get_userptr(struct device *dev,
>unsigned long vaddr,
> return buf;
>
> fail_map_sg:
>- dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
>- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
>+ dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir,
>DMA_ATTR_SKIP_CPU_SYNC);
>
> fail_sgt_init:
> sg_free_table(sgt);
>diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
>b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
>index 92072a0..6ddf953 100644
>--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
>+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
>@@ -142,9 +142,8 @@ static void *vb2_dma_sg_alloc(struct device *dev,
>unsigned long dma_attrs,
> * No need to sync to the device, this will happen later when the
> * prepare() memop is called.
> */
>- sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
>- buf->dma_dir,
>DMA_ATTR_SKIP_CPU_SYNC);
>- if (!sgt->nents)
>+ if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir,
>+ DMA_ATTR_SKIP_CPU_SYNC)) {
> goto fail_map;
>
> buf->handler.refcount = &buf->refcount;
>@@ -180,8 +179,8 @@ static void vb2_dma_sg_put(void *buf_priv)
> if (refcount_dec_and_test(&buf->refcount)) {
> dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
> buf->num_pages);
>- dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
>- buf->dma_dir,
>DMA_ATTR_SKIP_CPU_SYNC);
>+ dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir,
>+ DMA_ATTR_SKIP_CPU_SYNC);
> if (buf->vaddr)
> vm_unmap_ram(buf->vaddr, buf->num_pages);
> sg_free_table(buf->dma_sgt);
>@@ -202,8 +201,7 @@ static void vb2_dma_sg_prepare(void *buf_priv)
> if (buf->db_attach)
> return;
>
>- dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
>- buf->dma_dir);
>+ dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir);
> }
>
> static void vb2_dma_sg_finish(void *buf_priv)
>@@ -215,7 +213,7 @@ static void vb2_dma_sg_finish(void *buf_priv)
> if (buf->db_attach)
> return;
>
>- dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf-
>>dma_dir);
>+ dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir);
> }
>
> static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long
>vaddr,
>@@ -258,9 +256,8 @@ static void *vb2_dma_sg_get_userptr(struct device
>*dev, unsigned long vaddr,
> * No need to sync to the device, this will happen later when the
> * prepare() memop is called.
> */
>- sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
>- buf->dma_dir,
>DMA_ATTR_SKIP_CPU_SYNC);
>- if (!sgt->nents)
>+ if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir,
>+ DMA_ATTR_SKIP_CPU_SYNC)) {
> goto userptr_fail_map;
>
> return buf;
>@@ -286,8 +283,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
>
> dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
> __func__, buf->num_pages);
>- dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, buf-
>>dma_dir,
>- DMA_ATTR_SKIP_CPU_SYNC);
>+ dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir,
>DMA_ATTR_SKIP_CPU_SYNC);
> if (buf->vaddr)
> vm_unmap_ram(buf->vaddr, buf->num_pages);
> sg_free_table(buf->dma_sgt);
>@@ -410,8 +406,7 @@ static void vb2_dma_sg_dmabuf_ops_detach(struct
>dma_buf *dbuf,
>
> /* release the scatterlist cache */
> if (attach->dma_dir != DMA_NONE)
>- dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
>- attach->dma_dir);
>+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir);
> sg_free_table(sgt);
> kfree(attach);
> db_attach->priv = NULL;
>@@ -436,15 +431,12 @@ static struct sg_table
>*vb2_dma_sg_dmabuf_ops_map(
>
> /* release any previous cache */
> if (attach->dma_dir != DMA_NONE) {
>- dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
>- attach->dma_dir);
>+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir);
> attach->dma_dir = DMA_NONE;
> }
>
> /* mapping to the client with new direction */
>- sgt->nents = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
>- dma_dir);
>- if (!sgt->nents) {
>+ if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) {
> pr_err("failed to map scatterlist\n");
> mutex_unlock(lock);
> return ERR_PTR(-EIO);
>diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
>b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
>index c66fda4..bf5ac63 100644
>--- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
>+++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
>@@ -229,7 +229,7 @@ static int vb2_vmalloc_dmabuf_ops_attach(struct
>dma_buf *dbuf,
> kfree(attach);
> return ret;
> }
>- for_each_sg(sgt->sgl, sg, sgt->nents, i) {
>+ for_each_sgtable_sg(sgt, sg, i) {
> struct page *page = vmalloc_to_page(vaddr);
>
> if (!page) {
>@@ -259,8 +259,7 @@ static void vb2_vmalloc_dmabuf_ops_detach(struct
>dma_buf *dbuf,
>
> /* release the scatterlist cache */
> if (attach->dma_dir != DMA_NONE)
>- dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
>- attach->dma_dir);
>+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
>0);
> sg_free_table(sgt);
> kfree(attach);
> db_attach->priv = NULL;
>@@ -285,15 +284,12 @@ static struct sg_table
>*vb2_vmalloc_dmabuf_ops_map(
>
> /* release any previous cache */
> if (attach->dma_dir != DMA_NONE) {
>- dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
>- attach->dma_dir);
>+ dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
>0);
> attach->dma_dir = DMA_NONE;
> }
>
> /* mapping to the client with new direction */
>- sgt->nents = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
>- dma_dir);
>- if (!sgt->nents) {
>+ if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) {
> pr_err("failed to map scatterlist\n");
> mutex_unlock(lock);
> return ERR_PTR(-EIO);
>--
>1.9.1
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist wrappers
2020-05-12 17:52 ` Ruhl, Michael J
@ 2020-05-12 20:33 ` Marek Szyprowski
2020-05-13 12:01 ` Ruhl, Michael J
0 siblings, 1 reply; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 20:33 UTC (permalink / raw)
To: Ruhl, Michael J, dri-devel@lists.freedesktop.org,
iommu@lists.linux-foundation.org, linaro-mm-sig@lists.linaro.org,
linux-kernel@vger.kernel.org
Cc: Pawel Osciak, Bartlomiej Zolnierkiewicz, David Airlie,
Hans Verkuil, Mauro Carvalho Chehab, Robin Murphy,
Christoph Hellwig, linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
Hi Michael,
On 12.05.2020 19:52, Ruhl, Michael J wrote:
>> -----Original Message-----
>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>> Marek Szyprowski
>> Sent: Tuesday, May 12, 2020 5:01 AM
>> To: dri-devel@lists.freedesktop.org; iommu@lists.linux-foundation.org;
>> linaro-mm-sig@lists.linaro.org; linux-kernel@vger.kernel.org
>> Cc: Pawel Osciak <pawel@osciak.com>; Bartlomiej Zolnierkiewicz
>> <b.zolnierkie@samsung.com>; David Airlie <airlied@linux.ie>; linux-
>> media@vger.kernel.org; Hans Verkuil <hverkuil-cisco@xs4all.nl>; Mauro
>> Carvalho Chehab <mchehab@kernel.org>; Robin Murphy
>> <robin.murphy@arm.com>; Christoph Hellwig <hch@lst.de>; linux-arm-
>> kernel@lists.infradead.org; Marek Szyprowski
>> <m.szyprowski@samsung.com>
>> Subject: [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist wrappers
>>
>> Use recently introduced common wrappers operating directly on the struct
>> sg_table objects and scatterlist page iterators to make the code a bit
>> more compact, robust, easier to follow and copy/paste safe.
>>
>> No functional change, because the code already properly did all the
>> scaterlist related calls.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
>> vs. orig_nents misuse' thread:
>> https://lore.kernel.org/dri-devel/20200512085710.14688-1-
>> m.szyprowski@samsung.com/T/
>> ---
>> .../media/common/videobuf2/videobuf2-dma-contig.c | 41 ++++++++++----
>> --------
>> drivers/media/common/videobuf2/videobuf2-dma-sg.c | 32 +++++++--------
>> --
>> drivers/media/common/videobuf2/videobuf2-vmalloc.c | 12 +++----
>> 3 files changed, 34 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>> b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>> index d3a3ee5..bf31a9d 100644
>> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>> @@ -48,16 +48,15 @@ struct vb2_dc_buf {
>>
>> static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt)
>> {
>> - struct scatterlist *s;
>> dma_addr_t expected = sg_dma_address(sgt->sgl);
>> - unsigned int i;
>> + struct sg_dma_page_iter dma_iter;
>> unsigned long size = 0;
>>
>> - for_each_sg(sgt->sgl, s, sgt->nents, i) {
>> - if (sg_dma_address(s) != expected)
>> + for_each_sgtable_dma_page(sgt, &dma_iter, 0) {
>> + if (sg_page_iter_dma_address(&dma_iter) != expected)
>> break;
>> - expected = sg_dma_address(s) + sg_dma_len(s);
>> - size += sg_dma_len(s);
>> + expected += PAGE_SIZE;
>> + size += PAGE_SIZE;
> This code in drm_prime_t_contiguous_size and here. I seem to remember seeing
> the same pattern in other drivers.
>
> Would it worthwhile to make this a helper as well?
I think I've identified such patterns in all DRM drivers and replaced
with a common helper. So far I have no idea where to put such helper to
make it available for media/videobuf2, so those a few lines are indeed
duplicated here.
> Also, isn't the sg_dma_len() the actual length of the chunk we are looking at?
>
> If its I not PAGE_SIZE (ie. dma chunk is 4 * PAGE_SIZE?), does your loop/calculation still work?
scaterlist page iterators (for_each_sg_page/for_each_sg_dma_page and
their sgtable variants) always operates on PAGE_SIZE units. They
correctly handle larger sg_dma_len().
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread* RE: [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist wrappers
2020-05-12 20:33 ` Marek Szyprowski
@ 2020-05-13 12:01 ` Ruhl, Michael J
0 siblings, 0 replies; 53+ messages in thread
From: Ruhl, Michael J @ 2020-05-13 12:01 UTC (permalink / raw)
To: Marek Szyprowski, dri-devel@lists.freedesktop.org,
iommu@lists.linux-foundation.org, linaro-mm-sig@lists.linaro.org,
linux-kernel@vger.kernel.org
Cc: Pawel Osciak, Bartlomiej Zolnierkiewicz, David Airlie,
Hans Verkuil, Mauro Carvalho Chehab, Robin Murphy,
Christoph Hellwig, linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
>-----Original Message-----
>From: Marek Szyprowski <m.szyprowski@samsung.com>
>Sent: Tuesday, May 12, 2020 4:34 PM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>; dri-
>devel@lists.freedesktop.org; iommu@lists.linux-foundation.org; linaro-mm-
>sig@lists.linaro.org; linux-kernel@vger.kernel.org
>Cc: Pawel Osciak <pawel@osciak.com>; Bartlomiej Zolnierkiewicz
><b.zolnierkie@samsung.com>; David Airlie <airlied@linux.ie>; linux-
>media@vger.kernel.org; Hans Verkuil <hverkuil-cisco@xs4all.nl>; Mauro
>Carvalho Chehab <mchehab@kernel.org>; Robin Murphy
><robin.murphy@arm.com>; Christoph Hellwig <hch@lst.de>; linux-arm-
>kernel@lists.infradead.org
>Subject: Re: [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist
>wrappers
>
>Hi Michael,
>
>On 12.05.2020 19:52, Ruhl, Michael J wrote:
>>> -----Original Message-----
>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>> Marek Szyprowski
>>> Sent: Tuesday, May 12, 2020 5:01 AM
>>> To: dri-devel@lists.freedesktop.org; iommu@lists.linux-foundation.org;
>>> linaro-mm-sig@lists.linaro.org; linux-kernel@vger.kernel.org
>>> Cc: Pawel Osciak <pawel@osciak.com>; Bartlomiej Zolnierkiewicz
>>> <b.zolnierkie@samsung.com>; David Airlie <airlied@linux.ie>; linux-
>>> media@vger.kernel.org; Hans Verkuil <hverkuil-cisco@xs4all.nl>; Mauro
>>> Carvalho Chehab <mchehab@kernel.org>; Robin Murphy
>>> <robin.murphy@arm.com>; Christoph Hellwig <hch@lst.de>; linux-arm-
>>> kernel@lists.infradead.org; Marek Szyprowski
>>> <m.szyprowski@samsung.com>
>>> Subject: [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist
>wrappers
>>>
>>> Use recently introduced common wrappers operating directly on the struct
>>> sg_table objects and scatterlist page iterators to make the code a bit
>>> more compact, robust, easier to follow and copy/paste safe.
>>>
>>> No functional change, because the code already properly did all the
>>> scaterlist related calls.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>> For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
>>> vs. orig_nents misuse' thread:
>>> https://lore.kernel.org/dri-devel/20200512085710.14688-1-
>>> m.szyprowski@samsung.com/T/
>>> ---
>>> .../media/common/videobuf2/videobuf2-dma-contig.c | 41 ++++++++++-
>---
>>> --------
>>> drivers/media/common/videobuf2/videobuf2-dma-sg.c | 32 +++++++-----
>---
>>> --
>>> drivers/media/common/videobuf2/videobuf2-vmalloc.c | 12 +++----
>>> 3 files changed, 34 insertions(+), 51 deletions(-)
>>>
>>> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>>> b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>>> index d3a3ee5..bf31a9d 100644
>>> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>>> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>>> @@ -48,16 +48,15 @@ struct vb2_dc_buf {
>>>
>>> static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt)
>>> {
>>> - struct scatterlist *s;
>>> dma_addr_t expected = sg_dma_address(sgt->sgl);
>>> - unsigned int i;
>>> + struct sg_dma_page_iter dma_iter;
>>> unsigned long size = 0;
>>>
>>> - for_each_sg(sgt->sgl, s, sgt->nents, i) {
>>> - if (sg_dma_address(s) != expected)
>>> + for_each_sgtable_dma_page(sgt, &dma_iter, 0) {
>>> + if (sg_page_iter_dma_address(&dma_iter) != expected)
>>> break;
>>> - expected = sg_dma_address(s) + sg_dma_len(s);
>>> - size += sg_dma_len(s);
>>> + expected += PAGE_SIZE;
>>> + size += PAGE_SIZE;
>> This code in drm_prime_t_contiguous_size and here. I seem to remember
>seeing
>> the same pattern in other drivers.
>>
>> Would it worthwhile to make this a helper as well?
>I think I've identified such patterns in all DRM drivers and replaced
>with a common helper. So far I have no idea where to put such helper to
>make it available for media/videobuf2, so those a few lines are indeed
>duplicated here.
I was thinking of drivers outside of DRM/media. Specifically RDMA.
However, looking at that code, I see that my memory was a little off.
It is working with continuous pages, but not finding the size.
>> Also, isn't the sg_dma_len() the actual length of the chunk we are looking
>at?
>>
>> If its I not PAGE_SIZE (ie. dma chunk is 4 * PAGE_SIZE?), does your
>loop/calculation still work?
>
>scaterlist page iterators (for_each_sg_page/for_each_sg_dma_page and
>their sgtable variants) always operates on PAGE_SIZE units. They
>correctly handle larger sg_dma_len().
Ahh, ok, I see.
Thank you!
Mike
>
>Best regards
>--
>Marek Szyprowski, PhD
>Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (36 preceding siblings ...)
2020-05-12 9:00 ` [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist wrappers Marek Szyprowski
@ 2020-05-12 12:18 ` Christoph Hellwig
2020-05-12 13:00 ` Marek Szyprowski
2020-05-13 13:23 ` Robin Murphy
38 siblings, 1 reply; 53+ messages in thread
From: Christoph Hellwig @ 2020-05-12 12:18 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Bartlomiej Zolnierkiewicz, David Airlie, linux-kernel, dri-devel,
linaro-mm-sig, iommu, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel
On Tue, May 12, 2020 at 11:00:21AM +0200, Marek Szyprowski wrote:
> struct sg_table is a common structure used for describing a memory
> buffer. It consists of a scatterlist with memory pages and DMA addresses
> (sgl entry), as well as the number of scatterlist entries: CPU pages
> (orig_nents entry) and DMA mapped pages (nents entry).
>
> It turned out that it was a common mistake to misuse nents and orig_nents
> entries, calling DMA-mapping functions with a wrong number of entries or
> ignoring the number of mapped entries returned by the dma_map_sg
> function.
>
> To avoid such issues, lets introduce a common wrappers operating directly
> on the struct sg_table objects, which take care of the proper use of
> the nents and orig_nents entries.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
> vs. orig_nents misuse' thread:
> https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
> ---
> include/linux/dma-mapping.h | 79 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index b43116a..88f01cc 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -609,6 +609,85 @@ static inline void dma_sync_single_range_for_device(struct device *dev,
> return dma_sync_single_for_device(dev, addr + offset, size, dir);
> }
>
> +/**
> + * dma_map_sgtable - Map the given buffer for the DMA operations
> + * @dev: The device to perform a DMA operation
> + * @sgt: The sg_table object describing the buffer
> + * @dir: DMA direction
> + * @attrs: Optional DMA attributes for the map operation
> + *
> + * Maps a buffer described by a scatterlist stored in the given sg_table
> + * object for the @dir DMA operation by the @dev device. After success
> + * the ownership for the buffer is transferred to the DMA domain. One has
> + * to call dma_sync_sgtable_for_cpu() or dma_unmap_sgtable() to move the
> + * ownership of the buffer back to the CPU domain before touching the
> + * buffer by the CPU.
> + * Returns 0 on success or -EINVAL on error during mapping the buffer.
> + */
> +static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
> + enum dma_data_direction dir, unsigned long attrs)
> +{
> + int n = dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
> +
> + if (n > 0) {
> + sgt->nents = n;
> + return 0;
> + }
> + return -EINVAL;
Nit: code tend to be a tad easier to read if the the exceptional
condition is inside the branch block, so:
if (n <= 0)
return -EINVAL;
sgt->nents = n;
return 0;
Otherwise this looks good to me:
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects
2020-05-12 12:18 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Christoph Hellwig
@ 2020-05-12 13:00 ` Marek Szyprowski
2020-05-12 13:09 ` Christoph Hellwig
0 siblings, 1 reply; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 13:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Bartlomiej Zolnierkiewicz, David Airlie, linux-kernel, dri-devel,
linaro-mm-sig, iommu, Daniel Vetter, Robin Murphy,
linux-arm-kernel
Hi Christoph,
On 12.05.2020 14:18, Christoph Hellwig wrote:
> On Tue, May 12, 2020 at 11:00:21AM +0200, Marek Szyprowski wrote:
>> struct sg_table is a common structure used for describing a memory
>> buffer. It consists of a scatterlist with memory pages and DMA addresses
>> (sgl entry), as well as the number of scatterlist entries: CPU pages
>> (orig_nents entry) and DMA mapped pages (nents entry).
>>
>> It turned out that it was a common mistake to misuse nents and orig_nents
>> entries, calling DMA-mapping functions with a wrong number of entries or
>> ignoring the number of mapped entries returned by the dma_map_sg
>> function.
>>
>> To avoid such issues, lets introduce a common wrappers operating directly
>> on the struct sg_table objects, which take care of the proper use of
>> the nents and orig_nents entries.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
>> vs. orig_nents misuse' thread:
>> https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
>> ---
>> include/linux/dma-mapping.h | 79 +++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 79 insertions(+)
>>
>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>> index b43116a..88f01cc 100644
>> --- a/include/linux/dma-mapping.h
>> +++ b/include/linux/dma-mapping.h
>> @@ -609,6 +609,85 @@ static inline void dma_sync_single_range_for_device(struct device *dev,
>> return dma_sync_single_for_device(dev, addr + offset, size, dir);
>> }
>>
>> +/**
>> + * dma_map_sgtable - Map the given buffer for the DMA operations
>> + * @dev: The device to perform a DMA operation
>> + * @sgt: The sg_table object describing the buffer
>> + * @dir: DMA direction
>> + * @attrs: Optional DMA attributes for the map operation
>> + *
>> + * Maps a buffer described by a scatterlist stored in the given sg_table
>> + * object for the @dir DMA operation by the @dev device. After success
>> + * the ownership for the buffer is transferred to the DMA domain. One has
>> + * to call dma_sync_sgtable_for_cpu() or dma_unmap_sgtable() to move the
>> + * ownership of the buffer back to the CPU domain before touching the
>> + * buffer by the CPU.
>> + * Returns 0 on success or -EINVAL on error during mapping the buffer.
>> + */
>> +static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
>> + enum dma_data_direction dir, unsigned long attrs)
>> +{
>> + int n = dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
>> +
>> + if (n > 0) {
>> + sgt->nents = n;
>> + return 0;
>> + }
>> + return -EINVAL;
> Nit: code tend to be a tad easier to read if the the exceptional
> condition is inside the branch block, so:
>
> if (n <= 0)
> return -EINVAL;
> sgt->nents = n;
> return 0;
>
Indeed this version looks much better. I will resend it in a few minutes.
> Otherwise this looks good to me:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects
2020-05-12 13:00 ` Marek Szyprowski
@ 2020-05-12 13:09 ` Christoph Hellwig
2020-05-12 13:19 ` Marek Szyprowski
0 siblings, 1 reply; 53+ messages in thread
From: Christoph Hellwig @ 2020-05-12 13:09 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Bartlomiej Zolnierkiewicz, David Airlie, linux-kernel, dri-devel,
linaro-mm-sig, iommu, Daniel Vetter, Robin Murphy,
Christoph Hellwig, linux-arm-kernel
On Tue, May 12, 2020 at 03:00:31PM +0200, Marek Szyprowski wrote:
> > if (n <= 0)
> > return -EINVAL;
> > sgt->nents = n;
> > return 0;
> >
> Indeed this version looks much better. I will resend it in a few minutes.
I could also just fix it up when applying the patch.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects
2020-05-12 13:09 ` Christoph Hellwig
@ 2020-05-12 13:19 ` Marek Szyprowski
0 siblings, 0 replies; 53+ messages in thread
From: Marek Szyprowski @ 2020-05-12 13:19 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Bartlomiej Zolnierkiewicz, David Airlie, linux-kernel, dri-devel,
linaro-mm-sig, iommu, Daniel Vetter, Robin Murphy,
linux-arm-kernel
Hi Christoph,
On 12.05.2020 15:09, Christoph Hellwig wrote:
> On Tue, May 12, 2020 at 03:00:31PM +0200, Marek Szyprowski wrote:
>>> if (n <= 0)
>>> return -EINVAL;
>>> sgt->nents = n;
>>> return 0;
>>>
>> Indeed this version looks much better. I will resend it in a few minutes.
> I could also just fix it up when applying the patch.
Fine for me.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
` (37 preceding siblings ...)
2020-05-12 12:18 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Christoph Hellwig
@ 2020-05-13 13:23 ` Robin Murphy
38 siblings, 0 replies; 53+ messages in thread
From: Robin Murphy @ 2020-05-13 13:23 UTC (permalink / raw)
To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel
Cc: David Airlie, Daniel Vetter, Christoph Hellwig, linux-arm-kernel,
Bartlomiej Zolnierkiewicz
On 2020-05-12 10:00 am, Marek Szyprowski wrote:
> struct sg_table is a common structure used for describing a memory
> buffer. It consists of a scatterlist with memory pages and DMA addresses
> (sgl entry), as well as the number of scatterlist entries: CPU pages
> (orig_nents entry) and DMA mapped pages (nents entry).
>
> It turned out that it was a common mistake to misuse nents and orig_nents
> entries, calling DMA-mapping functions with a wrong number of entries or
> ignoring the number of mapped entries returned by the dma_map_sg
> function.
>
> To avoid such issues, lets introduce a common wrappers operating directly
Nit: "let's"
> on the struct sg_table objects, which take care of the proper use of
> the nents and orig_nents entries.
A few more documentation nitpicks below, but either way the
implementation itself (modulo Christoph's fixup) looks good;
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
> vs. orig_nents misuse' thread:
> https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
> ---
> include/linux/dma-mapping.h | 79 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index b43116a..88f01cc 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -609,6 +609,85 @@ static inline void dma_sync_single_range_for_device(struct device *dev,
> return dma_sync_single_for_device(dev, addr + offset, size, dir);
> }
>
> +/**
> + * dma_map_sgtable - Map the given buffer for the DMA operations
Either "for DMA operations", "for the DMA operation", or "for a DMA
operation", depending on the exact context. Or at that point, perhaps
just "for DMA".
> + * @dev: The device to perform a DMA operation
That doesn't quite parse, maybe "the device performing the DMA
operation", or "the device for which to perform the DMA operation",
depending on whether "DMA operation" means the mapping or the actual
hardware access?
> + * @sgt: The sg_table object describing the buffer
> + * @dir: DMA direction
> + * @attrs: Optional DMA attributes for the map operation
> + *
> + * Maps a buffer described by a scatterlist stored in the given sg_table
> + * object for the @dir DMA operation by the @dev device. After success
> + * the ownership for the buffer is transferred to the DMA domain. One has
> + * to call dma_sync_sgtable_for_cpu() or dma_unmap_sgtable() to move the
> + * ownership of the buffer back to the CPU domain before touching the
> + * buffer by the CPU.
> + * Returns 0 on success or -EINVAL on error during mapping the buffer.
Maybe make that a proper "Return:" section?
> + */
> +static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
> + enum dma_data_direction dir, unsigned long attrs)
> +{
> + int n = dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
> +
> + if (n > 0) {
> + sgt->nents = n;
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +/**
> + * dma_unmap_sgtable - Unmap the given buffer for the DMA operations
> + * @dev: The device to perform a DMA operation
Same two points as before.
> + * @sgt: The sg_table object describing the buffer
> + * @dir: DMA direction
> + * @attrs: Optional DMA attributes for the map operation
Presumably "the unmap operation", although it *is* true that some
attributes are expected to match those originally passed to
dma_map_sgtable()... not sure if kerneldoc can can stretch to that level
of detail concisely ;)
> + *
> + * Unmaps a buffer described by a scatterlist stored in the given sg_table
> + * object for the @dir DMA operation by the @dev device. After this function
> + * the ownership of the buffer is transferred back to the CPU domain.
> + */
> +static inline void dma_unmap_sgtable(struct device *dev, struct sg_table *sgt,
> + enum dma_data_direction dir, unsigned long attrs)
> +{
> + dma_unmap_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
> +}
> +
> +/**
> + * dma_sync_sgtable_for_cpu - Synchronize the given buffer for the CPU access
s/the CPU/CPU/
> + * @dev: The device to perform a DMA operation
As before.
> + * @sgt: The sg_table object describing the buffer
> + * @dir: DMA direction
> + *
> + * Performs the needed cache synchronization and moves the ownership of the
> + * buffer back to the CPU domain, so it is safe to perform any access to it
> + * by the CPU. Before doing any further DMA operations, one has to transfer
> + * the ownership of the buffer back to the DMA domain by calling the
> + * dma_sync_sgtable_for_device().
> + */
> +static inline void dma_sync_sgtable_for_cpu(struct device *dev,
> + struct sg_table *sgt, enum dma_data_direction dir)
> +{
> + dma_sync_sg_for_cpu(dev, sgt->sgl, sgt->orig_nents, dir);
> +}
> +
> +/**
> + * dma_sync_sgtable_for_device - Synchronize the given buffer for the DMA
That one doesn't even
> + * @dev: The device to perform a DMA operation
As before.
But of course, many thanks for taking the effort to add such complete
documentation in the first place :)
Cheers,
Robin.
> + * @sgt: The sg_table object describing the buffer
> + * @dir: DMA direction
> + *
> + * Performs the needed cache synchronization and moves the ownership of the
> + * buffer back to the DMA domain, so it is safe to perform the DMA operation.
> + * Once finished, one has to call dma_sync_sgtable_for_cpu() or
> + * dma_unmap_sgtable().
> + */
> +static inline void dma_sync_sgtable_for_device(struct device *dev,
> + struct sg_table *sgt, enum dma_data_direction dir)
> +{
> + dma_sync_sg_for_device(dev, sgt->sgl, sgt->orig_nents, dir);
> +}
> +
> #define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a, s, r, 0)
> #define dma_unmap_single(d, a, s, r) dma_unmap_single_attrs(d, a, s, r, 0)
> #define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 53+ messages in thread