Re: [Xen-devel] [PATCH 1/4] xen/arm: gic: Ensure we have an ISB between ack and do_IRQ()

[Andre Przywara <andre.przywara@xxxxxxx>]

On Fri, 23 Nov 2018 12:09:41 +0200
Andrii Anisov <andrii.anisov@xxxxxxxxx> wrote:

Hi,

> On 22.11.18 19:22, Julien Grall wrote:
> > My biggest worry is you are doing optimization on a vGIC that is
> > not fully compliant with how a GIC should behave (e.g edge vs
> > level) and with very fragile locking.  
> Yep, old VGIC locking looks pretty terrible.
> 
> > If you are interested, Andre can provides more details.  
> Being honest, I'm not fully understand edge vs level problem there.
> It would be good to get better view on it.

Fundamentally there is a semantic difference between edge and level
triggered IRQs:
When the guest has handled an *edge* IRQ (EOIed so the LR's state goes
to 0), this is done and dusted, and Xen doesn't need to care
about this anymore until the next IRQ occurs.

For level triggered IRQs, even though the guest has handled it, we need
to resample the (potentially virtual) IRQ line, as it may come up or
down at the *device*'s discretion: the interrupt reason might have gone
away (GPIO condition no longer true), even before we were able to
inject it, or there might be another interrupt reason not yet handled
(incoming serial character while serving a transmit interrupt). Also
typically it's up to the interrupt handler to confirm handling the
interrupt, either explicitly by clearing an interrupt bit in some
status register or implicitly, for instance by draining a FIFO, say on
a serial device. So even though from the (V)GIC's point of view the
interrupt has been processed (EOIed), it might still be pending.

My intimate "old Xen VGIC" knowledge has been swapped out from my brain
meanwhile, but IIRC Xen treats every IRQ as if it would be an edge IRQ.
Which works if the guest's interrupt handler behaves correctly. Most
IRQ handlers have a loop to iterate over all possible interrupt
reasons and process them, so the line goes indeed down before they EOI
the IRQ.

> > I can't comment without any numbers here. Bear in mind that we
> > fixed bugs in Xen 4.12 (including spectre/meltdown and missing
> > barriers) that wasn't backported to Xen 4.10. It is entirely
> > possible that it introduced slowness but it also ensure the code is
> > behaving correctly.
> For sure, I know that. It was rather a political decision.

Translating "applying a fix for a serious security issue" to "political
decision" is a rather, ummh, interesting way of seeing things ;-)

Cheers,
Andre.

> > Anyway, if there are performance regression we should investigate
> > them and discuss how we can address/limit them. Similarly for the
> > new vGIC, if you think it is too slow, then we need to know why
> > before we get rid of the old vGIC.  
> Yep.
> 
> > There are no guest prioritization at the moment. However, we may
> > want to introduce it to give priority to one guest over.  
> But still, processing of IRQs by a hypervisor (e.g. moving them from
> gic to vgic for guests IRQs) has a higher priority over any guest
> execution.
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel