| Message ID | 20200122112952.94284-3-anup.patel@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V TIME CSR for privileged mode | expand |
On Wed, 22 Jan 2020 11:30:36 GMT (+0000), Anup Patel wrote: > This patch extends CLINT emulation to provide rdtime callback for > TCG. This rdtime callback will be called wheneven TIME CSRs are > read in privileged modes. > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/sifive_clint.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c > index e5a8f75cee..805503dc27 100644 > --- a/hw/riscv/sifive_clint.c > +++ b/hw/riscv/sifive_clint.c > @@ -236,6 +236,7 @@ DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, uint32_t num_harts, > if (!env) { > continue; > } > + riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc); > env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, > &sifive_clint_timer_cb, cpu); > env->timecmp = 0; > -- > 2.17.1 Can you make this optional? Ideally via a command-line argument, but at a minimum as via the board configuration files. As it stands this will enable the direct rdtime implemnetation everywhere, and while that's sensible for the virt board I'd prefer to avoid changing the behavior of the sifive_u board in ways that differ from the hardware when that's easy.
On Thu, Jan 30, 2020 at 8:19 PM Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > On Wed, 22 Jan 2020 11:30:36 GMT (+0000), Anup Patel wrote: > > This patch extends CLINT emulation to provide rdtime callback for > > TCG. This rdtime callback will be called wheneven TIME CSRs are > > read in privileged modes. > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > hw/riscv/sifive_clint.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c > > index e5a8f75cee..805503dc27 100644 > > --- a/hw/riscv/sifive_clint.c > > +++ b/hw/riscv/sifive_clint.c > > @@ -236,6 +236,7 @@ DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, uint32_t num_harts, > > if (!env) { > > continue; > > } > > + riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc); > > env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, > > &sifive_clint_timer_cb, cpu); > > env->timecmp = 0; > > -- > > 2.17.1 > > Can you make this optional? Ideally via a command-line argument, but at a > minimum as via the board configuration files. As it stands this will enable > the direct rdtime implemnetation everywhere, and while that's sensible for the > virt board I'd prefer to avoid changing the behavior of the sifive_u board in > ways that differ from the hardware when that's easy. Command-line will unnecessary make things complicated for users. I think the better option is to make it board specific so that we can emulate exact HW behaviour in QEMU. This way since real-world SiFive unleashed board does not have TIME CSR even QEMU will not emulate TIME CSR for "sifive_u" machine. For now, we should definitely emulate TIME CSR for virt machine because of the performance improvement. Regards, Anup
diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c index e5a8f75cee..805503dc27 100644 --- a/hw/riscv/sifive_clint.c +++ b/hw/riscv/sifive_clint.c @@ -236,6 +236,7 @@ DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, uint32_t num_harts, if (!env) { continue; } + riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc); env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &sifive_clint_timer_cb, cpu); env->timecmp = 0;