[RFC PATCH v2 0/4] IORT SMMUv3 MSI support

[RFC PATCH v2 0/4] IORT SMMUv3 MSI support

[Hanjun Guo]

From: Hanjun Guo <hanjun.guo@linaro.org>

IORT revision C introduced SMMUv3 MSI support for control interrupts,
which introduced a device ID mapping index to retrieve the dev ID
and ITS parent, adding its support in this patch set, please refer
to each patch for detail commit message.

RFC v1 -> RFC v2:
 - Introduce a new API iort_set_device_domain() to find the MSI domain
   for an SMMUv3 (or any other IORT table node) to reduce the complex
   of doing that via acpi_configure_pmsi_domain().

Hanjun Guo (3):
  ACPICA: Add SMMUv3 device ID mapping index support
  ACPI: IORT: lookup iort node via fwnode
  ACPI: IORT: Skip SMMUv3 device ID map for two steps mappings

Lorenzo Pieralisi (1):
  ACPI: IORT: SMMUv3 nodes MSI support

 drivers/acpi/arm64/iort.c | 157 ++++++++++++++++++++++++++++++++++++++++++++--
 include/acpi/actbl2.h     |   1 +
 2 files changed, 152 insertions(+), 6 deletions(-)

-- 
1.7.12.4

[RFC PATCH v2 1/4] ACPICA: Add SMMUv3 device ID mapping index support

[Hanjun Guo]

From: Hanjun Guo <hanjun.guo@linaro.org>

SMMUv3 device ID mapping index is used for SMMUv3
MSIs, update the IORT to support that.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 include/acpi/actbl2.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 686b6f8..d90277e 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -810,6 +810,7 @@ struct acpi_iort_smmu_v3 {
 	u8 pxm;
 	u8 reserved1;
 	u16 reserved2;
+	u32 id_mapping_index;
 };
 
 /* Values for Model field above */
-- 
1.7.12.4

[RFC PATCH v2 1/4] ACPICA: Add SMMUv3 device ID mapping index support

[Lorenzo Pieralisi]

On Thu, Sep 21, 2017 at 09:17:15PM +0800, Hanjun Guo wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>
> 
> SMMUv3 device ID mapping index is used for SMMUv3
> MSIs, update the IORT to support that.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
>  include/acpi/actbl2.h | 1 +
>  1 file changed, 1 insertion(+)

Have you sent the related git pull ACPICA upstream ?

Lorenzo

> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index 686b6f8..d90277e 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -810,6 +810,7 @@ struct acpi_iort_smmu_v3 {
>  	u8 pxm;
>  	u8 reserved1;
>  	u16 reserved2;
> +	u32 id_mapping_index;
>  };
>  
>  /* Values for Model field above */
> -- 
> 1.7.12.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v2 2/4] ACPI: IORT: lookup iort node via fwnode

[Hanjun Guo]

From: Hanjun Guo <hanjun.guo@linaro.org>

Now we have a helper function iort_get_fwnode() which
lookup fwnode via iort node for SMMU, but sometimes we
just need something exctly the opposite, which means we
need to get the iort node via fwnode.

For example, we need to get SMMU's iort node when adding
support for SMMU MSI, but SMMU is not a named component
which has a associated device node in DSDT, that means
we can't match the ACPI full path name to get the iort
node for SMMU.

But with SMMU or other devices in IORT probed as platform
device, it created a fwnode to associate with the iort
node, so we introduce iort_get_iort_node() to get the
iort node via fwnode.

This can be extended to PMCG node usage in IORT too.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 drivers/acpi/arm64/iort.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 9565d57..db71d7f 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -126,6 +126,31 @@ static inline void iort_delete_fwnode(struct acpi_iort_node *node)
 	spin_unlock(&iort_fwnode_lock);
 }
 
+/**
+ * iort_get_iort_node() - Retrieve iort_node associated with an fwnode
+ *
+ * @fwnode: fwnode associated with device to be looked-up
+ *
+ * Returns: iort_node pointer on success, NULL on failure
+ */
+static inline
+struct acpi_iort_node *iort_get_iort_node(struct fwnode_handle *fwnode)
+{
+	struct iort_fwnode *curr;
+	struct acpi_iort_node *iort_node = NULL;
+
+	spin_lock(&iort_fwnode_lock);
+	list_for_each_entry(curr, &iort_fwnode_list, list) {
+		if (curr->fwnode == fwnode) {
+			iort_node = curr->iort_node;
+			break;
+		}
+	}
+	spin_unlock(&iort_fwnode_lock);
+
+	return iort_node;
+}
+
 typedef acpi_status (*iort_find_node_callback)
 	(struct acpi_iort_node *node, void *context);
 
@@ -424,9 +449,25 @@ static struct acpi_iort_node *iort_find_dev_node(struct device *dev)
 {
 	struct pci_bus *pbus;
 
-	if (!dev_is_pci(dev))
+	if (!dev_is_pci(dev)) {
+		struct acpi_iort_node *node;
+		/*
+		 * scan iort_fwnode_list to see if it's an iort platform
+		 * device (such as SMMU, PMCG),its iort node already cached
+		 * and associated with fwnode when iort platform devices
+		 * were initialized.
+		 */
+		node = iort_get_iort_node(dev->fwnode);
+		if (node)
+			return node;
+
+		/*
+		 * if not, then it should be a platform device defined in
+		 * DSDT/SSDT (with Named Component node in IORT)
+		 */
 		return iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
 				      iort_match_node_callback, dev);
+	}
 
 	/* Find a PCI root bus */
 	pbus = to_pci_dev(dev)->bus;
-- 
1.7.12.4

[RFC PATCH v2 3/4] ACPI: IORT: Skip SMMUv3 device ID map for two steps mappings

[Hanjun Guo]

From: Hanjun Guo <hanjun.guo@linaro.org>

IORT revision C introduced SMMUv3 MSI support which adding a
device ID mapping index in SMMUv3 sub table, to get the SMMUv3
device ID mapping for the output ID (dev ID for ITS) and the
link to which ITS.

So if a platform supports SMMUv3 MSI for control interrupt,
there will be a additional single map entry under SMMU, this
will not introduce any difference for devices just use one
step map to get its output ID and parent (ITS or SMMU), such
as PCI/NC/PMCG ---> ITS or PCI/NC ---> SMMU, but we need to
do the special handling for two steps map case such as
PCI/NC--->SMMUv3--->ITS.

Take a PCI hostbridge for example,

|----------------------|
|  Root Complex Node   |
|----------------------|
|    map entry[x]      |
|----------------------|
|       id value       |
| output_reference     |
|---|------------------|
    |
    |   |----------------------|
    |-->|        SMMUv3        |
        |----------------------|
        |     SMMU dev ID      |
        |     mapping index 0  |
        |----------------------|
        |      map entry[0]    |
        |----------------------|
        |       id value       |
        | output_reference-----------> ITS 1 (SMMU MSI domain)
        |----------------------|
        |      map entry[1]    |
        |----------------------|
        |       id value       |
        | output_reference-----------> ITS 2 (PCI MSI domain)
        |----------------------|

When the SMMU dev ID mapping index is 0, there is entry[0]
to map to a ITS, we need to skip that map entry for PCI
or NC (named component), or we may get the wrong ITS parent.

For now we have two APIs for ID mapping, iort_node_map_id()
and iort_node_map_platform_id(), and iort_node_map_id() is
used for optional two steps mapping, so we just need to
skip the map entry in iort_node_map_id() for non-SMMUv3
devices.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 drivers/acpi/arm64/iort.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index db71d7f..269959e 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -366,6 +366,34 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
 	return NULL;
 }
 
+static int iort_get_smmu_v3_id_mapping_index(struct acpi_iort_node *node,
+					     u32 *index)
+{
+	struct acpi_iort_smmu_v3 *smmu;
+
+	/*
+	 * SMMUv3 dev ID mapping index was introdueced in revision 1
+	 * table, not avaible in revision 0
+	 */
+	if (node->revision < 1)
+		return -EINVAL;
+
+	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+	/* if any of the gsi for control interrupts is not 0, ignore the MSI */
+	if (smmu->event_gsiv || smmu->pri_gsiv || smmu->gerr_gsiv
+	    || smmu->sync_gsiv)
+		return -EINVAL;
+
+	if (smmu->id_mapping_index >= node->mapping_count) {
+		pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
+		       node, node->type);
+		return -EINVAL;
+	}
+
+	*index = smmu->id_mapping_index;
+	return 0;
+}
+
 static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
 					       u32 id_in, u32 *id_out,
 					       u8 type_mask)
@@ -375,7 +403,9 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
 	/* Parse the ID mapping tree to find specified node type */
 	while (node) {
 		struct acpi_iort_id_mapping *map;
-		int i;
+		int i, ret = -EINVAL;
+		/* big enough for an invalid id index in practical */
+		u32 index = U32_MAX;
 
 		if (IORT_TYPE_MASK(node->type) & type_mask) {
 			if (id_out)
@@ -396,8 +426,19 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
 			goto fail_map;
 		}
 
+		/*
+		 *  we need to get SMMUv3 dev ID mapping index and skip its
+		 *  associated ID map for single mapping cases.
+		 */
+		if (node->type == ACPI_IORT_NODE_SMMU_V3)
+			ret = iort_get_smmu_v3_id_mapping_index(node, &index);
+
 		/* Do the ID translation */
 		for (i = 0; i < node->mapping_count; i++, map++) {
+			/* if it's a SMMUv3 device id mapping index, skip it */
+			if (!ret && i == index)
+				continue;
+
 			if (!iort_id_map(map, node->type, id, &id))
 				break;
 		}
-- 
1.7.12.4

[RFC PATCH v2 3/4] ACPI: IORT: Skip SMMUv3 device ID map for two steps mappings

[Lorenzo Pieralisi]

On Thu, Sep 21, 2017 at 09:17:17PM +0800, Hanjun Guo wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>
> 
> IORT revision C introduced SMMUv3 MSI support which adding a
> device ID mapping index in SMMUv3 sub table, to get the SMMUv3
> device ID mapping for the output ID (dev ID for ITS) and the
> link to which ITS.
> 
> So if a platform supports SMMUv3 MSI for control interrupt,
> there will be a additional single map entry under SMMU, this
> will not introduce any difference for devices just use one
> step map to get its output ID and parent (ITS or SMMU), such
> as PCI/NC/PMCG ---> ITS or PCI/NC ---> SMMU, but we need to
> do the special handling for two steps map case such as
> PCI/NC--->SMMUv3--->ITS.
> 
> Take a PCI hostbridge for example,
> 
> |----------------------|
> |  Root Complex Node   |
> |----------------------|
> |    map entry[x]      |
> |----------------------|
> |       id value       |
> | output_reference     |
> |---|------------------|
>     |
>     |   |----------------------|
>     |-->|        SMMUv3        |
>         |----------------------|
>         |     SMMU dev ID      |
>         |     mapping index 0  |
>         |----------------------|
>         |      map entry[0]    |
>         |----------------------|
>         |       id value       |
>         | output_reference-----------> ITS 1 (SMMU MSI domain)
>         |----------------------|
>         |      map entry[1]    |
>         |----------------------|
>         |       id value       |
>         | output_reference-----------> ITS 2 (PCI MSI domain)
>         |----------------------|
> 
> When the SMMU dev ID mapping index is 0, there is entry[0]
> to map to a ITS, we need to skip that map entry for PCI
> or NC (named component), or we may get the wrong ITS parent.

We do skip it because it is a single mapping that it is currently
not allowed for SMMUv3 components, right ?

Ok, we barf with a printk log message if we encounter such mapping
but the mapping won't resolve to the SMMUv3 MSI in the current
kernel.

Anyway, I think patch 3 and patch 4 should be partially squashed,
more about that in patch 4 comments.

Lorenzo

> For now we have two APIs for ID mapping, iort_node_map_id()
> and iort_node_map_platform_id(), and iort_node_map_id() is
> used for optional two steps mapping, so we just need to
> skip the map entry in iort_node_map_id() for non-SMMUv3
> devices.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
>  drivers/acpi/arm64/iort.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index db71d7f..269959e 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -366,6 +366,34 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>  	return NULL;
>  }
>  
> +static int iort_get_smmu_v3_id_mapping_index(struct acpi_iort_node *node,
> +					     u32 *index)
> +{
> +	struct acpi_iort_smmu_v3 *smmu;
> +
> +	/*
> +	 * SMMUv3 dev ID mapping index was introdueced in revision 1
> +	 * table, not avaible in revision 0
> +	 */
> +	if (node->revision < 1)
> +		return -EINVAL;
> +
> +	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> +	/* if any of the gsi for control interrupts is not 0, ignore the MSI */
> +	if (smmu->event_gsiv || smmu->pri_gsiv || smmu->gerr_gsiv
> +	    || smmu->sync_gsiv)
> +		return -EINVAL;
> +
> +	if (smmu->id_mapping_index >= node->mapping_count) {
> +		pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
> +		       node, node->type);
> +		return -EINVAL;
> +	}
> +
> +	*index = smmu->id_mapping_index;
> +	return 0;
> +}
> +
>  static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>  					       u32 id_in, u32 *id_out,
>  					       u8 type_mask)
> @@ -375,7 +403,9 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>  	/* Parse the ID mapping tree to find specified node type */
>  	while (node) {
>  		struct acpi_iort_id_mapping *map;
> -		int i;
> +		int i, ret = -EINVAL;
> +		/* big enough for an invalid id index in practical */
> +		u32 index = U32_MAX;
>  
>  		if (IORT_TYPE_MASK(node->type) & type_mask) {
>  			if (id_out)
> @@ -396,8 +426,19 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>  			goto fail_map;
>  		}
>  
> +		/*
> +		 *  we need to get SMMUv3 dev ID mapping index and skip its
> +		 *  associated ID map for single mapping cases.
> +		 */
> +		if (node->type == ACPI_IORT_NODE_SMMU_V3)
> +			ret = iort_get_smmu_v3_id_mapping_index(node, &index);
> +
>  		/* Do the ID translation */
>  		for (i = 0; i < node->mapping_count; i++, map++) {
> +			/* if it's a SMMUv3 device id mapping index, skip it */
> +			if (!ret && i == index)
> +				continue;
> +
>  			if (!iort_id_map(map, node->type, id, &id))
>  				break;
>  		}
> -- 
> 1.7.12.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v2 3/4] ACPI: IORT: Skip SMMUv3 device ID map for two steps mappings

