[PATCH v2 05/15] lib: sbi: Disable m/scounteren & enable mcountinhibit

[Jessica Clarke]

On 27 May 2021, at 23:06, Atish Patra <atishp at atishpatra.org> wrote:
> 
> On Wed, May 26, 2021 at 5:36 PM Jessica Clarke <jrtc27 at jrtc27.com> wrote:
>> 
>> On 27 May 2021, at 01:30, Atish Patra <atish.patra at wdc.com> wrote:
>>> 
>>> Currently, all bits in mcountern are enabled unconditionally at
>>> boot time. With SBI PMU extension, this should enabled only during
>>> performance monitoring for a particular event except the TM bit. However,
>>> this is done only if mcountinhibit is implemented because the supervisor
>>> mode can not start/stop any event without mcountinhibit.
>>> 
>>> Similarly, supervisor should take care enabling scounteren which allows
>>> U-mode to access pmu counters. Disable all bits in scounteren in M-mode.
>> 
>> Turning off CY or IR seems like a bad idea to me. Similarly, all of CY, IR and
>> TM should be enabled in scounteren by default, this is just needlessly hostile.
> 
> This happens only if the platform supports mcountinhibit. As per the
> priv spec, no counter should run unless the corresponding bit in
> mcountinhibit
> is cleared. This patch just follows the spec.

I don’t understand your reply. The privileged spec just says how the CSRs
behave, it doesn’t say that you need to make an SBI call (which isn’t part of
the privileged spec) for cycle and instret to count. Requiring that is
unnecessarily hostile; in practice, everything will want those two counters to
be enabled, because arbitrary userspace software wants to use them. They should
never be disabled by OpenSBI unless S-mode explicitly asks for them to be
disabled.

Jess