Re: [kvm-devel] [PATCH 3/3] virtio PCI device

[Anthony Liguori <aliguori@us.ibm.com>]

Dor Laor wrote:
> Anthony Liguori wrote:
> > This is a PCI device that implements a transport for virtio.  It 
> > allows virtio
> > devices to be used by QEMU based VMMs like KVM or Xen.
> >
> > ....
> >   
> While it's a little premature, we can start thinking of irq path 
> improvements.
> The current patch acks a private isr and afterwards apic eoi will also 
> be hit since its
> a level trig irq. This means 2 vmexits per irq.
> We can start with regular pci irqs and move afterwards to msi.
> Some other ugly hack options [we're better use msi]:
>    - Read the eoi directly from apic and save the first private isr ack

I must admit, that I don't know a whole lot about interrupt delivery.  
If we can avoid the private ISR ack then that would certainly be a good 
thing to do!  I think that would involve adding another bit to the 
virtqueues to indicate whether or not there is work to be handled.  It's 
really just moving the ISR to shared memory so that there's no plenty 
for accessing it.

Regards,

Anthony Liguori

>    - Convert the specific irq line to edge triggered and dont share it
> What do you guys think?
> > +/* A small wrapper to also acknowledge the interrupt when it's handled.
> > + * I really need an EIO hook for the vring so I can ack the 
> > interrupt once we
> > + * know that we'll be handling the IRQ but before we invoke the 
> > callback since
> > + * the callback may notify the host which results in the host 
> > attempting to
> > + * raise an interrupt that we would then mask once we acknowledged the
> > + * interrupt. */
> > +static irqreturn_t vp_interrupt(int irq, void *opaque)
> > +{
> > +    struct virtio_pci_device *vp_dev = opaque;
> > +    struct virtio_pci_vq_info *info;
> > +    irqreturn_t ret = IRQ_NONE;
> > +    u8 isr;
> > +
> > +    /* reading the ISR has the effect of also clearing it so it's very
> > +     * important to save off the value. */
> > +    isr = ioread8(vp_dev->ioaddr + VIRTIO_PCI_ISR);
> > +
> > +    /* It's definitely not us if the ISR was not high */
> > +    if (!isr)
> > +        return IRQ_NONE;
> > +
> > +    spin_lock(&vp_dev->lock);
> > +    list_for_each_entry(info, &vp_dev->virtqueues, node) {
> > +        if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
> > +            ret = IRQ_HANDLED;
> > +    }
> > +    spin_unlock(&vp_dev->lock);
> > +
> > +    return ret;
> > +}
> >   

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/