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

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2141 bytes --]

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.  It's probably a long shot, but
> doesn't seem too difficult to switch to synchronous disabling.  Thanks,

Need to check again - but there could be a good reason here as well.
Like for kvm_assigned_dev_raise_guest_irq: it handles different guest
IRQ types while its callers have different host IRQ types. So it has its
purpose.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]