2022-10-27 02:49:45

by Anshuman Khandual

[permalink] [raw]
Subject: [PATCH 0/2] arm64: errata: Workaround Cortex-A715 errata #2645198

This series adds Cortex-A715 partnumber and workarounds the errata #2645198
which gets triggered when an userspace page mapping permission changes from
executable to non-executable, corrupting both ESR_EL1/FAR_EL1 registers
when an instruction abort is taken.

This series applies on v6.1-rc1.

The errata description can be found here.

https://developer.arm.com/documentation/SDEN2148827/1000/?lang=en

Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Suzuki K Poulose <[email protected]>
Cc: James Morse <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

Anshuman Khandual (2):
arm64: Add Cortex-715 CPU part definition
arm64: errata: Workaround possible Cortex-A715 [ESR|FAR]_ELx corruption

Documentation/arm64/silicon-errata.rst | 2 ++
arch/arm64/Kconfig | 16 +++++++++++
arch/arm64/include/asm/cputype.h | 2 ++
arch/arm64/include/asm/hugetlb.h | 9 +++++++
arch/arm64/include/asm/pgtable.h | 24 +++++++++++++++++
arch/arm64/kernel/cpu_errata.c | 7 +++++
arch/arm64/mm/hugetlbpage.c | 37 ++++++++++++++++++++++++++
arch/arm64/tools/cpucaps | 1 +
8 files changed, 98 insertions(+)

--
2.25.1



2022-10-27 02:50:41

by Anshuman Khandual

[permalink] [raw]
Subject: [PATCH 1/2] arm64: Add Cortex-715 CPU part definition

Add the CPU Partnumbers for the new Arm designs.

Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Suzuki K Poulose <[email protected]>
Cc: James Morse <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anshuman Khandual <[email protected]>
---
arch/arm64/include/asm/cputype.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index abc418650fec..4b1ad810436f 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -80,6 +80,7 @@
#define ARM_CPU_PART_CORTEX_X1 0xD44
#define ARM_CPU_PART_CORTEX_A510 0xD46
#define ARM_CPU_PART_CORTEX_A710 0xD47
+#define ARM_CPU_PART_CORTEX_A715 0xD4D
#define ARM_CPU_PART_CORTEX_X2 0xD48
#define ARM_CPU_PART_NEOVERSE_N2 0xD49
#define ARM_CPU_PART_CORTEX_A78C 0xD4B
@@ -142,6 +143,7 @@
#define MIDR_CORTEX_X1 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X1)
#define MIDR_CORTEX_A510 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A510)
#define MIDR_CORTEX_A710 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A710)
+#define MIDR_CORTEX_A715 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A715)
#define MIDR_CORTEX_X2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X2)
#define MIDR_NEOVERSE_N2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N2)
#define MIDR_CORTEX_A78C MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A78C)
--
2.25.1


2022-10-27 02:55:58

by Anshuman Khandual

[permalink] [raw]
Subject: [PATCH 2/2] arm64: errata: Workaround possible Cortex-A715 [ESR|FAR]_ELx corruption

If a Cortex-A715 cpu sees a page mapping permissions change from executable
to non-executable, it may corrupt the ESR_ELx and FAR_ELx registers, on the
next instruction abort caused by permission fault.

Only user-space does executable to non-executable permission transition via
mprotect() system call which calls ptep_modify_prot_start() and ptep_modify
_prot_commit() helpers, while changing the page mapping. The platform code
can override these helpers via __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION.

Work around the problem via doing a break-before-make TLB invalidation, for
all executable user space mappings, that go through mprotect() system call.
This overrides ptep_modify_prot_start() and ptep_modify_prot_commit(), via
defining HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION on the platform thus giving
an opportunity to intercept user space exec mappings, and do the necessary
TLB invalidation. Similar interceptions are also implemented for HugeTLB.

Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anshuman Khandual <[email protected]>
---
Documentation/arm64/silicon-errata.rst | 2 ++
arch/arm64/Kconfig | 16 +++++++++++
arch/arm64/include/asm/hugetlb.h | 9 +++++++
arch/arm64/include/asm/pgtable.h | 24 +++++++++++++++++
arch/arm64/kernel/cpu_errata.c | 7 +++++
arch/arm64/mm/hugetlbpage.c | 37 ++++++++++++++++++++++++++
arch/arm64/tools/cpucaps | 1 +
7 files changed, 96 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index 808ade4cc008..ec5f889d7681 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -120,6 +120,8 @@ stable kernels.
+----------------+-----------------+-----------------+-----------------------------+
| ARM | Cortex-A710 | #2224489 | ARM64_ERRATUM_2224489 |
+----------------+-----------------+-----------------+-----------------------------+
+| ARM | Cortex-A715 | #2645198 | ARM64_ERRATUM_2645198 |
++----------------+-----------------+-----------------+-----------------------------+
| ARM | Cortex-X2 | #2119858 | ARM64_ERRATUM_2119858 |
+----------------+-----------------+-----------------+-----------------------------+
| ARM | Cortex-X2 | #2224489 | ARM64_ERRATUM_2224489 |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 505c8a1ccbe0..56c3381e9d94 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -964,6 +964,22 @@ config ARM64_ERRATUM_2457168

