[PATCH v2 2/2] arm64: cpufeature: Pretend that Apple A10 family does not support 32-bit EL0

Nick Chan posted 2 patches 2 months, 3 weeks ago
[PATCH v2 2/2] arm64: cpufeature: Pretend that Apple A10 family does not support 32-bit EL0
Posted by Nick Chan 2 months, 3 weeks ago
The Apple A10 family consists of physical performance and efficiency
cores, and only one of them can be active at a given time depending on
the current p-state. However, only the performance cores can execute
32-bit EL0. This results in logical cores that can only execute 32-bit
EL0 in high p-states.

Trying to support 32-bit EL0 on a CPU that can only execute it in certain
states is a bad idea. The A10 family only supports 16KB page size anyway
so many AArch32 executables won't run anyways. Pretend that it does not
support 32-bit EL0 at all.

Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 arch/arm64/kernel/cpufeature.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 718728a85430..386698f42172 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -3529,6 +3529,31 @@ void __init setup_boot_cpu_features(void)
 	setup_boot_cpu_capabilities();
 }
 
+static void __init bad_aarch32_el0_fixup(void)
+{
+	static const struct midr_range bad_aarch32_el0[] = {
+		MIDR_ALL_VERSIONS(MIDR_APPLE_A10_T2_HURRICANE_ZEPHYR),
+		MIDR_ALL_VERSIONS(MIDR_APPLE_A10X_HURRICANE_ZEPHYR),
+		{}
+	};
+
+	/*
+	 * The Apple A10 family can only execute 32-bit EL0 when in high
+	 * p-states. Pretend it does not support 32-bit EL0.
+	 */
+	if (is_midr_in_range_list(read_cpuid_id(), bad_aarch32_el0)) {
+		struct arm64_ftr_reg *regp;
+
+		regp = get_arm64_ftr_reg(SYS_ID_AA64PFR0_EL1);
+		if (!regp)
+			return;
+		u64 val = (regp->sys_val & ~ID_AA64PFR0_EL1_EL0_MASK)
+		  | ID_AA64PFR0_EL1_EL0_IMP;
+
+		update_cpu_ftr_reg(regp, val);
+	}
+}
+
 static void __init setup_system_capabilities(void)
 {
 	/*
@@ -3562,6 +3587,8 @@ static void __init setup_system_capabilities(void)
 
 void __init setup_system_features(void)
 {
+	bad_aarch32_el0_fixup();
+
 	setup_system_capabilities();
 
 	kpti_install_ng_mappings();
-- 
2.46.0
Re: [PATCH v2 2/2] arm64: cpufeature: Pretend that Apple A10 family does not support 32-bit EL0
Posted by Nick Chan 2 months, 2 weeks ago

On 9/9/2024 17:10, Nick Chan wrote:
> The Apple A10 family consists of physical performance and efficiency
> cores, and only one of them can be active at a given time depending on
> the current p-state. However, only the performance cores can execute
> 32-bit EL0. This results in logical cores that can only execute 32-bit
> EL0 in high p-states.
Further research shows that the MPIDR_EL1 values between the two core
types are different. And whether the two core type have any extra
differences is anyone's guess right now. So far, nothing seems to break
horribly without special workarounds for the MPIDR value (with cpufreq
enabled downstream) as:
1. There are no KVM, GIC, ACPI, PSCI or cpuidle
2. All CPUs switch P-mode and E-mode together

However, all of this is broken enough that this piece of code should go
into arch/arm64/kernel/cpu_errata.c, and also generate a
TAINT_CPU_OUT_OF_SPEC for these cursed CPUs.

> 
> Trying to support 32-bit EL0 on a CPU that can only execute it in certain
> states is a bad idea. The A10 family only supports 16KB page size anyway
> so many AArch32 executables won't run anyways. Pretend that it does not
> support 32-bit EL0 at all.
> 
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
>  arch/arm64/kernel/cpufeature.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 718728a85430..386698f42172 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -3529,6 +3529,31 @@ void __init setup_boot_cpu_features(void)
>  	setup_boot_cpu_capabilities();
>  }
>  
> +static void __init bad_aarch32_el0_fixup(void)
> +{
> +	static const struct midr_range bad_aarch32_el0[] = {
> +		MIDR_ALL_VERSIONS(MIDR_APPLE_A10_T2_HURRICANE_ZEPHYR),
> +		MIDR_ALL_VERSIONS(MIDR_APPLE_A10X_HURRICANE_ZEPHYR),
> +		{}
> +	};
> +
> +	/*
> +	 * The Apple A10 family can only execute 32-bit EL0 when in high
> +	 * p-states. Pretend it does not support 32-bit EL0.
> +	 */
> +	if (is_midr_in_range_list(read_cpuid_id(), bad_aarch32_el0)) {
> +		struct arm64_ftr_reg *regp;
> +
> +		regp = get_arm64_ftr_reg(SYS_ID_AA64PFR0_EL1);
> +		if (!regp)
> +			return;
> +		u64 val = (regp->sys_val & ~ID_AA64PFR0_EL1_EL0_MASK)
> +		  | ID_AA64PFR0_EL1_EL0_IMP;
> +
> +		update_cpu_ftr_reg(regp, val);
> +	}
> +}
> +
>  static void __init setup_system_capabilities(void)
>  {
>  	/*
> @@ -3562,6 +3587,8 @@ static void __init setup_system_capabilities(void)
>  
>  void __init setup_system_features(void)
>  {
> +	bad_aarch32_el0_fixup();
> +
>  	setup_system_capabilities();
>  
>  	kpti_install_ng_mappings();

Nick Chan
Re: [PATCH v2 2/2] arm64: cpufeature: Pretend that Apple A10 family does not support 32-bit EL0
Posted by Catalin Marinas 2 months, 2 weeks ago
On Mon, Sep 16, 2024 at 09:41:12PM +0800, Nick Chan wrote:
> On 9/9/2024 17:10, Nick Chan wrote:
> > The Apple A10 family consists of physical performance and efficiency
> > cores, and only one of them can be active at a given time depending on
> > the current p-state. However, only the performance cores can execute
> > 32-bit EL0. This results in logical cores that can only execute 32-bit
> > EL0 in high p-states.
> 
> Further research shows that the MPIDR_EL1 values between the two core
> types are different. And whether the two core type have any extra
> differences is anyone's guess right now. So far, nothing seems to break
> horribly without special workarounds for the MPIDR value (with cpufreq
> enabled downstream) as:
> 1. There are no KVM, GIC, ACPI, PSCI or cpuidle
> 2. All CPUs switch P-mode and E-mode together
> 
> However, all of this is broken enough that this piece of code should go
> into arch/arm64/kernel/cpu_errata.c, and also generate a
> TAINT_CPU_OUT_OF_SPEC for these cursed CPUs.

I wouldn't carry any additional logic in the kernel for such
configuration (long time ago Arm had something similar, the big.LITTLE
switcher, but the CPUs were fairly similar from a feature perspective).

> > Trying to support 32-bit EL0 on a CPU that can only execute it in certain
> > states is a bad idea. The A10 family only supports 16KB page size anyway
> > so many AArch32 executables won't run anyways. Pretend that it does not
> > support 32-bit EL0 at all.

CONFIG_COMPAT depends on ARM64_4K_PAGES || EXPERT. Do we really need
these patches in case one enables EXPERT and tries to run 32-bit
binaries that never ran on 16K pages before?

-- 
Catalin
Re: [PATCH v2 2/2] arm64: cpufeature: Pretend that Apple A10 family does not support 32-bit EL0
Posted by Nick Chan 2 months, 2 weeks ago

Catalin Marinas 於 2024/9/16 晚上11:30 寫道:
> On Mon, Sep 16, 2024 at 09:41:12PM +0800, Nick Chan wrote:
>> On 9/9/2024 17:10, Nick Chan wrote:
>>> The Apple A10 family consists of physical performance and efficiency
>>> cores, and only one of them can be active at a given time depending on
>>> the current p-state. However, only the performance cores can execute
>>> 32-bit EL0. This results in logical cores that can only execute 32-bit
>>> EL0 in high p-states.
>>
>> Further research shows that the MPIDR_EL1 values between the two core
>> types are different. And whether the two core type have any extra
>> differences is anyone's guess right now. So far, nothing seems to break
>> horribly without special workarounds for the MPIDR value (with cpufreq
>> enabled downstream) as:
>> 1. There are no KVM, GIC, ACPI, PSCI or cpuidle
>> 2. All CPUs switch P-mode and E-mode together
>>
>> However, all of this is broken enough that this piece of code should go
>> into arch/arm64/kernel/cpu_errata.c, and also generate a
>> TAINT_CPU_OUT_OF_SPEC for these cursed CPUs.
> 
> I wouldn't carry any additional logic in the kernel for such
> configuration (long time ago Arm had something similar, the big.LITTLE
> switcher, but the CPUs were fairly similar from a feature perspective).
This is fine from a functionality perspective, currently nothing that
accesses MPIDR after boot is used on A10(X). However, it does not sound
right either to not note that the kernel is running on a cursed CPU.

> 
>>> Trying to support 32-bit EL0 on a CPU that can only execute it in certain
>>> states is a bad idea. The A10 family only supports 16KB page size anyway
>>> so many AArch32 executables won't run anyways. Pretend that it does not
>>> support 32-bit EL0 at all.
> 
> CONFIG_COMPAT depends on ARM64_4K_PAGES || EXPERT. Do we really need
> these patches in case one enables EXPERT and tries to run 32-bit
> binaries that never ran on 16K pages before?
The worst thing that can happen is the 32-bit process crashes with illegal
instruction, the kernel will still be fine.

> 

Nick Chan

Re: [PATCH v2 2/2] arm64: cpufeature: Pretend that Apple A10 family does not support 32-bit EL0
Posted by Catalin Marinas 2 months, 2 weeks ago
On Tue, Sep 17, 2024 at 12:00:10AM +0800, Nick Chan wrote:
> Catalin Marinas 於 2024/9/16 晚上11:30 寫道:
> > On Mon, Sep 16, 2024 at 09:41:12PM +0800, Nick Chan wrote:
> >> On 9/9/2024 17:10, Nick Chan wrote:
> >>> Trying to support 32-bit EL0 on a CPU that can only execute it in certain
> >>> states is a bad idea. The A10 family only supports 16KB page size anyway
> >>> so many AArch32 executables won't run anyways. Pretend that it does not
> >>> support 32-bit EL0 at all.
> > 
> > CONFIG_COMPAT depends on ARM64_4K_PAGES || EXPERT. Do we really need
> > these patches in case one enables EXPERT and tries to run 32-bit
> > binaries that never ran on 16K pages before?
> 
> The worst thing that can happen is the 32-bit process crashes with illegal
> instruction, the kernel will still be fine.

Yes, but that's not my point. By default you do not get CONFIG_COMPAT
enabled with CONFIG_ARM64_16K_PAGES. So these patches are not really
necessary (unless one enabled EXPERT and turns on CONFIG_COMPAT but
these are specialist cases that I don't care about).

-- 
Catalin