From mboxrd@z Thu Jan  1 00:00:00 1970
From: Keir Fraser <keir.xen@gmail.com>
Subject: Re: [PATCH] x86: irq_move_cleanup_interrupt() must
 ignore legacy vectors
Date: Thu, 28 Mar 2013 19:58:04 +0000
Message-ID: <CD7A514C.4EDB2%keir.xen@gmail.com>
References: <51546EB902000078000C95B2@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <51546EB902000078000C95B2@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>, xen-devel <xen-devel@lists.xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Keir Fraser <keir@xen.org>, Marek Marczykowski <marmarek@invisiblethingslab.com>
List-Id: xen-devel@lists.xenproject.org

On 28/03/2013 15:24, "Jan Beulich" <JBeulich@suse.com> 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>

Acked-by: Keir Fraser <keir@xen.org>

> --- 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;
> 
> 
>