Re: [PATCH V3] arm64/cpuinfo: Define HWCAP name arrays per their actual bit definitions

[Anshuman Khandual <anshuman.khandual@arm.com>]

On 09/08/2020 01:11 PM, Will Deacon wrote:
> On Tue, Sep 08, 2020 at 10:43:12AM +0530, Anshuman Khandual wrote:
>>
>>
>> On 09/07/2020 05:46 PM, Will Deacon wrote:
>>> 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?
>>
>> Currently all existing HWCAP feature strings are lower case, above change
>> will make them into upper case instead. I could not find a method to force
>> convert #c into lower case constant strings in the macro definition. Would
>> not changing the HWCAP string case here, break user interface ?
> 
> Yes, we can't change the user-visible strings, but what's wrong with
> having e.g. KERNEL_HWCAP_fp instead of KERNEL_HWCAP_FP?

Unlike the new compat macros i.e COMPAT_KERNEL_HWCAP[2] in this patch,
KERNEL_HWCAP_XXX macros are already defined and are getting used else
where (arch/arm64/kernel/cpufeature.c) as well. [KERNEL_HWCAP_##c] can
only be used here, if the input string is in upper case. Otherwise all
these existing macros need to be changed first, which will result in
too much code churn.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel