2023-07-18 13:18:37

by Marco Pagani

[permalink] [raw]
Subject: [PATCH v10 0/4] fpga: add initial KUnit tests for the subsystem

This patch set introduces an initial set of KUnit test suites for the
core components of the FPGA subsystem.

Tests can be run using:
[user@localhost linux]$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/fpga/tests

v10:
- Improved code style and fixed comment typos for FPGA Manager ops

v9:
- Improved and commented test ops for the FPGA Manager test suite
- Fixed minor capitalization discrepancies

v8:
- Using memset to set header and payload of the test image
- Using sg_miter_* functions to check the image in the write_sg op
- Includes sorted in alphabetical order
- Add commas for fpga_bridge_ops
- Improved Makefile

v7:
- Dropped RFC prefix
- Add comments to explain fakes and helper functions
- Changed the implementation of the Bridge used in the Region suite

v6:
- Restructured the code into self-contained test modules
- Added tests for the basic behaviors of the components
- Improved programming tests for the FPGA Manager
- Fixed code/comments mismatch in the list of Bridges test case

v5:
- Removed most of the exported functions using shared buffers for stats
- Moved all KUnit expectations/assertions to the main test module
- Removed standalone use case to simplify the code
- Removed instances counters from fake components (using device.id instead)
- Set header size in the .parse_header op
- Improved bridge get_put_list test case

v4:
- Fix build error

v3:
- Calling fpga_bridges_put() between reconfigurations
- Functions for registering fake modules allocate and return context structs

v2:
- Restructured code into multiple suites to test components in isolation
- Reduced code duplication using init and exit methods
- Using a get_bridges() method to build the list of bridges just before programming
- Regions and Bridges are organized topologically
- Changed bitstream/bit to images
- Allocate images dynamically
- Renamed fpga-tests to fpga-test
- Simplified Kconfig
- Add license info to the fpga-test module

Marco Pagani (4):
fpga: add an initial KUnit suite for the FPGA Manager
fpga: add an initial KUnit suite for the FPGA Bridge
fpga: add an initial KUnit suite for the FPGA Region
fpga: add configuration for the FPGA KUnit test suites.

drivers/fpga/Kconfig | 2 +
drivers/fpga/Makefile | 3 +
drivers/fpga/tests/.kunitconfig | 5 +
drivers/fpga/tests/Kconfig | 11 +
drivers/fpga/tests/Makefile | 6 +
drivers/fpga/tests/fpga-bridge-test.c | 175 ++++++++++++++
drivers/fpga/tests/fpga-mgr-test.c | 327 ++++++++++++++++++++++++++
drivers/fpga/tests/fpga-region-test.c | 211 +++++++++++++++++
8 files changed, 740 insertions(+)
create mode 100644 drivers/fpga/tests/.kunitconfig
create mode 100644 drivers/fpga/tests/Kconfig
create mode 100644 drivers/fpga/tests/Makefile
create mode 100644 drivers/fpga/tests/fpga-bridge-test.c
create mode 100644 drivers/fpga/tests/fpga-mgr-test.c
create mode 100644 drivers/fpga/tests/fpga-region-test.c


base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
--
2.41.0



2023-07-18 13:22:25

by Marco Pagani

[permalink] [raw]
Subject: [PATCH v10 2/4] fpga: add an initial KUnit suite for the FPGA Bridge

The suite tests the basic behaviors of the FPGA Bridge including
the functions that operate on a list of bridges.

Signed-off-by: Marco Pagani <[email protected]>
Acked-by: Xu Yilun <[email protected]>
---
drivers/fpga/tests/fpga-bridge-test.c | 175 ++++++++++++++++++++++++++
1 file changed, 175 insertions(+)
create mode 100644 drivers/fpga/tests/fpga-bridge-test.c

diff --git a/drivers/fpga/tests/fpga-bridge-test.c b/drivers/fpga/tests/fpga-bridge-test.c
new file mode 100644
index 000000000000..1d258002cdd7
--- /dev/null
+++ b/drivers/fpga/tests/fpga-bridge-test.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for the FPGA Bridge
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <[email protected]>
+ */
+
+#include <kunit/test.h>
+#include <linux/device.h>
+#include <linux/fpga/fpga-bridge.h>
+#include <linux/module.h>
+#include <linux/types.h>
+
+struct bridge_stats {
+ bool enable;
+};
+
+struct bridge_ctx {
+ struct fpga_bridge *bridge;
+ struct platform_device *pdev;
+ struct bridge_stats stats;
+};
+
+static int op_enable_set(struct fpga_bridge *bridge, bool enable)
+{
+ struct bridge_stats *stats = bridge->priv;
+
+ stats->enable = enable;
+
+ return 0;
+}
+
+/*
+ * Fake FPGA bridge that implements only the enable_set op to track
+ * the state.
+ */
+static const struct fpga_bridge_ops fake_bridge_ops = {
+ .enable_set = op_enable_set,
+};
+
+/**
+ * register_test_bridge() - Register a fake FPGA bridge for testing.
+ * @test: KUnit test context object.
+ *
+ * Return: Context of the newly registered FPGA bridge.
+ */
+static struct bridge_ctx *register_test_bridge(struct kunit *test)
+{
+ struct bridge_ctx *ctx;
+
+ ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ ctx->pdev = platform_device_register_simple("bridge_pdev", PLATFORM_DEVID_AUTO, NULL, 0);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->pdev);
+
+ ctx->bridge = fpga_bridge_register(&ctx->pdev->dev, "Fake FPGA bridge", &fake_bridge_ops,
+ &ctx->stats);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->bridge));
+
+ return ctx;
+}
+
+static void unregister_test_bridge(struct bridge_ctx *ctx)
+{
+ fpga_bridge_unregister(ctx->bridge);
+ platform_device_unregister(ctx->pdev);
+}
+
+static void fpga_bridge_test_get(struct kunit *test)
+{
+ struct bridge_ctx *ctx = test->priv;
+ struct fpga_bridge *bridge;
+
+ bridge = fpga_bridge_get(&ctx->pdev->dev, NULL);
+ KUNIT_EXPECT_PTR_EQ(test, bridge, ctx->bridge);
+
+ bridge = fpga_bridge_get(&ctx->pdev->dev, NULL);
+ KUNIT_EXPECT_EQ(test, PTR_ERR(bridge), -EBUSY);
+
+ fpga_bridge_put(ctx->bridge);
+}
+
+static void fpga_bridge_test_toggle(struct kunit *test)
+{
+ struct bridge_ctx *ctx = test->priv;
+ int ret;
+
+ ret = fpga_bridge_disable(ctx->bridge);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_FALSE(test, ctx->stats.enable);
+
+ ret = fpga_bridge_enable(ctx->bridge);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_TRUE(test, ctx->stats.enable);
+}
+
+/* Test the functions for getting and controlling a list of bridges */
+static void fpga_bridge_test_get_put_list(struct kunit *test)
+{
+ struct list_head bridge_list;
+ struct bridge_ctx *ctx_0, *ctx_1;
+ int ret;
+
+ ctx_0 = test->priv;
+ ctx_1 = register_test_bridge(test);
+
+ INIT_LIST_HEAD(&bridge_list);
+
+ /* Get bridge 0 and add it to the list */
+ ret = fpga_bridge_get_to_list(&ctx_0->pdev->dev, NULL, &bridge_list);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_PTR_EQ(test, ctx_0->bridge,
+ list_first_entry_or_null(&bridge_list, struct fpga_bridge, node));
+
+ /* Get bridge 1 and add it to the list */
+ ret = fpga_bridge_get_to_list(&ctx_1->pdev->dev, NULL, &bridge_list);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_PTR_EQ(test, ctx_1->bridge,
+ list_first_entry_or_null(&bridge_list, struct fpga_bridge, node));
+
+ /* Disable an then enable both bridges from the list */
+ ret = fpga_bridges_disable(&bridge_list);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_FALSE(test, ctx_0->stats.enable);
+ KUNIT_EXPECT_FALSE(test, ctx_1->stats.enable);
+
+ ret = fpga_bridges_enable(&bridge_list);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_TRUE(test, ctx_0->stats.enable);
+ KUNIT_EXPECT_TRUE(test, ctx_1->stats.enable);
+
+ /* Put and remove both bridges from the list */
+ fpga_bridges_put(&bridge_list);
+
+ KUNIT_EXPECT_TRUE(test, list_empty(&bridge_list));
+
+ unregister_test_bridge(ctx_1);
+}
+
+static int fpga_bridge_test_init(struct kunit *test)
+{
+ test->priv = register_test_bridge(test);
+
+ return 0;
+}
+
+static void fpga_bridge_test_exit(struct kunit *test)
+{
+ unregister_test_bridge(test->priv);
+}
+
+static struct kunit_case fpga_bridge_test_cases[] = {
+ KUNIT_CASE(fpga_bridge_test_get),
+ KUNIT_CASE(fpga_bridge_test_toggle),
+ KUNIT_CASE(fpga_bridge_test_get_put_list),
+ {}
+};
+
+static struct kunit_suite fpga_bridge_suite = {
+ .name = "fpga_bridge",
+ .init = fpga_bridge_test_init,
+ .exit = fpga_bridge_test_exit,
+ .test_cases = fpga_bridge_test_cases,
+};
+
+kunit_test_suite(fpga_bridge_suite);
+
+MODULE_LICENSE("GPL");
--
2.41.0


