[PATCH 0/2] Register Zicntr in FDT when emulating is possible

[PATCH 0/2] Register Zicntr in FDT when emulating is possible

[Yao Zi]

OpenSBI is capable of emulating time CSR on HARTs without a full Zicntr
extension. Previously, we hardcoded Zicntr extension in the devicetree
for these cores, like JH7110 in mainline Linux[1]. This doesn't reflect
the hardware and may confuse pre-SBI bootloaders, like U-Boot running in
M-Mode.

To solve the issue, let's register Zicntr in FDT dynamically for cores
supporting it by SBI emulation, allowing pre-SBI stages to detect Zicntr
availability reliably with riscv,isa-extensions.

[1]: https://elixir.bootlin.com/linux/v6.14-rc3/source/arch/riscv/boot/dts/starfive/jh7110.dtsi#L61

Yao Zi (2):
  lib: sbi: hart: Detect existence of cycle and instret CSRs for Zicntr
  lib: utils: fdt: Claim Zicntr if time CSR emulation is possible

 include/sbi/sbi_hart.h    |  8 ++++++--
 lib/sbi/sbi_hart.c        | 30 ++++++++++++++++++++++--------
 lib/sbi/sbi_timer.c       |  2 +-
 lib/utils/fdt/fdt_fixup.c | 30 +++++++++++++++++++++++++++++-
 4 files changed, 58 insertions(+), 12 deletions(-)

-- 
2.48.1

[PATCH 1/2] lib: sbi: hart: Detect existence of cycle and instret CSRs for Zicntr

[Yao Zi]

Zicntr extension specifies three RO CSRs, time, cycle and instret. It
isn't precise to report Zicntr is fully supported with only time CSR
detected.

Since CSR emulating code only cares about the availablity of time CSR,
add additional definitions to represent the existance of cycle and
instret CSRs.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 include/sbi/sbi_hart.h |  8 ++++++--
 lib/sbi/sbi_hart.c     | 30 ++++++++++++++++++++++--------
 lib/sbi/sbi_timer.c    |  2 +-
 3 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index 4c36c77..0a740cf 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -37,8 +37,12 @@ enum sbi_hart_extensions {
 	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 HW time CSR, as specified in Zicntr extension */
+	SBI_HART_EXT_ZICNTR_TIME,
+	/** HART has HW cycle CSR, as specified in Zicntr extension */
+	SBI_HART_EXT_ZICNTR_CYCLE,
+	/** HART has HW instret CSR, as specified in Zicntr extension */
+	SBI_HART_EXT_ZICNTR_INSTRET,
 	/** HART has Zihpm extension */
 	SBI_HART_EXT_ZIHPM,
 	/** HART has Zkr extension */
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 8e2979b..e1d630d 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -669,7 +669,9 @@ const struct sbi_hart_ext_data sbi_hart_ext[] = {
 	__SBI_HART_EXT_DATA(smstateen, SBI_HART_EXT_SMSTATEEN),
 	__SBI_HART_EXT_DATA(sscofpmf, SBI_HART_EXT_SSCOFPMF),
 	__SBI_HART_EXT_DATA(sstc, SBI_HART_EXT_SSTC),
-	__SBI_HART_EXT_DATA(zicntr, SBI_HART_EXT_ZICNTR),
+	__SBI_HART_EXT_DATA(zicntr_time, SBI_HART_EXT_ZICNTR_TIME),
+	__SBI_HART_EXT_DATA(zicntr_cycle, SBI_HART_EXT_ZICNTR_CYCLE),
+	__SBI_HART_EXT_DATA(zicntr_instret, SBI_HART_EXT_ZICNTR_INSTRET),
 	__SBI_HART_EXT_DATA(zihpm, SBI_HART_EXT_ZIHPM),
 	__SBI_HART_EXT_DATA(zkr, SBI_HART_EXT_ZKR),
 	__SBI_HART_EXT_DATA(smcntrpmf, SBI_HART_EXT_SMCNTRPMF),
@@ -782,7 +784,7 @@ static int hart_detect_features(struct sbi_scratch *scratch)
 	struct sbi_hart_features *hfeatures =
 		sbi_scratch_offset_ptr(scratch, hart_features_offset);
 	unsigned long val, oldval;
-	bool has_zicntr = false;
+	bool has_time = false, has_cycle = false, has_instret = false;
 	int rc;
 
 	/* If hart features already detected then do nothing */
@@ -919,7 +921,13 @@ __pmp_skip:
 			CSR_SCOUNTOVF, SBI_HART_EXT_SSCOFPMF);
 	/* Detect if hart supports time CSR */
 	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
-			CSR_TIME, SBI_HART_EXT_ZICNTR);
+			CSR_TIME, SBI_HART_EXT_ZICNTR_TIME);
+	/* Detect if hart supports cycle CSR */
+	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
+			CSR_CYCLE, SBI_HART_EXT_ZICNTR_CYCLE);
+	/* Detect if hart supports instret CSR */
+	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
+			CSR_INSTRET, SBI_HART_EXT_ZICNTR_INSTRET);
 	/* Detect if hart has AIA local interrupt CSRs */
 	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
 			CSR_MTOPI, SBI_HART_EXT_SMAIA);
