| Message ID | 20231009074121.219686-2-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/6] dma-direct: add depdenencies to CONFIG_DMA_GLOBAL_POOL | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | warning | Series does not have a cover letter |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 9 this patch: 9 |
| netdev/cc_maintainers | success | CCed 4 of 4 maintainers |
| netdev/build_clang | success | Errors and warnings before: 9 this patch: 9 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 9 this patch: 9 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
Hi Christoph, On Mon, Oct 9, 2023 at 9:41 AM Christoph Hellwig <hch@lst.de> wrote: > CONFIG_DMA_GLOBAL_POOL can't be combined with other dma-coherent > allocators. Add dependencies to Kconfig to document this, and make > kconfig complain about unment dependencies if someone tries. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Thanks for your patch! > --- a/kernel/dma/Kconfig > +++ b/kernel/dma/Kconfig > @@ -135,6 +135,8 @@ config DMA_COHERENT_POOL > > config DMA_GLOBAL_POOL > select DMA_DECLARE_COHERENT > + depends on !ARCH_HAS_DMA_SET_UNCACHED > + depends on !DMA_DIRECT_REMAP > bool > > config DMA_DIRECT_REMAP riscv defconfig + CONFIG_NONPORTABLE=y + CONFIG_ARCH_R9A07G043=y: WARNING: unmet direct dependencies detected for DMA_GLOBAL_POOL Depends on [n]: !ARCH_HAS_DMA_SET_UNCACHED [=n] && !DMA_DIRECT_REMAP [=y] Selected by [y]: - ARCH_R9A07G043 [=y] && SOC_RENESAS [=y] && RISCV [=y] && NONPORTABLE [=y] Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Mon, Oct 09, 2023 at 10:43:57AM +0200, Geert Uytterhoeven wrote: > > config DMA_DIRECT_REMAP > > riscv defconfig + CONFIG_NONPORTABLE=y + CONFIG_ARCH_R9A07G043=y: > > WARNING: unmet direct dependencies detected for DMA_GLOBAL_POOL > Depends on [n]: !ARCH_HAS_DMA_SET_UNCACHED [=n] && !DMA_DIRECT_REMAP [=y] > Selected by [y]: > - ARCH_R9A07G043 [=y] && SOC_RENESAS [=y] && RISCV [=y] && NONPORTABLE [=y] And that's exactly what this patch is supposed to show. RISCV must not select DMA_DIRECT_REMAP at the same time as DMA_GLOBAL_POOL. I though the fix for that just went upstream?
Hi Christoph, On Mon, Oct 9, 2023 at 11:16 AM Christoph Hellwig <hch@lst.de> wrote: > On Mon, Oct 09, 2023 at 10:43:57AM +0200, Geert Uytterhoeven wrote: > > > config DMA_DIRECT_REMAP > > > > riscv defconfig + CONFIG_NONPORTABLE=y + CONFIG_ARCH_R9A07G043=y: > > > > WARNING: unmet direct dependencies detected for DMA_GLOBAL_POOL > > Depends on [n]: !ARCH_HAS_DMA_SET_UNCACHED [=n] && !DMA_DIRECT_REMAP [=y] > > Selected by [y]: > > - ARCH_R9A07G043 [=y] && SOC_RENESAS [=y] && RISCV [=y] && NONPORTABLE [=y] > > And that's exactly what this patch is supposed to show. RISCV must not > select DMA_DIRECT_REMAP at the same time as DMA_GLOBAL_POOL. I though > the fix for that just went upstream? The fix you are referring too is probably commit c1ec4b450ab729e3 ("soc: renesas: Make ARCH_R9A07G043 (riscv version) depend on NONPORTABLE") in next-20231006 and later. It is not yet upstream. Still, it merely makes ARCH_R9A07G043 (which selects DMA_GLOBAL_POOL) depend on ARCH_R9A07G043. RISCV_DMA_NONCOHERENT still selects DMA_DIRECT_REMAP, so both can end up being enabled. Gr{oetje,eeting}s, Geert
On Mon, Oct 09, 2023 at 11:34:55AM +0200, Geert Uytterhoeven wrote: > The fix you are referring too is probably commit c1ec4b450ab729e3 > ("soc: renesas: Make ARCH_R9A07G043 (riscv version) depend > on NONPORTABLE") in next-20231006 and later. It is not yet upstream. > > Still, it merely makes ARCH_R9A07G043 (which selects DMA_GLOBAL_POOL) > depend on ARCH_R9A07G043. > RISCV_DMA_NONCOHERENT still selects DMA_DIRECT_REMAP, so both can end > up being enabled. Ok, so we need to actually fix this properly. Lad, can you respin the fix to not select DMA_DIRECT_REMAP, for ARCH_R9A07G043?
Hi Christoph, CC soc On Mon, Oct 9, 2023 at 11:43 AM Christoph Hellwig <hch@lst.de> wrote: > On Mon, Oct 09, 2023 at 11:34:55AM +0200, Geert Uytterhoeven wrote: > > The fix you are referring too is probably commit c1ec4b450ab729e3 > > ("soc: renesas: Make ARCH_R9A07G043 (riscv version) depend > > on NONPORTABLE") in next-20231006 and later. It is not yet upstream. > > > > Still, it merely makes ARCH_R9A07G043 (which selects DMA_GLOBAL_POOL) > > depend on ARCH_R9A07G043. > > RISCV_DMA_NONCOHERENT still selects DMA_DIRECT_REMAP, so both can end > > up being enabled. > > Ok, so we need to actually fix this properly. Lad, can you respin > the fix to not select DMA_DIRECT_REMAP, for ARCH_R9A07G043? ARCH_R9A07G043 does not select DMA_DIRECT_REMAP directly, RISCV_DMA_NONCOHERENT does. And there are other users of RISCV_DMA_NONCOHERENT (RISCV_ISA_ZICBOM and ERRATA_THEAD_CMO). Should the selection of DMA_DIRECT_REMAP moved to their users? Note that the fix is already in soc/for-next, so we need coordination with the soc people. Gr{oetje,eeting}s, Geert
On 2023-10-09 10:51, Geert Uytterhoeven wrote: > Hi Christoph, > > CC soc > > On Mon, Oct 9, 2023 at 11:43 AM Christoph Hellwig <hch@lst.de> wrote: >> On Mon, Oct 09, 2023 at 11:34:55AM +0200, Geert Uytterhoeven wrote: >>> The fix you are referring too is probably commit c1ec4b450ab729e3 >>> ("soc: renesas: Make ARCH_R9A07G043 (riscv version) depend >>> on NONPORTABLE") in next-20231006 and later. It is not yet upstream. >>> >>> Still, it merely makes ARCH_R9A07G043 (which selects DMA_GLOBAL_POOL) >>> depend on ARCH_R9A07G043. >>> RISCV_DMA_NONCOHERENT still selects DMA_DIRECT_REMAP, so both can end >>> up being enabled. >> >> Ok, so we need to actually fix this properly. Lad, can you respin >> the fix to not select DMA_DIRECT_REMAP, for ARCH_R9A07G043? > > ARCH_R9A07G043 does not select DMA_DIRECT_REMAP directly, > RISCV_DMA_NONCOHERENT does. And there are other users of > RISCV_DMA_NONCOHERENT (RISCV_ISA_ZICBOM and ERRATA_THEAD_CMO). > Should the selection of DMA_DIRECT_REMAP moved to their users? No, the selection of DMA_GLOBAL_POOL should be removed from RISV_DMA_NONCOHERENT and selected directly by ARCH_R9A07G043 (along with any of the other implied symbols it needs). Or if as suggested this physical-attribute-remap wackiness is due to show up on more platforms as well, maybe have a common config for that which selects DMA_GLOBAL_POOL plus the relevant cache maintenance extensions as an equivalent to RISCV_DMA_NONCOHERENT, and can itself explicitly depend on NONPORTABLE for clarity. Thanks, Robin. > Note that the fix is already in soc/for-next, so we need coordination > with the soc people. > > Gr{oetje,eeting}s, > > Geert >
Hi Robin, On Mon, Oct 9, 2023 at 12:04 PM Robin Murphy <robin.murphy@arm.com> wrote: > On 2023-10-09 10:51, Geert Uytterhoeven wrote: > > On Mon, Oct 9, 2023 at 11:43 AM Christoph Hellwig <hch@lst.de> wrote: > >> On Mon, Oct 09, 2023 at 11:34:55AM +0200, Geert Uytterhoeven wrote: > >>> The fix you are referring too is probably commit c1ec4b450ab729e3 > >>> ("soc: renesas: Make ARCH_R9A07G043 (riscv version) depend > >>> on NONPORTABLE") in next-20231006 and later. It is not yet upstream. > >>> > >>> Still, it merely makes ARCH_R9A07G043 (which selects DMA_GLOBAL_POOL) > >>> depend on ARCH_R9A07G043. > >>> RISCV_DMA_NONCOHERENT still selects DMA_DIRECT_REMAP, so both can end > >>> up being enabled. > >> > >> Ok, so we need to actually fix this properly. Lad, can you respin > >> the fix to not select DMA_DIRECT_REMAP, for ARCH_R9A07G043? > > > > ARCH_R9A07G043 does not select DMA_DIRECT_REMAP directly, > > RISCV_DMA_NONCOHERENT does. And there are other users of > > RISCV_DMA_NONCOHERENT (RISCV_ISA_ZICBOM and ERRATA_THEAD_CMO). > > Should the selection of DMA_DIRECT_REMAP moved to their users? > > No, the selection of DMA_GLOBAL_POOL should be removed from > RISV_DMA_NONCOHERENT and selected directly by ARCH_R9A07G043 (along with > any of the other implied symbols it needs). Or if as suggested this > physical-attribute-remap wackiness is due to show up on more platforms > as well, maybe have a common config for that which selects > DMA_GLOBAL_POOL plus the relevant cache maintenance extensions as an > equivalent to RISCV_DMA_NONCOHERENT, and can itself explicitly depend on > NONPORTABLE for clarity. RISCV_DMA_NONCOHERENT does not select DMA_GLOBAL_POOL, ARCH_R9A07G043 selects DMA_GLOBAL_POOL. RISCV_DMA_NONCOHERENT does select DMA_DIRECT_REMAP if MMU. Gr{oetje,eeting}s, Geert
On Mon, Oct 09, 2023 at 01:10:26PM +0200, Geert Uytterhoeven wrote: > RISCV_DMA_NONCOHERENT does not select DMA_GLOBAL_POOL, > ARCH_R9A07G043 selects DMA_GLOBAL_POOL. > RISCV_DMA_NONCOHERENT does select DMA_DIRECT_REMAP if MMU. Yeah, and we'll basically need to split RISCV_DMA_NONCOHERENT into an option for each type of non-coherent support. This is why we should never have added support for any of the non-standard versions, as it's turning into a giant mess.
On 09/10/2023 1:48 pm, Christoph Hellwig wrote: > On Mon, Oct 09, 2023 at 01:10:26PM +0200, Geert Uytterhoeven wrote: >> RISCV_DMA_NONCOHERENT does not select DMA_GLOBAL_POOL, >> ARCH_R9A07G043 selects DMA_GLOBAL_POOL. >> RISCV_DMA_NONCOHERENT does select DMA_DIRECT_REMAP if MMU. Bleh, guess I should have known better than to trust my Monday morning memory without double-checking the code :) > Yeah, and we'll basically need to split RISCV_DMA_NONCOHERENT into > an option for each type of non-coherent support. This is why we > should never have added support for any of the non-standard versions, > as it's turning into a giant mess. Indeed the main point I was trying to get at is for ARCH_R9A07G043 (or rather possibly ERRATA_ANDES_CMO) to not select RISCV_DMA_NONCOHERENT in its current form, since that ending up selecting DMA_DIRECT_REMAP on a platform which can't support it is the thing that's most obviously wrong. Thanks, Robin.
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig index f488997b071712..4524db877eba36 100644 --- a/kernel/dma/Kconfig +++ b/kernel/dma/Kconfig @@ -135,6 +135,8 @@ config DMA_COHERENT_POOL config DMA_GLOBAL_POOL select DMA_DECLARE_COHERENT + depends on !ARCH_HAS_DMA_SET_UNCACHED + depends on !DMA_DIRECT_REMAP bool config DMA_DIRECT_REMAP
CONFIG_DMA_GLOBAL_POOL can't be combined with other dma-coherent allocators. Add dependencies to Kconfig to document this, and make kconfig complain about unment dependencies if someone tries. Signed-off-by: Christoph Hellwig <hch@lst.de> --- kernel/dma/Kconfig | 2 ++ 1 file changed, 2 insertions(+)