Re: [PATCH 1/2] KVM: Handle multiple interrupt sources

[Sheng Yang <sheng@linux.intel.com>]

On Saturday 11 October 2008 16:10:51 Amit Shah wrote:
> From: Sheng Yang <sheng@linux.intel.com>
>
> Keep a record of current interrupt state before injecting. Don't
> assert/deassert repeatedly, so that every caller of kvm_set_irq()
> can be identified as a separate interrupt source for the IOAPIC/PIC
> to implement logical OR of level triggered interrupts on one IRQ line.
>
> Notice that userspace devices are treated as one device for each IRQ
> line. The correctness of sharing interrupt for each IRQ line should be
> ensured by the userspace program (QEmu).
>
> [Amit: rebase to kvm.git HEAD]

Hi, Amit

Thanks for your work! 

But maybe I miss something. I suppose my later patch can work indepently? I 
think the second patch should solve the whole problem (sorry to reply it to 
the second rather than [0/2] which made confusion...). Can you have a check?

Thanks!
--
regards
Yang, Sheng
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  arch/x86/kvm/x86.c       |   13 ++++++++++++-
>  include/linux/kvm_host.h |    3 +++
>  virt/kvm/kvm_main.c      |   12 +++++++++---
>  3 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index dda478e..6f45428 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1816,7 +1816,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
>  			goto out;
>  		if (irqchip_in_kernel(kvm)) {
>  			mutex_lock(&kvm->lock);
> -			kvm_set_irq(kvm, irq_event.irq, irq_event.level);
> +			/*
> +			 * Take one IRQ line as from one device, shared IRQ
> +			 * line should also be handled in the userspace before
> +			 * use KVM_IRQ_LINE ioctl to change IRQ line state.
> +			 */
> +			if (kvm->userspace_intrsource_states[irq_event.irq]
> +					!= irq_event.level) {
> +				kvm_set_irq(kvm, irq_event.irq,
> +					    irq_event.level);
> +				kvm->userspace_intrsource_states[irq_event.irq]
> +						= irq_event.level;
> +			}
>  			mutex_unlock(&kvm->lock);
>  			r = 0;
>  		}
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 3833c48..d392e31 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -129,6 +129,8 @@ struct kvm {
>  	unsigned long mmu_notifier_seq;
>  	long mmu_notifier_count;
>  #endif
> +
> +	int userspace_intrsource_states[KVM_IOAPIC_NUM_PINS];
>  };
>
>  /* The guest did something we don't support. */
> @@ -306,6 +308,7 @@ struct kvm_assigned_dev_kernel {
>  	int host_irq;
>  	int guest_irq;
>  	int irq_requested;
> +	int irq_state;
>  	struct pci_dev *dev;
>  	struct kvm *kvm;
>  };
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index cf0ab8e..faa56fb 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -104,8 +104,11 @@ static void
> kvm_assigned_dev_interrupt_work_handler(struct work_struct *work) *
> finer-grained lock, update this
>  	 */
>  	mutex_lock(&assigned_dev->kvm->lock);
> -	kvm_set_irq(assigned_dev->kvm,
> -		    assigned_dev->guest_irq, 1);
> +	if (assigned_dev->irq_state == 0) {
> +		kvm_set_irq(assigned_dev->kvm,
> +			    assigned_dev->guest_irq, 1);
> +		assigned_dev->irq_state = 1;
> +	}
>  	mutex_unlock(&assigned_dev->kvm->lock);
>  	kvm_put_kvm(assigned_dev->kvm);
>  }
> @@ -134,7 +137,10 @@ static void kvm_assigned_dev_ack_irq(struct
> kvm_irq_ack_notifier *kian)
>
>  	dev = container_of(kian, struct kvm_assigned_dev_kernel,
>  			   ack_notifier);
> -	kvm_set_irq(dev->kvm, dev->guest_irq, 0);
> +	if (dev->irq_state == 1) {
> +		kvm_set_irq(dev->kvm, dev->guest_irq, 0);
> +		dev->irq_state = 0;
> +	}
>  	enable_irq(dev->host_irq);
>  }