Re: [RFC PATCH 0/5] KVM: x86: exit to user space on unhandled MSR accesses

[Bandan Das <bsd@redhat.com>]

Peter Hornyack <peterhornyack@google.com> writes:

> There are numerous MSRs that kvm does not currently handle. On Intel
> platforms we have observed guest VMs accessing some of these MSRs (for
> example, MSR_PLATFORM_INFO) and behaving poorly (to the point of guest OS
> crashes) when they receive a GP fault because the MSR is not emulated. This
> patchset adds a new kvm exit path for unhandled MSR accesses that allows
> user space to emulate additional MSRs without having to implement them in
> kvm.

^^ So, I am trying to understand this motivation. A while back when 
a patch was posted to emulate MSR_PLATFORM_INFO, it was rejected.
Why ? Because it seemed impossible to emulate it correctly (most concerns were
related to migration iirc). Although I haven't reviewed all patches in this series
yet, what I understand from the above message is-"It's ok to emulate
MSR_PLATFORM_INFO *incorrectly* as long as we are doing it in the user space."

I understand the part where it makes sense to move stuff to userspace.
But if kvm isn't emulating certain msrs yet, either we should add support,
or they haven't been added because it's not possible to emulate them
correctly. The logic that it's probably ok to let userspace do the (incorrect)
emulation is something I don't understand. It seems like the next in line
is to let userspace emulate thier own version of unimplemented x86 instructions.


> The core of the patchset modifies the vmx handle_rdmsr and handle_wrmsr
> functions to exit to user space on MSR reads/writes that kvm can't handle
> itself. Then, on the return path into kvm we check for outstanding user
> space MSR completions and either complete the MSR access successfully or
> inject a GP fault as kvm would do by default. This new exit path must be
> enabled for the vm via the KVM_CAP_UNHANDLED_MSR_EXITS capability.
>
> In the future we plan to extend this functionality to allow user space to
> register the MSRs that it would like to handle itself, even if kvm already
> provides an implementation. In the long-term we will move the

I seriously hope we don't do this!

Bandan
> implementation of all non-performance-sensitive MSRs to user space,
> reducing the potential attack surface of kvm and allowing us to respond to
> bugs more quickly.
>
> This patchset has been tested with our non-qemu user space hypervisor on
> vmx platforms; svm support is not implemented.
>
> Peter Hornyack (5):
>   KVM: x86: refactor vmx rdmsr/wrmsr completion into new functions
>   KVM: add KVM_EXIT_MSR exit reason and capability.
>   KVM: x86: add msr_exits_supported to kvm_x86_ops
>   KVM: x86: enable unhandled MSR exits for vmx
>   KVM: x86: add trace events for unhandled MSR exits
>
>  Documentation/virtual/kvm/api.txt |  48 +++++++++++++++
>  arch/x86/include/asm/kvm_host.h   |   2 +
>  arch/x86/kvm/svm.c                |   6 ++
>  arch/x86/kvm/trace.h              |  28 +++++++++
>  arch/x86/kvm/vmx.c                | 126 ++++++++++++++++++++++++++++++++++----
>  arch/x86/kvm/x86.c                |  13 ++++
>  include/trace/events/kvm.h        |   2 +-
>  include/uapi/linux/kvm.h          |  14 +++++
>  8 files changed, 227 insertions(+), 12 deletions(-)