Skip to content

Commit 7411240

Browse files
rajnesh-kanwalalistair23
authored andcommitted
target/riscv: More accurately model priv mode filtering.
In case of programmable counters configured to count inst/cycles we often end-up with counter not incrementing at all from kernel's perspective. For example: - Kernel configures hpm3 to count instructions and sets hpmcounter to -10000 and all modes except U mode are inhibited. - In QEMU we configure a timer to expire after ~10000 instructions. - Problem is, it's often the case that kernel might not even schedule Umode task and we hit the timer callback in QEMU. - In the timer callback we inject the interrupt into kernel, kernel runs the handler and reads hpmcounter3 value. - Given QEMU maintains individual counters to count for each privilege mode, and given umode never ran, the umode counter didn't increment and QEMU returns same value as was programmed by the kernel when starting the counter. - Kernel checks for overflow using previous and current value of the counter and reprograms the counter given there wasn't an overflow as per the counter value. (Which itself is a problem. We have QEMU telling kernel that counter3 overflowed but the counter value returned by QEMU doesn't seem to reflect that.). This change makes sure that timer is reprogrammed from the handler if the counter didn't overflow based on the counter value. Second, this change makes sure that whenever the counter is read, it's value is updated to reflect the latest count. Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Message-ID: <20240711-smcntrpmf_v7-v8-11-b7c38ae7b263@rivosinc.com> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
1 parent 22c721c commit 7411240

File tree

3 files changed

+33
-4
lines changed

3 files changed

+33
-4
lines changed

target/riscv/csr.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,9 @@ static target_ulong riscv_pmu_ctr_get_fixed_counters_val(CPURISCVState *env,
10391039
goto done;
10401040
}
10411041

1042+
/* Update counter before reading. */
1043+
riscv_pmu_update_fixed_ctrs(env, env->priv, env->virt_enabled);
1044+
10421045
if (!(cfg_val & MCYCLECFG_BIT_MINH)) {
10431046
curr_val += counter_arr[PRV_M];
10441047
}
@@ -1122,7 +1125,7 @@ static RISCVException write_mhpmcounterh(CPURISCVState *env, int csrno,
11221125
return RISCV_EXCP_NONE;
11231126
}
11241127

1125-
static RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val,
1128+
RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val,
11261129
bool upper_half, uint32_t ctr_idx)
11271130
{
11281131
PMUCTRState *counter = &env->pmu_ctrs[ctr_idx];

target/riscv/pmu.c

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,8 @@ static void pmu_timer_trigger_irq(RISCVCPU *cpu,
425425
target_ulong *mhpmevent_val;
426426
uint64_t of_bit_mask;
427427
int64_t irq_trigger_at;
428+
uint64_t curr_ctr_val, curr_ctrh_val;
429+
uint64_t ctr_val;
428430

429431
if (evt_idx != RISCV_PMU_EVENT_HW_CPU_CYCLES &&
430432
evt_idx != RISCV_PMU_EVENT_HW_INSTRUCTIONS) {
@@ -454,6 +456,26 @@ static void pmu_timer_trigger_irq(RISCVCPU *cpu,
454456
return;
455457
}
456458

459+
riscv_pmu_read_ctr(env, (target_ulong *)&curr_ctr_val, false, ctr_idx);
460+
ctr_val = counter->mhpmcounter_val;
461+
if (riscv_cpu_mxl(env) == MXL_RV32) {
462+
riscv_pmu_read_ctr(env, (target_ulong *)&curr_ctrh_val, true, ctr_idx);
463+
curr_ctr_val = curr_ctr_val | (curr_ctrh_val << 32);
464+
ctr_val = ctr_val |
465+
((uint64_t)counter->mhpmcounterh_val << 32);
466+
}
467+
468+
/*
469+
* We can not accommodate for inhibited modes when setting up timer. Check
470+
* if the counter has actually overflowed or not by comparing current
471+
* counter value (accommodated for inhibited modes) with software written
472+
* counter value.
473+
*/
474+
if (curr_ctr_val >= ctr_val) {
475+
riscv_pmu_setup_timer(env, curr_ctr_val, ctr_idx);
476+
return;
477+
}
478+
457479
if (cpu->pmu_avail_ctrs & BIT(ctr_idx)) {
458480
/* Generate interrupt only if OF bit is clear */
459481
if (!(*mhpmevent_val & of_bit_mask)) {
@@ -475,7 +497,7 @@ void riscv_pmu_timer_cb(void *priv)
475497

476498
int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, uint32_t ctr_idx)
477499
{
478-
uint64_t overflow_delta, overflow_at;
500+
uint64_t overflow_delta, overflow_at, curr_ns;
479501
int64_t overflow_ns, overflow_left = 0;
480502
RISCVCPU *cpu = env_archcpu(env);
481503
PMUCTRState *counter = &env->pmu_ctrs[ctr_idx];
@@ -506,8 +528,10 @@ int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, uint32_t ctr_idx)
506528
} else {
507529
return -1;
508530
}
509-
overflow_at = (uint64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
510-
overflow_ns;
531+
curr_ns = (uint64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
532+
overflow_at = curr_ns + overflow_ns;
533+
if (overflow_at <= curr_ns)
534+
overflow_at = UINT64_MAX;
511535

512536
if (overflow_at > INT64_MAX) {
513537
overflow_left += overflow_at - INT64_MAX;

target/riscv/pmu.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,7 @@ int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value,
3636
uint32_t ctr_idx);
3737
void riscv_pmu_update_fixed_ctrs(CPURISCVState *env, target_ulong newpriv,
3838
bool new_virt);
39+
RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val,
40+
bool upper_half, uint32_t ctr_idx);
3941

4042
#endif /* RISCV_PMU_H */

0 commit comments

Comments
 (0)