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

[Thomas Goetz <tcgoetz@xxxxxxxxx>]

On May 24, 2011, at 9:53 AM, Stefano Stabellini wrote:

> On Mon, 23 May 2011, Thomas Goetz wrote:
>> I'm running with the attached workaround and I'm the PS/2 issue is gone.
>> 
>> drivers/xen/events.c :: xen_map_pirq_gsi
>> 
>>        if ((strcmp(name, "ioapic-edge") != 0) && pirq_needs_eoi(irq)) {
>>                set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
>>                                handle_fasteoi_irq, name);
>>        } else {
>>                set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
>>                                handle_edge_irq, name);
>>        }
> 
> I had the same idea while reading your previous email.
> I think the following patch is better than strcmp name:
> 
> ---
> 
> Use the trigger info we already have to choose the irq handler
> 
> Do not use pirq_needs_eoi to decide which irq handler to use because Xen
> always returns true if the guest does not support pirq_eoi_map.
> Use the trigger information we already have from MP-tables and ACPI.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 7f676f8..8418398 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -627,6 +627,9 @@ int xen_allocate_pirq_gsi(unsigned gsi)
>  *
>  * Note: We don't assign an event channel until the irq actually started
>  * up.  Return an existing irq if we've already got one for the gsi.
> + *
> + * Shareable implies level triggered, not shareable implies edge
> + * triggered here.
>  */
> int xen_bind_pirq_gsi_to_irq(unsigned gsi,
>                            unsigned pirq, int shareable, char *name)
> @@ -665,16 +668,13 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
> 
>       pirq_query_unmask(irq);
>       /* We try to use the handler with the appropriate semantic for the
> -      * type of interrupt: if the interrupt doesn't need an eoi
> -      * (pirq_needs_eoi returns false), we treat it like an edge
> -      * triggered interrupt so we use handle_edge_irq.
> -      * As a matter of fact this only happens when the corresponding
> -      * physical interrupt is edge triggered or an msi.
> +      * type of interrupt: if the interrupt is an edge triggered
> +      * interrupt we use handle_edge_irq.
>        *
> -      * On the other hand if the interrupt needs an eoi (pirq_needs_eoi
> -      * returns true) we treat it like a level triggered interrupt so we
> -      * use handle_fasteoi_irq like the native code does for this kind of
> +      * On the other hand if the interrupt is level triggered we use
> +      * handle_fasteoi_irq like the native code does for this kind of
>        * interrupts.
> +      *
>        * Depending on the Xen version, pirq_needs_eoi might return true
>        * not only for level triggered interrupts but for edge triggered
>        * interrupts too. In any case Xen always honors the eoi mechanism,
> @@ -682,7 +682,7 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
>        * hasn't received an eoi yet. Therefore using the fasteoi handler
>        * is the right choice either way.
>        */
> -     if (pirq_needs_eoi(irq))
> +     if (shareable)
>               irq_set_chip_and_handler_name(irq, &xen_pirq_chip,
>                               handle_fasteoi_irq, name);
>       else
> 


I tested this patch and it worked fine for me. It will go into nightly testing 
tonight and I'll let you know if we hit any issues.

---
Tom Goetz
tcgoetz@xxxxxxxxx





_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel