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:

> On Wed, Aug 19, 2015 at 2:43 PM, Bandan Das <bsd@redhat.com> wrote:
>> 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.
>
> I wasn't aware of the past discussion of MSR_PLATFORM_INFO, I'll
> search for it - thanks for pointing it out.
>
> MSR_PLATFORM_INFO is just one example though. In addition to the
> benefits I already mentioned for user space MSR emulation (agility,
> reduced attack surface), this patchset offers a benefit even if user
> space declines to emulate the MSR by returning into kvm with
> KVM_EXIT_MSR_UNHANDLED: it makes it easier to monitor in the first
> place, via logging mechanisms in user space instead of in the kernel,
> for when guests are accessing MSRs that kvm doesn't implement.

Agreed, that would be more useful then how it's being handled currently.

> This patchset has already revealed a few other MSRs (IA32_MPERF,
> IA32_APERF) that guests in our test environment are accessing but
> which kvm doesn't implement yet. I haven't yet investigated deeply
> what these MSRs are used for and how they might be emulated (or if
> they can't actually be emulated correctly), but if we do discover an
> unimplemented non-performance-sensitive MSR that we could emulate
> correctly, with this patchset we would quickly just implement it in
> user space.

Ok, makes sense. But it seems like this could be an area of abuse as
well. For example, this could lessen the incentive to add emulation for
new msrs in kvm and rather just emulate them in userspace ?

However, as you mentioned, it's probably ok as long as there's a clear
demarcation of which msrs this is done for. This shouldn't be
the default behavior.

On a different note, if there's a related qemu change, can you please
point me to it ?

Thanks,
Bandan

>> 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(-)