Re: [PATCH RFC 11/39] KVM: x86/xen: evtchn signaling via eventfd

[Joao Martins]

On 11/30/20 6:01 PM, David Woodhouse wrote:
> On Mon, 2020-11-30 at 17:15 +0000, Joao Martins wrote:
>> On 11/30/20 4:48 PM, David Woodhouse wrote:
>>> On Mon, 2020-11-30 at 15:08 +0000, Joao Martins wrote:
>>>> On 11/30/20 12:55 PM, David Woodhouse wrote:
>>>>> On Mon, 2020-11-30 at 12:17 +0000, Joao Martins wrote:
>>>>>> On 11/30/20 9:41 AM, David Woodhouse wrote:
>>>>>>> On Wed, 2019-02-20 at 20:15 +0000, Joao Martins wrote:

[...]

>>>> I should comment on your other patch but: if we're going to make it generic for
>>>> the userspace hypercall handling, might as well move hyper-v there too. In this series,
>>>> I added KVM_EXIT_XEN, much like it exists KVM_EXIT_HYPERV -- but with a generic version
>>>> I wonder if a capability could gate KVM_EXIT_HYPERCALL to handle both guest types, while
>>>> disabling KVM_EXIT_HYPERV. But this is probably subject of its own separate patch :)
>>>
>>> There's a limit to how much consolidation we can do because the ABI is
>>> different; the args are in different registers.
>>>
>>
>> Yes. It would be optionally enabled of course and VMM would have to adjust to the new ABI
>> -- surely wouldn't want to break current users of KVM_EXIT_HYPERV.
> 
> True, but that means we'd have to keep KVM_EXIT_HYPERV around anyway,
> and can't actually *remove* it. The "consolidation" gives us more
> complexity, not less.
>
Fair point.

>>> I do suspect Hyper-V should have marshalled its arguments into the
>>> existing kvm_run->arch.hypercall and used KVM_EXIT_HYPERCALL but I
>>> don't think it makes sense to change it now since it's a user-facing
>>> ABI. I don't want to follow its lead by inventing *another* gratuitous
>>> exit type for Xen though.
>>>
>>
>> I definitely like the KVM_EXIT_HYPERCALL better than a KVM_EXIT_XEN userspace
>> exit type ;)
>>
>> But I guess you still need to co-relate a type of hypercall (Xen guest cap enabled?) to
>> tell it's Xen or KVM to specially enlighten certain opcodes (EVTCHNOP_send).
> 
> Sure, but if the VMM doesn't know what kind of guest it's hosting, we
> have bigger problems... :)
> 
Right :)

I was referring to the kernel here.

Eventually we need to special case things for a given guest type case e.g.

int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
{
...
        if (kvm_hv_hypercall_enabled(vcpu->kvm))
                return kvm_hv_hypercall(...);

        if (kvm_xen_hypercall_enabled(vcpu->kvm))
                return kvm_xen_hypercall(...);
...
}

And on kvm_xen_hypercall() for the cases VMM offloads to demarshal what the registers mean
e.g. for event channel send 64-bit guest: RAX for opcode and RDI/RSI for cmd and port.

The kernel logic wouldn't be much different at the core, so thought of tihs consolidation.
But the added complexity would have come from having to deal with two userspace exit types
-- indeed probably not worth the trouble as you pointed out.