Re: [PATCH v3 02/17] KVM: x86/xen: fix Xen hypercall page msr handling

[Vitaly Kuznetsov <vkuznets@redhat.com>]

David Woodhouse <dwmw2@infradead.org> writes:

> On Mon, 2020-12-14 at 23:22 +0100, Vitaly Kuznetsov wrote:
>> > Can you define kvm_hv_msr_enabled()?
>> > 
>> > Note kvm_hv_hypercall_enabled() is based on a value that gets written
>> > through the MSR, so it can't be that.
>> 
>> When Hyper-V emulation appeared in KVM we (unfortunately) didn't add a
>> capability to globaly enable and disable it so to be backwards
>> compatible we'll have to define kvm_emulating_hyperv() as 'true' for
>> now as that's how KVM behaves. This, however, doesn't mean we can't add
>> e.g. a module parameter to disable Hyper-V emulation. Also, we can
>> probably check guest CPUIDs and if Hyper-V's signature wasn't set we can
>> return 'false'.
>> 
>> <rant>
>> Having Hyper-V emulation in KVM 'always enabled' may not be a big deal
>> from functional point of view but may not be ideal from security
>> standpoint as bugs in arch/x86/kvm/hyperv.c become exploitable even from
>> Linux guests.
>> </rant>
>
> Indeed. And yet it can coexist with Xen support too, so it isn't even
> as simple as turning it off when Xen is enabled.
>
> Which is why I ended up just using Joao's patch unchanged. Short of
> going back in time to make Hyper-V support conditional when it was
> first introduced, I couldn't see a better answer.
>
> And regardless of the Hyper-V mess, what this patch does for Xen is
> precisely what you suggest: handle it first, before the switch(), *if*
> the Xen MSR is enabled.

Functionally I have no complaints, even with the suggested
'generalization' we'll be handling MSRs in the exact same sequence. You
are, however, right calling Hyper-V mess 'mess' and if we want to make
things cleaner we should probably start there (goes to my to-do
list...).

-- 
Vitaly