[Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Rene Rebe]

Hi all,

Alexander Graf implemented multi-boot kernel loading during
his work to run Darwin inside Qemu/KVM. As the boot loader
expects to load the kernel in an EFI environment a custom
booter is used to load the kernel using a legacy BIOS.

This is a port of the patch to the new extload / INT 19
machinery (including minor cleanups).

Signed-off-by: René Rebe <rene@exactcode.de>

Index: elf_ops.h
===================================================================
--- elf_ops.h	(revision 6501)
+++ elf_ops.h	(working copy)
@@ -195,6 +195,10 @@
      }

      if (ELF_MACHINE != ehdr.e_machine)
+#if (ELF_MACHINE == EM_X86_64) && !defined(CONFIG_USER_ONLY)
+      /* x86_64 systems can run i386 code as well */
+      if(ehdr.e_machine != EM_386)
+#endif
          goto fail;

      if (pentry)
Index: hw/pc.c
===================================================================
--- hw/pc.c	(revision 6501)
+++ hw/pc.c	(working copy)
@@ -533,6 +533,438 @@
      return size;
  }

+/* Generate an initial boot sector which sets state and jump to
+   a specified vector */
+static void generate_bootsect_multiboot(uint8_t *option_rom,
+                                        uint32_t mh_entry_addr, uint32_t bootinfo)
+{
+    uint8_t rom[512], *p, *reloc, *pgdt, *pmmaploop;
+    uint8_t sum;
+    uint32_t ip;
+    int i;
+    int mmaploop;
+
+    memset(rom, 0, sizeof(rom));
+
+    p = rom;
+    /* Make sure we have an option rom signature */
+    *p++ = 0x55;
+    *p++ = 0xaa;
+
+    /* ROM size in sectors*/
+    *p++ = 1;
+
+    /* Hook int19 */
+
+    *p++ = 0x50;                /* push ax */
+    *p++ = 0x1e;                /* push ds */
+    *p++ = 0x31; *p++ = 0xc0;   /* xor ax, ax */
+    *p++ = 0x8e; *p++ = 0xd8;   /* mov ax, ds */
+
+    *p++ = 0xc7; *p++ = 0x06;   /* movvw _start,0x64 */
+    *p++ = 0x64; *p++ = 0x00;
+    reloc = p;
+    *p++ = 0x00; *p++ = 0x00;
+
+    *p++ = 0x8c; *p++ = 0x0e;   /* mov cs,0x66 */
+    *p++ = 0x66; *p++ = 0x00;
+
+    *p++ = 0x1f;                /* pop ds */
+    *p++ = 0x58;                /* pop ax */
+    *p++ = 0xcb;                /* lret */
+
+    /* Actual code */
+    *reloc = (p - rom);
+
+    *p++ = 0xfa;                /* CLI */
+    *p++ = 0xfc;                /* CLD */
+
+    /* 660f011528000000            lgdt [0x28] */
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0x0f;                /* LGDT [0x128] */
+    *p++ = 0x01;
+    *p++ = 0x15;
+    pgdt=p; /* we calculate the gdt position later */
+    p+=4;
+
+    /* Initialize multiboot mmap structs using the 0x15(e820) */
+    *p++ = 0x31;                /* XOR BX,BX */
+    *p++ = 0xdb;
+
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0xbf;                /* MOV EDI,0x9004 */
+    *p++ = 0x04;
+    *p++ = 0x90;
+    *p++ = 0x00;
+    *p++ = 0x00;
+
+    pmmaploop = p;
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0xb8;                /* MOV EAX,0x20 */
+    *p++ = 0x20;
+    *p++ = 0x00;
+    *p++ = 0x00;
+    *p++ = 0x00;
+
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0x89;                /* MOV -4(EDI),EAX */
+    *p++ = 0x47;
+    *p++ = 0xfc;
+
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0xb8;                /* MOV EAX,0x0000e820 */
+    *p++ = 0x20;
+    *p++ = 0xe8;
+    *p++ = 0x00;
+    *p++ = 0x00;
+
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0xba;                /* MOV EDX,0x534d4150 */
+    *p++ = 0x50;
+    *p++ = 0x41;
+    *p++ = 0x4d;
+    *p++ = 0x53;
+
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0xb9;                /* MOV ECX,0x20 */
+    *p++ = 0x20;
+    *p++ = 0x00;
+    *p++ = 0x00;
+    *p++ = 0x00;
+
+    *p++ = 0xcd;                /* INT 0x15 */
+    *p++ = 0x15;
+
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0xb8;                /* MOV EAX, 0x24 */
+    *p++ = 0x24;
+    *p++ = 0x00;
+    *p++ = 0x00;
+    *p++ = 0x00;
+
+    *p++ = 0xf7;                /* MUL AX, BX */
+    *p++ = 0xe3;
+
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0x21;                /* AND EBX, EBX */
+    *p++ = 0xdb;
+
+    /* don't store if bx = 0 */
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0x0f;                /* JZ next instruction */
+    *p++ = 0x84;
+    *p++ = 0x07;
+    *p++ = 0x00;
+    *p++ = 0x00;
+    *p++ = 0x00;
+
+    /* store the amount of blocks in the bootinfo struct */
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0xa3;                /* MOV [bootinfo+0x2c], EAX */
+    *p++ = (bootinfo+0x2c);
+    *p++ = (bootinfo+0x2c) >> 8;
+    *p++ = (bootinfo+0x2c) >> 16;
+    *p++ = (bootinfo+0x2c) >> 24;
+
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0x05;                /* ADD EAX, 0x9004 */
+    *p++ = 0x04;
+    *p++ = 0x90;
+    *p++ = 0x00;
+    *p++ = 0x00;
+
+    *p++ = 0x89;                /* MOV DI, AX */
+    *p++ = 0xc7;
+
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0x21;                /* AND EBX, EBX */
+    *p++ = 0xdb;
+
+    /* process next entry */
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0x67;                /* 32-bit addr size */
+    *p++ = 0x0f;                /* JNZ mmaploop */
+    *p++ = 0x85;
+    mmaploop = (int)((long)pmmaploop) - ((long)p) - 4;
+    *p++ = mmaploop;
+    *p++ = mmaploop >> 8;
+    *p++ = mmaploop >> 16;
+    *p++ = mmaploop >> 24;
+
+    /* get us to protected mode now */
+
+    *p++ = 0x66;
+    *p++ = 0xb8;                /* MOV EAX,0x01 */
+    *p++ = 0x01;
+    *p++ = 0x00;
+    *p++ = 0x00;
+    *p++ = 0x00;
+
+    *p++ = 0x0f;                /* MOV CR0,EAX */
+    *p++ = 0x22;
+    *p++ = 0xc0;
+
+    /* the JMP sets CS for us and gets us to 32-bit */
+    ip = 0x000d0000 + (p - rom) + 8; /* set i to the IP after the JMP */
+    *p++ = 0x66;                /* 32-bit operand size */
+    *p++ = 0xea;                /* JMP */
+    *p++ = ip;        /* IP */
+    *p++ = ip >> 8;
+    *p++ = ip >> 16;
+    *p++ = ip >> 24;
+    *p++ = 0x08;
+    *p++ = 0x00;
+
+    /* initialize all other segments */
+    *p++ = 0xb8;                /* MOV EAX,0x10 */
+    *p++ = 0x10;
+    *p++ = 0x00;
+    *p++ = 0x00;
+    *p++ = 0x00;
+    for (i = 0; i < 6; i++) {
+        if (i == 1)                /* Skip CS */
+            continue;
+
+        *p++ = 0x8e;                /* MOV <seg>,EAX */
+        *p++ = 0xc0 + (i << 3);
+    }
+
+    /* EBX contains a pointer to the bootinfo struct */
+    *p++ = 0xbb;                /* MOV EBX,imm32 */
+    *p++ = bootinfo;
+    *p++ = bootinfo >> 8;
+    *p++ = bootinfo >> 16;
+    *p++ = bootinfo >> 24;
+
+    /* EAX has to contain the following magic */
+    *p++ = 0xb8;                /* MOV EAX,0x2badb002 */
+    *p++ = 0x02;
+    *p++ = 0xb0;
+    *p++ = 0xad;
+    *p++ = 0x2b;
+
+    /* Jump off to the kernel */
+    *p++ = 0xea;                /* JMP */
+    *p++ = mh_entry_addr;        /* IP */
+    *p++ = mh_entry_addr >> 8;
+    *p++ = mh_entry_addr >> 16;
+    *p++ = mh_entry_addr >> 24;
+    *p++ = 0x08;
+    *p++ = 0x00;
+
+    { /* GDT loading */
+        uint32_t gdt_base = 0x000d0000 + (p - rom); /* 0x00007c00 is the first IP */
+        uint32_t gdtr = gdt_base + 0x28;
+        uint8_t gdt[] = { /* GDT base: 0x00000100 */
+            /* 0x00 */
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            /* 0x08: code segment (base=0, limit=0xfffff, type=32bit code exec/read, DPL=0, 4k) */
+            0xff, 0xff, 0x00, 0x00, 0x00, 0x9a, 0xcf, 0x00,
+            /* 0x10: data segment (base=0, limit=0xfffff, type=32bit data read/write, DPL=0, 4k) */
+            0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0xcf, 0x00,
+            /* 0x18: code segment (base=0, limit=0x0ffff, type=16bit code exec/read/conf, DPL=0, 1b) */
+            0xff, 0xff, 0x00, 0x00, 0x00, 0x9e, 0x00, 0x00,
+            /* 0x20: data segment (base=0, limit=0x0ffff, type=16bit data read/write, DPL=0, 1b) */
+            0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0x00, 0x00,
+            /* 0x28: gdtdesc */
+            0x27, 0x00, gdt_base, gdt_base >> 8, gdt_base >> 16, gdt_base >> 24
+        };
+
+        memcpy(p, gdt, sizeof(gdt));
+        p+=sizeof(gdt);
+        *pgdt++ = gdtr;
+        *pgdt++ = gdtr >> 8;
+        *pgdt++ = gdtr >> 16;
+        *pgdt++ = gdtr >> 24;
+    }
+
+    fprintf(stderr, "qemu: multiboot loader code is %d bytes long.\n", (int)(p-rom));
+
+    /* sign rom */
+    sum = 0;
+    for (i = 0; i < (sizeof(rom) - 1); i++)
+        sum += rom[i];
+    rom[sizeof(rom) - 1] = -sum;
+
+    memcpy(option_rom, rom, sizeof(rom));
+}
+
+static int load_multiboot(uint8_t *option_rom,
+                          FILE *f,
+                          const char *kernel_filename,
+                          const char *initrd_filename,
+                          const char *kernel_cmdline,
+                          uint8_t *header)
+{
+    int i, is_multiboot = 0;
+    uint32_t flags = 0;
+    uint32_t mh_entry_addr;
+    uint32_t mh_load_addr;
+    uint32_t mb_kernel_size;
+    uint32_t mb_bootinfo = 0x90000;
+    uint32_t tmp_size;
+
+    /* Ok, let's see if it is a multiboot image */
+    for(i = 0; i < 8144; i += 4) { /* the header is 12x32bit long, so */
+                                   /* the latest entry may be 8192 - 48 */
+        if(ldl_p(header+i) == 0x1BADB002) {
+            uint32_t checksum = ldl_p(header+i+8);
+            flags = ldl_p(header+i+4);
+            checksum += flags;
+            checksum += (uint32_t)0x1BADB002;
+            if(!checksum) {
+                is_multiboot = 1;
+                break;
+            }
+        }
+    }
+
+    if(!is_multiboot) return 0; /* no multiboot */
+    fprintf(stderr, "qemu: I believe we found a multiboot image!\n");
+
+    if(flags & 0x00000004) { /* MULTIBOOT_HEADER_HAS_VBE */
+        fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
+    }
+    if(!(flags & 0x00010000)) { /* MULTIBOOT_HEADER_HAS_ADDR */
+        uint64_t elf_entry;
+        int kernel_size;
+        fclose(f);
+        kernel_size = load_elf(kernel_filename, 0, &elf_entry, NULL, NULL);
+        if(kernel_size < 0) {
+            fprintf(stderr, "Error while loading elf kernel\n");
+            exit(1);
+        }
+        mh_load_addr = mh_entry_addr = elf_entry;
+        mb_kernel_size = kernel_size;
+
+        fprintf(stderr, "qemu: loading multiboot-elf kernel (%#x bytes) with entry %#zx\n",
+                mb_kernel_size, (size_t)mh_entry_addr);
+    } else {
+        /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */
+        uint32_t mh_header_addr = ldl_p(header+i+12);
+        mh_load_addr = ldl_p(header+i+16);
+        uint32_t mh_load_end_addr = ldl_p(header+i+20);
+        uint32_t mh_bss_end_addr = ldl_p(header+i+24);
+        uint8_t *mb_kernel_addr = phys_ram_base + (mh_load_addr);
+        uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr);
+
+        mh_entry_addr = ldl_p(header+i+28);
+        mb_kernel_size = get_file_size(f) - mb_kernel_text_offset;
+
+        /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE.
+        uint32_t mh_mode_type = ldl_p(header+i+32);
+        uint32_t mh_width = ldl_p(header+i+36);
+        uint32_t mh_height = ldl_p(header+i+40);
+        uint32_t mh_depth = ldl_p(header+i+44); */
+
+        fprintf(stderr, "multiboot: mh_header_addr = %#x\n", mh_header_addr);
+        fprintf(stderr, "multiboot: mh_load_addr = %#x\n", mh_load_addr);
+        fprintf(stderr, "multiboot: mh_load_end_addr = %#x\n", mh_load_end_addr);
+        fprintf(stderr, "multiboot: mh_bss_end_addr = %#x\n", mh_bss_end_addr);
+
+        fseek(f, mb_kernel_text_offset, SEEK_SET);
+
+        fprintf(stderr, "qemu: loading multiboot kernel (%#x bytes) at %#zx\n",
+                mb_kernel_size, mb_kernel_addr - phys_ram_base);
+
+        if ((tmp_size=fread(mb_kernel_addr, 1, mb_kernel_size, f)) != mb_kernel_size) {
+            fprintf(stderr, "qemu: read error on multiboot kernel '%s' (%#x != %#x)\n", kernel_filename, tmp_size, mb_kernel_size);
+            exit(1);
+        }
+        fclose(f);
+    }
+
+
+    /* load modules */
+
+    stl_p(phys_ram_base + mb_bootinfo + 20, 0x0); /* mods_count */
+    if(initrd_filename) {
+        uint32_t mb_mod_info = mb_bootinfo + 0x100;
+        uint32_t mb_mod_cmdline = mb_bootinfo+ 0x300;
+        uint32_t mb_mod_start = mh_load_addr;
+        uint32_t mb_mod_length = mb_kernel_size;
+        char *next_initrd;
+        char *next_space;
+        int mb_mod_count = 0;
+
+        do {
+            next_initrd = strchr(initrd_filename, ',');
+            if(next_initrd)
+                *next_initrd = '\0';
+            /* if a space comes after the module filename, treat everything after that as parameters */
+            strcpy(phys_ram_base + mb_mod_cmdline, initrd_filename);
+            stl_p(phys_ram_base + mb_mod_info + 8, mb_mod_cmdline); /* string */
+            mb_mod_cmdline += strlen(initrd_filename) + 1;
+            if(next_space = strchr(initrd_filename, ' '))
+                *next_space = '\0';
+printf("multiboot loading module: %s\n", initrd_filename);
+            f = fopen(initrd_filename, "rb");
+            if(f) {
+                mb_mod_start = (mb_mod_start + mb_mod_length + (TARGET_PAGE_SIZE - 1))
+                             & (TARGET_PAGE_MASK);
+                mb_mod_length = get_file_size(f);
+
+                if ((tmp_size=fread((phys_ram_base + mb_mod_start), 1, mb_mod_length, f))
+                     != mb_mod_length) {
+                    fprintf(stderr, "qemu: read error on multiboot module '%s' (%#x != %#x)\n",
+                            initrd_filename, tmp_size, mb_mod_length);
+                    exit(1);
+                }
+
+                mb_mod_count++;
+                stl_p(phys_ram_base + mb_mod_info + 0, mb_mod_start);
+                stl_p(phys_ram_base + mb_mod_info + 4, mb_mod_start + mb_mod_length);
+printf("mod_start: %#x\nmod_end:   %#x\n", mb_mod_start, mb_mod_start + mb_mod_length);
+                stl_p(phys_ram_base + mb_mod_info + 12, 0x0); /* reserved */
+            }
+            initrd_filename = next_initrd+1;
+            mb_mod_info += 16;
+        } while(next_initrd);
+        stl_p(phys_ram_base + mb_bootinfo + 20, mb_mod_count); /* mods_count */
+        stl_p(phys_ram_base + mb_bootinfo + 24, mb_bootinfo + 0x100); /* mods_addr */
+    }
+
+    /* Commandline support */
+    stl_p(phys_ram_base + mb_bootinfo + 16, mb_bootinfo + 0x200);
+    strcpy((char*)(phys_ram_base + mb_bootinfo + 0x200), kernel_cmdline);
+
+    /* the kernel is where we want it to be now */
+
+#define MULTIBOOT_FLAGS_MEMORY (1 << 0)
+#define MULTIBOOT_FLAGS_BOOT_DEVICE (1 << 1)
+#define MULTIBOOT_FLAGS_CMDLINE (1 << 2)
+#define MULTIBOOT_FLAGS_MODULES (1 << 3)
+#define MULTIBOOT_FLAGS_MMAP (1 << 6)
+    stl_p(phys_ram_base + mb_bootinfo, MULTIBOOT_FLAGS_MEMORY
+                                     | MULTIBOOT_FLAGS_BOOT_DEVICE
+                                     | MULTIBOOT_FLAGS_CMDLINE
+                                     | MULTIBOOT_FLAGS_MODULES
+                                     | MULTIBOOT_FLAGS_MMAP);
+    stl_p(phys_ram_base + mb_bootinfo + 4, 640 * 1024); /* mem_lower */
+    stl_p(phys_ram_base + mb_bootinfo + 8, ram_size); /* mem_upper */
+    stl_p(phys_ram_base + mb_bootinfo + 12, 0x8001ffff); /* XXX: use the -boot switch? */
+    stl_p(phys_ram_base + mb_bootinfo + 48, 0x9000); /* mmap_addr */
+
+    fprintf(stderr, "multiboot: mh_entry_addr = %#x\n", mh_entry_addr);
+
+    generate_bootsect_multiboot(option_rom, mh_entry_addr, mb_bootinfo);
+
+    return 1; /* yes, we are multiboot */
+}
+
  static void load_linux(uint8_t *option_rom,
                         const char *kernel_filename,
  		       const char *initrd_filename,
@@ -544,7 +976,7 @@
      uint16_t real_seg;
      int setup_size, kernel_size, initrd_size, cmdline_size;
      uint32_t initrd_max;
-    uint8_t header[1024];
+    uint8_t header[8192];
      target_phys_addr_t real_addr, prot_addr, cmdline_addr, initrd_addr;
      FILE *f, *fi;

@@ -554,7 +986,7 @@
      /* load the kernel header */
      f = fopen(kernel_filename, "rb");
      if (!f || !(kernel_size = get_file_size(f)) ||
-	fread(header, 1, 1024, f) != 1024) {
+	fread(header, 1, 8192, f) != 8192) {
  	fprintf(stderr, "qemu: could not load kernel '%s'\n",
  		kernel_filename);
  	exit(1);
@@ -566,8 +998,14 @@
  #endif
      if (ldl_p(header+0x202) == 0x53726448)
  	protocol = lduw_p(header+0x206);
-    else
+    else {
+	/* This looks like a multiboot kernel. If it is, let's stop
+	   treating it like Linux. */
+	if (load_multiboot(option_rom, f, kernel_filename, initrd_filename,
+ 	                   kernel_cmdline, header))
+	    return;
  	protocol = 0;
+    }

      if (protocol < 0x200 || !(header[0x211] & 0x01)) {
  	/* Low kernel */
@@ -660,7 +1098,7 @@
      }

      /* store the finalized header and load the rest of the kernel */
-    cpu_physical_memory_write(real_addr, header, 1024);
+    cpu_physical_memory_write(real_addr, header, 8192);

      setup_size = header[0x1f1];
      if (setup_size == 0)
@@ -669,7 +1107,7 @@
      setup_size = (setup_size+1)*512;
      kernel_size -= setup_size;	/* Size of protected-mode code */

-    if (!fread_targphys_ok(real_addr+1024, setup_size-1024, f) ||
+    if (!fread_targphys_ok(real_addr+8192, setup_size-8192, f) ||
  	!fread_targphys_ok(prot_addr, kernel_size, f)) {
  	fprintf(stderr, "qemu: read error on kernel '%s'\n",
  		kernel_filename);


-- 
   René Rebe - ExactCODE GmbH - Europe, Germany, Berlin
   Geschäftsführer: Susanne Klaus, René Rebe
   Sitz: Berlin, Amtsgericht Charlottenburg HRB 105 123 B
   USt-IdNr.: DE251602478
   http://exactcode.de | http://t2-project.org | http://rene.rebe.name

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Nathan Froyd]

On Mon, Feb 02, 2009 at 07:08:00PM +0100, Rene Rebe wrote:
> +    /* 660f011528000000            lgdt [0x28] */
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0x0f;                /* LGDT [0x128] */
> +    *p++ = 0x01;
> +    *p++ = 0x15;
> +    pgdt=p; /* we calculate the gdt position later */
> +    p+=4;

The comments do not appear to agree with the code here.  (The operands
for lgdt are different, at the very least.)

-Nathan

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Alexander Graf]

Nathan Froyd wrote:
> On Mon, Feb 02, 2009 at 07:08:00PM +0100, Rene Rebe wrote:
>   
>> +    /* 660f011528000000            lgdt [0x28] */
>> +    *p++ = 0x66;                /* 32-bit operand size */
>> +    *p++ = 0x67;                /* 32-bit addr size */
>> +    *p++ = 0x0f;                /* LGDT [0x128] */
>> +    *p++ = 0x01;
>> +    *p++ = 0x15;
>> +    pgdt=p; /* we calculate the gdt position later */
>> +    p+=4;
>>     
>
> The comments do not appear to agree with the code here.  (The operands
> for lgdt are different, at the very least.)
>
> -Nathan
>   
Right, because the operands are actually calculated dynamically. Any
suggestions on how to improve that? Maybe LGDT [GDT]?

Alex

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Nathan Froyd]

On Mon, Feb 02, 2009 at 07:17:18PM +0100, Alexander Graf wrote:
> Nathan Froyd wrote:
> > On Mon, Feb 02, 2009 at 07:08:00PM +0100, Rene Rebe wrote:
> >> +    /* 660f011528000000            lgdt [0x28] */
> >> +    *p++ = 0x66;                /* 32-bit operand size */
> >> +    *p++ = 0x67;                /* 32-bit addr size */
> >> +    *p++ = 0x0f;                /* LGDT [0x128] */
> >> +    *p++ = 0x01;
> >> +    *p++ = 0x15;
> >> +    pgdt=p; /* we calculate the gdt position later */
> >> +    p+=4;
> >>     
> >
> > The comments do not appear to agree with the code here.  (The operands
> > for lgdt are different, at the very least.)
>
> Right, because the operands are actually calculated dynamically. Any
> suggestions on how to improve that? Maybe LGDT [GDT]?

Yes, I think that would be an improvement.  Editing the bytestring to
reflect that would be good, as well.

-Nathan

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Anthony Liguori]

