From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756655AbdACAL1 (ORCPT ); Mon, 2 Jan 2017 19:11:27 -0500 Received: from mail-pg0-f53.google.com ([74.125.83.53]:33862 "EHLO mail-pg0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755564AbdACAK4 (ORCPT ); Mon, 2 Jan 2017 19:10:56 -0500 Message-ID: <586AEB92.5020707@linaro.org> Date: Tue, 03 Jan 2017 08:08:50 +0800 From: Hanjun Guo User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Sinan Kaya , Marc Zyngier , "Rafael J. Wysocki" , Lorenzo Pieralisi CC: linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linuxarm@huawei.com, Thomas Gleixner , Greg KH , Tomasz Nowicki , Ma Jun , Kefeng Wang , Agustin Vega-Frias , charles.garcia-tobin@arm.com, huxinwei@huawei.com, yimin@huawei.com, Jon Masters Subject: Re: [PATCH v6 10/14] ACPI: ARM64: IORT: rework iort_node_get_id() for NC->SMMU->ITS case References: <1483363905-2806-1-git-send-email-hanjun.guo@linaro.org> <1483363905-2806-11-git-send-email-hanjun.guo@linaro.org> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> Tested-by: Majun >> Tested-by: Xinwei Kong >> Cc: Lorenzo Pieralisi >> --- >> 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 Thank you very much :) Hanjun