From 122d16da1313f1746a4cdd31a620bbb141be7060 Mon Sep 17 00:00:00 2001 From: Marco Crivellari Date: Wed, 24 Dec 2025 17:13:01 +0100 Subject: ipmi: Replace use of system_wq with system_percpu_wq This patch continues the effort to refactor workqueue APIs, which has begun with the changes introducing new workqueues and a new alloc_workqueue flag: commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq") commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag") The point of the refactoring is to eventually alter the default behavior of workqueues to become unbound by default so that their workload placement is optimized by the scheduler. Before that to happen after a careful review and conversion of each individual case, workqueue users must be converted to the better named new workqueues with no intended behaviour changes: system_wq -> system_percpu_wq system_unbound_wq -> system_dfl_wq This way the old obsolete workqueues (system_wq, system_unbound_wq) can be removed in the future. Suggested-by: Tejun Heo Signed-off-by: Marco Crivellari Message-ID: <20251224161301.135382-1-marco.crivellari@suse.com> Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index c41f51c82edd..869ac87a4b6a 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -987,7 +987,7 @@ static int deliver_response(struct ipmi_smi *intf, struct ipmi_recv_msg *msg) mutex_lock(&intf->user_msgs_mutex); list_add_tail(&msg->link, &intf->user_msgs); mutex_unlock(&intf->user_msgs_mutex); - queue_work(system_wq, &intf->smi_work); + queue_work(system_percpu_wq, &intf->smi_work); } return rv; @@ -4977,7 +4977,7 @@ void ipmi_smi_msg_received(struct ipmi_smi *intf, if (run_to_completion) smi_work(&intf->smi_work); else - queue_work(system_wq, &intf->smi_work); + queue_work(system_percpu_wq, &intf->smi_work); } EXPORT_SYMBOL(ipmi_smi_msg_received); @@ -4987,7 +4987,7 @@ void ipmi_smi_watchdog_pretimeout(struct ipmi_smi *intf) return; atomic_set(&intf->watchdog_pretimeouts_to_deliver, 1); - queue_work(system_wq, &intf->smi_work); + queue_work(system_percpu_wq, &intf->smi_work); } EXPORT_SYMBOL(ipmi_smi_watchdog_pretimeout); @@ -5162,7 +5162,7 @@ static bool ipmi_timeout_handler(struct ipmi_smi *intf, flags); } - queue_work(system_wq, &intf->smi_work); + queue_work(system_percpu_wq, &intf->smi_work); return need_timer; } @@ -5218,7 +5218,7 @@ static void ipmi_timeout(struct timer_list *unused) if (atomic_read(&stop_operation)) return; - queue_work(system_wq, &ipmi_timer_work); + queue_work(system_percpu_wq, &ipmi_timer_work); } static void need_waiter(struct ipmi_smi *intf) -- cgit v1.2.3 From 7fc3e2546cf3fa9a28a2acc92a512c779a8e5038 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Fri, 3 Apr 2026 17:05:58 +0800 Subject: ipmi: ssif_bmc: cancel response timer on remove The response timer can stay armed across device teardown. If it fires after remove, the callback dereferences the SSIF context and the i2c client after teardown has started. Cancel the timer in remove so the callback cannot run after the device is unregistered. Signed-off-by: Jian Zhang Message-ID: <20260403090603.3988423-1-zhangjian.3032@bytedance.com> Signed-off-by: Corey Minyard --- drivers/char/ipmi/ssif_bmc.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c index 7a52e3ea49ed..dc1d5bb4a460 100644 --- a/drivers/char/ipmi/ssif_bmc.c +++ b/drivers/char/ipmi/ssif_bmc.c @@ -843,6 +843,7 @@ static void ssif_bmc_remove(struct i2c_client *client) { struct ssif_bmc_ctx *ssif_bmc = i2c_get_clientdata(client); + timer_delete_sync(&ssif_bmc->response_timer); i2c_slave_unregister(client); misc_deregister(&ssif_bmc->miscdev); } -- cgit v1.2.3 From ea641be7a4faee4351f9c5ed6b188e1bbf5586a6 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Fri, 3 Apr 2026 17:05:59 +0800 Subject: ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure copy_to_user() returns the number of bytes that could not be copied, with a non-zero value indicating a partial or complete failure. The current code only checks for negative return values and treats all non-negative results as success. Treating any positive return value from copy_to_user() as an error and returning -EFAULT. Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver") Signed-off-by: Jian Zhang Message-ID: <20260403090603.3988423-2-zhangjian.3032@bytedance.com> Signed-off-by: Corey Minyard --- drivers/char/ipmi/ssif_bmc.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c index dc1d5bb4a460..fbc7d2cfd535 100644 --- a/drivers/char/ipmi/ssif_bmc.c +++ b/drivers/char/ipmi/ssif_bmc.c @@ -163,6 +163,8 @@ static ssize_t ssif_bmc_read(struct file *file, char __user *buf, size_t count, spin_unlock_irqrestore(&ssif_bmc->lock, flags); ret = copy_to_user(buf, &msg, count); + if (ret > 0) + ret = -EFAULT; } return (ret < 0) ? ret : count; -- cgit v1.2.3 From 1d38e849adb6851ee280aa1a1d687b2181549a66 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Fri, 3 Apr 2026 17:06:00 +0800 Subject: ipmi: ssif_bmc: fix message desynchronization after truncated response A truncated response, caused by host power-off, or other conditions, can lead to message desynchronization. Raw trace data (STOP loss scenario, add state transition comment): 1. T-1: Read response phase (SSIF_RES_SENDING) 8271.955342 WR_RCV [03] <- Read polling cmd 8271.955348 RD_REQ [04] <== SSIF_RES_SENDING <- start sending response 8271.955436 RD_PRO [b4] 8271.955527 RD_PRO [00] 8271.955618 RD_PRO [c1] 8271.955707 RD_PRO [00] 8271.955814 RD_PRO [ad] <== SSIF_RES_SENDING <- last byte <- !! STOP lost (truncated response) 2. T: New Write request arrives, BMC still in SSIF_RES_SENDING 8271.967973 WR_REQ [] <== SSIF_RES_SENDING >> SSIF_ABORTING <- log: unexpected WR_REQ in RES_SENDING 8271.968447 WR_RCV [02] <== SSIF_ABORTING <- do nothing 8271.968452 WR_RCV [02] <== SSIF_ABORTING <- do nothing 8271.968454 WR_RCV [18] <== SSIF_ABORTING <- do nothing 8271.968456 WR_RCV [01] <== SSIF_ABORTING <- do nothing 8271.968458 WR_RCV [66] <== SSIF_ABORTING <- do nothing 8271.978714 STOP [] <== SSIF_ABORTING >> SSIF_READY <- log: unexpected SLAVE STOP in state=SSIF_ABORTING 3. T+1: Next Read polling, treated as a fresh transaction 8271.979125 WR_REQ [] <== SSIF_READY >> SSIF_START 8271.979326 WR_RCV [03] <== SSIF_START >> SSIF_SMBUS_CMD <- smbus_cmd=0x03 8271.979331 RD_REQ [04] <== SSIF_RES_SENDING <- sending response 8271.979427 RD_PRO [b4] <- !! this is T's stale response -> desynchronization When in SSIF_ABORTING state, a newly arrived command should still be handled to avoid dropping the request or causing message desynchronization. Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver") Signed-off-by: Jian Zhang Message-ID: <20260403090603.3988423-3-zhangjian.3032@bytedance.com> Signed-off-by: Corey Minyard --- drivers/char/ipmi/ssif_bmc.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c index fbc7d2cfd535..a1e1a7f1e104 100644 --- a/drivers/char/ipmi/ssif_bmc.c +++ b/drivers/char/ipmi/ssif_bmc.c @@ -458,6 +458,15 @@ static bool supported_write_cmd(u8 cmd) return false; } +static bool supported_write_start_cmd(u8 cmd) +{ + if (cmd == SSIF_IPMI_SINGLEPART_WRITE || + cmd == SSIF_IPMI_MULTIPART_WRITE_START) + return true; + + return false; +} + /* Process the IPMI response that will be read by master */ static void handle_read_processed(struct ssif_bmc_ctx *ssif_bmc, u8 *val) { @@ -709,6 +718,11 @@ static void on_write_received_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val) ssif_bmc->state = SSIF_ABORTING; else ssif_bmc->state = SSIF_REQ_RECVING; + } else if (ssif_bmc->state == SSIF_ABORTING) { + if (supported_write_start_cmd(*val)) { + ssif_bmc->state = SSIF_SMBUS_CMD; + ssif_bmc->aborting = false; + } } /* This is response sending state */ -- cgit v1.2.3 From c9c99b7b7051eb7121b3224bfce181fb023b0269 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Fri, 3 Apr 2026 17:06:01 +0800 Subject: ipmi: ssif_bmc: change log level to dbg in irq callback Long-running tests indicate that this logging can occasionally disrupt timing and lead to request/response corruption. Irq handler need to be executed as fast as possible, most I2C slave IRQ implementations are byte-level, logging here can significantly affect transfer behavior and timing. It is recommended to use dev_dbg() for these messages. Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver") Signed-off-by: Jian Zhang Message-ID: <20260403090603.3988423-4-zhangjian.3032@bytedance.com> Signed-off-by: Corey Minyard --- drivers/char/ipmi/ssif_bmc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c index a1e1a7f1e104..646a1e9ffbb7 100644 --- a/drivers/char/ipmi/ssif_bmc.c +++ b/drivers/char/ipmi/ssif_bmc.c @@ -569,7 +569,7 @@ static void process_request_part(struct ssif_bmc_ctx *ssif_bmc) len = ssif_bmc->request.len + part->length; /* Do the bound check here, not allow the request len exceed 254 bytes */ if (len > IPMI_SSIF_PAYLOAD_MAX) { - dev_warn(&ssif_bmc->client->dev, + dev_dbg(&ssif_bmc->client->dev, "Warn: Request exceeded 254 bytes, aborting"); /* Request too long, aborting */ ssif_bmc->aborting = true; @@ -615,7 +615,7 @@ static void on_read_requested_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val) ssif_bmc->state == SSIF_START || ssif_bmc->state == SSIF_REQ_RECVING || ssif_bmc->state == SSIF_RES_SENDING) { - dev_warn(&ssif_bmc->client->dev, + dev_dbg(&ssif_bmc->client->dev, "Warn: %s unexpected READ REQUESTED in state=%s\n", __func__, state_to_string(ssif_bmc->state)); ssif_bmc->state = SSIF_ABORTING; @@ -624,7 +624,7 @@ static void on_read_requested_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val) } else if (ssif_bmc->state == SSIF_SMBUS_CMD) { if (!supported_read_cmd(ssif_bmc->part_buf.smbus_cmd)) { - dev_warn(&ssif_bmc->client->dev, "Warn: Unknown SMBus read command=0x%x", + dev_dbg(&ssif_bmc->client->dev, "Warn: Unknown SMBus read command=0x%x", ssif_bmc->part_buf.smbus_cmd); ssif_bmc->aborting = true; } @@ -659,7 +659,7 @@ static void on_read_processed_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val) ssif_bmc->state == SSIF_START || ssif_bmc->state == SSIF_REQ_RECVING || ssif_bmc->state == SSIF_SMBUS_CMD) { - dev_warn(&ssif_bmc->client->dev, + dev_dbg(&ssif_bmc->client->dev, "Warn: %s unexpected READ PROCESSED in state=%s\n", __func__, state_to_string(ssif_bmc->state)); ssif_bmc->state = SSIF_ABORTING; @@ -684,7 +684,7 @@ static void on_write_requested_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val) } else if (ssif_bmc->state == SSIF_START || ssif_bmc->state == SSIF_REQ_RECVING || ssif_bmc->state == SSIF_RES_SENDING) { - dev_warn(&ssif_bmc->client->dev, + dev_dbg(&ssif_bmc->client->dev, "Warn: %s unexpected WRITE REQUEST in state=%s\n", __func__, state_to_string(ssif_bmc->state)); ssif_bmc->state = SSIF_ABORTING; @@ -699,7 +699,7 @@ static void on_write_received_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val) { if (ssif_bmc->state == SSIF_READY || ssif_bmc->state == SSIF_RES_SENDING) { - dev_warn(&ssif_bmc->client->dev, + dev_dbg(&ssif_bmc->client->dev, "Warn: %s unexpected WRITE RECEIVED in state=%s\n", __func__, state_to_string(ssif_bmc->state)); ssif_bmc->state = SSIF_ABORTING; @@ -709,7 +709,7 @@ static void on_write_received_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val) } else if (ssif_bmc->state == SSIF_SMBUS_CMD) { if (!supported_write_cmd(ssif_bmc->part_buf.smbus_cmd)) { - dev_warn(&ssif_bmc->client->dev, "Warn: Unknown SMBus write command=0x%x", + dev_dbg(&ssif_bmc->client->dev, "Warn: Unknown SMBus write command=0x%x", ssif_bmc->part_buf.smbus_cmd); ssif_bmc->aborting = true; } @@ -738,7 +738,7 @@ static void on_stop_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val) ssif_bmc->state == SSIF_START || ssif_bmc->state == SSIF_SMBUS_CMD || ssif_bmc->state == SSIF_ABORTING) { - dev_warn(&ssif_bmc->client->dev, + dev_dbg(&ssif_bmc->client->dev, "Warn: %s unexpected SLAVE STOP in state=%s\n", __func__, state_to_string(ssif_bmc->state)); ssif_bmc->state = SSIF_READY; @@ -805,7 +805,7 @@ static int ssif_bmc_cb(struct i2c_client *client, enum i2c_slave_event event, u8 break; default: - dev_warn(&ssif_bmc->client->dev, "Warn: Unknown i2c slave event\n"); + dev_dbg(&ssif_bmc->client->dev, "Warn: Unknown i2c slave event\n"); break; } -- cgit v1.2.3 From d4464694f2a409fadbe17a70202242ff6b72ee30 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Fri, 3 Apr 2026 22:39:38 +0800 Subject: ipmi: ssif_bmc: add unit test for state machine Add some unit test for state machine when in SSIF_ABORTING state. Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver") Signed-off-by: Jian Zhang Message-ID: <20260403143939.434017-1-zhangjian.3032@bytedance.com> Signed-off-by: Corey Minyard --- drivers/char/ipmi/Kconfig | 10 ++ drivers/char/ipmi/ssif_bmc.c | 370 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 380 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig index 92bed266d07c..72559f6050eb 100644 --- a/drivers/char/ipmi/Kconfig +++ b/drivers/char/ipmi/Kconfig @@ -187,6 +187,16 @@ config SSIF_IPMI_BMC The driver implements the BMC side of the SMBus system interface (SSIF). +config SSIF_IPMI_BMC_KUNIT_TEST + bool "KUnit tests for SSIF IPMI BMC driver" if !KUNIT_ALL_TESTS + depends on KUNIT + depends on SSIF_IPMI_BMC + default KUNIT_ALL_TESTS + help + This option builds unit tests that exercise the SSIF BMC state + machine, including request handling, response transmission, + and error paths such as aborted or truncated transfers. + config IPMB_DEVICE_INTERFACE tristate 'IPMB Interface handler' depends on I2C diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c index 646a1e9ffbb7..1df0e9284ad9 100644 --- a/drivers/char/ipmi/ssif_bmc.c +++ b/drivers/char/ipmi/ssif_bmc.c @@ -18,6 +18,9 @@ #include #include #include +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST) +#include +#endif #define DEVICE_NAME "ipmi-ssif-host" @@ -886,6 +889,373 @@ static struct i2c_driver ssif_bmc_driver = { .id_table = ssif_bmc_id, }; +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST) +struct ssif_bmc_test_ctx { + struct ssif_bmc_ctx ssif_bmc; + struct i2c_client client; + struct i2c_adapter adapter; + struct i2c_algorithm algo; +}; + +static int ssif_bmc_test_init(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx; + + test_ctx = kunit_kzalloc(test, sizeof(*test_ctx), GFP_KERNEL); + if (!test_ctx) + return -ENOMEM; + + test_ctx->adapter.algo = &test_ctx->algo; + test_ctx->client.addr = 0x20; + test_ctx->client.adapter = &test_ctx->adapter; + + spin_lock_init(&test_ctx->ssif_bmc.lock); + init_waitqueue_head(&test_ctx->ssif_bmc.wait_queue); + test_ctx->ssif_bmc.client = &test_ctx->client; + i2c_set_clientdata(&test_ctx->client, &test_ctx->ssif_bmc); + + test->priv = test_ctx; + + return 0; +} + +static void ssif_bmc_test_exit(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + + if (test_ctx->ssif_bmc.response_timer_inited) + timer_delete_sync(&test_ctx->ssif_bmc.response_timer); +} + +static int ssif_bmc_test_run_event_val(struct ssif_bmc_test_ctx *test_ctx, + enum i2c_slave_event event, + u8 *value) +{ + return ssif_bmc_cb(&test_ctx->client, event, value); +} + +static int ssif_bmc_test_run_event(struct ssif_bmc_test_ctx *test_ctx, + enum i2c_slave_event event, u8 value) +{ + return ssif_bmc_test_run_event_val(test_ctx, event, &value); +} + +static void ssif_bmc_test_singlepart_req(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; + + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED, + GET_8BIT_ADDR(test_ctx->client.addr)); + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, + SSIF_IPMI_SINGLEPART_WRITE); + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2); + + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xaa); + + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x55); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY); + + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY); + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available); + KUNIT_EXPECT_TRUE(test, ssif_bmc->busy); + KUNIT_EXPECT_FALSE(test, ssif_bmc->aborting); + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 2); + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0xaa); + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[1], 0x55); + KUNIT_EXPECT_TRUE(test, ssif_bmc->response_timer_inited); +} + +static void ssif_bmc_test_restart_write_without_stop(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; + + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED, + GET_8BIT_ADDR(test_ctx->client.addr)), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, + SSIF_IPMI_SINGLEPART_WRITE), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xde), 0); + + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING); + + /* Write transaction, without stop, and new request coming */ + ssif_bmc_test_singlepart_req(test); +} + + +static void ssif_bmc_test_restart_after_invalid_command(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; + + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED, + GET_8BIT_ADDR(test_ctx->client.addr)), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xff), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0); + + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_ABORTING); + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting); + + /* After An Invalid Command, expect could handle new request */ + ssif_bmc_test_singlepart_req(test); +} + +static void ssif_bmc_test_singlepart_read_response_completion(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; + u8 value; + + ssif_bmc->state = SSIF_SMBUS_CMD; + ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ; + ssif_bmc->response.len = 2; + ssif_bmc->response.payload[0] = 0x11; + ssif_bmc->response.payload[1] = 0x22; + ssif_bmc->response_in_progress = true; + ssif_bmc->is_singlepart_read = true; + ssif_bmc->pec_support = true; + + value = 0; + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED, + &value), 0); + KUNIT_EXPECT_EQ(test, value, 2); + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING); + + value = 0; + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED, + &value), 0); + KUNIT_EXPECT_EQ(test, value, 0x11); + + value = 0; + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED, + &value), 0); + KUNIT_EXPECT_EQ(test, value, 0x22); + + value = 0; + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED, + &value), 0); + KUNIT_EXPECT_EQ(test, value, ssif_bmc->part_buf.pec); + + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0); + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY); + KUNIT_EXPECT_FALSE(test, ssif_bmc->response_in_progress); + KUNIT_EXPECT_EQ(test, ssif_bmc->response.len, 0); +} + +static void ssif_bmc_test_stop_during_start_discards_partial_request(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; + + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED, + GET_8BIT_ADDR(test_ctx->client.addr)), 0); + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_START); + + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0); + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY); + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available); + KUNIT_EXPECT_EQ(test, ssif_bmc->msg_idx, 0); + + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED, + GET_8BIT_ADDR(test_ctx->client.addr)), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, + SSIF_IPMI_SINGLEPART_WRITE), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x77), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY); + + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available); + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1); + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0x77); +} + +static void ssif_bmc_test_read_interrupts_partial_write(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; + u8 value = 0xff; + + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED, + GET_8BIT_ADDR(test_ctx->client.addr)), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, + SSIF_IPMI_SINGLEPART_WRITE), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xab), 0); + + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED, + &value), 0); + KUNIT_EXPECT_EQ(test, value, 0); + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_ABORTING); + + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0); + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY); + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available); + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 0); + + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED, + GET_8BIT_ADDR(test_ctx->client.addr)), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, + SSIF_IPMI_SINGLEPART_WRITE), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xcd), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY); + + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available); + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1); + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0xcd); +} + +static void ssif_bmc_test_write_interrupts_response_send(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; + u8 value = 0; + + ssif_bmc->state = SSIF_SMBUS_CMD; + ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ; + ssif_bmc->response.len = 1; + ssif_bmc->response.payload[0] = 0x66; + ssif_bmc->response_in_progress = true; + ssif_bmc->is_singlepart_read = true; + + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED, + &value), 0); + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING); + + /* READ_REQUESTED transaction */ + ssif_bmc_test_singlepart_req(test); +} + +static void ssif_bmc_test_write_interrupts_response_sending(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; + u8 value = 0; + + ssif_bmc->state = SSIF_SMBUS_CMD; + ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ; + ssif_bmc->response.len = 1; + ssif_bmc->response.payload[0] = 0x66; + ssif_bmc->response_in_progress = true; + ssif_bmc->is_singlepart_read = true; + + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED, + &value), 0); + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING); + + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED, + &value), 0); + KUNIT_EXPECT_EQ(test, value, 0x66); + + /* READ_REQUESTED transaction */ + ssif_bmc_test_singlepart_req(test); +} + +static void ssif_bmc_test_timeout_interrupt_allows_retry(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; + + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED, + GET_8BIT_ADDR(test_ctx->client.addr)), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, + SSIF_IPMI_SINGLEPART_WRITE), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x21), 0); + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY); + + KUNIT_ASSERT_TRUE(test, timer_pending(&ssif_bmc->response_timer)); + timer_delete_sync(&ssif_bmc->response_timer); + response_timeout(&ssif_bmc->response_timer); + + KUNIT_EXPECT_FALSE(test, ssif_bmc->busy); + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting); + KUNIT_EXPECT_FALSE(test, ssif_bmc->response_timer_inited); + + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED, + GET_8BIT_ADDR(test_ctx->client.addr)), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, + SSIF_IPMI_SINGLEPART_WRITE), 0); + KUNIT_EXPECT_FALSE(test, ssif_bmc->aborting); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x22), 0); + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY); + + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available); + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1); + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0x22); +} + +static struct kunit_case ssif_bmc_test_cases[] = { + KUNIT_CASE(ssif_bmc_test_singlepart_req), + KUNIT_CASE(ssif_bmc_test_restart_write_without_stop), + KUNIT_CASE(ssif_bmc_test_restart_after_invalid_command), + KUNIT_CASE(ssif_bmc_test_singlepart_read_response_completion), + KUNIT_CASE(ssif_bmc_test_stop_during_start_discards_partial_request), + KUNIT_CASE(ssif_bmc_test_read_interrupts_partial_write), + KUNIT_CASE(ssif_bmc_test_write_interrupts_response_send), + KUNIT_CASE(ssif_bmc_test_write_interrupts_response_sending), + KUNIT_CASE(ssif_bmc_test_timeout_interrupt_allows_retry), + {} +}; + +static struct kunit_suite ssif_bmc_test_suite = { + .name = "ssif_bmc_test", + .init = ssif_bmc_test_init, + .exit = ssif_bmc_test_exit, + .test_cases = ssif_bmc_test_cases, +}; + +kunit_test_suite(ssif_bmc_test_suite); +#endif + module_i2c_driver(ssif_bmc_driver); MODULE_AUTHOR("Quan Nguyen "); -- cgit v1.2.3 From 93b5d21e8b5cbdc3e439b94feee9b013e8170905 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Tue, 7 Apr 2026 17:46:47 +0800 Subject: ipmi: ssif_bmc: Fix KUnit test link failure when KUNIT=m Building with CONFIG_KUNIT=m and CONFIG_SSIF_IPMI_BMC_KUNIT_TEST=y results in link errors such as: undefined reference to `kunit_binary_assert_format' undefined reference to `__kunit_do_failed_assertion' This happens because the test code is built-in while the KUnit core is built as a module, so the required KUnit symbols are not available at link time. Fix this by requiring KUNIT to be built-in when enabling SSIF_IPMI_BMC_KUNIT_TEST. Signed-off-by: Jian Zhang Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202604071448.zUBjPYPu-lkp@intel.com/ Message-ID: <20260407094647.356661-1-zhangjian.3032@bytedance.com> Signed-off-by: Corey Minyard --- drivers/char/ipmi/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig index 72559f6050eb..669f76000197 100644 --- a/drivers/char/ipmi/Kconfig +++ b/drivers/char/ipmi/Kconfig @@ -189,7 +189,7 @@ config SSIF_IPMI_BMC config SSIF_IPMI_BMC_KUNIT_TEST bool "KUnit tests for SSIF IPMI BMC driver" if !KUNIT_ALL_TESTS - depends on KUNIT + depends on KUNIT=y depends on SSIF_IPMI_BMC default KUNIT_ALL_TESTS help -- cgit v1.2.3 From 91eb7ec7261254b6875909df767185838598e21e Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Mon, 13 Apr 2026 07:09:15 -0500 Subject: ipmi:ssif: Remove unnecessary indention A section was in {} that didn't need to be, move the variable definition to the top and set th eindentino properly. Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_ssif.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 37a5cb5c53f1..ce918fe987c6 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1658,6 +1658,7 @@ static int ssif_probe(struct i2c_client *client) int len = 0; int i; u8 slave_addr = 0; + unsigned int thread_num; struct ssif_addr_info *addr_info = NULL; mutex_lock(&ssif_infos_mutex); @@ -1876,22 +1877,17 @@ static int ssif_probe(struct i2c_client *client) ssif_info->handlers.request_events = request_events; ssif_info->handlers.set_need_watch = ssif_set_need_watch; - { - unsigned int thread_num; - - thread_num = ((i2c_adapter_id(ssif_info->client->adapter) - << 8) | - ssif_info->client->addr); - init_completion(&ssif_info->wake_thread); - ssif_info->thread = kthread_run(ipmi_ssif_thread, ssif_info, - "kssif%4.4x", thread_num); - if (IS_ERR(ssif_info->thread)) { - rv = PTR_ERR(ssif_info->thread); - dev_notice(&ssif_info->client->dev, - "Could not start kernel thread: error %d\n", - rv); - goto out; - } + thread_num = ((i2c_adapter_id(ssif_info->client->adapter) << 8) | + ssif_info->client->addr); + init_completion(&ssif_info->wake_thread); + ssif_info->thread = kthread_run(ipmi_ssif_thread, ssif_info, + "kssif%4.4x", thread_num); + if (IS_ERR(ssif_info->thread)) { + rv = PTR_ERR(ssif_info->thread); + dev_notice(&ssif_info->client->dev, + "Could not start kernel thread: error %d\n", + rv); + goto out; } dev_set_drvdata(&ssif_info->client->dev, ssif_info); -- cgit v1.2.3 From 75c486cb1bcaa1a3ec3a6438498176a3a4998ae4 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Mon, 13 Apr 2026 08:00:23 -0500 Subject: ipmi:ssif: Clean up kthread on errors If an error occurs after the ssif kthread is created, but before the main IPMI code starts the ssif interface, the ssif kthread will not be stopped. So make sure the kthread is stopped on an error condition if it is running. Fixes: 259307074bfc ("ipmi: Add SMBus interface driver (SSIF)") Reported-by: Li Xiao <<252270051@hdu.edu.cn> Cc: stable@vger.kernel.org Reviewed-by: Li Xiao <252270051@hdu.edu.cn> Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_ssif.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'drivers/char') diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index ce918fe987c6..b49500a1bd36 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1268,8 +1268,10 @@ static void shutdown_ssif(void *send_info) ssif_info->stopping = true; timer_delete_sync(&ssif_info->watch_timer); timer_delete_sync(&ssif_info->retry_timer); - if (ssif_info->thread) + if (ssif_info->thread) { kthread_stop(ssif_info->thread); + ssif_info->thread = NULL; + } } static void ssif_remove(struct i2c_client *client) @@ -1912,6 +1914,15 @@ static int ssif_probe(struct i2c_client *client) out: if (rv) { + /* + * If ipmi_register_smi() starts the interface, it will + * call shutdown and that will free the thread and set + * it to NULL. Otherwise it must be freed here. + */ + if (ssif_info->thread) { + kthread_stop(ssif_info->thread); + ssif_info->thread = NULL; + } if (addr_info) addr_info->client = NULL; -- cgit v1.2.3