On 11:23 Fri 07 Nov , Luc Michel wrote: > Hello, > > By working locally on a RISC-V CPU with the sstc extension, I noticed > that the sstc code (the `riscv_timer_write_timecmp' function) was > implicitly assuming that the object behind the env->rdtime_fn callback > was a ACLINT. This is not a correct assumption from the point of view of > the riscv_cpu_set_rdtime_fn function API since env->rdtime_fn_arg is a > `void *' and is not required to be a ACLINT. > > I reworked this and ended up with this series. It introduces the > RISCVCPUTimeSrcIf interface to replace the env->rdtime_fn callback and > break this dependency. This interface provides a mean to retrieve the > number of ticks (same as the rdtime_fn callback), and the tick frequency > that `riscv_timer_write_timecmp' needs. > > This will effectively allow other platforms with CPUs and the sstc > extension but no ACLINT to provide their own time source. For now only > the ACLINT implements this interface. > > The last two patches enhance the interface with a tick change notifier > registering mechanism. This allows the time source user (the CPU) to get > notified when the time source tick counter gets asynchronously modified > (reset to 0, ...), i.e., when the time register is written to. This is > then implemented in the ACLINT so that it does not have to include > time_helper.h and tinker with the CPU internals. This again will allow > new sources to be implemented more easily. It also ease maintenance by > keeping internal CPU mechanics contained into the RISC-V target code and > avoid potential duplication. > > Note that I would have liked to put the time_src interface as a qdev > link property on the CPU but given the current state of the various > RISC-V machines, this is not easy to do. Most of the time the CPU gets > realized before the ACLINT so it is too late to set the link property. > This would require further refactoring. > > Tested using `make check' and by booting Linux v6.17.6 in the virt > machine with 4 CPUs. I can see an initial `time' register write > (probably initial reset or OpenSBI) that correctly notifies the CPUs. > > Thanks > > Luc
Hi, Ping, patches missing review: 8 and 9. Thanks -- Luc > > Luc Michel (9): > target/riscv: drop unused include directive in time_helper.c > hw/intc/riscv_aclint: fix coding style > hw/intc/riscv_aclint: rename cpu_riscv_read_rtc to > riscv_aclint_mtimer_get_ticks > target/riscv: add the RISCVCPUTimeSrcIf interface > hw/intc/riscv_aclint: implement the RISCVCPUTimeSrcIf interface > target/riscv: replace env->rdtime_fn with a time source > hw/intc/riscv_aclint: riscv_aclint_mtimer_get_ticks: get rid of void* > argument > target/riscv: RISCVCPUTimeSrcIf: add register_time_change_notifier > hw/intc/riscv_aclint: implement the register_time_change_notifier > method > > include/hw/intc/riscv_aclint.h | 1 + > target/riscv/cpu-qom.h | 41 ++++++++++++++++++ > target/riscv/cpu.h | 9 ++-- > target/riscv/time_helper.h | 27 ++++++++++++ > hw/intc/riscv_aclint.c | 76 ++++++++++++++++++++++++---------- > target/riscv/cpu_helper.c | 7 ---- > target/riscv/csr.c | 24 +++++------ > target/riscv/time_helper.c | 42 +++++++++++++++---- > 8 files changed, 173 insertions(+), 54 deletions(-) > > -- > 2.51.0 > --
