Re: [Qemu-devel] [PATCH 5/5] aspeed/timer: Use signed muldiv for timer resets

[Cédric Le Goater]

On 3/14/19 11:47 AM, Christian Svensson wrote:
> Hi all,
> 
> I have a new patch but I'm not sure how you want me to post it.
> Should I do a "PATCH v2" with a single patch and this thread as the thread ID?

I would wait for feedback from Peter first. 

Thanks,

C. 

> Thanks,
> - Chris
> 
> 
> On Thu, Mar 14, 2019 at 10:05 AM Cédric Le Goater <address@hidden 
> <mailto:address@hidden>> wrote:
> 
>     Christian,
> 
>     Could you please provide a fix for this patch ? patchew complains, see
>     attached log.
> 
>     Thanks,
> 
>     C.
> 
>     On 3/14/19 9:42 AM, Cédric Le Goater wrote:
>     > From: Christian Svensson <address@hidden <mailto:address@hidden>>
>     >
>     > If the host decrements the counter register that results in a negative
>     > delta. This is then passed to muldiv64 which only handles unsigned
>     > numbers resulting in bogus results.
>     >
>     > This fix ensures the data being operated on is signed before it is
>     > ultimately casted to the final unsigned value.
>     >
>     > Test case: kexec a kernel using aspeed_timer and it will freeze on the
>     > second bootup when the kernel initializes the timer. With this patch
>     > that no longer happens and the timer appears to run OK.
>     >
>     > Signed-off-by: Christian Svensson <address@hidden 
> <mailto:address@hidden>>
>     > [clg: - checkpatch fixes ]
>     > Signed-off-by: Cédric Le Goater <address@hidden <mailto:address@hidden>>
>     > ---
>     >  hw/timer/aspeed_timer.c | 3 ++-
>     >  1 file changed, 2 insertions(+), 1 deletion(-)
>     >
>     > diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
>     > index 9988b8fbbf17..0b16eac8970c 100644
>     > --- a/hw/timer/aspeed_timer.c
>     > +++ b/hw/timer/aspeed_timer.c
>     > @@ -275,7 +275,8 @@ static void 
> aspeed_timer_set_value(AspeedTimerCtrlState *s, int timer, int reg,
>     >              int64_t delta = (int64_t) value - (int64_t) 
> calculate_ticks(t, now);
>     >              uint32_t rate = calculate_rate(t);
>     > 
>     > -            t->start += muldiv64(delta, NANOSECONDS_PER_SECOND, rate);
>     > +            t->start = (int64_t)t->start +
>     > +                ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
>     >              aspeed_timer_mod(t);
>     >          }
>     >          break;
>     >
>