[PATCH v2 00/10] Kconfig vs. default devices

Fabiano Rosas posted 10 patches 1 year, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230208192654.8854-1-farosas@suse.de
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>
hw/arm/Kconfig  | 7 +++++++
hw/i386/Kconfig | 8 ++++----
hw/usb/Kconfig  | 1 -
3 files changed, 11 insertions(+), 5 deletions(-)
[PATCH v2 00/10] Kconfig vs. default devices
Posted by Fabiano Rosas 1 year, 2 months ago
v2:
Applying the feedback received, all small tweaks.

Patch 6 still needs consensus on whether to apply the fix to Kconfig
or elsewhere. Link to the previous version:
https://lore.kernel.org/r/461ba038-31bf-49c4-758b-94ece36f136f@redhat.com

changelog:

- patch 1: moved isa-parallel to a build time check like the other
           patches;
- patch 3: tweaked commit message;
- patch 7: removed the default from XLNX_USB_SUBSYS.

v1:
https://lore.kernel.org/r/20230206140809.26028-1-farosas@suse.de

We currently have a situation where disabling a Kconfig might result
in a runtime error when QEMU selects the corresponding device as a
default value for an option. But first a disambiguation:

Kconfig default::
  a device "Foo" for which there's "config FOO default y" or "config X
  imply FOO" in Kconfig.

QEMU hardcoded default::
  a fallback; a device "Foo" that is chosen in case no corresponding
  option is given in the command line.

The issue I'm trying to solve is that there is no link between the two
"defaults" above, which means that when the user at build time
de-selects a Kconfig default, either via configs/devices/*/*.mak or
--without-default-devices, the subsequent invocation at runtime might
continue to try to create the missing device due to QEMU defaults.

Even a experienced user that tweaks the build via .mak files is not
required to know about what QEMU developers chose to use as fallbacks
in the code. Moreover, the person/entity that builds the code might
not be the same that runs it, which makes it even more confusing.

We do have -nodefaults in the command line, but that doesn't include
all types of fallbacks that might be set in the code. It also does not
cover individual CONFIGs and their respective use as a fallback in the
code.

So my proposal here is actually simple: Let's make sure every fallback
device creation *without* a validation check gets a hard dependency in
Kconfig. A validation check being something like:

if (has_defaults && object_get_class("foo") {
   create_foo();
}

Fabiano Rosas (10):
  hw/i386: Select CONFIG_PARALLEL for PC machines
  hw/i386: Select E1000E for q35
  hw/i386: Select VGA_PCI in Kconfig
  hw/i386: Select E1000_PCI for i440fx
  hw/arm: Select VIRTIO_NET for virt machine
  hw/arm: Select VIRTIO_BLK for virt machine
  hw/arm: Select XLNX_USB_SUBSYS for xlnx-zcu102 machine
  hw/arm: Select GICV3_TCG for sbsa-ref machine
  hw/arm: Select e1000e for sbsa-ref machine
  hw/arm: Select VGA_PCI for sbsa-ref machine

 hw/arm/Kconfig  | 7 +++++++
 hw/i386/Kconfig | 8 ++++----
 hw/usb/Kconfig  | 1 -
 3 files changed, 11 insertions(+), 5 deletions(-)

-- 
2.35.3
Re: [PATCH v2 00/10] Kconfig vs. default devices
Posted by Alex Bennée 1 year ago
Fabiano Rosas <farosas@suse.de> writes:

> v2:
> Applying the feedback received, all small tweaks.
>
> Patch 6 still needs consensus on whether to apply the fix to Kconfig
> or elsewhere. Link to the previous version:
> https://lore.kernel.org/r/461ba038-31bf-49c4-758b-94ece36f136f@redhat.com
>
> changelog:
>
> - patch 1: moved isa-parallel to a build time check like the other
>            patches;
> - patch 3: tweaked commit message;
> - patch 7: removed the default from XLNX_USB_SUBSYS.

I've queued the ARM tweaks to testing/next where I can add a test for it
in the end. I'll leave the x86 stuff for discussion of the more complete
solution that avoids hacky downstream patching.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v2 00/10] Kconfig vs. default devices
Posted by Philippe Mathieu-Daudé 1 year, 2 months ago
On 8/2/23 20:26, Fabiano Rosas wrote:

> We currently have a situation where disabling a Kconfig might result
> in a runtime error when QEMU selects the corresponding device as a
> default value for an option. But first a disambiguation:
> 
> Kconfig default::
>    a device "Foo" for which there's "config FOO default y" or "config X
>    imply FOO" in Kconfig.
> 
> QEMU hardcoded default::
>    a fallback; a device "Foo" that is chosen in case no corresponding
>    option is given in the command line.
> 
> The issue I'm trying to solve is that there is no link between the two
> "defaults" above, which means that when the user at build time
> de-selects a Kconfig default, either via configs/devices/*/*.mak or
> --without-default-devices, the subsequent invocation at runtime might
> continue to try to create the missing device due to QEMU defaults.

