* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC [not found] ` <20220808122652.GA6599@plvision.eu> @ 2022-08-08 12:58 ` Adrian Hunter 2022-08-08 14:06 ` Robin Murphy 0 siblings, 1 reply; 15+ messages in thread From: Adrian Hunter @ 2022-08-08 12:58 UTC (permalink / raw) To: Vadym Kochan, Christoph Hellwig, Robin Murphy Cc: Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu On 8/08/22 15:26, Vadym Kochan wrote: > On Mon, Aug 08, 2022 at 02:40:07PM +0300, Adrian Hunter wrote: >> On 8/08/22 12:52, Vadym Kochan wrote: >>> Hi Adrian, >>> >>> On Mon, Aug 08, 2022 at 12:19:10PM +0300, Adrian Hunter wrote: >>>> On 1/08/22 12:30, Vadym Kochan wrote: >>>>> Hi Florian, >>>>> >>>>> On Wed, Jul 27, 2022 at 07:45:32PM +0300, Vadym Kochan wrote: >>>>>> Hi Florian, >>>>>> >>>>>> On Tue, Jul 26, 2022 at 10:37:46AM -0700, Florian Fainelli wrote: >>>>>>> On 7/26/22 10:07, Vadym Kochan wrote: >>>>>>>> From: Elad Nachman <enachman@marvell.com> >>>>>>>> >>>>>>>> There is a limitation on AC5 SoC that mmc controller >>>>>>>> can't have DMA access over 2G memory. >>>>>>>> >>>>>>>> Signed-off-by: Elad Nachman <enachman@marvell.com> >>>>>>>> Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu> >>>>>>>> --- >>>>>>>> drivers/mmc/host/sdhci-xenon.c | 13 +++++++++++++ >>>>>>>> 1 file changed, 13 insertions(+) >>>>>>>> >>>>>>>> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c >>>>>>>> index 08e838400b52..666d06b58564 100644 >>>>>>>> --- a/drivers/mmc/host/sdhci-xenon.c >>>>>>>> +++ b/drivers/mmc/host/sdhci-xenon.c >>>>>>>> @@ -18,6 +18,7 @@ >>>>>>>> #include <linux/of.h> >>>>>>>> #include <linux/pm.h> >>>>>>>> #include <linux/pm_runtime.h> >>>>>>>> +#include <linux/mm.h> >>>>>>>> >>>>>>>> #include "sdhci-pltfm.h" >>>>>>>> #include "sdhci-xenon.h" >>>>>>>> @@ -422,6 +423,8 @@ static int xenon_probe_params(struct platform_device *pdev) >>>>>>>> struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host); >>>>>>>> u32 sdhc_id, nr_sdhc; >>>>>>>> u32 tuning_count; >>>>>>>> + struct device_node *np = pdev->dev.of_node; >>>>>>>> + struct sysinfo si; >>>>>>>> >>>>>>>> /* Disable HS200 on Armada AP806 */ >>>>>>>> if (priv->hw_version == XENON_AP806) >>>>>>>> @@ -450,6 +453,15 @@ static int xenon_probe_params(struct platform_device *pdev) >>>>>>>> } >>>>>>>> priv->tuning_count = tuning_count; >>>>>>>> >>>>>>>> + si_meminfo(&si); >>>>>>>> + >>>>>>>> + if (of_device_is_compatible(np, "marvell,ac5-sdhci") && >>>>>>>> + ((si.totalram * si.mem_unit) > 0x80000000 /*2G*/)) { >>>>>>> >>>>>>> Why not limit the DMA mask of the device and ensure, that bounce buffers get used so you can still do DMA? >>>>>>> >>>>>>> Also, you ought to be able to describe that limitation using Device Tree (assuming this is an option) and declaring a dedicated bus node for the SDHCI controller and providing a suitable dma-ranges property, see: arch/arm/boot/dts/bcm2711.dtsi and the 'soc' node for such examples. >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> I could use DMA only in 2 ways: >>>>> >>>>> #1 Use sdhci bounce buffer with SDMA mode >>>>> >>>>> But there was the issue that SDMA requires that SDHCI v4 mode should >>>>> be enabled, and when I enable it via sdhci_enable_v4_mode(host) >>>>> then I got error that EXT_CSD can't be recognized. >>>>> >>>>> But if I comment this line in sdhci.c: >>>>> >>>>> int sdhci_setup_host(struct sdhci_host *host) >>>>> { >>>>> ... >>>>> >>>>> /* SDMA does not support 64-bit DMA if v4 mode not set */ >>>>> if ((host->flags & SDHCI_USE_64_BIT_DMA) && !host->v4_mode) { >>>>> pr_info("XXX SDMA does not support 64-bit DMA if v4 mode not set\n"); >>>>> host->flags &= ~SDHCI_USE_SDMA; >>>>> } >>>>> >>>>> ... >>>>> } >>>>> >>>>> then everything is OK. >>>>> >>>>> #2 Use restricted-dma-pool in device-tree >>>>> >>>>> But I am not sure if it is good solution compared to #1. >>>>> >>>>> Setting only DMA mask did not help because after some time I got >>>>> "DMA overflow address" error stack-traces. >>>> >>>> AFAICT using a DMA mask is the correct way to solve this. If that >>>> reveals another issue then that must be resolved also. Can you show >>>> the stack traces and what kernel they are from? >>>> >>> >>> The Linux version is next-20220808 >>> >>> Here is a stacktrace: >>> >>> [ 40.537028] ------------[ cut here ]------------ >>> [ 40.541698] xenon-sdhci 805c0000.sdhci: DMA addr >>> 0x00000002ff700000+40960 overflow (mask 27fffffff, bus limit 0). >>> [ 40.551968] WARNING: CPU: 0 PID: 7 at kernel/dma/direct.h:103 >>> dma_direct_map_sg+0x28c/0x2b0 >> >> Do you have CONFIG_SWIOTLB=y? >> > > Yes. Additionally I pass "swiotlb=force", because as I understand it is > not enabled by default at least on arm64. > >> Is DMA mask correct? 2G mask would be 7fffffff not 27fffffff right? >> > > The RAM starts at 0x2_00000000 so thats why I use this mask. > > Event if to use forced swiotlb via command-line then swiotlb complains > about the mask (from what I checked is that because swiotlb will be > allocated at the top of the DMA memory, which is actually the end of > RAM. Ofcourse if to manually hard-code arm64_dma_phys_limit in > arch/arm64/mm/init.c then everything works ok and I see that swiotlb > does the job if to check via debugfs): It sounds like maybe SWIOTLB does not support your memory model? Adding SWIOTLB people. > > [ 1.129987] ------------[ cut here ]------------ > [ 1.134595] xenon-sdhci 805c0000.sdhci: swiotlb addr 0x00000002f7000000+512 overflow (mask 27fffffff, bus limit 0). > [ 1.142086] usb 1-1: new high-speed USB device number 2 using orion-ehci > [ 1.145025] WARNING: CPU: 0 PID: 24 at kernel/dma/swiotlb.c:880 swiotlb_map+0x1b4/0x1f0 > [ 1.159679] Modules linked in: > [ 1.162733] CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted > 5.19.0-next-20220808 #46 > [ 1.170202] Hardware name: Marvell A0-AC5X Board (DT) > [ 1.175243] Workqueue: events_freezable mmc_rescan > [ 1.180033] pstate: 604000c9 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS > BTYPE=--) > [ 1.186982] pc : swiotlb_map+0x1b4/0x1f0 > [ 1.190898] lr : swiotlb_map+0x1b4/0x1f0 > [ 1.194813] sp : ffff80000a8eb840 > [ 1.198119] x29: ffff80000a8eb840 x28: ffff80000a8ebb38 x27: > 0000000000000000 > [ 1.205245] x26: 0000000000000000 x25: ffff0000004de810 x24: > 0000000000000000 > [ 1.212369] x23: 0000000000000002 x22: 0000000000000002 x21: > 0000000000000000 > [ 1.219494] x20: 0000000000000200 x19: ffff0000004de810 x18: > ffffffffffffffff > [ 1.226619] x17: 203231352b303030 x16: 3030303766323030 x15: > 3030303030783020 > [ 1.233744] x14: 7264646120626c74 x13: 2e29302074696d69 x12: > 6c20737562202c66 > [ 1.240869] x11: 6666666666663732 x10: 206b73616d282077 x9 : > 6f697773203a6963 > [ 1.247994] x8 : 6864732e30303030 x7 : 205d353935343331 x6 : > ffff80000a665000 > [ 1.255118] x5 : 0000000000000000 x4 : 0000000000000000 x3 : > 00000000ffffffff > [ 1.262243] x2 : 0000000000000000 x1 : 0000000000000000 x0 : > ffff0000005a0000 > [ 1.269369] Call trace: > [ 1.271810] swiotlb_map+0x1b4/0x1f0 > [ 1.275380] dma_direct_map_sg+0x94/0x2b0 > [ 1.279382] __dma_map_sg_attrs+0x28/0xa0 > [ 1.283388] dma_map_sg_attrs+0x10/0x30 > [ 1.287219] sdhci_pre_dma_transfer+0xcc/0x150 > [ 1.291658] sdhci_send_command+0x644/0xea0 > [ 1.295834] sdhci_send_command_retry+0x40/0x140 > [ 1.300443] sdhci_request+0x70/0xd0 > [ 1.304012] __mmc_start_request+0x68/0x140 > [ 1.308188] mmc_start_request+0x78/0xa0 > [ 1.312103] mmc_wait_for_req+0x70/0x100 > [ 1.316020] mmc_send_adtc_data+0xd8/0x130 > [ 1.320109] mmc_get_ext_csd+0x70/0xe0 > [ 1.323850] mmc_init_card+0x7e8/0x1ad0 > [ 1.327680] mmc_attach_mmc+0xdc/0x180 > [ 1.331422] mmc_rescan+0x280/0x2f0 > [ 1.334905] process_one_work+0x1d0/0x320 > [ 1.338912] worker_thread+0x14c/0x450 > [ 1.342655] kthread+0x10c/0x110 > [ 1.345879] ret_from_fork+0x10/0x20 > [ 1.349450] ---[ end trace 0000000000000000 ]--- > [ 1.354091] ------------[ cut here ]------------ > > >>> [ 40.560321] Modules linked in: uio_pdrv_genirq >>> [ 40.564766] CPU: 0 PID: 7 Comm: kworker/0:0H Not tainted >>> 5.19.0-next-20220808 #20 >>> [ 40.572236] Hardware name: Marvell A0-AC5X Board (DT) >>> [ 40.577278] Workqueue: kblockd blk_mq_run_work_fn >>> [ 40.581980] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS >>> BTYPE=--) >>> [ 40.588929] pc : dma_direct_map_sg+0x28c/0x2b0 >>> [ 40.593366] lr : dma_direct_map_sg+0x28c/0x2b0 >>> [ 40.597801] sp : ffff80000a6eba20 >>> [ 40.601107] x29: ffff80000a6eba20 x28: ffff000006d66080 x27: >>> 0000000000000004 >>> [ 40.608233] x26: 0000000000000000 x25: ffff0000004de810 x24: >>> 0000000000000000 >>> [ 40.615358] x23: 0000000000000002 x22: fffffc0000000000 x21: >>> ffff800009153360 >>> [ 40.622484] x20: 0000000000000005 x19: 00000002ff700000 x18: >>> ffffffffffffffff >>> [ 40.629609] x17: 766f203036393034 x16: 2b30303030303766 x15: >>> 6632303030303030 >>> [ 40.636735] x14: 3078302072646461 x13: ffff80000a433d58 x12: >>> 00000000000002dc >>> [ 40.643860] x11: 00000000000000f4 x10: ffff80000a48bd58 x9 : >>> ffff80000a48bd58 >>> [ 40.650985] x8 : ffff80000a433d58 x7 : ffff80000a48bd58 x6 : >>> 0000000000000000 >>> [ 40.658110] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : >>> 0000000000000000 >>> [ 40.665234] x2 : 0000000000000000 x1 : 0000000000000000 x0 : >>> ffff0000004add00 >>> [ 40.672360] Call trace: >>> [ 40.674800] dma_direct_map_sg+0x28c/0x2b0 >>> [ 40.678890] __dma_map_sg_attrs+0x28/0xa0 >>> [ 40.682895] dma_map_sg_attrs+0x10/0x30 >>> [ 40.686725] sdhci_pre_dma_transfer+0xcc/0x150 >>> [ 40.691165] sdhci_pre_req+0x3c/0x50 >>> [ 40.694734] mmc_blk_mq_issue_rq+0x25c/0x9d0 >>> [ 40.698997] mmc_mq_queue_rq+0x118/0x2b0 >>> [ 40.702912] blk_mq_dispatch_rq_list+0x19c/0x800 >>> [ 40.707521] blk_mq_do_dispatch_sched+0x2bc/0x350 >>> [ 40.712218] __blk_mq_sched_dispatch_requests+0x10c/0x160 >>> [ 40.717608] blk_mq_sched_dispatch_requests+0x40/0x80 >>> [ 40.722651] __blk_mq_run_hw_queue+0x80/0xc0 >>> [ 40.726911] blk_mq_run_work_fn+0x24/0x30 >>> [ 40.730913] process_one_work+0x1d0/0x320 >>> [ 40.734919] worker_thread+0x14c/0x450 >>> [ 40.738662] kthread+0x10c/0x110 >>> [ 40.741885] ret_from_fork+0x10/0x20 >>> [ 40.745457] ---[ end trace 0000000000000000 ]--- >>> [ 40.750160] ------------[ cut here ]------------ >>> [ 40.754770] WARNING: CPU: 0 PID: 7 at drivers/mmc/host/sdhci.c:1152 >>> sdhci_send_command+0x7fc/0xea0 >>> [ 40.763719] Modules linked in: uio_pdrv_genirq >>> [ 40.768149] CPU: 0 PID: 7 Comm: kworker/0:0H Tainted: G W >>> 5.19.0-next-20220808 #20 >>> [ 40.777090] Hardware name: Marvell A0-AC5X Board (DT) >>> [ 40.782129] Workqueue: kblockd blk_mq_run_work_fn >>> [ 40.786827] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS >>> BTYPE=--) >>> [ 40.793775] pc : sdhci_send_command+0x7fc/0xea0 >>> [ 40.798297] lr : sdhci_send_command+0x644/0xea0 >>> [ 40.802821] sp : ffff80000a6eb980 >>> [ 40.806127] x29: ffff80000a6eb980 x28: ffff80000a6ebcf8 x27: >>> 0000000000000002 >>> [ 40.813253] x26: 0000000000000000 x25: 0000000000000000 x24: >>> ffff0000004237c0 >>> [ 40.820378] x23: 000000000000000b x22: ffff000006d825b8 x21: >>> ffff000006d82628 >>> [ 40.827502] x20: ffff000006d825b8 x19: ffff000000423580 x18: >>> ffffffffffffffff >>> [ 40.834627] x17: 766f203036393034 x16: 2b30303030303766 x15: >>> 6632303030303030 >>> [ 40.841751] x14: 3078302072646461 x13: ffff80000a433d58 x12: >>> 00000000000002dc >>> [ 40.848876] x11: 00000000000000f4 x10: ffff80000a48bd58 x9 : >>> ffff80000a48bd58 >>> [ 40.856001] x8 : ffff80000a433d58 x7 : 0000000000000000 x6 : >>> ffff0000004deaf8 >>> [ 40.863125] x5 : 0000000000000000 x4 : 0000000000000020 x3 : >>> 0000000000000000 >>> [ 40.870250] x2 : 0000000000020401 x1 : 0000000000000001 x0 : >>> 00000000ffffffe4 >>> [ 40.877374] Call trace: >>> [ 40.879813] sdhci_send_command+0x7fc/0xea0 >>> [ 40.883989] sdhci_send_command_retry+0x40/0x140 >>> [ 40.888597] sdhci_request+0x70/0xd0 >>> [ 40.892167] __mmc_start_request+0x68/0x140 >>> [ 40.896345] mmc_start_request+0x78/0xa0 >>> [ 40.900260] mmc_blk_mq_issue_rq+0x284/0x9d0 >>> [ 40.904521] mmc_mq_queue_rq+0x118/0x2b0 >>> [ 40.908436] blk_mq_dispatch_rq_list+0x19c/0x800 >>> [ 40.913045] blk_mq_do_dispatch_sched+0x2bc/0x350 >>> [ 40.917741] __blk_mq_sched_dispatch_requests+0x10c/0x160 >>> [ 40.923131] blk_mq_sched_dispatch_requests+0x40/0x80 >>> [ 40.928173] __blk_mq_run_hw_queue+0x80/0xc0 >>> [ 40.932435] blk_mq_run_work_fn+0x24/0x30 >>> [ 40.936437] process_one_work+0x1d0/0x320 >>> [ 40.940442] worker_thread+0x14c/0x450 >>> [ 40.944184] kthread+0x10c/0x110 >>> [ 40.947407] ret_from_fork+0x10/0x20 >>> [ 40.950977] ---[ end trace 0000000000000000 ]--- >>> [ 40.960738] ------------[ cut here ]------------ >>> [ 40.965361] WARNING: CPU: 1 PID: 49 at drivers/mmc/host/sdhci.c:1152 >>> sdhci_send_command+0x7fc/0xea0 >>> [ 40.974403] Modules linked in: uio_pdrv_genirq >>> [ 40.978843] CPU: 1 PID: 49 Comm: kworker/1:1H Tainted: G W >>> 5.19.0-next-20220808 #20 >>> [ 40.987873] Hardware name: Marvell A0-AC5X Board (DT) >>> [ 40.992913] Workqueue: kblockd blk_mq_run_work_fn >>> [ 40.997613] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS >>> BTYPE=--) >>> [ 41.004562] pc : sdhci_send_command+0x7fc/0xea0 >>> [ 41.009086] lr : sdhci_send_command+0x644/0xea0 >>> [ 41.013610] sp : ffff80000a9db980 >>> [ 41.016915] x29: ffff80000a9db980 x28: ffff80000a9dbcf8 x27: >>> 0000000000000002 >>> [ 41.024041] x26: 0000000000000000 x25: 0000000000000000 x24: >>> ffff0000004237c0 >>> [ 41.031166] x23: 000000000000000b x22: ffff000006d05eb8 x21: >>> ffff000006d05f28 >>> [ 41.038291] x20: ffff000006d05eb8 x19: ffff000000423580 x18: >>> fffffc0003fde640 >>> [ 41.045416] x17: 0000000000000000 x16: 0000000000000000 x15: >>> 0000000000000000 >>> [ 41.052540] x14: 0000000000000002 x13: 0000000000100000 x12: >>> 0000000000000000 >>> [ 41.059664] x11: 0000000000000000 x10: 0000000000000b10 x9 : >>> ffff80000a9db9e0 >>> [ 41.066789] x8 : ffff000000649af0 x7 : 0000000000000000 x6 : >>> ffff0000004deaf8 >>> [ 41.073914] x5 : 0000000000000000 x4 : 0000000000000020 x3 : >>> 0000000000000000 >>> [ 41.081039] x2 : 0000000000020401 x1 : 0000000000000001 x0 : >>> 00000000ffffffe4 >>> [ 41.088164] Call trace: >>> [ 41.090603] sdhci_send_command+0x7fc/0xea0 >>> [ 41.094779] sdhci_send_command_retry+0x40/0x140 >>> [ 41.099379] sdhci_request+0x70/0xd0 >>> [ 41.102949] __mmc_start_request+0x68/0x140 >>> [ 41.107127] mmc_start_request+0x78/0xa0 >>> [ 41.111042] mmc_blk_mq_issue_rq+0x284/0x9d0 >>> [ 41.115305] mmc_mq_queue_rq+0x118/0x2b0 >>> [ 41.119221] blk_mq_dispatch_rq_list+0x19c/0x800 >>> [ 41.123831] blk_mq_do_dispatch_sched+0x2bc/0x350 >>> [ 41.128528] __blk_mq_sched_dispatch_requests+0x10c/0x160 >>> [ 41.133917] blk_mq_sched_dispatch_requests+0x40/0x80 >>> [ 41.138959] __blk_mq_run_hw_queue+0x80/0xc0 >>> [ 41.143222] blk_mq_run_work_fn+0x24/0x30 >>> [ 41.147223] process_one_work+0x1d0/0x320 >>> [ 41.151230] worker_thread+0x14c/0x450 >>> [ 41.154973] kthread+0x10c/0x110 >>> [ 41.158195] ret_from_fork+0x10/0x20 >>> [ 41.161766] ---[ end trace 0000000000000000 ]--- >>> [ 41.171543] ------------[ cut here ]------------ >>> [ 41.176169] WARNING: CPU: 1 PID: 49 at drivers/mmc/host/sdhci.c:1152 >>> sdhci_send_command+0x7fc/0xea0 >>> [ 41.185210] Modules linked in: uio_pdrv_genirq >>> [ 41.189650] CPU: 1 PID: 49 Comm: kworker/1:1H Tainted: G W >>> 5.19.0-next-20220808 #20 >>> [ 41.198679] Hardware name: Marvell A0-AC5X Board (DT) >>> [ 41.203720] Workqueue: kblockd blk_mq_run_work_fn >>> [ 41.208420] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS >>> BTYPE=--) >>> [ 41.215369] pc : sdhci_send_command+0x7fc/0xea0 >>> [ 41.219892] lr : sdhci_send_command+0x644/0xea0 >>> [ 41.224414] sp : ffff80000a9db980 >>> [ 41.227721] x29: ffff80000a9db980 x28: ffff80000a9dbcf8 x27: >>> 0000000000000002 >>> [ 41.234846] x26: 0000000000000000 x25: 0000000000000000 x24: >>> ffff0000004237c0 >>> [ 41.241972] x23: 000000000000000b x22: ffff000006d001b8 x21: >>> ffff000006d00228 >>> [ 41.249097] x20: ffff000006d001b8 x19: ffff000000423580 x18: >>> 0000000000000000 >>> [ 41.256223] x17: 0000000000000000 x16: 0000000000000000 x15: >>> 0000000000000000 >>> [ 41.263348] x14: 0000000000000002 x13: 0000000000100000 x12: >>> 0000000000000000 >>> [ 41.270473] x11: 0000000000000001 x10: 0000000000000b10 x9 : >>> ffff80000a9db9e0 >>> [ 41.277598] x8 : ffff000000649af0 x7 : 0000000000000000 x6 : >>> ffff0000004deaf8 >>> [ 41.284722] x5 : 0000000000000000 x4 : 0000000000000020 x3 : >>> 0000000000000000 >>> [ 41.291846] x2 : 0000000000020401 x1 : 0000000000000001 x0 : >>> 00000000ffffffe4 >>> [ 41.298971] Call trace: >>> [ 41.301410] sdhci_send_command+0x7fc/0xea0 >>> [ 41.305588] sdhci_send_command_retry+0x40/0x140 >>> [ 41.310198] sdhci_request+0x70/0xd0 >>> [ 41.313769] __mmc_start_request+0x68/0x140 >>> [ 41.317946] mmc_start_request+0x78/0xa0 >>> [ 41.321862] mmc_blk_mq_issue_rq+0x284/0x9d0 >>> [ 41.326125] mmc_mq_queue_rq+0x118/0x2b0 >>> [ 41.330040] blk_mq_dispatch_rq_list+0x19c/0x800 >>> [ 41.334649] blk_mq_do_dispatch_sched+0x2bc/0x350 >>> [ 41.339346] __blk_mq_sched_dispatch_requests+0x10c/0x160 >>> [ 41.344735] blk_mq_sched_dispatch_requests+0x40/0x80 >>> [ 41.349779] __blk_mq_run_hw_queue+0x80/0xc0 >>> [ 41.354040] blk_mq_run_work_fn+0x24/0x30 >>> [ 41.358041] process_one_work+0x1d0/0x320 >>> [ 41.362048] worker_thread+0x14c/0x450 >>> [ 41.365791] kthread+0x10c/0x110 >>> [ 41.369014] ret_from_fork+0x10/0x20 >>> [ 41.372584] ---[ end trace 0000000000000000 ]--- >>> [ 41.382383] ------------[ cut here ]------------ >>> [ 41.387009] WARNING: CPU: 1 PID: 49 at drivers/mmc/host/sdhci.c:1152 >>> sdhci_send_command+0x7fc/0xea0 >>> [ 41.396051] Modules linked in: uio_pdrv_genirq >>> [ 41.400491] CPU: 1 PID: 49 Comm: kworker/1:1H Tainted: G W >>> 5.19.0-next-20220808 #20 >>> [ 41.409520] Hardware name: Marvell A0-AC5X Board (DT) >>> [ 41.414561] Workqueue: kblockd blk_mq_run_work_fn >>> [ 41.419261] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS >>> BTYPE=--) >>> [ 41.426210] pc : sdhci_send_command+0x7fc/0xea0 >>> [ 41.430734] lr : sdhci_send_command+0x644/0xea0 >>> [ 41.435256] sp : ffff80000a9db980 >>> [ 41.438562] x29: ffff80000a9db980 x28: ffff80000a9dbcf8 x27: >>> 0000000000000002 >>> [ 41.445687] x26: 0000000000000000 x25: 0000000000000000 x24: >>> ffff0000004237c0 >>> [ 41.452812] x23: 000000000000000b x22: ffff000006d004b8 x21: >>> ffff000006d00528 >>> [ 41.459937] x20: ffff000006d004b8 x19: ffff000000423580 x18: >>> 0000000000000000 >>> [ 41.467062] x17: 0000000000000000 x16: 0000000000000000 x15: >>> 0000000000000000 >>> [ 41.474187] x14: 0000000000000002 x13: 00000000000ff000 x12: >>> 0000000000000000 >>> [ 41.481311] x11: 0000000000000001 x10: 0000000000000b10 x9 : >>> ffff80000a9db9e0 >>> [ 41.488436] x8 : ffff000000649af0 x7 : 0000000000000000 x6 : >>> ffff0000004deaf8 >>> [ 41.495561] x5 : 0000000000000000 x4 : 0000000000000020 x3 : >>> 0000000000000000 >>> [ 41.502685] x2 : 0000000000020401 x1 : 0000000000000001 x0 : >>> 00000000ffffffe4 >>> [ 41.509811] Call trace: >>> [ 41.512250] sdhci_send_command+0x7fc/0xea0 >>> [ 41.516427] sdhci_send_command_retry+0x40/0x140 >>> [ 41.521035] sdhci_request+0x70/0xd0 >>> [ 41.524607] __mmc_start_request+0x68/0x140 >>> [ 41.528784] mmc_start_request+0x78/0xa0 >>> [ 41.532699] mmc_blk_mq_issue_rq+0x284/0x9d0 >>> [ 41.536961] mmc_mq_queue_rq+0x118/0x2b0 >>> [ 41.540876] blk_mq_dispatch_rq_list+0x19c/0x800 >>> [ 41.545485] blk_mq_do_dispatch_sched+0x2bc/0x350 >>> [ 41.550182] __blk_mq_sched_dispatch_requests+0x10c/0x160 >>> [ 41.555570] blk_mq_sched_dispatch_requests+0x40/0x80 >>> [ 41.560613] __blk_mq_run_hw_queue+0x80/0xc0 >>> [ 41.564874] blk_mq_run_work_fn+0x24/0x30 >>> [ 41.568875] process_one_work+0x1d0/0x320 >>> [ 41.572881] worker_thread+0x14c/0x450 >>> [ 41.576624] kthread+0x10c/0x110 >>> [ 41.579847] ret_from_fork+0x10/0x20 >>> [ 41.583428] ---[ end trace 0000000000000000 ]--- >>> >>> >>>>> >>>>>> Do I understand correctly that the swiotlb will be used in case DMA >>>>>> engine could not map the page in the specified range (limited by dma-ranges or >>>>>> dma-mask) ? >>>>>> >>>>>>> >>>>>>>> + host->quirks |= SDHCI_QUIRK_BROKEN_DMA; >>>>>>>> + host->quirks |= SDHCI_QUIRK_BROKEN_ADMA; >>>>>>>> + dev_info(mmc_dev(mmc), "Disabling DMA because of 2GB DMA access limit.\n"); >>>>>>>> + } >>>>>>>> + >>>>>>>> return xenon_phy_parse_params(dev, host); >>>>>>>> } >>>>>>>> >>>>>>>> @@ -682,6 +694,7 @@ static const struct of_device_id sdhci_xenon_dt_ids[] = { >>>>>>>> { .compatible = "marvell,armada-ap807-sdhci", .data = (void *)XENON_AP807}, >>>>>>>> { .compatible = "marvell,armada-cp110-sdhci", .data = (void *)XENON_CP110}, >>>>>>>> { .compatible = "marvell,armada-3700-sdhci", .data = (void *)XENON_A3700}, >>>>>>>> + { .compatible = "marvell,ac5-sdhci", .data = (void *)XENON_AP806}, >>>>>>>> {} >>>>>>>> }; >>>>>>>> MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids); >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Florian >>>>>> >>>>>> Thanks, >>>>>> >>>>> >>>>> Regards, >>>> >>> >>> Thanks, >>> Vadym Kochan >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-08-08 12:58 ` [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC Adrian Hunter @ 2022-08-08 14:06 ` Robin Murphy 2022-08-16 20:51 ` Vadym Kochan 0 siblings, 1 reply; 15+ messages in thread From: Robin Murphy @ 2022-08-08 14:06 UTC (permalink / raw) To: Adrian Hunter, Vadym Kochan, Christoph Hellwig Cc: Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu On 2022-08-08 13:58, Adrian Hunter wrote: > On 8/08/22 15:26, Vadym Kochan wrote: >> On Mon, Aug 08, 2022 at 02:40:07PM +0300, Adrian Hunter wrote: >>> On 8/08/22 12:52, Vadym Kochan wrote: >>>> Hi Adrian, >>>> >>>> On Mon, Aug 08, 2022 at 12:19:10PM +0300, Adrian Hunter wrote: >>>>> On 1/08/22 12:30, Vadym Kochan wrote: >>>>>> Hi Florian, >>>>>> >>>>>> On Wed, Jul 27, 2022 at 07:45:32PM +0300, Vadym Kochan wrote: >>>>>>> Hi Florian, >>>>>>> >>>>>>> On Tue, Jul 26, 2022 at 10:37:46AM -0700, Florian Fainelli wrote: >>>>>>>> On 7/26/22 10:07, Vadym Kochan wrote: >>>>>>>>> From: Elad Nachman <enachman@marvell.com> >>>>>>>>> >>>>>>>>> There is a limitation on AC5 SoC that mmc controller >>>>>>>>> can't have DMA access over 2G memory. >>>>>>>>> >>>>>>>>> Signed-off-by: Elad Nachman <enachman@marvell.com> >>>>>>>>> Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu> >>>>>>>>> --- >>>>>>>>> drivers/mmc/host/sdhci-xenon.c | 13 +++++++++++++ >>>>>>>>> 1 file changed, 13 insertions(+) >>>>>>>>> >>>>>>>>> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c >>>>>>>>> index 08e838400b52..666d06b58564 100644 >>>>>>>>> --- a/drivers/mmc/host/sdhci-xenon.c >>>>>>>>> +++ b/drivers/mmc/host/sdhci-xenon.c >>>>>>>>> @@ -18,6 +18,7 @@ >>>>>>>>> #include <linux/of.h> >>>>>>>>> #include <linux/pm.h> >>>>>>>>> #include <linux/pm_runtime.h> >>>>>>>>> +#include <linux/mm.h> >>>>>>>>> >>>>>>>>> #include "sdhci-pltfm.h" >>>>>>>>> #include "sdhci-xenon.h" >>>>>>>>> @@ -422,6 +423,8 @@ static int xenon_probe_params(struct platform_device *pdev) >>>>>>>>> struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host); >>>>>>>>> u32 sdhc_id, nr_sdhc; >>>>>>>>> u32 tuning_count; >>>>>>>>> + struct device_node *np = pdev->dev.of_node; >>>>>>>>> + struct sysinfo si; >>>>>>>>> >>>>>>>>> /* Disable HS200 on Armada AP806 */ >>>>>>>>> if (priv->hw_version == XENON_AP806) >>>>>>>>> @@ -450,6 +453,15 @@ static int xenon_probe_params(struct platform_device *pdev) >>>>>>>>> } >>>>>>>>> priv->tuning_count = tuning_count; >>>>>>>>> >>>>>>>>> + si_meminfo(&si); >>>>>>>>> + >>>>>>>>> + if (of_device_is_compatible(np, "marvell,ac5-sdhci") && >>>>>>>>> + ((si.totalram * si.mem_unit) > 0x80000000 /*2G*/)) { >>>>>>>> >>>>>>>> Why not limit the DMA mask of the device and ensure, that bounce buffers get used so you can still do DMA? >>>>>>>> >>>>>>>> Also, you ought to be able to describe that limitation using Device Tree (assuming this is an option) and declaring a dedicated bus node for the SDHCI controller and providing a suitable dma-ranges property, see: arch/arm/boot/dts/bcm2711.dtsi and the 'soc' node for such examples. >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> I could use DMA only in 2 ways: >>>>>> >>>>>> #1 Use sdhci bounce buffer with SDMA mode >>>>>> >>>>>> But there was the issue that SDMA requires that SDHCI v4 mode should >>>>>> be enabled, and when I enable it via sdhci_enable_v4_mode(host) >>>>>> then I got error that EXT_CSD can't be recognized. >>>>>> >>>>>> But if I comment this line in sdhci.c: >>>>>> >>>>>> int sdhci_setup_host(struct sdhci_host *host) >>>>>> { >>>>>> ... >>>>>> >>>>>> /* SDMA does not support 64-bit DMA if v4 mode not set */ >>>>>> if ((host->flags & SDHCI_USE_64_BIT_DMA) && !host->v4_mode) { >>>>>> pr_info("XXX SDMA does not support 64-bit DMA if v4 mode not set\n"); >>>>>> host->flags &= ~SDHCI_USE_SDMA; >>>>>> } >>>>>> >>>>>> ... >>>>>> } >>>>>> >>>>>> then everything is OK. >>>>>> >>>>>> #2 Use restricted-dma-pool in device-tree >>>>>> >>>>>> But I am not sure if it is good solution compared to #1. >>>>>> >>>>>> Setting only DMA mask did not help because after some time I got >>>>>> "DMA overflow address" error stack-traces. >>>>> >>>>> AFAICT using a DMA mask is the correct way to solve this. If that >>>>> reveals another issue then that must be resolved also. Can you show >>>>> the stack traces and what kernel they are from? >>>>> >>>> >>>> The Linux version is next-20220808 >>>> >>>> Here is a stacktrace: >>>> >>>> [ 40.537028] ------------[ cut here ]------------ >>>> [ 40.541698] xenon-sdhci 805c0000.sdhci: DMA addr >>>> 0x00000002ff700000+40960 overflow (mask 27fffffff, bus limit 0). >>>> [ 40.551968] WARNING: CPU: 0 PID: 7 at kernel/dma/direct.h:103 >>>> dma_direct_map_sg+0x28c/0x2b0 >>> >>> Do you have CONFIG_SWIOTLB=y? >>> >> >> Yes. Additionally I pass "swiotlb=force", because as I understand it is >> not enabled by default at least on arm64. If you think you need "swiotlb=force" then something's definitely wrong. SWIOTLB should be invoked automatically based on device DMA masks being set correctly. If you need to support devices with masks smaller than 32 bits you should have CONFIG_ZONE_DMA enabled as well. >>> Is DMA mask correct? 2G mask would be 7fffffff not 27fffffff right? >>> >> >> The RAM starts at 0x2_00000000 so thats why I use this mask. You can't have holes in DMA masks, they don't work that way. Presumably either the device interface itself drives DMA_BIT_MASK(34) or wider but the interconnect address map doesn't decode the full range, or the device drives DMA_BIT_MASK(31) and the interconnect adds a fixed offset of 0x200000000. Either way such a configuration should be described by devicetree dma-ranges property (or ACPI _DMA object) on the parent bus. The one thing to watch out for is that SWIOTLB doesn't necessarily interact very well with DMA offsets. Given the intent of of_dma_get_max_cpu_address(), I think it ought to work out OK now for current kernels on DT systems if everything is described correctly, but otherwise it's likely that you end up with ZONE_DMA either being empty or containing all memory, so the SWIOTLB buffer ends up being allocated anywhere such that it might not actually work as expected. Robin. >> >> Event if to use forced swiotlb via command-line then swiotlb complains >> about the mask (from what I checked is that because swiotlb will be >> allocated at the top of the DMA memory, which is actually the end of >> RAM. Ofcourse if to manually hard-code arm64_dma_phys_limit in >> arch/arm64/mm/init.c then everything works ok and I see that swiotlb >> does the job if to check via debugfs): > > It sounds like maybe SWIOTLB does not support your memory model? > Adding SWIOTLB people. > >> >> [ 1.129987] ------------[ cut here ]------------ >> [ 1.134595] xenon-sdhci 805c0000.sdhci: swiotlb addr 0x00000002f7000000+512 overflow (mask 27fffffff, bus limit 0). >> [ 1.142086] usb 1-1: new high-speed USB device number 2 using orion-ehci >> [ 1.145025] WARNING: CPU: 0 PID: 24 at kernel/dma/swiotlb.c:880 swiotlb_map+0x1b4/0x1f0 >> [ 1.159679] Modules linked in: >> [ 1.162733] CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted >> 5.19.0-next-20220808 #46 >> [ 1.170202] Hardware name: Marvell A0-AC5X Board (DT) >> [ 1.175243] Workqueue: events_freezable mmc_rescan >> [ 1.180033] pstate: 604000c9 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS >> BTYPE=--) >> [ 1.186982] pc : swiotlb_map+0x1b4/0x1f0 >> [ 1.190898] lr : swiotlb_map+0x1b4/0x1f0 >> [ 1.194813] sp : ffff80000a8eb840 >> [ 1.198119] x29: ffff80000a8eb840 x28: ffff80000a8ebb38 x27: >> 0000000000000000 >> [ 1.205245] x26: 0000000000000000 x25: ffff0000004de810 x24: >> 0000000000000000 >> [ 1.212369] x23: 0000000000000002 x22: 0000000000000002 x21: >> 0000000000000000 >> [ 1.219494] x20: 0000000000000200 x19: ffff0000004de810 x18: >> ffffffffffffffff >> [ 1.226619] x17: 203231352b303030 x16: 3030303766323030 x15: >> 3030303030783020 >> [ 1.233744] x14: 7264646120626c74 x13: 2e29302074696d69 x12: >> 6c20737562202c66 >> [ 1.240869] x11: 6666666666663732 x10: 206b73616d282077 x9 : >> 6f697773203a6963 >> [ 1.247994] x8 : 6864732e30303030 x7 : 205d353935343331 x6 : >> ffff80000a665000 >> [ 1.255118] x5 : 0000000000000000 x4 : 0000000000000000 x3 : >> 00000000ffffffff >> [ 1.262243] x2 : 0000000000000000 x1 : 0000000000000000 x0 : >> ffff0000005a0000 >> [ 1.269369] Call trace: >> [ 1.271810] swiotlb_map+0x1b4/0x1f0 >> [ 1.275380] dma_direct_map_sg+0x94/0x2b0 >> [ 1.279382] __dma_map_sg_attrs+0x28/0xa0 >> [ 1.283388] dma_map_sg_attrs+0x10/0x30 >> [ 1.287219] sdhci_pre_dma_transfer+0xcc/0x150 >> [ 1.291658] sdhci_send_command+0x644/0xea0 >> [ 1.295834] sdhci_send_command_retry+0x40/0x140 >> [ 1.300443] sdhci_request+0x70/0xd0 >> [ 1.304012] __mmc_start_request+0x68/0x140 >> [ 1.308188] mmc_start_request+0x78/0xa0 >> [ 1.312103] mmc_wait_for_req+0x70/0x100 >> [ 1.316020] mmc_send_adtc_data+0xd8/0x130 >> [ 1.320109] mmc_get_ext_csd+0x70/0xe0 >> [ 1.323850] mmc_init_card+0x7e8/0x1ad0 >> [ 1.327680] mmc_attach_mmc+0xdc/0x180 >> [ 1.331422] mmc_rescan+0x280/0x2f0 >> [ 1.334905] process_one_work+0x1d0/0x320 >> [ 1.338912] worker_thread+0x14c/0x450 >> [ 1.342655] kthread+0x10c/0x110 >> [ 1.345879] ret_from_fork+0x10/0x20 >> [ 1.349450] ---[ end trace 0000000000000000 ]--- >> [ 1.354091] ------------[ cut here ]------------ >> >> >>>> [ 40.560321] Modules linked in: uio_pdrv_genirq >>>> [ 40.564766] CPU: 0 PID: 7 Comm: kworker/0:0H Not tainted >>>> 5.19.0-next-20220808 #20 >>>> [ 40.572236] Hardware name: Marvell A0-AC5X Board (DT) >>>> [ 40.577278] Workqueue: kblockd blk_mq_run_work_fn >>>> [ 40.581980] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS >>>> BTYPE=--) >>>> [ 40.588929] pc : dma_direct_map_sg+0x28c/0x2b0 >>>> [ 40.593366] lr : dma_direct_map_sg+0x28c/0x2b0 >>>> [ 40.597801] sp : ffff80000a6eba20 >>>> [ 40.601107] x29: ffff80000a6eba20 x28: ffff000006d66080 x27: >>>> 0000000000000004 >>>> [ 40.608233] x26: 0000000000000000 x25: ffff0000004de810 x24: >>>> 0000000000000000 >>>> [ 40.615358] x23: 0000000000000002 x22: fffffc0000000000 x21: >>>> ffff800009153360 >>>> [ 40.622484] x20: 0000000000000005 x19: 00000002ff700000 x18: >>>> ffffffffffffffff >>>> [ 40.629609] x17: 766f203036393034 x16: 2b30303030303766 x15: >>>> 6632303030303030 >>>> [ 40.636735] x14: 3078302072646461 x13: ffff80000a433d58 x12: >>>> 00000000000002dc >>>> [ 40.643860] x11: 00000000000000f4 x10: ffff80000a48bd58 x9 : >>>> ffff80000a48bd58 >>>> [ 40.650985] x8 : ffff80000a433d58 x7 : ffff80000a48bd58 x6 : >>>> 0000000000000000 >>>> [ 40.658110] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : >>>> 0000000000000000 >>>> [ 40.665234] x2 : 0000000000000000 x1 : 0000000000000000 x0 : >>>> ffff0000004add00 >>>> [ 40.672360] Call trace: >>>> [ 40.674800] dma_direct_map_sg+0x28c/0x2b0 >>>> [ 40.678890] __dma_map_sg_attrs+0x28/0xa0 >>>> [ 40.682895] dma_map_sg_attrs+0x10/0x30 >>>> [ 40.686725] sdhci_pre_dma_transfer+0xcc/0x150 >>>> [ 40.691165] sdhci_pre_req+0x3c/0x50 >>>> [ 40.694734] mmc_blk_mq_issue_rq+0x25c/0x9d0 >>>> [ 40.698997] mmc_mq_queue_rq+0x118/0x2b0 >>>> [ 40.702912] blk_mq_dispatch_rq_list+0x19c/0x800 >>>> [ 40.707521] blk_mq_do_dispatch_sched+0x2bc/0x350 >>>> [ 40.712218] __blk_mq_sched_dispatch_requests+0x10c/0x160 >>>> [ 40.717608] blk_mq_sched_dispatch_requests+0x40/0x80 >>>> [ 40.722651] __blk_mq_run_hw_queue+0x80/0xc0 >>>> [ 40.726911] blk_mq_run_work_fn+0x24/0x30 >>>> [ 40.730913] process_one_work+0x1d0/0x320 >>>> [ 40.734919] worker_thread+0x14c/0x450 >>>> [ 40.738662] kthread+0x10c/0x110 >>>> [ 40.741885] ret_from_fork+0x10/0x20 >>>> [ 40.745457] ---[ end trace 0000000000000000 ]--- >>>> [ 40.750160] ------------[ cut here ]------------ >>>> [ 40.754770] WARNING: CPU: 0 PID: 7 at drivers/mmc/host/sdhci.c:1152 >>>> sdhci_send_command+0x7fc/0xea0 >>>> [ 40.763719] Modules linked in: uio_pdrv_genirq >>>> [ 40.768149] CPU: 0 PID: 7 Comm: kworker/0:0H Tainted: G W >>>> 5.19.0-next-20220808 #20 >>>> [ 40.777090] Hardware name: Marvell A0-AC5X Board (DT) >>>> [ 40.782129] Workqueue: kblockd blk_mq_run_work_fn >>>> [ 40.786827] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS >>>> BTYPE=--) >>>> [ 40.793775] pc : sdhci_send_command+0x7fc/0xea0 >>>> [ 40.798297] lr : sdhci_send_command+0x644/0xea0 >>>> [ 40.802821] sp : ffff80000a6eb980 >>>> [ 40.806127] x29: ffff80000a6eb980 x28: ffff80000a6ebcf8 x27: >>>> 0000000000000002 >>>> [ 40.813253] x26: 0000000000000000 x25: 0000000000000000 x24: >>>> ffff0000004237c0 >>>> [ 40.820378] x23: 000000000000000b x22: ffff000006d825b8 x21: >>>> ffff000006d82628 >>>> [ 40.827502] x20: ffff000006d825b8 x19: ffff000000423580 x18: >>>> ffffffffffffffff >>>> [ 40.834627] x17: 766f203036393034 x16: 2b30303030303766 x15: >>>> 6632303030303030 >>>> [ 40.841751] x14: 3078302072646461 x13: ffff80000a433d58 x12: >>>> 00000000000002dc >>>> [ 40.848876] x11: 00000000000000f4 x10: ffff80000a48bd58 x9 : >>>> ffff80000a48bd58 >>>> [ 40.856001] x8 : ffff80000a433d58 x7 : 0000000000000000 x6 : >>>> ffff0000004deaf8 >>>> [ 40.863125] x5 : 0000000000000000 x4 : 0000000000000020 x3 : >>>> 0000000000000000 >>>> [ 40.870250] x2 : 0000000000020401 x1 : 0000000000000001 x0 : >>>> 00000000ffffffe4 >>>> [ 40.877374] Call trace: >>>> [ 40.879813] sdhci_send_command+0x7fc/0xea0 >>>> [ 40.883989] sdhci_send_command_retry+0x40/0x140 >>>> [ 40.888597] sdhci_request+0x70/0xd0 >>>> [ 40.892167] __mmc_start_request+0x68/0x140 >>>> [ 40.896345] mmc_start_request+0x78/0xa0 >>>> [ 40.900260] mmc_blk_mq_issue_rq+0x284/0x9d0 >>>> [ 40.904521] mmc_mq_queue_rq+0x118/0x2b0 >>>> [ 40.908436] blk_mq_dispatch_rq_list+0x19c/0x800 >>>> [ 40.913045] blk_mq_do_dispatch_sched+0x2bc/0x350 >>>> [ 40.917741] __blk_mq_sched_dispatch_requests+0x10c/0x160 >>>> [ 40.923131] blk_mq_sched_dispatch_requests+0x40/0x80 >>>> [ 40.928173] __blk_mq_run_hw_queue+0x80/0xc0 >>>> [ 40.932435] blk_mq_run_work_fn+0x24/0x30 >>>> [ 40.936437] process_one_work+0x1d0/0x320 >>>> [ 40.940442] worker_thread+0x14c/0x450 >>>> [ 40.944184] kthread+0x10c/0x110 >>>> [ 40.947407] ret_from_fork+0x10/0x20 >>>> [ 40.950977] ---[ end trace 0000000000000000 ]--- >>>> [ 40.960738] ------------[ cut here ]------------ >>>> [ 40.965361] WARNING: CPU: 1 PID: 49 at drivers/mmc/host/sdhci.c:1152 >>>> sdhci_send_command+0x7fc/0xea0 >>>> [ 40.974403] Modules linked in: uio_pdrv_genirq >>>> [ 40.978843] CPU: 1 PID: 49 Comm: kworker/1:1H Tainted: G W >>>> 5.19.0-next-20220808 #20 >>>> [ 40.987873] Hardware name: Marvell A0-AC5X Board (DT) >>>> [ 40.992913] Workqueue: kblockd blk_mq_run_work_fn >>>> [ 40.997613] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS >>>> BTYPE=--) >>>> [ 41.004562] pc : sdhci_send_command+0x7fc/0xea0 >>>> [ 41.009086] lr : sdhci_send_command+0x644/0xea0 >>>> [ 41.013610] sp : ffff80000a9db980 >>>> [ 41.016915] x29: ffff80000a9db980 x28: ffff80000a9dbcf8 x27: >>>> 0000000000000002 >>>> [ 41.024041] x26: 0000000000000000 x25: 0000000000000000 x24: >>>> ffff0000004237c0 >>>> [ 41.031166] x23: 000000000000000b x22: ffff000006d05eb8 x21: >>>> ffff000006d05f28 >>>> [ 41.038291] x20: ffff000006d05eb8 x19: ffff000000423580 x18: >>>> fffffc0003fde640 >>>> [ 41.045416] x17: 0000000000000000 x16: 0000000000000000 x15: >>>> 0000000000000000 >>>> [ 41.052540] x14: 0000000000000002 x13: 0000000000100000 x12: >>>> 0000000000000000 >>>> [ 41.059664] x11: 0000000000000000 x10: 0000000000000b10 x9 : >>>> ffff80000a9db9e0 >>>> [ 41.066789] x8 : ffff000000649af0 x7 : 0000000000000000 x6 : >>>> ffff0000004deaf8 >>>> [ 41.073914] x5 : 0000000000000000 x4 : 0000000000000020 x3 : >>>> 0000000000000000 >>>> [ 41.081039] x2 : 0000000000020401 x1 : 0000000000000001 x0 : >>>> 00000000ffffffe4 >>>> [ 41.088164] Call trace: >>>> [ 41.090603] sdhci_send_command+0x7fc/0xea0 >>>> [ 41.094779] sdhci_send_command_retry+0x40/0x140 >>>> [ 41.099379] sdhci_request+0x70/0xd0 >>>> [ 41.102949] __mmc_start_request+0x68/0x140 >>>> [ 41.107127] mmc_start_request+0x78/0xa0 >>>> [ 41.111042] mmc_blk_mq_issue_rq+0x284/0x9d0 >>>> [ 41.115305] mmc_mq_queue_rq+0x118/0x2b0 >>>> [ 41.119221] blk_mq_dispatch_rq_list+0x19c/0x800 >>>> [ 41.123831] blk_mq_do_dispatch_sched+0x2bc/0x350 >>>> [ 41.128528] __blk_mq_sched_dispatch_requests+0x10c/0x160 >>>> [ 41.133917] blk_mq_sched_dispatch_requests+0x40/0x80 >>>> [ 41.138959] __blk_mq_run_hw_queue+0x80/0xc0 >>>> [ 41.143222] blk_mq_run_work_fn+0x24/0x30 >>>> [ 41.147223] process_one_work+0x1d0/0x320 >>>> [ 41.151230] worker_thread+0x14c/0x450 >>>> [ 41.154973] kthread+0x10c/0x110 >>>> [ 41.158195] ret_from_fork+0x10/0x20 >>>> [ 41.161766] ---[ end trace 0000000000000000 ]--- >>>> [ 41.171543] ------------[ cut here ]------------ >>>> [ 41.176169] WARNING: CPU: 1 PID: 49 at drivers/mmc/host/sdhci.c:1152 >>>> sdhci_send_command+0x7fc/0xea0 >>>> [ 41.185210] Modules linked in: uio_pdrv_genirq >>>> [ 41.189650] CPU: 1 PID: 49 Comm: kworker/1:1H Tainted: G W >>>> 5.19.0-next-20220808 #20 >>>> [ 41.198679] Hardware name: Marvell A0-AC5X Board (DT) >>>> [ 41.203720] Workqueue: kblockd blk_mq_run_work_fn >>>> [ 41.208420] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS >>>> BTYPE=--) >>>> [ 41.215369] pc : sdhci_send_command+0x7fc/0xea0 >>>> [ 41.219892] lr : sdhci_send_command+0x644/0xea0 >>>> [ 41.224414] sp : ffff80000a9db980 >>>> [ 41.227721] x29: ffff80000a9db980 x28: ffff80000a9dbcf8 x27: >>>> 0000000000000002 >>>> [ 41.234846] x26: 0000000000000000 x25: 0000000000000000 x24: >>>> ffff0000004237c0 >>>> [ 41.241972] x23: 000000000000000b x22: ffff000006d001b8 x21: >>>> ffff000006d00228 >>>> [ 41.249097] x20: ffff000006d001b8 x19: ffff000000423580 x18: >>>> 0000000000000000 >>>> [ 41.256223] x17: 0000000000000000 x16: 0000000000000000 x15: >>>> 0000000000000000 >>>> [ 41.263348] x14: 0000000000000002 x13: 0000000000100000 x12: >>>> 0000000000000000 >>>> [ 41.270473] x11: 0000000000000001 x10: 0000000000000b10 x9 : >>>> ffff80000a9db9e0 >>>> [ 41.277598] x8 : ffff000000649af0 x7 : 0000000000000000 x6 : >>>> ffff0000004deaf8 >>>> [ 41.284722] x5 : 0000000000000000 x4 : 0000000000000020 x3 : >>>> 0000000000000000 >>>> [ 41.291846] x2 : 0000000000020401 x1 : 0000000000000001 x0 : >>>> 00000000ffffffe4 >>>> [ 41.298971] Call trace: >>>> [ 41.301410] sdhci_send_command+0x7fc/0xea0 >>>> [ 41.305588] sdhci_send_command_retry+0x40/0x140 >>>> [ 41.310198] sdhci_request+0x70/0xd0 >>>> [ 41.313769] __mmc_start_request+0x68/0x140 >>>> [ 41.317946] mmc_start_request+0x78/0xa0 >>>> [ 41.321862] mmc_blk_mq_issue_rq+0x284/0x9d0 >>>> [ 41.326125] mmc_mq_queue_rq+0x118/0x2b0 >>>> [ 41.330040] blk_mq_dispatch_rq_list+0x19c/0x800 >>>> [ 41.334649] blk_mq_do_dispatch_sched+0x2bc/0x350 >>>> [ 41.339346] __blk_mq_sched_dispatch_requests+0x10c/0x160 >>>> [ 41.344735] blk_mq_sched_dispatch_requests+0x40/0x80 >>>> [ 41.349779] __blk_mq_run_hw_queue+0x80/0xc0 >>>> [ 41.354040] blk_mq_run_work_fn+0x24/0x30 >>>> [ 41.358041] process_one_work+0x1d0/0x320 >>>> [ 41.362048] worker_thread+0x14c/0x450 >>>> [ 41.365791] kthread+0x10c/0x110 >>>> [ 41.369014] ret_from_fork+0x10/0x20 >>>> [ 41.372584] ---[ end trace 0000000000000000 ]--- >>>> [ 41.382383] ------------[ cut here ]------------ >>>> [ 41.387009] WARNING: CPU: 1 PID: 49 at drivers/mmc/host/sdhci.c:1152 >>>> sdhci_send_command+0x7fc/0xea0 >>>> [ 41.396051] Modules linked in: uio_pdrv_genirq >>>> [ 41.400491] CPU: 1 PID: 49 Comm: kworker/1:1H Tainted: G W >>>> 5.19.0-next-20220808 #20 >>>> [ 41.409520] Hardware name: Marvell A0-AC5X Board (DT) >>>> [ 41.414561] Workqueue: kblockd blk_mq_run_work_fn >>>> [ 41.419261] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS >>>> BTYPE=--) >>>> [ 41.426210] pc : sdhci_send_command+0x7fc/0xea0 >>>> [ 41.430734] lr : sdhci_send_command+0x644/0xea0 >>>> [ 41.435256] sp : ffff80000a9db980 >>>> [ 41.438562] x29: ffff80000a9db980 x28: ffff80000a9dbcf8 x27: >>>> 0000000000000002 >>>> [ 41.445687] x26: 0000000000000000 x25: 0000000000000000 x24: >>>> ffff0000004237c0 >>>> [ 41.452812] x23: 000000000000000b x22: ffff000006d004b8 x21: >>>> ffff000006d00528 >>>> [ 41.459937] x20: ffff000006d004b8 x19: ffff000000423580 x18: >>>> 0000000000000000 >>>> [ 41.467062] x17: 0000000000000000 x16: 0000000000000000 x15: >>>> 0000000000000000 >>>> [ 41.474187] x14: 0000000000000002 x13: 00000000000ff000 x12: >>>> 0000000000000000 >>>> [ 41.481311] x11: 0000000000000001 x10: 0000000000000b10 x9 : >>>> ffff80000a9db9e0 >>>> [ 41.488436] x8 : ffff000000649af0 x7 : 0000000000000000 x6 : >>>> ffff0000004deaf8 >>>> [ 41.495561] x5 : 0000000000000000 x4 : 0000000000000020 x3 : >>>> 0000000000000000 >>>> [ 41.502685] x2 : 0000000000020401 x1 : 0000000000000001 x0 : >>>> 00000000ffffffe4 >>>> [ 41.509811] Call trace: >>>> [ 41.512250] sdhci_send_command+0x7fc/0xea0 >>>> [ 41.516427] sdhci_send_command_retry+0x40/0x140 >>>> [ 41.521035] sdhci_request+0x70/0xd0 >>>> [ 41.524607] __mmc_start_request+0x68/0x140 >>>> [ 41.528784] mmc_start_request+0x78/0xa0 >>>> [ 41.532699] mmc_blk_mq_issue_rq+0x284/0x9d0 >>>> [ 41.536961] mmc_mq_queue_rq+0x118/0x2b0 >>>> [ 41.540876] blk_mq_dispatch_rq_list+0x19c/0x800 >>>> [ 41.545485] blk_mq_do_dispatch_sched+0x2bc/0x350 >>>> [ 41.550182] __blk_mq_sched_dispatch_requests+0x10c/0x160 >>>> [ 41.555570] blk_mq_sched_dispatch_requests+0x40/0x80 >>>> [ 41.560613] __blk_mq_run_hw_queue+0x80/0xc0 >>>> [ 41.564874] blk_mq_run_work_fn+0x24/0x30 >>>> [ 41.568875] process_one_work+0x1d0/0x320 >>>> [ 41.572881] worker_thread+0x14c/0x450 >>>> [ 41.576624] kthread+0x10c/0x110 >>>> [ 41.579847] ret_from_fork+0x10/0x20 >>>> [ 41.583428] ---[ end trace 0000000000000000 ]--- >>>> >>>> >>>>>> >>>>>>> Do I understand correctly that the swiotlb will be used in case DMA >>>>>>> engine could not map the page in the specified range (limited by dma-ranges or >>>>>>> dma-mask) ? >>>>>>> >>>>>>>> >>>>>>>>> + host->quirks |= SDHCI_QUIRK_BROKEN_DMA; >>>>>>>>> + host->quirks |= SDHCI_QUIRK_BROKEN_ADMA; >>>>>>>>> + dev_info(mmc_dev(mmc), "Disabling DMA because of 2GB DMA access limit.\n"); >>>>>>>>> + } >>>>>>>>> + >>>>>>>>> return xenon_phy_parse_params(dev, host); >>>>>>>>> } >>>>>>>>> >>>>>>>>> @@ -682,6 +694,7 @@ static const struct of_device_id sdhci_xenon_dt_ids[] = { >>>>>>>>> { .compatible = "marvell,armada-ap807-sdhci", .data = (void *)XENON_AP807}, >>>>>>>>> { .compatible = "marvell,armada-cp110-sdhci", .data = (void *)XENON_CP110}, >>>>>>>>> { .compatible = "marvell,armada-3700-sdhci", .data = (void *)XENON_A3700}, >>>>>>>>> + { .compatible = "marvell,ac5-sdhci", .data = (void *)XENON_AP806}, >>>>>>>>> {} >>>>>>>>> }; >>>>>>>>> MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids); >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Florian >>>>>>> >>>>>>> Thanks, >>>>>>> >>>>>> >>>>>> Regards, >>>>> >>>> >>>> Thanks, >>>> Vadym Kochan >>> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-08-08 14:06 ` Robin Murphy @ 2022-08-16 20:51 ` Vadym Kochan 2022-08-17 13:43 ` Robin Murphy 0 siblings, 1 reply; 15+ messages in thread From: Vadym Kochan @ 2022-08-16 20:51 UTC (permalink / raw) To: Robin Murphy Cc: Adrian Hunter, Christoph Hellwig, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu Hi Robin, On Mon, Aug 08, 2022 at 03:06:51PM +0100, Robin Murphy wrote: > On 2022-08-08 13:58, Adrian Hunter wrote: > > On 8/08/22 15:26, Vadym Kochan wrote: > > > On Mon, Aug 08, 2022 at 02:40:07PM +0300, Adrian Hunter wrote: > > > > On 8/08/22 12:52, Vadym Kochan wrote: > > > > > Hi Adrian, > > > > > > > > > > On Mon, Aug 08, 2022 at 12:19:10PM +0300, Adrian Hunter wrote: > > > > > > On 1/08/22 12:30, Vadym Kochan wrote: > > > > > > > Hi Florian, > > > > > > > > > > > > > > On Wed, Jul 27, 2022 at 07:45:32PM +0300, Vadym Kochan wrote: > > > > > > > > Hi Florian, > > > > > > > > > > > > > > > > On Tue, Jul 26, 2022 at 10:37:46AM -0700, Florian Fainelli wrote: > > > > > > > > > On 7/26/22 10:07, Vadym Kochan wrote: > > > > > > > > > > From: Elad Nachman <enachman@marvell.com> > > > > > > > > > > > > > > > > > > > > There is a limitation on AC5 SoC that mmc controller > > > > > > > > > > can't have DMA access over 2G memory. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Elad Nachman <enachman@marvell.com> > > > > > > > > > > Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu> > > > > > > > > > > --- > > > > > > > > > > drivers/mmc/host/sdhci-xenon.c | 13 +++++++++++++ > > > > > > > > > > 1 file changed, 13 insertions(+) > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c > > > > > > > > > > index 08e838400b52..666d06b58564 100644 > > > > > > > > > > --- a/drivers/mmc/host/sdhci-xenon.c > > > > > > > > > > +++ b/drivers/mmc/host/sdhci-xenon.c > > > > > > > > > > @@ -18,6 +18,7 @@ > > > > > > > > > > #include <linux/of.h> > > > > > > > > > > #include <linux/pm.h> > > > > > > > > > > #include <linux/pm_runtime.h> > > > > > > > > > > +#include <linux/mm.h> > > > > > > > > > > #include "sdhci-pltfm.h" > > > > > > > > > > #include "sdhci-xenon.h" > > > > > > > > > > @@ -422,6 +423,8 @@ static int xenon_probe_params(struct platform_device *pdev) > > > > > > > > > > struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host); > > > > > > > > > > u32 sdhc_id, nr_sdhc; > > > > > > > > > > u32 tuning_count; > > > > > > > > > > + struct device_node *np = pdev->dev.of_node; > > > > > > > > > > + struct sysinfo si; > > > > > > > > > > /* Disable HS200 on Armada AP806 */ > > > > > > > > > > if (priv->hw_version == XENON_AP806) > > > > > > > > > > @@ -450,6 +453,15 @@ static int xenon_probe_params(struct platform_device *pdev) > > > > > > > > > > } > > > > > > > > > > priv->tuning_count = tuning_count; > > > > > > > > > > + si_meminfo(&si); > > > > > > > > > > + > > > > > > > > > > + if (of_device_is_compatible(np, "marvell,ac5-sdhci") && > > > > > > > > > > + ((si.totalram * si.mem_unit) > 0x80000000 /*2G*/)) { > > > > > > > > > > > > > > > > > > Why not limit the DMA mask of the device and ensure, that bounce buffers get used so you can still do DMA? > > > > > > > > > > > > > > > > > > Also, you ought to be able to describe that limitation using Device Tree (assuming this is an option) and declaring a dedicated bus node for the SDHCI controller and providing a suitable dma-ranges property, see: arch/arm/boot/dts/bcm2711.dtsi and the 'soc' node for such examples. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I could use DMA only in 2 ways: > > > > > > > > > > > > > > #1 Use sdhci bounce buffer with SDMA mode > > > > > > > > > > > > > > But there was the issue that SDMA requires that SDHCI v4 mode should > > > > > > > be enabled, and when I enable it via sdhci_enable_v4_mode(host) > > > > > > > then I got error that EXT_CSD can't be recognized. > > > > > > > > > > > > > > But if I comment this line in sdhci.c: > > > > > > > int sdhci_setup_host(struct sdhci_host *host) > > > > > > > { > > > > > > > ... > > > > > > > > > > > > > > /* SDMA does not support 64-bit DMA if v4 mode not set */ > > > > > > > if ((host->flags & SDHCI_USE_64_BIT_DMA) && !host->v4_mode) { > > > > > > > pr_info("XXX SDMA does not support 64-bit DMA if v4 mode not set\n"); > > > > > > > host->flags &= ~SDHCI_USE_SDMA; > > > > > > > } > > > > > > > > > > > > > > ... > > > > > > > } > > > > > > > > > > > > > > then everything is OK. > > > > > > > > > > > > > > #2 Use restricted-dma-pool in device-tree > > > > > > > > > > > > > > But I am not sure if it is good solution compared to #1. > > > > > > > > > > > > > > Setting only DMA mask did not help because after some time I got > > > > > > > "DMA overflow address" error stack-traces. > > > > > > > > > > > > AFAICT using a DMA mask is the correct way to solve this. If that > > > > > > reveals another issue then that must be resolved also. Can you show > > > > > > the stack traces and what kernel they are from? > > > > > > > > > > > > > > > > The Linux version is next-20220808 > > > > > > > > > > Here is a stacktrace: > > > > > > > > > > [ 40.537028] ------------[ cut here ]------------ > > > > > [ 40.541698] xenon-sdhci 805c0000.sdhci: DMA addr > > > > > 0x00000002ff700000+40960 overflow (mask 27fffffff, bus limit 0). > > > > > [ 40.551968] WARNING: CPU: 0 PID: 7 at kernel/dma/direct.h:103 > > > > > dma_direct_map_sg+0x28c/0x2b0 > > > > > > > > Do you have CONFIG_SWIOTLB=y? > > > > > > > > > > Yes. Additionally I pass "swiotlb=force", because as I understand it is > > > not enabled by default at least on arm64. > > If you think you need "swiotlb=force" then something's definitely wrong. > SWIOTLB should be invoked automatically based on device DMA masks being set > correctly. If you need to support devices with masks smaller than 32 bits > you should have CONFIG_ZONE_DMA enabled as well. > > > > > Is DMA mask correct? 2G mask would be 7fffffff not 27fffffff right? > > > > > > > > > > The RAM starts at 0x2_00000000 so thats why I use this mask. > > You can't have holes in DMA masks, they don't work that way. Presumably > either the device interface itself drives DMA_BIT_MASK(34) or wider but the > interconnect address map doesn't decode the full range, or the device drives > DMA_BIT_MASK(31) and the interconnect adds a fixed offset of 0x200000000. > Either way such a configuration should be described by devicetree dma-ranges > property (or ACPI _DMA object) on the parent bus. > > The one thing to watch out for is that SWIOTLB doesn't necessarily interact > very well with DMA offsets. Given the intent of > of_dma_get_max_cpu_address(), I think it ought to work out OK now for > current kernels on DT systems if everything is described correctly, but > otherwise it's likely that you end up with ZONE_DMA either being empty or > containing all memory, so the SWIOTLB buffer ends up being allocated > anywhere such that it might not actually work as expected. > > Robin. Hi Robin, Thank you for the reply. My understanding is that swiotlb is allocated (in case of arm64) in the following cases: #1 when it is forced from the kernel cmdline #2 when max_pfn is greater than arm64_dma_phys_limit (and this is used as the end from which to allocate the swiotlb pool in the top-botom direction via memblock API). #3 using restricted dma-pool Of course option #3 works fine because swiotlb is kind of forced to use particulary this range of memory. Both options #1 & #2 causes to use full memory mask even if to specify dma-ranges in the DT: dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; or if to specify the opposite: dma-ranges = <0x2 0x0 0x0 0x0 0x0 0x80000000>; just to make it lower than U32 to pass zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits) condition, but then it will be re-set in max_zone_phys() by: if (phys_start > U32_MAX) zone_mask = PHYS_ADDR_MAX; else if (phys_start > zone_mask) zone_mask = U32_MAX; So, currently I dont see how to pin swiotlb (I see it as a main problem) to some specific range of physical memory (particulary to the first 2G of RAM). Thanks, Vadym > > > > > > > Event if to use forced swiotlb via command-line then swiotlb complains > > > about the mask (from what I checked is that because swiotlb will be > > > allocated at the top of the DMA memory, which is actually the end of > > > RAM. Ofcourse if to manually hard-code arm64_dma_phys_limit in > > > arch/arm64/mm/init.c then everything works ok and I see that swiotlb > > > does the job if to check via debugfs): > > > > It sounds like maybe SWIOTLB does not support your memory model? > > Adding SWIOTLB people. > > > > > > > > [ 1.129987] ------------[ cut here ]------------ > > > [ 1.134595] xenon-sdhci 805c0000.sdhci: swiotlb addr 0x00000002f7000000+512 overflow (mask 27fffffff, bus limit 0). > > > [ 1.142086] usb 1-1: new high-speed USB device number 2 using orion-ehci > > > [ 1.145025] WARNING: CPU: 0 PID: 24 at kernel/dma/swiotlb.c:880 swiotlb_map+0x1b4/0x1f0 > > > [ 1.159679] Modules linked in: > > > [ 1.162733] CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted > > > 5.19.0-next-20220808 #46 > > > [ 1.170202] Hardware name: Marvell A0-AC5X Board (DT) > > > [ 1.175243] Workqueue: events_freezable mmc_rescan > > > [ 1.180033] pstate: 604000c9 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS > > > BTYPE=--) > > > [ 1.186982] pc : swiotlb_map+0x1b4/0x1f0 > > > [ 1.190898] lr : swiotlb_map+0x1b4/0x1f0 > > > [ 1.194813] sp : ffff80000a8eb840 > > > [ 1.198119] x29: ffff80000a8eb840 x28: ffff80000a8ebb38 x27: > > > 0000000000000000 > > > [ 1.205245] x26: 0000000000000000 x25: ffff0000004de810 x24: > > > 0000000000000000 > > > [ 1.212369] x23: 0000000000000002 x22: 0000000000000002 x21: > > > 0000000000000000 > > > [ 1.219494] x20: 0000000000000200 x19: ffff0000004de810 x18: > > > ffffffffffffffff > > > [ 1.226619] x17: 203231352b303030 x16: 3030303766323030 x15: > > > 3030303030783020 > > > [ 1.233744] x14: 7264646120626c74 x13: 2e29302074696d69 x12: > > > 6c20737562202c66 > > > [ 1.240869] x11: 6666666666663732 x10: 206b73616d282077 x9 : > > > 6f697773203a6963 > > > [ 1.247994] x8 : 6864732e30303030 x7 : 205d353935343331 x6 : > > > ffff80000a665000 > > > [ 1.255118] x5 : 0000000000000000 x4 : 0000000000000000 x3 : > > > 00000000ffffffff > > > [ 1.262243] x2 : 0000000000000000 x1 : 0000000000000000 x0 : > > > ffff0000005a0000 > > > [ 1.269369] Call trace: > > > [ 1.271810] swiotlb_map+0x1b4/0x1f0 > > > [ 1.275380] dma_direct_map_sg+0x94/0x2b0 > > > [ 1.279382] __dma_map_sg_attrs+0x28/0xa0 > > > [ 1.283388] dma_map_sg_attrs+0x10/0x30 > > > [ 1.287219] sdhci_pre_dma_transfer+0xcc/0x150 > > > [ 1.291658] sdhci_send_command+0x644/0xea0 > > > [ 1.295834] sdhci_send_command_retry+0x40/0x140 > > > [ 1.300443] sdhci_request+0x70/0xd0 > > > [ 1.304012] __mmc_start_request+0x68/0x140 > > > [ 1.308188] mmc_start_request+0x78/0xa0 > > > [ 1.312103] mmc_wait_for_req+0x70/0x100 > > > [ 1.316020] mmc_send_adtc_data+0xd8/0x130 > > > [ 1.320109] mmc_get_ext_csd+0x70/0xe0 > > > [ 1.323850] mmc_init_card+0x7e8/0x1ad0 > > > [ 1.327680] mmc_attach_mmc+0xdc/0x180 > > > [ 1.331422] mmc_rescan+0x280/0x2f0 > > > [ 1.334905] process_one_work+0x1d0/0x320 > > > [ 1.338912] worker_thread+0x14c/0x450 > > > [ 1.342655] kthread+0x10c/0x110 > > > [ 1.345879] ret_from_fork+0x10/0x20 > > > [ 1.349450] ---[ end trace 0000000000000000 ]--- > > > [ 1.354091] ------------[ cut here ]------------ > > > > > > > > > > > [ 40.560321] Modules linked in: uio_pdrv_genirq > > > > > [ 40.564766] CPU: 0 PID: 7 Comm: kworker/0:0H Not tainted > > > > > 5.19.0-next-20220808 #20 > > > > > [ 40.572236] Hardware name: Marvell A0-AC5X Board (DT) > > > > > [ 40.577278] Workqueue: kblockd blk_mq_run_work_fn > > > > > [ 40.581980] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS > > > > > BTYPE=--) > > > > > [ 40.588929] pc : dma_direct_map_sg+0x28c/0x2b0 > > > > > [ 40.593366] lr : dma_direct_map_sg+0x28c/0x2b0 > > > > > [ 40.597801] sp : ffff80000a6eba20 > > > > > [ 40.601107] x29: ffff80000a6eba20 x28: ffff000006d66080 x27: > > > > > 0000000000000004 > > > > > [ 40.608233] x26: 0000000000000000 x25: ffff0000004de810 x24: > > > > > 0000000000000000 > > > > > [ 40.615358] x23: 0000000000000002 x22: fffffc0000000000 x21: > > > > > ffff800009153360 > > > > > [ 40.622484] x20: 0000000000000005 x19: 00000002ff700000 x18: > > > > > ffffffffffffffff > > > > > [ 40.629609] x17: 766f203036393034 x16: 2b30303030303766 x15: > > > > > 6632303030303030 > > > > > [ 40.636735] x14: 3078302072646461 x13: ffff80000a433d58 x12: > > > > > 00000000000002dc > > > > > [ 40.643860] x11: 00000000000000f4 x10: ffff80000a48bd58 x9 : > > > > > ffff80000a48bd58 > > > > > [ 40.650985] x8 : ffff80000a433d58 x7 : ffff80000a48bd58 x6 : > > > > > 0000000000000000 > > > > > [ 40.658110] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : > > > > > 0000000000000000 > > > > > [ 40.665234] x2 : 0000000000000000 x1 : 0000000000000000 x0 : > > > > > ffff0000004add00 > > > > > [ 40.672360] Call trace: > > > > > [ 40.674800] dma_direct_map_sg+0x28c/0x2b0 > > > > > [ 40.678890] __dma_map_sg_attrs+0x28/0xa0 > > > > > [ 40.682895] dma_map_sg_attrs+0x10/0x30 > > > > > [ 40.686725] sdhci_pre_dma_transfer+0xcc/0x150 > > > > > [ 40.691165] sdhci_pre_req+0x3c/0x50 > > > > > [ 40.694734] mmc_blk_mq_issue_rq+0x25c/0x9d0 > > > > > [ 40.698997] mmc_mq_queue_rq+0x118/0x2b0 > > > > > [ 40.702912] blk_mq_dispatch_rq_list+0x19c/0x800 > > > > > [ 40.707521] blk_mq_do_dispatch_sched+0x2bc/0x350 > > > > > [ 40.712218] __blk_mq_sched_dispatch_requests+0x10c/0x160 > > > > > [ 40.717608] blk_mq_sched_dispatch_requests+0x40/0x80 > > > > > [ 40.722651] __blk_mq_run_hw_queue+0x80/0xc0 > > > > > [ 40.726911] blk_mq_run_work_fn+0x24/0x30 > > > > > [ 40.730913] process_one_work+0x1d0/0x320 > > > > > [ 40.734919] worker_thread+0x14c/0x450 > > > > > [ 40.738662] kthread+0x10c/0x110 > > > > > [ 40.741885] ret_from_fork+0x10/0x20 > > > > > [ 40.745457] ---[ end trace 0000000000000000 ]--- > > > > > [ 40.750160] ------------[ cut here ]------------ > > > > > [ 40.754770] WARNING: CPU: 0 PID: 7 at drivers/mmc/host/sdhci.c:1152 > > > > > sdhci_send_command+0x7fc/0xea0 > > > > > [ 40.763719] Modules linked in: uio_pdrv_genirq > > > > > [ 40.768149] CPU: 0 PID: 7 Comm: kworker/0:0H Tainted: G W > > > > > 5.19.0-next-20220808 #20 > > > > > [ 40.777090] Hardware name: Marvell A0-AC5X Board (DT) > > > > > [ 40.782129] Workqueue: kblockd blk_mq_run_work_fn > > > > > [ 40.786827] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS > > > > > BTYPE=--) > > > > > [ 40.793775] pc : sdhci_send_command+0x7fc/0xea0 > > > > > [ 40.798297] lr : sdhci_send_command+0x644/0xea0 > > > > > [ 40.802821] sp : ffff80000a6eb980 > > > > > [ 40.806127] x29: ffff80000a6eb980 x28: ffff80000a6ebcf8 x27: > > > > > 0000000000000002 > > > > > [ 40.813253] x26: 0000000000000000 x25: 0000000000000000 x24: > > > > > ffff0000004237c0 > > > > > [ 40.820378] x23: 000000000000000b x22: ffff000006d825b8 x21: > > > > > ffff000006d82628 > > > > > [ 40.827502] x20: ffff000006d825b8 x19: ffff000000423580 x18: > > > > > ffffffffffffffff > > > > > [ 40.834627] x17: 766f203036393034 x16: 2b30303030303766 x15: > > > > > 6632303030303030 > > > > > [ 40.841751] x14: 3078302072646461 x13: ffff80000a433d58 x12: > > > > > 00000000000002dc > > > > > [ 40.848876] x11: 00000000000000f4 x10: ffff80000a48bd58 x9 : > > > > > ffff80000a48bd58 > > > > > [ 40.856001] x8 : ffff80000a433d58 x7 : 0000000000000000 x6 : > > > > > ffff0000004deaf8 > > > > > [ 40.863125] x5 : 0000000000000000 x4 : 0000000000000020 x3 : > > > > > 0000000000000000 > > > > > [ 40.870250] x2 : 0000000000020401 x1 : 0000000000000001 x0 : > > > > > 00000000ffffffe4 > > > > > [ 40.877374] Call trace: > > > > > [ 40.879813] sdhci_send_command+0x7fc/0xea0 > > > > > [ 40.883989] sdhci_send_command_retry+0x40/0x140 > > > > > [ 40.888597] sdhci_request+0x70/0xd0 > > > > > [ 40.892167] __mmc_start_request+0x68/0x140 > > > > > [ 40.896345] mmc_start_request+0x78/0xa0 > > > > > [ 40.900260] mmc_blk_mq_issue_rq+0x284/0x9d0 > > > > > [ 40.904521] mmc_mq_queue_rq+0x118/0x2b0 > > > > > [ 40.908436] blk_mq_dispatch_rq_list+0x19c/0x800 > > > > > [ 40.913045] blk_mq_do_dispatch_sched+0x2bc/0x350 > > > > > [ 40.917741] __blk_mq_sched_dispatch_requests+0x10c/0x160 > > > > > [ 40.923131] blk_mq_sched_dispatch_requests+0x40/0x80 > > > > > [ 40.928173] __blk_mq_run_hw_queue+0x80/0xc0 > > > > > [ 40.932435] blk_mq_run_work_fn+0x24/0x30 > > > > > [ 40.936437] process_one_work+0x1d0/0x320 > > > > > [ 40.940442] worker_thread+0x14c/0x450 > > > > > [ 40.944184] kthread+0x10c/0x110 > > > > > [ 40.947407] ret_from_fork+0x10/0x20 > > > > > [ 40.950977] ---[ end trace 0000000000000000 ]--- > > > > > [ 40.960738] ------------[ cut here ]------------ > > > > > [ 40.965361] WARNING: CPU: 1 PID: 49 at drivers/mmc/host/sdhci.c:1152 > > > > > sdhci_send_command+0x7fc/0xea0 > > > > > [ 40.974403] Modules linked in: uio_pdrv_genirq > > > > > [ 40.978843] CPU: 1 PID: 49 Comm: kworker/1:1H Tainted: G W > > > > > 5.19.0-next-20220808 #20 > > > > > [ 40.987873] Hardware name: Marvell A0-AC5X Board (DT) > > > > > [ 40.992913] Workqueue: kblockd blk_mq_run_work_fn > > > > > [ 40.997613] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS > > > > > BTYPE=--) > > > > > [ 41.004562] pc : sdhci_send_command+0x7fc/0xea0 > > > > > [ 41.009086] lr : sdhci_send_command+0x644/0xea0 > > > > > [ 41.013610] sp : ffff80000a9db980 > > > > > [ 41.016915] x29: ffff80000a9db980 x28: ffff80000a9dbcf8 x27: > > > > > 0000000000000002 > > > > > [ 41.024041] x26: 0000000000000000 x25: 0000000000000000 x24: > > > > > ffff0000004237c0 > > > > > [ 41.031166] x23: 000000000000000b x22: ffff000006d05eb8 x21: > > > > > ffff000006d05f28 > > > > > [ 41.038291] x20: ffff000006d05eb8 x19: ffff000000423580 x18: > > > > > fffffc0003fde640 > > > > > [ 41.045416] x17: 0000000000000000 x16: 0000000000000000 x15: > > > > > 0000000000000000 > > > > > [ 41.052540] x14: 0000000000000002 x13: 0000000000100000 x12: > > > > > 0000000000000000 > > > > > [ 41.059664] x11: 0000000000000000 x10: 0000000000000b10 x9 : > > > > > ffff80000a9db9e0 > > > > > [ 41.066789] x8 : ffff000000649af0 x7 : 0000000000000000 x6 : > > > > > ffff0000004deaf8 > > > > > [ 41.073914] x5 : 0000000000000000 x4 : 0000000000000020 x3 : > > > > > 0000000000000000 > > > > > [ 41.081039] x2 : 0000000000020401 x1 : 0000000000000001 x0 : > > > > > 00000000ffffffe4 > > > > > [ 41.088164] Call trace: > > > > > [ 41.090603] sdhci_send_command+0x7fc/0xea0 > > > > > [ 41.094779] sdhci_send_command_retry+0x40/0x140 > > > > > [ 41.099379] sdhci_request+0x70/0xd0 > > > > > [ 41.102949] __mmc_start_request+0x68/0x140 > > > > > [ 41.107127] mmc_start_request+0x78/0xa0 > > > > > [ 41.111042] mmc_blk_mq_issue_rq+0x284/0x9d0 > > > > > [ 41.115305] mmc_mq_queue_rq+0x118/0x2b0 > > > > > [ 41.119221] blk_mq_dispatch_rq_list+0x19c/0x800 > > > > > [ 41.123831] blk_mq_do_dispatch_sched+0x2bc/0x350 > > > > > [ 41.128528] __blk_mq_sched_dispatch_requests+0x10c/0x160 > > > > > [ 41.133917] blk_mq_sched_dispatch_requests+0x40/0x80 > > > > > [ 41.138959] __blk_mq_run_hw_queue+0x80/0xc0 > > > > > [ 41.143222] blk_mq_run_work_fn+0x24/0x30 > > > > > [ 41.147223] process_one_work+0x1d0/0x320 > > > > > [ 41.151230] worker_thread+0x14c/0x450 > > > > > [ 41.154973] kthread+0x10c/0x110 > > > > > [ 41.158195] ret_from_fork+0x10/0x20 > > > > > [ 41.161766] ---[ end trace 0000000000000000 ]--- > > > > > [ 41.171543] ------------[ cut here ]------------ > > > > > [ 41.176169] WARNING: CPU: 1 PID: 49 at drivers/mmc/host/sdhci.c:1152 > > > > > sdhci_send_command+0x7fc/0xea0 > > > > > [ 41.185210] Modules linked in: uio_pdrv_genirq > > > > > [ 41.189650] CPU: 1 PID: 49 Comm: kworker/1:1H Tainted: G W > > > > > 5.19.0-next-20220808 #20 > > > > > [ 41.198679] Hardware name: Marvell A0-AC5X Board (DT) > > > > > [ 41.203720] Workqueue: kblockd blk_mq_run_work_fn > > > > > [ 41.208420] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS > > > > > BTYPE=--) > > > > > [ 41.215369] pc : sdhci_send_command+0x7fc/0xea0 > > > > > [ 41.219892] lr : sdhci_send_command+0x644/0xea0 > > > > > [ 41.224414] sp : ffff80000a9db980 > > > > > [ 41.227721] x29: ffff80000a9db980 x28: ffff80000a9dbcf8 x27: > > > > > 0000000000000002 > > > > > [ 41.234846] x26: 0000000000000000 x25: 0000000000000000 x24: > > > > > ffff0000004237c0 > > > > > [ 41.241972] x23: 000000000000000b x22: ffff000006d001b8 x21: > > > > > ffff000006d00228 > > > > > [ 41.249097] x20: ffff000006d001b8 x19: ffff000000423580 x18: > > > > > 0000000000000000 > > > > > [ 41.256223] x17: 0000000000000000 x16: 0000000000000000 x15: > > > > > 0000000000000000 > > > > > [ 41.263348] x14: 0000000000000002 x13: 0000000000100000 x12: > > > > > 0000000000000000 > > > > > [ 41.270473] x11: 0000000000000001 x10: 0000000000000b10 x9 : > > > > > ffff80000a9db9e0 > > > > > [ 41.277598] x8 : ffff000000649af0 x7 : 0000000000000000 x6 : > > > > > ffff0000004deaf8 > > > > > [ 41.284722] x5 : 0000000000000000 x4 : 0000000000000020 x3 : > > > > > 0000000000000000 > > > > > [ 41.291846] x2 : 0000000000020401 x1 : 0000000000000001 x0 : > > > > > 00000000ffffffe4 > > > > > [ 41.298971] Call trace: > > > > > [ 41.301410] sdhci_send_command+0x7fc/0xea0 > > > > > [ 41.305588] sdhci_send_command_retry+0x40/0x140 > > > > > [ 41.310198] sdhci_request+0x70/0xd0 > > > > > [ 41.313769] __mmc_start_request+0x68/0x140 > > > > > [ 41.317946] mmc_start_request+0x78/0xa0 > > > > > [ 41.321862] mmc_blk_mq_issue_rq+0x284/0x9d0 > > > > > [ 41.326125] mmc_mq_queue_rq+0x118/0x2b0 > > > > > [ 41.330040] blk_mq_dispatch_rq_list+0x19c/0x800 > > > > > [ 41.334649] blk_mq_do_dispatch_sched+0x2bc/0x350 > > > > > [ 41.339346] __blk_mq_sched_dispatch_requests+0x10c/0x160 > > > > > [ 41.344735] blk_mq_sched_dispatch_requests+0x40/0x80 > > > > > [ 41.349779] __blk_mq_run_hw_queue+0x80/0xc0 > > > > > [ 41.354040] blk_mq_run_work_fn+0x24/0x30 > > > > > [ 41.358041] process_one_work+0x1d0/0x320 > > > > > [ 41.362048] worker_thread+0x14c/0x450 > > > > > [ 41.365791] kthread+0x10c/0x110 > > > > > [ 41.369014] ret_from_fork+0x10/0x20 > > > > > [ 41.372584] ---[ end trace 0000000000000000 ]--- > > > > > [ 41.382383] ------------[ cut here ]------------ > > > > > [ 41.387009] WARNING: CPU: 1 PID: 49 at drivers/mmc/host/sdhci.c:1152 > > > > > sdhci_send_command+0x7fc/0xea0 > > > > > [ 41.396051] Modules linked in: uio_pdrv_genirq > > > > > [ 41.400491] CPU: 1 PID: 49 Comm: kworker/1:1H Tainted: G W > > > > > 5.19.0-next-20220808 #20 > > > > > [ 41.409520] Hardware name: Marvell A0-AC5X Board (DT) > > > > > [ 41.414561] Workqueue: kblockd blk_mq_run_work_fn > > > > > [ 41.419261] pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS > > > > > BTYPE=--) > > > > > [ 41.426210] pc : sdhci_send_command+0x7fc/0xea0 > > > > > [ 41.430734] lr : sdhci_send_command+0x644/0xea0 > > > > > [ 41.435256] sp : ffff80000a9db980 > > > > > [ 41.438562] x29: ffff80000a9db980 x28: ffff80000a9dbcf8 x27: > > > > > 0000000000000002 > > > > > [ 41.445687] x26: 0000000000000000 x25: 0000000000000000 x24: > > > > > ffff0000004237c0 > > > > > [ 41.452812] x23: 000000000000000b x22: ffff000006d004b8 x21: > > > > > ffff000006d00528 > > > > > [ 41.459937] x20: ffff000006d004b8 x19: ffff000000423580 x18: > > > > > 0000000000000000 > > > > > [ 41.467062] x17: 0000000000000000 x16: 0000000000000000 x15: > > > > > 0000000000000000 > > > > > [ 41.474187] x14: 0000000000000002 x13: 00000000000ff000 x12: > > > > > 0000000000000000 > > > > > [ 41.481311] x11: 0000000000000001 x10: 0000000000000b10 x9 : > > > > > ffff80000a9db9e0 > > > > > [ 41.488436] x8 : ffff000000649af0 x7 : 0000000000000000 x6 : > > > > > ffff0000004deaf8 > > > > > [ 41.495561] x5 : 0000000000000000 x4 : 0000000000000020 x3 : > > > > > 0000000000000000 > > > > > [ 41.502685] x2 : 0000000000020401 x1 : 0000000000000001 x0 : > > > > > 00000000ffffffe4 > > > > > [ 41.509811] Call trace: > > > > > [ 41.512250] sdhci_send_command+0x7fc/0xea0 > > > > > [ 41.516427] sdhci_send_command_retry+0x40/0x140 > > > > > [ 41.521035] sdhci_request+0x70/0xd0 > > > > > [ 41.524607] __mmc_start_request+0x68/0x140 > > > > > [ 41.528784] mmc_start_request+0x78/0xa0 > > > > > [ 41.532699] mmc_blk_mq_issue_rq+0x284/0x9d0 > > > > > [ 41.536961] mmc_mq_queue_rq+0x118/0x2b0 > > > > > [ 41.540876] blk_mq_dispatch_rq_list+0x19c/0x800 > > > > > [ 41.545485] blk_mq_do_dispatch_sched+0x2bc/0x350 > > > > > [ 41.550182] __blk_mq_sched_dispatch_requests+0x10c/0x160 > > > > > [ 41.555570] blk_mq_sched_dispatch_requests+0x40/0x80 > > > > > [ 41.560613] __blk_mq_run_hw_queue+0x80/0xc0 > > > > > [ 41.564874] blk_mq_run_work_fn+0x24/0x30 > > > > > [ 41.568875] process_one_work+0x1d0/0x320 > > > > > [ 41.572881] worker_thread+0x14c/0x450 > > > > > [ 41.576624] kthread+0x10c/0x110 > > > > > [ 41.579847] ret_from_fork+0x10/0x20 > > > > > [ 41.583428] ---[ end trace 0000000000000000 ]--- > > > > > > > > > > > > > > > > > > > > > > > > > Do I understand correctly that the swiotlb will be used in case DMA > > > > > > > > engine could not map the page in the specified range (limited by dma-ranges or > > > > > > > > dma-mask) ? > > > > > > > > > > > > > > > > > > > > > > > > > > > + host->quirks |= SDHCI_QUIRK_BROKEN_DMA; > > > > > > > > > > + host->quirks |= SDHCI_QUIRK_BROKEN_ADMA; > > > > > > > > > > + dev_info(mmc_dev(mmc), "Disabling DMA because of 2GB DMA access limit.\n"); > > > > > > > > > > + } > > > > > > > > > > + > > > > > > > > > > return xenon_phy_parse_params(dev, host); > > > > > > > > > > } > > > > > > > > > > @@ -682,6 +694,7 @@ static const struct of_device_id sdhci_xenon_dt_ids[] = { > > > > > > > > > > { .compatible = "marvell,armada-ap807-sdhci", .data = (void *)XENON_AP807}, > > > > > > > > > > { .compatible = "marvell,armada-cp110-sdhci", .data = (void *)XENON_CP110}, > > > > > > > > > > { .compatible = "marvell,armada-3700-sdhci", .data = (void *)XENON_A3700}, > > > > > > > > > > + { .compatible = "marvell,ac5-sdhci", .data = (void *)XENON_AP806}, > > > > > > > > > > {} > > > > > > > > > > }; > > > > > > > > > > MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids); > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > Florian > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > > > > > Thanks, > > > > > Vadym Kochan > > > > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-08-16 20:51 ` Vadym Kochan @ 2022-08-17 13:43 ` Robin Murphy 2022-08-17 16:07 ` Vadym Kochan 0 siblings, 1 reply; 15+ messages in thread From: Robin Murphy @ 2022-08-17 13:43 UTC (permalink / raw) To: Vadym Kochan Cc: Adrian Hunter, Christoph Hellwig, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu On 2022-08-16 21:51, Vadym Kochan wrote: [...] >> The one thing to watch out for is that SWIOTLB doesn't necessarily interact >> very well with DMA offsets. Given the intent of >> of_dma_get_max_cpu_address(), I think it ought to work out OK now for >> current kernels on DT systems if everything is described correctly, but >> otherwise it's likely that you end up with ZONE_DMA either being empty or >> containing all memory, so the SWIOTLB buffer ends up being allocated >> anywhere such that it might not actually work as expected. >> >> Robin. > > Hi Robin, > > Thank you for the reply. > > My understanding is that swiotlb is allocated (in case of arm64) > in the following cases: > > #1 when it is forced from the kernel cmdline > > #2 when max_pfn is greater than arm64_dma_phys_limit (and this is used > as the end from which to allocate the swiotlb pool in the > top-botom direction via memblock API). > > #3 using restricted dma-pool > > Of course option #3 works fine because swiotlb is kind of forced to use > particulary this range of memory. > > Both options #1 & #2 causes to use full memory mask even if to specify > dma-ranges in the DT: > > dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; > > or if to specify the opposite: > > dma-ranges = <0x2 0x0 0x0 0x0 0x0 0x80000000>; > > just to make it lower than U32 to pass > > zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits) > > condition, but then it will be re-set in max_zone_phys() by: > > > if (phys_start > U32_MAX) > zone_mask = PHYS_ADDR_MAX; > else if (phys_start > zone_mask) > zone_mask = U32_MAX; Ah, indeed I missed that, sorry. It seems that that change to stop assuming an offset kind of crossed over with the introduction of *_dma_get_max_cpu_address(), but now that that firmware property parsing *is* implemented, in principle it should be equally possible to evaluate the actual offsets as well, and decide whether an offset ZONE_DMA is appropriate or not. Either way, this is definitely the area which needs work if we want to to able to support topologies like this properly. > So, currently I dont see how to pin swiotlb (I see it as a main problem) to some specific range of physical > memory (particulary to the first 2G of RAM). Indeed, if ZONE_DMA and/or ZONE_DMA32 can't be set appropriately, then there's no way to guarantee correct allocation of any DMA buffers, short of hacking it with explicitly placed reserved-memory carveouts. Thanks, Robin. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-08-17 13:43 ` Robin Murphy @ 2022-08-17 16:07 ` Vadym Kochan 2022-08-17 17:23 ` Robin Murphy 0 siblings, 1 reply; 15+ messages in thread From: Vadym Kochan @ 2022-08-17 16:07 UTC (permalink / raw) To: Robin Murphy, Adrian Hunter Cc: Adrian Hunter, Christoph Hellwig, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim Hi Robin, Adrian, On Wed, Aug 17, 2022 at 02:43:46PM +0100, Robin Murphy wrote: > On 2022-08-16 21:51, Vadym Kochan wrote: > [...] > > > The one thing to watch out for is that SWIOTLB doesn't necessarily interact > > > very well with DMA offsets. Given the intent of > > > of_dma_get_max_cpu_address(), I think it ought to work out OK now for > > > current kernels on DT systems if everything is described correctly, but > > > otherwise it's likely that you end up with ZONE_DMA either being empty or > > > containing all memory, so the SWIOTLB buffer ends up being allocated > > > anywhere such that it might not actually work as expected. > > > > > > Robin. > > > > Hi Robin, > > > > Thank you for the reply. > > > > My understanding is that swiotlb is allocated (in case of arm64) > > in the following cases: > > > > #1 when it is forced from the kernel cmdline > > > > #2 when max_pfn is greater than arm64_dma_phys_limit (and this is used > > as the end from which to allocate the swiotlb pool in the > > top-botom direction via memblock API). > > > > #3 using restricted dma-pool > > > > Of course option #3 works fine because swiotlb is kind of forced to use > > particulary this range of memory. > > > > Both options #1 & #2 causes to use full memory mask even if to specify > > dma-ranges in the DT: > > > > dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; > > > > or if to specify the opposite: > > > > dma-ranges = <0x2 0x0 0x0 0x0 0x0 0x80000000>; > > > > just to make it lower than U32 to pass > > > > zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits) > > > > condition, but then it will be re-set in max_zone_phys() by: > > > > if (phys_start > U32_MAX) > > zone_mask = PHYS_ADDR_MAX; > > else if (phys_start > zone_mask) > > zone_mask = U32_MAX; > > Ah, indeed I missed that, sorry. It seems that that change to stop assuming > an offset kind of crossed over with the introduction of > *_dma_get_max_cpu_address(), but now that that firmware property parsing > *is* implemented, in principle it should be equally possible to evaluate the > actual offsets as well, and decide whether an offset ZONE_DMA is appropriate > or not. Either way, this is definitely the area which needs work if we want > to to able to support topologies like this properly. > > > So, currently I dont see how to pin swiotlb (I see it as a main problem) to some specific range of physical > > memory (particulary to the first 2G of RAM). > > Indeed, if ZONE_DMA and/or ZONE_DMA32 can't be set appropriately, then > there's no way to guarantee correct allocation of any DMA buffers, short of > hacking it with explicitly placed reserved-memory carveouts. > I have sent some time ago a solution which binds restricted-dma pool to the eMMC device, so Adrian, Robin do you think this can be acceptable as a temporary solution (at least conceptually) ? I was also thinking would it be OK to introduce something like bounced-dma pool (similar to the restricted one) which will reserve memory for the bounced buffers only ? It should not be hard as looks like it will re-use existing interface between dma and swiotlb ? In that case it would allow to map first 2G of memory to eMMC controller. > Thanks, > Robin. Thanks, Vadym ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-08-17 16:07 ` Vadym Kochan @ 2022-08-17 17:23 ` Robin Murphy 2022-08-18 12:07 ` Vadym Kochan 0 siblings, 1 reply; 15+ messages in thread From: Robin Murphy @ 2022-08-17 17:23 UTC (permalink / raw) To: Vadym Kochan, Adrian Hunter Cc: Christoph Hellwig, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim On 2022-08-17 17:07, Vadym Kochan wrote: > Hi Robin, Adrian, > > On Wed, Aug 17, 2022 at 02:43:46PM +0100, Robin Murphy wrote: >> On 2022-08-16 21:51, Vadym Kochan wrote: >> [...] >>>> The one thing to watch out for is that SWIOTLB doesn't necessarily interact >>>> very well with DMA offsets. Given the intent of >>>> of_dma_get_max_cpu_address(), I think it ought to work out OK now for >>>> current kernels on DT systems if everything is described correctly, but >>>> otherwise it's likely that you end up with ZONE_DMA either being empty or >>>> containing all memory, so the SWIOTLB buffer ends up being allocated >>>> anywhere such that it might not actually work as expected. >>>> >>>> Robin. >>> >>> Hi Robin, >>> >>> Thank you for the reply. >>> >>> My understanding is that swiotlb is allocated (in case of arm64) >>> in the following cases: >>> >>> #1 when it is forced from the kernel cmdline >>> >>> #2 when max_pfn is greater than arm64_dma_phys_limit (and this is used >>> as the end from which to allocate the swiotlb pool in the >>> top-botom direction via memblock API). >>> >>> #3 using restricted dma-pool >>> >>> Of course option #3 works fine because swiotlb is kind of forced to use >>> particulary this range of memory. >>> >>> Both options #1 & #2 causes to use full memory mask even if to specify >>> dma-ranges in the DT: >>> >>> dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; >>> >>> or if to specify the opposite: >>> >>> dma-ranges = <0x2 0x0 0x0 0x0 0x0 0x80000000>; >>> >>> just to make it lower than U32 to pass >>> >>> zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits) >>> >>> condition, but then it will be re-set in max_zone_phys() by: >>> >>> if (phys_start > U32_MAX) >>> zone_mask = PHYS_ADDR_MAX; >>> else if (phys_start > zone_mask) >>> zone_mask = U32_MAX; >> >> Ah, indeed I missed that, sorry. It seems that that change to stop assuming >> an offset kind of crossed over with the introduction of >> *_dma_get_max_cpu_address(), but now that that firmware property parsing >> *is* implemented, in principle it should be equally possible to evaluate the >> actual offsets as well, and decide whether an offset ZONE_DMA is appropriate >> or not. Either way, this is definitely the area which needs work if we want >> to to able to support topologies like this properly. >> >>> So, currently I dont see how to pin swiotlb (I see it as a main problem) to some specific range of physical >>> memory (particulary to the first 2G of RAM). >> >> Indeed, if ZONE_DMA and/or ZONE_DMA32 can't be set appropriately, then >> there's no way to guarantee correct allocation of any DMA buffers, short of >> hacking it with explicitly placed reserved-memory carveouts. >> > > I have sent some time ago a solution which binds restricted-dma pool to > the eMMC device, so Adrian, Robin do you think this can be acceptable as > a temporary solution (at least conceptually) ? > > I was also thinking would it be OK to introduce something like > bounced-dma pool (similar to the restricted one) which will reserve > memory for the bounced buffers only ? It should not be hard as looks > like it will re-use existing interface between dma and swiotlb ? In that > case it would allow to map first 2G of memory to eMMC controller. TBH I'd prefer to fix it (or at least work around it) more generally. Putting made-up things in devicetree to work around shortcomings in kernel code tends to be a hole that's hard to dig yourself back out of. As a bodge that would be just about justifiable in its own terms, does the diff below help at all? Thanks, Robin. ----->8----- diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index b9af30be813e..88f7b26f49db 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -451,7 +451,14 @@ void __init bootmem_init(void) */ void __init mem_init(void) { + /* + * Some platforms still manage to elude our attempt to calculate + * ZONE_DMA appropriately, so encourage the SWIOTLB allocation to go + * as low as it can anyway for the best chance of being usable. + */ + memblock_set_bottom_up(true); swiotlb_init(max_pfn > PFN_DOWN(arm64_dma_phys_limit), SWIOTLB_VERBOSE); + memblock_set_bottom_up(false); /* this will put all unused low memory onto the freelists */ memblock_free_all(); ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-08-17 17:23 ` Robin Murphy @ 2022-08-18 12:07 ` Vadym Kochan 2022-08-21 6:17 ` Christoph Hellwig 0 siblings, 1 reply; 15+ messages in thread From: Vadym Kochan @ 2022-08-18 12:07 UTC (permalink / raw) To: Robin Murphy Cc: Adrian Hunter, Christoph Hellwig, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim Hi Robin, On Wed, Aug 17, 2022 at 06:23:02PM +0100, Robin Murphy wrote: > On 2022-08-17 17:07, Vadym Kochan wrote: > > Hi Robin, Adrian, > > > > On Wed, Aug 17, 2022 at 02:43:46PM +0100, Robin Murphy wrote: > > > On 2022-08-16 21:51, Vadym Kochan wrote: > > > [...] > > > > > The one thing to watch out for is that SWIOTLB doesn't necessarily interact > > > > > very well with DMA offsets. Given the intent of > > > > > of_dma_get_max_cpu_address(), I think it ought to work out OK now for > > > > > current kernels on DT systems if everything is described correctly, but > > > > > otherwise it's likely that you end up with ZONE_DMA either being empty or > > > > > containing all memory, so the SWIOTLB buffer ends up being allocated > > > > > anywhere such that it might not actually work as expected. > > > > > > > > > > Robin. > > > > > > > > Hi Robin, > > > > > > > > Thank you for the reply. > > > > > > > > My understanding is that swiotlb is allocated (in case of arm64) > > > > in the following cases: > > > > > > > > #1 when it is forced from the kernel cmdline > > > > > > > > #2 when max_pfn is greater than arm64_dma_phys_limit (and this is used > > > > as the end from which to allocate the swiotlb pool in the > > > > top-botom direction via memblock API). > > > > > > > > #3 using restricted dma-pool > > > > > > > > Of course option #3 works fine because swiotlb is kind of forced to use > > > > particulary this range of memory. > > > > > > > > Both options #1 & #2 causes to use full memory mask even if to specify > > > > dma-ranges in the DT: > > > > > > > > dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; > > > > > > > > or if to specify the opposite: > > > > > > > > dma-ranges = <0x2 0x0 0x0 0x0 0x0 0x80000000>; > > > > > > > > just to make it lower than U32 to pass > > > > > > > > zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits) > > > > > > > > condition, but then it will be re-set in max_zone_phys() by: > > > > > > > > if (phys_start > U32_MAX) > > > > zone_mask = PHYS_ADDR_MAX; > > > > else if (phys_start > zone_mask) > > > > zone_mask = U32_MAX; > > > > > > Ah, indeed I missed that, sorry. It seems that that change to stop assuming > > > an offset kind of crossed over with the introduction of > > > *_dma_get_max_cpu_address(), but now that that firmware property parsing > > > *is* implemented, in principle it should be equally possible to evaluate the > > > actual offsets as well, and decide whether an offset ZONE_DMA is appropriate > > > or not. Either way, this is definitely the area which needs work if we want > > > to to able to support topologies like this properly. > > > > > > > So, currently I dont see how to pin swiotlb (I see it as a main problem) to some specific range of physical > > > > memory (particulary to the first 2G of RAM). > > > > > > Indeed, if ZONE_DMA and/or ZONE_DMA32 can't be set appropriately, then > > > there's no way to guarantee correct allocation of any DMA buffers, short of > > > hacking it with explicitly placed reserved-memory carveouts. > > > > > > > I have sent some time ago a solution which binds restricted-dma pool to > > the eMMC device, so Adrian, Robin do you think this can be acceptable as > > a temporary solution (at least conceptually) ? > > > > I was also thinking would it be OK to introduce something like > > bounced-dma pool (similar to the restricted one) which will reserve > > memory for the bounced buffers only ? It should not be hard as looks > > like it will re-use existing interface between dma and swiotlb ? In that > > case it would allow to map first 2G of memory to eMMC controller. > > TBH I'd prefer to fix it (or at least work around it) more generally. > Putting made-up things in devicetree to work around shortcomings in > kernel code tends to be a hole that's hard to dig yourself back out of. > As a bodge that would be just about justifiable in its own terms, does > the diff below help at all? > > Thanks, > Robin. > > ----->8----- > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > index b9af30be813e..88f7b26f49db 100644 > --- a/arch/arm64/mm/init.c > +++ b/arch/arm64/mm/init.c > @@ -451,7 +451,14 @@ void __init bootmem_init(void) > */ > void __init mem_init(void) > { > + /* > + * Some platforms still manage to elude our attempt to calculate > + * ZONE_DMA appropriately, so encourage the SWIOTLB allocation to go > + * as low as it can anyway for the best chance of being usable. > + */ > + memblock_set_bottom_up(true); > swiotlb_init(max_pfn > PFN_DOWN(arm64_dma_phys_limit), SWIOTLB_VERBOSE); > + memblock_set_bottom_up(false); > /* this will put all unused low memory onto the freelists */ > memblock_free_all(); It works with the following changes: #1 dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; #3 swiotlb="force" Is it OK to force the memory allocation from the start for the swiotlb ? Or may be do it by new CONFIG which will be enforced by the new CONFIG for the device on that particular SoC ? Thanks, Vadym ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-08-18 12:07 ` Vadym Kochan @ 2022-08-21 6:17 ` Christoph Hellwig 2022-08-22 10:06 ` Robin Murphy 0 siblings, 1 reply; 15+ messages in thread From: Christoph Hellwig @ 2022-08-21 6:17 UTC (permalink / raw) To: Vadym Kochan Cc: Robin Murphy, Adrian Hunter, Christoph Hellwig, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim On Thu, Aug 18, 2022 at 03:07:40PM +0300, Vadym Kochan wrote: > It works with the following changes: > > #1 dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; > > #3 swiotlb="force" > > Is it OK to force the memory allocation from the start for the swiotlb ? It should be ok, but isn't really optimal. I wonder if we should just allow DT to specify the swiotlb buffer location. Basically have yet another RESERVEDMEM_OF_DECLARE variant for it, which shouldn't be all that much work except for figuring out the interaction with the various kernel command line options. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-08-21 6:17 ` Christoph Hellwig @ 2022-08-22 10:06 ` Robin Murphy 2022-09-06 9:22 ` Vadym Kochan 2022-10-13 6:40 ` Vadym Kochan 0 siblings, 2 replies; 15+ messages in thread From: Robin Murphy @ 2022-08-22 10:06 UTC (permalink / raw) To: Christoph Hellwig, Vadym Kochan Cc: Adrian Hunter, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim On 2022-08-21 07:17, Christoph Hellwig wrote: > On Thu, Aug 18, 2022 at 03:07:40PM +0300, Vadym Kochan wrote: >> It works with the following changes: >> >> #1 dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; >> >> #3 swiotlb="force" >> >> Is it OK to force the memory allocation from the start for the swiotlb ? > > It should be ok, but isn't really optimal. > > I wonder if we should just allow DT to specify the swiotlb buffer > location. Basically have yet another RESERVEDMEM_OF_DECLARE variant > for it, which shouldn't be all that much work except for figuring > out the interaction with the various kernel command line options. We already have all the information we need in the DT (and ACPI), the arm64 init code just needs to do a better job of interpreting it properly. I'll see what I can come up with once I've finished what I'm currently tied up in. Thanks, Robin. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-08-22 10:06 ` Robin Murphy @ 2022-09-06 9:22 ` Vadym Kochan 2022-10-13 6:40 ` Vadym Kochan 1 sibling, 0 replies; 15+ messages in thread From: Vadym Kochan @ 2022-09-06 9:22 UTC (permalink / raw) To: Robin Murphy Cc: Christoph Hellwig, Adrian Hunter, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim Hi Robin, On Mon, Aug 22, 2022 at 11:06:43AM +0100, Robin Murphy wrote: > On 2022-08-21 07:17, Christoph Hellwig wrote: > > On Thu, Aug 18, 2022 at 03:07:40PM +0300, Vadym Kochan wrote: > > > It works with the following changes: > > > > > > #1 dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; > > > > > > #3 swiotlb="force" > > > > > > Is it OK to force the memory allocation from the start for the swiotlb ? > > > > It should be ok, but isn't really optimal. > > > > I wonder if we should just allow DT to specify the swiotlb buffer > > location. Basically have yet another RESERVEDMEM_OF_DECLARE variant > > for it, which shouldn't be all that much work except for figuring > > out the interaction with the various kernel command line options. > > We already have all the information we need in the DT (and ACPI), the arm64 > init code just needs to do a better job of interpreting it properly. I'll > see what I can come up with once I've finished what I'm currently tied up > in. > > Thanks, > Robin. Can I help you with something ? Thanks, ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-08-22 10:06 ` Robin Murphy 2022-09-06 9:22 ` Vadym Kochan @ 2022-10-13 6:40 ` Vadym Kochan 2022-11-08 19:05 ` Vadym Kochan 1 sibling, 1 reply; 15+ messages in thread From: Vadym Kochan @ 2022-10-13 6:40 UTC (permalink / raw) To: Robin Murphy, Christoph Hellwig Cc: Adrian Hunter, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim Hi Robin, On Mon, 22 Aug 2022 11:06:43 +0100, Robin Murphy <robin.murphy@arm.com> wrote: > On 2022-08-21 07:17, Christoph Hellwig wrote: > > On Thu, Aug 18, 2022 at 03:07:40PM +0300, Vadym Kochan wrote: > >> It works with the following changes: > >> > >> #1 dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; > >> > >> #3 swiotlb="force" > >> > >> Is it OK to force the memory allocation from the start for the swiotlb ? > > > > It should be ok, but isn't really optimal. > > > > I wonder if we should just allow DT to specify the swiotlb buffer > > location. Basically have yet another RESERVEDMEM_OF_DECLARE variant > > for it, which shouldn't be all that much work except for figuring > > out the interaction with the various kernel command line options. > > We already have all the information we need in the DT (and ACPI), the > arm64 init code just needs to do a better job of interpreting it > properly. I'll see what I can come up with once I've finished what I'm > currently tied up in. > > Thanks, > Robin. Sorry to disturb you, I just 'd like to know if you have some ideas to share or patches to test ? Thank you! ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-10-13 6:40 ` Vadym Kochan @ 2022-11-08 19:05 ` Vadym Kochan 2022-11-09 7:50 ` Adrian Hunter 0 siblings, 1 reply; 15+ messages in thread From: Vadym Kochan @ 2022-11-08 19:05 UTC (permalink / raw) To: Adrian Hunter, Christoph Hellwig Cc: Robin Murphy, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim Hi Adrian, On Thu, 13 Oct 2022 09:40:00 +0300, Vadym Kochan <vadym.kochan@plvision.eu> wrote: > Hi Robin, > > On Mon, 22 Aug 2022 11:06:43 +0100, Robin Murphy <robin.murphy@arm.com> wrote: > > On 2022-08-21 07:17, Christoph Hellwig wrote: > > > On Thu, Aug 18, 2022 at 03:07:40PM +0300, Vadym Kochan wrote: > > >> It works with the following changes: > > >> > > >> #1 dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; > > >> > > >> #3 swiotlb="force" > > >> > > >> Is it OK to force the memory allocation from the start for the swiotlb ? > > > > > > It should be ok, but isn't really optimal. > > > > > > I wonder if we should just allow DT to specify the swiotlb buffer > > > location. Basically have yet another RESERVEDMEM_OF_DECLARE variant > > > for it, which shouldn't be all that much work except for figuring > > > out the interaction with the various kernel command line options. > > > > We already have all the information we need in the DT (and ACPI), the > > arm64 init code just needs to do a better job of interpreting it > > properly. I'll see what I can come up with once I've finished what I'm > > currently tied up in. > > > > Thanks, > > Robin. > > Sorry to disturb you, I just 'd like to know if you have > some ideas to share or patches to test ? > > Thank you! > Since AC5X eMMC controller can fail to work on boards with >2GB memory, and considering that the best fix may not be easy (as it requires arm64 infra changes), so would it be OK to use PIO mode as temporary solution ? I understand that arm64 changes might not be trivial and it might take significant amount of time to implement considering this unusual case, I just think that better to make eMMC working even if it will be slow. Thanks, Vadym ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-11-08 19:05 ` Vadym Kochan @ 2022-11-09 7:50 ` Adrian Hunter 2022-11-09 8:40 ` Vadym Kochan 0 siblings, 1 reply; 15+ messages in thread From: Adrian Hunter @ 2022-11-09 7:50 UTC (permalink / raw) To: Vadym Kochan Cc: Robin Murphy, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim, Christoph Hellwig On 8/11/22 21:05, Vadym Kochan wrote: > Hi Adrian, > > On Thu, 13 Oct 2022 09:40:00 +0300, Vadym Kochan <vadym.kochan@plvision.eu> wrote: >> Hi Robin, >> >> On Mon, 22 Aug 2022 11:06:43 +0100, Robin Murphy <robin.murphy@arm.com> wrote: >>> On 2022-08-21 07:17, Christoph Hellwig wrote: >>>> On Thu, Aug 18, 2022 at 03:07:40PM +0300, Vadym Kochan wrote: >>>>> It works with the following changes: >>>>> >>>>> #1 dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; >>>>> >>>>> #3 swiotlb="force" >>>>> >>>>> Is it OK to force the memory allocation from the start for the swiotlb ? >>>> >>>> It should be ok, but isn't really optimal. >>>> >>>> I wonder if we should just allow DT to specify the swiotlb buffer >>>> location. Basically have yet another RESERVEDMEM_OF_DECLARE variant >>>> for it, which shouldn't be all that much work except for figuring >>>> out the interaction with the various kernel command line options. >>> >>> We already have all the information we need in the DT (and ACPI), the >>> arm64 init code just needs to do a better job of interpreting it >>> properly. I'll see what I can come up with once I've finished what I'm >>> currently tied up in. >>> >>> Thanks, >>> Robin. >> >> Sorry to disturb you, I just 'd like to know if you have >> some ideas to share or patches to test ? >> >> Thank you! >> > > Since AC5X eMMC controller can fail to work on boards with >2GB memory, > and considering that the best fix may not be easy (as it requires arm64 infra changes), > so would it be OK to use PIO mode as temporary solution ? > > I understand that arm64 changes might not be trivial and it might take significant > amount of time to implement considering this unusual case, I just think that better > to make eMMC working even if it will be slow. You can disable DMA if you wish: SDHCI_QUIRK_BROKEN_DMA | SDHCI_QUIRK_BROKEN_ADMA however did you consider SDMA: SDHCI_QUIRK_BROKEN_ADMA which uses a bounce buffer allocated by SDHCI? In any case, you need to add comments to the code and commit message explaining the swiotlb issue. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-11-09 7:50 ` Adrian Hunter @ 2022-11-09 8:40 ` Vadym Kochan 2022-11-09 9:29 ` Adrian Hunter 0 siblings, 1 reply; 15+ messages in thread From: Vadym Kochan @ 2022-11-09 8:40 UTC (permalink / raw) To: Adrian Hunter Cc: Robin Murphy, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim, Christoph Hellwig Hi Adrian, On Wed, 9 Nov 2022 09:50:06 +0200, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 8/11/22 21:05, Vadym Kochan wrote: > > Hi Adrian, > > > > On Thu, 13 Oct 2022 09:40:00 +0300, Vadym Kochan <vadym.kochan@plvision.eu> wrote: > >> Hi Robin, > >> > >> On Mon, 22 Aug 2022 11:06:43 +0100, Robin Murphy <robin.murphy@arm.com> wrote: > >>> On 2022-08-21 07:17, Christoph Hellwig wrote: > >>>> On Thu, Aug 18, 2022 at 03:07:40PM +0300, Vadym Kochan wrote: > >>>>> It works with the following changes: > >>>>> > >>>>> #1 dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; > >>>>> > >>>>> #3 swiotlb="force" > >>>>> > >>>>> Is it OK to force the memory allocation from the start for the swiotlb ? > >>>> > >>>> It should be ok, but isn't really optimal. > >>>> > >>>> I wonder if we should just allow DT to specify the swiotlb buffer > >>>> location. Basically have yet another RESERVEDMEM_OF_DECLARE variant > >>>> for it, which shouldn't be all that much work except for figuring > >>>> out the interaction with the various kernel command line options. > >>> > >>> We already have all the information we need in the DT (and ACPI), the > >>> arm64 init code just needs to do a better job of interpreting it > >>> properly. I'll see what I can come up with once I've finished what I'm > >>> currently tied up in. > >>> > >>> Thanks, > >>> Robin. > >> > >> Sorry to disturb you, I just 'd like to know if you have > >> some ideas to share or patches to test ? > >> > >> Thank you! > >> > > > > Since AC5X eMMC controller can fail to work on boards with >2GB memory, > > and considering that the best fix may not be easy (as it requires arm64 infra changes), > > so would it be OK to use PIO mode as temporary solution ? > > > > I understand that arm64 changes might not be trivial and it might take significant > > amount of time to implement considering this unusual case, I just think that better > > to make eMMC working even if it will be slow. > > You can disable DMA if you wish: > SDHCI_QUIRK_BROKEN_DMA | SDHCI_QUIRK_BROKEN_ADMA > however did you consider SDMA: > SDHCI_QUIRK_BROKEN_ADMA > which uses a bounce buffer allocated by SDHCI? > > In any case, you need to add comments to the code > and commit message explaining the swiotlb issue. > There is a snip from my earlier reply: [snip] > I could use DMA only in 2 ways: > > #1 Use sdhci bounce buffer with SDMA mode > > But there was the issue that SDMA requires that SDHCI v4 mode should > be enabled, and when I enable it via sdhci_enable_v4_mode(host) > then I got error that EXT_CSD can't be recognized. > > But if I comment this line in sdhci.c: > > int sdhci_setup_host(struct sdhci_host *host) > { > ... > > /* SDMA does not support 64-bit DMA if v4 mode not set */ > if ((host->flags & SDHCI_USE_64_BIT_DMA) && !host->v4_mode) { > pr_info("XXX SDMA does not support 64-bit DMA if v4 mode not set\n"); > host->flags &= ~SDHCI_USE_SDMA; > } > > ... > } > > then everything is OK. > > #2 Use restricted-dma-pool in device-tree > > But I am not sure if it is good solution compared to #1. > > Setting only DMA mask did not help because after some time I got > "DMA overflow address" error stack-traces. [snip] Regards, Vadym ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC 2022-11-09 8:40 ` Vadym Kochan @ 2022-11-09 9:29 ` Adrian Hunter 0 siblings, 0 replies; 15+ messages in thread From: Adrian Hunter @ 2022-11-09 9:29 UTC (permalink / raw) To: Vadym Kochan Cc: Robin Murphy, Florian Fainelli, Hu Ziji, Ulf Hansson, linux-mmc, linux-kernel, Elad Nachman, iommu, Mickey Rachamim, Christoph Hellwig On 9/11/22 10:40, Vadym Kochan wrote: > Hi Adrian, > > On Wed, 9 Nov 2022 09:50:06 +0200, Adrian Hunter <adrian.hunter@intel.com> wrote: >> On 8/11/22 21:05, Vadym Kochan wrote: >>> Hi Adrian, >>> >>> On Thu, 13 Oct 2022 09:40:00 +0300, Vadym Kochan <vadym.kochan@plvision.eu> wrote: >>>> Hi Robin, >>>> >>>> On Mon, 22 Aug 2022 11:06:43 +0100, Robin Murphy <robin.murphy@arm.com> wrote: >>>>> On 2022-08-21 07:17, Christoph Hellwig wrote: >>>>>> On Thu, Aug 18, 2022 at 03:07:40PM +0300, Vadym Kochan wrote: >>>>>>> It works with the following changes: >>>>>>> >>>>>>> #1 dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; >>>>>>> >>>>>>> #3 swiotlb="force" >>>>>>> >>>>>>> Is it OK to force the memory allocation from the start for the swiotlb ? >>>>>> >>>>>> It should be ok, but isn't really optimal. >>>>>> >>>>>> I wonder if we should just allow DT to specify the swiotlb buffer >>>>>> location. Basically have yet another RESERVEDMEM_OF_DECLARE variant >>>>>> for it, which shouldn't be all that much work except for figuring >>>>>> out the interaction with the various kernel command line options. >>>>> >>>>> We already have all the information we need in the DT (and ACPI), the >>>>> arm64 init code just needs to do a better job of interpreting it >>>>> properly. I'll see what I can come up with once I've finished what I'm >>>>> currently tied up in. >>>>> >>>>> Thanks, >>>>> Robin. >>>> >>>> Sorry to disturb you, I just 'd like to know if you have >>>> some ideas to share or patches to test ? >>>> >>>> Thank you! >>>> >>> >>> Since AC5X eMMC controller can fail to work on boards with >2GB memory, >>> and considering that the best fix may not be easy (as it requires arm64 infra changes), >>> so would it be OK to use PIO mode as temporary solution ? >>> >>> I understand that arm64 changes might not be trivial and it might take significant >>> amount of time to implement considering this unusual case, I just think that better >>> to make eMMC working even if it will be slow. >> >> You can disable DMA if you wish: >> SDHCI_QUIRK_BROKEN_DMA | SDHCI_QUIRK_BROKEN_ADMA >> however did you consider SDMA: >> SDHCI_QUIRK_BROKEN_ADMA >> which uses a bounce buffer allocated by SDHCI? >> >> In any case, you need to add comments to the code >> and commit message explaining the swiotlb issue. >> > > There is a snip from my earlier reply: > > [snip] > >> I could use DMA only in 2 ways: >> >> #1 Use sdhci bounce buffer with SDMA mode >> >> But there was the issue that SDMA requires that SDHCI v4 mode should >> be enabled, and when I enable it via sdhci_enable_v4_mode(host) >> then I got error that EXT_CSD can't be recognized. >> >> But if I comment this line in sdhci.c: >> >> int sdhci_setup_host(struct sdhci_host *host) >> { >> ... >> >> /* SDMA does not support 64-bit DMA if v4 mode not set */ >> if ((host->flags & SDHCI_USE_64_BIT_DMA) && !host->v4_mode) { Do you need 64-bit DMA? The 2G limitation is still within 32-bits isn't it? >> pr_info("XXX SDMA does not support 64-bit DMA if v4 mode not set\n"); >> host->flags &= ~SDHCI_USE_SDMA; >> } >> >> ... >> } >> >> then everything is OK. >> >> #2 Use restricted-dma-pool in device-tree >> >> But I am not sure if it is good solution compared to #1. >> >> Setting only DMA mask did not help because after some time I got >> "DMA overflow address" error stack-traces. > > [snip] > > Regards, > Vadym ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-11-09 9:29 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220726170711.30324-1-vadym.kochan@plvision.eu>
[not found] ` <139317dc-15e2-ac63-0e04-295e715a7747@gmail.com>
[not found] ` <20220727164532.GA19351@plvision.eu>
[not found] ` <20220801093044.GA22721@plvision.eu>
[not found] ` <9a248303-7a27-e90e-76b3-c01a00be4e3d@intel.com>
[not found] ` <20220808095237.GA15939@plvision.eu>
[not found] ` <6c94411c-4847-526c-d929-c9523aa65c11@intel.com>
[not found] ` <20220808122652.GA6599@plvision.eu>
2022-08-08 12:58 ` [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC Adrian Hunter
2022-08-08 14:06 ` Robin Murphy
2022-08-16 20:51 ` Vadym Kochan
2022-08-17 13:43 ` Robin Murphy
2022-08-17 16:07 ` Vadym Kochan
2022-08-17 17:23 ` Robin Murphy
2022-08-18 12:07 ` Vadym Kochan
2022-08-21 6:17 ` Christoph Hellwig
2022-08-22 10:06 ` Robin Murphy
2022-09-06 9:22 ` Vadym Kochan
2022-10-13 6:40 ` Vadym Kochan
2022-11-08 19:05 ` Vadym Kochan
2022-11-09 7:50 ` Adrian Hunter
2022-11-09 8:40 ` Vadym Kochan
2022-11-09 9:29 ` Adrian Hunter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox