Message-ID: <CABVgOSmdcOZ0+-k=SM4LibOVMKtcbF27p6N40kuDX_axTPZ=QQ@mail.gmail.com>
Date: Fri, 20 Jun 2025 17:47:39 +0800
From: David Gow <davidgow@...gle.com>
To: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Cc: Masahiro Yamada <masahiroy@...nel.org>, Nathan Chancellor <nathan@...nel.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, Willy Tarreau <w@....eu>, 
	Thomas Weißschuh <linux@...ssschuh.net>, 
	Brendan Higgins <brendan.higgins@...ux.dev>, Rae Moar <rmoar@...gle.com>, 
	Shuah Khan <shuah@...nel.org>, Jonathan Corbet <corbet@....net>, 
	Nicolas Schier <nicolas.schier@...ux.dev>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, 
	Alexandre Ghiti <alex@...ti.fr>, Christophe Leroy <christophe.leroy@...roup.eu>, 
	linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, 
	linux-doc@...r.kernel.org, linux-riscv@...ts.infradead.org, 
	workflows@...r.kernel.org
Subject: Re: [PATCH v3 13/16] kunit: Introduce UAPI testing framework

On Wed, 11 Jun 2025 at 15:38, Thomas Weißschuh
<thomas.weissschuh@...utronix.de> wrote:
>
> Enable running UAPI tests as part of kunit.
> The selftests are embedded into the kernel image and their output is
> forwarded to kunit for unified reporting.
>
> The implementation reuses parts of usermode drivers and usermode
> helpers. However these frameworks are not used directly as they make it
> impossible to retrieve a thread's exit code.
>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
>
> ---

It feels to me like there are three features hidden in here:
- KUnit helpers for manipulating vfs files
- A way of having KUnit tests run userspace helpers
- The full framework for writing/running whole tests in userspace.

It's really the first two which excite me personally most -- as they
give us a sort-of inverse to the kselftest "helper module" paradigm --
so we can test things which are impossible to test from within
kernelspace. So maybe those APIs should be exposed separately (so a
test can be written mostly in kernel-space using the KUnit framework
APIs, and just call out to a helper where needed). But I'm happy for
them to stay private functions until we have a test which actually
needs them.


> Currently this depends on CONFIG_KUNIT=y as it uses some non-exported
> symbols around process management.

That's fine for now, IMHO, but will make it difficult to use this on,
e.g., Red Hat setups, where CONFIG_KUNIT=m. Hopefully we can resolve
this by exporting some of the symbols?


In general, I'm happy with the implementation here. The fs stuff
probably needs a closer look from someone who knows the vfs better
than me, though.

Nevertheless,
Reviewed-by: David Gow <davidgow@...gle.com>

Cheers,
-- David

> ---
>  Documentation/dev-tools/kunit/api/index.rst |   5 +
>  Documentation/dev-tools/kunit/api/uapi.rst  |  12 ++
>  include/kunit/uapi.h                        |  24 +++
>  lib/kunit/Kconfig                           |  10 +
>  lib/kunit/Makefile                          |   2 +
>  lib/kunit/uapi.c                            | 287 ++++++++++++++++++++++++++++
>  6 files changed, 340 insertions(+)
>
> diff --git a/Documentation/dev-tools/kunit/api/index.rst b/Documentation/dev-tools/kunit/api/index.rst
> index 5cdb552a0808f25baeff5e47a9227b7b62c69e40..34d8fee9a97059d6da919a6fb1a7e359b5e0beef 100644
> --- a/Documentation/dev-tools/kunit/api/index.rst
> +++ b/Documentation/dev-tools/kunit/api/index.rst
> @@ -9,6 +9,7 @@ API Reference
>         test
>         resource
>         functionredirection
> +       uapi
>         clk
>         of
>         platformdevice
> @@ -32,6 +33,10 @@ Documentation/dev-tools/kunit/api/functionredirection.rst
>
>   - Documents the KUnit Function Redirection API
>
> +Documentation/dev-tools/kunit/api/uapi.rst
> +
> + - Documents the KUnit Userspace testing API
> +
>  Driver KUnit API
>  ================
>
> diff --git a/Documentation/dev-tools/kunit/api/uapi.rst b/Documentation/dev-tools/kunit/api/uapi.rst
> new file mode 100644
> index 0000000000000000000000000000000000000000..b4764424c629bf69194cf2786f52aef154b02bf8
> --- /dev/null
> +++ b/Documentation/dev-tools/kunit/api/uapi.rst
> @@ -0,0 +1,12 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +==================
> +Userspace Test API
> +==================
> +
> +This file documents all of the userspace testing API.
> +Userspace tests should be built as :ref:`userprogs <kbuild_userprogs>` and included into the test
> +module or kernel as :ref:`blobs <kbuild_blobs>`.
> +
> +.. kernel-doc:: include/kunit/uapi.h
> +   :internal:
> diff --git a/include/kunit/uapi.h b/include/kunit/uapi.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..a6181790c96a42df05839097991c1fbfd889cdbe
> --- /dev/null
> +++ b/include/kunit/uapi.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * KUnit Userspace testing API.
> + *
> + * Copyright (C) 2025, Linutronix GmbH.
> + * Author: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
> + */
> +
> +#ifndef _KUNIT_UAPI_H
> +#define _KUNIT_UAPI_H
> +
> +struct blob;
> +struct kunit;
> +
> +/**
> + * kunit_uapi_run_kselftest() - Run a userspace kselftest as part of kunit
> + * @test: The test context object.
> + * @executable: kselftest executable to run
> + *
> + * Runs the kselftest and forwards its TAP output and exit status to kunit.
> + */
> +void kunit_uapi_run_kselftest(struct kunit *test, const struct blob *executable);
> +
> +#endif /* _KUNIT_UAPI_H */
> diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> index a97897edd9642f3e5df7fdd9dee26ee5cf00d6a4..1f2f5f2213a7d8438cd2683955f22e34f3a036dd 100644
> --- a/lib/kunit/Kconfig
> +++ b/lib/kunit/Kconfig
> @@ -93,4 +93,14 @@ 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_UAPI
> +       def_bool y

Maybe it's worth making this configurable separately? I could imagine
people wanting an easy way to build a kernel without all of the test
blobs built-in.

> +       depends on KUNIT=y
> +       depends on CC_CAN_LINK_STATIC || ARCH_HAS_NOLIBC
> +       select HEADERS_INSTALL
> +       help
> +         Enables support for building and running userspace selftests as part of kunit.
> +         These tests should be statically linked and use kselftest.h or kselftest_harness.h
> +         for status reporting.
> +
>  endif # KUNIT
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index 656f1fa35abcc635e67d5b4cb1bc586b48415ac5..dafa09bd4241c24d31c4c19edecb67bf724127d7 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -12,6 +12,8 @@ kunit-objs +=                         test.o \
>                                         device.o \
>                                         platform.o
>
> +kunit-$(CONFIG_KUNIT_UAPI) +=          uapi.o
> +
>  ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
>  kunit-objs +=                          debugfs.o
>  endif
> diff --git a/lib/kunit/uapi.c b/lib/kunit/uapi.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..121146dda533b3f90aca37c20bd0e7a1d20cb3b5
> --- /dev/null
> +++ b/lib/kunit/uapi.c
> @@ -0,0 +1,287 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit Userspace testing API.
> + *
> + * Copyright (C) 2025, Linutronix GmbH.
> + * Author: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
> + */
> +
> +#include <linux/binfmts.h>
> +#include <linux/blob.h>
> +#include <linux/file.h>
> +#include <linux/fs.h>
> +#include <linux/fs_struct.h>
> +#include <linux/pid.h>
> +#include <linux/pipe_fs_i.h>
> +#include <linux/sched/task.h>
> +#include <linux/types.h>
> +
> +#include <kunit/test.h>
> +#include <kunit/uapi.h>
> +
> +#define KSFT_PASS      0
> +#define KSFT_FAIL      1
> +#define KSFT_XFAIL     2
> +#define KSFT_XPASS     3
> +#define KSFT_SKIP      4
> +
> +static struct vfsmount *kunit_uapi_mount_ramfs(void)
> +{
> +       struct file_system_type *type;
> +       struct vfsmount *mnt;
> +
> +       type = get_fs_type("ramfs");
> +       if (!type)
> +               return ERR_PTR(-ENODEV);
> +
> +       /* FIXME
> +        * The mount setup is supposed to look like this:
> +        * kunit_uapi_mount_ramfs() sets up a private mount,
> +        * with nothing visible except the new tmpfs.
> +        * Then each executable execution gets a new namespace on top of that
> +        * on which it can mount whatever it needs.
> +        * However I didn't manage to set this up, so keep everything simple
> +        * for now and let somebody familiar with the VFS figure this out.
> +        */
> +
> +       mnt = kern_mount(type);
> +       put_filesystem(type);
> +
> +       return mnt;
> +}
> +
> +static int kunit_uapi_write_file(struct vfsmount *mnt, const char *name, mode_t mode,
> +                                const u8 *data, size_t size)
> +{
> +       struct file *file;
> +       ssize_t written;
> +
> +       file = file_open_root_mnt(mnt, name, O_CREAT | O_WRONLY, mode);
> +       if (IS_ERR(file))
> +               return PTR_ERR(file);
> +
> +       written = kernel_write(file, data, size, NULL);
> +       filp_close(file, NULL);
> +       if (written != size) {
> +               if (written >= 0)
> +                       return -ENOMEM;
> +               return written;
> +       }
> +
> +       /* Flush delayed fput so exec can open the file read-only */
> +       flush_delayed_fput();
> +
> +       return 0;
> +}
> +
> +struct kunit_uapi_user_mode_thread_ctx {
> +       const char *executable;
> +
> +       /* Signals mnt, out, pwd and tgid */
> +       struct completion setup_done;
> +       struct vfsmount *mnt;
> +       struct file *out;
> +       struct path pwd;
> +       pid_t tgid;
> +
> +       /* Valid after wait(tgid) */
> +       int exec_err;
> +};
> +
> +static int kunit_uapi_user_mode_thread_init(void *data)
> +{
> +       struct kunit_uapi_user_mode_thread_ctx *ctx = data;
> +       const char *const argv[] = {
> +               ctx->executable,
> +               NULL
> +       };
> +       struct file *out[2];
> +       int err;
> +
> +       err = create_pipe_files(out, 0);
> +       if (err)
> +               return err;
> +
> +       /* stdin, use the *write* end to the pipe to have an unreadable input */
> +       err = replace_fd(0, out[1], 0);
> +       if (err < 0) {
> +               fput(out[0]);
> +               fput(out[1]);
> +               return err;
> +       }
> +
> +       /* stdout */
> +       err = replace_fd(1, out[1], 0);
> +       if (err < 0) {
> +               replace_fd(0, NULL, 0);
> +               fput(out[0]);
> +               fput(out[1]);
> +               return err;
> +       }
> +
> +       /* stderr */
> +       err = replace_fd(2, out[1], 0);
> +       if (err < 0) {
> +               replace_fd(0, NULL, 0);
> +               replace_fd(1, NULL, 0);
> +               fput(out[0]);
> +               fput(out[1]);
> +               return err;
> +       }
> +
> +       fput(out[1]);
> +
> +       ctx->out = out[0];
> +       ctx->tgid = current->tgid;
> +
> +       set_fs_pwd(current->fs, &ctx->pwd);
> +       kernel_sigaction(SIGKILL, SIG_DFL);
> +       kernel_sigaction(SIGABRT, SIG_DFL);
> +
> +       complete(&ctx->setup_done);
> +       ctx->exec_err = kernel_execve(ctx->executable, argv, NULL);
> +       if (!ctx->exec_err)
> +               return 0;
> +       do_exit(0);
> +}
> +
> +static size_t kunit_uapi_printk_subtest_lines(struct kunit *test, char *buf, size_t s)
> +{
> +       const char *ptr = buf, *newline;
> +       size_t n;
> +
> +       while (s) {
> +               newline = strnchr(ptr, s, '\n');
> +               if (!newline)
> +                       break;
> +
> +               n = newline - ptr + 1;
> +
> +               kunit_log(KERN_INFO, test, KUNIT_SUBSUBTEST_INDENT "%.*s", (int)n, ptr);
> +               ptr += n;
> +               s -= n;
> +       }
> +
> +       memmove(buf, ptr, s);
> +
> +       return s;
> +}
> +
> +static int kunit_uapi_forward_to_printk(struct kunit *test, struct file *output)
> +{
> +       /*
> +        * printk() automatically adds a newline after each message.
> +        * Therefore only fully accumulated lines can be forwarded.
> +        * Each line needs to fit into the buffer below.
> +        */
> +       char buf[512];
> +       size_t s = 0;
> +       ssize_t n;
> +
> +       while (1) {
> +               n = kernel_read(output, buf + s, sizeof(buf) - s, NULL);
> +               if (n <= 0)
> +                       return n;
> +               s = kunit_uapi_printk_subtest_lines(test, buf, s + n);
> +       }
> +}
> +
> +static void kunit_uapi_kill_pid(pid_t pid)
> +{
> +       struct pid *p;
> +
> +       p = find_get_pid(pid);
> +       kill_pid(p, SIGKILL, 1);
> +       put_pid(p);
> +}
> +
> +static int kunit_uapi_run_executable_in_mount(struct kunit *test, const char *executable,
> +                                                  struct vfsmount *mnt)
> +{
> +       struct kunit_uapi_user_mode_thread_ctx ctx = {
> +               .setup_done     = COMPLETION_INITIALIZER_ONSTACK(ctx.setup_done),
> +               .executable     = executable,
> +               .pwd            = {
> +                       .mnt    = mnt,
> +                       .dentry = mnt->mnt_root,
> +               },
> +       };
> +       int forward_err, wait_err, ret;
> +       pid_t pid;
> +
> +       /* If SIGCHLD is ignored do_wait won't populate the status. */
> +       kernel_sigaction(SIGCHLD, SIG_DFL);
> +       pid = user_mode_thread(kunit_uapi_user_mode_thread_init, &ctx, SIGCHLD);
> +       if (pid < 0) {
> +               kernel_sigaction(SIGCHLD, SIG_IGN);
> +               return pid;
> +       }
> +
> +       wait_for_completion(&ctx.setup_done);
> +
> +       forward_err = kunit_uapi_forward_to_printk(test, ctx.out);
> +       if (forward_err)
> +               kunit_uapi_kill_pid(ctx.tgid);
> +
> +       wait_err = kernel_wait(ctx.tgid, &ret);
> +
> +       /* Restore default kernel sig handler */
> +       kernel_sigaction(SIGCHLD, SIG_IGN);
> +
> +       if (ctx.exec_err)
> +               return ctx.exec_err;
> +       if (forward_err)
> +               return forward_err;
> +       if (wait_err < 0)
> +               return wait_err;
> +       return ret;
> +}
> +
> +static int kunit_uapi_run_executable(struct kunit *test, const struct blob *executable)
> +{
> +       const char *exe_name = kbasename(executable->path);
> +       struct vfsmount *mnt;
> +       int err;
> +
> +       mnt = kunit_uapi_mount_ramfs();
> +       if (IS_ERR(mnt))
> +               return PTR_ERR(mnt);
> +
> +       err = kunit_uapi_write_file(mnt, exe_name, 0755, executable->data, blob_size(executable));
> +
> +       if (!err)
> +               err = kunit_uapi_run_executable_in_mount(test, exe_name, mnt);
> +
> +       kern_unmount(mnt);
> +
> +       return err;
> +}
> +
> +void kunit_uapi_run_kselftest(struct kunit *test, const struct blob *executable)
> +{
> +       u8 exit_code, exit_signal;
> +       int err;
> +
> +       err = kunit_uapi_run_executable(test, executable);
> +       if (err < 0)
> +               KUNIT_FAIL(test, "Could not run test executable: %pe\n", ERR_PTR(err));
> +
> +       exit_code = err >> 8;
> +       exit_signal = err & 0xff;
> +
> +       if (exit_signal)
> +               KUNIT_FAIL(test, "kselftest exited with signal: %d\n", exit_signal);
> +       else if (exit_code == KSFT_PASS)
> +               ; /* Noop */
> +       else if (exit_code == KSFT_FAIL)
> +               KUNIT_FAIL(test, "kselftest exited with code KSFT_FAIL\n");
> +       else if (exit_code == KSFT_XPASS)
> +               KUNIT_FAIL(test, "kselftest exited with code KSFT_XPASS\n");
> +       else if (exit_code == KSFT_XFAIL)
> +               ; /* Noop */
> +       else if (exit_code == KSFT_SKIP)
> +               kunit_mark_skipped(test, "kselftest exited with code KSFT_SKIP\n");
> +       else
> +               KUNIT_FAIL(test, "kselftest exited with unknown exit code: %d\n", exit_code);
> +}
> +EXPORT_SYMBOL_GPL(kunit_uapi_run_kselftest);
>
> --
> 2.49.0
>

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5281 bytes)

Powered by blists - more mailing lists