[XTF PATCH v2 0/2] Remove Xen as a hard requirement to run XTF

[XTF PATCH v2 0/2] Remove Xen as a hard requirement to run XTF

[Alejandro Vallejo]

Hi,

I've included logic to exit QEMU using the ISA debug exit device. See patch 2.

Cheers,
Alejandro

Alejandro Vallejo (2):
  x86: Remove Xen as a hard requirement to run XTF.
  x86: Allow exiting QEMU in TCG/QEMU

 arch/x86/hvm/traps.c    | 16 ++++++++++++-
 arch/x86/pv/traps.c     |  5 ++++
 arch/x86/setup.c        | 53 +++++++++++++++++++++++++++++------------
 common/lib.c            |  2 +-
 common/report.c         |  8 ++++---
 include/xtf/framework.h |  3 +++
 6 files changed, 67 insertions(+), 20 deletions(-)


base-commit: 11b552a416d1164c63be4da670f16cf8a5a9044d
-- 
2.43.0

[XTF PATCH v2 1/2] x86: Remove Xen as a hard requirement to run XTF.

[Alejandro Vallejo]

If Xen isn't detected on CPUID, then:

 * Skip setting up Xenbus/PV-console/shared_info/hypercalls/qemu-debug.
 * Register COM1 as an output callback.
 * Attempt a QEMU exit via the ISA debug exit device

This patch enables running XTF on QEMU-TCG/KVM out of the box. And a
minor tweaks to set up baud rate make it work on real hardware too.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
I can't really integrate the checks on find_xen_leaves() as I intended because
many functions just don't need it (prepping xenbus, for instance), so I've left
things as they were in v1. It does work this way.

v2:
  * Use new now available rep_outsb() helper for the com1 writer
  * Make find_xen_leaves() fallible, and add an extra boolean to preserve
    the panic behaviour where it still matters.
---
 arch/x86/setup.c | 53 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/arch/x86/setup.c b/arch/x86/setup.c
index ba6f9c3..be42c84 100644
--- a/arch/x86/setup.c
+++ b/arch/x86/setup.c
@@ -102,12 +102,13 @@ static void collect_cpuid(cpuid_count_fn_t cpuid_fn)
  * Find the Xen CPUID leaves.  They may be at 0x4000_0000, or at 0x4000_0100
  * if Xen is e.g. providing a viridian interface to the guest too.
  */
-static unsigned int find_xen_leaves(void)
+static unsigned int find_xen_leaves(bool assert_found)
 {
+#define XEN_LEAVES_NOT_FOUND (-1U)
     static unsigned int leaves;
 
     if ( leaves )
-        return leaves;
+        goto out;
 
     for ( unsigned int l = XEN_CPUID_FIRST_LEAF;
           l < XEN_CPUID_FIRST_LEAF + 0x10000; l += 0x100 )
@@ -126,7 +127,13 @@ static unsigned int find_xen_leaves(void)
         }
     }
 
-    panic("Unable to locate Xen CPUID leaves\n");
+    leaves = XEN_LEAVES_NOT_FOUND;
+
+out:
+    if ( assert_found && (leaves == XEN_LEAVES_NOT_FOUND) )
+        panic("Unable to locate Xen CPUID leaves\n");
+
+    return leaves;
 }
 
 /*
@@ -140,7 +147,7 @@ static void init_hypercalls(void)
     if ( IS_DEFINED(CONFIG_HVM) )
     {
         uint32_t eax, ebx, ecx, edx;
-        unsigned int base = find_xen_leaves();
+        unsigned int base = find_xen_leaves(true);
 
         cpuid(base + 2, &eax, &ebx, &ecx, &edx);
         wrmsr(ebx, _u(hypercall_page));
@@ -248,6 +255,11 @@ static void qemu_console_write(const char *buf, size_t len)
     rep_outsb(buf, len, 0x12);
 }
 
+static void com1_console_write(const char *buf, size_t len)
+{
+    rep_outsb(buf, len, 0x3f8);
+}
+
 static void xen_console_write(const char *buf, size_t len)
 {
     hypercall_console_write(buf, len);
@@ -255,10 +267,18 @@ static void xen_console_write(const char *buf, size_t len)
 
 void arch_setup(void)
 {
-    if ( IS_DEFINED(CONFIG_HVM) && !pvh_start_info )
-        register_console_callback(qemu_console_write);
+    bool has_xen_hypervisor =
+        find_xen_leaves(IS_DEFINED(CONFIG_PV)) != XEN_LEAVES_NOT_FOUND;
 
-    register_console_callback(xen_console_write);
+    if ( has_xen_hypervisor )
+    {
+        if ( IS_DEFINED(CONFIG_HVM) && !pvh_start_info )
+            register_console_callback(qemu_console_write);
+
+        register_console_callback(xen_console_write);
+    }
+    else
+        register_console_callback(com1_console_write);
 
     collect_cpuid(IS_DEFINED(CONFIG_PV) ? pv_cpuid_count : cpuid_count);
 
@@ -266,15 +286,18 @@ void arch_setup(void)
 
     arch_init_traps();
 
-    init_hypercalls();
-
-    if ( !is_initdomain() )
+    if ( has_xen_hypervisor )
     {
-        setup_pv_console();
-        setup_xenbus();
-    }
+        init_hypercalls();
 
-    map_shared_info();
+        if ( !is_initdomain() )
+        {
+            setup_pv_console();
+            setup_xenbus();
+        }
+
+        map_shared_info();
+    }
 }
 
 int arch_get_domid(void)
@@ -282,7 +305,7 @@ int arch_get_domid(void)
     if ( IS_DEFINED(CONFIG_HVM) )
     {
         uint32_t eax, ebx, ecx, edx;
-        unsigned int base = find_xen_leaves();
+        unsigned int base = find_xen_leaves(true);
 
         cpuid_count(base + 4, 0, &eax, &ebx, &ecx, &edx);
 
-- 
2.43.0

[XTF PATCH v2 2/2] x86: Allow exiting QEMU in TCG/QEMU

[Alejandro Vallejo]

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

Re: [XTF PATCH v2 2/2] x86: Allow exiting QEMU in TCG/QEMU

[Roger Pau Monné]

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.

Re: [XTF PATCH v2 2/2] x86: Allow exiting QEMU in TCG/QEMU

[Alejandro Vallejo]

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

Re: [XTF PATCH v2 2/2] x86: Allow exiting QEMU in TCG/QEMU

[Roger Pau Monné]

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.

Re: [XTF PATCH v2 2/2] x86: Allow exiting QEMU in TCG/QEMU

[Alejandro Vallejo]

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

Re: [XTF PATCH v2 2/2] x86: Allow exiting QEMU in TCG/QEMU

[Roger Pau Monné]

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.

Re: [XTF PATCH v2 2/2] x86: Allow exiting QEMU in TCG/QEMU

[Alejandro Vallejo]

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

Re: [XTF PATCH v2 0/2] Remove Xen as a hard requirement to run XTF

[Alejandro Vallejo]

On Thu Oct 2, 2025 at 3:55 PM CEST, Alejandro Vallejo wrote:
> Hi,
>
> I've included logic to exit QEMU using the ISA debug exit device. See patch 2.
>
> Cheers,
> Alejandro
>
> Alejandro Vallejo (2):
>   x86: Remove Xen as a hard requirement to run XTF.
>   x86: Allow exiting QEMU in TCG/QEMU
>
>  arch/x86/hvm/traps.c    | 16 ++++++++++++-
>  arch/x86/pv/traps.c     |  5 ++++
>  arch/x86/setup.c        | 53 +++++++++++++++++++++++++++++------------
>  common/lib.c            |  2 +-
>  common/report.c         |  8 ++++---
>  include/xtf/framework.h |  3 +++
>  6 files changed, 67 insertions(+), 20 deletions(-)
>
>
> base-commit: 11b552a416d1164c63be4da670f16cf8a5a9044d

Ping