[PATCH v2 4/4] spi: Add KUnit coverage for DMA mapping error paths

Honghui Jiang <[email protected]>
Newsgroups org.kernel.vger.linux-spi,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest
Message-ID <[email protected]>
Add KUnit tests for the __spi_map_msg() error paths. The tests verify
that a later TX or RX mapping failure clears the mapping state of
earlier transfers and leaves cur_{tx,rx}_dma_dev identifying the
current mapping device.

A zero-length transfer causes sg_alloc_table() to return -EINVAL,
providing deterministic failure injection without test hooks.
Additional cases cover successful map/unmap and a message which
requires no mapping.

Build the DMA suite as a separate translation unit, exposing the two
internal mapping helpers only for KUnit through the local internal
header. Enable SPI in the default and all-tests KUnit configurations so
the suite is exercised there.

Signed-off-by: Honghui Jiang <[email protected]>
---
 drivers/spi/.kunitconfig                     |   4 +
 drivers/spi/Kconfig                          |  13 +
 drivers/spi/Makefile                         |   1 +
 drivers/spi/internals.h                      |   9 +-
 drivers/spi/spi.c                            |   9 +-
 drivers/spi/tests/Makefile                   |   3 +
 drivers/spi/tests/spi-dma-kunit.c            | 259 +++++++++++++++++++
 tools/testing/kunit/configs/all_tests.config |   1 +
 tools/testing/kunit/configs/default.config   |   1 +
 9 files changed, 296 insertions(+), 4 deletions(-)
 create mode 100644 drivers/spi/.kunitconfig
 create mode 100644 drivers/spi/tests/Makefile
 create mode 100644 drivers/spi/tests/spi-dma-kunit.c

diff --git a/drivers/spi/.kunitconfig b/drivers/spi/.kunitconfig
new file mode 100644
index 000000000..07fa092c8
--- /dev/null
+++ b/drivers/spi/.kunitconfig
@@ -0,0 +1,4 @@
+CONFIG_KUNIT=y
+CONFIG_SPI=y
+CONFIG_SPI_MASTER=y
+CONFIG_SPI_DMA_KUNIT_TEST=y
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 8782514bb..74382c31e 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -1360,6 +1360,19 @@ config SPI_SPIDEV
 	help
 	  This supports user mode SPI protocol drivers.
 
+config SPI_DMA_KUNIT_TEST
+	tristate "KUnit tests for SPI core DMA mapping" if !KUNIT_ALL_TESTS
+	depends on KUNIT && HAS_DMA
+	default KUNIT_ALL_TESTS
+	help
+	  Build KUnit tests for SPI core DMA mapping. The tests exercise
+	  partial TX and RX mapping failures, verify that the mapping state is
+	  unwound, and check that the current DMA devices identify the owner of
+	  those mappings when the error is returned. They also cover a
+	  successful mapping and a message which requires no mapping.
+
+	  If unsure say N.
+
 config SPI_LOOPBACK_TEST
 	tristate "spi loopback test framework support"
 	depends on m
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 9fa12498c..f693699f8 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_SPI_MUX)			+= spi-mux.o
 obj-$(CONFIG_SPI_OFFLOAD)		+= spi-offload.o
 obj-$(CONFIG_SPI_SPIDEV)		+= spidev.o
 obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
+obj-$(CONFIG_SPI_DMA_KUNIT_TEST)	+= tests/
 
 # SPI master controller drivers (bus)
 obj-$(CONFIG_SPI_AIROHA_SNFI)		+= spi-airoha-snfi.o
diff --git a/drivers/spi/internals.h b/drivers/spi/internals.h
index 1f459b895..c56c190b0 100644
--- a/drivers/spi/internals.h
+++ b/drivers/spi/internals.h
@@ -5,8 +5,8 @@
  *
  * Author: Boris Brezillon <[email protected]>
  *
- * Helpers needed by the spi or spi-mem logic. Should not be used outside of
- * spi-mem.c and spi.c.
+ * Helpers needed by the SPI core and its tests. Should not be used outside
+ * drivers/spi/.
  */
 
 #ifndef __LINUX_SPI_INTERNALS_H
@@ -20,6 +20,11 @@
 void spi_flush_queue(struct spi_controller *ctrl);
 
 #ifdef CONFIG_HAS_DMA
+#if IS_ENABLED(CONFIG_KUNIT)
+int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg);
+int __spi_unmap_msg(struct spi_controller *ctlr, struct spi_message *msg);
+#endif
+
 int spi_map_buf(struct spi_controller *ctlr, struct device *dev,
 		struct sg_table *sgt, void *buf, size_t len,
 		enum dma_data_direction dir);
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index e5b1531b9..6be7987ed 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -4,6 +4,7 @@
 // Copyright (C) 2005 David Brownell
 // Copyright (C) 2008 Secret Lab Technologies Ltd.
 
+#include <kunit/visibility.h>
 #include <linux/acpi.h>
 #include <linux/cache.h>
 #include <linux/clk/clk-conf.h>
@@ -1231,7 +1232,8 @@ void spi_unmap_buf(struct spi_controller *ctlr, struct device *dev,
 	spi_unmap_buf_attrs(ctlr, dev, sgt, dir, 0);
 }
 
-static int __spi_unmap_msg(struct spi_controller *ctlr, struct spi_message *msg)
+VISIBLE_IF_KUNIT
+int __spi_unmap_msg(struct spi_controller *ctlr, struct spi_message *msg)
 {
 	struct device *rx_dev = ctlr->cur_rx_dma_dev;
 	struct device *tx_dev = ctlr->cur_tx_dma_dev;
@@ -1254,8 +1256,10 @@ static int __spi_unmap_msg(struct spi_controller *ctlr, struct spi_message *msg)
 
 	return 0;
 }
+EXPORT_SYMBOL_IF_KUNIT(__spi_unmap_msg);
 
-static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
+VISIBLE_IF_KUNIT
+int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
 {
 	struct device *tx_dev, *rx_dev;
 	struct spi_transfer *xfer;
@@ -1321,6 +1325,7 @@ static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
 
 	return ret;
 }
+EXPORT_SYMBOL_IF_KUNIT(__spi_map_msg);
 
 static void spi_dma_sync_for_device(struct spi_controller *ctlr,
 				    struct spi_transfer *xfer)
diff --git a/drivers/spi/tests/Makefile b/drivers/spi/tests/Makefile
new file mode 100644
index 000000000..26689e0cb
--- /dev/null
+++ b/drivers/spi/tests/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_SPI_DMA_KUNIT_TEST) += spi-dma-kunit.o
diff --git a/drivers/spi/tests/spi-dma-kunit.c b/drivers/spi/tests/spi-dma-kunit.c
new file mode 100644
index 000000000..ee5476ff3
--- /dev/null
+++ b/drivers/spi/tests/spi-dma-kunit.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0
+// KUnit tests for the SPI core DMA mapping error paths.
+//
+// A mapping error must clear all SG tables and *_sg_mapped flags while
+// cur_{tx,rx}_dma_dev identify the devices used for the attempted mapping.
+// Zero-length transfers make sg_alloc_table() fail with -EINVAL, providing
+// deterministic failure injection without test hooks.
+
+#include <kunit/device.h>
+#include <kunit/test.h>
+#include <linux/dma-mapping.h>
+#include <linux/limits.h>
+#include <linux/spi/spi.h>
+
+#include "../internals.h"
+
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
+
+#define SPI_DMA_TEST_LEN		256
+#define SPI_DMA_TEST_XFERS		2
+
+struct spi_dma_test_ctx {
+	struct spi_controller	*ctlr;
+	struct spi_device	*spi;
+	struct device		*dma_dev;
+	struct device		*stale_dma_dev;
+	struct spi_transfer	xfer[SPI_DMA_TEST_XFERS];
+	struct spi_message	msg;
+	void			*buf[SPI_DMA_TEST_XFERS * 2];
+};
+
+static bool spi_dma_test_can_dma(struct spi_controller *ctlr,
+				 struct spi_device *spi,
+				 struct spi_transfer *xfer)
+{
+	/* Opt every transfer into the core DMA mapping path. */
+	return true;
+}
+
+/*
+ * A bare controller is sufficient because the mapping helpers do not
+ * dereference ctlr->dev. With dma_tx and dma_rx unset, both directions use
+ * dma_map_dev, so the controller need not be registered.
+ */
+static struct spi_dma_test_ctx *spi_dma_test_ctx_new(struct kunit *test)
+{
+	struct spi_dma_test_ctx *ctx;
+
+	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+	ctx->dma_dev = kunit_device_register(test, "spi-dma-error-path");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->dma_dev);
+	ctx->stale_dma_dev =
+		kunit_device_register(test, "spi-dma-stale-device");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->stale_dma_dev);
+
+	/* Keep both devices valid if an assertion aborts the test. */
+	KUNIT_ASSERT_EQ(test, 0,
+			dma_coerce_mask_and_coherent(ctx->dma_dev,
+						     DMA_BIT_MASK(64)));
+	KUNIT_ASSERT_EQ(test, 0,
+			dma_coerce_mask_and_coherent(ctx->stale_dma_dev,
+						     DMA_BIT_MASK(64)));
+
+	ctx->ctlr = kunit_kzalloc(test, sizeof(*ctx->ctlr), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->ctlr);
+
+	ctx->spi = kunit_kzalloc(test, sizeof(*ctx->spi), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->spi);
+
+	ctx->ctlr->can_dma = spi_dma_test_can_dma;
+	ctx->ctlr->dma_map_dev = ctx->dma_dev;
+	/* Normally initialized by spi_register_controller(). */
+	ctx->ctlr->max_dma_len = INT_MAX;
+
+	ctx->spi->controller = ctx->ctlr;
+	spi_message_init(&ctx->msg);
+	ctx->msg.spi = ctx->spi;
+
+	return ctx;
+}
+
+static void *spi_dma_test_buf(struct kunit *test, struct spi_dma_test_ctx *ctx,
+			      unsigned int slot)
+{
+	KUNIT_ASSERT_LT(test, slot, ARRAY_SIZE(ctx->buf));
+
+	ctx->buf[slot] = kunit_kzalloc(test, SPI_DMA_TEST_LEN, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->buf[slot]);
+
+	return ctx->buf[slot];
+}
+
+/*
+ * Emulate DMA devices retained from an earlier message. Using valid devices
+ * also lets the unfixed path reach the assertions instead of dereferencing
+ * NULL during cleanup.
+ */
+static void spi_dma_test_pin_stale_dma_devs(struct spi_dma_test_ctx *ctx)
+{
+	ctx->ctlr->cur_tx_dma_dev = ctx->stale_dma_dev;
+	ctx->ctlr->cur_rx_dma_dev = ctx->stale_dma_dev;
+}
+
+static void spi_dma_test_assert_dma_devs_published(struct kunit *test,
+						   struct spi_dma_test_ctx *ctx)
+{
+	KUNIT_ASSERT_PTR_EQ(test, ctx->ctlr->cur_tx_dma_dev, ctx->dma_dev);
+	KUNIT_ASSERT_PTR_EQ(test, ctx->ctlr->cur_rx_dma_dev, ctx->dma_dev);
+}
+
+static void spi_dma_test_assert_nothing_mapped(struct kunit *test,
+					       struct spi_dma_test_ctx *ctx,
+					       unsigned int nr_xfers)
+{
+	unsigned int i;
+
+	for (i = 0; i < nr_xfers; i++) {
+		KUNIT_ASSERT_FALSE_MSG(test, ctx->xfer[i].tx_sg_mapped,
+				       "xfer[%u] still claims a TX mapping after __spi_map_msg() failed",
+				       i);
+		KUNIT_ASSERT_FALSE_MSG(test, ctx->xfer[i].rx_sg_mapped,
+				       "xfer[%u] still claims an RX mapping after __spi_map_msg() failed",
+				       i);
+		KUNIT_EXPECT_NULL(test, ctx->xfer[i].tx_sg.sgl);
+		KUNIT_EXPECT_EQ(test, ctx->xfer[i].tx_sg.orig_nents, 0U);
+		KUNIT_EXPECT_EQ(test, ctx->xfer[i].tx_sg.nents, 0U);
+		KUNIT_EXPECT_NULL(test, ctx->xfer[i].rx_sg.sgl);
+		KUNIT_EXPECT_EQ(test, ctx->xfer[i].rx_sg.orig_nents, 0U);
+		KUNIT_EXPECT_EQ(test, ctx->xfer[i].rx_sg.nents, 0U);
+	}
+}
+
+/*
+ * xfer0 maps TX and RX; zero-length xfer1 then fails its TX mapping.
+ * The failure must unwind xfer0 and update cur_*_dma_dev.
+ */
+static void spi_dma_later_tx_fail_rolls_back_earlier(struct kunit *test)
+{
+	struct spi_dma_test_ctx *ctx = spi_dma_test_ctx_new(test);
+	int ret;
+
+	ctx->xfer[0].tx_buf = spi_dma_test_buf(test, ctx, 0);
+	ctx->xfer[0].rx_buf = spi_dma_test_buf(test, ctx, 1);
+	ctx->xfer[0].len = SPI_DMA_TEST_LEN;
+
+	ctx->xfer[1].tx_buf = spi_dma_test_buf(test, ctx, 2);
+	ctx->xfer[1].rx_buf = NULL;
+	ctx->xfer[1].len = 0;			/* forces -EINVAL */
+
+	spi_message_add_tail(&ctx->xfer[0], &ctx->msg);
+	spi_message_add_tail(&ctx->xfer[1], &ctx->msg);
+
+	spi_dma_test_pin_stale_dma_devs(ctx);
+
+	ret = __spi_map_msg(ctx->ctlr, &ctx->msg);
+	KUNIT_ASSERT_EQ(test, ret, -EINVAL);
+
+	spi_dma_test_assert_dma_devs_published(test, ctx);
+	spi_dma_test_assert_nothing_mapped(test, ctx, SPI_DMA_TEST_XFERS);
+
+	KUNIT_EXPECT_EQ(test, 0, __spi_unmap_msg(ctx->ctlr, &ctx->msg));
+}
+
+/*
+ * xfer0 maps TX and RX; zero-length RX-only xfer1 then fails.
+ * The failure must unwind xfer0 without leaving either mapping flag set.
+ */
+static void spi_dma_later_rx_fail_rolls_back_earlier(struct kunit *test)
+{
+	struct spi_dma_test_ctx *ctx = spi_dma_test_ctx_new(test);
+	int ret;
+
+	ctx->xfer[0].tx_buf = spi_dma_test_buf(test, ctx, 0);
+	ctx->xfer[0].rx_buf = spi_dma_test_buf(test, ctx, 1);
+	ctx->xfer[0].len = SPI_DMA_TEST_LEN;
+
+	ctx->xfer[1].tx_buf = NULL;
+	ctx->xfer[1].rx_buf = spi_dma_test_buf(test, ctx, 2);
+	ctx->xfer[1].len = 0;			/* forces -EINVAL */
+
+	spi_message_add_tail(&ctx->xfer[0], &ctx->msg);
+	spi_message_add_tail(&ctx->xfer[1], &ctx->msg);
+
+	spi_dma_test_pin_stale_dma_devs(ctx);
+
+	ret = __spi_map_msg(ctx->ctlr, &ctx->msg);
+	KUNIT_ASSERT_EQ(test, ret, -EINVAL);
+
+	spi_dma_test_assert_dma_devs_published(test, ctx);
+	spi_dma_test_assert_nothing_mapped(test, ctx, SPI_DMA_TEST_XFERS);
+
+	KUNIT_EXPECT_EQ(test, 0, __spi_unmap_msg(ctx->ctlr, &ctx->msg));
+}
+
+/* Ensure the error unwind does not affect successful mappings. */
+static void spi_dma_map_success_publishes_dma_devs(struct kunit *test)
+{
+	struct spi_dma_test_ctx *ctx = spi_dma_test_ctx_new(test);
+	int ret;
+
+	ctx->xfer[0].tx_buf = spi_dma_test_buf(test, ctx, 0);
+	ctx->xfer[0].rx_buf = spi_dma_test_buf(test, ctx, 1);
+	ctx->xfer[0].len = SPI_DMA_TEST_LEN;
+
+	spi_message_add_tail(&ctx->xfer[0], &ctx->msg);
+
+	ret = __spi_map_msg(ctx->ctlr, &ctx->msg);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	KUNIT_EXPECT_TRUE(test, ctx->xfer[0].tx_sg_mapped);
+	KUNIT_EXPECT_TRUE(test, ctx->xfer[0].rx_sg_mapped);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->ctlr->cur_tx_dma_dev, ctx->dma_dev);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->ctlr->cur_rx_dma_dev, ctx->dma_dev);
+
+	KUNIT_EXPECT_EQ(test, 0, __spi_unmap_msg(ctx->ctlr, &ctx->msg));
+
+	KUNIT_EXPECT_FALSE(test, ctx->xfer[0].tx_sg_mapped);
+	KUNIT_EXPECT_FALSE(test, ctx->xfer[0].rx_sg_mapped);
+	KUNIT_EXPECT_NULL(test, ctx->xfer[0].tx_sg.sgl);
+	KUNIT_EXPECT_NULL(test, ctx->xfer[0].rx_sg.sgl);
+}
+
+/* A transfer without buffers requires no DMA mapping. */
+static void spi_dma_map_nothing_is_success(struct kunit *test)
+{
+	struct spi_dma_test_ctx *ctx = spi_dma_test_ctx_new(test);
+	int ret;
+
+	ctx->xfer[0].tx_buf = NULL;
+	ctx->xfer[0].rx_buf = NULL;
+	ctx->xfer[0].len = SPI_DMA_TEST_LEN;
+
+	spi_message_add_tail(&ctx->xfer[0], &ctx->msg);
+
+	ret = __spi_map_msg(ctx->ctlr, &ctx->msg);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+
+	spi_dma_test_assert_nothing_mapped(test, ctx, 1);
+}
+
+static struct kunit_case spi_dma_error_path_cases[] = {
+	KUNIT_CASE(spi_dma_later_tx_fail_rolls_back_earlier),
+	KUNIT_CASE(spi_dma_later_rx_fail_rolls_back_earlier),
+	KUNIT_CASE(spi_dma_map_success_publishes_dma_devs),
+	KUNIT_CASE(spi_dma_map_nothing_is_success),
+	{}
+};
+
+static struct kunit_suite spi_dma_error_path_suite = {
+	.name = "spi_dma",
+	.test_cases = spi_dma_error_path_cases,
+};
+
+kunit_test_suite(spi_dma_error_path_suite);
+
+MODULE_DESCRIPTION("KUnit tests for SPI core DMA mapping");
+MODULE_LICENSE("GPL");
diff --git a/tools/testing/kunit/configs/all_tests.config b/tools/testing/kunit/configs/all_tests.config
index bccc2c771..7bdcbdec2 100644
--- a/tools/testing/kunit/configs/all_tests.config
+++ b/tools/testing/kunit/configs/all_tests.config
@@ -21,6 +21,7 @@ CONFIG_VFAT_FS=y
 CONFIG_PCI=y
 CONFIG_USB4=y
 CONFIG_I2C=y
+CONFIG_SPI=y
 
 CONFIG_NET=y
 CONFIG_MCTP=y
diff --git a/tools/testing/kunit/configs/default.config b/tools/testing/kunit/configs/default.config
index e67af7b9f..2f24147c9 100644
--- a/tools/testing/kunit/configs/default.config
+++ b/tools/testing/kunit/configs/default.config
@@ -1,3 +1,4 @@
 CONFIG_KUNIT=y
 CONFIG_KUNIT_EXAMPLE_TEST=y
 CONFIG_KUNIT_ALL_TESTS=y
+CONFIG_SPI=y
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.