From b9ea1e921c2721546f53270cffdc6866e20576dd Mon Sep 17 00:00:00 2001 From: Sasha Levin Date: Wed, 28 Oct 2026 23:45:06 -0400 Subject: ACPI: scan: Print the offline failure message in acpi_bus_offline() When a physical node of an ACPI device being hot-removed cannot be offlined, acpi_bus_offline() passes the physical device back to acpi_scan_try_to_offline() through ret_p and the latter prints "Offline failed." for it after the namespace walk. By then physical_node_lock is not held anymore, so the device may have been unbound from its ACPI companion and freed in the meantime. Print the message in acpi_bus_offline() when device_offline() fails in the second pass, while physical_node_lock keeps the device bound to its ACPI companion and so the reference taken by acpi_bind_one() is still held. acpi_scan_try_to_offline() only needs to know that something could not be offlined, so pass the ACPI device back through ret_p instead, like in the "Offline disabled." case. Fixes: 303bfdb1a14d ("ACPI / scan: Add second pass of companion offlining to hot-remove code") Suggested-by: Rafael J. Wysocki Signed-off-by: Sasha Levin Link: https://patch.msgid.link/20261029034507.2721580-2-sashal@kernel.org Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 96b8b0dfe339..b62f0b7f9809 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -166,8 +166,9 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data, if (ret >= 0) { pn->put_online = !ret; } else { - *ret_p = pn->dev; + *ret_p = &device->dev; if (second_pass) { + dev_warn(pn->dev, "Offline failed.\n"); status = AE_ERROR; break; } @@ -236,7 +237,6 @@ static int acpi_scan_try_to_offline(struct acpi_device *device) (void **)&errdev); if (errdev) { - dev_warn(errdev, "Offline failed.\n"); acpi_bus_online(handle, 0, NULL, NULL); acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, acpi_bus_online, -- cgit v1.2.3 From 17654aa629cba9a70e31cc1b7226a4fc1a1c801a Mon Sep 17 00:00:00 2001 From: Sasha Levin Date: Wed, 28 Oct 2026 23:45:07 -0400 Subject: ACPI: scan: Do not hold physical_node_lock across device_offline() and device_online() acpi_bus_offline() and acpi_bus_online() hold the physical_node_lock of the ACPI device being hot-removed while they call device_offline() and device_online() for its physical nodes. Offlining a CPU this way takes cpu_add_remove_lock and the CPU hotplug lock under physical_node_lock, and lockdep connects that to the uevent attribute of platform devices, whose store method takes physical_node_lock in acpi_primary_dev_companion(): physical_node_lock (acpi_bus_offline()) -> cpu_add_remove_lock -> cpu_hotplug_lock -> cpuhp_state-down (mce_cpu_pre_down() unregistering a device) -> device_links_lock (device_links_purge()) -> kn->active of a uevent attribute (device link unregistration) -> physical_node_lock (uevent_store() -> acpi_companion_match()) syzbot has reported this cycle twice. The uevent attribute waited for in the link unregistration is the device link's, not a platform device's, and the physical_node_lock taken at the end is that of the platform device's ACPI companion, not of the processor being ejected, so lockdep only connects them because they share lock classes and this is unlikely to deadlock. However, lockdep turns itself off after reporting it. It can be reproduced in a QEMU guest by writing to the uevent attribute of an ACPI-enumerated platform device, reading the uevent attribute of a device link while the link is removed, offlining a CPU and ejecting one through sysfs. physical_node_lock protects the list of physical nodes. Since acpi_unbind_one() blocks on it, holding it also keeps device_offline() and device_online() from running in parallel with whatever happens to a device after it has been unbound from its ACPI companion, but that is not necessary: a reference to each device is held while the lock is dropped, and the removal of the devices that can actually be taken offline here (CPUs, memory blocks and containers) is serialized by the device hotplug lock or acpi_scan_lock, both of which acpi_device_hotplug() holds throughout. So drop physical_node_lock around device_offline() and device_online(). Mark the nodes to offline with a new offline_pending flag, hold a reference to each device while the lock is dropped, and look its node up again afterwards to record whether it needs to be put back online, because it may have been unbound in the meantime. Physical nodes added while the lock is not held are not offlined by the current pass, which is no different from adding them right after it. Since acpi_container_offline() no longer runs under the container's physical_node_lock, the nesting annotation added to acpi_scan_is_offline() by commit 4c533c801d1c ("ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline()") is not needed anymore, so drop it. Fixes: 683058e315f0 ("ACPI / hotplug: Use device offline/online for graceful hot-removal") Reported-by: syzbot+cea7285bc17b76ffe39f@syzkaller.appspotmail.com Closes: https://lore.kernel.org/r/6ae18ed0.6200f071.51519.0f60.GAE@google.com Reported-by: syzbot+a265816368571f78ab88@syzkaller.appspotmail.com Closes: https://lore.kernel.org/r/6adf293a.67317f00.38c4bc.00af.GAE@google.com Signed-off-by: Sasha Levin Link: https://patch.msgid.link/20261029034507.2721580-3-sashal@kernel.org Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 106 +++++++++++++++++++++++++++++++++++++++--------- include/acpi/acpi_bus.h | 1 + 2 files changed, 88 insertions(+), 19 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index b62f0b7f9809..9218264f8da1 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -115,11 +115,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent) bool offline = true; char *envp[] = { "EVENT=offline", NULL }; - /* - * acpi_container_offline() calls this for all of the container's - * children under the container's physical_node_lock lock. - */ - mutex_lock_nested(&adev->physical_node_lock, SINGLE_DEPTH_NESTING); + mutex_lock(&adev->physical_node_lock); list_for_each_entry(pn, &adev->physical_node_list, node) if (device_supports_offline(pn->dev) && !dev_offline(pn->dev)) { @@ -134,6 +130,59 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent) return offline; } +static struct acpi_device_physical_node * +acpi_find_physical_node(struct acpi_device *adev, struct device *dev) +{ + struct acpi_device_physical_node *pn; + + lockdep_assert_held(&adev->physical_node_lock); + + list_for_each_entry(pn, &adev->physical_node_list, node) + if (pn->dev == dev) + return pn; + + return NULL; +} + +static struct device *acpi_next_offline_pending(struct acpi_device *adev) +{ + struct acpi_device_physical_node *pn; + + lockdep_assert_held(&adev->physical_node_lock); + + /* + * Find the first physical node that is still to be offlined, clear its + * offline_pending flag and return its device with a reference held. + */ + list_for_each_entry(pn, &adev->physical_node_list, node) + if (pn->offline_pending) { + pn->offline_pending = false; + return get_device(pn->dev); + } + + return NULL; +} + +static struct device *acpi_next_put_online(struct acpi_device *adev) +{ + struct acpi_device_physical_node *pn; + + lockdep_assert_held(&adev->physical_node_lock); + + /* + * Find the first physical node that has been offlined by + * acpi_bus_offline(), clear its put_online flag and return its device + * with a reference held. + */ + list_for_each_entry(pn, &adev->physical_node_list, node) + if (pn->put_online) { + pn->put_online = false; + return get_device(pn->dev); + } + + return NULL; +} + static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data, void **ret_p) { @@ -141,6 +190,7 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data, struct acpi_device_physical_node *pn; bool second_pass = (bool)data; acpi_status status = AE_OK; + struct device *dev; if (!device) return AE_OK; @@ -153,26 +203,43 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data, mutex_lock(&device->physical_node_lock); list_for_each_entry(pn, &device->physical_node_list, node) { - int ret; - if (second_pass) { /* Skip devices offlined by the first pass. */ - if (pn->put_online) - continue; + pn->offline_pending = !pn->put_online; } else { + pn->offline_pending = true; pn->put_online = false; } - ret = device_offline(pn->dev); + } + + /* + * Drop physical_node_lock around device_offline(), because the latter + * acquires locks, like the CPU hotplug ones, that must not nest under + * it. + */ + while ((dev = acpi_next_offline_pending(device))) { + int ret; + + mutex_unlock(&device->physical_node_lock); + ret = device_offline(dev); + mutex_lock(&device->physical_node_lock); + if (ret >= 0) { - pn->put_online = !ret; + /* The device may have been unbound in the meantime. */ + pn = acpi_find_physical_node(device, dev); + if (pn) + pn->put_online = !ret; } else { *ret_p = &device->dev; if (second_pass) { - dev_warn(pn->dev, "Offline failed.\n"); + dev_warn(dev, "Offline failed.\n"); status = AE_ERROR; - break; } } + + put_device(dev); + if (ACPI_FAILURE(status)) + break; } mutex_unlock(&device->physical_node_lock); @@ -184,18 +251,19 @@ static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data, void **ret_p) { struct acpi_device *device = acpi_fetch_acpi_dev(handle); - struct acpi_device_physical_node *pn; + struct device *dev; if (!device) return AE_OK; mutex_lock(&device->physical_node_lock); - list_for_each_entry(pn, &device->physical_node_list, node) - if (pn->put_online) { - device_online(pn->dev); - pn->put_online = false; - } + while ((dev = acpi_next_put_online(device))) { + mutex_unlock(&device->physical_node_lock); + device_online(dev); + mutex_lock(&device->physical_node_lock); + put_device(dev); + } mutex_unlock(&device->physical_node_lock); diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index f59b2de865e6..fad52a96d613 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -327,6 +327,7 @@ struct acpi_device_physical_node { struct device *dev; unsigned int node_id; bool put_online:1; + bool offline_pending:1; }; struct acpi_device_properties { -- cgit v1.2.3 From cf12faa1e76d43f97b1486aaec959b16baca498a Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Fri, 30 Oct 2026 05:18:49 -0700 Subject: ACPI: APEI: GHES: Fix severity check in ghes_log_hwerr() ghes_log_hwerr() is meant to count recoverable GHES errors in hwerror_data[], but it skips them and counts informational records instead. As a result, recoverable CPU, memory, CXL and vendor-specific errors reported through GHES never show up in a vmcore. ghes_do_proc() passes ghes_log_hwerr() the record severity after ghes_severity() has converted it to a GHES_SEV_* value. ghes_log_hwerr() compares it against CPER_SEV_RECOVERABLE, which is 0 in the CPER enum and matches GHES_SEV_NO, so only informational records pass and GHES_SEV_RECOVERABLE ones return early. Compare against GHES_SEV_RECOVERABLE instead. Recoverable PCIe errors that ghes_handle_aer() hands to AER are also counted by pci_dev_aer_stats_incr(), so with the check fixed they are counted twice under HWERR_RECOV_PCI. That is not addressed here. Fixes: 3fa805c37dd4 ("vmcoreinfo: track and log recoverable hardware errors") Cc: stable@vger.kernel.org Signed-off-by: Breno Leitao Reviewed-by: Shuai Xue Link: https://patch.msgid.link/20261030-ghes-hwerr-sev-v1-1-145ceb996c77@debian.org Signed-off-by: Rafael J. Wysocki --- drivers/acpi/apei/ghes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 663dc2e601cf..5d888c82efea 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -899,7 +899,7 @@ EXPORT_SYMBOL_NS_GPL(cxl_cper_kfifo_get, "CXL"); static void ghes_log_hwerr(int sev, guid_t *sec_type) { - if (sev != CPER_SEV_RECOVERABLE) + if (sev != GHES_SEV_RECOVERABLE) return; if (guid_equal(sec_type, &CPER_SEC_PROC_ARM) || -- cgit v1.2.3 From 4fad962a6080266170714f77a9dc1b97fb09980f Mon Sep 17 00:00:00 2001 From: Saranya Gopal Date: Fri, 6 Nov 2026 18:28:14 +0530 Subject: ACPI: s2idle: Handle shared _Ixx GPEs without an EC When first_ec is NULL, acpi_ec_dispatch_gpe() previously treated any active GPE as a wake source. On platforms where an _Ixx GPE is shared by user-wakeup and background events, that can cause unnecessary wakeups. Make acpi_ec_dispatch_gpe() return an int and return -ENODEV when no EC instance is available so that acpi_s2idle_wake() can handle that case: on x86, treat the first GPE armed for wake via an _Ixx method like the EC GPE, mirroring acpi_ec_dispatch_gpe(). That is, report a wakeup if the status bit is set for any enabled GPE other than the _Ixx one. Otherwise, cancel the SCI-related wakeup and dispatch the _Ixx GPE if its status bit is set, so the events behind it can be processed. Drivers handling genuine wake events are expected to report them via pm_wakeup_event(), which the common s2idle path will catch after synchronizing event processing. If no _Ixx GPEs have been armed, and in the non-x86 fallback, the previous any-GPE status check is retained. If GPEs are armed via more than one _Ixx method, the first one is used as the wake indicator and the remaining ones are treated as regular wake sources. Fixes: 46767b17fda3 ("ACPI: PM: s2idle: Add _Ixx GPE wake indicator support") Signed-off-by: Saranya Gopal Link: https://patch.msgid.link/20261106125814.1112331-1-saranya.gopal@intel.com Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 10 ++++---- drivers/acpi/internal.h | 6 ++--- drivers/acpi/sleep.c | 11 +++++++-- drivers/acpi/sleep.h | 8 +++++++ drivers/acpi/x86/s2idle.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 10 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index a89f10256dbb..db24a9a252a8 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -2167,19 +2167,19 @@ static bool acpi_ec_work_in_progress(struct acpi_ec *ec) return ec->events_in_progress + ec->queries_in_progress > 0; } -bool acpi_ec_dispatch_gpe(void) +int acpi_ec_dispatch_gpe(void) { bool work_in_progress = false; if (!first_ec) - return acpi_any_gpe_status_set(U32_MAX); + return -ENODEV; /* * Report wakeup if the status bit is set for any enabled GPE other * than the EC one. */ if (acpi_any_gpe_status_set(first_ec->gpe)) - return true; + return 1; /* * Cancel the SCI wakeup and process all pending events in case there @@ -2207,7 +2207,7 @@ bool acpi_ec_dispatch_gpe(void) spin_unlock_irq(&first_ec->lock); if (!work_in_progress) - return false; + return 0; pm_pr_dbg("ACPI EC GPE dispatched\n"); @@ -2224,7 +2224,7 @@ bool acpi_ec_dispatch_gpe(void) spin_unlock_irq(&first_ec->lock); } while (work_in_progress && !pm_wakeup_pending()); - return false; + return 0; } #endif /* CONFIG_PM_SLEEP */ diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 011da4ed6a80..472c383eaaec 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -237,7 +237,7 @@ void acpi_ec_register_opregions(struct acpi_device *adev); #ifdef CONFIG_PM_SLEEP void acpi_ec_flush_work(void); -bool acpi_ec_dispatch_gpe(void); +int acpi_ec_dispatch_gpe(void); #endif #else @@ -257,9 +257,9 @@ static inline void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit static inline void acpi_ec_register_opregions(struct acpi_device *adev) {} static inline void acpi_ec_flush_work(void) {} -static inline bool acpi_ec_dispatch_gpe(void) +static inline int acpi_ec_dispatch_gpe(void) { - return false; + return -EOPNOTSUPP; } #endif diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index a94bac4422d4..641056e0252b 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -760,6 +760,8 @@ bool acpi_s2idle_wake(void) return pm_wakeup_pending(); while (pm_wakeup_pending()) { + int ret; + /* * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the * SCI has not triggered while suspended, so bail out (the @@ -788,11 +790,16 @@ bool acpi_s2idle_wake(void) /* * Check non-EC GPE wakeups and if there are none, cancel the - * SCI-related wakeup and dispatch the EC GPE. + * SCI-related wakeup and dispatch the EC GPE. If there is no + * EC to consult, check the GPEs armed for wakeup via _Ixx + * methods instead. */ - if (acpi_ec_dispatch_gpe()) { + ret = acpi_ec_dispatch_gpe(); + if (ret > 0) { pm_pr_dbg("ACPI non-EC GPE wakeup\n"); return true; + } else if (ret < 0 && acpi_x86_s2idle_check_ixx_gpes()) { + return true; } acpi_os_wait_events_complete(); diff --git a/drivers/acpi/sleep.h b/drivers/acpi/sleep.h index 9c3cb109c5d2..955e9c22bf0d 100644 --- a/drivers/acpi/sleep.h +++ b/drivers/acpi/sleep.h @@ -20,6 +20,14 @@ extern int acpi_s2idle_prepare(void); extern bool acpi_s2idle_wake(void); extern void acpi_s2idle_restore(void); extern void acpi_s2idle_end(void); +#if defined(CONFIG_X86) && defined(CONFIG_SUSPEND) +extern bool acpi_x86_s2idle_check_ixx_gpes(void); +#else +static inline bool acpi_x86_s2idle_check_ixx_gpes(void) +{ + return acpi_any_gpe_status_set(U32_MAX); +} +#endif extern void acpi_s2idle_setup(void); diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c index c278ea14c0b3..b79bf0fa9a7f 100644 --- a/drivers/acpi/x86/s2idle.c +++ b/drivers/acpi/x86/s2idle.c @@ -24,6 +24,13 @@ #ifdef CONFIG_SUSPEND +/* + * First GPE armed for wake via an _Ixx method, U32_MAX if none. On + * platforms without an ACPI-enumerated EC, this GPE plays the role of + * the EC GPE in acpi_ec_dispatch_gpe(). + */ +static u32 acpi_s2idle_ixx_gpe = U32_MAX; + static void acpi_setup_ixx_gpes(void) { acpi_handle gpe_root; @@ -47,11 +54,65 @@ static void acpi_setup_ixx_gpes(void) acpi_mark_gpe_for_wake(NULL, i); acpi_set_gpe_wake_mask(NULL, i, ACPI_GPE_ENABLE); + if (acpi_s2idle_ixx_gpe == U32_MAX) + acpi_s2idle_ixx_gpe = i; pm_pr_dbg("ACPI: GPE0x%02x armed for wake via %s\n", i, gpe_nr_str); } } +/* + * acpi_x86_s2idle_check_ixx_gpes - Handle an SCI wakeup without an EC + * + * Called to decide whether or not to wake up the system when an SCI has + * triggered during suspend-to-idle on a platform without an EC instance, + * mirroring acpi_ec_dispatch_gpe() with the first GPE armed for wake via + * an _Ixx method playing the role of the EC GPE. + * + * Report a wakeup if the status bit is set for any enabled GPE other than + * the _Ixx one. If no _Ixx GPE has been armed, acpi_s2idle_ixx_gpe is + * U32_MAX and this amounts to checking all of the GPEs, which is the + * previous behavior. + * + * Otherwise, cancel the pending SCI-related wakeup, so it does not cause + * the pm_wakeup_pending() check in the caller to succeed by itself, and + * dispatch the _Ixx GPE if it is active, so the events behind it can be + * processed. If any of those events turn out to be genuine wake events, + * the drivers handling them are expected to call pm_wakeup_event() and + * the caller will catch that via pm_wakeup_pending() after synchronizing + * the event processing. + */ +bool acpi_x86_s2idle_check_ixx_gpes(void) +{ + acpi_event_status status = 0; + + /* + * Report wakeup if the status bit is set for any enabled GPE other + * than the _Ixx one. + */ + if (acpi_any_gpe_status_set(acpi_s2idle_ixx_gpe)) + return true; + + if (acpi_s2idle_ixx_gpe == U32_MAX) + return false; + + /* + * Cancel the SCI wakeup and dispatch the _Ixx GPE in case there are + * any wakeup events behind it. + */ + pm_system_cancel_wakeup(); + + acpi_get_gpe_status(NULL, acpi_s2idle_ixx_gpe, &status); + if (status & ACPI_EVENT_FLAG_STATUS_SET) { + pm_pr_dbg("ACPI _Ixx GPE 0x%02x dispatched\n", + acpi_s2idle_ixx_gpe); + + acpi_dispatch_gpe(NULL, acpi_s2idle_ixx_gpe); + } + + return false; +} + static bool sleep_no_lps0 __read_mostly; module_param(sleep_no_lps0, bool, 0644); MODULE_PARM_DESC(sleep_no_lps0, "Do not use the special LPS0 device interface"); -- cgit v1.2.3