@@ -938,8 +946,10 @@ __pmp_skip:
 
 #undef __check_ext_csr
 
-	/* Save trap based detection of Zicntr */
-	has_zicntr = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR);
+	/* Save trap based detection of Zicntr CSRs */
+	has_time = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_TIME);
+	has_cycle = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_CYCLE);
+	has_instret = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_INSTRET);
 
 	/* Let platform populate extensions */
 	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
@@ -947,9 +957,13 @@ __pmp_skip:
 	if (rc)
 		return rc;
 
-	/* Zicntr should only be detected using traps */
-	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR,
-				    has_zicntr);
+	/* Zicntr CSRs should only be detected using traps */
+	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_TIME,
+				    has_time);
+	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_CYCLE,
+				    has_cycle);
+	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_INSTRET,
+				    has_instret);
 
 	/* Extensions implied by other extensions and features */
 	if (hfeatures->mhpm_mask)
diff --git a/lib/sbi/sbi_timer.c b/lib/sbi/sbi_timer.c
index 86e0db5..b4db4ca 100644
--- a/lib/sbi/sbi_timer.c
+++ b/lib/sbi/sbi_timer.c
@@ -190,7 +190,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_ZICNTR))
+		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_TIME))
 			get_time_val = get_ticks;
 
 		ret = sbi_platform_timer_init(plat);
-- 
2.48.1

[PATCH 1/2] lib: sbi: hart: Detect existence of cycle and instret CSRs for Zicntr

[Xiang W]

? 2025-02-25?? 15:41 +0000?Yao Zi???
> Zicntr extension specifies three RO CSRs, time, cycle and instret. It
remove 'R0'

