[PATCH 0/4] arm64: errata: Workaround Neoverse-N1 #1542419

[PATCH 0/4] arm64: errata: Workaround Neoverse-N1 #1542419

[James Morse]

Hello!

Neoverse-N1 cores with the 'COHERENT_ICACHE' feature may fetch stale
instructions when software depends on prefetch-speculation-protection
instead of explicit synchronization. [0]

This concerns self modifying executables, like a JIT ... and the kernel.
The ARM-ARM has some fairly heavy rules for software that modifies
instructions that may be currently executing on another CPU.
There are some easier requirements if the instructions being modified
are just between Branches and NOP, (and a few others).

(See B2.2.5 "Concurrent modification and execution of instructions"
of DDI0487E.a for details).


A JIT can use all this to avoid synchronisation between threads. It
can generate new instructions at some new location, then update a
branch in the executable instructions to point at the new location.

Prefetch-speculation-protection guarantees that if another CPU sees
the new branch, it also sees the new instructions that were written
there.

On affected Neoverse-N1 cores, this goes wrong.


The workaround is to trap I-Cache maintenance and issue an
inner-shareable TLBI.

The affected cores have a Coherent I-Cache, so the I-Cache maintenance
isn't necessary. The core tells user-space it can skip it with
CTR_EL0.DIC. We also have to trap this register to hide the bit forcing
DIC-aware user-space to perform the maintenance.

Because the cache-maintenance wasn't needed, we can do the TLBI instead.
In fact, the I-Cache line-size isn't relevant anymore, we can reduce
the number of traps by produceing a fake value.

Unfortunately the bulk of user-space is not DIC-aware, it blindly does
the D-side and I-side cache maintenance. To make matters worse, the
kernel can only trap all cache maintenance from EL0 with SCTLR_EL1.UCI.
The normal-world can't trap Data/Instruction cache maintenance
independently, but EL3 firmware can.

To avoid trapping all cache-maintenance, this workaround depends on
a firmware component that only traps I-cache maintenance from EL0 and
performs the workaround.


For user-space, the kernel's work is now to trap CTR_EL0 to hide DIC,
and produce a fake IminLine. EL3 traps the now-necessary I-Cache
maintenance and performs the inner-shareable-TLBI that makes everything
better.

We can't yet detect whether EL3 has the workaround for any particular
erratum. We lamely print '(kernel portion)' as part of the CPU-feature
text.
If we get a mechanism to discover this stuff we can use SCTLR_EL1.UCI
on systems that have the interface, and don't workaround the issue.


While the kernel has some JIT like features, they don't rely on
prefetch-speculation-protection. In particular the module-loader
ends up calling kick_all_cpus_sync() as part of the 'heavy' rules
above. Patch 4 lists the cases the kernel modifies itself. Only one
of these depends on prefetch-speculation-protection, it is easy enough
to remove it.


... questions welcome ...

Thanks,

James

[0] https://developer.arm.com/docs/sden885747/latest/arm-neoverse-n1-mp050-software-developer-errata-notice

James Morse (4):
  arm64: errata: Hide CTR_EL0.DIC on systems affected by Neoverse-N1
    #1542419
  arm64: Fake the IminLine size on systems affected by Neoverse-N1
    #1542419
  arm64: compat: Workaround Neoverse-N1 #1542419 for compat user-space
  arm64: ftrace: Ensure synchronisation in PLT setup for Neoverse-N1
    #1542419

 arch/arm64/Kconfig               | 16 ++++++++++++++++
 arch/arm64/include/asm/cache.h   |  3 ++-
 arch/arm64/include/asm/cpucaps.h |  3 ++-
 arch/arm64/kernel/cpu_errata.c   | 30 ++++++++++++++++++++++++++++++
 arch/arm64/kernel/ftrace.c       | 12 +++++++++---
 arch/arm64/kernel/sys_compat.c   | 11 +++++++++++
 arch/arm64/kernel/traps.c        |  7 +++++++
 7 files changed, 77 insertions(+), 5 deletions(-)

-- 
2.20.1


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

[PATCH 1/4] arm64: errata: Hide CTR_EL0.DIC on systems affected by Neoverse-N1 #1542419

[James Morse]

Cores affected by Neoverse-N1 #1542419 could execute a stale instruction
when a branch is updated to point to freshly generated instructions.

To workaround this issue we need user-space to issue unnecessary
icache maintenance that we can trap. Start by hiding CTR_EL0.DIC.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/Kconfig               | 16 ++++++++++++++++
 arch/arm64/include/asm/cpucaps.h |  3 ++-
 arch/arm64/kernel/cpu_errata.c   | 30 ++++++++++++++++++++++++++++++
 arch/arm64/kernel/traps.c        |  3 +++
 4 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 41a9b4257b72..f2e1965d2461 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -559,6 +559,22 @@ config ARM64_ERRATUM_1463225
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_1542419
+	bool "Neoverse-N1: workaround mis-ordering of instruction fetches"
+	default y
+	help
+	  This option adds a workaround for ARM Neoverse-N1 erratum
+	  1542419.
+
+	  Affected Neoverse-N1 cores could execute a stale instruction when
+	  modified by another CPU. The workaround depends on a firmware
+	  counterpart.
+
+	  Workaround the issue by hiding the DIC feature from EL0. This
+	  forces user-space to perform cache maintenance.
+
+	  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 	bool "Cavium erratum 22375, 24313"
 	default y
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index f19fe4b9acc4..f05afaec18cd 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -52,7 +52,8 @@
 #define ARM64_HAS_IRQ_PRIO_MASKING		42
 #define ARM64_HAS_DCPODP			43
 #define ARM64_WORKAROUND_1463225		44
+#define ARM64_WORKAROUND_1542419		45
 
-#define ARM64_NCAPS				45
+#define ARM64_NCAPS				46
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 1e43ba5c79b7..a7de0d5dde9a 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -90,10 +90,18 @@ static void
 cpu_enable_trap_ctr_access(const struct arm64_cpu_capabilities *__unused)
 {
 	u64 mask = arm64_ftr_reg_ctrel0.strict_mask;
+	bool enable_uct_trap = false;
 
 	/* Trap CTR_EL0 access on this CPU, only if it has a mismatch */
 	if ((read_cpuid_cachetype() & mask) !=
 	    (arm64_ftr_reg_ctrel0.sys_val & mask))
+		enable_uct_trap = true;
+
+	/* ... or if this CPU is affected by an errata */
+	if (this_cpu_has_cap(ARM64_WORKAROUND_1542419))
+		enable_uct_trap = true;
+
+	if (enable_uct_trap)
 		sysreg_clear_set(sctlr_el1, SCTLR_EL1_UCT, 0);
 }
 
@@ -623,6 +631,18 @@ check_branch_predictor(const struct arm64_cpu_capabilities *entry, int scope)
 	return (need_wa > 0);
 }
 
+static bool __maybe_unused
+has_neoverse_n1_erratum_1542419(const struct arm64_cpu_capabilities *entry,
+				int scope)
+{
+	u32 midr = read_cpuid_id();
+	bool has_dic = read_cpuid_cachetype() & BIT(CTR_DIC_SHIFT);
+	const struct midr_range range = MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1);
+
+	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
+	return is_midr_in_range(midr, &range) && has_dic;
+}
+
 #ifdef CONFIG_HARDEN_EL2_VECTORS
 
 static const struct midr_range arm64_harden_el2_vectors[] = {
@@ -851,6 +871,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
 		.matches = has_cortex_a76_erratum_1463225,
 	},
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_1542419
+	{
+		/* we depend on the firmware portion for correctness */
+		.desc = "ARM erratum 1542419 (kernel portion)",
+		.capability = ARM64_WORKAROUND_1542419,
+		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
+		.matches = has_neoverse_n1_erratum_1542419,
+		.cpu_enable = cpu_enable_trap_ctr_access,
+	},
 #endif
 	{
 	}
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 34739e80211b..465f0a0f8f0a 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -470,6 +470,9 @@ static void ctr_read_handler(unsigned int esr, struct pt_regs *regs)
 	int rt = ESR_ELx_SYS64_ISS_RT(esr);
 	unsigned long val = arm64_ftr_reg_user_value(&arm64_ftr_reg_ctrel0);
 
+	if (cpus_have_const_cap(ARM64_WORKAROUND_1542419))
+		val &= ~BIT(CTR_DIC_SHIFT);
+
 	pt_regs_write_reg(regs, rt, val);
 
 	arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
-- 
2.20.1


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

Re: [PATCH 1/4] arm64: errata: Hide CTR_EL0.DIC on systems affected by Neoverse-N1 #1542419

[Suzuki K Poulose]

Hi James,

On 02/10/2019 10:49, James Morse wrote:
> Cores affected by Neoverse-N1 #1542419 could execute a stale instruction
> when a branch is updated to point to freshly generated instructions.
> 
> To workaround this issue we need user-space to issue unnecessary
> icache maintenance that we can trap. Start by hiding CTR_EL0.DIC.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>   arch/arm64/Kconfig               | 16 ++++++++++++++++
>   arch/arm64/include/asm/cpucaps.h |  3 ++-
>   arch/arm64/kernel/cpu_errata.c   | 30 ++++++++++++++++++++++++++++++
>   arch/arm64/kernel/traps.c        |  3 +++
>   4 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 41a9b4257b72..f2e1965d2461 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -559,6 +559,22 @@ config ARM64_ERRATUM_1463225
>   
>   	  If unsure, say Y.
>   
> +config ARM64_ERRATUM_1542419
> +	bool "Neoverse-N1: workaround mis-ordering of instruction fetches"
> +	default y
> +	help
> +	  This option adds a workaround for ARM Neoverse-N1 erratum
> +	  1542419.
> +
> +	  Affected Neoverse-N1 cores could execute a stale instruction when
> +	  modified by another CPU. The workaround depends on a firmware
> +	  counterpart.
> +
> +	  Workaround the issue by hiding the DIC feature from EL0. This
> +	  forces user-space to perform cache maintenance.
> +
> +	  If unsure, say Y.
> +
>   config CAVIUM_ERRATUM_22375
>   	bool "Cavium erratum 22375, 24313"
>   	default y
> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
> index f19fe4b9acc4..f05afaec18cd 100644
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ b/arch/arm64/include/asm/cpucaps.h
> @@ -52,7 +52,8 @@
>   #define ARM64_HAS_IRQ_PRIO_MASKING		42
>   #define ARM64_HAS_DCPODP			43
>   #define ARM64_WORKAROUND_1463225		44
> +#define ARM64_WORKAROUND_1542419		45
>   
> -#define ARM64_NCAPS				45
> +#define ARM64_NCAPS				46
>   
>   #endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 1e43ba5c79b7..a7de0d5dde9a 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -90,10 +90,18 @@ static void
>   cpu_enable_trap_ctr_access(const struct arm64_cpu_capabilities *__unused)
>   {
>   	u64 mask = arm64_ftr_reg_ctrel0.strict_mask;
> +	bool enable_uct_trap = false;
>   
>   	/* Trap CTR_EL0 access on this CPU, only if it has a mismatch */
>   	if ((read_cpuid_cachetype() & mask) !=
>   	    (arm64_ftr_reg_ctrel0.sys_val & mask))
> +		enable_uct_trap = true;
> +
> +	/* ... or if this CPU is affected by an errata */
> +	if (this_cpu_has_cap(ARM64_WORKAROUND_1542419))
> +		enable_uct_trap = true;

I think we need to trap the CTR accesses on all the CPUs if at least one
of the CPU is affected by the Errata. i.e, even if both the LITTLE and the
big CPU has DIC, we must trap this on both types to make sure the application
doesn't use a cached value of the DIC read on the CPU that is not affected.

So we could change this enable() routine a bit to :

cpu_enable_trap_ctr_access(const struct arm64_cpu_capabilities *cap)
{
	.... existing checks ....

	if (cap->capability == ARM64_WORKAROUND_1542419)
		enable_uct_trap = true;

}


Rest looks good to me.

Cheers
Suuzki

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

Re: [PATCH 1/4] arm64: errata: Hide CTR_EL0.DIC on systems affected by Neoverse-N1 #1542419

[James Morse]

Hi Suzuki,

On 07/10/2019 10:38, Suzuki K Poulose wrote:
> On 02/10/2019 10:49, James Morse wrote:
>> Cores affected by Neoverse-N1 #1542419 could execute a stale instruction
>> when a branch is updated to point to freshly generated instructions.
>>
>> To workaround this issue we need user-space to issue unnecessary
>> icache maintenance that we can trap. Start by hiding CTR_EL0.DIC.


>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index 1e43ba5c79b7..a7de0d5dde9a 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -90,10 +90,18 @@ static void
>>   cpu_enable_trap_ctr_access(const struct arm64_cpu_capabilities *__unused)
>>   {
>>       u64 mask = arm64_ftr_reg_ctrel0.strict_mask;
>> +    bool enable_uct_trap = false;
>>         /* Trap CTR_EL0 access on this CPU, only if it has a mismatch */
>>       if ((read_cpuid_cachetype() & mask) !=
>>           (arm64_ftr_reg_ctrel0.sys_val & mask))
>> +        enable_uct_trap = true;
>> +
>> +    /* ... or if this CPU is affected by an errata */
>> +    if (this_cpu_has_cap(ARM64_WORKAROUND_1542419))
>> +        enable_uct_trap = true;
> 
> I think we need to trap the CTR accesses on all the CPUs if at least one
> of the CPU is affected by the Errata. i.e, even if both the LITTLE and the
> big CPU has DIC, we must trap this on both types to make sure the application
> doesn't use a cached value of the DIC read on the CPU that is not affected.

Good point! ... I hadn't considered big-little.


Thanks,

James

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

[PATCH 2/4] arm64: Fake the IminLine size on systems affected by Neoverse-N1 #1542419

[James Morse]

Systems affected by Neoverse-N1 #1542419 support DIC so do not need to
perform icache maintenance once new instructions are cleaned to the PoU.
For the errata workaround, the kernel hides DIC from user-space, so that
the unnecessary cache maintenance can be trapped by firmware.

To reduce the number of traps, produce a fake IminLine value based on
PAGE_SIZE.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/include/asm/cache.h | 3 ++-
 arch/arm64/kernel/traps.c      | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
index 43da6dd29592..806e9dc2a852 100644
--- a/arch/arm64/include/asm/cache.h
+++ b/arch/arm64/include/asm/cache.h
@@ -11,6 +11,7 @@
 #define CTR_L1IP_MASK		3
 #define CTR_DMINLINE_SHIFT	16
 #define CTR_IMINLINE_SHIFT	0
+#define CTR_IMINLINE_MASK	0xf
 #define CTR_ERG_SHIFT		20
 #define CTR_CWG_SHIFT		24
 #define CTR_CWG_MASK		15
@@ -18,7 +19,7 @@
 #define CTR_DIC_SHIFT		29
 
 #define CTR_CACHE_MINLINE_MASK	\
-	(0xf << CTR_DMINLINE_SHIFT | 0xf << CTR_IMINLINE_SHIFT)
+	(0xf << CTR_DMINLINE_SHIFT | CTR_IMINLINE_MASK << CTR_IMINLINE_SHIFT)
 
 #define CTR_L1IP(ctr)		(((ctr) >> CTR_L1IP_SHIFT) & CTR_L1IP_MASK)
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 465f0a0f8f0a..991647a65fe8 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -470,9 +470,13 @@ static void ctr_read_handler(unsigned int esr, struct pt_regs *regs)
 	int rt = ESR_ELx_SYS64_ISS_RT(esr);
 	unsigned long val = arm64_ftr_reg_user_value(&arm64_ftr_reg_ctrel0);
 
-	if (cpus_have_const_cap(ARM64_WORKAROUND_1542419))
+	if (cpus_have_const_cap(ARM64_WORKAROUND_1542419)) {
 		val &= ~BIT(CTR_DIC_SHIFT);
 
+		val &= ~CTR_IMINLINE_MASK;
+		val |= PAGE_SHIFT - 2;
+	}
+
 	pt_regs_write_reg(regs, rt, val);
 
 	arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
-- 
2.20.1


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

Re: [PATCH 2/4] arm64: Fake the IminLine size on systems affected by Neoverse-N1 #1542419

[Suzuki K Poulose]

On 02/10/2019 10:49, James Morse wrote:
> Systems affected by Neoverse-N1 #1542419 support DIC so do not need to
> perform icache maintenance once new instructions are cleaned to the PoU.
> For the errata workaround, the kernel hides DIC from user-space, so that
> the unnecessary cache maintenance can be trapped by firmware.
> 
> To reduce the number of traps, produce a fake IminLine value based on
> PAGE_SIZE.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>   arch/arm64/include/asm/cache.h | 3 ++-
>   arch/arm64/kernel/traps.c      | 6 +++++-
>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
> index 43da6dd29592..806e9dc2a852 100644
> --- a/arch/arm64/include/asm/cache.h
> +++ b/arch/arm64/include/asm/cache.h
> @@ -11,6 +11,7 @@
>   #define CTR_L1IP_MASK		3
>   #define CTR_DMINLINE_SHIFT	16
>   #define CTR_IMINLINE_SHIFT	0
> +#define CTR_IMINLINE_MASK	0xf
>   #define CTR_ERG_SHIFT		20
>   #define CTR_CWG_SHIFT		24
>   #define CTR_CWG_MASK		15
> @@ -18,7 +19,7 @@
>   #define CTR_DIC_SHIFT		29
>   
>   #define CTR_CACHE_MINLINE_MASK	\
> -	(0xf << CTR_DMINLINE_SHIFT | 0xf << CTR_IMINLINE_SHIFT)
> +	(0xf << CTR_DMINLINE_SHIFT | CTR_IMINLINE_MASK << CTR_IMINLINE_SHIFT)
>   
>   #define CTR_L1IP(ctr)		(((ctr) >> CTR_L1IP_SHIFT) & CTR_L1IP_MASK)
>   
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 465f0a0f8f0a..991647a65fe8 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -470,9 +470,13 @@ static void ctr_read_handler(unsigned int esr, struct pt_regs *regs)
>   	int rt = ESR_ELx_SYS64_ISS_RT(esr);
>   	unsigned long val = arm64_ftr_reg_user_value(&arm64_ftr_reg_ctrel0);
>   
> -	if (cpus_have_const_cap(ARM64_WORKAROUND_1542419))
> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1542419)) {
>   		val &= ~BIT(CTR_DIC_SHIFT);
>   
> +		val &= ~CTR_IMINLINE_MASK;
> +		val |= PAGE_SHIFT - 2;

nit: I would do :
		val |= (PAGE_SHIFT - 2) & CTR_IMINLINE_MASK;

minor nit: Also do we need a comment here to say why we do the IMINLINE bit ? I
understand it is part of the commit, but having it here may be helpful.

Either way:

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

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

Re: [PATCH 2/4] arm64: Fake the IminLine size on systems affected by Neoverse-N1 #1542419

[James Morse]

Hi Suzuki,

On 07/10/2019 10:48, Suzuki K Poulose wrote:
> On 02/10/2019 10:49, James Morse wrote:
>> Systems affected by Neoverse-N1 #1542419 support DIC so do not need to
>> perform icache maintenance once new instructions are cleaned to the PoU.
>> For the errata workaround, the kernel hides DIC from user-space, so that
>> the unnecessary cache maintenance can be trapped by firmware.
>>
>> To reduce the number of traps, produce a fake IminLine value based on
>> PAGE_SIZE.


>>   diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>> index 465f0a0f8f0a..991647a65fe8 100644
>> --- a/arch/arm64/kernel/traps.c
>> +++ b/arch/arm64/kernel/traps.c
>> @@ -470,9 +470,13 @@ static void ctr_read_handler(unsigned int esr, struct pt_regs *regs)
>>       int rt = ESR_ELx_SYS64_ISS_RT(esr);
>>       unsigned long val = arm64_ftr_reg_user_value(&arm64_ftr_reg_ctrel0);
>>   -    if (cpus_have_const_cap(ARM64_WORKAROUND_1542419))
>> +    if (cpus_have_const_cap(ARM64_WORKAROUND_1542419)) {
>>           val &= ~BIT(CTR_DIC_SHIFT);
>>   +        val &= ~CTR_IMINLINE_MASK;
>> +        val |= PAGE_SHIFT - 2;

> nit: I would do :
>         val |= (PAGE_SHIFT - 2) & CTR_IMINLINE_MASK;

I guess that saves anyone having to worry about surprising PAGE_SHIFT values,


> minor nit: Also do we need a comment here to say why we do the IMINLINE bit ? I
> understand it is part of the commit, but having it here may be helpful.

Sure,


> Either way:
> 
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>


Thanks!

James

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

[PATCH 3/4] arm64: compat: Workaround Neoverse-N1 #1542419 for compat user-space

[James Morse]

Compat user-space is unable to perform ICIMVAU instructions from
user-space. Instead it uses a compat-syscall. Add the workaround for
Neoverse-N1 #1542419 to this code path.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/kernel/sys_compat.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/kernel/sys_compat.c b/arch/arm64/kernel/sys_compat.c
index f1cb64959427..1d09160fdd25 100644
--- a/arch/arm64/kernel/sys_compat.c
+++ b/arch/arm64/kernel/sys_compat.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/compat.h>
+#include <linux/cpufeature.h>
 #include <linux/personality.h>
 #include <linux/sched.h>
 #include <linux/sched/signal.h>
@@ -17,6 +18,7 @@
 
 #include <asm/cacheflush.h>
 #include <asm/system_misc.h>
+#include <asm/tlbflush.h>
 #include <asm/unistd.h>
 
 static long
@@ -30,6 +32,15 @@ __do_compat_cache_op(unsigned long start, unsigned long end)
 		if (fatal_signal_pending(current))
 			return 0;
 
+		if (cpus_have_const_cap(ARM64_WORKAROUND_1542419)) {
+			/*
+			 * The workround requires an inner-shareable tlbi.
+			 * We pick the reserved-ASID to minimise the impact.
+			 */
+			__tlbi(aside1is, 0);
+			dsb(ish);
+		}
+
 		ret = __flush_cache_user_range(start, start + chunk);
 		if (ret)
 			return ret;
-- 
2.20.1


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

[PATCH 4/4] arm64: ftrace: Ensure synchronisation in PLT setup for Neoverse-N1 #1542419

[James Morse]

CPUs affected by Neoverse-N1 #1542419 may execute a stale instruction if
it was recently modified. The affected sequence requires freshly written
instructions to be executable before a branch to them is updated.

There are very few places in the kernel that modify executable text,
all but one come with sufficient synchronisation:
 * The module loader's flush_module_icache() calls flush_icache_range(),
   which does a kick_all_cpus_sync()
 * bpf_int_jit_compile() calls flush_icache_range().
 * Kprobes calls aarch64_insn_patch_text(), which does its work in
   stop_machine().
 * static keys and ftrace both patch between nops and branches to
   existing kernel code (not generated code).

The affected sequence is the interaction between ftrace and modules.
The module PLT is cleaned using __flush_icache_range() as the trampoline
shouldn't be executable until we update the branch to it.

Drop the double-underscore so that this path runs kick_all_cpus_sync()
too.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/kernel/ftrace.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 171773257974..06e56b470315 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -121,10 +121,16 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 
 			/*
 			 * Ensure updated trampoline is visible to instruction
-			 * fetch before we patch in the branch.
+			 * fetch before we patch in the branch. Although the
+			 * architecture doesn't require an IPI in this case,
+			 * Neoverse-N1 erratum #1542419 does require one
+			 * if the TLB maintenance in module_enable_ro() is
+			 * skipped due to rodata_enabled. It doesn't seem worth
+			 * it to make it conditional given that this is
+			 * certainly not a fast-path.
 			 */
-			__flush_icache_range((unsigned long)&dst[0],
-					     (unsigned long)&dst[1]);
+			flush_icache_range((unsigned long)&dst[0],
+					   (unsigned long)&dst[1]);
 		}
 		addr = (unsigned long)dst;
 #else /* CONFIG_ARM64_MODULE_PLTS */
-- 
2.20.1


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

Re: [PATCH 4/4] arm64: ftrace: Ensure synchronisation in PLT setup for Neoverse-N1 #1542419

[Will Deacon]

On Wed, Oct 02, 2019 at 10:49:35AM +0100, James Morse wrote:
> CPUs affected by Neoverse-N1 #1542419 may execute a stale instruction if
> it was recently modified. The affected sequence requires freshly written
> instructions to be executable before a branch to them is updated.
> 
> There are very few places in the kernel that modify executable text,
> all but one come with sufficient synchronisation:
>  * The module loader's flush_module_icache() calls flush_icache_range(),
>    which does a kick_all_cpus_sync()
>  * bpf_int_jit_compile() calls flush_icache_range().
>  * Kprobes calls aarch64_insn_patch_text(), which does its work in
>    stop_machine().
>  * static keys and ftrace both patch between nops and branches to
>    existing kernel code (not generated code).
> 
> The affected sequence is the interaction between ftrace and modules.
> The module PLT is cleaned using __flush_icache_range() as the trampoline
> shouldn't be executable until we update the branch to it.
> 
> Drop the double-underscore so that this path runs kick_all_cpus_sync()
> too.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  arch/arm64/kernel/ftrace.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

I'll take this one as a fix, since it's not reliant on a firmware update.
One other thing -- please can you update silicon-errata.rst as part of the
other patches?

Will

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

Re: [PATCH 4/4] arm64: ftrace: Ensure synchronisation in PLT setup for Neoverse-N1 #1542419

[James Morse]

Hi Will,

On 04/10/2019 11:33, Will Deacon wrote:
> On Wed, Oct 02, 2019 at 10:49:35AM +0100, James Morse wrote:
>> CPUs affected by Neoverse-N1 #1542419 may execute a stale instruction if
>> it was recently modified. The affected sequence requires freshly written
>> instructions to be executable before a branch to them is updated.
>>
>> There are very few places in the kernel that modify executable text,
>> all but one come with sufficient synchronisation:
>>  * The module loader's flush_module_icache() calls flush_icache_range(),
>>    which does a kick_all_cpus_sync()
>>  * bpf_int_jit_compile() calls flush_icache_range().
>>  * Kprobes calls aarch64_insn_patch_text(), which does its work in
>>    stop_machine().
>>  * static keys and ftrace both patch between nops and branches to
>>    existing kernel code (not generated code).
>>
>> The affected sequence is the interaction between ftrace and modules.
>> The module PLT is cleaned using __flush_icache_range() as the trampoline
>> shouldn't be executable until we update the branch to it.
>>
>> Drop the double-underscore so that this path runs kick_all_cpus_sync()
>> too.

> I'll take this one as a fix, since it's not reliant on a firmware update.

> One other thing -- please can you update silicon-errata.rst as part of the
> other patches?

Gah, bother.

I'll fix that and repost this for rc2, noting its for v5.5.


Thanks,

James

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

Re: [PATCH 0/4] arm64: errata: Workaround Neoverse-N1 #1542419

[Will Deacon]

On Wed, Oct 02, 2019 at 10:49:31AM +0100, James Morse wrote:
> We can't yet detect whether EL3 has the workaround for any particular
> erratum. We lamely print '(kernel portion)' as part of the CPU-feature
> text.
> If we get a mechanism to discover this stuff we can use SCTLR_EL1.UCI
> on systems that have the interface, and don't workaround the issue.

Is the firmware portion available in ATF and is there a plan to add the
discoverability mechanism, or is it just wishful thinking?

Will

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

Re: [PATCH 0/4] arm64: errata: Workaround Neoverse-N1 #1542419

[James Morse]

Hi Will,

On 04/10/2019 11:31, Will Deacon wrote:
> On Wed, Oct 02, 2019 at 10:49:31AM +0100, James Morse wrote:
>> We can't yet detect whether EL3 has the workaround for any particular
>> erratum. We lamely print '(kernel portion)' as part of the CPU-feature
>> text.
>> If we get a mechanism to discover this stuff we can use SCTLR_EL1.UCI
>> on systems that have the interface, and don't workaround the issue.
> 
> Is the firmware portion available in ATF and is there a plan to add the
> discoverability mechanism, or is it just wishful thinking?

The ATF part is here:
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2173

Being able to discover whether ATF is doing its part is wishful thinking, but stuff like
this shows we need a way of finding out.


Thanks,

James

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