[PATCH] riscv: enable TIME CSR in U mode

[Atish Patra]

On Tue, Mar 16, 2021 at 1:53 PM Nick Kossifidis <mick at ics.forth.gr> wrote:
>
> Then we also need to modify the current perf code that assumes e.g. that
> instret is enabled. I'd suggest that until we have an SBI interface
> (since we also need to make sure that the enabled counters are also set
> on mcounteren) for enabling / disabling counters, we leave this to the
> firmware.
>

Just FYI:
I just sent the revised version of SBI PMU extension patch to the unix
platform mailing list.
https://lists.riscv.org/g/tech-unixplatformspec/message/598

I am mostly done with OpenSBI/Linux kernel implementation as well and
will be sending patches
sometime this week.

> Regards,
> Nick
>
> Στις 2021-03-16 08:24, Gary Guo έγραψε:
> > I don't think we should let the firmware to decide which performance
> > counter CSRs can be used by the userspace. No counter should be
> > allowed unless there is a reason to allow it, like the timer CSR.
> > Also, OpenSBI is not the only SBI implementation,; there could be no
> > SBIs at all (i.e. M mode) or have other SBIs that have different
> > defaults.
> >
> > Best,
> > Gary
> >
> > -----Original Message-----
> > From: Nick Kossifidis <mick at ics.forth.gr>
> > Sent: Thursday, March 4, 2021 5:35 PM
> > To: Gary Guo <gary at garyguo.net>
> > Cc: linux-riscv at lists.infradead.org
> > Subject: Re: [PATCH] riscv: enable TIME CSR in U mode
> >
> > Στις 2021-02-17 00:12, Gary Guo έγραψε:
> >> After ad5d112 we let the user mode to use rdtime directly for time
> >> access. This works if the hardware does not implement the TIME CSR and
> >> traps to the firmware. The spec however does allow a hardware
> >> implementation to redirect the CSR access to the memory-mapped MTIME
> >> CSR, and if this is performed it will check {M,S}COUNTEREN CSR to see
> >> if it is allowed.
> >>
> >> To prevent unhandled illegal instruction fault on these platforms, we
> >> can request these CSRs to be enabled in U-mode. For platforms that
> >> does not support the TIME CSR to MTIME CSR conversion, they will
> >> hardwire corresponding bit in COUNTEREN to zero and this would be a
> >> no-op.
> >>
> >> Signed-off-by: Gary Guo <gary at garyguo.net>
> >> ---
> >>  arch/riscv/kernel/head.S | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S index
> >> 0a4e81b8dc79..5a7a62190342 100644
> >> --- a/arch/riscv/kernel/head.S
> >> +++ b/arch/riscv/kernel/head.S
> >> @@ -190,6 +190,9 @@ ENTRY(_start_kernel)  .align 2
> >>  pmp_done:
> >>
> >> +    /* Allow user-mode to access time CSR */
> >> +    csrw, CSR_MCOUNTEREN, 2
> >> +
> >>      /*
> >>       * The hartid in a0 is expected later on, and we have no firmware
> >>       * to hand it to us.
> >> @@ -197,6 +200,9 @@ pmp_done:
> >>      csrr a0, CSR_MHARTID
> >>  #endif /* CONFIG_RISCV_M_MODE */
> >>
> >> +    /* Allow user-mode to access time CSR */
> >> +    csrw CSR_SCOUNTEREN, 2
> >> +
> >>      /* Load the global pointer */
> >>  .option push
> >>  .option norelax
> >
> > Shouldn't we use csrrs here to preserve current settings from the
> > firmware ? Also OpenSBI at this point sets both MCOUNTEREN/SCOUNTEREN
> > to -1, does anyone else modify that value ?
> >
> > Regards,
> > Nick
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish