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
signature.asc
Description: OpenPGP digital signature