Date: Thu, 19 Oct 2006 14:58:49 -0400 From: Kei Tokunaga <ktokunag@redhat.com> Subject: [RHEL5 PATCH] ACPIPHP doesn't work (p2p bridge and IOAPIC hotplug) BZ209677 https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=209677 This patch fixes a bug existing in p2p bridge and IOAPIC hotplug. The bug prevents us from operating p2p bridge and IOACPI hotplug on our PRIMEQUEST. # p2p bridge and IOACPI hotplug are our feature requests # for RHEL5 GA (IT70769, BZ182106) and they are important # for our business. These patches are in upstream from 2.6.19-rc1. We have verified that the patches work all right on Milestone4 and I have confirmed the patches apply cleanly to the latest CVS kernel of RHEL5 (kernel-2.6.18-1.2732.el5). The patch does the followings. - We need to assign resources to ioapics being hot-added. This patch changes pbus_assign_resources_sorted() to assign resources if the ioapic has no assigned resources. - This patch prevents pcibios_disable_device() from disabling interrupts of devices which is not enabled. - This patch adds pci_stop_bus_device() which stops a PCI device (detach the driver, remove from the global list and so on) and any children. This is needed for ACPI based PCI-to-PCI bridge hot-remove, and it will be also needed for ACPI based PCI root bridge hot-remove. - Contrary to PCI bridge hot-add, we need to follow the sequence below for PCI bridge hot-removal. (1) Stop devices (detach drivers, remove from the global list, etc.) (2) Unbind ACPI node from the devices (remove the _PRT entries) (3) Remove devices (remove from the device list, etc.) This patch fixes acpiphp driver to follow above sequence for P2P bridge hot-removal. - Currently acpiphp sets hpp values after starting devices, but the values should be set before starting devices. This patch fixes this bug. - Currently acpiphp initializes ioapics after starting devices, but ioapics should be initialized before starting devices. This patch fixes this bug. - Currently acpiphp initializes all ioapics under the bus on which hot-add event occured. It also initializes already working ioapics. This patch fixes this bug. - Currently acpiphp calls pci_enable_device() against all hot-added bridges, but acpiphp does not call pci_disable_device() against them in hot-remove. So ioapic hot-remove would fail. This patch fixes this issue. - This patch adds support for ioapics hot-remove. Thanks, Kei --- linux-2.6.18-1.2732.el5-kei/arch/ia64/pci/pci.c | 3 linux-2.6.18-1.2732.el5-kei/drivers/pci/hotplug/acpiphp.h | 5 linux-2.6.18-1.2732.el5-kei/drivers/pci/hotplug/acpiphp_glue.c | 125 ++++++++-- linux-2.6.18-1.2732.el5-kei/drivers/pci/remove.c | 37 ++ linux-2.6.18-1.2732.el5-kei/drivers/pci/setup-bus.c | 11 linux-2.6.18-1.2732.el5-kei/include/linux/pci.h | 1 6 files changed, 162 insertions(+), 20 deletions(-) diff -puN drivers/pci/hotplug/acpiphp_glue.c~acpiphp-fix drivers/pci/hotplug/acpiphp_glue.c --- linux-2.6.18-1.2732.el5/drivers/pci/hotplug/acpiphp_glue.c~acpiphp-fix 2006-10-19 13:05:33.000000000 -0400 +++ linux-2.6.18-1.2732.el5-kei/drivers/pci/hotplug/acpiphp_glue.c 2006-10-19 13:08:08.000000000 -0400 @@ -53,6 +53,8 @@ #include "acpiphp.h" static LIST_HEAD(bridge_list); +static LIST_HEAD(ioapic_list); +static DEFINE_SPINLOCK(ioapic_list_lock); #define MY_NAME "acpiphp_glue" @@ -797,6 +799,7 @@ ioapic_add(acpi_handle handle, u32 lvl, struct pci_dev *pdev; u32 gsi_base; u64 phys_addr; + struct acpiphp_ioapic *ioapic; /* Evaluate _STA if present */ status = acpi_evaluate_integer(handle, "_STA", NULL, &sta); @@ -811,41 +814,107 @@ ioapic_add(acpi_handle handle, u32 lvl, if (get_gsi_base(handle, &gsi_base)) return AE_OK; + ioapic = kmalloc(sizeof(*ioapic), GFP_KERNEL); + if (!ioapic) + return AE_NO_MEMORY; + pdev = get_apic_pci_info(handle); if (!pdev) - return AE_OK; + goto exit_kfree; - if (pci_enable_device(pdev)) { - pci_dev_put(pdev); - return AE_OK; - } + if (pci_enable_device(pdev)) + goto exit_pci_dev_put; pci_set_master(pdev); - if (pci_request_region(pdev, 0, "I/O APIC(acpiphp)")) { - pci_disable_device(pdev); - pci_dev_put(pdev); - return AE_OK; - } + if (pci_request_region(pdev, 0, "I/O APIC(acpiphp)")) + goto exit_pci_disable_device; phys_addr = pci_resource_start(pdev, 0); - if (acpi_register_ioapic(handle, phys_addr, gsi_base)) { - pci_release_region(pdev, 0); - pci_disable_device(pdev); - pci_dev_put(pdev); + if (acpi_register_ioapic(handle, phys_addr, gsi_base)) + goto exit_pci_release_region; + + ioapic->gsi_base = gsi_base; + ioapic->dev = pdev; + spin_lock(&ioapic_list_lock); + list_add_tail(&ioapic->list, &ioapic_list); + spin_unlock(&ioapic_list_lock); + + return AE_OK; + + exit_pci_release_region: + pci_release_region(pdev, 0); + exit_pci_disable_device: + pci_disable_device(pdev); + exit_pci_dev_put: + pci_dev_put(pdev); + exit_kfree: + kfree(ioapic); + + return AE_OK; +} + +static acpi_status +ioapic_remove(acpi_handle handle, u32 lvl, void *context, void **rv) +{ + acpi_status status; + unsigned long sta; + acpi_handle tmp; + u32 gsi_base; + struct acpiphp_ioapic *pos, *n, *ioapic = NULL; + + /* Evaluate _STA if present */ + status = acpi_evaluate_integer(handle, "_STA", NULL, &sta); + if (ACPI_SUCCESS(status) && sta != ACPI_STA_ALL) + return AE_CTRL_DEPTH; + + /* Scan only PCI bus scope */ + status = acpi_get_handle(handle, "_HID", &tmp); + if (ACPI_SUCCESS(status)) + return AE_CTRL_DEPTH; + + if (get_gsi_base(handle, &gsi_base)) return AE_OK; + + acpi_unregister_ioapic(handle, gsi_base); + + spin_lock(&ioapic_list_lock); + list_for_each_entry_safe(pos, n, &ioapic_list, list) { + if (pos->gsi_base != gsi_base) + continue; + ioapic = pos; + list_del(&ioapic->list); + break; } + spin_unlock(&ioapic_list_lock); + + if (!ioapic) + return AE_OK; + + pci_release_region(ioapic->dev, 0); + pci_disable_device(ioapic->dev); + pci_dev_put(ioapic->dev); + kfree(ioapic); return AE_OK; } static int acpiphp_configure_ioapics(acpi_handle handle) { + ioapic_add(handle, 0, NULL, NULL); acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, ACPI_UINT32_MAX, ioapic_add, NULL, NULL); return 0; } +static int acpiphp_unconfigure_ioapics(acpi_handle handle) +{ + ioapic_remove(handle, 0, NULL, NULL); + acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, + ACPI_UINT32_MAX, ioapic_remove, NULL, NULL); + return 0; +} + static int power_on_slot(struct acpiphp_slot *slot) { acpi_status status; @@ -1074,10 +1143,11 @@ static int enable_device(struct acpiphp_ pci_bus_assign_resources(bus); acpiphp_sanitize_bus(bus); + acpiphp_set_hpp_values(slot->bridge->handle, bus); + list_for_each_entry(func, &slot->funcs, sibling) + acpiphp_configure_ioapics(func->handle); pci_enable_bridges(bus); pci_bus_add_devices(bus); - acpiphp_set_hpp_values(slot->bridge->handle, bus); - acpiphp_configure_ioapics(slot->bridge->handle); /* associate pci_dev to our representation */ list_for_each (l, &slot->funcs) { @@ -1103,6 +1173,16 @@ static int enable_device(struct acpiphp_ return retval; } +static void disable_bridges(struct pci_bus *bus) +{ + struct pci_dev *dev; + list_for_each_entry(dev, &bus->devices, bus_list) { + if (dev->subordinate) { + disable_bridges(dev->subordinate); + pci_disable_device(dev); + } + } +} /** * disable_device - disable a slot @@ -1127,6 +1207,19 @@ static int disable_device(struct acpiphp func->bridge = NULL; } + if (func->pci_dev) { + pci_stop_bus_device(func->pci_dev); + if (func->pci_dev->subordinate) { + disable_bridges(func->pci_dev->subordinate); + pci_disable_device(func->pci_dev); + } + } + } + + list_for_each (l, &slot->funcs) { + func = list_entry(l, struct acpiphp_func, sibling); + + acpiphp_unconfigure_ioapics(func->handle); acpiphp_bus_trim(func->handle); /* try to remove anyway. * acpiphp_bus_add might have been failed */ diff -puN drivers/pci/hotplug/acpiphp.h~acpiphp-fix drivers/pci/hotplug/acpiphp.h --- linux-2.6.18-1.2732.el5/drivers/pci/hotplug/acpiphp.h~acpiphp-fix 2006-10-19 13:05:59.000000000 -0400 +++ linux-2.6.18-1.2732.el5-kei/drivers/pci/hotplug/acpiphp.h 2006-10-19 13:08:08.000000000 -0400 @@ -150,6 +150,11 @@ struct acpiphp_attention_info struct module *owner; }; +struct acpiphp_ioapic { + struct pci_dev *dev; + u32 gsi_base; + struct list_head list; +}; /* PCI bus bridge HID */ #define ACPI_PCI_HOST_HID "PNP0A03" diff -puN drivers/pci/remove.c~acpiphp-fix drivers/pci/remove.c --- linux-2.6.18-1.2732.el5/drivers/pci/remove.c~acpiphp-fix 2006-10-19 13:06:05.000000000 -0400 +++ linux-2.6.18-1.2732.el5-kei/drivers/pci/remove.c 2006-10-19 13:07:44.000000000 -0400 @@ -16,8 +16,11 @@ static void pci_free_resources(struct pc } } -static void pci_destroy_dev(struct pci_dev *dev) +static void pci_stop_dev(struct pci_dev *dev) { + if (!dev->global_list.next) + return; + if (!list_empty(&dev->global_list)) { pci_proc_detach_device(dev); pci_remove_sysfs_dev_files(dev); @@ -27,6 +30,11 @@ static void pci_destroy_dev(struct pci_d dev->global_list.next = dev->global_list.prev = NULL; up_write(&pci_bus_sem); } +} + +static void pci_destroy_dev(struct pci_dev *dev) +{ + pci_stop_dev(dev); /* Remove the device from the device lists, and prevent any further * list accesses from this device */ @@ -119,5 +127,32 @@ void pci_remove_behind_bridge(struct pci } } +static void pci_stop_bus_devices(struct pci_bus *bus) +{ + struct list_head *l, *n; + + list_for_each_safe(l, n, &bus->devices) { + struct pci_dev *dev = pci_dev_b(l); + pci_stop_bus_device(dev); + } +} + +/** + * pci_stop_bus_device - stop a PCI device and any children + * @dev: the device to stop + * + * Stop a PCI device (detach the driver, remove from the global list + * and so on). This also stop any subordinate buses and children in a + * depth-first manner. + */ +void pci_stop_bus_device(struct pci_dev *dev) +{ + if (dev->subordinate) + pci_stop_bus_devices(dev->subordinate); + + pci_stop_dev(dev); +} + EXPORT_SYMBOL(pci_remove_bus_device); EXPORT_SYMBOL(pci_remove_behind_bridge); +EXPORT_SYMBOL_GPL(pci_stop_bus_device); diff -puN include/linux/pci.h~acpiphp-fix include/linux/pci.h --- linux-2.6.18-1.2732.el5/include/linux/pci.h~acpiphp-fix 2006-10-19 13:06:12.000000000 -0400 +++ linux-2.6.18-1.2732.el5-kei/include/linux/pci.h 2006-10-19 13:07:44.000000000 -0400 @@ -436,6 +436,7 @@ extern struct pci_dev *pci_dev_get(struc extern void pci_dev_put(struct pci_dev *dev); extern void pci_remove_bus(struct pci_bus *b); extern void pci_remove_bus_device(struct pci_dev *dev); +extern void pci_stop_bus_device(struct pci_dev *dev); void pci_setup_cardbus(struct pci_bus *bus); /* Generic PCI functions exported to card drivers */ diff -puN arch/ia64/pci/pci.c~acpiphp-fix arch/ia64/pci/pci.c --- linux-2.6.18-1.2732.el5/arch/ia64/pci/pci.c~acpiphp-fix 2006-10-19 13:06:19.000000000 -0400 +++ linux-2.6.18-1.2732.el5-kei/arch/ia64/pci/pci.c 2006-10-19 13:08:13.000000000 -0400 @@ -562,7 +562,8 @@ pcibios_enable_device (struct pci_dev *d void pcibios_disable_device (struct pci_dev *dev) { - acpi_pci_irq_disable(dev); + if (dev->is_enabled) + acpi_pci_irq_disable(dev); } void diff -puN drivers/pci/setup-bus.c~acpiphp-fix drivers/pci/setup-bus.c --- linux-2.6.18-1.2732.el5/drivers/pci/setup-bus.c~acpiphp-fix 2006-10-19 13:06:28.000000000 -0400 +++ linux-2.6.18-1.2732.el5-kei/drivers/pci/setup-bus.c 2006-10-19 13:08:01.000000000 -0400 @@ -57,10 +57,17 @@ pbus_assign_resources_sorted(struct pci_ /* Don't touch classless devices or host bridges or ioapics. */ if (class == PCI_CLASS_NOT_DEFINED || - class == PCI_CLASS_BRIDGE_HOST || - class == PCI_CLASS_SYSTEM_PIC) + class == PCI_CLASS_BRIDGE_HOST) continue; + /* Don't touch ioapic devices already enabled by firmware */ + if (class == PCI_CLASS_SYSTEM_PIC) { + u16 command; + pci_read_config_word(dev, PCI_COMMAND, &command); + if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) + continue; + } + pdev_sort_resources(dev, &head); } _