Re: DMA mapping API(IOMMU) for ARM64?

Re: DMA mapping API(IOMMU) for ARM64?

[Hiroshi Doyu]

Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote @ Tue, 18 Jun 2013 12:32:44 +0200:

> On Tuesday 18 June 2013 11:19:14 Will Deacon wrote:
> > 
> > I've spoken briefly with Catalin about this and it seems like there might be
> > *some* scope for sharing common dma-mapping code between architectures.
> > There doesn't really need to be any architecture-specific details in
> > allocating iova space and the interface to the iommu drivers is
> > well-defined. The arch-specific stuff is bogged down in the non-coherent
> > buffer management, but that doesn't necessarily equate to lots of code.
> > 
> > So, the best bet would be to figure out how to split up what we have under
> > arch/arm/ so that it can be re-used by arm64 and potentially other
> > architectures (powerpc has some simple stuff for coherent mappings and some
> > less-simple stuff for non-coherent buffers).
> 
> I'm sure that other architectures are interested in sharing the code.
> Best move it into the global mm/ directory, at least the version for
> coherent mappings.

I'm guessing that this kind of unification usually takes some time
since we need to factor out the common code from the different
${ARCH}/mm/dma-mapping.c. Also some of ${ARCH} has IOMMU support in
it. So if we want to develop our 64-bit ARM with working DMA mapping
API during this transition, the step to take would be:

1, Copy arch/arm/mm/dma-mapping.c arch/arm64/mm/dma-mapping.c(+swiotlb)
2, Adapt arch/arm64/mm/dma-mapping.c(+swiotlb) to 64bit compatible.
3, Factor out the common code from them into mm/dma-mapping.c

Also I guess that there may be the possibility that some of DMA
mapping API may be added/tweaked to satisfy requirements from multiple
${ARCH}. We have already had some of such modifications which didn't
succeed to be upstreamed yet.

Re: DMA mapping API(IOMMU) for ARM64?

[Catalin Marinas]

On Tue, Jun 18, 2013 at 12:01:37PM +0100, Hiroshi Doyu wrote:
> Arnd Bergmann <arnd@arndb.de> wrote @ Tue, 18 Jun 2013 12:32:44 +0200:
> > On Tuesday 18 June 2013 11:19:14 Will Deacon wrote:
> > > I've spoken briefly with Catalin about this and it seems like there might be
> > > *some* scope for sharing common dma-mapping code between architectures.
> > > There doesn't really need to be any architecture-specific details in
> > > allocating iova space and the interface to the iommu drivers is
> > > well-defined. The arch-specific stuff is bogged down in the non-coherent
> > > buffer management, but that doesn't necessarily equate to lots of code.
> > > 
> > > So, the best bet would be to figure out how to split up what we have under
> > > arch/arm/ so that it can be re-used by arm64 and potentially other
> > > architectures (powerpc has some simple stuff for coherent mappings and some
> > > less-simple stuff for non-coherent buffers).
> > 
> > I'm sure that other architectures are interested in sharing the code.
> > Best move it into the global mm/ directory, at least the version for
> > coherent mappings.
> 
> I'm guessing that this kind of unification usually takes some time
> since we need to factor out the common code from the different
> ${ARCH}/mm/dma-mapping.c. Also some of ${ARCH} has IOMMU support in
> it. So if we want to develop our 64-bit ARM with working DMA mapping
> API during this transition, the step to take would be:
> 
> 1, Copy arch/arm/mm/dma-mapping.c arch/arm64/mm/dma-mapping.c(+swiotlb)
> 2, Adapt arch/arm64/mm/dma-mapping.c(+swiotlb) to 64bit compatible.
> 3, Factor out the common code from them into mm/dma-mapping.c

Or add the relevant code to something like lib/iommu-helper.c, convert
arm64 to use that and we can work on converting arm and other
architectures afterwards.

-- 
Catalin

Re: DMA mapping API(IOMMU) for ARM64?

[Arnd Bergmann]

On Tuesday 18 June 2013, Catalin Marinas wrote:
> > 1, Copy arch/arm/mm/dma-mapping.c arch/arm64/mm/dma-mapping.c(+swiotlb)
> > 2, Adapt arch/arm64/mm/dma-mapping.c(+swiotlb) to 64bit compatible.
> > 3, Factor out the common code from them into mm/dma-mapping.c
> 
> Or add the relevant code to something like lib/iommu-helper.c, convert
> arm64 to use that and we can work on converting arm and other
> architectures afterwards.

Yes, that would work too.

	Arnd

Re: DMA mapping API(IOMMU) for ARM64?

[Hiroshi Doyu]

Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote @ Tue, 18 Jun 2013 16:15:27 +0200:

> On Tuesday 18 June 2013, Catalin Marinas wrote:
> > > 1, Copy arch/arm/mm/dma-mapping.c arch/arm64/mm/dma-mapping.c(+swiotlb)
> > > 2, Adapt arch/arm64/mm/dma-mapping.c(+swiotlb) to 64bit compatible.
> > > 3, Factor out the common code from them into mm/dma-mapping.c
> > 
> > Or add the relevant code to something like lib/iommu-helper.c, convert
> > arm64 to use that and we can work on converting arm and other
> > architectures afterwards.
> 
> Yes, that would work too.

Ok, this seems to be the way to go. Thank you for the guide.

Re: DMA mapping API(IOMMU) for ARM64?

[Marek Szyprowski]

Hello,

On 6/18/2013 2:50 PM, Catalin Marinas wrote:
> On Tue, Jun 18, 2013 at 12:01:37PM +0100, Hiroshi Doyu wrote:
> > Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote @ Tue, 18 Jun 2013 12:32:44 +0200:
> > > On Tuesday 18 June 2013 11:19:14 Will Deacon wrote:
> > > > I've spoken briefly with Catalin about this and it seems like there might be
> > > > *some* scope for sharing common dma-mapping code between architectures.
> > > > There doesn't really need to be any architecture-specific details in
> > > > allocating iova space and the interface to the iommu drivers is
> > > > well-defined. The arch-specific stuff is bogged down in the non-coherent
> > > > buffer management, but that doesn't necessarily equate to lots of code.
> > > >
> > > > So, the best bet would be to figure out how to split up what we have under
> > > > arch/arm/ so that it can be re-used by arm64 and potentially other
> > > > architectures (powerpc has some simple stuff for coherent mappings and some
> > > > less-simple stuff for non-coherent buffers).
> > >
> > > I'm sure that other architectures are interested in sharing the code.
> > > Best move it into the global mm/ directory, at least the version for
> > > coherent mappings.
> >
> > I'm guessing that this kind of unification usually takes some time
> > since we need to factor out the common code from the different
> > ${ARCH}/mm/dma-mapping.c. Also some of ${ARCH} has IOMMU support in
> > it. So if we want to develop our 64-bit ARM with working DMA mapping
> > API during this transition, the step to take would be:
> >
> > 1, Copy arch/arm/mm/dma-mapping.c arch/arm64/mm/dma-mapping.c(+swiotlb)
> > 2, Adapt arch/arm64/mm/dma-mapping.c(+swiotlb) to 64bit compatible.
> > 3, Factor out the common code from them into mm/dma-mapping.c
>
> Or add the relevant code to something like lib/iommu-helper.c, convert
> arm64 to use that and we can work on converting arm and other
> architectures afterwards.

I think that this would be the best way of providing iommu support for 
dma mapping
subsystem(s). I initially put it in arm/mm/dma-mapping.c because it was 
the first
approach of integrating generic IOMMU with DMA-mapping and it was simply 
easier to
have all the code in one place. Now, when second client of such approach 
comes, it
should be much easier to identify which part of the code is common and 
which one is
specific to particular implementation. Feel free to ask any questions if 
you need
help. I don't have access to any ARM64 machine or simulator, so I can 
only review
the code.

Best regards
-- 
Marek Szyprowski
Samsung R&D Institute Poland

Re: DMA mapping API(IOMMU) for ARM64?

[Don Dutile]

On 06/18/2013 08:50 AM, Catalin Marinas wrote:
> On Tue, Jun 18, 2013 at 12:01:37PM +0100, Hiroshi Doyu wrote:
>> Arnd Bergmann<arnd@arndb.de>  wrote @ Tue, 18 Jun 2013 12:32:44 +0200:
>>> On Tuesday 18 June 2013 11:19:14 Will Deacon wrote:
>>>> I've spoken briefly with Catalin about this and it seems like there might be
>>>> *some* scope for sharing common dma-mapping code between architectures.
>>>> There doesn't really need to be any architecture-specific details in
>>>> allocating iova space and the interface to the iommu drivers is
>>>> well-defined. The arch-specific stuff is bogged down in the non-coherent
>>>> buffer management, but that doesn't necessarily equate to lots of code.
>>>>
>>>> So, the best bet would be to figure out how to split up what we have under
>>>> arch/arm/ so that it can be re-used by arm64 and potentially other
>>>> architectures (powerpc has some simple stuff for coherent mappings and some
>>>> less-simple stuff for non-coherent buffers).
>>>
>>> I'm sure that other architectures are interested in sharing the code.
>>> Best move it into the global mm/ directory, at least the version for
>>> coherent mappings.
>>
>> I'm guessing that this kind of unification usually takes some time
>> since we need to factor out the common code from the different
>> ${ARCH}/mm/dma-mapping.c. Also some of ${ARCH} has IOMMU support in
>> it. So if we want to develop our 64-bit ARM with working DMA mapping
>> API during this transition, the step to take would be:
>>
>> 1, Copy arch/arm/mm/dma-mapping.c arch/arm64/mm/dma-mapping.c(+swiotlb)
>> 2, Adapt arch/arm64/mm/dma-mapping.c(+swiotlb) to 64bit compatible.
>> 3, Factor out the common code from them into mm/dma-mapping.c
>
> Or add the relevant code to something like lib/iommu-helper.c, convert
> arm64 to use that and we can work on converting arm and other
> architectures afterwards.
>
why not in current drivers/iommu directory ?

Re: DMA mapping API(IOMMU) for ARM64?

[Catalin Marinas]

On Tue, Jun 18, 2013 at 11:08:15PM +0100, Don Dutile wrote:
> On 06/18/2013 08:50 AM, Catalin Marinas wrote:
> > On Tue, Jun 18, 2013 at 12:01:37PM +0100, Hiroshi Doyu wrote:
> >> Arnd Bergmann<arnd-r2nGTMty4D4@public.gmane.org>  wrote @ Tue, 18 Jun 2013 12:32:44 +0200:
> >>> On Tuesday 18 June 2013 11:19:14 Will Deacon wrote:
> >>>> I've spoken briefly with Catalin about this and it seems like there might be
> >>>> *some* scope for sharing common dma-mapping code between architectures.
> >>>> There doesn't really need to be any architecture-specific details in
> >>>> allocating iova space and the interface to the iommu drivers is
> >>>> well-defined. The arch-specific stuff is bogged down in the non-coherent
> >>>> buffer management, but that doesn't necessarily equate to lots of code.
> >>>>
> >>>> So, the best bet would be to figure out how to split up what we have under
> >>>> arch/arm/ so that it can be re-used by arm64 and potentially other
> >>>> architectures (powerpc has some simple stuff for coherent mappings and some
> >>>> less-simple stuff for non-coherent buffers).
> >>>
> >>> I'm sure that other architectures are interested in sharing the code.
> >>> Best move it into the global mm/ directory, at least the version for
> >>> coherent mappings.
> >>
> >> I'm guessing that this kind of unification usually takes some time
> >> since we need to factor out the common code from the different
> >> ${ARCH}/mm/dma-mapping.c. Also some of ${ARCH} has IOMMU support in
> >> it. So if we want to develop our 64-bit ARM with working DMA mapping
> >> API during this transition, the step to take would be:
> >>
> >> 1, Copy arch/arm/mm/dma-mapping.c arch/arm64/mm/dma-mapping.c(+swiotlb)
> >> 2, Adapt arch/arm64/mm/dma-mapping.c(+swiotlb) to 64bit compatible.
> >> 3, Factor out the common code from them into mm/dma-mapping.c
> >
> > Or add the relevant code to something like lib/iommu-helper.c, convert
> > arm64 to use that and we can work on converting arm and other
> > architectures afterwards.
> >
> why not in current drivers/iommu directory ?

That's not really specific to the iommu driver implementation, they have
a clearly defined interface. For similarity with swiotlb and given that
we already have iommu-helper.c, lib/ looks best to me. Anyway, let's
first see how the code looks like, it can be moved later.

-- 
Catalin

Re: DMA mapping API(IOMMU) for ARM64?

[Arnd Bergmann]

On Tuesday 18 June 2013, Hiroshi Doyu wrote:
> I'm guessing that this kind of unification usually takes some time
> since we need to factor out the common code from the different
> ${ARCH}/mm/dma-mapping.c. Also some of ${ARCH} has IOMMU support in
> it. So if we want to develop our 64-bit ARM with working DMA mapping
> API during this transition, the step to take would be:
> 
> 1, Copy arch/arm/mm/dma-mapping.c arch/arm64/mm/dma-mapping.c(+swiotlb)

Please don't.

> 2, Adapt arch/arm64/mm/dma-mapping.c(+swiotlb) to 64bit compatible.
> 3, Factor out the common code from them into mm/dma-mapping.c

I would suggest moving the ARM dma-mapping.c code to /mm or /lib
right away and then generalizing it there. Whoever wants to add
support for their architecture can modify the code to add whatever
changes they require.

	Arnd