2017-08-03 12:30:47

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v3 0/5] ACPI: DMA ranges management

This patch series is v3 of a previous posting:

v2->v3:
- Fixed DMA masks computation
- Fixed size computation overflow in acpi_dma_get_range()

v1->v2:
- Reworked acpi_dma_get_range() flow and logs
- Added IORT named component address limits
- Renamed acpi_dev_get_resources() helper function
- Rebased against v4.13-rc3

v2: http://lkml.kernel.org/r/[email protected]
v1: http://lkml.kernel.org/r/[email protected]

-- Original cover letter --

As reported in:

http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com

the bus connecting devices to an IOMMU bus can be smaller in size than
the IOMMU input address bits which results in devices DMA HW bugs in
particular related to IOVA allocation (ie chopping of higher address
bits owing to system bus HW capabilities mismatch with the IOMMU).

Fortunately this problem can be solved through an already present but never
used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define the DMA
window for a specific bus in ACPI and therefore all upstream devices
connected to it.

This small patch series enables _DMA parsing in ACPI core code and
use it in ACPI IORT code in order to detect DMA ranges for devices and
update their data structures to make them work with their related DMA
addressing restrictions.

Cc: Will Deacon <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Feng Kan <[email protected]>
Cc: Jon Masters <[email protected]>
Cc: Robert Moore <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>

Lorenzo Pieralisi (5):
ACPICA: resource_mgr: Allow _DMA method in walk resources
ACPI: Make acpi_dev_get_resources() method agnostic
ACPI: Introduce DMA ranges parsing
ACPI: Make acpi_dma_configure() DMA regions aware
ACPI/IORT: Add IORT named component memory address limits

drivers/acpi/acpica/rsxface.c | 7 ++--
drivers/acpi/arm64/iort.c | 57 ++++++++++++++++++++++++++-
drivers/acpi/resource.c | 82 +++++++++++++++++++++++++++++---------
drivers/acpi/scan.c | 91 +++++++++++++++++++++++++++++++++++++++----
include/acpi/acnames.h | 1 +
include/acpi/acpi_bus.h | 2 +
include/linux/acpi.h | 8 ++++
include/linux/acpi_iort.h | 5 ++-
8 files changed, 219 insertions(+), 34 deletions(-)

--
2.10.0


2017-08-03 12:30:56

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v3 3/5] ACPI: Introduce DMA ranges parsing

Some devices have limited addressing capabilities and cannot
reference the whole memory address space while carrying out DMA
operations (eg some devices with bus address bits range smaller than
system bus - which prevents them from using bus addresses that are
otherwise valid for the system).

The ACPI _DMA object allows bus devices to define the DMA window that is
actually addressable by devices that sit upstream the bus, therefore
providing a means to parse and initialize the devices DMA masks and
addressable DMA range size.

By relying on the generic ACPI kernel layer to retrieve and parse
resources, introduce ACPI core code to parse the _DMA object.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
drivers/acpi/resource.c | 35 +++++++++++++++++++
drivers/acpi/scan.c | 91 ++++++++++++++++++++++++++++++++++++++++++++-----
include/acpi/acpi_bus.h | 2 ++
include/linux/acpi.h | 8 +++++
4 files changed, 128 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 93f1b5c..d85e010 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -635,6 +635,41 @@ int acpi_dev_get_resources(struct acpi_device *adev, struct list_head *list,
}
EXPORT_SYMBOL_GPL(acpi_dev_get_resources);

+static int is_memory(struct acpi_resource *ares, void *not_used)
+{
+ struct resource_win win;
+ struct resource *res = &win.res;
+
+ memset(&win, 0, sizeof(win));
+
+ return !(acpi_dev_resource_memory(ares, res)
+ || acpi_dev_resource_address_space(ares, &win)
+ || acpi_dev_resource_ext_address_space(ares, &win));
+}
+
+/**
+ * acpi_dev_get_dma_resources - Get current DMA resources of a device.
+ * @adev: ACPI device node to get the resources for.
+ * @list: Head of the resultant list of resources (must be empty).
+ *
+ * Evaluate the _DMA method for the given device node and process its
+ * output.
+ *
+ * The resultant struct resource objects are put on the list pointed to
+ * by @list, that must be empty initially, as members of struct
+ * resource_entry objects. Callers of this routine should use
+ * %acpi_dev_free_resource_list() to free that list.
+ *
+ * The number of resources in the output list is returned on success,
+ * an error code reflecting the error condition is returned otherwise.
+ */
+int acpi_dev_get_dma_resources(struct acpi_device *adev, struct list_head *list)
+{
+ return __acpi_dev_get_resources(adev, list, is_memory, NULL,
+ METHOD_NAME__DMA);
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_dma_resources);
+
/**
* acpi_dev_filter_resource_type - Filter ACPI resource according to resource
* types
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 3389729..f024996 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1360,6 +1360,85 @@ 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
+ *
+ * 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.
+ *
+ * Return 0 on success, < 0 on failure.
+ */
+int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
+ u64 *size)
+{
+ 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;
+
+ /*
+ * Walk the device tree chasing an ACPI companion with a _DMA
+ * object while we go. Stop if we find a device with an ACPI
+ * companion containing a _DMA method.
+ */
+ do {
+ adev = ACPI_COMPANION(dma_dev);
+ if (adev && acpi_has_method(adev->handle, METHOD_NAME__DMA))
+ break;
+
+ dma_dev = dma_dev->parent;
+ } while (dma_dev);
+
+ if (!dma_dev)
+ return -ENODEV;
+
+ if (!acpi_has_method(adev->handle, METHOD_NAME__CRS)) {
+ acpi_handle_warn(adev->handle, "_DMA object only valid in object with valid _CRS\n");
+ return -EINVAL;
+ }
+
+ ret = acpi_dev_get_dma_resources(adev, &list);
+ if (ret > 0) {
+ list_for_each_entry(rentry, &list, node) {
+ if (dma_offset && rentry->offset != dma_offset) {
+ ret = -EINVAL;
+ dev_warn(dma_dev, "Can't handle multiple windows with different offsets\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;
+ }
+
+ *dma_addr = dma_start - dma_offset;
+ len = dma_end - dma_start;
+ *size = max(len, len + 1);
+ *offset = dma_offset;
+ }
+ out:
+ acpi_dev_free_resource_list(&list);
+
+ return ret >= 0 ? 0 : ret;
+}
+
+/**
* acpi_dma_configure - Set-up DMA configuration for the device.
* @dev: The pointer to the device
* @attr: device dma attributes
@@ -1367,20 +1446,16 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
{
const struct iommu_ops *iommu;
- u64 size;
+ u64 dma_addr = 0, size = 0;

- iort_set_dma_mask(dev);
+ iort_dma_setup(dev, &dma_addr, &size);

iommu = iort_iommu_configure(dev);
if (IS_ERR(iommu) && PTR_ERR(iommu) == -EPROBE_DEFER)
return -EPROBE_DEFER;

- size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
- /*
- * Assume dma valid range starts at 0 and covers the whole
- * coherent_dma_mask.
- */
- arch_setup_dma_ops(dev, 0, size, iommu, attr == DEV_DMA_COHERENT);
+ arch_setup_dma_ops(dev, dma_addr, size,
+ iommu, attr == DEV_DMA_COHERENT);

return 0;
}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 68bc6be..07eb963 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -578,6 +578,8 @@ struct acpi_pci_root {

bool acpi_dma_supported(struct acpi_device *adev);
enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
+int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
+ u64 *size);
int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr);
void acpi_dma_deconfigure(struct device *dev);

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index c749eef..a5eaff9 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -427,6 +427,8 @@ void acpi_dev_free_resource_list(struct list_head *list);
int acpi_dev_get_resources(struct acpi_device *adev, struct list_head *list,
int (*preproc)(struct acpi_resource *, void *),
void *preproc_data);
+int acpi_dev_get_dma_resources(struct acpi_device *adev,
+ struct list_head *list);
int acpi_dev_filter_resource_type(struct acpi_resource *ares,
unsigned long types);

@@ -774,6 +776,12 @@ 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)
+{
+ return -ENODEV;
+}
+
static inline int acpi_dma_configure(struct device *dev,
enum dev_dma_attr attr)
{
--
2.10.0

2017-08-03 12:30:50

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v3 1/5] ACPICA: resource_mgr: Allow _DMA method in walk resources

ACPICA commit 7636d2fb683d5699a5c14e949fba9ac52e91d4c1

The _DMA object contains a resource template, this change adds support
for the walk resources function so that ACPI devices containing a _DMA
object can actually parse it to detect DMA ranges for the respective
bus.

Link: https://github.com/acpica/acpica/commit/7636d2fb
Signed-off-by: Lorenzo Pieralisi <[email protected]>
---
drivers/acpi/acpica/rsxface.c | 7 ++++---
include/acpi/acnames.h | 1 +
2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpica/rsxface.c b/drivers/acpi/acpica/rsxface.c
index 59a4f9e..be65e65 100644
--- a/drivers/acpi/acpica/rsxface.c
+++ b/drivers/acpi/acpica/rsxface.c
@@ -615,7 +615,7 @@ ACPI_EXPORT_SYMBOL(acpi_walk_resource_buffer)
* device we are querying
* name - Method name of the resources we want.
* (METHOD_NAME__CRS, METHOD_NAME__PRS, or
- * METHOD_NAME__AEI)
+ * METHOD_NAME__AEI or METHOD_NAME__DMA)
* user_function - Called for each resource
* context - Passed to user_function
*
@@ -641,11 +641,12 @@ acpi_walk_resources(acpi_handle device_handle,
if (!device_handle || !user_function || !name ||
(!ACPI_COMPARE_NAME(name, METHOD_NAME__CRS) &&
!ACPI_COMPARE_NAME(name, METHOD_NAME__PRS) &&
- !ACPI_COMPARE_NAME(name, METHOD_NAME__AEI))) {
+ !ACPI_COMPARE_NAME(name, METHOD_NAME__AEI) &&
+ !ACPI_COMPARE_NAME(name, METHOD_NAME__DMA))) {
return_ACPI_STATUS(AE_BAD_PARAMETER);
}

- /* Get the _CRS/_PRS/_AEI resource list */
+ /* Get the _CRS/_PRS/_AEI/_DMA resource list */

buffer.length = ACPI_ALLOCATE_LOCAL_BUFFER;
status = acpi_rs_get_method_data(device_handle, name, &buffer);
diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h
index b421584..d8dd3bf 100644
--- a/include/acpi/acnames.h
+++ b/include/acpi/acnames.h
@@ -54,6 +54,7 @@
#define METHOD_NAME__CLS "_CLS"
#define METHOD_NAME__CRS "_CRS"
#define METHOD_NAME__DDN "_DDN"
+#define METHOD_NAME__DMA "_DMA"
#define METHOD_NAME__HID "_HID"
#define METHOD_NAME__INI "_INI"
#define METHOD_NAME__PLD "_PLD"
--
2.10.0

2017-08-03 12:31:01

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v3 5/5] ACPI/IORT: Add IORT named component memory address limits

IORT named components provide firmware configuration describing
how many address bits a given device is capable of generating
to address memory.

Add code to the kernel to retrieve memory address limits
configuration for IORT named components and configure DMA masks
accordingly.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Nate Watterson <[email protected]>
---
drivers/acpi/arm64/iort.c | 54 ++++++++++++++++++++++++++++++++---------------
1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 606af87..5b9e8dc2 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -680,6 +680,24 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
return ret ? NULL : ops;
}

+static int nc_dma_get_range(struct device *dev, u64 *size)
+{
+ struct acpi_iort_node *node;
+ struct acpi_iort_named_component *ncomp;
+
+ node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
+ iort_match_node_callback, dev);
+ if (!node)
+ return -ENODEV;
+
+ ncomp = (struct acpi_iort_named_component *)node->node_data;
+
+ *size = ncomp->memory_address_limit >= 64 ? U64_MAX :
+ 1ULL<<ncomp->memory_address_limit;
+
+ return 0;
+}
+
/**
* iort_dma_setup() - Set-up device DMA parameters.
*
@@ -708,24 +726,26 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)

size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);

- if (dev_is_pci(dev)) {
+ if (dev_is_pci(dev))
ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
- if (!ret) {
- msb = fls64(dmaaddr + size - 1);
- /*
- * Round-up to the power-of-two mask or set
- * the mask to the whole 64-bit address space
- * in case the DMA region covers the full
- * memory window.
- */
- mask = msb == 64 ? U64_MAX : (1ULL << msb) - 1;
- /*
- * Limit coherent and dma mask based on size
- * retrieved from firmware.
- */
- dev->coherent_dma_mask = mask;
- *dev->dma_mask = mask;
- }
+ else
+ ret = nc_dma_get_range(dev, &size);
+
+ if (!ret) {
+ msb = fls64(dmaaddr + size - 1);
+ /*
+ * Round-up to the power-of-two mask or set
+ * the mask to the whole 64-bit address space
+ * in case the DMA region covers the full
+ * memory window.
+ */
+ mask = msb == 64 ? U64_MAX : (1ULL << msb) - 1;
+ /*
+ * Limit coherent and dma mask based on size
+ * retrieved from firmware.
+ */
+ dev->coherent_dma_mask = mask;
+ *dev->dma_mask = mask;
}

*dma_addr = dmaaddr;
--
2.10.0

2017-08-03 12:31:19

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v3 4/5] ACPI: Make acpi_dma_configure() DMA regions aware

Current ACPI DMA configuration set-up device DMA capabilities through
kernel defaults that do not take into account platform specific DMA
configurations reported by firmware.

By leveraging the ACPI acpi_dev_get_dma_resources() API, add code
in acpi_dma_configure() to retrieve the DMA regions to correctly
set-up PCI devices DMA parameters.

Rework the ACPI IORT kernel API to make sure they can accommodate
the DMA set-up required by firmware. By making PCI devices DMA set-up
ACPI IORT specific, the kernel is shielded from unwanted regressions
that could be triggered by parsing DMA resources on arches that were
previously ignoring them (ie x86/ia64), leaving kernel behaviour
unchanged on those arches.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
drivers/acpi/arm64/iort.c | 37 +++++++++++++++++++++++++++++++++++--
include/linux/acpi_iort.h | 5 +++--
2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index a3215ee..606af87 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -681,12 +681,17 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
}

/**
- * iort_set_dma_mask - Set-up dma mask for a device.
+ * iort_dma_setup() - Set-up device DMA parameters.
*
* @dev: device to configure
+ * @dma_addr: device DMA address result pointer
+ * @size: DMA range size result pointer
*/
-void iort_set_dma_mask(struct device *dev)
+void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
{
+ u64 mask, dmaaddr = 0, size = 0, offset = 0;
+ int ret, msb;
+
/*
* Set default coherent_dma_mask to 32 bit. Drivers are expected to
* setup the correct supported mask.
@@ -700,6 +705,34 @@ void iort_set_dma_mask(struct device *dev)
*/
if (!dev->dma_mask)
dev->dma_mask = &dev->coherent_dma_mask;
+
+ size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
+
+ if (dev_is_pci(dev)) {
+ ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
+ if (!ret) {
+ msb = fls64(dmaaddr + size - 1);
+ /*
+ * Round-up to the power-of-two mask or set
+ * the mask to the whole 64-bit address space
+ * in case the DMA region covers the full
+ * memory window.
+ */
+ mask = msb == 64 ? U64_MAX : (1ULL << msb) - 1;
+ /*
+ * Limit coherent and dma mask based on size
+ * retrieved from firmware.
+ */
+ dev->coherent_dma_mask = mask;
+ *dev->dma_mask = mask;
+ }
+ }
+
+ *dma_addr = dmaaddr;
+ *dma_size = size;
+
+ dev->dma_pfn_offset = PFN_DOWN(offset);
+ dev_dbg(dev, "dma_pfn_offset(%#08llx)\n", offset);
}

/**
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 8379d40..8d3f0bf 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -36,7 +36,7 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id);
void acpi_configure_pmsi_domain(struct device *dev);
int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id);
/* IOMMU interface */
-void iort_set_dma_mask(struct device *dev);
+void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *size);
const struct iommu_ops *iort_iommu_configure(struct device *dev);
#else
static inline void acpi_iort_init(void) { }
@@ -47,7 +47,8 @@ static inline struct irq_domain *iort_get_device_domain(struct device *dev,
{ return NULL; }
static inline void acpi_configure_pmsi_domain(struct device *dev) { }
/* IOMMU interface */
-static inline void iort_set_dma_mask(struct device *dev) { }
+static inline void iort_dma_setup(struct device *dev, u64 *dma_addr,
+ u64 *size) { }
static inline
const struct iommu_ops *iort_iommu_configure(struct device *dev)
{ return NULL; }
--
2.10.0

2017-08-03 12:30:54

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v3 2/5] ACPI: Make acpi_dev_get_resources() method agnostic

The function acpi_dev_get_resources() is completely generic and
can be used to parse resource objects that are not necessarily
coming from the _CRS method but also from other objects eg _DMA
that have the same _CRS resource format.

Create an acpi_dev_get_resources() helper, internal to the ACPI
resources parsing compilation unit, __acpi_dev_get_resources(),
that takes a const char* parameter to detect which ACPI method should be
called to retrieve the resources list and make acpi_dev_get_resources()
call it with a method name _CRS leaving the API behaviour unchanged.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
drivers/acpi/resource.c | 53 +++++++++++++++++++++++++++++--------------------
1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index cd4c427..93f1b5c 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -573,6 +573,35 @@ static acpi_status acpi_dev_process_resource(struct acpi_resource *ares,
return AE_OK;
}

+static int __acpi_dev_get_resources(struct acpi_device *adev,
+ struct list_head *list,
+ int (*preproc)(struct acpi_resource *, void *),
+ void *preproc_data, char *method)
+{
+ struct res_proc_context c;
+ acpi_status status;
+
+ if (!adev || !adev->handle || !list_empty(list))
+ return -EINVAL;
+
+ if (!acpi_has_method(adev->handle, method))
+ return 0;
+
+ c.list = list;
+ c.preproc = preproc;
+ c.preproc_data = preproc_data;
+ c.count = 0;
+ c.error = 0;
+ status = acpi_walk_resources(adev->handle, method,
+ acpi_dev_process_resource, &c);
+ if (ACPI_FAILURE(status)) {
+ acpi_dev_free_resource_list(list);
+ return c.error ? c.error : -EIO;
+ }
+
+ return c.count;
+}
+
/**
* acpi_dev_get_resources - Get current resources of a device.
* @adev: ACPI device node to get the resources for.
@@ -601,28 +630,8 @@ int acpi_dev_get_resources(struct acpi_device *adev, struct list_head *list,
int (*preproc)(struct acpi_resource *, void *),
void *preproc_data)
{
- struct res_proc_context c;
- acpi_status status;
-
- if (!adev || !adev->handle || !list_empty(list))
- return -EINVAL;
-
- if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
- return 0;
-
- c.list = list;
- c.preproc = preproc;
- c.preproc_data = preproc_data;
- c.count = 0;
- c.error = 0;
- status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
- acpi_dev_process_resource, &c);
- if (ACPI_FAILURE(status)) {
- acpi_dev_free_resource_list(list);
- return c.error ? c.error : -EIO;
- }
-
- return c.count;
+ return __acpi_dev_get_resources(adev, list, preproc, preproc_data,
+ METHOD_NAME__CRS);
}
EXPORT_SYMBOL_GPL(acpi_dev_get_resources);

--
2.10.0

2017-08-03 13:25:59

by Hanjun Guo

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ACPI: DMA ranges management

On 2017/8/3 20:32, Lorenzo Pieralisi wrote:
> This patch series is v3 of a previous posting:
>
> v2->v3:
> - Fixed DMA masks computation
> - Fixed size computation overflow in acpi_dma_get_range()
>
> v1->v2:
> - Reworked acpi_dma_get_range() flow and logs
> - Added IORT named component address limits
> - Renamed acpi_dev_get_resources() helper function
> - Rebased against v4.13-rc3
>
> v2: http://lkml.kernel.org/r/[email protected]
> v1: http://lkml.kernel.org/r/[email protected]
>
> -- Original cover letter --
>
> As reported in:
>
> http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com
>
> the bus connecting devices to an IOMMU bus can be smaller in size than
> the IOMMU input address bits which results in devices DMA HW bugs in
> particular related to IOVA allocation (ie chopping of higher address
> bits owing to system bus HW capabilities mismatch with the IOMMU).
>
> Fortunately this problem can be solved through an already present but never
> used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define the DMA
> window for a specific bus in ACPI and therefore all upstream devices
> connected to it.
>
> This small patch series enables _DMA parsing in ACPI core code and
> use it in ACPI IORT code in order to detect DMA ranges for devices and
> update their data structures to make them work with their related DMA
> addressing restrictions.
>
> Cc: Will Deacon <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Feng Kan <[email protected]>
> Cc: Jon Masters <[email protected]>
> Cc: Robert Moore <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Zhang Rui <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>

with the whole patch set:

Reviewed-by: Hanjun Guo <[email protected]>

I tested this patch set with no _DMA in DSDT but with named
component in IORT table, seeing no regressions on D05.

Thanks
Hanjun

2017-08-03 15:16:50

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ACPI: DMA ranges management

On Thu, Aug 03, 2017 at 01:32:34PM +0100, Lorenzo Pieralisi wrote:
> This patch series is v3 of a previous posting:
>
> v2->v3:
> - Fixed DMA masks computation
> - Fixed size computation overflow in acpi_dma_get_range()
>
> v1->v2:
> - Reworked acpi_dma_get_range() flow and logs
> - Added IORT named component address limits
> - Renamed acpi_dev_get_resources() helper function
> - Rebased against v4.13-rc3
>
> v2: http://lkml.kernel.org/r/[email protected]
> v1: http://lkml.kernel.org/r/[email protected]

For the arm64 bits:

Acked-by: Will Deacon <[email protected]>

Will

2017-08-03 15:45:14

by Nate Watterson

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ACPI: DMA ranges management

Hi Lorenzo,

On 8/3/2017 8:32 AM, Lorenzo Pieralisi wrote:
> This patch series is v3 of a previous posting:
>
> v2->v3:
> - Fixed DMA masks computation
> - Fixed size computation overflow in acpi_dma_get_range()
>
> v1->v2:
> - Reworked acpi_dma_get_range() flow and logs
> - Added IORT named component address limits
> - Renamed acpi_dev_get_resources() helper function
> - Rebased against v4.13-rc3
>
> v2: http://lkml.kernel.org/r/[email protected]
> v1: http://lkml.kernel.org/r/[email protected]
>
> -- Original cover letter --
>
> As reported in:
>
> http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com
>
> the bus connecting devices to an IOMMU bus can be smaller in size than
> the IOMMU input address bits which results in devices DMA HW bugs in
> particular related to IOVA allocation (ie chopping of higher address
> bits owing to system bus HW capabilities mismatch with the IOMMU).
>
> Fortunately this problem can be solved through an already present but never
> used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define the DMA
> window for a specific bus in ACPI and therefore all upstream devices
> connected to it.
>
> This small patch series enables _DMA parsing in ACPI core code and
> use it in ACPI IORT code in order to detect DMA ranges for devices and
> update their data structures to make them work with their related DMA
> addressing restrictions.
>
> Cc: Will Deacon <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Feng Kan <[email protected]>
> Cc: Jon Masters <[email protected]>
> Cc: Robert Moore <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Zhang Rui <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
>
> Lorenzo Pieralisi (5):
> ACPICA: resource_mgr: Allow _DMA method in walk resources
> ACPI: Make acpi_dev_get_resources() method agnostic
> ACPI: Introduce DMA ranges parsing
> ACPI: Make acpi_dma_configure() DMA regions aware
> ACPI/IORT: Add IORT named component memory address limits
>
> drivers/acpi/acpica/rsxface.c | 7 ++--
> drivers/acpi/arm64/iort.c | 57 ++++++++++++++++++++++++++-
> drivers/acpi/resource.c | 82 +++++++++++++++++++++++++++++---------
> drivers/acpi/scan.c | 91 +++++++++++++++++++++++++++++++++++++++----
> include/acpi/acnames.h | 1 +
> include/acpi/acpi_bus.h | 2 +
> include/linux/acpi.h | 8 ++++
> include/linux/acpi_iort.h | 5 ++-
> 8 files changed, 219 insertions(+), 34 deletions(-)
>

I tested with named components and with _DMA objects at a variety of
sizes and verified the configured masks matched the expected values.

Tested-by: Nate Watterson <[email protected]>

One general question I had while testing the patch is whether it is
possible to define a complete 64-bit range using the _DMA method. For
instance, to get a 64-bit dma_mask I had to use a non-zero MIN value
so that LEN would not overflow.

QWORDMemory(
ResourceConsumer,
PosDecode, // _DEC
MinFixed, // _MIF
MaxFixed, // _MAF
Prefetchable, // _MEM
ReadWrite, // _RW
0, // _GRA
0x1000, // _MIN
0xffffffffffffffff, // _MAX
0x0, // _TRA
0xfffffffffffff000, // _LEN
,
,
,
)

-Nate

--
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

2017-08-03 17:21:39

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ACPI: DMA ranges management

On 03/08/17 16:45, Nate Watterson wrote:
> Hi Lorenzo,
>
> On 8/3/2017 8:32 AM, Lorenzo Pieralisi wrote:
>> This patch series is v3 of a previous posting:
>>
>> v2->v3:
>> - Fixed DMA masks computation
>> - Fixed size computation overflow in acpi_dma_get_range()
>>
>> v1->v2:
>> - Reworked acpi_dma_get_range() flow and logs
>> - Added IORT named component address limits
>> - Renamed acpi_dev_get_resources() helper function
>> - Rebased against v4.13-rc3
>>
>> v2:
>> http://lkml.kernel.org/r/[email protected]
>> v1:
>> http://lkml.kernel.org/r/[email protected]
>>
>> -- Original cover letter --
>>
>> As reported in:
>>
>> http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com
>>
>>
>> the bus connecting devices to an IOMMU bus can be smaller in size than
>> the IOMMU input address bits which results in devices DMA HW bugs in
>> particular related to IOVA allocation (ie chopping of higher address
>> bits owing to system bus HW capabilities mismatch with the IOMMU).
>>
>> Fortunately this problem can be solved through an already present but
>> never
>> used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define
>> the DMA
>> window for a specific bus in ACPI and therefore all upstream devices
>> connected to it.
>>
>> This small patch series enables _DMA parsing in ACPI core code and
>> use it in ACPI IORT code in order to detect DMA ranges for devices and
>> update their data structures to make them work with their related DMA
>> addressing restrictions.
>>
>> Cc: Will Deacon <[email protected]>
>> Cc: Hanjun Guo <[email protected]>
>> Cc: Feng Kan <[email protected]>
>> Cc: Jon Masters <[email protected]>
>> Cc: Robert Moore <[email protected]>
>> Cc: Robin Murphy <[email protected]>
>> Cc: Zhang Rui <[email protected]>
>> Cc: "Rafael J. Wysocki" <[email protected]>
>>
>> Lorenzo Pieralisi (5):
>> ACPICA: resource_mgr: Allow _DMA method in walk resources
>> ACPI: Make acpi_dev_get_resources() method agnostic
>> ACPI: Introduce DMA ranges parsing
>> ACPI: Make acpi_dma_configure() DMA regions aware
>> ACPI/IORT: Add IORT named component memory address limits
>>
>> drivers/acpi/acpica/rsxface.c | 7 ++--
>> drivers/acpi/arm64/iort.c | 57 ++++++++++++++++++++++++++-
>> drivers/acpi/resource.c | 82
>> +++++++++++++++++++++++++++++---------
>> drivers/acpi/scan.c | 91
>> +++++++++++++++++++++++++++++++++++++++----
>> include/acpi/acnames.h | 1 +
>> include/acpi/acpi_bus.h | 2 +
>> include/linux/acpi.h | 8 ++++
>> include/linux/acpi_iort.h | 5 ++-
>> 8 files changed, 219 insertions(+), 34 deletions(-)
>>
>
> I tested with named components and with _DMA objects at a variety of
> sizes and verified the configured masks matched the expected values.
>
> Tested-by: Nate Watterson <[email protected]>
>
> One general question I had while testing the patch is whether it is
> possible to define a complete 64-bit range using the _DMA method. For
> instance, to get a 64-bit dma_mask I had to use a non-zero MIN value
> so that LEN would not overflow.

As far as I understand ACPI, all addresses are at most 64-bit. Thus the
only valid DMA range of size 2^64 would be one with a minimum of 0 and
no offset, and that can already be described by the simple absence of a
_DMA object.

Robin.

>
> QWORDMemory(
> ResourceConsumer,
> PosDecode, // _DEC
> MinFixed, // _MIF
> MaxFixed, // _MAF
> Prefetchable, // _MEM
> ReadWrite, // _RW
> 0, // _GRA
> 0x1000, // _MIN
> 0xffffffffffffffff, // _MAX
> 0x0, // _TRA
> 0xfffffffffffff000, // _LEN
> ,
> ,
> ,
> )
>
> -Nate
>

2017-08-06 05:13:15

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] ACPI: Introduce DMA ranges parsing

Hi Lorenzo,

[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.13-rc3 next-20170804]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Lorenzo-Pieralisi/ACPICA-resource_mgr-Allow-_DMA-method-in-walk-resources/20170804-185152
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: ia64-allnoconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64

Note: the linux-review/Lorenzo-Pieralisi/ACPICA-resource_mgr-Allow-_DMA-method-in-walk-resources/20170804-185152 HEAD 37e91dddbbeb771d4df0001f716607f1dd8719f8 builds fine.
It only hurts bisectibility.

All errors (new ones prefixed by >>):

drivers/acpi/scan.c: In function 'acpi_dma_configure':
>> drivers/acpi/scan.c:1451:2: error: implicit declaration of function 'iort_dma_setup' [-Werror=implicit-function-declaration]
iort_dma_setup(dev, &dma_addr, &size);
^~~~~~~~~~~~~~
cc1: some warnings being treated as errors

vim +/iort_dma_setup +1451 drivers/acpi/scan.c

1440
1441 /**
1442 * acpi_dma_configure - Set-up DMA configuration for the device.
1443 * @dev: The pointer to the device
1444 * @attr: device dma attributes
1445 */
1446 int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
1447 {
1448 const struct iommu_ops *iommu;
1449 u64 dma_addr = 0, size = 0;
1450
> 1451 iort_dma_setup(dev, &dma_addr, &size);
1452
1453 iommu = iort_iommu_configure(dev);
1454 if (IS_ERR(iommu) && PTR_ERR(iommu) == -EPROBE_DEFER)
1455 return -EPROBE_DEFER;
1456
1457 arch_setup_dma_ops(dev, dma_addr, size,
1458 iommu, attr == DEV_DMA_COHERENT);
1459
1460 return 0;
1461 }
1462 EXPORT_SYMBOL_GPL(acpi_dma_configure);
1463

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.18 kB)
.config.gz (5.88 kB)
Download all attachments

2017-08-06 09:40:27

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] ACPI: Introduce DMA ranges parsing

On Sun, Aug 06, 2017 at 01:12:55PM +0800, kbuild test robot wrote:
> Hi Lorenzo,
>
> [auto build test ERROR on pm/linux-next]
> [also build test ERROR on v4.13-rc3 next-20170804]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Lorenzo-Pieralisi/ACPICA-resource_mgr-Allow-_DMA-method-in-walk-resources/20170804-185152
> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> config: ia64-allnoconfig (attached as .config)
> compiler: ia64-linux-gcc (GCC) 6.2.0
> reproduce:
> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=ia64
>
> Note: the linux-review/Lorenzo-Pieralisi/ACPICA-resource_mgr-Allow-_DMA-method-in-walk-resources/20170804-185152 HEAD 37e91dddbbeb771d4df0001f716607f1dd8719f8 builds fine.
> It only hurts bisectibility.

Sigh, it is a silly v2->v3 rebase mistake, a change hunk that should be
in patch 4 ended up in patch 3, series as a whole is fine but this must
be fixed.

Rafael, please let me know the best way to handle this, I think
me sending a v4 is the simplest so that you can just update the branch.

Apologies and thanks.

Lorenzo

> All errors (new ones prefixed by >>):
>
> drivers/acpi/scan.c: In function 'acpi_dma_configure':
> >> drivers/acpi/scan.c:1451:2: error: implicit declaration of function 'iort_dma_setup' [-Werror=implicit-function-declaration]
> iort_dma_setup(dev, &dma_addr, &size);
> ^~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
>
> vim +/iort_dma_setup +1451 drivers/acpi/scan.c
>
> 1440
> 1441 /**
> 1442 * acpi_dma_configure - Set-up DMA configuration for the device.
> 1443 * @dev: The pointer to the device
> 1444 * @attr: device dma attributes
> 1445 */
> 1446 int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
> 1447 {
> 1448 const struct iommu_ops *iommu;
> 1449 u64 dma_addr = 0, size = 0;
> 1450
> > 1451 iort_dma_setup(dev, &dma_addr, &size);
> 1452
> 1453 iommu = iort_iommu_configure(dev);
> 1454 if (IS_ERR(iommu) && PTR_ERR(iommu) == -EPROBE_DEFER)
> 1455 return -EPROBE_DEFER;
> 1456
> 1457 arch_setup_dma_ops(dev, dma_addr, size,
> 1458 iommu, attr == DEV_DMA_COHERENT);
> 1459
> 1460 return 0;
> 1461 }
> 1462 EXPORT_SYMBOL_GPL(acpi_dma_configure);
> 1463
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation


2017-08-06 11:21:11

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] ACPI: Introduce DMA ranges parsing

On Sun, Aug 6, 2017 at 11:42 AM, Lorenzo Pieralisi
<[email protected]> wrote:
> On Sun, Aug 06, 2017 at 01:12:55PM +0800, kbuild test robot wrote:
>> Hi Lorenzo,
>>
>> [auto build test ERROR on pm/linux-next]
>> [also build test ERROR on v4.13-rc3 next-20170804]
>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>
>> url: https://github.com/0day-ci/linux/commits/Lorenzo-Pieralisi/ACPICA-resource_mgr-Allow-_DMA-method-in-walk-resources/20170804-185152
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
>> config: ia64-allnoconfig (attached as .config)
>> compiler: ia64-linux-gcc (GCC) 6.2.0
>> reproduce:
>> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> # save the attached .config to linux build tree
>> make.cross ARCH=ia64
>>
>> Note: the linux-review/Lorenzo-Pieralisi/ACPICA-resource_mgr-Allow-_DMA-method-in-walk-resources/20170804-185152 HEAD 37e91dddbbeb771d4df0001f716607f1dd8719f8 builds fine.
>> It only hurts bisectibility.
>
> Sigh, it is a silly v2->v3 rebase mistake, a change hunk that should be
> in patch 4 ended up in patch 3, series as a whole is fine but this must
> be fixed.
>
> Rafael, please let me know the best way to handle this, I think
> me sending a v4 is the simplest so that you can just update the branch.

Please send updates of patches [3/4] and [4/4] alone.

> Apologies and thanks.

No worries.

2017-08-09 21:14:53

by Jeremy Linton

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ACPI: DMA ranges management

Hi,

Better late than never I guess..

On 08/03/2017 07:32 AM, Lorenzo Pieralisi wrote:
> This patch series is v3 of a previous posting:
>
> v2->v3:
> - Fixed DMA masks computation
> - Fixed size computation overflow in acpi_dma_get_range()
>
> v1->v2:
> - Reworked acpi_dma_get_range() flow and logs
> - Added IORT named component address limits
> - Renamed acpi_dev_get_resources() helper function
> - Rebased against v4.13-rc3
>
> v2: http://lkml.kernel.org/r/[email protected]
> v1: http://lkml.kernel.org/r/[email protected]
>
> -- Original cover letter --
>
> As reported in:
>
> http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com
>
> the bus connecting devices to an IOMMU bus can be smaller in size than
> the IOMMU input address bits which results in devices DMA HW bugs in
> particular related to IOVA allocation (ie chopping of higher address
> bits owing to system bus HW capabilities mismatch with the IOMMU).
>
> Fortunately this problem can be solved through an already present but never
> used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define the DMA
> window for a specific bus in ACPI and therefore all upstream devices
> connected to it.
>
> This small patch series enables _DMA parsing in ACPI core code and
> use it in ACPI IORT code in order to detect DMA ranges for devices and
> update their data structures to make them work with their related DMA
> addressing restrictions.
>
> Cc: Will Deacon <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Feng Kan <[email protected]>
> Cc: Jon Masters <[email protected]>
> Cc: Robert Moore <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Zhang Rui <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
>
> Lorenzo Pieralisi (5):
> ACPICA: resource_mgr: Allow _DMA method in walk resources
> ACPI: Make acpi_dev_get_resources() method agnostic
> ACPI: Introduce DMA ranges parsing
> ACPI: Make acpi_dma_configure() DMA regions aware
> ACPI/IORT: Add IORT named component memory address limits
>
> drivers/acpi/acpica/rsxface.c | 7 ++--
> drivers/acpi/arm64/iort.c | 57 ++++++++++++++++++++++++++-
> drivers/acpi/resource.c | 82 +++++++++++++++++++++++++++++---------
> drivers/acpi/scan.c | 91 +++++++++++++++++++++++++++++++++++++++----
> include/acpi/acnames.h | 1 +
> include/acpi/acpi_bus.h | 2 +
> include/linux/acpi.h | 8 ++++
> include/linux/acpi_iort.h | 5 ++-
> 8 files changed, 219 insertions(+), 34 deletions(-)

Ok, despite being merged already I think its worthwhile to say that I've
been testing this with:

Method(_DMA, 0, Serialized)
{
Return (ResourceTemplate()
{
QWORDMemory(
ResourceConsumer,
PosDecode, // _DEC
MinFixed, // _MIF
MaxFixed, // _MAF
Prefetchable, // _MEM
ReadWrite, // _RW
0, // _GRA
0x10000000, // _MIN
0x1fffffff, // _MAX
0x000000000, // _TRA
0x10000000, // _LEN
,
,
,
)
})
} // Method(_DMA)

(and a couple minor variations)

and a fair number of debug statements sprinkled around to verify that
the IOVAs are appropriately limited. So I don't see anything wrong with
the code and it appears to work and the devices behind a bridge limited
like this continue to work as long as sane values are placed in the
min/max/len fields.

Thanks,

Tested-by: Jeremy Linton <[email protected]>

2017-08-11 08:50:10

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ACPI: DMA ranges management

On Wed, Aug 09, 2017 at 04:14:43PM -0500, Jeremy Linton wrote:
> Hi,
>
> Better late than never I guess..
>
> On 08/03/2017 07:32 AM, Lorenzo Pieralisi wrote:
> >This patch series is v3 of a previous posting:
> >
> >v2->v3:
> > - Fixed DMA masks computation
> > - Fixed size computation overflow in acpi_dma_get_range()
> >
> >v1->v2:
> > - Reworked acpi_dma_get_range() flow and logs
> > - Added IORT named component address limits
> > - Renamed acpi_dev_get_resources() helper function
> > - Rebased against v4.13-rc3
> >
> >v2: http://lkml.kernel.org/r/[email protected]
> >v1: http://lkml.kernel.org/r/[email protected]
> >
> >-- Original cover letter --
> >
> >As reported in:
> >
> >http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com
> >
> >the bus connecting devices to an IOMMU bus can be smaller in size than
> >the IOMMU input address bits which results in devices DMA HW bugs in
> >particular related to IOVA allocation (ie chopping of higher address
> >bits owing to system bus HW capabilities mismatch with the IOMMU).
> >
> >Fortunately this problem can be solved through an already present but never
> >used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define the DMA
> >window for a specific bus in ACPI and therefore all upstream devices
> >connected to it.
> >
> >This small patch series enables _DMA parsing in ACPI core code and
> >use it in ACPI IORT code in order to detect DMA ranges for devices and
> >update their data structures to make them work with their related DMA
> >addressing restrictions.
> >
> >Cc: Will Deacon <[email protected]>
> >Cc: Hanjun Guo <[email protected]>
> >Cc: Feng Kan <[email protected]>
> >Cc: Jon Masters <[email protected]>
> >Cc: Robert Moore <[email protected]>
> >Cc: Robin Murphy <[email protected]>
> >Cc: Zhang Rui <[email protected]>
> >Cc: "Rafael J. Wysocki" <[email protected]>
> >
> >Lorenzo Pieralisi (5):
> > ACPICA: resource_mgr: Allow _DMA method in walk resources
> > ACPI: Make acpi_dev_get_resources() method agnostic
> > ACPI: Introduce DMA ranges parsing
> > ACPI: Make acpi_dma_configure() DMA regions aware
> > ACPI/IORT: Add IORT named component memory address limits
> >
> > drivers/acpi/acpica/rsxface.c | 7 ++--
> > drivers/acpi/arm64/iort.c | 57 ++++++++++++++++++++++++++-
> > drivers/acpi/resource.c | 82 +++++++++++++++++++++++++++++---------
> > drivers/acpi/scan.c | 91 +++++++++++++++++++++++++++++++++++++++----
> > include/acpi/acnames.h | 1 +
> > include/acpi/acpi_bus.h | 2 +
> > include/linux/acpi.h | 8 ++++
> > include/linux/acpi_iort.h | 5 ++-
> > 8 files changed, 219 insertions(+), 34 deletions(-)
>
> Ok, despite being merged already I think its worthwhile to say that
> I've been testing this with:
>
> Method(_DMA, 0, Serialized)
> {
> Return (ResourceTemplate()
> {
> QWORDMemory(
> ResourceConsumer,

I asked to update the ACPI specifications because this should be
ResourceProducer, we need an errata to sort this out before it
becomes a problem.

> PosDecode, // _DEC
> MinFixed, // _MIF
> MaxFixed, // _MAF
> Prefetchable, // _MEM
> ReadWrite, // _RW
> 0, // _GRA
> 0x10000000, // _MIN
> 0x1fffffff, // _MAX
> 0x000000000, // _TRA
> 0x10000000, // _LEN
> ,
> ,
> ,
> )
> })
> } // Method(_DMA)
>
> (and a couple minor variations)
>
> and a fair number of debug statements sprinkled around to verify
> that the IOVAs are appropriately limited. So I don't see anything
> wrong with the code and it appears to work and the devices behind a
> bridge limited like this continue to work as long as sane values are
> placed in the min/max/len fields.
>
> Thanks,
>
> Tested-by: Jeremy Linton <[email protected]>

Thank you very much Jeremy for testing it, appreciated please let me
know if you spot anything wrong with it on the machines you are
running tests on.

Lorenzo

2017-08-14 18:42:03

by Feng Kan

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ACPI: DMA ranges management

On Thu, Aug 3, 2017 at 5:32 AM, Lorenzo Pieralisi
<[email protected]> wrote:
> This patch series is v3 of a previous posting:
>
> v2->v3:
> - Fixed DMA masks computation
> - Fixed size computation overflow in acpi_dma_get_range()
>
> v1->v2:
> - Reworked acpi_dma_get_range() flow and logs
> - Added IORT named component address limits
> - Renamed acpi_dev_get_resources() helper function
> - Rebased against v4.13-rc3
>
> v2: http://lkml.kernel.org/r/[email protected]
> v1: http://lkml.kernel.org/r/[email protected]
>
> -- Original cover letter --
>
> As reported in:
>
> http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com
>
> the bus connecting devices to an IOMMU bus can be smaller in size than
> the IOMMU input address bits which results in devices DMA HW bugs in
> particular related to IOVA allocation (ie chopping of higher address
> bits owing to system bus HW capabilities mismatch with the IOMMU).
>
> Fortunately this problem can be solved through an already present but never
> used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define the DMA
> window for a specific bus in ACPI and therefore all upstream devices
> connected to it.
>
> This small patch series enables _DMA parsing in ACPI core code and
> use it in ACPI IORT code in order to detect DMA ranges for devices and
> update their data structures to make them work with their related DMA
> addressing restrictions.
>
> Cc: Will Deacon <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Feng Kan <[email protected]>
> Cc: Jon Masters <[email protected]>
> Cc: Robert Moore <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Zhang Rui <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
>
> Lorenzo Pieralisi (5):
> ACPICA: resource_mgr: Allow _DMA method in walk resources
> ACPI: Make acpi_dev_get_resources() method agnostic
> ACPI: Introduce DMA ranges parsing
> ACPI: Make acpi_dma_configure() DMA regions aware
> ACPI/IORT: Add IORT named component memory address limits
>
> drivers/acpi/acpica/rsxface.c | 7 ++--
> drivers/acpi/arm64/iort.c | 57 ++++++++++++++++++++++++++-
> drivers/acpi/resource.c | 82 +++++++++++++++++++++++++++++---------
> drivers/acpi/scan.c | 91 +++++++++++++++++++++++++++++++++++++++----
> include/acpi/acnames.h | 1 +
> include/acpi/acpi_bus.h | 2 +
> include/linux/acpi.h | 8 ++++
> include/linux/acpi_iort.h | 5 ++-
> 8 files changed, 219 insertions(+), 34 deletions(-)
>
> --
> 2.10.0
>
Works on XGene.
Tested-by: Feng Kan <[email protected]>

2017-12-01 10:39:17

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ACPI: DMA ranges management

On Thu, Nov 30, 2017 at 04:43:00PM -0800, Feng Kan wrote:
> On Wed, Nov 29, 2017 at 11:28 PM, Feng Kan <[email protected]> wrote:
> > On Thu, Aug 3, 2017 at 5:32 AM, Lorenzo Pieralisi
> > <[email protected]> wrote:
> >> This patch series is v3 of a previous posting:
> >>
> >> v2->v3:
> >> - Fixed DMA masks computation
> >> - Fixed size computation overflow in acpi_dma_get_range()
> >>
> >> v1->v2:
> >> - Reworked acpi_dma_get_range() flow and logs
> >> - Added IORT named component address limits
> >> - Renamed acpi_dev_get_resources() helper function
> >> - Rebased against v4.13-rc3
> >>
> >> v2: http://lkml.kernel.org/r/[email protected]
> >> v1: http://lkml.kernel.org/r/[email protected]
> >>
> >> -- Original cover letter --
> >>
> >> As reported in:
> >>
> >> http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com
> >>
> >> the bus connecting devices to an IOMMU bus can be smaller in size than
> >> the IOMMU input address bits which results in devices DMA HW bugs in
> >> particular related to IOVA allocation (ie chopping of higher address
> >> bits owing to system bus HW capabilities mismatch with the IOMMU).
> >>
> >> Fortunately this problem can be solved through an already present but never
> >> used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define the DMA
> >> window for a specific bus in ACPI and therefore all upstream devices
> >> connected to it.
> >>
> >> This small patch series enables _DMA parsing in ACPI core code and
> >> use it in ACPI IORT code in order to detect DMA ranges for devices and
> >> update their data structures to make them work with their related DMA
> >> addressing restrictions.
> >>
> >> Cc: Will Deacon <[email protected]>
> >> Cc: Hanjun Guo <[email protected]>
> >> Cc: Feng Kan <[email protected]>
> >> Cc: Jon Masters <[email protected]>
> >> Cc: Robert Moore <[email protected]>
> >> Cc: Robin Murphy <[email protected]>
> >> Cc: Zhang Rui <[email protected]>
> >> Cc: "Rafael J. Wysocki" <[email protected]>
> >>
> >> Lorenzo Pieralisi (5):
> >> ACPICA: resource_mgr: Allow _DMA method in walk resources
> >> ACPI: Make acpi_dev_get_resources() method agnostic
> >> ACPI: Introduce DMA ranges parsing
> >> ACPI: Make acpi_dma_configure() DMA regions aware
> >> ACPI/IORT: Add IORT named component memory address limits
> >>
> >> drivers/acpi/acpica/rsxface.c | 7 ++--
> >> drivers/acpi/arm64/iort.c | 57 ++++++++++++++++++++++++++-
> >> drivers/acpi/resource.c | 82 +++++++++++++++++++++++++++++---------
> >> drivers/acpi/scan.c | 91 +++++++++++++++++++++++++++++++++++++++----
> >> include/acpi/acnames.h | 1 +
> >> include/acpi/acpi_bus.h | 2 +
> >> include/linux/acpi.h | 8 ++++
> >> include/linux/acpi_iort.h | 5 ++-
> >> 8 files changed, 219 insertions(+), 34 deletions(-)
> >>
> >> --
> >> 2.10.0
> >>
> > Lorenzo:
> >
> > A network driver can use pci_set_dma_mask or its like to override what
> > is done with this patch here.
> > Which would result in iova allocation greater than the original _DMA
> > aperture. Should we force
> > the dma_set_mask to not change if an existing mask is already set?
>
> Let me clarify the question some more, in our system the IOMMU supports only
> 42 bits of address. With your _DMA aperture patch, the initial dma_mask and
> coherent_mask are correctly set by the code. However, the device driver can
> set the dma_mask and coherent mask at a later point which over writes the
> initial setting by your code. In which case, once the iova is exhausted with the
> 32 bit address, it will start seeking more iova address via the
> dma_limit. In this
> case it would fail my system since the iommu.aperture_end is that of 48 bits
> as derived from ias field in the SMMU.
>
> Should the dma_limit be the smallest of driver->dma_mask, iommu.aperture_end and
> your _DMA aperture size via ACPI table? Rather than just the
> driver->dma_mask and
> iommu.aperture_end. This will ensure the smallest/correct aperture is used.

IIUC Nate already reported this issue - I will sync with Robin to
check the status of this thread:

https://marc.info/?l=linux-acpi&m=150108156230455&w=2

Thanks,
Lorenzo

2017-12-01 00:43:27

by Feng Kan

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ACPI: DMA ranges management

On Wed, Nov 29, 2017 at 11:28 PM, Feng Kan <[email protected]> wrote:
> On Thu, Aug 3, 2017 at 5:32 AM, Lorenzo Pieralisi
> <[email protected]> wrote:
>> This patch series is v3 of a previous posting:
>>
>> v2->v3:
>> - Fixed DMA masks computation
>> - Fixed size computation overflow in acpi_dma_get_range()
>>
>> v1->v2:
>> - Reworked acpi_dma_get_range() flow and logs
>> - Added IORT named component address limits
>> - Renamed acpi_dev_get_resources() helper function
>> - Rebased against v4.13-rc3
>>
>> v2: http://lkml.kernel.org/r/[email protected]
>> v1: http://lkml.kernel.org/r/[email protected]
>>
>> -- Original cover letter --
>>
>> As reported in:
>>
>> http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com
>>
>> the bus connecting devices to an IOMMU bus can be smaller in size than
>> the IOMMU input address bits which results in devices DMA HW bugs in
>> particular related to IOVA allocation (ie chopping of higher address
>> bits owing to system bus HW capabilities mismatch with the IOMMU).
>>
>> Fortunately this problem can be solved through an already present but never
>> used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define the DMA
>> window for a specific bus in ACPI and therefore all upstream devices
>> connected to it.
>>
>> This small patch series enables _DMA parsing in ACPI core code and
>> use it in ACPI IORT code in order to detect DMA ranges for devices and
>> update their data structures to make them work with their related DMA
>> addressing restrictions.
>>
>> Cc: Will Deacon <[email protected]>
>> Cc: Hanjun Guo <[email protected]>
>> Cc: Feng Kan <[email protected]>
>> Cc: Jon Masters <[email protected]>
>> Cc: Robert Moore <[email protected]>
>> Cc: Robin Murphy <[email protected]>
>> Cc: Zhang Rui <[email protected]>
>> Cc: "Rafael J. Wysocki" <[email protected]>
>>
>> Lorenzo Pieralisi (5):
>> ACPICA: resource_mgr: Allow _DMA method in walk resources
>> ACPI: Make acpi_dev_get_resources() method agnostic
>> ACPI: Introduce DMA ranges parsing
>> ACPI: Make acpi_dma_configure() DMA regions aware
>> ACPI/IORT: Add IORT named component memory address limits
>>
>> drivers/acpi/acpica/rsxface.c | 7 ++--
>> drivers/acpi/arm64/iort.c | 57 ++++++++++++++++++++++++++-
>> drivers/acpi/resource.c | 82 +++++++++++++++++++++++++++++---------
>> drivers/acpi/scan.c | 91 +++++++++++++++++++++++++++++++++++++++----
>> include/acpi/acnames.h | 1 +
>> include/acpi/acpi_bus.h | 2 +
>> include/linux/acpi.h | 8 ++++
>> include/linux/acpi_iort.h | 5 ++-
>> 8 files changed, 219 insertions(+), 34 deletions(-)
>>
>> --
>> 2.10.0
>>
> Lorenzo:
>
> A network driver can use pci_set_dma_mask or its like to override what
> is done with this patch here.
> Which would result in iova allocation greater than the original _DMA
> aperture. Should we force
> the dma_set_mask to not change if an existing mask is already set?

Let me clarify the question some more, in our system the IOMMU supports only
42 bits of address. With your _DMA aperture patch, the initial dma_mask and
coherent_mask are correctly set by the code. However, the device driver can
set the dma_mask and coherent mask at a later point which over writes the
initial setting by your code. In which case, once the iova is exhausted with the
32 bit address, it will start seeking more iova address via the
dma_limit. In this
case it would fail my system since the iommu.aperture_end is that of 48 bits
as derived from ias field in the SMMU.

Should the dma_limit be the smallest of driver->dma_mask, iommu.aperture_end and
your _DMA aperture size via ACPI table? Rather than just the
driver->dma_mask and
iommu.aperture_end. This will ensure the smallest/correct aperture is used.

From 1585475179879896616@xxx Thu Nov 30 07:29:18 +0000 2017
X-GM-THRID: 1574713251984889929
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-30 07:29:18

by Feng Kan

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ACPI: DMA ranges management

On Thu, Aug 3, 2017 at 5:32 AM, Lorenzo Pieralisi
<[email protected]> wrote:
> This patch series is v3 of a previous posting:
>
> v2->v3:
> - Fixed DMA masks computation
> - Fixed size computation overflow in acpi_dma_get_range()
>
> v1->v2:
> - Reworked acpi_dma_get_range() flow and logs
> - Added IORT named component address limits
> - Renamed acpi_dev_get_resources() helper function
> - Rebased against v4.13-rc3
>
> v2: http://lkml.kernel.org/r/[email protected]
> v1: http://lkml.kernel.org/r/[email protected]
>
> -- Original cover letter --
>
> As reported in:
>
> http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com
>
> the bus connecting devices to an IOMMU bus can be smaller in size than
> the IOMMU input address bits which results in devices DMA HW bugs in
> particular related to IOVA allocation (ie chopping of higher address
> bits owing to system bus HW capabilities mismatch with the IOMMU).
>
> Fortunately this problem can be solved through an already present but never
> used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define the DMA
> window for a specific bus in ACPI and therefore all upstream devices
> connected to it.
>
> This small patch series enables _DMA parsing in ACPI core code and
> use it in ACPI IORT code in order to detect DMA ranges for devices and
> update their data structures to make them work with their related DMA
> addressing restrictions.
>
> Cc: Will Deacon <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Feng Kan <[email protected]>
> Cc: Jon Masters <[email protected]>
> Cc: Robert Moore <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Zhang Rui <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
>
> Lorenzo Pieralisi (5):
> ACPICA: resource_mgr: Allow _DMA method in walk resources
> ACPI: Make acpi_dev_get_resources() method agnostic
> ACPI: Introduce DMA ranges parsing
> ACPI: Make acpi_dma_configure() DMA regions aware
> ACPI/IORT: Add IORT named component memory address limits
>
> drivers/acpi/acpica/rsxface.c | 7 ++--
> drivers/acpi/arm64/iort.c | 57 ++++++++++++++++++++++++++-
> drivers/acpi/resource.c | 82 +++++++++++++++++++++++++++++---------
> drivers/acpi/scan.c | 91 +++++++++++++++++++++++++++++++++++++++----
> include/acpi/acnames.h | 1 +
> include/acpi/acpi_bus.h | 2 +
> include/linux/acpi.h | 8 ++++
> include/linux/acpi_iort.h | 5 ++-
> 8 files changed, 219 insertions(+), 34 deletions(-)
>
> --
> 2.10.0
>
Lorenzo:

A network driver can use pci_set_dma_mask or its like to override what
is done with this patch here.
Which would result in iova allocation greater than the original _DMA
aperture. Should we force
the dma_set_mask to not change if an existing mask is already set?

From 1575733072641928254@xxx Mon Aug 14 18:42:40 +0000 2017
X-GM-THRID: 1574713251984889929
X-Gmail-Labels: Inbox,Category Forums