[PATCH 0/5] Boot protocol changes

[PATCH 0/5] Boot protocol changes

[Rusty Russell]

Hi all,

	Jeremy had some boot changes for bzImages, but buried in there was an
update to the boot protocol to support Xen and lguest (and kvm-lite).
I've copied those fairly simple patches, and if HPA is happy I'd like to
push them for 2.6.24 (after correcting for the Great Arch Merge of
course).

Thanks,
Rusty.

[PATCH 1/5] update boot spec to 2.07

[Rusty Russell]

Proposed updates for version 2.07 of the boot protocol.  This includes:

load_flags.KEEP_SEGMENTS- flag to request/inhibit segment reloads
hardware_subarch	- what subarchitecture we're booting under
hardware_subarch_data	- per-architecture data

The intention of these changes is to make booting a paravirtualized
kernel work via the normal Linux boot protocol.  The intention is that
the bzImage payload can be a properly formed ELF file, so that the
bootloader can use its ELF notes and Phdrs to get more metadata about
the kernel and its requirements.

The ELF file could be the uncompressed kernel vmlinux itself; it would
only take small buildsystem changes to implement this.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Vivek Goyal <vgoyal@in.ibm.com>

---
 Documentation/i386/boot.txt    |   34 +++++++++++++++++++++++++++++++++-
 arch/i386/kernel/asm-offsets.c |    7 +++++++
 include/asm-i386/bootparam.h   |    9 +++++++--
 3 files changed, 47 insertions(+), 3 deletions(-)

diff -r cff7afab3bac Documentation/i386/boot.txt
--- a/Documentation/i386/boot.txt	Tue Oct 02 22:05:10 2007 +1000
+++ b/Documentation/i386/boot.txt	Tue Oct 02 22:05:10 2007 +1000
@@ -168,6 +168,8 @@ 0234/1	2.05+	relocatable_kernel Whether 
 0234/1	2.05+	relocatable_kernel Whether kernel is relocatable or not
 0235/3	N/A	pad2		Unused
 0238/4	2.06+	cmdline_size	Maximum size of the kernel command line
+023C/4	2.07+	hardware_subarch Hardware subarchitecture
+0240/8	2.07+	hardware_subarch_data Subarchitecture-specific data
 
 (1) For backwards compatibility, if the setup_sects field contains 0, the
     real value is 4.
@@ -204,7 +206,7 @@ boot loaders can ignore those fields.
 
 The byte order of all fields is littleendian (this is x86, after all.)
 
-Field name:	setup_secs
+Field name:	setup_sects
 Type:		read
 Offset/size:	0x1f1/1
 Protocol:	ALL
@@ -356,6 +358,13 @@ Protocol:	2.00+
 	- If 0, the protected-mode code is loaded at 0x10000.
 	- If 1, the protected-mode code is loaded at 0x100000.
 
+  Bit 6 (write): KEEP_SEGMENTS
+	Protocol: 2.07+
+	- if 0, reload the segment registers in the 32bit entry point.
+	- if 1, do not reload the segment registers in the 32bit entry point.
+		Assume that %cs %ds %ss %es are all set to flat segments with
+		a base of 0 (or the equivalent for their environment).
+
   Bit 7 (write): CAN_USE_HEAP
 	Set this bit to 1 to indicate that the value entered in the
 	heap_end_ptr is valid.  If this field is clear, some setup code
@@ -479,6 +488,29 @@ Protocol:	2.06+
   zero. This means that the command line can contain at most
   cmdline_size characters. With protocol version 2.05 and earlier, the
   maximum size was 255.
+
+Field name:	hardware_subarch
+Type:		write
+Offset/size:	0x23c/4
+Protocol:	2.07+
+
+  In a paravirtualized environment the hardware low level architectural
+  pieces such as interrupt handling, page table handling, and
+  accessing process control registers needs to be done differently.
+
+  This field allows the bootloader to inform the kernel we are in one
+  one of those environments.
+
+  0x00000000	The default x86/PC environment
+  0x00000001	lguest
+  0x00000002	Xen
+
+Field name:	hardware_subarch_data
+Type:		write
+Offset/size:	0x240/8
+Protocol:	2.07+
+
+  A pointer to data that is specific to hardware subarch
 
 
 **** THE KERNEL COMMAND LINE
diff -r cff7afab3bac arch/i386/kernel/asm-offsets.c
--- a/arch/i386/kernel/asm-offsets.c	Tue Oct 02 22:05:10 2007 +1000
+++ b/arch/i386/kernel/asm-offsets.c	Tue Oct 02 22:05:10 2007 +1000
@@ -15,6 +15,7 @@
 #include <asm/fixmap.h>
 #include <asm/processor.h>
 #include <asm/thread_info.h>
+#include <asm/bootparam.h>
 #include <asm/elf.h>
 
 #include <xen/interface/xen.h>
@@ -145,4 +146,10 @@ void foo(void)
 	OFFSET(LGUEST_PAGES_regs_errcode, lguest_pages, regs.errcode);
 	OFFSET(LGUEST_PAGES_regs, lguest_pages, regs);
 #endif
+
+	BLANK();
+	OFFSET(BP_scratch, boot_params, scratch);
+	OFFSET(BP_loadflags, boot_params, hdr.loadflags);
+	OFFSET(BP_hardware_subarch, boot_params, hdr.hardware_subarch);
+	OFFSET(BP_version, boot_params, hdr.version);
 }
