Re: [PATCH] KVM: Allow host IRQ sharing for assigned PCI 2.3 devices

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-01-09 23:05, Alex Williamson wrote:
> On Mon, 2012-01-09 at 22:25 +0100, Jan Kiszka wrote:
>> On 2012-01-09 20:45, Alex Williamson wrote:
>>> On Mon, 2012-01-09 at 15:03 +0100, Jan Kiszka wrote:
>>>> +static int kvm_vm_ioctl_set_pci_irq_mask(struct kvm *kvm,
>>>> +		struct kvm_assigned_pci_dev *assigned_dev)
>>>> +{
>>>> +	int r = 0;
>>>> +	struct kvm_assigned_dev_kernel *match;
>>>> +
>>>> +	mutex_lock(&kvm->lock);
>>>> +
>>>> +	match = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
>>>> +				      assigned_dev->assigned_dev_id);
>>>> +	if (!match) {
>>>> +		r = -ENODEV;
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +	mutex_lock(&match->intx_mask_lock);
>>>> +
>>>> +	match->flags &= ~KVM_DEV_ASSIGN_MASK_INTX;
>>>> +	match->flags |= assigned_dev->flags & KVM_DEV_ASSIGN_MASK_INTX;
>>>> +
>>>> +	if (match->irq_requested_type & KVM_DEV_IRQ_GUEST_INTX) {
>>>> +		if (assigned_dev->flags & KVM_DEV_ASSIGN_MASK_INTX) {
>>>> +			kvm_set_irq(match->kvm, match->irq_source_id,
>>>> +				    match->guest_irq, 0);
>>>> +			/*
>>>> +			 * Masking at hardware-level is performed on demand,
>>>> +			 * i.e. when an IRQ actually arrives at the host.
>>>> +			 */
>>>
>>> Is there any harm in doing this synchronous to the ioctl?  We're on a
>>> slow path here anyway since the mask is likely drive by a config space
>>> write.
>>
>> Not sure, maybe locking. What would be the advantage of doing it
>> synchronously?
> 
> It would just be a closer match to hardware.  I'm wondering (FUD) if
> there could be a case where a driver does some sensitive operations on
> the device that could be interfered with if the device generates that
> one last interrupt to actually disable interrupts instead of them being
> disabled after setting config space.

The guest driver will never see such an interrupt as we will notice on
its arrival that there is some mask pending.

>  It's probably a long shot, but
> doesn't seem too difficult to switch to synchronous disabling.

It is a bit as we have no PCI API in place to implement this. We only
have check-and-mask which does not mask if there is no IRQ raised. How
do you handle this in VFIO so far?

Really, I do not see an urgent need for synchronous masking and would
rather refrain from it until we are aware of a real problem with
asynchronous one as implemented here.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux