lkml.org 
[lkml]   [2016]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v5 14/14] drivers: acpi: iort: introduce iort_iommu_configure
From
Date
Hi Nate,

On 2016/9/13 16:14, Nate Watterson wrote:
> On 2016-09-09 10:23, Lorenzo Pieralisi wrote:
>> DT based systems have a generic kernel API to configure IOMMUs
>> for devices (ie of_iommu_configure()).
>>
>> On ARM based ACPI systems, the of_iommu_configure() equivalent can
>> be implemented atop ACPI IORT kernel API, with the corresponding
>> functions to map device identifiers to IOMMUs and retrieve the
>> corresponding IOMMU operations necessary for DMA operations set-up.
>>
>> By relying on the iommu_fwspec generic kernel infrastructure,
>> implement the IORT based IOMMU configuration for ARM ACPI systems
>> and hook it up in the ACPI kernel layer that implements DMA
>> configuration for a device.
>>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Hanjun Guo <hanjun.guo@linaro.org>
>> Cc: Tomasz Nowicki <tn@semihalf.com>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> ---
>> drivers/acpi/arm64/iort.c | 96
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/acpi/scan.c | 7 +++-
>> include/linux/acpi_iort.h | 6 +++
>> 3 files changed, 108 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index 7c68eb4..55a4ae9 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -19,6 +19,7 @@
>> #define pr_fmt(fmt) "ACPI: IORT: " fmt
>>
>> #include <linux/acpi_iort.h>
>> +#include <linux/iommu-fwspec.h>
>> #include <linux/kernel.h>
>> #include <linux/list.h>
>> #include <linux/pci.h>
>> @@ -27,6 +28,8 @@
>>
>> #define IORT_TYPE_MASK(type) (1 << (type))
>> #define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP)
>> +#define IORT_IOMMU_TYPE ((1 << ACPI_IORT_NODE_SMMU) | \
>> + (1 << ACPI_IORT_NODE_SMMU_V3))
>>
>> struct iort_its_msi_chip {
>> struct list_head list;
>> @@ -467,6 +470,99 @@ struct irq_domain *iort_get_device_domain(struct
>> device *dev, u32 req_id)
>> return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
>> }
>>
>> +static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
>> +{
>> + u32 *rid = data;
>> +
>> + *rid = alias;
>> + return 0;
>> +}
>> +
>> +static int arm_smmu_iort_xlate(struct device *dev, u32 streamid,
>> + struct fwnode_handle *fwnode)
>> +{
>> + int ret = iommu_fwspec_init(dev, fwnode);
>> +
>> + if (!ret)
>> + ret = iommu_fwspec_add_ids(dev, &streamid, 1);
>> +
>> + return ret;
>> +}
>> +
>> +static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
>> + struct acpi_iort_node *node,
>> + u32 streamid)
>> +{
>> + struct fwnode_handle *iort_fwnode = NULL;
>> + int ret;
>> +
>> + if (node) {
>> + iort_fwnode = iort_get_fwnode(node);
>> + if (!iort_fwnode)
>> + return NULL;
>> +
>> + ret = arm_smmu_iort_xlate(dev, streamid,
>> + iort_fwnode);
>> + if (!ret)
>> + return fwspec_iommu_get_ops(iort_fwnode);
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +/**
>> + * iort_iommu_configure - Set-up IOMMU configuration for a device.
>> + *
>> + * @dev: device to configure
>> + *
>> + * Returns: iommu_ops pointer on configuration success
>> + * NULL on configuration failure
>> + */
>> +const struct iommu_ops *iort_iommu_configure(struct device *dev)
>> +{
>> + struct acpi_iort_node *node, *parent;
>> + const struct iommu_ops *ops = NULL;
>> + u32 streamid = 0;
>> +
>> + if (dev_is_pci(dev)) {
>> + struct pci_bus *bus = to_pci_dev(dev)->bus;
>> + u32 rid;
>> +
>> + pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
>> + &rid);
>> +
>> + node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
>> + iort_match_node_callback, &bus->dev);
>> + if (!node)
>> + return NULL;
>> +
>> + parent = iort_node_map_rid(node, rid, &streamid,
>> + IORT_IOMMU_TYPE);
>> +
>> + ops = iort_iommu_xlate(dev, parent, streamid);
>> +
>> + } else {
>> + int i = 0;
>> +
>> + node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
>> + iort_match_node_callback, dev);
>> + if (!node)
>> + return NULL;
>> +
>
> Nothing wrong with your code here, but wanted to warn you that there
> appears to be a bug in iort_match_node_callback() for NAMED_COMPONENTS.
>
> iort_match_node_callback() {
> acpi_status status = AE_NOT_FOUND;
> ...
> case ACPI_IORT_NODE_NAMED_COMPONENT: {
> ...
> status = acpi_get_name(adev->handle, ACPI_FULL_PATHNAME, &buf);
> if (ACPI_FAILURE(status)) {
> dev_warn(dev, "Can't get device full path name\n");
> break;
> }
>
> ncomp = (struct acpi_iort_named_component *)node->node_data;
> if (!strcmp(ncomp->device_name, buf.pointer))
> status = AE_OK;
>
> acpi_os_free(buf.pointer);
> break;
> }
> ...
> return status;
> }
>
> Notice how if strcmp() fails, status remains set to the status of the call
> to acpi_get_name() which must have been OK since we would have broken out
> of the switch statement otherwise. This is causing all manner of platform
> devices not even described in the IORT to get hooked up using the IDs of
> the first properly iommu-attached NAMED_COMPONENT device found in the IORT.

As I said in previous email, please use the new version of IORT patch
set :)

Thanks
Hanjun

\
 
 \ /
  Last update: 2016-09-17 09:59    [W:1.267 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site