2019-06-10 18:57:48

by Asmaa Mnebhi

[permalink] [raw]
Subject: [PATCH v12 1/1] Add support for IPMB driver

Support receiving IPMB requests on a Satellite MC from the BMC.
Once a response is ready, this driver will send back a response
to the BMC via the IPMB channel.

Signed-off-by: Asmaa Mnebhi <[email protected]>
Acked-by: [email protected]
---
Documentation/IPMB.txt | 103 +++++++++++
drivers/char/ipmi/Kconfig | 8 +
drivers/char/ipmi/Makefile | 1 +
drivers/char/ipmi/ipmb_dev_int.c | 364 +++++++++++++++++++++++++++++++++++++++
4 files changed, 476 insertions(+)
create mode 100644 Documentation/IPMB.txt
create mode 100644 drivers/char/ipmi/ipmb_dev_int.c

diff --git a/Documentation/IPMB.txt b/Documentation/IPMB.txt
new file mode 100644
index 0000000..7160d53
--- /dev/null
+++ b/Documentation/IPMB.txt
@@ -0,0 +1,103 @@
+==============================
+IPMB Driver for a Satellite MC
+==============================
+
+The Intelligent Platform Management Bus or IPMB, is an
+I2C bus that provides a standardized interconnection between
+different boards within a chassis. This interconnection is
+between the baseboard management (BMC) and chassis electronics.
+IPMB is also associated with the messaging protocol through the
+IPMB bus.
+
+The devices using the IPMB are usually management
+controllers that perform management functions such as servicing
+the front panel interface, monitoring the baseboard,
+hot-swapping disk drivers in the system chassis, etc...
+
+When an IPMB is implemented in the system, the BMC serves as
+a controller to give system software access to the IPMB. The BMC
+sends IPMI requests to a device (usually a Satellite Management
+Controller or Satellite MC) via IPMB and the device
+sends a response back to the BMC.
+
+For more information on IPMB and the format of an IPMB message,
+refer to the IPMB and IPMI specifications.
+
+IPMB driver for Satellite MC
+----------------------------
+
+ipmb-dev-int - This is the driver needed on a Satellite MC to
+receive IPMB messages from a BMC and send a response back.
+This driver works with the I2C driver and a userspace
+program such as OpenIPMI:
+
+1) It is an I2C slave backend driver. So, it defines a callback
+function to set the Satellite MC as an I2C slave.
+This callback function handles the received IPMI requests.
+
+2) It defines the read and write functions to enable a user
+space program (such as OpenIPMI) to communicate with the kernel.
+
+
+Load the IPMB driver
+--------------------
+
+The driver needs to be loaded at boot time or manually first.
+First, make sure you have the following in your config file:
+CONFIG_IPMB_DEVICE_INTERFACE=y
+
+1) If you want the driver to be loaded at boot time:
+
+a) Add this entry to your ACPI table, under the appropriate SMBus:
+
+Device (SMB0) // Example SMBus host controller
+{
+ Name (_HID, "<Vendor-Specific HID>") // Vendor-Specific HID
+ Name (_UID, 0) // Unique ID of particular host controller
+ :
+ :
+ Device (IPMB)
+ {
+ Name (_HID, "IPMB0001") // IPMB device interface
+ Name (_UID, 0) // Unique device identifier
+ }
+}
+
+b) Example for device tree:
+
+&i2c2 {
+ status = "okay";
+
+ ipmb@10 {
+ compatible = "ipmb-dev";
+ reg = <0x10>;
+ };
+};
+
+2) Manually from Linux:
+modprobe ipmb-dev-int
+
+
+Instantiate the device
+----------------------
+
+After loading the driver, you can instantiate the device as
+described in 'Documentation/i2c/instantiating-devices'.
+If you have multiple BMCs, each connected to your Satellite MC via
+a different I2C bus, you can instantiate a device for each of
+those BMCs.
+The name of the instantiated device contains the I2C bus number
+associated with it as follows:
+
+BMC1 ------ IPMB/I2C bus 1 ---------| /dev/ipmb-1
+ Satellite MC
+BMC1 ------ IPMB/I2C bus 2 ---------| /dev/ipmb-2
+
+For instance, you can instantiate the ipmb-dev-int device from
+user space at the 7 bit address 0x10 on bus 2:
+
+ # echo ipmb-dev 0x1010 > /sys/bus/i2c/devices/i2c-2/new_device
+
+This will create the device file /dev/ipmb-2, which can be accessed
+by the user space program. The device needs to be instantiated
+before running the user space program.
diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index caac5d2..811fccb 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -75,6 +75,14 @@ config IPMI_SSIF
have a driver that must be accessed over an I2C bus instead of a
standard interface. This module requires I2C support.

+config IPMB_DEVICE_INTERFACE
+ tristate 'IPMB Interface handler'
+ depends on I2C_SLAVE
+ help
+ Provides a driver for a device (Satellite MC) to
+ receive requests and send responses back to the BMC via
+ the IPMB interface. This module requires I2C support.
+
config IPMI_POWERNV
depends on PPC_POWERNV
tristate 'POWERNV (OPAL firmware) IPMI interface'
diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index 3f06b20..0822adc 100644
--- a/drivers/char/ipmi/Makefile
+++ b/drivers/char/ipmi/Makefile
@@ -26,3 +26,4 @@ obj-$(CONFIG_IPMI_KCS_BMC) += kcs_bmc.o
obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o
obj-$(CONFIG_ASPEED_KCS_IPMI_BMC) += kcs_bmc_aspeed.o
obj-$(CONFIG_NPCM7XX_KCS_IPMI_BMC) += kcs_bmc_npcm7xx.o
+obj-$(CONFIG_IPMB_DEVICE_INTERFACE) += ipmb_dev_int.o
diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c
new file mode 100644
index 0000000..2895abf
--- /dev/null
+++ b/drivers/char/ipmi/ipmb_dev_int.c
@@ -0,0 +1,364 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * IPMB driver to receive a request and send a response
+ *
+ * Copyright (C) 2019 Mellanox Techologies, Ltd.
+ *
+ * This was inspired by Brendan Higgins' ipmi-bmc-bt-i2c driver.
+ */
+
+#include <linux/acpi.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/wait.h>
+
+#define MAX_MSG_LEN 128
+#define IPMB_REQUEST_LEN_MIN 7
+#define NETFN_RSP_BIT_MASK 0x4
+#define REQUEST_QUEUE_MAX_LEN 256
+
+#define IPMB_MSG_LEN_IDX 0
+#define RQ_SA_8BIT_IDX 1
+#define NETFN_LUN_IDX 2
+
+#define GET_7BIT_ADDR(addr_8bit) (addr_8bit >> 1)
+#define GET_8BIT_ADDR(addr_7bit) ((addr_7bit << 1) & 0xff)
+
+#define IPMB_MSG_PAYLOAD_LEN_MAX (MAX_MSG_LEN - IPMB_REQUEST_LEN_MIN - 1)
+
+#define SMBUS_MSG_HEADER_LENGTH 2
+#define SMBUS_MSG_IDX_OFFSET (SMBUS_MSG_HEADER_LENGTH + 1)
+
+struct ipmb_msg {
+ u8 len;
+ u8 rs_sa;
+ u8 netfn_rs_lun;
+ u8 checksum1;
+ u8 rq_sa;
+ u8 rq_seq_rq_lun;
+ u8 cmd;
+ u8 payload[IPMB_MSG_PAYLOAD_LEN_MAX];
+ /* checksum2 is included in payload */
+} __packed;
+
+struct ipmb_request_elem {
+ struct list_head list;
+ struct ipmb_msg request;
+};
+
+struct ipmb_dev {
+ struct i2c_client *client;
+ struct miscdevice miscdev;
+ struct ipmb_msg request;
+ struct list_head request_queue;
+ atomic_t request_queue_len;
+ size_t msg_idx;
+ spinlock_t lock;
+ wait_queue_head_t wait_queue;
+ struct mutex file_mutex;
+};
+
+static inline struct ipmb_dev *to_ipmb_dev(struct file *file)
+{
+ return container_of(file->private_data, struct ipmb_dev, miscdev);
+}
+
+static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
+ struct ipmb_request_elem *queue_elem;
+ struct ipmb_msg msg;
+ ssize_t ret;
+
+ memset(&msg, 0, sizeof(msg));
+
+ spin_lock_irq(&ipmb_dev->lock);
+
+ while (list_empty(&ipmb_dev->request_queue)) {
+ spin_unlock_irq(&ipmb_dev->lock);
+
+ if (file->f_flags & O_NONBLOCK)
+ return -EAGAIN;
+
+ ret = wait_event_interruptible(ipmb_dev->wait_queue,
+ !list_empty(&ipmb_dev->request_queue));
+ if (ret)
+ return ret;
+
+ spin_lock_irq(&ipmb_dev->lock);
+ }
+
+ queue_elem = list_first_entry(&ipmb_dev->request_queue,
+ struct ipmb_request_elem, list);
+ memcpy(&msg, &queue_elem->request, sizeof(msg));
+ list_del(&queue_elem->list);
+ kfree(queue_elem);
+ atomic_dec(&ipmb_dev->request_queue_len);
+
+ spin_unlock_irq(&ipmb_dev->lock);
+
+ count = min_t(size_t, count, msg.len + 1);
+ if (copy_to_user(buf, &msg, count))
+ ret = -EFAULT;
+
+ return ret < 0 ? ret : count;
+}
+
+static ssize_t ipmb_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
+ u8 rq_sa, netf_rq_lun, msg_len;
+ struct i2c_client rq_client;
+ u8 msg[MAX_MSG_LEN];
+ ssize_t ret;
+
+ if (count > sizeof(msg))
+ return -EINVAL;
+
+ if (copy_from_user(&msg, buf, count))
+ return -EFAULT;
+
+ if (count < msg[0])
+ return -EINVAL;
+
+ rq_sa = GET_7BIT_ADDR(msg[RQ_SA_8BIT_IDX]);
+ netf_rq_lun = msg[NETFN_LUN_IDX];
+
+ if (!(netf_rq_lun & NETFN_RSP_BIT_MASK))
+ return -EINVAL;
+
+ /*
+ * subtract rq_sa and netf_rq_lun from the length of the msg passed to
+ * i2c_smbus_write_block_data_local
+ */
+ msg_len = msg[IPMB_MSG_LEN_IDX] - SMBUS_MSG_HEADER_LENGTH;
+
+ strcpy(rq_client.name, "ipmb_requester");
+ rq_client.adapter = ipmb_dev->client->adapter;
+ rq_client.flags = ipmb_dev->client->flags;
+ rq_client.addr = rq_sa;
+
+ ret = i2c_smbus_write_block_data(&rq_client, netf_rq_lun, msg_len,
+ msg + SMBUS_MSG_IDX_OFFSET);
+
+ return ret ? : count;
+}
+
+static unsigned int ipmb_poll(struct file *file, poll_table *wait)
+{
+ struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
+ unsigned int mask = POLLOUT;
+
+ mutex_lock(&ipmb_dev->file_mutex);
+ poll_wait(file, &ipmb_dev->wait_queue, wait);
+
+ if (atomic_read(&ipmb_dev->request_queue_len))
+ mask |= POLLIN;
+ mutex_unlock(&ipmb_dev->file_mutex);
+
+ return mask;
+}
+
+static const struct file_operations ipmb_fops = {
+ .owner = THIS_MODULE,
+ .read = ipmb_read,
+ .write = ipmb_write,
+ .poll = ipmb_poll,
+};
+
+/* Called with ipmb_dev->lock held. */
+static void ipmb_handle_request(struct ipmb_dev *ipmb_dev)
+{
+ struct ipmb_request_elem *queue_elem;
+
+ if (atomic_read(&ipmb_dev->request_queue_len) >=
+ REQUEST_QUEUE_MAX_LEN)
+ return;
+
+ queue_elem = kmalloc(sizeof(*queue_elem), GFP_ATOMIC);
+ if (!queue_elem)
+ return;
+
+ memcpy(&queue_elem->request, &ipmb_dev->request,
+ sizeof(struct ipmb_msg));
+ list_add(&queue_elem->list, &ipmb_dev->request_queue);
+ atomic_inc(&ipmb_dev->request_queue_len);
+ wake_up_all(&ipmb_dev->wait_queue);
+}
+
+static u8 ipmb_verify_checksum1(struct ipmb_dev *ipmb_dev, u8 rs_sa)
+{
+ /* The 8 lsb of the sum is 0 when the checksum is valid */
+ return (rs_sa + ipmb_dev->request.netfn_rs_lun +
+ ipmb_dev->request.checksum1);
+}
+
+static bool is_ipmb_request(struct ipmb_dev *ipmb_dev, u8 rs_sa)
+{
+ if (ipmb_dev->msg_idx >= IPMB_REQUEST_LEN_MIN) {
+ if (ipmb_verify_checksum1(ipmb_dev, rs_sa))
+ return false;
+
+ /*
+ * Check whether this is an IPMB request or
+ * response.
+ * The 6 MSB of netfn_rs_lun are dedicated to the netfn
+ * while the remaining bits are dedicated to the lun.
+ * If the LSB of the netfn is cleared, it is associated
+ * with an IPMB request.
+ * If the LSB of the netfn is set, it is associated with
+ * an IPMB response.
+ */
+ if (!(ipmb_dev->request.netfn_rs_lun & NETFN_RSP_BIT_MASK))
+ return true;
+ }
+ return false;
+}
+
+/*
+ * The IPMB protocol only supports I2C Writes so there is no need
+ * to support I2C_SLAVE_READ* events.
+ * This i2c callback function only monitors IPMB request messages
+ * and adds them in a queue, so that they can be handled by
+ * receive_ipmb_request.
+ */
+static int ipmb_slave_cb(struct i2c_client *client,
+ enum i2c_slave_event event, u8 *val)
+{
+ struct ipmb_dev *ipmb_dev = i2c_get_clientdata(client);
+ u8 *buf = (u8 *)&ipmb_dev->request;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ipmb_dev->lock, flags);
+ switch (event) {
+ case I2C_SLAVE_WRITE_REQUESTED:
+ memset(&ipmb_dev->request, 0, sizeof(ipmb_dev->request));
+ ipmb_dev->msg_idx = 0;
+
+ /*
+ * At index 0, ipmb_msg stores the length of msg,
+ * skip it for now.
+ * The len will be populated once the whole
+ * buf is populated.
+ *
+ * The I2C bus driver's responsibility is to pass the
+ * data bytes to the backend driver; it does not
+ * forward the i2c slave address.
+ * Since the first byte in the IPMB message is the
+ * address of the responder, it is the responsibility
+ * of the IPMB driver to format the message properly.
+ * So this driver prepends the address of the responder
+ * to the received i2c data before the request message
+ * is handled in userland.
+ */
+ buf[++ipmb_dev->msg_idx] = GET_8BIT_ADDR(client->addr);
+ break;
+
+ case I2C_SLAVE_WRITE_RECEIVED:
+ if (ipmb_dev->msg_idx >= sizeof(struct ipmb_msg))
+ break;
+
+ buf[++ipmb_dev->msg_idx] = *val;
+ break;
+
+ case I2C_SLAVE_STOP:
+ ipmb_dev->request.len = ipmb_dev->msg_idx;
+
+ if (is_ipmb_request(ipmb_dev, GET_8BIT_ADDR(client->addr)))
+ ipmb_handle_request(ipmb_dev);
+ break;
+
+ default:
+ break;
+ }
+ spin_unlock_irqrestore(&ipmb_dev->lock, flags);
+
+ return 0;
+}
+
+static int ipmb_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct ipmb_dev *ipmb_dev;
+ int ret;
+
+ ipmb_dev = devm_kzalloc(&client->dev, sizeof(*ipmb_dev),
+ GFP_KERNEL);
+ if (!ipmb_dev)
+ return -ENOMEM;
+
+ spin_lock_init(&ipmb_dev->lock);
+ init_waitqueue_head(&ipmb_dev->wait_queue);
+ atomic_set(&ipmb_dev->request_queue_len, 0);
+ INIT_LIST_HEAD(&ipmb_dev->request_queue);
+
+ mutex_init(&ipmb_dev->file_mutex);
+
+ ipmb_dev->miscdev.minor = MISC_DYNAMIC_MINOR;
+
+ ipmb_dev->miscdev.name = devm_kasprintf(&client->dev, GFP_KERNEL,
+ "%s%d", "ipmb-",
+ client->adapter->nr);
+ ipmb_dev->miscdev.fops = &ipmb_fops;
+ ipmb_dev->miscdev.parent = &client->dev;
+ ret = misc_register(&ipmb_dev->miscdev);
+ if (ret)
+ return ret;
+
+ ipmb_dev->client = client;
+ i2c_set_clientdata(client, ipmb_dev);
+ ret = i2c_slave_register(client, ipmb_slave_cb);
+ if (ret) {
+ misc_deregister(&ipmb_dev->miscdev);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ipmb_remove(struct i2c_client *client)
+{
+ struct ipmb_dev *ipmb_dev = i2c_get_clientdata(client);
+
+ i2c_slave_unregister(client);
+ misc_deregister(&ipmb_dev->miscdev);
+
+ return 0;
+}
+
+static const struct i2c_device_id ipmb_id[] = {
+ { "ipmb-dev", 0 },
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, ipmb_id);
+
+static const struct acpi_device_id acpi_ipmb_id[] = {
+ { "IPMB0001", 0 },
+ {},
+};
+MODULE_DEVICE_TABLE(acpi, acpi_ipmb_id);
+
+static struct i2c_driver ipmb_driver = {
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "ipmb-dev",
+ .acpi_match_table = ACPI_PTR(acpi_ipmb_id),
+ },
+ .probe = ipmb_probe,
+ .remove = ipmb_remove,
+ .id_table = ipmb_id,
+};
+module_i2c_driver(ipmb_driver);
+
+MODULE_AUTHOR("Mellanox Technologies");
+MODULE_DESCRIPTION("IPMB driver");
+MODULE_LICENSE("GPL v2");
--
2.1.2


