[Qemu-devel] [PATCH 0/6] Fix -kernel with SeaBIOS

[Qemu-devel] [PATCH 0/6] Fix -kernel with SeaBIOS

[Alexander Graf]

SeaBIOS clears RAM between we write our -kernel image to RAM and the int19
handler gets triggered.

So in order to work around that, I sat down and implemented Avi's suggestion
of "downloading" all blobs in runtime from the fw_cfg interface

Thanks to glommer who talked me into doing it ;-).

Alexander Graf (6):
  Make fw_cfg interface 32-bit aware
  Introduce copy_rom
  Convert multiboot to fw_cfg backed data storage
  Move common option rom code to header file
  Convert linux bootrom to external rom and fw_cfg
  Add linuxboot to BLOBS

 Makefile                      |    2 +-
 hw/fw_cfg.c                   |    8 +-
 hw/fw_cfg.h                   |   13 +++-
 hw/loader.c                   |   38 +++++++++
 hw/loader.h                   |    1 +
 hw/pc.c                       |  169 +++++++++++++----------------------------
 pc-bios/optionrom/Makefile    |    2 +-
 pc-bios/optionrom/linuxboot.S |  140 ++++++++++++++++++++++++++++++++++
 pc-bios/optionrom/multiboot.S |  108 ++++++++++-----------------
 pc-bios/optionrom/optionrom.h |  107 ++++++++++++++++++++++++++
 10 files changed, 396 insertions(+), 192 deletions(-)
 create mode 100644 pc-bios/optionrom/linuxboot.S
 create mode 100644 pc-bios/optionrom/optionrom.h

[Qemu-devel] [PATCH 1/6] Make fw_cfg interface 32-bit aware

[Alexander Graf]

The fw_cfg interface can only handle up to 16 bits of data for its streams.
While that isn't too much of a problem when handling integers, we would
like to stream full kernel images over that interface!

So let's extend it to 32 bit length variables.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/fw_cfg.c |    8 ++++----
 hw/fw_cfg.h |    2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index a6d811b..3a3f694 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -39,7 +39,7 @@
 #define FW_CFG_SIZE 2
 
 typedef struct _FWCfgEntry {
-    uint16_t len;
+    uint32_t len;
     uint8_t *data;
     void *callback_opaque;
     FWCfgCallback callback;
@@ -48,7 +48,7 @@ typedef struct _FWCfgEntry {
 typedef struct _FWCfgState {
     FWCfgEntry entries[2][FW_CFG_MAX_ENTRY];
     uint16_t cur_entry;
-    uint16_t cur_offset;
+    uint32_t cur_offset;
 } FWCfgState;
 
 static void fw_cfg_write(FWCfgState *s, uint8_t value)
@@ -171,12 +171,12 @@ static const VMStateDescription vmstate_fw_cfg = {
     .minimum_version_id_old = 1,
     .fields      = (VMStateField []) {
         VMSTATE_UINT16(cur_entry, FWCfgState),
-        VMSTATE_UINT16(cur_offset, FWCfgState),
+        VMSTATE_UINT32(cur_offset, FWCfgState),
         VMSTATE_END_OF_LIST()
     }
 };
 
-int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint16_t len)
+int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint32_t len)
 {
     FWCfgState *s = opaque;
     int arch = !!(key & FW_CFG_ARCH_LOCAL);
diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 30dfec7..359d45a 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -28,7 +28,7 @@
 #ifndef NO_QEMU_PROTOS
 typedef void (*FWCfgCallback)(void *opaque, uint8_t *data);
 
-int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint16_t len);
+int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint32_t len);
 int fw_cfg_add_i16(void *opaque, uint16_t key, uint16_t value);
 int fw_cfg_add_i32(void *opaque, uint16_t key, uint32_t value);
 int fw_cfg_add_i64(void *opaque, uint16_t key, uint64_t value);
-- 
1.6.0.2

[Qemu-devel] [PATCH 2/6] Introduce copy_rom

[Alexander Graf]

We have several rom helpers currently, but none of them can get us
code that spans several roms into a pointer.

This patch introduces a function that copies over rom contents.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/loader.c |   38 ++++++++++++++++++++++++++++++++++++++
 hw/loader.h |    1 +
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/hw/loader.c b/hw/loader.c
index 9153b38..cab53c1 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -701,6 +701,44 @@ static Rom *find_rom(target_phys_addr_t addr)
     return NULL;
 }
 
+int copy_rom(uint8_t *dest, target_phys_addr_t addr, size_t size)
+{
+    target_phys_addr_t end = addr + size;
+    uint8_t *s, *d = dest;
+    size_t l = 0;
+    Rom *rom;
+
+    QTAILQ_FOREACH(rom, &roms, next) {
+        if (rom->max)
+            continue;
+        if (rom->min > addr)
+            continue;
+        if (rom->min + rom->romsize < addr)
+            continue;
+        if (rom->min > end)
+            break;
+        if (!rom->data)
+            continue;
+
+        d = dest + (rom->min - addr);
+        s = rom->data;
+        l = rom->romsize;
+
+        if (rom->min < addr) {
+            d = dest;
+            s += (addr - rom->min);
+            l -= (addr - rom->min);
+        }
+        if ((d + l) > (dest + size)) {
+            l = dest - d;
+        }
+
+        memcpy(d, s, l);
+    }
+
+    return (d + l) - dest;
+}
+
 void *rom_ptr(target_phys_addr_t addr)
 {
     Rom *rom;
diff --git a/hw/loader.h b/hw/loader.h
index 67dae57..6cfb03a 100644
--- a/hw/loader.h
+++ b/hw/loader.h
@@ -24,6 +24,7 @@ int rom_add_file(const char *file,
 int rom_add_blob(const char *name, const void *blob, size_t len,
                  target_phys_addr_t min, target_phys_addr_t max, int align);
 int rom_load_all(void);
+int copy_rom(uint8_t *dest, target_phys_addr_t addr, size_t size);
 void *rom_ptr(target_phys_addr_t addr);
 void do_info_roms(Monitor *mon);
 
-- 
1.6.0.2

[Qemu-devel] [PATCH 3/6] Convert multiboot to fw_cfg backed data storage

[Alexander Graf]

Right now we load the guest kernel to RAM, fire off the BIOS, hope it
doesn't clobber memory and run an option rom that jumps into the kernel.

That breaks with SeaBIOS, as that clears memory. So let's read all
kernel, module etc. data using the fw_cfg interface when in the int19
handler.

This patch implements said mechanism for multiboot.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/fw_cfg.h                   |    5 ++-
 hw/pc.c                       |   43 ++++++++++++++++-------
 pc-bios/optionrom/multiboot.S |   77 ++++++++++++++++++++++++++++++++---------
 3 files changed, 94 insertions(+), 31 deletions(-)

diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 359d45a..1e004b7 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -17,7 +17,10 @@
 #define FW_CFG_NUMA             0x0d
 #define FW_CFG_BOOT_MENU        0x0e
 #define FW_CFG_MAX_CPUS         0x0f
-#define FW_CFG_MAX_ENTRY        0x10
+#define FW_CFG_KERNEL_ENTRY     0x10
+#define FW_CFG_KERNEL_DATA      0x11
+#define FW_CFG_INITRD_DATA      0x12
+#define FW_CFG_MAX_ENTRY        0x13
 
 #define FW_CFG_WRITE_CHANNEL    0x4000
 #define FW_CFG_ARCH_LOCAL       0x8000
diff --git a/hw/pc.c b/hw/pc.c
index bf4718e..291ca1d 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -603,6 +603,8 @@ static int load_multiboot(void *fw_cfg,
     uint32_t mb_mod_end;
     uint8_t bootinfo[0x500];
     uint32_t cmdline = 0x200;
+    uint8_t *mb_kernel_data;
+    uint8_t *mb_bootinfo_data;
 
     /* Ok, let's see if it is a multiboot image.
        The header is 12x32bit long, so the latest entry may be 8192 - 48. */
@@ -643,6 +645,12 @@ static int load_multiboot(void *fw_cfg,
         mh_load_addr = mh_entry_addr = elf_entry;
         mb_kernel_size = kernel_size;
 
+        mb_kernel_data = qemu_malloc(mb_kernel_size);
+        if (copy_rom(mb_kernel_data, elf_entry, kernel_size) != kernel_size) {
+            fprintf(stderr, "Error while fetching elf kernel from rom\n");
+            exit(1);
+        }
+
 #ifdef DEBUG_MULTIBOOT
         fprintf(stderr, "qemu: loading multiboot-elf kernel (%#x bytes) with entry %#zx\n",
                 mb_kernel_size, (size_t)mh_entry_addr);
@@ -656,7 +664,6 @@ static int load_multiboot(void *fw_cfg,
         uint32_t mh_bss_end_addr = ldl_p(header+i+24);
 #endif
         uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr);
-        uint8_t *kernel;
 
         mh_entry_addr = ldl_p(header+i+28);
         mb_kernel_size = get_file_size(f) - mb_kernel_text_offset;
@@ -676,12 +683,9 @@ static int load_multiboot(void *fw_cfg,
                 mb_kernel_size, mh_load_addr);
 #endif
 
-        kernel = qemu_malloc(mb_kernel_size);
+        mb_kernel_data = qemu_malloc(mb_kernel_size);
         fseek(f, mb_kernel_text_offset, SEEK_SET);
-        fread(kernel, 1, mb_kernel_size, f);
-        rom_add_blob_fixed(kernel_filename, kernel, mb_kernel_size,
-                           mh_load_addr);
-        qemu_free(kernel);
+        fread(mb_kernel_data, 1, mb_kernel_size, f);
         fclose(f);
     }
 
@@ -732,9 +736,14 @@ static int load_multiboot(void *fw_cfg,
                 exit(1);
             }
             mb_mod_end = mb_mod_start + mb_mod_length;
-            rom_add_file_fixed(initrd_filename, mb_mod_start);
-
             mb_mod_count++;
+
+            /* append module data at the end of last module */
+            mb_kernel_data = qemu_realloc(mb_kernel_data,
+                                          mh_load_addr - mb_mod_end);
+            load_image(initrd_filename,
+                       mb_kernel_data + mb_mod_start - mh_load_addr);
+
             stl_p(bootinfo + mb_mod_info + 0, mb_mod_start);
             stl_p(bootinfo + mb_mod_info + 4, mb_mod_start + mb_mod_length);
             stl_p(bootinfo + mb_mod_info + 12, 0x0); /* reserved */
@@ -774,13 +783,21 @@ static int load_multiboot(void *fw_cfg,
     fprintf(stderr, "multiboot: mh_entry_addr = %#x\n", mh_entry_addr);
 #endif
 
+    /* save bootinfo off the stack */
+    mb_bootinfo_data = qemu_malloc(sizeof(bootinfo));
+    memcpy(mb_bootinfo_data, bootinfo, sizeof(bootinfo));
+
     /* Pass variables to option rom */
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_entry_addr);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, mb_bootinfo);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, mmap_addr);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, mh_entry_addr);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, mb_mod_end - mh_load_addr);
+    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, mb_kernel_data,
+                     mb_mod_end - mh_load_addr);
 
-    rom_add_blob_fixed("multiboot-info", bootinfo, sizeof(bootinfo),
-                       mb_bootinfo);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, mb_bootinfo);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, sizeof(bootinfo));
+    fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
+                     sizeof(bootinfo));
 
     option_rom[nb_option_roms] = "multiboot.bin";
     nb_option_roms++;
diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S
index e6cbefd..dafac73 100644
--- a/pc-bios/optionrom/multiboot.S
+++ b/pc-bios/optionrom/multiboot.S
@@ -26,6 +26,14 @@
 
 #define MULTIBOOT_MAGIC		0x2badb002
 
+#define GS_PROT_JUMP		0
+#define GS_GDT_DESC		6
+
+/* Break the translation block flow so -d cpu shows us values */
+#define DEBUG_HERE \
+	jmp		1f;				\
+	1:
+	
 /* Read a variable from the fw_cfg device.
    Clobbers:	%edx
    Out:		%eax */
@@ -44,12 +52,31 @@
 	bswap		%eax
 .endm
 
+/*
+ * Read a blob from the fw_cfg device.
+ * Requires _ADDR, _SIZE and _DATA values for the parameter.
+ *
+ * Clobbers:	%eax, %edx, %es, %ecx, %edi
+ */
+#define read_fw_blob(var) \
+	read_fw		var ## _ADDR;			\
+	mov		%eax, %edi;			\
+	read_fw		var ## _SIZE;			\
+	mov		%eax, %ecx;			\
+	mov		$var ## _DATA, %ax;		\
+	mov		$BIOS_CFG_IOPORT_CFG, %edx;	\
+	outw		%ax, (%dx);			\
+	mov		$BIOS_CFG_IOPORT_DATA, %dx;	\
+	cld;						\
+	DEBUG_HERE \
+	rep insb	(%dx), %es:(%edi);
+
 .code16
 .text
 	.global 	_start
 _start:
 	.short		0xaa55
-	.byte		1 /* (_end - _start) / 512 */
+	.byte		(_end - _start) / 512
 	push		%eax
 	push		%ds
 
@@ -57,10 +84,6 @@ _start:
 	xor		%ax, %ax
 	mov		%ax, %ds
 
-	/* save old int 19 */
-	mov		(0x19*4), %eax
-	mov		%eax, %cs:old_int19
-
 	/* install our int 19 handler */
 	movw		$int19_handler, (0x19*4)
 	mov		%cs, (0x19*4+2)
@@ -84,15 +107,34 @@ run_multiboot:
 	mov		%cs, %eax
 	shl		$0x4, %eax
 
-	/* fix the gdt descriptor to be PC relative */
-	mov		(gdt_desc+2), %ebx
-	add		%eax, %ebx
-	mov		%ebx, (gdt_desc+2)
+	/* set up a long jump descriptor that is PC relative */
 
-	/* fix the prot mode indirect jump to be PC relative */
+	/* move stack memory to %gs */
+	mov		%ss, %ecx
+	shl		$0x4, %ecx
+	mov		%esp, %ebx
+	add		%ebx, %ecx
+	sub		$0x20, %ecx
+	sub		$0x30, %esp
+	shr		$0x4, %ecx
+	mov		%cx, %gs
+
+	/* now push the indirect jump decriptor there */
 	mov		(prot_jump), %ebx
 	add		%eax, %ebx
-	mov		%ebx, (prot_jump)
+	movl		%ebx, %gs:GS_PROT_JUMP
+	mov		$8, %bx
+	movw		%bx, %gs:GS_PROT_JUMP + 4
+
+	/* fix the gdt descriptor to be PC relative */
+	movw		(gdt_desc), %bx
+	movw		%bx, %gs:GS_GDT_DESC
+	movl		(gdt_desc+2), %ebx
+	add		%eax, %ebx
+	movl		%ebx, %gs:GS_GDT_DESC + 2
+
+	/* Read the bootinfo struct into RAM */
+	read_fw_blob(FW_CFG_INITRD)
 
 	/* FS = bootinfo_struct */
 	read_fw		FW_CFG_INITRD_ADDR
@@ -100,7 +142,7 @@ run_multiboot:
 	mov		%ax, %fs
 
 	/* ES = mmap_addr */
-	read_fw		FW_CFG_INITRD_SIZE
+	mov		%eax, %fs:0x48
 	shr		$4, %eax
 	mov		%ax, %es
 
@@ -144,7 +186,7 @@ mmap_done:
 real_to_prot:
 	/* Load the GDT before going into protected mode */
 lgdt:
-	data32 lgdt	%cs:gdt_desc
+	data32 lgdt	%gs:GS_GDT_DESC
 
 	/* get us to protected mode now */
 	movl		$1, %eax
@@ -152,7 +194,7 @@ lgdt:
 
 	/* the LJMP sets CS for us and gets us to 32-bit */
 ljmp:
