From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Hildenbrand Subject: Re: [PATCH] KVM: x86: don't hold kvm->lock in KVM_SET_GSI_ROUTING Date: Wed, 26 Apr 2017 10:40:03 +0200 Message-ID: References: <20170425190306.20392-1-david@redhat.com> <20170425195954.GJ5713@potion> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: srutherford@google.com, kvm@vger.kernel.org, pbonzini@redhat.com To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:50908 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1036725AbdDZIkG (ORCPT ); Wed, 26 Apr 2017 04:40:06 -0400 In-Reply-To: <20170425195954.GJ5713@potion> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: On 25.04.2017 21:59, Radim Krčmář wrote: > 2017-04-25 21:03+0200, David Hildenbrand: >> We needed the lock to avoid racing with creation of the irqchip on x86. As >> kvm_set_irq_routing() calls srcu_synchronize_expedited(), this lock >> might be held for a longer time. >> >> Let's introduce an arch specific callback to check if we can actually >> add irq routes. For x86, all we have to do is check if we have an >> irqchip in the kernel. We don't need kvm->lock at that point as the >> irqchip is marked as inititalized only when actually fully created. >> >> Reported-by: Steve Rutherford >> Fixes: 1df6ddede10a ("KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP") >> Signed-off-by: David Hildenbrand >> --- >> 6 files changed, 18 insertions(+), 21 deletions(-) > > Nice! > > Reviewed-by: Radim Krčmář > >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> @@ -504,6 +504,7 @@ void vcpu_put(struct kvm_vcpu *vcpu); >> #ifdef __KVM_HAVE_IOAPIC >> void kvm_arch_post_irq_ack_notifier_list_update(struct kvm *kvm); >> void kvm_arch_post_irq_routing_update(struct kvm *kvm); >> +bool kvm_arch_can_set_irq_routing(struct kvm *kvm); > > (A nitpick: it might be useful even without __KVM_HAVE_IOAPIC so weak > linking would probably be cleaner for a slow path.) > >> #else >> static inline void kvm_arch_post_irq_ack_notifier_list_update(struct kvm *kvm) >> { >> @@ -511,6 +512,10 @@ static inline void kvm_arch_post_irq_ack_notifier_list_update(struct kvm *kvm) >> static inline void kvm_arch_post_irq_routing_update(struct kvm *kvm) >> { >> } >> +static bool kvm_arch_can_set_irq_routing(struct kvm *kvm) >> +{ >> + return true; >> +} >> #endif >> Makes sense, shall I resend or can you fix that up when applying? Thanks! -- Thanks, David