lkml.org 
[lkml]   [2020]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V3] arm64/cpuinfo: Define HWCAP name arrays per their actual bit definitions
On Mon, Aug 17, 2020 at 05:34:23PM +0530, Anshuman Khandual wrote:
> HWCAP name arrays (hwcap_str, compat_hwcap_str, compat_hwcap2_str) that are
> scanned for /proc/cpuinfo are detached from their bit definitions making it
> vulnerable and difficult to correlate. It is also bit problematic because
> during /proc/cpuinfo dump these arrays get traversed sequentially assuming
> they reflect and match actual HWCAP bit sequence, to test various features
> for a given CPU. This redefines name arrays per their HWCAP bit definitions
> . It also warns after detecting any feature which is not expected on arm64.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Dave Martin <Dave.Martin@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> This applies on 5.9-rc1
>
> Mark, since the patch has changed I have dropped your Acked-by: tag. Are you
> happy to give a new one ?
>
> Changes in V3:
>
> - Moved name arrays to (arch/arm64/kernel/cpuinfo.c) to prevent a build warning
> - Replaced string values with NULL for all compat features not possible on arm64
> - Changed compat_hwcap_str[] iteration on size as some NULL values are expected
> - Warn once after detecting any feature on arm64 that is not expected
>
> Changes in V2: (https://patchwork.kernel.org/patch/11533755/)
>
> - Defined COMPAT_KERNEL_HWCAP[2] and updated the name arrays per Mark
> - Updated the commit message as required
>
> Changes in V1: (https://patchwork.kernel.org/patch/11532945/)
>
> arch/arm64/include/asm/hwcap.h | 9 +++
> arch/arm64/kernel/cpuinfo.c | 172 ++++++++++++++++++++++-------------------
> 2 files changed, 100 insertions(+), 81 deletions(-)

[...]

> + [KERNEL_HWCAP_FP] = "fp",
> + [KERNEL_HWCAP_ASIMD] = "asimd",
> + [KERNEL_HWCAP_EVTSTRM] = "evtstrm",
> + [KERNEL_HWCAP_AES] = "aes",

It would be nice if the cap and the string were generated by the same
macro, along the lines of:

#define KERNEL_HWCAP(c) [KERNEL_HWCAP_##c] = #c,

Does making the constants mixed case break anything, or is it just really
churny to do?

> @@ -166,9 +167,18 @@ static int c_show(struct seq_file *m, void *v)
> seq_puts(m, "Features\t:");
> if (compat) {
> #ifdef CONFIG_COMPAT
> - for (j = 0; compat_hwcap_str[j]; j++)
> - if (compat_elf_hwcap & (1 << j))
> + for (j = 0; j < ARRAY_SIZE(compat_hwcap_str); j++) {
> + if (compat_elf_hwcap & (1 << j)) {
> + /*
> + * Warn once if any feature should not
> + * have been present on arm64 platform.
> + */
> + if (WARN_ON_ONCE(!compat_hwcap_str[j]))
> + continue;
> +
> seq_printf(m, " %s", compat_hwcap_str[j]);
> + }
> + }
>
> for (j = 0; compat_hwcap2_str[j]; j++)

Hmm, I find this pretty confusing now as compat_hwcap_str is not NULL
terminated and must be traversed with a loop bounded by ARRAY_SIZE(...),
whereas compat_hwcap2_str *is* NULL terminated and is traversed until you
hit the sentinel.

I think hwcap_str, compat_hwcap_str and compat_hwcap2_str should be
identical in this regard.

Will

\
 
 \ /
  Last update: 2020-09-07 19:50    [W:0.056 / U:19.764 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site