Re: [PATCH] riscv: enable TIME CSR in U mode

[Nick Kossifidis <mick@ics.forth.gr>]

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.

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@ics.forth.gr>
> Sent: Thursday, March 4, 2021 5:35 PM
> To: Gary Guo <gary@garyguo.net>
> Cc: linux-riscv@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@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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv