[PATCH v1 2/2] lib: sbi: Implement Sstc extension
Atish Patra
atishp at atishpatra.org
Fri Apr 22 13:25:07 PDT 2022
On Wed, Apr 13, 2022 at 10:21 PM Anup Patel <anup at brainfault.org> wrote:
>
> On Mon, Feb 28, 2022 at 1:56 PM Atish Patra <atishp at rivosinc.com> wrote:
> >
> > Recently, Sstc extension was ratified. It defines stimecmp which allows
> > the supervisor mode to directly update the timecmp value without the
> > need of the SBI call. The hardware also can inject the S-mode timer
> > interrupt direclty to the supervisor without going through the M-mode.
> > To maintain backward compatibility with the older software, SBI call
> > now uses stimecmp directly if the hardware supports.
> >
> > Implement the Sstc extension.
> >
> > Signed-off-by: Atish Patra <atishp at rivosinc.com>
>
> Please rebase this patch on latest OpenSBI. Also, see a few comments below.
>
> > ---
> > include/sbi/riscv_encoding.h | 4 ++++
> > include/sbi/sbi_hart.h | 4 +++-
> > lib/sbi/sbi_hart.c | 28 ++++++++++++++++++++++++++++
> > lib/sbi/sbi_timer.c | 25 ++++++++++++++++++++++---
> > 4 files changed, 57 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
> > index c0020f43f022..cce66ddc8b12 100644
> > --- a/include/sbi/riscv_encoding.h
> > +++ b/include/sbi/riscv_encoding.h
> > @@ -312,6 +312,10 @@
> > #define CSR_STVAL 0x143
> > #define CSR_SIP 0x144
> >
> > +/* Sstc extension */
> > +#define CSR_STIMECMP 0x14D
> > +#define CSR_STIMECMPH 0x15D
> > +
> > /* Supervisor Protection and Translation */
> > #define CSR_SATP 0x180
> >
> > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> > index 9e5a2b416017..93be7eddd6f1 100644
> > --- a/include/sbi/sbi_hart.h
> > +++ b/include/sbi/sbi_hart.h
> > @@ -28,9 +28,11 @@ enum sbi_hart_features {
> > SBI_HART_HAS_AIA = (1 << 5),
> > /** HART has MENVCFG CSR */
> > SBI_HART_HAS_MENVCFG = (1 << 6),
> > + /** HART has SSTC extension implemented in hardware */
> > + SBI_HART_HAS_SSTC = (1 << 7),
> >
> > /** Last index of Hart features*/
> > - SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_MENVCFG,
> > + SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
> > };
> >
> > struct sbi_scratch;
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index e08d9d32266a..8c9cad219c71 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -43,6 +43,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> > int cidx;
> > unsigned int num_mhpm = sbi_hart_mhpm_count(scratch);
> > uint64_t mhpmevent_init_val = 0;
> > + unsigned long menvcfg_val = 0;
> >
> > /* Enable FPU */
> > if (misa_extension('D') || misa_extension('F'))
> > @@ -54,6 +55,23 @@ static void mstatus_init(struct sbi_scratch *scratch)
> >
> > csr_write(CSR_MSTATUS, mstatus_val);
> >
> > + /*
> > + * The spec doesn't explicitly describe the reset value of menvcfg.
> > + * Enable access to stimecmp if sstc extension is present in the
> > + * hardware.
> > + */
> > + if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSTC)) {
> > +#if __riscv_xlen == 32
> > + menvcfg_val = csr_read(CSR_MENVCFGH);
> > + menvcfg_val |= MENVCFGH_STCE;
> > + csr_write(CSR_MENVCFGH, menvcfg_val);
> > +#else
> > + menvcfg_val = csr_read(CSR_MENVCFG);
> > + menvcfg_val |= MENVCFG_STCE;
> > + csr_write(CSR_MENVCFG, menvcfg_val);
> > +#endif
> > + }
> > +
> > /* Disable user mode usage of all perf counters except default ones (CY, TM, IR) */
> > if (misa_extension('S') &&
> > sbi_hart_has_feature(scratch, SBI_HART_HAS_SCOUNTEREN))
> > @@ -303,6 +321,8 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
> > break;
> > case SBI_HART_HAS_AIA:
> > fstr = "aia";
> > + case SBI_HART_HAS_SSTC:
> > + fstr = "sstc";
> > break;
> > default:
> > break;
> > @@ -533,6 +553,14 @@ __mhpm_skip:
> > if (!trap.cause)
> > hfeatures->features |= SBI_HART_HAS_MENVCFG;
> >
> > + /**
> > + * Detect if hart supports stimecmp CSR(Sstc extension) and menvcfg is
> > + * implemented.
> > + */
> > + csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
> > + if (!trap.cause && sbi_hart_has_feature(scratch, SBI_HART_HAS_MENVCFG))
> > + hfeatures->features |= SBI_HART_HAS_SSTC;
> > +
>
> You need to set menvcfg.STCE before detecting stimecmp CSR and restore
> menvcfg after detecting stimecmp CSR.
>
That won't be required. The STCE bit in menvcfg controls access of
stimecmp CSR from priv modes other than M.
Here is the relevant snippet from the spec.
Bit 63 of menvcfg (or bit 31 of menvcfgh) - named STCE (STimecmp
Enable) - enables stimecmp
for S-mode when set to one, and the same bit of henvcfg enables
vstimecmp for VS-mode.
When STCE in menvcfg is zero, an attempt to access stimecmp or
vstimecmp in a mode other
than M-mode raises an illegal instruction exception,
>
> > /* Detect if hart has AIA local interrupt CSRs */
> > csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
> > if (trap.cause)
> > diff --git a/lib/sbi/sbi_timer.c b/lib/sbi/sbi_timer.c
> > index acdba922b1e9..c89c7ca51100 100644
> > --- a/lib/sbi/sbi_timer.c
> > +++ b/lib/sbi/sbi_timer.c
> > @@ -124,16 +124,35 @@ void sbi_timer_set_delta_upper(ulong delta_upper)
> > void sbi_timer_event_start(u64 next_event)
> > {
> > sbi_pmu_ctr_incr_fw(SBI_PMU_FW_SET_TIMER);
> > - if (timer_dev && timer_dev->timer_event_start)
> > +
> > + /**
> > + * Update the stimecmp directly if available. This allows
> > + * the older software to leverage sstc extension on newer hardware.
> > + */
> > + if (sbi_hart_has_feature(sbi_scratch_thishart_ptr(), SBI_HART_HAS_SSTC)) {
> > +#if __riscv_xlen == 32
> > + csr_write(CSR_STIMECMP, next_event & 0xFFFFFFFF);
> > + csr_write(CSR_STIMECMPH, next_event >> 32);
> > +#else
> > + csr_write(CSR_STIMECMP, next_event);
> > +#endif
> > + } else if (timer_dev && timer_dev->timer_event_start) {
> > timer_dev->timer_event_start(next_event);
> > - csr_clear(CSR_MIP, MIP_STIP);
> > + csr_clear(CSR_MIP, MIP_STIP);
> > + }
>
> At this point in time, I think it is best to directly set stimecmp CSR
> upon SBI call because mip.STIP is read-only when Sstc extension
> is implemented.
>
> In future, we can think of an alternative approach to avoid touching
> stimecmp CSR from SBI call.
>
> > csr_set(CSR_MIE, MIP_MTIP);
> > }
> >
> > void sbi_timer_process(void)
> > {
> > csr_clear(CSR_MIE, MIP_MTIP);
> > - csr_set(CSR_MIP, MIP_STIP);
> > + /*
> > + * If sstc extension is available, supervisor can receive the timer
> > + * direclty without M-mode come in between. This function should
>
> s/direclty/directly/
>
> > + * only invoked if M-mode programs the timer for its own purpose.
> > + */
> > + if (!sbi_hart_has_feature(sbi_scratch_thishart_ptr(), SBI_HART_HAS_SSTC))
> > + csr_set(CSR_MIP, MIP_STIP);
> > }
> >
> > const struct sbi_timer_device *sbi_timer_get_device(void)
> > --
> > 2.30.2
> >
> >
> > --
> > opensbi mailing list
> > opensbi at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
>
> Regards,
> Anup
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
--
Regards,
Atish
More information about the opensbi
mailing list