Re: [patch 0/4] move irq protection role to separate lock v2

[Avi Kivity <avi@redhat.com>]

Christian Bornträger wrote:
>> @@ -2053,6 +2054,9 @@ static long kvm_vm_ioctl(struct file *fi
>>
>>  	if (kvm->mm != current->mm)
>>  		return -EIO;
>> +
>> +	mutex_lock(&kvm->vm_ioctl_lock);
>> +
>>  	switch (ioctl) {
>>  	case KVM_CREATE_VCPU:
>>  		r = kvm_vm_ioctl_create_vcpu(kvm, arg);
>> @@ -2228,6 +2232,7 @@ static long kvm_vm_ioctl(struct file *fi
>>  		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
>>  	}
>>  out:
>> +	mutex_unlock(&kvm->vm_ioctl_lock);
>>  	return r;
>>  }
>>     
>
> The thing that looks worrysome is that the s390 version of kvm_arch_vm_ioctl 
> has KVM_S390_INTERRUPT. This allows userspace to inject interrupts - which 
> would be serialized. The thing is, that external interrupts and I/O interrupts 
> are floating - which means they can arrive on all cpus. This is somewhat of a 
> fast path.
> On the other hand, kvm_s390_inject_vm already takes the kvm->lock to protect 
> agains hotplug. With this patch we might be able to remove the kvm->lock in 
> kvm_s390_inject_vm - that would reduce the impact. 
>
> This needs more thinking on our side.
>   

x86 actually shares the same problem.  KVM_IRQ_LINE interrupts may 
arrive at any vcpu.  Furthermore, with irqfd interrupts may be injected 
from userspace (the vm process or other processes) or from the kernel 
(assigned device, kernel virtio-net device).  So we have the same 
motivation to drop this lock and replace it by rcu for the fast paths.

-- 
error compiling committee.c: too many arguments to function