If unsure, say Y.

+config ARM64_ERRATUM_2645198
+ bool "Cortex-A715: 2645198: Workaround possible [ESR|FAR]_ELx corruption"
+ default y
+ help
+ This option adds the workaround for ARM Cortex-A715 erratum 2645198.
+
+ If a Cortex-A715 cpu sees a page mapping permissions change from executable
+ to non-executable, it may corrupt the ESR_ELx and FAR_ELx registers on the
+ next instruction abort caused by permission fault.
+
+ Only user-space does executable to non-executable permission transition via
+ mprotect() system call. Workaround the problem by doing a break-before-make
+ TLB invalidation, for all changes to executable user space mappings.
+
+ If unsure, say Y.
+
config CAVIUM_ERRATUM_22375
bool "Cavium erratum 22375, 24313"
default y
diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
index d20f5da2d76f..6a4a1ab8eb23 100644
--- a/arch/arm64/include/asm/hugetlb.h
+++ b/arch/arm64/include/asm/hugetlb.h
@@ -49,6 +49,15 @@ extern pte_t huge_ptep_get(pte_t *ptep);

void __init arm64_hugetlb_cma_reserve(void);

+#define huge_ptep_modify_prot_start huge_ptep_modify_prot_start
+extern pte_t huge_ptep_modify_prot_start(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep);
+
+#define huge_ptep_modify_prot_commit huge_ptep_modify_prot_commit
+extern void huge_ptep_modify_prot_commit(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep,
+ pte_t old_pte, pte_t new_pte);
+
#include <asm-generic/hugetlb.h>

#endif /* __ASM_HUGETLB_H */
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 71a1af42f0e8..c4c021277f20 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -1095,7 +1095,31 @@ static inline bool pud_sect_supported(void)
return PAGE_SIZE == SZ_4K;
}

+#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
+static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma,
+ unsigned long addr,
+ pte_t *ptep)
+{
+ pte_t pte = ptep_get_and_clear(vma->vm_mm, addr, ptep);

+ if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
+ /*
+ * Break-before-make (BBM) is required for all user space mappings
+ * when the permission changes from executable to non-executable
+ * in cases where cpu is affected with errata #2645198.
+ */
+ if (pte_user_exec(pte) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
+ __flush_tlb_range(vma, addr, addr + PAGE_SIZE, PAGE_SIZE, false, 3);
+ }
+ return pte;
+}
+
+static inline void ptep_modify_prot_commit(struct vm_area_struct *vma,
+ unsigned long addr,
+ pte_t *ptep, pte_t old_pte, pte_t pte)
+{
+ __set_pte_at(vma->vm_mm, addr, ptep, pte);
+}
#endif /* !__ASSEMBLY__ */

#endif /* __ASM_PGTABLE_H */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 89ac00084f38..307faa2b4395 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -661,6 +661,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus),
},
#endif
+#ifdef CONFIG_ARM64_ERRATUM_2645198
+ {
+ .desc = "ARM erratum 2645198",
+ .capability = ARM64_WORKAROUND_2645198,
+ ERRATA_MIDR_ALL_VERSIONS(MIDR_CORTEX_A715)
+ },
+#endif
#ifdef CONFIG_ARM64_ERRATUM_2077057
{
.desc = "ARM erratum 2077057",
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 35e9a468d13e..8cdcb3a34c27 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -559,3 +559,40 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
{
return __hugetlb_valid_size(size);
}
+
+pte_t huge_ptep_modify_prot_start(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep)
+{
+ pte_t pte = huge_ptep_get_and_clear(vma->vm_mm, addr, ptep);
+
+ if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
+ /*
+ * Break-before-make (BBM) is required for all user space mappings
+ * when the permission changes from executable to non-executable
+ * in cases where cpu is affected with errata #2645198.
+ */
+ if (pte_user_exec(pte) && cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
+ size_t pgsize = page_size(pte_page(pte));
+ int level = 3;
+
+ if (pgsize == PUD_SIZE)
+ level = 1;
+ else if ((pgsize == PMD_SIZE) || (pgsize == CONT_PMD_SIZE))
+ level = 2;
+ else if (pgsize == CONT_PTE_SIZE)
+ level = 3;
+ else
+ pr_warn("%s: unrecognized huge page size 0x%lx\n",
+ __func__, pgsize);
+ __flush_tlb_range(vma, addr, addr + pgsize, pgsize, false, level);
+ }
+ }
+ return pte;
+}
+
+void huge_ptep_modify_prot_commit(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep,
+ pte_t old_pte, pte_t pte)
+{
+ set_huge_pte_at(vma->vm_mm, addr, ptep, pte);
+}
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index f1c0347ec31a..2274d836fcfe 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -70,6 +70,7 @@ WORKAROUND_2038923
WORKAROUND_2064142
WORKAROUND_2077057
WORKAROUND_2457168
+WORKAROUND_2645198
WORKAROUND_2658417
WORKAROUND_TRBE_OVERWRITE_FILL_MODE
WORKAROUND_TSB_FLUSH_FAILURE
--
2.25.1


2022-11-09 19:32:40

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: errata: Workaround possible Cortex-A715 [ESR|FAR]_ELx corruption

On Thu, Oct 27, 2022 at 08:09:15AM +0530, Anshuman Khandual wrote:
> +#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
> +static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma,
> + unsigned long addr,
> + pte_t *ptep)
> +{
> + pte_t pte = ptep_get_and_clear(vma->vm_mm, addr, ptep);
>
> + if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> + /*
> + * Break-before-make (BBM) is required for all user space mappings
> + * when the permission changes from executable to non-executable
> + * in cases where cpu is affected with errata #2645198.
> + */
> + if (pte_user_exec(pte) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
> + __flush_tlb_range(vma, addr, addr + PAGE_SIZE, PAGE_SIZE, false, 3);

Why not flush_tlb_page() here?

But more importantly, can we not use ptep_clear_flush() instead (and
huge_ptep_clear_flush())? They return the pte and do the TLBI.

--
Catalin

2022-11-10 03:40:37

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: errata: Workaround possible Cortex-A715 [ESR|FAR]_ELx corruption



On 11/10/22 00:48, Catalin Marinas wrote:
> On Thu, Oct 27, 2022 at 08:09:15AM +0530, Anshuman Khandual wrote:
>> +#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
>> +static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma,
>> + unsigned long addr,
>> + pte_t *ptep)
>> +{
>> + pte_t pte = ptep_get_and_clear(vma->vm_mm, addr, ptep);
>>
>> + if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
>> + /*
>> + * Break-before-make (BBM) is required for all user space mappings
>> + * when the permission changes from executable to non-executable
>> + * in cases where cpu is affected with errata #2645198.
>> + */
>> + if (pte_user_exec(pte) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
>> + __flush_tlb_range(vma, addr, addr + PAGE_SIZE, PAGE_SIZE, false, 3);
>
> Why not flush_tlb_page() here?
>
> But more importantly, can we not use ptep_clear_flush() instead (and

Something like ...

ptep_modify_prot_start -

if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
if (pte_user_exec(READ_ONCE(*ptep)) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
return ptep_clear_flush(vma, addr, ptep);
} else {
return ptep_get_and_clear(vma->vm_mm, addr, ptep);
}

> huge_ptep_clear_flush())? They return the pte and do the TLBI.

huge_ptep_modify_prot_start -

if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
if (pte_user_exec(READ_ONCE(*ptep)) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
return huge_ptep_clear_flush(vma, addr, ptep);
} else {
return huge_ptep_get_and_clear(vma->vm_mm, addr, ptep);
}

pte_user_exec(READ_ONCE(*ptep) should identify an user exec mapping even though
ptep represents a cont PTE/PMD huge page ? OR should huge_ptep_get() helper be
used instead ? Regardless, using [huge_]ptep_clear_flush() here seems better.

2022-11-11 22:48:09

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: errata: Workaround possible Cortex-A715 [ESR|FAR]_ELx corruption

On Thu, Nov 10, 2022 at 08:45:07AM +0530, Anshuman Khandual wrote:
> On 11/10/22 00:48, Catalin Marinas wrote:
> > On Thu, Oct 27, 2022 at 08:09:15AM +0530, Anshuman Khandual wrote:
> >> +#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
> >> +static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma,
> >> + unsigned long addr,
> >> + pte_t *ptep)
> >> +{
> >> + pte_t pte = ptep_get_and_clear(vma->vm_mm, addr, ptep);
> >>
> >> + if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> >> + /*
> >> + * Break-before-make (BBM) is required for all user space mappings
> >> + * when the permission changes from executable to non-executable
> >> + * in cases where cpu is affected with errata #2645198.
> >> + */
> >> + if (pte_user_exec(pte) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
> >> + __flush_tlb_range(vma, addr, addr + PAGE_SIZE, PAGE_SIZE, false, 3);
> >
> > Why not flush_tlb_page() here?
> >
> > But more importantly, can we not use ptep_clear_flush() instead (and
>
> Something like ...
>
> ptep_modify_prot_start -
>
> if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> if (pte_user_exec(READ_ONCE(*ptep)) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
> return ptep_clear_flush(vma, addr, ptep);
> } else {
> return ptep_get_and_clear(vma->vm_mm, addr, ptep);
> }

Yes, this should work but avoid the 'else' when you have a return, so
something like:

if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
cpus_have_const_cap(ARM64_WORKAROUND_2645198) &&
pte_user_exec(READ_ONCE(*ptep)))
return ptep_clear_flush(vma, addr, ptep);

return ptep_get_and_clear(vma->vm_mm, addr, ptep);


> > huge_ptep_clear_flush())? They return the pte and do the TLBI.
>
> huge_ptep_modify_prot_start -
>
> if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> if (pte_user_exec(READ_ONCE(*ptep)) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
> return huge_ptep_clear_flush(vma, addr, ptep);
> } else {
> return huge_ptep_get_and_clear(vma->vm_mm, addr, ptep);
> }
>
> pte_user_exec(READ_ONCE(*ptep) should identify an user exec mapping even though
> ptep represents a cont PTE/PMD huge page ? OR should huge_ptep_get() helper be
> used instead ?

This should work as a shortcut. The contiguous ptes should all be the
same, so it's sufficient to check one of them.

--
Catalin

2022-11-12 14:40:33

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: errata: Workaround possible Cortex-A715 [ESR|FAR]_ELx corruption



On 11/12/22 04:06, Catalin Marinas wrote:
> On Thu, Nov 10, 2022 at 08:45:07AM +0530, Anshuman Khandual wrote:
>> On 11/10/22 00:48, Catalin Marinas wrote:
>>> On Thu, Oct 27, 2022 at 08:09:15AM +0530, Anshuman Khandual wrote:
>>>> +#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
>>>> +static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma,
>>>> + unsigned long addr,
>>>> + pte_t *ptep)
>>>> +{
>>>> + pte_t pte = ptep_get_and_clear(vma->vm_mm, addr, ptep);
>>>>
>>>> + if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
>>>> + /*
>>>> + * Break-before-make (BBM) is required for all user space mappings
>>>> + * when the permission changes from executable to non-executable
>>>> + * in cases where cpu is affected with errata #2645198.
>>>> + */
>>>> + if (pte_user_exec(pte) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
>>>> + __flush_tlb_range(vma, addr, addr + PAGE_SIZE, PAGE_SIZE, false, 3);
>>>
>>> Why not flush_tlb_page() here?
>>>
>>> But more importantly, can we not use ptep_clear_flush() instead (and
>>
>> Something like ...
>>
>> ptep_modify_prot_start -
>>
>> if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
>> if (pte_user_exec(READ_ONCE(*ptep)) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
>> return ptep_clear_flush(vma, addr, ptep);
>> } else {
>> return ptep_get_and_clear(vma->vm_mm, addr, ptep);
>> }
>
> Yes, this should work but avoid the 'else' when you have a return, so
> something like:
>
> if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
> cpus_have_const_cap(ARM64_WORKAROUND_2645198) &&
> pte_user_exec(READ_ONCE(*ptep)))
> return ptep_clear_flush(vma, addr, ptep);
>
> return ptep_get_and_clear(vma->vm_mm, addr, ptep);

Right, realized that later.

>
>
>>> huge_ptep_clear_flush())? They return the pte and do the TLBI.
>>
>> huge_ptep_modify_prot_start -
>>
>> if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
>> if (pte_user_exec(READ_ONCE(*ptep)) && cpus_have_const_cap(ARM64_WORKAROUND_2645198))
>> return huge_ptep_clear_flush(vma, addr, ptep);
>> } else {
>> return huge_ptep_get_and_clear(vma->vm_mm, addr, ptep);
>> }
>>
>> pte_user_exec(READ_ONCE(*ptep) should identify an user exec mapping even though
>> ptep represents a cont PTE/PMD huge page ? OR should huge_ptep_get() helper be
>> used instead ?
>
> This should work as a shortcut. The contiguous ptes should all be the
> same, so it's sufficient to check one of them.

Sure, will read the first one.