All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Hao Peng <flyingpenghao@gmail.com>
Cc: pbonzini@redhat.com, kvm@vger.kernel.org,
	 Peng Hao <flyingpeng@tencent.com>
Subject: Re: [PATCH] KVM: x86: Guard against division by zero in adjust_lapic_timer_advance()
Date: Mon, 12 Oct 2026 06:11:25 -0700	[thread overview]
Message-ID: <aszcfeG_qPq_2x9w@google.com> (raw)
In-Reply-To: <CAPm50aJ6uudDOAC-XN1qQAd=aEtKYVgKuQWwM7cmLOsruQR9xw@mail.gmail.com>

On Sat, Oct 10, 2026, Hao Peng wrote:
> On Sat, Oct 10, 2026 at 4:35 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Thu, Oct 08, 2026, Peng Hao wrote:
> > > When TSC calibration fails and userspace never programs a tsc khz,
> > > vcpu->arch.virtual_tsc_khz stays zero, but the lapic timer's
> > > software-mode expiry path still calls adjust_lapic_timer_advance()
> > > unconditionally on every timer expiration (unlike
> > > __wait_lapic_expire(), this path is not gated on tsc_scaling_ratio).
> > > Both the "too early" and "too late" branches do_div() by
> > > virtual_tsc_khz, so a zero khz crashes the host with #DE as soon as
> > > the timer advance drifts outside the adjust window.
> > >
> > > Bail out before either division.
> > >
> > > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> > > ---
> > >  arch/x86/kvm/lapic.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > > index 13e32c4915e2..4dcbc66c8e07 100644
> > > --- a/arch/x86/kvm/lapic.c
> > > +++ b/arch/x86/kvm/lapic.c
> > > @@ -1991,6 +1991,9 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu,
> > >           abs(advance_expire_delta) < LAPIC_TIMER_ADVANCE_ADJUST_MIN)
> > >               return;
> > >
> > > +     if (unlikely(!vcpu->arch.virtual_tsc_khz))
> > > +             return;
> >
> > Ugh.  I'd rather reject KVM_CREATE_VCPU, even though there is a rather surprising
> > amount of code in KVM that does indeed play nice with tsc_khz == 0.  I don't see
> > how the guest can possibly function with virtual_tsc_khz==0.
> >
> Agreed the guest is broken in that state -- virtual_tsc_mult is zero too, so
> nsec_to_cycles() returns 0 and the deadline timer fires immediately. But I don't
> think KVM_CREATE_VCPU is where to enforce it, because letting vCPU creation
>  succeed is what makes the state recoverable:
>   - kvm_arch_vcpu_create() deliberately ignores kvm_set_tsc_khz()'s -1
>   - KVM_SET_TSC_KHZ is only reachable once the vCPU exists. With
>       kvm_caps.has_tsc_control and a khz below kvm_caps.max_guest_tsc_khz it
>       recomputes virtual_tsc_{shift,mult} and the guest is fully
> functional again.
>  - max_tsc_khz does the same thing ahead of time, via default_tsc_khz.

I don't see how it's recoverable.  kvm_set_tsc_khz() will compute thresh_{lo,hi}=0,
and so use_scaling will always be true.  Which means this:

	ratio = mul_u64_u32_div(1ULL << kvm_caps.tsc_scaling_ratio_frac_bits,
				user_tsc_khz, tsc_khz);

will again hit a #DE, no?

Even if KVM somehow makes it past that, kvm_guest_time_update() will put the vCPU
into an infinite request loop as get_cpu_tsc_khz() will always return 0 for modern
CPUs with a constant TSC:

  static unsigned long get_cpu_tsc_khz(void)
  {
	if (cpu_feature_enabled(X86_FEATURE_CONSTANT_TSC))
		return tsc_khz;    <======================== always zero
	else
		return __this_cpu_read(cpu_tsc_khz);
  }

And so this code will re-request KVM_REQ_CLOCK_UPDATE and the vCPU will hang.

	tgt_tsc_hz = (u64)get_cpu_tsc_khz() * HZ_PER_KHZ;
	if (unlikely(tgt_tsc_hz == 0)) {
		preempt_enable();
		kvm_make_request(KVM_REQ_CLOCK_UPDATE, v);
		return 1;
	}


      reply	other threads:[~2026-10-12 13:11 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-08  9:00 [PATCH] KVM: x86: Guard against division by zero in adjust_lapic_timer_advance() Peng Hao
2026-10-08  9:15 ` sashiko-bot
2026-10-09 20:35 ` Sean Christopherson
2026-10-10  8:59   ` Hao Peng
2026-10-12 13:11     ` 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=aszcfeG_qPq_2x9w@google.com \
    --to=seanjc@google.com \
    --cc=flyingpeng@tencent.com \
    --cc=flyingpenghao@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.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.