[PATCH 0/4] IORT SMMUv3 MSI support

[PATCH 0/4] IORT SMMUv3 MSI support

[Hanjun Guo]

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 v2 -> v1:
 - Drop RFC tag;
 - return the index value directly from iort_get_id_mapping_index()
   then make the logic simple in iort_node_map_id();
 - To make sure ID mapping index is only ignored if all interrupts are
   GSIV based
 - Sqursh part of the patch 4 to patch 3

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 | 152 ++++++++++++++++++++++++++++++++++++++++++++--
 include/acpi/actbl2.h     |   1 +
 2 files changed, 147 insertions(+), 6 deletions(-)

-- 
1.9.1

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

[Hanjun Guo]

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.9.1

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

[Hanjun Guo]

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.9.1

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

[Hanjun Guo]

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.

Introduce iort_get_id_mapping_index() to get the index then
skip the map entry in iort_node_map_id(), also to get the
dev ID directly for iort_pmsi_get_dev_id() for ITS platform
MSI preparation.

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

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index db71d7f..1c1160e 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,6 +367,40 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
 	return NULL;
 }
 
+static int iort_get_id_mapping_index(struct acpi_iort_node *node)
+{
+	struct acpi_iort_smmu_v3 *smmu;
+
+	switch (node->type) {
+	case ACPI_IORT_NODE_SMMU_V3:
+		/*
+		 * SMMUv3 dev ID mapping index was introdueced in revision 1
+		 * table, not available in revision 0
+		 */
+		if (node->revision < 1)
+			return -EINVAL;
+
+		smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+		/*
+		 * ID mapping index is only ignored if all interrupts are
+		 * GSIV based
+		 */
+		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;
+		}
+
+		return smmu->id_mapping_index;
+	default:
+		return -EINVAL;
+	}
+}
+
 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 +410,7 @@ 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, index;
 
 		if (IORT_TYPE_MASK(node->type) & type_mask) {
 			if (id_out)
@@ -396,8 +431,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, error value
+		 * returned for index will be an invalid value in practical.
+		 */
+		index = iort_get_id_mapping_index(node);
+
 		/* 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 (i == index)
+				continue;
+
 			if (!iort_id_map(map, node->type, id, &id))
 				break;
 		}
@@ -507,16 +553,24 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
  */
 int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
 {
-	int i;
+	int i, index;
 	struct acpi_iort_node *node;
 
 	node = iort_find_dev_node(dev);
 	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))
+	index = iort_get_id_mapping_index(node);
+	/* if there is a valid index, go get the dev_id directly */
+	if (index >= 0) {
+		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;
-- 
1.9.1

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

[Lorenzo Pieralisi]

Hi Hanjun,

On Wed, Sep 27, 2017 at 09:20:14AM +0800, Hanjun Guo wrote:
> 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.

Is this actually true ? I think that currently we would simply skip
the entry and print an error log but we can't get a wrong ITS parent.

I am rewriting this commit (I will probably split it), it is doing the
right thing but the commit log is stale (probably caused by code
reshuffling).

Thanks,
Lorenzo

> Introduce iort_get_id_mapping_index() to get the index then
> skip the map entry in iort_node_map_id(), also to get the
> dev ID directly for iort_pmsi_get_dev_id() for ITS platform
> MSI preparation.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
>  drivers/acpi/arm64/iort.c | 64 +++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index db71d7f..1c1160e 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,6 +367,40 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>  	return NULL;
>  }
>  
> +static int iort_get_id_mapping_index(struct acpi_iort_node *node)
> +{
> +	struct acpi_iort_smmu_v3 *smmu;
> +
> +	switch (node->type) {
> +	case ACPI_IORT_NODE_SMMU_V3:
> +		/*
> +		 * SMMUv3 dev ID mapping index was introdueced in revision 1
> +		 * table, not available in revision 0
> +		 */
> +		if (node->revision < 1)
> +			return -EINVAL;
> +
> +		smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> +		/*
> +		 * ID mapping index is only ignored if all interrupts are
> +		 * GSIV based
> +		 */
> +		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;
> +		}
> +
> +		return smmu->id_mapping_index;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  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 +410,7 @@ 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, index;
>  
>  		if (IORT_TYPE_MASK(node->type) & type_mask) {
>  			if (id_out)
> @@ -396,8 +431,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, error value
> +		 * returned for index will be an invalid value in practical.
> +		 */
> +		index = iort_get_id_mapping_index(node);
> +
>  		/* 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 (i == index)
> +				continue;
> +
>  			if (!iort_id_map(map, node->type, id, &id))
>  				break;
>  		}
> @@ -507,16 +553,24 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
>   */
>  int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
>  {
> -	int i;
> +	int i, index;
>  	struct acpi_iort_node *node;
>  
>  	node = iort_find_dev_node(dev);
>  	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))
> +	index = iort_get_id_mapping_index(node);
> +	/* if there is a valid index, go get the dev_id directly */
> +	if (index >= 0) {
> +		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;
> -- 
> 1.9.1
> 

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

[Hanjun Guo]

Hi Lorenzo,

Sorry for the late reply, holidays in China for the past week.

At 2017/9/27 21:54, Lorenzo Pieralisi wrote:
> Hi Hanjun,
>
> On Wed, Sep 27, 2017 at 09:20:14AM +0800, Hanjun Guo wrote:
>> 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.
>
> Is this actually true ? I think that currently we would simply skip
> the entry and print an error log but we can't get a wrong ITS parent.

So the only valid single mapping under type SMMUv3 is SMMUv3's dev id
mapping, we need to fix the IORT spec as well.

>
> I am rewriting this commit (I will probably split it), it is doing the
> right thing but the commit log is stale (probably caused by code
> reshuffling).

Do I need to resend another version, or you can help to update it?
please let me know.

By the way, this patch set was tested on Hisilicon Hip08 with MSI
for SMMU, and it works fine.

Thanks
Hanjun

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

[Lorenzo Pieralisi]

On Tue, Oct 10, 2017 at 02:47:53PM +0800, Hanjun Guo wrote:
> Hi Lorenzo,
> 
> Sorry for the late reply, holidays in China for the past week.
> 
> At 2017/9/27 21:54, Lorenzo Pieralisi wrote:
> > Hi Hanjun,
> >
> > On Wed, Sep 27, 2017 at 09:20:14AM +0800, Hanjun Guo wrote:
> >> 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.
> >
> > Is this actually true ? I think that currently we would simply skip
> > the entry and print an error log but we can't get a wrong ITS parent.
> 
> So the only valid single mapping under type SMMUv3 is SMMUv3's dev id
> mapping, we need to fix the IORT spec as well.
> 
> >
> > I am rewriting this commit (I will probably split it), it is doing the
> > right thing but the commit log is stale (probably caused by code
> > reshuffling).
> 
> Do I need to resend another version, or you can help to update it?
> please let me know.

I reworked the patches, you can repost/retest them I made them available
in the branch below, we will have to add a guard around ACPICA smmu
struct (unfortunately I think we will have to use the ACPICA version as
a guard) or I can ask Rafael to pull the series if ACPICA goes via ACPI
tree (and your patch made it into the release - I will check ACPICA
upstream).

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iort/smmu-msi-for-4.15

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

[Hanjun Guo]

On 2017/10/10 17:20, Lorenzo Pieralisi wrote:
> On Tue, Oct 10, 2017 at 02:47:53PM +0800, Hanjun Guo wrote:
>> Hi Lorenzo,
>>
>> Sorry for the late reply, holidays in China for the past week.
>>
>> At 2017/9/27 21:54, Lorenzo Pieralisi wrote:
>>> Hi Hanjun,
>>>
>>> On Wed, Sep 27, 2017 at 09:20:14AM +0800, Hanjun Guo wrote:
>>>> 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.
>>> Is this actually true ? I think that currently we would simply skip
>>> the entry and print an error log but we can't get a wrong ITS parent.
>> So the only valid single mapping under type SMMUv3 is SMMUv3's dev id
>> mapping, we need to fix the IORT spec as well.
>>
>>> I am rewriting this commit (I will probably split it), it is doing the
>>> right thing but the commit log is stale (probably caused by code
>>> reshuffling).
>> Do I need to resend another version, or you can help to update it?
>> please let me know.
> I reworked the patches, you can repost/retest them I made them available
> in the branch below, we will have to add a guard around ACPICA smmu
> struct (unfortunately I think we will have to use the ACPICA version as
> a guard) or I can ask Rafael to pull the series if ACPICA goes via ACPI
> tree (and your patch made it into the release - I will check ACPICA
> upstream).

Bob already merged my pull request yesterday, I think it will be ready for
acpica release for this month.

>
> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iort/smmu-msi-for-4.15
>

Thanks! I will retest then repost.

Hanjun

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

[Lorenzo Pieralisi]

On Wed, Oct 11, 2017 at 01:26:10PM +0800, Hanjun Guo wrote:
> On 2017/10/10 17:20, Lorenzo Pieralisi wrote:
> > On Tue, Oct 10, 2017 at 02:47:53PM +0800, Hanjun Guo wrote:
> >> Hi Lorenzo,
> >>
> >> Sorry for the late reply, holidays in China for the past week.
> >>
> >> At 2017/9/27 21:54, Lorenzo Pieralisi wrote:
> >>> Hi Hanjun,
> >>>
> >>> On Wed, Sep 27, 2017 at 09:20:14AM +0800, Hanjun Guo wrote:
> >>>> 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.
> >>> Is this actually true ? I think that currently we would simply skip
> >>> the entry and print an error log but we can't get a wrong ITS parent.
> >> So the only valid single mapping under type SMMUv3 is SMMUv3's dev id
> >> mapping, we need to fix the IORT spec as well.
> >>
> >>> I am rewriting this commit (I will probably split it), it is doing the
> >>> right thing but the commit log is stale (probably caused by code
> >>> reshuffling).
> >> Do I need to resend another version, or you can help to update it?
> >> please let me know.
> > I reworked the patches, you can repost/retest them I made them available
> > in the branch below, we will have to add a guard around ACPICA smmu
> > struct (unfortunately I think we will have to use the ACPICA version as
> > a guard) or I can ask Rafael to pull the series if ACPICA goes via ACPI
> > tree (and your patch made it into the release - I will check ACPICA
> > upstream).
> 
> Bob already merged my pull request yesterday, I think it will be ready for
> acpica release for this month.

That's good, mind updating the patch series with an ACPICA guard in IORT
code in preparation for the pull request ?

> > git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iort/smmu-msi-for-4.15
> >
> 
> Thanks! I will retest then repost.

Thank you,
Lorenzo

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

[Hanjun Guo]

On 2017/10/11 18:24, Lorenzo Pieralisi wrote:
> On Wed, Oct 11, 2017 at 01:26:10PM +0800, Hanjun Guo wrote:
>> On 2017/10/10 17:20, Lorenzo Pieralisi wrote:
>>> On Tue, Oct 10, 2017 at 02:47:53PM +0800, Hanjun Guo wrote:
>>>> Hi Lorenzo,
>>>>
>>>> Sorry for the late reply, holidays in China for the past week.
>>>>
>>>> At 2017/9/27 21:54, Lorenzo Pieralisi wrote:
>>>>> Hi Hanjun,
>>>>>
>>>>> On Wed, Sep 27, 2017 at 09:20:14AM +0800, Hanjun Guo wrote:
>>>>>> 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.
>>>>> Is this actually true ? I think that currently we would simply skip
>>>>> the entry and print an error log but we can't get a wrong ITS parent.
>>>> So the only valid single mapping under type SMMUv3 is SMMUv3's dev id
>>>> mapping, we need to fix the IORT spec as well.
>>>>
>>>>> I am rewriting this commit (I will probably split it), it is doing the
>>>>> right thing but the commit log is stale (probably caused by code
>>>>> reshuffling).
>>>> Do I need to resend another version, or you can help to update it?
>>>> please let me know.
>>> I reworked the patches, you can repost/retest them I made them available
>>> in the branch below, we will have to add a guard around ACPICA smmu
>>> struct (unfortunately I think we will have to use the ACPICA version as
>>> a guard) or I can ask Rafael to pull the series if ACPICA goes via ACPI
>>> tree (and your patch made it into the release - I will check ACPICA
>>> upstream).
>> Bob already merged my pull request yesterday, I think it will be ready for
>> acpica release for this month.
> That's good, mind updating the patch series with an ACPICA guard in IORT
> code in preparation for the pull request ?

Do you mean drop the acpica patch in this patch set and adding the code below?

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 37a1b9f..a883bec 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -366,6 +366,7 @@ static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
        return NULL;
 }
 
+#if (ACPI_CA_VERSION > 0x20170929)
 static int iort_get_id_mapping_index(struct acpi_iort_node *node)
 {
        struct acpi_iort_smmu_v3 *smmu;
@@ -399,6 +400,12 @@ static int iort_get_id_mapping_index(struct acpi_iort_node *node)
                return -EINVAL;
        }
 }
+#else
+static int iort_get_id_mapping_index(struct acpi_iort_node *node)
+{
+ return -EINVAL;
+}
+#endif
 
Thanks
Hanjun

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

[Lorenzo Pieralisi]

On Thu, Oct 12, 2017 at 03:30:10PM +0800, Hanjun Guo wrote:
> On 2017/10/11 18:24, Lorenzo Pieralisi wrote:
> > On Wed, Oct 11, 2017 at 01:26:10PM +0800, Hanjun Guo wrote:
> >> On 2017/10/10 17:20, Lorenzo Pieralisi wrote:
> >>> On Tue, Oct 10, 2017 at 02:47:53PM +0800, Hanjun Guo wrote:
> >>>> Hi Lorenzo,
> >>>>
> >>>> Sorry for the late reply, holidays in China for the past week.
> >>>>
> >>>> At 2017/9/27 21:54, Lorenzo Pieralisi wrote:
> >>>>> Hi Hanjun,
> >>>>>
> >>>>> On Wed, Sep 27, 2017 at 09:20:14AM +0800, Hanjun Guo wrote:
> >>>>>> 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.
> >>>>> Is this actually true ? I think that currently we would simply skip
> >>>>> the entry and print an error log but we can't get a wrong ITS parent.
> >>>> So the only valid single mapping under type SMMUv3 is SMMUv3's dev id
> >>>> mapping, we need to fix the IORT spec as well.
> >>>>
> >>>>> I am rewriting this commit (I will probably split it), it is doing the
> >>>>> right thing but the commit log is stale (probably caused by code
> >>>>> reshuffling).
> >>>> Do I need to resend another version, or you can help to update it?
> >>>> please let me know.
> >>> I reworked the patches, you can repost/retest them I made them available
> >>> in the branch below, we will have to add a guard around ACPICA smmu
> >>> struct (unfortunately I think we will have to use the ACPICA version as
> >>> a guard) or I can ask Rafael to pull the series if ACPICA goes via ACPI
> >>> tree (and your patch made it into the release - I will check ACPICA
> >>> upstream).
> >> Bob already merged my pull request yesterday, I think it will be ready for
> >> acpica release for this month.
> > That's good, mind updating the patch series with an ACPICA guard in IORT
> > code in preparation for the pull request ?
> 
> Do you mean drop the acpica patch in this patch set and adding the code below?

Yes unless we find a "smarter" way of handling the dependency.

Thanks,
Lorenzo

> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 37a1b9f..a883bec 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -366,6 +366,7 @@ static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>         return NULL;
>  }
>  
> +#if (ACPI_CA_VERSION > 0x20170929)
>  static int iort_get_id_mapping_index(struct acpi_iort_node *node)
>  {
>         struct acpi_iort_smmu_v3 *smmu;
> @@ -399,6 +400,12 @@ static int iort_get_id_mapping_index(struct acpi_iort_node *node)
>                 return -EINVAL;
>         }
>  }
> +#else
> +static int iort_get_id_mapping_index(struct acpi_iort_node *node)
> +{
> + return -EINVAL;
> +}
> +#endif
>  
> Thanks
> Hanjun
> 

[PATCH 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 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().

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

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 1c1160e..06b38de 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -633,6 +633,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 index;
+
+	index = iort_get_id_mapping_index(node);
+	if (index < 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
@@ -1259,6 +1302,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.9.1