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/[email protected]/
[5] https://github.com/pdp7/linux/tree/b4/th1520-mmc
Changes since RFC v2:
- ADMA mode now works correctly due to a patch from Jisheng on the list
("riscv: dts: thead: set dma-noncoherent to soc bus") and this commit
from Icenowy that is now merged: 8eb8fe67e2c8 ("riscv: errata: fix
T-Head dcache.cva encoding").
- Expose __sdhci_execute_tuning from sdhci.c so that it can be called
from th1520_execute_tuning()
- Refactor the define macros for all the PHY related registers to make
it easier to understand the bit fields that the code is manipulating
- Replace magic numbers in the PHY register writes with proper defines
- Replace non_removable in dwcmshc_priv with check of mmc_host.caps
- Drop dt prop "thead,io-fixed-1v8" and instead check for existing
properties: "mmc-ddr-1_8v", "mmc-hs200-1_8v", or "mmc-hs400-1_8v"
- Rename dt prop from "thead,pull-up" to "thead,phy-pull-up" and
improve the description in the dt binding
- Replace pull_up_en in dwcmshc_priv with bit field in new flags field
- Create th1520_set_uhs_signaling() and call dwcmshc_set_uhs_signaling()
from it instead of adding th1520 code to dwcmshc_set_uhs_signaling()
- Return -EIO instead of -1 upon errors in th1520_execute_tuning()
Changes in RFC v2:
https://lore.kernel.org/linux-riscv/[email protected]/
- Expand dwcmshc_priv based on driver in the T-Head 5.10 kernel:
delay_line, non_removable, pull_up_en, io_fixed_1v8
- New boolean property "thead,pull-up" indicates phy pull-up config
- New boolean property "thead,io-fixed-1v8" indicates that io voltage
should be set to 1.8V during reset
- Add th1520_phy_1_8v_init() as voltage_switch op
- Add th1520_execute_tuning() as the platform_execute_tuning op
- Added th1520_sdhci_reset() as the .reset op. This function will set
io voltage to 1.8V after calling the standard sdhci_reset() function.
- Modified dwcmshc_set_uhs_signaling() to enable SDHCI_CTRL_VDD_180 when
io_fixed_1v8 is true
- Add many defines for register offsets and settings based on the mmc
support in the T-Head downstream v5.10 kernel
RFC v1 series:
https://lore.kernel.org/r/[email protected]
Signed-off-by: Drew Fustini <[email protected]>
---
Drew Fustini (6):
dt-bindings: mmc: sdhci-of-dwcmhsc: Add T-Head TH1520 support
mmc: sdhci: add __sdhci_execute_tuning() to header
mmc: sdhci-of-dwcmshc: Add support for T-Head TH1520
riscv: dts: thead: Add TH1520 mmc controller and sdhci clock
riscv: dts: thead: Enable BeagleV Ahead eMMC controller
riscv: dts: thead: Enable LicheePi 4A eMMC controller
.../bindings/mmc/snps,dwcmshc-sdhci.yaml | 4 +
arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts | 15 +
.../boot/dts/thead/th1520-lichee-module-4a.dtsi | 15 +
arch/riscv/boot/dts/thead/th1520.dtsi | 15 +
drivers/mmc/host/sdhci-of-dwcmshc.c | 456 +++++++++++++++++++++
drivers/mmc/host/sdhci.c | 2 +-
drivers/mmc/host/sdhci.h | 1 +
7 files changed, 507 insertions(+), 1 deletion(-)
---
base-commit: 3d01adbee80b2237c43e2e06d59e05aa243a0fe6
change-id: 20230921-th1520-mmc-518806aa55a8
Best regards,
--
Drew Fustini <[email protected]>
On Fri, Sep 22, 2023 at 07:41:37PM +0800, Xi Ruoyao wrote:
> On Thu, 2023-09-21 at 18:49 -0700, Drew Fustini 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/[email protected]/
> > [5] https://github.com/pdp7/linux/tree/b4/th1520-mmc
>
> I've tested this branch and successfully booted a rootfs on Lichee Pi 4A
> eMMC with rootdelay=10.
>
> Curiously is there some way to make it work without rootdelay?
Thank you for testing.
This is the kernel command line that I am using on both the lpi4 and
the ahead:
root=/dev/mmcblk0p3 ro rootfstype=ext4 rootwait console=ttyS0,115200
I seem to recall that before I used rootwait that there would be a VFS
oops because mmcblk0p3 didn't exist yet.
Have you tried rootwait instead of the 10 second delay?
I imagine an enforced delay would be very annoying. With "rootwait", I
don't notice any delay, it boots to the login prompt faster than I can
read the text scrolling by during boot. (my rootfs is a simple
buildroot).
thanks,
drew
On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <[email protected]> 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/[email protected]/
> [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
Regards,
--
Robert Nelson
https://rcn-ee.com/
On Fri, Sep 22, 2023 at 2:08 PM Robert Nelson <[email protected]> wrote:
>
> On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <[email protected]> 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/[email protected]/
> > [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
Regards,
--
Robert Nelson
https://rcn-ee.com/
在 2023-09-22星期五的 19:41 +0800，Xi Ruoyao写道：
> On Thu, 2023-09-21 at 18:49 -0700, Drew Fustini 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/[email protected]/
> > [5] https://github.com/pdp7/linux/tree/b4/th1520-mmc
>
> I've tested this branch and successfully booted a rootfs on Lichee Pi
> 4A
> eMMC with rootdelay=10.
>
> Curiously is there some way to make it work without rootdelay?
The answer is nearly no (although using an initrd will mitigate the
need of rootdelay).
MMC devices are known to be slow to probe, even on x86 devices.
>
> For everything except "Enable BeagleV Ahead eMMC controller":
>
> Tested-by: Xi Ruoyao <[email protected]>
>
On Thu, 2023-09-21 at 18:49 -0700, Drew Fustini 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/[email protected]/
> [5] https://github.com/pdp7/linux/tree/b4/th1520-mmc
I've tested this branch and successfully booted a rootfs on Lichee Pi 4A
eMMC with rootdelay=10.
Curiously is there some way to make it work without rootdelay?
For everything except "Enable BeagleV Ahead eMMC controller":
Tested-by: Xi Ruoyao <[email protected]>
--
Xi Ruoyao <[email protected]>
School of Aerospace Science and Technology, Xidian University
On Fri, 2023-09-22 at 09:23 -0700, Drew Fustini wrote:
> > I've tested this branch and successfully booted a rootfs on Lichee Pi 4A
> > eMMC with rootdelay=10.
> >
> > Curiously is there some way to make it work without rootdelay?
>
> Thank you for testing.
>
> This is the kernel command line that I am using on both the lpi4 and
> the ahead:
>
> root=/dev/mmcblk0p3 ro rootfstype=ext4 rootwait console=ttyS0,115200
>
> I seem to recall that before I used rootwait that there would be a VFS
> oops because mmcblk0p3 didn't exist yet.
>
> Have you tried rootwait instead of the 10 second delay?
I just tried rootwait several times and it works for me.
(When I tried it yesterday it did not work initially, so I switched to
rootdelay. I guess I'd mistyped something in the initial attempt).
--
Xi Ruoyao <[email protected]>
School of Aerospace Science and Technology, Xidian University
On Fri, Sep 22, 2023 at 05:48:21PM -0500, Robert Nelson wrote:
> On Fri, Sep 22, 2023 at 2:08 PM Robert Nelson <[email protected]> wrote:
> >
> > On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <[email protected]> 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/[email protected]/
> > > [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.
Thanks,
Drew
[1] https://gist.github.com/pdp7/73041ed808bbc7dd445836fb90574979
[2] https://lore.kernel.org/linux-riscv/[email protected]/
[3] https://lore.kernel.org/linux-riscv/[email protected]/
[4] https://gist.github.com/pdp7/91e72a663d3bb73eb28182337ad8bbcb
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 <[email protected]> wrote:
> > >
> > > On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <[email protected]> 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/[email protected]/
> > > > [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/[email protected]/
> [3] https://lore.kernel.org/linux-riscv/[email protected]/
> [4] https://gist.github.com/pdp7/91e72a663d3bb73eb28182337ad8bbcb
+ CC linux-mm and Robin Murphy
On Wed, Oct 4, 2023 at 12:42 PM Jisheng Zhang <[email protected]> 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 <[email protected]> wrote:
> > > >
> > > > On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <[email protected]> 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/[email protected]/
> > > > > [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/[email protected]/
> > [3] https://lore.kernel.org/linux-riscv/[email protected]/
> > [4] https://gist.github.com/pdp7/91e72a663d3bb73eb28182337ad8bbcb
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 <[email protected]> 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 <[email protected]> wrote:
>>>>>
>>>>> On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <[email protected]> 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/[email protected]/
>>>>>> [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/[email protected]/
>>> [3] https://lore.kernel.org/linux-riscv/[email protected]/
>>> [4] https://gist.github.com/pdp7/91e72a663d3bb73eb28182337ad8bbcb
在 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 <[email protected]>
> > 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
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini
> > > > > > <[email protected]> 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/[email protected]/
> > > > > > > [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/[email protected]/
> > > > [3]
> > > > https://lore.kernel.org/linux-riscv/[email protected]/
> > > > [4]
> > > > https://gist.github.com/pdp7/91e72a663d3bb73eb28182337ad8bbcb
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 <[email protected]> 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 <[email protected]> wrote:
> > > > > >
> > > > > > On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <[email protected]> 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/[email protected]/
> > > > > > > [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/[email protected]/
> > > > [3] https://lore.kernel.org/linux-riscv/[email protected]/
> > > > [4] https://gist.github.com/pdp7/91e72a663d3bb73eb28182337ad8bbcb
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.
在 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.
在 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.
Hi Jisheng,
On Wed, Oct 4, 2023 at 4:18 PM Jisheng Zhang <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote:
> > > > > > >
> > > > > > > On Thu, Sep 21, 2023 at 8:51 PM Drew Fustini <[email protected]> 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/[email protected]/
> > > > > > > > [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 -- [email protected]
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 Wed, Oct 4, 2023 at 3:18 PM Robin Murphy <[email protected]> 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
On Wed, Oct 4, 2023 at 5:03 PM Lad, Prabhakar
<[email protected]> wrote:
>
> On Wed, Oct 4, 2023 at 3:18 PM Robin Murphy <[email protected]> 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
On 2023-10-04 12:16 PM, Lad, Prabhakar wrote:
> On Wed, Oct 4, 2023 at 5:03 PM Lad, Prabhakar
> <[email protected]> wrote:
>>
>> On Wed, Oct 4, 2023 at 3:18 PM Robin Murphy <[email protected]> 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
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
>> <[email protected]> wrote:
>>>
>>> On Wed, Oct 4, 2023 at 3:18 PM Robin Murphy <[email protected]> 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
>
On Wed, Oct 4, 2023 at 8:38 PM Robin Murphy <[email protected]> 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
> >> <[email protected]> wrote:
> >>>
> >>> On Wed, Oct 4, 2023 at 3:18 PM Robin Murphy <[email protected]> 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
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.