diff mbox series

[7/8] arm64: cpufeature: Relax checks for AArch32 support at EL[0-2]

Message ID 20200414213114.2378-8-will@kernel.org (mailing list archive)
State New, archived
Headers show
Series Relax sanity checking for mismatched AArch32 EL1 | expand

Commit Message

Will Deacon April 14, 2020, 9:31 p.m. UTC
We don't need to be quite as strict about mismatched AArch32 support,
which is good because the friendly hardware folks have been busy
mismatching this to their hearts' content.

  * We don't care about EL2 or EL3 (there are silly comments concerning
    the latter, so remove those)

  * EL1 support is gated by the ARM64_HAS_32BIT_EL1 capability and handled
    gracefully when a mismatch occurs

  * EL1 support is gated by the ARM64_HAS_32BIT_EL0 capability and handled
    gracefully when a mismatch occurs

Relax the AArch32 checks to FTR_NONSTRICT.

Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/cpufeature.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

Suzuki K Poulose April 15, 2020, 10:50 a.m. UTC | #1
On 04/14/2020 10:31 PM, Will Deacon wrote:
> We don't need to be quite as strict about mismatched AArch32 support,
> which is good because the friendly hardware folks have been busy
> mismatching this to their hearts' content.
> 
>    * We don't care about EL2 or EL3 (there are silly comments concerning
>      the latter, so remove those)
> 
>    * EL1 support is gated by the ARM64_HAS_32BIT_EL1 capability and handled
>      gracefully when a mismatch occurs
> 
>    * EL1 support is gated by the ARM64_HAS_32BIT_EL0 capability and handled

s/EL1/EL0

>      gracefully when a mismatch occurs
> 
> Relax the AArch32 checks to FTR_NONSTRICT.

Agreed. We should do something similar for the features exposed by the
ELF_HWCAP, of course in a separate series.

> 
> Signed-off-by: Will Deacon <will@kernel.org>

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Will Deacon April 15, 2020, 10:58 a.m. UTC | #2
On Wed, Apr 15, 2020 at 11:50:58AM +0100, Suzuki K Poulose wrote:
> On 04/14/2020 10:31 PM, Will Deacon wrote:
> > We don't need to be quite as strict about mismatched AArch32 support,
> > which is good because the friendly hardware folks have been busy
> > mismatching this to their hearts' content.
> > 
> >    * We don't care about EL2 or EL3 (there are silly comments concerning
> >      the latter, so remove those)
> > 
> >    * EL1 support is gated by the ARM64_HAS_32BIT_EL1 capability and handled
> >      gracefully when a mismatch occurs
> > 
> >    * EL1 support is gated by the ARM64_HAS_32BIT_EL0 capability and handled
> 
> s/EL1/EL0
> 
> >      gracefully when a mismatch occurs
> > 
> > Relax the AArch32 checks to FTR_NONSTRICT.
> 
> Agreed. We should do something similar for the features exposed by the
> ELF_HWCAP, of course in a separate series.

Hmm, I didn't think we needed to touch the HWCAPs, as they're derived from
the sanitised feature register values. What am I missing?

Will
Suzuki K Poulose April 15, 2020, 11:37 a.m. UTC | #3
On 04/15/2020 11:58 AM, Will Deacon wrote:
> On Wed, Apr 15, 2020 at 11:50:58AM +0100, Suzuki K Poulose wrote:
>> On 04/14/2020 10:31 PM, Will Deacon wrote:
>>> We don't need to be quite as strict about mismatched AArch32 support,
>>> which is good because the friendly hardware folks have been busy
>>> mismatching this to their hearts' content.
>>>
>>>     * We don't care about EL2 or EL3 (there are silly comments concerning
>>>       the latter, so remove those)
>>>
>>>     * EL1 support is gated by the ARM64_HAS_32BIT_EL1 capability and handled
>>>       gracefully when a mismatch occurs
>>>
>>>     * EL1 support is gated by the ARM64_HAS_32BIT_EL0 capability and handled
>>
>> s/EL1/EL0
>>
>>>       gracefully when a mismatch occurs
>>>
>>> Relax the AArch32 checks to FTR_NONSTRICT.
>>
>> Agreed. We should do something similar for the features exposed by the
>> ELF_HWCAP, of course in a separate series.
> 
> Hmm, I didn't think we needed to touch the HWCAPs, as they're derived from
> the sanitised feature register values. What am I missing?

sorry, that was cryptic. I was suggesting to relax the ftr fields to
NONSTRICT for the fields covered by ELF HWCAPs (and other CPU hwcaps).

