Re: [Xen-devel] High CPU temp, suspend problem - xen 4.1.5-pre, linux 3.7.x

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.03.13 at 12:53, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 28/03/2013 10:50, Jan Beulich wrote:
>> x86: irq_move_cleanup_interrupt() must ignore legacy vectors
>>
>> 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 warning at once (could probably as
>> well be an ASSERT()).
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Under what circumstances would we have any vectors 0xe0-0xef programmed
> into the IOAPIC?  I cant think of any offhand.

Never. And I didn't say it would.

> As far as I am aware, it is not valid for any PIC interrupts to ever be
> up for moving, as they should only be delivered to the BSP.

Hence the WARN_ON() (or ASSERT()).

> In addition to the check you have, the scope of the loop should probably
> be reduced.  We should never be considering to move any vector larger
> than LAST_HIPRIORITY_VECTOR, which I believe are all LAPIC interrupts,
> making 8 useless iterations of the loop.

Agreed. Will update the patch to also do that.

>  I would also suggest that it
> is an ASSERT rather than a WARN, but that leaves us not fixing the bug
> at hand, as we have already verified that vector 0xe9 is not programmed
> into the IOAPIC.

So with you repeating this I think I didn't explain well enough
what I think is happening. Hence I'll try again: We possibly (on at
least one CPU for sure) have two vector_irq[] entries referring to
any particular legacy IRQ - one for the vector that the IO-APIC is
using, and one for the corresponding legacy vector. Hence there'll
be two iterations of the loop here looking at the _same_ IRQ, the
second of which (wrongly) being the one pointed to by the entry in
the legacy vector range. It is this second instance that the change
is suppressing, with the WARN_ON() being there to ascertain that
we indeed never get here for an IRQ handled through the 8259A.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel