| Message ID | 1602692161-107096-1-git-send-email-jianxin.xiong@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/5] RDMA/umem: Support importing dma-buf as user memory region | expand |
On Wed, Oct 14, 2020 at 09:16:01AM -0700, Jianxin Xiong wrote: > The dma-buf API have been used under the assumption that the sg lists > returned from dma_buf_map_attachment() are fully page aligned. Lots of > stuff can break otherwise all over the place. Clarify this in the > documentation and add a check when DMA API debug is enabled. > > Signed-off-by: Jianxin Xiong <jianxin.xiong@intel.com> lgtm, thanks for creating this and giving it a spin. I'll queue this up in drm-misc-next for 5.11, should show up in linux-next after the merge window is closed. Cheers, Daniel > --- > drivers/dma-buf/dma-buf.c | 21 +++++++++++++++++++++ > include/linux/dma-buf.h | 3 ++- > 2 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 844967f..7309c83 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -851,6 +851,9 @@ void dma_buf_unpin(struct dma_buf_attachment *attach) > * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR > * on error. May return -EINTR if it is interrupted by a signal. > * > + * On success, the DMA addresses and lengths in the returned scatterlist are > + * PAGE_SIZE aligned. > + * > * A mapping must be unmapped by using dma_buf_unmap_attachment(). Note that > * the underlying backing storage is pinned for as long as a mapping exists, > * therefore users/importers should not hold onto a mapping for undue amounts of > @@ -904,6 +907,24 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, > attach->dir = direction; > } > > +#ifdef CONFIG_DMA_API_DEBUG > + { > + struct scatterlist *sg; > + u64 addr; > + int len; > + int i; > + > + for_each_sgtable_dma_sg(sg_table, sg, i) { > + addr = sg_dma_address(sg); > + len = sg_dma_len(sg); > + if (!PAGE_ALIGNED(addr) || !PAGE_ALIGNED(len)) { > + pr_debug("%s: addr %llx or len %x is not page aligned!\n", > + __func__, addr, len); > + } > + } > + } > +#endif /* CONFIG_DMA_API_DEBUG */ > + > return sg_table; > } > EXPORT_SYMBOL_GPL(dma_buf_map_attachment); > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > index a2ca294e..4a5fa70 100644 > --- a/include/linux/dma-buf.h > +++ b/include/linux/dma-buf.h > @@ -145,7 +145,8 @@ struct dma_buf_ops { > * > * A &sg_table scatter list of or the backing storage of the DMA buffer, > * already mapped into the device address space of the &device attached > - * with the provided &dma_buf_attachment. > + * with the provided &dma_buf_attachment. The addresses and lengths in > + * the scatter list are PAGE_SIZE aligned. > * > * On failure, returns a negative error value wrapped into a pointer. > * May also return -EINTR when a signal was received while being > -- > 1.8.3.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Am 15.10.20 um 18:03 schrieb Daniel Vetter: > On Wed, Oct 14, 2020 at 09:16:01AM -0700, Jianxin Xiong wrote: >> The dma-buf API have been used under the assumption that the sg lists >> returned from dma_buf_map_attachment() are fully page aligned. Lots of >> stuff can break otherwise all over the place. Clarify this in the >> documentation and add a check when DMA API debug is enabled. >> >> Signed-off-by: Jianxin Xiong <jianxin.xiong@intel.com> Reviewed-by: Christian König <christian.koenig@amd.com> > lgtm, thanks for creating this and giving it a spin. > > I'll queue this up in drm-misc-next for 5.11, should show up in linux-next > after the merge window is closed. Thanks, I'm currently without landline internet and need to rely on my mobile. Christian. > > Cheers, Daniel > >> --- >> drivers/dma-buf/dma-buf.c | 21 +++++++++++++++++++++ >> include/linux/dma-buf.h | 3 ++- >> 2 files changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c >> index 844967f..7309c83 100644 >> --- a/drivers/dma-buf/dma-buf.c >> +++ b/drivers/dma-buf/dma-buf.c >> @@ -851,6 +851,9 @@ void dma_buf_unpin(struct dma_buf_attachment *attach) >> * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR >> * on error. May return -EINTR if it is interrupted by a signal. >> * >> + * On success, the DMA addresses and lengths in the returned scatterlist are >> + * PAGE_SIZE aligned. >> + * >> * A mapping must be unmapped by using dma_buf_unmap_attachment(). Note that >> * the underlying backing storage is pinned for as long as a mapping exists, >> * therefore users/importers should not hold onto a mapping for undue amounts of >> @@ -904,6 +907,24 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, >> attach->dir = direction; >> } >> >> +#ifdef CONFIG_DMA_API_DEBUG >> + { >> + struct scatterlist *sg; >> + u64 addr; >> + int len; >> + int i; >> + >> + for_each_sgtable_dma_sg(sg_table, sg, i) { >> + addr = sg_dma_address(sg); >> + len = sg_dma_len(sg); >> + if (!PAGE_ALIGNED(addr) || !PAGE_ALIGNED(len)) { >> + pr_debug("%s: addr %llx or len %x is not page aligned!\n", >> + __func__, addr, len); >> + } >> + } >> + } >> +#endif /* CONFIG_DMA_API_DEBUG */ >> + >> return sg_table; >> } >> EXPORT_SYMBOL_GPL(dma_buf_map_attachment); >> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h >> index a2ca294e..4a5fa70 100644 >> --- a/include/linux/dma-buf.h >> +++ b/include/linux/dma-buf.h >> @@ -145,7 +145,8 @@ struct dma_buf_ops { >> * >> * A &sg_table scatter list of or the backing storage of the DMA buffer, >> * already mapped into the device address space of the &device attached >> - * with the provided &dma_buf_attachment. >> + * with the provided &dma_buf_attachment. The addresses and lengths in >> + * the scatter list are PAGE_SIZE aligned. >> * >> * On failure, returns a negative error value wrapped into a pointer. >> * May also return -EINTR when a signal was received while being >> -- >> 1.8.3.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cchristian.koenig%40amd.com%7C58ee8712041e4e742fe408d87123e970%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637383746351346603%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=EwAEVcJa6gboHMEQ6XUymC%2BtjFoWd0wl8YUyzdnV5N8%3D&reserved=0
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 844967f..7309c83 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -851,6 +851,9 @@ void dma_buf_unpin(struct dma_buf_attachment *attach) * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR * on error. May return -EINTR if it is interrupted by a signal. * + * On success, the DMA addresses and lengths in the returned scatterlist are + * PAGE_SIZE aligned. + * * A mapping must be unmapped by using dma_buf_unmap_attachment(). Note that * the underlying backing storage is pinned for as long as a mapping exists, * therefore users/importers should not hold onto a mapping for undue amounts of @@ -904,6 +907,24 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, attach->dir = direction; } +#ifdef CONFIG_DMA_API_DEBUG + { + struct scatterlist *sg; + u64 addr; + int len; + int i; + + for_each_sgtable_dma_sg(sg_table, sg, i) { + addr = sg_dma_address(sg); + len = sg_dma_len(sg); + if (!PAGE_ALIGNED(addr) || !PAGE_ALIGNED(len)) { + pr_debug("%s: addr %llx or len %x is not page aligned!\n", + __func__, addr, len); + } + } + } +#endif /* CONFIG_DMA_API_DEBUG */ + return sg_table; } EXPORT_SYMBOL_GPL(dma_buf_map_attachment); diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index a2ca294e..4a5fa70 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -145,7 +145,8 @@ struct dma_buf_ops { * * A &sg_table scatter list of or the backing storage of the DMA buffer, * already mapped into the device address space of the &device attached - * with the provided &dma_buf_attachment. + * with the provided &dma_buf_attachment. The addresses and lengths in + * the scatter list are PAGE_SIZE aligned. * * On failure, returns a negative error value wrapped into a pointer. * May also return -EINTR when a signal was received while being
The dma-buf API have been used under the assumption that the sg lists returned from dma_buf_map_attachment() are fully page aligned. Lots of stuff can break otherwise all over the place. Clarify this in the documentation and add a check when DMA API debug is enabled. Signed-off-by: Jianxin Xiong <jianxin.xiong@intel.com> --- drivers/dma-buf/dma-buf.c | 21 +++++++++++++++++++++ include/linux/dma-buf.h | 3 ++- 2 files changed, 23 insertions(+), 1 deletion(-)