Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Matthew Leach <matthew.leach@collabora.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Thomas Gleixner <tglx@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	 Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org,  "H. Peter Anvin" <hpa@zytor.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 kernel@collabora.com
Subject: Re: [PATCH] KVM: x86: Reevaluate pending interrupts when enabling ExtINT through LVT0
Date: Wed, 4 Nov 2026 12:09:22 -0800	[thread overview]
Message-ID: <auuQ8pE439EB63tm@google.com> (raw)
In-Reply-To: <20261008-lapic-extint-lvt0-unmask-fix-v1-1-db2bf68fba75@collabora.com>

On Thu, Oct 08, 2026, Matthew Leach wrote:
> If the PIC output becomes asserted while LVT0 is masked, unmasking LVT0
> in ExtINT mode makes that interrupt injectable without setting
> KVM_REQ_EVENT. Without another event triggering reevaluation, the
> interrupt remains pending and a guest waiting for timer ticks can hang.
> 
> Fix by setting KVM_REQ_EVENT on an LVT0 write where pic interrupts
> become acceptable.

Why just for LVT0?  Isn't this a bug in general?  I.e. can't we simply do:

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d6e5fbc3c58c..ffdc67ca8fa3 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2554,6 +2554,8 @@ static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 			val |= APIC_LVT_MASKED;
 		val &= apic_lvt_mask[index];
 		kvm_lapic_set_reg(apic, reg, val);
+		if (!(val & APIC_LVT_MASKED))
+			kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
 		break;
 	}
 
Ahh, but there's nowhere for edge-triggered events to be latched, so this only
affects level-triggered events.  Which means, this?

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d6e5fbc3c58c..b72930a8516d 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2554,6 +2554,8 @@ static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 			val |= APIC_LVT_MASKED;
 		val &= apic_lvt_mask[index];
 		kvm_lapic_set_reg(apic, reg, val);
+		if (!(val & APIC_LVT_MASKED) && (val & APIC_LVT_LEVEL_TRIGGER))
+			kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
 		break;
 	}
 
> Signed-off-by: Matthew Leach <matthew.leach@collabora.com>
> ---
> During calibrate_delay_converge() we spin waiting for jiffies to increment on a
> tick interrupt. However, if KVM fails to deliver the interrupt this causes the
> guest to hang, spinning on the while loop.
> 
> Reproduce with:
> 
> qemu-system-x86_64 \
>   -cpu host \
>   -accel kvm \
>   -smp 1 \
>   -machine pc,hpet=off \
>   -kernel <bzimage> \
>   -nographic \
>   -append "console=ttyS0,115200 noapic apic=debug no-kvmclock clearcpuid=tsc"
> 
> Resulting in a hang at:
> 
> printk: legacy console [ttyS0] enabled
> printk: legacy console [ttyS0] enabled
> printk: legacy bootconsole [uart8250] disabled
> printk: legacy bootconsole [uart8250] disabled
> ACPI: Core revision 20260408
> ACPI: setting ELCR to 0200 (from 0c00)
> APIC: Switch to symmetric I/O mode setup
> Not enabling interrupt remapping due to skipped IO-APIC setup
> Enabled ExtINT on CPU#0
> 
> Note that the bug manifests itself only if a PIC interrupt is asserting during
> the mask window, so it is timing based. Using nested virt helps, or apply the
> following patch to the guest to provide enough time for a tick after masking:
> 
> --8<---------------cut here---------------start------------->8---
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index aa1e19979aa8..ddee09360dd0 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1509,7 +1509,7 @@ static void apic_clear_isr(void)
>  static void setup_local_APIC(void)
>  {
>  	int cpu = smp_processor_id();
> -	unsigned int value;
> +	unsigned int value, i;
>  
>  	if (apic_is_disabled) {
>  		disable_ioapic_support();
> @@ -1527,6 +1527,9 @@ static void setup_local_APIC(void)
>  	value &= ~APIC_SPIV_APIC_ENABLED;
>  	apic_write(APIC_SPIV, value);
>  
> +	for (i = 0; i < 10000000UL; i++)
> +		cpu_relax();
> +
>  #ifdef CONFIG_X86_32
>  	/* Pound the ESR really hard over the head with a big hammer - mbligh */
>  	if (lapic_is_integrated() && apic->disable_esr) {
> --8<---------------cut here---------------end--------------->8---
> ---
>  arch/x86/kvm/lapic.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index e1f3cea14765..ce7fbd6c45c2 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2520,6 +2520,14 @@ static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  			val |= APIC_LVT_MASKED;
>  		val &= apic_lvt_mask[index];
>  		kvm_lapic_set_reg(apic, reg, val);
> +
> +		/*
> +		 * Enabling ExtINT can make an already-asserted PIC interrupt
> +		 * deliverable without another transition on the PIC output.
> +		 */
> +		if (reg == APIC_LVT0 && kvm_apic_accept_pic_intr(apic->vcpu))
> +			kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
> +
>  		break;
>  	}
>  
> 
> ---
> base-commit: a90ee4305c4a5df72c11b31dacfdc76e00fcf78a
> change-id: 20261008-lapic-extint-lvt0-unmask-fix-52d2328af682
> 
> Best regards,
> --  
> Matt
> 
> 

      reply	other threads:[~2026-11-04 20:09 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-08 12:49 [PATCH] KVM: x86: Reevaluate pending interrupts when enabling ExtINT through LVT0 Matthew Leach
2026-11-04 20:09 ` Sean Christopherson [this message]

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=auuQ8pE439EB63tm@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=kernel@collabora.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew.leach@collabora.com \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@kernel.org \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox