[XTF PATCH] x86: Remove Xen as a hard requirement to run XTF.

Alejandro Vallejo posted 1 patch 4 days, 11 hours ago
Failed in applying to current master (apply log)
arch/x86/setup.c | 68 +++++++++++++++++++++++++++++++++++++++---------
1 file changed, 55 insertions(+), 13 deletions(-)
[XTF PATCH] x86: Remove Xen as a hard requirement to run XTF.
Posted by Alejandro Vallejo 4 days, 11 hours ago
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.

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 tested PV and HVM still run fine under Xen, and I did account for
viridian-enlightened guests, though I didn't test them.

---
 arch/x86/setup.c | 68 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 13 deletions(-)

diff --git a/arch/x86/setup.c b/arch/x86/setup.c
index 2ac212e..6172c7e 100644
--- a/arch/x86/setup.c
+++ b/arch/x86/setup.c
@@ -31,6 +31,8 @@ const char environment_description[] = ENVIRONMENT_DESCRIPTION;
 
 shared_info_t shared_info __page_aligned_bss;
 
+static bool has_xen_hypervisor;
+
 static void collect_cpuid(cpuid_count_fn_t cpuid_fn)
 {
     unsigned int tmp, eax, ebx, ecx, edx, addr = 0;
@@ -243,11 +245,19 @@ static void map_shared_info(void)
         panic("Failed to map shared_info: %d\n", rc);
 }
 
+static void pio_write(uint16_t port, const char *buf, size_t len)
+{
+    asm volatile("rep; outsb" : "+S" (buf), "+c" (len) : "d" (port));
+}
+
 static void qemu_console_write(const char *buf, size_t len)
 {
-    asm volatile("rep outsb"
-                 : "+S" (buf), "+c" (len)
-                 : "d" (0x12));
+    pio_write(0x12, buf, len);
+}
+
+static void com1_console_write(const char *buf, size_t len)
+{
+    pio_write(0x3f8, buf, len);
 }
 
 static void xen_console_write(const char *buf, size_t len)
@@ -255,12 +265,41 @@ static void xen_console_write(const char *buf, size_t len)
     hypercall_console_write(buf, len);
 }
 
+static bool detect_xen_runtime(void)
+{
+    uint32_t eax, ebx, ecx, edx;
+
+    /* PV tests always run under Xen */
+    if ( IS_DEFINED(CONFIG_PV) )
+        return true;
+
+    /* HVM tests may additionally run on non-Xen hypervisors or baremetal */
+    cpuid_count(0x40000000, 0, &eax, &ebx, &ecx, &edx);
+    if (  ebx == XEN_CPUID_SIGNATURE_EBX &&
+          ecx == XEN_CPUID_SIGNATURE_ECX &&
+          edx == XEN_CPUID_SIGNATURE_EDX )
+        return true;
+
+    /* Viridian guests have the Xen leaves higher up, so check there too */
+    cpuid_count(0x40000100, 0, &eax, &ebx, &ecx, &edx);
+    return ebx == XEN_CPUID_SIGNATURE_EBX &&
+           ecx == XEN_CPUID_SIGNATURE_ECX &&
+           edx == XEN_CPUID_SIGNATURE_EDX;
+}
+
 void arch_setup(void)
 {
-    if ( IS_DEFINED(CONFIG_HVM) && !pvh_start_info )
-        register_console_callback(qemu_console_write);
+    has_xen_hypervisor = detect_xen_runtime();
+
+    if ( has_xen_hypervisor )
+    {
+        if ( IS_DEFINED(CONFIG_HVM) && !pvh_start_info )
+            register_console_callback(qemu_console_write);
 
-    register_console_callback(xen_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);
 
@@ -268,15 +307,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)

base-commit: 0cbf4c35b06b2b285fc325b8458132e844c5cf0e
-- 
2.43.0
Re: [XTF PATCH] x86: Remove Xen as a hard requirement to run XTF.
Posted by Andrew Cooper 3 days, 3 hours ago
On 30/09/2025 9:54 am, Alejandro Vallejo wrote:
> 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.
>
> This patch enables running XTF on QEMU-TCG/KVM out of the box.

When I did a KVM branch for amluto,
https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen-test-framework.git;a=shortlog;h=refs/heads/kvm
was what was necessary to get tests to pass.

I can see the need for MB1 going away now that KVM uses the PVH
entrypoint, but is the rest really unnecessary?

> diff --git a/arch/x86/setup.c b/arch/x86/setup.c
> index 2ac212e..6172c7e 100644
> --- a/arch/x86/setup.c
> +++ b/arch/x86/setup.c
> @@ -243,11 +245,19 @@ static void map_shared_info(void)
>          panic("Failed to map shared_info: %d\n", rc);
>  }
>  
> +static void pio_write(uint16_t port, const char *buf, size_t len)
> +{
> +    asm volatile("rep; outsb" : "+S" (buf), "+c" (len) : "d" (port));
> +}

