* [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove
@ 2026-04-03 9:05 Jian Zhang
2026-04-03 9:05 ` [PATCH v2 2/5] ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure Jian Zhang
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Jian Zhang @ 2026-04-03 9:05 UTC (permalink / raw)
To: Corey Minyard, Quan Nguyen, openipmi-developer, linux-kernel
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 <zhangjian.3032@bytedance.com>
---
v2: use timer_delete_sync() to cancel the timer
drivers/char/ipmi/ssif_bmc.c | 1 +
1 file changed, 1 insertion(+)
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);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/5] ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure 2026-04-03 9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang @ 2026-04-03 9:05 ` Jian Zhang 2026-04-03 9:06 ` [PATCH v2 3/5] ipmi: ssif_bmc: fix message desynchronization after truncated response Jian Zhang ` (3 subsequent siblings) 4 siblings, 0 replies; 9+ messages in thread From: Jian Zhang @ 2026-04-03 9:05 UTC (permalink / raw) To: Corey Minyard, Quan Nguyen, openipmi-developer, linux-kernel 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 <zhangjian.3032@bytedance.com> --- drivers/char/ipmi/ssif_bmc.c | 2 ++ 1 file changed, 2 insertions(+) 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; -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/5] ipmi: ssif_bmc: fix message desynchronization after truncated response 2026-04-03 9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang 2026-04-03 9:05 ` [PATCH v2 2/5] ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure Jian Zhang @ 2026-04-03 9:06 ` Jian Zhang 2026-04-03 9:06 ` [PATCH v2 4/5] ipmi: ssif_bmc: change log level to dbg in irq callback Jian Zhang ` (2 subsequent siblings) 4 siblings, 0 replies; 9+ messages in thread From: Jian Zhang @ 2026-04-03 9:06 UTC (permalink / raw) To: Corey Minyard, Quan Nguyen, openipmi-developer, linux-kernel 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 <zhangjian.3032@bytedance.com> --- drivers/char/ipmi/ssif_bmc.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 */ -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 4/5] ipmi: ssif_bmc: change log level to dbg in irq callback 2026-04-03 9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang 2026-04-03 9:05 ` [PATCH v2 2/5] ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure Jian Zhang 2026-04-03 9:06 ` [PATCH v2 3/5] ipmi: ssif_bmc: fix message desynchronization after truncated response Jian Zhang @ 2026-04-03 9:06 ` Jian Zhang 2026-04-03 9:06 ` [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine Jian Zhang 2026-04-03 13:12 ` [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Corey Minyard 4 siblings, 0 replies; 9+ messages in thread From: Jian Zhang @ 2026-04-03 9:06 UTC (permalink / raw) To: Corey Minyard, Quan Nguyen, openipmi-developer, linux-kernel 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 <zhangjian.3032@bytedance.com> --- drivers/char/ipmi/ssif_bmc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine 2026-04-03 9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang ` (2 preceding siblings ...) 2026-04-03 9:06 ` [PATCH v2 4/5] ipmi: ssif_bmc: change log level to dbg in irq callback Jian Zhang @ 2026-04-03 9:06 ` Jian Zhang 2026-04-03 13:14 ` Corey Minyard 2026-04-03 13:12 ` [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Corey Minyard 4 siblings, 1 reply; 9+ messages in thread From: Jian Zhang @ 2026-04-03 9:06 UTC (permalink / raw) To: Corey Minyard, Quan Nguyen, openipmi-developer, linux-kernel 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 <zhangjian.3032@bytedance.com> --- v2: remove undefined symbol response_in_send drivers/char/ipmi/Kconfig | 10 + drivers/char/ipmi/ssif_bmc.c | 401 +++++++++++++++++++++++++++++++++++ 2 files changed, 411 insertions(+) diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig index 92bed266d07c..709820f80266 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=y + depends on SSIF_IPMI_BMC=y + 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..726626126cea 100644 --- a/drivers/char/ipmi/ssif_bmc.c +++ b/drivers/char/ipmi/ssif_bmc.c @@ -18,6 +18,9 @@ #include <linux/timer.h> #include <linux/jiffies.h> #include <linux/ipmi_ssif_bmc.h> +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST) +#include <kunit/test.h> +#endif #define DEVICE_NAME "ipmi-ssif-host" @@ -886,6 +889,404 @@ 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_restart_after_invalid_length(struct kunit *test) +{ + struct ssif_bmc_test_ctx *test_ctx = test->priv; + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; + int i; + + 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, + MAX_PAYLOAD_PER_TRANSACTION + 1), 0); + + for (i = 0; i < MAX_PAYLOAD_PER_TRANSACTION + 1; i++) + KUNIT_ASSERT_EQ(test, + ssif_bmc_test_run_event(test_ctx, + I2C_SLAVE_WRITE_RECEIVED, i), 0); + + KUNIT_EXPECT_EQ(test, + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0); + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting); + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available); + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING); + + 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_restart_after_invalid_length), + 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 <quan@os.amperecomputing.com>"); -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine 2026-04-03 9:06 ` [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine Jian Zhang @ 2026-04-03 13:14 ` Corey Minyard 2026-04-03 14:12 ` Jian Zhang 0 siblings, 1 reply; 9+ messages in thread From: Corey Minyard @ 2026-04-03 13:14 UTC (permalink / raw) To: Jian Zhang; +Cc: Quan Nguyen, openipmi-developer, linux-kernel On Fri, Apr 03, 2026 at 05:06:02PM +0800, Jian Zhang wrote: > 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 <zhangjian.3032@bytedance.com> > --- > v2: remove undefined symbol response_in_send > > drivers/char/ipmi/Kconfig | 10 + > drivers/char/ipmi/ssif_bmc.c | 401 +++++++++++++++++++++++++++++++++++ > 2 files changed, 411 insertions(+) > > diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig > index 92bed266d07c..709820f80266 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=y > + depends on SSIF_IPMI_BMC=y Any reason for the "=y" in the above two? It's best to remove them if they are not needed so it can work in a module. I can make that change, there's no need for a new series for that. Oh, and I forgot to say in the reply on the first change, thanks Quan for reviewing these. -corey > + 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..726626126cea 100644 > --- a/drivers/char/ipmi/ssif_bmc.c > +++ b/drivers/char/ipmi/ssif_bmc.c > @@ -18,6 +18,9 @@ > #include <linux/timer.h> > #include <linux/jiffies.h> > #include <linux/ipmi_ssif_bmc.h> > +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST) > +#include <kunit/test.h> > +#endif > > #define DEVICE_NAME "ipmi-ssif-host" > > @@ -886,6 +889,404 @@ 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_restart_after_invalid_length(struct kunit *test) > +{ > + struct ssif_bmc_test_ctx *test_ctx = test->priv; > + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; > + int i; > + > + 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, > + MAX_PAYLOAD_PER_TRANSACTION + 1), 0); > + > + for (i = 0; i < MAX_PAYLOAD_PER_TRANSACTION + 1; i++) > + KUNIT_ASSERT_EQ(test, > + ssif_bmc_test_run_event(test_ctx, > + I2C_SLAVE_WRITE_RECEIVED, i), 0); > + > + KUNIT_EXPECT_EQ(test, > + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0); > + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting); > + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available); > + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING); > + > + 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_restart_after_invalid_length), > + 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 <quan@os.amperecomputing.com>"); > -- > 2.20.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine 2026-04-03 13:14 ` Corey Minyard @ 2026-04-03 14:12 ` Jian Zhang 2026-04-03 14:31 ` Corey Minyard 0 siblings, 1 reply; 9+ messages in thread From: Jian Zhang @ 2026-04-03 14:12 UTC (permalink / raw) To: Corey Minyard; +Cc: Quan Nguyen, openipmi-developer, linux-kernel > 2026年4月3日 21:14，Corey Minyard <corey@minyard.net> 写道： > > On Fri, Apr 03, 2026 at 05:06:02PM +0800, Jian Zhang wrote: >> 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 <zhangjian.3032@bytedance.com> >> --- >> v2: remove undefined symbol response_in_send >> >> drivers/char/ipmi/Kconfig | 10 + >> drivers/char/ipmi/ssif_bmc.c | 401 +++++++++++++++++++++++++++++++++++ >> 2 files changed, 411 insertions(+) >> >> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig >> index 92bed266d07c..709820f80266 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=y >> + depends on SSIF_IPMI_BMC=y > > Any reason for the "=y" in the above two? > > It's best to remove them if they are not needed so it can work in a > module. I can make that change, there's no need for a new series for > that. Nothing special and agree, Thanks. There’s one more thing that needs adjustment as well. Sorry about that—I picked it from dev-6.1 without testing (at the time there was a build error, so I didn’t run the unit tests). The test “ssif_bmc_test_restart_after_invalid_length” also needs to be removed. I do have a local patch regarding the length limitation, but it still needs thorough testing. Sorry again, I’m not very familiar with it yet, I’ll pay more attention in the future! - Jian > > Oh, and I forgot to say in the reply on the first change, thanks Quan > for reviewing these. > > -corey > >> + 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..726626126cea 100644 >> --- a/drivers/char/ipmi/ssif_bmc.c >> +++ b/drivers/char/ipmi/ssif_bmc.c >> @@ -18,6 +18,9 @@ >> #include <linux/timer.h> >> #include <linux/jiffies.h> >> #include <linux/ipmi_ssif_bmc.h> >> +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST) >> +#include <kunit/test.h> >> +#endif >> >> #define DEVICE_NAME "ipmi-ssif-host" >> >> @@ -886,6 +889,404 @@ 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_restart_after_invalid_length(struct kunit *test) >> +{ >> + struct ssif_bmc_test_ctx *test_ctx = test->priv; >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; >> + int i; >> + >> + 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, >> + MAX_PAYLOAD_PER_TRANSACTION + 1), 0); >> + >> + for (i = 0; i < MAX_PAYLOAD_PER_TRANSACTION + 1; i++) >> + KUNIT_ASSERT_EQ(test, >> + ssif_bmc_test_run_event(test_ctx, >> + I2C_SLAVE_WRITE_RECEIVED, i), 0); >> + >> + KUNIT_EXPECT_EQ(test, >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0); >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting); >> + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available); >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING); >> + >> + 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_restart_after_invalid_length), >> + 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 <quan@os.amperecomputing.com>"); >> -- >> 2.20.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine 2026-04-03 14:12 ` Jian Zhang @ 2026-04-03 14:31 ` Corey Minyard 0 siblings, 0 replies; 9+ messages in thread From: Corey Minyard @ 2026-04-03 14:31 UTC (permalink / raw) To: Jian Zhang; +Cc: Quan Nguyen, openipmi-developer, linux-kernel On Fri, Apr 03, 2026 at 10:12:45PM +0800, Jian Zhang wrote: > > 2026年4月3日 21:14，Corey Minyard <corey@minyard.net> 写道： > > > > On Fri, Apr 03, 2026 at 05:06:02PM +0800, Jian Zhang wrote: > >> 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 <zhangjian.3032@bytedance.com> > >> --- > >> v2: remove undefined symbol response_in_send > >> > >> drivers/char/ipmi/Kconfig | 10 + > >> drivers/char/ipmi/ssif_bmc.c | 401 +++++++++++++++++++++++++++++++++++ > >> 2 files changed, 411 insertions(+) > >> > >> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig > >> index 92bed266d07c..709820f80266 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=y > >> + depends on SSIF_IPMI_BMC=y > > > > Any reason for the "=y" in the above two? > > > > It's best to remove them if they are not needed so it can work in a > > module. I can make that change, there's no need for a new series for > > that. > > Nothing special and agree, Thanks. > There’s one more thing that needs adjustment as well. > Sorry about that—I picked it from dev-6.1 without testing (at the time there was a build error, so I didn’t run the unit tests). > The test “ssif_bmc_test_restart_after_invalid_length” also needs to be removed. I do have a local patch regarding the length limitation, > but it still needs thorough testing. Sorry again, I’m not very familiar with it yet, I’ll pay more attention in the future! In that case, can you re-send just patch 5? Thanks, -corey > > - Jian > > > > Oh, and I forgot to say in the reply on the first change, thanks Quan > > for reviewing these. > > > > -corey > > > >> + 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..726626126cea 100644 > >> --- a/drivers/char/ipmi/ssif_bmc.c > >> +++ b/drivers/char/ipmi/ssif_bmc.c > >> @@ -18,6 +18,9 @@ > >> #include <linux/timer.h> > >> #include <linux/jiffies.h> > >> #include <linux/ipmi_ssif_bmc.h> > >> +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST) > >> +#include <kunit/test.h> > >> +#endif > >> > >> #define DEVICE_NAME "ipmi-ssif-host" > >> > >> @@ -886,6 +889,404 @@ 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_restart_after_invalid_length(struct kunit *test) > >> +{ > >> + struct ssif_bmc_test_ctx *test_ctx = test->priv; > >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc; > >> + int i; > >> + > >> + 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, > >> + MAX_PAYLOAD_PER_TRANSACTION + 1), 0); > >> + > >> + for (i = 0; i < MAX_PAYLOAD_PER_TRANSACTION + 1; i++) > >> + KUNIT_ASSERT_EQ(test, > >> + ssif_bmc_test_run_event(test_ctx, > >> + I2C_SLAVE_WRITE_RECEIVED, i), 0); > >> + > >> + KUNIT_EXPECT_EQ(test, > >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0); > >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting); > >> + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available); > >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING); > >> + > >> + 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_restart_after_invalid_length), > >> + 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 <quan@os.amperecomputing.com>"); > >> -- > >> 2.20.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove 2026-04-03 9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang ` (3 preceding siblings ...) 2026-04-03 9:06 ` [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine Jian Zhang @ 2026-04-03 13:12 ` Corey Minyard 4 siblings, 0 replies; 9+ messages in thread From: Corey Minyard @ 2026-04-03 13:12 UTC (permalink / raw) To: Jian Zhang; +Cc: Quan Nguyen, openipmi-developer, linux-kernel On Fri, Apr 03, 2026 at 05:05:58PM +0800, Jian Zhang wrote: > 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. Thanks for the updates on this. I have the new version in my tree. I have one question on the kunit patch, I'll ask it there. -corey > > Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com> > --- > v2: use timer_delete_sync() to cancel the timer > > drivers/char/ipmi/ssif_bmc.c | 1 + > 1 file changed, 1 insertion(+) > > 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); > } > -- > 2.20.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-04-03 14:31 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-03 9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang 2026-04-03 9:05 ` [PATCH v2 2/5] ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure Jian Zhang 2026-04-03 9:06 ` [PATCH v2 3/5] ipmi: ssif_bmc: fix message desynchronization after truncated response Jian Zhang 2026-04-03 9:06 ` [PATCH v2 4/5] ipmi: ssif_bmc: change log level to dbg in irq callback Jian Zhang 2026-04-03 9:06 ` [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine Jian Zhang 2026-04-03 13:14 ` Corey Minyard 2026-04-03 14:12 ` Jian Zhang 2026-04-03 14:31 ` Corey Minyard 2026-04-03 13:12 ` [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Corey Minyard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox