* [PATCH] riscv: enable TIME CSR in U mode
@ 2021-02-16 22:12 Gary Guo
2021-03-04 17:35 ` Nick Kossifidis
0 siblings, 1 reply; 5+ messages in thread
From: Gary Guo @ 2021-02-16 22:12 UTC (permalink / raw)
To: linux-riscv; +Cc: 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
--
2.20.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] riscv: enable TIME CSR in U mode 2021-02-16 22:12 [PATCH] riscv: enable TIME CSR in U mode Gary Guo @ 2021-03-04 17:35 ` Nick Kossifidis 2021-03-16 6:24 ` Gary Guo 0 siblings, 1 reply; 5+ messages in thread From: Nick Kossifidis @ 2021-03-04 17:35 UTC (permalink / raw) To: Gary Guo; +Cc: linux-riscv Στις 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] riscv: enable TIME CSR in U mode 2021-03-04 17:35 ` Nick Kossifidis @ 2021-03-16 6:24 ` Gary Guo 2021-03-16 20:52 ` Nick Kossifidis 0 siblings, 1 reply; 5+ messages in thread From: Gary Guo @ 2021-03-16 6:24 UTC (permalink / raw) To: Nick Kossifidis; +Cc: linux-riscv 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv: enable TIME CSR in U mode 2021-03-16 6:24 ` Gary Guo @ 2021-03-16 20:52 ` Nick Kossifidis 2021-03-17 2:50 ` Atish Patra 0 siblings, 1 reply; 5+ messages in thread From: Nick Kossifidis @ 2021-03-16 20:52 UTC (permalink / raw) To: Gary Guo; +Cc: Nick Kossifidis, linux-riscv 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv: enable TIME CSR in U mode 2021-03-16 20:52 ` Nick Kossifidis @ 2021-03-17 2:50 ` Atish Patra 0 siblings, 0 replies; 5+ messages in thread From: Atish Patra @ 2021-03-17 2:50 UTC (permalink / raw) To: Nick Kossifidis; +Cc: Gary Guo, linux-riscv On Tue, Mar 16, 2021 at 1:53 PM Nick Kossifidis <mick@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@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 -- Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-03-17 2:51 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-16 22:12 [PATCH] riscv: enable TIME CSR in U mode Gary Guo 2021-03-04 17:35 ` Nick Kossifidis 2021-03-16 6:24 ` Gary Guo 2021-03-16 20:52 ` Nick Kossifidis 2021-03-17 2:50 ` Atish Patra
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.