This patch series is v2 of a previous posting:
v1: http://lkml.kernel.org/r/[email protected]
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
-- 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 | 50 +++++++++++++++++++++++-
drivers/acpi/resource.c | 82 ++++++++++++++++++++++++++++++---------
drivers/acpi/scan.c | 90 +++++++++++++++++++++++++++++++++++++++----
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, 211 insertions(+), 34 deletions(-)
--
2.10.0
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
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 | 30 ++++++++++++++++++++++++++++--
drivers/acpi/scan.c | 12 ++++--------
include/linux/acpi_iort.h | 5 +++--
3 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index a3215ee..67b85ae 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;
+
/*
* Set default coherent_dma_mask to 32 bit. Drivers are expected to
* setup the correct supported mask.
@@ -700,6 +705,27 @@ 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) {
+ mask = __roundup_pow_of_two(dmaaddr + size) - 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/drivers/acpi/scan.c b/drivers/acpi/scan.c
index dcf5f22..ae3d580 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1445,20 +1445,16 @@ int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
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/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
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 | 40 ++++++++++++++++++++++++++++++----------
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 67b85ae..b85d19f 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 ? ~0ULL :
+ 1ULL<<ncomp->memory_address_limit;
+
+ return 0;
+}
+
/**
* iort_dma_setup() - Set-up device DMA parameters.
*
@@ -708,17 +726,19 @@ 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) {
- mask = __roundup_pow_of_two(dmaaddr + size) - 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) {
+ mask = __roundup_pow_of_two(dmaaddr + size) - 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
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 | 78 +++++++++++++++++++++++++++++++++++++++++++++++++
include/acpi/acpi_bus.h | 2 ++
include/linux/acpi.h | 8 +++++
4 files changed, 123 insertions(+)
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..dcf5f22 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1360,6 +1360,84 @@ 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 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;
+ *size = dma_end - dma_start + 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
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
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
On Mon, Jul 31, 2017 at 5:23 PM, Lorenzo Pieralisi
<[email protected]> wrote:
> This patch series is v2 of a previous posting:
>
> v1: http://lkml.kernel.org/r/[email protected]
>
> 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
>
> -- 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
Patches [1-3/5] are fine by me, but I need ACKs from the ARM side on
the last two ones.
Thanks,
Rafael
On Mon, Jul 31, 2017 at 8:23 AM, Lorenzo Pieralisi
<[email protected]> wrote:
> This patch series is v2 of a previous posting:
>
> v1: http://lkml.kernel.org/r/[email protected]
>
> 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
>
> -- 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 | 50 +++++++++++++++++++++++-
> drivers/acpi/resource.c | 82 ++++++++++++++++++++++++++++++---------
> drivers/acpi/scan.c | 90 +++++++++++++++++++++++++++++++++++++++----
> 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, 211 insertions(+), 34 deletions(-)
>
> --
> 2.10.0
>
Thanks, this was tested and works on the XGene system.
Tested-by: Feng Kan <[email protected]>
Hi Lorenzo,
On 2017/7/31 23:23, Lorenzo Pieralisi wrote:
> 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 | 40 ++++++++++++++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 67b85ae..b85d19f 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 ? ~0ULL :
> + 1ULL<<ncomp->memory_address_limit;
Just a question here, if the IORT table didn't configure this
value properly, will the device working properly? I'm asking this
because in the table of IORT of D05, this value is set to 0 so far
(SAS and network), but I can boot D05 OK with your patch set, not
sure if any further issues.
Thanks
Hanjun
On Tue, Aug 01, 2017 at 06:20:43PM +0800, Hanjun Guo wrote:
> Hi Lorenzo,
>
> On 2017/7/31 23:23, Lorenzo Pieralisi wrote:
> >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 | 40 ++++++++++++++++++++++++++++++----------
> > 1 file changed, 30 insertions(+), 10 deletions(-)
> >
> >diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> >index 67b85ae..b85d19f 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 ? ~0ULL :
> >+ 1ULL<<ncomp->memory_address_limit;
>
> Just a question here, if the IORT table didn't configure this
> value properly, will the device working properly? I'm asking this
> because in the table of IORT of D05, this value is set to 0 so far
> (SAS and network), but I can boot D05 OK with your patch set, not
> sure if any further issues.
Then you wonder why I wrote it as a separate patch. Why is that
value set to 0 (is that because that's the insane default ?) ?
It is a firmware bug and if things work ok with this patch applied
either this patch contains a bug or drivers override the DMA masks
to cancel out this patch effects.
Please fix the firmware.
Thanks,
Lorenzo
On 2017/8/1 19:21, Lorenzo Pieralisi wrote:
> On Tue, Aug 01, 2017 at 06:20:43PM +0800, Hanjun Guo wrote:
>> Hi Lorenzo,
>>
>> On 2017/7/31 23:23, Lorenzo Pieralisi wrote:
>>> 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 | 40 ++++++++++++++++++++++++++++++----------
>>> 1 file changed, 30 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>>> index 67b85ae..b85d19f 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 ? ~0ULL :
>>> + 1ULL<<ncomp->memory_address_limit;
>>
>> Just a question here, if the IORT table didn't configure this
>> value properly, will the device working properly? I'm asking this
>> because in the table of IORT of D05, this value is set to 0 so far
>> (SAS and network), but I can boot D05 OK with your patch set, not
>> sure if any further issues.
>
> Then you wonder why I wrote it as a separate patch. Why is that
> value set to 0 (is that because that's the insane default ?) ?
> It is a firmware bug and if things work ok with this patch applied
> either this patch contains a bug or drivers override the DMA masks
> to cancel out this patch effects.
Thanks for the reply, not a bug for this patch, I confirmed that
the driver override the DMA masks (both SAS and network set 64 bit
DMA mask).
>
> Please fix the firmware.
Sure.
Thanks
Hanjun
On Mon, Jul 31, 2017 at 08:57:19PM +0200, Rafael J. Wysocki wrote:
> On Mon, Jul 31, 2017 at 5:23 PM, Lorenzo Pieralisi
> <[email protected]> wrote:
> > This patch series is v2 of a previous posting:
> >
> > v1: http://lkml.kernel.org/r/[email protected]
> >
> > 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
> >
> > -- 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
>
> Patches [1-3/5] are fine by me, but I need ACKs from the ARM side on
> the last two ones.
Will, Robin, are you ok with this series ?
@Nate: I'd need your tested-by on v2 please (ie IORT named component
address limits handling) before we go ahead.
Thanks,
Lorenzo
Hi Lorenzo,
I ran a quick test and this seems to work for memory_address_limit < 64,
however memory_address_limit == 64 yields a mask of 0x0.
-Nate
On 7/31/2017 11:23 AM, Lorenzo Pieralisi wrote:
> 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 | 40 ++++++++++++++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 67b85ae..b85d19f 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 ? ~0ULL :
> + 1ULL<<ncomp->memory_address_limit;
> +
> + return 0;
> +}
> +
> /**
> * iort_dma_setup() - Set-up device DMA parameters.
> *
> @@ -708,17 +726,19 @@ 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) {
> - mask = __roundup_pow_of_two(dmaaddr + size) - 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) {
> + mask = __roundup_pow_of_two(dmaaddr + size) - 1;
> + /*
> + * Limit coherent and dma mask based on size
> + * retrieved from firmware.
> + */
> + dev->coherent_dma_mask = mask;
> + *dev->dma_mask = mask;
> }
>
> *dma_addr = dmaaddr;
>
--
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.
On Wed, Aug 02, 2017 at 01:31:03PM -0400, Nate Watterson wrote:
> Hi Lorenzo,
> I ran a quick test and this seems to work for memory_address_limit < 64,
> however memory_address_limit == 64 yields a mask of 0x0.
I will fix it - apologies. Thanks.
Lorenzo
> -Nate
>
> On 7/31/2017 11:23 AM, Lorenzo Pieralisi wrote:
> >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 | 40 ++++++++++++++++++++++++++++++----------
> > 1 file changed, 30 insertions(+), 10 deletions(-)
> >
> >diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> >index 67b85ae..b85d19f 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 ? ~0ULL :
> >+ 1ULL<<ncomp->memory_address_limit;
> >+
> >+ return 0;
> >+}
> >+
> > /**
> > * iort_dma_setup() - Set-up device DMA parameters.
> > *
> >@@ -708,17 +726,19 @@ 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) {
> >- mask = __roundup_pow_of_two(dmaaddr + size) - 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) {
> >+ mask = __roundup_pow_of_two(dmaaddr + size) - 1;
> >+ /*
> >+ * Limit coherent and dma mask based on size
> >+ * retrieved from firmware.
> >+ */
> >+ dev->coherent_dma_mask = mask;
> >+ *dev->dma_mask = mask;
> > }
> > *dma_addr = dmaaddr;
> >
>
> --
> 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.