[RFC PATCH 0/7] platform/chrome: Add Kunit tests for protocol device drivers

[RFC PATCH 0/7] platform/chrome: Add Kunit tests for protocol device drivers

[Tzung-Bi Shih]

The protocol device drivers under drivers/platform/chrome/ are responsible
to communicate to the ChromeOS EC (Embedded Controller).  They need to pack
the data in a pre-defined format and check if the EC responds accordingly.

The series adds some fundamental unit tests for the protocol.  It calls the
.cmd_xfer() and .pkt_xfer() callbacks (which are the most crucial parts for
the protocol), mocks the rest of the system, and checks if the interactions
are all correct.

The series isn't ready for landing.  It's more like a PoC for the
binary-level function redirection and its use cases.

The 1st patch adds ftrace stub which is originally from [1][2].  There is no
follow-up discussion about the ftrace stub.  As a result, the patch is still
on the mailing list.

The 2nd patch adds Kunit tests for cros_ec_i2c.  It relies on the ftrace stub
for redirecting cros_ec_{un,}register().

The 3rd patch uses static stub instead (if ftrace stub isn't really an option).
However, I'm not a big fan to change the production code (i.e. adding the
prologue in cros_ec_{un,}register()) for testing.

The 4th patch adds Kunit tests for cros_ec_spi.  It relies on the ftrace stub
for redirecting cros_ec_{un,}register() again.

The 5th patch calls .probe() directly instead of forcing the driver probe
needs to be synchronous.  In comparison with the 4th patch, I don't think
this is simpler.  I'd prefer to the way in the 4th patch.

After talked to Masami about the work, he suggested to use Kprobes for
function redirection.  The 6th patch adds kprobes stub.

The 7th patch uses kprobes stub instead for cros_ec_spi.

Questions:
- Are we going to support ftrace stub so that tests can use it?

- If ftrace stub isn't on the plate (e.g. due to too many dependencies), how
  about the kprobes stub?  Is it something we could pursue?

- (minor) I'm unsure if people would prefer 'kprobes stub' vs. 'kprobe stub'.

[1]: https://kunit.dev/mocking.html#binary-level-ftrace-et-al
[2]: https://lore.kernel.org/linux-kselftest/20220318021314.3225240-3-davidgow@google.com/

Daniel Latypov (1):
  kunit: expose ftrace-based API for stubbing out functions during tests

Tzung-Bi Shih (6):
  platform/chrome: kunit: cros_ec_i2c: Add tests with ftrace stub
  platform/chrome: kunit: cros_ec_i2c: Use static stub instead
  platform/chrome: kunit: cros_ec_spi: Add tests with ftrace stub
  platform/chrome: kunit: cros_ec_spi: Call .probe() directly
  kunit: Expose 'kprobes stub' API to redirect functions
  platform/chrome: kunit: cros_ec_spi: Use kprobes stub instead

 drivers/platform/chrome/Kconfig            |  17 +
 drivers/platform/chrome/Makefile           |   2 +
 drivers/platform/chrome/cros_ec.c          |   6 +
 drivers/platform/chrome/cros_ec_i2c_test.c | 479 +++++++++++++++++++++
 drivers/platform/chrome/cros_ec_spi_test.c | 355 +++++++++++++++
 include/kunit/ftrace_stub.h                |  84 ++++
 include/kunit/kprobes_stub.h               |  19 +
 kernel/trace/ftrace.c                      |   3 +
 lib/kunit/Kconfig                          |  18 +
 lib/kunit/Makefile                         |   8 +
 lib/kunit/ftrace_stub.c                    | 139 ++++++
 lib/kunit/kprobes_stub.c                   | 113 +++++
 lib/kunit/kunit-example-test.c             |  27 +-
 lib/kunit/stubs_example.kunitconfig        |  11 +
 14 files changed, 1280 insertions(+), 1 deletion(-)
 create mode 100644 drivers/platform/chrome/cros_ec_i2c_test.c
 create mode 100644 drivers/platform/chrome/cros_ec_spi_test.c
 create mode 100644 include/kunit/ftrace_stub.h
 create mode 100644 include/kunit/kprobes_stub.h
 create mode 100644 lib/kunit/ftrace_stub.c
 create mode 100644 lib/kunit/kprobes_stub.c
 create mode 100644 lib/kunit/stubs_example.kunitconfig

-- 
2.49.0.1101.gccaa498523-goog

[RFC PATCH 1/7] kunit: expose ftrace-based API for stubbing out functions during tests

[Tzung-Bi Shih]

From: Daniel Latypov <dlatypov@google.com>

Allow function redirection using ftrace and kernel livepatch. This is
basically equivalent to the static_stub support in the previous patch,
but does not require the function being replaced to be modified (save
for the addition of KUNIT_STUBBABLE/noinline).

This is hidden behind the CONFIG_KUNIT_FTRACE_STUBS option, and has a
number of dependencies, including ftrace, livepatch and
CONFIG_KALLSYMS_ALL. As a result, it only works on architectures where
these are available.

You can run the KUnit example tests with the following:
$ ./tools/testing/kunit/kunit.py run --kunitconfig lib/kunit/stubs_example.kunitconfig --arch=x86_64

To the end user, replacing a function is very simple, e.g.
  KUNIT_STUBBABLE void real_func(int n);
  void replacement_func(int n);

  /* in tests */
  kunit_activate_ftrace_stub(test, real_func, replacement_func);

The implementation is inspired by Steven's snippet here [1].

Some more details:
* stubbing is automatically undone at the end of tests
* it can also be manually undone with kunit_deactive_ftrace_stub()
* stubbing only applies when current->kunit_test == test
  * note: currently can't have more than one test running at a time
* KUNIT_STUBBABLE marks tests as noinline when CONFIG_KUNIT_STUBS is set
  * this ensures we can actually stub all calls
* KUNIT_STUBBABLE_TRAMPOLINE is a version that evaluates to
  __always_inline when stubbing is not enabled
  * This may need to be used with a wrapper function.
  * See the doc comment for more details.

Sharp-edges:
* kernel livepatch only works on some arches (not UML)
* if you don't use noinline/KUNIT_STUBBABLE, functions might be inlined
  and thus none of this works:
  * if it's always inlined, at least the attempt to stub will fail
  * if it's sometimes inlined, then the stub silently won't work

[1] https://lore.kernel.org/lkml/20220224091550.2b7e8784@gandalf.local.home
[2] https://lore.kernel.org/linux-kselftest/CAGS_qxqtMpjKX+CMF6eBUWfsc-TKR9-Bk+hYM=PHa0_wUtQQuA@mail.gmail.com/

Co-developed-by: David Gow <davidgow@google.com>
Signed-off-by: David Gow <davidgow@google.com>
Signed-off-by: Daniel Latypov <dlatypov@google.com>
[tzungbi:
  * Resolve contextual conflicts for rebasing.
  * klp_arch_set_pc() -> ftrace_regs_set_instruction_pointer().
  * Fix type check in kunit_activate_ftrace_stub() just like what has
    been done in kunit_activate_static_stub().
  * Include <kunit/ftrace_stub.h> in lib/kunit/ftrace_stub.c.
  * Fix typo in include/kunit/ftrace_stub.h as pointed out in [2].
    * eavlautes -> evaluates.
    * active -> activate.
  * Fix typo in kunit-example-test.c and ftrace_stub.c.
    * static -> ftrace.
  * EXPORT_SYMBOL_IF_KUNIT() for ftrace_location() as
    lib/kunit/ftrace_stub.c uses it.  If CONFIG_KUNIT=m, it can't find
    the symbol ftrace_location().
  * KUNIT_ASSERT_FAILURE() -> KUNIT_FAIL_AND_ABORT() due to
    7d4087b01389.
]
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 include/kunit/ftrace_stub.h         |  84 +++++++++++++++++
 kernel/trace/ftrace.c               |   3 +
 lib/kunit/Kconfig                   |  11 +++
 lib/kunit/Makefile                  |   4 +
 lib/kunit/ftrace_stub.c             | 139 ++++++++++++++++++++++++++++
 lib/kunit/kunit-example-test.c      |  27 +++++-
 lib/kunit/stubs_example.kunitconfig |  11 +++
 7 files changed, 278 insertions(+), 1 deletion(-)
 create mode 100644 include/kunit/ftrace_stub.h
 create mode 100644 lib/kunit/ftrace_stub.c
 create mode 100644 lib/kunit/stubs_example.kunitconfig

diff --git a/include/kunit/ftrace_stub.h b/include/kunit/ftrace_stub.h
new file mode 100644
index 000000000000..79c496b51ac5
--- /dev/null
+++ b/include/kunit/ftrace_stub.h
@@ -0,0 +1,84 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _KUNIT_FTRACE_STUB_H
+#define _KUNIT_FTRACE_STUB_H
+
+/** KUNIT_STUBBABLE - marks a function as stubbable when stubbing support is
+ * enabled.
+ *
+ * Stubbing uses ftrace internally, so we can only stub out functions when they
+ * are not inlined. This macro evaluates to noinline when stubbing support is
+ * enabled to thus make it safe.
+ *
+ * If you cannot add this annotation to the function, you can instead use
+ * KUNIT_STUBBABLE_TRAMPOLINE, which is the same, but evaluates to
+ * __always_inline when stubbing is not enabled.
+ *
+ * Consider copy_to_user, which is marked as __always_inline:
+ *
+ * .. code-block:: c
+ *	static KUNIT_STUBBABLE_TRAMPOLINE unsigned long
+ *	copy_to_user_trampoline(void __user *to, const void *from, unsigned long n)
+ *	{
+ *		return copy_to_user(to, from, n);
+ *	}
+ *
+ * Then we simply need to update our code to go through this function instead
+ * (in the places where we want to stub it out).
+ */
+#if IS_ENABLED(CONFIG_KUNIT_FTRACE_STUBS)
+#define KUNIT_STUBBABLE noinline
+#define KUNIT_STUBBABLE_TRAMPOLINE noinline
+#else
+#define KUNIT_STUBBABLE
+#define KUNIT_STUBBABLE_TRAMPOLINE __always_inline
+#endif
+
+struct kunit;
+
+/**
+ * kunit_activate_ftrace_stub() - makes all calls to @func go to @replacement during @test.
+ * @test: The test context object.
+ * @func: The function to stub out, must be annotated with KUNIT_STUBBABLE.
+ * @replacement: The function to replace @func with.
+ *
+ * All calls to @func will instead call @replacement for the duration of the
+ * current test. If called from outside the test's thread, the function will
+ * not be redirected.
+ *
+ * The redirection can be disabled again with kunit_deactivate_ftrace_stub().
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *	KUNIT_STUBBABLE int real_func(int n)
+ *	{
+ *		pr_info("real_func() called with %d", n);
+ *		return 0;
+ *	}
+ *
+ *	void replacement_func(int n)
+ *	{
+ *		pr_info("replacement_func() called with %d", n);
+ *		return 42;
+ *	}
+ *
+ *	void example_test(struct kunit *test)
+ *	{
+ *		kunit_activate_ftrace_stub(test, real_func, replacement_func);
+ *		KUNIT_EXPECT_EQ(test, real_func(1), 42);
+ *	}
+ *
+ */
+#define kunit_activate_ftrace_stub(test, func, replacement) do { \
+	typecheck_fn(typeof(&func), replacement); \
+	__kunit_activate_ftrace_stub(test, #func, func, replacement); \
+} while (0)
+
+void __kunit_activate_ftrace_stub(struct kunit *test,
+				  const char *name,
+				  void *real_fn_addr,
+				  void *replacement_addr);
+
+
+void kunit_deactivate_ftrace_stub(struct kunit *test, void *real_fn_addr);
+#endif  /* _KUNIT_STUB_H */
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 1a48aedb5255..7e86bc57d462 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -36,6 +36,8 @@
 #include <linux/rcupdate.h>
 #include <linux/kprobes.h>
 
+#include <kunit/visibility.h>
+
 #include <trace/events/sched.h>
 
 #include <asm/sections.h>
@@ -1663,6 +1665,7 @@ unsigned long ftrace_location(unsigned long ip)
 	}
 	return loc;
 }
+EXPORT_SYMBOL_IF_KUNIT(ftrace_location);
 
 /**
  * ftrace_text_reserved - return true if range contains an ftrace location
diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
index a97897edd964..933fda1df5c3 100644
--- a/lib/kunit/Kconfig
+++ b/lib/kunit/Kconfig
@@ -93,4 +93,15 @@ config KUNIT_AUTORUN_ENABLED
 	  In most cases this should be left as Y. Only if additional opt-in
 	  behavior is needed should this be set to N.
 
+config KUNIT_FTRACE_STUBS
+	bool "Support for stubbing out functions in KUnit tests with ftrace and kernel livepatch"
+	depends on FTRACE=y && FUNCTION_TRACER=y && MODULES=y && DEBUG_KERNEL=y && KALLSYMS_ALL=y && LIVEPATCH=y
+	help
+	  Builds support for stubbing out functions for the duration of KUnit
+	  test cases or suites using ftrace and kernel livepatch.
+	  See KUNIT_EXAMPLE_TEST for an example.
+
+	  NOTE: this does not work on all architectures (like UML, arm64) and
+	  relies on a lot of magic (see the dependencies list).
+
 endif # KUNIT
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 5aa51978e456..0ecb255576e2 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -29,3 +29,7 @@ obj-$(CONFIG_KUNIT_TEST) +=		assert_test.o
 endif
 
 obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=	kunit-example-test.o
+
+ifeq ($(CONFIG_KUNIT_FTRACE_STUBS),y)
+kunit-objs +=				ftrace_stub.o
+endif
diff --git a/lib/kunit/ftrace_stub.c b/lib/kunit/ftrace_stub.c
new file mode 100644
index 000000000000..1cf6edceb19c
--- /dev/null
+++ b/lib/kunit/ftrace_stub.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <kunit/test.h>
+#include <kunit/ftrace_stub.h>
+
+#include <linux/typecheck.h>
+
+#include <linux/ftrace.h>
+#include <linux/livepatch.h>
+#include <linux/sched.h>
+
+struct kunit_ftrace_stub_ctx {
+	struct kunit *test;
+	unsigned long real_fn_addr; /* used as a key to lookup the stub */
+	unsigned long replacement_addr;
+	struct ftrace_ops ops; /* a copy of kunit_stub_base_ops with .private set */
+};
+
+static void kunit_stub_trampoline(unsigned long ip, unsigned long parent_ip,
+				  struct ftrace_ops *ops,
+				  struct ftrace_regs *fregs)
+{
+	struct kunit_ftrace_stub_ctx *ctx = ops->private;
+	int lock_bit;
+
+	if (current->kunit_test != ctx->test)
+		return;
+
+	lock_bit = ftrace_test_recursion_trylock(ip, parent_ip);
+	KUNIT_ASSERT_GE(ctx->test, lock_bit, 0);
+
+	ftrace_regs_set_instruction_pointer(fregs, ctx->replacement_addr);
+
+	ftrace_test_recursion_unlock(lock_bit);
+}
+
+static struct ftrace_ops kunit_stub_base_ops = {
+	.func = &kunit_stub_trampoline,
+	.flags = FTRACE_OPS_FL_IPMODIFY |
+#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
+		FTRACE_OPS_FL_SAVE_REGS |
+#endif
+		FTRACE_OPS_FL_DYNAMIC
+};
+
+static void __kunit_ftrace_stub_resource_free(struct kunit_resource *res)
+{
+	struct kunit_ftrace_stub_ctx *ctx = res->data;
+
+	unregister_ftrace_function(&ctx->ops);
+	kfree(ctx);
+}
+
+/* Matching function for kunit_find_resource(). match_data is real_fn_addr. */
+static bool __kunit_ftrace_stub_resource_match(struct kunit *test,
+						struct kunit_resource *res,
+						void *match_real_fn_addr)
+{
+	/* This pointer is only valid if res is a ftrace stub resource. */
+	struct kunit_ftrace_stub_ctx *ctx = res->data;
+
+	/* Make sure the resource is a ftrace stub resource. */
+	if (res->free != &__kunit_ftrace_stub_resource_free)
+		return false;
+
+	return ctx->real_fn_addr == (unsigned long)match_real_fn_addr;
+}
+
+void kunit_deactivate_ftrace_stub(struct kunit *test, void *real_fn_addr)
+{
+	struct kunit_resource *res;
+
+	KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL,
+				"Tried to deactivate a NULL stub.");
+
+	/* Look up the existing stub for this function. */
+	res = kunit_find_resource(test,
+				  __kunit_ftrace_stub_resource_match,
+				  real_fn_addr);
+
+	/* Error out if the stub doesn't exist. */
+	KUNIT_ASSERT_PTR_NE_MSG(test, res, NULL,
+				"Tried to deactivate a nonexistent stub.");
+
+	/* Free the stub. We 'put' twice, as we got a reference
+	 * from kunit_find_resource(). The free function will deactivate the
+	 * ftrace stub.
+	 */
+	kunit_remove_resource(test, res);
+	kunit_put_resource(res);
+}
+EXPORT_SYMBOL_GPL(kunit_deactivate_ftrace_stub);
+
+void __kunit_activate_ftrace_stub(struct kunit *test,
+				  const char *name,
+				  void *real_fn_addr,
+				  void *replacement_addr)
+{
+	unsigned long ftrace_ip;
+	struct kunit_ftrace_stub_ctx *ctx;
+	int ret;
+
+	ftrace_ip = ftrace_location((unsigned long)real_fn_addr);
+	if (!ftrace_ip)
+		KUNIT_FAIL_AND_ABORT(test, "%s ip is invalid: not a function, or is marked notrace or inline", name);
+
+	/* Allocate the stub context, which contains pointers to the replacement
+	 * function and the test object. It's also registered as a KUnit
+	 * resource which can be looked up by address (to deactivate manually)
+	 * and is destroyed automatically on test exit.
+	 */
+	ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, ctx, "failed to allocate kunit stub for %s", name);
+
+	ctx->test = test;
+	ctx->ops = kunit_stub_base_ops;
+	ctx->ops.private = ctx;
+	ctx->real_fn_addr = (unsigned long)real_fn_addr;
+	ctx->replacement_addr = (unsigned long)replacement_addr;
+
+	ret = ftrace_set_filter_ip(&ctx->ops, ftrace_ip, 0, 0);
+	if (ret) {
+		kfree(ctx);
+		KUNIT_FAIL_AND_ABORT(test, "failed to set filter ip for %s: %d", name, ret);
+	}
+
+	ret = register_ftrace_function(&ctx->ops);
+	if (ret) {
+		kfree(ctx);
+		if (ret == -EBUSY)
+			KUNIT_FAIL_AND_ABORT(test, "failed to register stub (-EBUSY) for %s, likely due to already stubbing it?", name);
+		KUNIT_FAIL_AND_ABORT(test, "failed to register stub for %s: %d", name, ret);
+	}
+
+	/* Register the stub as a resource with a cleanup function */
+	kunit_alloc_resource(test, NULL,
+			     __kunit_ftrace_stub_resource_free,
+			     GFP_KERNEL, ctx);
+}
+EXPORT_SYMBOL_GPL(__kunit_activate_ftrace_stub);
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index 3056d6bc705d..1974e8d24a50 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -8,6 +8,7 @@
 
 #include <kunit/test.h>
 #include <kunit/static_stub.h>
+#include <kunit/ftrace_stub.h>
 
 /*
  * This is the most fundamental element of KUnit, the test case. A test case
@@ -152,7 +153,7 @@ static void example_all_expect_macros_test(struct kunit *test)
 }
 
 /* This is a function we'll replace with static stubs. */
-static int add_one(int i)
+static KUNIT_STUBBABLE int add_one(int i)
 {
 	/* This will trigger the stub if active. */
 	KUNIT_STATIC_STUB_REDIRECT(add_one, i);
@@ -277,6 +278,29 @@ static void example_slow_test(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, 1 + 1, 2);
 }
 
+/*
+ * This test shows the use of ftrace stubs.
+ */
+static void example_ftrace_stub_test(struct kunit *test)
+{
+#if !IS_ENABLED(CONFIG_KUNIT_FTRACE_STUBS)
+	kunit_skip(test, "KUNIT_FTRACE_STUBS not enabled");
+#else
+	/* By default, function is not stubbed. */
+	KUNIT_EXPECT_EQ(test, add_one(1), 2);
+
+	/* Replace add_one() with subtract_one(). */
+	kunit_activate_ftrace_stub(test, add_one, subtract_one);
+
+	/* add_one() is now replaced. */
+	KUNIT_EXPECT_EQ(test, add_one(1), 0);
+
+	/* Return add_one() to normal. */
+	kunit_deactivate_ftrace_stub(test, add_one);
+	KUNIT_EXPECT_EQ(test, add_one(1), 2);
+#endif
+}
+
 /*
  * Here we make a list of all the test cases we want to add to the test suite
  * below.
@@ -297,6 +321,7 @@ static struct kunit_case example_test_cases[] = {
 	KUNIT_CASE(example_priv_test),
 	KUNIT_CASE_PARAM(example_params_test, example_gen_params),
 	KUNIT_CASE_SLOW(example_slow_test),
+	KUNIT_CASE(example_ftrace_stub_test),
 	{}
 };
 
diff --git a/lib/kunit/stubs_example.kunitconfig b/lib/kunit/stubs_example.kunitconfig
new file mode 100644
index 000000000000..a47369199fb9
--- /dev/null
+++ b/lib/kunit/stubs_example.kunitconfig
@@ -0,0 +1,11 @@
+CONFIG_KUNIT=y
+CONFIG_KUNIT_FTRACE_STUBS=y
+CONFIG_KUNIT_EXAMPLE_TEST=y
+
+# Depedencies
+CONFIG_FTRACE=y
+CONFIG_FUNCTION_TRACER=y
+CONFIG_MODULES=y
+CONFIG_DEBUG_KERNEL=y
+CONFIG_KALLSYMS_ALL=y
+CONFIG_LIVEPATCH=y
-- 
2.49.0.1101.gccaa498523-goog

[RFC PATCH 2/7] platform/chrome: kunit: cros_ec_i2c: Add tests with ftrace stub

[Tzung-Bi Shih]

For running the tests:

$ ./tools/testing/kunit/kunit.py run \
	--arch=x86_64 \
	--kconfig_add CONFIG_CHROME_PLATFORMS=y \
	--kconfig_add CONFIG_CROS_EC=y \
	--kconfig_add CONFIG_FTRACE=y \
	--kconfig_add CONFIG_FUNCTION_TRACER=y \
	--kconfig_add CONFIG_MODULES=y \
	--kconfig_add CONFIG_DEBUG_KERNEL=y \
	--kconfig_add CONFIG_KALLSYMS_ALL=y \
	--kconfig_add CONFIG_LIVEPATCH=y \
	--kconfig_add CONFIG_KUNIT_FTRACE_STUBS=y \
	--kconfig_add CONFIG_I2C=y \
	--kconfig_add CONFIG_CROS_EC_I2C=y \
	--kconfig_add CONFIG_CROS_KUNIT_EC_I2C_TEST=y \
	cros_ec_i2c*

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/Kconfig            |   9 +
 drivers/platform/chrome/Makefile           |   1 +
 drivers/platform/chrome/cros_ec_i2c_test.c | 479 +++++++++++++++++++++
 3 files changed, 489 insertions(+)
 create mode 100644 drivers/platform/chrome/cros_ec_i2c_test.c

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 10941ac37305..5e0b44fb7ca7 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -327,4 +327,13 @@ config CROS_KUNIT_EC_PROTO_TEST
 	help
 	  Kunit tests for ChromeOS EC protocol.
 
+config CROS_KUNIT_EC_I2C_TEST
+	tristate "Kunit tests for ChromeOS EC over I2C" if !KUNIT_ALL_TESTS
+	depends on KUNIT && CROS_EC
+	default KUNIT_ALL_TESTS
+	depends on KUNIT_FTRACE_STUBS
+	depends on CROS_EC_I2C
+	help
+	  Kunit tests for ChromeOS EC over I2C.
+
 endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index b981a1bb5bd8..9808f25aea38 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -44,3 +44,4 @@ obj-$(CONFIG_WILCO_EC)			+= wilco_ec/
 # Kunit test cases
 obj-$(CONFIG_CROS_KUNIT_EC_PROTO_TEST)	+= cros_kunit_proto_test.o
 cros_kunit_proto_test-objs		:= cros_ec_proto_test_util.o cros_ec_proto_test.o
+obj-$(CONFIG_CROS_KUNIT_EC_I2C_TEST)	+= cros_ec_i2c_test.o
diff --git a/drivers/platform/chrome/cros_ec_i2c_test.c b/drivers/platform/chrome/cros_ec_i2c_test.c
new file mode 100644
index 000000000000..3a7f1a17d82d
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_i2c_test.c
@@ -0,0 +1,479 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Kunit tests for ChromeOS Embedded Controller I2C interface.
+ */
+#include <kunit/test.h>
+#include <kunit/ftrace_stub.h>
+
+#include <linux/i2c.h>
+#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/sched.h>
+
+#include "cros_ec.h"
+
+#define BUFSIZE 128
+#define I2C_ADDR 0x06
+
+struct cros_ec_i2c_test_priv {
+	struct i2c_adapter *fake_adap;
+	struct i2c_client *client;
+
+	int fake_cros_ec_register_called;
+	struct cros_ec_device *ec_dev;
+
+	int fake_cros_ec_unregister_called;
+
+	struct i2c_msg *i2c_msgs;
+	int i2c_msg_num;
+	int fake_i2c_xfer_res;
+	int fake_i2c_xfer_len;
+	u8 *fake_i2c_xfer_data;
+	u8 fake_i2c_xfer_sum;
+	int fake_i2c_xfer_ret;
+};
+
+static int fake_cros_ec_register(struct cros_ec_device *ec_dev)
+{
+	struct kunit *test = current->kunit_test;
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+
+	priv->fake_cros_ec_register_called += 1;
+
+	priv->ec_dev = ec_dev;
+	priv->ec_dev->din_size = BUFSIZE;
+	priv->ec_dev->din = kunit_kmalloc(test, priv->ec_dev->din_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->ec_dev->din);
+	priv->ec_dev->dout_size = BUFSIZE;
+	priv->ec_dev->dout = kunit_kmalloc(test, priv->ec_dev->dout_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->ec_dev->dout);
+	return 0;
+}
+
+static void fake_cros_ec_unregister(struct cros_ec_device *ec_dev)
+{
+	struct kunit *test = current->kunit_test;
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+
+	priv->fake_cros_ec_unregister_called += 1;
+}
+
+static int fake_cros_ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
+{
+	struct kunit *test = current->kunit_test;
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	int i;
+
+	priv->i2c_msgs = kunit_kmalloc_array(test, sizeof(struct i2c_msg), num, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->i2c_msgs);
+
+	for (i = 0; i < num; ++i) {
+		memcpy(priv->i2c_msgs + i, msgs + i, sizeof(struct i2c_msg));
+
+		priv->i2c_msgs[i].buf = kunit_kmalloc(test, msgs[i].len, GFP_KERNEL);
+		KUNIT_ASSERT_NOT_NULL(test, priv->i2c_msgs[i].buf);
+		memcpy(priv->i2c_msgs[i].buf, msgs[i].buf, msgs[i].len);
+
+		if (msgs[i].flags == I2C_M_RD) {
+			msgs[i].buf[0] = priv->fake_i2c_xfer_res;
+			msgs[i].buf[1] = priv->fake_i2c_xfer_len;
+
+			if (priv->fake_i2c_xfer_data)
+				memcpy(&msgs[i].buf[2], priv->fake_i2c_xfer_data,
+				       priv->fake_i2c_xfer_len);
+
+			msgs[i].buf[priv->fake_i2c_xfer_len + 2] = priv->fake_i2c_xfer_sum;
+		}
+	}
+	priv->i2c_msg_num = num;
+
+	return priv->fake_i2c_xfer_ret;
+}
+
+static u32 fake_cros_ec_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm fake_cros_ec_i2c_algorithm = {
+	.master_xfer = fake_cros_ec_i2c_xfer,
+	.functionality = fake_cros_ec_i2c_functionality,
+};
+
+static int cros_ec_i2c_test_init(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv;
+	static const struct i2c_board_info board_info = {
+		I2C_BOARD_INFO("cros-ec-i2c", I2C_ADDR),
+	};
+
+	kunit_activate_ftrace_stub(test, cros_ec_register, fake_cros_ec_register);
+	kunit_activate_ftrace_stub(test, cros_ec_unregister, fake_cros_ec_unregister);
+
+	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+	test->priv = priv;
+
+	priv->fake_adap = kunit_kzalloc(test, sizeof(*priv->fake_adap), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->fake_adap);
+
+	priv->fake_adap->owner = THIS_MODULE;
+	strscpy(priv->fake_adap->name, "cros-ec-i2c-test");
+	priv->fake_adap->algo = &fake_cros_ec_i2c_algorithm;
+	priv->fake_adap->retries = 3;
+
+	KUNIT_ASSERT_EQ(test, i2c_add_adapter(priv->fake_adap), 0);
+
+	priv->client = i2c_new_client_device(priv->fake_adap, &board_info);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->client);
+	KUNIT_EXPECT_EQ(test, priv->client->addr, I2C_ADDR);
+
+	KUNIT_EXPECT_EQ(test, priv->fake_cros_ec_register_called, 1);
+	KUNIT_ASSERT_NOT_NULL(test, priv->ec_dev);
+	KUNIT_EXPECT_EQ(test, priv->fake_cros_ec_unregister_called, 0);
+	return 0;
+}
+
+static void cros_ec_i2c_test_exit(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+
+	i2c_unregister_device(priv->client);
+	KUNIT_EXPECT_EQ(test, priv->fake_cros_ec_unregister_called, 1);
+
+	i2c_del_adapter(priv->fake_adap);
+
+	kunit_deactivate_ftrace_stub(test, cros_ec_register);
+	kunit_deactivate_ftrace_stub(test, cros_ec_unregister);
+}
+
+static int cros_ec_i2c_test_cmd_xfer_init(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv;
+
+	cros_ec_i2c_test_init(test);
+	priv = test->priv;
+	priv->ec_dev->proto_version = 2;
+	return 0;
+}
+
+static void cros_ec_i2c_test_cmd_xfer_normal(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command *msg;
+	int ret, i;
+	u8 sum;
+
+	msg = kunit_kmalloc(test, sizeof(*msg) + 2 /* max(outsize, insize) */, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, msg);
+	msg->version = 0x1000;
+	msg->command = 0x1001;
+	msg->outsize = 2;
+	msg->insize = 1;
+	msg->data[0] = 0xbe;
+	msg->data[1] = 0xef;
+
+	priv->fake_i2c_xfer_res = 0;
+	priv->fake_i2c_xfer_data = kunit_kmalloc(test, msg->insize, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->fake_i2c_xfer_data);
+	priv->fake_i2c_xfer_data[0] = 0xaa;
+	priv->fake_i2c_xfer_len = msg->insize;
+	priv->fake_i2c_xfer_ret = 2 /* # of i2c_msg */;
+	sum = priv->fake_i2c_xfer_res + priv->fake_i2c_xfer_len + priv->fake_i2c_xfer_data[0];
+	priv->fake_i2c_xfer_sum = sum;
+
+	ret = priv->ec_dev->cmd_xfer(priv->ec_dev, msg);
+	KUNIT_EXPECT_EQ(test, ret, 1 /* insize */);
+	KUNIT_EXPECT_EQ(test, msg->result, 0);
+	KUNIT_EXPECT_EQ(test, msg->data[0], 0xaa);
+
+	KUNIT_EXPECT_EQ(test, priv->i2c_msg_num, 2);
+	/*
+	 * Validate output message only which is supposed to be received by EC devices.
+	 */
+	KUNIT_EXPECT_EQ(test, priv->i2c_msgs[0].addr, I2C_ADDR);
+	KUNIT_EXPECT_EQ(test, priv->i2c_msgs[0].flags, 0);
+	/*
+	 * Total length of the message: EC_MSG_TX_HEADER_BYTES (version, command, length) +
+	 * outsize + EC_MSG_TX_TRAILER_BYTES (checksum).
+	 */
+	KUNIT_EXPECT_EQ(test, priv->i2c_msgs[0].len, EC_MSG_TX_PROTO_BYTES + 2 /* outsize */);
+	KUNIT_EXPECT_EQ(test, priv->i2c_msgs[0].buf[0], (u8)(EC_CMD_VERSION0 + 0x1000));
+	KUNIT_EXPECT_EQ(test, priv->i2c_msgs[0].buf[1], (u8)0x1001);
+	KUNIT_EXPECT_EQ(test, priv->i2c_msgs[0].buf[2], 2 /* outsize */);
+	for (sum = 0, i = 0; i < EC_MSG_TX_HEADER_BYTES + 2 /* outsize */; ++i)
+		sum += priv->i2c_msgs[0].buf[i];
+	KUNIT_EXPECT_EQ(test,
+			priv->i2c_msgs[0].buf[EC_MSG_TX_HEADER_BYTES + 2 /* outsize */], sum);
+}
+
+static void cros_ec_i2c_test_cmd_xfer_error(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command msg = {};
+	int ret;
+
+	priv->fake_i2c_xfer_ret = -EBUSY;
+	ret = priv->ec_dev->cmd_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EBUSY);
+
+	priv->fake_i2c_xfer_ret = 1;
+	ret = priv->ec_dev->cmd_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EIO);
+
+	priv->fake_i2c_xfer_ret = 2 /* # of i2c_msg */;
+	priv->fake_i2c_xfer_res = EC_RES_IN_PROGRESS;
+	ret = priv->ec_dev->cmd_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EAGAIN);
+}
+
+static void cros_ec_i2c_test_cmd_xfer_response_too_long(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command msg = {};
+	int ret;
+
+	priv->fake_i2c_xfer_res = 0;
+	priv->fake_i2c_xfer_data = kunit_kmalloc(test, 1, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->fake_i2c_xfer_data);
+	priv->fake_i2c_xfer_len = msg.insize + 1; /* make it greater than msg.insize */
+	priv->fake_i2c_xfer_ret = 2 /* # of i2c_msg */;
+
+	ret = priv->ec_dev->cmd_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -ENOSPC);
+}
+
+static void cros_ec_i2c_test_cmd_xfer_response_bad_checksum(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command msg = {};
+	int ret;
+
+	priv->fake_i2c_xfer_ret = 2 /* # of i2c_msg */;
+	priv->fake_i2c_xfer_sum = (u8)0xbad;
+
+	ret = priv->ec_dev->cmd_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EBADMSG);
+}
+
+static struct kunit_case cros_ec_i2c_test_cmd_xfer_cases[] = {
+	KUNIT_CASE(cros_ec_i2c_test_cmd_xfer_normal),
+	KUNIT_CASE(cros_ec_i2c_test_cmd_xfer_error),
+	KUNIT_CASE(cros_ec_i2c_test_cmd_xfer_response_too_long),
+	KUNIT_CASE(cros_ec_i2c_test_cmd_xfer_response_bad_checksum),
+	{},
+};
+
+static struct kunit_suite cros_ec_i2c_test_cmd_xfer_suite = {
+	.name = "cros_ec_i2c_test_cmd_xfer_suite",
+	.init = cros_ec_i2c_test_cmd_xfer_init,
+	.exit = cros_ec_i2c_test_exit,
+	.test_cases = cros_ec_i2c_test_cmd_xfer_cases,
+};
+
+static int cros_ec_i2c_test_pkt_xfer_init(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv;
+
+	cros_ec_i2c_test_init(test);
+	priv = test->priv;
+	priv->ec_dev->proto_version = 3;
+	return 0;
+}
+
+static void cros_ec_i2c_test_pkt_xfer_normal(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command *msg;
+	struct ec_host_request *request;
+	struct ec_host_response *response;
+	int ret, i;
+	u8 sum;
+
+	msg = kunit_kmalloc(test, sizeof(*msg) + 2 /* max(outsize, insize) */, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, msg);
+	msg->version = 0x1000;
+	msg->command = 0x1001;
+	msg->outsize = 2;
+	msg->insize = 1;
+	msg->data[0] = 0xbe;
+	msg->data[1] = 0xef;
+
+	priv->fake_i2c_xfer_res = 0;
+	priv->fake_i2c_xfer_len = sizeof(*response) + msg->insize;
+	priv->fake_i2c_xfer_data = kunit_kzalloc(test, priv->fake_i2c_xfer_len, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->fake_i2c_xfer_data);
+	response = (struct ec_host_response *)priv->fake_i2c_xfer_data;
+	response->struct_version = EC_HOST_RESPONSE_VERSION;
+	response->data_len = msg->insize;
+	priv->fake_i2c_xfer_data[sizeof(*response)] = 0xaa;
+	priv->fake_i2c_xfer_ret = 2 /* # of i2c_msg */;
+	for (sum = 0, i = 0; i < priv->fake_i2c_xfer_len; ++i)
+		sum += priv->fake_i2c_xfer_data[i];
+	response->checksum = -sum;
+
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, msg);
+	KUNIT_EXPECT_EQ(test, ret, 1 /* insize */);
+	KUNIT_EXPECT_EQ(test, msg->result, 0);
+	KUNIT_EXPECT_EQ(test, msg->data[0], 0xaa);
+
+	KUNIT_EXPECT_EQ(test, priv->i2c_msg_num, 2);
+	/*
+	 * Validate output message only which is supposed to be received by EC devices.
+	 */
+	KUNIT_EXPECT_EQ(test, priv->i2c_msgs[0].addr, I2C_ADDR);
+	KUNIT_EXPECT_EQ(test, priv->i2c_msgs[0].flags, 0);
+	/*
+	 * Total length of the message: sizeof(struct ec_host_request_i2c) + outsize.
+	 * The test can't access struct ec_host_request_i2c directly (in cros_ec_i2c.c).
+	 * However, it is a {u8 + struct ec_host_request} struct.
+	 */
+	KUNIT_EXPECT_EQ(test, priv->i2c_msgs[0].len, 1 + sizeof(*request) + 2 /* outsize */);
+	/*
+	 * i2c_msgs[0].buf is a struct ec_host_request_i2c.  Again, the test can't access the
+	 * struct directly.  Access the u8 and struct ec_host_request separately.
+	 */
+	KUNIT_EXPECT_EQ(test, priv->i2c_msgs[0].buf[0], EC_COMMAND_PROTOCOL_3);
+	request = (struct ec_host_request *)&priv->i2c_msgs[0].buf[1];
+	KUNIT_EXPECT_EQ(test, request->struct_version, EC_HOST_REQUEST_VERSION);
+	KUNIT_EXPECT_EQ(test, request->command, 0x1001);
+	KUNIT_EXPECT_EQ(test, request->command_version, (u8)0x1000);
+	KUNIT_EXPECT_EQ(test, request->data_len, 2 /* outsize */);
+	for (sum = 0, i = 0; i < sizeof(*request); ++i)
+		sum += ((u8 *)request)[i];
+	sum += 0xbe + 0xef;
+	KUNIT_EXPECT_EQ(test, sum, 0);
+}
+
+static void cros_ec_i2c_test_pkt_xfer_msg_too_long(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command msg = {
+		.insize = 10,
+		.outsize = 10,
+	};
+	int ret;
+
+	priv->ec_dev->din_size = 0;
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+
+	priv->ec_dev->din_size = BUFSIZE;
+	priv->ec_dev->dout_size = 0;
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+}
+
+static void cros_ec_i2c_test_pkt_xfer_error(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command msg = {};
+	int ret;
+
+	priv->fake_i2c_xfer_ret = -EBUSY;
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EBUSY);
+
+	priv->fake_i2c_xfer_ret = 1;
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EIO);
+}
+
+static void cros_ec_i2c_test_pkt_xfer_in_progress(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command msg = {};
+	int ret;
+
+	priv->fake_i2c_xfer_res = EC_RES_IN_PROGRESS;
+	priv->fake_i2c_xfer_ret = 2 /* # of i2c_msg */;
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EAGAIN);
+}
+
+static void cros_ec_i2c_test_pkt_xfer_to_v2_proto(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command msg = {};
+	int ret;
+
+	priv->fake_i2c_xfer_res = EC_RES_INVALID_COMMAND;
+	priv->fake_i2c_xfer_ret = 2 /* # of i2c_msg */;
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EPROTONOSUPPORT);
+}
+
+static void cros_ec_i2c_test_pkt_xfer_response_too_short(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command msg = {};
+	int ret;
+
+	priv->fake_i2c_xfer_len = sizeof(struct ec_host_response) - 1; /* make it shorter */
+	priv->fake_i2c_xfer_ret = 2 /* # of i2c_msg */;
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EBADMSG);
+}
+
+static void cros_ec_i2c_test_pkt_xfer_response_too_long(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command msg = {};
+	struct ec_host_response *response;
+	int ret;
+
+	priv->fake_i2c_xfer_len = sizeof(*response);
+	priv->fake_i2c_xfer_data = kunit_kzalloc(test, priv->fake_i2c_xfer_len, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->fake_i2c_xfer_data);
+	response = (struct ec_host_response *)priv->fake_i2c_xfer_data;
+	response->struct_version = EC_HOST_RESPONSE_VERSION;
+	response->data_len = 100;
+	priv->fake_i2c_xfer_ret = 2 /* # of i2c_msg */;
+
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EMSGSIZE);
+}
+
+static void cros_ec_i2c_test_pkt_xfer_bad_checksum(struct kunit *test)
+{
+	struct cros_ec_i2c_test_priv *priv = test->priv;
+	struct cros_ec_command msg = {};
+	struct ec_host_response *response;
+	int ret;
+
+	priv->fake_i2c_xfer_len = sizeof(*response);
+	priv->fake_i2c_xfer_data = kunit_kzalloc(test, priv->fake_i2c_xfer_len, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->fake_i2c_xfer_data);
+	response = (struct ec_host_response *)priv->fake_i2c_xfer_data;
+	response->struct_version = EC_HOST_RESPONSE_VERSION;
+	response->checksum = (u8)0xbad;
+	priv->fake_i2c_xfer_ret = 2 /* # of i2c_msg */;
+
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EBADMSG);
+}
+
+static struct kunit_case cros_ec_i2c_test_pkt_xfer_cases[] = {
+	KUNIT_CASE(cros_ec_i2c_test_pkt_xfer_normal),
+	KUNIT_CASE(cros_ec_i2c_test_pkt_xfer_msg_too_long),
+	KUNIT_CASE(cros_ec_i2c_test_pkt_xfer_error),
+	KUNIT_CASE(cros_ec_i2c_test_pkt_xfer_in_progress),
+	KUNIT_CASE(cros_ec_i2c_test_pkt_xfer_to_v2_proto),
+	KUNIT_CASE(cros_ec_i2c_test_pkt_xfer_response_too_short),
+	KUNIT_CASE(cros_ec_i2c_test_pkt_xfer_response_too_long),
+	KUNIT_CASE(cros_ec_i2c_test_pkt_xfer_bad_checksum),
+	{},
+};
+
+static struct kunit_suite cros_ec_i2c_test_pkt_xfer_suite = {
+	.name = "cros_ec_i2c_test_pkt_xfer_suite",
+	.init = cros_ec_i2c_test_pkt_xfer_init,
+	.exit = cros_ec_i2c_test_exit,
+	.test_cases = cros_ec_i2c_test_pkt_xfer_cases,
+};
+
+kunit_test_suites(
+	&cros_ec_i2c_test_cmd_xfer_suite,
+	&cros_ec_i2c_test_pkt_xfer_suite,
+);
+
+MODULE_LICENSE("GPL");
-- 
2.49.0.1101.gccaa498523-goog