Regards,
Xiang W
> isn't precise to report Zicntr is fully supported with only time CSR
> detected.
> 
> Since CSR emulating code only cares about the availablity of time CSR,
> add additional definitions to represent the existance of cycle and
> instret CSRs.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
> ?include/sbi/sbi_hart.h |? 8 ++++++--
> ?lib/sbi/sbi_hart.c???? | 30 ++++++++++++++++++++++--------
> ?lib/sbi/sbi_timer.c??? |? 2 +-
> ?3 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index 4c36c77..0a740cf 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -37,8 +37,12 @@ enum sbi_hart_extensions {
> ?	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 HW time CSR, as specified in Zicntr extension */
> +	SBI_HART_EXT_ZICNTR_TIME,
> +	/** HART has HW cycle CSR, as specified in Zicntr extension */
> +	SBI_HART_EXT_ZICNTR_CYCLE,
> +	/** HART has HW instret CSR, as specified in Zicntr extension */
> +	SBI_HART_EXT_ZICNTR_INSTRET,
> ?	/** HART has Zihpm extension */
> ?	SBI_HART_EXT_ZIHPM,
> ?	/** HART has Zkr extension */
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 8e2979b..e1d630d 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -669,7 +669,9 @@ const struct sbi_hart_ext_data sbi_hart_ext[] = {
> ?	__SBI_HART_EXT_DATA(smstateen, SBI_HART_EXT_SMSTATEEN),
> ?	__SBI_HART_EXT_DATA(sscofpmf, SBI_HART_EXT_SSCOFPMF),
> ?	__SBI_HART_EXT_DATA(sstc, SBI_HART_EXT_SSTC),
> -	__SBI_HART_EXT_DATA(zicntr, SBI_HART_EXT_ZICNTR),
> +	__SBI_HART_EXT_DATA(zicntr_time, SBI_HART_EXT_ZICNTR_TIME),
> +	__SBI_HART_EXT_DATA(zicntr_cycle, SBI_HART_EXT_ZICNTR_CYCLE),
> +	__SBI_HART_EXT_DATA(zicntr_instret, SBI_HART_EXT_ZICNTR_INSTRET),
> ?	__SBI_HART_EXT_DATA(zihpm, SBI_HART_EXT_ZIHPM),
> ?	__SBI_HART_EXT_DATA(zkr, SBI_HART_EXT_ZKR),
> ?	__SBI_HART_EXT_DATA(smcntrpmf, SBI_HART_EXT_SMCNTRPMF),
> @@ -782,7 +784,7 @@ static int hart_detect_features(struct sbi_scratch *scratch)
> ?	struct sbi_hart_features *hfeatures =
> ?		sbi_scratch_offset_ptr(scratch, hart_features_offset);
> ?	unsigned long val, oldval;
> -	bool has_zicntr = false;
> +	bool has_time = false, has_cycle = false, has_instret = false;
> ?	int rc;
> ?
> ?	/* If hart features already detected then do nothing */
> @@ -919,7 +921,13 @@ __pmp_skip:
> ?			CSR_SCOUNTOVF, SBI_HART_EXT_SSCOFPMF);
> ?	/* Detect if hart supports time CSR */
> ?	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> -			CSR_TIME, SBI_HART_EXT_ZICNTR);
> +			CSR_TIME, SBI_HART_EXT_ZICNTR_TIME);
> +	/* Detect if hart supports cycle CSR */
> +	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> +			CSR_CYCLE, SBI_HART_EXT_ZICNTR_CYCLE);
> +	/* Detect if hart supports instret CSR */
> +	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> +			CSR_INSTRET, SBI_HART_EXT_ZICNTR_INSTRET);
> ?	/* Detect if hart has AIA local interrupt CSRs */
> ?	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> ?			CSR_MTOPI, SBI_HART_EXT_SMAIA);
> @@ -938,8 +946,10 @@ __pmp_skip:
> ?
> ?#undef __check_ext_csr
> ?
> -	/* Save trap based detection of Zicntr */
> -	has_zicntr = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR);
> +	/* Save trap based detection of Zicntr CSRs */
> +	has_time = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_TIME);
> +	has_cycle = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_CYCLE);
> +	has_instret = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_INSTRET);
> ?
> ?	/* Let platform populate extensions */
> ?	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
> @@ -947,9 +957,13 @@ __pmp_skip:
> ?	if (rc)
> ?		return rc;
> ?
> -	/* Zicntr should only be detected using traps */
> -	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR,
> -				??? has_zicntr);
> +	/* Zicntr CSRs should only be detected using traps */
> +	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_TIME,
> +				??? has_time);
> +	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_CYCLE,
> +				??? has_cycle);
> +	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_INSTRET,
> +				??? has_instret);
> ?
> ?	/* Extensions implied by other extensions and features */
> ?	if (hfeatures->mhpm_mask)
> diff --git a/lib/sbi/sbi_timer.c b/lib/sbi/sbi_timer.c
> index 86e0db5..b4db4ca 100644
> --- a/lib/sbi/sbi_timer.c
> +++ b/lib/sbi/sbi_timer.c
> @@ -190,7 +190,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_ZICNTR))
> +		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_TIME))
> ?			get_time_val = get_ticks;
> ?
> ?		ret = sbi_platform_timer_init(plat);
> -- 
> 2.48.1
> 
> 

[PATCH 1/2] lib: sbi: hart: Detect existence of cycle and instret CSRs for Zicntr

[Yao Zi]

On Wed, Feb 26, 2025 at 02:26:50PM +0800, Xiang W wrote:
> ? 2025-02-25?? 15:41 +0000?Yao Zi???
> > Zicntr extension specifies three RO CSRs, time, cycle and instret. It
> remove 'R0'

It's RO (read only), not R0. Did you misread it?

For reference,

> RISC-V ISAs provide a set of up to thirty-two 64-bit performance
> counters and timers that are accessible via unprivileged XLEN-bit
> read-only CSR registers 0xC00?0xC1F (when XLEN=32, the upper 32 bits
> are accessed via CSR registers 0xC80?0xC9F). These counters are
> divided between the "Zicntr" and "Zihpm" extensions.[1]

Anyway, I could fix the commit message to prevent possible confusion.

> Regards,
> Xiang W

Best regards,
Yao Zi

[1]: The RISC-V Instruction Set, Manual Volume I:
     Unprivileged Architecture (Version 2024041) Chapter 8

> > isn't precise to report Zicntr is fully supported with only time CSR
> > detected.
> > 
> > Since CSR emulating code only cares about the availablity of time CSR,
> > add additional definitions to represent the existance of cycle and
> > instret CSRs.
> > 
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> > ?include/sbi/sbi_hart.h |? 8 ++++++--
> > ?lib/sbi/sbi_hart.c???? | 30 ++++++++++++++++++++++--------
> > ?lib/sbi/sbi_timer.c??? |? 2 +-
> > ?3 files changed, 29 insertions(+), 11 deletions(-)
> > 
> > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> > index 4c36c77..0a740cf 100644
> > --- a/include/sbi/sbi_hart.h
> > +++ b/include/sbi/sbi_hart.h
> > @@ -37,8 +37,12 @@ enum sbi_hart_extensions {
> > ?	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 HW time CSR, as specified in Zicntr extension */
> > +	SBI_HART_EXT_ZICNTR_TIME,
> > +	/** HART has HW cycle CSR, as specified in Zicntr extension */
> > +	SBI_HART_EXT_ZICNTR_CYCLE,
> > +	/** HART has HW instret CSR, as specified in Zicntr extension */
> > +	SBI_HART_EXT_ZICNTR_INSTRET,
> > ?	/** HART has Zihpm extension */
> > ?	SBI_HART_EXT_ZIHPM,
> > ?	/** HART has Zkr extension */
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 8e2979b..e1d630d 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -669,7 +669,9 @@ const struct sbi_hart_ext_data sbi_hart_ext[] = {
> > ?	__SBI_HART_EXT_DATA(smstateen, SBI_HART_EXT_SMSTATEEN),
> > ?	__SBI_HART_EXT_DATA(sscofpmf, SBI_HART_EXT_SSCOFPMF),
> > ?	__SBI_HART_EXT_DATA(sstc, SBI_HART_EXT_SSTC),
> > -	__SBI_HART_EXT_DATA(zicntr, SBI_HART_EXT_ZICNTR),
> > +	__SBI_HART_EXT_DATA(zicntr_time, SBI_HART_EXT_ZICNTR_TIME),
> > +	__SBI_HART_EXT_DATA(zicntr_cycle, SBI_HART_EXT_ZICNTR_CYCLE),
> > +	__SBI_HART_EXT_DATA(zicntr_instret, SBI_HART_EXT_ZICNTR_INSTRET),
> > ?	__SBI_HART_EXT_DATA(zihpm, SBI_HART_EXT_ZIHPM),
> > ?	__SBI_HART_EXT_DATA(zkr, SBI_HART_EXT_ZKR),
> > ?	__SBI_HART_EXT_DATA(smcntrpmf, SBI_HART_EXT_SMCNTRPMF),
> > @@ -782,7 +784,7 @@ static int hart_detect_features(struct sbi_scratch *scratch)
> > ?	struct sbi_hart_features *hfeatures =
> > ?		sbi_scratch_offset_ptr(scratch, hart_features_offset);
> > ?	unsigned long val, oldval;
> > -	bool has_zicntr = false;
> > +	bool has_time = false, has_cycle = false, has_instret = false;
> > ?	int rc;
> > ?
> > ?	/* If hart features already detected then do nothing */
> > @@ -919,7 +921,13 @@ __pmp_skip:
> > ?			CSR_SCOUNTOVF, SBI_HART_EXT_SSCOFPMF);
> > ?	/* Detect if hart supports time CSR */
> > ?	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> > -			CSR_TIME, SBI_HART_EXT_ZICNTR);
> > +			CSR_TIME, SBI_HART_EXT_ZICNTR_TIME);
> > +	/* Detect if hart supports cycle CSR */
> > +	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> > +			CSR_CYCLE, SBI_HART_EXT_ZICNTR_CYCLE);
> > +	/* Detect if hart supports instret CSR */
> > +	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> > +			CSR_INSTRET, SBI_HART_EXT_ZICNTR_INSTRET);
> > ?	/* Detect if hart has AIA local interrupt CSRs */
> > ?	__check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> > ?			CSR_MTOPI, SBI_HART_EXT_SMAIA);
> > @@ -938,8 +946,10 @@ __pmp_skip:
> > ?
> > ?#undef __check_ext_csr
> > ?
> > -	/* Save trap based detection of Zicntr */
> > -	has_zicntr = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR);
> > +	/* Save trap based detection of Zicntr CSRs */
> > +	has_time = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_TIME);
> > +	has_cycle = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_CYCLE);
> > +	has_instret = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_INSTRET);
> > ?
> > ?	/* Let platform populate extensions */
> > ?	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
> > @@ -947,9 +957,13 @@ __pmp_skip:
> > ?	if (rc)
> > ?		return rc;
> > ?
> > -	/* Zicntr should only be detected using traps */
> > -	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR,
> > -				??? has_zicntr);
> > +	/* Zicntr CSRs should only be detected using traps */
> > +	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_TIME,
> > +				??? has_time);
> > +	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_CYCLE,
> > +				??? has_cycle);
> > +	__sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_INSTRET,
> > +				??? has_instret);
> > ?
> > ?	/* Extensions implied by other extensions and features */
> > ?	if (hfeatures->mhpm_mask)
> > diff --git a/lib/sbi/sbi_timer.c b/lib/sbi/sbi_timer.c
> > index 86e0db5..b4db4ca 100644
> > --- a/lib/sbi/sbi_timer.c
> > +++ b/lib/sbi/sbi_timer.c
> > @@ -190,7 +190,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_ZICNTR))
> > +		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_TIME))
> > ?			get_time_val = get_ticks;
> > ?
> > ?		ret = sbi_platform_timer_init(plat);
> > -- 
> > 2.48.1
> > 
> > 
> 

[PATCH 1/2] lib: sbi: hart: Detect existence of cycle and instret CSRs for Zicntr

[Anup Patel]

On Tue, Feb 25, 2025 at 10:14?PM Yao Zi <ziyao@disroot.org> wrote:
>
> Zicntr extension specifies three RO CSRs, time, cycle and instret. It
> isn't precise to report Zicntr is fully supported with only time CSR
> detected.
>
> Since CSR emulating code only cares about the availablity of time CSR,
> add additional definitions to represent the existance of cycle and
> instret CSRs.
>
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  include/sbi/sbi_hart.h |  8 ++++++--
>  lib/sbi/sbi_hart.c     | 30 ++++++++++++++++++++++--------
>  lib/sbi/sbi_timer.c    |  2 +-
>  3 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index 4c36c77..0a740cf 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -37,8 +37,12 @@ enum sbi_hart_extensions {
>         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 HW time CSR, as specified in Zicntr extension */
> +       SBI_HART_EXT_ZICNTR_TIME,
> +       /** HART has HW cycle CSR, as specified in Zicntr extension */
> +       SBI_HART_EXT_ZICNTR_CYCLE,
> +       /** HART has HW instret CSR, as specified in Zicntr extension */
> +       SBI_HART_EXT_ZICNTR_INSTRET,

Zicntr_time, Zicntr_cycle, and Zicntr_instret are not real extension
names so let's not add pseudo extension names.

Instead, I suggest introducing "enum sbi_hart_csrs" for CSRs
detected at boot-time. We will also need "csrs" bitmap in
"struct sbi_hart_features".

The new "enum sbi_hart_csrs" can have:
SBI_HART_CSR_CYCLE,
SBI_HART_CSR_TIME,
SBI_HART_CSR_INSTRET

These bits will be set in "struct sbi_hart_features" when
corresponding CSRs are detected at boot-time.

The SBI_HART_EXT_ZICNTR is only available when all
three SBI_HART_CSR_CYCLE, SBI_HART_CSR_TIME,
and SBI_HART_CSR_INSTRET are available.

>         /** HART has Zihpm extension */
>         SBI_HART_EXT_ZIHPM,
>         /** HART has Zkr extension */
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 8e2979b..e1d630d 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -669,7 +669,9 @@ const struct sbi_hart_ext_data sbi_hart_ext[] = {
>         __SBI_HART_EXT_DATA(smstateen, SBI_HART_EXT_SMSTATEEN),
>         __SBI_HART_EXT_DATA(sscofpmf, SBI_HART_EXT_SSCOFPMF),
>         __SBI_HART_EXT_DATA(sstc, SBI_HART_EXT_SSTC),
> -       __SBI_HART_EXT_DATA(zicntr, SBI_HART_EXT_ZICNTR),
> +       __SBI_HART_EXT_DATA(zicntr_time, SBI_HART_EXT_ZICNTR_TIME),
> +       __SBI_HART_EXT_DATA(zicntr_cycle, SBI_HART_EXT_ZICNTR_CYCLE),
> +       __SBI_HART_EXT_DATA(zicntr_instret, SBI_HART_EXT_ZICNTR_INSTRET),
>         __SBI_HART_EXT_DATA(zihpm, SBI_HART_EXT_ZIHPM),
>         __SBI_HART_EXT_DATA(zkr, SBI_HART_EXT_ZKR),
>         __SBI_HART_EXT_DATA(smcntrpmf, SBI_HART_EXT_SMCNTRPMF),
> @@ -782,7 +784,7 @@ static int hart_detect_features(struct sbi_scratch *scratch)
>         struct sbi_hart_features *hfeatures =
>                 sbi_scratch_offset_ptr(scratch, hart_features_offset);
>         unsigned long val, oldval;
> -       bool has_zicntr = false;
> +       bool has_time = false, has_cycle = false, has_instret = false;
>         int rc;
>
>         /* If hart features already detected then do nothing */
> @@ -919,7 +921,13 @@ __pmp_skip:
>                         CSR_SCOUNTOVF, SBI_HART_EXT_SSCOFPMF);
>         /* Detect if hart supports time CSR */
>         __check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> -                       CSR_TIME, SBI_HART_EXT_ZICNTR);
> +                       CSR_TIME, SBI_HART_EXT_ZICNTR_TIME);
> +       /* Detect if hart supports cycle CSR */
> +       __check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> +                       CSR_CYCLE, SBI_HART_EXT_ZICNTR_CYCLE);
> +       /* Detect if hart supports instret CSR */
> +       __check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
> +                       CSR_INSTRET, SBI_HART_EXT_ZICNTR_INSTRET);
>         /* Detect if hart has AIA local interrupt CSRs */
>         __check_ext_csr(SBI_HART_PRIV_VER_UNKNOWN,
>                         CSR_MTOPI, SBI_HART_EXT_SMAIA);
> @@ -938,8 +946,10 @@ __pmp_skip:
>
>  #undef __check_ext_csr
>
> -       /* Save trap based detection of Zicntr */
> -       has_zicntr = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR);
> +       /* Save trap based detection of Zicntr CSRs */
> +       has_time = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_TIME);
> +       has_cycle = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_CYCLE);
> +       has_instret = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_INSTRET);
>
>         /* Let platform populate extensions */
>         rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
> @@ -947,9 +957,13 @@ __pmp_skip:
>         if (rc)
>                 return rc;
>
> -       /* Zicntr should only be detected using traps */
> -       __sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR,
> -                                   has_zicntr);
> +       /* Zicntr CSRs should only be detected using traps */
> +       __sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_TIME,
> +                                   has_time);
> +       __sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_CYCLE,
> +                                   has_cycle);
> +       __sbi_hart_update_extension(hfeatures, SBI_HART_EXT_ZICNTR_INSTRET,
> +                                   has_instret);
>
>         /* Extensions implied by other extensions and features */
>         if (hfeatures->mhpm_mask)
> diff --git a/lib/sbi/sbi_timer.c b/lib/sbi/sbi_timer.c
> index 86e0db5..b4db4ca 100644
> --- a/lib/sbi/sbi_timer.c
> +++ b/lib/sbi/sbi_timer.c
> @@ -190,7 +190,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_ZICNTR))
> +               if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_TIME))
>                         get_time_val = get_ticks;
>
>                 ret = sbi_platform_timer_init(plat);
> --
> 2.48.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Regards,
Anup

[PATCH 1/2] lib: sbi: hart: Detect existence of cycle and instret CSRs for Zicntr

[Yao Zi]

On Tue, Mar 25, 2025 at 01:45:25PM +0530, Anup Patel wrote:
> On Tue, Feb 25, 2025 at 10:14?PM Yao Zi <ziyao@disroot.org> wrote:
> >
> > Zicntr extension specifies three RO CSRs, time, cycle and instret. It
> > isn't precise to report Zicntr is fully supported with only time CSR
> > detected.
> >
> > Since CSR emulating code only cares about the availablity of time CSR,
> > add additional definitions to represent the existance of cycle and
> > instret CSRs.
> >
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  include/sbi/sbi_hart.h |  8 ++++++--
> >  lib/sbi/sbi_hart.c     | 30 ++++++++++++++++++++++--------
> >  lib/sbi/sbi_timer.c    |  2 +-
> >  3 files changed, 29 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> > index 4c36c77..0a740cf 100644
> > --- a/include/sbi/sbi_hart.h
> > +++ b/include/sbi/sbi_hart.h
> > @@ -37,8 +37,12 @@ enum sbi_hart_extensions {
> >         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 HW time CSR, as specified in Zicntr extension */
> > +       SBI_HART_EXT_ZICNTR_TIME,
> > +       /** HART has HW cycle CSR, as specified in Zicntr extension */
> > +       SBI_HART_EXT_ZICNTR_CYCLE,
> > +       /** HART has HW instret CSR, as specified in Zicntr extension */
> > +       SBI_HART_EXT_ZICNTR_INSTRET,
> 
> Zicntr_time, Zicntr_cycle, and Zicntr_instret are not real extension
> names so let's not add pseudo extension names.
> 
> Instead, I suggest introducing "enum sbi_hart_csrs" for CSRs
> detected at boot-time. We will also need "csrs" bitmap in
> "struct sbi_hart_features".
> 
> The new "enum sbi_hart_csrs" can have:
> SBI_HART_CSR_CYCLE,
> SBI_HART_CSR_TIME,
> SBI_HART_CSR_INSTRET
> 
> These bits will be set in "struct sbi_hart_features" when
> corresponding CSRs are detected at boot-time.

Makes sense to me, I'll take it in v2.

Thanks,
Yao Zi

[PATCH 2/2] lib: utils: fdt: Claim Zicntr if time CSR emulation is possible

[Yao Zi]

OpenSBI is capable of emulating time CSR through an external timer for
HARTs that don't implement a full Zicntr extension. Let's register
Zicntr extension in the FDT if CSR emulation is active.

This avoids hardcoding the extension in the devicetree, which may
confuse pre-SBI bootloaders.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 lib/utils/fdt/fdt_fixup.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/lib/utils/fdt/fdt_fixup.c b/lib/utils/fdt/fdt_fixup.c
index 192d6dd..341cada 100644
--- a/lib/utils/fdt/fdt_fixup.c
+++ b/lib/utils/fdt/fdt_fixup.c
@@ -16,6 +16,7 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 #include <sbi/sbi_error.h>
+#include <sbi/sbi_timer.h>
 #include <sbi_utils/fdt/fdt_fixup.h>
 #include <sbi_utils/fdt/fdt_pmu.h>
 #include <sbi_utils/fdt/fdt_helper.h>
@@ -107,10 +108,22 @@ int fdt_add_cpu_idle_states(void *fdt, const struct sbi_cpu_idle_state *state)
 
 void fdt_cpu_fixup(void *fdt)
 {
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
 	struct sbi_domain *dom = sbi_domain_thishart_ptr();
 	int err, cpu_offset, cpus_offset, len;
-	const char *mmu_type;
+	const char *mmu_type, *extensions;
 	u32 hartid, hartindex;
+	bool emulated_zicntr;
+
+	/*
+	 * Claim Zicntr extension in riscv,isa-extensions if
+	 *  1. OpenSBI can emulate time CSR with a timer
+	 *  2. The other two CSRs specified by Zicntr are available
+	 */
+	emulated_zicntr = sbi_timer_get_device() != NULL &&
+			  sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_CYCLE) &&
+			  sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR_INSTRET);
+
 
 	err = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) + 32);
 	if (err < 0)
@@ -140,6 +153,21 @@ void fdt_cpu_fixup(void *fdt)
 		    !mmu_type || !len)
 			fdt_setprop_string(fdt, cpu_offset, "status",
 					   "disabled");
+
+		if (!emulated_zicntr)
+			continue;
+
+		extensions = fdt_getprop(fdt, cpu_offset,
+					 "riscv,isa-extensions", &len);
+		if (!extensions ||
+		    !fdt_stringlist_contains(extensions, len, "zicntr")) {
+			err = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) + 16);
+			if (err)
+				continue;
+
+			fdt_appendprop_string(fdt, cpu_offset,
+					      "riscv,isa-extensions", "zicntr");
+		}
 	}
 }
 
-- 
2.48.1

[PATCH 0/2] Register Zicntr in FDT when emulating is possible

[Yao Zi]

On Tue, Feb 25, 2025 at 03:41:01PM +0000, Yao Zi wrote:
> OpenSBI is capable of emulating time CSR on HARTs without a full Zicntr
> extension. Previously, we hardcoded Zicntr extension in the devicetree
> for these cores, like JH7110 in mainline Linux[1]. This doesn't reflect
> the hardware and may confuse pre-SBI bootloaders, like U-Boot running in
> M-Mode.
> 
> To solve the issue, let's register Zicntr in FDT dynamically for cores
> supporting it by SBI emulation, allowing pre-SBI stages to detect Zicntr
> availability reliably with riscv,isa-extensions.

Ping on this series, are there any more comments? If no I'll improve the
commit message of PATCH 1 and send v2 soon.

Thanks,
Yao Zi

> [1]: https://elixir.bootlin.com/linux/v6.14-rc3/source/arch/riscv/boot/dts/starfive/jh7110.dtsi#L61
> 
> Yao Zi (2):
>   lib: sbi: hart: Detect existence of cycle and instret CSRs for Zicntr
>   lib: utils: fdt: Claim Zicntr if time CSR emulation is possible
> 
>  include/sbi/sbi_hart.h    |  8 ++++++--
>  lib/sbi/sbi_hart.c        | 30 ++++++++++++++++++++++--------
>  lib/sbi/sbi_timer.c       |  2 +-
>  lib/utils/fdt/fdt_fixup.c | 30 +++++++++++++++++++++++++++++-
>  4 files changed, 58 insertions(+), 12 deletions(-)
> 
> -- 
> 2.48.1
>