[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANiDSCtefXKw-xC3bskyggW-BzCmVPj6GGLvO=cCPZHbS1oTDA@mail.gmail.com>
Date: Mon, 9 Nov 2020 15:53:55 +0100
From: Ricardo Ribalda <ribalda@...omium.org>
To: Tomasz Figa <tfiga@...omium.org>
Cc: Christoph Hellwig <hch@....de>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Marek Szyprowski <m.szyprowski@...sung.com>,
"list@....net:IOMMU DRIVERS <iommu@...ts.linux-foundation.org>, Joerg
Roedel <joro@...tes.org>," <iommu@...ts.linux-foundation.org>,
Robin Murphy <robin.murphy@....com>,
Linux Doc Mailing List <linux-doc@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux Media Mailing List <linux-media@...r.kernel.org>
Subject: Re: [PATCH 8/8] WIP: add a dma_alloc_contiguous API
Hi Christoph
I have started now to give a try to your patchset. Sorry for the delay.
For uvc I have prepared this patch:
https://github.com/ribalda/linux/commit/9094fe223fe38f8c8ff21366d893b43cbbdf0113
I have tested successfully in a x86_64 noteboot..., yes I know there
is no change for that platform :).
I am trying to get hold of an arm device that can run the latest
kernel from upstream.
On the meanwhile if you could take a look to the patch to verify that
this the way that you expect the drivers to use your api I would
appreciate it
Thanks
On Wed, Oct 14, 2020 at 3:20 PM Tomasz Figa <tfiga@...omium.org> wrote:
>
> +CC Ricardo who will be looking into using this in the USB stack (UVC
> camera driver).
>
> On Wed, Sep 30, 2020 at 6:09 PM Christoph Hellwig <hch@....de> wrote:
> >
> > Add a new API that returns a virtually non-contigous array of pages
> > and dma address. This API is only implemented for dma-iommu and will
> > not be implemented for non-iommu DMA API instances that have to allocate
> > contiguous memory. It is up to the caller to check if the API is
> > available.
> >
> > The intent is that media drivers can use this API if either:
> >
> > - no kernel mapping or only temporary kernel mappings are required.
> > That is as a better replacement for DMA_ATTR_NO_KERNEL_MAPPING
> > - a kernel mapping is required for cached and DMA mapped pages, but
> > the driver also needs the pages to e.g. map them to userspace.
> > In that sense it is a replacement for some aspects of the recently
> > removed and never fully implemented DMA_ATTR_NON_CONSISTENT
> >
> > Signed-off-by: Christoph Hellwig <hch@....de>
> > ---
> > drivers/iommu/dma-iommu.c | 73 +++++++++++++++++++++++++------------
> > include/linux/dma-mapping.h | 9 +++++
> > kernel/dma/mapping.c | 35 ++++++++++++++++++
> > 3 files changed, 93 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> > index 7922f545cd5eef..158026a856622c 100644
> > --- a/drivers/iommu/dma-iommu.c
> > +++ b/drivers/iommu/dma-iommu.c
> > @@ -565,23 +565,12 @@ static struct page **__iommu_dma_alloc_pages(struct device *dev,
> > return pages;
> > }
> >
> > -/**
> > - * iommu_dma_alloc_remap - Allocate and map a buffer contiguous in IOVA space
> > - * @dev: Device to allocate memory for. Must be a real device
> > - * attached to an iommu_dma_domain
> > - * @size: Size of buffer in bytes
> > - * @dma_handle: Out argument for allocated DMA handle
> > - * @gfp: Allocation flags
> > - * @prot: pgprot_t to use for the remapped mapping
> > - * @attrs: DMA attributes for this allocation
> > - *
> > - * If @size is less than PAGE_SIZE, then a full CPU page will be allocated,
> > +/*
> > + * If size is less than PAGE_SIZE, then a full CPU page will be allocated,
> > * but an IOMMU which supports smaller pages might not map the whole thing.
> > - *
> > - * Return: Mapped virtual address, or NULL on failure.
> > */
> > -static void *iommu_dma_alloc_remap(struct device *dev, size_t size,
> > - dma_addr_t *dma_handle, gfp_t gfp, pgprot_t prot,
> > +static struct page **__iommu_dma_alloc_noncontiguous(struct device *dev,
> > + size_t size, dma_addr_t *dma_handle, gfp_t gfp, pgprot_t prot,
> > unsigned long attrs)
> > {
> > struct iommu_domain *domain = iommu_get_dma_domain(dev);
> > @@ -593,7 +582,6 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size,
> > struct page **pages;
> > struct sg_table sgt;
> > dma_addr_t iova;
> > - void *vaddr;
> >
> > *dma_handle = DMA_MAPPING_ERROR;
> >
> > @@ -636,17 +624,10 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size,
> > < size)
> > goto out_free_sg;
> >
> > - vaddr = dma_common_pages_remap(pages, size, prot,
> > - __builtin_return_address(0));
> > - if (!vaddr)
> > - goto out_unmap;
> > -
> > *dma_handle = iova;
> > sg_free_table(&sgt);
> > - return vaddr;
> > + return pages;
> >
> > -out_unmap:
> > - __iommu_dma_unmap(dev, iova, size);
> > out_free_sg:
> > sg_free_table(&sgt);
> > out_free_iova:
> > @@ -656,6 +637,46 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size,
> > return NULL;
> > }
> >
> > +static void *iommu_dma_alloc_remap(struct device *dev, size_t size,
> > + dma_addr_t *dma_handle, gfp_t gfp, pgprot_t prot,
> > + unsigned long attrs)
> > +{
> > + struct page **pages;
> > + void *vaddr;
> > +
> > + pages = __iommu_dma_alloc_noncontiguous(dev, size, dma_handle, gfp,
> > + prot, attrs);
> > + if (!pages)
> > + return NULL;
> > + vaddr = dma_common_pages_remap(pages, size, prot,
> > + __builtin_return_address(0));
> > + if (!vaddr)
> > + goto out_unmap;
> > + return vaddr;
> > +
> > +out_unmap:
> > + __iommu_dma_unmap(dev, *dma_handle, size);
> > + __iommu_dma_free_pages(pages, PAGE_ALIGN(size) >> PAGE_SHIFT);
> > + return NULL;
> > +}
> > +
> > +#ifdef CONFIG_DMA_REMAP
> > +static struct page **iommu_dma_alloc_noncontiguous(struct device *dev,
> > + size_t size, dma_addr_t *dma_handle, gfp_t gfp,
> > + unsigned long attrs)
> > +{
> > + return __iommu_dma_alloc_noncontiguous(dev, size, dma_handle, gfp,
> > + PAGE_KERNEL, attrs);
> > +}
> > +
> > +static void iommu_dma_free_noncontiguous(struct device *dev, size_t size,
> > + struct page **pages, dma_addr_t dma_handle)
> > +{
> > + __iommu_dma_unmap(dev, dma_handle, size);
> > + __iommu_dma_free_pages(pages, PAGE_ALIGN(size) >> PAGE_SHIFT);
> > +}
> > +#endif
> > +
> > static void iommu_dma_sync_single_for_cpu(struct device *dev,
> > dma_addr_t dma_handle, size_t size, enum dma_data_direction dir)
> > {
> > @@ -1110,6 +1131,10 @@ static const struct dma_map_ops iommu_dma_ops = {
> > .free = iommu_dma_free,
> > .alloc_pages = dma_common_alloc_pages,
> > .free_pages = dma_common_free_pages,
> > +#ifdef CONFIG_DMA_REMAP
> > + .alloc_noncontiguous = iommu_dma_alloc_noncontiguous,
> > + .free_noncontiguous = iommu_dma_free_noncontiguous,
> > +#endif
> > .mmap = iommu_dma_mmap,
> > .get_sgtable = iommu_dma_get_sgtable,
> > .map_page = iommu_dma_map_page,
> > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> > index 4b9b1d64f5ec9e..51bbc32365bb8d 100644
> > --- a/include/linux/dma-mapping.h
> > +++ b/include/linux/dma-mapping.h
> > @@ -74,6 +74,10 @@ struct dma_map_ops {
> > gfp_t gfp);
> > void (*free_pages)(struct device *dev, size_t size, struct page *vaddr,
> > dma_addr_t dma_handle, enum dma_data_direction dir);
> > + struct page **(*alloc_noncontiguous)(struct device *dev, size_t size,
> > + dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs);
> > + void (*free_noncontiguous)(struct device *dev, size_t size,
> > + struct page **pages, dma_addr_t dma_handle);
> > int (*mmap)(struct device *, struct vm_area_struct *,
> > void *, dma_addr_t, size_t,
> > unsigned long attrs);
> > @@ -384,6 +388,11 @@ void *dma_alloc_noncoherent(struct device *dev, size_t size,
> > dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp);
> > void dma_free_noncoherent(struct device *dev, size_t size, void *vaddr,
> > dma_addr_t dma_handle, enum dma_data_direction dir);
> > +bool dma_can_alloc_noncontiguous(struct device *dev);
> > +struct page **dma_alloc_noncontiguous(struct device *dev, size_t size,
> > + dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs);
> > +void dma_free_noncontiguous(struct device *dev, size_t size,
> > + struct page **pages, dma_addr_t dma_handle);
> >
> > static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
> > size_t size, enum dma_data_direction dir, unsigned long attrs)
> > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> > index 06115f59f4ffbf..6d975d1a20dd72 100644
> > --- a/kernel/dma/mapping.c
> > +++ b/kernel/dma/mapping.c
> > @@ -529,6 +529,41 @@ void dma_free_noncoherent(struct device *dev, size_t size, void *vaddr,
> > }
> > EXPORT_SYMBOL_GPL(dma_free_noncoherent);
> >
> > +bool dma_can_alloc_noncontiguous(struct device *dev)
> > +{
> > + const struct dma_map_ops *ops = get_dma_ops(dev);
> > +
> > + return ops && ops->free_noncontiguous;
> > +}
> > +EXPORT_SYMBOL_GPL(dma_can_alloc_noncontiguous);
> > +
> > +struct page **dma_alloc_noncontiguous(struct device *dev, size_t size,
> > + dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
> > +{
> > + const struct dma_map_ops *ops = get_dma_ops(dev);
> > +
> > + if (WARN_ON_ONCE(!dma_can_alloc_noncontiguous(dev)))
> > + return NULL;
> > + if (attrs & ~DMA_ATTR_ALLOC_SINGLE_PAGES) {
> > + dev_warn(dev, "invalid flags (0x%lx) for %s\n",
> > + attrs, __func__);
> > + return NULL;
> > + }
> > + return ops->alloc_noncontiguous(dev, size, dma_handle, gfp, attrs);
> > +}
> > +EXPORT_SYMBOL_GPL(dma_alloc_noncontiguous);
> > +
> > +void dma_free_noncontiguous(struct device *dev, size_t size,
> > + struct page **pages, dma_addr_t dma_handle)
> > +{
> > + const struct dma_map_ops *ops = get_dma_ops(dev);
> > +
> > + if (WARN_ON_ONCE(!dma_can_alloc_noncontiguous(dev)))
> > + return;
> > + ops->free_noncontiguous(dev, size, pages, dma_handle);
> > +}
> > +EXPORT_SYMBOL_GPL(dma_free_noncontiguous);
> > +
> > int dma_supported(struct device *dev, u64 mask)
> > {
> > const struct dma_map_ops *ops = get_dma_ops(dev);
> > --
> > 2.28.0
> >
--
Ricardo Ribalda
Powered by blists - more mailing lists