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

[Alex Williamson <alex.williamson@redhat.com>]

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,

Alex