On Tue, 2012-01-10 at 14:47 +0100, Jan Kiszka wrote:
> 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.
Right, I was thinking more about the affect at the hardware level.
> > 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.
Can't we use either pci_intx(pdev, 0) for pci 2.3 devices or for older
devices disable the interrupt handler (if configured) with
disable_irq_nosync(pdev->irq).
> How
> do you handle this in VFIO so far?
As suggested above, but I just fixed it, will checkin soon.
> 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.
Fair enough. Thanks,
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
