| Message ID | 20240411-dev-charlie-support_thead_vector_6_9-v1-8-4af9815ec746@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: Support vendor extensions and xtheadvector | expand |
On Thu, Apr 11, 2024 at 09:11:14PM -0700, Charlie Jenkins wrote: > Create vendor variants of the existing extension helpers. If the > existing functions were instead modified to support vendor extensions, a > branch based on the ext value being greater than > RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This additional > branch would have an unnecessary performance impact. > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> I've not looked at the "main" patch in the series that adds all of the probing and structures for representing this info yet beyond a cursory glance, but it feels like we're duplicating a bunch of infrastructure here before it is necessary. The IDs are all internal to Linux, so I'd rather we kept everything in the same structure until we have more than a handful of vendor extensions. With this patch (and the theadpmu stuff) we will have three vendor extensions which feels like a drop in the bucket compared to the standard ones. > --- > arch/riscv/include/asm/cpufeature.h | 54 +++++++++++++++++++++++++++++++++++++ > arch/riscv/kernel/cpufeature.c | 34 ++++++++++++++++++++--- > 2 files changed, 84 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > index db2ab037843a..8f19e3681b4f 100644 > --- a/arch/riscv/include/asm/cpufeature.h > +++ b/arch/riscv/include/asm/cpufeature.h > @@ -89,6 +89,10 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, unsigned i > #define riscv_isa_extension_available(isa_bitmap, ext) \ > __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext) > > +bool __riscv_isa_vendor_extension_available(const unsigned long *vendor_isa_bitmap, unsigned int bit); > +#define riscv_isa_vendor_extension_available(isa_bitmap, ext) \ > + __riscv_isa_vendor_extension_available(isa_bitmap, RISCV_ISA_VENDOR_EXT_##ext) > + > static __always_inline bool > __riscv_has_extension_likely_alternatives(const unsigned long ext) > { > @@ -117,6 +121,8 @@ __riscv_has_extension_unlikely_alternatives(const unsigned long ext) > return true; > } > > +/* Standard extension helpers */ > + > static __always_inline bool > riscv_has_extension_likely(const unsigned long ext) > { > @@ -163,4 +169,52 @@ static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsi > return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > } > > +/* Vendor extension helpers */ > + > +static __always_inline bool > +riscv_has_vendor_extension_likely(const unsigned long ext) > +{ > + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, > + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); > + > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) > + return __riscv_has_extension_likely_alternatives(ext); > + else > + return __riscv_isa_vendor_extension_available(NULL, ext); > +} > + > +static __always_inline bool > +riscv_has_vendor_extension_unlikely(const unsigned long ext) > +{ > + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, > + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); > + > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) > + return __riscv_has_extension_unlikely_alternatives(ext); > + else > + return __riscv_isa_vendor_extension_available(NULL, ext); > +} > + > +static __always_inline bool riscv_cpu_has_vendor_extension_likely(int cpu, const unsigned long ext) > +{ > + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, > + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); > + > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) > + return __riscv_has_extension_likely_alternatives(ext); > + else > + return __riscv_isa_vendor_extension_available(hart_isa_vendor[cpu].isa, ext); > +} > + > +static __always_inline bool riscv_cpu_has_vendor_extension_unlikely(int cpu, const unsigned long ext) > +{ > + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, > + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); > + > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) > + return __riscv_has_extension_unlikely_alternatives(ext); > + else > + return __riscv_isa_vendor_extension_available(hart_isa_vendor[cpu].isa, ext); > +} Same stuff about constant folding applies to these, I think these should just mirror the existing functions (if needed at all). Cheers, Conor.
On Fri, Apr 12, 2024 at 12:49:57PM +0100, Conor Dooley wrote: > On Thu, Apr 11, 2024 at 09:11:14PM -0700, Charlie Jenkins wrote: > > Create vendor variants of the existing extension helpers. If the > > existing functions were instead modified to support vendor extensions, a > > branch based on the ext value being greater than > > RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This additional > > branch would have an unnecessary performance impact. > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > I've not looked at the "main" patch in the series that adds all of the > probing and structures for representing this info yet beyond a cursory > glance, but it feels like we're duplicating a bunch of infrastructure > here before it is necessary. The IDs are all internal to Linux, so I'd > rather we kept everything in the same structure until we have more than > a handful of vendor extensions. With this patch (and the theadpmu stuff) > we will have three vendor extensions which feels like a drop in the > bucket compared to the standard ones. It is not duplicating infrastructure. If we merge this into the existing infrastructure, we would be littering if (ext > RISCV_ISA_VENDOR_EXT_BASE) in __riscv_isa_extension_available. This is particularily important exactly because we have so few vendor extensions currently so this check would be irrelevant in the vast majority of cases. It is also unecessary to push off the refactoring until we have some "sufficient" amount of vendor extensions to deem changing the infrastructure when I already have the patch available here. This does not introduce any extra overhead to existing functions and will be able to support vendors into the future. - Charlie > > > > --- > > arch/riscv/include/asm/cpufeature.h | 54 +++++++++++++++++++++++++++++++++++++ > > arch/riscv/kernel/cpufeature.c | 34 ++++++++++++++++++++--- > > 2 files changed, 84 insertions(+), 4 deletions(-) > > > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > > index db2ab037843a..8f19e3681b4f 100644 > > --- a/arch/riscv/include/asm/cpufeature.h > > +++ b/arch/riscv/include/asm/cpufeature.h > > @@ -89,6 +89,10 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, unsigned i > > #define riscv_isa_extension_available(isa_bitmap, ext) \ > > __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext) > > > > +bool __riscv_isa_vendor_extension_available(const unsigned long *vendor_isa_bitmap, unsigned int bit); > > +#define riscv_isa_vendor_extension_available(isa_bitmap, ext) \ > > + __riscv_isa_vendor_extension_available(isa_bitmap, RISCV_ISA_VENDOR_EXT_##ext) > > + > > static __always_inline bool > > __riscv_has_extension_likely_alternatives(const unsigned long ext) > > { > > @@ -117,6 +121,8 @@ __riscv_has_extension_unlikely_alternatives(const unsigned long ext) > > return true; > > } > > > > +/* Standard extension helpers */ > > + > > static __always_inline bool > > riscv_has_extension_likely(const unsigned long ext) > > { > > @@ -163,4 +169,52 @@ static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsi > > return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > > } > > > > +/* Vendor extension helpers */ > > + > > +static __always_inline bool > > +riscv_has_vendor_extension_likely(const unsigned long ext) > > +{ > > + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, > > + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); > > + > > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) > > + return __riscv_has_extension_likely_alternatives(ext); > > + else > > + return __riscv_isa_vendor_extension_available(NULL, ext); > > +} > > + > > +static __always_inline bool > > +riscv_has_vendor_extension_unlikely(const unsigned long ext) > > +{ > > + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, > > + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); > > + > > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) > > + return __riscv_has_extension_unlikely_alternatives(ext); > > + else > > + return __riscv_isa_vendor_extension_available(NULL, ext); > > +} > > + > > +static __always_inline bool riscv_cpu_has_vendor_extension_likely(int cpu, const unsigned long ext) > > +{ > > + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, > > + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); > > + > > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) > > + return __riscv_has_extension_likely_alternatives(ext); > > + else > > + return __riscv_isa_vendor_extension_available(hart_isa_vendor[cpu].isa, ext); > > +} > > + > > +static __always_inline bool riscv_cpu_has_vendor_extension_unlikely(int cpu, const unsigned long ext) > > +{ > > + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, > > + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); > > + > > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) > > + return __riscv_has_extension_unlikely_alternatives(ext); > > + else > > + return __riscv_isa_vendor_extension_available(hart_isa_vendor[cpu].isa, ext); > > +} > > Same stuff about constant folding applies to these, I think these should > just mirror the existing functions (if needed at all). > > Cheers, > Conor.
On Fri, Apr 12, 2024 at 10:43:02AM -0700, Charlie Jenkins wrote: > On Fri, Apr 12, 2024 at 12:49:57PM +0100, Conor Dooley wrote: > > On Thu, Apr 11, 2024 at 09:11:14PM -0700, Charlie Jenkins wrote: > > > Create vendor variants of the existing extension helpers. If the > > > existing functions were instead modified to support vendor extensions, a > > > branch based on the ext value being greater than > > > RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This additional > > > branch would have an unnecessary performance impact. > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > > I've not looked at the "main" patch in the series that adds all of the > > probing and structures for representing this info yet beyond a cursory > > glance, but it feels like we're duplicating a bunch of infrastructure > > here before it is necessary. The IDs are all internal to Linux, so I'd > > rather we kept everything in the same structure until we have more than > > a handful of vendor extensions. With this patch (and the theadpmu stuff) > > we will have three vendor extensions which feels like a drop in the > > bucket compared to the standard ones. > > It is not duplicating infrastructure. If we merge this into the existing > infrastructure, we would be littering if (ext > RISCV_ISA_VENDOR_EXT_BASE) > in __riscv_isa_extension_available. This is particularily important > exactly because we have so few vendor extensions currently so this check > would be irrelevant in the vast majority of cases. That's only because of your implementation. The existing vendor extension works fine without this littering. That's another thing actually, you forgot to convert over the user we already have :) > It is also unecessary to push off the refactoring until we have some > "sufficient" amount of vendor extensions to deem changing the > infrastructure when I already have the patch available here. This does > not introduce any extra overhead to existing functions and will be able > to support vendors into the future. Yeah, maybe that's true but this was my gut reaction before reading the other patch in detail (which I've still yet to do).
On Fri, Apr 12, 2024 at 09:40:03PM +0100, Conor Dooley wrote: > On Fri, Apr 12, 2024 at 10:43:02AM -0700, Charlie Jenkins wrote: > > On Fri, Apr 12, 2024 at 12:49:57PM +0100, Conor Dooley wrote: > > > On Thu, Apr 11, 2024 at 09:11:14PM -0700, Charlie Jenkins wrote: > > > > Create vendor variants of the existing extension helpers. If the > > > > existing functions were instead modified to support vendor extensions, a > > > > branch based on the ext value being greater than > > > > RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This additional > > > > branch would have an unnecessary performance impact. > > > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > > > > I've not looked at the "main" patch in the series that adds all of the > > > probing and structures for representing this info yet beyond a cursory > > > glance, but it feels like we're duplicating a bunch of infrastructure > > > here before it is necessary. The IDs are all internal to Linux, so I'd > > > rather we kept everything in the same structure until we have more than > > > a handful of vendor extensions. With this patch (and the theadpmu stuff) > > > we will have three vendor extensions which feels like a drop in the > > > bucket compared to the standard ones. > > > > It is not duplicating infrastructure. If we merge this into the existing > > infrastructure, we would be littering if (ext > RISCV_ISA_VENDOR_EXT_BASE) > > in __riscv_isa_extension_available. This is particularily important > > exactly because we have so few vendor extensions currently so this check > > would be irrelevant in the vast majority of cases. > > That's only because of your implementation. The existing vendor extension > works fine without this littering. That's another thing actually, you > forgot to convert over the user we already have :) Oh right, I will convert them over. The fundemental goal of this patch is to allow a way for vendors to support their own extensions without needing to populate riscv_isa_ext. This is to create separation between vendors so they do not impact each other. xlinuxenvcfg does not fit into this scheme however. This scheme assumes that a hart cannot have multiple vendors which that extension breaks. xlinuxenvcfg is really filling a hole in the standard isa that is applicible to all vendors and does not appear in the device tree so it is okay for that to live outside this scheme. > > > It is also unecessary to push off the refactoring until we have some > > "sufficient" amount of vendor extensions to deem changing the > > infrastructure when I already have the patch available here. This does > > not introduce any extra overhead to existing functions and will be able > > to support vendors into the future. > > Yeah, maybe that's true but this was my gut reaction before reading the > other patch in detail (which I've still yet to do). - Charlie
On Fri, Apr 12, 2024 at 02:03:48PM -0700, Charlie Jenkins wrote: > On Fri, Apr 12, 2024 at 09:40:03PM +0100, Conor Dooley wrote: > > On Fri, Apr 12, 2024 at 10:43:02AM -0700, Charlie Jenkins wrote: > > > On Fri, Apr 12, 2024 at 12:49:57PM +0100, Conor Dooley wrote: > > > > On Thu, Apr 11, 2024 at 09:11:14PM -0700, Charlie Jenkins wrote: > > > > > Create vendor variants of the existing extension helpers. If the > > > > > existing functions were instead modified to support vendor extensions, a > > > > > branch based on the ext value being greater than > > > > > RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This additional > > > > > branch would have an unnecessary performance impact. > > > > > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > > > > > > I've not looked at the "main" patch in the series that adds all of the > > > > probing and structures for representing this info yet beyond a cursory > > > > glance, but it feels like we're duplicating a bunch of infrastructure > > > > here before it is necessary. The IDs are all internal to Linux, so I'd > > > > rather we kept everything in the same structure until we have more than > > > > a handful of vendor extensions. With this patch (and the theadpmu stuff) > > > > we will have three vendor extensions which feels like a drop in the > > > > bucket compared to the standard ones. > > > > > > It is not duplicating infrastructure. If we merge this into the existing > > > infrastructure, we would be littering if (ext > RISCV_ISA_VENDOR_EXT_BASE) > > > in __riscv_isa_extension_available. This is particularily important > > > exactly because we have so few vendor extensions currently so this check > > > would be irrelevant in the vast majority of cases. > > > > That's only because of your implementation. The existing vendor extension > > works fine without this littering. That's another thing actually, you > > forgot to convert over the user we already have :) > > Oh right, I will convert them over. The fundemental goal of this patch > is to allow a way for vendors to support their own extensions without > needing to populate riscv_isa_ext. This is to create separation between > vendors so they do not impact each other. The one that needs converting is xandespmu. As I said on the other patch a minute I don't think isolating vendors for the internal representation is needed and can be left in hwprobe. I also don't think we can rely on a behaviour of "SiFive CPUs will always have SiFive's mvendorid" or that kinda thing, I've heard talk of the SoC vendor getting their mvendorid for custom CPU cores instead of the CPU vendor and it's possible for the SBI implementation to "adjust" the values also. > xlinuxenvcfg does not fit into this scheme however. This scheme assumes > that a hart cannot have multiple vendors which that extension breaks. > xlinuxenvcfg is really filling a hole in the standard isa that is > applicible to all vendors and does not appear in the device tree so it > is okay for that to live outside this scheme. Ye, xlinuxenvcfg is an internal psuedo-extension that should be treated more like a standard one than something vendor. > > > It is also unecessary to push off the refactoring until we have some > > > "sufficient" amount of vendor extensions to deem changing the > > > infrastructure when I already have the patch available here. This does > > > not introduce any extra overhead to existing functions and will be able > > > to support vendors into the future. > > > > Yeah, maybe that's true but this was my gut reaction before reading the > > other patch in detail (which I've still yet to do). > > - Charlie >
On Fri, Apr 12, 2024 at 10:34:10PM +0100, Conor Dooley wrote: > On Fri, Apr 12, 2024 at 02:03:48PM -0700, Charlie Jenkins wrote: > > On Fri, Apr 12, 2024 at 09:40:03PM +0100, Conor Dooley wrote: > > > On Fri, Apr 12, 2024 at 10:43:02AM -0700, Charlie Jenkins wrote: > > > > On Fri, Apr 12, 2024 at 12:49:57PM +0100, Conor Dooley wrote: > > > > > On Thu, Apr 11, 2024 at 09:11:14PM -0700, Charlie Jenkins wrote: > > > > > > Create vendor variants of the existing extension helpers. If the > > > > > > existing functions were instead modified to support vendor extensions, a > > > > > > branch based on the ext value being greater than > > > > > > RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This additional > > > > > > branch would have an unnecessary performance impact. > > > > > > > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > > > > > > > > I've not looked at the "main" patch in the series that adds all of the > > > > > probing and structures for representing this info yet beyond a cursory > > > > > glance, but it feels like we're duplicating a bunch of infrastructure > > > > > here before it is necessary. The IDs are all internal to Linux, so I'd > > > > > rather we kept everything in the same structure until we have more than > > > > > a handful of vendor extensions. With this patch (and the theadpmu stuff) > > > > > we will have three vendor extensions which feels like a drop in the > > > > > bucket compared to the standard ones. > > > > > > > > It is not duplicating infrastructure. If we merge this into the existing > > > > infrastructure, we would be littering if (ext > RISCV_ISA_VENDOR_EXT_BASE) > > > > in __riscv_isa_extension_available. This is particularily important > > > > exactly because we have so few vendor extensions currently so this check > > > > would be irrelevant in the vast majority of cases. > > > > > > That's only because of your implementation. The existing vendor extension > > > works fine without this littering. That's another thing actually, you > > > forgot to convert over the user we already have :) > > > > Oh right, I will convert them over. The fundemental goal of this patch > > is to allow a way for vendors to support their own extensions without > > needing to populate riscv_isa_ext. This is to create separation between > > vendors so they do not impact each other. > > The one that needs converting is xandespmu. As I said on the other patch > a minute I don't think isolating vendors for the internal representation > is needed and can be left in hwprobe. I also don't think we can rely on > a behaviour of "SiFive CPUs will always have SiFive's mvendorid" or that > kinda thing, I've heard talk of the SoC vendor getting their mvendorid > for custom CPU cores instead of the CPU vendor and it's possible for > the SBI implementation to "adjust" the values also. Okay that may be possible but that is up to the vendor when that happens. The vendor extensions are fundamentally different from the standard extensions and have even less guarantees of correctness which seems like it would invite more errata if multiple vendors implement the same vendor extensions. I can extract the code into a different file for each vendor so that is more clear. - Charlie > > > xlinuxenvcfg does not fit into this scheme however. This scheme assumes > > that a hart cannot have multiple vendors which that extension breaks. > > xlinuxenvcfg is really filling a hole in the standard isa that is > > applicible to all vendors and does not appear in the device tree so it > > is okay for that to live outside this scheme. > > Ye, xlinuxenvcfg is an internal psuedo-extension that should be treated > more like a standard one than something vendor. > > > > > It is also unecessary to push off the refactoring until we have some > > > > "sufficient" amount of vendor extensions to deem changing the > > > > infrastructure when I already have the patch available here. This does > > > > not introduce any extra overhead to existing functions and will be able > > > > to support vendors into the future. > > > > > > Yeah, maybe that's true but this was my gut reaction before reading the > > > other patch in detail (which I've still yet to do). > > > > - Charlie > >
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index db2ab037843a..8f19e3681b4f 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -89,6 +89,10 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, unsigned i #define riscv_isa_extension_available(isa_bitmap, ext) \ __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext) +bool __riscv_isa_vendor_extension_available(const unsigned long *vendor_isa_bitmap, unsigned int bit); +#define riscv_isa_vendor_extension_available(isa_bitmap, ext) \ + __riscv_isa_vendor_extension_available(isa_bitmap, RISCV_ISA_VENDOR_EXT_##ext) + static __always_inline bool __riscv_has_extension_likely_alternatives(const unsigned long ext) { @@ -117,6 +121,8 @@ __riscv_has_extension_unlikely_alternatives(const unsigned long ext) return true; } +/* Standard extension helpers */ + static __always_inline bool riscv_has_extension_likely(const unsigned long ext) { @@ -163,4 +169,52 @@ static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsi return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); } +/* Vendor extension helpers */ + +static __always_inline bool +riscv_has_vendor_extension_likely(const unsigned long ext) +{ + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_likely_alternatives(ext); + else + return __riscv_isa_vendor_extension_available(NULL, ext); +} + +static __always_inline bool +riscv_has_vendor_extension_unlikely(const unsigned long ext) +{ + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_unlikely_alternatives(ext); + else + return __riscv_isa_vendor_extension_available(NULL, ext); +} + +static __always_inline bool riscv_cpu_has_vendor_extension_likely(int cpu, const unsigned long ext) +{ + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_likely_alternatives(ext); + else + return __riscv_isa_vendor_extension_available(hart_isa_vendor[cpu].isa, ext); +} + +static __always_inline bool riscv_cpu_has_vendor_extension_unlikely(int cpu, const unsigned long ext) +{ + compiletime_assert(ext < RISCV_ISA_VENDOR_EXT_MAX, + "ext must be < RISCV_ISA_VENDOR_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_unlikely_alternatives(ext); + else + return __riscv_isa_vendor_extension_available(hart_isa_vendor[cpu].isa, ext); +} + #endif diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index f72fbdd0d7f5..41a4d2028428 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -78,6 +78,29 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, unsigned i } EXPORT_SYMBOL_GPL(__riscv_isa_extension_available); +/** + * __riscv_isa_vendor_extension_available() - Check whether given vendor + * extension is available or not + * + * @isa_bitmap: ISA bitmap to use + * @bit: bit position of the desired extension + * Return: true or false + * + * NOTE: If isa_bitmap is NULL then Host ISA bitmap will be used. + */ +bool __riscv_isa_vendor_extension_available(const unsigned long *isa_bitmap, unsigned int bit) +{ + const unsigned long *bmap = (isa_bitmap) ? isa_bitmap : riscv_isa_vendor; + + bit -= RISCV_ISA_VENDOR_EXT_BASE; + + if (bit < 0 || bit >= RISCV_ISA_VENDOR_EXT_MAX) + return false; + + return test_bit(bit, bmap) ? true : false; +} +EXPORT_SYMBOL_GPL(__riscv_isa_vendor_extension_available); + static bool riscv_isa_extension_check(int id) { switch (id) { @@ -930,14 +953,17 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin, id = PATCH_ID_CPUFEATURE_ID(alt->patch_id); - if (id >= RISCV_ISA_EXT_MAX) { + if (id >= RISCV_ISA_VENDOR_EXT_BASE) { + if (!__riscv_isa_vendor_extension_available(NULL, id)) + continue; + } else if (id < RISCV_ISA_EXT_MAX) { + if (!__riscv_isa_extension_available(NULL, id)) + continue; + } else { WARN(1, "This extension id:%d is not in ISA extension list", id); continue; } - if (!__riscv_isa_extension_available(NULL, id)) - continue; - value = PATCH_ID_CPUFEATURE_VALUE(alt->patch_id); if (!riscv_cpufeature_patch_check(id, value)) continue;
Create vendor variants of the existing extension helpers. If the existing functions were instead modified to support vendor extensions, a branch based on the ext value being greater than RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This additional branch would have an unnecessary performance impact. Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> --- arch/riscv/include/asm/cpufeature.h | 54 +++++++++++++++++++++++++++++++++++++ arch/riscv/kernel/cpufeature.c | 34 ++++++++++++++++++++--- 2 files changed, 84 insertions(+), 4 deletions(-)