From: Marc Zyngier <marc.zyngier@arm.com> To: Hanjun Guo <hanjun.guo@linaro.org>, Tomasz Nowicki <tn@semihalf.com>, tglx@linutronix.de, jason@lakedaemon.net, rjw@rjwysocki.net, bhelgaas@google.com, lorenzo.pieralisi@arm.com, robert.richter@caviumnetworks.com, shijie.huang@arm.com, Suravee.Suthikulpanit@amd.com Cc: al.stone@linaro.org, mw@semihalf.com, graeme.gregory@linaro.org, Catalin.Marinas@arm.com, will.deacon@arm.com, linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, ddaney.cavm@gmail.com, okaya@codeaurora.org, andrea.gallo@linaro.org, linux-pci@vger.kernel.org Subject: Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support Date: Wed, 22 Jun 2016 11:50:39 +0100 [thread overview] Message-ID: <576A6D7F.6010407@arm.com> (raw) In-Reply-To: <372d3b78-2bc5-94d1-49f6-c9229affa65d@linaro.org> On 21/06/16 08:12, Hanjun Guo wrote: > Hi Tomasz, > > Sorry for jumping out late, just one comment below. > > On 2016/6/20 19:02, Tomasz Nowicki wrote: >> IORT shows representation of IO topology for ARM based systems. >> It describes how various components are connected together on >> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec. >> >> Initial support allows to detect IORT table presence and save its >> root pointer obtained through acpi_get_table(). The pointer validity >> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap >> is not set while using IORT nodes we would dereference unmapped pointers. >> >> For the aforementioned reason call iort_table_detect() from acpi_init() >> which guarantees acpi_gbl_permanent_mmap to be set at that point. >> >> Add generic helpers which are helpful for scanning and retrieving >> information from IORT table content. List of the most important helpers: >> - iort_find_dev_node() finds IORT node for a given device >> - iort_node_map_rid() maps device RID and returns IORT node which provides >> final translation >> >> Signed-off-by: Tomasz Nowicki <tn@semihalf.com> >> --- >> drivers/acpi/Kconfig | 3 + >> drivers/acpi/Makefile | 1 + >> drivers/acpi/bus.c | 2 + >> drivers/acpi/iort.c | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/iort.h | 30 +++++++ > > [...] > >> +static acpi_status >> +iort_match_node_callback(struct acpi_iort_node *node, void *context) >> +{ >> + struct device *dev = context; >> + >> + switch (node->type) { >> + case ACPI_IORT_NODE_NAMED_COMPONENT: { >> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > > ACPI_ALLOCATE_BUFFER is used here, so ... > >> + struct acpi_device *adev = to_acpi_device_node(dev->fwnode); >> + struct acpi_iort_named_component *ncomp; >> + >> + if (!adev) >> + break; >> + >> + ncomp = (struct acpi_iort_named_component *)node->node_data; >> + >> + if (ACPI_FAILURE(acpi_get_name(adev->handle, >> + ACPI_FULL_PATHNAME, &buffer))) { >> + dev_warn(dev, "Can't get device full path name\n"); >> + break; >> + } >> + >> + if (!strcmp(ncomp->device_name, (char *)buffer.pointer)) >> + return AE_OK; > > ... we need to kfree(buffer.pointer) before we return or break. For the record, I've queued this patch on top: diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c index 496dcf6..3b9e55b 100644 --- a/drivers/acpi/iort.c +++ b/drivers/acpi/iort.c @@ -158,11 +158,15 @@ iort_match_node_callback(struct acpi_iort_node *node, void *context) if (ACPI_FAILURE(acpi_get_name(adev->handle, ACPI_FULL_PATHNAME, &buffer))) { dev_warn(dev, "Can't get device full path name\n"); - break; - } + } else { + int match; - if (!strcmp(ncomp->device_name, (char *)buffer.pointer)) - return AE_OK; + match = !strcmp(ncomp->device_name, buffer.pointer); + acpi_os_free(&buffer); + + if (match) + return AE_OK; + } break; } assuming that Rafael is OK with the general approach, of course. Thanks, M. -- Jazz is not dead. It just smells funny...
WARNING: multiple messages have this Message-ID (diff)
From: marc.zyngier@arm.com (Marc Zyngier) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support Date: Wed, 22 Jun 2016 11:50:39 +0100 [thread overview] Message-ID: <576A6D7F.6010407@arm.com> (raw) In-Reply-To: <372d3b78-2bc5-94d1-49f6-c9229affa65d@linaro.org> On 21/06/16 08:12, Hanjun Guo wrote: > Hi Tomasz, > > Sorry for jumping out late, just one comment below. > > On 2016/6/20 19:02, Tomasz Nowicki wrote: >> IORT shows representation of IO topology for ARM based systems. >> It describes how various components are connected together on >> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec. >> >> Initial support allows to detect IORT table presence and save its >> root pointer obtained through acpi_get_table(). The pointer validity >> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap >> is not set while using IORT nodes we would dereference unmapped pointers. >> >> For the aforementioned reason call iort_table_detect() from acpi_init() >> which guarantees acpi_gbl_permanent_mmap to be set at that point. >> >> Add generic helpers which are helpful for scanning and retrieving >> information from IORT table content. List of the most important helpers: >> - iort_find_dev_node() finds IORT node for a given device >> - iort_node_map_rid() maps device RID and returns IORT node which provides >> final translation >> >> Signed-off-by: Tomasz Nowicki <tn@semihalf.com> >> --- >> drivers/acpi/Kconfig | 3 + >> drivers/acpi/Makefile | 1 + >> drivers/acpi/bus.c | 2 + >> drivers/acpi/iort.c | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/iort.h | 30 +++++++ > > [...] > >> +static acpi_status >> +iort_match_node_callback(struct acpi_iort_node *node, void *context) >> +{ >> + struct device *dev = context; >> + >> + switch (node->type) { >> + case ACPI_IORT_NODE_NAMED_COMPONENT: { >> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > > ACPI_ALLOCATE_BUFFER is used here, so ... > >> + struct acpi_device *adev = to_acpi_device_node(dev->fwnode); >> + struct acpi_iort_named_component *ncomp; >> + >> + if (!adev) >> + break; >> + >> + ncomp = (struct acpi_iort_named_component *)node->node_data; >> + >> + if (ACPI_FAILURE(acpi_get_name(adev->handle, >> + ACPI_FULL_PATHNAME, &buffer))) { >> + dev_warn(dev, "Can't get device full path name\n"); >> + break; >> + } >> + >> + if (!strcmp(ncomp->device_name, (char *)buffer.pointer)) >> + return AE_OK; > > ... we need to kfree(buffer.pointer) before we return or break. For the record, I've queued this patch on top: diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c index 496dcf6..3b9e55b 100644 --- a/drivers/acpi/iort.c +++ b/drivers/acpi/iort.c @@ -158,11 +158,15 @@ iort_match_node_callback(struct acpi_iort_node *node, void *context) if (ACPI_FAILURE(acpi_get_name(adev->handle, ACPI_FULL_PATHNAME, &buffer))) { dev_warn(dev, "Can't get device full path name\n"); - break; - } + } else { + int match; - if (!strcmp(ncomp->device_name, (char *)buffer.pointer)) - return AE_OK; + match = !strcmp(ncomp->device_name, buffer.pointer); + acpi_os_free(&buffer); + + if (match) + return AE_OK; + } break; } assuming that Rafael is OK with the general approach, of course. Thanks, M. -- Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2016-06-22 10:50 UTC|newest] Thread overview: 91+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-06-20 11:02 [PATCH V7 0/8] Introduce ACPI world to ITS irqchip Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-21 7:12 ` Hanjun Guo 2016-06-21 7:12 ` Hanjun Guo 2016-06-21 7:12 ` Hanjun Guo 2016-06-21 7:12 ` Hanjun Guo 2016-06-22 10:50 ` Marc Zyngier [this message] 2016-06-22 10:50 ` Marc Zyngier 2016-06-22 11:06 ` Tomasz Nowicki 2016-06-22 11:06 ` Tomasz Nowicki 2016-06-22 12:03 ` Marc Zyngier 2016-06-22 12:03 ` Marc Zyngier 2016-06-21 11:23 ` Lorenzo Pieralisi 2016-06-21 11:23 ` Lorenzo Pieralisi 2016-06-21 17:36 ` Lorenzo Pieralisi 2016-06-21 17:36 ` Lorenzo Pieralisi 2016-06-22 12:35 ` Tomasz Nowicki 2016-06-22 12:35 ` Tomasz Nowicki 2016-06-22 12:35 ` Tomasz Nowicki 2016-06-22 12:40 ` Tomasz Nowicki 2016-06-22 12:40 ` Tomasz Nowicki 2016-06-22 12:40 ` Tomasz Nowicki 2016-06-22 12:40 ` Tomasz Nowicki 2016-06-22 13:25 ` Marc Zyngier 2016-06-22 13:25 ` Marc Zyngier 2016-06-22 13:52 ` Tomasz Nowicki 2016-06-22 13:52 ` Tomasz Nowicki 2016-06-22 13:52 ` Tomasz Nowicki 2016-06-22 14:51 ` Marc Zyngier 2016-06-22 14:51 ` Marc Zyngier 2016-06-23 1:34 ` Hanjun Guo 2016-06-23 1:34 ` Hanjun Guo 2016-06-23 1:34 ` Hanjun Guo 2016-07-26 13:19 ` Christopher Covington 2016-07-26 13:19 ` Christopher Covington 2016-07-26 13:19 ` Christopher Covington 2016-07-26 14:48 ` Marc Zyngier 2016-07-26 14:48 ` Marc Zyngier 2016-07-26 14:48 ` Marc Zyngier 2016-06-20 11:02 ` [PATCH V7 2/8] ACPI: Add new IORT functions to support MSI domain handling Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-21 7:28 ` Hanjun Guo 2016-06-21 7:28 ` Hanjun Guo 2016-06-20 11:02 ` [PATCH V7 3/8] PCI/MSI: Setup MSI domain on a per-device basis using IORT ACPI table Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-21 7:33 ` Hanjun Guo 2016-06-21 7:33 ` Hanjun Guo 2016-07-19 21:42 ` Bjorn Helgaas 2016-07-19 21:42 ` Bjorn Helgaas 2016-06-20 11:02 ` [PATCH V7 4/8] irqchip/gicv3-its: Cleanup for ITS domain initialization Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-21 7:44 ` Hanjun Guo 2016-06-21 7:44 ` Hanjun Guo 2016-06-21 7:44 ` Hanjun Guo 2016-06-21 7:44 ` Hanjun Guo 2016-06-20 11:02 ` [PATCH V7 5/8] irqchip/gicv3-its: Refactor ITS DT init code to prepare for ACPI Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` [PATCH V7 6/8] irqchip/gicv3-its: Probe ITS in the ACPI way Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-21 8:03 ` Hanjun Guo 2016-06-21 8:03 ` Hanjun Guo 2016-06-21 8:03 ` Hanjun Guo 2016-06-20 11:02 ` [PATCH V7 7/8] irqchip/gicv3-its: Factor out code that might be reused for ACPI Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` [PATCH V7 8/8] irqchip/gicv3-its: Use MADT ITS subtable to do PCI/MSI domain initialization Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-20 11:02 ` Tomasz Nowicki 2016-06-21 8:19 ` Hanjun Guo 2016-06-21 8:19 ` Hanjun Guo 2016-06-21 8:19 ` Hanjun Guo 2016-06-21 8:19 ` Hanjun Guo 2016-06-20 17:31 ` [PATCH V7 0/8] Introduce ACPI world to ITS irqchip Shanker Donthineni 2016-06-20 17:31 ` Shanker Donthineni 2016-06-20 17:31 ` Shanker Donthineni 2016-06-24 11:04 ` Tomasz Nowicki 2016-06-24 11:04 ` Tomasz Nowicki 2016-08-09 10:45 ` Robert Richter 2016-08-09 10:45 ` Robert Richter 2016-08-09 10:45 ` Robert Richter 2016-08-09 10:45 ` Robert Richter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=576A6D7F.6010407@arm.com \ --to=marc.zyngier@arm.com \ --cc=Catalin.Marinas@arm.com \ --cc=Suravee.Suthikulpanit@amd.com \ --cc=al.stone@linaro.org \ --cc=andrea.gallo@linaro.org \ --cc=bhelgaas@google.com \ --cc=ddaney.cavm@gmail.com \ --cc=graeme.gregory@linaro.org \ --cc=hanjun.guo@linaro.org \ --cc=jason@lakedaemon.net \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=mw@semihalf.com \ --cc=okaya@codeaurora.org \ --cc=rjw@rjwysocki.net \ --cc=robert.richter@caviumnetworks.com \ --cc=shijie.huang@arm.com \ --cc=tglx@linutronix.de \ --cc=tn@semihalf.com \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.