-	data32 ljmp	*%cs:prot_jump
+	data32 ljmp	*%gs:GS_PROT_JUMP
 
 prot_mode:
 .code32
@@ -165,8 +207,11 @@ prot_mode:
 	movl		%eax, %fs
 	movl		%eax, %gs
 
+	/* Read the kernel and modules into RAM */
+	read_fw_blob(FW_CFG_KERNEL)
+
 	/* Jump off to the kernel */
-	read_fw		FW_CFG_KERNEL_ADDR
+	read_fw		FW_CFG_KERNEL_ENTRY
 	mov		%eax, %ecx
 
 	/* EBX contains a pointer to the bootinfo struct */
@@ -180,8 +225,6 @@ ljmp2:
 
 /* Variables */
 .align 4, 0
-old_int19:	.long 0
-
 prot_jump:	.long prot_mode
 		.short 8
 
-- 
1.6.0.2

[Qemu-devel] [PATCH 4/6] Move common option rom code to header file

[Alexander Graf]

We will have a linux boot option rom soon, so let's take all functionality
that might be useful for both to a header file that both roms can include.

That way we only have to write fw_cfg access code once.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 pc-bios/optionrom/multiboot.S |   79 +-----------------------------
 pc-bios/optionrom/optionrom.h |  107 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 76 deletions(-)
 create mode 100644 pc-bios/optionrom/optionrom.h

diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S
index dafac73..be5c9fc 100644
--- a/pc-bios/optionrom/multiboot.S
+++ b/pc-bios/optionrom/multiboot.S
@@ -18,86 +18,15 @@
  *   Authors: Alexander Graf <agraf@suse.de>
  */
 
-#define NO_QEMU_PROTOS
-#include "../../hw/fw_cfg.h"
-
-#define BIOS_CFG_IOPORT_CFG	0x510
-#define BIOS_CFG_IOPORT_DATA	0x511
+#include "optionrom.h"
 
 #define MULTIBOOT_MAGIC		0x2badb002
 
 #define GS_PROT_JUMP		0
 #define GS_GDT_DESC		6
 
-/* Break the translation block flow so -d cpu shows us values */
-#define DEBUG_HERE \
-	jmp		1f;				\
-	1:
-	
-/* Read a variable from the fw_cfg device.
-   Clobbers:	%edx
-   Out:		%eax */
-.macro read_fw VAR
-	mov		$\VAR, %ax
-	mov		$BIOS_CFG_IOPORT_CFG, %dx
-	outw		%ax, (%dx)
-	mov		$BIOS_CFG_IOPORT_DATA, %dx
-	inb		(%dx), %al
-	shl		$8, %eax
-	inb		(%dx), %al
-	shl		$8, %eax
-	inb		(%dx), %al
-	shl		$8, %eax
-	inb		(%dx), %al
-	bswap		%eax
-.endm
 
-/*
- * Read a blob from the fw_cfg device.
- * Requires _ADDR, _SIZE and _DATA values for the parameter.
- *
- * Clobbers:	%eax, %edx, %es, %ecx, %edi
- */
-#define read_fw_blob(var) \
-	read_fw		var ## _ADDR;			\
-	mov		%eax, %edi;			\
-	read_fw		var ## _SIZE;			\
-	mov		%eax, %ecx;			\
-	mov		$var ## _DATA, %ax;		\
-	mov		$BIOS_CFG_IOPORT_CFG, %edx;	\
-	outw		%ax, (%dx);			\
-	mov		$BIOS_CFG_IOPORT_DATA, %dx;	\
-	cld;						\
-	DEBUG_HERE \
-	rep insb	(%dx), %es:(%edi);
-
-.code16
-.text
-	.global 	_start
-_start:
-	.short		0xaa55
-	.byte		(_end - _start) / 512
-	push		%eax
-	push		%ds
-
-	/* setup ds so we can access the IVT */
-	xor		%ax, %ax
-	mov		%ax, %ds
-
-	/* install our int 19 handler */
-	movw		$int19_handler, (0x19*4)
-	mov		%cs, (0x19*4+2)
-
-	pop		%ds
-	pop		%eax
-	lret
-
-int19_handler:
-	/* DS = CS */
-	movw		%cs, %ax
-	movw		%ax, %ds
-
-	/* fall through */
+BOOT_ROM_START
 
 run_multiboot:
 
@@ -249,6 +178,4 @@ gdt_desc:
 .short	(5 * 8) - 1
 .long	gdt
 
-.align 512, 0
-_end:
-
+BOOT_ROM_END
diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h
new file mode 100644
index 0000000..34d69af
--- /dev/null
+++ b/pc-bios/optionrom/optionrom.h
@@ -0,0 +1,107 @@
+/*
+ * Common Option ROM Functions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright Novell Inc, 2009
+ *   Authors: Alexander Graf <agraf@suse.de>
+ */
+
+
+#define NO_QEMU_PROTOS
+#include "../../hw/fw_cfg.h"
+
+#define BIOS_CFG_IOPORT_CFG	0x510
+#define BIOS_CFG_IOPORT_DATA	0x511
+
+/* Break the translation block flow so -d cpu shows us values */
+#define DEBUG_HERE \
+	jmp		1f;				\
+	1:
+	
+/*
+ * Read a variable from the fw_cfg device.
+ * Clobbers:	%edx
+ * Out:		%eax
+ */
+.macro read_fw VAR
+	mov		$\VAR, %ax
+	mov		$BIOS_CFG_IOPORT_CFG, %dx
+	outw		%ax, (%dx)
+	mov		$BIOS_CFG_IOPORT_DATA, %dx
+	inb		(%dx), %al
+	shl		$8, %eax
+	inb		(%dx), %al
+	shl		$8, %eax
+	inb		(%dx), %al
+	shl		$8, %eax
+	inb		(%dx), %al
+	bswap		%eax
+.endm
+
+/*
+ * Read a blob from the fw_cfg device.
+ * Requires _ADDR, _SIZE and _DATA values for the parameter.
+ *
+ * Clobbers:	%eax, %edx, %es, %ecx, %edi
+ */
+#define read_fw_blob(var)				\
+	read_fw		var ## _ADDR;			\
+	mov		%eax, %edi;			\
+	read_fw		var ## _SIZE;			\
+	mov		%eax, %ecx;			\
+	mov		$var ## _DATA, %ax;		\
+	mov		$BIOS_CFG_IOPORT_CFG, %edx;	\
+	outw		%ax, (%dx);			\
+	mov		$BIOS_CFG_IOPORT_DATA, %dx;	\
+	cld;						\
+	rep insb	(%dx), %es:(%edi);
+
+#define OPTION_ROM_START					\
+    .code16;						\
+    .text;						\
+	.global 	_start;				\
+    _start:;						\
+	.short		0xaa55;				\
+	.byte		(_end - _start) / 512;
+
+#define BOOT_ROM_START					\
+	OPTION_ROM_START				\
+	push		%eax;				\
+	push		%ds;				\
+							\
+	/* setup ds so we can access the IVT */		\
+	xor		%ax, %ax;			\
+	mov		%ax, %ds;			\
+							\
+	/* install our int 19 handler */		\
+	movw		$int19_handler, (0x19*4);	\
+	mov		%cs, (0x19*4+2);		\
+							\
+	pop		%ds;				\
+	pop		%eax;				\
+	lret;						\
+							\
+    int19_handler:;					\
+	/* DS = CS */					\
+	movw		%cs, %ax;			\
+	movw		%ax, %ds;
+
+#define OPTION_ROM_END					\
+    .align 512, 0;					\
+    _end:
+
+#define BOOT_ROM_END					\
+	OPTION_ROM_END
+
-- 
1.6.0.2

[Qemu-devel] [PATCH 5/6] Convert linux bootrom to external rom and fw_cfg

[Alexander Graf]

We already have a working multiboot implementation that uses fw_cfg to get
its kernel module etc. data in int19 runtime now.

