Re: [PATCH V5 1/2] ACPI / scan: Support multiple dma windows with different offsets
From: Robin Murphy
Date: Tue Oct 18 2022 - 09:06:21 EST
On 2022-10-18 14:00, Jianmin Lv wrote:
On 2022/10/18 下午8:56, Rafael J. Wysocki wrote:
On Tue, Oct 18, 2022 at 2:50 PM Jianmin Lv <lvjianmin@xxxxxxxxxxx> wrote:
On 2022/10/18 下午8:32, Yicong Yang wrote:
On 2022/10/18 20:00, Robin Murphy wrote:
On 2022-10-18 11:08, Rafael J. Wysocki wrote:
On Tue, Oct 18, 2022 at 11:33 AM Yicong Yang
<yangyicong@xxxxxxxxxx> wrote:
On 2022/9/11 17:06, Jianmin Lv wrote:
In DT systems configurations, of_dma_get_range() returns struct
bus_dma_region DMA regions; they are used to set-up devices
DMA windows with different offset available for translation
between DMA
address and CPU address.
In ACPI systems configuration, acpi_dma_get_range() does not return
DMA regions yet and that precludes setting up the
dev->dma_range_map
pointer and therefore DMA regions with multiple offsets.
Update acpi_dma_get_range() to return struct bus_dma_region
DMA regions like of_dma_get_range() does.
After updating acpi_dma_get_range(), acpi_arch_dma_setup() is
changed for
ARM64, where the original dma_addr and size are removed as these
arguments are now redundant, and pass 0 and U64_MAX for dma_base
and size of arch_setup_dma_ops; this is a simplification consistent
with what other ACPI architectures also pass to
iommu_setup_dma_ops().
Hi,
With this patch we met problem as well. The DMA coherent mask is
not set correctly
for a ehci usb controller and lead to the below calltrace:
[ 16.699259] ------------[ cut here ]------------
[ 16.703855] WARNING: CPU: 0 PID: 853 at
kernel/dma/mapping.c:499 dma_alloc_attrs+0xc0/0xf0
[ 16.712082] Modules linked in:
[ 16.715124] CPU: 0 PID: 853 Comm: kworker/0:3 Not tainted
6.1.0-rc1-pipe-deadlock+ #5
[ 16.722916] Hardware name: Huawei TaiShan 2280 V2/BC82AMDC,
BIOS 2280-V2 CS V5.B211.01 11/10/2021
[ 16.731745] Workqueue: events work_for_cpu_fn
[ 16.736083] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT
-SSBS BTYPE=--)
[ 16.743013] pc : dma_alloc_attrs+0xc0/0xf0
[ 16.747091] lr : dma_pool_alloc+0x11c/0x200
[ 16.751255] sp : ffff80001e46bb50
[ 16.754554] x29: ffff80001e46bb50 x28: 0000000000000000 x27:
0000000000000000
[ 16.761657] x26: ffff80000b33ce18 x25: ffff800009cc6c48 x24:
0000000000000000
[ 16.768759] x23: ffff00208c830918 x22: 0000000000001000 x21:
0000000000000cc0
[ 16.775861] x20: ffff00208ae82080 x19: ffff0020865c40d0 x18:
0000000000000030
[ 16.782964] x17: 626d756e20737562 x16: 2064656e67697373 x15:
ffff00208ae82640
[ 16.790066] x14: 0000000000000000 x13: 646e756f72616b72 x12:
6f77204348207379
[ 16.797167] x11: 73706f6e79532067 x10: ffff205f43980000 x9 :
ffff80000830b3ac
[ 16.804269] x8 : ffff0020861b1b00 x7 : 0000000000000000 x6 :
0000000000000000
[ 16.811371] x5 : 0000000000000000 x4 : 0000000000000000 x3 :
0000000000000cc0
[ 16.818472] x2 : ffff00208c830918 x1 : 0000000000001000 x0 :
0000000000000000
[ 16.825574] Call trace:
[ 16.828009] dma_alloc_attrs+0xc0/0xf0
[ 16.831741] dma_pool_alloc+0x11c/0x200
[ 16.835559] ehci_qh_alloc+0x60/0x12c
[ 16.839207] ehci_setup+0x18c/0x40c
[ 16.842680] ehci_pci_setup+0xb8/0x680
[ 16.846412] usb_add_hcd+0x310/0x5c0
[ 16.849973] usb_hcd_pci_probe+0x254/0x36c
[ 16.854051] ehci_pci_probe+0x40/0x60
[ 16.857698] local_pci_probe+0x48/0xb4
[ 16.861431] work_for_cpu_fn+0x24/0x40
[ 16.865163] process_one_work+0x1e0/0x450
[ 16.869155] worker_thread+0x2cc/0x44c
[ 16.872886] kthread+0x114/0x120
[ 16.876099] ret_from_fork+0x10/0x20
[ 16.879657] ---[ end trace 0000000000000000 ]---
After reverting this patch the problem resolved. Tested on the
latest 6.1-rc1.
OK, I'll queue up a revert of this and one more commit depending
on it.
FWIW it looks like the fix should be as simple as below.
Looks like it's the case. The change works on my platform, now the
ehci probed successfully again
with no calltrace:
Tested-by: Yicong Yang <yangyicong@xxxxxxxxxxxxx>
Robin.
----->8-----
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 558664d169fc..b6962bff1eae 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1509,6 +1509,7 @@ int acpi_dma_get_range(struct device *dev,
const struct bus_dma_region **map)
goto out;
}
+ *map = r;
list_for_each_entry(rentry, &list, node) {
if (rentry->res->start >= rentry->res->end) {
kfree(r);
@@ -1523,8 +1524,6 @@ int acpi_dma_get_range(struct device *dev,
const struct bus_dma_region **map)
r->offset = rentry->offset;
r++;
}
-
- *map = r;
}
out:
acpi_dev_free_resource_list(&list);
Ohh, yes, map got a wrong value of r because it has been changed.
Well, please send me a working patch by EOD tomorrow.
Ok!
In fact there's another related bug in the error path as well. I'm
writing up the patch now...
Thanks,
Robin
Maybe wo can fix it like this:
truct bus_dma_region *r, *orig_r;
...
orig_r = r = kcalloc(ret + 1, sizeof(*r), GFP_KERNEL);
...
*map = orig_r;
.