diff mbox series

[v6,06/11] misc: amd-sbi: Add support for AMD_SBI IOCTL

Message ID 20250324145815.1026314-7-akshay.gupta@amd.com (mailing list archive)
State Handled Elsewhere
Headers show
Series misc: Move AMD side band interface(SBI) functionality | expand

Commit Message

Gupta, Akshay March 24, 2025, 2:58 p.m. UTC
The present sbrmi module only support reporting power via hwmon.
However, AMD data center range of processors support various
system management functionality using custom protocols defined in
Advanced Platform Management Link (APML) specification.

Register a miscdevice, which creates a device /dev/sbrmiX with an IOCTL
interface for the user space to invoke the APML Mailbox protocol, which
is already defined in sbrmi_mailbox_xfer().

The APML protocols depend on a set of RMI registers. Having an IOCTL
as a single entry point will help in providing synchronization among
these protocols as multiple transactions on RMI register set may
create race condition.
Support for other protocols will be added in subsequent patches.

Open-sourced and widely used https://github.com/amd/esmi_oob_library
will continue to provide user-space programmable API.

Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
---
Changes since v5:
- Address review comment
 - Address Arnd comments
 - Add check for cmd in ioctl
 - Remove attribute packed from uapi header file structure apml_message{}
- Address kernel test robot warn:
 - warn: can 'data' even be NULL?

Changes since v4:
- Address review comment
 - Address Greg review comments
 - Not initialize ret
 - return on error
- Previously patch 4
- Fix documentation warning

Changes since v3:
- Previously patch 3
- Documentation and comments changes

Changes since v2:
- update the MACROS name as per feedback

Changes since v1:
- Previously patch 5
- Add IOCTL description in ioctl-number.rst
- Split patch as per suggestion.

 Documentation/misc-devices/index.rst          |  1 +
 .../userspace-api/ioctl/ioctl-number.rst      |  2 +
 drivers/misc/amd-sbi/rmi-core.c               | 85 +++++++++++++++++--
 drivers/misc/amd-sbi/rmi-core.h               | 15 ++--
 drivers/misc/amd-sbi/rmi-hwmon.c              | 15 ++--
 drivers/misc/amd-sbi/rmi-i2c.c                | 26 +++++-
 include/uapi/misc/amd-apml.h                  | 64 ++++++++++++++
 7 files changed, 181 insertions(+), 27 deletions(-)
 create mode 100644 include/uapi/misc/amd-apml.h

Comments

Arnd Bergmann March 24, 2025, 3:40 p.m. UTC | #1
On Mon, Mar 24, 2025, at 15:58, Akshay Gupta wrote:
> ---
> Changes since v5:
> - Address review comment
>  - Address Arnd comments
>  - Add check for cmd in ioctl

I think you missed one of my comments.

> +static long sbrmi_ioctl(struct file *fp, unsigned int cmd, unsigned 
> long arg)
> +{
> +	struct apml_message msg = { 0 };
> +	struct sbrmi_data *data;
> +	bool read = false;
> +	int ret;
> +
> +	if (cmd != SBRMI_IOCTL_CMD)
> +		return -ENOTTY;
> +
> +	if (_IOC_SIZE(cmd) != sizeof(msg))
> +		return -EINVAL;

You are checking the 'cmd' argument to the function now, which
is good. There is no need to also check _IOC_SIZE, since
that is implied by the definition.
rue;

> +
> +	switch (msg.cmd) {
> +	case 0 ... 0x999:
> +		/* Mailbox protocol */
> +		ret = rmi_mailbox_xfer(data, &msg);
> +		break;

What is however missing here is a specific check for the
individual commands: I don't think it's a good idea to
treat all 2458 mailbox commands the same way and just
pass them through unfiltered here, and I would also not
pass the specific 'cmd' as part of a multiplexer structure.

Instead, I think there should be a separate ioctl command
for each thing you can do with the mailbox. From the existing
driver it appears that these are the commands currently
supported:

enum sbrmi_msg_id {
        SBRMI_READ_PKG_PWR_CONSUMPTION = 0x1,
        SBRMI_WRITE_PKG_PWR_LIMIT,
        SBRMI_READ_PKG_PWR_LIMIT,
        SBRMI_READ_PKG_MAX_PWR_LIMIT,
};

which is just the first four out of the 2458, and clearly small
enough to justify one ioctl command each. I don't know what
the command specific arguments would look like, so it's
possible you may also want to define a separate structure
for each one.

       Arnd
Gupta, Akshay March 25, 2025, 12:35 p.m. UTC | #2
On 3/24/2025 9:10 PM, Arnd Bergmann wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Mon, Mar 24, 2025, at 15:58, Akshay Gupta wrote:
>> ---
>> Changes since v5:
>> - Address review comment
>>   - Address Arnd comments
>>   - Add check for cmd in ioctl
> I think you missed one of my comments.
>
>> +static long sbrmi_ioctl(struct file *fp, unsigned int cmd, unsigned
>> long arg)
>> +{
>> +     struct apml_message msg = { 0 };
>> +     struct sbrmi_data *data;
>> +     bool read = false;
>> +     int ret;
>> +
>> +     if (cmd != SBRMI_IOCTL_CMD)
>> +             return -ENOTTY;
>> +
>> +     if (_IOC_SIZE(cmd) != sizeof(msg))
>> +             return -EINVAL;
> You are checking the 'cmd' argument to the function now, which
> is good. There is no need to also check _IOC_SIZE, since
> that is implied by the definition.
> rue;
Ack.
>
>> +
>> +     switch (msg.cmd) {
>> +     case 0 ... 0x999:
>> +             /* Mailbox protocol */
>> +             ret = rmi_mailbox_xfer(data, &msg);
>> +             break;
> What is however missing here is a specific check for the
> individual commands: I don't think it's a good idea to
> treat all 2458 mailbox commands the same way and just
> pass them through unfiltered here, and I would also not
> pass the specific 'cmd' as part of a multiplexer structure.
>
> Instead, I think there should be a separate ioctl command
> for each thing you can do with the mailbox. From the existing
> driver it appears that these are the commands currently
> supported:
>
> enum sbrmi_msg_id {
>          SBRMI_READ_PKG_PWR_CONSUMPTION = 0x1,
>          SBRMI_WRITE_PKG_PWR_LIMIT,
>          SBRMI_READ_PKG_PWR_LIMIT,
>          SBRMI_READ_PKG_MAX_PWR_LIMIT,
> };
>
> which is just the first four out of the 2458, and clearly small
> enough to justify one ioctl command each. I don't know what
> the command specific arguments would look like, so it's
> possible you may also want to define a separate structure
> for each one.
>
>         Arnd

Link for the documentation: 
https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/57883.zip 
<https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/57883.zip>
You may refer to section: 5.4.2.2 SB-RMI Mailbox Service
At present, more than 70 mailbox commands ids are supported.
The number is increasing with each platforms.
however, the input and output for all mailbox commands are maintained as 
32 bit,
hence, we can use the same structure and differentiate the functionality.
As per current AMD's implementation the maximum can go up to 0xFF, I 
will update the
case to 0 ... 0XFF
Arnd Bergmann March 25, 2025, 1:37 p.m. UTC | #3
On Tue, Mar 25, 2025, at 13:35, Gupta, Akshay wrote:
> On 3/24/2025 9:10 PM, Arnd Bergmann wrote:
>
> Link for the documentation: 
> https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/57883.zip 
> <https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/57883.zip>
> You may refer to section: 5.4.2.2 SB-RMI Mailbox Service
> At present, more than 70 mailbox commands ids are supported.
> The number is increasing with each platforms.
> however, the input and output for all mailbox commands are maintained 
> as 32 bit, hence, we can use the same structure and differentiate the 
> functionality.

The large number of mailbox commands is exactly what I'm worried
about. Having 70 well-documented ioctl command numbers is not a problem,
since the numbers are cheap, but having an unfiltered pass-through
inteface between userspace and a PMIC is scary, and it's worse if
future hardware version has additional registers here.

A lot of the values reported through this interface could simply
be sysfs files, some look like they should integrate into existing
kernel subsystems (scheduler, pci, ...) and not be directly
visible to userspace.

      Arnd
Gupta, Akshay March 26, 2025, 11:46 a.m. UTC | #4
On 3/25/2025 7:07 PM, Arnd Bergmann wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Tue, Mar 25, 2025, at 13:35, Gupta, Akshay wrote:
>> On 3/24/2025 9:10 PM, Arnd Bergmann wrote:
>>
>> Link for the documentation:
>> https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/57883.zip
>> <https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/57883.zip>
>> You may refer to section: 5.4.2.2 SB-RMI Mailbox Service
>> At present, more than 70 mailbox commands ids are supported.
>> The number is increasing with each platforms.
>> however, the input and output for all mailbox commands are maintained
>> as 32 bit, hence, we can use the same structure and differentiate the
>> functionality.
> The large number of mailbox commands is exactly what I'm worried
> about. Having 70 well-documented ioctl command numbers is not a problem,
> since the numbers are cheap, but having an unfiltered pass-through
> inteface between userspace and a PMIC is scary, and it's worse if
> future hardware version has additional registers here.
This side band remote management interface(SBRMI) driver runs on the
Baseboard Management controller(BMC) connected to the processor(not on 
the processor itself)
to provides system monitoring and  control capabilities to the admin 
using out-of-band interface.

User space application on BMC calling the IOCTL would invoke a mailbox 
communication between
the BMC and System Management Unit(SMUFW) to addresses the individual 
command.
SMU would return error code: "Unsupported Command" as per platform support.

+--------------------+
|Processor           |
|Socket              |
|                    |
|SMU RMI             |
+--------------------+
             A
             |
         I2C/I3C
             |
             V
+--------------------+
|Driver <IOCTL> APP  |
|                    |
|                    |
|BMC                 |
+--------------------+
>
> A lot of the values reported through this interface could simply
> be sysfs files, some look like they should integrate into existing
> kernel subsystems (scheduler, pci, ...) and not be directly
> visible to userspace.
>
>        Arnd
We have provided integration to the hwmon subsystem for power sensor.
diff mbox series

Patch

diff --git a/Documentation/misc-devices/index.rst b/Documentation/misc-devices/index.rst
index 8c5b226d8313..081e79415e38 100644
--- a/Documentation/misc-devices/index.rst
+++ b/Documentation/misc-devices/index.rst
@@ -12,6 +12,7 @@  fit into other categories.
    :maxdepth: 2
 
    ad525x_dpot
+   amd-sbi
    apds990x
    bh1770glc
    c2port
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 6d1465315df3..5692b50b3c6f 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -392,6 +392,8 @@  Code  Seq#    Include File                                           Comments
                                                                      <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>
+0xF9  00-0F  uapi/misc/amd-apml.h		                     AMD side band system management interface driver
+                                                                     <mailto:naveenkrishna.chatradhi@amd.com>
 0xFD  all    linux/dm-ioctl.h
 0xFE  all    linux/isst_if.h
 ====  =====  ======================================================= ================================================================
diff --git a/drivers/misc/amd-sbi/rmi-core.c b/drivers/misc/amd-sbi/rmi-core.c
index 1d5e2556ab88..eafdd2799034 100644
--- a/drivers/misc/amd-sbi/rmi-core.c
+++ b/drivers/misc/amd-sbi/rmi-core.c
@@ -7,7 +7,10 @@ 
  */
 #include <linux/delay.h>
 #include <linux/err.h>
+#include <linux/fs.h>
 #include <linux/i2c.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/regmap.h>
 #include "rmi-core.h"
@@ -20,7 +23,7 @@ 
 #define TRIGGER_MAILBOX	0x01
 
 int rmi_mailbox_xfer(struct sbrmi_data *data,
-		     struct sbrmi_mailbox_msg *msg)
+		     struct apml_message *msg)
 {
 	unsigned int bytes;
 	int i, ret;
@@ -44,8 +47,8 @@  int rmi_mailbox_xfer(struct sbrmi_data *data,
 	 * Command Data In[31:0] to SBRMI::InBndMsg_inst[4:1]
 	 * SBRMI_x3C(MSB):SBRMI_x39(LSB)
 	 */
-	for (i = 0; i < 4; i++) {
-		byte = (msg->data_in >> i * 8) & 0xff;
+	for (i = 0; i < AMD_SBI_MB_DATA_SIZE; i++) {
+		byte = msg->data_in.reg_in[i];
 		ret = regmap_write(data->regmap, SBRMI_INBNDMSG1 + i, byte);
 		if (ret < 0)
 			goto exit_unlock;
@@ -74,13 +77,13 @@  int rmi_mailbox_xfer(struct sbrmi_data *data,
 	 * response Command Data Out[31:0] from SBRMI::OutBndMsg_inst[4:1]
 	 * {SBRMI_x34(MSB):SBRMI_x31(LSB)}.
 	 */
-	if (msg->read) {
-		for (i = 0; i < 4; i++) {
+	if (msg->data_in.reg_in[AMD_SBI_RD_FLAG_INDEX]) {
+		for (i = 0; i < AMD_SBI_MB_DATA_SIZE; i++) {
 			ret = regmap_read(data->regmap,
 					  SBRMI_OUTBNDMSG1 + i, &bytes);
 			if (ret < 0)
-				goto exit_unlock;
-			msg->data_out |= bytes << i * 8;
+				break;
+			msg->data_out.reg_out[i] = bytes;
 		}
 	}
 
@@ -90,8 +93,74 @@  int rmi_mailbox_xfer(struct sbrmi_data *data,
 	 */
 	ret = regmap_write(data->regmap, SBRMI_STATUS,
 			   sw_status | SW_ALERT_MASK);
-
 exit_unlock:
 	mutex_unlock(&data->lock);
 	return ret;
 }
+
+static long sbrmi_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+{
+	struct apml_message msg = { 0 };
+	struct sbrmi_data *data;
+	bool read = false;
+	int ret;
+
+	if (cmd != SBRMI_IOCTL_CMD)
+		return -ENOTTY;
+
+	if (_IOC_SIZE(cmd) != sizeof(msg))
+		return -EINVAL;
+
+	data = container_of(fp->private_data, struct sbrmi_data, sbrmi_misc_dev);
+
+	/* Copy the structure from user */
+	if (copy_from_user(&msg, (struct apml_message __user *)arg,
+			   sizeof(struct apml_message)))
+		return -EFAULT;
+
+	/* Is this a read/monitor/get request */
+	if (msg.data_in.reg_in[AMD_SBI_RD_FLAG_INDEX])
+		read = true;
+
+	switch (msg.cmd) {
+	case 0 ... 0x999:
+		/* Mailbox protocol */
+		ret = rmi_mailbox_xfer(data, &msg);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Copy results back to user only for get/monitor commands and firmware failures */
+	if ((read && !ret) || ret == -EPROTOTYPE) {
+		if (copy_to_user((struct apml_message __user *)arg, &msg,
+				 sizeof(struct apml_message)))
+			ret = -EFAULT;
+	}
+	return ret;
+}
+
+static const struct file_operations sbrmi_fops = {
+	.owner		= THIS_MODULE,
+	.unlocked_ioctl	= sbrmi_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
+};
+
+int create_misc_rmi_device(struct sbrmi_data *data,
+			   struct device *dev)
+{
+	data->sbrmi_misc_dev.name	= devm_kasprintf(dev,
+							 GFP_KERNEL,
+							 "sbrmi-%x",
+							 data->dev_static_addr);
+	data->sbrmi_misc_dev.minor	= MISC_DYNAMIC_MINOR;
+	data->sbrmi_misc_dev.fops	= &sbrmi_fops;
+	data->sbrmi_misc_dev.parent	= dev;
+	data->sbrmi_misc_dev.nodename	= devm_kasprintf(dev,
+							 GFP_KERNEL,
+							 "sbrmi-%x",
+							 data->dev_static_addr);
+	data->sbrmi_misc_dev.mode	= 0600;
+
+	return misc_register(&data->sbrmi_misc_dev);
+}
diff --git a/drivers/misc/amd-sbi/rmi-core.h b/drivers/misc/amd-sbi/rmi-core.h
index bbb6bb1cefde..e3a11575d19e 100644
--- a/drivers/misc/amd-sbi/rmi-core.h
+++ b/drivers/misc/amd-sbi/rmi-core.h
@@ -6,10 +6,12 @@ 
 #ifndef _SBRMI_CORE_H_
 #define _SBRMI_CORE_H_
 
+#include <linux/miscdevice.h>
 #include <linux/mutex.h>
 #include <linux/i2c.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <uapi/misc/amd-apml.h>
 
 /* SB-RMI registers */
 enum sbrmi_reg {
@@ -48,18 +50,15 @@  enum sbrmi_msg_id {
 
 /* Each client has this additional data */
 struct sbrmi_data {
+	struct miscdevice sbrmi_misc_dev;
 	struct regmap *regmap;
+	/* Mutex locking */
 	struct mutex lock;
 	u32 pwr_limit_max;
+	u8 dev_static_addr;
 };
 
-struct sbrmi_mailbox_msg {
-	u8 cmd;
-	bool read;
-	u32 data_in;
-	u32 data_out;
-};
-
-int rmi_mailbox_xfer(struct sbrmi_data *data, struct sbrmi_mailbox_msg *msg);
+int rmi_mailbox_xfer(struct sbrmi_data *data, struct apml_message *msg);
 int create_hwmon_sensor_device(struct device *dev, struct sbrmi_data *data);
+int create_misc_rmi_device(struct sbrmi_data *data, struct device *dev);
 #endif /*_SBRMI_CORE_H_*/
diff --git a/drivers/misc/amd-sbi/rmi-hwmon.c b/drivers/misc/amd-sbi/rmi-hwmon.c
index 68217f2afb89..318bf4fed967 100644
--- a/drivers/misc/amd-sbi/rmi-hwmon.c
+++ b/drivers/misc/amd-sbi/rmi-hwmon.c
@@ -6,6 +6,7 @@ 
  */
 #include <linux/err.h>
 #include <linux/hwmon.h>
+#include <uapi/misc/amd-apml.h>
 #include "rmi-core.h"
 
 /* Do not allow setting negative power limit */
@@ -15,7 +16,7 @@  static int sbrmi_read(struct device *dev, enum hwmon_sensor_types type,
 		      u32 attr, int channel, long *val)
 {
 	struct sbrmi_data *data = dev_get_drvdata(dev);
-	struct sbrmi_mailbox_msg msg = { 0 };
+	struct apml_message msg = { 0 };
 	int ret;
 
 	if (!data)
@@ -24,7 +25,7 @@  static int sbrmi_read(struct device *dev, enum hwmon_sensor_types type,
 	if (type != hwmon_power)
 		return -EINVAL;
 
-	msg.read = true;
+	msg.data_in.reg_in[AMD_SBI_RD_FLAG_INDEX] = 1;
 	switch (attr) {
 	case hwmon_power_input:
 		msg.cmd = SBRMI_READ_PKG_PWR_CONSUMPTION;
@@ -35,7 +36,7 @@  static int sbrmi_read(struct device *dev, enum hwmon_sensor_types type,
 		ret = rmi_mailbox_xfer(data, &msg);
 		break;
 	case hwmon_power_cap_max:
-		msg.data_out = data->pwr_limit_max;
+		msg.data_out.mb_out[AMD_SBI_RD_WR_DATA_INDEX] = data->pwr_limit_max;
 		ret = 0;
 		break;
 	default:
@@ -44,7 +45,7 @@  static int sbrmi_read(struct device *dev, enum hwmon_sensor_types type,
 	if (ret < 0)
 		return ret;
 	/* hwmon power attributes are in microWatt */
-	*val = (long)msg.data_out * 1000;
+	*val = (long)msg.data_out.mb_out[AMD_SBI_RD_WR_DATA_INDEX] * 1000;
 	return ret;
 }
 
@@ -52,7 +53,7 @@  static int sbrmi_write(struct device *dev, enum hwmon_sensor_types type,
 		       u32 attr, int channel, long val)
 {
 	struct sbrmi_data *data = dev_get_drvdata(dev);
-	struct sbrmi_mailbox_msg msg = { 0 };
+	struct apml_message msg = { 0 };
 
 	if (!data)
 		return -ENODEV;
@@ -68,8 +69,8 @@  static int sbrmi_write(struct device *dev, enum hwmon_sensor_types type,
 	val = clamp_val(val, SBRMI_PWR_MIN, data->pwr_limit_max);
 
 	msg.cmd = SBRMI_WRITE_PKG_PWR_LIMIT;
-	msg.data_in = val;
-	msg.read = false;
+	msg.data_in.mb_in[AMD_SBI_RD_WR_DATA_INDEX] = val;
+	msg.data_in.reg_in[AMD_SBI_RD_FLAG_INDEX] = 0;
 
 	return rmi_mailbox_xfer(data, &msg);
 }
diff --git a/drivers/misc/amd-sbi/rmi-i2c.c b/drivers/misc/amd-sbi/rmi-i2c.c
index 7a9801273a4c..d9308b52eab9 100644
--- a/drivers/misc/amd-sbi/rmi-i2c.c
+++ b/drivers/misc/amd-sbi/rmi-i2c.c
@@ -38,15 +38,15 @@  static int sbrmi_enable_alert(struct sbrmi_data *data)
 
 static int sbrmi_get_max_pwr_limit(struct sbrmi_data *data)
 {
-	struct sbrmi_mailbox_msg msg = { 0 };
+	struct apml_message msg = { 0 };
 	int ret;
 
 	msg.cmd = SBRMI_READ_PKG_MAX_PWR_LIMIT;
-	msg.read = true;
+	msg.data_in.reg_in[AMD_SBI_RD_FLAG_INDEX] = 1;
 	ret = rmi_mailbox_xfer(data, &msg);
 	if (ret < 0)
 		return ret;
-	data->pwr_limit_max = msg.data_out;
+	data->pwr_limit_max = msg.data_out.mb_out[AMD_SBI_RD_WR_DATA_INDEX];
 
 	return ret;
 }
@@ -81,8 +81,25 @@  static int sbrmi_i2c_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
+	data->dev_static_addr = client->addr;
 	dev_set_drvdata(dev, data);
-	return create_hwmon_sensor_device(dev, data);
+
+	ret = create_hwmon_sensor_device(dev, data);
+	if (ret < 0)
+		return ret;
+	return create_misc_rmi_device(data, dev);
+}
+
+static void sbrmi_i2c_remove(struct i2c_client *client)
+{
+	struct sbrmi_data *data = dev_get_drvdata(&client->dev);
+
+	misc_deregister(&data->sbrmi_misc_dev);
+	/* Assign fops and parent of misc dev to NULL */
+	data->sbrmi_misc_dev.fops = NULL;
+	data->sbrmi_misc_dev.parent = NULL;
+	dev_info(&client->dev, "Removed sbrmi-i2c driver\n");
+	return;
 }
 
 static const struct i2c_device_id sbrmi_id[] = {
@@ -105,6 +122,7 @@  static struct i2c_driver sbrmi_driver = {
 		.of_match_table = of_match_ptr(sbrmi_of_match),
 	},
 	.probe = sbrmi_i2c_probe,
+	.remove = sbrmi_i2c_remove,
 	.id_table = sbrmi_id,
 };
 
diff --git a/include/uapi/misc/amd-apml.h b/include/uapi/misc/amd-apml.h
new file mode 100644
index 000000000000..451cab135a08
--- /dev/null
+++ b/include/uapi/misc/amd-apml.h
@@ -0,0 +1,64 @@ 
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (C) 2021-2024 Advanced Micro Devices, Inc.
+ */
+#ifndef _AMD_APML_H_
+#define _AMD_APML_H_
+
+#include <linux/types.h>
+
+/* These are byte indexes into data_in and data_out arrays */
+#define AMD_SBI_RD_WR_DATA_INDEX	0
+#define AMD_SBI_REG_OFF_INDEX		0
+#define AMD_SBI_REG_VAL_INDEX		4
+#define AMD_SBI_RD_FLAG_INDEX		7
+
+#define AMD_SBI_MB_DATA_SIZE		4
+
+struct apml_message {
+	/*
+	 * [0]...[3] mailbox 32bit input
+	 * [7] read/write functionality
+	 */
+	union {
+		__u32 mb_in[2];
+		__u8 reg_in[8];
+	} data_in;
+	/*
+	 * 8 bit data for reg read,
+	 * 32 bit data in case of mailbox,
+	 */
+	union {
+		__u32 mb_out[2];
+		__u8 reg_out[8];
+	} data_out;
+	/* message ids:
+	 * Mailbox Messages:	0x0 ... 0x999
+	 */
+	__u32 cmd;
+};
+
+/*
+ * AMD sideband interface base IOCTL
+ */
+#define SB_BASE_IOCTL_NR	0xF9
+
+/**
+ * DOC: SBRMI_IOCTL_CMD
+ *
+ * @Parameters
+ *
+ * @struct apml_message
+ *	Pointer to the &struct apml_message that will contain the protocol
+ *	information
+ *
+ * @Description
+ * IOCTL command for APML messages using generic _IOWR
+ * The IOCTL provides userspace access to AMD sideband protocols
+ * The APML RMI module checks whether the cmd is
+ * - Mailbox message read/write(0x0~0x999)
+ * - returning "-EFAULT" if none of the above
+ */
+#define SBRMI_IOCTL_CMD		_IOWR(SB_BASE_IOCTL_NR, 0, struct apml_message)
+
+#endif /*_AMD_APML_H_*/