Re: [PATCH v2 2/2] hw/riscv: Provide rdtime callback for TCG in CLINT emulation

[Anup Patel]

On Thu, Jan 30, 2020 at 8:19 PM Palmer Dabbelt <address@hidden> 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 <address@hidden>
> > Reviewed-by: Alistair Francis <address@hidden>
> > ---
> >  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