[PATCH v5 5/6] platform/x86/amd/hsmp: ACPI HSMP refcounted sockets and coordinated release

[Muralidhara M K]

The ACPI driver binds one platform device per socket but shares a single
socket array and a single /dev/hsmp misc device across them. Replace the
is_probed flag with state that tracks this shared ownership:

 - miscdevice.this_device tells whether /dev/hsmp is registered, so the
   misc device is registered on the first socket and torn down last. A
   preceding change clears mdev.this_device on deregister so this gate
   stays reliable across a re-probe.

 - hsmp_acpi_sock_refs counts the sockets that have probed successfully.
   It is guarded by hsmp_sock_rwsem, which probe and remove already hold
   for write, so a plain counter is enough and no atomic refcount is
   needed. The shared socket array is allocated with kcalloc() on the first
   probe and freed by hsmp_acpi_sock_release() when the count drops back to
   zero.

hsmp_acpi_sock_release() is the single teardown helper: it deregisters
/dev/hsmp if registered, unmaps any metric-table DRAM, destroys the
per-socket mutexes and frees the array. The remove path and the
probe-failure path each call it once they are the last owner, so the
teardown lives in one place.

Both paths also clear this socket's dev, so a message issued after a
non-final unbind (or to a socket that failed to probe on a multi-socket
system, whose array stays alive and whose remove() is never called) cannot
reach the mailbox that devres is about to unmap.

Two lifetime fixes fall out of the array persisting across a non-final
unbind:

 - hsmp_get_tbl_dram_base() iounmap()s any stale metric_tbl_addr before
   remapping, so a rebind does not leak one mapping per cycle. It runs
   during (re)probe before the metric sysfs attribute is exposed, so no
   reader can be using the old mapping.

 - The ACPI path registers /dev/hsmp unparented by passing NULL to
   hsmp_misc_register(). Its per-socket devices can be unbound individually
   and out of order and the misc device outlives all but the last of them,
   so parenting it to one socket's device would leave a dangling parent.
   hsmp_misc_register() now takes the parent from its caller, so the
   platform driver keeps parenting /dev/hsmp to its single device.

hsmp_sock_rwsem is held for write across probe and remove, so the release
and probe-failure cleanup run with it already held; an upcoming change adds
its read side so the same lock also drains the lock-free data plane.

Signed-off-by: Muralidhara M K <muralidhara.mk@xxxxxxx>
---
 drivers/platform/x86/amd/hsmp/acpi.c | 122 ++++++++++++++++++++++-----
 drivers/platform/x86/amd/hsmp/hsmp.c |  20 +++++
 drivers/platform/x86/amd/hsmp/hsmp.h |   1 -
 3 files changed, 123 insertions(+), 20 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index a092d7589bcb..8339af1624f4 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -24,6 +24,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/rwsem.h>
+#include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/sysfs.h>
 #include <linux/topology.h>
@@ -42,6 +43,14 @@
 
 static struct hsmp_plat_device *hsmp_pdev;
 
+/*
+ * Number of ACPI socket platform devices that have probed successfully.
+ * Guarded by hsmp_sock_rwsem, which probe and remove hold for write, so a
+ * plain counter is enough; no atomic is needed. The shared socket array is
+ * allocated on the first probe and freed once this drops back to zero.
+ */
+static unsigned int hsmp_acpi_sock_refs;
+
 struct hsmp_sys_attr {
 	struct device_attribute dattr;
 	u32 msg_id;
@@ -611,6 +620,60 @@ static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
 
+/*
+ * Tear down the shared ACPI socket state once the last socket is gone:
+ * deregister /dev/hsmp if it was registered, unmap any metric-table DRAM,
+ * destroy the per-socket mutexes and free the socket array.
+ *
+ * Called with hsmp_sock_rwsem held for write by the remove and probe-failure
+ * paths. The write lock has drained any in-flight hsmp_send_message(), so
+ * unmapping the mailbox and freeing the array cannot race the lock-free data
+ * plane.
+ */
+static void hsmp_acpi_sock_release(void)
+{
+	lockdep_assert_held_write(&hsmp_sock_rwsem);
+
+	if (!IS_ERR_OR_NULL(hsmp_pdev->mdev.this_device))
+		hsmp_misc_deregister();
+	hsmp_unmap_metric_tbls(hsmp_pdev);
+	hsmp_destroy_metric_read_locks(hsmp_pdev);
+	kfree(hsmp_pdev->sock);
+	hsmp_pdev->sock = NULL;
+	hsmp_pdev->num_sockets = 0;
+	hsmp_pdev->proto_ver = 0;
+}
+
+/**
+ * hsmp_acpi_probe_failure_cleanup() - Undo a failed ACPI socket probe.
+ * @dev: ACPI companion device whose probe failed.
+ *
+ * This device never incremented hsmp_acpi_sock_refs, so clear its sock->dev
+ * and, if it was the only socket in play, release the shared state.
+ *
+ * Clearing sock->dev matters on multi-socket systems: when a non-first socket
+ * fails, the array stays alive (owned by an already-probed socket) and
+ * remove() is never called for this device, yet devres unmaps its mailbox once
+ * probe() returns. Without clearing dev, a later message to this index would
+ * pass every gate in hsmp_send_message() and reach the unmapped mailbox.
+ *
+ * sock is NULL if probe failed before hsmp_parse_acpi_table() set the drvdata.
+ *
+ * Called from hsmp_acpi_probe(), which already holds hsmp_sock_rwsem for write.
+ */
+static void hsmp_acpi_probe_failure_cleanup(struct device *dev)
+{
+	struct hsmp_socket *sock = dev_get_drvdata(dev);
+
+	lockdep_assert_held_write(&hsmp_sock_rwsem);
+
+	if (sock)
+		sock->dev = NULL;
+
+	if (!hsmp_acpi_sock_refs)
+		hsmp_acpi_sock_release();
+}
+
 static int hsmp_acpi_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -620,23 +683,24 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	/*
-	 * Multiple ACPI socket devices probe in parallel, but the is_probed
-	 * handshake and the one-time socket-array allocation below must run
-	 * exactly once.  Serialize the whole bring-up against concurrent
-	 * probe/remove by holding the socket rwsem for write.
+	 * Multiple ACPI socket devices probe in parallel, but the one-time
+	 * socket-array allocation and /dev/hsmp registration below must run
+	 * exactly once. Hold the socket rwsem for write across the whole
+	 * bring-up so it cannot race a concurrent probe or remove, and so the
+	 * probe-failure teardown drains the lock-free data plane.
 	 */
 	guard(rwsem_write)(&hsmp_sock_rwsem);
 
-	if (!hsmp_pdev->is_probed) {
+	if (!hsmp_pdev->sock) {
 		hsmp_pdev->num_sockets = topology_max_packages();
 		if (!hsmp_pdev->num_sockets) {
 			dev_err(&pdev->dev, "No CPU sockets detected\n");
 			return -ENODEV;
 		}
 
-		hsmp_pdev->sock = devm_kcalloc(&pdev->dev, hsmp_pdev->num_sockets,
-					       sizeof(*hsmp_pdev->sock),
-					       GFP_KERNEL);
+		hsmp_pdev->sock = kcalloc(hsmp_pdev->num_sockets,
+					  sizeof(*hsmp_pdev->sock),
+					  GFP_KERNEL);
 		if (!hsmp_pdev->sock)
 			return -ENOMEM;
 
@@ -646,35 +710,55 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
 	ret = init_acpi(&pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
+		hsmp_acpi_probe_failure_cleanup(&pdev->dev);
 		return ret;
 	}
 
-	if (!hsmp_pdev->is_probed) {
-		ret = hsmp_misc_register(&pdev->dev);
+	if (IS_ERR_OR_NULL(hsmp_pdev->mdev.this_device)) {
+		/*
+		 * Register /dev/hsmp unparented. It is a singleton shared by all
+		 * ACPI sockets and outlives all but the last of them, so
+		 * parenting it to this socket's device would leave a dangling
+		 * parent once that socket is unbound.
+		 */
+		ret = hsmp_misc_register(NULL);
 		if (ret) {
 			dev_err(&pdev->dev, "Failed to register misc device\n");
+			hsmp_acpi_probe_failure_cleanup(&pdev->dev);
 			return ret;
 		}
-		hsmp_pdev->is_probed = true;
-		dev_dbg(&pdev->dev, "AMD HSMP ACPI is probed successfully\n");
+		dev_dbg(&pdev->dev, "AMD HSMP ACPI misc device registered\n");
 	}
 
+	hsmp_acpi_sock_refs++;
+
 	return 0;
 }
 
 static void hsmp_acpi_remove(struct platform_device *pdev)
 {
+	struct hsmp_socket *sock = dev_get_drvdata(&pdev->dev);
+
+	/*
+	 * Serialize the decrement and any release it triggers against a
+	 * concurrent probe, and drain the lock-free data plane for the whole
+	 * teardown: this covers the per-socket unbind, whose mailbox devres
+	 * unmaps once we return, and the last unbind that frees the socket
+	 * array in hsmp_acpi_sock_release().
+	 */
 	guard(rwsem_write)(&hsmp_sock_rwsem);
 
 	/*
-	 * We register only one misc_device even on multi-socket system.
-	 * So, deregister should happen only once.
+	 * Clear this socket's dev so hsmp_send_message() rejects it before
+	 * devres unmaps the mailbox. On a non-final unbind the socket array
+	 * stays alive, so without this a later message to this index would
+	 * reach an unmapped iomem region.
 	 */
-	if (hsmp_pdev->is_probed) {
-		hsmp_misc_deregister();
-		hsmp_destroy_metric_read_locks(hsmp_pdev);
-		hsmp_pdev->is_probed = false;
-	}
+	sock->dev = NULL;
+
+	hsmp_acpi_sock_refs--;
+	if (!hsmp_acpi_sock_refs)
+		hsmp_acpi_sock_release();
 }
 
 static struct platform_driver amd_hsmp_driver = {
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 584fd9b1d31f..967307abe641 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -491,6 +491,18 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
 		dev_err(sock->dev, "Invalid DRAM address for metric table\n");
 		return -ENOMEM;
 	}
+	/*
+	 * The ACPI socket array is shared across sockets and outlives a
+	 * per-socket unbind, so metric_tbl_addr may hold a mapping from an
+	 * earlier bind of this socket. Unmap it before remapping so an
+	 * unbind/rebind cycle does not leak a metric-table mapping. This runs
+	 * during probe before the metric sysfs attribute is exposed, so no
+	 * reader can be using it.
+	 */
+	if (sock->metric_tbl_addr) {
+		iounmap(sock->metric_tbl_addr);
+		sock->metric_tbl_addr = NULL;
+	}
 	sock->metric_tbl_addr = ioremap(dram_addr, sizeof(struct hsmp_metric_table));
 	if (!sock->metric_tbl_addr) {
 		dev_err(sock->dev, "Failed to ioremap metric table addr\n");
@@ -528,6 +540,14 @@ int hsmp_misc_register(struct device *dev)
 	hsmp_pdev.mdev.name	= HSMP_CDEV_NAME;
 	hsmp_pdev.mdev.minor	= MISC_DYNAMIC_MINOR;
 	hsmp_pdev.mdev.fops	= &hsmp_fops;
+	/*
+	 * The caller chooses the parent. The platform driver has a single
+	 * device whose lifetime matches /dev/hsmp and parents it there. The
+	 * ACPI driver passes NULL: its /dev/hsmp is a singleton shared by
+	 * per-socket devices that can be unbound individually and out of order,
+	 * so parenting it to one would leave it attached to an already-removed
+	 * device.
+	 */
 	hsmp_pdev.mdev.parent	= dev;
 	hsmp_pdev.mdev.nodename	= HSMP_DEVNODE_NAME;
 	hsmp_pdev.mdev.mode	= 0644;
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index ec92c2a429bb..45dab9253c13 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -58,7 +58,6 @@ struct hsmp_plat_device {
 	struct hsmp_socket *sock;
 	u32 proto_ver;
 	u16 num_sockets;
-	bool is_probed;
 };
 
 int hsmp_cache_proto_ver(u16 sock_ind);
-- 
2.34.1