KVM: Move vcpu_load to arch-specific kvm_arch_vcpu_ioctl

Move the calls to vcpu_load() and vcpu_put() in to the architecture
specific implementations of kvm_arch_vcpu_ioctl() which dispatches
further architecture-specific ioctls on to other functions.

Some architectures support asynchronous vcpu ioctls which cannot call
vcpu_load() or take the vcpu->mutex, because that would prevent
concurrent execution with a running VCPU, which is the intended purpose
of these ioctls, for example because they inject interrupts.

We repeat the separate checks for these specifics in the architecture
code for MIPS, S390 and PPC, and avoid taking the vcpu->mutex and
calling vcpu_load for these ioctls.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index dd5f463b0e7264427a084dc56c9a4f4b75125d3c..9200b3def440a70ef6fbda756addf506532daf2e 100644 (file)
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -910,56 +910,65 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
        void __user *argp = (void __user *)arg;
        long r;
 
+       if (ioctl == KVM_INTERRUPT) {
+               struct kvm_mips_interrupt irq;
+
+               if (copy_from_user(&irq, argp, sizeof(irq)))
+                       return -EFAULT;
+               kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
+                         irq.irq);
+
+               return kvm_vcpu_ioctl_interrupt(vcpu, &irq);
+       }
+
+       vcpu_load(vcpu);
+
        switch (ioctl) {
        case KVM_SET_ONE_REG:
        case KVM_GET_ONE_REG: {
                struct kvm_one_reg reg;
 
+               r = -EFAULT;
                if (copy_from_user(&reg, argp, sizeof(reg)))
-                       return -EFAULT;
+                       break;
                if (ioctl == KVM_SET_ONE_REG)
-                       return kvm_mips_set_reg(vcpu, &reg);
+                       r = kvm_mips_set_reg(vcpu, &reg);
                else
-                       return kvm_mips_get_reg(vcpu, &reg);
+                       r = kvm_mips_get_reg(vcpu, &reg);
+               break;
        }
        case KVM_GET_REG_LIST: {
                struct kvm_reg_list __user *user_list = argp;
                struct kvm_reg_list reg_list;
                unsigned n;
 
+               r = -EFAULT;
                if (copy_from_user(&reg_list, user_list, sizeof(reg_list)))
-                       return -EFAULT;
+                       break;
                n = reg_list.n;
                reg_list.n = kvm_mips_num_regs(vcpu);
                if (copy_to_user(user_list, &reg_list, sizeof(reg_list)))
-                       return -EFAULT;
+                       break;
+               r = -E2BIG;
                if (n < reg_list.n)
-                       return -E2BIG;
-               return kvm_mips_copy_reg_indices(vcpu, user_list->reg);
-       }
-       case KVM_INTERRUPT:
-               {
-                       struct kvm_mips_interrupt irq;
-
-                       if (copy_from_user(&irq, argp, sizeof(irq)))
-                               return -EFAULT;
-                       kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
-                                 irq.irq);
-
-                       r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
                        break;
-               }
+               r = kvm_mips_copy_reg_indices(vcpu, user_list->reg);
+               break;
+       }
        case KVM_ENABLE_CAP: {
                struct kvm_enable_cap cap;
 
+               r = -EFAULT;
                if (copy_from_user(&cap, argp, sizeof(cap)))
-                       return -EFAULT;
+                       break;
                r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
                break;
        }
        default:
                r = -ENOIOCTLCMD;
        }
+
+       vcpu_put(vcpu);
        return r;
 }
 

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 4e2167a7ae19eca6c08efd145573b2a7289ce01e..ba8134a989c1bb0e2e7270b0071d08b0c3082721 100644 (file)
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1614,16 +1614,16 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
        void __user *argp = (void __user *)arg;
        long r;
 
