Ian Campbell
2011-Apr-18 16:21 UTC
[Xen-devel] [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG
# 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
Ian Jackson
2011-Apr-20 17:24 UTC
Re: [Xen-devel] [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG
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
Ian Campbell
2011-Apr-20 18:17 UTC
Re: [Xen-devel] [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG
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
Ian Jackson
2011-Apr-20 18:35 UTC
Re: [Xen-devel] [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG
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
Keir Fraser
2011-Apr-20 19:18 UTC
Re: [Xen-devel] [PATCH] tools: hvmloader: attempt to SHUTDOWN_crash on BUG
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