Re: [PATCH 7/7] KVM: arm64: Add irq_inject counter for kvm_stat

[Yoan Picchi <yoan.picchi@xxxxxxx>]

Hi Mark.

Thanks for all the reviews. I am a beginner and you gave me a lot to 
learn about.
I will reply to the other patch progressively once I understand better 
the issues.

On 23/03/2021 17:37, Marc Zyngier wrote:
> On Fri, 19 Mar 2021 16:17:11 +0000,
> Yoan Picchi <yoan.picchi@xxxxxxx> wrote:
> > Add a counter for interrupt injections. That is when kvm relay an
> > interrupt to the guest (for instance a timer, or a device interrupt
> > like from a network card)
> >
> > Signed-off-by: Yoan Picchi <yoan.picchi@xxxxxxx>
> > ---
> >   arch/arm64/include/asm/kvm_host.h | 2 ++
> >   arch/arm64/kvm/arm.c              | 2 ++
> >   arch/arm64/kvm/guest.c            | 2 ++
> >   arch/arm64/kvm/vgic/vgic.c        | 2 ++
> >   4 files changed, 8 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index fa59b669c..253acb8c2 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -551,6 +551,7 @@ struct kvm_vm_stat {
> >   	ulong memory_slot_unmaped;
> >   	ulong stage2_unmap_vm;
> >   	ulong cached_page_invalidated;
> > +	ulong irq_inject;
> >   };
> >   
> >   struct kvm_vcpu_stat {
> > @@ -567,6 +568,7 @@ struct kvm_vcpu_stat {
> >   	u64 mmio_exit_kernel;
> >   	u64 regular_page_mapped;
> >   	u64 huge_page_mapped;
> > +	u64 irq_inject;
> >   	u64 exits;
> >   };
> >   
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index fc4c95dd2..841551f14 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -895,6 +895,8 @@ static int vcpu_interrupt_line(struct kvm_vcpu *vcpu, int number, bool level)
> >   	bool set;
> >   	unsigned long *hcr;
> >   
> > +	vcpu->stat.irq_inject++;
> > +
> >   	if (number == KVM_ARM_IRQ_CPU_IRQ)
> >   		bit_index = __ffs(HCR_VI);
> >   	else /* KVM_ARM_IRQ_CPU_FIQ */
> > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > index 129c0d53d..f663b03ae 100644
> > --- a/arch/arm64/kvm/guest.c
> > +++ b/arch/arm64/kvm/guest.c
> > @@ -45,6 +45,8 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
> >   	VM_STAT("memory_slot_unmaped", memory_slot_unmaped),
> >   	VM_STAT("stage2_unmap_vm", stage2_unmap_vm),
> >   	VM_STAT("cached_page_invalidated", cached_page_invalidated),
> > +	VM_STAT("irq_inject", irq_inject),
> > +	VCPU_STAT("irq_inject", irq_inject),
> >   	VCPU_STAT("exits", exits),
> >   	VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns),
> >   	VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns),
> > diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> > index 1c597c988..9e504243b 100644
> > --- a/arch/arm64/kvm/vgic/vgic.c
> > +++ b/arch/arm64/kvm/vgic/vgic.c
> > @@ -458,6 +458,8 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
> >   
> >   	raw_spin_lock_irqsave(&irq->irq_lock, flags);
> >   
> > +	kvm->stat.irq_inject++;
> > +
> >   	if (!vgic_validate_injection(irq, level, owner)) {
> So even if the injection failed, you report an injection? And what
> about injection that occur via the MMIO interface? What about direct
> injection? What about a level interrupt that is forever high?
>
> 	M.
This one I actually started to fix this afternoon by moving the counter 
into vgic_queue_irq_unlock().
This way it is only incremented when the interrupt is inserted into a 
vcpu, and it also takes care of the
vgic_mmio injections. I also fixed the issue with the interrupt line so 
it only increment when the line
change of level.

I'm not sure about what you mean by direct injection yet though.

Kindly,
Yoan

_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm