2019-11-13 12:36:53

by Laurentiu Tudor

[permalink] [raw]
Subject: [PATCH v3 0/4] dma-mapping: introduce new dma unmap and sync variants

From: Laurentiu Tudor <[email protected]>

This series introduces a few new dma unmap and sync api variants that,
on top of what the originals do, return the virtual address
corresponding to the input dma address. In order to do that a new dma
map op is added, .get_virt_addr that takes the input dma address and
returns the virtual address backing it up.
The second patch adds an implementation for this new dma map op in the
generic iommu dma glue code and wires it in.
The third patch updates the dpaa2-eth driver to use the new apis.

Context: https://lkml.org/lkml/2019/5/31/684

Changes in v3
* drop useless check for null iommu domain (Robin)
* add dma_can_unmap_by_dma_addr() to check availability of
these new apis (Christoph)
* make apis work with direct dma (Christoph)
* add support for swiotlb (Robin)
* simplify dpaa2_eth driver code by using dma_unmap_single_desc()
instead of dma_unmap_page_desc()

Changes in v2
* use "dma_unmap_*_desc" names (Robin)
* return va/page instead of phys addr (Robin)

Changes since RFC
* completely changed the approach: added unmap and sync variants that
return the phys addr instead of adding a new dma to phys conversion
function

Laurentiu Tudor (4):
dma-mapping: introduce new dma unmap and sync api variants
iommu/dma: wire-up new dma map op .get_virt_addr
swiotlb: make new {unmap,sync}_desc dma apis work with swiotlb
dpaa2_eth: use new unmap and sync dma api variants

drivers/iommu/dma-iommu.c | 13 ++++
.../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 43 +++++-----
.../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 1 -
include/linux/dma-mapping.h | 45 +++++++++++
include/linux/swiotlb.h | 7 ++
kernel/dma/mapping.c | 78 +++++++++++++++++++
kernel/dma/swiotlb.c | 8 ++
7 files changed, 169 insertions(+), 26 deletions(-)

--
2.17.1


2019-11-13 20:14:38

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] dma-mapping: introduce new dma unmap and sync variants

From: Laurentiu Tudor <[email protected]>
Date: Wed, 13 Nov 2019 12:24:17 +0000

> From: Laurentiu Tudor <[email protected]>
>
> This series introduces a few new dma unmap and sync api variants that,
> on top of what the originals do, return the virtual address
> corresponding to the input dma address. In order to do that a new dma
> map op is added, .get_virt_addr that takes the input dma address and
> returns the virtual address backing it up.
> The second patch adds an implementation for this new dma map op in the
> generic iommu dma glue code and wires it in.
> The third patch updates the dpaa2-eth driver to use the new apis.

The driver should store the mapping in it's private software state if
it needs this kind of conversion.

This is huge precendence for this, and there is therefore no need to
add even more complication and methods and burdon to architecture code
for the sake of this.

Thank you.

2019-11-14 11:14:49

by Laurentiu Tudor

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] dma-mapping: introduce new dma unmap and sync variants

On 13.11.2019 22:11, David Miller wrote:
> From: Laurentiu Tudor <[email protected]>
> Date: Wed, 13 Nov 2019 12:24:17 +0000
>
>> From: Laurentiu Tudor <[email protected]>
>>
>> This series introduces a few new dma unmap and sync api variants that,
>> on top of what the originals do, return the virtual address
>> corresponding to the input dma address. In order to do that a new dma
>> map op is added, .get_virt_addr that takes the input dma address and
>> returns the virtual address backing it up.
>> The second patch adds an implementation for this new dma map op in the
>> generic iommu dma glue code and wires it in.
>> The third patch updates the dpaa2-eth driver to use the new apis.
>
> The driver should store the mapping in it's private software state if
> it needs this kind of conversion.

On this hardware there's no way of conveying additional frame
information, such as original va/pa behind the dma address. We have also
pondered on the idea of keeping this in some kind of data structure but
could not find a lock-less solution which obviously would bring
performance to the ground.
I'll let my colleagues maintaining these ethernet drivers to get into
more details, if required.

---
Best Regards, Laurentiu

2019-11-19 14:12:39

by Ioana Ciornei

[permalink] [raw]
Subject: RE: [PATCH v3 0/4] dma-mapping: introduce new dma unmap and sync variants

> Subject: Re: [PATCH v3 0/4] dma-mapping: introduce new dma unmap and sync
> variants
>
> On 13.11.2019 22:11, David Miller wrote:
> > From: Laurentiu Tudor <[email protected]>
> > Date: Wed, 13 Nov 2019 12:24:17 +0000
> >
> >> From: Laurentiu Tudor <[email protected]>
> >>
> >> This series introduces a few new dma unmap and sync api variants
> >> that, on top of what the originals do, return the virtual address
> >> corresponding to the input dma address. In order to do that a new dma
> >> map op is added, .get_virt_addr that takes the input dma address and
> >> returns the virtual address backing it up.
> >> The second patch adds an implementation for this new dma map op in
> >> the generic iommu dma glue code and wires it in.
> >> The third patch updates the dpaa2-eth driver to use the new apis.
> >
> > The driver should store the mapping in it's private software state if
> > it needs this kind of conversion.
>
> On this hardware there's no way of conveying additional frame information,
> such as original va/pa behind the dma address. We have also pondered on the
> idea of keeping this in some kind of data structure but could not find a lock-less
> solution which obviously would bring performance to the ground.
> I'll let my colleagues maintaining these ethernet drivers to get into more details,
> if required.
>

As Laurentiu pointed out before, keeping a mapping in the driver's private data doesn't
seem feasible without a locking mechanism which in turn would be an immense impact
performance wise.

We also feel that instead of hacking our individual drivers we should extend the
core DMA API to also fit this use case, which is exactly what
Laurentiu's patch set is doing.

Our hope is to come to a common understanding of the next steps since this
would unblock some activities currently in the backlog.

Ioana

2019-11-21 07:42:32

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] dma-mapping: introduce new dma unmap and sync variants

On Wed, Nov 13, 2019 at 12:11:32PM -0800, David Miller wrote:
> > This series introduces a few new dma unmap and sync api variants that,
> > on top of what the originals do, return the virtual address
> > corresponding to the input dma address. In order to do that a new dma
> > map op is added, .get_virt_addr that takes the input dma address and
> > returns the virtual address backing it up.
> > The second patch adds an implementation for this new dma map op in the
> > generic iommu dma glue code and wires it in.
> > The third patch updates the dpaa2-eth driver to use the new apis.
>
> The driver should store the mapping in it's private software state if
> it needs this kind of conversion.

I think the problem for this driver (and a few others) is that they
somehow manage to split I/O completions at a different boundary
than submissions. For me with my block I/O background this seems
weird, but I'll need networking folks to double check the theory.

> This is huge precendence for this, and there is therefore no need to
> add even more complication and methods and burdon to architecture code
> for the sake of this.

Unfortunately there are various drivers that abuse iommu_iova_to_phys
to get a struct page to unmap. Two of theose are network drivers
that went in through you (dpaa2 and thunder), additonally the
caam crypto driver (which I think is the same SOC family as dpaa,
but I'm not sure) and the AMD GPU driver.

We also have drivers that just don't unmap and this don't work with
iommus or dma-debug (IBM EMAC another net driver).

That being said I hate these new API, but I still think they are less
bad than these IOMMU API abuses people do now. If experienced
networking folks know a way to get rid of both I'm all for it.

2019-12-02 15:00:20

by Madalin-cristian Bucur

[permalink] [raw]
Subject: RE: [PATCH v3 0/4] dma-mapping: introduce new dma unmap and sync variants

> -----Original Message-----
> From: Christoph Hellwig <[email protected]>
> To: David Miller <[email protected]>
> Subject: Re: [PATCH v3 0/4] dma-mapping: introduce new dma unmap and sync
> variants
>
> On Wed, Nov 13, 2019 at 12:11:32PM -0800, David Miller wrote:
> > > This series introduces a few new dma unmap and sync api variants
> that,
> > > on top of what the originals do, return the virtual address
> > > corresponding to the input dma address. In order to do that a new dma
> > > map op is added, .get_virt_addr that takes the input dma address and
> > > returns the virtual address backing it up.
> > > The second patch adds an implementation for this new dma map op in
> the
> > > generic iommu dma glue code and wires it in.
> > > The third patch updates the dpaa2-eth driver to use the new apis.
> >
> > The driver should store the mapping in it's private software state if
> > it needs this kind of conversion.
>
> I think the problem for this driver (and a few others) is that they
> somehow manage to split I/O completions at a different boundary
> than submissions. For me with my block I/O background this seems
> weird, but I'll need networking folks to double check the theory.
>
> > This is huge precendence for this, and there is therefore no need to
> > add even more complication and methods and burdon to architecture code
> > for the sake of this.
>
> Unfortunately there are various drivers that abuse iommu_iova_to_phys
> to get a struct page to unmap. Two of theose are network drivers
> that went in through you (dpaa2 and thunder), additonally the
> caam crypto driver (which I think is the same SOC family as dpaa,
> but I'm not sure) and the AMD GPU driver.
>
> We also have drivers that just don't unmap and this don't work with
> iommus or dma-debug (IBM EMAC another net driver).
>
> That being said I hate these new API, but I still think they are less
> bad than these IOMMU API abuses people do now. If experienced
> networking folks know a way to get rid of both I'm all for it.

Hi,

will this API be included during the v5.5 kernel development cycle or is
there an alternative solution?

Thank you,
Madalin