Re: [PATCH 0/6] RISC-V: Add eMMC support for TH1520 boards

[Lad, Prabhakar]

On Wed, Oct 4, 2023 at 3:18 PM Robin Murphy <robin.murphy@xxxxxxx> wrote:
>
> On 04/10/2023 3:02 pm, Icenowy Zheng wrote:
> [...]
> >>>> I believe commit 484861e09f3e ("soc: renesas: Kconfig: Select the
> >>>> required configs for RZ/Five SoC") can cause regression on all
> >>>> non-dma-coherent riscv platforms with generic defconfig. This is
> >>>> a common issue. The logic here is: generic riscv defconfig
> >>>> selects
> >>>> ARCH_R9A07G043 which selects DMA_GLOBAL_POOL, which assumes all
> >>>> non-dma-coherent riscv platforms have a dma global pool, this
> >>>> assumption
> >>>> seems not correct. And I believe DMA_GLOBAL_POOL should not be
> >>>> selected by ARCH_SOCFAMILIY, instead, only ARCH under some
> >>>> specific
> >>>> conditions can select it globaly, for example NOMMU ARM and so
> >>>> on.
> >>>>
> >>>> Since this is a regression, what's proper fix? any suggestion is
> >>>> appreciated.
> >>
> >> I think the answer is to not select DMA_GLOBAL_POOL, since that is
> >> only
> >
> > Well I think for RISC-V, it's not NOMMU only but applicable for every
> > core that does not support Svpbmt or vendor-specific alternatives,
> > because the original RISC-V priv spec does not define memory attributes
> > in page table entries.
> >
> > For the Renesas/Andes case I think a pool is set by OpenSBI with
> > vendor-specific M-mode facility and then passed in DT, and the S-mode
> > (which MMU is enabled in) just sees fixed memory attributes, in this
> > case I think DMA_GLOBAL_POOL is needed.
>
> Oh wow, is that really a thing? In that case, either you just can't
> support this platform in a multi-platform kernel, or someone needs to do
> some fiddly work in dma-direct to a) introduce the notion of an optional
> global pool,
Looking at the code [0] we do have compile time check for
CONFIG_DMA_GLOBAL_POOL irrespective of this being present in DT or
not, instead if we make it compile time and runtime check ie either
check for DT node or see if pool is available and only then proceed
for allocation form this pool.

What are your thoughts on this?

[0] https://elixir.bootlin.com/linux/v6.6-rc4/source/kernel/dma/direct.c#L238

> and b) make it somehow cope with DMA_DIRECT_REMAP being
> enabled but non-functional.
>
DMA_DIRECT_REMAP config option is selected by NONCOHERENET config option anyway.

Cheers,
Prabhakar