Xen devel - Aug 2013 - HPET stack overflow, and general problems with do_IRQ()

Hello,

I have finally managed to get a full stack dump from affected hardware.

The logs can be found here (including hypervisor with debugging symbols):

http://xenbits.xen.org/people/andrewcoop/hpet-overflow-full-stackdump.tar.gz

The interesting log file is xen.pcpu0.stack.log

By my count (grepping for e008 as CS), there are are 8 exception frames
on the Xen stack (all stack page 6)

However, because of the early ack() at the LAPIC, and disabling of
interrupts, the vectors (in order of interrupts arriving) are

c1, 99, b1, b9, a9, a1, 91, 89

These 8 interrupts take a little more than half the available stack,
while the bottom half of the stack seems be a vmentry which failed
because of a hap pagefault.

One "solution" to the problem would be to extend the Xen
PRIMARY_STACK_SIZE to 3 pages rather than 2, but is hardly a good thing
to do.


I think that the fundamental problem is the early ack and re-enabling of
interrupts.  We have servers where 150 VMs using PCIpassthrough are
starting to run out of available entries in the IDTs.  While unlikely,
it would be possible to encounter a situation with 40 nested interrupts,
at which point there is a real danger of trashing the compat sysenter
trampoline, located at the base of stack page 3, and just a few more
before MCEs and NMIs will end up walking over the main stack.

While I hate to suggest this, the only sensible solution without edge
cases is to never enable interrupts in do_IRQ().  I suppose a slightly
less extreme solution could be to promote the TPR to 0xe0 and re-enable
interrupts, so high-priority processing can still occur?

Thought/comments?

Unfortunately, I am out of the office now until Monday 26th, with
limited access to internet during that time (Although I will still be
with internet tomorrow morning).  I will check emails when I can, but I
don''t expect to be able to make timely contributions to the above
discussion during this time.

~Andrew

>>> On 15.08.13 at 22:21, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> Hello,
> 
> I have finally managed to get a full stack dump from affected hardware.
> 
> The logs can be found here (including hypervisor with debugging symbols):
> 
>
http://xenbits.xen.org/people/andrewcoop/hpet-overflow-full-stackdump.tar.gz
> 
> The interesting log file is xen.pcpu0.stack.log
> 
> By my count (grepping for e008 as CS), there are are 8 exception frames
> on the Xen stack (all stack page 6)
> 
> However, because of the early ack() at the LAPIC, and disabling of
> interrupts, the vectors (in order of interrupts arriving) are
> 
> c1, 99, b1, b9, a9, a1, 91, 89
So these are all HPET interrupts as it seems to me. You said the
box just has 8 of them, so the fundamental problem is not the
general handling of interrupts that you talk about below, but the
fact that _all_ these channels are bound to CPU0: That''s an
insane side effect of the way channel management works when
there are (potentially) more CPUs than channels. So _I_ think
this is what needs fixing.

That''s even more so that the above sequence would be impossible
for guest interrupts (which don''t get EOI-ed immediately, and
interrupts don''t get re-enabled on that path either). Hence in the
discussion here we need to only be concerned of interrupts that
Xen uses for itself: timer, console, iommu, and HPET. Out of these,
timer and console - going through the IO-APIC - are safe from this
because of how io_apic.c implements the ->ack()/->end() pairs.
Both IOMMU implementations ack their IRQs in the LAPIC only in
->end(). And that''s what I suggested to switch HPET to too. And
other than I said about this earlier, disabling interrupts in the
->end() handler isn''t even necessary, as it already gets called with
them disabled.

So we have two possible fixes to the HPET, either of which is
very likely to deal with the problem on its own.

Jan

On 16/08/2013 08:53, "Jan Beulich" <JBeulich@suse.com> wrote:
>>>> On 15.08.13 at 22:21, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
>> Hello,
>> 
>> I have finally managed to get a full stack dump from affected hardware.
>> 
>> The logs can be found here (including hypervisor with debugging
symbols):
>> 
>>
http://xenbits.xen.org/people/andrewcoop/hpet-overflow-full-stackdump.tar.gz
>> 
>> The interesting log file is xen.pcpu0.stack.log
>> 
>> By my count (grepping for e008 as CS), there are are 8 exception frames
>> on the Xen stack (all stack page 6)
>> 
>> However, because of the early ack() at the LAPIC, and disabling of
>> interrupts, the vectors (in order of interrupts arriving) are
>> 
>> c1, 99, b1, b9, a9, a1, 91, 89
> 
> So these are all HPET interrupts as it seems to me. You said the
> box just has 8 of them, so the fundamental problem is not the
> general handling of interrupts that you talk about below, but the
> fact that _all_ these channels are bound to CPU0: That''s an
> insane side effect of the way channel management works when
> there are (potentially) more CPUs than channels. So _I_ think
> this is what needs fixing.
> 
> That''s even more so that the above sequence would be impossible
> for guest interrupts (which don''t get EOI-ed immediately, and
> interrupts don''t get re-enabled on that path either). Hence in the
> discussion here we need to only be concerned of interrupts that
> Xen uses for itself: timer, console, iommu, and HPET. Out of these,
> timer and console - going through the IO-APIC - are safe from this
> because of how io_apic.c implements the ->ack()/->end() pairs.
> Both IOMMU implementations ack their IRQs in the LAPIC only in
> ->end(). And that''s what I suggested to switch HPET to too. And
> other than I said about this earlier, disabling interrupts in the
> ->end() handler isn''t even necessary, as it already gets called
with
> them disabled.
> 
> So we have two possible fixes to the HPET, either of which is
> very likely to deal with the problem on its own.
Additionally, with per-vcpu stacks we could have a larger per-cpu irq stack.
It would be easier to grow that without ''wasting'' memory.
Although I think
Jan''s arguments above do make sense.

 -- Keir
> Jan
>