Re: [PATCH 1/3] s390/cpufeature: rework to allow more than only hwcap bits

Claudio Imbrenda
2022-07-12T16:50:30Z

On Tue, 12 Jul 2022 12:52:18 +0200
Steffen Eiden <seiden@linux.ibm.com> wrote:

> Rework cpufeature implementation to allow for various cpu feature
> indications, which is not only limited to hwcap bits. This is achieved
> by adding a sequential list of cpu feature numbers, where each of them
> is mapped to an entry which indicates what this number is about.
>
> Each entry contains a type member, which indicates what feature
> name space to look into (e.g. hwcap, or cpu facility). If wanted this
> allows also to automatically load modules only in e.g. z/VM
> configurations.
>
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

[...]

> +
> +/*
> + * cpu_have_feature - Test CPU features on module initialization
> + */
> +int cpu_have_feature(unsigned int num)
> +{
> + struct s390_cpu_feature *feature;
> +
> + feature = &s390_cpu_features[num];

I would put some check to make sure you are going past the end of the
array.

Maybe something like

if (num >= MAX_CPU_FEATURES) {
WARN(1, "Invalid feature %d", num);
return 0;
}

> + switch (feature->type) {
> + case TYPE_HWCAP:
> + return !!(elf_hwcap & (1UL << feature->num));
> + default:
> + WARN_ON_ONCE(1);
> + return 0;
> + }
> +}
> +EXPORT_SYMBOL(cpu_have_feature);
> diff --git a/arch/s390/kernel/processor.c b/arch/s390/kernel/processor.c
> index aa0e0e7fc773..a194611ba88c 100644
> --- a/arch/s390/kernel/processor.c
> +++ b/arch/s390/kernel/processor.c
> @@ -8,7 +8,6 @@
> #define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
>
> #include <linux/stop_machine.h>
> -#include <linux/cpufeature.h>
> #include <linux/bitops.h>
> #include <linux/kernel.h>
> #include <linux/random.h>
> @@ -96,15 +95,6 @@ void cpu_init(void)
> enter_lazy_tlb(&init_mm, current);
> }
>
> -/*
> - * cpu_have_feature - Test CPU features on module initialization
> - */
> -int cpu_have_feature(unsigned int num)
> -{
> - return elf_hwcap & (1UL << num);
> -}
> -EXPORT_SYMBOL(cpu_have_feature);
> -
> static void show_facilities(struct seq_file *m)
> {
> unsigned int bit;
> diff --git a/drivers/char/hw_random/s390-trng.c b/drivers/char/hw_random/s390-trng.c
> index 2beaa35c0d74..12fbac0ed8ca 100644
> --- a/drivers/char/hw_random/s390-trng.c
> +++ b/drivers/char/hw_random/s390-trng.c
> @@ -261,5 +261,5 @@ static void __exit trng_exit(void)
> trng_debug_exit();
> }
>
> -module_cpu_feature_match(MSA, trng_init);
> +module_cpu_feature_match(S390_CPU_FEATURE_MSA, trng_init);
> module_exit(trng_exit);
> diff --git a/drivers/s390/crypto/pkey_api.c b/drivers/s390/crypto/pkey_api.c
> index 7329caa7d467..5a05d1cdfec2 100644
> --- a/drivers/s390/crypto/pkey_api.c
> +++ b/drivers/s390/crypto/pkey_api.c
> @@ -2115,5 +2115,5 @@ static void __exit pkey_exit(void)
> pkey_debug_exit();
> }
>
> -module_cpu_feature_match(MSA, pkey_init);
> +module_cpu_feature_match(S390_CPU_FEATURE_MSA, pkey_init);
> module_exit(pkey_exit);