Rene Rebe wrote:
> Hi all,
>
> Alexander Graf implemented multi-boot kernel loading during
> his work to run Darwin inside Qemu/KVM. As the boot loader
> expects to load the kernel in an EFI environment a custom
> booter is used to load the kernel using a legacy BIOS.
>
> This is a port of the patch to the new extload / INT 19
> machinery (including minor cleanups).
>
> Signed-off-by: René Rebe <rene@exactcode.de>
>
> Index: elf_ops.h
> ===================================================================
> --- elf_ops.h    (revision 6501)
> +++ elf_ops.h    (working copy)
> @@ -195,6 +195,10 @@
>      }
>
>      if (ELF_MACHINE != ehdr.e_machine)
> +#if (ELF_MACHINE == EM_X86_64) && !defined(CONFIG_USER_ONLY)
> +      /* x86_64 systems can run i386 code as well */
> +      if(ehdr.e_machine != EM_386)
> +#endif
>          goto fail;
>
>      if (pentry)

Seems like an unrelated fix.

> Index: hw/pc.c
> ===================================================================
> --- hw/pc.c    (revision 6501)
> +++ hw/pc.c    (working copy)
> @@ -533,6 +533,438 @@
>      return size;
>  }
>
> +/* Generate an initial boot sector which sets state and jump to
> +   a specified vector */
> +static void generate_bootsect_multiboot(uint8_t *option_rom,
> +                                        uint32_t mh_entry_addr, 
> uint32_t bootinfo)
> +{
> +    uint8_t rom[512], *p, *reloc, *pgdt, *pmmaploop;
> +    uint8_t sum;
> +    uint32_t ip;
> +    int i;
> +    int mmaploop;
> +
> +    memset(rom, 0, sizeof(rom));
> +
> +    p = rom;
> +    /* Make sure we have an option rom signature */
> +    *p++ = 0x55;
> +    *p++ = 0xaa;
> +
> +    /* ROM size in sectors*/
> +    *p++ = 1;
> +
> +    /* Hook int19 */
> +
> +    *p++ = 0x50;                /* push ax */
> +    *p++ = 0x1e;                /* push ds */
> +    *p++ = 0x31; *p++ = 0xc0;   /* xor ax, ax */
> +    *p++ = 0x8e; *p++ = 0xd8;   /* mov ax, ds */
> +
> +    *p++ = 0xc7; *p++ = 0x06;   /* movvw _start,0x64 */
> +    *p++ = 0x64; *p++ = 0x00;
> +    reloc = p;
> +    *p++ = 0x00; *p++ = 0x00;
> +
> +    *p++ = 0x8c; *p++ = 0x0e;   /* mov cs,0x66 */
> +    *p++ = 0x66; *p++ = 0x00;
> +
> +    *p++ = 0x1f;                /* pop ds */
> +    *p++ = 0x58;                /* pop ax */
> +    *p++ = 0xcb;                /* lret */
> +    /* Actual code */
> +    *reloc = (p - rom);

The above could be refactored to common code.

> +    *p++ = 0xfa;                /* CLI */
> +    *p++ = 0xfc;                /* CLD */
> +
> +    /* 660f011528000000            lgdt [0x28] */
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0x0f;                /* LGDT [0x128] */
> +    *p++ = 0x01;
> +    *p++ = 0x15;
> +    pgdt=p; /* we calculate the gdt position later */
> +    p+=4;
> +
> +    /* Initialize multiboot mmap structs using the 0x15(e820) */
> +    *p++ = 0x31;                /* XOR BX,BX */
> +    *p++ = 0xdb;
> +
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0xbf;                /* MOV EDI,0x9004 */
> +    *p++ = 0x04;
> +    *p++ = 0x90;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +
> +    pmmaploop = p;
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0xb8;                /* MOV EAX,0x20 */
> +    *p++ = 0x20;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0x89;                /* MOV -4(EDI),EAX */
> +    *p++ = 0x47;
> +    *p++ = 0xfc;
> +
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0xb8;                /* MOV EAX,0x0000e820 */
> +    *p++ = 0x20;
> +    *p++ = 0xe8;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0xba;                /* MOV EDX,0x534d4150 */
> +    *p++ = 0x50;
> +    *p++ = 0x41;
> +    *p++ = 0x4d;
> +    *p++ = 0x53;
> +
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0xb9;                /* MOV ECX,0x20 */
> +    *p++ = 0x20;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +
> +    *p++ = 0xcd;                /* INT 0x15 */
> +    *p++ = 0x15;
> +
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0xb8;                /* MOV EAX, 0x24 */
> +    *p++ = 0x24;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +
> +    *p++ = 0xf7;                /* MUL AX, BX */
> +    *p++ = 0xe3;
> +
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0x21;                /* AND EBX, EBX */
> +    *p++ = 0xdb;
> +
> +    /* don't store if bx = 0 */
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0x0f;                /* JZ next instruction */
> +    *p++ = 0x84;
> +    *p++ = 0x07;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +
> +    /* store the amount of blocks in the bootinfo struct */
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0xa3;                /* MOV [bootinfo+0x2c], EAX */
> +    *p++ = (bootinfo+0x2c);
> +    *p++ = (bootinfo+0x2c) >> 8;
> +    *p++ = (bootinfo+0x2c) >> 16;
> +    *p++ = (bootinfo+0x2c) >> 24;
> +
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0x05;                /* ADD EAX, 0x9004 */
> +    *p++ = 0x04;
> +    *p++ = 0x90;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +
> +    *p++ = 0x89;                /* MOV DI, AX */
> +    *p++ = 0xc7;
> +
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0x21;                /* AND EBX, EBX */
> +    *p++ = 0xdb;
> +
> +    /* process next entry */
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0x67;                /* 32-bit addr size */
> +    *p++ = 0x0f;                /* JNZ mmaploop */
> +    *p++ = 0x85;
> +    mmaploop = (int)((long)pmmaploop) - ((long)p) - 4;
> +    *p++ = mmaploop;
> +    *p++ = mmaploop >> 8;
> +    *p++ = mmaploop >> 16;
> +    *p++ = mmaploop >> 24;
> +
> +    /* get us to protected mode now */
> +
> +    *p++ = 0x66;
> +    *p++ = 0xb8;                /* MOV EAX,0x01 */
> +    *p++ = 0x01;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +
> +    *p++ = 0x0f;                /* MOV CR0,EAX */
> +    *p++ = 0x22;
> +    *p++ = 0xc0;
> +
> +    /* the JMP sets CS for us and gets us to 32-bit */
> +    ip = 0x000d0000 + (p - rom) + 8; /* set i to the IP after the JMP */
> +    *p++ = 0x66;                /* 32-bit operand size */
> +    *p++ = 0xea;                /* JMP */
> +    *p++ = ip;        /* IP */
> +    *p++ = ip >> 8;
> +    *p++ = ip >> 16;
> +    *p++ = ip >> 24;
> +    *p++ = 0x08;
> +    *p++ = 0x00;
> +
> +    /* initialize all other segments */
> +    *p++ = 0xb8;                /* MOV EAX,0x10 */
> +    *p++ = 0x10;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +    *p++ = 0x00;
> +    for (i = 0; i < 6; i++) {
> +        if (i == 1)                /* Skip CS */
> +            continue;
> +
> +        *p++ = 0x8e;                /* MOV <seg>,EAX */
> +        *p++ = 0xc0 + (i << 3);
> +    }
> +
> +    /* EBX contains a pointer to the bootinfo struct */
> +    *p++ = 0xbb;                /* MOV EBX,imm32 */
> +    *p++ = bootinfo;
> +    *p++ = bootinfo >> 8;
> +    *p++ = bootinfo >> 16;
> +    *p++ = bootinfo >> 24;
> +
> +    /* EAX has to contain the following magic */
> +    *p++ = 0xb8;                /* MOV EAX,0x2badb002 */
> +    *p++ = 0x02;
> +    *p++ = 0xb0;
> +    *p++ = 0xad;
> +    *p++ = 0x2b;
> +
> +    /* Jump off to the kernel */
> +    *p++ = 0xea;                /* JMP */
> +    *p++ = mh_entry_addr;        /* IP */
> +    *p++ = mh_entry_addr >> 8;
> +    *p++ = mh_entry_addr >> 16;
> +    *p++ = mh_entry_addr >> 24;
> +    *p++ = 0x08;
> +    *p++ = 0x00;
> +
> +    { /* GDT loading */
> +        uint32_t gdt_base = 0x000d0000 + (p - rom); /* 0x00007c00 is 
> the first IP */
> +        uint32_t gdtr = gdt_base + 0x28;
> +        uint8_t gdt[] = { /* GDT base: 0x00000100 */
> +            /* 0x00 */
> +            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +            /* 0x08: code segment (base=0, limit=0xfffff, type=32bit 
> code exec/read, DPL=0, 4k) */
> +            0xff, 0xff, 0x00, 0x00, 0x00, 0x9a, 0xcf, 0x00,
> +            /* 0x10: data segment (base=0, limit=0xfffff, type=32bit 
> data read/write, DPL=0, 4k) */
> +            0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0xcf, 0x00,
> +            /* 0x18: code segment (base=0, limit=0x0ffff, type=16bit 
> code exec/read/conf, DPL=0, 1b) */
> +            0xff, 0xff, 0x00, 0x00, 0x00, 0x9e, 0x00, 0x00,
> +            /* 0x20: data segment (base=0, limit=0x0ffff, type=16bit 
> data read/write, DPL=0, 1b) */
> +            0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0x00, 0x00,
> +            /* 0x28: gdtdesc */
> +            0x27, 0x00, gdt_base, gdt_base >> 8, gdt_base >> 16, 
> gdt_base >> 24
> +        };
> +
> +        memcpy(p, gdt, sizeof(gdt));
> +        p+=sizeof(gdt);
> +        *pgdt++ = gdtr;
> +        *pgdt++ = gdtr >> 8;
> +        *pgdt++ = gdtr >> 16;
> +        *pgdt++ = gdtr >> 24;
> +    }

It was already pushing the limit to embed the linux loader.  This is a 
bit too much.  Perhaps we should split it to a separate .S file?  I 
guess we could apply it as is but I'd like to see this code (along with 
the Linux code) moved to another file at least.

> +    fprintf(stderr, "qemu: multiboot loader code is %d bytes 
> long.\n", (int)(p-rom));
> +
> +    /* sign rom */
> +    sum = 0;
> +    for (i = 0; i < (sizeof(rom) - 1); i++)
> +        sum += rom[i];
> +    rom[sizeof(rom) - 1] = -sum;
> +
> +    memcpy(option_rom, rom, sizeof(rom));

This too could be common code.

> +}
> +
> +static int load_multiboot(uint8_t *option_rom,
> +                          FILE *f,
> +                          const char *kernel_filename,
> +                          const char *initrd_filename,
> +                          const char *kernel_cmdline,
> +                          uint8_t *header)
> +{
> +    int i, is_multiboot = 0;
> +    uint32_t flags = 0;
> +    uint32_t mh_entry_addr;
> +    uint32_t mh_load_addr;
> +    uint32_t mb_kernel_size;
> +    uint32_t mb_bootinfo = 0x90000;
> +    uint32_t tmp_size;
> +
> +    /* Ok, let's see if it is a multiboot image */
> +    for(i = 0; i < 8144; i += 4) { /* the header is 12x32bit long, so */
> +                                   /* the latest entry may be 8192 - 
> 48 */
> +        if(ldl_p(header+i) == 0x1BADB002) {
> +            uint32_t checksum = ldl_p(header+i+8);
> +            flags = ldl_p(header+i+4);
> +            checksum += flags;
> +            checksum += (uint32_t)0x1BADB002;
> +            if(!checksum) {
> +                is_multiboot = 1;
> +                break;
> +            }
> +        }
> +    }
> +
> +    if(!is_multiboot) return 0; /* no multiboot */
> +    fprintf(stderr, "qemu: I believe we found a multiboot image!\n");
> +
> +    if(flags & 0x00000004) { /* MULTIBOOT_HEADER_HAS_VBE */
> +        fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
> +    }
> +    if(!(flags & 0x00010000)) { /* MULTIBOOT_HEADER_HAS_ADDR */
> +        uint64_t elf_entry;
> +        int kernel_size;
> +        fclose(f);
> +        kernel_size = load_elf(kernel_filename, 0, &elf_entry, NULL, 
> NULL);
> +        if(kernel_size < 0) {
> +            fprintf(stderr, "Error while loading elf kernel\n");
> +            exit(1);
> +        }
> +        mh_load_addr = mh_entry_addr = elf_entry;
> +        mb_kernel_size = kernel_size;
> +
> +        fprintf(stderr, "qemu: loading multiboot-elf kernel (%#x 
> bytes) with entry %#zx\n",
> +                mb_kernel_size, (size_t)mh_entry_addr);
> +    } else {
> +        /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */
> +        uint32_t mh_header_addr = ldl_p(header+i+12);
> +        mh_load_addr = ldl_p(header+i+16);

Mixing variable definitions and code.

> +        uint32_t mh_load_end_addr = ldl_p(header+i+20);
> +        uint32_t mh_bss_end_addr = ldl_p(header+i+24);
> +        uint8_t *mb_kernel_addr = phys_ram_base + (mh_load_addr);
> +        uint32_t mb_kernel_text_offset = i - (mh_header_addr - 
> mh_load_addr);
> +
> +        mh_entry_addr = ldl_p(header+i+28);
> +        mb_kernel_size = get_file_size(f) - mb_kernel_text_offset;
> +
> +        /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE.
> +        uint32_t mh_mode_type = ldl_p(header+i+32);
> +        uint32_t mh_width = ldl_p(header+i+36);
> +        uint32_t mh_height = ldl_p(header+i+40);
> +        uint32_t mh_depth = ldl_p(header+i+44); */
> +
> +        fprintf(stderr, "multiboot: mh_header_addr = %#x\n", 
> mh_header_addr);
> +        fprintf(stderr, "multiboot: mh_load_addr = %#x\n", 
> mh_load_addr);
> +        fprintf(stderr, "multiboot: mh_load_end_addr = %#x\n", 
> mh_load_end_addr);
> +        fprintf(stderr, "multiboot: mh_bss_end_addr = %#x\n", 
> mh_bss_end_addr);

It's a bit chatty by default.

> +        fseek(f, mb_kernel_text_offset, SEEK_SET);
> +
> +        fprintf(stderr, "qemu: loading multiboot kernel (%#x bytes) 
> at %#zx\n",
> +                mb_kernel_size, mb_kernel_addr - phys_ram_base);
> +
> +        if ((tmp_size=fread(mb_kernel_addr, 1, mb_kernel_size, f)) != 
> mb_kernel_size) {
> +            fprintf(stderr, "qemu: read error on multiboot kernel 
> '%s' (%#x != %#x)\n", kernel_filename, tmp_size, mb_kernel_size);
> +            exit(1);
> +        }
> +        fclose(f);
> +    }
> +
> +
> +    /* load modules */
> +
> +    stl_p(phys_ram_base + mb_bootinfo + 20, 0x0); /* mods_count */
> +    if(initrd_filename) {
> +        uint32_t mb_mod_info = mb_bootinfo + 0x100;
> +        uint32_t mb_mod_cmdline = mb_bootinfo+ 0x300;
> +        uint32_t mb_mod_start = mh_load_addr;
> +        uint32_t mb_mod_length = mb_kernel_size;
> +        char *next_initrd;
> +        char *next_space;
> +        int mb_mod_count = 0;
> +
> +        do {
> +            next_initrd = strchr(initrd_filename, ',');
> +            if(next_initrd)
> +                *next_initrd = '\0';
> +            /* if a space comes after the module filename, treat 
> everything after that as parameters */
> +            strcpy(phys_ram_base + mb_mod_cmdline, initrd_filename);
> +            stl_p(phys_ram_base + mb_mod_info + 8, mb_mod_cmdline); 
> /* string */
> +            mb_mod_cmdline += strlen(initrd_filename) + 1;
> +            if(next_space = strchr(initrd_filename, ' '))
> +                *next_space = '\0';
> +printf("multiboot loading module: %s\n", initrd_filename);

Weird indent.

What all uses multiboot at this point?  I know Xen does.

Some additional documentation would be nice too.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Rene Rebe]

Anthony Liguori wrote:
...
>> +        memcpy(p, gdt, sizeof(gdt));
>> +        p+=sizeof(gdt);
>> +        *pgdt++ = gdtr;
>> +        *pgdt++ = gdtr >> 8;
>> +        *pgdt++ = gdtr >> 16;
>> +        *pgdt++ = gdtr >> 24;
>> +    }
> 
> It was already pushing the limit to embed the linux loader.  This is a 
> bit too much.  Perhaps we should split it to a separate .S file?  I 
> guess we could apply it as is but I'd like to see this code (along with 
> the Linux code) moved to another file at least.

How do you want to assemble the .S file on non x86 builds, or just
ship a pre-built object like the BIOS?

>> +    fprintf(stderr, "qemu: multiboot loader code is %d bytes 
>> long.\n", (int)(p-rom));
>> +
>> +    /* sign rom */
>> +    sum = 0;
>> +    for (i = 0; i < (sizeof(rom) - 1); i++)
>> +        sum += rom[i];
>> +    rom[sizeof(rom) - 1] = -sum;
>> +
>> +    memcpy(option_rom, rom, sizeof(rom));
> 
> This too could be common code.

Ok.
+    } else {
>> +        /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */
>> +        uint32_t mh_header_addr = ldl_p(header+i+12);
>> +        mh_load_addr = ldl_p(header+i+16);
> 
> Mixing variable definitions and code.

Yuck.

>> +        fprintf(stderr, "multiboot: mh_header_addr = %#x\n", 
>> mh_header_addr);
>> +        fprintf(stderr, "multiboot: mh_load_addr = %#x\n", 
>> mh_load_addr);
>> +        fprintf(stderr, "multiboot: mh_load_end_addr = %#x\n", 
>> mh_load_end_addr);
>> +        fprintf(stderr, "multiboot: mh_bss_end_addr = %#x\n", 
>> mh_bss_end_addr);
> 
> It's a bit chatty by default.

Yes, indeed - though quite handy for a yet new  feature. I could
just drop those (maybe after testing booting some other multi-boot
kernel as well, ...).

> Weird indent.
> 
> What all uses multiboot at this point?  I know Xen does.

Maybe other homebrew desktop kernels. I remeber there also
went support into NetBSD some time ago.

> Some additional documentation would be nice too.

Greetings,
   Rene

-- 
   René Rebe - ExactCODE GmbH - Europe, Germany, Berlin
   http://exactcode.de | http://t2-project.org | http://rene.rebe.name

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Alexander Graf]

On 03.02.2009, at 09:15, Rene Rebe wrote:

> Anthony Liguori wrote:

[...]

>
>> Weird indent.
>> What all uses multiboot at this point?  I know Xen does.
>
> Maybe other homebrew desktop kernels. I remeber there also
> went support into NetBSD some time ago.

I know Kevin developed a kernel that was based on multiboot. The  
original purpose was to load the darwin bootloader, modified to run  
with multiboot. Hurd does multiboot too IIRC.

Alex

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Alexander Graf]

On 02.02.2009, at 23:42, Anthony Liguori wrote:

> Rene Rebe wrote:
>> Hi all,
>>
>> Alexander Graf implemented multi-boot kernel loading during
>> his work to run Darwin inside Qemu/KVM. As the boot loader
>> expects to load the kernel in an EFI environment a custom
>> booter is used to load the kernel using a legacy BIOS.
>>
>> This is a port of the patch to the new extload / INT 19
>> machinery (including minor cleanups).
>>
>> Signed-off-by: René Rebe <rene@exactcode.de>
>>
>> Index: elf_ops.h
>> ===================================================================
>> --- elf_ops.h    (revision 6501)
>> +++ elf_ops.h    (working copy)
>> @@ -195,6 +195,10 @@
>>     }
>>
>>     if (ELF_MACHINE != ehdr.e_machine)
>> +#if (ELF_MACHINE == EM_X86_64) && !defined(CONFIG_USER_ONLY)
>> +      /* x86_64 systems can run i386 code as well */
>> +      if(ehdr.e_machine != EM_386)
>> +#endif
>>         goto fail;
>>
>>     if (pentry)
>
> Seems like an unrelated fix.

Actually it's not. If you want to load a 32-bit elf (multiboot)  
executable on an x86_64 system, you need this patch.

Alex

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Rene Rebe]

Alexander Graf wrote:
> 
> On 03.02.2009, at 09:15, Rene Rebe wrote:
> 
>> Anthony Liguori wrote:
> 
> [...]
> 
>>
>>> Weird indent.
>>> What all uses multiboot at this point?  I know Xen does.
>>
>> Maybe other homebrew desktop kernels. I remeber there also
>> went support into NetBSD some time ago.
> 
> I know Kevin developed a kernel that was based on multiboot. The 
> original purpose was to load the darwin bootloader, modified to run with 
> multiboot. Hurd does multiboot too IIRC.

Indeed, how could we overlook to list it? For Hurd the multiboot
SPEC might even been defined initially?

-- 
   René Rebe - ExactCODE GmbH - Europe, Germany, Berlin
   http://exactcode.de | http://t2-project.org | http://rene.rebe.name

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Rene Rebe]

Alexander Graf wrote:
> 
> On 02.02.2009, at 23:42, Anthony Liguori wrote:
> 
>> Rene Rebe wrote:
>>> Hi all,
>>>
>>> Alexander Graf implemented multi-boot kernel loading during
>>> his work to run Darwin inside Qemu/KVM. As the boot loader
>>> expects to load the kernel in an EFI environment a custom
>>> booter is used to load the kernel using a legacy BIOS.
>>>
>>> This is a port of the patch to the new extload / INT 19
>>> machinery (including minor cleanups).
>>>
>>> Signed-off-by: René Rebe <rene@exactcode.de>
>>>
>>> Index: elf_ops.h
>>> ===================================================================
>>> --- elf_ops.h    (revision 6501)
>>> +++ elf_ops.h    (working copy)
>>> @@ -195,6 +195,10 @@
>>>     }
>>>
>>>     if (ELF_MACHINE != ehdr.e_machine)
>>> +#if (ELF_MACHINE == EM_X86_64) && !defined(CONFIG_USER_ONLY)
>>> +      /* x86_64 systems can run i386 code as well */
>>> +      if(ehdr.e_machine != EM_386)
>>> +#endif
>>>         goto fail;
>>>
>>>     if (pentry)
>>
>> Seems like an unrelated fix.
> 
> Actually it's not. If you want to load a 32-bit elf (multiboot) 
> executable on an x86_64 system, you need this patch.

Thanks, I was just about to re-reply to the other mail with this
question lost while answering.

-- 
   René Rebe - ExactCODE GmbH - Europe, Germany, Berlin
   http://exactcode.de | http://t2-project.org | http://rene.rebe.name

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Jamie Lokier]

Alexander Graf wrote:
> >>--- elf_ops.h    (revision 6501)
> >>+++ elf_ops.h    (working copy)
> >>@@ -195,6 +195,10 @@
> >>    }
> >>
> >>    if (ELF_MACHINE != ehdr.e_machine)
> >>+#if (ELF_MACHINE == EM_X86_64) && !defined(CONFIG_USER_ONLY)
> >>+      /* x86_64 systems can run i386 code as well */
> >>+      if(ehdr.e_machine != EM_386)
> >>+#endif
> >>        goto fail;
> >>
> >>    if (pentry)
> >
> >Seems like an unrelated fix.
> 
> Actually it's not. If you want to load a 32-bit elf (multiboot)  
> executable on an x86_64 system, you need this patch.

Why CONFIG_USER_ONLY?

Does it mean qemu-x86_64 (the user one) will behave differently given
a 32-bit ELF file if you built qemu-system-x86_64 at the same time?

-- Jamie

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Alexander Graf]

On 03.02.2009, at 11:53, Jamie Lokier wrote:

