Skip to contentSkip to conversation

ACPI based I/O APIC hot-plug

PATCH In review Kenji Kaneshige 3 participants 6 messages Apr 21, 2005 – Apr 25, 2005

Subsystems (2)
ACPI PCI
lore
Share & tools
lore

Kenji Kaneshige

author
#0 lore raw

PATCH 3/3 ACPI based I/O APIC hot-plug

This patch adds PCI based I/O xAPIC hot-add support to ACPIPHP driver. When PCI root bridge is hot-added, all PCI based I/O xAPICs under the root bridge are hot-added by this patch. Hot-remove support is TBD.

Signed-off-by Kenji Kaneshige <[email protected]>

---

 linux-2.6.12-rc2-mm3-kanesige/drivers/pci/hotplug/acpiphp_glue.c |  120 ++++++++++
 1 files changed, 120 insertions(+)
drivers/pci/hotplug/acpiphp_glue.c renamed from drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add +120 -0
diff -puN drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add drivers/pci/hotplug/acpiphp_glue.c
@@ -552,6 +552,125 @@ static void remove_bridge(acpi_handle ha
 	}
 }
 
+static struct pci_dev *get_apic_pci_info(acpi_handle handle)
+{
+	struct acpi_pci_id id;
+	struct pci_bus *bus;
+	struct pci_dev *dev;
+
+	if (ACPI_FAILURE(acpi_get_pci_id(handle, &id)))
+		return NULL;
+
+	bus = pci_find_bus(id.segment, id.bus);
+	if (!bus)
+		return NULL;
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		if (dev->devfn != PCI_DEVFN(id.device, id.function))
+			continue;
+		if ((dev->class >> 8) != PCI_CLASS_SYSTEM_PIC)
+			continue;
+		if ((dev->class & 0xff) == 0x10 || (dev->class & 0xff) == 0x20)
+			return dev;
+	}
+	return NULL;
+}
+
+static int get_gsi_base(acpi_handle handle, u32 *gsi_base)
+{
+	acpi_status status;
+	int result = -1;
+	unsigned long gsb;
+	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+	union acpi_object *obj;
+	void *table;
+
+	status = acpi_evaluate_integer(handle, "_GSB", NULL, &gsb);
+	if (ACPI_SUCCESS(status)) {
+		*gsi_base = (u32)gsb;
+		return 0;
+	}
+
+	status = acpi_evaluate_object(handle, "_MAT", NULL, &buffer);
+	if (ACPI_FAILURE(status) || !buffer.length || !buffer.pointer)
+		return result;
+
+	obj = buffer.pointer;
+	if (obj->type != ACPI_TYPE_BUFFER)
+		goto out;
+
+	table = obj->buffer.pointer;
+	switch (((acpi_table_entry_header *)table)->type) {
+	case ACPI_MADT_IOSAPIC:
+		*gsi_base = ((struct acpi_table_iosapic *)table)->global_irq_base;
+		result = 0;
+		break;
+	case ACPI_MADT_IOAPIC:
+		*gsi_base = ((struct acpi_table_ioapic *)table)->global_irq_base;
+		result = 0;
+		break;
+	default:
+		break;
+	}
+ out:
+	acpi_os_free(buffer.pointer);
+	return result;
+}
+
+static acpi_status
+ioapic_add(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	acpi_status status;
+	unsigned long sta;
+	acpi_handle tmp;
+	struct pci_dev *pdev;
+	u32 gsi_base;
+	u64 phys_addr;
+
+	/* Evaluate _STA if present */
+	status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
+	if (ACPI_SUCCESS(status) && sta != ACPI_STA_ALL)
+		return AE_CTRL_DEPTH;
+
+	/* Scan only PCI bus scope */
+	status = acpi_get_handle(handle, "_HID", &tmp);
+	if (ACPI_SUCCESS(status))
+		return AE_CTRL_DEPTH;
+
+	pdev = get_apic_pci_info(handle);
+	if (!pdev)
+		return AE_OK;
+
+	if (get_gsi_base(handle, &gsi_base))
+		return AE_OK;
+
+	if (pci_enable_device(pdev))
+		return AE_OK;
+
+	pci_set_master(pdev);
+
+	if (pci_request_region(pdev, 0, "I/O APIC(acpiphp)")) {
+		pci_disable_device(pdev);
+		return AE_OK;
+	}
+
+	phys_addr = pci_resource_start(pdev, 0);
+	if (acpi_register_ioapic(handle, phys_addr, gsi_base)) {
+		pci_release_region(pdev, 0);
+		pci_disable_device(pdev);
+		return AE_OK;
+	}
+
+	return AE_OK;
+}
+
+static int acpiphp_configure_ioapics(acpi_handle handle)
+{
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
+			    ACPI_UINT32_MAX, ioapic_add, NULL, NULL);
+	return 0;
+}
+
 static int power_on_slot(struct acpiphp_slot *slot)
 {
 	acpi_status status;

@@ -942,6 +1061,7 @@ static int acpiphp_configure_bridge (acp
 	acpiphp_sanitize_bus(bus);
 	acpiphp_set_hpp_values(handle, bus);
 	pci_enable_bridges(bus);
+	acpiphp_configure_ioapics(handle);
 	return 0;
 }

_

#1 lore raw
Quoted 1 line

+static struct pci_dev *get_apic_pci_info(acpi_handle handle)

Nitpick: follow function declaration style of file.

Quoted 4 lines
drivers/pci/hotplug/acpiphp_glue.c renamed from drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add +4 -0
+	struct acpi_pci_id id;
+	struct pci_bus *bus;
+	struct pci_dev *dev;
+

drivers/pci/hotplug/acpiphp_glue.c static void remove_bridge(acpi_handle ha · line 570

Quoted 10 lines
drivers/pci/hotplug/acpiphp_glue.c renamed from drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add +10 -0
+	if (ACPI_FAILURE(acpi_get_pci_id(handle, &id)))
+		return NULL;
+
+	bus = pci_find_bus(id.segment, id.bus);
+	if (!bus)
+		return NULL;
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		if (dev->devfn != PCI_DEVFN(id.device, id.function))
+			continue;

Use pci_get_slot() here rather than walking bus->devices yourself.

drivers/pci/hotplug/acpiphp_glue.c static void remove_bridge(acpi_handle ha · line 573

Quoted 3 lines
drivers/pci/hotplug/acpiphp_glue.c renamed from drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add +3 -0
+		if ((dev->class >> 8) != PCI_CLASS_SYSTEM_PIC)
+			continue;
+		if ((dev->class & 0xff) == 0x10 || (dev->class & 0xff) == 0x20)

What are 0x10 and 0x20? Looks like they should be #defines in include/linux/pci_ids.h.

Quoted 8 lines
drivers/pci/hotplug/acpiphp_glue.c renamed from drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add +8 -0
+static int get_gsi_base(acpi_handle handle, u32 *gsi_base)
+{
+	acpi_status status;
+	int result = -1;
+	unsigned long gsb;
+	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+	union acpi_object *obj;
+	void *table;

drivers/pci/hotplug/acpiphp_glue.c static void remove_bridge(acpi_handle ha · line 596

Quoted 10 lines
drivers/pci/hotplug/acpiphp_glue.c renamed from drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add +10 -0
+
+	status = acpi_evaluate_integer(handle, "_GSB", NULL, &gsb);
+	if (ACPI_SUCCESS(status)) {
+		*gsi_base = (u32)gsb;
+		return 0;
+	}
+
+	status = acpi_evaluate_object(handle, "_MAT", NULL, &buffer);
+	if (ACPI_FAILURE(status) || !buffer.length || !buffer.pointer)
+		return result;

Nothing can modify result before this point, so it'd be clearer to just "return -1" here.

author
#2 lore raw
Quoted 8 lines · Bjorn Helgaas

drivers/pci/hotplug/acpiphp_glue.c static void remove_bridge(acpi_handle ha · line 573

Quoted 4 lines
drivers/pci/hotplug/acpiphp_glue.c renamed from drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add +3 -0
+		if ((dev->class >> 8) != PCI_CLASS_SYSTEM_PIC)
+			continue;
+		if ((dev->class & 0xff) == 0x10 || (dev->class & 0xff) == 0x20)

What are 0x10 and 0x20? Looks like they should be #defines in include/linux/pci_ids.h.

0x10 and 0x20 are programing interfaces for I/O APIC and I/O xAPIC respectively. #define for these values looks good. But I don't know if I can put new #defines into pci_ids.h and how to name them because I could not find the header file (including pci_ids.h) that #defines the values for programming interfaces. So I want to add the comments to explain these values (0x10, 0x20) instead of adding new #defines into pci_ids.h for now.

Thanks,
Kenji Kaneshige

#3 lore raw
Quoted 10 lines · Kenji Kaneshige
Quoted 8 lines · Bjorn Helgaas

drivers/pci/hotplug/acpiphp_glue.c static void remove_bridge(acpi_handle ha · line 573

Quoted 4 lines
drivers/pci/hotplug/acpiphp_glue.c renamed from drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add +3 -0
+		if ((dev->class >> 8) != PCI_CLASS_SYSTEM_PIC)
+			continue;
+		if ((dev->class & 0xff) == 0x10 || (dev->class & 0xff) == 0x20)

What are 0x10 and 0x20? Looks like they should be #defines in include/linux/pci_ids.h.

Quoted 7 lines

0x10 and 0x20 are programing interfaces for I/O APIC and I/O xAPIC respectively. #define for these values looks good. But I don't know if I can put new #defines into pci_ids.h and how to name them because I could not find the header file (including pci_ids.h) that #defines the values for programming interfaces. So I want to add the comments to explain these values (0x10, 0x20) instead of adding new #defines into pci_ids.h for now.

I think your patch should just add the values to pci_ids.h. If somebody doesn't like that, he or she will complain and you can fall back to just using 0x10 and 0x20. But I suspect it will be fine.

#4 lore raw
Quoted 19 lines · Bjorn Helgaas
Quoted 10 lines · Kenji Kaneshige
Quoted 8 lines · Bjorn Helgaas

drivers/pci/hotplug/acpiphp_glue.c static void remove_bridge(acpi_handle ha · line 573

Quoted 4 lines
drivers/pci/hotplug/acpiphp_glue.c renamed from drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add +3 -0
+		if ((dev->class >> 8) != PCI_CLASS_SYSTEM_PIC)
+			continue;
+		if ((dev->class & 0xff) == 0x10 || (dev->class & 0xff) == 0x20)

What are 0x10 and 0x20? Looks like they should be #defines in include/linux/pci_ids.h.

Quoted 7 lines

0x10 and 0x20 are programing interfaces for I/O APIC and I/O xAPIC respectively. #define for these values looks good. But I don't know if I can put new #defines into pci_ids.h and how to name them because I could not find the header file (including pci_ids.h) that #defines the values for programming interfaces. So I want to add the comments to explain these values (0x10, 0x20) instead of adding new #defines into pci_ids.h for now.

Quoted 3 lines

I think your patch should just add the values to pci_ids.h. If somebody doesn't like that, he or she will complain and you can fall back to just using 0x10 and 0x20. But I suspect it will be fine.

Yes, I have no problem with that.

thanks,

greg k-h

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

author
#5 lore raw
In reply to Bjorn Helgaas
Quoted 21 lines · Bjorn Helgaas
Quoted 10 lines · Kenji Kaneshige
Quoted 7 lines · Bjorn Helgaas

drivers/pci/hotplug/acpiphp_glue.c static void remove_bridge(acpi_handle ha · line 573

Quoted 3 lines
drivers/pci/hotplug/acpiphp_glue.c renamed from drivers/pci/hotplug/acpiphp_glue.c~acpiphp_ioapic_add +3 -0
+		if ((dev->class >> 8) != PCI_CLASS_SYSTEM_PIC)
+			continue;
+		if ((dev->class & 0xff) == 0x10 || (dev->class & 0xff) == 0x20)

What are 0x10 and 0x20? Looks like they should be #defines in include/linux/pci_ids.h.

Quoted 7 lines

0x10 and 0x20 are programing interfaces for I/O APIC and I/O xAPIC respectively. #define for these values looks good. But I don't know if I can put new #defines into pci_ids.h and how to name them because I could not find the header file (including pci_ids.h) that #defines the values for programming interfaces. So I want to add the comments to explain these values (0x10, 0x20) instead of adding new #defines into pci_ids.h for now.

Quoted 5 lines

I think your patch should just add the values to pci_ids.h. If somebody doesn't like that, he or she will complain and you can fall back to just using 0x10 and 0x20. But I suspect it will be fine.

Thank you for advice. I'll try to add new #defines for I/O APIC and I/O xAPIC to pci_ids.h.

Thanks,
Kenji Kaneshige