The patch series changed acpi_dma_get_range to return dma regions
as of_dma_get_range, so that dev->dma_range_map can be initialized
conveniently.
And acpi_arch_dma_setup for ARM64 is changed wih removing dma_base
and size from it's parameters.
Remove ARCH_HAS_PHYS_TO_DMA for LoongArch and use generic
phys_to_dma/dma_to_phys in include/linux/dma-direct.h.
V1 -> V2
- Removed dma_base and size from acpi_arch_dma_setup' parameters
- Add patch to remove ARCH_HAS_PHYS_TO_DMA for LoongArch
V2 -> V3
- Add kerneldoc for acpi_dma_get_range changing
- Remove redundant code in acpi_arch_dma_setup, and check map
V3 -> V4
- Change title to "Use acpi_arch_dma_setup() and remove ARCH_HAS_PHYS_TO_DMA"
- Use resource_size() to get size
V4 -> V5
- Change commit log for patch: Support multiple dma windows with different offsets
- Remove a unnecessary blank line
- Fix a bug of acpi_dma_get_range
Jianmin Lv (2):
ACPI / scan: Support multiple dma windows with different offsets
LoongArch: Use acpi_arch_dma_setup() and remove ARCH_HAS_PHYS_TO_DMA
arch/loongarch/Kconfig | 1 -
arch/loongarch/kernel/dma.c | 52 ++++++++++++++--------------------
arch/loongarch/kernel/setup.c | 2 +-
drivers/acpi/arm64/dma.c | 28 ++++++++++--------
drivers/acpi/scan.c | 53 +++++++++++++++--------------------
include/acpi/acpi_bus.h | 3 +-
include/linux/acpi.h | 12 ++++----
7 files changed, 70 insertions(+), 81 deletions(-)
--
2.31.1
Use _DMA defined in ACPI spec for translation between
DMA address and CPU address, and implement acpi_arch_dma_setup
for initializing dev->dma_range_map, where acpi_dma_get_range
is called for parsing _DMA.
e.g.
If we have two dma ranges:
cpu address dma address size offset
0x200080000000 0x2080000000 0x400000000 0x1fe000000000
0x400080000000 0x4080000000 0x400000000 0x3fc000000000
_DMA for pci devices should be declared in host bridge as
flowing:
Name (_DMA, ResourceTemplate() {
QWordMemory (ResourceProducer,
PosDecode,
MinFixed,
MaxFixed,
NonCacheable,
ReadWrite,
0x0,
0x4080000000,
0x447fffffff,
0x3fc000000000,
0x400000000,
,
,
)
QWordMemory (ResourceProducer,
PosDecode,
MinFixed,
MaxFixed,
NonCacheable,
ReadWrite,
0x0,
0x2080000000,
0x247fffffff,
0x1fe000000000,
0x400000000,
,
,
)
})
Acked-by: Huacai Chen <[email protected]>
Signed-off-by: Jianmin Lv <[email protected]>
---
arch/loongarch/Kconfig | 1 -
arch/loongarch/kernel/dma.c | 52 ++++++++++++++---------------------
arch/loongarch/kernel/setup.c | 2 +-
include/linux/acpi.h | 9 ++++--
4 files changed, 28 insertions(+), 36 deletions(-)
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index c7dd6ad779af..551dd99e98b8 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -10,7 +10,6 @@ config LOONGARCH
select ARCH_ENABLE_MEMORY_HOTPLUG
select ARCH_ENABLE_MEMORY_HOTREMOVE
select ARCH_HAS_ACPI_TABLE_UPGRADE if ACPI
- select ARCH_HAS_PHYS_TO_DMA
select ARCH_HAS_PTE_SPECIAL
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_INLINE_READ_LOCK if !PREEMPTION
diff --git a/arch/loongarch/kernel/dma.c b/arch/loongarch/kernel/dma.c
index 8c9b5314a13e..7a9c6a9dd2d0 100644
--- a/arch/loongarch/kernel/dma.c
+++ b/arch/loongarch/kernel/dma.c
@@ -2,39 +2,29 @@
/*
* Copyright (C) 2020-2022 Loongson Technology Corporation Limited
*/
-#include <linux/init.h>
+#include <linux/acpi.h>
#include <linux/dma-direct.h>
-#include <linux/dma-mapping.h>
-#include <linux/dma-map-ops.h>
-#include <linux/swiotlb.h>
-#include <asm/bootinfo.h>
-#include <asm/dma.h>
-#include <asm/loongson.h>
-
-/*
- * We extract 4bit node id (bit 44~47) from Loongson-3's
- * 48bit physical address space and embed it into 40bit.
- */
-
-static int node_id_offset;
-
-dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
-{
- long nid = (paddr >> 44) & 0xf;
-
- return ((nid << 44) ^ paddr) | (nid << node_id_offset);
-}
-
-phys_addr_t dma_to_phys(struct device *dev, dma_addr_t daddr)
+void acpi_arch_dma_setup(struct device *dev)
{
- long nid = (daddr >> node_id_offset) & 0xf;
+ int ret;
+ u64 mask, end = 0;
+ const struct bus_dma_region *map = NULL;
+
+ ret = acpi_dma_get_range(dev, &map);
+ if (!ret && map) {
+ const struct bus_dma_region *r = map;
+
+ for (end = 0; r->size; r++) {
+ if (r->dma_start + r->size - 1 > end)
+ end = r->dma_start + r->size - 1;
+ }
+
+ mask = DMA_BIT_MASK(ilog2(end) + 1);
+ dev->bus_dma_limit = end;
+ dev->dma_range_map = map;
+ dev->coherent_dma_mask = min(dev->coherent_dma_mask, mask);
+ *dev->dma_mask = min(*dev->dma_mask, mask);
+ }
- return ((nid << node_id_offset) ^ daddr) | (nid << 44);
-}
-
-void __init plat_swiotlb_setup(void)
-{
- swiotlb_init(true, SWIOTLB_VERBOSE);
- node_id_offset = ((readl(LS7A_DMA_CFG) & LS7A_DMA_NODE_MASK) >> LS7A_DMA_NODE_SHF) + 36;
}
diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
index 8f5c2f9a1a83..d97c69dbe553 100644
--- a/arch/loongarch/kernel/setup.c
+++ b/arch/loongarch/kernel/setup.c
@@ -247,7 +247,7 @@ static void __init arch_mem_init(char **cmdline_p)
sparse_init();
memblock_set_bottom_up(true);
- plat_swiotlb_setup();
+ swiotlb_init(true, SWIOTLB_VERBOSE);
dma_contiguous_reserve(PFN_PHYS(max_low_pfn));
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index bb41623dab77..a71d73a0d43e 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -279,14 +279,17 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa) { }
void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
+#if defined(CONFIG_ARM64) || defined(CONFIG_LOONGARCH)
+void acpi_arch_dma_setup(struct device *dev);
+#else
+static inline void acpi_arch_dma_setup(struct device *dev) { }
+#endif
+
#ifdef CONFIG_ARM64
void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
-void acpi_arch_dma_setup(struct device *dev);
#else
static inline void
acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
-static inline void
-acpi_arch_dma_setup(struct device *dev) { }
#endif
int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
--
2.31.1
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().
Reviewed-by: Robin Murphy <[email protected]>
Signed-off-by: Jianmin Lv <[email protected]>
---
drivers/acpi/arm64/dma.c | 28 ++++++++++++---------
drivers/acpi/scan.c | 53 +++++++++++++++++-----------------------
include/acpi/acpi_bus.h | 3 +--
include/linux/acpi.h | 7 +++---
4 files changed, 44 insertions(+), 47 deletions(-)
diff --git a/drivers/acpi/arm64/dma.c b/drivers/acpi/arm64/dma.c
index f16739ad3cc0..93d796531af3 100644
--- a/drivers/acpi/arm64/dma.c
+++ b/drivers/acpi/arm64/dma.c
@@ -4,11 +4,12 @@
#include <linux/device.h>
#include <linux/dma-direct.h>
-void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
+void acpi_arch_dma_setup(struct device *dev)
{
int ret;
u64 end, mask;
- u64 dmaaddr = 0, size = 0, offset = 0;
+ u64 size = 0;
+ const struct bus_dma_region *map = NULL;
/*
* If @dev is expected to be DMA-capable then the bus code that created
@@ -26,7 +27,19 @@ void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
else
size = 1ULL << 32;
- ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
+ ret = acpi_dma_get_range(dev, &map);
+ if (!ret && map) {
+ const struct bus_dma_region *r = map;
+
+ for (end = 0; r->size; r++) {
+ if (r->dma_start + r->size - 1 > end)
+ end = r->dma_start + r->size - 1;
+ }
+
+ size = end + 1;
+ dev->dma_range_map = map;
+ }
+
if (ret == -ENODEV)
ret = iort_dma_get_ranges(dev, &size);
if (!ret) {
@@ -34,17 +47,10 @@ void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
* Limit coherent and dma mask based on size retrieved from
* firmware.
*/
- end = dmaaddr + size - 1;
+ end = size - 1;
mask = DMA_BIT_MASK(ilog2(end) + 1);
dev->bus_dma_limit = end;
dev->coherent_dma_mask = min(dev->coherent_dma_mask, mask);
*dev->dma_mask = min(*dev->dma_mask, mask);
}
-
- *dma_addr = dmaaddr;
- *dma_size = size;
-
- ret = dma_direct_set_offset(dev, dmaaddr + offset, dmaaddr, size);
-
- dev_dbg(dev, "dma_offset(%#08llx)%s\n", offset, ret ? " failed!" : "");
}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 42cec8120f18..f96ef8536037 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -20,6 +20,7 @@
#include <linux/platform_data/x86/apple.h>
#include <linux/pgtable.h>
#include <linux/crc32.h>
+#include <linux/dma-direct.h>
#include "internal.h"
@@ -1467,25 +1468,21 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
* acpi_dma_get_range() - Get device DMA parameters.
*
* @dev: device to configure
- * @dma_addr: pointer device DMA address result
- * @offset: pointer to the DMA offset result
- * @size: pointer to DMA range size result
+ * @map: pointer to DMA ranges result
*
- * Evaluate DMA regions and return respectively DMA region start, offset
- * and size in dma_addr, offset and size on parsing success; it does not
- * update the passed in values on failure.
+ * Evaluate DMA regions and return pointer to DMA regions on
+ * parsing success; it does not update the passed in values on failure.
*
* Return 0 on success, < 0 on failure.
*/
-int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
- u64 *size)
+int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
{
struct acpi_device *adev;
LIST_HEAD(list);
struct resource_entry *rentry;
int ret;
struct device *dma_dev = dev;
- u64 len, dma_start = U64_MAX, dma_end = 0, dma_offset = 0;
+ struct bus_dma_region *r;
/*
* Walk the device tree chasing an ACPI companion with a _DMA
@@ -1510,31 +1507,28 @@ int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
ret = acpi_dev_get_dma_resources(adev, &list);
if (ret > 0) {
+ r = kcalloc(ret + 1, sizeof(*r), GFP_KERNEL);
+ if (!r) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
list_for_each_entry(rentry, &list, node) {
- if (dma_offset && rentry->offset != dma_offset) {
+ if (rentry->res->start >= rentry->res->end) {
+ kfree(r);
ret = -EINVAL;
- dev_warn(dma_dev, "Can't handle multiple windows with different offsets\n");
+ dev_dbg(dma_dev, "Invalid DMA regions configuration\n");
goto out;
}
- dma_offset = rentry->offset;
- /* Take lower and upper limits */
- if (rentry->res->start < dma_start)
- dma_start = rentry->res->start;
- if (rentry->res->end > dma_end)
- dma_end = rentry->res->end;
- }
-
- if (dma_start >= dma_end) {
- ret = -EINVAL;
- dev_dbg(dma_dev, "Invalid DMA regions configuration\n");
- goto out;
+ r->cpu_start = rentry->res->start;
+ r->dma_start = rentry->res->start - rentry->offset;
+ r->size = resource_size(rentry->res);
+ r->offset = rentry->offset;
+ r++;
}
- *dma_addr = dma_start - dma_offset;
- len = dma_end - dma_start;
- *size = max(len, len + 1);
- *offset = dma_offset;
+ *map = r;
}
out:
acpi_dev_free_resource_list(&list);
@@ -1624,20 +1618,19 @@ int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
const u32 *input_id)
{
const struct iommu_ops *iommu;
- u64 dma_addr = 0, size = 0;
if (attr == DEV_DMA_NOT_SUPPORTED) {
set_dma_ops(dev, &dma_dummy_ops);
return 0;
}
- acpi_arch_dma_setup(dev, &dma_addr, &size);
+ acpi_arch_dma_setup(dev);
iommu = acpi_iommu_configure_id(dev, input_id);
if (PTR_ERR(iommu) == -EPROBE_DEFER)
return -EPROBE_DEFER;
- arch_setup_dma_ops(dev, dma_addr, size,
+ arch_setup_dma_ops(dev, 0, U64_MAX,
iommu, attr == DEV_DMA_COHERENT);
return 0;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e7d27373ff71..73ac4a1d6947 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -613,8 +613,7 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
int acpi_iommu_fwspec_init(struct device *dev, u32 id,
struct fwnode_handle *fwnode,
const struct iommu_ops *ops);
-int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
- u64 *size);
+int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map);
int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
const u32 *input_id);
static inline int acpi_dma_configure(struct device *dev,
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 6f64b2f3dc54..bb41623dab77 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -281,12 +281,12 @@ void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
#ifdef CONFIG_ARM64
void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
-void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size);
+void acpi_arch_dma_setup(struct device *dev);
#else
static inline void
acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
static inline void
-acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) { }
+acpi_arch_dma_setup(struct device *dev) { }
#endif
int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
@@ -977,8 +977,7 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
return DEV_DMA_NOT_SUPPORTED;
}
-static inline int acpi_dma_get_range(struct device *dev, u64 *dma_addr,
- u64 *offset, u64 *size)
+static inline int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
{
return -ENODEV;
}
--
2.31.1
On Sun, Sep 11, 2022 at 05:06:34PM +0800, 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().
>
> Reviewed-by: Robin Murphy <[email protected]>
> Signed-off-by: Jianmin Lv <[email protected]>
> ---
> drivers/acpi/arm64/dma.c | 28 ++++++++++++---------
> drivers/acpi/scan.c | 53 +++++++++++++++++-----------------------
> include/acpi/acpi_bus.h | 3 +--
> include/linux/acpi.h | 7 +++---
> 4 files changed, 44 insertions(+), 47 deletions(-)
Reviewed-by: Lorenzo Pieralisi <[email protected]>
> diff --git a/drivers/acpi/arm64/dma.c b/drivers/acpi/arm64/dma.c
> index f16739ad3cc0..93d796531af3 100644
> --- a/drivers/acpi/arm64/dma.c
> +++ b/drivers/acpi/arm64/dma.c
> @@ -4,11 +4,12 @@
> #include <linux/device.h>
> #include <linux/dma-direct.h>
>
> -void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> +void acpi_arch_dma_setup(struct device *dev)
> {
> int ret;
> u64 end, mask;
> - u64 dmaaddr = 0, size = 0, offset = 0;
> + u64 size = 0;
> + const struct bus_dma_region *map = NULL;
>
> /*
> * If @dev is expected to be DMA-capable then the bus code that created
> @@ -26,7 +27,19 @@ void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> else
> size = 1ULL << 32;
>
> - ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> + ret = acpi_dma_get_range(dev, &map);
> + if (!ret && map) {
> + const struct bus_dma_region *r = map;
> +
> + for (end = 0; r->size; r++) {
> + if (r->dma_start + r->size - 1 > end)
> + end = r->dma_start + r->size - 1;
> + }
> +
> + size = end + 1;
> + dev->dma_range_map = map;
> + }
> +
> if (ret == -ENODEV)
> ret = iort_dma_get_ranges(dev, &size);
> if (!ret) {
> @@ -34,17 +47,10 @@ void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> * Limit coherent and dma mask based on size retrieved from
> * firmware.
> */
> - end = dmaaddr + size - 1;
> + end = size - 1;
> mask = DMA_BIT_MASK(ilog2(end) + 1);
> dev->bus_dma_limit = end;
> dev->coherent_dma_mask = min(dev->coherent_dma_mask, mask);
> *dev->dma_mask = min(*dev->dma_mask, mask);
> }
> -
> - *dma_addr = dmaaddr;
> - *dma_size = size;
> -
> - ret = dma_direct_set_offset(dev, dmaaddr + offset, dmaaddr, size);
> -
> - dev_dbg(dev, "dma_offset(%#08llx)%s\n", offset, ret ? " failed!" : "");
> }
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 42cec8120f18..f96ef8536037 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -20,6 +20,7 @@
> #include <linux/platform_data/x86/apple.h>
> #include <linux/pgtable.h>
> #include <linux/crc32.h>
> +#include <linux/dma-direct.h>
>
> #include "internal.h"
>
> @@ -1467,25 +1468,21 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
> * acpi_dma_get_range() - Get device DMA parameters.
> *
> * @dev: device to configure
> - * @dma_addr: pointer device DMA address result
> - * @offset: pointer to the DMA offset result
> - * @size: pointer to DMA range size result
> + * @map: pointer to DMA ranges result
> *
> - * Evaluate DMA regions and return respectively DMA region start, offset
> - * and size in dma_addr, offset and size on parsing success; it does not
> - * update the passed in values on failure.
> + * Evaluate DMA regions and return pointer to DMA regions on
> + * parsing success; it does not update the passed in values on failure.
> *
> * Return 0 on success, < 0 on failure.
> */
> -int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
> - u64 *size)
> +int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
> {
> struct acpi_device *adev;
> LIST_HEAD(list);
> struct resource_entry *rentry;
> int ret;
> struct device *dma_dev = dev;
> - u64 len, dma_start = U64_MAX, dma_end = 0, dma_offset = 0;
> + struct bus_dma_region *r;
>
> /*
> * Walk the device tree chasing an ACPI companion with a _DMA
> @@ -1510,31 +1507,28 @@ int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
>
> ret = acpi_dev_get_dma_resources(adev, &list);
> if (ret > 0) {
> + r = kcalloc(ret + 1, sizeof(*r), GFP_KERNEL);
> + if (!r) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> list_for_each_entry(rentry, &list, node) {
> - if (dma_offset && rentry->offset != dma_offset) {
> + if (rentry->res->start >= rentry->res->end) {
> + kfree(r);
> ret = -EINVAL;
> - dev_warn(dma_dev, "Can't handle multiple windows with different offsets\n");
> + dev_dbg(dma_dev, "Invalid DMA regions configuration\n");
> goto out;
> }
> - dma_offset = rentry->offset;
>
> - /* Take lower and upper limits */
> - if (rentry->res->start < dma_start)
> - dma_start = rentry->res->start;
> - if (rentry->res->end > dma_end)
> - dma_end = rentry->res->end;
> - }
> -
> - if (dma_start >= dma_end) {
> - ret = -EINVAL;
> - dev_dbg(dma_dev, "Invalid DMA regions configuration\n");
> - goto out;
> + r->cpu_start = rentry->res->start;
> + r->dma_start = rentry->res->start - rentry->offset;
> + r->size = resource_size(rentry->res);
> + r->offset = rentry->offset;
> + r++;
> }
>
> - *dma_addr = dma_start - dma_offset;
> - len = dma_end - dma_start;
> - *size = max(len, len + 1);
> - *offset = dma_offset;
> + *map = r;
> }
> out:
> acpi_dev_free_resource_list(&list);
> @@ -1624,20 +1618,19 @@ int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
> const u32 *input_id)
> {
> const struct iommu_ops *iommu;
> - u64 dma_addr = 0, size = 0;
>
> if (attr == DEV_DMA_NOT_SUPPORTED) {
> set_dma_ops(dev, &dma_dummy_ops);
> return 0;
> }
>
> - acpi_arch_dma_setup(dev, &dma_addr, &size);
> + acpi_arch_dma_setup(dev);
>
> iommu = acpi_iommu_configure_id(dev, input_id);
> if (PTR_ERR(iommu) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
>
> - arch_setup_dma_ops(dev, dma_addr, size,
> + arch_setup_dma_ops(dev, 0, U64_MAX,
> iommu, attr == DEV_DMA_COHERENT);
>
> return 0;
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index e7d27373ff71..73ac4a1d6947 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -613,8 +613,7 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
> int acpi_iommu_fwspec_init(struct device *dev, u32 id,
> struct fwnode_handle *fwnode,
> const struct iommu_ops *ops);
> -int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
> - u64 *size);
> +int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map);
> int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
> const u32 *input_id);
> static inline int acpi_dma_configure(struct device *dev,
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 6f64b2f3dc54..bb41623dab77 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -281,12 +281,12 @@ void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
>
> #ifdef CONFIG_ARM64
> void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
> -void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size);
> +void acpi_arch_dma_setup(struct device *dev);
> #else
> static inline void
> acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
> static inline void
> -acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) { }
> +acpi_arch_dma_setup(struct device *dev) { }
> #endif
>
> int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
> @@ -977,8 +977,7 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
> return DEV_DMA_NOT_SUPPORTED;
> }
>
> -static inline int acpi_dma_get_range(struct device *dev, u64 *dma_addr,
> - u64 *offset, u64 *size)
> +static inline int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
> {
> return -ENODEV;
> }
> --
> 2.31.1
>
On Tue, Sep 13, 2022 at 10:21 AM Lorenzo Pieralisi
<[email protected]> wrote:
>
> On Sun, Sep 11, 2022 at 05:06:34PM +0800, 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().
> >
> > Reviewed-by: Robin Murphy <[email protected]>
> > Signed-off-by: Jianmin Lv <[email protected]>
> > ---
> > drivers/acpi/arm64/dma.c | 28 ++++++++++++---------
> > drivers/acpi/scan.c | 53 +++++++++++++++++-----------------------
> > include/acpi/acpi_bus.h | 3 +--
> > include/linux/acpi.h | 7 +++---
> > 4 files changed, 44 insertions(+), 47 deletions(-)
>
> Reviewed-by: Lorenzo Pieralisi <[email protected]>
Applied as 6.1 material along with the [2/2], thanks!
> > diff --git a/drivers/acpi/arm64/dma.c b/drivers/acpi/arm64/dma.c
> > index f16739ad3cc0..93d796531af3 100644
> > --- a/drivers/acpi/arm64/dma.c
> > +++ b/drivers/acpi/arm64/dma.c
> > @@ -4,11 +4,12 @@
> > #include <linux/device.h>
> > #include <linux/dma-direct.h>
> >
> > -void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> > +void acpi_arch_dma_setup(struct device *dev)
> > {
> > int ret;
> > u64 end, mask;
> > - u64 dmaaddr = 0, size = 0, offset = 0;
> > + u64 size = 0;
> > + const struct bus_dma_region *map = NULL;
> >
> > /*
> > * If @dev is expected to be DMA-capable then the bus code that created
> > @@ -26,7 +27,19 @@ void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> > else
> > size = 1ULL << 32;
> >
> > - ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> > + ret = acpi_dma_get_range(dev, &map);
> > + if (!ret && map) {
> > + const struct bus_dma_region *r = map;
> > +
> > + for (end = 0; r->size; r++) {
> > + if (r->dma_start + r->size - 1 > end)
> > + end = r->dma_start + r->size - 1;
> > + }
> > +
> > + size = end + 1;
> > + dev->dma_range_map = map;
> > + }
> > +
> > if (ret == -ENODEV)
> > ret = iort_dma_get_ranges(dev, &size);
> > if (!ret) {
> > @@ -34,17 +47,10 @@ void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> > * Limit coherent and dma mask based on size retrieved from
> > * firmware.
> > */
> > - end = dmaaddr + size - 1;
> > + end = size - 1;
> > mask = DMA_BIT_MASK(ilog2(end) + 1);
> > dev->bus_dma_limit = end;
> > dev->coherent_dma_mask = min(dev->coherent_dma_mask, mask);
> > *dev->dma_mask = min(*dev->dma_mask, mask);
> > }
> > -
> > - *dma_addr = dmaaddr;
> > - *dma_size = size;
> > -
> > - ret = dma_direct_set_offset(dev, dmaaddr + offset, dmaaddr, size);
> > -
> > - dev_dbg(dev, "dma_offset(%#08llx)%s\n", offset, ret ? " failed!" : "");
> > }
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index 42cec8120f18..f96ef8536037 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -20,6 +20,7 @@
> > #include <linux/platform_data/x86/apple.h>
> > #include <linux/pgtable.h>
> > #include <linux/crc32.h>
> > +#include <linux/dma-direct.h>
> >
> > #include "internal.h"
> >
> > @@ -1467,25 +1468,21 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
> > * acpi_dma_get_range() - Get device DMA parameters.
> > *
> > * @dev: device to configure
> > - * @dma_addr: pointer device DMA address result
> > - * @offset: pointer to the DMA offset result
> > - * @size: pointer to DMA range size result
> > + * @map: pointer to DMA ranges result
> > *
> > - * Evaluate DMA regions and return respectively DMA region start, offset
> > - * and size in dma_addr, offset and size on parsing success; it does not
> > - * update the passed in values on failure.
> > + * Evaluate DMA regions and return pointer to DMA regions on
> > + * parsing success; it does not update the passed in values on failure.
> > *
> > * Return 0 on success, < 0 on failure.
> > */
> > -int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
> > - u64 *size)
> > +int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
> > {
> > struct acpi_device *adev;
> > LIST_HEAD(list);
> > struct resource_entry *rentry;
> > int ret;
> > struct device *dma_dev = dev;
> > - u64 len, dma_start = U64_MAX, dma_end = 0, dma_offset = 0;
> > + struct bus_dma_region *r;
> >
> > /*
> > * Walk the device tree chasing an ACPI companion with a _DMA
> > @@ -1510,31 +1507,28 @@ int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
> >
> > ret = acpi_dev_get_dma_resources(adev, &list);
> > if (ret > 0) {
> > + r = kcalloc(ret + 1, sizeof(*r), GFP_KERNEL);
> > + if (!r) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > list_for_each_entry(rentry, &list, node) {
> > - if (dma_offset && rentry->offset != dma_offset) {
> > + if (rentry->res->start >= rentry->res->end) {
> > + kfree(r);
> > ret = -EINVAL;
> > - dev_warn(dma_dev, "Can't handle multiple windows with different offsets\n");
> > + dev_dbg(dma_dev, "Invalid DMA regions configuration\n");
> > goto out;
> > }
> > - dma_offset = rentry->offset;
> >
> > - /* Take lower and upper limits */
> > - if (rentry->res->start < dma_start)
> > - dma_start = rentry->res->start;
> > - if (rentry->res->end > dma_end)
> > - dma_end = rentry->res->end;
> > - }
> > -
> > - if (dma_start >= dma_end) {
> > - ret = -EINVAL;
> > - dev_dbg(dma_dev, "Invalid DMA regions configuration\n");
> > - goto out;
> > + r->cpu_start = rentry->res->start;
> > + r->dma_start = rentry->res->start - rentry->offset;
> > + r->size = resource_size(rentry->res);
> > + r->offset = rentry->offset;
> > + r++;
> > }
> >
> > - *dma_addr = dma_start - dma_offset;
> > - len = dma_end - dma_start;
> > - *size = max(len, len + 1);
> > - *offset = dma_offset;
> > + *map = r;
> > }
> > out:
> > acpi_dev_free_resource_list(&list);
> > @@ -1624,20 +1618,19 @@ int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
> > const u32 *input_id)
> > {
> > const struct iommu_ops *iommu;
> > - u64 dma_addr = 0, size = 0;
> >
> > if (attr == DEV_DMA_NOT_SUPPORTED) {
> > set_dma_ops(dev, &dma_dummy_ops);
> > return 0;
> > }
> >
> > - acpi_arch_dma_setup(dev, &dma_addr, &size);
> > + acpi_arch_dma_setup(dev);
> >
> > iommu = acpi_iommu_configure_id(dev, input_id);
> > if (PTR_ERR(iommu) == -EPROBE_DEFER)
> > return -EPROBE_DEFER;
> >
> > - arch_setup_dma_ops(dev, dma_addr, size,
> > + arch_setup_dma_ops(dev, 0, U64_MAX,
> > iommu, attr == DEV_DMA_COHERENT);
> >
> > return 0;
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index e7d27373ff71..73ac4a1d6947 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -613,8 +613,7 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
> > int acpi_iommu_fwspec_init(struct device *dev, u32 id,
> > struct fwnode_handle *fwnode,
> > const struct iommu_ops *ops);
> > -int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
> > - u64 *size);
> > +int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map);
> > int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
> > const u32 *input_id);
> > static inline int acpi_dma_configure(struct device *dev,
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index 6f64b2f3dc54..bb41623dab77 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -281,12 +281,12 @@ void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
> >
> > #ifdef CONFIG_ARM64
> > void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
> > -void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size);
> > +void acpi_arch_dma_setup(struct device *dev);
> > #else
> > static inline void
> > acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
> > static inline void
> > -acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) { }
> > +acpi_arch_dma_setup(struct device *dev) { }
> > #endif
> >
> > int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
> > @@ -977,8 +977,7 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
> > return DEV_DMA_NOT_SUPPORTED;
> > }
> >
> > -static inline int acpi_dma_get_range(struct device *dev, u64 *dma_addr,
> > - u64 *offset, u64 *size)
> > +static inline int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
> > {
> > return -ENODEV;
> > }
> > --
> > 2.31.1
> >
Hi,
Well it seems this patch regresses support for the size and offset
fields (as mentioned in the comment) which causes boot failures on
platform which utilize those fields.
ex: The UEFI/ACPI rpi4, which tosses the following on boot when the SD
(emmc2) controller is started:
15.630456] ------------[ cut here ]------------
[ 15.630479] sdhci-iproc BRCME88C:00: swiotlb addr
0xffffffffffffffff+65536 overflow (mask ffffffff, bus limit 0).
[ 15.630513] WARNING: CPU: 1 PID: 8 at kernel/dma/swiotlb.c:879
swiotlb_map+0x370/0x37c
[ 15.630531] Modules linked in: sdhci_iproc xhci_plat_hcd(+) dwc2(+)
mdio_bcm_unimac sdhci_pltfm udc_core sdhci
[ 15.630574] CPU: 1 PID: 8 Comm: kworker/u8:0 Not tainted
6.0.0-rc6bisect+ #315
[ 15.630582] Hardware name: Raspberry Pi Foundation Raspberry Pi 4
Model B/Raspberry Pi 4 Model B, BIOS EDK2-DEV 10/03/2022
[ 15.630590] Workqueue: events_unbound async_run_entry_fn
[ 15.630603] pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS
BTYPE=--)
[ 15.630610] pc : swiotlb_map+0x370/0x37c
[ 15.630619] lr : swiotlb_map+0x370/0x37c
[ 15.630627] sp : ffff800008063a10
[ 15.630632] x29: ffff800008063a10 x28: ffffb5f93db97008 x27:
ffffb5f93e4a8000
[ 15.630650] x26: ffffffffffffffff x25: ffffb5f93e4a8518 x24:
0000000000000000
[ 15.630668] x23: ffff44bb838bc010 x22: 0000000000000000 x21:
0000000000000000
[ 15.630685] x20: 0000000000010000 x19: 0000000117aa0080 x18:
ffffffffffffffff
[ 15.630702] x17: 6b61773d4d455453 x16: 0000000000000080 x15:
ffff800008063628
[ 15.630719] x14: 0000000000000001 x13: ffffb5f93ff0fca2 x12:
6f6c667265766f20
[ 15.630736] x11: 656820747563205b x10: 000000000000000a x9 :
ffffb5f93bb2f024
[ 15.630753] x8 : ffff8000080637a8 x7 : 0000000000000000 x6 :
ffff800008063830
[ 15.630770] x5 : ffff800008060000 x4 : 0000000000000000 x3 :
0000000000000027
[ 15.630787] x2 : 0000000000000001 x1 : ffff44bb80334000 x0 :
0000000000000065
[ 15.630804] Call trace:
[ 15.630809] swiotlb_map+0x370/0x37c
[ 15.630818] dma_direct_map_page+0x1f0/0x210
[ 15.630825] dma_map_page_attrs+0x58/0xc0
[ 15.630833] sdhci_allocate_bounce_buffer+0xa4/0x19c [sdhci]
[ 15.630858] sdhci_setup_host+0xa94/0xe3c [sdhci]
[ 15.630875] sdhci_add_host+0x20/0x60 [sdhci]
[ 15.630892] sdhci_iproc_probe+0x114/0x180 [sdhci_iproc]
[ 15.630906] platform_probe+0x70/0xcc
[ 15.630916] really_probe+0xc8/0x3e0
[ 15.630924] __driver_probe_device+0x84/0x190
[ 15.630932] driver_probe_device+0x44/0x100
[ 15.630940] __driver_attach_async_helper+0x5c/0x100
[ 15.630949] async_run_entry_fn+0x40/0x16c
[ 15.630956] process_one_work+0x2b8/0x704
[ 15.630966] worker_thread+0x7c/0x42c
[ 15.630973] kthread+0xf8/0x104
[ 15.630981] ret_from_fork+0x10/0x20
[ 15.630990] irq event stamp: 525362
[ 15.630996] hardirqs last enabled at (525361): [<ffffb5f93bb30724>]
console_trylock_spinning+0x194/0x1a0
[ 15.631005] hardirqs last disabled at (525362): [<ffffb5f93cb44df4>]
el1_dbg+0x24/0xa0
[ 15.631016] softirqs last enabled at (476786): [<ffffb5f93b9f0a78>]
__do_softirq+0x4b8/0x6a0
[ 15.631023] softirqs last disabled at (476703): [<ffffb5f93ba887a0>]
__irq_exit_rcu+0x13c/0x22c
[ 15.631033] ---[ end trace 0000000000000000 ]---
[ 15.632538] xhci-hcd PNP0D10:00: xHCI Host Controller
[ 15.635534] xhci-hcd PNP0D10:00: new USB bus registered, assigned bus
number 1
[ 15.642726] dwc2 BCM2848:00: supply vusb_a not found, using dummy
regulator
[ 15.679592] bcmgenet BCM6E4E:00: GENET 5.0 EPHY: 0x0000
[ 15.734563] mmc0: SDHCI controller on BCM2847:00 [BCM2847:00] using PIO
[ 15.748640] mmc1: SDHCI controller on BRCME88C:00 [BRCME88C:00] using DMA
[ 15.844849] xhci-hcd PNP0D10:00: can't setup: -12
[ 16.275199] xhci-hcd PNP0D10:00: USB bus 1 deregistered
[ 16.303108] ------------[ cut here ]------------
[ 16.303791] WARNING: CPU: 1 PID: 21 at drivers/mmc/host/sdhci.c:1152
sdhci_prepare_data.isra.0+0x288/0x660 [sdhci]
[ 16.303878] Modules linked in: genet(+) sdhci_iproc xhci_plat_hcd(+)
dwc2(+) mdio_bcm_unimac sdhci_pltfm udc_core sdhci
[ 16.303947] CPU: 1 PID: 21 Comm: kworker/1:0 Tainted: G W
6.0.0-rc6bisect+ #315
[ 16.303962] Hardware name: Raspberry Pi Foundation Raspberry Pi 4
Model B/Raspberry Pi 4 Model B, BIOS EDK2-DEV 10/03/2022
[ 16.303976] Workqueue: events_freezable mmc_rescan
[ 16.304001] pstate: a04000c5 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS
BTYPE=--)
[ 16.304013] pc : sdhci_prepare_data.isra.0+0x288/0x660 [sdhci]
[ 16.304032] lr : sdhci_prepare_data.isra.0+0x280/0x660 [sdhci]
[ 16.304049] sp : ffff8000080e38e0
[ 16.304057] x29: ffff8000080e38e0 x28: 0000000000000000 x27:
ffff44bc76a3a205
[ 16.304077] x26: 0000000000000000 x25: ffff44bb96bb4dc8 x24:
ffff8000080e3ab0
[ 16.304097] x23: 000000000000000b x22: 0000000000000000 x21:
0000000000000000
[ 16.304116] x20: ffff8000080e3ae8 x19: ffff44bb96bb49c0 x18:
0000000000000000
[ 16.304134] x17: ffff8ec338cc9000 x16: 0000000000000060 x15:
0000000000030300
[ 16.304153] x14: 0000000000000018 x13: 001fffffffffffff x12:
0000000000000001
[ 16.304171] x11: fffffbffefa66a48 x10: ffff44bc7efc5ee8 x9 :
ffffb5f93bb64948
[ 16.304189] x8 : ffffb5f93e9b3d38 x7 : 0000000000000018 x6 :
0000000070ce6ce5
[ 16.304206] x5 : 00ffffffffffffff x4 : 0000000000000020 x3 :
0000000000000002
[ 16.304224] x2 : 0000000000000000 x1 : fffffffffffdfbfe x0 :
00000000ffffffe4
[ 16.304243] Call trace:
[ 16.304254] sdhci_prepare_data.isra.0+0x288/0x660 [sdhci]
[ 16.304276] sdhci_send_command+0x220/0x330 [sdhci]
[ 16.304294] sdhci_send_command_retry+0x48/0x15c [sdhci]
[ 16.304311] sdhci_request+0x78/0xd0 [sdhci]
[ 16.304329] __mmc_start_request+0x7c/0x150
[ 16.304352] mmc_start_request+0x9c/0xc4
[ 16.304364] mmc_wait_for_req+0x78/0x10c
[ 16.304373] mmc_app_send_scr+0xf8/0x140
[ 16.304385] mmc_sd_setup_card+0x108/0x350
[ 16.304394] mmc_sd_init_card+0x148/0x4e0
[ 16.304403] mmc_attach_sd+0xc4/0x160
[ 16.304411] mmc_rescan_try_freq+0xe8/0x180
[ 16.304420] mmc_rescan+0x18c/0x1fc
[ 16.304426] process_one_work+0x2b8/0x704
[ 16.304440] worker_thread+0x7c/0x42c
[ 16.304448] kthread+0xf8/0x104
[ 16.304459] ret_from_fork+0x10/0x20
[ 16.304476] irq event stamp: 11012
[ 16.304484] hardirqs last enabled at (11011): [<ffffb5f93cb58564>]
_raw_spin_unlock_irqrestore+0x74/0x90
[ 16.304504] hardirqs last disabled at (11012): [<ffffb5f93cb58cb0>]
_raw_spin_lock_irqsave+0xe0/0x100
[ 16.304513] softirqs last enabled at (5926): [<ffffb5f93b9f0a78>]
__do_softirq+0x4b8/0x6a0
[ 16.304524] softirqs last disabled at (5917): [<ffffb5f93ba887a0>]
__irq_exit_rcu+0x13c/0x22c
[ 16.304536] ---[ end trace 0000000000000000 ]---
[ 16.311207] ------------[ cut here ]------------
The first part of the kernel log can be found here (until it expires in
a day or so) https://paste.centos.org/view/5437f080
and the ACPI/SSDT description of the devices with the size+translations
can be found here:
https://github.com/tianocore/edk2-platforms/blob/4d99e0382809d2adfa289285b3efb57fdffa4598/Platform/RaspberryPi/AcpiTables/Emmc.asl#L36
On 9/11/22 04: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().
>
> Reviewed-by: Robin Murphy <[email protected]>
> Signed-off-by: Jianmin Lv <[email protected]>
> ---
> drivers/acpi/arm64/dma.c | 28 ++++++++++++---------
> drivers/acpi/scan.c | 53 +++++++++++++++++-----------------------
> include/acpi/acpi_bus.h | 3 +--
> include/linux/acpi.h | 7 +++---
> 4 files changed, 44 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/acpi/arm64/dma.c b/drivers/acpi/arm64/dma.c
> index f16739ad3cc0..93d796531af3 100644
> --- a/drivers/acpi/arm64/dma.c
> +++ b/drivers/acpi/arm64/dma.c
> @@ -4,11 +4,12 @@
> #include <linux/device.h>
> #include <linux/dma-direct.h>
>
> -void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> +void acpi_arch_dma_setup(struct device *dev)
> {
> int ret;
> u64 end, mask;
> - u64 dmaaddr = 0, size = 0, offset = 0;
> + u64 size = 0;
> + const struct bus_dma_region *map = NULL;
>
> /*
> * If @dev is expected to be DMA-capable then the bus code that created
> @@ -26,7 +27,19 @@ void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> else
> size = 1ULL << 32;
>
> - ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> + ret = acpi_dma_get_range(dev, &map);
> + if (!ret && map) {
> + const struct bus_dma_region *r = map;
> +
> + for (end = 0; r->size; r++) {
> + if (r->dma_start + r->size - 1 > end)
> + end = r->dma_start + r->size - 1;
> + }
> +
> + size = end + 1;
> + dev->dma_range_map = map;
> + }
> +
> if (ret == -ENODEV)
> ret = iort_dma_get_ranges(dev, &size);
> if (!ret) {
> @@ -34,17 +47,10 @@ void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> * Limit coherent and dma mask based on size retrieved from
> * firmware.
> */
> - end = dmaaddr + size - 1;
> + end = size - 1;
> mask = DMA_BIT_MASK(ilog2(end) + 1);
> dev->bus_dma_limit = end;
> dev->coherent_dma_mask = min(dev->coherent_dma_mask, mask);
> *dev->dma_mask = min(*dev->dma_mask, mask);
> }
> -
> - *dma_addr = dmaaddr;
> - *dma_size = size;
> -
> - ret = dma_direct_set_offset(dev, dmaaddr + offset, dmaaddr, size);
> -
> - dev_dbg(dev, "dma_offset(%#08llx)%s\n", offset, ret ? " failed!" : "");
> }
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 42cec8120f18..f96ef8536037 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -20,6 +20,7 @@
> #include <linux/platform_data/x86/apple.h>
> #include <linux/pgtable.h>
> #include <linux/crc32.h>
> +#include <linux/dma-direct.h>
>
> #include "internal.h"
>
> @@ -1467,25 +1468,21 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
> * acpi_dma_get_range() - Get device DMA parameters.
> *
> * @dev: device to configure
> - * @dma_addr: pointer device DMA address result
> - * @offset: pointer to the DMA offset result
> - * @size: pointer to DMA range size result
> + * @map: pointer to DMA ranges result
> *
> - * Evaluate DMA regions and return respectively DMA region start, offset
> - * and size in dma_addr, offset and size on parsing success; it does not
> - * update the passed in values on failure.
> + * Evaluate DMA regions and return pointer to DMA regions on
> + * parsing success; it does not update the passed in values on failure.
> *
> * Return 0 on success, < 0 on failure.
> */
> -int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
> - u64 *size)
> +int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
> {
> struct acpi_device *adev;
> LIST_HEAD(list);
> struct resource_entry *rentry;
> int ret;
> struct device *dma_dev = dev;
> - u64 len, dma_start = U64_MAX, dma_end = 0, dma_offset = 0;
> + struct bus_dma_region *r;
>
> /*
> * Walk the device tree chasing an ACPI companion with a _DMA
> @@ -1510,31 +1507,28 @@ int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
>
> ret = acpi_dev_get_dma_resources(adev, &list);
> if (ret > 0) {
> + r = kcalloc(ret + 1, sizeof(*r), GFP_KERNEL);
> + if (!r) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> list_for_each_entry(rentry, &list, node) {
> - if (dma_offset && rentry->offset != dma_offset) {
> + if (rentry->res->start >= rentry->res->end) {
> + kfree(r);
> ret = -EINVAL;
> - dev_warn(dma_dev, "Can't handle multiple windows with different offsets\n");
> + dev_dbg(dma_dev, "Invalid DMA regions configuration\n");
> goto out;
> }
> - dma_offset = rentry->offset;
>
> - /* Take lower and upper limits */
> - if (rentry->res->start < dma_start)
> - dma_start = rentry->res->start;
> - if (rentry->res->end > dma_end)
> - dma_end = rentry->res->end;
> - }
> -
> - if (dma_start >= dma_end) {
> - ret = -EINVAL;
> - dev_dbg(dma_dev, "Invalid DMA regions configuration\n");
> - goto out;
> + r->cpu_start = rentry->res->start;
> + r->dma_start = rentry->res->start - rentry->offset;
> + r->size = resource_size(rentry->res);
> + r->offset = rentry->offset;
> + r++;
> }
>
> - *dma_addr = dma_start - dma_offset;
> - len = dma_end - dma_start;
> - *size = max(len, len + 1);
> - *offset = dma_offset;
> + *map = r;
> }
> out:
> acpi_dev_free_resource_list(&list);
> @@ -1624,20 +1618,19 @@ int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
> const u32 *input_id)
> {
> const struct iommu_ops *iommu;
> - u64 dma_addr = 0, size = 0;
>
> if (attr == DEV_DMA_NOT_SUPPORTED) {
> set_dma_ops(dev, &dma_dummy_ops);
> return 0;
> }
>
> - acpi_arch_dma_setup(dev, &dma_addr, &size);
> + acpi_arch_dma_setup(dev);
>
> iommu = acpi_iommu_configure_id(dev, input_id);
> if (PTR_ERR(iommu) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
>
> - arch_setup_dma_ops(dev, dma_addr, size,
> + arch_setup_dma_ops(dev, 0, U64_MAX,
> iommu, attr == DEV_DMA_COHERENT);
>
> return 0;
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index e7d27373ff71..73ac4a1d6947 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -613,8 +613,7 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
> int acpi_iommu_fwspec_init(struct device *dev, u32 id,
> struct fwnode_handle *fwnode,
> const struct iommu_ops *ops);
> -int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
> - u64 *size);
> +int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map);
> int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
> const u32 *input_id);
> static inline int acpi_dma_configure(struct device *dev,
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 6f64b2f3dc54..bb41623dab77 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -281,12 +281,12 @@ void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
>
> #ifdef CONFIG_ARM64
> void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
> -void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size);
> +void acpi_arch_dma_setup(struct device *dev);
> #else
> static inline void
> acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
> static inline void
> -acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) { }
> +acpi_arch_dma_setup(struct device *dev) { }
> #endif
>
> int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
> @@ -977,8 +977,7 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
> return DEV_DMA_NOT_SUPPORTED;
> }
>
> -static inline int acpi_dma_get_range(struct device *dev, u64 *dma_addr,
> - u64 *offset, u64 *size)
> +static inline int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
> {
> return -ENODEV;
> }
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.
Some investigation below...
> Reviewed-by: Robin Murphy <[email protected]>
> Signed-off-by: Jianmin Lv <[email protected]>
> ---
> drivers/acpi/arm64/dma.c | 28 ++++++++++++---------
> drivers/acpi/scan.c | 53 +++++++++++++++++-----------------------
> include/acpi/acpi_bus.h | 3 +--
> include/linux/acpi.h | 7 +++---
> 4 files changed, 44 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/acpi/arm64/dma.c b/drivers/acpi/arm64/dma.c
> index f16739ad3cc0..93d796531af3 100644
> --- a/drivers/acpi/arm64/dma.c
> +++ b/drivers/acpi/arm64/dma.c
> @@ -4,11 +4,12 @@
> #include <linux/device.h>
> #include <linux/dma-direct.h>
>
> -void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> +void acpi_arch_dma_setup(struct device *dev)
> {
> int ret;
> u64 end, mask;
> - u64 dmaaddr = 0, size = 0, offset = 0;
> + u64 size = 0;
> + const struct bus_dma_region *map = NULL;
>
> /*
> * If @dev is expected to be DMA-capable then the bus code that created
> @@ -26,7 +27,19 @@ void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> else
> size = 1ULL << 32;
>
> - ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> + ret = acpi_dma_get_range(dev, &map);
> + if (!ret && map) {
> + const struct bus_dma_region *r = map;
> +
> + for (end = 0; r->size; r++) {
> + if (r->dma_start + r->size - 1 > end)
> + end = r->dma_start + r->size - 1;
> + }
> +
DSDT reports a window of [mem 0x00000000-0xffffffff pref] in _DMA for the target device
but we're not retriving it correctly here. After adding some messages, it shows we haven't
enter this loop and make size as 1 and mask to 0 finally.
Please let me know if you need more information.
Thanks.
> + size = end + 1;
> + dev->dma_range_map = map;
> + }
> +
> if (ret == -ENODEV)
> ret = iort_dma_get_ranges(dev, &size);
> if (!ret) {
> @@ -34,17 +47,10 @@ void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> * Limit coherent and dma mask based on size retrieved from
> * firmware.
> */
> - end = dmaaddr + size - 1;
> + end = size - 1;
> mask = DMA_BIT_MASK(ilog2(end) + 1);
> dev->bus_dma_limit = end;
> dev->coherent_dma_mask = min(dev->coherent_dma_mask, mask);
> *dev->dma_mask = min(*dev->dma_mask, mask);
> }
> -
> - *dma_addr = dmaaddr;
> - *dma_size = size;
> -
> - ret = dma_direct_set_offset(dev, dmaaddr + offset, dmaaddr, size);
> -
> - dev_dbg(dev, "dma_offset(%#08llx)%s\n", offset, ret ? " failed!" : "");
> }
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 42cec8120f18..f96ef8536037 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -20,6 +20,7 @@
> #include <linux/platform_data/x86/apple.h>
> #include <linux/pgtable.h>
> #include <linux/crc32.h>
> +#include <linux/dma-direct.h>
>
> #include "internal.h"
>
> @@ -1467,25 +1468,21 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
> * acpi_dma_get_range() - Get device DMA parameters.
> *
> * @dev: device to configure
> - * @dma_addr: pointer device DMA address result
> - * @offset: pointer to the DMA offset result
> - * @size: pointer to DMA range size result
> + * @map: pointer to DMA ranges result
> *
> - * Evaluate DMA regions and return respectively DMA region start, offset
> - * and size in dma_addr, offset and size on parsing success; it does not
> - * update the passed in values on failure.
> + * Evaluate DMA regions and return pointer to DMA regions on
> + * parsing success; it does not update the passed in values on failure.
> *
> * Return 0 on success, < 0 on failure.
> */
> -int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
> - u64 *size)
> +int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
> {
> struct acpi_device *adev;
> LIST_HEAD(list);
> struct resource_entry *rentry;
> int ret;
> struct device *dma_dev = dev;
> - u64 len, dma_start = U64_MAX, dma_end = 0, dma_offset = 0;
> + struct bus_dma_region *r;
>
> /*
> * Walk the device tree chasing an ACPI companion with a _DMA
> @@ -1510,31 +1507,28 @@ int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
>
> ret = acpi_dev_get_dma_resources(adev, &list);
> if (ret > 0) {
> + r = kcalloc(ret + 1, sizeof(*r), GFP_KERNEL);
> + if (!r) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> list_for_each_entry(rentry, &list, node) {
> - if (dma_offset && rentry->offset != dma_offset) {
> + if (rentry->res->start >= rentry->res->end) {
> + kfree(r);
> ret = -EINVAL;
> - dev_warn(dma_dev, "Can't handle multiple windows with different offsets\n");
> + dev_dbg(dma_dev, "Invalid DMA regions configuration\n");
> goto out;
> }
> - dma_offset = rentry->offset;
>
> - /* Take lower and upper limits */
> - if (rentry->res->start < dma_start)
> - dma_start = rentry->res->start;
> - if (rentry->res->end > dma_end)
> - dma_end = rentry->res->end;
> - }
> -
> - if (dma_start >= dma_end) {
> - ret = -EINVAL;
> - dev_dbg(dma_dev, "Invalid DMA regions configuration\n");
> - goto out;
> + r->cpu_start = rentry->res->start;
> + r->dma_start = rentry->res->start - rentry->offset;
> + r->size = resource_size(rentry->res);
> + r->offset = rentry->offset;
> + r++;
> }
>
> - *dma_addr = dma_start - dma_offset;
> - len = dma_end - dma_start;
> - *size = max(len, len + 1);
> - *offset = dma_offset;
> + *map = r;
> }
> out:
> acpi_dev_free_resource_list(&list);
> @@ -1624,20 +1618,19 @@ int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
> const u32 *input_id)
> {
> const struct iommu_ops *iommu;
> - u64 dma_addr = 0, size = 0;
>
> if (attr == DEV_DMA_NOT_SUPPORTED) {
> set_dma_ops(dev, &dma_dummy_ops);
> return 0;
> }
>
> - acpi_arch_dma_setup(dev, &dma_addr, &size);
> + acpi_arch_dma_setup(dev);
>
> iommu = acpi_iommu_configure_id(dev, input_id);
> if (PTR_ERR(iommu) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
>
> - arch_setup_dma_ops(dev, dma_addr, size,
> + arch_setup_dma_ops(dev, 0, U64_MAX,
> iommu, attr == DEV_DMA_COHERENT);
>
> return 0;
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index e7d27373ff71..73ac4a1d6947 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -613,8 +613,7 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
> int acpi_iommu_fwspec_init(struct device *dev, u32 id,
> struct fwnode_handle *fwnode,
> const struct iommu_ops *ops);
> -int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
> - u64 *size);
> +int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map);
> int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
> const u32 *input_id);
> static inline int acpi_dma_configure(struct device *dev,
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 6f64b2f3dc54..bb41623dab77 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -281,12 +281,12 @@ void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
>
> #ifdef CONFIG_ARM64
> void acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa);
> -void acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size);
> +void acpi_arch_dma_setup(struct device *dev);
> #else
> static inline void
> acpi_numa_gicc_affinity_init(struct acpi_srat_gicc_affinity *pa) { }
> static inline void
> -acpi_arch_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) { }
> +acpi_arch_dma_setup(struct device *dev) { }
> #endif
>
> int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
> @@ -977,8 +977,7 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
> return DEV_DMA_NOT_SUPPORTED;
> }
>
> -static inline int acpi_dma_get_range(struct device *dev, u64 *dma_addr,
> - u64 *offset, u64 *size)
> +static inline int acpi_dma_get_range(struct device *dev, const struct bus_dma_region **map)
> {
> return -ENODEV;
> }
>
On Tue, Oct 18, 2022 at 11:33 AM Yicong Yang <[email protected]> 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.
Thanks!
On 2022-10-18 11:08, Rafael J. Wysocki wrote:
> On Tue, Oct 18, 2022 at 11:33 AM Yicong Yang <[email protected]> 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.
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);
On Tue, Oct 18, 2022 at 2:50 PM Jianmin Lv <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >
> >> 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.
> 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;
>
> >> .
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 <[email protected]> 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
>>>>>> <[email protected]> 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 <[email protected]>
>>>>
>>>>> 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;
>>>
>>>>> .
>
On 2022/10/18 下午8:56, Rafael J. Wysocki wrote:
> On Tue, Oct 18, 2022 at 2:50 PM Jianmin Lv <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>
>>>> 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!
>> 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;
>>
>>>> .
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 <[email protected]> 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 <[email protected]>
> 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);
>
> .
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 <[email protected]>
>>> 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
>>>>>>> <[email protected]> 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 <[email protected]>
>>>>>
>>>>>> 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;
>>>>
>>>>>> .
>>
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 <[email protected]> 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 <[email protected]>
>
>> 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.
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;
>> .
On Tue, Oct 18, 2022 at 12:08:43PM +0200, Rafael J. Wysocki wrote:
> On Tue, Oct 18, 2022 at 11:33 AM Yicong Yang <[email protected]> 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.
Hi Rafael,
there is a fix in the making (you probably noticed):
https://lore.kernel.org/linux-acpi/e94f99cfe09a64c590f009d21c566339117394e2.1666098844.git.robin.murphy@arm.com
please don't queue up a revert yet.
Thanks,
Lorenzo