On Tue, Aug 29 2017 at 11:39:00 am BST, Christoffer Dall <cdall@xxxxxxxxxx> wrote: > Level-triggered mapped IRQs are special because we only observe rising > edges as input to the VGIC, and we don't set the EOI flag and therefore > are not told when the level goes down, so that we can re-queue a new > interrupt when the level goes up. > > One way to solve this problem is to side-step the logic of the VGIC and > special case the validation in the injection path, but it has the > unfortunate drawback of having to peak into the physical GIC state > whenever we want to know if the interrupt is pending on the virtual > distributor. > > Instead, we can maintain the current semantics of a level triggered > interrupt by sort of treating it as an edge-triggered interrupt, > following from the fact that we only observe an asserting edge. This > requires us to be a bit careful when populating the LRs and when folding > the state back in though: > > * We lower the line level when populating the LR, so that when > subsequently observing an asserting edge, the VGIC will do the right > thing. > > * If the guest never acked the interrupt while running (for example if > it had masked interrupts at the CPU level while running), we have > to preserve the pending state of the LR and move it back to the > line_level field of the struct irq when folding LR state. > > If the guest never acked the interrupt while running, but changed the > device state and lowered the line (again with interrupts masked) then > we need to observe this change in the line_level. > > Both of the above situations are solved by sampling the physical line > and set the line level when folding the LR back. > > * Finally, if the guest never acked the interrupt while running and > sampling the line reveals that the device state has changed and the > line has been lowered, we must clear the physical active state, since > we will otherwise never be told when the interrupt becomes asserted > again. > > This has the added benefit of making the timer optimization patches > (https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a > bit simpler, because the timer code doesn't have to clear the active > state on the sync anymore. It also potentially improves the performance > of the timer implementation because the GIC knows the state or the LR > and only needs to clear the > active state when the pending bit in the LR is still set, where the > timer has to always clear it when returning from running the guest with > an injected timer interrupt. > > Signed-off-by: Christoffer Dall <cdall@xxxxxxxxxx> > --- > virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++ > virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++ > virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++ > virt/kvm/arm/vgic/vgic.h | 7 +++++++ > 4 files changed, 88 insertions(+) > > diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c > index e4187e5..f7c5cb5 100644 > --- a/virt/kvm/arm/vgic/vgic-v2.c > +++ b/virt/kvm/arm/vgic/vgic-v2.c > @@ -104,6 +104,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu) > irq->pending_latch = false; > } > > + /* > + * Level-triggered mapped IRQs are special because we only > + * observe rising edges as input to the VGIC. > + * > + * If the guest never acked the interrupt we have to sample > + * the physical line and set the line level, because the > + * device state could have changed or we simply need to > + * process the still pending interrupt later. > + * > + * If this causes us to lower the level, we have to also clear > + * the physical active state, since we will otherwise never be > + * told when the interrupt becomes asserted again. > + */ > + if (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) { I've just had a worrying though. Here, we're looking at the guest's view of the trigger. But shouldn't that be the *host's*? We're assuming that the two should match, and while they certainly do for the timer, this is not something that can be enforced for SPIs. What do you think? M. -- Jazz is not dead. It just smells funny.
