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

[PATCH v2 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.

v1 was a three-patch series. Patches 1 and 2 are no longer part of it:

 - Patch 1 (NULL dereference when reading crashlog data) is dropped in
   favor of Michael's "platform/x86/intel/pmt: complete pcidev to
   device update" (b86e5062cb88), which fixes the same bug.

 - Patch 2 (inverted walk_header() test in get_features()) has been
   applied as 3678e39c6aae.

Both are in the base of this posting, review-ilpo-next. The patch also
applies cleanly to for-next.

Testing: the patch was tested in an emulated (QEMU) environment on a
kernel with CONFIG_PROVE_LOCKING, covering the probe-time event, the
event after a manual trigger completes, triggers rejected while a log
is complete or the device is disabled, clear, trigger=0 and rearm while
the trigger poll is running, expiry of the poll window, module removal
with the poll pending, and re-probe. Hardware with this crashlog
support is available, but testing on it is still pending.

V2 changes:
  - Replaced the two uevent variables with a single
    INTEL_PMT_CRASHLOG_COMPLETE. INTEL_PMT_CRASHLOG_EVENT=PRESENT and
    COMPLETE=1 were both fixed and carried no information beyond the
    event itself.
  - Emit COMPLETE=0 when a manual trigger does not complete within the
    poll window, so userspace is not left waiting.
  - Reject manual triggers until probe has finished setting up the
    entry. This closes a window where a trigger during
    intel_pmt_dev_create() could queue work against a device freed by
    its error path (reported by sashiko on v1).
  - Dropped the cancel_*_sync() calls on the intel_pmt_dev_create()
    failure and skip paths. They ran after the device was already
    freed, but with triggers now rejected until setup completes no work
    can be pending there.
  - End the trigger poll without an event when the log is cleared or
    rearmed while it is running.
  - Cancel a stale poll only after the new trigger has been written, so
    a failed trigger leaves the previous poll in place.
  - Run the probe-time check under control_mutex, in the same critical
    section that enables triggers.
  - Run the poll on the freezable workqueue so it does not read the
    device during suspend.
  - Rebased onto review-ilpo-next and adapted to the crashlog register
    helpers now returning errors (Michal's additions). A failed status
    read counts as not complete.
  - Rewrote the ABI

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         |  32 ++++
 drivers/platform/x86/intel/pmt/crashlog.c     | 138 +++++++++++++++++-
 2 files changed, 165 insertions(+), 5 deletions(-)


base-commit: 9d3475065167197742b0882aff21cf8e78393277
-- 
2.43.0

[PATCH v2 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 userspace requests a manual trigger.
Userspace otherwise has to poll to discover that data is ready.

Emit a KOBJ_CHANGE uevent on the per-instance crashlog device when a
completed log is found at probe. After a manual trigger, poll the
completion bit at 100 ms intervals for up to 5 seconds and emit the same
event when capture completes, or when the poll window expires without
completion, so userspace is not left waiting. A clear or rearm while
polling ends the poll without an event. The poll runs on the freezable
workqueue so it does not read the device during suspend. The event
carries INTEL_PMT_CRASHLOG_COMPLETE set to 1 or 0 accordingly.

The class device's attributes are created after its KOBJ_ADD, so the
probe-time event is also the first point at which userspace can reliably
read a log that was already captured.

Initialize the work items before the device's sysfs attributes are
exposed, and reject manual triggers until setup of that crashlog
completes so a trigger cannot queue work against a device torn down by
a later probe failure. Disable and drain the work items during removal.

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

Assisted-by: LLM
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 .../ABI/testing/sysfs-class-intel_pmt         |  32 ++++
 drivers/platform/x86/intel/pmt/crashlog.c     | 138 +++++++++++++++++-
 2 files changed, 165 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-intel_pmt b/Documentation/ABI/testing/sysfs-class-intel_pmt
index ed4c886a21b1..cf95d1dcd4d6 100644
--- a/Documentation/ABI/testing/sysfs-class-intel_pmt
+++ b/Documentation/ABI/testing/sysfs-class-intel_pmt
@@ -66,6 +66,38 @@ Description:
 		can be determined from an XML file of specified GUID for the
 		parent device.
 
+What:		/sys/class/intel_pmt/crashlog<x>
+Date:		October 2026
+KernelVersion:	7.4
+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 clear
+		or rearm while polling ends trigger without an event.
+
+		The status may have changed by the time the event is
+		received, and the same crashlog may be reported more than
+		once. 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..2d006abce5bf 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,12 @@ struct crashlog_entry {
 	struct intel_pmt_entry		entry;
 	struct mutex			control_mutex;
 	const struct crashlog_info	*info;
+	struct work_struct		uevent_work;
+	struct delayed_work		uevent_poll_work;
+	u8				uevent_poll_tries;
+	bool				uevent_polling;
+	bool				uevent_complete;
+	bool				trigger_ready;
 };
 
 struct pmt_crashlog_priv {
@@ -122,6 +131,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 +283,80 @@ 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);
+	}
+}
+
 /*
  * sysfs
  */
@@ -309,8 +395,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);
 
@@ -454,8 +544,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 +596,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 +615,21 @@ trigger_store(struct device *dev, struct device_attribute *attr,
 		return -EEXIST;
 
 	ret = pmt_crashlog_set_execute(crashlog);
+	if (ret)
+		return ret;
 
-	return ret ?: count;
+	/*
+	 * 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 +755,8 @@ 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_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 +778,13 @@ 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->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)
@@ -670,6 +793,11 @@ static int pmt_crashlog_probe(struct auxiliary_device *auxdev,
 			continue;
 
 		priv->num_entries++;
+
+		scoped_guard(mutex, &crashlog->control_mutex) {
+			crashlog->trigger_ready = true;
+			pmt_crashlog_notify_pending(crashlog);
+		}
 	}
 
 	return 0;
-- 
2.43.0