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

On Tue, 23 Mar 2021 17:53:42 +0000,
Yoan Picchi [off-list ref] wrote:

Hi Mark.

s/k/c/, please!

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.

I think you should consider what I said in my reply to the cover
letter before going all out on every counter you have introduced in
this series.

[...]

quoted

quoted

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.

But if you do that, you start counting interrupts the guest itself
generates. What is the exact semantic of this counter? userspace
injected interrupts? Acked interrupts? Any interrupt?

Take my level interrupt example. The interrupt will be forever
pending, the guest will take as many interrupt as it can process, and
yet your counter will have been incremented *once*. What does your
counter mean then?

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

GICv4.{0,1}, where an interrupt gets directly delivered to the guest
without (too much) SW intervention. With this, directly injected LPIs
will never result in the counter being incremented, and yet can pin
the guest to the ground under interrupt load.

Again, defining the exact behaviour of the counter would avoid me
ranting away...

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel