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

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> +Thomas
>
> On 6/12/22 02:10, David Woodhouse wrote:
>> 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 concur CONFIG_KVM_XEN is confusing; CONFIG_XEN_EMULATION /
> CONFIG_XEN_EMU sounds better.
>
> Is it useful to have the CONFIG_XEN_EMU code under target/i386/ built
> without having the xenfv machine built in?
>
> I rather have hw/ and target/ features disentangled, so I'd use
> CONFIG_XEN_EMU under target/ and CONFIG_XENFV_MACHINE under hw/,
> eventually having CONFIG_XEN_EMU depending on CONFIG_XENFV_MACHINE
> and -- for now -- CONFIG_KVM.

You should also probably be aware of:

  Subject: [QEMU][PATCH v2 00/11] Introduce xenpv machine for arm architecture 
  Date: Thu, 1 Dec 2022 18:59:52 -0800
  Message-ID: <20221202030003.11441-1-vikram.garhwal@amd.com>

which moves some of the previously i386 only Xen code into common
backend code.

-- 
Alex Bennée