HART Feature Improvements

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 <[email protected]>

---
 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(-)

include/sbi/sbi_hart.h +15 -0

index 1d09374..3c37933 100644
@@ -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);

lib/sbi/sbi_hart.c +46 -6

index 229c14a..ed4e631 100644
@@ -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.

lib/sbi/sbi_init.c +2 -1

index 2b9e5ae..660b11f 100644
@@ -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

Quoted 3 lines · Anup Patel

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

Quoted 6 lines

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

Quoted 149 lines

Signed-off-by Anup Patel <[email protected]>

---
 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(-)

include/sbi/sbi_hart.h +15 -0

index 1d09374..3c37933 100644
@@ -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);

lib/sbi/sbi_hart.c +46 -6

index 229c14a..ed4e631 100644
@@ -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.

lib/sbi/sbi_init.c +2 -0

index 2b9e5ae..660b11f 100644
@@ -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);

Quoted 6 lines

lib/sbi/sbi_init.c +0 -1

        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 <[email protected]>

--
Regards,
Atish

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 <[email protected]>

---
 lib/sbi/riscv_asm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/sbi/riscv_asm.c +1 -2

index 5eab1ed..0ebf41d 100644
@@ -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

Quoted 20 lines · Anup Patel

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 <[email protected]>

---
 lib/sbi/riscv_asm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/sbi/riscv_asm.c +1 -1

index 5eab1ed..0ebf41d 100644
@@ -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";
 

Quoted 5 lines

lib/sbi/riscv_asm.c +0 -1

        if (!out)
                return;
--

2.34.1

Reviewed-by Atish Patra <[email protected]>

--
Regards,
Atish

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 <[email protected]>

---
 lib/sbi/sbi_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/sbi/sbi_init.c +1 -2

index 660b11f..c4def58 100644
@@ -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

Quoted 21 lines · Anup Patel

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 <[email protected]>

---
 lib/sbi/sbi_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/sbi/sbi_init.c +1 -1

index 660b11f..c4def58 100644
@@ -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);

Quoted 6 lines

lib/sbi/sbi_init.c +0 -1

        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 <[email protected]>

--
Regards,
Atish

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 <[email protected]>

---
 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(-)

include/sbi/sbi_hart.h +7 -11

index 3c37933..4665a62 100644
@@ -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,

lib/sbi/sbi_emulate_csr.c +2 -2

index dbb1755..aec9d3c 100644
@@ -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;

lib/sbi/sbi_hart.c +0 -0

index ed4e631..ed8a061 100644

lib/sbi/sbi_hart.c +3 -22

        /**

@@ -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);

lib/sbi/sbi_hart.c +0 -0

                /* Detect if hart supports sscofpmf */
                csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
                if (!trap.cause)

lib/sbi/sbi_pmu.c +2 -2

index 7ea0ca5..853229e 100644
@@ -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

Quoted 76 lines · Anup Patel

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 <[email protected]>

---
 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(-)

include/sbi/sbi_hart.h +7 -11

index 3c37933..4665a62 100644
@@ -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,

lib/sbi/sbi_emulate_csr.c +2 -2

index dbb1755..aec9d3c 100644
@@ -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;

lib/sbi/sbi_hart.c +0 -0

index ed4e631..ed8a061 100644

lib/sbi/sbi_hart.c static void mstatus_init(struct sbi_scratch *scratch) · line 63

Quoted 7 lines

lib/sbi/sbi_hart.c +1 -1

@@ -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 ?

Quoted 51 lines

lib/sbi/sbi_hart.c +3 -22

        /**

@@ -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:
 

lib/sbi/sbi_hart.c __mhpm_skip: · line 635

Quoted 4 lines

lib/sbi/sbi_hart.c +1 -1

        /* 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)

Quoted 19 lines

lib/sbi/sbi_hart.c +0 -0

                /* Detect if hart supports sscofpmf */
                csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
                if (!trap.cause)

lib/sbi/sbi_pmu.c +2 -2

index 7ea0ca5..853229e 100644
@@ -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);
 }
 
--

Quoted 2 lines

2.34.1

Other than that, LGTM.

Reviewed-by Atish Patra <[email protected]>

--
Regards,
Atish

Quoted 76 lines · Anup Patel

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 <[email protected]>

---
 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(-)

include/sbi/sbi_hart.h +7 -11

index 3c37933..4665a62 100644
@@ -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,

lib/sbi/sbi_emulate_csr.c +2 -2

index dbb1755..aec9d3c 100644
@@ -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;

lib/sbi/sbi_hart.c +0 -0

index ed4e631..ed8a061 100644

lib/sbi/sbi_hart.c static void mstatus_init(struct sbi_scratch *scratch) · line 63

Quoted 7 lines

lib/sbi/sbi_hart.c +1 -1

@@ -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 ?

Quoted 50 lines

lib/sbi/sbi_hart.c +3 -22

        /**

@@ -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);

lib/sbi/sbi_hart.c __mhpm_skip: · line 635

Quoted 5 lines

lib/sbi/sbi_hart.c +1 -1

@@ -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)

Quoted 17 lines

lib/sbi/sbi_hart.c +0 -0

                /* Detect if hart supports sscofpmf */
                csr_read_allowed(CSR_SCOUNTOVF, (unsigned long)&trap);
                if (!trap.cause)

lib/sbi/sbi_pmu.c +2 -1

index 7ea0ca5..853229e 100644
@@ -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);
 }
 

Quoted 3 lines

--
2.34.1

Other than that, LGTM.

Reviewed-by Atish Patra <[email protected]>

--
Regards,
Atish

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 <[email protected]>

---
 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(-)

include/sbi/sbi_hart.h +6 -8

index 4665a62..bc6b766 100644
@@ -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,

lib/sbi/sbi_hart.c +8 -15

index ed8a061..0fe88cb 100644
@@ -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)

lib/sbi/sbi_pmu.c +5 -6

index 853229e..386bf4d 100644
@@ -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

Quoted 148 lines · Anup Patel

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 <[email protected]>

---
 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(-)

include/sbi/sbi_hart.h +6 -8

index 4665a62..bc6b766 100644
@@ -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,

lib/sbi/sbi_hart.c +8 -15

index ed8a061..0fe88cb 100644
@@ -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)

lib/sbi/sbi_pmu.c +5 -5

index 853229e..386bf4d 100644
@@ -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);
 

Quoted 4 lines

lib/sbi/sbi_pmu.c +0 -1

        if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_10)
--

2.34.1

Reviewed-by Atish Patra <[email protected]>

--
Regards,
Atish

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 <[email protected]>

---
 include/sbi/sbi_hart.h |  6 ++----
 lib/sbi/sbi_hart.c     | 32 ++++++++++++--------------------
 2 files changed, 14 insertions(+), 24 deletions(-)

include/sbi/sbi_hart.h +2 -4

index bc6b766..c985674 100644
@@ -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,

lib/sbi/sbi_hart.c +12 -21

index 0fe88cb..5ee5ddd 100644
@@ -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

Quoted 108 lines · Anup Patel

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 <[email protected]>

---
 include/sbi/sbi_hart.h |  6 ++----
 lib/sbi/sbi_hart.c     | 32 ++++++++++++--------------------
 2 files changed, 14 insertions(+), 24 deletions(-)

include/sbi/sbi_hart.h +2 -4

index bc6b766..c985674 100644
@@ -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,

lib/sbi/sbi_hart.c +12 -20

index 0fe88cb..5ee5ddd 100644
@@ -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;
+       }
 

Quoted 5 lines

lib/sbi/sbi_hart.c +0 -1

        return;
 }
--

2.34.1

Reviewed-by Atish Patra <[email protected]>

--
Regards,
Atish

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 <[email protected]>

---
 lib/sbi/sbi_hart.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

lib/sbi/sbi_hart.c +3 -5

index 5ee5ddd..fdfb6d9 100644
@@ -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

Quoted 33 lines · Anup Patel

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 <[email protected]>

---
 lib/sbi/sbi_hart.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

lib/sbi/sbi_hart.c +3 -4

index 5ee5ddd..fdfb6d9 100644
@@ -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;
 

Quoted 5 lines

lib/sbi/sbi_hart.c +0 -1

        /* 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 <[email protected]>

--
Regards,
Atish

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 <[email protected]>

---
 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(-)

include/sbi/sbi_hart.h +18 -17

index c985674..8b21fd4 100644
@@ -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);
 

lib/sbi/sbi_hart.c +43 -52

index fdfb6d9..956d185 100644
@@ -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,

lib/sbi/sbi_hart.c +8 -8

 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;

lib/sbi/sbi_init.c +2 -2

index c4def58..d57efa7 100644
@@ -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",

lib/sbi/sbi_pmu.c +3 -3

index 386bf4d..b60799e 100644
@@ -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)

lib/sbi/sbi_timer.c +3 -3

index bf0f375..353886f 100644
@@ -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)

lib/sbi/sbi_trap.c +2 -2

index 86bab6d..2c509e5 100644
@@ -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);

lib/utils/fdt/fdt_pmu.c +1 -2

index a2b048f..8ba6b08 100644
@@ -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

Quoted 269 lines · Anup Patel

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 <[email protected]>

---
 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(-)

include/sbi/sbi_hart.h +18 -17

index c985674..8b21fd4 100644
@@ -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);
 

lib/sbi/sbi_hart.c +43 -53

index fdfb6d9..956d185 100644
@@ -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;

lib/sbi/sbi_hart.c void sbi_hart_get_priv_version_str(struct sbi_scratch *scratch, · line 452

Quoted 5 lines

lib/sbi/sbi_hart.c +3 -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.

Quoted 161 lines

lib/sbi/sbi_hart.c +8 -8

 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;

lib/sbi/sbi_init.c +2 -2

index c4def58..d57efa7 100644
@@ -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",

lib/sbi/sbi_pmu.c +3 -3

index 386bf4d..b60799e 100644
@@ -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)

lib/sbi/sbi_timer.c +3 -3

index bf0f375..353886f 100644
@@ -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)

lib/sbi/sbi_trap.c +2 -2

index 86bab6d..2c509e5 100644
@@ -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);

lib/utils/fdt/fdt_pmu.c +1 -1

index a2b048f..8ba6b08 100644
@@ -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");
 

Quoted 4 lines

lib/utils/fdt/fdt_pmu.c +0 -1

        return 0;
--

2.34.1

Other than that, LGTM.

Reviewed-by Atish Patra <[email protected]>

--
Regards,
Atish