> Alexander Graf wrote:
>>>> --- elf_ops.h    (revision 6501)
>>>> +++ elf_ops.h    (working copy)
>>>> @@ -195,6 +195,10 @@
>>>>   }
>>>>
>>>>   if (ELF_MACHINE != ehdr.e_machine)
>>>> +#if (ELF_MACHINE == EM_X86_64) && !defined(CONFIG_USER_ONLY)
>>>> +      /* x86_64 systems can run i386 code as well */
>>>> +      if(ehdr.e_machine != EM_386)
>>>> +#endif
>>>>       goto fail;
>>>>
>>>>   if (pentry)
>>>
>>> Seems like an unrelated fix.
>>
>> Actually it's not. If you want to load a 32-bit elf (multiboot)
>> executable on an x86_64 system, you need this patch.
>
> Why CONFIG_USER_ONLY?
>
> Does it mean qemu-x86_64 (the user one) will behave differently given
> a 32-bit ELF file if you built qemu-system-x86_64 at the same time?

It's !defined(), to ensure the userspace emulation behavior doesn't  
change. Right now, loading an i386 binary with qemu-x86_64 does not  
work - and I don't want to change that for now.

AFAIK CONFIG_USER_ONLY is defined when building qemu-x86_64, but not  
when building qemu-system-x86_64.

Alex

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Kevin Wolf]

Rene Rebe schrieb:
> Hi all,
> 
> Alexander Graf implemented multi-boot kernel loading during
> his work to run Darwin inside Qemu/KVM. As the boot loader
> expects to load the kernel in an EFI environment a custom
> booter is used to load the kernel using a legacy BIOS.
> 
> This is a port of the patch to the new extload / INT 19
> machinery (including minor cleanups).

The memory map doesn't seem to work. It's all zeros for me (haven't
looked for the reason yet) and that kills my kernel. When I disable the
memory map part, the kernel seems to boot correctly and it sees all the
modules with the right command lines.

Once these basic things work, another nice feature would be gzip
support. It took me some minutes until I realized that I tried to feed
it a gzipped kernel which didn't work, of course.

Kevin

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Paul Brook]

On Monday 02 February 2009, Rene Rebe wrote:
> Hi all,
>
> Alexander Graf implemented multi-boot kernel loading during
> his work to run Darwin inside Qemu/KVM. As the boot loader
> expects to load the kernel in an EFI environment a custom
> booter is used to load the kernel using a legacy BIOS.
>
> This is a port of the patch to the new extload / INT 19
> machinery (including minor cleanups).

This is introducing quite a lot of nontrivial target code in a hard to 
understand and maintain way. This seems like a bad idea.

I suspect it would be better to implement this as an actual boot rom, and have 
it read arguments from the firmware device. Ideally the same rom would also 
cover the linux kernel case.

Paul

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Rene Rebe]

Hi,

Kevin Wolf wrote:
> Rene Rebe schrieb:
>> Hi all,
>>
>> Alexander Graf implemented multi-boot kernel loading during
>> his work to run Darwin inside Qemu/KVM. As the boot loader
>> expects to load the kernel in an EFI environment a custom
>> booter is used to load the kernel using a legacy BIOS.
>>
>> This is a port of the patch to the new extload / INT 19
>> machinery (including minor cleanups).
> 
> The memory map doesn't seem to work. It's all zeros for me (haven't
> looked for the reason yet) and that kills my kernel. When I disable the
> memory map part, the kernel seems to boot correctly and it sees all the
> modules with the right command lines.
> 
> Once these basic things work, another nice feature would be gzip
> support. It took me some minutes until I realized that I tried to feed
> it a gzipped kernel which didn't work, of course.

I think I noticed the same with the GRUB example kernel. I'll
take a look at some later hour.

Is your kernel something you can share for testing?


-- 
   René Rebe - ExactCODE GmbH - Europe, Germany, Berlin
   http://exactcode.de | http://t2-project.org | http://rene.rebe.name

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Rene Rebe]

Paul Brook wrote:
> On Monday 02 February 2009, Rene Rebe wrote:
>> Hi all,
>>
>> Alexander Graf implemented multi-boot kernel loading during
>> his work to run Darwin inside Qemu/KVM. As the boot loader
>> expects to load the kernel in an EFI environment a custom
>> booter is used to load the kernel using a legacy BIOS.
>>
>> This is a port of the patch to the new extload / INT 19
>> machinery (including minor cleanups).
> 
> This is introducing quite a lot of nontrivial target code in a hard to 
> understand and maintain way. This seems like a bad idea.
> 
> I suspect it would be better to implement this as an actual boot rom, and have 
> it read arguments from the firmware device. Ideally the same rom would also 
> cover the linux kernel case.

Hm. As I'm not yet memorized the Qemu code and and build system
maybe someone wants to shape the existing option rom code
in the way envisioned to base this patch series on?

Another option would also to apply (a cleaned up) version
of the patch as it is quite local and intrusive code and
someone else further re-factors it into an externally built
option rom?


Greetings,

-- 
   René Rebe - ExactCODE GmbH - Europe, Germany, Berlin
   http://exactcode.de | http://t2-project.org | http://rene.rebe.name

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Kevin Wolf]

Rene Rebe schrieb:
> Hi,
> 
> Kevin Wolf wrote:
>> Rene Rebe schrieb:
>>> Hi all,
>>>
>>> Alexander Graf implemented multi-boot kernel loading during
>>> his work to run Darwin inside Qemu/KVM. As the boot loader
>>> expects to load the kernel in an EFI environment a custom
>>> booter is used to load the kernel using a legacy BIOS.
>>>
>>> This is a port of the patch to the new extload / INT 19
>>> machinery (including minor cleanups).
>>
>> The memory map doesn't seem to work. It's all zeros for me (haven't
>> looked for the reason yet) and that kills my kernel. When I disable the
>> memory map part, the kernel seems to boot correctly and it sees all the
>> modules with the right command lines.
>>
>> Once these basic things work, another nice feature would be gzip
>> support. It took me some minutes until I realized that I tried to feed
>> it a gzipped kernel which didn't work, of course.
> 
> I think I noticed the same with the GRUB example kernel. I'll
> take a look at some later hour.
> 
> Is your kernel something you can share for testing?

Sure. I tried it with with several homebrew kernels from
http://lowlevel.brainsware.org/wiki/index.php/Kategorie:Betriebssystem
(page is in German) - to be precise it was tyndur, meinOS, Xantorix and
lightOS. Links to the sources and/or binaries for each one are in the
respective Wiki articles.

I could also send you a tarball with all of them off-list if you like.

Kevin

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Rene Rebe]

Hi,

On 03.02.2009, at 15:31, Kevin Wolf wrote:

> Rene Rebe schrieb:
>> Hi,
>>
>> Kevin Wolf wrote:
>>> Rene Rebe schrieb:
>>>> Hi all,
>>>>
>>>> Alexander Graf implemented multi-boot kernel loading during
>>>> his work to run Darwin inside Qemu/KVM. As the boot loader
>>>> expects to load the kernel in an EFI environment a custom
>>>> booter is used to load the kernel using a legacy BIOS.
>>>>
>>>> This is a port of the patch to the new extload / INT 19
>>>> machinery (including minor cleanups).
>>>
>>> The memory map doesn't seem to work. It's all zeros for me (haven't
>>> looked for the reason yet) and that kills my kernel. When I  
>>> disable the
>>> memory map part, the kernel seems to boot correctly and it sees  
>>> all the
>>> modules with the right command lines.
>>>
>>> Once these basic things work, another nice feature would be gzip
>>> support. It took me some minutes until I realized that I tried to  
>>> feed
>>> it a gzipped kernel which didn't work, of course.
>>
>> I think I noticed the same with the GRUB example kernel. I'll
>> take a look at some later hour.
>>
>> Is your kernel something you can share for testing?
>
> Sure. I tried it with with several homebrew kernels from
> http://lowlevel.brainsware.org/wiki/index.php/Kategorie:Betriebssystem
> (page is in German) - to be precise it was tyndur, meinOS, Xantorix  
> and
> lightOS. Links to the sources and/or binaries for each one are in the
> respective Wiki articles.
>
> I could also send you a tarball with all of them off-list if you like.


 From reading the last mails I thought you would have your own kernel.

I now have the mmap, memory sizes (kB, not Bytes) fixed and some
other comments from the list taken into account (ARRAY_SIZE, etc.).

   http://svn.exactcode.de/t2/trunk/package/emulators/kvm/09-qemu-multiboot.patch

As I did not yet feel like extracting all the kernels and their modules
from the cited disk / floppy images so it's not yet tested with those
"homebrew" kernels.

Further comments welcome,

-- 
   René Rebe - ExactCODE GmbH - Europe, Germany, Berlin
   http://exactcode.de | http://t2-project.org | http://rene.rebe.name

Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support

[Kevin Wolf]

Rene Rebe schrieb:
>>> I think I noticed the same with the GRUB example kernel. I'll
>>> take a look at some later hour.
>>>
>>> Is your kernel something you can share for testing?
>>
>> Sure. I tried it with with several homebrew kernels from
>> http://lowlevel.brainsware.org/wiki/index.php/Kategorie:Betriebssystem
>> (page is in German) - to be precise it was tyndur, meinOS, Xantorix and
>> lightOS. Links to the sources and/or binaries for each one are in the
>> respective Wiki articles.
>>
>> I could also send you a tarball with all of them off-list if you like.
> 
> From reading the last mails I thought you would have your own kernel.

Well, I didn't say that I'm not involved in one of these. tyndur is the
one, even if the broken multiboot patch proved that there is room for
improvement in the error handling. (*cough* Triple fault... ;-))

> I now have the mmap, memory sizes (kB, not Bytes) fixed and some
> other comments from the list taken into account (ARRAY_SIZE, etc.).
> 
>  
> http://svn.exactcode.de/t2/trunk/package/emulators/kvm/09-qemu-multiboot.patch
> 
> 
> As I did not yet feel like extracting all the kernels and their modules
> from the cited disk / floppy images so it's not yet tested with those
> "homebrew" kernels.

Looks much better now. Not all systems complete the boot (I would
suspect the OSes to be buggy here though), but no horrible crashes any more.

Well, except that with the lightOS kernel I still managed to get a qemu
segfault in the fread in pc.c:939 when loading a module, and other
modules failed to load ("read error on multiboot module abc (0 != x)").
Interestingly the same modules are loaded without any problem if I use a
different kernel. I guess we'd better fix that before committing.

Kevin