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
