Re: [PATCH 01/50] x86/boot/e820: Introduce arch/x86/include/asm/e820/types.h

From: Ingo Molnar
Date: Mon Jan 30 2017 - 02:58:51 EST

* Sam Ravnborg <sam@xxxxxxxxxxxx> wrote:

> On Sat, Jan 28, 2017 at 11:11:22PM +0100, Ingo Molnar wrote:
> >
> > The plan is to keep the old UAPI header in place but the kernel won't
> > use it anymore - and after some time we'll try to remove it. (User-space
> > tools better have local copies of headers anyway, instead of relying
> > on kernel headers.)
> The idea with uapi is the the kernel provides a sane set of headers
> to be used by user space.
> So we avoid random copies that is maintained by random people in random
> ways resulting in random bugs.

Your argument is simplistic which presents a false dichotomy: maintaining a copy
or fully sharing the header are not the only two options available to share the
information in the headers between the kernel and tooling: for example perf uses a
half-automated method where headers are copied from the kernel, but also checked
automatically against the upstream kernel, and a (non-fatal) warning is emitted
during the build if the upstream header has changed.

For example today the perf build shows these UAPI header warnings:

Warning: arch/powerpc/include/uapi/asm/kvm.h differs from kernel
Warning: arch/arm/include/uapi/asm/kvm.h differs from kernel

... because new bits were added to those two UAPI headers. For example the new ARM
bits were:

triton:~/tip/tools/perf> diff -up ../arch/arm/include/uapi/asm/kvm.h ../../arch/arm/include/uapi/asm/kvm.h
--- ../arch/arm/include/uapi/asm/kvm.h 2017-01-23 10:10:18.846003002 +0100
+++ ../../arch/arm/include/uapi/asm/kvm.h 2017-01-28 09:35:12.383587930 +0100
@@ -84,6 +84,15 @@ struct kvm_regs {
#define KVM_VGIC_V2_DIST_SIZE 0x1000
#define KVM_VGIC_V2_CPU_SIZE 0x2000

+/* Supported VGICv3 address types */
+#define KVM_VGIC_V3_REDIST_SIZE (2 * SZ_64K)
+#define KVM_VGIC_V3_ITS_SIZE (2 * SZ_64K)
#define KVM_ARM_VCPU_POWER_OFF 0 /* CPU is started in OFF state */
#define KVM_ARM_VCPU_PSCI_0_2 1 /* CPU uses PSCI v0.2 */

... so for these changes the perf side copy can be updated safely. Had the changes
been more intricate, the changes can be copied too - while adopting the tooling
source code as well.

See the tools/perf/ script.

This IMHO is a far more intelligent and far more robust approach than blind
sharing or detached copies, because it:

- forces new changes from upstream to be considered and adapted by tooling

- header (and thus ABI) synchronization is guaranteed (eventually)

- it does not actually couple the two source code bases in a rigid fashion:

- the upstream kernel is free to change those headers (at least from perf's POV)
in any sane way, those changes can be adapted.

- every step is conscious and there's no way to accidentally break tooling via
header changes - nor does tooling hinder the kernel from progressing its source
code base.

It's basically a script based COW filesystem with guaranteed propagation and
guaranteed synchronization.

> The step(s) outlined here can only result in inconsistency and
> cannot benefit neither user space nor the kernel in the long run.

That's simply not true, see above.

> The uapi shall be lean and clean headers, and shall include
> no info whatsoever that is not relevant for user space.

I agree with that characterization, and that will be even more so with my changes:
my series makes uapi/asm/bootparam.h more self-contained, more lean - while still
defining the full ABI.

> But requiring all user space programs (diverse libc variants,
> other programs) to maintain their own copy can only result in
> inconsistencies that is the benefit for no one.

That's simply not true, see above.

Note that my changes try to keep the 'UAPI promise' (in that the old e820.h header
is still around), while still modifying the kernel side.

What _IS_ insane is to somehow construe the UAPI headers as a rigid construct that
forces the kernel source to keep using poorly chosen names like 'struct e820entry'