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 <srutherford@xxxxxxxxxx>
>> Fixes: 1df6ddede10a ("KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP")
>> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
>> ---
>> 6 files changed, 18 insertions(+), 21 deletions(-)
>
> Nice!
>
> Reviewed-by: Radim Krčmář <rkrcmar@xxxxxxxxxx>
>
>> 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
