Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1569031 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 01/50] x86/boot/e820: Introduce arch/x86/include/asm/e820/types.h Ingo Molnar <mingo@kernel.org> - 2017-01-28 23:20 +0100
    Re: [PATCH 01/50] x86/boot/e820: Introduce  arch/x86/include/asm/e820/types.h Sam Ravnborg <sam@ravnborg.org> - 2017-01-29 18:20 +0100
      Re: [PATCH 01/50] x86/boot/e820: Introduce  arch/x86/include/asm/e820/types.h Ingo Molnar <mingo@kernel.org> - 2017-01-30 09:00 +0100

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

First baby steps towards saner e820 headers: create an exact copy of
arch/x86/include/uapi/asm/e820.h and use it from the asm/e820.h file.

No other changes - this is done to decouple the code from UAPI headers,
plus to make sure that subsequent modifications to the file can be more
clearly seen.

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.)

This gives the kernel the freedom to reorganize the e820 code.

Cc: Alex Thorlton <athorlton@sgi.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Huang, Ying <ying.huang@intel.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul Jackson <pj@sgi.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/e820.h       |  4 +--
 arch/x86/include/asm/e820/types.h | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h
index 67313f3a9874..55c1d76c169f 100644
--- a/arch/x86/include/asm/e820.h
+++ b/arch/x86/include/asm/e820.h
@@ -5,12 +5,12 @@
  * E820_X_MAX is the maximum size of the extended E820 table.  The extended
  * table may contain up to 3 extra E820 entries per possible NUMA node, so we
  * make room for 3 * MAX_NUMNODES possible entries, beyond the standard 128.
- * Also note that E820_X_MAX *must* be defined before we include uapi/asm/e820.h.
+ * Also note that E820_X_MAX *must* be defined before we include asm/e820/types.h.
  */
 #include <linux/numa.h>
 #define E820_X_MAX (E820MAX + 3 * MAX_NUMNODES)
 
-#include <uapi/asm/e820.h>
+#include <asm/e820/types.h>
 
 #ifndef __ASSEMBLY__
 /* see comment in arch/x86/kernel/e820.c */
diff --git a/arch/x86/include/asm/e820/types.h b/arch/x86/include/asm/e820/types.h
new file mode 100644
index 000000000000..9dafe59cf6e2
--- /dev/null
+++ b/arch/x86/include/asm/e820/types.h
@@ -0,0 +1,81 @@
+#ifndef _UAPI_ASM_X86_E820_H
+#define _UAPI_ASM_X86_E820_H
+#define E820MAP	0x2d0		/* our map */
+#define E820MAX	128		/* number of entries in E820MAP */
+
+/*
+ * Legacy E820 BIOS limits us to 128 (E820MAX) nodes due to the
+ * constrained space in the zeropage.  If we have more nodes than
+ * that, and if we've booted off EFI firmware, then the EFI tables
+ * passed us from the EFI firmware can list more nodes.  Size our
+ * internal memory map tables to have room for these additional
+ * nodes, based on up to three entries per node for which the
+ * kernel was built: MAX_NUMNODES == (1 << CONFIG_NODES_SHIFT),
+ * plus E820MAX, allowing space for the possible duplicate E820
+ * entries that might need room in the same arrays, prior to the
+ * call to sanitize_e820_map() to remove duplicates.  The allowance
+ * of three memory map entries per node is "enough" entries for
+ * the initial hardware platform motivating this mechanism to make
+ * use of additional EFI map entries.  Future platforms may want
+ * to allow more than three entries per node or otherwise refine
+ * this size.
+ */
+
+#ifndef __KERNEL__
+#define E820_X_MAX E820MAX
+#endif
+
+#define E820NR	0x1e8		/* # entries in E820MAP */
+
+#define E820_RAM	1
+#define E820_RESERVED	2
+#define E820_ACPI	3
+#define E820_NVS	4
+#define E820_UNUSABLE	5
+#define E820_PMEM	7
+
+/*
+ * This is a non-standardized way to represent ADR or NVDIMM regions that
+ * persist over a reboot.  The kernel will ignore their special capabilities
+ * unless the CONFIG_X86_PMEM_LEGACY option is set.
+ *
+ * ( Note that older platforms also used 6 for the same type of memory,
+ *   but newer versions switched to 12 as 6 was assigned differently.  Some
+ *   time they will learn... )
+ */
+#define E820_PRAM	12
+
+/*
+ * reserved RAM used by kernel itself
+ * if CONFIG_INTEL_TXT is enabled, memory of this type will be
+ * included in the S3 integrity calculation and so should not include
+ * any memory that BIOS might alter over the S3 transition
+ */
+#define E820_RESERVED_KERN        128
+
+#ifndef __ASSEMBLY__
+#include <linux/types.h>
+struct e820entry {
+	__u64 addr;	/* start of memory segment */
+	__u64 size;	/* size of memory segment */
+	__u32 type;	/* type of memory segment */
+} __attribute__((packed));
+
+struct e820map {
+	__u32 nr_map;
+	struct e820entry map[E820_X_MAX];
+};
+
+#define ISA_START_ADDRESS	0xa0000
+#define ISA_END_ADDRESS		0x100000
+
+#define BIOS_BEGIN		0x000a0000
+#define BIOS_END		0x00100000
+
+#define BIOS_ROM_BASE		0xffe00000
+#define BIOS_ROM_END		0xffffffff
+
+#endif /* __ASSEMBLY__ */
+
+
+#endif /* _UAPI_ASM_X86_E820_H */
-- 
2.7.4

[toc] | [next] | [standalone]

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

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.

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

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

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.

	Sam

[toc] | [prev] | [next] | [standalone]

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

* Sam Ravnborg <sam@ravnborg.org> 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_ADDR_TYPE_DIST     2
+#define KVM_VGIC_V3_ADDR_TYPE_REDIST   3
+#define KVM_VGIC_ITS_ADDR_TYPE         4
+
+#define KVM_VGIC_V3_DIST_SIZE          SZ_64K
+#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/check-headers.sh 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' 
forever...

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web