Re: [RFC PATCH 10/21] i386/xen: handle guest hypercalls

From: David Woodhouse <dwmw2@infradead.org>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>, qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>, Paul Durrant <paul@xen.org>,
	Joao Martins <joao.m.martins@oracle.com>,
	Ankur Arora <ankur.a.arora@oracle.com>
Subject: Re: [RFC PATCH 10/21] i386/xen: handle guest hypercalls
Date: Tue, 06 Dec 2022 01:10:15 +0000	[thread overview]
Message-ID: <cb4cdc7944c74ee1918667ba455b9afd0fae351c.camel@infradead.org> (raw)
In-Reply-To: <efef965d-3c9b-abc0-ac5f-c252b10722bd@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 2546 bytes --]

On Mon, 2022-12-05 at 23:11 +0100, Philippe Mathieu-Daudé wrote:
> On 5/12/22 18:31, David Woodhouse wrote:
> > From: Joao Martins <joao.m.martins@oracle.com>
> > 
> > This means handling the new exit reason for Xen but still
> > crashing on purpose. As we implement each of the hypercalls
> > we will then return the right return code.
> > 
> > Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> > [dwmw2: Add CPL to hypercall tracing, disallow hypercalls from CPL > 0]
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > ---
> >   target/i386/kvm/kvm.c    |  5 +++++
> >   target/i386/trace-events |  3 +++
> >   target/i386/xen.c        | 45 ++++++++++++++++++++++++++++++++++++++++
> >   target/i386/xen.h        |  1 +
> >   4 files changed, 54 insertions(+)
> > 
> > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> > index 4b21d03250..6396d11f1e 100644
> > --- a/target/i386/kvm/kvm.c
> > +++ b/target/i386/kvm/kvm.c
> > @@ -5468,6 +5468,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> >           assert(run->msr.reason == KVM_MSR_EXIT_REASON_FILTER);
> >           ret = kvm_handle_wrmsr(cpu, run);
> >           break;
> > +#ifdef CONFIG_XEN
> 
> CONFIG_XEN is set when the _host_ has Xen development files available.
> 
> IIUC here you want to check if Xen HVM guest support is enabled.
> 
> You might want to use a different CONFIG_XEN_xxx key, which itself
> depends on CONFIG_XEN.

Yeah, I'd be interested in opinions on that one.

Strictly, the only one that *needs* to be a configure option is
CONFIG_XEN for the Xen libraries, which is support for actually running
on Xen.

Any time KVM is present, we *could* pull in the rest of the xenfv
machine support unconditionally, since that's no longer dependent on
true Xen.

But because there's a non-trivial amount of code in the event channel
and grant table stuff, *perhaps* we want to make it optional? I don't
really want to call that CONFIG_KVM_XEN since as noted, it's
theoretically possible to do it with TCG or other accelerators too. So
we could call it CONFIG_XEN_EMULATION.

I don't think we'd make that depend on CONFIG_XEN though, since none of
the actual Xen libraries would be needed once everything's implemented
and cleaned up.

So things like the xenfv machine code would then depend on
(CONFIG_XEN || CONFIG_XEN_EMULATION)... or we could make a new
automatic config symbol CONFIG_XEN_MACHINE which has the same effect?

Happy to do it however seems best.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]