[PATCH 0/4] Fixing a set of bugs for ioapic hotplug

[PATCH 0/4] Fixing a set of bugs for ioapic hotplug

[Rui Wang]

Hi all,

The 1st patch has been discussed before Bjorn went on vacation. I've fixed
all the issues. Bjorn, please advise how we'll move forward.

The remaining patches fix newly found bugs while testing ioapic hotplug.

Regards,
Rui

Rui Wang (4):
  x86/ioapic: Support hot-removal of IOAPICs present during boot
  x86/ioapic: Fix setup_res() failing to get resource
  x86/ioapic: Fix lost ioapic resource after hot-removal and hotadd
  x86/ioapic: Fix ioapic failing to request resource

 drivers/acpi/internal.h |  2 --
 drivers/acpi/ioapic.c   | 47 +++++++++++++++++++++++++++--------------------
 drivers/acpi/pci_root.c | 13 ++++++++++++-
 drivers/pci/setup-bus.c |  5 ++++-
 include/linux/acpi.h    |  6 ++++++
 5 files changed, 49 insertions(+), 24 deletions(-)

-- 
1.8.3.1

[PATCH 1/4] x86/ioapic: Support hot-removal of IOAPICs present during boot

[Rui 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 | 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..d818c61 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

Re: [PATCH 1/4] x86/ioapic: Support hot-removal of IOAPICs present during boot

[Bjorn Helgaas]

On Wed, Jul 27, 2016 at 12:13:14AM +0800, Rui Wang wrote:
> 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; }

It would be nicer if the interface change and header file munging
were in a separate patch so they wouldn't obscure the meat of the
change, i.e., the addition of calls to acpi_ioapic_add().

>  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);

What loadable module needs to call this?  It shouldn't be exported
unless there is such a module.

>  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..d818c61 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
> 

Re: [PATCH 1/4] x86/ioapic: Support hot-removal of IOAPICs present during boot

[Bjorn Helgaas]

On Tue, Jul 26, 2016 at 03:19:32PM -0500, Bjorn Helgaas wrote:
> On Wed, Jul 27, 2016 at 12:13:14AM +0800, Rui Wang wrote:
> > 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>

I should have added:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

The comments below are minor, and I'm happy if Rafael merges this
series via his tree.

> > ---
> >  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; }
> 
> It would be nicer if the interface change and header file munging
> were in a separate patch so they wouldn't obscure the meat of the
> change, i.e., the addition of calls to acpi_ioapic_add().
> 
> >  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);
> 
> What loadable module needs to call this?  It shouldn't be exported
> unless there is such a module.
> 
> >  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..d818c61 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
> > 
> --
> 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

[PATCH v2 0/5] Fixing a set of bugs for ioapic hotplug

[Rui Wang]

A set of patches fixing bugs found while testing IOAPIC hotplug.

Regards,
Rui

Changelog:

Changes from v1 to v2:
* Split the first patch into two as advised by Bjorn: "would be nicer if
the interface change and header file munging were in a separate patch so
they wouldn't obscure the meat of the change, i.e., the addition of calls
to acpi_ioapic_add()."
* Removed acpi_ioapic_add() as an exported symbol.
* Fixed some typos, and s/acpi/ACPI/, s/ioapic/IOAPIC/ throughout.
* Fixed a warning from 0-day testing.

Rui Wang (5):
  x86/ioapic: Change prototype of acpi_ioapic_add()
  x86/ioapic: Support hot-removal of IOAPICs present during boot
  x86/ioapic: Fix setup_res() failing to get resource
  x86/ioapic: Fix lost IOAPIC resource after hot-removal and hotadd
  x86/ioapic: Fix ioapic failing to request resource

 drivers/acpi/internal.h |  2 --
 drivers/acpi/ioapic.c   | 46 ++++++++++++++++++++++++++--------------------
 drivers/acpi/pci_root.c | 12 +++++++++++-
 drivers/pci/setup-bus.c |  5 ++++-
 include/linux/acpi.h    |  6 ++++++
 5 files changed, 47 insertions(+), 24 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/5] x86/ioapic: Change prototype of acpi_ioapic_add()

