Re: [PATCH v4 27/28] PCI, x86, ACPI: Enable ioapic hotplug supportwith acpi host bridge.

[rui wang]

On 8/28/13, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
> On Tue, Aug 27, 2013 at 3:25 AM, rui wang <ruiv.wang@xxxxxxxxx> wrote:
>> On 8/11/13, Yinghai Lu <yinghai@xxxxxxxxxx> 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@xxxxxxxxxx>
>>> ---
>>>  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@xxxxxxxxx>
>> 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@xxxxxxxxx>
>> ---
>>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/