[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANVTcTbD472CWTEM1URDE3Pf0YjMmukSYMkewZ0kFiKkKfcdEQ@mail.gmail.com>
Date: Wed, 28 Aug 2013 16:25:38 +0800
From: rui wang <ruiv.wang@...il.com>
To: Yinghai Lu <yinghai@...nel.org>
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 8/28/13, Yinghai Lu <yinghai@...nel.org> wrote:
> 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.
>
That would be great. Thanks.
Rui
> 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