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
