| Message ID | CANVTcTZOQWLHOs5+DpopTDMpMf2dgAzjUGEv4gGd4Z2Le5TM7A@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
FWIW, I'm using my "I/O Hook" (http://lwn.net/Articles/561359/) as a tool to test yinghai's IOAPIC patchset. It can be used to test any kind of hotplug. For people who don't yet have full hardware support and want to test the hotplug of IOAPIC, CPU, Memory, IOH, PCIe root, etc., it is an ideal tool. The latest version (v2) is at: http://lwn.net/Articles/561779/ Thanks Rui -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 27, 2013 at 3:25 AM, rui wang <ruiv.wang@gmail.com> wrote: > On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote: >> We need to have ioapic setup before normal pci drivers. >> otherwise other pci driver can not setup irq. >> >> So we should not treat them as normal pci devices. >> Also we will need to support ioapic hotplug without pci device around. >> >> We need to call ioapic add/remove during host-bridge add/remove. >> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> --- >> drivers/acpi/pci_root.c | 4 ++ >> drivers/pci/ioapic.c | 147 >> ++++++++++++++++++++++++++++++----------------- >> include/linux/pci-acpi.h | 8 +++ >> 3 files changed, 106 insertions(+), 53 deletions(-) >> >> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >> index 5917839..7577175 100644 >> --- a/drivers/acpi/pci_root.c >> +++ b/drivers/acpi/pci_root.c >> @@ -532,6 +532,8 @@ static int acpi_pci_root_add(struct acpi_device >> *device, >> pci_enable_bridges(root->bus); >> } >> >> + acpi_pci_ioapic_add(root); >> + >> pci_bus_add_devices(root->bus); >> return 1; >> >> @@ -546,6 +548,8 @@ static void acpi_pci_root_remove(struct acpi_device >> *device) >> >> pci_stop_root_bus(root->bus); >> >> + acpi_pci_ioapic_remove(root); >> + >> device_set_run_wake(root->bus->bridge, false); >> pci_acpi_remove_bus_pm_notifier(device); >> >> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c >> index 7d6b157..60351b2 100644 >> --- a/drivers/pci/ioapic.c >> +++ b/drivers/pci/ioapic.c >> @@ -22,101 +22,142 @@ >> #include <linux/slab.h> >> #include <acpi/acpi_bus.h> >> >> -struct ioapic { >> - acpi_handle handle; >> +struct acpi_pci_ioapic { >> + acpi_handle root_handle; >> u32 gsi_base; >> + struct pci_dev *pdev; >> + struct list_head list; >> }; >> >> -static int ioapic_probe(struct pci_dev *dev, const struct pci_device_id >> *ent) >> +static LIST_HEAD(ioapic_list); >> +static DEFINE_MUTEX(ioapic_list_lock); >> + >> +static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev, >> + u32 *pgsi_base) >> { >> - acpi_handle handle; >> acpi_status status; >> unsigned long long gsb; >> - struct ioapic *ioapic; >> + struct pci_dev *dev; >> + u32 gsi_base; >> int ret; >> char *type; >> - struct resource *res; >> + struct resource r; >> + struct resource *res = &r; >> + char objname[64]; >> + struct acpi_buffer buffer = {sizeof(objname), objname}; >> >> - handle = DEVICE_ACPI_HANDLE(&dev->dev); >> - if (!handle) >> - return -EINVAL; >> + *pdev = NULL; >> + *pgsi_base = 0; >> >> status = acpi_evaluate_integer(handle, "_GSB", NULL, &gsb); >> - if (ACPI_FAILURE(status)) >> - return -EINVAL; >> - >> - /* >> - * The previous code in acpiphp evaluated _MAT if _GSB failed, but >> - * ACPI spec 4.0 sec 6.2.2 requires _GSB for hot-pluggable I/O APICs. >> - */ >> + if (ACPI_FAILURE(status) || !gsb) >> + return; >> >> - ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL); >> - if (!ioapic) >> - return -ENOMEM; >> + dev = acpi_get_pci_dev(handle); >> + if (!dev) >> + return; >> >> - ioapic->handle = handle; >> - ioapic->gsi_base = (u32) gsb; >> + acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); >> >> - if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC) >> - type = "IOAPIC"; >> - else >> - type = "IOxAPIC"; >> + gsi_base = gsb; >> + type = "IOxAPIC"; >> >> ret = pci_enable_device(dev); >> if (ret < 0) >> - goto exit_free; >> + goto exit_put; >> >> pci_set_master(dev); >> >> + if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC) >> + type = "IOAPIC"; >> + >> if (pci_request_region(dev, 0, type)) >> goto exit_disable; >> >> res = &dev->resource[0]; >> - if (acpi_register_ioapic(ioapic->handle, res->start, ioapic->gsi_base)) >> + >> + if (acpi_register_ioapic(handle, res->start, gsi_base)) >> goto exit_release; > > Here acpi_register_ioapic() fails for IOAPICs already described by the > static MADT. So they won't be added to the ioapic_list. Subsequent > hot-removal will also fail because they're not found by > acpi_pci_ioapic_remove() on the ioapic_list. > > Here's what I used to fix it: > > From 2fff3297b4c71c88d92b04ba9ad0a8931b3e99b8 Mon Sep 17 00:00:00 2001 > From: Rui Wang <rui.y.wang@intel.com> > Date: Tue, 27 Aug 2013 11:23:43 -0400 > Subject: [PATCH] Hotadd of IOAPICs described in static MADT > > For IOAPICs described in static MADT, we already called __mp_register_ioapic() > in arch_early_irq_init(). During boot PCI root hotadd will call it again and > will find it already registered, thus register_ioapic() won't add it to the > ioapic_list. Subsequent hot-removal will also fail because it is not > found on the > ioapic_list. > > Signed-off-by: Rui Wang <rui.y.wang@intel.com> > --- > arch/x86/kernel/apic/io_apic.c | 4 +++- > drivers/pci/ioapic.c | 3 ++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c > index fb9bf06..15ab9f1 100644 > --- a/arch/x86/kernel/apic/io_apic.c > +++ b/arch/x86/kernel/apic/io_apic.c > @@ -3956,8 +3956,10 @@ int __mp_register_ioapic(int id, u32 address, > u32 gsi_base, bool hotadd) > > /* already registered ? */ > idx = __mp_find_ioapic(gsi_base); > - if (idx >= 0) > + if (idx >= 0) { > + ret = -EEXIST; > goto out; > + } > > idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS); > if (idx >= MAX_IO_APICS) { > diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c > index 41f7c69..83b002b 100644 > --- a/drivers/pci/ioapic.c > +++ b/drivers/pci/ioapic.c > @@ -126,7 +126,8 @@ static void handle_ioapic_add(acpi_handle handle, > struct pci_dev **pdev, > } > } > > - if (acpi_register_ioapic(handle, res->start, gsi_base)) { > + ret = acpi_register_ioapic(handle, res->start, gsi_base); > + if (ret && ret != -EEXIST) { > if (dev) > goto exit_release; > return; > -- ok, will put it my series. Thanks Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/28/13, Yinghai Lu <yinghai@kernel.org> wrote: > On Tue, Aug 27, 2013 at 3:25 AM, rui wang <ruiv.wang@gmail.com> wrote: >> On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote: >>> We need to have ioapic setup before normal pci drivers. >>> otherwise other pci driver can not setup irq. >>> >>> So we should not treat them as normal pci devices. >>> Also we will need to support ioapic hotplug without pci device around. >>> >>> We need to call ioapic add/remove during host-bridge add/remove. >>> >>> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >>> --- >>> drivers/acpi/pci_root.c | 4 ++ >>> drivers/pci/ioapic.c | 147 >>> ++++++++++++++++++++++++++++++----------------- >>> include/linux/pci-acpi.h | 8 +++ >>> 3 files changed, 106 insertions(+), 53 deletions(-) >>> >>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >>> index 5917839..7577175 100644 >>> --- a/drivers/acpi/pci_root.c >>> +++ b/drivers/acpi/pci_root.c >>> @@ -532,6 +532,8 @@ static int acpi_pci_root_add(struct acpi_device >>> *device, >>> pci_enable_bridges(root->bus); >>> } >>> >>> + acpi_pci_ioapic_add(root); >>> + >>> pci_bus_add_devices(root->bus); >>> return 1; >>> >>> @@ -546,6 +548,8 @@ static void acpi_pci_root_remove(struct acpi_device >>> *device) >>> >>> pci_stop_root_bus(root->bus); >>> >>> + acpi_pci_ioapic_remove(root); >>> + >>> device_set_run_wake(root->bus->bridge, false); >>> pci_acpi_remove_bus_pm_notifier(device); >>> >>> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c >>> index 7d6b157..60351b2 100644 >>> --- a/drivers/pci/ioapic.c >>> +++ b/drivers/pci/ioapic.c >>> @@ -22,101 +22,142 @@ >>> #include <linux/slab.h> >>> #include <acpi/acpi_bus.h> >>> >>> -struct ioapic { >>> - acpi_handle handle; >>> +struct acpi_pci_ioapic { >>> + acpi_handle root_handle; >>> u32 gsi_base; >>> + struct pci_dev *pdev; >>> + struct list_head list; >>> }; >>> >>> -static int ioapic_probe(struct pci_dev *dev, const struct pci_device_id >>> *ent) >>> +static LIST_HEAD(ioapic_list); >>> +static DEFINE_MUTEX(ioapic_list_lock); >>> + >>> +static void handle_ioapic_add(acpi_handle handle, struct pci_dev >>> **pdev, >>> + u32 *pgsi_base) >>> { >>> - acpi_handle handle; >>> acpi_status status; >>> unsigned long long gsb; >>> - struct ioapic *ioapic; >>> + struct pci_dev *dev; >>> + u32 gsi_base; >>> int ret; >>> char *type; >>> - struct resource *res; >>> + struct resource r; >>> + struct resource *res = &r; >>> + char objname[64]; >>> + struct acpi_buffer buffer = {sizeof(objname), objname}; >>> >>> - handle = DEVICE_ACPI_HANDLE(&dev->dev); >>> - if (!handle) >>> - return -EINVAL; >>> + *pdev = NULL; >>> + *pgsi_base = 0; >>> >>> status = acpi_evaluate_integer(handle, "_GSB", NULL, &gsb); >>> - if (ACPI_FAILURE(status)) >>> - return -EINVAL; >>> - >>> - /* >>> - * The previous code in acpiphp evaluated _MAT if _GSB failed, but >>> - * ACPI spec 4.0 sec 6.2.2 requires _GSB for hot-pluggable I/O >>> APICs. >>> - */ >>> + if (ACPI_FAILURE(status) || !gsb) >>> + return; >>> >>> - ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL); >>> - if (!ioapic) >>> - return -ENOMEM; >>> + dev = acpi_get_pci_dev(handle); >>> + if (!dev) >>> + return; >>> >>> - ioapic->handle = handle; >>> - ioapic->gsi_base = (u32) gsb; >>> + acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); >>> >>> - if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC) >>> - type = "IOAPIC"; >>> - else >>> - type = "IOxAPIC"; >>> + gsi_base = gsb; >>> + type = "IOxAPIC"; >>> >>> ret = pci_enable_device(dev); >>> if (ret < 0) >>> - goto exit_free; >>> + goto exit_put; >>> >>> pci_set_master(dev); >>> >>> + if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC) >>> + type = "IOAPIC"; >>> + >>> if (pci_request_region(dev, 0, type)) >>> goto exit_disable; >>> >>> res = &dev->resource[0]; >>> - if (acpi_register_ioapic(ioapic->handle, res->start, >>> ioapic->gsi_base)) >>> + >>> + if (acpi_register_ioapic(handle, res->start, gsi_base)) >>> goto exit_release; >> >> Here acpi_register_ioapic() fails for IOAPICs already described by the >> static MADT. So they won't be added to the ioapic_list. Subsequent >> hot-removal will also fail because they're not found by >> acpi_pci_ioapic_remove() on the ioapic_list. >> >> Here's what I used to fix it: >> >> From 2fff3297b4c71c88d92b04ba9ad0a8931b3e99b8 Mon Sep 17 00:00:00 2001 >> From: Rui Wang <rui.y.wang@intel.com> >> Date: Tue, 27 Aug 2013 11:23:43 -0400 >> Subject: [PATCH] Hotadd of IOAPICs described in static MADT >> >> For IOAPICs described in static MADT, we already called >> __mp_register_ioapic() >> in arch_early_irq_init(). During boot PCI root hotadd will call it again >> and >> will find it already registered, thus register_ioapic() won't add it to >> the >> ioapic_list. Subsequent hot-removal will also fail because it is not >> found on the >> ioapic_list. >> >> Signed-off-by: Rui Wang <rui.y.wang@intel.com> >> --- >> arch/x86/kernel/apic/io_apic.c | 4 +++- >> drivers/pci/ioapic.c | 3 ++- >> 2 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kernel/apic/io_apic.c >> b/arch/x86/kernel/apic/io_apic.c >> index fb9bf06..15ab9f1 100644 >> --- a/arch/x86/kernel/apic/io_apic.c >> +++ b/arch/x86/kernel/apic/io_apic.c >> @@ -3956,8 +3956,10 @@ int __mp_register_ioapic(int id, u32 address, >> u32 gsi_base, bool hotadd) >> >> /* already registered ? */ >> idx = __mp_find_ioapic(gsi_base); >> - if (idx >= 0) >> + if (idx >= 0) { >> + ret = -EEXIST; >> goto out; >> + } >> >> idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS); >> if (idx >= MAX_IO_APICS) { >> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c >> index 41f7c69..83b002b 100644 >> --- a/drivers/pci/ioapic.c >> +++ b/drivers/pci/ioapic.c >> @@ -126,7 +126,8 @@ static void handle_ioapic_add(acpi_handle handle, >> struct pci_dev **pdev, >> } >> } >> >> - if (acpi_register_ioapic(handle, res->start, gsi_base)) { >> + ret = acpi_register_ioapic(handle, res->start, gsi_base); >> + if (ret && ret != -EEXIST) { >> if (dev) >> goto exit_release; >> return; >> -- > > ok, will put it my series. > That would be great. Thanks. Rui > Thanks > > Yinghai > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index fb9bf06..15ab9f1 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -3956,8 +3956,10 @@ int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hotadd) /* already registered ? */ idx = __mp_find_ioapic(gsi_base); - if (idx >= 0) + if (idx >= 0) { + ret = -EEXIST; goto out; + } idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS); if (idx >= MAX_IO_APICS) { diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c index 41f7c69..83b002b 100644 --- a/drivers/pci/ioapic.c +++ b/drivers/pci/ioapic.c @@ -126,7 +126,8 @@ static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev, } } - if (acpi_register_ioapic(handle, res->start, gsi_base)) { + ret = acpi_register_ioapic(handle, res->start, gsi_base); + if (ret && ret != -EEXIST) { if (dev) goto exit_release;