Gautam Menghani <[email protected]> writes:
> A huge number of spurious interrupts can be seen immediately after a KVM
> on PowerNV guest boots up in XIVE mode.
>
> $ cat /proc/interrupts | grep SPU
> SPU: 223705 192439 273526 147623 Spurious interrupts
>
> This bug was introduced by commit ecd10702baae5 ("KVM: PPC: Book3S HV:
> Handle pending exceptions on guest entry with MSR_EE"). The root cause
> is that once LPCR_MER bit is set, it is supposed to be reset by
> software. But when a vCPU starts running with LPCR_MER set, the vCPU does
> not exit back to the host until the decrementer expires or there is an
> hcall, etc. This is because KVM on PowerNV guests have support for
> native XIVE, so they are not dependent on host for interrupt emulation.
> Due to this behaviour, a huge number of spurious interrupts are seen
> since LPCR_MER continues to be set and LPCR_MER cannot be reset until the
> vCPU exits to the host.
>
> Fix this behaviour by not using the LPCR_MER bit whenever native XIVE is
> available (currently in case of KVM on PowerNV only), as the XIVE hardware
> can present interrupts to the KVM guest vCPU directly. So the LPCR_MER
> functionality is not required. This reduces the number of spurious
> interrupts drastically.
>
Make sense.
> Fixes: ecd10702baae5 ("KVM: PPC: Book3S HV: Handle pending exceptions on
> guest entry with MSR_EE")
> Cc: [email protected] # 6.8+
> Reported-by: Timothy Pearson <[email protected]>
> Closes:
> https://lore.kernel.org/linuxppc-dev/582904882.11159.1786719390349.javamail.zim...@raptorengineeringinc.com
> Signed-off-by: Gautam Menghani <[email protected]>
> ---
> v4:
> 1. If interrupt mode is changed to XICS during guest reboot, MER should
> be used. (Amit)
>
> v3:
> 1. Continue the use of LPCR_MER when kernel-irqchip=off (Sashiko)
>
> v2:
> 1. Handle the case where xive_interrupt_pending() is true and also the
> external exception bit is set. (Narayana)
>
> arch/powerpc/include/asm/kvm_ppc.h | 7 +++++++
> arch/powerpc/kvm/book3s_hv.c | 2 +-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> b/arch/powerpc/include/asm/kvm_ppc.h
> index 169ea6a7fbad..9178406dc365 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -747,6 +747,11 @@ static inline int kvmppc_xive_enabled(struct kvm_vcpu
> *vcpu)
> return vcpu->arch.irq_type == KVMPPC_IRQ_XIVE;
> }
>
> +static inline bool kvmppc_xive_native_enabled(struct kvm_vcpu *vcpu)
> +{
> + return kvmppc_xive_enabled(vcpu) && vcpu->kvm->arch.xive_devices.native;
I was checking the code and I am not sure whether we need to also check
for "vcpu->kvm->arch.xive_devices.native"?
Can you explain why do we need to check for xive_devices.native pointer
to be non-NULL? Because if we don't need this then it simplifies the
code as we can directly use kvmppc_xive_enabled(vcpu) helper.
-ritesh
> +}
> +
> extern int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
> struct kvm_vcpu *vcpu, u32 cpu);
> extern void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu);
> @@ -782,6 +787,8 @@ static inline bool kvmppc_xive_rearm_escalation(struct
> kvm_vcpu *vcpu) { return
>
> static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
> { return 0; }
> +static inline bool kvmppc_xive_native_enabled(struct kvm_vcpu *vcpu) {
> return false; }
> +
> static inline int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
> struct kvm_vcpu *vcpu, u32 cpu) { return -EBUSY; }
> static inline void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu) { }
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index dbac3573b2c8..dec89c014c52 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4980,7 +4980,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64
> time_limit,
> if (!kvmhv_on_pseries() && (__kvmppc_get_msr_hv(vcpu) &
> MSR_EE))
> kvmppc_inject_interrupt_hv(vcpu,
>
> BOOK3S_INTERRUPT_EXTERNAL, 0);
> - else
> + else if (!kvmppc_xive_native_enabled(vcpu))
> lpcr |= LPCR_MER;
> } else {
> /*
> --
> 2.52.0
