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

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

On Tue, 2012-02-28 at 14:19 +0100, Jan Kiszka wrote:
> PCI 2.3 allows to generically disable IRQ sources at device level. This
> enables us to share legacy IRQs of such devices with other host devices
> when passing them to a guest.
> 
> The new IRQ sharing feature introduced here is optional, user space has
> to request it explicitly.

Is this really true?  Looks like it's automatic.

>  Moreover, user space can inform us about its
> view of PCI_COMMAND_INTX_DISABLE so that we can avoid unmasking the
> interrupt and signaling it if the guest masked it via the virtualized
> PCI config space.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> Changes in v4:
>  - Integrated doc changes as proposed by Alex
>  - Fixed deassign_host_irq /wrt MSI
>  - Fixed kvm_vm_ioctl_set_pci_irq_mask /wrt INTx unmasking of non-2.3
>    devices
> 
>  Documentation/virtual/kvm/api.txt |   41 +++++++
>  arch/x86/kvm/x86.c                |    1 +
>  include/linux/kvm.h               |    6 +
>  include/linux/kvm_host.h          |    2 +
>  virt/kvm/assigned-dev.c           |  209 +++++++++++++++++++++++++++++++-----
>  5 files changed, 230 insertions(+), 29 deletions(-)
[snip]
> +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.
> +			 */
> +		} else if (!(assigned_dev->flags & KVM_DEV_ASSIGN_PCI_2_3)) {
> +			/*
> +			 * Unmask the IRQ line if required. Unmasking at
> +			 * device level will be performed by user space.
> +			 */
> +			spin_lock_irq(&match->intx_lock);
> +			if (match->host_irq_disabled) {
> +				enable_irq(match->host_irq);
> +				match->host_irq_disabled = false;
> +			}
> +			spin_unlock_irq(&match->intx_lock);

This still looks broken.  If we start with a PCI 2.3 device with INTx
disabled, how does this ever kick start to get another interrupt?
Shouldn't we just call kvm_assigned_dev_ack_irq() here and handle both
INTx modes?  Thanks,

Alex