[PATCH 2/2] platform/x86/amd/hsmp: Report raw SMU mailbox status via smu_response

[Muralidhara M K]

Add a __u32 smu_response field to struct hsmp_message, set by
__hsmp_send_message() from the polled SMU mailbox status register.
hsmp_ioctl_msg() now always copies the message struct back to
userspace, even on failure, so smu_response is visible on every call.
Initialize it to HSMP_STATUS_NOT_READY before dispatch, so a request
rejected before reaching the SMU reports a defined value instead of
echoing back the caller's input.

Replace the reserved field in struct hsmp_telemetry_data with a __u32
smu_response, threaded through hsmp_metric_tbl_read_locked() as a new
optional out-parameter and surfaced by hsmp_ioctl_get_telemetry().
Change HSMP_IOCTL_GET_TELEMETRY_DATA from _IOW to _IOWR, since the
request struct is now written back to.

Both structs grow under #pragma pack(4) (44->48 and 16->20 bytes),
changing the encoded command number of both HSMP_IOCTL_CMD and
HSMP_IOCTL_GET_TELEMETRY_DATA; existing callers of either ioctl need
to rebuild against this header.

Fix hsmp_ioctl_get_telemetry()'s copy_to_user() error check to compare
ret >= 0 instead of !ret: hsmp_metric_tbl_read_locked() returns a byte
count on success, never exactly 0, so the old check never actually
fired and a failed copy of the smu_response-bearing req struct went
unreported.

Update Documentation/arch/x86/amd_hsmp.rst to match.

Co-developed-by: Muthusamy Ramalingam <muthusamy.ramalingam@xxxxxxx>
Signed-off-by: Muthusamy Ramalingam <muthusamy.ramalingam@xxxxxxx>
Signed-off-by: Muralidhara M K <muralidhara.mk@xxxxxxx>
---
 Documentation/arch/x86/amd_hsmp.rst  |  28 +++++---
 arch/x86/include/uapi/asm/amd_hsmp.h |  35 +++++++---
 drivers/platform/x86/amd/hsmp/hsmp.c | 101 ++++++++++++++++++---------
 3 files changed, 113 insertions(+), 51 deletions(-)

diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst
index c483c68dcfad..9d4aaa8936f4 100644
--- a/Documentation/arch/x86/amd_hsmp.rst
+++ b/Documentation/arch/x86/amd_hsmp.rst
@@ -188,13 +188,21 @@ The following IOCTLs are defined:
   The argument is a pointer to a::
 
     struct hsmp_message {
-    	__u32	msg_id;				/* Message ID */
-    	__u16	num_args;			/* Number of input argument words in message */
-    	__u16	response_sz;			/* Number of expected output/response words */
-    	__u32	args[HSMP_MAX_MSG_LEN];		/* argument/response buffer */
-    	__u16	sock_ind;			/* socket number */
+        __u32 msg_id;                      /* Message ID */
+        __u16 num_args;                    /* Number of input argument words in message */
+        __u16 response_sz;                 /* Number of expected output/response words */
+        __u32 args[HSMP_MAX_MSG_LEN];       /* argument/response buffer */
+        __u16 sock_ind;                    /* socket number */
+        __u32 smu_response;                /* Output: raw SMU mailbox status */
     };
 
+  ``msg`` is copied back to userspace after every call, regardless of
+  whether the ioctl succeeds or fails, so ``smu_response`` is always
+  visible: 0 means the message was never sent to the SMU (rejected
+  before dispatch, e.g. an invalid msg_id or argument count), 1 means
+  the SMU accepted it, and any other value is an SMU-specific error
+  code.
+
 ``ioctl(file, HSMP_IOCTL_GET_TELEMETRY_DATA, struct hsmp_telemetry_data *req)``
   Atomically fetch the firmware metric (telemetry) table for a socket.
   The ioctl copies the table in one shot, so unlike the metrics_bin
@@ -206,14 +214,18 @@ The following IOCTLs are defined:
         __u64 buf; /* User pointer to destination buffer */
         __u32 size; /* Size of @buf in bytes */
         __u16 sock_ind; /* Socket index */
-        __u16 reserved; /* Reserved, must be zero */
+        __u32 smu_response; /* Output: raw SMU mailbox status */
     };
 
   ``size`` must be non-zero and no larger than the table size firmware
   reports for that socket; a larger value is rejected with ``-EINVAL``
   rather than short-written, and a smaller one returns the leading
-  ``size`` bytes of the snapshot.  A non-zero ``reserved`` is also
-  rejected with ``-EINVAL``.
+  ``size`` bytes of the snapshot.  ``req`` is copied back to userspace
+  with ``smu_response`` filled in on every call, including failures:
+  1 means the HSMP_GET_METRIC_TABLE message that refilled the table
+  succeeded, 0 means it was never attempted (a request rejected before
+  dispatch, e.g. an invalid socket index or size), and any other value
+  is an SMU-specific error code.
 
   The table layout depends on the protocol version, which userspace
   reads from the ``protocol_version`` sysfs attribute.  On version 6
diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
index dcbe89dc4d2b..1ac385996a4e 100644
--- a/arch/x86/include/uapi/asm/amd_hsmp.h
+++ b/arch/x86/include/uapi/asm/amd_hsmp.h
@@ -88,6 +88,7 @@ struct hsmp_message {
 	__u16	response_sz;		/* Number of expected output/response words */
 	__u32	args[HSMP_MAX_MSG_LEN];	/* argument/response buffer */
 	__u16	sock_ind;		/* socket number */
+	__u32	smu_response;		/* Output: raw SMU mailbox status */
 };
 
 enum hsmp_msg_type {
@@ -644,13 +645,17 @@ struct hsmp_metric_table {
  *             short-written.  A smaller value returns the leading @size bytes
  *             of the snapshot.  The kernel does not write this field back.
  * @sock_ind:  Input. Socket index from which the metric table is read.
- * @reserved:  Reserved for future use.  Callers should set this to zero;
- *             future kernels may begin interpreting the field, so passing
- *             a non-zero value today is not forwards compatible.
+ * @smu_response: Output. Raw SMU mailbox status from the HSMP_GET_METRIC_TABLE
+ *             message that refilled the table (0 = not attempted/not ready,
+ *             1 = success, other values are command-specific SMU error
+ *             codes). Callers should set this to zero on input; the kernel
+ *             always overwrites it before returning.
  *
- * Placing @buf first lets all fields fall on their natural alignment under
- * the surrounding #pragma pack(4), so the struct is a tight 16 bytes with
- * the same wire layout on 32-bit and 64-bit userspace.
+ * Placing @buf first and @smu_response last lets all fields fall on their
+ * natural alignment under the surrounding #pragma pack(4); the compiler
+ * inserts 2 bytes of padding after @sock_ind so @smu_response lands on a
+ * 4-byte boundary, giving the struct the same 20-byte wire layout on 32-bit
+ * and 64-bit userspace.
  *
  * The metric table layout depends on the HSMP protocol version reported by
  * firmware, which userspace can read from the protocol_version sysfs
@@ -662,13 +667,20 @@ struct hsmp_telemetry_data {
 	__u64	buf;
 	__u32	size;
 	__u16	sock_ind;
-	__u16	reserved;
+	__u32	smu_response;
 };
 
 /* Reset to default packing */
 #pragma pack()
 
-/* Define unique ioctl command for hsmp msgs using generic _IOWR */
+/*
+ * Define unique ioctl command for hsmp msgs using generic _IOWR.
+ *
+ * The encoded command number embeds sizeof(struct hsmp_message), so it
+ * changes whenever that struct's size changes (as it just did when the
+ * __u32 smu_response field was added); existing callers need to rebuild
+ * against this header.
+ */
 #define HSMP_BASE_IOCTL_NR	0xF8
 #define HSMP_IOCTL_CMD		_IOWR(HSMP_BASE_IOCTL_NR, 0, struct hsmp_message)
 
@@ -678,11 +690,12 @@ struct hsmp_telemetry_data {
  * sysfs binary attribute path for tables larger than one page (such as the
  * ~13 KB table used by HSMP protocol version 7).
  *
- * The direction is _IOW because the kernel only reads the request struct;
- * the table itself is written to the buffer that @buf points at.
+ * The direction is _IOWR: the kernel reads the request struct (buf, size,
+ * sock_ind) and also writes back into it (smu_response), in addition to
+ * writing the table itself to the buffer that @buf points at.
  */
 #define HSMP_IOCTL_GET_TELEMETRY_DATA \
-	_IOW(HSMP_BASE_IOCTL_NR, 1, struct hsmp_telemetry_data)
+	_IOWR(HSMP_BASE_IOCTL_NR, 1, struct hsmp_telemetry_data)
 
 /**
  * enum hsmp_client_message_ids - Ryzen Master SMC (RMSMC) message IDs
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 635e6d5723ac..6a56a908912c 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -173,6 +173,14 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
 
 	mbinfo = &sock->mbinfo;
 
+	/*
+	 * Initialize this to HSMP_STATUS_NOT_READY up front so every return
+	 * path, including the raw register access failures below (before
+	 * the mailbox status is ever polled), reports a defined value
+	 * instead of echoing back whatever the caller happened to pass in.
+	 */
+	msg->smu_response = HSMP_STATUS_NOT_READY;
+
 	/* Clear the status register */
 	mbox_status = HSMP_STATUS_NOT_READY;
 	ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_resp_off, &mbox_status, HSMP_WR);
@@ -229,6 +237,14 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
 			usleep_range(1000, 2000);
 	}
 
+	/*
+	 * Surface the raw SMU mailbox status to the caller regardless of
+	 * outcome via msg->smu_response, so userspace can inspect the exact
+	 * hardware status without decoding it from the translated errno
+	 * below.
+	 */
+	msg->smu_response = mbox_status;
+
 	if (unlikely(mbox_status == HSMP_STATUS_NOT_READY)) {
 		dev_err(sock->dev, "Message ID 0x%X failure : SMU timeout (status = 0x%X)\n",
 			msg->msg_id, mbox_status);
@@ -471,6 +487,15 @@ static long hsmp_ioctl_msg(struct file *fp, unsigned long arg)
 	if (copy_struct_from_user(&msg, sizeof(msg), arguser, sizeof(struct hsmp_message)))
 		return -EFAULT;
 
+	/*
+	 * Initialize this to HSMP_STATUS_NOT_READY regardless of what the
+	 * caller passed in: it must only ever reflect a real SMU mailbox
+	 * status set by __hsmp_send_message(), never a stale value echoed
+	 * back on a failure (invalid/reserved msg_id, arg count mismatch,
+	 * bad socket, etc.) that is caught before the message is ever sent.
+	 */
+	msg.smu_response = HSMP_STATUS_NOT_READY;
+
 	/*
 	 * The client message set has no per-message descriptor table to
 	 * bounds-check msg_id against or index into, and no per-message
@@ -524,25 +549,20 @@ static long hsmp_ioctl_msg(struct file *fp, unsigned long arg)
 	}
 
 	ret = hsmp_send_message(&msg);
-	if (ret)
-		return ret;
 
 	/*
-	 * The client message set has no per-message response_sz to check
-	 * here (desc is NULL); validate_message() already bounded
-	 * msg.response_sz.
+	 * Always copy the message struct back to userspace, even on
+	 * failure, so the raw SMU mailbox status in msg.smu_response is
+	 * always visible to the caller regardless of outcome.
 	 */
-	if ((desc ? desc->response_sz : msg.response_sz) > 0) {
-		/* Copy results back to user for get/monitor commands */
-		if (copy_to_user(arguser, &msg, sizeof(struct hsmp_message)))
-			return -EFAULT;
-	}
+	if (copy_to_user(arguser, &msg, sizeof(struct hsmp_message)))
+		return -EFAULT;
 
-	return 0;
+	return ret;
 }
 
 static ssize_t hsmp_metric_tbl_read_locked(struct hsmp_socket *sock, char *buf,
-					   size_t size);
+					   size_t size, u32 *smu_response);
 
 /*
  * Fetch the firmware metric (telemetry) table for the requested socket and
@@ -558,6 +578,7 @@ static long hsmp_ioctl_get_telemetry(struct file *fp, unsigned long arg)
 	void *kbuf __free(kvfree) = NULL;
 	void __user *arguser = (void __user *)arg;
 	struct hsmp_telemetry_data req;
+	u32 smu_response = HSMP_STATUS_NOT_READY;
 	struct hsmp_socket *sock;
 	void __user *user_buf;
 	size_t tbl_size;
@@ -571,13 +592,6 @@ static long hsmp_ioctl_get_telemetry(struct file *fp, unsigned long arg)
 	if (copy_from_user(&req, arguser, sizeof(req)))
 		return -EFAULT;
 
-	/*
-	 * Reserved fields must be zero so future kernels can safely
-	 * repurpose them without breaking already-deployed userspace.
-	 */
-	if (req.reserved)
-		return -EINVAL;
-
 	user_buf = u64_to_user_ptr(req.buf);
 
 	/*
@@ -594,8 +608,10 @@ static long hsmp_ioctl_get_telemetry(struct file *fp, unsigned long arg)
 	 * which would leave a socket unbind waiting for the write lock.
 	 */
 	scoped_guard(rwsem_read, &hsmp_sock_rwsem) {
-		if (!hsmp_pdev.sock || req.sock_ind >= hsmp_pdev.num_sockets)
-			return -ENODEV;
+		if (!hsmp_pdev.sock || req.sock_ind >= hsmp_pdev.num_sockets) {
+			ret = -ENODEV;
+			break;
+		}
 
 		/*
 		 * Sanitize the user-controlled socket index against speculative
@@ -609,12 +625,16 @@ static long hsmp_ioctl_get_telemetry(struct file *fp, unsigned long arg)
 		 */
 		sock_ind = array_index_nospec(req.sock_ind, hsmp_pdev.num_sockets);
 		sock = &hsmp_pdev.sock[sock_ind];
-		if (!sock->metric_tbl_addr)
-			return -ENODEV;
+		if (!sock->metric_tbl_addr) {
+			ret = -ENODEV;
+			break;
+		}
 
 		tbl_size = sock->metric_tbl_size;
-		if (!tbl_size)
-			return -ENODEV;
+		if (!tbl_size) {
+			ret = -ENODEV;
+			break;
+		}
 
 		/*
 		 * A request shorter than the firmware table is served with the
@@ -624,8 +644,10 @@ static long hsmp_ioctl_get_telemetry(struct file *fp, unsigned long arg)
 		 * rejected rather than short-written, so a caller can never
 		 * mistake a partial copy for a full one.
 		 */
-		if (!req.size || req.size > tbl_size)
-			return -EINVAL;
+		if (!req.size || req.size > tbl_size) {
+			ret = -EINVAL;
+			break;
+		}
 
 		/*
 		 * The bounce buffer is overwritten in full by memcpy_fromio()
@@ -634,12 +656,25 @@ static long hsmp_ioctl_get_telemetry(struct file *fp, unsigned long arg)
 		 * on every ioctl call.
 		 */
 		kbuf = kvmalloc(tbl_size, GFP_KERNEL);
-		if (!kbuf)
-			return -ENOMEM;
+		if (!kbuf) {
+			ret = -ENOMEM;
+			break;
+		}
 
-		ret = hsmp_metric_tbl_read_locked(sock, kbuf, tbl_size);
+		ret = hsmp_metric_tbl_read_locked(sock, kbuf, tbl_size, &smu_response);
 	}
 
+	/*
+	 * Report the raw SMU mailbox status back to userspace whenever the
+	 * HSMP_GET_METRIC_TABLE message was actually attempted. smu_response
+	 * stays HSMP_STATUS_NOT_READY (its initialized value) for failures
+	 * caught before the message was ever sent, e.g. an invalid socket
+	 * index or buffer size.
+	 */
+	req.smu_response = smu_response;
+	if (copy_to_user(arguser, &req, sizeof(req)) && ret >= 0)
+		ret = -EFAULT;
+
 	if (ret < 0)
 		return ret;
 
@@ -667,7 +702,7 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
  * write while they bring sockets up and tear them down.
  */
 static ssize_t hsmp_metric_tbl_read_locked(struct hsmp_socket *sock, char *buf,
-					   size_t size)
+					   size_t size, u32 *smu_response)
 {
 	struct hsmp_message msg = { 0 };
 	int ret;
@@ -699,6 +734,8 @@ static ssize_t hsmp_metric_tbl_read_locked(struct hsmp_socket *sock, char *buf,
 	guard(mutex)(&sock->metric_read_lock);
 
 	ret = hsmp_send_message_locked(&msg);
+	if (smu_response)
+		*smu_response = msg.smu_response;
 	if (ret)
 		return ret;
 	memcpy_fromio(buf, sock->metric_tbl_addr, size);
@@ -710,7 +747,7 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
 {
 	guard(rwsem_read)(&hsmp_sock_rwsem);
 
-	return hsmp_metric_tbl_read_locked(sock, buf, size);
+	return hsmp_metric_tbl_read_locked(sock, buf, size, NULL);
 }
 EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_read, "AMD_HSMP");
 
-- 
2.34.1