Re: [UNTESTED] KVM: do not call kvm_set_irq from irq disabled section

[Avi Kivity <avi@redhat.com>]

On 04/21/2010 07:03 PM, Marcelo Tosatti wrote:
>>> diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
>>> index 47ca447..7ac7bbe 100644
>>> --- a/virt/kvm/assigned-dev.c
>>> +++ b/virt/kvm/assigned-dev.c
>>> @@ -64,24 +64,33 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
>>>   				    interrupt_work);
>>>   	kvm = assigned_dev->kvm;
>>>
>>> -	spin_lock_irq(&assigned_dev->assigned_dev_lock);
>>>   	if (assigned_dev->irq_requested_type&   KVM_DEV_IRQ_HOST_MSIX) {
>>>   		struct kvm_guest_msix_entry *guest_entries =
>>>   			assigned_dev->guest_msix_entries;
>>> +
>>> +		spin_lock_irq(&assigned_dev->assigned_dev_lock);
>>>   		for (i = 0; i<   assigned_dev->entries_nr; i++) {
>>>   			if (!(guest_entries[i].flags&
>>>   					KVM_ASSIGNED_MSIX_PENDING))
>>>   				continue;
>>>   			guest_entries[i].flags&= ~KVM_ASSIGNED_MSIX_PENDING;
>>> +			/*
>>> + 			 * If kvm_assigned_dev_intr sets pending for an
>>> + 			 * entry smaller than this work instance is
>>> + 			 * currently processing, a new work instance
>>> + 			 * will be queued.
>>> + 			 */
>>> +			spin_unlock_irq(&assigned_dev->assigned_dev_lock);
>>>        
>> What happens if assigned_dev->entries_nr is changed here?
>>      
> It does not. Interrupts and work instances are stopped before
> irq_requested_type or entries_nr can change.
>    

Interesting.  A comment please.

>    
>>>   			kvm_set_irq(assigned_dev->kvm,
>>>   				    assigned_dev->irq_source_id,
>>>   				    guest_entries[i].vector, 1);
>>> +			spin_lock_irq(&assigned_dev->assigned_dev_lock);
>>>   		}
>>> +		spin_unlock_irq(&assigned_dev->assigned_dev_lock);
>>>   	} else
>>>   		kvm_set_irq(assigned_dev->kvm, assigned_dev->irq_source_id,
>>>   			    assigned_dev->guest_irq, 1);
>>>
>>> -	spin_unlock_irq(&assigned_dev->assigned_dev_lock);
>>>   }
>>>        
>> A less dangerous fix is to copy all vectors to be triggered into a
>> local array, drop the lock, and replay the array into kvm_set_irq().
>> The else branch could do this as well.
>>      
> Its a bit large, 256 max vectors.
>    

Yeah.

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