[RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Pekka Enberg]

This patch adds a "--bios" command line option to "vm run". You can use this to
try to boot with SeaBIOS, for example:

  ./vm run --bios=/usr/share/seabios/bios.bin \
	 --disk $HOME/images/debian_lenny_amd64_standard.qcow2

This doesn't boot yet for obvious reasons but at least people can now start to
play with external BIOS images easily.

Cc: Yang Bai <hamo.by@gmail.com>
Cc: Matt Evans <matt@ozlabs.org>
Cc: Ron Minnich <rminnich@gmail.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: John Floren <john@jfloren.net>
Cc: Sasha Levin <levinsasha928@gmail.com>
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Asias He <asias.hejun@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
---
 tools/kvm/Makefile               |    2 +
 tools/kvm/builtin-run.c          |   32 +++++++++++++++++++----------
 tools/kvm/include/kvm/kvm.h      |    1 +
 tools/kvm/powerpc/boot.c         |    8 +++++++
 tools/kvm/x86/boot.c             |   41 ++++++++++++++++++++++++++++++++++++++
 tools/kvm/x86/include/kvm/bios.h |    3 +-
 6 files changed, 75 insertions(+), 12 deletions(-)
 create mode 100644 tools/kvm/powerpc/boot.c
 create mode 100644 tools/kvm/x86/boot.c

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index cfa5547..0a9c2cc 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -113,6 +113,7 @@ LIBFDT_OBJS = $(patsubst %,../../scripts/dtc/libfdt/%,$(LIBFDT_SRC))
 #x86
 ifeq ($(ARCH),x86)
 	DEFINES += -DCONFIG_X86
+	OBJS	+= x86/boot.o
 	OBJS	+= x86/cpuid.o
 	OBJS	+= x86/interrupt.o
 	OBJS	+= x86/ioport.o
@@ -129,6 +130,7 @@ endif
 # POWER/ppc:  Actually only support ppc64 currently.
 ifeq ($(uname_M), ppc64)
 	DEFINES += -DCONFIG_PPC
+	OBJS	+= powerpc/boot.o
 	OBJS	+= powerpc/ioport.o
 	OBJS	+= powerpc/irq.o
 	OBJS	+= powerpc/kvm.o
diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 466169e..f96c581 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -76,6 +76,7 @@ static const char *kernel_cmdline;
 static const char *kernel_filename;
 static const char *vmlinux_filename;
 static const char *initrd_filename;
+static const char *bios_filename;
 static const char *image_filename[MAX_DISK_IMAGES];
 static const char *console;
 static const char *dev;
@@ -458,6 +459,8 @@ static const struct option options[] = {
 			"Initial RAM disk image"),
 	OPT_STRING('p', "params", &kernel_cmdline, "params",
 			"Kernel command line arguments"),
+	OPT_STRING('b', "bios", &bios_filename, "bios",
+			"BIOS to boot in virtual machine"),
 
 	OPT_GROUP("Networking options:"),
 	OPT_CALLBACK_DEFAULT('n', "network", NULL, "network params",
@@ -1110,14 +1113,16 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 	printf("  # %s run -k %s -m %Lu -c %d --name %s\n", KVM_BINARY_NAME,
 		kernel_filename, ram_size / 1024 / 1024, nrcpus, guest_name);
 
-	if (!kvm__load_kernel(kvm, kernel_filename, initrd_filename,
-				real_cmdline, vidmode))
-		die("unable to load kernel %s", kernel_filename);
+	if (!bios_filename) {
+		if (!kvm__load_kernel(kvm, kernel_filename,
+				initrd_filename, real_cmdline, vidmode))
+			die("unable to load kernel %s", kernel_filename);
 
-	kvm->vmlinux = vmlinux_filename;
-	r = symbol_init(kvm);
-	if (r < 0)
-		pr_debug("symbol_init() failed with error %d\n", r);
+		kvm->vmlinux = vmlinux_filename;
+		r = symbol_init(kvm);
+		if (r < 0)
+			pr_debug("symbol_init() failed with error %d\n", r);
+	}
 
 	ioport__setup_arch();
 
@@ -1218,10 +1223,15 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 
 	kvm__start_timer(kvm);
 
-	kvm__arch_setup_firmware(kvm);
-	if (r < 0) {
-		pr_err("kvm__arch_setup_firmware() failed with error %d\n", r);
-		goto fail;
+	if (bios_filename) {
+		if (!kvm__load_bios(kvm, bios_filename))
+			die("unable to load bios %s: %s", bios_filename, strerror(errno));
+	} else {
+		kvm__arch_setup_firmware(kvm);
+		if (r < 0) {
+			pr_err("kvm__arch_setup_firmware() failed with error %d\n", r);
+			goto fail;
+		}
 	}
 
 	for (i = 0; i < nrcpus; i++) {
diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index 7870118..258d11a 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -39,6 +39,7 @@ int kvm__recommended_cpus(struct kvm *kvm);
 int kvm__max_cpus(struct kvm *kvm);
 void kvm__init_ram(struct kvm *kvm);
 int kvm__exit(struct kvm *kvm);
+bool kvm__load_bios(struct kvm *kvm, const char *bios_filename);
 bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
 			const char *initrd_filename, const char *kernel_cmdline, u16 vidmode);
 void kvm__start_timer(struct kvm *kvm);
diff --git a/tools/kvm/powerpc/boot.c b/tools/kvm/powerpc/boot.c
new file mode 100644
index 0000000..8c99831
--- /dev/null
+++ b/tools/kvm/powerpc/boot.c
@@ -0,0 +1,8 @@
+#include "kvm/kvm.h"
+
+#include <stdbool.h>
+
+bool kvm__load_bios(struct kvm *kvm, const char *bios_filename)
+{
+	return false;
+}
diff --git a/tools/kvm/x86/boot.c b/tools/kvm/x86/boot.c
new file mode 100644
index 0000000..383d9f7
--- /dev/null
+++ b/tools/kvm/x86/boot.c
@@ -0,0 +1,41 @@
+#include "kvm/kvm.h"
+
+#include "kvm/util.h"
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <stdbool.h>
+#include <fcntl.h>
+
+#define BIOS_SELECTOR	0xf000
+#define BIOS_IP		0xfff0
+#define BIOS_SP		0x8000
+
+bool kvm__load_bios(struct kvm *kvm, const char *bios_filename)
+{
+	struct stat st;
+	void *p;
+	int fd;
+	int nr;
+
+	fd = open(bios_filename, O_RDONLY);
+	if (fd < 0)
+		return false;
+
+	if (fstat(fd, &st))
+		return false;
+
+	if (st.st_size > MB_BIOS_SIZE)
+		die("BIOS image %s is too big to fit in memory (%lu KB).\n", bios_filename, st.st_size / 1024);
+
+	p = guest_flat_to_host(kvm, MB_BIOS_BEGIN);
+
+	while ((nr = read(fd, p, st.st_size)) > 0)
+		p += nr;
+
+	kvm->boot_selector	= BIOS_SELECTOR;
+	kvm->boot_ip		= BIOS_IP;
+	kvm->boot_sp		= BIOS_SP;
+
+	return true;
+}
diff --git a/tools/kvm/x86/include/kvm/bios.h b/tools/kvm/x86/include/kvm/bios.h
index de569bc..9d677ae 100644
--- a/tools/kvm/x86/include/kvm/bios.h
+++ b/tools/kvm/x86/include/kvm/bios.h
@@ -26,8 +26,9 @@
 
 #define E820_MAP_START			EBDA_START
 
-#define MB_BIOS_BEGIN			0x000f0000
+#define MB_BIOS_BEGIN			0x000e0000
 #define MB_BIOS_END			0x000fffff
+#define MB_BIOS_SIZE			(MB_BIOS_END - MB_BIOS_BEGIN + 1)
 
 #define VGA_RAM_BEGIN			0x000a0000
 #define VGA_RAM_END			0x000bffff
-- 
1.7.6.5

[RFC/PATCH 2/2] kvm tools, seabios: Add support for SeaBIOS debugging output

[Pekka Enberg]

SeaBIOS outputs debugging messages to special debug port ("0x0402"). This patch
unconditionally hooks the PIO port to LKVM so that when user specifies a
SeaBIOS image, you'll see this for "vm run":

  [penberg@tux kvm]$ ./vm run --bios /usr/share/seabios/bios.bin
    # lkvm run -k ../../arch/x86/boot/bzImage -m 448 -c 4 --name guest-25983
  Start bios (version 0.6.0)
  Unable to unlock ram - bridge not found
  Ram Size=0x00100000 (0x0000000000000000 high)

  <hangs here>

Once we have SeaBIOS fully operational, we probably should hide the feature
under "--debug-seabios" command line option.

Cc: Yang Bai <hamo.by@gmail.com>
Cc: Matt Evans <matt@ozlabs.org>
Cc: Ron Minnich <rminnich@gmail.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: John Floren <john@jfloren.net>
Cc: Sasha Levin <levinsasha928@gmail.com>
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Asias He <asias.hejun@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
---
 tools/kvm/x86/ioport.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/x86/ioport.c b/tools/kvm/x86/ioport.c
index 8a91bf2..86302e6 100644
--- a/tools/kvm/x86/ioport.c
+++ b/tools/kvm/x86/ioport.c
@@ -1,6 +1,7 @@
 #include "kvm/ioport.h"
 
 #include <stdlib.h>
+#include <stdio.h>
 
 static bool debug_io_out(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size)
 {
@@ -11,6 +12,21 @@ static struct ioport_operations debug_ops = {
 	.io_out		= debug_io_out,
 };
 
+static bool seabios_debug_io_out(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size)
+{
+	char ch;
+
+	ch = ioport__read8(data);
+
+	putchar(ch);
+
+	return true;
+}
+
+static struct ioport_operations seabios_debug_ops = {
+	.io_out		= seabios_debug_io_out,
+};
+
 static bool dummy_io_in(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size)
 {
 	return true;
@@ -56,4 +72,6 @@ void ioport__setup_arch(void)
 	/* PORT 03D4-03D5 - COLOR VIDEO - CRT CONTROL REGISTERS */
 	ioport__register(0x03D4, &dummy_read_write_ioport_ops, 1, NULL);
 	ioport__register(0x03D5, &dummy_write_only_ioport_ops, 1, NULL);
+
+	ioport__register(0x402, &seabios_debug_ops, 1, NULL);
 }
-- 
1.7.6.5

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Cyrill Gorcunov]

On Fri, Feb 24, 2012 at 05:05:29PM +0200, Pekka Enberg wrote:
> This patch adds a "--bios" command line option to "vm run". You can use this to
> try to boot with SeaBIOS, for example:
> 
>   ./vm run --bios=/usr/share/seabios/bios.bin \
> 	 --disk $HOME/images/debian_lenny_amd64_standard.qcow2
> 
> This doesn't boot yet for obvious reasons but at least people can now start to
> play with external BIOS images easily.

Hi Pekka, I believe it worth merging to start
working on this feature.

	Cyrill

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Anthony Liguori]

On 02/24/2012 09:05 AM, Pekka Enberg wrote:
> This patch adds a "--bios" command line option to "vm run". You can use this to
> try to boot with SeaBIOS, for example:
>
>    ./vm run --bios=/usr/share/seabios/bios.bin \
> 	 --disk $HOME/images/debian_lenny_amd64_standard.qcow2
>
> This doesn't boot yet for obvious reasons but at least people can now start to
> play with external BIOS images easily.

You may want to call it firmware as other platforms also have firmware.  For 
instance, it may be desirable to use the same interface to load SLOF with spapr.

>
> Cc: Yang Bai<hamo.by@gmail.com>
> Cc: Matt Evans<matt@ozlabs.org>
> Cc: Ron Minnich<rminnich@gmail.com>
> Cc: Anthony Liguori<aliguori@us.ibm.com>
> Cc: John Floren<john@jfloren.net>
> Cc: Sasha Levin<levinsasha928@gmail.com>
> Cc: Cyrill Gorcunov<gorcunov@openvz.org>
> Cc: Asias He<asias.hejun@gmail.com>
> Cc: Ingo Molnar<mingo@elte.hu>
> Signed-off-by: Pekka Enberg<penberg@kernel.org>
> ---
>   tools/kvm/Makefile               |    2 +
>   tools/kvm/builtin-run.c          |   32 +++++++++++++++++++----------
>   tools/kvm/include/kvm/kvm.h      |    1 +
>   tools/kvm/powerpc/boot.c         |    8 +++++++
>   tools/kvm/x86/boot.c             |   41 ++++++++++++++++++++++++++++++++++++++
>   tools/kvm/x86/include/kvm/bios.h |    3 +-
>   6 files changed, 75 insertions(+), 12 deletions(-)
>   create mode 100644 tools/kvm/powerpc/boot.c
>   create mode 100644 tools/kvm/x86/boot.c
>
> diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
> index cfa5547..0a9c2cc 100644
> --- a/tools/kvm/Makefile
> +++ b/tools/kvm/Makefile
> @@ -113,6 +113,7 @@ LIBFDT_OBJS = $(patsubst %,../../scripts/dtc/libfdt/%,$(LIBFDT_SRC))
>   #x86
>   ifeq ($(ARCH),x86)
>   	DEFINES += -DCONFIG_X86
> +	OBJS	+= x86/boot.o
>   	OBJS	+= x86/cpuid.o
>   	OBJS	+= x86/interrupt.o
>   	OBJS	+= x86/ioport.o
> @@ -129,6 +130,7 @@ endif
>   # POWER/ppc:  Actually only support ppc64 currently.
>   ifeq ($(uname_M), ppc64)
>   	DEFINES += -DCONFIG_PPC
> +	OBJS	+= powerpc/boot.o
>   	OBJS	+= powerpc/ioport.o
>   	OBJS	+= powerpc/irq.o
>   	OBJS	+= powerpc/kvm.o
> diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> index 466169e..f96c581 100644
> --- a/tools/kvm/builtin-run.c
> +++ b/tools/kvm/builtin-run.c
> @@ -76,6 +76,7 @@ static const char *kernel_cmdline;
>   static const char *kernel_filename;
>   static const char *vmlinux_filename;
>   static const char *initrd_filename;
> +static const char *bios_filename;
>   static const char *image_filename[MAX_DISK_IMAGES];
>   static const char *console;
>   static const char *dev;
> @@ -458,6 +459,8 @@ static const struct option options[] = {
>   			"Initial RAM disk image"),
>   	OPT_STRING('p', "params",&kernel_cmdline, "params",
>   			"Kernel command line arguments"),
> +	OPT_STRING('b', "bios",&bios_filename, "bios",
> +			"BIOS to boot in virtual machine"),
>
>   	OPT_GROUP("Networking options:"),
>   	OPT_CALLBACK_DEFAULT('n', "network", NULL, "network params",
> @@ -1110,14 +1113,16 @@ static int kvm_cmd_run_init(int argc, const char **argv)
>   	printf("  # %s run -k %s -m %Lu -c %d --name %s\n", KVM_BINARY_NAME,
>   		kernel_filename, ram_size / 1024 / 1024, nrcpus, guest_name);
>
> -	if (!kvm__load_kernel(kvm, kernel_filename, initrd_filename,
> -				real_cmdline, vidmode))
> -		die("unable to load kernel %s", kernel_filename);
> +	if (!bios_filename) {
> +		if (!kvm__load_kernel(kvm, kernel_filename,
> +				initrd_filename, real_cmdline, vidmode))
> +			die("unable to load kernel %s", kernel_filename);
>
> -	kvm->vmlinux = vmlinux_filename;
> -	r = symbol_init(kvm);
> -	if (r<  0)
> -		pr_debug("symbol_init() failed with error %d\n", r);
> +		kvm->vmlinux = vmlinux_filename;
> +		r = symbol_init(kvm);
> +		if (r<  0)
> +			pr_debug("symbol_init() failed with error %d\n", r);
> +	}
>
>   	ioport__setup_arch();
>
> @@ -1218,10 +1223,15 @@ static int kvm_cmd_run_init(int argc, const char **argv)
>
>   	kvm__start_timer(kvm);
>
> -	kvm__arch_setup_firmware(kvm);
> -	if (r<  0) {
> -		pr_err("kvm__arch_setup_firmware() failed with error %d\n", r);
> -		goto fail;
> +	if (bios_filename) {
> +		if (!kvm__load_bios(kvm, bios_filename))
> +			die("unable to load bios %s: %s", bios_filename, strerror(errno));
> +	} else {
> +		kvm__arch_setup_firmware(kvm);
> +		if (r<  0) {
> +			pr_err("kvm__arch_setup_firmware() failed with error %d\n", r);
> +			goto fail;
> +		}
>   	}
>
>   	for (i = 0; i<  nrcpus; i++) {
> diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
> index 7870118..258d11a 100644
> --- a/tools/kvm/include/kvm/kvm.h
> +++ b/tools/kvm/include/kvm/kvm.h
> @@ -39,6 +39,7 @@ int kvm__recommended_cpus(struct kvm *kvm);
>   int kvm__max_cpus(struct kvm *kvm);
>   void kvm__init_ram(struct kvm *kvm);
>   int kvm__exit(struct kvm *kvm);
> +bool kvm__load_bios(struct kvm *kvm, const char *bios_filename);
>   bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
>   			const char *initrd_filename, const char *kernel_cmdline, u16 vidmode);
>   void kvm__start_timer(struct kvm *kvm);
> diff --git a/tools/kvm/powerpc/boot.c b/tools/kvm/powerpc/boot.c
> new file mode 100644
> index 0000000..8c99831
> --- /dev/null
> +++ b/tools/kvm/powerpc/boot.c
> @@ -0,0 +1,8 @@
> +#include "kvm/kvm.h"
> +
> +#include<stdbool.h>
> +
> +bool kvm__load_bios(struct kvm *kvm, const char *bios_filename)
> +{
> +	return false;
> +}
> diff --git a/tools/kvm/x86/boot.c b/tools/kvm/x86/boot.c
> new file mode 100644
> index 0000000..383d9f7
> --- /dev/null
> +++ b/tools/kvm/x86/boot.c
> @@ -0,0 +1,41 @@
> +#include "kvm/kvm.h"
> +
> +#include "kvm/util.h"
> +
> +#include<sys/types.h>
> +#include<sys/stat.h>
> +#include<stdbool.h>
> +#include<fcntl.h>
> +
> +#define BIOS_SELECTOR	0xf000
> +#define BIOS_IP		0xfff0
> +#define BIOS_SP		0x8000
> +
> +bool kvm__load_bios(struct kvm *kvm, const char *bios_filename)
> +{
> +	struct stat st;
> +	void *p;
> +	int fd;
> +	int nr;
> +
> +	fd = open(bios_filename, O_RDONLY);
> +	if (fd<  0)
> +		return false;
> +
> +	if (fstat(fd,&st))
> +		return false;
> +
> +	if (st.st_size>  MB_BIOS_SIZE)
> +		die("BIOS image %s is too big to fit in memory (%lu KB).\n", bios_filename, st.st_size / 1024);
> +
> +	p = guest_flat_to_host(kvm, MB_BIOS_BEGIN);
> +
> +	while ((nr = read(fd, p, st.st_size))>  0)
> +		p += nr;
> +
> +	kvm->boot_selector	= BIOS_SELECTOR;
> +	kvm->boot_ip		= BIOS_IP;
> +	kvm->boot_sp		= BIOS_SP;
> +
> +	return true;
> +}
> diff --git a/tools/kvm/x86/include/kvm/bios.h b/tools/kvm/x86/include/kvm/bios.h
> index de569bc..9d677ae 100644
> --- a/tools/kvm/x86/include/kvm/bios.h
> +++ b/tools/kvm/x86/include/kvm/bios.h
> @@ -26,8 +26,9 @@
>
>   #define E820_MAP_START			EBDA_START
>
> -#define MB_BIOS_BEGIN			0x000f0000
> +#define MB_BIOS_BEGIN			0x000e0000
>   #define MB_BIOS_END			0x000fffff
> +#define MB_BIOS_SIZE			(MB_BIOS_END - MB_BIOS_BEGIN + 1)
>
>   #define VGA_RAM_BEGIN			0x000a0000
>   #define VGA_RAM_END			0x000bffff

FYI, this will work with SeaBIOS because it's currently 128k.  But the normal 
thing is to load the last 128k of the BIOS ROM at 1MB - 128k, with the full ROM 
mapped at 4GB - sizeof(ROM).

SeaBIOS certainly expects the ROM to be located in both places.

Also note that the area at 1MB - 128k is special.  There's two distinct regions 
mapped here, normal RAM and then the BIOS ROM.  Depending on the settings of the 
PAM registers, a guest can chose to see either piece of memory.  In fact, it can 
control read and write access independently such that writes go to RAM and reads 
come from the BIOS area.

This is used to implement shadowing.  SeaBIOS does special case this under KVM 
since it's not possible to execute out of MMIO under KVM.

Regards,

Anthony Liguori

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[ron minnich]

I think you need to look at the change floren and I worked up,
assuming I can get him to release it to you.

Our change is a bit less complex and adds fewer lines of code. You've
added an extra option to load a bios, and it will be somewhat limited
as those ever-demanding users continue to demand more and more :-)

What we did instead is let users specify the load address of the
kernel, and the IP from which to start. So our command line ends up
with --kernel being a seabios image, and using switches to set the
load address at e0000 and the initial IP at ffff0. Of course, now that
I know the memory is "reflected" to high memory too, the initial IP
could just as easily be fffffff0.

Anyway, John, can you give us a look at a patch please?

ron

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[John Floren]

Yeah, I'll try to work something up... got approval from Keith.

On Fri, Feb 24, 2012 at 8:27 AM, ron minnich <rminnich@gmail.com> wrote:
> I think you need to look at the change floren and I worked up,
> assuming I can get him to release it to you.
>
> Our change is a bit less complex and adds fewer lines of code. You've
> added an extra option to load a bios, and it will be somewhat limited
> as those ever-demanding users continue to demand more and more :-)
>
> What we did instead is let users specify the load address of the
> kernel, and the IP from which to start. So our command line ends up
> with --kernel being a seabios image, and using switches to set the
> load address at e0000 and the initial IP at ffff0. Of course, now that
> I know the memory is "reflected" to high memory too, the initial IP
> could just as easily be fffffff0.
>
> Anyway, John, can you give us a look at a patch please?
>
> ron

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Pekka Enberg]

Hi,

I played around with the "--debug-ioport" command line option and was able 
to cheat my way past SeaBIOS POST phase. Should SeaBIOS automatically pick 
up virtio devices and attempt to boot them?

And no, I'm not going to commit this, at least not for now.

			Pekka

>From 8070dd3cfd3a371f4d968fd2446514f03df1f149 Mon Sep 17 00:00:00 2001
From: Pekka Enberg <penberg@kernel.org>
Date: Fri, 24 Feb 2012 20:20:16 +0200
Subject: [PATCH] kvmtool, seabios: Ignore BIOS POST legacy registers

This makes SeaBIOS limp along all the way to boot menu:

  [penberg@tux kvm]$ ./vm run --firmware /usr/share/seabios/bios.bin
    # lkvm run -k ../../arch/x86/boot/bzImage -m 448 -c 4 --name guest-11658
  Start bios (version 0.6.0)
  Unable to unlock ram - bridge not found
  Ram Size=0x00100000 (0x0000000000000000 high)
  CPU Mhz=13
  Found 1 cpu(s) max supported 1 cpu(s)
  PCI: bus=0 devfn=0x08: vendor_id=0x1af4 device_id=0x1009
  PCI: bus=0 devfn=0x10: vendor_id=0x1af4 device_id=0x1009
  PCI: bus=0 devfn=0x18: vendor_id=0x1af4 device_id=0x1000
  region 0: 0x00000000
  region 1: 0x00000000
  WARNING - Unable to allocate resource at mptable_init:26!
  WARNING - Unable to allocate resource at smbios_init:381!
  Scan for VGA option rom
  WARNING - Timeout at i8042_wait_read:35!
  Found 2 lpt ports
  Found 4 serial ports
  Scan for option roms
  Press F12 for boot menu.

Signed-off-by: Pekka Enberg <penberg@kernel.org>
---
 tools/kvm/hw/i8042.c   |    8 ++++++++
 tools/kvm/x86/ioport.c |   18 ++++++++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/hw/i8042.c b/tools/kvm/hw/i8042.c
index 3a36425..542fffd 100644
--- a/tools/kvm/hw/i8042.c
+++ b/tools/kvm/hw/i8042.c
@@ -19,6 +19,7 @@
  * Registers
  */
 #define I8042_DATA_REG		0x60
+#define I8042_PORT_B_REG	0x61
 #define I8042_COMMAND_REG	0x64
 
 /*
@@ -307,6 +308,10 @@ static bool kbd_in(struct ioport *ioport, struct kvm *kvm, u16 port, void *data,
 		ioport__write32(data, value);
 		break;
 	}
+	case I8042_PORT_B_REG: {
+		ioport__write8(data, 0x20);
+		break;
+	}
 	default:
 		return false;
 	}
@@ -327,6 +332,9 @@ static bool kbd_out(struct ioport *ioport, struct kvm *kvm, u16 port, void *data
 		kbd_write_data(value);
 		break;
 	}
+	case I8042_PORT_B_REG: {
+		break;
+	}
 	default:
 		return false;
 	}
diff --git a/tools/kvm/x86/ioport.c b/tools/kvm/x86/ioport.c
index 86302e6..7b3f4e3 100644
--- a/tools/kvm/x86/ioport.c
+++ b/tools/kvm/x86/ioport.c
@@ -50,15 +50,24 @@ void ioport__setup_arch(void)
 {
 	/* Legacy ioport setup */
 
+	/* 0000 - 001F - DMA1 controller */
+	ioport__register(0x0000, &dummy_read_write_ioport_ops, 32, NULL);
+
 	/* 0x0020 - 0x003F - 8259A PIC 1 */
 	ioport__register(0x0020, &dummy_read_write_ioport_ops, 2, NULL);
 
 	/* PORT 0040-005F - PIT - PROGRAMMABLE INTERVAL TIMER (8253, 8254) */
 	ioport__register(0x0040, &dummy_read_write_ioport_ops, 4, NULL);
 
+	/* 0092 - PS/2 system control port A */
+	ioport__register(0x0092, &dummy_read_write_ioport_ops, 1, NULL);
+
 	/* 0x00A0 - 0x00AF - 8259A PIC 2 */
 	ioport__register(0x00A0, &dummy_read_write_ioport_ops, 2, NULL);
 
+	/* 00C0 - 001F - DMA2 controller */
+	ioport__register(0x00C0, &dummy_read_write_ioport_ops, 32, NULL);
+
 	/* PORT 00E0-00EF are 'motherboard specific' so we use them for our
 	   internal debugging purposes.  */
 	ioport__register(IOPORT_DBG, &debug_ops, 1, NULL);
@@ -69,9 +78,18 @@ void ioport__setup_arch(void)
 	/* 0x00F0 - 0x00FF - Math co-processor */
 	ioport__register(0x00F0, &dummy_write_only_ioport_ops, 2, NULL);
 
+	/* PORT 0278-027A - PARALLEL PRINTER PORT (usually LPT1, sometimes LPT2) */
+	ioport__register(0x0278, &dummy_read_write_ioport_ops, 3, NULL);
+
+	/* PORT 0378-037A - PARALLEL PRINTER PORT (usually LPT2, sometimes LPT3) */
+	ioport__register(0x0378, &dummy_read_write_ioport_ops, 3, NULL);
+
 	/* PORT 03D4-03D5 - COLOR VIDEO - CRT CONTROL REGISTERS */
 	ioport__register(0x03D4, &dummy_read_write_ioport_ops, 1, NULL);
 	ioport__register(0x03D5, &dummy_write_only_ioport_ops, 1, NULL);
 
 	ioport__register(0x402, &seabios_debug_ops, 1, NULL);
+
+	/* 0510 - QEMU BIOS configuration register */
+	ioport__register(0x510, &dummy_read_write_ioport_ops, 2, NULL);
 }
-- 
1.7.6.5

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Pekka Enberg]

On Fri, Feb 24, 2012 at 8:54 PM, Pekka Enberg <penberg@kernel.org> wrote:
> I played around with the "--debug-ioport" command line option and was able
> to cheat my way past SeaBIOS POST phase. Should SeaBIOS automatically pick
> up virtio devices and attempt to boot them?

Aah, I guess we need to implement proper support for QEMU BIOS config
port ("0x510") because the dummy port is accidentally asking for a
boot menu.

If I disable boot menu support from SeaBIOS, I'm now seeing this PCI
"out of address space" error which I suppose is what Ron and John were
talking about earlier:

[penberg@tux kvm]$ ./vm run --firmware
/home/penberg/seabios/out/bios.bin -d
~/images/debian_lenny_amd64_standard.qcow2 --debug-ioport
  Warning: Forcing read-only support for QCOW
  # lkvm run -k ../../arch/x86/boot/bzImage -m 448 -c 4 --name guest-12176
Start bios (version pre-1.6.4-20120224_205725-tux)
enabling shadow ram
Unable to unlock ram - bridge not found
qemu_cfg_present=0
Find memory size
Add to e820 map: 00000000 00100000 1
Add to e820 map: 00000000 00000000 1
Add to e820 map: fffc0000 00040000 2
Add to e820 map: 000a0000 00050000 -1
Add to e820 map: 000f0000 00010000 2
Add to e820 map: fffbc000 00004000 2
Ram Size=0x00100000 (0x0000000000000000 high)
malloc setup
pmm_malloc zone=0x000f0060 handle=ffffffff size=51132 align=10
ret=0x00083780 (detail=0x0008ff40)
Relocating init from 0x000e3950 to 0x00083780 (size 51132)
malloc fixup reloc
init ivt
init bda
Add to e820 map: 0009fc00 00000400 2
init pic
init timer
tsc calibrate start=25589844 end=25596132 diff=6288
CPU Mhz=3
math cp init
init mtrr
pci setup
=== PCI bus & bridge init ===
PCI: pci_bios_init_bus_rec bus = 0x0
=== PCI device probing ===
PCI probe
pmm_malloc zone=0x0008fe90 handle=ffffffff size=112 align=10
ret=0x000836b0 (detail=0x00083720)
PCI device 00:01.0 (vd=1af4:1001 c=0180)
pmm_malloc zone=0x0008fe90 handle=ffffffff size=112 align=10
ret=0x00083610 (detail=0x00083680)
PCI device 00:02.0 (vd=1af4:1000 c=0200)
Found 2 PCI devices (max PCI bus is 00)
=== PCI new allocation pass #1 ===
pmm_malloc zone=0x0008fe90 handle=ffffffff size=532 align=10
ret=0x000833c0 (detail=0x000835e0)
PCI: check devices
PCI: out of address space

                                Pekka

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Pekka Enberg]

On Fri, Feb 24, 2012 at 9:01 PM, Pekka Enberg <penberg@kernel.org> wrote:
> Aah, I guess we need to implement proper support for QEMU BIOS config
> port ("0x510") because the dummy port is accidentally asking for a
> boot menu.
>
> If I disable boot menu support from SeaBIOS, I'm now seeing this PCI
> "out of address space" error which I suppose is what Ron and John were
> talking about earlier:

[snip, snip]

So looking at SeaBIOS code, it seems to me we could simply make LKVM
lie to it by claiming to be coreboot and get away with it, no? We'd
basically avoid all the PCI allocation passes and such.

How difficult is it to initialize a struct cb_header data structure?
src/coreboot.c::find_cb_header() seems to be rather strict but
certainly nothing that we fundamentally can't do.

