[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAE9FiQX8bQsC0ezhO8ZtcS4XOuFmc50P9GeJsE3do5vd_NHycw@mail.gmail.com>
Date: Tue, 27 Aug 2013 23:22:09 -0700
From: Yinghai Lu <yinghai@...nel.org>
To: rui wang <ruiv.wang@...il.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...e.hu>,
"H. Peter Anvin" <hpa@...or.com>, Tony Luck <tony.luck@...el.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
"Rafael J. Wysocki" <rjw@...k.pl>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH v4 27/28] PCI, x86, ACPI: Enable ioapic hotplug support
with acpi host bridge.
On Tue, Aug 27, 2013 at 3:25 AM, rui wang <ruiv.wang@...il.com> wrote:
> On 8/11/13, Yinghai Lu <yinghai@...nel.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@...nel.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@...el.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@...el.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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists