Implement basic ACLINT support for the MTIMER register

[chiangkd]

Replace the timer defined in the emu_state_t structure, which originally compared with the instruction counter and triggered software timer interrupts, with the MTIMER memory-mapped peripheral. For each HART (currently only one), there is an MTIMECMP register connected to the MTIMER device.

Additionally, use the TIMER Extension to program the clock for scheduling the next timer event.

static inline sbi_ret_t handle_sbi_ecall_TIMER(vm_t *vm, int32_t fid)
{
    emu_state_t *data = PRIV(vm);
    switch (fid) {
    case SBI_TIMER__SET_TIMER:
        data->aclint.mtimecmp = vm->x_regs[RV_R_A0];
        data->aclint.mtimecmph = vm->x_regs[RV_R_A1];
        return (sbi_ret_t){SBI_SUCCESS, 0};
    default:
        return (sbi_ret_t){SBI_ERR_NOT_SUPPORTED, 0};
    }
}

[jserv]

I would like to ask @ranvd for reviewing and collaborating.

[jserv]

Replace the timer defined in the emu_state_t structure, which originally compared with the instruction counter and triggered software timer interrupts, with the MTIMER memory-mapped peripheral. For each HART (currently only one), there is an MTIMECMP register connected to the MTIMER device. Additionally, use the TIMER Extension to program the clock for scheduling the next timer event.

Could you provide some information on the approach used to validate ACLINT/MTIMER?

[shengwen-tw]

I'll take a look soon and maybe verify with virtio-gpu to see if there exists any potential problems.

[chiangkd]

Replace the timer defined in the emu_state_t structure, which originally compared with the instruction counter and triggered software timer interrupts, with the MTIMER memory-mapped peripheral. For each HART (currently only one), there is an MTIMECMP register connected to the MTIMER device. Additionally, use the TIMER Extension to program the clock for scheduling the next timer event.

Could you provide some information on the approach used to validate ACLINT/MTIMER?

Replacing timer with MTIMER perpheral also triggers timer interrupts normally.

Clarify that the MTIMER peripheral can also trigger timer interrupts within a scheduled period by monitoring /proc/interrupts reveals an increase in timer interrupts triggered by riscv-timer:

  1:        595  SiFive PLIC   1 Edge      ttyS0
  2:         19  SiFive PLIC   3 Edge      virtio1
  3:          0  SiFive PLIC   2 Edge      virtio0
  5:      58985  RISC-V INTC   5 Edge      riscv-timer
...
  1:        610  SiFive PLIC   1 Edge      ttyS0
  2:         19  SiFive PLIC   3 Edge      virtio1
  3:          0  SiFive PLIC   2 Edge      virtio0
  5:      59196  RISC-V INTC   5 Edge      riscv-timer
...
  1:        625  SiFive PLIC   1 Edge      ttyS0
  2:         19  SiFive PLIC   3 Edge      virtio1
  3:          0  SiFive PLIC   2 Edge      virtio0
  5:      59296  RISC-V INTC   5 Edge      riscv-timer

[jserv]

Clarify that the MTIMER peripheral can also trigger timer interrupts within a scheduled period by monitoring /proc/interrupts reveals an increase in timer interrupts triggered by riscv-timer:

  1:        595  SiFive PLIC   1 Edge      ttyS0
  2:         19  SiFive PLIC   3 Edge      virtio1
  3:          0  SiFive PLIC   2 Edge      virtio0
  5:      58985  RISC-V INTC   5 Edge      riscv-timer
...
  1:        610  SiFive PLIC   1 Edge      ttyS0
  2:         19  SiFive PLIC   3 Edge      virtio1
  3:          0  SiFive PLIC   2 Edge      virtio0
  5:      59196  RISC-V INTC   5 Edge      riscv-timer
...
  1:        625  SiFive PLIC   1 Edge      ttyS0
  2:         19  SiFive PLIC   3 Edge      virtio1
  3:          0  SiFive PLIC   2 Edge      virtio0
  5:      59296  RISC-V INTC   5 Edge      riscv-timer

The above should appear in git commit messages for future reference purpose.

[jserv]

