diff mbox series

kunit: tool: refactor internal kconfig handling, allow overriding

Message ID 20220519164512.3180360-1-dlatypov@google.com (mailing list archive)
State New
Delegated to: Brendan Higgins
Headers show
Series kunit: tool: refactor internal kconfig handling, allow overriding | expand

Commit Message

Daniel Latypov May 19, 2022, 4:45 p.m. UTC
Currently, you cannot ovewrwrite what's in your kunitconfig via
--kconfig_add.
Nor can you override something in a qemu_config via either means.

This patch makes it so we have this level of priority
* --kconfig_add
* kunitconfig file (the default or the one from --kunitconfig)
* qemu_config

The rationale for this order is that the more "dynamic" sources of
kconfig options should take priority.

--kconfig_add is obviously the most dynamic.
And for kunitconfig, users probably tweak the file manually or specify
--kunitconfig more often than they delve into qemu_config python files.

And internally, we convert the kconfigs from a python list into a set or
dict fairly often. We should just use a dict internally.
We exposed the set transform in the past since we didn't define __eq__,
so also take the chance to shore up the kunit_kconfig.Kconfig interface.

Example
=======

Let's consider the unrealistic example where someone would want to
disable CONFIG_KUNIT.
I.e. they run
$ ./tools/testing/kunit/kunit.py config --kconfig_add=CONFIG_KUNIT=n

Before
------
We'd write the following
> # CONFIG_KUNIT is not set
> CONFIG_KUNIT_ALL_TESTS=y
> CONFIG_KUNIT_TEST=y
> CONFIG_KUNIT=y
> CONFIG_KUNIT_EXAMPLE_TEST=y

And we'd error out with
> ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
> This is probably due to unsatisfied dependencies.
> Missing: # CONFIG_KUNIT is not set

After
-----
We'd write the following
> # CONFIG_KUNIT is not set
> CONFIG_KUNIT_TEST=y
> CONFIG_KUNIT_ALL_TESTS=y
> CONFIG_KUNIT_EXAMPLE_TEST=y

And we'd error out with
> ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
> This is probably due to unsatisfied dependencies.
> Missing: CONFIG_KUNIT_EXAMPLE_TEST=y, CONFIG_KUNIT_TEST=y, CONFIG_KUNIT_ALL_TESTS=y

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 tools/testing/kunit/kunit_config.py    | 49 +++++++++++++++-----------
 tools/testing/kunit/kunit_kernel.py    | 21 ++++++-----
 tools/testing/kunit/kunit_tool_test.py | 45 ++++++++++-------------
 3 files changed, 59 insertions(+), 56 deletions(-)


base-commit: 1b11063d32d7e11366e48be64215ff517ce32217

Comments

David Gow May 20, 2022, 6:12 a.m. UTC | #1
On Fri, May 20, 2022 at 12:45 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> Currently, you cannot ovewrwrite what's in your kunitconfig via
> --kconfig_add.
> Nor can you override something in a qemu_config via either means.
>
> This patch makes it so we have this level of priority
> * --kconfig_add
> * kunitconfig file (the default or the one from --kunitconfig)
> * qemu_config
>
> The rationale for this order is that the more "dynamic" sources of
> kconfig options should take priority.
>
> --kconfig_add is obviously the most dynamic.
> And for kunitconfig, users probably tweak the file manually or specify
> --kunitconfig more often than they delve into qemu_config python files.
>
> And internally, we convert the kconfigs from a python list into a set or
> dict fairly often. We should just use a dict internally.
> We exposed the set transform in the past since we didn't define __eq__,
> so also take the chance to shore up the kunit_kconfig.Kconfig interface.
>
> Example
> =======
>
> Let's consider the unrealistic example where someone would want to
> disable CONFIG_KUNIT.
> I.e. they run
> $ ./tools/testing/kunit/kunit.py config --kconfig_add=CONFIG_KUNIT=n
>
> Before
> ------
> We'd write the following
> > # CONFIG_KUNIT is not set
> > CONFIG_KUNIT_ALL_TESTS=y
> > CONFIG_KUNIT_TEST=y
> > CONFIG_KUNIT=y
> > CONFIG_KUNIT_EXAMPLE_TEST=y
>
> And we'd error out with
> > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
> > This is probably due to unsatisfied dependencies.
> > Missing: # CONFIG_KUNIT is not set
>
> After
> -----
> We'd write the following
> > # CONFIG_KUNIT is not set
> > CONFIG_KUNIT_TEST=y
> > CONFIG_KUNIT_ALL_TESTS=y
> > CONFIG_KUNIT_EXAMPLE_TEST=y
>
> And we'd error out with
> > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
> > This is probably due to unsatisfied dependencies.
> > Missing: CONFIG_KUNIT_EXAMPLE_TEST=y, CONFIG_KUNIT_TEST=y, CONFIG_KUNIT_ALL_TESTS=y
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---

I like this, but do think there are a few gaps this doesn't handle
properly. (Though exactly how we'd deal with them, I'm not yet sure.)

In particular, it's not possible to disable a pair of options where
one depends on the other: disabling the parent option will result in
the child one not being present in the generated config. This will
conflict both with "=y" and "=n/not set": we'd need a way to _remove_
a kconfig option for that to work.

The ideal thing would be for us to work out what the default value is,
and remove the option automatically if it matches, but that seems like
it'd be quite difficult. Otherwise, I guess a
--kconfig_{remove,delete,etc} option would work.

Otherwise, this seems okay at first glance, but I haven't had the time
to fully review it in detail yet...

Cheers,
-- David

>  tools/testing/kunit/kunit_config.py    | 49 +++++++++++++++-----------
>  tools/testing/kunit/kunit_kernel.py    | 21 ++++++-----
>  tools/testing/kunit/kunit_tool_test.py | 45 ++++++++++-------------
>  3 files changed, 59 insertions(+), 56 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py
> index 75a8dc1683d4..89443400b17e 100644
> --- a/tools/testing/kunit/kunit_config.py
> +++ b/tools/testing/kunit/kunit_config.py
> @@ -8,7 +8,7 @@
>
>  from dataclasses import dataclass
>  import re
> -from typing import List, Set
> +from typing import Dict, Iterable, Set
>
>  CONFIG_IS_NOT_SET_PATTERN = r'^# CONFIG_(\w+) is not set$'
>  CONFIG_PATTERN = r'^CONFIG_(\w+)=(\S+|".*")$'
> @@ -32,35 +32,46 @@ class Kconfig:
>         """Represents defconfig or .config specified using the Kconfig language."""
>
>         def __init__(self) -> None:
> -               self._entries = []  # type: List[KconfigEntry]
> +               self._entries = {}  # type: Dict[str, str]
>
> -       def entries(self) -> Set[KconfigEntry]:
> -               return set(self._entries)
> +       def __eq__(self, other) -> bool:
> +               if not isinstance(other, self.__class__):
> +                       return False
> +               return self._entries == other._entries
>
> -       def add_entry(self, entry: KconfigEntry) -> None:
> -               self._entries.append(entry)
> +       def __repr__(self) -> str:
> +               return ','.join(str(e) for e in self._as_entries())
> +
> +
> +       def _as_entries(self) -> Iterable[KconfigEntry]:
> +               for name, value in self._entries.items():
> +                       yield KconfigEntry(name, value)
> +
> +       def add_entry(self, name: str, value: str) -> None:
> +               self._entries[name] = value
>
>         def is_subset_of(self, other: 'Kconfig') -> bool:
> -               other_dict = {e.name: e.value for e in other.entries()}
> -               for a in self.entries():
> -                       b = other_dict.get(a.name)
> +               for name, value in self._entries.items():
> +                       b = other._entries.get(name)
>                         if b is None:
> -                               if a.value == 'n':
> +                               if value == 'n':
>                                         continue
>                                 return False
> -                       if a.value != b:
> +                       if value != b:
>                                 return False
>                 return True
>
> +       def set_diff(self, other: 'Kconfig') -> Set[KconfigEntry]:
> +               return set(self._as_entries()) - set(other._as_entries())
> +
>         def merge_in_entries(self, other: 'Kconfig') -> None:
> -               if other.is_subset_of(self):
> -                       return
> -               self._entries = list(self.entries().union(other.entries()))
> +               for name, value in other._entries.items():
> +                       self._entries[name] = value
>
>         def write_to_file(self, path: str) -> None:
>                 with open(path, 'a+') as f:
> -                       for entry in self.entries():
> -                               f.write(str(entry) + '\n')
> +                       for e in self._as_entries():
> +                               f.write(str(e) + '\n')
>
>  def parse_file(path: str) -> Kconfig:
>         with open(path, 'r') as f:
> @@ -78,14 +89,12 @@ def parse_from_string(blob: str) -> Kconfig:
>
>                 match = config_matcher.match(line)
>                 if match:
> -                       entry = KconfigEntry(match.group(1), match.group(2))
> -                       kconfig.add_entry(entry)
> +                       kconfig.add_entry(match.group(1), match.group(2))
>                         continue
>
>                 empty_match = is_not_set_matcher.match(line)
>                 if empty_match:
> -                       entry = KconfigEntry(empty_match.group(1), 'n')
> -                       kconfig.add_entry(entry)
> +                       kconfig.add_entry(empty_match.group(1), 'n')
>                         continue
>
>                 if line[0] == '#':
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index 3539efaf99ba..ebd2d91af710 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -53,8 +53,8 @@ class LinuxSourceTreeOperations:
>                 except subprocess.CalledProcessError as e:
>                         raise ConfigError(e.output.decode())
>
> -       def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None:
> -               pass
> +       def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> +               return base_kunitconfig
>
>         def make_allyesconfig(self, build_dir: str, make_options) -> None:
>                 raise ConfigError('Only the "um" arch is supported for alltests')
> @@ -109,9 +109,10 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
>                 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:
> +       def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
>                 kconfig = kunit_config.parse_from_string(self._kconfig)
> -               base_kunitconfig.merge_in_entries(kconfig)
> +               kconfig.merge_in_entries(base_kunitconfig)
> +               return kconfig
>
>         def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
>                 kernel_path = os.path.join(build_dir, self._kernel_path)
> @@ -265,9 +266,10 @@ class LinuxSourceTree:
>         def validate_config(self, build_dir: str) -> bool:
>                 kconfig_path = get_kconfig_path(build_dir)
>                 validated_kconfig = kunit_config.parse_file(kconfig_path)
> -               if self._kconfig.is_subset_of(validated_kconfig):
> +               invalid = self._kconfig.set_diff(validated_kconfig)
> +               if not invalid:
>                         return True
> -               invalid = self._kconfig.entries() - validated_kconfig.entries()
> +
>                 message = 'Not all Kconfig options selected in kunitconfig were in the generated .config.\n' \
>                           'This is probably due to unsatisfied dependencies.\n' \
>                           'Missing: ' + ', '.join([str(e) for e in invalid])
> @@ -282,7 +284,7 @@ class LinuxSourceTree:
>                 if build_dir and not os.path.exists(build_dir):
>                         os.mkdir(build_dir)
>                 try:
> -                       self._ops.make_arch_qemuconfig(self._kconfig)
> +                       self._kconfig = 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:
> @@ -303,7 +305,7 @@ class LinuxSourceTree:
>                         return True
>
>                 old_kconfig = kunit_config.parse_file(old_path)
> -               return old_kconfig.entries() != self._kconfig.entries()
> +               return old_kconfig != self._kconfig
>
>         def build_reconfig(self, build_dir: str, make_options) -> bool:
>                 """Creates a new .config if it is not a subset of the .kunitconfig."""
> @@ -313,7 +315,8 @@ class LinuxSourceTree:
>                         return self.build_config(build_dir, make_options)
>
>                 existing_kconfig = kunit_config.parse_file(kconfig_path)
> -               self._ops.make_arch_qemuconfig(self._kconfig)
> +               self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
> +
>                 if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir):
>                         return True
>                 print('Regenerating .config ...')
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index 25a2eb3bf114..3a8f638ff092 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -45,7 +45,7 @@ class KconfigTest(unittest.TestCase):
>                 self.assertTrue(kconfig0.is_subset_of(kconfig0))
>
>                 kconfig1 = kunit_config.Kconfig()
> -               kconfig1.add_entry(kunit_config.KconfigEntry('TEST', 'y'))
> +               kconfig1.add_entry('TEST', 'y')
>                 self.assertTrue(kconfig1.is_subset_of(kconfig1))
>                 self.assertTrue(kconfig0.is_subset_of(kconfig1))
>                 self.assertFalse(kconfig1.is_subset_of(kconfig0))
> @@ -56,40 +56,28 @@ class KconfigTest(unittest.TestCase):
>                 kconfig = kunit_config.parse_file(kconfig_path)
>
>                 expected_kconfig = kunit_config.Kconfig()
> -               expected_kconfig.add_entry(
> -                       kunit_config.KconfigEntry('UML', 'y'))
> -               expected_kconfig.add_entry(
> -                       kunit_config.KconfigEntry('MMU', 'y'))
> -               expected_kconfig.add_entry(
> -                       kunit_config.KconfigEntry('TEST', 'y'))
> -               expected_kconfig.add_entry(
> -                       kunit_config.KconfigEntry('EXAMPLE_TEST', 'y'))
> -               expected_kconfig.add_entry(
> -                       kunit_config.KconfigEntry('MK8', 'n'))
> -
> -               self.assertEqual(kconfig.entries(), expected_kconfig.entries())
> +               expected_kconfig.add_entry('UML', 'y')
> +               expected_kconfig.add_entry('MMU', 'y')
> +               expected_kconfig.add_entry('TEST', 'y')
> +               expected_kconfig.add_entry('EXAMPLE_TEST', 'y')
> +               expected_kconfig.add_entry('MK8', 'n')
> +
> +               self.assertEqual(kconfig, expected_kconfig)
>
>         def test_write_to_file(self):
>                 kconfig_path = os.path.join(test_tmpdir, '.config')
>
>                 expected_kconfig = kunit_config.Kconfig()
> -               expected_kconfig.add_entry(
> -                       kunit_config.KconfigEntry('UML', 'y'))
> -               expected_kconfig.add_entry(
> -                       kunit_config.KconfigEntry('MMU', 'y'))
> -               expected_kconfig.add_entry(
> -                       kunit_config.KconfigEntry('TEST', 'y'))
> -               expected_kconfig.add_entry(
> -                       kunit_config.KconfigEntry('EXAMPLE_TEST', 'y'))
> -               expected_kconfig.add_entry(
> -                       kunit_config.KconfigEntry('MK8', 'n'))
> +               expected_kconfig.add_entry('UML', 'y')
> +               expected_kconfig.add_entry('MMU', 'y')
> +               expected_kconfig.add_entry('TEST', 'y')
> +               expected_kconfig.add_entry('EXAMPLE_TEST', 'y')
> +               expected_kconfig.add_entry('MK8', 'n')
>
>                 expected_kconfig.write_to_file(kconfig_path)
>
>                 actual_kconfig = kunit_config.parse_file(kconfig_path)
> -
> -               self.assertEqual(actual_kconfig.entries(),
> -                                expected_kconfig.entries())
> +               self.assertEqual(actual_kconfig, expected_kconfig)
>
>  class KUnitParserTest(unittest.TestCase):
>
> @@ -381,8 +369,11 @@ class LinuxSourceTreeTest(unittest.TestCase):
>                         kunit_kernel.LinuxSourceTree('', kunitconfig_path=dir)
>
>         def test_kconfig_add(self):
> +               want_kconfig = kunit_config.Kconfig()
> +               want_kconfig.add_entry('NOT_REAL', 'y')
> +
>                 tree = kunit_kernel.LinuxSourceTree('', kconfig_add=['CONFIG_NOT_REAL=y'])
> -               self.assertIn(kunit_config.KconfigEntry('NOT_REAL', 'y'), tree._kconfig.entries())
> +               self.assertFalse(want_kconfig.set_diff(tree._kconfig))
>
>         def test_invalid_arch(self):
>                 with self.assertRaisesRegex(kunit_kernel.ConfigError, 'not a valid arch, options are.*x86_64'):
>
> base-commit: 1b11063d32d7e11366e48be64215ff517ce32217
> --
> 2.36.1.124.g0e6072fb45-goog
>
Daniel Latypov May 20, 2022, 6:13 p.m. UTC | #2
On Thu, May 19, 2022 at 11:13 PM David Gow <davidgow@google.com> wrote:
>
> I like this, but do think there are a few gaps this doesn't handle
> properly. (Though exactly how we'd deal with them, I'm not yet sure.)
>
> In particular, it's not possible to disable a pair of options where
> one depends on the other: disabling the parent option will result in
> the child one not being present in the generated config. This will
> conflict both with "=y" and "=n/not set": we'd need a way to _remove_
> a kconfig option for that to work.

Do you have an example?
Because what you describe sounds like how we want it to work, but I'm
not sure if I'm misunderstanding the scenario you describe.

I was considering the case mentioned in the commit description.
I.e. we do --kunitconfig_add=CONFIG_KUNIT=n to the default kunitconfig.
That gives us complaints about these
 CONFIG_KUNIT_EXAMPLE_TEST=y, CONFIG_KUNIT_TEST=y,
 CONFIG_KUNIT_ALL_TESTS=y
options no longer being in the generated .config.
And I think that's exactly how it _should_ work, as this flag is a
low-level tool for tweaking individual options.

IMO, anything complicated should be done by editing the
kunitconfig/qemu_config files, in which case it's a lot less
cumbersome to disable multiple options by just deleting them.

Daniel
Daniel Latypov May 20, 2022, 10:13 p.m. UTC | #3
On Fri, May 20, 2022 at 11:13 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> On Thu, May 19, 2022 at 11:13 PM David Gow <davidgow@google.com> wrote:
> >
> > I like this, but do think there are a few gaps this doesn't handle
> > properly. (Though exactly how we'd deal with them, I'm not yet sure.)
> >
> > In particular, it's not possible to disable a pair of options where
> > one depends on the other: disabling the parent option will result in
> > the child one not being present in the generated config. This will
> > conflict both with "=y" and "=n/not set": we'd need a way to _remove_
> > a kconfig option for that to work.
>
> Do you have an example?
> Because what you describe sounds like how we want it to work, but I'm
> not sure if I'm misunderstanding the scenario you describe.

Talking offline with David, we've come up with a small example.

If we add this kconfig somewhere
+config X
+       bool "X"
+       default y
+
+config Y
+       bool "Y"
+       default y
+       depends on X
+

Then running this will fail
$ ./tools/testing/kunit/kunit.py config --kunitconfig xy_kunitconfig
--kconfig_add=CONFIG_X=n --kconfig_add=CONFIG_Y=n

It will fail with this
This is probably due to unsatisfied dependencies.
Missing: # CONFIG_Y is not set

The problem is that kunit.py is looking for an explicit line saying
CONFIG_Y is not set.
But CONFIG_Y's dependencies are not met, so Kconfig doesn't write it out.

I assume we can treat the absence of it in the file as proof that it's not set.
I.e. the bug lies in the is_subset() logic we have in kunit.py?

I think there's also a general problem with kunit.py/kunitconfig not
really handling disabling options properly.
Daniel Latypov May 20, 2022, 10:35 p.m. UTC | #4
On Fri, May 20, 2022 at 3:13 PM Daniel Latypov <dlatypov@google.com> wrote:
> Talking offline with David, we've come up with a small example.
>
> If we add this kconfig somewhere
> +config X
> +       bool "X"
> +       default y
> +
> +config Y
> +       bool "Y"
> +       default y
> +       depends on X
> +
>
> Then running this will fail
> $ ./tools/testing/kunit/kunit.py config --kunitconfig xy_kunitconfig
> --kconfig_add=CONFIG_X=n --kconfig_add=CONFIG_Y=n
>
> It will fail with this
> This is probably due to unsatisfied dependencies.
> Missing: # CONFIG_Y is not set
>
> The problem is that kunit.py is looking for an explicit line saying
> CONFIG_Y is not set.
> But CONFIG_Y's dependencies are not met, so Kconfig doesn't write it out.
>
> I assume we can treat the absence of it in the file as proof that it's not set.
> I.e. the bug lies in the is_subset() logic we have in kunit.py?

Ah no, this is just me trying to be clever and avoid a call to this logic.
I tried to use set_diff() and check if that's empty as an alternative.
But the set_diff() logic is not aware of how to treat the absence of
options properly.

I'll send a v2 w/ a fix.
diff mbox series

Patch

diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py
index 75a8dc1683d4..89443400b17e 100644
--- a/tools/testing/kunit/kunit_config.py
+++ b/tools/testing/kunit/kunit_config.py
@@ -8,7 +8,7 @@ 
 
 from dataclasses import dataclass
 import re
-from typing import List, Set
+from typing import Dict, Iterable, Set
 
 CONFIG_IS_NOT_SET_PATTERN = r'^# CONFIG_(\w+) is not set$'
 CONFIG_PATTERN = r'^CONFIG_(\w+)=(\S+|".*")$'
@@ -32,35 +32,46 @@  class Kconfig:
 	"""Represents defconfig or .config specified using the Kconfig language."""
 
 	def __init__(self) -> None:
-		self._entries = []  # type: List[KconfigEntry]
+		self._entries = {}  # type: Dict[str, str]
 
-	def entries(self) -> Set[KconfigEntry]:
-		return set(self._entries)
+	def __eq__(self, other) -> bool:
+		if not isinstance(other, self.__class__):
+			return False
+		return self._entries == other._entries
 
