Re: [PATCH v2] KVM: PPC: Book3S HV: Avoid spurious interrupts caused by LPCR_MER bit

[Narayana Murty N <nnmlinux@linux.ibm.com>]

Hi,

Correction to my previous reply.

On 18/09/26 1:00 PM, Narayana Murty N wrote:
> Hi
> 
> On 18/09/26 11:38 AM, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential 
>> issue(s) to consider:
>> - [High] kvm: ppc: software-injected external interrupts lost on XIVE 
>> hosts
>> -- 
>>
>> commit 05773d5b089dd4410c7e3e3a1611da5479c7d1a4
>> Author: Gautam Menghani <gautam@linux.ibm.com>
>>
>> KVM: PPC: Book3S HV: Avoid spurious interrupts caused by LPCR_MER bit
>>
>> This commit aims to avoid a huge number of spurious interrupts on KVM 
>> PowerNV
>> guests in XIVE mode by not using the LPCR_MER bit when the hardware can
>> present interrupts directly. It modifies kvmhv_run_single_vcpu() to skip
>> setting LPCR_MER when xics_on_xive() returns true.
>>
>> Closes: https://lore.kernel.org/linuxppc- 
>> dev/582904882.11159.1786719390349.JavaMail.zimbra@raptorengineeringinc.com
>>
>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>>> index dbac3573b2c89..f4d12a97311b9 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 (!xics_on_xive())
>>>                   lpcr |= LPCR_MER;
>>
>> [Severity: High]
>> Does this unconditionally disable LPCR_MER on XIVE hosts, potentially 
>> dropping
>> software-injected external interrupts when MSR_EE=0?
>>
>> The outer condition in kvmhv_run_single_vcpu() checks if either a KVM 
>> software
>> interrupt (BOOK3S_IRQPRIO_EXTERNAL) or a hardware XIVE interrupt is 
>> pending.
>>
>> When MSR_EE=0 on guest entry, the patched code evaluates:
>>
>>      else if (!xics_on_xive())
>>          lpcr |= LPCR_MER;
>>
>> Since xics_on_xive() evaluates to true globally on bare-metal POWER9+ 
>> hosts
>> with XIVE enabled, regardless of whether the guest uses the in-kernel 
>> XIVE
>> irqchip, bypassing LPCR_MER here seems to cause software-injected 
>> interrupts
>> (such as those from userspace via the KVM_INTERRUPT ioctl) to lose the
>> hardware trap mechanism that normally fires when the guest later sets
>> MSR_EE=1.
>>
>> Could this lead to dropped or severely delayed external interrupts for 
>> guests
>> using a userspace irqchip?
> 
> No, interrupts are not dropped. KVM_INTERRUPT sets 
> BOOK3S_IRQPRIO_EXTERNAL in vcpu->arch.pending_exceptions and kicks the 
> vCPU out to the host. On the next kvmhv_run_single_vcpu() entry, 
> kvmppc_core_prepare_to_enter() re-evaluates the pending bit. The bit 
> persists until MSR_EE=1 allows delivery via vmppc_inject_interrupt_hv(). 
> LPCR_MER is not needed here as XIVE hardware handles the delivery 
> trigger directly If I am not wrong.
> 
> 
> Looks good to me, much better logic than v1. The v1 concern about the
> case where xive_interrupt_pending() is true alongside the external
> exception bit is now handled cleanly with the xics_on_xive() guard.
> 
> Feel free to add:
> 
> Reviewed-by: Narayana Murty N <nnmlinux@linux.ibm.com>
> 
> Thanks,
> Narayana Murty N.
>>
I misunderstood the case raised by Sashiko. On a PowerNV/XIVE host, QEMU 
can still run with:

kernel_irqchip=off

In that case interrupts are emulated in userspace, while xics_on_xive() 
is still true for the host.

A KVM_INTERRUPT can set BOOK3S_IRQPRIO_EXTERNAL without a corresponding 
XIVE interrupt pending. If MSR_EE is clear, the exception remains 
pending, and with:

else if (!xics_on_xive())
	lpcr |= LPCR_MER;

LPCR_MER is not set. The interrupt could then be delayed until some 
unrelated guest exit.

So my previous Reviewed-by was premature. Please drop:

Reviewed-by: Narayana Murty N <nnmlinux@linux.ibm.com>

Thanks,
Narayana
> 
>