| Message ID | 1583476525-13505-8-git-send-email-amit.kachhap@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: return address signing | expand |
Hi Amit, On 3/6/20 6:35 AM, Amit Daniel Kachhap wrote: [...] > > -static inline bool > -cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap) > -{ > - return !!(cap->type & ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU); > -} > - > -static inline bool > -cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap) > -{ > - return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU); > -} > - > /* > * Generic helper for handling capabilties with multiple (match,enable) pairs > * of call backs, sharing the same capability bit. > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index b12e386..865dce6 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -1363,6 +1363,19 @@ static bool can_use_gic_priorities(const struct arm64_cpu_capabilities *entry, > } > #endif > > +/* Internal helper functions to match cpu capability type */ > +static bool > +cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap) > +{ > + return !!(cap->type & ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU); > +} > + > +static bool > +cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap) > +{ > + return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU); > +} > + > static const struct arm64_cpu_capabilities arm64_features[] = { > { > .desc = "GIC system register CPU interface", > Seems that the signature of the functions above is changed during the migration. In particular you dropped "inline". Is there any specific reason?
Hi Vincenzo, On 3/10/20 5:50 PM, Vincenzo Frascino wrote: > Hi Amit, > > On 3/6/20 6:35 AM, Amit Daniel Kachhap wrote: > > [...] > >> >> -static inline bool >> -cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap) >> -{ >> - return !!(cap->type & ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU); >> -} >> - >> -static inline bool >> -cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap) >> -{ >> - return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU); >> -} >> - >> /* >> * Generic helper for handling capabilties with multiple (match,enable) pairs >> * of call backs, sharing the same capability bit. >> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c >> index b12e386..865dce6 100644 >> --- a/arch/arm64/kernel/cpufeature.c >> +++ b/arch/arm64/kernel/cpufeature.c >> @@ -1363,6 +1363,19 @@ static bool can_use_gic_priorities(const struct arm64_cpu_capabilities *entry, >> } >> #endif >> >> +/* Internal helper functions to match cpu capability type */ >> +static bool >> +cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap) >> +{ >> + return !!(cap->type & ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU); >> +} >> + >> +static bool >> +cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap) >> +{ >> + return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU); >> +} >> + >> static const struct arm64_cpu_capabilities arm64_features[] = { >> { >> .desc = "GIC system register CPU interface", >> > > Seems that the signature of the functions above is changed during the migration. > In particular you dropped "inline". Is there any specific reason? Earlier Catalin pointed me here [1]. I guess its not a hot-path function. [1]: https://www.spinics.net/lists/arm-kernel/msg789696.html Cheers, Amit >
On Tue, Mar 10, 2020 at 06:23:15PM +0530, Amit Kachhap wrote: > On 3/10/20 5:50 PM, Vincenzo Frascino wrote: > > On 3/6/20 6:35 AM, Amit Daniel Kachhap wrote: > > > > [...] > > > > > -static inline bool > > > -cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap) > > > -{ > > > - return !!(cap->type & ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU); > > > -} > > > - > > > -static inline bool > > > -cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap) > > > -{ > > > - return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU); > > > -} > > > - > > > /* > > > * Generic helper for handling capabilties with multiple (match,enable) pairs > > > * of call backs, sharing the same capability bit. > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > > index b12e386..865dce6 100644 > > > --- a/arch/arm64/kernel/cpufeature.c > > > +++ b/arch/arm64/kernel/cpufeature.c > > > @@ -1363,6 +1363,19 @@ static bool can_use_gic_priorities(const struct arm64_cpu_capabilities *entry, > > > } > > > #endif > > > +/* Internal helper functions to match cpu capability type */ > > > +static bool > > > +cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap) > > > +{ > > > + return !!(cap->type & ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU); > > > +} > > > + > > > +static bool > > > +cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap) > > > +{ > > > + return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU); > > > +} > > > + > > > static const struct arm64_cpu_capabilities arm64_features[] = { > > > { > > > .desc = "GIC system register CPU interface", > > > > > > > Seems that the signature of the functions above is changed during the migration. > > In particular you dropped "inline". Is there any specific reason? > > Earlier Catalin pointed me here [1]. I guess its not a hot-path function. > > [1]: https://www.spinics.net/lists/arm-kernel/msg789696.html Indeed, it had to be static inline in a header but not anymore in a .c file. Also if it's really essential to be inlined and the compiler doesn't do this automatically, use __always_inline. But my preference is not to bother unless you back it up by numbers.
On 3/11/20 10:50 AM, Catalin Marinas wrote: > On Tue, Mar 10, 2020 at 06:23:15PM +0530, Amit Kachhap wrote: >> On 3/10/20 5:50 PM, Vincenzo Frascino wrote: >>> On 3/6/20 6:35 AM, Amit Daniel Kachhap wrote: >>> >>> [...] >>> >>> Seems that the signature of the functions above is changed during the migration. >>> In particular you dropped "inline". Is there any specific reason? >> >> Earlier Catalin pointed me here [1]. I guess its not a hot-path function. >> >> [1]: https://www.spinics.net/lists/arm-kernel/msg789696.html > > Indeed, it had to be static inline in a header but not anymore in a .c > file. Also if it's really essential to be inlined and the compiler > doesn't do this automatically, use __always_inline. But my preference is > not to bother unless you back it up by numbers. > Ok, fine by me. Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 0fd1feb..ae9673a 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -340,18 +340,6 @@ static inline int cpucap_default_scope(const struct arm64_cpu_capabilities *cap) return cap->type & ARM64_CPUCAP_SCOPE_MASK; } -static inline bool -cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap) -{ - return !!(cap->type & ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU); -} - -static inline bool -cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap) -{ - return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU); -} - /* * Generic helper for handling capabilties with multiple (match,enable) pairs * of call backs, sharing the same capability bit. diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index b12e386..865dce6 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -1363,6 +1363,19 @@ static bool can_use_gic_priorities(const struct arm64_cpu_capabilities *entry, } #endif +/* Internal helper functions to match cpu capability type */ +static bool +cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap) +{ + return !!(cap->type & ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU); +} + +static bool +cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap) +{ + return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU); +} + static const struct arm64_cpu_capabilities arm64_features[] = { { .desc = "GIC system register CPU interface",
These helpers are used only by functions inside cpufeature.c and hence makes sense to be moved from cpufeature.h to cpufeature.c as they are not expected to be used globaly. This change helps in reducing the header file size as well as to add future cpu capability types without confusion. Only a cpu capability type macro is sufficient to expose those capabilities globally. Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com> --- Changes since v5: * New patch. arch/arm64/include/asm/cpufeature.h | 12 ------------ arch/arm64/kernel/cpufeature.c | 13 +++++++++++++ 2 files changed, 13 insertions(+), 12 deletions(-)