Re: [Qemu-devel] [PATCH for 4.1 v3] target/riscv: Expose time CSRs when allowed by [m|s]counteren

From: Bin Meng <bmeng.cn@gmail.com>
To: Palmer Dabbelt <palmer@sifive.com>
Cc: "open list:RISC-V" <qemu-riscv@nongnu.org>,
	Sagar Karandikar <sagark@eecs.berkeley.edu>,
	Bastian Koppelmann <kbastian@mail.uni-paderborn.de>,
	Jonathan Behrens <fintelia@gmail.com>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	Alistair Francis <Alistair.Francis@wdc.com>
Subject: Re: [Qemu-devel] [PATCH for 4.1 v3] target/riscv: Expose time CSRs when allowed by [m|s]counteren
Date: Wed, 26 Jun 2019 14:54:06 +0800	[thread overview]
Message-ID: <CAEUhbmVdVXaFhM2b1U2d5G0My5Gq3cMM1zhQKU3n051KuXu4HQ@mail.gmail.com> (raw)
In-Reply-To: <mhng-94c196fb-ebb6-4075-afa9-96989bbc037c@palmer-si-x1e>

Hi Palmer,

On Tue, Jun 25, 2019 at 5:57 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 24 Jun 2019 16:03:20 PDT (-0700), fintelia@gmail.com wrote:
> > Apparently my previous message didn't make it out onto the list (sorry
> > about all these email glitches!). I've included the message again below.
> > Hopefully either a patch like this one or something simpler that just hard
> > codes mcounteren.TM to zero (so QEMU is at least conformant) can be merged
> > in time for 4.1.
> >
> > On Fri, Jun 14, 2019 at 8:55 AM Jonathan Behrens <jonathan@fintelia.io>
> > wrote:
> >
> >> I'm not sure that is accurate. Based on the discussion here
> >> <https://forums.sifive.com/t/possible-bug-in-counteren-csrs/2123> the
> >> HiFive Unleashed actually does support reading the timer CSR from
> >> unprivileged modes (from that discussion it does so a little too well...
> >> but it should presumably be fixed in later iterations of the processor).
> >> And even if no real hardware supported this capability, it still might make
> >> sense to provide it in QEMU as an optimization.
>
> time and cycle are different registers: rdtime should trap, but rdcycle should
> work.  The hardware traps rdtime into machine mode and emulates it via a memory
> mapped register.  The bug in the FU540-C000 appears to be related to the
> counter enables, not the presence of the counters.

I don't think rdtime is required to mandatorily trap.

Per privileged spec v1.10, chapter 3.1.17 Counter-Enable Registers
([m|h|s]counteren), it says:

"When the CY, TM, IR, or HPMn bit in the mcounteren register is clear,
attempts to read the cycle, time, instret, or hpmcountern register
while executing in S-mode or U-mode will cause an illegal instruction
exception."

In the same chapter, it also says:

"Implementations can convert reads of the time CSR into loads to the
memory-mapped mtime register, or hard-wire the TM bits in mxcounteren
to 0 and emulate this functionality in M-mode software."

So per my understanding when mcounteren.TM is set, rdtime should NOT trap.

Regards,
Bin