If QEMU has a debug isa-debug-exit device, we can simply write to it
to exit rather than spinning after a failed hypercall.
While at it, reorder an out-of-order include.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
arch/x86/hvm/traps.c | 16 +++++++++++++++-
arch/x86/pv/traps.c | 5 +++++
common/lib.c | 2 +-
common/report.c | 8 +++++---
include/xtf/framework.h | 3 +++
5 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/arch/x86/hvm/traps.c b/arch/x86/hvm/traps.c
index ad7b8cb..b8c4d0c 100644
--- a/arch/x86/hvm/traps.c
+++ b/arch/x86/hvm/traps.c
@@ -1,5 +1,6 @@
-#include <xtf/traps.h>
+#include <xtf/hypercall.h>
#include <xtf/lib.h>
+#include <xtf/traps.h>
#include <arch/idt.h>
#include <arch/lib.h>
@@ -139,6 +140,19 @@ void arch_init_traps(void)
virt_to_gfn(__end_user_bss));
}
+void arch_shutdown(unsigned int reason)
+{
+ hypercall_shutdown(reason);
+
+ /*
+ * Not running under Xen. Attempt exit via the QEMU ISA debug exit device on
+ * its default port.
+ *
+ * QEMU's rc is (reason << 1) | 1, if "-device isa-debug-exit" is set.
+ */
+ outb(reason, 0x501);
+}
+
void __noreturn arch_crash_hard(void)
{
/*
diff --git a/arch/x86/pv/traps.c b/arch/x86/pv/traps.c
index 66ef40e..913bab2 100644
--- a/arch/x86/pv/traps.c
+++ b/arch/x86/pv/traps.c
@@ -206,6 +206,11 @@ void arch_init_traps(void)
panic("Failed to unmap page at NULL: %d\n", rc);
}
+void arch_shutdown(unsigned int reason)
+{
+ hypercall_shutdown(reason);
+}
+
void __noreturn arch_crash_hard(void)
{
/*
diff --git a/common/lib.c b/common/lib.c
index 7f1813f..f4de22e 100644
--- a/common/lib.c
+++ b/common/lib.c
@@ -25,7 +25,7 @@ void __noreturn panic(const char *fmt, ...)
printk("******************************\n");
- hypercall_shutdown(SHUTDOWN_crash);
+ arch_shutdown(SHUTDOWN_crash);
arch_crash_hard();
}
diff --git a/common/report.c b/common/report.c
index ffdf098..158876e 100644
--- a/common/report.c
+++ b/common/report.c
@@ -1,6 +1,8 @@
+#include <xtf/framework.h>
#include <xtf/lib.h>
#include <xtf/report.h>
-#include <xtf/hypercall.h>
+
+#include <xen/sched.h>
enum test_status {
STATUS_RUNNING, /**< Test not yet completed. */
@@ -124,8 +126,8 @@ bool xtf_status_reported(void)
void xtf_exit(void)
{
xtf_report_status();
- hypercall_shutdown(SHUTDOWN_poweroff);
- panic("xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned\n");
+ arch_shutdown(SHUTDOWN_poweroff);
+ panic("xtf_exit(): arch_shutdown(SHUTDOWN_poweroff) returned\n");
}
/*
diff --git a/include/xtf/framework.h b/include/xtf/framework.h
index 95de195..e852882 100644
--- a/include/xtf/framework.h
+++ b/include/xtf/framework.h
@@ -16,6 +16,9 @@ void arch_setup(void);
/* Set up test-specific configuration. */
void test_setup(void);
+/* Stop the machine. See SHUTDOWN_poweroff et al for reasons */
+void arch_shutdown(unsigned int reason);
+
/*
* In the case that normal shutdown actions have failed, contain execution as
* best as possible.
--
2.43.0
On Thu, Oct 02, 2025 at 03:55:34PM +0200, Alejandro Vallejo wrote: > If QEMU has a debug isa-debug-exit device, we can simply write to it > to exit rather than spinning after a failed hypercall. > > While at it, reorder an out-of-order include. > > Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> > --- > arch/x86/hvm/traps.c | 16 +++++++++++++++- > arch/x86/pv/traps.c | 5 +++++ > common/lib.c | 2 +- > common/report.c | 8 +++++--- > include/xtf/framework.h | 3 +++ > 5 files changed, 29 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/hvm/traps.c b/arch/x86/hvm/traps.c > index ad7b8cb..b8c4d0c 100644 > --- a/arch/x86/hvm/traps.c > +++ b/arch/x86/hvm/traps.c > @@ -1,5 +1,6 @@ > -#include <xtf/traps.h> > +#include <xtf/hypercall.h> > #include <xtf/lib.h> > +#include <xtf/traps.h> > > #include <arch/idt.h> > #include <arch/lib.h> > @@ -139,6 +140,19 @@ void arch_init_traps(void) > virt_to_gfn(__end_user_bss)); > } > > +void arch_shutdown(unsigned int reason) > +{ > + hypercall_shutdown(reason); This relies on the hypercall page being poised with `ret`, which is IMO fragile. I would rather have it poisoned with `int3` and prevent such stray accesses in the first place. > + > + /* > + * Not running under Xen. Attempt exit via the QEMU ISA debug exit device on > + * its default port. > + * > + * QEMU's rc is (reason << 1) | 1, if "-device isa-debug-exit" is set. > + */ > + outb(reason, 0x501); That's kind of weird? So even if we pass reason == 0, the exit code from QEMU will be 1 (and error)? Isn't there anyway to signal a clean shutdown, and hence QEMU exit code being 0? Thanks, Roger.
On Thu Oct 2, 2025 at 4:22 PM CEST, Roger Pau Monné wrote: > On Thu, Oct 02, 2025 at 03:55:34PM +0200, Alejandro Vallejo wrote: >> If QEMU has a debug isa-debug-exit device, we can simply write to it >> to exit rather than spinning after a failed hypercall. >> >> While at it, reorder an out-of-order include. >> >> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> >> --- >> arch/x86/hvm/traps.c | 16 +++++++++++++++- >> arch/x86/pv/traps.c | 5 +++++ >> common/lib.c | 2 +- >> common/report.c | 8 +++++--- >> include/xtf/framework.h | 3 +++ >> 5 files changed, 29 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/hvm/traps.c b/arch/x86/hvm/traps.c >> index ad7b8cb..b8c4d0c 100644 >> --- a/arch/x86/hvm/traps.c >> +++ b/arch/x86/hvm/traps.c >> @@ -1,5 +1,6 @@ >> -#include <xtf/traps.h> >> +#include <xtf/hypercall.h> >> #include <xtf/lib.h> >> +#include <xtf/traps.h> >> >> #include <arch/idt.h> >> #include <arch/lib.h> >> @@ -139,6 +140,19 @@ void arch_init_traps(void) >> virt_to_gfn(__end_user_bss)); >> } >> >> +void arch_shutdown(unsigned int reason) >> +{ >> + hypercall_shutdown(reason); > > This relies on the hypercall page being poised with `ret`, which is > IMO fragile. I would rather have it poisoned with `int3` and prevent > such stray accesses in the first place. I dont' mind caching Xen presence somewhere, but that involves some code motion from setup.c, which I wanted to avoid. At the core I just want to speed up testmaking by doing it from WSL rather than from a Xen host. > >> + >> + /* >> + * Not running under Xen. Attempt exit via the QEMU ISA debug exit device on >> + * its default port. >> + * >> + * QEMU's rc is (reason << 1) | 1, if "-device isa-debug-exit" is set. >> + */ >> + outb(reason, 0x501); > > That's kind of weird? So even if we pass reason == 0, the exit code > from QEMU will be 1 (and error)? > > Isn't there anyway to signal a clean shutdown, and hence QEMU exit > code being 0? Nope. It's hardcoded in QEMU itself. reason=0 => rc=1 reason=1 => rc=3 reason=2 => rc=5 ... and so on. I have something like this in my harness to avoid surprises: set +e qemu-system-x86_64 <...> RC="$?" printf "\n[QEMU exit] rc=${RC} reason=$(($RC / 2))\n" On other test harness I use for personal projects I take the convention that rc = 1 means success and anything else means failure, but that needs changes to the runner to integrate the assumptions somewhere, I don't think hardcoding my conventions is sensible. Cheers, Alejandro
On Thu, Oct 02, 2025 at 04:48:38PM +0200, Alejandro Vallejo wrote: > On Thu Oct 2, 2025 at 4:22 PM CEST, Roger Pau Monné wrote: > > On Thu, Oct 02, 2025 at 03:55:34PM +0200, Alejandro Vallejo wrote: > >> If QEMU has a debug isa-debug-exit device, we can simply write to it > >> to exit rather than spinning after a failed hypercall. > >> > >> While at it, reorder an out-of-order include. > >> > >> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> > >> --- > >> arch/x86/hvm/traps.c | 16 +++++++++++++++- > >> arch/x86/pv/traps.c | 5 +++++ > >> common/lib.c | 2 +- > >> common/report.c | 8 +++++--- > >> include/xtf/framework.h | 3 +++ > >> 5 files changed, 29 insertions(+), 5 deletions(-) > >> > >> diff --git a/arch/x86/hvm/traps.c b/arch/x86/hvm/traps.c > >> index ad7b8cb..b8c4d0c 100644 > >> --- a/arch/x86/hvm/traps.c > >> +++ b/arch/x86/hvm/traps.c > >> @@ -1,5 +1,6 @@ > >> -#include <xtf/traps.h> > >> +#include <xtf/hypercall.h> > >> #include <xtf/lib.h> > >> +#include <xtf/traps.h> > >> > >> #include <arch/idt.h> > >> #include <arch/lib.h> > >> @@ -139,6 +140,19 @@ void arch_init_traps(void) > >> virt_to_gfn(__end_user_bss)); > >> } > >> > >> +void arch_shutdown(unsigned int reason) > >> +{ > >> + hypercall_shutdown(reason); > > > > This relies on the hypercall page being poised with `ret`, which is > > IMO fragile. I would rather have it poisoned with `int3` and prevent > > such stray accesses in the first place. > > I dont' mind caching Xen presence somewhere, but that involves some code motion > from setup.c, which I wanted to avoid. I think it's very likely that at some point we will need to cache this? enum { NATIVE, XEN, QEMU, ... } hypervisor_env; Or similar. > At the core I just want to speed up testmaking by doing it from WSL rather than > from a Xen host. Right. I was pondering whether we want a QEMU target, but realistically QEMU should be able to run all the hvm* variants. > > > >> + > >> + /* > >> + * Not running under Xen. Attempt exit via the QEMU ISA debug exit device on > >> + * its default port. > >> + * > >> + * QEMU's rc is (reason << 1) | 1, if "-device isa-debug-exit" is set. > >> + */ > >> + outb(reason, 0x501); > > > > That's kind of weird? So even if we pass reason == 0, the exit code > > from QEMU will be 1 (and error)? > > > > Isn't there anyway to signal a clean shutdown, and hence QEMU exit > > code being 0? > > Nope. It's hardcoded in QEMU itself. > > reason=0 => rc=1 > reason=1 => rc=3 > reason=2 => rc=5 > > ... and so on. Hm, OK, I think it's lacking there's no way to signal a clean exit, but I guess QEMU had a reason for this. > I have something like this in my harness to avoid surprises: > > set +e > qemu-system-x86_64 <...> > RC="$?" > printf "\n[QEMU exit] rc=${RC} reason=$(($RC / 2))\n" > > On other test harness I use for personal projects I take the convention that > rc = 1 means success and anything else means failure, but that needs changes > to the runner to integrate the assumptions somewhere, I don't think hardcoding > my conventions is sensible. I see, I find it kind of lacking from QEMU, but never mind, not something we can change. Thanks, Roger.
On Thu Oct 2, 2025 at 5:37 PM CEST, Roger Pau Monné wrote: > On Thu, Oct 02, 2025 at 04:48:38PM +0200, Alejandro Vallejo wrote: >> On Thu Oct 2, 2025 at 4:22 PM CEST, Roger Pau Monné wrote: >> > On Thu, Oct 02, 2025 at 03:55:34PM +0200, Alejandro Vallejo wrote: >> >> If QEMU has a debug isa-debug-exit device, we can simply write to it >> >> to exit rather than spinning after a failed hypercall. >> >> >> >> While at it, reorder an out-of-order include. >> >> >> >> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> >> >> --- >> >> arch/x86/hvm/traps.c | 16 +++++++++++++++- >> >> arch/x86/pv/traps.c | 5 +++++ >> >> common/lib.c | 2 +- >> >> common/report.c | 8 +++++--- >> >> include/xtf/framework.h | 3 +++ >> >> 5 files changed, 29 insertions(+), 5 deletions(-) >> >> >> >> diff --git a/arch/x86/hvm/traps.c b/arch/x86/hvm/traps.c >> >> index ad7b8cb..b8c4d0c 100644 >> >> --- a/arch/x86/hvm/traps.c >> >> +++ b/arch/x86/hvm/traps.c >> >> @@ -1,5 +1,6 @@ >> >> -#include <xtf/traps.h> >> >> +#include <xtf/hypercall.h> >> >> #include <xtf/lib.h> >> >> +#include <xtf/traps.h> >> >> >> >> #include <arch/idt.h> >> >> #include <arch/lib.h> >> >> @@ -139,6 +140,19 @@ void arch_init_traps(void) >> >> virt_to_gfn(__end_user_bss)); >> >> } >> >> >> >> +void arch_shutdown(unsigned int reason) >> >> +{ >> >> + hypercall_shutdown(reason); >> > >> > This relies on the hypercall page being poised with `ret`, which is >> > IMO fragile. I would rather have it poisoned with `int3` and prevent >> > such stray accesses in the first place. >> >> I dont' mind caching Xen presence somewhere, but that involves some code motion >> from setup.c, which I wanted to avoid. > > I think it's very likely that at some point we will need to cache this? > > enum { > NATIVE, > XEN, > QEMU, > ... > } hypervisor_env; > > Or similar. Maybe NATIVE, XEN_VIRT and NON_XEN_VIRT? I see no reason to distinguish between TCG, KVM and any other accelerator; and QEMU is imprecise because we use for HVM. You could imagine chainloading XTF from GRUB to test the HVM env. > >> At the core I just want to speed up testmaking by doing it from WSL rather than >> from a Xen host. > > Right. I was pondering whether we want a QEMU target, but > realistically QEMU should be able to run all the hvm* variants. > >> > >> >> + >> >> + /* >> >> + * Not running under Xen. Attempt exit via the QEMU ISA debug exit device on >> >> + * its default port. >> >> + * >> >> + * QEMU's rc is (reason << 1) | 1, if "-device isa-debug-exit" is set. >> >> + */ >> >> + outb(reason, 0x501); >> > >> > That's kind of weird? So even if we pass reason == 0, the exit code >> > from QEMU will be 1 (and error)? >> > >> > Isn't there anyway to signal a clean shutdown, and hence QEMU exit >> > code being 0? >> >> Nope. It's hardcoded in QEMU itself. >> >> reason=0 => rc=1 >> reason=1 => rc=3 >> reason=2 => rc=5 >> >> ... and so on. > > Hm, OK, I think it's lacking there's no way to signal a clean exit, > but I guess QEMU had a reason for this. Seems pretty obvious it was intentional. As to what the intention was, your guess is as good as mine. Cheers, Alejandro
On Thu, Oct 02, 2025 at 07:48:28PM +0200, Alejandro Vallejo wrote: > On Thu Oct 2, 2025 at 5:37 PM CEST, Roger Pau Monné wrote: > > On Thu, Oct 02, 2025 at 04:48:38PM +0200, Alejandro Vallejo wrote: > >> On Thu Oct 2, 2025 at 4:22 PM CEST, Roger Pau Monné wrote: > >> > On Thu, Oct 02, 2025 at 03:55:34PM +0200, Alejandro Vallejo wrote: > >> >> If QEMU has a debug isa-debug-exit device, we can simply write to it > >> >> to exit rather than spinning after a failed hypercall. > >> >> > >> >> While at it, reorder an out-of-order include. > >> >> > >> >> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> > >> >> --- > >> >> arch/x86/hvm/traps.c | 16 +++++++++++++++- > >> >> arch/x86/pv/traps.c | 5 +++++ > >> >> common/lib.c | 2 +- > >> >> common/report.c | 8 +++++--- > >> >> include/xtf/framework.h | 3 +++ > >> >> 5 files changed, 29 insertions(+), 5 deletions(-) > >> >> > >> >> diff --git a/arch/x86/hvm/traps.c b/arch/x86/hvm/traps.c > >> >> index ad7b8cb..b8c4d0c 100644 > >> >> --- a/arch/x86/hvm/traps.c > >> >> +++ b/arch/x86/hvm/traps.c > >> >> @@ -1,5 +1,6 @@ > >> >> -#include <xtf/traps.h> > >> >> +#include <xtf/hypercall.h> > >> >> #include <xtf/lib.h> > >> >> +#include <xtf/traps.h> > >> >> > >> >> #include <arch/idt.h> > >> >> #include <arch/lib.h> > >> >> @@ -139,6 +140,19 @@ void arch_init_traps(void) > >> >> virt_to_gfn(__end_user_bss)); > >> >> } > >> >> > >> >> +void arch_shutdown(unsigned int reason) > >> >> +{ > >> >> + hypercall_shutdown(reason); > >> > > >> > This relies on the hypercall page being poised with `ret`, which is > >> > IMO fragile. I would rather have it poisoned with `int3` and prevent > >> > such stray accesses in the first place. > >> > >> I dont' mind caching Xen presence somewhere, but that involves some code motion > >> from setup.c, which I wanted to avoid. > > > > I think it's very likely that at some point we will need to cache this? > > > > enum { > > NATIVE, > > XEN, > > QEMU, > > ... > > } hypervisor_env; > > > > Or similar. > > Maybe NATIVE, XEN_VIRT and NON_XEN_VIRT? I see no reason to distinguish between > TCG, KVM and any other accelerator; and QEMU is imprecise because we use for > HVM. You could imagine chainloading XTF from GRUB to test the HVM env. Maybe not for XTF. IIRC KVM also offers some PV interfaces (like the PV timer) that native QEMU doesn't. Rather than having an exclusive hypervisor mode, we could signal what interfaces are available. For example Xen (and I bet KVM too) can expose native interfaces plus viridian extensions, in which case we might want to detect both if present. That would require using a separate boolean for each extra interface. IOW: bool xen_hypercall; bool viridian_foo; bool qemu_debug; ... (Possibly not the best naming) BTW, is it possible for a guest to discover whether the "isa-debug-exit" functionality is present? Sorry, I'm possibly derailing this patch series. Regards, Roger.
On Fri Oct 3, 2025 at 10:06 AM CEST, Roger Pau Monné wrote: > On Thu, Oct 02, 2025 at 07:48:28PM +0200, Alejandro Vallejo wrote: >> On Thu Oct 2, 2025 at 5:37 PM CEST, Roger Pau Monné wrote: >> > On Thu, Oct 02, 2025 at 04:48:38PM +0200, Alejandro Vallejo wrote: >> >> On Thu Oct 2, 2025 at 4:22 PM CEST, Roger Pau Monné wrote: >> >> > On Thu, Oct 02, 2025 at 03:55:34PM +0200, Alejandro Vallejo wrote: >> >> >> If QEMU has a debug isa-debug-exit device, we can simply write to it >> >> >> to exit rather than spinning after a failed hypercall. >> >> >> >> >> >> While at it, reorder an out-of-order include. >> >> >> >> >> >> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> >> >> >> --- >> >> >> arch/x86/hvm/traps.c | 16 +++++++++++++++- >> >> >> arch/x86/pv/traps.c | 5 +++++ >> >> >> common/lib.c | 2 +- >> >> >> common/report.c | 8 +++++--- >> >> >> include/xtf/framework.h | 3 +++ >> >> >> 5 files changed, 29 insertions(+), 5 deletions(-) >> >> >> >> >> >> diff --git a/arch/x86/hvm/traps.c b/arch/x86/hvm/traps.c >> >> >> index ad7b8cb..b8c4d0c 100644 >> >> >> --- a/arch/x86/hvm/traps.c >> >> >> +++ b/arch/x86/hvm/traps.c >> >> >> @@ -1,5 +1,6 @@ >> >> >> -#include <xtf/traps.h> >> >> >> +#include <xtf/hypercall.h> >> >> >> #include <xtf/lib.h> >> >> >> +#include <xtf/traps.h> >> >> >> >> >> >> #include <arch/idt.h> >> >> >> #include <arch/lib.h> >> >> >> @@ -139,6 +140,19 @@ void arch_init_traps(void) >> >> >> virt_to_gfn(__end_user_bss)); >> >> >> } >> >> >> >> >> >> +void arch_shutdown(unsigned int reason) >> >> >> +{ >> >> >> + hypercall_shutdown(reason); >> >> > >> >> > This relies on the hypercall page being poised with `ret`, which is >> >> > IMO fragile. I would rather have it poisoned with `int3` and prevent >> >> > such stray accesses in the first place. >> >> >> >> I dont' mind caching Xen presence somewhere, but that involves some code motion >> >> from setup.c, which I wanted to avoid. >> > >> > I think it's very likely that at some point we will need to cache this? >> > >> > enum { >> > NATIVE, >> > XEN, >> > QEMU, >> > ... >> > } hypervisor_env; >> > >> > Or similar. >> >> Maybe NATIVE, XEN_VIRT and NON_XEN_VIRT? I see no reason to distinguish between >> TCG, KVM and any other accelerator; and QEMU is imprecise because we use for >> HVM. You could imagine chainloading XTF from GRUB to test the HVM env. > > Maybe not for XTF. IIRC KVM also offers some PV interfaces (like the > PV timer) that native QEMU doesn't. Sure, but we don't want to test KVM PV. It _could_ be used for it, but KVM has its own unit testing facilities already. https://gitlab.com/kvm-unit-tests/kvm-unit-tests.git > > Rather than having an exclusive hypervisor mode, we could signal what > interfaces are available. For example Xen (and I bet KVM too) can > expose native interfaces plus viridian extensions, in which case we > might want to detect both if present. That would require using a > separate boolean for each extra interface. IOW: > > bool xen_hypercall; > bool viridian_foo; > bool qemu_debug; > ... > > (Possibly not the best naming) I'm of the opinion of not adding things not strictly required. > > BTW, is it possible for a guest to discover whether the > "isa-debug-exit" functionality is present? Besides ensuring a read gets zero, no. From the QEMU sources: static uint64_t debug_exit_read(void *opaque, hwaddr addr, unsigned size) { return 0; } static void debug_exit_write(void *opaque, hwaddr addr, uint64_t val, unsigned width) { qemu_system_shutdown_request_with_code(SHUTDOWN_CAUSE_GUEST_SHUTDOWN, (val << 1) | 1); } I didn't see any signaling anywhere in CPUID or elsewhere. Though I admit it was years ago that I last checked, this isn't the sort of feature that changes very often. > > Sorry, I'm possibly derailing this patch series. Can only mean you find it interesting. That's always good :) But to concretise actions, I think I'll keep it simple for the time being and add a single `cpu_has_xen` global boolean; then place the shutdown hypercall before the QEMU exit device write, gated by cpu_has_xen. That prevents making a hypercall when the "wrong" hypervisor is present (or none). Cheers, Alejandro
© 2016 - 2025 Red Hat, Inc.