arm64: default dma_ops is set to coherent_dma_ops results into DMA FAILURE

PATCH New Ritesh Harjani 3 participants 8 messages Apr 22, 2014 – Apr 23, 2014 arm64

lore

Share & tools

lore

Ritesh Harjani

author

2014-04-22 12:48 UTC #0 lore raw

Hi Catalin/Will,

This is regarding the default dma_ops that we populate for arm64.

PROBLEM:
Currently in arch/arm64/mm/dma-mapping.c we set dma_ops = &coherent_swiotlb_dma_ops.

The problem with this is, lets say that there is a dma device which has not populated its dev->archdata.dma_ops, then this dma device will get the coherent dma_ops in which we dont do any cache maintainance.

So, if the dma driver do kmalloc, make some changes to the buffer and after dma_map_single gives it to the dma for the transfer, due to no cache maintenance performed, result will be DMA transfer failed.

Earlier noncoherent ops code was not present at all for arm64, so may be we were setting default ops to coherent ops. But now that we have noncoherent ops in place shall we make the default dma_ops to noncoherent (as what arm also does) ?

Please let me know if I am missing something here.

Thanks
Ritesh

Will Deacon

2014-04-22 13:20 UTC #1 lore raw

Quoted 1 line · Ritesh Harjani

Hi Catalin/Will,

Hi Ritesh,

Quoted 16 lines

This is regarding the default dma_ops that we populate for arm64.
PROBLEM:
Currently in arch/arm64/mm/dma-mapping.c we set dma_ops = &coherent_swiotlb_dma_ops.
The problem with this is, lets say that there is a dma device which has not populated its dev->archdata.dma_ops, then this dma device will get the coherent dma_ops in which we dont do any cache maintainance.
So, if the dma driver do kmalloc, make some changes to the buffer and after dma_map_single gives it to the dma for the transfer, due to no cache maintenance performed, result will be DMA transfer failed.

Quoted 4 lines

Earlier noncoherent ops code was not present at all for arm64, so may be we were setting default ops to coherent ops. But now that we have noncoherent ops in place shall we make the default dma_ops to noncoherent (as what arm also does) ?

The problem with changing the default assumption to non-coherent is that existing coherent platforms that rely on the coherent DMA ops being selected without any additional DT properties (e.g. dma-coherent) will break with this change.

This puts us into a nasty situation where we have the opposite policy from arch/arm/ regarding default DMA ops, rendering device-trees potentially incompatible between the two architectures.

I'd be inclined to align the two, but it will break any users relying on the current behaviour (I believe this includes Applied with their X-gene SoC).

Will

Catalin Marinas

maintainer

2014-04-22 14:04 UTC #2 lore raw

Quoted 25 lines · Will Deacon

Quoted 10 lines · Ritesh Harjani
This is regarding the default dma_ops that we populate for arm64.
PROBLEM:
Currently in arch/arm64/mm/dma-mapping.c we set dma_ops = &coherent_swiotlb_dma_ops.
The problem with this is, lets say that there is a dma device which has not populated its dev->archdata.dma_ops, then this dma device will get the coherent dma_ops in which we dont do any cache maintainance.
Quoted 8 lines
So, if the dma driver do kmalloc, make some changes to the buffer and after dma_map_single gives it to the dma for the transfer, due to no cache maintenance performed, result will be DMA transfer failed.
Earlier noncoherent ops code was not present at all for arm64, so may be we were setting default ops to coherent ops. But now that we have noncoherent ops in place shall we make the default dma_ops to noncoherent (as what arm also does) ?
The problem with changing the default assumption to non-coherent is that existing coherent platforms that rely on the coherent DMA ops being selected without any additional DT properties (e.g. dma-coherent) will break with this change.

Quoted 6 lines

This puts us into a nasty situation where we have the opposite policy from arch/arm/ regarding default DMA ops, rendering device-trees potentially incompatible between the two architectures.
I'd be inclined to align the two, but it will break any users relying on the current behaviour (I believe this includes Applied with their X-gene SoC).

If there aren't any DMA-capable X-gene drivers in mainline yet, we could make this change (probably even if they are, though we need input from the Applied guys on how disruptive this is).

But do we have a documented way on describing which SoC is coherent? The longer term plan is for a full description of the system topology but we are not there yet. IIRC Santosh has been looking at this as well but I haven't followed the recent developments.

There are some instances of "dma-coherent" properties described in Documentation/devicetree/, we could use them and just need to find a way in the arm64 to handle this automatically without SoC-specific initialisation (and independent of the driver code).

Signature · 1 line

Catalin

Will Deacon

2014-04-22 15:21 UTC #3 lore raw

Quoted 30 lines · Catalin Marinas

Quoted 29 lines · Will Deacon
Quoted 10 lines · Ritesh Harjani
This is regarding the default dma_ops that we populate for arm64.
PROBLEM:
Currently in arch/arm64/mm/dma-mapping.c we set dma_ops = &coherent_swiotlb_dma_ops.
The problem with this is, lets say that there is a dma device which has not populated its dev->archdata.dma_ops, then this dma device will get the coherent dma_ops in which we dont do any cache maintainance.
Quoted 8 lines
So, if the dma driver do kmalloc, make some changes to the buffer and after dma_map_single gives it to the dma for the transfer, due to no cache maintenance performed, result will be DMA transfer failed.
Earlier noncoherent ops code was not present at all for arm64, so may be we were setting default ops to coherent ops. But now that we have noncoherent ops in place shall we make the default dma_ops to noncoherent (as what arm also does) ?
The problem with changing the default assumption to non-coherent is that existing coherent platforms that rely on the coherent DMA ops being selected without any additional DT properties (e.g. dma-coherent) will break with this change.
This puts us into a nasty situation where we have the opposite policy from arch/arm/ regarding default DMA ops, rendering device-trees potentially incompatible between the two architectures.

Quoted 6 lines

Quoted 2 lines
I'd be inclined to align the two, but it will break any users relying on the current behaviour (I believe this includes Applied with their X-gene SoC).
If there aren't any DMA-capable X-gene drivers in mainline yet, we could make this change (probably even if they are, though we need input from the Applied guys on how disruptive this is).

I'd certainly be supportive of such a change.

Quoted 9 lines

But do we have a documented way on describing which SoC is coherent? The longer term plan is for a full description of the system topology but we are not there yet. IIRC Santosh has been looking at this as well but I haven't followed the recent developments.
There are some instances of "dma-coherent" properties described in Documentation/devicetree/, we could use them and just need to find a way in the arm64 to handle this automatically without SoC-specific initialisation (and independent of the driver code).

I think we can default to non-coherent ops without merging support for the "dma-coherent" properties initially. Then we can ask the Applied folks to get involved with Santosh's thread, which will hopefully result in something usable for both arm and arm64.

Will

Ritesh Harjani

author

2014-04-23 05:29 UTC #4 lore raw

Hi Will/Catalin,

Thanks for explaining this. As you guys said we can default to noncoherent ops for arm64 as well, I am submitting this patch for the same.

Catalin, as you said that there are no current dma-capable driver which assumes coherent ops, so I guess this will not break anything.

Ritesh Harjani (1):
arm64: Make default dma_ops to be noncoherent

 arch/arm64/mm/dma-mapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Signature · 1 line

1.8.1.3

Ritesh Harjani

author

2014-04-23 05:29 UTC #5 lore raw

PATCH 1/1 arm64: Make default dma_ops to be noncoherent

Currently arm64 dma_ops is by default made coherent which makes it opposite in default policy from arm.

Make default dma_ops to be noncoherent (same as arm), as currently there aren't any dma-capable drivers which assumes coherent ops

Signed-off-by Ritesh Harjani <[email protected]>

---
 arch/arm64/mm/dma-mapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/mm/dma-mapping.c +1 -1

index 0ba347e..1f65963 100644
@@ -311,7 +311,7 @@ static int __init swiotlb_late_init(void)
 {
 	size_t swiotlb_size = min(SZ_64M, MAX_ORDER_NR_PAGES << PAGE_SHIFT);
 
-	dma_ops = &coherent_swiotlb_dma_ops;
+	dma_ops = &noncoherent_swiotlb_dma_ops;
 
 	return swiotlb_late_init_with_default_size(swiotlb_size);
 }

Signature · 1 line

1.8.1.3

Will Deacon

2014-04-23 09:05 UTC #6 lore raw

PATCH 1/1 arm64: Make default dma_ops to be noncoherent

Quoted 9 lines · Ritesh Harjani

Currently arm64 dma_ops is by default made coherent which makes it opposite in default policy from arm.
Make default dma_ops to be noncoherent (same as arm), as currently there aren't any dma-capable drivers which assumes coherent ops
Signed-off-by Ritesh Harjani <[email protected]>

Acked-by Will Deacon <[email protected]>

Will

Ritesh Harjani

author

2014-04-23 10:00 UTC #7 lore raw

PATCH 1/1 arm64: Make default dma_ops to be noncoherent

Quoted 5 lines · Will Deacon

Quoted 4 lines · Ritesh Harjani
Currently arm64 dma_ops is by default made coherent which makes it opposite in default policy from arm.

Quoted 7 lines

Quoted 5 lines
Make default dma_ops to be noncoherent (same as arm), as currently there aren't any dma-capable drivers which assumes coherent ops
Signed-off-by Ritesh Harjani <[email protected]>
Acked-by Will Deacon <[email protected]>

Thanks!!

Quoted 2 lines

Will

Ritesh