[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] Re: Losing PS/2 Interrupts

  • To: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
  • From: Thomas Goetz <tcgoetz@xxxxxxxxx>
  • Date: Mon, 23 May 2011 13:28:28 -0400
  • Cc: Simon Graham <simon.graham@xxxxxxxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx, Jan Beulich <JBeulich@xxxxxxxxxx>, Konrad Rzeszutek Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
  • Delivery-date: Mon, 23 May 2011 10:29:25 -0700
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:mime-version:content-type:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to:x-mailer; b=aFvdS0+y93AHnHU+Hkbq5w4gzmwMtN7cdGjRzFhIO4+q77cIFZKVadyzaWuODt/NRn 2PkRDlKuLDSwWJRYTAXr/DYKffP9fWPQoZ1gb/y0AF6pqe9n5TcyJVgm7ZH+3SarS3fj 4OEOFsiol5Y03GrY6oz5ZUYrBQmrUtEBPn+5o=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>

On May 23, 2011, at 1:16 PM, Thomas Goetz wrote:

> On May 23, 2011, at 9:45 AM, Stefano Stabellini wrote:
>> On Mon, 23 May 2011, Thomas Goetz wrote:
>>> My assumption is that at the point that the i8042 driver reads the data 
>>> register a new interrupt happens. There is gap in
>>> time between when the data register is read and when the event channel 
>>> pending state is cleared. Since the hypervisor
>>> ACKed the previous real interrupt before delivering it to the guest, there 
>>> is nothing to stop the i8042 device from
>>> interrupting immediately after the data register is read. If it interrupt 
>>> before the event channel pending state is
>>> cleared, then it will not be delivered to the guest and the EOI mechanism 
>>> will be set up, but I haven't found anything in
>>> that that will set up a delayed delivery of the second interrupt.
>>> In this situation the i8042 device has every reason to believe the second 
>>> interrupt will be delivered. The previous
>>> interrupt was received and handled. Nothing is masked.
>>> Am I missing something?
>> I am assuming you have the latest version of my fixes to
>> drivers/xen/events.c
> I'll have a version ported from your 2.6.39 tree to my 2.6.38 tree. I'll 
> update my copy of your tree and make sure it's up to date.
>> The problem you are describing shouldn't happen because the interrupt
>> handler returned by request_irq to i8042 is handle_edge_irq that calls
>> chip->irq_ack() before handle_irq_event().
> I checked on which method it is using and it's using handle_fasteoi_irq. In 
> fatc all of the IRQs under 16 are despite most being edge. Log snippet below. 
> I'm looking into why pirq_needs_eoi is returning the wrong answer now.

pirq_needs_eoi checks info->u.pirq.flags & PIRQ_NEEDS_EOI. PIRQ_NEEDS_EOI is 
only set by pirq_query_unmask which sets it based on the hypercall 
PHYSDEVOP_irq_status_query which in Xen 4.0.1 and Xen unstable always returns 
an EOI is needed. Stefano, I don't see any changes in your 2.6.39 tree that 
would effect this.

Relevant code snippets included below:

        if (pirq_needs_eoi(irq)) {
                printk(KERN_ERR "%s: irq %d handle_fasteoi_irq\n", 
__FUNCTION__, irq);
                set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
                                handle_fasteoi_irq, name);
        } else {
                printk(KERN_ERR "%s: irq %d handle_edge_irq\n", __FUNCTION__, 
                set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
                                handle_edge_irq, name);

static bool pirq_needs_eoi(unsigned irq)
        struct irq_info *info = info_for_irq(irq);

        BUG_ON(info->type != IRQT_PIRQ);

        return info->u.pirq.flags & PIRQ_NEEDS_EOI;

static void pirq_query_unmask(int irq)
        struct physdev_irq_status_query irq_status;
        struct irq_info *info = info_for_irq(irq);

        BUG_ON(info->type != IRQT_PIRQ);

        irq_status.irq = pirq_from_irq(irq);
        if (HYPERVISOR_physdev_op(PHYSDEVOP_irq_status_query, &irq_status))
                irq_status.flags = 0;

        printk(KERN_ERR "%s: irq %d needs eoi %d\n", __FUNCTION__, irq, 
(irq_status.flags & XENIRQSTAT_needs_eoi) == XENIRQSTAT_needs_eoi);

        info->u.pirq.flags &= ~PIRQ_NEEDS_EOI;
        if (irq_status.flags & XENIRQSTAT_needs_eoi)
                info->u.pirq.flags |= PIRQ_NEEDS_EOI;

    case PHYSDEVOP_irq_status_query: {
        struct physdev_irq_status_query irq_status_query;
        ret = -EFAULT;
        if ( copy_from_guest(&irq_status_query, arg, 1) != 0 )
        irq = irq_status_query.irq;
        ret = -EINVAL;
        if ( (irq < 0) || (irq >= v->domain->nr_pirqs) )
        irq_status_query.flags = 0;
         * Even edge-triggered or message-based IRQs can need masking from
         * time to time. If teh guest is not dynamically checking for this
         * via the new pirq_eoi_map mechanism, it must conservatively always
         * execute the EOI hypercall. In practice, this only really makes a
         * difference for maskable MSI sources, and if those are supported
         * then dom0 is probably modern anyway.
        irq_status_query.flags |= XENIRQSTAT_needs_eoi;
        if ( pirq_shared(v->domain, irq) )
            irq_status_query.flags |= XENIRQSTAT_shared;
        ret = copy_to_guest(arg, &irq_status_query, 1) ? -EFAULT : 0;

Tom Goetz

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.