This will keep bitrotting if we don't cover such configs in our CI.

Why do you want to get this fixed BTW? I'm not sure there is a big
interest (as in "almost no users").

I tried to do that few years ago [*] and Thomas said:

"in our CI, we should test what users really need,
  and not each and every distantly possible combination."

[*] 
https://lore.kernel.org/qemu-devel/81aca179-4320-f00b-d9dc-7eca449ebce7@redhat.com/

> Fabiano Rosas (10):
>    hw/i386: Select CONFIG_PARALLEL for PC machines
>    hw/i386: Select E1000E for q35
>    hw/i386: Select VGA_PCI in Kconfig
>    hw/i386: Select E1000_PCI for i440fx
>    hw/arm: Select VIRTIO_NET for virt machine
>    hw/arm: Select VIRTIO_BLK for virt machine
>    hw/arm: Select XLNX_USB_SUBSYS for xlnx-zcu102 machine
>    hw/arm: Select GICV3_TCG for sbsa-ref machine
>    hw/arm: Select e1000e for sbsa-ref machine
>    hw/arm: Select VGA_PCI for sbsa-ref machine
> 
>   hw/arm/Kconfig  | 7 +++++++
>   hw/i386/Kconfig | 8 ++++----
>   hw/usb/Kconfig  | 1 -
>   3 files changed, 11 insertions(+), 5 deletions(-)
>
Re: [PATCH v2 00/10] Kconfig vs. default devices
Posted by Thomas Huth 1 year, 2 months ago
On 08/02/2023 20.43, Philippe Mathieu-Daudé wrote:
> On 8/2/23 20:26, Fabiano Rosas wrote:
> 
>> We currently have a situation where disabling a Kconfig might result
>> in a runtime error when QEMU selects the corresponding device as a
>> default value for an option. But first a disambiguation:
>>
>> Kconfig default::
>>    a device "Foo" for which there's "config FOO default y" or "config X
>>    imply FOO" in Kconfig.
>>
>> QEMU hardcoded default::
>>    a fallback; a device "Foo" that is chosen in case no corresponding
>>    option is given in the command line.
>>
>> The issue I'm trying to solve is that there is no link between the two
>> "defaults" above, which means that when the user at build time
>> de-selects a Kconfig default, either via configs/devices/*/*.mak or
>> --without-default-devices, the subsequent invocation at runtime might
>> continue to try to create the missing device due to QEMU defaults.
> 
> This will keep bitrotting if we don't cover such configs in our CI.
> 
> Why do you want to get this fixed BTW? I'm not sure there is a big
> interest (as in "almost no users").
> 
> I tried to do that few years ago [*] and Thomas said:
> 
> "in our CI, we should test what users really need,
>   and not each and every distantly possible combination."

You're mis-quoting me here. That comment was made when we were talking about 
very arbitrary configs that likely nobody is going to use.
Fabiano's series here is about the --without-default-devices configure 
option which everybody could add to their set of "configure" options easily.

  Thomas


Re: [PATCH v2 00/10] Kconfig vs. default devices
Posted by Alex Bennée 1 year ago
Thomas Huth <thuth@redhat.com> writes:

> On 08/02/2023 20.43, Philippe Mathieu-Daudé wrote:
>> On 8/2/23 20:26, Fabiano Rosas wrote:
>> 
>>> We currently have a situation where disabling a Kconfig might result
>>> in a runtime error when QEMU selects the corresponding device as a
>>> default value for an option. But first a disambiguation:
>>>
>>> Kconfig default::
>>>    a device "Foo" for which there's "config FOO default y" or "config X
>>>    imply FOO" in Kconfig.
>>>
>>> QEMU hardcoded default::
>>>    a fallback; a device "Foo" that is chosen in case no corresponding
>>>    option is given in the command line.
>>>
>>> The issue I'm trying to solve is that there is no link between the two
>>> "defaults" above, which means that when the user at build time
>>> de-selects a Kconfig default, either via configs/devices/*/*.mak or
>>> --without-default-devices, the subsequent invocation at runtime might
>>> continue to try to create the missing device due to QEMU defaults.
>> This will keep bitrotting if we don't cover such configs in our CI.
>> Why do you want to get this fixed BTW? I'm not sure there is a big
>> interest (as in "almost no users").
>> I tried to do that few years ago [*] and Thomas said:
>> "in our CI, we should test what users really need,
>>   and not each and every distantly possible combination."
>
> You're mis-quoting me here. That comment was made when we were talking
> about very arbitrary configs that likely nobody is going to use.
> Fabiano's series here is about the --without-default-devices configure
> option which everybody could add to their set of "configure" options
> easily.

Indeed - while trying to reduce the compile time I ran into this with a
plain --without-default-devices check. We also have in the meantime
introduced --with-devices-FOO so we can do minimal builds.

>
>  Thomas


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v2 00/10] Kconfig vs. default devices
Posted by Fabiano Rosas 1 year, 2 months ago
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 8/2/23 20:26, Fabiano Rosas wrote:
>
>> We currently have a situation where disabling a Kconfig might result
>> in a runtime error when QEMU selects the corresponding device as a
>> default value for an option. But first a disambiguation:
>> 
>> Kconfig default::
>>    a device "Foo" for which there's "config FOO default y" or "config X
>>    imply FOO" in Kconfig.
>> 
>> QEMU hardcoded default::
>>    a fallback; a device "Foo" that is chosen in case no corresponding
>>    option is given in the command line.
>> 
>> The issue I'm trying to solve is that there is no link between the two
>> "defaults" above, which means that when the user at build time
>> de-selects a Kconfig default, either via configs/devices/*/*.mak or
>> --without-default-devices, the subsequent invocation at runtime might
>> continue to try to create the missing device due to QEMU defaults.
>
> This will keep bitrotting if we don't cover such configs in our CI.
>
> Why do you want to get this fixed BTW? I'm not sure there is a big
> interest (as in "almost no users").
>
> I tried to do that few years ago [*] and Thomas said:
>
> "in our CI, we should test what users really need,
>   and not each and every distantly possible combination."
>
> [*] 
> https://lore.kernel.org/qemu-devel/81aca179-4320-f00b-d9dc-7eca449ebce7@redhat.com/

If we're talking about turning the defaults off
(--without-default-devices), we already have that in the CI, we just
cannot ensure that it works because we cannot run 'make check' on
it. I'm just trying to improve that situation.

(not sure if that is clear, but this goes along with this other series
https://lore.kernel.org/r/20230208194700.11035-1-farosas@suse.de to fix
make check)

If we're talking about general fiddling with CONFIGS, then I'm inclined
to agree with Thomas that the CI doesn't need to test every single
combination. However, it's a basic maintenance task to ensure that if
your project has toggles, that they can actually be toggled.

As for use-cases, I don't have a specific one for disabling all the
defaults. For individual CONFIGs I would like to be able to produce a
slimmer build sometime in the distant future once we untangle everything
that's tangled today.