Re: [PATCH v16 03/16] arm64/delay: move, fixup usecs_to_cycles()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] qcom/rpmh: integer overflow in rpmh_xloops_to_cycles() causes premature wakeups
--

commit cfe9540cf38fefd87b6b20f22814052cba488f5d
Author: Ankur Arora <ankur.a.arora@oracle.com>

arm64/delay: move, fixup usecs_to_cycles()

Updates xloops_to_cycles() to use mul_u64_u64_shr() instead of 32-bit fixed
point arithmetic to avoid truncation for delay values larger than ~10s. Also
moves delay constants and functions to a new header and renames colliding
macros in qcom/rpmh-rsc.c.

> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index f881af35ecfc5..dabe0ccba3706 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c

[ ... ]

> @@ -146,14 +146,16 @@ enum {
>   *  +---------------------------------------------------+
>   */
>  
> -#define USECS_TO_CYCLES(time_usecs)			\
> -	xloops_to_cycles((time_usecs) * 0x10C7UL)
> -
> -static inline unsigned long xloops_to_cycles(u64 xloops)
> +static inline u64 rpmh_xloops_to_cycles(u64 xloops)
>  {
>  	return (xloops * loops_per_jiffy * HZ) >> 32;
>  }

[Severity: High]
Does this code overflow the 64-bit bounds for sleep delays longer than ~224
seconds?

The core arm64 delay code in this patch was updated to use 128-bit arithmetic
(mul_u64_u64_shr) to avoid overflow for delays larger than ~10s. However, it
looks like the old fixed-point math was preserved here during the rename to
rpmh_xloops_to_cycles().

>  
> +static inline u64 rpmh_usecs_to_cycles(u64 time_usecs)
> +{
> +	return rpmh_xloops_to_cycles((time_usecs) * 0x10C7UL);
> +}
> +

[ ... ]

> @@ -828,7 +830,7 @@ void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv)
>  	wakeup_us = ktime_to_us(wakeup);
>  
>  	/* Convert the wakeup to arch timer scale */
> -	wakeup_cycles = USECS_TO_CYCLES(wakeup_us);
> +	wakeup_cycles = rpmh_usecs_to_cycles(wakeup_us);

[Severity: High]
If wakeup_us is derived from the time to the next hrtimer, which could be
hours or days away during deep system idle/suspend, will passing this into
rpmh_usecs_to_cycles() program a prematurely small, corrupted cycle count
into the hardware due to the truncation mentioned above?

>  	wakeup_cycles += arch_timer_read_counter();
>  
>  exit:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016070837.440748-1-ankur.a.arora@oracle.com?part=3