All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Huang, Kai" <kai.huang@intel.com>
To: "seanjc@google.com" <seanjc@google.com>,
	"hao_zhang_kdev@163.com" <hao_zhang_kdev@163.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>
Subject: Re: [PATCH v4 1/4] KVM: x86: ioapic: Update remote_irr only after successful delivery
Date: Tue, 11 Aug 2026 11:11:45 +0000	[thread overview]
Message-ID: <3b3db5de497f6196ddc17558b71aa746b0adf1e7.camel@intel.com> (raw)
In-Reply-To: <anrQfoE8HRvsaao_@192.168.1.215>

On Tue, 2026-08-11 at 15:34 +0800, Hao Zhang wrote:
> From: Hao Zhang <zhanghao1@kylinos.cn>
> 
> The I/O APIC sets remote_irr for level-triggered interrupts that have
> been accepted by a local APIC and are waiting for an EOI.
> 
> ioapic_service() currently treats any non-zero return from
> kvm_irq_delivery_to_apic() as successful delivery.  But
> kvm_irq_delivery_to_apic() returns -1 when no destination is found.
> Setting remote_irr for that failed delivery leaves the pin blocked
> forever waiting for an EOI that will never be generated.
> 
> Update remote_irr only when the delivery result is positive, i.e. when
> at least one local APIC accepted the interrupt.
> 
> Fixes: 4925663a079c ("KVM: Report IRQ injection status to userspace.")

The code LGTM, but I am not sure this is the right commit for the Fixes tag.

Looking at the relevant diff:

+static int ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx)
 {
        union ioapic_redir_entry *pent;
+       int injected = -1;
 
        pent = &ioapic->redirtbl[idx];
 
        if (!pent->fields.mask) {
-               int injected = ioapic_deliver(ioapic, idx);
+               injected = ioapic_deliver(ioapic, idx);
                if (injected && pent->fields.trig_mode == IOAPIC_LEVEL_TRIG)
                        pent->fields.remote_irr = 1;
        }

This commit didn't change any logic related to remote_irr, since it didn't touch
ioapic_deliver() at all.

The one that is close seems to be 

  ff4b9df877b30 ("KVM: IOAPIC: only set remote_irr if interrupt was injected")

since it actually touched the logic of when to set remote_irr:

@@ -89,8 +89,8 @@ static void ioapic_service(struct kvm_ioapic *ioapic, unsigned
int idx)
        pent = &ioapic->redirtbl[idx];
 
        if (!pent->fields.mask) {
-               ioapic_deliver(ioapic, idx);
-               if (pent->fields.trig_mode == IOAPIC_LEVEL_TRIG)
+               int injected = ioapic_deliver(ioapic, idx);
+               if (injected && pent->fields.trig_mode == IOAPIC_LEVEL_TRIG)
                        pent->fields.remote_irr = 1;
        }

But looking closer, at that time the ioapic_deliver() could only return 0 or 1
(or >1 I didn't check), i.e., it couldn't return -1 at all.

In fact, even when commit 4925663a079c was merged the then ioapic_deliver()
couldn't return -1.

So AFAICT both of the above two commits are not the right one.

I haven't found which one to blame, but it seems at some point the function
kvm_irq_delivery_to_apic() was introduced and at some other point it was changed
to be able to return -1.  But I didn't dig further.

  parent reply	other threads:[~2026-08-11 11:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11  7:34 [PATCH v4 1/4] KVM: x86: ioapic: Update remote_irr only after successful delivery Hao Zhang
2026-08-11  7:39 ` [PATCH v4 2/4] KVM: selftests: Verify failed IOAPIC delivery preserves state Hao Zhang
2026-08-11  7:41 ` [PATCH v4 3/4] KVM: x86: ioapic: Preserve irr_delivered for duplicate edge interrupts Hao Zhang
2026-08-11 11:27   ` Huang, Kai
2026-08-11  7:43 ` [PATCH v4 4/4] KVM: x86: ioapic: Clear irr_delivered on level-triggered RTE Hao Zhang
2026-08-11 11:43   ` Huang, Kai
2026-08-11 11:11 ` Huang, Kai [this message]
2026-08-11 23:32   ` [PATCH v4 1/4] KVM: x86: ioapic: Update remote_irr only after successful delivery Huang, Kai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3b3db5de497f6196ddc17558b71aa746b0adf1e7.camel@intel.com \
    --to=kai.huang@intel.com \
    --cc=hao_zhang_kdev@163.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.