[Hanjun Guo]

On 09/22/2017 08:53 PM, Lorenzo Pieralisi wrote:
> On Thu, Sep 21, 2017 at 09:17:17PM +0800, Hanjun Guo wrote:
>> From: Hanjun Guo <hanjun.guo@linaro.org>
>>
>> IORT revision C introduced SMMUv3 MSI support which adding a
>> device ID mapping index in SMMUv3 sub table, to get the SMMUv3
>> device ID mapping for the output ID (dev ID for ITS) and the
>> link to which ITS.
>>
>> So if a platform supports SMMUv3 MSI for control interrupt,
>> there will be a additional single map entry under SMMU, this
>> will not introduce any difference for devices just use one
>> step map to get its output ID and parent (ITS or SMMU), such
>> as PCI/NC/PMCG ---> ITS or PCI/NC ---> SMMU, but we need to
>> do the special handling for two steps map case such as
>> PCI/NC--->SMMUv3--->ITS.
>>
>> Take a PCI hostbridge for example,
>>
>> |----------------------|
>> |  Root Complex Node   |
>> |----------------------|
>> |    map entry[x]      |
>> |----------------------|
>> |       id value       |
>> | output_reference     |
>> |---|------------------|
>>      |
>>      |   |----------------------|
>>      |-->|        SMMUv3        |
>>          |----------------------|
>>          |     SMMU dev ID      |
>>          |     mapping index 0  |
>>          |----------------------|
>>          |      map entry[0]    |
>>          |----------------------|
>>          |       id value       |
>>          | output_reference-----------> ITS 1 (SMMU MSI domain)
>>          |----------------------|
>>          |      map entry[1]    |
>>          |----------------------|
>>          |       id value       |
>>          | output_reference-----------> ITS 2 (PCI MSI domain)
>>          |----------------------|
>>
>> When the SMMU dev ID mapping index is 0, there is entry[0]
>> to map to a ITS, we need to skip that map entry for PCI
>> or NC (named component), or we may get the wrong ITS parent.
>
> We do skip it because it is a single mapping that it is currently
> not allowed for SMMUv3 components, right ?
>
> Ok, we barf with a printk log message if we encounter such mapping
> but the mapping won't resolve to the SMMUv3 MSI in the current
> kernel.

This is not clear in the spec, maybe we also need to update the IORT
spec about it.

Thanks
Hanjun

[RFC PATCH v2 3/4] ACPI: IORT: Skip SMMUv3 device ID map for two steps mappings

[Robin Murphy]

On 21/09/17 14:17, Hanjun Guo wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>
> 
> IORT revision C introduced SMMUv3 MSI support which adding a
> device ID mapping index in SMMUv3 sub table, to get the SMMUv3
> device ID mapping for the output ID (dev ID for ITS) and the
> link to which ITS.
> 
> So if a platform supports SMMUv3 MSI for control interrupt,
> there will be a additional single map entry under SMMU, this
> will not introduce any difference for devices just use one
> step map to get its output ID and parent (ITS or SMMU), such
> as PCI/NC/PMCG ---> ITS or PCI/NC ---> SMMU, but we need to
> do the special handling for two steps map case such as
> PCI/NC--->SMMUv3--->ITS.
> 
> Take a PCI hostbridge for example,
> 
> |----------------------|
> |  Root Complex Node   |
> |----------------------|
> |    map entry[x]      |
> |----------------------|
> |       id value       |
> | output_reference     |
> |---|------------------|
>     |
>     |   |----------------------|
>     |-->|        SMMUv3        |
>         |----------------------|
>         |     SMMU dev ID      |
>         |     mapping index 0  |
>         |----------------------|
>         |      map entry[0]    |
>         |----------------------|
>         |       id value       |
>         | output_reference-----------> ITS 1 (SMMU MSI domain)
>         |----------------------|
>         |      map entry[1]    |
>         |----------------------|
>         |       id value       |
>         | output_reference-----------> ITS 2 (PCI MSI domain)
>         |----------------------|
> 
> When the SMMU dev ID mapping index is 0, there is entry[0]
> to map to a ITS, we need to skip that map entry for PCI
> or NC (named component), or we may get the wrong ITS parent.
> 
> For now we have two APIs for ID mapping, iort_node_map_id()
> and iort_node_map_platform_id(), and iort_node_map_id() is
> used for optional two steps mapping, so we just need to
> skip the map entry in iort_node_map_id() for non-SMMUv3
> devices.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
>  drivers/acpi/arm64/iort.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index db71d7f..269959e 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -366,6 +366,34 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>  	return NULL;
>  }
>  
> +static int iort_get_smmu_v3_id_mapping_index(struct acpi_iort_node *node,
> +					     u32 *index)
> +{
> +	struct acpi_iort_smmu_v3 *smmu;
> +
> +	/*
> +	 * SMMUv3 dev ID mapping index was introdueced in revision 1
> +	 * table, not avaible in revision 0
> +	 */
> +	if (node->revision < 1)
> +		return -EINVAL;
> +
> +	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> +	/* if any of the gsi for control interrupts is not 0, ignore the MSI */
> +	if (smmu->event_gsiv || smmu->pri_gsiv || smmu->gerr_gsiv
> +	    || smmu->sync_gsiv)

To repeat my previous comments, the ID mapping index is only ignored if
*all* interrupts are GSIV-based.

It would be quite reasonable for gerr to be wired while everything else
uses MSIs, especially since that's effectively the only reliable way to
get MSI aborts reported if things are completely hosed.

> +		return -EINVAL;
> +
> +	if (smmu->id_mapping_index >= node->mapping_count) {
> +		pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
> +		       node, node->type);
> +		return -EINVAL;
> +	}
> +
> +	*index = smmu->id_mapping_index;
> +	return 0;
> +}
> +
>  static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>  					       u32 id_in, u32 *id_out,
>  					       u8 type_mask)
> @@ -375,7 +403,9 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>  	/* Parse the ID mapping tree to find specified node type */
>  	while (node) {
>  		struct acpi_iort_id_mapping *map;
> -		int i;
> +		int i, ret = -EINVAL;
> +		/* big enough for an invalid id index in practical */
> +		u32 index = U32_MAX;

And again, more of a style nit, but indices must be sufficiently small
to fit into the positive half of an int, so *_id_mapping_index() really
doesn't need this complication of passing a separate output argument.

Robin.

>  		if (IORT_TYPE_MASK(node->type) & type_mask) {
>  			if (id_out)
> @@ -396,8 +426,19 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>  			goto fail_map;
>  		}
>  
> +		/*
> +		 *  we need to get SMMUv3 dev ID mapping index and skip its
> +		 *  associated ID map for single mapping cases.
> +		 */
> +		if (node->type == ACPI_IORT_NODE_SMMU_V3)
> +			ret = iort_get_smmu_v3_id_mapping_index(node, &index);
> +
>  		/* Do the ID translation */
>  		for (i = 0; i < node->mapping_count; i++, map++) {
> +			/* if it's a SMMUv3 device id mapping index, skip it */
> +			if (!ret && i == index)
> +				continue;
> +
>  			if (!iort_id_map(map, node->type, id, &id))
>  				break;
>  		}
> 

[RFC PATCH v2 3/4] ACPI: IORT: Skip SMMUv3 device ID map for two steps mappings

[Hanjun Guo]

Hi Robin,

Sorry for the late reply, in Linaro Connect now..

On 09/22/2017 09:21 PM, Robin Murphy wrote:
> On 21/09/17 14:17, Hanjun Guo wrote:
>> From: Hanjun Guo <hanjun.guo@linaro.org>
>>
>> IORT revision C introduced SMMUv3 MSI support which adding a
>> device ID mapping index in SMMUv3 sub table, to get the SMMUv3
>> device ID mapping for the output ID (dev ID for ITS) and the
>> link to which ITS.
>>
>> So if a platform supports SMMUv3 MSI for control interrupt,
>> there will be a additional single map entry under SMMU, this
>> will not introduce any difference for devices just use one
>> step map to get its output ID and parent (ITS or SMMU), such
>> as PCI/NC/PMCG ---> ITS or PCI/NC ---> SMMU, but we need to
>> do the special handling for two steps map case such as
>> PCI/NC--->SMMUv3--->ITS.
>>
>> Take a PCI hostbridge for example,
>>
>> |----------------------|
>> |  Root Complex Node   |
>> |----------------------|
>> |    map entry[x]      |
>> |----------------------|
>> |       id value       |
>> | output_reference     |
>> |---|------------------|
>>      |
>>      |   |----------------------|
>>      |-->|        SMMUv3        |
>>          |----------------------|
>>          |     SMMU dev ID      |
>>          |     mapping index 0  |
>>          |----------------------|
>>          |      map entry[0]    |
>>          |----------------------|
>>          |       id value       |
>>          | output_reference-----------> ITS 1 (SMMU MSI domain)
>>          |----------------------|
>>          |      map entry[1]    |
>>          |----------------------|
>>          |       id value       |
>>          | output_reference-----------> ITS 2 (PCI MSI domain)
>>          |----------------------|
>>
>> When the SMMU dev ID mapping index is 0, there is entry[0]
>> to map to a ITS, we need to skip that map entry for PCI
>> or NC (named component), or we may get the wrong ITS parent.
>>
>> For now we have two APIs for ID mapping, iort_node_map_id()
>> and iort_node_map_platform_id(), and iort_node_map_id() is
>> used for optional two steps mapping, so we just need to
>> skip the map entry in iort_node_map_id() for non-SMMUv3
>> devices.
>>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> ---
>>   drivers/acpi/arm64/iort.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 42 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index db71d7f..269959e 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -366,6 +366,34 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>>   	return NULL;
>>   }
>>
>> +static int iort_get_smmu_v3_id_mapping_index(struct acpi_iort_node *node,
>> +					     u32 *index)
>> +{
>> +	struct acpi_iort_smmu_v3 *smmu;
>> +
>> +	/*
>> +	 * SMMUv3 dev ID mapping index was introdueced in revision 1
>> +	 * table, not avaible in revision 0
>> +	 */
>> +	if (node->revision < 1)
>> +		return -EINVAL;
>> +
>> +	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>> +	/* if any of the gsi for control interrupts is not 0, ignore the MSI */
>> +	if (smmu->event_gsiv || smmu->pri_gsiv || smmu->gerr_gsiv
>> +	    || smmu->sync_gsiv)
>
> To repeat my previous comments, the ID mapping index is only ignored if
> *all* interrupts are GSIV-based.
>
> It would be quite reasonable for gerr to be wired while everything else
> uses MSIs, especially since that's effectively the only reliable way to
> get MSI aborts reported if things are completely hosed.

OK, I missed this point, so I will modify the code as below

if (smmu->event_gsiv && smmu->pri_gsiv && smmu->gerr_gsiv
     && smmu->sync_gsiv)
	return -EINVAL;

Am I doing this right with above code?

>
>> +		return -EINVAL;
>> +
>> +	if (smmu->id_mapping_index >= node->mapping_count) {
>> +		pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
>> +		       node, node->type);
>> +		return -EINVAL;
>> +	}
>> +
>> +	*index = smmu->id_mapping_index;
>> +	return 0;
>> +}
>> +
>>   static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>>   					       u32 id_in, u32 *id_out,
>>   					       u8 type_mask)
>> @@ -375,7 +403,9 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>>   	/* Parse the ID mapping tree to find specified node type */
>>   	while (node) {
>>   		struct acpi_iort_id_mapping *map;
>> -		int i;
>> +		int i, ret = -EINVAL;
>> +		/* big enough for an invalid id index in practical */
>> +		u32 index = U32_MAX;
>
> And again, more of a style nit, but indices must be sufficiently small
> to fit into the positive half of an int, so *_id_mapping_index() really
> doesn't need this complication of passing a separate output argument.

Sorry, I reused the code which Lorenzo sent me, will update the code
in next version.

Thanks
Hanjun

[RFC PATCH v2 4/4] ACPI: IORT: SMMUv3 nodes MSI support

[Hanjun Guo]

From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Since we make single mappings valid for SMMUv3 (and PMCG), also
we have a mapping index for SMMUv3 MSI, we can directly use that
index to get the map entry, then retrieve dev ID and ITS parent
to add SMMUv3 MSI support.

Introduce a new API iort_set_device_domain() to find the MSI domain
for an SMMUv3 (or any other IORT table node) to reduce the complex
of doing that via acpi_configure_pmsi_domain(), then reuse the
iort_node_get_id() to get the dev id for SMMU MSI.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 drivers/acpi/arm64/iort.c | 99 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 81 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 269959e..bbab2ab 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -357,7 +357,8 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
 
 	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
 		if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
-		    node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
+		    node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX ||
+		    node->type == ACPI_IORT_NODE_SMMU_V3) {
 			*id_out = map->output_base;
 			return parent;
 		}
@@ -366,8 +367,8 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
 	return NULL;
 }
 
-static int iort_get_smmu_v3_id_mapping_index(struct acpi_iort_node *node,
-					     u32 *index)
+static int iort_get_id_mapping_index(struct acpi_iort_node *node,
+				     u32 *index)
 {
 	struct acpi_iort_smmu_v3 *smmu;
 
@@ -378,20 +379,28 @@ static int iort_get_smmu_v3_id_mapping_index(struct acpi_iort_node *node,
 	if (node->revision < 1)
 		return -EINVAL;
 
-	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
-	/* if any of the gsi for control interrupts is not 0, ignore the MSI */
-	if (smmu->event_gsiv || smmu->pri_gsiv || smmu->gerr_gsiv
-	    || smmu->sync_gsiv)
-		return -EINVAL;
+	switch (node->type) {
+	case ACPI_IORT_NODE_SMMU_V3:
+		smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+		/*
+		 * if any of the gsi for control interrupts is not 0,
+		 * ignore the MSI
+		 */
+		if (smmu->event_gsiv || smmu->pri_gsiv || smmu->gerr_gsiv
+		    || smmu->sync_gsiv)
+			return -EINVAL;
 
-	if (smmu->id_mapping_index >= node->mapping_count) {
-		pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
-		       node, node->type);
+		if (smmu->id_mapping_index >= node->mapping_count) {
+			pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
+			       node, node->type);
+			return -EINVAL;
+		}
+
+		*index = smmu->id_mapping_index;
+		return 0;
+	default:
 		return -EINVAL;
 	}
-
-	*index = smmu->id_mapping_index;
-	return 0;
 }
 
 static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
@@ -431,7 +440,7 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
 		 *  associated ID map for single mapping cases.
 		 */
 		if (node->type == ACPI_IORT_NODE_SMMU_V3)
