| Message ID | 20230907021635.1002738-4-peterlin@andestech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support Andes PMU extension | expand |
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;
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
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
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
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
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.
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;