On 12/24/22 09:52, Paolo Bonzini wrote:
> On 12/22/22 21:30, Michal Luczaj wrote:
>> + idx = srcu_read_lock(&kvm->srcu);
>> +
>> mutex_lock(&kvm->lock);
>> evtchnfd = idr_find(&kvm->arch.xen.evtchn_ports, port);
>> mutex_unlock(&kvm->lock);
>>
>
> This lock/unlock pair can cause a deadlock because it's inside the SRCU
> read side critical section. Fortunately it's simpler to just use
> mutex_lock for the whole function instead of using two small critical
> sections, and then SRCU is not needed.
Ah, I didn't think using a single mutex_lock would be ok. And maybe that's
a silly question, but how can this lock/unlock pair cause a deadlock? My
assumption was it's a *sleepable* RCU after all.
> However, the same issue exists in kvm_xen_hcall_evtchn_send where
> idr_find is not protected by kvm->lock. In that case, it is important
> to use rcu_read_lock/unlock() around it, because idr_remove() will *not*
> use synchronize_srcu() to wait for readers to complete.
Isn't kvm_xen_hcall_evtchn_send() already protected by srcu_read_lock()?
vcpu_enter_guest
kvm_vcpu_srcu_read_lock <--
kvm_x86_handle_exit
vmx_handle_exit
__vmx_handle_exit (via kvm_vmx_exit_handlers)
kvm_emulate_hypercall
kvm_xen_hypercall
kvm_xen_hcall_evtchn_send
Thanks,
Michal
