[Qemu-devel] [PATCH 0/2] extboot reloaded.

[Qemu-devel] [PATCH 0/2] extboot reloaded.

[Gerd Hoffmann]

  Hi,

How about this one?  extboot rom and interface are unmodified.

The windup in qemu is very different though.  It is now implemented as
isa bus device (which is what extboot actually is).  It is properly
integrated into qdev.  It is much less invasive than the previous
version.  The patch just drops a hw/extboot.c file into the tree and
adds it to Makefile.target.  No code changes anywhere else in qemu.

The magic words to activate extboot are '-device extboot,drive=<name>'.

cheers,
  Gerd

[Qemu-devel] [PATCH 1/2] extboot: add option rom.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile                    |    2 +-
 pc-bios/optionrom/Makefile  |    2 +-
 pc-bios/optionrom/extboot.S |  680 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 682 insertions(+), 2 deletions(-)
 create mode 100644 pc-bios/optionrom/extboot.S

diff --git a/Makefile b/Makefile
index fe5d868..2d6f244 100644
--- a/Makefile
+++ b/Makefile
@@ -260,7 +260,7 @@ pxe-e1000.bin pxe-i82559er.bin \
 pxe-ne2k_pci.bin pxe-pcnet.bin \
 pxe-rtl8139.bin pxe-virtio.bin \
 bamboo.dtb petalogix-s3adsp1800.dtb \
-multiboot.bin
+multiboot.bin extboot.bin
 else
 BLOBS=
 endif
diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index b01a54e..73e74d8 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 extboot.bin
 
 %.img: %.o
 	$(call quiet-command,$(LD) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
diff --git a/pc-bios/optionrom/extboot.S b/pc-bios/optionrom/extboot.S
new file mode 100644
index 0000000..79a4674
--- /dev/null
+++ b/pc-bios/optionrom/extboot.S
@@ -0,0 +1,680 @@
+/*
+ * Extended 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, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * Copyright IBM Corporation, 2007
+ *   Authors: Anthony Liguori <aliguori@us.ibm.com>
+ */
+
+.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
+
+	/* there is one more bootable HD */
+	incb 0x0475
+
+	/* save old int 19 */
+	mov (0x19*4), %eax
+	mov %eax, %cs:old_int19
+
+	/* install out int 19 handler */
+	movw $int19_handler, (0x19*4)
+	mov %cs, (0x19*4+2)
+
+	pop %ds
+	pop %eax
+	lret
+
+int19_handler:
+	push %eax
+	push %bx
+	push %cx
+	push %dx
+	push %ds
+
+	/* setup ds to access IVT */
+	xor %ax, %ax
+	mov %ax, %ds
+
+	movw $0x404, %dx
+	inb %dx, %al
+	cmp $1, %al
+	je 1f
+	jmp 3f
+
+1: /* hook int13: intb(0x404) == 1 */
+	/* save old int 13 to int 2c */
+	mov (0x13*4), %eax
+	mov %eax, %cs:old_int13
+
+	/* install our int 13 handler */
+	movw $int13_handler, (0x13*4)
+	mov %cs, (0x13*4+2)
+
+3: /* fall through: inb(0x404) == anything else */
+	/* restore previous int $0x19 handler */
+	mov %cs:old_int19,%eax
+	mov %eax,(0x19*4)
+	
+	pop %ds
+	pop %dx
+	pop %cx
+	pop %bx
+	pop %eax
+	ljmpw *%cs:old_int19
+
+#define FLAGS_CF	0x01
+
+/* The two macro below clear/set the carry flag to indicate the status
+ * of the interrupt execution. It is not enough to issue a clc/stc instruction,
+ * since the value of the flags register will be overwritten by whatever is
+ * in the stack frame
+ */
+.macro clc_stack
+	push %bp
+	mov %sp, %bp
+	/* 8 = 2 (bp, just pushed) + 2 (ip) + 3 (real mode interrupt frame) */
+	and $(~FLAGS_CF), 8(%bp)
+	pop %bp
+.endm
+
+.macro stc_stack
+	push %bp
+	/* 8 = 2 (bp, just pushed) + 2 (ip) + 3 (real mode interrupt frame) */
+	or $(FLAGS_CF), 8(%bp)
+	pop %bp
+.endm
+
+/* we clobber %bx */
+.macro alloca size
+	push %ds
+	push %bp
+	mov %sp, %bp  /* remember the current stack position */
+
+	mov %ss, %bx
+	mov %bx, %ds
+
+	sub \size, %sp
+	and $(~0x0F), %sp
+	mov %sp, %bx
+
+	push %bp
+	mov 0(%bp), %bp
+.endm
+
+/* we clobber %bp */
+.macro allocbpa size
+	mov %sp, %bp  /* remember the current stack position */
+	sub \size, %sp
+	and $(~0x0F), %sp
+	push %bp
+	mov %sp, %bp
+	add $2, %bp
+.endm
+
+.macro freea
+	pop %sp
+	add $2, %sp
+	pop %ds
+.endm
+
+.macro freebpa
+	pop %sp
+.endm
+
+.macro dump reg
+	push %ax
+	push %dx
+
+	mov \reg, %ax
+	mov $0x406, %dx
+	outw %ax, %dx
+
+	pop %dx
+	pop %ax
+.endm
+
+.macro callout value
+	push %bp
+	push %bx
+	mov %sp, %bp
+	alloca $16
+	push %ax
+	push %dx
+
+	mov %ax, 0(%bx)     /* ax */
+	mov 0(%bp), %ax     /* bx */
+	mov %ax, 2(%bx)
+	mov %cx, 4(%bx)     /* cx */
+	mov %dx, 6(%bx)     /* dx */
+	mov %si, 8(%bx)     /* si */
+	mov %ds, 10(%bx)    /* ds */
+	mov %es, 12(%bx)    /* ds */
+	movw \value, 14(%bx) /* value */
+
+	mov %bx, %ax
+	shr $4, %ax
+	mov %ds, %dx
+	add %dx, %ax
+
+	mov $0x407, %dx
+	outw %ax, %dx
+
+	pop %dx
+	pop %ax
+	freea
+	pop %bx
+	pop %bp
+.endm
+
+send_command:
+	push %bp
+	mov %sp, %bp
+	push %ax
+	push %bx
+	push %dx
+
+	mov 4(%bp), %ax
+	shr $4, %ax
+	and $0x0FFF, %ax
+	mov %ss, %bx
+	add %bx, %ax
+
+	mov $0x405, %dx
+	outw %ax, %dx
+
+	pop %dx
+	pop %bx
+	pop %ax
+	pop %bp
+
+	push %ax
+	mov 2(%bx), %ax
+	pop %ax
+
+	ret
+
+add32:  /* lo, hi, lo, hi */
+	push %bp
+	mov %sp, %bp
+
+	movw 4(%bp), %cx  /* hi */
+	movw 6(%bp), %dx  /* lo */
+
+	add  10(%bp), %dx
+	jnc 1f
+	add $1, %cx
+1:	add 8(%bp), %cx
+
+	pop %bp
+	ret
+
+mul32:  /* lo,      hi,     lo,     hi */
+	/* 10(%bp), 8(%bp), 6(%bp), 4(%bp) */
+	push %bp
+	mov %sp, %bp
+	push %ax
+	push %bx
+
+	xor %cx, %cx
+	xor %dx, %dx
+
+	/* for (i = 0; i < 16;) */
+	xor %bx, %bx
+0:
+	cmp $16, %bx
+	jge 2f
+
+	mov 6(%bp), %ax
+	and $1, %ax
+	cmp $1, %ax
+	jne 1f
+	push 10(%bp)
+	push 8(%bp)
+	push %dx
+	push %cx
+	call add32
+	add $8, %sp
+1:
+	shlw $1, 8(%bp)
+	movw 10(%bp), %ax
+	and $0x8000, %ax
+	cmp $0x8000, %ax
+	jne 1f
+	orw $1, 8(%bp)
+1:
+	shlw $1, 10(%bp)
+	shrw $1, 6(%bp)
+
+	/* i++) { */
+	add $1, %bx
+	jmp 0b
+
+2:
+	pop %bx
+	pop %ax
+	pop %bp
+	ret
+
+disk_reset:
+	movb $0, %ah
+	clc_stack
+	ret
+
+/* this really should be a function, not a macro but i'm lazy */
+.macro read_write_disk_sectors cmd
+	push %ax
+	push %bx
+	push %cx
+	push %dx
+	push %si
+
+	push %bp
+	sub $10, %sp
+	mov %sp, %bp
+
+	/* save nb_sectors */
+	mov %al, 6(%bp)
+	movb $0, 7(%bp)
+
+	/* save buffer */
+	mov %bx, 8(%bp)
+
+	/* cylinders */
+	xor %ax, %ax
+	mov %cl, %al
+	shl $2, %ax
+	and $0x300, %ax
+	mov %ch, %al
+	mov %ax, 0(%bp)
+
+	/* heads */
+	xor %ax, %ax
+	mov %dh, %al
+	mov %ax, 2(%bp)
+
+	/* sectors - 1 */
+	xor %ax, %ax
+	mov %cl, %al
+	and $0x3F, %al
+	sub $1, %ax
+	mov %ax, 4(%bp)
+
+	alloca $16
+
+	movw $0, 0(%bx) /* read c,h,s */
+	push %bx
+	call send_command
+	add $2, %sp
+
+	mov 6(%bx), %ax /* total_sectors */
+	mov 2(%bp), %si /* *= heads */
+	mul %si
+	add 4(%bp), %ax /* += sectors - 1 */
+
+	push 4(%bx) /* total_heads */
+	push $0
+	push 6(%bx) /* total_sectors */
+	push $0
+	call mul32
+	add $8, %sp
+
+	push 0(%bp) /* cylinders */
+	push $0
+	push %dx
+	push %cx
+	call mul32
+	add $8, %sp
+
+	add %ax, %dx
+	jnc 1f
+	add $1, %cx
+1:
+	freea
+
+	alloca $16
+
+	movw \cmd, 0(%bx) /* read */
+	movw 6(%bp), %ax /* nb_sectors */
+	movw %ax, 2(%bx)
+	movw %es, 4(%bx) /* segment */
+	movw 8(%bp), %ax /* offset */
+	mov %ax, 6(%bx)
+	movw %dx, 8(%bx) /* sector */
+	movw %cx, 10(%bx)
+	movw $0, 12(%bx)
+	movw $0, 14(%bx)
+
+	push %bx
+	call send_command
+	add $2, %sp
+
+	freea
+
+	add $10, %sp
+	pop %bp
+
+	pop %si
+	pop %dx
+	pop %cx
+	pop %bx
+	pop %ax
+
+	mov $0, %ah
+	clc_stack
+	ret
+.endm
+
+read_disk_sectors:
+	read_write_disk_sectors $0x01
+
+write_disk_sectors:
+	read_write_disk_sectors $0x02
+
+read_disk_drive_parameters:
+	push %bx
+
+	/* allocate memory for packet, pointer gets returned in bx */
+	alloca $16
+
+	/* issue command */
+	movw $0, 0(%bx) /* cmd = 0, read c,h,s */
+	push %bx
+	call send_command
+	add $2, %sp
+
+	/* normalize sector value */
+	movb 6(%bx), %cl
+	andb $0x3F, %cl
+	movb %cl, 6(%bx)
+
+	/* normalize cylinders */
+	subw $2, 2(%bx)
+
+	/* normalize heads */
+	subw $1, 4(%bx)
+
+	/* return code */
+	mov $0, %ah
+
+	/* cylinders */
+	movb 2(%bx), %ch
+	movb 3(%bx), %cl
+	shlb $6, %cl
+	andb $0xC0, %cl
+
+	/* sectors */
+	orb 6(%bx), %cl
+
+	/* heads */
+	movb 4(%bx), %dh
+
+	/* drives */
+	movb $1, %dl
+
+	/* status */
+	mov $0, %ah
+
+	freea
+
+	pop %bx
+
+	/* do this last since it's the most sensitive */
+	clc_stack
+	ret
+
+alternate_disk_reset:
+	movb $0, %ah
+	clc_stack
+	ret
+
+read_disk_drive_size:
+	push %bx
+	alloca $16
+
+	movw $0, 0(%bx) /* cmd = 0, read c,h,s */
+	push %bx
+	call send_command
+	add $2, %sp
+
+	/* cylinders - 1 to cx:dx */
+	mov 2(%bx), %dx
+	xor %cx, %cx
+	sub $1, %dx
+
+	/* heads */
+	push 4(%bx)
+	push $0
+	push %dx
+	push %cx
+	call mul32
+	add $8, %sp
+
+	/* sectors */
+	push 6(%bx)
+	push $0
+	push %dx
+	push %cx
+	call mul32
+	add $8, %sp
+
+	/* status */
+	mov $3, %ah
+
+	freea
+	pop %bx
+
+	clc_stack
+	ret
+
+check_if_extensions_present:
+	mov $0x30, %ah
+	mov $0xAA55, %bx
+	mov $0x07, %cx
+	clc_stack
+	ret
+
+.macro extended_read_write_sectors cmd
+	cmpb $10, 0(%si)
+	jg 1f
+	mov $1, %ah
+	stc_stack
+	ret
+1:
+	push %ax
+	push %bp
+	allocbpa $16
+
+	movw \cmd, 0(%bp) /* read */
+	movw 2(%si), %ax   /* nb_sectors */
+	movw %ax, 2(%bp)
+	movw 4(%si), %ax   /* offset */
+	movw %ax, 6(%bp)
+	movw 6(%si), %ax   /* segment */
+	movw %ax, 4(%bp)
+	movw 8(%si), %ax   /* block */
+	movw %ax, 8(%bp)
+	movw 10(%si), %ax
+	movw %ax, 10(%bp)
+	movw 12(%si), %ax
+	movw %ax, 12(%bp)
+	movw 14(%si), %ax
+	movw %ax, 14(%bp)
+
+	push %bp
+	call send_command
+	add $2, %sp
+
+	freebpa
+	pop %bp
+	pop %ax
+
+	mov $0, %ah
+	clc_stack
+	ret
+.endm
+
+extended_read_sectors:
+	extended_read_write_sectors $0x01
+
+extended_write_sectors:
+	extended_read_write_sectors $0x02
+
+get_extended_drive_parameters:
+	push %ax
+	push %bp
+	push %cx
+	push %dx
+
+	allocbpa $16
+
+	movw $0, 0(%bp) /* read c,h,s */
+	push %bp
+	call send_command
+	add $2, %sp
+
+	/* write size */
+	movw $26, 0(%si)
+
+	/* set flags to 2 */
+	movw $2, 2(%si)
+
+	/* cylinders */
+	mov 2(%bp), %ax
+	mov %ax, 4(%si)
+	xor %ax, %ax
+	mov %ax, 6(%si)
+
+	/* heads */
+	mov 4(%bp), %ax
+	mov %ax, 8(%si)
+	xor %ax, %ax
+	mov %ax, 10(%si)
+
+	/* sectors */
+	mov 6(%bp), %ax
+	mov %ax, 12(%si)
+	xor %ax, %ax
+	mov %ax, 14(%si)
+
+	/* set total number of sectors */
+	mov 8(%bp), %ax
+	mov %ax, 16(%si)
+	mov 10(%bp), %ax
+	mov %ax, 18(%si)
+	mov 12(%bp), %ax
+	mov %ax, 20(%si)
+	mov 14(%bp), %ax
+	mov %ax, 22(%si)
+
+	/* number of bytes per sector */
+	movw $512, 24(%si)
+
+	freebpa
+
+	pop %dx
+	pop %cx
+	pop %bp
+	pop %ax
+
+	mov $0, %ah
+	clc_stack
+	ret
+
+terminate_disk_emulation:
+	mov $1, %ah
+	stc_stack
+	ret
+
+int13_handler:
+	cmp $0x80, %dl
+	je 1f
+	ljmpw *%cs:old_int13
+1:
+	cmp $0x0, %ah
+	jne 1f
+	call disk_reset
+	iret
+1:
+	cmp $0x2, %ah
+	jne 1f
+	call read_disk_sectors
+	iret
+1:
+	cmp $0x8, %ah
+	jne 1f
+	call read_disk_drive_parameters
+	iret
+1:
+	cmp $0x15, %ah
+	jne 1f
+	call read_disk_drive_size
+	iret
+1:
+	cmp $0x41, %ah
+	jne 1f
+	call check_if_extensions_present
+	iret
+1:
+	cmp $0x42, %ah
+	jne 1f
+	call extended_read_sectors
+	iret
+1:
+	cmp $0x48, %ah
+	jne 1f
+	call get_extended_drive_parameters
+	iret
+1:
+	cmp $0x4b, %ah
+	jne 1f
+	call terminate_disk_emulation
+	iret
+1:
+	cmp $0x0d, %ah
+	jne 1f
+	call alternate_disk_reset
+	iret
+1:
+	cmp $0x03, %ah
+	jne 1f
+	call write_disk_sectors
+	iret
+1:
+	cmp $0x43, %ah
+	jne 1f
+	call extended_write_sectors
+	iret
+1:
+	int $0x18  /* boot failed */
+	iret
+
+/* Variables */
+.align 4, 0
+old_int13:	.long 0
+old_int19:	.long 0
+	
+.align 512, 0
+_end:
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/2] extboot: qemu code.

[Gerd Hoffmann]

booting from scsi + virtio works now.

usage: -device extboot,drive=<name>

where <name> is the name of your boot drive.  You can either use the
names qemu assignes by default (virtio0, scsi0-hd0, ...) or explitly
name your drives via '-drive id=<name>,...' and use that one.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.target |    2 +-
 hw/extboot.c    |  168 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 169 insertions(+), 1 deletions(-)
 create mode 100644 hw/extboot.c

diff --git a/Makefile.target b/Makefile.target
index 7068dc5..b165456 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -197,7 +197,7 @@ obj-i386-y += fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
 obj-i386-y += cirrus_vga.o apic.o ioapic.o parallel.o acpi.o piix_pci.o
 obj-i386-y += usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
-obj-i386-y += ne2000-isa.o
+obj-i386-y += ne2000-isa.o extboot.o
 
 # shared objects
 obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
diff --git a/hw/extboot.c b/hw/extboot.c
new file mode 100644
index 0000000..3465558
--- /dev/null
+++ b/hw/extboot.c
@@ -0,0 +1,168 @@
+/*
+ * Extended boot option ROM support.
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw.h"
+#include "pc.h"
+#include "isa.h"
+#include "block.h"
+#include "loader.h"
+
+typedef struct ExtbootState {
+    ISADevice dev;
+    DriveInfo *dinfo;
+    BlockDriverState *bs;
+} ExtbootState;
+
+union extboot_cmd
+{
+    uint16_t type;
+    struct {
+	uint16_t type;
+	uint16_t cylinders;
+	uint16_t heads;
+	uint16_t sectors;
+	uint64_t nb_sectors;
+    } query_geometry;
+    struct {
+	uint16_t type;
+	uint16_t nb_sectors;
+	uint16_t segment;
+	uint16_t offset;
+	uint64_t sector;
+    } xfer;
+};
+
+static void get_translated_chs(BlockDriverState *bs, int *c, int *h, int *s)
+{
+    bdrv_get_geometry_hint(bs, c, h, s);
+
+    if (*c <= 1024) {
+	*c >>= 0;
+	*h <<= 0;
+    } else if (*c <= 2048) {
+	*c >>= 1;
+	*h <<= 1;
+    } else if (*c <= 4096) {
+	*c >>= 2;
+	*h <<= 2;
+    } else if (*c <= 8192) {
+	*c >>= 3;
+	*h <<= 3;
+    } else {
+	*c >>= 4;
+	*h <<= 4;
+    }
+
+    /* what is the correct algorithm for this?? */
+    if (*h == 256) {
+	*h = 255;
+	*c = *c + 1;
+    }
+}
+
+static uint32_t extboot_read(void *opaque, uint32_t addr)
+{
+    return 1;
+}
+
+static void extboot_write(void *opaque, uint32_t addr, uint32_t value)
+{
+    ExtbootState *s = opaque;
+    union extboot_cmd cmd;
+    int cylinders, heads, sectors, err;
+    uint64_t nb_sectors;
+    target_phys_addr_t pa = 0;
+    int blen = 0;
+    void *buf = NULL;
+
+    cpu_physical_memory_read((value & 0xFFFF) << 4, (uint8_t *)&cmd,
+                             sizeof(cmd));
+
+    if (cmd.type == 0x01 || cmd.type == 0x02) {
+	pa = cmd.xfer.segment * 16 + cmd.xfer.offset;
+        blen = cmd.xfer.nb_sectors * 512;
+        buf = qemu_memalign(512, blen);
+    }
+
+    switch (cmd.type) {
+    case 0x00:
+        get_translated_chs(s->bs, &cylinders, &heads, &sectors);
+	bdrv_get_geometry(s->bs, &nb_sectors);
+	cmd.query_geometry.cylinders = cylinders;
+	cmd.query_geometry.heads = heads;
+	cmd.query_geometry.sectors = sectors;
+	cmd.query_geometry.nb_sectors = nb_sectors;
+	break;
+    case 0x01:
+	err = bdrv_read(s->bs, cmd.xfer.sector, buf, cmd.xfer.nb_sectors);
+	if (err)
+	    printf("Read failed\n");
+
+        cpu_physical_memory_write(pa, buf, blen);
+
+	break;
+    case 0x02:
+        cpu_physical_memory_read(pa, buf, blen);
+
+	err = bdrv_write(s->bs, cmd.xfer.sector, buf, cmd.xfer.nb_sectors);
+	if (err)
+	    printf("Write failed\n");
+
+	break;
+    }
+
+    cpu_physical_memory_write((value & 0xFFFF) << 4, (uint8_t *)&cmd,
+                              sizeof(cmd));
+    if (buf)
+        qemu_free(buf);
+}
+
+static int extboot_init(ISADevice *dev)
+{
+    ExtbootState *s = DO_UPCAST(ExtbootState, dev, dev);
+    int cyls, heads, secs;
+
+    if (!s->dinfo) {
+        fprintf(stderr, "extboot: no drive specified\n");
+        return -1;
+    }
+    if (!s->dinfo->bdrv) {
+        fprintf(stderr, "extboot: drive %s is empty\n", s->dinfo->id);
+        return -1;
+    }
+
+    s->bs = s->dinfo->bdrv;
+    bdrv_guess_geometry(s->bs, &cyls, &heads, &secs);
+    bdrv_set_geometry_hint(s->bs, cyls, heads, secs);
+    register_ioport_read(0x404, 1, 1, extboot_read, s);
+    register_ioport_write(0x405, 1, 2, extboot_write, s);
+    rom_add_option("extboot.bin");
+    return 0;
+}
+
+static ISADeviceInfo extboot_info = {
+    .qdev.name  = "extboot",
+    .qdev.size  = sizeof(ExtbootState),
+    .init       = extboot_init,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_DRIVE("drive", ExtbootState, dinfo),
+        DEFINE_PROP_END_OF_LIST(),
+    },
+};
+
+static void extboot_register(void)
+{
+    isa_qdev_register(&extboot_info);
+};
+
+device_init(extboot_register);
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Anthony Liguori]

