| Message ID | 1466598909-27504-1-git-send-email-tn@semihalf.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 22.06.2016 14:35, 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. > http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf > > 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> The updated patch incorporates fixes and addresses Lorenzo's comments. The whole series is here: https://github.com/semihalf-nowicki-tomasz/linux.git (its-acpi-v7.1) Thanks, Tomasz
On 22/06/16 13:35, 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. > http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf > > 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 | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/iort.h | 30 +++++++ > 5 files changed, 253 insertions(+) > create mode 100644 drivers/acpi/iort.c > create mode 100644 include/linux/iort.h > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > index b7e2e77..848471f 100644 > --- a/drivers/acpi/Kconfig > +++ b/drivers/acpi/Kconfig > @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT > config ACPI_CCA_REQUIRED > bool > > +config IORT_TABLE > + bool > + > config ACPI_DEBUGGER > bool "AML debugger interface" > select ACPI_DEBUG > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > index 251ce85..c7c9b29 100644 > --- a/drivers/acpi/Makefile > +++ b/drivers/acpi/Makefile > @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o > obj-$(CONFIG_ACPI_BGRT) += bgrt.o > obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o > obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o > +obj-$(CONFIG_IORT_TABLE) += iort.o > > # processor has its own "processor." module_param namespace > processor-y := processor_driver.o > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 31e8da6..176c17d 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -33,6 +33,7 @@ > #ifdef CONFIG_X86 > #include <asm/mpspec.h> > #endif > +#include <linux/iort.h> > #include <linux/pci.h> > #include <acpi/apei.h> > #include <linux/dmi.h> > @@ -1118,6 +1119,7 @@ static int __init acpi_init(void) > } > > pci_mmcfg_late_init(); > + iort_table_detect(); > acpi_scan_init(); > acpi_ec_init(); > acpi_debugfs_init(); > diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c > new file mode 100644 > index 0000000..fcfa008f > --- /dev/null > +++ b/drivers/acpi/iort.c > @@ -0,0 +1,217 @@ > +/* > + * Copyright (C) 2016, Semihalf > + * Author: Tomasz Nowicki <tn@semihalf.com> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * This file implements early detection/parsing of I/O mapping > + * reported to OS through firmware via I/O Remapping Table (IORT) > + * IORT document number: ARM DEN 0049A > + */ > + > +#define pr_fmt(fmt) "ACPI: IORT: " fmt > + > +#include <linux/iort.h> > +#include <linux/kernel.h> > +#include <linux/pci.h> > + > +typedef acpi_status (*iort_find_node_callback) > + (struct acpi_iort_node *node, void *context); > + > +/* Root pointer to the mapped IORT table */ > +static struct acpi_table_header *iort_table; > + > +static struct acpi_iort_node * > +iort_scan_node(enum acpi_iort_node_type type, > + iort_find_node_callback callback, void *context) > +{ > + struct acpi_iort_node *iort_node, *iort_end; > + struct acpi_table_iort *iort; > + int i; > + > + /* Get the first IORT node */ > + iort = (struct acpi_table_iort *)iort_table; > + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort, > + iort->node_offset); > + iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table, > + iort_table->length); > + > + for (i = 0; i < iort->node_count; i++) { > + if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND, > + "IORT node pointer overflows, bad table!\n")) > + return NULL; > + > + if (iort_node->type == type) { > + if (ACPI_SUCCESS(callback(iort_node, context))) > + return iort_node; > + } > + > + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node, > + iort_node->length); > + } > + > + return NULL; > +} > + > +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 }; > + 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"); > + } else { > + int match; > + > + match = !strcmp(ncomp->device_name, buffer.pointer); > + kfree(buffer.pointer); Why did you change this to a naked kfree? The ACPI code clearly states: /* * Allocate a new buffer. We directectly call acpi_os_allocate here to * purposefully bypass the (optionally enabled) internal allocation * tracking mechanism since we only want to track internal * allocations. Note: The caller should use acpi_os_free to free this * buffer created via ACPI_ALLOCATE_BUFFER. */ Thanks, M.
On 22.06.2016 15:25, Marc Zyngier wrote: > On 22/06/16 13:35, 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. >> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf >> >> 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 | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/iort.h | 30 +++++++ >> 5 files changed, 253 insertions(+) >> create mode 100644 drivers/acpi/iort.c >> create mode 100644 include/linux/iort.h >> >> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig >> index b7e2e77..848471f 100644 >> --- a/drivers/acpi/Kconfig >> +++ b/drivers/acpi/Kconfig >> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT >> config ACPI_CCA_REQUIRED >> bool >> >> +config IORT_TABLE >> + bool >> + >> config ACPI_DEBUGGER >> bool "AML debugger interface" >> select ACPI_DEBUG >> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile >> index 251ce85..c7c9b29 100644 >> --- a/drivers/acpi/Makefile >> +++ b/drivers/acpi/Makefile >> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o >> obj-$(CONFIG_ACPI_BGRT) += bgrt.o >> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o >> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o >> +obj-$(CONFIG_IORT_TABLE) += iort.o >> >> # processor has its own "processor." module_param namespace >> processor-y := processor_driver.o >> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c >> index 31e8da6..176c17d 100644 >> --- a/drivers/acpi/bus.c >> +++ b/drivers/acpi/bus.c >> @@ -33,6 +33,7 @@ >> #ifdef CONFIG_X86 >> #include <asm/mpspec.h> >> #endif >> +#include <linux/iort.h> >> #include <linux/pci.h> >> #include <acpi/apei.h> >> #include <linux/dmi.h> >> @@ -1118,6 +1119,7 @@ static int __init acpi_init(void) >> } >> >> pci_mmcfg_late_init(); >> + iort_table_detect(); >> acpi_scan_init(); >> acpi_ec_init(); >> acpi_debugfs_init(); >> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c >> new file mode 100644 >> index 0000000..fcfa008f >> --- /dev/null >> +++ b/drivers/acpi/iort.c >> @@ -0,0 +1,217 @@ >> +/* >> + * Copyright (C) 2016, Semihalf >> + * Author: Tomasz Nowicki <tn@semihalf.com> >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms and conditions of the GNU General Public License, >> + * version 2, as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope it will be useful, but WITHOUT >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >> + * more details. >> + * >> + * This file implements early detection/parsing of I/O mapping >> + * reported to OS through firmware via I/O Remapping Table (IORT) >> + * IORT document number: ARM DEN 0049A >> + */ >> + >> +#define pr_fmt(fmt) "ACPI: IORT: " fmt >> + >> +#include <linux/iort.h> >> +#include <linux/kernel.h> >> +#include <linux/pci.h> >> + >> +typedef acpi_status (*iort_find_node_callback) >> + (struct acpi_iort_node *node, void *context); >> + >> +/* Root pointer to the mapped IORT table */ >> +static struct acpi_table_header *iort_table; >> + >> +static struct acpi_iort_node * >> +iort_scan_node(enum acpi_iort_node_type type, >> + iort_find_node_callback callback, void *context) >> +{ >> + struct acpi_iort_node *iort_node, *iort_end; >> + struct acpi_table_iort *iort; >> + int i; >> + >> + /* Get the first IORT node */ >> + iort = (struct acpi_table_iort *)iort_table; >> + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort, >> + iort->node_offset); >> + iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table, >> + iort_table->length); >> + >> + for (i = 0; i < iort->node_count; i++) { >> + if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND, >> + "IORT node pointer overflows, bad table!\n")) >> + return NULL; >> + >> + if (iort_node->type == type) { >> + if (ACPI_SUCCESS(callback(iort_node, context))) >> + return iort_node; >> + } >> + >> + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node, >> + iort_node->length); >> + } >> + >> + return NULL; >> +} >> + >> +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 }; >> + 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"); >> + } else { >> + int match; >> + >> + match = !strcmp(ncomp->device_name, buffer.pointer); >> + kfree(buffer.pointer); > > Why did you change this to a naked kfree? The ACPI code clearly states: > > /* > * Allocate a new buffer. We directectly call acpi_os_allocate here to > * purposefully bypass the (optionally enabled) internal allocation > * tracking mechanism since we only want to track internal > * allocations. Note: The caller should use acpi_os_free to free this > * buffer created via ACPI_ALLOCATE_BUFFER. > */ Yes we should use symmetric free function to acpi_os_allocate here, which means acpi_os_free should be used here. My main motivation was to free buffer.pointer instead of &buffer Would you mind to fix that (s/kfree/acpi_os_free) before merging ? Thanks, Tomasz
On 22/06/16 14:52, Tomasz Nowicki wrote: > On 22.06.2016 15:25, Marc Zyngier wrote: >> On 22/06/16 13:35, 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. >>> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf >>> >>> 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 | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>> include/linux/iort.h | 30 +++++++ >>> 5 files changed, 253 insertions(+) >>> create mode 100644 drivers/acpi/iort.c >>> create mode 100644 include/linux/iort.h >>> >>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig >>> index b7e2e77..848471f 100644 >>> --- a/drivers/acpi/Kconfig >>> +++ b/drivers/acpi/Kconfig >>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT >>> config ACPI_CCA_REQUIRED >>> bool >>> >>> +config IORT_TABLE >>> + bool >>> + >>> config ACPI_DEBUGGER >>> bool "AML debugger interface" >>> select ACPI_DEBUG >>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile >>> index 251ce85..c7c9b29 100644 >>> --- a/drivers/acpi/Makefile >>> +++ b/drivers/acpi/Makefile >>> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o >>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o >>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o >>> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o >>> +obj-$(CONFIG_IORT_TABLE) += iort.o >>> >>> # processor has its own "processor." module_param namespace >>> processor-y := processor_driver.o >>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c >>> index 31e8da6..176c17d 100644 >>> --- a/drivers/acpi/bus.c >>> +++ b/drivers/acpi/bus.c >>> @@ -33,6 +33,7 @@ >>> #ifdef CONFIG_X86 >>> #include <asm/mpspec.h> >>> #endif >>> +#include <linux/iort.h> >>> #include <linux/pci.h> >>> #include <acpi/apei.h> >>> #include <linux/dmi.h> >>> @@ -1118,6 +1119,7 @@ static int __init acpi_init(void) >>> } >>> >>> pci_mmcfg_late_init(); >>> + iort_table_detect(); >>> acpi_scan_init(); >>> acpi_ec_init(); >>> acpi_debugfs_init(); >>> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c >>> new file mode 100644 >>> index 0000000..fcfa008f >>> --- /dev/null >>> +++ b/drivers/acpi/iort.c >>> @@ -0,0 +1,217 @@ >>> +/* >>> + * Copyright (C) 2016, Semihalf >>> + * Author: Tomasz Nowicki <tn@semihalf.com> >>> + * >>> + * This program is free software; you can redistribute it and/or modify it >>> + * under the terms and conditions of the GNU General Public License, >>> + * version 2, as published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope it will be useful, but WITHOUT >>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >>> + * more details. >>> + * >>> + * This file implements early detection/parsing of I/O mapping >>> + * reported to OS through firmware via I/O Remapping Table (IORT) >>> + * IORT document number: ARM DEN 0049A >>> + */ >>> + >>> +#define pr_fmt(fmt) "ACPI: IORT: " fmt >>> + >>> +#include <linux/iort.h> >>> +#include <linux/kernel.h> >>> +#include <linux/pci.h> >>> + >>> +typedef acpi_status (*iort_find_node_callback) >>> + (struct acpi_iort_node *node, void *context); >>> + >>> +/* Root pointer to the mapped IORT table */ >>> +static struct acpi_table_header *iort_table; >>> + >>> +static struct acpi_iort_node * >>> +iort_scan_node(enum acpi_iort_node_type type, >>> + iort_find_node_callback callback, void *context) >>> +{ >>> + struct acpi_iort_node *iort_node, *iort_end; >>> + struct acpi_table_iort *iort; >>> + int i; >>> + >>> + /* Get the first IORT node */ >>> + iort = (struct acpi_table_iort *)iort_table; >>> + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort, >>> + iort->node_offset); >>> + iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table, >>> + iort_table->length); >>> + >>> + for (i = 0; i < iort->node_count; i++) { >>> + if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND, >>> + "IORT node pointer overflows, bad table!\n")) >>> + return NULL; >>> + >>> + if (iort_node->type == type) { >>> + if (ACPI_SUCCESS(callback(iort_node, context))) >>> + return iort_node; >>> + } >>> + >>> + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node, >>> + iort_node->length); >>> + } >>> + >>> + return NULL; >>> +} >>> + >>> +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 }; >>> + 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"); >>> + } else { >>> + int match; >>> + >>> + match = !strcmp(ncomp->device_name, buffer.pointer); >>> + kfree(buffer.pointer); >> >> Why did you change this to a naked kfree? The ACPI code clearly states: >> >> /* >> * Allocate a new buffer. We directectly call acpi_os_allocate here to >> * purposefully bypass the (optionally enabled) internal allocation >> * tracking mechanism since we only want to track internal >> * allocations. Note: The caller should use acpi_os_free to free this >> * buffer created via ACPI_ALLOCATE_BUFFER. >> */ > > Yes we should use symmetric free function to acpi_os_allocate here, > which means acpi_os_free should be used here. > > My main motivation was to free buffer.pointer instead of &buffer > > Would you mind to fix that (s/kfree/acpi_os_free) before merging ? Sure. M.
On 2016/6/22 22:51, Marc Zyngier wrote: > On 22/06/16 14:52, Tomasz Nowicki wrote: >> On 22.06.2016 15:25, Marc Zyngier wrote: >>> On 22/06/16 13:35, 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. >>>> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf >>>> >>>> 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 | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> include/linux/iort.h | 30 +++++++ >>>> 5 files changed, 253 insertions(+) >>>> create mode 100644 drivers/acpi/iort.c >>>> create mode 100644 include/linux/iort.h >>>> >>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig >>>> index b7e2e77..848471f 100644 >>>> --- a/drivers/acpi/Kconfig >>>> +++ b/drivers/acpi/Kconfig >>>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT >>>> config ACPI_CCA_REQUIRED >>>> bool >>>> >>>> +config IORT_TABLE >>>> + bool >>>> + >>>> config ACPI_DEBUGGER >>>> bool "AML debugger interface" >>>> select ACPI_DEBUG >>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile >>>> index 251ce85..c7c9b29 100644 >>>> --- a/drivers/acpi/Makefile >>>> +++ b/drivers/acpi/Makefile >>>> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o >>>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o >>>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o >>>> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o >>>> +obj-$(CONFIG_IORT_TABLE) += iort.o >>>> >>>> # processor has its own "processor." module_param namespace >>>> processor-y := processor_driver.o >>>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c >>>> index 31e8da6..176c17d 100644 >>>> --- a/drivers/acpi/bus.c >>>> +++ b/drivers/acpi/bus.c >>>> @@ -33,6 +33,7 @@ >>>> #ifdef CONFIG_X86 >>>> #include <asm/mpspec.h> >>>> #endif >>>> +#include <linux/iort.h> >>>> #include <linux/pci.h> >>>> #include <acpi/apei.h> >>>> #include <linux/dmi.h> >>>> @@ -1118,6 +1119,7 @@ static int __init acpi_init(void) >>>> } >>>> >>>> pci_mmcfg_late_init(); >>>> + iort_table_detect(); >>>> acpi_scan_init(); >>>> acpi_ec_init(); >>>> acpi_debugfs_init(); >>>> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c >>>> new file mode 100644 >>>> index 0000000..fcfa008f >>>> --- /dev/null >>>> +++ b/drivers/acpi/iort.c >>>> @@ -0,0 +1,217 @@ >>>> +/* >>>> + * Copyright (C) 2016, Semihalf >>>> + * Author: Tomasz Nowicki <tn@semihalf.com> >>>> + * >>>> + * This program is free software; you can redistribute it and/or modify it >>>> + * under the terms and conditions of the GNU General Public License, >>>> + * version 2, as published by the Free Software Foundation. >>>> + * >>>> + * This program is distributed in the hope it will be useful, but WITHOUT >>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >>>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >>>> + * more details. >>>> + * >>>> + * This file implements early detection/parsing of I/O mapping >>>> + * reported to OS through firmware via I/O Remapping Table (IORT) >>>> + * IORT document number: ARM DEN 0049A >>>> + */ >>>> + >>>> +#define pr_fmt(fmt) "ACPI: IORT: " fmt >>>> + >>>> +#include <linux/iort.h> >>>> +#include <linux/kernel.h> >>>> +#include <linux/pci.h> >>>> + >>>> +typedef acpi_status (*iort_find_node_callback) >>>> + (struct acpi_iort_node *node, void *context); >>>> + >>>> +/* Root pointer to the mapped IORT table */ >>>> +static struct acpi_table_header *iort_table; >>>> + >>>> +static struct acpi_iort_node * >>>> +iort_scan_node(enum acpi_iort_node_type type, >>>> + iort_find_node_callback callback, void *context) >>>> +{ >>>> + struct acpi_iort_node *iort_node, *iort_end; >>>> + struct acpi_table_iort *iort; >>>> + int i; >>>> + >>>> + /* Get the first IORT node */ >>>> + iort = (struct acpi_table_iort *)iort_table; >>>> + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort, >>>> + iort->node_offset); >>>> + iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table, >>>> + iort_table->length); >>>> + >>>> + for (i = 0; i < iort->node_count; i++) { >>>> + if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND, >>>> + "IORT node pointer overflows, bad table!\n")) >>>> + return NULL; >>>> + >>>> + if (iort_node->type == type) { >>>> + if (ACPI_SUCCESS(callback(iort_node, context))) >>>> + return iort_node; >>>> + } >>>> + >>>> + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node, >>>> + iort_node->length); >>>> + } >>>> + >>>> + return NULL; >>>> +} >>>> + >>>> +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 }; >>>> + 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"); >>>> + } else { >>>> + int match; >>>> + >>>> + match = !strcmp(ncomp->device_name, buffer.pointer); >>>> + kfree(buffer.pointer); >>> Why did you change this to a naked kfree? The ACPI code clearly states: >>> >>> /* >>> * Allocate a new buffer. We directectly call acpi_os_allocate here to >>> * purposefully bypass the (optionally enabled) internal allocation >>> * tracking mechanism since we only want to track internal >>> * allocations. Note: The caller should use acpi_os_free to free this >>> * buffer created via ACPI_ALLOCATE_BUFFER. >>> */ >> Yes we should use symmetric free function to acpi_os_allocate here, >> which means acpi_os_free should be used here. >> >> My main motivation was to free buffer.pointer instead of &buffer It needs to be free buffer.pointer. >> >> Would you mind to fix that (s/kfree/acpi_os_free) before merging ? > Sure. I tracked Marc's tree on git.kernel.org, it's acpi_os_free(buffer.pointer) there so my worry is gone :) Thanks Hanjun
Hi Marc, On 06/22/2016 09:34 PM, Hanjun Guo wrote: > On 2016/6/22 22:51, Marc Zyngier wrote: >> On 22/06/16 14:52, Tomasz Nowicki wrote: >>> On 22.06.2016 15:25, Marc Zyngier wrote: >>>> On 22/06/16 13:35, 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. >>>>> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf >>>>> >>>>> 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 | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> include/linux/iort.h | 30 +++++++ >>>>> 5 files changed, 253 insertions(+) >>>>> create mode 100644 drivers/acpi/iort.c >>>>> create mode 100644 include/linux/iort.h > I tracked Marc's tree on git.kernel.org, it's acpi_os_free(buffer.pointer) there so > my worry is gone :) Do you plan on submitting this during the 4.8 merge window? Thanks, Cov
On Tue, 26 Jul 2016 09:19:15 -0400 Christopher Covington <cov@codeaurora.org> wrote: Hi Christopher, > Hi Marc, > > On 06/22/2016 09:34 PM, Hanjun Guo wrote: > > On 2016/6/22 22:51, Marc Zyngier wrote: > >> On 22/06/16 14:52, Tomasz Nowicki wrote: > >>> On 22.06.2016 15:25, Marc Zyngier wrote: > >>>> On 22/06/16 13:35, 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. > >>>>> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf > >>>>> > >>>>> 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 | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++ > >>>>> include/linux/iort.h | 30 +++++++ > >>>>> 5 files changed, 253 insertions(+) > >>>>> create mode 100644 drivers/acpi/iort.c > >>>>> create mode 100644 include/linux/iort.h > > > I tracked Marc's tree on git.kernel.org, it's acpi_os_free(buffer.pointer) there so > > my worry is gone :) > > Do you plan on submitting this during the 4.8 merge window? Short of having received the necessary Ack from the ACPI maintainer, the answer is unfortunately negative. I'm hoping that the new split ACPI maintenance between generic code and arm64 will help getting things moving for 4.9. Thanks, M.
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index b7e2e77..848471f 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT config ACPI_CCA_REQUIRED bool +config IORT_TABLE + bool + config ACPI_DEBUGGER bool "AML debugger interface" select ACPI_DEBUG diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 251ce85..c7c9b29 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o obj-$(CONFIG_ACPI_BGRT) += bgrt.o obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o +obj-$(CONFIG_IORT_TABLE) += iort.o # processor has its own "processor." module_param namespace processor-y := processor_driver.o diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 31e8da6..176c17d 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -33,6 +33,7 @@ #ifdef CONFIG_X86 #include <asm/mpspec.h> #endif +#include <linux/iort.h> #include <linux/pci.h> #include <acpi/apei.h> #include <linux/dmi.h> @@ -1118,6 +1119,7 @@ static int __init acpi_init(void) } pci_mmcfg_late_init(); + iort_table_detect(); acpi_scan_init(); acpi_ec_init(); acpi_debugfs_init(); diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c new file mode 100644 index 0000000..fcfa008f --- /dev/null +++ b/drivers/acpi/iort.c @@ -0,0 +1,217 @@ +/* + * Copyright (C) 2016, Semihalf + * Author: Tomasz Nowicki <tn@semihalf.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * This file implements early detection/parsing of I/O mapping + * reported to OS through firmware via I/O Remapping Table (IORT) + * IORT document number: ARM DEN 0049A + */ + +#define pr_fmt(fmt) "ACPI: IORT: " fmt + +#include <linux/iort.h> +#include <linux/kernel.h> +#include <linux/pci.h> + +typedef acpi_status (*iort_find_node_callback) + (struct acpi_iort_node *node, void *context); + +/* Root pointer to the mapped IORT table */ +static struct acpi_table_header *iort_table; + +static struct acpi_iort_node * +iort_scan_node(enum acpi_iort_node_type type, + iort_find_node_callback callback, void *context) +{ + struct acpi_iort_node *iort_node, *iort_end; + struct acpi_table_iort *iort; + int i; + + /* Get the first IORT node */ + iort = (struct acpi_table_iort *)iort_table; + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort, + iort->node_offset); + iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table, + iort_table->length); + + for (i = 0; i < iort->node_count; i++) { + if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND, + "IORT node pointer overflows, bad table!\n")) + return NULL; + + if (iort_node->type == type) { + if (ACPI_SUCCESS(callback(iort_node, context))) + return iort_node; + } + + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node, + iort_node->length); + } + + return NULL; +} + +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 }; + 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"); + } else { + int match; + + match = !strcmp(ncomp->device_name, buffer.pointer); + kfree(buffer.pointer); + + if (match) + return AE_OK; + } + + break; + } + case ACPI_IORT_NODE_PCI_ROOT_COMPLEX: { + struct acpi_iort_root_complex *pci_rc; + struct pci_bus *bus; + + bus = to_pci_bus(dev); + pci_rc = (struct acpi_iort_root_complex *)node->node_data; + + /* + * It is assumed that PCI segment numbers maps one-to-one + * with root complexes. Each segment number can represent only + * one root complex. + */ + if (pci_rc->pci_segment_number == pci_domain_nr(bus)) + return AE_OK; + + break; + } + } + + return AE_NOT_FOUND; +} + +static int +iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, 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; + 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; + } + + if (rid_in < map->input_base || + (rid_in > map->input_base + map->id_count)) + return -ENXIO; + + *rid_out = map->output_base + (rid_in - map->input_base); + return 0; +} + +static struct acpi_iort_node * +iort_node_map_rid(struct acpi_iort_node *node, u32 rid_in, + u32 *rid_out, u8 type) +{ + u32 rid = rid_in; + + /* Parse the ID mapping tree to find specified node type */ + while (node) { + struct acpi_iort_id_mapping *map; + int i; + + if (node->type == type) { + if (rid_out) + *rid_out = rid; + return node; + } + + if (!node->mapping_offset || !node->mapping_count) + goto fail_map; + + map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node, + node->mapping_offset); + + /* Firmware bug! */ + if (!map->output_reference) { + pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n", + node, node->type); + goto fail_map; + } + + /* Do the RID translation */ + for (i = 0; i < node->mapping_count; i++, map++) { + if(!iort_id_map(map, node->type, rid, &rid)) + break; + } + + if (i == node->mapping_count) + goto fail_map; + + node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table, + map->output_reference); + } + +fail_map: + /* Map input RID to output RID unchanged on mapping failure*/ + if (rid_out) + *rid_out = rid_in; + return NULL; +} + +static struct acpi_iort_node * +iort_find_dev_node(struct device *dev) +{ + struct pci_bus *pbus; + + if (!dev_is_pci(dev)) + 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; + while (!pci_is_root_bus(pbus)) + pbus = pbus->parent; + + return iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX, + iort_match_node_callback, &pbus->dev); +} + +void __init iort_table_detect(void) +{ + acpi_status status; + + status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table); + if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { + const char *msg = acpi_format_exception(status); + pr_err("Failed to get table, %s\n", msg); + } +} diff --git a/include/linux/iort.h b/include/linux/iort.h new file mode 100644 index 0000000..cde6809 --- /dev/null +++ b/include/linux/iort.h @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2016, Semihalf + * Author: Tomasz Nowicki <tn@semihalf.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple + * Place - Suite 330, Boston, MA 02111-1307 USA. + */ + +#ifndef __IORT_H__ +#define __IORT_H__ + +#include <linux/acpi.h> + +#ifdef CONFIG_IORT_TABLE +void iort_table_detect(void); +#else +static inline void iort_table_detect(void) { } +#endif + +#endif /* __IORT_H__ */
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. http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf 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 | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/iort.h | 30 +++++++ 5 files changed, 253 insertions(+) create mode 100644 drivers/acpi/iort.c create mode 100644 include/linux/iort.h