[RFC PATCH 3/7] platform/chrome: kunit: cros_ec_i2c: Use static stub instead

[Tzung-Bi Shih]

For running the tests:

$ ./tools/testing/kunit/kunit.py run \
        --arch=x86_64 \
        --kconfig_add CONFIG_CHROME_PLATFORMS=y \
        --kconfig_add CONFIG_CROS_EC=y \
        --kconfig_add CONFIG_I2C=y \
        --kconfig_add CONFIG_CROS_EC_I2C=y \
        --kconfig_add CONFIG_CROS_KUNIT_EC_I2C_TEST=y \
        cros_ec_i2c*

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/Kconfig            |  1 -
 drivers/platform/chrome/cros_ec.c          |  6 ++++++
 drivers/platform/chrome/cros_ec_i2c_test.c | 10 +++++-----
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 5e0b44fb7ca7..bf10c0625be8 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -331,7 +331,6 @@ config CROS_KUNIT_EC_I2C_TEST
 	tristate "Kunit tests for ChromeOS EC over I2C" if !KUNIT_ALL_TESTS
 	depends on KUNIT && CROS_EC
 	default KUNIT_ALL_TESTS
-	depends on KUNIT_FTRACE_STUBS
 	depends on CROS_EC_I2C
 	help
 	  Kunit tests for ChromeOS EC over I2C.
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 110771a8645e..62327fa0ff34 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -18,6 +18,8 @@
 #include <linux/slab.h>
 #include <linux/suspend.h>
 
