Edwin, Xiantao, I''m observing I/O-APIC IRQs never getting migrated on a system using directed EOI. Looking at end_level_ioapic_irq(), I fail to see how that case is being handled (other than in the Linux implementation, which this code supposedly is a clone of). Additionally it seems wasteful to check whether to move the IRQ in mask_and_ack_level_ioapic_irq(), as ack_APIC_irq() cannot possibly have resulted in clearing any of the IRR bits that io_apic_level_ack_pending() is looking at. Thanks for any comments, Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> On 29.11.10 at 14:14, "Jan Beulich" <JBeulich@novell.com> wrote: > Edwin, Xiantao, > > I''m observing I/O-APIC IRQs never getting migrated on a system > using directed EOI. Looking at end_level_ioapic_irq(), I fail > to see how that case is being handled (other than in the Linux > implementation, which this code supposedly is a clone of). > > Additionally it seems wasteful to check whether to move the IRQ in > mask_and_ack_level_ioapic_irq(), as ack_APIC_irq() cannot > possibly have resulted in clearing any of the IRR bits that > io_apic_level_ack_pending() is looking at.Below is a tentative (lightly tested) fix for this. Comments? Jan --- a/xen/arch/x86/io_apic.c +++ b/xen/arch/x86/io_apic.c @@ -1634,11 +1634,14 @@ static void mask_and_ack_level_ioapic_ir ack_APIC_irq(); + if ( directed_eoi_enabled ) + return; + if ((irq_desc[irq].status & IRQ_MOVE_PENDING) && !io_apic_level_ack_pending(irq)) - move_native_irq(irq); + move_masked_irq(irq); - if (!directed_eoi_enabled && !(v & (1 << (i & 0x1f)))) { + if ( !(v & (1 << (i & 0x1f))) ) { atomic_inc(&irq_mis_count); spin_lock(&ioapic_lock); __edge_IO_APIC_irq(irq); @@ -1654,12 +1657,22 @@ static void end_level_ioapic_irq (unsign if ( !ioapic_ack_new ) { - if ( irq_desc[irq].status & IRQ_DISABLED ) - return; - if ( directed_eoi_enabled ) + { + if ( !(irq_desc[irq].status & (IRQ_DISABLED|IRQ_MOVE_PENDING)) ) + { + eoi_IO_APIC_irq(irq); + return; + } + + mask_IO_APIC_irq(irq); eoi_IO_APIC_irq(irq); - else + if ( (irq_desc[irq].status & IRQ_MOVE_PENDING) && + !io_apic_level_ack_pending(irq) ) + move_masked_irq(irq); + } + + if ( !(irq_desc[irq].status & IRQ_DISABLED) ) unmask_IO_APIC_irq(irq); return; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hi, Jan
I think the patch is fine. In directed eoi case, the
io_apic_level_ack_pending check should be always true before eoi_IO_APIC_irq is
called , so move_masked_irq is never called and no IRQ migration occurs.
Xiantao
Jan Beulich wrote:>>>> On 29.11.10 at 14:14, "Jan Beulich"
<JBeulich@novell.com> wrote:
>>>> Edwin, Xiantao,
>>
>> I''m observing I/O-APIC IRQs never getting migrated on a system
>> using directed EOI. Looking at end_level_ioapic_irq(), I fail
>> to see how that case is being handled (other than in the Linux
>> implementation, which this code supposedly is a clone of).
>>
>> Additionally it seems wasteful to check whether to move the IRQ in
>> mask_and_ack_level_ioapic_irq(), as ack_APIC_irq() cannot
>> possibly have resulted in clearing any of the IRR bits that
>> io_apic_level_ack_pending() is looking at.
>
> Below is a tentative (lightly tested) fix for this. Comments?
>
> Jan
>
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -1634,11 +1634,14 @@ static void mask_and_ack_level_ioapic_ir
>
> ack_APIC_irq();
>
> + if ( directed_eoi_enabled )
> + return;
> +
> if ((irq_desc[irq].status & IRQ_MOVE_PENDING) &&
> !io_apic_level_ack_pending(irq))
> - move_native_irq(irq);
> + move_masked_irq(irq);
>
> - if (!directed_eoi_enabled && !(v & (1 << (i &
0x1f)))) {
> + if ( !(v & (1 << (i & 0x1f))) ) {
> atomic_inc(&irq_mis_count);
> spin_lock(&ioapic_lock);
> __edge_IO_APIC_irq(irq);
> @@ -1654,12 +1657,22 @@ static void end_level_ioapic_irq (unsign
>
> if ( !ioapic_ack_new )
> {
> - if ( irq_desc[irq].status & IRQ_DISABLED )
> - return;
> -
> if ( directed_eoi_enabled )
> + {
> + if ( !(irq_desc[irq].status &
> (IRQ_DISABLED|IRQ_MOVE_PENDING)) ) + {
> + eoi_IO_APIC_irq(irq);
> + return;
> + }
> +
> + mask_IO_APIC_irq(irq);
> eoi_IO_APIC_irq(irq);
> - else
> + if ( (irq_desc[irq].status & IRQ_MOVE_PENDING) &&
> + !io_apic_level_ack_pending(irq) )
> + move_masked_irq(irq);
> + }
> +
> + if ( !(irq_desc[irq].status & IRQ_DISABLED) )
> unmask_IO_APIC_irq(irq);
>
> return;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel