Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754535Ab3H1GWO (ORCPT ); Wed, 28 Aug 2013 02:22:14 -0400 Received: from mail-oa0-f53.google.com ([209.85.219.53]:55362 "EHLO mail-oa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753132Ab3H1GWK (ORCPT ); Wed, 28 Aug 2013 02:22:10 -0400 MIME-Version: 1.0 In-Reply-To: References: <1376189294-32022-1-git-send-email-yinghai@kernel.org> <1376189294-32022-28-git-send-email-yinghai@kernel.org> Date: Tue, 27 Aug 2013 23:22:09 -0700 X-Google-Sender-Auth: DCJXGGvY3sQJPP_NimhzMahyEu8 Message-ID: Subject: Re: [PATCH v4 27/28] PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge. From: Yinghai Lu To: rui wang Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Tony Luck , Bjorn Helgaas , "Rafael J. Wysocki" , "linux-pci@vger.kernel.org" , Linux Kernel Mailing List , ACPI Devel Maling List Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6721 Lines: 204 On Tue, Aug 27, 2013 at 3:25 AM, rui wang wrote: > On 8/11/13, Yinghai Lu 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 >> --- >> 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 >> #include >> >> -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 > 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 > --- > 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/