Another option is to extend the Xen codepaths to support LKVM too.

                                Pekka

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[John Floren]

On Fri, Feb 24, 2012 at 11:18 AM, Pekka Enberg <penberg@kernel.org> wrote:
> On Fri, Feb 24, 2012 at 9:01 PM, Pekka Enberg <penberg@kernel.org> wrote:
>> Aah, I guess we need to implement proper support for QEMU BIOS config
>> port ("0x510") because the dummy port is accidentally asking for a
>> boot menu.
>>
>> If I disable boot menu support from SeaBIOS, I'm now seeing this PCI
>> "out of address space" error which I suppose is what Ron and John were
>> talking about earlier:
>
> [snip, snip]
>
> So looking at SeaBIOS code, it seems to me we could simply make LKVM
> lie to it by claiming to be coreboot and get away with it, no? We'd
> basically avoid all the PCI allocation passes and such.
>
> How difficult is it to initialize a struct cb_header data structure?
> src/coreboot.c::find_cb_header() seems to be rather strict but
> certainly nothing that we fundamentally can't do.
>
> Another option is to extend the Xen codepaths to support LKVM too.
>
>                                Pekka

Just tell SeaBIOS to compile for Coreboot. That seemed to get me going.

John

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[John Floren]

[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]

Here are some small changes I made to get SeaBIOS to work on lkvm.
Some may not be entirely needed.

Oh, and I've also attached the .config I used.

John

On Fri, Feb 24, 2012 at 11:29 AM, John Floren <john@jfloren.net> wrote:
> On Fri, Feb 24, 2012 at 11:18 AM, Pekka Enberg <penberg@kernel.org> wrote:
>> On Fri, Feb 24, 2012 at 9:01 PM, Pekka Enberg <penberg@kernel.org> wrote:
>>> Aah, I guess we need to implement proper support for QEMU BIOS config
>>> port ("0x510") because the dummy port is accidentally asking for a
>>> boot menu.
>>>
>>> If I disable boot menu support from SeaBIOS, I'm now seeing this PCI
>>> "out of address space" error which I suppose is what Ron and John were
>>> talking about earlier:
>>
>> [snip, snip]
>>
>> So looking at SeaBIOS code, it seems to me we could simply make LKVM
>> lie to it by claiming to be coreboot and get away with it, no? We'd
>> basically avoid all the PCI allocation passes and such.
>>
>> How difficult is it to initialize a struct cb_header data structure?
>> src/coreboot.c::find_cb_header() seems to be rather strict but
>> certainly nothing that we fundamentally can't do.
>>
>> Another option is to extend the Xen codepaths to support LKVM too.
>>
>>                                Pekka
>
> Just tell SeaBIOS to compile for Coreboot. That seemed to get me going.
>
> John

[-- Attachment #2: for-lkvm.patch --]
[-- Type: text/x-patch, Size: 5837 bytes --]

From 3f304bb583627d000be0c8ea8039c0c7c8a4c781 Mon Sep 17 00:00:00 2001
From: John Floren <john@jfloren.net>
Date: Fri, 24 Feb 2012 17:11:44 -0800
Subject: [PATCH] Tweaks to allow SeaBIOS to boot on lkvm. Remember to build
 for Coreboot when you do make menuconfig.

---
 src/Kconfig      |    8 ++++----
 src/clock.c      |    3 +++
 src/coreboot.c   |    4 ++--
 src/paravirt.c   |    4 ++--
 src/pirtable.c   |    2 +-
 src/post.c       |    8 ++++----
 src/virtio-blk.c |   25 +++++++++++++++----------
 7 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/src/Kconfig b/src/Kconfig
index 250663a..21e13aa 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -108,7 +108,7 @@ menu "Hardware support"
         help
             Support for AHCI disk code.
     config VIRTIO_BLK
-        depends on DRIVES && !COREBOOT
+        depends on DRIVES
         bool "virtio-blk controllers"
         default y
         help
@@ -298,13 +298,13 @@ endmenu
 
 menu "BIOS Tables"
     config PIRTABLE
-        depends on !COREBOOT
+#        depends on !COREBOOT
         bool "PIR table"
         default y
         help
             Support generation of a PIR table in 0xf000 segment.
     config MPTABLE
-        depends on !COREBOOT
+#        depends on !COREBOOT
         bool "MPTable"
         default y
         help
@@ -316,7 +316,7 @@ menu "BIOS Tables"
             Support generation of SM BIOS tables.  This is also
             sometimes called DMI.
     config ACPI
-        depends on !COREBOOT
+#        depends on !COREBOOT
         bool "ACPI"
         default y
         help
diff --git a/src/clock.c b/src/clock.c
index e8a48a1..ee6c15b 100644
--- a/src/clock.c
+++ b/src/clock.c
@@ -68,6 +68,7 @@ u8 no_tsc VAR16VISIBLE;
 static void
 calibrate_tsc(void)
 {
+if (0) {
     u32 eax, ebx, ecx, edx, cpuid_features = 0;
     cpuid(0, &eax, &ebx, &ecx, &edx);
     if (eax > 0)
@@ -107,6 +108,8 @@ calibrate_tsc(void)
 
     dprintf(1, "CPU Mhz=%u\n", hz / 1000000);
 }
+SET_GLOBAL(cpu_khz, 2128000);
+}
 
 static u64
 emulate_tsc(void)
diff --git a/src/coreboot.c b/src/coreboot.c
index e328c15..976f20d 100644
--- a/src/coreboot.c
+++ b/src/coreboot.c
@@ -179,9 +179,9 @@ coreboot_fill_map(void)
 fail:
     // No table found..  Use 16Megs as a dummy value.
     dprintf(1, "Unable to find coreboot table!\n");
-    RamSize = 16*1024*1024;
+    RamSize = 512*1024*1024;
     RamSizeOver4G = 0;
-    add_e820(0, 16*1024*1024, E820_RAM);
+    add_e820(0, RamSize, E820_RAM);
     return;
 }
 
diff --git a/src/paravirt.c b/src/paravirt.c
index 9cf77de..c38a84d 100644
--- a/src/paravirt.c
+++ b/src/paravirt.c
@@ -46,8 +46,8 @@ void qemu_cfg_port_probe(void)
     char *sig = "QEMU";
     int i;
 
-    if (CONFIG_COREBOOT)
-        return;
+//    if (CONFIG_COREBOOT)
+//        return;
 
     qemu_cfg_present = 1;
 
diff --git a/src/pirtable.c b/src/pirtable.c
index 4c3f1ff..7cb358e 100644
--- a/src/pirtable.c
+++ b/src/pirtable.c
@@ -17,7 +17,7 @@ struct pir_table {
 } PACKED;
 
 extern struct pir_table PIR_TABLE;
-#if CONFIG_PIRTABLE && !CONFIG_COREBOOT
+#if CONFIG_PIRTABLE //&& !CONFIG_COREBOOT
 struct pir_table PIR_TABLE __aligned(16) VAR16EXPORT = {
     .pir = {
         .version = 0x0100,
diff --git a/src/post.c b/src/post.c
index b4ad1fa..880a238 100644
--- a/src/post.c
+++ b/src/post.c
@@ -157,10 +157,10 @@ ram_probe(void)
 static void
 init_bios_tables(void)
 {
-    if (CONFIG_COREBOOT) {
-        coreboot_copy_biostable();
-        return;
-    }
+//    if (CONFIG_COREBOOT) {
+//        coreboot_copy_biostable();
+//        return;
+//    }
     if (usingXen()) {
 	xen_copy_biostables();
 	return;
diff --git a/src/virtio-blk.c b/src/virtio-blk.c
index b869189..9022601 100644
--- a/src/virtio-blk.c
+++ b/src/virtio-blk.c
@@ -28,6 +28,7 @@ struct virtiodrive_s {
 static int
 virtio_blk_op(struct disk_op_s *op, int write)
 {
+    int i = 0;
     struct virtiodrive_s *vdrive_g =
         container_of(op->drive_g, struct virtiodrive_s, drive);
     struct vring_virtqueue *vq = GET_GLOBAL(vdrive_g->vq);
@@ -51,7 +52,7 @@ virtio_blk_op(struct disk_op_s *op, int write)
             .length	= sizeof(status),
         },
     };
-
+dprintf(3, "in virtio_blk_op\n");
     /* Add to virtqueue and kick host */
     if (write)
         vring_add_buf(vq, sg, 2, 1, 0, 0);
@@ -60,9 +61,15 @@ virtio_blk_op(struct disk_op_s *op, int write)
     vring_kick(GET_GLOBAL(vdrive_g->ioaddr), vq, 1);
 
     /* Wait for reply */
-    while (!vring_more_used(vq))
+    while (!vring_more_used(vq)) {
         usleep(5);
-
+	if (++i > 1000) {
+		vring_kick(GET_GLOBAL(vdrive_g->ioaddr), vq, 1);
+		i = 0;
+		dprintf(1, "THIS SHOULD NEVER HAPPEN! Re-kicking virtio queue!\n");
+	}
+    }
+dprintf(3, "got reply\n");
     /* Reclaim virtqueue element */
     vring_get_buf(vq, NULL);
 
@@ -77,8 +84,8 @@ virtio_blk_op(struct disk_op_s *op, int write)
 int
 process_virtio_blk_op(struct disk_op_s *op)
 {
-    if (! CONFIG_VIRTIO_BLK || CONFIG_COREBOOT)
-        return 0;
+//    if (! CONFIG_VIRTIO_BLK || CONFIG_COREBOOT)
+//        return 0;
     switch (op->command) {
     case CMD_READ:
         return virtio_blk_op(op, 0);
@@ -100,7 +107,7 @@ static void
 init_virtio_blk(struct pci_device *pci)
 {
     u16 bdf = pci->bdf;
-    dprintf(1, "found virtio-blk at %x:%x\n", pci_bdf_to_bus(bdf),
+    dprintf(3, "found virtio-blk at %x:%x\n", pci_bdf_to_bus(bdf),
             pci_bdf_to_dev(bdf));
     struct virtiodrive_s *vdrive_g = malloc_fseg(sizeof(*vdrive_g));
     if (!vdrive_g) {
@@ -159,10 +166,8 @@ void
 virtio_blk_setup(void)
 {
     ASSERT32FLAT();
-    if (! CONFIG_VIRTIO_BLK || CONFIG_COREBOOT)
-        return;
-
-    dprintf(3, "init virtio-blk\n");
+//    if (! CONFIG_VIRTIO_BLK || CONFIG_COREBOOT)
+//        return;
 
     struct pci_device *pci;
     foreachpci(pci) {
-- 
1.7.9


[-- Attachment #3: .config --]
[-- Type: application/octet-stream, Size: 1272 bytes --]

#
# Automatically generated make config: don't edit
# SeaBIOS Configuration
# Thu Feb 23 16:09:18 2012
#

#
# General Features
#
CONFIG_COREBOOT=y
# CONFIG_THREADS is not set
# CONFIG_RELOCATE_INIT is not set
# CONFIG_BOOTMENU is not set
# CONFIG_BOOTORDER is not set
# CONFIG_COREBOOT_FLASH is not set

#
# Hardware support
#
# CONFIG_ATA is not set
# CONFIG_AHCI is not set
CONFIG_VIRTIO_BLK=y
# CONFIG_FLOPPY is not set
# CONFIG_USB is not set
CONFIG_SERIAL=y
# CONFIG_LPT is not set

#
# BIOS interfaces
#
CONFIG_DRIVES=y
# CONFIG_CDROM_BOOT is not set
# CONFIG_PCIBIOS is not set
# CONFIG_APMBIOS is not set
# CONFIG_PNPBIOS is not set
# CONFIG_OPTIONROMS is not set
CONFIG_BOOT=y
# CONFIG_KEYBOARD is not set
# CONFIG_MOUSE is not set
# CONFIG_S3_RESUME is not set
# CONFIG_VGAHOOKS is not set
# CONFIG_DISABLE_A20 is not set

#
# BIOS Tables
#
CONFIG_PIRTABLE=y
# CONFIG_MPTABLE is not set
# CONFIG_SMBIOS is not set
# CONFIG_ACPI is not set

#
# VGA ROM
#
CONFIG_NO_VGABIOS=y
# CONFIG_VGA_STANDARD_VGA is not set
# CONFIG_VGA_CIRRUS is not set
# CONFIG_VGA_BOCHS is not set
# CONFIG_VGA_GEODEGX2 is not set
# CONFIG_VGA_GEODELX is not set
# CONFIG_BUILD_VGABIOS is not set

#
# Debugging
#
CONFIG_DEBUG_LEVEL=8
CONFIG_DEBUG_SERIAL=y
CONFIG_DEBUG_SERIAL_PORT=0x3f8

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Gerd Hoffmann]

  Hi,

> So looking at SeaBIOS code, it seems to me we could simply make LKVM
> lie to it by claiming to be coreboot and get away with it, no? We'd
> basically avoid all the PCI allocation passes and such.

I doubt this is going to fly if you want seabios boot from a
virtio-blk-pci device ...

cheers,
  Gerd

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Gleb Natapov]

On Mon, Feb 27, 2012 at 11:44:57AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > So looking at SeaBIOS code, it seems to me we could simply make LKVM
> > lie to it by claiming to be coreboot and get away with it, no? We'd
> > basically avoid all the PCI allocation passes and such.
> 
> I doubt this is going to fly if you want seabios boot from a
> virtio-blk-pci device ...
> 
AFAIK kvm-tool initialize PCI by itself so it actually may work (and,
very likely, the best thing to do).

--
			Gleb.

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[ron minnich]

On Mon, Feb 27, 2012 at 3:15 AM, Gleb Natapov <gleb@redhat.com> wrote:

> AFAIK kvm-tool initialize PCI by itself so it actually may work (and,
> very likely, the best thing to do).

we're going in circles. kvm-tool does not set the base address of some
of the BARs and seabios does not do it quite correctly. That's what
we're trying to decide how to fix. Probably the fastest way out is
just run coreboot+seabios for kvm-tool.

ron

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Gleb Natapov]

On Mon, Feb 27, 2012 at 08:24:02AM -0800, ron minnich wrote:
> On Mon, Feb 27, 2012 at 3:15 AM, Gleb Natapov <gleb@redhat.com> wrote:
> 
> > AFAIK kvm-tool initialize PCI by itself so it actually may work (and,
> > very likely, the best thing to do).
> 
> we're going in circles. kvm-tool does not set the base address of some
> of the BARs and seabios does not do it quite correctly. That's what
> we're trying to decide how to fix. Probably the fastest way out is
> just run coreboot+seabios for kvm-tool.
> 
Then make kvm-tool set it. Why do you need coreboot/seabios for that?

--
			Gleb.

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[ron minnich]

On Mon, Feb 27, 2012 at 8:38 AM, Gleb Natapov <gleb@redhat.com> wrote:

> Then make kvm-tool set it. Why do you need coreboot/seabios for that?

First thing we looked at. For a number of reasons it seems ugly.
Either we can pick a bunch of fixed base addresses for the kvm-tool
resources, and track them all as new devices are added to kvm-tool
over time (yuck), or we can replicate some of the pci BAR code in
kvm-tool that already exists in seabios/coreboot, which also seems
yucky.

kvm-tool provides a more realistic environment in some ways for guests
than qemu. Less is initialized. Given that coreboot does the things we
need done, and it's easy to build, I don't see a reason not to use it.

Of course due to the ongoing ld alignment bug issue, I can't build
seabios to test this idea, so have not. If I had time I'd put in a
patch to seabios to not depend on this feature, since it's broken on
every linux system I own, but ...

ron

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Gleb Natapov]

On Mon, Feb 27, 2012 at 08:46:10AM -0800, ron minnich wrote:
> On Mon, Feb 27, 2012 at 8:38 AM, Gleb Natapov <gleb@redhat.com> wrote:
> 
> > Then make kvm-tool set it. Why do you need coreboot/seabios for that?
> 
> First thing we looked at. For a number of reasons it seems ugly.
That is not "ugly", this is by design. Isn't it.

> Either we can pick a bunch of fixed base addresses for the kvm-tool
> resources, and track them all as new devices are added to kvm-tool
> over time (yuck), or we can replicate some of the pci BAR code in
> kvm-tool that already exists in seabios/coreboot, which also seems
> yucky.
kvm-tool design goal, as far as I can tell, was to be as much self
contained as possible. kvm-tool should be usable without seabios at
all if legacy bios functionality is not needed. To achieve that some
firmware functionality (mostly HW initialization related) is already
replicated in kvm-tool. Again, this is by design. Coreboot, when used
in conjunction with seabios on real HW, takes care of HW initialization,
so if kvm-tool want to follow its design direction it should assume role
of the coreboot and load Seabios only when legacy bios functionality is
needed.

BTW there is code duplication between coreboot and seabios too. Both of
them can be used to init QEMU HW.

> 
> kvm-tool provides a more realistic environment in some ways for guests
> than qemu.
How is kvm-tool provides a more realistic environment? You get it
opposite.

>             Less is initialized.
Nothing is initialized in QEMU. Every single bit of initialization is
done by a guest code. Be it Seabios/coreboot/openfirmware/tianocore or
guest OS itself. And yes, qemu can run all of the firmwares above
without single line of code to support any of them.

>                                  Given that coreboot does the things we
> need done, and it's easy to build, I don't see a reason not to use it.
I am not very familiar with coreboot, but I think you will have to write
kvm-tool platform support for coreboot before you can run it in
kvm-tool.

--
			Gleb.

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[ron minnich]

I'm less interested in this argument than getting something that
works, however it happens, so I'll let it stop with the comment that i
don't agree with you :-)

thanks

ron

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Gerd Hoffmann]

On 02/24/12 19:54, Pekka Enberg wrote:
> Hi,
> 
> I played around with the "--debug-ioport" command line option and was able 
> to cheat my way past SeaBIOS POST phase. Should SeaBIOS automatically pick 
> up virtio devices and attempt to boot them?

Yes, it can handle virtio-blk (and soon virtio-scsi too).

You probably want to implement the fw_cfg (firmware config) interface
which is used as communication path between qemu and seabios, to pass
around stuff like e820 table and option roms.

Also http://sgabios.googlecode.com/svn/trunk is nice when working with a
serial console.

HTH,
  Gerd

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Pekka Enberg]

On Fri, 2012-02-24 at 08:27 -0800, ron minnich wrote:
> I think you need to look at the change floren and I worked up,
> assuming I can get him to release it to you.
> 
> Our change is a bit less complex and adds fewer lines of code. You've
> added an extra option to load a bios, and it will be somewhat limited
> as those ever-demanding users continue to demand more and more :-)
> 
> What we did instead is let users specify the load address of the
> kernel, and the IP from which to start. So our command line ends up
> with --kernel being a seabios image, and using switches to set the
> load address at e0000 and the initial IP at ffff0. Of course, now that
> I know the memory is "reflected" to high memory too, the initial IP
> could just as easily be fffffff0.

I went ahead and applied my patches. Feel free to extend it to support
the kind of configuration options you need.

I personally want a simple command line option that does everything for
me automatically.

			Pekka

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[ron minnich]

On Fri, Feb 24, 2012 at 10:00 AM, Pekka Enberg <penberg@kernel.org> wrote:

> I personally want a simple command line option that does everything for
> me automatically.

it's a great goal, and I like the swtich a lot. But if you want that
-firmware option why not have it turn into an automated setting of the
options that John's got in his patch? It really is less code overall
and I love the fact that kvm tool is so tiny, I'm anxious to keep it
that way :-)

ron

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Pekka Enberg]

On Fri, Feb 24, 2012 at 10:00 AM, Pekka Enberg <penberg@kernel.org> wrote:
>> I personally want a simple command line option that does everything for
>> me automatically.

On Fri, Feb 24, 2012 at 9:07 PM, ron minnich <rminnich@gmail.com> wrote:
> it's a great goal, and I like the swtich a lot. But if you want that
> -firmware option why not have it turn into an automated setting of the
> options that John's got in his patch? It really is less code overall
> and I love the fact that kvm tool is so tiny, I'm anxious to keep it
> that way :-)

Hey, feel free to change it that way. I wanted to be able to play with
seabios too and simply got bored while waiting for someone to send a
patch to allow that.

                        Pekka

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Pekka Enberg]

On Fri, 2012-02-24 at 09:23 -0600, Anthony Liguori wrote:
> On 02/24/2012 09:05 AM, Pekka Enberg wrote:
> > This patch adds a "--bios" command line option to "vm run". You can use this to
> > try to boot with SeaBIOS, for example:
> >
> >    ./vm run --bios=/usr/share/seabios/bios.bin \
> > 	 --disk $HOME/images/debian_lenny_amd64_standard.qcow2
> >
> > This doesn't boot yet for obvious reasons but at least people can now start to
> > play with external BIOS images easily.
> 
> You may want to call it firmware as other platforms also have firmware.  For 
> instance, it may be desirable to use the same interface to load SLOF with spapr.

I renamed the command line option to "--firmware".

Re: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"

[Matt Evans]

On 25 Feb 2012, at 02:23, Anthony Liguori <aliguori@us.ibm.com> wrote:

> On 02/24/2012 09:05 AM, Pekka Enberg wrote:
>> This patch adds a "--bios" command line option to "vm run". You can use this to
>> try to boot with SeaBIOS, for example:
>> 
>>   ./vm run --bios=/usr/share/seabios/bios.bin \
>>     --disk $HOME/images/debian_lenny_amd64_standard.qcow2
>> 
>> This doesn't boot yet for obvious reasons but at least people can now start to
>> play with external BIOS images easily.
> 
> You may want to call it firmware as other platforms also have firmware.  For instance, it may be desirable to use the same interface to load SLOF with spapr.

Seconded, "BIOS" is a bit of an arch-specific phrase... and antique ;)


Matt


> 
>> 
>> Cc: Yang Bai<hamo.by@gmail.com>
>> Cc: Matt Evans<matt@ozlabs.org>
>> Cc: Ron Minnich<rminnich@gmail.com>
>> Cc: Anthony Liguori<aliguori@us.ibm.com>
>> Cc: John Floren<john@jfloren.net>
>> Cc: Sasha Levin<levinsasha928@gmail.com>
>> Cc: Cyrill Gorcunov<gorcunov@openvz.org>
>> Cc: Asias He<asias.hejun@gmail.com>
>> Cc: Ingo Molnar<mingo@elte.hu>
>> Signed-off-by: Pekka Enberg<penberg@kernel.org>
>> ---
>>  tools/kvm/Makefile               |    2 +
>>  tools/kvm/builtin-run.c          |   32 +++++++++++++++++++----------
>>  tools/kvm/include/kvm/kvm.h      |    1 +
>>  tools/kvm/powerpc/boot.c         |    8 +++++++
>>  tools/kvm/x86/boot.c             |   41 ++++++++++++++++++++++++++++++++++++++
>>  tools/kvm/x86/include/kvm/bios.h |    3 +-
>>  6 files changed, 75 insertions(+), 12 deletions(-)
>>  create mode 100644 tools/kvm/powerpc/boot.c
>>  create mode 100644 tools/kvm/x86/boot.c
>> 
>> diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
>> index cfa5547..0a9c2cc 100644
>> --- a/tools/kvm/Makefile
>> +++ b/tools/kvm/Makefile
>> @@ -113,6 +113,7 @@ LIBFDT_OBJS = $(patsubst %,../../scripts/dtc/libfdt/%,$(LIBFDT_SRC))
>>  #x86
>>  ifeq ($(ARCH),x86)
>>      DEFINES += -DCONFIG_X86
>> +    OBJS    += x86/boot.o
>>      OBJS    += x86/cpuid.o
>>      OBJS    += x86/interrupt.o
>>      OBJS    += x86/ioport.o
>> @@ -129,6 +130,7 @@ endif
>>  # POWER/ppc:  Actually only support ppc64 currently.
>>  ifeq ($(uname_M), ppc64)
>>      DEFINES += -DCONFIG_PPC
>> +    OBJS    += powerpc/boot.o
>>      OBJS    += powerpc/ioport.o
>>      OBJS    += powerpc/irq.o
>>      OBJS    += powerpc/kvm.o
>> diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
>> index 466169e..f96c581 100644
>> --- a/tools/kvm/builtin-run.c
>> +++ b/tools/kvm/builtin-run.c
>> @@ -76,6 +76,7 @@ static const char *kernel_cmdline;
>>  static const char *kernel_filename;
>>  static const char *vmlinux_filename;
>>  static const char *initrd_filename;
>> +static const char *bios_filename;
>>  static const char *image_filename[MAX_DISK_IMAGES];
>>  static const char *console;
>>  static const char *dev;
>> @@ -458,6 +459,8 @@ static const struct option options[] = {
>>              "Initial RAM disk image"),
>>      OPT_STRING('p', "params",&kernel_cmdline, "params",
>>              "Kernel command line arguments"),
>> +    OPT_STRING('b', "bios",&bios_filename, "bios",
>> +            "BIOS to boot in virtual machine"),
>> 
>>      OPT_GROUP("Networking options:"),
>>      OPT_CALLBACK_DEFAULT('n', "network", NULL, "network params",
>> @@ -1110,14 +1113,16 @@ static int kvm_cmd_run_init(int argc, const char **argv)
>>      printf("  # %s run -k %s -m %Lu -c %d --name %s\n", KVM_BINARY_NAME,
>>          kernel_filename, ram_size / 1024 / 1024, nrcpus, guest_name);
>> 
>> -    if (!kvm__load_kernel(kvm, kernel_filename, initrd_filename,
>> -                real_cmdline, vidmode))
>> -        die("unable to load kernel %s", kernel_filename);
>> +    if (!bios_filename) {
>> +        if (!kvm__load_kernel(kvm, kernel_filename,
>> +                initrd_filename, real_cmdline, vidmode))
>> +            die("unable to load kernel %s", kernel_filename);
>> 
>> -    kvm->vmlinux = vmlinux_filename;
>> -    r = symbol_init(kvm);
>> -    if (r<  0)
>> -        pr_debug("symbol_init() failed with error %d\n", r);
>> +        kvm->vmlinux = vmlinux_filename;
>> +        r = symbol_init(kvm);
>> +        if (r<  0)
>> +            pr_debug("symbol_init() failed with error %d\n", r);
>> +    }
>> 
>>      ioport__setup_arch();
>> 
>> @@ -1218,10 +1223,15 @@ static int kvm_cmd_run_init(int argc, const char **argv)
>> 
>>      kvm__start_timer(kvm);
>> 
>> -    kvm__arch_setup_firmware(kvm);
>> -    if (r<  0) {
>> -        pr_err("kvm__arch_setup_firmware() failed with error %d\n", r);
>> -        goto fail;
>> +    if (bios_filename) {
>> +        if (!kvm__load_bios(kvm, bios_filename))
>> +            die("unable to load bios %s: %s", bios_filename, strerror(errno));
>> +    } else {
>> +        kvm__arch_setup_firmware(kvm);
>> +        if (r<  0) {
>> +            pr_err("kvm__arch_setup_firmware() failed with error %d\n", r);
>> +            goto fail;
>> +        }
>>      }
>> 
>>      for (i = 0; i<  nrcpus; i++) {
>> diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
>> index 7870118..258d11a 100644
>> --- a/tools/kvm/include/kvm/kvm.h
>> +++ b/tools/kvm/include/kvm/kvm.h
>> @@ -39,6 +39,7 @@ int kvm__recommended_cpus(struct kvm *kvm);
>>  int kvm__max_cpus(struct kvm *kvm);
>>  void kvm__init_ram(struct kvm *kvm);
>>  int kvm__exit(struct kvm *kvm);
>> +bool kvm__load_bios(struct kvm *kvm, const char *bios_filename);
>>  bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
>>              const char *initrd_filename, const char *kernel_cmdline, u16 vidmode);
>>  void kvm__start_timer(struct kvm *kvm);
>> diff --git a/tools/kvm/powerpc/boot.c b/tools/kvm/powerpc/boot.c
>> new file mode 100644
>> index 0000000..8c99831
>> --- /dev/null
>> +++ b/tools/kvm/powerpc/boot.c
>> @@ -0,0 +1,8 @@
>> +#include "kvm/kvm.h"
>> +
>> +#include<stdbool.h>
>> +
>> +bool kvm__load_bios(struct kvm *kvm, const char *bios_filename)
>> +{
>> +    return false;
>> +}
>> diff --git a/tools/kvm/x86/boot.c b/tools/kvm/x86/boot.c
>> new file mode 100644
>> index 0000000..383d9f7
>> --- /dev/null
>> +++ b/tools/kvm/x86/boot.c
>> @@ -0,0 +1,41 @@
>> +#include "kvm/kvm.h"
>> +
>> +#include "kvm/util.h"
>> +
>> +#include<sys/types.h>
>> +#include<sys/stat.h>
>> +#include<stdbool.h>
>> +#include<fcntl.h>
>> +
>> +#define BIOS_SELECTOR    0xf000
>> +#define BIOS_IP        0xfff0
>> +#define BIOS_SP        0x8000
>> +
>> +bool kvm__load_bios(struct kvm *kvm, const char *bios_filename)
>> +{
>> +    struct stat st;
>> +    void *p;
>> +    int fd;
>> +    int nr;
>> +
>> +    fd = open(bios_filename, O_RDONLY);
>> +    if (fd<  0)
>> +        return false;
>> +
>> +    if (fstat(fd,&st))
>> +        return false;
>> +
>> +    if (st.st_size>  MB_BIOS_SIZE)
>> +        die("BIOS image %s is too big to fit in memory (%lu KB).\n", bios_filename, st.st_size / 1024);
>> +
>> +    p = guest_flat_to_host(kvm, MB_BIOS_BEGIN);
>> +
>> +    while ((nr = read(fd, p, st.st_size))>  0)
>> +        p += nr;
>> +
>> +    kvm->boot_selector    = BIOS_SELECTOR;
>> +    kvm->boot_ip        = BIOS_IP;
>> +    kvm->boot_sp        = BIOS_SP;
>> +
>> +    return true;
>> +}
>> diff --git a/tools/kvm/x86/include/kvm/bios.h b/tools/kvm/x86/include/kvm/bios.h
>> index de569bc..9d677ae 100644
>> --- a/tools/kvm/x86/include/kvm/bios.h
>> +++ b/tools/kvm/x86/include/kvm/bios.h
>> @@ -26,8 +26,9 @@
>> 
>>  #define E820_MAP_START            EBDA_START
>> 
>> -#define MB_BIOS_BEGIN            0x000f0000
>> +#define MB_BIOS_BEGIN            0x000e0000
>>  #define MB_BIOS_END            0x000fffff
>> +#define MB_BIOS_SIZE            (MB_BIOS_END - MB_BIOS_BEGIN + 1)
>> 
>>  #define VGA_RAM_BEGIN            0x000a0000
>>  #define VGA_RAM_END            0x000bffff
> 
> FYI, this will work with SeaBIOS because it's currently 128k.  But the normal thing is to load the last 128k of the BIOS ROM at 1MB - 128k, with the full ROM mapped at 4GB - sizeof(ROM).
> 
> SeaBIOS certainly expects the ROM to be located in both places.
> 
> Also note that the area at 1MB - 128k is special.  There's two distinct regions mapped here, normal RAM and then the BIOS ROM.  Depending on the settings of the PAM registers, a guest can chose to see either piece of memory.  In fact, it can control read and write access independently such that writes go to RAM and reads come from the BIOS area.
> 
> This is used to implement shadowing.  SeaBIOS does special case this under KVM since it's not possible to execute out of MMIO under KVM.
> 
> Regards,
> 
> Anthony Liguori