Re: [PATCH] x86/x86_64_defconfig: Enable the serial console

[Guillaume Tucker]

On 12/10/2020 04:58, Willy Tarreau wrote:
> Hi Enric,
> 
> On Sun, Oct 11, 2020 at 07:05:55PM +0200, Enric Balletbo i Serra wrote:
>> For arm64 (i.e : arm64_defconfig):
>>     1. Someone renames CONFIG_A to CONFIG_AB, sends a patch, and as he did a
>> grep, the patch modifies all the defconfigs.
>>     2. The patch is accepted and merged in linux-next.
>>     3. KernelCI builds linux-next, boots the kernel on the hardware and all the
>> tests continue passing.
>>
>>
>> For x86:
>>     1. Someone renames CONFIG_A to CONFIG_AB, sends a patch and as he did a grep
>> the patches modifies all the defconfigs.
>>     2. The patch is accepted and merged in linux-next.
>>     3. KernelCI builds linux-next, boots the kernel on the hardware, and some
>> tests start to fail or are skipped.
>>     4. The maintainer is noticed about the behavior change, so he will need to
>> look at the problem, and find it.
>>     5. The maintainer sends a patch.
>>     6. The patch is accepted, but he needs to tag the release as per kernel <
>> x.y.z version it should use CONFIG_A and for kernel > x.y.z it should pick
>> CONFIG_AB.
>>     7. KernelCI builds linux-next, boots the kernel on the hardware and all the
>> tests pass again.
> 
> Previously I thought I understood your needs, but now I don't anymore. You
> seem to be saying that you're not testing *anything* outside of defconfig,
> and that as such you'd like defconfig to be complete enough to provide good
> coverage. This sounds a bit odd to me. And what if in the arm64 case, the
> CONFIG_YOUR_V4L2_DEVICE is *not* added to defconfig ? You're in the same
> situation.
> 
> We all know it's not fun to have to deal with local config snippets, but
> as soon as you plan to boot on a specific hardware, this is unavoidable.
> Also, config symbols are rarely renamed. Most often they are moved under
> new entries (e.g. CONFIG_VENDOR_FOO) which are enabled by default, so
> that updating your old configuration using "make olddefconfig" is enough
> to update it.
> 
> What I'm understanding from your proposed change is not to support
> KernelCI, but to support Chromebooks by default. This could make more
> sense if that's a relevant platform whose support is currently limited
> by default, I'm not able to judge that, but at least it seems to me
> this would make more sense than having specific configs for KernelCI.

This is correct, KernelCI doesn't really need these configs to be
upstreamed.  It's useful as Enric pointed out, but there are
already several specific config fragments being managed by the
KernelCI build system as one would expect, and we can take care
of one more if need be.

However, it was found while adding some x86 Chromebooks[1] to
KernelCI that x86_64_defconfig lacked some basic things for
anyone to be able to boot a kernel with a serial console enabled
on those.  That is what this patch is really about.  When doing
upstream kernel development and building your own kernel, it is
obviously a very useful thing to have.

Agreed, it is easy enough for a developer to turn these configs
on when required.  But it's not entirely trivial to find out
which configs to turn on, especially when you don't have access
to the kernel log.  I went through the Chrome OS 4.14 kernel
config fragments to get there.  Everyone would probably not
agree, but it does seem to me that the convenience of having it
upstream outweighs the costs.

If it's about size or performance, anyone can compare the kernel
image sizes and other things with the KernelCI (staging) build
artifacts based on v5.9[2].

As mentioned earlier in this thread, there aren't any written
rules about what goes into x86_64_defconfig and what does not.
Based on past history, and looking at it from a developer's point
of view rather than KernelCI, does it make sense in this case?

Thanks,
Guillaume


[1] HP Intel x360 "octopus" and AMD 11A-G6-EE "grunt":
    https://staging.kernelci.org/test/plan/id/5f8101a97ba4fdae00cafbb0/
    https://staging.kernelci.org/test/plan/id/5f81003f56c3586920cafbb4/

[2] Plain x86_64_defconfig:
    https://storage.staging.kernelci.org/kernelci/staging-mainline/staging-mainline-20201011.0/x86_64/x86_64_defconfig/gcc-8/
    with "x86 Chromebook" fragment:
    https://storage.staging.kernelci.org/kernelci/staging-mainline/staging-mainline-20201011.0/x86_64/x86_64_defconfig+x86-chromebook/gcc-8/