Suzuki
Will Deacon April 15, 2020, 12:29 p.m. UTC | #4
On Wed, Apr 15, 2020 at 12:37:31PM +0100, Suzuki K Poulose wrote:
> On 04/15/2020 11:58 AM, Will Deacon wrote:
> > On Wed, Apr 15, 2020 at 11:50:58AM +0100, Suzuki K Poulose wrote:
> > > On 04/14/2020 10:31 PM, Will Deacon wrote:
> > > > We don't need to be quite as strict about mismatched AArch32 support,
> > > > which is good because the friendly hardware folks have been busy
> > > > mismatching this to their hearts' content.
> > > > 
> > > >     * We don't care about EL2 or EL3 (there are silly comments concerning
> > > >       the latter, so remove those)
> > > > 
> > > >     * EL1 support is gated by the ARM64_HAS_32BIT_EL1 capability and handled
> > > >       gracefully when a mismatch occurs
> > > > 
> > > >     * EL1 support is gated by the ARM64_HAS_32BIT_EL0 capability and handled
> > > 
> > > s/EL1/EL0
> > > 
> > > >       gracefully when a mismatch occurs
> > > > 
> > > > Relax the AArch32 checks to FTR_NONSTRICT.
> > > 
> > > Agreed. We should do something similar for the features exposed by the
> > > ELF_HWCAP, of course in a separate series.
> > 
> > Hmm, I didn't think we needed to touch the HWCAPs, as they're derived from
> > the sanitised feature register values. What am I missing?
> 
> sorry, that was cryptic. I was suggesting to relax the ftr fields to
> NONSTRICT for the fields covered by ELF HWCAPs (and other CPU hwcaps).

Ah, gotcha. Given that the HWCAPs usually describe EL0 features, I say we
can punt this down the road until people give us hardware with mismatched
AArch32 at EL0.

Will
Suzuki K Poulose April 17, 2020, 9:37 a.m. UTC | #5
On 04/15/2020 01:29 PM, Will Deacon wrote:
> On Wed, Apr 15, 2020 at 12:37:31PM +0100, Suzuki K Poulose wrote:
>> On 04/15/2020 11:58 AM, Will Deacon wrote:
>>> On Wed, Apr 15, 2020 at 11:50:58AM +0100, Suzuki K Poulose wrote:
>>>> On 04/14/2020 10:31 PM, Will Deacon wrote:
>>>>> We don't need to be quite as strict about mismatched AArch32 support,
>>>>> which is good because the friendly hardware folks have been busy
>>>>> mismatching this to their hearts' content.
>>>>>
>>>>>      * We don't care about EL2 or EL3 (there are silly comments concerning
>>>>>        the latter, so remove those)
>>>>>
>>>>>      * EL1 support is gated by the ARM64_HAS_32BIT_EL1 capability and handled
>>>>>        gracefully when a mismatch occurs
>>>>>
>>>>>      * EL1 support is gated by the ARM64_HAS_32BIT_EL0 capability and handled
>>>>
>>>> s/EL1/EL0
>>>>
>>>>>        gracefully when a mismatch occurs
>>>>>
>>>>> Relax the AArch32 checks to FTR_NONSTRICT.
>>>>
>>>> Agreed. We should do something similar for the features exposed by the
>>>> ELF_HWCAP, of course in a separate series.
>>>
>>> Hmm, I didn't think we needed to touch the HWCAPs, as they're derived from
>>> the sanitised feature register values. What am I missing?
>>
>> sorry, that was cryptic. I was suggesting to relax the ftr fields to
>> NONSTRICT for the fields covered by ELF HWCAPs (and other CPU hwcaps).
> 
> Ah, gotcha. Given that the HWCAPs usually describe EL0 features, I say we
> can punt this down the road until people give us hardware with mismatched
> AArch32 at EL0.

Btw, this is not just mismatched AArch32, but mismatched AArch64 HWCAPs
too, which I believe exists. Anyways as you said, we can delay this
until we get the reports :-)

Suzuki
diff mbox series

Patch

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9e0321e3e581..680a453ca8c4 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -172,11 +172,10 @@  static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_GIC_SHIFT, 4, 0),
 	S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_ASIMD_SHIFT, 4, ID_AA64PFR0_ASIMD_NI),
 	S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_FP_SHIFT, 4, ID_AA64PFR0_FP_NI),
-	/* Linux doesn't care about the EL3 */
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL3_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL2_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_SHIFT, 4, ID_AA64PFR0_EL1_64BIT_ONLY),
-	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL0_SHIFT, 4, ID_AA64PFR0_EL0_64BIT_ONLY),
+	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL2_SHIFT, 4, 0),
+	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_SHIFT, 4, ID_AA64PFR0_EL1_64BIT_ONLY),
+	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL0_SHIFT, 4, ID_AA64PFR0_EL0_64BIT_ONLY),
 	ARM64_FTR_END,
 };
 
@@ -867,9 +866,6 @@  void update_cpu_features(int cpu,
 	taint |= check_update_ftr_reg(SYS_ID_AA64MMFR2_EL1, cpu,
 				      info->reg_id_aa64mmfr2, boot->reg_id_aa64mmfr2);
 
-	/*
-	 * EL3 is not our concern.
-	 */
 	taint |= check_update_ftr_reg(SYS_ID_AA64PFR0_EL1, cpu,
 				      info->reg_id_aa64pfr0, boot->reg_id_aa64pfr0);
 	taint |= check_update_ftr_reg(SYS_ID_AA64PFR1_EL1, cpu,