* [PATCH 0/4] Support Andes PMU extension
@ 2023-09-07 2:16 Yu Chien Peter Lin
2023-09-07 2:16 ` [PATCH 1/4] riscv: errata: Rename defines for Andes Yu Chien Peter Lin
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Yu Chien Peter Lin @ 2023-09-07 2:16 UTC (permalink / raw)
To: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp, anup,
prabhakar.mahadev-lad.rj
Cc: ajones, heiko, samuel, geert+renesas, n.shubin, dminus, ycliang,
tim609, locus84, dylan, Yu Chien Peter Lin
This patch series introduces the Andes PMU errata, which
adds support for perf sampling and mode filtering with
the Andes PMU extension.
The custom PMU extension serves the same purpose as Sscofpmf.
Its non-standard local interrupt is assigned to bit 18 in the
custom S-mode local interrupt pending CSR (slip), while the
interrupt cause is (256 + 18).
This series is dependent on the series from Prabhakar,
- https://patchwork.kernel.org/project/linux-riscv/cover/20230818135723.80612-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
The feature needs the PMU device callbacks in OpenSBI.
The OpenSBI and Linux patches can be found on Andes Technology GitHub
- https://github.com/andestech/opensbi/commits/andes-pmu-support
- https://github.com/andestech/linux/commits/andes-pmu-support
The PMU device tree node of AX45MP:
- https://github.com/andestech/opensbi/blob/andes-pmu-support/docs/pmu_support.md#example-3
Tested hardware:
- ASUS Tinker-V (RZ/Five, AX45MP single core)
- Andes AE350 (AX45MP quad core)
Locus Wei-Han Chen (1):
riscv: andes: Support symbolic FW and HW raw events
Yu Chien Peter Lin (3):
riscv: errata: Rename defines for Andes
irqchip/riscv-intc: Support large non-standard hwirq number
riscv: errata: Add Andes PMU errata
arch/riscv/Kconfig.errata | 13 ++
arch/riscv/errata/andes/errata.c | 55 +++++++-
arch/riscv/include/asm/errata_list.h | 45 ++++++-
arch/riscv/include/asm/vendorid_list.h | 2 +-
arch/riscv/kernel/alternative.c | 2 +-
drivers/irqchip/irq-riscv-intc.c | 10 +-
drivers/perf/riscv_pmu_sbi.c | 20 ++-
.../arch/riscv/andes/ax45/firmware.json | 68 ++++++++++
.../arch/riscv/andes/ax45/instructions.json | 127 ++++++++++++++++++
.../arch/riscv/andes/ax45/memory.json | 57 ++++++++
.../arch/riscv/andes/ax45/microarch.json | 77 +++++++++++
tools/perf/pmu-events/arch/riscv/mapfile.csv | 1 +
12 files changed, 453 insertions(+), 24 deletions(-)
create mode 100644 tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json
create mode 100644 tools/perf/pmu-events/arch/riscv/andes/ax45/instructions.json
create mode 100644 tools/perf/pmu-events/arch/riscv/andes/ax45/memory.json
create mode 100644 tools/perf/pmu-events/arch/riscv/andes/ax45/microarch.json
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] riscv: errata: Rename defines for Andes
2023-09-07 2:16 [PATCH 0/4] Support Andes PMU extension Yu Chien Peter Lin
@ 2023-09-07 2:16 ` Yu Chien Peter Lin
2023-09-07 2:16 ` [RFC PATCH 2/4] irqchip/riscv-intc: Support large non-standard hwirq number Yu Chien Peter Lin
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Yu Chien Peter Lin @ 2023-09-07 2:16 UTC (permalink / raw)
To: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp, anup,
prabhakar.mahadev-lad.rj
Cc: ajones, heiko, samuel, geert+renesas, n.shubin, dminus, ycliang,
tim609, locus84, dylan, Yu Chien Peter Lin
Using "ANDES" rather than "ANDESTECH" to unify the naming
convention with OpenSBI and U-Boot, and reduce the number
of characters per line.
Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
---
Hi Prabhakar,
Sorry for the churn.
We've made the decision to abbreviate andestech in our code, the former
is commonly used in documentation, comment and device tree compatible
string.
---
arch/riscv/errata/andes/errata.c | 10 +++++-----
arch/riscv/include/asm/errata_list.h | 4 ++--
arch/riscv/include/asm/vendorid_list.h | 2 +-
arch/riscv/kernel/alternative.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/errata/andes/errata.c b/arch/riscv/errata/andes/errata.c
index 197db68cc8da..d2e1abcac967 100644
--- a/arch/riscv/errata/andes/errata.c
+++ b/arch/riscv/errata/andes/errata.c
@@ -18,9 +18,9 @@
#include <asm/sbi.h>
#include <asm/vendorid_list.h>
-#define ANDESTECH_AX45MP_MARCHID 0x8000000000008a45UL
-#define ANDESTECH_AX45MP_MIMPID 0x500UL
-#define ANDESTECH_SBI_EXT_ANDES 0x0900031E
+#define ANDES_AX45MP_MARCHID 0x8000000000008a45UL
+#define ANDES_AX45MP_MIMPID 0x500UL
+#define ANDES_SBI_EXT_ANDES 0x0900031E
#define ANDES_SBI_EXT_IOCP_SW_WORKAROUND 1
@@ -32,7 +32,7 @@ static long ax45mp_iocp_sw_workaround(void)
* ANDES_SBI_EXT_IOCP_SW_WORKAROUND SBI EXT checks if the IOCP is missing and
* cache is controllable only then CMO will be applied to the platform.
*/
- ret = sbi_ecall(ANDESTECH_SBI_EXT_ANDES, ANDES_SBI_EXT_IOCP_SW_WORKAROUND,
+ ret = sbi_ecall(ANDES_SBI_EXT_ANDES, ANDES_SBI_EXT_IOCP_SW_WORKAROUND,
0, 0, 0, 0, 0, 0);
return ret.error ? 0 : ret.value;
@@ -43,7 +43,7 @@ static bool errata_probe_iocp(unsigned int stage, unsigned long arch_id, unsigne
if (!IS_ENABLED(CONFIG_ERRATA_ANDES_CMO))
return false;
- if (arch_id != ANDESTECH_AX45MP_MARCHID || impid != ANDESTECH_AX45MP_MIMPID)
+ if (arch_id != ANDES_AX45MP_MARCHID || impid != ANDES_AX45MP_MIMPID)
return false;
if (!ax45mp_iocp_sw_workaround())
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index e2ecd01bfac7..56ab40e64092 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -12,8 +12,8 @@
#include <asm/vendorid_list.h>
#ifdef CONFIG_ERRATA_ANDES
-#define ERRATA_ANDESTECH_NO_IOCP 0
-#define ERRATA_ANDESTECH_NUMBER 1
+#define ERRATA_ANDES_NO_IOCP 0
+#define ERRATA_ANDES_NUMBER 1
#endif
#ifdef CONFIG_ERRATA_SIFIVE
diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
index e55407ace0c3..2f2bb0c84f9a 100644
--- a/arch/riscv/include/asm/vendorid_list.h
+++ b/arch/riscv/include/asm/vendorid_list.h
@@ -5,7 +5,7 @@
#ifndef ASM_VENDOR_LIST_H
#define ASM_VENDOR_LIST_H
-#define ANDESTECH_VENDOR_ID 0x31e
+#define ANDES_VENDOR_ID 0x31e
#define SIFIVE_VENDOR_ID 0x489
#define THEAD_VENDOR_ID 0x5b7
diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
index 319a1da0358b..0128b161bfda 100644
--- a/arch/riscv/kernel/alternative.c
+++ b/arch/riscv/kernel/alternative.c
@@ -43,7 +43,7 @@ static void riscv_fill_cpu_mfr_info(struct cpu_manufacturer_info_t *cpu_mfr_info
switch (cpu_mfr_info->vendor_id) {
#ifdef CONFIG_ERRATA_ANDES
- case ANDESTECH_VENDOR_ID:
+ case ANDES_VENDOR_ID:
cpu_mfr_info->patch_func = andes_errata_patch_func;
break;
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH 2/4] irqchip/riscv-intc: Support large non-standard hwirq number
2023-09-07 2:16 [PATCH 0/4] Support Andes PMU extension Yu Chien Peter Lin
2023-09-07 2:16 ` [PATCH 1/4] riscv: errata: Rename defines for Andes Yu Chien Peter Lin
@ 2023-09-07 2:16 ` Yu Chien Peter Lin
2023-09-07 10:22 ` Clément Léger
2023-09-07 13:06 ` Anup Patel
2023-09-07 2:16 ` [PATCH 3/4] riscv: errata: Add Andes PMU errata Yu Chien Peter Lin
2023-09-07 2:16 ` [PATCH 4/4] riscv: andes: Support symbolic FW and HW raw events Yu Chien Peter Lin
3 siblings, 2 replies; 15+ messages in thread
From: Yu Chien Peter Lin @ 2023-09-07 2:16 UTC (permalink / raw)
To: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp, anup,
prabhakar.mahadev-lad.rj
Cc: ajones, heiko, samuel, geert+renesas, n.shubin, dminus, ycliang,
tim609, locus84, dylan, Yu Chien Peter Lin
Currently, the implementation of the RISC-V INTC driver uses the
interrupt cause as hwirq and has a limitation of supporting a
maximum of 64 hwirqs. However, according to the privileged spec,
interrupt cause >= 16 are defined for platform use.
This limitation prevents us from fully utilizing the available
local interrupt sources. Additionally, the hwirqs used on RISC-V
are sparse, with only interrupt numbers 1, 5 and 9 (plus Sscofpmf
or T-Head's PMU irq) being currently used for supervisor mode.
The patch switches to using irq_domain_create_tree() which
creates the radix tree map, allowing us to handle a larger
number of hwirqs.
Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
---
There are 3 hwirqs of local interrupt source exceed 64 defined in
AX45MP datasheet [1] Table 56: AX45MP-1C scause Value After Trap:
- 256+16 Slave port ECC error interrupt (S-mode)
- 256+17 Bus write transaction error interrupt (S-mode)
- 256+18 Performance monitor overflow interrupt(S-mode)
[1] http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
---
drivers/irqchip/irq-riscv-intc.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index 4adeee1bc391..76e1229c45de 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -24,8 +24,8 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
{
unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;
- if (unlikely(cause >= BITS_PER_LONG))
- panic("unexpected interrupt cause");
+ if (!irq_find_mapping(intc_domain, cause))
+ panic("unexpected interrupt cause: %ld", cause);
generic_handle_domain_irq(intc_domain, cause);
}
@@ -117,8 +117,8 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)
{
int rc;
- intc_domain = irq_domain_create_linear(fn, BITS_PER_LONG,
- &riscv_intc_domain_ops, NULL);
+ intc_domain = irq_domain_create_tree(fn, &riscv_intc_domain_ops,
+ NULL);
if (!intc_domain) {
pr_err("unable to add IRQ domain\n");
return -ENXIO;
@@ -132,8 +132,6 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)
riscv_set_intc_hwnode_fn(riscv_intc_hwnode);
- pr_info("%d local interrupts mapped\n", BITS_PER_LONG);
-
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] riscv: errata: Add Andes PMU errata
2023-09-07 2:16 [PATCH 0/4] Support Andes PMU extension Yu Chien Peter Lin
2023-09-07 2:16 ` [PATCH 1/4] riscv: errata: Rename defines for Andes Yu Chien Peter Lin
2023-09-07 2:16 ` [RFC PATCH 2/4] irqchip/riscv-intc: Support large non-standard hwirq number Yu Chien Peter Lin
@ 2023-09-07 2:16 ` Yu Chien Peter Lin
2023-09-07 2:48 ` Samuel Holland
2023-09-07 9:27 ` Conor Dooley
2023-09-07 2:16 ` [PATCH 4/4] riscv: andes: Support symbolic FW and HW raw events Yu Chien Peter Lin
3 siblings, 2 replies; 15+ messages in thread
From: Yu Chien Peter Lin @ 2023-09-07 2:16 UTC (permalink / raw)
To: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp, anup,
prabhakar.mahadev-lad.rj
Cc: ajones, heiko, samuel, geert+renesas, n.shubin, dminus, ycliang,
tim609, locus84, dylan, Yu Chien Peter Lin
Before the ratification of Sscofpmf, the Andes PMU extension
implements the same mechanism and is compatible with existing
SBI PMU driver of perf to support event sampling and mode
filtering with programmable hardware performance counters.
This patch adds PMU support for Andes 45-series CPUs by
introducing a CPU errata.
Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
Signed-off-by: Locus Wei-Han Chen <locus84@andestech.com>
Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
---
arch/riscv/Kconfig.errata | 13 ++++++++
arch/riscv/errata/andes/errata.c | 45 +++++++++++++++++++++++++++-
arch/riscv/include/asm/errata_list.h | 43 ++++++++++++++++++++++++--
drivers/perf/riscv_pmu_sbi.c | 20 +++++++++----
4 files changed, 111 insertions(+), 10 deletions(-)
diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
index 92c779764b27..a342b209c169 100644
--- a/arch/riscv/Kconfig.errata
+++ b/arch/riscv/Kconfig.errata
@@ -21,6 +21,19 @@ config ERRATA_ANDES_CMO
If you don't know what to do here, say "Y".
+config ERRATA_ANDES_PMU
+ bool "Apply Andes PMU errata"
+ depends on ERRATA_ANDES && RISCV_PMU_SBI
+ default y
+ help
+ The Andes 45-series cores implement a PMU overflow extension
+ very similar to the core SSCOFPMF extension.
+
+ This will apply the overflow errata to handle the non-standard
+ behaviour via the regular SBI PMU driver and interface.
+
+ If you don't know what to do here, say "Y".
+
config ERRATA_SIFIVE
bool "SiFive errata"
depends on RISCV_ALTERNATIVE
diff --git a/arch/riscv/errata/andes/errata.c b/arch/riscv/errata/andes/errata.c
index d2e1abcac967..19256691f1ba 100644
--- a/arch/riscv/errata/andes/errata.c
+++ b/arch/riscv/errata/andes/errata.c
@@ -56,11 +56,54 @@ static bool errata_probe_iocp(unsigned int stage, unsigned long arch_id, unsigne
return true;
}
+static bool errata_probe_pmu(unsigned int stage,
+ unsigned long arch_id, unsigned long impid)
+{
+ if (!IS_ENABLED(CONFIG_ERRATA_ANDES_PMU))
+ return false;
+
+ if ((arch_id & 0xff) != 0x45)
+ return false;
+
+ if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
+ return false;
+
+ return true;
+}
+
+static u32 andes_errata_probe(unsigned int stage,
+ unsigned long archid, unsigned long impid)
+{
+ u32 cpu_req_errata = 0;
+
+ if (errata_probe_pmu(stage, archid, impid))
+ cpu_req_errata |= BIT(ERRATA_ANDES_PMU);
+
+ return cpu_req_errata;
+}
+
void __init_or_module andes_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
unsigned long archid, unsigned long impid,
unsigned int stage)
{
+ struct alt_entry *alt;
+ u32 cpu_req_errata = andes_errata_probe(stage, archid, impid);
+ u32 tmp;
+
errata_probe_iocp(stage, archid, impid);
- /* we have nothing to patch here ATM so just return back */
+ for (alt = begin; alt < end; alt++) {
+ if (alt->vendor_id != ANDES_VENDOR_ID)
+ continue;
+ if (alt->patch_id >= ERRATA_ANDES_NUMBER)
+ continue;
+
+ tmp = (1U << alt->patch_id);
+ if (cpu_req_errata & tmp) {
+ mutex_lock(&text_mutex);
+ patch_text_nosync(ALT_OLD_PTR(alt), ALT_ALT_PTR(alt),
+ alt->alt_len);
+ mutex_unlock(&text_mutex);
+ }
+ }
}
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 56ab40e64092..bb4c276e2c7f 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -13,7 +13,8 @@
#ifdef CONFIG_ERRATA_ANDES
#define ERRATA_ANDES_NO_IOCP 0
-#define ERRATA_ANDES_NUMBER 1
+#define ERRATA_ANDES_PMU 1
+#define ERRATA_ANDES_NUMBER 2
#endif
#ifdef CONFIG_ERRATA_SIFIVE
@@ -150,15 +151,51 @@ asm volatile(ALTERNATIVE_2( \
#define THEAD_C9XX_RV_IRQ_PMU 17
#define THEAD_C9XX_CSR_SCOUNTEROF 0x5c5
+#define ANDES_RV_IRQ_PMU 18
+#define ANDES_SLI_CAUSE_BASE 256
+#define ANDES_CSR_SCOUNTEROF 0x9d4
+#define ANDES_CSR_SLIE 0x9c4
+#define ANDES_CSR_SLIP 0x9c5
+
#define ALT_SBI_PMU_OVERFLOW(__ovl) \
-asm volatile(ALTERNATIVE( \
+asm volatile(ALTERNATIVE_2( \
"csrr %0, " __stringify(CSR_SSCOUNTOVF), \
"csrr %0, " __stringify(THEAD_C9XX_CSR_SCOUNTEROF), \
THEAD_VENDOR_ID, ERRATA_THEAD_PMU, \
- CONFIG_ERRATA_THEAD_PMU) \
+ CONFIG_ERRATA_THEAD_PMU, \
+ "csrr %0, " __stringify(ANDES_CSR_SCOUNTEROF), \
+ ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
+ CONFIG_ERRATA_ANDES_PMU) \
: "=r" (__ovl) : \
: "memory")
+#define ALT_SBI_PMU_OVF_CLEAR_PENDING(__irq_num) \
+asm volatile(ALTERNATIVE( \
+ "csrc " __stringify(CSR_IP) ", %0\n\t", \
+ "csrc " __stringify(ANDES_CSR_SLIP) ", %0\n\t", \
+ ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
+ CONFIG_ERRATA_ANDES_PMU) \
+ : : "r"(BIT(__irq_num)) \
+ : "memory")
+
+#define ALT_SBI_PMU_OVF_DISABLE(__irq_num) \
+asm volatile(ALTERNATIVE( \
+ "csrc " __stringify(CSR_IE) ", %0\n\t", \
+ "csrc " __stringify(ANDES_CSR_SLIE) ", %0\n\t", \
+ ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
+ CONFIG_ERRATA_ANDES_PMU) \
+ : : "r"(BIT(__irq_num)) \
+ : "memory")
+
+#define ALT_SBI_PMU_OVF_ENABLE(__irq_num) \
+asm volatile(ALTERNATIVE( \
+ "csrs " __stringify(CSR_IE) ", %0\n\t", \
+ "csrs " __stringify(ANDES_CSR_SLIE) ", %0\n\t", \
+ ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
+ CONFIG_ERRATA_ANDES_PMU) \
+ : : "r"(BIT(__irq_num)) \
+ : "memory")
+
#endif /* __ASSEMBLY__ */
#endif
diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
index 9a51053b1f99..8b67f202d2ae 100644
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -687,7 +687,7 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
fidx = find_first_bit(cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS);
event = cpu_hw_evt->events[fidx];
if (!event) {
- csr_clear(CSR_SIP, BIT(riscv_pmu_irq_num));
+ ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
return IRQ_NONE;
}
@@ -701,7 +701,7 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
* Overflow interrupt pending bit should only be cleared after stopping
* all the counters to avoid any race condition.
*/
- csr_clear(CSR_SIP, BIT(riscv_pmu_irq_num));
+ ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
/* No overflow bit is set */
if (!overflow)
@@ -773,8 +773,8 @@ static int pmu_sbi_starting_cpu(unsigned int cpu, struct hlist_node *node)
if (riscv_pmu_use_irq) {
cpu_hw_evt->irq = riscv_pmu_irq;
- csr_clear(CSR_IP, BIT(riscv_pmu_irq_num));
- csr_set(CSR_IE, BIT(riscv_pmu_irq_num));
+ ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
+ ALT_SBI_PMU_OVF_ENABLE(riscv_pmu_irq_num);
enable_percpu_irq(riscv_pmu_irq, IRQ_TYPE_NONE);
}
@@ -785,7 +785,7 @@ static int pmu_sbi_dying_cpu(unsigned int cpu, struct hlist_node *node)
{
if (riscv_pmu_use_irq) {
disable_percpu_irq(riscv_pmu_irq);
- csr_clear(CSR_IE, BIT(riscv_pmu_irq_num));
+ ALT_SBI_PMU_OVF_DISABLE(riscv_pmu_irq_num);
}
/* Disable all counters access for user mode now */
@@ -809,6 +809,10 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde
riscv_cached_mimpid(0) == 0) {
riscv_pmu_irq_num = THEAD_C9XX_RV_IRQ_PMU;
riscv_pmu_use_irq = true;
+ } else if (IS_ENABLED(CONFIG_ERRATA_ANDES_PMU) &&
+ riscv_cached_mvendorid(0) == ANDES_VENDOR_ID) {
+ riscv_pmu_irq_num = ANDES_RV_IRQ_PMU;
+ riscv_pmu_use_irq = true;
}
if (!riscv_pmu_use_irq)
@@ -821,7 +825,11 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde
return -ENODEV;
}
- riscv_pmu_irq = irq_create_mapping(domain, riscv_pmu_irq_num);
+ if (IS_ENABLED(CONFIG_ERRATA_ANDES_PMU))
+ riscv_pmu_irq = irq_create_mapping(
+ domain, ANDES_SLI_CAUSE_BASE + riscv_pmu_irq_num);
+ else
+ riscv_pmu_irq = irq_create_mapping(domain, riscv_pmu_irq_num);
if (!riscv_pmu_irq) {
pr_err("Failed to map PMU interrupt for node\n");
return -ENODEV;
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] riscv: andes: Support symbolic FW and HW raw events
2023-09-07 2:16 [PATCH 0/4] Support Andes PMU extension Yu Chien Peter Lin
` (2 preceding siblings ...)
2023-09-07 2:16 ` [PATCH 3/4] riscv: errata: Add Andes PMU errata Yu Chien Peter Lin
@ 2023-09-07 2:16 ` Yu Chien Peter Lin
3 siblings, 0 replies; 15+ messages in thread
From: Yu Chien Peter Lin @ 2023-09-07 2:16 UTC (permalink / raw)
To: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp, anup,
prabhakar.mahadev-lad.rj
Cc: ajones, heiko, samuel, geert+renesas, n.shubin, dminus, ycliang,
tim609, locus84, dylan, Yu Chien Peter Lin
From: Locus Wei-Han Chen <locus84@andestech.com>
This patch adds the Andes AX45 JSON files in the perf tool,
allowing perf to be used with symbolic event names.
Signed-off-by: Locus Wei-Han Chen <locus84@andestech.com>
Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
---
.../arch/riscv/andes/ax45/firmware.json | 68 ++++++++++
.../arch/riscv/andes/ax45/instructions.json | 127 ++++++++++++++++++
.../arch/riscv/andes/ax45/memory.json | 57 ++++++++
.../arch/riscv/andes/ax45/microarch.json | 77 +++++++++++
tools/perf/pmu-events/arch/riscv/mapfile.csv | 1 +
5 files changed, 330 insertions(+)
create mode 100644 tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json
create mode 100644 tools/perf/pmu-events/arch/riscv/andes/ax45/instructions.json
create mode 100644 tools/perf/pmu-events/arch/riscv/andes/ax45/memory.json
create mode 100644 tools/perf/pmu-events/arch/riscv/andes/ax45/microarch.json
diff --git a/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json b/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json
new file mode 100644
index 000000000000..9b4a032186a7
--- /dev/null
+++ b/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json
@@ -0,0 +1,68 @@
+[
+ {
+ "ArchStdEvent": "FW_MISALIGNED_LOAD"
+ },
+ {
+ "ArchStdEvent": "FW_MISALIGNED_STORE"
+ },
+ {
+ "ArchStdEvent": "FW_ACCESS_LOAD"
+ },
+ {
+ "ArchStdEvent": "FW_ACCESS_STORE"
+ },
+ {
+ "ArchStdEvent": "FW_ILLEGAL_INSN"
+ },
+ {
+ "ArchStdEvent": "FW_SET_TIMER"
+ },
+ {
+ "ArchStdEvent": "FW_IPI_SENT"
+ },
+ {
+ "ArchStdEvent": "FW_IPI_RECEIVED"
+ },
+ {
+ "ArchStdEvent": "FW_FENCE_I_SENT"
+ },
+ {
+ "ArchStdEvent": "FW_FENCE_I_RECEIVED"
+ },
+ {
+ "ArchStdEvent": "FW_SFENCE_VMA_SENT"
+ },
+ {
+ "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
+ },
+ {
+ "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
+ },
+ {
+ "ArchStdEvent": "FW_SFENCE_VMA_ASID_RECEIVED"
+ },
+ {
+ "ArchStdEvent": "FW_HFENCE_GVMA_SENT"
+ },
+ {
+ "ArchStdEvent": "FW_HFENCE_GVMA_RECEIVED"
+ },
+ {
+ "ArchStdEvent": "FW_HFENCE_GVMA_VMID_SENT"
+ },
+ {
+ "ArchStdEvent": "FW_HFENCE_GVMA_VMID_RECEIVED"
+ },
+ {
+ "ArchStdEvent": "FW_HFENCE_VVMA_SENT"
+ },
+ {
+ "ArchStdEvent": "FW_HFENCE_VVMA_RECEIVED"
+ },
+ {
+ "ArchStdEvent": "FW_HFENCE_VVMA_ASID_SENT"
+ },
+ {
+ "ArchStdEvent": "FW_HFENCE_VVMA_ASID_RECEIVED"
+ }
+]
diff --git a/tools/perf/pmu-events/arch/riscv/andes/ax45/instructions.json b/tools/perf/pmu-events/arch/riscv/andes/ax45/instructions.json
new file mode 100644
index 000000000000..713a08c1a40f
--- /dev/null
+++ b/tools/perf/pmu-events/arch/riscv/andes/ax45/instructions.json
@@ -0,0 +1,127 @@
+[
+ {
+ "EventCode": "0x10",
+ "EventName": "cycle_count",
+ "BriefDescription": "Cycle count"
+ },
+ {
+ "EventCode": "0x20",
+ "EventName": "inst_count",
+ "BriefDescription": "Retired instruction count"
+ },
+ {
+ "EventCode": "0x30",
+ "EventName": "int_load_inst",
+ "BriefDescription": "Integer load instruction count"
+ },
+ {
+ "EventCode": "0x40",
+ "EventName": "int_store_inst",
+ "BriefDescription": "Integer store instruction count"
+ },
+ {
+ "EventCode": "0x50",
+ "EventName": "atomic_inst",
+ "BriefDescription": "Atomic instruction count"
+ },
+ {
+ "EventCode": "0x60",
+ "EventName": "sys_inst",
+ "BriefDescription": "System instruction count"
+ },
+ {
+ "EventCode": "0x70",
+ "EventName": "int_compute_inst",
+ "BriefDescription": "Integer computational instruction count"
+ },
+ {
+ "EventCode": "0x80",
+ "EventName": "condition_br",
+ "BriefDescription": "Conditional branch instruction count"
+ },
+ {
+ "EventCode": "0x90",
+ "EventName": "taken_condition_br",
+ "BriefDescription": "Taken conditional branch instruction count"
+ },
+ {
+ "EventCode": "0xA0",
+ "EventName": "jal_inst",
+ "BriefDescription": "JAL instruction count"
+ },
+ {
+ "EventCode": "0xB0",
+ "EventName": "jalr_inst",
+ "BriefDescription": "JALR instruction count"
+ },
+ {
+ "EventCode": "0xC0",
+ "EventName": "ret_inst",
+ "BriefDescription": "Return instruction count"
+ },
+ {
+ "EventCode": "0xD0",
+ "EventName": "control_trans_inst",
+ "BriefDescription": "Control transfer instruction count"
+ },
+ {
+ "EventCode": "0xE0",
+ "EventName": "ex9_inst",
+ "BriefDescription": "EXEC.IT instruction count"
+ },
+ {
+ "EventCode": "0xF0",
+ "EventName": "int_mul_inst",
+ "BriefDescription": "Integer multiplication instruction count"
+ },
+ {
+ "EventCode": "0x100",
+ "EventName": "int_div_rem_inst",
+ "BriefDescription": "Integer division/remainder instruction count"
+ },
+ {
+ "EventCode": "0x110",
+ "EventName": "float_load_inst",
+ "BriefDescription": "Floating-point load instruction count"
+ },
+ {
+ "EventCode": "0x120",
+ "EventName": "float_store_inst",
+ "BriefDescription": "Floating-point store instruction count"
+ },
+ {
+ "EventCode": "0x130",
+ "EventName": "float_add_sub_inst",
+ "BriefDescription": "Floating-point addition/subtraction instruction count"
+ },
+ {
+ "EventCode": "0x140",
+ "EventName": "float_mul_inst",
+ "BriefDescription": "Floating-point multiplication instruction count"
+ },
+ {
+ "EventCode": "0x150",
+ "EventName": "float_fused_muladd_inst",
+ "BriefDescription": "Floating-point fused multiply-add instruction count"
+ },
+ {
+ "EventCode": "0x160",
+ "EventName": "float_div_sqrt_inst",
+ "BriefDescription": "Floating-point division or square-root instruction count"
+ },
+ {
+ "EventCode": "0x170",
+ "EventName": "other_float_inst",
+ "BriefDescription": "Other floating-point instruction count"
+ },
+ {
+ "EventCode": "0x180",
+ "EventName": "int_mul_add_sub_inst",
+ "BriefDescription": "Integer multiplication and add/sub instruction count"
+ },
+ {
+ "EventCode": "0x190",
+ "EventName": "retired_ops",
+ "BriefDescription": "Retired operation count"
+ }
+]
diff --git a/tools/perf/pmu-events/arch/riscv/andes/ax45/memory.json b/tools/perf/pmu-events/arch/riscv/andes/ax45/memory.json
new file mode 100644
index 000000000000..c7401b526c77
--- /dev/null
+++ b/tools/perf/pmu-events/arch/riscv/andes/ax45/memory.json
@@ -0,0 +1,57 @@
+[
+ {
+ "EventCode": "0x01",
+ "EventName": "ilm_access",
+ "BriefDescription": "ILM access"
+ },
+ {
+ "EventCode": "0x11",
+ "EventName": "dlm_access",
+ "BriefDescription": "DLM access"
+ },
+ {
+ "EventCode": "0x21",
+ "EventName": "icache_access",
+ "BriefDescription": "ICACHE access"
+ },
+ {
+ "EventCode": "0x31",
+ "EventName": "icache_miss",
+ "BriefDescription": "ICACHE miss"
+ },
+ {
+ "EventCode": "0x41",
+ "EventName": "dcache_access",
+ "BriefDescription": "DCACHE access"
+ },
+ {
+ "EventCode": "0x51",
+ "EventName": "dcache_miss",
+ "BriefDescription": "DCACHE miss"
+ },
+ {
+ "EventCode": "0x61",
+ "EventName": "dcache_load_access",
+ "BriefDescription": "DCACHE load access"
+ },
+ {
+ "EventCode": "0x71",
+ "EventName": "dcache_load_miss",
+ "BriefDescription": "DCACHE load miss"
+ },
+ {
+ "EventCode": "0x81",
+ "EventName": "dcache_store_access",
+ "BriefDescription": "DCACHE store access"
+ },
+ {
+ "EventCode": "0x91",
+ "EventName": "dcache_store_miss",
+ "BriefDescription": "DCACHE store miss"
+ },
+ {
+ "EventCode": "0xA1",
+ "EventName": "dcache_wb",
+ "BriefDescription": "DCACHE writeback"
+ }
+]
diff --git a/tools/perf/pmu-events/arch/riscv/andes/ax45/microarch.json b/tools/perf/pmu-events/arch/riscv/andes/ax45/microarch.json
new file mode 100644
index 000000000000..a6d378cbaa74
--- /dev/null
+++ b/tools/perf/pmu-events/arch/riscv/andes/ax45/microarch.json
@@ -0,0 +1,77 @@
+[
+ {
+ "EventCode": "0xB1",
+ "EventName": "cycle_wait_icache_fill",
+ "BriefDescription": "Cycles waiting for ICACHE fill data"
+ },
+ {
+ "EventCode": "0xC1",
+ "EventName": "cycle_wait_dcache_fill",
+ "BriefDescription": "Cycles waiting for DCACHE fill data"
+ },
+ {
+ "EventCode": "0xD1",
+ "EventName": "uncached_ifetch_from_bus",
+ "BriefDescription": "Uncached ifetch data access from bus"
+ },
+ {
+ "EventCode": "0xE1",
+ "EventName": "uncached_load_from_bus",
+ "BriefDescription": "Uncached load data access from bus"
+ },
+ {
+ "EventCode": "0xF1",
+ "EventName": "cycle_wait_uncached_ifetch",
+ "BriefDescription": "Cycles waiting for uncached ifetch data from bus"
+ },
+ {
+ "EventCode": "0x101",
+ "EventName": "cycle_wait_uncached_load",
+ "BriefDescription": "Cycles waiting for uncached load data from bus"
+ },
+ {
+ "EventCode": "0x111",
+ "EventName": "main_itlb_access",
+ "BriefDescription": "Main ITLB access"
+ },
+ {
+ "EventCode": "0x121",
+ "EventName": "main_itlb_miss",
+ "BriefDescription": "Main ITLB miss"
+ },
+ {
+ "EventCode": "0x131",
+ "EventName": "main_dtlb_access",
+ "BriefDescription": "Main DTLB access"
+ },
+ {
+ "EventCode": "0x141",
+ "EventName": "main_dtlb_miss",
+ "BriefDescription": "Main DTLB miss"
+ },
+ {
+ "EventCode": "0x151",
+ "EventName": "cycle_wait_itlb_fill",
+ "BriefDescription": "Cycles waiting for Main ITLB fill data"
+ },
+ {
+ "EventCode": "0x161",
+ "EventName": "pipe_stall_cycle_dtlb_miss",
+ "BriefDescription": "Pipeline stall cycles caused by Main DTLB miss"
+ },
+ {
+ "EventCode": "0x02",
+ "EventName": "mispredict_condition_br",
+ "BriefDescription": "Misprediction of conditional branches"
+ },
+ {
+ "EventCode": "0x12",
+ "EventName": "mispredict_take_condition_br",
+ "BriefDescription": "Misprediction of taken conditional branches"
+ },
+ {
+ "EventCode": "0x22",
+ "EventName": "mispredict_target_ret_inst",
+ "BriefDescription": "Misprediction of targets of Return instructions"
+ }
+]
diff --git a/tools/perf/pmu-events/arch/riscv/mapfile.csv b/tools/perf/pmu-events/arch/riscv/mapfile.csv
index c61b3d6ef616..5bf09af14c1b 100644
--- a/tools/perf/pmu-events/arch/riscv/mapfile.csv
+++ b/tools/perf/pmu-events/arch/riscv/mapfile.csv
@@ -15,3 +15,4 @@
#
#MVENDORID-MARCHID-MIMPID,Version,Filename,EventType
0x489-0x8000000000000007-0x[[:xdigit:]]+,v1,sifive/u74,core
+0x31e-0x8000000000008a45-0x[[:xdigit:]]+,v1,andes/ax45,core
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] riscv: errata: Add Andes PMU errata
2023-09-07 2:16 ` [PATCH 3/4] riscv: errata: Add Andes PMU errata Yu Chien Peter Lin
@ 2023-09-07 2:48 ` Samuel Holland
2023-09-11 2:38 ` Yu-Chien Peter Lin
2023-09-07 9:27 ` Conor Dooley
1 sibling, 1 reply; 15+ messages in thread
From: Samuel Holland @ 2023-09-07 2:48 UTC (permalink / raw)
To: Yu Chien Peter Lin
Cc: ajones, heiko, samuel, geert+renesas, n.shubin, dminus, ycliang,
tim609, locus84, dylan, linux-riscv, linux-arm-kernel,
linux-kernel, linux-perf-users, paul.walmsley, palmer, aou,
conor.dooley, atishp, anup, prabhakar.mahadev-lad.rj
On 2023-09-06 9:16 PM, Yu Chien Peter Lin wrote:
> Before the ratification of Sscofpmf, the Andes PMU extension
> implements the same mechanism and is compatible with existing
> SBI PMU driver of perf to support event sampling and mode
> filtering with programmable hardware performance counters.
>
> This patch adds PMU support for Andes 45-series CPUs by
> introducing a CPU errata.
>
> Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> Signed-off-by: Locus Wei-Han Chen <locus84@andestech.com>
> Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
> Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> ---
> arch/riscv/Kconfig.errata | 13 ++++++++
> arch/riscv/errata/andes/errata.c | 45 +++++++++++++++++++++++++++-
> arch/riscv/include/asm/errata_list.h | 43 ++++++++++++++++++++++++--
> drivers/perf/riscv_pmu_sbi.c | 20 +++++++++----
> 4 files changed, 111 insertions(+), 10 deletions(-)
>
> diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> index 92c779764b27..a342b209c169 100644
> --- a/arch/riscv/Kconfig.errata
> +++ b/arch/riscv/Kconfig.errata
> @@ -21,6 +21,19 @@ config ERRATA_ANDES_CMO
>
> If you don't know what to do here, say "Y".
>
> +config ERRATA_ANDES_PMU
> + bool "Apply Andes PMU errata"
> + depends on ERRATA_ANDES && RISCV_PMU_SBI
> + default y
> + help
> + The Andes 45-series cores implement a PMU overflow extension
> + very similar to the core SSCOFPMF extension.
> +
> + This will apply the overflow errata to handle the non-standard
> + behaviour via the regular SBI PMU driver and interface.
> +
> + If you don't know what to do here, say "Y".
> +
> config ERRATA_SIFIVE
> bool "SiFive errata"
> depends on RISCV_ALTERNATIVE
> diff --git a/arch/riscv/errata/andes/errata.c b/arch/riscv/errata/andes/errata.c
> index d2e1abcac967..19256691f1ba 100644
> --- a/arch/riscv/errata/andes/errata.c
> +++ b/arch/riscv/errata/andes/errata.c
> @@ -56,11 +56,54 @@ static bool errata_probe_iocp(unsigned int stage, unsigned long arch_id, unsigne
> return true;
> }
>
> +static bool errata_probe_pmu(unsigned int stage,
> + unsigned long arch_id, unsigned long impid)
> +{
> + if (!IS_ENABLED(CONFIG_ERRATA_ANDES_PMU))
> + return false;
> +
> + if ((arch_id & 0xff) != 0x45)
> + return false;
> +
> + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> + return false;
> +
> + return true;
> +}
> +
> +static u32 andes_errata_probe(unsigned int stage,
> + unsigned long archid, unsigned long impid)
> +{
> + u32 cpu_req_errata = 0;
> +
> + if (errata_probe_pmu(stage, archid, impid))
> + cpu_req_errata |= BIT(ERRATA_ANDES_PMU);
> +
> + return cpu_req_errata;
> +}
> +
> void __init_or_module andes_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
> unsigned long archid, unsigned long impid,
> unsigned int stage)
> {
> + struct alt_entry *alt;
> + u32 cpu_req_errata = andes_errata_probe(stage, archid, impid);
> + u32 tmp;
> +
> errata_probe_iocp(stage, archid, impid);
>
> - /* we have nothing to patch here ATM so just return back */
> + for (alt = begin; alt < end; alt++) {
> + if (alt->vendor_id != ANDES_VENDOR_ID)
> + continue;
> + if (alt->patch_id >= ERRATA_ANDES_NUMBER)
> + continue;
> +
> + tmp = (1U << alt->patch_id);
> + if (cpu_req_errata & tmp) {
> + mutex_lock(&text_mutex);
> + patch_text_nosync(ALT_OLD_PTR(alt), ALT_ALT_PTR(alt),
> + alt->alt_len);
> + mutex_unlock(&text_mutex);
> + }
> + }
> }
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 56ab40e64092..bb4c276e2c7f 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -13,7 +13,8 @@
>
> #ifdef CONFIG_ERRATA_ANDES
> #define ERRATA_ANDES_NO_IOCP 0
> -#define ERRATA_ANDES_NUMBER 1
> +#define ERRATA_ANDES_PMU 1
> +#define ERRATA_ANDES_NUMBER 2
> #endif
>
> #ifdef CONFIG_ERRATA_SIFIVE
> @@ -150,15 +151,51 @@ asm volatile(ALTERNATIVE_2( \
> #define THEAD_C9XX_RV_IRQ_PMU 17
> #define THEAD_C9XX_CSR_SCOUNTEROF 0x5c5
>
> +#define ANDES_RV_IRQ_PMU 18
> +#define ANDES_SLI_CAUSE_BASE 256
> +#define ANDES_CSR_SCOUNTEROF 0x9d4
> +#define ANDES_CSR_SLIE 0x9c4
> +#define ANDES_CSR_SLIP 0x9c5
> +
> #define ALT_SBI_PMU_OVERFLOW(__ovl) \
> -asm volatile(ALTERNATIVE( \
> +asm volatile(ALTERNATIVE_2( \
> "csrr %0, " __stringify(CSR_SSCOUNTOVF), \
> "csrr %0, " __stringify(THEAD_C9XX_CSR_SCOUNTEROF), \
> THEAD_VENDOR_ID, ERRATA_THEAD_PMU, \
> - CONFIG_ERRATA_THEAD_PMU) \
> + CONFIG_ERRATA_THEAD_PMU, \
> + "csrr %0, " __stringify(ANDES_CSR_SCOUNTEROF), \
> + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> + CONFIG_ERRATA_ANDES_PMU) \
> : "=r" (__ovl) : \
> : "memory")
>
> +#define ALT_SBI_PMU_OVF_CLEAR_PENDING(__irq_num) \
> +asm volatile(ALTERNATIVE( \
> + "csrc " __stringify(CSR_IP) ", %0\n\t", \
> + "csrc " __stringify(ANDES_CSR_SLIP) ", %0\n\t", \
> + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> + CONFIG_ERRATA_ANDES_PMU) \
> + : : "r"(BIT(__irq_num)) \
> + : "memory")
> +
> +#define ALT_SBI_PMU_OVF_DISABLE(__irq_num) \
> +asm volatile(ALTERNATIVE( \
> + "csrc " __stringify(CSR_IE) ", %0\n\t", \
> + "csrc " __stringify(ANDES_CSR_SLIE) ", %0\n\t", \
> + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> + CONFIG_ERRATA_ANDES_PMU) \
> + : : "r"(BIT(__irq_num)) \
> + : "memory")
> +
> +#define ALT_SBI_PMU_OVF_ENABLE(__irq_num) \
> +asm volatile(ALTERNATIVE( \
> + "csrs " __stringify(CSR_IE) ", %0\n\t", \
> + "csrs " __stringify(ANDES_CSR_SLIE) ", %0\n\t", \
> + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> + CONFIG_ERRATA_ANDES_PMU) \
> + : : "r"(BIT(__irq_num)) \
> + : "memory")
> +
> #endif /* __ASSEMBLY__ */
>
> #endif
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 9a51053b1f99..8b67f202d2ae 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -687,7 +687,7 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
> fidx = find_first_bit(cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS);
> event = cpu_hw_evt->events[fidx];
> if (!event) {
> - csr_clear(CSR_SIP, BIT(riscv_pmu_irq_num));
> + ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
> return IRQ_NONE;
> }
>
> @@ -701,7 +701,7 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
> * Overflow interrupt pending bit should only be cleared after stopping
> * all the counters to avoid any race condition.
> */
> - csr_clear(CSR_SIP, BIT(riscv_pmu_irq_num));
> + ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
>
> /* No overflow bit is set */
> if (!overflow)
> @@ -773,8 +773,8 @@ static int pmu_sbi_starting_cpu(unsigned int cpu, struct hlist_node *node)
>
> if (riscv_pmu_use_irq) {
> cpu_hw_evt->irq = riscv_pmu_irq;
> - csr_clear(CSR_IP, BIT(riscv_pmu_irq_num));
> - csr_set(CSR_IE, BIT(riscv_pmu_irq_num));
> + ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
> + ALT_SBI_PMU_OVF_ENABLE(riscv_pmu_irq_num);
> enable_percpu_irq(riscv_pmu_irq, IRQ_TYPE_NONE);
> }
>
> @@ -785,7 +785,7 @@ static int pmu_sbi_dying_cpu(unsigned int cpu, struct hlist_node *node)
> {
> if (riscv_pmu_use_irq) {
> disable_percpu_irq(riscv_pmu_irq);
> - csr_clear(CSR_IE, BIT(riscv_pmu_irq_num));
> + ALT_SBI_PMU_OVF_DISABLE(riscv_pmu_irq_num);
> }
>
> /* Disable all counters access for user mode now */
> @@ -809,6 +809,10 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde
> riscv_cached_mimpid(0) == 0) {
> riscv_pmu_irq_num = THEAD_C9XX_RV_IRQ_PMU;
> riscv_pmu_use_irq = true;
> + } else if (IS_ENABLED(CONFIG_ERRATA_ANDES_PMU) &&
> + riscv_cached_mvendorid(0) == ANDES_VENDOR_ID) {
> + riscv_pmu_irq_num = ANDES_RV_IRQ_PMU;
> + riscv_pmu_use_irq = true;
> }
>
> if (!riscv_pmu_use_irq)
> @@ -821,7 +825,11 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde
> return -ENODEV;
> }
>
> - riscv_pmu_irq = irq_create_mapping(domain, riscv_pmu_irq_num);
> + if (IS_ENABLED(CONFIG_ERRATA_ANDES_PMU))
> + riscv_pmu_irq = irq_create_mapping(
> + domain, ANDES_SLI_CAUSE_BASE + riscv_pmu_irq_num);
> + else
> + riscv_pmu_irq = irq_create_mapping(domain, riscv_pmu_irq_num);
If the code here needs to be different, then it must check that it is actually
running on an Andes core, not just that the errata Kconfig option is enabled.
However, I suggest setting riscv_pmu_irq_num to the real IRQ number:
riscv_pmu_irq_num = ANDES_SLI_CAUSE_BASE + ANDES_RV_IRQ_PMU;
and then adding a new variable for the mask:
riscv_pmu_irq_mask = BIT(riscv_pmu_irq_num % BITS_PER_LONG);
which handles the large IRQ number somewhat more generically, and reduces the
number of bit operations needed elsewhere in the driver.
Or we could use IRQ chip operations here instead of direct CSR acccess. But
maybe the direct CSR access is needed for performance?
Regards,
Samuel
> if (!riscv_pmu_irq) {
> pr_err("Failed to map PMU interrupt for node\n");
> return -ENODEV;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] riscv: errata: Add Andes PMU errata
2023-09-07 2:16 ` [PATCH 3/4] riscv: errata: Add Andes PMU errata Yu Chien Peter Lin
2023-09-07 2:48 ` Samuel Holland
@ 2023-09-07 9:27 ` Conor Dooley
2023-09-07 11:02 ` Conor Dooley
1 sibling, 1 reply; 15+ messages in thread
From: Conor Dooley @ 2023-09-07 9:27 UTC (permalink / raw)
To: Yu Chien Peter Lin
Cc: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp, anup,
prabhakar.mahadev-lad.rj, ajones, heiko, samuel, geert+renesas,
n.shubin, dminus, ycliang, tim609, locus84, dylan
[-- Attachment #1: Type: text/plain, Size: 9575 bytes --]
Hey,
On Thu, Sep 07, 2023 at 10:16:34AM +0800, Yu Chien Peter Lin wrote:
> Before the ratification of Sscofpmf, the Andes PMU extension
> implements the same mechanism and is compatible with existing
> SBI PMU driver of perf to support event sampling and mode
> filtering with programmable hardware performance counters.
If it actually was, you'd not need to modify the driver ;)
> This patch adds PMU support for Andes 45-series CPUs by
> introducing a CPU errata.
I don't really understand this in all honesty. You don't have Sscofpmf
support with a bug, you have something that is Sscofpmf-adjactent that
predates it. Why claim to support an extension that you do not, only to
have to come along and try to clean things up afterwards, instead of
accurately declaring what you do support from the outset?
(and just because someone already got away with it, doesn't mean that
you get a free pass on it, sorry)
Thanks,
Conor.
> Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> Signed-off-by: Locus Wei-Han Chen <locus84@andestech.com>
btw, what did Locus Wei-Han Chen do here? Are you missing
a Co-developed-by: tag?
> Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
> Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> ---
> arch/riscv/Kconfig.errata | 13 ++++++++
> arch/riscv/errata/andes/errata.c | 45 +++++++++++++++++++++++++++-
> arch/riscv/include/asm/errata_list.h | 43 ++++++++++++++++++++++++--
> drivers/perf/riscv_pmu_sbi.c | 20 +++++++++----
> 4 files changed, 111 insertions(+), 10 deletions(-)
>
> diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> index 92c779764b27..a342b209c169 100644
> --- a/arch/riscv/Kconfig.errata
> +++ b/arch/riscv/Kconfig.errata
> @@ -21,6 +21,19 @@ config ERRATA_ANDES_CMO
>
> If you don't know what to do here, say "Y".
>
> +config ERRATA_ANDES_PMU
> + bool "Apply Andes PMU errata"
> + depends on ERRATA_ANDES && RISCV_PMU_SBI
> + default y
> + help
> + The Andes 45-series cores implement a PMU overflow extension
> + very similar to the core SSCOFPMF extension.
> +
> + This will apply the overflow errata to handle the non-standard
> + behaviour via the regular SBI PMU driver and interface.
> +
> + If you don't know what to do here, say "Y".
> +
> config ERRATA_SIFIVE
> bool "SiFive errata"
> depends on RISCV_ALTERNATIVE
> diff --git a/arch/riscv/errata/andes/errata.c b/arch/riscv/errata/andes/errata.c
> index d2e1abcac967..19256691f1ba 100644
> --- a/arch/riscv/errata/andes/errata.c
> +++ b/arch/riscv/errata/andes/errata.c
> @@ -56,11 +56,54 @@ static bool errata_probe_iocp(unsigned int stage, unsigned long arch_id, unsigne
> return true;
> }
>
> +static bool errata_probe_pmu(unsigned int stage,
> + unsigned long arch_id, unsigned long impid)
> +{
> + if (!IS_ENABLED(CONFIG_ERRATA_ANDES_PMU))
> + return false;
> +
> + if ((arch_id & 0xff) != 0x45)
> + return false;
> +
> + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> + return false;
> +
> + return true;
> +}
> +
> +static u32 andes_errata_probe(unsigned int stage,
> + unsigned long archid, unsigned long impid)
> +{
> + u32 cpu_req_errata = 0;
> +
> + if (errata_probe_pmu(stage, archid, impid))
> + cpu_req_errata |= BIT(ERRATA_ANDES_PMU);
> +
> + return cpu_req_errata;
> +}
> +
> void __init_or_module andes_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
> unsigned long archid, unsigned long impid,
> unsigned int stage)
> {
> + struct alt_entry *alt;
> + u32 cpu_req_errata = andes_errata_probe(stage, archid, impid);
> + u32 tmp;
> +
> errata_probe_iocp(stage, archid, impid);
>
> - /* we have nothing to patch here ATM so just return back */
> + for (alt = begin; alt < end; alt++) {
> + if (alt->vendor_id != ANDES_VENDOR_ID)
> + continue;
> + if (alt->patch_id >= ERRATA_ANDES_NUMBER)
> + continue;
> +
> + tmp = (1U << alt->patch_id);
> + if (cpu_req_errata & tmp) {
> + mutex_lock(&text_mutex);
> + patch_text_nosync(ALT_OLD_PTR(alt), ALT_ALT_PTR(alt),
> + alt->alt_len);
> + mutex_unlock(&text_mutex);
> + }
> + }
> }
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 56ab40e64092..bb4c276e2c7f 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -13,7 +13,8 @@
>
> #ifdef CONFIG_ERRATA_ANDES
> #define ERRATA_ANDES_NO_IOCP 0
> -#define ERRATA_ANDES_NUMBER 1
> +#define ERRATA_ANDES_PMU 1
> +#define ERRATA_ANDES_NUMBER 2
> #endif
>
> #ifdef CONFIG_ERRATA_SIFIVE
> @@ -150,15 +151,51 @@ asm volatile(ALTERNATIVE_2( \
> #define THEAD_C9XX_RV_IRQ_PMU 17
> #define THEAD_C9XX_CSR_SCOUNTEROF 0x5c5
>
> +#define ANDES_RV_IRQ_PMU 18
> +#define ANDES_SLI_CAUSE_BASE 256
> +#define ANDES_CSR_SCOUNTEROF 0x9d4
> +#define ANDES_CSR_SLIE 0x9c4
> +#define ANDES_CSR_SLIP 0x9c5
> +
> #define ALT_SBI_PMU_OVERFLOW(__ovl) \
> -asm volatile(ALTERNATIVE( \
> +asm volatile(ALTERNATIVE_2( \
> "csrr %0, " __stringify(CSR_SSCOUNTOVF), \
> "csrr %0, " __stringify(THEAD_C9XX_CSR_SCOUNTEROF), \
> THEAD_VENDOR_ID, ERRATA_THEAD_PMU, \
> - CONFIG_ERRATA_THEAD_PMU) \
> + CONFIG_ERRATA_THEAD_PMU, \
> + "csrr %0, " __stringify(ANDES_CSR_SCOUNTEROF), \
> + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> + CONFIG_ERRATA_ANDES_PMU) \
> : "=r" (__ovl) : \
> : "memory")
>
> +#define ALT_SBI_PMU_OVF_CLEAR_PENDING(__irq_num) \
> +asm volatile(ALTERNATIVE( \
> + "csrc " __stringify(CSR_IP) ", %0\n\t", \
> + "csrc " __stringify(ANDES_CSR_SLIP) ", %0\n\t", \
> + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> + CONFIG_ERRATA_ANDES_PMU) \
> + : : "r"(BIT(__irq_num)) \
> + : "memory")
> +
> +#define ALT_SBI_PMU_OVF_DISABLE(__irq_num) \
> +asm volatile(ALTERNATIVE( \
> + "csrc " __stringify(CSR_IE) ", %0\n\t", \
> + "csrc " __stringify(ANDES_CSR_SLIE) ", %0\n\t", \
> + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> + CONFIG_ERRATA_ANDES_PMU) \
> + : : "r"(BIT(__irq_num)) \
> + : "memory")
> +
> +#define ALT_SBI_PMU_OVF_ENABLE(__irq_num) \
> +asm volatile(ALTERNATIVE( \
> + "csrs " __stringify(CSR_IE) ", %0\n\t", \
> + "csrs " __stringify(ANDES_CSR_SLIE) ", %0\n\t", \
> + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> + CONFIG_ERRATA_ANDES_PMU) \
> + : : "r"(BIT(__irq_num)) \
> + : "memory")
> +
> #endif /* __ASSEMBLY__ */
>
> #endif
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 9a51053b1f99..8b67f202d2ae 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -687,7 +687,7 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
> fidx = find_first_bit(cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS);
> event = cpu_hw_evt->events[fidx];
> if (!event) {
> - csr_clear(CSR_SIP, BIT(riscv_pmu_irq_num));
> + ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
> return IRQ_NONE;
> }
>
> @@ -701,7 +701,7 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
> * Overflow interrupt pending bit should only be cleared after stopping
> * all the counters to avoid any race condition.
> */
> - csr_clear(CSR_SIP, BIT(riscv_pmu_irq_num));
> + ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
>
> /* No overflow bit is set */
> if (!overflow)
> @@ -773,8 +773,8 @@ static int pmu_sbi_starting_cpu(unsigned int cpu, struct hlist_node *node)
>
> if (riscv_pmu_use_irq) {
> cpu_hw_evt->irq = riscv_pmu_irq;
> - csr_clear(CSR_IP, BIT(riscv_pmu_irq_num));
> - csr_set(CSR_IE, BIT(riscv_pmu_irq_num));
> + ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
> + ALT_SBI_PMU_OVF_ENABLE(riscv_pmu_irq_num);
> enable_percpu_irq(riscv_pmu_irq, IRQ_TYPE_NONE);
> }
>
> @@ -785,7 +785,7 @@ static int pmu_sbi_dying_cpu(unsigned int cpu, struct hlist_node *node)
> {
> if (riscv_pmu_use_irq) {
> disable_percpu_irq(riscv_pmu_irq);
> - csr_clear(CSR_IE, BIT(riscv_pmu_irq_num));
> + ALT_SBI_PMU_OVF_DISABLE(riscv_pmu_irq_num);
> }
>
> /* Disable all counters access for user mode now */
> @@ -809,6 +809,10 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde
> riscv_cached_mimpid(0) == 0) {
> riscv_pmu_irq_num = THEAD_C9XX_RV_IRQ_PMU;
> riscv_pmu_use_irq = true;
> + } else if (IS_ENABLED(CONFIG_ERRATA_ANDES_PMU) &&
> + riscv_cached_mvendorid(0) == ANDES_VENDOR_ID) {
> + riscv_pmu_irq_num = ANDES_RV_IRQ_PMU;
> + riscv_pmu_use_irq = true;
> }
>
> if (!riscv_pmu_use_irq)
> @@ -821,7 +825,11 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde
> return -ENODEV;
> }
>
> - riscv_pmu_irq = irq_create_mapping(domain, riscv_pmu_irq_num);
> + if (IS_ENABLED(CONFIG_ERRATA_ANDES_PMU))
> + riscv_pmu_irq = irq_create_mapping(
> + domain, ANDES_SLI_CAUSE_BASE + riscv_pmu_irq_num);
> + else
> + riscv_pmu_irq = irq_create_mapping(domain, riscv_pmu_irq_num);
> if (!riscv_pmu_irq) {
> pr_err("Failed to map PMU interrupt for node\n");
> return -ENODEV;
> --
> 2.34.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH 2/4] irqchip/riscv-intc: Support large non-standard hwirq number
2023-09-07 2:16 ` [RFC PATCH 2/4] irqchip/riscv-intc: Support large non-standard hwirq number Yu Chien Peter Lin
@ 2023-09-07 10:22 ` Clément Léger
2023-09-11 8:04 ` Yu-Chien Peter Lin
2023-09-07 13:06 ` Anup Patel
1 sibling, 1 reply; 15+ messages in thread
From: Clément Léger @ 2023-09-07 10:22 UTC (permalink / raw)
To: Yu Chien Peter Lin, linux-riscv, linux-arm-kernel, linux-kernel,
linux-perf-users, paul.walmsley, palmer, aou, conor.dooley,
atishp, anup, prabhakar.mahadev-lad.rj
Cc: ajones, heiko, samuel, geert+renesas, n.shubin, dminus, ycliang,
tim609, locus84, dylan
On 07/09/2023 04:16, Yu Chien Peter Lin wrote:
> Currently, the implementation of the RISC-V INTC driver uses the
> interrupt cause as hwirq and has a limitation of supporting a
> maximum of 64 hwirqs. However, according to the privileged spec,
> interrupt cause >= 16 are defined for platform use.
>
> This limitation prevents us from fully utilizing the available
> local interrupt sources. Additionally, the hwirqs used on RISC-V
> are sparse, with only interrupt numbers 1, 5 and 9 (plus Sscofpmf
> or T-Head's PMU irq) being currently used for supervisor mode.
>
> The patch switches to using irq_domain_create_tree() which
> creates the radix tree map, allowing us to handle a larger
> number of hwirqs.
>
> Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
> Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
>
> ---
> There are 3 hwirqs of local interrupt source exceed 64 defined in
> AX45MP datasheet [1] Table 56: AX45MP-1C scause Value After Trap:
> - 256+16 Slave port ECC error interrupt (S-mode)
> - 256+17 Bus write transaction error interrupt (S-mode)
> - 256+18 Performance monitor overflow interrupt(S-mode)
>
> [1] http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
> ---
> drivers/irqchip/irq-riscv-intc.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> index 4adeee1bc391..76e1229c45de 100644
> --- a/drivers/irqchip/irq-riscv-intc.c
> +++ b/drivers/irqchip/irq-riscv-intc.c
> @@ -24,8 +24,8 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
> {
> unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;
>
> - if (unlikely(cause >= BITS_PER_LONG))
> - panic("unexpected interrupt cause");
> + if (!irq_find_mapping(intc_domain, cause))
> + panic("unexpected interrupt cause: %ld", cause);
Hi Yu,
It seems like generic_handle_domain_irq() returns -EINVAL if provided
with NULL (which will happen if the interrupt does not have a mapping
due to __irq_resolve_mapping returning NULL) so maybe it is possible to
remove this check (since __irq_resolve_mapping() is also called by
generic_handle_domain_irq()) and panic if generic_handle_domain_irq()
returns -EINVAL ? That would avoid calling __irq_resolve_mapping() twice
for really rare cases.
Clément
>
> generic_handle_domain_irq(intc_domain, cause);
> }
> @@ -117,8 +117,8 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)
> {
> int rc;
>
> - intc_domain = irq_domain_create_linear(fn, BITS_PER_LONG,
> - &riscv_intc_domain_ops, NULL);
> + intc_domain = irq_domain_create_tree(fn, &riscv_intc_domain_ops,
> + NULL);
> if (!intc_domain) {
> pr_err("unable to add IRQ domain\n");
> return -ENXIO;
> @@ -132,8 +132,6 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)
>
> riscv_set_intc_hwnode_fn(riscv_intc_hwnode);
>
> - pr_info("%d local interrupts mapped\n", BITS_PER_LONG);
> -
> return 0;
> }
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] riscv: errata: Add Andes PMU errata
2023-09-07 9:27 ` Conor Dooley
@ 2023-09-07 11:02 ` Conor Dooley
2023-09-11 2:48 ` Yu-Chien Peter Lin
0 siblings, 1 reply; 15+ messages in thread
From: Conor Dooley @ 2023-09-07 11:02 UTC (permalink / raw)
To: Yu Chien Peter Lin
Cc: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp, anup,
prabhakar.mahadev-lad.rj, ajones, heiko, samuel, geert+renesas,
n.shubin, dminus, ycliang, tim609, locus84, dylan
[-- Attachment #1: Type: text/plain, Size: 10848 bytes --]
On Thu, Sep 07, 2023 at 10:27:03AM +0100, Conor Dooley wrote:
> Hey,
>
> On Thu, Sep 07, 2023 at 10:16:34AM +0800, Yu Chien Peter Lin wrote:
> > Before the ratification of Sscofpmf, the Andes PMU extension
> > implements the same mechanism and is compatible with existing
> > SBI PMU driver of perf to support event sampling and mode
> > filtering with programmable hardware performance counters.
>
> If it actually was, you'd not need to modify the driver ;)
>
> > This patch adds PMU support for Andes 45-series CPUs by
> > introducing a CPU errata.
>
> I don't really understand this in all honesty. You don't have Sscofpmf
> support with a bug, you have something that is Sscofpmf-adjactent that
> predates it. Why claim to support an extension that you do not, only to
> have to come along and try to clean things up afterwards, instead of
> accurately declaring what you do support from the outset?
Reading this again, I don't think that I have been particularly clear,
sorry. My point is that this is not a fix for a bug in your
implementation of Sscofpmf, but rather adding probing for what is
effectively a custom ISA extension that happens to be very similar to
the standard one. As it is not an implementation bug, errata should
not be abused to support vendor extensions, and either DT or ACPI should
be used to inform the operating system about its presence.
Cheers,
Conor.
>
> (and just because someone already got away with it, doesn't mean that
> you get a free pass on it, sorry)
>
> Thanks,
> Conor.
>
> > Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> > Signed-off-by: Locus Wei-Han Chen <locus84@andestech.com>
>
> btw, what did Locus Wei-Han Chen do here? Are you missing
> a Co-developed-by: tag?
>
> > Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
> > Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> > ---
> > arch/riscv/Kconfig.errata | 13 ++++++++
> > arch/riscv/errata/andes/errata.c | 45 +++++++++++++++++++++++++++-
> > arch/riscv/include/asm/errata_list.h | 43 ++++++++++++++++++++++++--
> > drivers/perf/riscv_pmu_sbi.c | 20 +++++++++----
> > 4 files changed, 111 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> > index 92c779764b27..a342b209c169 100644
> > --- a/arch/riscv/Kconfig.errata
> > +++ b/arch/riscv/Kconfig.errata
> > @@ -21,6 +21,19 @@ config ERRATA_ANDES_CMO
> >
> > If you don't know what to do here, say "Y".
> >
> > +config ERRATA_ANDES_PMU
> > + bool "Apply Andes PMU errata"
> > + depends on ERRATA_ANDES && RISCV_PMU_SBI
> > + default y
> > + help
> > + The Andes 45-series cores implement a PMU overflow extension
> > + very similar to the core SSCOFPMF extension.
> > +
> > + This will apply the overflow errata to handle the non-standard
> > + behaviour via the regular SBI PMU driver and interface.
> > +
> > + If you don't know what to do here, say "Y".
> > +
> > config ERRATA_SIFIVE
> > bool "SiFive errata"
> > depends on RISCV_ALTERNATIVE
> > diff --git a/arch/riscv/errata/andes/errata.c b/arch/riscv/errata/andes/errata.c
> > index d2e1abcac967..19256691f1ba 100644
> > --- a/arch/riscv/errata/andes/errata.c
> > +++ b/arch/riscv/errata/andes/errata.c
> > @@ -56,11 +56,54 @@ static bool errata_probe_iocp(unsigned int stage, unsigned long arch_id, unsigne
> > return true;
> > }
> >
> > +static bool errata_probe_pmu(unsigned int stage,
> > + unsigned long arch_id, unsigned long impid)
> > +{
> > + if (!IS_ENABLED(CONFIG_ERRATA_ANDES_PMU))
> > + return false;
> > +
> > + if ((arch_id & 0xff) != 0x45)
> > + return false;
> > +
> > + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +static u32 andes_errata_probe(unsigned int stage,
> > + unsigned long archid, unsigned long impid)
> > +{
> > + u32 cpu_req_errata = 0;
> > +
> > + if (errata_probe_pmu(stage, archid, impid))
> > + cpu_req_errata |= BIT(ERRATA_ANDES_PMU);
> > +
> > + return cpu_req_errata;
> > +}
> > +
> > void __init_or_module andes_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
> > unsigned long archid, unsigned long impid,
> > unsigned int stage)
> > {
> > + struct alt_entry *alt;
> > + u32 cpu_req_errata = andes_errata_probe(stage, archid, impid);
> > + u32 tmp;
> > +
> > errata_probe_iocp(stage, archid, impid);
> >
> > - /* we have nothing to patch here ATM so just return back */
> > + for (alt = begin; alt < end; alt++) {
> > + if (alt->vendor_id != ANDES_VENDOR_ID)
> > + continue;
> > + if (alt->patch_id >= ERRATA_ANDES_NUMBER)
> > + continue;
> > +
> > + tmp = (1U << alt->patch_id);
> > + if (cpu_req_errata & tmp) {
> > + mutex_lock(&text_mutex);
> > + patch_text_nosync(ALT_OLD_PTR(alt), ALT_ALT_PTR(alt),
> > + alt->alt_len);
> > + mutex_unlock(&text_mutex);
> > + }
> > + }
> > }
> > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > index 56ab40e64092..bb4c276e2c7f 100644
> > --- a/arch/riscv/include/asm/errata_list.h
> > +++ b/arch/riscv/include/asm/errata_list.h
> > @@ -13,7 +13,8 @@
> >
> > #ifdef CONFIG_ERRATA_ANDES
> > #define ERRATA_ANDES_NO_IOCP 0
> > -#define ERRATA_ANDES_NUMBER 1
> > +#define ERRATA_ANDES_PMU 1
> > +#define ERRATA_ANDES_NUMBER 2
> > #endif
> >
> > #ifdef CONFIG_ERRATA_SIFIVE
> > @@ -150,15 +151,51 @@ asm volatile(ALTERNATIVE_2( \
> > #define THEAD_C9XX_RV_IRQ_PMU 17
> > #define THEAD_C9XX_CSR_SCOUNTEROF 0x5c5
> >
> > +#define ANDES_RV_IRQ_PMU 18
> > +#define ANDES_SLI_CAUSE_BASE 256
> > +#define ANDES_CSR_SCOUNTEROF 0x9d4
> > +#define ANDES_CSR_SLIE 0x9c4
> > +#define ANDES_CSR_SLIP 0x9c5
> > +
> > #define ALT_SBI_PMU_OVERFLOW(__ovl) \
> > -asm volatile(ALTERNATIVE( \
> > +asm volatile(ALTERNATIVE_2( \
> > "csrr %0, " __stringify(CSR_SSCOUNTOVF), \
> > "csrr %0, " __stringify(THEAD_C9XX_CSR_SCOUNTEROF), \
> > THEAD_VENDOR_ID, ERRATA_THEAD_PMU, \
> > - CONFIG_ERRATA_THEAD_PMU) \
> > + CONFIG_ERRATA_THEAD_PMU, \
> > + "csrr %0, " __stringify(ANDES_CSR_SCOUNTEROF), \
> > + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> > + CONFIG_ERRATA_ANDES_PMU) \
> > : "=r" (__ovl) : \
> > : "memory")
> >
> > +#define ALT_SBI_PMU_OVF_CLEAR_PENDING(__irq_num) \
> > +asm volatile(ALTERNATIVE( \
> > + "csrc " __stringify(CSR_IP) ", %0\n\t", \
> > + "csrc " __stringify(ANDES_CSR_SLIP) ", %0\n\t", \
> > + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> > + CONFIG_ERRATA_ANDES_PMU) \
> > + : : "r"(BIT(__irq_num)) \
> > + : "memory")
> > +
> > +#define ALT_SBI_PMU_OVF_DISABLE(__irq_num) \
> > +asm volatile(ALTERNATIVE( \
> > + "csrc " __stringify(CSR_IE) ", %0\n\t", \
> > + "csrc " __stringify(ANDES_CSR_SLIE) ", %0\n\t", \
> > + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> > + CONFIG_ERRATA_ANDES_PMU) \
> > + : : "r"(BIT(__irq_num)) \
> > + : "memory")
> > +
> > +#define ALT_SBI_PMU_OVF_ENABLE(__irq_num) \
> > +asm volatile(ALTERNATIVE( \
> > + "csrs " __stringify(CSR_IE) ", %0\n\t", \
> > + "csrs " __stringify(ANDES_CSR_SLIE) ", %0\n\t", \
> > + ANDES_VENDOR_ID, ERRATA_ANDES_PMU, \
> > + CONFIG_ERRATA_ANDES_PMU) \
> > + : : "r"(BIT(__irq_num)) \
> > + : "memory")
> > +
> > #endif /* __ASSEMBLY__ */
> >
> > #endif
> > diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> > index 9a51053b1f99..8b67f202d2ae 100644
> > --- a/drivers/perf/riscv_pmu_sbi.c
> > +++ b/drivers/perf/riscv_pmu_sbi.c
> > @@ -687,7 +687,7 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
> > fidx = find_first_bit(cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS);
> > event = cpu_hw_evt->events[fidx];
> > if (!event) {
> > - csr_clear(CSR_SIP, BIT(riscv_pmu_irq_num));
> > + ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
> > return IRQ_NONE;
> > }
> >
> > @@ -701,7 +701,7 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
> > * Overflow interrupt pending bit should only be cleared after stopping
> > * all the counters to avoid any race condition.
> > */
> > - csr_clear(CSR_SIP, BIT(riscv_pmu_irq_num));
> > + ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
> >
> > /* No overflow bit is set */
> > if (!overflow)
> > @@ -773,8 +773,8 @@ static int pmu_sbi_starting_cpu(unsigned int cpu, struct hlist_node *node)
> >
> > if (riscv_pmu_use_irq) {
> > cpu_hw_evt->irq = riscv_pmu_irq;
> > - csr_clear(CSR_IP, BIT(riscv_pmu_irq_num));
> > - csr_set(CSR_IE, BIT(riscv_pmu_irq_num));
> > + ALT_SBI_PMU_OVF_CLEAR_PENDING(riscv_pmu_irq_num);
> > + ALT_SBI_PMU_OVF_ENABLE(riscv_pmu_irq_num);
> > enable_percpu_irq(riscv_pmu_irq, IRQ_TYPE_NONE);
> > }
> >
> > @@ -785,7 +785,7 @@ static int pmu_sbi_dying_cpu(unsigned int cpu, struct hlist_node *node)
> > {
> > if (riscv_pmu_use_irq) {
> > disable_percpu_irq(riscv_pmu_irq);
> > - csr_clear(CSR_IE, BIT(riscv_pmu_irq_num));
> > + ALT_SBI_PMU_OVF_DISABLE(riscv_pmu_irq_num);
> > }
> >
> > /* Disable all counters access for user mode now */
> > @@ -809,6 +809,10 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde
> > riscv_cached_mimpid(0) == 0) {
> > riscv_pmu_irq_num = THEAD_C9XX_RV_IRQ_PMU;
> > riscv_pmu_use_irq = true;
> > + } else if (IS_ENABLED(CONFIG_ERRATA_ANDES_PMU) &&
> > + riscv_cached_mvendorid(0) == ANDES_VENDOR_ID) {
> > + riscv_pmu_irq_num = ANDES_RV_IRQ_PMU;
> > + riscv_pmu_use_irq = true;
> > }
> >
> > if (!riscv_pmu_use_irq)
> > @@ -821,7 +825,11 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde
> > return -ENODEV;
> > }
> >
> > - riscv_pmu_irq = irq_create_mapping(domain, riscv_pmu_irq_num);
> > + if (IS_ENABLED(CONFIG_ERRATA_ANDES_PMU))
> > + riscv_pmu_irq = irq_create_mapping(
> > + domain, ANDES_SLI_CAUSE_BASE + riscv_pmu_irq_num);
> > + else
> > + riscv_pmu_irq = irq_create_mapping(domain, riscv_pmu_irq_num);
> > if (!riscv_pmu_irq) {
> > pr_err("Failed to map PMU interrupt for node\n");
> > return -ENODEV;
> > --
> > 2.34.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH 2/4] irqchip/riscv-intc: Support large non-standard hwirq number
2023-09-07 2:16 ` [RFC PATCH 2/4] irqchip/riscv-intc: Support large non-standard hwirq number Yu Chien Peter Lin
2023-09-07 10:22 ` Clément Léger
@ 2023-09-07 13:06 ` Anup Patel
2023-09-11 8:12 ` Yu-Chien Peter Lin
1 sibling, 1 reply; 15+ messages in thread
From: Anup Patel @ 2023-09-07 13:06 UTC (permalink / raw)
To: Yu Chien Peter Lin
Cc: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp,
prabhakar.mahadev-lad.rj, ajones, heiko, samuel, geert+renesas,
n.shubin, dminus, ycliang, tim609, locus84, dylan
On Thu, Sep 7, 2023 at 7:48 AM Yu Chien Peter Lin
<peterlin@andestech.com> wrote:
>
> Currently, the implementation of the RISC-V INTC driver uses the
> interrupt cause as hwirq and has a limitation of supporting a
> maximum of 64 hwirqs. However, according to the privileged spec,
> interrupt cause >= 16 are defined for platform use.
>
> This limitation prevents us from fully utilizing the available
> local interrupt sources. Additionally, the hwirqs used on RISC-V
> are sparse, with only interrupt numbers 1, 5 and 9 (plus Sscofpmf
> or T-Head's PMU irq) being currently used for supervisor mode.
>
> The patch switches to using irq_domain_create_tree() which
> creates the radix tree map, allowing us to handle a larger
> number of hwirqs.
>
> Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
> Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
>
> ---
> There are 3 hwirqs of local interrupt source exceed 64 defined in
> AX45MP datasheet [1] Table 56: AX45MP-1C scause Value After Trap:
> - 256+16 Slave port ECC error interrupt (S-mode)
> - 256+17 Bus write transaction error interrupt (S-mode)
> - 256+18 Performance monitor overflow interrupt(S-mode)
>
> [1] http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
> ---
> drivers/irqchip/irq-riscv-intc.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> index 4adeee1bc391..76e1229c45de 100644
> --- a/drivers/irqchip/irq-riscv-intc.c
> +++ b/drivers/irqchip/irq-riscv-intc.c
> @@ -24,8 +24,8 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
> {
> unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;
>
> - if (unlikely(cause >= BITS_PER_LONG))
> - panic("unexpected interrupt cause");
> + if (!irq_find_mapping(intc_domain, cause))
> + panic("unexpected interrupt cause: %ld", cause);
Checking irq_find_mapping() is redundant here instead check the return
value of generic_handle_domain_irq() and print warning on error.
>
> generic_handle_domain_irq(intc_domain, cause);
> }
> @@ -117,8 +117,8 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)
> {
> int rc;
>
> - intc_domain = irq_domain_create_linear(fn, BITS_PER_LONG,
> - &riscv_intc_domain_ops, NULL);
> + intc_domain = irq_domain_create_tree(fn, &riscv_intc_domain_ops,
> + NULL);
This is incomplete because you have additional customization on-top-of
vanilla RISC-V INTC.
I suggest to do the following:
1) Define an enum of types of INTC (such as generic, andestech, etc)
2) Define new compatible string "andestec,cpu-intc" for you custom INTC
and pass that information to riscv_intc_init_common()
3) Extend riscv_intc_domain_map() to use custom andestech_intc_chip
for the custom local irqs. The andestech_intc_chip will provide andes
specific mask/unmask mechanism.
> if (!intc_domain) {
> pr_err("unable to add IRQ domain\n");
> return -ENXIO;
> @@ -132,8 +132,6 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)
>
> riscv_set_intc_hwnode_fn(riscv_intc_hwnode);
>
> - pr_info("%d local interrupts mapped\n", BITS_PER_LONG);
> -
> return 0;
> }
>
> --
> 2.34.1
>
Regards,
Anup
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] riscv: errata: Add Andes PMU errata
2023-09-07 2:48 ` Samuel Holland
@ 2023-09-11 2:38 ` Yu-Chien Peter Lin
0 siblings, 0 replies; 15+ messages in thread
From: Yu-Chien Peter Lin @ 2023-09-11 2:38 UTC (permalink / raw)
To: Samuel Holland
Cc: ajones, heiko, samuel, geert+renesas, n.shubin, dminus, ycliang,
tim609, locus84, dylan, linux-riscv, linux-arm-kernel,
linux-kernel, linux-perf-users, paul.walmsley, palmer, aou,
conor.dooley, atishp, anup, prabhakar.mahadev-lad.rj
Hi Samuel,
On Wed, Sep 06, 2023 at 09:48:35PM -0500, Samuel Holland wrote:
> If the code here needs to be different, then it must check that it is actually
> running on an Andes core, not just that the errata Kconfig option is enabled.
Thank you for catching this, will fix in PATCH v2.
> However, I suggest setting riscv_pmu_irq_num to the real IRQ number:
> riscv_pmu_irq_num = ANDES_SLI_CAUSE_BASE + ANDES_RV_IRQ_PMU;
> and then adding a new variable for the mask:
> riscv_pmu_irq_mask = BIT(riscv_pmu_irq_num % BITS_PER_LONG);
> which handles the large IRQ number somewhat more generically, and reduces the
> number of bit operations needed elsewhere in the driver.
I will make changes according to your suggestions. Thank you!
> Or we could use IRQ chip operations here instead of direct CSR acccess. But
> maybe the direct CSR access is needed for performance?
>
> Regards,
> Samuel
Best regards,
Peter Lin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] riscv: errata: Add Andes PMU errata
2023-09-07 11:02 ` Conor Dooley
@ 2023-09-11 2:48 ` Yu-Chien Peter Lin
2023-09-11 12:35 ` Conor Dooley
0 siblings, 1 reply; 15+ messages in thread
From: Yu-Chien Peter Lin @ 2023-09-11 2:48 UTC (permalink / raw)
To: Conor Dooley
Cc: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp, anup,
prabhakar.mahadev-lad.rj, ajones, heiko, samuel, geert+renesas,
n.shubin, dminus, ycliang, tim609, locus84, dylan
On Thu, Sep 07, 2023 at 12:02:46PM +0100, Conor Dooley wrote:
> On Thu, Sep 07, 2023 at 10:27:03AM +0100, Conor Dooley wrote:
> > Hey,
> >
> > On Thu, Sep 07, 2023 at 10:16:34AM +0800, Yu Chien Peter Lin wrote:
> > > Before the ratification of Sscofpmf, the Andes PMU extension
> > > implements the same mechanism and is compatible with existing
> > > SBI PMU driver of perf to support event sampling and mode
> > > filtering with programmable hardware performance counters.
> >
> > If it actually was, you'd not need to modify the driver ;)
> >
> > > This patch adds PMU support for Andes 45-series CPUs by
> > > introducing a CPU errata.
> >
> > I don't really understand this in all honesty. You don't have Sscofpmf
> > support with a bug, you have something that is Sscofpmf-adjactent that
> > predates it. Why claim to support an extension that you do not, only to
> > have to come along and try to clean things up afterwards, instead of
> > accurately declaring what you do support from the outset?
>
> Reading this again, I don't think that I have been particularly clear,
> sorry. My point is that this is not a fix for a bug in your
> implementation of Sscofpmf, but rather adding probing for what is
> effectively a custom ISA extension that happens to be very similar to
> the standard one. As it is not an implementation bug, errata should
> not be abused to support vendor extensions, and either DT or ACPI should
> be used to inform the operating system about its presence.
>
> Cheers,
> Conor.
>
> >
> > (and just because someone already got away with it, doesn't mean that
> > you get a free pass on it, sorry)
Apologize for any confusion caused by the name. I thought it would make it
easier to find the related functions and files in OpenSBI, didn't expect
that it may have misled people to abuse the use of errata, you are right,
I should have chosen my words more carefully.
In my opinion, this is simply a pre-sepc solution to enable missing perf
features before the standard is finalized. The underlying logic remains the
same, so we can still use the errata to patch a few lines of CSR accesses
and align with other vendors. This way, we can make minimal changes and
avoid performance overhead to the driver.
> > Thanks,
> > Conor.
> >
> > > Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> > > Signed-off-by: Locus Wei-Han Chen <locus84@andestech.com>
> >
> > btw, what did Locus Wei-Han Chen do here? Are you missing
> > a Co-developed-by: tag?
Yes I missed the CD-tag, will fix it.
Thanks for the review.
> > > Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
> > > Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> > > ---
> > > arch/riscv/Kconfig.errata | 13 ++++++++
> > > arch/riscv/errata/andes/errata.c | 45 +++++++++++++++++++++++++++-
> > > arch/riscv/include/asm/errata_list.h | 43 ++++++++++++++++++++++++--
> > > drivers/perf/riscv_pmu_sbi.c | 20 +++++++++----
> > > 4 files changed, 111 insertions(+), 10 deletions(-)
Best regards,
Peter Lin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH 2/4] irqchip/riscv-intc: Support large non-standard hwirq number
2023-09-07 10:22 ` Clément Léger
@ 2023-09-11 8:04 ` Yu-Chien Peter Lin
0 siblings, 0 replies; 15+ messages in thread
From: Yu-Chien Peter Lin @ 2023-09-11 8:04 UTC (permalink / raw)
To: Clément Léger
Cc: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp, anup,
prabhakar.mahadev-lad.rj, ajones, heiko, samuel, geert+renesas,
n.shubin, dminus, ycliang, tim609, locus84, dylan
On Thu, Sep 07, 2023 at 12:22:55PM +0200, Clément Léger wrote:
>
>
> On 07/09/2023 04:16, Yu Chien Peter Lin wrote:
> > Currently, the implementation of the RISC-V INTC driver uses the
> > interrupt cause as hwirq and has a limitation of supporting a
> > maximum of 64 hwirqs. However, according to the privileged spec,
> > interrupt cause >= 16 are defined for platform use.
> >
> > This limitation prevents us from fully utilizing the available
> > local interrupt sources. Additionally, the hwirqs used on RISC-V
> > are sparse, with only interrupt numbers 1, 5 and 9 (plus Sscofpmf
> > or T-Head's PMU irq) being currently used for supervisor mode.
> >
> > The patch switches to using irq_domain_create_tree() which
> > creates the radix tree map, allowing us to handle a larger
> > number of hwirqs.
> >
> > Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> > Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
> > Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> >
> > ---
> > There are 3 hwirqs of local interrupt source exceed 64 defined in
> > AX45MP datasheet [1] Table 56: AX45MP-1C scause Value After Trap:
> > - 256+16 Slave port ECC error interrupt (S-mode)
> > - 256+17 Bus write transaction error interrupt (S-mode)
> > - 256+18 Performance monitor overflow interrupt(S-mode)
> >
> > [1] http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
> > ---
> > drivers/irqchip/irq-riscv-intc.c | 10 ++++------
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> > index 4adeee1bc391..76e1229c45de 100644
> > --- a/drivers/irqchip/irq-riscv-intc.c
> > +++ b/drivers/irqchip/irq-riscv-intc.c
> > @@ -24,8 +24,8 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
> > {
> > unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;
> >
> > - if (unlikely(cause >= BITS_PER_LONG))
> > - panic("unexpected interrupt cause");
> > + if (!irq_find_mapping(intc_domain, cause))
> > + panic("unexpected interrupt cause: %ld", cause);
>
> Hi Yu,
>
> It seems like generic_handle_domain_irq() returns -EINVAL if provided
> with NULL (which will happen if the interrupt does not have a mapping
> due to __irq_resolve_mapping returning NULL) so maybe it is possible to
> remove this check (since __irq_resolve_mapping() is also called by
> generic_handle_domain_irq()) and panic if generic_handle_domain_irq()
> returns -EINVAL ? That would avoid calling __irq_resolve_mapping() twice
> for really rare cases.
>
> Clément
Hi Clément,
Thanks for catching this, will fix it in the patch v2.
Best regards,
Peter Lin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH 2/4] irqchip/riscv-intc: Support large non-standard hwirq number
2023-09-07 13:06 ` Anup Patel
@ 2023-09-11 8:12 ` Yu-Chien Peter Lin
0 siblings, 0 replies; 15+ messages in thread
From: Yu-Chien Peter Lin @ 2023-09-11 8:12 UTC (permalink / raw)
To: Anup Patel
Cc: linux-riscv, linux-arm-kernel, linux-kernel, linux-perf-users,
paul.walmsley, palmer, aou, conor.dooley, atishp,
prabhakar.mahadev-lad.rj, ajones, heiko, samuel, geert+renesas,
n.shubin, dminus, ycliang, tim609, locus84, dylan
On Thu, Sep 07, 2023 at 06:36:52PM +0530, Anup Patel wrote:
> On Thu, Sep 7, 2023 at 7:48 AM Yu Chien Peter Lin
> <peterlin@andestech.com> wrote:
> >
> > Currently, the implementation of the RISC-V INTC driver uses the
> > interrupt cause as hwirq and has a limitation of supporting a
> > maximum of 64 hwirqs. However, according to the privileged spec,
> > interrupt cause >= 16 are defined for platform use.
> >
> > This limitation prevents us from fully utilizing the available
> > local interrupt sources. Additionally, the hwirqs used on RISC-V
> > are sparse, with only interrupt numbers 1, 5 and 9 (plus Sscofpmf
> > or T-Head's PMU irq) being currently used for supervisor mode.
> >
> > The patch switches to using irq_domain_create_tree() which
> > creates the radix tree map, allowing us to handle a larger
> > number of hwirqs.
> >
> > Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> > Reviewed-by: Charles Ci-Jyun Wu <dminus@andestech.com>
> > Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> >
> > ---
> > There are 3 hwirqs of local interrupt source exceed 64 defined in
> > AX45MP datasheet [1] Table 56: AX45MP-1C scause Value After Trap:
> > - 256+16 Slave port ECC error interrupt (S-mode)
> > - 256+17 Bus write transaction error interrupt (S-mode)
> > - 256+18 Performance monitor overflow interrupt(S-mode)
> >
> > [1] http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
> > ---
> > drivers/irqchip/irq-riscv-intc.c | 10 ++++------
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> > index 4adeee1bc391..76e1229c45de 100644
> > --- a/drivers/irqchip/irq-riscv-intc.c
> > +++ b/drivers/irqchip/irq-riscv-intc.c
> > @@ -24,8 +24,8 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
> > {
> > unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;
> >
> > - if (unlikely(cause >= BITS_PER_LONG))
> > - panic("unexpected interrupt cause");
> > + if (!irq_find_mapping(intc_domain, cause))
> > + panic("unexpected interrupt cause: %ld", cause);
>
> Checking irq_find_mapping() is redundant here instead check the return
> value of generic_handle_domain_irq() and print warning on error.
>
> >
> > generic_handle_domain_irq(intc_domain, cause);
> > }
> > @@ -117,8 +117,8 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)
> > {
> > int rc;
> >
> > - intc_domain = irq_domain_create_linear(fn, BITS_PER_LONG,
> > - &riscv_intc_domain_ops, NULL);
> > + intc_domain = irq_domain_create_tree(fn, &riscv_intc_domain_ops,
> > + NULL);
>
> This is incomplete because you have additional customization on-top-of
> vanilla RISC-V INTC.
>
> I suggest to do the following:
> 1) Define an enum of types of INTC (such as generic, andestech, etc)
> 2) Define new compatible string "andestec,cpu-intc" for you custom INTC
> and pass that information to riscv_intc_init_common()
> 3) Extend riscv_intc_domain_map() to use custom andestech_intc_chip
> for the custom local irqs. The andestech_intc_chip will provide andes
> specific mask/unmask mechanism.
Hi Anup,
Sure, we will introduce the Andes INTC for a custom IRQ chip.
Thanks,
Peter Lin
> > if (!intc_domain) {
> > pr_err("unable to add IRQ domain\n");
> > return -ENXIO;
> > @@ -132,8 +132,6 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)
> >
> > riscv_set_intc_hwnode_fn(riscv_intc_hwnode);
> >
> > - pr_info("%d local interrupts mapped\n", BITS_PER_LONG);
> > -
> > return 0;
> > }
> >
> > --
> > 2.34.1
> >
>
> Regards,
> Anup
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] riscv: errata: Add Andes PMU errata
2023-09-11 2:48 ` Yu-Chien Peter Lin
@ 2023-09-11 12:35 ` Conor Dooley
0 siblings, 0 replies; 15+ messages in thread
From: Conor Dooley @ 2023-09-11 12:35 UTC (permalink / raw)
To: Yu-Chien Peter Lin
Cc: Conor Dooley, linux-riscv, linux-arm-kernel, linux-kernel,
linux-perf-users, paul.walmsley, palmer, aou, atishp, anup,
prabhakar.mahadev-lad.rj, ajones, heiko, samuel, geert+renesas,
n.shubin, dminus, ycliang, tim609, locus84, dylan
[-- Attachment #1: Type: text/plain, Size: 3523 bytes --]
On Mon, Sep 11, 2023 at 10:48:44AM +0800, Yu-Chien Peter Lin wrote:
> On Thu, Sep 07, 2023 at 12:02:46PM +0100, Conor Dooley wrote:
> > On Thu, Sep 07, 2023 at 10:27:03AM +0100, Conor Dooley wrote:
> > > Hey,
> > >
> > > On Thu, Sep 07, 2023 at 10:16:34AM +0800, Yu Chien Peter Lin wrote:
> > > > Before the ratification of Sscofpmf, the Andes PMU extension
> > > > implements the same mechanism and is compatible with existing
> > > > SBI PMU driver of perf to support event sampling and mode
> > > > filtering with programmable hardware performance counters.
> > >
> > > If it actually was, you'd not need to modify the driver ;)
> > >
> > > > This patch adds PMU support for Andes 45-series CPUs by
> > > > introducing a CPU errata.
> > >
> > > I don't really understand this in all honesty. You don't have Sscofpmf
> > > support with a bug, you have something that is Sscofpmf-adjactent that
> > > predates it. Why claim to support an extension that you do not, only to
> > > have to come along and try to clean things up afterwards, instead of
> > > accurately declaring what you do support from the outset?
> >
> > Reading this again, I don't think that I have been particularly clear,
> > sorry. My point is that this is not a fix for a bug in your
> > implementation of Sscofpmf, but rather adding probing for what is
> > effectively a custom ISA extension that happens to be very similar to
> > the standard one. As it is not an implementation bug, errata should
> > not be abused to support vendor extensions, and either DT or ACPI should
> > be used to inform the operating system about its presence.
> >
> > Cheers,
> > Conor.
> >
> > >
> > > (and just because someone already got away with it, doesn't mean that
> > > you get a free pass on it, sorry)
>
> Apologize for any confusion caused by the name. I thought it would make it
> easier to find the related functions and files in OpenSBI, didn't expect
> that it may have misled people to abuse the use of errata, you are right,
> I should have chosen my words more carefully.
I'm not sure what you mean here. The commit message is not the problem,
nor is the comparison in it to Sscofpmf - my issue is with pretending
that things like this are errata and using mvendorid etc to probe them.
> In my opinion, this is simply a pre-sepc solution to enable missing perf
> features before the standard is finalized.
Right, I don't think there's any disagreement about that. What I am
objecting to is adding more cases of pretending that something never
intended to be the standard extension is the standard extension with an
erratum. I know the T-Head stuff does do exactly this, but I don't think
we should continue to allow feature probing using the errata framework,
but instead communicate supported features via DT or ACPI. (If I could
re-review the T-Head PMU patch now, I'd say the same thing there).
Maybe Palmer et al disagree & there is some extenuating circumstance
that applies here.
> The underlying logic remains the
> same, so we can still use the errata to patch a few lines of CSR accesses
> and align with other vendors. This way, we can make minimal changes and
> avoid performance overhead to the driver.
I think you're conflating errata with alternatives. I have no problem
with the use of alternatives for this purpose & the perf parts of the
patch are okay to me, if the overhead of having a platform specific ops
struct is unacceptable.
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-09-11 21:36 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-07 2:16 [PATCH 0/4] Support Andes PMU extension Yu Chien Peter Lin
2023-09-07 2:16 ` [PATCH 1/4] riscv: errata: Rename defines for Andes Yu Chien Peter Lin
2023-09-07 2:16 ` [RFC PATCH 2/4] irqchip/riscv-intc: Support large non-standard hwirq number Yu Chien Peter Lin
2023-09-07 10:22 ` Clément Léger
2023-09-11 8:04 ` Yu-Chien Peter Lin
2023-09-07 13:06 ` Anup Patel
2023-09-11 8:12 ` Yu-Chien Peter Lin
2023-09-07 2:16 ` [PATCH 3/4] riscv: errata: Add Andes PMU errata Yu Chien Peter Lin
2023-09-07 2:48 ` Samuel Holland
2023-09-11 2:38 ` Yu-Chien Peter Lin
2023-09-07 9:27 ` Conor Dooley
2023-09-07 11:02 ` Conor Dooley
2023-09-11 2:48 ` Yu-Chien Peter Lin
2023-09-11 12:35 ` Conor Dooley
2023-09-07 2:16 ` [PATCH 4/4] riscv: andes: Support symbolic FW and HW raw events Yu Chien Peter Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).