Skip to contentSkip to conversation

Misc OpenSBI HART ISA extension improvements

PATCH Superseded Anup Patel 2 participants 11 messages Jul 5, 2023 – Jul 6, 2023

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Anup Patel

author
#0 lore raw

cover Misc OpenSBI HART ISA extension improvements

This series does assorted improvements to OpenSBI HART ISA extensions.

These patches can also be found in zicntr_zihpm_v1 branch at:
https://github.com/avpatel/opensbi.git

Anup Patel (4):
lib: sbi: Add Zicntr as a HART ISA extension
lib: sbi: Add Zihpm as a HART ISA extension
lib: sbi: Alphabetically sort HART ISA extensions
lib: sbi: Rename hart_pmu_get_allowed_bits() function

 include/sbi/sbi_hart.h | 12 +++++++-----
 lib/sbi/sbi_hart.c     | 28 +++++++++++++++++-----------
 lib/sbi/sbi_timer.c    |  2 +-
 3 files changed, 25 insertions(+), 17 deletions(-)
Signature · 1 line
2.34.1
author
#1 lore raw

PATCH 1/4 lib: sbi: Add Zicntr as a HART ISA extension

Recently ratified Zicntr ISA extension covers cycle, time and instret CSRs so we replace the "time" ISA extension with "zicntr" ISA extension in OpenSBI.

---
 include/sbi/sbi_hart.h | 4 ++--
 lib/sbi/sbi_hart.c     | 6 +++---
 lib/sbi/sbi_timer.c    | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)
include/sbi/sbi_hart.h +2 -2
index 95b40e7..d48940d 100644
@@ -28,8 +28,8 @@ enum sbi_hart_priv_versions {
 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 Zicntr extension (i.e. HW cycle, time & instret CSRs) */
+	SBI_HART_EXT_ZICNTR,
 	/** HART has AIA M-mode CSRs */
 	SBI_HART_EXT_SMAIA,
 	/** HART has Smstateen CSR **/
lib/sbi/sbi_hart.c +3 -3
index 6e52cbd..c470482 100644
@@ -441,8 +441,8 @@ static inline char *sbi_hart_extension_id2string(int ext)
 	case SBI_HART_EXT_SSCOFPMF:
 		estr = "sscofpmf";
 		break;
-	case SBI_HART_EXT_TIME:
-		estr = "time";
+	case SBI_HART_EXT_ZICNTR:
+		estr = "zicntr";
 		break;
 	case SBI_HART_EXT_SMAIA:
 		estr = "smaia";
@@ -676,7 +676,7 @@ __mhpm_skip:
 	csr_read_allowed(CSR_TIME, (unsigned long)&trap);
 	if (!trap.cause)
 		__sbi_hart_update_extension(hfeatures,
-					SBI_HART_EXT_TIME, true);
+					SBI_HART_EXT_ZICNTR, true);
 
 	/* Detect if hart has AIA local interrupt CSRs */
 	csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);
lib/sbi/sbi_timer.c +1 -1
index 4b24cbe..7b618de 100644
@@ -188,7 +188,7 @@ int sbi_timer_init(struct sbi_scratch *scratch, bool cold_boot)
 		if (!time_delta_off)
 			return SBI_ENOMEM;
 
-		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_TIME))
+		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR))
 			get_time_val = get_ticks;
 	} else {
 		if (!time_delta_off)
Signature · 1 line
2.34.1
#2 lore raw

PATCH 1/4 lib: sbi: Add Zicntr as a HART ISA extension

? 2023-07-05???? 17:28 +0530?Anup Patel???

Quoted 5 lines

Recently ratified Zicntr ISA extension covers cycle, time and instret CSRs so we replace the "time" ISA extension with "zicntr" ISA extension in OpenSBI.

Look good to me.

Quoted 62 lines

---

?include/sbi/sbi_hart.h | 4 ++--
?lib/sbi/sbi_hart.c???? | 6 +++---
?lib/sbi/sbi_timer.c??? | 2 +-

?3 files changed, 6 insertions(+), 6 deletions(-)

include/sbi/sbi_hart.h +0 -0
index 95b40e7..d48940d 100644
@@ -28,8 +28,8 @@ enum sbi_hart_priv_versions {
 ?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 Zicntr extension (i.e. HW cycle, time & instret CSRs) */ +???????SBI_HART_EXT_ZICNTR,
????????/** HART has AIA M-mode CSRs */ ????????SBI_HART_EXT_SMAIA,
????????/** HART has Smstateen CSR **/

lib/sbi/sbi_hart.c +0 -0
index 6e52cbd..c470482 100644
@@ -441,8 +441,8 @@ static inline char *sbi_hart_extension_id2string(int ext)

????????case SBI_HART_EXT_SSCOFPMF:
????????????????estr = "sscofpmf";
????????????????break;
-???????case SBI_HART_EXT_TIME:
-???????????????estr = "time";
+???????case SBI_HART_EXT_ZICNTR:
+???????????????estr = "zicntr";
????????????????break;
????????case SBI_HART_EXT_SMAIA:
????????????????estr = "smaia";

lib/sbi/sbi_hart.c +1 -1
@@ -676,7 +676,7 @@ __mhpm_skip:
 ????????csr_read_allowed(CSR_TIME, (unsigned long)&trap);
 ????????if (!trap.cause)
 ????????????????__sbi_hart_update_extension(hfeatures,
-???????????????????????????????????????SBI_HART_EXT_TIME, true);
+???????????????????????????????????????SBI_HART_EXT_ZICNTR, true);

?
????????/* Detect if hart has AIA local interrupt CSRs */ ????????csr_read_allowed(CSR_MTOPI, (unsigned long)&trap);

lib/sbi/sbi_timer.c +0 -0
index 4b24cbe..7b618de 100644
@@ -188,7 +188,7 @@ int sbi_timer_init(struct sbi_scratch *scratch, bool cold_boot)
 ????????????????if (!time_delta_off)
 ????????????????????????return SBI_ENOMEM;

?
-???????????????if (sbi_hart_has_extension(scratch, SBI_HART_EXT_TIME)) +???????????????if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR)) ????????????????????????get_time_val = get_ticks;
????????} else {
????????????????if (!time_delta_off)

Signature · 1 line
2.34.1
author
#3 lore raw

PATCH 2/4 lib: sbi: Add Zihpm as a HART ISA extension

In reply to Anup Patel

Recently ratified Zihpm ISA extension covers all [m]hpm* CSRs so we add Zihpm as a HART ISA extension in OpenSBI.

---
 include/sbi/sbi_hart.h | 2 ++
 lib/sbi/sbi_hart.c     | 6 ++++++
 2 files changed, 8 insertions(+)
include/sbi/sbi_hart.h +2 -0
index d48940d..938248f 100644
@@ -36,6 +36,8 @@ enum sbi_hart_extensions {
 	SBI_HART_EXT_SMSTATEEN,
 	/** HART has Sstc extension */
 	SBI_HART_EXT_SSTC,
+	/** HART has Zihpm extension */
+	SBI_HART_EXT_ZIHPM,
 
 	/** Maximum index of Hart extension */
 	SBI_HART_EXT_MAX,
lib/sbi/sbi_hart.c +6 -0
index c470482..435771c 100644
@@ -453,6 +453,9 @@ static inline char *sbi_hart_extension_id2string(int ext)
 	case SBI_HART_EXT_SMSTATEEN:
 		estr = "smstateen";
 		break;
+	case SBI_HART_EXT_ZIHPM:
+		estr = "zihpm";
+		break;
 	default:
 		break;
 	}
@@ -637,6 +640,9 @@ __pmp_skip:
 	 */
 
 __mhpm_skip:
+	if (hfeatures->mhpm_count)
+		__sbi_hart_update_extension(hfeatures,
+					SBI_HART_EXT_ZIHPM, true);
 
 #undef __check_csr_64
 #undef __check_csr_32
Signature · 1 line
2.34.1
#4 lore raw

PATCH 2/4 lib: sbi: Add Zihpm as a HART ISA extension

? 2023-07-05???? 17:28 +0530?Anup Patel???

Quoted 4 lines

Recently ratified Zihpm ISA extension covers all [m]hpm* CSRs so we add Zihpm as a HART ISA extension in OpenSBI.

Look good to me.

Quoted 46 lines

---
?include/sbi/sbi_hart.h | 2 ++
?lib/sbi/sbi_hart.c???? | 6 ++++++
?2 files changed, 8 insertions(+)

include/sbi/sbi_hart.h +0 -0
index d48940d..938248f 100644
@@ -36,6 +36,8 @@ enum sbi_hart_extensions {
 ????????SBI_HART_EXT_SMSTATEEN,

????????/** HART has Sstc extension */
????????SBI_HART_EXT_SSTC,
+???????/** HART has Zihpm extension */
+???????SBI_HART_EXT_ZIHPM,
?
????????/** Maximum index of Hart extension */
????????SBI_HART_EXT_MAX,

lib/sbi/sbi_hart.c +0 -0
index c470482..435771c 100644
@@ -453,6 +453,9 @@ static inline char *sbi_hart_extension_id2string(int ext)

????????case SBI_HART_EXT_SMSTATEEN:
????????????????estr = "smstateen";
????????????????break;
+???????case SBI_HART_EXT_ZIHPM:
+???????????????estr = "zihpm";
+???????????????break;
????????default:
????????????????break;
????????}

lib/sbi/sbi_hart.c +0 -0
@@ -637,6 +640,9 @@ __pmp_skip:

???????? */
?
?__mhpm_skip:
+???????if (hfeatures->mhpm_count) +???????????????__sbi_hart_update_extension(hfeatures, +???????????????????????????????????????SBI_HART_EXT_ZIHPM, true); ?
?#undef __check_csr_64
?#undef __check_csr_32

Signature · 1 line
2.34.1
#5 lore raw

PATCH 2/4 lib: sbi: Add Zihpm as a HART ISA extension

In reply to Anup Patel

? 2023-07-05???? 17:28 +0530?Anup Patel???

Quoted 37 lines

Recently ratified Zihpm ISA extension covers all [m]hpm* CSRs so we add Zihpm as a HART ISA extension in OpenSBI.

---
?include/sbi/sbi_hart.h | 2 ++
?lib/sbi/sbi_hart.c???? | 6 ++++++
?2 files changed, 8 insertions(+)

include/sbi/sbi_hart.h +0 -0
index d48940d..938248f 100644
@@ -36,6 +36,8 @@ enum sbi_hart_extensions {
 ????????SBI_HART_EXT_SMSTATEEN,

????????/** HART has Sstc extension */
????????SBI_HART_EXT_SSTC,
+???????/** HART has Zihpm extension */
+???????SBI_HART_EXT_ZIHPM,
?
????????/** Maximum index of Hart extension */
????????SBI_HART_EXT_MAX,

lib/sbi/sbi_hart.c +0 -0
index c470482..435771c 100644
@@ -453,6 +453,9 @@ static inline char *sbi_hart_extension_id2string(int ext)

????????case SBI_HART_EXT_SMSTATEEN:
????????????????estr = "smstateen";
????????????????break;
+???????case SBI_HART_EXT_ZIHPM:
+???????????????estr = "zihpm";
+???????????????break;
????????default:
????????????????break;
????????}

lib/sbi/sbi_hart.c +0 -0
@@ -637,6 +640,9 @@ __pmp_skip:
Quoted 6 lines

???????? */
?
?__mhpm_skip:
+???????if (hfeatures->mhpm_count) +???????????????__sbi_hart_update_extension(hfeatures, +???????????????????????????????????????SBI_HART_EXT_ZIHPM, true);

allwinner D1 sets mhpm_count in sbi_platform_extensions_init. We should call __sbi_hart_update_extension again in the code of D1.

Regards,
Xiang W

Quoted 7 lines

?
?#undef __check_csr_64
?#undef __check_csr_32

Signature · 1 line
2.34.1
author
#6 lore raw

PATCH 2/4 lib: sbi: Add Zihpm as a HART ISA extension

Quoted 41 lines · On Wed, Jul 5, 2023 at 8:36?PM Xiang W <[email protected]> wrote:

? 2023-07-05???? 17:28 +0530?Anup Patel???

Quoted 39 lines

Recently ratified Zihpm ISA extension covers all [m]hpm* CSRs so we add Zihpm as a HART ISA extension in OpenSBI.

---
 include/sbi/sbi_hart.h | 2 ++
 lib/sbi/sbi_hart.c     | 6 ++++++
 2 files changed, 8 insertions(+)
include/sbi/sbi_hart.h +2 -0
index d48940d..938248f 100644
@@ -36,6 +36,8 @@ enum sbi_hart_extensions {
        SBI_HART_EXT_SMSTATEEN,
        /** HART has Sstc extension */
        SBI_HART_EXT_SSTC,
+       /** HART has Zihpm extension */
+       SBI_HART_EXT_ZIHPM,
 
        /** Maximum index of Hart extension */
        SBI_HART_EXT_MAX,
lib/sbi/sbi_hart.c +3 -0
index c470482..435771c 100644
@@ -453,6 +453,9 @@ static inline char *sbi_hart_extension_id2string(int ext)
        case SBI_HART_EXT_SMSTATEEN:
                estr = "smstateen";
                break;
+       case SBI_HART_EXT_ZIHPM:
+               estr = "zihpm";
+               break;
        default:
                break;
        }
@@ -637,6 +640,9 @@ __pmp_skip:
         */
 
Quoted 6 lines

lib/sbi/sbi_hart.c __pmp_skip: · line 645

Quoted 4 lines
lib/sbi/sbi_hart.c +3 -0
 __mhpm_skip:
+       if (hfeatures->mhpm_count)
+               __sbi_hart_update_extension(hfeatures,
+                                       SBI_HART_EXT_ZIHPM, true);

allwinner D1 sets mhpm_count in sbi_platform_extensions_init. We should call __sbi_hart_update_extension again in the code of D1.

Ahh, yes. I forgot about D1. Good catch.

We should set SBI_HART_EXT_ZIHPM in hart_detect_features() after calling sbi_platform_extensions_init().

I will update this in v2.

Regards,
Anup

Quoted 11 lines

Regards,
Xiang W

Quoted 7 lines
lib/sbi/sbi_hart.c +0 -1
 #undef __check_csr_64
 #undef __check_csr_32
--

2.34.1

author
#7 lore raw

PATCH 3/4 lib: sbi: Alphabetically sort HART ISA extensions

In reply to Anup Patel

Let us follow alphabetical order for HART ISA extension so that it is simpler to maintain.

---
 include/sbi/sbi_hart.h | 10 +++++-----
 lib/sbi/sbi_hart.c     | 16 ++++++++--------
 2 files changed, 13 insertions(+), 13 deletions(-)
include/sbi/sbi_hart.h +5 -5
index 938248f..b97f78c 100644
@@ -26,16 +26,16 @@ enum sbi_hart_priv_versions {
 
 /** Possible ISA extensions of a hart */
 enum sbi_hart_extensions {
-	/** Hart has Sscofpmt extension */
-	SBI_HART_EXT_SSCOFPMF = 0,
-	/** HART has Zicntr extension (i.e. HW cycle, time & instret CSRs) */
-	SBI_HART_EXT_ZICNTR,
 	/** HART has AIA M-mode CSRs */
-	SBI_HART_EXT_SMAIA,
+	SBI_HART_EXT_SMAIA = 0,
 	/** HART has Smstateen CSR **/
 	SBI_HART_EXT_SMSTATEEN,
+	/** Hart has Sscofpmt extension */
+	SBI_HART_EXT_SSCOFPMF,
 	/** HART has Sstc extension */
 	SBI_HART_EXT_SSTC,
+	/** HART has Zicntr extension (i.e. HW cycle, time & instret CSRs) */
+	SBI_HART_EXT_ZICNTR,
 	/** HART has Zihpm extension */
 	SBI_HART_EXT_ZIHPM,
 
lib/sbi/sbi_hart.c +8 -8
index 435771c..380e615 100644
@@ -438,20 +438,20 @@ static inline char *sbi_hart_extension_id2string(int ext)
 	char *estr = NULL;
 
 	switch (ext) {
-	case SBI_HART_EXT_SSCOFPMF:
-		estr = "sscofpmf";
-		break;
-	case SBI_HART_EXT_ZICNTR:
-		estr = "zicntr";
-		break;
 	case SBI_HART_EXT_SMAIA:
 		estr = "smaia";
 		break;
+	case SBI_HART_EXT_SMSTATEEN:
+		estr = "smstateen";
+		break;
+	case SBI_HART_EXT_SSCOFPMF:
+		estr = "sscofpmf";
+		break;
 	case SBI_HART_EXT_SSTC:
 		estr = "sstc";
 		break;
-	case SBI_HART_EXT_SMSTATEEN:
-		estr = "smstateen";
+	case SBI_HART_EXT_ZICNTR:
+		estr = "zicntr";
 		break;
 	case SBI_HART_EXT_ZIHPM:
 		estr = "zihpm";
Signature · 1 line
2.34.1
#8 lore raw

PATCH 3/4 lib: sbi: Alphabetically sort HART ISA extensions

? 2023-07-05???? 17:28 +0530?Anup Patel???

Quoted 4 lines

Let us follow alphabetical order for HART ISA extension so that it is simpler to maintain.

Look good to me.

Quoted 68 lines

---
?include/sbi/sbi_hart.h | 10 +++++----- ?lib/sbi/sbi_hart.c???? | 16 ++++++++-------- ?2 files changed, 13 insertions(+), 13 deletions(-)

include/sbi/sbi_hart.h +0 -0
index 938248f..b97f78c 100644
@@ -26,16 +26,16 @@ enum sbi_hart_priv_versions {

?
?/** Possible ISA extensions of a hart */
?enum sbi_hart_extensions {
-???????/** Hart has Sscofpmt extension */ -???????SBI_HART_EXT_SSCOFPMF = 0,
-???????/** HART has Zicntr extension (i.e. HW cycle, time & instret CSRs) */ -???????SBI_HART_EXT_ZICNTR,
????????/** HART has AIA M-mode CSRs */ -???????SBI_HART_EXT_SMAIA,
+???????SBI_HART_EXT_SMAIA = 0,
????????/** HART has Smstateen CSR **/ ????????SBI_HART_EXT_SMSTATEEN,
+???????/** Hart has Sscofpmt extension */ +???????SBI_HART_EXT_SSCOFPMF,
????????/** HART has Sstc extension */
????????SBI_HART_EXT_SSTC,
+???????/** HART has Zicntr extension (i.e. HW cycle, time & instret CSRs) */ +???????SBI_HART_EXT_ZICNTR,
????????/** HART has Zihpm extension */ ????????SBI_HART_EXT_ZIHPM,
?

lib/sbi/sbi_hart.c +0 -0
index 435771c..380e615 100644
@@ -438,20 +438,20 @@ static inline char *sbi_hart_extension_id2string(int ext)
 ????????char *estr = NULL;

?
????????switch (ext) {
-???????case SBI_HART_EXT_SSCOFPMF:
-???????????????estr = "sscofpmf";
-???????????????break;
-???????case SBI_HART_EXT_ZICNTR:
-???????????????estr = "zicntr";
-???????????????break;
????????case SBI_HART_EXT_SMAIA:
????????????????estr = "smaia";
????????????????break;
+???????case SBI_HART_EXT_SMSTATEEN:
+???????????????estr = "smstateen";
+???????????????break;
+???????case SBI_HART_EXT_SSCOFPMF:
+???????????????estr = "sscofpmf";
+???????????????break;
????????case SBI_HART_EXT_SSTC:
????????????????estr = "sstc";
????????????????break;
-???????case SBI_HART_EXT_SMSTATEEN:
-???????????????estr = "smstateen";
+???????case SBI_HART_EXT_ZICNTR:
+???????????????estr = "zicntr";
????????????????break;
????????case SBI_HART_EXT_ZIHPM:
????????????????estr = "zihpm";

Signature · 1 line
2.34.1
author
#9 lore raw

PATCH 4/4 lib: sbi: Rename hart_pmu_get_allowed_bits() function

In reply to Anup Patel

The hart_pmu_get_allowed_bits() function detects implemented bits of mhpm counters so let us rename this function accordingly.

---
 lib/sbi/sbi_hart.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
lib/sbi/sbi_hart.c +2 -2
index 380e615..25277a5 100644
@@ -527,7 +527,7 @@ static unsigned long hart_pmp_get_allowed_addr(void)
 	return val;
 }
 
-static int hart_pmu_get_allowed_bits(void)
+static int hart_mhpm_get_allowed_bits(void)
 {
 	unsigned long val = ~(0UL);
 	struct sbi_trap_info trap = {0};
@@ -628,7 +628,7 @@ __pmp_skip:
 
 	/* Detect number of MHPM counters */
 	__check_csr(CSR_MHPMCOUNTER3, 0, 1UL, mhpm_count, __mhpm_skip);
-	hfeatures->mhpm_bits = hart_pmu_get_allowed_bits();
+	hfeatures->mhpm_bits = hart_mhpm_get_allowed_bits();
 
 	__check_csr_4(CSR_MHPMCOUNTER4, 0, 1UL, mhpm_count, __mhpm_skip);
 	__check_csr_8(CSR_MHPMCOUNTER8, 0, 1UL, mhpm_count, __mhpm_skip);
Signature · 1 line
2.34.1
#10 lore raw

PATCH 4/4 lib: sbi: Rename hart_pmu_get_allowed_bits() function

? 2023-07-05???? 17:28 +0530?Anup Patel???

Quoted 4 lines

The hart_pmu_get_allowed_bits() function detects implemented bits of mhpm counters so let us rename this function accordingly.

Look good to me.

Quoted 30 lines

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

lib/sbi/sbi_hart.c +0 -0
index 380e615..25277a5 100644
@@ -527,7 +527,7 @@ static unsigned long hart_pmp_get_allowed_addr(void)
 ????????return val;
 ?}

?

lib/sbi/sbi_hart.c +1 -1
-static int hart_pmu_get_allowed_bits(void)
+static int hart_mhpm_get_allowed_bits(void)

?{
????????unsigned long val = ~(0UL);
????????struct sbi_trap_info trap = {0};

lib/sbi/sbi_hart.c +0 -0
@@ -628,7 +628,7 @@ __pmp_skip:

?
????????/* Detect number of MHPM counters */ ????????__check_csr(CSR_MHPMCOUNTER3, 0, 1UL, mhpm_count, __mhpm_skip); -???????hfeatures->mhpm_bits = hart_pmu_get_allowed_bits(); +???????hfeatures->mhpm_bits = hart_mhpm_get_allowed_bits(); ?
????????__check_csr_4(CSR_MHPMCOUNTER4, 0, 1UL, mhpm_count, __mhpm_skip); ????????__check_csr_8(CSR_MHPMCOUNTER8, 0, 1UL, mhpm_count, __mhpm_skip);

Signature · 1 line
2.34.1