| Message ID | 20231016054755.915155-2-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [01/12] riscv: RISCV_NONSTANDARD_CACHE_OPS shouldn't depend on RISCV_DMA_NONCOHERENT | expand |
Hey, On Mon, Oct 16, 2023 at 07:47:43AM +0200, Christoph Hellwig wrote: > RISCV_NONSTANDARD_CACHE_OPS is also used for the pmem cache maintenance > helpers, which are built into the kernel unconditionally. You surely have better insight than I do here, but is this actually required? This patch seems to allow creation of a kernel where the cache maintenance operations could be used for pmem, but would be otherwise unavailable, which seems counter intuitive to me. Why would someone want to provide the pmem helpers with cache maintenance operations, but not provide them generally? I also don't really understand what the unconditional nature of the pmem helpers has to do with anything, as this patch does not unconditionally provide any cache management operations, only relax the conditions under which the non-standard cache management operations can be provided. What am I missing? > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > arch/riscv/Kconfig | 1 - > drivers/cache/Kconfig | 2 +- > drivers/soc/renesas/Kconfig | 2 +- > 3 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index d607ab0f7c6daf..0ac0b538379718 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -277,7 +277,6 @@ config RISCV_DMA_NONCOHERENT > > config RISCV_NONSTANDARD_CACHE_OPS > bool > - depends on RISCV_DMA_NONCOHERENT > help > This enables function pointer support for non-standard noncoherent > systems to handle cache management. > diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig > index a57677f908f3ba..d6e5e3abaad8af 100644 > --- a/drivers/cache/Kconfig > +++ b/drivers/cache/Kconfig > @@ -3,7 +3,7 @@ menu "Cache Drivers" > > config AX45MP_L2_CACHE > bool "Andes Technology AX45MP L2 Cache controller" > - depends on RISCV_DMA_NONCOHERENT > + depends on RISCV > select RISCV_NONSTANDARD_CACHE_OPS > help > Support for the L2 cache controller on Andes Technology AX45MP platforms. > diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig > index 12040ce116a551..880c544bb2dfda 100644 > --- a/drivers/soc/renesas/Kconfig > +++ b/drivers/soc/renesas/Kconfig > @@ -335,7 +335,7 @@ config ARCH_R9A07G043 > bool "RISC-V Platform support for RZ/Five" > depends on NONPORTABLE > select ARCH_RZG2L > - select AX45MP_L2_CACHE if RISCV_DMA_NONCOHERENT FWIW, this one is already gone in linux-next, as part of fixing some randconfig issues. Cheers, Conor. > + select AX45MP_L2_CACHE > select DMA_GLOBAL_POOL > select ERRATA_ANDES if RISCV_SBI > select ERRATA_ANDES_CMO if ERRATA_ANDES > -- > 2.39.2 >
On Mon, Oct 16, 2023 at 01:49:41PM +0100, Conor Dooley wrote: > Hey, > > On Mon, Oct 16, 2023 at 07:47:43AM +0200, Christoph Hellwig wrote: > > RISCV_NONSTANDARD_CACHE_OPS is also used for the pmem cache maintenance > > helpers, which are built into the kernel unconditionally. > > You surely have better insight than I do here, but is this actually > required? > This patch seems to allow creation of a kernel where the cache > maintenance operations could be used for pmem, but would be otherwise > unavailable, which seems counter intuitive to me. > > Why would someone want to provide the pmem helpers with cache > maintenance operations, but not provide them generally? > Even if all your periphals are cache coherent (very common on server class hardware) you still need cache maintenance for pmem. No need to force the extra text size and runtime overhead for non-coherent DMA. > I also don't really understand what the unconditional nature of the pmem > helpers has to do with anything, as this patch does not unconditionally > provide any cache management operations, only relax the conditions under > which the non-standard cache management operations can be provided. They simply were broken if a platform had non-standard cache mem but only coherent DMA before. That's probably more a theoretical than practial case, but still worth fixing.
On Mon, Oct 16, 2023 at 03:17:16PM +0200, Christoph Hellwig wrote: > On Mon, Oct 16, 2023 at 01:49:41PM +0100, Conor Dooley wrote: > > Hey, > > > > On Mon, Oct 16, 2023 at 07:47:43AM +0200, Christoph Hellwig wrote: > > > RISCV_NONSTANDARD_CACHE_OPS is also used for the pmem cache maintenance > > > helpers, which are built into the kernel unconditionally. > > > > You surely have better insight than I do here, but is this actually > > required? > > This patch seems to allow creation of a kernel where the cache > > maintenance operations could be used for pmem, but would be otherwise > > unavailable, which seems counter intuitive to me. > > > > Why would someone want to provide the pmem helpers with cache > > maintenance operations, but not provide them generally? > > > > Even if all your periphals are cache coherent (very common on server > class hardware) you still need cache maintenance for pmem. No need > to force the extra text size and runtime overhead for non-coherent DMA. Ah, right. > > I also don't really understand what the unconditional nature of the pmem > > helpers has to do with anything, as this patch does not unconditionally > > provide any cache management operations, only relax the conditions under > > which the non-standard cache management operations can be provided. > > They simply were broken if a platform had non-standard cache mem but > only coherent DMA before. That's probably more a theoretical than > practial case, but still worth fixing. And this part of it makes more sense with the above use-case explained. Acked-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor.
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index d607ab0f7c6daf..0ac0b538379718 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -277,7 +277,6 @@ config RISCV_DMA_NONCOHERENT config RISCV_NONSTANDARD_CACHE_OPS bool - depends on RISCV_DMA_NONCOHERENT help This enables function pointer support for non-standard noncoherent systems to handle cache management. diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig index a57677f908f3ba..d6e5e3abaad8af 100644 --- a/drivers/cache/Kconfig +++ b/drivers/cache/Kconfig @@ -3,7 +3,7 @@ menu "Cache Drivers" config AX45MP_L2_CACHE bool "Andes Technology AX45MP L2 Cache controller" - depends on RISCV_DMA_NONCOHERENT + depends on RISCV select RISCV_NONSTANDARD_CACHE_OPS help Support for the L2 cache controller on Andes Technology AX45MP platforms. diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig index 12040ce116a551..880c544bb2dfda 100644 --- a/drivers/soc/renesas/Kconfig +++ b/drivers/soc/renesas/Kconfig @@ -335,7 +335,7 @@ config ARCH_R9A07G043 bool "RISC-V Platform support for RZ/Five" depends on NONPORTABLE select ARCH_RZG2L - select AX45MP_L2_CACHE if RISCV_DMA_NONCOHERENT + select AX45MP_L2_CACHE select DMA_GLOBAL_POOL select ERRATA_ANDES if RISCV_SBI select ERRATA_ANDES_CMO if ERRATA_ANDES
RISCV_NONSTANDARD_CACHE_OPS is also used for the pmem cache maintenance helpers, which are built into the kernel unconditionally. Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/riscv/Kconfig | 1 - drivers/cache/Kconfig | 2 +- drivers/soc/renesas/Kconfig | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-)