18.06.2012 23:14, Bastian Blank wrote:
Package: seabios Version: 1.7.0-1 Severity: wishlist
Please enable Xen support in seabios. It will be used by the next Xen release and maybe the version in Wheezy.
Do you know what's the outcome of this? As far as I remember, it is just a config option, but it is not enabled by default, do you know why?
Kevin, can you comment please?
Thank you!
/mjt
On Tue, Jun 19, 2012 at 01:36:31PM +0400, Michael Tokarev wrote:
Do you know what's the outcome of this? As far as I remember, it is just a config option, but it is not enabled by default, do you know why?
The outcome right now is: it does not work. For some reason if fails to start the VGA bios. Will have to do some debugging. 1.6.3.2 works flawless for this.
Bastian
On Tue, 2012-06-19 at 13:45 +0200, Bastian Blank wrote:
On Tue, Jun 19, 2012 at 01:36:31PM +0400, Michael Tokarev wrote:
Do you know what's the outcome of this? As far as I remember, it is just a config option, but it is not enabled by default, do you know why?
The outcome right now is: it does not work. For some reason if fails to start the VGA bios. Will have to do some debugging. 1.6.3.2 works flawless for this.
I have yet to try the 1.7 stream or later (Xen 4.2 is going to release using a 1.6.3.x). I should add this to my list...
Ian.
On Tue, 2012-06-19 at 13:36 +0400, Michael Tokarev wrote:
18.06.2012 23:14, Bastian Blank wrote:
Package: seabios Version: 1.7.0-1 Severity: wishlist
Please enable Xen support in seabios. It will be used by the next Xen release and maybe the version in Wheezy.
Do you know what's the outcome of this? As far as I remember, it is just a config option, but it is not enabled by default, do you know why?
I seem to have made it "default n" but I don't recall why. Possibly I just didn't realise that people would be building a single SeaBIOS for multiple uses -- I expected that folks would build one tailored to their specific usecase. Or maybe it's just a hangover from my being used to Linux's policy of defaulting things like this to off...
Ian.
Kevin, can you comment please?
Thank you!
/mjt
SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
On Tue, Jun 19, 2012 at 05:44:08PM +0100, Ian Campbell wrote:
On Tue, 2012-06-19 at 13:36 +0400, Michael Tokarev wrote:
18.06.2012 23:14, Bastian Blank wrote:
Package: seabios Version: 1.7.0-1 Severity: wishlist
Please enable Xen support in seabios. It will be used by the next Xen release and maybe the version in Wheezy.
Do you know what's the outcome of this? As far as I remember, it is just a config option, but it is not enabled by default, do you know why?
I seem to have made it "default n" but I don't recall why. Possibly I just didn't realise that people would be building a single SeaBIOS for multiple uses -- I expected that folks would build one tailored to their specific usecase. Or maybe it's just a hangover from my being used to Linux's policy of defaulting things like this to off...
I also don't recall why.
Kevin, can you comment please?
I looked, and it seems harmless to turn it on by default.
-Kevin
On Tue, 2012-06-19 at 19:11 -0400, Kevin O'Connor wrote:
On Tue, Jun 19, 2012 at 05:44:08PM +0100, Ian Campbell wrote:
On Tue, 2012-06-19 at 13:36 +0400, Michael Tokarev wrote:
18.06.2012 23:14, Bastian Blank wrote:
Package: seabios Version: 1.7.0-1 Severity: wishlist
Please enable Xen support in seabios. It will be used by the next Xen release and maybe the version in Wheezy.
Do you know what's the outcome of this? As far as I remember, it is just a config option, but it is not enabled by default, do you know why?
I seem to have made it "default n" but I don't recall why. Possibly I just didn't realise that people would be building a single SeaBIOS for multiple uses -- I expected that folks would build one tailored to their specific usecase. Or maybe it's just a hangover from my being used to Linux's policy of defaulting things like this to off...
I also don't recall why.
Kevin, can you comment please?
I looked, and it seems harmless to turn it on by default.
This seems pretty simple. See below (builds, but doesn't boot on Xen due to pre-existing problem which I need to investigate)
8<---------------------------------
From 91d2f5005a7624a953c5c4a0160d03e03a3928ba Mon Sep 17 00:00:00 2001
From: Ian Campbell ian.campbell@citrix.com Date: Wed, 20 Jun 2012 09:41:18 +0100 Subject: [PATCH] enable Xen support by default.
Signed-off-by: Ian Campbell ian.campbell@citrix.com --- src/Kconfig | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/Kconfig b/src/Kconfig index 25b2b1b..8120ff7 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -13,7 +13,7 @@ menu "General Features" config XEN depends on !COREBOOT bool "Build for Xen HVM" - default n + default y help Configure to be used by xen hvmloader, for a HVM guest.
On Wed, 2012-06-20 at 10:22 +0100, Ian Campbell wrote:
Subject: [PATCH] enable Xen support by default.
In this context I thought it would also be useful to make CONFIG_DEBUG_IO_PORT dynamic. However with the below I get lots of build errors about xen_cpuid_base not being defined. I got similar errors without the VAR16VISIBLE and GET_GLOBAL hunks. I suspect this is due to the variable being used in both 32 and 16 bit mode and my not knowing what I'm doing in that regard ;-)
diff --git a/src/Kconfig b/src/Kconfig index 8120ff7..4487844 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -364,8 +364,10 @@ menu "Debugging" config DEBUG_IO_PORT depends on DEBUG_IO hex "Debug IO port address" - default 0x0402 + default 0x00 help Bochs uses the 0x0402 address by default, whereas Xen makes the 0xe9 IO address available for guests use. + + The default (0) is to autodetect which to use. endmenu diff --git a/src/output.c b/src/output.c index 37c4942..2826e00 100644 --- a/src/output.c +++ b/src/output.c @@ -11,6 +11,7 @@ #include "bregs.h" // struct bregs #include "config.h" // CONFIG_* #include "biosvar.h" // GET_GLOBAL +#include "xen.h" // usingXen
struct putcinfo { void (*func)(struct putcinfo *info, char c); @@ -69,6 +70,15 @@ debug_serial_flush(void) return; }
+static int debug_io_port(void) +{ + if (CONFIG_DEBUG_IO_PORT) + return CONFIG_DEBUG_IO_PORT; + if (usingXen()) + return 0xe9; + return 0x402; +} + // Write a character to debug port(s). static void putc_debug(struct putcinfo *action, char c) @@ -77,7 +87,7 @@ putc_debug(struct putcinfo *action, char c) return; if (CONFIG_DEBUG_IO) // Send character to debug port. - outb(c, CONFIG_DEBUG_IO_PORT); + outb(c, debug_io_port()); if (c == '\n') debug_serial('\r'); debug_serial(c); diff --git a/src/xen.c b/src/xen.c index 961e316..a3e12d8 100644 --- a/src/xen.c +++ b/src/xen.c @@ -13,7 +13,7 @@
#define INFO_PHYSICAL_ADDRESS 0x00001000
-u32 xen_cpuid_base = 0; +u32 xen_cpuid_base VAR16VISIBLE = 0;
struct xen_seabios_info { char signature[14]; /* XenHVMSeaBIOS\0 */ diff --git a/src/xen.h b/src/xen.h index cc506a6..3eb7771 100644 --- a/src/xen.h +++ b/src/xen.h @@ -3,6 +3,7 @@
#include "config.h" // CONFIG_* #include "types.h" // u32 +#include "biosvar.h" // GET_GLOBAL
extern u32 xen_cpuid_base;
@@ -14,7 +15,7 @@ void xen_copy_biostables(void); static inline int usingXen(void) { if (!CONFIG_XEN) return 0; - return (xen_cpuid_base != 0); + return (GET_GLOBAL(xen_cpuid_base) != 0); }
unsigned long xen_hypercall_page;
On Wed, Jun 20, 2012 at 10:25:13AM +0100, Ian Campbell wrote:
In this context I thought it would also be useful to make CONFIG_DEBUG_IO_PORT dynamic.
I think it is easier to either wire 0x402 into Xen or switch completely to it. Is there something like a standard for it?
Bastian
On Wed, Jun 20, 2012 at 10:25:13AM +0100, Ian Campbell wrote:
On Wed, 2012-06-20 at 10:22 +0100, Ian Campbell wrote:
Subject: [PATCH] enable Xen support by default.
In this context I thought it would also be useful to make CONFIG_DEBUG_IO_PORT dynamic. However with the below I get lots of build errors about xen_cpuid_base not being defined. I got similar errors without the VAR16VISIBLE and GET_GLOBAL hunks. I suspect this is due to the variable being used in both 32 and 16 bit mode and my not knowing what I'm doing in that regard ;-)
xen_cpuid_base is defined in xen.c which is only compiled in 32bit mode, so you can't declare a variable as VAR16VISIBLE there.
I wonder if it is simpler to define a "u16 DebugOutputPort VAR16VISIBLE = CONFIG_DEBUG_IO_PORT" in output.c and then override it early in the xen boot sequence though.
-Kevin
On Mon, 2012-06-25 at 18:55 -0400, Kevin O'Connor wrote:
On Wed, Jun 20, 2012 at 10:25:13AM +0100, Ian Campbell wrote:
On Wed, 2012-06-20 at 10:22 +0100, Ian Campbell wrote:
Subject: [PATCH] enable Xen support by default.
In this context I thought it would also be useful to make CONFIG_DEBUG_IO_PORT dynamic. However with the below I get lots of build errors about xen_cpuid_base not being defined. I got similar errors without the VAR16VISIBLE and GET_GLOBAL hunks. I suspect this is due to the variable being used in both 32 and 16 bit mode and my not knowing what I'm doing in that regard ;-)
xen_cpuid_base is defined in xen.c which is only compiled in 32bit mode, so you can't declare a variable as VAR16VISIBLE there.
Ah, right yes, thanks! I think I keep tripping over that...
I wonder if it is simpler to define a "u16 DebugOutputPort VAR16VISIBLE = CONFIG_DEBUG_IO_PORT" in output.c and then override it early in the xen boot sequence though.
Yes, this makes sense. I actually went one further and nuked the Kconfig option, since it's only real non-default use was Xen. So now I makde it default to 0x402 and set it to 0xe9 in the Xen case.
8<-----------------------------------------------------
From 903d84d2c320543ac07ae1298d57643575d6ffc8 Mon Sep 17 00:00:00 2001
From: Ian Campbell ian.campbell@citrix.com Date: Wed, 27 Jun 2012 11:39:01 +0100 Subject: [PATCH] Xen: Autodetect debug I/O port at runtime instead of via Kconfig
This allows a common image which supports Xen to still print debug
Signed-off-by: Ian Campbell ian.campbell@citrix.com --- src/Kconfig | 7 ------- src/output.c | 4 +++- src/util.h | 1 + src/xen.c | 5 +++++ 4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/Kconfig b/src/Kconfig index 8120ff7..8932c9e 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -361,11 +361,4 @@ menu "Debugging" information by outputing strings in a special port present in the IO space.
- config DEBUG_IO_PORT - depends on DEBUG_IO - hex "Debug IO port address" - default 0x0402 - help - Bochs uses the 0x0402 address by default, whereas Xen - makes the 0xe9 IO address available for guests use. endmenu diff --git a/src/output.c b/src/output.c index 37c4942..25300d0 100644 --- a/src/output.c +++ b/src/output.c @@ -23,6 +23,8 @@ struct putcinfo {
#define DEBUG_TIMEOUT 100000
+u16 DebugOutputPort VAR16VISIBLE = 0x402; + void debug_serial_setup(void) { @@ -77,7 +79,7 @@ putc_debug(struct putcinfo *action, char c) return; if (CONFIG_DEBUG_IO) // Send character to debug port. - outb(c, CONFIG_DEBUG_IO_PORT); + outb(c, DebugOutputPort); if (c == '\n') debug_serial('\r'); debug_serial(c); diff --git a/src/util.h b/src/util.h index dbee0e5..ef8ec7c 100644 --- a/src/util.h +++ b/src/util.h @@ -231,6 +231,7 @@ int wait_preempt(void); void check_preempt(void);
// output.c +extern u16 DebugOutputPort; void debug_serial_setup(void); void panic(const char *fmt, ...) __attribute__ ((format (printf, 1, 2))) __noreturn; diff --git a/src/xen.c b/src/xen.c index 961e316..128e6c0 100644 --- a/src/xen.c +++ b/src/xen.c @@ -64,6 +64,9 @@ void xen_probe(void) dprintf(1, "Found hypervisor signature "%s" at %x\n", signature, base); if (strcmp(signature, "XenVMMXenVMM") == 0) { + /* Set debug_io_port first, so the following messages work. */ + DebugOutputPort = 0xe9; + dprintf(1, "Found Xen hypervisor signature at %x\n", base); if ((eax - base) < 2) panic("Insufficient Xen cpuid leaves. eax=%x at base %x\n", eax, base); @@ -71,6 +74,8 @@ void xen_probe(void) break; } } + if (!xen_cpuid_base) + dprintf(1, "No Xen hypervisor found.\n"); }
static int hypercall_xen_version( int cmd, void *arg)
On Wed, 2012-06-27 at 11:55 +0100, Ian Campbell wrote:
On Mon, 2012-06-25 at 18:55 -0400, Kevin O'Connor wrote:
On Wed, Jun 20, 2012 at 10:25:13AM +0100, Ian Campbell wrote:
On Wed, 2012-06-20 at 10:22 +0100, Ian Campbell wrote:
Subject: [PATCH] enable Xen support by default.
In this context I thought it would also be useful to make CONFIG_DEBUG_IO_PORT dynamic. However with the below I get lots of build errors about xen_cpuid_base not being defined. I got similar errors without the VAR16VISIBLE and GET_GLOBAL hunks. I suspect this is due to the variable being used in both 32 and 16 bit mode and my not knowing what I'm doing in that regard ;-)
xen_cpuid_base is defined in xen.c which is only compiled in 32bit mode, so you can't declare a variable as VAR16VISIBLE there.
Ah, right yes, thanks! I think I keep tripping over that...
I wonder if it is simpler to define a "u16 DebugOutputPort VAR16VISIBLE = CONFIG_DEBUG_IO_PORT" in output.c and then override it early in the xen boot sequence though.
Yes, this makes sense. I actually went one further and nuked the Kconfig option, since it's only real non-default use was Xen. So now I makde it default to 0x402 and set it to 0xe9 in the Xen case.
Forgot to say that with d51c4a0df1ae7bcb20ec3d569e409cf50f3ed760 + Xen: add definition of xen_hypercall_pa Xen: Autodetect debug I/O port at runti enable Xen support by default.
(first one sent to the list just now, other two in this thread).
SeaBIOS using a config derived from xen/tools/firmware/seabios-config (which is the 1.6.x based config we use) works for me while a config from "make defconfig" does not. I will investigate further.
Ian.
On Wed, 2012-06-27 at 12:10 +0100, Ian Campbell wrote:
SeaBIOS using a config derived from xen/tools/firmware/seabios-config (which is the 1.6.x based config we use) works for me while a config from "make defconfig" does not. I will investigate further.
CONFIG_USE_SMM is the culprit.
Xen doesn't support SMM mode for HVM guests. Seems like the best option at this point is to disable SMM if usingXen().
I'll also include this in the series I'm about to send out.
8<---------------------------
From a28c892b3c998e433de0816d5f862481690f5fe2 Mon Sep 17 00:00:00 2001
From: Ian Campbell ian.campbell@citrix.com Date: Wed, 27 Jun 2012 16:15:15 +0100 Subject: [PATCH] SMM: Disable use of SMM when running under Xen
Xen does not support SMM mode.
Signed-off-by: Ian Campbell ian.campbell@citrix.com --- src/smm.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/smm.c b/src/smm.c index 72e5e88..d0d1476 100644 --- a/src/smm.c +++ b/src/smm.c @@ -10,6 +10,7 @@ #include "config.h" // CONFIG_* #include "ioport.h" // outb #include "pci_ids.h" // PCI_VENDOR_ID_INTEL +#include "xen.h" // usingXen
ASM32FLAT( ".global smm_relocation_start\n" @@ -151,6 +152,8 @@ smm_init(void) return; if (!CONFIG_USE_SMM) return; + if (usingXen()) + return;
dprintf(3, "init smm\n"); pci_find_init_device(smm_init_tbl, NULL);
28.06.2012 14:07, Ian Campbell wrote:
On Wed, 2012-06-27 at 12:10 +0100, Ian Campbell wrote:
SeaBIOS using a config derived from xen/tools/firmware/seabios-config (which is the 1.6.x based config we use) works for me while a config from "make defconfig" does not. I will investigate further.
CONFIG_USE_SMM is the culprit.
Xen doesn't support SMM mode for HVM guests. Seems like the best option at this point is to disable SMM if usingXen().
I'll also include this in the series I'm about to send out.
So, what's the bottom line of all this discussion? Please excuse me but I don't understand what it is all about... And it looks like with at least stock 1.7 version of seabios it isn't a good idea to make universal image (with xen support included), right?
Bastian, what do you think, is it okay to build a separate image for xen for now?
Thanks,
/mjt
On Mon, 2012-07-02 at 13:47 +0400, Michael Tokarev wrote:
28.06.2012 14:07, Ian Campbell wrote:
On Wed, 2012-06-27 at 12:10 +0100, Ian Campbell wrote:
SeaBIOS using a config derived from xen/tools/firmware/seabios-config (which is the 1.6.x based config we use) works for me while a config from "make defconfig" does not. I will investigate further.
CONFIG_USE_SMM is the culprit.
Xen doesn't support SMM mode for HVM guests. Seems like the best option at this point is to disable SMM if usingXen().
I'll also include this in the series I'm about to send out.
So, what's the bottom line of all this discussion? Please excuse me but I don't understand what it is all about... And it looks like with at least stock 1.7 version of seabios it isn't a good idea to make universal image (with xen support included), right?
Right.
I sent out a series which fixed all the issues I discovered in the course of investigating this bug, including the SMM issue: http://marc.info/?l=xen-devel&m=134087840707895
Bastian, what do you think, is it okay to build a separate image for xen for now?
Backporting the above fixes if/when they are accepted would be another option.
FWIW the .config used by xen-unstable to build SeaBIOS 1.6.x is http://xenbits.xen.org/hg/xen-unstable.hg/file/4f92bdf3370c/tools/firmware/s...
Ian.
On Mon, 2012-07-02 at 11:04 +0100, Ian Campbell wrote:
On Mon, 2012-07-02 at 13:47 +0400, Michael Tokarev wrote:
28.06.2012 14:07, Ian Campbell wrote:
On Wed, 2012-06-27 at 12:10 +0100, Ian Campbell wrote:
SeaBIOS using a config derived from xen/tools/firmware/seabios-config (which is the 1.6.x based config we use) works for me while a config from "make defconfig" does not. I will investigate further.
CONFIG_USE_SMM is the culprit.
Xen doesn't support SMM mode for HVM guests. Seems like the best option at this point is to disable SMM if usingXen().
I'll also include this in the series I'm about to send out.
So, what's the bottom line of all this discussion? Please excuse me but I don't understand what it is all about... And it looks like with at least stock 1.7 version of seabios it isn't a good idea to make universal image (with xen support included), right?
Right.
I sent out a series which fixed all the issues I discovered in the course of investigating this bug, including the SMM issue: http://marc.info/?l=xen-devel&m=134087840707895
Bastian, what do you think, is it okay to build a separate image for xen for now?
Backporting the above fixes if/when they are accepted would be another option.
FYI these are now in SeaBIOS mainline... I imagine they would backport trivially but if you want me to take a look please let me knw!
FWIW the .config used by xen-unstable to build SeaBIOS 1.6.x is http://xenbits.xen.org/hg/xen-unstable.hg/file/4f92bdf3370c/tools/firmware/s...
Ian.
On Mon, Jul 02, 2012 at 01:47:42PM +0400, Michael Tokarev wrote:
So, what's the bottom line of all this discussion? Please excuse me but I don't understand what it is all about... And it looks like with at least stock 1.7 version of seabios it isn't a good idea to make universal image (with xen support included), right?
I see not why. Two things are needed to get working Xen support: - Enable XEN - Don't try to use SMM on Xen.
I have some windows test systems running with this patched seabios.
The debug i/o port is a different story and needs to be addressed with acceptance by all emulation stuff.
Bastian
On Wed, Jun 27, 2012 at 11:55:11AM +0100, Ian Campbell wrote:
Yes, this makes sense. I actually went one further and nuked the Kconfig option, since it's only real non-default use was Xen. So now I makde it default to 0x402 and set it to 0xe9 in the Xen case.
[...]
--- a/src/output.c +++ b/src/output.c @@ -23,6 +23,8 @@ struct putcinfo {
#define DEBUG_TIMEOUT 100000
+u16 DebugOutputPort VAR16VISIBLE = 0x402;
void debug_serial_setup(void) { @@ -77,7 +79,7 @@ putc_debug(struct putcinfo *action, char c) return; if (CONFIG_DEBUG_IO) // Send character to debug port.
outb(c, CONFIG_DEBUG_IO_PORT);
outb(c, DebugOutputPort);
That needs to be GET_GLOBAL(DebugOutputPort).
[...]
--- a/src/xen.c +++ b/src/xen.c @@ -64,6 +64,9 @@ void xen_probe(void) dprintf(1, "Found hypervisor signature "%s" at %x\n", signature, base); if (strcmp(signature, "XenVMMXenVMM") == 0) {
/* Set debug_io_port first, so the following messages work. */
DebugOutputPort = 0xe9;
dprintf(1, "Found Xen hypervisor signature at %x\n", base);
The seabios version should probably be printed again as well.
-Kevin
On Wed, 2012-06-27 at 08:48 -0400, Kevin O'Connor wrote:
On Wed, Jun 27, 2012 at 11:55:11AM +0100, Ian Campbell wrote:
Yes, this makes sense. I actually went one further and nuked the Kconfig option, since it's only real non-default use was Xen. So now I makde it default to 0x402 and set it to 0xe9 in the Xen case.
[...]
--- a/src/output.c +++ b/src/output.c @@ -23,6 +23,8 @@ struct putcinfo {
#define DEBUG_TIMEOUT 100000
+u16 DebugOutputPort VAR16VISIBLE = 0x402;
void debug_serial_setup(void) { @@ -77,7 +79,7 @@ putc_debug(struct putcinfo *action, char c) return; if (CONFIG_DEBUG_IO) // Send character to debug port.
outb(c, CONFIG_DEBUG_IO_PORT);
outb(c, DebugOutputPort);
That needs to be GET_GLOBAL(DebugOutputPort).
[...]
--- a/src/xen.c +++ b/src/xen.c @@ -64,6 +64,9 @@ void xen_probe(void) dprintf(1, "Found hypervisor signature "%s" at %x\n", signature, base); if (strcmp(signature, "XenVMMXenVMM") == 0) {
/* Set debug_io_port first, so the following messages work. */
DebugOutputPort = 0xe9;
dprintf(1, "Found Xen hypervisor signature at %x\n", base);
The seabios version should probably be printed again as well.
Thanks, I'll make those updates and resend a short series of all the outstanding patches from Debian bug #678042.
Ian.