Re: [PATCH] x86: irq_move_cleanup_interrupt() must ignore legacy vectors

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 28/03/2013 15:24, Jan Beulich wrote:
> Since the main loop in the function includes legacy vectors, and since
> vector_irq[] gets set up for legacy vectors regardless of whether those
> get handled through the IO-APIC, it must not do anything on this vector
> range. In fact, we should never get here for IRQs not handled through
> the IO-APIC, so add a respective assertion at once. For that assertion
> to not have false positives we however need to suppress setting up IRQ2
> as an 8259A interrupt (which wasn't correct anyway).
>
> Furthermore, there's no point iterating over the vectors past
> LAST_HIPRIORITY_VECTOR, so terminate the loop accordingly.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- a/xen/arch/x86/i8259.c
> +++ b/xen/arch/x86/i8259.c
> @@ -416,6 +416,8 @@ void __init init_IRQ(void)
>      for (irq = 0; platform_legacy_irq(irq); irq++) {
>          struct irq_desc *desc = irq_to_desc(irq);
>          
> +        if ( irq == 2 ) /* IRQ2 doesn't exist */
> +            continue;
>          desc->handler = &i8259A_irq_type;
>          per_cpu(vector_irq, cpu)[FIRST_LEGACY_VECTOR + irq] = irq;
>          cpumask_copy(desc->arch.cpu_mask, cpumask_of(cpu));
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -616,7 +616,9 @@ void irq_move_cleanup_interrupt(struct c
>      ack_APIC_irq();
>  
>      me = smp_processor_id();
> -    for (vector = FIRST_DYNAMIC_VECTOR; vector < NR_VECTORS; vector++) {
> +    for ( vector = FIRST_DYNAMIC_VECTOR;
> +          vector <= LAST_HIPRIORITY_VECTOR; vector++)
> +    {
>          unsigned int irq;
>          unsigned int irr;
>          struct irq_desc *desc;
> @@ -625,6 +627,12 @@ void irq_move_cleanup_interrupt(struct c
>          if ((int)irq < 0)
>              continue;
>  
> +        if ( vector >= FIRST_LEGACY_VECTOR && vector <= LAST_LEGACY_VECTOR )
> +        {
> +            ASSERT(IO_APIC_IRQ(irq));
> +            continue;
> +        }
> +
>          desc = irq_to_desc(irq);
>          if (!desc)
>              continue;
>
>
>