| Message ID | 20220811171619.1154755-7-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Implement FEAT_PMUv3p5 | expand |
On 8/11/22 10:16, Peter Maydell wrote: > In pmccntr_op_finish() and pmevcntr_op_finish() we calculate the next > point at which we will get an overflow and need to fire the PMU > interrupt or set the overflow flag. We do this by calculating the > number of nanoseconds to the overflow event and then adding it to > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL). However, we don't check > whether that signed addition overflows, which can happen if the next > PMU interrupt would happen massively far in the future (250 years or > more). > > Since QEMU assumes that "when the QEMU_CLOCK_VIRTUAL rolls over" is > "never", the sensible behaviour in this situation is simply to not > try to set the timer if it would be beyond that point. Detect the > overflow, and skip setting the timer in that case. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target/arm/helper.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 434885d024a..b7a420981f8 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -1227,10 +1227,13 @@ static void pmccntr_op_finish(CPUARMState *env) > int64_t overflow_in = cycles_ns_per(remaining_cycles); > > if (overflow_in > 0) { > - int64_t overflow_at = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > - overflow_in; > - ARMCPU *cpu = env_archcpu(env); > - timer_mod_anticipate_ns(cpu->pmu_timer, overflow_at); > + int64_t overflow_at; > + > + if (!sadd64_overflow(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), > + overflow_in, &overflow_at)) { Maybe my mailer, but it looks like alignment is off on the continuation line here... > + int64_t overflow_at; > + > + if (!sadd64_overflow(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), > + overflow_in, &overflow_at)) { but not here. Anyway, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/arm/helper.c b/target/arm/helper.c index 434885d024a..b7a420981f8 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -1227,10 +1227,13 @@ static void pmccntr_op_finish(CPUARMState *env) int64_t overflow_in = cycles_ns_per(remaining_cycles); if (overflow_in > 0) { - int64_t overflow_at = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + - overflow_in; - ARMCPU *cpu = env_archcpu(env); - timer_mod_anticipate_ns(cpu->pmu_timer, overflow_at); + int64_t overflow_at; + + if (!sadd64_overflow(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), + overflow_in, &overflow_at)) { + ARMCPU *cpu = env_archcpu(env); + timer_mod_anticipate_ns(cpu->pmu_timer, overflow_at); + } } #endif @@ -1275,10 +1278,13 @@ static void pmevcntr_op_finish(CPUARMState *env, uint8_t counter) int64_t overflow_in = pm_events[event_idx].ns_per_count(delta); if (overflow_in > 0) { - int64_t overflow_at = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + - overflow_in; - ARMCPU *cpu = env_archcpu(env); - timer_mod_anticipate_ns(cpu->pmu_timer, overflow_at); + int64_t overflow_at; + + if (!sadd64_overflow(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), + overflow_in, &overflow_at)) { + ARMCPU *cpu = env_archcpu(env); + timer_mod_anticipate_ns(cpu->pmu_timer, overflow_at); + } } #endif
In pmccntr_op_finish() and pmevcntr_op_finish() we calculate the next point at which we will get an overflow and need to fire the PMU interrupt or set the overflow flag. We do this by calculating the number of nanoseconds to the overflow event and then adding it to qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL). However, we don't check whether that signed addition overflows, which can happen if the next PMU interrupt would happen massively far in the future (250 years or more). Since QEMU assumes that "when the QEMU_CLOCK_VIRTUAL rolls over" is "never", the sensible behaviour in this situation is simply to not try to set the timer if it would be beyond that point. Detect the overflow, and skip setting the timer in that case. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)