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

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

[Lad, Prabhakar]

+ CC linux-mm and Robin Murphy

On Wed, Oct 4, 2023 at 12:42 PM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> On Mon, Oct 02, 2023 at 09:37:44PM -0700, Drew Fustini wrote:
> > On Fri, Sep 22, 2023 at 05:48:21PM -0500, Robert Nelson wrote:
> > > On Fri, Sep 22, 2023 at 2:08 PM Robert Nelson <robertcnelson@gmail.com> wrote:
> > > >
> > > > On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <dfustini@baylibre.com> wrote:
> > > > >
> > > > > This series adds support for the eMMC on the BeagleV Ahead and the
> > > > > Sipeed LicheePi 4A. This allows the kernel to boot with the rootfs on
> > > > > eMMC.
> > > > >
> > > > > I tested on top of v6.6-rc2 with this config [1]. I was able to boot
> > > > > both the Ahead [2] and LPi4a [3] from eMMC. The following prerequisites
> > > > > are required:
> > > > >
> > > > >   [PATCH v2] riscv: dts: thead: set dma-noncoherent to soc bus [4]
> > > > >
> > > > > I pushed a branch [5] with this patch series and the above patch for
> > > > > those that find a git branch easier to test.
> > > > >
> > > > > Please note that only the MMC controller connected to the eMMC device
> > > > > is enabled in the device trees for these two boards. I did not yet
> > > > > attempt to configure and use the microSD card slot. My preference is to
> > > > > address that in a future patch series.
> > > > >
> > > > > References:
> > > > > [1] https://gist.github.com/pdp7/5fbdcf2a65eb1abdd3a29d519c19cdd2
> > > > > [2] https://gist.github.com/pdp7/91a801a5f8d1070c53509eda9800ad78
> > > > > [3] https://gist.github.com/pdp7/1445c3c991e88fd69c60165cef65726a
> > > > > [4] https://lore.kernel.org/linux-riscv/20230912072232.2455-1-jszhang@kernel.org/
> > > > > [5] https://github.com/pdp7/linux/tree/b4/th1520-mmc
> > > >
> > > > This patchset came out very nice!
> > > >
> > > > v6.6-rc2 with Last RFC v2:
> > > >
> > > > [    4.066630] mmc0: SDHCI controller on ffe7080000.mmc
> > > > [ffe7080000.mmc] using PIO
> > > >
> > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > >
> > > > /dev/mmcblk0:
> > > >  Timing cached reads:   1516 MB in  2.00 seconds = 758.09 MB/sec
> > > >  Timing buffered disk reads:  84 MB in  3.01 seconds =  27.94 MB/sec
> > > >
> > > > vs v6.6-rc2 with this patchset:
> > > >
> > > >  [    4.096837] mmc0: SDHCI controller on ffe7080000.mmc
> > > > [ffe7080000.mmc] using DMA
> > > >
> > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > >
> > > > /dev/mmcblk0:
> > > >  Timing cached reads:   1580 MB in  2.00 seconds = 790.97 MB/sec
> > > >  Timing buffered disk reads: 418 MB in  3.00 seconds = 139.11 MB/sec
> > >
> > > Drew pointed out on Slack, this was not quite right.. After more
> > > digging by Drew, CONFIG_DMA_GLOBAL_POOL is causing a DMA limitation
> > > with the multiplatform defconfig. so with,
> > >
> > > ./scripts/config --disable CONFIG_ARCH_R9A07G043
> > >
> > > (to remove CONFIG_DMA_GLOBAL_POOL)... another 2x in buffered reads..
> > >
> > > [    4.059242] mmc0: SDHCI controller on ffe7080000.mmc
> > > [ffe7080000.mmc] using ADMA 64-bit
> > >
> > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > >
> > > /dev/mmcblk0:
> > >  Timing cached reads:   1600 MB in  2.00 seconds = 800.93 MB/sec
> > >  Timing buffered disk reads: 892 MB in  3.00 seconds = 297.06 MB/sec
> >
> > It seems CONFIG_DMA_GLOBAL_POOL=y causes ADMA buffer alloc to fail [1]:
> >
> >   mmc0: Unable to allocate ADMA buffers - falling back to standard DMA
> >
> > Prabhakar's AX45MP non-coherent DMA support [2] series introduced the
> > selection of DMA_GLOBAL_POOL for ARCH_R9A07G043 and the riscv defconfig
> > selects ARCH_R9A07G043.
> >
> > Patch 5 in the series [3] states that:
> >
> >   With DMA_GLOBAL_POOL enabled all DMA allocations happen from this
> >   region and synchronization callbacks are implemented to synchronize
> >   when doing DMA transactions.
> >
> > This example of a "shared-dma-pool" node was given:
> >
> >         pma_resv0@58000000 {
> >             compatible = "shared-dma-pool";
> >             reg = <0x0 0x58000000 0x0 0x08000000>;
> >             no-map;
> >             linux,dma-default;
> >         };
> >
> > I've copied that to th1520-beaglev-ahead.dts. The address of 0x58000000
> > has no significance on th1520, but the existence of shared-dma-pool
> > seems to fix the problem. ADMA mode [4] is now working even though
> > CONFIG_DMA_GLOBAL_POOL=y.
>
> + Christoph, Lad
>
> IMHO, this is not TH1520 specific but a generic issue.
>
> 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.
>
> Thanks
>
> >
> > Thanks,
> > Drew
> >
> > [1] https://gist.github.com/pdp7/73041ed808bbc7dd445836fb90574979
> > [2] https://lore.kernel.org/linux-riscv/20230818135723.80612-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
> > [3] https://lore.kernel.org/linux-riscv/20230818135723.80612-6-prabhakar.mahadev-lad.rj@bp.renesas.com/
> > [4] https://gist.github.com/pdp7/91e72a663d3bb73eb28182337ad8bbcb

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

[Robin Murphy]

On 04/10/2023 2:02 pm, Lad, Prabhakar wrote:
> + CC linux-mm and Robin Murphy
> 
> On Wed, Oct 4, 2023 at 12:42 PM Jisheng Zhang <jszhang@kernel.org> wrote:
>>
>> On Mon, Oct 02, 2023 at 09:37:44PM -0700, Drew Fustini wrote:
>>> On Fri, Sep 22, 2023 at 05:48:21PM -0500, Robert Nelson wrote:
>>>> On Fri, Sep 22, 2023 at 2:08 PM Robert Nelson <robertcnelson@gmail.com> wrote:
>>>>>
>>>>> On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <dfustini@baylibre.com> wrote:
>>>>>>
>>>>>> This series adds support for the eMMC on the BeagleV Ahead and the
>>>>>> Sipeed LicheePi 4A. This allows the kernel to boot with the rootfs on
>>>>>> eMMC.
>>>>>>
>>>>>> I tested on top of v6.6-rc2 with this config [1]. I was able to boot
>>>>>> both the Ahead [2] and LPi4a [3] from eMMC. The following prerequisites
>>>>>> are required:
>>>>>>
>>>>>>    [PATCH v2] riscv: dts: thead: set dma-noncoherent to soc bus [4]
>>>>>>
>>>>>> I pushed a branch [5] with this patch series and the above patch for
>>>>>> those that find a git branch easier to test.
>>>>>>
>>>>>> Please note that only the MMC controller connected to the eMMC device
>>>>>> is enabled in the device trees for these two boards. I did not yet
>>>>>> attempt to configure and use the microSD card slot. My preference is to
>>>>>> address that in a future patch series.
>>>>>>
>>>>>> References:
>>>>>> [1] https://gist.github.com/pdp7/5fbdcf2a65eb1abdd3a29d519c19cdd2
>>>>>> [2] https://gist.github.com/pdp7/91a801a5f8d1070c53509eda9800ad78
>>>>>> [3] https://gist.github.com/pdp7/1445c3c991e88fd69c60165cef65726a
>>>>>> [4] https://lore.kernel.org/linux-riscv/20230912072232.2455-1-jszhang@kernel.org/
>>>>>> [5] https://github.com/pdp7/linux/tree/b4/th1520-mmc
>>>>>
>>>>> This patchset came out very nice!
>>>>>
>>>>> v6.6-rc2 with Last RFC v2:
>>>>>
>>>>> [    4.066630] mmc0: SDHCI controller on ffe7080000.mmc
>>>>> [ffe7080000.mmc] using PIO
>>>>>
>>>>> debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
>>>>>
>>>>> /dev/mmcblk0:
>>>>>   Timing cached reads:   1516 MB in  2.00 seconds = 758.09 MB/sec
>>>>>   Timing buffered disk reads:  84 MB in  3.01 seconds =  27.94 MB/sec
>>>>>
>>>>> vs v6.6-rc2 with this patchset:
>>>>>
>>>>>   [    4.096837] mmc0: SDHCI controller on ffe7080000.mmc
>>>>> [ffe7080000.mmc] using DMA
>>>>>
>>>>> debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
>>>>>
>>>>> /dev/mmcblk0:
>>>>>   Timing cached reads:   1580 MB in  2.00 seconds = 790.97 MB/sec
>>>>>   Timing buffered disk reads: 418 MB in  3.00 seconds = 139.11 MB/sec
>>>>
>>>> Drew pointed out on Slack, this was not quite right.. After more
>>>> digging by Drew, CONFIG_DMA_GLOBAL_POOL is causing a DMA limitation
>>>> with the multiplatform defconfig. so with,
>>>>
>>>> ./scripts/config --disable CONFIG_ARCH_R9A07G043
>>>>
>>>> (to remove CONFIG_DMA_GLOBAL_POOL)... another 2x in buffered reads..
>>>>
>>>> [    4.059242] mmc0: SDHCI controller on ffe7080000.mmc
>>>> [ffe7080000.mmc] using ADMA 64-bit
>>>>
>>>> debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
>>>>
>>>> /dev/mmcblk0:
>>>>   Timing cached reads:   1600 MB in  2.00 seconds = 800.93 MB/sec
>>>>   Timing buffered disk reads: 892 MB in  3.00 seconds = 297.06 MB/sec
>>>
>>> It seems CONFIG_DMA_GLOBAL_POOL=y causes ADMA buffer alloc to fail [1]:
>>>
>>>    mmc0: Unable to allocate ADMA buffers - falling back to standard DMA
>>>
>>> Prabhakar's AX45MP non-coherent DMA support [2] series introduced the
>>> selection of DMA_GLOBAL_POOL for ARCH_R9A07G043 and the riscv defconfig
>>> selects ARCH_R9A07G043.
>>>
>>> Patch 5 in the series [3] states that:
>>>
>>>    With DMA_GLOBAL_POOL enabled all DMA allocations happen from this
>>>    region and synchronization callbacks are implemented to synchronize
>>>    when doing DMA transactions.
>>>
>>> This example of a "shared-dma-pool" node was given:
>>>
>>>          pma_resv0@58000000 {
>>>              compatible = "shared-dma-pool";
>>>              reg = <0x0 0x58000000 0x0 0x08000000>;
>>>              no-map;
>>>              linux,dma-default;
>>>          };
>>>
>>> I've copied that to th1520-beaglev-ahead.dts. The address of 0x58000000
>>> has no significance on th1520, but the existence of shared-dma-pool
>>> seems to fix the problem. ADMA mode [4] is now working even though
>>> CONFIG_DMA_GLOBAL_POOL=y.
>>
>> + Christoph, Lad
>>
>> IMHO, this is not TH1520 specific but a generic issue.
>>
>> 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 
designed for nommu cases where non-cacheable memory lives in a fixed 
place in the physical address map, and regular kernel pages can't be 
remapped. As far as I'm aware, RISCV_DMA_NONCOHERENT is the thing you 
want, such that DMA_DIRECT_REMAP can dynamically provide non-cacheable 
coherent buffers for non-hardware-coherent devices.

Thanks,
Robin.

>>
>> Thanks
>>
>>>
>>> Thanks,
>>> Drew
>>>
>>> [1] https://gist.github.com/pdp7/73041ed808bbc7dd445836fb90574979
>>> [2] https://lore.kernel.org/linux-riscv/20230818135723.80612-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
>>> [3] https://lore.kernel.org/linux-riscv/20230818135723.80612-6-prabhakar.mahadev-lad.rj@bp.renesas.com/
>>> [4] https://gist.github.com/pdp7/91e72a663d3bb73eb28182337ad8bbcb

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

[Icenowy Zheng]

在 2023-10-04星期三的 14:49 +0100，Robin Murphy写道：
> On 04/10/2023 2:02 pm, Lad, Prabhakar wrote:
> > + CC linux-mm and Robin Murphy
> > 
> > On Wed, Oct 4, 2023 at 12:42 PM Jisheng Zhang <jszhang@kernel.org>
> > wrote:
> > > 
> > > On Mon, Oct 02, 2023 at 09:37:44PM -0700, Drew Fustini wrote:
> > > > On Fri, Sep 22, 2023 at 05:48:21PM -0500, Robert Nelson wrote:
> > > > > On Fri, Sep 22, 2023 at 2:08 PM Robert Nelson
> > > > > <robertcnelson@gmail.com> wrote:
> > > > > > 
> > > > > > On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini
> > > > > > <dfustini@baylibre.com> wrote:
> > > > > > > 
> > > > > > > This series adds support for the eMMC on the BeagleV
> > > > > > > Ahead and the
> > > > > > > Sipeed LicheePi 4A. This allows the kernel to boot with
> > > > > > > the rootfs on
> > > > > > > eMMC.
> > > > > > > 
> > > > > > > I tested on top of v6.6-rc2 with this config [1]. I was
> > > > > > > able to boot
> > > > > > > both the Ahead [2] and LPi4a [3] from eMMC. The following
> > > > > > > prerequisites
> > > > > > > are required:
> > > > > > > 
> > > > > > >    [PATCH v2] riscv: dts: thead: set dma-noncoherent to
> > > > > > > soc bus [4]
> > > > > > > 
> > > > > > > I pushed a branch [5] with this patch series and the
> > > > > > > above patch for
> > > > > > > those that find a git branch easier to test.
> > > > > > > 
> > > > > > > Please note that only the MMC controller connected to the
> > > > > > > eMMC device
> > > > > > > is enabled in the device trees for these two boards. I
> > > > > > > did not yet
> > > > > > > attempt to configure and use the microSD card slot. My
> > > > > > > preference is to
> > > > > > > address that in a future patch series.
> > > > > > > 
> > > > > > > References:
> > > > > > > [1]
> > > > > > > https://gist.github.com/pdp7/5fbdcf2a65eb1abdd3a29d519c19cdd2
> > > > > > > [2]
> > > > > > > https://gist.github.com/pdp7/91a801a5f8d1070c53509eda9800ad78
> > > > > > > [3]
> > > > > > > https://gist.github.com/pdp7/1445c3c991e88fd69c60165cef65726a
> > > > > > > [4]
> > > > > > > https://lore.kernel.org/linux-riscv/20230912072232.2455-1-jszhang@kernel.org/
> > > > > > > [5] https://github.com/pdp7/linux/tree/b4/th1520-mmc
> > > > > > 
> > > > > > This patchset came out very nice!
> > > > > > 
> > > > > > v6.6-rc2 with Last RFC v2:
> > > > > > 
> > > > > > [    4.066630] mmc0: SDHCI controller on ffe7080000.mmc
> > > > > > [ffe7080000.mmc] using PIO
> > > > > > 
> > > > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > > > > 
> > > > > > /dev/mmcblk0:
> > > > > >   Timing cached reads:   1516 MB in  2.00 seconds = 758.09
> > > > > > MB/sec
> > > > > >   Timing buffered disk reads:  84 MB in  3.01 seconds = 
> > > > > > 27.94 MB/sec
> > > > > > 
> > > > > > vs v6.6-rc2 with this patchset:
> > > > > > 
> > > > > >   [    4.096837] mmc0: SDHCI controller on ffe7080000.mmc
> > > > > > [ffe7080000.mmc] using DMA
> > > > > > 
> > > > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > > > > 
> > > > > > /dev/mmcblk0:
> > > > > >   Timing cached reads:   1580 MB in  2.00 seconds = 790.97
> > > > > > MB/sec
> > > > > >   Timing buffered disk reads: 418 MB in  3.00 seconds =
> > > > > > 139.11 MB/sec
> > > > > 
> > > > > Drew pointed out on Slack, this was not quite right.. After
> > > > > more
> > > > > digging by Drew, CONFIG_DMA_GLOBAL_POOL is causing a DMA
> > > > > limitation
> > > > > with the multiplatform defconfig. so with,
> > > > > 
> > > > > ./scripts/config --disable CONFIG_ARCH_R9A07G043
> > > > > 
> > > > > (to remove CONFIG_DMA_GLOBAL_POOL)... another 2x in buffered
> > > > > reads..
> > > > > 
> > > > > [    4.059242] mmc0: SDHCI controller on ffe7080000.mmc
> > > > > [ffe7080000.mmc] using ADMA 64-bit
> > > > > 
> > > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > > > 
> > > > > /dev/mmcblk0:
> > > > >   Timing cached reads:   1600 MB in  2.00 seconds = 800.93
> > > > > MB/sec
> > > > >   Timing buffered disk reads: 892 MB in  3.00 seconds =
> > > > > 297.06 MB/sec
> > > > 
> > > > It seems CONFIG_DMA_GLOBAL_POOL=y causes ADMA buffer alloc to
> > > > fail [1]:
> > > > 
> > > >    mmc0: Unable to allocate ADMA buffers - falling back to
> > > > standard DMA
> > > > 
> > > > Prabhakar's AX45MP non-coherent DMA support [2] series
> > > > introduced the
> > > > selection of DMA_GLOBAL_POOL for ARCH_R9A07G043 and the riscv
> > > > defconfig
> > > > selects ARCH_R9A07G043.
> > > > 
> > > > Patch 5 in the series [3] states that:
> > > > 
> > > >    With DMA_GLOBAL_POOL enabled all DMA allocations happen from
> > > > this
> > > >    region and synchronization callbacks are implemented to
> > > > synchronize
> > > >    when doing DMA transactions.
> > > > 
> > > > This example of a "shared-dma-pool" node was given:
> > > > 
> > > >          pma_resv0@58000000 {
> > > >              compatible = "shared-dma-pool";
> > > >              reg = <0x0 0x58000000 0x0 0x08000000>;
> > > >              no-map;
> > > >              linux,dma-default;
> > > >          };
> > > > 
> > > > I've copied that to th1520-beaglev-ahead.dts. The address of
> > > > 0x58000000
> > > > has no significance on th1520, but the existence of shared-dma-
> > > > pool
> > > > seems to fix the problem. ADMA mode [4] is now working even
> > > > though
> > > > CONFIG_DMA_GLOBAL_POOL=y.
> > > 
> > > + Christoph, Lad
> > > 
> > > IMHO, this is not TH1520 specific but a generic issue.
> > > 
> > > 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.

> designed for nommu cases where non-cacheable memory lives in a fixed 
> place in the physical address map, and regular kernel pages can't be 
> remapped. As far as I'm aware, RISCV_DMA_NONCOHERENT is the thing you
> want, such that DMA_DIRECT_REMAP can dynamically provide non-
> cacheable 
> coherent buffers for non-hardware-coherent devices.
> 
> Thanks,
> Robin.
> 
> > > 
> > > Thanks
> > > 
> > > > 
> > > > Thanks,
> > > > Drew
> > > > 
> > > > [1]
> > > > https://gist.github.com/pdp7/73041ed808bbc7dd445836fb90574979
> > > > [2]
> > > > https://lore.kernel.org/linux-riscv/20230818135723.80612-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
> > > > [3]
> > > > https://lore.kernel.org/linux-riscv/20230818135723.80612-6-prabhakar.mahadev-lad.rj@bp.renesas.com/
> > > > [4]
> > > > https://gist.github.com/pdp7/91e72a663d3bb73eb28182337ad8bbcb

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

[Robin Murphy]

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, and b) make it somehow cope with DMA_DIRECT_REMAP being 
enabled but non-functional.

Thanks,
Robin.

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

[Icenowy Zheng]

在 2023-10-04星期三的 15:18 +0100，Robin Murphy写道：
> 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 

Emmmm thus RZ/Five should `depends on NONPORTABLE`?

> some fiddly work in dma-direct to a) introduce the notion of an
> optional 
> global pool, and b) make it somehow cope with DMA_DIRECT_REMAP being 
> enabled but non-functional.
> 
> Thanks,
> Robin.

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

[Icenowy Zheng]

在 2023-10-04星期三的 15:18 +0100，Robin Murphy写道：
> 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 

Well, considering RZ/Five enables some spec-non-conformant local memory
(which bypasses MMU) that makes even running generic user space
binaries not so viable (PIE ones may still run, but those built to be
on the default fixed location of binutils will conflict with the MMU-
bypassing local memory), not supporting it in a multi-platform kernel
doesn't look like a big deal.

> some fiddly work in dma-direct to a) introduce the notion of an
> optional 
> global pool, and b) make it somehow cope with DMA_DIRECT_REMAP being 
> enabled but non-functional.
> 
> Thanks,
> Robin.

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@arm.com> 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

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

[Lad, Prabhakar]

On Wed, Oct 4, 2023 at 5:03 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> On Wed, Oct 4, 2023 at 3:18 PM Robin Murphy <robin.murphy@arm.com> 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?
>
Something like the below:

diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index f2fc203fb8a1..7bf41a4634a4 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -198,6 +198,7 @@ int dma_release_from_global_coherent(int order,
void *vaddr);
 int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
                size_t size, int *ret);
 int dma_init_global_coherent(phys_addr_t phys_addr, size_t size);
+bool dma_global_pool_available(void);
 #else
 static inline void *dma_alloc_from_global_coherent(struct device *dev,
                ssize_t size, dma_addr_t *dma_handle)
@@ -213,6 +214,10 @@ static inline int
dma_mmap_from_global_coherent(struct vm_area_struct *vma,
 {
        return 0;
 }
+static inline bool dma_global_pool_available(void)
+{
+       return false;
+}
 #endif /* CONFIG_DMA_GLOBAL_POOL */

 /*
diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
index c21abc77c53e..605f243b8262 100644
--- a/kernel/dma/coherent.c
+++ b/kernel/dma/coherent.c
@@ -277,6 +277,14 @@ int dma_mmap_from_dev_coherent(struct device
*dev, struct vm_area_struct *vma,
 #ifdef CONFIG_DMA_GLOBAL_POOL
 static struct dma_coherent_mem *dma_coherent_default_memory __ro_after_init;

+bool dma_global_pool_available(void)
+{
+       if (!dma_coherent_default_memory)
+               return false;
+
+       return true;
+}
+
 void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
                                     dma_addr_t *dma_handle)
 {
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 9596ae1aa0da..a599bb731ceb 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -235,7 +235,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
                 * If there is a global pool, always allocate from it for
                 * non-coherent devices.
                 */
-               if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL))
+               if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
dma_global_pool_available())
                        return dma_alloc_from_global_coherent(dev, size,
                                        dma_handle);


Cheers,
Prabhakar

> [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

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

[Samuel Holland]

On 2023-10-04 12:16 PM, Lad, Prabhakar wrote:
> On Wed, Oct 4, 2023 at 5:03 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
>>
>> On Wed, Oct 4, 2023 at 3:18 PM Robin Murphy <robin.murphy@arm.com> 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?
>>
> Something like the below:
> 
> diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
> index f2fc203fb8a1..7bf41a4634a4 100644
> --- a/include/linux/dma-map-ops.h
> +++ b/include/linux/dma-map-ops.h
> @@ -198,6 +198,7 @@ int dma_release_from_global_coherent(int order,
> void *vaddr);
>  int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>                 size_t size, int *ret);
>  int dma_init_global_coherent(phys_addr_t phys_addr, size_t size);
> +bool dma_global_pool_available(void);
>  #else
>  static inline void *dma_alloc_from_global_coherent(struct device *dev,
>                 ssize_t size, dma_addr_t *dma_handle)
> @@ -213,6 +214,10 @@ static inline int
> dma_mmap_from_global_coherent(struct vm_area_struct *vma,
>  {
>         return 0;
>  }
> +static inline bool dma_global_pool_available(void)
> +{
> +       return false;
> +}
>  #endif /* CONFIG_DMA_GLOBAL_POOL */
> 
>  /*
> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
> index c21abc77c53e..605f243b8262 100644
> --- a/kernel/dma/coherent.c
> +++ b/kernel/dma/coherent.c
> @@ -277,6 +277,14 @@ int dma_mmap_from_dev_coherent(struct device
> *dev, struct vm_area_struct *vma,
>  #ifdef CONFIG_DMA_GLOBAL_POOL
>  static struct dma_coherent_mem *dma_coherent_default_memory __ro_after_init;
> 
> +bool dma_global_pool_available(void)
> +{
> +       if (!dma_coherent_default_memory)
> +               return false;
> +
> +       return true;
> +}
> +
>  void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>                                      dma_addr_t *dma_handle)
>  {
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 9596ae1aa0da..a599bb731ceb 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -235,7 +235,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>                  * If there is a global pool, always allocate from it for
>                  * non-coherent devices.
>                  */
> -               if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL))
> +               if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> dma_global_pool_available())
>                         return dma_alloc_from_global_coherent(dev, size,
>                                         dma_handle);

dma_alloc_from_global_coherent() already checks dma_coherent_default_memory, so
the solution could be even simpler:

--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -232,12 +232,12 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 					      attrs);

 		/*
-		 * If there is a global pool, always allocate from it for
+		 * If there is a global pool, always try to allocate from it for
 		 * non-coherent devices.
 		 */
-		if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL))
-			return dma_alloc_from_global_coherent(dev, size,
-					dma_handle);
+		ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
+		if (ret)
+			return ret;

 		/*
 		 * Otherwise remap if the architecture is asking for it.  But

Regards,
Samuel

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

[Robin Murphy]

On 2023-10-04 19:49, Samuel Holland wrote:
> On 2023-10-04 12:16 PM, Lad, Prabhakar wrote:
>> On Wed, Oct 4, 2023 at 5:03 PM Lad, Prabhakar
>> <prabhakar.csengg@gmail.com> wrote:
>>>
>>> On Wed, Oct 4, 2023 at 3:18 PM Robin Murphy <robin.murphy@arm.com> 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?
>>>
>> Something like the below:
>>
>> diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
>> index f2fc203fb8a1..7bf41a4634a4 100644
>> --- a/include/linux/dma-map-ops.h
>> +++ b/include/linux/dma-map-ops.h
>> @@ -198,6 +198,7 @@ int dma_release_from_global_coherent(int order,
>> void *vaddr);
>>   int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>                  size_t size, int *ret);
>>   int dma_init_global_coherent(phys_addr_t phys_addr, size_t size);
>> +bool dma_global_pool_available(void);
>>   #else
>>   static inline void *dma_alloc_from_global_coherent(struct device *dev,
>>                  ssize_t size, dma_addr_t *dma_handle)
>> @@ -213,6 +214,10 @@ static inline int
>> dma_mmap_from_global_coherent(struct vm_area_struct *vma,
>>   {
>>          return 0;
>>   }
>> +static inline bool dma_global_pool_available(void)
>> +{
>> +       return false;
>> +}
>>   #endif /* CONFIG_DMA_GLOBAL_POOL */
>>
>>   /*
>> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
>> index c21abc77c53e..605f243b8262 100644
>> --- a/kernel/dma/coherent.c
>> +++ b/kernel/dma/coherent.c
>> @@ -277,6 +277,14 @@ int dma_mmap_from_dev_coherent(struct device
>> *dev, struct vm_area_struct *vma,
>>   #ifdef CONFIG_DMA_GLOBAL_POOL
>>   static struct dma_coherent_mem *dma_coherent_default_memory __ro_after_init;
>>
>> +bool dma_global_pool_available(void)
>> +{
>> +       if (!dma_coherent_default_memory)
>> +               return false;
>> +
>> +       return true;
>> +}
>> +
>>   void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>                                       dma_addr_t *dma_handle)
>>   {
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index 9596ae1aa0da..a599bb731ceb 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -235,7 +235,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>                   * If there is a global pool, always allocate from it for
>>                   * non-coherent devices.
>>                   */
>> -               if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL))
>> +               if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
>> dma_global_pool_available())
>>                          return dma_alloc_from_global_coherent(dev, size,
>>                                          dma_handle);
> 
> dma_alloc_from_global_coherent() already checks dma_coherent_default_memory, so
> the solution could be even simpler:
> 
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -232,12 +232,12 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>   					      attrs);
> 
>   		/*
> -		 * If there is a global pool, always allocate from it for
> +		 * If there is a global pool, always try to allocate from it for
>   		 * non-coherent devices.
>   		 */
> -		if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL))
> -			return dma_alloc_from_global_coherent(dev, size,
> -					dma_handle);
> +		ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
> +		if (ret)
> +			return ret;

So if allocation fails because the pool is full, we should go ahead and 
remap something that can't work? ;)

The dma_global_pool_available() idea sort of works, but I'm still 
concerned about the case where it *should* have been available but the 
platform has been misconfigured, so again we fall through to 
DMA_DIRECT_REMAP "successfully" returning a coherent buffer that isn't, 
and the user's filesystem gets corrupted. Or at best, they get confused 
by weird errors from random devices going wrong. That's why I said it 
would be fiddly - the current state of DMA_GLOBAL_POOL as a binary 
arch-wide thing is relatively robust and easy to reason about, but 
attempting to generalise it further is... less so.

Thanks,
Robin.

> 
>   		/*
>   		 * Otherwise remap if the architecture is asking for it.  But
> 
> Regards,
> Samuel
> 

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

[Lad, Prabhakar]

On Wed, Oct 4, 2023 at 8:38 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2023-10-04 19:49, Samuel Holland wrote:
> > On 2023-10-04 12:16 PM, Lad, Prabhakar wrote:
> >> On Wed, Oct 4, 2023 at 5:03 PM Lad, Prabhakar
> >> <prabhakar.csengg@gmail.com> wrote:
> >>>
> >>> On Wed, Oct 4, 2023 at 3:18 PM Robin Murphy <robin.murphy@arm.com> 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?
> >>>
> >> Something like the below:
> >>
> >> diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
> >> index f2fc203fb8a1..7bf41a4634a4 100644
> >> --- a/include/linux/dma-map-ops.h
> >> +++ b/include/linux/dma-map-ops.h
> >> @@ -198,6 +198,7 @@ int dma_release_from_global_coherent(int order,
> >> void *vaddr);
> >>   int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
> >>                  size_t size, int *ret);
> >>   int dma_init_global_coherent(phys_addr_t phys_addr, size_t size);
> >> +bool dma_global_pool_available(void);
> >>   #else
> >>   static inline void *dma_alloc_from_global_coherent(struct device *dev,
> >>                  ssize_t size, dma_addr_t *dma_handle)
> >> @@ -213,6 +214,10 @@ static inline int
> >> dma_mmap_from_global_coherent(struct vm_area_struct *vma,
> >>   {
> >>          return 0;
> >>   }
> >> +static inline bool dma_global_pool_available(void)
> >> +{
> >> +       return false;
> >> +}
> >>   #endif /* CONFIG_DMA_GLOBAL_POOL */
> >>
> >>   /*
> >> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
> >> index c21abc77c53e..605f243b8262 100644
> >> --- a/kernel/dma/coherent.c
> >> +++ b/kernel/dma/coherent.c
> >> @@ -277,6 +277,14 @@ int dma_mmap_from_dev_coherent(struct device
> >> *dev, struct vm_area_struct *vma,
> >>   #ifdef CONFIG_DMA_GLOBAL_POOL
> >>   static struct dma_coherent_mem *dma_coherent_default_memory __ro_after_init;
> >>
> >> +bool dma_global_pool_available(void)
> >> +{
> >> +       if (!dma_coherent_default_memory)
> >> +               return false;
> >> +
> >> +       return true;
> >> +}
> >> +
> >>   void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
> >>                                       dma_addr_t *dma_handle)
> >>   {
> >> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> >> index 9596ae1aa0da..a599bb731ceb 100644
> >> --- a/kernel/dma/direct.c
> >> +++ b/kernel/dma/direct.c
> >> @@ -235,7 +235,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> >>                   * If there is a global pool, always allocate from it for
> >>                   * non-coherent devices.
> >>                   */
> >> -               if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL))
> >> +               if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> >> dma_global_pool_available())
> >>                          return dma_alloc_from_global_coherent(dev, size,
> >>                                          dma_handle);
> >
> > dma_alloc_from_global_coherent() already checks dma_coherent_default_memory, so
> > the solution could be even simpler:
> >
> > --- a/kernel/dma/direct.c
> > +++ b/kernel/dma/direct.c
> > @@ -232,12 +232,12 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> >                                             attrs);
> >
> >               /*
> > -              * If there is a global pool, always allocate from it for
> > +              * If there is a global pool, always try to allocate from it for
> >                * non-coherent devices.
> >                */
> > -             if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL))
> > -                     return dma_alloc_from_global_coherent(dev, size,
> > -                                     dma_handle);
> > +             ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
> > +             if (ret)
> > +                     return ret;
>
> So if allocation fails because the pool is full, we should go ahead and
> remap something that can't work? ;)
>
> The dma_global_pool_available() idea sort of works, but I'm still
> concerned about the case where it *should* have been available but the
> platform has been misconfigured, so again we fall through to
>
If the platform is misconfigured it is bound to fail anyway so should
we consider that as a valid case?

> DMA_DIRECT_REMAP "successfully" returning a coherent buffer that isn't,
> and the user's filesystem gets corrupted. Or at best, they get confused
> by weird errors from random devices going wrong. That's why I said it
> would be fiddly - the current state of DMA_GLOBAL_POOL as a binary
> arch-wide thing is relatively robust and easy to reason about, but
> attempting to generalise it further is... less so.
>
> Thanks,
> Robin.
>

Cheers,
Prabhakar

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

[Christoph Hellwig]

No.  The global pool is a last resort only for nommu and other very
limited setups.  It should never be enable in a general-purpose kernel,
and if your hardware requires it is a failed designed and should not
be supported in a general purpose kernel build.

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

[Jisheng Zhang]

On Wed, Oct 04, 2023 at 02:49:56PM +0100, Robin Murphy wrote:
> On 04/10/2023 2:02 pm, Lad, Prabhakar wrote:
> > + CC linux-mm and Robin Murphy
> > 
> > On Wed, Oct 4, 2023 at 12:42 PM Jisheng Zhang <jszhang@kernel.org> wrote:
> > > 
> > > On Mon, Oct 02, 2023 at 09:37:44PM -0700, Drew Fustini wrote:
> > > > On Fri, Sep 22, 2023 at 05:48:21PM -0500, Robert Nelson wrote:
> > > > > On Fri, Sep 22, 2023 at 2:08 PM Robert Nelson <robertcnelson@gmail.com> wrote:
> > > > > > 
> > > > > > On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <dfustini@baylibre.com> wrote:
> > > > > > > 
> > > > > > > This series adds support for the eMMC on the BeagleV Ahead and the
> > > > > > > Sipeed LicheePi 4A. This allows the kernel to boot with the rootfs on
> > > > > > > eMMC.
> > > > > > > 
> > > > > > > I tested on top of v6.6-rc2 with this config [1]. I was able to boot
> > > > > > > both the Ahead [2] and LPi4a [3] from eMMC. The following prerequisites
> > > > > > > are required:
> > > > > > > 
> > > > > > >    [PATCH v2] riscv: dts: thead: set dma-noncoherent to soc bus [4]
> > > > > > > 
> > > > > > > I pushed a branch [5] with this patch series and the above patch for
> > > > > > > those that find a git branch easier to test.
> > > > > > > 
> > > > > > > Please note that only the MMC controller connected to the eMMC device
> > > > > > > is enabled in the device trees for these two boards. I did not yet
> > > > > > > attempt to configure and use the microSD card slot. My preference is to
> > > > > > > address that in a future patch series.
> > > > > > > 
> > > > > > > References:
> > > > > > > [1] https://gist.github.com/pdp7/5fbdcf2a65eb1abdd3a29d519c19cdd2
> > > > > > > [2] https://gist.github.com/pdp7/91a801a5f8d1070c53509eda9800ad78
> > > > > > > [3] https://gist.github.com/pdp7/1445c3c991e88fd69c60165cef65726a
> > > > > > > [4] https://lore.kernel.org/linux-riscv/20230912072232.2455-1-jszhang@kernel.org/
> > > > > > > [5] https://github.com/pdp7/linux/tree/b4/th1520-mmc
> > > > > > 
> > > > > > This patchset came out very nice!
> > > > > > 
> > > > > > v6.6-rc2 with Last RFC v2:
> > > > > > 
> > > > > > [    4.066630] mmc0: SDHCI controller on ffe7080000.mmc
> > > > > > [ffe7080000.mmc] using PIO
> > > > > > 
> > > > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > > > > 
> > > > > > /dev/mmcblk0:
> > > > > >   Timing cached reads:   1516 MB in  2.00 seconds = 758.09 MB/sec
> > > > > >   Timing buffered disk reads:  84 MB in  3.01 seconds =  27.94 MB/sec
> > > > > > 
> > > > > > vs v6.6-rc2 with this patchset:
> > > > > > 
> > > > > >   [    4.096837] mmc0: SDHCI controller on ffe7080000.mmc
> > > > > > [ffe7080000.mmc] using DMA
> > > > > > 
> > > > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > > > > 
> > > > > > /dev/mmcblk0:
> > > > > >   Timing cached reads:   1580 MB in  2.00 seconds = 790.97 MB/sec
> > > > > >   Timing buffered disk reads: 418 MB in  3.00 seconds = 139.11 MB/sec
> > > > > 
> > > > > Drew pointed out on Slack, this was not quite right.. After more
> > > > > digging by Drew, CONFIG_DMA_GLOBAL_POOL is causing a DMA limitation
> > > > > with the multiplatform defconfig. so with,
> > > > > 
> > > > > ./scripts/config --disable CONFIG_ARCH_R9A07G043
> > > > > 
> > > > > (to remove CONFIG_DMA_GLOBAL_POOL)... another 2x in buffered reads..
> > > > > 
> > > > > [    4.059242] mmc0: SDHCI controller on ffe7080000.mmc
> > > > > [ffe7080000.mmc] using ADMA 64-bit
> > > > > 
> > > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > > > 
> > > > > /dev/mmcblk0:
> > > > >   Timing cached reads:   1600 MB in  2.00 seconds = 800.93 MB/sec
> > > > >   Timing buffered disk reads: 892 MB in  3.00 seconds = 297.06 MB/sec
> > > > 
> > > > It seems CONFIG_DMA_GLOBAL_POOL=y causes ADMA buffer alloc to fail [1]:
> > > > 
> > > >    mmc0: Unable to allocate ADMA buffers - falling back to standard DMA
> > > > 
> > > > Prabhakar's AX45MP non-coherent DMA support [2] series introduced the
> > > > selection of DMA_GLOBAL_POOL for ARCH_R9A07G043 and the riscv defconfig
> > > > selects ARCH_R9A07G043.
> > > > 
> > > > Patch 5 in the series [3] states that:
> > > > 
> > > >    With DMA_GLOBAL_POOL enabled all DMA allocations happen from this
> > > >    region and synchronization callbacks are implemented to synchronize
> > > >    when doing DMA transactions.
> > > > 
> > > > This example of a "shared-dma-pool" node was given:
> > > > 
> > > >          pma_resv0@58000000 {
> > > >              compatible = "shared-dma-pool";
> > > >              reg = <0x0 0x58000000 0x0 0x08000000>;
> > > >              no-map;
> > > >              linux,dma-default;
> > > >          };
> > > > 
> > > > I've copied that to th1520-beaglev-ahead.dts. The address of 0x58000000
> > > > has no significance on th1520, but the existence of shared-dma-pool
> > > > seems to fix the problem. ADMA mode [4] is now working even though
> > > > CONFIG_DMA_GLOBAL_POOL=y.
> > > 
> > > + Christoph, Lad
> > > 
> > > IMHO, this is not TH1520 specific but a generic issue.
> > > 
> > > 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
> designed for nommu cases where non-cacheable memory lives in a fixed place
> in the physical address map, and regular kernel pages can't be remapped. As
> far as I'm aware, RISCV_DMA_NONCOHERENT is the thing you want, such that
> DMA_DIRECT_REMAP can dynamically provide non-cacheable coherent buffers for
> non-hardware-coherent devices.

Thank Robin!
AFAIK, ARCH_R9A07G043 needs the dma global pool to handle its CMO. So
it looks like ARCH_R9A07G043 can't be enabled in riscv generic
defconfig. And we also need a special solution to prevent random config
from selecting ARCH_R9A07G043 by chance for other platforms

Thanks
> 
> Thanks,
> Robin.
> 
> > > 
> > > Thanks
> > > 
> > > > 
> > > > Thanks,
> > > > Drew
> > > > 
> > > > [1] https://gist.github.com/pdp7/73041ed808bbc7dd445836fb90574979
> > > > [2] https://lore.kernel.org/linux-riscv/20230818135723.80612-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
> > > > [3] https://lore.kernel.org/linux-riscv/20230818135723.80612-6-prabhakar.mahadev-lad.rj@bp.renesas.com/
> > > > [4] https://gist.github.com/pdp7/91e72a663d3bb73eb28182337ad8bbcb

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

[Geert Uytterhoeven]

Hi Jisheng,

On Wed, Oct 4, 2023 at 4:18 PM Jisheng Zhang <jszhang@kernel.org> wrote:
> On Wed, Oct 04, 2023 at 02:49:56PM +0100, Robin Murphy wrote:
> > On 04/10/2023 2:02 pm, Lad, Prabhakar wrote:
> > > + CC linux-mm and Robin Murphy
> > >
> > > On Wed, Oct 4, 2023 at 12:42 PM Jisheng Zhang <jszhang@kernel.org> wrote:
 > > >
> > > > On Mon, Oct 02, 2023 at 09:37:44PM -0700, Drew Fustini wrote:
> > > > > On Fri, Sep 22, 2023 at 05:48:21PM -0500, Robert Nelson wrote:
> > > > > > On Fri, Sep 22, 2023 at 2:08 PM Robert Nelson <robertcnelson@gmail.com> wrote:
> > > > > > >
> > > > > > > On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <dfustini@baylibre.com> wrote:
> > > > > > > >
> > > > > > > > This series adds support for the eMMC on the BeagleV Ahead and the
> > > > > > > > Sipeed LicheePi 4A. This allows the kernel to boot with the rootfs on
> > > > > > > > eMMC.
> > > > > > > >
> > > > > > > > I tested on top of v6.6-rc2 with this config [1]. I was able to boot
> > > > > > > > both the Ahead [2] and LPi4a [3] from eMMC. The following prerequisites
> > > > > > > > are required:
> > > > > > > >
> > > > > > > >    [PATCH v2] riscv: dts: thead: set dma-noncoherent to soc bus [4]
> > > > > > > >
> > > > > > > > I pushed a branch [5] with this patch series and the above patch for
> > > > > > > > those that find a git branch easier to test.
> > > > > > > >
> > > > > > > > Please note that only the MMC controller connected to the eMMC device
> > > > > > > > is enabled in the device trees for these two boards. I did not yet
> > > > > > > > attempt to configure and use the microSD card slot. My preference is to
> > > > > > > > address that in a future patch series.
> > > > > > > >
> > > > > > > > References:
> > > > > > > > [1] https://gist.github.com/pdp7/5fbdcf2a65eb1abdd3a29d519c19cdd2
> > > > > > > > [2] https://gist.github.com/pdp7/91a801a5f8d1070c53509eda9800ad78
> > > > > > > > [3] https://gist.github.com/pdp7/1445c3c991e88fd69c60165cef65726a
> > > > > > > > [4] https://lore.kernel.org/linux-riscv/20230912072232.2455-1-jszhang@kernel.org/
> > > > > > > > [5] https://github.com/pdp7/linux/tree/b4/th1520-mmc
> > > > > > >
> > > > > > > This patchset came out very nice!
> > > > > > >
> > > > > > > v6.6-rc2 with Last RFC v2:
> > > > > > >
> > > > > > > [    4.066630] mmc0: SDHCI controller on ffe7080000.mmc
> > > > > > > [ffe7080000.mmc] using PIO
> > > > > > >
> > > > > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > > > > >
> > > > > > > /dev/mmcblk0:
> > > > > > >   Timing cached reads:   1516 MB in  2.00 seconds = 758.09 MB/sec
> > > > > > >   Timing buffered disk reads:  84 MB in  3.01 seconds =  27.94 MB/sec
> > > > > > >
> > > > > > > vs v6.6-rc2 with this patchset:
> > > > > > >
> > > > > > >   [    4.096837] mmc0: SDHCI controller on ffe7080000.mmc
> > > > > > > [ffe7080000.mmc] using DMA
> > > > > > >
> > > > > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > > > > >
> > > > > > > /dev/mmcblk0:
> > > > > > >   Timing cached reads:   1580 MB in  2.00 seconds = 790.97 MB/sec
> > > > > > >   Timing buffered disk reads: 418 MB in  3.00 seconds = 139.11 MB/sec
> > > > > >
> > > > > > Drew pointed out on Slack, this was not quite right.. After more
> > > > > > digging by Drew, CONFIG_DMA_GLOBAL_POOL is causing a DMA limitation
> > > > > > with the multiplatform defconfig. so with,
> > > > > >
> > > > > > ./scripts/config --disable CONFIG_ARCH_R9A07G043
> > > > > >
> > > > > > (to remove CONFIG_DMA_GLOBAL_POOL)... another 2x in buffered reads..
> > > > > >
> > > > > > [    4.059242] mmc0: SDHCI controller on ffe7080000.mmc
> > > > > > [ffe7080000.mmc] using ADMA 64-bit
> > > > > >
> > > > > > debian@BeagleV:~$ sudo hdparm -tT /dev/mmcblk0
> > > > > >
> > > > > > /dev/mmcblk0:
> > > > > >   Timing cached reads:   1600 MB in  2.00 seconds = 800.93 MB/sec
> > > > > >   Timing buffered disk reads: 892 MB in  3.00 seconds = 297.06 MB/sec
> > > > >
> > > > > It seems CONFIG_DMA_GLOBAL_POOL=y causes ADMA buffer alloc to fail [1]:
> > > > >
> > > > >    mmc0: Unable to allocate ADMA buffers - falling back to standard DMA
> > > > >
> > > > > Prabhakar's AX45MP non-coherent DMA support [2] series introduced the
> > > > > selection of DMA_GLOBAL_POOL for ARCH_R9A07G043 and the riscv defconfig
> > > > > selects ARCH_R9A07G043.
> > > > >
> > > > > Patch 5 in the series [3] states that:
> > > > >
> > > > >    With DMA_GLOBAL_POOL enabled all DMA allocations happen from this
> > > > >    region and synchronization callbacks are implemented to synchronize
> > > > >    when doing DMA transactions.
> > > > >
> > > > > This example of a "shared-dma-pool" node was given:
> > > > >
> > > > >          pma_resv0@58000000 {
> > > > >              compatible = "shared-dma-pool";
> > > > >              reg = <0x0 0x58000000 0x0 0x08000000>;
> > > > >              no-map;
> > > > >              linux,dma-default;
> > > > >          };
> > > > >
> > > > > I've copied that to th1520-beaglev-ahead.dts. The address of 0x58000000
> > > > > has no significance on th1520, but the existence of shared-dma-pool
> > > > > seems to fix the problem. ADMA mode [4] is now working even though
> > > > > CONFIG_DMA_GLOBAL_POOL=y.
> > > >
> > > > + Christoph, Lad
> > > >
> > > > IMHO, this is not TH1520 specific but a generic issue.
> > > >
> > > > 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
> > designed for nommu cases where non-cacheable memory lives in a fixed place
> > in the physical address map, and regular kernel pages can't be remapped. As
> > far as I'm aware, RISCV_DMA_NONCOHERENT is the thing you want, such that
> > DMA_DIRECT_REMAP can dynamically provide non-cacheable coherent buffers for
> > non-hardware-coherent devices.
>
> Thank Robin!
> AFAIK, ARCH_R9A07G043 needs the dma global pool to handle its CMO. So
> it looks like ARCH_R9A07G043 can't be enabled in riscv generic
> defconfig. And we also need a special solution to prevent random config
> from selecting ARCH_R9A07G043 by chance for other platforms

There will be a similar issue with e.g. Starlight, as ERRATA_STARFIVE_JH7100
(not yet upstream, but in esmil/visionfive) also selects DMA_GLOBAL_POOL.

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