* [PATCH v1 0/4] kunit: tool: add support for QEMU
@ 2021-05-07 21:31 Brendan Higgins
2021-05-07 21:31 ` [PATCH v1 1/4] kunit: Add 'kunit_shutdown' option Brendan Higgins
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Brendan Higgins @ 2021-05-07 21:31 UTC (permalink / raw)
To: shuah, davidgow
Cc: linux-kselftest, kunit-dev, linux-kernel, corbet, linux-doc,
sboyd, keescook, frowand.list, dlatypov, Brendan Higgins
TL;DR: Add support to kunit_tool to dispatch tests via QEMU. Also add
support to immediately shutdown a kernel after running KUnit tests.
Background
----------
KUnit has supported running on all architectures for quite some time;
however, kunit_tool - the script commonly used to invoke KUnit tests -
has only fully supported KUnit run on UML. Its functionality has been
broken up for some time to separate the configure, build, run, and parse
phases making it possible to be used in part on other architectures to a
small extent. Nevertheless, kunit_tool has not supported running tests
on other architectures.
What this patchset does
-----------------------
This patchset introduces first class support to kunit_tool for KUnit to
be run on many popular architectures via QEMU. It does this by adding
two new flags: `--arch` and `--cross_compile`.
`--arch` allows an architecture to be specified by the name the
architecture is given in `arch/`. It uses the specified architecture to
select a minimal amount of Kconfigs and QEMU configs needed for the
architecture to run in QEMU and provide a console from which KTAP
results can be scraped.
`--cross_compile` allows a toolchain prefix to be specified to make
similar to how `CROSS_COMPILE` is used.
Additionally, this patchset revives the previously considered "kunit:
tool: add support for QEMU"[1] patchs. The motivation for this new
kernel command line flags, `kunit_shutdown`, is to better support
running KUnit tests inside of QEMU. For most popular architectures, QEMU
can be made to terminate when the Linux kernel that is being run is
reboted, halted, or powered off. As Kees pointed out in a previous
discussion[2], it is possible to make a kernel initrd that can reboot
the kernel immediately, doing this for every architecture would likely
be infeasible. Instead, just having an option for the kernel to shutdown
when it is done with testing seems a lot simpler, especially since it is
an option which would only available in testing configurations of the
kernel anyway.
Changes since last revision
---------------------------
Mostly fixed lots of minor issues suggested/poited out by David and
Daniel. Also reworked how qemu_configs are loaded: Now each config is in
its own Python file and is loaded dynamically. Given the number of
improvements that address the biggest concerns I had in the last RFC, I
decided to promote this to a normal patch set.
What discussion remains for this patchset?
------------------------------------------
I am still hoping to see some discussion regarding the kunit_shutdown
patch: I want to make sure with the added context of QEMU running under
kunit_tool that this is now a reasonable approach. Nevertheless, I am
pretty happy with this patchset as is, and I did not get any negative
feedback on the previous revision, so I think we can probably just move
forward as is.
Brendan Higgins (3):
Documentation: Add kunit_shutdown to kernel-parameters.txt
kunit: tool: add support for QEMU
Documentation: kunit: document support for QEMU in kunit_tool
David Gow (1):
kunit: Add 'kunit_shutdown' option
.../admin-guide/kernel-parameters.txt | 8 +
Documentation/dev-tools/kunit/usage.rst | 37 +++-
lib/kunit/executor.c | 20 ++
tools/testing/kunit/kunit.py | 57 +++++-
tools/testing/kunit/kunit_config.py | 7 +-
tools/testing/kunit/kunit_kernel.py | 172 +++++++++++++++---
tools/testing/kunit/kunit_parser.py | 2 +-
tools/testing/kunit/kunit_tool_test.py | 18 +-
tools/testing/kunit/qemu_config.py | 17 ++
tools/testing/kunit/qemu_configs/alpha.py | 10 +
tools/testing/kunit/qemu_configs/arm.py | 13 ++
tools/testing/kunit/qemu_configs/arm64.py | 12 ++
tools/testing/kunit/qemu_configs/i386.py | 10 +
tools/testing/kunit/qemu_configs/powerpc.py | 12 ++
tools/testing/kunit/qemu_configs/riscv.py | 31 ++++
tools/testing/kunit/qemu_configs/s390.py | 14 ++
tools/testing/kunit/qemu_configs/sparc.py | 10 +
tools/testing/kunit/qemu_configs/x86_64.py | 10 +
18 files changed, 411 insertions(+), 49 deletions(-)
create mode 100644 tools/testing/kunit/qemu_config.py
create mode 100644 tools/testing/kunit/qemu_configs/alpha.py
create mode 100644 tools/testing/kunit/qemu_configs/arm.py
create mode 100644 tools/testing/kunit/qemu_configs/arm64.py
create mode 100644 tools/testing/kunit/qemu_configs/i386.py
create mode 100644 tools/testing/kunit/qemu_configs/powerpc.py
create mode 100644 tools/testing/kunit/qemu_configs/riscv.py
create mode 100644 tools/testing/kunit/qemu_configs/s390.py
create mode 100644 tools/testing/kunit/qemu_configs/sparc.py
create mode 100644 tools/testing/kunit/qemu_configs/x86_64.py
base-commit: 38182162b50aa4e970e5997df0a0c4288147a153
--
2.31.1.607.g51e8a6a459-goog
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 1/4] kunit: Add 'kunit_shutdown' option
2021-05-07 21:31 [PATCH v1 0/4] kunit: tool: add support for QEMU Brendan Higgins
@ 2021-05-07 21:31 ` Brendan Higgins
2021-05-15 7:58 ` David Gow
2021-05-07 21:31 ` [PATCH v1 2/4] Documentation: Add kunit_shutdown to kernel-parameters.txt Brendan Higgins
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Brendan Higgins @ 2021-05-07 21:31 UTC (permalink / raw)
To: shuah, davidgow
Cc: linux-kselftest, kunit-dev, linux-kernel, corbet, linux-doc,
sboyd, keescook, frowand.list, dlatypov, Brendan Higgins
From: David Gow <davidgow@google.com>
Add a new kernel command-line option, 'kunit_shutdown', which allows the
user to specify that the kernel poweroff, halt, or reboot after
completing all KUnit tests; this is very handy for running KUnit tests
on UML or a VM so that the UML/VM process exits cleanly immediately
after running all tests without needing a special initramfs.
Signed-off-by: David Gow <davidgow@google.com>
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Tested-By: Daniel Latypov <dlatypov@google.com>
---
lib/kunit/executor.c | 20 ++++++++++++++++++++
tools/testing/kunit/kunit_kernel.py | 2 +-
tools/testing/kunit/kunit_parser.py | 2 +-
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 15832ed446685..7db619624437c 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
+#include <linux/reboot.h>
#include <kunit/test.h>
#include <linux/glob.h>
#include <linux/moduleparam.h>
@@ -18,6 +19,9 @@ module_param(filter_glob, charp, 0);
MODULE_PARM_DESC(filter_glob,
"Filter which KUnit test suites run at boot-time, e.g. list*");
+static char *kunit_shutdown;
+core_param(kunit_shutdown, kunit_shutdown, charp, 0644);
+
static struct kunit_suite * const *
kunit_filter_subsuite(struct kunit_suite * const * const subsuite)
{
@@ -82,6 +86,20 @@ static struct suite_set kunit_filter_suites(void)
return filtered;
}
+static void kunit_handle_shutdown(void)
+{
+ if (!kunit_shutdown)
+ return;
+
+ if (!strcmp(kunit_shutdown, "poweroff"))
+ kernel_power_off();
+ else if (!strcmp(kunit_shutdown, "halt"))
+ kernel_halt();
+ else if (!strcmp(kunit_shutdown, "reboot"))
+ kernel_restart(NULL);
+
+}
+
static void kunit_print_tap_header(struct suite_set *suite_set)
{
struct kunit_suite * const * const *suites, * const *subsuite;
@@ -112,6 +130,8 @@ int kunit_run_all_tests(void)
kfree(suite_set.start);
}
+ kunit_handle_shutdown();
+
return 0;
}
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 89a7d4024e878..e22ade9d91ad5 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -208,7 +208,7 @@ class LinuxSourceTree(object):
def run_kernel(self, args=None, build_dir='', filter_glob='', timeout=None) -> Iterator[str]:
if not args:
args = []
- args.extend(['mem=1G', 'console=tty'])
+ args.extend(['mem=1G', 'console=tty','kunit_shutdown=halt'])
if filter_glob:
args.append('kunit.filter_glob='+filter_glob)
self._ops.linux_bin(args, timeout, build_dir)
diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index e8bcc139702e2..8d8d4d70b39dd 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -49,7 +49,7 @@ class TestStatus(Enum):
kunit_start_re = re.compile(r'TAP version [0-9]+$')
kunit_end_re = re.compile('(List of all partitions:|'
- 'Kernel panic - not syncing: VFS:)')
+ 'Kernel panic - not syncing: VFS:|reboot: System halted)')
def isolate_kunit_output(kernel_output) -> Iterator[str]:
started = False
--
2.31.1.607.g51e8a6a459-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 2/4] Documentation: Add kunit_shutdown to kernel-parameters.txt
2021-05-07 21:31 [PATCH v1 0/4] kunit: tool: add support for QEMU Brendan Higgins
2021-05-07 21:31 ` [PATCH v1 1/4] kunit: Add 'kunit_shutdown' option Brendan Higgins
@ 2021-05-07 21:31 ` Brendan Higgins
2021-05-15 7:58 ` David Gow
2021-05-07 21:31 ` [PATCH v1 3/4] kunit: tool: add support for QEMU Brendan Higgins
2021-05-07 21:31 ` [PATCH v1 4/4] Documentation: kunit: document support for QEMU in kunit_tool Brendan Higgins
3 siblings, 1 reply; 16+ messages in thread
From: Brendan Higgins @ 2021-05-07 21:31 UTC (permalink / raw)
To: shuah, davidgow
Cc: linux-kselftest, kunit-dev, linux-kernel, corbet, linux-doc,
sboyd, keescook, frowand.list, dlatypov, Brendan Higgins
Add kunit_shutdown, an option to specify that the kernel shutsdown after
running KUnit tests, to the kernel-parameters.txt documentation.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
Documentation/admin-guide/kernel-parameters.txt | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 5bcc229d31e24..bfd64c01698ba 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2244,6 +2244,14 @@
0: force disabled
1: force enabled
+ kunit_shutdown=[KERNEL UNIT TESTING FRAMEWORK] Shutdown kernel after
+ running built-in tests. Tests configured as modules will
+ not be run.
+ Default: (flag not present) don't shutdown
+ poweroff: poweroff the kernel after running tests
+ halt: halt the kernel after running tests
+ reboot: reboot the kernel after running tests
+
kvm.ignore_msrs=[KVM] Ignore guest accesses to unhandled MSRs.
Default is 0 (don't ignore, but inject #GP)
--
2.31.1.607.g51e8a6a459-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 3/4] kunit: tool: add support for QEMU
2021-05-07 21:31 [PATCH v1 0/4] kunit: tool: add support for QEMU Brendan Higgins
2021-05-07 21:31 ` [PATCH v1 1/4] kunit: Add 'kunit_shutdown' option Brendan Higgins
2021-05-07 21:31 ` [PATCH v1 2/4] Documentation: Add kunit_shutdown to kernel-parameters.txt Brendan Higgins
@ 2021-05-07 21:31 ` Brendan Higgins
2021-05-15 7:59 ` David Gow
2021-05-07 21:31 ` [PATCH v1 4/4] Documentation: kunit: document support for QEMU in kunit_tool Brendan Higgins
3 siblings, 1 reply; 16+ messages in thread
From: Brendan Higgins @ 2021-05-07 21:31 UTC (permalink / raw)
To: shuah, davidgow
Cc: linux-kselftest, kunit-dev, linux-kernel, corbet, linux-doc,
sboyd, keescook, frowand.list, dlatypov, Brendan Higgins
Add basic support to run QEMU via kunit_tool. Add support for i386,
x86_64, arm, arm64, and a bunch more.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Tested-by: David Gow <davidgow@google.com>
---
Changes since last revision:
- A number of minor obvious issues pointed out by David and Daniel.
- Added facility for merging Kconfigs at Daniel's suggestion.
- Broke out qemu_configs each into their own config file which is loaded
dynamically - mostly at David's suggestion.
---
tools/testing/kunit/kunit.py | 57 ++++++-
tools/testing/kunit/kunit_config.py | 7 +-
tools/testing/kunit/kunit_kernel.py | 170 ++++++++++++++++----
tools/testing/kunit/kunit_tool_test.py | 18 ++-
tools/testing/kunit/qemu_config.py | 17 ++
tools/testing/kunit/qemu_configs/alpha.py | 10 ++
tools/testing/kunit/qemu_configs/arm.py | 13 ++
tools/testing/kunit/qemu_configs/arm64.py | 12 ++
tools/testing/kunit/qemu_configs/i386.py | 10 ++
tools/testing/kunit/qemu_configs/powerpc.py | 12 ++
tools/testing/kunit/qemu_configs/riscv.py | 31 ++++
tools/testing/kunit/qemu_configs/s390.py | 14 ++
tools/testing/kunit/qemu_configs/sparc.py | 10 ++
tools/testing/kunit/qemu_configs/x86_64.py | 10 ++
14 files changed, 350 insertions(+), 41 deletions(-)
create mode 100644 tools/testing/kunit/qemu_config.py
create mode 100644 tools/testing/kunit/qemu_configs/alpha.py
create mode 100644 tools/testing/kunit/qemu_configs/arm.py
create mode 100644 tools/testing/kunit/qemu_configs/arm64.py
create mode 100644 tools/testing/kunit/qemu_configs/i386.py
create mode 100644 tools/testing/kunit/qemu_configs/powerpc.py
create mode 100644 tools/testing/kunit/qemu_configs/riscv.py
create mode 100644 tools/testing/kunit/qemu_configs/s390.py
create mode 100644 tools/testing/kunit/qemu_configs/sparc.py
create mode 100644 tools/testing/kunit/qemu_configs/x86_64.py
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 5da8fb3762f98..be8d8d4a4e08f 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -70,10 +70,10 @@ def build_tests(linux: kunit_kernel.LinuxSourceTree,
kunit_parser.print_with_timestamp('Building KUnit Kernel ...')
build_start = time.time()
- success = linux.build_um_kernel(request.alltests,
- request.jobs,
- request.build_dir,
- request.make_options)
+ success = linux.build_kernel(request.alltests,
+ request.jobs,
+ request.build_dir,
+ request.make_options)
build_end = time.time()
if not success:
return KunitResult(KunitStatus.BUILD_FAILURE,
@@ -189,6 +189,31 @@ def add_common_opts(parser) -> None:
'will get automatically appended.',
metavar='kunitconfig')
+ parser.add_argument('--arch',
+ help=('Specifies the architecture to run tests under. '
+ 'The architecture specified here must match the '
+ 'string passed to the ARCH make param, '
+ 'e.g. i386, x86_64, arm, um, etc. Non-UML '
+ 'architectures run on QEMU.'),
+ type=str, default='um', metavar='arch')
+
+ parser.add_argument('--cross_compile',
+ help=('Sets make\'s CROSS_COMPILE variable; it should '
+ 'be set to a toolchain path prefix (the prefix '
+ 'of gcc and other tools in your toolchain, for '
+ 'example `sparc64-linux-gnu-` if you have the '
+ 'sparc toolchain installed on your system, or '
+ '`$HOME/toolchains/microblaze/gcc-9.2.0-nolibc/microblaze-linux/bin/microblaze-linux-` '
+ 'if you have downloaded the microblaze toolchain '
+ 'from the 0-day website to a directory in your '
+ 'home directory called `toolchains`).'),
+ metavar='cross_compile')
+
+ parser.add_argument('--qemu_config',
+ help=('Takes a path to a path to a file containing '
+ 'a QemuArchParams object.'),
+ type=str, metavar='qemu_config')
+
def add_build_opts(parser) -> None:
parser.add_argument('--jobs',
help='As in the make command, "Specifies the number of '
@@ -270,7 +295,11 @@ def main(argv, linux=None):
os.mkdir(cli_args.build_dir)
if not linux:
- linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
+ linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
+ kunitconfig_path=cli_args.kunitconfig,
+ arch=cli_args.arch,
+ cross_compile=cli_args.cross_compile,
+ qemu_config_path=cli_args.qemu_config)
request = KunitRequest(cli_args.raw_output,
cli_args.timeout,
@@ -289,7 +318,11 @@ def main(argv, linux=None):
os.mkdir(cli_args.build_dir)
if not linux:
- linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
+ linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
+ kunitconfig_path=cli_args.kunitconfig,
+ arch=cli_args.arch,
+ cross_compile=cli_args.cross_compile,
+ qemu_config_path=cli_args.qemu_config)
request = KunitConfigRequest(cli_args.build_dir,
cli_args.make_options)
@@ -301,7 +334,11 @@ def main(argv, linux=None):
sys.exit(1)
elif cli_args.subcommand == 'build':
if not linux:
- linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
+ linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
+ kunitconfig_path=cli_args.kunitconfig,
+ arch=cli_args.arch,
+ cross_compile=cli_args.cross_compile,
+ qemu_config_path=cli_args.qemu_config)
request = KunitBuildRequest(cli_args.jobs,
cli_args.build_dir,
@@ -315,7 +352,11 @@ def main(argv, linux=None):
sys.exit(1)
elif cli_args.subcommand == 'exec':
if not linux:
- linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
+ linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
+ kunitconfig_path=cli_args.kunitconfig,
+ arch=cli_args.arch,
+ cross_compile=cli_args.cross_compile,
+ qemu_config_path=cli_args.qemu_config)
exec_request = KunitExecRequest(cli_args.timeout,
cli_args.build_dir,
diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py
index 1e2683dcc0e7a..c77c7d2ef6220 100644
--- a/tools/testing/kunit/kunit_config.py
+++ b/tools/testing/kunit/kunit_config.py
@@ -52,8 +52,13 @@ class Kconfig(object):
return False
return True
+ def merge_in_entries(self, other: 'Kconfig') -> None:
+ if other.is_subset_of(self):
+ return
+ self._entries = list(self.entries().union(other.entries()))
+
def write_to_file(self, path: str) -> None:
- with open(path, 'w') as f:
+ with open(path, 'a+') as f:
for entry in self.entries():
f.write(str(entry) + '\n')
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index e22ade9d91ad5..2bd196fd69e5c 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -6,23 +6,31 @@
# Author: Felix Guo <felixguoxiuping@gmail.com>
# Author: Brendan Higgins <brendanhiggins@google.com>
+from __future__ import annotations
+import importlib.util
import logging
import subprocess
import os
import shutil
import signal
from typing import Iterator
+from typing import Optional
from contextlib import ExitStack
+from collections import namedtuple
+
import kunit_config
import kunit_parser
+import qemu_config
KCONFIG_PATH = '.config'
KUNITCONFIG_PATH = '.kunitconfig'
DEFAULT_KUNITCONFIG_PATH = 'arch/um/configs/kunit_defconfig'
BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
OUTFILE_PATH = 'test.log'
+ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
+QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
def get_file_path(build_dir, default):
if build_dir:
@@ -40,6 +48,10 @@ class BuildError(Exception):
class LinuxSourceTreeOperations(object):
"""An abstraction over command line operations performed on a source tree."""
+ def __init__(self, linux_arch: str, cross_compile: Optional[str]):
+ self._linux_arch = linux_arch
+ self._cross_compile = cross_compile
+
def make_mrproper(self) -> None:
try:
subprocess.check_output(['make', 'mrproper'], stderr=subprocess.STDOUT)
@@ -48,12 +60,21 @@ class LinuxSourceTreeOperations(object):
except subprocess.CalledProcessError as e:
raise ConfigError(e.output.decode())
+ def make_arch_qemuconfig(self, kconfig: kunit_config.Kconfig) -> None:
+ pass
+
+ def make_allyesconfig(self, build_dir, make_options) -> None:
+ raise ConfigError('Only the "um" arch is supported for alltests')
+
def make_olddefconfig(self, build_dir, make_options) -> None:
- command = ['make', 'ARCH=um', 'olddefconfig']
+ command = ['make', 'ARCH=' + self._linux_arch, 'olddefconfig']
+ if self._cross_compile:
+ command += ['CROSS_COMPILE=' + self._cross_compile]
if make_options:
command.extend(make_options)
if build_dir:
command += ['O=' + build_dir]
+ print('Populating config with:\n$', ' '.join(command))
try:
subprocess.check_output(command, stderr=subprocess.STDOUT)
except OSError as e:
@@ -61,6 +82,79 @@ class LinuxSourceTreeOperations(object):
except subprocess.CalledProcessError as e:
raise ConfigError(e.output.decode())
+ def make(self, jobs, build_dir, make_options) -> None:
+ command = ['make', 'ARCH=' + self._linux_arch, '--jobs=' + str(jobs)]
+ if make_options:
+ command.extend(make_options)
+ if self._cross_compile:
+ command += ['CROSS_COMPILE=' + self._cross_compile]
+ if build_dir:
+ command += ['O=' + build_dir]
+ print('Building with:\n$', ' '.join(command))
+ try:
+ proc = subprocess.Popen(command,
+ stderr=subprocess.PIPE,
+ stdout=subprocess.DEVNULL)
+ except OSError as e:
+ raise BuildError('Could not call execute make: ' + str(e))
+ except subprocess.CalledProcessError as e:
+ raise BuildError(e.output)
+ _, stderr = proc.communicate()
+ if proc.returncode != 0:
+ raise BuildError(stderr.decode())
+ if stderr: # likely only due to build warnings
+ print(stderr.decode())
+
+ def run(self, params, timeout, build_dir, outfile) -> None:
+ pass
+
+
+class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
+
+ def __init__(self, qemu_arch_params: qemu_config.QemuArchParams, cross_compile: Optional[str]):
+ super().__init__(linux_arch=qemu_arch_params.linux_arch,
+ cross_compile=cross_compile)
+ self._qemuconfig = qemu_arch_params.qemuconfig
+ self._qemu_arch = qemu_arch_params.qemu_arch
+ self._kernel_path = qemu_arch_params.kernel_path
+ self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot'
+ self._extra_qemu_params = qemu_arch_params.extra_qemu_params
+
+ def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None:
+ qemuconfig = kunit_config.Kconfig()
+ qemuconfig.parse_from_string(self._qemuconfig)
+ base_kunitconfig.merge_in_entries(qemuconfig)
+
+ def run(self, params, timeout, build_dir, outfile):
+ kernel_path = os.path.join(build_dir, self._kernel_path)
+ qemu_command = ['qemu-system-' + self._qemu_arch,
+ '-nodefaults',
+ '-m', '1024',
+ '-kernel', kernel_path,
+ '-append', '\'' + ' '.join(params + [self._kernel_command_line]) + '\'',
+ '-no-reboot',
+ '-nographic',
+ '-serial stdio'] + self._extra_qemu_params
+ print('Running tests with:\n$', ' '.join(qemu_command))
+ with open(outfile, 'w') as output:
+ process = subprocess.Popen(' '.join(qemu_command),
+ stdin=subprocess.PIPE,
+ stdout=output,
+ stderr=subprocess.STDOUT,
+ text=True, shell=True)
+ try:
+ process.wait(timeout=timeout)
+ except Exception as e:
+ print(e)
+ process.terminate()
+ return process
+
+class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
+ """An abstraction over command line operations performed on a source tree."""
+
+ def __init__(self, cross_compile=None):
+ super().__init__(linux_arch='um', cross_compile=cross_compile)
+
def make_allyesconfig(self, build_dir, make_options) -> None:
kunit_parser.print_with_timestamp(
'Enabling all CONFIGs for UML...')
@@ -83,32 +177,16 @@ class LinuxSourceTreeOperations(object):
kunit_parser.print_with_timestamp(
'Starting Kernel with all configs takes a few minutes...')
- def make(self, jobs, build_dir, make_options) -> None:
- command = ['make', 'ARCH=um', '--jobs=' + str(jobs)]
- if make_options:
- command.extend(make_options)
- if build_dir:
- command += ['O=' + build_dir]
- try:
- proc = subprocess.Popen(command,
- stderr=subprocess.PIPE,
- stdout=subprocess.DEVNULL)
- except OSError as e:
- raise BuildError('Could not call make command: ' + str(e))
- _, stderr = proc.communicate()
- if proc.returncode != 0:
- raise BuildError(stderr.decode())
- if stderr: # likely only due to build warnings
- print(stderr.decode())
-
- def linux_bin(self, params, timeout, build_dir) -> None:
+ def run(self, params, timeout, build_dir, outfile):
"""Runs the Linux UML binary. Must be named 'linux'."""
linux_bin = get_file_path(build_dir, 'linux')
outfile = get_outfile_path(build_dir)
with open(outfile, 'w') as output:
process = subprocess.Popen([linux_bin] + params,
+ stdin=subprocess.PIPE,
stdout=output,
- stderr=subprocess.STDOUT)
+ stderr=subprocess.STDOUT,
+ text=True)
process.wait(timeout)
def get_kconfig_path(build_dir) -> str:
@@ -120,13 +198,49 @@ def get_kunitconfig_path(build_dir) -> str:
def get_outfile_path(build_dir) -> str:
return get_file_path(build_dir, OUTFILE_PATH)
+def get_source_tree_ops(arch: str, cross_compile: Optional[str]) -> LinuxSourceTreeOperations:
+ config_path = os.path.join(QEMU_CONFIGS_DIR, arch + '.py')
+ if arch == 'um':
+ return LinuxSourceTreeOperationsUml(cross_compile=cross_compile)
+ elif os.path.isfile(config_path):
+ return get_source_tree_ops_from_qemu_config(config_path, cross_compile)[1]
+ else:
+ raise ConfigError(arch + ' is not a valid arch')
+
+def get_source_tree_ops_from_qemu_config(config_path: str,
+ cross_compile: Optional[str]) -> tuple[
+ str, LinuxSourceTreeOperations]:
+ abs_config_path = os.path.abspath(config_path)
+ if os.path.commonpath([ABS_TOOL_PATH, abs_config_path]) != ABS_TOOL_PATH:
+ raise ConfigError('Given QEMU config file is not in a child directory of KUnit tool.')
+ relative_config_path = os.path.relpath(abs_config_path, ABS_TOOL_PATH)
+ spec = importlib.util.spec_from_file_location('.' + relative_config_path, config_path)
+ config = importlib.util.module_from_spec(spec)
+ # TODO(brendanhiggins@google.com): I looked this up and apparently other
+ # Python projects have noted that pytype complains that "No attribute
+ # 'exec_module' on _importlib_modulespec._Loader". Disabling for now.
+ spec.loader.exec_module(config) # pytype: disable=attribute-error
+ return config.QEMU_ARCH.linux_arch, LinuxSourceTreeOperationsQemu(
+ config.QEMU_ARCH, cross_compile=cross_compile)
+
class LinuxSourceTree(object):
"""Represents a Linux kernel source tree with KUnit tests."""
- def __init__(self, build_dir: str, load_config=True, kunitconfig_path='') -> None:
+ def __init__(
+ self,
+ build_dir: str,
+ load_config=True,
+ kunitconfig_path='',
+ arch=None,
+ cross_compile=None,
+ qemu_config_path=None) -> None:
signal.signal(signal.SIGINT, self.signal_handler)
-
- self._ops = LinuxSourceTreeOperations()
+ if qemu_config_path:
+ self._arch, self._ops = get_source_tree_ops_from_qemu_config(
+ qemu_config_path, cross_compile)
+ else:
+ self._arch = 'um' if arch is None else arch
+ self._ops = get_source_tree_ops(self._arch, cross_compile)
if not load_config:
return
@@ -170,8 +284,9 @@ class LinuxSourceTree(object):
kconfig_path = get_kconfig_path(build_dir)
if build_dir and not os.path.exists(build_dir):
os.mkdir(build_dir)
- self._kconfig.write_to_file(kconfig_path)
try:
+ self._ops.make_arch_qemuconfig(self._kconfig)
+ self._kconfig.write_to_file(kconfig_path)
self._ops.make_olddefconfig(build_dir, make_options)
except ConfigError as e:
logging.error(e)
@@ -184,6 +299,7 @@ class LinuxSourceTree(object):
if os.path.exists(kconfig_path):
existing_kconfig = kunit_config.Kconfig()
existing_kconfig.read_from_file(kconfig_path)
+ self._ops.make_arch_qemuconfig(self._kconfig)
if not self._kconfig.is_subset_of(existing_kconfig):
print('Regenerating .config ...')
os.remove(kconfig_path)
@@ -194,7 +310,7 @@ class LinuxSourceTree(object):
print('Generating .config ...')
return self.build_config(build_dir, make_options)
- def build_um_kernel(self, alltests, jobs, build_dir, make_options) -> bool:
+ def build_kernel(self, alltests, jobs, build_dir, make_options) -> bool:
try:
if alltests:
self._ops.make_allyesconfig(build_dir, make_options)
@@ -211,8 +327,8 @@ class LinuxSourceTree(object):
args.extend(['mem=1G', 'console=tty','kunit_shutdown=halt'])
if filter_glob:
args.append('kunit.filter_glob='+filter_glob)
- self._ops.linux_bin(args, timeout, build_dir)
outfile = get_outfile_path(build_dir)
+ self._ops.run(args, timeout, build_dir, outfile)
subprocess.call(['stty', 'sane'])
with open(outfile, 'r') as file:
for line in file:
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 2e809dd956a77..a3b7f68e1cf9f 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -303,7 +303,7 @@ class KUnitMainTest(unittest.TestCase):
self.linux_source_mock = mock.Mock()
self.linux_source_mock.build_reconfig = mock.Mock(return_value=True)
- self.linux_source_mock.build_um_kernel = mock.Mock(return_value=True)
+ self.linux_source_mock.build_kernel = mock.Mock(return_value=True)
self.linux_source_mock.run_kernel = mock.Mock(return_value=all_passed_log)
def test_config_passes_args_pass(self):
@@ -314,7 +314,7 @@ class KUnitMainTest(unittest.TestCase):
def test_build_passes_args_pass(self):
kunit.main(['build'], self.linux_source_mock)
self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 0)
- self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, '.kunit', None)
+ self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, '.kunit', None)
self.assertEqual(self.linux_source_mock.run_kernel.call_count, 0)
def test_exec_passes_args_pass(self):
@@ -396,7 +396,7 @@ class KUnitMainTest(unittest.TestCase):
def test_build_builddir(self):
build_dir = '.kunit'
kunit.main(['build', '--build_dir', build_dir], self.linux_source_mock)
- self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, build_dir, None)
+ self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, build_dir, None)
def test_exec_builddir(self):
build_dir = '.kunit'
@@ -410,14 +410,22 @@ class KUnitMainTest(unittest.TestCase):
mock_linux_init.return_value = self.linux_source_mock
kunit.main(['run', '--kunitconfig=mykunitconfig'])
# Just verify that we parsed and initialized it correctly here.
- mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig')
+ mock_linux_init.assert_called_once_with('.kunit',
+ kunitconfig_path='mykunitconfig',
+ arch='um',
+ cross_compile=None,
+ qemu_config_path=None)
@mock.patch.object(kunit_kernel, 'LinuxSourceTree')
def test_config_kunitconfig(self, mock_linux_init):
mock_linux_init.return_value = self.linux_source_mock
kunit.main(['config', '--kunitconfig=mykunitconfig'])
# Just verify that we parsed and initialized it correctly here.
- mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig')
+ mock_linux_init.assert_called_once_with('.kunit',
+ kunitconfig_path='mykunitconfig',
+ arch='um',
+ cross_compile=None,
+ qemu_config_path=None)
if __name__ == '__main__':
unittest.main()
diff --git a/tools/testing/kunit/qemu_config.py b/tools/testing/kunit/qemu_config.py
new file mode 100644
index 0000000000000..aff1fe0442dbc
--- /dev/null
+++ b/tools/testing/kunit/qemu_config.py
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Collection of configs for building non-UML kernels and running them on QEMU.
+#
+# Copyright (C) 2021, Google LLC.
+# Author: Brendan Higgins <brendanhiggins@google.com>
+
+from collections import namedtuple
+
+
+QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',
+ 'qemuconfig',
+ 'qemu_arch',
+ 'kernel_path',
+ 'kernel_command_line',
+ 'extra_qemu_params'])
+
diff --git a/tools/testing/kunit/qemu_configs/alpha.py b/tools/testing/kunit/qemu_configs/alpha.py
new file mode 100644
index 0000000000000..2cc64f848ca2c
--- /dev/null
+++ b/tools/testing/kunit/qemu_configs/alpha.py
@@ -0,0 +1,10 @@
+from ..qemu_config import QemuArchParams
+
+QEMU_ARCH = QemuArchParams(linux_arch='alpha',
+ qemuconfig='''
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y''',
+ qemu_arch='alpha',
+ kernel_path='arch/alpha/boot/vmlinux',
+ kernel_command_line='console=ttyS0',
+ extra_qemu_params=[''])
diff --git a/tools/testing/kunit/qemu_configs/arm.py b/tools/testing/kunit/qemu_configs/arm.py
new file mode 100644
index 0000000000000..29a043b0531a0
--- /dev/null
+++ b/tools/testing/kunit/qemu_configs/arm.py
@@ -0,0 +1,13 @@
+from ..qemu_config import QemuArchParams
+
+QEMU_ARCH = QemuArchParams(linux_arch='arm',
+ qemuconfig='''
+CONFIG_ARCH_VIRT=y
+CONFIG_SERIAL_AMBA_PL010=y
+CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
+CONFIG_SERIAL_AMBA_PL011=y
+CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
+ qemu_arch='arm',
+ kernel_path='arch/arm/boot/zImage',
+ kernel_command_line='console=ttyAMA0',
+ extra_qemu_params=['-machine virt'])
diff --git a/tools/testing/kunit/qemu_configs/arm64.py b/tools/testing/kunit/qemu_configs/arm64.py
new file mode 100644
index 0000000000000..1ba200bc99f0f
--- /dev/null
+++ b/tools/testing/kunit/qemu_configs/arm64.py
@@ -0,0 +1,12 @@
+from ..qemu_config import QemuArchParams
+
+QEMU_ARCH = QemuArchParams(linux_arch='arm64',
+ qemuconfig='''
+CONFIG_SERIAL_AMBA_PL010=y
+CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
+CONFIG_SERIAL_AMBA_PL011=y
+CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
+ qemu_arch='aarch64',
+ kernel_path='arch/arm64/boot/Image.gz',
+ kernel_command_line='console=ttyAMA0',
+ extra_qemu_params=['-machine virt', '-cpu cortex-a57'])
diff --git a/tools/testing/kunit/qemu_configs/i386.py b/tools/testing/kunit/qemu_configs/i386.py
new file mode 100644
index 0000000000000..3998af306468e
--- /dev/null
+++ b/tools/testing/kunit/qemu_configs/i386.py
@@ -0,0 +1,10 @@
+from ..qemu_config import QemuArchParams
+
+QEMU_ARCH = QemuArchParams(linux_arch='i386',
+ qemuconfig='''
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y''',
+ qemu_arch='x86_64',
+ kernel_path='arch/x86/boot/bzImage',
+ kernel_command_line='console=ttyS0',
+ extra_qemu_params=[''])
diff --git a/tools/testing/kunit/qemu_configs/powerpc.py b/tools/testing/kunit/qemu_configs/powerpc.py
new file mode 100644
index 0000000000000..46292ce9e368e
--- /dev/null
+++ b/tools/testing/kunit/qemu_configs/powerpc.py
@@ -0,0 +1,12 @@
+from ..qemu_config import QemuArchParams
+
+QEMU_ARCH = QemuArchParams(linux_arch='powerpc',
+ qemuconfig='''
+CONFIG_PPC64=y
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_HVC_CONSOLE=y''',
+ qemu_arch='ppc64',
+ kernel_path='vmlinux',
+ kernel_command_line='console=ttyS0',
+ extra_qemu_params=['-M pseries', '-cpu power8'])
diff --git a/tools/testing/kunit/qemu_configs/riscv.py b/tools/testing/kunit/qemu_configs/riscv.py
new file mode 100644
index 0000000000000..de8c62d465723
--- /dev/null
+++ b/tools/testing/kunit/qemu_configs/riscv.py
@@ -0,0 +1,31 @@
+from ..qemu_config import QemuArchParams
+import os
+import os.path
+import sys
+
+GITHUB_OPENSBI_URL = 'https://github.com/qemu/qemu/raw/master/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin'
+OPENSBI_FILE = os.path.basename(GITHUB_OPENSBI_URL)
+
+if not os.path.isfile(OPENSBI_FILE):
+ print('\n\nOpenSBI file is not in the current working directory.\n'
+ 'Would you like me to download it for you from:\n' + GITHUB_OPENSBI_URL + ' ?\n')
+ response = input('yes/[no]: ')
+ if response.strip() == 'yes':
+ os.system('wget ' + GITHUB_OPENSBI_URL)
+ else:
+ sys.exit()
+
+QEMU_ARCH = QemuArchParams(linux_arch='riscv',
+ qemuconfig='''
+CONFIG_SOC_VIRT=y
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_SERIAL_EARLYCON_RISCV_SBI=y''',
+ qemu_arch='riscv64',
+ kernel_path='arch/riscv/boot/Image',
+ kernel_command_line='console=ttyS0',
+ extra_qemu_params=[
+ '-machine virt',
+ '-cpu rv64',
+ '-bios opensbi-riscv64-generic-fw_dynamic.bin'])
diff --git a/tools/testing/kunit/qemu_configs/s390.py b/tools/testing/kunit/qemu_configs/s390.py
new file mode 100644
index 0000000000000..04c90332f1098
--- /dev/null
+++ b/tools/testing/kunit/qemu_configs/s390.py
@@ -0,0 +1,14 @@
+from ..qemu_config import QemuArchParams
+
+QEMU_ARCH = QemuArchParams(linux_arch='s390',
+ qemuconfig='''
+CONFIG_EXPERT=y
+CONFIG_TUNE_ZEC12=y
+CONFIG_NUMA=y
+CONFIG_MODULES=y''',
+ qemu_arch='s390x',
+ kernel_path='arch/s390/boot/bzImage',
+ kernel_command_line='console=ttyS0',
+ extra_qemu_params=[
+ '-machine s390-ccw-virtio',
+ '-cpu qemu',])
diff --git a/tools/testing/kunit/qemu_configs/sparc.py b/tools/testing/kunit/qemu_configs/sparc.py
new file mode 100644
index 0000000000000..f26b5f27cc5a1
--- /dev/null
+++ b/tools/testing/kunit/qemu_configs/sparc.py
@@ -0,0 +1,10 @@
+from ..qemu_config import QemuArchParams
+
+QEMU_ARCH = QemuArchParams(linux_arch='sparc',
+ qemuconfig='''
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y''',
+ qemu_arch='sparc',
+ kernel_path='arch/sparc/boot/zImage',
+ kernel_command_line='console=ttyS0 mem=256M',
+ extra_qemu_params=['-m 256'])
diff --git a/tools/testing/kunit/qemu_configs/x86_64.py b/tools/testing/kunit/qemu_configs/x86_64.py
new file mode 100644
index 0000000000000..bd5ab733b92ac
--- /dev/null
+++ b/tools/testing/kunit/qemu_configs/x86_64.py
@@ -0,0 +1,10 @@
+from ..qemu_config import QemuArchParams
+
+QEMU_ARCH = QemuArchParams(linux_arch='x86_64',
+ qemuconfig='''
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y''',
+ qemu_arch='x86_64',
+ kernel_path='arch/x86/boot/bzImage',
+ kernel_command_line='console=ttyS0',
+ extra_qemu_params=[''])
--
2.31.1.607.g51e8a6a459-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 4/4] Documentation: kunit: document support for QEMU in kunit_tool
2021-05-07 21:31 [PATCH v1 0/4] kunit: tool: add support for QEMU Brendan Higgins
` (2 preceding siblings ...)
2021-05-07 21:31 ` [PATCH v1 3/4] kunit: tool: add support for QEMU Brendan Higgins
@ 2021-05-07 21:31 ` Brendan Higgins
2021-05-15 8:01 ` David Gow
3 siblings, 1 reply; 16+ messages in thread
From: Brendan Higgins @ 2021-05-07 21:31 UTC (permalink / raw)
To: shuah, davidgow
Cc: linux-kselftest, kunit-dev, linux-kernel, corbet, linux-doc,
sboyd, keescook, frowand.list, dlatypov, Brendan Higgins
Document QEMU support, what it does, and how to use it in kunit_tool.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Documentation/dev-tools/kunit/usage.rst | 37 +++++++++++++++++++++----
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index 650f99590df57..b74bd7c87cc20 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -612,14 +612,39 @@ only things to be aware of when doing so.
The biggest impediment will likely be that certain KUnit features and
infrastructure may not support your target environment. For example, at this
time the KUnit Wrapper (``tools/testing/kunit/kunit.py``) does not work outside
-of UML. Unfortunately, there is no way around this. Using UML (or even just a
-particular architecture) allows us to make a lot of assumptions that make it
-possible to do things which might otherwise be impossible.
+of UML and QEMU. Unfortunately, there is no way around this. Using UML and QEMU
+(or even just a particular architecture) allows us to make a lot of assumptions
+that make it possible to do things which might otherwise be impossible.
Nevertheless, all core KUnit framework features are fully supported on all
-architectures, and using them is straightforward: all you need to do is to take
-your kunitconfig, your Kconfig options for the tests you would like to run, and
-merge them into whatever config your are using for your platform. That's it!
+architectures, and using them is straightforward: Most popular architectures
+are supported directly in the KUnit Wrapper via QEMU. Currently, supported
+architectures on QEMU include:
+
+* i386
+* x86_64
+* arm
+* arm64
+* alpha
+* powerpc
+* riscv
+* s390
+* sparc
+
+In order to run KUnit tests on one of these architectures via QEMU with the
+KUnit wrapper, all you need to do is specify the flags ``--arch`` and
+``--cross_compile`` when invoking the KUnit Wrapper. For example, we could run
+the default KUnit tests on ARM in the following manner (assuming we have an ARM
+toolchain installed):
+
+.. code-block:: bash
+
+ tools/testing/kunit/kunit.py run --timeout=60 --jobs=12 --arch=arm --cross_compile=arm-linux-gnueabihf-
+
+Alternatively, if you want to run your tests on real hardware or in some other
+emulation environment, all you need to do is to take your kunitconfig, your
+Kconfig options for the tests you would like to run, and merge them into
+whatever config your are using for your platform. That's it!
For example, let's say you have the following kunitconfig:
--
2.31.1.607.g51e8a6a459-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 1/4] kunit: Add 'kunit_shutdown' option
2021-05-07 21:31 ` [PATCH v1 1/4] kunit: Add 'kunit_shutdown' option Brendan Higgins
@ 2021-05-15 7:58 ` David Gow
2021-05-18 20:52 ` Brendan Higgins
0 siblings, 1 reply; 16+ messages in thread
From: David Gow @ 2021-05-15 7:58 UTC (permalink / raw)
To: Brendan Higgins
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
[-- Attachment #1: Type: text/plain, Size: 4001 bytes --]
On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> From: David Gow <davidgow@google.com>
>
> Add a new kernel command-line option, 'kunit_shutdown', which allows the
> user to specify that the kernel poweroff, halt, or reboot after
> completing all KUnit tests; this is very handy for running KUnit tests
> on UML or a VM so that the UML/VM process exits cleanly immediately
> after running all tests without needing a special initramfs.
>
> Signed-off-by: David Gow <davidgow@google.com>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> Tested-By: Daniel Latypov <dlatypov@google.com>
> ---
Obviously I'm okay with this change, but I did find a minor whitespace
nitpick below.
> lib/kunit/executor.c | 20 ++++++++++++++++++++
> tools/testing/kunit/kunit_kernel.py | 2 +-
> tools/testing/kunit/kunit_parser.py | 2 +-
> 3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 15832ed446685..7db619624437c 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +#include <linux/reboot.h>
> #include <kunit/test.h>
> #include <linux/glob.h>
> #include <linux/moduleparam.h>
> @@ -18,6 +19,9 @@ module_param(filter_glob, charp, 0);
> MODULE_PARM_DESC(filter_glob,
> "Filter which KUnit test suites run at boot-time, e.g. list*");
>
> +static char *kunit_shutdown;
> +core_param(kunit_shutdown, kunit_shutdown, charp, 0644);
> +
> static struct kunit_suite * const *
> kunit_filter_subsuite(struct kunit_suite * const * const subsuite)
> {
> @@ -82,6 +86,20 @@ static struct suite_set kunit_filter_suites(void)
> return filtered;
> }
>
> +static void kunit_handle_shutdown(void)
> +{
> + if (!kunit_shutdown)
> + return;
> +
> + if (!strcmp(kunit_shutdown, "poweroff"))
> + kernel_power_off();
> + else if (!strcmp(kunit_shutdown, "halt"))
> + kernel_halt();
> + else if (!strcmp(kunit_shutdown, "reboot"))
> + kernel_restart(NULL);
> +
> +}
> +
> static void kunit_print_tap_header(struct suite_set *suite_set)
> {
> struct kunit_suite * const * const *suites, * const *subsuite;
> @@ -112,6 +130,8 @@ int kunit_run_all_tests(void)
> kfree(suite_set.start);
> }
>
> + kunit_handle_shutdown();
> +
> return 0;
> }
>
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index 89a7d4024e878..e22ade9d91ad5 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -208,7 +208,7 @@ class LinuxSourceTree(object):
> def run_kernel(self, args=None, build_dir='', filter_glob='', timeout=None) -> Iterator[str]:
> if not args:
> args = []
> - args.extend(['mem=1G', 'console=tty'])
> + args.extend(['mem=1G', 'console=tty','kunit_shutdown=halt'])
Nit: space here between 'console=tty', and 'kunit_shutdown=halt'.
> if filter_glob:
> args.append('kunit.filter_glob='+filter_glob)
> self._ops.linux_bin(args, timeout, build_dir)
> diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
> index e8bcc139702e2..8d8d4d70b39dd 100644
> --- a/tools/testing/kunit/kunit_parser.py
> +++ b/tools/testing/kunit/kunit_parser.py
> @@ -49,7 +49,7 @@ class TestStatus(Enum):
>
> kunit_start_re = re.compile(r'TAP version [0-9]+$')
> kunit_end_re = re.compile('(List of all partitions:|'
> - 'Kernel panic - not syncing: VFS:)')
> + 'Kernel panic - not syncing: VFS:|reboot: System halted)')
>
> def isolate_kunit_output(kernel_output) -> Iterator[str]:
> started = False
> --
> 2.31.1.607.g51e8a6a459-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4000 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/4] Documentation: Add kunit_shutdown to kernel-parameters.txt
2021-05-07 21:31 ` [PATCH v1 2/4] Documentation: Add kunit_shutdown to kernel-parameters.txt Brendan Higgins
@ 2021-05-15 7:58 ` David Gow
0 siblings, 0 replies; 16+ messages in thread
From: David Gow @ 2021-05-15 7:58 UTC (permalink / raw)
To: Brendan Higgins
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> Add kunit_shutdown, an option to specify that the kernel shutsdown after
> running KUnit tests, to the kernel-parameters.txt documentation.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> ---
Looks good to me.
Reviewed-by: David Gow <davidgow@google.com>
> Documentation/admin-guide/kernel-parameters.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 5bcc229d31e24..bfd64c01698ba 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2244,6 +2244,14 @@
> 0: force disabled
> 1: force enabled
>
> + kunit_shutdown=[KERNEL UNIT TESTING FRAMEWORK] Shutdown kernel after
> + running built-in tests. Tests configured as modules will
> + not be run.
> + Default: (flag not present) don't shutdown
> + poweroff: poweroff the kernel after running tests
> + halt: halt the kernel after running tests
> + reboot: reboot the kernel after running tests
> +
> kvm.ignore_msrs=[KVM] Ignore guest accesses to unhandled MSRs.
> Default is 0 (don't ignore, but inject #GP)
>
> --
> 2.31.1.607.g51e8a6a459-goog
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/4] kunit: tool: add support for QEMU
2021-05-07 21:31 ` [PATCH v1 3/4] kunit: tool: add support for QEMU Brendan Higgins
@ 2021-05-15 7:59 ` David Gow
2021-05-18 3:01 ` David Gow
2021-05-18 20:27 ` Brendan Higgins
0 siblings, 2 replies; 16+ messages in thread
From: David Gow @ 2021-05-15 7:59 UTC (permalink / raw)
To: Brendan Higgins
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
[-- Attachment #1: Type: text/plain, Size: 36871 bytes --]
On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> Add basic support to run QEMU via kunit_tool. Add support for i386,
> x86_64, arm, arm64, and a bunch more.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Tested-by: David Gow <davidgow@google.com>
> ---
>
> Changes since last revision:
>
> - A number of minor obvious issues pointed out by David and Daniel.
> - Added facility for merging Kconfigs at Daniel's suggestion.
> - Broke out qemu_configs each into their own config file which is loaded
> dynamically - mostly at David's suggestion.
>
> ---
This seems pretty good to me. I only have one real complaint --
qemu_configs needing to be in a subdirectory of ./tools/testing/kunit
-- but am able to tolerate that (even if I'd prefer not to have it) if
it's documented properly.
Otherwise, save for a couple of minor nitpicks, this seems good to go.
Reviewed-by: David Gow <davidgow@google.com>
> tools/testing/kunit/kunit.py | 57 ++++++-
> tools/testing/kunit/kunit_config.py | 7 +-
> tools/testing/kunit/kunit_kernel.py | 170 ++++++++++++++++----
> tools/testing/kunit/kunit_tool_test.py | 18 ++-
> tools/testing/kunit/qemu_config.py | 17 ++
> tools/testing/kunit/qemu_configs/alpha.py | 10 ++
> tools/testing/kunit/qemu_configs/arm.py | 13 ++
> tools/testing/kunit/qemu_configs/arm64.py | 12 ++
> tools/testing/kunit/qemu_configs/i386.py | 10 ++
> tools/testing/kunit/qemu_configs/powerpc.py | 12 ++
> tools/testing/kunit/qemu_configs/riscv.py | 31 ++++
> tools/testing/kunit/qemu_configs/s390.py | 14 ++
> tools/testing/kunit/qemu_configs/sparc.py | 10 ++
> tools/testing/kunit/qemu_configs/x86_64.py | 10 ++
> 14 files changed, 350 insertions(+), 41 deletions(-)
> create mode 100644 tools/testing/kunit/qemu_config.py
> create mode 100644 tools/testing/kunit/qemu_configs/alpha.py
> create mode 100644 tools/testing/kunit/qemu_configs/arm.py
> create mode 100644 tools/testing/kunit/qemu_configs/arm64.py
> create mode 100644 tools/testing/kunit/qemu_configs/i386.py
> create mode 100644 tools/testing/kunit/qemu_configs/powerpc.py
> create mode 100644 tools/testing/kunit/qemu_configs/riscv.py
> create mode 100644 tools/testing/kunit/qemu_configs/s390.py
> create mode 100644 tools/testing/kunit/qemu_configs/sparc.py
> create mode 100644 tools/testing/kunit/qemu_configs/x86_64.py
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index 5da8fb3762f98..be8d8d4a4e08f 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -70,10 +70,10 @@ def build_tests(linux: kunit_kernel.LinuxSourceTree,
> kunit_parser.print_with_timestamp('Building KUnit Kernel ...')
>
> build_start = time.time()
> - success = linux.build_um_kernel(request.alltests,
> - request.jobs,
> - request.build_dir,
> - request.make_options)
> + success = linux.build_kernel(request.alltests,
> + request.jobs,
> + request.build_dir,
> + request.make_options)
> build_end = time.time()
> if not success:
> return KunitResult(KunitStatus.BUILD_FAILURE,
> @@ -189,6 +189,31 @@ def add_common_opts(parser) -> None:
> 'will get automatically appended.',
> metavar='kunitconfig')
>
> + parser.add_argument('--arch',
> + help=('Specifies the architecture to run tests under. '
> + 'The architecture specified here must match the '
> + 'string passed to the ARCH make param, '
> + 'e.g. i386, x86_64, arm, um, etc. Non-UML '
> + 'architectures run on QEMU.'),
> + type=str, default='um', metavar='arch')
> +
> + parser.add_argument('--cross_compile',
> + help=('Sets make\'s CROSS_COMPILE variable; it should '
> + 'be set to a toolchain path prefix (the prefix '
> + 'of gcc and other tools in your toolchain, for '
> + 'example `sparc64-linux-gnu-` if you have the '
> + 'sparc toolchain installed on your system, or '
> + '`$HOME/toolchains/microblaze/gcc-9.2.0-nolibc/microblaze-linux/bin/microblaze-linux-` '
Super-minor nit: is there a shorter example we can give that's not
much longer than all the surrounding lines?
> + 'if you have downloaded the microblaze toolchain '
> + 'from the 0-day website to a directory in your '
> + 'home directory called `toolchains`).'),
> + metavar='cross_compile')
> +
> + parser.add_argument('--qemu_config',
> + help=('Takes a path to a path to a file containing '
> + 'a QemuArchParams object.'),
> + type=str, metavar='qemu_config')
> +
If (as noted below) this file needs to be in a subdirectory of the
kunit_tool directory, we should document this, or maybe make this path
relative to the kunit_tool directory.
> def add_build_opts(parser) -> None:
> parser.add_argument('--jobs',
> help='As in the make command, "Specifies the number of '
> @@ -270,7 +295,11 @@ def main(argv, linux=None):
> os.mkdir(cli_args.build_dir)
>
> if not linux:
> - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
> + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> + kunitconfig_path=cli_args.kunitconfig,
> + arch=cli_args.arch,
> + cross_compile=cli_args.cross_compile,
> + qemu_config_path=cli_args.qemu_config)
>
> request = KunitRequest(cli_args.raw_output,
> cli_args.timeout,
> @@ -289,7 +318,11 @@ def main(argv, linux=None):
> os.mkdir(cli_args.build_dir)
>
> if not linux:
> - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
> + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> + kunitconfig_path=cli_args.kunitconfig,
> + arch=cli_args.arch,
> + cross_compile=cli_args.cross_compile,
> + qemu_config_path=cli_args.qemu_config)
>
> request = KunitConfigRequest(cli_args.build_dir,
> cli_args.make_options)
> @@ -301,7 +334,11 @@ def main(argv, linux=None):
> sys.exit(1)
> elif cli_args.subcommand == 'build':
> if not linux:
> - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
> + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> + kunitconfig_path=cli_args.kunitconfig,
> + arch=cli_args.arch,
> + cross_compile=cli_args.cross_compile,
> + qemu_config_path=cli_args.qemu_config)
>
> request = KunitBuildRequest(cli_args.jobs,
> cli_args.build_dir,
> @@ -315,7 +352,11 @@ def main(argv, linux=None):
> sys.exit(1)
> elif cli_args.subcommand == 'exec':
> if not linux:
> - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
> + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> + kunitconfig_path=cli_args.kunitconfig,
> + arch=cli_args.arch,
> + cross_compile=cli_args.cross_compile,
> + qemu_config_path=cli_args.qemu_config)
>
> exec_request = KunitExecRequest(cli_args.timeout,
> cli_args.build_dir,
> diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py
> index 1e2683dcc0e7a..c77c7d2ef6220 100644
> --- a/tools/testing/kunit/kunit_config.py
> +++ b/tools/testing/kunit/kunit_config.py
> @@ -52,8 +52,13 @@ class Kconfig(object):
> return False
> return True
>
> + def merge_in_entries(self, other: 'Kconfig') -> None:
> + if other.is_subset_of(self):
> + return
> + self._entries = list(self.entries().union(other.entries()))
> +
> def write_to_file(self, path: str) -> None:
> - with open(path, 'w') as f:
> + with open(path, 'a+') as f:
> for entry in self.entries():
> f.write(str(entry) + '\n')
>
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index e22ade9d91ad5..2bd196fd69e5c 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -6,23 +6,31 @@
> # Author: Felix Guo <felixguoxiuping@gmail.com>
> # Author: Brendan Higgins <brendanhiggins@google.com>
>
> +from __future__ import annotations
> +import importlib.util
> import logging
> import subprocess
> import os
> import shutil
> import signal
> from typing import Iterator
> +from typing import Optional
>
> from contextlib import ExitStack
>
> +from collections import namedtuple
> +
> import kunit_config
> import kunit_parser
> +import qemu_config
>
> KCONFIG_PATH = '.config'
> KUNITCONFIG_PATH = '.kunitconfig'
> DEFAULT_KUNITCONFIG_PATH = 'arch/um/configs/kunit_defconfig'
> BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
> OUTFILE_PATH = 'test.log'
> +ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
> +QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
>
> def get_file_path(build_dir, default):
> if build_dir:
> @@ -40,6 +48,10 @@ class BuildError(Exception):
> class LinuxSourceTreeOperations(object):
> """An abstraction over command line operations performed on a source tree."""
>
> + def __init__(self, linux_arch: str, cross_compile: Optional[str]):
> + self._linux_arch = linux_arch
> + self._cross_compile = cross_compile
> +
> def make_mrproper(self) -> None:
> try:
> subprocess.check_output(['make', 'mrproper'], stderr=subprocess.STDOUT)
> @@ -48,12 +60,21 @@ class LinuxSourceTreeOperations(object):
> except subprocess.CalledProcessError as e:
> raise ConfigError(e.output.decode())
>
> + def make_arch_qemuconfig(self, kconfig: kunit_config.Kconfig) -> None:
> + pass
> +
> + def make_allyesconfig(self, build_dir, make_options) -> None:
> + raise ConfigError('Only the "um" arch is supported for alltests')
> +
> def make_olddefconfig(self, build_dir, make_options) -> None:
> - command = ['make', 'ARCH=um', 'olddefconfig']
> + command = ['make', 'ARCH=' + self._linux_arch, 'olddefconfig']
> + if self._cross_compile:
> + command += ['CROSS_COMPILE=' + self._cross_compile]
> if make_options:
> command.extend(make_options)
> if build_dir:
> command += ['O=' + build_dir]
> + print('Populating config with:\n$', ' '.join(command))
> try:
> subprocess.check_output(command, stderr=subprocess.STDOUT)
> except OSError as e:
> @@ -61,6 +82,79 @@ class LinuxSourceTreeOperations(object):
> except subprocess.CalledProcessError as e:
> raise ConfigError(e.output.decode())
>
> + def make(self, jobs, build_dir, make_options) -> None:
> + command = ['make', 'ARCH=' + self._linux_arch, '--jobs=' + str(jobs)]
> + if make_options:
> + command.extend(make_options)
> + if self._cross_compile:
> + command += ['CROSS_COMPILE=' + self._cross_compile]
> + if build_dir:
> + command += ['O=' + build_dir]
> + print('Building with:\n$', ' '.join(command))
> + try:
> + proc = subprocess.Popen(command,
> + stderr=subprocess.PIPE,
> + stdout=subprocess.DEVNULL)
> + except OSError as e:
> + raise BuildError('Could not call execute make: ' + str(e))
> + except subprocess.CalledProcessError as e:
> + raise BuildError(e.output)
> + _, stderr = proc.communicate()
> + if proc.returncode != 0:
> + raise BuildError(stderr.decode())
> + if stderr: # likely only due to build warnings
> + print(stderr.decode())
> +
> + def run(self, params, timeout, build_dir, outfile) -> None:
> + pass
> +
> +
> +class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
> +
> + def __init__(self, qemu_arch_params: qemu_config.QemuArchParams, cross_compile: Optional[str]):
> + super().__init__(linux_arch=qemu_arch_params.linux_arch,
> + cross_compile=cross_compile)
> + self._qemuconfig = qemu_arch_params.qemuconfig
> + self._qemu_arch = qemu_arch_params.qemu_arch
> + self._kernel_path = qemu_arch_params.kernel_path
> + self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot'
> + self._extra_qemu_params = qemu_arch_params.extra_qemu_params
> +
> + def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None:
> + qemuconfig = kunit_config.Kconfig()
> + qemuconfig.parse_from_string(self._qemuconfig)
> + base_kunitconfig.merge_in_entries(qemuconfig)
> +
> + def run(self, params, timeout, build_dir, outfile):
> + kernel_path = os.path.join(build_dir, self._kernel_path)
> + qemu_command = ['qemu-system-' + self._qemu_arch,
> + '-nodefaults',
> + '-m', '1024',
> + '-kernel', kernel_path,
> + '-append', '\'' + ' '.join(params + [self._kernel_command_line]) + '\'',
> + '-no-reboot',
> + '-nographic',
> + '-serial stdio'] + self._extra_qemu_params
> + print('Running tests with:\n$', ' '.join(qemu_command))
> + with open(outfile, 'w') as output:
> + process = subprocess.Popen(' '.join(qemu_command),
> + stdin=subprocess.PIPE,
> + stdout=output,
> + stderr=subprocess.STDOUT,
> + text=True, shell=True)
> + try:
> + process.wait(timeout=timeout)
> + except Exception as e:
> + print(e)
> + process.terminate()
> + return process
> +
> +class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
> + """An abstraction over command line operations performed on a source tree."""
> +
> + def __init__(self, cross_compile=None):
> + super().__init__(linux_arch='um', cross_compile=cross_compile)
> +
> def make_allyesconfig(self, build_dir, make_options) -> None:
> kunit_parser.print_with_timestamp(
> 'Enabling all CONFIGs for UML...')
> @@ -83,32 +177,16 @@ class LinuxSourceTreeOperations(object):
> kunit_parser.print_with_timestamp(
> 'Starting Kernel with all configs takes a few minutes...')
>
> - def make(self, jobs, build_dir, make_options) -> None:
> - command = ['make', 'ARCH=um', '--jobs=' + str(jobs)]
> - if make_options:
> - command.extend(make_options)
> - if build_dir:
> - command += ['O=' + build_dir]
> - try:
> - proc = subprocess.Popen(command,
> - stderr=subprocess.PIPE,
> - stdout=subprocess.DEVNULL)
> - except OSError as e:
> - raise BuildError('Could not call make command: ' + str(e))
> - _, stderr = proc.communicate()
> - if proc.returncode != 0:
> - raise BuildError(stderr.decode())
> - if stderr: # likely only due to build warnings
> - print(stderr.decode())
> -
> - def linux_bin(self, params, timeout, build_dir) -> None:
> + def run(self, params, timeout, build_dir, outfile):
> """Runs the Linux UML binary. Must be named 'linux'."""
> linux_bin = get_file_path(build_dir, 'linux')
> outfile = get_outfile_path(build_dir)
> with open(outfile, 'w') as output:
> process = subprocess.Popen([linux_bin] + params,
> + stdin=subprocess.PIPE,
> stdout=output,
> - stderr=subprocess.STDOUT)
> + stderr=subprocess.STDOUT,
> + text=True)
> process.wait(timeout)
>
> def get_kconfig_path(build_dir) -> str:
> @@ -120,13 +198,49 @@ def get_kunitconfig_path(build_dir) -> str:
> def get_outfile_path(build_dir) -> str:
> return get_file_path(build_dir, OUTFILE_PATH)
>
> +def get_source_tree_ops(arch: str, cross_compile: Optional[str]) -> LinuxSourceTreeOperations:
> + config_path = os.path.join(QEMU_CONFIGS_DIR, arch + '.py')
> + if arch == 'um':
> + return LinuxSourceTreeOperationsUml(cross_compile=cross_compile)
> + elif os.path.isfile(config_path):
> + return get_source_tree_ops_from_qemu_config(config_path, cross_compile)[1]
> + else:
> + raise ConfigError(arch + ' is not a valid arch')
> +
> +def get_source_tree_ops_from_qemu_config(config_path: str,
> + cross_compile: Optional[str]) -> tuple[
> + str, LinuxSourceTreeOperations]:
> + abs_config_path = os.path.abspath(config_path)
> + if os.path.commonpath([ABS_TOOL_PATH, abs_config_path]) != ABS_TOOL_PATH:
> + raise ConfigError('Given QEMU config file is not in a child directory of KUnit tool.')
I really don't like this requirement: it feels very arbitrary and
undercuts the value of the --qemu_config option a bit: we almost might
as well keep everything in the QEMU_CONFIG_DIR.
I assume it's here because of the need to import the QemuArchParams
definition: I take it there's no convenient way to do that?
At the very least, let's document this restriction properly, as it was
a bit of a weird one I wasn't expecting. Then people can either put
their custom qemu configs in the qemu_configs/ directory, or in
something like a custom_qemu_configs/ one. And if we later can work
out a more convenient way of removing this restriction entirely, we
can do so.
> + relative_config_path = os.path.relpath(abs_config_path, ABS_TOOL_PATH)
> + spec = importlib.util.spec_from_file_location('.' + relative_config_path, config_path)
> + config = importlib.util.module_from_spec(spec)
> + # TODO(brendanhiggins@google.com): I looked this up and apparently other
> + # Python projects have noted that pytype complains that "No attribute
> + # 'exec_module' on _importlib_modulespec._Loader". Disabling for now.
> + spec.loader.exec_module(config) # pytype: disable=attribute-error
> + return config.QEMU_ARCH.linux_arch, LinuxSourceTreeOperationsQemu(
> + config.QEMU_ARCH, cross_compile=cross_compile)
> +
> class LinuxSourceTree(object):
> """Represents a Linux kernel source tree with KUnit tests."""
>
> - def __init__(self, build_dir: str, load_config=True, kunitconfig_path='') -> None:
> + def __init__(
> + self,
> + build_dir: str,
> + load_config=True,
> + kunitconfig_path='',
> + arch=None,
> + cross_compile=None,
> + qemu_config_path=None) -> None:
> signal.signal(signal.SIGINT, self.signal_handler)
> -
> - self._ops = LinuxSourceTreeOperations()
> + if qemu_config_path:
> + self._arch, self._ops = get_source_tree_ops_from_qemu_config(
> + qemu_config_path, cross_compile)
> + else:
> + self._arch = 'um' if arch is None else arch
> + self._ops = get_source_tree_ops(self._arch, cross_compile)
>
> if not load_config:
> return
> @@ -170,8 +284,9 @@ class LinuxSourceTree(object):
> kconfig_path = get_kconfig_path(build_dir)
> if build_dir and not os.path.exists(build_dir):
> os.mkdir(build_dir)
> - self._kconfig.write_to_file(kconfig_path)
> try:
> + self._ops.make_arch_qemuconfig(self._kconfig)
> + self._kconfig.write_to_file(kconfig_path)
> self._ops.make_olddefconfig(build_dir, make_options)
> except ConfigError as e:
> logging.error(e)
> @@ -184,6 +299,7 @@ class LinuxSourceTree(object):
> if os.path.exists(kconfig_path):
> existing_kconfig = kunit_config.Kconfig()
> existing_kconfig.read_from_file(kconfig_path)
> + self._ops.make_arch_qemuconfig(self._kconfig)
> if not self._kconfig.is_subset_of(existing_kconfig):
> print('Regenerating .config ...')
> os.remove(kconfig_path)
> @@ -194,7 +310,7 @@ class LinuxSourceTree(object):
> print('Generating .config ...')
> return self.build_config(build_dir, make_options)
>
> - def build_um_kernel(self, alltests, jobs, build_dir, make_options) -> bool:
> + def build_kernel(self, alltests, jobs, build_dir, make_options) -> bool:
> try:
> if alltests:
> self._ops.make_allyesconfig(build_dir, make_options)
> @@ -211,8 +327,8 @@ class LinuxSourceTree(object):
> args.extend(['mem=1G', 'console=tty','kunit_shutdown=halt'])
> if filter_glob:
> args.append('kunit.filter_glob='+filter_glob)
> - self._ops.linux_bin(args, timeout, build_dir)
> outfile = get_outfile_path(build_dir)
> + self._ops.run(args, timeout, build_dir, outfile)
> subprocess.call(['stty', 'sane'])
> with open(outfile, 'r') as file:
> for line in file:
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index 2e809dd956a77..a3b7f68e1cf9f 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -303,7 +303,7 @@ class KUnitMainTest(unittest.TestCase):
>
> self.linux_source_mock = mock.Mock()
> self.linux_source_mock.build_reconfig = mock.Mock(return_value=True)
> - self.linux_source_mock.build_um_kernel = mock.Mock(return_value=True)
> + self.linux_source_mock.build_kernel = mock.Mock(return_value=True)
> self.linux_source_mock.run_kernel = mock.Mock(return_value=all_passed_log)
>
> def test_config_passes_args_pass(self):
> @@ -314,7 +314,7 @@ class KUnitMainTest(unittest.TestCase):
> def test_build_passes_args_pass(self):
> kunit.main(['build'], self.linux_source_mock)
> self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 0)
> - self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, '.kunit', None)
> + self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, '.kunit', None)
> self.assertEqual(self.linux_source_mock.run_kernel.call_count, 0)
>
> def test_exec_passes_args_pass(self):
> @@ -396,7 +396,7 @@ class KUnitMainTest(unittest.TestCase):
> def test_build_builddir(self):
> build_dir = '.kunit'
> kunit.main(['build', '--build_dir', build_dir], self.linux_source_mock)
> - self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, build_dir, None)
> + self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, build_dir, None)
>
> def test_exec_builddir(self):
> build_dir = '.kunit'
> @@ -410,14 +410,22 @@ class KUnitMainTest(unittest.TestCase):
> mock_linux_init.return_value = self.linux_source_mock
> kunit.main(['run', '--kunitconfig=mykunitconfig'])
> # Just verify that we parsed and initialized it correctly here.
> - mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig')
> + mock_linux_init.assert_called_once_with('.kunit',
> + kunitconfig_path='mykunitconfig',
> + arch='um',
> + cross_compile=None,
> + qemu_config_path=None)
>
> @mock.patch.object(kunit_kernel, 'LinuxSourceTree')
> def test_config_kunitconfig(self, mock_linux_init):
> mock_linux_init.return_value = self.linux_source_mock
> kunit.main(['config', '--kunitconfig=mykunitconfig'])
> # Just verify that we parsed and initialized it correctly here.
> - mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig')
> + mock_linux_init.assert_called_once_with('.kunit',
> + kunitconfig_path='mykunitconfig',
> + arch='um',
> + cross_compile=None,
> + qemu_config_path=None)
>
> if __name__ == '__main__':
> unittest.main()
> diff --git a/tools/testing/kunit/qemu_config.py b/tools/testing/kunit/qemu_config.py
> new file mode 100644
> index 0000000000000..aff1fe0442dbc
> --- /dev/null
> +++ b/tools/testing/kunit/qemu_config.py
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Collection of configs for building non-UML kernels and running them on QEMU.
> +#
> +# Copyright (C) 2021, Google LLC.
> +# Author: Brendan Higgins <brendanhiggins@google.com>
> +
> +from collections import namedtuple
> +
> +
> +QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',
> + 'qemuconfig',
> + 'qemu_arch',
> + 'kernel_path',
> + 'kernel_command_line',
> + 'extra_qemu_params'])
> +
Nit: newline at end of file.
> diff --git a/tools/testing/kunit/qemu_configs/alpha.py b/tools/testing/kunit/qemu_configs/alpha.py
> new file mode 100644
> index 0000000000000..2cc64f848ca2c
> --- /dev/null
> +++ b/tools/testing/kunit/qemu_configs/alpha.py
> @@ -0,0 +1,10 @@
> +from ..qemu_config import QemuArchParams
> +
> +QEMU_ARCH = QemuArchParams(linux_arch='alpha',
> + qemuconfig='''
> +CONFIG_SERIAL_8250=y
> +CONFIG_SERIAL_8250_CONSOLE=y''',
> + qemu_arch='alpha',
> + kernel_path='arch/alpha/boot/vmlinux',
> + kernel_command_line='console=ttyS0',
> + extra_qemu_params=[''])
> diff --git a/tools/testing/kunit/qemu_configs/arm.py b/tools/testing/kunit/qemu_configs/arm.py
> new file mode 100644
> index 0000000000000..29a043b0531a0
> --- /dev/null
> +++ b/tools/testing/kunit/qemu_configs/arm.py
> @@ -0,0 +1,13 @@
> +from ..qemu_config import QemuArchParams
> +
> +QEMU_ARCH = QemuArchParams(linux_arch='arm',
> + qemuconfig='''
> +CONFIG_ARCH_VIRT=y
> +CONFIG_SERIAL_AMBA_PL010=y
> +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> +CONFIG_SERIAL_AMBA_PL011=y
> +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
> + qemu_arch='arm',
> + kernel_path='arch/arm/boot/zImage',
> + kernel_command_line='console=ttyAMA0',
> + extra_qemu_params=['-machine virt'])
> diff --git a/tools/testing/kunit/qemu_configs/arm64.py b/tools/testing/kunit/qemu_configs/arm64.py
> new file mode 100644
> index 0000000000000..1ba200bc99f0f
> --- /dev/null
> +++ b/tools/testing/kunit/qemu_configs/arm64.py
> @@ -0,0 +1,12 @@
> +from ..qemu_config import QemuArchParams
> +
> +QEMU_ARCH = QemuArchParams(linux_arch='arm64',
> + qemuconfig='''
> +CONFIG_SERIAL_AMBA_PL010=y
> +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> +CONFIG_SERIAL_AMBA_PL011=y
> +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
> + qemu_arch='aarch64',
> + kernel_path='arch/arm64/boot/Image.gz',
> + kernel_command_line='console=ttyAMA0',
> + extra_qemu_params=['-machine virt', '-cpu cortex-a57'])
> diff --git a/tools/testing/kunit/qemu_configs/i386.py b/tools/testing/kunit/qemu_configs/i386.py
> new file mode 100644
> index 0000000000000..3998af306468e
> --- /dev/null
> +++ b/tools/testing/kunit/qemu_configs/i386.py
> @@ -0,0 +1,10 @@
> +from ..qemu_config import QemuArchParams
> +
> +QEMU_ARCH = QemuArchParams(linux_arch='i386',
> + qemuconfig='''
> +CONFIG_SERIAL_8250=y
> +CONFIG_SERIAL_8250_CONSOLE=y''',
> + qemu_arch='x86_64',
> + kernel_path='arch/x86/boot/bzImage',
> + kernel_command_line='console=ttyS0',
> + extra_qemu_params=[''])
> diff --git a/tools/testing/kunit/qemu_configs/powerpc.py b/tools/testing/kunit/qemu_configs/powerpc.py
> new file mode 100644
> index 0000000000000..46292ce9e368e
> --- /dev/null
> +++ b/tools/testing/kunit/qemu_configs/powerpc.py
> @@ -0,0 +1,12 @@
> +from ..qemu_config import QemuArchParams
> +
> +QEMU_ARCH = QemuArchParams(linux_arch='powerpc',
> + qemuconfig='''
> +CONFIG_PPC64=y
> +CONFIG_SERIAL_8250=y
> +CONFIG_SERIAL_8250_CONSOLE=y
> +CONFIG_HVC_CONSOLE=y''',
> + qemu_arch='ppc64',
> + kernel_path='vmlinux',
> + kernel_command_line='console=ttyS0',
> + extra_qemu_params=['-M pseries', '-cpu power8'])
> diff --git a/tools/testing/kunit/qemu_configs/riscv.py b/tools/testing/kunit/qemu_configs/riscv.py
> new file mode 100644
> index 0000000000000..de8c62d465723
> --- /dev/null
> +++ b/tools/testing/kunit/qemu_configs/riscv.py
> @@ -0,0 +1,31 @@
> +from ..qemu_config import QemuArchParams
> +import os
> +import os.path
> +import sys
> +
> +GITHUB_OPENSBI_URL = 'https://github.com/qemu/qemu/raw/master/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin'
> +OPENSBI_FILE = os.path.basename(GITHUB_OPENSBI_URL)
> +
> +if not os.path.isfile(OPENSBI_FILE):
> + print('\n\nOpenSBI file is not in the current working directory.\n'
> + 'Would you like me to download it for you from:\n' + GITHUB_OPENSBI_URL + ' ?\n')
> + response = input('yes/[no]: ')
> + if response.strip() == 'yes':
> + os.system('wget ' + GITHUB_OPENSBI_URL)
> + else:
> + sys.exit()
> +
> +QEMU_ARCH = QemuArchParams(linux_arch='riscv',
> + qemuconfig='''
> +CONFIG_SOC_VIRT=y
> +CONFIG_SERIAL_8250=y
> +CONFIG_SERIAL_8250_CONSOLE=y
> +CONFIG_SERIAL_OF_PLATFORM=y
> +CONFIG_SERIAL_EARLYCON_RISCV_SBI=y''',
> + qemu_arch='riscv64',
> + kernel_path='arch/riscv/boot/Image',
> + kernel_command_line='console=ttyS0',
> + extra_qemu_params=[
> + '-machine virt',
> + '-cpu rv64',
> + '-bios opensbi-riscv64-generic-fw_dynamic.bin'])
> diff --git a/tools/testing/kunit/qemu_configs/s390.py b/tools/testing/kunit/qemu_configs/s390.py
> new file mode 100644
> index 0000000000000..04c90332f1098
> --- /dev/null
> +++ b/tools/testing/kunit/qemu_configs/s390.py
> @@ -0,0 +1,14 @@
> +from ..qemu_config import QemuArchParams
> +
> +QEMU_ARCH = QemuArchParams(linux_arch='s390',
> + qemuconfig='''
> +CONFIG_EXPERT=y
> +CONFIG_TUNE_ZEC12=y
> +CONFIG_NUMA=y
> +CONFIG_MODULES=y''',
> + qemu_arch='s390x',
> + kernel_path='arch/s390/boot/bzImage',
> + kernel_command_line='console=ttyS0',
> + extra_qemu_params=[
> + '-machine s390-ccw-virtio',
> + '-cpu qemu',])
> diff --git a/tools/testing/kunit/qemu_configs/sparc.py b/tools/testing/kunit/qemu_configs/sparc.py
> new file mode 100644
> index 0000000000000..f26b5f27cc5a1
> --- /dev/null
> +++ b/tools/testing/kunit/qemu_configs/sparc.py
> @@ -0,0 +1,10 @@
> +from ..qemu_config import QemuArchParams
> +
> +QEMU_ARCH = QemuArchParams(linux_arch='sparc',
> + qemuconfig='''
> +CONFIG_SERIAL_8250=y
> +CONFIG_SERIAL_8250_CONSOLE=y''',
> + qemu_arch='sparc',
> + kernel_path='arch/sparc/boot/zImage',
> + kernel_command_line='console=ttyS0 mem=256M',
> + extra_qemu_params=['-m 256'])
> diff --git a/tools/testing/kunit/qemu_configs/x86_64.py b/tools/testing/kunit/qemu_configs/x86_64.py
> new file mode 100644
> index 0000000000000..bd5ab733b92ac
> --- /dev/null
> +++ b/tools/testing/kunit/qemu_configs/x86_64.py
> @@ -0,0 +1,10 @@
> +from ..qemu_config import QemuArchParams
> +
> +QEMU_ARCH = QemuArchParams(linux_arch='x86_64',
> + qemuconfig='''
> +CONFIG_SERIAL_8250=y
> +CONFIG_SERIAL_8250_CONSOLE=y''',
> + qemu_arch='x86_64',
> + kernel_path='arch/x86/boot/bzImage',
> + kernel_command_line='console=ttyS0',
> + extra_qemu_params=[''])
> --
> 2.31.1.607.g51e8a6a459-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4000 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 4/4] Documentation: kunit: document support for QEMU in kunit_tool
2021-05-07 21:31 ` [PATCH v1 4/4] Documentation: kunit: document support for QEMU in kunit_tool Brendan Higgins
@ 2021-05-15 8:01 ` David Gow
2021-05-18 21:08 ` Brendan Higgins
0 siblings, 1 reply; 16+ messages in thread
From: David Gow @ 2021-05-15 8:01 UTC (permalink / raw)
To: Brendan Higgins
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
[-- Attachment #1: Type: text/plain, Size: 4077 bytes --]
On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> Document QEMU support, what it does, and how to use it in kunit_tool.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> ---
This is a good start, and probably meets the minimum requirements, but
I do have a number of comments and suggestions below.
Cheers,
-- David
> Documentation/dev-tools/kunit/usage.rst | 37 +++++++++++++++++++++----
> 1 file changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
> index 650f99590df57..b74bd7c87cc20 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -612,14 +612,39 @@ only things to be aware of when doing so.
> The biggest impediment will likely be that certain KUnit features and
> infrastructure may not support your target environment. For example, at this
> time the KUnit Wrapper (``tools/testing/kunit/kunit.py``) does not work outside
> -of UML. Unfortunately, there is no way around this. Using UML (or even just a
> -particular architecture) allows us to make a lot of assumptions that make it
> -possible to do things which might otherwise be impossible.
> +of UML and QEMU. Unfortunately, there is no way around this. Using UML and QEMU
> +(or even just a particular architecture) allows us to make a lot of assumptions
> +that make it possible to do things which might otherwise be impossible.
This is a bit more awkward now, and I don't think gives quite the
right impression. Particularly the "Unfortunately, there is no way
around this." bit: there's no fundamental reason that someone couldn't
implement support for some other emulator (or even a setup which
pushed to real hardware and read results over serial), it'd just take
a bit of work to implement (like this patch series has done for qemu).
Personally, I think it'd be easiest to simplify this section and say
that kunit_tool currently only fully supports some architectures, via
UML and QEMU.
>
> Nevertheless, all core KUnit framework features are fully supported on all
> -architectures, and using them is straightforward: all you need to do is to take
> -your kunitconfig, your Kconfig options for the tests you would like to run, and
> -merge them into whatever config your are using for your platform. That's it!
> +architectures, and using them is straightforward: Most popular architectures
> +are supported directly in the KUnit Wrapper via QEMU. Currently, supported
> +architectures on QEMU include:
> +
> +* i386
> +* x86_64
> +* arm
> +* arm64
> +* alpha
> +* powerpc
> +* riscv
> +* s390
> +* sparc
> +
> +In order to run KUnit tests on one of these architectures via QEMU with the
> +KUnit wrapper, all you need to do is specify the flags ``--arch`` and
> +``--cross_compile`` when invoking the KUnit Wrapper. For example, we could run
> +the default KUnit tests on ARM in the following manner (assuming we have an ARM
> +toolchain installed):
> +
> +.. code-block:: bash
> +
> + tools/testing/kunit/kunit.py run --timeout=60 --jobs=12 --arch=arm --cross_compile=arm-linux-gnueabihf-
> +
Is it worth also documenting the --qemu_config option here?
(Particularly given the restriction on its path?) Or is that something
best added to the kunit_tool page?
That being said, changes to the kunit_tool page are probably worth
adding as a section in the updated page:
https://patchwork.kernel.org/project/linux-kselftest/patch/20210417034553.1048895-1-davidgow@google.com/
At the very least, it'd be nice to have the new QEMU-related options
documented there.
> +Alternatively, if you want to run your tests on real hardware or in some other
> +emulation environment, all you need to do is to take your kunitconfig, your
> +Kconfig options for the tests you would like to run, and merge them into
> +whatever config your are using for your platform. That's it!
>
> For example, let's say you have the following kunitconfig:
>
> --
> 2.31.1.607.g51e8a6a459-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4000 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/4] kunit: tool: add support for QEMU
2021-05-15 7:59 ` David Gow
@ 2021-05-18 3:01 ` David Gow
2021-05-18 20:43 ` Brendan Higgins
2021-05-18 20:27 ` Brendan Higgins
1 sibling, 1 reply; 16+ messages in thread
From: David Gow @ 2021-05-18 3:01 UTC (permalink / raw)
To: Brendan Higgins
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
On Sat, May 15, 2021 at 3:59 PM David Gow <davidgow@google.com> wrote:
>
> On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
> <brendanhiggins@google.com> wrote:
> >
> > Add basic support to run QEMU via kunit_tool. Add support for i386,
> > x86_64, arm, arm64, and a bunch more.
> >
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > Tested-by: David Gow <davidgow@google.com>
> > ---
> >
> > Changes since last revision:
> >
> > - A number of minor obvious issues pointed out by David and Daniel.
> > - Added facility for merging Kconfigs at Daniel's suggestion.
> > - Broke out qemu_configs each into their own config file which is loaded
> > dynamically - mostly at David's suggestion.
> >
> > ---
>
> This seems pretty good to me. I only have one real complaint --
> qemu_configs needing to be in a subdirectory of ./tools/testing/kunit
> -- but am able to tolerate that (even if I'd prefer not to have it) if
> it's documented properly.
>
> Otherwise, save for a couple of minor nitpicks, this seems good to go.
>
> Reviewed-by: David Gow <davidgow@google.com>
>
>
One thing I forgot to mention is that I'm not 100% sure about the
Kconfig fragments being embedded in the qemu_configs. I still kind-of
prefer the idea of them being in separate config files. While I don't
think this is necessarily a blocker, I did just realise that, by
default, kunit.py run --arch=<non-UM-arch> will pull its default
.kunitconfig from arch/um/configs/kunit_defconfig, which definitely
feels awkward when UML is not otherwise involved.
Some further thoughts below (which range a bit from "practical
suggestion" to "overcomplicated ponderings", so don't feel the
pressure to take all of them).
(...snip...)
> > diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> > index e22ade9d91ad5..2bd196fd69e5c 100644
> > --- a/tools/testing/kunit/kunit_kernel.py
> > +++ b/tools/testing/kunit/kunit_kernel.py
> > @@ -6,23 +6,31 @@
> > # Author: Felix Guo <felixguoxiuping@gmail.com>
> > # Author: Brendan Higgins <brendanhiggins@google.com>
> >
> > +from __future__ import annotations
> > +import importlib.util
> > import logging
> > import subprocess
> > import os
> > import shutil
> > import signal
> > from typing import Iterator
> > +from typing import Optional
> >
> > from contextlib import ExitStack
> >
> > +from collections import namedtuple
> > +
> > import kunit_config
> > import kunit_parser
> > +import qemu_config
> >
> > KCONFIG_PATH = '.config'
> > KUNITCONFIG_PATH = '.kunitconfig'
> > DEFAULT_KUNITCONFIG_PATH = 'arch/um/configs/kunit_defconfig'
This being in arch/um doesn't seem great if its being used for non-UML
builds. Is it worth either:
(a) moving this somewhere else (e.g., tools/testing/kunit/configs as
with the BROKEN_ALLCONFIG_PATH beflow), or
(b) giving each architecture its own kunit_defconfig, possibly in
place of the qemuconfig member of QemuArchParams
I'm leaning towards (b), which solves two different sources of
ugliness in one go, though it would appear to have the downside that
the default .kunitconfig could end up being architecture specific,
which isn't great.
> > BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
> > OUTFILE_PATH = 'test.log'
> > +ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
> > +QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
> >
(...snip...)
> > diff --git a/tools/testing/kunit/qemu_config.py b/tools/testing/kunit/qemu_config.py
> > new file mode 100644
> > index 0000000000000..aff1fe0442dbc
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_config.py
> > @@ -0,0 +1,17 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Collection of configs for building non-UML kernels and running them on QEMU.
> > +#
> > +# Copyright (C) 2021, Google LLC.
> > +# Author: Brendan Higgins <brendanhiggins@google.com>
> > +
> > +from collections import namedtuple
> > +
> > +
> > +QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',
> > + 'qemuconfig',
As mentioned, I'm not thrilled about keeping the Kconfig inline here,
and would kind-of prefer it to be in another file. I could live with
it if I have to, though. Regardless, 'qemuconfig' is not a
super-descriptive name, particularly as it's not clear if this is
configuring QEMU (no, that's extra_qemu_params'), or configuring the
kernel for QEMU compatibility.
> > + 'qemu_arch',
> > + 'kernel_path',
> > + 'kernel_command_line',
> > + 'extra_qemu_params'])
> > +
>
> Nit: newline at end of file.
>
>
>
> > diff --git a/tools/testing/kunit/qemu_configs/alpha.py b/tools/testing/kunit/qemu_configs/alpha.py
> > new file mode 100644
> > index 0000000000000..2cc64f848ca2c
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/alpha.py
> > @@ -0,0 +1,10 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='alpha',
> > + qemuconfig='''
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y''',
If these were in a separate file, they could be shared across alpha,
i386, x86_64, etc. Of course, that wouldn't gel well with putting them
in arch/.../config. If there were some way of listing multiple files,
it could form part of the config for several more architectures,
though that's probably overcomplicating things.
> > + qemu_arch='alpha',
> > + kernel_path='arch/alpha/boot/vmlinux',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[''])
> > diff --git a/tools/testing/kunit/qemu_configs/arm.py b/tools/testing/kunit/qemu_configs/arm.py
> > new file mode 100644
> > index 0000000000000..29a043b0531a0
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/arm.py
> > @@ -0,0 +1,13 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='arm',
> > + qemuconfig='''
> > +CONFIG_ARCH_VIRT=y
> > +CONFIG_SERIAL_AMBA_PL010=y
> > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > +CONFIG_SERIAL_AMBA_PL011=y
> > +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
Similarly, if in a separate file and there were some multiple-file
mechanism, these could mostly be shared between arm & arm64 (ARCH_VIRT
being the only problem). Again, probably overcomplicating it at this
point though.
> > + qemu_arch='arm',
> > + kernel_path='arch/arm/boot/zImage',
> > + kernel_command_line='console=ttyAMA0',
> > + extra_qemu_params=['-machine virt'])
> > diff --git a/tools/testing/kunit/qemu_configs/arm64.py b/tools/testing/kunit/qemu_configs/arm64.py
> > new file mode 100644
> > index 0000000000000..1ba200bc99f0f
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/arm64.py
> > @@ -0,0 +1,12 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='arm64',
> > + qemuconfig='''
> > +CONFIG_SERIAL_AMBA_PL010=y
> > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > +CONFIG_SERIAL_AMBA_PL011=y
> > +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
> > + qemu_arch='aarch64',
> > + kernel_path='arch/arm64/boot/Image.gz',
> > + kernel_command_line='console=ttyAMA0',
> > + extra_qemu_params=['-machine virt', '-cpu cortex-a57'])
> > diff --git a/tools/testing/kunit/qemu_configs/i386.py b/tools/testing/kunit/qemu_configs/i386.py
> > new file mode 100644
> > index 0000000000000..3998af306468e
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/i386.py
> > @@ -0,0 +1,10 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='i386',
> > + qemuconfig='''
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > + qemu_arch='x86_64',
> > + kernel_path='arch/x86/boot/bzImage',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[''])
> > diff --git a/tools/testing/kunit/qemu_configs/powerpc.py b/tools/testing/kunit/qemu_configs/powerpc.py
> > new file mode 100644
> > index 0000000000000..46292ce9e368e
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/powerpc.py
> > @@ -0,0 +1,12 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='powerpc',
> > + qemuconfig='''
> > +CONFIG_PPC64=y
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y
> > +CONFIG_HVC_CONSOLE=y''',
> > + qemu_arch='ppc64',
> > + kernel_path='vmlinux',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=['-M pseries', '-cpu power8'])
> > diff --git a/tools/testing/kunit/qemu_configs/riscv.py b/tools/testing/kunit/qemu_configs/riscv.py
> > new file mode 100644
> > index 0000000000000..de8c62d465723
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/riscv.py
> > @@ -0,0 +1,31 @@
> > +from ..qemu_config import QemuArchParams
> > +import os
> > +import os.path
> > +import sys
> > +
> > +GITHUB_OPENSBI_URL = 'https://github.com/qemu/qemu/raw/master/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin'
> > +OPENSBI_FILE = os.path.basename(GITHUB_OPENSBI_URL)
> > +
> > +if not os.path.isfile(OPENSBI_FILE):
> > + print('\n\nOpenSBI file is not in the current working directory.\n'
> > + 'Would you like me to download it for you from:\n' + GITHUB_OPENSBI_URL + ' ?\n')
> > + response = input('yes/[no]: ')
> > + if response.strip() == 'yes':
> > + os.system('wget ' + GITHUB_OPENSBI_URL)
> > + else:
> > + sys.exit()
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='riscv',
> > + qemuconfig='''
> > +CONFIG_SOC_VIRT=y
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y
> > +CONFIG_SERIAL_OF_PLATFORM=y
> > +CONFIG_SERIAL_EARLYCON_RISCV_SBI=y''',
> > + qemu_arch='riscv64',
> > + kernel_path='arch/riscv/boot/Image',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[
> > + '-machine virt',
> > + '-cpu rv64',
> > + '-bios opensbi-riscv64-generic-fw_dynamic.bin'])
> > diff --git a/tools/testing/kunit/qemu_configs/s390.py b/tools/testing/kunit/qemu_configs/s390.py
> > new file mode 100644
> > index 0000000000000..04c90332f1098
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/s390.py
> > @@ -0,0 +1,14 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='s390',
> > + qemuconfig='''
> > +CONFIG_EXPERT=y
> > +CONFIG_TUNE_ZEC12=y
> > +CONFIG_NUMA=y
> > +CONFIG_MODULES=y''',
> > + qemu_arch='s390x',
> > + kernel_path='arch/s390/boot/bzImage',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[
> > + '-machine s390-ccw-virtio',
> > + '-cpu qemu',])
> > diff --git a/tools/testing/kunit/qemu_configs/sparc.py b/tools/testing/kunit/qemu_configs/sparc.py
> > new file mode 100644
> > index 0000000000000..f26b5f27cc5a1
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/sparc.py
> > @@ -0,0 +1,10 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='sparc',
> > + qemuconfig='''
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > + qemu_arch='sparc',
> > + kernel_path='arch/sparc/boot/zImage',
> > + kernel_command_line='console=ttyS0 mem=256M',
> > + extra_qemu_params=['-m 256'])
> > diff --git a/tools/testing/kunit/qemu_configs/x86_64.py b/tools/testing/kunit/qemu_configs/x86_64.py
> > new file mode 100644
> > index 0000000000000..bd5ab733b92ac
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/x86_64.py
> > @@ -0,0 +1,10 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='x86_64',
> > + qemuconfig='''
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > + qemu_arch='x86_64',
> > + kernel_path='arch/x86/boot/bzImage',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[''])
> > --
> > 2.31.1.607.g51e8a6a459-goog
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/4] kunit: tool: add support for QEMU
2021-05-15 7:59 ` David Gow
2021-05-18 3:01 ` David Gow
@ 2021-05-18 20:27 ` Brendan Higgins
1 sibling, 0 replies; 16+ messages in thread
From: Brendan Higgins @ 2021-05-18 20:27 UTC (permalink / raw)
To: David Gow
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
On Sat, May 15, 2021 at 12:59 AM David Gow <davidgow@google.com> wrote:
>
> On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
> <brendanhiggins@google.com> wrote:
> >
> > Add basic support to run QEMU via kunit_tool. Add support for i386,
> > x86_64, arm, arm64, and a bunch more.
> >
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > Tested-by: David Gow <davidgow@google.com>
> > ---
> >
> > Changes since last revision:
> >
> > - A number of minor obvious issues pointed out by David and Daniel.
> > - Added facility for merging Kconfigs at Daniel's suggestion.
> > - Broke out qemu_configs each into their own config file which is loaded
> > dynamically - mostly at David's suggestion.
> >
> > ---
>
> This seems pretty good to me. I only have one real complaint --
> qemu_configs needing to be in a subdirectory of ./tools/testing/kunit
> -- but am able to tolerate that (even if I'd prefer not to have it) if
> it's documented properly.
>
> Otherwise, save for a couple of minor nitpicks, this seems good to go.
>
> Reviewed-by: David Gow <davidgow@google.com>
>
>
> > tools/testing/kunit/kunit.py | 57 ++++++-
> > tools/testing/kunit/kunit_config.py | 7 +-
> > tools/testing/kunit/kunit_kernel.py | 170 ++++++++++++++++----
> > tools/testing/kunit/kunit_tool_test.py | 18 ++-
> > tools/testing/kunit/qemu_config.py | 17 ++
> > tools/testing/kunit/qemu_configs/alpha.py | 10 ++
> > tools/testing/kunit/qemu_configs/arm.py | 13 ++
> > tools/testing/kunit/qemu_configs/arm64.py | 12 ++
> > tools/testing/kunit/qemu_configs/i386.py | 10 ++
> > tools/testing/kunit/qemu_configs/powerpc.py | 12 ++
> > tools/testing/kunit/qemu_configs/riscv.py | 31 ++++
> > tools/testing/kunit/qemu_configs/s390.py | 14 ++
> > tools/testing/kunit/qemu_configs/sparc.py | 10 ++
> > tools/testing/kunit/qemu_configs/x86_64.py | 10 ++
> > 14 files changed, 350 insertions(+), 41 deletions(-)
> > create mode 100644 tools/testing/kunit/qemu_config.py
> > create mode 100644 tools/testing/kunit/qemu_configs/alpha.py
> > create mode 100644 tools/testing/kunit/qemu_configs/arm.py
> > create mode 100644 tools/testing/kunit/qemu_configs/arm64.py
> > create mode 100644 tools/testing/kunit/qemu_configs/i386.py
> > create mode 100644 tools/testing/kunit/qemu_configs/powerpc.py
> > create mode 100644 tools/testing/kunit/qemu_configs/riscv.py
> > create mode 100644 tools/testing/kunit/qemu_configs/s390.py
> > create mode 100644 tools/testing/kunit/qemu_configs/sparc.py
> > create mode 100644 tools/testing/kunit/qemu_configs/x86_64.py
> >
> > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> > index 5da8fb3762f98..be8d8d4a4e08f 100755
> > --- a/tools/testing/kunit/kunit.py
> > +++ b/tools/testing/kunit/kunit.py
> > @@ -70,10 +70,10 @@ def build_tests(linux: kunit_kernel.LinuxSourceTree,
> > kunit_parser.print_with_timestamp('Building KUnit Kernel ...')
> >
> > build_start = time.time()
> > - success = linux.build_um_kernel(request.alltests,
> > - request.jobs,
> > - request.build_dir,
> > - request.make_options)
> > + success = linux.build_kernel(request.alltests,
> > + request.jobs,
> > + request.build_dir,
> > + request.make_options)
> > build_end = time.time()
> > if not success:
> > return KunitResult(KunitStatus.BUILD_FAILURE,
> > @@ -189,6 +189,31 @@ def add_common_opts(parser) -> None:
> > 'will get automatically appended.',
> > metavar='kunitconfig')
> >
> > + parser.add_argument('--arch',
> > + help=('Specifies the architecture to run tests under. '
> > + 'The architecture specified here must match the '
> > + 'string passed to the ARCH make param, '
> > + 'e.g. i386, x86_64, arm, um, etc. Non-UML '
> > + 'architectures run on QEMU.'),
> > + type=str, default='um', metavar='arch')
> > +
> > + parser.add_argument('--cross_compile',
> > + help=('Sets make\'s CROSS_COMPILE variable; it should '
> > + 'be set to a toolchain path prefix (the prefix '
> > + 'of gcc and other tools in your toolchain, for '
> > + 'example `sparc64-linux-gnu-` if you have the '
> > + 'sparc toolchain installed on your system, or '
> > + '`$HOME/toolchains/microblaze/gcc-9.2.0-nolibc/microblaze-linux/bin/microblaze-linux-` '
> Super-minor nit: is there a shorter example we can give that's not
> much longer than all the surrounding lines?
I wanted something which actually uses the paths that matches real gcc
toolchains and they don't get much shorter than this (I guess I could
drop $HOME/toolchains/microblaze, but it would still be very long).
Think maybe I should just drop this secondary example entirely? Anyone
trying to download and use a toolchain probably has better resources
to follow anyway.
> > + 'if you have downloaded the microblaze toolchain '
> > + 'from the 0-day website to a directory in your '
> > + 'home directory called `toolchains`).'),
> > + metavar='cross_compile')
> > +
> > + parser.add_argument('--qemu_config',
> > + help=('Takes a path to a path to a file containing '
> > + 'a QemuArchParams object.'),
> > + type=str, metavar='qemu_config')
> > +
>
> If (as noted below) this file needs to be in a subdirectory of the
> kunit_tool directory, we should document this, or maybe make this path
> relative to the kunit_tool directory.
I'll go into more detail below, but I was able to change this to take
a config from anywhere.
> > def add_build_opts(parser) -> None:
> > parser.add_argument('--jobs',
> > help='As in the make command, "Specifies the number of '
> > @@ -270,7 +295,11 @@ def main(argv, linux=None):
> > os.mkdir(cli_args.build_dir)
> >
> > if not linux:
> > - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
> > + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> > + kunitconfig_path=cli_args.kunitconfig,
> > + arch=cli_args.arch,
> > + cross_compile=cli_args.cross_compile,
> > + qemu_config_path=cli_args.qemu_config)
> >
> > request = KunitRequest(cli_args.raw_output,
> > cli_args.timeout,
> > @@ -289,7 +318,11 @@ def main(argv, linux=None):
> > os.mkdir(cli_args.build_dir)
> >
> > if not linux:
> > - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
> > + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> > + kunitconfig_path=cli_args.kunitconfig,
> > + arch=cli_args.arch,
> > + cross_compile=cli_args.cross_compile,
> > + qemu_config_path=cli_args.qemu_config)
> >
> > request = KunitConfigRequest(cli_args.build_dir,
> > cli_args.make_options)
> > @@ -301,7 +334,11 @@ def main(argv, linux=None):
> > sys.exit(1)
> > elif cli_args.subcommand == 'build':
> > if not linux:
> > - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
> > + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> > + kunitconfig_path=cli_args.kunitconfig,
> > + arch=cli_args.arch,
> > + cross_compile=cli_args.cross_compile,
> > + qemu_config_path=cli_args.qemu_config)
> >
> > request = KunitBuildRequest(cli_args.jobs,
> > cli_args.build_dir,
> > @@ -315,7 +352,11 @@ def main(argv, linux=None):
> > sys.exit(1)
> > elif cli_args.subcommand == 'exec':
> > if not linux:
> > - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
> > + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> > + kunitconfig_path=cli_args.kunitconfig,
> > + arch=cli_args.arch,
> > + cross_compile=cli_args.cross_compile,
> > + qemu_config_path=cli_args.qemu_config)
> >
> > exec_request = KunitExecRequest(cli_args.timeout,
> > cli_args.build_dir,
> > diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py
> > index 1e2683dcc0e7a..c77c7d2ef6220 100644
> > --- a/tools/testing/kunit/kunit_config.py
> > +++ b/tools/testing/kunit/kunit_config.py
> > @@ -52,8 +52,13 @@ class Kconfig(object):
> > return False
> > return True
> >
> > + def merge_in_entries(self, other: 'Kconfig') -> None:
> > + if other.is_subset_of(self):
> > + return
> > + self._entries = list(self.entries().union(other.entries()))
> > +
> > def write_to_file(self, path: str) -> None:
> > - with open(path, 'w') as f:
> > + with open(path, 'a+') as f:
> > for entry in self.entries():
> > f.write(str(entry) + '\n')
> >
> > diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> > index e22ade9d91ad5..2bd196fd69e5c 100644
> > --- a/tools/testing/kunit/kunit_kernel.py
> > +++ b/tools/testing/kunit/kunit_kernel.py
> > @@ -6,23 +6,31 @@
> > # Author: Felix Guo <felixguoxiuping@gmail.com>
> > # Author: Brendan Higgins <brendanhiggins@google.com>
> >
> > +from __future__ import annotations
> > +import importlib.util
> > import logging
> > import subprocess
> > import os
> > import shutil
> > import signal
> > from typing import Iterator
> > +from typing import Optional
> >
> > from contextlib import ExitStack
> >
> > +from collections import namedtuple
> > +
> > import kunit_config
> > import kunit_parser
> > +import qemu_config
> >
> > KCONFIG_PATH = '.config'
> > KUNITCONFIG_PATH = '.kunitconfig'
> > DEFAULT_KUNITCONFIG_PATH = 'arch/um/configs/kunit_defconfig'
> > BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
> > OUTFILE_PATH = 'test.log'
> > +ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
> > +QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
> >
> > def get_file_path(build_dir, default):
> > if build_dir:
> > @@ -40,6 +48,10 @@ class BuildError(Exception):
> > class LinuxSourceTreeOperations(object):
> > """An abstraction over command line operations performed on a source tree."""
> >
> > + def __init__(self, linux_arch: str, cross_compile: Optional[str]):
> > + self._linux_arch = linux_arch
> > + self._cross_compile = cross_compile
> > +
> > def make_mrproper(self) -> None:
> > try:
> > subprocess.check_output(['make', 'mrproper'], stderr=subprocess.STDOUT)
> > @@ -48,12 +60,21 @@ class LinuxSourceTreeOperations(object):
> > except subprocess.CalledProcessError as e:
> > raise ConfigError(e.output.decode())
> >
> > + def make_arch_qemuconfig(self, kconfig: kunit_config.Kconfig) -> None:
> > + pass
> > +
> > + def make_allyesconfig(self, build_dir, make_options) -> None:
> > + raise ConfigError('Only the "um" arch is supported for alltests')
> > +
> > def make_olddefconfig(self, build_dir, make_options) -> None:
> > - command = ['make', 'ARCH=um', 'olddefconfig']
> > + command = ['make', 'ARCH=' + self._linux_arch, 'olddefconfig']
> > + if self._cross_compile:
> > + command += ['CROSS_COMPILE=' + self._cross_compile]
> > if make_options:
> > command.extend(make_options)
> > if build_dir:
> > command += ['O=' + build_dir]
> > + print('Populating config with:\n$', ' '.join(command))
> > try:
> > subprocess.check_output(command, stderr=subprocess.STDOUT)
> > except OSError as e:
> > @@ -61,6 +82,79 @@ class LinuxSourceTreeOperations(object):
> > except subprocess.CalledProcessError as e:
> > raise ConfigError(e.output.decode())
> >
> > + def make(self, jobs, build_dir, make_options) -> None:
> > + command = ['make', 'ARCH=' + self._linux_arch, '--jobs=' + str(jobs)]
> > + if make_options:
> > + command.extend(make_options)
> > + if self._cross_compile:
> > + command += ['CROSS_COMPILE=' + self._cross_compile]
> > + if build_dir:
> > + command += ['O=' + build_dir]
> > + print('Building with:\n$', ' '.join(command))
> > + try:
> > + proc = subprocess.Popen(command,
> > + stderr=subprocess.PIPE,
> > + stdout=subprocess.DEVNULL)
> > + except OSError as e:
> > + raise BuildError('Could not call execute make: ' + str(e))
> > + except subprocess.CalledProcessError as e:
> > + raise BuildError(e.output)
> > + _, stderr = proc.communicate()
> > + if proc.returncode != 0:
> > + raise BuildError(stderr.decode())
> > + if stderr: # likely only due to build warnings
> > + print(stderr.decode())
> > +
> > + def run(self, params, timeout, build_dir, outfile) -> None:
> > + pass
> > +
> > +
> > +class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
> > +
> > + def __init__(self, qemu_arch_params: qemu_config.QemuArchParams, cross_compile: Optional[str]):
> > + super().__init__(linux_arch=qemu_arch_params.linux_arch,
> > + cross_compile=cross_compile)
> > + self._qemuconfig = qemu_arch_params.qemuconfig
> > + self._qemu_arch = qemu_arch_params.qemu_arch
> > + self._kernel_path = qemu_arch_params.kernel_path
> > + self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot'
> > + self._extra_qemu_params = qemu_arch_params.extra_qemu_params
> > +
> > + def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None:
> > + qemuconfig = kunit_config.Kconfig()
> > + qemuconfig.parse_from_string(self._qemuconfig)
> > + base_kunitconfig.merge_in_entries(qemuconfig)
> > +
> > + def run(self, params, timeout, build_dir, outfile):
> > + kernel_path = os.path.join(build_dir, self._kernel_path)
> > + qemu_command = ['qemu-system-' + self._qemu_arch,
> > + '-nodefaults',
> > + '-m', '1024',
> > + '-kernel', kernel_path,
> > + '-append', '\'' + ' '.join(params + [self._kernel_command_line]) + '\'',
> > + '-no-reboot',
> > + '-nographic',
> > + '-serial stdio'] + self._extra_qemu_params
> > + print('Running tests with:\n$', ' '.join(qemu_command))
> > + with open(outfile, 'w') as output:
> > + process = subprocess.Popen(' '.join(qemu_command),
> > + stdin=subprocess.PIPE,
> > + stdout=output,
> > + stderr=subprocess.STDOUT,
> > + text=True, shell=True)
> > + try:
> > + process.wait(timeout=timeout)
> > + except Exception as e:
> > + print(e)
> > + process.terminate()
> > + return process
> > +
> > +class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
> > + """An abstraction over command line operations performed on a source tree."""
> > +
> > + def __init__(self, cross_compile=None):
> > + super().__init__(linux_arch='um', cross_compile=cross_compile)
> > +
> > def make_allyesconfig(self, build_dir, make_options) -> None:
> > kunit_parser.print_with_timestamp(
> > 'Enabling all CONFIGs for UML...')
> > @@ -83,32 +177,16 @@ class LinuxSourceTreeOperations(object):
> > kunit_parser.print_with_timestamp(
> > 'Starting Kernel with all configs takes a few minutes...')
> >
> > - def make(self, jobs, build_dir, make_options) -> None:
> > - command = ['make', 'ARCH=um', '--jobs=' + str(jobs)]
> > - if make_options:
> > - command.extend(make_options)
> > - if build_dir:
> > - command += ['O=' + build_dir]
> > - try:
> > - proc = subprocess.Popen(command,
> > - stderr=subprocess.PIPE,
> > - stdout=subprocess.DEVNULL)
> > - except OSError as e:
> > - raise BuildError('Could not call make command: ' + str(e))
> > - _, stderr = proc.communicate()
> > - if proc.returncode != 0:
> > - raise BuildError(stderr.decode())
> > - if stderr: # likely only due to build warnings
> > - print(stderr.decode())
> > -
> > - def linux_bin(self, params, timeout, build_dir) -> None:
> > + def run(self, params, timeout, build_dir, outfile):
> > """Runs the Linux UML binary. Must be named 'linux'."""
> > linux_bin = get_file_path(build_dir, 'linux')
> > outfile = get_outfile_path(build_dir)
> > with open(outfile, 'w') as output:
> > process = subprocess.Popen([linux_bin] + params,
> > + stdin=subprocess.PIPE,
> > stdout=output,
> > - stderr=subprocess.STDOUT)
> > + stderr=subprocess.STDOUT,
> > + text=True)
> > process.wait(timeout)
> >
> > def get_kconfig_path(build_dir) -> str:
> > @@ -120,13 +198,49 @@ def get_kunitconfig_path(build_dir) -> str:
> > def get_outfile_path(build_dir) -> str:
> > return get_file_path(build_dir, OUTFILE_PATH)
> >
> > +def get_source_tree_ops(arch: str, cross_compile: Optional[str]) -> LinuxSourceTreeOperations:
> > + config_path = os.path.join(QEMU_CONFIGS_DIR, arch + '.py')
> > + if arch == 'um':
> > + return LinuxSourceTreeOperationsUml(cross_compile=cross_compile)
> > + elif os.path.isfile(config_path):
> > + return get_source_tree_ops_from_qemu_config(config_path, cross_compile)[1]
> > + else:
> > + raise ConfigError(arch + ' is not a valid arch')
> > +
> > +def get_source_tree_ops_from_qemu_config(config_path: str,
> > + cross_compile: Optional[str]) -> tuple[
> > + str, LinuxSourceTreeOperations]:
> > + abs_config_path = os.path.abspath(config_path)
> > + if os.path.commonpath([ABS_TOOL_PATH, abs_config_path]) != ABS_TOOL_PATH:
> > + raise ConfigError('Given QEMU config file is not in a child directory of KUnit tool.')
>
> I really don't like this requirement: it feels very arbitrary and
> undercuts the value of the --qemu_config option a bit: we almost might
> as well keep everything in the QEMU_CONFIG_DIR.
>
> I assume it's here because of the need to import the QemuArchParams
> definition: I take it there's no convenient way to do that?
I thought it was a requirement, but I was wrong. I was able to fix it.
In the next revision, no such requirement will exist.
> At the very least, let's document this restriction properly, as it was
> a bit of a weird one I wasn't expecting. Then people can either put
> their custom qemu configs in the qemu_configs/ directory, or in
> something like a custom_qemu_configs/ one. And if we later can work
> out a more convenient way of removing this restriction entirely, we
> can do so.
No worries, we won't have to have the configs live in any particular
location in future revisions.
> > + relative_config_path = os.path.relpath(abs_config_path, ABS_TOOL_PATH)
> > + spec = importlib.util.spec_from_file_location('.' + relative_config_path, config_path)
> > + config = importlib.util.module_from_spec(spec)
> > + # TODO(brendanhiggins@google.com): I looked this up and apparently other
> > + # Python projects have noted that pytype complains that "No attribute
> > + # 'exec_module' on _importlib_modulespec._Loader". Disabling for now.
> > + spec.loader.exec_module(config) # pytype: disable=attribute-error
> > + return config.QEMU_ARCH.linux_arch, LinuxSourceTreeOperationsQemu(
> > + config.QEMU_ARCH, cross_compile=cross_compile)
> > +
> > class LinuxSourceTree(object):
> > """Represents a Linux kernel source tree with KUnit tests."""
> >
> > - def __init__(self, build_dir: str, load_config=True, kunitconfig_path='') -> None:
> > + def __init__(
> > + self,
> > + build_dir: str,
> > + load_config=True,
> > + kunitconfig_path='',
> > + arch=None,
> > + cross_compile=None,
> > + qemu_config_path=None) -> None:
> > signal.signal(signal.SIGINT, self.signal_handler)
> > -
> > - self._ops = LinuxSourceTreeOperations()
> > + if qemu_config_path:
> > + self._arch, self._ops = get_source_tree_ops_from_qemu_config(
> > + qemu_config_path, cross_compile)
> > + else:
> > + self._arch = 'um' if arch is None else arch
> > + self._ops = get_source_tree_ops(self._arch, cross_compile)
> >
> > if not load_config:
> > return
> > @@ -170,8 +284,9 @@ class LinuxSourceTree(object):
> > kconfig_path = get_kconfig_path(build_dir)
> > if build_dir and not os.path.exists(build_dir):
> > os.mkdir(build_dir)
> > - self._kconfig.write_to_file(kconfig_path)
> > try:
> > + self._ops.make_arch_qemuconfig(self._kconfig)
> > + self._kconfig.write_to_file(kconfig_path)
> > self._ops.make_olddefconfig(build_dir, make_options)
> > except ConfigError as e:
> > logging.error(e)
> > @@ -184,6 +299,7 @@ class LinuxSourceTree(object):
> > if os.path.exists(kconfig_path):
> > existing_kconfig = kunit_config.Kconfig()
> > existing_kconfig.read_from_file(kconfig_path)
> > + self._ops.make_arch_qemuconfig(self._kconfig)
> > if not self._kconfig.is_subset_of(existing_kconfig):
> > print('Regenerating .config ...')
> > os.remove(kconfig_path)
> > @@ -194,7 +310,7 @@ class LinuxSourceTree(object):
> > print('Generating .config ...')
> > return self.build_config(build_dir, make_options)
> >
> > - def build_um_kernel(self, alltests, jobs, build_dir, make_options) -> bool:
> > + def build_kernel(self, alltests, jobs, build_dir, make_options) -> bool:
> > try:
> > if alltests:
> > self._ops.make_allyesconfig(build_dir, make_options)
> > @@ -211,8 +327,8 @@ class LinuxSourceTree(object):
> > args.extend(['mem=1G', 'console=tty','kunit_shutdown=halt'])
> > if filter_glob:
> > args.append('kunit.filter_glob='+filter_glob)
> > - self._ops.linux_bin(args, timeout, build_dir)
> > outfile = get_outfile_path(build_dir)
> > + self._ops.run(args, timeout, build_dir, outfile)
> > subprocess.call(['stty', 'sane'])
> > with open(outfile, 'r') as file:
> > for line in file:
> > diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> > index 2e809dd956a77..a3b7f68e1cf9f 100755
> > --- a/tools/testing/kunit/kunit_tool_test.py
> > +++ b/tools/testing/kunit/kunit_tool_test.py
> > @@ -303,7 +303,7 @@ class KUnitMainTest(unittest.TestCase):
> >
> > self.linux_source_mock = mock.Mock()
> > self.linux_source_mock.build_reconfig = mock.Mock(return_value=True)
> > - self.linux_source_mock.build_um_kernel = mock.Mock(return_value=True)
> > + self.linux_source_mock.build_kernel = mock.Mock(return_value=True)
> > self.linux_source_mock.run_kernel = mock.Mock(return_value=all_passed_log)
> >
> > def test_config_passes_args_pass(self):
> > @@ -314,7 +314,7 @@ class KUnitMainTest(unittest.TestCase):
> > def test_build_passes_args_pass(self):
> > kunit.main(['build'], self.linux_source_mock)
> > self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 0)
> > - self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, '.kunit', None)
> > + self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, '.kunit', None)
> > self.assertEqual(self.linux_source_mock.run_kernel.call_count, 0)
> >
> > def test_exec_passes_args_pass(self):
> > @@ -396,7 +396,7 @@ class KUnitMainTest(unittest.TestCase):
> > def test_build_builddir(self):
> > build_dir = '.kunit'
> > kunit.main(['build', '--build_dir', build_dir], self.linux_source_mock)
> > - self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, build_dir, None)
> > + self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, build_dir, None)
> >
> > def test_exec_builddir(self):
> > build_dir = '.kunit'
> > @@ -410,14 +410,22 @@ class KUnitMainTest(unittest.TestCase):
> > mock_linux_init.return_value = self.linux_source_mock
> > kunit.main(['run', '--kunitconfig=mykunitconfig'])
> > # Just verify that we parsed and initialized it correctly here.
> > - mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig')
> > + mock_linux_init.assert_called_once_with('.kunit',
> > + kunitconfig_path='mykunitconfig',
> > + arch='um',
> > + cross_compile=None,
> > + qemu_config_path=None)
> >
> > @mock.patch.object(kunit_kernel, 'LinuxSourceTree')
> > def test_config_kunitconfig(self, mock_linux_init):
> > mock_linux_init.return_value = self.linux_source_mock
> > kunit.main(['config', '--kunitconfig=mykunitconfig'])
> > # Just verify that we parsed and initialized it correctly here.
> > - mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig')
> > + mock_linux_init.assert_called_once_with('.kunit',
> > + kunitconfig_path='mykunitconfig',
> > + arch='um',
> > + cross_compile=None,
> > + qemu_config_path=None)
> >
> > if __name__ == '__main__':
> > unittest.main()
> > diff --git a/tools/testing/kunit/qemu_config.py b/tools/testing/kunit/qemu_config.py
> > new file mode 100644
> > index 0000000000000..aff1fe0442dbc
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_config.py
> > @@ -0,0 +1,17 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Collection of configs for building non-UML kernels and running them on QEMU.
> > +#
> > +# Copyright (C) 2021, Google LLC.
> > +# Author: Brendan Higgins <brendanhiggins@google.com>
> > +
> > +from collections import namedtuple
> > +
> > +
> > +QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',
> > + 'qemuconfig',
> > + 'qemu_arch',
> > + 'kernel_path',
> > + 'kernel_command_line',
> > + 'extra_qemu_params'])
> > +
>
> Nit: newline at end of file.
Thanks, will fix.
> > diff --git a/tools/testing/kunit/qemu_configs/alpha.py b/tools/testing/kunit/qemu_configs/alpha.py
> > new file mode 100644
> > index 0000000000000..2cc64f848ca2c
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/alpha.py
> > @@ -0,0 +1,10 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='alpha',
> > + qemuconfig='''
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > + qemu_arch='alpha',
> > + kernel_path='arch/alpha/boot/vmlinux',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[''])
> > diff --git a/tools/testing/kunit/qemu_configs/arm.py b/tools/testing/kunit/qemu_configs/arm.py
> > new file mode 100644
> > index 0000000000000..29a043b0531a0
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/arm.py
> > @@ -0,0 +1,13 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='arm',
> > + qemuconfig='''
> > +CONFIG_ARCH_VIRT=y
> > +CONFIG_SERIAL_AMBA_PL010=y
> > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > +CONFIG_SERIAL_AMBA_PL011=y
> > +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
> > + qemu_arch='arm',
> > + kernel_path='arch/arm/boot/zImage',
> > + kernel_command_line='console=ttyAMA0',
> > + extra_qemu_params=['-machine virt'])
> > diff --git a/tools/testing/kunit/qemu_configs/arm64.py b/tools/testing/kunit/qemu_configs/arm64.py
> > new file mode 100644
> > index 0000000000000..1ba200bc99f0f
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/arm64.py
> > @@ -0,0 +1,12 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='arm64',
> > + qemuconfig='''
> > +CONFIG_SERIAL_AMBA_PL010=y
> > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > +CONFIG_SERIAL_AMBA_PL011=y
> > +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
> > + qemu_arch='aarch64',
> > + kernel_path='arch/arm64/boot/Image.gz',
> > + kernel_command_line='console=ttyAMA0',
> > + extra_qemu_params=['-machine virt', '-cpu cortex-a57'])
> > diff --git a/tools/testing/kunit/qemu_configs/i386.py b/tools/testing/kunit/qemu_configs/i386.py
> > new file mode 100644
> > index 0000000000000..3998af306468e
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/i386.py
> > @@ -0,0 +1,10 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='i386',
> > + qemuconfig='''
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > + qemu_arch='x86_64',
> > + kernel_path='arch/x86/boot/bzImage',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[''])
> > diff --git a/tools/testing/kunit/qemu_configs/powerpc.py b/tools/testing/kunit/qemu_configs/powerpc.py
> > new file mode 100644
> > index 0000000000000..46292ce9e368e
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/powerpc.py
> > @@ -0,0 +1,12 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='powerpc',
> > + qemuconfig='''
> > +CONFIG_PPC64=y
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y
> > +CONFIG_HVC_CONSOLE=y''',
> > + qemu_arch='ppc64',
> > + kernel_path='vmlinux',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=['-M pseries', '-cpu power8'])
> > diff --git a/tools/testing/kunit/qemu_configs/riscv.py b/tools/testing/kunit/qemu_configs/riscv.py
> > new file mode 100644
> > index 0000000000000..de8c62d465723
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/riscv.py
> > @@ -0,0 +1,31 @@
> > +from ..qemu_config import QemuArchParams
> > +import os
> > +import os.path
> > +import sys
> > +
> > +GITHUB_OPENSBI_URL = 'https://github.com/qemu/qemu/raw/master/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin'
> > +OPENSBI_FILE = os.path.basename(GITHUB_OPENSBI_URL)
> > +
> > +if not os.path.isfile(OPENSBI_FILE):
> > + print('\n\nOpenSBI file is not in the current working directory.\n'
> > + 'Would you like me to download it for you from:\n' + GITHUB_OPENSBI_URL + ' ?\n')
> > + response = input('yes/[no]: ')
> > + if response.strip() == 'yes':
> > + os.system('wget ' + GITHUB_OPENSBI_URL)
> > + else:
> > + sys.exit()
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='riscv',
> > + qemuconfig='''
> > +CONFIG_SOC_VIRT=y
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y
> > +CONFIG_SERIAL_OF_PLATFORM=y
> > +CONFIG_SERIAL_EARLYCON_RISCV_SBI=y''',
> > + qemu_arch='riscv64',
> > + kernel_path='arch/riscv/boot/Image',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[
> > + '-machine virt',
> > + '-cpu rv64',
> > + '-bios opensbi-riscv64-generic-fw_dynamic.bin'])
> > diff --git a/tools/testing/kunit/qemu_configs/s390.py b/tools/testing/kunit/qemu_configs/s390.py
> > new file mode 100644
> > index 0000000000000..04c90332f1098
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/s390.py
> > @@ -0,0 +1,14 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='s390',
> > + qemuconfig='''
> > +CONFIG_EXPERT=y
> > +CONFIG_TUNE_ZEC12=y
> > +CONFIG_NUMA=y
> > +CONFIG_MODULES=y''',
> > + qemu_arch='s390x',
> > + kernel_path='arch/s390/boot/bzImage',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[
> > + '-machine s390-ccw-virtio',
> > + '-cpu qemu',])
> > diff --git a/tools/testing/kunit/qemu_configs/sparc.py b/tools/testing/kunit/qemu_configs/sparc.py
> > new file mode 100644
> > index 0000000000000..f26b5f27cc5a1
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/sparc.py
> > @@ -0,0 +1,10 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='sparc',
> > + qemuconfig='''
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > + qemu_arch='sparc',
> > + kernel_path='arch/sparc/boot/zImage',
> > + kernel_command_line='console=ttyS0 mem=256M',
> > + extra_qemu_params=['-m 256'])
> > diff --git a/tools/testing/kunit/qemu_configs/x86_64.py b/tools/testing/kunit/qemu_configs/x86_64.py
> > new file mode 100644
> > index 0000000000000..bd5ab733b92ac
> > --- /dev/null
> > +++ b/tools/testing/kunit/qemu_configs/x86_64.py
> > @@ -0,0 +1,10 @@
> > +from ..qemu_config import QemuArchParams
> > +
> > +QEMU_ARCH = QemuArchParams(linux_arch='x86_64',
> > + qemuconfig='''
> > +CONFIG_SERIAL_8250=y
> > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > + qemu_arch='x86_64',
> > + kernel_path='arch/x86/boot/bzImage',
> > + kernel_command_line='console=ttyS0',
> > + extra_qemu_params=[''])
> > --
> > 2.31.1.607.g51e8a6a459-goog
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/4] kunit: tool: add support for QEMU
2021-05-18 3:01 ` David Gow
@ 2021-05-18 20:43 ` Brendan Higgins
2021-05-21 3:53 ` David Gow
0 siblings, 1 reply; 16+ messages in thread
From: Brendan Higgins @ 2021-05-18 20:43 UTC (permalink / raw)
To: David Gow
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
On Mon, May 17, 2021 at 8:01 PM David Gow <davidgow@google.com> wrote:
>
> On Sat, May 15, 2021 at 3:59 PM David Gow <davidgow@google.com> wrote:
> >
> > On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
> > <brendanhiggins@google.com> wrote:
> > >
> > > Add basic support to run QEMU via kunit_tool. Add support for i386,
> > > x86_64, arm, arm64, and a bunch more.
> > >
> > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > > Tested-by: David Gow <davidgow@google.com>
> > > ---
> > >
> > > Changes since last revision:
> > >
> > > - A number of minor obvious issues pointed out by David and Daniel.
> > > - Added facility for merging Kconfigs at Daniel's suggestion.
> > > - Broke out qemu_configs each into their own config file which is loaded
> > > dynamically - mostly at David's suggestion.
> > >
> > > ---
> >
> > This seems pretty good to me. I only have one real complaint --
> > qemu_configs needing to be in a subdirectory of ./tools/testing/kunit
> > -- but am able to tolerate that (even if I'd prefer not to have it) if
> > it's documented properly.
> >
> > Otherwise, save for a couple of minor nitpicks, this seems good to go.
> >
> > Reviewed-by: David Gow <davidgow@google.com>
> >
> >
>
> One thing I forgot to mention is that I'm not 100% sure about the
> Kconfig fragments being embedded in the qemu_configs. I still kind-of
> prefer the idea of them being in separate config files. While I don't
I don't feel strongly either way, but I don't have a good idea on how
to implement your idea well. How about we leave it for now, and if you
decide you really want to do something about it, you can do it?
> think this is necessarily a blocker, I did just realise that, by
> default, kunit.py run --arch=<non-UM-arch> will pull its default
> .kunitconfig from arch/um/configs/kunit_defconfig, which definitely
> feels awkward when UML is not otherwise involved.
Hmmm...this file is identical to
tools/testing/kunit/configs/all_tests.config. Maybe we should just use
that instead?
> Some further thoughts below (which range a bit from "practical
> suggestion" to "overcomplicated ponderings", so don't feel the
> pressure to take all of them).
>
> (...snip...)
>
> > > diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> > > index e22ade9d91ad5..2bd196fd69e5c 100644
> > > --- a/tools/testing/kunit/kunit_kernel.py
> > > +++ b/tools/testing/kunit/kunit_kernel.py
> > > @@ -6,23 +6,31 @@
> > > # Author: Felix Guo <felixguoxiuping@gmail.com>
> > > # Author: Brendan Higgins <brendanhiggins@google.com>
> > >
> > > +from __future__ import annotations
> > > +import importlib.util
> > > import logging
> > > import subprocess
> > > import os
> > > import shutil
> > > import signal
> > > from typing import Iterator
> > > +from typing import Optional
> > >
> > > from contextlib import ExitStack
> > >
> > > +from collections import namedtuple
> > > +
> > > import kunit_config
> > > import kunit_parser
> > > +import qemu_config
> > >
> > > KCONFIG_PATH = '.config'
> > > KUNITCONFIG_PATH = '.kunitconfig'
> > > DEFAULT_KUNITCONFIG_PATH = 'arch/um/configs/kunit_defconfig'
>
> This being in arch/um doesn't seem great if its being used for non-UML
> builds. Is it worth either:
> (a) moving this somewhere else (e.g., tools/testing/kunit/configs as
> with the BROKEN_ALLCONFIG_PATH beflow), or
How about we use: tools/testing/kunit/configs/all_tests.config ? The
file is identical.
> (b) giving each architecture its own kunit_defconfig, possibly in
> place of the qemuconfig member of QemuArchParams
>
> I'm leaning towards (b), which solves two different sources of
> ugliness in one go, though it would appear to have the downside that
> the default .kunitconfig could end up being architecture specific,
> which isn't great.
Yeah, I am not a fan of trying to solve that problem in this patchset.
This is sounding more and more like what should be follow-on work to
me.
> > > BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
> > > OUTFILE_PATH = 'test.log'
> > > +ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
> > > +QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
> > >
>
> (...snip...)
>
> > > diff --git a/tools/testing/kunit/qemu_config.py b/tools/testing/kunit/qemu_config.py
> > > new file mode 100644
> > > index 0000000000000..aff1fe0442dbc
> > > --- /dev/null
> > > +++ b/tools/testing/kunit/qemu_config.py
> > > @@ -0,0 +1,17 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +#
> > > +# Collection of configs for building non-UML kernels and running them on QEMU.
> > > +#
> > > +# Copyright (C) 2021, Google LLC.
> > > +# Author: Brendan Higgins <brendanhiggins@google.com>
> > > +
> > > +from collections import namedtuple
> > > +
> > > +
> > > +QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',
> > > + 'qemuconfig',
>
> As mentioned, I'm not thrilled about keeping the Kconfig inline here,
> and would kind-of prefer it to be in another file. I could live with
> it if I have to, though. Regardless, 'qemuconfig' is not a
It will be fixed in the next revision.
> super-descriptive name, particularly as it's not clear if this is
> configuring QEMU (no, that's extra_qemu_params'), or configuring the
> kernel for QEMU compatibility.
Any suggestions on a better name? qemu_build_config_path? These
configs contain configs for configuring, building, and running kernels
on QEMU.
> > > + 'qemu_arch',
> > > + 'kernel_path',
> > > + 'kernel_command_line',
> > > + 'extra_qemu_params'])
> > > +
> >
> > Nit: newline at end of file.
> >
> >
> >
> > > diff --git a/tools/testing/kunit/qemu_configs/alpha.py b/tools/testing/kunit/qemu_configs/alpha.py
> > > new file mode 100644
> > > index 0000000000000..2cc64f848ca2c
> > > --- /dev/null
> > > +++ b/tools/testing/kunit/qemu_configs/alpha.py
> > > @@ -0,0 +1,10 @@
> > > +from ..qemu_config import QemuArchParams
> > > +
> > > +QEMU_ARCH = QemuArchParams(linux_arch='alpha',
> > > + qemuconfig='''
> > > +CONFIG_SERIAL_8250=y
> > > +CONFIG_SERIAL_8250_CONSOLE=y''',
>
> If these were in a separate file, they could be shared across alpha,
> i386, x86_64, etc. Of course, that wouldn't gel well with putting them
> in arch/.../config. If there were some way of listing multiple files,
> it could form part of the config for several more architectures,
> though that's probably overcomplicating things.
Yeah, like I said, I have sympathy for what you are saying here, but
it really feels like something that can and should be addressed in
follow on patches. We could totally address this issue later by
expanding this field to take either a string containing a Kconfig, or
a path to an external Kconfig; if we do so, it won't cause any
migration issues in the future.
> > > + qemu_arch='alpha',
> > > + kernel_path='arch/alpha/boot/vmlinux',
> > > + kernel_command_line='console=ttyS0',
> > > + extra_qemu_params=[''])
> > > diff --git a/tools/testing/kunit/qemu_configs/arm.py b/tools/testing/kunit/qemu_configs/arm.py
> > > new file mode 100644
> > > index 0000000000000..29a043b0531a0
> > > --- /dev/null
> > > +++ b/tools/testing/kunit/qemu_configs/arm.py
> > > @@ -0,0 +1,13 @@
> > > +from ..qemu_config import QemuArchParams
> > > +
> > > +QEMU_ARCH = QemuArchParams(linux_arch='arm',
> > > + qemuconfig='''
> > > +CONFIG_ARCH_VIRT=y
> > > +CONFIG_SERIAL_AMBA_PL010=y
> > > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > > +CONFIG_SERIAL_AMBA_PL011=y
> > > +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
>
> Similarly, if in a separate file and there were some multiple-file
> mechanism, these could mostly be shared between arm & arm64 (ARCH_VIRT
> being the only problem). Again, probably overcomplicating it at this
> point though.
Right.
> > > + qemu_arch='arm',
> > > + kernel_path='arch/arm/boot/zImage',
> > > + kernel_command_line='console=ttyAMA0',
> > > + extra_qemu_params=['-machine virt'])
> > > diff --git a/tools/testing/kunit/qemu_configs/arm64.py b/tools/testing/kunit/qemu_configs/arm64.py
> > > new file mode 100644
> > > index 0000000000000..1ba200bc99f0f
> > > --- /dev/null
> > > +++ b/tools/testing/kunit/qemu_configs/arm64.py
> > > @@ -0,0 +1,12 @@
> > > +from ..qemu_config import QemuArchParams
> > > +
> > > +QEMU_ARCH = QemuArchParams(linux_arch='arm64',
> > > + qemuconfig='''
> > > +CONFIG_SERIAL_AMBA_PL010=y
> > > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > > +CONFIG_SERIAL_AMBA_PL011=y
> > > +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
> > > + qemu_arch='aarch64',
> > > + kernel_path='arch/arm64/boot/Image.gz',
> > > + kernel_command_line='console=ttyAMA0',
> > > + extra_qemu_params=['-machine virt', '-cpu cortex-a57'])
> > > diff --git a/tools/testing/kunit/qemu_configs/i386.py b/tools/testing/kunit/qemu_configs/i386.py
> > > new file mode 100644
> > > index 0000000000000..3998af306468e
> > > --- /dev/null
> > > +++ b/tools/testing/kunit/qemu_configs/i386.py
> > > @@ -0,0 +1,10 @@
> > > +from ..qemu_config import QemuArchParams
> > > +
> > > +QEMU_ARCH = QemuArchParams(linux_arch='i386',
> > > + qemuconfig='''
> > > +CONFIG_SERIAL_8250=y
> > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > > + qemu_arch='x86_64',
> > > + kernel_path='arch/x86/boot/bzImage',
> > > + kernel_command_line='console=ttyS0',
> > > + extra_qemu_params=[''])
> > > diff --git a/tools/testing/kunit/qemu_configs/powerpc.py b/tools/testing/kunit/qemu_configs/powerpc.py
> > > new file mode 100644
> > > index 0000000000000..46292ce9e368e
> > > --- /dev/null
> > > +++ b/tools/testing/kunit/qemu_configs/powerpc.py
> > > @@ -0,0 +1,12 @@
> > > +from ..qemu_config import QemuArchParams
> > > +
> > > +QEMU_ARCH = QemuArchParams(linux_arch='powerpc',
> > > + qemuconfig='''
> > > +CONFIG_PPC64=y
> > > +CONFIG_SERIAL_8250=y
> > > +CONFIG_SERIAL_8250_CONSOLE=y
> > > +CONFIG_HVC_CONSOLE=y''',
> > > + qemu_arch='ppc64',
> > > + kernel_path='vmlinux',
> > > + kernel_command_line='console=ttyS0',
> > > + extra_qemu_params=['-M pseries', '-cpu power8'])
> > > diff --git a/tools/testing/kunit/qemu_configs/riscv.py b/tools/testing/kunit/qemu_configs/riscv.py
> > > new file mode 100644
> > > index 0000000000000..de8c62d465723
> > > --- /dev/null
> > > +++ b/tools/testing/kunit/qemu_configs/riscv.py
> > > @@ -0,0 +1,31 @@
> > > +from ..qemu_config import QemuArchParams
> > > +import os
> > > +import os.path
> > > +import sys
> > > +
> > > +GITHUB_OPENSBI_URL = 'https://github.com/qemu/qemu/raw/master/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin'
> > > +OPENSBI_FILE = os.path.basename(GITHUB_OPENSBI_URL)
> > > +
> > > +if not os.path.isfile(OPENSBI_FILE):
> > > + print('\n\nOpenSBI file is not in the current working directory.\n'
> > > + 'Would you like me to download it for you from:\n' + GITHUB_OPENSBI_URL + ' ?\n')
> > > + response = input('yes/[no]: ')
> > > + if response.strip() == 'yes':
> > > + os.system('wget ' + GITHUB_OPENSBI_URL)
> > > + else:
> > > + sys.exit()
> > > +
> > > +QEMU_ARCH = QemuArchParams(linux_arch='riscv',
> > > + qemuconfig='''
> > > +CONFIG_SOC_VIRT=y
> > > +CONFIG_SERIAL_8250=y
> > > +CONFIG_SERIAL_8250_CONSOLE=y
> > > +CONFIG_SERIAL_OF_PLATFORM=y
> > > +CONFIG_SERIAL_EARLYCON_RISCV_SBI=y''',
> > > + qemu_arch='riscv64',
> > > + kernel_path='arch/riscv/boot/Image',
> > > + kernel_command_line='console=ttyS0',
> > > + extra_qemu_params=[
> > > + '-machine virt',
> > > + '-cpu rv64',
> > > + '-bios opensbi-riscv64-generic-fw_dynamic.bin'])
> > > diff --git a/tools/testing/kunit/qemu_configs/s390.py b/tools/testing/kunit/qemu_configs/s390.py
> > > new file mode 100644
> > > index 0000000000000..04c90332f1098
> > > --- /dev/null
> > > +++ b/tools/testing/kunit/qemu_configs/s390.py
> > > @@ -0,0 +1,14 @@
> > > +from ..qemu_config import QemuArchParams
> > > +
> > > +QEMU_ARCH = QemuArchParams(linux_arch='s390',
> > > + qemuconfig='''
> > > +CONFIG_EXPERT=y
> > > +CONFIG_TUNE_ZEC12=y
> > > +CONFIG_NUMA=y
> > > +CONFIG_MODULES=y''',
> > > + qemu_arch='s390x',
> > > + kernel_path='arch/s390/boot/bzImage',
> > > + kernel_command_line='console=ttyS0',
> > > + extra_qemu_params=[
> > > + '-machine s390-ccw-virtio',
> > > + '-cpu qemu',])
> > > diff --git a/tools/testing/kunit/qemu_configs/sparc.py b/tools/testing/kunit/qemu_configs/sparc.py
> > > new file mode 100644
> > > index 0000000000000..f26b5f27cc5a1
> > > --- /dev/null
> > > +++ b/tools/testing/kunit/qemu_configs/sparc.py
> > > @@ -0,0 +1,10 @@
> > > +from ..qemu_config import QemuArchParams
> > > +
> > > +QEMU_ARCH = QemuArchParams(linux_arch='sparc',
> > > + qemuconfig='''
> > > +CONFIG_SERIAL_8250=y
> > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > > + qemu_arch='sparc',
> > > + kernel_path='arch/sparc/boot/zImage',
> > > + kernel_command_line='console=ttyS0 mem=256M',
> > > + extra_qemu_params=['-m 256'])
> > > diff --git a/tools/testing/kunit/qemu_configs/x86_64.py b/tools/testing/kunit/qemu_configs/x86_64.py
> > > new file mode 100644
> > > index 0000000000000..bd5ab733b92ac
> > > --- /dev/null
> > > +++ b/tools/testing/kunit/qemu_configs/x86_64.py
> > > @@ -0,0 +1,10 @@
> > > +from ..qemu_config import QemuArchParams
> > > +
> > > +QEMU_ARCH = QemuArchParams(linux_arch='x86_64',
> > > + qemuconfig='''
> > > +CONFIG_SERIAL_8250=y
> > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > > + qemu_arch='x86_64',
> > > + kernel_path='arch/x86/boot/bzImage',
> > > + kernel_command_line='console=ttyS0',
> > > + extra_qemu_params=[''])
> > > --
> > > 2.31.1.607.g51e8a6a459-goog
> > >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 1/4] kunit: Add 'kunit_shutdown' option
2021-05-15 7:58 ` David Gow
@ 2021-05-18 20:52 ` Brendan Higgins
0 siblings, 0 replies; 16+ messages in thread
From: Brendan Higgins @ 2021-05-18 20:52 UTC (permalink / raw)
To: David Gow
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
On Sat, May 15, 2021 at 12:58 AM David Gow <davidgow@google.com> wrote:
>
> On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
> <brendanhiggins@google.com> wrote:
> >
> > From: David Gow <davidgow@google.com>
> >
> > Add a new kernel command-line option, 'kunit_shutdown', which allows the
> > user to specify that the kernel poweroff, halt, or reboot after
> > completing all KUnit tests; this is very handy for running KUnit tests
> > on UML or a VM so that the UML/VM process exits cleanly immediately
> > after running all tests without needing a special initramfs.
> >
> > Signed-off-by: David Gow <davidgow@google.com>
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> > Tested-By: Daniel Latypov <dlatypov@google.com>
> > ---
>
> Obviously I'm okay with this change, but I did find a minor whitespace
> nitpick below.
>
> > lib/kunit/executor.c | 20 ++++++++++++++++++++
> > tools/testing/kunit/kunit_kernel.py | 2 +-
> > tools/testing/kunit/kunit_parser.py | 2 +-
> > 3 files changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> > index 15832ed446685..7db619624437c 100644
> > --- a/lib/kunit/executor.c
> > +++ b/lib/kunit/executor.c
> > @@ -1,5 +1,6 @@
> > // SPDX-License-Identifier: GPL-2.0
> >
> > +#include <linux/reboot.h>
> > #include <kunit/test.h>
> > #include <linux/glob.h>
> > #include <linux/moduleparam.h>
> > @@ -18,6 +19,9 @@ module_param(filter_glob, charp, 0);
> > MODULE_PARM_DESC(filter_glob,
> > "Filter which KUnit test suites run at boot-time, e.g. list*");
> >
> > +static char *kunit_shutdown;
> > +core_param(kunit_shutdown, kunit_shutdown, charp, 0644);
> > +
> > static struct kunit_suite * const *
> > kunit_filter_subsuite(struct kunit_suite * const * const subsuite)
> > {
> > @@ -82,6 +86,20 @@ static struct suite_set kunit_filter_suites(void)
> > return filtered;
> > }
> >
> > +static void kunit_handle_shutdown(void)
> > +{
> > + if (!kunit_shutdown)
> > + return;
> > +
> > + if (!strcmp(kunit_shutdown, "poweroff"))
> > + kernel_power_off();
> > + else if (!strcmp(kunit_shutdown, "halt"))
> > + kernel_halt();
> > + else if (!strcmp(kunit_shutdown, "reboot"))
> > + kernel_restart(NULL);
> > +
> > +}
> > +
> > static void kunit_print_tap_header(struct suite_set *suite_set)
> > {
> > struct kunit_suite * const * const *suites, * const *subsuite;
> > @@ -112,6 +130,8 @@ int kunit_run_all_tests(void)
> > kfree(suite_set.start);
> > }
> >
> > + kunit_handle_shutdown();
> > +
> > return 0;
> > }
> >
> > diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> > index 89a7d4024e878..e22ade9d91ad5 100644
> > --- a/tools/testing/kunit/kunit_kernel.py
> > +++ b/tools/testing/kunit/kunit_kernel.py
> > @@ -208,7 +208,7 @@ class LinuxSourceTree(object):
> > def run_kernel(self, args=None, build_dir='', filter_glob='', timeout=None) -> Iterator[str]:
> > if not args:
> > args = []
> > - args.extend(['mem=1G', 'console=tty'])
> > + args.extend(['mem=1G', 'console=tty','kunit_shutdown=halt'])
> Nit: space here between 'console=tty', and 'kunit_shutdown=halt'.
Whoops, I probably introduced it after addressing a rebase conflict.
Thanks, will fix.
> > if filter_glob:
> > args.append('kunit.filter_glob='+filter_glob)
> > self._ops.linux_bin(args, timeout, build_dir)
> > diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
> > index e8bcc139702e2..8d8d4d70b39dd 100644
> > --- a/tools/testing/kunit/kunit_parser.py
> > +++ b/tools/testing/kunit/kunit_parser.py
> > @@ -49,7 +49,7 @@ class TestStatus(Enum):
> >
> > kunit_start_re = re.compile(r'TAP version [0-9]+$')
> > kunit_end_re = re.compile('(List of all partitions:|'
> > - 'Kernel panic - not syncing: VFS:)')
> > + 'Kernel panic - not syncing: VFS:|reboot: System halted)')
> >
> > def isolate_kunit_output(kernel_output) -> Iterator[str]:
> > started = False
> > --
> > 2.31.1.607.g51e8a6a459-goog
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 4/4] Documentation: kunit: document support for QEMU in kunit_tool
2021-05-15 8:01 ` David Gow
@ 2021-05-18 21:08 ` Brendan Higgins
0 siblings, 0 replies; 16+ messages in thread
From: Brendan Higgins @ 2021-05-18 21:08 UTC (permalink / raw)
To: David Gow
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
On Sat, May 15, 2021 at 1:01 AM David Gow <davidgow@google.com> wrote:
>
> On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
> <brendanhiggins@google.com> wrote:
> >
> > Document QEMU support, what it does, and how to use it in kunit_tool.
> >
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > ---
>
> This is a good start, and probably meets the minimum requirements, but
> I do have a number of comments and suggestions below.
>
> Cheers,
> -- David
>
> > Documentation/dev-tools/kunit/usage.rst | 37 +++++++++++++++++++++----
> > 1 file changed, 31 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
> > index 650f99590df57..b74bd7c87cc20 100644
> > --- a/Documentation/dev-tools/kunit/usage.rst
> > +++ b/Documentation/dev-tools/kunit/usage.rst
> > @@ -612,14 +612,39 @@ only things to be aware of when doing so.
> > The biggest impediment will likely be that certain KUnit features and
> > infrastructure may not support your target environment. For example, at this
> > time the KUnit Wrapper (``tools/testing/kunit/kunit.py``) does not work outside
> > -of UML. Unfortunately, there is no way around this. Using UML (or even just a
> > -particular architecture) allows us to make a lot of assumptions that make it
> > -possible to do things which might otherwise be impossible.
> > +of UML and QEMU. Unfortunately, there is no way around this. Using UML and QEMU
> > +(or even just a particular architecture) allows us to make a lot of assumptions
> > +that make it possible to do things which might otherwise be impossible.
>
> This is a bit more awkward now, and I don't think gives quite the
> right impression. Particularly the "Unfortunately, there is no way
> around this." bit: there's no fundamental reason that someone couldn't
> implement support for some other emulator (or even a setup which
> pushed to real hardware and read results over serial), it'd just take
> a bit of work to implement (like this patch series has done for qemu).
>
> Personally, I think it'd be easiest to simplify this section and say
> that kunit_tool currently only fully supports some architectures, via
> UML and QEMU.
Cool, I will fix that in the next revision.
> >
> > Nevertheless, all core KUnit framework features are fully supported on all
> > -architectures, and using them is straightforward: all you need to do is to take
> > -your kunitconfig, your Kconfig options for the tests you would like to run, and
> > -merge them into whatever config your are using for your platform. That's it!
> > +architectures, and using them is straightforward: Most popular architectures
> > +are supported directly in the KUnit Wrapper via QEMU. Currently, supported
> > +architectures on QEMU include:
> > +
> > +* i386
> > +* x86_64
> > +* arm
> > +* arm64
> > +* alpha
> > +* powerpc
> > +* riscv
> > +* s390
> > +* sparc
> > +
> > +In order to run KUnit tests on one of these architectures via QEMU with the
> > +KUnit wrapper, all you need to do is specify the flags ``--arch`` and
> > +``--cross_compile`` when invoking the KUnit Wrapper. For example, we could run
> > +the default KUnit tests on ARM in the following manner (assuming we have an ARM
> > +toolchain installed):
> > +
> > +.. code-block:: bash
> > +
> > + tools/testing/kunit/kunit.py run --timeout=60 --jobs=12 --arch=arm --cross_compile=arm-linux-gnueabihf-
> > +
>
> Is it worth also documenting the --qemu_config option here?
> (Particularly given the restriction on its path?) Or is that something
> best added to the kunit_tool page?
>
> That being said, changes to the kunit_tool page are probably worth
> adding as a section in the updated page:
> https://patchwork.kernel.org/project/linux-kselftest/patch/20210417034553.1048895-1-davidgow@google.com/
>
> At the very least, it'd be nice to have the new QEMU-related options
> documented there.
Probably a good idea. However, I will ask Shuah to pick up the
Documentation changes before I document the new options to avoid
conflicts.
> > +Alternatively, if you want to run your tests on real hardware or in some other
> > +emulation environment, all you need to do is to take your kunitconfig, your
> > +Kconfig options for the tests you would like to run, and merge them into
> > +whatever config your are using for your platform. That's it!
> >
> > For example, let's say you have the following kunitconfig:
> >
> > --
> > 2.31.1.607.g51e8a6a459-goog
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/4] kunit: tool: add support for QEMU
2021-05-18 20:43 ` Brendan Higgins
@ 2021-05-21 3:53 ` David Gow
2021-05-21 20:57 ` Brendan Higgins
0 siblings, 1 reply; 16+ messages in thread
From: David Gow @ 2021-05-21 3:53 UTC (permalink / raw)
To: Brendan Higgins
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
On Wed, May 19, 2021 at 4:43 AM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> On Mon, May 17, 2021 at 8:01 PM David Gow <davidgow@google.com> wrote:
> >
> > On Sat, May 15, 2021 at 3:59 PM David Gow <davidgow@google.com> wrote:
> > >
> > > On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
> > > <brendanhiggins@google.com> wrote:
> > > >
> > > > Add basic support to run QEMU via kunit_tool. Add support for i386,
> > > > x86_64, arm, arm64, and a bunch more.
> > > >
> > > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > > > Tested-by: David Gow <davidgow@google.com>
> > > > ---
> > > >
> > > > Changes since last revision:
> > > >
> > > > - A number of minor obvious issues pointed out by David and Daniel.
> > > > - Added facility for merging Kconfigs at Daniel's suggestion.
> > > > - Broke out qemu_configs each into their own config file which is loaded
> > > > dynamically - mostly at David's suggestion.
> > > >
> > > > ---
> > >
> > > This seems pretty good to me. I only have one real complaint --
> > > qemu_configs needing to be in a subdirectory of ./tools/testing/kunit
> > > -- but am able to tolerate that (even if I'd prefer not to have it) if
> > > it's documented properly.
> > >
> > > Otherwise, save for a couple of minor nitpicks, this seems good to go.
> > >
> > > Reviewed-by: David Gow <davidgow@google.com>
> > >
> > >
> >
> > One thing I forgot to mention is that I'm not 100% sure about the
> > Kconfig fragments being embedded in the qemu_configs. I still kind-of
> > prefer the idea of them being in separate config files. While I don't
>
> I don't feel strongly either way, but I don't have a good idea on how
> to implement your idea well. How about we leave it for now, and if you
> decide you really want to do something about it, you can do it?
>
> > think this is necessarily a blocker, I did just realise that, by
> > default, kunit.py run --arch=<non-UM-arch> will pull its default
> > .kunitconfig from arch/um/configs/kunit_defconfig, which definitely
> > feels awkward when UML is not otherwise involved.
>
> Hmmm...this file is identical to
> tools/testing/kunit/configs/all_tests.config. Maybe we should just use
> that instead?
>
That sounds like a better plan. It looks like all_tests.config isn't
used anywhere, anyway. I might rename it and replace the
arch/um/.../kunit_defconfig version in another patch, then.
> > Some further thoughts below (which range a bit from "practical
> > suggestion" to "overcomplicated ponderings", so don't feel the
> > pressure to take all of them).
> >
> > (...snip...)
> >
> > > > diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> > > > index e22ade9d91ad5..2bd196fd69e5c 100644
> > > > --- a/tools/testing/kunit/kunit_kernel.py
> > > > +++ b/tools/testing/kunit/kunit_kernel.py
> > > > @@ -6,23 +6,31 @@
> > > > # Author: Felix Guo <felixguoxiuping@gmail.com>
> > > > # Author: Brendan Higgins <brendanhiggins@google.com>
> > > >
> > > > +from __future__ import annotations
> > > > +import importlib.util
> > > > import logging
> > > > import subprocess
> > > > import os
> > > > import shutil
> > > > import signal
> > > > from typing import Iterator
> > > > +from typing import Optional
> > > >
> > > > from contextlib import ExitStack
> > > >
> > > > +from collections import namedtuple
> > > > +
> > > > import kunit_config
> > > > import kunit_parser
> > > > +import qemu_config
> > > >
> > > > KCONFIG_PATH = '.config'
> > > > KUNITCONFIG_PATH = '.kunitconfig'
> > > > DEFAULT_KUNITCONFIG_PATH = 'arch/um/configs/kunit_defconfig'
> >
> > This being in arch/um doesn't seem great if its being used for non-UML
> > builds. Is it worth either:
> > (a) moving this somewhere else (e.g., tools/testing/kunit/configs as
> > with the BROKEN_ALLCONFIG_PATH beflow), or
>
> How about we use: tools/testing/kunit/configs/all_tests.config ? The
> file is identical.
Yeah: I'm not thrilled with the name all_tests.config, but since it
doesn't appear to be being used anywhere, I might just rename it in
another patch.
> > (b) giving each architecture its own kunit_defconfig, possibly in
> > place of the qemuconfig member of QemuArchParams
> >
> > I'm leaning towards (b), which solves two different sources of
> > ugliness in one go, though it would appear to have the downside that
> > the default .kunitconfig could end up being architecture specific,
> > which isn't great.
>
> Yeah, I am not a fan of trying to solve that problem in this patchset.
> This is sounding more and more like what should be follow-on work to
> me.
Yeah, I'm not sure exactly what that should look like yet, anyway.
Let's keep things as they are in this patch. I'll put a follow-up
patch to use all_tests.config rather than the arch/um one (possibly as
part of my "default to ALL_TESTS" patchset), and if we think of
something better that is more architecture specific, we'll do that.
> > > > BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
> > > > OUTFILE_PATH = 'test.log'
> > > > +ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
> > > > +QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
> > > >
> >
> > (...snip...)
> >
> > > > diff --git a/tools/testing/kunit/qemu_config.py b/tools/testing/kunit/qemu_config.py
> > > > new file mode 100644
> > > > index 0000000000000..aff1fe0442dbc
> > > > --- /dev/null
> > > > +++ b/tools/testing/kunit/qemu_config.py
> > > > @@ -0,0 +1,17 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +#
> > > > +# Collection of configs for building non-UML kernels and running them on QEMU.
> > > > +#
> > > > +# Copyright (C) 2021, Google LLC.
> > > > +# Author: Brendan Higgins <brendanhiggins@google.com>
> > > > +
> > > > +from collections import namedtuple
> > > > +
> > > > +
> > > > +QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',
> > > > + 'qemuconfig',
> >
> > As mentioned, I'm not thrilled about keeping the Kconfig inline here,
> > and would kind-of prefer it to be in another file. I could live with
> > it if I have to, though. Regardless, 'qemuconfig' is not a
>
> It will be fixed in the next revision.
>
> > super-descriptive name, particularly as it's not clear if this is
> > configuring QEMU (no, that's extra_qemu_params'), or configuring the
> > kernel for QEMU compatibility.
>
> Any suggestions on a better name? qemu_build_config_path? These
> configs contain configs for configuring, building, and running kernels
> on QEMU.
I don't think we need "qemu" in the name, as this is already part of
the QemuArchParams struct, and isn't a qemu config, but a kernel one.
Something along the lines of "kernel_config" (or just "kconfig") maybe?
> > > > + 'qemu_arch',
> > > > + 'kernel_path',
> > > > + 'kernel_command_line',
> > > > + 'extra_qemu_params'])
> > > > +
> > >
> > > Nit: newline at end of file.
> > >
> > >
> > >
> > > > diff --git a/tools/testing/kunit/qemu_configs/alpha.py b/tools/testing/kunit/qemu_configs/alpha.py
> > > > new file mode 100644
> > > > index 0000000000000..2cc64f848ca2c
> > > > --- /dev/null
> > > > +++ b/tools/testing/kunit/qemu_configs/alpha.py
> > > > @@ -0,0 +1,10 @@
> > > > +from ..qemu_config import QemuArchParams
> > > > +
> > > > +QEMU_ARCH = QemuArchParams(linux_arch='alpha',
> > > > + qemuconfig='''
> > > > +CONFIG_SERIAL_8250=y
> > > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> >
> > If these were in a separate file, they could be shared across alpha,
> > i386, x86_64, etc. Of course, that wouldn't gel well with putting them
> > in arch/.../config. If there were some way of listing multiple files,
> > it could form part of the config for several more architectures,
> > though that's probably overcomplicating things.
>
> Yeah, like I said, I have sympathy for what you are saying here, but
> it really feels like something that can and should be addressed in
> follow on patches. We could totally address this issue later by
> expanding this field to take either a string containing a Kconfig, or
> a path to an external Kconfig; if we do so, it won't cause any
> migration issues in the future.
>
Yeah: I think we can solve this if it actually becomes a problem. No
need to change anything here.
> > > > + qemu_arch='alpha',
> > > > + kernel_path='arch/alpha/boot/vmlinux',
> > > > + kernel_command_line='console=ttyS0',
> > > > + extra_qemu_params=[''])
> > > > diff --git a/tools/testing/kunit/qemu_configs/arm.py b/tools/testing/kunit/qemu_configs/arm.py
> > > > new file mode 100644
> > > > index 0000000000000..29a043b0531a0
> > > > --- /dev/null
> > > > +++ b/tools/testing/kunit/qemu_configs/arm.py
> > > > @@ -0,0 +1,13 @@
> > > > +from ..qemu_config import QemuArchParams
> > > > +
> > > > +QEMU_ARCH = QemuArchParams(linux_arch='arm',
> > > > + qemuconfig='''
> > > > +CONFIG_ARCH_VIRT=y
> > > > +CONFIG_SERIAL_AMBA_PL010=y
> > > > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > > > +CONFIG_SERIAL_AMBA_PL011=y
> > > > +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
> >
> > Similarly, if in a separate file and there were some multiple-file
> > mechanism, these could mostly be shared between arm & arm64 (ARCH_VIRT
> > being the only problem). Again, probably overcomplicating it at this
> > point though.
>
> Right.
>
> > > > + qemu_arch='arm',
> > > > + kernel_path='arch/arm/boot/zImage',
> > > > + kernel_command_line='console=ttyAMA0',
> > > > + extra_qemu_params=['-machine virt'])
> > > > diff --git a/tools/testing/kunit/qemu_configs/arm64.py b/tools/testing/kunit/qemu_configs/arm64.py
> > > > new file mode 100644
> > > > index 0000000000000..1ba200bc99f0f
> > > > --- /dev/null
> > > > +++ b/tools/testing/kunit/qemu_configs/arm64.py
> > > > @@ -0,0 +1,12 @@
> > > > +from ..qemu_config import QemuArchParams
> > > > +
> > > > +QEMU_ARCH = QemuArchParams(linux_arch='arm64',
> > > > + qemuconfig='''
> > > > +CONFIG_SERIAL_AMBA_PL010=y
> > > > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > > > +CONFIG_SERIAL_AMBA_PL011=y
> > > > +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
> > > > + qemu_arch='aarch64',
> > > > + kernel_path='arch/arm64/boot/Image.gz',
> > > > + kernel_command_line='console=ttyAMA0',
> > > > + extra_qemu_params=['-machine virt', '-cpu cortex-a57'])
> > > > diff --git a/tools/testing/kunit/qemu_configs/i386.py b/tools/testing/kunit/qemu_configs/i386.py
> > > > new file mode 100644
> > > > index 0000000000000..3998af306468e
> > > > --- /dev/null
> > > > +++ b/tools/testing/kunit/qemu_configs/i386.py
> > > > @@ -0,0 +1,10 @@
> > > > +from ..qemu_config import QemuArchParams
> > > > +
> > > > +QEMU_ARCH = QemuArchParams(linux_arch='i386',
> > > > + qemuconfig='''
> > > > +CONFIG_SERIAL_8250=y
> > > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > > > + qemu_arch='x86_64',
> > > > + kernel_path='arch/x86/boot/bzImage',
> > > > + kernel_command_line='console=ttyS0',
> > > > + extra_qemu_params=[''])
> > > > diff --git a/tools/testing/kunit/qemu_configs/powerpc.py b/tools/testing/kunit/qemu_configs/powerpc.py
> > > > new file mode 100644
> > > > index 0000000000000..46292ce9e368e
> > > > --- /dev/null
> > > > +++ b/tools/testing/kunit/qemu_configs/powerpc.py
> > > > @@ -0,0 +1,12 @@
> > > > +from ..qemu_config import QemuArchParams
> > > > +
> > > > +QEMU_ARCH = QemuArchParams(linux_arch='powerpc',
> > > > + qemuconfig='''
> > > > +CONFIG_PPC64=y
> > > > +CONFIG_SERIAL_8250=y
> > > > +CONFIG_SERIAL_8250_CONSOLE=y
> > > > +CONFIG_HVC_CONSOLE=y''',
> > > > + qemu_arch='ppc64',
> > > > + kernel_path='vmlinux',
> > > > + kernel_command_line='console=ttyS0',
> > > > + extra_qemu_params=['-M pseries', '-cpu power8'])
> > > > diff --git a/tools/testing/kunit/qemu_configs/riscv.py b/tools/testing/kunit/qemu_configs/riscv.py
> > > > new file mode 100644
> > > > index 0000000000000..de8c62d465723
> > > > --- /dev/null
> > > > +++ b/tools/testing/kunit/qemu_configs/riscv.py
> > > > @@ -0,0 +1,31 @@
> > > > +from ..qemu_config import QemuArchParams
> > > > +import os
> > > > +import os.path
> > > > +import sys
> > > > +
> > > > +GITHUB_OPENSBI_URL = 'https://github.com/qemu/qemu/raw/master/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin'
> > > > +OPENSBI_FILE = os.path.basename(GITHUB_OPENSBI_URL)
> > > > +
> > > > +if not os.path.isfile(OPENSBI_FILE):
> > > > + print('\n\nOpenSBI file is not in the current working directory.\n'
> > > > + 'Would you like me to download it for you from:\n' + GITHUB_OPENSBI_URL + ' ?\n')
> > > > + response = input('yes/[no]: ')
> > > > + if response.strip() == 'yes':
> > > > + os.system('wget ' + GITHUB_OPENSBI_URL)
> > > > + else:
> > > > + sys.exit()
> > > > +
> > > > +QEMU_ARCH = QemuArchParams(linux_arch='riscv',
> > > > + qemuconfig='''
> > > > +CONFIG_SOC_VIRT=y
> > > > +CONFIG_SERIAL_8250=y
> > > > +CONFIG_SERIAL_8250_CONSOLE=y
> > > > +CONFIG_SERIAL_OF_PLATFORM=y
> > > > +CONFIG_SERIAL_EARLYCON_RISCV_SBI=y''',
> > > > + qemu_arch='riscv64',
> > > > + kernel_path='arch/riscv/boot/Image',
> > > > + kernel_command_line='console=ttyS0',
> > > > + extra_qemu_params=[
> > > > + '-machine virt',
> > > > + '-cpu rv64',
> > > > + '-bios opensbi-riscv64-generic-fw_dynamic.bin'])
> > > > diff --git a/tools/testing/kunit/qemu_configs/s390.py b/tools/testing/kunit/qemu_configs/s390.py
> > > > new file mode 100644
> > > > index 0000000000000..04c90332f1098
> > > > --- /dev/null
> > > > +++ b/tools/testing/kunit/qemu_configs/s390.py
> > > > @@ -0,0 +1,14 @@
> > > > +from ..qemu_config import QemuArchParams
> > > > +
> > > > +QEMU_ARCH = QemuArchParams(linux_arch='s390',
> > > > + qemuconfig='''
> > > > +CONFIG_EXPERT=y
> > > > +CONFIG_TUNE_ZEC12=y
> > > > +CONFIG_NUMA=y
> > > > +CONFIG_MODULES=y''',
> > > > + qemu_arch='s390x',
> > > > + kernel_path='arch/s390/boot/bzImage',
> > > > + kernel_command_line='console=ttyS0',
> > > > + extra_qemu_params=[
> > > > + '-machine s390-ccw-virtio',
> > > > + '-cpu qemu',])
> > > > diff --git a/tools/testing/kunit/qemu_configs/sparc.py b/tools/testing/kunit/qemu_configs/sparc.py
> > > > new file mode 100644
> > > > index 0000000000000..f26b5f27cc5a1
> > > > --- /dev/null
> > > > +++ b/tools/testing/kunit/qemu_configs/sparc.py
> > > > @@ -0,0 +1,10 @@
> > > > +from ..qemu_config import QemuArchParams
> > > > +
> > > > +QEMU_ARCH = QemuArchParams(linux_arch='sparc',
> > > > + qemuconfig='''
> > > > +CONFIG_SERIAL_8250=y
> > > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > > > + qemu_arch='sparc',
> > > > + kernel_path='arch/sparc/boot/zImage',
> > > > + kernel_command_line='console=ttyS0 mem=256M',
> > > > + extra_qemu_params=['-m 256'])
> > > > diff --git a/tools/testing/kunit/qemu_configs/x86_64.py b/tools/testing/kunit/qemu_configs/x86_64.py
> > > > new file mode 100644
> > > > index 0000000000000..bd5ab733b92ac
> > > > --- /dev/null
> > > > +++ b/tools/testing/kunit/qemu_configs/x86_64.py
> > > > @@ -0,0 +1,10 @@
> > > > +from ..qemu_config import QemuArchParams
> > > > +
> > > > +QEMU_ARCH = QemuArchParams(linux_arch='x86_64',
> > > > + qemuconfig='''
> > > > +CONFIG_SERIAL_8250=y
> > > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > > > + qemu_arch='x86_64',
> > > > + kernel_path='arch/x86/boot/bzImage',
> > > > + kernel_command_line='console=ttyS0',
> > > > + extra_qemu_params=[''])
> > > > --
> > > > 2.31.1.607.g51e8a6a459-goog
> > > >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/4] kunit: tool: add support for QEMU
2021-05-21 3:53 ` David Gow
@ 2021-05-21 20:57 ` Brendan Higgins
0 siblings, 0 replies; 16+ messages in thread
From: Brendan Higgins @ 2021-05-21 20:57 UTC (permalink / raw)
To: David Gow
Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK,
KUnit Development, Linux Kernel Mailing List, Jonathan Corbet,
open list:DOCUMENTATION, Stephen Boyd, Kees Cook, Frank Rowand,
Daniel Latypov
On Thu, May 20, 2021 at 8:53 PM David Gow <davidgow@google.com> wrote:
>
> On Wed, May 19, 2021 at 4:43 AM Brendan Higgins
> <brendanhiggins@google.com> wrote:
> >
> > On Mon, May 17, 2021 at 8:01 PM David Gow <davidgow@google.com> wrote:
> > >
> > > On Sat, May 15, 2021 at 3:59 PM David Gow <davidgow@google.com> wrote:
> > > >
> > > > On Sat, May 8, 2021 at 5:31 AM Brendan Higgins
> > > > <brendanhiggins@google.com> wrote:
> > > > >
> > > > > Add basic support to run QEMU via kunit_tool. Add support for i386,
> > > > > x86_64, arm, arm64, and a bunch more.
> > > > >
> > > > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > > > > Tested-by: David Gow <davidgow@google.com>
> > > > > ---
> > > > >
> > > > > Changes since last revision:
> > > > >
> > > > > - A number of minor obvious issues pointed out by David and Daniel.
> > > > > - Added facility for merging Kconfigs at Daniel's suggestion.
> > > > > - Broke out qemu_configs each into their own config file which is loaded
> > > > > dynamically - mostly at David's suggestion.
> > > > >
> > > > > ---
> > > >
> > > > This seems pretty good to me. I only have one real complaint --
> > > > qemu_configs needing to be in a subdirectory of ./tools/testing/kunit
> > > > -- but am able to tolerate that (even if I'd prefer not to have it) if
> > > > it's documented properly.
> > > >
> > > > Otherwise, save for a couple of minor nitpicks, this seems good to go.
> > > >
> > > > Reviewed-by: David Gow <davidgow@google.com>
> > > >
> > > >
> > >
> > > One thing I forgot to mention is that I'm not 100% sure about the
> > > Kconfig fragments being embedded in the qemu_configs. I still kind-of
> > > prefer the idea of them being in separate config files. While I don't
> >
> > I don't feel strongly either way, but I don't have a good idea on how
> > to implement your idea well. How about we leave it for now, and if you
> > decide you really want to do something about it, you can do it?
> >
> > > think this is necessarily a blocker, I did just realise that, by
> > > default, kunit.py run --arch=<non-UM-arch> will pull its default
> > > .kunitconfig from arch/um/configs/kunit_defconfig, which definitely
> > > feels awkward when UML is not otherwise involved.
> >
> > Hmmm...this file is identical to
> > tools/testing/kunit/configs/all_tests.config. Maybe we should just use
> > that instead?
> >
>
> That sounds like a better plan. It looks like all_tests.config isn't
> used anywhere, anyway. I might rename it and replace the
> arch/um/.../kunit_defconfig version in another patch, then.
>
> > > Some further thoughts below (which range a bit from "practical
> > > suggestion" to "overcomplicated ponderings", so don't feel the
> > > pressure to take all of them).
> > >
> > > (...snip...)
> > >
> > > > > diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> > > > > index e22ade9d91ad5..2bd196fd69e5c 100644
> > > > > --- a/tools/testing/kunit/kunit_kernel.py
> > > > > +++ b/tools/testing/kunit/kunit_kernel.py
> > > > > @@ -6,23 +6,31 @@
> > > > > # Author: Felix Guo <felixguoxiuping@gmail.com>
> > > > > # Author: Brendan Higgins <brendanhiggins@google.com>
> > > > >
> > > > > +from __future__ import annotations
> > > > > +import importlib.util
> > > > > import logging
> > > > > import subprocess
> > > > > import os
> > > > > import shutil
> > > > > import signal
> > > > > from typing import Iterator
> > > > > +from typing import Optional
> > > > >
> > > > > from contextlib import ExitStack
> > > > >
> > > > > +from collections import namedtuple
> > > > > +
> > > > > import kunit_config
> > > > > import kunit_parser
> > > > > +import qemu_config
> > > > >
> > > > > KCONFIG_PATH = '.config'
> > > > > KUNITCONFIG_PATH = '.kunitconfig'
> > > > > DEFAULT_KUNITCONFIG_PATH = 'arch/um/configs/kunit_defconfig'
> > >
> > > This being in arch/um doesn't seem great if its being used for non-UML
> > > builds. Is it worth either:
> > > (a) moving this somewhere else (e.g., tools/testing/kunit/configs as
> > > with the BROKEN_ALLCONFIG_PATH beflow), or
> >
> > How about we use: tools/testing/kunit/configs/all_tests.config ? The
> > file is identical.
>
> Yeah: I'm not thrilled with the name all_tests.config, but since it
> doesn't appear to be being used anywhere, I might just rename it in
> another patch.
>
> > > (b) giving each architecture its own kunit_defconfig, possibly in
> > > place of the qemuconfig member of QemuArchParams
> > >
> > > I'm leaning towards (b), which solves two different sources of
> > > ugliness in one go, though it would appear to have the downside that
> > > the default .kunitconfig could end up being architecture specific,
> > > which isn't great.
> >
> > Yeah, I am not a fan of trying to solve that problem in this patchset.
> > This is sounding more and more like what should be follow-on work to
> > me.
>
> Yeah, I'm not sure exactly what that should look like yet, anyway.
>
> Let's keep things as they are in this patch. I'll put a follow-up
> patch to use all_tests.config rather than the arch/um one (possibly as
> part of my "default to ALL_TESTS" patchset), and if we think of
> something better that is more architecture specific, we'll do that.
>
> > > > > BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
> > > > > OUTFILE_PATH = 'test.log'
> > > > > +ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
> > > > > +QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
> > > > >
> > >
> > > (...snip...)
> > >
> > > > > diff --git a/tools/testing/kunit/qemu_config.py b/tools/testing/kunit/qemu_config.py
> > > > > new file mode 100644
> > > > > index 0000000000000..aff1fe0442dbc
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/kunit/qemu_config.py
> > > > > @@ -0,0 +1,17 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +#
> > > > > +# Collection of configs for building non-UML kernels and running them on QEMU.
> > > > > +#
> > > > > +# Copyright (C) 2021, Google LLC.
> > > > > +# Author: Brendan Higgins <brendanhiggins@google.com>
> > > > > +
> > > > > +from collections import namedtuple
> > > > > +
> > > > > +
> > > > > +QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',
> > > > > + 'qemuconfig',
> > >
> > > As mentioned, I'm not thrilled about keeping the Kconfig inline here,
> > > and would kind-of prefer it to be in another file. I could live with
> > > it if I have to, though. Regardless, 'qemuconfig' is not a
> >
> > It will be fixed in the next revision.
> >
> > > super-descriptive name, particularly as it's not clear if this is
> > > configuring QEMU (no, that's extra_qemu_params'), or configuring the
> > > kernel for QEMU compatibility.
> >
> > Any suggestions on a better name? qemu_build_config_path? These
> > configs contain configs for configuring, building, and running kernels
> > on QEMU.
>
> I don't think we need "qemu" in the name, as this is already part of
> the QemuArchParams struct, and isn't a qemu config, but a kernel one.
>
> Something along the lines of "kernel_config" (or just "kconfig") maybe?
Doh, I got lost at some point and in my last email I thought you were
talking about the name QemuArchParams, not the qemuconfig field within
that struct. Looking back it is entirely clear to me that that is what
you were talking about, but for some reason I had a brainfart in my
last response. So yes, your suggestion sounds very reasonable. Will
fix.
> > > > > + 'qemu_arch',
> > > > > + 'kernel_path',
> > > > > + 'kernel_command_line',
> > > > > + 'extra_qemu_params'])
> > > > > +
> > > >
> > > > Nit: newline at end of file.
> > > >
> > > >
> > > >
> > > > > diff --git a/tools/testing/kunit/qemu_configs/alpha.py b/tools/testing/kunit/qemu_configs/alpha.py
> > > > > new file mode 100644
> > > > > index 0000000000000..2cc64f848ca2c
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/kunit/qemu_configs/alpha.py
> > > > > @@ -0,0 +1,10 @@
> > > > > +from ..qemu_config import QemuArchParams
> > > > > +
> > > > > +QEMU_ARCH = QemuArchParams(linux_arch='alpha',
> > > > > + qemuconfig='''
> > > > > +CONFIG_SERIAL_8250=y
> > > > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > >
> > > If these were in a separate file, they could be shared across alpha,
> > > i386, x86_64, etc. Of course, that wouldn't gel well with putting them
> > > in arch/.../config. If there were some way of listing multiple files,
> > > it could form part of the config for several more architectures,
> > > though that's probably overcomplicating things.
> >
> > Yeah, like I said, I have sympathy for what you are saying here, but
> > it really feels like something that can and should be addressed in
> > follow on patches. We could totally address this issue later by
> > expanding this field to take either a string containing a Kconfig, or
> > a path to an external Kconfig; if we do so, it won't cause any
> > migration issues in the future.
> >
>
> Yeah: I think we can solve this if it actually becomes a problem. No
> need to change anything here.
>
> > > > > + qemu_arch='alpha',
> > > > > + kernel_path='arch/alpha/boot/vmlinux',
> > > > > + kernel_command_line='console=ttyS0',
> > > > > + extra_qemu_params=[''])
> > > > > diff --git a/tools/testing/kunit/qemu_configs/arm.py b/tools/testing/kunit/qemu_configs/arm.py
> > > > > new file mode 100644
> > > > > index 0000000000000..29a043b0531a0
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/kunit/qemu_configs/arm.py
> > > > > @@ -0,0 +1,13 @@
> > > > > +from ..qemu_config import QemuArchParams
> > > > > +
> > > > > +QEMU_ARCH = QemuArchParams(linux_arch='arm',
> > > > > + qemuconfig='''
> > > > > +CONFIG_ARCH_VIRT=y
> > > > > +CONFIG_SERIAL_AMBA_PL010=y
> > > > > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > > > > +CONFIG_SERIAL_AMBA_PL011=y
> > > > > +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
> > >
> > > Similarly, if in a separate file and there were some multiple-file
> > > mechanism, these could mostly be shared between arm & arm64 (ARCH_VIRT
> > > being the only problem). Again, probably overcomplicating it at this
> > > point though.
> >
> > Right.
> >
> > > > > + qemu_arch='arm',
> > > > > + kernel_path='arch/arm/boot/zImage',
> > > > > + kernel_command_line='console=ttyAMA0',
> > > > > + extra_qemu_params=['-machine virt'])
> > > > > diff --git a/tools/testing/kunit/qemu_configs/arm64.py b/tools/testing/kunit/qemu_configs/arm64.py
> > > > > new file mode 100644
> > > > > index 0000000000000..1ba200bc99f0f
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/kunit/qemu_configs/arm64.py
> > > > > @@ -0,0 +1,12 @@
> > > > > +from ..qemu_config import QemuArchParams
> > > > > +
> > > > > +QEMU_ARCH = QemuArchParams(linux_arch='arm64',
> > > > > + qemuconfig='''
> > > > > +CONFIG_SERIAL_AMBA_PL010=y
> > > > > +CONFIG_SERIAL_AMBA_PL010_CONSOLE=y
> > > > > +CONFIG_SERIAL_AMBA_PL011=y
> > > > > +CONFIG_SERIAL_AMBA_PL011_CONSOLE=y''',
> > > > > + qemu_arch='aarch64',
> > > > > + kernel_path='arch/arm64/boot/Image.gz',
> > > > > + kernel_command_line='console=ttyAMA0',
> > > > > + extra_qemu_params=['-machine virt', '-cpu cortex-a57'])
> > > > > diff --git a/tools/testing/kunit/qemu_configs/i386.py b/tools/testing/kunit/qemu_configs/i386.py
> > > > > new file mode 100644
> > > > > index 0000000000000..3998af306468e
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/kunit/qemu_configs/i386.py
> > > > > @@ -0,0 +1,10 @@
> > > > > +from ..qemu_config import QemuArchParams
> > > > > +
> > > > > +QEMU_ARCH = QemuArchParams(linux_arch='i386',
> > > > > + qemuconfig='''
> > > > > +CONFIG_SERIAL_8250=y
> > > > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > > > > + qemu_arch='x86_64',
> > > > > + kernel_path='arch/x86/boot/bzImage',
> > > > > + kernel_command_line='console=ttyS0',
> > > > > + extra_qemu_params=[''])
> > > > > diff --git a/tools/testing/kunit/qemu_configs/powerpc.py b/tools/testing/kunit/qemu_configs/powerpc.py
> > > > > new file mode 100644
> > > > > index 0000000000000..46292ce9e368e
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/kunit/qemu_configs/powerpc.py
> > > > > @@ -0,0 +1,12 @@
> > > > > +from ..qemu_config import QemuArchParams
> > > > > +
> > > > > +QEMU_ARCH = QemuArchParams(linux_arch='powerpc',
> > > > > + qemuconfig='''
> > > > > +CONFIG_PPC64=y
> > > > > +CONFIG_SERIAL_8250=y
> > > > > +CONFIG_SERIAL_8250_CONSOLE=y
> > > > > +CONFIG_HVC_CONSOLE=y''',
> > > > > + qemu_arch='ppc64',
> > > > > + kernel_path='vmlinux',
> > > > > + kernel_command_line='console=ttyS0',
> > > > > + extra_qemu_params=['-M pseries', '-cpu power8'])
> > > > > diff --git a/tools/testing/kunit/qemu_configs/riscv.py b/tools/testing/kunit/qemu_configs/riscv.py
> > > > > new file mode 100644
> > > > > index 0000000000000..de8c62d465723
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/kunit/qemu_configs/riscv.py
> > > > > @@ -0,0 +1,31 @@
> > > > > +from ..qemu_config import QemuArchParams
> > > > > +import os
> > > > > +import os.path
> > > > > +import sys
> > > > > +
> > > > > +GITHUB_OPENSBI_URL = 'https://github.com/qemu/qemu/raw/master/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin'
> > > > > +OPENSBI_FILE = os.path.basename(GITHUB_OPENSBI_URL)
> > > > > +
> > > > > +if not os.path.isfile(OPENSBI_FILE):
> > > > > + print('\n\nOpenSBI file is not in the current working directory.\n'
> > > > > + 'Would you like me to download it for you from:\n' + GITHUB_OPENSBI_URL + ' ?\n')
> > > > > + response = input('yes/[no]: ')
> > > > > + if response.strip() == 'yes':
> > > > > + os.system('wget ' + GITHUB_OPENSBI_URL)
> > > > > + else:
> > > > > + sys.exit()
> > > > > +
> > > > > +QEMU_ARCH = QemuArchParams(linux_arch='riscv',
> > > > > + qemuconfig='''
> > > > > +CONFIG_SOC_VIRT=y
> > > > > +CONFIG_SERIAL_8250=y
> > > > > +CONFIG_SERIAL_8250_CONSOLE=y
> > > > > +CONFIG_SERIAL_OF_PLATFORM=y
> > > > > +CONFIG_SERIAL_EARLYCON_RISCV_SBI=y''',
> > > > > + qemu_arch='riscv64',
> > > > > + kernel_path='arch/riscv/boot/Image',
> > > > > + kernel_command_line='console=ttyS0',
> > > > > + extra_qemu_params=[
> > > > > + '-machine virt',
> > > > > + '-cpu rv64',
> > > > > + '-bios opensbi-riscv64-generic-fw_dynamic.bin'])
> > > > > diff --git a/tools/testing/kunit/qemu_configs/s390.py b/tools/testing/kunit/qemu_configs/s390.py
> > > > > new file mode 100644
> > > > > index 0000000000000..04c90332f1098
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/kunit/qemu_configs/s390.py
> > > > > @@ -0,0 +1,14 @@
> > > > > +from ..qemu_config import QemuArchParams
> > > > > +
> > > > > +QEMU_ARCH = QemuArchParams(linux_arch='s390',
> > > > > + qemuconfig='''
> > > > > +CONFIG_EXPERT=y
> > > > > +CONFIG_TUNE_ZEC12=y
> > > > > +CONFIG_NUMA=y
> > > > > +CONFIG_MODULES=y''',
> > > > > + qemu_arch='s390x',
> > > > > + kernel_path='arch/s390/boot/bzImage',
> > > > > + kernel_command_line='console=ttyS0',
> > > > > + extra_qemu_params=[
> > > > > + '-machine s390-ccw-virtio',
> > > > > + '-cpu qemu',])
> > > > > diff --git a/tools/testing/kunit/qemu_configs/sparc.py b/tools/testing/kunit/qemu_configs/sparc.py
> > > > > new file mode 100644
> > > > > index 0000000000000..f26b5f27cc5a1
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/kunit/qemu_configs/sparc.py
> > > > > @@ -0,0 +1,10 @@
> > > > > +from ..qemu_config import QemuArchParams
> > > > > +
> > > > > +QEMU_ARCH = QemuArchParams(linux_arch='sparc',
> > > > > + qemuconfig='''
> > > > > +CONFIG_SERIAL_8250=y
> > > > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > > > > + qemu_arch='sparc',
> > > > > + kernel_path='arch/sparc/boot/zImage',
> > > > > + kernel_command_line='console=ttyS0 mem=256M',
> > > > > + extra_qemu_params=['-m 256'])
> > > > > diff --git a/tools/testing/kunit/qemu_configs/x86_64.py b/tools/testing/kunit/qemu_configs/x86_64.py
> > > > > new file mode 100644
> > > > > index 0000000000000..bd5ab733b92ac
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/kunit/qemu_configs/x86_64.py
> > > > > @@ -0,0 +1,10 @@
> > > > > +from ..qemu_config import QemuArchParams
> > > > > +
> > > > > +QEMU_ARCH = QemuArchParams(linux_arch='x86_64',
> > > > > + qemuconfig='''
> > > > > +CONFIG_SERIAL_8250=y
> > > > > +CONFIG_SERIAL_8250_CONSOLE=y''',
> > > > > + qemu_arch='x86_64',
> > > > > + kernel_path='arch/x86/boot/bzImage',
> > > > > + kernel_command_line='console=ttyS0',
> > > > > + extra_qemu_params=[''])
> > > > > --
> > > > > 2.31.1.607.g51e8a6a459-goog
> > > > >
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-05-21 20:57 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07 21:31 [PATCH v1 0/4] kunit: tool: add support for QEMU Brendan Higgins
2021-05-07 21:31 ` [PATCH v1 1/4] kunit: Add 'kunit_shutdown' option Brendan Higgins
2021-05-15 7:58 ` David Gow
2021-05-18 20:52 ` Brendan Higgins
2021-05-07 21:31 ` [PATCH v1 2/4] Documentation: Add kunit_shutdown to kernel-parameters.txt Brendan Higgins
2021-05-15 7:58 ` David Gow
2021-05-07 21:31 ` [PATCH v1 3/4] kunit: tool: add support for QEMU Brendan Higgins
2021-05-15 7:59 ` David Gow
2021-05-18 3:01 ` David Gow
2021-05-18 20:43 ` Brendan Higgins
2021-05-21 3:53 ` David Gow
2021-05-21 20:57 ` Brendan Higgins
2021-05-18 20:27 ` Brendan Higgins
2021-05-07 21:31 ` [PATCH v1 4/4] Documentation: kunit: document support for QEMU in kunit_tool Brendan Higgins
2021-05-15 8:01 ` David Gow
2021-05-18 21:08 ` Brendan Higgins
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.