| Message ID | 1483363905-2806-11-git-send-email-hanjun.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
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@linaro.org> >> Tested-by: Majun <majun258@huawei.com> >> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com> >> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >> --- >> 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@codeaurora.org> Thank you very much :) Hanjun
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; }