diff mbox series

riscv: select DMA_DIRECT_REMAP by RISCV_ISA_SVPBMT and ERRATA_THEAD_MAE

Message ID 20250116172950.1989748-1-geomatsi@gmail.com (mailing list archive)
State New
Headers show
Series riscv: select DMA_DIRECT_REMAP by RISCV_ISA_SVPBMT and ERRATA_THEAD_MAE | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR fail PR summary
conchuod/patch-1-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh took 107.00s
conchuod/patch-1-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh took 1033.78s
conchuod/patch-1-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh took 1210.79s
conchuod/patch-1-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh took 16.27s
conchuod/patch-1-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh took 18.05s
conchuod/patch-1-test-6 success .github/scripts/patches/tests/checkpatch.sh took 0.52s
conchuod/patch-1-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh took 36.13s
conchuod/patch-1-test-8 success .github/scripts/patches/tests/header_inline.sh took 0.00s
conchuod/patch-1-test-9 success .github/scripts/patches/tests/kdoc.sh took 0.48s
conchuod/patch-1-test-10 success .github/scripts/patches/tests/module_param.sh took 0.01s
conchuod/patch-1-test-11 fail .github/scripts/patches/tests/verify_fixes.sh took 0.02s
conchuod/patch-1-test-12 success .github/scripts/patches/tests/verify_signedoff.sh took 0.02s

Commit Message

Sergey Matyukevich Jan. 16, 2025, 5:29 p.m. UTC
Select DMA_DIRECT_REMAP for the RISC-V extensions that allow to set
page-based memory types in PTEs according to the requested DMA
attributes. This is the purpose of Svpbmt or XTheadMae extensions.
Zicbom or XTheadCmo serve a different purpose, providing instructions
to flush/invalidate cache blocks.

Fixes: 381cae169853 ("riscv: only select DMA_DIRECT_REMAP from RISCV_ISA_ZICBOM and ERRATA_THEAD_PBMT")

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 arch/riscv/Kconfig        | 2 +-
 arch/riscv/Kconfig.errata | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Christoph Hellwig Jan. 17, 2025, 7:52 a.m. UTC | #1
On Thu, Jan 16, 2025 at 08:29:35PM +0300, Sergey Matyukevich wrote:
> Select DMA_DIRECT_REMAP for the RISC-V extensions that allow to set
> page-based memory types in PTEs according to the requested DMA
> attributes. This is the purpose of Svpbmt or XTheadMae extensions.
> Zicbom or XTheadCmo serve a different purpose, providing instructions
> to flush/invalidate cache blocks.

Please explain what this is supposed to solve, because the above
explanation dosn't make any sense.  DMA_DIRECT_REMAP is one
of the implementations supporting dma coherent allocatiosn for
non-coherent devices.  So selecting it from something that
just keyes off support for an extension, but not the dma
implementation is wrong.
Sergey Matyukevich Jan. 17, 2025, 8:38 a.m. UTC | #2
On Fri, Jan 17, 2025 at 08:52:38AM +0100, Christoph Hellwig wrote:
> On Thu, Jan 16, 2025 at 08:29:35PM +0300, Sergey Matyukevich wrote:
> > Select DMA_DIRECT_REMAP for the RISC-V extensions that allow to set
> > page-based memory types in PTEs according to the requested DMA
> > attributes. This is the purpose of Svpbmt or XTheadMae extensions.
> > Zicbom or XTheadCmo serve a different purpose, providing instructions
> > to flush/invalidate cache blocks.
> 
> Please explain what this is supposed to solve, because the above
> explanation dosn't make any sense.  DMA_DIRECT_REMAP is one
> of the implementations supporting dma coherent allocatiosn for
> non-coherent devices.  So selecting it from something that
> just keyes off support for an extension, but not the dma
> implementation is wrong.

Now DMA_DIRECT_REMAP is selected either by Zicbom (standard) or XTheadCmo
(vendor) RISC-V extensions. However neither of them can help to implement
DMA_DIRECT_REMAP on RISC-V. So selection of DMA_DIRECT_REMAP has been
moved in Kconfigs to Svpbmt and XTheadMae extensions.
Christoph Hellwig Jan. 17, 2025, 9:58 a.m. UTC | #3
On Fri, Jan 17, 2025 at 11:38:47AM +0300, Sergey Matyukevich wrote:
> > Please explain what this is supposed to solve, because the above
> > explanation dosn't make any sense.  DMA_DIRECT_REMAP is one
> > of the implementations supporting dma coherent allocatiosn for
> > non-coherent devices.  So selecting it from something that
> > just keyes off support for an extension, but not the dma
> > implementation is wrong.
> 
> Now DMA_DIRECT_REMAP is selected either by Zicbom (standard) or XTheadCmo
> (vendor) RISC-V extensions.

Because they need DMA_DIRECT_REMAP to implement DMA coherent.

> However neither of them can help to implement
> DMA_DIRECT_REMAP on RISC-V. So selection of DMA_DIRECT_REMAP has been
> moved in Kconfigs to Svpbmt and XTheadMae extensions.

But Svpbmt does not imply that you even need DMA_DIRECT_REMAP.

Are you tying to solve a problem here?  If so can you explain it?
Sergey Matyukevich Jan. 17, 2025, 11:01 a.m. UTC | #4
Hi Christoph,

On Fri, Jan 17, 2025 at 10:58:32AM +0100, Christoph Hellwig wrote:
> On Fri, Jan 17, 2025 at 11:38:47AM +0300, Sergey Matyukevich wrote:
> > > Please explain what this is supposed to solve, because the above
> > > explanation dosn't make any sense.  DMA_DIRECT_REMAP is one
> > > of the implementations supporting dma coherent allocatiosn for
> > > non-coherent devices.  So selecting it from something that
> > > just keyes off support for an extension, but not the dma
> > > implementation is wrong.
> > 
> > Now DMA_DIRECT_REMAP is selected either by Zicbom (standard) or XTheadCmo
> > (vendor) RISC-V extensions.
> 
> Because they need DMA_DIRECT_REMAP to implement DMA coherent.
> 
> > However neither of them can help to implement
> > DMA_DIRECT_REMAP on RISC-V. So selection of DMA_DIRECT_REMAP has been
> > moved in Kconfigs to Svpbmt and XTheadMae extensions.
> 
> But Svpbmt does not imply that you even need DMA_DIRECT_REMAP.
> 
> Are you tying to solve a problem here?  If so can you explain it?

Sure. In brief, this about the choice between DMA_GLOBAL_POOL and
DMA_DIRECT_REMAP. We can use either of them to work with non-coherent
devices. But on RISC-V those two options require different extensions.

If we select DMA_GLOBAL_POOL, then we need only cache operations. So on
RISC-V it is enough to have Zicbom (standard) or XTheadCmo (vendor) or
any other vendor specific cache ops implemented using RISCV_ALTERNATIVE
or RISCV_NONSTANDARD_CACHE_OPS.

Using DMA_DIRECT_REMAP requires not only cache management operations,
but also a way to modify page attributes, e.g. to mark it non-cacheable. 
So on RISC-V in addition to CMO extensions we also need extensions for
page-based memory types such as Svpbmt (standard) or XTheadMae (vendor).

Current RISC-V Kconfig files enable DMA_DIRECT_REMAP for Zicbom and
XTheadCmo. According to the above comments, this is not good:
- it is wrong since Zicbom alone is not enough for DMA_DIRECT_REMAP
- it prevents using DMA_GLOBAL_POOL for platforms without support for
  page-based memory attributes

My suggestion was to move DMA_DIRECT_REMAP to the Kconfig entries for
Svpbmt and XTheadMae. In this case platforms without Svpbmt support
can disable it in kernel config and switch to DMA_GLOBAL_POOL instead.

IIUC one of your concerns was selecting DMA_DIRECT_REMAP under config
option not related to DMA implementations. Does it make sense to add
additional layer similar to RISCV_DMA_NONCOHERENT ?

Regards,
Sergey
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index d4a7ca0388c0..a5dabb744009 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -603,6 +603,7 @@  config RISCV_ISA_SVPBMT
 	depends on 64BIT && MMU
 	depends on RISCV_ALTERNATIVE
 	default y
+	select DMA_DIRECT_REMAP
 	help
 	   Adds support to dynamically detect the presence of the Svpbmt
 	   ISA-extension (Supervisor-mode: page-based memory types) and
@@ -787,7 +788,6 @@  config RISCV_ISA_ZICBOM
 	depends on RISCV_ALTERNATIVE
 	default y
 	select RISCV_DMA_NONCOHERENT
-	select DMA_DIRECT_REMAP
 	help
 	   Adds support to dynamically detect the presence of the ZICBOM
 	   extension (Cache Block Management Operations) and enable its
diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
index 2acc7d876e1f..3bcae5bd3231 100644
--- a/arch/riscv/Kconfig.errata
+++ b/arch/riscv/Kconfig.errata
@@ -86,6 +86,7 @@  config ERRATA_THEAD_MAE
 	bool "Apply T-Head's memory attribute extension (XTheadMae) errata"
 	depends on ERRATA_THEAD && 64BIT && MMU
 	select RISCV_ALTERNATIVE_EARLY
+	select DMA_DIRECT_REMAP
 	default y
 	help
 	  This will apply the memory attribute extension errata to handle the
@@ -96,7 +97,6 @@  config ERRATA_THEAD_MAE
 config ERRATA_THEAD_CMO
 	bool "Apply T-Head cache management errata"
 	depends on ERRATA_THEAD && MMU
-	select DMA_DIRECT_REMAP
 	select RISCV_DMA_NONCOHERENT
 	select RISCV_NONSTANDARD_CACHE_OPS
 	default y