Re: [PATCH v4 18/47] x86/kvm: Get local APIC bus frequency from PV CPUID Timing Info

[David Woodhouse <dwmw2@xxxxxxxxxxxxx>]

On Fri, 29 May 2026 11:24:50 -0700, Sean Christopherson wrote:
> On Fri, May 29, 2026, sashiko-bot@xxxxxxxxxx wrote:
> > > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> > > index 4fe9c69bf40b3..c1139182121d5 100644
> > > --- a/arch/x86/kernel/kvm.c
> > > +++ b/arch/x86/kernel/kvm.c
> > 
> > [ ... ]
> > 
> > > @@ -1039,6 +1040,13 @@ static void __init kvm_init_platform(void)
> > >                     x86_init.hyper.get_tsc_khz = kvm_get_tsc_khz;
> > >                     x86_init.hyper.get_cpu_khz = kvm_get_tsc_khz;
> > >             }
> > > +
> > > +#ifdef CONFIG_X86_LOCAL_APIC
> > > +           /* The leaf also includes the local APIC bus/timer 
> > > frequency.*/
> > > +           apic_khz = cpuid_ebx(timing_info_leaf);
> > > +           if (apic_khz)
> > > +                  lapic_timer_period = apic_khz * 1000 / HZ;
> > 
> > [Severity: High]
> > Could the intermediate multiplication of apic_khz * 1000 overflow here?
> > 
> > Since apic_khz is a u32, the multiplication is evaluated in 32-bit unsigned
> > arithmetic. If the hypervisor reports an APIC timer frequency greater than
> > ~4.29 GHz (apic_khz > 4294967), this calculation will silently overflow.
> 
> Hmm, easy enough to use mul_u64_u32_div() (I think that's the write helper for
> this?).

Yep.

> But this problem pre-exits in almost every other path that sets 
> lapic_timer_period.
> So while I tried to avoid doing yet more tangentially related cleanup, it 
> seems
> like adding a helper to set lapic_timer_period is the way to go.  That would 
> also
> allow making lapic_timer_period local to arch/x86/kernel/apic/apic.c.
> 
> *sigh*

Yay, more patches!

Reviewed-by: David Woodhouse <dwmw@xxxxxxxxxxxx>

Attachment: smime.p7s
Description: S/MIME cryptographic signature