* [PATCH] platform/x86/amd/hsmp: Add new error code and error logs
@ 2024-11-11 9:17 Suma Hegde
2024-11-11 11:26 ` Ilpo Järvinen
0 siblings, 1 reply; 2+ messages in thread
From: Suma Hegde @ 2024-11-11 9:17 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
Firmware is updated to send HSMP_ERR_PREREQ_NOT_SATISFIED(0xFD) and
HSMP_ERR_SMU_BUSY(0xFC) error codes. Add them here.
Add error logs to make it easy to understand the failure reason for
different error conditions.
Replace pr_err() with dev_err() to identify the driver printing the
error.
When file is opened in WRITE mode, then GET messages are not allowed
and when file is opened in READ mode, SET messages are not allowed.
In these scenerios, return EPERM error to userspace instead of
EINVALID.
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
drivers/platform/x86/amd/hsmp/hsmp.c | 40 +++++++++++++++++++---------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 82d8ba2e1204..f29dd93fbf0b 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -7,8 +7,6 @@
* This file provides a device implementation for HSMP interface
*/
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
#include <asm/amd_hsmp.h>
#include <asm/amd_nb.h>
@@ -25,6 +23,8 @@
#define HSMP_STATUS_OK 0x01
#define HSMP_ERR_INVALID_MSG 0xFE
#define HSMP_ERR_INVALID_INPUT 0xFF
+#define HSMP_ERR_PREREQ_NOT_SATISFIED 0xFD
+#define HSMP_ERR_SMU_BUSY 0xFC
/* Timeout in millsec */
#define HSMP_MSG_TIMEOUT 100
@@ -61,7 +61,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
mbox_status = HSMP_STATUS_NOT_READY;
ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_resp_off, &mbox_status, HSMP_WR);
if (ret) {
- pr_err("Error %d clearing mailbox status register\n", ret);
+ dev_err(sock->dev, "Error %d clearing mailbox status register\n", ret);
return ret;
}
@@ -71,7 +71,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_arg_off + (index << 2),
&msg->args[index], HSMP_WR);
if (ret) {
- pr_err("Error %d writing message argument %d\n", ret, index);
+ dev_err(sock->dev, "Error %d writing message argument %d\n", ret, index);
return ret;
}
index++;
@@ -80,7 +80,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
/* Write the message ID which starts the operation */
ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_id_off, &msg->msg_id, HSMP_WR);
if (ret) {
- pr_err("Error %d writing message ID %u\n", ret, msg->msg_id);
+ dev_err(sock->dev, "Error %d writing message ID %u\n", ret, msg->msg_id);
return ret;
}
@@ -97,7 +97,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
while (time_before(jiffies, timeout)) {
ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_resp_off, &mbox_status, HSMP_RD);
if (ret) {
- pr_err("Error %d reading mailbox status\n", ret);
+ dev_err(sock->dev, "Error %d reading mailbox status\n", ret);
return ret;
}
@@ -110,14 +110,28 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
}
if (unlikely(mbox_status == HSMP_STATUS_NOT_READY)) {
+ dev_err(sock->dev, "Message ID 0x%X failure : SMU tmeout (status = 0x%X)\n",
+ msg->msg_id, mbox_status);
return -ETIMEDOUT;
} else if (unlikely(mbox_status == HSMP_ERR_INVALID_MSG)) {
+ dev_err(sock->dev, "Message ID 0x%X failure : Invalid message (status = 0x%X)\n",
+ msg->msg_id, mbox_status);
return -ENOMSG;
} else if (unlikely(mbox_status == HSMP_ERR_INVALID_INPUT)) {
+ dev_err(sock->dev, "Message ID 0x%X failure : Invalid arguments (status = 0x%X)\n",
+ msg->msg_id, mbox_status);
return -EINVAL;
+ } else if (unlikely(mbox_status == HSMP_ERR_PREREQ_NOT_SATISFIED)) {
+ dev_err(sock->dev, "Message ID 0x%X failure : Prerequisite not satisfied (status = 0x%X)\n",
+ msg->msg_id, mbox_status);
+ return -EREMOTEIO;
+ } else if (unlikely(mbox_status == HSMP_ERR_SMU_BUSY)) {
+ dev_err(sock->dev, "Message ID 0x%X failure : SMU BUSY (status = 0x%X)\n",
+ msg->msg_id, mbox_status);
+ return -EBUSY;
} else if (unlikely(mbox_status != HSMP_STATUS_OK)) {
- pr_err("Message ID %u unknown failure (status = 0x%X)\n",
- msg->msg_id, mbox_status);
+ dev_err(sock->dev, "Message ID 0x%X unknown failure (status = 0x%X)\n",
+ msg->msg_id, mbox_status);
return -EIO;
}
@@ -133,8 +147,8 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_arg_off + (index << 2),
&msg->args[index], HSMP_RD);
if (ret) {
- pr_err("Error %d reading response %u for message ID:%u\n",
- ret, index, msg->msg_id);
+ dev_err(sock->dev, "Error %d reading response %u for message ID:%u\n",
+ ret, index, msg->msg_id);
break;
}
index++;
@@ -248,7 +262,7 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
* Execute only set/configure commands
*/
if (hsmp_msg_desc_table[msg.msg_id].type != HSMP_SET)
- return -EINVAL;
+ return -EPERM;
break;
case FMODE_READ:
/*
@@ -256,7 +270,7 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
* Execute only get/monitor commands
*/
if (hsmp_msg_desc_table[msg.msg_id].type != HSMP_GET)
- return -EINVAL;
+ return -EPERM;
break;
case FMODE_READ | FMODE_WRITE:
/*
@@ -265,7 +279,7 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
*/
break;
default:
- return -EINVAL;
+ return -EPERM;
}
ret = hsmp_send_message(&msg);
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] platform/x86/amd/hsmp: Add new error code and error logs
2024-11-11 9:17 [PATCH] platform/x86/amd/hsmp: Add new error code and error logs Suma Hegde
@ 2024-11-11 11:26 ` Ilpo Järvinen
0 siblings, 0 replies; 2+ messages in thread
From: Ilpo Järvinen @ 2024-11-11 11:26 UTC (permalink / raw)
To: Suma Hegde; +Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi
On Mon, 11 Nov 2024, Suma Hegde wrote:
> Firmware is updated to send HSMP_ERR_PREREQ_NOT_SATISFIED(0xFD) and
> HSMP_ERR_SMU_BUSY(0xFC) error codes. Add them here.
>
> Add error logs to make it easy to understand the failure reason for
> different error conditions.
>
> Replace pr_err() with dev_err() to identify the driver printing the
> error.
>
> When file is opened in WRITE mode, then GET messages are not allowed
> and when file is opened in READ mode, SET messages are not allowed.
> In these scenerios, return EPERM error to userspace instead of
> EINVALID.
Hi Suma,
Please split UAPI change into own patch as it's something that might have
to be reverted so I prefer to have that as minimal as possible.
--
i.
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> drivers/platform/x86/amd/hsmp/hsmp.c | 40 +++++++++++++++++++---------
> 1 file changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 82d8ba2e1204..f29dd93fbf0b 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -7,8 +7,6 @@
> * This file provides a device implementation for HSMP interface
> */
>
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> #include <asm/amd_hsmp.h>
> #include <asm/amd_nb.h>
>
> @@ -25,6 +23,8 @@
> #define HSMP_STATUS_OK 0x01
> #define HSMP_ERR_INVALID_MSG 0xFE
> #define HSMP_ERR_INVALID_INPUT 0xFF
> +#define HSMP_ERR_PREREQ_NOT_SATISFIED 0xFD
> +#define HSMP_ERR_SMU_BUSY 0xFC
>
> /* Timeout in millsec */
> #define HSMP_MSG_TIMEOUT 100
> @@ -61,7 +61,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
> mbox_status = HSMP_STATUS_NOT_READY;
> ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_resp_off, &mbox_status, HSMP_WR);
> if (ret) {
> - pr_err("Error %d clearing mailbox status register\n", ret);
> + dev_err(sock->dev, "Error %d clearing mailbox status register\n", ret);
> return ret;
> }
>
> @@ -71,7 +71,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
> ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_arg_off + (index << 2),
> &msg->args[index], HSMP_WR);
> if (ret) {
> - pr_err("Error %d writing message argument %d\n", ret, index);
> + dev_err(sock->dev, "Error %d writing message argument %d\n", ret, index);
> return ret;
> }
> index++;
> @@ -80,7 +80,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
> /* Write the message ID which starts the operation */
> ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_id_off, &msg->msg_id, HSMP_WR);
> if (ret) {
> - pr_err("Error %d writing message ID %u\n", ret, msg->msg_id);
> + dev_err(sock->dev, "Error %d writing message ID %u\n", ret, msg->msg_id);
> return ret;
> }
>
> @@ -97,7 +97,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
> while (time_before(jiffies, timeout)) {
> ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_resp_off, &mbox_status, HSMP_RD);
> if (ret) {
> - pr_err("Error %d reading mailbox status\n", ret);
> + dev_err(sock->dev, "Error %d reading mailbox status\n", ret);
> return ret;
> }
>
> @@ -110,14 +110,28 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
> }
>
> if (unlikely(mbox_status == HSMP_STATUS_NOT_READY)) {
> + dev_err(sock->dev, "Message ID 0x%X failure : SMU tmeout (status = 0x%X)\n",
> + msg->msg_id, mbox_status);
> return -ETIMEDOUT;
> } else if (unlikely(mbox_status == HSMP_ERR_INVALID_MSG)) {
> + dev_err(sock->dev, "Message ID 0x%X failure : Invalid message (status = 0x%X)\n",
> + msg->msg_id, mbox_status);
> return -ENOMSG;
> } else if (unlikely(mbox_status == HSMP_ERR_INVALID_INPUT)) {
> + dev_err(sock->dev, "Message ID 0x%X failure : Invalid arguments (status = 0x%X)\n",
> + msg->msg_id, mbox_status);
> return -EINVAL;
> + } else if (unlikely(mbox_status == HSMP_ERR_PREREQ_NOT_SATISFIED)) {
> + dev_err(sock->dev, "Message ID 0x%X failure : Prerequisite not satisfied (status = 0x%X)\n",
> + msg->msg_id, mbox_status);
> + return -EREMOTEIO;
> + } else if (unlikely(mbox_status == HSMP_ERR_SMU_BUSY)) {
> + dev_err(sock->dev, "Message ID 0x%X failure : SMU BUSY (status = 0x%X)\n",
> + msg->msg_id, mbox_status);
> + return -EBUSY;
> } else if (unlikely(mbox_status != HSMP_STATUS_OK)) {
> - pr_err("Message ID %u unknown failure (status = 0x%X)\n",
> - msg->msg_id, mbox_status);
> + dev_err(sock->dev, "Message ID 0x%X unknown failure (status = 0x%X)\n",
> + msg->msg_id, mbox_status);
> return -EIO;
> }
>
> @@ -133,8 +147,8 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
> ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_arg_off + (index << 2),
> &msg->args[index], HSMP_RD);
> if (ret) {
> - pr_err("Error %d reading response %u for message ID:%u\n",
> - ret, index, msg->msg_id);
> + dev_err(sock->dev, "Error %d reading response %u for message ID:%u\n",
> + ret, index, msg->msg_id);
> break;
> }
> index++;
> @@ -248,7 +262,7 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> * Execute only set/configure commands
> */
> if (hsmp_msg_desc_table[msg.msg_id].type != HSMP_SET)
> - return -EINVAL;
> + return -EPERM;
> break;
> case FMODE_READ:
> /*
> @@ -256,7 +270,7 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> * Execute only get/monitor commands
> */
> if (hsmp_msg_desc_table[msg.msg_id].type != HSMP_GET)
> - return -EINVAL;
> + return -EPERM;
> break;
> case FMODE_READ | FMODE_WRITE:
> /*
> @@ -265,7 +279,7 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> */
> break;
> default:
> - return -EINVAL;
> + return -EPERM;
> }
>
> ret = hsmp_send_message(&msg);
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-11-11 11:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-11 9:17 [PATCH] platform/x86/amd/hsmp: Add new error code and error logs Suma Hegde
2024-11-11 11:26 ` Ilpo Järvinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox