[RFC PATCH 4/5] KVM: x86: enable unhandled MSR exits for vmx
Set the vm's unhandled_msr_exits flag when user space calls the
KVM_ENABLE_CAP ioctl with KVM_CAP_UNHANDLED_MSR_EXITS. After kvm fails
to handle a guest rdmsr or wrmsr, check this flag and exit to user space
with KVM_EXIT_MSR rather than immediately injecting a GP fault.
On reentry into kvm, use the complete_userspace_io callback path to call
vmx_complete_userspace_msr. Complete the MSR access if user space was
able to handle it successfully, or fail the MSR access and inject a GP
fault if user space could not handle the access.
Signed-off-by: Peter Hornyack
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/vmx.c | 74 -
arch/x86/kvm/x86.c | 12 +++
3 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index a6e145b1e271..1b06cea06a8e 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -682,6 +682,7 @@ struct kvm_arch {
u32 bsp_vcpu_id;
u64 disabled_quirks;
+ bool unhandled_msr_exits;
};
struct kvm_vm_stat {
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 27fec385d79d..ba26d382d785 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include "kvm_cache_regs.h"
#include "x86.h"
@@ -5491,6 +5492,53 @@ static void fail_wrmsr(struct kvm_vcpu *vcpu, const
struct msr_data *msr)
kvm_inject_gp(vcpu, 0);
}
+/*
+ * On success, returns 1 so that __vcpu_run() will happen next. On error,
+ * returns 0.
+ */
+static int vmx_complete_userspace_msr(struct kvm_vcpu *vcpu)
+{
+ struct msr_data msr;
+
+ if (vcpu->run->msr.index != vcpu->arch.regs[VCPU_REGS_RCX]) {
+ pr_debug("msr.index 0x%x changed, does not match ecx 0x%lx\n",
+vcpu->run->msr.index,
+vcpu->arch.regs[VCPU_REGS_RCX]);
+ goto err_out;
+ }
+ msr.index = vcpu->run->msr.index;
+ msr.data = vcpu->run->msr.data;
+ msr.host_initiated = false;
+
+ switch (vcpu->run->msr.direction) {
+ case KVM_EXIT_MSR_RDMSR:
+ if (vcpu->run->msr.handled == KVM_EXIT_MSR_HANDLED)
+ complete_rdmsr(vcpu, &msr);
+ else
+ fail_rdmsr(vcpu, &msr);
+ break;
+ case KVM_EXIT_MSR_WRMSR:
+ if (vcpu->run->msr.handled == KVM_EXIT_MSR_HANDLED)
+ complete_wrmsr(vcpu, &msr);
+ else
+ fail_wrmsr(vcpu, &msr);
+ break;
+ default:
+ pr_debug("bad msr.direction %u\n", vcpu->run->msr.direction);
+ goto err_out;
+ }
+
+ return 1;
+err_out:
+ vcpu->run->exit_reason = KVM_EXIT_MSR;
+ vcpu->run->msr.direction = KVM_EXIT_MSR_COMPLETION_FAILED;
+ return 0;
+}
+
+/*
+ * Returns 1 if the rdmsr handling is complete; returns 0 if kvm should exit to
+ * user space to handle this rdmsr.
+ */
static int handle_rdmsr(struct kvm_vcpu *vcpu)
{
u32 ecx = vcpu->arch.regs[VCPU_REGS_RCX];
@@ -5499,6 +5547,16 @@ static int handle_rdmsr(struct kvm_vcpu *vcpu)
msr_info.index = ecx;
msr_info.host_initiated = false;
if (vmx_get_msr(vcpu, &msr_info)) {
+ if (vcpu->kvm->arch.unhandled_msr_exits) {
+ vcpu->run->exit_reason = KVM_EXIT_MSR;
+ vcpu->run->msr.direction = KVM_EXIT_MSR_RDMSR;
+ vcpu->run->msr.index = msr_info.index;
+ vcpu->run->msr.data = 0;
+ vcpu->run->msr.handled = 0;
+ vcpu->arch.complete_userspace_io =
+ vmx_complete_userspace_msr;
+ return 0;
+ }
fail_rdmsr(vcpu, &msr_info);
return 1;
}
@@ -5508,6 +5566,10 @@ static int handle_rdmsr(struct kvm_vcpu *vcpu)
return 1;
}
+/*
+ * Returns 1 if the wrmsr handling is complete; returns 0 if kvm should exit to
+ * user space to handle this wrmsr.
+ */
static int handle_wrmsr(struct kvm_vcpu *vcpu)
{
struct msr_data msr;
@@ -5519,6 +5581,16 @@ static int handle_wrmsr(struct kvm_vcpu *vcpu)
msr.index = ecx;
msr.host_initiated = false;
if (kvm_set_msr(vcpu, &msr) != 0) {
+ if (vcpu->kvm->arch.unhandled_msr_exits) {
+ vcpu->run->exit_reason = KVM_EXIT_MSR;
+ vcpu->run->msr.direction = KVM_EXIT_MSR_WRMSR;
+ vcpu->run->msr.index = msr.index;
+ vcpu->run->msr.data = msr.data;
+ vcpu->run->msr.handled = 0;
+ vcpu->arch.complete_userspace_io =
+ vmx_complete_userspace_msr;
+ return 0;
Re: [RFC PATCH 4/5] KVM: x86: enable unhandled MSR exits for vmx
Peter Hornyack writes:
> Set the vm's unhandled_msr_exits flag when user space calls the
> KVM_ENABLE_CAP ioctl with KVM_CAP_UNHANDLED_MSR_EXITS. After kvm fails
> to handle a guest rdmsr or wrmsr, check this flag and exit to user space
> with KVM_EXIT_MSR rather than immediately injecting a GP fault.
>
> On reentry into kvm, use the complete_userspace_io callback path to call
> vmx_complete_userspace_msr. Complete the MSR access if user space was
> able to handle it successfully, or fail the MSR access and inject a GP
> fault if user space could not handle the access.
>
...
> +static int vmx_complete_userspace_msr(struct kvm_vcpu *vcpu)
> +{
> + struct msr_data msr;
> +
> + if (vcpu->run->msr.index != vcpu->arch.regs[VCPU_REGS_RCX]) {
> + pr_debug("msr.index 0x%x changed, does not match ecx 0x%lx\n",
> + vcpu->run->msr.index,
> + vcpu->arch.regs[VCPU_REGS_RCX]);
> + goto err_out;
> + }
> + msr.index = vcpu->run->msr.index;
> + msr.data = vcpu->run->msr.data;
> + msr.host_initiated = false;
> +
> + switch (vcpu->run->msr.direction) {
> + case KVM_EXIT_MSR_RDMSR:
> + if (vcpu->run->msr.handled == KVM_EXIT_MSR_HANDLED)
> + complete_rdmsr(vcpu, &msr);
> + else
> + fail_rdmsr(vcpu, &msr);
> + break;
Should we check for MSR_UNHANDLED ? Could be "debugging relief" if userspace is
filling it up with random crap! I think it should be ok if the trace function
catches that too.
> + case KVM_EXIT_MSR_WRMSR:
> + if (vcpu->run->msr.handled == KVM_EXIT_MSR_HANDLED)
> + complete_wrmsr(vcpu, &msr);
> + else
> + fail_wrmsr(vcpu, &msr);
> + break;
> + default:
> + pr_debug("bad msr.direction %u\n", vcpu->run->msr.direction);
> + goto err_out;
> + }
> +
> + return 1;
> +err_out:
> + vcpu->run->exit_reason = KVM_EXIT_MSR;
> + vcpu->run->msr.direction = KVM_EXIT_MSR_COMPLETION_FAILED;
> + return 0;
> +}
> +
> +/*
> + * Returns 1 if the rdmsr handling is complete; returns 0 if kvm should exit
> to
> + * user space to handle this rdmsr.
> + */
> static int handle_rdmsr(struct kvm_vcpu *vcpu)
> {
> u32 ecx = vcpu->arch.regs[VCPU_REGS_RCX];
> @@ -5499,6 +5547,16 @@ static int handle_rdmsr(struct kvm_vcpu *vcpu)
> msr_info.index = ecx;
> msr_info.host_initiated = false;
> if (vmx_get_msr(vcpu, &msr_info)) {
> + if (vcpu->kvm->arch.unhandled_msr_exits) {
> + vcpu->run->exit_reason = KVM_EXIT_MSR;
> + vcpu->run->msr.direction = KVM_EXIT_MSR_RDMSR;
> + vcpu->run->msr.index = msr_info.index;
> + vcpu->run->msr.data = 0;
> + vcpu->run->msr.handled = 0;
> + vcpu->arch.complete_userspace_io =
> + vmx_complete_userspace_msr;
> + return 0;
> + }
> fail_rdmsr(vcpu, &msr_info);
> return 1;
> }
> @@ -5508,6 +5566,10 @@ static int handle_rdmsr(struct kvm_vcpu *vcpu)
> return 1;
> }
>
> +/*
> + * Returns 1 if the wrmsr handling is complete; returns 0 if kvm should exit
> to
> + * user space to handle this wrmsr.
> + */
> static int handle_wrmsr(struct kvm_vcpu *vcpu)
> {
> struct msr_data msr;
> @@ -5519,6 +5581,16 @@ static int handle_wrmsr(struct kvm_vcpu *vcpu)
> msr.index = ecx;
> msr.host_initiated = false;
> if (kvm_set_msr(vcpu, &msr) != 0) {
> + if (vcpu->kvm->arch.unhandled_msr_exits) {
I think kvm should ultimately decide which msrs it can let userspace
handle even if userspace has explicitly enabled the ioctl.
Bandan
> + vcpu->run->exit_reason = KVM_EXIT_MSR;
> + vcpu->run->msr.direction = KVM_EXIT_MSR_WRMSR;
> + vcpu->run->msr.index = msr.index;
> + vcpu->run->msr.data = msr.data;
> + vcpu->run->msr.handled = 0;
> + vcpu->arch.complete_userspace_io =
> + vmx_complete_userspace_msr;
> + return 0;
> + }
> fail_wrmsr(vcpu, &msr);
> return 1;
> }
> @@ -8163,7 +8235,7 @@ static bool vmx_xsaves_supported(void)
>
> static bool vmx_msr_exits_supported(void)
> {
> - return false;
> + return true;
> }
>
> static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 4bbc2a1676c9..5c22f4655741 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2461,6 +2461,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long
> ext)
> case KVM_CAP_ENABLE_CAP_VM:
> case KVM_CAP_DISABLE_QUIRKS:
> case KVM_CAP_SET_BOOT_CPU_ID
