* [PATCH V2 0/3] ioapic hot-removal bugs @ 2016-06-08 6:59 Rui Wang 2016-06-08 6:59 ` [PATCH V2 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot Rui Wang ` (2 more replies) 0 siblings, 3 replies; 31+ messages in thread From: Rui Wang @ 2016-06-08 6:59 UTC (permalink / raw) To: tglx, rjw, tony.luck, bhelgaas Cc: linux-acpi, linux-pci, linux-kernel, rui.y.wang Hi Thomas, Here's the v2 patchset according to your suggestion. While testing ioapic hotplug, two bugs were found. 1) acpi_ioapic_add() is only called during hotadd of ioapics. Those already present during system boot are not added, and thus cannot be hot-removed. 2) ioapics[i].iomem_res were assigned the wrong pointers, causing panic while hot-removing ioapics. On a 4-socket brickland, hot-removal of the 3 sockets can be done only after applying the first two patches. The 3rd patch is optional. Regards, Rui v2: split the second patch into a one liner and a cleanup patch according to Thomas. Rui Wang (3): x86/ioapic: Support hot-removal of IOAPICs present during boot x86/ioapic: Fix wrong pointers in ioapic_setup_resources() x86/ioapic: Simplify ioapic_setup_resources() arch/x86/kernel/apic/io_apic.c | 18 +++++++----------- drivers/acpi/internal.h | 2 -- drivers/acpi/ioapic.c | 7 ++++--- drivers/acpi/pci_root.c | 2 +- drivers/pci/setup-bus.c | 5 ++++- include/linux/acpi.h | 3 +++ 6 files changed, 19 insertions(+), 18 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH V2 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-08 6:59 [PATCH V2 0/3] ioapic hot-removal bugs Rui Wang @ 2016-06-08 6:59 ` Rui Wang 2016-06-08 8:05 ` kbuild test robot 2016-06-08 9:32 ` [PATCH V3 " Rui Wang 2016-06-08 6:59 ` [PATCH V2 2/3] x86/ioapic: Fix wrong pointers in ioapic_setup_resources() Rui Wang 2016-06-08 6:59 ` [PATCH V2 3/3] x86/ioapic: Simplify ioapic_setup_resources() Rui Wang 2 siblings, 2 replies; 31+ messages in thread From: Rui Wang @ 2016-06-08 6:59 UTC (permalink / raw) To: tglx, rjw, tony.luck, bhelgaas Cc: linux-acpi, linux-pci, linux-kernel, rui.y.wang IOAPICs present during system boot aren't added to ioapic_list, thus are unable to be hot-removed. Fix it by calling acpi_ioapic_add() during root bus enumeration. Signed-off-by: Rui Wang <rui.y.wang@intel.com> --- drivers/acpi/internal.h | 2 -- drivers/acpi/ioapic.c | 7 ++++--- drivers/acpi/pci_root.c | 2 +- drivers/pci/setup-bus.c | 5 ++++- include/linux/acpi.h | 3 +++ 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 9bb0773..bb567a7 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -40,10 +40,8 @@ int acpi_sysfs_init(void); void acpi_container_init(void); void acpi_memory_hotplug_init(void); #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC -int acpi_ioapic_add(struct acpi_pci_root *root); int acpi_ioapic_remove(struct acpi_pci_root *root); #else -static inline int acpi_ioapic_add(struct acpi_pci_root *root) { return 0; } static inline int acpi_ioapic_remove(struct acpi_pci_root *root) { return 0; } #endif #ifdef CONFIG_ACPI_DOCK diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c index ccdc8db..0f272e2 100644 --- a/drivers/acpi/ioapic.c +++ b/drivers/acpi/ioapic.c @@ -189,16 +189,17 @@ exit: return AE_OK; } -int acpi_ioapic_add(struct acpi_pci_root *root) +int acpi_ioapic_add(acpi_handle root_handle) { acpi_status status, retval = AE_OK; - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root->device->handle, + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root_handle, UINT_MAX, handle_ioapic_add, NULL, - root->device->handle, (void **)&retval); + root_handle, (void **)&retval); return ACPI_SUCCESS(status) && ACPI_SUCCESS(retval) ? 0 : -ENODEV; } +EXPORT_SYMBOL_GPL(acpi_ioapic_add); int acpi_ioapic_remove(struct acpi_pci_root *root) { diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index ae3fe4e..53f5965 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -614,7 +614,7 @@ static int acpi_pci_root_add(struct acpi_device *device, if (hotadd) { pcibios_resource_survey_bus(root->bus); pci_assign_unassigned_root_bus_resources(root->bus); - acpi_ioapic_add(root); + acpi_ioapic_add(root->device->handle); } pci_lock_rescan_remove(); diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 55641a3..e32c356 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -25,6 +25,7 @@ #include <linux/ioport.h> #include <linux/cache.h> #include <linux/slab.h> +#include <linux/acpi.h> #include "pci.h" unsigned int pci_flags; @@ -1779,8 +1780,10 @@ void __init pci_assign_unassigned_resources(void) { struct pci_bus *root_bus; - list_for_each_entry(root_bus, &pci_root_buses, node) + list_for_each_entry(root_bus, &pci_root_buses, node) { pci_assign_unassigned_root_bus_resources(root_bus); + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); + } } void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 288fac5..3ed22df 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -262,6 +262,9 @@ int acpi_unmap_cpu(int cpu); #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr); +int acpi_ioapic_add(acpi_handle root); +#else +static inline int acpi_ioapic_add(acpi_handle root) { return 0; } #endif int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH V2 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-08 6:59 ` [PATCH V2 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot Rui Wang @ 2016-06-08 8:05 ` kbuild test robot 2016-06-08 9:32 ` [PATCH V3 " Rui Wang 1 sibling, 0 replies; 31+ messages in thread From: kbuild test robot @ 2016-06-08 8:05 UTC (permalink / raw) Cc: kbuild-all, tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang [-- Attachment #1: Type: text/plain, Size: 1656 bytes --] Hi, [auto build test ERROR on pm/linux-next] [also build test ERROR on v4.7-rc2 next-20160608] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Rui-Wang/x86-ioapic-Support-hot-removal-of-IOAPICs-present-during-boot/20160608-151907 base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next config: mips-txx9 (attached as .config) compiler: mips-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=mips All errors (new ones prefixed by >>): drivers/pci/setup-bus.c: In function 'pci_assign_unassigned_resources': >> drivers/pci/setup-bus.c:1785:3: error: implicit declaration of function 'acpi_ioapic_add' [-Werror=implicit-function-declaration] acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); ^ cc1: some warnings being treated as errors vim +/acpi_ioapic_add +1785 drivers/pci/setup-bus.c 1779 void __init pci_assign_unassigned_resources(void) 1780 { 1781 struct pci_bus *root_bus; 1782 1783 list_for_each_entry(root_bus, &pci_root_buses, node) { 1784 pci_assign_unassigned_root_bus_resources(root_bus); > 1785 acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); 1786 } 1787 } 1788 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/octet-stream, Size: 22126 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V2 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot @ 2016-06-08 8:05 ` kbuild test robot 0 siblings, 0 replies; 31+ messages in thread From: kbuild test robot @ 2016-06-08 8:05 UTC (permalink / raw) To: Rui Wang Cc: kbuild-all, tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang [-- Attachment #1: Type: text/plain, Size: 1656 bytes --] Hi, [auto build test ERROR on pm/linux-next] [also build test ERROR on v4.7-rc2 next-20160608] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Rui-Wang/x86-ioapic-Support-hot-removal-of-IOAPICs-present-during-boot/20160608-151907 base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next config: mips-txx9 (attached as .config) compiler: mips-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=mips All errors (new ones prefixed by >>): drivers/pci/setup-bus.c: In function 'pci_assign_unassigned_resources': >> drivers/pci/setup-bus.c:1785:3: error: implicit declaration of function 'acpi_ioapic_add' [-Werror=implicit-function-declaration] acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); ^ cc1: some warnings being treated as errors vim +/acpi_ioapic_add +1785 drivers/pci/setup-bus.c 1779 void __init pci_assign_unassigned_resources(void) 1780 { 1781 struct pci_bus *root_bus; 1782 1783 list_for_each_entry(root_bus, &pci_root_buses, node) { 1784 pci_assign_unassigned_root_bus_resources(root_bus); > 1785 acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); 1786 } 1787 } 1788 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/octet-stream, Size: 22126 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH V3 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-08 6:59 ` [PATCH V2 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot Rui Wang 2016-06-08 8:05 ` kbuild test robot @ 2016-06-08 9:32 ` Rui Wang 2016-06-10 12:57 ` Thomas Gleixner 2016-06-10 16:43 ` Bjorn Helgaas 1 sibling, 2 replies; 31+ messages in thread From: Rui Wang @ 2016-06-08 9:32 UTC (permalink / raw) To: tglx, rjw, tony.luck, bhelgaas Cc: linux-acpi, linux-pci, linux-kernel, rui.y.wang v3: Previous versions break mips. This version fixes it. IOAPICs present during system boot aren't added to ioapic_list, thus are unable to be hot-removed. Fix it by calling acpi_ioapic_add() during root bus enumeration. Signed-off-by: Rui Wang <rui.y.wang@intel.com> --- drivers/acpi/internal.h | 2 -- drivers/acpi/ioapic.c | 7 ++++--- drivers/acpi/pci_root.c | 2 +- drivers/pci/setup-bus.c | 7 ++++++- include/linux/acpi.h | 3 +++ 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 9bb0773..bb567a7 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -40,10 +40,8 @@ int acpi_sysfs_init(void); void acpi_container_init(void); void acpi_memory_hotplug_init(void); #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC -int acpi_ioapic_add(struct acpi_pci_root *root); int acpi_ioapic_remove(struct acpi_pci_root *root); #else -static inline int acpi_ioapic_add(struct acpi_pci_root *root) { return 0; } static inline int acpi_ioapic_remove(struct acpi_pci_root *root) { return 0; } #endif #ifdef CONFIG_ACPI_DOCK diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c index ccdc8db..0f272e2 100644 --- a/drivers/acpi/ioapic.c +++ b/drivers/acpi/ioapic.c @@ -189,16 +189,17 @@ exit: return AE_OK; } -int acpi_ioapic_add(struct acpi_pci_root *root) +int acpi_ioapic_add(acpi_handle root_handle) { acpi_status status, retval = AE_OK; - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root->device->handle, + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root_handle, UINT_MAX, handle_ioapic_add, NULL, - root->device->handle, (void **)&retval); + root_handle, (void **)&retval); return ACPI_SUCCESS(status) && ACPI_SUCCESS(retval) ? 0 : -ENODEV; } +EXPORT_SYMBOL_GPL(acpi_ioapic_add); int acpi_ioapic_remove(struct acpi_pci_root *root) { diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index ae3fe4e..53f5965 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -614,7 +614,7 @@ static int acpi_pci_root_add(struct acpi_device *device, if (hotadd) { pcibios_resource_survey_bus(root->bus); pci_assign_unassigned_root_bus_resources(root->bus); - acpi_ioapic_add(root); + acpi_ioapic_add(root->device->handle); } pci_lock_rescan_remove(); diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 55641a3..0658921 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -25,6 +25,7 @@ #include <linux/ioport.h> #include <linux/cache.h> #include <linux/slab.h> +#include <linux/acpi.h> #include "pci.h" unsigned int pci_flags; @@ -1779,8 +1780,12 @@ void __init pci_assign_unassigned_resources(void) { struct pci_bus *root_bus; - list_for_each_entry(root_bus, &pci_root_buses, node) + list_for_each_entry(root_bus, &pci_root_buses, node) { pci_assign_unassigned_root_bus_resources(root_bus); +#ifdef CONFIG_X86 + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); +#endif + } } void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 288fac5..3ed22df 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -262,6 +262,9 @@ int acpi_unmap_cpu(int cpu); #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr); +int acpi_ioapic_add(acpi_handle root); +#else +static inline int acpi_ioapic_add(acpi_handle root) { return 0; } #endif int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH V3 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-08 9:32 ` [PATCH V3 " Rui Wang @ 2016-06-10 12:57 ` Thomas Gleixner 2016-06-10 13:56 ` Rafael J. Wysocki 2016-06-10 16:43 ` Bjorn Helgaas 1 sibling, 1 reply; 31+ messages in thread From: Thomas Gleixner @ 2016-06-10 12:57 UTC (permalink / raw) To: Rui Wang; +Cc: rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel On Wed, 8 Jun 2016, Rui Wang wrote: > v3: Previous versions break mips. This version fixes it. > > IOAPICs present during system boot aren't added to ioapic_list, > thus are unable to be hot-removed. Fix it by calling > acpi_ioapic_add() during root bus enumeration. Who will pick that one up? I already took 2/3 of this series as they are independent. Thanks, tglx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V3 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-10 12:57 ` Thomas Gleixner @ 2016-06-10 13:56 ` Rafael J. Wysocki 0 siblings, 0 replies; 31+ messages in thread From: Rafael J. Wysocki @ 2016-06-10 13:56 UTC (permalink / raw) To: Thomas Gleixner Cc: Rui Wang, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel On Friday, June 10, 2016 02:57:52 PM Thomas Gleixner wrote: > On Wed, 8 Jun 2016, Rui Wang wrote: > > > v3: Previous versions break mips. This version fixes it. > > > > IOAPICs present during system boot aren't added to ioapic_list, > > thus are unable to be hot-removed. Fix it by calling > > acpi_ioapic_add() during root bus enumeration. > > Who will pick that one up? I already took 2/3 of this series as they are > independent. I can pick it up. Thanks, Rafael ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V3 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-08 9:32 ` [PATCH V3 " Rui Wang 2016-06-10 12:57 ` Thomas Gleixner @ 2016-06-10 16:43 ` Bjorn Helgaas 2016-06-12 6:06 ` Rui Wang 1 sibling, 1 reply; 31+ messages in thread From: Bjorn Helgaas @ 2016-06-10 16:43 UTC (permalink / raw) To: Rui Wang Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel On Wed, Jun 08, 2016 at 05:32:44PM +0800, Rui Wang wrote: > v3: Previous versions break mips. This version fixes it. > > IOAPICs present during system boot aren't added to ioapic_list, > thus are unable to be hot-removed. Fix it by calling > acpi_ioapic_add() during root bus enumeration. > > Signed-off-by: Rui Wang <rui.y.wang@intel.com> > --- > drivers/acpi/internal.h | 2 -- > drivers/acpi/ioapic.c | 7 ++++--- > drivers/acpi/pci_root.c | 2 +- > drivers/pci/setup-bus.c | 7 ++++++- > include/linux/acpi.h | 3 +++ > 5 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > index 9bb0773..bb567a7 100644 > --- a/drivers/acpi/internal.h > +++ b/drivers/acpi/internal.h > @@ -40,10 +40,8 @@ int acpi_sysfs_init(void); > void acpi_container_init(void); > void acpi_memory_hotplug_init(void); > #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC > -int acpi_ioapic_add(struct acpi_pci_root *root); > int acpi_ioapic_remove(struct acpi_pci_root *root); > #else > -static inline int acpi_ioapic_add(struct acpi_pci_root *root) { return 0; } > static inline int acpi_ioapic_remove(struct acpi_pci_root *root) { return 0; } > #endif > #ifdef CONFIG_ACPI_DOCK > diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c > index ccdc8db..0f272e2 100644 > --- a/drivers/acpi/ioapic.c > +++ b/drivers/acpi/ioapic.c > @@ -189,16 +189,17 @@ exit: > return AE_OK; > } > > -int acpi_ioapic_add(struct acpi_pci_root *root) > +int acpi_ioapic_add(acpi_handle root_handle) > { > acpi_status status, retval = AE_OK; > > - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root->device->handle, > + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root_handle, > UINT_MAX, handle_ioapic_add, NULL, > - root->device->handle, (void **)&retval); > + root_handle, (void **)&retval); > > return ACPI_SUCCESS(status) && ACPI_SUCCESS(retval) ? 0 : -ENODEV; > } > +EXPORT_SYMBOL_GPL(acpi_ioapic_add); > > int acpi_ioapic_remove(struct acpi_pci_root *root) > { > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index ae3fe4e..53f5965 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -614,7 +614,7 @@ static int acpi_pci_root_add(struct acpi_device *device, > if (hotadd) { > pcibios_resource_survey_bus(root->bus); > pci_assign_unassigned_root_bus_resources(root->bus); > - acpi_ioapic_add(root); > + acpi_ioapic_add(root->device->handle); > } > > pci_lock_rescan_remove(); > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 55641a3..0658921 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -25,6 +25,7 @@ > #include <linux/ioport.h> > #include <linux/cache.h> > #include <linux/slab.h> > +#include <linux/acpi.h> > #include "pci.h" > > unsigned int pci_flags; > @@ -1779,8 +1780,12 @@ void __init pci_assign_unassigned_resources(void) > { > struct pci_bus *root_bus; > > - list_for_each_entry(root_bus, &pci_root_buses, node) > + list_for_each_entry(root_bus, &pci_root_buses, node) { > pci_assign_unassigned_root_bus_resources(root_bus); > +#ifdef CONFIG_X86 > + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); > +#endif This seems like a strange place to call acpi_ioapic_add(). Your object is to call acpi_ioapic_add() during root bus enumeration. I assume we *can't* call acpi_ioapic_add() from acpi_pci_root_add() at boot time, for some reason you'll explain. But is there a reason we have to call it from pci_assign_unassigned_resources() (where it requires an ifdef) instead of from pcibios_assign_resources(), which is already x86-specific? In acpi_pci_root_add(), we have this: acpi_pci_root_add(...) { ... if (hotadd) acpi_ioapic_add(root); So the obvious question is why don't we just remove the "if (hotadd)" and call acpi_ioapic_add() always. I'm sure the reason is some ordering problem, but we need a comment in acpi_pci_root_add() about why the obvious solution doesn't work. > + } > } > > void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge) > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 288fac5..3ed22df 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -262,6 +262,9 @@ int acpi_unmap_cpu(int cpu); > > #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC > int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr); > +int acpi_ioapic_add(acpi_handle root); > +#else > +static inline int acpi_ioapic_add(acpi_handle root) { return 0; } > #endif > > int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base); > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V3 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-10 16:43 ` Bjorn Helgaas @ 2016-06-12 6:06 ` Rui Wang 2016-06-16 17:09 ` Bjorn Helgaas 0 siblings, 1 reply; 31+ messages in thread From: Rui Wang @ 2016-06-12 6:06 UTC (permalink / raw) To: helgaas Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang On Saturday, June 11, 2016 12:43 AM, Bjorn Helgaas wrote: > On Wed, Jun 08, 2016 at 05:32:44PM +0800, Rui Wang wrote: > > @@ -1779,8 +1780,12 @@ void __init > > pci_assign_unassigned_resources(void) > > { > > struct pci_bus *root_bus; > > > > - list_for_each_entry(root_bus, &pci_root_buses, node) > > + list_for_each_entry(root_bus, &pci_root_buses, node) { > > pci_assign_unassigned_root_bus_resources(root_bus); > > +#ifdef CONFIG_X86 > > + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); > > +#endif > > This seems like a strange place to call acpi_ioapic_add(). Your object is to call > acpi_ioapic_add() during root bus enumeration. > > I assume we *can't* call acpi_ioapic_add() from acpi_pci_root_add() at boot > time, for some reason you'll explain. But is there a reason we have to call it > from pci_assign_unassigned_resources() (where it requires an ifdef) instead > of from pcibios_assign_resources(), which is already x86-specific? > > In acpi_pci_root_add(), we have this: > > acpi_pci_root_add(...) > { > ... > if (hotadd) > acpi_ioapic_add(root); > > So the obvious question is why don't we just remove the "if (hotadd)" > and call acpi_ioapic_add() always. > > I'm sure the reason is some ordering problem, but we need a comment in > acpi_pci_root_add() about why the obvious solution doesn't work. > Hi Bjorn, Yes it's an ording issue. acpi_ioapic_add() and also ioapic_insert_resources() have to be later than pci initialization in order to deal with IOAPICs mapped on a PCI BAR. There's a comment about this inside pcibios_resource_survey() above ioapic_insert_resources(). We can also add a comment inside acpi_pci_root_add(), though. And yes calling acpi_ioapic_add() in pcibios_assign_resources() doesn't require ifdef CONFIG_X86. But it'll require a loop to iterate through the root buses, and call acpi_ioapic_add() within the loop. pci_assign_unassigned_resources() already has that loop. Do you still prefer adding it to pcibios_assign_resources() ? Regards, Rui ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V3 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-12 6:06 ` Rui Wang @ 2016-06-16 17:09 ` Bjorn Helgaas 2016-06-22 7:13 ` Rui Wang 2016-06-22 7:40 ` [PATCH V4 " Rui Wang 0 siblings, 2 replies; 31+ messages in thread From: Bjorn Helgaas @ 2016-06-16 17:09 UTC (permalink / raw) To: Rui Wang Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel On Sun, Jun 12, 2016 at 02:06:09PM +0800, Rui Wang wrote: > On Saturday, June 11, 2016 12:43 AM, Bjorn Helgaas wrote: > > On Wed, Jun 08, 2016 at 05:32:44PM +0800, Rui Wang wrote: > > > @@ -1779,8 +1780,12 @@ void __init > > > pci_assign_unassigned_resources(void) > > > { > > > struct pci_bus *root_bus; > > > > > > - list_for_each_entry(root_bus, &pci_root_buses, node) > > > + list_for_each_entry(root_bus, &pci_root_buses, node) { > > > pci_assign_unassigned_root_bus_resources(root_bus); > > > +#ifdef CONFIG_X86 > > > + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); > > > +#endif > > > > This seems like a strange place to call acpi_ioapic_add(). Your object is to call > > acpi_ioapic_add() during root bus enumeration. > > > > I assume we *can't* call acpi_ioapic_add() from acpi_pci_root_add() at boot > > time, for some reason you'll explain. But is there a reason we have to call it > > from pci_assign_unassigned_resources() (where it requires an ifdef) instead > > of from pcibios_assign_resources(), which is already x86-specific? > > > > In acpi_pci_root_add(), we have this: > > > > acpi_pci_root_add(...) > > { > > ... > > if (hotadd) > > acpi_ioapic_add(root); > > > > So the obvious question is why don't we just remove the "if (hotadd)" > > and call acpi_ioapic_add() always. > > > > I'm sure the reason is some ordering problem, but we need a comment in > > acpi_pci_root_add() about why the obvious solution doesn't work. > > Yes it's an ording issue. acpi_ioapic_add() and also ioapic_insert_resources() > have to be later than pci initialization in order to deal with IOAPICs mapped > on a PCI BAR. There's a comment about this inside pcibios_resource_survey() > above ioapic_insert_resources(). We can also add a comment inside > acpi_pci_root_add(), though. > > And yes calling acpi_ioapic_add() in pcibios_assign_resources() doesn't require > ifdef CONFIG_X86. But it'll require a loop to iterate through the root buses, > and call acpi_ioapic_add() within the loop. pci_assign_unassigned_resources() > already has that loop. Do you still prefer adding it to > pcibios_assign_resources() ? ioapic_insert_resources() is x86-specific, but I'm not sure why; it seems like it does things that should be applicable to ia64 as well. acpi_ioapic_add() is not x86-specific, and it is called from acpi_pci_root_add() for the hot-add case. You're adding an x86-xpecific call in pci_assign_unassigned_resources(). Why should the hot-add case be for all arches, but the boot-time case only for x86? ^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH V3 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-16 17:09 ` Bjorn Helgaas @ 2016-06-22 7:13 ` Rui Wang 2016-06-22 14:53 ` Bjorn Helgaas 2016-06-22 7:40 ` [PATCH V4 " Rui Wang 1 sibling, 1 reply; 31+ messages in thread From: Rui Wang @ 2016-06-22 7:13 UTC (permalink / raw) To: helgaas Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang On Friday, June 17, 2016 1:10 AM, Bjorn Helgaas wrote: > ioapic_insert_resources() is x86-specific, but I'm not sure why; it seems > like it does things that should be applicable to ia64 as well. > > acpi_ioapic_add() is not x86-specific, and it is called from > acpi_pci_root_add() for the hot-add case. You're adding an x86-xpecific call > in pci_assign_unassigned_resources(). Why should the hot-add case be for > all arches, but the boot-time case only for x86? Hi Bjorn, It turns out that IOAPIC hotplug has not been pursued on ia64. There were demos showing CPU sockets online/offline on ia64 but the CPUs had no IIO, thus no IOAPIC hotplug. So to answer the first question: ioapic_insert_resources() is x86-specific because it's inserting what has been setup in io_apic_init_mappings() which, through mpc_ioapic_addr(), is capable of handling both the static case (acpi_parse_ioapic(), etc.) and the hotplug case (acpi_ioapic_add()). But on ia64, there's only the static case through acpi_parse_iosapic(), no need for the hotplug case yet. To answer the second question: acpi_ioapic_add() is in effect x86-specific, because it's an empty function when CONFIG_ACPI_HOTPLUG_IOAPIC isn't defined. And CONFIG_ACPI_HOTPLUG_IOAPIC depends on CONFIG_X86_IO_APIC (see drivers/acpi/Kconfig). This was introduced in c183619b6 (x86/irq, ACPI: Implement ACPI driver to support IOAPIC hotplug). That commit shows the dependency. I'll send a newer version with comments explaining these. Thanks Rui ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V3 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-22 7:13 ` Rui Wang @ 2016-06-22 14:53 ` Bjorn Helgaas 2016-06-24 15:18 ` Rui Wang 0 siblings, 1 reply; 31+ messages in thread From: Bjorn Helgaas @ 2016-06-22 14:53 UTC (permalink / raw) To: Rui Wang Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel On Wed, Jun 22, 2016 at 03:13:32PM +0800, Rui Wang wrote: > On Friday, June 17, 2016 1:10 AM, Bjorn Helgaas wrote: > > ioapic_insert_resources() is x86-specific, but I'm not sure why; it seems > > like it does things that should be applicable to ia64 as well. > > > > acpi_ioapic_add() is not x86-specific, and it is called from > > acpi_pci_root_add() for the hot-add case. You're adding an x86-xpecific call > > in pci_assign_unassigned_resources(). Why should the hot-add case be for > > all arches, but the boot-time case only for x86? > > Hi Bjorn, > > It turns out that IOAPIC hotplug has not been pursued on ia64. There were > demos showing CPU sockets online/offline on ia64 but the CPUs had no IIO, > thus no IOAPIC hotplug. That doesn't mean we need to write code that's gratuitously x86-specific. > So to answer the first question: > ioapic_insert_resources() is x86-specific because it's inserting what has > been setup in io_apic_init_mappings() which, through mpc_ioapic_addr(), is > capable of handling both the static case (acpi_parse_ioapic(), etc.) and the > hotplug case (acpi_ioapic_add()). But on ia64, there's only the static > case through acpi_parse_iosapic(), no need for the hotplug case yet. ioapic_insert_resources() inserts IOAPIC resources on x86. Where are IOSAPIC resources inserted on ia64? > To answer the second question: > acpi_ioapic_add() is in effect x86-specific, because it's an empty function > when CONFIG_ACPI_HOTPLUG_IOAPIC isn't defined. And CONFIG_ACPI_HOTPLUG_IOAPIC > depends on CONFIG_X86_IO_APIC (see drivers/acpi/Kconfig). This was introduced > in c183619b6 (x86/irq, ACPI: Implement ACPI driver to support IOAPIC hotplug). > That commit shows the dependency. > > I'll send a newer version with comments explaining these. I'd rather have code that is not x86-specific than comments explaining why the code is x86-specific. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V3 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-22 14:53 ` Bjorn Helgaas @ 2016-06-24 15:18 ` Rui Wang 0 siblings, 0 replies; 31+ messages in thread From: Rui Wang @ 2016-06-24 15:18 UTC (permalink / raw) To: helgaas Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang On Wed, June 22, 2016 10:54 PM Bjorn Helgaas wrote: > On Wed, Jun 22, 2016 at 03:13:32PM +0800, Rui Wang wrote: > > On Friday, June 17, 2016 1:10 AM, Bjorn Helgaas wrote: > > > ioapic_insert_resources() is x86-specific, but I'm not sure why; it > > > seems like it does things that should be applicable to ia64 as well. > > > > > > acpi_ioapic_add() is not x86-specific, and it is called from > > > acpi_pci_root_add() for the hot-add case. You're adding an > > > x86-xpecific call in pci_assign_unassigned_resources(). Why should > > > the hot-add case be for all arches, but the boot-time case only for x86? > > > > Hi Bjorn, > > > > It turns out that IOAPIC hotplug has not been pursued on ia64. There > > were demos showing CPU sockets online/offline on ia64 but the CPUs had > > no IIO, thus no IOAPIC hotplug. > > That doesn't mean we need to write code that's gratuitously x86-specific. > > > So to answer the first question: > > ioapic_insert_resources() is x86-specific because it's inserting what > > has been setup in io_apic_init_mappings() which, through > > mpc_ioapic_addr(), is capable of handling both the static case > > (acpi_parse_ioapic(), etc.) and the hotplug case (acpi_ioapic_add()). > > But on ia64, there's only the static case through acpi_parse_iosapic(), no > need for the hotplug case yet. > > ioapic_insert_resources() inserts IOAPIC resources on x86. Where are > IOSAPIC resources inserted on ia64? > I'm not sure. Its physical address from MADT is parsed in acpi_parse_iosapic() and ioremap()'d in iosapic_init(). Only the virtual address is recorded. The physical address doesn't seem to be recorded and inserted explicitly. But it could be in a "_CRS" and gets inserted implicitly. The iosapic on ia64 uses its own data structures very different from the ioapic on x86. I think it's hard to unify them using a common set of functions, without rewriting the whole framework. Thanks Rui ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH V4 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-16 17:09 ` Bjorn Helgaas 2016-06-22 7:13 ` Rui Wang @ 2016-06-22 7:40 ` Rui Wang 2016-06-22 15:14 ` Bjorn Helgaas 1 sibling, 1 reply; 31+ messages in thread From: Rui Wang @ 2016-06-22 7:40 UTC (permalink / raw) To: helgaas Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang v4: Add comments explaining when to call acpi_ioapic_add(). v3: Previous versions break mips. This version fixes it. IOAPICs present during system boot aren't added to ioapic_list, thus are unable to be hot-removed. Fix it by calling acpi_ioapic_add() during root bus enumeration. Signed-off-by: Rui Wang <rui.y.wang@intel.com> --- drivers/acpi/internal.h | 2 -- drivers/acpi/ioapic.c | 7 ++++--- drivers/acpi/pci_root.c | 13 ++++++++++++- drivers/pci/setup-bus.c | 7 ++++++- include/linux/acpi.h | 3 +++ 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 27cc7fe..6d8e67e 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -40,10 +40,8 @@ int acpi_sysfs_init(void); void acpi_container_init(void); void acpi_memory_hotplug_init(void); #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC -int acpi_ioapic_add(struct acpi_pci_root *root); int acpi_ioapic_remove(struct acpi_pci_root *root); #else -static inline int acpi_ioapic_add(struct acpi_pci_root *root) { return 0; } static inline int acpi_ioapic_remove(struct acpi_pci_root *root) { return 0; } #endif #ifdef CONFIG_ACPI_DOCK diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c index ccdc8db..0f272e2 100644 --- a/drivers/acpi/ioapic.c +++ b/drivers/acpi/ioapic.c @@ -189,16 +189,17 @@ exit: return AE_OK; } -int acpi_ioapic_add(struct acpi_pci_root *root) +int acpi_ioapic_add(acpi_handle root_handle) { acpi_status status, retval = AE_OK; - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root->device->handle, + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root_handle, UINT_MAX, handle_ioapic_add, NULL, - root->device->handle, (void **)&retval); + root_handle, (void **)&retval); return ACPI_SUCCESS(status) && ACPI_SUCCESS(retval) ? 0 : -ENODEV; } +EXPORT_SYMBOL_GPL(acpi_ioapic_add); int acpi_ioapic_remove(struct acpi_pci_root *root) { diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index ae3fe4e..31e4440 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -614,7 +614,18 @@ static int acpi_pci_root_add(struct acpi_device *device, if (hotadd) { pcibios_resource_survey_bus(root->bus); pci_assign_unassigned_root_bus_resources(root->bus); - acpi_ioapic_add(root); + + /* + * This is only called for the hotadd case. For the boot-time + * case, we need to wait until after PCI initialization in + * order to deal with IOAPICs mapped in on a PCI BAR. + * + * This is currently x86-specific, because acpi_ioapic_add() + * is an empty function without CONFIG_ACPI_HOTPLUG_IOAPIC. + * And CONFIG_ACPI_HOTPLUG_IOAPIC depends on CONFIG_X86_IO_APIC + * (see drivers/acpi/Kconfig). + */ + acpi_ioapic_add(root->device->handle); } pci_lock_rescan_remove(); diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 55641a3..0658921 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -25,6 +25,7 @@ #include <linux/ioport.h> #include <linux/cache.h> #include <linux/slab.h> +#include <linux/acpi.h> #include "pci.h" unsigned int pci_flags; @@ -1779,8 +1780,12 @@ void __init pci_assign_unassigned_resources(void) { struct pci_bus *root_bus; - list_for_each_entry(root_bus, &pci_root_buses, node) + list_for_each_entry(root_bus, &pci_root_buses, node) { pci_assign_unassigned_root_bus_resources(root_bus); +#ifdef CONFIG_X86 + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); +#endif + } } void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 288fac5..3ed22df 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -262,6 +262,9 @@ int acpi_unmap_cpu(int cpu); #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr); +int acpi_ioapic_add(acpi_handle root); +#else +static inline int acpi_ioapic_add(acpi_handle root) { return 0; } #endif int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH V4 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-22 7:40 ` [PATCH V4 " Rui Wang @ 2016-06-22 15:14 ` Bjorn Helgaas 2016-06-23 5:11 ` 0 siblings, 1 reply; 31+ messages in thread From: Bjorn Helgaas @ 2016-06-22 15:14 UTC (permalink / raw) To: Rui Wang Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel On Wed, Jun 22, 2016 at 03:40:19PM +0800, Rui Wang wrote: > v4: Add comments explaining when to call acpi_ioapic_add(). > v3: Previous versions break mips. This version fixes it. > > IOAPICs present during system boot aren't added to ioapic_list, > thus are unable to be hot-removed. Fix it by calling > acpi_ioapic_add() during root bus enumeration. > > Signed-off-by: Rui Wang <rui.y.wang@intel.com> > --- > drivers/acpi/internal.h | 2 -- > drivers/acpi/ioapic.c | 7 ++++--- > drivers/acpi/pci_root.c | 13 ++++++++++++- > drivers/pci/setup-bus.c | 7 ++++++- > include/linux/acpi.h | 3 +++ > 5 files changed, 25 insertions(+), 7 deletions(-) > > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > index 27cc7fe..6d8e67e 100644 > --- a/drivers/acpi/internal.h > +++ b/drivers/acpi/internal.h > @@ -40,10 +40,8 @@ int acpi_sysfs_init(void); > void acpi_container_init(void); > void acpi_memory_hotplug_init(void); > #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC > -int acpi_ioapic_add(struct acpi_pci_root *root); > int acpi_ioapic_remove(struct acpi_pci_root *root); > #else > -static inline int acpi_ioapic_add(struct acpi_pci_root *root) { return 0; } > static inline int acpi_ioapic_remove(struct acpi_pci_root *root) { return 0; } > #endif > #ifdef CONFIG_ACPI_DOCK > diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c > index ccdc8db..0f272e2 100644 > --- a/drivers/acpi/ioapic.c > +++ b/drivers/acpi/ioapic.c > @@ -189,16 +189,17 @@ exit: > return AE_OK; > } > > -int acpi_ioapic_add(struct acpi_pci_root *root) > +int acpi_ioapic_add(acpi_handle root_handle) > { > acpi_status status, retval = AE_OK; > > - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root->device->handle, > + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root_handle, > UINT_MAX, handle_ioapic_add, NULL, > - root->device->handle, (void **)&retval); > + root_handle, (void **)&retval); > > return ACPI_SUCCESS(status) && ACPI_SUCCESS(retval) ? 0 : -ENODEV; > } > +EXPORT_SYMBOL_GPL(acpi_ioapic_add); > > int acpi_ioapic_remove(struct acpi_pci_root *root) > { > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index ae3fe4e..31e4440 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -614,7 +614,18 @@ static int acpi_pci_root_add(struct acpi_device *device, > if (hotadd) { > pcibios_resource_survey_bus(root->bus); > pci_assign_unassigned_root_bus_resources(root->bus); > - acpi_ioapic_add(root); > + > + /* > + * This is only called for the hotadd case. For the boot-time > + * case, we need to wait until after PCI initialization in > + * order to deal with IOAPICs mapped in on a PCI BAR. > + * > + * This is currently x86-specific, because acpi_ioapic_add() > + * is an empty function without CONFIG_ACPI_HOTPLUG_IOAPIC. > + * And CONFIG_ACPI_HOTPLUG_IOAPIC depends on CONFIG_X86_IO_APIC > + * (see drivers/acpi/Kconfig). > + */ > + acpi_ioapic_add(root->device->handle); > } > > pci_lock_rescan_remove(); > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 55641a3..0658921 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -25,6 +25,7 @@ > #include <linux/ioport.h> > #include <linux/cache.h> > #include <linux/slab.h> > +#include <linux/acpi.h> > #include "pci.h" > > unsigned int pci_flags; > @@ -1779,8 +1780,12 @@ void __init pci_assign_unassigned_resources(void) > { > struct pci_bus *root_bus; > > - list_for_each_entry(root_bus, &pci_root_buses, node) > + list_for_each_entry(root_bus, &pci_root_buses, node) { > pci_assign_unassigned_root_bus_resources(root_bus); > +#ifdef CONFIG_X86 > + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); > +#endif Doesn't this do the right thing even if you omit the #ifdefs, since you define a stub function below? > + } > } > > void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge) > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 288fac5..3ed22df 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -262,6 +262,9 @@ int acpi_unmap_cpu(int cpu); > > #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC > int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr); > +int acpi_ioapic_add(acpi_handle root); > +#else > +static inline int acpi_ioapic_add(acpi_handle root) { return 0; } > #endif > > int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base); > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V4 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-22 15:14 ` Bjorn Helgaas 2016-06-23 5:11 ` @ 2016-06-23 5:11 ` 0 siblings, 0 replies; 31+ messages in thread From: Unknown, @ 2016-06-23 5:11 UTC (permalink / raw) To: helgaas Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang From: Rui Wang <rui.y.wang@intel.com> On Wed, June 22, 2016 11:15 PM Bjorn Helgaas wrote: > [...] > > @@ -1779,8 +1780,12 @@ void __init > > pci_assign_unassigned_resources(void) > > { > > struct pci_bus *root_bus; > > > > - list_for_each_entry(root_bus, &pci_root_buses, node) > > + list_for_each_entry(root_bus, &pci_root_buses, node) { > > pci_assign_unassigned_root_bus_resources(root_bus); > > +#ifdef CONFIG_X86 > > + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); > > +#endif > > Doesn't this do the right thing even if you omit the #ifdefs, since you > define a stub function below? > No. Without the '#ifdef CONFIG_X86' it breaks MIPS arch. The stub function is within 'ifdef CONFIG_ACPI'. On archs without ACPI it doesn't compile due to 'undefined reference to acpi_ioapic_add'. Thanks Rui ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V4 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot @ 2016-06-23 5:11 ` 0 siblings, 0 replies; 31+ messages in thread From: @ 2016-06-23 5:11 UTC (permalink / raw) To: helgaas Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang From: Rui Wang <rui.y.wang@intel.com> On Wed, June 22, 2016 11:15 PM Bjorn Helgaas wrote: > [...] > > @@ -1779,8 +1780,12 @@ void __init > > pci_assign_unassigned_resources(void) > > { > > struct pci_bus *root_bus; > > > > - list_for_each_entry(root_bus, &pci_root_buses, node) > > + list_for_each_entry(root_bus, &pci_root_buses, node) { > > pci_assign_unassigned_root_bus_resources(root_bus); > > +#ifdef CONFIG_X86 > > + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); > > +#endif > > Doesn't this do the right thing even if you omit the #ifdefs, since you > define a stub function below? > No. Without the '#ifdef CONFIG_X86' it breaks MIPS arch. The stub function is within 'ifdef CONFIG_ACPI'. On archs without ACPI it doesn't compile due to 'undefined reference to acpi_ioapic_add'. Thanks Rui ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V4 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot @ 2016-06-23 5:11 ` 0 siblings, 0 replies; 31+ messages in thread From: @ 2016-06-23 5:11 UTC (permalink / raw) To: helgaas Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang From: Rui Wang <rui.y.wang@intel.com> On Wed, June 22, 2016 11:15 PM Bjorn Helgaas wrote: > [...] > > @@ -1779,8 +1780,12 @@ void __init > > pci_assign_unassigned_resources(void) > > { > > struct pci_bus *root_bus; > > > > - list_for_each_entry(root_bus, &pci_root_buses, node) > > + list_for_each_entry(root_bus, &pci_root_buses, node) { > > pci_assign_unassigned_root_bus_resources(root_bus); > > +#ifdef CONFIG_X86 > > + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); > > +#endif > > Doesn't this do the right thing even if you omit the #ifdefs, since you > define a stub function below? > No. Without the '#ifdef CONFIG_X86' it breaks MIPS arch. The stub function is within 'ifdef CONFIG_ACPI'. On archs without ACPI it doesn't compile due to 'undefined reference to acpi_ioapic_add'. Thanks Rui ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V4 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-23 5:11 ` (?) (?) @ 2016-06-23 17:34 ` Bjorn Helgaas 2016-06-24 15:30 ` Rui Wang 2016-06-26 3:44 ` [PATCH V5 " Rui Wang -1 siblings, 2 replies; 31+ messages in thread From: Bjorn Helgaas @ 2016-06-23 17:34 UTC (permalink / raw) To: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang On Thu, Jun 23, 2016 at 01:11:41PM +0800, wrote: > From: Rui Wang <rui.y.wang@intel.com> > > On Wed, June 22, 2016 11:15 PM Bjorn Helgaas wrote: > > [...] > > > @@ -1779,8 +1780,12 @@ void __init > > > pci_assign_unassigned_resources(void) > > > { > > > struct pci_bus *root_bus; > > > > > > - list_for_each_entry(root_bus, &pci_root_buses, node) > > > + list_for_each_entry(root_bus, &pci_root_buses, node) { > > > pci_assign_unassigned_root_bus_resources(root_bus); > > > +#ifdef CONFIG_X86 > > > + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); > > > +#endif > > > > Doesn't this do the right thing even if you omit the #ifdefs, since you > > define a stub function below? > > > > No. Without the '#ifdef CONFIG_X86' it breaks MIPS arch. The stub function is > within 'ifdef CONFIG_ACPI'. On archs without ACPI it doesn't compile due to > 'undefined reference to acpi_ioapic_add'. But this *could* be made to work by defining a stub for the non-CONFIG_ACPI case. That's what we generally do to avoid ifdefs in the code. Bjorn ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V4 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-23 17:34 ` Bjorn Helgaas @ 2016-06-24 15:30 ` Rui Wang 2016-06-26 3:44 ` [PATCH V5 " Rui Wang 1 sibling, 0 replies; 31+ messages in thread From: Rui Wang @ 2016-06-24 15:30 UTC (permalink / raw) To: helgaas Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang On Fri, Jun 24, 2016 1:35 AM Bjorn Helgaas wrote: > On Thu, Jun 23, 2016 at 01:11:41PM +0800, Rui Wang wrote: > > On Wed, June 22, 2016 11:15 PM Bjorn Helgaas wrote: > > > [...] > > > > @@ -1779,8 +1780,12 @@ void __init > > > > pci_assign_unassigned_resources(void) > > > > { > > > > struct pci_bus *root_bus; > > > > > > > > - list_for_each_entry(root_bus, &pci_root_buses, node) > > > > + list_for_each_entry(root_bus, &pci_root_buses, node) { > > > > pci_assign_unassigned_root_bus_resources(root_bus); > > > > +#ifdef CONFIG_X86 > > > > + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); > > > > +#endif > > > > > > Doesn't this do the right thing even if you omit the #ifdefs, since > > > you define a stub function below? > > > > > > > No. Without the '#ifdef CONFIG_X86' it breaks MIPS arch. The stub > > function is within 'ifdef CONFIG_ACPI'. On archs without ACPI it > > doesn't compile due to 'undefined reference to acpi_ioapic_add'. > > But this *could* be made to work by defining a stub for the non- > CONFIG_ACPI case. That's what we generally do to avoid ifdefs in the code. That can be done. I'll make the change and do some cross-compiling tests first. Thanks Rui ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH V5 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-23 17:34 ` Bjorn Helgaas 2016-06-24 15:30 ` Rui Wang @ 2016-06-26 3:44 ` Rui Wang 2016-08-08 20:22 ` Bjorn Helgaas 1 sibling, 1 reply; 31+ messages in thread From: Rui Wang @ 2016-06-26 3:44 UTC (permalink / raw) To: helgaas Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang v5: Remove #ifdef CONFIG_X86 from setup-bus.c, making it neutral to archs. v4: Add comments explaining when to call acpi_ioapic_add(). v3: Previous versions break mips. This version fixes it. IOAPICs present during system boot aren't added to ioapic_list, thus are unable to be hot-removed. Fix it by calling acpi_ioapic_add() during root bus enumeration. Signed-off-by: Rui Wang <rui.y.wang@intel.com> --- drivers/acpi/internal.h | 2 -- drivers/acpi/ioapic.c | 7 ++++--- drivers/acpi/pci_root.c | 13 ++++++++++++- drivers/pci/setup-bus.c | 5 ++++- include/linux/acpi.h | 6 ++++++ 5 files changed, 26 insertions(+), 7 deletions(-) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 27cc7fe..6d8e67e 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -40,10 +40,8 @@ int acpi_sysfs_init(void); void acpi_container_init(void); void acpi_memory_hotplug_init(void); #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC -int acpi_ioapic_add(struct acpi_pci_root *root); int acpi_ioapic_remove(struct acpi_pci_root *root); #else -static inline int acpi_ioapic_add(struct acpi_pci_root *root) { return 0; } static inline int acpi_ioapic_remove(struct acpi_pci_root *root) { return 0; } #endif #ifdef CONFIG_ACPI_DOCK diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c index ccdc8db..0f272e2 100644 --- a/drivers/acpi/ioapic.c +++ b/drivers/acpi/ioapic.c @@ -189,16 +189,17 @@ exit: return AE_OK; } -int acpi_ioapic_add(struct acpi_pci_root *root) +int acpi_ioapic_add(acpi_handle root_handle) { acpi_status status, retval = AE_OK; - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root->device->handle, + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root_handle, UINT_MAX, handle_ioapic_add, NULL, - root->device->handle, (void **)&retval); + root_handle, (void **)&retval); return ACPI_SUCCESS(status) && ACPI_SUCCESS(retval) ? 0 : -ENODEV; } +EXPORT_SYMBOL_GPL(acpi_ioapic_add); int acpi_ioapic_remove(struct acpi_pci_root *root) { diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index ae3fe4e..31e4440 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -614,7 +614,18 @@ static int acpi_pci_root_add(struct acpi_device *device, if (hotadd) { pcibios_resource_survey_bus(root->bus); pci_assign_unassigned_root_bus_resources(root->bus); - acpi_ioapic_add(root); + + /* + * This is only called for the hotadd case. For the boot-time + * case, we need to wait until after PCI initialization in + * order to deal with IOAPICs mapped in on a PCI BAR. + * + * This is currently x86-specific, because acpi_ioapic_add() + * is an empty function without CONFIG_ACPI_HOTPLUG_IOAPIC. + * And CONFIG_ACPI_HOTPLUG_IOAPIC depends on CONFIG_X86_IO_APIC + * (see drivers/acpi/Kconfig). + */ + acpi_ioapic_add(root->device->handle); } pci_lock_rescan_remove(); diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 55641a3..e32c356 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -25,6 +25,7 @@ #include <linux/ioport.h> #include <linux/cache.h> #include <linux/slab.h> +#include <linux/acpi.h> #include "pci.h" unsigned int pci_flags; @@ -1779,8 +1780,10 @@ void __init pci_assign_unassigned_resources(void) { struct pci_bus *root_bus; - list_for_each_entry(root_bus, &pci_root_buses, node) + list_for_each_entry(root_bus, &pci_root_buses, node) { pci_assign_unassigned_root_bus_resources(root_bus); + acpi_ioapic_add(ACPI_HANDLE(root_bus->bridge)); + } } void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 288fac5..f5114dc 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -680,6 +680,12 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev) #endif /* !CONFIG_ACPI */ +#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC +int acpi_ioapic_add(acpi_handle root); +#else +static inline int acpi_ioapic_add(acpi_handle root) { return 0; } +#endif + #ifdef CONFIG_ACPI void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 pm1a_ctrl, u32 pm1b_ctrl)); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH V5 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-06-26 3:44 ` [PATCH V5 " Rui Wang @ 2016-08-08 20:22 ` Bjorn Helgaas 2016-08-09 3:23 ` Rui Wang 0 siblings, 1 reply; 31+ messages in thread From: Bjorn Helgaas @ 2016-08-08 20:22 UTC (permalink / raw) To: Rui Wang Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel On Sun, Jun 26, 2016 at 11:44:57AM +0800, Rui Wang wrote: > v5: Remove #ifdef CONFIG_X86 from setup-bus.c, making it neutral to archs. > v4: Add comments explaining when to call acpi_ioapic_add(). > v3: Previous versions break mips. This version fixes it. > > IOAPICs present during system boot aren't added to ioapic_list, > thus are unable to be hot-removed. Fix it by calling > acpi_ioapic_add() during root bus enumeration. > > Signed-off-by: Rui Wang <rui.y.wang@intel.com> Hi Rui, Where are we at with this? If there's anything that still needs to be merged, can you rebase it to v4.8-rc1 and post a new, complete, series? Bjorn ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V5 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-08-08 20:22 ` Bjorn Helgaas @ 2016-08-09 3:23 ` Rui Wang 2016-08-09 12:09 ` Rafael J. Wysocki 0 siblings, 1 reply; 31+ messages in thread From: Rui Wang @ 2016-08-09 3:23 UTC (permalink / raw) To: helgaas Cc: tglx, rjw, tony.luck, bhelgaas, linux-acpi, linux-pci, linux-kernel, rui.y.wang On Tuesday, Aug 9, 2016 4:23 AM, Bjorn Helgaas wrote: > On Sun, Jun 26, 2016 at 11:44:57AM +0800, Rui Wang wrote: > > v5: Remove #ifdef CONFIG_X86 from setup-bus.c, making it neutral to > archs. > > v4: Add comments explaining when to call acpi_ioapic_add(). > > v3: Previous versions break mips. This version fixes it. > > > > IOAPICs present during system boot aren't added to ioapic_list, thus > > are unable to be hot-removed. Fix it by calling > > acpi_ioapic_add() during root bus enumeration. > > > > Signed-off-by: Rui Wang <rui.y.wang@intel.com> > > Hi Rui, > > Where are we at with this? If there's anything that still needs to be merged, > can you rebase it to v4.8-rc1 and post a new, complete, series? > Hi Bjorn, Yes. These patches are needed for IOAPIC hotplug to work. So Thomas & Rafael, any advice? Thanks Rui ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH V5 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot 2016-08-09 3:23 ` Rui Wang @ 2016-08-09 12:09 ` Rafael J. Wysocki 0 siblings, 0 replies; 31+ messages in thread From: Rafael J. Wysocki @ 2016-08-09 12:09 UTC (permalink / raw) To: Rui Wang Cc: Bjorn Helgaas, Thomas Gleixner, Rafael J. Wysocki, Tony Luck, Bjorn Helgaas, ACPI Devel Maling List, Linux PCI, Linux Kernel Mailing List On Tue, Aug 9, 2016 at 5:23 AM, Rui Wang <rui.y.wang@intel.com> wrote: > On Tuesday, Aug 9, 2016 4:23 AM, Bjorn Helgaas wrote: >> On Sun, Jun 26, 2016 at 11:44:57AM +0800, Rui Wang wrote: >> > v5: Remove #ifdef CONFIG_X86 from setup-bus.c, making it neutral to >> archs. >> > v4: Add comments explaining when to call acpi_ioapic_add(). >> > v3: Previous versions break mips. This version fixes it. >> > >> > IOAPICs present during system boot aren't added to ioapic_list, thus >> > are unable to be hot-removed. Fix it by calling >> > acpi_ioapic_add() during root bus enumeration. >> > >> > Signed-off-by: Rui Wang <rui.y.wang@intel.com> >> >> Hi Rui, >> >> Where are we at with this? If there's anything that still needs to be merged, >> can you rebase it to v4.8-rc1 and post a new, complete, series? >> > Hi Bjorn, > > Yes. These patches are needed for IOAPIC hotplug to work. > > So Thomas & Rafael, any advice? Can you please do as per what Bjorn said? That is, rebase the series on top of 4.8-rc1 and post it afresh? Thanks, Rafael ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH V2 2/3] x86/ioapic: Fix wrong pointers in ioapic_setup_resources() 2016-06-08 6:59 [PATCH V2 0/3] ioapic hot-removal bugs Rui Wang 2016-06-08 6:59 ` [PATCH V2 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot Rui Wang @ 2016-06-08 6:59 ` Rui Wang 2016-06-10 9:45 ` [tip:x86/urgent] " tip-bot for Rui Wang ` (2 more replies) 2016-06-08 6:59 ` [PATCH V2 3/3] x86/ioapic: Simplify ioapic_setup_resources() Rui Wang 2 siblings, 3 replies; 31+ messages in thread From: Rui Wang @ 2016-06-08 6:59 UTC (permalink / raw) To: tglx, rjw, tony.luck, bhelgaas Cc: linux-acpi, linux-pci, linux-kernel, rui.y.wang On a 4-socket brickland, hot-removing one ioapic is fine. Hot-removing the 2nd one causes panic in mp_unregister_ioapic() while calling release_resource(). It is because the iomem_res pointer has already been released when removing the first ioapic. Fix it by assigning the correct pointers to ioapics[i].iomem_res in ioapic_setup_resources(). To explain the use of &res[num] here: res is assigned to ioapic_resources, and later in ioapic_insert_resources() we do struct resource *r = ioapic_resources; for_each_ioapic(i) { insert_resource(&iomem_resource, r); r++; } Here r is treated as an arry of struct resource, and the r++ ensures that each element of the array is inserted separately. Thus we should call release_resouce() on each element at &res[num]. Signed-off-by: Rui Wang <rui.y.wang@intel.com> --- arch/x86/kernel/apic/io_apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 84e33ff..446702e 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2588,8 +2588,8 @@ static struct resource * __init ioapic_setup_resources(void) res[num].flags = IORESOURCE_MEM | IORESOURCE_BUSY; snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i); mem += IOAPIC_RESOURCE_NAME_SIZE; + ioapics[i].iomem_res = &res[num]; num++; - ioapics[i].iomem_res = res; } ioapic_resources = res; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [tip:x86/urgent] x86/ioapic: Fix wrong pointers in ioapic_setup_resources() 2016-06-08 6:59 ` [PATCH V2 2/3] x86/ioapic: Fix wrong pointers in ioapic_setup_resources() Rui Wang @ 2016-06-10 9:45 ` tip-bot for Rui Wang 2016-06-10 12:01 ` [tip:x86/apic] x86/ioapic: Fix incorrect " tip-bot for Rui Wang 2016-06-10 12:48 ` [tip:x86/urgent] " tip-bot for Rui Wang 2 siblings, 0 replies; 31+ messages in thread From: tip-bot for Rui Wang @ 2016-06-10 9:45 UTC (permalink / raw) To: linux-tip-commits; +Cc: tglx, mingo, rui.y.wang, linux-kernel, hpa Commit-ID: 0e1c672041819474181a41ce07e4734750789170 Gitweb: http://git.kernel.org/tip/0e1c672041819474181a41ce07e4734750789170 Author: Rui Wang <rui.y.wang@intel.com> AuthorDate: Wed, 8 Jun 2016 14:59:52 +0800 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Fri, 10 Jun 2016 11:41:47 +0200 x86/ioapic: Fix wrong pointers in ioapic_setup_resources() On a 4-socket brickland, hot-removing one ioapic is fine. Hot-removing the 2nd one causes panic in mp_unregister_ioapic() while calling release_resource(). It is because the iomem_res pointer has already been released when removing the first ioapic. To explain the use of &res[num] here: res is assigned to ioapic_resources, and later in ioapic_insert_resources() we do struct resource *r = ioapic_resources; for_each_ioapic(i) { insert_resource(&iomem_resource, r); r++; } Here r is treated as an arry of struct resource, and the r++ ensures that each element of the array is inserted separately. Thus we should call release_resouce() on each element at &res[num]. Fix it by assigning the correct pointers to ioapics[i].iomem_res in ioapic_setup_resources(). Signed-off-by: Rui Wang <rui.y.wang@intel.com> Cc: tony.luck@intel.com Cc: linux-pci@vger.kernel.org Cc: rjw@rjwysocki.net Cc: linux-acpi@vger.kernel.org Cc: bhelgaas@google.com Link: http://lkml.kernel.org/r/1465369193-4816-3-git-send-email-rui.y.wang@intel.com Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/kernel/apic/io_apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 84e33ff..446702e 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2588,8 +2588,8 @@ static struct resource * __init ioapic_setup_resources(void) res[num].flags = IORESOURCE_MEM | IORESOURCE_BUSY; snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i); mem += IOAPIC_RESOURCE_NAME_SIZE; + ioapics[i].iomem_res = &res[num]; num++; - ioapics[i].iomem_res = res; } ioapic_resources = res; ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [tip:x86/apic] x86/ioapic: Fix incorrect pointers in ioapic_setup_resources() 2016-06-08 6:59 ` [PATCH V2 2/3] x86/ioapic: Fix wrong pointers in ioapic_setup_resources() Rui Wang 2016-06-10 9:45 ` [tip:x86/urgent] " tip-bot for Rui Wang @ 2016-06-10 12:01 ` tip-bot for Rui Wang 2016-06-10 12:48 ` [tip:x86/urgent] " tip-bot for Rui Wang 2 siblings, 0 replies; 31+ messages in thread From: tip-bot for Rui Wang @ 2016-06-10 12:01 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, rui.y.wang, tglx, mingo, hpa Commit-ID: 3d98217b6726e0c342ac6363346bd9751fd61ec9 Gitweb: http://git.kernel.org/tip/3d98217b6726e0c342ac6363346bd9751fd61ec9 Author: Rui Wang <rui.y.wang@intel.com> AuthorDate: Wed, 8 Jun 2016 14:59:52 +0800 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Fri, 10 Jun 2016 13:55:03 +0200 x86/ioapic: Fix incorrect pointers in ioapic_setup_resources() On a 4-socket Brickland system, hot-removing one ioapic is fine. Hot-removing the 2nd one causes panic in mp_unregister_ioapic() while calling release_resource(). It is because the iomem_res pointer has already been released when removing the first ioapic. To explain the use of &res[num] here: res is assigned to ioapic_resources, and later in ioapic_insert_resources() we do: struct resource *r = ioapic_resources; for_each_ioapic(i) { insert_resource(&iomem_resource, r); r++; } Here 'r' is treated as an arry of 'struct resource', and the r++ ensures that each element of the array is inserted separately. Thus we should call release_resouce() on each element at &res[num]. Fix it by assigning the correct pointers to ioapics[i].iomem_res in ioapic_setup_resources(). Signed-off-by: Rui Wang <rui.y.wang@intel.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: tony.luck@intel.com Cc: linux-pci@vger.kernel.org Cc: rjw@rjwysocki.net Cc: linux-acpi@vger.kernel.org Cc: bhelgaas@google.com Link: http://lkml.kernel.org/r/1465369193-4816-3-git-send-email-rui.y.wang@intel.com Signed-off-by: Ingo Molnar <mingo@kernel.org> Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/kernel/apic/io_apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 84e33ff..446702e 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2588,8 +2588,8 @@ static struct resource * __init ioapic_setup_resources(void) res[num].flags = IORESOURCE_MEM | IORESOURCE_BUSY; snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i); mem += IOAPIC_RESOURCE_NAME_SIZE; + ioapics[i].iomem_res = &res[num]; num++; - ioapics[i].iomem_res = res; } ioapic_resources = res; ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [tip:x86/urgent] x86/ioapic: Fix incorrect pointers in ioapic_setup_resources() 2016-06-08 6:59 ` [PATCH V2 2/3] x86/ioapic: Fix wrong pointers in ioapic_setup_resources() Rui Wang 2016-06-10 9:45 ` [tip:x86/urgent] " tip-bot for Rui Wang 2016-06-10 12:01 ` [tip:x86/apic] x86/ioapic: Fix incorrect " tip-bot for Rui Wang @ 2016-06-10 12:48 ` tip-bot for Rui Wang 2 siblings, 0 replies; 31+ messages in thread From: tip-bot for Rui Wang @ 2016-06-10 12:48 UTC (permalink / raw) To: linux-tip-commits; +Cc: rui.y.wang, tglx, mingo, linux-kernel, hpa Commit-ID: 9d98bcec731756b8688b59ec998707924d716d7b Gitweb: http://git.kernel.org/tip/9d98bcec731756b8688b59ec998707924d716d7b Author: Rui Wang <rui.y.wang@intel.com> AuthorDate: Wed, 8 Jun 2016 14:59:52 +0800 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Fri, 10 Jun 2016 14:45:54 +0200 x86/ioapic: Fix incorrect pointers in ioapic_setup_resources() On a 4-socket Brickland system, hot-removing one ioapic is fine. Hot-removing the 2nd one causes panic in mp_unregister_ioapic() while calling release_resource(). It is because the iomem_res pointer has already been released when removing the first ioapic. To explain the use of &res[num] here: res is assigned to ioapic_resources, and later in ioapic_insert_resources() we do: struct resource *r = ioapic_resources; for_each_ioapic(i) { insert_resource(&iomem_resource, r); r++; } Here 'r' is treated as an arry of 'struct resource', and the r++ ensures that each element of the array is inserted separately. Thus we should call release_resouce() on each element at &res[num]. Fix it by assigning the correct pointers to ioapics[i].iomem_res in ioapic_setup_resources(). Signed-off-by: Rui Wang <rui.y.wang@intel.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: tony.luck@intel.com Cc: linux-pci@vger.kernel.org Cc: rjw@rjwysocki.net Cc: linux-acpi@vger.kernel.org Cc: bhelgaas@google.com Link: http://lkml.kernel.org/r/1465369193-4816-3-git-send-email-rui.y.wang@intel.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/kernel/apic/io_apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 84e33ff..446702e 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2588,8 +2588,8 @@ static struct resource * __init ioapic_setup_resources(void) res[num].flags = IORESOURCE_MEM | IORESOURCE_BUSY; snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i); mem += IOAPIC_RESOURCE_NAME_SIZE; + ioapics[i].iomem_res = &res[num]; num++; - ioapics[i].iomem_res = res; } ioapic_resources = res; ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH V2 3/3] x86/ioapic: Simplify ioapic_setup_resources() 2016-06-08 6:59 [PATCH V2 0/3] ioapic hot-removal bugs Rui Wang 2016-06-08 6:59 ` [PATCH V2 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot Rui Wang 2016-06-08 6:59 ` [PATCH V2 2/3] x86/ioapic: Fix wrong pointers in ioapic_setup_resources() Rui Wang @ 2016-06-08 6:59 ` Rui Wang 2016-06-10 9:48 ` [tip:x86/apic] " tip-bot for Rui Wang 2016-06-10 12:54 ` tip-bot for Rui Wang 2 siblings, 2 replies; 31+ messages in thread From: Rui Wang @ 2016-06-08 6:59 UTC (permalink / raw) To: tglx, rjw, tony.luck, bhelgaas Cc: linux-acpi, linux-pci, linux-kernel, rui.y.wang Optimize the function by removing the variable 'num'. Signed-off-by: Rui Wang <rui.y.wang@intel.com> --- arch/x86/kernel/apic/io_apic.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 446702e..e587295 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2567,29 +2567,25 @@ static struct resource * __init ioapic_setup_resources(void) unsigned long n; struct resource *res; char *mem; - int i, num = 0; + int i; - for_each_ioapic(i) - num++; - if (num == 0) + if (nr_ioapics == 0) return NULL; n = IOAPIC_RESOURCE_NAME_SIZE + sizeof(struct resource); - n *= num; + n *= nr_ioapics; mem = alloc_bootmem(n); res = (void *)mem; - mem += sizeof(struct resource) * num; + mem += sizeof(struct resource) * nr_ioapics; - num = 0; for_each_ioapic(i) { - res[num].name = mem; - res[num].flags = IORESOURCE_MEM | IORESOURCE_BUSY; + res[i].name = mem; + res[i].flags = IORESOURCE_MEM | IORESOURCE_BUSY; snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i); mem += IOAPIC_RESOURCE_NAME_SIZE; - ioapics[i].iomem_res = &res[num]; - num++; + ioapics[i].iomem_res = &res[i]; } ioapic_resources = res; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [tip:x86/apic] x86/ioapic: Simplify ioapic_setup_resources() 2016-06-08 6:59 ` [PATCH V2 3/3] x86/ioapic: Simplify ioapic_setup_resources() Rui Wang @ 2016-06-10 9:48 ` tip-bot for Rui Wang 2016-06-10 12:54 ` tip-bot for Rui Wang 1 sibling, 0 replies; 31+ messages in thread From: tip-bot for Rui Wang @ 2016-06-10 9:48 UTC (permalink / raw) To: linux-tip-commits; +Cc: hpa, rui.y.wang, tglx, linux-kernel, mingo Commit-ID: 0286d538082b854b4e1f05b29c23556fa344e145 Gitweb: http://git.kernel.org/tip/0286d538082b854b4e1f05b29c23556fa344e145 Author: Rui Wang <rui.y.wang@intel.com> AuthorDate: Wed, 8 Jun 2016 14:59:53 +0800 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Fri, 10 Jun 2016 11:43:52 +0200 x86/ioapic: Simplify ioapic_setup_resources() Optimize the function by removing the variable 'num'. Signed-off-by: Rui Wang <rui.y.wang@intel.com> Cc: tony.luck@intel.com Cc: linux-pci@vger.kernel.org Cc: rjw@rjwysocki.net Cc: linux-acpi@vger.kernel.org Cc: bhelgaas@google.com Link: http://lkml.kernel.org/r/1465369193-4816-4-git-send-email-rui.y.wang@intel.com Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/kernel/apic/io_apic.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 446702e..e587295 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2567,29 +2567,25 @@ static struct resource * __init ioapic_setup_resources(void) unsigned long n; struct resource *res; char *mem; - int i, num = 0; + int i; - for_each_ioapic(i) - num++; - if (num == 0) + if (nr_ioapics == 0) return NULL; n = IOAPIC_RESOURCE_NAME_SIZE + sizeof(struct resource); - n *= num; + n *= nr_ioapics; mem = alloc_bootmem(n); res = (void *)mem; - mem += sizeof(struct resource) * num; + mem += sizeof(struct resource) * nr_ioapics; - num = 0; for_each_ioapic(i) { - res[num].name = mem; - res[num].flags = IORESOURCE_MEM | IORESOURCE_BUSY; + res[i].name = mem; + res[i].flags = IORESOURCE_MEM | IORESOURCE_BUSY; snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i); mem += IOAPIC_RESOURCE_NAME_SIZE; - ioapics[i].iomem_res = &res[num]; - num++; + ioapics[i].iomem_res = &res[i]; } ioapic_resources = res; ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [tip:x86/apic] x86/ioapic: Simplify ioapic_setup_resources() 2016-06-08 6:59 ` [PATCH V2 3/3] x86/ioapic: Simplify ioapic_setup_resources() Rui Wang 2016-06-10 9:48 ` [tip:x86/apic] " tip-bot for Rui Wang @ 2016-06-10 12:54 ` tip-bot for Rui Wang 1 sibling, 0 replies; 31+ messages in thread From: tip-bot for Rui Wang @ 2016-06-10 12:54 UTC (permalink / raw) To: linux-tip-commits; +Cc: tglx, hpa, linux-kernel, rui.y.wang, mingo Commit-ID: 4855531eb8582a98cb905d2baf86021254d7a675 Gitweb: http://git.kernel.org/tip/4855531eb8582a98cb905d2baf86021254d7a675 Author: Rui Wang <rui.y.wang@intel.com> AuthorDate: Wed, 8 Jun 2016 14:59:53 +0800 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Fri, 10 Jun 2016 14:48:18 +0200 x86/ioapic: Simplify ioapic_setup_resources() Optimize the function by removing the variable 'num'. Signed-off-by: Rui Wang <rui.y.wang@intel.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: tony.luck@intel.com Cc: linux-pci@vger.kernel.org Cc: rjw@rjwysocki.net Cc: linux-acpi@vger.kernel.org Cc: bhelgaas@google.com Link: http://lkml.kernel.org/r/1465369193-4816-4-git-send-email-rui.y.wang@intel.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/kernel/apic/io_apic.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 446702e..e587295 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2567,29 +2567,25 @@ static struct resource * __init ioapic_setup_resources(void) unsigned long n; struct resource *res; char *mem; - int i, num = 0; + int i; - for_each_ioapic(i) - num++; - if (num == 0) + if (nr_ioapics == 0) return NULL; n = IOAPIC_RESOURCE_NAME_SIZE + sizeof(struct resource); - n *= num; + n *= nr_ioapics; mem = alloc_bootmem(n); res = (void *)mem; - mem += sizeof(struct resource) * num; + mem += sizeof(struct resource) * nr_ioapics; - num = 0; for_each_ioapic(i) { - res[num].name = mem; - res[num].flags = IORESOURCE_MEM | IORESOURCE_BUSY; + res[i].name = mem; + res[i].flags = IORESOURCE_MEM | IORESOURCE_BUSY; snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i); mem += IOAPIC_RESOURCE_NAME_SIZE; - ioapics[i].iomem_res = &res[num]; - num++; + ioapics[i].iomem_res = &res[i]; } ioapic_resources = res; ^ permalink raw reply related [flat|nested] 31+ messages in thread
end of thread, other threads:[~2016-08-09 12:09 UTC | newest] Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-08 6:59 [PATCH V2 0/3] ioapic hot-removal bugs Rui Wang 2016-06-08 6:59 ` [PATCH V2 1/3] x86/ioapic: Support hot-removal of IOAPICs present during boot Rui Wang 2016-06-08 8:05 ` kbuild test robot 2016-06-08 8:05 ` kbuild test robot 2016-06-08 9:32 ` [PATCH V3 " Rui Wang 2016-06-10 12:57 ` Thomas Gleixner 2016-06-10 13:56 ` Rafael J. Wysocki 2016-06-10 16:43 ` Bjorn Helgaas 2016-06-12 6:06 ` Rui Wang 2016-06-16 17:09 ` Bjorn Helgaas 2016-06-22 7:13 ` Rui Wang 2016-06-22 14:53 ` Bjorn Helgaas 2016-06-24 15:18 ` Rui Wang 2016-06-22 7:40 ` [PATCH V4 " Rui Wang 2016-06-22 15:14 ` Bjorn Helgaas 2016-06-23 5:11 ` Unknown, 2016-06-23 5:11 ` 2016-06-23 5:11 ` 2016-06-23 17:34 ` Bjorn Helgaas 2016-06-24 15:30 ` Rui Wang 2016-06-26 3:44 ` [PATCH V5 " Rui Wang 2016-08-08 20:22 ` Bjorn Helgaas 2016-08-09 3:23 ` Rui Wang 2016-08-09 12:09 ` Rafael J. Wysocki 2016-06-08 6:59 ` [PATCH V2 2/3] x86/ioapic: Fix wrong pointers in ioapic_setup_resources() Rui Wang 2016-06-10 9:45 ` [tip:x86/urgent] " tip-bot for Rui Wang 2016-06-10 12:01 ` [tip:x86/apic] x86/ioapic: Fix incorrect " tip-bot for Rui Wang 2016-06-10 12:48 ` [tip:x86/urgent] " tip-bot for Rui Wang 2016-06-08 6:59 ` [PATCH V2 3/3] x86/ioapic: Simplify ioapic_setup_resources() Rui Wang 2016-06-10 9:48 ` [tip:x86/apic] " tip-bot for Rui Wang 2016-06-10 12:54 ` tip-bot for Rui Wang
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.