* [PATCH 01/11] lib: sbi: Detect and print privileged spec version
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 1:42 ` Atish Patra
2022-04-29 15:51 ` [PATCH 02/11] lib: sbi: Remove 's' and 'u' from misa_string() output Anup Patel
` (10 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
It is possible to guess privileged spec versions based on the CSRs
that where introduced in different privleged spec versions. In future,
if we are not able guess privileged spec version then we can have
platform provide it.
We add privileged spec version as per-hart feature and try to guess
it based on presense of mcounteren, mcountinhibit, and menvcfg CSRs.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_hart.h | 15 ++++++++++++
lib/sbi/sbi_hart.c | 52 +++++++++++++++++++++++++++++++++++++-----
lib/sbi/sbi_init.c | 2 ++
3 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index 1d09374..3c37933 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -12,6 +12,18 @@
#include <sbi/sbi_types.h>
+/** Possible privileged specification versions of a hart */
+enum sbi_hart_priv_versions {
+ /** Unknown privileged specification */
+ SBI_HART_PRIV_VER_UNKNOWN = 0,
+ /** Privileged specification v1.10 */
+ SBI_HART_PRIV_VER_1_10 = 1,
+ /** Privileged specification v1.11 */
+ SBI_HART_PRIV_VER_1_11 = 2,
+ /** Privileged specification v1.12 */
+ SBI_HART_PRIV_VER_1_12 = 3,
+};
+
/** Possible feature flags of a hart */
enum sbi_hart_features {
/** Hart has S-mode counter enable */
@@ -56,6 +68,9 @@ unsigned long sbi_hart_pmp_granularity(struct sbi_scratch *scratch);
unsigned int sbi_hart_pmp_addrbits(struct sbi_scratch *scratch);
unsigned int sbi_hart_mhpm_bits(struct sbi_scratch *scratch);
int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
+int sbi_hart_priv_version(struct sbi_scratch *scratch);
+void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
+ char *version_str, int nvstr);
bool sbi_hart_has_feature(struct sbi_scratch *scratch, unsigned long feature);
void sbi_hart_get_features_str(struct sbi_scratch *scratch,
char *features_str, int nfstr);
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 229c14a..ed4e631 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -28,6 +28,7 @@ extern void __sbi_expected_trap_hext(void);
void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
struct hart_features {
+ int priv_version;
unsigned long features;
unsigned int pmp_count;
unsigned int pmp_addr_bits;
@@ -334,6 +335,39 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
return 0;
}
+int sbi_hart_priv_version(struct sbi_scratch *scratch)
+{
+ struct hart_features *hfeatures =
+ sbi_scratch_offset_ptr(scratch, hart_features_offset);
+
+ return hfeatures->priv_version;
+}
+
+void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
+ char *version_str, int nvstr)
+{
+ char *temp;
+ struct hart_features *hfeatures =
+ sbi_scratch_offset_ptr(scratch, hart_features_offset);
+
+ switch (hfeatures->priv_version) {
+ case SBI_HART_PRIV_VER_1_10:
+ temp = "v1.10";
+ break;
+ case SBI_HART_PRIV_VER_1_11:
+ temp = "v1.11";
+ break;
+ case SBI_HART_PRIV_VER_1_12:
+ temp = "v1.12";
+ break;
+ default:
+ temp = "unknown";
+ break;
+ }
+
+ sbi_snprintf(version_str, nvstr, "%s", temp);
+}
+
/**
* Check whether a particular hart feature is available
*
@@ -583,6 +617,7 @@ __mhpm_skip:
/* Detect if hart supports SCOUNTEREN feature */
val = csr_read_allowed(CSR_SCOUNTEREN, (unsigned long)&trap);
if (!trap.cause) {
+ hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
csr_write_allowed(CSR_SCOUNTEREN, (unsigned long)&trap, val);
if (!trap.cause)
hfeatures->features |= SBI_HART_HAS_SCOUNTEREN;
@@ -591,6 +626,7 @@ __mhpm_skip:
/* Detect if hart supports MCOUNTEREN feature */
val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
if (!trap.cause) {
+ hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
csr_write_allowed(CSR_MCOUNTEREN, (unsigned long)&trap, val);
if (!trap.cause)
hfeatures->features |= SBI_HART_HAS_MCOUNTEREN;
@@ -600,8 +636,17 @@ __mhpm_skip:
val = csr_read_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap);
if (!trap.cause) {
csr_write_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap, val);
- if (!trap.cause)
+ if (!trap.cause) {
+ hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
hfeatures->features |= SBI_HART_HAS_MCOUNTINHIBIT;
+ }
+ }
+
+ /* Detect if hart has menvcfg CSR */
+ csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
+ if (!trap.cause) {
+ hfeatures->priv_version = SBI_HART_PRIV_VER_1_12;
+ hfeatures->features |= SBI_HART_HAS_MENVCFG;
}
/* Counter overflow/filtering is not useful without mcounter/inhibit */
@@ -625,11 +670,6 @@ __mhpm_skip:
hfeatures->features |= SBI_HART_HAS_AIA;
__aia_skip:
- /* Detect if hart has menvcfg CSR */
- csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
- if (!trap.cause)
- hfeatures->features |= SBI_HART_HAS_MENVCFG;
-
/**
* Detect if hart supports stimecmp CSR(Sstc extension) and menvcfg is
* implemented.
diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
index 2b9e5ae..660b11f 100644
--- a/lib/sbi/sbi_init.c
+++ b/lib/sbi/sbi_init.c
@@ -139,6 +139,8 @@ static void sbi_boot_print_hart(struct sbi_scratch *scratch, u32 hartid)
/* Boot HART details */
sbi_printf("Boot HART ID : %u\n", hartid);
sbi_printf("Boot HART Domain : %s\n", dom->name);
+ sbi_hart_get_priv_version_str(scratch, str, sizeof(str));
+ sbi_printf("Boot HART Priv Version : %s\n", str);
misa_string(xlen, str, sizeof(str));
sbi_printf("Boot HART ISA : %s\n", str);
sbi_hart_get_features_str(scratch, str, sizeof(str));
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 01/11] lib: sbi: Detect and print privileged spec version
2022-04-29 15:51 ` [PATCH 01/11] lib: sbi: Detect and print privileged spec version Anup Patel
@ 2022-05-04 1:42 ` Atish Patra
2022-05-07 4:57 ` Anup Patel
0 siblings, 1 reply; 37+ messages in thread
From: Atish Patra @ 2022-05-04 1:42 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> It is possible to guess privileged spec versions based on the CSRs
> that where introduced in different privleged spec versions. In future,
/s/privleged/privileged
> if we are not able guess privileged spec version then we can have
> platform provide it.
>
> We add privileged spec version as per-hart feature and try to guess
> it based on presense of mcounteren, mcountinhibit, and menvcfg CSRs.
>
/s/presense/presence
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_hart.h | 15 ++++++++++++
> lib/sbi/sbi_hart.c | 52 +++++++++++++++++++++++++++++++++++++-----
> lib/sbi/sbi_init.c | 2 ++
> 3 files changed, 63 insertions(+), 6 deletions(-)
>
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index 1d09374..3c37933 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -12,6 +12,18 @@
>
> #include <sbi/sbi_types.h>
>
> +/** Possible privileged specification versions of a hart */
> +enum sbi_hart_priv_versions {
> + /** Unknown privileged specification */
> + SBI_HART_PRIV_VER_UNKNOWN = 0,
> + /** Privileged specification v1.10 */
> + SBI_HART_PRIV_VER_1_10 = 1,
> + /** Privileged specification v1.11 */
> + SBI_HART_PRIV_VER_1_11 = 2,
> + /** Privileged specification v1.12 */
> + SBI_HART_PRIV_VER_1_12 = 3,
> +};
> +
> /** Possible feature flags of a hart */
> enum sbi_hart_features {
> /** Hart has S-mode counter enable */
> @@ -56,6 +68,9 @@ unsigned long sbi_hart_pmp_granularity(struct sbi_scratch *scratch);
> unsigned int sbi_hart_pmp_addrbits(struct sbi_scratch *scratch);
> unsigned int sbi_hart_mhpm_bits(struct sbi_scratch *scratch);
> int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
> +int sbi_hart_priv_version(struct sbi_scratch *scratch);
> +void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> + char *version_str, int nvstr);
> bool sbi_hart_has_feature(struct sbi_scratch *scratch, unsigned long feature);
> void sbi_hart_get_features_str(struct sbi_scratch *scratch,
> char *features_str, int nfstr);
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 229c14a..ed4e631 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -28,6 +28,7 @@ extern void __sbi_expected_trap_hext(void);
> void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
>
> struct hart_features {
> + int priv_version;
> unsigned long features;
> unsigned int pmp_count;
> unsigned int pmp_addr_bits;
> @@ -334,6 +335,39 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
> return 0;
> }
>
> +int sbi_hart_priv_version(struct sbi_scratch *scratch)
> +{
> + struct hart_features *hfeatures =
> + sbi_scratch_offset_ptr(scratch, hart_features_offset);
> +
> + return hfeatures->priv_version;
> +}
> +
> +void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> + char *version_str, int nvstr)
> +{
> + char *temp;
> + struct hart_features *hfeatures =
> + sbi_scratch_offset_ptr(scratch, hart_features_offset);
> +
> + switch (hfeatures->priv_version) {
> + case SBI_HART_PRIV_VER_1_10:
> + temp = "v1.10";
> + break;
> + case SBI_HART_PRIV_VER_1_11:
> + temp = "v1.11";
> + break;
> + case SBI_HART_PRIV_VER_1_12:
> + temp = "v1.12";
> + break;
> + default:
> + temp = "unknown";
> + break;
> + }
> +
> + sbi_snprintf(version_str, nvstr, "%s", temp);
> +}
> +
> /**
> * Check whether a particular hart feature is available
> *
> @@ -583,6 +617,7 @@ __mhpm_skip:
> /* Detect if hart supports SCOUNTEREN feature */
> val = csr_read_allowed(CSR_SCOUNTEREN, (unsigned long)&trap);
> if (!trap.cause) {
> + hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> csr_write_allowed(CSR_SCOUNTEREN, (unsigned long)&trap, val);
> if (!trap.cause)
> hfeatures->features |= SBI_HART_HAS_SCOUNTEREN;
> @@ -591,6 +626,7 @@ __mhpm_skip:
> /* Detect if hart supports MCOUNTEREN feature */
> val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
> if (!trap.cause) {
> + hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> csr_write_allowed(CSR_MCOUNTEREN, (unsigned long)&trap, val);
> if (!trap.cause)
> hfeatures->features |= SBI_HART_HAS_MCOUNTEREN;
> @@ -600,8 +636,17 @@ __mhpm_skip:
> val = csr_read_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap);
> if (!trap.cause) {
> csr_write_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap, val);
> - if (!trap.cause)
> + if (!trap.cause) {
> + hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
> hfeatures->features |= SBI_HART_HAS_MCOUNTINHIBIT;
> + }
> + }
> +
> + /* Detect if hart has menvcfg CSR */
> + csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
> + if (!trap.cause) {
> + hfeatures->priv_version = SBI_HART_PRIV_VER_1_12;
> + hfeatures->features |= SBI_HART_HAS_MENVCFG;
> }
>
> /* Counter overflow/filtering is not useful without mcounter/inhibit */
> @@ -625,11 +670,6 @@ __mhpm_skip:
> hfeatures->features |= SBI_HART_HAS_AIA;
> __aia_skip:
>
> - /* Detect if hart has menvcfg CSR */
> - csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
> - if (!trap.cause)
> - hfeatures->features |= SBI_HART_HAS_MENVCFG;
> -
> /**
> * Detect if hart supports stimecmp CSR(Sstc extension) and menvcfg is
> * implemented.
> diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> index 2b9e5ae..660b11f 100644
> --- a/lib/sbi/sbi_init.c
> +++ b/lib/sbi/sbi_init.c
> @@ -139,6 +139,8 @@ static void sbi_boot_print_hart(struct sbi_scratch *scratch, u32 hartid)
> /* Boot HART details */
> sbi_printf("Boot HART ID : %u\n", hartid);
> sbi_printf("Boot HART Domain : %s\n", dom->name);
> + sbi_hart_get_priv_version_str(scratch, str, sizeof(str));
> + sbi_printf("Boot HART Priv Version : %s\n", str);
> misa_string(xlen, str, sizeof(str));
> sbi_printf("Boot HART ISA : %s\n", str);
> sbi_hart_get_features_str(scratch, str, sizeof(str));
> --
> 2.34.1
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 01/11] lib: sbi: Detect and print privileged spec version
2022-05-04 1:42 ` Atish Patra
@ 2022-05-07 4:57 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 4:57 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:12 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > It is possible to guess privileged spec versions based on the CSRs
> > that where introduced in different privleged spec versions. In future,
>
> /s/privleged/privileged
>
> > if we are not able guess privileged spec version then we can have
> > platform provide it.
> >
> > We add privileged spec version as per-hart feature and try to guess
> > it based on presense of mcounteren, mcountinhibit, and menvcfg CSRs.
> >
>
> /s/presense/presence
>
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_hart.h | 15 ++++++++++++
> > lib/sbi/sbi_hart.c | 52 +++++++++++++++++++++++++++++++++++++-----
> > lib/sbi/sbi_init.c | 2 ++
> > 3 files changed, 63 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> > index 1d09374..3c37933 100644
> > --- a/include/sbi/sbi_hart.h
> > +++ b/include/sbi/sbi_hart.h
> > @@ -12,6 +12,18 @@
> >
> > #include <sbi/sbi_types.h>
> >
> > +/** Possible privileged specification versions of a hart */
> > +enum sbi_hart_priv_versions {
> > + /** Unknown privileged specification */
> > + SBI_HART_PRIV_VER_UNKNOWN = 0,
> > + /** Privileged specification v1.10 */
> > + SBI_HART_PRIV_VER_1_10 = 1,
> > + /** Privileged specification v1.11 */
> > + SBI_HART_PRIV_VER_1_11 = 2,
> > + /** Privileged specification v1.12 */
> > + SBI_HART_PRIV_VER_1_12 = 3,
> > +};
> > +
> > /** Possible feature flags of a hart */
> > enum sbi_hart_features {
> > /** Hart has S-mode counter enable */
> > @@ -56,6 +68,9 @@ unsigned long sbi_hart_pmp_granularity(struct sbi_scratch *scratch);
> > unsigned int sbi_hart_pmp_addrbits(struct sbi_scratch *scratch);
> > unsigned int sbi_hart_mhpm_bits(struct sbi_scratch *scratch);
> > int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
> > +int sbi_hart_priv_version(struct sbi_scratch *scratch);
> > +void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> > + char *version_str, int nvstr);
> > bool sbi_hart_has_feature(struct sbi_scratch *scratch, unsigned long feature);
> > void sbi_hart_get_features_str(struct sbi_scratch *scratch,
> > char *features_str, int nfstr);
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 229c14a..ed4e631 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -28,6 +28,7 @@ extern void __sbi_expected_trap_hext(void);
> > void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
> >
> > struct hart_features {
> > + int priv_version;
> > unsigned long features;
> > unsigned int pmp_count;
> > unsigned int pmp_addr_bits;
> > @@ -334,6 +335,39 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
> > return 0;
> > }
> >
> > +int sbi_hart_priv_version(struct sbi_scratch *scratch)
> > +{
> > + struct hart_features *hfeatures =
> > + sbi_scratch_offset_ptr(scratch, hart_features_offset);
> > +
> > + return hfeatures->priv_version;
> > +}
> > +
> > +void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> > + char *version_str, int nvstr)
> > +{
> > + char *temp;
> > + struct hart_features *hfeatures =
> > + sbi_scratch_offset_ptr(scratch, hart_features_offset);
> > +
> > + switch (hfeatures->priv_version) {
> > + case SBI_HART_PRIV_VER_1_10:
> > + temp = "v1.10";
> > + break;
> > + case SBI_HART_PRIV_VER_1_11:
> > + temp = "v1.11";
> > + break;
> > + case SBI_HART_PRIV_VER_1_12:
> > + temp = "v1.12";
> > + break;
> > + default:
> > + temp = "unknown";
> > + break;
> > + }
> > +
> > + sbi_snprintf(version_str, nvstr, "%s", temp);
> > +}
> > +
> > /**
> > * Check whether a particular hart feature is available
> > *
> > @@ -583,6 +617,7 @@ __mhpm_skip:
> > /* Detect if hart supports SCOUNTEREN feature */
> > val = csr_read_allowed(CSR_SCOUNTEREN, (unsigned long)&trap);
> > if (!trap.cause) {
> > + hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> > csr_write_allowed(CSR_SCOUNTEREN, (unsigned long)&trap, val);
> > if (!trap.cause)
> > hfeatures->features |= SBI_HART_HAS_SCOUNTEREN;
> > @@ -591,6 +626,7 @@ __mhpm_skip:
> > /* Detect if hart supports MCOUNTEREN feature */
> > val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
> > if (!trap.cause) {
> > + hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> > csr_write_allowed(CSR_MCOUNTEREN, (unsigned long)&trap, val);
> > if (!trap.cause)
> > hfeatures->features |= SBI_HART_HAS_MCOUNTEREN;
> > @@ -600,8 +636,17 @@ __mhpm_skip:
> > val = csr_read_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap);
> > if (!trap.cause) {
> > csr_write_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap, val);
> > - if (!trap.cause)
> > + if (!trap.cause) {
> > + hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
> > hfeatures->features |= SBI_HART_HAS_MCOUNTINHIBIT;
> > + }
> > + }
> > +
> > + /* Detect if hart has menvcfg CSR */
> > + csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
> > + if (!trap.cause) {
> > + hfeatures->priv_version = SBI_HART_PRIV_VER_1_12;
> > + hfeatures->features |= SBI_HART_HAS_MENVCFG;
> > }
> >
> > /* Counter overflow/filtering is not useful without mcounter/inhibit */
> > @@ -625,11 +670,6 @@ __mhpm_skip:
> > hfeatures->features |= SBI_HART_HAS_AIA;
> > __aia_skip:
> >
> > - /* Detect if hart has menvcfg CSR */
> > - csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
> > - if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_MENVCFG;
> > -
> > /**
> > * Detect if hart supports stimecmp CSR(Sstc extension) and menvcfg is
> > * implemented.
> > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> > index 2b9e5ae..660b11f 100644
> > --- a/lib/sbi/sbi_init.c
> > +++ b/lib/sbi/sbi_init.c
> > @@ -139,6 +139,8 @@ static void sbi_boot_print_hart(struct sbi_scratch *scratch, u32 hartid)
> > /* Boot HART details */
> > sbi_printf("Boot HART ID : %u\n", hartid);
> > sbi_printf("Boot HART Domain : %s\n", dom->name);
> > + sbi_hart_get_priv_version_str(scratch, str, sizeof(str));
> > + sbi_printf("Boot HART Priv Version : %s\n", str);
> > misa_string(xlen, str, sizeof(str));
> > sbi_printf("Boot HART ISA : %s\n", str);
> > sbi_hart_get_features_str(scratch, str, sizeof(str));
> > --
> > 2.34.1
> >
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
I have taken care of the typos at the time of merging this patch.
Applied this patch to the riscv/opensbi repo
Regards,
Anup
>
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 02/11] lib: sbi: Remove 's' and 'u' from misa_string() output
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
2022-04-29 15:51 ` [PATCH 01/11] lib: sbi: Detect and print privileged spec version Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 1:42 ` Atish Patra
2022-04-29 15:51 ` [PATCH 03/11] lib: sbi: Update the name of ISA string printed at boot time Anup Patel
` (9 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
Both 's' and 'u' are not treated as ISA extensions since these are
privilege modes so let's remove it from misa_string() output.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
lib/sbi/riscv_asm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/sbi/riscv_asm.c b/lib/sbi/riscv_asm.c
index 5eab1ed..0ebf41d 100644
--- a/lib/sbi/riscv_asm.c
+++ b/lib/sbi/riscv_asm.c
@@ -53,7 +53,7 @@ int misa_xlen(void)
void misa_string(int xlen, char *out, unsigned int out_sz)
{
unsigned int i, pos = 0;
- const char valid_isa_order[] = "iemafdqclbjtpvnsuhkorwxyzg";
+ const char valid_isa_order[] = "iemafdqclbjtpvnhkorwxyzg";
if (!out)
return;
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 02/11] lib: sbi: Remove 's' and 'u' from misa_string() output
2022-04-29 15:51 ` [PATCH 02/11] lib: sbi: Remove 's' and 'u' from misa_string() output Anup Patel
@ 2022-05-04 1:42 ` Atish Patra
2022-05-07 4:58 ` Anup Patel
0 siblings, 1 reply; 37+ messages in thread
From: Atish Patra @ 2022-05-04 1:42 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Both 's' and 'u' are not treated as ISA extensions since these are
> privilege modes so let's remove it from misa_string() output.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> lib/sbi/riscv_asm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/sbi/riscv_asm.c b/lib/sbi/riscv_asm.c
> index 5eab1ed..0ebf41d 100644
> --- a/lib/sbi/riscv_asm.c
> +++ b/lib/sbi/riscv_asm.c
> @@ -53,7 +53,7 @@ int misa_xlen(void)
> void misa_string(int xlen, char *out, unsigned int out_sz)
> {
> unsigned int i, pos = 0;
> - const char valid_isa_order[] = "iemafdqclbjtpvnsuhkorwxyzg";
> + const char valid_isa_order[] = "iemafdqclbjtpvnhkorwxyzg";
>
> if (!out)
> return;
> --
> 2.34.1
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 02/11] lib: sbi: Remove 's' and 'u' from misa_string() output
2022-05-04 1:42 ` Atish Patra
@ 2022-05-07 4:58 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 4:58 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:12 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > Both 's' and 'u' are not treated as ISA extensions since these are
> > privilege modes so let's remove it from misa_string() output.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > lib/sbi/riscv_asm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/sbi/riscv_asm.c b/lib/sbi/riscv_asm.c
> > index 5eab1ed..0ebf41d 100644
> > --- a/lib/sbi/riscv_asm.c
> > +++ b/lib/sbi/riscv_asm.c
> > @@ -53,7 +53,7 @@ int misa_xlen(void)
> > void misa_string(int xlen, char *out, unsigned int out_sz)
> > {
> > unsigned int i, pos = 0;
> > - const char valid_isa_order[] = "iemafdqclbjtpvnsuhkorwxyzg";
> > + const char valid_isa_order[] = "iemafdqclbjtpvnhkorwxyzg";
> >
> > if (!out)
> > return;
> > --
> > 2.34.1
> >
>
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
>
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 03/11] lib: sbi: Update the name of ISA string printed at boot time
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
2022-04-29 15:51 ` [PATCH 01/11] lib: sbi: Detect and print privileged spec version Anup Patel
2022-04-29 15:51 ` [PATCH 02/11] lib: sbi: Remove 's' and 'u' from misa_string() output Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 1:55 ` Atish Patra
2022-04-29 15:51 ` [PATCH 04/11] lib: sbi: Remove MCOUNTEREN and SCOUNTEREN hart features Anup Patel
` (8 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
The ISA string printed at boot time is not the complete ISA string
representing all single letter and multi-letter extensions rather
it is base ISA string derived from misa CSR so let us update the
boot print accordingly.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
lib/sbi/sbi_init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
index 660b11f..c4def58 100644
--- a/lib/sbi/sbi_init.c
+++ b/lib/sbi/sbi_init.c
@@ -142,7 +142,7 @@ static void sbi_boot_print_hart(struct sbi_scratch *scratch, u32 hartid)
sbi_hart_get_priv_version_str(scratch, str, sizeof(str));
sbi_printf("Boot HART Priv Version : %s\n", str);
misa_string(xlen, str, sizeof(str));
- sbi_printf("Boot HART ISA : %s\n", str);
+ sbi_printf("Boot HART Base ISA : %s\n", str);
sbi_hart_get_features_str(scratch, str, sizeof(str));
sbi_printf("Boot HART Features : %s\n", str);
sbi_printf("Boot HART PMP Count : %d\n",
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 03/11] lib: sbi: Update the name of ISA string printed at boot time
2022-04-29 15:51 ` [PATCH 03/11] lib: sbi: Update the name of ISA string printed at boot time Anup Patel
@ 2022-05-04 1:55 ` Atish Patra
2022-05-07 4:59 ` Anup Patel
0 siblings, 1 reply; 37+ messages in thread
From: Atish Patra @ 2022-05-04 1:55 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The ISA string printed at boot time is not the complete ISA string
> representing all single letter and multi-letter extensions rather
> it is base ISA string derived from misa CSR so let us update the
> boot print accordingly.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> lib/sbi/sbi_init.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> index 660b11f..c4def58 100644
> --- a/lib/sbi/sbi_init.c
> +++ b/lib/sbi/sbi_init.c
> @@ -142,7 +142,7 @@ static void sbi_boot_print_hart(struct sbi_scratch *scratch, u32 hartid)
> sbi_hart_get_priv_version_str(scratch, str, sizeof(str));
> sbi_printf("Boot HART Priv Version : %s\n", str);
> misa_string(xlen, str, sizeof(str));
> - sbi_printf("Boot HART ISA : %s\n", str);
> + sbi_printf("Boot HART Base ISA : %s\n", str);
> sbi_hart_get_features_str(scratch, str, sizeof(str));
> sbi_printf("Boot HART Features : %s\n", str);
> sbi_printf("Boot HART PMP Count : %d\n",
> --
> 2.34.1
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 03/11] lib: sbi: Update the name of ISA string printed at boot time
2022-05-04 1:55 ` Atish Patra
@ 2022-05-07 4:59 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 4:59 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:26 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > The ISA string printed at boot time is not the complete ISA string
> > representing all single letter and multi-letter extensions rather
> > it is base ISA string derived from misa CSR so let us update the
> > boot print accordingly.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > lib/sbi/sbi_init.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> > index 660b11f..c4def58 100644
> > --- a/lib/sbi/sbi_init.c
> > +++ b/lib/sbi/sbi_init.c
> > @@ -142,7 +142,7 @@ static void sbi_boot_print_hart(struct sbi_scratch *scratch, u32 hartid)
> > sbi_hart_get_priv_version_str(scratch, str, sizeof(str));
> > sbi_printf("Boot HART Priv Version : %s\n", str);
> > misa_string(xlen, str, sizeof(str));
> > - sbi_printf("Boot HART ISA : %s\n", str);
> > + sbi_printf("Boot HART Base ISA : %s\n", str);
> > sbi_hart_get_features_str(scratch, str, sizeof(str));
> > sbi_printf("Boot HART Features : %s\n", str);
> > sbi_printf("Boot HART PMP Count : %d\n",
> > --
> > 2.34.1
> >
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
>
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 04/11] lib: sbi: Remove MCOUNTEREN and SCOUNTEREN hart features
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
` (2 preceding siblings ...)
2022-04-29 15:51 ` [PATCH 03/11] lib: sbi: Update the name of ISA string printed at boot time Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 1:51 ` Atish Patra
2022-04-29 15:51 ` [PATCH 05/11] lib: sbi: Remove MCOUNTINHIBT hart feature Anup Patel
` (7 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
If a hart implements privileged spec v1.10 (or higher) then we can
safely assume that [m|s]counteren CSR are present and we don't need
MCOUNTEREN and SCOUNTEREN as hart features.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_hart.h | 18 +++++++-----------
lib/sbi/sbi_emulate_csr.c | 4 ++--
lib/sbi/sbi_hart.c | 29 +++++------------------------
lib/sbi/sbi_pmu.c | 3 ++-
4 files changed, 16 insertions(+), 38 deletions(-)
diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index 3c37933..4665a62 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -26,24 +26,20 @@ enum sbi_hart_priv_versions {
/** Possible feature flags of a hart */
enum sbi_hart_features {
- /** Hart has S-mode counter enable */
- SBI_HART_HAS_SCOUNTEREN = (1 << 0),
- /** Hart has M-mode counter enable */
- SBI_HART_HAS_MCOUNTEREN = (1 << 1),
/** Hart has counter inhibit CSR */
- SBI_HART_HAS_MCOUNTINHIBIT = (1 << 2),
+ SBI_HART_HAS_MCOUNTINHIBIT = (1 << 0),
/** Hart has sscofpmf extension */
- SBI_HART_HAS_SSCOFPMF = (1 << 3),
+ SBI_HART_HAS_SSCOFPMF = (1 << 1),
/** HART has timer csr implementation in hardware */
- SBI_HART_HAS_TIME = (1 << 4),
+ SBI_HART_HAS_TIME = (1 << 2),
/** HART has AIA local interrupt CSRs */
- SBI_HART_HAS_AIA = (1 << 5),
+ SBI_HART_HAS_AIA = (1 << 3),
/** HART has menvcfg CSR */
- SBI_HART_HAS_MENVCFG = (1 << 6),
+ SBI_HART_HAS_MENVCFG = (1 << 4),
/** HART has mstateen CSR **/
- SBI_HART_HAS_SMSTATEEN = (1 << 7),
+ SBI_HART_HAS_SMSTATEEN = (1 << 5),
/** HART has SSTC extension implemented in hardware */
- SBI_HART_HAS_SSTC = (1 << 8),
+ SBI_HART_HAS_SSTC = (1 << 6),
/** Last index of Hart features*/
SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
diff --git a/lib/sbi/sbi_emulate_csr.c b/lib/sbi/sbi_emulate_csr.c
index dbb1755..aec9d3c 100644
--- a/lib/sbi/sbi_emulate_csr.c
+++ b/lib/sbi/sbi_emulate_csr.c
@@ -24,7 +24,7 @@ static bool hpm_allowed(int hpm_num, ulong prev_mode, bool virt)
struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
if (prev_mode <= PRV_S) {
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTEREN)) {
+ if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10) {
cen &= csr_read(CSR_MCOUNTEREN);
if (virt)
cen &= csr_read(CSR_HCOUNTEREN);
@@ -33,7 +33,7 @@ static bool hpm_allowed(int hpm_num, ulong prev_mode, bool virt)
}
}
if (prev_mode == PRV_U) {
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SCOUNTEREN))
+ if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
cen &= csr_read(CSR_SCOUNTEREN);
else
cen = 0;
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index ed4e631..ed8a061 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -58,7 +58,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
/* 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))
+ sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
csr_write(CSR_SCOUNTEREN, 7);
/**
@@ -66,7 +66,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
* Supervisor mode usage for all counters are enabled by default
* But counters will not run until mcountinhibit is set.
*/
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTEREN))
+ if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
csr_write(CSR_MCOUNTEREN, -1);
/* All programmable counters will start running at runtime after S-mode request */
@@ -402,12 +402,6 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
return NULL;
switch (feature) {
- case SBI_HART_HAS_SCOUNTEREN:
- fstr = "scounteren";
- break;
- case SBI_HART_HAS_MCOUNTEREN:
- fstr = "mcounteren";
- break;
case SBI_HART_HAS_MCOUNTINHIBIT:
fstr = "mcountinhibit";
break;
@@ -614,23 +608,10 @@ __mhpm_skip:
#undef __check_csr_2
#undef __check_csr
- /* Detect if hart supports SCOUNTEREN feature */
- val = csr_read_allowed(CSR_SCOUNTEREN, (unsigned long)&trap);
- if (!trap.cause) {
- hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
- csr_write_allowed(CSR_SCOUNTEREN, (unsigned long)&trap, val);
- if (!trap.cause)
- hfeatures->features |= SBI_HART_HAS_SCOUNTEREN;
- }
-
- /* Detect if hart supports MCOUNTEREN feature */
+ /* Detect if hart supports Priv v1.10 */
val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
- if (!trap.cause) {
+ if (!trap.cause)
hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
- csr_write_allowed(CSR_MCOUNTEREN, (unsigned long)&trap, val);
- if (!trap.cause)
- hfeatures->features |= SBI_HART_HAS_MCOUNTEREN;
- }
/* Detect if hart supports MCOUNTINHIBIT feature */
val = csr_read_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap);
@@ -651,7 +632,7 @@ __mhpm_skip:
/* Counter overflow/filtering is not useful without mcounter/inhibit */
if (hfeatures->features & SBI_HART_HAS_MCOUNTINHIBIT &&
- hfeatures->features & SBI_HART_HAS_MCOUNTEREN) {
+ hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
/* Detect if hart supports sscofpmf */
csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
if (!trap.cause)
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 7ea0ca5..853229e 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -729,7 +729,8 @@ void sbi_pmu_exit(struct sbi_scratch *scratch)
if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
- csr_write(CSR_MCOUNTEREN, -1);
+ if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
+ csr_write(CSR_MCOUNTEREN, -1);
pmu_reset_event_map(hartid);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 04/11] lib: sbi: Remove MCOUNTEREN and SCOUNTEREN hart features
2022-04-29 15:51 ` [PATCH 04/11] lib: sbi: Remove MCOUNTEREN and SCOUNTEREN hart features Anup Patel
@ 2022-05-04 1:51 ` Atish Patra
2022-05-04 1:55 ` Atish Patra
2022-05-07 5:00 ` Anup Patel
0 siblings, 2 replies; 37+ messages in thread
From: Atish Patra @ 2022-05-04 1:51 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> If a hart implements privileged spec v1.10 (or higher) then we can
> safely assume that [m|s]counteren CSR are present and we don't need
> MCOUNTEREN and SCOUNTEREN as hart features.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_hart.h | 18 +++++++-----------
> lib/sbi/sbi_emulate_csr.c | 4 ++--
> lib/sbi/sbi_hart.c | 29 +++++------------------------
> lib/sbi/sbi_pmu.c | 3 ++-
> 4 files changed, 16 insertions(+), 38 deletions(-)
>
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index 3c37933..4665a62 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -26,24 +26,20 @@ enum sbi_hart_priv_versions {
>
> /** Possible feature flags of a hart */
> enum sbi_hart_features {
> - /** Hart has S-mode counter enable */
> - SBI_HART_HAS_SCOUNTEREN = (1 << 0),
> - /** Hart has M-mode counter enable */
> - SBI_HART_HAS_MCOUNTEREN = (1 << 1),
> /** Hart has counter inhibit CSR */
> - SBI_HART_HAS_MCOUNTINHIBIT = (1 << 2),
> + SBI_HART_HAS_MCOUNTINHIBIT = (1 << 0),
> /** Hart has sscofpmf extension */
> - SBI_HART_HAS_SSCOFPMF = (1 << 3),
> + SBI_HART_HAS_SSCOFPMF = (1 << 1),
> /** HART has timer csr implementation in hardware */
> - SBI_HART_HAS_TIME = (1 << 4),
> + SBI_HART_HAS_TIME = (1 << 2),
> /** HART has AIA local interrupt CSRs */
> - SBI_HART_HAS_AIA = (1 << 5),
> + SBI_HART_HAS_AIA = (1 << 3),
> /** HART has menvcfg CSR */
> - SBI_HART_HAS_MENVCFG = (1 << 6),
> + SBI_HART_HAS_MENVCFG = (1 << 4),
> /** HART has mstateen CSR **/
> - SBI_HART_HAS_SMSTATEEN = (1 << 7),
> + SBI_HART_HAS_SMSTATEEN = (1 << 5),
> /** HART has SSTC extension implemented in hardware */
> - SBI_HART_HAS_SSTC = (1 << 8),
> + SBI_HART_HAS_SSTC = (1 << 6),
>
> /** Last index of Hart features*/
> SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
> diff --git a/lib/sbi/sbi_emulate_csr.c b/lib/sbi/sbi_emulate_csr.c
> index dbb1755..aec9d3c 100644
> --- a/lib/sbi/sbi_emulate_csr.c
> +++ b/lib/sbi/sbi_emulate_csr.c
> @@ -24,7 +24,7 @@ static bool hpm_allowed(int hpm_num, ulong prev_mode, bool virt)
> struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>
> if (prev_mode <= PRV_S) {
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTEREN)) {
> + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10) {
> cen &= csr_read(CSR_MCOUNTEREN);
> if (virt)
> cen &= csr_read(CSR_HCOUNTEREN);
> @@ -33,7 +33,7 @@ static bool hpm_allowed(int hpm_num, ulong prev_mode, bool virt)
> }
> }
> if (prev_mode == PRV_U) {
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SCOUNTEREN))
> + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> cen &= csr_read(CSR_SCOUNTEREN);
> else
> cen = 0;
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index ed4e631..ed8a061 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -58,7 +58,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
>
> /* 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))
> + sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> csr_write(CSR_SCOUNTEREN, 7);
>
mstatus_init invokes sbi_hart_priv_version 4 times. Keep the priv
version in a local variable ?
> /**
> @@ -66,7 +66,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> * Supervisor mode usage for all counters are enabled by default
> * But counters will not run until mcountinhibit is set.
> */
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTEREN))
> + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> csr_write(CSR_MCOUNTEREN, -1);
>
> /* All programmable counters will start running at runtime after S-mode request */
> @@ -402,12 +402,6 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
> return NULL;
>
> switch (feature) {
> - case SBI_HART_HAS_SCOUNTEREN:
> - fstr = "scounteren";
> - break;
> - case SBI_HART_HAS_MCOUNTEREN:
> - fstr = "mcounteren";
> - break;
> case SBI_HART_HAS_MCOUNTINHIBIT:
> fstr = "mcountinhibit";
> break;
> @@ -614,23 +608,10 @@ __mhpm_skip:
> #undef __check_csr_2
> #undef __check_csr
>
> - /* Detect if hart supports SCOUNTEREN feature */
> - val = csr_read_allowed(CSR_SCOUNTEREN, (unsigned long)&trap);
> - if (!trap.cause) {
> - hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> - csr_write_allowed(CSR_SCOUNTEREN, (unsigned long)&trap, val);
> - if (!trap.cause)
> - hfeatures->features |= SBI_HART_HAS_SCOUNTEREN;
> - }
> -
> - /* Detect if hart supports MCOUNTEREN feature */
> + /* Detect if hart supports Priv v1.10 */
> val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
> - if (!trap.cause) {
> + if (!trap.cause)
> hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> - csr_write_allowed(CSR_MCOUNTEREN, (unsigned long)&trap, val);
> - if (!trap.cause)
> - hfeatures->features |= SBI_HART_HAS_MCOUNTEREN;
> - }
>
> /* Detect if hart supports MCOUNTINHIBIT feature */
> val = csr_read_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap);
> @@ -651,7 +632,7 @@ __mhpm_skip:
>
> /* Counter overflow/filtering is not useful without mcounter/inhibit */
> if (hfeatures->features & SBI_HART_HAS_MCOUNTINHIBIT &&
> - hfeatures->features & SBI_HART_HAS_MCOUNTEREN) {
> + hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
nit comment:
this should be SBI_HART_PRIV_VER_1_10 in this patch.
the check should be SBI_HART_PRIV_VER_1_12 when you remove
mcountinhibit (PATCH 5)
> /* Detect if hart supports sscofpmf */
> csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
> if (!trap.cause)
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 7ea0ca5..853229e 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -729,7 +729,8 @@ void sbi_pmu_exit(struct sbi_scratch *scratch)
> if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
>
> - csr_write(CSR_MCOUNTEREN, -1);
> + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> + csr_write(CSR_MCOUNTEREN, -1);
> pmu_reset_event_map(hartid);
> }
>
> --
> 2.34.1
>
Other than that, LGTM.
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 04/11] lib: sbi: Remove MCOUNTEREN and SCOUNTEREN hart features
2022-05-04 1:51 ` Atish Patra
@ 2022-05-04 1:55 ` Atish Patra
2022-05-07 5:00 ` Anup Patel
1 sibling, 0 replies; 37+ messages in thread
From: Atish Patra @ 2022-05-04 1:55 UTC (permalink / raw)
To: opensbi
On Tue, May 3, 2022 at 6:51 PM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > If a hart implements privileged spec v1.10 (or higher) then we can
> > safely assume that [m|s]counteren CSR are present and we don't need
> > MCOUNTEREN and SCOUNTEREN as hart features.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_hart.h | 18 +++++++-----------
> > lib/sbi/sbi_emulate_csr.c | 4 ++--
> > lib/sbi/sbi_hart.c | 29 +++++------------------------
> > lib/sbi/sbi_pmu.c | 3 ++-
> > 4 files changed, 16 insertions(+), 38 deletions(-)
> >
> > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> > index 3c37933..4665a62 100644
> > --- a/include/sbi/sbi_hart.h
> > +++ b/include/sbi/sbi_hart.h
> > @@ -26,24 +26,20 @@ enum sbi_hart_priv_versions {
> >
> > /** Possible feature flags of a hart */
> > enum sbi_hart_features {
> > - /** Hart has S-mode counter enable */
> > - SBI_HART_HAS_SCOUNTEREN = (1 << 0),
> > - /** Hart has M-mode counter enable */
> > - SBI_HART_HAS_MCOUNTEREN = (1 << 1),
> > /** Hart has counter inhibit CSR */
> > - SBI_HART_HAS_MCOUNTINHIBIT = (1 << 2),
> > + SBI_HART_HAS_MCOUNTINHIBIT = (1 << 0),
> > /** Hart has sscofpmf extension */
> > - SBI_HART_HAS_SSCOFPMF = (1 << 3),
> > + SBI_HART_HAS_SSCOFPMF = (1 << 1),
> > /** HART has timer csr implementation in hardware */
> > - SBI_HART_HAS_TIME = (1 << 4),
> > + SBI_HART_HAS_TIME = (1 << 2),
> > /** HART has AIA local interrupt CSRs */
> > - SBI_HART_HAS_AIA = (1 << 5),
> > + SBI_HART_HAS_AIA = (1 << 3),
> > /** HART has menvcfg CSR */
> > - SBI_HART_HAS_MENVCFG = (1 << 6),
> > + SBI_HART_HAS_MENVCFG = (1 << 4),
> > /** HART has mstateen CSR **/
> > - SBI_HART_HAS_SMSTATEEN = (1 << 7),
> > + SBI_HART_HAS_SMSTATEEN = (1 << 5),
> > /** HART has SSTC extension implemented in hardware */
> > - SBI_HART_HAS_SSTC = (1 << 8),
> > + SBI_HART_HAS_SSTC = (1 << 6),
> >
> > /** Last index of Hart features*/
> > SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
> > diff --git a/lib/sbi/sbi_emulate_csr.c b/lib/sbi/sbi_emulate_csr.c
> > index dbb1755..aec9d3c 100644
> > --- a/lib/sbi/sbi_emulate_csr.c
> > +++ b/lib/sbi/sbi_emulate_csr.c
> > @@ -24,7 +24,7 @@ static bool hpm_allowed(int hpm_num, ulong prev_mode, bool virt)
> > struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > if (prev_mode <= PRV_S) {
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTEREN)) {
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10) {
> > cen &= csr_read(CSR_MCOUNTEREN);
> > if (virt)
> > cen &= csr_read(CSR_HCOUNTEREN);
> > @@ -33,7 +33,7 @@ static bool hpm_allowed(int hpm_num, ulong prev_mode, bool virt)
> > }
> > }
> > if (prev_mode == PRV_U) {
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SCOUNTEREN))
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> > cen &= csr_read(CSR_SCOUNTEREN);
> > else
> > cen = 0;
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index ed4e631..ed8a061 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -58,7 +58,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> >
> > /* 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))
> > + sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> > csr_write(CSR_SCOUNTEREN, 7);
> >
>
> mstatus_init invokes sbi_hart_priv_version 4 times. Keep the priv
> version in a local variable ?
>
> > /**
> > @@ -66,7 +66,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> > * Supervisor mode usage for all counters are enabled by default
> > * But counters will not run until mcountinhibit is set.
> > */
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTEREN))
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> > csr_write(CSR_MCOUNTEREN, -1);
> >
> > /* All programmable counters will start running at runtime after S-mode request */
> > @@ -402,12 +402,6 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
> > return NULL;
> >
> > switch (feature) {
> > - case SBI_HART_HAS_SCOUNTEREN:
> > - fstr = "scounteren";
> > - break;
> > - case SBI_HART_HAS_MCOUNTEREN:
> > - fstr = "mcounteren";
> > - break;
> > case SBI_HART_HAS_MCOUNTINHIBIT:
> > fstr = "mcountinhibit";
> > break;
> > @@ -614,23 +608,10 @@ __mhpm_skip:
> > #undef __check_csr_2
> > #undef __check_csr
> >
> > - /* Detect if hart supports SCOUNTEREN feature */
> > - val = csr_read_allowed(CSR_SCOUNTEREN, (unsigned long)&trap);
> > - if (!trap.cause) {
> > - hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> > - csr_write_allowed(CSR_SCOUNTEREN, (unsigned long)&trap, val);
> > - if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_SCOUNTEREN;
> > - }
> > -
> > - /* Detect if hart supports MCOUNTEREN feature */
> > + /* Detect if hart supports Priv v1.10 */
> > val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
> > - if (!trap.cause) {
> > + if (!trap.cause)
> > hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> > - csr_write_allowed(CSR_MCOUNTEREN, (unsigned long)&trap, val);
> > - if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_MCOUNTEREN;
> > - }
> >
> > /* Detect if hart supports MCOUNTINHIBIT feature */
> > val = csr_read_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap);
> > @@ -651,7 +632,7 @@ __mhpm_skip:
> >
> > /* Counter overflow/filtering is not useful without mcounter/inhibit */
> > if (hfeatures->features & SBI_HART_HAS_MCOUNTINHIBIT &&
> > - hfeatures->features & SBI_HART_HAS_MCOUNTEREN) {
> > + hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
>
> nit comment:
> this should be SBI_HART_PRIV_VER_1_10 in this patch.
> the check should be SBI_HART_PRIV_VER_1_12 when you remove
> mcountinhibit (PATCH 5)
Ignore the above comment. It is doing the correct thing. I misread the
following line. My bad.
>
> > /* Detect if hart supports sscofpmf */
> > csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
> > if (!trap.cause)
> > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> > index 7ea0ca5..853229e 100644
> > --- a/lib/sbi/sbi_pmu.c
> > +++ b/lib/sbi/sbi_pmu.c
> > @@ -729,7 +729,8 @@ void sbi_pmu_exit(struct sbi_scratch *scratch)
> > if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> > csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
> >
> > - csr_write(CSR_MCOUNTEREN, -1);
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> > + csr_write(CSR_MCOUNTEREN, -1);
> > pmu_reset_event_map(hartid);
> > }
> >
> > --
> > 2.34.1
> >
>
> Other than that, LGTM.
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
> --
> Regards,
> Atish
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 04/11] lib: sbi: Remove MCOUNTEREN and SCOUNTEREN hart features
2022-05-04 1:51 ` Atish Patra
2022-05-04 1:55 ` Atish Patra
@ 2022-05-07 5:00 ` Anup Patel
1 sibling, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 5:00 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:21 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > If a hart implements privileged spec v1.10 (or higher) then we can
> > safely assume that [m|s]counteren CSR are present and we don't need
> > MCOUNTEREN and SCOUNTEREN as hart features.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_hart.h | 18 +++++++-----------
> > lib/sbi/sbi_emulate_csr.c | 4 ++--
> > lib/sbi/sbi_hart.c | 29 +++++------------------------
> > lib/sbi/sbi_pmu.c | 3 ++-
> > 4 files changed, 16 insertions(+), 38 deletions(-)
> >
> > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> > index 3c37933..4665a62 100644
> > --- a/include/sbi/sbi_hart.h
> > +++ b/include/sbi/sbi_hart.h
> > @@ -26,24 +26,20 @@ enum sbi_hart_priv_versions {
> >
> > /** Possible feature flags of a hart */
> > enum sbi_hart_features {
> > - /** Hart has S-mode counter enable */
> > - SBI_HART_HAS_SCOUNTEREN = (1 << 0),
> > - /** Hart has M-mode counter enable */
> > - SBI_HART_HAS_MCOUNTEREN = (1 << 1),
> > /** Hart has counter inhibit CSR */
> > - SBI_HART_HAS_MCOUNTINHIBIT = (1 << 2),
> > + SBI_HART_HAS_MCOUNTINHIBIT = (1 << 0),
> > /** Hart has sscofpmf extension */
> > - SBI_HART_HAS_SSCOFPMF = (1 << 3),
> > + SBI_HART_HAS_SSCOFPMF = (1 << 1),
> > /** HART has timer csr implementation in hardware */
> > - SBI_HART_HAS_TIME = (1 << 4),
> > + SBI_HART_HAS_TIME = (1 << 2),
> > /** HART has AIA local interrupt CSRs */
> > - SBI_HART_HAS_AIA = (1 << 5),
> > + SBI_HART_HAS_AIA = (1 << 3),
> > /** HART has menvcfg CSR */
> > - SBI_HART_HAS_MENVCFG = (1 << 6),
> > + SBI_HART_HAS_MENVCFG = (1 << 4),
> > /** HART has mstateen CSR **/
> > - SBI_HART_HAS_SMSTATEEN = (1 << 7),
> > + SBI_HART_HAS_SMSTATEEN = (1 << 5),
> > /** HART has SSTC extension implemented in hardware */
> > - SBI_HART_HAS_SSTC = (1 << 8),
> > + SBI_HART_HAS_SSTC = (1 << 6),
> >
> > /** Last index of Hart features*/
> > SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
> > diff --git a/lib/sbi/sbi_emulate_csr.c b/lib/sbi/sbi_emulate_csr.c
> > index dbb1755..aec9d3c 100644
> > --- a/lib/sbi/sbi_emulate_csr.c
> > +++ b/lib/sbi/sbi_emulate_csr.c
> > @@ -24,7 +24,7 @@ static bool hpm_allowed(int hpm_num, ulong prev_mode, bool virt)
> > struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >
> > if (prev_mode <= PRV_S) {
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTEREN)) {
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10) {
> > cen &= csr_read(CSR_MCOUNTEREN);
> > if (virt)
> > cen &= csr_read(CSR_HCOUNTEREN);
> > @@ -33,7 +33,7 @@ static bool hpm_allowed(int hpm_num, ulong prev_mode, bool virt)
> > }
> > }
> > if (prev_mode == PRV_U) {
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SCOUNTEREN))
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> > cen &= csr_read(CSR_SCOUNTEREN);
> > else
> > cen = 0;
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index ed4e631..ed8a061 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -58,7 +58,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> >
> > /* 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))
> > + sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> > csr_write(CSR_SCOUNTEREN, 7);
> >
>
> mstatus_init invokes sbi_hart_priv_version 4 times. Keep the priv
> version in a local variable ?
>
> > /**
> > @@ -66,7 +66,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> > * Supervisor mode usage for all counters are enabled by default
> > * But counters will not run until mcountinhibit is set.
> > */
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTEREN))
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> > csr_write(CSR_MCOUNTEREN, -1);
> >
> > /* All programmable counters will start running at runtime after S-mode request */
> > @@ -402,12 +402,6 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
> > return NULL;
> >
> > switch (feature) {
> > - case SBI_HART_HAS_SCOUNTEREN:
> > - fstr = "scounteren";
> > - break;
> > - case SBI_HART_HAS_MCOUNTEREN:
> > - fstr = "mcounteren";
> > - break;
> > case SBI_HART_HAS_MCOUNTINHIBIT:
> > fstr = "mcountinhibit";
> > break;
> > @@ -614,23 +608,10 @@ __mhpm_skip:
> > #undef __check_csr_2
> > #undef __check_csr
> >
> > - /* Detect if hart supports SCOUNTEREN feature */
> > - val = csr_read_allowed(CSR_SCOUNTEREN, (unsigned long)&trap);
> > - if (!trap.cause) {
> > - hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> > - csr_write_allowed(CSR_SCOUNTEREN, (unsigned long)&trap, val);
> > - if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_SCOUNTEREN;
> > - }
> > -
> > - /* Detect if hart supports MCOUNTEREN feature */
> > + /* Detect if hart supports Priv v1.10 */
> > val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
> > - if (!trap.cause) {
> > + if (!trap.cause)
> > hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> > - csr_write_allowed(CSR_MCOUNTEREN, (unsigned long)&trap, val);
> > - if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_MCOUNTEREN;
> > - }
> >
> > /* Detect if hart supports MCOUNTINHIBIT feature */
> > val = csr_read_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap);
> > @@ -651,7 +632,7 @@ __mhpm_skip:
> >
> > /* Counter overflow/filtering is not useful without mcounter/inhibit */
> > if (hfeatures->features & SBI_HART_HAS_MCOUNTINHIBIT &&
> > - hfeatures->features & SBI_HART_HAS_MCOUNTEREN) {
> > + hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
>
> nit comment:
> this should be SBI_HART_PRIV_VER_1_10 in this patch.
> the check should be SBI_HART_PRIV_VER_1_12 when you remove
> mcountinhibit (PATCH 5)
>
> > /* Detect if hart supports sscofpmf */
> > csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
> > if (!trap.cause)
> > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> > index 7ea0ca5..853229e 100644
> > --- a/lib/sbi/sbi_pmu.c
> > +++ b/lib/sbi/sbi_pmu.c
> > @@ -729,7 +729,8 @@ void sbi_pmu_exit(struct sbi_scratch *scratch)
> > if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> > csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
> >
> > - csr_write(CSR_MCOUNTEREN, -1);
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> > + csr_write(CSR_MCOUNTEREN, -1);
> > pmu_reset_event_map(hartid);
> > }
> >
> > --
> > 2.34.1
> >
>
> Other than that, LGTM.
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 05/11] lib: sbi: Remove MCOUNTINHIBT hart feature
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
` (3 preceding siblings ...)
2022-04-29 15:51 ` [PATCH 04/11] lib: sbi: Remove MCOUNTEREN and SCOUNTEREN hart features Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 1:56 ` Atish Patra
2022-04-29 15:51 ` [PATCH 06/11] lib: sbi: Remove MENVCFG " Anup Patel
` (6 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
If a hart implements privileged spec v1.11 (or higher) then we can
safely assume that mcountinhibit CSR is present and we don't need
MCOUNTINHIBT as a hart feature.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_hart.h | 14 ++++++--------
lib/sbi/sbi_hart.c | 23 ++++++++---------------
lib/sbi/sbi_pmu.c | 10 +++++-----
3 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index 4665a62..bc6b766 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -26,20 +26,18 @@ enum sbi_hart_priv_versions {
/** Possible feature flags of a hart */
enum sbi_hart_features {
- /** Hart has counter inhibit CSR */
- SBI_HART_HAS_MCOUNTINHIBIT = (1 << 0),
/** Hart has sscofpmf extension */
- SBI_HART_HAS_SSCOFPMF = (1 << 1),
+ SBI_HART_HAS_SSCOFPMF = (1 << 0),
/** HART has timer csr implementation in hardware */
- SBI_HART_HAS_TIME = (1 << 2),
+ SBI_HART_HAS_TIME = (1 << 1),
/** HART has AIA local interrupt CSRs */
- SBI_HART_HAS_AIA = (1 << 3),
+ SBI_HART_HAS_AIA = (1 << 2),
/** HART has menvcfg CSR */
- SBI_HART_HAS_MENVCFG = (1 << 4),
+ SBI_HART_HAS_MENVCFG = (1 << 3),
/** HART has mstateen CSR **/
- SBI_HART_HAS_SMSTATEEN = (1 << 5),
+ SBI_HART_HAS_SMSTATEEN = (1 << 4),
/** HART has SSTC extension implemented in hardware */
- SBI_HART_HAS_SSTC = (1 << 6),
+ SBI_HART_HAS_SSTC = (1 << 5),
/** Last index of Hart features*/
SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index ed8a061..0fe88cb 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -70,7 +70,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
csr_write(CSR_MCOUNTEREN, -1);
/* All programmable counters will start running@runtime after S-mode request */
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
+ if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
/**
@@ -402,9 +402,6 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
return NULL;
switch (feature) {
- case SBI_HART_HAS_MCOUNTINHIBIT:
- fstr = "mcountinhibit";
- break;
case SBI_HART_HAS_SSCOFPMF:
fstr = "sscofpmf";
break;
@@ -613,26 +610,22 @@ __mhpm_skip:
if (!trap.cause)
hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
- /* Detect if hart supports MCOUNTINHIBIT feature */
+ /* Detect if hart supports Priv v1.11 */
val = csr_read_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap);
- if (!trap.cause) {
- csr_write_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap, val);
- if (!trap.cause) {
- hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
- hfeatures->features |= SBI_HART_HAS_MCOUNTINHIBIT;
- }
- }
+ if (!trap.cause &&
+ (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_10))
+ hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
/* Detect if hart has menvcfg CSR */
csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
- if (!trap.cause) {
+ if (!trap.cause &&
+ (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_11)) {
hfeatures->priv_version = SBI_HART_PRIV_VER_1_12;
hfeatures->features |= SBI_HART_HAS_MENVCFG;
}
/* Counter overflow/filtering is not useful without mcounter/inhibit */
- if (hfeatures->features & SBI_HART_HAS_MCOUNTINHIBIT &&
- hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
+ if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
/* Detect if hart supports sscofpmf */
csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
if (!trap.cause)
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 853229e..386bf4d 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -300,7 +300,7 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
if (cidx > num_hw_ctrs || cidx == 1)
return SBI_EINVAL;
- if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
+ if (sbi_hart_priv_version(scratch) < SBI_HART_PRIV_VER_1_11)
goto skip_inhibit_update;
/*
@@ -372,7 +372,7 @@ static int pmu_ctr_stop_hw(uint32_t cidx)
struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
unsigned long mctr_inhbt;
- if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
+ if (sbi_hart_priv_version(scratch) < SBI_HART_PRIV_VER_1_11)
return 0;
mctr_inhbt = csr_read(CSR_MCOUNTINHIBIT);
@@ -524,7 +524,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
!sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
return fixed_ctr;
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
+ if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
mctr_inhbt = csr_read(CSR_MCOUNTINHIBIT);
for (i = 0; i < num_hw_events; i++) {
temp = &hw_event_map[i];
@@ -551,7 +551,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
if (active_events[hartid][cbase] != SBI_PMU_EVENT_IDX_INVALID)
continue;
/* If mcountinhibit is supported, the bit must be enabled */
- if ((sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT)) &&
+ if ((sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11) &&
!__test_bit(cbase, &mctr_inhbt))
continue;
/* We found a valid counter that is not started yet */
@@ -726,7 +726,7 @@ void sbi_pmu_exit(struct sbi_scratch *scratch)
{
u32 hartid = current_hartid();
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
+ if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 05/11] lib: sbi: Remove MCOUNTINHIBT hart feature
2022-04-29 15:51 ` [PATCH 05/11] lib: sbi: Remove MCOUNTINHIBT hart feature Anup Patel
@ 2022-05-04 1:56 ` Atish Patra
2022-05-07 5:00 ` Anup Patel
0 siblings, 1 reply; 37+ messages in thread
From: Atish Patra @ 2022-05-04 1:56 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> If a hart implements privileged spec v1.11 (or higher) then we can
> safely assume that mcountinhibit CSR is present and we don't need
> MCOUNTINHIBT as a hart feature.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_hart.h | 14 ++++++--------
> lib/sbi/sbi_hart.c | 23 ++++++++---------------
> lib/sbi/sbi_pmu.c | 10 +++++-----
> 3 files changed, 19 insertions(+), 28 deletions(-)
>
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index 4665a62..bc6b766 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -26,20 +26,18 @@ enum sbi_hart_priv_versions {
>
> /** Possible feature flags of a hart */
> enum sbi_hart_features {
> - /** Hart has counter inhibit CSR */
> - SBI_HART_HAS_MCOUNTINHIBIT = (1 << 0),
> /** Hart has sscofpmf extension */
> - SBI_HART_HAS_SSCOFPMF = (1 << 1),
> + SBI_HART_HAS_SSCOFPMF = (1 << 0),
> /** HART has timer csr implementation in hardware */
> - SBI_HART_HAS_TIME = (1 << 2),
> + SBI_HART_HAS_TIME = (1 << 1),
> /** HART has AIA local interrupt CSRs */
> - SBI_HART_HAS_AIA = (1 << 3),
> + SBI_HART_HAS_AIA = (1 << 2),
> /** HART has menvcfg CSR */
> - SBI_HART_HAS_MENVCFG = (1 << 4),
> + SBI_HART_HAS_MENVCFG = (1 << 3),
> /** HART has mstateen CSR **/
> - SBI_HART_HAS_SMSTATEEN = (1 << 5),
> + SBI_HART_HAS_SMSTATEEN = (1 << 4),
> /** HART has SSTC extension implemented in hardware */
> - SBI_HART_HAS_SSTC = (1 << 6),
> + SBI_HART_HAS_SSTC = (1 << 5),
>
> /** Last index of Hart features*/
> SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index ed8a061..0fe88cb 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -70,7 +70,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> csr_write(CSR_MCOUNTEREN, -1);
>
> /* All programmable counters will start running at runtime after S-mode request */
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
> csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
>
> /**
> @@ -402,9 +402,6 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
> return NULL;
>
> switch (feature) {
> - case SBI_HART_HAS_MCOUNTINHIBIT:
> - fstr = "mcountinhibit";
> - break;
> case SBI_HART_HAS_SSCOFPMF:
> fstr = "sscofpmf";
> break;
> @@ -613,26 +610,22 @@ __mhpm_skip:
> if (!trap.cause)
> hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
>
> - /* Detect if hart supports MCOUNTINHIBIT feature */
> + /* Detect if hart supports Priv v1.11 */
> val = csr_read_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap);
> - if (!trap.cause) {
> - csr_write_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap, val);
> - if (!trap.cause) {
> - hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
> - hfeatures->features |= SBI_HART_HAS_MCOUNTINHIBIT;
> - }
> - }
> + if (!trap.cause &&
> + (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_10))
> + hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
>
> /* Detect if hart has menvcfg CSR */
> csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
> - if (!trap.cause) {
> + if (!trap.cause &&
> + (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_11)) {
> hfeatures->priv_version = SBI_HART_PRIV_VER_1_12;
> hfeatures->features |= SBI_HART_HAS_MENVCFG;
> }
>
> /* Counter overflow/filtering is not useful without mcounter/inhibit */
> - if (hfeatures->features & SBI_HART_HAS_MCOUNTINHIBIT &&
> - hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> + if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> /* Detect if hart supports sscofpmf */
> csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
> if (!trap.cause)
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 853229e..386bf4d 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -300,7 +300,7 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
> if (cidx > num_hw_ctrs || cidx == 1)
> return SBI_EINVAL;
>
> - if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> + if (sbi_hart_priv_version(scratch) < SBI_HART_PRIV_VER_1_11)
> goto skip_inhibit_update;
>
> /*
> @@ -372,7 +372,7 @@ static int pmu_ctr_stop_hw(uint32_t cidx)
> struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> unsigned long mctr_inhbt;
>
> - if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> + if (sbi_hart_priv_version(scratch) < SBI_HART_PRIV_VER_1_11)
> return 0;
>
> mctr_inhbt = csr_read(CSR_MCOUNTINHIBIT);
> @@ -524,7 +524,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
> !sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> return fixed_ctr;
>
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
> mctr_inhbt = csr_read(CSR_MCOUNTINHIBIT);
> for (i = 0; i < num_hw_events; i++) {
> temp = &hw_event_map[i];
> @@ -551,7 +551,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
> if (active_events[hartid][cbase] != SBI_PMU_EVENT_IDX_INVALID)
> continue;
> /* If mcountinhibit is supported, the bit must be enabled */
> - if ((sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT)) &&
> + if ((sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11) &&
> !__test_bit(cbase, &mctr_inhbt))
> continue;
> /* We found a valid counter that is not started yet */
> @@ -726,7 +726,7 @@ void sbi_pmu_exit(struct sbi_scratch *scratch)
> {
> u32 hartid = current_hartid();
>
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
> csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
>
> if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> --
> 2.34.1
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 05/11] lib: sbi: Remove MCOUNTINHIBT hart feature
2022-05-04 1:56 ` Atish Patra
@ 2022-05-07 5:00 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 5:00 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:26 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > If a hart implements privileged spec v1.11 (or higher) then we can
> > safely assume that mcountinhibit CSR is present and we don't need
> > MCOUNTINHIBT as a hart feature.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_hart.h | 14 ++++++--------
> > lib/sbi/sbi_hart.c | 23 ++++++++---------------
> > lib/sbi/sbi_pmu.c | 10 +++++-----
> > 3 files changed, 19 insertions(+), 28 deletions(-)
> >
> > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> > index 4665a62..bc6b766 100644
> > --- a/include/sbi/sbi_hart.h
> > +++ b/include/sbi/sbi_hart.h
> > @@ -26,20 +26,18 @@ enum sbi_hart_priv_versions {
> >
> > /** Possible feature flags of a hart */
> > enum sbi_hart_features {
> > - /** Hart has counter inhibit CSR */
> > - SBI_HART_HAS_MCOUNTINHIBIT = (1 << 0),
> > /** Hart has sscofpmf extension */
> > - SBI_HART_HAS_SSCOFPMF = (1 << 1),
> > + SBI_HART_HAS_SSCOFPMF = (1 << 0),
> > /** HART has timer csr implementation in hardware */
> > - SBI_HART_HAS_TIME = (1 << 2),
> > + SBI_HART_HAS_TIME = (1 << 1),
> > /** HART has AIA local interrupt CSRs */
> > - SBI_HART_HAS_AIA = (1 << 3),
> > + SBI_HART_HAS_AIA = (1 << 2),
> > /** HART has menvcfg CSR */
> > - SBI_HART_HAS_MENVCFG = (1 << 4),
> > + SBI_HART_HAS_MENVCFG = (1 << 3),
> > /** HART has mstateen CSR **/
> > - SBI_HART_HAS_SMSTATEEN = (1 << 5),
> > + SBI_HART_HAS_SMSTATEEN = (1 << 4),
> > /** HART has SSTC extension implemented in hardware */
> > - SBI_HART_HAS_SSTC = (1 << 6),
> > + SBI_HART_HAS_SSTC = (1 << 5),
> >
> > /** Last index of Hart features*/
> > SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index ed8a061..0fe88cb 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -70,7 +70,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> > csr_write(CSR_MCOUNTEREN, -1);
> >
> > /* All programmable counters will start running at runtime after S-mode request */
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
> > csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
> >
> > /**
> > @@ -402,9 +402,6 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
> > return NULL;
> >
> > switch (feature) {
> > - case SBI_HART_HAS_MCOUNTINHIBIT:
> > - fstr = "mcountinhibit";
> > - break;
> > case SBI_HART_HAS_SSCOFPMF:
> > fstr = "sscofpmf";
> > break;
> > @@ -613,26 +610,22 @@ __mhpm_skip:
> > if (!trap.cause)
> > hfeatures->priv_version = SBI_HART_PRIV_VER_1_10;
> >
> > - /* Detect if hart supports MCOUNTINHIBIT feature */
> > + /* Detect if hart supports Priv v1.11 */
> > val = csr_read_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap);
> > - if (!trap.cause) {
> > - csr_write_allowed(CSR_MCOUNTINHIBIT, (unsigned long)&trap, val);
> > - if (!trap.cause) {
> > - hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
> > - hfeatures->features |= SBI_HART_HAS_MCOUNTINHIBIT;
> > - }
> > - }
> > + if (!trap.cause &&
> > + (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_10))
> > + hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
> >
> > /* Detect if hart has menvcfg CSR */
> > csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
> > - if (!trap.cause) {
> > + if (!trap.cause &&
> > + (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_11)) {
> > hfeatures->priv_version = SBI_HART_PRIV_VER_1_12;
> > hfeatures->features |= SBI_HART_HAS_MENVCFG;
> > }
> >
> > /* Counter overflow/filtering is not useful without mcounter/inhibit */
> > - if (hfeatures->features & SBI_HART_HAS_MCOUNTINHIBIT &&
> > - hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> > + if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> > /* Detect if hart supports sscofpmf */
> > csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
> > if (!trap.cause)
> > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> > index 853229e..386bf4d 100644
> > --- a/lib/sbi/sbi_pmu.c
> > +++ b/lib/sbi/sbi_pmu.c
> > @@ -300,7 +300,7 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
> > if (cidx > num_hw_ctrs || cidx == 1)
> > return SBI_EINVAL;
> >
> > - if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> > + if (sbi_hart_priv_version(scratch) < SBI_HART_PRIV_VER_1_11)
> > goto skip_inhibit_update;
> >
> > /*
> > @@ -372,7 +372,7 @@ static int pmu_ctr_stop_hw(uint32_t cidx)
> > struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> > unsigned long mctr_inhbt;
> >
> > - if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> > + if (sbi_hart_priv_version(scratch) < SBI_HART_PRIV_VER_1_11)
> > return 0;
> >
> > mctr_inhbt = csr_read(CSR_MCOUNTINHIBIT);
> > @@ -524,7 +524,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
> > !sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> > return fixed_ctr;
> >
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
> > mctr_inhbt = csr_read(CSR_MCOUNTINHIBIT);
> > for (i = 0; i < num_hw_events; i++) {
> > temp = &hw_event_map[i];
> > @@ -551,7 +551,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
> > if (active_events[hartid][cbase] != SBI_PMU_EVENT_IDX_INVALID)
> > continue;
> > /* If mcountinhibit is supported, the bit must be enabled */
> > - if ((sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT)) &&
> > + if ((sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11) &&
> > !__test_bit(cbase, &mctr_inhbt))
> > continue;
> > /* We found a valid counter that is not started yet */
> > @@ -726,7 +726,7 @@ void sbi_pmu_exit(struct sbi_scratch *scratch)
> > {
> > u32 hartid = current_hartid();
> >
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
> > csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
> >
> > if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
> > --
> > 2.34.1
> >
>
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 06/11] lib: sbi: Remove MENVCFG hart feature
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
` (4 preceding siblings ...)
2022-04-29 15:51 ` [PATCH 05/11] lib: sbi: Remove MCOUNTINHIBT hart feature Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 1:57 ` Atish Patra
2022-04-29 15:51 ` [PATCH 07/11] lib: sbi: Fix AIA feature detection Anup Patel
` (5 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
If a hart implements privileged spec v1.12 (or higher) then we can
safely assume that menvcfg CSR is present and we don't need MENVCFG
as a hart feature.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_hart.h | 6 ++----
lib/sbi/sbi_hart.c | 32 ++++++++++++--------------------
2 files changed, 14 insertions(+), 24 deletions(-)
diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index bc6b766..c985674 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -32,12 +32,10 @@ enum sbi_hart_features {
SBI_HART_HAS_TIME = (1 << 1),
/** HART has AIA local interrupt CSRs */
SBI_HART_HAS_AIA = (1 << 2),
- /** HART has menvcfg CSR */
- SBI_HART_HAS_MENVCFG = (1 << 3),
/** HART has mstateen CSR **/
- SBI_HART_HAS_SMSTATEEN = (1 << 4),
+ SBI_HART_HAS_SMSTATEEN = (1 << 3),
/** HART has SSTC extension implemented in hardware */
- SBI_HART_HAS_SSTC = (1 << 5),
+ SBI_HART_HAS_SSTC = (1 << 4),
/** Last index of Hart features*/
SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 0fe88cb..5ee5ddd 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -97,10 +97,8 @@ static void mstatus_init(struct sbi_scratch *scratch)
mstateen_val |= ((uint64_t)csr_read(CSR_MSTATEEN0H)) << 32;
#endif
mstateen_val |= SMSTATEEN_STATEN;
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MENVCFG))
- mstateen_val |= SMSTATEEN0_HSENVCFG;
- else
- mstateen_val &= ~SMSTATEEN0_HSENVCFG;
+ mstateen_val |= SMSTATEEN0_HSENVCFG;
+
if (sbi_hart_has_feature(scratch, SBI_HART_HAS_AIA))
mstateen_val |= (SMSTATEEN0_AIA | SMSTATEEN0_SVSLCT |
SMSTATEEN0_IMSIC);
@@ -113,7 +111,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
#endif
}
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MENVCFG)) {
+ if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_12) {
menvcfg_val = csr_read(CSR_MENVCFG);
/*
@@ -413,9 +411,6 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
case SBI_HART_HAS_SSTC:
fstr = "sstc";
break;
- case SBI_HART_HAS_MENVCFG:
- fstr = "menvcfg";
- break;
case SBI_HART_HAS_SMSTATEEN:
fstr = "smstateen";
break;
@@ -616,13 +611,11 @@ __mhpm_skip:
(hfeatures->priv_version >= SBI_HART_PRIV_VER_1_10))
hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
- /* Detect if hart has menvcfg CSR */
+ /* Detect if hart supports Priv v1.12 */
csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
if (!trap.cause &&
- (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_11)) {
+ (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_11))
hfeatures->priv_version = SBI_HART_PRIV_VER_1_12;
- hfeatures->features |= SBI_HART_HAS_MENVCFG;
- }
/* Counter overflow/filtering is not useful without mcounter/inhibit */
if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
@@ -644,20 +637,19 @@ __mhpm_skip:
hfeatures->features |= SBI_HART_HAS_AIA;
__aia_skip:
- /**
- * Detect if hart supports stimecmp CSR(Sstc extension) and menvcfg is
- * implemented.
- */
- if (hfeatures->features & SBI_HART_HAS_MENVCFG) {
+ /* Detect if hart supports stimecmp CSR(Sstc extension) */
+ if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
if (!trap.cause)
hfeatures->features |= SBI_HART_HAS_SSTC;
}
/* Detect if hart supports mstateen CSRs */
- val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
- if (!trap.cause)
- hfeatures->features |= SBI_HART_HAS_SMSTATEEN;
+ if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
+ val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
+ if (!trap.cause)
+ hfeatures->features |= SBI_HART_HAS_SMSTATEEN;
+ }
return;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 06/11] lib: sbi: Remove MENVCFG hart feature
2022-04-29 15:51 ` [PATCH 06/11] lib: sbi: Remove MENVCFG " Anup Patel
@ 2022-05-04 1:57 ` Atish Patra
2022-05-07 5:03 ` Anup Patel
0 siblings, 1 reply; 37+ messages in thread
From: Atish Patra @ 2022-05-04 1:57 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> If a hart implements privileged spec v1.12 (or higher) then we can
> safely assume that menvcfg CSR is present and we don't need MENVCFG
> as a hart feature.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_hart.h | 6 ++----
> lib/sbi/sbi_hart.c | 32 ++++++++++++--------------------
> 2 files changed, 14 insertions(+), 24 deletions(-)
>
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index bc6b766..c985674 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -32,12 +32,10 @@ enum sbi_hart_features {
> SBI_HART_HAS_TIME = (1 << 1),
> /** HART has AIA local interrupt CSRs */
> SBI_HART_HAS_AIA = (1 << 2),
> - /** HART has menvcfg CSR */
> - SBI_HART_HAS_MENVCFG = (1 << 3),
> /** HART has mstateen CSR **/
> - SBI_HART_HAS_SMSTATEEN = (1 << 4),
> + SBI_HART_HAS_SMSTATEEN = (1 << 3),
> /** HART has SSTC extension implemented in hardware */
> - SBI_HART_HAS_SSTC = (1 << 5),
> + SBI_HART_HAS_SSTC = (1 << 4),
>
> /** Last index of Hart features*/
> SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 0fe88cb..5ee5ddd 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -97,10 +97,8 @@ static void mstatus_init(struct sbi_scratch *scratch)
> mstateen_val |= ((uint64_t)csr_read(CSR_MSTATEEN0H)) << 32;
> #endif
> mstateen_val |= SMSTATEEN_STATEN;
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MENVCFG))
> - mstateen_val |= SMSTATEEN0_HSENVCFG;
> - else
> - mstateen_val &= ~SMSTATEEN0_HSENVCFG;
> + mstateen_val |= SMSTATEEN0_HSENVCFG;
> +
> if (sbi_hart_has_feature(scratch, SBI_HART_HAS_AIA))
> mstateen_val |= (SMSTATEEN0_AIA | SMSTATEEN0_SVSLCT |
> SMSTATEEN0_IMSIC);
> @@ -113,7 +111,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> #endif
> }
>
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MENVCFG)) {
> + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_12) {
> menvcfg_val = csr_read(CSR_MENVCFG);
>
> /*
> @@ -413,9 +411,6 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
> case SBI_HART_HAS_SSTC:
> fstr = "sstc";
> break;
> - case SBI_HART_HAS_MENVCFG:
> - fstr = "menvcfg";
> - break;
> case SBI_HART_HAS_SMSTATEEN:
> fstr = "smstateen";
> break;
> @@ -616,13 +611,11 @@ __mhpm_skip:
> (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_10))
> hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
>
> - /* Detect if hart has menvcfg CSR */
> + /* Detect if hart supports Priv v1.12 */
> csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
> if (!trap.cause &&
> - (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_11)) {
> + (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_11))
> hfeatures->priv_version = SBI_HART_PRIV_VER_1_12;
> - hfeatures->features |= SBI_HART_HAS_MENVCFG;
> - }
>
> /* Counter overflow/filtering is not useful without mcounter/inhibit */
> if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> @@ -644,20 +637,19 @@ __mhpm_skip:
> hfeatures->features |= SBI_HART_HAS_AIA;
> __aia_skip:
>
> - /**
> - * Detect if hart supports stimecmp CSR(Sstc extension) and menvcfg is
> - * implemented.
> - */
> - if (hfeatures->features & SBI_HART_HAS_MENVCFG) {
> + /* Detect if hart supports stimecmp CSR(Sstc extension) */
> + if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
> if (!trap.cause)
> hfeatures->features |= SBI_HART_HAS_SSTC;
> }
>
> /* Detect if hart supports mstateen CSRs */
> - val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
> - if (!trap.cause)
> - hfeatures->features |= SBI_HART_HAS_SMSTATEEN;
> + if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> + val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
> + if (!trap.cause)
> + hfeatures->features |= SBI_HART_HAS_SMSTATEEN;
> + }
>
> return;
> }
> --
> 2.34.1
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 06/11] lib: sbi: Remove MENVCFG hart feature
2022-05-04 1:57 ` Atish Patra
@ 2022-05-07 5:03 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 5:03 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:27 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > If a hart implements privileged spec v1.12 (or higher) then we can
> > safely assume that menvcfg CSR is present and we don't need MENVCFG
> > as a hart feature.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_hart.h | 6 ++----
> > lib/sbi/sbi_hart.c | 32 ++++++++++++--------------------
> > 2 files changed, 14 insertions(+), 24 deletions(-)
> >
> > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> > index bc6b766..c985674 100644
> > --- a/include/sbi/sbi_hart.h
> > +++ b/include/sbi/sbi_hart.h
> > @@ -32,12 +32,10 @@ enum sbi_hart_features {
> > SBI_HART_HAS_TIME = (1 << 1),
> > /** HART has AIA local interrupt CSRs */
> > SBI_HART_HAS_AIA = (1 << 2),
> > - /** HART has menvcfg CSR */
> > - SBI_HART_HAS_MENVCFG = (1 << 3),
> > /** HART has mstateen CSR **/
> > - SBI_HART_HAS_SMSTATEEN = (1 << 4),
> > + SBI_HART_HAS_SMSTATEEN = (1 << 3),
> > /** HART has SSTC extension implemented in hardware */
> > - SBI_HART_HAS_SSTC = (1 << 5),
> > + SBI_HART_HAS_SSTC = (1 << 4),
> >
> > /** Last index of Hart features*/
> > SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 0fe88cb..5ee5ddd 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -97,10 +97,8 @@ static void mstatus_init(struct sbi_scratch *scratch)
> > mstateen_val |= ((uint64_t)csr_read(CSR_MSTATEEN0H)) << 32;
> > #endif
> > mstateen_val |= SMSTATEEN_STATEN;
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MENVCFG))
> > - mstateen_val |= SMSTATEEN0_HSENVCFG;
> > - else
> > - mstateen_val &= ~SMSTATEEN0_HSENVCFG;
> > + mstateen_val |= SMSTATEEN0_HSENVCFG;
> > +
> > if (sbi_hart_has_feature(scratch, SBI_HART_HAS_AIA))
> > mstateen_val |= (SMSTATEEN0_AIA | SMSTATEEN0_SVSLCT |
> > SMSTATEEN0_IMSIC);
> > @@ -113,7 +111,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> > #endif
> > }
> >
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MENVCFG)) {
> > + if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_12) {
> > menvcfg_val = csr_read(CSR_MENVCFG);
> >
> > /*
> > @@ -413,9 +411,6 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
> > case SBI_HART_HAS_SSTC:
> > fstr = "sstc";
> > break;
> > - case SBI_HART_HAS_MENVCFG:
> > - fstr = "menvcfg";
> > - break;
> > case SBI_HART_HAS_SMSTATEEN:
> > fstr = "smstateen";
> > break;
> > @@ -616,13 +611,11 @@ __mhpm_skip:
> > (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_10))
> > hfeatures->priv_version = SBI_HART_PRIV_VER_1_11;
> >
> > - /* Detect if hart has menvcfg CSR */
> > + /* Detect if hart supports Priv v1.12 */
> > csr_read_allowed(CSR_MENVCFG, (unsigned long)&trap);
> > if (!trap.cause &&
> > - (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_11)) {
> > + (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_11))
> > hfeatures->priv_version = SBI_HART_PRIV_VER_1_12;
> > - hfeatures->features |= SBI_HART_HAS_MENVCFG;
> > - }
> >
> > /* Counter overflow/filtering is not useful without mcounter/inhibit */
> > if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> > @@ -644,20 +637,19 @@ __mhpm_skip:
> > hfeatures->features |= SBI_HART_HAS_AIA;
> > __aia_skip:
> >
> > - /**
> > - * Detect if hart supports stimecmp CSR(Sstc extension) and menvcfg is
> > - * implemented.
> > - */
> > - if (hfeatures->features & SBI_HART_HAS_MENVCFG) {
> > + /* Detect if hart supports stimecmp CSR(Sstc extension) */
> > + if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> > csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
> > if (!trap.cause)
> > hfeatures->features |= SBI_HART_HAS_SSTC;
> > }
> >
> > /* Detect if hart supports mstateen CSRs */
> > - val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
> > - if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_SMSTATEEN;
> > + if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> > + val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
> > + if (!trap.cause)
> > + hfeatures->features |= SBI_HART_HAS_SMSTATEEN;
> > + }
> >
> > return;
> > }
> > --
> > 2.34.1
> >
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
>
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 07/11] lib: sbi: Fix AIA feature detection
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
` (5 preceding siblings ...)
2022-04-29 15:51 ` [PATCH 06/11] lib: sbi: Remove MENVCFG " Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 1:57 ` Atish Patra
2022-04-29 15:51 ` [PATCH 08/11] lib: sbi: Convert hart features into hart extensions Anup Patel
` (4 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
The AIA feature detection uses unnecessary goto which is not need
and AIA case in sbi_hart_feature_id2string() does not break. This
patch fixes both issues in AIA feature detection.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
lib/sbi/sbi_hart.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 5ee5ddd..fdfb6d9 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -408,6 +408,7 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
break;
case SBI_HART_HAS_AIA:
fstr = "aia";
+ break;
case SBI_HART_HAS_SSTC:
fstr = "sstc";
break;
@@ -632,10 +633,8 @@ __mhpm_skip:
/* Detect if hart has AIA local interrupt CSRs */
csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
- if (trap.cause)
- goto __aia_skip;
- hfeatures->features |= SBI_HART_HAS_AIA;
-__aia_skip:
+ if (!trap.cause)
+ hfeatures->features |= SBI_HART_HAS_AIA;
/* Detect if hart supports stimecmp CSR(Sstc extension) */
if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 07/11] lib: sbi: Fix AIA feature detection
2022-04-29 15:51 ` [PATCH 07/11] lib: sbi: Fix AIA feature detection Anup Patel
@ 2022-05-04 1:57 ` Atish Patra
2022-05-07 5:03 ` Anup Patel
0 siblings, 1 reply; 37+ messages in thread
From: Atish Patra @ 2022-05-04 1:57 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The AIA feature detection uses unnecessary goto which is not need
> and AIA case in sbi_hart_feature_id2string() does not break. This
> patch fixes both issues in AIA feature detection.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> lib/sbi/sbi_hart.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 5ee5ddd..fdfb6d9 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -408,6 +408,7 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
> break;
> case SBI_HART_HAS_AIA:
> fstr = "aia";
> + break;
> case SBI_HART_HAS_SSTC:
> fstr = "sstc";
> break;
> @@ -632,10 +633,8 @@ __mhpm_skip:
>
> /* Detect if hart has AIA local interrupt CSRs */
> csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
> - if (trap.cause)
> - goto __aia_skip;
> - hfeatures->features |= SBI_HART_HAS_AIA;
> -__aia_skip:
> + if (!trap.cause)
> + hfeatures->features |= SBI_HART_HAS_AIA;
>
> /* Detect if hart supports stimecmp CSR(Sstc extension) */
> if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> --
> 2.34.1
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 07/11] lib: sbi: Fix AIA feature detection
2022-05-04 1:57 ` Atish Patra
@ 2022-05-07 5:03 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 5:03 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:28 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > The AIA feature detection uses unnecessary goto which is not need
> > and AIA case in sbi_hart_feature_id2string() does not break. This
> > patch fixes both issues in AIA feature detection.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > lib/sbi/sbi_hart.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 5ee5ddd..fdfb6d9 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -408,6 +408,7 @@ static inline char *sbi_hart_feature_id2string(unsigned long feature)
> > break;
> > case SBI_HART_HAS_AIA:
> > fstr = "aia";
> > + break;
> > case SBI_HART_HAS_SSTC:
> > fstr = "sstc";
> > break;
> > @@ -632,10 +633,8 @@ __mhpm_skip:
> >
> > /* Detect if hart has AIA local interrupt CSRs */
> > csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
> > - if (trap.cause)
> > - goto __aia_skip;
> > - hfeatures->features |= SBI_HART_HAS_AIA;
> > -__aia_skip:
> > + if (!trap.cause)
> > + hfeatures->features |= SBI_HART_HAS_AIA;
> >
> > /* Detect if hart supports stimecmp CSR(Sstc extension) */
> > if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> > --
> > 2.34.1
> >
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
>
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 08/11] lib: sbi: Convert hart features into hart extensions
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
` (6 preceding siblings ...)
2022-04-29 15:51 ` [PATCH 07/11] lib: sbi: Fix AIA feature detection Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 2:00 ` Atish Patra
2022-04-29 15:51 ` [PATCH 09/11] lib: sbi: Detect hart features only once for each hart Anup Patel
` (3 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
Since past few years, we have been using "hart features" in OpenSBI
to represent all optionalities and multi-letter extensions defined
by the RISC-V specifications.
The RISC-V profiles specification has taken a different approach and
started assigning extension names for all optionalities which did not
have any extension name previously.
(Refer, https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc)
Inspired from the RISC-V profiles specification, we convert OpenSBI
hart features into hart extensions. Going forward, we align the
extension naming with RISC-V profiles specification. Currently, only
"time CSR" and "AIA CSR" have not been assigned extension name but
for everything else we have a name.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_hart.h | 35 ++++++------
lib/sbi/sbi_hart.c | 116 +++++++++++++++++++---------------------
lib/sbi/sbi_init.c | 4 +-
lib/sbi/sbi_pmu.c | 6 +--
lib/sbi/sbi_timer.c | 6 +--
lib/sbi/sbi_trap.c | 4 +-
lib/utils/fdt/fdt_pmu.c | 2 +-
7 files changed, 83 insertions(+), 90 deletions(-)
diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index c985674..8b21fd4 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -24,21 +24,21 @@ enum sbi_hart_priv_versions {
SBI_HART_PRIV_VER_1_12 = 3,
};
-/** Possible feature flags of a hart */
-enum sbi_hart_features {
- /** Hart has sscofpmf extension */
- SBI_HART_HAS_SSCOFPMF = (1 << 0),
- /** HART has timer csr implementation in hardware */
- SBI_HART_HAS_TIME = (1 << 1),
- /** HART has AIA local interrupt CSRs */
- SBI_HART_HAS_AIA = (1 << 2),
- /** HART has mstateen CSR **/
- SBI_HART_HAS_SMSTATEEN = (1 << 3),
- /** HART has SSTC extension implemented in hardware */
- SBI_HART_HAS_SSTC = (1 << 4),
+/** Possible ISA extensions of a hart */
+enum sbi_hart_extensions {
+ /** Hart has Sscofpmt extension */
+ SBI_HART_EXT_SSCOFPMF = 0,
+ /** HART has HW time CSR (extension name not available) */
+ SBI_HART_EXT_TIME,
+ /** HART has AIA CSRs (extension name not available) */
+ SBI_HART_EXT_AIA,
+ /** HART has Smstateen CSR **/
+ SBI_HART_EXT_SMSTATEEN,
+ /** HART has Sstc extension */
+ SBI_HART_EXT_SSTC,
- /** Last index of Hart features*/
- SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
+ /** Maximum index of Hart extension */
+ SBI_HART_EXT_MAX,
};
struct sbi_scratch;
@@ -63,9 +63,10 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
int sbi_hart_priv_version(struct sbi_scratch *scratch);
void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
char *version_str, int nvstr);
-bool sbi_hart_has_feature(struct sbi_scratch *scratch, unsigned long feature);
-void sbi_hart_get_features_str(struct sbi_scratch *scratch,
- char *features_str, int nfstr);
+bool sbi_hart_has_extension(struct sbi_scratch *scratch,
+ enum sbi_hart_extensions ext);
+void sbi_hart_get_extensions_str(struct sbi_scratch *scratch,
+ char *extension_str, int nestr);
void __attribute__((noreturn)) sbi_hart_hang(void);
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index fdfb6d9..956d185 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -29,7 +29,7 @@ void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
struct hart_features {
int priv_version;
- unsigned long features;
+ unsigned long extensions;
unsigned int pmp_count;
unsigned int pmp_addr_bits;
unsigned long pmp_gran;
@@ -83,7 +83,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
for (cidx = 0; cidx < num_mhpm; cidx++) {
#if __riscv_xlen == 32
csr_write_num(CSR_MHPMEVENT3 + cidx, mhpmevent_init_val & 0xFFFFFFFF);
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
+ if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
csr_write_num(CSR_MHPMEVENT3H + cidx,
mhpmevent_init_val >> BITS_PER_LONG);
#else
@@ -91,7 +91,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
#endif
}
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SMSTATEEN)) {
+ if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMSTATEEN)) {
mstateen_val = csr_read(CSR_MSTATEEN0);
#if __riscv_xlen == 32
mstateen_val |= ((uint64_t)csr_read(CSR_MSTATEEN0H)) << 32;
@@ -99,7 +99,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
mstateen_val |= SMSTATEEN_STATEN;
mstateen_val |= SMSTATEEN0_HSENVCFG;
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_AIA))
+ if (sbi_hart_has_extension(scratch, SBI_HART_EXT_AIA))
mstateen_val |= (SMSTATEEN0_AIA | SMSTATEEN0_SVSLCT |
SMSTATEEN0_IMSIC);
else
@@ -153,7 +153,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
* Enable access to stimecmp if sstc extension is present in the
* hardware.
*/
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSTC)) {
+ if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSTC)) {
#if __riscv_xlen == 32
unsigned long menvcfgh_val;
menvcfgh_val = csr_read(CSR_MENVCFGH);
@@ -207,7 +207,7 @@ static int delegate_traps(struct sbi_scratch *scratch)
/* Send M-mode interrupts and most exceptions to S-mode */
interrupts = MIP_SSIP | MIP_STIP | MIP_SEIP;
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
+ if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
interrupts |= MIP_LCOFIP;
exceptions = (1U << CAUSE_MISALIGNED_FETCH) | (1U << CAUSE_BREAKPOINT) |
@@ -367,102 +367,94 @@ void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
}
/**
- * Check whether a particular hart feature is available
+ * Check whether a particular hart extension is available
*
* @param scratch pointer to the HART scratch space
- * @param feature the feature to check
- * @returns true (feature available) or false (feature not available)
+ * @param ext the extension number to check
+ * @returns true (available) or false (not available)
*/
-bool sbi_hart_has_feature(struct sbi_scratch *scratch, unsigned long feature)
+bool sbi_hart_has_extension(struct sbi_scratch *scratch,
+ enum sbi_hart_extensions ext)
{
struct hart_features *hfeatures =
sbi_scratch_offset_ptr(scratch, hart_features_offset);
- if (hfeatures->features & feature)
+ if (hfeatures->extensions & BIT(ext))
return true;
else
return false;
}
-static unsigned long hart_get_features(struct sbi_scratch *scratch)
+static inline char *sbi_hart_extension_id2string(int ext)
{
- struct hart_features *hfeatures =
- sbi_scratch_offset_ptr(scratch, hart_features_offset);
-
- return hfeatures->features;
-}
-
-static inline char *sbi_hart_feature_id2string(unsigned long feature)
-{
- char *fstr = NULL;
-
- if (!feature)
- return NULL;
+ char *estr = NULL;
- switch (feature) {
- case SBI_HART_HAS_SSCOFPMF:
- fstr = "sscofpmf";
+ switch (ext) {
+ case SBI_HART_EXT_SSCOFPMF:
+ estr = "sscofpmf";
break;
- case SBI_HART_HAS_TIME:
- fstr = "time";
+ case SBI_HART_EXT_TIME:
+ estr = "time";
break;
- case SBI_HART_HAS_AIA:
- fstr = "aia";
+ case SBI_HART_EXT_AIA:
+ estr = "aia";
break;
- case SBI_HART_HAS_SSTC:
- fstr = "sstc";
+ case SBI_HART_EXT_SSTC:
+ estr = "sstc";
break;
- case SBI_HART_HAS_SMSTATEEN:
- fstr = "smstateen";
+ case SBI_HART_EXT_SMSTATEEN:
+ estr = "smstateen";
break;
default:
break;
}
- return fstr;
+ return estr;
}
/**
- * Get the hart features in string format
+ * Get the hart extensions in string format
*
* @param scratch pointer to the HART scratch space
- * @param features_str pointer to a char array where the features string will be
- * updated
- * @param nfstr length of the features_str. The feature string will be truncated
- * if nfstr is not long enough.
+ * @param extensions_str pointer to a char array where the extensions string
+ * will be updated
+ * @param nestr length of the features_str. The feature string will be
+ * truncated if nestr is not long enough.
*/
-void sbi_hart_get_features_str(struct sbi_scratch *scratch,
- char *features_str, int nfstr)
+void sbi_hart_get_extensions_str(struct sbi_scratch *scratch,
+ char *extensions_str, int nestr)
{
- unsigned long features, feat = 1UL;
+ struct hart_features *hfeatures =
+ sbi_scratch_offset_ptr(scratch, hart_features_offset);
+ int offset = 0, ext = 0;
char *temp;
- int offset = 0;
- if (!features_str || nfstr <= 0)
+ if (!extensions_str || nestr <= 0)
return;
- sbi_memset(features_str, 0, nfstr);
+ sbi_memset(extensions_str, 0, nestr);
- features = hart_get_features(scratch);
- if (!features)
+ if (!hfeatures->extensions)
goto done;
do {
- if (features & feat) {
- temp = sbi_hart_feature_id2string(feat);
+ if (hfeatures->extensions & BIT(ext)) {
+ temp = sbi_hart_extension_id2string(ext);
if (temp) {
- sbi_snprintf(features_str + offset, nfstr,
+ sbi_snprintf(extensions_str + offset,
+ nestr - offset,
"%s,", temp);
offset = offset + sbi_strlen(temp) + 1;
}
}
- feat = feat << 1;
- } while (feat <= SBI_HART_HAS_LAST_FEATURE);
+
+ ext++;
+ } while (ext <= SBI_HART_EXT_MAX);
done:
if (offset)
- features_str[offset - 1] = '\0';
+ extensions_str[offset - 1] = '\0';
else
- sbi_strncpy(features_str, "none", nfstr);
+ sbi_strncpy(extensions_str, "none", nestr);
}
static unsigned long hart_pmp_get_allowed_addr(void)
@@ -523,7 +515,7 @@ static void hart_detect_features(struct sbi_scratch *scratch)
/* Reset hart features */
hfeatures = sbi_scratch_offset_ptr(scratch, hart_features_offset);
- hfeatures->features = 0;
+ hfeatures->extensions = 0;
hfeatures->pmp_count = 0;
hfeatures->mhpm_count = 0;
@@ -623,31 +615,31 @@ __mhpm_skip:
/* Detect if hart supports sscofpmf */
csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
if (!trap.cause)
- hfeatures->features |= SBI_HART_HAS_SSCOFPMF;
+ hfeatures->extensions |= BIT(SBI_HART_EXT_SSCOFPMF);
}
/* Detect if hart supports time CSR */
csr_read_allowed(CSR_TIME, (unsigned long)&trap);
if (!trap.cause)
- hfeatures->features |= SBI_HART_HAS_TIME;
+ hfeatures->extensions |= BIT(SBI_HART_EXT_TIME);
/* Detect if hart has AIA local interrupt CSRs */
csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
if (!trap.cause)
- hfeatures->features |= SBI_HART_HAS_AIA;
+ hfeatures->extensions |= BIT(SBI_HART_EXT_AIA);
/* Detect if hart supports stimecmp CSR(Sstc extension) */
if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
if (!trap.cause)
- hfeatures->features |= SBI_HART_HAS_SSTC;
+ hfeatures->extensions |= BIT(SBI_HART_EXT_SSTC);
}
/* Detect if hart supports mstateen CSRs */
if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
if (!trap.cause)
- hfeatures->features |= SBI_HART_HAS_SMSTATEEN;
+ hfeatures->extensions |= BIT(SBI_HART_EXT_SMSTATEEN);
}
return;
diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
index c4def58..d57efa7 100644
--- a/lib/sbi/sbi_init.c
+++ b/lib/sbi/sbi_init.c
@@ -143,8 +143,8 @@ static void sbi_boot_print_hart(struct sbi_scratch *scratch, u32 hartid)
sbi_printf("Boot HART Priv Version : %s\n", str);
misa_string(xlen, str, sizeof(str));
sbi_printf("Boot HART Base ISA : %s\n", str);
- sbi_hart_get_features_str(scratch, str, sizeof(str));
- sbi_printf("Boot HART Features : %s\n", str);
+ sbi_hart_get_extensions_str(scratch, str, sizeof(str));
+ sbi_printf("Boot HART ISA Extensions : %s\n", str);
sbi_printf("Boot HART PMP Count : %d\n",
sbi_hart_pmp_count(scratch));
sbi_printf("Boot HART PMP Granularity : %lu\n",
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 386bf4d..b60799e 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -313,7 +313,7 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
__clear_bit(cidx, &mctr_inhbt);
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
+ if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
pmu_ctr_enable_irq_hw(cidx);
csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
@@ -474,7 +474,7 @@ static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
* Always set the OVF bit(disable interrupts) and inhibit counting of
* events in M-mode. The OVF bit should be enabled during the start call.
*/
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
+ if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
mhpmevent_val = (mhpmevent_val & ~MHPMEVENT_SSCOF_MASK) |
MHPMEVENT_MINH | MHPMEVENT_OF;
@@ -521,7 +521,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
*/
fixed_ctr = pmu_ctr_find_fixed_fw(event_idx);
if (fixed_ctr >= 0 &&
- !sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
+ !sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
return fixed_ctr;
if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
diff --git a/lib/sbi/sbi_timer.c b/lib/sbi/sbi_timer.c
index bf0f375..353886f 100644
--- a/lib/sbi/sbi_timer.c
+++ b/lib/sbi/sbi_timer.c
@@ -129,7 +129,7 @@ void sbi_timer_event_start(u64 next_event)
* 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 (sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_SSTC)) {
#if __riscv_xlen == 32
csr_write(CSR_STIMECMP, next_event & 0xFFFFFFFF);
csr_write(CSR_STIMECMPH, next_event >> 32);
@@ -151,7 +151,7 @@ void sbi_timer_process(void)
* directly without M-mode come in between. This function should
* 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))
+ if (!sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_SSTC))
csr_set(CSR_MIP, MIP_STIP);
}
@@ -180,7 +180,7 @@ int sbi_timer_init(struct sbi_scratch *scratch, bool cold_boot)
if (!time_delta_off)
return SBI_ENOMEM;
- if (sbi_hart_has_feature(scratch, SBI_HART_HAS_TIME))
+ if (sbi_hart_has_extension(scratch, SBI_HART_EXT_TIME))
get_time_val = get_ticks;
} else {
if (!time_delta_off)
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index 86bab6d..2c509e5 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -272,8 +272,8 @@ struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs)
}
if (mcause & (1UL << (__riscv_xlen - 1))) {
- if (sbi_hart_has_feature(sbi_scratch_thishart_ptr(),
- SBI_HART_HAS_AIA))
+ if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
+ SBI_HART_EXT_AIA))
rc = sbi_trap_aia_irq(regs, mcause);
else
rc = sbi_trap_nonaia_irq(regs, mcause);
diff --git a/lib/utils/fdt/fdt_pmu.c b/lib/utils/fdt/fdt_pmu.c
index a2b048f..8ba6b08 100644
--- a/lib/utils/fdt/fdt_pmu.c
+++ b/lib/utils/fdt/fdt_pmu.c
@@ -53,7 +53,7 @@ int fdt_pmu_fixup(void *fdt)
fdt_delprop(fdt, pmu_offset, "riscv,event-to-mhpmcounters");
fdt_delprop(fdt, pmu_offset, "riscv,event-to-mhpmevent");
fdt_delprop(fdt, pmu_offset, "riscv,raw-event-to-mhpmcounters");
- if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
+ if (!sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
fdt_delprop(fdt, pmu_offset, "interrupts-extended");
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 08/11] lib: sbi: Convert hart features into hart extensions
2022-04-29 15:51 ` [PATCH 08/11] lib: sbi: Convert hart features into hart extensions Anup Patel
@ 2022-05-04 2:00 ` Atish Patra
2022-05-07 5:04 ` Anup Patel
0 siblings, 1 reply; 37+ messages in thread
From: Atish Patra @ 2022-05-04 2:00 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Since past few years, we have been using "hart features" in OpenSBI
> to represent all optionalities and multi-letter extensions defined
> by the RISC-V specifications.
>
> The RISC-V profiles specification has taken a different approach and
> started assigning extension names for all optionalities which did not
> have any extension name previously.
> (Refer, https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc)
>
> Inspired from the RISC-V profiles specification, we convert OpenSBI
> hart features into hart extensions. Going forward, we align the
> extension naming with RISC-V profiles specification. Currently, only
> "time CSR" and "AIA CSR" have not been assigned extension name but
> for everything else we have a name.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_hart.h | 35 ++++++------
> lib/sbi/sbi_hart.c | 116 +++++++++++++++++++---------------------
> lib/sbi/sbi_init.c | 4 +-
> lib/sbi/sbi_pmu.c | 6 +--
> lib/sbi/sbi_timer.c | 6 +--
> lib/sbi/sbi_trap.c | 4 +-
> lib/utils/fdt/fdt_pmu.c | 2 +-
> 7 files changed, 83 insertions(+), 90 deletions(-)
>
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index c985674..8b21fd4 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -24,21 +24,21 @@ enum sbi_hart_priv_versions {
> SBI_HART_PRIV_VER_1_12 = 3,
> };
>
> -/** Possible feature flags of a hart */
> -enum sbi_hart_features {
> - /** Hart has sscofpmf extension */
> - SBI_HART_HAS_SSCOFPMF = (1 << 0),
> - /** HART has timer csr implementation in hardware */
> - SBI_HART_HAS_TIME = (1 << 1),
> - /** HART has AIA local interrupt CSRs */
> - SBI_HART_HAS_AIA = (1 << 2),
> - /** HART has mstateen CSR **/
> - SBI_HART_HAS_SMSTATEEN = (1 << 3),
> - /** HART has SSTC extension implemented in hardware */
> - SBI_HART_HAS_SSTC = (1 << 4),
> +/** Possible ISA extensions of a hart */
> +enum sbi_hart_extensions {
> + /** Hart has Sscofpmt extension */
> + SBI_HART_EXT_SSCOFPMF = 0,
> + /** HART has HW time CSR (extension name not available) */
> + SBI_HART_EXT_TIME,
> + /** HART has AIA CSRs (extension name not available) */
> + SBI_HART_EXT_AIA,
> + /** HART has Smstateen CSR **/
> + SBI_HART_EXT_SMSTATEEN,
> + /** HART has Sstc extension */
> + SBI_HART_EXT_SSTC,
>
> - /** Last index of Hart features*/
> - SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
> + /** Maximum index of Hart extension */
> + SBI_HART_EXT_MAX,
> };
>
> struct sbi_scratch;
> @@ -63,9 +63,10 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
> int sbi_hart_priv_version(struct sbi_scratch *scratch);
> void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> char *version_str, int nvstr);
> -bool sbi_hart_has_feature(struct sbi_scratch *scratch, unsigned long feature);
> -void sbi_hart_get_features_str(struct sbi_scratch *scratch,
> - char *features_str, int nfstr);
> +bool sbi_hart_has_extension(struct sbi_scratch *scratch,
> + enum sbi_hart_extensions ext);
> +void sbi_hart_get_extensions_str(struct sbi_scratch *scratch,
> + char *extension_str, int nestr);
>
> void __attribute__((noreturn)) sbi_hart_hang(void);
>
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index fdfb6d9..956d185 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -29,7 +29,7 @@ void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
>
> struct hart_features {
> int priv_version;
> - unsigned long features;
> + unsigned long extensions;
> unsigned int pmp_count;
> unsigned int pmp_addr_bits;
> unsigned long pmp_gran;
> @@ -83,7 +83,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> for (cidx = 0; cidx < num_mhpm; cidx++) {
> #if __riscv_xlen == 32
> csr_write_num(CSR_MHPMEVENT3 + cidx, mhpmevent_init_val & 0xFFFFFFFF);
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> csr_write_num(CSR_MHPMEVENT3H + cidx,
> mhpmevent_init_val >> BITS_PER_LONG);
> #else
> @@ -91,7 +91,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> #endif
> }
>
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SMSTATEEN)) {
> + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMSTATEEN)) {
> mstateen_val = csr_read(CSR_MSTATEEN0);
> #if __riscv_xlen == 32
> mstateen_val |= ((uint64_t)csr_read(CSR_MSTATEEN0H)) << 32;
> @@ -99,7 +99,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> mstateen_val |= SMSTATEEN_STATEN;
> mstateen_val |= SMSTATEEN0_HSENVCFG;
>
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_AIA))
> + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_AIA))
> mstateen_val |= (SMSTATEEN0_AIA | SMSTATEEN0_SVSLCT |
> SMSTATEEN0_IMSIC);
> else
> @@ -153,7 +153,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> * Enable access to stimecmp if sstc extension is present in the
> * hardware.
> */
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSTC)) {
> + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSTC)) {
> #if __riscv_xlen == 32
> unsigned long menvcfgh_val;
> menvcfgh_val = csr_read(CSR_MENVCFGH);
> @@ -207,7 +207,7 @@ static int delegate_traps(struct sbi_scratch *scratch)
>
> /* Send M-mode interrupts and most exceptions to S-mode */
> interrupts = MIP_SSIP | MIP_STIP | MIP_SEIP;
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> interrupts |= MIP_LCOFIP;
>
> exceptions = (1U << CAUSE_MISALIGNED_FETCH) | (1U << CAUSE_BREAKPOINT) |
> @@ -367,102 +367,94 @@ void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> }
>
> /**
> - * Check whether a particular hart feature is available
> + * Check whether a particular hart extension is available
> *
> * @param scratch pointer to the HART scratch space
> - * @param feature the feature to check
> - * @returns true (feature available) or false (feature not available)
> + * @param ext the extension number to check
> + * @returns true (available) or false (not available)
> */
> -bool sbi_hart_has_feature(struct sbi_scratch *scratch, unsigned long feature)
> +bool sbi_hart_has_extension(struct sbi_scratch *scratch,
> + enum sbi_hart_extensions ext)
> {
> struct hart_features *hfeatures =
> sbi_scratch_offset_ptr(scratch, hart_features_offset);
>
> - if (hfeatures->features & feature)
> + if (hfeatures->extensions & BIT(ext))
> return true;
> else
> return false;
> }
>
> -static unsigned long hart_get_features(struct sbi_scratch *scratch)
> +static inline char *sbi_hart_extension_id2string(int ext)
> {
> - struct hart_features *hfeatures =
> - sbi_scratch_offset_ptr(scratch, hart_features_offset);
> -
> - return hfeatures->features;
> -}
> -
> -static inline char *sbi_hart_feature_id2string(unsigned long feature)
> -{
> - char *fstr = NULL;
> -
> - if (!feature)
> - return NULL;
> + char *estr = NULL;
>
> - switch (feature) {
> - case SBI_HART_HAS_SSCOFPMF:
> - fstr = "sscofpmf";
> + switch (ext) {
> + case SBI_HART_EXT_SSCOFPMF:
> + estr = "sscofpmf";
> break;
> - case SBI_HART_HAS_TIME:
> - fstr = "time";
> + case SBI_HART_EXT_TIME:
> + estr = "time";
> break;
> - case SBI_HART_HAS_AIA:
> - fstr = "aia";
> + case SBI_HART_EXT_AIA:
> + estr = "aia";
> break;
> - case SBI_HART_HAS_SSTC:
> - fstr = "sstc";
> + case SBI_HART_EXT_SSTC:
> + estr = "sstc";
> break;
> - case SBI_HART_HAS_SMSTATEEN:
> - fstr = "smstateen";
> + case SBI_HART_EXT_SMSTATEEN:
> + estr = "smstateen";
> break;
> default:
> break;
> }
>
> - return fstr;
> + return estr;
> }
>
> /**
> - * Get the hart features in string format
> + * Get the hart extensions in string format
> *
> * @param scratch pointer to the HART scratch space
> - * @param features_str pointer to a char array where the features string will be
> - * updated
> - * @param nfstr length of the features_str. The feature string will be truncated
> - * if nfstr is not long enough.
> + * @param extensions_str pointer to a char array where the extensions string
> + * will be updated
> + * @param nestr length of the features_str. The feature string will be
> + * truncated if nestr is not long enough.
> */
> -void sbi_hart_get_features_str(struct sbi_scratch *scratch,
> - char *features_str, int nfstr)
> +void sbi_hart_get_extensions_str(struct sbi_scratch *scratch,
> + char *extensions_str, int nestr)
> {
> - unsigned long features, feat = 1UL;
> + struct hart_features *hfeatures =
> + sbi_scratch_offset_ptr(scratch, hart_features_offset);
> + int offset = 0, ext = 0;
> char *temp;
> - int offset = 0;
>
> - if (!features_str || nfstr <= 0)
> + if (!extensions_str || nestr <= 0)
> return;
> - sbi_memset(features_str, 0, nfstr);
> + sbi_memset(extensions_str, 0, nestr);
>
> - features = hart_get_features(scratch);
> - if (!features)
> + if (!hfeatures->extensions)
> goto done;
>
> do {
> - if (features & feat) {
> - temp = sbi_hart_feature_id2string(feat);
> + if (hfeatures->extensions & BIT(ext)) {
> + temp = sbi_hart_extension_id2string(ext);
> if (temp) {
> - sbi_snprintf(features_str + offset, nfstr,
> + sbi_snprintf(extensions_str + offset,
> + nestr - offset,
> "%s,", temp);
> offset = offset + sbi_strlen(temp) + 1;
> }
> }
> - feat = feat << 1;
> - } while (feat <= SBI_HART_HAS_LAST_FEATURE);
> +
> + ext++;
> + } while (ext <= SBI_HART_EXT_MAX);
>
it should be ext < SBI_HART_EXT_MAX as SBI_HART_EXT_MAX is no longer
the last extension.
> done:
> if (offset)
> - features_str[offset - 1] = '\0';
> + extensions_str[offset - 1] = '\0';
> else
> - sbi_strncpy(features_str, "none", nfstr);
> + sbi_strncpy(extensions_str, "none", nestr);
> }
>
> static unsigned long hart_pmp_get_allowed_addr(void)
> @@ -523,7 +515,7 @@ static void hart_detect_features(struct sbi_scratch *scratch)
>
> /* Reset hart features */
> hfeatures = sbi_scratch_offset_ptr(scratch, hart_features_offset);
> - hfeatures->features = 0;
> + hfeatures->extensions = 0;
> hfeatures->pmp_count = 0;
> hfeatures->mhpm_count = 0;
>
> @@ -623,31 +615,31 @@ __mhpm_skip:
> /* Detect if hart supports sscofpmf */
> csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
> if (!trap.cause)
> - hfeatures->features |= SBI_HART_HAS_SSCOFPMF;
> + hfeatures->extensions |= BIT(SBI_HART_EXT_SSCOFPMF);
> }
>
> /* Detect if hart supports time CSR */
> csr_read_allowed(CSR_TIME, (unsigned long)&trap);
> if (!trap.cause)
> - hfeatures->features |= SBI_HART_HAS_TIME;
> + hfeatures->extensions |= BIT(SBI_HART_EXT_TIME);
>
> /* Detect if hart has AIA local interrupt CSRs */
> csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
> if (!trap.cause)
> - hfeatures->features |= SBI_HART_HAS_AIA;
> + hfeatures->extensions |= BIT(SBI_HART_EXT_AIA);
>
> /* Detect if hart supports stimecmp CSR(Sstc extension) */
> if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
> if (!trap.cause)
> - hfeatures->features |= SBI_HART_HAS_SSTC;
> + hfeatures->extensions |= BIT(SBI_HART_EXT_SSTC);
> }
>
> /* Detect if hart supports mstateen CSRs */
> if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
> if (!trap.cause)
> - hfeatures->features |= SBI_HART_HAS_SMSTATEEN;
> + hfeatures->extensions |= BIT(SBI_HART_EXT_SMSTATEEN);
> }
>
> return;
> diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> index c4def58..d57efa7 100644
> --- a/lib/sbi/sbi_init.c
> +++ b/lib/sbi/sbi_init.c
> @@ -143,8 +143,8 @@ static void sbi_boot_print_hart(struct sbi_scratch *scratch, u32 hartid)
> sbi_printf("Boot HART Priv Version : %s\n", str);
> misa_string(xlen, str, sizeof(str));
> sbi_printf("Boot HART Base ISA : %s\n", str);
> - sbi_hart_get_features_str(scratch, str, sizeof(str));
> - sbi_printf("Boot HART Features : %s\n", str);
> + sbi_hart_get_extensions_str(scratch, str, sizeof(str));
> + sbi_printf("Boot HART ISA Extensions : %s\n", str);
> sbi_printf("Boot HART PMP Count : %d\n",
> sbi_hart_pmp_count(scratch));
> sbi_printf("Boot HART PMP Granularity : %lu\n",
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 386bf4d..b60799e 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -313,7 +313,7 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
>
> __clear_bit(cidx, &mctr_inhbt);
>
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> pmu_ctr_enable_irq_hw(cidx);
> csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
>
> @@ -474,7 +474,7 @@ static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
> * Always set the OVF bit(disable interrupts) and inhibit counting of
> * events in M-mode. The OVF bit should be enabled during the start call.
> */
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> mhpmevent_val = (mhpmevent_val & ~MHPMEVENT_SSCOF_MASK) |
> MHPMEVENT_MINH | MHPMEVENT_OF;
>
> @@ -521,7 +521,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
> */
> fixed_ctr = pmu_ctr_find_fixed_fw(event_idx);
> if (fixed_ctr >= 0 &&
> - !sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> + !sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> return fixed_ctr;
>
> if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
> diff --git a/lib/sbi/sbi_timer.c b/lib/sbi/sbi_timer.c
> index bf0f375..353886f 100644
> --- a/lib/sbi/sbi_timer.c
> +++ b/lib/sbi/sbi_timer.c
> @@ -129,7 +129,7 @@ void sbi_timer_event_start(u64 next_event)
> * 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 (sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_SSTC)) {
> #if __riscv_xlen == 32
> csr_write(CSR_STIMECMP, next_event & 0xFFFFFFFF);
> csr_write(CSR_STIMECMPH, next_event >> 32);
> @@ -151,7 +151,7 @@ void sbi_timer_process(void)
> * directly without M-mode come in between. This function should
> * 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))
> + if (!sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_SSTC))
> csr_set(CSR_MIP, MIP_STIP);
> }
>
> @@ -180,7 +180,7 @@ int sbi_timer_init(struct sbi_scratch *scratch, bool cold_boot)
> if (!time_delta_off)
> return SBI_ENOMEM;
>
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_TIME))
> + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_TIME))
> get_time_val = get_ticks;
> } else {
> if (!time_delta_off)
> diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
> index 86bab6d..2c509e5 100644
> --- a/lib/sbi/sbi_trap.c
> +++ b/lib/sbi/sbi_trap.c
> @@ -272,8 +272,8 @@ struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs)
> }
>
> if (mcause & (1UL << (__riscv_xlen - 1))) {
> - if (sbi_hart_has_feature(sbi_scratch_thishart_ptr(),
> - SBI_HART_HAS_AIA))
> + if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
> + SBI_HART_EXT_AIA))
> rc = sbi_trap_aia_irq(regs, mcause);
> else
> rc = sbi_trap_nonaia_irq(regs, mcause);
> diff --git a/lib/utils/fdt/fdt_pmu.c b/lib/utils/fdt/fdt_pmu.c
> index a2b048f..8ba6b08 100644
> --- a/lib/utils/fdt/fdt_pmu.c
> +++ b/lib/utils/fdt/fdt_pmu.c
> @@ -53,7 +53,7 @@ int fdt_pmu_fixup(void *fdt)
> fdt_delprop(fdt, pmu_offset, "riscv,event-to-mhpmcounters");
> fdt_delprop(fdt, pmu_offset, "riscv,event-to-mhpmevent");
> fdt_delprop(fdt, pmu_offset, "riscv,raw-event-to-mhpmcounters");
> - if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> + if (!sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> fdt_delprop(fdt, pmu_offset, "interrupts-extended");
>
> return 0;
> --
> 2.34.1
>
Other than that, LGTM.
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 08/11] lib: sbi: Convert hart features into hart extensions
2022-05-04 2:00 ` Atish Patra
@ 2022-05-07 5:04 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 5:04 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:30 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > Since past few years, we have been using "hart features" in OpenSBI
> > to represent all optionalities and multi-letter extensions defined
> > by the RISC-V specifications.
> >
> > The RISC-V profiles specification has taken a different approach and
> > started assigning extension names for all optionalities which did not
> > have any extension name previously.
> > (Refer, https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc)
> >
> > Inspired from the RISC-V profiles specification, we convert OpenSBI
> > hart features into hart extensions. Going forward, we align the
> > extension naming with RISC-V profiles specification. Currently, only
> > "time CSR" and "AIA CSR" have not been assigned extension name but
> > for everything else we have a name.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_hart.h | 35 ++++++------
> > lib/sbi/sbi_hart.c | 116 +++++++++++++++++++---------------------
> > lib/sbi/sbi_init.c | 4 +-
> > lib/sbi/sbi_pmu.c | 6 +--
> > lib/sbi/sbi_timer.c | 6 +--
> > lib/sbi/sbi_trap.c | 4 +-
> > lib/utils/fdt/fdt_pmu.c | 2 +-
> > 7 files changed, 83 insertions(+), 90 deletions(-)
> >
> > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> > index c985674..8b21fd4 100644
> > --- a/include/sbi/sbi_hart.h
> > +++ b/include/sbi/sbi_hart.h
> > @@ -24,21 +24,21 @@ enum sbi_hart_priv_versions {
> > SBI_HART_PRIV_VER_1_12 = 3,
> > };
> >
> > -/** Possible feature flags of a hart */
> > -enum sbi_hart_features {
> > - /** Hart has sscofpmf extension */
> > - SBI_HART_HAS_SSCOFPMF = (1 << 0),
> > - /** HART has timer csr implementation in hardware */
> > - SBI_HART_HAS_TIME = (1 << 1),
> > - /** HART has AIA local interrupt CSRs */
> > - SBI_HART_HAS_AIA = (1 << 2),
> > - /** HART has mstateen CSR **/
> > - SBI_HART_HAS_SMSTATEEN = (1 << 3),
> > - /** HART has SSTC extension implemented in hardware */
> > - SBI_HART_HAS_SSTC = (1 << 4),
> > +/** Possible ISA extensions of a hart */
> > +enum sbi_hart_extensions {
> > + /** Hart has Sscofpmt extension */
> > + SBI_HART_EXT_SSCOFPMF = 0,
> > + /** HART has HW time CSR (extension name not available) */
> > + SBI_HART_EXT_TIME,
> > + /** HART has AIA CSRs (extension name not available) */
> > + SBI_HART_EXT_AIA,
> > + /** HART has Smstateen CSR **/
> > + SBI_HART_EXT_SMSTATEEN,
> > + /** HART has Sstc extension */
> > + SBI_HART_EXT_SSTC,
> >
> > - /** Last index of Hart features*/
> > - SBI_HART_HAS_LAST_FEATURE = SBI_HART_HAS_SSTC,
> > + /** Maximum index of Hart extension */
> > + SBI_HART_EXT_MAX,
> > };
> >
> > struct sbi_scratch;
> > @@ -63,9 +63,10 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
> > int sbi_hart_priv_version(struct sbi_scratch *scratch);
> > void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> > char *version_str, int nvstr);
> > -bool sbi_hart_has_feature(struct sbi_scratch *scratch, unsigned long feature);
> > -void sbi_hart_get_features_str(struct sbi_scratch *scratch,
> > - char *features_str, int nfstr);
> > +bool sbi_hart_has_extension(struct sbi_scratch *scratch,
> > + enum sbi_hart_extensions ext);
> > +void sbi_hart_get_extensions_str(struct sbi_scratch *scratch,
> > + char *extension_str, int nestr);
> >
> > void __attribute__((noreturn)) sbi_hart_hang(void);
> >
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index fdfb6d9..956d185 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -29,7 +29,7 @@ void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
> >
> > struct hart_features {
> > int priv_version;
> > - unsigned long features;
> > + unsigned long extensions;
> > unsigned int pmp_count;
> > unsigned int pmp_addr_bits;
> > unsigned long pmp_gran;
> > @@ -83,7 +83,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> > for (cidx = 0; cidx < num_mhpm; cidx++) {
> > #if __riscv_xlen == 32
> > csr_write_num(CSR_MHPMEVENT3 + cidx, mhpmevent_init_val & 0xFFFFFFFF);
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> > + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> > csr_write_num(CSR_MHPMEVENT3H + cidx,
> > mhpmevent_init_val >> BITS_PER_LONG);
> > #else
> > @@ -91,7 +91,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> > #endif
> > }
> >
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SMSTATEEN)) {
> > + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SMSTATEEN)) {
> > mstateen_val = csr_read(CSR_MSTATEEN0);
> > #if __riscv_xlen == 32
> > mstateen_val |= ((uint64_t)csr_read(CSR_MSTATEEN0H)) << 32;
> > @@ -99,7 +99,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> > mstateen_val |= SMSTATEEN_STATEN;
> > mstateen_val |= SMSTATEEN0_HSENVCFG;
> >
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_AIA))
> > + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_AIA))
> > mstateen_val |= (SMSTATEEN0_AIA | SMSTATEEN0_SVSLCT |
> > SMSTATEEN0_IMSIC);
> > else
> > @@ -153,7 +153,7 @@ static void mstatus_init(struct sbi_scratch *scratch)
> > * Enable access to stimecmp if sstc extension is present in the
> > * hardware.
> > */
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSTC)) {
> > + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSTC)) {
> > #if __riscv_xlen == 32
> > unsigned long menvcfgh_val;
> > menvcfgh_val = csr_read(CSR_MENVCFGH);
> > @@ -207,7 +207,7 @@ static int delegate_traps(struct sbi_scratch *scratch)
> >
> > /* Send M-mode interrupts and most exceptions to S-mode */
> > interrupts = MIP_SSIP | MIP_STIP | MIP_SEIP;
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> > + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> > interrupts |= MIP_LCOFIP;
> >
> > exceptions = (1U << CAUSE_MISALIGNED_FETCH) | (1U << CAUSE_BREAKPOINT) |
> > @@ -367,102 +367,94 @@ void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> > }
> >
> > /**
> > - * Check whether a particular hart feature is available
> > + * Check whether a particular hart extension is available
> > *
> > * @param scratch pointer to the HART scratch space
> > - * @param feature the feature to check
> > - * @returns true (feature available) or false (feature not available)
> > + * @param ext the extension number to check
> > + * @returns true (available) or false (not available)
> > */
> > -bool sbi_hart_has_feature(struct sbi_scratch *scratch, unsigned long feature)
> > +bool sbi_hart_has_extension(struct sbi_scratch *scratch,
> > + enum sbi_hart_extensions ext)
> > {
> > struct hart_features *hfeatures =
> > sbi_scratch_offset_ptr(scratch, hart_features_offset);
> >
> > - if (hfeatures->features & feature)
> > + if (hfeatures->extensions & BIT(ext))
> > return true;
> > else
> > return false;
> > }
> >
> > -static unsigned long hart_get_features(struct sbi_scratch *scratch)
> > +static inline char *sbi_hart_extension_id2string(int ext)
> > {
> > - struct hart_features *hfeatures =
> > - sbi_scratch_offset_ptr(scratch, hart_features_offset);
> > -
> > - return hfeatures->features;
> > -}
> > -
> > -static inline char *sbi_hart_feature_id2string(unsigned long feature)
> > -{
> > - char *fstr = NULL;
> > -
> > - if (!feature)
> > - return NULL;
> > + char *estr = NULL;
> >
> > - switch (feature) {
> > - case SBI_HART_HAS_SSCOFPMF:
> > - fstr = "sscofpmf";
> > + switch (ext) {
> > + case SBI_HART_EXT_SSCOFPMF:
> > + estr = "sscofpmf";
> > break;
> > - case SBI_HART_HAS_TIME:
> > - fstr = "time";
> > + case SBI_HART_EXT_TIME:
> > + estr = "time";
> > break;
> > - case SBI_HART_HAS_AIA:
> > - fstr = "aia";
> > + case SBI_HART_EXT_AIA:
> > + estr = "aia";
> > break;
> > - case SBI_HART_HAS_SSTC:
> > - fstr = "sstc";
> > + case SBI_HART_EXT_SSTC:
> > + estr = "sstc";
> > break;
> > - case SBI_HART_HAS_SMSTATEEN:
> > - fstr = "smstateen";
> > + case SBI_HART_EXT_SMSTATEEN:
> > + estr = "smstateen";
> > break;
> > default:
> > break;
> > }
> >
> > - return fstr;
> > + return estr;
> > }
> >
> > /**
> > - * Get the hart features in string format
> > + * Get the hart extensions in string format
> > *
> > * @param scratch pointer to the HART scratch space
> > - * @param features_str pointer to a char array where the features string will be
> > - * updated
> > - * @param nfstr length of the features_str. The feature string will be truncated
> > - * if nfstr is not long enough.
> > + * @param extensions_str pointer to a char array where the extensions string
> > + * will be updated
> > + * @param nestr length of the features_str. The feature string will be
> > + * truncated if nestr is not long enough.
> > */
> > -void sbi_hart_get_features_str(struct sbi_scratch *scratch,
> > - char *features_str, int nfstr)
> > +void sbi_hart_get_extensions_str(struct sbi_scratch *scratch,
> > + char *extensions_str, int nestr)
> > {
> > - unsigned long features, feat = 1UL;
> > + struct hart_features *hfeatures =
> > + sbi_scratch_offset_ptr(scratch, hart_features_offset);
> > + int offset = 0, ext = 0;
> > char *temp;
> > - int offset = 0;
> >
> > - if (!features_str || nfstr <= 0)
> > + if (!extensions_str || nestr <= 0)
> > return;
> > - sbi_memset(features_str, 0, nfstr);
> > + sbi_memset(extensions_str, 0, nestr);
> >
> > - features = hart_get_features(scratch);
> > - if (!features)
> > + if (!hfeatures->extensions)
> > goto done;
> >
> > do {
> > - if (features & feat) {
> > - temp = sbi_hart_feature_id2string(feat);
> > + if (hfeatures->extensions & BIT(ext)) {
> > + temp = sbi_hart_extension_id2string(ext);
> > if (temp) {
> > - sbi_snprintf(features_str + offset, nfstr,
> > + sbi_snprintf(extensions_str + offset,
> > + nestr - offset,
> > "%s,", temp);
> > offset = offset + sbi_strlen(temp) + 1;
> > }
> > }
> > - feat = feat << 1;
> > - } while (feat <= SBI_HART_HAS_LAST_FEATURE);
> > +
> > + ext++;
> > + } while (ext <= SBI_HART_EXT_MAX);
> >
>
> it should be ext < SBI_HART_EXT_MAX as SBI_HART_EXT_MAX is no longer
> the last extension.
Good catch. I have fixed it at the time of merging this patch.
>
> > done:
> > if (offset)
> > - features_str[offset - 1] = '\0';
> > + extensions_str[offset - 1] = '\0';
> > else
> > - sbi_strncpy(features_str, "none", nfstr);
> > + sbi_strncpy(extensions_str, "none", nestr);
> > }
> >
> > static unsigned long hart_pmp_get_allowed_addr(void)
> > @@ -523,7 +515,7 @@ static void hart_detect_features(struct sbi_scratch *scratch)
> >
> > /* Reset hart features */
> > hfeatures = sbi_scratch_offset_ptr(scratch, hart_features_offset);
> > - hfeatures->features = 0;
> > + hfeatures->extensions = 0;
> > hfeatures->pmp_count = 0;
> > hfeatures->mhpm_count = 0;
> >
> > @@ -623,31 +615,31 @@ __mhpm_skip:
> > /* Detect if hart supports sscofpmf */
> > csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
> > if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_SSCOFPMF;
> > + hfeatures->extensions |= BIT(SBI_HART_EXT_SSCOFPMF);
> > }
> >
> > /* Detect if hart supports time CSR */
> > csr_read_allowed(CSR_TIME, (unsigned long)&trap);
> > if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_TIME;
> > + hfeatures->extensions |= BIT(SBI_HART_EXT_TIME);
> >
> > /* Detect if hart has AIA local interrupt CSRs */
> > csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
> > if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_AIA;
> > + hfeatures->extensions |= BIT(SBI_HART_EXT_AIA);
> >
> > /* Detect if hart supports stimecmp CSR(Sstc extension) */
> > if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> > csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
> > if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_SSTC;
> > + hfeatures->extensions |= BIT(SBI_HART_EXT_SSTC);
> > }
> >
> > /* Detect if hart supports mstateen CSRs */
> > if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> > val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
> > if (!trap.cause)
> > - hfeatures->features |= SBI_HART_HAS_SMSTATEEN;
> > + hfeatures->extensions |= BIT(SBI_HART_EXT_SMSTATEEN);
> > }
> >
> > return;
> > diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> > index c4def58..d57efa7 100644
> > --- a/lib/sbi/sbi_init.c
> > +++ b/lib/sbi/sbi_init.c
> > @@ -143,8 +143,8 @@ static void sbi_boot_print_hart(struct sbi_scratch *scratch, u32 hartid)
> > sbi_printf("Boot HART Priv Version : %s\n", str);
> > misa_string(xlen, str, sizeof(str));
> > sbi_printf("Boot HART Base ISA : %s\n", str);
> > - sbi_hart_get_features_str(scratch, str, sizeof(str));
> > - sbi_printf("Boot HART Features : %s\n", str);
> > + sbi_hart_get_extensions_str(scratch, str, sizeof(str));
> > + sbi_printf("Boot HART ISA Extensions : %s\n", str);
> > sbi_printf("Boot HART PMP Count : %d\n",
> > sbi_hart_pmp_count(scratch));
> > sbi_printf("Boot HART PMP Granularity : %lu\n",
> > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> > index 386bf4d..b60799e 100644
> > --- a/lib/sbi/sbi_pmu.c
> > +++ b/lib/sbi/sbi_pmu.c
> > @@ -313,7 +313,7 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
> >
> > __clear_bit(cidx, &mctr_inhbt);
> >
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> > + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> > pmu_ctr_enable_irq_hw(cidx);
> > csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
> >
> > @@ -474,7 +474,7 @@ static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
> > * Always set the OVF bit(disable interrupts) and inhibit counting of
> > * events in M-mode. The OVF bit should be enabled during the start call.
> > */
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> > + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> > mhpmevent_val = (mhpmevent_val & ~MHPMEVENT_SSCOF_MASK) |
> > MHPMEVENT_MINH | MHPMEVENT_OF;
> >
> > @@ -521,7 +521,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
> > */
> > fixed_ctr = pmu_ctr_find_fixed_fw(event_idx);
> > if (fixed_ctr >= 0 &&
> > - !sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> > + !sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> > return fixed_ctr;
> >
> > if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_11)
> > diff --git a/lib/sbi/sbi_timer.c b/lib/sbi/sbi_timer.c
> > index bf0f375..353886f 100644
> > --- a/lib/sbi/sbi_timer.c
> > +++ b/lib/sbi/sbi_timer.c
> > @@ -129,7 +129,7 @@ void sbi_timer_event_start(u64 next_event)
> > * 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 (sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_SSTC)) {
> > #if __riscv_xlen == 32
> > csr_write(CSR_STIMECMP, next_event & 0xFFFFFFFF);
> > csr_write(CSR_STIMECMPH, next_event >> 32);
> > @@ -151,7 +151,7 @@ void sbi_timer_process(void)
> > * directly without M-mode come in between. This function should
> > * 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))
> > + if (!sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_SSTC))
> > csr_set(CSR_MIP, MIP_STIP);
> > }
> >
> > @@ -180,7 +180,7 @@ int sbi_timer_init(struct sbi_scratch *scratch, bool cold_boot)
> > if (!time_delta_off)
> > return SBI_ENOMEM;
> >
> > - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_TIME))
> > + if (sbi_hart_has_extension(scratch, SBI_HART_EXT_TIME))
> > get_time_val = get_ticks;
> > } else {
> > if (!time_delta_off)
> > diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
> > index 86bab6d..2c509e5 100644
> > --- a/lib/sbi/sbi_trap.c
> > +++ b/lib/sbi/sbi_trap.c
> > @@ -272,8 +272,8 @@ struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs)
> > }
> >
> > if (mcause & (1UL << (__riscv_xlen - 1))) {
> > - if (sbi_hart_has_feature(sbi_scratch_thishart_ptr(),
> > - SBI_HART_HAS_AIA))
> > + if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
> > + SBI_HART_EXT_AIA))
> > rc = sbi_trap_aia_irq(regs, mcause);
> > else
> > rc = sbi_trap_nonaia_irq(regs, mcause);
> > diff --git a/lib/utils/fdt/fdt_pmu.c b/lib/utils/fdt/fdt_pmu.c
> > index a2b048f..8ba6b08 100644
> > --- a/lib/utils/fdt/fdt_pmu.c
> > +++ b/lib/utils/fdt/fdt_pmu.c
> > @@ -53,7 +53,7 @@ int fdt_pmu_fixup(void *fdt)
> > fdt_delprop(fdt, pmu_offset, "riscv,event-to-mhpmcounters");
> > fdt_delprop(fdt, pmu_offset, "riscv,event-to-mhpmevent");
> > fdt_delprop(fdt, pmu_offset, "riscv,raw-event-to-mhpmcounters");
> > - if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOFPMF))
> > + if (!sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> > fdt_delprop(fdt, pmu_offset, "interrupts-extended");
> >
> > return 0;
> > --
> > 2.34.1
> >
>
> Other than that, LGTM.
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
>
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 09/11] lib: sbi: Detect hart features only once for each hart
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
` (7 preceding siblings ...)
2022-04-29 15:51 ` [PATCH 08/11] lib: sbi: Convert hart features into hart extensions Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 2:01 ` Atish Patra
2022-04-29 15:51 ` [PATCH 10/11] lib: sbi: Add sbi_hart_update_extension() function Anup Patel
` (2 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
Currently, the hart_detect_features() is called everytime a hart
is stopped and started again which is unnecessary work.
We update hart_detect_features() to detect hart features only
once for each hart.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
lib/sbi/sbi_hart.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 956d185..28a7e75 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -28,6 +28,7 @@ extern void __sbi_expected_trap_hext(void);
void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
struct hart_features {
+ bool detected;
int priv_version;
unsigned long extensions;
unsigned int pmp_count;
@@ -510,11 +511,15 @@ static int hart_pmu_get_allowed_bits(void)
static void hart_detect_features(struct sbi_scratch *scratch)
{
struct sbi_trap_info trap = {0};
- struct hart_features *hfeatures;
+ struct hart_features *hfeatures =
+ sbi_scratch_offset_ptr(scratch, hart_features_offset);
unsigned long val, oldval;
- /* Reset hart features */
- hfeatures = sbi_scratch_offset_ptr(scratch, hart_features_offset);
+ /* If hart features already detected then do nothing */
+ if (hfeatures->detected)
+ return;
+
+ /* Clear hart features */
hfeatures->extensions = 0;
hfeatures->pmp_count = 0;
hfeatures->mhpm_count = 0;
@@ -642,6 +647,9 @@ __mhpm_skip:
hfeatures->extensions |= BIT(SBI_HART_EXT_SMSTATEEN);
}
+ /* Mark hart feature detection done */
+ hfeatures->detected = true;
+
return;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 09/11] lib: sbi: Detect hart features only once for each hart
2022-04-29 15:51 ` [PATCH 09/11] lib: sbi: Detect hart features only once for each hart Anup Patel
@ 2022-05-04 2:01 ` Atish Patra
2022-05-07 5:04 ` Anup Patel
0 siblings, 1 reply; 37+ messages in thread
From: Atish Patra @ 2022-05-04 2:01 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Currently, the hart_detect_features() is called everytime a hart
> is stopped and started again which is unnecessary work.
>
> We update hart_detect_features() to detect hart features only
> once for each hart.
>
Great.
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> lib/sbi/sbi_hart.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 956d185..28a7e75 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -28,6 +28,7 @@ extern void __sbi_expected_trap_hext(void);
> void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
>
> struct hart_features {
> + bool detected;
> int priv_version;
> unsigned long extensions;
> unsigned int pmp_count;
> @@ -510,11 +511,15 @@ static int hart_pmu_get_allowed_bits(void)
> static void hart_detect_features(struct sbi_scratch *scratch)
> {
> struct sbi_trap_info trap = {0};
> - struct hart_features *hfeatures;
> + struct hart_features *hfeatures =
> + sbi_scratch_offset_ptr(scratch, hart_features_offset);
> unsigned long val, oldval;
>
> - /* Reset hart features */
> - hfeatures = sbi_scratch_offset_ptr(scratch, hart_features_offset);
> + /* If hart features already detected then do nothing */
> + if (hfeatures->detected)
> + return;
> +
> + /* Clear hart features */
> hfeatures->extensions = 0;
> hfeatures->pmp_count = 0;
> hfeatures->mhpm_count = 0;
> @@ -642,6 +647,9 @@ __mhpm_skip:
> hfeatures->extensions |= BIT(SBI_HART_EXT_SMSTATEEN);
> }
>
> + /* Mark hart feature detection done */
> + hfeatures->detected = true;
> +
> return;
> }
>
> --
> 2.34.1
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 09/11] lib: sbi: Detect hart features only once for each hart
2022-05-04 2:01 ` Atish Patra
@ 2022-05-07 5:04 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 5:04 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:31 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > Currently, the hart_detect_features() is called everytime a hart
> > is stopped and started again which is unnecessary work.
> >
> > We update hart_detect_features() to detect hart features only
> > once for each hart.
> >
>
> Great.
>
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > lib/sbi/sbi_hart.c | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 956d185..28a7e75 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -28,6 +28,7 @@ extern void __sbi_expected_trap_hext(void);
> > void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
> >
> > struct hart_features {
> > + bool detected;
> > int priv_version;
> > unsigned long extensions;
> > unsigned int pmp_count;
> > @@ -510,11 +511,15 @@ static int hart_pmu_get_allowed_bits(void)
> > static void hart_detect_features(struct sbi_scratch *scratch)
> > {
> > struct sbi_trap_info trap = {0};
> > - struct hart_features *hfeatures;
> > + struct hart_features *hfeatures =
> > + sbi_scratch_offset_ptr(scratch, hart_features_offset);
> > unsigned long val, oldval;
> >
> > - /* Reset hart features */
> > - hfeatures = sbi_scratch_offset_ptr(scratch, hart_features_offset);
> > + /* If hart features already detected then do nothing */
> > + if (hfeatures->detected)
> > + return;
> > +
> > + /* Clear hart features */
> > hfeatures->extensions = 0;
> > hfeatures->pmp_count = 0;
> > hfeatures->mhpm_count = 0;
> > @@ -642,6 +647,9 @@ __mhpm_skip:
> > hfeatures->extensions |= BIT(SBI_HART_EXT_SMSTATEEN);
> > }
> >
> > + /* Mark hart feature detection done */
> > + hfeatures->detected = true;
> > +
> > return;
> > }
> >
> > --
> > 2.34.1
> >
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
>
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 10/11] lib: sbi: Add sbi_hart_update_extension() function
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
` (8 preceding siblings ...)
2022-04-29 15:51 ` [PATCH 09/11] lib: sbi: Detect hart features only once for each hart Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 2:02 ` Atish Patra
2022-04-29 15:51 ` [PATCH 11/11] lib: sbi_platform: Add callback to populate HART extensions Anup Patel
2022-05-05 16:30 ` [PATCH 00/11] HART Feature Improvements Xiang W
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
We add sbi_hart_update_extension() function which allow platforms
to enable/disable hart extensions.
Signed-off-by: Anup Patel <anup@brainfault.org>
---
include/sbi/sbi_hart.h | 3 +++
lib/sbi/sbi_hart.c | 43 +++++++++++++++++++++++++++++++++++++-----
2 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index 8b21fd4..4661506 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -63,6 +63,9 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
int sbi_hart_priv_version(struct sbi_scratch *scratch);
void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
char *version_str, int nvstr);
+void sbi_hart_update_extension(struct sbi_scratch *scratch,
+ enum sbi_hart_extensions ext,
+ bool enable);
bool sbi_hart_has_extension(struct sbi_scratch *scratch,
enum sbi_hart_extensions ext);
void sbi_hart_get_extensions_str(struct sbi_scratch *scratch,
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 28a7e75..3fc8899 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -367,6 +367,34 @@ void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
sbi_snprintf(version_str, nvstr, "%s", temp);
}
+static inline void __sbi_hart_update_extension(
+ struct hart_features *hfeatures,
+ enum sbi_hart_extensions ext,
+ bool enable)
+{
+ if (enable)
+ hfeatures->extensions |= BIT(ext);
+ else
+ hfeatures->extensions &= ~BIT(ext);
+}
+
+/**
+ * Enable/Disable a particular hart extension
+ *
+ * @param scratch pointer to the HART scratch space
+ * @param ext the extension number to check
+ * @param enable new state of hart extension
+ */
+void sbi_hart_update_extension(struct sbi_scratch *scratch,
+ enum sbi_hart_extensions ext,
+ bool enable)
+{
+ struct hart_features *hfeatures =
+ sbi_scratch_offset_ptr(scratch, hart_features_offset);
+
+ __sbi_hart_update_extension(hfeatures, ext, enable);
+}
+
/**
* Check whether a particular hart extension is available
*
@@ -620,31 +648,36 @@ __mhpm_skip:
/* Detect if hart supports sscofpmf */
csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
if (!trap.cause)
- hfeatures->extensions |= BIT(SBI_HART_EXT_SSCOFPMF);
+ __sbi_hart_update_extension(hfeatures,
+ SBI_HART_EXT_SSCOFPMF, true);
}
/* Detect if hart supports time CSR */
csr_read_allowed(CSR_TIME, (unsigned long)&trap);
if (!trap.cause)
- hfeatures->extensions |= BIT(SBI_HART_EXT_TIME);
+ __sbi_hart_update_extension(hfeatures,
+ SBI_HART_EXT_TIME, true);
/* Detect if hart has AIA local interrupt CSRs */
csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
if (!trap.cause)
- hfeatures->extensions |= BIT(SBI_HART_EXT_AIA);
+ __sbi_hart_update_extension(hfeatures,
+ SBI_HART_EXT_AIA, true);
/* Detect if hart supports stimecmp CSR(Sstc extension) */
if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
if (!trap.cause)
- hfeatures->extensions |= BIT(SBI_HART_EXT_SSTC);
+ __sbi_hart_update_extension(hfeatures,
+ SBI_HART_EXT_SSTC, true);
}
/* Detect if hart supports mstateen CSRs */
if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
if (!trap.cause)
- hfeatures->extensions |= BIT(SBI_HART_EXT_SMSTATEEN);
+ __sbi_hart_update_extension(hfeatures,
+ SBI_HART_EXT_SMSTATEEN, true);
}
/* Mark hart feature detection done */
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 10/11] lib: sbi: Add sbi_hart_update_extension() function
2022-04-29 15:51 ` [PATCH 10/11] lib: sbi: Add sbi_hart_update_extension() function Anup Patel
@ 2022-05-04 2:02 ` Atish Patra
2022-05-07 5:04 ` Anup Patel
0 siblings, 1 reply; 37+ messages in thread
From: Atish Patra @ 2022-05-04 2:02 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> We add sbi_hart_update_extension() function which allow platforms
> to enable/disable hart extensions.
>
> Signed-off-by: Anup Patel <anup@brainfault.org>
> ---
> include/sbi/sbi_hart.h | 3 +++
> lib/sbi/sbi_hart.c | 43 +++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 41 insertions(+), 5 deletions(-)
>
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index 8b21fd4..4661506 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -63,6 +63,9 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
> int sbi_hart_priv_version(struct sbi_scratch *scratch);
> void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> char *version_str, int nvstr);
> +void sbi_hart_update_extension(struct sbi_scratch *scratch,
> + enum sbi_hart_extensions ext,
> + bool enable);
> bool sbi_hart_has_extension(struct sbi_scratch *scratch,
> enum sbi_hart_extensions ext);
> void sbi_hart_get_extensions_str(struct sbi_scratch *scratch,
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 28a7e75..3fc8899 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -367,6 +367,34 @@ void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> sbi_snprintf(version_str, nvstr, "%s", temp);
> }
>
> +static inline void __sbi_hart_update_extension(
> + struct hart_features *hfeatures,
> + enum sbi_hart_extensions ext,
> + bool enable)
> +{
> + if (enable)
> + hfeatures->extensions |= BIT(ext);
> + else
> + hfeatures->extensions &= ~BIT(ext);
> +}
> +
> +/**
> + * Enable/Disable a particular hart extension
> + *
> + * @param scratch pointer to the HART scratch space
> + * @param ext the extension number to check
> + * @param enable new state of hart extension
> + */
> +void sbi_hart_update_extension(struct sbi_scratch *scratch,
> + enum sbi_hart_extensions ext,
> + bool enable)
> +{
> + struct hart_features *hfeatures =
> + sbi_scratch_offset_ptr(scratch, hart_features_offset);
> +
> + __sbi_hart_update_extension(hfeatures, ext, enable);
> +}
> +
> /**
> * Check whether a particular hart extension is available
> *
> @@ -620,31 +648,36 @@ __mhpm_skip:
> /* Detect if hart supports sscofpmf */
> csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
> if (!trap.cause)
> - hfeatures->extensions |= BIT(SBI_HART_EXT_SSCOFPMF);
> + __sbi_hart_update_extension(hfeatures,
> + SBI_HART_EXT_SSCOFPMF, true);
> }
>
> /* Detect if hart supports time CSR */
> csr_read_allowed(CSR_TIME, (unsigned long)&trap);
> if (!trap.cause)
> - hfeatures->extensions |= BIT(SBI_HART_EXT_TIME);
> + __sbi_hart_update_extension(hfeatures,
> + SBI_HART_EXT_TIME, true);
>
> /* Detect if hart has AIA local interrupt CSRs */
> csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
> if (!trap.cause)
> - hfeatures->extensions |= BIT(SBI_HART_EXT_AIA);
> + __sbi_hart_update_extension(hfeatures,
> + SBI_HART_EXT_AIA, true);
>
> /* Detect if hart supports stimecmp CSR(Sstc extension) */
> if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
> if (!trap.cause)
> - hfeatures->extensions |= BIT(SBI_HART_EXT_SSTC);
> + __sbi_hart_update_extension(hfeatures,
> + SBI_HART_EXT_SSTC, true);
> }
>
> /* Detect if hart supports mstateen CSRs */
> if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
> if (!trap.cause)
> - hfeatures->extensions |= BIT(SBI_HART_EXT_SMSTATEEN);
> + __sbi_hart_update_extension(hfeatures,
> + SBI_HART_EXT_SMSTATEEN, true);
> }
>
> /* Mark hart feature detection done */
> --
> 2.34.1
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 10/11] lib: sbi: Add sbi_hart_update_extension() function
2022-05-04 2:02 ` Atish Patra
@ 2022-05-07 5:04 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 5:04 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:33 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > We add sbi_hart_update_extension() function which allow platforms
> > to enable/disable hart extensions.
> >
> > Signed-off-by: Anup Patel <anup@brainfault.org>
> > ---
> > include/sbi/sbi_hart.h | 3 +++
> > lib/sbi/sbi_hart.c | 43 +++++++++++++++++++++++++++++++++++++-----
> > 2 files changed, 41 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> > index 8b21fd4..4661506 100644
> > --- a/include/sbi/sbi_hart.h
> > +++ b/include/sbi/sbi_hart.h
> > @@ -63,6 +63,9 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
> > int sbi_hart_priv_version(struct sbi_scratch *scratch);
> > void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> > char *version_str, int nvstr);
> > +void sbi_hart_update_extension(struct sbi_scratch *scratch,
> > + enum sbi_hart_extensions ext,
> > + bool enable);
> > bool sbi_hart_has_extension(struct sbi_scratch *scratch,
> > enum sbi_hart_extensions ext);
> > void sbi_hart_get_extensions_str(struct sbi_scratch *scratch,
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 28a7e75..3fc8899 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -367,6 +367,34 @@ void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch,
> > sbi_snprintf(version_str, nvstr, "%s", temp);
> > }
> >
> > +static inline void __sbi_hart_update_extension(
> > + struct hart_features *hfeatures,
> > + enum sbi_hart_extensions ext,
> > + bool enable)
> > +{
> > + if (enable)
> > + hfeatures->extensions |= BIT(ext);
> > + else
> > + hfeatures->extensions &= ~BIT(ext);
> > +}
> > +
> > +/**
> > + * Enable/Disable a particular hart extension
> > + *
> > + * @param scratch pointer to the HART scratch space
> > + * @param ext the extension number to check
> > + * @param enable new state of hart extension
> > + */
> > +void sbi_hart_update_extension(struct sbi_scratch *scratch,
> > + enum sbi_hart_extensions ext,
> > + bool enable)
> > +{
> > + struct hart_features *hfeatures =
> > + sbi_scratch_offset_ptr(scratch, hart_features_offset);
> > +
> > + __sbi_hart_update_extension(hfeatures, ext, enable);
> > +}
> > +
> > /**
> > * Check whether a particular hart extension is available
> > *
> > @@ -620,31 +648,36 @@ __mhpm_skip:
> > /* Detect if hart supports sscofpmf */
> > csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
> > if (!trap.cause)
> > - hfeatures->extensions |= BIT(SBI_HART_EXT_SSCOFPMF);
> > + __sbi_hart_update_extension(hfeatures,
> > + SBI_HART_EXT_SSCOFPMF, true);
> > }
> >
> > /* Detect if hart supports time CSR */
> > csr_read_allowed(CSR_TIME, (unsigned long)&trap);
> > if (!trap.cause)
> > - hfeatures->extensions |= BIT(SBI_HART_EXT_TIME);
> > + __sbi_hart_update_extension(hfeatures,
> > + SBI_HART_EXT_TIME, true);
> >
> > /* Detect if hart has AIA local interrupt CSRs */
> > csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
> > if (!trap.cause)
> > - hfeatures->extensions |= BIT(SBI_HART_EXT_AIA);
> > + __sbi_hart_update_extension(hfeatures,
> > + SBI_HART_EXT_AIA, true);
> >
> > /* Detect if hart supports stimecmp CSR(Sstc extension) */
> > if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> > csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
> > if (!trap.cause)
> > - hfeatures->extensions |= BIT(SBI_HART_EXT_SSTC);
> > + __sbi_hart_update_extension(hfeatures,
> > + SBI_HART_EXT_SSTC, true);
> > }
> >
> > /* Detect if hart supports mstateen CSRs */
> > if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> > val = csr_read_allowed(CSR_MSTATEEN0, (unsigned long)&trap);
> > if (!trap.cause)
> > - hfeatures->extensions |= BIT(SBI_HART_EXT_SMSTATEEN);
> > + __sbi_hart_update_extension(hfeatures,
> > + SBI_HART_EXT_SMSTATEEN, true);
> > }
> >
> > /* Mark hart feature detection done */
> > --
> > 2.34.1
> >
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
>
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 11/11] lib: sbi_platform: Add callback to populate HART extensions
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
` (9 preceding siblings ...)
2022-04-29 15:51 ` [PATCH 10/11] lib: sbi: Add sbi_hart_update_extension() function Anup Patel
@ 2022-04-29 15:51 ` Anup Patel
2022-05-04 2:05 ` Atish Patra
2022-05-05 16:30 ` [PATCH 00/11] HART Feature Improvements Xiang W
11 siblings, 1 reply; 37+ messages in thread
From: Anup Patel @ 2022-04-29 15:51 UTC (permalink / raw)
To: opensbi
We add platform specific extensions_init() callback which allows
platforms to populate HART extensions for each HART. For example,
the generic platform can populate HART extensions from HART ISA
string described in DeviceTree.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_platform.h | 18 ++++++++++++++++++
lib/sbi/sbi_hart.c | 18 ++++++++++++++----
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
index 2c777ac..87024db 100644
--- a/include/sbi/sbi_platform.h
+++ b/include/sbi/sbi_platform.h
@@ -89,6 +89,9 @@ struct sbi_platform_operations {
*/
int (*misa_get_xlen)(void);
+ /** Initialize (or populate) HART extensions for the platform */
+ int (*extensions_init)(void);
+
/** Initialize (or populate) domains for the platform */
int (*domains_init)(void);
@@ -453,6 +456,21 @@ static inline int sbi_platform_misa_xlen(const struct sbi_platform *plat)
return -1;
}
+/**
+ * Initialize (or populate) HART extensions for the platform
+ *
+ * @param plat pointer to struct sbi_platform
+ *
+ * @return 0 on success and negative error code on failure
+ */
+static inline int sbi_platform_extensions_init(
+ const struct sbi_platform *plat)
+{
+ if (plat && sbi_platform_ops(plat)->extensions_init)
+ return sbi_platform_ops(plat)->extensions_init();
+ return 0;
+}
+
/**
* Initialize (or populate) domains for the platform
*
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 3fc8899..8284792 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -536,16 +536,17 @@ static int hart_pmu_get_allowed_bits(void)
return num_bits;
}
-static void hart_detect_features(struct sbi_scratch *scratch)
+static int hart_detect_features(struct sbi_scratch *scratch)
{
struct sbi_trap_info trap = {0};
struct hart_features *hfeatures =
sbi_scratch_offset_ptr(scratch, hart_features_offset);
unsigned long val, oldval;
+ int rc;
/* If hart features already detected then do nothing */
if (hfeatures->detected)
- return;
+ return 0;
/* Clear hart features */
hfeatures->extensions = 0;
@@ -680,10 +681,15 @@ __mhpm_skip:
SBI_HART_EXT_SMSTATEEN, true);
}
+ /* Let platform populate extensions */
+ rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr());
+ if (rc)
+ return rc;
+
/* Mark hart feature detection done */
hfeatures->detected = true;
- return;
+ return 0;
}
int sbi_hart_reinit(struct sbi_scratch *scratch)
@@ -705,6 +711,8 @@ int sbi_hart_reinit(struct sbi_scratch *scratch)
int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot)
{
+ int rc;
+
if (cold_boot) {
if (misa_extension('H'))
sbi_hart_expected_trap = &__sbi_expected_trap_hext;
@@ -715,7 +723,9 @@ int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot)
return SBI_ENOMEM;
}
- hart_detect_features(scratch);
+ rc = hart_detect_features(scratch);
+ if (rc)
+ return rc;
return sbi_hart_reinit(scratch);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 11/11] lib: sbi_platform: Add callback to populate HART extensions
2022-04-29 15:51 ` [PATCH 11/11] lib: sbi_platform: Add callback to populate HART extensions Anup Patel
@ 2022-05-04 2:05 ` Atish Patra
2022-05-07 5:05 ` Anup Patel
0 siblings, 1 reply; 37+ messages in thread
From: Atish Patra @ 2022-05-04 2:05 UTC (permalink / raw)
To: opensbi
On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> We add platform specific extensions_init() callback which allows
> platforms to populate HART extensions for each HART. For example,
> the generic platform can populate HART extensions from HART ISA
> string described in DeviceTree.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_platform.h | 18 ++++++++++++++++++
> lib/sbi/sbi_hart.c | 18 ++++++++++++++----
> 2 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index 2c777ac..87024db 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -89,6 +89,9 @@ struct sbi_platform_operations {
> */
> int (*misa_get_xlen)(void);
>
> + /** Initialize (or populate) HART extensions for the platform */
> + int (*extensions_init)(void);
> +
> /** Initialize (or populate) domains for the platform */
> int (*domains_init)(void);
>
> @@ -453,6 +456,21 @@ static inline int sbi_platform_misa_xlen(const struct sbi_platform *plat)
> return -1;
> }
>
> +/**
> + * Initialize (or populate) HART extensions for the platform
> + *
> + * @param plat pointer to struct sbi_platform
> + *
> + * @return 0 on success and negative error code on failure
> + */
> +static inline int sbi_platform_extensions_init(
> + const struct sbi_platform *plat)
> +{
> + if (plat && sbi_platform_ops(plat)->extensions_init)
> + return sbi_platform_ops(plat)->extensions_init();
> + return 0;
> +}
> +
> /**
> * Initialize (or populate) domains for the platform
> *
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 3fc8899..8284792 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -536,16 +536,17 @@ static int hart_pmu_get_allowed_bits(void)
> return num_bits;
> }
>
> -static void hart_detect_features(struct sbi_scratch *scratch)
> +static int hart_detect_features(struct sbi_scratch *scratch)
> {
> struct sbi_trap_info trap = {0};
> struct hart_features *hfeatures =
> sbi_scratch_offset_ptr(scratch, hart_features_offset);
> unsigned long val, oldval;
> + int rc;
>
> /* If hart features already detected then do nothing */
> if (hfeatures->detected)
> - return;
> + return 0;
>
> /* Clear hart features */
> hfeatures->extensions = 0;
> @@ -680,10 +681,15 @@ __mhpm_skip:
> SBI_HART_EXT_SMSTATEEN, true);
> }
>
> + /* Let platform populate extensions */
> + rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr());
> + if (rc)
> + return rc;
> +
> /* Mark hart feature detection done */
> hfeatures->detected = true;
>
> - return;
> + return 0;
> }
>
> int sbi_hart_reinit(struct sbi_scratch *scratch)
> @@ -705,6 +711,8 @@ int sbi_hart_reinit(struct sbi_scratch *scratch)
>
> int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot)
> {
> + int rc;
> +
> if (cold_boot) {
> if (misa_extension('H'))
> sbi_hart_expected_trap = &__sbi_expected_trap_hext;
> @@ -715,7 +723,9 @@ int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot)
> return SBI_ENOMEM;
> }
>
> - hart_detect_features(scratch);
> + rc = hart_detect_features(scratch);
> + if (rc)
> + return rc;
>
> return sbi_hart_reinit(scratch);
> }
> --
> 2.34.1
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 11/11] lib: sbi_platform: Add callback to populate HART extensions
2022-05-04 2:05 ` Atish Patra
@ 2022-05-07 5:05 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-07 5:05 UTC (permalink / raw)
To: opensbi
On Wed, May 4, 2022 at 7:36 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Apr 29, 2022 at 8:52 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > We add platform specific extensions_init() callback which allows
> > platforms to populate HART extensions for each HART. For example,
> > the generic platform can populate HART extensions from HART ISA
> > string described in DeviceTree.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > include/sbi/sbi_platform.h | 18 ++++++++++++++++++
> > lib/sbi/sbi_hart.c | 18 ++++++++++++++----
> > 2 files changed, 32 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> > index 2c777ac..87024db 100644
> > --- a/include/sbi/sbi_platform.h
> > +++ b/include/sbi/sbi_platform.h
> > @@ -89,6 +89,9 @@ struct sbi_platform_operations {
> > */
> > int (*misa_get_xlen)(void);
> >
> > + /** Initialize (or populate) HART extensions for the platform */
> > + int (*extensions_init)(void);
> > +
>
>
> > /** Initialize (or populate) domains for the platform */
> > int (*domains_init)(void);
> >
> > @@ -453,6 +456,21 @@ static inline int sbi_platform_misa_xlen(const struct sbi_platform *plat)
> > return -1;
> > }
> >
> > +/**
> > + * Initialize (or populate) HART extensions for the platform
> > + *
> > + * @param plat pointer to struct sbi_platform
> > + *
> > + * @return 0 on success and negative error code on failure
> > + */
> > +static inline int sbi_platform_extensions_init(
> > + const struct sbi_platform *plat)
> > +{
> > + if (plat && sbi_platform_ops(plat)->extensions_init)
> > + return sbi_platform_ops(plat)->extensions_init();
> > + return 0;
> > +}
> > +
>
>
>
> > /**
> > * Initialize (or populate) domains for the platform
> > *
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 3fc8899..8284792 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -536,16 +536,17 @@ static int hart_pmu_get_allowed_bits(void)
> > return num_bits;
> > }
> >
> > -static void hart_detect_features(struct sbi_scratch *scratch)
> > +static int hart_detect_features(struct sbi_scratch *scratch)
> > {
> > struct sbi_trap_info trap = {0};
> > struct hart_features *hfeatures =
> > sbi_scratch_offset_ptr(scratch, hart_features_offset);
> > unsigned long val, oldval;
> > + int rc;
> >
> > /* If hart features already detected then do nothing */
> > if (hfeatures->detected)
> > - return;
> > + return 0;
> >
> > /* Clear hart features */
> > hfeatures->extensions = 0;
> > @@ -680,10 +681,15 @@ __mhpm_skip:
> > SBI_HART_EXT_SMSTATEEN, true);
> > }
> >
> > + /* Let platform populate extensions */
> > + rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr());
> > + if (rc)
> > + return rc;
> > +
> > /* Mark hart feature detection done */
> > hfeatures->detected = true;
> >
> > - return;
> > + return 0;
> > }
> >
> > int sbi_hart_reinit(struct sbi_scratch *scratch)
> > @@ -705,6 +711,8 @@ int sbi_hart_reinit(struct sbi_scratch *scratch)
> >
> > int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot)
> > {
> > + int rc;
> > +
> > if (cold_boot) {
> > if (misa_extension('H'))
> > sbi_hart_expected_trap = &__sbi_expected_trap_hext;
> > @@ -715,7 +723,9 @@ int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot)
> > return SBI_ENOMEM;
> > }
> >
> > - hart_detect_features(scratch);
> > + rc = hart_detect_features(scratch);
> > + if (rc)
> > + return rc;
> >
> > return sbi_hart_reinit(scratch);
> > }
> > --
> > 2.34.1
> >
>
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 00/11] HART Feature Improvements
2022-04-29 15:51 [PATCH 00/11] HART Feature Improvements Anup Patel
` (10 preceding siblings ...)
2022-04-29 15:51 ` [PATCH 11/11] lib: sbi_platform: Add callback to populate HART extensions Anup Patel
@ 2022-05-05 16:30 ` Xiang W
2022-05-06 3:21 ` Anup Patel
11 siblings, 1 reply; 37+ messages in thread
From: Xiang W @ 2022-05-05 16:30 UTC (permalink / raw)
To: opensbi
? 2022-04-29???? 21:21 +0530?Anup Patel???
> This series does following improvements to OpenSBI hart features:
> 1) Add per-HART priv spec version
Detecting the privileged spec version by CSR is not friendly to some
non-standard implementations. I recommend defining the privileged spec
version via a macro in the platform code's header file
Regards,
Xiang
> 2) Remove redundant per-HART features
> 3) Convert HART features into HART extensions
> 4) Platform callback to populate HART extensions
>
> These patches can also be found in the hart_feat_v1 brach at:
> https://github.com/avpatel/opensbi.git
>
> Anup Patel (11):
> ? lib: sbi: Detect and print privileged spec version
> ? lib: sbi: Remove 's' and 'u' from misa_string() output
> ? lib: sbi: Update the name of ISA string printed at boot time
> ? lib: sbi: Remove MCOUNTEREN and SCOUNTEREN hart features
> ? lib: sbi: Remove MCOUNTINHIBT hart feature
> ? lib: sbi: Remove MENVCFG hart feature
> ? lib: sbi: Fix AIA feature detection
> ? lib: sbi: Convert hart features into hart extensions
> ? lib: sbi: Detect hart features only once for each hart
> ? lib: sbi: Add sbi_hart_update_extension() function
> ? lib: sbi_platform: Add callback to populate HART extensions
>
> ?include/sbi/sbi_hart.h???? |? 61 ++++----
> ?include/sbi/sbi_platform.h |? 18 +++
> ?lib/sbi/riscv_asm.c??????? |?? 2 +-
> ?lib/sbi/sbi_emulate_csr.c? |?? 4 +-
> ?lib/sbi/sbi_hart.c???????? | 288 +++++++++++++++++++++----------------
> ?lib/sbi/sbi_init.c???????? |?? 8 +-
> ?lib/sbi/sbi_pmu.c????????? |? 19 +--
> ?lib/sbi/sbi_timer.c??????? |?? 6 +-
> ?lib/sbi/sbi_trap.c???????? |?? 4 +-
> ?lib/utils/fdt/fdt_pmu.c??? |?? 2 +-
> ?10 files changed, 246 insertions(+), 166 deletions(-)
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 00/11] HART Feature Improvements
2022-05-05 16:30 ` [PATCH 00/11] HART Feature Improvements Xiang W
@ 2022-05-06 3:21 ` Anup Patel
0 siblings, 0 replies; 37+ messages in thread
From: Anup Patel @ 2022-05-06 3:21 UTC (permalink / raw)
To: opensbi
On Thu, May 5, 2022 at 10:00 PM Xiang W <wxjstz@126.com> wrote:
>
> ? 2022-04-29???? 21:21 +0530?Anup Patel???
> > This series does following improvements to OpenSBI hart features:
> > 1) Add per-HART priv spec version
>
> Detecting the privileged spec version by CSR is not friendly to some
> non-standard implementations. I recommend defining the privileged spec
> version via a macro in the platform code's header file
I disagree. Hard coding priv spec version as macro in platform header
file because we support running OpenSBI generic firmware binaries on
multiple RISC-V platforms (i.e. same binary should run on RISC-V Priv
v1.10, v1.11 and v1.12).
To address this, we have PATCH10 which adds a platform callback for
platforms to enable/disable HART extension based on DT (or something
else). The same callback can also be used by platform to read priv spec
version from DT (or something else) and update it using some generic
library function.
Unfortunately, there is no standard way (not even in DT) to specify priv
spec version so first we need to at least propose ad DT binding to
describe Priv spec version. The RISC-V profiles spec is using a special
notation for Priv spec version and we can use that as wel (e.g. Priv v1.12
is represented as extension Ss1p12).
In any case, there we will be separate series for generic platform to
parse extensions and priv spec version from ISA string.
Regards,
Anup
>
> Regards,
> Xiang
>
> > 2) Remove redundant per-HART features
> > 3) Convert HART features into HART extensions
> > 4) Platform callback to populate HART extensions
> >
> > These patches can also be found in the hart_feat_v1 brach at:
> > https://github.com/avpatel/opensbi.git
> >
> > Anup Patel (11):
> > lib: sbi: Detect and print privileged spec version
>
> > lib: sbi: Remove 's' and 'u' from misa_string() output
> > lib: sbi: Update the name of ISA string printed at boot time
> > lib: sbi: Remove MCOUNTEREN and SCOUNTEREN hart features
> > lib: sbi: Remove MCOUNTINHIBT hart feature
> > lib: sbi: Remove MENVCFG hart feature
> > lib: sbi: Fix AIA feature detection
> > lib: sbi: Convert hart features into hart extensions
> > lib: sbi: Detect hart features only once for each hart
> > lib: sbi: Add sbi_hart_update_extension() function
> > lib: sbi_platform: Add callback to populate HART extensions
> >
> > include/sbi/sbi_hart.h | 61 ++++----
> > include/sbi/sbi_platform.h | 18 +++
> > lib/sbi/riscv_asm.c | 2 +-
> > lib/sbi/sbi_emulate_csr.c | 4 +-
> > lib/sbi/sbi_hart.c | 288 +++++++++++++++++++++----------------
> > lib/sbi/sbi_init.c | 8 +-
> > lib/sbi/sbi_pmu.c | 19 +--
> > lib/sbi/sbi_timer.c | 6 +-
> > lib/sbi/sbi_trap.c | 4 +-
> > lib/utils/fdt/fdt_pmu.c | 2 +-
> > 10 files changed, 246 insertions(+), 166 deletions(-)
> >
> > --
> > 2.34.1
> >
> >
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread