All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] platforms/x86: Add AMD system management interface
@ 2022-02-03 12:04 Naveen Krishna Chatradhi
  2022-02-03 12:04 ` [PATCH v2 2/2] Documentation: Add x86/amd_hsmp driver Naveen Krishna Chatradhi
  2022-02-03 15:48 ` [PATCH v2 1/2] platforms/x86: Add AMD system management interface Nathan Fontenot
  0 siblings, 2 replies; 8+ messages in thread
From: Naveen Krishna Chatradhi @ 2022-02-03 12:04 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hdegoede, Siva.Sathappan, carlos.bilbao, Nathan.Fontenot,
	Suma Hegde, Naveen Krishna Chatradhi

From: Suma Hegde <suma.hegde@amd.com>

Recent Fam19h EPYC server line of processors from AMD support system
management functionality via HSMP (Host System Management Port) interface.

The Host System Management Port (HSMP) is an interface to provide
OS-level software with access to system management functions via a
set of mailbox registers.

More details on the interface can be found in chapter
"7 Host System Management Port (HSMP)" of the following PPR
https://www.amd.com/system/files/TechDocs/55898_B1_pub_0.50.zip

This patch adds new amd_hsmp module under the drivers/platforms/x86/
which creates miscdevice with an IOCTL interface to the user space.
/dev/hsmp is for running the hsmp mailbox commands.

Major features support by the interface include monitor and/or control of
a. boostlimit
b. current power, power limit, max power limit
c. c0 residency
d. prochot status
e. clocks
f. ddr bandwidth, utilization
g. data fabric P-state

Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>
---
Changes since v1:
1. Add supported model check
   . This interface is supported only on server line of CPUs.
2. Handle Reserved messages
3. Add brief descriptions of the Messages
4. Add Carlos Bilbao's reviewed-by

 .../userspace-api/ioctl/ioctl-number.rst      |   2 +
 arch/x86/include/asm/amd_hsmp.h               |  16 +
 arch/x86/include/uapi/asm/amd_hsmp.h          |  56 +++
 drivers/platform/x86/Kconfig                  |  13 +
 drivers/platform/x86/Makefile                 |   1 +
 drivers/platform/x86/amd_hsmp.c               | 450 ++++++++++++++++++
 6 files changed, 538 insertions(+)
 create mode 100644 arch/x86/include/asm/amd_hsmp.h
 create mode 100644 arch/x86/include/uapi/asm/amd_hsmp.h
 create mode 100644 drivers/platform/x86/amd_hsmp.c

diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 687efcf245c1..663e316d320c 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -372,6 +372,8 @@ Code  Seq#    Include File                                           Comments
                                                                      <mailto:thomas@winischhofer.net>
 0xF6  all                                                            LTTng Linux Trace Toolkit Next Generation
                                                                      <mailto:mathieu.desnoyers@efficios.com>
+0xF8  all    arch/x86/include/uapi/asm/amd_hsmp.h                    AMD HSMP EPYC system management interface driver
+                                                                     <mailto:nchatrad@amd.com>
 0xFD  all    linux/dm-ioctl.h
 0xFE  all    linux/isst_if.h
 ====  =====  ======================================================= ================================================================
diff --git a/arch/x86/include/asm/amd_hsmp.h b/arch/x86/include/asm/amd_hsmp.h
new file mode 100644
index 000000000000..db2846bb3c37
--- /dev/null
+++ b/arch/x86/include/asm/amd_hsmp.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _ASM_X86_AMD_HSMP_H_
+#define _ASM_X86_AMD_HSMP_H_
+
+#include <uapi/asm/amd_hsmp.h>
+
+#if (defined(CONFIG_AMD_HSMP) || defined(CONFIG_AMD_HSMP_MODULE))
+int hsmp_send_message(struct hsmp_message *msg);
+#else
+int hsmp_send_message(struct hsmp_message *msg)
+{
+	return -ENODEV;
+}
+#endif
+#endif /*_ASM_X86_AMD_HSMP_H_*/
diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
new file mode 100644
index 000000000000..42cdac8a331d
--- /dev/null
+++ b/arch/x86/include/uapi/asm/amd_hsmp.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _UAPI_ASM_X86_AMD_HSMP_H_
+#define _UAPI_ASM_X86_AMD_HSMP_H_
+
+#include <linux/types.h>
+
+#pragma pack(4)
+
+#define HSMP_MAX_MSG_LEN 8
+
+/*
+ * HSMP Messages supported
+ */
+enum hsmp_message_ids {
+	HSMP_TEST = 1,			/* 01h Increments input value by 1 */
+	HSMP_GET_SMU_VER,		/* 02h SMU FW version */
+	HSMP_GET_PROTO_VER,		/* 03h HSMP interface version */
+	HSMP_GET_SOCKET_POWER,		/* 04h average package power consumption */
+	HSMP_SET_SOCKET_POWER_LIMIT,	/* 05h Set the socket power limit */
+	HSMP_GET_SOCKET_POWER_LIMIT,	/* 06h Get current socket power limit */
+	HSMP_GET_SOCKET_POWER_LIMIT_MAX,/* 07h Get maximum socket power value */
+	HSMP_SET_BOOST_LIMIT,		/* 08h Set a core maximum frequency limit */
+	HSMP_SET_BOOST_LIMIT_SOCKET,	/* 09h Set socket maximum frequency level */
+	HSMP_GET_BOOST_LIMIT,		/* 0Ah Get current frequency limit */
+	HSMP_GET_PROC_HOT,		/* 0Bh Get PROCHOT status */
+	HSMP_SET_XGMI_LINK_WIDTH,	/* 0Ch Set max and min width of xGMI Link */
+	HSMP_SET_DF_PSTATE,		/* 0Dh Alter APEnable/Disable messages behavior */
+	HSMP_SET_AUTO_DF_PSTATE,	/* 0Eh Enable DF P-State Performance Boost algorithm */
+	HSMP_GET_FCLK_MCLK,		/* 0Fh Get FCLK and MEMCLK for current socket */
+	HSMP_GET_CCLK_THROTTLE_LIMIT,	/* 10h Get CCLK frequency limit in socket */
+	HSMP_GET_C0_PERCENT,		/* 11h Get average C0 residency in socket */
+	HSMP_SET_NBIO_DPM_LEVEL,	/* 12h Set max/min LCLK DPM Level for a given NBIO */
+	HSMP_RESERVED,			/* 13h Reserved */
+	HSMP_GET_DDR_BANDWIDTH,		/* 14h Get theoretical maximum and current DDR Bandwidth */
+	HSMP_GET_TEMP_MONITOR,		/* 15h Get per-DIMM temperature and refresh rates */
+	HSMP_MSG_ID_MAX,
+};
+
+struct hsmp_message {
+	__u32	msg_id;				/* Message ID */
+	__u16	num_args;			/* Number of arguments in message */
+	__u16	response_sz;			/* Number of expected response words */
+	__u32	args[HSMP_MAX_MSG_LEN];		/* Argument(s) */
+	__u32	response[HSMP_MAX_MSG_LEN];	/* Response word(s) */
+	__u16	sock_ind;			/* socket number */
+};
+
+/* Reset to default packing */
+#pragma pack()
+
+/* Define unique ioctl command for hsmp msgs using generic _IOWR */
+#define HSMP_BASE_IOCTL_NR			0xF8
+#define HSMP_IOCTL_CMD				_IOWR(HSMP_BASE_IOCTL_NR, 0, struct hsmp_message)
+
+#endif /*_ASM_X86_AMD_HSMP_H_*/
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 24deeeb29af2..0906c36ea07b 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -210,6 +210,19 @@ config AMD_PMC
 	  If you choose to compile this driver as a module the module will be
 	  called amd-pmc.
 
+config AMD_HSMP
+	tristate "AMD HSMP Driver"
+	depends on AMD_NB && X86_64
+	help
+	  The driver provides a way for user space tools to monitor and manage
+	  system management functionality on EPYC server CPUs from AMD.
+
+	  Host System Management Port (HSMP) interface is a mailbox interface
+	  between the x86 core and the System Management Unit (SMU) firmware.
+
+	  If you choose to compile this driver as a module the module will be
+	  called amd_hsmp.
+
 config ADV_SWBUTTON
 	tristate "Advantech ACPI Software Button Driver"
 	depends on ACPI && INPUT
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index c12a9b044fd8..b3a93a5053a3 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
 
 # AMD
 obj-$(CONFIG_AMD_PMC)		+= amd-pmc.o
+obj-$(CONFIG_AMD_HSMP)		+= amd_hsmp.o
 
 # Advantech
 obj-$(CONFIG_ADV_SWBUTTON)	+= adv_swbutton.o
diff --git a/drivers/platform/x86/amd_hsmp.c b/drivers/platform/x86/amd_hsmp.c
new file mode 100644
index 000000000000..29b22db726bf
--- /dev/null
+++ b/drivers/platform/x86/amd_hsmp.c
@@ -0,0 +1,450 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD HSMP Platform Driver
+ * Copyright (c) 2022, AMD.
+ * All Rights Reserved.
+ *
+ * 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>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/semaphore.h>
+
+#define DRIVER_NAME		"amd_hsmp"
+#define DRIVER_VERSION		"1.0"
+
+/* HSMP Status / Error codes */
+#define HSMP_STATUS_NOT_READY	0x00
+#define HSMP_STATUS_OK		0x01
+#define HSMP_ERR_INVALID_MSG	0xFE
+#define HSMP_ERR_INVALID_INPUT	0xFF
+
+/* Timeout in millsec */
+#define HSMP_MSG_TIMEOUT	100
+#define HSMP_SHORT_SLEEP	1
+
+#define HSMP_WR			true
+#define HSMP_RD			false
+
+/*
+ * To access specific HSMP mailbox register, s/w writes the SMN address of HSMP mailbox
+ * register into the SMN_INDEX register, and reads/writes the SMN_DATA reg.
+ * Below are required SMN address for HSMP Mailbox register offsets in SMU address space
+ */
+#define SMN_HSMP_MSG_ID		0x3B10534
+#define SMN_HSMP_MSG_RESP	0x3B10980
+#define SMN_HSMP_MSG_DATA	0x3B109E0
+
+#define HSMP_INDEX_REG		0xc4
+#define HSMP_DATA_REG		0xc8
+
+static struct semaphore *hsmp_sem;
+
+static struct miscdevice hsmp_device;
+
+/* List of "Configure/SET" msgs */
+static u32 hsmp_set_msgs[] = {
+	HSMP_SET_SOCKET_POWER_LIMIT,
+	HSMP_SET_BOOST_LIMIT,
+	HSMP_SET_BOOST_LIMIT_SOCKET,
+	HSMP_SET_XGMI_LINK_WIDTH,
+	HSMP_SET_DF_PSTATE,
+	HSMP_SET_AUTO_DF_PSTATE,
+	HSMP_SET_NBIO_DPM_LEVEL
+};
+
+/* List of "Monitor/GET" msgs */
+static u32 hsmp_get_msgs[] = {
+	HSMP_TEST,
+	HSMP_GET_SMU_VER,
+	HSMP_GET_PROTO_VER,
+	HSMP_GET_SOCKET_POWER,
+	HSMP_GET_SOCKET_POWER_LIMIT,
+	HSMP_GET_SOCKET_POWER_LIMIT_MAX,
+	HSMP_GET_BOOST_LIMIT,
+	HSMP_GET_PROC_HOT,
+	HSMP_GET_FCLK_MCLK,
+	HSMP_GET_CCLK_THROTTLE_LIMIT,
+	HSMP_GET_C0_PERCENT,
+	HSMP_GET_DDR_BANDWIDTH,
+	HSMP_GET_TEMP_MONITOR
+};
+
+static int amd_hsmp_rdwr(struct pci_dev *root, u32 address,
+			 u32 *value, bool write)
+{
+	int ret;
+
+	ret = pci_write_config_dword(root, HSMP_INDEX_REG, address);
+	if (ret)
+		return ret;
+
+	ret = (write ? pci_write_config_dword(root, HSMP_DATA_REG, *value)
+		     : pci_read_config_dword(root, HSMP_DATA_REG, value));
+
+	return ret;
+}
+
+/*
+ * Send a message to the HSMP port via PCI-e config space registers.
+ *
+ * The caller is expected to zero out any unused arguments.
+ * If a response is expected, the number of response words should be greater than 0.
+ *
+ * Returns 0 for success and populates the requested number of arguments.
+ * Returns a negative error code for failure.
+ */
+static int __hsmp_send_message(struct pci_dev *root, struct hsmp_message *msg)
+{
+	unsigned long timeout, short_sleep;
+	u32 mbox_status;
+	u32 arg_num;
+	int ret;
+
+	/* Clear the status register */
+	mbox_status = HSMP_STATUS_NOT_READY;
+	ret = amd_hsmp_rdwr(root, SMN_HSMP_MSG_RESP, &mbox_status, HSMP_WR);
+	if (ret) {
+		pr_err("Error %d clearing mailbox status register\n", ret);
+		return ret;
+	}
+
+	arg_num = 0;
+	/* Write any message arguments */
+	while (arg_num < msg->num_args) {
+		ret = amd_hsmp_rdwr(root, SMN_HSMP_MSG_DATA + (arg_num << 2),
+				    &msg->args[arg_num], HSMP_WR);
+		if (ret) {
+			pr_err("Error %d writing message argument %d\n", ret, arg_num);
+			return ret;
+		}
+		arg_num++;
+	}
+
+	/* Write the message ID which starts the operation */
+	ret = amd_hsmp_rdwr(root, SMN_HSMP_MSG_ID, &msg->msg_id, HSMP_WR);
+	if (ret) {
+		pr_err("Error %d writing message ID %u\n", ret, msg->msg_id);
+		return ret;
+	}
+
+	/*
+	 * Depending on when the trigger write completes relative to the SMU
+	 * firmware 1 ms cycle, the operation may take from tens of us to 1 ms
+	 * to complete. Some operations may take more. Therefore we will try
+	 * a few short duration sleeps and switch to long sleeps if we don't
+	 * succeed quickly.
+	 */
+	short_sleep = jiffies + msecs_to_jiffies(HSMP_SHORT_SLEEP);
+	timeout	= jiffies + msecs_to_jiffies(HSMP_MSG_TIMEOUT);
+
+	while (time_before(jiffies, timeout)) {
+		ret = amd_hsmp_rdwr(root, SMN_HSMP_MSG_RESP, &mbox_status, HSMP_RD);
+		if (ret) {
+			pr_err("Error %d reading mailbox status\n", ret);
+			return ret;
+		}
+
+		if (mbox_status != HSMP_STATUS_NOT_READY)
+			break;
+		if (time_before(jiffies, short_sleep))
+			usleep_range(50, 100);
+		else
+			usleep_range(1000, 2000);
+	}
+
+	if (unlikely(mbox_status == HSMP_STATUS_NOT_READY)) {
+		return -ETIMEDOUT;
+	} else if (unlikely(mbox_status == HSMP_ERR_INVALID_MSG)) {
+		return -ENOMSG;
+	} else if (unlikely(mbox_status == HSMP_ERR_INVALID_INPUT)) {
+		return -EINVAL;
+	} else if (unlikely(mbox_status != HSMP_STATUS_OK)) {
+		pr_err("Message ID %u unknown failure (status = 0x%X)\n",
+		       msg->msg_id, mbox_status);
+		return -EIO;
+	}
+
+	/* SMU has responded OK. Read response data */
+	arg_num = 0;
+	while (arg_num < msg->response_sz) {
+		ret = amd_hsmp_rdwr(root, SMN_HSMP_MSG_DATA + (arg_num << 2),
+				    &msg->response[arg_num], HSMP_RD);
+		if (ret) {
+			pr_err("Error %d reading response %u for message ID:%u\n",
+			       ret, arg_num, msg->msg_id);
+			break;
+		}
+		arg_num++;
+	}
+
+	return ret;
+}
+
+int hsmp_send_message(struct hsmp_message *msg)
+{
+	struct amd_northbridge *nb;
+	int ret;
+
+	if (!msg)
+		return -EINVAL;
+
+	nb = node_to_amd_nb(msg->sock_ind);
+	if (!nb || !nb->root)
+		return -ENODEV;
+
+	if (msg->msg_id < HSMP_TEST || msg->msg_id >= HSMP_MSG_ID_MAX ||
+	    msg->msg_id == HSMP_RESERVED)
+		return -EINVAL;
+
+	if (msg->num_args > HSMP_MAX_MSG_LEN || msg->response_sz > HSMP_MAX_MSG_LEN)
+		return -EINVAL;
+
+	/*
+	 * The time taken by smu operation to complete is between
+	 * 10us to 1ms. Sometime it may take more time.
+	 * In SMP system timeout of 100 millisecs should
+	 * be enough for the previous thread to finish the operation
+	 */
+	ret = down_timeout(&hsmp_sem[msg->sock_ind],
+			   msecs_to_jiffies(HSMP_MSG_TIMEOUT));
+	if (ret < 0)
+		return ret;
+
+	ret = __hsmp_send_message(nb->root, msg);
+
+	up(&hsmp_sem[msg->sock_ind]);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(hsmp_send_message);
+
+static int hsmp_test(u16 sock_ind)
+{
+	struct hsmp_message msg = { 0 };
+	struct amd_northbridge *nb;
+	int ret = -ENODEV;
+
+	nb = node_to_amd_nb(sock_ind);
+	if (!nb || !nb->root)
+		return ret;
+
+	/*
+	 * Test the hsmp port by performing TEST command. The test message takes
+	 * one argument and returns the value of that argument + 1.
+	 */
+	msg.sock_ind	= sock_ind;
+	msg.response_sz = 1;
+	msg.num_args	= 1;
+	msg.msg_id	= HSMP_TEST;
+	msg.args[0]	= 0xDEADBEEF;
+
+	ret = __hsmp_send_message(nb->root, &msg);
+	if (ret)
+		return ret;
+
+	if (msg.response[0] != (msg.args[0] + 1)) {
+		pr_err("Socket %d test message failed, Expected 0x%08X, received 0x%08X\n",
+		       sock_ind, msg.args[0] + 1, msg.response[0]);
+		return -EBADE;
+	}
+
+	return 0;
+}
+
+static int search_msg_list(u32 msgid, u32 *list, int size)
+{
+	int i;
+
+	for (i = 0; i < size; i++) {
+		if (msgid == list[i])
+			return 0;
+	}
+	return -EINVAL;
+}
+
+static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+{
+	int __user *arguser = (int  __user *)arg;
+	struct hsmp_message msg = { 0 };
+	int ret;
+
+	if (copy_struct_from_user(&msg, sizeof(msg), arguser, sizeof(struct hsmp_message)))
+		return -EFAULT;
+
+	switch (fp->f_mode & (FMODE_WRITE | FMODE_READ)) {
+	case FMODE_WRITE:
+		/* check if the msgid is a set msg */
+		ret = search_msg_list(msg.msg_id, hsmp_set_msgs, ARRAY_SIZE(hsmp_set_msgs));
+		if (ret)
+			return ret;
+
+		return hsmp_send_message(&msg);
+	case FMODE_READ:
+		/* check if the msgid is a get msg */
+		ret = search_msg_list(msg.msg_id, hsmp_get_msgs, ARRAY_SIZE(hsmp_get_msgs));
+		if (ret)
+			return ret;
+		break;
+	case FMODE_READ | FMODE_WRITE:
+		ret = search_msg_list(msg.msg_id, hsmp_set_msgs, ARRAY_SIZE(hsmp_set_msgs));
+		if (!ret)
+			return hsmp_send_message(&msg);
+
+		ret = search_msg_list(msg.msg_id, hsmp_get_msgs, ARRAY_SIZE(hsmp_get_msgs));
+		if (ret)
+			return ret;
+
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret =  hsmp_send_message(&msg);
+	if (ret)
+		return ret;
+
+	if (copy_to_user(arguser, &msg, sizeof(struct hsmp_message)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static const struct file_operations hsmp_fops = {
+	.owner		= THIS_MODULE,
+	.unlocked_ioctl	= hsmp_ioctl,
+	.compat_ioctl	= hsmp_ioctl,
+};
+
+static int hsmp_pltdrv_probe(struct platform_device *pdev)
+{
+	int ret, i;
+
+	hsmp_sem = devm_kzalloc(&pdev->dev,
+				(amd_nb_num() * sizeof(struct semaphore)),
+				GFP_KERNEL);
+	if (!hsmp_sem)
+		return -ENOMEM;
+
+	for (i = 0; i < amd_nb_num(); i++)
+		sema_init(&hsmp_sem[i], 1);
+
+	hsmp_device.name	= "hsmp_cdev";
+	hsmp_device.minor	= MISC_DYNAMIC_MINOR;
+	hsmp_device.fops	= &hsmp_fops;
+	hsmp_device.parent	= &pdev->dev;
+	hsmp_device.nodename	= "hsmp";
+	hsmp_device.mode	= 0644;
+
+	ret = misc_register(&hsmp_device);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int hsmp_pltdrv_remove(struct platform_device *pdev)
+{
+	misc_deregister(&hsmp_device);
+
+	return 0;
+}
+
+static struct platform_driver amd_hsmp_driver = {
+	.probe		= hsmp_pltdrv_probe,
+	.remove		= hsmp_pltdrv_remove,
+	.driver		= {
+		.name	= DRIVER_NAME,
+	},
+};
+
+static bool check_model_support(void)
+{
+	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
+	    boot_cpu_data.x86 != 0x19)
+		return false;
+
+	switch (boot_cpu_data.x86_model) {
+	case 0x00 ... 0x1f:
+	case 0x30 ... 0x3f:
+		return true;
+	default:
+		return false;
+	}
+
+	return false;
+}
+
+static struct platform_device *amd_hsmp_platdev;
+
+static int __init hsmp_plt_init(void)
+{
+	int ret = -ENODEV;
+	u16 num_sockets;
+	int i;
+
+	if (!check_model_support()) {
+		pr_err("HSMP is not supported on Family:%x model:%x\n",
+		       boot_cpu_data.x86, boot_cpu_data.x86_model);
+		return ret;
+	}
+
+	/*
+	 * amd_nb_num() returns number of SMN/DF interfaces present in the system
+	 * if we have N SMN/DF interfaces that ideally means N sockets
+	 */
+	num_sockets = amd_nb_num();
+	if (num_sockets == 0)
+		return ret;
+
+	/* Test the hsmp interface on each socket */
+	for (i = 0; i < num_sockets; i++) {
+		ret = hsmp_test(i);
+		if (ret)
+			return ret;
+	}
+
+	ret = platform_driver_register(&amd_hsmp_driver);
+	if (ret)
+		return ret;
+
+	amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, -1);
+	if (!amd_hsmp_platdev) {
+		ret = -ENOMEM;
+		goto drv_unregister;
+	}
+
+	ret = platform_device_add(amd_hsmp_platdev);
+	if (ret) {
+		platform_device_put(amd_hsmp_platdev);
+		goto drv_unregister;
+	}
+
+	return 0;
+
+drv_unregister:
+	platform_driver_unregister(&amd_hsmp_driver);
+	return ret;
+}
+
+static void __exit hsmp_plt_exit(void)
+{
+	platform_device_unregister(amd_hsmp_platdev);
+	platform_driver_unregister(&amd_hsmp_driver);
+}
+
+device_initcall(hsmp_plt_init);
+module_exit(hsmp_plt_exit);
+
+MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver");
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] Documentation: Add x86/amd_hsmp driver
  2022-02-03 12:04 [PATCH v2 1/2] platforms/x86: Add AMD system management interface Naveen Krishna Chatradhi
@ 2022-02-03 12:04 ` Naveen Krishna Chatradhi
  2022-02-03 15:52   ` Nathan Fontenot
  2022-02-03 15:48 ` [PATCH v2 1/2] platforms/x86: Add AMD system management interface Nathan Fontenot
  1 sibling, 1 reply; 8+ messages in thread
From: Naveen Krishna Chatradhi @ 2022-02-03 12:04 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hdegoede, Siva.Sathappan, carlos.bilbao, Nathan.Fontenot,
	Naveen Krishna Chatradhi

This documentation for amd_hsmp driver explains how to use the
device interface.

Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Changes since v1:
None

 Documentation/x86/amd_hsmp.rst | 93 ++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/x86/amd_hsmp.rst

diff --git a/Documentation/x86/amd_hsmp.rst b/Documentation/x86/amd_hsmp.rst
new file mode 100644
index 000000000000..b77b888f810a
--- /dev/null
+++ b/Documentation/x86/amd_hsmp.rst
@@ -0,0 +1,93 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============================================
+AMD HSMP interface
+============================================
+
+Newer Fam19h EPYC server line of processors from AMD support system
+management functionality via HSMP (Host System Management Port).
+
+The Host System Management Port (HSMP) is an interface to provide
+OS-level software with access to system management functions via a
+set of mailbox registers.
+
+More details on the interface can be found in chapter
+"7 Host System Management Port (HSMP)" of the following PPR
+https://www.amd.com/system/files/TechDocs/55898_B1_pub_0.50.zip
+
+
+HSMP device
+============================================
+
+amd_hsmp driver under the drivers/platforms/x86/ creates miscdevice
+/dev/hsmp to let user space programs run hsmp mailbox commands.
+
+$ ls -al /dev/hsmp
+crw-r--r-- 1 root root 10, 123 Jan 21 21:41 /dev/hsmp
+
+Characteristics of the dev node:
+ * Write mode is used for running set/configure commands
+ * Read mode is used for running get/status monitor commands
+
+Access restrictions:
+ * Only root user is allowed to open the file in write mode.
+ * The file can be opened in read mode by all the users.
+
+In-kernel integration:
+ * Other subsystems in the kernel can use the exported transport
+   function hsmp_send_message().
+ * Locking across callers is taken care by the driver.
+
+Features support by the interface include monitor and/or control of
+a. boostlimit
+b. current power, power limit, max power limit
+c. c0 residency
+d. prochot status
+e. clocks (fclk, mclk and cclk)
+f. ddr bandwidth, utilization
+g. data fabric P-state
+
+
+An example
+==========
+
+To access hsmp device from a C program.
+First, you need to include the headers::
+
+  #include <linux/amd_hsmp.h>
+Which defines the supported messages/message IDs.
+
+Next thing, open the device file, as follows::
+
+  int file;
+
+  file = open("/dev/hsmp", O_RDWR);
+  if (file < 0) {
+    /* ERROR HANDLING; you can check errno to see what went wrong */
+    exit(1);
+  }
+
+The following IOCTL is defined:
+
+``ioctl(file, HSMP_IOCTL_CMD, struct hsmp_message *msg)``
+  The argument is a pointer to a::
+
+    struct hsmp_message {
+	__u32	msg_id;				/* Message ID */
+	__u16	num_args;			/* Number of arguments in message */
+	__u16	response_sz;			/* Number of expected response words */
+	__u32	args[HSMP_MAX_MSG_LEN];		/* Argument(s) */
+	__u32	response[HSMP_MAX_MSG_LEN];	/* Response word(s) */
+	__u16	sock_ind;			/* socket number */
+    };
+
+The ioctl would return a non-zero on failure; you can read errno to see
+what happened. The transaction returns 0 on success.
+
+More details on the interface can be found in chapter
+"7 Host System Management Port (HSMP)" of the following PPR
+https://www.amd.com/system/files/TechDocs/55898_B1_pub_0.50.zip
+
+User space C-APIs are made available by linking against the esmi library,
+which is provided by the E-SMS project https://developer.amd.com/e-sms/.
+See: https://github.com/amd/esmi_ib_library
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] platforms/x86: Add AMD system management interface
  2022-02-03 12:04 [PATCH v2 1/2] platforms/x86: Add AMD system management interface Naveen Krishna Chatradhi
  2022-02-03 12:04 ` [PATCH v2 2/2] Documentation: Add x86/amd_hsmp driver Naveen Krishna Chatradhi
@ 2022-02-03 15:48 ` Nathan Fontenot
  2022-02-03 16:03   ` Carlos Bilbao
  1 sibling, 1 reply; 8+ messages in thread
From: Nathan Fontenot @ 2022-02-03 15:48 UTC (permalink / raw)
  To: Naveen Krishna Chatradhi, platform-driver-x86
  Cc: hdegoede, Siva.Sathappan, carlos.bilbao, Nathan.Fontenot, Suma Hegde

On 2/3/22 06:04, Naveen Krishna Chatradhi wrote:
> From: Suma Hegde <suma.hegde@amd.com>
> 
> Recent Fam19h EPYC server line of processors from AMD support system
> management functionality via HSMP (Host System Management Port) interface.
> 
> The Host System Management Port (HSMP) is an interface to provide
> OS-level software with access to system management functions via a
> set of mailbox registers.
> 
> More details on the interface can be found in chapter
> "7 Host System Management Port (HSMP)" of the following PPR
> https://www.amd.com/system/files/TechDocs/55898_B1_pub_0.50.zip
> 
> This patch adds new amd_hsmp module under the drivers/platforms/x86/
> which creates miscdevice with an IOCTL interface to the user space.
> /dev/hsmp is for running the hsmp mailbox commands.
> 
> Major features support by the interface include monitor and/or control of
> a. boostlimit
> b. current power, power limit, max power limit
> c. c0 residency
> d. prochot status
> e. clocks
> f. ddr bandwidth, utilization
> g. data fabric P-state
> 
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
> Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>
> ---
> Changes since v1:
> 1. Add supported model check
>    . This interface is supported only on server line of CPUs.
> 2. Handle Reserved messages
> 3. Add brief descriptions of the Messages
> 4. Add Carlos Bilbao's reviewed-by
> 
>  .../userspace-api/ioctl/ioctl-number.rst      |   2 +
>  arch/x86/include/asm/amd_hsmp.h               |  16 +
>  arch/x86/include/uapi/asm/amd_hsmp.h          |  56 +++
>  drivers/platform/x86/Kconfig                  |  13 +
>  drivers/platform/x86/Makefile                 |   1 +
>  drivers/platform/x86/amd_hsmp.c               | 450 ++++++++++++++++++
>  6 files changed, 538 insertions(+)
>  create mode 100644 arch/x86/include/asm/amd_hsmp.h
>  create mode 100644 arch/x86/include/uapi/asm/amd_hsmp.h
>  create mode 100644 drivers/platform/x86/amd_hsmp.c
> 
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 687efcf245c1..663e316d320c 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -372,6 +372,8 @@ Code  Seq#    Include File                                           Comments
>                                                                       <mailto:thomas@winischhofer.net>
>  0xF6  all                                                            LTTng Linux Trace Toolkit Next Generation
>                                                                       <mailto:mathieu.desnoyers@efficios.com>
> +0xF8  all    arch/x86/include/uapi/asm/amd_hsmp.h                    AMD HSMP EPYC system management interface driver
> +                                                                     <mailto:nchatrad@amd.com>
>  0xFD  all    linux/dm-ioctl.h
>  0xFE  all    linux/isst_if.h
>  ====  =====  ======================================================= ================================================================
> diff --git a/arch/x86/include/asm/amd_hsmp.h b/arch/x86/include/asm/amd_hsmp.h
> new file mode 100644
> index 000000000000..db2846bb3c37
> --- /dev/null
> +++ b/arch/x86/include/asm/amd_hsmp.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +
> +#ifndef _ASM_X86_AMD_HSMP_H_
> +#define _ASM_X86_AMD_HSMP_H_
> +
> +#include <uapi/asm/amd_hsmp.h>
> +
> +#if (defined(CONFIG_AMD_HSMP) || defined(CONFIG_AMD_HSMP_MODULE))
> +int hsmp_send_message(struct hsmp_message *msg);
> +#else
> +int hsmp_send_message(struct hsmp_message *msg)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +#endif /*_ASM_X86_AMD_HSMP_H_*/
> diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
> new file mode 100644
> index 000000000000..42cdac8a331d
> --- /dev/null
> +++ b/arch/x86/include/uapi/asm/amd_hsmp.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +
> +#ifndef _UAPI_ASM_X86_AMD_HSMP_H_
> +#define _UAPI_ASM_X86_AMD_HSMP_H_
> +
> +#include <linux/types.h>
> +
> +#pragma pack(4)
> +
> +#define HSMP_MAX_MSG_LEN 8
> +
> +/*
> + * HSMP Messages supported
> + */
> +enum hsmp_message_ids {
> +	HSMP_TEST = 1,			/* 01h Increments input value by 1 */
> +	HSMP_GET_SMU_VER,		/* 02h SMU FW version */
> +	HSMP_GET_PROTO_VER,		/* 03h HSMP interface version */
> +	HSMP_GET_SOCKET_POWER,		/* 04h average package power consumption */
> +	HSMP_SET_SOCKET_POWER_LIMIT,	/* 05h Set the socket power limit */
> +	HSMP_GET_SOCKET_POWER_LIMIT,	/* 06h Get current socket power limit */
> +	HSMP_GET_SOCKET_POWER_LIMIT_MAX,/* 07h Get maximum socket power value */
> +	HSMP_SET_BOOST_LIMIT,		/* 08h Set a core maximum frequency limit */
> +	HSMP_SET_BOOST_LIMIT_SOCKET,	/* 09h Set socket maximum frequency level */
> +	HSMP_GET_BOOST_LIMIT,		/* 0Ah Get current frequency limit */
> +	HSMP_GET_PROC_HOT,		/* 0Bh Get PROCHOT status */
> +	HSMP_SET_XGMI_LINK_WIDTH,	/* 0Ch Set max and min width of xGMI Link */
> +	HSMP_SET_DF_PSTATE,		/* 0Dh Alter APEnable/Disable messages behavior */
> +	HSMP_SET_AUTO_DF_PSTATE,	/* 0Eh Enable DF P-State Performance Boost algorithm */
> +	HSMP_GET_FCLK_MCLK,		/* 0Fh Get FCLK and MEMCLK for current socket */
> +	HSMP_GET_CCLK_THROTTLE_LIMIT,	/* 10h Get CCLK frequency limit in socket */
> +	HSMP_GET_C0_PERCENT,		/* 11h Get average C0 residency in socket */
> +	HSMP_SET_NBIO_DPM_LEVEL,	/* 12h Set max/min LCLK DPM Level for a given NBIO */
> +	HSMP_RESERVED,			/* 13h Reserved */
> +	HSMP_GET_DDR_BANDWIDTH,		/* 14h Get theoretical maximum and current DDR Bandwidth */
> +	HSMP_GET_TEMP_MONITOR,		/* 15h Get per-DIMM temperature and refresh rates */
> +	HSMP_MSG_ID_MAX,
> +};
> +
> +struct hsmp_message {
> +	__u32	msg_id;				/* Message ID */
> +	__u16	num_args;			/* Number of arguments in message */
> +	__u16	response_sz;			/* Number of expected response words */
> +	__u32	args[HSMP_MAX_MSG_LEN];		/* Argument(s) */
> +	__u32	response[HSMP_MAX_MSG_LEN];	/* Response word(s) */
> +	__u16	sock_ind;			/* socket number */
> +};
> +
> +/* Reset to default packing */
> +#pragma pack()
> +
> +/* Define unique ioctl command for hsmp msgs using generic _IOWR */
> +#define HSMP_BASE_IOCTL_NR			0xF8
> +#define HSMP_IOCTL_CMD				_IOWR(HSMP_BASE_IOCTL_NR, 0, struct hsmp_message)
> +
> +#endif /*_ASM_X86_AMD_HSMP_H_*/
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 24deeeb29af2..0906c36ea07b 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -210,6 +210,19 @@ config AMD_PMC
>  	  If you choose to compile this driver as a module the module will be
>  	  called amd-pmc.
>  
> +config AMD_HSMP
> +	tristate "AMD HSMP Driver"
> +	depends on AMD_NB && X86_64
> +	help
> +	  The driver provides a way for user space tools to monitor and manage
> +	  system management functionality on EPYC server CPUs from AMD.
> +
> +	  Host System Management Port (HSMP) interface is a mailbox interface
> +	  between the x86 core and the System Management Unit (SMU) firmware.
> +
> +	  If you choose to compile this driver as a module the module will be
> +	  called amd_hsmp.
> +
>  config ADV_SWBUTTON
>  	tristate "Advantech ACPI Software Button Driver"
>  	depends on ACPI && INPUT
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index c12a9b044fd8..b3a93a5053a3 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
>  
>  # AMD
>  obj-$(CONFIG_AMD_PMC)		+= amd-pmc.o
> +obj-$(CONFIG_AMD_HSMP)		+= amd_hsmp.o
>  
>  # Advantech
>  obj-$(CONFIG_ADV_SWBUTTON)	+= adv_swbutton.o
> diff --git a/drivers/platform/x86/amd_hsmp.c b/drivers/platform/x86/amd_hsmp.c
> new file mode 100644
> index 000000000000..29b22db726bf
> --- /dev/null
> +++ b/drivers/platform/x86/amd_hsmp.c
> @@ -0,0 +1,450 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD HSMP Platform Driver
> + * Copyright (c) 2022, AMD.
> + * All Rights Reserved.
> + *
> + * 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>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/semaphore.h>
> +
> +#define DRIVER_NAME		"amd_hsmp"
> +#define DRIVER_VERSION		"1.0"
> +
> +/* HSMP Status / Error codes */
> +#define HSMP_STATUS_NOT_READY	0x00
> +#define HSMP_STATUS_OK		0x01
> +#define HSMP_ERR_INVALID_MSG	0xFE
> +#define HSMP_ERR_INVALID_INPUT	0xFF
> +
> +/* Timeout in millsec */
> +#define HSMP_MSG_TIMEOUT	100
> +#define HSMP_SHORT_SLEEP	1
> +
> +#define HSMP_WR			true
> +#define HSMP_RD			false
> +
> +/*
> + * To access specific HSMP mailbox register, s/w writes the SMN address of HSMP mailbox
> + * register into the SMN_INDEX register, and reads/writes the SMN_DATA reg.
> + * Below are required SMN address for HSMP Mailbox register offsets in SMU address space
> + */
> +#define SMN_HSMP_MSG_ID		0x3B10534
> +#define SMN_HSMP_MSG_RESP	0x3B10980
> +#define SMN_HSMP_MSG_DATA	0x3B109E0
> +
> +#define HSMP_INDEX_REG		0xc4
> +#define HSMP_DATA_REG		0xc8
> +
> +static struct semaphore *hsmp_sem;
> +
> +static struct miscdevice hsmp_device;
> +
> +/* List of "Configure/SET" msgs */
> +static u32 hsmp_set_msgs[] = {
> +	HSMP_SET_SOCKET_POWER_LIMIT,
> +	HSMP_SET_BOOST_LIMIT,
> +	HSMP_SET_BOOST_LIMIT_SOCKET,
> +	HSMP_SET_XGMI_LINK_WIDTH,
> +	HSMP_SET_DF_PSTATE,
> +	HSMP_SET_AUTO_DF_PSTATE,
> +	HSMP_SET_NBIO_DPM_LEVEL
> +};
> +
> +/* List of "Monitor/GET" msgs */
> +static u32 hsmp_get_msgs[] = {
> +	HSMP_TEST,
> +	HSMP_GET_SMU_VER,
> +	HSMP_GET_PROTO_VER,
> +	HSMP_GET_SOCKET_POWER,
> +	HSMP_GET_SOCKET_POWER_LIMIT,
> +	HSMP_GET_SOCKET_POWER_LIMIT_MAX,
> +	HSMP_GET_BOOST_LIMIT,
> +	HSMP_GET_PROC_HOT,
> +	HSMP_GET_FCLK_MCLK,
> +	HSMP_GET_CCLK_THROTTLE_LIMIT,
> +	HSMP_GET_C0_PERCENT,
> +	HSMP_GET_DDR_BANDWIDTH,
> +	HSMP_GET_TEMP_MONITOR
> +};
> +
> +static int amd_hsmp_rdwr(struct pci_dev *root, u32 address,
> +			 u32 *value, bool write)
> +{
> +	int ret;
> +
> +	ret = pci_write_config_dword(root, HSMP_INDEX_REG, address);
> +	if (ret)
> +		return ret;
> +
> +	ret = (write ? pci_write_config_dword(root, HSMP_DATA_REG, *value)
> +		     : pci_read_config_dword(root, HSMP_DATA_REG, value));
> +
> +	return ret;
> +}
> +
> +/*
> + * Send a message to the HSMP port via PCI-e config space registers.
> + *
> + * The caller is expected to zero out any unused arguments.
> + * If a response is expected, the number of response words should be greater than 0.
> + *
> + * Returns 0 for success and populates the requested number of arguments.
> + * Returns a negative error code for failure.
> + */
> +static int __hsmp_send_message(struct pci_dev *root, struct hsmp_message *msg)
> +{
> +	unsigned long timeout, short_sleep;
> +	u32 mbox_status;
> +	u32 arg_num;
> +	int ret;
> +
> +	/* Clear the status register */
> +	mbox_status = HSMP_STATUS_NOT_READY;
> +	ret = amd_hsmp_rdwr(root, SMN_HSMP_MSG_RESP, &mbox_status, HSMP_WR);
> +	if (ret) {
> +		pr_err("Error %d clearing mailbox status register\n", ret);
> +		return ret;
> +	}
> +
> +	arg_num = 0;
> +	/* Write any message arguments */
> +	while (arg_num < msg->num_args) {
> +		ret = amd_hsmp_rdwr(root, SMN_HSMP_MSG_DATA + (arg_num << 2),
> +				    &msg->args[arg_num], HSMP_WR);
> +		if (ret) {
> +			pr_err("Error %d writing message argument %d\n", ret, arg_num);
> +			return ret;
> +		}
> +		arg_num++;
> +	}
> +
> +	/* Write the message ID which starts the operation */
> +	ret = amd_hsmp_rdwr(root, SMN_HSMP_MSG_ID, &msg->msg_id, HSMP_WR);
> +	if (ret) {
> +		pr_err("Error %d writing message ID %u\n", ret, msg->msg_id);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Depending on when the trigger write completes relative to the SMU
> +	 * firmware 1 ms cycle, the operation may take from tens of us to 1 ms
> +	 * to complete. Some operations may take more. Therefore we will try
> +	 * a few short duration sleeps and switch to long sleeps if we don't
> +	 * succeed quickly.
> +	 */
> +	short_sleep = jiffies + msecs_to_jiffies(HSMP_SHORT_SLEEP);
> +	timeout	= jiffies + msecs_to_jiffies(HSMP_MSG_TIMEOUT);
> +
> +	while (time_before(jiffies, timeout)) {
> +		ret = amd_hsmp_rdwr(root, SMN_HSMP_MSG_RESP, &mbox_status, HSMP_RD);
> +		if (ret) {
> +			pr_err("Error %d reading mailbox status\n", ret);
> +			return ret;
> +		}
> +
> +		if (mbox_status != HSMP_STATUS_NOT_READY)
> +			break;
> +		if (time_before(jiffies, short_sleep))
> +			usleep_range(50, 100);
> +		else
> +			usleep_range(1000, 2000);
> +	}
> +
> +	if (unlikely(mbox_status == HSMP_STATUS_NOT_READY)) {
> +		return -ETIMEDOUT;
> +	} else if (unlikely(mbox_status == HSMP_ERR_INVALID_MSG)) {
> +		return -ENOMSG;
> +	} else if (unlikely(mbox_status == HSMP_ERR_INVALID_INPUT)) {
> +		return -EINVAL;
> +	} else if (unlikely(mbox_status != HSMP_STATUS_OK)) {
> +		pr_err("Message ID %u unknown failure (status = 0x%X)\n",
> +		       msg->msg_id, mbox_status);
> +		return -EIO;
> +	}
> +
> +	/* SMU has responded OK. Read response data */
> +	arg_num = 0;
> +	while (arg_num < msg->response_sz) {
> +		ret = amd_hsmp_rdwr(root, SMN_HSMP_MSG_DATA + (arg_num << 2),
> +				    &msg->response[arg_num], HSMP_RD);
> +		if (ret) {
> +			pr_err("Error %d reading response %u for message ID:%u\n",
> +			       ret, arg_num, msg->msg_id);
> +			break;
> +		}
> +		arg_num++;
> +	}
> +
> +	return ret;
> +}
> +
> +int hsmp_send_message(struct hsmp_message *msg)
> +{
> +	struct amd_northbridge *nb;
> +	int ret;
> +
> +	if (!msg)
> +		return -EINVAL;
> +
> +	nb = node_to_amd_nb(msg->sock_ind);
> +	if (!nb || !nb->root)
> +		return -ENODEV;
> +
> +	if (msg->msg_id < HSMP_TEST || msg->msg_id >= HSMP_MSG_ID_MAX ||
> +	    msg->msg_id == HSMP_RESERVED)
> +		return -EINVAL;
> +
> +	if (msg->num_args > HSMP_MAX_MSG_LEN || msg->response_sz > HSMP_MAX_MSG_LEN)
> +		return -EINVAL;
> +
> +	/*
> +	 * The time taken by smu operation to complete is between
> +	 * 10us to 1ms. Sometime it may take more time.
> +	 * In SMP system timeout of 100 millisecs should
> +	 * be enough for the previous thread to finish the operation
> +	 */
> +	ret = down_timeout(&hsmp_sem[msg->sock_ind],
> +			   msecs_to_jiffies(HSMP_MSG_TIMEOUT));
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = __hsmp_send_message(nb->root, msg);
> +
> +	up(&hsmp_sem[msg->sock_ind]);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(hsmp_send_message);
> +
> +static int hsmp_test(u16 sock_ind)
> +{
> +	struct hsmp_message msg = { 0 };
> +	struct amd_northbridge *nb;
> +	int ret = -ENODEV;
> +
> +	nb = node_to_amd_nb(sock_ind);
> +	if (!nb || !nb->root)
> +		return ret;
> +
> +	/*
> +	 * Test the hsmp port by performing TEST command. The test message takes
> +	 * one argument and returns the value of that argument + 1.
> +	 */
> +	msg.sock_ind	= sock_ind;
> +	msg.response_sz = 1;
> +	msg.num_args	= 1;
> +	msg.msg_id	= HSMP_TEST;
> +	msg.args[0]	= 0xDEADBEEF;
> +
> +	ret = __hsmp_send_message(nb->root, &msg);
> +	if (ret)
> +		return ret;
> +
> +	if (msg.response[0] != (msg.args[0] + 1)) {
> +		pr_err("Socket %d test message failed, Expected 0x%08X, received 0x%08X\n",
> +		       sock_ind, msg.args[0] + 1, msg.response[0]);
> +		return -EBADE;
> +	}
> +
> +	return 0;
> +}
> +
> +static int search_msg_list(u32 msgid, u32 *list, int size)
> +{
> +	int i;
> +
> +	for (i = 0; i < size; i++) {
> +		if (msgid == list[i])
> +			return 0;
> +	}
> +	return -EINVAL;
> +}
> +
> +static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +{
> +	int __user *arguser = (int  __user *)arg;
> +	struct hsmp_message msg = { 0 };
> +	int ret;
> +
> +	if (copy_struct_from_user(&msg, sizeof(msg), arguser, sizeof(struct hsmp_message)))

I'm curious why you used two different sizeof() calls here, won't they both return the
same size.

> +		return -EFAULT;
> +
> +	switch (fp->f_mode & (FMODE_WRITE | FMODE_READ)) {
> +	case FMODE_WRITE:
> +		/* check if the msgid is a set msg */
> +		ret = search_msg_list(msg.msg_id, hsmp_set_msgs, ARRAY_SIZE(hsmp_set_msgs));
> +		if (ret)
> +			return ret;
> +
> +		return hsmp_send_message(&msg);
> +	case FMODE_READ:
> +		/* check if the msgid is a get msg */
> +		ret = search_msg_list(msg.msg_id, hsmp_get_msgs, ARRAY_SIZE(hsmp_get_msgs));
> +		if (ret)
> +			return ret;
> +		break;
> +	case FMODE_READ | FMODE_WRITE:
> +		ret = search_msg_list(msg.msg_id, hsmp_set_msgs, ARRAY_SIZE(hsmp_set_msgs));
> +		if (!ret)
> +			return hsmp_send_message(&msg);
> +
> +		ret = search_msg_list(msg.msg_id, hsmp_get_msgs, ARRAY_SIZE(hsmp_get_msgs));
> +		if (ret)
> +			return ret;
> +
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret =  hsmp_send_message(&msg);
              ^
You have an extra space here.

Checkpatch also reports a few places where you have trailing whitespace and your use
of DOS line endings in the file. Please make sure you run checkpatch and clean up any
issues.

> +	if (ret)
> +		return ret;
> +
> +	if (copy_to_user(arguser, &msg, sizeof(struct hsmp_message)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
> +static const struct file_operations hsmp_fops = {
> +	.owner		= THIS_MODULE,
> +	.unlocked_ioctl	= hsmp_ioctl,
> +	.compat_ioctl	= hsmp_ioctl,
> +};
> +
> +static int hsmp_pltdrv_probe(struct platform_device *pdev)
> +{
> +	int ret, i;
> +
> +	hsmp_sem = devm_kzalloc(&pdev->dev,
> +				(amd_nb_num() * sizeof(struct semaphore)),
> +				GFP_KERNEL);
> +	if (!hsmp_sem)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < amd_nb_num(); i++)
> +		sema_init(&hsmp_sem[i], 1);
> +
> +	hsmp_device.name	= "hsmp_cdev";
> +	hsmp_device.minor	= MISC_DYNAMIC_MINOR;
> +	hsmp_device.fops	= &hsmp_fops;
> +	hsmp_device.parent	= &pdev->dev;
> +	hsmp_device.nodename	= "hsmp";
> +	hsmp_device.mode	= 0644;
> +
> +	ret = misc_register(&hsmp_device);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int hsmp_pltdrv_remove(struct platform_device *pdev)
> +{
> +	misc_deregister(&hsmp_device);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver amd_hsmp_driver = {
> +	.probe		= hsmp_pltdrv_probe,
> +	.remove		= hsmp_pltdrv_remove,
> +	.driver		= {
> +		.name	= DRIVER_NAME,
> +	},
> +};
> +
> +static bool check_model_support(void)
> +{
> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
> +	    boot_cpu_data.x86 != 0x19)

Check for boot_cpu_data.x86 < 0x19, lets avoid having to do driver updates any
time future families support HSMP.

> +		return false;
> +
> +	switch (boot_cpu_data.x86_model) {
> +	case 0x00 ... 0x1f:
> +	case 0x30 ... 0x3f:
> +		return true;
> +	default:
> +		return false;
> +	}

I don't think the model check here is necessary. Adding a model check will only
add driver updates for any new models.

HSMP is only supported on f19h so we do want to keep that. For the different
models that may not support HSMP, the call to hsmp_test() in the init routine
will fail if HSMP isn't supported.

> +
> +	return false;
> +}
> +
> +static struct platform_device *amd_hsmp_platdev;
> +
> +static int __init hsmp_plt_init(void)
> +{
> +	int ret = -ENODEV;
> +	u16 num_sockets;
> +	int i;
> +
> +	if (!check_model_support()) {
> +		pr_err("HSMP is not supported on Family:%x model:%x\n",
> +		       boot_cpu_data.x86, boot_cpu_data.x86_model);
> +		return ret;

Should return -EOPNOTSUPP here instead of -ENODEV.

-Nathan

> +	}
> +
> +	/*
> +	 * amd_nb_num() returns number of SMN/DF interfaces present in the system
> +	 * if we have N SMN/DF interfaces that ideally means N sockets
> +	 */
> +	num_sockets = amd_nb_num();
> +	if (num_sockets == 0)
> +		return ret;
> +
> +	/* Test the hsmp interface on each socket */
> +	for (i = 0; i < num_sockets; i++) {
> +		ret = hsmp_test(i);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = platform_driver_register(&amd_hsmp_driver);
> +	if (ret)
> +		return ret;
> +
> +	amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, -1);
> +	if (!amd_hsmp_platdev) {
> +		ret = -ENOMEM;
> +		goto drv_unregister;
> +	}
> +
> +	ret = platform_device_add(amd_hsmp_platdev);
> +	if (ret) {
> +		platform_device_put(amd_hsmp_platdev);
> +		goto drv_unregister;
> +	}
> +
> +	return 0;
> +
> +drv_unregister:
> +	platform_driver_unregister(&amd_hsmp_driver);
> +	return ret;
> +}
> +
> +static void __exit hsmp_plt_exit(void)
> +{
> +	platform_device_unregister(amd_hsmp_platdev);
> +	platform_driver_unregister(&amd_hsmp_driver);
> +}
> +
> +device_initcall(hsmp_plt_init);
> +module_exit(hsmp_plt_exit);
> +
> +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver");
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] Documentation: Add x86/amd_hsmp driver
  2022-02-03 12:04 ` [PATCH v2 2/2] Documentation: Add x86/amd_hsmp driver Naveen Krishna Chatradhi
@ 2022-02-03 15:52   ` Nathan Fontenot
  0 siblings, 0 replies; 8+ messages in thread
From: Nathan Fontenot @ 2022-02-03 15:52 UTC (permalink / raw)
  To: Naveen Krishna Chatradhi, platform-driver-x86
  Cc: hdegoede, Siva.Sathappan, carlos.bilbao, Nathan.Fontenot

On 2/3/22 06:04, Naveen Krishna Chatradhi wrote:
> This documentation for amd_hsmp driver explains how to use the
> device interface.
> 
> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
> ---
> Changes since v1:
> None
> 
>  Documentation/x86/amd_hsmp.rst | 93 ++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/x86/amd_hsmp.rst
> 
> diff --git a/Documentation/x86/amd_hsmp.rst b/Documentation/x86/amd_hsmp.rst
> new file mode 100644
> index 000000000000..b77b888f810a
> --- /dev/null
> +++ b/Documentation/x86/amd_hsmp.rst
> @@ -0,0 +1,93 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +============================================
> +AMD HSMP interface
> +============================================
> +
> +Newer Fam19h EPYC server line of processors from AMD support system
> +management functionality via HSMP (Host System Management Port).
> +
> +The Host System Management Port (HSMP) is an interface to provide
> +OS-level software with access to system management functions via a
> +set of mailbox registers.
> +
> +More details on the interface can be found in chapter
> +"7 Host System Management Port (HSMP)" of the following PPR
> +https://www.amd.com/system/files/TechDocs/55898_B1_pub_0.50.zip
> +
> +
> +HSMP device
> +============================================
> +
> +amd_hsmp driver under the drivers/platforms/x86/ creates miscdevice
> +/dev/hsmp to let user space programs run hsmp mailbox commands.
> +
> +$ ls -al /dev/hsmp
> +crw-r--r-- 1 root root 10, 123 Jan 21 21:41 /dev/hsmp
> +
> +Characteristics of the dev node:
> + * Write mode is used for running set/configure commands
> + * Read mode is used for running get/status monitor commands
> +
> +Access restrictions:
> + * Only root user is allowed to open the file in write mode.
> + * The file can be opened in read mode by all the users.
> +
> +In-kernel integration:
> + * Other subsystems in the kernel can use the exported transport
> +   function hsmp_send_message().
> + * Locking across callers is taken care by the driver.
> +
> +Features support by the interface include monitor and/or control of
> +a. boostlimit
> +b. current power, power limit, max power limit
> +c. c0 residency
> +d. prochot status
> +e. clocks (fclk, mclk and cclk)
> +f. ddr bandwidth, utilization
> +g. data fabric P-state
> +

You may want to avoid listing out HSMP interfaces. The exact interfaces supported
are family/model specific. Instead, refer users to the PPR specific for their
family/model to get the list of supported interfaces.

-Nathan

> +
> +An example
> +==========
> +
> +To access hsmp device from a C program.
> +First, you need to include the headers::
> +
> +  #include <linux/amd_hsmp.h>
> +Which defines the supported messages/message IDs.
> +
> +Next thing, open the device file, as follows::
> +
> +  int file;
> +
> +  file = open("/dev/hsmp", O_RDWR);
> +  if (file < 0) {
> +    /* ERROR HANDLING; you can check errno to see what went wrong */
> +    exit(1);
> +  }
> +
> +The following IOCTL is defined:
> +
> +``ioctl(file, HSMP_IOCTL_CMD, struct hsmp_message *msg)``
> +  The argument is a pointer to a::
> +
> +    struct hsmp_message {
> +	__u32	msg_id;				/* Message ID */
> +	__u16	num_args;			/* Number of arguments in message */
> +	__u16	response_sz;			/* Number of expected response words */
> +	__u32	args[HSMP_MAX_MSG_LEN];		/* Argument(s) */
> +	__u32	response[HSMP_MAX_MSG_LEN];	/* Response word(s) */
> +	__u16	sock_ind;			/* socket number */
> +    };
> +
> +The ioctl would return a non-zero on failure; you can read errno to see
> +what happened. The transaction returns 0 on success.
> +
> +More details on the interface can be found in chapter
> +"7 Host System Management Port (HSMP)" of the following PPR
> +https://www.amd.com/system/files/TechDocs/55898_B1_pub_0.50.zip
> +
> +User space C-APIs are made available by linking against the esmi library,
> +which is provided by the E-SMS project https://developer.amd.com/e-sms/.
> +See: https://github.com/amd/esmi_ib_library

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] platforms/x86: Add AMD system management interface
  2022-02-03 15:48 ` [PATCH v2 1/2] platforms/x86: Add AMD system management interface Nathan Fontenot
@ 2022-02-03 16:03   ` Carlos Bilbao
  2022-02-03 16:17     ` Nathan Fontenot
  0 siblings, 1 reply; 8+ messages in thread
From: Carlos Bilbao @ 2022-02-03 16:03 UTC (permalink / raw)
  To: Nathan Fontenot, Naveen Krishna Chatradhi, platform-driver-x86
  Cc: hdegoede, Siva.Sathappan, Nathan.Fontenot, Suma Hegde

Hello,

On 2/3/2022 9:48 AM, Nathan Fontenot wrote:
> On 2/3/22 06:04, Naveen Krishna Chatradhi wrote:
>> From: Suma Hegde <suma.hegde@amd.com>
>>
>> (...)
>> +static bool check_model_support(void)
>> +{
>> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
>> +	    boot_cpu_data.x86 != 0x19)
> 
> Check for boot_cpu_data.x86 < 0x19, lets avoid having to do driver updates any
> time future families support HSMP.
> 
>> +		return false;
>> +
>> +	switch (boot_cpu_data.x86_model) {
>> +	case 0x00 ... 0x1f:
>> +	case 0x30 ... 0x3f:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
> 
> I don't think the model check here is necessary. Adding a model check will only
> add driver updates for any new models.
> 
> HSMP is only supported on f19h so we do want to keep that. For the different
> models that may not support HSMP, the call to hsmp_test() in the init routine
> will fail if HSMP isn't supported.

So, for this I think Naveen was trying to avoid people with Fam19th client processors 
complaining about errors in HSMP and filing bugs. You are equally right pointing out 
that updating this in the future would be inconvenient. Personally, if I put myself in 
the shoes of an owner of one of those systems, I would be a bit disappointed to run HSMP
and just fail. So, maybe a solution in the middle would be to remove the model-checking code 
but also include a warning/clarification in the documentation. 

Thanks,
Carlos.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] platforms/x86: Add AMD system management interface
  2022-02-03 16:03   ` Carlos Bilbao
@ 2022-02-03 16:17     ` Nathan Fontenot
  0 siblings, 0 replies; 8+ messages in thread
From: Nathan Fontenot @ 2022-02-03 16:17 UTC (permalink / raw)
  To: Carlos Bilbao, Naveen Krishna Chatradhi, platform-driver-x86
  Cc: hdegoede, Siva.Sathappan, Nathan.Fontenot, Suma Hegde

On 2/3/22 10:03, Carlos Bilbao wrote:
> Hello,
> 
> On 2/3/2022 9:48 AM, Nathan Fontenot wrote:
>> On 2/3/22 06:04, Naveen Krishna Chatradhi wrote:
>>> From: Suma Hegde <suma.hegde@amd.com>
>>>
>>> (...)
>>> +static bool check_model_support(void)
>>> +{
>>> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
>>> +	    boot_cpu_data.x86 != 0x19)
>>
>> Check for boot_cpu_data.x86 < 0x19, lets avoid having to do driver updates any
>> time future families support HSMP.
>>
>>> +		return false;
>>> +
>>> +	switch (boot_cpu_data.x86_model) {
>>> +	case 0x00 ... 0x1f:
>>> +	case 0x30 ... 0x3f:
>>> +		return true;
>>> +	default:
>>> +		return false;
>>> +	}
>>
>> I don't think the model check here is necessary. Adding a model check will only
>> add driver updates for any new models.
>>
>> HSMP is only supported on f19h so we do want to keep that. For the different
>> models that may not support HSMP, the call to hsmp_test() in the init routine
>> will fail if HSMP isn't supported.
> 
> So, for this I think Naveen was trying to avoid people with Fam19th client processors 
> complaining about errors in HSMP and filing bugs. You are equally right pointing out 
> that updating this in the future would be inconvenient. Personally, if I put myself in 
> the shoes of an owner of one of those systems, I would be a bit disappointed to run HSMP
> and just fail. So, maybe a solution in the middle would be to remove the model-checking code 
> but also include a warning/clarification in the documentation. 

I agree that some kind of updated message would be good.

The family check is already made and an appropriate message that HSMP is not supported
is made.

Removing the model check and using hsmp_test() needs to account for hsmp_test failing
in two ways. The call can fail because we are on a model that does not support
HSMP, or it can fail because HSMP has been disabled in the BIOS.

The hsmp_test() call should output a message along the lines of "HSMP is not supported
on this family/model or has been disabled in BIOS."

-Nathan

> 
> Thanks,
> Carlos.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] platforms/x86: Add AMD system management interface
  2022-02-03 17:52 ` Song Liu
@ 2022-02-03 17:58   ` Chatradhi, Naveen Krishna
  0 siblings, 0 replies; 8+ messages in thread
From: Chatradhi, Naveen Krishna @ 2022-02-03 17:58 UTC (permalink / raw)
  To: Song Liu
  Cc: Nathan.Fontenot, Siva.Sathappan, carlos.bilbao, hdegoede,
	platform-driver-x86, suma.hegde

Hi Song,

On 2/3/2022 11:22 PM, Song Liu wrote:
> [CAUTION: External Email]
>
> On Thu, Feb 03, 2022 at 05:34:49PM +0530, Naveen Krishna Chatradhi wrote:
>> From: Suma Hegde <suma.hegde@amd.com>
>>
> [...]
>>   .../userspace-api/ioctl/ioctl-number.rst      |   2 +
>>   arch/x86/include/asm/amd_hsmp.h               |  16 +
>>   arch/x86/include/uapi/asm/amd_hsmp.h          |  56 +++
>>   drivers/platform/x86/Kconfig                  |  13 +
>>   drivers/platform/x86/Makefile                 |   1 +
>>   drivers/platform/x86/amd_hsmp.c               | 450 ++++++++++++++++++
> We added new files. Shall we update MAINTAINERS?
Sure, i will update the MAINTAINERS
>
>>   6 files changed, 538 insertions(+)
>>   create mode 100644 arch/x86/include/asm/amd_hsmp.h
>>   create mode 100644 arch/x86/include/uapi/asm/amd_hsmp.h
>>   create mode 100644 drivers/platform/x86/amd_hsmp.c
>>
>> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
>> index 687efcf245c1..663e316d320c 100644
>> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
>> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
>> @@ -372,6 +372,8 @@ Code  Seq#    Include File                                           Comments
> [...]
>
>> diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
>> new file mode 100644
>> index 000000000000..42cdac8a331d
>> --- /dev/null
>> +++ b/arch/x86/include/uapi/asm/amd_hsmp.h
>> @@ -0,0 +1,56 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +
>> +#ifndef _UAPI_ASM_X86_AMD_HSMP_H_
>> +#define _UAPI_ASM_X86_AMD_HSMP_H_
>> +
>> +#include <linux/types.h>
>> +
>> +#pragma pack(4)
>> +
>> +#define HSMP_MAX_MSG_LEN 8
>> +
>> +/*
>> + * HSMP Messages supported
>> + */
>> +enum hsmp_message_ids {
>> +     HSMP_TEST = 1,                  /* 01h Increments input value by 1 */
>> +     HSMP_GET_SMU_VER,               /* 02h SMU FW version */
>> +     HSMP_GET_PROTO_VER,             /* 03h HSMP interface version */
>> +     HSMP_GET_SOCKET_POWER,          /* 04h average package power consumption */
>> +     HSMP_SET_SOCKET_POWER_LIMIT,    /* 05h Set the socket power limit */
>> +     HSMP_GET_SOCKET_POWER_LIMIT,    /* 06h Get current socket power limit */
>> +     HSMP_GET_SOCKET_POWER_LIMIT_MAX,/* 07h Get maximum socket power value */
>> +     HSMP_SET_BOOST_LIMIT,           /* 08h Set a core maximum frequency limit */
>> +     HSMP_SET_BOOST_LIMIT_SOCKET,    /* 09h Set socket maximum frequency level */
>> +     HSMP_GET_BOOST_LIMIT,           /* 0Ah Get current frequency limit */
>> +     HSMP_GET_PROC_HOT,              /* 0Bh Get PROCHOT status */
>> +     HSMP_SET_XGMI_LINK_WIDTH,       /* 0Ch Set max and min width of xGMI Link */
>> +     HSMP_SET_DF_PSTATE,             /* 0Dh Alter APEnable/Disable messages behavior */
>> +     HSMP_SET_AUTO_DF_PSTATE,        /* 0Eh Enable DF P-State Performance Boost algorithm */
>> +     HSMP_GET_FCLK_MCLK,             /* 0Fh Get FCLK and MEMCLK for current socket */
>> +     HSMP_GET_CCLK_THROTTLE_LIMIT,   /* 10h Get CCLK frequency limit in socket */
>> +     HSMP_GET_C0_PERCENT,            /* 11h Get average C0 residency in socket */
>> +     HSMP_SET_NBIO_DPM_LEVEL,        /* 12h Set max/min LCLK DPM Level for a given NBIO */
>> +     HSMP_RESERVED,                  /* 13h Reserved */
>> +     HSMP_GET_DDR_BANDWIDTH,         /* 14h Get theoretical maximum and current DDR Bandwidth */
>> +     HSMP_GET_TEMP_MONITOR,          /* 15h Get per-DIMM temperature and refresh rates */
>> +     HSMP_MSG_ID_MAX,
>> +};
>> +
>> +struct hsmp_message {
>> +     __u32   msg_id;                         /* Message ID */
>> +     __u16   num_args;                       /* Number of arguments in message */
>> +     __u16   response_sz;                    /* Number of expected response words */
>> +     __u32   args[HSMP_MAX_MSG_LEN];         /* Argument(s) */
>> +     __u32   response[HSMP_MAX_MSG_LEN];     /* Response word(s) */
>> +     __u16   sock_ind;                       /* socket number */
>> +};
> IIUC, we rely on user space to know (from other sources) the proper num_args
> and response_sz for each message_id. The only check applied by the kernel is
> "num_args <= HSMP_MAX_MSG_LEN && response_sz <= HSMP_MAX_MSG_LEN". How about
> we explicitly call out those constraints in amd_hsmp.h? Maybe something like:
>
> struct hsmp_msg_format {
>         int msg_id;
>         int num_args;
>         int response_sz;
> };
>
> struct hsmp_msg_format hsmp_msg_format_table = {
>         { HSMP_TEST, 1, 1},
>         /* more */
> };
Good input, will do.
>
> Thanks,
> Song

Thanks,

Naveenk

>
> [...]
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] platforms/x86: Add AMD system management interface
       [not found] <<20220203120450.199598-1-nchatrad@amd.com>
@ 2022-02-03 17:52 ` Song Liu
  2022-02-03 17:58   ` Chatradhi, Naveen Krishna
  0 siblings, 1 reply; 8+ messages in thread
From: Song Liu @ 2022-02-03 17:52 UTC (permalink / raw)
  To: nchatrad
  Cc: Nathan.Fontenot, Siva.Sathappan, carlos.bilbao, hdegoede,
	platform-driver-x86, suma.hegde

On Thu, Feb 03, 2022 at 05:34:49PM +0530, Naveen Krishna Chatradhi wrote:
> From: Suma Hegde <suma.hegde@amd.com>
>
[...]
>
>  .../userspace-api/ioctl/ioctl-number.rst      |   2 +
>  arch/x86/include/asm/amd_hsmp.h               |  16 +
>  arch/x86/include/uapi/asm/amd_hsmp.h          |  56 +++
>  drivers/platform/x86/Kconfig                  |  13 +
>  drivers/platform/x86/Makefile                 |   1 +
>  drivers/platform/x86/amd_hsmp.c               | 450 ++++++++++++++++++

We added new files. Shall we update MAINTAINERS?

>  6 files changed, 538 insertions(+)
>  create mode 100644 arch/x86/include/asm/amd_hsmp.h
>  create mode 100644 arch/x86/include/uapi/asm/amd_hsmp.h
>  create mode 100644 drivers/platform/x86/amd_hsmp.c
>
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 687efcf245c1..663e316d320c 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -372,6 +372,8 @@ Code  Seq#    Include File                                           Comments
[...]

> diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
> new file mode 100644
> index 000000000000..42cdac8a331d
> --- /dev/null
> +++ b/arch/x86/include/uapi/asm/amd_hsmp.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +
> +#ifndef _UAPI_ASM_X86_AMD_HSMP_H_
> +#define _UAPI_ASM_X86_AMD_HSMP_H_
> +
> +#include <linux/types.h>
> +
> +#pragma pack(4)
> +
> +#define HSMP_MAX_MSG_LEN 8
> +
> +/*
> + * HSMP Messages supported
> + */
> +enum hsmp_message_ids {
> +     HSMP_TEST = 1,                  /* 01h Increments input value by 1 */
> +     HSMP_GET_SMU_VER,               /* 02h SMU FW version */
> +     HSMP_GET_PROTO_VER,             /* 03h HSMP interface version */
> +     HSMP_GET_SOCKET_POWER,          /* 04h average package power consumption */
> +     HSMP_SET_SOCKET_POWER_LIMIT,    /* 05h Set the socket power limit */
> +     HSMP_GET_SOCKET_POWER_LIMIT,    /* 06h Get current socket power limit */
> +     HSMP_GET_SOCKET_POWER_LIMIT_MAX,/* 07h Get maximum socket power value */
> +     HSMP_SET_BOOST_LIMIT,           /* 08h Set a core maximum frequency limit */
> +     HSMP_SET_BOOST_LIMIT_SOCKET,    /* 09h Set socket maximum frequency level */
> +     HSMP_GET_BOOST_LIMIT,           /* 0Ah Get current frequency limit */
> +     HSMP_GET_PROC_HOT,              /* 0Bh Get PROCHOT status */
> +     HSMP_SET_XGMI_LINK_WIDTH,       /* 0Ch Set max and min width of xGMI Link */
> +     HSMP_SET_DF_PSTATE,             /* 0Dh Alter APEnable/Disable messages behavior */
> +     HSMP_SET_AUTO_DF_PSTATE,        /* 0Eh Enable DF P-State Performance Boost algorithm */
> +     HSMP_GET_FCLK_MCLK,             /* 0Fh Get FCLK and MEMCLK for current socket */
> +     HSMP_GET_CCLK_THROTTLE_LIMIT,   /* 10h Get CCLK frequency limit in socket */
> +     HSMP_GET_C0_PERCENT,            /* 11h Get average C0 residency in socket */
> +     HSMP_SET_NBIO_DPM_LEVEL,        /* 12h Set max/min LCLK DPM Level for a given NBIO */
> +     HSMP_RESERVED,                  /* 13h Reserved */
> +     HSMP_GET_DDR_BANDWIDTH,         /* 14h Get theoretical maximum and current DDR Bandwidth */
> +     HSMP_GET_TEMP_MONITOR,          /* 15h Get per-DIMM temperature and refresh rates */
> +     HSMP_MSG_ID_MAX,
> +};
> +
> +struct hsmp_message {
> +     __u32   msg_id;                         /* Message ID */
> +     __u16   num_args;                       /* Number of arguments in message */
> +     __u16   response_sz;                    /* Number of expected response words */
> +     __u32   args[HSMP_MAX_MSG_LEN];         /* Argument(s) */
> +     __u32   response[HSMP_MAX_MSG_LEN];     /* Response word(s) */
> +     __u16   sock_ind;                       /* socket number */
> +};

IIUC, we rely on user space to know (from other sources) the proper num_args
and response_sz for each message_id. The only check applied by the kernel is
"num_args <= HSMP_MAX_MSG_LEN && response_sz <= HSMP_MAX_MSG_LEN". How about
we explicitly call out those constraints in amd_hsmp.h? Maybe something like:

struct hsmp_msg_format {
       int msg_id;
       int num_args;
       int response_sz;
};

struct hsmp_msg_format hsmp_msg_format_table = {
       { HSMP_TEST, 1, 1},
       /* more */
};

Thanks,
Song

[...]



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-02-03 17:58 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-03 12:04 [PATCH v2 1/2] platforms/x86: Add AMD system management interface Naveen Krishna Chatradhi
2022-02-03 12:04 ` [PATCH v2 2/2] Documentation: Add x86/amd_hsmp driver Naveen Krishna Chatradhi
2022-02-03 15:52   ` Nathan Fontenot
2022-02-03 15:48 ` [PATCH v2 1/2] platforms/x86: Add AMD system management interface Nathan Fontenot
2022-02-03 16:03   ` Carlos Bilbao
2022-02-03 16:17     ` Nathan Fontenot
     [not found] <<20220203120450.199598-1-nchatrad@amd.com>
2022-02-03 17:52 ` Song Liu
2022-02-03 17:58   ` Chatradhi, Naveen Krishna

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.