| Message ID | 881ef0bcf9aa971e995fbdd00776c5140a7b5b3d.1730298502.git.leon@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Provide a new two step DMA mapping API | expand |
(nits) On 10/30/24 8:12 AM, Leon Romanovsky wrote: > From: Christoph Hellwig <hch@lst.de> > > Add an explanation of the newly added IOVA-based mapping API. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > --- > Documentation/core-api/dma-api.rst | 70 ++++++++++++++++++++++++++++++ > 1 file changed, 70 insertions(+) > > diff --git a/Documentation/core-api/dma-api.rst b/Documentation/core-api/dma-api.rst > index 8e3cce3d0a23..6095696a65a7 100644 > --- a/Documentation/core-api/dma-api.rst > +++ b/Documentation/core-api/dma-api.rst > @@ -530,6 +530,76 @@ routines, e.g.::: > .... > } > > +Part Ie - IOVA-based DMA mappings > +--------------------------------- > + > +These APIs allow a very efficient mapping when using an IOMMU. They are an > +optional path that requires extra code and are only recommended for drivers > +where DMA mapping performance, or the space usage for storing the DMA addresses > +matter. All the consideration from the previous section apply here as well. considerations > + > +:: > + > + bool dma_iova_try_alloc(struct device *dev, struct dma_iova_state *state, > + phys_addr_t phys, size_t size); > + > +Is used to try to allocate IOVA space for mapping operation. If it returns > +false this API can't be used for the given device and the normal streaming > +DMA mapping API should be used. The ``struct dma_iova_state`` is allocated > +by the driver and must be kept around until unmap time. > + > +:: > + > + static inline bool dma_use_iova(struct dma_iova_state *state) > + > +Can be used by the driver to check if the IOVA-based API is used after a > +call to dma_iova_try_alloc. This can be useful in the unmap path. > + > +:: > + > + int dma_iova_link(struct device *dev, struct dma_iova_state *state, > + phys_addr_t phys, size_t offset, size_t size, > + enum dma_data_direction dir, unsigned long attrs); > + > +Is used to link ranges to the IOVA previously allocated. The start of all > +but the first call to dma_iova_link for a given state must be aligned > +to the DMA merge boundary returned by ``dma_get_merge_boundary())``, and > +the size of all but the last range must be aligned to the DMA merge boundary > +as well. > + > +:: > + > + int dma_iova_sync(struct device *dev, struct dma_iova_state *state, > + size_t offset, size_t size); > + > +Must be called to sync the IOMMU page tables for IOVA-range mapped by one or > +more calls to ``dma_iova_link()``. > + > +For drivers that use a one-shot mapping, all ranges can be unmapped and the > +IOVA freed by calling: > + > +:: > + > + void dma_iova_destroy(struct device *dev, struct dma_iova_state *state, > + enum dma_data_direction dir, unsigned long attrs); > + > +Alternatively drivers can dynamically manage the IOVA space by unmapping > +and mapping individual regions. In that case > + > +:: > + > + void dma_iova_unlink(struct device *dev, struct dma_iova_state *state, > + size_t offset, size_t size, enum dma_data_direction dir, > + unsigned long attrs); > + > +is used to unmap a range previous mapped, and previously > + > +:: > + > + void dma_iova_free(struct device *dev, struct dma_iova_state *state); > + > +is used to free the IOVA space. All regions must have been unmapped using > +``dma_iova_unlink()`` before calling ``dma_iova_free()``. > > Part II - Non-coherent DMA allocations > --------------------------------------
On Wed, Oct 30, 2024 at 06:41:21PM -0700, Randy Dunlap wrote: > (nits) > > On 10/30/24 8:12 AM, Leon Romanovsky wrote: > > From: Christoph Hellwig <hch@lst.de> > > > > Add an explanation of the newly added IOVA-based mapping API. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > --- > > Documentation/core-api/dma-api.rst | 70 ++++++++++++++++++++++++++++++ > > 1 file changed, 70 insertions(+) <...> > > +These APIs allow a very efficient mapping when using an IOMMU. They are an > > +optional path that requires extra code and are only recommended for drivers > > +where DMA mapping performance, or the space usage for storing the DMA addresses > > +matter. All the consideration from the previous section apply here as well. > > considerations <...> > > +is used to unmap a range previous mapped, and > > previously Thanks
Leon Romanovsky <leon@kernel.org> writes: > From: Christoph Hellwig <hch@lst.de> > > Add an explanation of the newly added IOVA-based mapping API. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > --- > Documentation/core-api/dma-api.rst | 70 ++++++++++++++++++++++++++++++ > 1 file changed, 70 insertions(+) > > diff --git a/Documentation/core-api/dma-api.rst b/Documentation/core-api/dma-api.rst > index 8e3cce3d0a23..6095696a65a7 100644 > --- a/Documentation/core-api/dma-api.rst > +++ b/Documentation/core-api/dma-api.rst > @@ -530,6 +530,76 @@ routines, e.g.::: > .... > } > > +Part Ie - IOVA-based DMA mappings > +--------------------------------- > + > +These APIs allow a very efficient mapping when using an IOMMU. They are an > +optional path that requires extra code and are only recommended for drivers > +where DMA mapping performance, or the space usage for storing the DMA addresses > +matter. All the consideration from the previous section apply here as well. > + > +:: > + > + bool dma_iova_try_alloc(struct device *dev, struct dma_iova_state *state, > + phys_addr_t phys, size_t size); > + > +Is used to try to allocate IOVA space for mapping operation. If it returns > +false this API can't be used for the given device and the normal streaming > +DMA mapping API should be used. The ``struct dma_iova_state`` is allocated > +by the driver and must be kept around until unmap time. So, I see that you have nice kernel-doc comments for these; why not just pull them in here with a kernel-doc directive rather than duplicating the information? Thanks, jon
On Fri, Nov 08, 2024 at 12:34:21PM -0700, Jonathan Corbet wrote: > Leon Romanovsky <leon@kernel.org> writes: > > > From: Christoph Hellwig <hch@lst.de> > > > > Add an explanation of the newly added IOVA-based mapping API. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > --- > > Documentation/core-api/dma-api.rst | 70 ++++++++++++++++++++++++++++++ > > 1 file changed, 70 insertions(+) > > > > diff --git a/Documentation/core-api/dma-api.rst b/Documentation/core-api/dma-api.rst > > index 8e3cce3d0a23..6095696a65a7 100644 > > --- a/Documentation/core-api/dma-api.rst > > +++ b/Documentation/core-api/dma-api.rst > > @@ -530,6 +530,76 @@ routines, e.g.::: > > .... > > } > > > > +Part Ie - IOVA-based DMA mappings > > +--------------------------------- > > + > > +These APIs allow a very efficient mapping when using an IOMMU. They are an > > +optional path that requires extra code and are only recommended for drivers > > +where DMA mapping performance, or the space usage for storing the DMA addresses > > +matter. All the consideration from the previous section apply here as well. > > + > > +:: > > + > > + bool dma_iova_try_alloc(struct device *dev, struct dma_iova_state *state, > > + phys_addr_t phys, size_t size); > > + > > +Is used to try to allocate IOVA space for mapping operation. If it returns > > +false this API can't be used for the given device and the normal streaming > > +DMA mapping API should be used. The ``struct dma_iova_state`` is allocated > > +by the driver and must be kept around until unmap time. > > So, I see that you have nice kernel-doc comments for these; why not just > pull them in here with a kernel-doc directive rather than duplicating > the information? Can I you please point me to commit/lore link/documentation with example of such directive and I will do it? Thanks > > Thanks, > > jon
Leon Romanovsky <leon@kernel.org> writes: >> So, I see that you have nice kernel-doc comments for these; why not just >> pull them in here with a kernel-doc directive rather than duplicating >> the information? > > Can I you please point me to commit/lore link/documentation with example > of such directive and I will do it? Documentation/doc-guide/kernel-doc.rst has all the information you need. It could be as simple as replacing your inline descriptions with: .. kernel-doc:: drivers/iommu/dma-iommu.c :export: That will pull in documentation for other, unrelated functions, though; assuming you don't want those, something like: .. kernel-doc:: drivers/iommu/dma-iommu.c :identifiers: dma_iova_try_alloc dma_iova_free ... Then do a docs build and see the nice results you get :) Thanks, jon
On Fri, Nov 08, 2024 at 01:13:27PM -0700, Jonathan Corbet wrote: > Leon Romanovsky <leon@kernel.org> writes: > > >> So, I see that you have nice kernel-doc comments for these; why not just > >> pull them in here with a kernel-doc directive rather than duplicating > >> the information? > > > > Can I you please point me to commit/lore link/documentation with example > > of such directive and I will do it? > > Documentation/doc-guide/kernel-doc.rst has all the information you need. > It could be as simple as replacing your inline descriptions with: > > .. kernel-doc:: drivers/iommu/dma-iommu.c > :export: > > That will pull in documentation for other, unrelated functions, though; > assuming you don't want those, something like: > > .. kernel-doc:: drivers/iommu/dma-iommu.c > :identifiers: dma_iova_try_alloc dma_iova_free ... > > Then do a docs build and see the nice results you get :) Thanks for the explanation, will change it. > > Thanks, > > jon
On Fri, Nov 08, 2024 at 10:27:36PM +0200, Leon Romanovsky wrote: > On Fri, Nov 08, 2024 at 01:13:27PM -0700, Jonathan Corbet wrote: > > Leon Romanovsky <leon@kernel.org> writes: > > > > >> So, I see that you have nice kernel-doc comments for these; why not just > > >> pull them in here with a kernel-doc directive rather than duplicating > > >> the information? > > > > > > Can I you please point me to commit/lore link/documentation with example > > > of such directive and I will do it? > > > > Documentation/doc-guide/kernel-doc.rst has all the information you need. > > It could be as simple as replacing your inline descriptions with: > > > > .. kernel-doc:: drivers/iommu/dma-iommu.c > > :export: > > > > That will pull in documentation for other, unrelated functions, though; > > assuming you don't want those, something like: > > > > .. kernel-doc:: drivers/iommu/dma-iommu.c > > :identifiers: dma_iova_try_alloc dma_iova_free ... > > > > Then do a docs build and see the nice results you get :) > > Thanks for the explanation, will change it. Jonathan, I tried this today and the output (HTML) in the new section looks so different from the rest of dma-api.rst that I lean to leave the current doc implementation as is. Thanks > > > > > Thanks, > > > > jon >
On Sun, Nov 10, 2024 at 12:41:30PM +0200, Leon Romanovsky wrote: > I tried this today and the output (HTML) in the new section looks > so different from the rest of dma-api.rst that I lean to leave > the current doc implementation as is. Yeah. The whole DMA API documentation shows it's age and could use a major revamp, but for now I'd prefer to stick to the way it is done. If we have any volunteers for bringing it up to standards I'd be glad to help with input and review.
On Sun, Nov 10, 2024 at 10:39 PM Christoph Hellwig <hch@lst.de> wrote: > > On Sun, Nov 10, 2024 at 12:41:30PM +0200, Leon Romanovsky wrote: > > I tried this today and the output (HTML) in the new section looks > > so different from the rest of dma-api.rst that I lean to leave > > the current doc implementation as is. > > Yeah. The whole DMA API documentation shows it's age and could use > a major revamp, but for now I'd prefer to stick to the way it is done. > > If we have any volunteers for bringing it up to standards I'd be glad > to help with input and review. Jonathan, if you agree, I can take this up? > >
anish kumar <yesanishhere@gmail.com> writes: > On Sun, Nov 10, 2024 at 10:39 PM Christoph Hellwig <hch@lst.de> wrote: >> >> On Sun, Nov 10, 2024 at 12:41:30PM +0200, Leon Romanovsky wrote: >> > I tried this today and the output (HTML) in the new section looks >> > so different from the rest of dma-api.rst that I lean to leave >> > the current doc implementation as is. >> >> Yeah. The whole DMA API documentation shows it's age and could use >> a major revamp, but for now I'd prefer to stick to the way it is done. >> >> If we have any volunteers for bringing it up to standards I'd be glad >> to help with input and review. > > Jonathan, if you agree, I can take this up? I am happy to see help with the documentation, but agreement from the authors and maintainers of the DMA-mapping documentation is rather more important than agreement from me. jon
diff --git a/Documentation/core-api/dma-api.rst b/Documentation/core-api/dma-api.rst index 8e3cce3d0a23..6095696a65a7 100644 --- a/Documentation/core-api/dma-api.rst +++ b/Documentation/core-api/dma-api.rst @@ -530,6 +530,76 @@ routines, e.g.::: .... } +Part Ie - IOVA-based DMA mappings +--------------------------------- + +These APIs allow a very efficient mapping when using an IOMMU. They are an +optional path that requires extra code and are only recommended for drivers +where DMA mapping performance, or the space usage for storing the DMA addresses +matter. All the consideration from the previous section apply here as well. + +:: + + bool dma_iova_try_alloc(struct device *dev, struct dma_iova_state *state, + phys_addr_t phys, size_t size); + +Is used to try to allocate IOVA space for mapping operation. If it returns +false this API can't be used for the given device and the normal streaming +DMA mapping API should be used. The ``struct dma_iova_state`` is allocated +by the driver and must be kept around until unmap time. + +:: + + static inline bool dma_use_iova(struct dma_iova_state *state) + +Can be used by the driver to check if the IOVA-based API is used after a +call to dma_iova_try_alloc. This can be useful in the unmap path. + +:: + + int dma_iova_link(struct device *dev, struct dma_iova_state *state, + phys_addr_t phys, size_t offset, size_t size, + enum dma_data_direction dir, unsigned long attrs); + +Is used to link ranges to the IOVA previously allocated. The start of all +but the first call to dma_iova_link for a given state must be aligned +to the DMA merge boundary returned by ``dma_get_merge_boundary())``, and +the size of all but the last range must be aligned to the DMA merge boundary +as well. + +:: + + int dma_iova_sync(struct device *dev, struct dma_iova_state *state, + size_t offset, size_t size); + +Must be called to sync the IOMMU page tables for IOVA-range mapped by one or +more calls to ``dma_iova_link()``. + +For drivers that use a one-shot mapping, all ranges can be unmapped and the +IOVA freed by calling: + +:: + + void dma_iova_destroy(struct device *dev, struct dma_iova_state *state, + enum dma_data_direction dir, unsigned long attrs); + +Alternatively drivers can dynamically manage the IOVA space by unmapping +and mapping individual regions. In that case + +:: + + void dma_iova_unlink(struct device *dev, struct dma_iova_state *state, + size_t offset, size_t size, enum dma_data_direction dir, + unsigned long attrs); + +is used to unmap a range previous mapped, and + +:: + + void dma_iova_free(struct device *dev, struct dma_iova_state *state); + +is used to free the IOVA space. All regions must have been unmapped using +``dma_iova_unlink()`` before calling ``dma_iova_free()``. Part II - Non-coherent DMA allocations --------------------------------------