Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751554AbdLLRrW (ORCPT ); Tue, 12 Dec 2017 12:47:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50164 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752279AbdLLRrP (ORCPT ); Tue, 12 Dec 2017 12:47:15 -0500 Subject: Re: [PATCH] KVM: introduce kvm_arch_vcpu_async_ioctl To: Paolo Bonzini , linux-kernel@vger.kernel.org, kvm@vger.kernel.org Cc: kvm-ppc@vger.kernel.org, cohuck@redhat.com, christoffer.dall@linaro.org, James Hogan , Paul Mackerras , Christian Borntraeger References: <1513099915-107126-1-git-send-email-pbonzini@redhat.com> From: David Hildenbrand Organization: Red Hat GmbH Message-ID: Date: Tue, 12 Dec 2017 18:47:11 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1513099915-107126-1-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 12 Dec 2017 17:47:14 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3841 Lines: 131 > > switch (ioctl) { > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index ba8134a989c1..2e700753e35c 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -1607,12 +1607,11 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu, > return -EINVAL; > } > > -long kvm_arch_vcpu_ioctl(struct file *filp, > - unsigned int ioctl, unsigned long arg) > +long kvm_arch_vcpu_async_ioctl(struct file *filp, > + unsigned int ioctl, unsigned long arg) are you mixing spaces and tabs? > { > struct kvm_vcpu *vcpu = filp->private_data; > void __user *argp = (void __user *)arg; > - long r; > > if (ioctl == KVM_INTERRUPT) { > struct kvm_interrupt irq; > @@ -1620,6 +1619,15 @@ long kvm_arch_vcpu_ioctl(struct file *filp, > return -EFAULT; > return kvm_vcpu_ioctl_interrupt(vcpu, &irq); > } > + return -ENOIOCTLCMD; > +} > + > +long kvm_arch_vcpu_ioctl(struct file *filp, > + unsigned int ioctl, unsigned long arg) dito > +{ > + struct kvm_vcpu *vcpu = filp->private_data; > + void __user *argp = (void __user *)arg; > + long r; > > vcpu_load(vcpu); > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 9700d71cb691..40f0ae5a883f 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -3725,13 +3725,11 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu, > return r; > } > > -long kvm_arch_vcpu_ioctl(struct file *filp, > - unsigned int ioctl, unsigned long arg) > +long kvm_arch_vcpu_async_ioctl(struct file *filp, > + unsigned int ioctl, unsigned long arg) > { > struct kvm_vcpu *vcpu = filp->private_data; > void __user *argp = (void __user *)arg; > - int idx; > - long r; > > switch (ioctl) { > case KVM_S390_IRQ: { > @@ -3752,6 +3750,16 @@ long kvm_arch_vcpu_ioctl(struct file *filp, > return kvm_s390_inject_vcpu(vcpu, &s390irq); > } > } > + return -ENOIOCTLCMD; > +} > + > +long kvm_arch_vcpu_ioctl(struct file *filp, > + unsigned int ioctl, unsigned long arg) > +{ > + struct kvm_vcpu *vcpu = filp->private_data; > + void __user *argp = (void __user *)arg; > + int idx; > + long r; > > vcpu_load(vcpu); > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 09de0ff3d677..a900d20a5320 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -736,6 +736,8 @@ long kvm_arch_dev_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg); > long kvm_arch_vcpu_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg); > +long kvm_arch_vcpu_async_ioctl(struct file *filp, > + unsigned int ioctl, unsigned long arg); > int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf); > > int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext); > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 19c184fa1839..262d6c910fe7 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2547,13 +2547,13 @@ static long kvm_vcpu_ioctl(struct file *filp, > #if defined(CONFIG_S390) || defined(CONFIG_PPC) || defined(CONFIG_MIPS) can we now also get rid of this ugly define? > /* > * Special cases: vcpu ioctls that are asynchronous to vcpu execution, > - * so vcpu_load() would break it. > + * so mutex_lock() would break it. > */ > - if (ioctl == KVM_S390_INTERRUPT || ioctl == KVM_S390_IRQ || ioctl == KVM_INTERRUPT) > - return kvm_arch_vcpu_ioctl(filp, ioctl, arg); > + r = kvm_arch_vcpu_async_ioctl(filp, ioctl, arg); > + if (r != -ENOIOCTLCMD) > + return r; > #endif > > - unrelated change but the right thing to do :) > if (mutex_lock_killable(&vcpu->mutex)) > return -EINTR; > switch (ioctl) { > Looks good to me! -- Thanks, David / dhildenb