| Message ID | 20240426-dev-charlie-support_thead_vector_6_9-v4-7-b692f3c516ec@rivosinc.com |
|---|---|
| State | Accepted |
| Commit | d4c8d79f5199055da38f880f782a3e62c599ff5d |
| Headers | show |
| Series | [v4,01/16] dt-bindings: riscv: Add xtheadvector ISA extension description | expand |
On Fri, Apr 26, 2024 at 02:29:21PM -0700, Charlie Jenkins wrote: > The __riscv_has_extension_likely() and __riscv_has_extension_unlikely() > functions from the vendor_extensions.h can be used to simplify the > standard extension checking code as well. Migrate those functions to > cpufeature.h and reorganize the code in the file to use the functions. > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > --- > arch/riscv/include/asm/cpufeature.h | 78 +++++++++++++++++------------- > arch/riscv/include/asm/vendor_extensions.h | 28 ----------- > 2 files changed, 44 insertions(+), 62 deletions(-) > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > index fedd479ccfd1..17896ec9ec11 100644 > --- a/arch/riscv/include/asm/cpufeature.h > +++ b/arch/riscv/include/asm/cpufeature.h > @@ -98,59 +98,66 @@ extern bool riscv_isa_fallback; > > unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap); > > +#define EXT_ALL_VENDORS 0 It's not really "all vendors", it's standard. Otherwise, this seems all grand to me, Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Cheers, Conor.
On Wed, May 01, 2024 at 12:37:14PM +0100, Conor Dooley wrote: > On Fri, Apr 26, 2024 at 02:29:21PM -0700, Charlie Jenkins wrote: > > The __riscv_has_extension_likely() and __riscv_has_extension_unlikely() > > functions from the vendor_extensions.h can be used to simplify the > > standard extension checking code as well. Migrate those functions to > > cpufeature.h and reorganize the code in the file to use the functions. > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > --- > > arch/riscv/include/asm/cpufeature.h | 78 +++++++++++++++++------------- > > arch/riscv/include/asm/vendor_extensions.h | 28 ----------- > > 2 files changed, 44 insertions(+), 62 deletions(-) > > > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > > index fedd479ccfd1..17896ec9ec11 100644 > > --- a/arch/riscv/include/asm/cpufeature.h > > +++ b/arch/riscv/include/asm/cpufeature.h > > @@ -98,59 +98,66 @@ extern bool riscv_isa_fallback; > > > > unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap); > > > > +#define EXT_ALL_VENDORS 0 > > It's not really "all vendors", it's standard. Otherwise, this seems all This hooks up into the alternatives: ALTERNATIVE("nop", "j %l[l_yes]", %[vendor], %[ext], 1) Where the "vendor" argument is supposed to be 0 if the alternative is applicable to all vendors. Is there a better way to convey this? - Charlie > grand to me, > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > Cheers, > Conor.
On Wed, May 01, 2024 at 12:48:13PM -0700, Charlie Jenkins wrote: > On Wed, May 01, 2024 at 12:37:14PM +0100, Conor Dooley wrote: > > On Fri, Apr 26, 2024 at 02:29:21PM -0700, Charlie Jenkins wrote: > > > The __riscv_has_extension_likely() and __riscv_has_extension_unlikely() > > > functions from the vendor_extensions.h can be used to simplify the > > > standard extension checking code as well. Migrate those functions to > > > cpufeature.h and reorganize the code in the file to use the functions. > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > --- > > > arch/riscv/include/asm/cpufeature.h | 78 +++++++++++++++++------------- > > > arch/riscv/include/asm/vendor_extensions.h | 28 ----------- > > > 2 files changed, 44 insertions(+), 62 deletions(-) > > > > > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > > > index fedd479ccfd1..17896ec9ec11 100644 > > > --- a/arch/riscv/include/asm/cpufeature.h > > > +++ b/arch/riscv/include/asm/cpufeature.h > > > @@ -98,59 +98,66 @@ extern bool riscv_isa_fallback; > > > > > > unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap); > > > > > > +#define EXT_ALL_VENDORS 0 > > > > It's not really "all vendors", it's standard. Otherwise, this seems all > > This hooks up into the alternatives: > > ALTERNATIVE("nop", "j %l[l_yes]", %[vendor], %[ext], 1) Yeah, I know what you're using it for, I just find the naming odd. > > Where the "vendor" argument is supposed to be 0 if the alternative is > applicable to all vendors. Is there a better way to convey this? s/EXT_ALL_VENDORS/STANDARD_EXT/?
On Wed, May 01, 2024 at 09:15:44PM +0100, Conor Dooley wrote: > On Wed, May 01, 2024 at 12:48:13PM -0700, Charlie Jenkins wrote: > > On Wed, May 01, 2024 at 12:37:14PM +0100, Conor Dooley wrote: > > > On Fri, Apr 26, 2024 at 02:29:21PM -0700, Charlie Jenkins wrote: > > > > The __riscv_has_extension_likely() and __riscv_has_extension_unlikely() > > > > functions from the vendor_extensions.h can be used to simplify the > > > > standard extension checking code as well. Migrate those functions to > > > > cpufeature.h and reorganize the code in the file to use the functions. > > > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > > --- > > > > arch/riscv/include/asm/cpufeature.h | 78 +++++++++++++++++------------- > > > > arch/riscv/include/asm/vendor_extensions.h | 28 ----------- > > > > 2 files changed, 44 insertions(+), 62 deletions(-) > > > > > > > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > > > > index fedd479ccfd1..17896ec9ec11 100644 > > > > --- a/arch/riscv/include/asm/cpufeature.h > > > > +++ b/arch/riscv/include/asm/cpufeature.h > > > > @@ -98,59 +98,66 @@ extern bool riscv_isa_fallback; > > > > > > > > unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap); > > > > > > > > +#define EXT_ALL_VENDORS 0 > > > > > > It's not really "all vendors", it's standard. Otherwise, this seems all > > > > This hooks up into the alternatives: > > > > ALTERNATIVE("nop", "j %l[l_yes]", %[vendor], %[ext], 1) > > Yeah, I know what you're using it for, I just find the naming odd. > > > > Where the "vendor" argument is supposed to be 0 if the alternative is > > applicable to all vendors. Is there a better way to convey this? > > s/EXT_ALL_VENDORS/STANDARD_EXT/? Sure :) - Charlie
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index fedd479ccfd1..17896ec9ec11 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -98,59 +98,66 @@ extern bool riscv_isa_fallback; unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap); +#define EXT_ALL_VENDORS 0 + bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, unsigned int bit); #define riscv_isa_extension_available(isa_bitmap, ext) \ __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext) -static __always_inline bool -riscv_has_extension_likely(const unsigned long ext) +static __always_inline bool __riscv_has_extension_likely(const unsigned long vendor, + const unsigned long ext) { - compiletime_assert(ext < RISCV_ISA_EXT_MAX, - "ext must be < RISCV_ISA_EXT_MAX"); - - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { - asm goto( - ALTERNATIVE("j %l[l_no]", "nop", 0, %[ext], 1) - : - : [ext] "i" (ext) - : - : l_no); - } else { - if (!__riscv_isa_extension_available(NULL, ext)) - goto l_no; - } + asm goto(ALTERNATIVE("j %l[l_no]", "nop", %[vendor], %[ext], 1) + : + : [vendor] "i" (vendor), [ext] "i" (ext) + : + : l_no); return true; l_no: return false; } -static __always_inline bool -riscv_has_extension_unlikely(const unsigned long ext) +static __always_inline bool __riscv_has_extension_unlikely(const unsigned long vendor, + const unsigned long ext) { - compiletime_assert(ext < RISCV_ISA_EXT_MAX, - "ext must be < RISCV_ISA_EXT_MAX"); - - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { - asm goto( - ALTERNATIVE("nop", "j %l[l_yes]", 0, %[ext], 1) - : - : [ext] "i" (ext) - : - : l_yes); - } else { - if (__riscv_isa_extension_available(NULL, ext)) - goto l_yes; - } + asm goto(ALTERNATIVE("nop", "j %l[l_yes]", %[vendor], %[ext], 1) + : + : [vendor] "i" (vendor), [ext] "i" (ext) + : + : l_yes); return false; l_yes: return true; } +static __always_inline bool riscv_has_extension_unlikely(const unsigned long ext) +{ + compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_unlikely(EXT_ALL_VENDORS, ext); + + return __riscv_isa_extension_available(NULL, ext); +} + +static __always_inline bool riscv_has_extension_likely(const unsigned long ext) +{ + compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_likely(EXT_ALL_VENDORS, ext); + + return __riscv_isa_extension_available(NULL, ext); +} + static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext) { - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext)) + compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && + __riscv_has_extension_likely(EXT_ALL_VENDORS, ext)) return true; return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); @@ -158,7 +165,10 @@ static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsign static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext) { - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext)) + compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && + __riscv_has_extension_unlikely(EXT_ALL_VENDORS, ext)) return true; return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); diff --git a/arch/riscv/include/asm/vendor_extensions.h b/arch/riscv/include/asm/vendor_extensions.h index 74b82433e0a2..880bb86ec890 100644 --- a/arch/riscv/include/asm/vendor_extensions.h +++ b/arch/riscv/include/asm/vendor_extensions.h @@ -39,34 +39,6 @@ bool __riscv_isa_vendor_extension_available(int cpu, unsigned long vendor, unsig __riscv_isa_vendor_extension_available(VENDOR_EXT_ALL_CPUS, vendor, \ RISCV_ISA_VENDOR_EXT_##ext) -static __always_inline bool __riscv_has_extension_likely(const unsigned long vendor, - const unsigned long ext) -{ - asm goto(ALTERNATIVE("j %l[l_no]", "nop", %[vendor], %[ext], 1) - : - : [vendor] "i" (vendor), [ext] "i" (ext) - : - : l_no); - - return true; -l_no: - return false; -} - -static __always_inline bool __riscv_has_extension_unlikely(const unsigned long vendor, - const unsigned long ext) -{ - asm goto(ALTERNATIVE("nop", "j %l[l_yes]", %[vendor], %[ext], 1) - : - : [vendor] "i" (vendor), [ext] "i" (ext) - : - : l_yes); - - return false; -l_yes: - return true; -} - static __always_inline bool riscv_has_vendor_extension_likely(const unsigned long vendor, const unsigned long ext) {
The __riscv_has_extension_likely() and __riscv_has_extension_unlikely() functions from the vendor_extensions.h can be used to simplify the standard extension checking code as well. Migrate those functions to cpufeature.h and reorganize the code in the file to use the functions. Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> --- arch/riscv/include/asm/cpufeature.h | 78 +++++++++++++++++------------- arch/riscv/include/asm/vendor_extensions.h | 28 ----------- 2 files changed, 44 insertions(+), 62 deletions(-)