Messages in this thread | | | Subject | Re: [PATCH V5 1/2] ACPI / scan: Support multiple dma windows with different offsets | From | Jianmin Lv <> | Date | Tue, 18 Oct 2022 21:14:20 +0800 |
| |
On 2022/10/18 下午9:06, Robin Murphy wrote: > 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@loongson.cn> >>> 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@huawei.com> 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@hisilicon.com> >>>>> >>>>>> 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.
Do you mean the bug of *kfree(r)*, it should be original r instead changed one, yes?
> I'm writing up the patch now... > Ok, thank you very much for making patch for the issue.
> 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; >>>> >>>>>> . >>
| |