I've factored out rep_movsb() in the proper place for library functions,
and without the rebasing issue reinserting the erroneous ;.

> @@ -255,12 +265,41 @@ static void xen_console_write(const char *buf, size_t len)
>      hypercall_console_write(buf, len);
>  }
>  
> +static bool detect_xen_runtime(void)
> +{
> +    uint32_t eax, ebx, ecx, edx;
> +
> +    /* PV tests always run under Xen */
> +    if ( IS_DEFINED(CONFIG_PV) )
> +        return true;
> +
> +    /* HVM tests may additionally run on non-Xen hypervisors or baremetal */
> +    cpuid_count(0x40000000, 0, &eax, &ebx, &ecx, &edx);
> +    if (  ebx == XEN_CPUID_SIGNATURE_EBX &&
> +          ecx == XEN_CPUID_SIGNATURE_ECX &&
> +          edx == XEN_CPUID_SIGNATURE_EDX )
> +        return true;
> +
> +    /* Viridian guests have the Xen leaves higher up, so check there too */
> +    cpuid_count(0x40000100, 0, &eax, &ebx, &ecx, &edx);
> +    return ebx == XEN_CPUID_SIGNATURE_EBX &&
> +           ecx == XEN_CPUID_SIGNATURE_ECX &&
> +           edx == XEN_CPUID_SIGNATURE_EDX;
> +}

This isn't quite correct.  There's a find_xen_leaves() helper which
should do what you want.

~Andrew

Re: [XTF PATCH] x86: Remove Xen as a hard requirement to run XTF.
Posted by Alejandro Vallejo 2 days, 9 hours ago
On Wed Oct 1, 2025 at 7:10 PM CEST, Andrew Cooper wrote:
> On 30/09/2025 9:54 am, Alejandro Vallejo wrote:
>> 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.
>>
>> This patch enables running XTF on QEMU-TCG/KVM out of the box.
>
> When I did a KVM branch for amluto,
> https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen-test-framework.git;a=shortlog;h=refs/heads/kvm
> was what was necessary to get tests to pass.
>
> I can see the need for MB1 going away now that KVM uses the PVH
> entrypoint, but is the rest really unnecessary?

I do run it, as-is and it works.

Depends what you're after. As-is the commit spins at the end for a lack of the
code knowing how to turn off QEMU. 0xF4 isn't the default qemu-debug-exit PIO,
(that'd be 0x501), but I assume that was the intent and you (or someone else)
got it from the TCG unit tests, that have a non-default PIO.

I didn't include something like that because it relies on the user having set
up " -device isa-debug-exit" in their QEMU cmdline. Thinking about it, the worst
case scenario is that the PIO write is ignored, so I'll just go with that.

Also, I'm not sure QEMU implements PIO 0xE9? I've only seen it in Xen. This
patch writes directly to COM1, and that seems like the better option.

>
>> diff --git a/arch/x86/setup.c b/arch/x86/setup.c
>> index 2ac212e..6172c7e 100644
>> --- a/arch/x86/setup.c
>> +++ b/arch/x86/setup.c
>> @@ -243,11 +245,19 @@ static void map_shared_info(void)
>>          panic("Failed to map shared_info: %d\n", rc);
>>  }
>>  
>> +static void pio_write(uint16_t port, const char *buf, size_t len)
>> +{
>> +    asm volatile("rep; outsb" : "+S" (buf), "+c" (len) : "d" (port));
>> +}
>
> I've factored out rep_movsb() in the proper place for library functions,
> and without the rebasing issue reinserting the erroneous ;.

You meant rep_outsb(). Too much ERMS is bad for you ;). It's just the commit
title that's wrong, the patch is still good. So I'll resend with using that
helper.

I'm a bit triggered by the port being the last argument rather than the first,
but I'll get over it.

>
>> @@ -255,12 +265,41 @@ static void xen_console_write(const char *buf, size_t len)
>>      hypercall_console_write(buf, len);
>>  }
>>  
>> +static bool detect_xen_runtime(void)
>> +{
>> +    uint32_t eax, ebx, ecx, edx;
>> +
>> +    /* PV tests always run under Xen */
>> +    if ( IS_DEFINED(CONFIG_PV) )
>> +        return true;
>> +
>> +    /* HVM tests may additionally run on non-Xen hypervisors or baremetal */
>> +    cpuid_count(0x40000000, 0, &eax, &ebx, &ecx, &edx);
>> +    if (  ebx == XEN_CPUID_SIGNATURE_EBX &&
>> +          ecx == XEN_CPUID_SIGNATURE_ECX &&
>> +          edx == XEN_CPUID_SIGNATURE_EDX )
>> +        return true;
>> +
>> +    /* Viridian guests have the Xen leaves higher up, so check there too */
>> +    cpuid_count(0x40000100, 0, &eax, &ebx, &ecx, &edx);
>> +    return ebx == XEN_CPUID_SIGNATURE_EBX &&
>> +           ecx == XEN_CPUID_SIGNATURE_ECX &&
>> +           edx == XEN_CPUID_SIGNATURE_EDX;
>> +}
>
> This isn't quite correct.  There's a find_xen_leaves() helper which
> should do what you want.

Right. find_xen_leaves() wants adjusting so it returns a sentinel leaf when no such
leaf exists, like -1ULL, or 0. I'll go with the former tentatively.

Now that I've noticed the existing detection function I might be able to
just do early returns where it's invoked rather than avoiding calling them
altogether.

I'll send a v2 shortly.

Cheers,
Alejandro
Re: [XTF PATCH] x86: Remove Xen as a hard requirement to run XTF.
Posted by Andrew Cooper 2 days, 8 hours ago
On 02/10/2025 11:41 am, Alejandro Vallejo wrote:
> On Wed Oct 1, 2025 at 7:10 PM CEST, Andrew Cooper wrote:
>> On 30/09/2025 9:54 am, Alejandro Vallejo wrote:
>>> 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.
>>>
>>> This patch enables running XTF on QEMU-TCG/KVM out of the box.
>> When I did a KVM branch for amluto,
>> https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen-test-framework.git;a=shortlog;h=refs/heads/kvm
>> was what was necessary to get tests to pass.
>>
>> I can see the need for MB1 going away now that KVM uses the PVH
>> entrypoint, but is the rest really unnecessary?
> I do run it, as-is and it works.
>
> Depends what you're after. As-is the commit spins at the end for a lack of the
> code knowing how to turn off QEMU. 0xF4 isn't the default qemu-debug-exit PIO,
> (that'd be 0x501), but I assume that was the intent and you (or someone else)
> got it from the TCG unit tests, that have a non-default PIO.
>
> I didn't include something like that because it relies on the user having set
> up " -device isa-debug-exit" in their QEMU cmdline. Thinking about it, the worst
> case scenario is that the PIO write is ignored, so I'll just go with that.
>
> Also, I'm not sure QEMU implements PIO 0xE9? I've only seen it in Xen. This
> patch writes directly to COM1, and that seems like the better option.

I can't remember exactly where I got that -device isa-debug-exit
invocation from, but it was some example documentation from QEMU itself.

If there's a better way then I'm all ears, but spinning is no use for
non-interactive tasks such as using ./xtf-runner for all tests.

>
>>> diff --git a/arch/x86/setup.c b/arch/x86/setup.c
>>> index 2ac212e..6172c7e 100644
>>> --- a/arch/x86/setup.c
>>> +++ b/arch/x86/setup.c
>>> @@ -243,11 +245,19 @@ static void map_shared_info(void)
>>>          panic("Failed to map shared_info: %d\n", rc);
>>>  }
>>>  
>>> +static void pio_write(uint16_t port, const char *buf, size_t len)
>>> +{
>>> +    asm volatile("rep; outsb" : "+S" (buf), "+c" (len) : "d" (port));
>>> +}
>> I've factored out rep_movsb() in the proper place for library functions,
>> and without the rebasing issue reinserting the erroneous ;.
> You meant rep_outsb(). Too much ERMS is bad for you ;).

Urgh I thought I fixed all of those typos.  Oh well.

>  It's just the commit
> title that's wrong, the patch is still good. So I'll resend with using that
> helper.
>
> I'm a bit triggered by the port being the last argument rather than the first,
> but I'll get over it.

For better or worse, that's how outb() and friends are defined (which
itself is a side effect of AT&T syntax).

~Andrew