[PATCH v3 0/1] platform/x86/intel/pmt: Notify userspace when crashlogs complete

[PATCH v3 0/1] platform/x86/intel/pmt: Notify userspace when crashlogs complete

[David E. Box]

This patch adds a uevent so userspace can find out when an Intel PMT
crashlog is available without polling.

It emits a KOBJ_CHANGE uevent on the crashlog<x> device carrying
INTEL_PMT_CRASHLOG_COMPLETE. The value is 1 when a completed crashlog is
found at probe, or when a manually triggered capture completes. The value
is 0 when a manual trigger does not complete within the 5 second poll
window. The ABI documentation is updated to match.

The main change in v3 is that the probe-time check no longer reads the
crashlog registers from probe. The register access callbacks added by
Michael's "platform/x86/intel/pmt: Add register access callbacks"
(7fdfccf87676) are there for the upcoming xe support for Crescent Island
(CRI). That support implements read_reg() and holds the lock it takes
across intel_vsec_register() when it registers late, after its firmware
is ready. With pmt_crashlog already loaded, crashlog probe runs
synchronously inside that call, and the v2 probe-time read deadlocked on
the lock its own caller held. Michael found this on CRI. In v3 the check
runs from a work item, which waits for registration to finish.

One rule follows from this and is noted in the code: a parent must not
hold its read_reg() lock while removing the crashlog device, because
remove waits for these work items. The CRI xe code only removes the
device under that lock if devm_add_action_or_reset() fails inside
intel_vsec_add_aux().

Testing: Michael tested v3 on CRI hardware. It was also tested in QEMU
on a kernel with CONFIG_PROVE_LOCKING, covering the cases listed for v2
plus disabling the crashlog during the trigger poll. A test parent
driver modelled the CRI late-bind path: read_reg() callbacks under a
mutex held across intel_vsec_register(), a slow registration that keeps
the lock held after the device appears, and a trigger that reaches
control_mutex before the probe-time check.

Tool use: an LLM coding assistant was used to analyze the deadlock,
draft the changes and the QEMU tests, and review the patch with
sashiko. I reviewed and tested the result.

V3 changes:
  - Run the probe-time check from a work item queued at the end of
    probe instead of from probe, so a read_reg() callback is never
    called from a synchronous probe that may run under the parent's
    lock (reported and tested by Michael on CRI).
  - Queue the probe-time check on the freezable workqueue, like the
    poll (sashiko).
  - Enable triggers once probe has created every entry, rather than per
    entry, so abort_probe never waits on a work item blocked on its
    caller's lock.
  - If a trigger is accepted before the probe-time check runs, the
    check skips its notification. The trigger's poll reports the
    capture, so it is not also reported as found at probe.
  - Disabling the crashlog while the trigger poll runs ends the poll
    without an event, like clear and rearm (sashiko).
  - ABI: note that events may be merged, list writing 0 to trigger and
    disabling as ending the poll, and update Date and KernelVersion.
  - Rebased onto review-ilpo-next. The patch also applies cleanly to
    for-next.

v2: https://lore.kernel.org/all/cover.1792721526.git.david.e.box@linux.intel.com/
v1: https://lore.kernel.org/all/20261001220351.1592138-1-david.e.box@linux.intel.com/

David E. Box (1):
  platform/x86/intel/pmt: Notify userspace when crashlogs complete

 .../ABI/testing/sysfs-class-intel_pmt         |  35 ++++
 drivers/platform/x86/intel/pmt/crashlog.c     | 180 +++++++++++++++++-
 2 files changed, 209 insertions(+), 6 deletions(-)


base-commit: 5680519168f65666d2c03cc6ed8fe2b7052aee12
-- 
2.53.0

[PATCH v3 1/1] platform/x86/intel/pmt: Notify userspace when crashlogs complete

[David E. Box]

A crashlog may already be complete when the driver binds, or it may
complete asynchronously after a manual trigger. Userspace currently has to
poll the status attributes to find out when data is ready.

Emit a KOBJ_CHANGE uevent on the crashlog<x> device carrying
INTEL_PMT_CRASHLOG_COMPLETE:

  - at probe, set to 1 if a completed crashlog is present
  - after a manual trigger, set to 1 when the capture completes, or 0
    if it has not completed within 5 seconds

Writing 0 to trigger, clearing, rearming or disabling the crashlog while
the trigger poll is running ends it without an event. Because the class
device's attributes are created after its KOBJ_ADD, the probe-time event is
also the first point at which userspace can reliably read a log captured
before the driver bound.
  
Trigger completion is polled every 100 ms from a delayed work item. The
probe-time check also runs from a work item, queued at the end of probe,
rather than from probe itself. This is because register access may go
through the parent's read_reg() callback, and the upcoming xe support for
Crescent Island (CRI) holds the lock that callback takes across
intel_vsec_register() when it registers late, after its firmware is ready.
With pmt_crashlog already loaded, probe runs synchronously within that
call, so reading a register from probe would deadlock on the lock the
caller already holds. Both work items run on the freezable workqueue so the
device is not accessed during suspend.

Manual triggers are rejected with -EBUSY until probe has set up all
entries, and the probe-time check is skipped once a trigger has been
accepted, so each capture is reported once. The work items are disabled and
drained on removal.

Document the uevent in the sysfs-class-intel_pmt ABI file.

Tested-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 .../ABI/testing/sysfs-class-intel_pmt         |  35 ++++
 drivers/platform/x86/intel/pmt/crashlog.c     | 180 +++++++++++++++++-
 2 files changed, 209 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-intel_pmt b/Documentation/ABI/testing/sysfs-class-intel_pmt
index ed4c886a21b1..71896e17a507 100644
--- a/Documentation/ABI/testing/sysfs-class-intel_pmt
+++ b/Documentation/ABI/testing/sysfs-class-intel_pmt
@@ -66,6 +66,41 @@ Description:
 		can be determined from an XML file of specified GUID for the
 		parent device.
 
+What:		/sys/class/intel_pmt/crashlog<x>
+Date:		November 2026
+KernelVersion:	7.5
+Contact:	David Box <david.e.box@linux.intel.com>
+Description:
+		The driver emits a KOBJ_CHANGE uevent for the crashlog<x>
+		device with the following environment variable:
+
+		INTEL_PMT_CRASHLOG_COMPLETE
+			The crashlog complete status at the end of a check:
+			1 if set, 0 if not. It reports the status only, not
+			what caused the crashlog to be captured.
+
+		The event is currently sent when a completed crashlog is
+		found at probe, and after a write of 1 to trigger. It
+		returns 1 when the crashlog completes, or 0 for trigger if
+		it has not completed within the poll time. Writing 0 to
+		trigger, writing clear or rearm, or disabling the crashlog
+		while polling ends the poll without an event.
+
+		The status may have changed by the time the event is
+		received, the same crashlog may be reported more than
+		once, and events sent in quick succession may be merged
+		into one carrying the latest status. Consumers should read
+		the status attributes before acting. Consumers that start
+		after the device was probed should check the complete
+		status of existing devices, for example when handling a
+		coldplug add event, since an event emitted before the
+		uevent listener starts is not replayed.
+
+		Other KOBJ_CHANGE events may be added in the future with
+		different variables. Consumers should match on
+		INTEL_PMT_CRASHLOG_COMPLETE and ignore variables they do not
+		recognize.
+
 What:		/sys/class/intel_pmt/crashlog<x>/crashlog
 Date:		October 2020
 KernelVersion:	5.10
diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
index d7018b495217..1796023a2f0e 100644
--- a/drivers/platform/x86/intel/pmt/crashlog.c
+++ b/drivers/platform/x86/intel/pmt/crashlog.c
@@ -13,13 +13,16 @@
 #include <linux/intel_vsec.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/kobject.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/pci.h>
 #include <linux/printk.h>
 #include <linux/slab.h>
+#include <linux/sysfs.h>
 #include <linux/uaccess.h>
 #include <linux/overflow.h>
+#include <linux/workqueue.h>
 
 #include "class.h"
 
@@ -115,6 +118,14 @@ struct crashlog_entry {
 	struct intel_pmt_entry		entry;
 	struct mutex			control_mutex;
 	const struct crashlog_info	*info;
+	struct work_struct		pending_work;
+	struct work_struct		uevent_work;
+	struct delayed_work		uevent_poll_work;
+	u8				uevent_poll_tries;
+	bool				uevent_polling;
+	bool				uevent_complete;
+	bool				trigger_ready;
+	bool				triggered;
 };
 
 struct pmt_crashlog_priv {
@@ -122,6 +133,9 @@ struct pmt_crashlog_priv {
 	struct crashlog_entry	entry[];
 };
 
+#define PMT_CRASHLOG_UEVENT_POLL_MS		100
+#define PMT_CRASHLOG_UEVENT_POLL_MAX_TRIES	50
+
 /*
  * I/O
  */
@@ -271,6 +285,93 @@ static int pmt_crashlog_set_rearm(struct crashlog_entry *crashlog)
 	return pmt_crashlog_rmw(crashlog, crashlog->info->control.rearm, true);
 }
 
+static void pmt_crashlog_uevent_fn(struct work_struct *work)
+{
+	struct crashlog_entry *crashlog =
+		container_of(work, struct crashlog_entry, uevent_work);
+	char *envp[] = {
+		READ_ONCE(crashlog->uevent_complete) ?
+			"INTEL_PMT_CRASHLOG_COMPLETE=1" :
+			"INTEL_PMT_CRASHLOG_COMPLETE=0",
+		NULL,
+	};
+
+	kobject_uevent_env(crashlog->entry.kobj, KOBJ_CHANGE, envp);
+}
+
+static void pmt_crashlog_uevent_poll_fn(struct work_struct *work)
+{
+	struct crashlog_entry *crashlog =
+		container_of(to_delayed_work(work), struct crashlog_entry,
+			     uevent_poll_work);
+	bool complete;
+	int ret;
+
+	guard(mutex)(&crashlog->control_mutex);
+
+	/* The cycle was ended by a clear or rearm */
+	if (!crashlog->uevent_polling)
+		return;
+
+	/* A newer trigger_store() re-armed us; that cycle owns the notification */
+	if (delayed_work_pending(&crashlog->uevent_poll_work))
+		return;
+
+	ret = pmt_crashlog_complete(crashlog, &complete);
+	if (!ret && complete) {
+		crashlog->uevent_polling = false;
+		WRITE_ONCE(crashlog->uevent_complete, true);
+		schedule_work(&crashlog->uevent_work);
+		return;
+	}
+
+	if (++crashlog->uevent_poll_tries < PMT_CRASHLOG_UEVENT_POLL_MAX_TRIES) {
+		queue_delayed_work(system_freezable_wq, &crashlog->uevent_poll_work,
+				   msecs_to_jiffies(PMT_CRASHLOG_UEVENT_POLL_MS));
+		return;
+	}
+
+	/* Trigger did not complete within the poll window */
+	crashlog->uevent_polling = false;
+	WRITE_ONCE(crashlog->uevent_complete, false);
+	schedule_work(&crashlog->uevent_work);
+}
+
+static void pmt_crashlog_end_poll(struct crashlog_entry *crashlog)
+{
+	lockdep_assert_held(&crashlog->control_mutex);
+
+	crashlog->uevent_polling = false;
+	cancel_delayed_work(&crashlog->uevent_poll_work);
+}
+
+static void pmt_crashlog_notify_pending(struct crashlog_entry *crashlog)
+{
+	bool complete;
+	int ret;
+
+	lockdep_assert_held(&crashlog->control_mutex);
+
+	ret = pmt_crashlog_complete(crashlog, &complete);
+	if (!ret && complete) {
+		WRITE_ONCE(crashlog->uevent_complete, true);
+		schedule_work(&crashlog->uevent_work);
+	}
+}
+
+/* Probe-time check for a crashlog completed before the driver bound */
+static void pmt_crashlog_pending_fn(struct work_struct *work)
+{
+	struct crashlog_entry *crashlog =
+		container_of(work, struct crashlog_entry, pending_work);
+
+	guard(mutex)(&crashlog->control_mutex);
+
+	/* A trigger since probe owns the notification for its capture */
+	if (!crashlog->triggered)
+		pmt_crashlog_notify_pending(crashlog);
+}
+
 /*
  * sysfs
  */
@@ -309,8 +410,12 @@ clear_store(struct device *dev, struct device_attribute *attr,
 	guard(mutex)(&crashlog->control_mutex);
 
 	ret = pmt_crashlog_set_clear(crashlog);
+	if (ret)
+		return ret;
 
-	return ret ?: count;
+	pmt_crashlog_end_poll(crashlog);
+
+	return count;
 }
 static DEVICE_ATTR_RW(clear);
 
@@ -399,8 +504,13 @@ enable_store(struct device *dev, struct device_attribute *attr,
 	guard(mutex)(&crashlog->control_mutex);
 
 	ret = pmt_crashlog_set_disable(crashlog, !enabled);
+	if (ret)
+		return ret;
 
-	return ret ?: count;
+	if (!enabled)
+		pmt_crashlog_end_poll(crashlog);
+
+	return count;
 }
 static DEVICE_ATTR_RW(enable);
 
@@ -454,8 +564,12 @@ rearm_store(struct device *dev, struct device_attribute *attr, const char *buf,
 	guard(mutex)(&crashlog->control_mutex);
 
 	ret = pmt_crashlog_set_rearm(crashlog);
+	if (ret)
+		return ret;
 
-	return ret ?: count;
+	pmt_crashlog_end_poll(crashlog);
+
+	return count;
 }
 static DEVICE_ATTR_RW(rearm);
 
@@ -502,9 +616,17 @@ trigger_store(struct device *dev, struct device_attribute *attr,
 
 	if (!trigger) {
 		ret = pmt_crashlog_set_clear(crashlog);
-		return ret ?: count;
+		if (ret)
+			return ret;
+
+		pmt_crashlog_end_poll(crashlog);
+		return count;
 	}
 
+	/* reject triggers until probe has finished setting up this entry */
+	if (!crashlog->trigger_ready)
+		return -EBUSY;
+
 	/* we cannot trigger a new crash if one is still pending */
 	ret = pmt_crashlog_complete(crashlog, &complete);
 	if (ret)
@@ -513,8 +635,23 @@ trigger_store(struct device *dev, struct device_attribute *attr,
 		return -EEXIST;
 
 	ret = pmt_crashlog_set_execute(crashlog);
+	if (ret)
+		return ret;
 
-	return ret ?: count;
+	crashlog->triggered = true;
+
+	/*
+	 * Use non-sync since uevent_poll_fn() takes this same mutex, so
+	 * cancel_delayed_work_sync() here could deadlock against it.
+	 */
+	cancel_delayed_work(&crashlog->uevent_poll_work);
+
+	crashlog->uevent_polling = true;
+	crashlog->uevent_poll_tries = 0;
+	queue_delayed_work(system_freezable_wq, &crashlog->uevent_poll_work,
+			   msecs_to_jiffies(PMT_CRASHLOG_UEVENT_POLL_MS));
+
+	return count;
 }
 static DEVICE_ATTR_RW(trigger);
 
@@ -640,6 +777,9 @@ static void pmt_crashlog_remove(struct auxiliary_device *auxdev)
 	for (i = 0; i < priv->num_entries; i++) {
 		struct crashlog_entry *crashlog = &priv->entry[i];
 
+		disable_work_sync(&crashlog->pending_work);
+		disable_delayed_work_sync(&crashlog->uevent_poll_work);
+		disable_work_sync(&crashlog->uevent_work);
 		intel_pmt_dev_destroy(&crashlog->entry, &pmt_crashlog_ns);
 		mutex_destroy(&crashlog->control_mutex);
 	}
@@ -661,7 +801,14 @@ static int pmt_crashlog_probe(struct auxiliary_device *auxdev,
 	auxiliary_set_drvdata(auxdev, priv);
 
 	for (i = 0; i < intel_vsec_dev->num_resources; i++) {
-		struct intel_pmt_entry *entry = &priv->entry[priv->num_entries].entry;
+		struct crashlog_entry *crashlog = &priv->entry[priv->num_entries];
+		struct intel_pmt_entry *entry = &crashlog->entry;
+
+		/* trigger_store() rejects triggers until trigger_ready is set below */
+		INIT_WORK(&crashlog->pending_work, pmt_crashlog_pending_fn);
+		INIT_WORK(&crashlog->uevent_work, pmt_crashlog_uevent_fn);
+		INIT_DELAYED_WORK(&crashlog->uevent_poll_work,
+				  pmt_crashlog_uevent_poll_fn);
 
 		ret = intel_pmt_dev_create(entry, &pmt_crashlog_ns, intel_vsec_dev, i);
 		if (ret < 0)
@@ -672,6 +819,27 @@ static int pmt_crashlog_probe(struct auxiliary_device *auxdev,
 		priv->num_entries++;
 	}
 
+	/*
+	 * Don't read crashlog registers from probe. The read may go through the
+	 * parent's read_reg() callback, which can take a lock that the parent
+	 * holds across intel_vsec_register() and so across a synchronous probe.
+	 * From a work item, the read just waits for registration to finish.
+	 *
+	 * Enable triggers and queue the work only once every entry is created.
+	 * abort_probe waits for the work items in pmt_crashlog_remove(), which
+	 * would deadlock if one were already blocked on a lock held by our
+	 * caller. For the same reason, a parent must not hold that lock while
+	 * removing the device.
+	 */
+	for (i = 0; i < priv->num_entries; i++) {
+		struct crashlog_entry *crashlog = &priv->entry[i];
+
+		scoped_guard(mutex, &crashlog->control_mutex)
+			crashlog->trigger_ready = true;
+
+		queue_work(system_freezable_wq, &crashlog->pending_work);
+	}
+
 	return 0;
 abort_probe:
 	pmt_crashlog_remove(auxdev);
-- 
2.53.0