From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 01/15] KVM: Prepare for moving vcpu_load/vcpu_put into arch specific code Date: Mon, 27 Nov 2017 17:53:01 +0100 Message-ID: <838db374-6040-c805-82f3-187a2cdfc40d@redhat.com> References: <20171125205718.7731-1-christoffer.dall@linaro.org> <20171125205718.7731-2-christoffer.dall@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20171125205718.7731-2-christoffer.dall@linaro.org> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-Archive: List-Post: To: Christoffer Dall , kvm@vger.kernel.org Cc: Andrew Jones , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Marc Zyngier , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, James Hogan , linux-mips@linux-mips.org, Alexander Graf , kvm-ppc@vger.kernel.org, Christian Borntraeger , Cornelia Huck , linux-s390@vger.kernel.org List-ID: On 25/11/2017 21:57, Christoffer Dall wrote: > In preparation for moving calls to vcpu_load() and vcpu_put() into the > architecture specific implementations of the KVM vcpu ioctls, move the > calls in the main kvm_vcpu_ioctl() dispatcher function to each case > of the ioctl select statement. This allows us to move the vcpu_load() > and vcpu_put() calls into architecture specific implementations of vcpu > ioctls, one by one. > > Signed-off-by: Christoffer Dall > --- > virt/kvm/kvm_main.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 48 insertions(+), 5 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 9deb5a245b83..fafafcc38b5a 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2528,16 +2528,15 @@ static long kvm_vcpu_ioctl(struct file *filp, > return kvm_arch_vcpu_ioctl(filp, ioctl, arg); > #endif > > - > - r = vcpu_load(vcpu); > - if (r) > - return r; > switch (ioctl) { > case KVM_RUN: { > struct pid *oldpid; > r = -EINVAL; > if (arg) > goto out; > + r = vcpu_load(vcpu); > + if (r) > + goto out; > oldpid = rcu_access_pointer(vcpu->pid); If it is not a problem for ARM, maybe it would actually be best to leave the locking in kvm_vcpu_ioctl (with the already existing exception of KVM_INTERRUPT). This would make vcpu_load void, and would also let you keep the PID adjustment in common code. This would be more similar to the previous version, but without introducing __vcpu_load/__vcpu_put. Looks good apart from this doubt! Thanks, Paolo From mboxrd@z Thu Jan 1 00:00:00 1970 From: pbonzini@redhat.com (Paolo Bonzini) Date: Mon, 27 Nov 2017 17:53:01 +0100 Subject: [PATCH 01/15] KVM: Prepare for moving vcpu_load/vcpu_put into arch specific code In-Reply-To: <20171125205718.7731-2-christoffer.dall@linaro.org> References: <20171125205718.7731-1-christoffer.dall@linaro.org> <20171125205718.7731-2-christoffer.dall@linaro.org> Message-ID: <838db374-6040-c805-82f3-187a2cdfc40d@redhat.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 25/11/2017 21:57, Christoffer Dall wrote: > In preparation for moving calls to vcpu_load() and vcpu_put() into the > architecture specific implementations of the KVM vcpu ioctls, move the > calls in the main kvm_vcpu_ioctl() dispatcher function to each case > of the ioctl select statement. This allows us to move the vcpu_load() > and vcpu_put() calls into architecture specific implementations of vcpu > ioctls, one by one. > > Signed-off-by: Christoffer Dall > --- > virt/kvm/kvm_main.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 48 insertions(+), 5 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 9deb5a245b83..fafafcc38b5a 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2528,16 +2528,15 @@ static long kvm_vcpu_ioctl(struct file *filp, > return kvm_arch_vcpu_ioctl(filp, ioctl, arg); > #endif > > - > - r = vcpu_load(vcpu); > - if (r) > - return r; > switch (ioctl) { > case KVM_RUN: { > struct pid *oldpid; > r = -EINVAL; > if (arg) > goto out; > + r = vcpu_load(vcpu); > + if (r) > + goto out; > oldpid = rcu_access_pointer(vcpu->pid); If it is not a problem for ARM, maybe it would actually be best to leave the locking in kvm_vcpu_ioctl (with the already existing exception of KVM_INTERRUPT). This would make vcpu_load void, and would also let you keep the PID adjustment in common code. This would be more similar to the previous version, but without introducing __vcpu_load/__vcpu_put. Looks good apart from this doubt! Thanks, Paolo From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Date: Mon, 27 Nov 2017 16:53:01 +0000 Subject: Re: [PATCH 01/15] KVM: Prepare for moving vcpu_load/vcpu_put into arch specific code Message-Id: <838db374-6040-c805-82f3-187a2cdfc40d@redhat.com> List-Id: References: <20171125205718.7731-1-christoffer.dall@linaro.org> <20171125205718.7731-2-christoffer.dall@linaro.org> In-Reply-To: <20171125205718.7731-2-christoffer.dall@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Christoffer Dall , kvm@vger.kernel.org Cc: Andrew Jones , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Marc Zyngier , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, James Hogan , linux-mips@linux-mips.org, Alexander Graf , kvm-ppc@vger.kernel.org, Christian Borntraeger , Cornelia Huck , linux-s390@vger.kernel.org On 25/11/2017 21:57, Christoffer Dall wrote: > In preparation for moving calls to vcpu_load() and vcpu_put() into the > architecture specific implementations of the KVM vcpu ioctls, move the > calls in the main kvm_vcpu_ioctl() dispatcher function to each case > of the ioctl select statement. This allows us to move the vcpu_load() > and vcpu_put() calls into architecture specific implementations of vcpu > ioctls, one by one. > > Signed-off-by: Christoffer Dall > --- > virt/kvm/kvm_main.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 48 insertions(+), 5 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 9deb5a245b83..fafafcc38b5a 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2528,16 +2528,15 @@ static long kvm_vcpu_ioctl(struct file *filp, > return kvm_arch_vcpu_ioctl(filp, ioctl, arg); > #endif > > - > - r = vcpu_load(vcpu); > - if (r) > - return r; > switch (ioctl) { > case KVM_RUN: { > struct pid *oldpid; > r = -EINVAL; > if (arg) > goto out; > + r = vcpu_load(vcpu); > + if (r) > + goto out; > oldpid = rcu_access_pointer(vcpu->pid); If it is not a problem for ARM, maybe it would actually be best to leave the locking in kvm_vcpu_ioctl (with the already existing exception of KVM_INTERRUPT). This would make vcpu_load void, and would also let you keep the PID adjustment in common code. This would be more similar to the previous version, but without introducing __vcpu_load/__vcpu_put. Looks good apart from this doubt! Thanks, Paolo