-	def add_entry(self, entry: KconfigEntry) -> None:
-		self._entries.append(entry)
+	def __repr__(self) -> str:
+		return ','.join(str(e) for e in self._as_entries())
+
+
+	def _as_entries(self) -> Iterable[KconfigEntry]:
+		for name, value in self._entries.items():
+			yield KconfigEntry(name, value)
+
+	def add_entry(self, name: str, value: str) -> None:
+		self._entries[name] = value
 
 	def is_subset_of(self, other: 'Kconfig') -> bool:
-		other_dict = {e.name: e.value for e in other.entries()}
-		for a in self.entries():
-			b = other_dict.get(a.name)
+		for name, value in self._entries.items():
+			b = other._entries.get(name)
 			if b is None:
-				if a.value == 'n':
+				if value == 'n':
 					continue
 				return False
-			if a.value != b:
+			if value != b:
 				return False
 		return True
 
+	def set_diff(self, other: 'Kconfig') -> Set[KconfigEntry]:
+		return set(self._as_entries()) - set(other._as_entries())
+
 	def merge_in_entries(self, other: 'Kconfig') -> None:
-		if other.is_subset_of(self):
-			return
-		self._entries = list(self.entries().union(other.entries()))
+		for name, value in other._entries.items():
+			self._entries[name] = value
 
 	def write_to_file(self, path: str) -> None:
 		with open(path, 'a+') as f:
-			for entry in self.entries():
-				f.write(str(entry) + '\n')
+			for e in self._as_entries():
+				f.write(str(e) + '\n')
 
 def parse_file(path: str) -> Kconfig:
 	with open(path, 'r') as f:
@@ -78,14 +89,12 @@  def parse_from_string(blob: str) -> Kconfig:
 
 		match = config_matcher.match(line)
 		if match:
-			entry = KconfigEntry(match.group(1), match.group(2))
-			kconfig.add_entry(entry)
+			kconfig.add_entry(match.group(1), match.group(2))
 			continue
 
 		empty_match = is_not_set_matcher.match(line)
 		if empty_match:
-			entry = KconfigEntry(empty_match.group(1), 'n')
-			kconfig.add_entry(entry)
+			kconfig.add_entry(empty_match.group(1), 'n')
 			continue
 
 		if line[0] == '#':
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 3539efaf99ba..ebd2d91af710 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -53,8 +53,8 @@  class LinuxSourceTreeOperations:
 		except subprocess.CalledProcessError as e:
 			raise ConfigError(e.output.decode())
 
-	def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None:
-		pass
+	def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
+		return base_kunitconfig
 
 	def make_allyesconfig(self, build_dir: str, make_options) -> None:
 		raise ConfigError('Only the "um" arch is supported for alltests')
@@ -109,9 +109,10 @@  class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
 		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:
+	def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
 		kconfig = kunit_config.parse_from_string(self._kconfig)
-		base_kunitconfig.merge_in_entries(kconfig)
+		kconfig.merge_in_entries(base_kunitconfig)
+		return kconfig
 
 	def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
 		kernel_path = os.path.join(build_dir, self._kernel_path)
@@ -265,9 +266,10 @@  class LinuxSourceTree:
 	def validate_config(self, build_dir: str) -> bool:
 		kconfig_path = get_kconfig_path(build_dir)
 		validated_kconfig = kunit_config.parse_file(kconfig_path)
-		if self._kconfig.is_subset_of(validated_kconfig):
+		invalid = self._kconfig.set_diff(validated_kconfig)
+		if not invalid:
 			return True
-		invalid = self._kconfig.entries() - validated_kconfig.entries()
+
 		message = 'Not all Kconfig options selected in kunitconfig were in the generated .config.\n' \
 			  'This is probably due to unsatisfied dependencies.\n' \
 			  'Missing: ' + ', '.join([str(e) for e in invalid])
