From 401e7e88d4ef80188ffa07095ac00456f901b8c4 Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Mon, 28 Jan 2019 11:08:54 +0800 Subject: ipmi_si: fix use-after-free of resource->name When we excute the following commands, we got oops rmmod ipmi_si cat /proc/ioports [ 1623.482380] Unable to handle kernel paging request at virtual address ffff00000901d478 [ 1623.482382] Mem abort info: [ 1623.482383] ESR = 0x96000007 [ 1623.482385] Exception class = DABT (current EL), IL = 32 bits [ 1623.482386] SET = 0, FnV = 0 [ 1623.482387] EA = 0, S1PTW = 0 [ 1623.482388] Data abort info: [ 1623.482389] ISV = 0, ISS = 0x00000007 [ 1623.482390] CM = 0, WnR = 0 [ 1623.482393] swapper pgtable: 4k pages, 48-bit VAs, pgdp = 00000000d7d94a66 [ 1623.482395] [ffff00000901d478] pgd=000000dffbfff003, pud=000000dffbffe003, pmd=0000003f5d06e003, pte=0000000000000000 [ 1623.482399] Internal error: Oops: 96000007 [#1] SMP [ 1623.487407] Modules linked in: ipmi_si(E) nls_utf8 isofs rpcrdma ib_iser ib_srpt target_core_mod ib_srp scsi_transport_srp ib_ipoib rdma_ucm ib_umad rdma_cm ib_cm dm_mirror dm_region_hash dm_log iw_cm dm_mod aes_ce_blk crypto_simd cryptd aes_ce_cipher ses ghash_ce sha2_ce enclosure sha256_arm64 sg sha1_ce hisi_sas_v2_hw hibmc_drm sbsa_gwdt hisi_sas_main ip_tables mlx5_ib ib_uverbs marvell ib_core mlx5_core ixgbe mdio hns_dsaf ipmi_devintf hns_enet_drv ipmi_msghandler hns_mdio [last unloaded: ipmi_si] [ 1623.532410] CPU: 30 PID: 11438 Comm: cat Kdump: loaded Tainted: G E 5.0.0-rc3+ #168 [ 1623.541498] Hardware name: Huawei TaiShan 2280 /BC11SPCD, BIOS 1.37 11/21/2017 [ 1623.548822] pstate: a0000005 (NzCv daif -PAN -UAO) [ 1623.553684] pc : string+0x28/0x98 [ 1623.557040] lr : vsnprintf+0x368/0x5e8 [ 1623.560837] sp : ffff000013213a80 [ 1623.564191] x29: ffff000013213a80 x28: ffff00001138abb5 [ 1623.569577] x27: ffff000013213c18 x26: ffff805f67d06049 [ 1623.574963] x25: 0000000000000000 x24: ffff00001138abb5 [ 1623.580349] x23: 0000000000000fb7 x22: ffff0000117ed000 [ 1623.585734] x21: ffff000011188fd8 x20: ffff805f67d07000 [ 1623.591119] x19: ffff805f67d06061 x18: ffffffffffffffff [ 1623.596505] x17: 0000000000000200 x16: 0000000000000000 [ 1623.601890] x15: ffff0000117ed748 x14: ffff805f67d07000 [ 1623.607276] x13: ffff805f67d0605e x12: 0000000000000000 [ 1623.612661] x11: 0000000000000000 x10: 0000000000000000 [ 1623.618046] x9 : 0000000000000000 x8 : 000000000000000f [ 1623.623432] x7 : ffff805f67d06061 x6 : fffffffffffffffe [ 1623.628817] x5 : 0000000000000012 x4 : ffff00000901d478 [ 1623.634203] x3 : ffff0a00ffffff04 x2 : ffff805f67d07000 [ 1623.639588] x1 : ffff805f67d07000 x0 : ffffffffffffffff [ 1623.644974] Process cat (pid: 11438, stack limit = 0x000000008d4cbc10) [ 1623.651592] Call trace: [ 1623.654068] string+0x28/0x98 [ 1623.657071] vsnprintf+0x368/0x5e8 [ 1623.660517] seq_vprintf+0x70/0x98 [ 1623.668009] seq_printf+0x7c/0xa0 [ 1623.675530] r_show+0xc8/0xf8 [ 1623.682558] seq_read+0x330/0x440 [ 1623.689877] proc_reg_read+0x78/0xd0 [ 1623.697346] __vfs_read+0x60/0x1a0 [ 1623.704564] vfs_read+0x94/0x150 [ 1623.711339] ksys_read+0x6c/0xd8 [ 1623.717939] __arm64_sys_read+0x24/0x30 [ 1623.725077] el0_svc_common+0x120/0x148 [ 1623.732035] el0_svc_handler+0x30/0x40 [ 1623.738757] el0_svc+0x8/0xc [ 1623.744520] Code: d1000406 aa0103e2 54000149 b4000080 (39400085) [ 1623.753441] ---[ end trace f91b6a4937de9835 ]--- [ 1623.760871] Kernel panic - not syncing: Fatal exception [ 1623.768935] SMP: stopping secondary CPUs [ 1623.775718] Kernel Offset: disabled [ 1623.781998] CPU features: 0x002,21006008 [ 1623.788777] Memory Limit: none [ 1623.798329] Starting crashdump kernel... [ 1623.805202] Bye! If io_setup is called successful in try_smi_init() but try_smi_init() goes out_err before calling ipmi_register_smi(), so ipmi_unregister_smi() will not be called while removing module. It leads to the resource that allocated in io_setup() can not be freed, but the name(DEVICE_NAME) of resource is freed while removing the module. It causes use-after-free when cat /proc/ioports. Fix this by calling io_cleanup() while try_smi_init() goes to out_err. and don't call io_cleanup() until io_setup() returns successful to avoid warning prints. Fixes: 93c303d2045b ("ipmi_si: Clean up shutdown a bit") Cc: stable@vger.kernel.org Reported-by: NuoHan Qiao Suggested-by: Corey Minyard Signed-off-by: Yang Yingliang Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_intf.c | 5 +++++ drivers/char/ipmi/ipmi_si_mem_io.c | 5 +++-- drivers/char/ipmi/ipmi_si_port_io.c | 5 +++-- 3 files changed, 11 insertions(+), 4 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index dc8603d34320..f1b9fda6b9df 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -2085,6 +2085,11 @@ static int try_smi_init(struct smi_info *new_smi) WARN_ON(new_smi->io.dev->init_name != NULL); out_err: + if (rv && new_smi->io.io_cleanup) { + new_smi->io.io_cleanup(&new_smi->io); + new_smi->io.io_cleanup = NULL; + } + kfree(init_name); return rv; } diff --git a/drivers/char/ipmi/ipmi_si_mem_io.c b/drivers/char/ipmi/ipmi_si_mem_io.c index fd0ec8d6bf0e..75583612ab10 100644 --- a/drivers/char/ipmi/ipmi_si_mem_io.c +++ b/drivers/char/ipmi/ipmi_si_mem_io.c @@ -81,8 +81,6 @@ int ipmi_si_mem_setup(struct si_sm_io *io) if (!addr) return -ENODEV; - io->io_cleanup = mem_cleanup; - /* * Figure out the actual readb/readw/readl/etc routine to use based * upon the register size. @@ -141,5 +139,8 @@ int ipmi_si_mem_setup(struct si_sm_io *io) mem_region_cleanup(io, io->io_size); return -EIO; } + + io->io_cleanup = mem_cleanup; + return 0; } diff --git a/drivers/char/ipmi/ipmi_si_port_io.c b/drivers/char/ipmi/ipmi_si_port_io.c index ef6dffcea9fa..03924c32b6e9 100644 --- a/drivers/char/ipmi/ipmi_si_port_io.c +++ b/drivers/char/ipmi/ipmi_si_port_io.c @@ -68,8 +68,6 @@ int ipmi_si_port_setup(struct si_sm_io *io) if (!addr) return -ENODEV; - io->io_cleanup = port_cleanup; - /* * Figure out the actual inb/inw/inl/etc routine to use based * upon the register size. @@ -109,5 +107,8 @@ int ipmi_si_port_setup(struct si_sm_io *io) return -EIO; } } + + io->io_cleanup = port_cleanup; + return 0; } -- cgit v1.2.3 From a1466ec5b671651b848df17fc9233ecbb7d35f9f Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Mon, 22 Oct 2018 15:30:57 -0500 Subject: ipmi: Fix SSIF flag requests Commit 89986496de141 ("ipmi: Turn off all activity on an idle ipmi interface") modified the IPMI code to only request events when the driver had somethine waiting for events. The SSIF code, however, was using the event fetch request to also fetch the flags. Add a timer and the proper handling for the upper layer telling whether flags fetches are required. Reported-by: Kamlakant Patel Signed-off-by: Corey Minyard Tested-by: Kamlakant Patel --- drivers/char/ipmi/ipmi_ssif.c | 64 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 12 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index b7a1ae2afaea..1aacc1144d2a 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -90,6 +90,12 @@ #define SSIF_MSG_JIFFIES ((SSIF_MSG_USEC * 1000) / TICK_NSEC) #define SSIF_MSG_PART_JIFFIES ((SSIF_MSG_PART_USEC * 1000) / TICK_NSEC) +/* + * Timeout for the watch, only used for get flag timer. + */ +#define SSIF_WATCH_TIMEOUT_MSEC 100 +#define SSIF_WATCH_TIMEOUT_JIFFIES msecs_to_jiffies(SSIF_WATCH_TIMEOUT_MSEC) + enum ssif_intf_state { SSIF_NORMAL, SSIF_GETTING_FLAGS, @@ -270,6 +276,9 @@ struct ssif_info { struct timer_list retry_timer; int retries_left; + bool need_watch; /* Need to look for flags? */ + struct timer_list watch_timer; /* Flag fetch timer. */ + /* Info from SSIF cmd */ unsigned char max_xmit_msg_size; unsigned char max_recv_msg_size; @@ -560,6 +569,26 @@ static void retry_timeout(struct timer_list *t) start_get(ssif_info); } +static void watch_timeout(struct timer_list *t) +{ + struct ssif_info *ssif_info = from_timer(ssif_info, t, watch_timer); + unsigned long oflags, *flags; + + if (ssif_info->stopping) + return; + + flags = ipmi_ssif_lock_cond(ssif_info, &oflags); + if (ssif_info->need_watch) { + mod_timer(&ssif_info->watch_timer, + jiffies + SSIF_WATCH_TIMEOUT_JIFFIES); + if (SSIF_IDLE(ssif_info)) { + start_flag_fetch(ssif_info, flags); /* Releases lock */ + return; + } + ssif_info->req_flags = true; + } + ipmi_ssif_unlock_cond(ssif_info, flags); +} static void ssif_alert(struct i2c_client *client, enum i2c_alert_protocol type, unsigned int data) @@ -1073,8 +1102,7 @@ static int get_smi_info(void *send_info, struct ipmi_smi_info *data) } /* - * Instead of having our own timer to periodically check the message - * flags, we let the message handler drive us. + * Upper layer wants us to request events. */ static void request_events(void *send_info) { @@ -1085,18 +1113,27 @@ static void request_events(void *send_info) return; flags = ipmi_ssif_lock_cond(ssif_info, &oflags); - /* - * Request flags first, not events, because the lower layer - * doesn't have a way to send an attention. But make sure - * event checking still happens. - */ ssif_info->req_events = true; - if (SSIF_IDLE(ssif_info)) - start_flag_fetch(ssif_info, flags); - else { - ssif_info->req_flags = true; - ipmi_ssif_unlock_cond(ssif_info, flags); + ipmi_ssif_unlock_cond(ssif_info, flags); +} + +/* + * Upper layer is changing the flag saying whether we need to request + * flags periodically or not. + */ +static void ssif_set_need_watch(void *send_info, bool enable) +{ + struct ssif_info *ssif_info = (struct ssif_info *) send_info; + unsigned long oflags, *flags; + + flags = ipmi_ssif_lock_cond(ssif_info, &oflags); + if (enable != ssif_info->need_watch) { + ssif_info->need_watch = enable; + if (ssif_info->need_watch) + mod_timer(&ssif_info->watch_timer, + jiffies + SSIF_WATCH_TIMEOUT_JIFFIES); } + ipmi_ssif_unlock_cond(ssif_info, flags); } static int ssif_start_processing(void *send_info, @@ -1223,6 +1260,7 @@ static void shutdown_ssif(void *send_info) schedule_timeout(1); ssif_info->stopping = true; + del_timer_sync(&ssif_info->watch_timer); del_timer_sync(&ssif_info->retry_timer); if (ssif_info->thread) { complete(&ssif_info->wake_thread); @@ -1708,6 +1746,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) spin_lock_init(&ssif_info->lock); ssif_info->ssif_state = SSIF_NORMAL; timer_setup(&ssif_info->retry_timer, retry_timeout, 0); + timer_setup(&ssif_info->watch_timer, watch_timeout, 0); for (i = 0; i < SSIF_NUM_STATS; i++) atomic_set(&ssif_info->stats[i], 0); @@ -1721,6 +1760,7 @@ 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.set_need_watch = ssif_set_need_watch; { unsigned int thread_num; -- cgit v1.2.3 From c65ea996595005be470fbfa16711deba414fd33b Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 23 Oct 2018 11:29:02 -0500 Subject: ipmi: Fix how the lower layers are told to watch for messages The IPMI driver has a mechanism to tell the lower layers it needs to watch for messages, commands, and watchdogs (so it doesn't needlessly poll). However, it needed some extensions, it needed a way to tell what is being waited for so it could set the timeout appropriately. The update to the lower layer was also being done once a second at best because it was done in the main timeout handler. However, if a command is sent and a response message is coming back, it needed to be started immediately. So modify the code to update immediately if it needs to be enabled. Disable is still lazy. Signed-off-by: Corey Minyard Tested-by: Kamlakant Patel --- drivers/char/ipmi/ipmi_msghandler.c | 119 +++++++++++++++++++++++++----------- drivers/char/ipmi/ipmi_si_intf.c | 5 +- drivers/char/ipmi/ipmi_ssif.c | 26 +++++--- include/linux/ipmi_smi.h | 36 ++++++++--- 4 files changed, 134 insertions(+), 52 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index c518659b4d9f..2e008efa735f 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -529,9 +529,22 @@ struct ipmi_smi { unsigned int waiting_events_count; /* How many events in queue? */ char delivering_events; char event_msg_printed; + + /* How many users are waiting for events? */ atomic_t event_waiters; unsigned int ticks_to_req_ev; - int last_needs_timer; + + /* How many users are waiting for commands? */ + atomic_t command_waiters; + + /* How many users are waiting for watchdogs? */ + atomic_t watchdog_waiters; + + /* + * Tells what the lower layer has last been asked to watch for, + * messages and/or watchdogs. Protected by xmit_msgs_lock. + */ + unsigned int last_watch_mask; /* * The event receiver for my BMC, only really used at panic @@ -1078,6 +1091,29 @@ static int intf_err_seq(struct ipmi_smi *intf, return rv; } +/* Must be called with xmit_msgs_lock held. */ +static void smi_tell_to_watch(struct ipmi_smi *intf, + unsigned int flags, + struct ipmi_smi_msg *smi_msg) +{ + if (flags & IPMI_WATCH_MASK_CHECK_MESSAGES) { + if (!smi_msg) + return; + + if (!smi_msg->needs_response) + return; + } + + if (!intf->handlers->set_need_watch) + return; + + if ((intf->last_watch_mask & flags) == flags) + return; + + intf->last_watch_mask |= flags; + intf->handlers->set_need_watch(intf->send_info, + intf->last_watch_mask); +} int ipmi_create_user(unsigned int if_num, const struct ipmi_user_hndl *handler, @@ -1141,8 +1177,9 @@ int ipmi_create_user(unsigned int if_num, spin_unlock_irqrestore(&intf->seq_lock, flags); if (handler->ipmi_watchdog_pretimeout) { /* User wants pretimeouts, so make sure to watch for them. */ - if (atomic_inc_return(&intf->event_waiters) == 1) - need_waiter(intf); + if (atomic_inc_return(&intf->watchdog_waiters) == 1) + smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_WATCHDOG, + NULL); } srcu_read_unlock(&ipmi_interfaces_srcu, index); *user = new_user; @@ -1214,7 +1251,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user) user->handler->shutdown(user->handler_data); if (user->handler->ipmi_watchdog_pretimeout) - atomic_dec(&intf->event_waiters); + atomic_dec(&intf->watchdog_waiters); if (user->gets_events) atomic_dec(&intf->event_waiters); @@ -1569,8 +1606,8 @@ int ipmi_register_for_cmd(struct ipmi_user *user, goto out_unlock; } - if (atomic_inc_return(&intf->event_waiters) == 1) - need_waiter(intf); + if (atomic_inc_return(&intf->command_waiters) == 1) + smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_COMMANDS, NULL); list_add_rcu(&rcvr->link, &intf->cmd_rcvrs); @@ -1620,7 +1657,7 @@ int ipmi_unregister_for_cmd(struct ipmi_user *user, synchronize_rcu(); release_ipmi_user(user, index); while (rcvrs) { - atomic_dec(&intf->event_waiters); + atomic_dec(&intf->command_waiters); rcvr = rcvrs; rcvrs = rcvr->next; kfree(rcvr); @@ -1737,22 +1774,21 @@ static struct ipmi_smi_msg *smi_add_send_msg(struct ipmi_smi *intf, return smi_msg; } - 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; + unsigned long flags = 0; - if (run_to_completion) { - smi_msg = smi_add_send_msg(intf, smi_msg, priority); - } else { - unsigned long flags; - + if (!run_to_completion) spin_lock_irqsave(&intf->xmit_msgs_lock, flags); - smi_msg = smi_add_send_msg(intf, smi_msg, priority); + smi_msg = smi_add_send_msg(intf, smi_msg, priority); + + smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES, smi_msg); + + if (!run_to_completion) spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags); - } if (smi_msg) handlers->sender(intf->send_info, smi_msg); @@ -1950,6 +1986,9 @@ static int i_ipmi_req_ipmb(struct ipmi_smi *intf, ipmb_seq, broadcast, source_address, source_lun); + /* We will be getting a response in the BMC message queue. */ + smi_msg->needs_response = true; + /* * Copy the message into the recv message data, so we * can retransmit it later if necessary. @@ -2137,6 +2176,7 @@ static int i_ipmi_request(struct ipmi_user *user, goto out; } } + smi_msg->needs_response = false; rcu_read_lock(); if (intf->in_shutdown) { @@ -3351,6 +3391,8 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, INIT_LIST_HEAD(&intf->hp_xmit_msgs); spin_lock_init(&intf->events_lock); atomic_set(&intf->event_waiters, 0); + atomic_set(&intf->watchdog_waiters, 0); + atomic_set(&intf->command_waiters, 0); intf->ticks_to_req_ev = IPMI_REQUEST_EV_TIME; INIT_LIST_HEAD(&intf->waiting_events); intf->waiting_events_count = 0; @@ -4365,6 +4407,9 @@ static void smi_recv_tasklet(unsigned long val) intf->curr_msg = newmsg; } } + + smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES, newmsg); + if (!run_to_completion) spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags); if (newmsg) @@ -4492,7 +4537,7 @@ 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, - unsigned int *waiting_msgs) + unsigned int *watch_mask) { struct ipmi_recv_msg *msg; @@ -4504,7 +4549,7 @@ static void check_msg_timeout(struct ipmi_smi *intf, struct seq_table *ent, if (timeout_period < ent->timeout) { ent->timeout -= timeout_period; - (*waiting_msgs)++; + *watch_mask |= IPMI_WATCH_MASK_CHECK_MESSAGES; return; } @@ -4523,7 +4568,7 @@ static void check_msg_timeout(struct ipmi_smi *intf, struct seq_table *ent, struct ipmi_smi_msg *smi_msg; /* More retries, send again. */ - (*waiting_msgs)++; + *watch_mask |= IPMI_WATCH_MASK_CHECK_MESSAGES; /* * Start with the max timer, set to normal timer after @@ -4575,13 +4620,13 @@ static unsigned int ipmi_timeout_handler(struct ipmi_smi *intf, struct ipmi_recv_msg *msg, *msg2; unsigned long flags; int i; - unsigned int waiting_msgs = 0; + unsigned int watch_mask = 0; if (!intf->bmc_registered) { kref_get(&intf->refcount); if (!schedule_work(&intf->bmc_reg_work)) { kref_put(&intf->refcount, intf_free); - waiting_msgs++; + watch_mask |= IPMI_WATCH_MASK_INTERNAL; } } @@ -4601,7 +4646,7 @@ static unsigned int ipmi_timeout_handler(struct ipmi_smi *intf, for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) check_msg_timeout(intf, &intf->seq_table[i], &timeouts, timeout_period, i, - &flags, &waiting_msgs); + &flags, &watch_mask); spin_unlock_irqrestore(&intf->seq_lock, flags); list_for_each_entry_safe(msg, msg2, &timeouts, link) @@ -4632,7 +4677,7 @@ static unsigned int ipmi_timeout_handler(struct ipmi_smi *intf, tasklet_schedule(&intf->recv_tasklet); - return waiting_msgs; + return watch_mask; } static void ipmi_request_event(struct ipmi_smi *intf) @@ -4652,37 +4697,43 @@ static atomic_t stop_operation; static void ipmi_timeout(struct timer_list *unused) { struct ipmi_smi *intf; - int nt = 0, index; + unsigned int watch_mask = 0; + int index; + unsigned long flags; if (atomic_read(&stop_operation)) return; index = srcu_read_lock(&ipmi_interfaces_srcu); list_for_each_entry_rcu(intf, &ipmi_interfaces, link) { - int lnt = 0; - if (atomic_read(&intf->event_waiters)) { intf->ticks_to_req_ev--; if (intf->ticks_to_req_ev == 0) { ipmi_request_event(intf); intf->ticks_to_req_ev = IPMI_REQUEST_EV_TIME; } - lnt++; + watch_mask |= IPMI_WATCH_MASK_INTERNAL; } - lnt += ipmi_timeout_handler(intf, IPMI_TIMEOUT_TIME); + if (atomic_read(&intf->watchdog_waiters)) + watch_mask |= IPMI_WATCH_MASK_CHECK_WATCHDOG; - lnt = !!lnt; - if (lnt != intf->last_needs_timer && - intf->handlers->set_need_watch) - intf->handlers->set_need_watch(intf->send_info, lnt); - intf->last_needs_timer = lnt; + if (atomic_read(&intf->command_waiters)) + watch_mask |= IPMI_WATCH_MASK_CHECK_COMMANDS; + + watch_mask |= ipmi_timeout_handler(intf, IPMI_TIMEOUT_TIME); - nt += lnt; + spin_lock_irqsave(&intf->xmit_msgs_lock, flags); + if (watch_mask != intf->last_watch_mask && + intf->handlers->set_need_watch) + intf->handlers->set_need_watch(intf->send_info, + watch_mask); + intf->last_watch_mask = watch_mask; + spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags); } srcu_read_unlock(&ipmi_interfaces_srcu, index); - if (nt) + if (watch_mask) mod_timer(&ipmi_timer, jiffies + IPMI_TIMEOUT_JIFFIES); } diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index f1b9fda6b9df..c81c84a723b6 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1060,10 +1060,13 @@ static void request_events(void *send_info) atomic_set(&smi_info->req_events, 1); } -static void set_need_watch(void *send_info, bool enable) +static void set_need_watch(void *send_info, unsigned int watch_mask) { struct smi_info *smi_info = send_info; unsigned long flags; + int enable; + + enable = !!(watch_mask & ~IPMI_WATCH_MASK_INTERNAL); atomic_set(&smi_info->need_watch, enable); spin_lock_irqsave(&smi_info->si_lock, flags); diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 1aacc1144d2a..a1219af32105 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -93,8 +93,8 @@ /* * Timeout for the watch, only used for get flag timer. */ -#define SSIF_WATCH_TIMEOUT_MSEC 100 -#define SSIF_WATCH_TIMEOUT_JIFFIES msecs_to_jiffies(SSIF_WATCH_TIMEOUT_MSEC) +#define SSIF_WATCH_MSG_TIMEOUT msecs_to_jiffies(10) +#define SSIF_WATCH_WATCHDOG_TIMEOUT msecs_to_jiffies(250) enum ssif_intf_state { SSIF_NORMAL, @@ -276,7 +276,7 @@ struct ssif_info { struct timer_list retry_timer; int retries_left; - bool need_watch; /* Need to look for flags? */ + long watch_timeout; /* Timeout for flags check, 0 if off. */ struct timer_list watch_timer; /* Flag fetch timer. */ /* Info from SSIF cmd */ @@ -578,9 +578,9 @@ static void watch_timeout(struct timer_list *t) return; flags = ipmi_ssif_lock_cond(ssif_info, &oflags); - if (ssif_info->need_watch) { + if (ssif_info->watch_timeout) { mod_timer(&ssif_info->watch_timer, - jiffies + SSIF_WATCH_TIMEOUT_JIFFIES); + jiffies + ssif_info->watch_timeout); if (SSIF_IDLE(ssif_info)) { start_flag_fetch(ssif_info, flags); /* Releases lock */ return; @@ -1121,17 +1121,23 @@ static void request_events(void *send_info) * Upper layer is changing the flag saying whether we need to request * flags periodically or not. */ -static void ssif_set_need_watch(void *send_info, bool enable) +static void ssif_set_need_watch(void *send_info, unsigned int watch_mask) { struct ssif_info *ssif_info = (struct ssif_info *) send_info; unsigned long oflags, *flags; + long timeout = 0; + + if (watch_mask & IPMI_WATCH_MASK_CHECK_MESSAGES) + timeout = SSIF_WATCH_MSG_TIMEOUT; + else if (watch_mask & ~IPMI_WATCH_MASK_INTERNAL) + timeout = SSIF_WATCH_WATCHDOG_TIMEOUT; flags = ipmi_ssif_lock_cond(ssif_info, &oflags); - if (enable != ssif_info->need_watch) { - ssif_info->need_watch = enable; - if (ssif_info->need_watch) + if (timeout != ssif_info->watch_timeout) { + ssif_info->watch_timeout = timeout; + if (ssif_info->watch_timeout) mod_timer(&ssif_info->watch_timer, - jiffies + SSIF_WATCH_TIMEOUT_JIFFIES); + jiffies + ssif_info->watch_timeout); } ipmi_ssif_unlock_cond(ssif_info, flags); } diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h index 8c4e2ab696c3..da6abb06a5dc 100644 --- a/include/linux/ipmi_smi.h +++ b/include/linux/ipmi_smi.h @@ -30,6 +30,17 @@ struct device; /* Structure for the low-level drivers. */ struct ipmi_smi; +/* + * Flags for set_check_watch() below. Tells if the SMI should be + * waiting for watchdog timeouts, commands and/or messages. There is + * also an internal flag for the message handler, SMIs should ignore + * it. + */ +#define IPMI_WATCH_MASK_INTERNAL (1 << 0) +#define IPMI_WATCH_MASK_CHECK_MESSAGES (1 << 1) +#define IPMI_WATCH_MASK_CHECK_WATCHDOG (1 << 2) +#define IPMI_WATCH_MASK_CHECK_COMMANDS (1 << 3) + /* * Messages to/from the lower layer. The smi interface will take one * of these to send. After the send has occurred and a response has @@ -55,8 +66,16 @@ struct ipmi_smi_msg { int rsp_size; unsigned char rsp[IPMI_MAX_MSG_LENGTH]; - /* Will be called when the system is done with the message - (presumably to free it). */ + /* + * There should be a response message coming back in the BMC + * message queue. + */ + bool needs_response; + + /* + * Will be called when the system is done with the message + * (presumably to free it). + */ void (*done)(struct ipmi_smi_msg *msg); }; @@ -105,12 +124,15 @@ struct ipmi_smi_handlers { /* * 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. + * interface watch for received messages and watchdog + * pretimeouts (basically do a "Get Flags", 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. watch_mask is from + * IPMI_WATCH_MASK_xxx above. The interface should run slower + * timeouts for just watchdog checking or faster timeouts when + * waiting for the message queue. */ - void (*set_need_watch)(void *send_info, bool enable); + void (*set_need_watch)(void *send_info, unsigned int watch_mask); /* * Called when flushing all pending messages. -- cgit v1.2.3 From e1891cffd4c4896a899337a243273f0e23c028df Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 24 Oct 2018 15:17:04 -0500 Subject: ipmi: Make the smi watcher be disabled immediately when not needed The code to tell the lower layer to enable or disable watching for certain things was lazy in disabling, it waited until a timer tick to see if a disable was necessary. Not a really big deal, but it could be improved. Modify the code to enable and disable watching immediately and don't do it from the background timer any more. Signed-off-by: Corey Minyard Tested-by: Kamlakant Patel --- drivers/char/ipmi/ipmi_msghandler.c | 164 ++++++++++++++++++++---------------- drivers/char/ipmi/ipmi_si_intf.c | 2 +- drivers/char/ipmi/ipmi_ssif.c | 2 +- include/linux/ipmi_smi.h | 17 +--- 4 files changed, 96 insertions(+), 89 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 2e008efa735f..5bc84fbbeee5 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -534,15 +534,20 @@ struct ipmi_smi { atomic_t event_waiters; unsigned int ticks_to_req_ev; + spinlock_t watch_lock; /* For dealing with watch stuff below. */ + /* How many users are waiting for commands? */ - atomic_t command_waiters; + unsigned int command_waiters; /* How many users are waiting for watchdogs? */ - atomic_t watchdog_waiters; + unsigned int watchdog_waiters; + + /* How many users are waiting for message responses? */ + unsigned int response_waiters; /* * Tells what the lower layer has last been asked to watch for, - * messages and/or watchdogs. Protected by xmit_msgs_lock. + * messages and/or watchdogs. Protected by watch_lock. */ unsigned int last_watch_mask; @@ -938,6 +943,64 @@ static void deliver_err_response(struct ipmi_smi *intf, deliver_local_response(intf, msg); } +static void smi_add_watch(struct ipmi_smi *intf, unsigned int flags) +{ + unsigned long iflags; + + if (!intf->handlers->set_need_watch) + return; + + spin_lock_irqsave(&intf->watch_lock, iflags); + if (flags & IPMI_WATCH_MASK_CHECK_MESSAGES) + intf->response_waiters++; + + if (flags & IPMI_WATCH_MASK_CHECK_WATCHDOG) + intf->watchdog_waiters++; + + if (flags & IPMI_WATCH_MASK_CHECK_COMMANDS) + intf->command_waiters++; + + if ((intf->last_watch_mask & flags) != flags) { + intf->last_watch_mask |= flags; + intf->handlers->set_need_watch(intf->send_info, + intf->last_watch_mask); + } + spin_unlock_irqrestore(&intf->watch_lock, iflags); +} + +static void smi_remove_watch(struct ipmi_smi *intf, unsigned int flags) +{ + unsigned long iflags; + + if (!intf->handlers->set_need_watch) + return; + + spin_lock_irqsave(&intf->watch_lock, iflags); + if (flags & IPMI_WATCH_MASK_CHECK_MESSAGES) + intf->response_waiters--; + + if (flags & IPMI_WATCH_MASK_CHECK_WATCHDOG) + intf->watchdog_waiters--; + + if (flags & IPMI_WATCH_MASK_CHECK_COMMANDS) + intf->command_waiters--; + + flags = 0; + if (intf->response_waiters) + flags |= IPMI_WATCH_MASK_CHECK_MESSAGES; + if (intf->watchdog_waiters) + flags |= IPMI_WATCH_MASK_CHECK_WATCHDOG; + if (intf->command_waiters) + flags |= IPMI_WATCH_MASK_CHECK_COMMANDS; + + if (intf->last_watch_mask != flags) { + intf->last_watch_mask = flags; + intf->handlers->set_need_watch(intf->send_info, + intf->last_watch_mask); + } + spin_unlock_irqrestore(&intf->watch_lock, iflags); +} + /* * Find the next sequence number not being used and add the given * message with the given timeout to the sequence table. This must be @@ -981,6 +1044,7 @@ static int intf_next_seq(struct ipmi_smi *intf, *seq = i; *seqid = intf->seq_table[i].seqid; intf->curr_seq = (i+1)%IPMI_IPMB_NUM_SEQ; + smi_add_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES); need_waiter(intf); } else { rv = -EAGAIN; @@ -1019,6 +1083,7 @@ static int intf_find_seq(struct ipmi_smi *intf, && (ipmi_addr_equal(addr, &msg->addr))) { *recv_msg = msg; intf->seq_table[seq].inuse = 0; + smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES); rv = 0; } } @@ -1080,6 +1145,7 @@ static int intf_err_seq(struct ipmi_smi *intf, struct seq_table *ent = &intf->seq_table[seq]; ent->inuse = 0; + smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES); msg = ent->recv_msg; rv = 0; } @@ -1091,30 +1157,6 @@ static int intf_err_seq(struct ipmi_smi *intf, return rv; } -/* Must be called with xmit_msgs_lock held. */ -static void smi_tell_to_watch(struct ipmi_smi *intf, - unsigned int flags, - struct ipmi_smi_msg *smi_msg) -{ - if (flags & IPMI_WATCH_MASK_CHECK_MESSAGES) { - if (!smi_msg) - return; - - if (!smi_msg->needs_response) - return; - } - - if (!intf->handlers->set_need_watch) - return; - - if ((intf->last_watch_mask & flags) == flags) - return; - - intf->last_watch_mask |= flags; - intf->handlers->set_need_watch(intf->send_info, - intf->last_watch_mask); -} - int ipmi_create_user(unsigned int if_num, const struct ipmi_user_hndl *handler, void *handler_data, @@ -1175,12 +1217,9 @@ int ipmi_create_user(unsigned int if_num, spin_lock_irqsave(&intf->seq_lock, flags); list_add_rcu(&new_user->link, &intf->users); spin_unlock_irqrestore(&intf->seq_lock, flags); - if (handler->ipmi_watchdog_pretimeout) { + if (handler->ipmi_watchdog_pretimeout) /* User wants pretimeouts, so make sure to watch for them. */ - if (atomic_inc_return(&intf->watchdog_waiters) == 1) - smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_WATCHDOG, - NULL); - } + smi_add_watch(intf, IPMI_WATCH_MASK_CHECK_WATCHDOG); srcu_read_unlock(&ipmi_interfaces_srcu, index); *user = new_user; return 0; @@ -1251,7 +1290,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user) user->handler->shutdown(user->handler_data); if (user->handler->ipmi_watchdog_pretimeout) - atomic_dec(&intf->watchdog_waiters); + smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_WATCHDOG); if (user->gets_events) atomic_dec(&intf->event_waiters); @@ -1264,6 +1303,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user) if (intf->seq_table[i].inuse && (intf->seq_table[i].recv_msg->user == user)) { intf->seq_table[i].inuse = 0; + smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES); ipmi_free_recv_msg(intf->seq_table[i].recv_msg); } } @@ -1606,8 +1646,7 @@ int ipmi_register_for_cmd(struct ipmi_user *user, goto out_unlock; } - if (atomic_inc_return(&intf->command_waiters) == 1) - smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_COMMANDS, NULL); + smi_add_watch(intf, IPMI_WATCH_MASK_CHECK_COMMANDS); list_add_rcu(&rcvr->link, &intf->cmd_rcvrs); @@ -1657,7 +1696,7 @@ int ipmi_unregister_for_cmd(struct ipmi_user *user, synchronize_rcu(); release_ipmi_user(user, index); while (rcvrs) { - atomic_dec(&intf->command_waiters); + smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_COMMANDS); rcvr = rcvrs; rcvrs = rcvr->next; kfree(rcvr); @@ -1785,8 +1824,6 @@ static void smi_send(struct ipmi_smi *intf, spin_lock_irqsave(&intf->xmit_msgs_lock, flags); smi_msg = smi_add_send_msg(intf, smi_msg, priority); - smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES, smi_msg); - if (!run_to_completion) spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags); @@ -1986,9 +2023,6 @@ static int i_ipmi_req_ipmb(struct ipmi_smi *intf, ipmb_seq, broadcast, source_address, source_lun); - /* We will be getting a response in the BMC message queue. */ - smi_msg->needs_response = true; - /* * Copy the message into the recv message data, so we * can retransmit it later if necessary. @@ -2176,7 +2210,6 @@ static int i_ipmi_request(struct ipmi_user *user, goto out; } } - smi_msg->needs_response = false; rcu_read_lock(); if (intf->in_shutdown) { @@ -3390,9 +3423,8 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, INIT_LIST_HEAD(&intf->xmit_msgs); INIT_LIST_HEAD(&intf->hp_xmit_msgs); spin_lock_init(&intf->events_lock); + spin_lock_init(&intf->watch_lock); atomic_set(&intf->event_waiters, 0); - atomic_set(&intf->watchdog_waiters, 0); - atomic_set(&intf->command_waiters, 0); intf->ticks_to_req_ev = IPMI_REQUEST_EV_TIME; INIT_LIST_HEAD(&intf->waiting_events); intf->waiting_events_count = 0; @@ -4408,8 +4440,6 @@ static void smi_recv_tasklet(unsigned long val) } } - smi_tell_to_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES, newmsg); - if (!run_to_completion) spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags); if (newmsg) @@ -4537,7 +4567,7 @@ 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, - unsigned int *watch_mask) + bool *need_timer) { struct ipmi_recv_msg *msg; @@ -4549,13 +4579,14 @@ static void check_msg_timeout(struct ipmi_smi *intf, struct seq_table *ent, if (timeout_period < ent->timeout) { ent->timeout -= timeout_period; - *watch_mask |= IPMI_WATCH_MASK_CHECK_MESSAGES; + *need_timer = true; return; } if (ent->retries_left == 0) { /* The message has used all its retries. */ ent->inuse = 0; + smi_remove_watch(intf, IPMI_WATCH_MASK_CHECK_MESSAGES); msg = ent->recv_msg; list_add_tail(&msg->link, timeouts); if (ent->broadcast) @@ -4568,7 +4599,7 @@ static void check_msg_timeout(struct ipmi_smi *intf, struct seq_table *ent, struct ipmi_smi_msg *smi_msg; /* More retries, send again. */ - *watch_mask |= IPMI_WATCH_MASK_CHECK_MESSAGES; + *need_timer = true; /* * Start with the max timer, set to normal timer after @@ -4613,20 +4644,20 @@ static void check_msg_timeout(struct ipmi_smi *intf, struct seq_table *ent, } } -static unsigned int ipmi_timeout_handler(struct ipmi_smi *intf, - unsigned long timeout_period) +static bool ipmi_timeout_handler(struct ipmi_smi *intf, + unsigned long timeout_period) { struct list_head timeouts; struct ipmi_recv_msg *msg, *msg2; unsigned long flags; int i; - unsigned int watch_mask = 0; + bool need_timer = false; if (!intf->bmc_registered) { kref_get(&intf->refcount); if (!schedule_work(&intf->bmc_reg_work)) { kref_put(&intf->refcount, intf_free); - watch_mask |= IPMI_WATCH_MASK_INTERNAL; + need_timer = true; } } @@ -4646,7 +4677,7 @@ static unsigned int ipmi_timeout_handler(struct ipmi_smi *intf, for (i = 0; i < IPMI_IPMB_NUM_SEQ; i++) check_msg_timeout(intf, &intf->seq_table[i], &timeouts, timeout_period, i, - &flags, &watch_mask); + &flags, &need_timer); spin_unlock_irqrestore(&intf->seq_lock, flags); list_for_each_entry_safe(msg, msg2, &timeouts, link) @@ -4677,7 +4708,7 @@ static unsigned int ipmi_timeout_handler(struct ipmi_smi *intf, tasklet_schedule(&intf->recv_tasklet); - return watch_mask; + return need_timer; } static void ipmi_request_event(struct ipmi_smi *intf) @@ -4697,9 +4728,8 @@ static atomic_t stop_operation; static void ipmi_timeout(struct timer_list *unused) { struct ipmi_smi *intf; - unsigned int watch_mask = 0; + bool need_timer = false; int index; - unsigned long flags; if (atomic_read(&stop_operation)) return; @@ -4712,28 +4742,14 @@ static void ipmi_timeout(struct timer_list *unused) ipmi_request_event(intf); intf->ticks_to_req_ev = IPMI_REQUEST_EV_TIME; } - watch_mask |= IPMI_WATCH_MASK_INTERNAL; + need_timer = true; } - if (atomic_read(&intf->watchdog_waiters)) - watch_mask |= IPMI_WATCH_MASK_CHECK_WATCHDOG; - - if (atomic_read(&intf->command_waiters)) - watch_mask |= IPMI_WATCH_MASK_CHECK_COMMANDS; - - watch_mask |= ipmi_timeout_handler(intf, IPMI_TIMEOUT_TIME); - - spin_lock_irqsave(&intf->xmit_msgs_lock, flags); - if (watch_mask != intf->last_watch_mask && - intf->handlers->set_need_watch) - intf->handlers->set_need_watch(intf->send_info, - watch_mask); - intf->last_watch_mask = watch_mask; - spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags); + need_timer |= ipmi_timeout_handler(intf, IPMI_TIMEOUT_TIME); } srcu_read_unlock(&ipmi_interfaces_srcu, index); - if (watch_mask) + if (need_timer) mod_timer(&ipmi_timer, jiffies + IPMI_TIMEOUT_JIFFIES); } diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index c81c84a723b6..ae99d6a14789 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1066,7 +1066,7 @@ static void set_need_watch(void *send_info, unsigned int watch_mask) unsigned long flags; int enable; - enable = !!(watch_mask & ~IPMI_WATCH_MASK_INTERNAL); + enable = !!watch_mask; atomic_set(&smi_info->need_watch, enable); spin_lock_irqsave(&smi_info->si_lock, flags); diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index a1219af32105..e4abaa8e22bc 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1129,7 +1129,7 @@ static void ssif_set_need_watch(void *send_info, unsigned int watch_mask) if (watch_mask & IPMI_WATCH_MASK_CHECK_MESSAGES) timeout = SSIF_WATCH_MSG_TIMEOUT; - else if (watch_mask & ~IPMI_WATCH_MASK_INTERNAL) + else if (watch_mask) timeout = SSIF_WATCH_WATCHDOG_TIMEOUT; flags = ipmi_ssif_lock_cond(ssif_info, &oflags); diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h index da6abb06a5dc..4dc66157d872 100644 --- a/include/linux/ipmi_smi.h +++ b/include/linux/ipmi_smi.h @@ -32,14 +32,11 @@ struct ipmi_smi; /* * Flags for set_check_watch() below. Tells if the SMI should be - * waiting for watchdog timeouts, commands and/or messages. There is - * also an internal flag for the message handler, SMIs should ignore - * it. + * waiting for watchdog timeouts, commands and/or messages. */ -#define IPMI_WATCH_MASK_INTERNAL (1 << 0) -#define IPMI_WATCH_MASK_CHECK_MESSAGES (1 << 1) -#define IPMI_WATCH_MASK_CHECK_WATCHDOG (1 << 2) -#define IPMI_WATCH_MASK_CHECK_COMMANDS (1 << 3) +#define IPMI_WATCH_MASK_CHECK_MESSAGES (1 << 0) +#define IPMI_WATCH_MASK_CHECK_WATCHDOG (1 << 1) +#define IPMI_WATCH_MASK_CHECK_COMMANDS (1 << 2) /* * Messages to/from the lower layer. The smi interface will take one @@ -66,12 +63,6 @@ struct ipmi_smi_msg { int rsp_size; unsigned char rsp[IPMI_MAX_MSG_LENGTH]; - /* - * There should be a response message coming back in the BMC - * message queue. - */ - bool needs_response; - /* * Will be called when the system is done with the message * (presumably to free it). -- cgit v1.2.3 From d5a2197be15ef600c4878d7aaaea8bfb5a06f0d9 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Tue, 30 Oct 2018 11:46:52 +0000 Subject: ipmi: clean an indentation issue, remove extraneous space Trivial fix to clean up an indentation issue, remove space Signed-off-by: Colin Ian King Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_ssif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index e4abaa8e22bc..11ac025b9a94 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1800,7 +1800,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) ssif_info, &ssif_info->client->dev, slave_addr); - if (rv) { + if (rv) { pr_err("Unable to register device: error %d\n", rv); goto out_remove_attr; } -- cgit v1.2.3 From 95ac0daa0ca98ad8ac23769021144f8bf04cf406 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Fri, 16 Nov 2018 09:55:47 -0600 Subject: ipmi: Fix return value when a message is truncated The user can ask the message to be returned even if it didn't supply enough memory for the data, and it will return an error but still fills in as much data as possible. However, the return value wasn't being set to an error, it was being overwritten. Create a second return value for that case. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_devintf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_devintf.c b/drivers/char/ipmi/ipmi_devintf.c index effab11887ca..99c9f581a1f3 100644 --- a/drivers/char/ipmi/ipmi_devintf.c +++ b/drivers/char/ipmi/ipmi_devintf.c @@ -207,7 +207,7 @@ static int handle_recv(struct ipmi_file_private *priv, struct list_head *entry; struct ipmi_recv_msg *msg; unsigned long flags; - int rv = 0; + int rv = 0, rv2 = 0; /* We claim a mutex because we don't want two users getting something from the queue at a time. @@ -250,7 +250,7 @@ static int handle_recv(struct ipmi_file_private *priv, if (msg->msg.data_len > 0) { if (rsp->msg.data_len < msg->msg.data_len) { - rv = -EMSGSIZE; + rv2 = -EMSGSIZE; if (trunc) msg->msg.data_len = rsp->msg.data_len; else @@ -274,7 +274,7 @@ static int handle_recv(struct ipmi_file_private *priv, mutex_unlock(&priv->recv_mutex); ipmi_free_recv_msg(msg); - return 0; + return rv2; recv_putback_on_err: /* If we got an error, put the message back onto -- cgit v1.2.3 From 42c7c6ef1e6fa5fc0425120f06f045190b1dda2d Mon Sep 17 00:00:00 2001 From: Nicholas Mc Guire Date: Wed, 21 Nov 2018 16:08:25 +0100 Subject: ipmi: kcs_bmc: handle devm_kasprintf() failure case devm_kasprintf() may return NULL if internal allocation failed so this assignment is not safe. Moved the error exit path and added the !NULL which then allows the devres manager to take care of cleanup. Signed-off-by: Nicholas Mc Guire Fixes: cd2315d471f4 ("ipmi: kcs_bmc: don't change device name") Signed-off-by: Corey Minyard Reviewed-by: Haiyue Wang --- drivers/char/ipmi/kcs_bmc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c index e6124bd548df..ed4dc3b1843e 100644 --- a/drivers/char/ipmi/kcs_bmc.c +++ b/drivers/char/ipmi/kcs_bmc.c @@ -440,12 +440,13 @@ struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv, u32 channel) kcs_bmc->data_in = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL); kcs_bmc->data_out = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL); kcs_bmc->kbuffer = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL); - if (!kcs_bmc->data_in || !kcs_bmc->data_out || !kcs_bmc->kbuffer) - return NULL; kcs_bmc->miscdev.minor = MISC_DYNAMIC_MINOR; kcs_bmc->miscdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s%u", DEVICE_NAME, channel); + if (!kcs_bmc->data_in || !kcs_bmc->data_out || !kcs_bmc->kbuffer || + !kcs_bmc->miscdev.name) + return NULL; kcs_bmc->miscdev.fops = &kcs_bmc_fops; return kcs_bmc; -- cgit v1.2.3 From 83af41947c5cbdf33c78f4818bb0cb23da39d132 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 27 Nov 2018 18:56:04 -0600 Subject: ipmi:ssif: Change some pr_xxx to dev_xxx calls Trying to make things more consistent and compliant with what should be in drivers. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_ssif.c | 116 +++++++++++++++++++++++++++--------------- 1 file changed, 74 insertions(+), 42 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 11ac025b9a94..8b5aec5430f1 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -28,6 +28,7 @@ */ #define pr_fmt(fmt) "ipmi_ssif: " fmt +#define dev_fmt(fmt) "ipmi_ssif: " fmt #if defined(MODVERSIONS) #include @@ -328,7 +329,8 @@ static void deliver_recv_msg(struct ssif_info *ssif_info, { if (msg->rsp_size < 0) { return_hosed_msg(ssif_info, msg); - pr_err("%s: Malformed message: rsp_size = %d\n", + dev_err(&ssif_info->client->dev, + "%s: Malformed message: rsp_size = %d\n", __func__, msg->rsp_size); } else { ipmi_smi_msg_received(ssif_info->intf, msg); @@ -545,7 +547,8 @@ static void start_get(struct ssif_info *ssif_info) if (rv < 0) { /* request failed, just return the error. */ if (ssif_info->ssif_debug & SSIF_DEBUG_MSG) - pr_info("Error from i2c_non_blocking_op(5)\n"); + dev_dbg(&ssif_info->client->dev, + "Error from i2c_non_blocking_op(5)\n"); msg_done_handler(ssif_info, -EIO, NULL, 0); } @@ -647,7 +650,8 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, ssif_inc_stat(ssif_info, receive_errors); if (ssif_info->ssif_debug & SSIF_DEBUG_MSG) - pr_info("Error in msg_done_handler: %d\n", result); + dev_dbg(&ssif_info->client->dev, + "%s: Error %d\n", __func__, result); len = 0; goto continue_op; } @@ -672,7 +676,8 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, ssif_info->recv, I2C_SMBUS_BLOCK_DATA); if (rv < 0) { if (ssif_info->ssif_debug & SSIF_DEBUG_MSG) - pr_info("Error from i2c_non_blocking_op(1)\n"); + dev_dbg(&ssif_info->client->dev, + "Error from i2c_non_blocking_op(1)\n"); result = -EIO; } else @@ -685,7 +690,8 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, if (len == 0) { result = -EIO; if (ssif_info->ssif_debug & SSIF_DEBUG_MSG) - pr_info("Middle message with no data\n"); + dev_dbg(&ssif_info->client->dev, + "Middle message with no data\n"); goto continue_op; } @@ -698,7 +704,8 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, /* All blocks but the last must have 31 data bytes. */ result = -EIO; if (ssif_info->ssif_debug & SSIF_DEBUG_MSG) - pr_info("Received middle message <31\n"); + dev_dbg(&ssif_info->client->dev, + "Received middle message <31\n"); goto continue_op; } @@ -707,7 +714,8 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, /* Received message too big, abort the operation. */ result = -E2BIG; if (ssif_info->ssif_debug & SSIF_DEBUG_MSG) - pr_info("Received message too big\n"); + dev_dbg(&ssif_info->client->dev, + "Received message too big\n"); goto continue_op; } @@ -738,7 +746,8 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, I2C_SMBUS_BLOCK_DATA); if (rv < 0) { if (ssif_info->ssif_debug & SSIF_DEBUG_MSG) - pr_info("Error from ssif_i2c_send\n"); + dev_dbg(&ssif_info->client->dev, + "Error from ssif_i2c_send\n"); result = -EIO; } else @@ -755,7 +764,8 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, } if (ssif_info->ssif_debug & SSIF_DEBUG_STATE) - pr_info("DONE 1: state = %d, result=%d\n", + dev_dbg(&ssif_info->client->dev, + "DONE 1: state = %d, result=%d\n", ssif_info->ssif_state, result); flags = ipmi_ssif_lock_cond(ssif_info, &oflags); @@ -789,8 +799,9 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, */ ssif_info->ssif_state = SSIF_NORMAL; ipmi_ssif_unlock_cond(ssif_info, flags); - pr_warn("Error getting flags: %d %d, %x\n", - result, len, (len >= 3) ? data[2] : 0); + dev_warn(&ssif_info->client->dev, + "Error getting flags: %d %d, %x\n", + result, len, (len >= 3) ? data[2] : 0); } else if (data[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2 || data[1] != IPMI_GET_MSG_FLAGS_CMD) { /* @@ -798,8 +809,9 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, * response to a previous command. */ ipmi_ssif_unlock_cond(ssif_info, flags); - pr_warn("Invalid response getting flags: %x %x\n", - data[0], data[1]); + dev_warn(&ssif_info->client->dev, + "Invalid response getting flags: %x %x\n", + data[0], data[1]); } else { ssif_inc_stat(ssif_info, flag_fetches); ssif_info->msg_flags = data[3]; @@ -811,12 +823,14 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, /* We cleared the flags. */ if ((result < 0) || (len < 3) || (data[2] != 0)) { /* Error clearing flags */ - pr_warn("Error clearing flags: %d %d, %x\n", - result, len, (len >= 3) ? data[2] : 0); + dev_warn(&ssif_info->client->dev, + "Error clearing flags: %d %d, %x\n", + result, len, (len >= 3) ? data[2] : 0); } else if (data[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2 || data[1] != IPMI_CLEAR_MSG_FLAGS_CMD) { - pr_warn("Invalid response clearing flags: %x %x\n", - data[0], data[1]); + dev_warn(&ssif_info->client->dev, + "Invalid response clearing flags: %x %x\n", + data[0], data[1]); } ssif_info->ssif_state = SSIF_NORMAL; ipmi_ssif_unlock_cond(ssif_info, flags); @@ -832,8 +846,9 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, handle_flags(ssif_info, flags); } else if (msg->rsp[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2 || msg->rsp[1] != IPMI_READ_EVENT_MSG_BUFFER_CMD) { - pr_warn("Invalid response getting events: %x %x\n", - msg->rsp[0], msg->rsp[1]); + dev_warn(&ssif_info->client->dev, + "Invalid response getting events: %x %x\n", + msg->rsp[0], msg->rsp[1]); msg->done(msg); /* Take off the event flag. */ ssif_info->msg_flags &= ~EVENT_MSG_BUFFER_FULL; @@ -855,8 +870,9 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, handle_flags(ssif_info, flags); } else if (msg->rsp[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2 || msg->rsp[1] != IPMI_GET_MSG_CMD) { - pr_warn("Invalid response clearing flags: %x %x\n", - msg->rsp[0], msg->rsp[1]); + dev_warn(&ssif_info->client->dev, + "Invalid response clearing flags: %x %x\n", + msg->rsp[0], msg->rsp[1]); msg->done(msg); /* Take off the msg flag. */ @@ -882,7 +898,8 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result, ipmi_ssif_unlock_cond(ssif_info, flags); if (ssif_info->ssif_debug & SSIF_DEBUG_STATE) - pr_info("DONE 2: state = %d.\n", ssif_info->ssif_state); + dev_dbg(&ssif_info->client->dev, + "DONE 2: state = %d.\n", ssif_info->ssif_state); } static void msg_written_handler(struct ssif_info *ssif_info, int result, @@ -902,7 +919,8 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result, ssif_inc_stat(ssif_info, send_errors); if (ssif_info->ssif_debug & SSIF_DEBUG_MSG) - pr_info("%s: Out of retries\n", __func__); + dev_dbg(&ssif_info->client->dev, + "%s: Out of retries\n", __func__); msg_done_handler(ssif_info, -EIO, NULL, 0); return; } @@ -914,7 +932,8 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result, * handle it. */ if (ssif_info->ssif_debug & SSIF_DEBUG_MSG) - pr_info("Error in msg_written_handler: %d\n", result); + dev_dbg(&ssif_info->client->dev, + "%s: Error %d\n", __func__, result); msg_done_handler(ssif_info, result, NULL, 0); return; @@ -958,7 +977,8 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result, ssif_inc_stat(ssif_info, send_errors); if (ssif_info->ssif_debug & SSIF_DEBUG_MSG) - pr_info("Error from i2c_non_blocking_op(3)\n"); + dev_dbg(&ssif_info->client->dev, + "Error from i2c_non_blocking_op(3)\n"); msg_done_handler(ssif_info, -EIO, NULL, 0); } } else { @@ -1014,7 +1034,8 @@ static int start_resend(struct ssif_info *ssif_info) rv = ssif_i2c_send(ssif_info, msg_written_handler, I2C_SMBUS_WRITE, command, ssif_info->data, I2C_SMBUS_BLOCK_DATA); if (rv && (ssif_info->ssif_debug & SSIF_DEBUG_MSG)) - pr_info("Error from i2c_non_blocking_op(4)\n"); + dev_dbg(&ssif_info->client->dev, + "Error from i2c_non_blocking_op(4)\n"); return rv; } @@ -1083,7 +1104,8 @@ static void sender(void *send_info, struct timespec64 t; ktime_get_real_ts64(&t); - pr_info("**Enqueue %02x %02x: %lld.%6.6ld\n", + dev_dbg(&ssif_info->client->dev, + "**Enqueue %02x %02x: %lld.%6.6ld\n", msg->data[0], msg->data[1], (long long)t.tv_sec, (long)t.tv_nsec / NSEC_PER_USEC); } @@ -1614,7 +1636,8 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) slave_addr = find_slave_address(client, slave_addr); - pr_info("Trying %s-specified SSIF interface at i2c address 0x%x, adapter %s, slave address 0x%x\n", + dev_info(&client->dev, + "Trying %s-specified SSIF interface at i2c address 0x%x, adapter %s, slave address 0x%x\n", ipmi_addr_src_to_str(ssif_info->addr_source), client->addr, client->adapter->name, slave_addr); @@ -1629,7 +1652,8 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) if (!rv && (len >= 3) && (resp[2] == 0)) { if (len < 7) { if (ssif_dbg_probe) - pr_info("SSIF info too short: %d\n", len); + dev_dbg(&ssif_info->client->dev, + "SSIF info too short: %d\n", len); goto no_support; } @@ -1666,7 +1690,8 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) } else { no_support: /* Assume no multi-part or PEC support */ - pr_info("Error fetching SSIF: %d %d %2.2x, your system probably doesn't support this command so using defaults\n", + dev_info(&ssif_info->client->dev, + "Error fetching SSIF: %d %d %2.2x, your system probably doesn't support this command so using defaults\n", rv, len, resp[2]); ssif_info->max_xmit_msg_size = 32; @@ -1683,16 +1708,18 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) msg[2] = WDT_PRE_TIMEOUT_INT; rv = do_cmd(client, 3, msg, &len, resp); if (rv || (len < 3) || (resp[2] != 0)) - pr_warn("Unable to clear message flags: %d %d %2.2x\n", - rv, len, resp[2]); + dev_warn(&ssif_info->client->dev, + "Unable to clear message flags: %d %d %2.2x\n", + rv, len, resp[2]); /* Attempt to enable the event buffer. */ msg[0] = IPMI_NETFN_APP_REQUEST << 2; msg[1] = IPMI_GET_BMC_GLOBAL_ENABLES_CMD; rv = do_cmd(client, 2, msg, &len, resp); if (rv || (len < 4) || (resp[2] != 0)) { - pr_warn("Error getting global enables: %d %d %2.2x\n", - rv, len, resp[2]); + dev_warn(&ssif_info->client->dev, + "Error getting global enables: %d %d %2.2x\n", + rv, len, resp[2]); rv = 0; /* Not fatal */ goto found; } @@ -1710,8 +1737,9 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) msg[2] = ssif_info->global_enables | IPMI_BMC_EVT_MSG_BUFF; rv = do_cmd(client, 3, msg, &len, resp); if (rv || (len < 2)) { - pr_warn("Error setting global enables: %d %d %2.2x\n", - rv, len, resp[2]); + dev_warn(&ssif_info->client->dev, + "Error setting global enables: %d %d %2.2x\n", + rv, len, resp[2]); rv = 0; /* Not fatal */ goto found; } @@ -1731,8 +1759,9 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) msg[2] = ssif_info->global_enables | IPMI_BMC_RCV_MSG_INTR; rv = do_cmd(client, 3, msg, &len, resp); if (rv || (len < 2)) { - pr_warn("Error setting global enables: %d %d %2.2x\n", - rv, len, resp[2]); + dev_warn(&ssif_info->client->dev, + "Error setting global enables: %d %d %2.2x\n", + rv, len, resp[2]); rv = 0; /* Not fatal */ goto found; } @@ -1745,8 +1774,9 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) found: if (ssif_dbg_probe) { - pr_info("ssif_probe: i2c_probe found device at i2c address %x\n", - client->addr); + dev_dbg(&ssif_info->client->dev, + "%s: i2c_probe found device at i2c address %x\n", + __func__, client->addr); } spin_lock_init(&ssif_info->lock); @@ -1801,7 +1831,8 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) &ssif_info->client->dev, slave_addr); if (rv) { - pr_err("Unable to register device: error %d\n", rv); + dev_err(&ssif_info->client->dev, + "Unable to register device: error %d\n", rv); goto out_remove_attr; } @@ -1810,7 +1841,8 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) if (addr_info) addr_info->client = NULL; - dev_err(&client->dev, "Unable to start IPMI SSIF: %d\n", rv); + dev_err(&ssif_info->client->dev, + "Unable to start IPMI SSIF: %d\n", rv); kfree(ssif_info); } kfree(resp); -- cgit v1.2.3 From 16ccdb552e70e7ae231e7934699fc217e8035d6f Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 24 Jan 2019 22:22:19 +0200 Subject: ipmi: Use defined constant for UUID representation Instead of magic number use pre-defined constant for UUID binary and string representations. While here, drop the implementation details of guid_t type. Signed-off-by: Andy Shevchenko [Also converted a "17" in the error string to UUID_SIZE + 1] Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 5bc84fbbeee5..79911914c1cb 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -2749,7 +2749,7 @@ static ssize_t guid_show(struct device *dev, struct device_attribute *attr, if (!guid_set) return -ENOENT; - return snprintf(buf, 38, "%pUl\n", guid.b); + return snprintf(buf, UUID_STRING_LEN + 1 + 1, "%pUl\n", &guid); } static DEVICE_ATTR_RO(guid); @@ -3148,11 +3148,11 @@ static void guid_handler(struct ipmi_smi *intf, struct ipmi_recv_msg *msg) goto out; } - if (msg->msg.data_len < 17) { + if (msg->msg.data_len < UUID_SIZE + 1) { bmc->dyn_guid_set = 0; dev_warn(intf->si_dev, - "The GUID response from the BMC was too short, it was %d but should have been 17. Assuming GUID is not available.\n", - msg->msg.data_len); + "The GUID response from the BMC was too short, it was %d but should have been %d. Assuming GUID is not available.\n", + msg->msg.data_len, UUID_SIZE + 1); goto out; } -- cgit v1.2.3 From f32043901a389ee8b5417c0cec547a84a2f9b38a Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 24 Jan 2019 22:22:20 +0200 Subject: ipmi: Use dedicated API for copying a UUID Use guid_copy() instead of memcpy() to hide guid_t implementation details and to show we expect guid_t in a raw buffer. Signed-off-by: Andy Shevchenko Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 79911914c1cb..e8ba67834746 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -3156,7 +3156,7 @@ static void guid_handler(struct ipmi_smi *intf, struct ipmi_recv_msg *msg) goto out; } - memcpy(bmc->fetch_guid.b, msg->msg.data + 1, 16); + guid_copy(&bmc->fetch_guid, (guid_t *)(msg->msg.data + 1)); /* * Make sure the guid data is available before setting * dyn_guid_set. -- cgit v1.2.3 From 41b766d661bf94a364960862cfc248a78313dbd3 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 21 Feb 2019 12:10:07 -0600 Subject: ipmi_si: Fix crash when using hard-coded device When excuting a command like: modprobe ipmi_si ports=0xffc0e3 type=bt The system would get an oops. The trouble here is that ipmi_si_hardcode_find_bmc() is called before ipmi_si_platform_init(), but initialization of the hard-coded device creates an IPMI platform device, which won't be initialized yet. The real trouble is that hard-coded devices aren't created with any device, and the fixup is done later. So do it right, create the hard-coded devices as normal platform devices. This required adding some new resource types to the IPMI platform code for passing information required by the hard-coded device and adding some code to remove the hard-coded platform devices on module removal. To enforce the "hard-coded devices passed by the user take priority over firmware devices" rule, some special code was added to check and see if a hard-coded device already exists. Reported-by: Yang Yingliang Cc: stable@vger.kernel.org # v4.15+ Signed-off-by: Corey Minyard Tested-by: Yang Yingliang --- drivers/char/ipmi/ipmi_si.h | 4 +- drivers/char/ipmi/ipmi_si_hardcode.c | 236 ++++++++++++++++++++++++++--------- drivers/char/ipmi/ipmi_si_intf.c | 23 +++- drivers/char/ipmi/ipmi_si_platform.c | 29 +++-- 4 files changed, 216 insertions(+), 76 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h index 52f6152d1fcb..7ae52c17618e 100644 --- a/drivers/char/ipmi/ipmi_si.h +++ b/drivers/char/ipmi/ipmi_si.h @@ -25,7 +25,9 @@ void ipmi_irq_finish_setup(struct si_sm_io *io); int ipmi_si_remove_by_dev(struct device *dev); void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, unsigned long addr); -int ipmi_si_hardcode_find_bmc(void); +void ipmi_hardcode_init(void); +void ipmi_si_hardcode_exit(void); +int ipmi_si_hardcode_match(int addr_type, unsigned long addr); void ipmi_si_platform_init(void); void ipmi_si_platform_shutdown(void); diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c b/drivers/char/ipmi/ipmi_si_hardcode.c index 487642809c58..1e5783961b0d 100644 --- a/drivers/char/ipmi/ipmi_si_hardcode.c +++ b/drivers/char/ipmi/ipmi_si_hardcode.c @@ -3,6 +3,7 @@ #define pr_fmt(fmt) "ipmi_hardcode: " fmt #include +#include #include "ipmi_si.h" /* @@ -12,23 +13,22 @@ #define SI_MAX_PARMS 4 -static char *si_type[SI_MAX_PARMS]; #define MAX_SI_TYPE_STR 30 -static char si_type_str[MAX_SI_TYPE_STR]; +static char si_type_str[MAX_SI_TYPE_STR] __initdata; static unsigned long addrs[SI_MAX_PARMS]; static unsigned int num_addrs; static unsigned int ports[SI_MAX_PARMS]; static unsigned int num_ports; -static int irqs[SI_MAX_PARMS]; -static unsigned int num_irqs; -static int regspacings[SI_MAX_PARMS]; -static unsigned int num_regspacings; -static int regsizes[SI_MAX_PARMS]; -static unsigned int num_regsizes; -static int regshifts[SI_MAX_PARMS]; -static unsigned int num_regshifts; -static int slave_addrs[SI_MAX_PARMS]; /* Leaving 0 chooses the default value */ -static unsigned int num_slave_addrs; +static int irqs[SI_MAX_PARMS] __initdata; +static unsigned int num_irqs __initdata; +static int regspacings[SI_MAX_PARMS] __initdata; +static unsigned int num_regspacings __initdata; +static int regsizes[SI_MAX_PARMS] __initdata; +static unsigned int num_regsizes __initdata; +static int regshifts[SI_MAX_PARMS] __initdata; +static unsigned int num_regshifts __initdata; +static int slave_addrs[SI_MAX_PARMS] __initdata; +static unsigned int num_slave_addrs __initdata; module_param_string(type, si_type_str, MAX_SI_TYPE_STR, 0); MODULE_PARM_DESC(type, "Defines the type of each interface, each" @@ -73,12 +73,133 @@ MODULE_PARM_DESC(slave_addrs, "Set the default IPMB slave address for" " overridden by this parm. This is an array indexed" " by interface number."); -int ipmi_si_hardcode_find_bmc(void) +static struct platform_device *ipmi_hc_pdevs[SI_MAX_PARMS]; + +static void __init ipmi_hardcode_init_one(const char *si_type_str, + unsigned int i, + unsigned long addr, + unsigned int flags) { - int ret = -ENODEV; - int i; - struct si_sm_io io; + struct platform_device *pdev; + unsigned int num_r = 1, size; + struct resource r[4]; + struct property_entry p[6]; + enum si_type si_type; + unsigned int regspacing, regsize; + int rv; + + memset(p, 0, sizeof(p)); + memset(r, 0, sizeof(r)); + + if (!si_type_str || !*si_type_str || strcmp(si_type_str, "kcs") == 0) { + size = 2; + si_type = SI_KCS; + } else if (strcmp(si_type_str, "smic") == 0) { + size = 2; + si_type = SI_SMIC; + } else if (strcmp(si_type_str, "bt") == 0) { + size = 3; + si_type = SI_BT; + } else if (strcmp(si_type_str, "invalid") == 0) { + /* + * Allow a firmware-specified interface to be + * disabled. + */ + size = 1; + si_type = SI_TYPE_INVALID; + } else { + pr_warn("Interface type specified for interface %d, was invalid: %s\n", + i, si_type_str); + return; + } + + regsize = regsizes[i]; + if (regsize == 0) + regsize = DEFAULT_REGSIZE; + + p[0] = PROPERTY_ENTRY_U8("ipmi-type", si_type); + p[1] = PROPERTY_ENTRY_U8("slave-addr", slave_addrs[i]); + p[2] = PROPERTY_ENTRY_U8("addr-source", SI_HARDCODED); + p[3] = PROPERTY_ENTRY_U8("reg-shift", regshifts[i]); + p[4] = PROPERTY_ENTRY_U8("reg-size", regsize); + /* Last entry must be left NULL to terminate it. */ + + /* + * Register spacing is derived from the resources in + * the IPMI platform code. + */ + regspacing = regspacings[i]; + if (regspacing == 0) + regspacing = regsize; + + r[0].start = addr; + r[0].end = r[0].start + regsize - 1; + r[0].name = "IPMI Address 1"; + r[0].flags = flags; + + if (size > 1) { + r[1].start = r[0].start + regspacing; + r[1].end = r[1].start + regsize - 1; + r[1].name = "IPMI Address 2"; + r[1].flags = flags; + num_r++; + } + + if (size > 2) { + r[2].start = r[1].start + regspacing; + r[2].end = r[2].start + regsize - 1; + r[2].name = "IPMI Address 3"; + r[2].flags = flags; + num_r++; + } + + if (irqs[i]) { + r[num_r].start = irqs[i]; + r[num_r].end = irqs[i]; + r[num_r].name = "IPMI IRQ"; + r[num_r].flags = IORESOURCE_IRQ; + num_r++; + } + + pdev = platform_device_alloc("hardcode-ipmi-si", i); + if (!pdev) { + pr_err("Error allocating IPMI platform device %d\n", i); + return; + } + + rv = platform_device_add_resources(pdev, r, num_r); + if (rv) { + dev_err(&pdev->dev, + "Unable to add hard-code resources: %d\n", rv); + goto err; + } + + rv = platform_device_add_properties(pdev, p); + if (rv) { + dev_err(&pdev->dev, + "Unable to add hard-code properties: %d\n", rv); + goto err; + } + + rv = platform_device_add(pdev); + if (rv) { + dev_err(&pdev->dev, + "Unable to add hard-code device: %d\n", rv); + goto err; + } + + ipmi_hc_pdevs[i] = pdev; + return; + +err: + platform_device_put(pdev); +} + +void __init ipmi_hardcode_init(void) +{ + unsigned int i; char *str; + char *si_type[SI_MAX_PARMS]; /* Parse out the si_type string into its components. */ str = si_type_str; @@ -95,54 +216,45 @@ int ipmi_si_hardcode_find_bmc(void) } } - memset(&io, 0, sizeof(io)); for (i = 0; i < SI_MAX_PARMS; i++) { - if (!ports[i] && !addrs[i]) - continue; - - io.addr_source = SI_HARDCODED; - pr_info("probing via hardcoded address\n"); - - if (!si_type[i] || strcmp(si_type[i], "kcs") == 0) { - io.si_type = SI_KCS; - } else if (strcmp(si_type[i], "smic") == 0) { - io.si_type = SI_SMIC; - } else if (strcmp(si_type[i], "bt") == 0) { - io.si_type = SI_BT; - } else { - pr_warn("Interface type specified for interface %d, was invalid: %s\n", - i, si_type[i]); - continue; - } + if (i < num_ports && ports[i]) + ipmi_hardcode_init_one(si_type[i], i, ports[i], + IORESOURCE_IO); + if (i < num_addrs && addrs[i]) + ipmi_hardcode_init_one(si_type[i], i, addrs[i], + IORESOURCE_MEM); + } +} - if (ports[i]) { - /* An I/O port */ - io.addr_data = ports[i]; - io.addr_type = IPMI_IO_ADDR_SPACE; - } else if (addrs[i]) { - /* A memory port */ - io.addr_data = addrs[i]; - io.addr_type = IPMI_MEM_ADDR_SPACE; - } else { - pr_warn("Interface type specified for interface %d, but port and address were not set or set to zero\n", - i); - continue; - } +void ipmi_si_hardcode_exit(void) +{ + unsigned int i; - io.addr = NULL; - io.regspacing = regspacings[i]; - if (!io.regspacing) - io.regspacing = DEFAULT_REGSPACING; - io.regsize = regsizes[i]; - if (!io.regsize) - io.regsize = DEFAULT_REGSIZE; - io.regshift = regshifts[i]; - io.irq = irqs[i]; - if (io.irq) - io.irq_setup = ipmi_std_irq_setup; - io.slave_addr = slave_addrs[i]; - - ret = ipmi_si_add_smi(&io); + for (i = 0; i < SI_MAX_PARMS; i++) { + if (ipmi_hc_pdevs[i]) + platform_device_unregister(ipmi_hc_pdevs[i]); } - return ret; +} + +/* + * Returns true of the given address exists as a hardcoded address, + * false if not. + */ +int ipmi_si_hardcode_match(int addr_type, unsigned long addr) +{ + unsigned int i; + + if (addr_type == IPMI_IO_ADDR_SPACE) { + for (i = 0; i < num_ports; i++) { + if (ports[i] == addr) + return 1; + } + } else { + for (i = 0; i < num_addrs; i++) { + if (addrs[i] == addr) + return 1; + } + } + + return 0; } diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index ae99d6a14789..abbd526626d5 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1865,6 +1865,18 @@ int ipmi_si_add_smi(struct si_sm_io *io) int rv = 0; struct smi_info *new_smi, *dup; + /* + * If the user gave us a hard-coded device at the same + * address, they presumably want us to use it and not what is + * in the firmware. + */ + if (io->addr_source != SI_HARDCODED && + ipmi_si_hardcode_match(io->addr_type, io->addr_data)) { + dev_info(io->dev, + "Hard-coded device at this address already exists"); + return -ENODEV; + } + if (!io->io_setup) { if (io->addr_type == IPMI_IO_ADDR_SPACE) { io->io_setup = ipmi_si_port_setup; @@ -2097,7 +2109,7 @@ static int try_smi_init(struct smi_info *new_smi) return rv; } -static int init_ipmi_si(void) +static int __init init_ipmi_si(void) { struct smi_info *e; enum ipmi_addr_src type = SI_INVALID; @@ -2105,11 +2117,9 @@ static int init_ipmi_si(void) if (initialized) return 0; - pr_info("IPMI System Interface driver\n"); + ipmi_hardcode_init(); - /* If the user gave us a device, they presumably want us to use it */ - if (!ipmi_si_hardcode_find_bmc()) - goto do_scan; + pr_info("IPMI System Interface driver\n"); ipmi_si_platform_init(); @@ -2121,7 +2131,6 @@ static int init_ipmi_si(void) with multiple BMCs we assume that there will be several instances of a given type so if we succeed in registering a type then also try to register everything else of the same type */ -do_scan: mutex_lock(&smi_infos_lock); list_for_each_entry(e, &smi_infos, link) { /* Try to register a device if it has an IRQ and we either @@ -2307,6 +2316,8 @@ static void cleanup_ipmi_si(void) list_for_each_entry_safe(e, tmp_e, &smi_infos, link) cleanup_one_si(e); mutex_unlock(&smi_infos_lock); + + ipmi_si_hardcode_exit(); } module_exit(cleanup_ipmi_si); diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c index 15cf819f884f..8158d03542f4 100644 --- a/drivers/char/ipmi/ipmi_si_platform.c +++ b/drivers/char/ipmi/ipmi_si_platform.c @@ -128,8 +128,6 @@ ipmi_get_info_from_resources(struct platform_device *pdev, if (res_second->start > io->addr_data) io->regspacing = res_second->start - io->addr_data; } - io->regsize = DEFAULT_REGSIZE; - io->regshift = 0; return res; } @@ -137,7 +135,7 @@ ipmi_get_info_from_resources(struct platform_device *pdev, static int platform_ipmi_probe(struct platform_device *pdev) { struct si_sm_io io; - u8 type, slave_addr, addr_source; + u8 type, slave_addr, addr_source, regsize, regshift; int rv; rv = device_property_read_u8(&pdev->dev, "addr-source", &addr_source); @@ -149,7 +147,7 @@ static int platform_ipmi_probe(struct platform_device *pdev) if (addr_source == SI_SMBIOS) { if (!si_trydmi) return -ENODEV; - } else { + } else if (addr_source != SI_HARDCODED) { if (!si_tryplatform) return -ENODEV; } @@ -169,11 +167,23 @@ static int platform_ipmi_probe(struct platform_device *pdev) case SI_BT: io.si_type = type; break; + case SI_TYPE_INVALID: /* User disabled this in hardcode. */ + return -ENODEV; default: dev_err(&pdev->dev, "ipmi-type property is invalid\n"); return -EINVAL; } + io.regsize = DEFAULT_REGSIZE; + rv = device_property_read_u8(&pdev->dev, "reg-size", ®size); + if (!rv) + io.regsize = regsize; + + io.regshift = 0; + rv = device_property_read_u8(&pdev->dev, "reg-shift", ®shift); + if (!rv) + io.regshift = regshift; + if (!ipmi_get_info_from_resources(pdev, &io)) return -EINVAL; @@ -193,7 +203,8 @@ static int platform_ipmi_probe(struct platform_device *pdev) io.dev = &pdev->dev; - pr_info("ipmi_si: SMBIOS: %s %#lx regsize %d spacing %d irq %d\n", + pr_info("ipmi_si: %s: %s %#lx regsize %d spacing %d irq %d\n", + ipmi_addr_src_to_str(addr_source), (io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem", io.addr_data, io.regsize, io.regspacing, io.irq); @@ -358,6 +369,9 @@ static int acpi_ipmi_probe(struct platform_device *pdev) goto err_free; } + io.regsize = DEFAULT_REGSIZE; + io.regshift = 0; + res = ipmi_get_info_from_resources(pdev, &io); if (!res) { rv = -EINVAL; @@ -420,8 +434,9 @@ static int ipmi_remove(struct platform_device *pdev) } static const struct platform_device_id si_plat_ids[] = { - { "dmi-ipmi-si", 0 }, - { } + { "dmi-ipmi-si", 0 }, + { "hardcode-ipmi-si", 0 }, + { } }; struct platform_driver ipmi_platform_driver = { -- cgit v1.2.3 From f25eb4483268c3352973f7ed888f26b7fb9662db Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 21 Feb 2019 12:43:20 -0600 Subject: ipmi_si: Convert some types into unsigned Things dealing with registers and addresses are all unsigned, make them so. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_sm.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_si_sm.h b/drivers/char/ipmi/ipmi_si_sm.h index aaddf047d923..41aaa555d566 100644 --- a/drivers/char/ipmi/ipmi_si_sm.h +++ b/drivers/char/ipmi/ipmi_si_sm.h @@ -42,11 +42,11 @@ struct si_sm_io { * state machine shouldn't touch these. */ void __iomem *addr; - int regspacing; - int regsize; - int regshift; + unsigned int regspacing; + unsigned int regsize; + unsigned int regshift; int addr_type; - long addr_data; + unsigned long addr_data; enum ipmi_addr_src addr_source; /* ACPI, PCI, SMBIOS, hardcode, etc. */ void (*addr_source_cleanup)(struct si_sm_io *io); void *addr_source_data; -- cgit v1.2.3 From f6296bdc49c4d3794b5a3ef0012fc0889e2f3f94 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 21 Feb 2019 12:53:00 -0600 Subject: ipmi_si: Rename addr_type to addr_space to match what it does Make the naming consistent, and make the values an enum. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si.h | 5 +---- drivers/char/ipmi/ipmi_si_hardcode.c | 4 ++-- drivers/char/ipmi/ipmi_si_hotmod.c | 7 ++++--- drivers/char/ipmi/ipmi_si_intf.c | 14 +++++++------- drivers/char/ipmi/ipmi_si_parisc.c | 2 +- drivers/char/ipmi/ipmi_si_pci.c | 4 ++-- drivers/char/ipmi/ipmi_si_platform.c | 14 +++++++------- drivers/char/ipmi/ipmi_si_sm.h | 6 +++++- 8 files changed, 29 insertions(+), 27 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h index 7ae52c17618e..3efc8a71aab4 100644 --- a/drivers/char/ipmi/ipmi_si.h +++ b/drivers/char/ipmi/ipmi_si.h @@ -9,9 +9,6 @@ #include #include "ipmi_si_sm.h" -#define IPMI_IO_ADDR_SPACE 0 -#define IPMI_MEM_ADDR_SPACE 1 - #define DEFAULT_REGSPACING 1 #define DEFAULT_REGSIZE 1 @@ -27,7 +24,7 @@ void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, unsigned long addr); void ipmi_hardcode_init(void); void ipmi_si_hardcode_exit(void); -int ipmi_si_hardcode_match(int addr_type, unsigned long addr); +int ipmi_si_hardcode_match(int addr_space, unsigned long addr); void ipmi_si_platform_init(void); void ipmi_si_platform_shutdown(void); diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c b/drivers/char/ipmi/ipmi_si_hardcode.c index 1e5783961b0d..86ac9b8a3219 100644 --- a/drivers/char/ipmi/ipmi_si_hardcode.c +++ b/drivers/char/ipmi/ipmi_si_hardcode.c @@ -240,11 +240,11 @@ void ipmi_si_hardcode_exit(void) * Returns true of the given address exists as a hardcoded address, * false if not. */ -int ipmi_si_hardcode_match(int addr_type, unsigned long addr) +int ipmi_si_hardcode_match(int addr_space, unsigned long addr) { unsigned int i; - if (addr_type == IPMI_IO_ADDR_SPACE) { + if (addr_space == IPMI_IO_ADDR_SPACE) { for (i = 0; i < num_ports; i++) { if (ports[i] == addr) return 1; diff --git a/drivers/char/ipmi/ipmi_si_hotmod.c b/drivers/char/ipmi/ipmi_si_hotmod.c index c0067fd0480d..f3d455bbf3cc 100644 --- a/drivers/char/ipmi/ipmi_si_hotmod.c +++ b/drivers/char/ipmi/ipmi_si_hotmod.c @@ -106,7 +106,7 @@ static int hotmod_handler(const char *val, const struct kernel_param *kp) char *next, *curr, *s, *n, *o; enum hotmod_op op; enum si_type si_type; - int addr_space; + enum ipmi_addr_space addr_space; unsigned long addr; int regspacing; int regsize; @@ -150,9 +150,10 @@ static int hotmod_handler(const char *val, const struct kernel_param *kp) break; si_type = ival; - rv = parse_str(hotmod_as, &addr_space, "address space", &curr); + rv = parse_str(hotmod_as, &ival, "address space", &curr); if (rv) break; + addr_space = ival; s = strchr(curr, ','); if (s) { @@ -215,7 +216,7 @@ static int hotmod_handler(const char *val, const struct kernel_param *kp) io.addr_source = SI_HOTMOD; io.si_type = si_type; io.addr_data = addr; - io.addr_type = addr_space; + io.addr_space = addr_space; io.addr = NULL; io.regspacing = regspacing; diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index abbd526626d5..54e3b4f2c024 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1645,7 +1645,7 @@ static ssize_t ipmi_params_show(struct device *dev, return snprintf(buf, 200, "%s,%s,0x%lx,rsp=%d,rsi=%d,rsh=%d,irq=%d,ipmb=%d\n", si_to_str[smi_info->io.si_type], - addr_space_to_str[smi_info->io.addr_type], + addr_space_to_str[smi_info->io.addr_space], smi_info->io.addr_data, smi_info->io.regspacing, smi_info->io.regsize, @@ -1843,7 +1843,7 @@ static struct smi_info *find_dup_si(struct smi_info *info) struct smi_info *e; list_for_each_entry(e, &smi_infos, link) { - if (e->io.addr_type != info->io.addr_type) + if (e->io.addr_space != info->io.addr_space) continue; if (e->io.addr_data == info->io.addr_data) { /* @@ -1871,16 +1871,16 @@ int ipmi_si_add_smi(struct si_sm_io *io) * in the firmware. */ if (io->addr_source != SI_HARDCODED && - ipmi_si_hardcode_match(io->addr_type, io->addr_data)) { + ipmi_si_hardcode_match(io->addr_space, io->addr_data)) { dev_info(io->dev, "Hard-coded device at this address already exists"); return -ENODEV; } if (!io->io_setup) { - if (io->addr_type == IPMI_IO_ADDR_SPACE) { + if (io->addr_space == IPMI_IO_ADDR_SPACE) { io->io_setup = ipmi_si_port_setup; - } else if (io->addr_type == IPMI_MEM_ADDR_SPACE) { + } else if (io->addr_space == IPMI_MEM_ADDR_SPACE) { io->io_setup = ipmi_si_mem_setup; } else { return -EINVAL; @@ -1942,7 +1942,7 @@ static int try_smi_init(struct smi_info *new_smi) pr_info("Trying %s-specified %s state machine at %s address 0x%lx, slave address 0x%x, irq %d\n", ipmi_addr_src_to_str(new_smi->io.addr_source), si_to_str[new_smi->io.si_type], - addr_space_to_str[new_smi->io.addr_type], + addr_space_to_str[new_smi->io.addr_space], new_smi->io.addr_data, new_smi->io.slave_addr, new_smi->io.irq); @@ -2289,7 +2289,7 @@ void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, mutex_lock(&smi_infos_lock); list_for_each_entry_safe(e, tmp_e, &smi_infos, link) { - if (e->io.addr_type != addr_space) + if (e->io.addr_space != addr_space) continue; if (e->io.si_type != si_type) continue; diff --git a/drivers/char/ipmi/ipmi_si_parisc.c b/drivers/char/ipmi/ipmi_si_parisc.c index f3c99820f564..11c9160275df 100644 --- a/drivers/char/ipmi/ipmi_si_parisc.c +++ b/drivers/char/ipmi/ipmi_si_parisc.c @@ -15,7 +15,7 @@ static int __init ipmi_parisc_probe(struct parisc_device *dev) io.si_type = SI_KCS; io.addr_source = SI_DEVICETREE; - io.addr_type = IPMI_MEM_ADDR_SPACE; + io.addr_space = IPMI_MEM_ADDR_SPACE; io.addr_data = dev->hpa.start; io.regsize = 1; io.regspacing = 1; diff --git a/drivers/char/ipmi/ipmi_si_pci.c b/drivers/char/ipmi/ipmi_si_pci.c index ce00c0da5866..ce93fc7a1e36 100644 --- a/drivers/char/ipmi/ipmi_si_pci.c +++ b/drivers/char/ipmi/ipmi_si_pci.c @@ -107,10 +107,10 @@ static int ipmi_pci_probe(struct pci_dev *pdev, io.addr_source_data = pdev; if (pci_resource_flags(pdev, 0) & IORESOURCE_IO) { - io.addr_type = IPMI_IO_ADDR_SPACE; + io.addr_space = IPMI_IO_ADDR_SPACE; io.io_setup = ipmi_si_port_setup; } else { - io.addr_type = IPMI_MEM_ADDR_SPACE; + io.addr_space = IPMI_MEM_ADDR_SPACE; io.io_setup = ipmi_si_mem_setup; } io.addr_data = pci_resource_start(pdev, 0); diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c index 8158d03542f4..f690e9edb08c 100644 --- a/drivers/char/ipmi/ipmi_si_platform.c +++ b/drivers/char/ipmi/ipmi_si_platform.c @@ -107,11 +107,11 @@ ipmi_get_info_from_resources(struct platform_device *pdev, res = platform_get_resource(pdev, IORESOURCE_IO, 0); if (res) { - io->addr_type = IPMI_IO_ADDR_SPACE; + io->addr_space = IPMI_IO_ADDR_SPACE; } else { res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (res) - io->addr_type = IPMI_MEM_ADDR_SPACE; + io->addr_space = IPMI_MEM_ADDR_SPACE; } if (!res) { dev_err(&pdev->dev, "no I/O or memory address\n"); @@ -121,7 +121,7 @@ ipmi_get_info_from_resources(struct platform_device *pdev, io->regspacing = DEFAULT_REGSPACING; res_second = platform_get_resource(pdev, - (io->addr_type == IPMI_IO_ADDR_SPACE) ? + (io->addr_space == IPMI_IO_ADDR_SPACE) ? IORESOURCE_IO : IORESOURCE_MEM, 1); if (res_second) { @@ -205,7 +205,7 @@ static int platform_ipmi_probe(struct platform_device *pdev) pr_info("ipmi_si: %s: %s %#lx regsize %d spacing %d irq %d\n", ipmi_addr_src_to_str(addr_source), - (io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem", + (io.addr_space == IPMI_IO_ADDR_SPACE) ? "io" : "mem", io.addr_data, io.regsize, io.regspacing, io.irq); ipmi_si_add_smi(&io); @@ -277,9 +277,9 @@ static int of_ipmi_probe(struct platform_device *pdev) io.irq_setup = ipmi_std_irq_setup; if (resource.flags & IORESOURCE_IO) - io.addr_type = IPMI_IO_ADDR_SPACE; + io.addr_space = IPMI_IO_ADDR_SPACE; else - io.addr_type = IPMI_MEM_ADDR_SPACE; + io.addr_space = IPMI_MEM_ADDR_SPACE; io.addr_data = resource.start; @@ -310,7 +310,7 @@ static int find_slave_address(struct si_sm_io *io, int slave_addr) if (!slave_addr) { u32 flags = IORESOURCE_IO; - if (io->addr_type == IPMI_MEM_ADDR_SPACE) + if (io->addr_space == IPMI_MEM_ADDR_SPACE) flags = IORESOURCE_MEM; slave_addr = ipmi_dmi_get_slave_addr(io->si_type, flags, diff --git a/drivers/char/ipmi/ipmi_si_sm.h b/drivers/char/ipmi/ipmi_si_sm.h index 41aaa555d566..499db820fadb 100644 --- a/drivers/char/ipmi/ipmi_si_sm.h +++ b/drivers/char/ipmi/ipmi_si_sm.h @@ -26,6 +26,10 @@ enum si_type { SI_TYPE_INVALID, SI_KCS, SI_SMIC, SI_BT }; +enum ipmi_addr_space { + IPMI_IO_ADDR_SPACE, IPMI_MEM_ADDR_SPACE +}; + /* * The structure for doing I/O in the state machine. The state * machine doesn't have the actual I/O routines, they are done through @@ -45,7 +49,7 @@ struct si_sm_io { unsigned int regspacing; unsigned int regsize; unsigned int regshift; - int addr_type; + enum ipmi_addr_space addr_space; unsigned long addr_data; enum ipmi_addr_src addr_source; /* ACPI, PCI, SMBIOS, hardcode, etc. */ void (*addr_source_cleanup)(struct si_sm_io *io); -- cgit v1.2.3 From 3cd83bac481dc4fde8bedf09c8aecb3652e7e547 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 21 Feb 2019 14:21:17 -0600 Subject: ipmi: Consolidate the adding of platform devices It was being done in two different places now that hard-coded devices use platform devices, and it's about to be three with hotmod switching to platform devices. So put the code in one place. This required some rework on some interfaces to make the type space clean. Signed-off-by: Corey Minyard --- drivers/char/ipmi/Kconfig | 5 ++ drivers/char/ipmi/Makefile | 1 + drivers/char/ipmi/ipmi_dmi.c | 139 +++++++++-------------------------- drivers/char/ipmi/ipmi_dmi.h | 2 +- drivers/char/ipmi/ipmi_plat_data.c | 121 ++++++++++++++++++++++++++++++ drivers/char/ipmi/ipmi_plat_data.h | 22 ++++++ drivers/char/ipmi/ipmi_si.h | 1 + drivers/char/ipmi/ipmi_si_hardcode.c | 117 +++++------------------------ drivers/char/ipmi/ipmi_si_platform.c | 11 +-- 9 files changed, 205 insertions(+), 214 deletions(-) create mode 100644 drivers/char/ipmi/ipmi_plat_data.c create mode 100644 drivers/char/ipmi/ipmi_plat_data.h (limited to 'drivers/char') diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig index c108441882cc..94719fc6ff9d 100644 --- a/drivers/char/ipmi/Kconfig +++ b/drivers/char/ipmi/Kconfig @@ -18,6 +18,10 @@ menuconfig IPMI_HANDLER If unsure, say N. config IPMI_DMI_DECODE + select IPMI_PLAT_DATA + bool + +config IPMI_PLAT_DATA bool if IPMI_HANDLER @@ -56,6 +60,7 @@ config IPMI_DEVICE_INTERFACE config IPMI_SI tristate 'IPMI System Interface handler' + select IPMI_PLAT_DATA help Provides a driver for System Interfaces (KCS, SMIC, BT). Currently, only KCS and SMIC are supported. If diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile index 7a3baf301a8f..3f06b2062475 100644 --- a/drivers/char/ipmi/Makefile +++ b/drivers/char/ipmi/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_IPMI_HANDLER) += ipmi_msghandler.o obj-$(CONFIG_IPMI_DEVICE_INTERFACE) += ipmi_devintf.o obj-$(CONFIG_IPMI_SI) += ipmi_si.o obj-$(CONFIG_IPMI_DMI_DECODE) += ipmi_dmi.o +obj-$(CONFIG_IPMI_PLAT_DATA) += ipmi_plat_data.o obj-$(CONFIG_IPMI_SSIF) += ipmi_ssif.o obj-$(CONFIG_IPMI_POWERNV) += ipmi_powernv.o obj-$(CONFIG_IPMI_WATCHDOG) += ipmi_watchdog.o diff --git a/drivers/char/ipmi/ipmi_dmi.c b/drivers/char/ipmi/ipmi_dmi.c index 249880457b17..ff0b199be472 100644 --- a/drivers/char/ipmi/ipmi_dmi.c +++ b/drivers/char/ipmi/ipmi_dmi.c @@ -14,6 +14,7 @@ #include #include "ipmi_si_sm.h" #include "ipmi_dmi.h" +#include "ipmi_plat_data.h" #define IPMI_DMI_TYPE_KCS 0x01 #define IPMI_DMI_TYPE_SMIC 0x02 @@ -22,7 +23,7 @@ struct ipmi_dmi_info { enum si_type si_type; - u32 flags; + unsigned int space; /* addr space for si, intf# for ssif */ unsigned long addr; u8 slave_addr; struct ipmi_dmi_info *next; @@ -33,133 +34,60 @@ static struct ipmi_dmi_info *ipmi_dmi_infos; static int ipmi_dmi_nr __initdata; static void __init dmi_add_platform_ipmi(unsigned long base_addr, - u32 flags, + unsigned int space, u8 slave_addr, int irq, int offset, int type) { - struct platform_device *pdev; - struct resource r[4]; - unsigned int num_r = 1, size; - struct property_entry p[5]; - unsigned int pidx = 0; - char *name; - int rv; - enum si_type si_type; + const char *name; struct ipmi_dmi_info *info; + struct ipmi_plat_data p; - memset(p, 0, sizeof(p)); + memset(&p, 0, sizeof(p)); name = "dmi-ipmi-si"; switch (type) { case IPMI_DMI_TYPE_SSIF: name = "dmi-ipmi-ssif"; - offset = 1; - size = 1; - si_type = SI_TYPE_INVALID; + p.type = SI_TYPE_INVALID; break; case IPMI_DMI_TYPE_BT: - size = 3; - si_type = SI_BT; + p.type = SI_BT; break; case IPMI_DMI_TYPE_KCS: - size = 2; - si_type = SI_KCS; + p.type = SI_KCS; break; case IPMI_DMI_TYPE_SMIC: - size = 2; - si_type = SI_SMIC; + p.type = SI_SMIC; break; default: pr_err("Invalid IPMI type: %d\n", type); return; } - if (si_type != SI_TYPE_INVALID) - p[pidx++] = PROPERTY_ENTRY_U8("ipmi-type", si_type); - - p[pidx++] = PROPERTY_ENTRY_U8("slave-addr", slave_addr); - p[pidx++] = PROPERTY_ENTRY_U8("addr-source", SI_SMBIOS); + memset(&p, 0, sizeof(p)); + p.addr = base_addr; + p.space = space; + p.regspacing = offset; + p.irq = irq; + p.slave_addr = slave_addr; + p.addr_source = SI_SMBIOS; info = kmalloc(sizeof(*info), GFP_KERNEL); if (!info) { pr_warn("Could not allocate dmi info\n"); } else { - info->si_type = si_type; - info->flags = flags; + info->si_type = p.type; + info->space = space; info->addr = base_addr; info->slave_addr = slave_addr; info->next = ipmi_dmi_infos; ipmi_dmi_infos = info; } - pdev = platform_device_alloc(name, ipmi_dmi_nr); - if (!pdev) { - pr_err("Error allocation IPMI platform device\n"); - return; - } - - if (type == IPMI_DMI_TYPE_SSIF) { - p[pidx++] = PROPERTY_ENTRY_U16("i2c-addr", base_addr); - goto add_properties; - } - - memset(r, 0, sizeof(r)); - - r[0].start = base_addr; - r[0].end = r[0].start + offset - 1; - r[0].name = "IPMI Address 1"; - r[0].flags = flags; - - if (size > 1) { - r[1].start = r[0].start + offset; - r[1].end = r[1].start + offset - 1; - r[1].name = "IPMI Address 2"; - r[1].flags = flags; - num_r++; - } - - if (size > 2) { - r[2].start = r[1].start + offset; - r[2].end = r[2].start + offset - 1; - r[2].name = "IPMI Address 3"; - r[2].flags = flags; - num_r++; - } - - if (irq) { - r[num_r].start = irq; - r[num_r].end = irq; - r[num_r].name = "IPMI IRQ"; - r[num_r].flags = IORESOURCE_IRQ; - num_r++; - } - - rv = platform_device_add_resources(pdev, r, num_r); - if (rv) { - dev_err(&pdev->dev, "Unable to add resources: %d\n", rv); - goto err; - } - -add_properties: - rv = platform_device_add_properties(pdev, p); - if (rv) { - dev_err(&pdev->dev, "Unable to add properties: %d\n", rv); - goto err; - } - - rv = platform_device_add(pdev); - if (rv) { - dev_err(&pdev->dev, "Unable to add device: %d\n", rv); - goto err; - } - - ipmi_dmi_nr++; - return; - -err: - platform_device_put(pdev); + if (ipmi_platform_add(name, ipmi_dmi_nr, &p)) + ipmi_dmi_nr++; } /* @@ -169,14 +97,14 @@ err: * This function allows an ACPI-specified IPMI device to look up the * slave address from the DMI table. */ -int ipmi_dmi_get_slave_addr(enum si_type si_type, u32 flags, +int ipmi_dmi_get_slave_addr(enum si_type si_type, unsigned int space, unsigned long base_addr) { struct ipmi_dmi_info *info = ipmi_dmi_infos; while (info) { if (info->si_type == si_type && - info->flags == flags && + info->space == space && info->addr == base_addr) return info->slave_addr; info = info->next; @@ -197,13 +125,13 @@ EXPORT_SYMBOL(ipmi_dmi_get_slave_addr); static void __init dmi_decode_ipmi(const struct dmi_header *dm) { - const u8 *data = (const u8 *) dm; - u32 flags = IORESOURCE_IO; - unsigned long base_addr; - u8 len = dm->length; - u8 slave_addr; - int irq = 0, offset; - int type; + const u8 *data = (const u8 *) dm; + int space = IPMI_IO_ADDR_SPACE; + unsigned long base_addr; + u8 len = dm->length; + u8 slave_addr; + int irq = 0, offset = 0; + int type; if (len < DMI_IPMI_MIN_LENGTH) return; @@ -218,8 +146,7 @@ static void __init dmi_decode_ipmi(const struct dmi_header *dm) } if (len >= DMI_IPMI_VER2_LENGTH) { if (type == IPMI_DMI_TYPE_SSIF) { - offset = 0; - flags = 0; + space = 0; /* Match I2C interface 0. */ base_addr = data[DMI_IPMI_ADDR] >> 1; if (base_addr == 0) { /* @@ -236,7 +163,7 @@ static void __init dmi_decode_ipmi(const struct dmi_header *dm) base_addr &= DMI_IPMI_IO_MASK; } else { /* Memory */ - flags = IORESOURCE_MEM; + space = IPMI_MEM_ADDR_SPACE; } /* @@ -280,7 +207,7 @@ static void __init dmi_decode_ipmi(const struct dmi_header *dm) offset = 1; } - dmi_add_platform_ipmi(base_addr, flags, slave_addr, irq, + dmi_add_platform_ipmi(base_addr, space, slave_addr, irq, offset, type); } diff --git a/drivers/char/ipmi/ipmi_dmi.h b/drivers/char/ipmi/ipmi_dmi.h index 8d2b094db8e6..2dbec0461d0c 100644 --- a/drivers/char/ipmi/ipmi_dmi.h +++ b/drivers/char/ipmi/ipmi_dmi.h @@ -4,6 +4,6 @@ */ #ifdef CONFIG_IPMI_DMI_DECODE -int ipmi_dmi_get_slave_addr(enum si_type si_type, u32 flags, +int ipmi_dmi_get_slave_addr(enum si_type si_type, unsigned int space, unsigned long base_addr); #endif diff --git a/drivers/char/ipmi/ipmi_plat_data.c b/drivers/char/ipmi/ipmi_plat_data.c new file mode 100644 index 000000000000..8f0ca2a848eb --- /dev/null +++ b/drivers/char/ipmi/ipmi_plat_data.c @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/* + * Add an IPMI platform device. + */ + +#include +#include "ipmi_plat_data.h" +#include "ipmi_si.h" + +struct platform_device *ipmi_platform_add(const char *name, unsigned int inst, + struct ipmi_plat_data *p) +{ + struct platform_device *pdev; + unsigned int num_r = 1, size, pidx = 0; + struct resource r[4]; + struct property_entry pr[6]; + u32 flags; + int rv; + + memset(pr, 0, sizeof(pr)); + memset(r, 0, sizeof(r)); + + if (p->type == SI_BT) + size = 3; + else if (p->type == SI_TYPE_INVALID) + size = 0; + else + size = 2; + + if (p->regsize == 0) + p->regsize = DEFAULT_REGSIZE; + if (p->regspacing == 0) + p->regspacing = p->regsize; + + pr[pidx++] = PROPERTY_ENTRY_U8("ipmi-type", p->type); + if (p->slave_addr) + pr[pidx++] = PROPERTY_ENTRY_U8("slave-addr", p->slave_addr); + pr[pidx++] = PROPERTY_ENTRY_U8("addr-source", p->addr_source); + if (p->regshift) + pr[pidx++] = PROPERTY_ENTRY_U8("reg-shift", p->regshift); + pr[pidx++] = PROPERTY_ENTRY_U8("reg-size", p->regsize); + /* Last entry must be left NULL to terminate it. */ + + pdev = platform_device_alloc(name, inst); + if (!pdev) { + pr_err("Error allocating IPMI platform device %s.%d\n", + name, inst); + return NULL; + } + + if (size == 0) + /* An invalid or SSIF interface, no resources. */ + goto add_properties; + + /* + * Register spacing is derived from the resources in + * the IPMI platform code. + */ + + if (p->space == IPMI_IO_ADDR_SPACE) + flags = IORESOURCE_IO; + else + flags = IORESOURCE_MEM; + + r[0].start = p->addr; + r[0].end = r[0].start + p->regsize - 1; + r[0].name = "IPMI Address 1"; + r[0].flags = flags; + + if (size > 1) { + r[1].start = r[0].start + p->regspacing; + r[1].end = r[1].start + p->regsize - 1; + r[1].name = "IPMI Address 2"; + r[1].flags = flags; + num_r++; + } + + if (size > 2) { + r[2].start = r[1].start + p->regspacing; + r[2].end = r[2].start + p->regsize - 1; + r[2].name = "IPMI Address 3"; + r[2].flags = flags; + num_r++; + } + + if (p->irq) { + r[num_r].start = p->irq; + r[num_r].end = p->irq; + r[num_r].name = "IPMI IRQ"; + r[num_r].flags = IORESOURCE_IRQ; + num_r++; + } + + rv = platform_device_add_resources(pdev, r, num_r); + if (rv) { + dev_err(&pdev->dev, + "Unable to add hard-code resources: %d\n", rv); + goto err; + } + add_properties: + rv = platform_device_add_properties(pdev, pr); + if (rv) { + dev_err(&pdev->dev, + "Unable to add hard-code properties: %d\n", rv); + goto err; + } + + rv = platform_device_add(pdev); + if (rv) { + dev_err(&pdev->dev, + "Unable to add hard-code device: %d\n", rv); + goto err; + } + return pdev; + +err: + platform_device_put(pdev); + return NULL; +} +EXPORT_SYMBOL(ipmi_platform_add); diff --git a/drivers/char/ipmi/ipmi_plat_data.h b/drivers/char/ipmi/ipmi_plat_data.h new file mode 100644 index 000000000000..567cfcec8ada --- /dev/null +++ b/drivers/char/ipmi/ipmi_plat_data.h @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ + +/* + * Generic code to add IPMI platform devices. + */ + +#include + +struct ipmi_plat_data { + unsigned int type; /* si_type for si, SI_INVALID for others */ + unsigned int space; /* addr_space for si, intf# for ssif. */ + unsigned long addr; + unsigned int regspacing; + unsigned int regsize; + unsigned int regshift; + unsigned int irq; + unsigned int slave_addr; + enum ipmi_addr_src addr_source; +}; + +struct platform_device *ipmi_platform_add(const char *name, unsigned int inst, + struct ipmi_plat_data *p); diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h index 3efc8a71aab4..49b211bf9ad7 100644 --- a/drivers/char/ipmi/ipmi_si.h +++ b/drivers/char/ipmi/ipmi_si.h @@ -7,6 +7,7 @@ */ #include +#include #include "ipmi_si_sm.h" #define DEFAULT_REGSPACING 1 diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c b/drivers/char/ipmi/ipmi_si_hardcode.c index 86ac9b8a3219..cb58298d80f5 100644 --- a/drivers/char/ipmi/ipmi_si_hardcode.c +++ b/drivers/char/ipmi/ipmi_si_hardcode.c @@ -5,6 +5,7 @@ #include #include #include "ipmi_si.h" +#include "ipmi_plat_data.h" /* * There can be 4 IO ports passed in (with or without IRQs), 4 addresses, @@ -78,121 +79,39 @@ static struct platform_device *ipmi_hc_pdevs[SI_MAX_PARMS]; static void __init ipmi_hardcode_init_one(const char *si_type_str, unsigned int i, unsigned long addr, - unsigned int flags) + enum ipmi_addr_space addr_space) { - struct platform_device *pdev; - unsigned int num_r = 1, size; - struct resource r[4]; - struct property_entry p[6]; - enum si_type si_type; - unsigned int regspacing, regsize; - int rv; + struct ipmi_plat_data p; - memset(p, 0, sizeof(p)); - memset(r, 0, sizeof(r)); + memset(&p, 0, sizeof(p)); if (!si_type_str || !*si_type_str || strcmp(si_type_str, "kcs") == 0) { - size = 2; - si_type = SI_KCS; + p.type = SI_KCS; } else if (strcmp(si_type_str, "smic") == 0) { - size = 2; - si_type = SI_SMIC; + p.type = SI_SMIC; } else if (strcmp(si_type_str, "bt") == 0) { - size = 3; - si_type = SI_BT; + p.type = SI_BT; } else if (strcmp(si_type_str, "invalid") == 0) { /* * Allow a firmware-specified interface to be * disabled. */ - size = 1; - si_type = SI_TYPE_INVALID; + p.type = SI_TYPE_INVALID; } else { pr_warn("Interface type specified for interface %d, was invalid: %s\n", i, si_type_str); return; } - regsize = regsizes[i]; - if (regsize == 0) - regsize = DEFAULT_REGSIZE; + p.regsize = regsizes[i]; + p.slave_addr = slave_addrs[i]; + p.addr_source = SI_HARDCODED; + p.regshift = regshifts[i]; + p.regsize = regsizes[i]; + p.addr = addr; + p.space = addr_space; - p[0] = PROPERTY_ENTRY_U8("ipmi-type", si_type); - p[1] = PROPERTY_ENTRY_U8("slave-addr", slave_addrs[i]); - p[2] = PROPERTY_ENTRY_U8("addr-source", SI_HARDCODED); - p[3] = PROPERTY_ENTRY_U8("reg-shift", regshifts[i]); - p[4] = PROPERTY_ENTRY_U8("reg-size", regsize); - /* Last entry must be left NULL to terminate it. */ - - /* - * Register spacing is derived from the resources in - * the IPMI platform code. - */ - regspacing = regspacings[i]; - if (regspacing == 0) - regspacing = regsize; - - r[0].start = addr; - r[0].end = r[0].start + regsize - 1; - r[0].name = "IPMI Address 1"; - r[0].flags = flags; - - if (size > 1) { - r[1].start = r[0].start + regspacing; - r[1].end = r[1].start + regsize - 1; - r[1].name = "IPMI Address 2"; - r[1].flags = flags; - num_r++; - } - - if (size > 2) { - r[2].start = r[1].start + regspacing; - r[2].end = r[2].start + regsize - 1; - r[2].name = "IPMI Address 3"; - r[2].flags = flags; - num_r++; - } - - if (irqs[i]) { - r[num_r].start = irqs[i]; - r[num_r].end = irqs[i]; - r[num_r].name = "IPMI IRQ"; - r[num_r].flags = IORESOURCE_IRQ; - num_r++; - } - - pdev = platform_device_alloc("hardcode-ipmi-si", i); - if (!pdev) { - pr_err("Error allocating IPMI platform device %d\n", i); - return; - } - - rv = platform_device_add_resources(pdev, r, num_r); - if (rv) { - dev_err(&pdev->dev, - "Unable to add hard-code resources: %d\n", rv); - goto err; - } - - rv = platform_device_add_properties(pdev, p); - if (rv) { - dev_err(&pdev->dev, - "Unable to add hard-code properties: %d\n", rv); - goto err; - } - - rv = platform_device_add(pdev); - if (rv) { - dev_err(&pdev->dev, - "Unable to add hard-code device: %d\n", rv); - goto err; - } - - ipmi_hc_pdevs[i] = pdev; - return; - -err: - platform_device_put(pdev); + ipmi_hc_pdevs[i] = ipmi_platform_add("hardcode-ipmi-si", i, &p); } void __init ipmi_hardcode_init(void) @@ -219,10 +138,10 @@ void __init ipmi_hardcode_init(void) for (i = 0; i < SI_MAX_PARMS; i++) { if (i < num_ports && ports[i]) ipmi_hardcode_init_one(si_type[i], i, ports[i], - IORESOURCE_IO); + IPMI_IO_ADDR_SPACE); if (i < num_addrs && addrs[i]) ipmi_hardcode_init_one(si_type[i], i, addrs[i], - IORESOURCE_MEM); + IPMI_MEM_ADDR_SPACE); } } diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c index f690e9edb08c..f2b5ac0986f2 100644 --- a/drivers/char/ipmi/ipmi_si_platform.c +++ b/drivers/char/ipmi/ipmi_si_platform.c @@ -307,15 +307,10 @@ static int of_ipmi_probe(struct platform_device *dev) static int find_slave_address(struct si_sm_io *io, int slave_addr) { #ifdef CONFIG_IPMI_DMI_DECODE - if (!slave_addr) { - u32 flags = IORESOURCE_IO; - - if (io->addr_space == IPMI_MEM_ADDR_SPACE) - flags = IORESOURCE_MEM; - - slave_addr = ipmi_dmi_get_slave_addr(io->si_type, flags, + if (!slave_addr) + slave_addr = ipmi_dmi_get_slave_addr(io->si_type, + io->addr_space, io->addr_data); - } #endif return slave_addr; -- cgit v1.2.3 From 3bb8ea400cbe2ae4d5b51e4306ff9d14d6c6627c Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 21 Feb 2019 14:23:07 -0600 Subject: ipmi_si: Switch hotmod to use a platform device Don't force the main code to create one for it. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_hotmod.c | 208 ++++++++++++++++------------------- drivers/char/ipmi/ipmi_si_intf.c | 2 +- drivers/char/ipmi/ipmi_si_platform.c | 1 + 3 files changed, 98 insertions(+), 113 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_si_hotmod.c b/drivers/char/ipmi/ipmi_si_hotmod.c index f3d455bbf3cc..f0728be00bd2 100644 --- a/drivers/char/ipmi/ipmi_si_hotmod.c +++ b/drivers/char/ipmi/ipmi_si_hotmod.c @@ -10,7 +10,9 @@ #include #include +#include #include "ipmi_si.h" +#include "ipmi_plat_data.h" static int hotmod_handler(const char *val, const struct kernel_param *kp); @@ -54,8 +56,8 @@ static const struct hotmod_vals hotmod_as[] = { { NULL } }; -static int parse_str(const struct hotmod_vals *v, int *val, char *name, - char **curr) +static int parse_str(const struct hotmod_vals *v, unsigned int *val, char *name, + const char **curr) { char *s; int i; @@ -80,7 +82,7 @@ static int parse_str(const struct hotmod_vals *v, int *val, char *name, } static int check_hotmod_int_op(const char *curr, const char *option, - const char *name, int *val) + const char *name, unsigned int *val) { char *n; @@ -99,22 +101,93 @@ static int check_hotmod_int_op(const char *curr, const char *option, return 0; } +static int parse_hotmod_str(const char *curr, enum hotmod_op *op, + struct ipmi_plat_data *h) +{ + char *s, *o; + int rv; + unsigned int ival; + + rv = parse_str(hotmod_ops, &ival, "operation", &curr); + if (rv) + return rv; + *op = ival; + + rv = parse_str(hotmod_si, &ival, "interface type", &curr); + if (rv) + return rv; + h->type = ival; + + rv = parse_str(hotmod_as, &ival, "address space", &curr); + if (rv) + return rv; + h->space = ival; + + s = strchr(curr, ','); + if (s) { + *s = '\0'; + s++; + } + rv = kstrtoul(curr, 0, &h->addr); + if (rv) { + pr_warn("Invalid hotmod address '%s': %d\n", curr, rv); + return rv; + } + + while (s) { + curr = s; + s = strchr(curr, ','); + if (s) { + *s = '\0'; + s++; + } + o = strchr(curr, '='); + if (o) { + *o = '\0'; + o++; + } + rv = check_hotmod_int_op(curr, o, "rsp", &h->regspacing); + if (rv < 0) + return rv; + else if (rv) + continue; + rv = check_hotmod_int_op(curr, o, "rsi", &h->regsize); + if (rv < 0) + return rv; + else if (rv) + continue; + rv = check_hotmod_int_op(curr, o, "rsh", &h->regshift); + if (rv < 0) + return rv; + else if (rv) + continue; + rv = check_hotmod_int_op(curr, o, "irq", &h->irq); + if (rv < 0) + return rv; + else if (rv) + continue; + rv = check_hotmod_int_op(curr, o, "ipmb", &h->slave_addr); + if (rv < 0) + return rv; + else if (rv) + continue; + + pr_warn("Invalid hotmod option '%s'\n", curr); + return -EINVAL; + } + + h->addr_source = SI_HOTMOD; + return 0; +} + +static atomic_t hotmod_nr; + static int hotmod_handler(const char *val, const struct kernel_param *kp) { - char *str = kstrdup(val, GFP_KERNEL); + char *str = kstrdup(val, GFP_KERNEL), *curr, *next; int rv; - char *next, *curr, *s, *n, *o; - enum hotmod_op op; - enum si_type si_type; - enum ipmi_addr_space addr_space; - unsigned long addr; - int regspacing; - int regsize; - int regshift; - int irq; - int ipmb; - int ival; - int len; + struct ipmi_plat_data h; + unsigned int len, ival; if (!str) return -ENOMEM; @@ -128,11 +201,7 @@ static int hotmod_handler(const char *val, const struct kernel_param *kp) } for (curr = str; curr; curr = next) { - regspacing = 1; - regsize = 1; - regshift = 0; - irq = 0; - ipmb = 0; /* Choose the default if not specified */ + enum hotmod_op op; next = strchr(curr, ':'); if (next) { @@ -140,102 +209,17 @@ static int hotmod_handler(const char *val, const struct kernel_param *kp) next++; } - rv = parse_str(hotmod_ops, &ival, "operation", &curr); - if (rv) - break; - op = ival; - - rv = parse_str(hotmod_si, &ival, "interface type", &curr); + memset(&h, 0, sizeof(h)); + rv = parse_hotmod_str(curr, &op, &h); if (rv) - break; - si_type = ival; - - rv = parse_str(hotmod_as, &ival, "address space", &curr); - if (rv) - break; - addr_space = ival; - - s = strchr(curr, ','); - if (s) { - *s = '\0'; - s++; - } - addr = simple_strtoul(curr, &n, 0); - if ((*n != '\0') || (*curr == '\0')) { - pr_warn("Invalid hotmod address '%s'\n", curr); - break; - } - - while (s) { - curr = s; - s = strchr(curr, ','); - if (s) { - *s = '\0'; - s++; - } - o = strchr(curr, '='); - if (o) { - *o = '\0'; - o++; - } - rv = check_hotmod_int_op(curr, o, "rsp", ®spacing); - if (rv < 0) - goto out; - else if (rv) - continue; - rv = check_hotmod_int_op(curr, o, "rsi", ®size); - if (rv < 0) - goto out; - else if (rv) - continue; - rv = check_hotmod_int_op(curr, o, "rsh", ®shift); - if (rv < 0) - goto out; - else if (rv) - continue; - rv = check_hotmod_int_op(curr, o, "irq", &irq); - if (rv < 0) - goto out; - else if (rv) - continue; - rv = check_hotmod_int_op(curr, o, "ipmb", &ipmb); - if (rv < 0) - goto out; - else if (rv) - continue; - - rv = -EINVAL; - pr_warn("Invalid hotmod option '%s'\n", curr); goto out; - } if (op == HM_ADD) { - struct si_sm_io io; - - memset(&io, 0, sizeof(io)); - io.addr_source = SI_HOTMOD; - io.si_type = si_type; - io.addr_data = addr; - io.addr_space = addr_space; - - io.addr = NULL; - io.regspacing = regspacing; - if (!io.regspacing) - io.regspacing = DEFAULT_REGSPACING; - io.regsize = regsize; - if (!io.regsize) - io.regsize = DEFAULT_REGSIZE; - io.regshift = regshift; - io.irq = irq; - if (io.irq) - io.irq_setup = ipmi_std_irq_setup; - io.slave_addr = ipmb; - - rv = ipmi_si_add_smi(&io); - if (rv) - goto out; + ipmi_platform_add("hotmod-ipmi-si", + atomic_inc_return(&hotmod_nr), + &h); } else { - ipmi_si_remove_by_data(addr_space, si_type, addr); + ipmi_si_remove_by_data(h.space, h.type, h.addr); } } rv = len; diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 54e3b4f2c024..c8af06dfa244 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1870,7 +1870,7 @@ int ipmi_si_add_smi(struct si_sm_io *io) * address, they presumably want us to use it and not what is * in the firmware. */ - if (io->addr_source != SI_HARDCODED && + if (io->addr_source != SI_HARDCODED && io->addr_source != SI_HOTMOD && ipmi_si_hardcode_match(io->addr_space, io->addr_data)) { dev_info(io->dev, "Hard-coded device at this address already exists"); diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c index f2b5ac0986f2..0f493fe83701 100644 --- a/drivers/char/ipmi/ipmi_si_platform.c +++ b/drivers/char/ipmi/ipmi_si_platform.c @@ -431,6 +431,7 @@ static int ipmi_remove(struct platform_device *pdev) static const struct platform_device_id si_plat_ids[] = { { "dmi-ipmi-si", 0 }, { "hardcode-ipmi-si", 0 }, + { "hotmod-ipmi-si", 0 }, { } }; -- cgit v1.2.3 From 1a84df2df8ebb1083cb57be6808fbf36d9cabe0e Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 21 Feb 2019 17:04:17 -0600 Subject: ipmi_si: Remove hardcode IPMI devices by scanning the platform bus Instead of keeping track of each one, just scan the platform bus for hardcode devices and remove them. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_hardcode.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c b/drivers/char/ipmi/ipmi_si_hardcode.c index cb58298d80f5..540b0871bff0 100644 --- a/drivers/char/ipmi/ipmi_si_hardcode.c +++ b/drivers/char/ipmi/ipmi_si_hardcode.c @@ -74,8 +74,6 @@ MODULE_PARM_DESC(slave_addrs, "Set the default IPMB slave address for" " overridden by this parm. This is an array indexed" " by interface number."); -static struct platform_device *ipmi_hc_pdevs[SI_MAX_PARMS]; - static void __init ipmi_hardcode_init_one(const char *si_type_str, unsigned int i, unsigned long addr, @@ -111,7 +109,7 @@ static void __init ipmi_hardcode_init_one(const char *si_type_str, p.addr = addr; p.space = addr_space; - ipmi_hc_pdevs[i] = ipmi_platform_add("hardcode-ipmi-si", i, &p); + ipmi_platform_add("hardcode-ipmi-si", i, &p); } void __init ipmi_hardcode_init(void) @@ -145,13 +143,23 @@ void __init ipmi_hardcode_init(void) } } + +static int pdev_match_name(struct device *dev, void *data) +{ + struct platform_device *pdev = to_platform_device(dev); + + return strcmp(pdev->name, "hardcode-ipmi-si") == 0; +} + void ipmi_si_hardcode_exit(void) { - unsigned int i; + struct device *dev; - for (i = 0; i < SI_MAX_PARMS; i++) { - if (ipmi_hc_pdevs[i]) - platform_device_unregister(ipmi_hc_pdevs[i]); + while ((dev = bus_find_device(&platform_bus_type, NULL, NULL, + pdev_match_name))) { + struct platform_device *pdev = to_platform_device(dev); + + platform_device_unregister(pdev); } } -- cgit v1.2.3 From bdb57b7bc16252599cbcb826dfdf7e394dd2af4b Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 21 Feb 2019 17:21:50 -0600 Subject: ipmi_si: Remove hotmod devices on removal and exit When a hotmod-added device is removed or when the module is removed, remove the platform devices that was created for it. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si.h | 5 +++-- drivers/char/ipmi/ipmi_si_hotmod.c | 32 +++++++++++++++++++++++++++++++- drivers/char/ipmi/ipmi_si_intf.c | 12 +++++++++--- 3 files changed, 43 insertions(+), 6 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h index 49b211bf9ad7..2ae9ebdfab0d 100644 --- a/drivers/char/ipmi/ipmi_si.h +++ b/drivers/char/ipmi/ipmi_si.h @@ -21,10 +21,11 @@ void ipmi_irq_start_cleanup(struct si_sm_io *io); int ipmi_std_irq_setup(struct si_sm_io *io); void ipmi_irq_finish_setup(struct si_sm_io *io); int ipmi_si_remove_by_dev(struct device *dev); -void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, - unsigned long addr); +struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type, + unsigned long addr); void ipmi_hardcode_init(void); void ipmi_si_hardcode_exit(void); +void ipmi_si_hotmod_exit(void); int ipmi_si_hardcode_match(int addr_space, unsigned long addr); void ipmi_si_platform_init(void); void ipmi_si_platform_shutdown(void); diff --git a/drivers/char/ipmi/ipmi_si_hotmod.c b/drivers/char/ipmi/ipmi_si_hotmod.c index f0728be00bd2..230b10e7d288 100644 --- a/drivers/char/ipmi/ipmi_si_hotmod.c +++ b/drivers/char/ipmi/ipmi_si_hotmod.c @@ -219,7 +219,18 @@ static int hotmod_handler(const char *val, const struct kernel_param *kp) atomic_inc_return(&hotmod_nr), &h); } else { - ipmi_si_remove_by_data(h.space, h.type, h.addr); + struct device *dev; + + dev = ipmi_si_remove_by_data(h.space, h.type, h.addr); + if (dev && dev_is_platform(dev)) { + struct platform_device *pdev; + + pdev = to_platform_device(dev); + if (strcmp(pdev->name, "hotmod-ipmi-si") == 0) + platform_device_unregister(pdev); + } + if (dev) + put_device(dev); } } rv = len; @@ -227,3 +238,22 @@ out: kfree(str); return rv; } + +static int pdev_match_name(struct device *dev, void *data) +{ + struct platform_device *pdev = to_platform_device(dev); + + return strcmp(pdev->name, "hotmod-ipmi-si") == 0; +} + +void ipmi_si_hotmod_exit(void) +{ + struct device *dev; + + while ((dev = bus_find_device(&platform_bus_type, NULL, NULL, + pdev_match_name))) { + struct platform_device *pdev = to_platform_device(dev); + + platform_device_unregister(pdev); + } +} diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index c8af06dfa244..9b3a272af7dd 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -2281,11 +2281,12 @@ int ipmi_si_remove_by_dev(struct device *dev) return rv; } -void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, - unsigned long addr) +struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type, + unsigned long addr) { /* remove */ struct smi_info *e, *tmp_e; + struct device *dev = NULL; mutex_lock(&smi_infos_lock); list_for_each_entry_safe(e, tmp_e, &smi_infos, link) { @@ -2293,10 +2294,14 @@ void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, continue; if (e->io.si_type != si_type) continue; - if (e->io.addr_data == addr) + if (e->io.addr_data == addr) { + dev = get_device(e->io.dev); cleanup_one_si(e); + } } mutex_unlock(&smi_infos_lock); + + return dev; } static void cleanup_ipmi_si(void) @@ -2318,6 +2323,7 @@ static void cleanup_ipmi_si(void) mutex_unlock(&smi_infos_lock); ipmi_si_hardcode_exit(); + ipmi_si_hotmod_exit(); } module_exit(cleanup_ipmi_si); -- cgit v1.2.3 From e17c657100d592115fcfad27472cd611fd74b861 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 21 Feb 2019 17:41:47 -0600 Subject: ipmi_si: Consolidate scanning the platform bus The same basic code was in two places. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si.h | 1 + drivers/char/ipmi/ipmi_si_hardcode.c | 16 +--------------- drivers/char/ipmi/ipmi_si_hotmod.c | 16 +--------------- drivers/char/ipmi/ipmi_si_platform.c | 20 ++++++++++++++++++++ 4 files changed, 23 insertions(+), 30 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h index 2ae9ebdfab0d..357a229c9012 100644 --- a/drivers/char/ipmi/ipmi_si.h +++ b/drivers/char/ipmi/ipmi_si.h @@ -29,6 +29,7 @@ void ipmi_si_hotmod_exit(void); int ipmi_si_hardcode_match(int addr_space, unsigned long addr); void ipmi_si_platform_init(void); void ipmi_si_platform_shutdown(void); +void ipmi_remove_platform_device_by_name(char *name); extern struct platform_driver ipmi_platform_driver; diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c b/drivers/char/ipmi/ipmi_si_hardcode.c index 540b0871bff0..01946cad3d13 100644 --- a/drivers/char/ipmi/ipmi_si_hardcode.c +++ b/drivers/char/ipmi/ipmi_si_hardcode.c @@ -144,23 +144,9 @@ void __init ipmi_hardcode_init(void) } -static int pdev_match_name(struct device *dev, void *data) -{ - struct platform_device *pdev = to_platform_device(dev); - - return strcmp(pdev->name, "hardcode-ipmi-si") == 0; -} - void ipmi_si_hardcode_exit(void) { - struct device *dev; - - while ((dev = bus_find_device(&platform_bus_type, NULL, NULL, - pdev_match_name))) { - struct platform_device *pdev = to_platform_device(dev); - - platform_device_unregister(pdev); - } + ipmi_remove_platform_device_by_name("hardcode-ipmi-si"); } /* diff --git a/drivers/char/ipmi/ipmi_si_hotmod.c b/drivers/char/ipmi/ipmi_si_hotmod.c index 230b10e7d288..1433055a9705 100644 --- a/drivers/char/ipmi/ipmi_si_hotmod.c +++ b/drivers/char/ipmi/ipmi_si_hotmod.c @@ -239,21 +239,7 @@ out: return rv; } -static int pdev_match_name(struct device *dev, void *data) -{ - struct platform_device *pdev = to_platform_device(dev); - - return strcmp(pdev->name, "hotmod-ipmi-si") == 0; -} - void ipmi_si_hotmod_exit(void) { - struct device *dev; - - while ((dev = bus_find_device(&platform_bus_type, NULL, NULL, - pdev_match_name))) { - struct platform_device *pdev = to_platform_device(dev); - - platform_device_unregister(pdev); - } + ipmi_remove_platform_device_by_name("hotmod-ipmi-si"); } diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c index 0f493fe83701..54c7ded2a1ff 100644 --- a/drivers/char/ipmi/ipmi_si_platform.c +++ b/drivers/char/ipmi/ipmi_si_platform.c @@ -428,6 +428,26 @@ static int ipmi_remove(struct platform_device *pdev) return ipmi_si_remove_by_dev(&pdev->dev); } +static int pdev_match_name(struct device *dev, void *data) +{ + struct platform_device *pdev = to_platform_device(dev); + const char *name = data; + + return strcmp(pdev->name, name) == 0; +} + +void ipmi_remove_platform_device_by_name(char *name) +{ + struct device *dev; + + while ((dev = bus_find_device(&platform_bus_type, NULL, name, + pdev_match_name))) { + struct platform_device *pdev = to_platform_device(dev); + + platform_device_unregister(pdev); + } +} + static const struct platform_device_id si_plat_ids[] = { { "dmi-ipmi-si", 0 }, { "hardcode-ipmi-si", 0 }, -- cgit v1.2.3 From 90b2d4f15ff737fc03432c44cbb741da32ce06c4 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 21 Feb 2019 12:27:26 -0600 Subject: ipmi_si: Remove hacks for adding a dummy platform devices All the IPMI address sources now supply a real device. This cheap hack is no longer necessary. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_intf.c | 45 +++------------------------------------- 1 file changed, 3 insertions(+), 42 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 9b3a272af7dd..b1732882b97e 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -229,15 +229,9 @@ struct smi_info { /* From the get device id response... */ struct ipmi_device_id device_id; - /* Default driver model device. */ - struct platform_device *pdev; - /* Have we added the device group to the device? */ bool dev_group_added; - /* Have we added the platform device? */ - bool pdev_registered; - /* Counters and things for the proc filesystem. */ atomic_t stats[SI_NUM_STATS]; @@ -1969,24 +1963,9 @@ static int try_smi_init(struct smi_info *new_smi) /* Do this early so it's available for logs. */ if (!new_smi->io.dev) { - init_name = kasprintf(GFP_KERNEL, "ipmi_si.%d", - 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->si_num); - if (!new_smi->pdev) { - pr_err("Unable to allocate platform device\n"); - rv = -ENOMEM; - goto out_err; - } - new_smi->io.dev = &new_smi->pdev->dev; - new_smi->io.dev->driver = &ipmi_platform_driver.driver; - /* Nulled by device_add() */ - new_smi->io.dev->init_name = init_name; + pr_err("IPMI interface added with no device\n"); + rv = EIO; + goto out_err; } /* Allocate the state machine's data and initialize it. */ @@ -2059,17 +2038,6 @@ static int try_smi_init(struct smi_info *new_smi) atomic_set(&new_smi->req_events, 1); } - if (new_smi->pdev && !new_smi->pdev_registered) { - rv = platform_device_add(new_smi->pdev); - if (rv) { - dev_err(new_smi->io.dev, - "Unable to register system interface device: %d\n", - rv); - goto out_err; - } - new_smi->pdev_registered = true; - } - dev_set_drvdata(new_smi->io.dev, new_smi); rv = device_add_group(new_smi->io.dev, &ipmi_si_dev_attr_group); if (rv) { @@ -2253,13 +2221,6 @@ static void cleanup_one_si(struct smi_info *smi_info) if (smi_info->intf) ipmi_unregister_smi(smi_info->intf); - if (smi_info->pdev) { - if (smi_info->pdev_registered) - platform_device_unregister(smi_info->pdev); - else - platform_device_put(smi_info->pdev); - } - kfree(smi_info); } -- cgit v1.2.3 From 038903593317691cf5f87ba1286627f5e8ab5660 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 22 Feb 2019 22:55:30 +0300 Subject: ipmi_si: Potential array underflow in hotmod_handler() The "ival" variable needs to signed so that we don't read before the start of the str[] array. This would only happen the user passed in a module parameter that was just comprised of space characters. Fixes: e80444ae4fc3 ("ipmi_si: Switch hotmod to use a platform device") Signed-off-by: Dan Carpenter Message-Id: <20190222195530.GA306@kadam> Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_si_hotmod.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_si_hotmod.c b/drivers/char/ipmi/ipmi_si_hotmod.c index 1433055a9705..03140f6cdf6f 100644 --- a/drivers/char/ipmi/ipmi_si_hotmod.c +++ b/drivers/char/ipmi/ipmi_si_hotmod.c @@ -187,7 +187,8 @@ static int hotmod_handler(const char *val, const struct kernel_param *kp) char *str = kstrdup(val, GFP_KERNEL), *curr, *next; int rv; struct ipmi_plat_data h; - unsigned int len, ival; + unsigned int len; + int ival; if (!str) return -ENOMEM; -- cgit v1.2.3