So what's missing is a working linux boot option rom. While at it I figured it
would be a good idea to take the opcode generator out of pc.c and instead use
a proper option rom, like we do with multiboot.

So here it is - an fw_cfg using option rom for -kernel with linux!

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/fw_cfg.h                   |    8 ++-
 hw/pc.c                       |  126 +++++++------------------------------
 pc-bios/optionrom/Makefile    |    2 +-
 pc-bios/optionrom/linuxboot.S |  140 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 172 insertions(+), 104 deletions(-)
 create mode 100644 pc-bios/optionrom/linuxboot.S

diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 1e004b7..7070c94 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -20,7 +20,13 @@
 #define FW_CFG_KERNEL_ENTRY     0x10
 #define FW_CFG_KERNEL_DATA      0x11
 #define FW_CFG_INITRD_DATA      0x12
-#define FW_CFG_MAX_ENTRY        0x13
+#define FW_CFG_CMDLINE_ADDR     0x13
+#define FW_CFG_CMDLINE_SIZE     0x14
+#define FW_CFG_CMDLINE_DATA     0x15
+#define FW_CFG_SETUP_ADDR       0x16
+#define FW_CFG_SETUP_SIZE       0x17
+#define FW_CFG_SETUP_DATA       0x18
+#define FW_CFG_MAX_ENTRY        0x19
 
 #define FW_CFG_WRITE_CHANNEL    0x4000
 #define FW_CFG_ARCH_LOCAL       0x8000
diff --git a/hw/pc.c b/hw/pc.c
index 291ca1d..ece6c29 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -487,85 +487,6 @@ static void *bochs_bios_init(void)
     return fw_cfg;
 }
 
-/* Generate an initial boot sector which sets state and jump to
-   a specified vector */
-static void generate_bootsect(uint32_t gpr[8], uint16_t segs[6], uint16_t ip)
-{
-    uint8_t rom[512], *p, *reloc;
-    uint8_t sum;
-    int i;
-
-    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 */
-
-    for (i = 0; i < 6; i++) {
-	if (i == 1)		/* Skip CS */
-	    continue;
-
-	*p++ = 0xb8;		/* MOV AX,imm16 */
-	*p++ = segs[i];
-	*p++ = segs[i] >> 8;
-	*p++ = 0x8e;		/* MOV <seg>,AX */
-	*p++ = 0xc0 + (i << 3);
-    }
-
-    for (i = 0; i < 8; i++) {
-	*p++ = 0x66;		/* 32-bit operand size */
-	*p++ = 0xb8 + i;	/* MOV <reg>,imm32 */
-	*p++ = gpr[i];
-	*p++ = gpr[i] >> 8;
-	*p++ = gpr[i] >> 16;
-	*p++ = gpr[i] >> 24;
-    }
-
-    *p++ = 0xea;		/* JMP FAR */
-    *p++ = ip;			/* IP */
-    *p++ = ip >> 8;
-    *p++ = segs[1];		/* CS */
-    *p++ = segs[1] >> 8;
-
-    /* sign rom */
-    sum = 0;
-    for (i = 0; i < (sizeof(rom) - 1); i++)
-        sum += rom[i];
-    rom[sizeof(rom) - 1] = -sum;
-
-    rom_add_blob("linux-bootsect", rom, sizeof(rom),
-                 PC_ROM_MIN_OPTION, PC_ROM_MAX, PC_ROM_ALIGN);
-}
-
 static long get_file_size(FILE *f)
 {
     long where, size;
@@ -812,12 +733,9 @@ static void load_linux(void *fw_cfg,
                        target_phys_addr_t max_ram_size)
 {
     uint16_t protocol;
-    uint32_t gpr[8];
-    uint16_t seg[6];
-    uint16_t real_seg;
     int setup_size, kernel_size, initrd_size = 0, cmdline_size;
     uint32_t initrd_max;
-    uint8_t header[8192], *setup, *kernel;
+    uint8_t header[8192], *setup, *kernel, *initrd_data;
     target_phys_addr_t real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
     FILE *f;
     char *vmode;
@@ -886,9 +804,11 @@ static void load_linux(void *fw_cfg,
     if (initrd_max >= max_ram_size-ACPI_DATA_SIZE)
     	initrd_max = max_ram_size-ACPI_DATA_SIZE-1;
 
-    /* kernel command line */
-    rom_add_blob_fixed("cmdline", kernel_cmdline,
-                       strlen(kernel_cmdline)+1, cmdline_addr);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline)+1);
+    fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
+                     (uint8_t*)strdup(kernel_cmdline),
+                     strlen(kernel_cmdline)+1);
 
     if (protocol >= 0x202) {
 	stl_p(header+0x228, cmdline_addr);
@@ -937,7 +857,13 @@ static void load_linux(void *fw_cfg,
 
 	initrd_size = get_image_size(initrd_filename);
         initrd_addr = (initrd_max-initrd_size) & ~4095;
-        rom_add_file_fixed(initrd_filename, initrd_addr);
+
+        initrd_data = qemu_malloc(initrd_size);
+        load_image(initrd_filename, initrd_data);
+
+        fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
+        fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
+        fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size);
 
 	stl_p(header+0x218, initrd_addr);
 	stl_p(header+0x21c, initrd_size);
@@ -957,21 +883,17 @@ static void load_linux(void *fw_cfg,
     fread(kernel, 1, kernel_size, f);
     fclose(f);
     memcpy(setup, header, MIN(sizeof(header), setup_size));
-    rom_add_blob_fixed("linux-setup", setup,
-                       setup_size, real_addr);
-    rom_add_blob_fixed(kernel_filename, kernel,
-                       kernel_size, prot_addr);
-    qemu_free(setup);
-    qemu_free(kernel);
-
-    /* generate bootsector to set up the initial register state */
-    real_seg = real_addr >> 4;
-    seg[0] = seg[2] = seg[3] = seg[4] = seg[4] = real_seg;
-    seg[1] = real_seg+0x20;	/* CS */
-    memset(gpr, 0, sizeof gpr);
-    gpr[4] = cmdline_addr-real_addr-16;	/* SP (-16 is paranoia) */
-
-    generate_bootsect(gpr, seg, 0);
+
+    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
+    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, kernel, kernel_size);
+
+    fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_ADDR, real_addr);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size);
+    fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup, setup_size);
+
+    option_rom[nb_option_roms] = "linuxboot.bin";
+    nb_option_roms++;
 }
 
 static const int ide_iobase[2] = { 0x1f0, 0x170 };
diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index b01a54e..54db882 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -13,7 +13,7 @@ CFLAGS += -I$(SRC_PATH)
 CFLAGS += $(call cc-option, $(CFLAGS), -fno-stack-protector)
 QEMU_CFLAGS = $(CFLAGS)
 
-build-all: multiboot.bin
+build-all: multiboot.bin linuxboot.bin
 
 %.img: %.o
 	$(call quiet-command,$(LD) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
diff --git a/pc-bios/optionrom/linuxboot.S b/pc-bios/optionrom/linuxboot.S
new file mode 100644
index 0000000..842dd3d
--- /dev/null
+++ b/pc-bios/optionrom/linuxboot.S
@@ -0,0 +1,140 @@
+/*
+ * Linux Boot Option ROM
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright Novell Inc, 2009
+ *   Authors: Alexander Graf <agraf@suse.de>
+ *
+ * Based on code in hw/pc.c.
+ */
+
+#include "optionrom.h"
+
+BOOT_ROM_START
+
+run_linuxboot:
+
+	cli
+	cld
+
+	jmp		copy_kernel
+boot_kernel:
+
+	read_fw		FW_CFG_SETUP_ADDR
+
+	mov		%eax, %ebx
+	shr		$4, %ebx
+
+	/* All segments contain real_addr */
+	mov		%bx, %ds
+	mov		%bx, %es
+	mov		%bx, %fs
+	mov		%bx, %gs
+	mov		%bx, %ss
+
+	/* CX = CS we want to jump to */
+	add		$0x20, %bx
+	mov		%bx, %cx
+
+	/* SP = cmdline_addr-real_addr-16 */
+	read_fw		FW_CFG_CMDLINE_ADDR
+	mov		%eax, %ebx
+	read_fw		FW_CFG_SETUP_ADDR
+	sub		%eax, %ebx
+	sub		$16, %ebx
+	mov		%ebx, %esp
+
+	/* Build indirect retf descriptor */
+	pushw		%cx		/* CS */
+	xor		%ax, %ax
+	pushw		%ax		/* IP = 0 */
+
+	/* Clear registers */
+	xor		%eax, %eax
+	xor		%ebx, %ebx
+	xor		%ecx, %ecx
+	xor		%edx, %edx
+	xor		%edi, %edi
+	xor		%ebp, %ebp
+
+	/* Jump to Linux */
+	retf
+
+
+copy_kernel:
+
+	/* We need to load the kernel into memory we can't access in 16 bit
+	   mode, so let's get into 32 bit mode, write the kernel and jump
+	   back again. */
+
+	/* Set DS to SS+SP - 0x10, so we can write our GDT descriptor there */
+	mov		%ss, %eax
+	shl		$4, %eax
+	add		%esp, %eax
+	sub		$0x10, %eax
+	shr		$4, %eax
+
+	/* Now create the GDT descriptor */
+	mov		%cs, %eax
+	shl		$4, %eax
+	movw		$((3 * 8) - 1), %bx
+	movw		%bx, %gs:0
+	movl		$gdt, %ebx
+	add		%eax, %ebx
+	movl		%ebx, %gs:2
+
+	/* And load the GDT */
+	data32 lgdt	%gs:0
+
+	/* Get us to protected mode now */
+	mov		$1, %eax
+	mov		%eax, %cr0
+
+	/* So we can set DS to a 32-bit segment */
+	mov		$0x10, %eax
+	mov		%eax, %ds
+
+	/* We're now running in 16-bit CS, but 32-bit DS! */
+
+	/* Load kernel and initrd */
+	read_fw_blob(FW_CFG_KERNEL)
+	read_fw_blob(FW_CFG_INITRD)
+	read_fw_blob(FW_CFG_CMDLINE)
+	read_fw_blob(FW_CFG_SETUP)
+
+	/* And now jump into Linux! */
+	mov		$0, %eax
+	mov		%eax, %cr0
+
+	/* DS = CS */
+	mov		%cs, %ax
+	mov		%ax, %ds
+
+	jmp		boot_kernel
+
+/* Variables */
+
+.align 4, 0
+gdt:
+	/* 0x00 */
+.byte	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+
+	/* 0x08: code segment (base=0, limit=0xfffff, type=32bit code exec/read, DPL=0, 4k) */
+.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x9a, 0xcf, 0x00
+
+	/* 0x10: data segment (base=0, limit=0xfffff, type=32bit data read/write, DPL=0, 4k) */
+.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0xcf, 0x00
+
+BOOT_ROM_END
-- 
1.6.0.2

[Qemu-devel] [PATCH 6/6] Add linuxboot to BLOBS

[Alexander Graf]

We should install linuxboot.bin too, so let's add it to the to-be-installed
blobs.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 30f1c9d..a6647c2 100644
--- a/Makefile
+++ b/Makefile
@@ -256,7 +256,7 @@ video.x openbios-sparc32 openbios-sparc64 openbios-ppc \
 pxe-ne2k_pci.bin pxe-rtl8139.bin pxe-pcnet.bin pxe-e1000.bin \
 pxe-virtio.bin pxe-eepro100.bin pxe-pcnet.bin \
 bamboo.dtb petalogix-s3adsp1800.dtb \
-multiboot.bin
+multiboot.bin linuxboot.bin
 else
 BLOBS=
 endif
-- 
1.6.0.2

[Qemu-devel] Re: [PATCH 1/6] Make fw_cfg interface 32-bit aware

[Anthony Liguori]

Alexander Graf wrote:
> The fw_cfg interface can only handle up to 16 bits of data for its streams.
> While that isn't too much of a problem when handling integers, we would
> like to stream full kernel images over that interface!
>
> So let's extend it to 32 bit length variables.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/fw_cfg.c |    8 ++++----
>  hw/fw_cfg.h |    2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
> index a6d811b..3a3f694 100644
> --- a/hw/fw_cfg.c
> +++ b/hw/fw_cfg.c
> @@ -39,7 +39,7 @@
>  #define FW_CFG_SIZE 2
>  
>  typedef struct _FWCfgEntry {
> -    uint16_t len;
> +    uint32_t len;
>      uint8_t *data;
>      void *callback_opaque;
>      FWCfgCallback callback;
> @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry {
>  typedef struct _FWCfgState {
>      FWCfgEntry entries[2][FW_CFG_MAX_ENTRY];
>      uint16_t cur_entry;
> -    uint16_t cur_offset;
> +    uint32_t cur_offset;
>  } FWCfgState;
>  
>  static void fw_cfg_write(FWCfgState *s, uint8_t value)
> @@ -171,12 +171,12 @@ static const VMStateDescription vmstate_fw_cfg = {
>      .minimum_version_id_old = 1,
>      .fields      = (VMStateField []) {
>          VMSTATE_UINT16(cur_entry, FWCfgState),
> -        VMSTATE_UINT16(cur_offset, FWCfgState),
> +        VMSTATE_UINT32(cur_offset, FWCfgState),
>          VMSTATE_END_OF_LIST()
>      }
>  };
>  
> -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint16_t len)
> +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint32_t len)
>  {
>      FWCfgState *s = opaque;
>      int arch = !!(key & FW_CFG_ARCH_LOCAL);
>   

We need to bump a version here.

> diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
> index 30dfec7..359d45a 100644
> --- a/hw/fw_cfg.h
> +++ b/hw/fw_cfg.h
> @@ -28,7 +28,7 @@
>  #ifndef NO_QEMU_PROTOS
>  typedef void (*FWCfgCallback)(void *opaque, uint8_t *data);
>  
> -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint16_t len);
> +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint32_t len);
>  int fw_cfg_add_i16(void *opaque, uint16_t key, uint16_t value);
>  int fw_cfg_add_i32(void *opaque, uint16_t key, uint32_t value);
>  int fw_cfg_add_i64(void *opaque, uint16_t key, uint64_t value);
>   

[Qemu-devel] Re: [PATCH 2/6] Introduce copy_rom

[Anthony Liguori]

