Hi Marc, On 3/19/20 4:21 PM, Marc Zyngier wrote: > Hi Eric, > > On 2020-03-19 15:05, Auger Eric wrote: >> Hi Marc, >> >> On 3/4/20 9:33 PM, Marc Zyngier wrote: >>> The vgic-state debugfs file could do with showing the pending state >>> of the HW-backed SGIs. Plug it into the low-level code. >>> >>> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> >>> --- >>> virt/kvm/arm/vgic/vgic-debug.c | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/virt/kvm/arm/vgic/vgic-debug.c >>> b/virt/kvm/arm/vgic/vgic-debug.c >>> index cc12fe9b2df3..b13a9e3f99dd 100644 >>> --- a/virt/kvm/arm/vgic/vgic-debug.c >>> +++ b/virt/kvm/arm/vgic/vgic-debug.c >>> @@ -178,6 +178,8 @@ static void print_irq_state(struct seq_file *s, >>> struct vgic_irq *irq, >>> struct kvm_vcpu *vcpu) >>> { >>> char *type; >>> + bool pending; >> nit: can be directly initialized to irq->pending_latch >>> + >>> if (irq->intid < VGIC_NR_SGIS) >>> type = "SGI"; >>> else if (irq->intid < VGIC_NR_PRIVATE_IRQS) >>> @@ -190,6 +192,16 @@ static void print_irq_state(struct seq_file *s, >>> struct vgic_irq *irq, >>> if (irq->intid ==0 || irq->intid == VGIC_NR_PRIVATE_IRQS) >>> print_header(s, irq, vcpu); >>> >>> + pending = irq->pending_latch; >>> + if (irq->hw && vgic_irq_is_sgi(irq->intid)) { >>> + int err; >>> + >>> + err = irq_get_irqchip_state(irq->host_irq, >>> + IRQCHIP_STATE_PENDING, >>> + &pending); >>> + WARN_ON_ONCE(err); >>> + } >>> + >>> seq_printf(s, " %s %4d " >>> " %2d " >>> "%d%d%d%d%d%d%d " >>> @@ -201,7 +213,7 @@ static void print_irq_state(struct seq_file *s, >>> struct vgic_irq *irq, >>> "\n", >>> type, irq->intid, >>> (irq->target_vcpu) ? irq->target_vcpu->vcpu_id : -1, >>> - irq->pending_latch, >>> + pending, >>> irq->line_level, >>> irq->active, >>> irq->enabled, >>> >> The patch looks good to me but I am now lost about how we retrieve the >> pending stat of other hw mapped interrupts. Looks we use >> irq->pending_latch always. Is that correct? > > Correct. GICv4.0 doesn't give us an architectural way to look at the > vLPI pending state (there isn't even a guarantee about when the GIC > will stop writing to memory, if it ever does). > > With GICv4.1, you can introspect the HW state for SGIs. You can also > look at the vLPI state by peeking at the virtual pending table, but > you'd need to unmap the VPE first, which I obviously don't want to do > for this debug interface, specially as it can be used whilst the guest > is up and running. OK for vLPIs, what about other HW mapped IRQs (arch timer?) Eric > > In the future, we'll have to implement that in order to support guest > save/restore from a GICv4.1 system. I haven't given much thought to it > though. > >> For the patch: >> Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx> > > Thanks, > > M.