On Thu, 2008-07-17 at 09:02 +0300, Avi Kivity wrote:
> Han, Weidong wrote:
> > Avi Kivity wrote:
> >
> >>> +static void kvm_pci_pt_work_fn(struct work_struct *work) +{
> >>> + struct kvm_pci_pt_dev_list *match;
> >>> + struct kvm_pci_pt_work *int_work;
> >>> + int source;
> >>> + unsigned long flags;
> >>> + int guest_irq;
> >>> + int host_irq;
> >>> +
> >>> + int_work = container_of(work, struct kvm_pci_pt_work, work); +
> >>> + source = int_work->source ? KVM_PT_SOURCE_IRQ_ACK :
> >>> KVM_PT_SOURCE_IRQ; + + /* This is taken to safely inject irq
> >>>
> > inside
> >
> >>> the guest. When + * the interrupt injection (or the ioapic code)
> >>> uses a + * finer-grained lock, update this
> >>> + */
> >>> + mutex_lock(&int_work->kvm->lock);
> >>> + read_lock_irqsave(&kvm_pci_pt_lock, flags);
> >>> + match =
> >>>
> > kvm_find_pci_pt_dev(&int_work->kvm->arch.pci_pt_dev_head,
> >
> >>> NULL, + int_work->irq, source);
> >>> + if (!match) {
> >>> + printk(KERN_ERR "%s: no matching device assigned to
> >>>
> > guest "
> >
> >>> + "found for irq %d, source = %d!\n",
> >>> + __func__, int_work->irq, int_work->source);
> >>> + read_unlock_irqrestore(&kvm_pci_pt_lock, flags); +
> >>>
> > goto out;
> >
> >>> + }
> >>> + guest_irq = match->pt_dev.guest.irq;
> >>> + host_irq = match->pt_dev.host.irq;
> >>> + read_unlock_irqrestore(&kvm_pci_pt_lock, flags);
> >>> +
> >>> + if (source == KVM_PT_SOURCE_IRQ)
> >>> + kvm_set_irq(int_work->kvm, guest_irq, 1);
> >>> + else {
> >>> + kvm_set_irq(int_work->kvm, int_work->irq, 0);
> >>> + enable_irq(host_irq);
> >>> + }
> >>> +out:
> >>> + mutex_unlock(&int_work->kvm->lock);
> >>> + kvm_put_kvm(int_work->kvm);
> >>> +}
> >>>
> >>> +
> >>> +/* FIXME: Implement the OR logic needed to make shared interrupts
> >>> on + * this line behave properly + */
> >>>
> >>>
> >> Isn't this a showstopper? There is no easy way for a user to avoid
> >> sharing, especially as we have only three pci irqs at present.
What do you mean by only 3 (for passthrough we use whatever interrupt
the host is using for that device)?
> >>
> >>
> >
> > Currently it's not easy to avoid sharing. I think we can support MSI for
> > assgined device to solve sharing problem.
> >
>
> MSI is definitely the right direction, but we also need to support
> the
> OR logic for guests that do not support MSI.
>
The main problems that I am ware of with shared interrupt are as
follows:
1. Protection
A bad guest can keep the interrupt line up, and if its shared then it
interferes with other devices that might not be assigned to this guest.
2. Performance
The current implementation disables the irq when the interrupt is
received on the host and re-enables it when the guest acks the interrupt
on the virtual ioapic.
Clearly its not nice to do that when other devices uses the same irq...
When removing the enable-disable of the irq, it causes serious
performance degradation.
The throughput goes down from 700 to 400 Mb/sec.
The number of interrupts per second on the host (from the NIC) is 130000
(compared to 22200 before).
Regardless, I think that the right way to go would be to merge the
current code (with the non-shared interrupt limitation) and then
continue to work on additional features like shared interrupts support.
Do you agree?
Regards,
Ben
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