[Rui Wang]

Change the argument of acpi_ioapic_add() to a generic ACPI handle, and
move its prototype from drivers/acpi/internal.h to include/linux/acpi.h
so that it can be called from outside the pci_root driver.

Signed-off-by: Rui Wang <rui.y.wang@intel.com>
---
 drivers/acpi/internal.h | 2 --
 drivers/acpi/ioapic.c   | 6 +++---
 drivers/acpi/pci_root.c | 2 +-
 include/linux/acpi.h    | 6 ++++++
 4 files changed, 10 insertions(+), 6 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..2449377 100644
--- a/drivers/acpi/ioapic.c
+++ b/drivers/acpi/ioapic.c
@@ -189,13 +189,13 @@ 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;
 }
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/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

[PATCH v2 2/5] x86/ioapic: Support hot-removal of IOAPICs present during boot

[Rui 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/pci_root.c | 10 ++++++++++
 drivers/pci/setup-bus.c |  5 ++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 53f5965..fa9c83a 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -614,6 +614,16 @@ 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);
+		/*
+		 * 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);
 	}
 
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)
-- 
1.8.3.1

Re: [PATCH v2 2/5] x86/ioapic: Support hot-removal of IOAPICs present during boot

[Bjorn Helgaas]

On Wed, Jul 27, 2016 at 11:44:12PM +0800, Rui Wang wrote:
> 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>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

I assume Rafael will merge this since it's mostly ACPI changes.  Let me
know if you need anything else from me.

> ---
>  drivers/acpi/pci_root.c | 10 ++++++++++
>  drivers/pci/setup-bus.c |  5 ++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 53f5965..fa9c83a 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -614,6 +614,16 @@ 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);
> +		/*
> +		 * 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);
>  	}
>  
> 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)
> -- 
> 1.8.3.1
> 

[PATCH v2 3/5] x86/ioapic: Fix setup_res() failing to get resource

[Rui Wang]

setup_res() doesn't actually get any resource because it mistakenly
checks the return value of acpi_dev_filter_resource_type(), which
returns 0 on success, and 1 on failure. Fix it by taking the return
value of non-zero as failing to match the specified resource type.

Signed-off-by: Rui Wang <rui.y.wang@intel.com>
---
 drivers/acpi/ioapic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
index 2449377..8ab6d42 100644
--- a/drivers/acpi/ioapic.c
+++ b/drivers/acpi/ioapic.c
@@ -46,7 +46,7 @@ static acpi_status setup_res(struct acpi_resource *acpi_res, void *data)
 	struct resource_win win;
 
 	res->flags = 0;
-	if (acpi_dev_filter_resource_type(acpi_res, IORESOURCE_MEM) == 0)
+	if (acpi_dev_filter_resource_type(acpi_res, IORESOURCE_MEM))
 		return AE_OK;
 
 	if (!acpi_dev_resource_memory(acpi_res, res)) {
-- 
1.8.3.1

[PATCH v2 4/5] x86/ioapic: Fix lost IOAPIC resource after hot-removal and hotadd

[Rui Wang]

IOAPIC resource at 0xfecxxxxx gets lost from /proc/iomem after
hot-removing and then hot-adding the IOAPIC device.

After system boot, in /proc/iomem:
fec00000-fecfffff : PNP0003:00
  fec00000-fec003ff : IOAPIC 0
  fec01000-fec013ff : IOAPIC 1
  fec40000-fec403ff : IOAPIC 2
  fec80000-fec803ff : IOAPIC 3
  fecc0000-fecc03ff : IOAPIC 4

Then hot-remove IOAPIC 2 and hot-add it again:
fec00000-fecfffff : PNP0003:00
  fec00000-fec003ff : IOAPIC 0
  fec01000-fec013ff : IOAPIC 1
  fec80000-fec803ff : IOAPIC 3
  fecc0000-fecc03ff : IOAPIC 4

The range at 0xfec40000 is lost from /proc/iomem. It is because
handle_ioapic_add() requests resource from either PCI config BAR or
ACPI "_CRS", not both. But Intel platforms map the IOxAPIC registers
both at the PCI config BAR (called MBAR, dynamic), and at the ACPI
"_CRS" (called ABAR, static). The 0xfecX_YZ00 to 0xfecX_YZFF range
appears in "_CRS" of each IOAPIC device. Both ranges should be claimed
from /proc/iomem for exclusive use.

Signed-off-by: Rui Wang <rui.y.wang@intel.com>
---
 drivers/acpi/ioapic.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
index 8ab6d42..ee20111 100644
--- a/drivers/acpi/ioapic.c
+++ b/drivers/acpi/ioapic.c
@@ -97,7 +97,7 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl,
 	unsigned long long gsi_base;
 	struct acpi_pci_ioapic *ioapic;
 	struct pci_dev *dev = NULL;
-	struct resource *res = NULL;
+	struct resource *res = NULL, *pci_res = NULL, *crs_res;
 	char *type = NULL;
 
 	if (!acpi_is_ioapic(handle, &type))
@@ -137,23 +137,28 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl,
 		pci_set_master(dev);
 		if (pci_request_region(dev, 0, type))
 			goto exit_disable;
-		res = &dev->resource[0];
+		pci_res = &dev->resource[0];
 		ioapic->pdev = dev;
 	} else {
 		pci_dev_put(dev);
 		dev = NULL;
+	}
 
-		res = &ioapic->res;
-		acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
-		if (res->flags == 0) {
-			acpi_handle_warn(handle, "failed to get resource\n");
-			goto exit_free;
-		} else if (request_resource(&iomem_resource, res)) {
-			acpi_handle_warn(handle, "failed to insert resource\n");
-			goto exit_free;
-		}
+	crs_res = &ioapic->res;
+	acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, crs_res);
+	if (crs_res->flags == 0) {
+		acpi_handle_warn(handle, "failed to get resource\n");
+		goto exit_release;
+	} else if (request_resource(&iomem_resource, crs_res)) {
+		acpi_handle_warn(handle, "failed to insert resource\n");
+		goto exit_release;
 	}
 
+	/* try pci resource first, then "_CRS" resource */
+	res = pci_res;
+	if (!res || !res->flags)
+		res = crs_res;
+
 	if (acpi_register_ioapic(handle, res->start, (u32)gsi_base)) {
 		acpi_handle_warn(handle, "failed to register IOAPIC\n");
 		goto exit_release;
@@ -174,14 +179,13 @@ done:
 exit_release:
 	if (dev)
 		pci_release_region(dev, 0);
-	else
-		release_resource(res);
+	if (ioapic->res.flags && ioapic->res.parent)
+		release_resource(&ioapic->res);
 exit_disable:
 	if (dev)
 		pci_disable_device(dev);
 exit_put:
 	pci_dev_put(dev);
-exit_free:
 	kfree(ioapic);
 exit:
 	mutex_unlock(&ioapic_list_lock);
@@ -217,9 +221,9 @@ int acpi_ioapic_remove(struct acpi_pci_root *root)
 			pci_release_region(ioapic->pdev, 0);
 			pci_disable_device(ioapic->pdev);
 			pci_dev_put(ioapic->pdev);
-		} else if (ioapic->res.flags && ioapic->res.parent) {
-			release_resource(&ioapic->res);
 		}
+		if (ioapic->res.flags && ioapic->res.parent)
+			release_resource(&ioapic->res);
 		list_del(&ioapic->list);
 		kfree(ioapic);
 	}
-- 
1.8.3.1

[PATCH v2 5/5] x86/ioapic: Fix ioapic failing to request resource

[Rui Wang]

handle_ioapic_add() uses request_resource() to request ACPI "_CRS"
resources. This can fail with the following error message:

[  247.325693] ACPI: \_SB_.IIO1.AID1: failed to insert resource

This happens when there are multiple IOAPICs and DSDT groups their
"_CRS" resources as the children of a parent resource, as seen from
/proc/iomem:

fec00000-fecfffff : PNP0003:00
  fec00000-fec003ff : IOAPIC 0
  fec01000-fec013ff : IOAPIC 1
  fec40000-fec403ff : IOAPIC 2

In this case request_resource() fails because there's a conflicting
resource which is the parent (fec0000-fecfffff). Fix it by using
insert_resource() which can request resources by taking the conflicting
resource as the parent.

Signed-off-by: Rui Wang <rui.y.wang@intel.com>
---
 drivers/acpi/ioapic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
index ee20111..6d7ce6e 100644
--- a/drivers/acpi/ioapic.c
+++ b/drivers/acpi/ioapic.c
@@ -146,10 +146,12 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl,
 
 	crs_res = &ioapic->res;
 	acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, crs_res);
+	crs_res->name = type;
+	crs_res->flags |= IORESOURCE_BUSY;
 	if (crs_res->flags == 0) {
 		acpi_handle_warn(handle, "failed to get resource\n");
 		goto exit_release;
-	} else if (request_resource(&iomem_resource, crs_res)) {
+	} else if (insert_resource(&iomem_resource, crs_res)) {
 		acpi_handle_warn(handle, "failed to insert resource\n");
 		goto exit_release;
 	}
-- 
1.8.3.1

Re: [PATCH 1/4] x86/ioapic: Support hot-removal of IOAPICs present during boot boot

[Rui Wang]

On Wed, July 27, 2016 4:20 AM Bjorn Helgaas wrote:
> On Wed, Jul 27, 2016 at 12:13:14AM +0800, Rui Wang wrote:
> > 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; }
> 
> It would be nicer if the interface change and header file munging were in a
> separate patch so they wouldn't obscure the meat of the change, i.e., the
> addition of calls to acpi_ioapic_add().

Sure. I'll split it in a newer version.

> 
> >  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);
> 
> What loadable module needs to call this?  It shouldn't be exported unless
> there is such a module.

It's called from the ioapic driver which is built-in. I'll remove the export.

Thanks
Rui

[PATCH 2/4] x86/ioapic: Fix setup_res() failing to get resource

[Rui Wang]

setup_res() doesn't actually get any resoure because it mistakenly
checks the return value of acpi_dev_filter_resource_type(), which
returns 0 on success, and 1 on failure. Fix it by taking the return
value of non-zero as failing to match the specified resource type.

Signed-off-by: Rui Wang <rui.y.wang@intel.com>
---
 drivers/acpi/ioapic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
index 0f272e2..daf4a40 100644
--- a/drivers/acpi/ioapic.c
+++ b/drivers/acpi/ioapic.c
@@ -46,7 +46,7 @@ static acpi_status setup_res(struct acpi_resource *acpi_res, void *data)
 	struct resource_win win;
 
 	res->flags = 0;
-	if (acpi_dev_filter_resource_type(acpi_res, IORESOURCE_MEM) == 0)
+	if (acpi_dev_filter_resource_type(acpi_res, IORESOURCE_MEM))
 		return AE_OK;
 
 	if (!acpi_dev_resource_memory(acpi_res, res)) {
-- 
1.8.3.1

Re: [PATCH 2/4] x86/ioapic: Fix setup_res() failing to get resource

[Bjorn Helgaas]

On Wed, Jul 27, 2016 at 12:13:15AM +0800, Rui Wang wrote:
> setup_res() doesn't actually get any resoure because it mistakenly

s/resoure/resource/

> checks the return value of acpi_dev_filter_resource_type(), which
> returns 0 on success, and 1 on failure. Fix it by taking the return
> value of non-zero as failing to match the specified resource type.
> 
> Signed-off-by: Rui Wang <rui.y.wang@intel.com>
> ---
>  drivers/acpi/ioapic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
> index 0f272e2..daf4a40 100644
> --- a/drivers/acpi/ioapic.c
> +++ b/drivers/acpi/ioapic.c
> @@ -46,7 +46,7 @@ static acpi_status setup_res(struct acpi_resource *acpi_res, void *data)
>  	struct resource_win win;
>  
>  	res->flags = 0;
> -	if (acpi_dev_filter_resource_type(acpi_res, IORESOURCE_MEM) == 0)
> +	if (acpi_dev_filter_resource_type(acpi_res, IORESOURCE_MEM))
>  		return AE_OK;
>  
>  	if (!acpi_dev_resource_memory(acpi_res, res)) {
> -- 
> 1.8.3.1
> 

[PATCH 3/4] x86/ioapic: Fix lost ioapic resource after hot-removal and hotadd

[Rui Wang]

ioapic resource at 0xfecxxxxx gets lost from /proc/iomem after
hot-removing and then hot-adding the ioapic devices.

After system boot, in /proc/iomem:
fec00000-fecfffff : PNP0003:00
  fec00000-fec003ff : IOAPIC 0
  fec01000-fec013ff : IOAPIC 1
  fec40000-fec403ff : IOAPIC 2
  fec80000-fec803ff : IOAPIC 3
  fecc0000-fecc03ff : IOAPIC 4

Then hot-remove IOAPIC 2 and hot-add it again:
fec00000-fecfffff : PNP0003:00
  fec00000-fec003ff : IOAPIC 0
  fec01000-fec013ff : IOAPIC 1
  fec80000-fec803ff : IOAPIC 3
  fecc0000-fecc03ff : IOAPIC 4

The range at 0xfec40000 is lost from /proc/iomem. It is because
handle_ioapic_add() requests resource from either PCI config BAR or
acpi _CRS, not both. But Intel platforms map the IOxAPIC registers
at both the PCI config BAR (called MBAR) and the 0xfecX_YZ00 to
0xfecX_Y2FF range (called ABAR). Both of the ranges should be claimed
from /proc/iomem for exclusive use.

Signed-off-by: Rui Wang <rui.y.wang@intel.com>
---
 drivers/acpi/ioapic.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
index daf4a40..80b0b1a 100644
--- a/drivers/acpi/ioapic.c
+++ b/drivers/acpi/ioapic.c
@@ -97,7 +97,7 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl,
 	unsigned long long gsi_base;
 	struct acpi_pci_ioapic *ioapic;
 	struct pci_dev *dev = NULL;
-	struct resource *res = NULL;
+	struct resource *res = NULL, *pci_res, *crs_res;
 	char *type = NULL;
 
 	if (!acpi_is_ioapic(handle, &type))
@@ -137,23 +137,28 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl,
 		pci_set_master(dev);
 		if (pci_request_region(dev, 0, type))
 			goto exit_disable;
-		res = &dev->resource[0];
+		pci_res = &dev->resource[0];
 		ioapic->pdev = dev;
 	} else {
 		pci_dev_put(dev);
 		dev = NULL;
+	}
 
-		res = &ioapic->res;
-		acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
-		if (res->flags == 0) {
-			acpi_handle_warn(handle, "failed to get resource\n");
-			goto exit_free;
-		} else if (request_resource(&iomem_resource, res)) {
-			acpi_handle_warn(handle, "failed to insert resource\n");
-			goto exit_free;
-		}
+	crs_res = &ioapic->res;
+	acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, crs_res);
+	if (crs_res->flags == 0) {
+		acpi_handle_warn(handle, "failed to get resource\n");
+		goto exit_release;
+	} else if (request_resource(&iomem_resource, crs_res)) {
+		acpi_handle_warn(handle, "failed to insert resource\n");
+		goto exit_release;
 	}
 
+	/* try pci resource first, then "_CRS" resource */
+	res = pci_res;
+	if (!res || !res->flags)
+		res = crs_res;
+
 	if (acpi_register_ioapic(handle, res->start, (u32)gsi_base)) {
 		acpi_handle_warn(handle, "failed to register IOAPIC\n");
 		goto exit_release;
@@ -174,14 +179,13 @@ done:
 exit_release:
 	if (dev)
 		pci_release_region(dev, 0);
-	else
-		release_resource(res);
+	if (ioapic->res.flags && ioapic->res.parent)
+		release_resource(&ioapic->res);
 exit_disable:
 	if (dev)
 		pci_disable_device(dev);
 exit_put:
 	pci_dev_put(dev);
-exit_free:
 	kfree(ioapic);
 exit:
 	mutex_unlock(&ioapic_list_lock);
@@ -218,9 +222,9 @@ int acpi_ioapic_remove(struct acpi_pci_root *root)
 			pci_release_region(ioapic->pdev, 0);
 			pci_disable_device(ioapic->pdev);
 			pci_dev_put(ioapic->pdev);
-		} else if (ioapic->res.flags && ioapic->res.parent) {
-			release_resource(&ioapic->res);
 		}
+		if (ioapic->res.flags && ioapic->res.parent)
+			release_resource(&ioapic->res);
 		list_del(&ioapic->list);
 		kfree(ioapic);
 	}
-- 
1.8.3.1

Re: [PATCH 3/4] x86/ioapic: Fix lost ioapic resource after hot-removal and hotadd

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2071 bytes --]

Hi,

[auto build test WARNING on pm/linux-next]
[also build test WARNING on v4.7 next-20160726]
[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/Fixing-a-set-of-bugs-for-ioapic-hotplug/20160727-003628
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-x015-201630 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/acpi/ioapic.c: In function 'handle_ioapic_add':
>> drivers/acpi/ioapic.c:159:18: warning: 'pci_res' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (!res || !res->flags)
                  ~~~^~~~~~~

vim +/pci_res +159 drivers/acpi/ioapic.c

   143			pci_dev_put(dev);
   144			dev = NULL;
   145		}
   146	
   147		crs_res = &ioapic->res;
   148		acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, crs_res);
   149		if (crs_res->flags == 0) {
   150			acpi_handle_warn(handle, "failed to get resource\n");
   151			goto exit_release;
   152		} else if (request_resource(&iomem_resource, crs_res)) {
   153			acpi_handle_warn(handle, "failed to insert resource\n");
   154			goto exit_release;
   155		}
   156	
   157		/* try pci resource first, then "_CRS" resource */
   158		res = pci_res;
 > 159		if (!res || !res->flags)
   160			res = crs_res;
   161	
   162		if (acpi_register_ioapic(handle, res->start, (u32)gsi_base)) {
   163			acpi_handle_warn(handle, "failed to register IOAPIC\n");
   164			goto exit_release;
   165		}
   166	done:
   167		list_add(&ioapic->list, &ioapic_list);

---
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: 26180 bytes --]

Re: [PATCH 3/4] x86/ioapic: Fix lost ioapic resource after hot-removal and hotadd

[Bjorn Helgaas]

On Wed, Jul 27, 2016 at 12:13:16AM +0800, Rui Wang wrote:
> ioapic resource at 0xfecxxxxx gets lost from /proc/iomem after
> hot-removing and then hot-adding the ioapic devices.
> 
> After system boot, in /proc/iomem:
> fec00000-fecfffff : PNP0003:00
>   fec00000-fec003ff : IOAPIC 0
>   fec01000-fec013ff : IOAPIC 1
>   fec40000-fec403ff : IOAPIC 2
>   fec80000-fec803ff : IOAPIC 3
>   fecc0000-fecc03ff : IOAPIC 4
> 
> Then hot-remove IOAPIC 2 and hot-add it again:
> fec00000-fecfffff : PNP0003:00
>   fec00000-fec003ff : IOAPIC 0
>   fec01000-fec013ff : IOAPIC 1
>   fec80000-fec803ff : IOAPIC 3
>   fecc0000-fecc03ff : IOAPIC 4
> 
> The range at 0xfec40000 is lost from /proc/iomem. It is because
> handle_ioapic_add() requests resource from either PCI config BAR or
> acpi _CRS, not both. But Intel platforms map the IOxAPIC registers

s/acpi/ACPI/
s/ioapic/IOAPIC/ throughout

> at both the PCI config BAR (called MBAR) and the 0xfecX_YZ00 to
> 0xfecX_Y2FF range (called ABAR). Both of the ranges should be claimed

I guess you mean the 0xfecX_YZ00-0xfecX_Y2FF range appears in _CRS?

> from /proc/iomem for exclusive use.
> 
> Signed-off-by: Rui Wang <rui.y.wang@intel.com>
> ---
>  drivers/acpi/ioapic.c | 36 ++++++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
> index daf4a40..80b0b1a 100644
> --- a/drivers/acpi/ioapic.c
> +++ b/drivers/acpi/ioapic.c
> @@ -97,7 +97,7 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl,
>  	unsigned long long gsi_base;
>  	struct acpi_pci_ioapic *ioapic;
>  	struct pci_dev *dev = NULL;
> -	struct resource *res = NULL;
> +	struct resource *res = NULL, *pci_res, *crs_res;
>  	char *type = NULL;
>  
>  	if (!acpi_is_ioapic(handle, &type))
> @@ -137,23 +137,28 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl,
>  		pci_set_master(dev);
>  		if (pci_request_region(dev, 0, type))
>  			goto exit_disable;
> -		res = &dev->resource[0];
> +		pci_res = &dev->resource[0];
>  		ioapic->pdev = dev;
>  	} else {
>  		pci_dev_put(dev);
>  		dev = NULL;
> +	}
>  
> -		res = &ioapic->res;
> -		acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
> -		if (res->flags == 0) {
> -			acpi_handle_warn(handle, "failed to get resource\n");
> -			goto exit_free;
> -		} else if (request_resource(&iomem_resource, res)) {
> -			acpi_handle_warn(handle, "failed to insert resource\n");
> -			goto exit_free;
> -		}
> +	crs_res = &ioapic->res;
> +	acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, crs_res);
> +	if (crs_res->flags == 0) {
> +		acpi_handle_warn(handle, "failed to get resource\n");
> +		goto exit_release;
> +	} else if (request_resource(&iomem_resource, crs_res)) {
> +		acpi_handle_warn(handle, "failed to insert resource\n");
> +		goto exit_release;
>  	}
>  
> +	/* try pci resource first, then "_CRS" resource */
> +	res = pci_res;
> +	if (!res || !res->flags)
> +		res = crs_res;
> +
>  	if (acpi_register_ioapic(handle, res->start, (u32)gsi_base)) {
>  		acpi_handle_warn(handle, "failed to register IOAPIC\n");
>  		goto exit_release;
> @@ -174,14 +179,13 @@ done:
>  exit_release:
>  	if (dev)
>  		pci_release_region(dev, 0);
> -	else
> -		release_resource(res);
> +	if (ioapic->res.flags && ioapic->res.parent)
> +		release_resource(&ioapic->res);
>  exit_disable:
>  	if (dev)
>  		pci_disable_device(dev);
>  exit_put:
>  	pci_dev_put(dev);
> -exit_free:
>  	kfree(ioapic);
>  exit:
>  	mutex_unlock(&ioapic_list_lock);
> @@ -218,9 +222,9 @@ int acpi_ioapic_remove(struct acpi_pci_root *root)
>  			pci_release_region(ioapic->pdev, 0);
>  			pci_disable_device(ioapic->pdev);
>  			pci_dev_put(ioapic->pdev);
> -		} else if (ioapic->res.flags && ioapic->res.parent) {
> -			release_resource(&ioapic->res);
>  		}
> +		if (ioapic->res.flags && ioapic->res.parent)
> +			release_resource(&ioapic->res);
>  		list_del(&ioapic->list);
>  		kfree(ioapic);
>  	}
> -- 
> 1.8.3.1
> 

Re: [PATCH 3/4] x86/ioapic: Fix lost ioapic resource after hot-removal and hotadd

[Rui Wang]

On Wed, July 27, 2016 4:24 AM, Bjorn Helgaas wrote:
> On Wed, Jul 27, 2016 at 12:13:16AM +0800, Rui Wang wrote:
> > ioapic resource at 0xfecxxxxx gets lost from /proc/iomem after
> > hot-removing and then hot-adding the ioapic devices.
> >
> > After system boot, in /proc/iomem:
> > fec00000-fecfffff : PNP0003:00
> >   fec00000-fec003ff : IOAPIC 0
> >   fec01000-fec013ff : IOAPIC 1
> >   fec40000-fec403ff : IOAPIC 2
> >   fec80000-fec803ff : IOAPIC 3
> >   fecc0000-fecc03ff : IOAPIC 4
> >
> > Then hot-remove IOAPIC 2 and hot-add it again:
> > fec00000-fecfffff : PNP0003:00
> >   fec00000-fec003ff : IOAPIC 0
> >   fec01000-fec013ff : IOAPIC 1
> >   fec80000-fec803ff : IOAPIC 3
> >   fecc0000-fecc03ff : IOAPIC 4
> >
> > The range at 0xfec40000 is lost from /proc/iomem. It is because
> > handle_ioapic_add() requests resource from either PCI config BAR or
> > acpi _CRS, not both. But Intel platforms map the IOxAPIC registers
> 
> s/acpi/ACPI/
> s/ioapic/IOAPIC/ throughout
> 
> > at both the PCI config BAR (called MBAR) and the 0xfecX_YZ00 to
> > 0xfecX_Y2FF range (called ABAR). Both of the ranges should be claimed
> 
> I guess you mean the 0xfecX_YZ00-0xfecX_Y2FF range appears in _CRS?

Yes. That range appears in _CRS for each IOAPIC. I'll make it cleaner in
the commit message.

Thanks
Rui

[PATCH 4/4] x86/ioapic: Fix ioapic failing to request resource

[Rui Wang]

handle_ioapic_add() uses request_resource() to request ACPI "_CRS"
resources. This can fail with the following error message:

[  247.325693] ACPI: \_SB_.IIO1.AID1: failed to insert resource

This happens when there are multiple ioapics and DSDT groups their
"_CRS" resources as the children of a parent resource, as seen from
/proc/iomem:

fec00000-fecfffff : PNP0003:00
  fec00000-fec003ff : IOAPIC 0
  fec01000-fec013ff : IOAPIC 1
  fec40000-fec403ff : IOAPIC 2

In this case request_resource() fails because there's a conflicting
resource which is the parent (fec0000-fecfffff). Fix it by using
insert_resoure() which can request resources by taking the conflicting
resource as the parent.

Signed-off-by: Rui Wang <rui.y.wang@intel.com>
---
 drivers/acpi/ioapic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
index 80b0b1a..2c38ce0 100644
--- a/drivers/acpi/ioapic.c
+++ b/drivers/acpi/ioapic.c
@@ -146,10 +146,12 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl,
 
 	crs_res = &ioapic->res;
 	acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, crs_res);
+	crs_res->name = type;
+	crs_res->flags |= IORESOURCE_BUSY;
 	if (crs_res->flags == 0) {
 		acpi_handle_warn(handle, "failed to get resource\n");
 		goto exit_release;
-	} else if (request_resource(&iomem_resource, crs_res)) {
+	} else if (insert_resource(&iomem_resource, crs_res)) {
 		acpi_handle_warn(handle, "failed to insert resource\n");
 		goto exit_release;
 	}
-- 
1.8.3.1