diff -r cff7afab3bac include/asm-i386/bootparam.h
--- a/include/asm-i386/bootparam.h	Tue Oct 02 22:05:10 2007 +1000
+++ b/include/asm-i386/bootparam.h	Tue Oct 02 22:05:10 2007 +1000
@@ -25,8 +25,9 @@ struct setup_header {
 	u16	kernel_version;
 	u8	type_of_loader;
 	u8	loadflags;
-#define LOADED_HIGH	0x01
-#define CAN_USE_HEAP	0x80
+#define LOADED_HIGH	(1<<0)
+#define KEEP_SEGMENTS	(1<<6)
+#define CAN_USE_HEAP	(1<<7)
 	u16	setup_move_size;
 	u32	code32_start;
 	u32	ramdisk_image;
@@ -38,6 +39,10 @@ struct setup_header {
 	u32	initrd_addr_max;
 	u32	kernel_alignment;
 	u8	relocatable_kernel;
+	u8	_pad2[3];
+	u32	cmdline_size;
+	u32	hardware_subarch;
+	u64	hardware_subarch_data;
 } __attribute__((packed));
 
 struct sys_desc_table {

[PATCH 2/5] add WEAK() for creating weak asm labels

[Rusty Russell]

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

---
 include/linux/linkage.h |    6 ++++++
 1 file changed, 6 insertions(+)

===================================================================
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -34,6 +34,12 @@
   name:
 #endif
 
+#ifndef WEAK
+#define WEAK(name)	   \
+	.weak name;	   \
+	name:
+#endif
+
 #define KPROBE_ENTRY(name) \
   .pushsection .kprobes.text, "ax"; \
   ENTRY(name)

[PATCH 3/5] i386: paravirt boot sequence

[Rusty Russell]

This patch uses the updated boot protocol to do paravirtualized boot.
If the boot version is >= 2.07, then it will do two things:

 1. Check the bootparams loadflags to see if we should reload the
    segment registers and clear interrupts.  This is appropriate
    for normal native boot and some paravirtualized environments, but
    inapproprate for others.

 2. Check the hardware architecture, and dispatch to the appropriate
    kernel entrypoint.  If the bootloader doesn't set this, then we
    simply do the normal boot sequence.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Vivek Goyal <vgoyal@in.ibm.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 arch/i386/boot/compressed/head.S |   14 +++++++++--
 arch/i386/boot/compressed/misc.c |    4 +++
 arch/i386/boot/header.S          |    7 ++++-
 arch/i386/kernel/head.S          |   47 ++++++++++++++++++++++++++++++++++----
 4 files changed, 65 insertions(+), 7 deletions(-)

diff -r 5d471e4c931d arch/i386/boot/compressed/head.S
--- a/arch/i386/boot/compressed/head.S	Tue Oct 02 22:13:34 2007 +1000
+++ b/arch/i386/boot/compressed/head.S	Tue Oct 02 22:20:25 2007 +1000
@@ -27,19 +27,30 @@
 #include <asm/segment.h>
 #include <asm/page.h>
 #include <asm/boot.h>
+#include <asm/asm-offsets.h>
 
 .section ".text.head","ax",@progbits
 	.globl startup_32
 
 startup_32:
-	cld
-	cli
+	/* check to see if KEEP_SEGMENTS flag is meaningful */
+	cmpw $0x207, BP_version(%esi)
+	jb 1f
+
+	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
+	 * us to not reload segments */
+	testb $(1<<6), BP_loadflags(%esi)
+	jnz 2f
+
+1:	cli
 	movl $(__BOOT_DS),%eax
 	movl %eax,%ds
 	movl %eax,%es
 	movl %eax,%fs
 	movl %eax,%gs
 	movl %eax,%ss
+
+2:	cld
 
 /* Calculate the delta between where we were compiled to run
  * at and where we were actually loaded at.  This can only be done
diff -r 5d471e4c931d arch/i386/boot/compressed/misc.c
--- a/arch/i386/boot/compressed/misc.c	Tue Oct 02 22:13:34 2007 +1000
+++ b/arch/i386/boot/compressed/misc.c	Tue Oct 02 22:13:34 2007 +1000
@@ -246,6 +246,9 @@ static void putstr(const char *s)
 {
 	int x,y,pos;
 	char c;
+
+	if (RM_SCREEN_INFO.orig_video_mode == 0 && lines == 0 && cols == 0)
+		return;
 
 	x = RM_SCREEN_INFO.orig_x;
 	y = RM_SCREEN_INFO.orig_y;
diff -r 5d471e4c931d arch/i386/boot/header.S
--- a/arch/i386/boot/header.S	Tue Oct 02 22:13:34 2007 +1000
+++ b/arch/i386/boot/header.S	Tue Oct 02 22:13:34 2007 +1000
@@ -119,7 +119,7 @@ 1:
 	# Part 2 of the header, from the old setup.S
 
 		.ascii	"HdrS"		# header signature
-		.word	0x0206		# header version number (>= 0x0105)
+		.word	0x0207		# header version number (>= 0x0105)
 					# or else old loadlin-1.5 will fail)
 		.globl realmode_swtch
 realmode_swtch:	.word	0, 0		# default_switch, SETUPSEG
@@ -214,6 +214,11 @@ cmdline_size:   .long   COMMAND_LINE_SIZ
                                                 #added with boot protocol
                                                 #version 2.06
 
+hardware_subarch:	.long 0			# subarchitecture, added with 2.07
+						# default to 0 for normal x86 PC
+
+hardware_subarch_data:	.quad 0
+
 # End of setup header #####################################################
 
 	.section ".inittext", "ax"
diff -r 5d471e4c931d arch/i386/kernel/head.S
--- a/arch/i386/kernel/head.S	Tue Oct 02 22:13:34 2007 +1000
+++ b/arch/i386/kernel/head.S	Tue Oct 02 22:21:01 2007 +1000
@@ -70,22 +70,30 @@ INIT_MAP_BEYOND_END = BOOTBITMAP_SIZE + 
  */
 .section .text.head,"ax",@progbits
 ENTRY(startup_32)
+	/* check to see if KEEP_SEGMENTS flag is meaningful */
+	cmpw $0x207, BP_version(%esi)
+	jb 1f
+
+	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
+		us to not reload segments */
+	testb $(1<<6), BP_loadflags(%esi)
+	jnz 2f
 
 /*
  * Set segments to known values.
  */
-	cld
-	lgdt boot_gdt_descr - __PAGE_OFFSET
+1:	lgdt boot_gdt_descr - __PAGE_OFFSET
 	movl $(__BOOT_DS),%eax
 	movl %eax,%ds
 	movl %eax,%es
 	movl %eax,%fs
 	movl %eax,%gs
+2:
 
 /*
  * Clear BSS first so that there are no surprises...
- * No need to cld as DF is already clear from cld above...
- */
+ */
+	cld
 	xorl %eax,%eax
 	movl $__bss_start - __PAGE_OFFSET,%edi
 	movl $__bss_stop - __PAGE_OFFSET,%ecx
@@ -119,6 +127,35 @@ 2:
 	movsl
 1:
 
+#ifdef CONFIG_PARAVIRT
+	cmpw $0x207, (boot_params + BP_version - __PAGE_OFFSET)
+	jb default_entry
+
+	/* Paravirt-compatible boot parameters.  Look to see what architecture
+		we're booting under. */
+	movl (boot_params + BP_hardware_subarch - __PAGE_OFFSET), %eax
+	cmpl $num_subarch_entries, %eax
+	jae bad_subarch
+
+	movl subarch_entries - __PAGE_OFFSET(,%eax,4), %eax
+	subl $__PAGE_OFFSET, %eax
+	jmp *%eax
+
+bad_subarch:
+WEAK(lguest_entry)
+WEAK(xen_entry)
+	/* Unknown implementation; there's really
+	   nothing we can do at this point. */
+	ud2a
+.data
+subarch_entries:
+	.long default_entry		/* normal x86/PC */
+	.long lguest_entry		/* lguest hypervisor */
+	.long xen_entry			/* Xen hypervisor */
+num_subarch_entries = (. - subarch_entries) / 4
+.previous
+#endif /* CONFIG_PARAVIRT */
+
 /*
  * Initialize page tables.  This creates a PDE and a set of page
  * tables, which are located immediately beyond _end.  The variable
@@ -131,6 +168,7 @@ 1:
  */
 page_pde_offset = (__PAGE_OFFSET >> 20);
 
+default_entry:
 	movl $(pg0 - __PAGE_OFFSET), %edi
 	movl $(swapper_pg_dir - __PAGE_OFFSET), %edx
 	movl $0x007, %eax			/* 0x007 = PRESENT+RW+USER */

[PATCH 4/5] Revert lguest magic and use hook in head.S

[Rusty Russell]

Version 2.07 of the boot protocol uses 0x23C for the hardware_subarch
field, that for lguest is "1".  This allows us to use the standard
boot entry point rather than the "GenuineLguest" string hack.

This entry point also clears the BSS and copies the boot parameters
and commandline for us, saving more code.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

---
 Documentation/lguest/lguest.c |   31 ++++---------------------------
 arch/i386/kernel/head.S       |    8 ++++++++
 drivers/lguest/lguest_asm.S   |    9 +++------
 3 files changed, 15 insertions(+), 33 deletions(-)

diff -r 2fdc577cfe5c Documentation/lguest/lguest.c
--- a/Documentation/lguest/lguest.c	Tue Oct 02 22:21:05 2007 +1000
+++ b/Documentation/lguest/lguest.c	Tue Oct 02 23:00:09 2007 +1000
@@ -251,23 +251,6 @@ static void *get_pages(unsigned int num)
 	return addr;
 }
 
-/* To find out where to start we look for the magic Guest string, which marks
- * the code we see in lguest_asm.S.  This is a hack which we are currently
- * plotting to replace with the normal Linux entry point. */
-static unsigned long entry_point(const void *start, const void *end)
-{
-	const void *p;
-
-	/* The scan gives us the physical starting address.  We boot with
-	 * pagetables set up with virtual and physical the same, so that's
-	 * OK. */
-	for (p = start; p < end; p++)
-		if (memcmp(p, "GenuineLguest", strlen("GenuineLguest")) == 0)
-			return to_guest_phys(p + strlen("GenuineLguest"));
-
-	errx(1, "Is this image a genuine lguest?");
-}
-
 /* This routine is used to load the kernel or initrd.  It tries mmap, but if
  * that fails (Plan 9's kernel file isn't nicely aligned on page boundaries),
  * it falls back to reading the memory in. */
@@ -303,7 +286,6 @@ static void map_at(int fd, void *addr, u
  * We return the starting address. */
 static unsigned long map_elf(int elf_fd, const Elf32_Ehdr *ehdr)
 {
-	void *start = (void *)-1, *end = NULL;
 	Elf32_Phdr phdr[ehdr->e_phnum];
 	unsigned int i;
 
@@ -335,19 +317,13 @@ static unsigned long map_elf(int elf_fd,
 		verbose("Section %i: size %i addr %p\n",
 			i, phdr[i].p_memsz, (void *)phdr[i].p_paddr);
 
-		/* We track the first and last address we mapped, so we can
-		 * tell entry_point() where to scan. */
-		if (from_guest_phys(phdr[i].p_paddr) < start)
-			start = from_guest_phys(phdr[i].p_paddr);
-		if (from_guest_phys(phdr[i].p_paddr) + phdr[i].p_filesz > end)
-			end=from_guest_phys(phdr[i].p_paddr)+phdr[i].p_filesz;
-
 		/* We map this section of the file at its physical address. */
 		map_at(elf_fd, from_guest_phys(phdr[i].p_paddr),
 		       phdr[i].p_offset, phdr[i].p_filesz);
 	}
 
-	return entry_point(start, end);
+	/* The entry point is given in the ELF header. */
+	return ehdr->e_entry;
 }
 
 /*L:160 Unfortunately the entire ELF image isn't compressed: the segments
@@ -374,7 +350,8 @@ static unsigned long unpack_bzimage(int 
 
 	verbose("Unpacked size %i addr %p\n", len, img);
 
-	return entry_point(img, img + len);
+	/* The entry point for a bzImage is always the first byte */
+	return (unsigned long)img;
 }
 
 /*L:150 A bzImage, unlike an ELF file, is not meant to be loaded.  You're
@@ -1684,8 +1661,15 @@ int main(int argc, char *argv[])
 	*(u32 *)(boot + 0x228) = 4096;
 	concat(boot + 4096, argv+optind+2);
 
-	/* The guest type value of "1" tells the Guest it's under lguest. */
-	*(int *)(boot + 0x23c) = 1;
+	/* Boot protocol version: 2.07 supports the fields for lguest. */
+	*(u16 *)(boot + 0x206) = 0x207;
+
+	/* The hardware_subarch value of "1" tells the Guest it's an lguest. */
+	*(u32 *)(boot + 0x23c) = 1;
+
+	/* Set bit 6 of the loadflags (aka. KEEP_SEGMENTS) so the entry path
+	 * does not ttry to reload segment registers. */
+	*(u8 *)(boot + 0x211) |= (1 << 6);
 
 	/* We tell the kernel to initialize the Guest: this returns the open
 	 * /dev/lguest file descriptor. */
diff -r 2fdc577cfe5c arch/i386/lguest/boot.c
--- a/arch/i386/lguest/boot.c	Tue Oct 02 22:21:05 2007 +1000
+++ b/arch/i386/lguest/boot.c	Tue Oct 02 23:27:22 2007 +1000
@@ -938,18 +938,8 @@ static unsigned lguest_patch(u8 type, u1
 /*G:030 Once we get to lguest_init(), we know we're a Guest.  The paravirt_ops
  * structure in the kernel provides a single point for (almost) every routine
  * we have to override to avoid privileged instructions. */
-__init void lguest_init(void *boot)
-{
-	/* Copy boot parameters first: the Launcher put the physical location
-	 * in %esi, and head.S converted that to a virtual address and handed
-	 * it to us.  We use "__memcpy" because "memcpy" sometimes tries to do
-	 * tricky things to go faster, and we're not ready for that. */
-	__memcpy(&boot_params, boot, PARAM_SIZE);
-	/* The boot parameters also tell us where the command-line is: save
-	 * that, too. */
-	__memcpy(boot_command_line, __va(boot_params.hdr.cmd_line_ptr),
-	       COMMAND_LINE_SIZE);
-
+__init void lguest_init(void)
+{
 	/* We're under lguest, paravirt is enabled, and we're running at
 	 * privilege level 1, not 0 as normal. */
 	paravirt_ops.name = "lguest";
@@ -1018,11 +1008,6 @@ __init void lguest_init(void *boot)
 	 * the normal data segment to get through booting. */
 	asm volatile ("mov %0, %%fs" : : "r" (__KERNEL_DS) : "memory");
 
-	/* Clear the part of the kernel data which is expected to be zero.
-	 * Normally it will be anyway, but if we're loading from a bzImage with
-	 * CONFIG_RELOCATALE=y, the relocations will be sitting here. */
-	memset(__bss_start, 0, __bss_stop - __bss_start);
-
 	/* The Host uses the top of the Guest's virtual address space for the
 	 * Host<->Guest Switcher, and it tells us how much it needs in
 	 * lguest_data.reserve_mem, set up on the LGUEST_INIT hypercall. */
diff -r 2fdc577cfe5c arch/i386/lguest/head.S
--- a/arch/i386/lguest/head.S	Tue Oct 02 22:21:05 2007 +1000
+++ b/arch/i386/lguest/head.S	Tue Oct 02 23:26:19 2007 +1000
@@ -5,11 +5,8 @@
 #include <asm/thread_info.h>
 #include <asm/processor-flags.h>
 
-/*G:020 This is where we begin: we have a magic signature which the launcher
- * looks for.  The plan is that the Linux boot protocol will be extended with a
- * "platform type" field which will guide us here from the normal entry point,
- * but for the moment this suffices.  The normal boot code uses %esi for the
- * boot header, so we do too.
+/*G:020 This is where we begin: head.S notes that the boot header's platform
+ * type field is "1" (lguest), so calls us here.  The boot header is in %esi.
  *
  * WARNING: be very careful here!  We're running at addresses equal to physical
  * addesses (around 0), not above PAGE_OFFSET as most code expectes
@@ -19,19 +16,14 @@
  * The .section line puts this code in .init.text so it will be discarded after
  * boot. */
 .section .init.text, "ax", @progbits
-.ascii "GenuineLguest"
+ENTRY(lguest_entry)
 	/* Make initial hypercall now, so we can set up the pagetables. */
 	movl $LHCALL_LGUEST_INIT, %eax
 	movl $lguest_data - __PAGE_OFFSET, %edx
 	int $LGUEST_TRAP_ENTRY
 
-	/* Set up boot information pointer to hand to lguest_init(): it wants
-	 * a virtual address. */
-	movl %esi, %eax
-	addl $__PAGE_OFFSET, %eax
-
 	/* The Host put the toplevel pagetable in lguest_data.pgdir.  The movsl
-	 * instruction uses %esi, so we needed to save it above. */
+	 * instruction uses %esi implicitly. */
 	movl lguest_data - __PAGE_OFFSET + LGUEST_DATA_pgdir, %esi
 
 	/* Copy first 32 entries of page directory to __PAGE_OFFSET entries.
@@ -46,7 +38,6 @@
 
 	/* Set up the initial stack so we can run C code. */
  	movl $(init_thread_union+THREAD_SIZE),%esp
-
 
 	/* Jumps are relative, and we're running __PAGE_OFFSET too low at the
 	 * moment. */

[PATCH 5/5] lguest: loading bzImage directly

[Rusty Russell]

Now arch/i386/boot/compressed/head.S understands the hardware_platform field,
we can directly execute bzImages.  No more horrific unpacking code.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 Documentation/lguest/lguest.c    |   97 ++++++++++++--------------------------
 arch/i386/boot/compressed/head.S |    6 ++
 drivers/lguest/lguest.c          |    5 +
 3 files changed, 42 insertions(+), 66 deletions(-)

diff -r b0480fd71a72 Documentation/lguest/lguest.c
--- a/Documentation/lguest/lguest.c	Tue Oct 02 22:28:13 2007 +1000
+++ b/Documentation/lguest/lguest.c	Tue Oct 02 22:52:07 2007 +1000
@@ -326,74 +326,39 @@ static unsigned long map_elf(int elf_fd,
 	return ehdr->e_entry;
 }
 
-/*L:160 Unfortunately the entire ELF image isn't compressed: the segments
- * which need loading are extracted and compressed raw.  This denies us the
- * information we need to make a fully-general loader. */
-static unsigned long unpack_bzimage(int fd)
-{
-	gzFile f;
-	int ret, len = 0;
-	/* A bzImage always gets loaded at physical address 1M.  This is
-	 * actually configurable as CONFIG_PHYSICAL_START, but as the comment
-	 * there says, "Don't change this unless you know what you are doing".
-	 * Indeed. */
-	void *img = from_guest_phys(0x100000);
-
-	/* gzdopen takes our file descriptor (carefully placed at the start of
-	 * the GZIP header we found) and returns a gzFile. */
-	f = gzdopen(fd, "rb");
-	/* We read it into memory in 64k chunks until we hit the end. */
-	while ((ret = gzread(f, img + len, 65536)) > 0)
-		len += ret;
-	if (ret < 0)
-		err(1, "reading image from bzImage");
-
-	verbose("Unpacked size %i addr %p\n", len, img);
-
-	/* The entry point for a bzImage is always the first byte */
-	return (unsigned long)img;
-}
-
 /*L:150 A bzImage, unlike an ELF file, is not meant to be loaded.  You're
- * supposed to jump into it and it will unpack itself.  We can't do that
- * because the Guest can't run the unpacking code, and adding features to
- * lguest kills puppies, so we don't want to.
- *
- * The bzImage is formed by putting the decompressing code in front of the
- * compressed kernel code.  So we can simple scan through it looking for the
- * first "gzip" header, and start decompressing from there. */
+ * supposed to jump into it and it will unpack itself.  We used to have to
+ * perform some hairy magic because the unpacking code scared me.
+ *
+ * Fortunately, Jeremy Fitzhardinge convinced me it wasn't that hard and wrote
+ * a small patch to jump over the tricky bits in the guest, so now we just read
+ * the funky header so we know where in the file to load, and away we go! */
 static unsigned long load_bzimage(int fd)
 {
-	unsigned char c;
-	int state = 0;
-
-	/* GZIP header is 0x1F 0x8B <method> <flags>... <compressed-by>. */
-	while (read(fd, &c, 1) == 1) {
-		switch (state) {
-		case 0:
-			if (c == 0x1F)
-				state++;
-			break;
-		case 1:
-			if (c == 0x8B)
-				state++;
-			else
-				state = 0;
-			break;
-		case 2 ... 8:
-			state++;
-			break;
-		case 9:
-			/* Seek back to the start of the gzip header. */
-			lseek(fd, -10, SEEK_CUR);
-			/* One final check: "compressed under UNIX". */
-			if (c != 0x03)
-				state = -1;
-			else
-				return unpack_bzimage(fd);
-		}
-	}
-	errx(1, "Could not find kernel in bzImage");
+	u8 hdr[1024];
+	int r;
+	/* Modern bzImages get loaded at 1M. */
+	void *p = from_guest_phys(0x100000);
+
+	/* Go back to the start of the file and read the header.  It should be
+	 * a Linux boot header (see Documentation/i386/boot.txt) */
+	lseek(fd, 0, SEEK_SET);
+	read(fd, hdr, sizeof(hdr));
+
+	/* At offset 0x202, we expect the magic "HdrS" */
+	if (memcmp(hdr + 0x202, "HdrS", 4) != 0)
+		errx(1, "This doesn't look like a bzImage to me");
+
+	/* The byte at 0x1F1 tells us how many extra sectors of
+	 * header: skip over them all. */
+	lseek(fd, (unsigned long)(hdr[0x1F1]+1) * 512, SEEK_SET);
+
+	/* Now read everything into memory. in nice big chunks. */
+	while ((r = read(fd, p, 65536)) > 0)
+		p += r;
+
+	/* Finally, 0x214 tells us where to start the kernel. */
+	return *(unsigned long *)&hdr[0x214];
 }
 
 /*L:140 Loading the kernel is easy when it's a "vmlinux", but most kernels

Re: [PATCH 5/5] lguest: loading bzImage directly

[Chris Malley]

Hi guys

Would it not be clearer to #include <asm/bootparam.h> and use 
the relevant named members of struct setup_header / struct boot_params
rather than the hard-coded values 0x202, 0x1F1, 0x214 ?

--
Chris


On Wed, 2007-10-03 at 09:40 +1000, Rusty Russell wrote:
[snip]
> +	u8 hdr[1024];
> +	int r;
> +	/* Modern bzImages get loaded at 1M. */
> +	void *p = from_guest_phys(0x100000);
> +
> +	/* Go back to the start of the file and read the header.  It should be
> +	 * a Linux boot header (see Documentation/i386/boot.txt) */
> +	lseek(fd, 0, SEEK_SET);
> +	read(fd, hdr, sizeof(hdr));
> +
> +	/* At offset 0x202, we expect the magic "HdrS" */
> +	if (memcmp(hdr + 0x202, "HdrS", 4) != 0)
> +		errx(1, "This doesn't look like a bzImage to me");
> +
> +	/* The byte at 0x1F1 tells us how many extra sectors of
> +	 * header: skip over them all. */
> +	lseek(fd, (unsigned long)(hdr[0x1F1]+1) * 512, SEEK_SET);
> +
> +	/* Now read everything into memory. in nice big chunks. */
> +	while ((r = read(fd, p, 65536)) > 0)
> +		p += r;
> +
> +	/* Finally, 0x214 tells us where to start the kernel. */
> +	return *(unsigned long *)&hdr[0x214];
>  }
>  
>  /*L:140 Loading the kernel is easy when it's a "vmlinux", but most kernels
> 

Re: [PATCH 5/5] lguest: loading bzImage directly

[H. Peter Anvin]

Chris Malley wrote:
> Hi guys
> 
> Would it not be clearer to #include <asm/bootparam.h> and use 
> the relevant named members of struct setup_header / struct boot_params
> rather than the hard-coded values 0x202, 0x1F1, 0x214 ?
> 

Yes, please.  I have a patch in -mm which already does that across the 
kernel.

	-hpa

Re: [PATCH 5/5] lguest: loading bzImage directly

[Rusty Russell]

On Wed, 2007-10-03 at 10:37 +0100, Chris Malley wrote:
> Hi guys
> 
> Would it not be clearer to #include <asm/bootparam.h> and use 
> the relevant named members of struct setup_header / struct boot_params
> rather than the hard-coded values 0x202, 0x1F1, 0x214 ?

Yes, but unfortunately bootparam.h wasn't designed to be included from
userspace.

The patch would look like this, but it makes me wonder if it'd be better
to put all these user-exposed types in bootparam.h and have the other
headers include them.  hpa?

diff -r 6bb527d113a8 include/asm-i386/Kbuild
--- a/include/asm-i386/Kbuild	Wed Oct 03 13:49:31 2007 +1000
+++ b/include/asm-i386/Kbuild	Thu Oct 04 09:53:08 2007 +1000
@@ -6,7 +6,10 @@ header-y += msr-index.h
 header-y += msr-index.h
 header-y += ptrace-abi.h
 header-y += ucontext.h
+header-y += bootparam.h
 
+unifdef-y += e820.h
+unifdef-y += ist.h
 unifdef-y += msr.h
 unifdef-y += mtrr.h
 unifdef-y += vm86.h
diff -r 6bb527d113a8 include/asm-i386/bootparam.h
--- a/include/asm-i386/bootparam.h	Wed Oct 03 13:49:31 2007 +1000
+++ b/include/asm-i386/bootparam.h	Thu Oct 04 09:45:12 2007 +1000
@@ -10,82 +10,82 @@
 #include <video/edid.h>
 
 struct setup_header {
-	u8	setup_sects;
-	u16	root_flags;
-	u32	syssize;
-	u16	ram_size;
-	u16	vid_mode;
-	u16	root_dev;
-	u16	boot_flag;
-	u16	jump;
-	u32	header;
-	u16	version;
-	u32	realmode_swtch;
-	u16	start_sys;
-	u16	kernel_version;
-	u8	type_of_loader;
-	u8	loadflags;
+	__u8	setup_sects;
+	__u16	root_flags;
+	__u32	syssize;
+	__u16	ram_size;
+	__u16	vid_mode;
+	__u16	root_dev;
+	__u16	boot_flag;
+	__u16	jump;
+	__u32	header;
+	__u16	version;
+	__u32	realmode_swtch;
+	__u16	start_sys;
+	__u16	kernel_version;
+	__u8	type_of_loader;
+	__u8	loadflags;
 #define LOADED_HIGH	(1<<0)
 #define KEEP_SEGMENTS	(1<<6)
 #define CAN_USE_HEAP	(1<<7)
-	u16	setup_move_size;
-	u32	code32_start;
-	u32	ramdisk_image;
-	u32	ramdisk_size;
-	u32	bootsect_kludge;
-	u16	heap_end_ptr;
-	u16	_pad1;
-	u32	cmd_line_ptr;
-	u32	initrd_addr_max;
-	u32	kernel_alignment;
-	u8	relocatable_kernel;
-	u8	_pad2[3];
-	u32	cmdline_size;
-	u32	hardware_subarch;
-	u64	hardware_subarch_data;
+	__u16	setup_move_size;
+	__u32	code32_start;
+	__u32	ramdisk_image;
+	__u32	ramdisk_size;
+	__u32	bootsect_kludge;
+	__u16	heap_end_ptr;
+	__u16	_pad1;
+	__u32	cmd_line_ptr;
+	__u32	initrd_addr_max;
+	__u32	kernel_alignment;
+	__u8	relocatable_kernel;
+	__u8	_pad2[3];
+	__u32	cmdline_size;
+	__u32	hardware_subarch;
+	__u64	hardware_subarch_data;
 } __attribute__((packed));
 
 struct sys_desc_table {
-	u16 length;
-	u8  table[14];
+	__u16 length;
+	__u8  table[14];
 };
 
 struct efi_info {
-	u32 _pad1;
-	u32 efi_systab;
-	u32 efi_memdesc_size;
-	u32 efi_memdesc_version;
-	u32 efi_memmap;
-	u32 efi_memmap_size;
-	u32 _pad2[2];
+	__u32 _pad1;
+	__u32 efi_systab;
+	__u32 efi_memdesc_size;
+	__u32 efi_memdesc_version;
+	__u32 efi_memmap;
+	__u32 efi_memmap_size;
+	__u32 _pad2[2];
 };
 
 /* The so-called "zeropage" */
 struct boot_params {
 	struct screen_info screen_info;			/* 0x000 */
 	struct apm_bios_info apm_bios_info;		/* 0x040 */
-	u8  _pad2[12];					/* 0x054 */
+	__u8  _pad2[12];				/* 0x054 */
 	struct ist_info ist_info;			/* 0x060 */
-	u8  _pad3[16];					/* 0x070 */
-	u8  hd0_info[16];	/* obsolete! */		/* 0x080 */
-	u8  hd1_info[16];	/* obsolete! */		/* 0x090 */
+	__u8  _pad3[16];				/* 0x070 */
+	__u8  hd0_info[16];	/* obsolete! */		/* 0x080 */
+	__u8  hd1_info[16];	/* obsolete! */		/* 0x090 */
 	struct sys_desc_table sys_desc_table;		/* 0x0a0 */
-	u8  _pad4[144];					/* 0x0b0 */
+	__u8  _pad4[144];				/* 0x0b0 */
 	struct edid_info edid_info;			/* 0x140 */
 	struct efi_info efi_info;			/* 0x1c0 */
-	u32 alt_mem_k;					/* 0x1e0 */
-	u32 scratch;		/* Scratch field! */	/* 0x1e4 */
-	u8  e820_entries;				/* 0x1e8 */
-	u8  eddbuf_entries;				/* 0x1e9 */
-	u8  edd_mbr_sig_buf_entries;			/* 0x1ea */
-	u8  _pad6[6];					/* 0x1eb */
+	__u32 alt_mem_k;				/* 0x1e0 */
+	__u32 scratch;		/* Scratch field! */	/* 0x1e4 */
+	__u8  e820_entries;				/* 0x1e8 */
+	__u8  eddbuf_entries;				/* 0x1e9 */
+	__u8  edd_mbr_sig_buf_entries;			/* 0x1ea */
+	__u8  _pad6[6];					/* 0x1eb */
 	struct setup_header hdr;    /* setup header */	/* 0x1f1 */
-	u8  _pad7[0x290-0x1f1-sizeof(struct setup_header)];
-	u32 edd_mbr_sig_buffer[EDD_MBR_SIG_MAX];	/* 0x290 */
+	__u8  _pad7[0x290-0x1f1-sizeof(struct setup_header)];
+	__u32 edd_mbr_sig_buffer[EDD_MBR_SIG_MAX];	/* 0x290 */
 	struct e820entry e820_map[E820MAX];		/* 0x2d0 */
-	u8  _pad8[48];					/* 0xcd0 */
+	__u8  _pad8[48];				/* 0xcd0 */
 	struct edd_info eddbuf[EDDMAXNR];		/* 0xd00 */
-	u8  _pad9[276];					/* 0xeec */
+	__u8  _pad9[276];				/* 0xeec */
 } __attribute__((packed));
 
 #endif /* _ASM_BOOTPARAM_H */
diff -r 6bb527d113a8 include/asm-i386/e820.h
--- a/include/asm-i386/e820.h	Wed Oct 03 13:49:31 2007 +1000
+++ b/include/asm-i386/e820.h	Thu Oct 04 09:44:56 2007 +1000
@@ -26,16 +26,17 @@
 #ifndef __ASSEMBLY__
 
 struct e820entry {
-	u64 addr;	/* start of memory segment */
-	u64 size;	/* size of memory segment */
-	u32 type;	/* type of memory segment */
+	__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;
+	__u32 nr_map;
 	struct e820entry map[E820MAX];
 };
 
+#ifdef __KERNEL__
 extern struct e820map e820;
 
 extern int e820_all_mapped(unsigned long start, unsigned long end,
@@ -56,5 +57,5 @@ static inline void e820_mark_nosave_regi
 #endif
 
 #endif/*!__ASSEMBLY__*/
-
+#endif/*__KERNEL__*/
 #endif/*__E820_HEADER*/
diff -r 6bb527d113a8 include/asm-i386/ist.h
--- a/include/asm-i386/ist.h	Wed Oct 03 13:49:31 2007 +1000
+++ b/include/asm-i386/ist.h	Thu Oct 04 09:45:39 2007 +1000
@@ -17,16 +17,16 @@
  */
 
 
-#ifdef __KERNEL__
-
 #include <linux/types.h>
 
 struct ist_info {
-	u32 signature;
-	u32 command;
-	u32 event;
-	u32 perf_level;
+	__u32 signature;
+	__u32 command;
+	__u32 event;
+	__u32 perf_level;
 };
+
+#ifdef __KERNEL__
 
 extern struct ist_info ist_info;
 
diff -r 6bb527d113a8 include/linux/Kbuild
--- a/include/linux/Kbuild	Wed Oct 03 13:49:31 2007 +1000
+++ b/include/linux/Kbuild	Thu Oct 04 09:51:30 2007 +1000
@@ -189,6 +189,7 @@ unifdef-y += dccp.h
 unifdef-y += dccp.h
 unifdef-y += dirent.h
 unifdef-y += dlm.h
+unifdef-y += edd.h
 unifdef-y += elfcore.h
 unifdef-y += errno.h
 unifdef-y += errqueue.h
@@ -308,6 +309,7 @@ unifdef-y += rtnetlink.h
 unifdef-y += rtnetlink.h
 unifdef-y += scc.h
 unifdef-y += sched.h
+unifdef-y += screen_info.h
 unifdef-y += sdla.h
 unifdef-y += selinux_netlink.h
 unifdef-y += sem.h
diff -r 6bb527d113a8 include/linux/apm_bios.h
--- a/include/linux/apm_bios.h	Wed Oct 03 13:49:31 2007 +1000
+++ b/include/linux/apm_bios.h	Thu Oct 04 09:37:28 2007 +1000
@@ -16,28 +16,28 @@
  * General Public License for more details.
  */
 
+#include <linux/types.h>
+
+struct apm_bios_info {
+	__u16	version;
+	__u16	cseg;
+	__u32	offset;
+	__u16	cseg_16;
+	__u16	dseg;
+	__u16	flags;
+	__u16	cseg_len;
+	__u16	cseg_16_len;
+	__u16	dseg_len;
+};
+
+#ifdef __KERNEL__
+
 typedef unsigned short	apm_event_t;
 typedef unsigned short	apm_eventinfo_t;
-
-#ifdef __KERNEL__
-
-#include <linux/types.h>
 
 #define APM_CS		(GDT_ENTRY_APMBIOS_BASE * 8)
 #define APM_CS_16	(APM_CS + 8)
 #define APM_DS		(APM_CS_16 + 8)
-
-struct apm_bios_info {
-	u16	version;
-	u16	cseg;
-	u32	offset;
-	u16	cseg_16;
-	u16	dseg;
-	u16	flags;
-	u16	cseg_len;
-	u16	cseg_16_len;
-	u16	dseg_len;
-};
 
 /* Results of APM Installation Check */
 #define APM_16_BIT_SUPPORT	0x0001
diff -r 6bb527d113a8 include/linux/edd.h
--- a/include/linux/edd.h	Wed Oct 03 13:49:31 2007 +1000
+++ b/include/linux/edd.h	Thu Oct 04 09:43:22 2007 +1000
@@ -67,113 +67,113 @@
 #define EDD_INFO_USE_INT13_FN50                (1 << 7)
 
 struct edd_device_params {
-	u16 length;
-	u16 info_flags;
-	u32 num_default_cylinders;
-	u32 num_default_heads;
-	u32 sectors_per_track;
-	u64 number_of_sectors;
-	u16 bytes_per_sector;
-	u32 dpte_ptr;		/* 0xFFFFFFFF for our purposes */
-	u16 key;		/* = 0xBEDD */
-	u8 device_path_info_length;	/* = 44 */
-	u8 reserved2;
-	u16 reserved3;
-	u8 host_bus_type[4];
-	u8 interface_type[8];
+	__u16 length;
+	__u16 info_flags;
+	__u32 num_default_cylinders;
+	__u32 num_default_heads;
+	__u32 sectors_per_track;
+	__u64 number_of_sectors;
+	__u16 bytes_per_sector;
+	__u32 dpte_ptr;		/* 0xFFFFFFFF for our purposes */
+	__u16 key;		/* = 0xBEDD */
+	__u8 device_path_info_length;	/* = 44 */
+	__u8 reserved2;
+	__u16 reserved3;
+	__u8 host_bus_type[4];
+	__u8 interface_type[8];
 	union {
 		struct {
-			u16 base_address;
-			u16 reserved1;
-			u32 reserved2;
+			__u16 base_address;
+			__u16 reserved1;
+			__u32 reserved2;
 		} __attribute__ ((packed)) isa;
 		struct {
-			u8 bus;
-			u8 slot;
-			u8 function;
-			u8 channel;
-			u32 reserved;
+			__u8 bus;
+			__u8 slot;
+			__u8 function;
+			__u8 channel;
+			__u32 reserved;
 		} __attribute__ ((packed)) pci;
 		/* pcix is same as pci */
 		struct {
-			u64 reserved;
+			__u64 reserved;
 		} __attribute__ ((packed)) ibnd;
 		struct {
-			u64 reserved;
+			__u64 reserved;
 		} __attribute__ ((packed)) xprs;
 		struct {
-			u64 reserved;
+			__u64 reserved;
 		} __attribute__ ((packed)) htpt;
 		struct {
-			u64 reserved;
+			__u64 reserved;
 		} __attribute__ ((packed)) unknown;
 	} interface_path;
 	union {
 		struct {
-			u8 device;
-			u8 reserved1;
-			u16 reserved2;
-			u32 reserved3;
-			u64 reserved4;
+			__u8 device;
+			__u8 reserved1;
+			__u16 reserved2;
+			__u32 reserved3;
+			__u64 reserved4;
 		} __attribute__ ((packed)) ata;
 		struct {
-			u8 device;
-			u8 lun;
-			u8 reserved1;
-			u8 reserved2;
-			u32 reserved3;
-			u64 reserved4;
+			__u8 device;
+			__u8 lun;
+			__u8 reserved1;
+			__u8 reserved2;
+			__u32 reserved3;
+			__u64 reserved4;
 		} __attribute__ ((packed)) atapi;
 		struct {
-			u16 id;
-			u64 lun;
-			u16 reserved1;
-			u32 reserved2;
+			__u16 id;
+			__u64 lun;
+			__u16 reserved1;
+			__u32 reserved2;
 		} __attribute__ ((packed)) scsi;
 		struct {
-			u64 serial_number;
-			u64 reserved;
+			__u64 serial_number;
+			__u64 reserved;
 		} __attribute__ ((packed)) usb;
 		struct {
-			u64 eui;
-			u64 reserved;
+			__u64 eui;
+			__u64 reserved;
 		} __attribute__ ((packed)) i1394;
 		struct {
-			u64 wwid;
-			u64 lun;
+			__u64 wwid;
+			__u64 lun;
 		} __attribute__ ((packed)) fibre;
 		struct {
-			u64 identity_tag;
-			u64 reserved;
+			__u64 identity_tag;
+			__u64 reserved;
 		} __attribute__ ((packed)) i2o;
 		struct {
-			u32 array_number;
-			u32 reserved1;
-			u64 reserved2;
+			__u32 array_number;
+			__u32 reserved1;
+			__u64 reserved2;
 		} __attribute__ ((packed)) raid;
 		struct {
-			u8 device;
-			u8 reserved1;
-			u16 reserved2;
-			u32 reserved3;
-			u64 reserved4;
+			__u8 device;
+			__u8 reserved1;
+			__u16 reserved2;
+			__u32 reserved3;
+			__u64 reserved4;
 		} __attribute__ ((packed)) sata;
 		struct {
-			u64 reserved1;
-			u64 reserved2;
+			__u64 reserved1;
+			__u64 reserved2;
 		} __attribute__ ((packed)) unknown;
 	} device_path;
-	u8 reserved4;
-	u8 checksum;
+	__u8 reserved4;
+	__u8 checksum;
 } __attribute__ ((packed));
 
 struct edd_info {
-	u8 device;
-	u8 version;
-	u16 interface_support;
-	u16 legacy_max_cylinder;
-	u8 legacy_max_head;
-	u8 legacy_sectors_per_track;
+	__u8 device;
+	__u8 version;
+	__u16 interface_support;
+	__u16 legacy_max_cylinder;
+	__u8 legacy_max_head;
+	__u8 legacy_sectors_per_track;
 	struct edd_device_params params;
 } __attribute__ ((packed));
 
@@ -184,8 +184,9 @@ struct edd {
 	unsigned char edd_info_nr;
 };
 
+#ifdef __KERNEL__
 extern struct edd edd;
-
+#endif /* __KERNEL__ */
 #endif				/*!__ASSEMBLY__ */
 
 #endif				/* _LINUX_EDD_H */
diff -r 6bb527d113a8 include/linux/screen_info.h
--- a/include/linux/screen_info.h	Wed Oct 03 13:49:31 2007 +1000
+++ b/include/linux/screen_info.h	Thu Oct 04 09:40:29 2007 +1000
@@ -8,53 +8,42 @@
  */
 
 struct screen_info {
-	u8  orig_x;		/* 0x00 */
-	u8  orig_y;		/* 0x01 */
-	u16 ext_mem_k;		/* 0x02 */
-	u16 orig_video_page;	/* 0x04 */
-	u8  orig_video_mode;	/* 0x06 */
-	u8  orig_video_cols;	/* 0x07 */
-	u16 unused2;		/* 0x08 */
-	u16 orig_video_ega_bx;	/* 0x0a */
-	u16 unused3;		/* 0x0c */
-	u8  orig_video_lines;	/* 0x0e */
-	u8  orig_video_isVGA;	/* 0x0f */
-	u16 orig_video_points;	/* 0x10 */
+	__u8  orig_x;		/* 0x00 */
+	__u8  orig_y;		/* 0x01 */
+	__u16 ext_mem_k;	/* 0x02 */
+	__u16 orig_video_page;	/* 0x04 */
+	__u8  orig_video_mode;	/* 0x06 */
+	__u8  orig_video_cols;	/* 0x07 */
+	__u16 unused2;		/* 0x08 */
+	__u16 orig_video_ega_bx;/* 0x0a */
+	__u16 unused3;		/* 0x0c */
+	__u8  orig_video_lines;	/* 0x0e */
+	__u8  orig_video_isVGA;	/* 0x0f */
+	__u16 orig_video_points;/* 0x10 */
 
 	/* VESA graphic mode -- linear frame buffer */
-	u16 lfb_width;		/* 0x12 */
-	u16 lfb_height;		/* 0x14 */
-	u16 lfb_depth;		/* 0x16 */
-	u32 lfb_base;		/* 0x18 */
-	u32 lfb_size;		/* 0x1c */
-	u16 cl_magic, cl_offset; /* 0x20 */
-	u16 lfb_linelength;	/* 0x24 */
-	u8  red_size;		/* 0x26 */
-	u8  red_pos;		/* 0x27 */
-	u8  green_size;		/* 0x28 */
-	u8  green_pos;		/* 0x29 */
-	u8  blue_size;		/* 0x2a */
-	u8  blue_pos;		/* 0x2b */
-	u8  rsvd_size;		/* 0x2c */
-	u8  rsvd_pos;		/* 0x2d */
-	u16 vesapm_seg;		/* 0x2e */
-	u16 vesapm_off;		/* 0x30 */
-	u16 pages;		/* 0x32 */
-	u16 vesa_attributes;	/* 0x34 */
-	u32 capabilities;       /* 0x36 */
-	u8  _reserved[6];	/* 0x3a */
+	__u16 lfb_width;	/* 0x12 */
+	__u16 lfb_height;	/* 0x14 */
+	__u16 lfb_depth;	/* 0x16 */
+	__u32 lfb_base;		/* 0x18 */
+	__u32 lfb_size;		/* 0x1c */
+	__u16 cl_magic, cl_offset; /* 0x20 */
+	__u16 lfb_linelength;	/* 0x24 */
+	__u8  red_size;		/* 0x26 */
+	__u8  red_pos;		/* 0x27 */
+	__u8  green_size;	/* 0x28 */
+	__u8  green_pos;	/* 0x29 */
+	__u8  blue_size;	/* 0x2a */
+	__u8  blue_pos;		/* 0x2b */
+	__u8  rsvd_size;	/* 0x2c */
+	__u8  rsvd_pos;		/* 0x2d */
+	__u16 vesapm_seg;	/* 0x2e */
+	__u16 vesapm_off;	/* 0x30 */
+	__u16 pages;		/* 0x32 */
+	__u16 vesa_attributes;	/* 0x34 */
+	__u32 capabilities;     /* 0x36 */
+	__u8  _reserved[6];	/* 0x3a */
 } __attribute__((packed));
-
-extern struct screen_info screen_info;
-
-#define ORIG_X			(screen_info.orig_x)
-#define ORIG_Y			(screen_info.orig_y)
-#define ORIG_VIDEO_MODE		(screen_info.orig_video_mode)
-#define ORIG_VIDEO_COLS 	(screen_info.orig_video_cols)
-#define ORIG_VIDEO_EGA_BX	(screen_info.orig_video_ega_bx)
-#define ORIG_VIDEO_LINES	(screen_info.orig_video_lines)
-#define ORIG_VIDEO_ISVGA	(screen_info.orig_video_isVGA)
-#define ORIG_VIDEO_POINTS       (screen_info.orig_video_points)
 
 #define VIDEO_TYPE_MDA		0x10	/* Monochrome Text Display	*/
 #define VIDEO_TYPE_CGA		0x11	/* CGA Display 			*/
@@ -74,4 +63,17 @@ extern struct screen_info screen_info;
 
 #define VIDEO_TYPE_PMAC		0x60	/* PowerMacintosh frame buffer. */
 
+#ifdef __KERNEL__
+extern struct screen_info screen_info;
+
+#define ORIG_X			(screen_info.orig_x)
+#define ORIG_Y			(screen_info.orig_y)
+#define ORIG_VIDEO_MODE		(screen_info.orig_video_mode)
+#define ORIG_VIDEO_COLS 	(screen_info.orig_video_cols)
+#define ORIG_VIDEO_EGA_BX	(screen_info.orig_video_ega_bx)
+#define ORIG_VIDEO_LINES	(screen_info.orig_video_lines)
+#define ORIG_VIDEO_ISVGA	(screen_info.orig_video_isVGA)
+#define ORIG_VIDEO_POINTS       (screen_info.orig_video_points)
+#endif /* __KERNEL__ */
+
 #endif /* _SCREEN_INFO_H */
diff -r 6bb527d113a8 include/video/Kbuild
--- a/include/video/Kbuild	Wed Oct 03 13:49:31 2007 +1000
+++ b/include/video/Kbuild	Thu Oct 04 09:50:15 2007 +1000
@@ -1,1 +1,2 @@ unifdef-y += sisfb.h
 unifdef-y += sisfb.h
+unifdef-y += edid.h
diff -r 6bb527d113a8 include/video/edid.h
--- a/include/video/edid.h	Wed Oct 03 13:49:31 2007 +1000
+++ b/include/video/edid.h	Thu Oct 04 09:48:14 2007 +1000
@@ -1,17 +1,16 @@
 #ifndef __linux_video_edid_h__
 #define __linux_video_edid_h__
 
-#ifdef __KERNEL__
+#if !defined(__KERNEL__) || defined(CONFIG_X86)
 
-
-#ifdef CONFIG_X86
 struct edid_info {
 	unsigned char dummy[128];
 };
 
+#ifdef __KERNEL__
 extern struct edid_info edid_info;
-#endif /* CONFIG_X86 */
-
 #endif /* __KERNEL__ */
 
+#endif
+
 #endif /* __linux_video_edid_h__ */

Re: [PATCH 5/5] lguest: loading bzImage directly

[H. Peter Anvin]

Rusty Russell wrote:
> On Wed, 2007-10-03 at 10:37 +0100, Chris Malley wrote:
>> Hi guys
>>
>> Would it not be clearer to #include <asm/bootparam.h> and use 
>> the relevant named members of struct setup_header / struct boot_params
>> rather than the hard-coded values 0x202, 0x1F1, 0x214 ?
> 
> Yes, but unfortunately bootparam.h wasn't designed to be included from
> userspace.
> 
> The patch would look like this, but it makes me wonder if it'd be better
> to put all these user-exposed types in bootparam.h and have the other
> headers include them.  hpa?
> 

I don't have a strong preference either way, but I think what you have 
here is fine.

	-hpa

Re: [PATCH 5/5] lguest: loading bzImage directly

[Kjartan Maraas]

to., 04.10.2007 kl. 10.02 +1000, skrev Rusty Russell:
> On Wed, 2007-10-03 at 10:37 +0100, Chris Malley wrote:
> > Hi guys
> > 
> > Would it not be clearer to #include <asm/bootparam.h> and use 
> > the relevant named members of struct setup_header / struct boot_params
> > rather than the hard-coded values 0x202, 0x1F1, 0x214 ?
> 
> Yes, but unfortunately bootparam.h wasn't designed to be included from
> userspace.
> 
[snip]

This change seems to have broken build of the battstat applet in
gnome-applets or rather the included apmlib in there. Intended?

Any pointers on how to adapt the code in case it was?

> diff -r 6bb527d113a8 include/linux/apm_bios.h
> --- a/include/linux/apm_bios.h	Wed Oct 03 13:49:31 2007 +1000
> +++ b/include/linux/apm_bios.h	Thu Oct 04 09:37:28 2007 +1000
> @@ -16,28 +16,28 @@
>   * General Public License for more details.
>   */
>  
> +#include <linux/types.h>
> +
> +struct apm_bios_info {
> +	__u16	version;
> +	__u16	cseg;
> +	__u32	offset;
> +	__u16	cseg_16;
> +	__u16	dseg;
> +	__u16	flags;
> +	__u16	cseg_len;
> +	__u16	cseg_16_len;
> +	__u16	dseg_len;
> +};
> +
> +#ifdef __KERNEL__
> +
>  typedef unsigned short	apm_event_t;
>  typedef unsigned short	apm_eventinfo_t;
> -
> -#ifdef __KERNEL__
> -
> -#include <linux/types.h>
>  
>  #define APM_CS		(GDT_ENTRY_APMBIOS_BASE * 8)
>  #define APM_CS_16	(APM_CS + 8)
>  #define APM_DS		(APM_CS_16 + 8)
> -
> -struct apm_bios_info {
> -	u16	version;
> -	u16	cseg;
> -	u32	offset;
> -	u16	cseg_16;
> -	u16	dseg;
> -	u16	flags;
> -	u16	cseg_len;
> -	u16	cseg_16_len;
> -	u16	dseg_len;
> -};
>  
>  /* Results of APM Installation Check */
>  #define APM_16_BIT_SUPPORT	0x0001

Cheers
Kjartan

Re: [PATCH 5/5] lguest: loading bzImage directly

[H. Peter Anvin]

Kjartan Maraas wrote:
> to., 04.10.2007 kl. 10.02 +1000, skrev Rusty Russell:
>> On Wed, 2007-10-03 at 10:37 +0100, Chris Malley wrote:
>>> Hi guys
>>>
>>> Would it not be clearer to #include <asm/bootparam.h> and use 
>>> the relevant named members of struct setup_header / struct boot_params
>>> rather than the hard-coded values 0x202, 0x1F1, 0x214 ?
>> Yes, but unfortunately bootparam.h wasn't designed to be included from
>> userspace.
>>
> [snip]
> 
> This change seems to have broken build of the battstat applet in
> gnome-applets or rather the included apmlib in there. Intended?
> 
> Any pointers on how to adapt the code in case it was?
> 

Perhaps you could actually give some detail how it broke the code?!

	-hpa

Re: [PATCH 5/5] lguest: loading bzImage directly

[Kjartan Maraas]

lø., 24.11.2007 kl. 14.14 -0800, skrev H. Peter Anvin:
> Kjartan Maraas wrote:
> > to., 04.10.2007 kl. 10.02 +1000, skrev Rusty Russell:
> >> On Wed, 2007-10-03 at 10:37 +0100, Chris Malley wrote:
> >>> Hi guys
> >>>
> >>> Would it not be clearer to #include <asm/bootparam.h> and use 
> >>> the relevant named members of struct setup_header / struct boot_params
> >>> rather than the hard-coded values 0x202, 0x1F1, 0x214 ?
> >> Yes, but unfortunately bootparam.h wasn't designed to be included from
> >> userspace.
> >>
> > [snip]
> > 
> > This change seems to have broken build of the battstat applet in
> > gnome-applets or rather the included apmlib in there. Intended?
> > 
> > Any pointers on how to adapt the code in case it was?
> > 
> 
> Perhaps you could actually give some detail how it broke the code?!
> 
Sorry for being terse. It looks to me like apm_event_t is gone, and that
breaks this bit of code:

Making all in apmlib
make[1]: Entering directory
`/home/kmaraas/cvs/gnome/gnome-applets/battstat/apmlib'
gcc -DHAVE_CONFIG_H -I. -I../.. -I../.. -I../../apmlib -DORBIT2=1
-pthread -I/opt/gnome2/include/panel-2.0 -I/opt/gnome2/include/gtk-2.0
-I/opt/gnome2/include/libgnomeui-2.0
-I/opt/gnome2/include/libbonoboui-2.0 -I/opt/gnome2/lib/gtk-2.0/include
-I/opt/gnome2/include/atk-1.0 -I/opt/gnome2/include/cairo
-I/opt/gnome2/include/pango-1.0 -I/opt/gnome2/include/glib-2.0
-I/opt/gnome2/lib/glib-2.0/include -I/opt/gnome2/include/libgnome-2.0
-I/opt/gnome2/include/libgnomecanvas-2.0
-I/opt/gnome2/include/gnome-vfs-2.0
-I/opt/gnome2/lib/gnome-vfs-2.0/include
-I/opt/gnome2/include/libbonobo-2.0 -I/opt/gnome2/include/orbit-2.0
-I/opt/gnome2/include/bonobo-activation-2.0
-I/opt/gnome2/include/libart-2.0 -I/opt/gnome2/include/gconf/2
-DG_LOG_DOMAIN=\"battstat_applet\"  -Wall -g -O0 -D_FORTIFY_SOURCE=2
-Wall -g -O0 -D_FORTIFY_SOURCE=2 -MT apmlib.o -MD -MP
-MF .deps/apmlib.Tpo -c -o apmlib.o apmlib.c
In file included from apmlib.c:32:
apm.h:56: error: expected declaration specifiers or ‘...’ before
‘apm_event_t’
apm.h:63: error: expected ‘)’ before ‘event’
apmlib.c:243: error: expected declaration specifiers or ‘...’ before
‘apm_event_t’
apmlib.c: In function ‘apm_get_events’:
apmlib.c:257: error: ‘events’ undeclared (first use in this function)
apmlib.c:257: error: (Each undeclared identifier is reported only once
apmlib.c:257: error: for each function it appears in.)
apmlib.c:257: error: ‘apm_event_t’ undeclared (first use in this
function)
apmlib.c:258: warning: control reaches end of non-void function
apmlib.c: At top level:
apmlib.c:366: error: expected ‘)’ before ‘event’
make[1]: *** [apmlib.o] Error 1
make[1]: Leaving directory
`/home/kmaraas/cvs/gnome/gnome-applets/battstat/apmlib'
make: *** [all-recursive] Error 1

Cheers
Kjartan

Re: [PATCH 1/5] update boot spec to 2.07

[rae l]

On 10/3/07, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Proposed updates for version 2.07 of the boot protocol.  This includes:
>
...
> Signed-off-by: Jeremy Fitzhardinge < jeremy@xensource.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Eric W. Biederman" < ebiederm@xmission.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Vivek Goyal <vgoyal@in.ibm.com >
>
> ---
>  Documentation/i386/boot.txt    |   34
Sugguestion is you also add an entry to the header of the file
(Documentation/i386/boot.txt):

something like this:

diff --git a/Documentation/i386/boot.txt b/Documentation/i386/boot.txt
index fc49b79..645bbd7 100644
--- a/Documentation/i386/boot.txt
+++ b/Documentation/i386/boot.txt
@@ -42,6 +42,9 @@ Protocol 2.05:	(Kernel 2.6.20) Make protected mode
kernel relocatable.
 Protocol 2.06:	(Kernel 2.6.22) Added a field that contains the size of
 		the boot command line

+Protocol 2.07:	(Kernel 2.6.23) Added two fields hardware_subarch and
+		hardware_subarch_data to describe the paravirtualized
+		environment.

 **** MEMORY LAYOUT

-- 
Denis Cheng
Linux Application Developer

"One of my most productive days was throwing away 1000 lines of code."
 - Ken Thompson.

Re: [PATCH 0/5] Boot protocol changes

[H. Peter Anvin]

Rusty Russell wrote:
> Hi all,
> 
> 	Jeremy had some boot changes for bzImages, but buried in there was an
> update to the boot protocol to support Xen and lguest (and kvm-lite).
> I've copied those fairly simple patches, and if HPA is happy I'd like to
> push them for 2.6.24 (after correcting for the Great Arch Merge of
> course).
> 

Acked-by: H. Peter Anvin <hpa@zytor.com>

	-hpa

Re: [PATCH 0/5] Boot protocol changes

[Jeremy Fitzhardinge]

Rusty Russell wrote:
> Hi all,
>
> 	Jeremy had some boot changes for bzImages, but buried in there was an
> update to the boot protocol to support Xen and lguest (and kvm-lite).
> I've copied those fairly simple patches, and if HPA is happy I'd like to
> push them for 2.6.24 (after correcting for the Great Arch Merge of
> course).

Ah, good.  I was thinking about reviving this work.  The main problem is
that sticking an ELF header at the 1 meg mark (the address of the
bzImage "payload") breaks 32-bit bootloaders which think they can just
jump to 32-bit code there.  I started a conversation with Eric at KS
about it, but we didn't reach any conclusions.

This series looks like a good start for Xen, but we still need to work
out where to stash the metadata which normally lives in ELF notes.  
Using ELF is convenient for Xen because it lets a large chunk of domain
builder code be reused; on the other hand, loading a plain bzImage is
pretty simple, so maybe it isn't such a big deal.

HPA, Eric: if we don't go the "embed ELF" path, where's a good
backwards-compatible place to stash the note data?  If we do go with
"embed ELF", how should we go about doing it?  Arrange to put the ELF
headers before the 1M mark?

    J

Re: [PATCH 0/5] Boot protocol changes

[H. Peter Anvin]

Jeremy Fitzhardinge wrote:
> Rusty Russell wrote:
>> Hi all,
>>
>> 	Jeremy had some boot changes for bzImages, but buried in there was an
>> update to the boot protocol to support Xen and lguest (and kvm-lite).
>> I've copied those fairly simple patches, and if HPA is happy I'd like to
>> push them for 2.6.24 (after correcting for the Great Arch Merge of
>> course).
> 
> Ah, good.  I was thinking about reviving this work.  The main problem is
> that sticking an ELF header at the 1 meg mark (the address of the
> bzImage "payload") breaks 32-bit bootloaders which think they can just
> jump to 32-bit code there.  I started a conversation with Eric at KS
> about it, but we didn't reach any conclusions.
> 
> This series looks like a good start for Xen, but we still need to work
> out where to stash the metadata which normally lives in ELF notes.  
> Using ELF is convenient for Xen because it lets a large chunk of domain
> builder code be reused; on the other hand, loading a plain bzImage is
> pretty simple, so maybe it isn't such a big deal.
> 
> HPA, Eric: if we don't go the "embed ELF" path, where's a good
> backwards-compatible place to stash the note data?  If we do go with
> "embed ELF", how should we go about doing it?  Arrange to put the ELF
> headers before the 1M mark?
> 

This sounds like another good reason to do the ELF image as the 
postcompression image.  The interface to the embedded compression 
routine is then unchanged, and we get the "full vmlinux" with any notes 
that belongs there.

I'll try to get an implementation of that done -- it really shouldn't be 
very hard.

	-hpa

Re: [PATCH 0/5] Boot protocol changes

[Jeremy Fitzhardinge]

H. Peter Anvin wrote:
>> This series looks like a good start for Xen, but we still need to work
>> out where to stash the metadata which normally lives in ELF notes. 
>> Using ELF is convenient for Xen because it lets a large chunk of domain
>> builder code be reused; on the other hand, loading a plain bzImage is
>> pretty simple, so maybe it isn't such a big deal.
>>
>> HPA, Eric: if we don't go the "embed ELF" path, where's a good
>> backwards-compatible place to stash the note data?  If we do go with
>> "embed ELF", how should we go about doing it?  Arrange to put the ELF
>> headers before the 1M mark?
>>
>
> This sounds like another good reason to do the ELF image as the
> postcompression image.  The interface to the embedded compression
> routine is then unchanged, and we get the "full vmlinux" with any
> notes that belongs there.
>
> I'll try to get an implementation of that done -- it really shouldn't
> be very hard.

Please explain what you're proposing again, because my memory of your
plan from last time wouldn't help in this case.  Are you proposing that
the bzImage contains compressed data that its expecting the bootloader
to decompress?  Won't that completely break backwards compatibility?  If
we don't care about backwards compatibility with old bootloaders, then
it doesn't matter what we do one way or the other.

    J

Re: [PATCH 0/5] Boot protocol changes

[H. Peter Anvin]

Jeremy Fitzhardinge wrote:
> H. Peter Anvin wrote:
>>> This series looks like a good start for Xen, but we still need to work
>>> out where to stash the metadata which normally lives in ELF notes. 
>>> Using ELF is convenient for Xen because it lets a large chunk of domain
>>> builder code be reused; on the other hand, loading a plain bzImage is
>>> pretty simple, so maybe it isn't such a big deal.
>>>
>>> HPA, Eric: if we don't go the "embed ELF" path, where's a good
>>> backwards-compatible place to stash the note data?  If we do go with
>>> "embed ELF", how should we go about doing it?  Arrange to put the ELF
>>> headers before the 1M mark?
>>>
>> This sounds like another good reason to do the ELF image as the
>> postcompression image.  The interface to the embedded compression
>> routine is then unchanged, and we get the "full vmlinux" with any
>> notes that belongs there.
>>
>> I'll try to get an implementation of that done -- it really shouldn't
>> be very hard.
> 
> Please explain what you're proposing again, because my memory of your
> plan from last time wouldn't help in this case.  Are you proposing that
> the bzImage contains compressed data that its expecting the bootloader
> to decompress?  Won't that completely break backwards compatibility?  If
> we don't care about backwards compatibility with old bootloaders, then
> it doesn't matter what we do one way or the other.
> 

No, not at all.

I'm proposing that the existing bzImage format be retained, but that the 
payload of the decompressor (already a gzip file) simply be vmlinux.gz 
-- i.e. a gzip compressed ELF file, notes and all.  A pointer in the 
header will point to the offset of the payload (this is new, obviously.)

The decompression stub is adjusted to expect an ELF image, instead of a 
raw binary.

Existing bootloaders (16- or 32-bit) simply load the bzImage the way 
they do now; new bootloaders have the option of accessing the vmlinux.gz 
directly if they either want to load it themselves or want to examine 
the notes.

	-hpa

Re: [PATCH 0/5] Boot protocol changes

[H. Peter Anvin]

H. Peter Anvin wrote:
>  
> No, not at all.
> 
> I'm proposing that the existing bzImage format be retained, but that the 
> payload of the decompressor (already a gzip file) simply be vmlinux.gz 
> -- i.e. a gzip compressed ELF file, notes and all.  A pointer in the 
> header will point to the offset of the payload (this is new, obviously.)
> 
> The decompression stub is adjusted to expect an ELF image, instead of a 
> raw binary.
> 
> Existing bootloaders (16- or 32-bit) simply load the bzImage the way 
> they do now; new bootloaders have the option of accessing the vmlinux.gz 
> directly if they either want to load it themselves or want to examine 
> the notes.
> 

Slight correction: it does, of course, break loaders which root through 
the bzImage for a gzip header and decode that themselves and place in 
memory.  These loaders are pretty broken, though; they can't deal with 
the fact that the physical address of the kernel is configurable, for 
one thing.

	-hpa

Re: [PATCH 0/5] Boot protocol changes

[Jeremy Fitzhardinge]

H. Peter Anvin wrote:
> I'm proposing that the existing bzImage format be retained, but that
> the payload of the decompressor (already a gzip file) simply be
> vmlinux.gz -- i.e. a gzip compressed ELF file, notes and all.  A
> pointer in the header will point to the offset of the payload (this is
> new, obviously.)
>
> The decompression stub is adjusted to expect an ELF image, instead of
> a raw binary.

It could, or just treat it as a raw binary at 1M+offset to skip the headers.

> Existing bootloaders (16- or 32-bit) simply load the bzImage the way
> they do now; new bootloaders have the option of accessing the
> vmlinux.gz directly if they either want to load it themselves or want
> to examine the notes. 

OK, but that has the same problem as making the payload an ELF file:
32-bit bootloaders which simply jump to 1M will be jumping into data
rather than code - and I got the impression from taking to Eric at KS
that there are such bootloaders.  

If that's not an issue, then I still think the payload should be a plain
ELF file (possibly self-decompressing, or just a plain uncompressed
vmlinux, if that's what's desired).  Still think making a protected-mode
bootloader do the decompression is the wrong way to go about this; ELF
is enough.

    J

Re: [PATCH 0/5] Boot protocol changes

[H. Peter Anvin]

Jeremy Fitzhardinge wrote:
> H. Peter Anvin wrote:
>> I'm proposing that the existing bzImage format be retained, but that
>> the payload of the decompressor (already a gzip file) simply be
>> vmlinux.gz -- i.e. a gzip compressed ELF file, notes and all.  A
>> pointer in the header will point to the offset of the payload (this is
>> new, obviously.)
>>
>> The decompression stub is adjusted to expect an ELF image, instead of
>> a raw binary.
> 
> It could, or just treat it as a raw binary at 1M+offset to skip the headers.

It would be cleaner to actually parse the ELF; it's only a handful of 
lines of code (we don't have to support arbitrary placement of sections, 
obviously, which makes the problem simpler.)

>> Existing bootloaders (16- or 32-bit) simply load the bzImage the way
>> they do now; new bootloaders have the option of accessing the
>> vmlinux.gz directly if they either want to load it themselves or want
>> to examine the notes. 
> 
> OK, but that has the same problem as making the payload an ELF file:
> 32-bit bootloaders which simply jump to 1M will be jumping into data
> rather than code - and I got the impression from taking to Eric at KS
> that there are such bootloaders.  

Uhm, no it doesn't.  Those bootloaders jump to the decompressor, not to 
the payload.  The decompressor interface hasn't changed.

> If that's not an issue, then I still think the payload should be a plain
> ELF file (possibly self-decompressing, or just a plain uncompressed
> vmlinux, if that's what's desired).  Still think making a protected-mode
> bootloader do the decompression is the wrong way to go about this; ELF
> is enough.

It doesn't have to if it doesn't want to; it only needs to do so if it 
wants to access the kernel as an ELF.  Again, it has the advantage that 
the ELF is the real vmlinux, no funnies.

	-hpa