2019-06-10 19:53:16

by Corey Minyard

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] Add support for IPMB driver

On Mon, Jun 10, 2019 at 02:57:02PM -0400, Asmaa Mnebhi wrote:
> Support receiving IPMB requests on a Satellite MC from the BMC.
> Once a response is ready, this driver will send back a response
> to the BMC via the IPMB channel.

Ok, you have been patient enough on this, and I think it's good
now :). I've added it to the IPMI for-next tree.

I did move the config section to outside the CONFIG_IPMI_MSGHANLDER
section, as it's not dependent on that option.

-corey

>
> Signed-off-by: Asmaa Mnebhi <[email protected]>
> Acked-by: [email protected]
> ---
> Documentation/IPMB.txt | 103 +++++++++++
> drivers/char/ipmi/Kconfig | 8 +
> drivers/char/ipmi/Makefile | 1 +
> drivers/char/ipmi/ipmb_dev_int.c | 364 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 476 insertions(+)
> create mode 100644 Documentation/IPMB.txt
> create mode 100644 drivers/char/ipmi/ipmb_dev_int.c
>
> diff --git a/Documentation/IPMB.txt b/Documentation/IPMB.txt
> new file mode 100644
> index 0000000..7160d53
> --- /dev/null
> +++ b/Documentation/IPMB.txt
> @@ -0,0 +1,103 @@
> +==============================
> +IPMB Driver for a Satellite MC
> +==============================
> +
> +The Intelligent Platform Management Bus or IPMB, is an
> +I2C bus that provides a standardized interconnection between
> +different boards within a chassis. This interconnection is
> +between the baseboard management (BMC) and chassis electronics.
> +IPMB is also associated with the messaging protocol through the
> +IPMB bus.
> +
> +The devices using the IPMB are usually management
> +controllers that perform management functions such as servicing
> +the front panel interface, monitoring the baseboard,
> +hot-swapping disk drivers in the system chassis, etc...
> +
> +When an IPMB is implemented in the system, the BMC serves as
> +a controller to give system software access to the IPMB. The BMC
> +sends IPMI requests to a device (usually a Satellite Management
> +Controller or Satellite MC) via IPMB and the device
> +sends a response back to the BMC.
> +
> +For more information on IPMB and the format of an IPMB message,
> +refer to the IPMB and IPMI specifications.
> +
> +IPMB driver for Satellite MC
> +----------------------------
> +
> +ipmb-dev-int - This is the driver needed on a Satellite MC to
> +receive IPMB messages from a BMC and send a response back.
> +This driver works with the I2C driver and a userspace
> +program such as OpenIPMI:
> +
> +1) It is an I2C slave backend driver. So, it defines a callback
> +function to set the Satellite MC as an I2C slave.
> +This callback function handles the received IPMI requests.
> +
> +2) It defines the read and write functions to enable a user
> +space program (such as OpenIPMI) to communicate with the kernel.
> +
> +
> +Load the IPMB driver
> +--------------------
> +
> +The driver needs to be loaded at boot time or manually first.
> +First, make sure you have the following in your config file:
> +CONFIG_IPMB_DEVICE_INTERFACE=y
> +
> +1) If you want the driver to be loaded at boot time:
> +
> +a) Add this entry to your ACPI table, under the appropriate SMBus:
> +
> +Device (SMB0) // Example SMBus host controller
> +{
> + Name (_HID, "<Vendor-Specific HID>") // Vendor-Specific HID
> + Name (_UID, 0) // Unique ID of particular host controller
> + :
> + :
> + Device (IPMB)
> + {
> + Name (_HID, "IPMB0001") // IPMB device interface
> + Name (_UID, 0) // Unique device identifier
> + }
> +}
> +
> +b) Example for device tree:
> +
> +&i2c2 {
> + status = "okay";
> +
> + ipmb@10 {
> + compatible = "ipmb-dev";
> + reg = <0x10>;
> + };
> +};
> +
> +2) Manually from Linux:
> +modprobe ipmb-dev-int
> +
> +
> +Instantiate the device
> +----------------------
> +
> +After loading the driver, you can instantiate the device as
> +described in 'Documentation/i2c/instantiating-devices'.
> +If you have multiple BMCs, each connected to your Satellite MC via
> +a different I2C bus, you can instantiate a device for each of
> +those BMCs.
> +The name of the instantiated device contains the I2C bus number
> +associated with it as follows:
> +
> +BMC1 ------ IPMB/I2C bus 1 ---------| /dev/ipmb-1
> + Satellite MC
> +BMC1 ------ IPMB/I2C bus 2 ---------| /dev/ipmb-2
> +
> +For instance, you can instantiate the ipmb-dev-int device from
> +user space at the 7 bit address 0x10 on bus 2:
> +
> + # echo ipmb-dev 0x1010 > /sys/bus/i2c/devices/i2c-2/new_device
> +
> +This will create the device file /dev/ipmb-2, which can be accessed
> +by the user space program. The device needs to be instantiated
> +before running the user space program.
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index caac5d2..811fccb 100644
> --- a/drivers/char/ipmi/Kconfig
> +++ b/drivers/char/ipmi/Kconfig
> @@ -75,6 +75,14 @@ config IPMI_SSIF
> have a driver that must be accessed over an I2C bus instead of a
> standard interface. This module requires I2C support.
>
> +config IPMB_DEVICE_INTERFACE
> + tristate 'IPMB Interface handler'
> + depends on I2C_SLAVE
> + help
> + Provides a driver for a device (Satellite MC) to
> + receive requests and send responses back to the BMC via
> + the IPMB interface. This module requires I2C support.
> +
> config IPMI_POWERNV
> depends on PPC_POWERNV
> tristate 'POWERNV (OPAL firmware) IPMI interface'
> diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
> index 3f06b20..0822adc 100644
> --- a/drivers/char/ipmi/Makefile
> +++ b/drivers/char/ipmi/Makefile
> @@ -26,3 +26,4 @@ obj-$(CONFIG_IPMI_KCS_BMC) += kcs_bmc.o
> obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o
> obj-$(CONFIG_ASPEED_KCS_IPMI_BMC) += kcs_bmc_aspeed.o
> obj-$(CONFIG_NPCM7XX_KCS_IPMI_BMC) += kcs_bmc_npcm7xx.o
> +obj-$(CONFIG_IPMB_DEVICE_INTERFACE) += ipmb_dev_int.o
> diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c
> new file mode 100644
> index 0000000..2895abf
> --- /dev/null
> +++ b/drivers/char/ipmi/ipmb_dev_int.c
> @@ -0,0 +1,364 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * IPMB driver to receive a request and send a response
> + *
> + * Copyright (C) 2019 Mellanox Techologies, Ltd.
> + *
> + * This was inspired by Brendan Higgins' ipmi-bmc-bt-i2c driver.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/poll.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/wait.h>
> +
> +#define MAX_MSG_LEN 128
> +#define IPMB_REQUEST_LEN_MIN 7
> +#define NETFN_RSP_BIT_MASK 0x4
> +#define REQUEST_QUEUE_MAX_LEN 256
> +
> +#define IPMB_MSG_LEN_IDX 0
> +#define RQ_SA_8BIT_IDX 1
> +#define NETFN_LUN_IDX 2
> +
> +#define GET_7BIT_ADDR(addr_8bit) (addr_8bit >> 1)
> +#define GET_8BIT_ADDR(addr_7bit) ((addr_7bit << 1) & 0xff)
> +
> +#define IPMB_MSG_PAYLOAD_LEN_MAX (MAX_MSG_LEN - IPMB_REQUEST_LEN_MIN - 1)
> +
> +#define SMBUS_MSG_HEADER_LENGTH 2
> +#define SMBUS_MSG_IDX_OFFSET (SMBUS_MSG_HEADER_LENGTH + 1)
> +
> +struct ipmb_msg {
> + u8 len;
> + u8 rs_sa;
> + u8 netfn_rs_lun;
> + u8 checksum1;
> + u8 rq_sa;
> + u8 rq_seq_rq_lun;
> + u8 cmd;
> + u8 payload[IPMB_MSG_PAYLOAD_LEN_MAX];
> + /* checksum2 is included in payload */
> +} __packed;
> +
> +struct ipmb_request_elem {
> + struct list_head list;
> + struct ipmb_msg request;
> +};
> +
> +struct ipmb_dev {
> + struct i2c_client *client;
> + struct miscdevice miscdev;
> + struct ipmb_msg request;
> + struct list_head request_queue;
> + atomic_t request_queue_len;
> + size_t msg_idx;
> + spinlock_t lock;
> + wait_queue_head_t wait_queue;
> + struct mutex file_mutex;
> +};
> +
> +static inline struct ipmb_dev *to_ipmb_dev(struct file *file)
> +{
> + return container_of(file->private_data, struct ipmb_dev, miscdev);
> +}
> +
> +static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count,
> + loff_t *ppos)
> +{
> + struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
> + struct ipmb_request_elem *queue_elem;
> + struct ipmb_msg msg;
> + ssize_t ret;
> +
> + memset(&msg, 0, sizeof(msg));
> +
> + spin_lock_irq(&ipmb_dev->lock);
> +
> + while (list_empty(&ipmb_dev->request_queue)) {
> + spin_unlock_irq(&ipmb_dev->lock);
> +
> + if (file->f_flags & O_NONBLOCK)
> + return -EAGAIN;
> +
> + ret = wait_event_interruptible(ipmb_dev->wait_queue,
> + !list_empty(&ipmb_dev->request_queue));
> + if (ret)
> + return ret;
> +
> + spin_lock_irq(&ipmb_dev->lock);
> + }
> +
> + queue_elem = list_first_entry(&ipmb_dev->request_queue,
> + struct ipmb_request_elem, list);
> + memcpy(&msg, &queue_elem->request, sizeof(msg));
> + list_del(&queue_elem->list);
> + kfree(queue_elem);
> + atomic_dec(&ipmb_dev->request_queue_len);
> +
> + spin_unlock_irq(&ipmb_dev->lock);
> +
> + count = min_t(size_t, count, msg.len + 1);
> + if (copy_to_user(buf, &msg, count))
> + ret = -EFAULT;
> +
> + return ret < 0 ? ret : count;
> +}
> +
> +static ssize_t ipmb_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
> + u8 rq_sa, netf_rq_lun, msg_len;
> + struct i2c_client rq_client;
> + u8 msg[MAX_MSG_LEN];
> + ssize_t ret;
> +
> + if (count > sizeof(msg))
> + return -EINVAL;
> +
> + if (copy_from_user(&msg, buf, count))
> + return -EFAULT;
> +
> + if (count < msg[0])
> + return -EINVAL;
> +
> + rq_sa = GET_7BIT_ADDR(msg[RQ_SA_8BIT_IDX]);
> + netf_rq_lun = msg[NETFN_LUN_IDX];
> +
> + if (!(netf_rq_lun & NETFN_RSP_BIT_MASK))
> + return -EINVAL;
> +
> + /*
> + * subtract rq_sa and netf_rq_lun from the length of the msg passed to
> + * i2c_smbus_write_block_data_local
> + */
> + msg_len = msg[IPMB_MSG_LEN_IDX] - SMBUS_MSG_HEADER_LENGTH;
> +
> + strcpy(rq_client.name, "ipmb_requester");
> + rq_client.adapter = ipmb_dev->client->adapter;
> + rq_client.flags = ipmb_dev->client->flags;
> + rq_client.addr = rq_sa;
> +
> + ret = i2c_smbus_write_block_data(&rq_client, netf_rq_lun, msg_len,
> + msg + SMBUS_MSG_IDX_OFFSET);
> +
> + return ret ? : count;
> +}
> +
> +static unsigned int ipmb_poll(struct file *file, poll_table *wait)
> +{
> + struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
> + unsigned int mask = POLLOUT;
> +
> + mutex_lock(&ipmb_dev->file_mutex);
> + poll_wait(file, &ipmb_dev->wait_queue, wait);
> +
> + if (atomic_read(&ipmb_dev->request_queue_len))
> + mask |= POLLIN;
> + mutex_unlock(&ipmb_dev->file_mutex);
> +
> + return mask;
> +}
> +
> +static const struct file_operations ipmb_fops = {
> + .owner = THIS_MODULE,
> + .read = ipmb_read,
> + .write = ipmb_write,
> + .poll = ipmb_poll,
> +};
> +
> +/* Called with ipmb_dev->lock held. */
> +static void ipmb_handle_request(struct ipmb_dev *ipmb_dev)
> +{
> + struct ipmb_request_elem *queue_elem;
> +
> + if (atomic_read(&ipmb_dev->request_queue_len) >=
> + REQUEST_QUEUE_MAX_LEN)
> + return;
> +
> + queue_elem = kmalloc(sizeof(*queue_elem), GFP_ATOMIC);
> + if (!queue_elem)
> + return;
> +
> + memcpy(&queue_elem->request, &ipmb_dev->request,
> + sizeof(struct ipmb_msg));
> + list_add(&queue_elem->list, &ipmb_dev->request_queue);
> + atomic_inc(&ipmb_dev->request_queue_len);
> + wake_up_all(&ipmb_dev->wait_queue);
> +}
> +
> +static u8 ipmb_verify_checksum1(struct ipmb_dev *ipmb_dev, u8 rs_sa)
> +{
> + /* The 8 lsb of the sum is 0 when the checksum is valid */
> + return (rs_sa + ipmb_dev->request.netfn_rs_lun +
> + ipmb_dev->request.checksum1);
> +}
> +
> +static bool is_ipmb_request(struct ipmb_dev *ipmb_dev, u8 rs_sa)
> +{
> + if (ipmb_dev->msg_idx >= IPMB_REQUEST_LEN_MIN) {
> + if (ipmb_verify_checksum1(ipmb_dev, rs_sa))
> + return false;
> +
> + /*
> + * Check whether this is an IPMB request or
> + * response.
> + * The 6 MSB of netfn_rs_lun are dedicated to the netfn
> + * while the remaining bits are dedicated to the lun.
> + * If the LSB of the netfn is cleared, it is associated
> + * with an IPMB request.
> + * If the LSB of the netfn is set, it is associated with
> + * an IPMB response.
> + */
> + if (!(ipmb_dev->request.netfn_rs_lun & NETFN_RSP_BIT_MASK))
> + return true;
> + }
> + return false;
> +}
> +
> +/*
> + * The IPMB protocol only supports I2C Writes so there is no need
> + * to support I2C_SLAVE_READ* events.
> + * This i2c callback function only monitors IPMB request messages
> + * and adds them in a queue, so that they can be handled by
> + * receive_ipmb_request.
> + */
> +static int ipmb_slave_cb(struct i2c_client *client,
> + enum i2c_slave_event event, u8 *val)
> +{
> + struct ipmb_dev *ipmb_dev = i2c_get_clientdata(client);
> + u8 *buf = (u8 *)&ipmb_dev->request;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ipmb_dev->lock, flags);
> + switch (event) {
> + case I2C_SLAVE_WRITE_REQUESTED:
> + memset(&ipmb_dev->request, 0, sizeof(ipmb_dev->request));
> + ipmb_dev->msg_idx = 0;
> +
> + /*
> + * At index 0, ipmb_msg stores the length of msg,
> + * skip it for now.
> + * The len will be populated once the whole
> + * buf is populated.
> + *
> + * The I2C bus driver's responsibility is to pass the
> + * data bytes to the backend driver; it does not
> + * forward the i2c slave address.
> + * Since the first byte in the IPMB message is the
> + * address of the responder, it is the responsibility
> + * of the IPMB driver to format the message properly.
> + * So this driver prepends the address of the responder
> + * to the received i2c data before the request message
> + * is handled in userland.
> + */
> + buf[++ipmb_dev->msg_idx] = GET_8BIT_ADDR(client->addr);
> + break;
> +
> + case I2C_SLAVE_WRITE_RECEIVED:
> + if (ipmb_dev->msg_idx >= sizeof(struct ipmb_msg))
> + break;
> +
> + buf[++ipmb_dev->msg_idx] = *val;
> + break;
> +
> + case I2C_SLAVE_STOP:
> + ipmb_dev->request.len = ipmb_dev->msg_idx;
> +
> + if (is_ipmb_request(ipmb_dev, GET_8BIT_ADDR(client->addr)))
> + ipmb_handle_request(ipmb_dev);
> + break;
> +
> + default:
> + break;
> + }
> + spin_unlock_irqrestore(&ipmb_dev->lock, flags);
> +
> + return 0;
> +}
> +
> +static int ipmb_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct ipmb_dev *ipmb_dev;
> + int ret;
> +
> + ipmb_dev = devm_kzalloc(&client->dev, sizeof(*ipmb_dev),
> + GFP_KERNEL);
> + if (!ipmb_dev)
> + return -ENOMEM;
> +
> + spin_lock_init(&ipmb_dev->lock);
> + init_waitqueue_head(&ipmb_dev->wait_queue);
> + atomic_set(&ipmb_dev->request_queue_len, 0);
> + INIT_LIST_HEAD(&ipmb_dev->request_queue);
> +
> + mutex_init(&ipmb_dev->file_mutex);
> +
> + ipmb_dev->miscdev.minor = MISC_DYNAMIC_MINOR;
> +
> + ipmb_dev->miscdev.name = devm_kasprintf(&client->dev, GFP_KERNEL,
> + "%s%d", "ipmb-",
> + client->adapter->nr);
> + ipmb_dev->miscdev.fops = &ipmb_fops;
> + ipmb_dev->miscdev.parent = &client->dev;
> + ret = misc_register(&ipmb_dev->miscdev);
> + if (ret)
> + return ret;
> +
> + ipmb_dev->client = client;
> + i2c_set_clientdata(client, ipmb_dev);
> + ret = i2c_slave_register(client, ipmb_slave_cb);
> + if (ret) {
> + misc_deregister(&ipmb_dev->miscdev);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ipmb_remove(struct i2c_client *client)
> +{
> + struct ipmb_dev *ipmb_dev = i2c_get_clientdata(client);
> +
> + i2c_slave_unregister(client);
> + misc_deregister(&ipmb_dev->miscdev);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id ipmb_id[] = {
> + { "ipmb-dev", 0 },
> + {},
> +};
> +MODULE_DEVICE_TABLE(i2c, ipmb_id);
> +
> +static const struct acpi_device_id acpi_ipmb_id[] = {
> + { "IPMB0001", 0 },
> + {},
> +};
> +MODULE_DEVICE_TABLE(acpi, acpi_ipmb_id);
> +
> +static struct i2c_driver ipmb_driver = {
> + .driver = {
> + .owner = THIS_MODULE,
> + .name = "ipmb-dev",
> + .acpi_match_table = ACPI_PTR(acpi_ipmb_id),
> + },
> + .probe = ipmb_probe,
> + .remove = ipmb_remove,
> + .id_table = ipmb_id,
> +};
> +module_i2c_driver(ipmb_driver);
> +
> +MODULE_AUTHOR("Mellanox Technologies");
> +MODULE_DESCRIPTION("IPMB driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.1.2
>

2019-06-10 20:02:01

by Asmaa Mnebhi

[permalink] [raw]
Subject: RE: [PATCH v12 1/1] Add support for IPMB driver

Thank you Corey!

-----Original Message-----
From: Corey Minyard <[email protected]> On Behalf Of Corey Minyard
Sent: Monday, June 10, 2019 3:51 PM
To: Asmaa Mnebhi <[email protected]>
Cc: [email protected]; Vadim Pasternak <[email protected]>; Michael Shych <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v12 1/1] Add support for IPMB driver

On Mon, Jun 10, 2019 at 02:57:02PM -0400, Asmaa Mnebhi wrote:
> Support receiving IPMB requests on a Satellite MC from the BMC.
> Once a response is ready, this driver will send back a response to the
> BMC via the IPMB channel.

Ok, you have been patient enough on this, and I think it's good now :). I've added it to the IPMI for-next tree.

I did move the config section to outside the CONFIG_IPMI_MSGHANLDER section, as it's not dependent on that option.

-corey

>
> Signed-off-by: Asmaa Mnebhi <[email protected]>
> Acked-by: [email protected]
> ---
> Documentation/IPMB.txt | 103 +++++++++++
> drivers/char/ipmi/Kconfig | 8 +
> drivers/char/ipmi/Makefile | 1 +
> drivers/char/ipmi/ipmb_dev_int.c | 364
> +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 476 insertions(+)
> create mode 100644 Documentation/IPMB.txt create mode 100644
> drivers/char/ipmi/ipmb_dev_int.c
>
> diff --git a/Documentation/IPMB.txt b/Documentation/IPMB.txt new file
> mode 100644 index 0000000..7160d53
> --- /dev/null
> +++ b/Documentation/IPMB.txt
> @@ -0,0 +1,103 @@
> +==============================
> +IPMB Driver for a Satellite MC
> +==============================
> +
> +The Intelligent Platform Management Bus or IPMB, is an I2C bus that
> +provides a standardized interconnection between different boards
> +within a chassis. This interconnection is between the baseboard
> +management (BMC) and chassis electronics.
> +IPMB is also associated with the messaging protocol through the IPMB
> +bus.
> +
> +The devices using the IPMB are usually management controllers that
> +perform management functions such as servicing the front panel
> +interface, monitoring the baseboard, hot-swapping disk drivers in the
> +system chassis, etc...
> +
> +When an IPMB is implemented in the system, the BMC serves as a
> +controller to give system software access to the IPMB. The BMC sends
> +IPMI requests to a device (usually a Satellite Management Controller
> +or Satellite MC) via IPMB and the device sends a response back to the
> +BMC.
> +
> +For more information on IPMB and the format of an IPMB message, refer
> +to the IPMB and IPMI specifications.
> +
> +IPMB driver for Satellite MC
> +----------------------------
> +
> +ipmb-dev-int - This is the driver needed on a Satellite MC to receive
> +IPMB messages from a BMC and send a response back.
> +This driver works with the I2C driver and a userspace program such as
> +OpenIPMI:
> +
> +1) It is an I2C slave backend driver. So, it defines a callback
> +function to set the Satellite MC as an I2C slave.
> +This callback function handles the received IPMI requests.
> +
> +2) It defines the read and write functions to enable a user space
> +program (such as OpenIPMI) to communicate with the kernel.
> +
> +
> +Load the IPMB driver
> +--------------------
> +
> +The driver needs to be loaded at boot time or manually first.
> +First, make sure you have the following in your config file:
> +CONFIG_IPMB_DEVICE_INTERFACE=y
> +
> +1) If you want the driver to be loaded at boot time:
> +
> +a) Add this entry to your ACPI table, under the appropriate SMBus:
> +
> +Device (SMB0) // Example SMBus host controller {
> + Name (_HID, "<Vendor-Specific HID>") // Vendor-Specific HID
> + Name (_UID, 0) // Unique ID of particular host controller
> + :
> + :
> + Device (IPMB)
> + {
> + Name (_HID, "IPMB0001") // IPMB device interface
> + Name (_UID, 0) // Unique device identifier
> + }
> +}
> +
> +b) Example for device tree:
> +
> +&i2c2 {
> + status = "okay";
> +
> + ipmb@10 {
> + compatible = "ipmb-dev";
> + reg = <0x10>;
> + };
> +};
> +
> +2) Manually from Linux:
> +modprobe ipmb-dev-int
> +
> +
> +Instantiate the device
> +----------------------
> +
> +After loading the driver, you can instantiate the device as described
> +in 'Documentation/i2c/instantiating-devices'.
> +If you have multiple BMCs, each connected to your Satellite MC via a
> +different I2C bus, you can instantiate a device for each of those
> +BMCs.
> +The name of the instantiated device contains the I2C bus number
> +associated with it as follows:
> +
> +BMC1 ------ IPMB/I2C bus 1 ---------| /dev/ipmb-1
> + Satellite MC
> +BMC1 ------ IPMB/I2C bus 2 ---------| /dev/ipmb-2
> +
> +For instance, you can instantiate the ipmb-dev-int device from user
> +space at the 7 bit address 0x10 on bus 2:
> +
> + # echo ipmb-dev 0x1010 > /sys/bus/i2c/devices/i2c-2/new_device
> +
> +This will create the device file /dev/ipmb-2, which can be accessed
> +by the user space program. The device needs to be instantiated before
> +running the user space program.
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index caac5d2..811fccb 100644
> --- a/drivers/char/ipmi/Kconfig
> +++ b/drivers/char/ipmi/Kconfig
> @@ -75,6 +75,14 @@ config IPMI_SSIF
> have a driver that must be accessed over an I2C bus instead of a
> standard interface. This module requires I2C support.
>
> +config IPMB_DEVICE_INTERFACE
> + tristate 'IPMB Interface handler'
> + depends on I2C_SLAVE
> + help
> + Provides a driver for a device (Satellite MC) to
> + receive requests and send responses back to the BMC via
> + the IPMB interface. This module requires I2C support.
> +
> config IPMI_POWERNV
> depends on PPC_POWERNV
> tristate 'POWERNV (OPAL firmware) IPMI interface'
> diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
> index 3f06b20..0822adc 100644
> --- a/drivers/char/ipmi/Makefile
> +++ b/drivers/char/ipmi/Makefile
> @@ -26,3 +26,4 @@ obj-$(CONFIG_IPMI_KCS_BMC) += kcs_bmc.o
> obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o
> obj-$(CONFIG_ASPEED_KCS_IPMI_BMC) += kcs_bmc_aspeed.o
> obj-$(CONFIG_NPCM7XX_KCS_IPMI_BMC) += kcs_bmc_npcm7xx.o
> +obj-$(CONFIG_IPMB_DEVICE_INTERFACE) += ipmb_dev_int.o
> diff --git a/drivers/char/ipmi/ipmb_dev_int.c
> b/drivers/char/ipmi/ipmb_dev_int.c
> new file mode 100644
> index 0000000..2895abf
> --- /dev/null
> +++ b/drivers/char/ipmi/ipmb_dev_int.c
> @@ -0,0 +1,364 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * IPMB driver to receive a request and send a response
> + *
> + * Copyright (C) 2019 Mellanox Techologies, Ltd.
> + *
> + * This was inspired by Brendan Higgins' ipmi-bmc-bt-i2c driver.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/poll.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/wait.h>
> +
> +#define MAX_MSG_LEN 128
> +#define IPMB_REQUEST_LEN_MIN 7
> +#define NETFN_RSP_BIT_MASK 0x4
> +#define REQUEST_QUEUE_MAX_LEN 256
> +
> +#define IPMB_MSG_LEN_IDX 0
> +#define RQ_SA_8BIT_IDX 1
> +#define NETFN_LUN_IDX 2
> +
> +#define GET_7BIT_ADDR(addr_8bit) (addr_8bit >> 1)
> +#define GET_8BIT_ADDR(addr_7bit) ((addr_7bit << 1) & 0xff)
> +
> +#define IPMB_MSG_PAYLOAD_LEN_MAX (MAX_MSG_LEN - IPMB_REQUEST_LEN_MIN
> +- 1)
> +
> +#define SMBUS_MSG_HEADER_LENGTH 2
> +#define SMBUS_MSG_IDX_OFFSET (SMBUS_MSG_HEADER_LENGTH + 1)
> +
> +struct ipmb_msg {
> + u8 len;
> + u8 rs_sa;
> + u8 netfn_rs_lun;
> + u8 checksum1;
> + u8 rq_sa;
> + u8 rq_seq_rq_lun;
> + u8 cmd;
> + u8 payload[IPMB_MSG_PAYLOAD_LEN_MAX];
> + /* checksum2 is included in payload */ } __packed;
> +
> +struct ipmb_request_elem {
> + struct list_head list;
> + struct ipmb_msg request;
> +};
> +
> +struct ipmb_dev {
> + struct i2c_client *client;
> + struct miscdevice miscdev;
> + struct ipmb_msg request;
> + struct list_head request_queue;
> + atomic_t request_queue_len;
> + size_t msg_idx;
> + spinlock_t lock;
> + wait_queue_head_t wait_queue;
> + struct mutex file_mutex;
> +};
> +
> +static inline struct ipmb_dev *to_ipmb_dev(struct file *file) {
> + return container_of(file->private_data, struct ipmb_dev, miscdev); }
> +
> +static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count,
> + loff_t *ppos)
> +{
> + struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
> + struct ipmb_request_elem *queue_elem;
> + struct ipmb_msg msg;
> + ssize_t ret;
> +
> + memset(&msg, 0, sizeof(msg));
> +
> + spin_lock_irq(&ipmb_dev->lock);
> +
> + while (list_empty(&ipmb_dev->request_queue)) {
> + spin_unlock_irq(&ipmb_dev->lock);
> +
> + if (file->f_flags & O_NONBLOCK)
> + return -EAGAIN;
> +
> + ret = wait_event_interruptible(ipmb_dev->wait_queue,
> + !list_empty(&ipmb_dev->request_queue));
> + if (ret)
> + return ret;
> +
> + spin_lock_irq(&ipmb_dev->lock);
> + }
> +
> + queue_elem = list_first_entry(&ipmb_dev->request_queue,
> + struct ipmb_request_elem, list);
> + memcpy(&msg, &queue_elem->request, sizeof(msg));
> + list_del(&queue_elem->list);
> + kfree(queue_elem);
> + atomic_dec(&ipmb_dev->request_queue_len);
> +
> + spin_unlock_irq(&ipmb_dev->lock);
> +
> + count = min_t(size_t, count, msg.len + 1);
> + if (copy_to_user(buf, &msg, count))
> + ret = -EFAULT;
> +
> + return ret < 0 ? ret : count;
> +}
> +
> +static ssize_t ipmb_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
> + u8 rq_sa, netf_rq_lun, msg_len;
> + struct i2c_client rq_client;
> + u8 msg[MAX_MSG_LEN];
> + ssize_t ret;
> +
> + if (count > sizeof(msg))
> + return -EINVAL;
> +
> + if (copy_from_user(&msg, buf, count))
> + return -EFAULT;
> +
> + if (count < msg[0])
> + return -EINVAL;
> +
> + rq_sa = GET_7BIT_ADDR(msg[RQ_SA_8BIT_IDX]);
> + netf_rq_lun = msg[NETFN_LUN_IDX];
> +
> + if (!(netf_rq_lun & NETFN_RSP_BIT_MASK))
> + return -EINVAL;
> +
> + /*
> + * subtract rq_sa and netf_rq_lun from the length of the msg passed to
> + * i2c_smbus_write_block_data_local
> + */
> + msg_len = msg[IPMB_MSG_LEN_IDX] - SMBUS_MSG_HEADER_LENGTH;
> +
> + strcpy(rq_client.name, "ipmb_requester");
> + rq_client.adapter = ipmb_dev->client->adapter;
> + rq_client.flags = ipmb_dev->client->flags;
> + rq_client.addr = rq_sa;
> +
> + ret = i2c_smbus_write_block_data(&rq_client, netf_rq_lun, msg_len,
> + msg + SMBUS_MSG_IDX_OFFSET);
> +
> + return ret ? : count;
> +}
> +
> +static unsigned int ipmb_poll(struct file *file, poll_table *wait) {
> + struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
> + unsigned int mask = POLLOUT;
> +
> + mutex_lock(&ipmb_dev->file_mutex);
> + poll_wait(file, &ipmb_dev->wait_queue, wait);
> +
> + if (atomic_read(&ipmb_dev->request_queue_len))
> + mask |= POLLIN;
> + mutex_unlock(&ipmb_dev->file_mutex);
> +
> + return mask;
> +}
> +
> +static const struct file_operations ipmb_fops = {
> + .owner = THIS_MODULE,
> + .read = ipmb_read,
> + .write = ipmb_write,
> + .poll = ipmb_poll,
> +};
> +
> +/* Called with ipmb_dev->lock held. */ static void
> +ipmb_handle_request(struct ipmb_dev *ipmb_dev) {
> + struct ipmb_request_elem *queue_elem;
> +
> + if (atomic_read(&ipmb_dev->request_queue_len) >=
> + REQUEST_QUEUE_MAX_LEN)
> + return;
> +
> + queue_elem = kmalloc(sizeof(*queue_elem), GFP_ATOMIC);
> + if (!queue_elem)
> + return;
> +
> + memcpy(&queue_elem->request, &ipmb_dev->request,
> + sizeof(struct ipmb_msg));
> + list_add(&queue_elem->list, &ipmb_dev->request_queue);
> + atomic_inc(&ipmb_dev->request_queue_len);
> + wake_up_all(&ipmb_dev->wait_queue);
> +}
> +
> +static u8 ipmb_verify_checksum1(struct ipmb_dev *ipmb_dev, u8 rs_sa)
> +{
> + /* The 8 lsb of the sum is 0 when the checksum is valid */
> + return (rs_sa + ipmb_dev->request.netfn_rs_lun +
> + ipmb_dev->request.checksum1);
> +}
> +
> +static bool is_ipmb_request(struct ipmb_dev *ipmb_dev, u8 rs_sa) {
> + if (ipmb_dev->msg_idx >= IPMB_REQUEST_LEN_MIN) {
> + if (ipmb_verify_checksum1(ipmb_dev, rs_sa))
> + return false;
> +
> + /*
> + * Check whether this is an IPMB request or
> + * response.
> + * The 6 MSB of netfn_rs_lun are dedicated to the netfn
> + * while the remaining bits are dedicated to the lun.
> + * If the LSB of the netfn is cleared, it is associated
> + * with an IPMB request.
> + * If the LSB of the netfn is set, it is associated with
> + * an IPMB response.
> + */
> + if (!(ipmb_dev->request.netfn_rs_lun & NETFN_RSP_BIT_MASK))
> + return true;
> + }
> + return false;
> +}
> +
> +/*
> + * The IPMB protocol only supports I2C Writes so there is no need
> + * to support I2C_SLAVE_READ* events.
> + * This i2c callback function only monitors IPMB request messages
> + * and adds them in a queue, so that they can be handled by
> + * receive_ipmb_request.
> + */
> +static int ipmb_slave_cb(struct i2c_client *client,
> + enum i2c_slave_event event, u8 *val) {
> + struct ipmb_dev *ipmb_dev = i2c_get_clientdata(client);
> + u8 *buf = (u8 *)&ipmb_dev->request;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ipmb_dev->lock, flags);
> + switch (event) {
> + case I2C_SLAVE_WRITE_REQUESTED:
> + memset(&ipmb_dev->request, 0, sizeof(ipmb_dev->request));
> + ipmb_dev->msg_idx = 0;
> +
> + /*
> + * At index 0, ipmb_msg stores the length of msg,
> + * skip it for now.
> + * The len will be populated once the whole
> + * buf is populated.
> + *
> + * The I2C bus driver's responsibility is to pass the
> + * data bytes to the backend driver; it does not
> + * forward the i2c slave address.
> + * Since the first byte in the IPMB message is the
> + * address of the responder, it is the responsibility
> + * of the IPMB driver to format the message properly.
> + * So this driver prepends the address of the responder
> + * to the received i2c data before the request message
> + * is handled in userland.
> + */
> + buf[++ipmb_dev->msg_idx] = GET_8BIT_ADDR(client->addr);
> + break;
> +
> + case I2C_SLAVE_WRITE_RECEIVED:
> + if (ipmb_dev->msg_idx >= sizeof(struct ipmb_msg))
> + break;
> +
> + buf[++ipmb_dev->msg_idx] = *val;
> + break;
> +
> + case I2C_SLAVE_STOP:
> + ipmb_dev->request.len = ipmb_dev->msg_idx;
> +
> + if (is_ipmb_request(ipmb_dev, GET_8BIT_ADDR(client->addr)))
> + ipmb_handle_request(ipmb_dev);
> + break;
> +
> + default:
> + break;
> + }
> + spin_unlock_irqrestore(&ipmb_dev->lock, flags);
> +
> + return 0;
> +}
> +
> +static int ipmb_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct ipmb_dev *ipmb_dev;
> + int ret;
> +
> + ipmb_dev = devm_kzalloc(&client->dev, sizeof(*ipmb_dev),
> + GFP_KERNEL);
> + if (!ipmb_dev)
> + return -ENOMEM;
> +
> + spin_lock_init(&ipmb_dev->lock);
> + init_waitqueue_head(&ipmb_dev->wait_queue);
> + atomic_set(&ipmb_dev->request_queue_len, 0);
> + INIT_LIST_HEAD(&ipmb_dev->request_queue);
> +
> + mutex_init(&ipmb_dev->file_mutex);
> +
> + ipmb_dev->miscdev.minor = MISC_DYNAMIC_MINOR;
> +
> + ipmb_dev->miscdev.name = devm_kasprintf(&client->dev, GFP_KERNEL,
> + "%s%d", "ipmb-",
> + client->adapter->nr);
> + ipmb_dev->miscdev.fops = &ipmb_fops;
> + ipmb_dev->miscdev.parent = &client->dev;
> + ret = misc_register(&ipmb_dev->miscdev);
> + if (ret)
> + return ret;
> +
> + ipmb_dev->client = client;
> + i2c_set_clientdata(client, ipmb_dev);
> + ret = i2c_slave_register(client, ipmb_slave_cb);
> + if (ret) {
> + misc_deregister(&ipmb_dev->miscdev);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ipmb_remove(struct i2c_client *client) {
> + struct ipmb_dev *ipmb_dev = i2c_get_clientdata(client);
> +
> + i2c_slave_unregister(client);
> + misc_deregister(&ipmb_dev->miscdev);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id ipmb_id[] = {
> + { "ipmb-dev", 0 },
> + {},
> +};
> +MODULE_DEVICE_TABLE(i2c, ipmb_id);
> +
> +static const struct acpi_device_id acpi_ipmb_id[] = {
> + { "IPMB0001", 0 },
> + {},
> +};
> +MODULE_DEVICE_TABLE(acpi, acpi_ipmb_id);
> +
> +static struct i2c_driver ipmb_driver = {
> + .driver = {
> + .owner = THIS_MODULE,
> + .name = "ipmb-dev",
> + .acpi_match_table = ACPI_PTR(acpi_ipmb_id),
> + },
> + .probe = ipmb_probe,
> + .remove = ipmb_remove,
> + .id_table = ipmb_id,
> +};
> +module_i2c_driver(ipmb_driver);
> +
> +MODULE_AUTHOR("Mellanox Technologies"); MODULE_DESCRIPTION("IPMB
> +driver"); MODULE_LICENSE("GPL v2");
> --
> 2.1.2
>

2019-06-10 21:56:05

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] Add support for IPMB driver

On 6/10/19 11:57 AM, Asmaa Mnebhi wrote:
> Support receiving IPMB requests on a Satellite MC from the BMC.
> Once a response is ready, this driver will send back a response
> to the BMC via the IPMB channel.
>
> Signed-off-by: Asmaa Mnebhi <[email protected]>
> Acked-by: [email protected]
> ---
> Documentation/IPMB.txt | 103 +++++++++++
> drivers/char/ipmi/Kconfig | 8 +
> drivers/char/ipmi/Makefile | 1 +
> drivers/char/ipmi/ipmb_dev_int.c | 364 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 476 insertions(+)
> create mode 100644 Documentation/IPMB.txt
> create mode 100644 drivers/char/ipmi/ipmb_dev_int.c
>
> diff --git a/Documentation/IPMB.txt b/Documentation/IPMB.txt
> new file mode 100644
> index 0000000..7160d53
> --- /dev/null
> +++ b/Documentation/IPMB.txt
> @@ -0,0 +1,103 @@

[...]

> +Instantiate the device
> +----------------------
> +
> +After loading the driver, you can instantiate the device as
> +described in 'Documentation/i2c/instantiating-devices'.
> +If you have multiple BMCs, each connected to your Satellite MC via
> +a different I2C bus, you can instantiate a device for each of
> +those BMCs.
> +The name of the instantiated device contains the I2C bus number
> +associated with it as follows:
> +
> +BMC1 ------ IPMB/I2C bus 1 ---------| /dev/ipmb-1
> + Satellite MC
> +BMC1 ------ IPMB/I2C bus 2 ---------| /dev/ipmb-2
> +
> +For instance, you can instantiate the ipmb-dev-int device from
> +user space at the 7 bit address 0x10 on bus 2:
> +
> + # echo ipmb-dev 0x1010 > /sys/bus/i2c/devices/i2c-2/new_device

This is confusing.

Can you clarify how the 7-bit address is extracted from that hex number?
Is the 7-bit address repeated (duplicated)?
Is the 7-bit address the low order (low bits) 0x10 or the 0x10 at bits
15-8 (or 14-8)?

> +This will create the device file /dev/ipmb-2, which can be accessed
> +by the user space program. The device needs to be instantiated
> +before running the user space program.

thanks.
--
~Randy

2019-06-11 14:04:45

by Asmaa Mnebhi

[permalink] [raw]
Subject: RE: [PATCH v12 1/1] Add support for IPMB driver

Hello Randy,

Please refer to the slave-interface linux documentation:
https://www.kernel.org/doc/Documentation/i2c/slave-interface

Thanks.
Asmaa

-----Original Message-----
From: Randy Dunlap <[email protected]>
Sent: Monday, June 10, 2019 5:56 PM
To: Asmaa Mnebhi <[email protected]>; [email protected]; [email protected]; Vadim Pasternak <[email protected]>; Michael Shych <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: [PATCH v12 1/1] Add support for IPMB driver

On 6/10/19 11:57 AM, Asmaa Mnebhi wrote:
> Support receiving IPMB requests on a Satellite MC from the BMC.
> Once a response is ready, this driver will send back a response to the
> BMC via the IPMB channel.
>
> Signed-off-by: Asmaa Mnebhi <[email protected]>
> Acked-by: [email protected]
> ---
> Documentation/IPMB.txt | 103 +++++++++++
> drivers/char/ipmi/Kconfig | 8 +
> drivers/char/ipmi/Makefile | 1 +
> drivers/char/ipmi/ipmb_dev_int.c | 364
> +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 476 insertions(+)
> create mode 100644 Documentation/IPMB.txt create mode 100644
> drivers/char/ipmi/ipmb_dev_int.c
>
> diff --git a/Documentation/IPMB.txt b/Documentation/IPMB.txt new file
> mode 100644 index 0000000..7160d53
> --- /dev/null
> +++ b/Documentation/IPMB.txt
> @@ -0,0 +1,103 @@

[...]

> +Instantiate the device
> +----------------------
> +
> +After loading the driver, you can instantiate the device as described
> +in 'Documentation/i2c/instantiating-devices'.
> +If you have multiple BMCs, each connected to your Satellite MC via a
> +different I2C bus, you can instantiate a device for each of those
> +BMCs.
> +The name of the instantiated device contains the I2C bus number
> +associated with it as follows:
> +
> +BMC1 ------ IPMB/I2C bus 1 ---------| /dev/ipmb-1
> + Satellite MC
> +BMC1 ------ IPMB/I2C bus 2 ---------| /dev/ipmb-2
> +
> +For instance, you can instantiate the ipmb-dev-int device from user
> +space at the 7 bit address 0x10 on bus 2:
> +
> + # echo ipmb-dev 0x1010 > /sys/bus/i2c/devices/i2c-2/new_device

This is confusing.

Can you clarify how the 7-bit address is extracted from that hex number?
Is the 7-bit address repeated (duplicated)?
Is the 7-bit address the low order (low bits) 0x10 or the 0x10 at bits
15-8 (or 14-8)?

> +This will create the device file /dev/ipmb-2, which can be accessed
> +by the user space program. The device needs to be instantiated before
> +running the user space program.

thanks.
--
~Randy

2019-06-11 16:47:37

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v12 1/1] Add support for IPMB driver

On 6/11/19 6:28 AM, Asmaa Mnebhi wrote:
> Hello Randy,
>
> Please refer to the slave-interface linux documentation:
> https://www.kernel.org/doc/Documentation/i2c/slave-interface

OK, thanks.

> Thanks.
> Asmaa
>
> -----Original Message-----
> From: Randy Dunlap <[email protected]>
> Sent: Monday, June 10, 2019 5:56 PM
> To: Asmaa Mnebhi <[email protected]>; [email protected]; [email protected]; Vadim Pasternak <[email protected]>; Michael Shych <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH v12 1/1] Add support for IPMB driver
>
> On 6/10/19 11:57 AM, Asmaa Mnebhi wrote:
>> Support receiving IPMB requests on a Satellite MC from the BMC.
>> Once a response is ready, this driver will send back a response to the
>> BMC via the IPMB channel.
>>
>> Signed-off-by: Asmaa Mnebhi <[email protected]>
>> Acked-by: [email protected]
>> ---
>> Documentation/IPMB.txt | 103 +++++++++++
>> drivers/char/ipmi/Kconfig | 8 +
>> drivers/char/ipmi/Makefile | 1 +
>> drivers/char/ipmi/ipmb_dev_int.c | 364
>> +++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 476 insertions(+)
>> create mode 100644 Documentation/IPMB.txt create mode 100644
>> drivers/char/ipmi/ipmb_dev_int.c
>>
>> diff --git a/Documentation/IPMB.txt b/Documentation/IPMB.txt new file
>> mode 100644 index 0000000..7160d53
>> --- /dev/null
>> +++ b/Documentation/IPMB.txt
>> @@ -0,0 +1,103 @@
>
> [...]
>
>> +Instantiate the device
>> +----------------------
>> +
>> +After loading the driver, you can instantiate the device as described
>> +in 'Documentation/i2c/instantiating-devices'.
>> +If you have multiple BMCs, each connected to your Satellite MC via a
>> +different I2C bus, you can instantiate a device for each of those
>> +BMCs.
>> +The name of the instantiated device contains the I2C bus number
>> +associated with it as follows:
>> +
>> +BMC1 ------ IPMB/I2C bus 1 ---------| /dev/ipmb-1
>> + Satellite MC
>> +BMC1 ------ IPMB/I2C bus 2 ---------| /dev/ipmb-2
>> +
>> +For instance, you can instantiate the ipmb-dev-int device from user
>> +space at the 7 bit address 0x10 on bus 2:
>> +
>> + # echo ipmb-dev 0x1010 > /sys/bus/i2c/devices/i2c-2/new_device
>
> This is confusing.
>
> Can you clarify how the 7-bit address is extracted from that hex number?
> Is the 7-bit address repeated (duplicated)?
> Is the 7-bit address the low order (low bits) 0x10 or the 0x10 at bits
> 15-8 (or 14-8)?
>
>> +This will create the device file /dev/ipmb-2, which can be accessed
>> +by the user space program. The device needs to be instantiated before
>> +running the user space program.
>
> thanks.
> --
> ~Randy
>


--
~Randy