Hi all,
This series split out second half of my previous series
"[PATCH 0/4] MIPS DMA coherence fixes".
It intends to use dma_default_coherent to determine the default coherency of
devicetree probed devices instead of hardcoding it with Kconfig options.
For some MIPS systems, dma_default_coherent is determined with either
bootloader or hardware registers in platform initilization code, and devicetree
does not explicility specify the coherency of the device, so we need the ability
to change the default coherency of devicetree probed devices.
For other platforms that supports noncoherent, dma_default_coherent is a fixed
value set by arch code. It's defaulted to false for most archs except RISC-V.
Thanks
- Jiaxun
---
v2:
- Add PATCH 1 to help with backporting
- Use Kconfig option to set dma_default_coherent
Jiaxun Yang (5):
of: address: Fix default coherency for MIPS
dma-mapping: Provide a fallback dma_default_coherent
dma-mapping: Provide CONFIG_ARCH_DMA_DEFAULT_COHERENT
riscv: Select ARCH_DMA_DEFAULT_COHERENT
of: address: Always use dma_default_coherent for default coherency
arch/powerpc/Kconfig | 1 -
arch/riscv/Kconfig | 2 +-
drivers/of/Kconfig | 4 ----
drivers/of/address.c | 2 +-
include/linux/dma-map-ops.h | 1 +
kernel/dma/Kconfig | 3 +++
kernel/dma/mapping.c | 6 +++++-
7 files changed, 11 insertions(+), 8 deletions(-)
--
2.37.1 (Apple Git-137.1)
DT-based MIPS doesn't use OF_DMA_DEFAULT_COHERENT, but
might override the system-wide default at runtime.
Use dma_default_coherent to override default coherence for
MIPS.
Signed-off-by: Jiaxun Yang <[email protected]>
---
drivers/of/address.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 4c0b169ef9bf..c105d66a1fa4 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -1105,6 +1105,14 @@ bool of_dma_is_coherent(struct device_node *np)
struct device_node *node;
bool is_coherent = IS_ENABLED(CONFIG_OF_DMA_DEFAULT_COHERENT);
+ /*
+ * DT-based MIPS doesn't use OF_DMA_DEFAULT_COHERENT, but
+ * might override the system-wide default at runtime.
+ */
+#if defined(CONFIG_MIPS) && defined(CONFIG_DMA_NONCOHERENT)
+ is_coherent = dma_default_coherent;
+#endif
+
node = of_node_get(np);
while (node) {
--
2.37.1 (Apple Git-137.1)
dma_default_coherent was decleared unconditionally at kernel/dma/mapping.c
but only decleared when any of non-coherent options is enabled in dma-map-ops.h.
Guard the declaration in mapping.c with non-coherent options and provide
a fallback definition.
Signed-off-by: Jiaxun Yang <[email protected]>
---
include/linux/dma-map-ops.h | 1 +
kernel/dma/mapping.c | 4 ++++
2 files changed, 5 insertions(+)
diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index 41bf4bdb117a..0a1aaf7163b3 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -269,6 +269,7 @@ static inline bool dev_is_dma_coherent(struct device *dev)
return dev->dma_coherent;
}
#else
+#define dma_default_coherent true
static inline bool dev_is_dma_coherent(struct device *dev)
{
return true;
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 68106e3791f6..80f9663ffe26 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -17,7 +17,11 @@
#include "debug.h"
#include "direct.h"
+#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
+ defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
+ defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
bool dma_default_coherent;
+#endif
/*
* Managed DMA API
--
2.37.1 (Apple Git-137.1)
Provide a kconfig option to allow arches to manipulate default
value of dma_default_coherent in Kconfig.
Signed-off-by: Jiaxun Yang <[email protected]>
---
kernel/dma/Kconfig | 3 +++
kernel/dma/mapping.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 56866aaa2ae1..968108fdf9bf 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -76,6 +76,9 @@ config ARCH_HAS_DMA_PREP_COHERENT
config ARCH_HAS_FORCE_DMA_UNENCRYPTED
bool
+config ARCH_DMA_DEFAULT_COHERENT
+ bool
+
config SWIOTLB
bool
select NEED_DMA_MAP_STATE
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 80f9663ffe26..9a4db5cce600 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -20,7 +20,7 @@
#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
-bool dma_default_coherent;
+bool dma_default_coherent = IS_ENABLED(CONFIG_ARCH_DMA_DEFAULT_COHERENT);
#endif
/*
--
2.37.1 (Apple Git-137.1)
For riscv our assumption is unless a device states it is non-coherent,
we take it to be DMA coherent.
Select ARCH_DMA_DEFAULT_COHERENT to ensure dma_default_coherent
is always initialized to true.
Signed-off-by: Jiaxun Yang <[email protected]>
---
arch/riscv/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 1d46a268ce16..b71ce992c0c0 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -233,6 +233,7 @@ config LOCKDEP_SUPPORT
config RISCV_DMA_NONCOHERENT
bool
+ select ARCH_DMA_DEFAULT_COHERENT
select ARCH_HAS_DMA_PREP_COHERENT
select ARCH_HAS_SETUP_DMA_OPS
select ARCH_HAS_SYNC_DMA_FOR_CPU
--
2.37.1 (Apple Git-137.1)
As for now all arches have dma_default_coherent matched with default
DMA coherency for of devices, so there is no need to have a standalone
config option.
Note for PowerPC: CONFIG_OF_DMA_DEFUALT_COHERENT was only selected when
CONFIG_NOT_COHERENT_CACHE is false, in this case dma_default_coherent will
be true, so it still matches present behavior.
Note for RISCV: dma_default_coherent is set to true in this series.
Signed-off-by: Jiaxun Yang <[email protected]>
---
arch/powerpc/Kconfig | 1 -
arch/riscv/Kconfig | 1 -
drivers/of/Kconfig | 4 ----
drivers/of/address.c | 10 +---------
4 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2c9cdf1d8761..c67e5da714f7 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -272,7 +272,6 @@ config PPC
select NEED_PER_CPU_PAGE_FIRST_CHUNK if PPC64
select NEED_SG_DMA_LENGTH
select OF
- select OF_DMA_DEFAULT_COHERENT if !NOT_COHERENT_CACHE
select OF_EARLY_FLATTREE
select OLD_SIGACTION if PPC32
select OLD_SIGSUSPEND
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index b71ce992c0c0..a79663191228 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -119,7 +119,6 @@ config RISCV
select MODULES_USE_ELF_RELA if MODULES
select MODULE_SECTIONS if MODULES
select OF
- select OF_DMA_DEFAULT_COHERENT
select OF_EARLY_FLATTREE
select OF_IRQ
select PCI_DOMAINS_GENERIC if PCI
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 644386833a7b..e40f10bf2ba4 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -102,8 +102,4 @@ config OF_OVERLAY
config OF_NUMA
bool
-config OF_DMA_DEFAULT_COHERENT
- # arches should select this if DMA is coherent by default for OF devices
- bool
-
endif # OF
diff --git a/drivers/of/address.c b/drivers/of/address.c
index c105d66a1fa4..23ade4919853 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -1103,15 +1103,7 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
bool of_dma_is_coherent(struct device_node *np)
{
struct device_node *node;
- bool is_coherent = IS_ENABLED(CONFIG_OF_DMA_DEFAULT_COHERENT);
-
- /*
- * DT-based MIPS doesn't use OF_DMA_DEFAULT_COHERENT, but
- * might override the system-wide default at runtime.
- */
-#if defined(CONFIG_MIPS) && defined(CONFIG_DMA_NONCOHERENT)
- is_coherent = dma_default_coherent;
-#endif
+ bool is_coherent = dma_default_coherent;
node = of_node_get(np);
--
2.37.1 (Apple Git-137.1)
On Thu, Feb 23, 2023 at 11:36:43AM +0000, Jiaxun Yang wrote:
> For riscv our assumption is unless a device states it is non-coherent,
> we take it to be DMA coherent.
>
> Select ARCH_DMA_DEFAULT_COHERENT to ensure dma_default_coherent
> is always initialized to true.
>
> Signed-off-by: Jiaxun Yang <[email protected]>
> ---
> arch/riscv/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 1d46a268ce16..b71ce992c0c0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -233,6 +233,7 @@ config LOCKDEP_SUPPORT
>
> config RISCV_DMA_NONCOHERENT
> bool
> + select ARCH_DMA_DEFAULT_COHERENT
Since we are always coherent by default, I feel like you should put this
in the main "config RISCV" section, where OF_DMA_DEFAULT_COHERENT
currently is, no?
Wouldn't bother respinning for that unless the dma folk have comments
for you.
> select ARCH_HAS_DMA_PREP_COHERENT
> select ARCH_HAS_SETUP_DMA_OPS
> select ARCH_HAS_SYNC_DMA_FOR_CPU
> --
> 2.37.1 (Apple Git-137.1)
>
On Thu, Feb 23, 2023 at 11:36:41AM +0000, Jiaxun Yang wrote:
> dma_default_coherent was decleared unconditionally at kernel/dma/mapping.c
> but only decleared when any of non-coherent options is enabled in dma-map-ops.h.
Overly lone line here.
> }
> #else
> +#define dma_default_coherent true
> static inline bool dev_is_dma_coherent(struct device *dev)
Please keep an empty line before the function declaration.
> +config ARCH_DMA_DEFAULT_COHERENT
> + bool
Please add a comment here explaining when an architecture should
ѕelect this symbol.
> - select OF_DMA_DEFAULT_COHERENT if !NOT_COHERENT_CACHE
Doesn't powerpc need to select CONFIG_ARCH_DMA_DEFAULT_COHERENT now,
or even better should be doing that in the patch adding that
symbol?
In fact I wonder if adding CONFIG_ARCH_DMA_DEFAULT_COHERENT and removing
OF_DMA_DEFAULT_COHERENT should be one patch, as that seems to bring
over the intent a little better I'd say.
> 2023年3月1日 13:06，Christoph Hellwig <[email protected]> 写道：
>
>> - select OF_DMA_DEFAULT_COHERENT if !NOT_COHERENT_CACHE
>
> Doesn't powerpc need to select CONFIG_ARCH_DMA_DEFAULT_COHERENT now,
> or even better should be doing that in the patch adding that
> symbol?
If I read the code correctly for powerpc OF_DMA_DEFAULT_COHERENT is only selected
with !NOT_COHERENT_CACHE, which means non-coherent dma support is disabled….
>
> In fact I wonder if adding CONFIG_ARCH_DMA_DEFAULT_COHERENT and removing
> OF_DMA_DEFAULT_COHERENT should be one patch, as that seems to bring
> over the intent a little better I'd say.
Jiaxun Yang <[email protected]> writes:
>> 2023年3月1日 13:06，Christoph Hellwig <[email protected]> 写道：
>>
>>> - select OF_DMA_DEFAULT_COHERENT if !NOT_COHERENT_CACHE
>>
>> Doesn't powerpc need to select CONFIG_ARCH_DMA_DEFAULT_COHERENT now,
>> or even better should be doing that in the patch adding that
>> symbol?
>
> If I read the code correctly for powerpc OF_DMA_DEFAULT_COHERENT is only selected
> with !NOT_COHERENT_CACHE, which means non-coherent dma support is disabled….
I think you're right, but it's not easy to understand.
powerpc's NOT_COHERENT_CACHE selects:
select ARCH_HAS_DMA_PREP_COHERENT
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select ARCH_HAS_SYNC_DMA_FOR_CPU
Then in your patch 3 you do:
#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
-bool dma_default_coherent;
+bool dma_default_coherent = IS_ENABLED(CONFIG_ARCH_DMA_DEFAULT_COHERENT);
#endif
So for powerpc if NOT_COHERENT_CACHE=n, then none of those ARCH_HAS
symbols are defined, and so CONFIG_ARCH_DMA_DEFAULT_COHERENT is never used.
But like I said it's not very obvious, and it also seems fragile to
future changes.
So it seems it would be more future proof, and self documenting for
powerpc to just have:
select ARCH_DMA_DEFAULT_COHERENT if !NOT_COHERENT_CACHE
cheers
On Thu, 23 Feb 2023 14:20:27 PST (-0800), Conor Dooley wrote:
> On Thu, Feb 23, 2023 at 11:36:43AM +0000, Jiaxun Yang wrote:
>> For riscv our assumption is unless a device states it is non-coherent,
>> we take it to be DMA coherent.
>>
>> Select ARCH_DMA_DEFAULT_COHERENT to ensure dma_default_coherent
>> is always initialized to true.
>>
>> Signed-off-by: Jiaxun Yang <[email protected]>
>> ---
>> arch/riscv/Kconfig | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 1d46a268ce16..b71ce992c0c0 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -233,6 +233,7 @@ config LOCKDEP_SUPPORT
>>
>> config RISCV_DMA_NONCOHERENT
>> bool
>> + select ARCH_DMA_DEFAULT_COHERENT
>
> Since we are always coherent by default, I feel like you should put this
> in the main "config RISCV" section, where OF_DMA_DEFAULT_COHERENT
> currently is, no?
Seems reasonable to me. With that
Reviewed-by: Palmer Dabbelt <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>
as I'm assuming these should all stay together.
Thanks!
>
> Wouldn't bother respinning for that unless the dma folk have comments
> for you.
>
>> select ARCH_HAS_DMA_PREP_COHERENT
>> select ARCH_HAS_SETUP_DMA_OPS
>> select ARCH_HAS_SYNC_DMA_FOR_CPU
>> --
>> 2.37.1 (Apple Git-137.1)
>>