2023-07-18 13:22:28

by Marco Pagani

[permalink] [raw]
Subject: [PATCH v10 4/4] fpga: add configuration for the FPGA KUnit test suites.

Add configuration for the KUnit test suites for the core components
of the FPGA subsystem.

Signed-off-by: Marco Pagani <[email protected]>
Acked-by: Xu Yilun <[email protected]>
---
drivers/fpga/Kconfig | 2 ++
drivers/fpga/Makefile | 3 +++
drivers/fpga/tests/.kunitconfig | 5 +++++
drivers/fpga/tests/Kconfig | 11 +++++++++++
drivers/fpga/tests/Makefile | 6 ++++++
5 files changed, 27 insertions(+)
create mode 100644 drivers/fpga/tests/.kunitconfig
create mode 100644 drivers/fpga/tests/Kconfig
create mode 100644 drivers/fpga/tests/Makefile

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 0a00763b9f28..2f689ac4ba3a 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -276,4 +276,6 @@ config FPGA_MGR_LATTICE_SYSCONFIG_SPI
FPGA manager driver support for Lattice FPGAs programming over slave
SPI sysCONFIG interface.

+source "drivers/fpga/tests/Kconfig"
+
endif # FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 72e554b4d2f7..352a2612623e 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -55,3 +55,6 @@ obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000) += dfl-n3000-nios.o

# Drivers for FPGAs which implement DFL
obj-$(CONFIG_FPGA_DFL_PCI) += dfl-pci.o
+
+# KUnit tests
+obj-$(CONFIG_FPGA_KUNIT_TESTS) += tests/
diff --git a/drivers/fpga/tests/.kunitconfig b/drivers/fpga/tests/.kunitconfig
new file mode 100644
index 000000000000..a1c2a2974c39
--- /dev/null
+++ b/drivers/fpga/tests/.kunitconfig
@@ -0,0 +1,5 @@
+CONFIG_KUNIT=y
+CONFIG_FPGA=y
+CONFIG_FPGA_REGION=y
+CONFIG_FPGA_BRIDGE=y
+CONFIG_FPGA_KUNIT_TESTS=y
diff --git a/drivers/fpga/tests/Kconfig b/drivers/fpga/tests/Kconfig
new file mode 100644
index 000000000000..e4a64815f16d
--- /dev/null
+++ b/drivers/fpga/tests/Kconfig
@@ -0,0 +1,11 @@
+config FPGA_KUNIT_TESTS
+ tristate "KUnit test for the FPGA subsystem" if !KUNIT_ALL_TESTS
+ depends on FPGA && FPGA_REGION && FPGA_BRIDGE && KUNIT=y
+ default KUNIT_ALL_TESTS
+ help
+ This builds unit tests for the FPGA subsystem
+
+ For more information on KUnit and unit tests in general,
+ please refer to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
diff --git a/drivers/fpga/tests/Makefile b/drivers/fpga/tests/Makefile
new file mode 100644
index 000000000000..bb78215c645c
--- /dev/null
+++ b/drivers/fpga/tests/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for KUnit test suites for the FPGA subsystem
+#
+
+obj-$(CONFIG_FPGA_KUNIT_TESTS) += fpga-mgr-test.o fpga-bridge-test.o fpga-region-test.o
--
2.41.0


2023-07-18 13:45:43

by Marco Pagani

[permalink] [raw]
Subject: [PATCH v10 1/4] fpga: add an initial KUnit suite for the FPGA Manager

The suite tests the basic behaviors of the FPGA Manager including
programming using a single contiguous buffer and a scatter gather table.

Signed-off-by: Marco Pagani <[email protected]>
---
drivers/fpga/tests/fpga-mgr-test.c | 327 +++++++++++++++++++++++++++++
1 file changed, 327 insertions(+)
create mode 100644 drivers/fpga/tests/fpga-mgr-test.c

diff --git a/drivers/fpga/tests/fpga-mgr-test.c b/drivers/fpga/tests/fpga-mgr-test.c
new file mode 100644
index 000000000000..6acec55b60ce
--- /dev/null
+++ b/drivers/fpga/tests/fpga-mgr-test.c
@@ -0,0 +1,327 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for the FPGA Manager
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Author: Marco Pagani <[email protected]>
+ */
+
+#include <kunit/test.h>
+#include <linux/device.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/module.h>
+#include <linux/scatterlist.h>
+#include <linux/types.h>
+
+#define HEADER_FILL 'H'
+#define IMAGE_FILL 'P'
+#define IMAGE_BLOCK 1024
+
+#define HEADER_SIZE IMAGE_BLOCK
+#define IMAGE_SIZE (IMAGE_BLOCK * 4)
+
+struct mgr_stats {
+ bool header_match;
+ bool image_match;
+ u32 seq_num;
+ u32 op_parse_header_seq;
+ u32 op_write_init_seq;
+ u32 op_write_seq;
+ u32 op_write_sg_seq;
+ u32 op_write_complete_seq;
+ enum fpga_mgr_states op_parse_header_state;
+ enum fpga_mgr_states op_write_init_state;
+ enum fpga_mgr_states op_write_state;
+ enum fpga_mgr_states op_write_sg_state;
+ enum fpga_mgr_states op_write_complete_state;
+};
+
+struct mgr_ctx {
+ struct fpga_image_info *img_info;
+ struct fpga_manager *mgr;
+ struct platform_device *pdev;
+ struct mgr_stats stats;
+};
+
+/**
+ * init_test_buffer() - Allocate and initialize a test image in a buffer.
+ * @test: KUnit test context object.
+ * @count: image size in bytes.
+ *
+ * Return: pointer to the newly allocated image.
+ */
+static char *init_test_buffer(struct kunit *test, size_t count)
+{
+ char *buf;
+
+ KUNIT_ASSERT_GE(test, count, HEADER_SIZE);
+
+ buf = kunit_kzalloc(test, count, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
+
+ memset(buf, HEADER_FILL, HEADER_SIZE);
+ memset(buf + HEADER_SIZE, IMAGE_FILL, count - HEADER_SIZE);
+
+ return buf;
+}
+
+/*
+ * Check the image header. Do not return an error code if the image check fails
+ * since, in this case, it is a failure of the FPGA manager itself, not this
+ * op that tests it.
+ */
+static int op_parse_header(struct fpga_manager *mgr, struct fpga_image_info *info,
+ const char *buf, size_t count)
+{
+ struct mgr_stats *stats = mgr->priv;
+ size_t i;
+
+ stats->op_parse_header_state = mgr->state;
+ stats->op_parse_header_seq = stats->seq_num++;
+
+ /* Set header_size and data_size for later */
+ info->header_size = HEADER_SIZE;
+ info->data_size = info->count - HEADER_SIZE;
+
+ stats->header_match = true;
+ for (i = 0; i < info->header_size; i++) {
+ if (buf[i] != HEADER_FILL) {
+ stats->header_match = false;
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static int op_write_init(struct fpga_manager *mgr, struct fpga_image_info *info,
+ const char *buf, size_t count)
+{
+ struct mgr_stats *stats = mgr->priv;
+
+ stats->op_write_init_state = mgr->state;
+ stats->op_write_init_seq = stats->seq_num++;
+
+ return 0;
+}
+
+/*
+ * Check the image data. As with op_parse_header, do not return an error code
+ * if the image check fails.
+ */
+static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
+{
+ struct mgr_stats *stats = mgr->priv;
+ size_t i;
+
+ stats->op_write_state = mgr->state;
+ stats->op_write_seq = stats->seq_num++;
+
+ stats->image_match = true;
+ for (i = 0; i < count; i++) {
+ if (buf[i] != IMAGE_FILL) {
+ stats->image_match = false;
+ break;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Check the image data, but first skip the header since write_sg will get
+ * the whole image in sg_table. As with op_parse_header, do not return an
+ * error code if the image check fails.
+ */
+static int op_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
+{
+ struct mgr_stats *stats = mgr->priv;
+ struct sg_mapping_iter miter;
+ char *img;
+ size_t i;
+
+ stats->op_write_sg_state = mgr->state;
+ stats->op_write_sg_seq = stats->seq_num++;
+
+ stats->image_match = true;
+ sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
+
+ if (!sg_miter_skip(&miter, HEADER_SIZE)) {
+ stats->image_match = false;
+ goto out;
+ }
+
+ while (sg_miter_next(&miter)) {
+ img = miter.addr;
+ for (i = 0; i < miter.length; i++) {
+ if (img[i] != IMAGE_FILL) {
+ stats->image_match = false;
+ goto out;
+ }
+ }
+ }
+out:
+ sg_miter_stop(&miter);
+ return 0;
+}
+
+static int op_write_complete(struct fpga_manager *mgr, struct fpga_image_info *info)
+{
+ struct mgr_stats *stats = mgr->priv;
+
+ stats->op_write_complete_state = mgr->state;
+ stats->op_write_complete_seq = stats->seq_num++;
+
+ return 0;
+}
+
+/*
+ * Fake FPGA manager that implements all ops required to check the programming
+ * sequence using a single contiguous buffer and a scatter gather table.
+ */
+static const struct fpga_manager_ops fake_mgr_ops = {
+ .skip_header = true,
+ .parse_header = op_parse_header,
+ .write_init = op_write_init,
+ .write = op_write,
+ .write_sg = op_write_sg,
+ .write_complete = op_write_complete,
+};
+
+static void fpga_mgr_test_get(struct kunit *test)
+{
+ struct mgr_ctx *ctx = test->priv;
+ struct fpga_manager *mgr;
+
+ mgr = fpga_mgr_get(&ctx->pdev->dev);
+ KUNIT_EXPECT_PTR_EQ(test, mgr, ctx->mgr);
+
+ fpga_mgr_put(ctx->mgr);
+}
+
+static void fpga_mgr_test_lock(struct kunit *test)
+{
+ struct mgr_ctx *ctx = test->priv;
+ int ret;
+
+ ret = fpga_mgr_lock(ctx->mgr);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ ret = fpga_mgr_lock(ctx->mgr);
+ KUNIT_EXPECT_EQ(test, ret, -EBUSY);
+
+ fpga_mgr_unlock(ctx->mgr);
+}
+
+/* Check the programming sequence using an image in a buffer */
+static void fpga_mgr_test_img_load_buf(struct kunit *test)
+{
+ struct mgr_ctx *ctx = test->priv;
+ char *img_buf;
+ int ret;
+
+ img_buf = init_test_buffer(test, IMAGE_SIZE);
+
+ ctx->img_info->count = IMAGE_SIZE;
+ ctx->img_info->buf = img_buf;
+
+ ret = fpga_mgr_load(ctx->mgr, ctx->img_info);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_TRUE(test, ctx->stats.header_match);
+ KUNIT_EXPECT_TRUE(test, ctx->stats.image_match);
+
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_parse_header_state, FPGA_MGR_STATE_PARSE_HEADER);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_state, FPGA_MGR_STATE_WRITE_INIT);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_state, FPGA_MGR_STATE_WRITE);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_state, FPGA_MGR_STATE_WRITE_COMPLETE);
+
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_seq, ctx->stats.op_parse_header_seq + 1);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_seq, ctx->stats.op_parse_header_seq + 2);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_seq, ctx->stats.op_parse_header_seq + 3);
+}
+
+/* Check the programming sequence using an image in a scatter gather table */
+static void fpga_mgr_test_img_load_sgt(struct kunit *test)
+{
+ struct mgr_ctx *ctx = test->priv;
+ struct sg_table *sgt;
+ char *img_buf;
+ int ret;
+
+ img_buf = init_test_buffer(test, IMAGE_SIZE);
+
+ sgt = kunit_kzalloc(test, sizeof(*sgt), GFP_KERNEL);
+ ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+ sg_init_one(sgt->sgl, img_buf, IMAGE_SIZE);
+
+ ctx->img_info->sgt = sgt;
+
+ ret = fpga_mgr_load(ctx->mgr, ctx->img_info);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_TRUE(test, ctx->stats.header_match);
+ KUNIT_EXPECT_TRUE(test, ctx->stats.image_match);
+
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_parse_header_state, FPGA_MGR_STATE_PARSE_HEADER);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_state, FPGA_MGR_STATE_WRITE_INIT);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_sg_state, FPGA_MGR_STATE_WRITE);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_state, FPGA_MGR_STATE_WRITE_COMPLETE);
+
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_seq, ctx->stats.op_parse_header_seq + 1);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_sg_seq, ctx->stats.op_parse_header_seq + 2);
+ KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_seq, ctx->stats.op_parse_header_seq + 3);
+
+ sg_free_table(ctx->img_info->sgt);
+}
+
+static int fpga_mgr_test_init(struct kunit *test)
+{
+ struct mgr_ctx *ctx;
+
+ ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ ctx->pdev = platform_device_register_simple("mgr_pdev", PLATFORM_DEVID_AUTO, NULL, 0);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->pdev);
+
+ ctx->mgr = devm_fpga_mgr_register(&ctx->pdev->dev, "Fake FPGA Manager", &fake_mgr_ops,
+ &ctx->stats);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->mgr));
+
+ ctx->img_info = fpga_image_info_alloc(&ctx->pdev->dev);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->img_info);
+
+ test->priv = ctx;
+
+ return 0;
+}
+
+static void fpga_mgr_test_exit(struct kunit *test)
+{
+ struct mgr_ctx *ctx = test->priv;
+
+ fpga_image_info_free(ctx->img_info);
+ platform_device_unregister(ctx->pdev);
+}
+
+static struct kunit_case fpga_mgr_test_cases[] = {
+ KUNIT_CASE(fpga_mgr_test_get),
+ KUNIT_CASE(fpga_mgr_test_lock),
+ KUNIT_CASE(fpga_mgr_test_img_load_buf),
+ KUNIT_CASE(fpga_mgr_test_img_load_sgt),
+ {}
+};
+
+static struct kunit_suite fpga_mgr_suite = {
+ .name = "fpga_mgr",
+ .init = fpga_mgr_test_init,
+ .exit = fpga_mgr_test_exit,
+ .test_cases = fpga_mgr_test_cases,
+};
+
+kunit_test_suite(fpga_mgr_suite);
+
+MODULE_LICENSE("GPL");
--
2.41.0


2023-07-23 07:58:24

by Xu Yilun

[permalink] [raw]
Subject: Re: [PATCH v10 1/4] fpga: add an initial KUnit suite for the FPGA Manager

On 2023-07-18 at 15:03:01 +0200, Marco Pagani wrote:
> The suite tests the basic behaviors of the FPGA Manager including
> programming using a single contiguous buffer and a scatter gather table.
>
> Signed-off-by: Marco Pagani <[email protected]>

Acked-by: Xu Yilun <[email protected]>

2023-07-23 08:04:47

by Xu Yilun

[permalink] [raw]
Subject: Re: [PATCH v10 0/4] fpga: add initial KUnit tests for the subsystem

On 2023-07-18 at 15:03:00 +0200, Marco Pagani wrote:
> This patch set introduces an initial set of KUnit test suites for the
> core components of the FPGA subsystem.

Applied this series.

Thanks,
Yilun

2023-09-25 01:31:19

by Jinjie Ruan

[permalink] [raw]
Subject: Re: [PATCH v10 2/4] fpga: add an initial KUnit suite for the FPGA Bridge



On 2023/7/18 21:03, Marco Pagani wrote:
> The suite tests the basic behaviors of the FPGA Bridge including
> the functions that operate on a list of bridges.
>
> Signed-off-by: Marco Pagani <[email protected]>
> Acked-by: Xu Yilun <[email protected]>
> ---
> drivers/fpga/tests/fpga-bridge-test.c | 175 ++++++++++++++++++++++++++
> 1 file changed, 175 insertions(+)
> create mode 100644 drivers/fpga/tests/fpga-bridge-test.c
>
> diff --git a/drivers/fpga/tests/fpga-bridge-test.c b/drivers/fpga/tests/fpga-bridge-test.c
> new file mode 100644
> index 000000000000..1d258002cdd7
> --- /dev/null
> +++ b/drivers/fpga/tests/fpga-bridge-test.c
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the FPGA Bridge
> + *
> + * Copyright (C) 2023 Red Hat, Inc.
> + *
> + * Author: Marco Pagani <[email protected]>
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/device.h>
> +#include <linux/fpga/fpga-bridge.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +
> +struct bridge_stats {
> + bool enable;
> +};
> +
> +struct bridge_ctx {
> + struct fpga_bridge *bridge;
> + struct platform_device *pdev;
> + struct bridge_stats stats;
> +};
> +
> +static int op_enable_set(struct fpga_bridge *bridge, bool enable)
> +{
> + struct bridge_stats *stats = bridge->priv;
> +
> + stats->enable = enable;
> +
> + return 0;
> +}
> +
> +/*
> + * Fake FPGA bridge that implements only the enable_set op to track
> + * the state.
> + */
> +static const struct fpga_bridge_ops fake_bridge_ops = {
> + .enable_set = op_enable_set,
> +};
> +
> +/**
> + * register_test_bridge() - Register a fake FPGA bridge for testing.
> + * @test: KUnit test context object.
> + *
> + * Return: Context of the newly registered FPGA bridge.
> + */
> +static struct bridge_ctx *register_test_bridge(struct kunit *test)
> +{
> + struct bridge_ctx *ctx;
> +
> + ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> + ctx->pdev = platform_device_register_simple("bridge_pdev", PLATFORM_DEVID_AUTO, NULL, 0);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->pdev);
> +
> + ctx->bridge = fpga_bridge_register(&ctx->pdev->dev, "Fake FPGA bridge", &fake_bridge_ops,
> + &ctx->stats);
> + KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->bridge));
> +
> + return ctx;
> +}
> +
> +static void unregister_test_bridge(struct bridge_ctx *ctx)
> +{
> + fpga_bridge_unregister(ctx->bridge);
> + platform_device_unregister(ctx->pdev);
> +}
> +
> +static void fpga_bridge_test_get(struct kunit *test)
> +{
> + struct bridge_ctx *ctx = test->priv;
> + struct fpga_bridge *bridge;
> +
> + bridge = fpga_bridge_get(&ctx->pdev->dev, NULL);
> + KUNIT_EXPECT_PTR_EQ(test, bridge, ctx->bridge);
> +
> + bridge = fpga_bridge_get(&ctx->pdev->dev, NULL);
> + KUNIT_EXPECT_EQ(test, PTR_ERR(bridge), -EBUSY);
> +
> + fpga_bridge_put(ctx->bridge);
> +}


modprobe fpga-bridge-test and then the below null-ptr-deref occurs.
It seems that ctx->pdev->dev->driver is NULL and the pdev->dev need
to be associated with a platform driver.

general protection fault, probably for non-canonical address
0xdffffc0000000002: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
CPU: 7 PID: 1864 Comm: kunit_try_catch Tainted: G N
6.6.0-rc2+ #49
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.15.0-1 04/01/2014
RIP: 0010:__fpga_bridge_get+0xca/0x160 [fpga_bridge]
Code: 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 86 00 00 00 48 b8 00
00 00 00 00 fc ff df 48 8b 5b 68 48 8d 7b 10 48 89 fa 48 c1 ea 03 <80>
3c 02 00 75 71 48 8b 7b 10 e8 a7 5e 1a e1 84 c0 74 34 66 90 48
RSP: 0018:ffff888106b0fe08 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff8409be0f
RDX: 0000000000000002 RSI: 0000000000000008 RDI: 0000000000000010
RBP: ffff8881011bc008 R08: 0000000000000001 R09: ffffed1020d61fb3
R10: ffff888106b0fd9f R11: ffff888106b0f850 R12: ffff8881011bc000
R13: ffff8881011bc2e8 R14: ffff888104b675a8 R15: ffff88810868b080
FS: 0000000000000000(0000) GS:ffff888119f80000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fad255ed8a0 CR3: 0000000005086006 CR4: 0000000000770ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<TASK>
? die_addr+0x3d/0xa0
? exc_general_protection+0x144/0x220
? asm_exc_general_protection+0x22/0x30
? mutex_trylock+0xcf/0x150
? __fpga_bridge_get+0xca/0x160 [fpga_bridge]
? __fpga_bridge_get+0x4e/0x160 [fpga_bridge]
fpga_bridge_test_get+0xb0/0x240 [fpga_bridge_test]
? _raw_spin_lock_irqsave+0x8d/0xe0
? op_enable_set+0x90/0x90 [fpga_bridge_test]
? __sched_text_end+0xa/0xa
? fpga_bridge_test_init+0x12/0x50 [fpga_bridge_test]
? kunit_try_run_case+0xdd/0x250
? kunit_try_run_case_cleanup+0xe0/0xe0
kunit_generic_run_threadfn_adapter+0x4a/0x90
? kunit_try_catch_throw+0x80/0x80
kthread+0x2b5/0x380
? kthread_complete_and_exit+0x20/0x20
ret_from_fork+0x2d/0x70
? kthread_complete_and_exit+0x20/0x20
ret_from_fork_asm+0x11/0x20
</TASK>
Modules linked in: fpga_bridge_test(+) fpga_bridge
Dumping ftrace buffer:
(ftrace buffer empty)
---[ end trace 0000000000000000 ]---
RIP: 0010:__fpga_bridge_get+0xca/0x160 [fpga_bridge]
Code: 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 86 00 00 00 48 b8 00
00 00 00 00 fc ff df 48 8b 5b 68 48 8d 7b 10 48 89 fa 48 c1 ea 03 <80>
3c 02 00 75 71 48 8b 7b 10 e8 a7 5e 1a e1 84 c0 74 34 66 90 48
RSP: 0018:ffff888106b0fe08 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff8409be0f
RDX: 0000000000000002 RSI: 0000000000000008 RDI: 0000000000000010
RBP: ffff8881011bc008 R08: 0000000000000001 R09: ffffed1020d61fb3
R10: ffff888106b0fd9f R11: ffff888106b0f850 R12: ffff8881011bc000
R13: ffff8881011bc2e8 R14: ffff888104b675a8 R15: ffff88810868b080
FS: 0000000000000000(0000) GS:ffff888119f80000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fad255ed8a0 CR3: 0000000005086006 CR4: 0000000000770ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Kernel panic - not syncing: Fatal exception
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 1 seconds..

Fixes: 9e6823481e5f ("fpga: add an initial KUnit suite for the FPGA
Bridge")
Reported-by: Jinjie Ruan <[email protected]>

> +
> +static void fpga_bridge_test_toggle(struct kunit *test)
> +{
> + struct bridge_ctx *ctx = test->priv;
> + int ret;
> +
> + ret = fpga_bridge_disable(ctx->bridge);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> + KUNIT_EXPECT_FALSE(test, ctx->stats.enable);
> +
> + ret = fpga_bridge_enable(ctx->bridge);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> + KUNIT_EXPECT_TRUE(test, ctx->stats.enable);
> +}
> +
> +/* Test the functions for getting and controlling a list of bridges */
> +static void fpga_bridge_test_get_put_list(struct kunit *test)
> +{
> + struct list_head bridge_list;
> + struct bridge_ctx *ctx_0, *ctx_1;
> + int ret;
> +
> + ctx_0 = test->priv;
> + ctx_1 = register_test_bridge(test);
> +
> + INIT_LIST_HEAD(&bridge_list);
> +
> + /* Get bridge 0 and add it to the list */
> + ret = fpga_bridge_get_to_list(&ctx_0->pdev->dev, NULL, &bridge_list);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + KUNIT_EXPECT_PTR_EQ(test, ctx_0->bridge,
> + list_first_entry_or_null(&bridge_list, struct fpga_bridge, node));
> +
> + /* Get bridge 1 and add it to the list */
> + ret = fpga_bridge_get_to_list(&ctx_1->pdev->dev, NULL, &bridge_list);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + KUNIT_EXPECT_PTR_EQ(test, ctx_1->bridge,
> + list_first_entry_or_null(&bridge_list, struct fpga_bridge, node));
> +
> + /* Disable an then enable both bridges from the list */
> + ret = fpga_bridges_disable(&bridge_list);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + KUNIT_EXPECT_FALSE(test, ctx_0->stats.enable);
> + KUNIT_EXPECT_FALSE(test, ctx_1->stats.enable);
> +
> + ret = fpga_bridges_enable(&bridge_list);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + KUNIT_EXPECT_TRUE(test, ctx_0->stats.enable);
> + KUNIT_EXPECT_TRUE(test, ctx_1->stats.enable);
> +
> + /* Put and remove both bridges from the list */
> + fpga_bridges_put(&bridge_list);
> +
> + KUNIT_EXPECT_TRUE(test, list_empty(&bridge_list));
> +
> + unregister_test_bridge(ctx_1);
> +}
> +
> +static int fpga_bridge_test_init(struct kunit *test)
> +{
> + test->priv = register_test_bridge(test);
> +
> + return 0;
> +}
> +
> +static void fpga_bridge_test_exit(struct kunit *test)
> +{
> + unregister_test_bridge(test->priv);
> +}
> +
> +static struct kunit_case fpga_bridge_test_cases[] = {
> + KUNIT_CASE(fpga_bridge_test_get),
> + KUNIT_CASE(fpga_bridge_test_toggle),
> + KUNIT_CASE(fpga_bridge_test_get_put_list),
> + {}
> +};
> +
> +static struct kunit_suite fpga_bridge_suite = {
> + .name = "fpga_bridge",
> + .init = fpga_bridge_test_init,
> + .exit = fpga_bridge_test_exit,
> + .test_cases = fpga_bridge_test_cases,
> +};
> +
> +kunit_test_suite(fpga_bridge_suite);
> +
> +MODULE_LICENSE("GPL");

2023-09-25 01:37:19

by Jinjie Ruan

[permalink] [raw]
Subject: Re: [PATCH v10 1/4] fpga: add an initial KUnit suite for the FPGA Manager



On 1970/1/1 8:00, Marco Pagani wrote:
> The suite tests the basic behaviors of the FPGA Manager including
> programming using a single contiguous buffer and a scatter gather table.
>
> Signed-off-by: Marco Pagani <[email protected]>
> ---
> drivers/fpga/tests/fpga-mgr-test.c | 327 +++++++++++++++++++++++++++++
> 1 file changed, 327 insertions(+)
> create mode 100644 drivers/fpga/tests/fpga-mgr-test.c
>
> diff --git a/drivers/fpga/tests/fpga-mgr-test.c b/drivers/fpga/tests/fpga-mgr-test.c
> new file mode 100644
> index 000000000000..6acec55b60ce
> --- /dev/null
> +++ b/drivers/fpga/tests/fpga-mgr-test.c
> @@ -0,0 +1,327 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the FPGA Manager
> + *
> + * Copyright (C) 2023 Red Hat, Inc.
> + *
> + * Author: Marco Pagani <[email protected]>
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/device.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/module.h>
> +#include <linux/scatterlist.h>
> +#include <linux/types.h>
> +
> +#define HEADER_FILL 'H'
> +#define IMAGE_FILL 'P'
> +#define IMAGE_BLOCK 1024
> +
> +#define HEADER_SIZE IMAGE_BLOCK
> +#define IMAGE_SIZE (IMAGE_BLOCK * 4)
> +
> +struct mgr_stats {
> + bool header_match;
> + bool image_match;
> + u32 seq_num;
> + u32 op_parse_header_seq;
> + u32 op_write_init_seq;
> + u32 op_write_seq;
> + u32 op_write_sg_seq;
> + u32 op_write_complete_seq;
> + enum fpga_mgr_states op_parse_header_state;
> + enum fpga_mgr_states op_write_init_state;
> + enum fpga_mgr_states op_write_state;
> + enum fpga_mgr_states op_write_sg_state;
> + enum fpga_mgr_states op_write_complete_state;
> +};
> +
> +struct mgr_ctx {
> + struct fpga_image_info *img_info;
> + struct fpga_manager *mgr;
> + struct platform_device *pdev;
> + struct mgr_stats stats;
> +};
> +
> +/**
> + * init_test_buffer() - Allocate and initialize a test image in a buffer.
> + * @test: KUnit test context object.
> + * @count: image size in bytes.
> + *
> + * Return: pointer to the newly allocated image.
> + */
> +static char *init_test_buffer(struct kunit *test, size_t count)
> +{
> + char *buf;
> +
> + KUNIT_ASSERT_GE(test, count, HEADER_SIZE);
> +
> + buf = kunit_kzalloc(test, count, GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
> +
> + memset(buf, HEADER_FILL, HEADER_SIZE);
> + memset(buf + HEADER_SIZE, IMAGE_FILL, count - HEADER_SIZE);
> +
> + return buf;
> +}
> +
> +/*
> + * Check the image header. Do not return an error code if the image check fails
> + * since, in this case, it is a failure of the FPGA manager itself, not this
> + * op that tests it.
> + */
> +static int op_parse_header(struct fpga_manager *mgr, struct fpga_image_info *info,
> + const char *buf, size_t count)
> +{
> + struct mgr_stats *stats = mgr->priv;
> + size_t i;
> +
> + stats->op_parse_header_state = mgr->state;
> + stats->op_parse_header_seq = stats->seq_num++;
> +
> + /* Set header_size and data_size for later */
> + info->header_size = HEADER_SIZE;
> + info->data_size = info->count - HEADER_SIZE;
> +
> + stats->header_match = true;
> + for (i = 0; i < info->header_size; i++) {
> + if (buf[i] != HEADER_FILL) {
> + stats->header_match = false;
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int op_write_init(struct fpga_manager *mgr, struct fpga_image_info *info,
> + const char *buf, size_t count)
> +{
> + struct mgr_stats *stats = mgr->priv;
> +
> + stats->op_write_init_state = mgr->state;
> + stats->op_write_init_seq = stats->seq_num++;
> +
> + return 0;
> +}
> +
> +/*
> + * Check the image data. As with op_parse_header, do not return an error code
> + * if the image check fails.
> + */
> +static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> + struct mgr_stats *stats = mgr->priv;
> + size_t i;
> +
> + stats->op_write_state = mgr->state;
> + stats->op_write_seq = stats->seq_num++;
> +
> + stats->image_match = true;
> + for (i = 0; i < count; i++) {
> + if (buf[i] != IMAGE_FILL) {
> + stats->image_match = false;
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Check the image data, but first skip the header since write_sg will get
> + * the whole image in sg_table. As with op_parse_header, do not return an
> + * error code if the image check fails.
> + */
> +static int op_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
> +{
> + struct mgr_stats *stats = mgr->priv;
> + struct sg_mapping_iter miter;
> + char *img;
> + size_t i;
> +
> + stats->op_write_sg_state = mgr->state;
> + stats->op_write_sg_seq = stats->seq_num++;
> +
> + stats->image_match = true;
> + sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
> +
> + if (!sg_miter_skip(&miter, HEADER_SIZE)) {
> + stats->image_match = false;
> + goto out;
> + }
> +
> + while (sg_miter_next(&miter)) {
> + img = miter.addr;
> + for (i = 0; i < miter.length; i++) {
> + if (img[i] != IMAGE_FILL) {
> + stats->image_match = false;
> + goto out;
> + }
> + }
> + }
> +out:
> + sg_miter_stop(&miter);
> + return 0;
> +}
> +
> +static int op_write_complete(struct fpga_manager *mgr, struct fpga_image_info *info)
> +{
> + struct mgr_stats *stats = mgr->priv;
> +
> + stats->op_write_complete_state = mgr->state;
> + stats->op_write_complete_seq = stats->seq_num++;
> +
> + return 0;
> +}
> +
> +/*
> + * Fake FPGA manager that implements all ops required to check the programming
> + * sequence using a single contiguous buffer and a scatter gather table.
> + */
> +static const struct fpga_manager_ops fake_mgr_ops = {
> + .skip_header = true,
> + .parse_header = op_parse_header,
> + .write_init = op_write_init,
> + .write = op_write,
> + .write_sg = op_write_sg,
> + .write_complete = op_write_complete,
> +};
> +
> +static void fpga_mgr_test_get(struct kunit *test)
> +{
> + struct mgr_ctx *ctx = test->priv;
> + struct fpga_manager *mgr;
> +
> + mgr = fpga_mgr_get(&ctx->pdev->dev);
> + KUNIT_EXPECT_PTR_EQ(test, mgr, ctx->mgr);


modprobe fpga-mgr-test and then the below null-ptr-deref occurs.
It seems that ctx->pdev->dev->driver is NULL and the pdev->dev need
to be associated with a platform driver.

general protection fault, probably for non-canonical address
0xdffffc0000000002: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
CPU: 4 PID: 1866 Comm: kunit_try_catch Tainted: G N
6.6.0-rc2+ #49
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.15.0-1 04/01/2014
RIP: 0010:fpga_mgr_get+0x87/0xd0 [fpga_mgr]
Code: 48 8d 7b 68 48 89 fa 48 c1 ea 03 80 3c 02 00 75 59 48 b8 00
00 00 00 00 fc ff df 48 8b 5b 68 48 8d 7b 10 48 89 fa 48 c1 ea 03 <80>
3c 02 00 75 33 48 8b 7b 10 e8 7a 43 1a e1 84 c0 74 08 4c 89 e0
RSP: 0018:ffff8881054cfe18 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff83f6a96c
RDX: 0000000000000002 RSI: 0000000000000004 RDI: 0000000000000010
RBP: ffff88810d185808 R08: 0000000000000001 R09: ffffed1020d107ca
R10: ffff888106883e53 R11: ffff8881054cfa50 R12: ffff88810d185800
R13: ffff888106408520 R14: ffff88810683f5a8 R15: ffff888105e4b080
FS: 0000000000000000(0000) GS:ffff888119e00000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f537e5ed8a0 CR3: 0000000005086001 CR4: 0000000000770ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<TASK>
? die_addr+0x3d/0xa0
? exc_general_protection+0x144/0x220
? asm_exc_general_protection+0x22/0x30
? kobject_put+0x5c/0x310
? fpga_mgr_get+0x87/0xd0 [fpga_mgr]
? fpga_mgr_get+0x28/0xd0 [fpga_mgr]
fpga_mgr_test_get+0xb4/0x1b0 [fpga_mgr_test]
? platform_device_register_resndata.constprop.0+0xc0/0xc0
[fpga_mgr_test]
? fpga_mgr_test_lock+0x1f0/0x1f0 [fpga_mgr_test]
? __sched_text_end+0xa/0xa
? kunit_try_run_case+0xdd/0x250
? kunit_try_run_case_cleanup+0xe0/0xe0
kunit_generic_run_threadfn_adapter+0x4a/0x90
? kunit_try_catch_throw+0x80/0x80
kthread+0x2b5/0x380
? kthread_complete_and_exit+0x20/0x20
ret_from_fork+0x2d/0x70
? kthread_complete_and_exit+0x20/0x20
ret_from_fork_asm+0x11/0x20
</TASK>
Modules linked in: fpga_mgr_test(+) fpga_mgr
Dumping ftrace buffer:
(ftrace buffer empty)
---[ end trace 0000000000000000 ]---
RIP: 0010:fpga_mgr_get+0x87/0xd0 [fpga_mgr]
Code: 48 8d 7b 68 48 89 fa 48 c1 ea 03 80 3c 02 00 75 59 48 b8 00
00 00 00 00 fc ff df 48 8b 5b 68 48 8d 7b 10 48 89 fa 48 c1 ea 03 <80>
3c 02 00 75 33 48 8b 7b 10 e8 7a 43 1a e1 84 c0 74 08 4c 89 e0
RSP: 0018:ffff8881054cfe18 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff83f6a96c
RDX: 0000000000000002 RSI: 0000000000000004 RDI: 0000000000000010
RBP: ffff88810d185808 R08: 0000000000000001 R09: ffffed1020d107ca
R10: ffff888106883e53 R11: ffff8881054cfa50 R12: ffff88810d185800
R13: ffff888106408520 R14: ffff88810683f5a8 R15: ffff888105e4b080
FS: 0000000000000000(0000) GS:ffff888119e00000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f537e5ed8a0 CR3: 0000000005086001 CR4: 0000000000770ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Kernel panic - not syncing: Fatal exception
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 1 seconds..

Fixes: ccbc1c302115 ("fpga: add an initial KUnit suite for the FPGA
Manager")
Reported-by: Jinjie Ruan <[email protected]>

> +
> + fpga_mgr_put(ctx->mgr);
> +}
> +
> +static void fpga_mgr_test_lock(struct kunit *test)
> +{
> + struct mgr_ctx *ctx = test->priv;
> + int ret;
> +
> + ret = fpga_mgr_lock(ctx->mgr);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + ret = fpga_mgr_lock(ctx->mgr);
> + KUNIT_EXPECT_EQ(test, ret, -EBUSY);
> +
> + fpga_mgr_unlock(ctx->mgr);
> +}
> +
> +/* Check the programming sequence using an image in a buffer */
> +static void fpga_mgr_test_img_load_buf(struct kunit *test)
> +{
> + struct mgr_ctx *ctx = test->priv;
> + char *img_buf;
> + int ret;
> +
> + img_buf = init_test_buffer(test, IMAGE_SIZE);
> +
> + ctx->img_info->count = IMAGE_SIZE;
> + ctx->img_info->buf = img_buf;
> +
> + ret = fpga_mgr_load(ctx->mgr, ctx->img_info);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + KUNIT_EXPECT_TRUE(test, ctx->stats.header_match);
> + KUNIT_EXPECT_TRUE(test, ctx->stats.image_match);
> +
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_parse_header_state, FPGA_MGR_STATE_PARSE_HEADER);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_state, FPGA_MGR_STATE_WRITE_INIT);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_state, FPGA_MGR_STATE_WRITE);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_state, FPGA_MGR_STATE_WRITE_COMPLETE);
> +
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_seq, ctx->stats.op_parse_header_seq + 1);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_seq, ctx->stats.op_parse_header_seq + 2);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_seq, ctx->stats.op_parse_header_seq + 3);
> +}
> +
> +/* Check the programming sequence using an image in a scatter gather table */
> +static void fpga_mgr_test_img_load_sgt(struct kunit *test)
> +{
> + struct mgr_ctx *ctx = test->priv;
> + struct sg_table *sgt;
> + char *img_buf;
> + int ret;
> +
> + img_buf = init_test_buffer(test, IMAGE_SIZE);
> +
> + sgt = kunit_kzalloc(test, sizeof(*sgt), GFP_KERNEL);
> + ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> + sg_init_one(sgt->sgl, img_buf, IMAGE_SIZE);
> +
> + ctx->img_info->sgt = sgt;
> +
> + ret = fpga_mgr_load(ctx->mgr, ctx->img_info);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + KUNIT_EXPECT_TRUE(test, ctx->stats.header_match);
> + KUNIT_EXPECT_TRUE(test, ctx->stats.image_match);
> +
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_parse_header_state, FPGA_MGR_STATE_PARSE_HEADER);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_state, FPGA_MGR_STATE_WRITE_INIT);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_sg_state, FPGA_MGR_STATE_WRITE);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_state, FPGA_MGR_STATE_WRITE_COMPLETE);
> +
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_init_seq, ctx->stats.op_parse_header_seq + 1);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_sg_seq, ctx->stats.op_parse_header_seq + 2);
> + KUNIT_EXPECT_EQ(test, ctx->stats.op_write_complete_seq, ctx->stats.op_parse_header_seq + 3);
> +
> + sg_free_table(ctx->img_info->sgt);
> +}
> +
> +static int fpga_mgr_test_init(struct kunit *test)
> +{
> + struct mgr_ctx *ctx;
> +
> + ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> + ctx->pdev = platform_device_register_simple("mgr_pdev", PLATFORM_DEVID_AUTO, NULL, 0);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->pdev);
> +
> + ctx->mgr = devm_fpga_mgr_register(&ctx->pdev->dev, "Fake FPGA Manager", &fake_mgr_ops,
> + &ctx->stats);
> + KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(ctx->mgr));
> +
> + ctx->img_info = fpga_image_info_alloc(&ctx->pdev->dev);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->img_info);
> +
> + test->priv = ctx;
> +
> + return 0;
> +}
> +
> +static void fpga_mgr_test_exit(struct kunit *test)
> +{
> + struct mgr_ctx *ctx = test->priv;
> +
> + fpga_image_info_free(ctx->img_info);
> + platform_device_unregister(ctx->pdev);
> +}
> +
> +static struct kunit_case fpga_mgr_test_cases[] = {
> + KUNIT_CASE(fpga_mgr_test_get),
> + KUNIT_CASE(fpga_mgr_test_lock),
> + KUNIT_CASE(fpga_mgr_test_img_load_buf),
> + KUNIT_CASE(fpga_mgr_test_img_load_sgt),
> + {}
> +};
> +
> +static struct kunit_suite fpga_mgr_suite = {
> + .name = "fpga_mgr",
> + .init = fpga_mgr_test_init,
> + .exit = fpga_mgr_test_exit,
> + .test_cases = fpga_mgr_test_cases,
> +};
> +
> +kunit_test_suite(fpga_mgr_suite);
> +
> +MODULE_LICENSE("GPL");

2023-09-25 19:23:12

by Marco Pagani

[permalink] [raw]
Subject: Re: [PATCH v10 1/4] fpga: add an initial KUnit suite for the FPGA Manager



On 2023-09-25 03:36, Ruan Jinjie wrote:
>
>
> On 1970/1/1 8:00, Marco Pagani wrote:
>> The suite tests the basic behaviors of the FPGA Manager including
>> programming using a single contiguous buffer and a scatter gather table.
>>
>> Signed-off-by: Marco Pagani <[email protected]>

[...]

> modprobe fpga-mgr-test and then the below null-ptr-deref occurs.
> It seems that ctx->pdev->dev->driver is NULL and the pdev->dev need
> to be associated with a platform driver.

Hi, thanks for reporting the issue. I had not noticed it since it does not
happen when running the tests on UML. I will send a fix soon.

Marco


> general protection fault, probably for non-canonical address
> 0xdffffc0000000002: 0000 [#1] PREEMPT SMP KASAN
> KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
> CPU: 4 PID: 1866 Comm: kunit_try_catch Tainted: G N
> 6.6.0-rc2+ #49
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.15.0-1 04/01/2014
> RIP: 0010:fpga_mgr_get+0x87/0xd0 [fpga_mgr]
> Code: 48 8d 7b 68 48 89 fa 48 c1 ea 03 80 3c 02 00 75 59 48 b8 00
> 00 00 00 00 fc ff df 48 8b 5b 68 48 8d 7b 10 48 89 fa 48 c1 ea 03 <80>
> 3c 02 00 75 33 48 8b 7b 10 e8 7a 43 1a e1 84 c0 74 08 4c 89 e0
> RSP: 0018:ffff8881054cfe18 EFLAGS: 00010202
> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff83f6a96c
> RDX: 0000000000000002 RSI: 0000000000000004 RDI: 0000000000000010
> RBP: ffff88810d185808 R08: 0000000000000001 R09: ffffed1020d107ca
> R10: ffff888106883e53 R11: ffff8881054cfa50 R12: ffff88810d185800
> R13: ffff888106408520 R14: ffff88810683f5a8 R15: ffff888105e4b080
> FS: 0000000000000000(0000) GS:ffff888119e00000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f537e5ed8a0 CR3: 0000000005086001 CR4: 0000000000770ee0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 55555554
> Call Trace:
> <TASK>
> ? die_addr+0x3d/0xa0
> ? exc_general_protection+0x144/0x220
> ? asm_exc_general_protection+0x22/0x30
> ? kobject_put+0x5c/0x310
> ? fpga_mgr_get+0x87/0xd0 [fpga_mgr]
> ? fpga_mgr_get+0x28/0xd0 [fpga_mgr]
> fpga_mgr_test_get+0xb4/0x1b0 [fpga_mgr_test]
> ? platform_device_register_resndata.constprop.0+0xc0/0xc0
> [fpga_mgr_test]
> ? fpga_mgr_test_lock+0x1f0/0x1f0 [fpga_mgr_test]
> ? __sched_text_end+0xa/0xa
> ? kunit_try_run_case+0xdd/0x250
> ? kunit_try_run_case_cleanup+0xe0/0xe0
> kunit_generic_run_threadfn_adapter+0x4a/0x90
> ? kunit_try_catch_throw+0x80/0x80
> kthread+0x2b5/0x380
> ? kthread_complete_and_exit+0x20/0x20
> ret_from_fork+0x2d/0x70
> ? kthread_complete_and_exit+0x20/0x20
> ret_from_fork_asm+0x11/0x20
> </TASK>
> Modules linked in: fpga_mgr_test(+) fpga_mgr
> Dumping ftrace buffer:
> (ftrace buffer empty)
> ---[ end trace 0000000000000000 ]---
> RIP: 0010:fpga_mgr_get+0x87/0xd0 [fpga_mgr]
> Code: 48 8d 7b 68 48 89 fa 48 c1 ea 03 80 3c 02 00 75 59 48 b8 00
> 00 00 00 00 fc ff df 48 8b 5b 68 48 8d 7b 10 48 89 fa 48 c1 ea 03 <80>
> 3c 02 00 75 33 48 8b 7b 10 e8 7a 43 1a e1 84 c0 74 08 4c 89 e0
> RSP: 0018:ffff8881054cfe18 EFLAGS: 00010202
> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff83f6a96c
> RDX: 0000000000000002 RSI: 0000000000000004 RDI: 0000000000000010
> RBP: ffff88810d185808 R08: 0000000000000001 R09: ffffed1020d107ca
> R10: ffff888106883e53 R11: ffff8881054cfa50 R12: ffff88810d185800
> R13: ffff888106408520 R14: ffff88810683f5a8 R15: ffff888105e4b080
> FS: 0000000000000000(0000) GS:ffff888119e00000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f537e5ed8a0 CR3: 0000000005086001 CR4: 0000000000770ee0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 55555554
> Kernel panic - not syncing: Fatal exception
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Kernel Offset: disabled
> Rebooting in 1 seconds..
>
> Fixes: ccbc1c302115 ("fpga: add an initial KUnit suite for the FPGA
> Manager")
> Reported-by: Jinjie Ruan <[email protected]>
>

[...]