[{"id":1812504,"web_url":"http://patchwork.ozlabs.org/comment/1812504/","msgid":"<25c1daca-1d8b-48e7-af2d-5cf47d0d278b@redhat.com>","list_archive_url":null,"date":"2017-11-29T17:17:52","subject":"Re: [PATCH v2 01/16] KVM: Take vcpu->mutex outside vcpu_load","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":"On 29.11.2017 17:41, Christoffer Dall wrote:\n> As we're about to call vcpu_load() from architecture-specific\n> implementations of the KVM vcpu ioctls, but yet we access data\n> structures protected by the vcpu->mutex in the generic code, factor\n> this logic out from vcpu_load().\n> \n> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>\n> ---\n>  arch/x86/kvm/vmx.c       |  4 +---\n>  arch/x86/kvm/x86.c       | 20 +++++++-------------\n>  include/linux/kvm_host.h |  2 +-\n>  virt/kvm/kvm_main.c      | 17 ++++++-----------\n>  4 files changed, 15 insertions(+), 28 deletions(-)\n> \n> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c\n> index 714a067..e7c46d2 100644\n> --- a/arch/x86/kvm/vmx.c\n> +++ b/arch/x86/kvm/vmx.c\n> @@ -9559,10 +9559,8 @@ static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)\n>  static void vmx_free_vcpu_nested(struct kvm_vcpu *vcpu)\n>  {\n>         struct vcpu_vmx *vmx = to_vmx(vcpu);\n> -       int r;\n>  \n> -       r = vcpu_load(vcpu);\n> -       BUG_ON(r);\n> +       vcpu_load(vcpu);\nI am most likely missing something, why don't we have to take the lock\nin these cases?\n\n>         vmx_switch_vmcs(vcpu, &vmx->vmcs01);\n>         free_nested(vmx);\n>         vcpu_put(vcpu);\n> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c\n> index 34c85aa..9b8f864 100644\n> --- a/arch/x86/kvm/x86.c\n> +++ b/arch/x86/kvm/x86.c\n> @@ -7747,16 +7747,12 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,\n>  \n>  int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)\n>  {\n> -\tint r;\n> -\n>  \tkvm_vcpu_mtrr_init(vcpu);\n> -\tr = vcpu_load(vcpu);\n> -\tif (r)\n> -\t\treturn r;\n> +\tvcpu_load(vcpu);\n>  \tkvm_vcpu_reset(vcpu, false);\n>  \tkvm_mmu_setup(vcpu);\n>  \tvcpu_put(vcpu);\n> -\treturn r;\n> +\treturn 0;\n>  }\n>  \n>  void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)\n> @@ -7766,13 +7762,15 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)\n>  \n>  \tkvm_hv_vcpu_postcreate(vcpu);\n>  \n> -\tif (vcpu_load(vcpu))\n> +\tif (mutex_lock_killable(&vcpu->mutex))\n>  \t\treturn;\n> +\tvcpu_load(vcpu);\n>  \tmsr.data = 0x0;\n>  \tmsr.index = MSR_IA32_TSC;\n>  \tmsr.host_initiated = true;\n>  \tkvm_write_tsc(vcpu, &msr);\n>  \tvcpu_put(vcpu);\n> +\tmutex_unlock(&vcpu->mutex);\n>  \n>  \tif (!kvmclock_periodic_sync)\n>  \t\treturn;\n> @@ -7783,11 +7781,9 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)\n>  \n>  void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)\n>  {\n> -\tint r;\n>  \tvcpu->arch.apf.msr_val = 0;\n>  \n> -\tr = vcpu_load(vcpu);\n> -\tBUG_ON(r);\n> +\tvcpu_load(vcpu);\n>  \tkvm_mmu_unload(vcpu);\n>  \tvcpu_put(vcpu);\n>  \n> @@ -8155,9 +8151,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)\n>  \n>  static void kvm_unload_vcpu_mmu(struct kvm_vcpu *vcpu)\n>  {\n> -\tint r;\n> -\tr = vcpu_load(vcpu);\n> -\tBUG_ON(r);\n> +\tvcpu_load(vcpu);\n>  \tkvm_mmu_unload(vcpu);\n>  \tvcpu_put(vcpu);\n>  }\n> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h\n> index 2e754b7..a000dd8 100644\n> --- a/include/linux/kvm_host.h\n> +++ b/include/linux/kvm_host.h\n> @@ -533,7 +533,7 @@ static inline int kvm_vcpu_get_idx(struct kvm_vcpu *vcpu)\n>  int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id);\n>  void kvm_vcpu_uninit(struct kvm_vcpu *vcpu);\n>  \n> -int __must_check vcpu_load(struct kvm_vcpu *vcpu);\n> +void vcpu_load(struct kvm_vcpu *vcpu);\n>  void vcpu_put(struct kvm_vcpu *vcpu);\n>  \n>  #ifdef __KVM_HAVE_IOAPIC\n> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c\n> index f169ecc..39961fb 100644\n> --- a/virt/kvm/kvm_main.c\n> +++ b/virt/kvm/kvm_main.c\n> @@ -146,17 +146,12 @@ bool kvm_is_reserved_pfn(kvm_pfn_t pfn)\n>  /*\n>   * Switches to specified vcpu, until a matching vcpu_put()\n>   */\n> -int vcpu_load(struct kvm_vcpu *vcpu)\n> +void vcpu_load(struct kvm_vcpu *vcpu)\n>  {\n> -\tint cpu;\n> -\n> -\tif (mutex_lock_killable(&vcpu->mutex))\n> -\t\treturn -EINTR;\n> -\tcpu = get_cpu();\n> +\tint cpu = get_cpu();\n\nmissing empty line.\n\n>  \tpreempt_notifier_register(&vcpu->preempt_notifier);\n>  \tkvm_arch_vcpu_load(vcpu, cpu);\n>  \tput_cpu();\n> -\treturn 0;\n>  }\n>  EXPORT_SYMBOL_GPL(vcpu_load);\n>  \n> @@ -166,7 +161,6 @@ void vcpu_put(struct kvm_vcpu *vcpu)\n>  \tkvm_arch_vcpu_put(vcpu);\n>  \tpreempt_notifier_unregister(&vcpu->preempt_notifier);\n>  \tpreempt_enable();\n> -\tmutex_unlock(&vcpu->mutex);\n>  }\n>  EXPORT_SYMBOL_GPL(vcpu_put);\n>  \n> @@ -2529,9 +2523,9 @@ static long kvm_vcpu_ioctl(struct file *filp,\n>  #endif\n>  \n>  \n> -\tr = vcpu_load(vcpu);\n> -\tif (r)\n> -\t\treturn r;\n> +\tif (mutex_lock_killable(&vcpu->mutex))\n> +\t\treturn -EINTR;\n> +\tvcpu_load(vcpu);\n>  \tswitch (ioctl) {\n>  \tcase KVM_RUN: {\n>  \t\tstruct pid *oldpid;\n> @@ -2704,6 +2698,7 @@ static long kvm_vcpu_ioctl(struct file *filp,\n>  \t}\n>  out:\n>  \tvcpu_put(vcpu);\n> +\tmutex_unlock(&vcpu->mutex);\n>  \tkfree(fpu);\n>  \tkfree(kvm_sregs);\n>  \treturn r;\n>","headers":{"Return-Path":"<kvm-ppc-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yn6gD6QW1z9s9Y\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 30 Nov 2017 04:18:00 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S933760AbdK2RR6 (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tWed, 29 Nov 2017 12:17:58 -0500","from mx1.redhat.com ([209.132.183.28]:56332 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S932288AbdK2RR5 (ORCPT <rfc822;kvm-ppc@vger.kernel.org>);\n\tWed, 29 Nov 2017 12:17:57 -0500","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 200998553E;\n\tWed, 29 Nov 2017 17:17:57 +0000 (UTC)","from [10.36.117.80] (ovpn-117-80.ams2.redhat.com [10.36.117.80])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id AFEB45D6A8;\n\tWed, 29 Nov 2017 17:17:53 +0000 (UTC)"],"Subject":"Re: [PATCH v2 01/16] KVM: Take vcpu->mutex outside vcpu_load","To":"Christoffer Dall <christoffer.dall@linaro.org>, kvm@vger.kernel.org","Cc":"Andrew Jones <drjones@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>,\n\t=?utf-8?b?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>,\n\tMarc Zyngier <marc.zyngier@arm.com>, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org, James Hogan <jhogan@kernel.org>,\n\tlinux-mips@linux-mips.org, Alexander Graf <agraf@suse.com>,\n\tkvm-ppc@vger.kernel.org, \n\tChristian Borntraeger <borntraeger@de.ibm.com>, Cornelia Huck\n\t<cohuck@redhat.com>, linux-s390@vger.kernel.org","References":"<20171129164116.16167-1-christoffer.dall@linaro.org>\n\t<20171129164116.16167-2-christoffer.dall@linaro.org>","From":"David Hildenbrand <david@redhat.com>","Organization":"Red Hat GmbH","Message-ID":"<25c1daca-1d8b-48e7-af2d-5cf47d0d278b@redhat.com>","Date":"Wed, 29 Nov 2017 18:17:52 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<20171129164116.16167-2-christoffer.dall@linaro.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tWed, 29 Nov 2017 17:17:57 +0000 (UTC)","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<kvm-ppc.vger.kernel.org>","X-Mailing-List":"kvm-ppc@vger.kernel.org"}},{"id":1812508,"web_url":"http://patchwork.ozlabs.org/comment/1812508/","msgid":"<1cf0f391-960c-a457-29e5-f31ee410a9d1@redhat.com>","list_archive_url":null,"date":"2017-11-29T17:20:45","subject":"Re: [PATCH v2 01/16] KVM: Take vcpu->mutex outside vcpu_load","submitter":{"id":2701,"url":"http://patchwork.ozlabs.org/api/people/2701/","name":"Paolo Bonzini","email":"pbonzini@redhat.com"},"content":"On 29/11/2017 18:17, David Hildenbrand wrote:\n> On 29.11.2017 17:41, Christoffer Dall wrote:\n>> As we're about to call vcpu_load() from architecture-specific\n>> implementations of the KVM vcpu ioctls, but yet we access data\n>> structures protected by the vcpu->mutex in the generic code, factor\n>> this logic out from vcpu_load().\n>>\n>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>\n>> ---\n>>  arch/x86/kvm/vmx.c       |  4 +---\n>>  arch/x86/kvm/x86.c       | 20 +++++++-------------\n>>  include/linux/kvm_host.h |  2 +-\n>>  virt/kvm/kvm_main.c      | 17 ++++++-----------\n>>  4 files changed, 15 insertions(+), 28 deletions(-)\n>>\n>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c\n>> index 714a067..e7c46d2 100644\n>> --- a/arch/x86/kvm/vmx.c\n>> +++ b/arch/x86/kvm/vmx.c\n>> @@ -9559,10 +9559,8 @@ static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)\n>>  static void vmx_free_vcpu_nested(struct kvm_vcpu *vcpu)\n>>  {\n>>         struct vcpu_vmx *vmx = to_vmx(vcpu);\n>> -       int r;\n>>  \n>> -       r = vcpu_load(vcpu);\n>> -       BUG_ON(r);\n>> +       vcpu_load(vcpu);\n> I am most likely missing something, why don't we have to take the lock\n> in these cases?\n\nSee earlier discussion, at these points there can be no concurrent\naccess; the file descriptor is not accessible yet, or is already gone.\n\nPaolo\n\n>>         vmx_switch_vmcs(vcpu, &vmx->vmcs01);\n>>         free_nested(vmx);\n>>         vcpu_put(vcpu);\n>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c\n>> index 34c85aa..9b8f864 100644\n>> --- a/arch/x86/kvm/x86.c\n>> +++ b/arch/x86/kvm/x86.c\n>> @@ -7747,16 +7747,12 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,\n>>  \n>>  int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)\n>>  {\n>> -\tint r;\n>> -\n>>  \tkvm_vcpu_mtrr_init(vcpu);\n>> -\tr = vcpu_load(vcpu);\n>> -\tif (r)\n>> -\t\treturn r;\n>> +\tvcpu_load(vcpu);\n>>  \tkvm_vcpu_reset(vcpu, false);\n>>  \tkvm_mmu_setup(vcpu);\n>>  \tvcpu_put(vcpu);\n>> -\treturn r;\n>> +\treturn 0;\n>>  }\n>>  \n>>  void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)\n>> @@ -7766,13 +7762,15 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)\n>>  \n>>  \tkvm_hv_vcpu_postcreate(vcpu);\n>>  \n>> -\tif (vcpu_load(vcpu))\n>> +\tif (mutex_lock_killable(&vcpu->mutex))\n>>  \t\treturn;\n>> +\tvcpu_load(vcpu);\n>>  \tmsr.data = 0x0;\n>>  \tmsr.index = MSR_IA32_TSC;\n>>  \tmsr.host_initiated = true;\n>>  \tkvm_write_tsc(vcpu, &msr);\n>>  \tvcpu_put(vcpu);\n>> +\tmutex_unlock(&vcpu->mutex);\n>>  \n>>  \tif (!kvmclock_periodic_sync)\n>>  \t\treturn;\n>> @@ -7783,11 +7781,9 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)\n>>  \n>>  void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)\n>>  {\n>> -\tint r;\n>>  \tvcpu->arch.apf.msr_val = 0;\n>>  \n>> -\tr = vcpu_load(vcpu);\n>> -\tBUG_ON(r);\n>> +\tvcpu_load(vcpu);\n>>  \tkvm_mmu_unload(vcpu);\n>>  \tvcpu_put(vcpu);\n>>  \n>> @@ -8155,9 +8151,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)\n>>  \n>>  static void kvm_unload_vcpu_mmu(struct kvm_vcpu *vcpu)\n>>  {\n>> -\tint r;\n>> -\tr = vcpu_load(vcpu);\n>> -\tBUG_ON(r);\n>> +\tvcpu_load(vcpu);\n>>  \tkvm_mmu_unload(vcpu);\n>>  \tvcpu_put(vcpu);\n>>  }\n>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h\n>> index 2e754b7..a000dd8 100644\n>> --- a/include/linux/kvm_host.h\n>> +++ b/include/linux/kvm_host.h\n>> @@ -533,7 +533,7 @@ static inline int kvm_vcpu_get_idx(struct kvm_vcpu *vcpu)\n>>  int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id);\n>>  void kvm_vcpu_uninit(struct kvm_vcpu *vcpu);\n>>  \n>> -int __must_check vcpu_load(struct kvm_vcpu *vcpu);\n>> +void vcpu_load(struct kvm_vcpu *vcpu);\n>>  void vcpu_put(struct kvm_vcpu *vcpu);\n>>  \n>>  #ifdef __KVM_HAVE_IOAPIC\n>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c\n>> index f169ecc..39961fb 100644\n>> --- a/virt/kvm/kvm_main.c\n>> +++ b/virt/kvm/kvm_main.c\n>> @@ -146,17 +146,12 @@ bool kvm_is_reserved_pfn(kvm_pfn_t pfn)\n>>  /*\n>>   * Switches to specified vcpu, until a matching vcpu_put()\n>>   */\n>> -int vcpu_load(struct kvm_vcpu *vcpu)\n>> +void vcpu_load(struct kvm_vcpu *vcpu)\n>>  {\n>> -\tint cpu;\n>> -\n>> -\tif (mutex_lock_killable(&vcpu->mutex))\n>> -\t\treturn -EINTR;\n>> -\tcpu = get_cpu();\n>> +\tint cpu = get_cpu();\n> \n> missing empty line.\n> \n>>  \tpreempt_notifier_register(&vcpu->preempt_notifier);\n>>  \tkvm_arch_vcpu_load(vcpu, cpu);\n>>  \tput_cpu();\n>> -\treturn 0;\n>>  }\n>>  EXPORT_SYMBOL_GPL(vcpu_load);\n>>  \n>> @@ -166,7 +161,6 @@ void vcpu_put(struct kvm_vcpu *vcpu)\n>>  \tkvm_arch_vcpu_put(vcpu);\n>>  \tpreempt_notifier_unregister(&vcpu->preempt_notifier);\n>>  \tpreempt_enable();\n>> -\tmutex_unlock(&vcpu->mutex);\n>>  }\n>>  EXPORT_SYMBOL_GPL(vcpu_put);\n>>  \n>> @@ -2529,9 +2523,9 @@ static long kvm_vcpu_ioctl(struct file *filp,\n>>  #endif\n>>  \n>>  \n>> -\tr = vcpu_load(vcpu);\n>> -\tif (r)\n>> -\t\treturn r;\n>> +\tif (mutex_lock_killable(&vcpu->mutex))\n>> +\t\treturn -EINTR;\n>> +\tvcpu_load(vcpu);\n>>  \tswitch (ioctl) {\n>>  \tcase KVM_RUN: {\n>>  \t\tstruct pid *oldpid;\n>> @@ -2704,6 +2698,7 @@ static long kvm_vcpu_ioctl(struct file *filp,\n>>  \t}\n>>  out:\n>>  \tvcpu_put(vcpu);\n>> +\tmutex_unlock(&vcpu->mutex);\n>>  \tkfree(fpu);\n>>  \tkfree(kvm_sregs);\n>>  \treturn r;\n>>\n> \n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe kvm-ppc\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<kvm-ppc-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yn6kf1nCqz9t2V\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 30 Nov 2017 04:20:58 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752236AbdK2RU4 (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tWed, 29 Nov 2017 12:20:56 -0500","from mx1.redhat.com ([209.132.183.28]:14936 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1750783AbdK2RU4 (ORCPT <rfc822;kvm-ppc@vger.kernel.org>);\n\tWed, 29 Nov 2017 12:20:56 -0500","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id D6807C01C108;\n\tWed, 29 Nov 2017 17:20:55 +0000 (UTC)","from [10.36.117.214] (ovpn-117-214.ams2.redhat.com [10.36.117.214])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 7AFEA60F80;\n\tWed, 29 Nov 2017 17:20:47 +0000 (UTC)"],"Subject":"Re: [PATCH v2 01/16] KVM: Take vcpu->mutex outside vcpu_load","To":"David Hildenbrand <david@redhat.com>,\n\tChristoffer Dall <christoffer.dall@linaro.org>, kvm@vger.kernel.org","Cc":"Andrew Jones <drjones@redhat.com>, =?utf-8?b?UmFkaW0gS3LEjW3DocWZ?=\n\t<rkrcmar@redhat.com>,         Marc Zyngier <marc.zyngier@arm.com>,\n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, \n\tJames Hogan <jhogan@kernel.org>, linux-mips@linux-mips.org, \n\tAlexander Graf <agraf@suse.com>, kvm-ppc@vger.kernel.org, \n\tChristian Borntraeger <borntraeger@de.ibm.com>, Cornelia Huck\n\t<cohuck@redhat.com>, linux-s390@vger.kernel.org","References":"<20171129164116.16167-1-christoffer.dall@linaro.org>\n\t<20171129164116.16167-2-christoffer.dall@linaro.org>\n\t<25c1daca-1d8b-48e7-af2d-5cf47d0d278b@redhat.com>","From":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<1cf0f391-960c-a457-29e5-f31ee410a9d1@redhat.com>","Date":"Wed, 29 Nov 2017 18:20:45 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<25c1daca-1d8b-48e7-af2d-5cf47d0d278b@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tWed, 29 Nov 2017 17:20:56 +0000 (UTC)","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<kvm-ppc.vger.kernel.org>","X-Mailing-List":"kvm-ppc@vger.kernel.org"}},{"id":1812510,"web_url":"http://patchwork.ozlabs.org/comment/1812510/","msgid":"<51b1bb38-7fa8-d785-3281-5a239639989e@redhat.com>","list_archive_url":null,"date":"2017-11-29T17:22:29","subject":"Re: [PATCH v2 01/16] KVM: Take vcpu->mutex outside vcpu_load","submitter":{"id":70402,"url":"http://patchwork.ozlabs.org/api/people/70402/","name":"David Hildenbrand","email":"david@redhat.com"},"content":"On 29.11.2017 18:20, Paolo Bonzini wrote:\n> On 29/11/2017 18:17, David Hildenbrand wrote:\n>> On 29.11.2017 17:41, Christoffer Dall wrote:\n>>> As we're about to call vcpu_load() from architecture-specific\n>>> implementations of the KVM vcpu ioctls, but yet we access data\n>>> structures protected by the vcpu->mutex in the generic code, factor\n>>> this logic out from vcpu_load().\n>>>\n>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>\n>>> ---\n>>>  arch/x86/kvm/vmx.c       |  4 +---\n>>>  arch/x86/kvm/x86.c       | 20 +++++++-------------\n>>>  include/linux/kvm_host.h |  2 +-\n>>>  virt/kvm/kvm_main.c      | 17 ++++++-----------\n>>>  4 files changed, 15 insertions(+), 28 deletions(-)\n>>>\n>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c\n>>> index 714a067..e7c46d2 100644\n>>> --- a/arch/x86/kvm/vmx.c\n>>> +++ b/arch/x86/kvm/vmx.c\n>>> @@ -9559,10 +9559,8 @@ static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)\n>>>  static void vmx_free_vcpu_nested(struct kvm_vcpu *vcpu)\n>>>  {\n>>>         struct vcpu_vmx *vmx = to_vmx(vcpu);\n>>> -       int r;\n>>>  \n>>> -       r = vcpu_load(vcpu);\n>>> -       BUG_ON(r);\n>>> +       vcpu_load(vcpu);\n>> I am most likely missing something, why don't we have to take the lock\n>> in these cases?\n> \n> See earlier discussion, at these points there can be no concurrent\n> access; the file descriptor is not accessible yet, or is already gone.\n> \n> Paolo\n\nThanks, this belongs into the patch description then.","headers":{"Return-Path":"<kvm-ppc-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yn6mg0bNHz9t3Z\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 30 Nov 2017 04:22:43 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S934090AbdK2RWh (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tWed, 29 Nov 2017 12:22:37 -0500","from mx1.redhat.com ([209.132.183.28]:59235 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S934059AbdK2RWe (ORCPT <rfc822;kvm-ppc@vger.kernel.org>);\n\tWed, 29 Nov 2017 12:22:34 -0500","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id AB1B9C058ED4;\n\tWed, 29 Nov 2017 17:22:33 +0000 (UTC)","from [10.36.117.80] (ovpn-117-80.ams2.redhat.com [10.36.117.80])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 8A50B5D6A8;\n\tWed, 29 Nov 2017 17:22:30 +0000 (UTC)"],"Subject":"Re: [PATCH v2 01/16] KVM: Take vcpu->mutex outside vcpu_load","To":"Paolo Bonzini <pbonzini@redhat.com>,\n\tChristoffer Dall <christoffer.dall@linaro.org>, kvm@vger.kernel.org","Cc":"Andrew Jones <drjones@redhat.com>, =?utf-8?b?UmFkaW0gS3LEjW3DocWZ?=\n\t<rkrcmar@redhat.com>,         Marc Zyngier <marc.zyngier@arm.com>,\n\tkvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, \n\tJames Hogan <jhogan@kernel.org>, linux-mips@linux-mips.org, \n\tAlexander Graf <agraf@suse.com>, kvm-ppc@vger.kernel.org, \n\tChristian Borntraeger <borntraeger@de.ibm.com>, Cornelia Huck\n\t<cohuck@redhat.com>, linux-s390@vger.kernel.org","References":"<20171129164116.16167-1-christoffer.dall@linaro.org>\n\t<20171129164116.16167-2-christoffer.dall@linaro.org>\n\t<25c1daca-1d8b-48e7-af2d-5cf47d0d278b@redhat.com>\n\t<1cf0f391-960c-a457-29e5-f31ee410a9d1@redhat.com>","From":"David Hildenbrand <david@redhat.com>","Organization":"Red Hat GmbH","Message-ID":"<51b1bb38-7fa8-d785-3281-5a239639989e@redhat.com>","Date":"Wed, 29 Nov 2017 18:22:29 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<1cf0f391-960c-a457-29e5-f31ee410a9d1@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.32]);\n\tWed, 29 Nov 2017 17:22:33 +0000 (UTC)","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<kvm-ppc.vger.kernel.org>","X-Mailing-List":"kvm-ppc@vger.kernel.org"}},{"id":1812529,"web_url":"http://patchwork.ozlabs.org/comment/1812529/","msgid":"<CAMJs5B_sYAQDF6Z3tDD4u8S3gJWns+gR1CE_hrcD83L_wmpEwQ@mail.gmail.com>","list_archive_url":null,"date":"2017-11-29T17:35:16","subject":"Re: [PATCH v2 01/16] KVM: Take vcpu->mutex outside vcpu_load","submitter":{"id":26352,"url":"http://patchwork.ozlabs.org/api/people/26352/","name":"Christoffer Dall","email":"christoffer.dall@linaro.org"},"content":"On Wed, Nov 29, 2017 at 5:22 PM, David Hildenbrand <david@redhat.com> wrote:\n> On 29.11.2017 18:20, Paolo Bonzini wrote:\n>> On 29/11/2017 18:17, David Hildenbrand wrote:\n>>> On 29.11.2017 17:41, Christoffer Dall wrote:\n>>>> As we're about to call vcpu_load() from architecture-specific\n>>>> implementations of the KVM vcpu ioctls, but yet we access data\n>>>> structures protected by the vcpu->mutex in the generic code, factor\n>>>> this logic out from vcpu_load().\n>>>>\n>>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>\n>>>> ---\n>>>>  arch/x86/kvm/vmx.c       |  4 +---\n>>>>  arch/x86/kvm/x86.c       | 20 +++++++-------------\n>>>>  include/linux/kvm_host.h |  2 +-\n>>>>  virt/kvm/kvm_main.c      | 17 ++++++-----------\n>>>>  4 files changed, 15 insertions(+), 28 deletions(-)\n>>>>\n>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c\n>>>> index 714a067..e7c46d2 100644\n>>>> --- a/arch/x86/kvm/vmx.c\n>>>> +++ b/arch/x86/kvm/vmx.c\n>>>> @@ -9559,10 +9559,8 @@ static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)\n>>>>  static void vmx_free_vcpu_nested(struct kvm_vcpu *vcpu)\n>>>>  {\n>>>>         struct vcpu_vmx *vmx = to_vmx(vcpu);\n>>>> -       int r;\n>>>>\n>>>> -       r = vcpu_load(vcpu);\n>>>> -       BUG_ON(r);\n>>>> +       vcpu_load(vcpu);\n>>> I am most likely missing something, why don't we have to take the lock\n>>> in these cases?\n>>\n>> See earlier discussion, at these points there can be no concurrent\n>> access; the file descriptor is not accessible yet, or is already gone.\n>>\n>> Paolo\n>\n> Thanks, this belongs into the patch description then.\n>\nFair enough, I'll add that.\n\nThanks for having a look.\n\n-Christoffer\n--\nTo unsubscribe from this list: send the line \"unsubscribe kvm-ppc\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<kvm-ppc-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"e4Yp7ejg\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yn7404803z9sRn\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 30 Nov 2017 04:36:00 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S934226AbdK2RfU (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tWed, 29 Nov 2017 12:35:20 -0500","from mail-pg0-f66.google.com ([74.125.83.66]:43369 \"EHLO\n\tmail-pg0-f66.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S934260AbdK2RfR (ORCPT\n\t<rfc822;kvm-ppc@vger.kernel.org>); Wed, 29 Nov 2017 12:35:17 -0500","by mail-pg0-f66.google.com with SMTP id b18so1791675pgv.10\n\tfor <kvm-ppc@vger.kernel.org>; Wed, 29 Nov 2017 09:35:17 -0800 (PST)","by 10.100.241.6 with HTTP; Wed, 29 Nov 2017 09:35:16 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=u+Zl8aUd7ZiL7VT1NHATrRqYm86YaspodFN/+Z+E1Sw=;\n\tb=e4Yp7ejgTPX17b35f0Fuzi6aY+qHXHf7DTHYpD4pW+pJatE9I97W8vMyP/ZeUgHU5s\n\t+nPnkjwXNimaNZTyUZvvJPRuaR5wig0M/+0Zjx+wJQGMYfNQRPJq0QPtoMIpxW1cxLqt\n\t8Mbz/aHRO4wsLYy7aXNAjPh2lan0oAfBzUc+o=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=u+Zl8aUd7ZiL7VT1NHATrRqYm86YaspodFN/+Z+E1Sw=;\n\tb=NM8jcMzYs8x52UiG/+bR+oSnSkfFiGmCeLo9BfUt+PPPM/GBI29tri8+TDAJf/WJ2X\n\t4LQ6GPbh6yE4OynxjD0BKuSS3JKQslj0z1sCEL8tL8eTIYu2ffntwatzVCYl7/U1VwrD\n\t1HThq1xFUfbHZyc6oUZJhCen0M+0fvcBi5DVCfFG9tfc7x5tWQnFcJu26wWLlWbwUepg\n\t1ZB3G2vAdQnwZfVq9oF/TQBI1pkxnOKh/m2zk40A3KJZ5ke1B/cnKo9scQlt3kQTyYuh\n\tOB6bronzc2bRqv1sZtrXeYEOdCZ+Y4S5yR+F+J62KkV2VvwAzkobrBQPDifg1n2eh0Wd\n\tQq9g==","X-Gm-Message-State":"AJaThX5FpXnLLDHR8ria2pDlffRqDfkT0SSBJFxzt87LpbSf+ypRmL+k\n\tu+RvC7GF5Yamm4d5Tpz7W0SYUvf90YMSWDwjpjYA4JXu","X-Google-Smtp-Source":"AGs4zMa6y1U7vO/1bDi1i2Cz/ndpC0ksxRfhaBL3W9T3yxrT9VE8/IEnH8BRyM2/05SF+99koby9JChj8Fp9EJXlzHk=","X-Received":"by 10.98.213.71 with SMTP id d68mr3634671pfg.171.1511976917165; \n\tWed, 29 Nov 2017 09:35:17 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<51b1bb38-7fa8-d785-3281-5a239639989e@redhat.com>","References":"<20171129164116.16167-1-christoffer.dall@linaro.org>\n\t<20171129164116.16167-2-christoffer.dall@linaro.org>\n\t<25c1daca-1d8b-48e7-af2d-5cf47d0d278b@redhat.com>\n\t<1cf0f391-960c-a457-29e5-f31ee410a9d1@redhat.com>\n\t<51b1bb38-7fa8-d785-3281-5a239639989e@redhat.com>","From":"Christoffer Dall <christoffer.dall@linaro.org>","Date":"Wed, 29 Nov 2017 17:35:16 +0000","Message-ID":"<CAMJs5B_sYAQDF6Z3tDD4u8S3gJWns+gR1CE_hrcD83L_wmpEwQ@mail.gmail.com>","Subject":"Re: [PATCH v2 01/16] KVM: Take vcpu->mutex outside vcpu_load","To":"David Hildenbrand <david@redhat.com>","Cc":"Paolo Bonzini <pbonzini@redhat.com>, \"kvm@vger.kernel.org\"\n\t<kvm@vger.kernel.org>,         Andrew Jones <drjones@redhat.com>,\n\t=?utf-8?q?Radim_Kr=C4=8Dm?= =?utf-8?b?w6HFmQ==?= <rkrcmar@redhat.com>,\n\tMarc Zyngier <marc.zyngier@arm.com>,\n\t\"kvmarm@lists.cs.columbia.edu\" <kvmarm@lists.cs.columbia.edu>,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t<linux-arm-kernel@lists.infradead.org>, James Hogan <jhogan@kernel.org>,\n\tlinux-mips@linux-mips.org,         Alexander Graf <agraf@suse.com>,\n\tkvm-ppc <kvm-ppc@vger.kernel.org>, Christian Borntraeger\n\t<borntraeger@de.ibm.com>, Cornelia Huck <cohuck@redhat.com>,\n\tlinux-s390@vger.kernel.org","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<kvm-ppc.vger.kernel.org>","X-Mailing-List":"kvm-ppc@vger.kernel.org"}}]