Re: [PATCH v6 10/14] ACPI: ARM64: IORT: rework iort_node_get_id() for NC->SMMU->ITS case

From: Hanjun Guo
Date: Mon Jan 02 2017 - 19:12:36 EST

Hi Sinan,

On 01/03/2017 06:30 AM, Sinan Kaya wrote:
Hi Hanjun,

On 1/2/2017 8:31 AM, Hanjun Guo wrote:
iort_node_get_id() for now only support NC(named componant)->SMMU
or NC->ITS cases, we also have other device topology such NC->
SMMU->ITS, so rework iort_node_get_id() for those cases.

Signed-off-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx>
Tested-by: Majun <majun258@xxxxxxxxxx>
Tested-by: Xinwei Kong <kong.kongxinwei@xxxxxxxxxxxxx>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
---
drivers/acpi/arm64/iort.c | 61 ++++++++++++++++++++++++++---------------------
1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 6b72fcb..99f079b 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -292,22 +292,28 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
return status;
}

-static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
- u32 *rid_out)
+static int iort_id_single_map(struct acpi_iort_id_mapping *map, u8 type,
+ u32 *rid_out)
{
/* Single mapping does not care for input id */
if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
- *rid_out = map->output_base;
+ if (rid_out)
+ *rid_out = map->output_base;
return 0;
}

pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node type %d, skipping ID map\n",
map, type);
- return -ENXIO;
}

+ return -ENXIO;
+}
+
+static int iort_id_map(struct acpi_iort_id_mapping *map, u32 rid_in,
+ u32 *rid_out)
+{
if (rid_in < map->input_base ||
(rid_in >= map->input_base + map->id_count))
return -ENXIO;
@@ -324,33 +330,34 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
struct acpi_iort_node *parent;
struct acpi_iort_id_mapping *map;

- if (!node->mapping_offset || !node->mapping_count ||
- index >= node->mapping_count)
- return NULL;
-
- map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
- node->mapping_offset);
+ while (node) {
+ if (!node->mapping_offset || !node->mapping_count ||
+ index >= node->mapping_count)
+ return NULL;

- /* Firmware bug! */
- if (!map->output_reference) {
- pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
- node, node->type);
- return NULL;
- }
+ map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
+ node->mapping_offset);

- parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
- map->output_reference);
+ /* Firmware bug! */
+ if (!map->output_reference) {
+ pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
+ node, node->type);
+ return NULL;
+ }

- if (!(IORT_TYPE_MASK(parent->type) & type_mask))
- return NULL;
+ parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+ map->output_reference);

- if (map[index].flags & ACPI_IORT_ID_SINGLE_MAPPING) {
- if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
- node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
- if (id_out)
- *id_out = map[index].output_base;
- return parent;
+ /* go upstream to find its parent */
+ if (!(IORT_TYPE_MASK(parent->type) & type_mask)) {
+ node = parent;
+ continue;
}
+
+ if (iort_id_single_map(&map[index], node->type, id_out))
+ break;
+
+ return parent;
}

return NULL;
@@ -388,7 +395,7 @@ static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,

/* Do the RID translation */
for (i = 0; i < node->mapping_count; i++, map++) {
- if (!iort_id_map(map, node->type, rid, &rid))
+ if (!iort_id_map(map, rid, &rid))
break;
}



I wanted to follow up on your note for NC->SMMU->ITS case as I do have this use case on the
Qualcomm QDF2400 server and HIDMA DMA Engine. HIDMA is capable of sending MSI interrupts
towards the GIC ITS.

I don't know if this patch is supposed to fix the NC->SMMU->ITS case as it suggests in the commit
message but it doesn't seems to be working for me. Maybe, it was a to do for you. It wasn't quite
clear from the commit.

I noticed this issue too after I sent out this patch set, sorry :(


I debugged the code and came up with the following patch. Feel free to incorporate/rework with
your existing patch.

A named node can have an output ID of 0x20 and SMMU can have an output
parameter of 0x80000. The device ID needs to be 0x80000+0x20 for this
use case.

I think in your case, there are muti input IDs with multi output IDs,
such as:

stream id request id
NC (0x00~0x30) --------> SMMU (0x80000~0x80000+0x30) ------------> ITS

In my patch, I just think named component is single mapping only, and
multi ID mappings for PCI RC, that's the wrong assumption, I will
incorporate your patch to fix the problem in next version.


With the addition of this patch on top of the first 11 patches, I'm also providing my tested by here
for the first 11 patches.

Tested-by: Sinan Kaya <okaya@xxxxxxxxxxxxxx>

Thank you very much :)

Hanjun