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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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@xxxxxxx>
> >> >> ---
> >> >>  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.