Skip to contentSkip to conversation

Fix ptrace software breakpoints

PATCH New Will Deacon 5 participants 15 messages Jul 15, 2010 – Aug 4, 2010 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

cover Fix ptrace software breakpoints

When using GDB on a quad-core Cortex-A9 (Versatile Express) board, software breakpoints do not work if the inferior is scheduled onto a different CPU from the debugger.

When GDB changes the code of another context via the ptrace POKETEXT mechanism, the I-cache must be invalidated before the inferior is allowed to resume. If a copy-on-write is triggered by the copy_to_user_page function, the new page mappings must be used by the inferior in order to pick up the new instructions.

This patch series addresses this problem by:

(a) Providing a workaround for a known TLB issue on some revisions of the Cortex-A9.
(b) Performing correct I-cache invalidation in the flush_ptrace_access code.

All feedback/comments/tested-bys welcome.

Cc Rob Clark <[email protected]>
Cc Catalin Marinas <[email protected]>

Will Deacon (2):
ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a
faulty ASID
ARM: flush_ptrace_access: invalidate all I-caches

 arch/arm/Kconfig                |   12 ++++++++++++
 arch/arm/include/asm/tlbflush.h |    8 ++++++++
 arch/arm/mm/flush.c             |    4 ++--
 3 files changed, 22 insertions(+), 2 deletions(-)
#1 lore raw

PATCH 1/2 ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID

On versions of the Cortex-A9 prior to r2p0, performing TLB invalidations by ASID match can result in the incorrect ASID being broadcast to other CPUs. As a consequence of this, the targetted TLB entries are not invalidated across the system.

This workaround changes the TLB flushing routines to invalidate entries regardless of the ASID.

---
 arch/arm/Kconfig                |   12 ++++++++++++
 arch/arm/include/asm/tlbflush.h |    8 ++++++++
 2 files changed, 20 insertions(+), 0 deletions(-)
arch/arm/Kconfig +12 -0
index 98922f7..4824fb4 100644
@@ -1027,6 +1027,18 @@ config PL310_ERRATA_588369
 	   is not correctly implemented in PL310 as clean lines are not
 	   invalidated as a result of these operations. Note that this errata
 	   uses Texas Instrument's secure monitor api.
+
+config ARM_ERRATA_720789
+	bool "ARM errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID"
+	depends on CPU_V7 && SMP
+	help
+	  This option enables the workaround for the 720789 Cortex-A9 (prior to
+	  r2p0) erratum. A faulty ASID can be sent to the other CPUs for the
+	  broadcasted CP15 TLB maintenance operations TLBIASIDIS and TLBIMVAIS.
+	  As a consequence of this erratum, some TLB entries which should be
+	  invalidated are not, resulting in an incoherency in the system page
+	  tables. The workaround changes the TLB flushing routines to invalidate
+	  entries regardless of the ASID.
 endmenu
 
 source "arch/arm/common/Kconfig"
arch/arm/include/asm/tlbflush.h +8 -0
index bd863d8..33b546a 100644
@@ -378,7 +378,11 @@ static inline void local_flush_tlb_mm(struct mm_struct *mm)
 	if (tlb_flag(TLB_V6_I_ASID))
 		asm("mcr p15, 0, %0, c8, c5, 2" : : "r" (asid) : "cc");
 	if (tlb_flag(TLB_V7_UIS_ASID))
+#ifdef CONFIG_ARM_ERRATA_720789
+		asm("mcr p15, 0, %0, c8, c3, 0" : : "r" (zero) : "cc");
+#else
 		asm("mcr p15, 0, %0, c8, c3, 2" : : "r" (asid) : "cc");
+#endif
 
 	if (tlb_flag(TLB_BTB)) {
 		/* flush the branch target cache */
@@ -424,7 +428,11 @@ local_flush_tlb_page(struct vm_area_struct *vma, unsigned long uaddr)
 	if (tlb_flag(TLB_V6_I_PAGE))
 		asm("mcr p15, 0, %0, c8, c5, 1" : : "r" (uaddr) : "cc");
 	if (tlb_flag(TLB_V7_UIS_PAGE))
+#ifdef CONFIG_ARM_ERRATA_720789
+		asm("mcr p15, 0, %0, c8, c3, 3" : : "r" (uaddr & PAGE_MASK) : "cc");
+#else
 		asm("mcr p15, 0, %0, c8, c3, 1" : : "r" (uaddr) : "cc");
+#endif
 
 	if (tlb_flag(TLB_BTB)) {
 		/* flush the branch target cache */
Signature · 1 line
1.6.3.3
#2 lore raw

PATCH 2/2 ARM: flush_ptrace_access: invalidate all I-caches

copy_to_user_page can be used by access_process_vm to write to an executable page of a process using a mapping acquired by kmap. For systems with I-cache aliasing, flushing the I-cache using the Kernel mapping may leave stale data in the I-cache if the user mapping is of a different colour.

This patch replaces the coherent_kern_range call in flush_ptrace_access with a D-cache flush followed by a system-wide I-cache invalidation. This is required on all systems where the size of a way in the I-cache is larger than PAGE_SIZE.

---
 arch/arm/mm/flush.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mm/flush.c +2 -2
index c6844cb..45896a9 100644
@@ -120,8 +120,8 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 
 	/* VIPT non-aliasing cache */
 	if (vma->vm_flags & VM_EXEC) {
-		unsigned long addr = (unsigned long)kaddr;
-		__cpuc_coherent_kern_range(addr, addr + len);
+		__cpuc_flush_dcache_area(kaddr, len);
+		__flush_icache_all();
 #ifdef CONFIG_SMP
 		if (cache_ops_need_broadcast())
 			smp_call_function(flush_ptrace_access_other,
Signature · 1 line
1.6.3.3
#3 lore raw

PATCH 2/2 ARM: flush_ptrace_access: invalidate all I-caches

Quoted 22 lines · Will Deacon

copy_to_user_page can be used by access_process_vm to write to an executable page of a process using a mapping acquired by kmap. For systems with I-cache aliasing, flushing the I-cache using the Kernel mapping may leave stale data in the I-cache if the user mapping is of a different colour.

This patch replaces the coherent_kern_range call in flush_ptrace_access with a D-cache flush followed by a system-wide I-cache invalidation. This is required on all systems where the size of a way in the I-cache is larger than PAGE_SIZE.

---
 arch/arm/mm/flush.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mm/flush.c +0 -0
index c6844cb..45896a9 100644

arch/arm/mm/flush.c void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, · line 124

Quoted 7 lines
arch/arm/mm/flush.c +2 -2
@@ -120,8 +120,8 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 
 	/* VIPT non-aliasing cache */
 	if (vma->vm_flags & VM_EXEC) {
-		unsigned long addr = (unsigned long)kaddr;
-		__cpuc_coherent_kern_range(addr, addr + len);
+		__cpuc_flush_dcache_area(kaddr, len);
+		__flush_icache_all();

NAK.

If we have aliases in the I-cache, there's probably more places that need to be fixed - and in any case I think the VIPT aliasing case should be used in that instance.

This code is for non-aliasing D and I caches, and works as follows.

1. We flush the data out of the D cache, line by line, on at least the local CPU (and optionally the other CPUs.) Since we disable preemption, we will own the cache lines.

2. We invalidate the I cache, line by line, on at least local CPU (and optionally the other CPUs.)

3. If the I-cache invalidate wasn't broadcast, we flush the entire I-cache on the other CPUs.

So, what CPUs report themselves as having VIPT non-aliasing caches but actually have an aliasing I-cache?

#4 lore raw

PATCH 2/2 ARM: flush_ptrace_access: invalidate all I-caches

Hi Russell,

Quoted 19 lines

arch/arm/mm/flush.c void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, · line 124

Quoted 12 lines
arch/arm/mm/flush.c +2 -2
index c6844cb..45896a9 100644
@@ -120,8 +120,8 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 
 	/* VIPT non-aliasing cache */
 	if (vma->vm_flags & VM_EXEC) {
-		unsigned long addr = (unsigned long)kaddr;
-		__cpuc_coherent_kern_range(addr, addr + len);
+		__cpuc_flush_dcache_area(kaddr, len);
+		__flush_icache_all();

NAK.

If we have aliases in the I-cache, there's probably more places that need to be fixed - and in any case I think the VIPT aliasing case should be used in that instance.

Quoted 8 lines

This code is for non-aliasing D and I caches, and works as follows.

1. We flush the data out of the D cache, line by line, on at least the local CPU (and optionally the other CPUs.) Since we disable preemption, we will own the cache lines.

2. We invalidate the I cache, line by line, on at least local CPU (and optionally the other CPUs.)

(1) and (2) are done in __cpuc_coherent_kern_range. The problem is that the (2) part doesn't deal with aliases and only invalidates for the given MVA.

Quoted 2 lines

3. If the I-cache invalidate wasn't broadcast, we flush the entire I-cache on the other CPUs.

Yep.

Quoted 2 lines

So, what CPUs report themselves as having VIPT non-aliasing caches but actually have an aliasing I-cache?

As far as I understand, all ARM cores have aliasing I-caches if it's a VIPT cache with way size greater than the page size. The D-cache has additional logic for handling aliasing which is what the VIPT non-aliasing characteristics refer to.

Will

#5 lore raw

PATCH 2/2 ARM: flush_ptrace_access: invalidate all I-caches

Quoted 8 lines · Will Deacon
Quoted 2 lines · Russell King

So, what CPUs report themselves as having VIPT non-aliasing caches but actually have an aliasing I-cache?

As far as I understand, all ARM cores have aliasing I-caches if it's a VIPT cache with way size greater than the page size. The D-cache has additional logic for handling aliasing which is what the VIPT non-aliasing characteristics refer to.

Just to confirm what Will said (and I checked this in the past with our lead architect) - if the CTR reports VIPT I-cache, it is a true VIPT I-cache with potential aliases if the way size is greater than the page size (e.g. on vexpress, C-A9 has 32K I-cache and 4 ways).

On the D side, the VIPT on ARMv7 is always non-aliasing (e.g. it behaves like PIPT).

Anyway, it's not that bad since most places where this matters use __flush_icache_all(). This includes my patches for cache maintenance via set_pte_at().

Signature · 1 line
Catalin
#6 lore raw

PATCH 2/2 ARM: flush_ptrace_access: invalidate all I-caches

In reply to Will Deacon

Russell,

Quoted 15 lines
Quoted 15 lines

arch/arm/mm/flush.c void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, · line 124

Quoted 12 lines
arch/arm/mm/flush.c +2 -2
index c6844cb..45896a9 100644
@@ -120,8 +120,8 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 
 	/* VIPT non-aliasing cache */
 	if (vma->vm_flags & VM_EXEC) {
-		unsigned long addr = (unsigned long)kaddr;
-		__cpuc_coherent_kern_range(addr, addr + len);
+		__cpuc_flush_dcache_area(kaddr, len);
+		__flush_icache_all();

NAK.

Quoted 3 lines
Quoted 3 lines

If we have aliases in the I-cache, there's probably more places that need to be fixed - and in any case I think the VIPT aliasing case should be used in that instance.

So I looked into this problem a bit more in order to confirm it's an aliasing issue.

GDB sets breakpoints inside ld-linux.so so that it can control the loading of shared libraries and insert any pending breakpoints.

If the inferior has a PID of 3233, we can see the dynamic loader living at 0x40000000:

root at will-lucid-nfs:~/tmp# cat /proc/3233/maps
...
40000000-40016000 r-xp 00000000 00:0b 3493469 /lib/ld-2.11.1.so
40016000-40017000 rw-p 00000000 00:00 0
4001d000-4001f000 rw-p 00015000 00:0b 3493469 /lib/ld-2.11.1.so

GDB tries to insert a breakpoint on __dl_debug_state, at address 0x40002590. Some examples of when this works (I hacked flush_ptrace_access to print uaddr and dst):

[  110.910000] writing to user address 0x40002590 using Kernel mapping 0xbfe6c590
...
[  147.530000] writing to user address 0x40002590 using Kernel mapping 0xbfe7e590

Now, this can fail with a SIGILL:

(gdb) start
Temporary breakpoint 1 at 0x83b4: file test.c, line 7. Starting program: /root/tmp/test

Program received signal SIGILL, Illegal instruction. 0x40002590 in ?? () from /lib/ld-linux.so.3

This happens when GDB has removed a breakpoint instruction, but it still exists in the I-cache because the mapping used to invalidate the relevant way was of a different colour. In dmesg, we see:

[  150.780000] writing to user address 0x40002590 using Kernel mapping 0xbfe8b590

The I-cache in use is a 4-way 32KB cache, so we need 13 bits to represent an index into an 8KB way and a byte index into the resulting line. Since the top bit is not invariant to address translation, we have two colours.

Looking at the GDB examples:

0x40002590 and 0xbfe6c590 have bit 12 == 0, so they are the same colour. 0x40002590 and 0xbfe7e590 have bit 12 == 0, so they are the same colour. 0x40002590 and 0xbfe8b590 have bit 12 == 0 and bit 12 == 1 respectively.

In the last case, the wrong colour is invalidated in the I-cache and a SIGILL occurs. This problem can be fixed either by invalidating the whole I-cache (which is what is done in the patch I posted) or we could invalidate the correct colour, but this would mean implementing something like kmap_coherent from the MIPS world.

It would be good to get this fixed. Do you still object to my original patch or can I submit it to the patch system if I get some tested-bys?

Thanks,

Will

#7 lore raw

PATCH 2/2 ARM: flush_ptrace_access: invalidate all I-caches

In reply to Russell King - ARM Linux

Hi Russell,

Quoted 26 lines

Subject: Re: [PATCH 2/2] ARM: flush_ptrace_access: invalidate all I-caches

Quoted 23 lines · Will Deacon

copy_to_user_page can be used by access_process_vm to write to an executable page of a process using a mapping acquired by kmap. For systems with I-cache aliasing, flushing the I-cache using the Kernel mapping may leave stale data in the I-cache if the user mapping is of a different colour.

This patch replaces the coherent_kern_range call in flush_ptrace_access with a D-cache flush followed by a system-wide I-cache invalidation. This is required on all systems where the size of a way in the I-cache is larger than PAGE_SIZE.

---
 arch/arm/mm/flush.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mm/flush.c +0 -0
index c6844cb..45896a9 100644
@@ -120,8 +120,8 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 
Quoted 9 lines

arch/arm/mm/flush.c void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, · line 124

Quoted 6 lines
arch/arm/mm/flush.c +2 -2
 	/* VIPT non-aliasing cache */
 	if (vma->vm_flags & VM_EXEC) {
-		unsigned long addr = (unsigned long)kaddr;
-		__cpuc_coherent_kern_range(addr, addr + len);
+		__cpuc_flush_dcache_area(kaddr, len);
+		__flush_icache_all();

NAK.

How about something like this instead?:

arch/arm/mm/flush.c +14 -4
index c6844cb..6b03043 100644
@@ -20,10 +20,9 @@
 
 #include "mm.h"
 
-#ifdef CONFIG_CPU_CACHE_VIPT
-
 #define ALIAS_FLUSH_START      0xffff4000
 
+#ifdef CONFIG_CPU_CACHE_VIPT
 static void flush_pfn_alias(unsigned long pfn, unsigned long vaddr)
 {
        unsigned long to = ALIAS_FLUSH_START + (CACHE_COLOUR(vaddr) << PAGE_SHIFT);
@@ -93,6 +92,18 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig
 #define flush_pfn_alias(pfn,vaddr)     do { } while (0)
 #endif
 
+static void flush_icache_alias(unsigned long pfn, unsigned long vaddr, unsigned long len)
+{
+       unsigned long colour = CACHE_COLOUR(vaddr);
+       unsigned long offset = vaddr & (PAGE_SIZE - 1);
+       unsigned long to;
+
+       set_pte_ext(TOP_PTE(ALIAS_FLUSH_START) + colour, pfn_pte(pfn, PAGE_KERNEL), 0);
+       to = ALIAS_FLUSH_START + (colour << PAGE_SHIFT) + offset;
+       flush_tlb_kernel_page(to);
+       flush_icache_range(to, to + len);
+}
+
 #ifdef CONFIG_SMP
 static void flush_ptrace_access_other(void *args)
 {
@@ -120,8 +131,7 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 
        /* VIPT non-aliasing cache */
        if (vma->vm_flags & VM_EXEC) {
-               unsigned long addr = (unsigned long)kaddr;
-               __cpuc_coherent_kern_range(addr, addr + len);
+               flush_icache_alias(page_to_pfn(page), uaddr, len);
 #ifdef CONFIG_SMP
                if (cache_ops_need_broadcast())
                        smp_call_function(flush_ptrace_access_other,

That way we only use __flush_icache_all for non-aliasing D-caches in the case that we have to broadcast the flushing using IPI.

Will

#8 lore raw

PATCH 1/2 ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID

In reply to Will Deacon

Will,

Quoted 9 lines

-----Original Message-----
From: linux-arm-kernel-bounces at lists.infradead.org [mailto:linux-arm- kernel-bounces at lists.infradead.org] On Behalf Of Will Deacon
Sent: Thursday, July 15, 2010 9:24 PM
To: linux-arm-kernel at lists.infradead.org
Cc: Will Deacon
Subject: [PATCH 1/2] ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID

Quoted 9 lines

On versions of the Cortex-A9 prior to r2p0, performing TLB invalidations by
ASID match can result in the incorrect ASID being broadcast to other CPUs. As a consequence of this, the targetted TLB entries are not invalidated across the system.

This workaround changes the TLB flushing routines to invalidate entries regardless of the ASID.

Just a curious question. How costly is this ?

Quoted 76 lines
---
 arch/arm/Kconfig                |   12 ++++++++++++
 arch/arm/include/asm/tlbflush.h |    8 ++++++++
 2 files changed, 20 insertions(+), 0 deletions(-)
arch/arm/Kconfig +0 -0
index 98922f7..4824fb4 100644
@@ -1027,6 +1027,18 @@ config PL310_ERRATA_588369
 	   is not correctly implemented in PL310 as clean lines are not
 	   invalidated as a result of these operations. Note that this

errata

arch/arm/Kconfig +3 -0
 	   uses Texas Instrument's secure monitor api.
+
+config ARM_ERRATA_720789
+	bool "ARM errata: TLBIASIDIS and TLBIMVAIS operations can broadcast

a faulty ASID"

arch/arm/Kconfig +3 -0
+	depends on CPU_V7 && SMP
+	help
+	  This option enables the workaround for the 720789 Cortex-A9 (prior

to

arch/arm/Kconfig +2 -0
+	  r2p0) erratum. A faulty ASID can be sent to the other CPUs for the
+	  broadcasted CP15 TLB maintenance operations TLBIASIDIS and

TLBIMVAIS.

arch/arm/Kconfig +2 -0
+	  As a consequence of this erratum, some TLB entries which should be
+	  invalidated are not, resulting in an incoherency in the system

page

arch/arm/Kconfig +1 -0
+	  tables. The workaround changes the TLB flushing routines to

invalidate

arch/arm/Kconfig +1 -0
+	  entries regardless of the ASID.
 endmenu
 
 source "arch/arm/common/Kconfig"
arch/arm/include/asm/tlbflush.h +4 -0
index bd863d8..33b546a 100644
@@ -378,7 +378,11 @@ static inline void local_flush_tlb_mm(struct
 mm_struct *mm)
 	if (tlb_flag(TLB_V6_I_ASID))
 		asm("mcr p15, 0, %0, c8, c5, 2" : : "r" (asid) : "cc");
 	if (tlb_flag(TLB_V7_UIS_ASID))
+#ifdef CONFIG_ARM_ERRATA_720789
+		asm("mcr p15, 0, %0, c8, c3, 0" : : "r" (zero) : "cc");
+#else
 		asm("mcr p15, 0, %0, c8, c3, 2" : : "r" (asid) : "cc");
+#endif
 
 	if (tlb_flag(TLB_BTB)) {
arch/arm/include/asm/tlbflush.h +4 -0
 		/* flush the branch target cache */
@@ -424,7 +428,11 @@ local_flush_tlb_page(struct vm_area_struct *vma,
 unsigned long uaddr)
 	if (tlb_flag(TLB_V6_I_PAGE))
 		asm("mcr p15, 0, %0, c8, c5, 1" : : "r" (uaddr) : "cc");
 	if (tlb_flag(TLB_V7_UIS_PAGE))
+#ifdef CONFIG_ARM_ERRATA_720789
+		asm("mcr p15, 0, %0, c8, c3, 3" : : "r" (uaddr & PAGE_MASK) :
 "cc");
+#else
 		asm("mcr p15, 0, %0, c8, c3, 1" : : "r" (uaddr) : "cc");
+#endif
 
arch/arm/include/asm/tlbflush.h +0 -1
 	if (tlb_flag(TLB_BTB)) {
 		/* flush the branch target cache */
--

1.6.3.3

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

#9 lore raw

PATCH 1/2 ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID

Hi Santosh,

Quoted 9 lines
Quoted 9 lines

Subject: [PATCH 1/2] ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID

On versions of the Cortex-A9 prior to r2p0, performing TLB invalidations by
ASID match can result in the incorrect ASID being broadcast to other CPUs. As a consequence of this, the targetted TLB entries are not invalidated across the system.

Quoted 4 lines
Quoted 3 lines

This workaround changes the TLB flushing routines to invalidate entries regardless of the ASID.

Just a curious question. How costly is this ?

I think the cost really depends on whether or not the evicted TLB entries are available in the D-cache. If they are, then a TLB miss occurring because of this workaround will hit in the data cache and the overhead will be small. Missing in the D-cache is certainly going to add an overhead, but that depends on your memory system.

Of course, this workaround should only be enabled on platforms that suffer from the erratum, where it's worth sacrificing a few cycles in order to get a working virtual memory system!

Will

#10 lore raw

PATCH 1/2 ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID

Quoted 37 lines

-----Original Message-----
From: Will Deacon [mailto:will.deacon at arm.com]
Sent: Thursday, July 15, 2010 10:02 PM
To: Shilimkar, Santosh; linux-arm-kernel at lists.infradead.org
Subject: RE: [PATCH 1/2] ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID

Hi Santosh,

Quoted 1 line
Quoted 1 line

Subject: [PATCH 1/2] ARM: errata: TLBIASIDIS and TLBIMVAIS operations

can

Quoted 3 lines
Quoted 3 lines

broadcast a faulty ASID

On versions of the Cortex-A9 prior to r2p0, performing TLB

invalidations

Quoted 2 lines
Quoted 2 lines

by
ASID match can result in the incorrect ASID being broadcast to other

CPUs.

Quoted 1 line
Quoted 1 line

As a consequence of this, the targetted TLB entries are not

invalidated

Quoted 3 lines
Quoted 3 lines

across the system.

This workaround changes the TLB flushing routines to invalidate

entries

Quoted 3 lines
Quoted 2 lines

regardless of the ASID.

Just a curious question. How costly is this ?

I think the cost really depends on whether or not the evicted TLB entries are available in the D-cache. If they are, then a TLB miss occurring because
of this workaround will hit in the data cache and the overhead will be small.
Missing in the D-cache is certainly going to add an overhead, but that depends
on your memory system.

Quoted 4 lines

Of course, this workaround should only be enabled on platforms that suffer from the erratum, where it's worth sacrificing a few cycles in order to get
a working virtual memory system!

Thanks for quick response!!

Regards
Santosh

#11 lore raw

cover Fix ptrace software breakpoints

In reply to Will Deacon

fyi, my gdb test (git://github.com/robclark/gdb-test.git) seems to be passing reliable so far (I haven't seen it fail once yet) with these patches.. without these patches, it was triggering passing the breakpoing 90+% of the time.

I'll keep you posted if I notice any issues, but so far so good!

thx

BR,
-R

Quoted 28 lines · Will Deacon

When using GDB on a quad-core Cortex-A9 (Versatile Express) board, software breakpoints do not work if the inferior is scheduled onto a different CPU from the debugger.

When GDB changes the code of another context via the ptrace POKETEXT mechanism, the I-cache must be invalidated before the inferior is allowed to resume. If a copy-on-write is triggered by the copy_to_user_page function, the new page mappings must be used by the inferior in order to pick up the new instructions.

This patch series addresses this problem by:

(a) Providing a workaround for a known TLB issue on some revisions of the Cortex-A9.
(b) Performing correct I-cache invalidation in the flush_ptrace_access code.

All feedback/comments/tested-bys welcome.

Cc Rob Clark <[email protected]>
Cc Catalin Marinas <[email protected]>

Will Deacon (2):
ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a
faulty ASID
ARM: flush_ptrace_access: invalidate all I-caches

arch/arm/Kconfig                |   12 ++++++++++++
arch/arm/include/asm/tlbflush.h |    8 ++++++++
arch/arm/mm/flush.c             |    4 ++--
3 files changed, 22 insertions(+), 2 deletions(-)
#12 lore raw

cover Fix ptrace software breakpoints

Hi Rob,

Quoted 5 lines

fyi, my gdb test (git://github.com/robclark/gdb-test.git) seems to be passing reliable so far (I haven't seen it fail once yet) with these patches.. without these patches, it was triggering passing the breakpoing 90+% of the time.

I'll keep you posted if I notice any issues, but so far so good!

Does this mean you haven't seen it fail yet :)? If so, any chance I could have a tested-by please?

Will

#13 lore raw

cover Fix ptrace software breakpoints

In reply to Will Deacon
Quoted 24 lines · Will Deacon

When using GDB on a quad-core Cortex-A9 (Versatile Express) board, software breakpoints do not work if the inferior is scheduled onto a different CPU from the debugger.

When GDB changes the code of another context via the ptrace POKETEXT mechanism, the I-cache must be invalidated before the inferior is allowed to resume. If a copy-on-write is triggered by the copy_to_user_page function, the new page mappings must be used by the inferior in order to pick up the new instructions.

This patch series addresses this problem by:

(a) Providing a workaround for a known TLB issue on some revisions of the Cortex-A9.
(b) Performing correct I-cache invalidation in the flush_ptrace_access code.

All feedback/comments/tested-bys welcome.

Cc Rob Clark<[email protected]>
Cc Catalin Marinas<[email protected]>

Will Deacon (2):
ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a
faulty ASID
ARM: flush_ptrace_access: invalidate all I-caches

Quoted 5 lines
  arch/arm/Kconfig                |   12 ++++++++++++
  arch/arm/include/asm/tlbflush.h |    8 ++++++++
  arch/arm/mm/flush.c             |    4 ++--
  3 files changed, 22 insertions(+), 2 deletions(-)

Tested on omap4430 (cortex-a9 SMP).. with this patchset I have yet to see it fail, in regular use nor in the test case I wrote to show the problem (git://github.com/robclark/gdb-test.git). Without these patches, I'd see SIGILL and missed breakpoints 90+% of the time in regular use of gdb (basically gdb was unusable unless I booted with nosmp or disabled cache), and 100% of the time in my gdb-test.

#14 lore raw

cover Fix ptrace software breakpoints

Hi Rob,

Quoted 11 lines
Quoted 10 lines

Will Deacon (2):
ARM: errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a
faulty ASID
ARM: flush_ptrace_access: invalidate all I-caches

  arch/arm/Kconfig                |   12 ++++++++++++
  arch/arm/include/asm/tlbflush.h |    8 ++++++++
  arch/arm/mm/flush.c             |    4 ++--
  3 files changed, 22 insertions(+), 2 deletions(-)
Quoted 8 lines

Tested on omap4430 (cortex-a9 SMP).. with this patchset I have yet to see it fail, in regular use nor in the test case I wrote to show the problem (git://github.com/robclark/gdb-test.git). Without these patches, I'd see SIGILL and missed breakpoints 90+% of the time in regular use of gdb (basically gdb was unusable unless I booted with nosmp or disabled cache), and 100% of the time in my gdb-test.

Thanks for testing this. I've seen the same results on my Versatile Express board running Ubuntu Lucid.

Russell - can I submit these to the patch system please or do you have any outstanding concerns?

Cheers,

Will