-			ret = iort_get_smmu_v3_id_mapping_index(node, &index);
+			ret = iort_get_id_mapping_index(node, &index);
 
 		/* Do the ID translation */
 		for (i = 0; i < node->mapping_count; i++, map++) {
@@ -555,9 +564,18 @@ int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
 	if (!node)
 		return -ENODEV;
 
-	for (i = 0; i < node->mapping_count; i++) {
-		if (iort_node_map_platform_id(node, dev_id, IORT_MSI_TYPE, i))
-			return 0;
+	if (node->type == ACPI_IORT_NODE_SMMU_V3) {
+		u32 index;
+
+		if (!iort_get_id_mapping_index(node, &index)) {
+			if (iort_node_get_id(node, dev_id, index))
+				return 0;
+		}
+	} else {
+		for (i = 0; i < node->mapping_count; i++) {
+			if (iort_node_map_platform_id(node, dev_id, IORT_MSI_TYPE, i))
+				return 0;
+		}
 	}
 
 	return -ENODEV;
@@ -620,6 +638,49 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
 	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
 }
 
+static void iort_set_device_domain(struct device *dev,
+				   struct acpi_iort_node *node)
+{
+	struct acpi_iort_its_group *its;
+	struct acpi_iort_node *msi_parent;
+	struct acpi_iort_id_mapping *map;
+	struct fwnode_handle *iort_fwnode;
+	struct irq_domain *domain;
+	int ret, index;
+
+	ret = iort_get_id_mapping_index(node, &index);
+	if (ret < 0)
+		return;
+
+	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
+			   node->mapping_offset + index * sizeof(*map));
+
+	/* Firmware bug! */
+	if (!map->output_reference ||
+	    !(map->flags & ACPI_IORT_ID_SINGLE_MAPPING)) {
+		pr_err(FW_BUG "[node %p type %d] Invalid MSI mapping\n",
+		       node, node->type);
+		return;
+	}
+
+	msi_parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+				  map->output_reference);
+
+	if (!msi_parent || msi_parent->type != ACPI_IORT_NODE_ITS_GROUP)
+		return;
+
+	/* Move to ITS specific data */
+	its = (struct acpi_iort_its_group *)msi_parent->node_data;
+
+	iort_fwnode = iort_find_domain_token(its->identifiers[0]);
+	if (!iort_fwnode)
+		return;
+
+	domain = irq_find_matching_fwnode(iort_fwnode, DOMAIN_BUS_PLATFORM_MSI);
+	if (domain)
+		dev_set_msi_domain(dev, domain);
+}
+
 /**
  * iort_get_platform_device_domain() - Find MSI domain related to a
  * platform device
@@ -1246,6 +1307,8 @@ static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
 	/* Configure DMA for the page table walker */
 	acpi_dma_configure(&pdev->dev, attr);
 
+	iort_set_device_domain(&pdev->dev, node);
+
 	ret = platform_device_add(pdev);
 	if (ret)
 		goto dma_deconfigure;
-- 
1.7.12.4

[RFC PATCH v2 4/4] ACPI: IORT: SMMUv3 nodes MSI support

[Lorenzo Pieralisi]

On Thu, Sep 21, 2017 at 09:17:18PM +0800, Hanjun Guo wrote:
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> 
> Since we make single mappings valid for SMMUv3 (and PMCG), also
> we have a mapping index for SMMUv3 MSI, we can directly use that
> index to get the map entry, then retrieve dev ID and ITS parent
> to add SMMUv3 MSI support.
> 
> Introduce a new API iort_set_device_domain() to find the MSI domain
> for an SMMUv3 (or any other IORT table node) to reduce the complex
> of doing that via acpi_configure_pmsi_domain(), then reuse the
> iort_node_get_id() to get the dev id for SMMU MSI.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
>  drivers/acpi/arm64/iort.c | 99 ++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 81 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 269959e..bbab2ab 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -357,7 +357,8 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>  
>  	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
>  		if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
> -		    node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
> +		    node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX ||
> +		    node->type == ACPI_IORT_NODE_SMMU_V3) {
>  			*id_out = map->output_base;
>  			return parent;
>  		}
> @@ -366,8 +367,8 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>  	return NULL;
>  }
>  
> -static int iort_get_smmu_v3_id_mapping_index(struct acpi_iort_node *node,
> -					     u32 *index)
> +static int iort_get_id_mapping_index(struct acpi_iort_node *node,
> +				     u32 *index)
>  {
>  	struct acpi_iort_smmu_v3 *smmu;
>  
> @@ -378,20 +379,28 @@ static int iort_get_smmu_v3_id_mapping_index(struct acpi_iort_node *node,
>  	if (node->revision < 1)
>  		return -EINVAL;
>  
> -	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> -	/* if any of the gsi for control interrupts is not 0, ignore the MSI */
> -	if (smmu->event_gsiv || smmu->pri_gsiv || smmu->gerr_gsiv
> -	    || smmu->sync_gsiv)
> -		return -EINVAL;
> +	switch (node->type) {
> +	case ACPI_IORT_NODE_SMMU_V3:
> +		smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> +		/*
> +		 * if any of the gsi for control interrupts is not 0,
> +		 * ignore the MSI
> +		 */
> +		if (smmu->event_gsiv || smmu->pri_gsiv || smmu->gerr_gsiv
> +		    || smmu->sync_gsiv)
> +			return -EINVAL;
>  
> -	if (smmu->id_mapping_index >= node->mapping_count) {
> -		pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
> -		       node, node->type);
> +		if (smmu->id_mapping_index >= node->mapping_count) {
> +			pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
> +			       node, node->type);
> +			return -EINVAL;
> +		}
> +
> +		*index = smmu->id_mapping_index;
> +		return 0;
> +	default:
>  		return -EINVAL;
>  	}
> -
> -	*index = smmu->id_mapping_index;
> -	return 0;
>  }
>  
>  static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
> @@ -431,7 +440,7 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>  		 *  associated ID map for single mapping cases.
>  		 */
>  		if (node->type == ACPI_IORT_NODE_SMMU_V3)

I wanted to use:

iort_get_id_mapping_index()

so that the node type check is done *in* that function and you do
not need to check it here.

> -			ret = iort_get_smmu_v3_id_mapping_index(node, &index);
> +			ret = iort_get_id_mapping_index(node, &index);
>  
>  		/* Do the ID translation */
>  		for (i = 0; i < node->mapping_count; i++, map++) {
> @@ -555,9 +564,18 @@ int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
>  	if (!node)
>  		return -ENODEV;
>  
> -	for (i = 0; i < node->mapping_count; i++) {
> -		if (iort_node_map_platform_id(node, dev_id, IORT_MSI_TYPE, i))
> -			return 0;
> +	if (node->type == ACPI_IORT_NODE_SMMU_V3) {

Ditto.

> +		u32 index;
> +
> +		if (!iort_get_id_mapping_index(node, &index)) {
> +			if (iort_node_get_id(node, dev_id, index))
> +				return 0;
> +		}
> +	} else {
> +		for (i = 0; i < node->mapping_count; i++) {
> +			if (iort_node_map_platform_id(node, dev_id, IORT_MSI_TYPE, i))
> +				return 0;
> +		}
>  	}
>  
>  	return -ENODEV;
> @@ -620,6 +638,49 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
>  	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
>  }

All changes above can be squashed with patch 3.

> +static void iort_set_device_domain(struct device *dev,
> +				   struct acpi_iort_node *node)
> +{
> +	struct acpi_iort_its_group *its;
> +	struct acpi_iort_node *msi_parent;
> +	struct acpi_iort_id_mapping *map;
> +	struct fwnode_handle *iort_fwnode;
> +	struct irq_domain *domain;
> +	int ret, index;
> +
> +	ret = iort_get_id_mapping_index(node, &index);
> +	if (ret < 0)
> +		return;
> +
> +	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
> +			   node->mapping_offset + index * sizeof(*map));
> +
> +	/* Firmware bug! */
> +	if (!map->output_reference ||
> +	    !(map->flags & ACPI_IORT_ID_SINGLE_MAPPING)) {
> +		pr_err(FW_BUG "[node %p type %d] Invalid MSI mapping\n",
> +		       node, node->type);
> +		return;
> +	}
> +
> +	msi_parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> +				  map->output_reference);
> +
> +	if (!msi_parent || msi_parent->type != ACPI_IORT_NODE_ITS_GROUP)
> +		return;
> +
> +	/* Move to ITS specific data */
> +	its = (struct acpi_iort_its_group *)msi_parent->node_data;
> +
> +	iort_fwnode = iort_find_domain_token(its->identifiers[0]);
> +	if (!iort_fwnode)
> +		return;
> +
> +	domain = irq_find_matching_fwnode(iort_fwnode, DOMAIN_BUS_PLATFORM_MSI);
> +	if (domain)
> +		dev_set_msi_domain(dev, domain);
> +}
> +
>  /**
>   * iort_get_platform_device_domain() - Find MSI domain related to a
>   * platform device
> @@ -1246,6 +1307,8 @@ static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
>  	/* Configure DMA for the page table walker */
>  	acpi_dma_configure(&pdev->dev, attr);
>  
> +	iort_set_device_domain(&pdev->dev, node);
> +

..and then you introduce iort_set_device_domain() and use it in a
separate patch.

With these changes I think we are ready to queue the series.

Thanks,
Lorenzo

>  	ret = platform_device_add(pdev);
>  	if (ret)
>  		goto dma_deconfigure;
> -- 
> 1.7.12.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v2 4/4] ACPI: IORT: SMMUv3 nodes MSI support

[Hanjun Guo]

On 09/22/2017 09:07 PM, Lorenzo Pieralisi wrote:
> On Thu, Sep 21, 2017 at 09:17:18PM +0800, Hanjun Guo wrote:
>> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>
>> Since we make single mappings valid for SMMUv3 (and PMCG), also
>> we have a mapping index for SMMUv3 MSI, we can directly use that
>> index to get the map entry, then retrieve dev ID and ITS parent
>> to add SMMUv3 MSI support.
>>
>> Introduce a new API iort_set_device_domain() to find the MSI domain
>> for an SMMUv3 (or any other IORT table node) to reduce the complex
>> of doing that via acpi_configure_pmsi_domain(), then reuse the
>> iort_node_get_id() to get the dev id for SMMU MSI.
>>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> ---
>>   drivers/acpi/arm64/iort.c | 99 ++++++++++++++++++++++++++++++++++++++---------
>>   1 file changed, 81 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index 269959e..bbab2ab 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -357,7 +357,8 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>>
>>   	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
>>   		if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
>> -		    node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
>> +		    node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX ||
>> +		    node->type == ACPI_IORT_NODE_SMMU_V3) {
>>   			*id_out = map->output_base;
>>   			return parent;
>>   		}
>> @@ -366,8 +367,8 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>>   	return NULL;
>>   }
>>
>> -static int iort_get_smmu_v3_id_mapping_index(struct acpi_iort_node *node,
>> -					     u32 *index)
>> +static int iort_get_id_mapping_index(struct acpi_iort_node *node,
>> +				     u32 *index)
>>   {
>>   	struct acpi_iort_smmu_v3 *smmu;
>>
>> @@ -378,20 +379,28 @@ static int iort_get_smmu_v3_id_mapping_index(struct acpi_iort_node *node,
>>   	if (node->revision < 1)
>>   		return -EINVAL;
>>
>> -	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>> -	/* if any of the gsi for control interrupts is not 0, ignore the MSI */
>> -	if (smmu->event_gsiv || smmu->pri_gsiv || smmu->gerr_gsiv
>> -	    || smmu->sync_gsiv)
>> -		return -EINVAL;
>> +	switch (node->type) {
>> +	case ACPI_IORT_NODE_SMMU_V3:
>> +		smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>> +		/*
>> +		 * if any of the gsi for control interrupts is not 0,
>> +		 * ignore the MSI
>> +		 */
>> +		if (smmu->event_gsiv || smmu->pri_gsiv || smmu->gerr_gsiv
>> +		    || smmu->sync_gsiv)
>> +			return -EINVAL;
>>
>> -	if (smmu->id_mapping_index >= node->mapping_count) {
>> -		pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
>> -		       node, node->type);
>> +		if (smmu->id_mapping_index >= node->mapping_count) {
>> +			pr_err(FW_BUG "[node %p type %d] ID mapping index overflows valid mappings\n",
>> +			       node, node->type);
>> +			return -EINVAL;
>> +		}
>> +
>> +		*index = smmu->id_mapping_index;
>> +		return 0;
>> +	default:
>>   		return -EINVAL;
>>   	}
>> -
>> -	*index = smmu->id_mapping_index;
>> -	return 0;
>>   }
>>
>>   static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>> @@ -431,7 +440,7 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>>   		 *  associated ID map for single mapping cases.
>>   		 */
>>   		if (node->type == ACPI_IORT_NODE_SMMU_V3)
>
> I wanted to use:
>
> iort_get_id_mapping_index()
>
> so that the node type check is done *in* that function and you do
> not need to check it here.
>
>> -			ret = iort_get_smmu_v3_id_mapping_index(node, &index);
>> +			ret = iort_get_id_mapping_index(node, &index);
>>
>>   		/* Do the ID translation */
>>   		for (i = 0; i < node->mapping_count; i++, map++) {
>> @@ -555,9 +564,18 @@ int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
>>   	if (!node)
>>   		return -ENODEV;
>>
>> -	for (i = 0; i < node->mapping_count; i++) {
>> -		if (iort_node_map_platform_id(node, dev_id, IORT_MSI_TYPE, i))
>> -			return 0;
>> +	if (node->type == ACPI_IORT_NODE_SMMU_V3) {
>
> Ditto.
>
>> +		u32 index;
>> +
>> +		if (!iort_get_id_mapping_index(node, &index)) {
>> +			if (iort_node_get_id(node, dev_id, index))
>> +				return 0;
>> +		}
>> +	} else {
>> +		for (i = 0; i < node->mapping_count; i++) {
>> +			if (iort_node_map_platform_id(node, dev_id, IORT_MSI_TYPE, i))
>> +				return 0;
>> +		}
>>   	}
>>
>>   	return -ENODEV;
>> @@ -620,6 +638,49 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
>>   	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
>>   }
>
> All changes above can be squashed with patch 3.
>
>> +static void iort_set_device_domain(struct device *dev,
>> +				   struct acpi_iort_node *node)
>> +{
>> +	struct acpi_iort_its_group *its;
>> +	struct acpi_iort_node *msi_parent;
>> +	struct acpi_iort_id_mapping *map;
>> +	struct fwnode_handle *iort_fwnode;
>> +	struct irq_domain *domain;
>> +	int ret, index;
>> +
>> +	ret = iort_get_id_mapping_index(node, &index);
>> +	if (ret < 0)
>> +		return;
>> +
>> +	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
>> +			   node->mapping_offset + index * sizeof(*map));
>> +
>> +	/* Firmware bug! */
>> +	if (!map->output_reference ||
>> +	    !(map->flags & ACPI_IORT_ID_SINGLE_MAPPING)) {
>> +		pr_err(FW_BUG "[node %p type %d] Invalid MSI mapping\n",
>> +		       node, node->type);
>> +		return;
>> +	}
>> +
>> +	msi_parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>> +				  map->output_reference);
>> +
>> +	if (!msi_parent || msi_parent->type != ACPI_IORT_NODE_ITS_GROUP)
>> +		return;
>> +
>> +	/* Move to ITS specific data */
>> +	its = (struct acpi_iort_its_group *)msi_parent->node_data;
>> +
>> +	iort_fwnode = iort_find_domain_token(its->identifiers[0]);
>> +	if (!iort_fwnode)
>> +		return;
>> +
>> +	domain = irq_find_matching_fwnode(iort_fwnode, DOMAIN_BUS_PLATFORM_MSI);
>> +	if (domain)
>> +		dev_set_msi_domain(dev, domain);
>> +}
>> +
>>   /**
>>    * iort_get_platform_device_domain() - Find MSI domain related to a
>>    * platform device
>> @@ -1246,6 +1307,8 @@ static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
>>   	/* Configure DMA for the page table walker */
>>   	acpi_dma_configure(&pdev->dev, attr);
>>
>> +	iort_set_device_domain(&pdev->dev, node);
>> +
>
> ..and then you introduce iort_set_device_domain() and use it in a
> separate patch.
>
> With these changes I think we are ready to queue the series.

Updated already, will send a new version soon.

Thanks
Hanjun

[RFC PATCH v2 0/4] IORT SMMUv3 MSI support

[Lorenzo Pieralisi]

On Thu, Sep 21, 2017 at 09:17:14PM +0800, Hanjun Guo wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>
> 
> IORT revision C introduced SMMUv3 MSI support for control interrupts,
> which introduced a device ID mapping index to retrieve the dev ID
> and ITS parent, adding its support in this patch set, please refer
> to each patch for detail commit message.
> 
> RFC v1 -> RFC v2:
>  - Introduce a new API iort_set_device_domain() to find the MSI domain
>    for an SMMUv3 (or any other IORT table node) to reduce the complex
>    of doing that via acpi_configure_pmsi_domain().
> 
> Hanjun Guo (3):
>   ACPICA: Add SMMUv3 device ID mapping index support
>   ACPI: IORT: lookup iort node via fwnode
>   ACPI: IORT: Skip SMMUv3 device ID map for two steps mappings
> 
> Lorenzo Pieralisi (1):
>   ACPI: IORT: SMMUv3 nodes MSI support
> 
>  drivers/acpi/arm64/iort.c | 157 ++++++++++++++++++++++++++++++++++++++++++++--
>  include/acpi/actbl2.h     |   1 +
>  2 files changed, 152 insertions(+), 6 deletions(-)

Please drop RFC tag from this series, comments on respective patches.

Lorenzo