You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
From #114 the discussion on the mailinglist was to use this following patch instead and upstream it straight to mainline. Not really sure about the reasons why but why not.
Quoting myself:
Ah, yes - I missed that there were two - one for systems with int128 and one for them
without. Hm, that does complicate things.
Given that we only have one operand that is ever negative, how do you feel about a patch
that goes along the lines of:
if (delta >= 0) {
t->start += muldiv64(delta, NANOSECONDS_PER_SECOND, rate);
} else {
t->start = (int64_t)t->start - muldiv64(-delta, NANOSECONDS_PER_SECOND, rate);
}
That should avoid any issues I think, save us/me from implementing a signed muldiv,
but cost a bit on the readable code side.
The text was updated successfully, but these errors were encountered:
It's complicated, but OpenBMC upstream took the original patch with a caveat note that that particular patch probably wouldn't make it mainline upstream. So this issue is to patch the mainline upstream.
From 5719d0770f212b2d3c3e9e30dd7e585b83a7f92b Mon Sep 17 00:00:00 2001
From: Christian Svensson <[email protected]>
Date: Thu, 14 Mar 2019 10:41:21 +0100
Subject: [PATCH] aspeed/timer: Ensure positive muldiv delta
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 delta being operated on is positive.
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 <[email protected]>
---
hw/timer/aspeed_timer.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index 5c786e5128..bd65b9a167 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -261,7 +261,11 @@ 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);
+ if (delta >= 0) {
+ t->start += muldiv64(delta, NANOSECONDS_PER_SECOND, rate);
+ } else {
+ t->start -= muldiv64(-delta, NANOSECONDS_PER_SECOND, rate);
+ }
aspeed_timer_mod(t);
}
break;
--
2.21.0.360.g471c308f928-goog
From #114 the discussion on the mailinglist was to use this following patch instead and upstream it straight to mainline. Not really sure about the reasons why but why not.
Quoting myself:
The text was updated successfully, but these errors were encountered: