On Thu, Apr 29, 2021 at 1:51 PM 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.

Hmmm, I'm wondering if I'm seeing unrelated breakages. Applied these patches on top of 55ba0fe059a5 ("Merge tag 'for-5.13-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux")

$ make mrproper $ rm -rf .kunit/* # just in case $ ./tools/testing/kunit/kunit.py run --arch=arm64 ... ERROR:root:arch/arm64/Makefile:25: ld does not support --fix-cortex-a53-843419; kernel may be susceptible to erratum arch/arm64/Makefile:44: Detected assembler with broken .inst; disassembly will be unreliable gcc: error: unrecognized command-line option ‘-mlittle-endian’

So it seems like my version of ld might be out of date, but my gcc is 10.2.1 Is there a minimum version of the toolchain this would expect that we can call out in the commit message (and in the Documentation)?

--arch=x86_64 worked just fine for me, however, which is mainly what I was interested in.

I've mainly just left some nits and comments regarding typechecking.

Signed-off-by: Brendan Higgins brendanhiggins@google.com

tools/testing/kunit/kunit.py | 33 +++- tools/testing/kunit/kunit_config.py | 2 +- tools/testing/kunit/kunit_kernel.py | 207 +++++++++++++++++++++---- tools/testing/kunit/kunit_tool_test.py | 15 +- 4 files changed, 217 insertions(+), 40 deletions(-)

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index d5144fcb03acd..07ef80062873b 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,
  

@@ -187,6 +187,14 @@ def add_common_opts(parser) -> None: help='Path to Kconfig fragment that enables KUnit tests', metavar='kunitconfig')

•   parser.add_argument('--arch',
  

•                       help='Specifies the architecture to run tests under.',
  

optional: perhaps add "(e.g. x86_64, um)" Most people using this would be able to infer that, but I prefer strings that are basically enums to document examples of useful values. (And x86 is probably the one most people want to use this flag for).

•                       type=str, default='um', metavar='arch')
  

•   parser.add_argument('--cross_compile',
  

•                       help='Sets make\'s CROSS_COMPILE variable.',
  

•                       metavar='cross_compile')
  

def add_build_opts(parser) -> None: parser.add_argument('--jobs', help='As in the make command, "Specifies the number of ' @@ -268,7 +276,10 @@ 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)
  
            request = KunitRequest(cli_args.raw_output,
                                   cli_args.timeout,
  

@@ -287,7 +298,9 @@ 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)
  
            request = KunitConfigRequest(cli_args.build_dir,
                                         cli_args.make_options)
  

@@ -299,7 +312,9 @@ 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)
  
            request = KunitBuildRequest(cli_args.jobs,
                                        cli_args.build_dir,
  

@@ -313,7 +328,9 @@ 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)
  
            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..07d76be392544 100644 --- a/tools/testing/kunit/kunit_config.py +++ b/tools/testing/kunit/kunit_config.py @@ -53,7 +53,7 @@ class Kconfig(object): return True

    def write_to_file(self, path: str) -> None:

•           with open(path, 'w') as f:
  

•           with open(path, 'a+') as f:
  

I might be missing something, but do we need this?

w => a means we wouldn't truncate the file if it exists. I can imagine I'm missing something that makes it necessary. + => opens for read/write: we don't do any reads with f afaict.

                    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 7d5b77967d48f..b8b3b76aaa17e 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -15,6 +15,8 @@ from typing import Iterator

from contextlib import ExitStack

+from collections import namedtuple

import kunit_config import kunit_parser

@@ -40,6 +42,10 @@ class BuildError(Exception): class LinuxSourceTreeOperations(object): """An abstraction over command line operations performed on a source tree."""

•   def __init__(self, linux_arch, cross_compile):
  

nit: let's annotate these as `linux_arch: str, cross_compile: str` (or is it Optional[str] here?) I can see a reader thinking arch might be an enum and that cross_compile might be a bool.

•           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 +54,18 @@ class LinuxSourceTreeOperations(object): except subprocess.CalledProcessError as e: raise ConfigError(e.output.decode())

•   def make_arch_qemuconfig(self, build_dir):
  

•           pass
  

•   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(' '.join(command))
            try:
                    subprocess.check_output(command, stderr=subprocess.STDOUT)
            except OSError as e:
  

@@ -61,6 +73,154 @@ 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(' '.join(command))
  

•           try:
  

•                   proc = subprocess.Popen(command,
  

•                                           stderr=subprocess.PIPE,
  

•                                           stdout=subprocess.DEVNULL)
  

•           except OSError as e:
  

•                   raise BuildError('Could not call execute make: ' + e)
  

pytype complains about this. You'd want `+ 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
  

+QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',

•                                          'qemuconfig',
  

•                                          'qemu_arch',
  

•                                          'kernel_path',
  

•                                          'kernel_command_line',
  

•                                          'extra_qemu_params'])
  

+QEMU_ARCHS = {

•   'i386'          : QemuArchParams(linux_arch='i386',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
  

•                           qemu_arch='x86_64',
  

•                           kernel_path='arch/x86/boot/bzImage',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['']),
  

•   'x86_64'        : QemuArchParams(linux_arch='x86_64',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
  

•                           qemu_arch='x86_64',
  

•                           kernel_path='arch/x86/boot/bzImage',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['']),
  

•   'arm'           : 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']),
  

•   'arm64'         : 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']),
  

•   'alpha'         : QemuArchParams(linux_arch='alpha',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
  

•                           qemu_arch='alpha',
  

•                           kernel_path='arch/alpha/boot/vmlinux',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['']),
  

•   'powerpc'       : QemuArchParams(linux_arch='powerpc',
  

•                           qemuconfig='CONFIG_PPC64=y\nCONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y\nCONFIG_HVC_CONSOLE=y',
  

•                           qemu_arch='ppc64',
  

•                           kernel_path='vmlinux',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['-M pseries', '-cpu power8']),
  

•   'riscv'         : QemuArchParams(linux_arch='riscv',
  

•                           qemuconfig='CONFIG_SOC_VIRT=y\nCONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y\nCONFIG_SERIAL_OF_PLATFORM=y\nCONFIG_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']),
  

•   's390'          : QemuArchParams(linux_arch='s390',
  

•                           qemuconfig='CONFIG_EXPERT=y\nCONFIG_TUNE_ZEC12=y\nCONFIG_NUMA=y\nCONFIG_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',]),
  

•   'sparc'         : QemuArchParams(linux_arch='sparc',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_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']),
  

+}

Oh my. I don't know enough to say if there's a better way of doing this.

But I think we should probably split this out into a separate python file, if this mapping remains necessary. E.g. in a qemu_configs.py file or the like.

+class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):

I've called out the two errors pytype gives already, but mypy is even worse about this new class :(

$ mypy tools/testing/kunit/*.py tools/testing/kunit/kunit_kernel.py:90: error: Unsupported operand types for + ("str" and "OSError") tools/testing/kunit/kunit_kernel.py:278: error: Incompatible types in assignment (expression has type "LinuxSourceTreeOperationsQemu", variable has type "Optional[LinuxSourceTreeOperationsUml]") tools/testing/kunit/kunit_kernel.py:298: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_mrproper" tools/testing/kunit/kunit_kernel.py:324: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_arch_qemuconfig" tools/testing/kunit/kunit_kernel.py:325: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_olddefconfig" tools/testing/kunit/kunit_kernel.py:352: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_allyesconfig" tools/testing/kunit/kunit_kernel.py:353: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_arch_qemuconfig" tools/testing/kunit/kunit_kernel.py:354: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_olddefconfig" tools/testing/kunit/kunit_kernel.py:355: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make" tools/testing/kunit/kunit_kernel.py:368: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "run"

So to make up for mypy being less smart, we can do this and get it down 2 errors

diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index b8b3b76aaa17..a0aaa28db4c1 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -11,7 +11,7 @@ import subprocess import os import shutil import signal -from typing import Iterator +from typing import Iterator, Union

from contextlib import ExitStack

@@ -269,15 +269,16 @@ class LinuxSourceTree(object):

def __init__(self, build_dir: str, load_config=True, kunitconfig_path='', arch=None, cross_compile=None) -> None: signal.signal(signal.SIGINT, self.signal_handler) - self._ops = None + ops = None # type: Union[None, LinuxSourceTreeOperationsUml, LinuxSourceTreeOperationsQemu] if arch is None or arch == 'um': self._arch = 'um' - self._ops = LinuxSourceTreeOperationsUml() + ops = LinuxSourceTreeOperationsUml() elif arch in QEMU_ARCHS: self._arch = arch - self._ops = LinuxSourceTreeOperationsQemu(QEMU_ARCHS[arch], cross_compile=cross_compile) + ops = LinuxSourceTreeOperationsQemu(QEMU_ARCHS[arch], cross_compile=cross_compile) else: raise ConfigError(arch + ' is not a valid arch') + self._ops = ops

if not load_config: return

•   def __init__(self, qemu_arch_params, cross_compile):
  

•           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
  

•           print(self._kernel_path)
  

looks like a leftover debugging print statement?

•           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, build_dir):
  

•           qemuconfig = kunit_config.Kconfig()
  

•           qemuconfig.parse_from_string(self._qemuconfig)
  

•           qemuconfig.write_to_file(get_kconfig_path(build_dir))
  

•   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(' '.join(qemu_command))
  

ditto, a debug statement? Though this one could be useful to actually print out for the user if we include some more context in the message.

•           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):
  

•           super().__init__(linux_arch='um', cross_compile=None)
  

•   def make_allyesconfig(self, build_dir, make_options) -> None:
            kunit_parser.print_with_timestamp(
                    'Enabling all CONFIGs for UML...')
  

@@ -83,32 +243,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: @@ -123,10 +267,17 @@ def get_outfile_path(build_dir) -> str: 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) -> None:
            signal.signal(signal.SIGINT, self.signal_handler)
  

•           self._ops = LinuxSourceTreeOperations()
  

•           self._ops = None
  

•           if arch is None or arch == 'um':
  

•                   self._arch = 'um'
  

•                   self._ops = LinuxSourceTreeOperationsUml()
  

•           elif arch in QEMU_ARCHS:
  

•                   self._arch = arch
  

•                   self._ops = LinuxSourceTreeOperationsQemu(QEMU_ARCHS[arch], cross_compile=cross_compile)
  

•           else:
  

•                   raise ConfigError(arch + ' is not a valid arch')
  
            if not load_config:
                    return
  

@@ -170,6 +321,7 @@ class LinuxSourceTree(object): os.mkdir(build_dir) self._kconfig.write_to_file(kconfig_path) try:

•                   self._ops.make_arch_qemuconfig(build_dir)
                    self._ops.make_olddefconfig(build_dir, make_options)
            except ConfigError as e:
                    logging.error(e)
  

@@ -192,10 +344,13 @@ 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:
  

•                           if self._arch != 'um':
  

•                                   raise ConfigError('Only the "um" arch is supported for alltests')
                            self._ops.make_allyesconfig(build_dir, make_options)
  

Minor note: pytype doesn't like this. The code is correct, but pytype can't figure out that ops can't be the QEMU variant.

Pytype recognizes comments like " # pytype: disable=attribute-error" but mypy doesn't. So I don't know that there's a way we can make both of them happy :/

•                   self._ops.make_arch_qemuconfig(build_dir)
                    self._ops.make_olddefconfig(build_dir, make_options)
                    self._ops.make(jobs, build_dir, make_options)
            except (ConfigError, BuildError) as e:
  

@@ -209,8 +364,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 1ad3049e90698..25e8be95a575d 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -297,7 +297,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):
  

@@ -308,7 +308,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):
  

@@ -390,7 +390,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'
  

@@ -404,14 +404,19 @@ 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)
  
    @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')
  
  

if __name__ == '__main__': unittest.main() -- 2.31.1.498.g6c1eba8ee3d-goog

On Fri, Apr 30, 2021 at 1:01 PM 'Brendan Higgins' via KUnit Development kunit-dev@googlegroups.com wrote:

On Thu, Apr 29, 2021 at 4:40 PM Daniel Latypov dlatypov@google.com wrote:

...

On Thu, Apr 29, 2021 at 1:51 PM 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.

Hmmm, I'm wondering if I'm seeing unrelated breakages. Applied these patches on top of 55ba0fe059a5 ("Merge tag 'for-5.13-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux")

$ make mrproper $ rm -rf .kunit/* # just in case $ ./tools/testing/kunit/kunit.py run --arch=arm64 ...

Huh, did you use a arm64 cross compiler? Maybe that's your issue.

I didn't (and realized that like 2 minutes after I sent the email). Please disregard.

...

ERROR:root:arch/arm64/Makefile:25: ld does not support --fix-cortex-a53-843419; kernel may be susceptible to erratum arch/arm64/Makefile:44: Detected assembler with broken .inst; disassembly will be unreliable gcc: error: unrecognized command-line option ‘-mlittle-endian’

So it seems like my version of ld might be out of date, but my gcc is 10.2.1 Is there a minimum version of the toolchain this would expect that we can call out in the commit message (and in the Documentation)?

--arch=x86_64 worked just fine for me, however, which is mainly what I was interested in.

I've mainly just left some nits and comments regarding typechecking.

...

Signed-off-by: Brendan Higgins brendanhiggins@google.com

tools/testing/kunit/kunit.py | 33 +++- tools/testing/kunit/kunit_config.py | 2 +- tools/testing/kunit/kunit_kernel.py | 207 +++++++++++++++++++++---- tools/testing/kunit/kunit_tool_test.py | 15 +- 4 files changed, 217 insertions(+), 40 deletions(-)

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index d5144fcb03acd..07ef80062873b 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,
  

@@ -187,6 +187,14 @@ def add_common_opts(parser) -> None: help='Path to Kconfig fragment that enables KUnit tests', metavar='kunitconfig')

•   parser.add_argument('--arch',
  

•                       help='Specifies the architecture to run tests under.',
  

optional: perhaps add "(e.g. x86_64, um)" Most people using this would be able to infer that, but I prefer strings that are basically enums to document examples of useful values.

Good point, probably want to mention that I take the string name from the argument passed into the ARCH make variable.

...

(And x86 is probably the one most people want to use this flag for).

Yep, but Linux refers to it as i386. Just trying to be consistent.

Sorry, I meant to say x86_64 there. i386 is what we should call it, yeah.

...

...

•                       type=str, default='um', metavar='arch')
  

•   parser.add_argument('--cross_compile',
  

•                       help='Sets make\'s CROSS_COMPILE variable.',
  

•                       metavar='cross_compile')
  

def add_build_opts(parser) -> None: parser.add_argument('--jobs', help='As in the make command, "Specifies the number of ' @@ -268,7 +276,10 @@ 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)
  
            request = KunitRequest(cli_args.raw_output,
                                   cli_args.timeout,
  

@@ -287,7 +298,9 @@ 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)
  
            request = KunitConfigRequest(cli_args.build_dir,
                                         cli_args.make_options)
  

@@ -299,7 +312,9 @@ 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)
  
            request = KunitBuildRequest(cli_args.jobs,
                                        cli_args.build_dir,
  

@@ -313,7 +328,9 @@ 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)
  
            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..07d76be392544 100644 --- a/tools/testing/kunit/kunit_config.py +++ b/tools/testing/kunit/kunit_config.py @@ -53,7 +53,7 @@ class Kconfig(object): return True

    def write_to_file(self, path: str) -> None:

•           with open(path, 'w') as f:
  

•           with open(path, 'a+') as f:
  

I might be missing something, but do we need this?

w => a means we wouldn't truncate the file if it exists. I can imagine I'm missing something that makes it necessary.

• => opens for read/write: we don't do any reads with f afaict.

Yeah, probably only needs to be a, not a+. But we do want append for adding the arch configs. Idea is that we want to append those onto whatever a base kunitconfig.

Nevertheless, your concern is valid, might be a better way to do this. Thoughts?

Would it be cleaner to operate on Kconfig objects directly instead of having the write_to_file() function do appends? I'm worried this might lead to unintentional statefulness in other cases.

...

...

                    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 7d5b77967d48f..b8b3b76aaa17e 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -15,6 +15,8 @@ from typing import Iterator

from contextlib import ExitStack

+from collections import namedtuple

import kunit_config import kunit_parser

@@ -40,6 +42,10 @@ class BuildError(Exception): class LinuxSourceTreeOperations(object): """An abstraction over command line operations performed on a source tree."""

•   def __init__(self, linux_arch, cross_compile):
  

nit: let's annotate these as `linux_arch: str, cross_compile: str` (or is it Optional[str] here?) I can see a reader thinking arch might be an enum and that cross_compile might be a bool.

Fair. Will do.

...

...

•           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 +54,18 @@ class LinuxSourceTreeOperations(object): except subprocess.CalledProcessError as e: raise ConfigError(e.output.decode())

•   def make_arch_qemuconfig(self, build_dir):
  

•           pass
  

•   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(' '.join(command))
            try:
                    subprocess.check_output(command, stderr=subprocess.STDOUT)
            except OSError as e:
  

@@ -61,6 +73,154 @@ 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(' '.join(command))
  

•           try:
  

•                   proc = subprocess.Popen(command,
  

•                                           stderr=subprocess.PIPE,
  

•                                           stdout=subprocess.DEVNULL)
  

•           except OSError as e:
  

•                   raise BuildError('Could not call execute make: ' + e)
  

pytype complains about this. You'd want `+ str(e)`

Got it. Will do. (I should probably also run pytype.)

...

...

•           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
  

+QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',

•                                          'qemuconfig',
  

•                                          'qemu_arch',
  

•                                          'kernel_path',
  

•                                          'kernel_command_line',
  

•                                          'extra_qemu_params'])
  

+QEMU_ARCHS = {

•   'i386'          : QemuArchParams(linux_arch='i386',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
  

•                           qemu_arch='x86_64',
  

•                           kernel_path='arch/x86/boot/bzImage',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['']),
  

•   'x86_64'        : QemuArchParams(linux_arch='x86_64',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
  

•                           qemu_arch='x86_64',
  

•                           kernel_path='arch/x86/boot/bzImage',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['']),
  

•   'arm'           : 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']),
  

•   'arm64'         : 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']),
  

•   'alpha'         : QemuArchParams(linux_arch='alpha',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
  

•                           qemu_arch='alpha',
  

•                           kernel_path='arch/alpha/boot/vmlinux',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['']),
  

•   'powerpc'       : QemuArchParams(linux_arch='powerpc',
  

•                           qemuconfig='CONFIG_PPC64=y\nCONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y\nCONFIG_HVC_CONSOLE=y',
  

•                           qemu_arch='ppc64',
  

•                           kernel_path='vmlinux',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['-M pseries', '-cpu power8']),
  

•   'riscv'         : QemuArchParams(linux_arch='riscv',
  

•                           qemuconfig='CONFIG_SOC_VIRT=y\nCONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y\nCONFIG_SERIAL_OF_PLATFORM=y\nCONFIG_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']),
  

•   's390'          : QemuArchParams(linux_arch='s390',
  

•                           qemuconfig='CONFIG_EXPERT=y\nCONFIG_TUNE_ZEC12=y\nCONFIG_NUMA=y\nCONFIG_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',]),
  

•   'sparc'         : QemuArchParams(linux_arch='sparc',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_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']),
  

+}

Oh my. I don't know enough to say if there's a better way of doing this.

Yeah, I know it's gross, but I did not want to put too much effort into until I got some feedback on it.

...

But I think we should probably split this out into a separate python file, if this mapping remains necessary. E.g. in a qemu_configs.py file or the like.

Definitely an improvement. Any other thoughts on how to make this look less gross?

Unfortunately not. Like you, I'm hoping someone else might have some better ideas how we can maybe push this config out of python.

But if we don't find anything else, I think having the hard-coding isolated into its own package is fine, tbh.

...

...

+class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):

I've called out the two errors pytype gives already, but mypy is even worse about this new class :(

$ mypy tools/testing/kunit/*.py tools/testing/kunit/kunit_kernel.py:90: error: Unsupported operand types for + ("str" and "OSError") tools/testing/kunit/kunit_kernel.py:278: error: Incompatible types in assignment (expression has type "LinuxSourceTreeOperationsQemu", variable has type "Optional[LinuxSourceTreeOperationsUml]") tools/testing/kunit/kunit_kernel.py:298: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_mrproper" tools/testing/kunit/kunit_kernel.py:324: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_arch_qemuconfig" tools/testing/kunit/kunit_kernel.py:325: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_olddefconfig" tools/testing/kunit/kunit_kernel.py:352: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_allyesconfig" tools/testing/kunit/kunit_kernel.py:353: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_arch_qemuconfig" tools/testing/kunit/kunit_kernel.py:354: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make_olddefconfig" tools/testing/kunit/kunit_kernel.py:355: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "make" tools/testing/kunit/kunit_kernel.py:368: error: Item "None" of "Optional[LinuxSourceTreeOperationsUml]" has no attribute "run"

So to make up for mypy being less smart, we can do this and get it down 2 errors

Sounds good. Will do.

...

diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index b8b3b76aaa17..a0aaa28db4c1 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -11,7 +11,7 @@ import subprocess import os import shutil import signal -from typing import Iterator +from typing import Iterator, Union

from contextlib import ExitStack

@@ -269,15 +269,16 @@ class LinuxSourceTree(object):

    def __init__(self, build_dir: str, load_config=True,

kunitconfig_path='', arch=None, cross_compile=None) -> None: signal.signal(signal.SIGINT, self.signal_handler)

•           self._ops = None
  

•           ops = None # type: Union[None,
  

LinuxSourceTreeOperationsUml, LinuxSourceTreeOperationsQemu] if arch is None or arch == 'um': self._arch = 'um'

•                   self._ops = LinuxSourceTreeOperationsUml()
  

•                   ops = LinuxSourceTreeOperationsUml()
            elif arch in QEMU_ARCHS:
                    self._arch = arch
  

•                   self._ops =
  

LinuxSourceTreeOperationsQemu(QEMU_ARCHS[arch], cross_compile=cross_compile)

•                   ops =
  

LinuxSourceTreeOperationsQemu(QEMU_ARCHS[arch], cross_compile=cross_compile) else: raise ConfigError(arch + ' is not a valid arch')

•           self._ops = ops
  
            if not load_config:
                    return
  
  

...

•   def __init__(self, qemu_arch_params, cross_compile):
  

•           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
  

•           print(self._kernel_path)
  

looks like a leftover debugging print statement?

Oh yeah. Whoops.

...

...

•           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, build_dir):
  

•           qemuconfig = kunit_config.Kconfig()
  

•           qemuconfig.parse_from_string(self._qemuconfig)
  

•           qemuconfig.write_to_file(get_kconfig_path(build_dir))
  

•   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(' '.join(qemu_command))
  

ditto, a debug statement? Though this one could be useful to actually print out for the user if we include some more context in the message.

Yeah, this was initially a debug, but then I ended up finding it pretty useful since it spits out a full qemu command that you can just run outside of the tool, so I left it in.

Yeah, I think this one was quite useful after running it a bit. Maybe something like 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):
  

•           super().__init__(linux_arch='um', cross_compile=None)
  

•   def make_allyesconfig(self, build_dir, make_options) -> None:
            kunit_parser.print_with_timestamp(
                    'Enabling all CONFIGs for UML...')
  

@@ -83,32 +243,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: @@ -123,10 +267,17 @@ def get_outfile_path(build_dir) -> str: 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) -> None:
            signal.signal(signal.SIGINT, self.signal_handler)
  

•           self._ops = LinuxSourceTreeOperations()
  

•           self._ops = None
  

•           if arch is None or arch == 'um':
  

•                   self._arch = 'um'
  

•                   self._ops = LinuxSourceTreeOperationsUml()
  

•           elif arch in QEMU_ARCHS:
  

•                   self._arch = arch
  

•                   self._ops = LinuxSourceTreeOperationsQemu(QEMU_ARCHS[arch], cross_compile=cross_compile)
  

•           else:
  

•                   raise ConfigError(arch + ' is not a valid arch')
  
            if not load_config:
                    return
  

@@ -170,6 +321,7 @@ class LinuxSourceTree(object): os.mkdir(build_dir) self._kconfig.write_to_file(kconfig_path) try:

•                   self._ops.make_arch_qemuconfig(build_dir)
                    self._ops.make_olddefconfig(build_dir, make_options)
            except ConfigError as e:
                    logging.error(e)
  

@@ -192,10 +344,13 @@ 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:
  

•                           if self._arch != 'um':
  

•                                   raise ConfigError('Only the "um" arch is supported for alltests')
                            self._ops.make_allyesconfig(build_dir, make_options)
  

Minor note: pytype doesn't like this. The code is correct, but pytype can't figure out that ops can't be the QEMU variant.

Pytype recognizes comments like " # pytype: disable=attribute-error" but mypy doesn't. So I don't know that there's a way we can make both of them happy :/

Hmmm...I could just add make_allyesconfig() to the base class and make it raise. That might be cleaner too.

Hmm, that's probably a good idea.

Side note: I think we might want to keep the existing check as well just so it's more obvious to a reader that only uml is supported, even at the cost of some duplication. But that's just personal preference.

...

...

•                   self._ops.make_arch_qemuconfig(build_dir)
                    self._ops.make_olddefconfig(build_dir, make_options)
                    self._ops.make(jobs, build_dir, make_options)
            except (ConfigError, BuildError) as e:
  

@@ -209,8 +364,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 1ad3049e90698..25e8be95a575d 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -297,7 +297,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):
  

@@ -308,7 +308,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):
  

@@ -390,7 +390,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'
  

@@ -404,14 +404,19 @@ 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)
  
    @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')
  
  

if __name__ == '__main__': unittest.main() -- 2.31.1.498.g6c1eba8ee3d-goog

-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/CAFd5g44OL0BQ74ZGEnYzsLYVXEbOB-h....

On Fri, Apr 30, 2021 at 4:51 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

I've tested this out on a few architectures (x86-32, ppc64, etc), and it all seems to work pretty well. I've got quite a few comments below. The main one -- the location of the qemu configs -- is basically echoing what Daniel said and what you've done on the Gerrit version, but I felt was worth having on the record.

Otherwise, a bunch of nitpicks around missing places cross_compile should be supported.

Still, great to see this happening: it's nice to be able to test on non-UML architectures more easily.

Tested-by: David Gow davidgow@google.com

-- David

---

tools/testing/kunit/kunit.py | 33 +++- tools/testing/kunit/kunit_config.py | 2 +- tools/testing/kunit/kunit_kernel.py | 207 +++++++++++++++++++++---- tools/testing/kunit/kunit_tool_test.py | 15 +- 4 files changed, 217 insertions(+), 40 deletions(-)

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index d5144fcb03acd..07ef80062873b 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,
  

@@ -187,6 +187,14 @@ def add_common_opts(parser) -> None: help='Path to Kconfig fragment that enables KUnit tests', metavar='kunitconfig')

•   parser.add_argument('--arch',
  

•                       help='Specifies the architecture to run tests under.',
  

•                       type=str, default='um', metavar='arch')
  

It'd be nice to note explicitly that non-'um' values here will run under qemu.

•   parser.add_argument('--cross_compile',
  

•                       help='Sets make\'s CROSS_COMPILE variable.',
  

•                       metavar='cross_compile')
  

It'd be nice to have a slightly more detailed description here (that its the compiler name's prefix or something).

def add_build_opts(parser) -> None: parser.add_argument('--jobs', help='As in the make command, "Specifies the number of ' @@ -268,7 +276,10 @@ 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)
  
            request = KunitRequest(cli_args.raw_output,
                                   cli_args.timeout,
  

@@ -287,7 +298,9 @@ 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)
  
            request = KunitConfigRequest(cli_args.build_dir,
                                         cli_args.make_options)
  

@@ -299,7 +312,9 @@ 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)
  

Why aren't we passing cross_compile through here? It's pretty important for the 'build' step.

            request = KunitBuildRequest(cli_args.jobs,
                                        cli_args.build_dir,

@@ -313,7 +328,9 @@ 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)
  
            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..07d76be392544 100644 --- a/tools/testing/kunit/kunit_config.py +++ b/tools/testing/kunit/kunit_config.py @@ -53,7 +53,7 @@ class Kconfig(object): return True

    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 7d5b77967d48f..b8b3b76aaa17e 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -15,6 +15,8 @@ from typing import Iterator

from contextlib import ExitStack

+from collections import namedtuple

import kunit_config import kunit_parser

@@ -40,6 +42,10 @@ class BuildError(Exception): class LinuxSourceTreeOperations(object): """An abstraction over command line operations performed on a source tree."""

•   def __init__(self, linux_arch, cross_compile):
  

•           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 +54,18 @@ class LinuxSourceTreeOperations(object): except subprocess.CalledProcessError as e: raise ConfigError(e.output.decode())

•   def make_arch_qemuconfig(self, build_dir):
  

•           pass
  

•   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(' '.join(command))
            try:
                    subprocess.check_output(command, stderr=subprocess.STDOUT)
            except OSError as e:
  

@@ -61,6 +73,154 @@ 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(' '.join(command))
  

•           try:
  

•                   proc = subprocess.Popen(command,
  

•                                           stderr=subprocess.PIPE,
  

•                                           stdout=subprocess.DEVNULL)
  

•           except OSError as e:
  

•                   raise BuildError('Could not call execute make: ' + 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
  

+QemuArchParams = namedtuple('QemuArchParams', ['linux_arch',

•                                          'qemuconfig',
  

•                                          'qemu_arch',
  

•                                          'kernel_path',
  

•                                          'kernel_command_line',
  

•                                          'extra_qemu_params'])
  

+QEMU_ARCHS = {

•   'i386'          : QemuArchParams(linux_arch='i386',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
  

I'm not keen on these configs being condensed like this in the source. As mentioned below, moving the QemuArchParams to a separate file helps, but even then it'd be nice to at least have each entry on a separate line like with 'arm' below.

The other option would be to have this as a path to a separate Kconfig/.kunitconfig file with these configs, rather than having them inline here. I fear that could lead to every architecture needing several support files, though some (e.g. x86/x86_64/alpha/sparc) could share a file.

•                           qemu_arch='x86_64',
  

•                           kernel_path='arch/x86/boot/bzImage',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['']),
  

•   'x86_64'        : QemuArchParams(linux_arch='x86_64',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
  

•                           qemu_arch='x86_64',
  

•                           kernel_path='arch/x86/boot/bzImage',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['']),
  

•   'arm'           : 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']),
  

•   'arm64'         : 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']),
  

•   'alpha'         : QemuArchParams(linux_arch='alpha',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y',
  

•                           qemu_arch='alpha',
  

•                           kernel_path='arch/alpha/boot/vmlinux',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['']),
  

•   'powerpc'       : QemuArchParams(linux_arch='powerpc',
  

•                           qemuconfig='CONFIG_PPC64=y\nCONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y\nCONFIG_HVC_CONSOLE=y',
  

•                           qemu_arch='ppc64',
  

•                           kernel_path='vmlinux',
  

•                           kernel_command_line='console=ttyS0',
  

•                           extra_qemu_params=['-M pseries', '-cpu power8']),
  

•   'riscv'         : QemuArchParams(linux_arch='riscv',
  

•                           qemuconfig='CONFIG_SOC_VIRT=y\nCONFIG_SERIAL_8250=y\nCONFIG_SERIAL_8250_CONSOLE=y\nCONFIG_SERIAL_OF_PLATFORM=y\nCONFIG_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']),
  

•   's390'          : QemuArchParams(linux_arch='s390',
  

•                           qemuconfig='CONFIG_EXPERT=y\nCONFIG_TUNE_ZEC12=y\nCONFIG_NUMA=y\nCONFIG_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',]),
  

•   'sparc'         : QemuArchParams(linux_arch='sparc',
  

•                           qemuconfig='CONFIG_SERIAL_8250=y\nCONFIG_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']),
  

+}

So, as mentioned, I agree with Daniel in thinking that this really doesn't belong here. Ideally, I think these configs should be specified somewhere else, either as a (alas, very long) series of command-line arguments, or in a separate file. Moving them to a single 'qemu_configs.py' as Daniel mentioned would be an improvement, but IMHO still falls well short of ideal. I think each config should be in its own file, as there are several reasons someone might want to have multiple different configs for a given architecture and we'll want people to be able to add/remove these easily in their own environments.

The proposed solution of having each config in a separate file -- which we discussed offline -- is a big improvement: https://kunit-review.googlesource.com/c/linux/+/4489

I still think that the python format here is a bit verbose and ugly, but it's probably worth it compared to adding a parser for a whole new type of config file. Similarly, I'm not sure about keeping a separate qemu_configs.py around with the builtin defaults rather than just having them all as separate files all the time. (Even appreciating that that requires some filename magic to find the correct file.) These are all more nitpicky though.

+class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):

•   def __init__(self, qemu_arch_params, cross_compile):
  

•           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
  

•           print(self._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, build_dir):
  

•           qemuconfig = kunit_config.Kconfig()
  

•           qemuconfig.parse_from_string(self._qemuconfig)
  

•           qemuconfig.write_to_file(get_kconfig_path(build_dir))
  

•   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(' '.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):
  

•           super().__init__(linux_arch='um', cross_compile=None)
  

Why is cross_compile None here? Shouldn't it be possible to cross compile UML kernels? (This should, I think, be possible for i386 on an x86_64 host, for instance.)

    def make_allyesconfig(self, build_dir, make_options) -> None:
            kunit_parser.print_with_timestamp(
                    'Enabling all CONFIGs for UML...')

@@ -83,32 +243,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: @@ -123,10 +267,17 @@ def get_outfile_path(build_dir) -> str: 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) -> None:
            signal.signal(signal.SIGINT, self.signal_handler)
  

•           self._ops = LinuxSourceTreeOperations()
  

•           self._ops = None
  

•           if arch is None or arch == 'um':
  

•                   self._arch = 'um'
  

•                   self._ops = LinuxSourceTreeOperationsUml()
  

Again, we should probably pass cross_compile through here.

•           elif arch in QEMU_ARCHS:
  

•                   self._arch = arch
  

•                   self._ops = LinuxSourceTreeOperationsQemu(QEMU_ARCHS[arch], cross_compile=cross_compile)
  

•           else:
  

•                   raise ConfigError(arch + ' is not a valid arch')
  
            if not load_config:
                    return
  

@@ -170,6 +321,7 @@ class LinuxSourceTree(object): os.mkdir(build_dir) self._kconfig.write_to_file(kconfig_path) try:

•                   self._ops.make_arch_qemuconfig(build_dir)
                    self._ops.make_olddefconfig(build_dir, make_options)
            except ConfigError as e:
                    logging.error(e)
  

@@ -192,10 +344,13 @@ 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:
  

•                           if self._arch != 'um':
  

•                                   raise ConfigError('Only the "um" arch is supported for alltests')
                            self._ops.make_allyesconfig(build_dir, make_options)
  

•                   self._ops.make_arch_qemuconfig(build_dir)
                    self._ops.make_olddefconfig(build_dir, make_options)
                    self._ops.make(jobs, build_dir, make_options)
            except (ConfigError, BuildError) as e:
  

@@ -209,8 +364,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 1ad3049e90698..25e8be95a575d 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -297,7 +297,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):
  

@@ -308,7 +308,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):
  

@@ -390,7 +390,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'
  

@@ -404,14 +404,19 @@ 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)
  
    @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')
  
  

if __name__ == '__main__': unittest.main() -- 2.31.1.498.g6c1eba8ee3d-goog