mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Jinmeng Zhou <jinmengzhou22@gmail.com>
Cc: pbonzini@redhat.com, tglx@kernel.org, mingo@redhat.com,
	bp@alien8.de,  dave.hansen@linux.intel.com, hpa@zytor.com,
	feng.wu@intel.com, x86@kernel.org,  kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	 Jinmeng Zhou <zhoujinmeng@bytedance.com>,
	Guixiong Wei <weiguixiong@bytedance.com>
Subject: Re: [PATCH] KVM: x86: Wake blocked vCPUs before offlining a CPU
Date: Thu, 24 Sep 2026 07:55:42 -0700	[thread overview]
Message-ID: <arU57oqykUfeKqv2@google.com> (raw)
In-Reply-To: <20260924080648.51014-1-zhoujinmeng@bytedance.com>

On Thu, Sep 24, 2026, Jinmeng Zhou wrote:
> Posted-interrupt wakeup state is tied to the physical CPU on which a vCPU
> blocks. CPU hotplug migrates sleeping tasks before KVM's CPU offline
> callback, but the vCPU remains on the old CPU's wakeup list and its posted
> interrupt descriptor still targets the old physical APIC.
> 
> If a device posts an interrupt after the old CPU becomes unavailable, the
> wakeup notification cannot make the blocked vCPU runnable. The vCPU cannot
> repair the stale notification destination because that happens only after
> the vCPU is scheduled back in.
> 
> Add an architecture hook to KVM's CPU offline path and a corresponding
> optional x86 vendor callback. After CPUHP_AP_SCHED_WAIT_EMPTY has migrated
> tasks away from the dying CPU, have VMX set KVM_REQ_UNBLOCK and wake every
> vCPU on that CPU's posted-interrupt wakeup list. The vCPUs then unblock on
> online CPUs and the existing load path rebuilds their wakeup-list and
> posted-interrupt destination state.
> 
> Fixes: bf9f6ac8d749 ("KVM: Update Posted-Interrupts Descriptor when vCPU is blocked")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jinmeng Zhou <zhoujinmeng@bytedance.com>
> Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>

SoB chain is wrong.  Whoever sends the patch needs to come last.  And presumably
there's a missing "Co-developed-by: Guixiong Wei <weiguixiong@bytedance.com>".
The patch also needs a "From: Jinmeng Zhou <zhoujinmeng@bytedance.com>" since
you're using a different address to send the email.

> diff --git a/arch/x86/kvm/vmx/posted_intr.c b/arch/x86/kvm/vmx/posted_intr.c
> index 4a6d9a17da238..2d6cca1606bd1 100644
> --- a/arch/x86/kvm/vmx/posted_intr.c
> +++ b/arch/x86/kvm/vmx/posted_intr.c
> @@ -266,6 +266,29 @@ void pi_wakeup_handler(void)
>  	raw_spin_unlock(spinlock);
>  }
>  
> +void pi_wakeup_cpu_offline(unsigned int cpu)
> +{
> +	struct list_head *wakeup_list = &per_cpu(wakeup_vcpus_on_cpu, cpu);
> +	raw_spinlock_t *spinlock = &per_cpu(wakeup_vcpus_on_cpu_lock, cpu);
> +	struct vcpu_vt *vt;
> +	unsigned long flags;
> +
> +	/*
> +	 * CPUHP_AP_SCHED_WAIT_EMPTY has already migrated tasks away from the

IIUC, CPUHP_AP_SCHED_WAIT_EMPTY just waits for task migrations to complete,
CPUHP_AP_ACTIVE => sched_cpu_deactivate() is what actually initiates the migration.
That matters because I think it means we can repurpose CPUHP_AP_X86_KVM_CLK_ONLINE.

> +	 * dying CPU.  Force blocked vCPUs to leave the block loop so that their
> +	 * PI wakeup state is rebuilt on an online CPU before the old notification
> +	 * destination becomes unreachable.
> +	 */
> +	raw_spin_lock_irqsave(spinlock, flags);
> +	list_for_each_entry(vt, wakeup_list, pi_wakeup_list) {
> +		struct kvm_vcpu *vcpu = vt_to_vcpu(vt);
> +
> +		kvm_make_request(KVM_REQ_UNBLOCK, vcpu);

Hrm, KVM_REQ_UNBLOCK is going to cause spurious wakeups for the vCPU.  That isn't
the end of the world, but it's definitely undesirable, especially since these
flows are shared with SUSPEND+RESUME.

If we attach to CPUHP_AP_X86_KVM_CLK_ONLINE, can we do a bare __kvm_vcpu_wake_up(),
so that the task (temporarily) wakes up and gets migrated to a new pCPU before this
pCPU goes down?

> +		kvm_vcpu_wake_up(vcpu);
> +	}
> +	raw_spin_unlock_irqrestore(spinlock, flags);
> +}
> +
>  void __init pi_init_cpu(int cpu)
>  {
>  	INIT_LIST_HEAD(&per_cpu(wakeup_vcpus_on_cpu, cpu));
> diff --git a/arch/x86/kvm/vmx/posted_intr.h b/arch/x86/kvm/vmx/posted_intr.h
> index a4af39948cf04..41414c583a063 100644
> --- a/arch/x86/kvm/vmx/posted_intr.h
> +++ b/arch/x86/kvm/vmx/posted_intr.h
> @@ -11,6 +11,7 @@
>  void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu);
>  void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu);
>  void pi_wakeup_handler(void);
> +void pi_wakeup_cpu_offline(unsigned int cpu);
>  void __init pi_init_cpu(int cpu);
>  void pi_apicv_pre_state_restore(struct kvm_vcpu *vcpu);
>  bool pi_has_pending_interrupt(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 79468ddfe4736..ea0ded5d64509 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -9820,6 +9820,11 @@ void kvm_arch_disable_virtualization_cpu(void)
>  		__module_get(THIS_MODULE);
>  }
>  
> +void kvm_arch_prepare_cpu_offline(unsigned int cpu)

I don't love adding an arch hook for this, because CPUHP_AP_KVM_ONLINE is tied to
KVM_GENERIC_HARDWARE_ENABLING=y.  I think I'd rather turn CPUHP_AP_X86_KVM_CLK_ONLINE
into a slightly more generic CPUHP_AP_X86_KVM_ONLINE

> +{
> +	kvm_x86_call(prepare_cpu_offline)(cpu);

My vote for the names would just be "cpu_offline", i.e. no "prepare".

      reply	other threads:[~2026-09-24 14:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24  8:06 Jinmeng Zhou
2026-09-24 14:55 ` 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=arU57oqykUfeKqv2@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=feng.wu@intel.com \
    --cc=hpa@zytor.com \
    --cc=jinmengzhou22@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=tglx@kernel.org \
    --cc=weiguixiong@bytedance.com \
    --cc=x86@kernel.org \
    --cc=zhoujinmeng@bytedance.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®