diff mbox series

[v3,09/17] riscv: drivers: Convert xandespmu to use the vendor extension framework

Message ID 20240420-dev-charlie-support_thead_vector_6_9-v3-9-67cff4271d1d@rivosinc.com (mailing list archive)
State New
Headers show
Series riscv: Support vendor extensions and xtheadvector | expand

Commit Message

Charlie Jenkins April 21, 2024, 1:04 a.m. UTC
Migrate xandespmu out of riscv_isa_ext and into a new Andes-specific
vendor namespace.

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
---
 arch/riscv/Kconfig.vendor                        | 10 +++++++
 arch/riscv/include/asm/hwcap.h                   |  1 -
 arch/riscv/include/asm/vendor_extensions/andes.h | 19 +++++++++++++
 arch/riscv/kernel/cpufeature.c                   |  1 -
 arch/riscv/kernel/vendor_extensions.c            | 11 ++++++++
 arch/riscv/kernel/vendor_extensions/Makefile     |  1 +
 arch/riscv/kernel/vendor_extensions/andes.c      | 35 ++++++++++++++++++++++++
 drivers/perf/riscv_pmu_sbi.c                     |  8 ++++--
 8 files changed, 81 insertions(+), 5 deletions(-)

Comments

Conor Dooley April 26, 2024, 4:25 p.m. UTC | #1
On Sat, Apr 20, 2024 at 06:04:41PM -0700, Charlie Jenkins wrote:
> Migrate xandespmu out of riscv_isa_ext and into a new Andes-specific
> vendor namespace.
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 8cbe6e5f9c39..84760ce61e03 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -24,6 +24,8 @@
>  #include <asm/errata_list.h>
>  #include <asm/sbi.h>
>  #include <asm/cpufeature.h>
> +#include <asm/vendorid_list.h>
> +#include <asm/vendor_extensions/andes.h>
>  
>  #define ALT_SBI_PMU_OVERFLOW(__ovl)					\
>  asm volatile(ALTERNATIVE_2(						\
> @@ -32,7 +34,7 @@ asm volatile(ALTERNATIVE_2(						\
>  		THEAD_VENDOR_ID, ERRATA_THEAD_PMU,			\
>  		CONFIG_ERRATA_THEAD_PMU,				\
>  	"csrr %0, " __stringify(ANDES_CSR_SCOUNTEROF),			\
> -		0, RISCV_ISA_EXT_XANDESPMU,				\
> +		ANDES_VENDOR_ID, RISCV_ISA_VENDOR_EXT_XANDESPMU,	\
>  		CONFIG_ANDES_CUSTOM_PMU)				\
>  	: "=r" (__ovl) :						\
>  	: "memory")
> @@ -41,7 +43,7 @@ asm volatile(ALTERNATIVE_2(						\
>  asm volatile(ALTERNATIVE(						\
>  	"csrc " __stringify(CSR_IP) ", %0\n\t",				\
>  	"csrc " __stringify(ANDES_CSR_SLIP) ", %0\n\t",			\
> -		0, RISCV_ISA_EXT_XANDESPMU,				\
> +		ANDES_VENDOR_ID, RISCV_ISA_VENDOR_EXT_XANDESPMU,	\
>  		CONFIG_ANDES_CUSTOM_PMU)				\
>  	: : "r"(__irq_mask)						\
>  	: "memory")
> @@ -837,7 +839,7 @@ 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 (riscv_isa_extension_available(NULL, XANDESPMU) &&
> +	} else if (riscv_isa_vendor_extension_available(-1, XANDESPMU) &&

What's the rationale for this not using riscv_has_extension_unlikely()?
Happens once in probe so don't bother? I forget if we discussed it when
the code was added, but it would save us from the NULL/-1 syntax,
neither of which I think is a good interface.

Also, I'd prob drop the "drivers" from $subject.

I'll come back and look at the rest of this Monday, it's a sunny Friday
here and I've still got my devicetree patch queue to clear..

Cheers,
Conor.

>  		   IS_ENABLED(CONFIG_ANDES_CUSTOM_PMU)) {
>  		riscv_pmu_irq_num = ANDES_SLI_CAUSE_BASE + ANDES_RV_IRQ_PMOVI;
>  		riscv_pmu_use_irq = true;
> 
> -- 
> 2.44.0
>
Charlie Jenkins April 26, 2024, 8:34 p.m. UTC | #2
On Fri, Apr 26, 2024 at 05:25:20PM +0100, Conor Dooley wrote:
> On Sat, Apr 20, 2024 at 06:04:41PM -0700, Charlie Jenkins wrote:
> > Migrate xandespmu out of riscv_isa_ext and into a new Andes-specific
> > vendor namespace.
> > diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> > index 8cbe6e5f9c39..84760ce61e03 100644
> > --- a/drivers/perf/riscv_pmu_sbi.c
> > +++ b/drivers/perf/riscv_pmu_sbi.c
> > @@ -24,6 +24,8 @@
> >  #include <asm/errata_list.h>
> >  #include <asm/sbi.h>
> >  #include <asm/cpufeature.h>
> > +#include <asm/vendorid_list.h>
> > +#include <asm/vendor_extensions/andes.h>
> >  
> >  #define ALT_SBI_PMU_OVERFLOW(__ovl)					\
> >  asm volatile(ALTERNATIVE_2(						\
> > @@ -32,7 +34,7 @@ asm volatile(ALTERNATIVE_2(						\
> >  		THEAD_VENDOR_ID, ERRATA_THEAD_PMU,			\
> >  		CONFIG_ERRATA_THEAD_PMU,				\
> >  	"csrr %0, " __stringify(ANDES_CSR_SCOUNTEROF),			\
> > -		0, RISCV_ISA_EXT_XANDESPMU,				\
> > +		ANDES_VENDOR_ID, RISCV_ISA_VENDOR_EXT_XANDESPMU,	\
> >  		CONFIG_ANDES_CUSTOM_PMU)				\
> >  	: "=r" (__ovl) :						\
> >  	: "memory")
> > @@ -41,7 +43,7 @@ asm volatile(ALTERNATIVE_2(						\
> >  asm volatile(ALTERNATIVE(						\
> >  	"csrc " __stringify(CSR_IP) ", %0\n\t",				\
> >  	"csrc " __stringify(ANDES_CSR_SLIP) ", %0\n\t",			\
> > -		0, RISCV_ISA_EXT_XANDESPMU,				\
> > +		ANDES_VENDOR_ID, RISCV_ISA_VENDOR_EXT_XANDESPMU,	\
> >  		CONFIG_ANDES_CUSTOM_PMU)				\
> >  	: : "r"(__irq_mask)						\
> >  	: "memory")
> > @@ -837,7 +839,7 @@ 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 (riscv_isa_extension_available(NULL, XANDESPMU) &&
> > +	} else if (riscv_isa_vendor_extension_available(-1, XANDESPMU) &&
> 
> What's the rationale for this not using riscv_has_extension_unlikely()?
> Happens once in probe so don't bother? I forget if we discussed it when
> the code was added, but it would save us from the NULL/-1 syntax,
> neither of which I think is a good interface.

Doesn't look like something that was ever commented on in the series,
but I may have missed it. I can change this to use the alternatives.

This also wasn't supposed to be -1, it's supposed to be the id of the
vendor.

> 
> Also, I'd prob drop the "drivers" from $subject.
> 
> I'll come back and look at the rest of this Monday, it's a sunny Friday
> here and I've still got my devicetree patch queue to clear..
> 

- Charlie

> Cheers,
> Conor.
> 
> >  		   IS_ENABLED(CONFIG_ANDES_CUSTOM_PMU)) {
> >  		riscv_pmu_irq_num = ANDES_SLI_CAUSE_BASE + ANDES_RV_IRQ_PMOVI;
> >  		riscv_pmu_use_irq = true;
> > 
> > -- 
> > 2.44.0
> >
Conor Dooley April 26, 2024, 8:46 p.m. UTC | #3
On Fri, Apr 26, 2024 at 01:34:19PM -0700, Charlie Jenkins wrote:
> On Fri, Apr 26, 2024 at 05:25:20PM +0100, Conor Dooley wrote:
> > On Sat, Apr 20, 2024 at 06:04:41PM -0700, Charlie Jenkins wrote:
> > > Migrate xandespmu out of riscv_isa_ext and into a new Andes-specific
> > > vendor namespace.
> > > diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> > > index 8cbe6e5f9c39..84760ce61e03 100644
> > > --- a/drivers/perf/riscv_pmu_sbi.c
> > > +++ b/drivers/perf/riscv_pmu_sbi.c
> > > @@ -24,6 +24,8 @@
> > >  #include <asm/errata_list.h>
> > >  #include <asm/sbi.h>
> > >  #include <asm/cpufeature.h>
> > > +#include <asm/vendorid_list.h>
> > > +#include <asm/vendor_extensions/andes.h>
> > >  
> > >  #define ALT_SBI_PMU_OVERFLOW(__ovl)					\
> > >  asm volatile(ALTERNATIVE_2(						\
> > > @@ -32,7 +34,7 @@ asm volatile(ALTERNATIVE_2(						\
> > >  		THEAD_VENDOR_ID, ERRATA_THEAD_PMU,			\
> > >  		CONFIG_ERRATA_THEAD_PMU,				\
> > >  	"csrr %0, " __stringify(ANDES_CSR_SCOUNTEROF),			\
> > > -		0, RISCV_ISA_EXT_XANDESPMU,				\
> > > +		ANDES_VENDOR_ID, RISCV_ISA_VENDOR_EXT_XANDESPMU,	\
> > >  		CONFIG_ANDES_CUSTOM_PMU)				\
> > >  	: "=r" (__ovl) :						\
> > >  	: "memory")
> > > @@ -41,7 +43,7 @@ asm volatile(ALTERNATIVE_2(						\
> > >  asm volatile(ALTERNATIVE(						\
> > >  	"csrc " __stringify(CSR_IP) ", %0\n\t",				\
> > >  	"csrc " __stringify(ANDES_CSR_SLIP) ", %0\n\t",			\
> > > -		0, RISCV_ISA_EXT_XANDESPMU,				\
> > > +		ANDES_VENDOR_ID, RISCV_ISA_VENDOR_EXT_XANDESPMU,	\
> > >  		CONFIG_ANDES_CUSTOM_PMU)				\
> > >  	: : "r"(__irq_mask)						\
> > >  	: "memory")
> > > @@ -837,7 +839,7 @@ 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 (riscv_isa_extension_available(NULL, XANDESPMU) &&
> > > +	} else if (riscv_isa_vendor_extension_available(-1, XANDESPMU) &&
> > 
> > What's the rationale for this not using riscv_has_extension_unlikely()?
> > Happens once in probe so don't bother? I forget if we discussed it when
> > the code was added, but it would save us from the NULL/-1 syntax,
> > neither of which I think is a good interface.
> 
> Doesn't look like something that was ever commented on in the series,
> but I may have missed it. I can change this to use the alternatives.

Yeha, not really a question for you but thinking aloud and wondering if
someone would remind me. I really don't like
riscv_isa_extension_available() because it doesn't respect config
options etc, but ultimately I think the series that Clement is currently
working on for Zc* is could be the saviour there, as the callbacks his
most recent version has I think could make it much easier to hook in and
turn off extensions. Should be helpful for the sort of confusing shit
that Eric was complaining about last week on Andy's vector series.

> 
> This also wasn't supposed to be -1, it's supposed to be the id of the
> vendor.
> 
> > 
> > Also, I'd prob drop the "drivers" from $subject.
> > 
> > I'll come back and look at the rest of this Monday, it's a sunny Friday
> > here and I've still got my devicetree patch queue to clear..
> > 
> 
> - Charlie
> 
> > Cheers,
> > Conor.
> > 
> > >  		   IS_ENABLED(CONFIG_ANDES_CUSTOM_PMU)) {
> > >  		riscv_pmu_irq_num = ANDES_SLI_CAUSE_BASE + ANDES_RV_IRQ_PMOVI;
> > >  		riscv_pmu_use_irq = true;
> > > 
> > > -- 
> > > 2.44.0
> > > 
> 
>
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor
index d57254f28ea6..d5555ed696fb 100644
--- a/arch/riscv/Kconfig.vendor
+++ b/arch/riscv/Kconfig.vendor
@@ -7,3 +7,13 @@  config RISCV_ISA_VENDOR_EXT_THEAD
 	  requested by hardware probing to be ignored.
 
 	  If you don't know what to do here, say Y.
+
+config RISCV_ISA_VENDOR_EXT_ANDES
+	bool "Andes vendor extension support"
+	default y
+	help
+	  Say N here if you want to disable all Andes vendor extension
+	  support. This will cause any Andes vendor extensions that are
+	  requested by hardware probing to be ignored.
+
+	  If you don't know what to do here, say Y.
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e17d0078a651..1f2d2599c655 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -80,7 +80,6 @@ 
 #define RISCV_ISA_EXT_ZFA		71
 #define RISCV_ISA_EXT_ZTSO		72
 #define RISCV_ISA_EXT_ZACAS		73
-#define RISCV_ISA_EXT_XANDESPMU		74
 
 #define RISCV_ISA_EXT_XLINUXENVCFG	127
 
diff --git a/arch/riscv/include/asm/vendor_extensions/andes.h b/arch/riscv/include/asm/vendor_extensions/andes.h
new file mode 100644
index 000000000000..54b0ad1a409d
--- /dev/null
+++ b/arch/riscv/include/asm/vendor_extensions/andes.h
@@ -0,0 +1,19 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_RISCV_VENDOR_EXTENSIONS_ANDES_H
+#define _ASM_RISCV_VENDOR_EXTENSIONS_ANDES_H
+
+#include <asm/vendor_extensions.h>
+
+#include <linux/types.h>
+
+#define RISCV_ISA_VENDOR_EXT_XANDESPMU		0
+
+/*
+ * Extension keys should be strictly less than max.
+ * It is safe to increment this when necessary.
+ */
+#define RISCV_ISA_VENDOR_EXT_MAX_ANDES			32
+
+extern const struct riscv_isa_vendor_ext_data_list riscv_isa_vendor_ext_list_andes;
+
+#endif
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 17371887abcc..0cef08a3d891 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -289,7 +289,6 @@  const struct riscv_isa_ext_data riscv_isa_ext[] = {
 	__RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
 	__RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
 	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
-	__RISCV_ISA_EXT_DATA(xandespmu, RISCV_ISA_EXT_XANDESPMU),
 };
 
 const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
diff --git a/arch/riscv/kernel/vendor_extensions.c b/arch/riscv/kernel/vendor_extensions.c
index eced93eec5a6..e31f16b8cec9 100644
--- a/arch/riscv/kernel/vendor_extensions.c
+++ b/arch/riscv/kernel/vendor_extensions.c
@@ -5,6 +5,7 @@ 
 
 #include <asm/vendorid_list.h>
 #include <asm/vendor_extensions.h>
+#include <asm/vendor_extensions/andes.h>
 #include <asm/vendor_extensions/thead.h>
 
 #include <linux/array_size.h>
@@ -14,6 +15,9 @@  const struct riscv_isa_vendor_ext_data_list *riscv_isa_vendor_ext_list[] = {
 #ifdef CONFIG_RISCV_ISA_VENDOR_EXT_THEAD
 	&riscv_isa_vendor_ext_list_thead,
 #endif
+#ifdef CONFIG_RISCV_ISA_VENDOR_EXT_ANDES
+	&riscv_isa_vendor_ext_list_andes,
+#endif
 };
 
 const size_t riscv_isa_vendor_ext_list_size = ARRAY_SIZE(riscv_isa_vendor_ext_list);
@@ -42,6 +46,13 @@  bool __riscv_isa_vendor_extension_available(int cpu, unsigned long vendor, unsig
 		cpu_bmap = riscv_isa_vendor_ext_list_thead.per_hart_vendor_bitmap;
 		bmap_size = riscv_isa_vendor_ext_list_thead.bitmap_size;
 		break;
+#endif
+#ifdef CONFIG_RISCV_VENDOR_EXT_ANDES
+	case ANDES_VENDOR_ID:
+		bmap = riscv_isa_vendor_ext_list_andes.vendor_bitmap;
+		cpu_bmap = riscv_isa_vendor_ext_list_andes.per_hart_vendor_bitmap;
+		bmap_size = riscv_isa_vendor_ext_list_andes.bitmap_size;
+		break;
 #endif
 	default:
 		return false;
diff --git a/arch/riscv/kernel/vendor_extensions/Makefile b/arch/riscv/kernel/vendor_extensions/Makefile
index 3383066baaab..8f1c5a4dc38f 100644
--- a/arch/riscv/kernel/vendor_extensions/Makefile
+++ b/arch/riscv/kernel/vendor_extensions/Makefile
@@ -1,3 +1,4 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-$(CONFIG_RISCV_ISA_VENDOR_EXT_THEAD)	+= thead.o
+obj-$(CONFIG_RISCV_ISA_VENDOR_EXT_ANDES)	+= andes.o
diff --git a/arch/riscv/kernel/vendor_extensions/andes.c b/arch/riscv/kernel/vendor_extensions/andes.c
new file mode 100644
index 000000000000..8451106d2e07
--- /dev/null
+++ b/arch/riscv/kernel/vendor_extensions/andes.c
@@ -0,0 +1,35 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <asm/cpufeature.h>
+#include <asm/vendor_extensions.h>
+#include <asm/vendor_extensions/andes.h>
+
+#include <linux/array_size.h>
+#include <linux/types.h>
+
+const struct riscv_isa_ext_data riscv_isa_vendor_ext_andes[] = {
+	__RISCV_ISA_EXT_DATA(xandespmu, RISCV_ISA_VENDOR_EXT_XANDESPMU),
+};
+
+/*
+ * The first member of this struct must be a bitmap named isa so it can be
+ * compatible with riscv_isainfo even though the sizes of the bitmaps may be
+ * different.
+ */
+struct riscv_isavendorinfo_andes {
+	DECLARE_BITMAP(isa, RISCV_ISA_VENDOR_EXT_MAX_ANDES);
+};
+
+/* Hart specific T-Head vendor extension support */
+static struct riscv_isavendorinfo_andes hart_vendorinfo_andes[NR_CPUS];
+
+/* Set of T-Head vendor extensions supported on all harts */
+DECLARE_BITMAP(vendorinfo_andes, RISCV_ISA_VENDOR_EXT_MAX_ANDES);
+
+const struct riscv_isa_vendor_ext_data_list riscv_isa_vendor_ext_list_andes = {
+	.ext_data = riscv_isa_vendor_ext_andes,
+	.per_hart_vendor_bitmap = (struct riscv_isainfo *)hart_vendorinfo_andes,
+	.vendor_bitmap = vendorinfo_andes,
+	.ext_data_count = ARRAY_SIZE(riscv_isa_vendor_ext_andes),
+	.bitmap_size = RISCV_ISA_VENDOR_EXT_MAX_ANDES
+};
diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
index 8cbe6e5f9c39..84760ce61e03 100644
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -24,6 +24,8 @@ 
 #include <asm/errata_list.h>
 #include <asm/sbi.h>
 #include <asm/cpufeature.h>
+#include <asm/vendorid_list.h>
+#include <asm/vendor_extensions/andes.h>
 
 #define ALT_SBI_PMU_OVERFLOW(__ovl)					\
 asm volatile(ALTERNATIVE_2(						\
@@ -32,7 +34,7 @@  asm volatile(ALTERNATIVE_2(						\
 		THEAD_VENDOR_ID, ERRATA_THEAD_PMU,			\
 		CONFIG_ERRATA_THEAD_PMU,				\
 	"csrr %0, " __stringify(ANDES_CSR_SCOUNTEROF),			\
-		0, RISCV_ISA_EXT_XANDESPMU,				\
+		ANDES_VENDOR_ID, RISCV_ISA_VENDOR_EXT_XANDESPMU,	\
 		CONFIG_ANDES_CUSTOM_PMU)				\
 	: "=r" (__ovl) :						\
 	: "memory")
@@ -41,7 +43,7 @@  asm volatile(ALTERNATIVE_2(						\
 asm volatile(ALTERNATIVE(						\
 	"csrc " __stringify(CSR_IP) ", %0\n\t",				\
 	"csrc " __stringify(ANDES_CSR_SLIP) ", %0\n\t",			\
-		0, RISCV_ISA_EXT_XANDESPMU,				\
+		ANDES_VENDOR_ID, RISCV_ISA_VENDOR_EXT_XANDESPMU,	\
 		CONFIG_ANDES_CUSTOM_PMU)				\
 	: : "r"(__irq_mask)						\
 	: "memory")
@@ -837,7 +839,7 @@  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 (riscv_isa_extension_available(NULL, XANDESPMU) &&
+	} else if (riscv_isa_vendor_extension_available(-1, XANDESPMU) &&
 		   IS_ENABLED(CONFIG_ANDES_CUSTOM_PMU)) {
 		riscv_pmu_irq_num = ANDES_SLI_CAUSE_BASE + ANDES_RV_IRQ_PMOVI;
 		riscv_pmu_use_irq = true;