[PATCH] lib: sbi: Only enable TM bit in scounteren

[Jessica Clarke]

On 14 May 2025, at 01:25, Atish Patra <atishp at rivosinc.com> wrote:

> The S-mode should disable Cycle and instruction counter for user space
> to avoid side channel attacks. The Linux kernel already does this so that
> any random user space code shouldn't be able to monitor cycle/instruction
> without higher privilege mode involvement.
> 
> Remove the CY/IR bits in scountern in OpenSBI.

Isn’t this a breaking change? S-mode OSes that are happy to allow
U-mode to read the counters are now broken, such as FreeBSD. BBL always
set scounteren to all ones (with bits above 2 being pointless without
programming mhpmeventX), and OpenSBI did that until it switched to this
behaviour of CY|TM|IR. So whether or not it was explicit in the
specification what these were initialised to, S-mode OSes exist that
assumed CY, TM and IR at least were available. Please do not break
things; treat the firmware<->S-mode interface in the same way as
kernel<->userspace, i.e. don’t break userspace/S-mode.

OSes that want to opt out can. And if you really want to push for this,
you need to wait for OSes to be changed to explicitly initialise
scounteren and only do it once old versions are sufficiently rare. But
you can’t just go doing this with no warning / deprecation period.

NAK.

Jess

> Signed-off-by: Atish Patra <atishp at rivosinc.com>
> ---
> To: anup Patel <anup at brainfault.org>
> Cc: opensbi at lists.infradead.org
> ---
> lib/sbi/sbi_hart.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index fc4925b57625..177f356b7b77 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -49,10 +49,10 @@ static void mstatus_init(struct sbi_scratch *scratch)
> 
> csr_write(CSR_MSTATUS, mstatus_val);
> 
> - /* Disable user mode usage of all perf counters except default ones (CY, TM, IR) */
> + /* Disable user mode usage of all perf counters except TM */
> if (misa_extension('S') &&
>    sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> - csr_write(CSR_SCOUNTEREN, 7);
> + csr_write(CSR_SCOUNTEREN, 0x02);
> 
> /**
> * OpenSBI doesn't use any PMU counters in M-mode.
> 
> ---
> base-commit: 316daaf1c299c29ac46e52145f65521f48ec63b5
> change-id: 20250512-fix_scounteren-8b7b682bb864
> --
> Regards,
> Atish patra
> 
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi