[PATCH v4 0/5] Add support for T-HEAD C9xx PMU extensions

[PATCH v4 0/5] Add support for T-HEAD C9xx PMU extensions

[Heiko Stuebner]

The T-HEAD C9XX cores implement functionality very similar to SSCOFPMF.
Instead of implementing a separate interface into SBI as done in v1,
we can add the C9XX tidbits with quite minimal overhead and leaverage
the existing SBI pmu interface including giving access to the firmware
counters.

The current only hickup is the detection override in sbi_hart, where
I still need to find out why the MHPMCOUNTERs are not writeable
at _that_ point of the boot, but with this series on top of openSBI 1.1
and the matching kernel perf patch I get reasonable results for example
for:

perf stat -e cycles -e instructions -e L1-icache-load-misses -e L1-icache-loads ls

 Performance counter stats for 'ls':

          17119496      cycles
           2867765      instructions              #    0.17  insn per cycle
             55929      L1-icache-load-misses     #    1.61% of all L1-icache accesses
           3467346      L1-icache-loads

       0.030156216 seconds time elapsed

       0.000000000 seconds user
       0.023496000 seconds sys

changes in v4:
- use the new sbi_pmu_device structure for holding the
  overrides and extend it where needed

changes in v3:
- follow Atish's advice and implement an abstraction
  to not pollute the core pmu with cpu-specific variants

changes in v2:
- don't implement a separate interface but instead modify
  the sbi-pmu to allow the c9xx to use standard pmu interface


Heiko Stuebner (5):
  lib: sbi: do platform-specific extension population earlier
  lib: sbi_pmu: move pmu irq information into pmu itself
  lib: sbi_pmu: add override for counter data
  platform: generic: add extensions_init handler and platform-override
  platform: generic: allwinner: add support for c9xx pmu

 include/sbi/sbi_pmu.h                        |  16 +++
 lib/sbi/sbi_hart.c                           |  21 ++-
 lib/sbi/sbi_pmu.c                            |  19 +++
 platform/generic/allwinner/sun20i-d1.c       |  72 +++++++++++
 platform/generic/include/platform_override.h |   1 +
 platform/generic/include/thead_c9xx.h        | 127 +++++++++++++++++++
 platform/generic/platform.c                  |   9 ++
 7 files changed, 258 insertions(+), 7 deletions(-)
 create mode 100644 platform/generic/include/thead_c9xx.h

-- 
2.35.1

[PATCH v4 1/5] lib: sbi: do platform-specific extension population earlier

[Heiko Stuebner]

Some of the more specific detections in hart_detect_features()
might need to check platform-specific extensions so might need
the platform-extensions being populated earlier.

So move the call to sbi_platform_extensions_init() to an earlier
place.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 lib/sbi/sbi_hart.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 1294868..9540e5c 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -553,6 +553,11 @@ static int hart_detect_features(struct sbi_scratch *scratch)
 	hfeatures->pmp_count = 0;
 	hfeatures->mhpm_count = 0;
 
+	/* Let platform populate extensions */
+	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr());
+	if (rc)
+		return rc;
+
 #define __check_csr(__csr, __rdonly, __wrval, __field, __skip)	\
 	oldval = csr_read_allowed(__csr, (ulong)&trap);			\
 	if (!trap.cause) {						\
@@ -681,11 +686,6 @@ __mhpm_skip:
 					SBI_HART_EXT_SMSTATEEN, true);
 	}
 
-	/* Let platform populate extensions */
-	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr());
-	if (rc)
-		return rc;
-
 	/* Mark hart feature detection done */
 	hfeatures->detected = true;
 
-- 
2.35.1

[PATCH v4 1/5] lib: sbi: do platform-specific extension population earlier

[Guo Ren]

Acked-by: Guo Ren <guoren@kernel.org>

On Mon, Sep 26, 2022 at 6:16 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> Some of the more specific detections in hart_detect_features()
> might need to check platform-specific extensions so might need
> the platform-extensions being populated earlier.
>
> So move the call to sbi_platform_extensions_init() to an earlier
> place.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  lib/sbi/sbi_hart.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 1294868..9540e5c 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -553,6 +553,11 @@ static int hart_detect_features(struct sbi_scratch *scratch)
>         hfeatures->pmp_count = 0;
>         hfeatures->mhpm_count = 0;
>
> +       /* Let platform populate extensions */
> +       rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr());
> +       if (rc)
> +               return rc;
> +
>  #define __check_csr(__csr, __rdonly, __wrval, __field, __skip) \
>         oldval = csr_read_allowed(__csr, (ulong)&trap);                 \
>         if (!trap.cause) {                                              \
> @@ -681,11 +686,6 @@ __mhpm_skip:
>                                         SBI_HART_EXT_SMSTATEEN, true);
>         }
>
> -       /* Let platform populate extensions */
> -       rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr());
> -       if (rc)
> -               return rc;
> -
>         /* Mark hart feature detection done */
>         hfeatures->detected = true;
>
> --
> 2.35.1
>


-- 
Best Regards
 Guo Ren

[PATCH v4 2/5] lib: sbi_pmu: move pmu irq information into pmu itself

[Heiko Stuebner]

Don't spread checking for pmu extensions through the code
but instead introduce a sbi-pmu function that other code can
call to get the correct information about the existence of the
pmu interrupt.

Add a sbi_pmu_device override function to allow overridung this
bit as well if needed.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 include/sbi/sbi_pmu.h |  8 ++++++++
 lib/sbi/sbi_hart.c    |  4 ++--
 lib/sbi/sbi_pmu.c     | 12 ++++++++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index a2ce42d..c365243 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -73,6 +73,11 @@ struct sbi_pmu_device {
 	 * Note: 0 <= counter_index < SBI_PMU_HW_CTR_MAX
 	 */
 	void (*hw_counter_disable_irq)(uint32_t counter_index);
+
+	/**
+	 * Custom function returning the machine-specific irq-bit.
+	 */
+	int (*hw_counter_irq_bit)(void);
 };
 
 /** Get the PMU platform device */
@@ -87,6 +92,9 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot);
 /** Reset PMU during hart exit */
 void sbi_pmu_exit(struct sbi_scratch *scratch);
 
+/** Return the pmu irq bit depending on extension existence */
+int sbi_pmu_irq_bit(void);
+
 /**
  * Add the hardware event to counter mapping information. This should be called
  * from the platform code to update the mapping table.
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 9540e5c..45fbcde 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -19,6 +19,7 @@
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_math.h>
 #include <sbi/sbi_platform.h>
+#include <sbi/sbi_pmu.h>
 #include <sbi/sbi_string.h>
 #include <sbi/sbi_trap.h>
 
@@ -208,8 +209,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_extension(scratch, SBI_HART_EXT_SSCOFPMF))
-		interrupts |= MIP_LCOFIP;
+	interrupts |= sbi_pmu_irq_bit();
 
 	exceptions = (1U << CAUSE_MISALIGNED_FETCH) | (1U << CAUSE_BREAKPOINT) |
 		     (1U << CAUSE_USER_ECALL);
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 214d5e8..91d9ccc 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -344,6 +344,18 @@ skip_inhibit_update:
 	return 0;
 }
 
+int sbi_pmu_irq_bit(void)
+{
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
+
+	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
+		return MIP_LCOFIP;
+	if (pmu_dev && pmu_dev->hw_counter_irq_bit)
+		return pmu_dev->hw_counter_irq_bit();
+
+	return 0;
+}
+
 static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
 			    uint64_t ival, bool ival_update)
 {
-- 
2.35.1

[PATCH v4 2/5] lib: sbi_pmu: move pmu irq information into pmu itself

[Guo Ren]

Reviewed-by: Guo Ren <guoren@kernel.org>

On Mon, Sep 26, 2022 at 6:16 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> Don't spread checking for pmu extensions through the code
> but instead introduce a sbi-pmu function that other code can
> call to get the correct information about the existence of the
> pmu interrupt.
>
> Add a sbi_pmu_device override function to allow overridung this
> bit as well if needed.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  include/sbi/sbi_pmu.h |  8 ++++++++
>  lib/sbi/sbi_hart.c    |  4 ++--
>  lib/sbi/sbi_pmu.c     | 12 ++++++++++++
>  3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> index a2ce42d..c365243 100644
> --- a/include/sbi/sbi_pmu.h
> +++ b/include/sbi/sbi_pmu.h
> @@ -73,6 +73,11 @@ struct sbi_pmu_device {
>          * Note: 0 <= counter_index < SBI_PMU_HW_CTR_MAX
>          */
>         void (*hw_counter_disable_irq)(uint32_t counter_index);
> +
> +       /**
> +        * Custom function returning the machine-specific irq-bit.
> +        */
> +       int (*hw_counter_irq_bit)(void);
>  };
>
>  /** Get the PMU platform device */
> @@ -87,6 +92,9 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot);
>  /** Reset PMU during hart exit */
>  void sbi_pmu_exit(struct sbi_scratch *scratch);
>
> +/** Return the pmu irq bit depending on extension existence */
> +int sbi_pmu_irq_bit(void);
> +
>  /**
>   * Add the hardware event to counter mapping information. This should be called
>   * from the platform code to update the mapping table.
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 9540e5c..45fbcde 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -19,6 +19,7 @@
>  #include <sbi/sbi_hart.h>
>  #include <sbi/sbi_math.h>
>  #include <sbi/sbi_platform.h>
> +#include <sbi/sbi_pmu.h>
>  #include <sbi/sbi_string.h>
>  #include <sbi/sbi_trap.h>
>
> @@ -208,8 +209,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_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> -               interrupts |= MIP_LCOFIP;
> +       interrupts |= sbi_pmu_irq_bit();
>
>         exceptions = (1U << CAUSE_MISALIGNED_FETCH) | (1U << CAUSE_BREAKPOINT) |
>                      (1U << CAUSE_USER_ECALL);
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 214d5e8..91d9ccc 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -344,6 +344,18 @@ skip_inhibit_update:
>         return 0;
>  }
>
> +int sbi_pmu_irq_bit(void)
> +{
> +       struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> +
> +       if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> +               return MIP_LCOFIP;
> +       if (pmu_dev && pmu_dev->hw_counter_irq_bit)
> +               return pmu_dev->hw_counter_irq_bit();
> +
> +       return 0;
> +}
> +
>  static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
>                             uint64_t ival, bool ival_update)
>  {
> --
> 2.35.1
>


-- 
Best Regards
 Guo Ren

[PATCH v4 2/5] lib: sbi_pmu: move pmu irq information into pmu itself

[Atish Patra]

On Mon, Sep 26, 2022 at 3:18 AM Heiko Stuebner <heiko@sntech.de> wrote:
>
> Don't spread checking for pmu extensions through the code
> but instead introduce a sbi-pmu function that other code can
> call to get the correct information about the existence of the
> pmu interrupt.
>
> Add a sbi_pmu_device override function to allow overridung this
> bit as well if needed.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  include/sbi/sbi_pmu.h |  8 ++++++++
>  lib/sbi/sbi_hart.c    |  4 ++--
>  lib/sbi/sbi_pmu.c     | 12 ++++++++++++
>  3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> index a2ce42d..c365243 100644
> --- a/include/sbi/sbi_pmu.h
> +++ b/include/sbi/sbi_pmu.h
> @@ -73,6 +73,11 @@ struct sbi_pmu_device {
>          * Note: 0 <= counter_index < SBI_PMU_HW_CTR_MAX
>          */
>         void (*hw_counter_disable_irq)(uint32_t counter_index);
> +
> +       /**
> +        * Custom function returning the machine-specific irq-bit.
> +        */
> +       int (*hw_counter_irq_bit)(void);
>  };
>
>  /** Get the PMU platform device */
> @@ -87,6 +92,9 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot);
>  /** Reset PMU during hart exit */
>  void sbi_pmu_exit(struct sbi_scratch *scratch);
>
> +/** Return the pmu irq bit depending on extension existence */
> +int sbi_pmu_irq_bit(void);
> +
>  /**
>   * Add the hardware event to counter mapping information. This should be called
>   * from the platform code to update the mapping table.
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 9540e5c..45fbcde 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -19,6 +19,7 @@
>  #include <sbi/sbi_hart.h>
>  #include <sbi/sbi_math.h>
>  #include <sbi/sbi_platform.h>
> +#include <sbi/sbi_pmu.h>
>  #include <sbi/sbi_string.h>
>  #include <sbi/sbi_trap.h>
>
> @@ -208,8 +209,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_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> -               interrupts |= MIP_LCOFIP;
> +       interrupts |= sbi_pmu_irq_bit();
>
>         exceptions = (1U << CAUSE_MISALIGNED_FETCH) | (1U << CAUSE_BREAKPOINT) |
>                      (1U << CAUSE_USER_ECALL);
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 214d5e8..91d9ccc 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -344,6 +344,18 @@ skip_inhibit_update:
>         return 0;
>  }
>
> +int sbi_pmu_irq_bit(void)
> +{
> +       struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> +
> +       if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> +               return MIP_LCOFIP;
> +       if (pmu_dev && pmu_dev->hw_counter_irq_bit)
> +               return pmu_dev->hw_counter_irq_bit();
> +
> +       return 0;
> +}
> +
>  static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
>                             uint64_t ival, bool ival_update)
>  {
> --
> 2.35.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi


Reviewed-by: Atish Patra <atishp@rivosinc.com>

-- 
Regards,
Atish

[PATCH v4 3/5] lib: sbi_pmu: add override for counter data

[Heiko Stuebner]

In general counter-data is auto-detected but some platforms
may implement counters in a way that breaks this detection.

Implement an abstraction that those platforms can hook into
and override the counter-data.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 include/sbi/sbi_pmu.h | 8 ++++++++
 lib/sbi/sbi_hart.c    | 7 +++++++
 lib/sbi/sbi_pmu.c     | 7 +++++++
 3 files changed, 22 insertions(+)

diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index c365243..d257b14 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -78,6 +78,11 @@ struct sbi_pmu_device {
 	 * Custom function returning the machine-specific irq-bit.
 	 */
 	int (*hw_counter_irq_bit)(void);
+
+	/**
+	 * Override autodetected counter data.
+	 */
+	void (*hw_counter_data)(unsigned int *count, unsigned int *bits);
 };
 
 /** Get the PMU platform device */
@@ -95,6 +100,9 @@ void sbi_pmu_exit(struct sbi_scratch *scratch);
 /** Return the pmu irq bit depending on extension existence */
 int sbi_pmu_irq_bit(void);
 
+/** Allow non-standard platforms to override probed counter information */
+void sbi_pmu_override_counter_data(unsigned int *count, unsigned int *bits);
+
 /**
  * Add the hardware event to counter mapping information. This should be called
  * from the platform code to update the mapping table.
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 45fbcde..6506a19 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -632,6 +632,13 @@ __mhpm_skip:
 #undef __check_csr_2
 #undef __check_csr
 
+	/**
+	 * Allow non-standard implementations to override the detected
+	 * values for number of counters and bits.
+	 */
+	sbi_pmu_override_counter_data(&hfeatures->mhpm_count,
+				      &hfeatures->mhpm_bits);
+
 	/* Detect if hart supports Priv v1.10 */
 	val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
 	if (!trap.cause)
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 91d9ccc..c8becf3 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -852,3 +852,10 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
 
 	return 0;
 }
+
+void sbi_pmu_override_counter_data(unsigned int *count,
+				   unsigned int *bits)
+{
+	if (pmu_dev && pmu_dev->hw_counter_data)
+		pmu_dev->hw_counter_data(count, bits);
+}
-- 
2.35.1

[PATCH v4 3/5] lib: sbi_pmu: add override for counter data

[Guo Ren]

Reviewed-by: Guo Ren <guoren@kernel.org>

On Mon, Sep 26, 2022 at 6:16 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> In general counter-data is auto-detected but some platforms
> may implement counters in a way that breaks this detection.
>
> Implement an abstraction that those platforms can hook into
> and override the counter-data.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  include/sbi/sbi_pmu.h | 8 ++++++++
>  lib/sbi/sbi_hart.c    | 7 +++++++
>  lib/sbi/sbi_pmu.c     | 7 +++++++
>  3 files changed, 22 insertions(+)
>
> diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> index c365243..d257b14 100644
> --- a/include/sbi/sbi_pmu.h
> +++ b/include/sbi/sbi_pmu.h
> @@ -78,6 +78,11 @@ struct sbi_pmu_device {
>          * Custom function returning the machine-specific irq-bit.
>          */
>         int (*hw_counter_irq_bit)(void);
> +
> +       /**
> +        * Override autodetected counter data.
> +        */
> +       void (*hw_counter_data)(unsigned int *count, unsigned int *bits);
>  };
>
>  /** Get the PMU platform device */
> @@ -95,6 +100,9 @@ void sbi_pmu_exit(struct sbi_scratch *scratch);
>  /** Return the pmu irq bit depending on extension existence */
>  int sbi_pmu_irq_bit(void);
>
> +/** Allow non-standard platforms to override probed counter information */
> +void sbi_pmu_override_counter_data(unsigned int *count, unsigned int *bits);
> +
>  /**
>   * Add the hardware event to counter mapping information. This should be called
>   * from the platform code to update the mapping table.
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 45fbcde..6506a19 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -632,6 +632,13 @@ __mhpm_skip:
>  #undef __check_csr_2
>  #undef __check_csr
>
> +       /**
> +        * Allow non-standard implementations to override the detected
> +        * values for number of counters and bits.
> +        */
> +       sbi_pmu_override_counter_data(&hfeatures->mhpm_count,
> +                                     &hfeatures->mhpm_bits);
> +
>         /* Detect if hart supports Priv v1.10 */
>         val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
>         if (!trap.cause)
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 91d9ccc..c8becf3 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -852,3 +852,10 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
>
>         return 0;
>  }
> +
> +void sbi_pmu_override_counter_data(unsigned int *count,
> +                                  unsigned int *bits)
> +{
> +       if (pmu_dev && pmu_dev->hw_counter_data)
> +               pmu_dev->hw_counter_data(count, bits);
> +}
> --
> 2.35.1
>


-- 
Best Regards
 Guo Ren

[PATCH v4 3/5] lib: sbi_pmu: add override for counter data

[Atish Patra]

On Mon, Sep 26, 2022 at 3:18 AM Heiko Stuebner <heiko@sntech.de> wrote:
>
> In general counter-data is auto-detected but some platforms
> may implement counters in a way that breaks this detection.
>
> Implement an abstraction that those platforms can hook into
> and override the counter-data.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  include/sbi/sbi_pmu.h | 8 ++++++++
>  lib/sbi/sbi_hart.c    | 7 +++++++
>  lib/sbi/sbi_pmu.c     | 7 +++++++
>  3 files changed, 22 insertions(+)
>
> diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> index c365243..d257b14 100644
> --- a/include/sbi/sbi_pmu.h
> +++ b/include/sbi/sbi_pmu.h
> @@ -78,6 +78,11 @@ struct sbi_pmu_device {
>          * Custom function returning the machine-specific irq-bit.
>          */
>         int (*hw_counter_irq_bit)(void);
> +
> +       /**
> +        * Override autodetected counter data.
> +        */
> +       void (*hw_counter_data)(unsigned int *count, unsigned int *bits);
>  };
>
>  /** Get the PMU platform device */
> @@ -95,6 +100,9 @@ void sbi_pmu_exit(struct sbi_scratch *scratch);
>  /** Return the pmu irq bit depending on extension existence */
>  int sbi_pmu_irq_bit(void);
>
> +/** Allow non-standard platforms to override probed counter information */
> +void sbi_pmu_override_counter_data(unsigned int *count, unsigned int *bits);
> +
>  /**
>   * Add the hardware event to counter mapping information. This should be called
>   * from the platform code to update the mapping table.
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 45fbcde..6506a19 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -632,6 +632,13 @@ __mhpm_skip:
>  #undef __check_csr_2
>  #undef __check_csr
>
> +       /**
> +        * Allow non-standard implementations to override the detected
> +        * values for number of counters and bits.
> +        */
> +       sbi_pmu_override_counter_data(&hfeatures->mhpm_count,
> +                                     &hfeatures->mhpm_bits);
> +
>         /* Detect if hart supports Priv v1.10 */
>         val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
>         if (!trap.cause)
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 91d9ccc..c8becf3 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -852,3 +852,10 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
>
>         return 0;
>  }
> +
> +void sbi_pmu_override_counter_data(unsigned int *count,
> +                                  unsigned int *bits)
> +{
> +       if (pmu_dev && pmu_dev->hw_counter_data)
> +               pmu_dev->hw_counter_data(count, bits);
> +}
> --
> 2.35.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

We may need similar functionality in the future as well where the
platform may override the hart feature details.
Should we pass the hfeatures in the extension_init callback so that a
specific platform override can access it ?

-- 
Regards,
Atish

[PATCH v4 3/5] lib: sbi_pmu: add override for counter data

[Heiko Stübner]

Am Donnerstag, 29. September 2022, 10:28:57 CEST schrieb Atish Patra:
> On Mon, Sep 26, 2022 at 3:18 AM Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > In general counter-data is auto-detected but some platforms
> > may implement counters in a way that breaks this detection.
> >
> > Implement an abstraction that those platforms can hook into
> > and override the counter-data.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  include/sbi/sbi_pmu.h | 8 ++++++++
> >  lib/sbi/sbi_hart.c    | 7 +++++++
> >  lib/sbi/sbi_pmu.c     | 7 +++++++
> >  3 files changed, 22 insertions(+)
> >
> > diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> > index c365243..d257b14 100644
> > --- a/include/sbi/sbi_pmu.h
> > +++ b/include/sbi/sbi_pmu.h
> > @@ -78,6 +78,11 @@ struct sbi_pmu_device {
> >          * Custom function returning the machine-specific irq-bit.
> >          */
> >         int (*hw_counter_irq_bit)(void);
> > +
> > +       /**
> > +        * Override autodetected counter data.
> > +        */
> > +       void (*hw_counter_data)(unsigned int *count, unsigned int *bits);
> >  };
> >
> >  /** Get the PMU platform device */
> > @@ -95,6 +100,9 @@ void sbi_pmu_exit(struct sbi_scratch *scratch);
> >  /** Return the pmu irq bit depending on extension existence */
> >  int sbi_pmu_irq_bit(void);
> >
> > +/** Allow non-standard platforms to override probed counter information */
> > +void sbi_pmu_override_counter_data(unsigned int *count, unsigned int *bits);
> > +
> >  /**
> >   * Add the hardware event to counter mapping information. This should be called
> >   * from the platform code to update the mapping table.
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 45fbcde..6506a19 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -632,6 +632,13 @@ __mhpm_skip:
> >  #undef __check_csr_2
> >  #undef __check_csr
> >
> > +       /**
> > +        * Allow non-standard implementations to override the detected
> > +        * values for number of counters and bits.
> > +        */
> > +       sbi_pmu_override_counter_data(&hfeatures->mhpm_count,
> > +                                     &hfeatures->mhpm_bits);
> > +
> >         /* Detect if hart supports Priv v1.10 */
> >         val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
> >         if (!trap.cause)
> > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> > index 91d9ccc..c8becf3 100644
> > --- a/lib/sbi/sbi_pmu.c
> > +++ b/lib/sbi/sbi_pmu.c
> > @@ -852,3 +852,10 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
> >
> >         return 0;
> >  }
> > +
> > +void sbi_pmu_override_counter_data(unsigned int *count,
> > +                                  unsigned int *bits)
> > +{
> > +       if (pmu_dev && pmu_dev->hw_counter_data)
> > +               pmu_dev->hw_counter_data(count, bits);
> > +}
> > --
> > 2.35.1
> >
> >
> > --
> > opensbi mailing list
> > opensbi at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
> 
> We may need similar functionality in the future as well where the
> platform may override the hart feature details.
> Should we pass the hfeatures in the extension_init callback so that a
> specific platform override can access it ?

you tell me :-) .

I.e. I didn't want to expose the previously unexposed hfeatures struct
(right now it's local to sbi_hart) but if it's ok to expose it, directly
setting the hfeatures values of course makes a lot of sense also for
future uses.


Heiko

[PATCH v4 4/5] platform: generic: add extensions_init handler and platform-override

[Heiko Stuebner]

Init of non-standard extensions is a platform-specific thing,
so allow generic platforms to do this via a platform-override.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 platform/generic/include/platform_override.h | 1 +
 platform/generic/platform.c                  | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/platform/generic/include/platform_override.h b/platform/generic/include/platform_override.h
index e55da25..06a26dd 100644
--- a/platform/generic/include/platform_override.h
+++ b/platform/generic/include/platform_override.h
@@ -22,6 +22,7 @@ struct platform_override {
 	void (*early_exit)(const struct fdt_match *match);
 	void (*final_exit)(const struct fdt_match *match);
 	int (*fdt_fixup)(void *fdt, const struct fdt_match *match);
+	int (*extensions_init)(const struct fdt_match *match);
 	int (*vendor_ext_check)(long extid, const struct fdt_match *match);
 	int (*vendor_ext_provider)(long extid, long funcid,
 				   const struct sbi_trap_regs *regs,
diff --git a/platform/generic/platform.c b/platform/generic/platform.c
index bf51aba..35545fb 100644
--- a/platform/generic/platform.c
+++ b/platform/generic/platform.c
@@ -204,6 +204,14 @@ static void generic_final_exit(void)
 		generic_plat->final_exit(generic_plat_match);
 }
 
+static int generic_extensions_init(void)
+{
+	if (generic_plat && generic_plat->extensions_init)
+		return generic_plat->extensions_init(generic_plat_match);
+
+	return 0;
+}
+
 static int generic_domains_init(void)
 {
 	return fdt_domains_populate(fdt_get_address());
@@ -257,6 +265,7 @@ const struct sbi_platform_operations platform_ops = {
 	.final_init		= generic_final_init,
 	.early_exit		= generic_early_exit,
 	.final_exit		= generic_final_exit,
+	.extensions_init	= generic_extensions_init,
 	.domains_init		= generic_domains_init,
 	.console_init		= generic_console_init,
 	.irqchip_init		= fdt_irqchip_init,
-- 
2.35.1

[PATCH v4 4/5] platform: generic: add extensions_init handler and platform-override

[Guo Ren]

Acked-by: Guo Ren <guoren@kernel.org>

On Mon, Sep 26, 2022 at 6:16 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> Init of non-standard extensions is a platform-specific thing,
> so allow generic platforms to do this via a platform-override.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  platform/generic/include/platform_override.h | 1 +
>  platform/generic/platform.c                  | 9 +++++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/platform/generic/include/platform_override.h b/platform/generic/include/platform_override.h
> index e55da25..06a26dd 100644
> --- a/platform/generic/include/platform_override.h
> +++ b/platform/generic/include/platform_override.h
> @@ -22,6 +22,7 @@ struct platform_override {
>         void (*early_exit)(const struct fdt_match *match);
>         void (*final_exit)(const struct fdt_match *match);
>         int (*fdt_fixup)(void *fdt, const struct fdt_match *match);
> +       int (*extensions_init)(const struct fdt_match *match);
>         int (*vendor_ext_check)(long extid, const struct fdt_match *match);
>         int (*vendor_ext_provider)(long extid, long funcid,
>                                    const struct sbi_trap_regs *regs,
> diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> index bf51aba..35545fb 100644
> --- a/platform/generic/platform.c
> +++ b/platform/generic/platform.c
> @@ -204,6 +204,14 @@ static void generic_final_exit(void)
>                 generic_plat->final_exit(generic_plat_match);
>  }
>
> +static int generic_extensions_init(void)
> +{
> +       if (generic_plat && generic_plat->extensions_init)
> +               return generic_plat->extensions_init(generic_plat_match);
> +
> +       return 0;
> +}
> +
>  static int generic_domains_init(void)
>  {
>         return fdt_domains_populate(fdt_get_address());
> @@ -257,6 +265,7 @@ const struct sbi_platform_operations platform_ops = {
>         .final_init             = generic_final_init,
>         .early_exit             = generic_early_exit,
>         .final_exit             = generic_final_exit,
> +       .extensions_init        = generic_extensions_init,
>         .domains_init           = generic_domains_init,
>         .console_init           = generic_console_init,
>         .irqchip_init           = fdt_irqchip_init,
> --
> 2.35.1
>


-- 
Best Regards
 Guo Ren

[PATCH v4 5/5] platform: generic: allwinner: add support for c9xx pmu

[Heiko Stuebner]

With the T-HEAD C9XX cores being designed before or during ratification
of the SSCOFPMF extension, they implement a PMU extension that behaves
very similar but not equal to it by providing overflow interrupts though
in a slightly different registers format.

The sun20i-d1 is using this core. So implement the necessary overrides
to allow its pmu to be used via the standard sbi-pmu extension.

For now it's also the only soc using this core, so keep the additional
code in the d1-space for now.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 platform/generic/allwinner/sun20i-d1.c |  72 ++++++++++++++
 platform/generic/include/thead_c9xx.h  | 127 +++++++++++++++++++++++++
 2 files changed, 199 insertions(+)
 create mode 100644 platform/generic/include/thead_c9xx.h

diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
index 5b2656c..a624b8b 100644
--- a/platform/generic/allwinner/sun20i-d1.c
+++ b/platform/generic/allwinner/sun20i-d1.c
@@ -5,11 +5,13 @@
  */
 
 #include <platform_override.h>
+#include <thead_c9xx.h>
 #include <sbi/riscv_io.h>
 #include <sbi/sbi_bitops.h>
 #include <sbi/sbi_ecall_interface.h>
 #include <sbi/sbi_error.h>
 #include <sbi/sbi_hsm.h>
+#include <sbi/sbi_pmu.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/irqchip/fdt_irqchip_plic.h>
 
@@ -199,6 +201,75 @@ static int sun20i_d1_final_init(bool cold_boot, const struct fdt_match *match)
 	return 0;
 }
 
+static void thead_c9xx_pmu_ctr_enable_irq(uint32_t ctr_idx)
+{
+	unsigned long val;
+	unsigned long mip_val;
+
+	if (ctr_idx >= SBI_PMU_HW_CTR_MAX)
+		return;
+
+	mip_val = csr_read(CSR_MIP);
+	/**
+	 * Clear out the OF bit so that next interrupt can be enabled.
+	 * This should be done only when the corresponding overflow interrupt
+	 * bit is cleared. That indicates that software has already handled the
+	 * previous interrupts or the hardware yet to set an overflow interrupt.
+	 * Otherwise, there will be race conditions where we may clear the bit
+	 * the software is yet to handle the interrupt.
+	 */
+	if (!(mip_val & THEAD_C9XX_MIP_MOIP)) {
+		val = csr_read(THEAD_C9XX_CSR_MCOUNTEROF);
+		val &= ~(1 << ctr_idx);
+		csr_write(THEAD_C9XX_CSR_MCOUNTEROF, val);
+	}
+
+	/**
+	 * SSCOFPMF uses the OF bit for enabling/disabling the interrupt,
+	 * while the C9XX has designated enable bits.
+	 * So enable per-counter interrupt on C9xx here.
+	 */
+	val = csr_read(THEAD_C9XX_CSR_MCOUNTERINTEN);
+	val |= (1 << ctr_idx);
+	csr_write(THEAD_C9XX_CSR_MCOUNTERINTEN, val);
+}
+
+static void thead_c9xx_pmu_ctr_disable_irq(uint32_t ctr_idx)
+{
+	unsigned long val;
+
+	val = csr_read(THEAD_C9XX_CSR_MCOUNTERINTEN);
+	val &= ~(1 << ctr_idx);
+	csr_write(THEAD_C9XX_CSR_MCOUNTERINTEN, val);
+}
+
+static int thead_c9xx_pmu_irq_bit(void)
+{
+	return THEAD_C9XX_MIP_MOIP;
+}
+
+static void thead_c9xx_pmu_counter_data(unsigned int *count,
+					unsigned int *bits)
+{
+	/* auto-detection doesn't work on t-head c9xx cores */
+	*count = 29;
+	*bits = 64;
+}
+
+const struct sbi_pmu_device thead_c9xx_pmu_device = {
+	.hw_counter_enable_irq = thead_c9xx_pmu_ctr_enable_irq,
+	.hw_counter_disable_irq = thead_c9xx_pmu_ctr_disable_irq,
+	.hw_counter_irq_bit = thead_c9xx_pmu_irq_bit,
+	.hw_counter_data = thead_c9xx_pmu_counter_data,
+};
+
+static int sun20i_d1_extensions_init(const struct fdt_match *match)
+{
+	sbi_pmu_set_device(&thead_c9xx_pmu_device);
+
+	return 0;
+}
+
 static const struct fdt_match sun20i_d1_match[] = {
 	{ .compatible = "allwinner,sun20i-d1" },
 	{ },
@@ -207,4 +278,5 @@ static const struct fdt_match sun20i_d1_match[] = {
 const struct platform_override sun20i_d1 = {
 	.match_table	= sun20i_d1_match,
 	.final_init	= sun20i_d1_final_init,
+	.extensions_init = sun20i_d1_extensions_init,
 };
diff --git a/platform/generic/include/thead_c9xx.h b/platform/generic/include/thead_c9xx.h
new file mode 100644
index 0000000..bab0408
--- /dev/null
+++ b/platform/generic/include/thead_c9xx.h
@@ -0,0 +1,127 @@
+#ifndef __RISCV_THEAD_C9XX_H____
+#define __RISCV_THEAD_C9XX_H____
+
+/* T-HEAD C9xx M mode CSR.  */
+#define THEAD_C9XX_CSR_MXSTATUS		0x7c0
+#define THEAD_C9XX_CSR_MHCR		0x7c1
+#define THEAD_C9XX_CSR_MCOR		0x7c2
+#define THEAD_C9XX_CSR_MCCR2		0x7c3
+#define THEAD_C9XX_CSR_MCER2		0x7c4
+#define THEAD_C9XX_CSR_MHINT		0x7c5
+#define THEAD_C9XX_CSR_MRMR		0x7c6
+#define THEAD_C9XX_CSR_MRVBR		0x7c7
+#define THEAD_C9XX_CSR_MCER		0x7c8
+#define THEAD_C9XX_CSR_MCOUNTERWEN	0x7c9
+#define THEAD_C9XX_CSR_MCOUNTERINTEN	0x7ca
+#define THEAD_C9XX_CSR_MCOUNTEROF	0x7cb
+#define THEAD_C9XX_CSR_MHINT2		0x7cc
+#define THEAD_C9XX_CSR_MHINT3		0x7cd
+#define THEAD_C9XX_CSR_MRADDR		0x7e0
+#define THEAD_C9XX_CSR_MEXSTATUS	0x7e1
+#define THEAD_C9XX_CSR_MNMICAUSE	0x7e2
+#define THEAD_C9XX_CSR_MNMIPC		0x7e3
+#define THEAD_C9XX_CSR_MHPMCR		0x7f0
+#define THEAD_C9XX_CSR_MHPMSR		0x7f1
+#define THEAD_C9XX_CSR_MHPMER		0x7f2
+#define THEAD_C9XX_CSR_MSMPR		0x7f3
+#define THEAD_C9XX_CSR_MTEECFG		0x7f4
+#define THEAD_C9XX_CSR_MZONEID		0x7f5
+#define THEAD_C9XX_CSR_ML2CPID		0x7f6
+#define THEAD_C9XX_CSR_ML2WP		0x7f7
+#define THEAD_C9XX_CSR_MDTCMCR		0x7f8
+#define THEAD_C9XX_CSR_USP		0x7d1
+#define THEAD_C9XX_CSR_MCINS		0x7d2
+#define THEAD_C9XX_CSR_MCINDEX		0x7d3
+#define THEAD_C9XX_CSR_MCDATA0		0x7d4
+#define THEAD_C9XX_CSR_MCDATA1		0x7d5
+#define THEAD_C9XX_CSR_MEICR		0x7d6
+#define THEAD_C9XX_CSR_MEICR2		0x7d7
+#define THEAD_C9XX_CSR_MBEADDR		0x7d8
+#define THEAD_C9XX_CSR_MCPUID		0xfc0
+#define THEAD_C9XX_CSR_MAPBADDR		0xfc1
+#define THEAD_C9XX_CSR_MWMSR		0xfc2
+#define THEAD_C9XX_CSR_MHALTCAUSE	0xfe0
+#define THEAD_C9XX_CSR_MDBGINFO		0xfe1
+#define THEAD_C9XX_CSR_MPCFIFO		0xfe2
+
+/* T-HEAD C9xx S mode CSR.  */
+#define THEAD_C9XX_CSR_SXSTATUS		0x5c0
+#define THEAD_C9XX_CSR_SHCR		0x5c1
+#define THEAD_C9XX_CSR_SCER2		0x5c2
+#define THEAD_C9XX_CSR_SCER		0x5c3
+#define THEAD_C9XX_CSR_SCOUNTERINTEN	0x5c4
+#define THEAD_C9XX_CSR_SCOUNTEROF	0x5c5
+#define THEAD_C9XX_CSR_SHINT		0x5c6
+#define THEAD_C9XX_CSR_SHINT2		0x5c7
+#define THEAD_C9XX_CSR_SHPMINHIBIT	0x5c8
+#define THEAD_C9XX_CSR_SHPMCR		0x5c9
+#define THEAD_C9XX_CSR_SHPMSR		0x5ca
+#define THEAD_C9XX_CSR_SHPMER		0x5cb
+#define THEAD_C9XX_CSR_SL2CPID		0x5cc
+#define THEAD_C9XX_CSR_SL2WP		0x5cd
+#define THEAD_C9XX_CSR_SBEADDR		0x5d0
+#define THEAD_C9XX_CSR_SCYCLE		0x5e0
+#define THEAD_C9XX_CSR_SHPMCOUNTER1	0x5e1
+#define THEAD_C9XX_CSR_SHPMCOUNTER2	0x5e2
+#define THEAD_C9XX_CSR_SHPMCOUNTER3	0x5e3
+#define THEAD_C9XX_CSR_SHPMCOUNTER4	0x5e4
+#define THEAD_C9XX_CSR_SHPMCOUNTER5	0x5e5
+#define THEAD_C9XX_CSR_SHPMCOUNTER6	0x5e6
+#define THEAD_C9XX_CSR_SHPMCOUNTER7	0x5e7
+#define THEAD_C9XX_CSR_SHPMCOUNTER8	0x5e8
+#define THEAD_C9XX_CSR_SHPMCOUNTER9	0x5e9
+#define THEAD_C9XX_CSR_SHPMCOUNTER10	0x5ea
+#define THEAD_C9XX_CSR_SHPMCOUNTER11	0x5eb
+#define THEAD_C9XX_CSR_SHPMCOUNTER12	0x5ec
+#define THEAD_C9XX_CSR_SHPMCOUNTER13	0x5ed
+#define THEAD_C9XX_CSR_SHPMCOUNTER14	0x5ee
+#define THEAD_C9XX_CSR_SHPMCOUNTER15	0x5ef
+#define THEAD_C9XX_CSR_SHPMCOUNTER16	0x5f0
+#define THEAD_C9XX_CSR_SHPMCOUNTER17	0x5f1
+#define THEAD_C9XX_CSR_SHPMCOUNTER18	0x5f2
+#define THEAD_C9XX_CSR_SHPMCOUNTER19	0x5f3
+#define THEAD_C9XX_CSR_SHPMCOUNTER20	0x5f4
+#define THEAD_C9XX_CSR_SHPMCOUNTER21	0x5f5
+#define THEAD_C9XX_CSR_SHPMCOUNTER22	0x5f6
+#define THEAD_C9XX_CSR_SHPMCOUNTER23	0x5f7
+#define THEAD_C9XX_CSR_SHPMCOUNTER24	0x5f8
+#define THEAD_C9XX_CSR_SHPMCOUNTER25	0x5f9
+#define THEAD_C9XX_CSR_SHPMCOUNTER26	0x5fa
+#define THEAD_C9XX_CSR_SHPMCOUNTER27	0x5fb
+#define THEAD_C9XX_CSR_SHPMCOUNTER28	0x5fc
+#define THEAD_C9XX_CSR_SHPMCOUNTER29	0x5fd
+#define THEAD_C9XX_CSR_SHPMCOUNTER30	0x5fe
+#define THEAD_C9XX_CSR_SHPMCOUNTER31	0x5ff
+
+/* T-HEAD C9xx U mode CSR.  */
+#define THEAD_C9XX_CSR_FXCR		0x800
+
+/* T-HEAD C9xx MMU extentions.  */
+#define THEAD_C9XX_CSR_SMIR		0x9c0
+#define THEAD_C9XX_CSR_SMEL		0x9c1
+#define THEAD_C9XX_CSR_SMEH		0x9c2
+#define THEAD_C9XX_CSR_SMCIR		0x9c3
+
+/* T-HEAD C9xx Security CSR(May be droped).  */
+#define THEAD_C9XX_CSR_MEBR		0xbe0
+#define THEAD_C9XX_CSR_NT_MSTATUS	0xbe1
+#define THEAD_C9XX_CSR_NT_MIE		0xbe2
+#define THEAD_C9XX_CSR_NT_MTVEC		0xbe3
+#define THEAD_C9XX_CSR_NT_MTVT		0xbe4
+#define THEAD_C9XX_CSR_NT_MEPC		0xbe5
+#define THEAD_C9XX_CSR_NT_MCAUSE	0xbe6
+#define THEAD_C9XX_CSR_NT_MIP		0xbe7
+#define THEAD_C9XX_CSR_NT_MINTSTATE	0xbe8
+#define THEAD_C9XX_CSR_NT_MXSTATUS	0xbe9
+#define THEAD_C9XX_CSR_NT_MEBR		0xbea
+#define THEAD_C9XX_CSR_NT_MSP		0xbeb
+#define THEAD_C9XX_CSR_T_USP		0xbec
+#define THEAD_C9XX_CSR_T_MDCR		0xbed
+#define THEAD_C9XX_CSR_T_MPCR		0xbee
+#define THEAD_C9XX_CSR_PMPTEECFG	0xbef
+
+/* T-HEAD C9xx MIP CSR extension */
+#define THEAD_C9XX_IRQ_PMU_OVF		17
+#define THEAD_C9XX_MIP_MOIP		(_UL(1) << THEAD_C9XX_IRQ_PMU_OVF)
+
+#endif
-- 
2.35.1

[PATCH v4 5/5] platform: generic: allwinner: add support for c9xx pmu

[Guo Ren]

On Mon, Sep 26, 2022 at 6:16 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> With the T-HEAD C9XX cores being designed before or during ratification
> of the SSCOFPMF extension, they implement a PMU extension that behaves
> very similar but not equal to it by providing overflow interrupts though
> in a slightly different registers format.
>
> The sun20i-d1 is using this core. So implement the necessary overrides
> to allow its pmu to be used via the standard sbi-pmu extension.
>
> For now it's also the only soc using this core, so keep the additional
> code in the d1-space for now.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  platform/generic/allwinner/sun20i-d1.c |  72 ++++++++++++++
>  platform/generic/include/thead_c9xx.h  | 127 +++++++++++++++++++++++++
>  2 files changed, 199 insertions(+)
>  create mode 100644 platform/generic/include/thead_c9xx.h
>
> diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
> index 5b2656c..a624b8b 100644
> --- a/platform/generic/allwinner/sun20i-d1.c
> +++ b/platform/generic/allwinner/sun20i-d1.c
> @@ -5,11 +5,13 @@
>   */
>
>  #include <platform_override.h>
> +#include <thead_c9xx.h>
>  #include <sbi/riscv_io.h>
>  #include <sbi/sbi_bitops.h>
>  #include <sbi/sbi_ecall_interface.h>
>  #include <sbi/sbi_error.h>
>  #include <sbi/sbi_hsm.h>
> +#include <sbi/sbi_pmu.h>
>  #include <sbi_utils/fdt/fdt_helper.h>
>  #include <sbi_utils/irqchip/fdt_irqchip_plic.h>
>
> @@ -199,6 +201,75 @@ static int sun20i_d1_final_init(bool cold_boot, const struct fdt_match *match)
>         return 0;
>  }
>
> +static void thead_c9xx_pmu_ctr_enable_irq(uint32_t ctr_idx)
> +{
> +       unsigned long val;
> +       unsigned long mip_val;
> +
> +       if (ctr_idx >= SBI_PMU_HW_CTR_MAX)
> +               return;
> +
> +       mip_val = csr_read(CSR_MIP);
> +       /**
> +        * Clear out the OF bit so that next interrupt can be enabled.
> +        * This should be done only when the corresponding overflow interrupt
> +        * bit is cleared. That indicates that software has already handled the
> +        * previous interrupts or the hardware yet to set an overflow interrupt.
> +        * Otherwise, there will be race conditions where we may clear the bit
> +        * the software is yet to handle the interrupt.
> +        */
> +       if (!(mip_val & THEAD_C9XX_MIP_MOIP)) {
> +               val = csr_read(THEAD_C9XX_CSR_MCOUNTEROF);
> +               val &= ~(1 << ctr_idx);
> +               csr_write(THEAD_C9XX_CSR_MCOUNTEROF, val);
> +       }
> +
> +       /**
> +        * SSCOFPMF uses the OF bit for enabling/disabling the interrupt,
> +        * while the C9XX has designated enable bits.
> +        * So enable per-counter interrupt on C9xx here.
> +        */
> +       val = csr_read(THEAD_C9XX_CSR_MCOUNTERINTEN);
> +       val |= (1 << ctr_idx);
> +       csr_write(THEAD_C9XX_CSR_MCOUNTERINTEN, val);
Use csr_set

> +}
> +
> +static void thead_c9xx_pmu_ctr_disable_irq(uint32_t ctr_idx)
> +{
> +       unsigned long val;
> +
> +       val = csr_read(THEAD_C9XX_CSR_MCOUNTERINTEN);
> +       val &= ~(1 << ctr_idx);
> +       csr_write(THEAD_C9XX_CSR_MCOUNTERINTEN, val);
csr_clear

> +}
> +
> +static int thead_c9xx_pmu_irq_bit(void)
> +{
> +       return THEAD_C9XX_MIP_MOIP;
> +}
> +
> +static void thead_c9xx_pmu_counter_data(unsigned int *count,
> +                                       unsigned int *bits)
> +{
> +       /* auto-detection doesn't work on t-head c9xx cores */
> +       *count = 29;
> +       *bits = 64;
> +}
> +
> +const struct sbi_pmu_device thead_c9xx_pmu_device = {
> +       .hw_counter_enable_irq = thead_c9xx_pmu_ctr_enable_irq,
> +       .hw_counter_disable_irq = thead_c9xx_pmu_ctr_disable_irq,
> +       .hw_counter_irq_bit = thead_c9xx_pmu_irq_bit,
> +       .hw_counter_data = thead_c9xx_pmu_counter_data,
> +};
> +
> +static int sun20i_d1_extensions_init(const struct fdt_match *match)
> +{
> +       sbi_pmu_set_device(&thead_c9xx_pmu_device);
> +
> +       return 0;
> +}
> +
>  static const struct fdt_match sun20i_d1_match[] = {
>         { .compatible = "allwinner,sun20i-d1" },
>         { },
> @@ -207,4 +278,5 @@ static const struct fdt_match sun20i_d1_match[] = {
>  const struct platform_override sun20i_d1 = {
>         .match_table    = sun20i_d1_match,
>         .final_init     = sun20i_d1_final_init,
> +       .extensions_init = sun20i_d1_extensions_init,
>  };
> diff --git a/platform/generic/include/thead_c9xx.h b/platform/generic/include/thead_c9xx.h
> new file mode 100644
> index 0000000..bab0408
> --- /dev/null
> +++ b/platform/generic/include/thead_c9xx.h
> @@ -0,0 +1,127 @@
> +#ifndef __RISCV_THEAD_C9XX_H____
> +#define __RISCV_THEAD_C9XX_H____
> +
> +/* T-HEAD C9xx M mode CSR.  */
> +#define THEAD_C9XX_CSR_MXSTATUS                0x7c0
> +#define THEAD_C9XX_CSR_MHCR            0x7c1
> +#define THEAD_C9XX_CSR_MCOR            0x7c2
> +#define THEAD_C9XX_CSR_MCCR2           0x7c3
> +#define THEAD_C9XX_CSR_MCER2           0x7c4
> +#define THEAD_C9XX_CSR_MHINT           0x7c5
> +#define THEAD_C9XX_CSR_MRMR            0x7c6
> +#define THEAD_C9XX_CSR_MRVBR           0x7c7
> +#define THEAD_C9XX_CSR_MCER            0x7c8
> +#define THEAD_C9XX_CSR_MCOUNTERWEN     0x7c9
> +#define THEAD_C9XX_CSR_MCOUNTERINTEN   0x7ca
> +#define THEAD_C9XX_CSR_MCOUNTEROF      0x7cb
> +#define THEAD_C9XX_CSR_MHINT2          0x7cc
> +#define THEAD_C9XX_CSR_MHINT3          0x7cd
> +#define THEAD_C9XX_CSR_MRADDR          0x7e0
> +#define THEAD_C9XX_CSR_MEXSTATUS       0x7e1
> +#define THEAD_C9XX_CSR_MNMICAUSE       0x7e2
> +#define THEAD_C9XX_CSR_MNMIPC          0x7e3
> +#define THEAD_C9XX_CSR_MHPMCR          0x7f0
> +#define THEAD_C9XX_CSR_MHPMSR          0x7f1
> +#define THEAD_C9XX_CSR_MHPMER          0x7f2
> +#define THEAD_C9XX_CSR_MSMPR           0x7f3
> +#define THEAD_C9XX_CSR_MTEECFG         0x7f4
> +#define THEAD_C9XX_CSR_MZONEID         0x7f5
> +#define THEAD_C9XX_CSR_ML2CPID         0x7f6
> +#define THEAD_C9XX_CSR_ML2WP           0x7f7
> +#define THEAD_C9XX_CSR_MDTCMCR         0x7f8
> +#define THEAD_C9XX_CSR_USP             0x7d1
> +#define THEAD_C9XX_CSR_MCINS           0x7d2
> +#define THEAD_C9XX_CSR_MCINDEX         0x7d3
> +#define THEAD_C9XX_CSR_MCDATA0         0x7d4
> +#define THEAD_C9XX_CSR_MCDATA1         0x7d5
> +#define THEAD_C9XX_CSR_MEICR           0x7d6
> +#define THEAD_C9XX_CSR_MEICR2          0x7d7
> +#define THEAD_C9XX_CSR_MBEADDR         0x7d8
> +#define THEAD_C9XX_CSR_MCPUID          0xfc0
> +#define THEAD_C9XX_CSR_MAPBADDR                0xfc1
> +#define THEAD_C9XX_CSR_MWMSR           0xfc2
> +#define THEAD_C9XX_CSR_MHALTCAUSE      0xfe0
> +#define THEAD_C9XX_CSR_MDBGINFO                0xfe1
> +#define THEAD_C9XX_CSR_MPCFIFO         0xfe2
> +
> +/* T-HEAD C9xx S mode CSR.  */
> +#define THEAD_C9XX_CSR_SXSTATUS                0x5c0
> +#define THEAD_C9XX_CSR_SHCR            0x5c1
> +#define THEAD_C9XX_CSR_SCER2           0x5c2
> +#define THEAD_C9XX_CSR_SCER            0x5c3
> +#define THEAD_C9XX_CSR_SCOUNTERINTEN   0x5c4
> +#define THEAD_C9XX_CSR_SCOUNTEROF      0x5c5
> +#define THEAD_C9XX_CSR_SHINT           0x5c6
> +#define THEAD_C9XX_CSR_SHINT2          0x5c7
> +#define THEAD_C9XX_CSR_SHPMINHIBIT     0x5c8
> +#define THEAD_C9XX_CSR_SHPMCR          0x5c9
> +#define THEAD_C9XX_CSR_SHPMSR          0x5ca
> +#define THEAD_C9XX_CSR_SHPMER          0x5cb
> +#define THEAD_C9XX_CSR_SL2CPID         0x5cc
> +#define THEAD_C9XX_CSR_SL2WP           0x5cd
> +#define THEAD_C9XX_CSR_SBEADDR         0x5d0
> +#define THEAD_C9XX_CSR_SCYCLE          0x5e0
> +#define THEAD_C9XX_CSR_SHPMCOUNTER1    0x5e1
> +#define THEAD_C9XX_CSR_SHPMCOUNTER2    0x5e2
> +#define THEAD_C9XX_CSR_SHPMCOUNTER3    0x5e3
> +#define THEAD_C9XX_CSR_SHPMCOUNTER4    0x5e4
> +#define THEAD_C9XX_CSR_SHPMCOUNTER5    0x5e5
> +#define THEAD_C9XX_CSR_SHPMCOUNTER6    0x5e6
> +#define THEAD_C9XX_CSR_SHPMCOUNTER7    0x5e7
> +#define THEAD_C9XX_CSR_SHPMCOUNTER8    0x5e8
> +#define THEAD_C9XX_CSR_SHPMCOUNTER9    0x5e9
> +#define THEAD_C9XX_CSR_SHPMCOUNTER10   0x5ea
> +#define THEAD_C9XX_CSR_SHPMCOUNTER11   0x5eb
> +#define THEAD_C9XX_CSR_SHPMCOUNTER12   0x5ec
> +#define THEAD_C9XX_CSR_SHPMCOUNTER13   0x5ed
> +#define THEAD_C9XX_CSR_SHPMCOUNTER14   0x5ee
> +#define THEAD_C9XX_CSR_SHPMCOUNTER15   0x5ef
> +#define THEAD_C9XX_CSR_SHPMCOUNTER16   0x5f0
> +#define THEAD_C9XX_CSR_SHPMCOUNTER17   0x5f1
> +#define THEAD_C9XX_CSR_SHPMCOUNTER18   0x5f2
> +#define THEAD_C9XX_CSR_SHPMCOUNTER19   0x5f3
> +#define THEAD_C9XX_CSR_SHPMCOUNTER20   0x5f4
> +#define THEAD_C9XX_CSR_SHPMCOUNTER21   0x5f5
> +#define THEAD_C9XX_CSR_SHPMCOUNTER22   0x5f6
> +#define THEAD_C9XX_CSR_SHPMCOUNTER23   0x5f7
> +#define THEAD_C9XX_CSR_SHPMCOUNTER24   0x5f8
> +#define THEAD_C9XX_CSR_SHPMCOUNTER25   0x5f9
> +#define THEAD_C9XX_CSR_SHPMCOUNTER26   0x5fa
> +#define THEAD_C9XX_CSR_SHPMCOUNTER27   0x5fb
> +#define THEAD_C9XX_CSR_SHPMCOUNTER28   0x5fc
> +#define THEAD_C9XX_CSR_SHPMCOUNTER29   0x5fd
> +#define THEAD_C9XX_CSR_SHPMCOUNTER30   0x5fe
> +#define THEAD_C9XX_CSR_SHPMCOUNTER31   0x5ff
Have we used the above CSR_SH* CSR in the patchset series?

> +
> +/* T-HEAD C9xx U mode CSR.  */
> +#define THEAD_C9XX_CSR_FXCR            0x800
It's about the float point register, maybe it should not contain in
the PMU patch.

> +
> +/* T-HEAD C9xx MMU extentions.  */
> +#define THEAD_C9XX_CSR_SMIR            0x9c0
> +#define THEAD_C9XX_CSR_SMEL            0x9c1
> +#define THEAD_C9XX_CSR_SMEH            0x9c2
> +#define THEAD_C9XX_CSR_SMCIR           0x9c3
Ditto.

> +
> +/* T-HEAD C9xx Security CSR(May be droped).  */
> +#define THEAD_C9XX_CSR_MEBR            0xbe0
> +#define THEAD_C9XX_CSR_NT_MSTATUS      0xbe1
> +#define THEAD_C9XX_CSR_NT_MIE          0xbe2
> +#define THEAD_C9XX_CSR_NT_MTVEC                0xbe3
> +#define THEAD_C9XX_CSR_NT_MTVT         0xbe4
> +#define THEAD_C9XX_CSR_NT_MEPC         0xbe5
> +#define THEAD_C9XX_CSR_NT_MCAUSE       0xbe6
> +#define THEAD_C9XX_CSR_NT_MIP          0xbe7
> +#define THEAD_C9XX_CSR_NT_MINTSTATE    0xbe8
> +#define THEAD_C9XX_CSR_NT_MXSTATUS     0xbe9
> +#define THEAD_C9XX_CSR_NT_MEBR         0xbea
> +#define THEAD_C9XX_CSR_NT_MSP          0xbeb
> +#define THEAD_C9XX_CSR_T_USP           0xbec
> +#define THEAD_C9XX_CSR_T_MDCR          0xbed
> +#define THEAD_C9XX_CSR_T_MPCR          0xbee
> +#define THEAD_C9XX_CSR_PMPTEECFG       0xbef
Ditto

> +
> +/* T-HEAD C9xx MIP CSR extension */
> +#define THEAD_C9XX_IRQ_PMU_OVF         17
> +#define THEAD_C9XX_MIP_MOIP            (_UL(1) << THEAD_C9XX_IRQ_PMU_OVF)
> +
> +#endif
> --
> 2.35.1
>


-- 
Best Regards
 Guo Ren

[PATCH v4 5/5] platform: generic: allwinner: add support for c9xx pmu

[Heiko Stübner]

Hi,

Am Dienstag, 27. September 2022, 01:30:15 CEST schrieb Guo Ren:
> On Mon, Sep 26, 2022 at 6:16 PM Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > With the T-HEAD C9XX cores being designed before or during ratification
> > of the SSCOFPMF extension, they implement a PMU extension that behaves
> > very similar but not equal to it by providing overflow interrupts though
> > in a slightly different registers format.
> >
> > The sun20i-d1 is using this core. So implement the necessary overrides
> > to allow its pmu to be used via the standard sbi-pmu extension.
> >
> > For now it's also the only soc using this core, so keep the additional
> > code in the d1-space for now.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  platform/generic/allwinner/sun20i-d1.c |  72 ++++++++++++++
> >  platform/generic/include/thead_c9xx.h  | 127 +++++++++++++++++++++++++
> >  2 files changed, 199 insertions(+)
> >  create mode 100644 platform/generic/include/thead_c9xx.h
> >
> > diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
> > index 5b2656c..a624b8b 100644
> > --- a/platform/generic/allwinner/sun20i-d1.c
> > +++ b/platform/generic/allwinner/sun20i-d1.c
> > @@ -5,11 +5,13 @@
> >   */
> >
> >  #include <platform_override.h>
> > +#include <thead_c9xx.h>
> >  #include <sbi/riscv_io.h>
> >  #include <sbi/sbi_bitops.h>
> >  #include <sbi/sbi_ecall_interface.h>
> >  #include <sbi/sbi_error.h>
> >  #include <sbi/sbi_hsm.h>
> > +#include <sbi/sbi_pmu.h>
> >  #include <sbi_utils/fdt/fdt_helper.h>
> >  #include <sbi_utils/irqchip/fdt_irqchip_plic.h>
> >
> > @@ -199,6 +201,75 @@ static int sun20i_d1_final_init(bool cold_boot, const struct fdt_match *match)
> >         return 0;
> >  }
> >
> > +static void thead_c9xx_pmu_ctr_enable_irq(uint32_t ctr_idx)
> > +{
> > +       unsigned long val;
> > +       unsigned long mip_val;
> > +
> > +       if (ctr_idx >= SBI_PMU_HW_CTR_MAX)
> > +               return;
> > +
> > +       mip_val = csr_read(CSR_MIP);
> > +       /**
> > +        * Clear out the OF bit so that next interrupt can be enabled.
> > +        * This should be done only when the corresponding overflow interrupt
> > +        * bit is cleared. That indicates that software has already handled the
> > +        * previous interrupts or the hardware yet to set an overflow interrupt.
> > +        * Otherwise, there will be race conditions where we may clear the bit
> > +        * the software is yet to handle the interrupt.
> > +        */
> > +       if (!(mip_val & THEAD_C9XX_MIP_MOIP)) {
> > +               val = csr_read(THEAD_C9XX_CSR_MCOUNTEROF);
> > +               val &= ~(1 << ctr_idx);
> > +               csr_write(THEAD_C9XX_CSR_MCOUNTEROF, val);
> > +       }
> > +
> > +       /**
> > +        * SSCOFPMF uses the OF bit for enabling/disabling the interrupt,
> > +        * while the C9XX has designated enable bits.
> > +        * So enable per-counter interrupt on C9xx here.
> > +        */
> > +       val = csr_read(THEAD_C9XX_CSR_MCOUNTERINTEN);
> > +       val |= (1 << ctr_idx);
> > +       csr_write(THEAD_C9XX_CSR_MCOUNTERINTEN, val);
> Use csr_set

great idea. I've added this to my v5-branch but will wait a bit for more
review comments to hopefully drop in :-)

> > +}
> > +
> > +static void thead_c9xx_pmu_ctr_disable_irq(uint32_t ctr_idx)
> > +{
> > +       unsigned long val;
> > +
> > +       val = csr_read(THEAD_C9XX_CSR_MCOUNTERINTEN);
> > +       val &= ~(1 << ctr_idx);
> > +       csr_write(THEAD_C9XX_CSR_MCOUNTERINTEN, val);
> csr_clear
> 
> > +}
> > +
> > +static int thead_c9xx_pmu_irq_bit(void)
> > +{
> > +       return THEAD_C9XX_MIP_MOIP;
> > +}
> > +
> > +static void thead_c9xx_pmu_counter_data(unsigned int *count,
> > +                                       unsigned int *bits)
> > +{
> > +       /* auto-detection doesn't work on t-head c9xx cores */
> > +       *count = 29;
> > +       *bits = 64;
> > +}
> > +
> > +const struct sbi_pmu_device thead_c9xx_pmu_device = {
> > +       .hw_counter_enable_irq = thead_c9xx_pmu_ctr_enable_irq,
> > +       .hw_counter_disable_irq = thead_c9xx_pmu_ctr_disable_irq,
> > +       .hw_counter_irq_bit = thead_c9xx_pmu_irq_bit,
> > +       .hw_counter_data = thead_c9xx_pmu_counter_data,
> > +};
> > +
> > +static int sun20i_d1_extensions_init(const struct fdt_match *match)
> > +{
> > +       sbi_pmu_set_device(&thead_c9xx_pmu_device);
> > +
> > +       return 0;
> > +}
> > +
> >  static const struct fdt_match sun20i_d1_match[] = {
> >         { .compatible = "allwinner,sun20i-d1" },
> >         { },
> > @@ -207,4 +278,5 @@ static const struct fdt_match sun20i_d1_match[] = {
> >  const struct platform_override sun20i_d1 = {
> >         .match_table    = sun20i_d1_match,
> >         .final_init     = sun20i_d1_final_init,
> > +       .extensions_init = sun20i_d1_extensions_init,
> >  };
> > diff --git a/platform/generic/include/thead_c9xx.h b/platform/generic/include/thead_c9xx.h
> > new file mode 100644
> > index 0000000..bab0408
> > --- /dev/null
> > +++ b/platform/generic/include/thead_c9xx.h
> > @@ -0,0 +1,127 @@
> > +#ifndef __RISCV_THEAD_C9XX_H____
> > +#define __RISCV_THEAD_C9XX_H____
> > +
> > +/* T-HEAD C9xx M mode CSR.  */
> > +#define THEAD_C9XX_CSR_MXSTATUS                0x7c0
> > +#define THEAD_C9XX_CSR_MHCR            0x7c1
> > +#define THEAD_C9XX_CSR_MCOR            0x7c2
> > +#define THEAD_C9XX_CSR_MCCR2           0x7c3
> > +#define THEAD_C9XX_CSR_MCER2           0x7c4
> > +#define THEAD_C9XX_CSR_MHINT           0x7c5
> > +#define THEAD_C9XX_CSR_MRMR            0x7c6
> > +#define THEAD_C9XX_CSR_MRVBR           0x7c7
> > +#define THEAD_C9XX_CSR_MCER            0x7c8
> > +#define THEAD_C9XX_CSR_MCOUNTERWEN     0x7c9
> > +#define THEAD_C9XX_CSR_MCOUNTERINTEN   0x7ca
> > +#define THEAD_C9XX_CSR_MCOUNTEROF      0x7cb
> > +#define THEAD_C9XX_CSR_MHINT2          0x7cc
> > +#define THEAD_C9XX_CSR_MHINT3          0x7cd
> > +#define THEAD_C9XX_CSR_MRADDR          0x7e0
> > +#define THEAD_C9XX_CSR_MEXSTATUS       0x7e1
> > +#define THEAD_C9XX_CSR_MNMICAUSE       0x7e2
> > +#define THEAD_C9XX_CSR_MNMIPC          0x7e3
> > +#define THEAD_C9XX_CSR_MHPMCR          0x7f0
> > +#define THEAD_C9XX_CSR_MHPMSR          0x7f1
> > +#define THEAD_C9XX_CSR_MHPMER          0x7f2
> > +#define THEAD_C9XX_CSR_MSMPR           0x7f3
> > +#define THEAD_C9XX_CSR_MTEECFG         0x7f4
> > +#define THEAD_C9XX_CSR_MZONEID         0x7f5
> > +#define THEAD_C9XX_CSR_ML2CPID         0x7f6
> > +#define THEAD_C9XX_CSR_ML2WP           0x7f7
> > +#define THEAD_C9XX_CSR_MDTCMCR         0x7f8
> > +#define THEAD_C9XX_CSR_USP             0x7d1
> > +#define THEAD_C9XX_CSR_MCINS           0x7d2
> > +#define THEAD_C9XX_CSR_MCINDEX         0x7d3
> > +#define THEAD_C9XX_CSR_MCDATA0         0x7d4
> > +#define THEAD_C9XX_CSR_MCDATA1         0x7d5
> > +#define THEAD_C9XX_CSR_MEICR           0x7d6
> > +#define THEAD_C9XX_CSR_MEICR2          0x7d7
> > +#define THEAD_C9XX_CSR_MBEADDR         0x7d8
> > +#define THEAD_C9XX_CSR_MCPUID          0xfc0
> > +#define THEAD_C9XX_CSR_MAPBADDR                0xfc1
> > +#define THEAD_C9XX_CSR_MWMSR           0xfc2
> > +#define THEAD_C9XX_CSR_MHALTCAUSE      0xfe0
> > +#define THEAD_C9XX_CSR_MDBGINFO                0xfe1
> > +#define THEAD_C9XX_CSR_MPCFIFO         0xfe2
> > +
> > +/* T-HEAD C9xx S mode CSR.  */
> > +#define THEAD_C9XX_CSR_SXSTATUS                0x5c0
> > +#define THEAD_C9XX_CSR_SHCR            0x5c1
> > +#define THEAD_C9XX_CSR_SCER2           0x5c2
> > +#define THEAD_C9XX_CSR_SCER            0x5c3
> > +#define THEAD_C9XX_CSR_SCOUNTERINTEN   0x5c4
> > +#define THEAD_C9XX_CSR_SCOUNTEROF      0x5c5
> > +#define THEAD_C9XX_CSR_SHINT           0x5c6
> > +#define THEAD_C9XX_CSR_SHINT2          0x5c7
> > +#define THEAD_C9XX_CSR_SHPMINHIBIT     0x5c8
> > +#define THEAD_C9XX_CSR_SHPMCR          0x5c9
> > +#define THEAD_C9XX_CSR_SHPMSR          0x5ca
> > +#define THEAD_C9XX_CSR_SHPMER          0x5cb
> > +#define THEAD_C9XX_CSR_SL2CPID         0x5cc
> > +#define THEAD_C9XX_CSR_SL2WP           0x5cd
> > +#define THEAD_C9XX_CSR_SBEADDR         0x5d0
> > +#define THEAD_C9XX_CSR_SCYCLE          0x5e0
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER1    0x5e1
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER2    0x5e2
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER3    0x5e3
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER4    0x5e4
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER5    0x5e5
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER6    0x5e6
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER7    0x5e7
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER8    0x5e8
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER9    0x5e9
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER10   0x5ea
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER11   0x5eb
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER12   0x5ec
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER13   0x5ed
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER14   0x5ee
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER15   0x5ef
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER16   0x5f0
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER17   0x5f1
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER18   0x5f2
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER19   0x5f3
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER20   0x5f4
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER21   0x5f5
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER22   0x5f6
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER23   0x5f7
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER24   0x5f8
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER25   0x5f9
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER26   0x5fa
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER27   0x5fb
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER28   0x5fc
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER29   0x5fd
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER30   0x5fe
> > +#define THEAD_C9XX_CSR_SHPMCOUNTER31   0x5ff
> Have we used the above CSR_SH* CSR in the patchset series?
>
> > +
> > +/* T-HEAD C9xx U mode CSR.  */
> > +#define THEAD_C9XX_CSR_FXCR            0x800
> It's about the float point register, maybe it should not contain in
> the PMU patch.

My intention was to directly introduce one semi-complete header
with the t-head extensions, so that people in the future don't need
to look this up in some vendor tree when adding support for
other stuff.

I guess I'll leave that to SBI people to decide if that is ok or
if they want a minimal header instead.


Heiko

[PATCH v4 5/5] platform: generic: allwinner: add support for c9xx pmu

[Guo Ren]

On Tue, Sep 27, 2022 at 7:41 PM Heiko St?bner <heiko@sntech.de> wrote:
>
> Hi,
>
> Am Dienstag, 27. September 2022, 01:30:15 CEST schrieb Guo Ren:
> > On Mon, Sep 26, 2022 at 6:16 PM Heiko Stuebner <heiko@sntech.de> wrote:
> > >
> > > With the T-HEAD C9XX cores being designed before or during ratification
> > > of the SSCOFPMF extension, they implement a PMU extension that behaves
> > > very similar but not equal to it by providing overflow interrupts though
> > > in a slightly different registers format.
> > >
> > > The sun20i-d1 is using this core. So implement the necessary overrides
> > > to allow its pmu to be used via the standard sbi-pmu extension.
> > >
> > > For now it's also the only soc using this core, so keep the additional
> > > code in the d1-space for now.
> > >
> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > ---
> > >  platform/generic/allwinner/sun20i-d1.c |  72 ++++++++++++++
> > >  platform/generic/include/thead_c9xx.h  | 127 +++++++++++++++++++++++++
> > >  2 files changed, 199 insertions(+)
> > >  create mode 100644 platform/generic/include/thead_c9xx.h
> > >
> > > diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
> > > index 5b2656c..a624b8b 100644
> > > --- a/platform/generic/allwinner/sun20i-d1.c
> > > +++ b/platform/generic/allwinner/sun20i-d1.c
> > > @@ -5,11 +5,13 @@
> > >   */
> > >
> > >  #include <platform_override.h>
> > > +#include <thead_c9xx.h>
> > >  #include <sbi/riscv_io.h>
> > >  #include <sbi/sbi_bitops.h>
> > >  #include <sbi/sbi_ecall_interface.h>
> > >  #include <sbi/sbi_error.h>
> > >  #include <sbi/sbi_hsm.h>
> > > +#include <sbi/sbi_pmu.h>
> > >  #include <sbi_utils/fdt/fdt_helper.h>
> > >  #include <sbi_utils/irqchip/fdt_irqchip_plic.h>
> > >
> > > @@ -199,6 +201,75 @@ static int sun20i_d1_final_init(bool cold_boot, const struct fdt_match *match)
> > >         return 0;
> > >  }
> > >
> > > +static void thead_c9xx_pmu_ctr_enable_irq(uint32_t ctr_idx)
> > > +{
> > > +       unsigned long val;
> > > +       unsigned long mip_val;
> > > +
> > > +       if (ctr_idx >= SBI_PMU_HW_CTR_MAX)
> > > +               return;
> > > +
> > > +       mip_val = csr_read(CSR_MIP);
> > > +       /**
> > > +        * Clear out the OF bit so that next interrupt can be enabled.
> > > +        * This should be done only when the corresponding overflow interrupt
> > > +        * bit is cleared. That indicates that software has already handled the
> > > +        * previous interrupts or the hardware yet to set an overflow interrupt.
> > > +        * Otherwise, there will be race conditions where we may clear the bit
> > > +        * the software is yet to handle the interrupt.
> > > +        */
> > > +       if (!(mip_val & THEAD_C9XX_MIP_MOIP)) {
> > > +               val = csr_read(THEAD_C9XX_CSR_MCOUNTEROF);
> > > +               val &= ~(1 << ctr_idx);
> > > +               csr_write(THEAD_C9XX_CSR_MCOUNTEROF, val);
> > > +       }
> > > +
> > > +       /**
> > > +        * SSCOFPMF uses the OF bit for enabling/disabling the interrupt,
> > > +        * while the C9XX has designated enable bits.
> > > +        * So enable per-counter interrupt on C9xx here.
> > > +        */
> > > +       val = csr_read(THEAD_C9XX_CSR_MCOUNTERINTEN);
> > > +       val |= (1 << ctr_idx);
> > > +       csr_write(THEAD_C9XX_CSR_MCOUNTERINTEN, val);
> > Use csr_set
>
> great idea. I've added this to my v5-branch but will wait a bit for more
> review comments to hopefully drop in :-)
>
> > > +}
> > > +
> > > +static void thead_c9xx_pmu_ctr_disable_irq(uint32_t ctr_idx)
> > > +{
> > > +       unsigned long val;
> > > +
> > > +       val = csr_read(THEAD_C9XX_CSR_MCOUNTERINTEN);
> > > +       val &= ~(1 << ctr_idx);
> > > +       csr_write(THEAD_C9XX_CSR_MCOUNTERINTEN, val);
> > csr_clear
> >
> > > +}
> > > +
> > > +static int thead_c9xx_pmu_irq_bit(void)
> > > +{
> > > +       return THEAD_C9XX_MIP_MOIP;
> > > +}
> > > +
> > > +static void thead_c9xx_pmu_counter_data(unsigned int *count,
> > > +                                       unsigned int *bits)
> > > +{
> > > +       /* auto-detection doesn't work on t-head c9xx cores */
> > > +       *count = 29;
> > > +       *bits = 64;
> > > +}
> > > +
> > > +const struct sbi_pmu_device thead_c9xx_pmu_device = {
> > > +       .hw_counter_enable_irq = thead_c9xx_pmu_ctr_enable_irq,
> > > +       .hw_counter_disable_irq = thead_c9xx_pmu_ctr_disable_irq,
> > > +       .hw_counter_irq_bit = thead_c9xx_pmu_irq_bit,
> > > +       .hw_counter_data = thead_c9xx_pmu_counter_data,
> > > +};
> > > +
> > > +static int sun20i_d1_extensions_init(const struct fdt_match *match)
> > > +{
> > > +       sbi_pmu_set_device(&thead_c9xx_pmu_device);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  static const struct fdt_match sun20i_d1_match[] = {
> > >         { .compatible = "allwinner,sun20i-d1" },
> > >         { },
> > > @@ -207,4 +278,5 @@ static const struct fdt_match sun20i_d1_match[] = {
> > >  const struct platform_override sun20i_d1 = {
> > >         .match_table    = sun20i_d1_match,
> > >         .final_init     = sun20i_d1_final_init,
> > > +       .extensions_init = sun20i_d1_extensions_init,
> > >  };
> > > diff --git a/platform/generic/include/thead_c9xx.h b/platform/generic/include/thead_c9xx.h
> > > new file mode 100644
> > > index 0000000..bab0408
> > > --- /dev/null
> > > +++ b/platform/generic/include/thead_c9xx.h
> > > @@ -0,0 +1,127 @@
> > > +#ifndef __RISCV_THEAD_C9XX_H____
> > > +#define __RISCV_THEAD_C9XX_H____
> > > +
> > > +/* T-HEAD C9xx M mode CSR.  */
> > > +#define THEAD_C9XX_CSR_MXSTATUS                0x7c0
> > > +#define THEAD_C9XX_CSR_MHCR            0x7c1
> > > +#define THEAD_C9XX_CSR_MCOR            0x7c2
> > > +#define THEAD_C9XX_CSR_MCCR2           0x7c3
> > > +#define THEAD_C9XX_CSR_MCER2           0x7c4
> > > +#define THEAD_C9XX_CSR_MHINT           0x7c5
> > > +#define THEAD_C9XX_CSR_MRMR            0x7c6
> > > +#define THEAD_C9XX_CSR_MRVBR           0x7c7
> > > +#define THEAD_C9XX_CSR_MCER            0x7c8
> > > +#define THEAD_C9XX_CSR_MCOUNTERWEN     0x7c9
> > > +#define THEAD_C9XX_CSR_MCOUNTERINTEN   0x7ca
> > > +#define THEAD_C9XX_CSR_MCOUNTEROF      0x7cb
> > > +#define THEAD_C9XX_CSR_MHINT2          0x7cc
> > > +#define THEAD_C9XX_CSR_MHINT3          0x7cd
> > > +#define THEAD_C9XX_CSR_MRADDR          0x7e0
> > > +#define THEAD_C9XX_CSR_MEXSTATUS       0x7e1
> > > +#define THEAD_C9XX_CSR_MNMICAUSE       0x7e2
> > > +#define THEAD_C9XX_CSR_MNMIPC          0x7e3
> > > +#define THEAD_C9XX_CSR_MHPMCR          0x7f0
> > > +#define THEAD_C9XX_CSR_MHPMSR          0x7f1
> > > +#define THEAD_C9XX_CSR_MHPMER          0x7f2
> > > +#define THEAD_C9XX_CSR_MSMPR           0x7f3
> > > +#define THEAD_C9XX_CSR_MTEECFG         0x7f4
> > > +#define THEAD_C9XX_CSR_MZONEID         0x7f5
> > > +#define THEAD_C9XX_CSR_ML2CPID         0x7f6
> > > +#define THEAD_C9XX_CSR_ML2WP           0x7f7
> > > +#define THEAD_C9XX_CSR_MDTCMCR         0x7f8
> > > +#define THEAD_C9XX_CSR_USP             0x7d1
> > > +#define THEAD_C9XX_CSR_MCINS           0x7d2
> > > +#define THEAD_C9XX_CSR_MCINDEX         0x7d3
> > > +#define THEAD_C9XX_CSR_MCDATA0         0x7d4
> > > +#define THEAD_C9XX_CSR_MCDATA1         0x7d5
> > > +#define THEAD_C9XX_CSR_MEICR           0x7d6
> > > +#define THEAD_C9XX_CSR_MEICR2          0x7d7
> > > +#define THEAD_C9XX_CSR_MBEADDR         0x7d8
> > > +#define THEAD_C9XX_CSR_MCPUID          0xfc0
> > > +#define THEAD_C9XX_CSR_MAPBADDR                0xfc1
> > > +#define THEAD_C9XX_CSR_MWMSR           0xfc2
> > > +#define THEAD_C9XX_CSR_MHALTCAUSE      0xfe0
> > > +#define THEAD_C9XX_CSR_MDBGINFO                0xfe1
> > > +#define THEAD_C9XX_CSR_MPCFIFO         0xfe2
> > > +
> > > +/* T-HEAD C9xx S mode CSR.  */
> > > +#define THEAD_C9XX_CSR_SXSTATUS                0x5c0
> > > +#define THEAD_C9XX_CSR_SHCR            0x5c1
> > > +#define THEAD_C9XX_CSR_SCER2           0x5c2
> > > +#define THEAD_C9XX_CSR_SCER            0x5c3
> > > +#define THEAD_C9XX_CSR_SCOUNTERINTEN   0x5c4
> > > +#define THEAD_C9XX_CSR_SCOUNTEROF      0x5c5
> > > +#define THEAD_C9XX_CSR_SHINT           0x5c6
> > > +#define THEAD_C9XX_CSR_SHINT2          0x5c7
> > > +#define THEAD_C9XX_CSR_SHPMINHIBIT     0x5c8
> > > +#define THEAD_C9XX_CSR_SHPMCR          0x5c9
> > > +#define THEAD_C9XX_CSR_SHPMSR          0x5ca
> > > +#define THEAD_C9XX_CSR_SHPMER          0x5cb
> > > +#define THEAD_C9XX_CSR_SL2CPID         0x5cc
> > > +#define THEAD_C9XX_CSR_SL2WP           0x5cd
> > > +#define THEAD_C9XX_CSR_SBEADDR         0x5d0
> > > +#define THEAD_C9XX_CSR_SCYCLE          0x5e0
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER1    0x5e1
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER2    0x5e2
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER3    0x5e3
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER4    0x5e4
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER5    0x5e5
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER6    0x5e6
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER7    0x5e7
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER8    0x5e8
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER9    0x5e9
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER10   0x5ea
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER11   0x5eb
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER12   0x5ec
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER13   0x5ed
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER14   0x5ee
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER15   0x5ef
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER16   0x5f0
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER17   0x5f1
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER18   0x5f2
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER19   0x5f3
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER20   0x5f4
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER21   0x5f5
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER22   0x5f6
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER23   0x5f7
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER24   0x5f8
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER25   0x5f9
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER26   0x5fa
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER27   0x5fb
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER28   0x5fc
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER29   0x5fd
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER30   0x5fe
> > > +#define THEAD_C9XX_CSR_SHPMCOUNTER31   0x5ff
> > Have we used the above CSR_SH* CSR in the patchset series?
> >
> > > +
> > > +/* T-HEAD C9xx U mode CSR.  */
> > > +#define THEAD_C9XX_CSR_FXCR            0x800
> > It's about the float point register, maybe it should not contain in
> > the PMU patch.
>
> My intention was to directly introduce one semi-complete header
> with the t-head extensions, so that people in the future don't need
> to look this up in some vendor tree when adding support for
> other stuff.
>
> I guess I'll leave that to SBI people to decide if that is ok or
> if they want a minimal header instead.
Okay, after csr_set/clear finished

Reviewed-by: Guo Ren <guoren@kernel.org>

>
>
> Heiko
>
>
>


-- 
Best Regards
 Guo Ren