From ce7fa1c38d07102d4dc4627f757a3f2467069d86 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 4 Apr 2018 09:15:23 -0500 Subject: [PATCH 01/35] ipmi: Add a way to tune some timeouts By default the retry timeout is 1 second. Allow that to be modified, primarily for slow operations, like firmware writes. Also, the timeout was driven by a 1 second timer, so 1 second really meant between 0 and 1 second. Set the default to 2 seconds so it means between 1 and 2 seconds. Also allow the time the interface automatically stays in mainenance mode to be modified from it's default 30 seconds. Also consolidate some of the timeout and retry setup. Signed-off-by: Corey Minyard more --- drivers/char/ipmi/ipmi_msghandler.c | 80 +++++++++++++++++------------ 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 361148938801..add8130be517 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -116,17 +116,39 @@ MODULE_PARM_DESC(panic_op, "Sets if the IPMI driver will attempt to store panic static struct proc_dir_entry *proc_ipmi_root; #endif /* CONFIG_IPMI_PROC_INTERFACE */ -/* Remain in auto-maintenance mode for this amount of time (in ms). */ -#define IPMI_MAINTENANCE_MODE_TIMEOUT 30000 - #define MAX_EVENTS_IN_QUEUE 25 +/* Remain in auto-maintenance mode for this amount of time (in ms). */ +static unsigned long maintenance_mode_timeout_ms = 30000; +module_param(maintenance_mode_timeout_ms, ulong, 0644); +MODULE_PARM_DESC(maintenance_mode_timeout_ms, + "The time (milliseconds) after the last maintenance message that the connection stays in maintenance mode."); + /* * Don't let a message sit in a queue forever, always time it with at lest * the max message timer. This is in milliseconds. */ #define MAX_MSG_TIMEOUT 60000 +/* + * Timeout times below are in milliseconds, and are done off a 1 + * second timer. So setting the value to 1000 would mean anything + * between 0 and 1000ms. So really the only reasonable minimum + * setting it 2000ms, which is between 1 and 2 seconds. + */ + +/* The default timeout for message retries. */ +static unsigned long default_retry_ms = 2000; +module_param(default_retry_ms, ulong, 0644); +MODULE_PARM_DESC(default_retry_ms, + "The time (milliseconds) between retry sends"); + +/* The default maximum number of retries */ +static unsigned int default_max_retries = 4; +module_param(default_max_retries, uint, 0644); +MODULE_PARM_DESC(default_max_retries, + "The time (milliseconds) between retry sends in maintenance mode"); + /* Call every ~1000 ms. */ #define IPMI_TIMEOUT_TIME 1000 @@ -884,6 +906,11 @@ static int intf_next_seq(ipmi_smi_t intf, int rv = 0; unsigned int i; + if (timeout == 0) + timeout = default_retry_ms; + if (retries < 0) + retries = default_max_retries; + for (i = intf->curr_seq; (i+1)%IPMI_IPMB_NUM_SEQ != intf->curr_seq; i = (i+1)%IPMI_IPMB_NUM_SEQ) { if (!intf->seq_table[i].inuse) @@ -1636,6 +1663,14 @@ static void smi_send(ipmi_smi_t intf, const struct ipmi_smi_handlers *handlers, handlers->sender(intf->send_info, smi_msg); } +static bool is_maintenance_mode_cmd(struct kernel_ipmi_msg *msg) +{ + return (((msg->netfn == IPMI_NETFN_APP_REQUEST) + && ((msg->cmd == IPMI_COLD_RESET_CMD) + || (msg->cmd == IPMI_WARM_RESET_CMD))) + || (msg->netfn == IPMI_NETFN_FIRMWARE_REQUEST)); +} + /* * Separate from ipmi_request so that the user does not have to be * supplied in certain circumstances (mainly at panic time). If @@ -1728,13 +1763,10 @@ static int i_ipmi_request(ipmi_user_t user, goto out_err; } - if (((msg->netfn == IPMI_NETFN_APP_REQUEST) - && ((msg->cmd == IPMI_COLD_RESET_CMD) - || (msg->cmd == IPMI_WARM_RESET_CMD))) - || (msg->netfn == IPMI_NETFN_FIRMWARE_REQUEST)) { + if (is_maintenance_mode_cmd(msg)) { spin_lock_irqsave(&intf->maintenance_mode_lock, flags); intf->auto_maintenance_timeout - = IPMI_MAINTENANCE_MODE_TIMEOUT; + = maintenance_mode_timeout_ms; if (!intf->maintenance_mode && !intf->maintenance_mode_enable) { intf->maintenance_mode_enable = true; @@ -1779,27 +1811,17 @@ static int i_ipmi_request(ipmi_user_t user, goto out_err; } - if (retries < 0) { - if (addr->addr_type == IPMI_IPMB_BROADCAST_ADDR_TYPE) - retries = 0; /* Don't retry broadcasts. */ - else - retries = 4; - } if (addr->addr_type == IPMI_IPMB_BROADCAST_ADDR_TYPE) { - /* - * Broadcasts add a zero at the beginning of the - * message, but otherwise is the same as an IPMB - * address. - */ - addr->addr_type = IPMI_IPMB_ADDR_TYPE; - broadcast = 1; + /* + * Broadcasts add a zero at the beginning of the + * message, but otherwise is the same as an IPMB + * address. + */ + addr->addr_type = IPMI_IPMB_ADDR_TYPE; + broadcast = 1; + retries = 0; /* Don't retry broadcasts. */ } - - /* Default to 1 second retries. */ - if (retry_time_ms == 0) - retry_time_ms = 1000; - /* * 9 for the header and 1 for the checksum, plus * possibly one for the broadcast. @@ -1914,12 +1936,6 @@ static int i_ipmi_request(ipmi_user_t user, goto out_err; } - retries = 4; - - /* Default to 1 second retries. */ - if (retry_time_ms == 0) - retry_time_ms = 1000; - /* 11 for the header and 1 for the checksum. */ if ((msg->data_len + 12) > IPMI_MAX_MSG_LENGTH) { ipmi_inc_stat(intf, sent_invalid_commands); From 252e30c1e7d847c09d9480e4b17ba0485059f576 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 12:59:17 -0500 Subject: [PATCH 02/35] ipmi: Add a maintenance mode for IPMB messages If you send a command to another BMC that might take some extra time, increase the timeouts temporarily. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index add8130be517..dcfbf2e3c8c5 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -143,6 +143,12 @@ module_param(default_retry_ms, ulong, 0644); MODULE_PARM_DESC(default_retry_ms, "The time (milliseconds) between retry sends"); +/* The default timeout for maintenance mode message retries. */ +static unsigned long default_maintenance_retry_ms = 3000; +module_param(default_maintenance_retry_ms, ulong, 0644); +MODULE_PARM_DESC(default_maintenance_retry_ms, + "The time (milliseconds) between retry sends in maintenance mode"); + /* The default maximum number of retries */ static unsigned int default_max_retries = 4; module_param(default_max_retries, uint, 0644); @@ -524,6 +530,13 @@ struct ipmi_smi { int auto_maintenance_timeout; spinlock_t maintenance_mode_lock; /* Used in a timer... */ + /* + * If we are doing maintenance on something on IPMB, extend + * the timeout time to avoid timeouts writing firmware and + * such. + */ + int ipmb_maintenance_mode_timeout; + /* * A cheap hack, if this is non-null and a message to an * interface comes in with a NULL user, call this routine with @@ -1861,6 +1874,15 @@ static int i_ipmi_request(ipmi_user_t user, spin_lock_irqsave(&(intf->seq_lock), flags); + if (is_maintenance_mode_cmd(msg)) + intf->ipmb_maintenance_mode_timeout = + maintenance_mode_timeout_ms; + + if (intf->ipmb_maintenance_mode_timeout && + retry_time_ms == 0) + /* Different default in maintenance mode */ + retry_time_ms = default_maintenance_retry_ms; + /* * Create a sequence number with a 1 second * timeout and 4 retries. @@ -4710,6 +4732,12 @@ static unsigned int ipmi_timeout_handler(ipmi_smi_t intf, */ INIT_LIST_HEAD(&timeouts); spin_lock_irqsave(&intf->seq_lock, flags); + if (intf->ipmb_maintenance_mode_timeout) { + if (intf->ipmb_maintenance_mode_timeout <= timeout_period) + intf->ipmb_maintenance_mode_timeout = 0; + else + intf->ipmb_maintenance_mode_timeout -= timeout_period; + } for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) check_msg_timeout(intf, &(intf->seq_table[i]), &timeouts, timeout_period, i, From d1b29b9742a2a9a7931dcd59615a27ee9cf2c804 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 28 Mar 2018 11:35:47 -0500 Subject: [PATCH 03/35] ipmi:watchdog: Rework locking and handling Simplify things by creating one set of message handling data for setting the watchdog and doing a heartbeat. Rework the locking to avoid some (probably not very important) races and to avoid a fairly unlikely infinite recursion. Get rid of ipmi_ignore_heartbeat, it wasn't used, and use watchdog_user to tell if we have a working IPMI device below us. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_watchdog.c | 305 ++++++++++++++---------------- 1 file changed, 145 insertions(+), 160 deletions(-) diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c index 22bc287eac2d..d903096f882d 100644 --- a/drivers/char/ipmi/ipmi_watchdog.c +++ b/drivers/char/ipmi/ipmi_watchdog.c @@ -153,7 +153,7 @@ static DEFINE_SPINLOCK(ipmi_read_lock); static char data_to_read; static DECLARE_WAIT_QUEUE_HEAD(read_q); static struct fasync_struct *fasync_q; -static char pretimeout_since_last_heartbeat; +static atomic_t pretimeout_since_last_heartbeat; static char expect_close; static int ifnum_to_use = -1; @@ -303,9 +303,6 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started " /* Default state of the timer. */ static unsigned char ipmi_watchdog_state = WDOG_TIMEOUT_NONE; -/* If shutting down via IPMI, we ignore the heartbeat. */ -static int ipmi_ignore_heartbeat; - /* Is someone using the watchdog? Only one user is allowed. */ static unsigned long ipmi_wdog_open; @@ -329,35 +326,33 @@ static int testing_nmi; static int nmi_handler_registered; #endif -static int ipmi_heartbeat(void); +static int __ipmi_heartbeat(void); /* - * We use a mutex to make sure that only one thing can send a set - * timeout at one time, because we only have one copy of the data. - * The mutex is claimed when the set_timeout is sent and freed - * when both messages are free. + * We use a mutex to make sure that only one thing can send a set a + * message at one time. The mutex is claimed when a message is sent + * and freed when both the send and receive messages are free. */ -static atomic_t set_timeout_tofree = ATOMIC_INIT(0); -static DEFINE_MUTEX(set_timeout_lock); -static DECLARE_COMPLETION(set_timeout_wait); -static void set_timeout_free_smi(struct ipmi_smi_msg *msg) +static atomic_t msg_tofree = ATOMIC_INIT(0); +static DECLARE_COMPLETION(msg_wait); +static void msg_free_smi(struct ipmi_smi_msg *msg) { - if (atomic_dec_and_test(&set_timeout_tofree)) - complete(&set_timeout_wait); + if (atomic_dec_and_test(&msg_tofree)) + complete(&msg_wait); } -static void set_timeout_free_recv(struct ipmi_recv_msg *msg) +static void msg_free_recv(struct ipmi_recv_msg *msg) { - if (atomic_dec_and_test(&set_timeout_tofree)) - complete(&set_timeout_wait); + if (atomic_dec_and_test(&msg_tofree)) + complete(&msg_wait); } -static struct ipmi_smi_msg set_timeout_smi_msg = { - .done = set_timeout_free_smi +static struct ipmi_smi_msg smi_msg = { + .done = msg_free_smi }; -static struct ipmi_recv_msg set_timeout_recv_msg = { - .done = set_timeout_free_recv +static struct ipmi_recv_msg recv_msg = { + .done = msg_free_recv }; -static int i_ipmi_set_timeout(struct ipmi_smi_msg *smi_msg, +static int __ipmi_set_timeout(struct ipmi_smi_msg *smi_msg, struct ipmi_recv_msg *recv_msg, int *send_heartbeat_now) { @@ -368,9 +363,6 @@ static int i_ipmi_set_timeout(struct ipmi_smi_msg *smi_msg, int hbnow = 0; - /* These can be cleared as we are setting the timeout. */ - pretimeout_since_last_heartbeat = 0; - data[0] = 0; WDOG_SET_TIMER_USE(data[0], WDOG_TIMER_USE_SMS_OS); @@ -414,46 +406,48 @@ static int i_ipmi_set_timeout(struct ipmi_smi_msg *smi_msg, smi_msg, recv_msg, 1); - if (rv) { - printk(KERN_WARNING PFX "set timeout error: %d\n", - rv); - } + if (rv) + pr_warn(PFX "set timeout error: %d\n", rv); + else if (send_heartbeat_now) + *send_heartbeat_now = hbnow; - if (send_heartbeat_now) - *send_heartbeat_now = hbnow; + return rv; +} + +static int _ipmi_set_timeout(int do_heartbeat) +{ + int send_heartbeat_now; + int rv; + + if (!watchdog_user) + return -ENODEV; + + atomic_set(&msg_tofree, 2); + + rv = __ipmi_set_timeout(&smi_msg, + &recv_msg, + &send_heartbeat_now); + if (rv) + return rv; + + wait_for_completion(&msg_wait); + + if ((do_heartbeat == IPMI_SET_TIMEOUT_FORCE_HB) + || ((send_heartbeat_now) + && (do_heartbeat == IPMI_SET_TIMEOUT_HB_IF_NECESSARY))) + rv = __ipmi_heartbeat(); return rv; } static int ipmi_set_timeout(int do_heartbeat) { - int send_heartbeat_now; int rv; + mutex_lock(&ipmi_watchdog_mutex); + rv = _ipmi_set_timeout(do_heartbeat); + mutex_unlock(&ipmi_watchdog_mutex); - /* We can only send one of these at a time. */ - mutex_lock(&set_timeout_lock); - - atomic_set(&set_timeout_tofree, 2); - - rv = i_ipmi_set_timeout(&set_timeout_smi_msg, - &set_timeout_recv_msg, - &send_heartbeat_now); - if (rv) { - mutex_unlock(&set_timeout_lock); - goto out; - } - - wait_for_completion(&set_timeout_wait); - - mutex_unlock(&set_timeout_lock); - - if ((do_heartbeat == IPMI_SET_TIMEOUT_FORCE_HB) - || ((send_heartbeat_now) - && (do_heartbeat == IPMI_SET_TIMEOUT_HB_IF_NECESSARY))) - rv = ipmi_heartbeat(); - -out: return rv; } @@ -531,7 +525,7 @@ static void panic_halt_ipmi_set_timeout(void) while (atomic_read(&panic_done_count) != 0) ipmi_poll_interface(watchdog_user); atomic_add(1, &panic_done_count); - rv = i_ipmi_set_timeout(&panic_halt_smi_msg, + rv = __ipmi_set_timeout(&panic_halt_smi_msg, &panic_halt_recv_msg, &send_heartbeat_now); if (rv) { @@ -546,69 +540,22 @@ static void panic_halt_ipmi_set_timeout(void) ipmi_poll_interface(watchdog_user); } -/* - * We use a mutex to make sure that only one thing can send a - * heartbeat at one time, because we only have one copy of the data. - * The semaphore is claimed when the set_timeout is sent and freed - * when both messages are free. - */ -static atomic_t heartbeat_tofree = ATOMIC_INIT(0); -static DEFINE_MUTEX(heartbeat_lock); -static DECLARE_COMPLETION(heartbeat_wait); -static void heartbeat_free_smi(struct ipmi_smi_msg *msg) +static int __ipmi_heartbeat(void) { - if (atomic_dec_and_test(&heartbeat_tofree)) - complete(&heartbeat_wait); -} -static void heartbeat_free_recv(struct ipmi_recv_msg *msg) -{ - if (atomic_dec_and_test(&heartbeat_tofree)) - complete(&heartbeat_wait); -} -static struct ipmi_smi_msg heartbeat_smi_msg = { - .done = heartbeat_free_smi -}; -static struct ipmi_recv_msg heartbeat_recv_msg = { - .done = heartbeat_free_recv -}; - -static int ipmi_heartbeat(void) -{ - struct kernel_ipmi_msg msg; - int rv; + struct kernel_ipmi_msg msg; + int rv; struct ipmi_system_interface_addr addr; - int timeout_retries = 0; - - if (ipmi_ignore_heartbeat) - return 0; - - if (ipmi_start_timer_on_heartbeat) { - ipmi_start_timer_on_heartbeat = 0; - ipmi_watchdog_state = action_val; - return ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB); - } else if (pretimeout_since_last_heartbeat) { - /* - * A pretimeout occurred, make sure we set the timeout. - * We don't want to set the action, though, we want to - * leave that alone (thus it can't be combined with the - * above operation. - */ - return ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY); - } - - mutex_lock(&heartbeat_lock); + int timeout_retries = 0; restart: - atomic_set(&heartbeat_tofree, 2); - /* * Don't reset the timer if we have the timer turned off, that * re-enables the watchdog. */ - if (ipmi_watchdog_state == WDOG_TIMEOUT_NONE) { - mutex_unlock(&heartbeat_lock); + if (ipmi_watchdog_state == WDOG_TIMEOUT_NONE) return 0; - } + + atomic_set(&msg_tofree, 2); addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE; addr.channel = IPMI_BMC_CHANNEL; @@ -623,26 +570,23 @@ static int ipmi_heartbeat(void) 0, &msg, NULL, - &heartbeat_smi_msg, - &heartbeat_recv_msg, + &smi_msg, + &recv_msg, 1); if (rv) { - mutex_unlock(&heartbeat_lock); - printk(KERN_WARNING PFX "heartbeat failure: %d\n", - rv); + pr_warn(PFX "heartbeat send failure: %d\n", rv); return rv; } /* Wait for the heartbeat to be sent. */ - wait_for_completion(&heartbeat_wait); + wait_for_completion(&msg_wait); - if (heartbeat_recv_msg.msg.data[0] == IPMI_WDOG_TIMER_NOT_INIT_RESP) { + if (recv_msg.msg.data[0] == IPMI_WDOG_TIMER_NOT_INIT_RESP) { timeout_retries++; if (timeout_retries > 3) { - printk(KERN_ERR PFX ": Unable to restore the IPMI" - " watchdog's settings, giving up.\n"); + pr_err(PFX ": Unable to restore the IPMI watchdog's settings, giving up.\n"); rv = -EIO; - goto out_unlock; + goto out; } /* @@ -651,18 +595,17 @@ static int ipmi_heartbeat(void) * to restore the timer's info. Note that we still hold * the heartbeat lock, to keep a heartbeat from happening * in this process, so must say no heartbeat to avoid a - * deadlock on this mutex. + * deadlock on this mutex */ - rv = ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB); + rv = _ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB); if (rv) { - printk(KERN_ERR PFX ": Unable to send the command to" - " set the watchdog's settings, giving up.\n"); - goto out_unlock; + pr_err(PFX ": Unable to send the command to set the watchdog's settings, giving up.\n"); + goto out; } - /* We might need a new heartbeat, so do it now */ + /* Might need a heartbeat send, go ahead and do it. */ goto restart; - } else if (heartbeat_recv_msg.msg.data[0] != 0) { + } else if (recv_msg.msg.data[0] != 0) { /* * Got an error in the heartbeat response. It was already * reported in ipmi_wdog_msg_handler, but we should return @@ -671,8 +614,43 @@ static int ipmi_heartbeat(void) rv = -EINVAL; } -out_unlock: - mutex_unlock(&heartbeat_lock); +out: + return rv; +} + +static int _ipmi_heartbeat(void) +{ + int rv; + + if (!watchdog_user) + return -ENODEV; + + if (ipmi_start_timer_on_heartbeat) { + ipmi_start_timer_on_heartbeat = 0; + ipmi_watchdog_state = action_val; + rv = _ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB); + } else if (atomic_cmpxchg(&pretimeout_since_last_heartbeat, 1, 0)) { + /* + * A pretimeout occurred, make sure we set the timeout. + * We don't want to set the action, though, we want to + * leave that alone (thus it can't be combined with the + * above operation. + */ + rv = _ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY); + } else { + rv = __ipmi_heartbeat(); + } + + return rv; +} + +static int ipmi_heartbeat(void) +{ + int rv; + + mutex_lock(&ipmi_watchdog_mutex); + rv = _ipmi_heartbeat(); + mutex_unlock(&ipmi_watchdog_mutex); return rv; } @@ -700,7 +678,7 @@ static int ipmi_ioctl(struct file *file, if (i) return -EFAULT; timeout = val; - return ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY); + return _ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY); case WDIOC_GETTIMEOUT: i = copy_to_user(argp, &timeout, sizeof(timeout)); @@ -713,7 +691,7 @@ static int ipmi_ioctl(struct file *file, if (i) return -EFAULT; pretimeout = val; - return ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY); + return _ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY); case WDIOC_GETPRETIMEOUT: i = copy_to_user(argp, &pretimeout, sizeof(pretimeout)); @@ -722,7 +700,7 @@ static int ipmi_ioctl(struct file *file, return 0; case WDIOC_KEEPALIVE: - return ipmi_heartbeat(); + return _ipmi_heartbeat(); case WDIOC_SETOPTIONS: i = copy_from_user(&val, argp, sizeof(int)); @@ -730,13 +708,13 @@ static int ipmi_ioctl(struct file *file, return -EFAULT; if (val & WDIOS_DISABLECARD) { ipmi_watchdog_state = WDOG_TIMEOUT_NONE; - ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB); + _ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB); ipmi_start_timer_on_heartbeat = 0; } if (val & WDIOS_ENABLECARD) { ipmi_watchdog_state = action_val; - ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB); + _ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB); } return 0; @@ -810,7 +788,7 @@ static ssize_t ipmi_read(struct file *file, * Reading returns if the pretimeout has gone off, and it only does * it once per pretimeout. */ - spin_lock(&ipmi_read_lock); + spin_lock_irq(&ipmi_read_lock); if (!data_to_read) { if (file->f_flags & O_NONBLOCK) { rv = -EAGAIN; @@ -821,9 +799,9 @@ static ssize_t ipmi_read(struct file *file, add_wait_queue(&read_q, &wait); while (!data_to_read) { set_current_state(TASK_INTERRUPTIBLE); - spin_unlock(&ipmi_read_lock); + spin_unlock_irq(&ipmi_read_lock); schedule(); - spin_lock(&ipmi_read_lock); + spin_lock_irq(&ipmi_read_lock); } remove_wait_queue(&read_q, &wait); @@ -835,7 +813,7 @@ static ssize_t ipmi_read(struct file *file, data_to_read = 0; out: - spin_unlock(&ipmi_read_lock); + spin_unlock_irq(&ipmi_read_lock); if (rv == 0) { if (copy_to_user(buf, &data_to_read, 1)) @@ -873,10 +851,10 @@ static __poll_t ipmi_poll(struct file *file, poll_table *wait) poll_wait(file, &read_q, wait); - spin_lock(&ipmi_read_lock); + spin_lock_irq(&ipmi_read_lock); if (data_to_read) mask |= (EPOLLIN | EPOLLRDNORM); - spin_unlock(&ipmi_read_lock); + spin_unlock_irq(&ipmi_read_lock); return mask; } @@ -894,8 +872,10 @@ static int ipmi_close(struct inode *ino, struct file *filep) { if (iminor(ino) == WATCHDOG_MINOR) { if (expect_close == 42) { + mutex_lock(&ipmi_watchdog_mutex); ipmi_watchdog_state = WDOG_TIMEOUT_NONE; - ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB); + _ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB); + mutex_unlock(&ipmi_watchdog_mutex); } else { printk(KERN_CRIT PFX "Unexpected close, not stopping watchdog!\n"); @@ -950,12 +930,13 @@ static void ipmi_wdog_pretimeout_handler(void *handler_data) if (atomic_inc_and_test(&preop_panic_excl)) panic("Watchdog pre-timeout"); } else if (preop_val == WDOG_PREOP_GIVE_DATA) { - spin_lock(&ipmi_read_lock); + unsigned long flags; + + spin_lock_irqsave(&ipmi_read_lock, flags); data_to_read = 1; wake_up_interruptible(&read_q); kill_fasync(&fasync_q, SIGIO, POLL_IN); - - spin_unlock(&ipmi_read_lock); + spin_unlock_irqrestore(&ipmi_read_lock, flags); } } @@ -963,7 +944,7 @@ static void ipmi_wdog_pretimeout_handler(void *handler_data) * On some machines, the heartbeat will give an error and not * work unless we re-enable the timer. So do so. */ - pretimeout_since_last_heartbeat = 1; + atomic_set(&pretimeout_since_last_heartbeat, 1); } static const struct ipmi_user_hndl ipmi_hndlrs = { @@ -1063,34 +1044,38 @@ static void ipmi_register_watchdog(int ipmi_intf) static void ipmi_unregister_watchdog(int ipmi_intf) { int rv; + ipmi_user_t loc_user = watchdog_user; - if (!watchdog_user) - goto out; + if (!loc_user) + return; if (watchdog_ifnum != ipmi_intf) - goto out; + return; /* Make sure no one can call us any more. */ misc_deregister(&ipmi_wdog_miscdev); + watchdog_user = NULL; + /* * Wait to make sure the message makes it out. The lower layer has * pointers to our buffers, we want to make sure they are done before * we release our memory. */ - while (atomic_read(&set_timeout_tofree)) - schedule_timeout_uninterruptible(1); + while (atomic_read(&msg_tofree)) + msg_free_smi(NULL); + + mutex_lock(&ipmi_watchdog_mutex); /* Disconnect from IPMI. */ - rv = ipmi_destroy_user(watchdog_user); - if (rv) { - printk(KERN_WARNING PFX "error unlinking from IPMI: %d\n", - rv); - } - watchdog_user = NULL; + rv = ipmi_destroy_user(loc_user); + if (rv) + pr_warn(PFX "error unlinking from IPMI: %d\n", rv); - out: - return; + /* If it comes back, restart it properly. */ + ipmi_start_timer_on_heartbeat = 1; + + mutex_unlock(&ipmi_watchdog_mutex); } #ifdef HAVE_DIE_NMI @@ -1124,7 +1109,7 @@ ipmi_nmi(unsigned int val, struct pt_regs *regs) /* On some machines, the heartbeat will give an error and not work unless we re-enable the timer. So do so. */ - pretimeout_since_last_heartbeat = 1; + atomic_set(&pretimeout_since_last_heartbeat, 1); if (atomic_inc_and_test(&preop_panic_excl)) nmi_panic(regs, PFX "pre-timeout"); } From e2384917044ec27dd61acb9c3591eaa4c8fd4667 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 28 Mar 2018 13:04:06 -0500 Subject: [PATCH 04/35] ipmi:watchdog: Replace printk() with pr_xxx() And clean broken strings. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_watchdog.c | 50 +++++++++++++------------------ 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c index d903096f882d..25b86c7a1dec 100644 --- a/drivers/char/ipmi/ipmi_watchdog.c +++ b/drivers/char/ipmi/ipmi_watchdog.c @@ -530,8 +530,7 @@ static void panic_halt_ipmi_set_timeout(void) &send_heartbeat_now); if (rv) { atomic_sub(1, &panic_done_count); - printk(KERN_WARNING PFX - "Unable to extend the watchdog timeout."); + pr_warn(PFX "Unable to extend the watchdog timeout."); } else { if (send_heartbeat_now) panic_halt_ipmi_heartbeat(); @@ -877,8 +876,8 @@ static int ipmi_close(struct inode *ino, struct file *filep) _ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB); mutex_unlock(&ipmi_watchdog_mutex); } else { - printk(KERN_CRIT PFX - "Unexpected close, not stopping watchdog!\n"); + pr_crit(PFX + "Unexpected close, not stopping watchdog!\n"); ipmi_heartbeat(); } clear_bit(0, &ipmi_wdog_open); @@ -912,11 +911,9 @@ static void ipmi_wdog_msg_handler(struct ipmi_recv_msg *msg, { if (msg->msg.cmd == IPMI_WDOG_RESET_TIMER && msg->msg.data[0] == IPMI_WDOG_TIMER_NOT_INIT_RESP) - printk(KERN_INFO PFX "response: The IPMI controller appears" - " to have been reset, will attempt to reinitialize" - " the watchdog timer\n"); + pr_info(PFX "response: The IPMI controller appears to have been reset, will attempt to reinitialize the watchdog timer\n"); else if (msg->msg.data[0] != 0) - printk(KERN_ERR PFX "response: Error %x on cmd %x\n", + pr_err(PFX "response: Error %x on cmd %x\n", msg->msg.data[0], msg->msg.cmd); @@ -966,7 +963,7 @@ static void ipmi_register_watchdog(int ipmi_intf) rv = ipmi_create_user(ipmi_intf, &ipmi_hndlrs, NULL, &watchdog_user); if (rv < 0) { - printk(KERN_CRIT PFX "Unable to register with ipmi\n"); + pr_crit(PFX "Unable to register with ipmi\n"); goto out; } @@ -983,7 +980,7 @@ static void ipmi_register_watchdog(int ipmi_intf) if (rv < 0) { ipmi_destroy_user(watchdog_user); watchdog_user = NULL; - printk(KERN_CRIT PFX "Unable to register misc device\n"); + pr_crit(PFX "Unable to register misc device\n"); } #ifdef HAVE_DIE_NMI @@ -1005,9 +1002,8 @@ static void ipmi_register_watchdog(int ipmi_intf) rv = ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB); if (rv) { - printk(KERN_WARNING PFX "Error starting timer to" - " test NMI: 0x%x. The NMI pretimeout will" - " likely not work\n", rv); + pr_warn(PFX "Error starting timer to test NMI: 0x%x. The NMI pretimeout will likely not work\n", + rv); rv = 0; goto out_restore; } @@ -1015,9 +1011,7 @@ static void ipmi_register_watchdog(int ipmi_intf) msleep(1500); if (testing_nmi != 2) { - printk(KERN_WARNING PFX "IPMI NMI didn't seem to" - " occur. The NMI pretimeout will" - " likely not work\n"); + pr_warn(PFX "IPMI NMI didn't seem to occur. The NMI pretimeout will likely not work\n"); } out_restore: testing_nmi = 0; @@ -1033,7 +1027,7 @@ static void ipmi_register_watchdog(int ipmi_intf) start_now = 0; /* Disable this function after first startup. */ ipmi_watchdog_state = action_val; ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB); - printk(KERN_INFO PFX "Starting now!\n"); + pr_info(PFX "Starting now!\n"); } else { /* Stop the timer now. */ ipmi_watchdog_state = WDOG_TIMEOUT_NONE; @@ -1273,9 +1267,7 @@ static void check_parms(void) if (preaction_val == WDOG_PRETIMEOUT_NMI) { do_nmi = 1; if (preop_val == WDOG_PREOP_GIVE_DATA) { - printk(KERN_WARNING PFX "Pretimeout op is to give data" - " but NMI pretimeout is enabled, setting" - " pretimeout op to none\n"); + pr_warn(PFX "Pretimeout op is to give data but NMI pretimeout is enabled, setting pretimeout op to none\n"); preop_op("preop_none", NULL); do_nmi = 0; } @@ -1284,8 +1276,7 @@ static void check_parms(void) rv = register_nmi_handler(NMI_UNKNOWN, ipmi_nmi, 0, "ipmi"); if (rv) { - printk(KERN_WARNING PFX - "Can't register nmi handler\n"); + pr_warn(PFX "Can't register nmi handler\n"); return; } else nmi_handler_registered = 1; @@ -1302,20 +1293,19 @@ static int __init ipmi_wdog_init(void) if (action_op(action, NULL)) { action_op("reset", NULL); - printk(KERN_INFO PFX "Unknown action '%s', defaulting to" - " reset\n", action); + pr_info(PFX "Unknown action '%s', defaulting to reset\n", + action); } if (preaction_op(preaction, NULL)) { preaction_op("pre_none", NULL); - printk(KERN_INFO PFX "Unknown preaction '%s', defaulting to" - " none\n", preaction); + pr_info(PFX "Unknown preaction '%s', defaulting to none\n", + preaction); } if (preop_op(preop, NULL)) { preop_op("preop_none", NULL); - printk(KERN_INFO PFX "Unknown preop '%s', defaulting to" - " none\n", preop); + pr_info(PFX "Unknown preop '%s', defaulting to none\n", preop); } check_parms(); @@ -1333,11 +1323,11 @@ static int __init ipmi_wdog_init(void) atomic_notifier_chain_unregister(&panic_notifier_list, &wdog_panic_notifier); unregister_reboot_notifier(&wdog_reboot_notifier); - printk(KERN_WARNING PFX "can't register smi watcher\n"); + pr_warn(PFX "can't register smi watcher\n"); return rv; } - printk(KERN_INFO PFX "driver initialized\n"); + pr_info(PFX "driver initialized\n"); return 0; } From 91e2dd0a47bae19600f13dcc9e0761082c50afa6 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 28 Mar 2018 13:19:25 -0500 Subject: [PATCH 05/35] ipmi: Add a panic handler for IPMI users Users of the IPMI code had their own panic handlers, but the order was not necessarily right, the base IPMI code would need to handle the panic first, and the user had no way to know if the IPMI interface could run at panic time. Add a panic handler to the user interface, it is called if non-NULL and the interface the user is on is capable of panic handling. It also cleans up the panic log handling a bit to reuse the existing interface loop in the main panic handler. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 210 ++++++++++++++-------------- include/linux/ipmi.h | 6 + 2 files changed, 108 insertions(+), 108 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index dcfbf2e3c8c5..9ffbb5f9c7bd 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -4956,13 +4956,15 @@ static void device_id_fetcher(ipmi_smi_t intf, struct ipmi_recv_msg *msg) } } -static void send_panic_events(char *str) +static void send_panic_events(ipmi_smi_t intf, char *str) { - struct kernel_ipmi_msg msg; - ipmi_smi_t intf; - unsigned char data[16]; + struct kernel_ipmi_msg msg; + unsigned char data[16]; struct ipmi_system_interface_addr *si; - struct ipmi_addr addr; + struct ipmi_addr addr; + char *p = str; + struct ipmi_ipmb_addr *ipmb; + int j; if (ipmi_send_panic_event == IPMI_SEND_PANIC_EVENT_NONE) return; @@ -4993,15 +4995,8 @@ static void send_panic_events(char *str) data[7] = str[2]; } - /* For every registered interface, send the event. */ - list_for_each_entry_rcu(intf, &ipmi_interfaces, link) { - if (!intf->handlers || !intf->handlers->poll) - /* Interface is not ready or can't run at panic time. */ - continue; - - /* Send the event announcing the panic. */ - ipmi_panic_request_and_wait(intf, &addr, &msg); - } + /* Send the event announcing the panic. */ + ipmi_panic_request_and_wait(intf, &addr, &msg); /* * On every interface, dump a bunch of OEM event holding the @@ -5010,111 +5005,100 @@ static void send_panic_events(char *str) if (ipmi_send_panic_event != IPMI_SEND_PANIC_EVENT_STRING || !str) return; - /* For every registered interface, send the event. */ - list_for_each_entry_rcu(intf, &ipmi_interfaces, link) { - char *p = str; - struct ipmi_ipmb_addr *ipmb; - int j; + /* + * intf_num is used as an marker to tell if the + * interface is valid. Thus we need a read barrier to + * make sure data fetched before checking intf_num + * won't be used. + */ + smp_rmb(); - if (intf->intf_num == -1) - /* Interface was not ready yet. */ - continue; + /* + * First job here is to figure out where to send the + * OEM events. There's no way in IPMI to send OEM + * events using an event send command, so we have to + * find the SEL to put them in and stick them in + * there. + */ - /* - * intf_num is used as an marker to tell if the - * interface is valid. Thus we need a read barrier to - * make sure data fetched before checking intf_num - * won't be used. - */ - smp_rmb(); + /* Get capabilities from the get device id. */ + intf->local_sel_device = 0; + intf->local_event_generator = 0; + intf->event_receiver = 0; - /* - * First job here is to figure out where to send the - * OEM events. There's no way in IPMI to send OEM - * events using an event send command, so we have to - * find the SEL to put them in and stick them in - * there. - */ + /* Request the device info from the local MC. */ + msg.netfn = IPMI_NETFN_APP_REQUEST; + msg.cmd = IPMI_GET_DEVICE_ID_CMD; + msg.data = NULL; + msg.data_len = 0; + intf->null_user_handler = device_id_fetcher; + ipmi_panic_request_and_wait(intf, &addr, &msg); - /* Get capabilities from the get device id. */ - intf->local_sel_device = 0; - intf->local_event_generator = 0; - intf->event_receiver = 0; - - /* Request the device info from the local MC. */ - msg.netfn = IPMI_NETFN_APP_REQUEST; - msg.cmd = IPMI_GET_DEVICE_ID_CMD; + if (intf->local_event_generator) { + /* Request the event receiver from the local MC. */ + msg.netfn = IPMI_NETFN_SENSOR_EVENT_REQUEST; + msg.cmd = IPMI_GET_EVENT_RECEIVER_CMD; msg.data = NULL; msg.data_len = 0; - intf->null_user_handler = device_id_fetcher; + intf->null_user_handler = event_receiver_fetcher; ipmi_panic_request_and_wait(intf, &addr, &msg); + } + intf->null_user_handler = NULL; - if (intf->local_event_generator) { - /* Request the event receiver from the local MC. */ - msg.netfn = IPMI_NETFN_SENSOR_EVENT_REQUEST; - msg.cmd = IPMI_GET_EVENT_RECEIVER_CMD; - msg.data = NULL; - msg.data_len = 0; - intf->null_user_handler = event_receiver_fetcher; - ipmi_panic_request_and_wait(intf, &addr, &msg); - } - intf->null_user_handler = NULL; - + /* + * Validate the event receiver. The low bit must not + * be 1 (it must be a valid IPMB address), it cannot + * be zero, and it must not be my address. + */ + if (((intf->event_receiver & 1) == 0) + && (intf->event_receiver != 0) + && (intf->event_receiver != intf->addrinfo[0].address)) { /* - * Validate the event receiver. The low bit must not - * be 1 (it must be a valid IPMB address), it cannot - * be zero, and it must not be my address. + * The event receiver is valid, send an IPMB + * message. */ - if (((intf->event_receiver & 1) == 0) - && (intf->event_receiver != 0) - && (intf->event_receiver != intf->addrinfo[0].address)) { - /* - * The event receiver is valid, send an IPMB - * message. - */ - ipmb = (struct ipmi_ipmb_addr *) &addr; - ipmb->addr_type = IPMI_IPMB_ADDR_TYPE; - ipmb->channel = 0; /* FIXME - is this right? */ - ipmb->lun = intf->event_receiver_lun; - ipmb->slave_addr = intf->event_receiver; - } else if (intf->local_sel_device) { - /* - * The event receiver was not valid (or was - * me), but I am an SEL device, just dump it - * in my SEL. - */ - si = (struct ipmi_system_interface_addr *) &addr; - si->addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE; - si->channel = IPMI_BMC_CHANNEL; - si->lun = 0; - } else - continue; /* No where to send the event. */ + ipmb = (struct ipmi_ipmb_addr *) &addr; + ipmb->addr_type = IPMI_IPMB_ADDR_TYPE; + ipmb->channel = 0; /* FIXME - is this right? */ + ipmb->lun = intf->event_receiver_lun; + ipmb->slave_addr = intf->event_receiver; + } else if (intf->local_sel_device) { + /* + * The event receiver was not valid (or was + * me), but I am an SEL device, just dump it + * in my SEL. + */ + si = (struct ipmi_system_interface_addr *) &addr; + si->addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE; + si->channel = IPMI_BMC_CHANNEL; + si->lun = 0; + } else + return; /* No where to send the event. */ - msg.netfn = IPMI_NETFN_STORAGE_REQUEST; /* Storage. */ - msg.cmd = IPMI_ADD_SEL_ENTRY_CMD; - msg.data = data; - msg.data_len = 16; + msg.netfn = IPMI_NETFN_STORAGE_REQUEST; /* Storage. */ + msg.cmd = IPMI_ADD_SEL_ENTRY_CMD; + msg.data = data; + msg.data_len = 16; - j = 0; - while (*p) { - int size = strlen(p); + j = 0; + while (*p) { + int size = strlen(p); - if (size > 11) - size = 11; - data[0] = 0; - data[1] = 0; - data[2] = 0xf0; /* OEM event without timestamp. */ - data[3] = intf->addrinfo[0].address; - data[4] = j++; /* sequence # */ - /* - * Always give 11 bytes, so strncpy will fill - * it with zeroes for me. - */ - strncpy(data+5, p, 11); - p += size; + if (size > 11) + size = 11; + data[0] = 0; + data[1] = 0; + data[2] = 0xf0; /* OEM event without timestamp. */ + data[3] = intf->addrinfo[0].address; + data[4] = j++; /* sequence # */ + /* + * Always give 11 bytes, so strncpy will fill + * it with zeroes for me. + */ + strncpy(data+5, p, 11); + p += size; - ipmi_panic_request_and_wait(intf, &addr, &msg); - } + ipmi_panic_request_and_wait(intf, &addr, &msg); } } @@ -5125,6 +5109,7 @@ static int panic_event(struct notifier_block *this, void *ptr) { ipmi_smi_t intf; + ipmi_user_t user; if (has_panicked) return NOTIFY_DONE; @@ -5132,10 +5117,13 @@ static int panic_event(struct notifier_block *this, /* For every registered interface, set it to run to completion. */ list_for_each_entry_rcu(intf, &ipmi_interfaces, link) { - if (!intf->handlers) + if (!intf->handlers || intf->intf_num == -1) /* Interface is not ready. */ continue; + if (!intf->handlers->poll) + continue; + /* * If we were interrupted while locking xmit_msgs_lock or * waiting_rcv_msgs_lock, the corresponding list may be @@ -5157,9 +5145,15 @@ static int panic_event(struct notifier_block *this, if (intf->handlers->set_run_to_completion) intf->handlers->set_run_to_completion(intf->send_info, 1); - } - send_panic_events(ptr); + list_for_each_entry_rcu(user, &intf->users, link) { + if (user->handler->ipmi_panic_handler) + user->handler->ipmi_panic_handler( + user->handler_data); + } + + send_panic_events(intf, ptr); + } return NOTIFY_DONE; } diff --git a/include/linux/ipmi.h b/include/linux/ipmi.h index 8b0626cec980..39a29fb3131b 100644 --- a/include/linux/ipmi.h +++ b/include/linux/ipmi.h @@ -77,6 +77,12 @@ struct ipmi_user_hndl { /* Called when the interface detects a watchdog pre-timeout. If this is NULL, it will be ignored for the user. */ void (*ipmi_watchdog_pretimeout)(void *handler_data); + + /* + * If not NULL, called at panic time after the interface has + * been set up to handle run to completion. + */ + void (*ipmi_panic_handler)(void *handler_data); }; /* Create a new user of the IPMI layer on the given interface number. */ From 5194970c5b4ccb651e4b8d32a1b617d7d2892d68 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 28 Mar 2018 13:30:38 -0500 Subject: [PATCH 06/35] ipmi:watchdog: Use the IPMI panic handler instead of the system one This is a cleaner interface and the main IPMI panic handler does setup required by the watchdog handler. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_watchdog.c | 60 ++++++++++++------------------- 1 file changed, 23 insertions(+), 37 deletions(-) diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c index 25b86c7a1dec..ab3eff581052 100644 --- a/drivers/char/ipmi/ipmi_watchdog.c +++ b/drivers/char/ipmi/ipmi_watchdog.c @@ -944,9 +944,31 @@ static void ipmi_wdog_pretimeout_handler(void *handler_data) atomic_set(&pretimeout_since_last_heartbeat, 1); } +static void ipmi_wdog_panic_handler(void *user_data) +{ + static int panic_event_handled; + + /* + * On a panic, if we have a panic timeout, make sure to extend + * the watchdog timer to a reasonable value to complete the + * panic, if the watchdog timer is running. Plus the + * pretimeout is meaningless at panic time. + */ + if (watchdog_user && !panic_event_handled && + ipmi_watchdog_state != WDOG_TIMEOUT_NONE) { + /* Make sure we do this only once. */ + panic_event_handled = 1; + + timeout = panic_wdt_timeout; + pretimeout = 0; + panic_halt_ipmi_set_timeout(); + } +} + static const struct ipmi_user_hndl ipmi_hndlrs = { .ipmi_recv_hndl = ipmi_wdog_msg_handler, - .ipmi_watchdog_pretimeout = ipmi_wdog_pretimeout_handler + .ipmi_watchdog_pretimeout = ipmi_wdog_pretimeout_handler, + .ipmi_panic_handler = ipmi_wdog_panic_handler }; static void ipmi_register_watchdog(int ipmi_intf) @@ -1146,36 +1168,6 @@ static struct notifier_block wdog_reboot_notifier = { .priority = 0 }; -static int wdog_panic_handler(struct notifier_block *this, - unsigned long event, - void *unused) -{ - static int panic_event_handled; - - /* On a panic, if we have a panic timeout, make sure to extend - the watchdog timer to a reasonable value to complete the - panic, if the watchdog timer is running. Plus the - pretimeout is meaningless at panic time. */ - if (watchdog_user && !panic_event_handled && - ipmi_watchdog_state != WDOG_TIMEOUT_NONE) { - /* Make sure we do this only once. */ - panic_event_handled = 1; - - timeout = panic_wdt_timeout; - pretimeout = 0; - panic_halt_ipmi_set_timeout(); - } - - return NOTIFY_OK; -} - -static struct notifier_block wdog_panic_notifier = { - .notifier_call = wdog_panic_handler, - .next = NULL, - .priority = 150 /* priority: INT_MAX >= x >= 0 */ -}; - - static void ipmi_new_smi(int if_num, struct device *device) { ipmi_register_watchdog(if_num); @@ -1311,8 +1303,6 @@ static int __init ipmi_wdog_init(void) check_parms(); register_reboot_notifier(&wdog_reboot_notifier); - atomic_notifier_chain_register(&panic_notifier_list, - &wdog_panic_notifier); rv = ipmi_smi_watcher_register(&smi_watcher); if (rv) { @@ -1320,8 +1310,6 @@ static int __init ipmi_wdog_init(void) if (nmi_handler_registered) unregister_nmi_handler(NMI_UNKNOWN, "ipmi"); #endif - atomic_notifier_chain_unregister(&panic_notifier_list, - &wdog_panic_notifier); unregister_reboot_notifier(&wdog_reboot_notifier); pr_warn(PFX "can't register smi watcher\n"); return rv; @@ -1342,8 +1330,6 @@ static void __exit ipmi_wdog_exit(void) unregister_nmi_handler(NMI_UNKNOWN, "ipmi"); #endif - atomic_notifier_chain_unregister(&panic_notifier_list, - &wdog_panic_notifier); unregister_reboot_notifier(&wdog_reboot_notifier); } module_exit(ipmi_wdog_exit); From f41382ae57f40b833c1080483770c59a791ac34f Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 3 Apr 2018 18:02:39 -0500 Subject: [PATCH 07/35] ipmi: Clean up some debug code Replace ifdefs in the code with a simple function. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 58 +++++++++++++---------------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 9ffbb5f9c7bd..a4c3336cec06 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -42,6 +42,25 @@ static void need_waiter(ipmi_smi_t intf); static int handle_one_recv_msg(ipmi_smi_t intf, struct ipmi_smi_msg *msg); +#ifdef DEBUG +static void ipmi_debug_msg(const char *title, unsigned char *data, + unsigned int len) +{ + int i, pos; + char buf[100]; + + pos = snprintf(buf, sizeof(buf), "%s: ", title); + for (i = 0; i < len; i++) + pos += snprintf(buf + pos, sizeof(buf) - pos, + " %2.2x", data[i]); + pr_debug("%s\n", buf); +} +#else +static void ipmi_debug_msg(const char *title, unsigned char *data, + unsigned int len) +{ } +#endif + static int initialized; enum ipmi_panic_event_op { @@ -2051,14 +2070,7 @@ static int i_ipmi_request(ipmi_user_t user, goto out_err; } -#ifdef DEBUG_MSGING - { - int m; - for (m = 0; m < smi_msg->data_size; m++) - printk(" %2.2x", smi_msg->data[m]); - printk("\n"); - } -#endif + ipmi_debug_msg("Send", smi_msg->data, smi_msg->data_size); smi_send(intf, intf->handlers, smi_msg, priority); rcu_read_unlock(); @@ -3736,15 +3748,8 @@ static int handle_ipmb_get_msg_cmd(ipmi_smi_t intf, msg->data[10] = ipmb_checksum(&(msg->data[6]), 4); msg->data_size = 11; -#ifdef DEBUG_MSGING - { - int m; - printk("Invalid command:"); - for (m = 0; m < msg->data_size; m++) - printk(" %2.2x", msg->data[m]); - printk("\n"); - } -#endif + ipmi_debug_msg("Invalid command:", msg->data, msg->data_size); + rcu_read_lock(); if (!intf->in_shutdown) { smi_send(intf, intf->handlers, msg, 0); @@ -4247,13 +4252,7 @@ static int handle_one_recv_msg(ipmi_smi_t intf, int requeue; int chan; -#ifdef DEBUG_MSGING - int m; - printk("Recv:"); - for (m = 0; m < msg->rsp_size; m++) - printk(" %2.2x", msg->rsp[m]); - printk("\n"); -#endif + ipmi_debug_msg("Recv:", msg->rsp, msg->rsp_size); if (msg->rsp_size < 2) { /* Message is too small to be correct. */ dev_warn(intf->si_dev, @@ -4614,15 +4613,8 @@ smi_from_recv_msg(ipmi_smi_t intf, struct ipmi_recv_msg *recv_msg, smi_msg->data_size = recv_msg->msg.data_len; smi_msg->msgid = STORE_SEQ_IN_MSGID(seq, seqid); -#ifdef DEBUG_MSGING - { - int m; - printk("Resend: "); - for (m = 0; m < smi_msg->data_size; m++) - printk(" %2.2x", smi_msg->data[m]); - printk("\n"); - } -#endif + ipmi_debug_msg("Resend: ", smi_msg->data, smi_msg->data_size); + return smi_msg; } From c81c5fc2128e3a596900859f10e294e30bc49b24 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 3 Apr 2018 18:20:52 -0500 Subject: [PATCH 08/35] ipmi:devintf: Clean up some coding style issues Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_devintf.c | 40 ++++++++++++++------------------ 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/drivers/char/ipmi/ipmi_devintf.c b/drivers/char/ipmi/ipmi_devintf.c index 8ecfd47806fa..f862838e2c10 100644 --- a/drivers/char/ipmi/ipmi_devintf.c +++ b/drivers/char/ipmi/ipmi_devintf.c @@ -45,17 +45,17 @@ static void file_receive_handler(struct ipmi_recv_msg *msg, int was_empty; unsigned long flags; - spin_lock_irqsave(&(priv->recv_msg_lock), flags); + spin_lock_irqsave(&priv->recv_msg_lock, flags); - was_empty = list_empty(&(priv->recv_msgs)); - list_add_tail(&(msg->link), &(priv->recv_msgs)); + was_empty = list_empty(&priv->recv_msgs); + list_add_tail(&msg->link, &priv->recv_msgs); if (was_empty) { wake_up_interruptible(&priv->wait); kill_fasync(&priv->fasync_queue, SIGIO, POLL_IN); } - spin_unlock_irqrestore(&(priv->recv_msg_lock), flags); + spin_unlock_irqrestore(&priv->recv_msg_lock, flags); } static __poll_t ipmi_poll(struct file *file, poll_table *wait) @@ -68,7 +68,7 @@ static __poll_t ipmi_poll(struct file *file, poll_table *wait) spin_lock_irqsave(&priv->recv_msg_lock, flags); - if (!list_empty(&(priv->recv_msgs))) + if (!list_empty(&priv->recv_msgs)) mask |= (EPOLLIN | EPOLLRDNORM); spin_unlock_irqrestore(&priv->recv_msg_lock, flags); @@ -110,7 +110,7 @@ static int ipmi_open(struct inode *inode, struct file *file) rv = ipmi_create_user(if_num, &ipmi_hndlrs, priv, - &(priv->user)); + &priv->user); if (rv) { kfree(priv); goto out; @@ -118,8 +118,8 @@ static int ipmi_open(struct inode *inode, struct file *file) file->private_data = priv; - spin_lock_init(&(priv->recv_msg_lock)); - INIT_LIST_HEAD(&(priv->recv_msgs)); + spin_lock_init(&priv->recv_msg_lock); + INIT_LIST_HEAD(&priv->recv_msgs); init_waitqueue_head(&priv->wait); priv->fasync_queue = NULL; mutex_init(&priv->recv_mutex); @@ -146,7 +146,6 @@ static int ipmi_release(struct inode *inode, struct file *file) list_for_each_entry_safe(msg, next, &priv->recv_msgs, link) ipmi_free_recv_msg(msg); - kfree(priv); return 0; @@ -189,8 +188,7 @@ static int handle_send_req(ipmi_user_t user, if (copy_from_user(msg.data, req->msg.data, - req->msg.data_len)) - { + req->msg.data_len)) { rv = -EFAULT; goto out; } @@ -233,25 +231,24 @@ static int handle_recv(struct ipmi_file_private *priv, mutex_lock(&priv->recv_mutex); /* Grab the message off the list. */ - spin_lock_irqsave(&(priv->recv_msg_lock), flags); + spin_lock_irqsave(&priv->recv_msg_lock, flags); if (list_empty(&(priv->recv_msgs))) { - spin_unlock_irqrestore(&(priv->recv_msg_lock), flags); + spin_unlock_irqrestore(&priv->recv_msg_lock, flags); rv = -EAGAIN; goto recv_err; } entry = priv->recv_msgs.next; msg = list_entry(entry, struct ipmi_recv_msg, link); list_del(entry); - spin_unlock_irqrestore(&(priv->recv_msg_lock), flags); + spin_unlock_irqrestore(&priv->recv_msg_lock, flags); addr_len = ipmi_addr_length(msg->addr.addr_type); - if (rsp->addr_len < addr_len) - { + if (rsp->addr_len < addr_len) { rv = -EINVAL; goto recv_putback_on_err; } - if (copy_to_user(rsp->addr, &(msg->addr), addr_len)) { + if (copy_to_user(rsp->addr, &msg->addr, addr_len)) { rv = -EFAULT; goto recv_putback_on_err; } @@ -273,8 +270,7 @@ static int handle_recv(struct ipmi_file_private *priv, if (copy_to_user(rsp->msg.data, msg->msg.data, - msg->msg.data_len)) - { + msg->msg.data_len)) { rv = -EFAULT; goto recv_putback_on_err; } @@ -294,9 +290,9 @@ static int handle_recv(struct ipmi_file_private *priv, recv_putback_on_err: /* If we got an error, put the message back onto the head of the queue. */ - spin_lock_irqsave(&(priv->recv_msg_lock), flags); - list_add(entry, &(priv->recv_msgs)); - spin_unlock_irqrestore(&(priv->recv_msg_lock), flags); + spin_lock_irqsave(&priv->recv_msg_lock, flags); + list_add(entry, &priv->recv_msgs); + spin_unlock_irqrestore(&priv->recv_msg_lock, flags); recv_err: mutex_unlock(&priv->recv_mutex); return rv; From 6dc1181f9fbcf7ba0e62adfaea41666f00ee9d18 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 4 Apr 2018 08:54:05 -0500 Subject: [PATCH 09/35] ipmi: Clean up comments in include files. Make the comments correct and consistent. Signed-off-by: Corey Minyard --- include/linux/ipmi.h | 105 +++++++++++++++++++++-------------- include/linux/ipmi_smi.h | 115 +++++++++++++++++++++++---------------- 2 files changed, 134 insertions(+), 86 deletions(-) diff --git a/include/linux/ipmi.h b/include/linux/ipmi.h index 39a29fb3131b..3474f04cf9aa 100644 --- a/include/linux/ipmi.h +++ b/include/linux/ipmi.h @@ -23,8 +23,10 @@ struct module; struct device; -/* Opaque type for a IPMI message user. One of these is needed to - send and receive messages. */ +/* + * Opaque type for a IPMI message user. One of these is needed to + * send and receive messages. + */ typedef struct ipmi_user *ipmi_user_t; /* @@ -37,8 +39,10 @@ typedef struct ipmi_user *ipmi_user_t; struct ipmi_recv_msg { struct list_head link; - /* The type of message as defined in the "Receive Types" - defines above. */ + /* + * The type of message as defined in the "Receive Types" + * defines above. + */ int recv_type; ipmi_user_t user; @@ -46,19 +50,25 @@ struct ipmi_recv_msg { long msgid; struct kernel_ipmi_msg msg; - /* The user_msg_data is the data supplied when a message was - sent, if this is a response to a sent message. If this is - not a response to a sent message, then user_msg_data will - be NULL. If the user above is NULL, then this will be the - intf. */ + /* + * The user_msg_data is the data supplied when a message was + * sent, if this is a response to a sent message. If this is + * not a response to a sent message, then user_msg_data will + * be NULL. If the user above is NULL, then this will be the + * intf. + */ void *user_msg_data; - /* Call this when done with the message. It will presumably free - the message and do any other necessary cleanup. */ + /* + * Call this when done with the message. It will presumably free + * the message and do any other necessary cleanup. + */ void (*done)(struct ipmi_recv_msg *msg); - /* Place-holder for the data, don't make any assumptions about - the size or existence of this, since it may change. */ + /* + * Place-holder for the data, don't make any assumptions about + * the size or existence of this, since it may change. + */ unsigned char msg_data[IPMI_MAX_MSG_LENGTH]; }; @@ -66,16 +76,20 @@ struct ipmi_recv_msg { void ipmi_free_recv_msg(struct ipmi_recv_msg *msg); struct ipmi_user_hndl { - /* Routine type to call when a message needs to be routed to - the upper layer. This will be called with some locks held, - the only IPMI routines that can be called are ipmi_request - and the alloc/free operations. The handler_data is the - variable supplied when the receive handler was registered. */ + /* + * Routine type to call when a message needs to be routed to + * the upper layer. This will be called with some locks held, + * the only IPMI routines that can be called are ipmi_request + * and the alloc/free operations. The handler_data is the + * variable supplied when the receive handler was registered. + */ void (*ipmi_recv_hndl)(struct ipmi_recv_msg *msg, void *user_msg_data); - /* Called when the interface detects a watchdog pre-timeout. If - this is NULL, it will be ignored for the user. */ + /* + * Called when the interface detects a watchdog pre-timeout. If + * this is NULL, it will be ignored for the user. + */ void (*ipmi_watchdog_pretimeout)(void *handler_data); /* @@ -91,12 +105,14 @@ int ipmi_create_user(unsigned int if_num, void *handler_data, ipmi_user_t *user); -/* Destroy the given user of the IPMI layer. Note that after this - function returns, the system is guaranteed to not call any - callbacks for the user. Thus as long as you destroy all the users - before you unload a module, you will be safe. And if you destroy - the users before you destroy the callback structures, it should be - safe, too. */ +/* + * Destroy the given user of the IPMI layer. Note that after this + * function returns, the system is guaranteed to not call any + * callbacks for the user. Thus as long as you destroy all the users + * before you unload a module, you will be safe. And if you destroy + * the users before you destroy the callback structures, it should be + * safe, too. + */ int ipmi_destroy_user(ipmi_user_t user); /* Get the IPMI version of the BMC we are talking to. */ @@ -104,12 +120,15 @@ int ipmi_get_version(ipmi_user_t user, unsigned char *major, unsigned char *minor); -/* Set and get the slave address and LUN that we will use for our - source messages. Note that this affects the interface, not just - this user, so it will affect all users of this interface. This is - so some initialization code can come in and do the OEM-specific - things it takes to determine your address (if not the BMC) and set - it for everyone else. Note that each channel can have its own address. */ +/* + * Set and get the slave address and LUN that we will use for our + * source messages. Note that this affects the interface, not just + * this user, so it will affect all users of this interface. This is + * so some initialization code can come in and do the OEM-specific + * things it takes to determine your address (if not the BMC) and set + * it for everyone else. Note that each channel can have its own + * address. + */ int ipmi_set_my_address(ipmi_user_t user, unsigned int channel, unsigned char address); @@ -235,14 +254,18 @@ int ipmi_set_gets_events(ipmi_user_t user, bool val); struct ipmi_smi_watcher { struct list_head link; - /* You must set the owner to the current module, if you are in - a module (generally just set it to "THIS_MODULE"). */ + /* + * You must set the owner to the current module, if you are in + * a module (generally just set it to "THIS_MODULE"). + */ struct module *owner; - /* These two are called with read locks held for the interface - the watcher list. So you can add and remove users from the - IPMI interface, send messages, etc., but you cannot add - or remove SMI watchers or SMI interfaces. */ + /* + * These two are called with read locks held for the interface + * the watcher list. So you can add and remove users from the + * IPMI interface, send messages, etc., but you cannot add + * or remove SMI watchers or SMI interfaces. + */ void (*new_smi)(int if_num, struct device *dev); void (*smi_gone)(int if_num); }; @@ -250,8 +273,10 @@ struct ipmi_smi_watcher { int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher); int ipmi_smi_watcher_unregister(struct ipmi_smi_watcher *watcher); -/* The following are various helper functions for dealing with IPMI - addresses. */ +/* + * The following are various helper functions for dealing with IPMI + * addresses. + */ /* Return the maximum length of an IPMI address given it's type. */ unsigned int ipmi_addr_length(int addr_type); diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h index af457b5a689e..9e5c3079d232 100644 --- a/include/linux/ipmi_smi.h +++ b/include/linux/ipmi_smi.h @@ -22,8 +22,10 @@ struct device; -/* This files describes the interface for IPMI system management interface - drivers to bind into the IPMI message handler. */ +/* + * This files describes the interface for IPMI system management interface + * drivers to bind into the IPMI message handler. + */ /* Structure for the low-level drivers. */ typedef struct ipmi_smi *ipmi_smi_t; @@ -61,10 +63,12 @@ struct ipmi_smi_msg { struct ipmi_smi_handlers { struct module *owner; - /* The low-level interface cannot start sending messages to - the upper layer until this function is called. This may - not be NULL, the lower layer must take the interface from - this call. */ + /* + * The low-level interface cannot start sending messages to + * the upper layer until this function is called. This may + * not be NULL, the lower layer must take the interface from + * this call. + */ int (*start_processing)(void *send_info, ipmi_smi_t new_intf); @@ -75,25 +79,31 @@ struct ipmi_smi_handlers { */ int (*get_smi_info)(void *send_info, struct ipmi_smi_info *data); - /* Called to enqueue an SMI message to be sent. This - operation is not allowed to fail. If an error occurs, it - should report back the error in a received message. It may - do this in the current call context, since no write locks - are held when this is run. Message are delivered one at - a time by the message handler, a new message will not be - delivered until the previous message is returned. */ + /* + * Called to enqueue an SMI message to be sent. This + * operation is not allowed to fail. If an error occurs, it + * should report back the error in a received message. It may + * do this in the current call context, since no write locks + * are held when this is run. Message are delivered one at + * a time by the message handler, a new message will not be + * delivered until the previous message is returned. + */ void (*sender)(void *send_info, struct ipmi_smi_msg *msg); - /* Called by the upper layer to request that we try to get - events from the BMC we are attached to. */ + /* + * Called by the upper layer to request that we try to get + * events from the BMC we are attached to. + */ void (*request_events)(void *send_info); - /* Called by the upper layer when some user requires that the - interface watch for events, received messages, watchdog - pretimeouts, or not. Used by the SMI to know if it should - watch for these. This may be NULL if the SMI does not - implement it. */ + /* + * Called by the upper layer when some user requires that the + * interface watch for events, received messages, watchdog + * pretimeouts, or not. Used by the SMI to know if it should + * watch for these. This may be NULL if the SMI does not + * implement it. + */ void (*set_need_watch)(void *send_info, bool enable); /* @@ -101,28 +111,36 @@ struct ipmi_smi_handlers { */ void (*flush_messages)(void *send_info); - /* Called when the interface should go into "run to - completion" mode. If this call sets the value to true, the - interface should make sure that all messages are flushed - out and that none are pending, and any new requests are run - to completion immediately. */ + /* + * Called when the interface should go into "run to + * completion" mode. If this call sets the value to true, the + * interface should make sure that all messages are flushed + * out and that none are pending, and any new requests are run + * to completion immediately. + */ void (*set_run_to_completion)(void *send_info, bool run_to_completion); - /* Called to poll for work to do. This is so upper layers can - poll for operations during things like crash dumps. */ + /* + * Called to poll for work to do. This is so upper layers can + * poll for operations during things like crash dumps. + */ void (*poll)(void *send_info); - /* Enable/disable firmware maintenance mode. Note that this - is *not* the modes defined, this is simply an on/off - setting. The message handler does the mode handling. Note - that this is called from interrupt context, so it cannot - block. */ + /* + * Enable/disable firmware maintenance mode. Note that this + * is *not* the modes defined, this is simply an on/off + * setting. The message handler does the mode handling. Note + * that this is called from interrupt context, so it cannot + * block. + */ void (*set_maintenance_mode)(void *send_info, bool enable); - /* Tell the handler that we are using it/not using it. The - message handler get the modules that this handler belongs - to; this function lets the SMI claim any modules that it - uses. These may be NULL if this is not required. */ + /* + * Tell the handler that we are using it/not using it. The + * message handler get the modules that this handler belongs + * to; this function lets the SMI claim any modules that it + * uses. These may be NULL if this is not required. + */ int (*inc_usecount)(void *send_info); void (*dec_usecount)(void *send_info); }; @@ -143,7 +161,8 @@ struct ipmi_device_id { #define ipmi_version_major(v) ((v)->ipmi_version & 0xf) #define ipmi_version_minor(v) ((v)->ipmi_version >> 4) -/* Take a pointer to an IPMI response and extract device id information from +/* + * Take a pointer to an IPMI response and extract device id information from * it. @netfn is in the IPMI_NETFN_ format, so may need to be shifted from * a SI response. */ @@ -187,12 +206,14 @@ static inline int ipmi_demangle_device_id(uint8_t netfn, uint8_t cmd, return 0; } -/* Add a low-level interface to the IPMI driver. Note that if the - interface doesn't know its slave address, it should pass in zero. - The low-level interface should not deliver any messages to the - upper layer until the start_processing() function in the handlers - is called, and the lower layer must get the interface from that - call. */ +/* + * Add a low-level interface to the IPMI driver. Note that if the + * interface doesn't know its slave address, it should pass in zero. + * The low-level interface should not deliver any messages to the + * upper layer until the start_processing() function in the handlers + * is called, and the lower layer must get the interface from that + * call. + */ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, void *send_info, struct device *dev, @@ -223,9 +244,11 @@ static inline void ipmi_free_smi_msg(struct ipmi_smi_msg *msg) } #ifdef CONFIG_IPMI_PROC_INTERFACE -/* Allow the lower layer to add things to the proc filesystem - directory for this interface. Note that the entry will - automatically be dstroyed when the interface is destroyed. */ +/* + * Allow the lower layer to add things to the proc filesystem + * directory for this interface. Note that the entry will + * automatically be dstroyed when the interface is destroyed. + */ int ipmi_smi_add_proc_entry(ipmi_smi_t smi, char *name, const struct file_operations *proc_ops, void *data); From 42c2dc7e66a72f3f8daea60fd7604736552d1563 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 12:10:16 -0500 Subject: [PATCH 10/35] ipmi: Break up i_ipmi_request It was huge, and easily broken into pieces. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 654 +++++++++++++++------------- 1 file changed, 344 insertions(+), 310 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index a4c3336cec06..e26e5b6900ee 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -1703,6 +1703,332 @@ static bool is_maintenance_mode_cmd(struct kernel_ipmi_msg *msg) || (msg->netfn == IPMI_NETFN_FIRMWARE_REQUEST)); } +static int i_ipmi_req_sysintf(ipmi_smi_t intf, + struct ipmi_addr *addr, + long msgid, + struct kernel_ipmi_msg *msg, + struct ipmi_smi_msg *smi_msg, + struct ipmi_recv_msg *recv_msg, + int retries, + unsigned int retry_time_ms) +{ + struct ipmi_system_interface_addr *smi_addr; + + if (msg->netfn & 1) + /* Responses are not allowed to the SMI. */ + return -EINVAL; + + smi_addr = (struct ipmi_system_interface_addr *) addr; + if (smi_addr->lun > 3) { + ipmi_inc_stat(intf, sent_invalid_commands); + return -EINVAL; + } + + memcpy(&recv_msg->addr, smi_addr, sizeof(*smi_addr)); + + if ((msg->netfn == IPMI_NETFN_APP_REQUEST) + && ((msg->cmd == IPMI_SEND_MSG_CMD) + || (msg->cmd == IPMI_GET_MSG_CMD) + || (msg->cmd == IPMI_READ_EVENT_MSG_BUFFER_CMD))) { + /* + * We don't let the user do these, since we manage + * the sequence numbers. + */ + ipmi_inc_stat(intf, sent_invalid_commands); + return -EINVAL; + } + + if (is_maintenance_mode_cmd(msg)) { + unsigned long flags; + + spin_lock_irqsave(&intf->maintenance_mode_lock, flags); + intf->auto_maintenance_timeout + = maintenance_mode_timeout_ms; + if (!intf->maintenance_mode + && !intf->maintenance_mode_enable) { + intf->maintenance_mode_enable = true; + maintenance_mode_update(intf); + } + spin_unlock_irqrestore(&intf->maintenance_mode_lock, + flags); + } + + if (msg->data_len + 2 > IPMI_MAX_MSG_LENGTH) { + ipmi_inc_stat(intf, sent_invalid_commands); + return -EMSGSIZE; + } + + smi_msg->data[0] = (msg->netfn << 2) | (smi_addr->lun & 0x3); + smi_msg->data[1] = msg->cmd; + smi_msg->msgid = msgid; + smi_msg->user_data = recv_msg; + if (msg->data_len > 0) + memcpy(&smi_msg->data[2], msg->data, msg->data_len); + smi_msg->data_size = msg->data_len + 2; + ipmi_inc_stat(intf, sent_local_commands); + + return 0; +} + +static int i_ipmi_req_ipmb(ipmi_smi_t intf, + struct ipmi_addr *addr, + long msgid, + struct kernel_ipmi_msg *msg, + struct ipmi_smi_msg *smi_msg, + struct ipmi_recv_msg *recv_msg, + unsigned char source_address, + unsigned char source_lun, + int retries, + unsigned int retry_time_ms) +{ + struct ipmi_ipmb_addr *ipmb_addr; + unsigned char ipmb_seq; + long seqid; + int broadcast = 0; + struct ipmi_channel *chans; + int rv = 0; + + if (addr->channel >= IPMI_MAX_CHANNELS) { + ipmi_inc_stat(intf, sent_invalid_commands); + return -EINVAL; + } + + chans = READ_ONCE(intf->channel_list)->c; + + if (chans[addr->channel].medium != IPMI_CHANNEL_MEDIUM_IPMB) { + ipmi_inc_stat(intf, sent_invalid_commands); + return -EINVAL; + } + + if (addr->addr_type == IPMI_IPMB_BROADCAST_ADDR_TYPE) { + /* + * Broadcasts add a zero at the beginning of the + * message, but otherwise is the same as an IPMB + * address. + */ + addr->addr_type = IPMI_IPMB_ADDR_TYPE; + broadcast = 1; + retries = 0; /* Don't retry broadcasts. */ + } + + /* + * 9 for the header and 1 for the checksum, plus + * possibly one for the broadcast. + */ + if ((msg->data_len + 10 + broadcast) > IPMI_MAX_MSG_LENGTH) { + ipmi_inc_stat(intf, sent_invalid_commands); + return -EMSGSIZE; + } + + ipmb_addr = (struct ipmi_ipmb_addr *) addr; + if (ipmb_addr->lun > 3) { + ipmi_inc_stat(intf, sent_invalid_commands); + return -EINVAL; + } + + memcpy(&recv_msg->addr, ipmb_addr, sizeof(*ipmb_addr)); + + if (recv_msg->msg.netfn & 0x1) { + /* + * It's a response, so use the user's sequence + * from msgid. + */ + ipmi_inc_stat(intf, sent_ipmb_responses); + format_ipmb_msg(smi_msg, msg, ipmb_addr, msgid, + msgid, broadcast, + source_address, source_lun); + + /* + * Save the receive message so we can use it + * to deliver the response. + */ + smi_msg->user_data = recv_msg; + } else { + /* It's a command, so get a sequence for it. */ + unsigned long flags; + + spin_lock_irqsave(&intf->seq_lock, flags); + + if (is_maintenance_mode_cmd(msg)) + intf->ipmb_maintenance_mode_timeout = + maintenance_mode_timeout_ms; + + if (intf->ipmb_maintenance_mode_timeout && retry_time_ms == 0) + /* Different default in maintenance mode */ + retry_time_ms = default_maintenance_retry_ms; + + /* + * Create a sequence number with a 1 second + * timeout and 4 retries. + */ + rv = intf_next_seq(intf, + recv_msg, + retry_time_ms, + retries, + broadcast, + &ipmb_seq, + &seqid); + if (rv) + /* + * We have used up all the sequence numbers, + * probably, so abort. + */ + goto out_err; + + ipmi_inc_stat(intf, sent_ipmb_commands); + + /* + * Store the sequence number in the message, + * so that when the send message response + * comes back we can start the timer. + */ + format_ipmb_msg(smi_msg, msg, ipmb_addr, + STORE_SEQ_IN_MSGID(ipmb_seq, seqid), + ipmb_seq, broadcast, + source_address, source_lun); + + /* + * Copy the message into the recv message data, so we + * can retransmit it later if necessary. + */ + memcpy(recv_msg->msg_data, smi_msg->data, + smi_msg->data_size); + recv_msg->msg.data = recv_msg->msg_data; + recv_msg->msg.data_len = smi_msg->data_size; + + /* + * We don't unlock until here, because we need + * to copy the completed message into the + * recv_msg before we release the lock. + * Otherwise, race conditions may bite us. I + * know that's pretty paranoid, but I prefer + * to be correct. + */ +out_err: + spin_unlock_irqrestore(&intf->seq_lock, flags); + } + + return rv; +} + +static int i_ipmi_req_lan(ipmi_smi_t intf, + struct ipmi_addr *addr, + long msgid, + struct kernel_ipmi_msg *msg, + struct ipmi_smi_msg *smi_msg, + struct ipmi_recv_msg *recv_msg, + unsigned char source_lun, + int retries, + unsigned int retry_time_ms) +{ + struct ipmi_lan_addr *lan_addr; + unsigned char ipmb_seq; + long seqid; + struct ipmi_channel *chans; + int rv = 0; + + if (addr->channel >= IPMI_MAX_CHANNELS) { + ipmi_inc_stat(intf, sent_invalid_commands); + return -EINVAL; + } + + chans = READ_ONCE(intf->channel_list)->c; + + if ((chans[addr->channel].medium + != IPMI_CHANNEL_MEDIUM_8023LAN) + && (chans[addr->channel].medium + != IPMI_CHANNEL_MEDIUM_ASYNC)) { + ipmi_inc_stat(intf, sent_invalid_commands); + return -EINVAL; + } + + /* 11 for the header and 1 for the checksum. */ + if ((msg->data_len + 12) > IPMI_MAX_MSG_LENGTH) { + ipmi_inc_stat(intf, sent_invalid_commands); + return -EMSGSIZE; + } + + lan_addr = (struct ipmi_lan_addr *) addr; + if (lan_addr->lun > 3) { + ipmi_inc_stat(intf, sent_invalid_commands); + return -EINVAL; + } + + memcpy(&recv_msg->addr, lan_addr, sizeof(*lan_addr)); + + if (recv_msg->msg.netfn & 0x1) { + /* + * It's a response, so use the user's sequence + * from msgid. + */ + ipmi_inc_stat(intf, sent_lan_responses); + format_lan_msg(smi_msg, msg, lan_addr, msgid, + msgid, source_lun); + + /* + * Save the receive message so we can use it + * to deliver the response. + */ + smi_msg->user_data = recv_msg; + } else { + /* It's a command, so get a sequence for it. */ + unsigned long flags; + + spin_lock_irqsave(&intf->seq_lock, flags); + + /* + * Create a sequence number with a 1 second + * timeout and 4 retries. + */ + rv = intf_next_seq(intf, + recv_msg, + retry_time_ms, + retries, + 0, + &ipmb_seq, + &seqid); + if (rv) + /* + * We have used up all the sequence numbers, + * probably, so abort. + */ + goto out_err; + + ipmi_inc_stat(intf, sent_lan_commands); + + /* + * Store the sequence number in the message, + * so that when the send message response + * comes back we can start the timer. + */ + format_lan_msg(smi_msg, msg, lan_addr, + STORE_SEQ_IN_MSGID(ipmb_seq, seqid), + ipmb_seq, source_lun); + + /* + * Copy the message into the recv message data, so we + * can retransmit it later if necessary. + */ + memcpy(recv_msg->msg_data, smi_msg->data, + smi_msg->data_size); + recv_msg->msg.data = recv_msg->msg_data; + recv_msg->msg.data_len = smi_msg->data_size; + + /* + * We don't unlock until here, because we need + * to copy the completed message into the + * recv_msg before we release the lock. + * Otherwise, race conditions may bite us. I + * know that's pretty paranoid, but I prefer + * to be correct. + */ +out_err: + spin_unlock_irqrestore(&intf->seq_lock, flags); + } + + return rv; +} + /* * Separate from ipmi_request so that the user does not have to be * supplied in certain circumstances (mainly at panic time). If @@ -1723,11 +2049,9 @@ static int i_ipmi_request(ipmi_user_t user, int retries, unsigned int retry_time_ms) { - int rv = 0; - struct ipmi_smi_msg *smi_msg; - struct ipmi_recv_msg *recv_msg; - unsigned long flags; - + struct ipmi_smi_msg *smi_msg; + struct ipmi_recv_msg *recv_msg; + int rv = 0; if (supplied_recv) recv_msg = supplied_recv; @@ -1765,322 +2089,32 @@ static int i_ipmi_request(ipmi_user_t user, recv_msg->msg = *msg; if (addr->addr_type == IPMI_SYSTEM_INTERFACE_ADDR_TYPE) { - struct ipmi_system_interface_addr *smi_addr; - - if (msg->netfn & 1) { - /* Responses are not allowed to the SMI. */ - rv = -EINVAL; - goto out_err; - } - - smi_addr = (struct ipmi_system_interface_addr *) addr; - if (smi_addr->lun > 3) { - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EINVAL; - goto out_err; - } - - memcpy(&recv_msg->addr, smi_addr, sizeof(*smi_addr)); - - if ((msg->netfn == IPMI_NETFN_APP_REQUEST) - && ((msg->cmd == IPMI_SEND_MSG_CMD) - || (msg->cmd == IPMI_GET_MSG_CMD) - || (msg->cmd == IPMI_READ_EVENT_MSG_BUFFER_CMD))) { - /* - * We don't let the user do these, since we manage - * the sequence numbers. - */ - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EINVAL; - goto out_err; - } - - if (is_maintenance_mode_cmd(msg)) { - spin_lock_irqsave(&intf->maintenance_mode_lock, flags); - intf->auto_maintenance_timeout - = maintenance_mode_timeout_ms; - if (!intf->maintenance_mode - && !intf->maintenance_mode_enable) { - intf->maintenance_mode_enable = true; - maintenance_mode_update(intf); - } - spin_unlock_irqrestore(&intf->maintenance_mode_lock, - flags); - } - - if ((msg->data_len + 2) > IPMI_MAX_MSG_LENGTH) { - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EMSGSIZE; - goto out_err; - } - - smi_msg->data[0] = (msg->netfn << 2) | (smi_addr->lun & 0x3); - smi_msg->data[1] = msg->cmd; - smi_msg->msgid = msgid; - smi_msg->user_data = recv_msg; - if (msg->data_len > 0) - memcpy(&(smi_msg->data[2]), msg->data, msg->data_len); - smi_msg->data_size = msg->data_len + 2; - ipmi_inc_stat(intf, sent_local_commands); + rv = i_ipmi_req_sysintf(intf, addr, msgid, msg, smi_msg, + recv_msg, retries, retry_time_ms); } else if (is_ipmb_addr(addr) || is_ipmb_bcast_addr(addr)) { - struct ipmi_ipmb_addr *ipmb_addr; - unsigned char ipmb_seq; - long seqid; - int broadcast = 0; - struct ipmi_channel *chans; - - if (addr->channel >= IPMI_MAX_CHANNELS) { - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EINVAL; - goto out_err; - } - - chans = READ_ONCE(intf->channel_list)->c; - - if (chans[addr->channel].medium != IPMI_CHANNEL_MEDIUM_IPMB) { - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EINVAL; - goto out_err; - } - - if (addr->addr_type == IPMI_IPMB_BROADCAST_ADDR_TYPE) { - /* - * Broadcasts add a zero at the beginning of the - * message, but otherwise is the same as an IPMB - * address. - */ - addr->addr_type = IPMI_IPMB_ADDR_TYPE; - broadcast = 1; - retries = 0; /* Don't retry broadcasts. */ - } - - /* - * 9 for the header and 1 for the checksum, plus - * possibly one for the broadcast. - */ - if ((msg->data_len + 10 + broadcast) > IPMI_MAX_MSG_LENGTH) { - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EMSGSIZE; - goto out_err; - } - - ipmb_addr = (struct ipmi_ipmb_addr *) addr; - if (ipmb_addr->lun > 3) { - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EINVAL; - goto out_err; - } - - memcpy(&recv_msg->addr, ipmb_addr, sizeof(*ipmb_addr)); - - if (recv_msg->msg.netfn & 0x1) { - /* - * It's a response, so use the user's sequence - * from msgid. - */ - ipmi_inc_stat(intf, sent_ipmb_responses); - format_ipmb_msg(smi_msg, msg, ipmb_addr, msgid, - msgid, broadcast, - source_address, source_lun); - - /* - * Save the receive message so we can use it - * to deliver the response. - */ - smi_msg->user_data = recv_msg; - } else { - /* It's a command, so get a sequence for it. */ - - spin_lock_irqsave(&(intf->seq_lock), flags); - - if (is_maintenance_mode_cmd(msg)) - intf->ipmb_maintenance_mode_timeout = - maintenance_mode_timeout_ms; - - if (intf->ipmb_maintenance_mode_timeout && - retry_time_ms == 0) - /* Different default in maintenance mode */ - retry_time_ms = default_maintenance_retry_ms; - - /* - * Create a sequence number with a 1 second - * timeout and 4 retries. - */ - rv = intf_next_seq(intf, - recv_msg, - retry_time_ms, - retries, - broadcast, - &ipmb_seq, - &seqid); - if (rv) { - /* - * We have used up all the sequence numbers, - * probably, so abort. - */ - spin_unlock_irqrestore(&(intf->seq_lock), - flags); - goto out_err; - } - - ipmi_inc_stat(intf, sent_ipmb_commands); - - /* - * Store the sequence number in the message, - * so that when the send message response - * comes back we can start the timer. - */ - format_ipmb_msg(smi_msg, msg, ipmb_addr, - STORE_SEQ_IN_MSGID(ipmb_seq, seqid), - ipmb_seq, broadcast, - source_address, source_lun); - - /* - * Copy the message into the recv message data, so we - * can retransmit it later if necessary. - */ - memcpy(recv_msg->msg_data, smi_msg->data, - smi_msg->data_size); - recv_msg->msg.data = recv_msg->msg_data; - recv_msg->msg.data_len = smi_msg->data_size; - - /* - * We don't unlock until here, because we need - * to copy the completed message into the - * recv_msg before we release the lock. - * Otherwise, race conditions may bite us. I - * know that's pretty paranoid, but I prefer - * to be correct. - */ - spin_unlock_irqrestore(&(intf->seq_lock), flags); - } + rv = i_ipmi_req_ipmb(intf, addr, msgid, msg, smi_msg, recv_msg, + source_address, source_lun, + retries, retry_time_ms); } else if (is_lan_addr(addr)) { - struct ipmi_lan_addr *lan_addr; - unsigned char ipmb_seq; - long seqid; - struct ipmi_channel *chans; - - if (addr->channel >= IPMI_MAX_CHANNELS) { - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EINVAL; - goto out_err; - } - - chans = READ_ONCE(intf->channel_list)->c; - - if ((chans[addr->channel].medium - != IPMI_CHANNEL_MEDIUM_8023LAN) - && (chans[addr->channel].medium - != IPMI_CHANNEL_MEDIUM_ASYNC)) { - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EINVAL; - goto out_err; - } - - /* 11 for the header and 1 for the checksum. */ - if ((msg->data_len + 12) > IPMI_MAX_MSG_LENGTH) { - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EMSGSIZE; - goto out_err; - } - - lan_addr = (struct ipmi_lan_addr *) addr; - if (lan_addr->lun > 3) { - ipmi_inc_stat(intf, sent_invalid_commands); - rv = -EINVAL; - goto out_err; - } - - memcpy(&recv_msg->addr, lan_addr, sizeof(*lan_addr)); - - if (recv_msg->msg.netfn & 0x1) { - /* - * It's a response, so use the user's sequence - * from msgid. - */ - ipmi_inc_stat(intf, sent_lan_responses); - format_lan_msg(smi_msg, msg, lan_addr, msgid, - msgid, source_lun); - - /* - * Save the receive message so we can use it - * to deliver the response. - */ - smi_msg->user_data = recv_msg; - } else { - /* It's a command, so get a sequence for it. */ - - spin_lock_irqsave(&(intf->seq_lock), flags); - - /* - * Create a sequence number with a 1 second - * timeout and 4 retries. - */ - rv = intf_next_seq(intf, - recv_msg, - retry_time_ms, - retries, - 0, - &ipmb_seq, - &seqid); - if (rv) { - /* - * We have used up all the sequence numbers, - * probably, so abort. - */ - spin_unlock_irqrestore(&(intf->seq_lock), - flags); - goto out_err; - } - - ipmi_inc_stat(intf, sent_lan_commands); - - /* - * Store the sequence number in the message, - * so that when the send message response - * comes back we can start the timer. - */ - format_lan_msg(smi_msg, msg, lan_addr, - STORE_SEQ_IN_MSGID(ipmb_seq, seqid), - ipmb_seq, source_lun); - - /* - * Copy the message into the recv message data, so we - * can retransmit it later if necessary. - */ - memcpy(recv_msg->msg_data, smi_msg->data, - smi_msg->data_size); - recv_msg->msg.data = recv_msg->msg_data; - recv_msg->msg.data_len = smi_msg->data_size; - - /* - * We don't unlock until here, because we need - * to copy the completed message into the - * recv_msg before we release the lock. - * Otherwise, race conditions may bite us. I - * know that's pretty paranoid, but I prefer - * to be correct. - */ - spin_unlock_irqrestore(&(intf->seq_lock), flags); - } + rv = i_ipmi_req_lan(intf, addr, msgid, msg, smi_msg, recv_msg, + source_lun, retries, retry_time_ms); } else { /* Unknown address type. */ ipmi_inc_stat(intf, sent_invalid_commands); rv = -EINVAL; - goto out_err; } - ipmi_debug_msg("Send", smi_msg->data, smi_msg->data_size); + if (rv) { +out_err: + ipmi_free_smi_msg(smi_msg); + ipmi_free_recv_msg(recv_msg); + } else { + ipmi_debug_msg("Send", smi_msg->data, smi_msg->data_size); - smi_send(intf, intf->handlers, smi_msg, priority); + smi_send(intf, intf->handlers, smi_msg, priority); + } rcu_read_unlock(); - return 0; - - out_err: - rcu_read_unlock(); - ipmi_free_smi_msg(smi_msg); - ipmi_free_recv_msg(recv_msg); return rv; } From aa7a8f9e1b03c75c5288966770bd7f80cddb7b83 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 12:27:55 -0500 Subject: [PATCH 11/35] ipmi: Clean up some style issues in the message handler Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 83 ++++++++++++----------------- 1 file changed, 35 insertions(+), 48 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index e26e5b6900ee..a84f1389bab0 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -772,7 +772,7 @@ EXPORT_SYMBOL(ipmi_smi_watcher_register); int ipmi_smi_watcher_unregister(struct ipmi_smi_watcher *watcher) { mutex_lock(&smi_watchers_mutex); - list_del(&(watcher->link)); + list_del(&watcher->link); mutex_unlock(&smi_watchers_mutex); return 0; } @@ -994,19 +994,19 @@ static int intf_find_seq(ipmi_smi_t intf, if (seq >= IPMI_IPMB_NUM_SEQ) return -EINVAL; - spin_lock_irqsave(&(intf->seq_lock), flags); + spin_lock_irqsave(&intf->seq_lock, flags); if (intf->seq_table[seq].inuse) { struct ipmi_recv_msg *msg = intf->seq_table[seq].recv_msg; if ((msg->addr.channel == channel) && (msg->msg.cmd == cmd) && (msg->msg.netfn == netfn) - && (ipmi_addr_equal(addr, &(msg->addr)))) { + && (ipmi_addr_equal(addr, &msg->addr))) { *recv_msg = msg; intf->seq_table[seq].inuse = 0; rv = 0; } } - spin_unlock_irqrestore(&(intf->seq_lock), flags); + spin_unlock_irqrestore(&intf->seq_lock, flags); return rv; } @@ -1024,18 +1024,18 @@ static int intf_start_seq_timer(ipmi_smi_t intf, GET_SEQ_FROM_MSGID(msgid, seq, seqid); - spin_lock_irqsave(&(intf->seq_lock), flags); + spin_lock_irqsave(&intf->seq_lock, flags); /* * We do this verification because the user can be deleted * while a message is outstanding. */ if ((intf->seq_table[seq].inuse) && (intf->seq_table[seq].seqid == seqid)) { - struct seq_table *ent = &(intf->seq_table[seq]); + struct seq_table *ent = &intf->seq_table[seq]; ent->timeout = ent->orig_timeout; rv = 0; } - spin_unlock_irqrestore(&(intf->seq_lock), flags); + spin_unlock_irqrestore(&intf->seq_lock, flags); return rv; } @@ -1054,20 +1054,20 @@ static int intf_err_seq(ipmi_smi_t intf, GET_SEQ_FROM_MSGID(msgid, seq, seqid); - spin_lock_irqsave(&(intf->seq_lock), flags); + spin_lock_irqsave(&intf->seq_lock, flags); /* * We do this verification because the user can be deleted * while a message is outstanding. */ if ((intf->seq_table[seq].inuse) && (intf->seq_table[seq].seqid == seqid)) { - struct seq_table *ent = &(intf->seq_table[seq]); + struct seq_table *ent = &intf->seq_table[seq]; ent->inuse = 0; msg = ent->recv_msg; rv = 0; } - spin_unlock_irqrestore(&(intf->seq_lock), flags); + spin_unlock_irqrestore(&intf->seq_lock, flags); if (msg) deliver_err_response(msg, err); @@ -1594,21 +1594,19 @@ static inline void format_ipmb_msg(struct ipmi_smi_msg *smi_msg, smi_msg->data[3] = 0; smi_msg->data[i+3] = ipmb_addr->slave_addr; smi_msg->data[i+4] = (msg->netfn << 2) | (ipmb_addr->lun & 0x3); - smi_msg->data[i+5] = ipmb_checksum(&(smi_msg->data[i+3]), 2); + smi_msg->data[i+5] = ipmb_checksum(&smi_msg->data[i + 3], 2); smi_msg->data[i+6] = source_address; smi_msg->data[i+7] = (ipmb_seq << 2) | source_lun; smi_msg->data[i+8] = msg->cmd; /* Now tack on the data to the message. */ if (msg->data_len > 0) - memcpy(&(smi_msg->data[i+9]), msg->data, - msg->data_len); + memcpy(&smi_msg->data[i + 9], msg->data, msg->data_len); smi_msg->data_size = msg->data_len + 9; /* Now calculate the checksum and tack it on. */ smi_msg->data[i+smi_msg->data_size] - = ipmb_checksum(&(smi_msg->data[i+6]), - smi_msg->data_size-6); + = ipmb_checksum(&smi_msg->data[i + 6], smi_msg->data_size - 6); /* * Add on the checksum size and the offset from the @@ -1633,21 +1631,19 @@ static inline void format_lan_msg(struct ipmi_smi_msg *smi_msg, smi_msg->data[3] = lan_addr->session_handle; smi_msg->data[4] = lan_addr->remote_SWID; smi_msg->data[5] = (msg->netfn << 2) | (lan_addr->lun & 0x3); - smi_msg->data[6] = ipmb_checksum(&(smi_msg->data[4]), 2); + smi_msg->data[6] = ipmb_checksum(&smi_msg->data[4], 2); smi_msg->data[7] = lan_addr->local_SWID; smi_msg->data[8] = (ipmb_seq << 2) | source_lun; smi_msg->data[9] = msg->cmd; /* Now tack on the data to the message. */ if (msg->data_len > 0) - memcpy(&(smi_msg->data[10]), msg->data, - msg->data_len); + memcpy(&smi_msg->data[10], msg->data, msg->data_len); smi_msg->data_size = msg->data_len + 10; /* Now calculate the checksum and tack it on. */ smi_msg->data[smi_msg->data_size] - = ipmb_checksum(&(smi_msg->data[7]), - smi_msg->data_size-7); + = ipmb_checksum(&smi_msg->data[7], smi_msg->data_size - 7); /* * Add on the checksum size and the offset from the @@ -3612,7 +3608,7 @@ static void cleanup_smi_msgs(ipmi_smi_t intf) } for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) { - ent = &(intf->seq_table[i]); + ent = &intf->seq_table[i]; if (!ent->inuse) continue; deliver_err_response(ent->recv_msg, IPMI_ERR_UNSPECIFIED); @@ -3700,7 +3696,7 @@ static int handle_ipmb_get_msg_rsp(ipmi_smi_t intf, msg->rsp[3] & 0x0f, msg->rsp[8], (msg->rsp[4] >> 2) & (~1), - (struct ipmi_addr *) &(ipmb_addr), + (struct ipmi_addr *) &ipmb_addr, &recv_msg)) { /* * We were unable to find the sequence number, @@ -3710,9 +3706,7 @@ static int handle_ipmb_get_msg_rsp(ipmi_smi_t intf, return 0; } - memcpy(recv_msg->msg_data, - &(msg->rsp[9]), - msg->rsp_size - 9); + memcpy(recv_msg->msg_data, &msg->rsp[9], msg->rsp_size - 9); /* * The other fields matched, so no need to set them, except * for netfn, which needs to be the response that was @@ -3773,13 +3767,13 @@ static int handle_ipmb_get_msg_cmd(ipmi_smi_t intf, msg->data[2] = msg->rsp[3]; msg->data[3] = msg->rsp[6]; msg->data[4] = ((netfn + 1) << 2) | (msg->rsp[7] & 0x3); - msg->data[5] = ipmb_checksum(&(msg->data[3]), 2); + msg->data[5] = ipmb_checksum(&msg->data[3], 2); msg->data[6] = intf->addrinfo[msg->rsp[3] & 0xf].address; /* rqseq/lun */ msg->data[7] = (msg->rsp[7] & 0xfc) | (msg->rsp[4] & 0x3); msg->data[8] = msg->rsp[8]; /* cmd */ msg->data[9] = IPMI_INVALID_CMD_COMPLETION_CODE; - msg->data[10] = ipmb_checksum(&(msg->data[6]), 4); + msg->data[10] = ipmb_checksum(&msg->data[6], 4); msg->data_size = 11; ipmi_debug_msg("Invalid command:", msg->data, msg->data_size); @@ -3832,8 +3826,7 @@ static int handle_ipmb_get_msg_cmd(ipmi_smi_t intf, * at the end also needs to be removed. */ recv_msg->msg.data_len = msg->rsp_size - 10; - memcpy(recv_msg->msg_data, - &(msg->rsp[9]), + memcpy(recv_msg->msg_data, &msg->rsp[9], msg->rsp_size - 10); deliver_response(recv_msg); } @@ -3881,7 +3874,7 @@ static int handle_lan_get_msg_rsp(ipmi_smi_t intf, msg->rsp[3] & 0x0f, msg->rsp[10], (msg->rsp[6] >> 2) & (~1), - (struct ipmi_addr *) &(lan_addr), + (struct ipmi_addr *) &lan_addr, &recv_msg)) { /* * We were unable to find the sequence number, @@ -3891,9 +3884,7 @@ static int handle_lan_get_msg_rsp(ipmi_smi_t intf, return 0; } - memcpy(recv_msg->msg_data, - &(msg->rsp[11]), - msg->rsp_size - 11); + memcpy(recv_msg->msg_data, &msg->rsp[11], msg->rsp_size - 11); /* * The other fields matched, so no need to set them, except * for netfn, which needs to be the response that was @@ -3993,8 +3984,7 @@ static int handle_lan_get_msg_cmd(ipmi_smi_t intf, * at the end also needs to be removed. */ recv_msg->msg.data_len = msg->rsp_size - 12; - memcpy(recv_msg->msg_data, - &(msg->rsp[11]), + memcpy(recv_msg->msg_data, &msg->rsp[11], msg->rsp_size - 12); deliver_response(recv_msg); } @@ -4084,7 +4074,7 @@ static int handle_oem_get_msg_cmd(ipmi_smi_t intf, * requirements */ smi_addr = ((struct ipmi_system_interface_addr *) - &(recv_msg->addr)); + &recv_msg->addr); smi_addr->addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE; smi_addr->channel = IPMI_BMC_CHANNEL; smi_addr->lun = msg->rsp[0] & 3; @@ -4101,8 +4091,7 @@ static int handle_oem_get_msg_cmd(ipmi_smi_t intf, * the Channel Byte in the "GET MESSAGE" command */ recv_msg->msg.data_len = msg->rsp_size - 4; - memcpy(recv_msg->msg_data, - &(msg->rsp[4]), + memcpy(recv_msg->msg_data, &msg->rsp[4], msg->rsp_size - 4); deliver_response(recv_msg); } @@ -4117,14 +4106,14 @@ static void copy_event_into_recv_msg(struct ipmi_recv_msg *recv_msg, struct ipmi_system_interface_addr *smi_addr; recv_msg->msgid = 0; - smi_addr = (struct ipmi_system_interface_addr *) &(recv_msg->addr); + smi_addr = (struct ipmi_system_interface_addr *) &recv_msg->addr; smi_addr->addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE; smi_addr->channel = IPMI_BMC_CHANNEL; smi_addr->lun = msg->rsp[0] & 3; recv_msg->recv_type = IPMI_ASYNC_EVENT_RECV_TYPE; recv_msg->msg.netfn = msg->rsp[0] >> 2; recv_msg->msg.cmd = msg->rsp[1]; - memcpy(recv_msg->msg_data, &(msg->rsp[3]), msg->rsp_size - 3); + memcpy(recv_msg->msg_data, &msg->rsp[3], msg->rsp_size - 3); recv_msg->msg.data = recv_msg->msg_data; recv_msg->msg.data_len = msg->rsp_size - 3; } @@ -4187,7 +4176,7 @@ static int handle_read_event_rsp(ipmi_smi_t intf, copy_event_into_recv_msg(recv_msg, msg); recv_msg->user = user; kref_get(&user->refcount); - list_add_tail(&(recv_msg->link), &msgs); + list_add_tail(&recv_msg->link, &msgs); } rcu_read_unlock(); @@ -4214,7 +4203,7 @@ static int handle_read_event_rsp(ipmi_smi_t intf, } copy_event_into_recv_msg(recv_msg, msg); - list_add_tail(&(recv_msg->link), &(intf->waiting_events)); + list_add_tail(&recv_msg->link, &intf->waiting_events); intf->waiting_events_count++; } else if (!intf->event_msg_printed) { /* @@ -4227,7 +4216,7 @@ static int handle_read_event_rsp(ipmi_smi_t intf, } out: - spin_unlock_irqrestore(&(intf->events_lock), flags); + spin_unlock_irqrestore(&intf->events_lock, flags); return rv; } @@ -4258,15 +4247,13 @@ static int handle_bmc_rsp(ipmi_smi_t intf, recv_msg->recv_type = IPMI_RESPONSE_RECV_TYPE; recv_msg->msgid = msg->msgid; smi_addr = ((struct ipmi_system_interface_addr *) - &(recv_msg->addr)); + &recv_msg->addr); smi_addr->addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE; smi_addr->channel = IPMI_BMC_CHANNEL; smi_addr->lun = msg->rsp[0] & 3; recv_msg->msg.netfn = msg->rsp[0] >> 2; recv_msg->msg.cmd = msg->rsp[1]; - memcpy(recv_msg->msg_data, - &(msg->rsp[2]), - msg->rsp_size - 2); + memcpy(recv_msg->msg_data, &msg->rsp[2], msg->rsp_size - 2); recv_msg->msg.data = recv_msg->msg_data; recv_msg->msg.data_len = msg->rsp_size - 2; deliver_response(recv_msg); @@ -4765,7 +4752,7 @@ static unsigned int ipmi_timeout_handler(ipmi_smi_t intf, intf->ipmb_maintenance_mode_timeout -= timeout_period; } for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) - check_msg_timeout(intf, &(intf->seq_table[i]), + check_msg_timeout(intf, &intf->seq_table[i], &timeouts, timeout_period, i, &flags, &waiting_msgs); spin_unlock_irqrestore(&intf->seq_lock, flags); From 6aa2dd0092d2762cc12cccf5142313858f4153a4 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 14:40:30 -0500 Subject: [PATCH 12/35] ipmi_devintf: Small lock rework The mutex didn't really serve any useful purpose, from what I can tell, and it would just get in the way. So remove it. Removing that required a mutex around the default value setting and getting, so just use the receive mutex for that. Also pull the fasync stuff outside of the lock for adding the data to the queue, since it didn't need to be there. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_devintf.c | 83 +++++++++++--------------------- 1 file changed, 28 insertions(+), 55 deletions(-) diff --git a/drivers/char/ipmi/ipmi_devintf.c b/drivers/char/ipmi/ipmi_devintf.c index f862838e2c10..71698365591f 100644 --- a/drivers/char/ipmi/ipmi_devintf.c +++ b/drivers/char/ipmi/ipmi_devintf.c @@ -37,7 +37,6 @@ struct ipmi_file_private unsigned int default_retry_time_ms; }; -static DEFINE_MUTEX(ipmi_mutex); static void file_receive_handler(struct ipmi_recv_msg *msg, void *handler_data) { @@ -46,16 +45,14 @@ static void file_receive_handler(struct ipmi_recv_msg *msg, unsigned long flags; spin_lock_irqsave(&priv->recv_msg_lock, flags); - was_empty = list_empty(&priv->recv_msgs); list_add_tail(&msg->link, &priv->recv_msgs); + spin_unlock_irqrestore(&priv->recv_msg_lock, flags); if (was_empty) { wake_up_interruptible(&priv->wait); kill_fasync(&priv->fasync_queue, SIGIO, POLL_IN); } - - spin_unlock_irqrestore(&priv->recv_msg_lock, flags); } static __poll_t ipmi_poll(struct file *file, poll_table *wait) @@ -79,13 +76,8 @@ static __poll_t ipmi_poll(struct file *file, poll_table *wait) static int ipmi_fasync(int fd, struct file *file, int on) { struct ipmi_file_private *priv = file->private_data; - int result; - mutex_lock(&ipmi_mutex); /* could race against open() otherwise */ - result = fasync_helper(fd, file, on, &priv->fasync_queue); - mutex_unlock(&ipmi_mutex); - - return (result); + return fasync_helper(fd, file, on, &priv->fasync_queue); } static const struct ipmi_user_hndl ipmi_hndlrs = @@ -99,12 +91,10 @@ static int ipmi_open(struct inode *inode, struct file *file) int rv; struct ipmi_file_private *priv; - priv = kmalloc(sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; - mutex_lock(&ipmi_mutex); priv->file = file; rv = ipmi_create_user(if_num, @@ -129,7 +119,6 @@ static int ipmi_open(struct inode *inode, struct file *file) priv->default_retry_time_ms = 0; out: - mutex_unlock(&ipmi_mutex); return rv; } @@ -137,7 +126,7 @@ static int ipmi_release(struct inode *inode, struct file *file) { struct ipmi_file_private *priv = file->private_data; int rv; - struct ipmi_recv_msg *msg, *next; + struct ipmi_recv_msg *msg, *next; rv = ipmi_destroy_user(priv->user); if (rv) @@ -303,9 +292,9 @@ static int copyout_recv(struct ipmi_recv *rsp, void __user *to) return copy_to_user(to, rsp, sizeof(struct ipmi_recv)) ? -EFAULT : 0; } -static int ipmi_ioctl(struct file *file, - unsigned int cmd, - unsigned long data) +static long ipmi_ioctl(struct file *file, + unsigned int cmd, + unsigned long data) { int rv = -EINVAL; struct ipmi_file_private *priv = file->private_data; @@ -316,16 +305,20 @@ static int ipmi_ioctl(struct file *file, case IPMICTL_SEND_COMMAND: { struct ipmi_req req; + int retries; + unsigned int retry_time_ms; if (copy_from_user(&req, arg, sizeof(req))) { rv = -EFAULT; break; } - rv = handle_send_req(priv->user, - &req, - priv->default_retries, - priv->default_retry_time_ms); + mutex_lock(&priv->recv_mutex); + retries = priv->default_retries; + retry_time_ms = priv->default_retry_time_ms; + mutex_unlock(&priv->recv_mutex); + + rv = handle_send_req(priv->user, &req, retries, retry_time_ms); break; } @@ -565,8 +558,10 @@ static int ipmi_ioctl(struct file *file, break; } + mutex_lock(&priv->recv_mutex); priv->default_retries = parms.retries; priv->default_retry_time_ms = parms.retry_time_ms; + mutex_unlock(&priv->recv_mutex); rv = 0; break; } @@ -575,8 +570,10 @@ static int ipmi_ioctl(struct file *file, { struct ipmi_timing_parms parms; + mutex_lock(&priv->recv_mutex); parms.retries = priv->default_retries; parms.retry_time_ms = priv->default_retry_time_ms; + mutex_unlock(&priv->recv_mutex); if (copy_to_user(arg, &parms, sizeof(parms))) { rv = -EFAULT; @@ -616,25 +613,7 @@ static int ipmi_ioctl(struct file *file, return rv; } -/* - * Note: it doesn't make sense to take the BKL here but - * not in compat_ipmi_ioctl. -arnd - */ -static long ipmi_unlocked_ioctl(struct file *file, - unsigned int cmd, - unsigned long data) -{ - int ret; - - mutex_lock(&ipmi_mutex); - ret = ipmi_ioctl(file, cmd, data); - mutex_unlock(&ipmi_mutex); - - return ret; -} - #ifdef CONFIG_COMPAT - /* * The following code contains code for supporting 32-bit compatible * ioctls on 64-bit kernels. This allows running 32-bit apps on the @@ -745,15 +724,21 @@ static long compat_ipmi_ioctl(struct file *filep, unsigned int cmd, { struct ipmi_req rp; struct compat_ipmi_req r32; + int retries; + unsigned int retry_time_ms; if (copy_from_user(&r32, compat_ptr(arg), sizeof(r32))) return -EFAULT; get_compat_ipmi_req(&rp, &r32); + mutex_lock(&priv->recv_mutex); + retries = priv->default_retries; + retry_time_ms = priv->default_retry_time_ms; + mutex_unlock(&priv->recv_mutex); + return handle_send_req(priv->user, &rp, - priv->default_retries, - priv->default_retry_time_ms); + retries, retry_time_ms); } case COMPAT_IPMICTL_SEND_COMMAND_SETTIME: { @@ -787,25 +772,13 @@ static long compat_ipmi_ioctl(struct file *filep, unsigned int cmd, return ipmi_ioctl(filep, cmd, arg); } } - -static long unlocked_compat_ipmi_ioctl(struct file *filep, unsigned int cmd, - unsigned long arg) -{ - int ret; - - mutex_lock(&ipmi_mutex); - ret = compat_ipmi_ioctl(filep, cmd, arg); - mutex_unlock(&ipmi_mutex); - - return ret; -} #endif static const struct file_operations ipmi_fops = { .owner = THIS_MODULE, - .unlocked_ioctl = ipmi_unlocked_ioctl, + .unlocked_ioctl = ipmi_ioctl, #ifdef CONFIG_COMPAT - .compat_ioctl = unlocked_compat_ipmi_ioctl, + .compat_ioctl = compat_ipmi_ioctl, #endif .open = ipmi_open, .release = ipmi_release, From b7780dab90e8da302bc7d0d1b39b538b822017e8 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 16:44:12 -0500 Subject: [PATCH 13/35] ipmi: Add shutdown functions for users and interfaces Since things that IPMI uses can be hot-swapped, the users and interfaces really need to be able to handle this. Add the functions so the users and interfaces can implement them, the actual function will be added after everything is ready. Signed-off-by: Corey Minyard --- include/linux/ipmi.h | 8 ++++++++ include/linux/ipmi_smi.h | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/include/linux/ipmi.h b/include/linux/ipmi.h index 3474f04cf9aa..d89bea1e457a 100644 --- a/include/linux/ipmi.h +++ b/include/linux/ipmi.h @@ -97,6 +97,14 @@ struct ipmi_user_hndl { * been set up to handle run to completion. */ void (*ipmi_panic_handler)(void *handler_data); + + /* + * Called when the interface has been removed. After this returns + * the user handle will be invalid. The interface may or may + * not be usable when this is called, but it will return errors + * if it is not usable. + */ + void (*shutdown)(void *handler_data); }; /* Create a new user of the IPMI layer on the given interface number. */ diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h index 9e5c3079d232..bdcda4741c89 100644 --- a/include/linux/ipmi_smi.h +++ b/include/linux/ipmi_smi.h @@ -72,6 +72,12 @@ struct ipmi_smi_handlers { int (*start_processing)(void *send_info, ipmi_smi_t new_intf); + /* + * When called, the low-level interface should disable all + * processing, it should be complete shut down when it returns. + */ + void (*shutdown)(void *send_info); + /* * Get the detailed private info of the low level interface and store * it into the structure of ipmi_smi_data. For example: the From 2911c9886c8d5435a8778ce4888f6e4b8d7c900c Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 20:50:48 -0500 Subject: [PATCH 14/35] ipmi: Rename ipmi_user_t to struct ipmi_user * Get rid of that non-compliance in the user files. Include files will come later. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_devintf.c | 4 +-- drivers/char/ipmi/ipmi_msghandler.c | 54 ++++++++++++++--------------- drivers/char/ipmi/ipmi_poweroff.c | 32 ++++++++--------- drivers/char/ipmi/ipmi_watchdog.c | 4 +-- 4 files changed, 47 insertions(+), 47 deletions(-) diff --git a/drivers/char/ipmi/ipmi_devintf.c b/drivers/char/ipmi/ipmi_devintf.c index 71698365591f..b36a91fb7cd5 100644 --- a/drivers/char/ipmi/ipmi_devintf.c +++ b/drivers/char/ipmi/ipmi_devintf.c @@ -26,7 +26,7 @@ struct ipmi_file_private { - ipmi_user_t user; + struct ipmi_user *user; spinlock_t recv_msg_lock; struct list_head recv_msgs; struct file *file; @@ -140,7 +140,7 @@ static int ipmi_release(struct inode *inode, struct file *file) return 0; } -static int handle_send_req(ipmi_user_t user, +static int handle_send_req(struct ipmi_user *user, struct ipmi_req *req, int retries, unsigned int retry_time_ms) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index a84f1389bab0..97045e6c5b96 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -216,7 +216,7 @@ struct ipmi_user { struct cmd_rcvr { struct list_head link; - ipmi_user_t user; + struct ipmi_user *user; unsigned char netfn; unsigned char cmd; unsigned int chans; @@ -906,7 +906,7 @@ static void deliver_response(struct ipmi_recv_msg *msg) * risk. At this moment, simply skip it in that case. */ - ipmi_user_t user = msg->user; + struct ipmi_user *user = msg->user; user->handler->ipmi_recv_hndl(msg, user->handler_data); } } @@ -1079,10 +1079,10 @@ static int intf_err_seq(ipmi_smi_t intf, int ipmi_create_user(unsigned int if_num, const struct ipmi_user_hndl *handler, void *handler_data, - ipmi_user_t *user) + struct ipmi_user **user) { unsigned long flags; - ipmi_user_t new_user; + struct ipmi_user *new_user; int rv = 0; ipmi_smi_t intf; @@ -1206,11 +1206,11 @@ EXPORT_SYMBOL(ipmi_get_smi_info); static void free_user(struct kref *ref) { - ipmi_user_t user = container_of(ref, struct ipmi_user, refcount); + struct ipmi_user *user = container_of(ref, struct ipmi_user, refcount); kfree(user); } -int ipmi_destroy_user(ipmi_user_t user) +int ipmi_destroy_user(struct ipmi_user *user) { ipmi_smi_t intf = user->intf; int i; @@ -1277,7 +1277,7 @@ int ipmi_destroy_user(ipmi_user_t user) } EXPORT_SYMBOL(ipmi_destroy_user); -int ipmi_get_version(ipmi_user_t user, +int ipmi_get_version(struct ipmi_user *user, unsigned char *major, unsigned char *minor) { @@ -1295,7 +1295,7 @@ int ipmi_get_version(ipmi_user_t user, } EXPORT_SYMBOL(ipmi_get_version); -int ipmi_set_my_address(ipmi_user_t user, +int ipmi_set_my_address(struct ipmi_user *user, unsigned int channel, unsigned char address) { @@ -1306,7 +1306,7 @@ int ipmi_set_my_address(ipmi_user_t user, } EXPORT_SYMBOL(ipmi_set_my_address); -int ipmi_get_my_address(ipmi_user_t user, +int ipmi_get_my_address(struct ipmi_user *user, unsigned int channel, unsigned char *address) { @@ -1317,7 +1317,7 @@ int ipmi_get_my_address(ipmi_user_t user, } EXPORT_SYMBOL(ipmi_get_my_address); -int ipmi_set_my_LUN(ipmi_user_t user, +int ipmi_set_my_LUN(struct ipmi_user *user, unsigned int channel, unsigned char LUN) { @@ -1328,7 +1328,7 @@ int ipmi_set_my_LUN(ipmi_user_t user, } EXPORT_SYMBOL(ipmi_set_my_LUN); -int ipmi_get_my_LUN(ipmi_user_t user, +int ipmi_get_my_LUN(struct ipmi_user *user, unsigned int channel, unsigned char *address) { @@ -1339,7 +1339,7 @@ int ipmi_get_my_LUN(ipmi_user_t user, } EXPORT_SYMBOL(ipmi_get_my_LUN); -int ipmi_get_maintenance_mode(ipmi_user_t user) +int ipmi_get_maintenance_mode(struct ipmi_user *user) { int mode; unsigned long flags; @@ -1359,7 +1359,7 @@ static void maintenance_mode_update(ipmi_smi_t intf) intf->send_info, intf->maintenance_mode_enable); } -int ipmi_set_maintenance_mode(ipmi_user_t user, int mode) +int ipmi_set_maintenance_mode(struct ipmi_user *user, int mode) { int rv = 0; unsigned long flags; @@ -1396,7 +1396,7 @@ int ipmi_set_maintenance_mode(ipmi_user_t user, int mode) } EXPORT_SYMBOL(ipmi_set_maintenance_mode); -int ipmi_set_gets_events(ipmi_user_t user, bool val) +int ipmi_set_gets_events(struct ipmi_user *user, bool val) { unsigned long flags; ipmi_smi_t intf = user->intf; @@ -1486,7 +1486,7 @@ static int is_cmd_rcvr_exclusive(ipmi_smi_t intf, return 1; } -int ipmi_register_for_cmd(ipmi_user_t user, +int ipmi_register_for_cmd(struct ipmi_user *user, unsigned char netfn, unsigned char cmd, unsigned int chans) @@ -1525,7 +1525,7 @@ int ipmi_register_for_cmd(ipmi_user_t user, } EXPORT_SYMBOL(ipmi_register_for_cmd); -int ipmi_unregister_for_cmd(ipmi_user_t user, +int ipmi_unregister_for_cmd(struct ipmi_user *user, unsigned char netfn, unsigned char cmd, unsigned int chans) @@ -2031,7 +2031,7 @@ static int i_ipmi_req_lan(ipmi_smi_t intf, * messages are supplied, they will be freed, even if an error * occurs. */ -static int i_ipmi_request(ipmi_user_t user, +static int i_ipmi_request(struct ipmi_user *user, ipmi_smi_t intf, struct ipmi_addr *addr, long msgid, @@ -2126,7 +2126,7 @@ static int check_addr(ipmi_smi_t intf, return 0; } -int ipmi_request_settime(ipmi_user_t user, +int ipmi_request_settime(struct ipmi_user *user, struct ipmi_addr *addr, long msgid, struct kernel_ipmi_msg *msg, @@ -2158,7 +2158,7 @@ int ipmi_request_settime(ipmi_user_t user, } EXPORT_SYMBOL(ipmi_request_settime); -int ipmi_request_supply_msgs(ipmi_user_t user, +int ipmi_request_supply_msgs(struct ipmi_user *user, struct ipmi_addr *addr, long msgid, struct kernel_ipmi_msg *msg, @@ -3394,7 +3394,7 @@ static void ipmi_poll(ipmi_smi_t intf) handle_new_recv_msgs(intf); } -void ipmi_poll_interface(ipmi_user_t user) +void ipmi_poll_interface(struct ipmi_user *user) { ipmi_poll(user->intf); } @@ -3619,7 +3619,7 @@ int ipmi_unregister_smi(ipmi_smi_t intf) { struct ipmi_smi_watcher *w; int intf_num = intf->intf_num; - ipmi_user_t user; + struct ipmi_user *user; mutex_lock(&smi_watchers_mutex); mutex_lock(&ipmi_interfaces_mutex); @@ -3730,7 +3730,7 @@ static int handle_ipmb_get_msg_cmd(ipmi_smi_t intf, unsigned char netfn; unsigned char cmd; unsigned char chan; - ipmi_user_t user = NULL; + struct ipmi_user *user = NULL; struct ipmi_ipmb_addr *ipmb_addr; struct ipmi_recv_msg *recv_msg; @@ -3908,7 +3908,7 @@ static int handle_lan_get_msg_cmd(ipmi_smi_t intf, unsigned char netfn; unsigned char cmd; unsigned char chan; - ipmi_user_t user = NULL; + struct ipmi_user *user = NULL; struct ipmi_lan_addr *lan_addr; struct ipmi_recv_msg *recv_msg; @@ -4007,7 +4007,7 @@ static int handle_oem_get_msg_cmd(ipmi_smi_t intf, unsigned char netfn; unsigned char cmd; unsigned char chan; - ipmi_user_t user = NULL; + struct ipmi_user *user = NULL; struct ipmi_system_interface_addr *smi_addr; struct ipmi_recv_msg *recv_msg; @@ -4123,7 +4123,7 @@ static int handle_read_event_rsp(ipmi_smi_t intf, { struct ipmi_recv_msg *recv_msg, *recv_msg2; struct list_head msgs; - ipmi_user_t user; + struct ipmi_user *user; int rv = 0; int deliver_count = 0; unsigned long flags; @@ -4470,7 +4470,7 @@ static void handle_new_recv_msgs(ipmi_smi_t intf) * deliver pretimeouts to all the users. */ if (atomic_add_unless(&intf->watchdog_pretimeouts_to_deliver, -1, 0)) { - ipmi_user_t user; + struct ipmi_user *user; rcu_read_lock(); list_for_each_entry_rcu(user, &intf->users, link) { @@ -5122,7 +5122,7 @@ static int panic_event(struct notifier_block *this, void *ptr) { ipmi_smi_t intf; - ipmi_user_t user; + struct ipmi_user *user; if (has_panicked) return NOTIFY_DONE; diff --git a/drivers/char/ipmi/ipmi_poweroff.c b/drivers/char/ipmi/ipmi_poweroff.c index 7996337852f2..f6e19410dc57 100644 --- a/drivers/char/ipmi/ipmi_poweroff.c +++ b/drivers/char/ipmi/ipmi_poweroff.c @@ -39,9 +39,9 @@ static int ifnum_to_use = -1; /* Our local state. */ static int ready; -static ipmi_user_t ipmi_user; +static struct ipmi_user *ipmi_user; static int ipmi_ifnum; -static void (*specific_poweroff_func)(ipmi_user_t user); +static void (*specific_poweroff_func)(struct ipmi_user *user); /* Holds the old poweroff function so we can restore it on removal. */ static void (*old_poweroff_func)(void); @@ -118,7 +118,7 @@ static const struct ipmi_user_hndl ipmi_poweroff_handler = { }; -static int ipmi_request_wait_for_response(ipmi_user_t user, +static int ipmi_request_wait_for_response(struct ipmi_user *user, struct ipmi_addr *addr, struct kernel_ipmi_msg *send_msg) { @@ -138,7 +138,7 @@ static int ipmi_request_wait_for_response(ipmi_user_t user, } /* Wait for message to complete, spinning. */ -static int ipmi_request_in_rc_mode(ipmi_user_t user, +static int ipmi_request_in_rc_mode(struct ipmi_user *user, struct ipmi_addr *addr, struct kernel_ipmi_msg *send_msg) { @@ -178,9 +178,9 @@ static int ipmi_request_in_rc_mode(ipmi_user_t user, #define IPMI_MOTOROLA_MANUFACTURER_ID 0x0000A1 #define IPMI_MOTOROLA_PPS_IPMC_PRODUCT_ID 0x0051 -static void (*atca_oem_poweroff_hook)(ipmi_user_t user); +static void (*atca_oem_poweroff_hook)(struct ipmi_user *user); -static void pps_poweroff_atca(ipmi_user_t user) +static void pps_poweroff_atca(struct ipmi_user *user) { struct ipmi_system_interface_addr smi_addr; struct kernel_ipmi_msg send_msg; @@ -208,7 +208,7 @@ static void pps_poweroff_atca(ipmi_user_t user) return; } -static int ipmi_atca_detect(ipmi_user_t user) +static int ipmi_atca_detect(struct ipmi_user *user) { struct ipmi_system_interface_addr smi_addr; struct kernel_ipmi_msg send_msg; @@ -245,7 +245,7 @@ static int ipmi_atca_detect(ipmi_user_t user) return !rv; } -static void ipmi_poweroff_atca(ipmi_user_t user) +static void ipmi_poweroff_atca(struct ipmi_user *user) { struct ipmi_system_interface_addr smi_addr; struct kernel_ipmi_msg send_msg; @@ -309,13 +309,13 @@ static void ipmi_poweroff_atca(ipmi_user_t user) #define IPMI_CPI1_PRODUCT_ID 0x000157 #define IPMI_CPI1_MANUFACTURER_ID 0x0108 -static int ipmi_cpi1_detect(ipmi_user_t user) +static int ipmi_cpi1_detect(struct ipmi_user *user) { return ((mfg_id == IPMI_CPI1_MANUFACTURER_ID) && (prod_id == IPMI_CPI1_PRODUCT_ID)); } -static void ipmi_poweroff_cpi1(ipmi_user_t user) +static void ipmi_poweroff_cpi1(struct ipmi_user *user) { struct ipmi_system_interface_addr smi_addr; struct ipmi_ipmb_addr ipmb_addr; @@ -424,7 +424,7 @@ static void ipmi_poweroff_cpi1(ipmi_user_t user) */ #define DELL_IANA_MFR_ID {0xA2, 0x02, 0x00} -static int ipmi_dell_chassis_detect(ipmi_user_t user) +static int ipmi_dell_chassis_detect(struct ipmi_user *user) { const char ipmi_version_major = ipmi_version & 0xF; const char ipmi_version_minor = (ipmi_version >> 4) & 0xF; @@ -445,7 +445,7 @@ static int ipmi_dell_chassis_detect(ipmi_user_t user) #define HP_IANA_MFR_ID 0x0b #define HP_BMC_PROD_ID 0x8201 -static int ipmi_hp_chassis_detect(ipmi_user_t user) +static int ipmi_hp_chassis_detect(struct ipmi_user *user) { if (mfg_id == HP_IANA_MFR_ID && prod_id == HP_BMC_PROD_ID @@ -461,13 +461,13 @@ static int ipmi_hp_chassis_detect(ipmi_user_t user) #define IPMI_NETFN_CHASSIS_REQUEST 0 #define IPMI_CHASSIS_CONTROL_CMD 0x02 -static int ipmi_chassis_detect(ipmi_user_t user) +static int ipmi_chassis_detect(struct ipmi_user *user) { /* Chassis support, use it. */ return (capabilities & 0x80); } -static void ipmi_poweroff_chassis(ipmi_user_t user) +static void ipmi_poweroff_chassis(struct ipmi_user *user) { struct ipmi_system_interface_addr smi_addr; struct kernel_ipmi_msg send_msg; @@ -517,8 +517,8 @@ static void ipmi_poweroff_chassis(ipmi_user_t user) /* Table of possible power off functions. */ struct poweroff_function { char *platform_type; - int (*detect)(ipmi_user_t user); - void (*poweroff_func)(ipmi_user_t user); + int (*detect)(struct ipmi_user *user); + void (*poweroff_func)(struct ipmi_user *user); }; static struct poweroff_function poweroff_functions[] = { diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c index ab3eff581052..ca1c5c5109f0 100644 --- a/drivers/char/ipmi/ipmi_watchdog.c +++ b/drivers/char/ipmi/ipmi_watchdog.c @@ -125,7 +125,7 @@ static DEFINE_MUTEX(ipmi_watchdog_mutex); static bool nowayout = WATCHDOG_NOWAYOUT; -static ipmi_user_t watchdog_user; +static struct ipmi_user *watchdog_user; static int watchdog_ifnum; /* Default the timeout to 10 seconds. */ @@ -1060,7 +1060,7 @@ static void ipmi_register_watchdog(int ipmi_intf) static void ipmi_unregister_watchdog(int ipmi_intf) { int rv; - ipmi_user_t loc_user = watchdog_user; + struct ipmi_user *loc_user = watchdog_user; if (!loc_user) return; From a567b6230066e3a2c964e2dc914e7f9a436806c4 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 20:54:04 -0500 Subject: [PATCH 15/35] ipmi: Change ipmi_smi_t to struct ipmi_smi * Get rid of this coding style violation in the user files. Include files will come later. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 178 ++++++++++++++-------------- drivers/char/ipmi/ipmi_si_intf.c | 6 +- drivers/char/ipmi/ipmi_ssif.c | 10 +- 3 files changed, 99 insertions(+), 95 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 97045e6c5b96..9fa04b7afbd0 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -37,9 +37,9 @@ static struct ipmi_recv_msg *ipmi_alloc_recv_msg(void); static int ipmi_init_msghandler(void); static void smi_recv_tasklet(unsigned long); -static void handle_new_recv_msgs(ipmi_smi_t intf); -static void need_waiter(ipmi_smi_t intf); -static int handle_one_recv_msg(ipmi_smi_t intf, +static void handle_new_recv_msgs(struct ipmi_smi *intf); +static void need_waiter(struct ipmi_smi *intf); +static int handle_one_recv_msg(struct ipmi_smi *intf, struct ipmi_smi_msg *msg); #ifdef DEBUG @@ -207,7 +207,7 @@ struct ipmi_user { void *handler_data; /* The interface this user is bound to. */ - ipmi_smi_t intf; + struct ipmi_smi *intf; /* Does this interface receive IPMI events? */ bool gets_events; @@ -322,7 +322,7 @@ struct bmc_device { }; #define to_bmc_device(x) container_of((x), struct bmc_device, pdev.dev) -static int bmc_get_device_id(ipmi_smi_t intf, struct bmc_device *bmc, +static int bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc, struct ipmi_device_id *id, bool *guid_set, guid_t *guid); @@ -564,7 +564,8 @@ struct ipmi_smi { * * Protected by bmc_reg_mutex. */ - void (*null_user_handler)(ipmi_smi_t intf, struct ipmi_recv_msg *msg); + void (*null_user_handler)(struct ipmi_smi *intf, + struct ipmi_recv_msg *msg); /* * When we are scanning the channels for an SMI, this will @@ -590,12 +591,12 @@ struct ipmi_smi { }; #define to_si_intf_from_dev(device) container_of(device, struct ipmi_smi, dev) -static void __get_guid(ipmi_smi_t intf); -static void __ipmi_bmc_unregister(ipmi_smi_t intf); -static int __ipmi_bmc_register(ipmi_smi_t intf, +static void __get_guid(struct ipmi_smi *intf); +static void __ipmi_bmc_unregister(struct ipmi_smi *intf); +static int __ipmi_bmc_register(struct ipmi_smi *intf, struct ipmi_device_id *id, bool guid_set, guid_t *guid, int intf_num); -static int __scan_channels(ipmi_smi_t intf, struct ipmi_device_id *id); +static int __scan_channels(struct ipmi_smi *intf, struct ipmi_device_id *id); /** @@ -674,7 +675,7 @@ static void free_smi_msg_list(struct list_head *q) } } -static void clean_up_interface_data(ipmi_smi_t intf) +static void clean_up_interface_data(struct ipmi_smi *intf) { int i; struct cmd_rcvr *rcvr, *rcvr2; @@ -706,7 +707,7 @@ static void clean_up_interface_data(ipmi_smi_t intf) static void intf_free(struct kref *ref) { - ipmi_smi_t intf = container_of(ref, struct ipmi_smi, refcount); + struct ipmi_smi *intf = container_of(ref, struct ipmi_smi, refcount); clean_up_interface_data(intf); kfree(intf); @@ -714,13 +715,13 @@ static void intf_free(struct kref *ref) struct watcher_entry { int intf_num; - ipmi_smi_t intf; + struct ipmi_smi *intf; struct list_head link; }; int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher) { - ipmi_smi_t intf; + struct ipmi_smi *intf; LIST_HEAD(to_deliver); struct watcher_entry *e, *e2; @@ -888,7 +889,7 @@ EXPORT_SYMBOL(ipmi_addr_length); static void deliver_response(struct ipmi_recv_msg *msg) { if (!msg->user) { - ipmi_smi_t intf = msg->user_msg_data; + struct ipmi_smi *intf = msg->user_msg_data; /* Special handling for NULL users. */ if (intf->null_user_handler) { @@ -927,7 +928,7 @@ deliver_err_response(struct ipmi_recv_msg *msg, int err) * message with the given timeout to the sequence table. This must be * called with the interface's seq_lock held. */ -static int intf_next_seq(ipmi_smi_t intf, +static int intf_next_seq(struct ipmi_smi *intf, struct ipmi_recv_msg *recv_msg, unsigned long timeout, int retries, @@ -980,7 +981,7 @@ static int intf_next_seq(ipmi_smi_t intf, * guard against message coming in after their timeout and the * sequence number being reused). */ -static int intf_find_seq(ipmi_smi_t intf, +static int intf_find_seq(struct ipmi_smi *intf, unsigned char seq, short channel, unsigned char cmd, @@ -1013,7 +1014,7 @@ static int intf_find_seq(ipmi_smi_t intf, /* Start the timer for a specific sequence table entry. */ -static int intf_start_seq_timer(ipmi_smi_t intf, +static int intf_start_seq_timer(struct ipmi_smi *intf, long msgid) { int rv = -ENODEV; @@ -1041,7 +1042,7 @@ static int intf_start_seq_timer(ipmi_smi_t intf, } /* Got an error for the send message for a specific sequence number. */ -static int intf_err_seq(ipmi_smi_t intf, +static int intf_err_seq(struct ipmi_smi *intf, long msgid, unsigned int err) { @@ -1084,7 +1085,7 @@ int ipmi_create_user(unsigned int if_num, unsigned long flags; struct ipmi_user *new_user; int rv = 0; - ipmi_smi_t intf; + struct ipmi_smi *intf; /* * There is no module usecount here, because it's not @@ -1180,7 +1181,7 @@ EXPORT_SYMBOL(ipmi_create_user); int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data) { int rv = 0; - ipmi_smi_t intf; + struct ipmi_smi *intf; const struct ipmi_smi_handlers *handlers; mutex_lock(&ipmi_interfaces_mutex); @@ -1212,7 +1213,7 @@ static void free_user(struct kref *ref) int ipmi_destroy_user(struct ipmi_user *user) { - ipmi_smi_t intf = user->intf; + struct ipmi_smi *intf = user->intf; int i; unsigned long flags; struct cmd_rcvr *rcvr; @@ -1352,7 +1353,7 @@ int ipmi_get_maintenance_mode(struct ipmi_user *user) } EXPORT_SYMBOL(ipmi_get_maintenance_mode); -static void maintenance_mode_update(ipmi_smi_t intf) +static void maintenance_mode_update(struct ipmi_smi *intf) { if (intf->handlers->set_maintenance_mode) intf->handlers->set_maintenance_mode( @@ -1363,7 +1364,7 @@ int ipmi_set_maintenance_mode(struct ipmi_user *user, int mode) { int rv = 0; unsigned long flags; - ipmi_smi_t intf = user->intf; + struct ipmi_smi *intf = user->intf; spin_lock_irqsave(&intf->maintenance_mode_lock, flags); if (intf->maintenance_mode != mode) { @@ -1399,7 +1400,7 @@ EXPORT_SYMBOL(ipmi_set_maintenance_mode); int ipmi_set_gets_events(struct ipmi_user *user, bool val) { unsigned long flags; - ipmi_smi_t intf = user->intf; + struct ipmi_smi *intf = user->intf; struct ipmi_recv_msg *msg, *msg2; struct list_head msgs; @@ -1456,7 +1457,7 @@ int ipmi_set_gets_events(struct ipmi_user *user, bool val) } EXPORT_SYMBOL(ipmi_set_gets_events); -static struct cmd_rcvr *find_cmd_rcvr(ipmi_smi_t intf, +static struct cmd_rcvr *find_cmd_rcvr(struct ipmi_smi *intf, unsigned char netfn, unsigned char cmd, unsigned char chan) @@ -1471,7 +1472,7 @@ static struct cmd_rcvr *find_cmd_rcvr(ipmi_smi_t intf, return NULL; } -static int is_cmd_rcvr_exclusive(ipmi_smi_t intf, +static int is_cmd_rcvr_exclusive(struct ipmi_smi *intf, unsigned char netfn, unsigned char cmd, unsigned int chans) @@ -1491,7 +1492,7 @@ int ipmi_register_for_cmd(struct ipmi_user *user, unsigned char cmd, unsigned int chans) { - ipmi_smi_t intf = user->intf; + struct ipmi_smi *intf = user->intf; struct cmd_rcvr *rcvr; int rv = 0; @@ -1530,7 +1531,7 @@ int ipmi_unregister_for_cmd(struct ipmi_user *user, unsigned char cmd, unsigned int chans) { - ipmi_smi_t intf = user->intf; + struct ipmi_smi *intf = user->intf; struct cmd_rcvr *rcvr; struct cmd_rcvr *rcvrs = NULL; int i, rv = -ENOENT; @@ -1654,7 +1655,7 @@ static inline void format_lan_msg(struct ipmi_smi_msg *smi_msg, smi_msg->msgid = msgid; } -static struct ipmi_smi_msg *smi_add_send_msg(ipmi_smi_t intf, +static struct ipmi_smi_msg *smi_add_send_msg(struct ipmi_smi *intf, struct ipmi_smi_msg *smi_msg, int priority) { @@ -1672,7 +1673,8 @@ static struct ipmi_smi_msg *smi_add_send_msg(ipmi_smi_t intf, } -static void smi_send(ipmi_smi_t intf, const struct ipmi_smi_handlers *handlers, +static void smi_send(struct ipmi_smi *intf, + const struct ipmi_smi_handlers *handlers, struct ipmi_smi_msg *smi_msg, int priority) { int run_to_completion = intf->run_to_completion; @@ -1699,7 +1701,7 @@ static bool is_maintenance_mode_cmd(struct kernel_ipmi_msg *msg) || (msg->netfn == IPMI_NETFN_FIRMWARE_REQUEST)); } -static int i_ipmi_req_sysintf(ipmi_smi_t intf, +static int i_ipmi_req_sysintf(struct ipmi_smi *intf, struct ipmi_addr *addr, long msgid, struct kernel_ipmi_msg *msg, @@ -1766,7 +1768,7 @@ static int i_ipmi_req_sysintf(ipmi_smi_t intf, return 0; } -static int i_ipmi_req_ipmb(ipmi_smi_t intf, +static int i_ipmi_req_ipmb(struct ipmi_smi *intf, struct ipmi_addr *addr, long msgid, struct kernel_ipmi_msg *msg, @@ -1907,7 +1909,7 @@ static int i_ipmi_req_ipmb(ipmi_smi_t intf, return rv; } -static int i_ipmi_req_lan(ipmi_smi_t intf, +static int i_ipmi_req_lan(struct ipmi_smi *intf, struct ipmi_addr *addr, long msgid, struct kernel_ipmi_msg *msg, @@ -2032,7 +2034,7 @@ static int i_ipmi_req_lan(ipmi_smi_t intf, * occurs. */ static int i_ipmi_request(struct ipmi_user *user, - ipmi_smi_t intf, + struct ipmi_smi *intf, struct ipmi_addr *addr, long msgid, struct kernel_ipmi_msg *msg, @@ -2114,7 +2116,7 @@ static int i_ipmi_request(struct ipmi_user *user, return rv; } -static int check_addr(ipmi_smi_t intf, +static int check_addr(struct ipmi_smi *intf, struct ipmi_addr *addr, unsigned char *saddr, unsigned char *lun) @@ -2190,7 +2192,8 @@ int ipmi_request_supply_msgs(struct ipmi_user *user, } EXPORT_SYMBOL(ipmi_request_supply_msgs); -static void bmc_device_id_handler(ipmi_smi_t intf, struct ipmi_recv_msg *msg) +static void bmc_device_id_handler(struct ipmi_smi *intf, + struct ipmi_recv_msg *msg) { int rv; @@ -2222,7 +2225,7 @@ static void bmc_device_id_handler(ipmi_smi_t intf, struct ipmi_recv_msg *msg) } static int -send_get_device_id_cmd(ipmi_smi_t intf) +send_get_device_id_cmd(struct ipmi_smi *intf) { struct ipmi_system_interface_addr si; struct kernel_ipmi_msg msg; @@ -2250,7 +2253,7 @@ send_get_device_id_cmd(ipmi_smi_t intf) -1, 0); } -static int __get_device_id(ipmi_smi_t intf, struct bmc_device *bmc) +static int __get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc) { int rv; @@ -2284,7 +2287,7 @@ static int __get_device_id(ipmi_smi_t intf, struct bmc_device *bmc) * Except for the first time this is called (in ipmi_register_smi()), * this will always return good data; */ -static int __bmc_get_device_id(ipmi_smi_t intf, struct bmc_device *bmc, +static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc, struct ipmi_device_id *id, bool *guid_set, guid_t *guid, int intf_num) { @@ -2417,7 +2420,7 @@ static int __bmc_get_device_id(ipmi_smi_t intf, struct bmc_device *bmc, return rv; } -static int bmc_get_device_id(ipmi_smi_t intf, struct bmc_device *bmc, +static int bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc, struct ipmi_device_id *id, bool *guid_set, guid_t *guid) { @@ -2427,7 +2430,7 @@ static int bmc_get_device_id(ipmi_smi_t intf, struct bmc_device *bmc, #ifdef CONFIG_IPMI_PROC_INTERFACE static int smi_ipmb_proc_show(struct seq_file *m, void *v) { - ipmi_smi_t intf = m->private; + struct ipmi_smi *intf = m->private; int i; seq_printf(m, "%x", intf->addrinfo[0].address); @@ -2452,7 +2455,7 @@ static const struct file_operations smi_ipmb_proc_ops = { static int smi_version_proc_show(struct seq_file *m, void *v) { - ipmi_smi_t intf = m->private; + struct ipmi_smi *intf = m->private; struct ipmi_device_id id; int rv; @@ -2481,7 +2484,7 @@ static const struct file_operations smi_version_proc_ops = { static int smi_stats_proc_show(struct seq_file *m, void *v) { - ipmi_smi_t intf = m->private; + struct ipmi_smi *intf = m->private; seq_printf(m, "sent_invalid_commands: %u\n", ipmi_get_stat(intf, sent_invalid_commands)); @@ -2554,7 +2557,7 @@ static const struct file_operations smi_stats_proc_ops = { .release = single_release, }; -int ipmi_smi_add_proc_entry(ipmi_smi_t smi, char *name, +int ipmi_smi_add_proc_entry(struct ipmi_smi *smi, char *name, const struct file_operations *proc_ops, void *data) { @@ -2589,7 +2592,7 @@ int ipmi_smi_add_proc_entry(ipmi_smi_t smi, char *name, } EXPORT_SYMBOL(ipmi_smi_add_proc_entry); -static int add_proc_entries(ipmi_smi_t smi, int num) +static int add_proc_entries(struct ipmi_smi *smi, int num) { int rv = 0; @@ -2616,7 +2619,7 @@ static int add_proc_entries(ipmi_smi_t smi, int num) return rv; } -static void remove_proc_entries(ipmi_smi_t smi) +static void remove_proc_entries(struct ipmi_smi *smi) { struct ipmi_proc_entry *entry; @@ -2965,7 +2968,7 @@ cleanup_bmc_device(struct kref *ref) /* * Must be called with intf->bmc_reg_mutex held. */ -static void __ipmi_bmc_unregister(ipmi_smi_t intf) +static void __ipmi_bmc_unregister(struct ipmi_smi *intf) { struct bmc_device *bmc = intf->bmc; @@ -2985,7 +2988,7 @@ static void __ipmi_bmc_unregister(ipmi_smi_t intf) intf->bmc_registered = false; } -static void ipmi_bmc_unregister(ipmi_smi_t intf) +static void ipmi_bmc_unregister(struct ipmi_smi *intf) { mutex_lock(&intf->bmc_reg_mutex); __ipmi_bmc_unregister(intf); @@ -2995,7 +2998,7 @@ static void ipmi_bmc_unregister(ipmi_smi_t intf) /* * Must be called with intf->bmc_reg_mutex held. */ -static int __ipmi_bmc_register(ipmi_smi_t intf, +static int __ipmi_bmc_register(struct ipmi_smi *intf, struct ipmi_device_id *id, bool guid_set, guid_t *guid, int intf_num) { @@ -3157,7 +3160,7 @@ static int __ipmi_bmc_register(ipmi_smi_t intf, } static int -send_guid_cmd(ipmi_smi_t intf, int chan) +send_guid_cmd(struct ipmi_smi *intf, int chan) { struct kernel_ipmi_msg msg; struct ipmi_system_interface_addr si; @@ -3184,7 +3187,7 @@ send_guid_cmd(ipmi_smi_t intf, int chan) -1, 0); } -static void guid_handler(ipmi_smi_t intf, struct ipmi_recv_msg *msg) +static void guid_handler(struct ipmi_smi *intf, struct ipmi_recv_msg *msg) { struct bmc_device *bmc = intf->bmc; @@ -3219,7 +3222,7 @@ static void guid_handler(ipmi_smi_t intf, struct ipmi_recv_msg *msg) wake_up(&intf->waitq); } -static void __get_guid(ipmi_smi_t intf) +static void __get_guid(struct ipmi_smi *intf) { int rv; struct bmc_device *bmc = intf->bmc; @@ -3240,7 +3243,7 @@ static void __get_guid(ipmi_smi_t intf) } static int -send_channel_info_cmd(ipmi_smi_t intf, int chan) +send_channel_info_cmd(struct ipmi_smi *intf, int chan) { struct kernel_ipmi_msg msg; unsigned char data[1]; @@ -3270,7 +3273,7 @@ send_channel_info_cmd(ipmi_smi_t intf, int chan) } static void -channel_handler(ipmi_smi_t intf, struct ipmi_recv_msg *msg) +channel_handler(struct ipmi_smi *intf, struct ipmi_recv_msg *msg) { int rv = 0; int ch; @@ -3342,7 +3345,7 @@ channel_handler(ipmi_smi_t intf, struct ipmi_recv_msg *msg) /* * Must be holding intf->bmc_reg_mutex to call this. */ -static int __scan_channels(ipmi_smi_t intf, struct ipmi_device_id *id) +static int __scan_channels(struct ipmi_smi *intf, struct ipmi_device_id *id) { int rv; @@ -3386,7 +3389,7 @@ static int __scan_channels(ipmi_smi_t intf, struct ipmi_device_id *id) return 0; } -static void ipmi_poll(ipmi_smi_t intf) +static void ipmi_poll(struct ipmi_smi *intf) { if (intf->handlers->poll) intf->handlers->poll(intf->send_info); @@ -3402,7 +3405,8 @@ EXPORT_SYMBOL(ipmi_poll_interface); static void redo_bmc_reg(struct work_struct *work) { - ipmi_smi_t intf = container_of(work, struct ipmi_smi, bmc_reg_work); + struct ipmi_smi *intf = container_of(work, struct ipmi_smi, + bmc_reg_work); if (!intf->in_shutdown) bmc_get_device_id(intf, NULL, NULL, NULL, NULL); @@ -3417,8 +3421,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, { int i, j; int rv; - ipmi_smi_t intf; - ipmi_smi_t tintf; + struct ipmi_smi *intf, *tintf; struct list_head *link; struct ipmi_device_id id; @@ -3563,7 +3566,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, } EXPORT_SYMBOL(ipmi_register_smi); -static void deliver_smi_err_response(ipmi_smi_t intf, +static void deliver_smi_err_response(struct ipmi_smi *intf, struct ipmi_smi_msg *msg, unsigned char err) { @@ -3575,7 +3578,7 @@ static void deliver_smi_err_response(ipmi_smi_t intf, handle_one_recv_msg(intf, msg); } -static void cleanup_smi_msgs(ipmi_smi_t intf) +static void cleanup_smi_msgs(struct ipmi_smi *intf) { int i; struct seq_table *ent; @@ -3615,7 +3618,7 @@ static void cleanup_smi_msgs(ipmi_smi_t intf) } } -int ipmi_unregister_smi(ipmi_smi_t intf) +int ipmi_unregister_smi(struct ipmi_smi *intf) { struct ipmi_smi_watcher *w; int intf_num = intf->intf_num; @@ -3661,7 +3664,7 @@ int ipmi_unregister_smi(ipmi_smi_t intf) } EXPORT_SYMBOL(ipmi_unregister_smi); -static int handle_ipmb_get_msg_rsp(ipmi_smi_t intf, +static int handle_ipmb_get_msg_rsp(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { struct ipmi_ipmb_addr ipmb_addr; @@ -3722,7 +3725,7 @@ static int handle_ipmb_get_msg_rsp(ipmi_smi_t intf, return 0; } -static int handle_ipmb_get_msg_cmd(ipmi_smi_t intf, +static int handle_ipmb_get_msg_cmd(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { struct cmd_rcvr *rcvr; @@ -3835,7 +3838,7 @@ static int handle_ipmb_get_msg_cmd(ipmi_smi_t intf, return rv; } -static int handle_lan_get_msg_rsp(ipmi_smi_t intf, +static int handle_lan_get_msg_rsp(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { struct ipmi_lan_addr lan_addr; @@ -3900,7 +3903,7 @@ static int handle_lan_get_msg_rsp(ipmi_smi_t intf, return 0; } -static int handle_lan_get_msg_cmd(ipmi_smi_t intf, +static int handle_lan_get_msg_cmd(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { struct cmd_rcvr *rcvr; @@ -3999,7 +4002,7 @@ static int handle_lan_get_msg_cmd(ipmi_smi_t intf, * the OEM. See IPMI 2.0 specification, Chapter 6 and * Chapter 22, sections 22.6 and 22.24 for more details. */ -static int handle_oem_get_msg_cmd(ipmi_smi_t intf, +static int handle_oem_get_msg_cmd(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { struct cmd_rcvr *rcvr; @@ -4118,7 +4121,7 @@ static void copy_event_into_recv_msg(struct ipmi_recv_msg *recv_msg, recv_msg->msg.data_len = msg->rsp_size - 3; } -static int handle_read_event_rsp(ipmi_smi_t intf, +static int handle_read_event_rsp(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { struct ipmi_recv_msg *recv_msg, *recv_msg2; @@ -4221,7 +4224,7 @@ static int handle_read_event_rsp(ipmi_smi_t intf, return rv; } -static int handle_bmc_rsp(ipmi_smi_t intf, +static int handle_bmc_rsp(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { struct ipmi_recv_msg *recv_msg; @@ -4267,7 +4270,7 @@ static int handle_bmc_rsp(ipmi_smi_t intf, * 0 if the message should be freed, or -1 if the message should not * be freed or requeued. */ -static int handle_one_recv_msg(ipmi_smi_t intf, +static int handle_one_recv_msg(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { int requeue; @@ -4425,7 +4428,7 @@ static int handle_one_recv_msg(ipmi_smi_t intf, /* * If there are messages in the queue or pretimeouts, handle them. */ -static void handle_new_recv_msgs(ipmi_smi_t intf) +static void handle_new_recv_msgs(struct ipmi_smi *intf) { struct ipmi_smi_msg *smi_msg; unsigned long flags = 0; @@ -4485,7 +4488,7 @@ static void handle_new_recv_msgs(ipmi_smi_t intf) static void smi_recv_tasklet(unsigned long val) { unsigned long flags = 0; /* keep us warning-free. */ - ipmi_smi_t intf = (ipmi_smi_t) val; + struct ipmi_smi *intf = (struct ipmi_smi *) val; int run_to_completion = intf->run_to_completion; struct ipmi_smi_msg *newmsg = NULL; @@ -4527,7 +4530,7 @@ static void smi_recv_tasklet(unsigned long val) } /* Handle a new message from the lower layer. */ -void ipmi_smi_msg_received(ipmi_smi_t intf, +void ipmi_smi_msg_received(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { unsigned long flags = 0; /* keep us warning-free. */ @@ -4608,7 +4611,7 @@ void ipmi_smi_msg_received(ipmi_smi_t intf, } EXPORT_SYMBOL(ipmi_smi_msg_received); -void ipmi_smi_watchdog_pretimeout(ipmi_smi_t intf) +void ipmi_smi_watchdog_pretimeout(struct ipmi_smi *intf) { if (intf->in_shutdown) return; @@ -4619,7 +4622,7 @@ void ipmi_smi_watchdog_pretimeout(ipmi_smi_t intf) EXPORT_SYMBOL(ipmi_smi_watchdog_pretimeout); static struct ipmi_smi_msg * -smi_from_recv_msg(ipmi_smi_t intf, struct ipmi_recv_msg *recv_msg, +smi_from_recv_msg(struct ipmi_smi *intf, struct ipmi_recv_msg *recv_msg, unsigned char seq, long seqid) { struct ipmi_smi_msg *smi_msg = ipmi_alloc_smi_msg(); @@ -4639,7 +4642,7 @@ smi_from_recv_msg(ipmi_smi_t intf, struct ipmi_recv_msg *recv_msg, return smi_msg; } -static void check_msg_timeout(ipmi_smi_t intf, struct seq_table *ent, +static void check_msg_timeout(struct ipmi_smi *intf, struct seq_table *ent, struct list_head *timeouts, unsigned long timeout_period, int slot, unsigned long *flags, @@ -4721,7 +4724,7 @@ static void check_msg_timeout(ipmi_smi_t intf, struct seq_table *ent, } } -static unsigned int ipmi_timeout_handler(ipmi_smi_t intf, +static unsigned int ipmi_timeout_handler(struct ipmi_smi *intf, unsigned long timeout_period) { struct list_head timeouts; @@ -4788,7 +4791,7 @@ static unsigned int ipmi_timeout_handler(ipmi_smi_t intf, return waiting_msgs; } -static void ipmi_request_event(ipmi_smi_t intf) +static void ipmi_request_event(struct ipmi_smi *intf) { /* No event requests when in maintenance mode. */ if (intf->maintenance_mode_enable) @@ -4804,7 +4807,7 @@ static atomic_t stop_operation; static void ipmi_timeout(struct timer_list *unused) { - ipmi_smi_t intf; + struct ipmi_smi *intf; int nt = 0; if (atomic_read(&stop_operation)) @@ -4839,7 +4842,7 @@ static void ipmi_timeout(struct timer_list *unused) mod_timer(&ipmi_timer, jiffies + IPMI_TIMEOUT_JIFFIES); } -static void need_waiter(ipmi_smi_t intf) +static void need_waiter(struct ipmi_smi *intf) { /* Racy, but worst case we start the timer twice. */ if (!timer_pending(&ipmi_timer)) @@ -4910,8 +4913,8 @@ static void dummy_recv_done_handler(struct ipmi_recv_msg *msg) /* * Inside a panic, send a message and wait for a response. */ -static void ipmi_panic_request_and_wait(ipmi_smi_t intf, - struct ipmi_addr *addr, +static void ipmi_panic_request_and_wait(struct ipmi_smi *intf, + struct ipmi_addr *addr, struct kernel_ipmi_msg *msg) { struct ipmi_smi_msg smi_msg; @@ -4942,7 +4945,8 @@ static void ipmi_panic_request_and_wait(ipmi_smi_t intf, ipmi_poll(intf); } -static void event_receiver_fetcher(ipmi_smi_t intf, struct ipmi_recv_msg *msg) +static void event_receiver_fetcher(struct ipmi_smi *intf, + struct ipmi_recv_msg *msg) { if ((msg->addr.addr_type == IPMI_SYSTEM_INTERFACE_ADDR_TYPE) && (msg->msg.netfn == IPMI_NETFN_SENSOR_EVENT_RESPONSE) @@ -4954,7 +4958,7 @@ static void event_receiver_fetcher(ipmi_smi_t intf, struct ipmi_recv_msg *msg) } } -static void device_id_fetcher(ipmi_smi_t intf, struct ipmi_recv_msg *msg) +static void device_id_fetcher(struct ipmi_smi *intf, struct ipmi_recv_msg *msg) { if ((msg->addr.addr_type == IPMI_SYSTEM_INTERFACE_ADDR_TYPE) && (msg->msg.netfn == IPMI_NETFN_APP_RESPONSE) @@ -4969,7 +4973,7 @@ static void device_id_fetcher(ipmi_smi_t intf, struct ipmi_recv_msg *msg) } } -static void send_panic_events(ipmi_smi_t intf, char *str) +static void send_panic_events(struct ipmi_smi *intf, char *str) { struct kernel_ipmi_msg msg; unsigned char data[16]; @@ -5121,7 +5125,7 @@ static int panic_event(struct notifier_block *this, unsigned long event, void *ptr) { - ipmi_smi_t intf; + struct ipmi_smi *intf; struct ipmi_user *user; if (has_panicked) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index ff870aa91cfe..24702ab853a8 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -123,7 +123,7 @@ enum si_stat_indexes { struct smi_info { int intf_num; - ipmi_smi_t intf; + struct ipmi_smi *intf; struct si_sm_data *si_sm; const struct si_sm_handlers *handlers; spinlock_t si_lock; @@ -1143,8 +1143,8 @@ irqreturn_t ipmi_si_irq_handler(int irq, void *data) return IRQ_HANDLED; } -static int smi_start_processing(void *send_info, - ipmi_smi_t intf) +static int smi_start_processing(void *send_info, + struct ipmi_smi *intf) { struct smi_info *new_smi = send_info; int enable = 0; diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 35a82f4bfd78..2db97fd08a8f 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -193,7 +193,7 @@ typedef void (*ssif_i2c_done)(struct ssif_info *ssif_info, int result, unsigned char *data, unsigned int len); struct ssif_info { - ipmi_smi_t intf; + struct ipmi_smi *intf; int intf_num; spinlock_t lock; struct ipmi_smi_msg *waiting_msg; @@ -315,7 +315,7 @@ static void ipmi_ssif_unlock_cond(struct ssif_info *ssif_info, static void deliver_recv_msg(struct ssif_info *ssif_info, struct ipmi_smi_msg *msg) { - ipmi_smi_t intf = ssif_info->intf; + struct ipmi_smi *intf = ssif_info->intf; if (!intf) { ipmi_free_smi_msg(msg); @@ -452,7 +452,7 @@ static void start_recv_msg_fetch(struct ssif_info *ssif_info, static void handle_flags(struct ssif_info *ssif_info, unsigned long *flags) { if (ssif_info->msg_flags & WDT_PRE_TIMEOUT_INT) { - ipmi_smi_t intf = ssif_info->intf; + struct ipmi_smi *intf = ssif_info->intf; /* Watchdog pre-timeout */ ssif_inc_stat(ssif_info, watchdog_pretimeouts); start_clear_flags(ssif_info, flags); @@ -1113,8 +1113,8 @@ static void dec_usecount(void *send_info) i2c_put_adapter(ssif_info->client->adapter); } -static int ssif_start_processing(void *send_info, - ipmi_smi_t intf) +static int ssif_start_processing(void *send_info, + struct ipmi_smi *intf) { struct ssif_info *ssif_info = send_info; From ac93bd0c9e163fd3e2edfb4b5af22955b408431a Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 21:37:56 -0500 Subject: [PATCH 16/35] ipmi: Fix some counter issues Counters would not be pegged properly on some errors. Have deliver_response() return an error so the counters can be incremented properly. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 76 +++++++++++++++++------------ 1 file changed, 45 insertions(+), 31 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 9fa04b7afbd0..c21c4e021dab 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -886,18 +886,17 @@ unsigned int ipmi_addr_length(int addr_type) } EXPORT_SYMBOL(ipmi_addr_length); -static void deliver_response(struct ipmi_recv_msg *msg) +static int deliver_response(struct ipmi_smi *intf, struct ipmi_recv_msg *msg) { - if (!msg->user) { - struct ipmi_smi *intf = msg->user_msg_data; + int rv = 0; + if (!msg->user) { /* Special handling for NULL users. */ if (intf->null_user_handler) { intf->null_user_handler(intf, msg); - ipmi_inc_stat(intf, handled_local_responses); } else { /* No handler, so give up. */ - ipmi_inc_stat(intf, unhandled_local_responses); + rv = -EINVAL; } ipmi_free_recv_msg(msg); } else if (!oops_in_progress) { @@ -910,17 +909,28 @@ static void deliver_response(struct ipmi_recv_msg *msg) struct ipmi_user *user = msg->user; user->handler->ipmi_recv_hndl(msg, user->handler_data); } + + return rv; } -static void -deliver_err_response(struct ipmi_recv_msg *msg, int err) +static void deliver_local_response(struct ipmi_smi *intf, + struct ipmi_recv_msg *msg) +{ + if (deliver_response(intf, msg)) + ipmi_inc_stat(intf, unhandled_local_responses); + else + ipmi_inc_stat(intf, handled_local_responses); +} + +static void deliver_err_response(struct ipmi_smi *intf, + struct ipmi_recv_msg *msg, int err) { msg->recv_type = IPMI_RESPONSE_RECV_TYPE; msg->msg_data[0] = err; msg->msg.netfn |= 1; /* Convert to a response. */ msg->msg.data_len = 1; msg->msg.data = msg->msg_data; - deliver_response(msg); + deliver_local_response(intf, msg); } /* @@ -1071,7 +1081,7 @@ static int intf_err_seq(struct ipmi_smi *intf, spin_unlock_irqrestore(&intf->seq_lock, flags); if (msg) - deliver_err_response(msg, err); + deliver_err_response(intf, msg, err); return rv; } @@ -1443,7 +1453,7 @@ int ipmi_set_gets_events(struct ipmi_user *user, bool val) list_for_each_entry_safe(msg, msg2, &msgs, link) { msg->user = user; kref_get(&user->refcount); - deliver_response(msg); + deliver_local_response(intf, msg); } spin_lock_irqsave(&intf->events_lock, flags); @@ -3614,7 +3624,7 @@ static void cleanup_smi_msgs(struct ipmi_smi *intf) ent = &intf->seq_table[i]; if (!ent->inuse) continue; - deliver_err_response(ent->recv_msg, IPMI_ERR_UNSPECIFIED); + deliver_err_response(intf, ent->recv_msg, IPMI_ERR_UNSPECIFIED); } } @@ -3719,8 +3729,10 @@ static int handle_ipmb_get_msg_rsp(struct ipmi_smi *intf, recv_msg->msg.data = recv_msg->msg_data; recv_msg->msg.data_len = msg->rsp_size - 10; recv_msg->recv_type = IPMI_RESPONSE_RECV_TYPE; - ipmi_inc_stat(intf, handled_ipmb_responses); - deliver_response(recv_msg); + if (deliver_response(intf, recv_msg)) + ipmi_inc_stat(intf, unhandled_ipmb_responses); + else + ipmi_inc_stat(intf, handled_ipmb_responses); return 0; } @@ -3793,9 +3805,6 @@ static int handle_ipmb_get_msg_cmd(struct ipmi_smi *intf, } rcu_read_unlock(); } else { - /* Deliver the message to the user. */ - ipmi_inc_stat(intf, handled_commands); - recv_msg = ipmi_alloc_recv_msg(); if (!recv_msg) { /* @@ -3831,7 +3840,10 @@ static int handle_ipmb_get_msg_cmd(struct ipmi_smi *intf, recv_msg->msg.data_len = msg->rsp_size - 10; memcpy(recv_msg->msg_data, &msg->rsp[9], msg->rsp_size - 10); - deliver_response(recv_msg); + if (deliver_response(intf, recv_msg)) + ipmi_inc_stat(intf, unhandled_commands); + else + ipmi_inc_stat(intf, handled_commands); } } @@ -3897,8 +3909,10 @@ static int handle_lan_get_msg_rsp(struct ipmi_smi *intf, recv_msg->msg.data = recv_msg->msg_data; recv_msg->msg.data_len = msg->rsp_size - 12; recv_msg->recv_type = IPMI_RESPONSE_RECV_TYPE; - ipmi_inc_stat(intf, handled_lan_responses); - deliver_response(recv_msg); + if (deliver_response(intf, recv_msg)) + ipmi_inc_stat(intf, unhandled_lan_responses); + else + ipmi_inc_stat(intf, handled_lan_responses); return 0; } @@ -3949,9 +3963,6 @@ static int handle_lan_get_msg_cmd(struct ipmi_smi *intf, */ rv = 0; } else { - /* Deliver the message to the user. */ - ipmi_inc_stat(intf, handled_commands); - recv_msg = ipmi_alloc_recv_msg(); if (!recv_msg) { /* @@ -3989,7 +4000,10 @@ static int handle_lan_get_msg_cmd(struct ipmi_smi *intf, recv_msg->msg.data_len = msg->rsp_size - 12; memcpy(recv_msg->msg_data, &msg->rsp[11], msg->rsp_size - 12); - deliver_response(recv_msg); + if (deliver_response(intf, recv_msg)) + ipmi_inc_stat(intf, unhandled_commands); + else + ipmi_inc_stat(intf, handled_commands); } } @@ -4057,9 +4071,6 @@ static int handle_oem_get_msg_cmd(struct ipmi_smi *intf, rv = 0; } else { - /* Deliver the message to the user. */ - ipmi_inc_stat(intf, handled_commands); - recv_msg = ipmi_alloc_recv_msg(); if (!recv_msg) { /* @@ -4096,7 +4107,10 @@ static int handle_oem_get_msg_cmd(struct ipmi_smi *intf, recv_msg->msg.data_len = msg->rsp_size - 4; memcpy(recv_msg->msg_data, &msg->rsp[4], msg->rsp_size - 4); - deliver_response(recv_msg); + if (deliver_response(intf, recv_msg)) + ipmi_inc_stat(intf, unhandled_commands); + else + ipmi_inc_stat(intf, handled_commands); } } @@ -4187,7 +4201,7 @@ static int handle_read_event_rsp(struct ipmi_smi *intf, /* Now deliver all the messages. */ list_for_each_entry_safe(recv_msg, recv_msg2, &msgs, link) { list_del(&recv_msg->link); - deliver_response(recv_msg); + deliver_local_response(intf, recv_msg); } } else if (intf->waiting_events_count < MAX_EVENTS_IN_QUEUE) { /* @@ -4259,7 +4273,7 @@ static int handle_bmc_rsp(struct ipmi_smi *intf, memcpy(recv_msg->msg_data, &msg->rsp[2], msg->rsp_size - 2); recv_msg->msg.data = recv_msg->msg_data; recv_msg->msg.data_len = msg->rsp_size - 2; - deliver_response(recv_msg); + deliver_local_response(intf, recv_msg); } return 0; @@ -4336,7 +4350,7 @@ static int handle_one_recv_msg(struct ipmi_smi *intf, recv_msg->msg.data = recv_msg->msg_data; recv_msg->msg.data_len = 1; recv_msg->msg_data[0] = msg->rsp[2]; - deliver_response(recv_msg); + deliver_local_response(intf, recv_msg); } else if ((msg->rsp[0] == ((IPMI_NETFN_APP_REQUEST|1) << 2)) && (msg->rsp[1] == IPMI_GET_MSG_CMD)) { struct ipmi_channel *chans; @@ -4761,7 +4775,7 @@ static unsigned int ipmi_timeout_handler(struct ipmi_smi *intf, spin_unlock_irqrestore(&intf->seq_lock, flags); list_for_each_entry_safe(msg, msg2, &timeouts, link) - deliver_err_response(msg, IPMI_TIMEOUT_COMPLETION_CODE); + deliver_err_response(intf, msg, IPMI_TIMEOUT_COMPLETION_CODE); /* * Maintenance mode handling. Check the timeout From e86ee2d44b44056243da17c120ad258717cedf9b Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 22:05:04 -0500 Subject: [PATCH 17/35] ipmi: Rework locking and shutdown for hot remove To handle hot remove of interfaces, a lot of rework had to be done to the locking. Several things were switched over to srcu and shutdown for users and interfaces was added for cleaner shutdown. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 492 ++++++++++++++++------------ 1 file changed, 281 insertions(+), 211 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index c21c4e021dab..a27b50ac2b7f 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -197,8 +197,12 @@ MODULE_PARM_DESC(default_max_retries, struct ipmi_user { struct list_head link; - /* Set to false when the user is destroyed. */ - bool valid; + /* + * Set to NULL when the user is destroyed, a pointer to myself + * so srcu_dereference can be used on it. + */ + struct ipmi_user *self; + struct srcu_struct release_barrier; struct kref refcount; @@ -213,6 +217,23 @@ struct ipmi_user { bool gets_events; }; +static struct ipmi_user *acquire_ipmi_user(struct ipmi_user *user, int *index) + __acquires(user->release_barrier) +{ + struct ipmi_user *ruser; + + *index = srcu_read_lock(&user->release_barrier); + ruser = srcu_dereference(user->self, &user->release_barrier); + if (!ruser) + srcu_read_unlock(&user->release_barrier, *index); + return ruser; +} + +static void release_ipmi_user(struct ipmi_user *user, int index) +{ + srcu_read_unlock(&user->release_barrier, index); +} + struct cmd_rcvr { struct list_head link; @@ -444,10 +465,11 @@ struct ipmi_smi { struct list_head link; /* - * The list of upper layers that are using me. seq_lock - * protects this. + * The list of upper layers that are using me. seq_lock write + * protects this. Read protection is with srcu. */ struct list_head users; + struct srcu_struct users_srcu; /* Used for wake ups at startup. */ wait_queue_head_t waitq; @@ -467,12 +489,6 @@ struct ipmi_smi { bool in_bmc_register; /* Handle recursive situations. Yuck. */ struct work_struct bmc_reg_work; - /* - * This is the lower-layer's sender routine. Note that you - * must either be holding the ipmi_interfaces_mutex or be in - * an umpreemptible region to use this. You must fetch the - * value into a local variable and make sure it is not NULL. - */ const struct ipmi_smi_handlers *handlers; void *send_info; @@ -615,6 +631,7 @@ static DEFINE_MUTEX(ipmidriver_mutex); static LIST_HEAD(ipmi_interfaces); static DEFINE_MUTEX(ipmi_interfaces_mutex); +DEFINE_STATIC_SRCU(ipmi_interfaces_srcu); /* * List of watchers that want to know when smi's are added and deleted. @@ -715,58 +732,32 @@ static void intf_free(struct kref *ref) struct watcher_entry { int intf_num; - struct ipmi_smi *intf; + struct ipmi_smi *intf; struct list_head link; }; int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher) { struct ipmi_smi *intf; - LIST_HEAD(to_deliver); - struct watcher_entry *e, *e2; + int index; mutex_lock(&smi_watchers_mutex); - mutex_lock(&ipmi_interfaces_mutex); - - /* Build a list of things to deliver. */ - list_for_each_entry(intf, &ipmi_interfaces, link) { - if (intf->intf_num == -1) - continue; - e = kmalloc(sizeof(*e), GFP_KERNEL); - if (!e) - goto out_err; - kref_get(&intf->refcount); - e->intf = intf; - e->intf_num = intf->intf_num; - list_add_tail(&e->link, &to_deliver); - } - - /* We will succeed, so add it to the list. */ list_add(&watcher->link, &smi_watchers); - mutex_unlock(&ipmi_interfaces_mutex); + index = srcu_read_lock(&ipmi_interfaces_srcu); + list_for_each_entry_rcu(intf, &ipmi_interfaces, link) { + int intf_num = READ_ONCE(intf->intf_num); - list_for_each_entry_safe(e, e2, &to_deliver, link) { - list_del(&e->link); - watcher->new_smi(e->intf_num, e->intf->si_dev); - kref_put(&e->intf->refcount, intf_free); - kfree(e); + if (intf_num == -1) + continue; + watcher->new_smi(intf_num, intf->si_dev); } + srcu_read_unlock(&ipmi_interfaces_srcu, index); mutex_unlock(&smi_watchers_mutex); return 0; - - out_err: - mutex_unlock(&ipmi_interfaces_mutex); - mutex_unlock(&smi_watchers_mutex); - list_for_each_entry_safe(e, e2, &to_deliver, link) { - list_del(&e->link); - kref_put(&e->intf->refcount, intf_free); - kfree(e); - } - return -ENOMEM; } EXPORT_SYMBOL(ipmi_smi_watcher_register); @@ -787,12 +778,14 @@ call_smi_watchers(int i, struct device *dev) { struct ipmi_smi_watcher *w; + mutex_lock(&smi_watchers_mutex); list_for_each_entry(w, &smi_watchers, link) { if (try_module_get(w->owner)) { w->new_smi(i, dev); module_put(w->owner); } } + mutex_unlock(&smi_watchers_mutex); } static int @@ -905,9 +898,17 @@ static int deliver_response(struct ipmi_smi *intf, struct ipmi_recv_msg *msg) * receive handler doesn't much meaning and has a deadlock * risk. At this moment, simply skip it in that case. */ + int index; + struct ipmi_user *user = acquire_ipmi_user(msg->user, &index); - struct ipmi_user *user = msg->user; - user->handler->ipmi_recv_hndl(msg, user->handler_data); + if (user) { + user->handler->ipmi_recv_hndl(msg, user->handler_data); + release_ipmi_user(msg->user, index); + } else { + /* User went away, give up. */ + ipmi_free_recv_msg(msg); + rv = -EINVAL; + } } return rv; @@ -1094,7 +1095,7 @@ int ipmi_create_user(unsigned int if_num, { unsigned long flags; struct ipmi_user *new_user; - int rv = 0; + int rv = 0, index; struct ipmi_smi *intf; /* @@ -1129,7 +1130,7 @@ int ipmi_create_user(unsigned int if_num, if (!new_user) return -ENOMEM; - mutex_lock(&ipmi_interfaces_mutex); + index = srcu_read_lock(&ipmi_interfaces_srcu); list_for_each_entry_rcu(intf, &ipmi_interfaces, link) { if (intf->intf_num == if_num) goto found; @@ -1139,6 +1140,10 @@ int ipmi_create_user(unsigned int if_num, goto out_kfree; found: + rv = init_srcu_struct(&new_user->release_barrier); + if (rv) + goto out_kfree; + /* Note that each existing user holds a refcount to the interface. */ kref_get(&intf->refcount); @@ -1148,26 +1153,7 @@ int ipmi_create_user(unsigned int if_num, new_user->intf = intf; new_user->gets_events = false; - if (!try_module_get(intf->handlers->owner)) { - rv = -ENODEV; - goto out_kref; - } - - if (intf->handlers->inc_usecount) { - rv = intf->handlers->inc_usecount(intf->send_info); - if (rv) { - module_put(intf->handlers->owner); - goto out_kref; - } - } - - /* - * Hold the lock so intf->handlers is guaranteed to be good - * until now - */ - mutex_unlock(&ipmi_interfaces_mutex); - - new_user->valid = true; + rcu_assign_pointer(new_user->self, new_user); spin_lock_irqsave(&intf->seq_lock, flags); list_add_rcu(&new_user->link, &intf->users); spin_unlock_irqrestore(&intf->seq_lock, flags); @@ -1176,13 +1162,12 @@ int ipmi_create_user(unsigned int if_num, if (atomic_inc_return(&intf->event_waiters) == 1) need_waiter(intf); } + srcu_read_unlock(&ipmi_interfaces_srcu, index); *user = new_user; return 0; -out_kref: - kref_put(&intf->refcount, intf_free); out_kfree: - mutex_unlock(&ipmi_interfaces_mutex); + srcu_read_unlock(&ipmi_interfaces_srcu, index); kfree(new_user); return rv; } @@ -1190,26 +1175,25 @@ EXPORT_SYMBOL(ipmi_create_user); int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data) { - int rv = 0; + int rv, index; struct ipmi_smi *intf; - const struct ipmi_smi_handlers *handlers; - mutex_lock(&ipmi_interfaces_mutex); + index = srcu_read_lock(&ipmi_interfaces_srcu); list_for_each_entry_rcu(intf, &ipmi_interfaces, link) { if (intf->intf_num == if_num) goto found; } + srcu_read_unlock(&ipmi_interfaces_srcu, index); + /* Not found, return an error */ - rv = -EINVAL; - mutex_unlock(&ipmi_interfaces_mutex); - return rv; + return -EINVAL; found: - handlers = intf->handlers; - rv = -ENOSYS; - if (handlers->get_smi_info) - rv = handlers->get_smi_info(intf->send_info, data); - mutex_unlock(&ipmi_interfaces_mutex); + if (!intf->handlers->get_smi_info) + rv = -ENOTTY; + else + rv = intf->handlers->get_smi_info(intf->send_info, data); + srcu_read_unlock(&ipmi_interfaces_srcu, index); return rv; } @@ -1221,7 +1205,7 @@ static void free_user(struct kref *ref) kfree(user); } -int ipmi_destroy_user(struct ipmi_user *user) +static void _ipmi_destroy_user(struct ipmi_user *user) { struct ipmi_smi *intf = user->intf; int i; @@ -1229,7 +1213,22 @@ int ipmi_destroy_user(struct ipmi_user *user) struct cmd_rcvr *rcvr; struct cmd_rcvr *rcvrs = NULL; - user->valid = false; + if (!acquire_ipmi_user(user, &i)) { + /* + * The user has already been cleaned up, just make sure + * nothing is using it and return. + */ + synchronize_srcu(&user->release_barrier); + return; + } + + rcu_assign_pointer(user->self, NULL); + release_ipmi_user(user, i); + + synchronize_srcu(&user->release_barrier); + + if (user->handler->shutdown) + user->handler->shutdown(user->handler_data); if (user->handler->ipmi_watchdog_pretimeout) atomic_dec(&intf->event_waiters); @@ -1254,7 +1253,7 @@ int ipmi_destroy_user(struct ipmi_user *user) * Remove the user from the command receiver's table. First * we build a list of everything (not using the standard link, * since other things may be using it till we do - * synchronize_rcu()) then free everything in that list. + * synchronize_srcu()) then free everything in that list. */ mutex_lock(&intf->cmd_rcvrs_mutex); list_for_each_entry_rcu(rcvr, &intf->cmd_rcvrs, link) { @@ -1272,16 +1271,14 @@ int ipmi_destroy_user(struct ipmi_user *user) kfree(rcvr); } - mutex_lock(&ipmi_interfaces_mutex); - if (intf->handlers) { - module_put(intf->handlers->owner); - if (intf->handlers->dec_usecount) - intf->handlers->dec_usecount(intf->send_info); - } - mutex_unlock(&ipmi_interfaces_mutex); - kref_put(&intf->refcount, intf_free); +} +int ipmi_destroy_user(struct ipmi_user *user) +{ + _ipmi_destroy_user(user); + + cleanup_srcu_struct(&user->release_barrier); kref_put(&user->refcount, free_user); return 0; @@ -1293,16 +1290,20 @@ int ipmi_get_version(struct ipmi_user *user, unsigned char *minor) { struct ipmi_device_id id; - int rv; + int rv, index; + + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; rv = bmc_get_device_id(user->intf, NULL, &id, NULL, NULL); - if (rv) - return rv; + if (!rv) { + *major = ipmi_version_major(&id); + *minor = ipmi_version_minor(&id); + } + release_ipmi_user(user, index); - *major = ipmi_version_major(&id); - *minor = ipmi_version_minor(&id); - - return 0; + return rv; } EXPORT_SYMBOL(ipmi_get_version); @@ -1310,9 +1311,17 @@ int ipmi_set_my_address(struct ipmi_user *user, unsigned int channel, unsigned char address) { + int index; + + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; + if (channel >= IPMI_MAX_CHANNELS) return -EINVAL; user->intf->addrinfo[channel].address = address; + release_ipmi_user(user, index); + return 0; } EXPORT_SYMBOL(ipmi_set_my_address); @@ -1321,9 +1330,17 @@ int ipmi_get_my_address(struct ipmi_user *user, unsigned int channel, unsigned char *address) { + int index; + + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; + if (channel >= IPMI_MAX_CHANNELS) return -EINVAL; *address = user->intf->addrinfo[channel].address; + release_ipmi_user(user, index); + return 0; } EXPORT_SYMBOL(ipmi_get_my_address); @@ -1332,9 +1349,17 @@ int ipmi_set_my_LUN(struct ipmi_user *user, unsigned int channel, unsigned char LUN) { + int index; + + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; + if (channel >= IPMI_MAX_CHANNELS) return -EINVAL; user->intf->addrinfo[channel].lun = LUN & 0x3; + release_ipmi_user(user, index); + return 0; } EXPORT_SYMBOL(ipmi_set_my_LUN); @@ -1343,21 +1368,34 @@ int ipmi_get_my_LUN(struct ipmi_user *user, unsigned int channel, unsigned char *address) { + int index; + + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; + if (channel >= IPMI_MAX_CHANNELS) return -EINVAL; *address = user->intf->addrinfo[channel].lun; + release_ipmi_user(user, index); + return 0; } EXPORT_SYMBOL(ipmi_get_my_LUN); int ipmi_get_maintenance_mode(struct ipmi_user *user) { - int mode; + int mode, index; unsigned long flags; + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; + spin_lock_irqsave(&user->intf->maintenance_mode_lock, flags); mode = user->intf->maintenance_mode; spin_unlock_irqrestore(&user->intf->maintenance_mode_lock, flags); + release_ipmi_user(user, index); return mode; } @@ -1372,10 +1410,14 @@ static void maintenance_mode_update(struct ipmi_smi *intf) int ipmi_set_maintenance_mode(struct ipmi_user *user, int mode) { - int rv = 0; + int rv = 0, index; unsigned long flags; struct ipmi_smi *intf = user->intf; + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; + spin_lock_irqsave(&intf->maintenance_mode_lock, flags); if (intf->maintenance_mode != mode) { switch (mode) { @@ -1402,6 +1444,7 @@ int ipmi_set_maintenance_mode(struct ipmi_user *user, int mode) } out_unlock: spin_unlock_irqrestore(&intf->maintenance_mode_lock, flags); + release_ipmi_user(user, index); return rv; } @@ -1413,6 +1456,11 @@ int ipmi_set_gets_events(struct ipmi_user *user, bool val) struct ipmi_smi *intf = user->intf; struct ipmi_recv_msg *msg, *msg2; struct list_head msgs; + int index; + + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; INIT_LIST_HEAD(&msgs); @@ -1462,6 +1510,7 @@ int ipmi_set_gets_events(struct ipmi_user *user, bool val) out: spin_unlock_irqrestore(&intf->events_lock, flags); + release_ipmi_user(user, index); return 0; } @@ -1504,8 +1553,11 @@ int ipmi_register_for_cmd(struct ipmi_user *user, { struct ipmi_smi *intf = user->intf; struct cmd_rcvr *rcvr; - int rv = 0; + int rv = 0, index; + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; rcvr = kmalloc(sizeof(*rcvr), GFP_KERNEL); if (!rcvr) @@ -1531,6 +1583,7 @@ int ipmi_register_for_cmd(struct ipmi_user *user, mutex_unlock(&intf->cmd_rcvrs_mutex); if (rv) kfree(rcvr); + release_ipmi_user(user, index); return rv; } @@ -1544,7 +1597,11 @@ int ipmi_unregister_for_cmd(struct ipmi_user *user, struct ipmi_smi *intf = user->intf; struct cmd_rcvr *rcvr; struct cmd_rcvr *rcvrs = NULL; - int i, rv = -ENOENT; + int i, rv = -ENOENT, index; + + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; mutex_lock(&intf->cmd_rcvrs_mutex); for (i = 0; i < IPMI_NUM_CHANNELS; i++) { @@ -1565,12 +1622,14 @@ int ipmi_unregister_for_cmd(struct ipmi_user *user, } mutex_unlock(&intf->cmd_rcvrs_mutex); synchronize_rcu(); + release_ipmi_user(user, index); while (rcvrs) { atomic_dec(&intf->event_waiters); rcvr = rcvrs; rcvrs = rcvr->next; kfree(rcvr); } + return rv; } EXPORT_SYMBOL(ipmi_unregister_for_cmd); @@ -2065,8 +2124,10 @@ static int i_ipmi_request(struct ipmi_user *user, recv_msg = supplied_recv; else { recv_msg = ipmi_alloc_recv_msg(); - if (recv_msg == NULL) - return -ENOMEM; + if (recv_msg == NULL) { + rv = -ENOMEM; + goto out; + } } recv_msg->user_msg_data = user_msg_data; @@ -2076,7 +2137,8 @@ static int i_ipmi_request(struct ipmi_user *user, smi_msg = ipmi_alloc_smi_msg(); if (smi_msg == NULL) { ipmi_free_recv_msg(recv_msg); - return -ENOMEM; + rv = -ENOMEM; + goto out; } } @@ -2088,6 +2150,7 @@ static int i_ipmi_request(struct ipmi_user *user, recv_msg->user = user; if (user) + /* The put happens when the message is freed. */ kref_get(&user->refcount); recv_msg->msgid = msgid; /* @@ -2123,6 +2186,7 @@ static int i_ipmi_request(struct ipmi_user *user, } rcu_read_unlock(); +out: return rv; } @@ -2148,25 +2212,32 @@ int ipmi_request_settime(struct ipmi_user *user, unsigned int retry_time_ms) { unsigned char saddr = 0, lun = 0; - int rv; + int rv, index; if (!user) return -EINVAL; + + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; + rv = check_addr(user->intf, addr, &saddr, &lun); - if (rv) - return rv; - return i_ipmi_request(user, - user->intf, - addr, - msgid, - msg, - user_msg_data, - NULL, NULL, - priority, - saddr, - lun, - retries, - retry_time_ms); + if (!rv) + rv = i_ipmi_request(user, + user->intf, + addr, + msgid, + msg, + user_msg_data, + NULL, NULL, + priority, + saddr, + lun, + retries, + retry_time_ms); + + release_ipmi_user(user, index); + return rv; } EXPORT_SYMBOL(ipmi_request_settime); @@ -2180,25 +2251,32 @@ int ipmi_request_supply_msgs(struct ipmi_user *user, int priority) { unsigned char saddr = 0, lun = 0; - int rv; + int rv, index; if (!user) return -EINVAL; + + user = acquire_ipmi_user(user, &index); + if (!user) + return -ENODEV; + rv = check_addr(user->intf, addr, &saddr, &lun); - if (rv) - return rv; - return i_ipmi_request(user, - user->intf, - addr, - msgid, - msg, - user_msg_data, - supplied_smi, - supplied_recv, - priority, - saddr, - lun, - -1, 0); + if (!rv) + rv = i_ipmi_request(user, + user->intf, + addr, + msgid, + msg, + user_msg_data, + supplied_smi, + supplied_recv, + priority, + saddr, + lun, + -1, 0); + + release_ipmi_user(user, index); + return rv; } EXPORT_SYMBOL(ipmi_request_supply_msgs); @@ -3455,6 +3533,13 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, if (!intf) return -ENOMEM; + rv = init_srcu_struct(&intf->users_srcu); + if (rv) { + kfree(intf); + return rv; + } + + intf->bmc = &intf->tmp_bmc; INIT_LIST_HEAD(&intf->bmc->intfs); mutex_init(&intf->bmc->dyn_mutex); @@ -3507,7 +3592,6 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, intf->proc_dir = NULL; #endif - mutex_lock(&smi_watchers_mutex); mutex_lock(&ipmi_interfaces_mutex); /* Look for a hole in the numbers. */ i = 0; @@ -3552,11 +3636,10 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, if (intf->proc_dir) remove_proc_entries(intf); #endif - intf->handlers = NULL; list_del_rcu(&intf->link); mutex_unlock(&ipmi_interfaces_mutex); - mutex_unlock(&smi_watchers_mutex); - synchronize_rcu(); + synchronize_srcu(&ipmi_interfaces_srcu); + cleanup_srcu_struct(&intf->users_srcu); kref_put(&intf->refcount, intf_free); } else { /* @@ -3567,9 +3650,9 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, smp_wmb(); intf->intf_num = i; mutex_unlock(&ipmi_interfaces_mutex); + /* After this point the interface is legal to use. */ call_smi_watchers(i, intf->si_dev); - mutex_unlock(&smi_watchers_mutex); } return rv; @@ -3631,45 +3714,49 @@ static void cleanup_smi_msgs(struct ipmi_smi *intf) int ipmi_unregister_smi(struct ipmi_smi *intf) { struct ipmi_smi_watcher *w; - int intf_num = intf->intf_num; - struct ipmi_user *user; + int intf_num = intf->intf_num, index; - mutex_lock(&smi_watchers_mutex); mutex_lock(&ipmi_interfaces_mutex); intf->intf_num = -1; intf->in_shutdown = true; list_del_rcu(&intf->link); mutex_unlock(&ipmi_interfaces_mutex); - synchronize_rcu(); + synchronize_srcu(&ipmi_interfaces_srcu); + + /* At this point no users can be added to the interface. */ + + /* + * Call all the watcher interfaces to tell them that + * an interface is going away. + */ + mutex_lock(&smi_watchers_mutex); + list_for_each_entry(w, &smi_watchers, link) + w->smi_gone(intf_num); + mutex_unlock(&smi_watchers_mutex); + + index = srcu_read_lock(&intf->users_srcu); + while (!list_empty(&intf->users)) { + struct ipmi_user *user = + container_of(list_next_rcu(&intf->users), + struct ipmi_user, link); + + _ipmi_destroy_user(user); + } + srcu_read_unlock(&intf->users_srcu, index); + + if (intf->handlers->shutdown) + intf->handlers->shutdown(intf->send_info); cleanup_smi_msgs(intf); - /* Clean up the effects of users on the lower-level software. */ - mutex_lock(&ipmi_interfaces_mutex); - rcu_read_lock(); - list_for_each_entry_rcu(user, &intf->users, link) { - module_put(intf->handlers->owner); - if (intf->handlers->dec_usecount) - intf->handlers->dec_usecount(intf->send_info); - } - rcu_read_unlock(); - intf->handlers = NULL; - mutex_unlock(&ipmi_interfaces_mutex); - #ifdef CONFIG_IPMI_PROC_INTERFACE remove_proc_entries(intf); #endif ipmi_bmc_unregister(intf); - /* - * Call all the watcher interfaces to tell them that - * an interface is gone. - */ - list_for_each_entry(w, &smi_watchers, link) - w->smi_gone(intf_num); - mutex_unlock(&smi_watchers_mutex); - + cleanup_srcu_struct(&intf->users_srcu); kref_put(&intf->refcount, intf_free); + return 0; } EXPORT_SYMBOL(ipmi_unregister_smi); @@ -4141,8 +4228,7 @@ static int handle_read_event_rsp(struct ipmi_smi *intf, struct ipmi_recv_msg *recv_msg, *recv_msg2; struct list_head msgs; struct ipmi_user *user; - int rv = 0; - int deliver_count = 0; + int rv = 0, deliver_count = 0, index; unsigned long flags; if (msg->rsp_size < 19) { @@ -4166,7 +4252,7 @@ static int handle_read_event_rsp(struct ipmi_smi *intf, * Allocate and fill in one message for every user that is * getting events. */ - rcu_read_lock(); + index = srcu_read_lock(&intf->users_srcu); list_for_each_entry_rcu(user, &intf->users, link) { if (!user->gets_events) continue; @@ -4195,7 +4281,7 @@ static int handle_read_event_rsp(struct ipmi_smi *intf, kref_get(&user->refcount); list_add_tail(&recv_msg->link, &msgs); } - rcu_read_unlock(); + srcu_read_unlock(&intf->users_srcu, index); if (deliver_count) { /* Now deliver all the messages. */ @@ -4242,7 +4328,7 @@ static int handle_bmc_rsp(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { struct ipmi_recv_msg *recv_msg; - struct ipmi_user *user; + struct ipmi_system_interface_addr *smi_addr; recv_msg = (struct ipmi_recv_msg *) msg->user_data; if (recv_msg == NULL) { @@ -4251,30 +4337,19 @@ static int handle_bmc_rsp(struct ipmi_smi *intf, return 0; } - user = recv_msg->user; - /* Make sure the user still exists. */ - if (user && !user->valid) { - /* The user for the message went away, so give up. */ - ipmi_inc_stat(intf, unhandled_local_responses); - ipmi_free_recv_msg(recv_msg); - } else { - struct ipmi_system_interface_addr *smi_addr; - - ipmi_inc_stat(intf, handled_local_responses); - recv_msg->recv_type = IPMI_RESPONSE_RECV_TYPE; - recv_msg->msgid = msg->msgid; - smi_addr = ((struct ipmi_system_interface_addr *) - &recv_msg->addr); - smi_addr->addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE; - smi_addr->channel = IPMI_BMC_CHANNEL; - smi_addr->lun = msg->rsp[0] & 3; - recv_msg->msg.netfn = msg->rsp[0] >> 2; - recv_msg->msg.cmd = msg->rsp[1]; - memcpy(recv_msg->msg_data, &msg->rsp[2], msg->rsp_size - 2); - recv_msg->msg.data = recv_msg->msg_data; - recv_msg->msg.data_len = msg->rsp_size - 2; - deliver_local_response(intf, recv_msg); - } + recv_msg->recv_type = IPMI_RESPONSE_RECV_TYPE; + recv_msg->msgid = msg->msgid; + smi_addr = ((struct ipmi_system_interface_addr *) + &recv_msg->addr); + smi_addr->addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE; + smi_addr->channel = IPMI_BMC_CHANNEL; + smi_addr->lun = msg->rsp[0] & 3; + recv_msg->msg.netfn = msg->rsp[0] >> 2; + recv_msg->msg.cmd = msg->rsp[1]; + memcpy(recv_msg->msg_data, &msg->rsp[2], msg->rsp_size - 2); + recv_msg->msg.data = recv_msg->msg_data; + recv_msg->msg.data_len = msg->rsp_size - 2; + deliver_local_response(intf, recv_msg); return 0; } @@ -4327,7 +4402,7 @@ static int handle_one_recv_msg(struct ipmi_smi *intf, * It's a response to a response we sent. For this we * deliver a send message response to the user. */ - struct ipmi_recv_msg *recv_msg = msg->user_data; + struct ipmi_recv_msg *recv_msg = msg->user_data; requeue = 0; if (msg->rsp_size < 2) @@ -4342,10 +4417,6 @@ static int handle_one_recv_msg(struct ipmi_smi *intf, if (!recv_msg) goto out; - /* Make sure the user still exists. */ - if (!recv_msg->user || !recv_msg->user->valid) - goto out; - recv_msg->recv_type = IPMI_RESPONSE_RESPONSE_TYPE; recv_msg->msg.data = recv_msg->msg_data; recv_msg->msg.data_len = 1; @@ -4488,14 +4559,15 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf) */ if (atomic_add_unless(&intf->watchdog_pretimeouts_to_deliver, -1, 0)) { struct ipmi_user *user; + int index; - rcu_read_lock(); + index = srcu_read_lock(&intf->users_srcu); list_for_each_entry_rcu(user, &intf->users, link) { if (user->handler->ipmi_watchdog_pretimeout) user->handler->ipmi_watchdog_pretimeout( user->handler_data); } - rcu_read_unlock(); + srcu_read_unlock(&intf->users_srcu, index); } } @@ -4662,8 +4734,7 @@ static void check_msg_timeout(struct ipmi_smi *intf, struct seq_table *ent, int slot, unsigned long *flags, unsigned int *waiting_msgs) { - struct ipmi_recv_msg *msg; - const struct ipmi_smi_handlers *handlers; + struct ipmi_recv_msg *msg; if (intf->in_shutdown) return; @@ -4721,8 +4792,7 @@ static void check_msg_timeout(struct ipmi_smi *intf, struct seq_table *ent, * only for messages to the local MC, which don't get * resent. */ - handlers = intf->handlers; - if (handlers) { + if (intf->handlers) { if (is_lan_addr(&ent->recv_msg->addr)) ipmi_inc_stat(intf, retransmitted_lan_commands); @@ -4730,7 +4800,7 @@ static void check_msg_timeout(struct ipmi_smi *intf, struct seq_table *ent, ipmi_inc_stat(intf, retransmitted_ipmb_commands); - smi_send(intf, handlers, smi_msg, 0); + smi_send(intf, intf->handlers, smi_msg, 0); } else ipmi_free_smi_msg(smi_msg); @@ -4822,12 +4892,12 @@ static atomic_t stop_operation; static void ipmi_timeout(struct timer_list *unused) { struct ipmi_smi *intf; - int nt = 0; + int nt = 0, index; if (atomic_read(&stop_operation)) return; - rcu_read_lock(); + index = srcu_read_lock(&ipmi_interfaces_srcu); list_for_each_entry_rcu(intf, &ipmi_interfaces, link) { int lnt = 0; @@ -4850,7 +4920,7 @@ static void ipmi_timeout(struct timer_list *unused) nt += lnt; } - rcu_read_unlock(); + srcu_read_unlock(&ipmi_interfaces_srcu, index); if (nt) mod_timer(&ipmi_timer, jiffies + IPMI_TIMEOUT_JIFFIES); From 7960f18a56475bf2177c5ff56c72eb4c12c56440 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 22:03:35 -0500 Subject: [PATCH 18/35] ipmi_si: Convert over to a shutdown handler Move the shutdown handling to a shutdown function called from the IPMI core code. That makes for a cleaner shutdown. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_intf.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 24702ab853a8..00a324060dcd 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1209,9 +1209,11 @@ static void set_maintenance_mode(void *send_info, bool enable) atomic_set(&smi_info->req_events, 0); } +static void shutdown_smi(void *send_info); static const struct ipmi_smi_handlers handlers = { .owner = THIS_MODULE, .start_processing = smi_start_processing, + .shutdown = shutdown_smi, .get_smi_info = get_smi_info, .sender = sender, .request_events = request_events, @@ -2301,20 +2303,9 @@ static int init_ipmi_si(void) } module_init(init_ipmi_si); -static void shutdown_one_si(struct smi_info *smi_info) +static void shutdown_smi(void *send_info) { - int rv = 0; - - if (smi_info->intf) { - ipmi_smi_t intf = smi_info->intf; - - smi_info->intf = NULL; - rv = ipmi_unregister_smi(intf); - if (rv) { - pr_err(PFX "Unable to unregister device: errno=%d\n", - rv); - } - } + struct smi_info *smi_info = send_info; if (smi_info->dev_group_added) { device_remove_group(smi_info->io.dev, &ipmi_si_dev_attr_group); @@ -2372,6 +2363,20 @@ static void shutdown_one_si(struct smi_info *smi_info) smi_info->si_sm = NULL; } +static void shutdown_one_si(struct smi_info *smi_info) +{ + int rv; + ipmi_smi_t intf = smi_info->intf; + + if (!intf) + return; + + smi_info->intf = NULL; + rv = ipmi_unregister_smi(intf); + if (rv) + pr_err(PFX "Unable to unregister device: errno=%d\n", rv); +} + static void cleanup_one_si(struct smi_info *smi_info) { if (!smi_info) From a313dec6401f0ceaec77262b411d77c18aef27ee Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 22:06:45 -0500 Subject: [PATCH 19/35] ipmi_ssif: Convert over to a shutdown handler Move the shutdown handling to a shutdown function called from the IPMI core code. That makes for a cleaner shutdown. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_ssif.c | 55 ++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 2db97fd08a8f..0f3dd94519d9 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1225,25 +1225,9 @@ static const struct attribute_group ipmi_ssif_dev_attr_group = { .attrs = ipmi_ssif_dev_attrs, }; -static int ssif_remove(struct i2c_client *client) +static void shutdown_ssif(void *send_info) { - struct ssif_info *ssif_info = i2c_get_clientdata(client); - struct ssif_addr_info *addr_info; - int rv; - - if (!ssif_info) - return 0; - - /* - * After this point, we won't deliver anything asychronously - * to the message handler. We can unregister ourself. - */ - rv = ipmi_unregister_smi(ssif_info->intf); - if (rv) { - pr_err(PFX "Unable to unregister device: errno=%d\n", rv); - return rv; - } - ssif_info->intf = NULL; + struct ssif_info *ssif_info = send_info; device_remove_group(&ssif_info->client->dev, &ipmi_ssif_dev_attr_group); dev_set_drvdata(&ssif_info->client->dev, NULL); @@ -1259,6 +1243,33 @@ static int ssif_remove(struct i2c_client *client) kthread_stop(ssif_info->thread); } + /* + * No message can be outstanding now, we have removed the + * upper layer and it permitted us to do so. + */ + kfree(ssif_info); +} + +static int ssif_remove(struct i2c_client *client) +{ + struct ssif_info *ssif_info = i2c_get_clientdata(client); + struct ipmi_smi *intf; + struct ssif_addr_info *addr_info; + int rv; + + if (!ssif_info) + return 0; + + /* + * After this point, we won't deliver anything asychronously + * to the message handler. We can unregister ourself. + */ + intf = ssif_info->intf; + ssif_info->intf = NULL; + rv = ipmi_unregister_smi(intf); + if (rv) + pr_err(PFX "Unable to unregister device: errno=%d\n", rv); + list_for_each_entry(addr_info, &ssif_infos, link) { if (addr_info->client == client) { addr_info->client = NULL; @@ -1266,12 +1277,7 @@ static int ssif_remove(struct i2c_client *client) } } - /* - * No message can be outstanding now, we have removed the - * upper layer and it permitted us to do so. - */ - kfree(ssif_info); - return 0; + return rv; } static int do_cmd(struct i2c_client *client, int len, unsigned char *msg, @@ -1697,6 +1703,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) ssif_info->handlers.owner = THIS_MODULE; ssif_info->handlers.start_processing = ssif_start_processing; + ssif_info->handlers.shutdown = shutdown_ssif; ssif_info->handlers.get_smi_info = get_smi_info; ssif_info->handlers.sender = sender; ssif_info->handlers.request_events = request_events; From 8d17929ad50f2c9d4cf55e8f3eb249a60f429a0d Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 22:07:33 -0500 Subject: [PATCH 20/35] ipmi: Remove condition on interface shutdown Now that the interfaces have shutdown handlers, this no longer needs to be conditional. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index a27b50ac2b7f..7ddadab65f33 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -3744,8 +3744,7 @@ int ipmi_unregister_smi(struct ipmi_smi *intf) } srcu_read_unlock(&intf->users_srcu, index); - if (intf->handlers->shutdown) - intf->handlers->shutdown(intf->send_info); + intf->handlers->shutdown(intf->send_info); cleanup_smi_msgs(intf); From f0258c95304e7712143bddd3a2af53bc9e549a7b Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 22:09:36 -0500 Subject: [PATCH 21/35] ipmi_ssif: Remove usecount handling Now that we can handle hot remove there is no need for usecounts for interfaces. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_ssif.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 0f3dd94519d9..8c72f271d4b4 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1094,25 +1094,6 @@ static void request_events(void *send_info) } } -static int inc_usecount(void *send_info) -{ - struct ssif_info *ssif_info = send_info; - - if (!i2c_get_adapter(i2c_adapter_id(ssif_info->client->adapter))) - return -ENODEV; - - i2c_use_client(ssif_info->client); - return 0; -} - -static void dec_usecount(void *send_info) -{ - struct ssif_info *ssif_info = send_info; - - i2c_release_client(ssif_info->client); - i2c_put_adapter(ssif_info->client->adapter); -} - static int ssif_start_processing(void *send_info, struct ipmi_smi *intf) { @@ -1707,8 +1688,6 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) ssif_info->handlers.get_smi_info = get_smi_info; ssif_info->handlers.sender = sender; ssif_info->handlers.request_events = request_events; - ssif_info->handlers.inc_usecount = inc_usecount; - ssif_info->handlers.dec_usecount = dec_usecount; { unsigned int thread_num; From 8eb005bf6e58ca08802ac8170e5d3c8486cb2d79 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 5 Apr 2018 22:10:08 -0500 Subject: [PATCH 22/35] ipmi: Remove usecount function from interfaces All the users are now gone. Signed-off-by: Corey Minyard --- include/linux/ipmi_smi.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h index bdcda4741c89..16662b0423bf 100644 --- a/include/linux/ipmi_smi.h +++ b/include/linux/ipmi_smi.h @@ -140,15 +140,6 @@ struct ipmi_smi_handlers { * block. */ void (*set_maintenance_mode)(void *send_info, bool enable); - - /* - * Tell the handler that we are using it/not using it. The - * message handler get the modules that this handler belongs - * to; this function lets the SMI claim any modules that it - * uses. These may be NULL if this is not required. - */ - int (*inc_usecount)(void *send_info); - void (*dec_usecount)(void *send_info); }; struct ipmi_device_id { From e56710d26eba07abca34c39c50e02e854436000a Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Fri, 6 Apr 2018 16:30:36 -0500 Subject: [PATCH 23/35] ipmi_devintf: Add an error return on invalid ioctls Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_devintf.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/char/ipmi/ipmi_devintf.c b/drivers/char/ipmi/ipmi_devintf.c index b36a91fb7cd5..1a486aec99b6 100644 --- a/drivers/char/ipmi/ipmi_devintf.c +++ b/drivers/char/ipmi/ipmi_devintf.c @@ -608,6 +608,10 @@ static long ipmi_ioctl(struct file *file, rv = ipmi_set_maintenance_mode(priv->user, mode); break; } + + default: + rv = -ENOTTY; + break; } return rv; From 6a0d23ed338ed7015128378e0ceec03eaa3d91e2 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 11 Apr 2018 12:41:33 -0500 Subject: [PATCH 24/35] ipmi: ipmi_unregister_smi() cannot fail, have it return void Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 4 +--- drivers/char/ipmi/ipmi_si_intf.c | 5 +---- drivers/char/ipmi/ipmi_ssif.c | 4 +--- include/linux/ipmi_smi.h | 2 +- 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 7ddadab65f33..946bfcb1eeee 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -3711,7 +3711,7 @@ static void cleanup_smi_msgs(struct ipmi_smi *intf) } } -int ipmi_unregister_smi(struct ipmi_smi *intf) +void ipmi_unregister_smi(struct ipmi_smi *intf) { struct ipmi_smi_watcher *w; int intf_num = intf->intf_num, index; @@ -3755,8 +3755,6 @@ int ipmi_unregister_smi(struct ipmi_smi *intf) cleanup_srcu_struct(&intf->users_srcu); kref_put(&intf->refcount, intf_free); - - return 0; } EXPORT_SYMBOL(ipmi_unregister_smi); diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 00a324060dcd..2222caf4bab7 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -2365,16 +2365,13 @@ static void shutdown_smi(void *send_info) static void shutdown_one_si(struct smi_info *smi_info) { - int rv; ipmi_smi_t intf = smi_info->intf; if (!intf) return; smi_info->intf = NULL; - rv = ipmi_unregister_smi(intf); - if (rv) - pr_err(PFX "Unable to unregister device: errno=%d\n", rv); + ipmi_unregister_smi(intf); } static void cleanup_one_si(struct smi_info *smi_info) diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 8c72f271d4b4..17cae7a41b70 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1247,9 +1247,7 @@ static int ssif_remove(struct i2c_client *client) */ intf = ssif_info->intf; ssif_info->intf = NULL; - rv = ipmi_unregister_smi(intf); - if (rv) - pr_err(PFX "Unable to unregister device: errno=%d\n", rv); + ipmi_unregister_smi(intf); list_for_each_entry(addr_info, &ssif_infos, link) { if (addr_info->client == client) { diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h index 16662b0423bf..26ba57c307f0 100644 --- a/include/linux/ipmi_smi.h +++ b/include/linux/ipmi_smi.h @@ -220,7 +220,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, * Remove a low-level interface from the IPMI driver. This will * return an error if the interface is still in use by a user. */ -int ipmi_unregister_smi(ipmi_smi_t intf); +void ipmi_unregister_smi(ipmi_smi_t intf); /* * The lower layer reports received messages through this interface. From 5ce1a7dc806efb2181e93d4a088b281c4cff4eaa Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 11 Apr 2018 13:11:54 -0500 Subject: [PATCH 25/35] ipmi: Get rid of ipmi_user_t and ipmi_smi_t in include files Convert over to struct ipmi_user * and struct ipmi_smi *. Signed-off-by: Corey Minyard --- include/linux/ipmi.h | 34 +++++++++++++++++----------------- include/linux/ipmi_smi.h | 12 ++++++------ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/include/linux/ipmi.h b/include/linux/ipmi.h index d89bea1e457a..41f5c086f670 100644 --- a/include/linux/ipmi.h +++ b/include/linux/ipmi.h @@ -45,7 +45,7 @@ struct ipmi_recv_msg { */ int recv_type; - ipmi_user_t user; + struct ipmi_user *user; struct ipmi_addr addr; long msgid; struct kernel_ipmi_msg msg; @@ -111,7 +111,7 @@ struct ipmi_user_hndl { int ipmi_create_user(unsigned int if_num, const struct ipmi_user_hndl *handler, void *handler_data, - ipmi_user_t *user); + struct ipmi_user **user); /* * Destroy the given user of the IPMI layer. Note that after this @@ -121,10 +121,10 @@ int ipmi_create_user(unsigned int if_num, * the users before you destroy the callback structures, it should be * safe, too. */ -int ipmi_destroy_user(ipmi_user_t user); +int ipmi_destroy_user(struct ipmi_user *user); /* Get the IPMI version of the BMC we are talking to. */ -int ipmi_get_version(ipmi_user_t user, +int ipmi_get_version(struct ipmi_user *user, unsigned char *major, unsigned char *minor); @@ -137,16 +137,16 @@ int ipmi_get_version(ipmi_user_t user, * it for everyone else. Note that each channel can have its own * address. */ -int ipmi_set_my_address(ipmi_user_t user, +int ipmi_set_my_address(struct ipmi_user *user, unsigned int channel, unsigned char address); -int ipmi_get_my_address(ipmi_user_t user, +int ipmi_get_my_address(struct ipmi_user *user, unsigned int channel, unsigned char *address); -int ipmi_set_my_LUN(ipmi_user_t user, +int ipmi_set_my_LUN(struct ipmi_user *user, unsigned int channel, unsigned char LUN); -int ipmi_get_my_LUN(ipmi_user_t user, +int ipmi_get_my_LUN(struct ipmi_user *user, unsigned int channel, unsigned char *LUN); @@ -163,7 +163,7 @@ int ipmi_get_my_LUN(ipmi_user_t user, * it makes no sense to do it here. However, this can be used if you * have unusual requirements. */ -int ipmi_request_settime(ipmi_user_t user, +int ipmi_request_settime(struct ipmi_user *user, struct ipmi_addr *addr, long msgid, struct kernel_ipmi_msg *msg, @@ -181,7 +181,7 @@ int ipmi_request_settime(ipmi_user_t user, * change as the system changes, so don't use it unless you REALLY * have to. */ -int ipmi_request_supply_msgs(ipmi_user_t user, +int ipmi_request_supply_msgs(struct ipmi_user *user, struct ipmi_addr *addr, long msgid, struct kernel_ipmi_msg *msg, @@ -197,7 +197,7 @@ int ipmi_request_supply_msgs(ipmi_user_t user, * way. This is useful if you need to spin waiting for something to * happen in the IPMI driver. */ -void ipmi_poll_interface(ipmi_user_t user); +void ipmi_poll_interface(struct ipmi_user *user); /* * When commands come in to the SMS, the user can register to receive @@ -208,11 +208,11 @@ void ipmi_poll_interface(ipmi_user_t user); * error. Channels are specified as a bitfield, use IPMI_CHAN_ALL to * mean all channels. */ -int ipmi_register_for_cmd(ipmi_user_t user, +int ipmi_register_for_cmd(struct ipmi_user *user, unsigned char netfn, unsigned char cmd, unsigned int chans); -int ipmi_unregister_for_cmd(ipmi_user_t user, +int ipmi_unregister_for_cmd(struct ipmi_user *user, unsigned char netfn, unsigned char cmd, unsigned int chans); @@ -243,8 +243,8 @@ int ipmi_unregister_for_cmd(ipmi_user_t user, * * See the IPMI_MAINTENANCE_MODE_xxx defines for what the mode means. */ -int ipmi_get_maintenance_mode(ipmi_user_t user); -int ipmi_set_maintenance_mode(ipmi_user_t user, int mode); +int ipmi_get_maintenance_mode(struct ipmi_user *user); +int ipmi_set_maintenance_mode(struct ipmi_user *user, int mode); /* * When the user is created, it will not receive IPMI events by @@ -252,7 +252,7 @@ int ipmi_set_maintenance_mode(ipmi_user_t user, int mode); * The first user that sets this to TRUE will receive all events that * have been queued while no one was waiting for events. */ -int ipmi_set_gets_events(ipmi_user_t user, bool val); +int ipmi_set_gets_events(struct ipmi_user *user, bool val); /* * Called when a new SMI is registered. This will also be called on @@ -330,7 +330,7 @@ struct ipmi_smi_info { union ipmi_smi_info_union addr_info; }; -/* This is to get the private info of ipmi_smi_t */ +/* This is to get the private info of struct ipmi_smi */ extern int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data); #endif /* __LINUX_IPMI_H */ diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h index 26ba57c307f0..0d438662a821 100644 --- a/include/linux/ipmi_smi.h +++ b/include/linux/ipmi_smi.h @@ -69,8 +69,8 @@ struct ipmi_smi_handlers { * not be NULL, the lower layer must take the interface from * this call. */ - int (*start_processing)(void *send_info, - ipmi_smi_t new_intf); + int (*start_processing)(void *send_info, + struct ipmi_smi *new_intf); /* * When called, the low-level interface should disable all @@ -220,7 +220,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, * Remove a low-level interface from the IPMI driver. This will * return an error if the interface is still in use by a user. */ -void ipmi_unregister_smi(ipmi_smi_t intf); +void ipmi_unregister_smi(struct ipmi_smi *intf); /* * The lower layer reports received messages through this interface. @@ -228,11 +228,11 @@ void ipmi_unregister_smi(ipmi_smi_t intf); * the lower layer gets an error sending a message, it should format * an error response in the message response. */ -void ipmi_smi_msg_received(ipmi_smi_t intf, +void ipmi_smi_msg_received(struct ipmi_smi *intf, struct ipmi_smi_msg *msg); /* The lower layer received a watchdog pre-timeout on interface. */ -void ipmi_smi_watchdog_pretimeout(ipmi_smi_t intf); +void ipmi_smi_watchdog_pretimeout(struct ipmi_smi *intf); struct ipmi_smi_msg *ipmi_alloc_smi_msg(void); static inline void ipmi_free_smi_msg(struct ipmi_smi_msg *msg) @@ -246,7 +246,7 @@ static inline void ipmi_free_smi_msg(struct ipmi_smi_msg *msg) * directory for this interface. Note that the entry will * automatically be dstroyed when the interface is destroyed. */ -int ipmi_smi_add_proc_entry(ipmi_smi_t smi, char *name, +int ipmi_smi_add_proc_entry(struct ipmi_smi *smi, char *name, const struct file_operations *proc_ops, void *data); #endif From ebb339a597b5f06cea5ea9d984c56bc4d2fa11f6 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 11 Apr 2018 13:22:34 -0500 Subject: [PATCH 26/35] ipmi_ssif: Get rid of unused intf_num Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_ssif.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 17cae7a41b70..4cbaa5ebf897 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -194,7 +194,6 @@ typedef void (*ssif_i2c_done)(struct ssif_info *ssif_info, int result, struct ssif_info { struct ipmi_smi *intf; - int intf_num; spinlock_t lock; struct ipmi_smi_msg *waiting_msg; struct ipmi_smi_msg *curr_msg; @@ -290,8 +289,6 @@ struct ssif_info { static bool initialized; -static atomic_t next_intf = ATOMIC_INIT(0); - static void return_hosed_msg(struct ssif_info *ssif_info, struct ipmi_smi_msg *msg); static void start_next_msg(struct ssif_info *ssif_info, unsigned long *flags); @@ -1663,8 +1660,6 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) } found: - ssif_info->intf_num = atomic_inc_return(&next_intf); - if (ssif_dbg_probe) { pr_info("ssif_probe: i2c_probe found device at i2c address %x\n", client->addr); From 0fbecb4f478232070d3161e7b4a222eaba0bcbcd Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 11 Apr 2018 13:24:27 -0500 Subject: [PATCH 27/35] ipmi: Remove smi->intf checks Due to changes in the way shutdown is done, it is no longer required to check that the interface is set. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_intf.c | 11 +++-------- drivers/char/ipmi/ipmi_ssif.c | 12 +++--------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 2222caf4bab7..de4cfbc7ee0d 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -287,10 +287,7 @@ static void deliver_recv_msg(struct smi_info *smi_info, struct ipmi_smi_msg *msg) { /* Deliver the message to the upper layer. */ - if (smi_info->intf) - ipmi_smi_msg_received(smi_info->intf, msg); - else - ipmi_free_smi_msg(msg); + ipmi_smi_msg_received(smi_info->intf, msg); } static void return_hosed_msg(struct smi_info *smi_info, int cCode) @@ -471,8 +468,7 @@ static void handle_flags(struct smi_info *smi_info) start_clear_flags(smi_info); smi_info->msg_flags &= ~WDT_PRE_TIMEOUT_INT; - if (smi_info->intf) - ipmi_smi_watchdog_pretimeout(smi_info->intf); + ipmi_smi_watchdog_pretimeout(smi_info->intf); } else if (smi_info->msg_flags & RECEIVE_MSG_AVAIL) { /* Messages available. */ smi_info->curr_msg = alloc_msg_handle_irq(smi_info); @@ -798,8 +794,7 @@ static enum si_sm_result smi_event_handler(struct smi_info *smi_info, * We prefer handling attn over new messages. But don't do * this if there is not yet an upper layer to handle anything. */ - if (likely(smi_info->intf) && - (si_sm_result == SI_SM_ATTN || smi_info->got_attn)) { + if (si_sm_result == SI_SM_ATTN || smi_info->got_attn) { unsigned char msg[2]; if (smi_info->si_state != SI_NORMAL) { diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 4cbaa5ebf897..ecabfe27c828 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -312,17 +312,13 @@ static void ipmi_ssif_unlock_cond(struct ssif_info *ssif_info, static void deliver_recv_msg(struct ssif_info *ssif_info, struct ipmi_smi_msg *msg) { - struct ipmi_smi *intf = ssif_info->intf; - - if (!intf) { - ipmi_free_smi_msg(msg); - } else if (msg->rsp_size < 0) { + if (msg->rsp_size < 0) { return_hosed_msg(ssif_info, msg); pr_err(PFX "Malformed message in deliver_recv_msg: rsp_size = %d\n", msg->rsp_size); } else { - ipmi_smi_msg_received(intf, msg); + ipmi_smi_msg_received(ssif_info->intf, msg); } } @@ -449,12 +445,10 @@ static void start_recv_msg_fetch(struct ssif_info *ssif_info, static void handle_flags(struct ssif_info *ssif_info, unsigned long *flags) { if (ssif_info->msg_flags & WDT_PRE_TIMEOUT_INT) { - struct ipmi_smi *intf = ssif_info->intf; /* Watchdog pre-timeout */ ssif_inc_stat(ssif_info, watchdog_pretimeouts); start_clear_flags(ssif_info, flags); - if (intf) - ipmi_smi_watchdog_pretimeout(intf); + ipmi_smi_watchdog_pretimeout(ssif_info->intf); } else if (ssif_info->msg_flags & RECEIVE_MSG_AVAIL) /* Messages available. */ start_recv_msg_fetch(ssif_info, flags); From 57bccb4e8755d092f4c3c9e356a18f89a1488490 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 11 Apr 2018 13:28:34 -0500 Subject: [PATCH 28/35] ipmi_si: Rename intf_num to si_num There is already an intf_num in the main IPMI device structure, use a different name in the ipmi_si code to avoid confusion. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_intf.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index de4cfbc7ee0d..7352d8fe73f8 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -122,7 +122,7 @@ enum si_stat_indexes { }; struct smi_info { - int intf_num; + int si_num; struct ipmi_smi *intf; struct si_sm_data *si_sm; const struct si_sm_handlers *handlers; @@ -957,8 +957,8 @@ static inline int ipmi_thread_busy_wait(enum si_sm_result smi_result, { unsigned int max_busy_us = 0; - if (smi_info->intf_num < num_max_busy_us) - max_busy_us = kipmid_max_busy_us[smi_info->intf_num]; + if (smi_info->si_num < num_max_busy_us) + max_busy_us = kipmid_max_busy_us[smi_info->si_num]; if (max_busy_us == 0 || smi_result != SI_SM_CALL_WITH_DELAY) ipmi_si_set_not_busy(busy_until); else if (!ipmi_si_is_busy(busy_until)) { @@ -1160,8 +1160,8 @@ static int smi_start_processing(void *send_info, /* * Check if the user forcefully enabled the daemon. */ - if (new_smi->intf_num < num_force_kipmid) - enable = force_kipmid[new_smi->intf_num]; + if (new_smi->si_num < num_force_kipmid) + enable = force_kipmid[new_smi->si_num]; /* * The BT interface is efficient enough to not need a thread, * and there is no need for a thread if we have interrupts. @@ -1171,7 +1171,7 @@ static int smi_start_processing(void *send_info, if (enable) { new_smi->thread = kthread_run(ipmi_thread, new_smi, - "kipmi%d", new_smi->intf_num); + "kipmi%d", new_smi->si_num); if (IS_ERR(new_smi->thread)) { dev_notice(new_smi->io.dev, "Could not start" " kernel thread due to error %ld, only using" @@ -2053,19 +2053,19 @@ static int try_smi_init(struct smi_info *new_smi) goto out_err; } - new_smi->intf_num = smi_num; + new_smi->si_num = smi_num; /* Do this early so it's available for logs. */ if (!new_smi->io.dev) { init_name = kasprintf(GFP_KERNEL, "ipmi_si.%d", - new_smi->intf_num); + new_smi->si_num); /* * If we don't already have a device from something * else (like PCI), then register a new one. */ new_smi->pdev = platform_device_alloc("ipmi_si", - new_smi->intf_num); + new_smi->si_num); if (!new_smi->pdev) { pr_err(PFX "Unable to allocate platform device\n"); rv = -ENOMEM; From 93c303d2045b30572d8d5e74d3ad80692acfebbe Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 11 Apr 2018 13:53:52 -0500 Subject: [PATCH 29/35] ipmi_si: Clean up shutdown a bit Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_intf.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 7352d8fe73f8..f2c39bf91bf5 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -261,7 +261,6 @@ static int num_max_busy_us; static bool unload_when_empty = true; static int try_smi_init(struct smi_info *smi); -static void shutdown_one_si(struct smi_info *smi_info); static void cleanup_one_si(struct smi_info *smi_info); static void cleanup_ipmi_si(void); @@ -2003,14 +2002,8 @@ int ipmi_si_add_smi(struct si_sm_io *io) list_add_tail(&new_smi->link, &smi_infos); - if (initialized) { + if (initialized) rv = try_smi_init(new_smi); - if (rv) { - cleanup_one_si(new_smi); - mutex_unlock(&smi_infos_lock); - return rv; - } - } out_err: mutex_unlock(&smi_infos_lock); return rv; @@ -2220,7 +2213,8 @@ static int try_smi_init(struct smi_info *new_smi) return 0; out_err: - shutdown_one_si(new_smi); + ipmi_unregister_smi(new_smi->intf); + new_smi->intf = NULL; kfree(init_name); @@ -2358,17 +2352,10 @@ static void shutdown_smi(void *send_info) smi_info->si_sm = NULL; } -static void shutdown_one_si(struct smi_info *smi_info) -{ - ipmi_smi_t intf = smi_info->intf; - - if (!intf) - return; - - smi_info->intf = NULL; - ipmi_unregister_smi(intf); -} - +/* + * Must be called with smi_infos_lock held, to serialize the + * smi_info->intf check. + */ static void cleanup_one_si(struct smi_info *smi_info) { if (!smi_info) @@ -2376,7 +2363,10 @@ static void cleanup_one_si(struct smi_info *smi_info) list_del(&smi_info->link); - shutdown_one_si(smi_info); + if (smi_info->intf) { + ipmi_unregister_smi(smi_info->intf); + smi_info->intf = NULL; + } if (smi_info->pdev) { if (smi_info->pdev_registered) From 6b2e54f7babea6c782a9c1af9d01c7f999bb4c7d Mon Sep 17 00:00:00 2001 From: Haiyue Wang Date: Thu, 22 Mar 2018 20:50:15 +0800 Subject: [PATCH 30/35] ipmi: add an NPCM7xx KCS BMC driver This driver exposes the Keyboard Controller Style (KCS) interface on Novoton NPCM7xx SoCs as a character device. Such SOCs are commonly used as a BaseBoard Management Controller (BMC) on a server board, and KCS interface is commonly used to perform the in-band IPMI communication between the server and its BMC. Signed-off-by: Avi Fishman Signed-off-by: Haiyue Wang Signed-off-by: Corey Minyard --- .../bindings/ipmi/npcm7xx-kcs-bmc.txt | 39 ++++ drivers/char/ipmi/Kconfig | 15 ++ drivers/char/ipmi/Makefile | 1 + drivers/char/ipmi/kcs_bmc_npcm7xx.c | 204 ++++++++++++++++++ 4 files changed, 259 insertions(+) create mode 100644 Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt create mode 100644 drivers/char/ipmi/kcs_bmc_npcm7xx.c diff --git a/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt b/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt new file mode 100644 index 000000000000..3538a214fff1 --- /dev/null +++ b/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt @@ -0,0 +1,39 @@ +* Nuvoton NPCM7xx KCS (Keyboard Controller Style) IPMI interface + +The Nuvoton SOCs (NPCM7xx) are commonly used as BMCs +(Baseboard Management Controllers) and the KCS interface can be +used to perform in-band IPMI communication with their host. + +Required properties: +- compatible : should be one of + "nuvoton,npcm750-kcs-bmc" +- interrupts : interrupt generated by the controller +- kcs_chan : The KCS channel number in the controller + +Example: + + lpc_kcs: lpc_kcs@f0007000 { + compatible = "nuvoton,npcm750-lpc-kcs", "simple-mfd", "syscon"; + reg = <0xf0007000 0x40>; + reg-io-width = <1>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0xf0007000 0x40>; + + kcs1: kcs1@0 { + compatible = "nuvoton,npcm750-kcs-bmc"; + reg = <0x0 0x40>; + interrupts = <0 9 4>; + kcs_chan = <1>; + status = "disabled"; + }; + + kcs2: kcs2@0 { + compatible = "nuvoton,npcm750-kcs-bmc"; + reg = <0x0 0x40>; + interrupts = <0 9 4>; + kcs_chan = <2>; + status = "disabled"; + }; + }; \ No newline at end of file diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig index 3bda116c8aa0..470f976cf850 100644 --- a/drivers/char/ipmi/Kconfig +++ b/drivers/char/ipmi/Kconfig @@ -111,6 +111,21 @@ config ASPEED_KCS_IPMI_BMC The driver implements the BMC side of the KCS contorller, it provides the access of KCS IO space for BMC side. +config NPCM7XX_KCS_IPMI_BMC + depends on ARCH_NPCM7XX || COMPILE_TEST + select IPMI_KCS_BMC + select REGMAP_MMIO + tristate "NPCM7xx KCS IPMI BMC driver" + help + Provides a driver for the KCS (Keyboard Controller Style) IPMI + interface found on Nuvoton NPCM7xx SOCs. + + The driver implements the BMC side of the KCS contorller, it + provides the access of KCS IO space for BMC side. + + This support is also available as a module. If so, the module + will be called kcs_bmc_npcm7xx. + config ASPEED_BT_IPMI_BMC depends on ARCH_ASPEED || COMPILE_TEST depends on REGMAP && REGMAP_MMIO && MFD_SYSCON diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile index 21e9e872d973..7a3baf301a8f 100644 --- a/drivers/char/ipmi/Makefile +++ b/drivers/char/ipmi/Makefile @@ -24,3 +24,4 @@ obj-$(CONFIG_IPMI_POWEROFF) += ipmi_poweroff.o 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 diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c new file mode 100644 index 000000000000..7bc898c5d372 --- /dev/null +++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c @@ -0,0 +1,204 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018, Nuvoton Corporation. + * Copyright (c) 2018, Intel Corporation. + */ + +#define pr_fmt(fmt) "nuvoton-kcs-bmc: " fmt + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "kcs_bmc.h" + +#define DEVICE_NAME "npcm-kcs-bmc" +#define KCS_CHANNEL_MAX 3 + +#define KCS1ST 0x0C +#define KCS2ST 0x1E +#define KCS3ST 0x30 + +#define KCS1DO 0x0E +#define KCS2DO 0x20 +#define KCS3DO 0x32 + +#define KCS1DI 0x10 +#define KCS2DI 0x22 +#define KCS3DI 0x34 + +#define KCS1CTL 0x18 +#define KCS2CTL 0x2A +#define KCS3CTL 0x3C +#define KCS_CTL_IBFIE BIT(0) + +/* + * 7.2.4 Core KCS Registers + * Registers in this module are 8 bits. An 8-bit register must be accessed + * by an 8-bit read or write. + * + * sts: KCS Channel n Status Register (KCSnST). + * dob: KCS Channel n Data Out Buffer Register (KCSnDO). + * dib: KCS Channel n Data In Buffer Register (KCSnDI). + * ctl: KCS Channel n Control Register (KCSnCTL). + */ +struct npcm7xx_kcs_reg { + u32 sts; + u32 dob; + u32 dib; + u32 ctl; +}; + +struct npcm7xx_kcs_bmc { + struct regmap *map; + + const struct npcm7xx_kcs_reg *reg; +}; + +static const struct npcm7xx_kcs_reg npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = { + { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL }, + { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL }, + { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL }, +}; + +static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg) +{ + struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); + u32 val = 0; + int rc; + + rc = regmap_read(priv->map, reg, &val); + WARN(rc != 0, "regmap_read() failed: %d\n", rc); + + return rc == 0 ? (u8)val : 0; +} + +static void npcm7xx_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data) +{ + struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); + int rc; + + rc = regmap_write(priv->map, reg, data); + WARN(rc != 0, "regmap_write() failed: %d\n", rc); +} + +static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable) +{ + struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); + + regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE, + enable ? KCS_CTL_IBFIE : 0); +} + +static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg) +{ + struct kcs_bmc *kcs_bmc = arg; + + if (!kcs_bmc_handle_event(kcs_bmc)) + return IRQ_HANDLED; + + return IRQ_NONE; +} + +static int npcm7xx_kcs_config_irq(struct kcs_bmc *kcs_bmc, + struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + int irq; + + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return irq; + + return devm_request_irq(dev, irq, npcm7xx_kcs_irq, IRQF_SHARED, + dev_name(dev), kcs_bmc); +} + +static int npcm7xx_kcs_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct npcm7xx_kcs_bmc *priv; + struct kcs_bmc *kcs_bmc; + u32 chan; + int rc; + + rc = of_property_read_u32(dev->of_node, "kcs_chan", &chan); + if (rc != 0 || chan == 0 || chan > KCS_CHANNEL_MAX) { + dev_err(dev, "no valid 'kcs_chan' configured\n"); + return -ENODEV; + } + + kcs_bmc = kcs_bmc_alloc(dev, sizeof(*priv), chan); + if (!kcs_bmc) + return -ENOMEM; + + priv = kcs_bmc_priv(kcs_bmc); + priv->map = syscon_node_to_regmap(dev->parent->of_node); + if (IS_ERR(priv->map)) { + dev_err(dev, "Couldn't get regmap\n"); + return -ENODEV; + } + priv->reg = &npcm7xx_kcs_reg_tbl[chan - 1]; + + kcs_bmc->ioreg.idr = priv->reg->dib; + kcs_bmc->ioreg.odr = priv->reg->dob; + kcs_bmc->ioreg.str = priv->reg->sts; + kcs_bmc->io_inputb = npcm7xx_kcs_inb; + kcs_bmc->io_outputb = npcm7xx_kcs_outb; + + dev_set_drvdata(dev, kcs_bmc); + + npcm7xx_kcs_enable_channel(kcs_bmc, true); + rc = npcm7xx_kcs_config_irq(kcs_bmc, pdev); + if (rc) + return rc; + + rc = misc_register(&kcs_bmc->miscdev); + if (rc) { + dev_err(dev, "Unable to register device\n"); + return rc; + } + + pr_info("channel=%u idr=0x%x odr=0x%x str=0x%x\n", + chan, + kcs_bmc->ioreg.idr, kcs_bmc->ioreg.odr, kcs_bmc->ioreg.str); + + return 0; +} + +static int npcm7xx_kcs_remove(struct platform_device *pdev) +{ + struct kcs_bmc *kcs_bmc = dev_get_drvdata(&pdev->dev); + + misc_deregister(&kcs_bmc->miscdev); + + return 0; +} + +static const struct of_device_id npcm_kcs_bmc_match[] = { + { .compatible = "nuvoton,npcm750-kcs-bmc" }, + { } +}; +MODULE_DEVICE_TABLE(of, npcm_kcs_bmc_match); + +static struct platform_driver npcm_kcs_bmc_driver = { + .driver = { + .name = DEVICE_NAME, + .of_match_table = npcm_kcs_bmc_match, + }, + .probe = npcm7xx_kcs_probe, + .remove = npcm7xx_kcs_remove, +}; +module_platform_driver(npcm_kcs_bmc_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Avi Fishman "); +MODULE_AUTHOR("Haiyue Wang "); +MODULE_DESCRIPTION("NPCM7xx device interface to the KCS BMC device"); From 1211229399b3f9b8b0e337c898efb09695f71c91 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 19 Apr 2018 08:24:29 -0500 Subject: [PATCH 31/35] ipmi_ssif: Fix uninitialized variable issue Currently, function ssif_remove returns _rv_, which is a variable that is never initialized. Fix this by removing variable _rv_ and return 0 instead. Addresses-Coverity-ID: 1467999 ("Uninitialized scalar variable") Fixes: 6a0d23ed338e ("ipmi: ipmi_unregister_smi() cannot fail, have it return void") Signed-off-by: Gustavo A. R. Silva Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_ssif.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index ecabfe27c828..37f9ae2de6a4 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1227,7 +1227,6 @@ static int ssif_remove(struct i2c_client *client) struct ssif_info *ssif_info = i2c_get_clientdata(client); struct ipmi_smi *intf; struct ssif_addr_info *addr_info; - int rv; if (!ssif_info) return 0; @@ -1247,7 +1246,7 @@ static int ssif_remove(struct i2c_client *client) } } - return rv; + return 0; } static int do_cmd(struct i2c_client *client, int len, unsigned char *msg, From 163475ebf9f3d1b516c1f8ee4f59eb8ff8e97ee8 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 18 Apr 2018 13:01:21 -0500 Subject: [PATCH 32/35] ipmi: Remove the proc interface It has been deprecated long enough, get rid of it. Signed-off-by: Corey Minyard --- drivers/char/ipmi/Kconfig | 8 - drivers/char/ipmi/ipmi_msghandler.c | 264 ---------------------------- drivers/char/ipmi/ipmi_si_intf.c | 125 ------------- drivers/char/ipmi/ipmi_ssif.c | 89 ---------- include/linux/ipmi_smi.h | 11 -- 5 files changed, 497 deletions(-) diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig index 470f976cf850..c108441882cc 100644 --- a/drivers/char/ipmi/Kconfig +++ b/drivers/char/ipmi/Kconfig @@ -22,14 +22,6 @@ config IPMI_DMI_DECODE if IPMI_HANDLER -config IPMI_PROC_INTERFACE - bool 'Provide an interface for IPMI stats in /proc (deprecated)' - depends on PROC_FS - default y - help - Do not use this any more, use sysfs for this info. It will be - removed in future kernel versions. - config IPMI_PANIC_EVENT bool 'Generate a panic event to all BMCs on a panic' help diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 946bfcb1eeee..606d561fe0e2 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -131,10 +131,6 @@ module_param_cb(panic_op, &panic_op_ops, NULL, 0600); MODULE_PARM_DESC(panic_op, "Sets if the IPMI driver will attempt to store panic information in the event log in the event of a panic. Set to 'none' for no, 'event' for a single event, or 'string' for a generic event and the panic string in IPMI OEM events."); -#ifdef CONFIG_IPMI_PROC_INTERFACE -static struct proc_dir_entry *proc_ipmi_root; -#endif /* CONFIG_IPMI_PROC_INTERFACE */ - #define MAX_EVENTS_IN_QUEUE 25 /* Remain in auto-maintenance mode for this amount of time (in ms). */ @@ -315,13 +311,6 @@ struct ipmi_my_addrinfo { unsigned char lun; }; -#ifdef CONFIG_IPMI_PROC_INTERFACE -struct ipmi_proc_entry { - char *name; - struct ipmi_proc_entry *next; -}; -#endif - /* * Note that the product id, manufacturer id, guid, and device id are * immutable in this structure, so dyn_mutex is not required for @@ -492,15 +481,6 @@ struct ipmi_smi { const struct ipmi_smi_handlers *handlers; void *send_info; -#ifdef CONFIG_IPMI_PROC_INTERFACE - /* A list of proc entries for this interface. */ - struct mutex proc_entry_lock; - struct ipmi_proc_entry *proc_entries; - - struct proc_dir_entry *proc_dir; - char proc_dir_name[10]; -#endif - /* Driver-model device for the system interface. */ struct device *si_dev; @@ -2515,216 +2495,6 @@ static int bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc, return __bmc_get_device_id(intf, bmc, id, guid_set, guid, -1); } -#ifdef CONFIG_IPMI_PROC_INTERFACE -static int smi_ipmb_proc_show(struct seq_file *m, void *v) -{ - struct ipmi_smi *intf = m->private; - int i; - - seq_printf(m, "%x", intf->addrinfo[0].address); - for (i = 1; i < IPMI_MAX_CHANNELS; i++) - seq_printf(m, " %x", intf->addrinfo[i].address); - seq_putc(m, '\n'); - - return 0; -} - -static int smi_ipmb_proc_open(struct inode *inode, struct file *file) -{ - return single_open(file, smi_ipmb_proc_show, PDE_DATA(inode)); -} - -static const struct file_operations smi_ipmb_proc_ops = { - .open = smi_ipmb_proc_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - -static int smi_version_proc_show(struct seq_file *m, void *v) -{ - struct ipmi_smi *intf = m->private; - struct ipmi_device_id id; - int rv; - - rv = bmc_get_device_id(intf, NULL, &id, NULL, NULL); - if (rv) - return rv; - - seq_printf(m, "%u.%u\n", - ipmi_version_major(&id), - ipmi_version_minor(&id)); - - return 0; -} - -static int smi_version_proc_open(struct inode *inode, struct file *file) -{ - return single_open(file, smi_version_proc_show, PDE_DATA(inode)); -} - -static const struct file_operations smi_version_proc_ops = { - .open = smi_version_proc_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - -static int smi_stats_proc_show(struct seq_file *m, void *v) -{ - struct ipmi_smi *intf = m->private; - - seq_printf(m, "sent_invalid_commands: %u\n", - ipmi_get_stat(intf, sent_invalid_commands)); - seq_printf(m, "sent_local_commands: %u\n", - ipmi_get_stat(intf, sent_local_commands)); - seq_printf(m, "handled_local_responses: %u\n", - ipmi_get_stat(intf, handled_local_responses)); - seq_printf(m, "unhandled_local_responses: %u\n", - ipmi_get_stat(intf, unhandled_local_responses)); - seq_printf(m, "sent_ipmb_commands: %u\n", - ipmi_get_stat(intf, sent_ipmb_commands)); - seq_printf(m, "sent_ipmb_command_errs: %u\n", - ipmi_get_stat(intf, sent_ipmb_command_errs)); - seq_printf(m, "retransmitted_ipmb_commands: %u\n", - ipmi_get_stat(intf, retransmitted_ipmb_commands)); - seq_printf(m, "timed_out_ipmb_commands: %u\n", - ipmi_get_stat(intf, timed_out_ipmb_commands)); - seq_printf(m, "timed_out_ipmb_broadcasts: %u\n", - ipmi_get_stat(intf, timed_out_ipmb_broadcasts)); - seq_printf(m, "sent_ipmb_responses: %u\n", - ipmi_get_stat(intf, sent_ipmb_responses)); - seq_printf(m, "handled_ipmb_responses: %u\n", - ipmi_get_stat(intf, handled_ipmb_responses)); - seq_printf(m, "invalid_ipmb_responses: %u\n", - ipmi_get_stat(intf, invalid_ipmb_responses)); - seq_printf(m, "unhandled_ipmb_responses: %u\n", - ipmi_get_stat(intf, unhandled_ipmb_responses)); - seq_printf(m, "sent_lan_commands: %u\n", - ipmi_get_stat(intf, sent_lan_commands)); - seq_printf(m, "sent_lan_command_errs: %u\n", - ipmi_get_stat(intf, sent_lan_command_errs)); - seq_printf(m, "retransmitted_lan_commands: %u\n", - ipmi_get_stat(intf, retransmitted_lan_commands)); - seq_printf(m, "timed_out_lan_commands: %u\n", - ipmi_get_stat(intf, timed_out_lan_commands)); - seq_printf(m, "sent_lan_responses: %u\n", - ipmi_get_stat(intf, sent_lan_responses)); - seq_printf(m, "handled_lan_responses: %u\n", - ipmi_get_stat(intf, handled_lan_responses)); - seq_printf(m, "invalid_lan_responses: %u\n", - ipmi_get_stat(intf, invalid_lan_responses)); - seq_printf(m, "unhandled_lan_responses: %u\n", - ipmi_get_stat(intf, unhandled_lan_responses)); - seq_printf(m, "handled_commands: %u\n", - ipmi_get_stat(intf, handled_commands)); - seq_printf(m, "invalid_commands: %u\n", - ipmi_get_stat(intf, invalid_commands)); - seq_printf(m, "unhandled_commands: %u\n", - ipmi_get_stat(intf, unhandled_commands)); - seq_printf(m, "invalid_events: %u\n", - ipmi_get_stat(intf, invalid_events)); - seq_printf(m, "events: %u\n", - ipmi_get_stat(intf, events)); - seq_printf(m, "failed rexmit LAN msgs: %u\n", - ipmi_get_stat(intf, dropped_rexmit_lan_commands)); - seq_printf(m, "failed rexmit IPMB msgs: %u\n", - ipmi_get_stat(intf, dropped_rexmit_ipmb_commands)); - return 0; -} - -static int smi_stats_proc_open(struct inode *inode, struct file *file) -{ - return single_open(file, smi_stats_proc_show, PDE_DATA(inode)); -} - -static const struct file_operations smi_stats_proc_ops = { - .open = smi_stats_proc_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - -int ipmi_smi_add_proc_entry(struct ipmi_smi *smi, char *name, - const struct file_operations *proc_ops, - void *data) -{ - int rv = 0; - struct proc_dir_entry *file; - struct ipmi_proc_entry *entry; - - /* Create a list element. */ - entry = kmalloc(sizeof(*entry), GFP_KERNEL); - if (!entry) - return -ENOMEM; - entry->name = kstrdup(name, GFP_KERNEL); - if (!entry->name) { - kfree(entry); - return -ENOMEM; - } - - file = proc_create_data(name, 0, smi->proc_dir, proc_ops, data); - if (!file) { - kfree(entry->name); - kfree(entry); - rv = -ENOMEM; - } else { - mutex_lock(&smi->proc_entry_lock); - /* Stick it on the list. */ - entry->next = smi->proc_entries; - smi->proc_entries = entry; - mutex_unlock(&smi->proc_entry_lock); - } - - return rv; -} -EXPORT_SYMBOL(ipmi_smi_add_proc_entry); - -static int add_proc_entries(struct ipmi_smi *smi, int num) -{ - int rv = 0; - - sprintf(smi->proc_dir_name, "%d", num); - smi->proc_dir = proc_mkdir(smi->proc_dir_name, proc_ipmi_root); - if (!smi->proc_dir) - rv = -ENOMEM; - - if (rv == 0) - rv = ipmi_smi_add_proc_entry(smi, "stats", - &smi_stats_proc_ops, - smi); - - if (rv == 0) - rv = ipmi_smi_add_proc_entry(smi, "ipmb", - &smi_ipmb_proc_ops, - smi); - - if (rv == 0) - rv = ipmi_smi_add_proc_entry(smi, "version", - &smi_version_proc_ops, - smi); - - return rv; -} - -static void remove_proc_entries(struct ipmi_smi *smi) -{ - struct ipmi_proc_entry *entry; - - mutex_lock(&smi->proc_entry_lock); - while (smi->proc_entries) { - entry = smi->proc_entries; - smi->proc_entries = entry->next; - - remove_proc_entry(entry->name, smi->proc_dir); - kfree(entry->name); - kfree(entry); - } - mutex_unlock(&smi->proc_entry_lock); - remove_proc_entry(smi->proc_dir_name, proc_ipmi_root); -} -#endif /* CONFIG_IPMI_PROC_INTERFACE */ - static ssize_t device_id_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -3564,9 +3334,6 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, intf->seq_table[j].seqid = 0; } intf->curr_seq = 0; -#ifdef CONFIG_IPMI_PROC_INTERFACE - mutex_init(&intf->proc_entry_lock); -#endif spin_lock_init(&intf->waiting_rcv_msgs_lock); INIT_LIST_HEAD(&intf->waiting_rcv_msgs); tasklet_init(&intf->recv_tasklet, @@ -3588,10 +3355,6 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, for (i = 0; i < IPMI_NUM_STATS; i++) atomic_set(&intf->stats[i], 0); -#ifdef CONFIG_IPMI_PROC_INTERFACE - intf->proc_dir = NULL; -#endif - mutex_lock(&ipmi_interfaces_mutex); /* Look for a hole in the numbers. */ i = 0; @@ -3622,20 +3385,10 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, mutex_lock(&intf->bmc_reg_mutex); rv = __scan_channels(intf, &id); mutex_unlock(&intf->bmc_reg_mutex); - if (rv) - goto out; - -#ifdef CONFIG_IPMI_PROC_INTERFACE - rv = add_proc_entries(intf, i); -#endif out: if (rv) { ipmi_bmc_unregister(intf); -#ifdef CONFIG_IPMI_PROC_INTERFACE - if (intf->proc_dir) - remove_proc_entries(intf); -#endif list_del_rcu(&intf->link); mutex_unlock(&ipmi_interfaces_mutex); synchronize_srcu(&ipmi_interfaces_srcu); @@ -3748,9 +3501,6 @@ void ipmi_unregister_smi(struct ipmi_smi *intf) cleanup_smi_msgs(intf); -#ifdef CONFIG_IPMI_PROC_INTERFACE - remove_proc_entries(intf); -#endif ipmi_bmc_unregister(intf); cleanup_srcu_struct(&intf->users_srcu); @@ -5277,16 +5027,6 @@ static int ipmi_init_msghandler(void) pr_info("ipmi message handler version " IPMI_DRIVER_VERSION "\n"); -#ifdef CONFIG_IPMI_PROC_INTERFACE - proc_ipmi_root = proc_mkdir("ipmi", NULL); - if (!proc_ipmi_root) { - pr_err(PFX "Unable to create IPMI proc dir"); - driver_unregister(&ipmidriver.driver); - return -ENOMEM; - } - -#endif /* CONFIG_IPMI_PROC_INTERFACE */ - timer_setup(&ipmi_timer, ipmi_timeout, 0); mod_timer(&ipmi_timer, jiffies + IPMI_TIMEOUT_JIFFIES); @@ -5325,10 +5065,6 @@ static void __exit cleanup_ipmi(void) atomic_inc(&stop_operation); del_timer_sync(&ipmi_timer); -#ifdef CONFIG_IPMI_PROC_INTERFACE - proc_remove(proc_ipmi_root); -#endif /* CONFIG_IPMI_PROC_INTERFACE */ - driver_unregister(&ipmidriver.driver); initialized = 0; diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index f2c39bf91bf5..ad353be871bf 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1588,102 +1588,6 @@ static int try_enable_event_buffer(struct smi_info *smi_info) return rv; } -#ifdef CONFIG_IPMI_PROC_INTERFACE -static int smi_type_proc_show(struct seq_file *m, void *v) -{ - struct smi_info *smi = m->private; - - seq_printf(m, "%s\n", si_to_str[smi->io.si_type]); - - return 0; -} - -static int smi_type_proc_open(struct inode *inode, struct file *file) -{ - return single_open(file, smi_type_proc_show, PDE_DATA(inode)); -} - -static const struct file_operations smi_type_proc_ops = { - .open = smi_type_proc_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - -static int smi_si_stats_proc_show(struct seq_file *m, void *v) -{ - struct smi_info *smi = m->private; - - seq_printf(m, "interrupts_enabled: %d\n", - smi->io.irq && !smi->interrupt_disabled); - seq_printf(m, "short_timeouts: %u\n", - smi_get_stat(smi, short_timeouts)); - seq_printf(m, "long_timeouts: %u\n", - smi_get_stat(smi, long_timeouts)); - seq_printf(m, "idles: %u\n", - smi_get_stat(smi, idles)); - seq_printf(m, "interrupts: %u\n", - smi_get_stat(smi, interrupts)); - seq_printf(m, "attentions: %u\n", - smi_get_stat(smi, attentions)); - seq_printf(m, "flag_fetches: %u\n", - smi_get_stat(smi, flag_fetches)); - seq_printf(m, "hosed_count: %u\n", - smi_get_stat(smi, hosed_count)); - seq_printf(m, "complete_transactions: %u\n", - smi_get_stat(smi, complete_transactions)); - seq_printf(m, "events: %u\n", - smi_get_stat(smi, events)); - seq_printf(m, "watchdog_pretimeouts: %u\n", - smi_get_stat(smi, watchdog_pretimeouts)); - seq_printf(m, "incoming_messages: %u\n", - smi_get_stat(smi, incoming_messages)); - return 0; -} - -static int smi_si_stats_proc_open(struct inode *inode, struct file *file) -{ - return single_open(file, smi_si_stats_proc_show, PDE_DATA(inode)); -} - -static const struct file_operations smi_si_stats_proc_ops = { - .open = smi_si_stats_proc_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - -static int smi_params_proc_show(struct seq_file *m, void *v) -{ - struct smi_info *smi = m->private; - - seq_printf(m, - "%s,%s,0x%lx,rsp=%d,rsi=%d,rsh=%d,irq=%d,ipmb=%d\n", - si_to_str[smi->io.si_type], - addr_space_to_str[smi->io.addr_type], - smi->io.addr_data, - smi->io.regspacing, - smi->io.regsize, - smi->io.regshift, - smi->io.irq, - smi->io.slave_addr); - - return 0; -} - -static int smi_params_proc_open(struct inode *inode, struct file *file) -{ - return single_open(file, smi_params_proc_show, PDE_DATA(inode)); -} - -static const struct file_operations smi_params_proc_ops = { - .open = smi_params_proc_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; -#endif - #define IPMI_SI_ATTR(name) \ static ssize_t ipmi_##name##_show(struct device *dev, \ struct device_attribute *attr, \ @@ -2172,35 +2076,6 @@ static int try_smi_init(struct smi_info *new_smi) goto out_err; } -#ifdef CONFIG_IPMI_PROC_INTERFACE - rv = ipmi_smi_add_proc_entry(new_smi->intf, "type", - &smi_type_proc_ops, - new_smi); - if (rv) { - dev_err(new_smi->io.dev, - "Unable to create proc entry: %d\n", rv); - goto out_err; - } - - rv = ipmi_smi_add_proc_entry(new_smi->intf, "si_stats", - &smi_si_stats_proc_ops, - new_smi); - if (rv) { - dev_err(new_smi->io.dev, - "Unable to create proc entry: %d\n", rv); - goto out_err; - } - - rv = ipmi_smi_add_proc_entry(new_smi->intf, "params", - &smi_params_proc_ops, - new_smi); - if (rv) { - dev_err(new_smi->io.dev, - "Unable to create proc entry: %d\n", rv); - goto out_err; - } -#endif - /* Don't increment till we know we have succeeded. */ smi_num++; diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 37f9ae2de6a4..22f634eb09fd 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1316,72 +1316,6 @@ static int ssif_detect(struct i2c_client *client, struct i2c_board_info *info) return rv; } -#ifdef CONFIG_IPMI_PROC_INTERFACE -static int smi_type_proc_show(struct seq_file *m, void *v) -{ - seq_puts(m, "ssif\n"); - - return 0; -} - -static int smi_type_proc_open(struct inode *inode, struct file *file) -{ - return single_open(file, smi_type_proc_show, inode->i_private); -} - -static const struct file_operations smi_type_proc_ops = { - .open = smi_type_proc_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - -static int smi_stats_proc_show(struct seq_file *m, void *v) -{ - struct ssif_info *ssif_info = m->private; - - seq_printf(m, "sent_messages: %u\n", - ssif_get_stat(ssif_info, sent_messages)); - seq_printf(m, "sent_messages_parts: %u\n", - ssif_get_stat(ssif_info, sent_messages_parts)); - seq_printf(m, "send_retries: %u\n", - ssif_get_stat(ssif_info, send_retries)); - seq_printf(m, "send_errors: %u\n", - ssif_get_stat(ssif_info, send_errors)); - seq_printf(m, "received_messages: %u\n", - ssif_get_stat(ssif_info, received_messages)); - seq_printf(m, "received_message_parts: %u\n", - ssif_get_stat(ssif_info, received_message_parts)); - seq_printf(m, "receive_retries: %u\n", - ssif_get_stat(ssif_info, receive_retries)); - seq_printf(m, "receive_errors: %u\n", - ssif_get_stat(ssif_info, receive_errors)); - seq_printf(m, "flag_fetches: %u\n", - ssif_get_stat(ssif_info, flag_fetches)); - seq_printf(m, "hosed: %u\n", - ssif_get_stat(ssif_info, hosed)); - seq_printf(m, "events: %u\n", - ssif_get_stat(ssif_info, events)); - seq_printf(m, "watchdog_pretimeouts: %u\n", - ssif_get_stat(ssif_info, watchdog_pretimeouts)); - seq_printf(m, "alerts: %u\n", - ssif_get_stat(ssif_info, alerts)); - return 0; -} - -static int smi_stats_proc_open(struct inode *inode, struct file *file) -{ - return single_open(file, smi_stats_proc_show, PDE_DATA(inode)); -} - -static const struct file_operations smi_stats_proc_ops = { - .open = smi_stats_proc_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; -#endif - static int strcmp_nospace(char *s1, char *s2) { while (*s1 && *s2) { @@ -1712,24 +1646,6 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) goto out_remove_attr; } -#ifdef CONFIG_IPMI_PROC_INTERFACE - rv = ipmi_smi_add_proc_entry(ssif_info->intf, "type", - &smi_type_proc_ops, - ssif_info); - if (rv) { - pr_err(PFX "Unable to create proc entry: %d\n", rv); - goto out_err_unreg; - } - - rv = ipmi_smi_add_proc_entry(ssif_info->intf, "ssif_stats", - &smi_stats_proc_ops, - ssif_info); - if (rv) { - pr_err(PFX "Unable to create proc entry: %d\n", rv); - goto out_err_unreg; - } -#endif - out: if (rv) { /* @@ -1747,11 +1663,6 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) kfree(resp); return rv; -#ifdef CONFIG_IPMI_PROC_INTERFACE -out_err_unreg: - ipmi_unregister_smi(ssif_info->intf); -#endif - out_remove_attr: device_remove_group(&ssif_info->client->dev, &ipmi_ssif_dev_attr_group); dev_set_drvdata(&ssif_info->client->dev, NULL); diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h index 0d438662a821..7d5fd38d5282 100644 --- a/include/linux/ipmi_smi.h +++ b/include/linux/ipmi_smi.h @@ -240,15 +240,4 @@ static inline void ipmi_free_smi_msg(struct ipmi_smi_msg *msg) msg->done(msg); } -#ifdef CONFIG_IPMI_PROC_INTERFACE -/* - * Allow the lower layer to add things to the proc filesystem - * directory for this interface. Note that the entry will - * automatically be dstroyed when the interface is destroyed. - */ -int ipmi_smi_add_proc_entry(struct ipmi_smi *smi, char *name, - const struct file_operations *proc_ops, - void *data); -#endif - #endif /* __LINUX_IPMI_SMI_H */ From fe50a7d0393a552e4539da2d31261a59d6415950 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 22 May 2018 08:14:51 -0500 Subject: [PATCH 33/35] ipmi:bt: Set the timeout before doing a capabilities check There was one place where the timeout value for an operation was not being set, if a capabilities request was done from idle. Move the timeout value setting to before where that change might be requested. IMHO the cause here is the invisible returns in the macros. Maybe that's a job for later, though. Reported-by: Nordmark Claes Signed-off-by: Corey Minyard Cc: stable@vger.kernel.org --- drivers/char/ipmi/ipmi_bt_sm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/char/ipmi/ipmi_bt_sm.c b/drivers/char/ipmi/ipmi_bt_sm.c index fd4ea8d87d4b..a3397664f800 100644 --- a/drivers/char/ipmi/ipmi_bt_sm.c +++ b/drivers/char/ipmi/ipmi_bt_sm.c @@ -504,11 +504,12 @@ static enum si_sm_result bt_event(struct si_sm_data *bt, long time) if (status & BT_H_BUSY) /* clear a leftover H_BUSY */ BT_CONTROL(BT_H_BUSY); + bt->timeout = bt->BT_CAP_req2rsp; + /* Read BT capabilities if it hasn't been done yet */ if (!bt->BT_CAP_outreqs) BT_STATE_CHANGE(BT_STATE_CAPABILITIES_BEGIN, SI_SM_CALL_WITHOUT_DELAY); - bt->timeout = bt->BT_CAP_req2rsp; BT_SI_SM_RETURN(SI_SM_IDLE); case BT_STATE_XACTION_START: From 58aae18eb95ad95486bc98717c213ff48d94b98c Mon Sep 17 00:00:00 2001 From: Avi Fishman Date: Mon, 21 May 2018 15:39:59 +0300 Subject: [PATCH 34/35] ipmi: NPCM7xx KCS BMC: enable interrupt to the host Original kcs_bmc_npcm7xx.c was missing enabling to send interrupt to the host on writes to output buffer. This patch fixes it by setting the bits that enables the generation of IRQn events by hardware control based on the status of the OBF flag. Signed-off-by: Avi Fishman Reviewed-by: Haiyue Wang Signed-off-by: Corey Minyard --- drivers/char/ipmi/kcs_bmc_npcm7xx.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c index 7bc898c5d372..722f7391fe1f 100644 --- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c +++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c @@ -39,6 +39,12 @@ #define KCS3CTL 0x3C #define KCS_CTL_IBFIE BIT(0) +#define KCS1IE 0x1C +#define KCS2IE 0x2E +#define KCS3IE 0x40 +#define KCS_IE_IRQE BIT(0) +#define KCS_IE_HIRQE BIT(3) + /* * 7.2.4 Core KCS Registers * Registers in this module are 8 bits. An 8-bit register must be accessed @@ -48,12 +54,14 @@ * dob: KCS Channel n Data Out Buffer Register (KCSnDO). * dib: KCS Channel n Data In Buffer Register (KCSnDI). * ctl: KCS Channel n Control Register (KCSnCTL). + * ie : KCS Channel n Interrupt Enable Register (KCSnIE). */ struct npcm7xx_kcs_reg { u32 sts; u32 dob; u32 dib; u32 ctl; + u32 ie; }; struct npcm7xx_kcs_bmc { @@ -63,9 +71,9 @@ struct npcm7xx_kcs_bmc { }; static const struct npcm7xx_kcs_reg npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = { - { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL }, - { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL }, - { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL }, + { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL, .ie = KCS1IE }, + { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL, .ie = KCS2IE }, + { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL, .ie = KCS3IE }, }; static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg) @@ -95,6 +103,9 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable) regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE, enable ? KCS_CTL_IBFIE : 0); + + regmap_update_bits(priv->map, priv->reg->ie, KCS_IE_IRQE | KCS_IE_HIRQE, + enable ? KCS_IE_IRQE | KCS_IE_HIRQE : 0); } static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg) From 048f7c3e352eeef50ed2c14dd89683f8a3af2f9b Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 24 May 2018 15:07:29 -0500 Subject: [PATCH 35/35] ipmi: Properly release srcu locks on error conditions When SRCU was added for handling hotplug, some error conditions were not handled properly. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 43 +++++++++++++++++------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 606d561fe0e2..51832b8a2c62 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -1291,18 +1291,19 @@ int ipmi_set_my_address(struct ipmi_user *user, unsigned int channel, unsigned char address) { - int index; + int index, rv = 0; user = acquire_ipmi_user(user, &index); if (!user) return -ENODEV; if (channel >= IPMI_MAX_CHANNELS) - return -EINVAL; - user->intf->addrinfo[channel].address = address; + rv = -EINVAL; + else + user->intf->addrinfo[channel].address = address; release_ipmi_user(user, index); - return 0; + return rv; } EXPORT_SYMBOL(ipmi_set_my_address); @@ -1310,18 +1311,19 @@ int ipmi_get_my_address(struct ipmi_user *user, unsigned int channel, unsigned char *address) { - int index; + int index, rv = 0; user = acquire_ipmi_user(user, &index); if (!user) return -ENODEV; if (channel >= IPMI_MAX_CHANNELS) - return -EINVAL; - *address = user->intf->addrinfo[channel].address; + rv = -EINVAL; + else + *address = user->intf->addrinfo[channel].address; release_ipmi_user(user, index); - return 0; + return rv; } EXPORT_SYMBOL(ipmi_get_my_address); @@ -1329,15 +1331,16 @@ int ipmi_set_my_LUN(struct ipmi_user *user, unsigned int channel, unsigned char LUN) { - int index; + int index, rv = 0; user = acquire_ipmi_user(user, &index); if (!user) return -ENODEV; if (channel >= IPMI_MAX_CHANNELS) - return -EINVAL; - user->intf->addrinfo[channel].lun = LUN & 0x3; + rv = -EINVAL; + else + user->intf->addrinfo[channel].lun = LUN & 0x3; release_ipmi_user(user, index); return 0; @@ -1348,18 +1351,19 @@ int ipmi_get_my_LUN(struct ipmi_user *user, unsigned int channel, unsigned char *address) { - int index; + int index, rv = 0; user = acquire_ipmi_user(user, &index); if (!user) return -ENODEV; if (channel >= IPMI_MAX_CHANNELS) - return -EINVAL; - *address = user->intf->addrinfo[channel].lun; + rv = -EINVAL; + else + *address = user->intf->addrinfo[channel].lun; release_ipmi_user(user, index); - return 0; + return rv; } EXPORT_SYMBOL(ipmi_get_my_LUN); @@ -1540,8 +1544,10 @@ int ipmi_register_for_cmd(struct ipmi_user *user, return -ENODEV; rcvr = kmalloc(sizeof(*rcvr), GFP_KERNEL); - if (!rcvr) - return -ENOMEM; + if (!rcvr) { + rv = -ENOMEM; + goto out_release; + } rcvr->cmd = cmd; rcvr->netfn = netfn; rcvr->chans = chans; @@ -1559,10 +1565,11 @@ int ipmi_register_for_cmd(struct ipmi_user *user, list_add_rcu(&rcvr->link, &intf->cmd_rcvrs); - out_unlock: +out_unlock: mutex_unlock(&intf->cmd_rcvrs_mutex); if (rv) kfree(rcvr); +out_release: release_ipmi_user(user, index); return rv;