Re: [PATCH -v7 1/3] x86 boot: setup data

Previous thread: [PATCH -v7 0/3] x86 boot: 32-bit boot protocol by Huang, Ying on Tuesday, October 23, 2007 - 1:06 am. (4 messages)

Next thread: [PATCH -v7 3/3] x86 boot: document for 32 bit boot protocol by Huang, Ying on Tuesday, October 23, 2007 - 1:06 am. (1 message)
From: Huang, Ying
Date: Tuesday, October 23, 2007 - 1:06 am

This patch add a field of 64-bit physical pointer to NULL terminated
single linked list of struct setup_data to real-mode kernel
header. This is used as a more extensible boot parameters passing
mechanism.

Signed-off-by: Huang Ying <ying.huang@intel.com>

---

 arch/x86/boot/header.S      |    6 +++++-
 arch/x86/kernel/e820_64.c   |    6 +++---
 arch/x86/kernel/head64.c    |   26 ++++++++++++++++++++++++++
 arch/x86/kernel/head_32.S   |   37 ++++++++++++++++++++++++++++++++++++-
 arch/x86/kernel/setup_32.c  |   25 +++++++++++++++++++++++--
 arch/x86/kernel/setup_64.c  |   22 ++++++++++++++++++++--
 arch/x86/mm/discontig_32.c  |    3 ++-
 include/asm-x86/bootparam.h |   12 ++++++++++++
 include/asm-x86/e820_64.h   |    1 +
 include/asm-x86/setup_32.h  |    7 +++++++
 include/asm-x86/setup_64.h  |    2 ++
 11 files changed, 137 insertions(+), 10 deletions(-)

Index: linux-2.6/include/asm-x86/bootparam.h
===================================================================
--- linux-2.6.orig/include/asm-x86/bootparam.h	2007-10-23 10:01:35.000000000 +0800
+++ linux-2.6/include/asm-x86/bootparam.h	2007-10-23 10:48:48.000000000 +0800
@@ -9,6 +9,17 @@
 #include <asm/ist.h>
 #include <video/edid.h>
 
+/* setup data types */
+#define SETUP_NONE			0
+
+/* extensible setup data list node */
+struct setup_data {
+	u64 next;
+	u32 type;
+	u32 len;
+	u8 data[0];
+};
+
 struct setup_header {
 	u8	setup_sects;
 	u16	root_flags;
@@ -46,6 +57,7 @@
 	u32	cmdline_size;
 	u32	hardware_subarch;
 	u64	hardware_subarch_data;
+	u64	setup_data;
 } __attribute__((packed));
 
 struct sys_desc_table {
Index: linux-2.6/arch/x86/boot/header.S
===================================================================
--- linux-2.6.orig/arch/x86/boot/header.S	2007-10-23 10:01:35.000000000 +0800
+++ linux-2.6/arch/x86/boot/header.S	2007-10-23 13:51:29.000000000 +0800
@@ -119,7 +119,7 @@
 	# Part 2 of the header, from the old setup.S
 
 		.ascii	"HdrS"		# header signature
-		.word	0x0207		# ...
From: Jeremy Fitzhardinge
Date: Tuesday, October 23, 2007 - 3:08 pm

As a general comment, I can't say I'm thrilled about sticking the copied
setup data at the end of the initial pagetables.  This is already a
fairly complex area, and changing it touches a surprisingly large number
of places.  I wonder if there isn't a better way to deal with this
(possibly by fixing up the generation of the initial pagetables in the
process).

Or more simply, define a max size for this data, and copy it into an
initdata area (which, being initdata, can be quite large without wasting

What are the alignment rules for this structure?  Is it always 64-bit




-

From: H. Peter Anvin
Date: Tuesday, October 23, 2007 - 3:15 pm

No, he doesn't.  The boot protocol version is communication between the 
boot loader and the kernel (specifically, it is communication FROM the 
kernel TO the bootloader), not between internal bits of the kernel.  Th

Since the pointer is safely inside the setup template, even if the 
bootloader doesn't understand boot protocol version 2.08 it will have 
copied it.

So this is fine.

	-hpa
-

From: Jeremy Fitzhardinge
Date: Tuesday, October 23, 2007 - 3:20 pm

OK.

    J
-

From: H. Peter Anvin
Date: Tuesday, October 23, 2007 - 3:32 pm

Correct.  If it fails to respect this restriction, it might even 
overwrite kernel code.  To make the bootloader's job easier, we have 
retconned the jump instruction at the beginning of the header to also be 
a limit marker.

	-hpa
-

From: H. Peter Anvin
Date: Tuesday, October 23, 2007 - 3:52 pm

The initdata solution is fairly sensible.  It's easy, and this data 
really isn't expected to grow very fast at all.  Unfortunately we don't 
*have* an initdata bss section at the moment, so a large buffer would 
bloat the kernel binary -- at least the uncompressed one.  One school of 
thought says that this should be fixed :) [*]

However, appending to bss (like the pagetables already are) *should* be 
a reasonable option; what really should be done there is instead of 
replacing init_pg_tables_end with setup_data_end we should except 
additional dynamic data items here, and have an initial_data_end 
variable for the extreme end of any initial data no matter the source. 
It still does touch a lot of places, but at least that way they will be 
fixed once and for all.  It's somewhat questionable if it isn't easier 
for this particular job to just fix the initial bss issue.

Furthermore, on looking through the code again, I see a bunch of 

It's x86, so alignment is soft - it presumably *should* be 64-bit 
aligned, but nothing break if the boot loader doesn't.

	-hpa

[*] A very clean way to do that is to put said section right at the 
beginning of the bss, and move __init_end after it:

   .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
         __bss_start = .;                /* BSS */
	*(.init.bss)
	. = ALIGN(4096);
         __init_end = .;
         *(.bss.page_aligned)
         *(.bss)
         . = ALIGN(4);
         __bss_stop = .;
         _end = . ;
         /* This is where the kernel creates the early boot page tables */
         . = ALIGN(4096);
         pg0 = . ;
   }

On x86-64, this entails moving .data_nosave; it probably can be moved to 
the same relative position as on i386 (although I have not verified that 
that is true.)
-

From: Jeremy Fitzhardinge
Date: Tuesday, October 23, 2007 - 4:26 pm

This was more or less a rhetorical question - the code spends some
effort in rounding len up to some alignment, so its probably worth
documenting with the structure.


    J
-

From: Andi Kleen
Date: Tuesday, October 23, 2007 - 4:18 pm

From: H. Peter Anvin
Date: Tuesday, October 23, 2007 - 4:55 pm

Indeed it could.  For i386, the equivalent code would have another 
significant benefit: reserving memory and then mapping and accessing it 
later would (at least eventually) allow accesses > 4 GB on PAE kernels 
(or with a PSE36 hack, on non-PAE kernels.)

	-hpa
-

From: Huang, Ying
Date: Tuesday, October 23, 2007 - 8:08 pm

For i386, the bootmem allocator covers at most 0~796M memory area. So
some early reserve memory area can not be revered with bootmem allocator
later. Should we fix bootmem allocator firstly?

Best Regards,
Huang Ying
-

From: Andrew Morton
Date: Wednesday, October 24, 2007 - 3:48 pm

On Tue, 23 Oct 2007 16:55:13 -0700

Cleaner sounds good.  I'll duck these patches for now.

This one doesn't apply, btw: setup_32.h disappeared.
-

Previous thread: [PATCH -v7 0/3] x86 boot: 32-bit boot protocol by Huang, Ying on Tuesday, October 23, 2007 - 1:06 am. (4 messages)

Next thread: [PATCH -v7 3/3] x86 boot: document for 32 bit boot protocol by Huang, Ying on Tuesday, October 23, 2007 - 1:06 am. (1 message)