Gerd Hoffmann wrote:
>   Hi,
>
> How about this one?  extboot rom and interface are unmodified.
>
> The windup in qemu is very different though.  It is now implemented as
> isa bus device (which is what extboot actually is).  It is properly
> integrated into qdev.  It is much less invasive than the previous
> version.  The patch just drops a hw/extboot.c file into the tree and
> adds it to Makefile.target.  No code changes anywhere else in qemu.
>
> The magic words to activate extboot are '-device extboot,drive=<name>'.
>   

If we were going to introduce a light weight block protocol like this, 
I'd rather it be implemented in SeaBIOS as a block driver.  Then we 
reuse all of the int13 code, it gets registered as a BCV device allowing 
selection in the boot menu, and requires no special options on the 
command line.

It would require a mechanism to do enumeration and identification though.

Regards,

Anthony Liguori

> cheers,
>   Gerd
>
>
>
>   

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Gerd Hoffmann]

On 11/16/09 20:24, Anthony Liguori wrote:
> If we were going to introduce a light weight block protocol like this,
> I'd rather it be implemented in SeaBIOS as a block driver.

Ok.  That would mean to replace the option rom with a seabios patch. 
The qemu code will be unmodified though (except for the rom loading).

> Then we reuse
> all of the int13 code, it gets registered as a BCV device allowing
> selection in the boot menu,

Fine.

> and requires no special options on the
> command line.
>
> It would require a mechanism to do enumeration and identification though.

Huh?  Do you want export *all* block devices via extboot?  Will IDE 
drives show up twice then?

I would export only one and export it only on explicit user request, so 
it can be used for cases where seabios has no native support ...

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Anthony Liguori]

Gerd Hoffmann wrote:
> On 11/16/09 20:24, Anthony Liguori wrote:
>> If we were going to introduce a light weight block protocol like this,
>> I'd rather it be implemented in SeaBIOS as a block driver.
>
> Ok.  That would mean to replace the option rom with a seabios patch. 
> The qemu code will be unmodified though (except for the rom loading).

Yup.

>> and requires no special options on the
>> command line.
>>
>> It would require a mechanism to do enumeration and identification 
>> though.
>
> Huh?  Do you want export *all* block devices via extboot?  Will IDE 
> drives show up twice then?

No, because SeaBIOS already has an ATA driver so we wouldn't want to 
expose IDE on the extboot bus.

> I would export only one and export it only on explicit user request, 
> so it can be used for cases where seabios has no native support ...

If we expose all of the disks, then the user can choose which one they 
want to boot from without having to shut down qemu and modify the 
command line.

It's not obvious to me how we could control the position in the IPL 
table of each disk though.  I guess we could have a priority parameter 
that was part of the disk enumeration that controlled how each device 
was added in the table.  Do you have any thoughts Kevin?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Paul Brook]

> >> It would require a mechanism to do enumeration and identification
> >> though.
> >
> > Huh?  Do you want export *all* block devices via extboot?  Will IDE
> > drives show up twice then?
> 
> No, because SeaBIOS already has an ATA driver so we wouldn't want to
> expose IDE on the extboot bus.

"SeaBIOS already has an ATA driver" doesn't seem a convincing argument for 
making IDE special, because I expect the BIOS will grow support for other 
block devices.

> > I would export only one and export it only on explicit user request,
> > so it can be used for cases where seabios has no native support ...
> 
> If we expose all of the disks, then the user can choose which one they
> want to boot from without having to shut down qemu and modify the
> command line.

Either expose ever block device qemu knows about, or have extboot as a regular 
block device with no special handling. I strongly prefer the latter option.
In fact I'd much prefer to see extboot rewritten to just virtio-block.
Simple virtio clients really aren't that hard to write.

Paul

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Gerd Hoffmann]

On 11/17/09 12:36, Paul Brook wrote:
>>>> It would require a mechanism to do enumeration and identification
>>>> though.
>>>
>>> Huh?  Do you want export *all* block devices via extboot?  Will IDE
>>> drives show up twice then?
>>
>> No, because SeaBIOS already has an ATA driver so we wouldn't want to
>> expose IDE on the extboot bus.
>
> "SeaBIOS already has an ATA driver" doesn't seem a convincing argument for
> making IDE special, because I expect the BIOS will grow support for other
> block devices.

One of the reasons why I'd enable it only on explicit user request, so 
it can be used (temporarily) for cases where the bios has no native support.

> Either expose ever block device qemu knows about, or have extboot as a regular
> block device with no special handling. I strongly prefer the latter option.

Me too.

> In fact I'd much prefer to see extboot rewritten to just virtio-block.

Hmm, I'd prefer something which is *not* used by the guest OS, so it is 
a pure bootloader thing.  When using it to boot from scsi you don't want 
to have the disk show up twice (as virtio and scsi) in the guest.

Some non-pci virtio proxy could do the trick though, i.e. have 
virtio-blk-pci and virtio-blk-boot.  Would probably also simplify things 
on the seabios side as native virtio support code could be reused then.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Paul Brook]

> > In fact I'd much prefer to see extboot rewritten to just virtio-block.
> 
> Hmm, I'd prefer something which is *not* used by the guest OS, so it is
> a pure bootloader thing.  When using it to boot from scsi you don't want
> to have the disk show up twice (as virtio and scsi) in the guest.

You're assuming noone ever writes OS support for extboot...

Paul

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Gerd Hoffmann]

On 11/17/09 13:36, Paul Brook wrote:
>>> In fact I'd much prefer to see extboot rewritten to just virtio-block.
>>
>> Hmm, I'd prefer something which is *not* used by the guest OS, so it is
>> a pure bootloader thing.  When using it to boot from scsi you don't want
>> to have the disk show up twice (as virtio and scsi) in the guest.
>
> You're assuming noone ever writes OS support for extboot...

Which would be almost as silly as writing OS support for bios-int13 ...

cheers
   Gerd

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Paul Brook]

On Tuesday 17 November 2009, Gerd Hoffmann wrote:
> On 11/17/09 13:36, Paul Brook wrote:
> >>> In fact I'd much prefer to see extboot rewritten to just virtio-block.
> >>
> >> Hmm, I'd prefer something which is *not* used by the guest OS, so it is
> >> a pure bootloader thing.  When using it to boot from scsi you don't want
> >> to have the disk show up twice (as virtio and scsi) in the guest.
> >
> > You're assuming noone ever writes OS support for extboot...
> 
> Which would be almost as silly as writing OS support for bios-int13 ...

Not entirely. int13 is a software interface, extboot is a hardware interface.
Look at it the other way round: If I already have my low performance boot 
device exposed via extboot (on an otherwise diskless client), why should I 
have to also expose it via virtio-blk just so that the guest can access it for 
installing kernel upgrades.

Paul

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Alexander Graf]

Paul Brook wrote:
> On Tuesday 17 November 2009, Gerd Hoffmann wrote:
>   
>> On 11/17/09 13:36, Paul Brook wrote:
>>     
>>>>> In fact I'd much prefer to see extboot rewritten to just virtio-block.
>>>>>           
>>>> Hmm, I'd prefer something which is *not* used by the guest OS, so it is
>>>> a pure bootloader thing.  When using it to boot from scsi you don't want
>>>> to have the disk show up twice (as virtio and scsi) in the guest.
>>>>         
>>> You're assuming noone ever writes OS support for extboot...
>>>       
>> Which would be almost as silly as writing OS support for bios-int13 ...
>>     
>
> Not entirely. int13 is a software interface, extboot is a hardware interface.
> Look at it the other way round: If I already have my low performance boot 
> device exposed via extboot (on an otherwise diskless client), why should I 
> have to also expose it via virtio-blk just so that the guest can access it for 
> installing kernel upgrades.
>   

Because that's not what you'd use it for. That's what -kernel and
-initrd are there for.

IMHO having a BIOS backdoor is a good thing in general. If anyone wants
to destroy their user experience by writing a driver for that in their
OS, I'm good with that, but let's not expose things twice _to users_ as
the default case.


Alex

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Anthony Liguori]

Alexander Graf wrote:
>
> Because that's not what you'd use it for. That's what -kernel and
> -initrd are there for.
>
> IMHO having a BIOS backdoor is a good thing in general. If anyone wants
> to destroy their user experience by writing a driver for that in their
> OS, I'm good with that, but let's not expose things twice _to users_ as
> the default case.
>   

N.B.  regardless of whether we have an alternative, simple block 
interface or we have proper drivers in the BIOS, we still have the same 
problem to solve.  We need a way to communicate the desired boot 
priority of each block device.  This becomes quite a bit more 
complicated if we're doing proper drivers in BIOS.

Regards,

Anthony Liguori
> Alex
>
>
>   

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Kevin O'Connor]

On Mon, Nov 16, 2009 at 03:14:33PM -0600, Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>> Huh?  Do you want export *all* block devices via extboot?  Will IDE  
>> drives show up twice then?
>
> No, because SeaBIOS already has an ATA driver so we wouldn't want to  
> expose IDE on the extboot bus.

One possibility would be for QEMU to tell the bios the type of each
"extboot" drive - that way seabios could ignore drives it already had
native drivers for.

>> I would export only one and export it only on explicit user request,  
>> so it can be used for cases where seabios has no native support ...
>
> If we expose all of the disks, then the user can choose which one they  
> want to boot from without having to shut down qemu and modify the  
> command line.
>
> It's not obvious to me how we could control the position in the IPL  
> table of each disk though.  I guess we could have a priority parameter  
> that was part of the disk enumeration that controlled how each device  
> was added in the table.  Do you have any thoughts Kevin?

Adding to the BCV list is done via the add_bcv_internal() call in
src/boot.c.  The list order is determined by the type of drive and its
internal id - usually drives are arranged in the list in the order
they are found.

The order of hard drives is determined by the BCV list.  There is only
one entry in the IPL table for booting from a hard drive - selecting
"-boot c" prioritizes this IPL - hard drive booting always boots from
the "C" drive.  It's possible to tell SeaBIOS to map the C drive to
something other than the first item on the BCV list, but that
functionality is currently only available via the boot menu.

As an aside, I'm open to adding both an "extboot" driver and
additional native drivers to SeaBIOS.  Both have their uses.

-Kevin

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Anthony Liguori]

Kevin O'Connor wrote:
> The order of hard drives is determined by the BCV list.  There is only
> one entry in the IPL table for booting from a hard drive - selecting
> "-boot c" prioritizes this IPL - hard drive booting always boots from
> the "C" drive.  It's possible to tell SeaBIOS to map the C drive to
> something other than the first item on the BCV list, but that
> functionality is currently only available via the boot menu.
>   

This is the bit that confused me.  Does an IPL entry for the hard disk 
result in each BCV device being booted in order of BCV priority or is 
there a means to have multiple IPL entries?  IOW, how would we support a 
boot order of first hard disk, second hard disk, then CD-ROM?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Kevin O'Connor]

On Wed, Nov 18, 2009 at 08:42:10AM -0600, Anthony Liguori wrote:
> Kevin O'Connor wrote:
>> The order of hard drives is determined by the BCV list.  There is only
>> one entry in the IPL table for booting from a hard drive - selecting
>> "-boot c" prioritizes this IPL - hard drive booting always boots from
>> the "C" drive.  It's possible to tell SeaBIOS to map the C drive to
>> something other than the first item on the BCV list, but that
>> functionality is currently only available via the boot menu.
>
> This is the bit that confused me.

The IPL and BCV stuff is confusing.

An IPL is a way to boot the machine (floppy, hard drive, cdrom, BEVs
(eg, network card)).  A BCV is a hard drive.

So, there is only one IPL entry for booting from a hard drive, though
one may have many hard drives.

>Does an IPL entry for the hard disk  
> result in each BCV device being booted in order of BCV priority or is  
> there a means to have multiple IPL entries?  IOW, how would we support a  
> boot order of first hard disk, second hard disk, then CD-ROM?

With full compatibility, it's not possible to have a boot order of
first hard drive, then second hard drive.

The only compatible way to boot from a hard drive is to map that hard
drive to drive "C".  Once a drive is mapped, the mapping can't be
changed.  (Theoretically, seabios, could alter the mappings of
internally handled drives, but there is no way to remap a hard drive
added by an option rom.)

The BCV list is looked at in the last stage of POST - each drive is
mapped at that point.  The first drive on the list becomes the "C"
drive (unless one asks SeaBIOS to move a drive up by selecting it in
the boot menu - in which case that drive becomes drive "C").  Each
successive entry in the list becomes "D", "E", etc.

The IPL list is looked at during boot (after POST) and seabios
attempts each method in the list.  The "-boot" parameter instructs
seabios what order of IPL entries to try during boot.

As for adding two IPL entries for hard drives - it wouldn't be useful
because one can only boot from the "C" drive.

Hope this helps.
-Kevin

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Jamie Lokier]

Gerd Hoffmann wrote:
> On 11/16/09 20:24, Anthony Liguori wrote:
> >If we were going to introduce a light weight block protocol like this,
> >I'd rather it be implemented in SeaBIOS as a block driver.
> 
> Ok.  That would mean to replace the option rom with a seabios patch. 
> The qemu code will be unmodified though (except for the rom loading).

Can't extboot be used with other BIOSes, though, being an option ROM?

-- Jamie

Re: [Qemu-devel] [PATCH 0/2] extboot reloaded.

[Anthony Liguori]

Jamie Lokier wrote:
> Gerd Hoffmann wrote:
>   
>> On 11/16/09 20:24, Anthony Liguori wrote:
>>     
>>> If we were going to introduce a light weight block protocol like this,
>>> I'd rather it be implemented in SeaBIOS as a block driver.
>>>       
>> Ok.  That would mean to replace the option rom with a seabios patch. 
>> The qemu code will be unmodified though (except for the rom loading).
>>     
>
> Can't extboot be used with other BIOSes, though, being an option ROM?
>   

Except the "bus" that extboot requires doesn't exist anywhere but in qemu.

> -- Jamie
>   

Regards,

Anthony Liguori