[V2,2/2] arm64: errata: Workaround possible Cortex-A715 [ESR|FAR]_ELx corruption

Message ID 20221113012645.190301-3-anshuman.khandual@arm.com
State New
Headers
Series arm64: errata: Workaround Cortex-A715 errata #2645198 |

Commit Message

Anshuman Khandual Nov. 13, 2022, 1:26 a.m. UTC
  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 <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/arm64/silicon-errata.rst |  2 ++
 arch/arm64/Kconfig                     | 16 ++++++++++++++++
 arch/arm64/include/asm/hugetlb.h       |  9 +++++++++
 arch/arm64/include/asm/pgtable.h       |  9 +++++++++
 arch/arm64/kernel/cpu_errata.c         |  7 +++++++
 arch/arm64/mm/hugetlbpage.c            | 21 +++++++++++++++++++++
 arch/arm64/mm/mmu.c                    | 21 +++++++++++++++++++++
 arch/arm64/tools/cpucaps               |  1 +
 8 files changed, 86 insertions(+)
  

Comments

Will Deacon Nov. 15, 2022, 1:38 p.m. UTC | #1
On Sun, Nov 13, 2022 at 06:56:45AM +0530, Anshuman Khandual wrote:
> 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 <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  Documentation/arm64/silicon-errata.rst |  2 ++
>  arch/arm64/Kconfig                     | 16 ++++++++++++++++
>  arch/arm64/include/asm/hugetlb.h       |  9 +++++++++
>  arch/arm64/include/asm/pgtable.h       |  9 +++++++++
>  arch/arm64/kernel/cpu_errata.c         |  7 +++++++
>  arch/arm64/mm/hugetlbpage.c            | 21 +++++++++++++++++++++
>  arch/arm64/mm/mmu.c                    | 21 +++++++++++++++++++++
>  arch/arm64/tools/cpucaps               |  1 +
>  8 files changed, 86 insertions(+)

[...]

> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 9a7c38965154..c1fb0ce1473c 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1702,3 +1702,24 @@ static int __init prevent_bootmem_remove_init(void)
>  }
>  early_initcall(prevent_bootmem_remove_init);
>  #endif
> +
> +pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
> +{
> +	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> +		pte_t pte = READ_ONCE(*ptep);
> +		/*
> +		 * 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))
> +			return ptep_clear_flush(vma, addr, ptep);
> +	}
> +	return ptep_get_and_clear(vma->vm_mm, addr, ptep);
> +}
> +
> +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);
> +}

So these are really similar to the generic copies and, in looking at
change_pte_range(), it appears that we already invalidate the TLB, it just
happens _after_ writing the new version.

So with your change, I think we end up invalidating twice. Can we instead
change the generic code to invalidate the TLB before writing the new entry?

Will
  
Will Deacon Nov. 15, 2022, 1:42 p.m. UTC | #2
On Tue, Nov 15, 2022 at 01:38:54PM +0000, Will Deacon wrote:
> On Sun, Nov 13, 2022 at 06:56:45AM +0530, Anshuman Khandual wrote:
> > 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 <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-doc@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > ---
> >  Documentation/arm64/silicon-errata.rst |  2 ++
> >  arch/arm64/Kconfig                     | 16 ++++++++++++++++
> >  arch/arm64/include/asm/hugetlb.h       |  9 +++++++++
> >  arch/arm64/include/asm/pgtable.h       |  9 +++++++++
> >  arch/arm64/kernel/cpu_errata.c         |  7 +++++++
> >  arch/arm64/mm/hugetlbpage.c            | 21 +++++++++++++++++++++
> >  arch/arm64/mm/mmu.c                    | 21 +++++++++++++++++++++
> >  arch/arm64/tools/cpucaps               |  1 +
> >  8 files changed, 86 insertions(+)
> 
> [...]
> 
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index 9a7c38965154..c1fb0ce1473c 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -1702,3 +1702,24 @@ static int __init prevent_bootmem_remove_init(void)
> >  }
> >  early_initcall(prevent_bootmem_remove_init);
> >  #endif
> > +
> > +pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
> > +{
> > +	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> > +		pte_t pte = READ_ONCE(*ptep);
> > +		/*
> > +		 * 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))
> > +			return ptep_clear_flush(vma, addr, ptep);
> > +	}
> > +	return ptep_get_and_clear(vma->vm_mm, addr, ptep);
> > +}
> > +
> > +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);
> > +}
> 
> So these are really similar to the generic copies and, in looking at
> change_pte_range(), it appears that we already invalidate the TLB, it just
> happens _after_ writing the new version.
> 
> So with your change, I think we end up invalidating twice. Can we instead
> change the generic code to invalidate the TLB before writing the new entry?

Bah, scratch that, the invalidations are all batched, aren't they?

It just seems silly that we have to add all this code just to do a TLB
invalidation.

Will
  
Catalin Marinas Nov. 15, 2022, 2:02 p.m. UTC | #3
On Sun, Nov 13, 2022 at 06:56:45AM +0530, Anshuman Khandual wrote:
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index 35e9a468d13e..6552947ca7fa 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -559,3 +559,24 @@ 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)
> +{
> +	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> +		pte_t pte = READ_ONCE(*ptep);

Not sure whether the generated code would be any different but we should
probably add the check for the CPU capability in the 'if' condition
above, before the READ_ONCE (which expands to some asm volatile):

	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
	    cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
		pte_t pte = ...
		...
	}

> +		/*
> +		 * 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))
> +			return huge_ptep_clear_flush(vma, addr, ptep);
> +	}
> +	return huge_ptep_get_and_clear(vma->vm_mm, addr, ptep);
> +}
> +
> +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/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 9a7c38965154..c1fb0ce1473c 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1702,3 +1702,24 @@ static int __init prevent_bootmem_remove_init(void)
>  }
>  early_initcall(prevent_bootmem_remove_init);
>  #endif
> +
> +pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
> +{
> +	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> +		pte_t pte = READ_ONCE(*ptep);
> +		/*
> +		 * 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))
> +			return ptep_clear_flush(vma, addr, ptep);
> +	}

Same here.

> +	return ptep_get_and_clear(vma->vm_mm, addr, ptep);
> +}
> +
> +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);
> +}
  
Anshuman Khandual Nov. 16, 2022, 4:42 a.m. UTC | #4
On 11/15/22 19:32, Catalin Marinas wrote:
> On Sun, Nov 13, 2022 at 06:56:45AM +0530, Anshuman Khandual wrote:
>> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
>> index 35e9a468d13e..6552947ca7fa 100644
>> --- a/arch/arm64/mm/hugetlbpage.c
>> +++ b/arch/arm64/mm/hugetlbpage.c
>> @@ -559,3 +559,24 @@ 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)
>> +{
>> +	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
>> +		pte_t pte = READ_ONCE(*ptep);
> 
> Not sure whether the generated code would be any different but we should
> probably add the check for the CPU capability in the 'if' condition
> above, before the READ_ONCE (which expands to some asm volatile):

Sure, will do.

> 
> 	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
> 	    cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
> 		pte_t pte = ...
> 		...
> 	}

The local variable 'pte_t pte' can be dropped as well.

> 
>> +		/*
>> +		 * 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))
>> +			return huge_ptep_clear_flush(vma, addr, ptep);
>> +	}
>> +	return huge_ptep_get_and_clear(vma->vm_mm, addr, ptep);
>> +}
>> +
>> +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/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 9a7c38965154..c1fb0ce1473c 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1702,3 +1702,24 @@ static int __init prevent_bootmem_remove_init(void)
>>  }
>>  early_initcall(prevent_bootmem_remove_init);
>>  #endif
>> +
>> +pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
>> +{
>> +	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
>> +		pte_t pte = READ_ONCE(*ptep);
>> +		/*
>> +		 * 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))
>> +			return ptep_clear_flush(vma, addr, ptep);
>> +	}
> 
> Same here.

Sure, will do.

> 
>> +	return ptep_get_and_clear(vma->vm_mm, addr, ptep);
>> +}
>> +
>> +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);
>> +}
> 
Planning to apply the following change after this patch.

diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 6552947ca7fa..cd8d96e1fa1a 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -562,14 +562,14 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
 
 pte_t huge_ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
 {
-       if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
-               pte_t pte = READ_ONCE(*ptep);
+       if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
+           cpus_have_const_cap(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))
+               if (pte_user_exec(READ_ONCE(*ptep)))
                        return huge_ptep_clear_flush(vma, addr, ptep);
        }
        return huge_ptep_get_and_clear(vma->vm_mm, addr, ptep);
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index c1fb0ce1473c..ec305ea3942c 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1705,14 +1705,14 @@ early_initcall(prevent_bootmem_remove_init);
 
 pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
 {
-       if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
-               pte_t pte = READ_ONCE(*ptep);
+       if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
+           cpus_have_const_cap(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))
+               if (pte_user_exec(READ_ONCE(*ptep)))
                        return ptep_clear_flush(vma, addr, ptep);
        }
        return ptep_get_and_clear(vma->vm_mm, addr, ptep);
  
Anshuman Khandual Nov. 16, 2022, 5:04 a.m. UTC | #5
On 11/13/22 06:56, Anshuman Khandual wrote:
> +pte_t huge_ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
> +{
> +	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> +		pte_t pte = READ_ONCE(*ptep);
> +		/*
> +		 * 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))
> +			return huge_ptep_clear_flush(vma, addr, ptep);
> +	}
> +	return huge_ptep_get_and_clear(vma->vm_mm, addr, ptep);
> +}
> +
> +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/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 9a7c38965154..c1fb0ce1473c 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1702,3 +1702,24 @@ static int __init prevent_bootmem_remove_init(void)
>  }
>  early_initcall(prevent_bootmem_remove_init);
>  #endif
> +
> +pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
> +{
> +	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> +		pte_t pte = READ_ONCE(*ptep);
> +		/*
> +		 * 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))
> +			return ptep_clear_flush(vma, addr, ptep);
> +	}
> +	return ptep_get_and_clear(vma->vm_mm, addr, ptep);
> +}
> +
> +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);
> +}

Will change this __set_pte_at() to set_pte_at() instead like the generic version, which also
makes sure that page_table_check_pte_set() gets called on the way, also making it similar to
the HugeTLB implementation huge_ptep_modify_prot_commit().
  
Anshuman Khandual Nov. 16, 2022, 5:49 a.m. UTC | #6
On 11/15/22 19:12, Will Deacon wrote:
> On Tue, Nov 15, 2022 at 01:38:54PM +0000, Will Deacon wrote:
>> On Sun, Nov 13, 2022 at 06:56:45AM +0530, Anshuman Khandual wrote:
>>> 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 <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-doc@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>>  Documentation/arm64/silicon-errata.rst |  2 ++
>>>  arch/arm64/Kconfig                     | 16 ++++++++++++++++
>>>  arch/arm64/include/asm/hugetlb.h       |  9 +++++++++
>>>  arch/arm64/include/asm/pgtable.h       |  9 +++++++++
>>>  arch/arm64/kernel/cpu_errata.c         |  7 +++++++
>>>  arch/arm64/mm/hugetlbpage.c            | 21 +++++++++++++++++++++
>>>  arch/arm64/mm/mmu.c                    | 21 +++++++++++++++++++++
>>>  arch/arm64/tools/cpucaps               |  1 +
>>>  8 files changed, 86 insertions(+)
>>
>> [...]
>>
>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>> index 9a7c38965154..c1fb0ce1473c 100644
>>> --- a/arch/arm64/mm/mmu.c
>>> +++ b/arch/arm64/mm/mmu.c
>>> @@ -1702,3 +1702,24 @@ static int __init prevent_bootmem_remove_init(void)
>>>  }
>>>  early_initcall(prevent_bootmem_remove_init);
>>>  #endif
>>> +
>>> +pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
>>> +{
>>> +	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
>>> +		pte_t pte = READ_ONCE(*ptep);
>>> +		/*
>>> +		 * 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))
>>> +			return ptep_clear_flush(vma, addr, ptep);
>>> +	}
>>> +	return ptep_get_and_clear(vma->vm_mm, addr, ptep);
>>> +}
>>> +
>>> +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);
>>> +}
>>
>> So these are really similar to the generic copies and, in looking at
>> change_pte_range(), it appears that we already invalidate the TLB, it just
>> happens _after_ writing the new version.
>>
>> So with your change, I think we end up invalidating twice. Can we instead
>> change the generic code to invalidate the TLB before writing the new entry?
> 
> Bah, scratch that, the invalidations are all batched, aren't they?

Right.

> 
> It just seems silly that we have to add all this code just to do a TLB
> invalidation.

Right, but only when affected by this errata. Otherwise it is just same as the
existing generic definitions.
  
Catalin Marinas Nov. 16, 2022, 9 a.m. UTC | #7
On Wed, Nov 16, 2022 at 10:12:34AM +0530, Anshuman Khandual wrote:
> Planning to apply the following change after this patch.
> 
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index 6552947ca7fa..cd8d96e1fa1a 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -562,14 +562,14 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
>  
>  pte_t huge_ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
>  {
> -       if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> -               pte_t pte = READ_ONCE(*ptep);
> +       if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
> +           cpus_have_const_cap(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))
> +               if (pte_user_exec(READ_ONCE(*ptep)))
>                         return huge_ptep_clear_flush(vma, addr, ptep);
>         }
>         return huge_ptep_get_and_clear(vma->vm_mm, addr, ptep);
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index c1fb0ce1473c..ec305ea3942c 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1705,14 +1705,14 @@ early_initcall(prevent_bootmem_remove_init);
>  
>  pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
>  {
> -       if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
> -               pte_t pte = READ_ONCE(*ptep);
> +       if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
> +           cpus_have_const_cap(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))
> +               if (pte_user_exec(READ_ONCE(*ptep)))
>                         return ptep_clear_flush(vma, addr, ptep);
>         }
>         return ptep_get_and_clear(vma->vm_mm, addr, ptep);

It looks fine to me. Thanks.
  

Patch

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..fe76e5823d91 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -1096,6 +1096,15 @@  static inline bool pud_sect_supported(void)
 }
 
 
+#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
+#define ptep_modify_prot_start ptep_modify_prot_start
+extern pte_t ptep_modify_prot_start(struct vm_area_struct *vma,
+				    unsigned long addr, pte_t *ptep);
+
+#define ptep_modify_prot_commit ptep_modify_prot_commit
+extern void ptep_modify_prot_commit(struct vm_area_struct *vma,
+				    unsigned long addr, pte_t *ptep,
+				    pte_t old_pte, pte_t new_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..6552947ca7fa 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -559,3 +559,24 @@  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)
+{
+	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
+		pte_t pte = READ_ONCE(*ptep);
+		/*
+		 * 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))
+			return huge_ptep_clear_flush(vma, addr, ptep);
+	}
+	return huge_ptep_get_and_clear(vma->vm_mm, addr, ptep);
+}
+
+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/mm/mmu.c b/arch/arm64/mm/mmu.c
index 9a7c38965154..c1fb0ce1473c 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1702,3 +1702,24 @@  static int __init prevent_bootmem_remove_init(void)
 }
 early_initcall(prevent_bootmem_remove_init);
 #endif
+
+pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
+{
+	if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198)) {
+		pte_t pte = READ_ONCE(*ptep);
+		/*
+		 * 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))
+			return ptep_clear_flush(vma, addr, ptep);
+	}
+	return ptep_get_and_clear(vma->vm_mm, addr, ptep);
+}
+
+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);
+}
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