Re: [PATCH v6 09/18] arm64: enable ptrauth earlier

From: Vincenzo Frascino <vincenzo.frascino@arm.com>
To: Amit Kachhap <amit.kachhap@arm.com>,
	linux-arm-kernel@lists.infradead.org
Cc: Mark Rutland <mark.rutland@arm.com>,
	Kees Cook <keescook@chromium.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Kristina Martsenko <kristina.martsenko@arm.com>,
	Dave Martin <Dave.Martin@arm.com>,
	Mark Brown <broonie@kernel.org>,
	James Morse <james.morse@arm.com>,
	Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
	Will Deacon <will@kernel.org>, Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH v6 09/18] arm64: enable ptrauth earlier
Date: Wed, 11 Mar 2020 10:26:52 +0000	[thread overview]
Message-ID: <92f04bd6-dced-a8c5-827d-0dfa5eabfb6a@arm.com> (raw)
In-Reply-To: <1ea33eee-186c-d073-8687-43302e00c807@arm.com>

Hi Amit,

On 3/11/20 6:26 AM, Amit Kachhap wrote:

[...]

>>
>> My expectation is that you should call early_park_cpu to do that if the
>> secondary does not support PTRAUTH similar to what you did in v2 of this series:
>>
>> ENTRY(__cpu_secondary_checkptrauth)
>> #ifdef CONFIG_ARM64_PTR_AUTH
>>         /* Check if the CPU supports ptrauth */
>>         mrs     x2, id_aa64isar1_el1
>>         ubfx    x2, x2, #ID_AA64ISAR1_APA_SHIFT, #8
>>         cbnz    x2, 1f
>> alternative_if ARM64_HAS_ADDRESS_AUTH
>>         mov     x3, 1
>> alternative_else
>>         mov     x3, 0
>> alternative_endif
>>         cbz     x3, 1f
>>         /* Park the mismatched secondary CPU */
>>         early_park_cpu CPU_STUCK_REASON_NO_PTRAUTH
>> #endif
>> 1:     ret
>> ENDPROC(__cpu_secondary_checkptrauth)
>>
>> and then check it during the secondary_startup, similar to what happens for
>> 52BIT_VA for example.
>>
>> In this way "update_early_cpu_boot_status" would update the
>> CPU_STUCK_REASON_NO_PTRAUTH flag.
> 
> This way it was implemented earlier. Catalin suggested the pointer auth
> variation in cpus is not critical enough and cpufeature framework can park it
> little later [1].
> 
> Agree that I should have removed all definitions of CPU_STUCK_REASON_NO_PTRAUTH
> and prevent unnecessary confusions.
> 
> [1] : https://www.spinics.net/lists/arm-kernel/msg780766.html

It was either or :) Sorry I did not see Catalin's comment, please go ahead and
remove the definition of CPU_STUCK_REASON_NO_PTRAUTH and the code that uses it
in this case.

Maybe you want to expand as well your commit message (which already seems
covering this case) to explain why it is possible to let the cpu framework deal
with this case. This should make things clear according to me.

Another question that still remains is: do we need to introduce early_park_cpu
as part of this series? Since it does not seem you are using it anywhere.

>>
>> [...]
>>

-- 
Regards,
Vincenzo

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