If this pull request is ready for reviewing, use git rebase -i to squash and rework the git commit message. Then, request the reviewers via GitHub website.

[jserv]

I defer @ranvd for confirmation.

[jserv]

Related pull request: #46

[chiangkd]

After correcting the error above, the kernel gets stuck after logging out [ 0.010981] clint: registering percpu irq failed [-16]. This error may occur in my implementation as well. I'm still figuring out the reason.

If CONFIG_RISCV_M_MODE is not enable, RV_IRQ_SOFT and RV_IRQ_TIMER default to supervisor level interrupts IRQ_S_SOFT (1) and IRQ_S_TIMER (5). However, in the device tree, the CLINT IRQ numbers are set to 3 and 7, which can trigger invalid hardware interrupts:

[    0.002716] clint: clint@4400000: invalid irq 0 (hwirq 3)
[    0.003920] Failed to initialize '/soc@F0000000/clint@4400000': -19

To resolve this, directly set CLINT IRQ number with 1 and 5 for supervior software and timer interrupt:

clint0: clint@4400000 {
    #interrupt-cells = <1>;
    compatible = "sifive,clint0";
    reg = <0x4400000 0x000C000>;
    interrupts-extended = <&cpu0_intc 1 &cpu0_intc 5>;
};

However, this setup triggers the following warning:

[    0.007611] genirq: Flags mismatch irq 5. 00004400 (clint-timer) vs. 00004400 (riscv-timer)

This indicates that both clint-timer and riscv-timer are reporting the same set of flags 0x00004400 (IRQF_PERCPU | IRQF_NO_SUSPEND) for IRQ 5. riscv-timer is managed by the riscv,cpu-intc interrupt controller and should not be altered.

Does it make sense to prioritize adding M-mode privilege levels support for semu first?

[jserv]

Does it make sense to prioritize adding M-mode privilege levels support for semu first?

Yes, M-mode is the highest privilege mode and controls all physical resources and interrupts. Could you check with @ranvd to confirm if there might be any overlapping efforts with pull request #46 ?

[ranvd]

This indicates that both clint-timer and riscv-timer are reporting the same set of flags 0x00004400 (IRQF_PERCPU | IRQF_NO_SUSPEND) for IRQ 5. riscv-timer is managed by the riscv,cpu-intc interrupt controller and should not be altered.

Thank you for providing this information! I reviewed some .config files from some RISC-V simulator repositories and noticed that they don't enable the CLINT driver. So, I navigated the Kconfig at arch/riscv/Kconfig, and I found that CLINT_TIMER is selected by RISCV if !MMU, and RISCV_TIMER is selected by RISCV if RISCV_SBI is set. Besides, RISCV_SBI depends on !RISCV_M_MODE, and RISCV_M_MODE is !MMU by default.

Since we are running the Linux kernel in supervisor mode, RISCV_M_MODE is not set, and MMU is set by default. Can we say that we can't enable both CLINT_TIMER and RISCV_TIMER at the same time? Or at least it is not recommended.

Does it make sense to prioritize adding M-mode privilege levels support for semu first?

So, according to the clues above, I suggest not implementing M-mode for semu since semu already support SBI for the Linux kernel.

[chiangkd]

So, according to the clues above, I suggest not implementing M-mode for semu since semu already support SBI for the Linux kernel.

I agree with the previous discussion. The /drivers/clocksource/timer-clint.c file provides supporting evidence:

...
 * Most of the M-mode (i.e. NoMMU) RISC-V systems usually have a
 * CLINT MMIO timer device.
...

Additionally, riscv-timer.c mentions:

...
 * All RISC-V systems have a timer attached to every hart.  These timers can
 * either be read from the "time" and "timeh" CSRs, and can use the SBI to
 * setup events, or directly accessed using MMIO registers.
...

As this PR overlaps with PR #46, it should be closed. I will create a new PR to implement time CSR for generating software timer interrupts, originally triggered by instruction count:

if (vm.insn_count > emu.timer)
    vm.sip |= RV_INT_STI_BIT;
else
    vm.sip &= ~RV_INT_STI_BIT;

This new feature can be integrated after PR #46 is complete.