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

[Cédric Le Goater]

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;
> 

> --- Begin Message ---
>
>
>
> Subject: 
>
> Re: [Qemu-devel] [PATCH 0/5] aspeed/timer: Fix slowdowns in recent Linux
>
>
>
>
> Date: 
>
> Thu, 14 Mar 2019 01:59:59 -0700 (PDT)
>
>
>
>
> Patchew URL: https://patchew.org/QEMU/address@hidden/
>
>
>
> Hi,
>
> This series failed the address@hidden build test. Please find the testing 
> commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> time make address@hidden SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
>
>   CC      migration/xbzrle.o
>   CC      migration/postcopy-ram.o
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c: In function 
> 'aspeed_timer_set_value':
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:19: error: '__int128_t' 
> undeclared (first use in this function); did you mean '__int128'?
>                  ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
>                    ^~~~~~~~~~
>                    __int128
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:19: note: each undeclared 
> identifier is reported only once for each function it appears in
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:30: error: expected ')' before 
> 'delta'
>                  ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
>                  ~            ^~~~~
>                               )
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c:276:22: error: unused variable 
> 'rate' [-Werror=unused-variable]
>              uint32_t rate = calculate_rate(t);
>                       ^~~~
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c:275:21: error: unused variable 
> 'delta' [-Werror=unused-variable]
>              int64_t delta = (int64_t) value - (int64_t) calculate_ticks(t, 
> now);
>                      ^~~~~
> cc1: all warnings being treated as errors
>
>
> The full log is available at
> http://patchew.org/logs/address@hidden/address@hidden/?type=message.
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to address@hidden
>
> --- End Message ---