| Message ID | 20090817134136.GB8710@zod.rchland.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Aug 17, 2009 at 09:41:36AM -0400, Josh Boyer wrote: >The ptrace POKETEXT interface allows a process to modify the text pages of >a child process being ptraced, usually to insert breakpoints via trap >instructions. The kernel eventually calls copy_to_user_page, which in turn >calls __flush_icache_range to invalidate the icache lines for the child >process. > >However, this function does not work on 44x due to the icache being virtually >indexed. This was noticed by a breakpoint being triggered after it had been >cleared by ltrace on a 440EPx board. The convenient solution is to do a >flash invalidate of the icache in the __flush_icache_range function. > >Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com> > >--- > >diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S >index 15f28e0..c9805a4 100644 >--- a/arch/powerpc/kernel/misc_32.S >+++ b/arch/powerpc/kernel/misc_32.S >@@ -346,6 +346,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE) > 2: icbi 0,r6 > addi r6,r6,L1_CACHE_BYTES > bdnz 2b >+#ifdef CONFIG_44x >+ iccci r0, r0 >+#endif Olof pointed out that we could probably do the iccci before the icbi loop and just skip that loop entirely on 44x. This is most certainly valid, but at this particular moment I don't have time to try and reproduce the issue with an alternative fix and I wanted to get _something_ out there to fix the issue. I suck for that, I know. josh
On Mon, 2009-08-17 at 12:07 -0400, Josh Boyer wrote: > > Olof pointed out that we could probably do the iccci before the icbi loop and > just skip that loop entirely on 44x. This is most certainly valid, but at > this particular moment I don't have time to try and reproduce the issue with > an alternative fix and I wanted to get _something_ out there to fix the issue. > > I suck for that, I know. Well, I can massage your patch if you want. The fact is, the icbi loop and iccci are definitely redundant :-) Cheers, Ben.
On Tue, Aug 18, 2009 at 07:46:28AM +1000, Benjamin Herrenschmidt wrote: >On Mon, 2009-08-17 at 12:07 -0400, Josh Boyer wrote: >> >> Olof pointed out that we could probably do the iccci before the icbi loop and >> just skip that loop entirely on 44x. This is most certainly valid, but at >> this particular moment I don't have time to try and reproduce the issue with >> an alternative fix and I wanted to get _something_ out there to fix the issue. >> >> I suck for that, I know. > >Well, I can massage your patch if you want. The fact is, the icbi loop >and iccci are definitely redundant :-) You can if you'd like. My biggest concern is getting time to recreate. I think I'll have time later in the week if you'd like to wait until then. I simply didn't want to send out a patch that I wasn't sure fixed the issue. josh
On Mon, 2009-08-17 at 20:16 -0400, Josh Boyer wrote: > > You can if you'd like. My biggest concern is getting time to > recreate. I > think I'll have time later in the week if you'd like to wait until > then. > I simply didn't want to send out a patch that I wasn't sure fixed the > issue. > That's ok. It's a bug fix so it's less constrained by the upcoming merge window and we can send it back to -stable later. Cheers, Ben.
On Mon, 2009-08-17 at 09:41 -0400, Josh Boyer wrote: > The ptrace POKETEXT interface allows a process to modify the text pages of > a child process being ptraced, usually to insert breakpoints via trap > instructions. The kernel eventually calls copy_to_user_page, which in turn > calls __flush_icache_range to invalidate the icache lines for the child > process. > > However, this function does not work on 44x due to the icache being virtually > indexed. This was noticed by a breakpoint being triggered after it had been > cleared by ltrace on a 440EPx board. The convenient solution is to do a > flash invalidate of the icache in the __flush_icache_range function. > > Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com> I've put it in -test for now, but I'd like you to respin when you get a chance with: - removing the icbi loop in the 440 case - adding a nice fat comment explaining why next to the iccci instruction itself so we don't be tempted to remove it. Cheers, Ben. > --- > > diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S > index 15f28e0..c9805a4 100644 > --- a/arch/powerpc/kernel/misc_32.S > +++ b/arch/powerpc/kernel/misc_32.S > @@ -346,6 +346,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE) > 2: icbi 0,r6 > addi r6,r6,L1_CACHE_BYTES > bdnz 2b > +#ifdef CONFIG_44x > + iccci r0, r0 > +#endif > sync /* additional sync needed on g4 */ > isync > blr
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S index 15f28e0..c9805a4 100644 --- a/arch/powerpc/kernel/misc_32.S +++ b/arch/powerpc/kernel/misc_32.S @@ -346,6 +346,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE) 2: icbi 0,r6 addi r6,r6,L1_CACHE_BYTES bdnz 2b +#ifdef CONFIG_44x + iccci r0, r0 +#endif sync /* additional sync needed on g4 */ isync blr
The ptrace POKETEXT interface allows a process to modify the text pages of a child process being ptraced, usually to insert breakpoints via trap instructions. The kernel eventually calls copy_to_user_page, which in turn calls __flush_icache_range to invalidate the icache lines for the child process. However, this function does not work on 44x due to the icache being virtually indexed. This was noticed by a breakpoint being triggered after it had been cleared by ltrace on a 440EPx board. The convenient solution is to do a flash invalidate of the icache in the __flush_icache_range function. Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com> ---