-       switch (ioctl) {
-       case KVM_INTERRUPT: {
+       if (ioctl == KVM_INTERRUPT) {
                struct kvm_interrupt irq;
-               r = -EFAULT;
                if (copy_from_user(&irq, argp, sizeof(irq)))
-                       goto out;
-               r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
-               goto out;
+                       return -EFAULT;
+               return kvm_vcpu_ioctl_interrupt(vcpu, &irq);
        }
 
+       vcpu_load(vcpu);
+
+       switch (ioctl) {
        case KVM_ENABLE_CAP:
        {
                struct kvm_enable_cap cap;
@@ -1663,6 +1663,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
        }
 
 out:
+       vcpu_put(vcpu);
        return r;
 }
 

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 3bd2f83e1fa981679752e48eab9db3285c17349d..9700d71cb691169caaf197d8e3195d87c0658b80 100644 (file)
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3737,24 +3737,25 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
        case KVM_S390_IRQ: {
                struct kvm_s390_irq s390irq;
 
-               r = -EFAULT;
                if (copy_from_user(&s390irq, argp, sizeof(s390irq)))
-                       break;
-               r = kvm_s390_inject_vcpu(vcpu, &s390irq);
-               break;
+                       return -EFAULT;
+               return kvm_s390_inject_vcpu(vcpu, &s390irq);
        }
        case KVM_S390_INTERRUPT: {
                struct kvm_s390_interrupt s390int;
                struct kvm_s390_irq s390irq;
 
-               r = -EFAULT;
                if (copy_from_user(&s390int, argp, sizeof(s390int)))
-                       break;
+                       return -EFAULT;
                if (s390int_to_s390irq(&s390int, &s390irq))
                        return -EINVAL;
-               r = kvm_s390_inject_vcpu(vcpu, &s390irq);
-               break;
+               return kvm_s390_inject_vcpu(vcpu, &s390irq);
        }
+       }
+
+       vcpu_load(vcpu);
+
+       switch (ioctl) {
        case KVM_S390_STORE_STATUS:
                idx = srcu_read_lock(&vcpu->kvm->srcu);
                r = kvm_s390_vcpu_store_status(vcpu, arg);
@@ -3879,6 +3880,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
        default:
                r = -ENOTTY;
        }
+
+       vcpu_put(vcpu);
        return r;
 }
 

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 981b61531ab746a2d0489d7f5f47e3231a3c64d1..fafaef072f1b5d9c228b448f636604d6e5e0b6c3 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3484,6 +3484,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
                void *buffer;
        } u;
 
+       vcpu_load(vcpu);
+
        u.buffer = NULL;
        switch (ioctl) {
        case KVM_GET_LAPIC: {
@@ -3509,8 +3511,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
                if (!lapic_in_kernel(vcpu))
                        goto out;
                u.lapic = memdup_user(argp, sizeof(*u.lapic));
-               if (IS_ERR(u.lapic))
-                       return PTR_ERR(u.lapic);
+               if (IS_ERR(u.lapic)) {
+                       r = PTR_ERR(u.lapic);
+                       goto out_nofree;
+               }
 
                r = kvm_vcpu_ioctl_set_lapic(vcpu, u.lapic);
                break;
@@ -3684,8 +3688,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
        }
        case KVM_SET_XSAVE: {
                u.xsave = memdup_user(argp, sizeof(*u.xsave));
-               if (IS_ERR(u.xsave))
-                       return PTR_ERR(u.xsave);
+               if (IS_ERR(u.xsave)) {
+                       r = PTR_ERR(u.xsave);
+                       goto out_nofree;
+               }
 
                r = kvm_vcpu_ioctl_x86_set_xsave(vcpu, u.xsave);
                break;
@@ -3707,8 +3713,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
        }
        case KVM_SET_XCRS: {
                u.xcrs = memdup_user(argp, sizeof(*u.xcrs));
-               if (IS_ERR(u.xcrs))
-                       return PTR_ERR(u.xcrs);
+               if (IS_ERR(u.xcrs)) {
+                       r = PTR_ERR(u.xcrs);
+                       goto out_nofree;
+               }
 
                r = kvm_vcpu_ioctl_x86_set_xcrs(vcpu, u.xcrs);
                break;
@@ -3752,6 +3760,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
        }
 out:
        kfree(u.buffer);
+out_nofree:
+       vcpu_put(vcpu);
        return r;
 }
 

diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index aa6167086211dd039a751e956fe7e28898276eda..cd7d90c9f644d18743020672afbbc7ae06dcbbb9 100644 (file)
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1003,66 +1003,88 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
        struct kvm_vcpu *vcpu = filp->private_data;
        void __user *argp = (void __user *)arg;
        struct kvm_device_attr attr;
+       long r;
+
+       vcpu_load(vcpu);
 
        switch (ioctl) {
        case KVM_ARM_VCPU_INIT: {
                struct kvm_vcpu_init init;
 
+               r = -EFAULT;
                if (copy_from_user(&init, argp, sizeof(init)))
-                       return -EFAULT;
+                       break;
 
-               return kvm_arch_vcpu_ioctl_vcpu_init(vcpu, &init);
+               r = kvm_arch_vcpu_ioctl_vcpu_init(vcpu, &init);
+               break;
        }
        case KVM_SET_ONE_REG:
        case KVM_GET_ONE_REG: {
                struct kvm_one_reg reg;
 
+               r = -ENOEXEC;
                if (unlikely(!kvm_vcpu_initialized(vcpu)))
-                       return -ENOEXEC;
+                       break;
 
+               r = -EFAULT;
                if (copy_from_user(&reg, argp, sizeof(reg)))
-                       return -EFAULT;
+                       break;
+
                if (ioctl == KVM_SET_ONE_REG)
-                       return kvm_arm_set_reg(vcpu, &reg);
+                       r = kvm_arm_set_reg(vcpu, &reg);
                else
-                       return kvm_arm_get_reg(vcpu, &reg);
+                       r = kvm_arm_get_reg(vcpu, &reg);
+               break;
        }
        case KVM_GET_REG_LIST: {
                struct kvm_reg_list __user *user_list = argp;
                struct kvm_reg_list reg_list;
                unsigned n;
 
+               r = -ENOEXEC;
                if (unlikely(!kvm_vcpu_initialized(vcpu)))
-                       return -ENOEXEC;
+                       break;
 
+               r = -EFAULT;
                if (copy_from_user(&reg_list, user_list, sizeof(reg_list)))
-                       return -EFAULT;
+                       break;
                n = reg_list.n;
                reg_list.n = kvm_arm_num_regs(vcpu);
                if (copy_to_user(user_list, &reg_list, sizeof(reg_list)))
-                       return -EFAULT;
+                       break;
+               r = -E2BIG;
                if (n < reg_list.n)
-                       return -E2BIG;
-               return kvm_arm_copy_reg_indices(vcpu, user_list->reg);
+                       break;
+               r = kvm_arm_copy_reg_indices(vcpu, user_list->reg);
+               break;
        }
        case KVM_SET_DEVICE_ATTR: {
+               r = -EFAULT;
                if (copy_from_user(&attr, argp, sizeof(attr)))
-                       return -EFAULT;
-               return kvm_arm_vcpu_set_attr(vcpu, &attr);
+                       break;
+               r = kvm_arm_vcpu_set_attr(vcpu, &attr);
+               break;
        }
        case KVM_GET_DEVICE_ATTR: {
+               r = -EFAULT;
                if (copy_from_user(&attr, argp, sizeof(attr)))
-                       return -EFAULT;
-               return kvm_arm_vcpu_get_attr(vcpu, &attr);
+                       break;
+               r = kvm_arm_vcpu_get_attr(vcpu, &attr);
+               break;
        }
        case KVM_HAS_DEVICE_ATTR: {
+               r = -EFAULT;
                if (copy_from_user(&attr, argp, sizeof(attr)))
-                       return -EFAULT;
-               return kvm_arm_vcpu_has_attr(vcpu, &attr);
+                       break;
+               r = kvm_arm_vcpu_has_attr(vcpu, &attr);
+               break;
        }
        default:
-               return -EINVAL;
+               r = -EINVAL;
        }
+
+       vcpu_put(vcpu);
+       return r;
 }
 
 /**

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ca0ec9fb72ce9aacb93a7ae4f007323568335b94..19c184fa1839ac76063c68fbb136d66be784e74b 100644 (file)
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2724,9 +2724,7 @@ out_free1:
                break;
        }
        default:
-               vcpu_load(vcpu);
                r = kvm_arch_vcpu_ioctl(filp, ioctl, arg);
-               vcpu_put(vcpu);
        }
 out:
        mutex_unlock(&vcpu->mutex);