Alexander Graf wrote:
> We have several rom helpers currently, but none of them can get us
> code that spans several roms into a pointer.
>
> This patch introduces a function that copies over rom contents.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/loader.c |   38 ++++++++++++++++++++++++++++++++++++++
>  hw/loader.h |    1 +
>  2 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/hw/loader.c b/hw/loader.c
> index 9153b38..cab53c1 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -701,6 +701,44 @@ static Rom *find_rom(target_phys_addr_t addr)
>      return NULL;
>  }
>  
> +int copy_rom(uint8_t *dest, target_phys_addr_t addr, size_t size)
> +{
> +    target_phys_addr_t end = addr + size;
> +    uint8_t *s, *d = dest;
> +    size_t l = 0;
> +    Rom *rom;
> +
> +    QTAILQ_FOREACH(rom, &roms, next) {
> +        if (rom->max)
> +            continue;
> +        if (rom->min > addr)
> +            continue;
> +        if (rom->min + rom->romsize < addr)
> +            continue;
> +        if (rom->min > end)
> +            break;
> +        if (!rom->data)
> +            continue;
> +
> +        d = dest + (rom->min - addr);
> +        s = rom->data;
> +        l = rom->romsize;
> +
> +        if (rom->min < addr) {
> +            d = dest;
> +            s += (addr - rom->min);
> +            l -= (addr - rom->min);
> +        }
> +        if ((d + l) > (dest + size)) {
> +            l = dest - d;
> +        }
> +
> +        memcpy(d, s, l);
> +    }
> +
> +    return (d + l) - dest;
> +}
> +
>  void *rom_ptr(target_phys_addr_t addr)
>  {
>      Rom *rom;
> diff --git a/hw/loader.h b/hw/loader.h
> index 67dae57..6cfb03a 100644
> --- a/hw/loader.h
> +++ b/hw/loader.h
> @@ -24,6 +24,7 @@ int rom_add_file(const char *file,
>  int rom_add_blob(const char *name, const void *blob, size_t len,
>                   target_phys_addr_t min, target_phys_addr_t max, int align);
>  int rom_load_all(void);
> +int copy_rom(uint8_t *dest, target_phys_addr_t addr, size_t size);
>   

rom_copy() would have fit better.

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 1/6] Make fw_cfg interface 32-bit aware

[Alexander Graf]

Anthony Liguori wrote:
> Alexander Graf wrote:
>> The fw_cfg interface can only handle up to 16 bits of data for its
>> streams.
>> While that isn't too much of a problem when handling integers, we would
>> like to stream full kernel images over that interface!
>>
>> So let's extend it to 32 bit length variables.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  hw/fw_cfg.c |    8 ++++----
>>  hw/fw_cfg.h |    2 +-
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
>> index a6d811b..3a3f694 100644
>> --- a/hw/fw_cfg.c
>> +++ b/hw/fw_cfg.c
>> @@ -39,7 +39,7 @@
>>  #define FW_CFG_SIZE 2
>>  
>>  typedef struct _FWCfgEntry {
>> -    uint16_t len;
>> +    uint32_t len;
>>      uint8_t *data;
>>      void *callback_opaque;
>>      FWCfgCallback callback;
>> @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry {
>>  typedef struct _FWCfgState {
>>      FWCfgEntry entries[2][FW_CFG_MAX_ENTRY];
>>      uint16_t cur_entry;
>> -    uint16_t cur_offset;
>> +    uint32_t cur_offset;
>>  } FWCfgState;
>>  
>>  static void fw_cfg_write(FWCfgState *s, uint8_t value)
>> @@ -171,12 +171,12 @@ static const VMStateDescription vmstate_fw_cfg = {
>>      .minimum_version_id_old = 1,
>>      .fields      = (VMStateField []) {
>>          VMSTATE_UINT16(cur_entry, FWCfgState),
>> -        VMSTATE_UINT16(cur_offset, FWCfgState),
>> +        VMSTATE_UINT32(cur_offset, FWCfgState),
>>          VMSTATE_END_OF_LIST()
>>      }
>>  };
>>  
>> -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data,
>> uint16_t len)
>> +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data,
>> uint32_t len)
>>  {
>>      FWCfgState *s = opaque;
>>      int arch = !!(key & FW_CFG_ARCH_LOCAL);
>>   
>
> We need to bump a version here.

Sure - which one?


Alex

[Qemu-devel] Re: [PATCH 1/6] Make fw_cfg interface 32-bit aware

[Anthony Liguori]

Alexander Graf wrote:
> Anthony Liguori wrote:
>   
>> Alexander Graf wrote:
>>     
>>> The fw_cfg interface can only handle up to 16 bits of data for its
>>> streams.
>>> While that isn't too much of a problem when handling integers, we would
>>> like to stream full kernel images over that interface!
>>>
>>> So let's extend it to 32 bit length variables.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>  hw/fw_cfg.c |    8 ++++----
>>>  hw/fw_cfg.h |    2 +-
>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
>>> index a6d811b..3a3f694 100644
>>> --- a/hw/fw_cfg.c
>>> +++ b/hw/fw_cfg.c
>>> @@ -39,7 +39,7 @@
>>>  #define FW_CFG_SIZE 2
>>>  
>>>  typedef struct _FWCfgEntry {
>>> -    uint16_t len;
>>> +    uint32_t len;
>>>      uint8_t *data;
>>>      void *callback_opaque;
>>>      FWCfgCallback callback;
>>> @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry {
>>>  typedef struct _FWCfgState {
>>>      FWCfgEntry entries[2][FW_CFG_MAX_ENTRY];
>>>      uint16_t cur_entry;
>>> -    uint16_t cur_offset;
>>> +    uint32_t cur_offset;
>>>  } FWCfgState;
>>>  
>>>  static void fw_cfg_write(FWCfgState *s, uint8_t value)
>>> @@ -171,12 +171,12 @@ static const VMStateDescription vmstate_fw_cfg = {
>>>      .minimum_version_id_old = 1,
>>>      .fields      = (VMStateField []) {
>>>          VMSTATE_UINT16(cur_entry, FWCfgState),
>>> -        VMSTATE_UINT16(cur_offset, FWCfgState),
>>> +        VMSTATE_UINT32(cur_offset, FWCfgState),
>>>          VMSTATE_END_OF_LIST()
>>>      }
>>>  };
>>>  
>>> -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data,
>>> uint16_t len)
>>> +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data,
>>> uint32_t len)
>>>  {
>>>      FWCfgState *s = opaque;
>>>      int arch = !!(key & FW_CFG_ARCH_LOCAL);
>>>   
>>>       
>> We need to bump a version here.
>>     
>
> Sure - which one?
>   

The version_id field in vmstate_fw_cfg.  You also have to try to support 
older versions which means you may want to either split cur_offset into 
a high and low or ask Juan what the appropriate vodoo would be.

Regards,

Anthony Liguori

> Alex
>   

[Qemu-devel] Re: [PATCH 2/6] Introduce copy_rom

[Alexander Graf]

On 11.11.2009, at 22:57, Anthony Liguori wrote:

> Alexander Graf wrote:
>> We have several rom helpers currently, but none of them can get us
>> code that spans several roms into a pointer.
>>
>> This patch introduces a function that copies over rom contents.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> hw/loader.c |   38 ++++++++++++++++++++++++++++++++++++++
>> hw/loader.h |    1 +
>> 2 files changed, 39 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/loader.c b/hw/loader.c
>> index 9153b38..cab53c1 100644
>> --- a/hw/loader.c
>> +++ b/hw/loader.c
>> @@ -701,6 +701,44 @@ static Rom *find_rom(target_phys_addr_t addr)
>>     return NULL;
>> }
>> +int copy_rom(uint8_t *dest, target_phys_addr_t addr, size_t size)
>> +{
>> +    target_phys_addr_t end = addr + size;
>> +    uint8_t *s, *d = dest;
>> +    size_t l = 0;
>> +    Rom *rom;
>> +
>> +    QTAILQ_FOREACH(rom, &roms, next) {
>> +        if (rom->max)
>> +            continue;
>> +        if (rom->min > addr)
>> +            continue;
>> +        if (rom->min + rom->romsize < addr)
>> +            continue;
>> +        if (rom->min > end)
>> +            break;
>> +        if (!rom->data)
>> +            continue;
>> +
>> +        d = dest + (rom->min - addr);
>> +        s = rom->data;
>> +        l = rom->romsize;
>> +
>> +        if (rom->min < addr) {
>> +            d = dest;
>> +            s += (addr - rom->min);
>> +            l -= (addr - rom->min);
>> +        }
>> +        if ((d + l) > (dest + size)) {
>> +            l = dest - d;
>> +        }
>> +
>> +        memcpy(d, s, l);
>> +    }
>> +
>> +    return (d + l) - dest;
>> +}
>> +
>> void *rom_ptr(target_phys_addr_t addr)
>> {
>>     Rom *rom;
>> diff --git a/hw/loader.h b/hw/loader.h
>> index 67dae57..6cfb03a 100644
>> --- a/hw/loader.h
>> +++ b/hw/loader.h
>> @@ -24,6 +24,7 @@ int rom_add_file(const char *file,
>> int rom_add_blob(const char *name, const void *blob, size_t len,
>>                  target_phys_addr_t min, target_phys_addr_t max,  
>> int align);
>> int rom_load_all(void);
>> +int copy_rom(uint8_t *dest, target_phys_addr_t addr, size_t size);
>>
>
> rom_copy() would have fit better.

Ok, will rename in v2.


Alex

[Qemu-devel] Re: [PATCH 1/6] Make fw_cfg interface 32-bit aware

[Alexander Graf]

On 11.11.2009, at 23:22, Anthony Liguori wrote:

> Alexander Graf wrote:
>> Anthony Liguori wrote:
>>
>>> Alexander Graf wrote:
>>>
>>>> The fw_cfg interface can only handle up to 16 bits of data for its
>>>> streams.
>>>> While that isn't too much of a problem when handling integers, we  
>>>> would
>>>> like to stream full kernel images over that interface!
>>>>
>>>> So let's extend it to 32 bit length variables.
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>> hw/fw_cfg.c |    8 ++++----
>>>> hw/fw_cfg.h |    2 +-
>>>> 2 files changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
>>>> index a6d811b..3a3f694 100644
>>>> --- a/hw/fw_cfg.c
>>>> +++ b/hw/fw_cfg.c
>>>> @@ -39,7 +39,7 @@
>>>> #define FW_CFG_SIZE 2
>>>>  typedef struct _FWCfgEntry {
>>>> -    uint16_t len;
>>>> +    uint32_t len;
>>>>     uint8_t *data;
>>>>     void *callback_opaque;
>>>>     FWCfgCallback callback;
>>>> @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry {
>>>> typedef struct _FWCfgState {
>>>>     FWCfgEntry entries[2][FW_CFG_MAX_ENTRY];
>>>>     uint16_t cur_entry;
>>>> -    uint16_t cur_offset;
>>>> +    uint32_t cur_offset;
>>>> } FWCfgState;
>>>>  static void fw_cfg_write(FWCfgState *s, uint8_t value)
>>>> @@ -171,12 +171,12 @@ static const VMStateDescription  
>>>> vmstate_fw_cfg = {
>>>>     .minimum_version_id_old = 1,
>>>>     .fields      = (VMStateField []) {
>>>>         VMSTATE_UINT16(cur_entry, FWCfgState),
>>>> -        VMSTATE_UINT16(cur_offset, FWCfgState),
>>>> +        VMSTATE_UINT32(cur_offset, FWCfgState),
>>>>         VMSTATE_END_OF_LIST()
>>>>     }
>>>> };
>>>> -int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data,
>>>> uint16_t len)
>>>> +int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data,
>>>> uint32_t len)
>>>> {
>>>>     FWCfgState *s = opaque;
>>>>     int arch = !!(key & FW_CFG_ARCH_LOCAL);
>>>>
>>> We need to bump a version here.
>>>
>>
>> Sure - which one?
>>
>
> The version_id field in vmstate_fw_cfg.  You also have to try to  
> support older versions which means you may want to either split  
> cur_offset into a high and low or ask Juan what the appropriate  
> vodoo would be.

Juan, I'd really love to learn some new voodoo :-).
This whole new qdev whatever based save format was supposed to make  
things like this easy, right? I would've known what to do with the old  
code ...

Alex

[Qemu-devel] Re: [PATCH 1/6] Make fw_cfg interface 32-bit aware

[Anthony Liguori]

Alexander Graf wrote:
> Juan, I'd really love to learn some new voodoo :-).
> This whole new qdev whatever based save format was supposed to make 
> things like this easy, right? I would've known what to do with the old 
> code ...

I think Juan's mentioned something about writing a doc explaining how to 
use VMState correctly.  I think it would certainly be helpful for 
situations like this.

But the most important part of VMState is that it converts something 
that was previously open coded and opaque to something that is 
data-driven and introspectable.  I think it's done an extremely good job 
of achieving those goals.   As we get everything converted, we can 
potentially figure out some ways to make this all a bit easier to 
understand.  Right now, I think how we support backwards compatibility 
is admittedly awkward.

Regards,

Anthony Liguori

> Alex

Re: [Qemu-devel] [PATCH 0/6] Fix -kernel with SeaBIOS

[Christoph Hellwig]

Thanks Alexander,

I hope this goes into qemu.git ASAP - without it it's totally unusable
for my test setup, and it of course bit my silently when preparing a
demo for a confernence where only reverting back to and older qemu
helped..

Re: [Qemu-devel] [PATCH 0/6] Fix -kernel with SeaBIOS

[Alexander Graf]

Am 12.11.2009 um 15:53 schrieb Christoph Hellwig <hch@lst.de>:

> Thanks Alexander,
>
> I hope this goes into qemu.git ASAP - without it it's totally unusable
> for my test setup, and it of course bit my silently when preparing a
> demo for a confernence where only reverting back to and older qemu
> helped..
>

Yeah, I'm only waiting for a reply from Juan. If I don't see anything  
until tonight I'll send out my (untested) compat version that uses  
load_old.

Alex

[Qemu-devel] Re: [PATCH 1/6] Make fw_cfg interface 32-bit aware

[Glauber Costa]

On Thu, Nov 12, 2009 at 09:53:10PM +0100, Alexander Graf wrote:
> The fw_cfg interface can only handle up to 16 bits of data for its streams.
> While that isn't too much of a problem when handling integers, we would
> like to stream full kernel images over that interface!
> 
> So let's extend it to 32 bit length variables.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> v1 -> v2:
> 
>   - add savevm compat code (untested!)
> ---
>  hw/fw_cfg.c |   30 ++++++++++++++++++++++++------
>  hw/fw_cfg.h |    2 +-
>  2 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
> index a6d811b..0cd6f68 100644
> --- a/hw/fw_cfg.c
> +++ b/hw/fw_cfg.c
> @@ -39,7 +39,7 @@
>  #define FW_CFG_SIZE 2
>  
>  typedef struct _FWCfgEntry {
> -    uint16_t len;
> +    uint32_t len;
>      uint8_t *data;
>      void *callback_opaque;
>      FWCfgCallback callback;
> @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry {
>  typedef struct _FWCfgState {
>      FWCfgEntry entries[2][FW_CFG_MAX_ENTRY];
>      uint16_t cur_entry;
> -    uint16_t cur_offset;
> +    uint32_t cur_offset;
>  } FWCfgState;
>  
>  static void fw_cfg_write(FWCfgState *s, uint8_t value)
> @@ -164,19 +164,37 @@ static void fw_cfg_reset(void *opaque)
>      fw_cfg_select(s, 0);
>  }
>  
> +static int fw_cfg_load_old(QEMUFile *f, void *opaque, int version_id)
> +{
> +    FWCfgState *s = opaque;
> +    uint16_t cur_offset;
> +
> +    if (version_id != 1)
> +        return -EINVAL;
> +
> +    qemu_get_be16s(f, &s->cur_entry);
> +
> +    /* Convert old 16 bit value to new 32 bit width */
> +    qemu_get_be16s(f, &cur_offset);
> +    s->cur_offset = cur_offset;
> +
> +    return 0;
> +}
> +
>  static const VMStateDescription vmstate_fw_cfg = {
>      .name = "fw_cfg",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .minimum_version_id_old = 1,
> +    .load_state_old = fw_cfg_load_old,
>      .fields      = (VMStateField []) {
>          VMSTATE_UINT16(cur_entry, FWCfgState),
> -        VMSTATE_UINT16(cur_offset, FWCfgState),
> +        VMSTATE_UINT32(cur_offset, FWCfgState),
>          VMSTATE_END_OF_LIST()
>      }
>  };

Why don't we just add another field for the upper bits, and add it through
VMSTATE_UINT16_V ?

[Qemu-devel] Re: [PATCH 1/6] Make fw_cfg interface 32-bit aware

[Alexander Graf]

On 13.11.2009, at 01:48, Glauber Costa wrote:

> On Thu, Nov 12, 2009 at 09:53:10PM +0100, Alexander Graf wrote:
>> The fw_cfg interface can only handle up to 16 bits of data for its  
>> streams.
>> While that isn't too much of a problem when handling integers, we  
>> would
>> like to stream full kernel images over that interface!
>>
>> So let's extend it to 32 bit length variables.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>
>> ---
>>
>> v1 -> v2:
>>
>>  - add savevm compat code (untested!)
>> ---
>> hw/fw_cfg.c |   30 ++++++++++++++++++++++++------
>> hw/fw_cfg.h |    2 +-
>> 2 files changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
>> index a6d811b..0cd6f68 100644
>> --- a/hw/fw_cfg.c
>> +++ b/hw/fw_cfg.c
>> @@ -39,7 +39,7 @@
>> #define FW_CFG_SIZE 2
>>
>> typedef struct _FWCfgEntry {
>> -    uint16_t len;
>> +    uint32_t len;
>>     uint8_t *data;
>>     void *callback_opaque;
>>     FWCfgCallback callback;
>> @@ -48,7 +48,7 @@ typedef struct _FWCfgEntry {
>> typedef struct _FWCfgState {
>>     FWCfgEntry entries[2][FW_CFG_MAX_ENTRY];
>>     uint16_t cur_entry;
>> -    uint16_t cur_offset;
>> +    uint32_t cur_offset;
>> } FWCfgState;
>>
>> static void fw_cfg_write(FWCfgState *s, uint8_t value)
>> @@ -164,19 +164,37 @@ static void fw_cfg_reset(void *opaque)
>>     fw_cfg_select(s, 0);
>> }
>>
>> +static int fw_cfg_load_old(QEMUFile *f, void *opaque, int  
>> version_id)
>> +{
>> +    FWCfgState *s = opaque;
>> +    uint16_t cur_offset;
>> +
>> +    if (version_id != 1)
>> +        return -EINVAL;
>> +
>> +    qemu_get_be16s(f, &s->cur_entry);
>> +
>> +    /* Convert old 16 bit value to new 32 bit width */
>> +    qemu_get_be16s(f, &cur_offset);
>> +    s->cur_offset = cur_offset;
>> +
>> +    return 0;
>> +}
>> +
>> static const VMStateDescription vmstate_fw_cfg = {
>>     .name = "fw_cfg",
>> -    .version_id = 1,
>> -    .minimum_version_id = 1,
>> +    .version_id = 2,
>> +    .minimum_version_id = 2,
>>     .minimum_version_id_old = 1,
>> +    .load_state_old = fw_cfg_load_old,
>>     .fields      = (VMStateField []) {
>>         VMSTATE_UINT16(cur_entry, FWCfgState),
>> -        VMSTATE_UINT16(cur_offset, FWCfgState),
>> +        VMSTATE_UINT32(cur_offset, FWCfgState),
>>         VMSTATE_END_OF_LIST()
>>     }
>> };
>
> Why don't we just add another field for the upper bits, and add it  
> through
> VMSTATE_UINT16_V ?

Because that would mean I'd have to deal with it in the code later on  
and I don't see the point of writing code that's not in the load/save  
cycle because of limitations there.

Alex

[Qemu-devel] Re: [PATCH 1/6] Make fw_cfg interface 32-bit aware

[Juan Quintela]

Alexander Graf <agraf@suse.de> wrote:
> On 13.11.2009, at 01:48, Glauber Costa wrote:
> Because that would mean I'd have to deal with it in the code later on
> and I don't see the point of writing code that's not in the load/save
> cycle because of limitations there.

Hi

could you take a look at this one?

This don't use the old_state function and should work as well.
I haven't tested it yet (test machine down), but will do a bit later.

Later, Juan.

PD. Yeap, I would have to add the HACK types to hw.h as several places
    have decided to change the size of several fields.

>From 25f7a6e401d72a0584fa4630a9dc97ce34520f7b Mon Sep 17 00:00:00 2001
From: Juan Quintela <quintela@redhat.com>
Date: Fri, 13 Nov 2009 11:56:38 +0100
Subject: [PATCH] fw_cfg: change cur_offset to 32 bits


Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/fw_cfg.c |   44 +++++++++++++++++++++++++++++++++++++++-----
 hw/fw_cfg.h |    2 +-
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index a6d811b..b79d58f 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -39,7 +39,7 @@
 #define FW_CFG_SIZE 2

 typedef struct _FWCfgEntry {
-    uint16_t len;
+    uint32_t len;
     uint8_t *data;
     void *callback_opaque;
     FWCfgCallback callback;
@@ -48,7 +48,7 @@ typedef struct _FWCfgEntry {
 typedef struct _FWCfgState {
     FWCfgEntry entries[2][FW_CFG_MAX_ENTRY];
     uint16_t cur_entry;
-    uint16_t cur_offset;
+    uint32_t cur_offset;
 } FWCfgState;

 static void fw_cfg_write(FWCfgState *s, uint8_t value)
@@ -164,19 +164,53 @@ static void fw_cfg_reset(void *opaque)
     fw_cfg_select(s, 0);
 }

+/* Save restore 32 bit int as uint16_t
+   This is a Big hack, but it is how the old state did it.
+   Or we broke compatibility in the state, or we can't use struct tm
+ */
+
+static int get_uint32_as_uint16(QEMUFile *f, void *pv, size_t size)
+{
+    uint32_t *v = pv;
+    *v = qemu_get_be16(f);
+    return 0;
+}
+
+static void put_unused(QEMUFile *f, void *pv, size_t size)
+{
+    fprintf(stderr, "uint32_as_uint16 is only used for backward compatibilty.\n");
+    fprintf(stderr, "This functions shouldn't be called.\n");
+}
+
+const VMStateInfo vmstate_hack_uint32_as_uint16 = {
+    .name = "int32_as_uint16",
+    .get  = get_uint32_as_uint16,
+    .put  = put_unused,
+};
+
+#define VMSTATE_UINT16_HACK(_f, _s, _t)                                    \
+    VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_hack_uint32_as_uint16, uint32_t)
+
+
+static bool is_version_1(void *opaque, int version_id)
+{
+    return version_id == 1;
+}
+
 static const VMStateDescription vmstate_fw_cfg = {
     .name = "fw_cfg",
-    .version_id = 1,
+    .version_id = 2,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
     .fields      = (VMStateField []) {
         VMSTATE_UINT16(cur_entry, FWCfgState),
-        VMSTATE_UINT16(cur_offset, FWCfgState),
+        VMSTATE_UINT16_HACK(cur_offset, FWCfgState, is_version_1),
+        VMSTATE_UINT32_V(cur_offset, FWCfgState, 2),
         VMSTATE_END_OF_LIST()
     }
 };

-int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint16_t len)
+int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint32_t len)
 {
     FWCfgState *s = opaque;
     int arch = !!(key & FW_CFG_ARCH_LOCAL);
diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 30dfec7..359d45a 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -28,7 +28,7 @@
 #ifndef NO_QEMU_PROTOS
 typedef void (*FWCfgCallback)(void *opaque, uint8_t *data);

-int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint16_t len);
+int fw_cfg_add_bytes(void *opaque, uint16_t key, uint8_t *data, uint32_t len);
 int fw_cfg_add_i16(void *opaque, uint16_t key, uint16_t value);
 int fw_cfg_add_i32(void *opaque, uint16_t key, uint32_t value);
 int fw_cfg_add_i64(void *opaque, uint16_t key, uint64_t value);
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH 1/6] Make fw_cfg interface 32-bit aware

[Alexander Graf]

Am 13.11.2009 um 11:59 schrieb Juan Quintela <quintela@redhat.com>:

> Alexander Graf <agraf@suse.de> wrote:
>> On 13.11.2009, at 01:48, Glauber Costa wrote:
>> Because that would mean I'd have to deal with it in the code later on
>> and I don't see the point of writing code that's not in the load/save
>> cycle because of limitations there.
>
> Hi
>
> could you take a look at this one?
>
> This don't use the old_state function and should work as well.
> I haven't tested it yet (test machine down), but will do a bit later.

Well I suppose your untested is better than my untested :-).


When it comes to save/restore please take anything from Juan rather  
than my patch.

I don't have access to a PC until Monday anyways and IMHO -kernel is a  
pretty important feature for developers that sholdn't stay vroken for  
too long in HEAD.

Alex