Add support for IPMB driver
Subsystems (2)
cover 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.
Asmaa Mnebhi (1):
Add support for IPMB driver
drivers/char/ipmi/Kconfig | 8 + drivers/char/ipmi/Makefile | 1 + drivers/char/ipmi/ipmb_dev_int.c | 418 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 427 insertions(+) create mode 100644 drivers/char/ipmi/ipmb_dev_int.c
Signature · 1 line
2.1.2
PATCH 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.
--- drivers/char/ipmi/Kconfig | 8 + drivers/char/ipmi/Makefile | 1 + drivers/char/ipmi/ipmb_dev_int.c | 418 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 427 insertions(+) create mode 100644 drivers/char/ipmi/ipmb_dev_int.c
drivers/char/ipmi/Kconfig +8 -0
@@ -74,6 +74,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' + select I2C + 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'
drivers/char/ipmi/Makefile +1 -0
@@ -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
drivers/char/ipmi/ipmb_dev_int.c new file +418 -0
418 lines not on this page · raw message
Signature · 1 line
2.1.2
Quoted 3 lines · Asmaa Mnebhi
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.
I haven't done a detailed analysis of this, but from a cursory look it looks structurally good. I made a few small comments inline.
But there is really very little here that is IPMB-specific. You could push the few IPMB-specific formatting things up to userland, I think.
I am wondering if you could make this into a generic I2C slave interface. I'm adding the I2C mailing list to the CC and for their review.
For the I2C mailing list...
IPMB is a protocol that lets IPMI devices talk between themselves, so it is master/slave on all device on the IPMB. It's basically designed so that modular parts of the system (like a power supply) can have their own IPMI management controller that handles things for that device, and a management system can discover these and use them through the main IPMI management controller. It's used like a normal packet-oriented bus, with only writes, no reads.
It seems to me that you could create a slave-specific I2C device that only received messages and took responses to those messages. You could use the normal I2C dev interface to to the sends and open a separate I2C slave dev interface to do the receives.
-corey
Quoted 170 lines
Signed-off-by Asmaa Mnebhi <[email protected]>--- drivers/char/ipmi/Kconfig | 8 + drivers/char/ipmi/Makefile | 1 + drivers/char/ipmi/ipmb_dev_int.c | 418 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 427 insertions(+) create mode 100644 drivers/char/ipmi/ipmb_dev_int.cdrivers/char/ipmi/Kconfig +8
-0@@ -74,6 +74,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' + select I2C + 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'drivers/char/ipmi/Makefile +1
-0@@ -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.odrivers/char/ipmi/ipmb_dev_int.c new file +127
-0@@ -0,0 +1,418 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* +* Mellanox IPMB driver to receive a request and send a response +* +* Copyright (C) 2018 Mellanox Techologies, Ltd. +* +* This was inspired by Brendan Higgins' ipmi-bmc-bt-i2c driver. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License +* version 2 as published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*/ + +#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 PFX "IPMB DEV INT: " + +#define DEVICE_NAME "ipmb-dev" + +#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 IPMB_MSG_PAYLOAD_LEN_MAX \ + (MAX_MSG_LEN - IPMB_REQUEST_LEN_MIN - 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; + +static u32 ipmb_msg_len (struct ipmb_msg *ipmb_msg) +{ +return ipmb_msg->len +1 ; +} + +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; +struct ipmb_msg response; +size_t msg_idx; +spinlock_t lock; +wait_queue_head_t wait_queue; +struct mutex file_mutex; +}; + +static int receive_ipmb_request (struct ipmb_dev *ipmb_dev_p, +bool non_blocking, +struct ipmb_msg *ipmb_request) +{ +struct ipmb_request_elem *queue_elem; +unsigned long flags; +int res; + +if (!non_blocking) { + +try_again: + res =wait_event_interruptible (ipmb_dev_p->wait_queue, +atomic_read (&ipmb_dev_p->request_queue_len)); +if (res) +return res; + } + +spin_lock_irqsave (&ipmb_dev_p->lock, flags); + +if (!atomic_read (&ipmb_dev_p->request_queue_len)) { +spin_unlock_irqrestore (&ipmb_dev_p->lock, flags); +if (non_blocking) +return -EAGAIN ; + goto try_again; + } + +if (list_empty (&ipmb_dev_p->request_queue)) { +pr_err (PFX "request_queue is empty\n" ); +return -EIO ; + } + + queue_elem =list_first_entry (&ipmb_dev_p->request_queue, + struct ipmb_request_elem, list); +memcpy (ipmb_request, &queue_elem->request,sizeof (*ipmb_request)); +list_del (&queue_elem->list); +kfree (queue_elem); +atomic_dec (&ipmb_dev_p->request_queue_len); + +spin_unlock_irqrestore (&ipmb_dev_p->lock, flags); + +return 0 ; +} + +static inline struct ipmb_dev *to_ipmb_dev (struct file *file) +{ + return container_of(file->private_data, struct ipmb_dev, miscdev); +}
drivers/char/ipmi/ipmb_dev_int.c line 137
Quoted 10 lines
drivers/char/ipmi/ipmb_dev_int.c new file +10
-0+ +static ssize_t ipmb_read (struct file *file,char __user *buf,size_t count, +loff_t *ppos) +{ +struct ipmb_dev *ipmb_dev_p =to_ipmb_dev (file); +struct ipmb_msg msg; +ssize_t ret; + +if (!ipmb_dev_p) +return -ENOMEM ;
I don't think this can happen. Same for other things like this.
Quoted 50 lines
drivers/char/ipmi/ipmb_dev_int.c new file +50
-0+ +memset (&msg,0 ,sizeof (msg)); + +mutex_lock (&ipmb_dev_p->file_mutex); + ret =receive_ipmb_request (ipmb_dev_p, file->f_flags &O_NONBLOCK , + &msg); +if (ret <0 ) + goto out; + count =min_t (size_t, count,ipmb_msg_len (&msg)); +if (copy_to_user (buf, &msg, count)) { + ret = -EFAULT ; + goto out; + } + +out: +mutex_unlock (&ipmb_dev_p->file_mutex); +if (ret <0 ) +return ret; +else +return count; +} + +static s32 i2c_smbus_write_block_data_local (struct i2c_client *client, +u8 command,u8 length, +u16 requester_i2c_addr, +const char *msg) +{ +union i2c_smbus_data data; +int ret; + +if (length >I2C_SMBUS_BLOCK_MAX ) + length =I2C_SMBUS_BLOCK_MAX ; + + data.block[0 ] = length; +memcpy (&data.block[1 ], msg, length); + + ret =i2c_smbus_xfer (client->adapter, requester_i2c_addr, + client->flags, +I2C_SMBUS_WRITE , command, +I2C_SMBUS_BLOCK_DATA , &data); + +return ret; +} + +static ssize_t ipmb_write (struct file *file,const char __user *buf, +size_t count,loff_t *ppos) +{ +struct ipmb_dev *ipmb_dev_p =to_ipmb_dev (file); +u8 msg[MAX_MSG_LEN ]; +ssize_t ret;
drivers/char/ipmi/ipmb_dev_int.c line 197
Quoted 10 lines
drivers/char/ipmi/ipmb_dev_int.c new file +10
-0+u8 rq_sa, netf_rq_lun, msg_len; + +if (!ipmb_dev_p) +return -ENOMEM ; + +if (count >sizeof (msg)) +return -EINVAL ; + +if (copy_from_user (&msg, buf, count) || count < msg[0 ]) +return -EINVAL ;
copy_from_user() should return -EFAULT on an error.
Quoted 34 lines
drivers/char/ipmi/ipmb_dev_int.c new file +34
-0+ + rq_sa = (u16)(msg[RQ_SA_8BIT_IDX ] >>1 ); + netf_rq_lun = msg[NETFN_LUN_IDX ]; +/* +* 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 ] -2 ; + +mutex_lock (&ipmb_dev_p->file_mutex); + ret =i2c_smbus_write_block_data_local (ipmb_dev_p->client, + netf_rq_lun, msg_len, rq_sa, msg +3 ); +mutex_unlock (&ipmb_dev_p->file_mutex); + +if (ret) +return ret; +else +return count; +} + +static unsigned int ipmb_poll (struct file *file,poll_table *wait) +{ +struct ipmb_dev *ipmb_dev_p =to_ipmb_dev (file); +unsigned int mask =0 ; + +mutex_lock (&ipmb_dev_p->file_mutex); +poll_wait (file, &ipmb_dev_p->wait_queue, wait); + +if (atomic_read (&ipmb_dev_p->request_queue_len)) + mask |=POLLIN ; + mask |=POLLOUT ; +mutex_unlock (&ipmb_dev_p->file_mutex); +return mask; +}
drivers/char/ipmi/ipmb_dev_int.c line 241
Quoted 10 lines
drivers/char/ipmi/ipmb_dev_int.c new file +10
-0+ +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 int handle_request (struct ipmb_dev *ipmb_dev_p)
The return value from handle_request is ignored below, do you need to return it?
drivers/char/ipmi/ipmb_dev_int.c line 247
Quoted 6 lines
drivers/char/ipmi/ipmb_dev_int.c new file +6
-0+{ +struct ipmb_request_elem *queue_elem; + +if (atomic_read (&ipmb_dev_p->request_queue_len) >= +REQUEST_QUEUE_MAX_LEN ) +return -EFAULT ;
EFAULT is not what you want here. I'm not sure exactly what to do here, but you should probably just drop the data on the floor, maybe have a counter to count dropped packets.
Quoted 174 lines
drivers/char/ipmi/ipmb_dev_int.c new file +171
-0+ + queue_elem =kmalloc (sizeof (*queue_elem),GFP_KERNEL ); +if (!queue_elem) +return -ENOMEM ; + +memcpy (&queue_elem->request, &ipmb_dev_p->request, +sizeof (struct ipmb_msg )); +list_add (&queue_elem->list, &ipmb_dev_p->request_queue); +atomic_inc (&ipmb_dev_p->request_queue_len); +wake_up_all (&ipmb_dev_p->wait_queue); + +return 0 ; +} + +static u8 verify_checksum1 (struct ipmb_dev *ipmb_dev_p,u8 rs_sa) +{ +return (rs_sa + ipmb_dev_p->request.netfn_rs_lun + + ipmb_dev_p->request.checksum1); +} + +static bool is_ipmb_request (struct ipmb_dev *ipmb_dev_p,u8 rs_sa) +{ +if (ipmb_dev_p->msg_idx >=IPMB_REQUEST_LEN_MIN ) { +if (verify_checksum1 (ipmb_dev_p, 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_p->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_p =i2c_get_clientdata (client); +u8 *buf = (u8 *)&ipmb_dev_p->request; + +spin_lock (&ipmb_dev_p->lock); +switch (event) { +case I2C_SLAVE_WRITE_REQUESTED : +memset (&ipmb_dev_p->request,0 ,sizeof (ipmb_dev_p->request)); + ipmb_dev_p->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_p->msg_idx] = (u8)(client->addr <<1 ); +break ; + +case I2C_SLAVE_WRITE_RECEIVED : +if (ipmb_dev_p->msg_idx >=sizeof (struct ipmb_msg )) +break ; + + buf[++ipmb_dev_p->msg_idx] = *val; +break ; + +case I2C_SLAVE_STOP : + ipmb_dev_p->request.len = ipmb_dev_p->msg_idx; + +if (is_ipmb_request (ipmb_dev_p, (u8)(client->addr <<1 ))) +handle_request (ipmb_dev_p); +break ; + +default : +break ; + } +spin_unlock (&ipmb_dev_p->lock); + +return 0 ; +} + +static int ipmb_probe (struct i2c_client *client, +const struct i2c_device_id *id) +{ +struct ipmb_dev *ipmb_dev_p; +int ret; + + ipmb_dev_p =devm_kzalloc (&client->dev,sizeof (*ipmb_dev_p), +GFP_KERNEL ); +if (!ipmb_dev_p) +return -ENOMEM ; + +spin_lock_init (&ipmb_dev_p->lock); +init_waitqueue_head (&ipmb_dev_p->wait_queue); +atomic_set (&ipmb_dev_p->request_queue_len,0 ); +INIT_LIST_HEAD (&ipmb_dev_p->request_queue); + +mutex_init (&ipmb_dev_p->file_mutex); + + ipmb_dev_p->miscdev.minor =MISC_DYNAMIC_MINOR ; + ipmb_dev_p->miscdev.name =DEVICE_NAME ; + ipmb_dev_p->miscdev.fops = &ipmb_fops; + ipmb_dev_p->miscdev.parent = &client->dev; + ret =misc_register (&ipmb_dev_p->miscdev); +if (ret) +return ret; + + ipmb_dev_p->client = client; +i2c_set_clientdata (client, ipmb_dev_p); + ret =i2c_slave_register (client, ipmb_slave_cb); +if (ret) { +misc_deregister (&ipmb_dev_p->miscdev); +return ret; + } + +return 0 ; +} + +static int ipmb_remove (struct i2c_client *client) +{ +struct ipmb_dev *ipmb_dev_p =i2c_get_clientdata (client); + +if (!ipmb_dev_p) +return 0 ; + +i2c_slave_unregister (client); +misc_deregister (&ipmb_dev_p->miscdev); + +return 0 ; +} + +static const struct i2c_device_id ipmb_id[] = { + {"ipmb-dev" ,0 }, + {}, +}; +MODULE_DEVICE_TABLE (i2c, ipmb_id); + +static struct i2c_driver ipmb_driver = { + .driver = { + .name ="ipmb-dev" , + }, + .probe = ipmb_probe, + .remove = ipmb_remove, + .id_table = ipmb_id, +}; +module_i2c_driver (ipmb_driver); + +MODULE_AUTHOR ("Mellanox Technologies" ); +MODULE_DESCRIPTION ("Mellanox BlueField IPMB driver" ); +MODULE_LICENSE ("GPL" );Signature · 1 line
2.1.2
-----Original Message-----
From: Corey Minyard <[email protected]> On Behalf Of Corey Minyard
Sent: Tuesday, April 16, 2019 12:16 PM
To: Asmaa Mnebhi <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: [PATCH v1 1/1] Add support for IPMB driver
Quoted 3 lines · Asmaa Mnebhi
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.
I haven't done a detailed analysis of this, but from a cursory look it looks structurally good. I made a few small comments inline.
But there is really very little here that is IPMB-specific.
You could push the few IPMB-specific formatting things up to userland, I think.
I am wondering if you could make this into a generic I2C slave interface. I'm adding the I2C mailing list to the CC and for their review.
For the I2C mailing list...
IPMB is a protocol that lets IPMI devices talk between themselves, so it is master/slave on all device on the IPMB. It's basically designed so that modular parts of the system (like a power supply) can have their own IPMI management controller that handles things for that device, and a management system can discover these and use them through the main IPMI management controller. It's used like a normal packet-oriented bus, with only writes, no reads.
It seems to me that you could create a slave-specific I2C device that only received messages and took responses to those messages. You could use the normal I2C dev interface to to the sends and open a separate I2C slave dev interface to do the receives.
-corey
Quoted 168 lines
Signed-off-by Asmaa Mnebhi <[email protected]>--- drivers/char/ipmi/Kconfig | 8 + drivers/char/ipmi/Makefile | 1 + drivers/char/ipmi/ipmb_dev_int.c | 418 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 427 insertions(+) create mode 100644 drivers/char/ipmi/ipmb_dev_int.cdrivers/char/ipmi/Kconfig +8
-0@@ -74,6 +74,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' + select I2C + 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'drivers/char/ipmi/Makefile +1
-0@@ -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.odrivers/char/ipmi/ipmb_dev_int.c new file +123
-0@@ -0,0 +1,418 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* +* Mellanox IPMB driver to receive a request and send a response +* +* Copyright (C) 2018 Mellanox Techologies, Ltd. +* +* This was inspired by Brendan Higgins' ipmi-bmc-bt-i2c driver. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License +* version 2 as published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*/ + +#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 PFX "IPMB DEV INT: " + +#define DEVICE_NAME "ipmb-dev" + +#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 IPMB_MSG_PAYLOAD_LEN_MAX \ + (MAX_MSG_LEN - IPMB_REQUEST_LEN_MIN - 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; + +static u32 ipmb_msg_len (struct ipmb_msg *ipmb_msg) { +return ipmb_msg->len +1 ; +} + +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; +struct ipmb_msg response; +size_t msg_idx; +spinlock_t lock; +wait_queue_head_t wait_queue; +struct mutex file_mutex; +}; + +static int receive_ipmb_request (struct ipmb_dev *ipmb_dev_p, +bool non_blocking, +struct ipmb_msg *ipmb_request) +{ +struct ipmb_request_elem *queue_elem; +unsigned long flags; +int res; + +if (!non_blocking) { + +try_again: + res =wait_event_interruptible (ipmb_dev_p->wait_queue, +atomic_read (&ipmb_dev_p->request_queue_len)); +if (res) +return res; + } + +spin_lock_irqsave (&ipmb_dev_p->lock, flags); + +if (!atomic_read (&ipmb_dev_p->request_queue_len)) { +spin_unlock_irqrestore (&ipmb_dev_p->lock, flags); +if (non_blocking) +return -EAGAIN ; + goto try_again; + } + +if (list_empty (&ipmb_dev_p->request_queue)) { +pr_err (PFX "request_queue is empty\n" ); +return -EIO ; + } + + queue_elem =list_first_entry (&ipmb_dev_p->request_queue, + struct ipmb_request_elem, list); +memcpy (ipmb_request, &queue_elem->request,sizeof (*ipmb_request)); +list_del (&queue_elem->list); +kfree (queue_elem); +atomic_dec (&ipmb_dev_p->request_queue_len); + +spin_unlock_irqrestore (&ipmb_dev_p->lock, flags); + +return 0 ; +} + +static inline struct ipmb_dev *to_ipmb_dev (struct file *file) { + return container_of(file->private_data, struct ipmb_dev, miscdev); }
drivers/char/ipmi/ipmb_dev_int.c line 133
Quoted 10 lines
drivers/char/ipmi/ipmb_dev_int.c new file +10
-0+ +static ssize_t ipmb_read (struct file *file,char __user *buf,size_t count, +loff_t *ppos) +{ +struct ipmb_dev *ipmb_dev_p =to_ipmb_dev (file); +struct ipmb_msg msg; +ssize_t ret; + +if (!ipmb_dev_p) +return -ENOMEM ;
I don't think this can happen. Same for other things like this.
Quoted 50 lines
drivers/char/ipmi/ipmb_dev_int.c new file +50
-0+ +memset (&msg,0 ,sizeof (msg)); + +mutex_lock (&ipmb_dev_p->file_mutex); + ret =receive_ipmb_request (ipmb_dev_p, file->f_flags &O_NONBLOCK , + &msg); +if (ret <0 ) + goto out; + count =min_t (size_t, count,ipmb_msg_len (&msg)); +if (copy_to_user (buf, &msg, count)) { + ret = -EFAULT ; + goto out; + } + +out: +mutex_unlock (&ipmb_dev_p->file_mutex); +if (ret <0 ) +return ret; +else +return count; +} + +static s32 i2c_smbus_write_block_data_local (struct i2c_client *client, +u8 command,u8 length, +u16 requester_i2c_addr, +const char *msg) +{ +union i2c_smbus_data data; +int ret; + +if (length >I2C_SMBUS_BLOCK_MAX ) + length =I2C_SMBUS_BLOCK_MAX ; + + data.block[0 ] = length; +memcpy (&data.block[1 ], msg, length); + + ret =i2c_smbus_xfer (client->adapter, requester_i2c_addr, + client->flags, +I2C_SMBUS_WRITE , command, +I2C_SMBUS_BLOCK_DATA , &data); + +return ret; +} + +static ssize_t ipmb_write (struct file *file,const char __user *buf, +size_t count,loff_t *ppos) +{ +struct ipmb_dev *ipmb_dev_p =to_ipmb_dev (file); +u8 msg[MAX_MSG_LEN ]; +ssize_t ret;
drivers/char/ipmi/ipmb_dev_int.c line 197
Quoted 10 lines
drivers/char/ipmi/ipmb_dev_int.c new file +10
-0+u8 rq_sa, netf_rq_lun, msg_len; + +if (!ipmb_dev_p) +return -ENOMEM ; + +if (count >sizeof (msg)) +return -EINVAL ; + +if (copy_from_user (&msg, buf, count) || count < msg[0 ]) +return -EINVAL ;
copy_from_user() should return -EFAULT on an error.
Quoted 33 lines
drivers/char/ipmi/ipmb_dev_int.c new file +33
-0+ + rq_sa = (u16)(msg[RQ_SA_8BIT_IDX ] >>1 ); + netf_rq_lun = msg[NETFN_LUN_IDX ]; +/* +* 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 ] -2 ; + +mutex_lock (&ipmb_dev_p->file_mutex); + ret =i2c_smbus_write_block_data_local (ipmb_dev_p->client, + netf_rq_lun, msg_len, rq_sa, msg +3 ); +mutex_unlock (&ipmb_dev_p->file_mutex); + +if (ret) +return ret; +else +return count; +} + +static unsigned int ipmb_poll (struct file *file,poll_table *wait) { +struct ipmb_dev *ipmb_dev_p =to_ipmb_dev (file); +unsigned int mask =0 ; + +mutex_lock (&ipmb_dev_p->file_mutex); +poll_wait (file, &ipmb_dev_p->wait_queue, wait); + +if (atomic_read (&ipmb_dev_p->request_queue_len)) + mask |=POLLIN ; + mask |=POLLOUT ; +mutex_unlock (&ipmb_dev_p->file_mutex); +return mask; +}
drivers/char/ipmi/ipmb_dev_int.c
Quoted 10 lines
drivers/char/ipmi/ipmb_dev_int.c new file +10
-0+ +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 int +handle_request (struct ipmb_dev *ipmb_dev_p)
The return value from handle_request is ignored below, do you need to return it?
drivers/char/ipmi/ipmb_dev_int.c line 247
Quoted 6 lines
drivers/char/ipmi/ipmb_dev_int.c new file +6
-0+{ +struct ipmb_request_elem *queue_elem; + +if (atomic_read (&ipmb_dev_p->request_queue_len) >= +REQUEST_QUEUE_MAX_LEN ) +return -EFAULT ;
EFAULT is not what you want here. I'm not sure exactly what to do here, but you should probably just drop the data on the floor, maybe have a counter to count dropped packets.
Quoted 169 lines
drivers/char/ipmi/ipmb_dev_int.c new file +166
-1+ + queue_elem =kmalloc (sizeof (*queue_elem),GFP_KERNEL ); +if (!queue_elem) +return -ENOMEM ; + +memcpy (&queue_elem->request, &ipmb_dev_p->request, +sizeof (struct ipmb_msg )); +list_add (&queue_elem->list, &ipmb_dev_p->request_queue); +atomic_inc (&ipmb_dev_p->request_queue_len); +wake_up_all (&ipmb_dev_p->wait_queue); + +return 0 ; +} + +static u8 verify_checksum1 (struct ipmb_dev *ipmb_dev_p,u8 rs_sa) { +return (rs_sa + ipmb_dev_p->request.netfn_rs_lun + + ipmb_dev_p->request.checksum1); +} + +static bool is_ipmb_request (struct ipmb_dev *ipmb_dev_p,u8 rs_sa) { +if (ipmb_dev_p->msg_idx >=IPMB_REQUEST_LEN_MIN ) { +if (verify_checksum1 (ipmb_dev_p, 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_p->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_p =i2c_get_clientdata (client); +u8 *buf = (u8 *)&ipmb_dev_p->request; + +spin_lock (&ipmb_dev_p->lock); +switch (event) { +case I2C_SLAVE_WRITE_REQUESTED : +memset (&ipmb_dev_p->request,0 ,sizeof (ipmb_dev_p->request)); + ipmb_dev_p->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_p->msg_idx] = (u8)(client->addr <<1 ); +break ; + +case I2C_SLAVE_WRITE_RECEIVED : +if (ipmb_dev_p->msg_idx >=sizeof (struct ipmb_msg )) +break ; + + buf[++ipmb_dev_p->msg_idx] = *val; +break ; + +case I2C_SLAVE_STOP : + ipmb_dev_p->request.len = ipmb_dev_p->msg_idx; + +if (is_ipmb_request (ipmb_dev_p, (u8)(client->addr <<1 ))) +handle_request (ipmb_dev_p); +break ; + +default : +break ; + } +spin_unlock (&ipmb_dev_p->lock); + +return 0 ; +} + +static int ipmb_probe (struct i2c_client *client, +const struct i2c_device_id *id) +{ +struct ipmb_dev *ipmb_dev_p; +int ret; + + ipmb_dev_p =devm_kzalloc (&client->dev,sizeof (*ipmb_dev_p), +GFP_KERNEL ); +if (!ipmb_dev_p) +return -ENOMEM ; + +spin_lock_init (&ipmb_dev_p->lock); +init_waitqueue_head (&ipmb_dev_p->wait_queue); +atomic_set (&ipmb_dev_p->request_queue_len,0 ); +INIT_LIST_HEAD (&ipmb_dev_p->request_queue); + +mutex_init (&ipmb_dev_p->file_mutex); + + ipmb_dev_p->miscdev.minor =MISC_DYNAMIC_MINOR ; + ipmb_dev_p->miscdev.name =DEVICE_NAME ; + ipmb_dev_p->miscdev.fops = &ipmb_fops; + ipmb_dev_p->miscdev.parent = &client->dev; + ret =misc_register (&ipmb_dev_p->miscdev); +if (ret) +return ret; + + ipmb_dev_p->client = client; +i2c_set_clientdata (client, ipmb_dev_p); + ret =i2c_slave_register (client, ipmb_slave_cb); +if (ret) { +misc_deregister (&ipmb_dev_p->miscdev); +return ret; + } + +return 0 ; +} + +static int ipmb_remove (struct i2c_client *client) { +struct ipmb_dev *ipmb_dev_p =i2c_get_clientdata (client); + +if (!ipmb_dev_p) +return 0 ; + +i2c_slave_unregister (client); +misc_deregister (&ipmb_dev_p->miscdev); + +return 0 ; +} + +static const struct i2c_device_id ipmb_id[] = { + {"ipmb-dev" ,0 }, + {}, +}; +MODULE_DEVICE_TABLE (i2c, ipmb_id); + +static struct i2c_driver ipmb_driver = { + .driver = { + .name ="ipmb-dev" , + }, + .probe = ipmb_probe, + .remove = ipmb_remove, + .id_table = ipmb_id, +}; +module_i2c_driver (ipmb_driver); + +MODULE_AUTHOR("Mellanox Technologies"); MODULE_DESCRIPTION("Mellanox +BlueField IPMB driver"); MODULE_LICENSE("GPL"); --2.1.2