Re: [Xen-devel] [RFC PATCH 21/24] ARM: vITS: handle INVALL command

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 12 Dec 2016, Andre Przywara wrote:
> >> The _pending_ table is exactly that: one bit per VLPI.
> > 
> > Actually the spec says about the pending table, ch 6.1.2:
> > 
> > "Each Redistributor maintains entries in a separate LPI Pending table
> > that indicates the pending state of each LPI when GICR_CTLR.EnableLPIs
> > == 1 in the Redistributor:
> >   0 The LPI is not pending.
> >   1 The LPI is pending.
> > 
> > For a given LPI:
> > • The corresponding byte in the LPI Pending table is (base address + (N / 
> > 8)).
> > • The bit position in the byte is (N MOD 8)."
> 
>         ^^^
> 
> > It seems to me that each LPI is supposed to have a byte, not a bit. Am I
> > looking at the wrong table?
> 
> Well, the explanation could indeed be a bit more explicit, but it's
> really meant to be a bit:
> 1) The two lines above describe how to address a single bit in a
> byte-addressed array.
> 2) The following paragraphs talk about "the first 1KB" when it comes to
> non-LPI interrupts. This matches 8192 bits.
> 3) In section 6.1 the spec states: "Memory-backed storage for LPI
> pending _bits_ in an LPI Pending table."
> 4) The actual instance, Marc's Linux driver, also speaks of a bit:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-gic-v3-its.c#n785

I was mislead by the doc. Better this way.


> >> This special handling for the interrupt storm just stems from the fact
> >> that have to keep LPIs enabled on the h/w interrupt controller level,
> >> despite the guest having disabled it on it's own _virtual_ GIC. So once
> >> the guest enables it again, we are in line with the current GICv2/GICv3,
> >> aren't we? Do we have interrupt storm detection/prevention in the moment?
> > 
> > No, that's not the cause of the storm. Julien described well before:
> > 
> >   given that LPIs are edge-triggered, there is no deactivate state (see 4.1
> >   in ARM IHI 0069C). So as soon as the priority drop is done, the same LPIs 
> > could
> >   potentially be raised again. This could generate a storm.
> > 
> > The problem is that Xen has to do priority drop upon receiving an pLPI,
> > but, given that LPIs don't have a deactivate state, the priority drop is
> > enough to let the hardware inject a second LPI, even if the guest didn't
> > EOI the first one yet.
> > 
> > In the case of SPIs, the hardware cannot inject a second interrupt after
> > Xen does priority drop, it has to wait for the guest to EOI it.
> 
> I understand that ...
> 
> >> And please keep in mind that LPIs are _always_ edge triggered: So once
> >> we EOI an LPI on the host, this "very same" LPI is gone from a h/w GIC
> >> point of view, the next incoming interrupt must have been triggered by a
> >> new interrupt condition in the device (new network packet, for
> >> instance).
> > 
> > This is actually the problem: what if the guest configured the device on
> > purpose to keep generating LPIs without pause? Nice and simple way to
> > take down the host.
> 
> I see, I just wasn't sure we were talking about the same thing: actual
> interrupt storm triggered by the device vs. "virtual" interrupt storm
> due to an interrupt line not being lowered by the IRQ handler.
> And I was hoping for the latter, but well ...
> So thanks for the clarification.
> 
> >> In contrast to GICv2 this applies to _every_ LPI.
> >> So I am not sure we should really care _too_ much about this (apart from
> >> the "guest has disabled it" part): Once we assign a device to a guest,
> >> we lose some control over the machine anyway and at least trust the
> >> device to not completely block the system.
> > 
> > No we don't! Hardware engineers make mistakes too!
> 
> You tell me ... ;-)
> 
> > We have to protect
> > Xen from devices which purposely or mistakenly generate interrupt
> > storms. This is actually a pretty common problem.
> 
> I see, though this doesn't make this whole problem easier ;-)
> 
> >> I don't see how the ITS differs in that respect from the GICv3/GICv2.
> > 
> > It differs because in the case of GICv2, every single interrupt has to
> > be EOI'd by the guest.
> 
> Sure, though I think technically it's "deactivated" here that matters
> (we EOI LPIs as well). And since LPIs have no active state, this makes
> the difference.
> 
> > Therefore the Xen scheduler can still decide to
> > schedule it out. In the case of the ITS, Xen could be stuck in an
> > interrupt handling loop.
> 
> So I was wondering as we might need to relax our new strict "No
> (unprivileged) guest ever causes a host ITS command to be queued" rule a
> bit, because:
> - Disabling an LPI is a separate issue, as we can trigger this in Xen
> interrupt context once we decide that this is an interrupt storm.
> - But enabling it again has to both happen in timely manner (as the
> guest expects interrupts to come in) and to be triggered by a guest
> action, which causes the INV command to be send when handling a guest fault.
> 
> Now this INV command (and possibly a follow-up SYNC) for enabling an LPI
> would be the only critical ones, so I was wondering if we could ensure
> that these commands can always be queued immediately, by making sure we
> have at least two ITS command queue slots available all of the time.
> Other ITS commands (triggered by device pass-throughs, for instance),
> would then have to potentially wait if we foresee that they could fill
> up the host command queue.
> Something like QoS for ITS commands.
> And I think we should map the maximum command queue size on the host
> (1MB => 32768 commands) to make this scenario less likely.
> 
> I will need to think about this a bit further, maybe implement something
> as a proof of concept.

On one hand, I'd say that it should be rare to re-enable an interrupt,
after it was disabled due to a storm. Extremely rare. It should be OK
to issue a physical INV and SYNC in that event.

However, if that happens, it could very well be because the guest is
trying to take down the host, and issuing physical ITS commands in
response to a malicious guest action, could be a good way to help the
attacker. We need to be extra careful.

Given that a storm is supposed to be an exceptional circumstance, it is
OK to enforce very strict limits on the amount of times we are willing
to issue physical ITS commands as a consequence of a guest action. For
example, we could decide to do it just once, or twice, then label the
guest as "untrustworthy" and destroy it. After all if a storm keeps
happening, it must be due to a malicious guest or faulty hardware - in
both cases it is best to terminate the VM.

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