+#include <kunit/static_stub.h>
+
 #include "cros_ec.h"
 
 static struct cros_ec_platform ec_p = {
@@ -179,6 +181,8 @@ static int cros_ec_ready_event(struct notifier_block *nb,
  */
 int cros_ec_register(struct cros_ec_device *ec_dev)
 {
+	KUNIT_STATIC_STUB_REDIRECT(cros_ec_register, ec_dev);
+
 	struct device *dev = ec_dev->dev;
 	int err = 0;
 
@@ -318,6 +322,8 @@ EXPORT_SYMBOL(cros_ec_register);
  */
 void cros_ec_unregister(struct cros_ec_device *ec_dev)
 {
+	KUNIT_STATIC_STUB_REDIRECT(cros_ec_unregister, ec_dev);
+
 	platform_device_unregister(ec_dev->pd);
 	platform_device_unregister(ec_dev->ec);
 	mutex_destroy(&ec_dev->lock);
diff --git a/drivers/platform/chrome/cros_ec_i2c_test.c b/drivers/platform/chrome/cros_ec_i2c_test.c
index 3a7f1a17d82d..5d41cdeec4b7 100644
--- a/drivers/platform/chrome/cros_ec_i2c_test.c
+++ b/drivers/platform/chrome/cros_ec_i2c_test.c
@@ -3,7 +3,7 @@
  * Kunit tests for ChromeOS Embedded Controller I2C interface.
  */
 #include <kunit/test.h>
-#include <kunit/ftrace_stub.h>
+#include <kunit/static_stub.h>
 
 #include <linux/i2c.h>
 #include <linux/platform_data/cros_ec_proto.h>
@@ -106,8 +106,8 @@ static int cros_ec_i2c_test_init(struct kunit *test)
 		I2C_BOARD_INFO("cros-ec-i2c", I2C_ADDR),
 	};
 
-	kunit_activate_ftrace_stub(test, cros_ec_register, fake_cros_ec_register);
-	kunit_activate_ftrace_stub(test, cros_ec_unregister, fake_cros_ec_unregister);
+	kunit_activate_static_stub(test, cros_ec_register, fake_cros_ec_register);
+	kunit_activate_static_stub(test, cros_ec_unregister, fake_cros_ec_unregister);
 
 	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
@@ -142,8 +142,8 @@ static void cros_ec_i2c_test_exit(struct kunit *test)
 
 	i2c_del_adapter(priv->fake_adap);
 
-	kunit_deactivate_ftrace_stub(test, cros_ec_register);
-	kunit_deactivate_ftrace_stub(test, cros_ec_unregister);
+	kunit_deactivate_static_stub(test, cros_ec_register);
+	kunit_deactivate_static_stub(test, cros_ec_unregister);
 }
 
 static int cros_ec_i2c_test_cmd_xfer_init(struct kunit *test)
-- 
2.49.0.1101.gccaa498523-goog

[RFC PATCH 4/7] platform/chrome: kunit: cros_ec_spi: Add tests with ftrace stub

[Tzung-Bi Shih]

For running the tests:

$ ./tools/testing/kunit/kunit.py run \
	--arch=x86_64 \
	--kconfig_add CONFIG_FTRACE=y \
	--kconfig_add CONFIG_FUNCTION_TRACER=y \
	--kconfig_add CONFIG_MODULES=y \
	--kconfig_add CONFIG_DEBUG_KERNEL=y \
	--kconfig_add CONFIG_KALLSYMS_ALL=y \
	--kconfig_add CONFIG_LIVEPATCH=y \
	--kconfig_add CONFIG_KUNIT_FTRACE_STUBS=y \
	--kconfig_add CONFIG_CHROME_PLATFORMS=y \
	--kconfig_add CONFIG_CROS_EC=y \
	--kconfig_add CONFIG_SPI=y \
	--kconfig_add CONFIG_CROS_EC_SPI=y \
	--kconfig_add CONFIG_CROS_KUNIT_EC_SPI_TEST=y \
	cros_ec_spi*

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/Kconfig            |   9 +
 drivers/platform/chrome/Makefile           |   1 +
 drivers/platform/chrome/cros_ec_spi_test.c | 361 +++++++++++++++++++++
 3 files changed, 371 insertions(+)
 create mode 100644 drivers/platform/chrome/cros_ec_spi_test.c

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index bf10c0625be8..aa13e871a31f 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -335,4 +335,13 @@ config CROS_KUNIT_EC_I2C_TEST
 	help
 	  Kunit tests for ChromeOS EC over I2C.
 
+config CROS_KUNIT_EC_SPI_TEST
+	tristate "Kunit tests for ChromeOS EC over SPI" if !KUNIT_ALL_TESTS
+	depends on KUNIT && CROS_EC
+	default KUNIT_ALL_TESTS
+	depends on KUNIT_FTRACE_STUBS
+	depends on CROS_EC_SPI
+	help
+	  Kunit tests for ChromeOS EC over SPI.
+
 endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 9808f25aea38..74649b8f21b3 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -45,3 +45,4 @@ obj-$(CONFIG_WILCO_EC)			+= wilco_ec/
 obj-$(CONFIG_CROS_KUNIT_EC_PROTO_TEST)	+= cros_kunit_proto_test.o
 cros_kunit_proto_test-objs		:= cros_ec_proto_test_util.o cros_ec_proto_test.o
 obj-$(CONFIG_CROS_KUNIT_EC_I2C_TEST)	+= cros_ec_i2c_test.o
+obj-$(CONFIG_CROS_KUNIT_EC_SPI_TEST)	+= cros_ec_spi_test.o
diff --git a/drivers/platform/chrome/cros_ec_spi_test.c b/drivers/platform/chrome/cros_ec_spi_test.c
new file mode 100644
index 000000000000..2a021569a726
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_spi_test.c
@@ -0,0 +1,361 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Kunit tests for ChromeOS Embedded Controller SPI interface.
+ */
+#include <kunit/test.h>
+#include <kunit/ftrace_stub.h>
+
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/sched.h>
+#include <linux/spi/spi.h>
+
+#include "cros_ec.h"
+
+#define BUFSIZE 128
+#define SPI_BUS_NUM 0x6
+
+struct fake_xt {
+	u8 *buf;
+	size_t len;
+
+	struct list_head list;
+};
+
+struct cros_ec_spi_test_priv {
+	struct class *fake_class;
+	struct device dev;
+	struct spi_controller *fake_ctlr;
+	struct spi_device *fake_spi_device;
+
+	int fake_cros_ec_register_called;
+	struct cros_ec_device *ec_dev;
+
+	int fake_cros_ec_unregister_called;
+
+	struct list_head tx_head;
+	struct list_head rx_head;
+};
+
+static struct fake_xt *queue_fake_xt(struct kunit *test, struct list_head *head, size_t len)
+{
+	struct fake_xt *xt = kunit_kmalloc(test, sizeof(*xt), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, xt);
+
+	xt->buf = kunit_kzalloc(test, len, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, xt->buf);
+	xt->len = len;
+	list_add_tail(&xt->list, head);
+	return xt;
+}
+
+static struct fake_xt *dequeue_fake_xt(struct list_head *head)
+{
+	struct fake_xt *xt = list_first_entry(head, struct fake_xt, list);
+	list_del(&xt->list);
+	return xt;
+}
+
+static int fake_cros_ec_register(struct cros_ec_device *ec_dev)
+{
+	struct kunit *test = current->kunit_test;
+	struct cros_ec_spi_test_priv *priv = test->priv;
+
+	priv->fake_cros_ec_register_called += 1;
+
+	priv->ec_dev = ec_dev;
+	priv->ec_dev->din_size = BUFSIZE;
+	priv->ec_dev->din = kunit_kmalloc(test, priv->ec_dev->din_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->ec_dev->din);
+	priv->ec_dev->dout_size = BUFSIZE;
+	priv->ec_dev->dout = kunit_kmalloc(test, priv->ec_dev->dout_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv->ec_dev->dout);
+	return 0;
+}
+
+static void fake_cros_ec_unregister(struct cros_ec_device *ec_dev)
+{
+	struct kunit *test = current->kunit_test;
+	struct cros_ec_spi_test_priv *priv = test->priv;
+
+	priv->fake_cros_ec_unregister_called += 1;
+}
+
+static int fake_spi_transfer_one_message(struct spi_controller *ctlr, struct spi_message *msg)
+{
+	/* Note: the function's context is another kernel thread. */
+	struct kunit *test = spi_controller_get_devdata(ctlr);
+	struct cros_ec_spi_test_priv *priv = test->priv;
+	struct spi_transfer *t;
+
+	list_for_each_entry(t, &msg->transfers, transfer_list) {
+		if (t->tx_buf) {
+			struct fake_xt *tx = queue_fake_xt(test, &priv->tx_head, t->len);
+			memcpy(tx->buf, t->tx_buf, t->len);
+		}
+
+		if (t->rx_buf && !list_empty(&priv->rx_head)) {
+			struct fake_xt *rx = dequeue_fake_xt(&priv->rx_head);
+			memcpy(t->rx_buf, rx->buf, min(t->len, rx->len));
+		}
+	}
+
+	msg->status = 0;
+	spi_finalize_current_message(ctlr);
+	return 0;
+}
+
+static int find_target_driver(struct device_driver *drv, void *data)
+{
+	struct device_driver **pdrv = data;
+
+	if (strcmp(drv->name, "cros-ec-spi") == 0)
+		*pdrv = drv;
+	return 0;
+}
+
+static int cros_ec_spi_test_init(struct kunit *test)
+{
+	struct cros_ec_spi_test_priv *priv;
+	struct spi_board_info board_info = {};
+	int ret;
+	struct device_driver *drv;
+	enum probe_type orig;
+
+	kunit_activate_ftrace_stub(test, cros_ec_register, fake_cros_ec_register);
+	kunit_activate_ftrace_stub(test, cros_ec_unregister, fake_cros_ec_unregister);
+
+	sized_strscpy(board_info.modalias, "cros-ec-spi", SPI_NAME_SIZE);
+
+	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+	test->priv = priv;
+
+	priv->fake_class = class_create("fake-class");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->fake_class);
+
+	priv->dev.class = priv->fake_class;
+	priv->dev.parent = NULL;
+	dev_set_name(&priv->dev, "cros-ec-spi-test-dev");
+	ret = device_register(&priv->dev);
+	if (ret)
+		put_device(&priv->dev);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	priv->fake_ctlr = spi_alloc_host(&priv->dev, 0);
+	KUNIT_ASSERT_NOT_NULL(test, priv->fake_ctlr);
+	spi_controller_set_devdata(priv->fake_ctlr, test);
+
+	priv->fake_ctlr->transfer_one_message = fake_spi_transfer_one_message;
+	priv->fake_ctlr->bus_num = SPI_BUS_NUM;
+	ret = spi_register_controller(priv->fake_ctlr);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	/*
+	 * Force to use synchronous probe so that the upcoming SPI device gets
+	 * probed correctly and synchronously.
+	 */
+	ret = bus_for_each_drv(&spi_bus_type, NULL, &drv, find_target_driver);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	KUNIT_ASSERT_NOT_NULL(test, drv);
+	orig = drv->probe_type;
+	drv->probe_type = PROBE_FORCE_SYNCHRONOUS;
+
+	priv->fake_spi_device = spi_new_device(priv->fake_ctlr, &board_info);
+	/* Restore to original probe type. */
+	drv->probe_type = orig;
+	KUNIT_ASSERT_NOT_NULL(test, priv->fake_spi_device);
+
+	KUNIT_EXPECT_EQ(test, priv->fake_cros_ec_register_called, 1);
+	KUNIT_ASSERT_NOT_NULL(test, priv->ec_dev);
+	KUNIT_EXPECT_EQ(test, priv->fake_cros_ec_unregister_called, 0);
+
+	INIT_LIST_HEAD(&priv->tx_head);
+	INIT_LIST_HEAD(&priv->rx_head);
+
+	return 0;
+}
+
+static void cros_ec_spi_test_exit(struct kunit *test)
+{
+	struct cros_ec_spi_test_priv *priv = test->priv;
+
+	spi_unregister_device(priv->fake_spi_device);
+	KUNIT_EXPECT_EQ(test, priv->fake_cros_ec_unregister_called, 1);
+
+	spi_unregister_controller(priv->fake_ctlr);
+	device_del(&priv->dev);
+	class_destroy(priv->fake_class);
+
+	kunit_deactivate_ftrace_stub(test, cros_ec_register);
+	kunit_deactivate_ftrace_stub(test, cros_ec_unregister);
+}
+
+static int cros_ec_spi_test_cmd_xfer_init(struct kunit *test)
+{
+	struct cros_ec_spi_test_priv *priv;
+
+	cros_ec_spi_test_init(test);
+	priv = test->priv;
+	priv->ec_dev->proto_version = 2;
+	return 0;
+}
+
+static void cros_ec_spi_test_cmd_xfer_normal(struct kunit *test)
+{
+	struct cros_ec_spi_test_priv *priv = test->priv;
+	struct cros_ec_command *msg;
+	struct fake_xt *xt;
+	int ret, i, len;
+	u8 sum;
+
+	msg = kunit_kzalloc(test, sizeof(*msg) + 2 /* max(outsize, insize) */, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, msg);
+	msg->version = 0x1000;
+	msg->command = 0x1001;
+	msg->outsize = 2;
+	msg->insize = 1;
+	msg->data[0] = 0xbe;
+	msg->data[1] = 0xef;
+
+	/*
+	 * cros_ec_spi sends out the msg and tries to read further EC_MSG_TX_PROTO_BYTES +
+	 * msg->outsize bytes to verify if the EC can proceed the command by looking for
+	 * EC_SPI_PAST_END, EC_SPI_RX_BAD_DATA, or EC_SPI_NOT_READY.  It's fine even if we
+	 * prepare 0 length data.
+	 */
+	queue_fake_xt(test, &priv->rx_head, 0);
+
+	len = 1 /* for EC_SPI_FRAME_START */ + EC_MSG_RX_PROTO_BYTES + msg->insize;
+	xt = queue_fake_xt(test, &priv->rx_head, len);
+	xt->buf[0] = EC_SPI_FRAME_START;
+	xt->buf[1] = 0 /* result */;
+	xt->buf[2] = msg->insize;
+	xt->buf[3] = 0xaa;
+	for (sum = 0, i = 1; i < len - 1; ++i)
+		sum += xt->buf[i];
+	xt->buf[len - 1] = sum;
+
+	ret = priv->ec_dev->cmd_xfer(priv->ec_dev, msg);
+	KUNIT_EXPECT_EQ(test, ret, 1 /* insize */);
+	KUNIT_EXPECT_EQ(test, msg->result, 0);
+	KUNIT_EXPECT_EQ(test, msg->data[0], 0xaa);
+
+	KUNIT_EXPECT_FALSE(test, list_empty(&priv->tx_head));
+	xt = dequeue_fake_xt(&priv->tx_head);
+	KUNIT_EXPECT_TRUE(test, list_empty(&priv->tx_head));
+	KUNIT_EXPECT_NOT_NULL(test, xt);
+	KUNIT_EXPECT_NOT_NULL(test, xt->buf);
+	KUNIT_EXPECT_EQ(test, xt->len, EC_MSG_TX_PROTO_BYTES + 2 /* outsize */);
+	KUNIT_EXPECT_EQ(test, xt->buf[0], (u8)(EC_CMD_VERSION0 + 0x1000));
+	KUNIT_EXPECT_EQ(test, xt->buf[1], (u8)0x1001);
+	KUNIT_EXPECT_EQ(test, xt->buf[2], 2 /* outsize */);
+	KUNIT_EXPECT_EQ(test, xt->buf[3], 0xbe);
+	KUNIT_EXPECT_EQ(test, xt->buf[4], 0xef);
+	for (sum = 0, i = 0; i < EC_MSG_TX_HEADER_BYTES + 2 /* outsize */; ++i)
+		sum += xt->buf[i];
+	KUNIT_EXPECT_EQ(test, xt->buf[EC_MSG_TX_HEADER_BYTES + 2 /* outsize */], sum);
+}
+
+static struct kunit_case cros_ec_spi_test_cmd_xfer_cases[] = {
+	KUNIT_CASE(cros_ec_spi_test_cmd_xfer_normal),
+	{}
+};
+
+static struct kunit_suite cros_ec_spi_test_cmd_xfer_suite = {
+	.name = "cros_ec_spi_test_cmd_xfer_suite",
+	.init = cros_ec_spi_test_cmd_xfer_init,
+	.exit = cros_ec_spi_test_exit,
+	.test_cases = cros_ec_spi_test_cmd_xfer_cases,
+};
+
+static int cros_ec_spi_test_pkt_xfer_init(struct kunit *test)
+{
+	struct cros_ec_spi_test_priv *priv;
+
+	cros_ec_spi_test_init(test);
+	priv = test->priv;
+	priv->ec_dev->proto_version = 3;
+	return 0;
+}
+
+static void cros_ec_spi_test_pkt_xfer_normal(struct kunit *test)
+{
+	struct cros_ec_spi_test_priv *priv = test->priv;
+	struct cros_ec_command *msg;
+	struct fake_xt *xt;
+	struct ec_host_request *request;
+	struct ec_host_response *response;
+	int ret, i, len;
+	u8 sum;
+
+	msg = kunit_kzalloc(test, sizeof(*msg) + 2 /* max(outsize, insize) */, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, msg);
+	msg->version = 0x1000;
+	msg->command = 0x1001;
+	msg->outsize = 2;
+	msg->insize = 1;
+	msg->data[0] = 0xbe;
+	msg->data[1] = 0xef;
+
+	/*
+	 * cros_ec_spi sends out the msg and tries to read further EC_MSG_TX_PROTO_BYTES +
+	 * msg->outsize bytes to verify if the EC can proceed the command by looking for
+	 * EC_SPI_PAST_END, EC_SPI_RX_BAD_DATA, or EC_SPI_NOT_READY.  It's fine even if we
+	 * prepare 0 length data.
+	 */
+	queue_fake_xt(test, &priv->rx_head, 0);
+
+	len = 1 /* for EC_SPI_FRAME_START */ + sizeof(*response) + msg->insize;
+	xt = queue_fake_xt(test, &priv->rx_head, len);
+	xt->buf[0] = EC_SPI_FRAME_START;
+	response = (struct ec_host_response *)&xt->buf[1];
+	response->struct_version = EC_HOST_RESPONSE_VERSION;
+	response->result = 0;
+	response->data_len = msg->insize;
+	xt->buf[sizeof(*response) + msg->insize] = 0xaa;
+	for (sum = 0, i = 1; i < len; ++i)
+		sum += xt->buf[i];
+	response->checksum = -sum;
+
+	ret = priv->ec_dev->pkt_xfer(priv->ec_dev, msg);
+	KUNIT_EXPECT_EQ(test, ret, 1 /* insize */);
+	KUNIT_EXPECT_EQ(test, msg->result, 0);
+	KUNIT_EXPECT_EQ(test, msg->data[0], 0xaa);
+
+	KUNIT_EXPECT_FALSE(test, list_empty(&priv->tx_head));
+	xt = dequeue_fake_xt(&priv->tx_head);
+	KUNIT_EXPECT_TRUE(test, list_empty(&priv->tx_head));
+	KUNIT_EXPECT_NOT_NULL(test, xt);
+	KUNIT_EXPECT_NOT_NULL(test, xt->buf);
+	KUNIT_EXPECT_EQ(test, xt->len, sizeof(*request) + 2 /* outsize */);
+	request = (struct ec_host_request *)xt->buf;
+	KUNIT_EXPECT_EQ(test, request->struct_version, EC_HOST_REQUEST_VERSION);
+	KUNIT_EXPECT_EQ(test, request->command_version, (u8)0x1000);
+	KUNIT_EXPECT_EQ(test, request->command, 0x1001);
+	KUNIT_EXPECT_EQ(test, request->data_len, 2 /* outsize */);
+	KUNIT_EXPECT_EQ(test, xt->buf[sizeof(*request)], 0xbe);
+	KUNIT_EXPECT_EQ(test, xt->buf[sizeof(*request) + 1], 0xef);
+	for (sum = 0, i = 0; i < sizeof(*request) + 2 /* outsize */; ++i)
+		sum += xt->buf[i];
+	KUNIT_EXPECT_EQ(test, sum, 0);
+}
+
+static struct kunit_case cros_ec_spi_test_pkt_xfer_cases[] = {
+	KUNIT_CASE(cros_ec_spi_test_pkt_xfer_normal),
+	{}
+};
+
+static struct kunit_suite cros_ec_spi_test_pkt_xfer_suite = {
+	.name = "cros_ec_spi_test_pkt_xfer_suite",
+	.init = cros_ec_spi_test_pkt_xfer_init,
+	.exit = cros_ec_spi_test_exit,
+	.test_cases = cros_ec_spi_test_pkt_xfer_cases,
+};
+
+kunit_test_suites(
+	&cros_ec_spi_test_cmd_xfer_suite,
+	&cros_ec_spi_test_pkt_xfer_suite,
+);
+
+MODULE_LICENSE("GPL");
-- 
2.49.0.1101.gccaa498523-goog

[RFC PATCH 5/7] platform/chrome: kunit: cros_ec_spi: Call .probe() directly

[Tzung-Bi Shih]

Get the spi_driver and call .probe() directly.

For running the tests:

$ ./tools/testing/kunit/kunit.py run \
	--arch=x86_64 \
	--kconfig_add CONFIG_FTRACE=y \
	--kconfig_add CONFIG_FUNCTION_TRACER=y \
	--kconfig_add CONFIG_MODULES=y \
	--kconfig_add CONFIG_DEBUG_KERNEL=y \
	--kconfig_add CONFIG_KALLSYMS_ALL=y \
	--kconfig_add CONFIG_LIVEPATCH=y \
	--kconfig_add CONFIG_KUNIT_FTRACE_STUBS=y \
	--kconfig_add CONFIG_CHROME_PLATFORMS=y \
	--kconfig_add CONFIG_CROS_EC=y \
	--kconfig_add CONFIG_SPI=y \
	--kconfig_add CONFIG_CROS_EC_SPI=y \
	--kconfig_add CONFIG_CROS_KUNIT_EC_SPI_TEST=y \
	cros_ec_spi*

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/cros_ec_spi_test.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_spi_test.c b/drivers/platform/chrome/cros_ec_spi_test.c
index 2a021569a726..52dea75ecabf 100644
--- a/drivers/platform/chrome/cros_ec_spi_test.c
+++ b/drivers/platform/chrome/cros_ec_spi_test.c
@@ -27,6 +27,7 @@ struct cros_ec_spi_test_priv {
 	struct device dev;
 	struct spi_controller *fake_ctlr;
 	struct spi_device *fake_spi_device;
+	struct spi_driver *spi_drv;
 
 	int fake_cros_ec_register_called;
 	struct cros_ec_device *ec_dev;
@@ -117,16 +118,12 @@ static int find_target_driver(struct device_driver *drv, void *data)
 static int cros_ec_spi_test_init(struct kunit *test)
 {
 	struct cros_ec_spi_test_priv *priv;
-	struct spi_board_info board_info = {};
 	int ret;
 	struct device_driver *drv;
-	enum probe_type orig;
 
 	kunit_activate_ftrace_stub(test, cros_ec_register, fake_cros_ec_register);
 	kunit_activate_ftrace_stub(test, cros_ec_unregister, fake_cros_ec_unregister);
 
-	sized_strscpy(board_info.modalias, "cros-ec-spi", SPI_NAME_SIZE);
-
 	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 	test->priv = priv;
@@ -151,21 +148,17 @@ static int cros_ec_spi_test_init(struct kunit *test)
 	ret = spi_register_controller(priv->fake_ctlr);
 	KUNIT_ASSERT_EQ(test, ret, 0);
 
-	/*
-	 * Force to use synchronous probe so that the upcoming SPI device gets
-	 * probed correctly and synchronously.
-	 */
 	ret = bus_for_each_drv(&spi_bus_type, NULL, &drv, find_target_driver);
 	KUNIT_ASSERT_EQ(test, ret, 0);
 	KUNIT_ASSERT_NOT_NULL(test, drv);
-	orig = drv->probe_type;
-	drv->probe_type = PROBE_FORCE_SYNCHRONOUS;
 
-	priv->fake_spi_device = spi_new_device(priv->fake_ctlr, &board_info);
-	/* Restore to original probe type. */
-	drv->probe_type = orig;
+	priv->fake_spi_device = spi_alloc_device(priv->fake_ctlr);
 	KUNIT_ASSERT_NOT_NULL(test, priv->fake_spi_device);
 
+	priv->spi_drv = container_of(drv, struct spi_driver, driver);
+	ret = priv->spi_drv->probe(priv->fake_spi_device);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
 	KUNIT_EXPECT_EQ(test, priv->fake_cros_ec_register_called, 1);
 	KUNIT_ASSERT_NOT_NULL(test, priv->ec_dev);
 	KUNIT_EXPECT_EQ(test, priv->fake_cros_ec_unregister_called, 0);
@@ -180,9 +173,10 @@ static void cros_ec_spi_test_exit(struct kunit *test)
 {
 	struct cros_ec_spi_test_priv *priv = test->priv;
 
-	spi_unregister_device(priv->fake_spi_device);
+	priv->spi_drv->remove(priv->fake_spi_device);
 	KUNIT_EXPECT_EQ(test, priv->fake_cros_ec_unregister_called, 1);
 
+	spi_dev_put(priv->fake_spi_device);
 	spi_unregister_controller(priv->fake_ctlr);
 	device_del(&priv->dev);
 	class_destroy(priv->fake_class);
-- 
2.49.0.1101.gccaa498523-goog

[RFC PATCH 6/7] kunit: Expose 'kprobes stub' API to redirect functions

[Tzung-Bi Shih]

Add function redirection API based on Kprobes.

Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 include/kunit/kprobes_stub.h |  19 ++++++
 lib/kunit/Kconfig            |   7 +++
 lib/kunit/Makefile           |   4 ++
 lib/kunit/kprobes_stub.c     | 113 +++++++++++++++++++++++++++++++++++
 4 files changed, 143 insertions(+)
 create mode 100644 include/kunit/kprobes_stub.h
 create mode 100644 lib/kunit/kprobes_stub.c

diff --git a/include/kunit/kprobes_stub.h b/include/kunit/kprobes_stub.h
new file mode 100644
index 000000000000..af77c86fe48e
--- /dev/null
+++ b/include/kunit/kprobes_stub.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _KUNIT_KPROBES_STUB_H
+#define _KUNIT_KPROBES_STUB_H
+
+struct kunit;
+
+#define kunit_activate_kprobes_stub(test, func, replacement) do { \
+	typecheck_fn(typeof(&func), replacement); \
+	__kunit_activate_kprobes_stub(test, #func, func, replacement); \
+} while (0)
+
+void __kunit_activate_kprobes_stub(struct kunit *test,
+				  const char *name,
+				  void *real_fn_addr,
+				  void *replacement_addr);
+
+void kunit_deactivate_kprobes_stub(struct kunit *test, void *real_fn_addr);
+
+#endif  /* _KUNIT_KPROBES_STUB_H */
diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
index 933fda1df5c3..13fdd3471060 100644
--- a/lib/kunit/Kconfig
+++ b/lib/kunit/Kconfig
@@ -104,4 +104,11 @@ config KUNIT_FTRACE_STUBS
 	  NOTE: this does not work on all architectures (like UML, arm64) and
 	  relies on a lot of magic (see the dependencies list).
 
+config KUNIT_KPROBES_STUBS
+	bool "Support for stubbing out functions in KUnit tests with kprobes"
+	depends on KPROBES
+	help
+	  Builds support for stubbing out functions for the duration of KUnit
+	  test cases or suites using kprobes.
+
 endif # KUNIT
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 0ecb255576e2..727ce86eb61f 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -33,3 +33,7 @@ obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=	kunit-example-test.o
 ifeq ($(CONFIG_KUNIT_FTRACE_STUBS),y)
 kunit-objs +=				ftrace_stub.o
 endif
+
+ifeq ($(CONFIG_KUNIT_KPROBES_STUBS),y)
+kunit-objs +=				kprobes_stub.o
+endif
diff --git a/lib/kunit/kprobes_stub.c b/lib/kunit/kprobes_stub.c
new file mode 100644
index 000000000000..95f1dcba346b
--- /dev/null
+++ b/lib/kunit/kprobes_stub.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit function redirection (kprobes stubbing) API.
+ */
+
+#include <kunit/test.h>
+#include <kunit/kprobes_stub.h>
+
+#include <linux/kprobes.h>
+
+struct kunit_kprobes_stub_ctx {
+	unsigned long real_fn_addr;
+	unsigned long replacement_addr;
+	struct kprobe kp;
+};
+
+static void __kunit_kprobes_stub_resource_free(struct kunit_resource *res)
+{
+	struct kunit_kprobes_stub_ctx *ctx = res->data;
+
+	unregister_kprobe(&ctx->kp);
+	kfree(ctx);
+}
+
+static int kprobe_handler(struct kprobe *kp, struct pt_regs *regs)
+{
+	struct kunit_kprobes_stub_ctx *ctx = container_of(kp, struct kunit_kprobes_stub_ctx, kp);
+
+	instruction_pointer_set(regs, ctx->replacement_addr);
+	return 1;
+}
+
+/* Matching function for kunit_find_resource().  match_data is real_fn_addr. */
+static bool __kunit_kprobes_stub_resource_match(struct kunit *test,
+						struct kunit_resource *res,
+						void *match_real_fn_addr)
+{
+	struct kunit_kprobes_stub_ctx *ctx = res->data;
+
+	/* Make sure the resource is a kprobes stub resource. */
+	if (res->free != &__kunit_kprobes_stub_resource_free)
+		return false;
+
+	return ctx->real_fn_addr == (unsigned long)match_real_fn_addr;
+}
+
+void __kunit_activate_kprobes_stub(struct kunit *test,
+				   const char *name,
+				   void *real_fn_addr,
+				   void *replacement_addr)
+{
+	struct kunit_kprobes_stub_ctx *ctx;
+	struct kunit_resource *res;
+
+	KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL,
+				"Tried to activate a stub for function NULL");
+
+	/* If the replacement address is NULL, deactivate the stub. */
+	if (!replacement_addr) {
+		kunit_deactivate_kprobes_stub(test, real_fn_addr);
+		return;
+	}
+
+	/* Look up any existing stubs for this function, and replace them. */
+	res = kunit_find_resource(test,
+				  __kunit_kprobes_stub_resource_match,
+				  real_fn_addr);
+	if (res) {
+		ctx = res->data;
+		ctx->replacement_addr = (unsigned long)replacement_addr;
+
+		/* We got an extra reference from find_resource(), so put it. */
+		kunit_put_resource(res);
+	} else {
+		ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+		KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, ctx, "Failed to allocate kunit stub for %s",
+						 name);
+
+		ctx->real_fn_addr = (unsigned long)real_fn_addr;
+		ctx->replacement_addr = (unsigned long)replacement_addr;
+
+		ctx->kp.addr = real_fn_addr;
+		ctx->kp.pre_handler = kprobe_handler;
+		KUNIT_ASSERT_EQ_MSG(test, register_kprobe(&ctx->kp), 0,
+				    "Failed to allocate kunit stub for %s", name);
+
+		kunit_alloc_resource(test, NULL,
+				     __kunit_kprobes_stub_resource_free,
+				     GFP_KERNEL, ctx);
+	}
+}
+EXPORT_SYMBOL_GPL(__kunit_activate_kprobes_stub);
+
+void kunit_deactivate_kprobes_stub(struct kunit *test, void *real_fn_addr)
+{
+	struct kunit_resource *res;
+
+	KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL, "Tried to deactivate a NULL stub.");
+
+	res = kunit_find_resource(test,
+				  __kunit_kprobes_stub_resource_match,
+				  real_fn_addr);
+	KUNIT_ASSERT_PTR_NE_MSG(test, res, NULL,
+				"Tried to deactivate a nonexistent stub.");
+
+	/*
+	 * Free the stub. We 'put' twice, as we got a reference
+	 * from kunit_find_resource()
+	 */
+	kunit_remove_resource(test, res);
+	kunit_put_resource(res);
+}
+EXPORT_SYMBOL_GPL(kunit_deactivate_kprobes_stub);
-- 
2.49.0.1101.gccaa498523-goog

[RFC PATCH 7/7] platform/chrome: kunit: cros_ec_spi: Use kprobes stub instead

[Tzung-Bi Shih]

For running the tests:

$ ./tools/testing/kunit/kunit.py run \
	--arch=x86_64 \
	--kconfig_add CONFIG_KPROBES=y \
	--kconfig_add CONFIG_KUNIT_KPROBES_STUBS=y \
	--kconfig_add CONFIG_CHROME_PLATFORMS=y \
	--kconfig_add CONFIG_CROS_EC=y \
	--kconfig_add CONFIG_SPI=y \
	--kconfig_add CONFIG_CROS_EC_SPI=y \
	--kconfig_add CONFIG_CROS_KUNIT_EC_SPI_TEST=y \
	cros_ec_spi*

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/Kconfig            |  2 +-
 drivers/platform/chrome/cros_ec_spi_test.c | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index aa13e871a31f..aacce3323384 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -339,7 +339,7 @@ config CROS_KUNIT_EC_SPI_TEST
 	tristate "Kunit tests for ChromeOS EC over SPI" if !KUNIT_ALL_TESTS
 	depends on KUNIT && CROS_EC
 	default KUNIT_ALL_TESTS
-	depends on KUNIT_FTRACE_STUBS
+	depends on KUNIT_KPROBES_STUBS
 	depends on CROS_EC_SPI
 	help
 	  Kunit tests for ChromeOS EC over SPI.
diff --git a/drivers/platform/chrome/cros_ec_spi_test.c b/drivers/platform/chrome/cros_ec_spi_test.c
index 52dea75ecabf..74fcbd37b87b 100644
--- a/drivers/platform/chrome/cros_ec_spi_test.c
+++ b/drivers/platform/chrome/cros_ec_spi_test.c
@@ -3,7 +3,7 @@
  * Kunit tests for ChromeOS Embedded Controller SPI interface.
  */
 #include <kunit/test.h>
-#include <kunit/ftrace_stub.h>
+#include <kunit/kprobes_stub.h>
 
 #include <linux/platform_data/cros_ec_commands.h>
 #include <linux/platform_data/cros_ec_proto.h>
@@ -121,8 +121,8 @@ static int cros_ec_spi_test_init(struct kunit *test)
 	int ret;
 	struct device_driver *drv;
 
-	kunit_activate_ftrace_stub(test, cros_ec_register, fake_cros_ec_register);
-	kunit_activate_ftrace_stub(test, cros_ec_unregister, fake_cros_ec_unregister);
+	kunit_activate_kprobes_stub(test, cros_ec_register, fake_cros_ec_register);
+	kunit_activate_kprobes_stub(test, cros_ec_unregister, fake_cros_ec_unregister);
 
 	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
@@ -181,8 +181,8 @@ static void cros_ec_spi_test_exit(struct kunit *test)
 	device_del(&priv->dev);
 	class_destroy(priv->fake_class);
 
-	kunit_deactivate_ftrace_stub(test, cros_ec_register);
-	kunit_deactivate_ftrace_stub(test, cros_ec_unregister);
+	kunit_deactivate_kprobes_stub(test, cros_ec_register);
+	kunit_deactivate_kprobes_stub(test, cros_ec_unregister);
 }
 
 static int cros_ec_spi_test_cmd_xfer_init(struct kunit *test)
-- 
2.49.0.1101.gccaa498523-goog

Re: [RFC PATCH 0/7] platform/chrome: Add Kunit tests for protocol device drivers

[Daniel Latypov]

On Tue, May 20, 2025 at 1:25 AM 'Tzung-Bi Shih' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> The protocol device drivers under drivers/platform/chrome/ are responsible
> to communicate to the ChromeOS EC (Embedded Controller).  They need to pack
> the data in a pre-defined format and check if the EC responds accordingly.
>
> The series adds some fundamental unit tests for the protocol.  It calls the
> .cmd_xfer() and .pkt_xfer() callbacks (which are the most crucial parts for
> the protocol), mocks the rest of the system, and checks if the interactions
> are all correct.
>
> The series isn't ready for landing.  It's more like a PoC for the
> binary-level function redirection and its use cases.
>
> The 1st patch adds ftrace stub which is originally from [1][2].  There is no
> follow-up discussion about the ftrace stub.  As a result, the patch is still
> on the mailing list.
>
> The 2nd patch adds Kunit tests for cros_ec_i2c.  It relies on the ftrace stub
> for redirecting cros_ec_{un,}register().
>
> The 3rd patch uses static stub instead (if ftrace stub isn't really an option).
> However, I'm not a big fan to change the production code (i.e. adding the
> prologue in cros_ec_{un,}register()) for testing.
>
> The 4th patch adds Kunit tests for cros_ec_spi.  It relies on the ftrace stub
> for redirecting cros_ec_{un,}register() again.
>
> The 5th patch calls .probe() directly instead of forcing the driver probe
> needs to be synchronous.  In comparison with the 4th patch, I don't think
> this is simpler.  I'd prefer to the way in the 4th patch.
>
> After talked to Masami about the work, he suggested to use Kprobes for
> function redirection.  The 6th patch adds kprobes stub.
>
> The 7th patch uses kprobes stub instead for cros_ec_spi.
>
> Questions:
> - Are we going to support ftrace stub so that tests can use it?
>
> - If ftrace stub isn't on the plate (e.g. due to too many dependencies), how
>   about the kprobes stub?  Is it something we could pursue?

Quick comment,
If I recall, the thought process was that we could consider it in the
future if there was enough demand for it.

We have these drawbacks with the current ftrace stubs:
* doesn't compile on all arches
* silently doesn't work on inlined functions <== scariest one to me
* is more complicated and has more dependencies

So it felt like the better move to go with static stubs which has none
of those drawbacks (works on all arches, all functions, and is dead
simple) as opposed to simultaneously introducing two ways to do the
same thing.

You mention you don't like how static stubs requires modifying the
code-under-test.
Since it gets eliminated by the preprocessor unless you're compiling
for KUnit, is the concern more so about how it conceptually feels
wrong to do so?
For the Android GKI kernel, they have (or had) KUnit enabled so there
is potentially concern about real runtime cost there, not sure if you
have something similar in mind.

But stepping back, ftrace_stubs technically require modifying the code
to make sure funcs are marked as `noinline`, which this patch series
does not do.
I've not looked at cros_ec_{un,}register() to check if they're at risk
of inlining, but wanted to call that out, that ftrace stubs
technically don't handle your usage pattern 100% properly.

>
> - (minor) I'm unsure if people would prefer 'kprobes stub' vs. 'kprobe stub'.
>

I'd personally vote for kprobe_stub.

Daniel

Re: [RFC PATCH 0/7] platform/chrome: Add Kunit tests for protocol device drivers

[Tzung-Bi Shih]

On Tue, May 20, 2025 at 09:04:53AM -0700, Daniel Latypov wrote:
> On Tue, May 20, 2025 at 1:25 AM 'Tzung-Bi Shih' via KUnit Development
> <kunit-dev@googlegroups.com> wrote:
> > [...]
> > Questions:
> > - Are we going to support ftrace stub so that tests can use it?
> >
> > - If ftrace stub isn't on the plate (e.g. due to too many dependencies), how
> >   about the kprobes stub?  Is it something we could pursue?
> 
> Quick comment,
> If I recall, the thought process was that we could consider it in the
> future if there was enough demand for it.

Sorry for the late reply. I was doing some more experiments and stuck by
some else.

> We have these drawbacks with the current ftrace stubs:
> * doesn't compile on all arches
> * silently doesn't work on inlined functions <== scariest one to me

I see. I did some experiments and realized that kprobe stubs also share
the same concern. Thus I'm wondering if there is a way that kprobe stub
detects the redirection may fail, at least it can skip the test case
(e.g. register_kprobe() returns -ENOENT when it can't find the symbol
via kprobe_lookup_name()). But it seems no way if the target function
is partially inlined.

> You mention you don't like how static stubs requires modifying the
> code-under-test.
> Since it gets eliminated by the preprocessor unless you're compiling
> for KUnit, is the concern more so about how it conceptually feels
> wrong to do so?
> For the Android GKI kernel, they have (or had) KUnit enabled so there
> is potentially concern about real runtime cost there, not sure if you
> have something similar in mind.

Not exactly. Ideally, I think we shouldn't modify the CUT. I'm wondering
if there is a way to not change the CUT but also break the external
dependencies.

> But stepping back, ftrace_stubs technically require modifying the code
> to make sure funcs are marked as `noinline`, which this patch series
> does not do.
> I've not looked at cros_ec_{un,}register() to check if they're at risk
> of inlining, but wanted to call that out, that ftrace stubs
> technically don't handle your usage pattern 100% properly.

True. They could be partially inlined even though they are exported symbols.

Re: [RFC PATCH 0/7] platform/chrome: Add Kunit tests for protocol device drivers

[Daniel Latypov]

On Mon, Jun 30, 2025 at 3:32 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Tue, May 20, 2025 at 09:04:53AM -0700, Daniel Latypov wrote:

(snip)

>
> > We have these drawbacks with the current ftrace stubs:
> > * doesn't compile on all arches
> > * silently doesn't work on inlined functions <== scariest one to me
>
> I see. I did some experiments and realized that kprobe stubs also share
> the same concern. Thus I'm wondering if there is a way that kprobe stub
> detects the redirection may fail, at least it can skip the test case
> (e.g. register_kprobe() returns -ENOENT when it can't find the symbol
> via kprobe_lookup_name()). But it seems no way if the target function
> is partially inlined.

Yeah, any such approach will need to be cautious of inlining, so
they'd have to always be "for power users only: only use this if you
understand the risk"
Skipping when we can detect the user is obviously doing the wrong
thing makes that a *bit* more palatable.

It's not like static stubs can detect at runtime if you've failed to
add the necessary corresponding KUNIT_STATIC_STUB_REDIRECT() either,
for example.

But I still personally lean slightly against adding either kprobe or
ftrace stubs, see below.

>
> > You mention you don't like how static stubs requires modifying the
> > code-under-test.
> > Since it gets eliminated by the preprocessor unless you're compiling
> > for KUnit, is the concern more so about how it conceptually feels
> > wrong to do so?
> > For the Android GKI kernel, they have (or had) KUnit enabled so there
> > is potentially concern about real runtime cost there, not sure if you
> > have something similar in mind.
>
> Not exactly. Ideally, I think we shouldn't modify the CUT. I'm wondering
> if there is a way to not change the CUT but also break the external
> dependencies.
>
> > But stepping back, ftrace_stubs technically require modifying the code
> > to make sure funcs are marked as `noinline`, which this patch series
> > does not do.
...
> They could be partially inlined even though they are exported symbols.

So to summarize, right now we're stuck with having to modify the code.
(Unless someone can come up with something really clever, but not too clever)

To make it concrete, the current approach would look like:

int func(char* arg1, int arg2) {
  KUNIT_STATIC_STUB_REDIRECT(func, arg1, arg2);
  ... // unchanged
}

vs an ftrace/kprobe approach that needs a conditional `noinline`

KUNIT_STUBBABLE int func(char* arg1, int arg2) {
  ... // unchanged
}

The latter is definitely simpler and less burdensome.
But I don't know if it's simpler enough to warrant a second
implementation existing for me personally.

E.g. we already have some people who justifiably say it's too hard to
figure out KUnit, so this is another decision point where a user might
get stuck with "how should I know which one I should use?" and give
up.

Compatibility tangent:
A smaller annoyance is KUNIT_STATIC_STUB_REDIRECT and KUNIT_STUBBABLE
are incompatible (and always will be?)

E.g. imagine a func has KUNIT_STUBBABLE on it, but a person authoring
a new test wants needs to run without kprobe support, so they must add
KUNIT_STATIC_STUB_REDIRECT.
I can imagine an author deciding to make the func have *both* macros on it...
That feels like a worst case outcome from the perspective of "we
shouldn't modify the CUT just for the sake of tests" :\

To be clear, the right approach in this scenario is to 1) swap to
KUNIT_STATIC_STUB_REDIRECT and update the previous tests to use static
stubs, 2) then add your new tests.
But I can imagine that won't always happen, esp. if it crosses
maintainer boundaries.

Re: [RFC PATCH 0/7] platform/chrome: Add Kunit tests for protocol device drivers

[Tzung-Bi Shih]

On Tue, Jul 01, 2025 at 02:22:40PM +0900, Daniel Latypov wrote:
> On Mon, Jun 30, 2025 at 3:32 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > On Tue, May 20, 2025 at 09:04:53AM -0700, Daniel Latypov wrote:
> > > You mention you don't like how static stubs requires modifying the
> > > code-under-test.
> > > Since it gets eliminated by the preprocessor unless you're compiling
> > > for KUnit, is the concern more so about how it conceptually feels
> > > wrong to do so?
> > > For the Android GKI kernel, they have (or had) KUnit enabled so there
> > > is potentially concern about real runtime cost there, not sure if you
> > > have something similar in mind.
> >
> > Not exactly. Ideally, I think we shouldn't modify the CUT. I'm wondering
> > if there is a way to not change the CUT but also break the external
> > dependencies.
> >
> > > But stepping back, ftrace_stubs technically require modifying the code
> > > to make sure funcs are marked as `noinline`, which this patch series
> > > does not do.
> ...
> > They could be partially inlined even though they are exported symbols.
> 
> So to summarize, right now we're stuck with having to modify the code.
> (Unless someone can come up with something really clever, but not too clever)
> 
> To make it concrete, the current approach would look like:
> 
> int func(char* arg1, int arg2) {
>   KUNIT_STATIC_STUB_REDIRECT(func, arg1, arg2);
>   ... // unchanged
> }
> 
> vs an ftrace/kprobe approach that needs a conditional `noinline`
> 
> KUNIT_STUBBABLE int func(char* arg1, int arg2) {
>   ... // unchanged
> }
> 
> The latter is definitely simpler and less burdensome.
> But I don't know if it's simpler enough to warrant a second
> implementation existing for me personally.

Instead of KUNIT_STUBBABLE macros, I was thinking of:

diff --git a/Makefile b/Makefile
index 35e6e5240c61..40319083f58b 100644
--- a/Makefile
+++ b/Makefile
@@ -979,6 +979,10 @@ ifdef CONFIG_DEBUG_SECTION_MISMATCH
 KBUILD_CFLAGS += -fno-inline-functions-called-once
 endif
 
+ifdef CONFIG_KUNIT_KPROBE_STUBS
+KBUILD_CFLAGS += -fno-inline
+endif
+
 # `rustc`'s `-Zfunction-sections` applies to data too (as of 1.59.0).
 ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
 KBUILD_CFLAGS_KERNEL += -ffunction-sections -fdata-sections


I don't know what are most people's usages. I always run KUnit tests in qemu
or at least in some real devices that I less care about the performance.
Thus, turning inline off globally in such environments is totally acceptable.