From 89ec2f2ee104970329139e6526a075113c92f650 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 5 Sep 2013 23:39:20 +0200 Subject: ACPI / hotplug / PCI: Don't trim devices before scanning the namespace In acpiphp_bus_add() we first remove device objects corresponding to the given handle and the ACPI namespace branch below it, which are then re-created by acpi_bus_scan(). This used to be done to clean up after surprise removals, but now we do the cleanup through trim_stale_devices() which checks if the devices in question are actually gone before removing them, so the device hierarchy trimming in acpiphp_bus_add() is not necessary any more and, moreover, it may lead to problems if it removes device objects corresponding to devices that are actually present. For this reason, remove the leftover acpiphp_bus_trim() from acpiphp_bus_add(). Reported-by: Alex Williamson Signed-off-by: Rafael J. Wysocki diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index 8054ddc..3f78212 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -487,7 +487,6 @@ static void acpiphp_bus_add(acpi_handle handle) { struct acpi_device *adev = NULL; - acpiphp_bus_trim(handle); acpi_bus_scan(handle); acpi_bus_get_device(handle, &adev); if (adev) -- cgit v0.10.2 From 2dc41281b1d1178befe4b76adf817570a7f45ec1 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 6 Sep 2013 15:41:32 +0200 Subject: ACPI / hotplug / PCI: Avoid doing too much for spurious notifies Sometimes we may get a spurious device check or bus check notify for a hotplug device and in those cases we should avoid doing all of the configuration work needed when something actually changes. To that end, check the return value of pci_scan_slot() in enable_slot() and bail out early if it is 0. This turns out to help reduce the amount of diagnostic output from the ACPIPHP subsystem and speed up boot on at least one system that generates multiple device check notifies for PCIe devices on the root bus during boot. Reported-and-tested-by: Alex Williamson Signed-off-by: Rafael J. Wysocki diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index 3f78212..6529022 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -542,12 +542,12 @@ static void __ref enable_slot(struct acpiphp_slot *slot) struct acpiphp_func *func; int max, pass; LIST_HEAD(add_list); + int nr_found; list_for_each_entry(func, &slot->funcs, sibling) acpiphp_bus_add(func_to_handle(func)); - pci_scan_slot(bus, PCI_DEVFN(slot->device, 0)); - + nr_found = pci_scan_slot(bus, PCI_DEVFN(slot->device, 0)); max = acpiphp_max_busnr(bus); for (pass = 0; pass < 2; pass++) { list_for_each_entry(dev, &bus->devices, bus_list) { @@ -566,8 +566,11 @@ static void __ref enable_slot(struct acpiphp_slot *slot) } } } - __pci_bus_assign_resources(bus, &add_list, NULL); + /* Nothing more to do here if there are no new devices on this bus. */ + if (!nr_found && (slot->flags & SLOT_ENABLED)) + return; + acpiphp_sanitize_bus(bus); acpiphp_set_hpp_values(bus); acpiphp_set_acpi_region(slot); -- cgit v0.10.2 From e532e84ea11399a6066f31641425a76dd012ce77 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 6 Sep 2013 15:41:41 +0200 Subject: ACPI / hotplug / PCI: Use _OST to notify firmware about notify status The spec suggests that we should use _OST to notify the platform about the status of notifications it sends us, for example so that it doesn't repeate a notification that has been handled already. This turns out to help reduce the amount of diagnostic output from the ACPIPHP subsystem and speed up boot on at least one system that generates multiple device check notifies for PCIe devices on the root bus during boot. Reported-and-tested-by: Alex Williamson Signed-off-by: Rafael J. Wysocki diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index 6529022..1971d29 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -870,6 +870,8 @@ static void hotplug_event_work(struct work_struct *work) hotplug_event(hp_work->handle, hp_work->type, context); acpi_scan_lock_release(); + acpi_evaluate_hotplug_ost(hp_work->handle, hp_work->type, + ACPI_OST_SC_SUCCESS, NULL); kfree(hp_work); /* allocated in handle_hotplug_event() */ put_bridge(context->func.parent); } @@ -885,11 +887,15 @@ static void hotplug_event_work(struct work_struct *work) static void handle_hotplug_event(acpi_handle handle, u32 type, void *data) { struct acpiphp_context *context; + u32 ost_code = ACPI_OST_SC_SUCCESS; switch (type) { case ACPI_NOTIFY_BUS_CHECK: case ACPI_NOTIFY_DEVICE_CHECK: + break; case ACPI_NOTIFY_EJECT_REQUEST: + ost_code = ACPI_OST_SC_EJECT_IN_PROGRESS; + acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL); break; case ACPI_NOTIFY_DEVICE_WAKE: @@ -898,20 +904,21 @@ static void handle_hotplug_event(acpi_handle handle, u32 type, void *data) case ACPI_NOTIFY_FREQUENCY_MISMATCH: acpi_handle_err(handle, "Device cannot be configured due " "to a frequency mismatch\n"); - return; + goto out; case ACPI_NOTIFY_BUS_MODE_MISMATCH: acpi_handle_err(handle, "Device cannot be configured due " "to a bus mode mismatch\n"); - return; + goto out; case ACPI_NOTIFY_POWER_FAULT: acpi_handle_err(handle, "Device has suffered a power fault\n"); - return; + goto out; default: acpi_handle_warn(handle, "Unsupported event type 0x%x\n", type); - return; + ost_code = ACPI_OST_SC_UNRECOGNIZED_NOTIFY; + goto out; } mutex_lock(&acpiphp_context_lock); @@ -920,8 +927,14 @@ static void handle_hotplug_event(acpi_handle handle, u32 type, void *data) get_bridge(context->func.parent); acpiphp_put_context(context); alloc_acpi_hp_work(handle, type, context, hotplug_event_work); + mutex_unlock(&acpiphp_context_lock); + return; } mutex_unlock(&acpiphp_context_lock); + ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; + + out: + acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL); } /* -- cgit v0.10.2 From a47d8c8e72a5fa2e69117674c4b0b6cc79c5bc53 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Sun, 8 Sep 2013 00:07:28 +0200 Subject: ACPI / hotplug / PCI: Avoid parent bus rescans on spurious device checks In the current ACPIPHP notify handler we always go directly for a rescan of the parent bus if we get a device check notification for a device that is not a bridge. However, this obviously is overzealous if nothing really changes, because this way we may rescan the whole PCI hierarchy pretty much in vain. That happens on Alex Williamson's machine whose ACPI tables contain device objects that are supposed to coresspond to PCIe root ports, but those ports aren't physically present (or at least they aren't visible in the PCI config space to us). The BIOS generates multiple device check notifies for those objects during boot and for each of them we go straight for the parent bus rescan, but the parent bus is the root bus in this particular case. In consequence, we rescan the whole PCI bus from the top several times in a row, which is completely unnecessary, increases boot time by 50% (after previous fixes) and generates excess dmesg output from the PCI subsystem. Fix the problem by checking if we can find anything new in the slot corresponding to the device we've got a device check notify for and doing nothig if that's not the case. The spec (ACPI 5.0, Section 5.6.6) appears to mandate this behavior, as it says: Device Check. Used to notify OSPM that the device either appeared or disappeared. If the device has appeared, OSPM will re-enumerate from the parent. If the device has disappeared, OSPM will invalidate the state of the device. OSPM may optimize out re-enumeration. Therefore, according to the spec, we are free to do nothing if nothing changes. References: https://bugzilla.kernel.org/show_bug.cgi?id=60865 Reported-and-tested-by: Alex Williamson Signed-off-by: Rafael J. Wysocki diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index 1971d29..9d6e535 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -528,6 +528,16 @@ static void check_hotplug_bridge(struct acpiphp_slot *slot, struct pci_dev *dev) } } +static int acpiphp_rescan_slot(struct acpiphp_slot *slot) +{ + struct acpiphp_func *func; + + list_for_each_entry(func, &slot->funcs, sibling) + acpiphp_bus_add(func_to_handle(func)); + + return pci_scan_slot(slot->bus, PCI_DEVFN(slot->device, 0)); +} + /** * enable_slot - enable, configure a slot * @slot: slot to be enabled @@ -544,10 +554,7 @@ static void __ref enable_slot(struct acpiphp_slot *slot) LIST_HEAD(add_list); int nr_found; - list_for_each_entry(func, &slot->funcs, sibling) - acpiphp_bus_add(func_to_handle(func)); - - nr_found = pci_scan_slot(bus, PCI_DEVFN(slot->device, 0)); + nr_found = acpiphp_rescan_slot(slot); max = acpiphp_max_busnr(bus); for (pass = 0; pass < 2; pass++) { list_for_each_entry(dev, &bus->devices, bus_list) { @@ -840,11 +847,22 @@ static void hotplug_event(acpi_handle handle, u32 type, void *data) case ACPI_NOTIFY_DEVICE_CHECK: /* device check */ dbg("%s: Device check notify on %s\n", __func__, objname); - if (bridge) + if (bridge) { acpiphp_check_bridge(bridge); - else - acpiphp_check_bridge(func->parent); + } else { + struct acpiphp_slot *slot = func->slot; + int ret; + /* + * Check if anything has changed in the slot and rescan + * from the parent if that's the case. + */ + mutex_lock(&slot->crit_sect); + ret = acpiphp_rescan_slot(slot); + mutex_unlock(&slot->crit_sect); + if (ret) + acpiphp_check_bridge(func->parent); + } break; case ACPI_NOTIFY_EJECT_REQUEST: -- cgit v0.10.2