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

[Christian Svensson]

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?

Thanks,
- Chris


On Thu, Mar 14, 2019 at 10:05 AM Cédric Le Goater <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>
> >
> > 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>
> > [clg: - checkpatch fixes ]
> > Signed-off-by: Cédric Le Goater <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;
> >
>
>