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

Reply via email to