@@ -282,7 +284,7 @@  class LinuxSourceTree:
 		if build_dir and not os.path.exists(build_dir):
 			os.mkdir(build_dir)
 		try:
-			self._ops.make_arch_qemuconfig(self._kconfig)
+			self._kconfig = 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:
@@ -303,7 +305,7 @@  class LinuxSourceTree:
 			return True
 
 		old_kconfig = kunit_config.parse_file(old_path)
-		return old_kconfig.entries() != self._kconfig.entries()
+		return old_kconfig != self._kconfig
 
 	def build_reconfig(self, build_dir: str, make_options) -> bool:
 		"""Creates a new .config if it is not a subset of the .kunitconfig."""
@@ -313,7 +315,8 @@  class LinuxSourceTree:
 			return self.build_config(build_dir, make_options)
 
 		existing_kconfig = kunit_config.parse_file(kconfig_path)
-		self._ops.make_arch_qemuconfig(self._kconfig)
+		self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
+
 		if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir):
 			return True
 		print('Regenerating .config ...')
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 25a2eb3bf114..3a8f638ff092 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -45,7 +45,7 @@  class KconfigTest(unittest.TestCase):
 		self.assertTrue(kconfig0.is_subset_of(kconfig0))
 
 		kconfig1 = kunit_config.Kconfig()
-		kconfig1.add_entry(kunit_config.KconfigEntry('TEST', 'y'))
+		kconfig1.add_entry('TEST', 'y')
 		self.assertTrue(kconfig1.is_subset_of(kconfig1))
 		self.assertTrue(kconfig0.is_subset_of(kconfig1))
 		self.assertFalse(kconfig1.is_subset_of(kconfig0))
@@ -56,40 +56,28 @@  class KconfigTest(unittest.TestCase):
 		kconfig = kunit_config.parse_file(kconfig_path)
 
 		expected_kconfig = kunit_config.Kconfig()
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('UML', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('MMU', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('TEST', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('EXAMPLE_TEST', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('MK8', 'n'))
-
-		self.assertEqual(kconfig.entries(), expected_kconfig.entries())
+		expected_kconfig.add_entry('UML', 'y')
+		expected_kconfig.add_entry('MMU', 'y')
+		expected_kconfig.add_entry('TEST', 'y')
+		expected_kconfig.add_entry('EXAMPLE_TEST', 'y')
+		expected_kconfig.add_entry('MK8', 'n')
+
+		self.assertEqual(kconfig, expected_kconfig)
 
 	def test_write_to_file(self):
 		kconfig_path = os.path.join(test_tmpdir, '.config')
 
 		expected_kconfig = kunit_config.Kconfig()
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('UML', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('MMU', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('TEST', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('EXAMPLE_TEST', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('MK8', 'n'))
+		expected_kconfig.add_entry('UML', 'y')
+		expected_kconfig.add_entry('MMU', 'y')
+		expected_kconfig.add_entry('TEST', 'y')
+		expected_kconfig.add_entry('EXAMPLE_TEST', 'y')
+		expected_kconfig.add_entry('MK8', 'n')
 
 		expected_kconfig.write_to_file(kconfig_path)
 
 		actual_kconfig = kunit_config.parse_file(kconfig_path)
-
-		self.assertEqual(actual_kconfig.entries(),
-				 expected_kconfig.entries())
+		self.assertEqual(actual_kconfig, expected_kconfig)
 
 class KUnitParserTest(unittest.TestCase):
 
@@ -381,8 +369,11 @@  class LinuxSourceTreeTest(unittest.TestCase):
 			kunit_kernel.LinuxSourceTree('', kunitconfig_path=dir)
 
 	def test_kconfig_add(self):
+		want_kconfig = kunit_config.Kconfig()
+		want_kconfig.add_entry('NOT_REAL', 'y')
+
 		tree = kunit_kernel.LinuxSourceTree('', kconfig_add=['CONFIG_NOT_REAL=y'])
-		self.assertIn(kunit_config.KconfigEntry('NOT_REAL', 'y'), tree._kconfig.entries())
+		self.assertFalse(want_kconfig.set_diff(tree._kconfig))
 
 	def test_invalid_arch(self):
 		with self.assertRaisesRegex(kunit_kernel.ConfigError, 'not a valid arch, options are.*x86_64'):