Mailing List Archive: [PATCH] tools: hvmloader: attempt to SHUTDOWN

[PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG

Apr 18, 2011, 9:21 AM

Post #1 of 5 (182 views)

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1303143704 -3600
# Node ID 05abea47f4dc670974cd513a0e74db54d22eacc9
# Parent 052ffd72382924eee479ca479fd1574750a8adf7
tools: hvmloader: attempt to SHUTDOWN_crash on BUG

If we have got as far as having enabled hypercalls then this signals to the
toolstack that something went wrong. Otherwise they tend to assume the guest
has either shutdown or rebooted which can lead to simply trying again
repeatedly.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 052ffd723829 -r 05abea47f4dc tools/firmware/hvmloader/hvmloader.c
--- a/tools/firmware/hvmloader/hvmloader.c Mon Apr 18 14:53:03 2011 +0100
+++ b/tools/firmware/hvmloader/hvmloader.c Mon Apr 18 17:21:44 2011 +0100
@@ -114,6 +114,8 @@ unsigned long pci_mem_end = PCI_MEM_END;

enum virtual_vga virtual_vga = VGA_none;

+int hypercalls_available = 0;
+
static void init_hypercalls(void)
{
uint32_t eax, ebx, ecx, edx;
@@ -146,6 +148,7 @@ static void init_hypercalls(void)
cpuid(base + 1, &eax, &ebx, &ecx, &edx);
hypercall_xen_version(XENVER_extraversion, extraversion);
printf("Detected Xen v%u.%u%s\n", eax >> 16, eax & 0xffff, extraversion);
+ hypercalls_available = 1;
}

/*
diff -r 052ffd723829 -r 05abea47f4dc tools/firmware/hvmloader/hypercall.h
--- a/tools/firmware/hvmloader/hypercall.h Mon Apr 18 14:53:03 2011 +0100
+++ b/tools/firmware/hvmloader/hypercall.h Mon Apr 18 17:21:44 2011 +0100
@@ -35,6 +35,9 @@
#include <xen/xen.h>
#include "config.h"

+/* Have hypercalls been successfully enabled */
+extern int hypercalls_available;
+
/*
* NB. Hypercall address needs to be relative to a linkage symbol for
* some version of ld to relocate the relative calls properly.
diff -r 052ffd723829 -r 05abea47f4dc tools/firmware/hvmloader/util.c
--- a/tools/firmware/hvmloader/util.c Mon Apr 18 14:53:03 2011 +0100
+++ b/tools/firmware/hvmloader/util.c Mon Apr 18 17:21:44 2011 +0100
@@ -24,6 +24,7 @@
#include <stdint.h>
#include <xen/xen.h>
#include <xen/memory.h>
+#include <xen/sched.h>

void wrmsr(uint32_t idx, uint64_t v)
{
@@ -547,7 +548,15 @@ void __assert_failed(char *assertion, ch

void __bug(char *file, int line)
{
+ struct sched_shutdown shutdown = {
+ .reason = SHUTDOWN_crash,
+ };
+
printf("HVMLoader bug at %s:%d\n", file, line);
+ if (hypercalls_available) {
+ printf("SCHEDOP_shutdown: reason=SHUTDOWN_crash\n");
+ hypercall_sched_op(SCHEDOP_shutdown, &shutdown);
+ }
for ( ; ; )
asm volatile ( "ud2" );
}

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG [ In reply to ]

Ian.Jackson at eu

Apr 20, 2011, 10:24 AM

Post #2 of 5 (170 views)

Permalink

Ian Campbell writes ("[Xen-devel] [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG"):
> tools: hvmloader: attempt to SHUTDOWN_crash on BUG
>
> If we have got as far as having enabled hypercalls then this signals to the
> toolstack that something went wrong. Otherwise they tend to assume the guest
> has either shutdown or rebooted which can lead to simply trying again
> repeatedly.

Surely in principle this is wrong. Anything other than a deliberate
decision by hvmloader to exit cleanly (how would that ever happen?)
should be treated as a crash.

Perhaps the toolstack for an hvm domain should set the shutdown reason
to "crashed" before unpausing ? It looks like this is possible with
an appropriate domctl ...

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG [ In reply to ]

Ian.Campbell at eu

Apr 20, 2011, 11:17 AM

Post #3 of 5 (179 views)

Permalink

Keir already applied this, but...

On Wed, 2011-04-20 at 18:24 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG"):
> > tools: hvmloader: attempt to SHUTDOWN_crash on BUG
> >
> > If we have got as far as having enabled hypercalls then this signals to the
> > toolstack that something went wrong. Otherwise they tend to assume the guest
> > has either shutdown or rebooted which can lead to simply trying again
> > repeatedly.
>
> Surely in principle this is wrong. Anything other than a deliberate
> decision by hvmloader to exit cleanly (how would that ever happen?)
> should be treated as a crash.

The toolstack can't tell the difference between hvmloader and the
subsequent bootloader then OS doing stuff and they may not be PV'd up.

In this case, the triple fault resulting from an hvmloader crash could
also be a legitimate attempt by a guest to reboot itself, since it's one
of the ways to reset the processor.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG [ In reply to ]

Ian.Jackson at eu

Apr 20, 2011, 11:35 AM

Post #4 of 5 (181 views)

Permalink

Ian Campbell writes ("Re: [Xen-devel] [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG"):
> The toolstack can't tell the difference between hvmloader and the
> subsequent bootloader then OS doing stuff and they may not be PV'd up.

Indeed. But hvmloader can put the reason code back just before it
launches the guest bootloader (or perhaps just before it enters the
BIOS).

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG [ In reply to ]

keir.xen at gmail

Apr 20, 2011, 12:18 PM

Post #5 of 5 (174 views)

Permalink

On 20/04/2011 19:35, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:

> Ian Campbell writes ("Re: [Xen-devel] [PATCH] tools: hvmloader: attempt to
> SHUTDOWN_crash on BUG"):
>> The toolstack can't tell the difference between hvmloader and the
>> subsequent bootloader then OS doing stuff and they may not be PV'd up.
>
> Indeed. But hvmloader can put the reason code back just before it
> launches the guest bootloader (or perhaps just before it enters the
> BIOS).

Currently the reason code cannot be changed after it has been latched by the
hypervisor. Secondly, even if it could be changed, we wouldn't want to
change it to a new value after hvmloader has run; we'd want to reset it to
no-code. That would be cleanest with a new sched_op. However it is arguable
whether this is worthwhile, since I've never heard of hvmloader crashing
except on an explicit bug/assert statement (and even those are rare, and
during development work). So it's kind of a non problem.

Also I already applied Ian's patch. :-) (I still maintain tools/firmware/,
at least until seabios goes in).

-- Keir

> Ian.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel