Right, it is the pte table walk that is blowing up.
I just noted that 2.6 lacks a tophys() call in its table walk
so I removed that one(there is one more tophys call but I don't think
it should be removed).
Try this addon patch:
no difference
OK, thinking a bit more, this part should not be executed as
copy_tofrom_user executes in kernel space.
Any chance you can stick a HW breakpoint on FixupDAR?
Perhaps there is something different with kernel
virtual address to phys address?
A simple topys() works in 2.4, but perhaps not in 2.6?
this is the part of interest:
FixupDAR: /* Entry point for dcbx workaround. */
/* fetch instruction from memory. */
mfspr r10, SPRN_SRR0
andis. r11, r10, 0x8000
tophys (r11, r10)
beq- 139b /* Branch if user space address */
140: lwz r11,0(r11)
Probably better to walk the kernel page table too. Does this
make a difference(needs the tophys() patch I posted earlier):
From 862dda30c3d3d3bedcc605e8520626408a26891c Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <redacted>
Date: Sat, 17 Oct 2009 13:54:03 +0200
Subject: [PATCH] 8xx: Walk the page table for kernel addresses too.
---
arch/powerpc/kernel/head_8xx.S | 25 ++++++++++++-------------
1 files changed, 12 insertions(+), 13 deletions(-)
@@ -532,28 +532,27 @@ DARFixed:/* Return from dcbx instruction bug workaround, r10 holds value of DAR*bydecodingtheregistersusedbythedcbxinstructionandaddingthem.*DARissettothecalculatedaddressandr10alsoholdstheEAonexit.*/-#define NO_SELF_MODIFYING_CODE /* define if you don't want to use self modifying code */-nop/*Afewnopstomakethemodified_instr:spacebelowcachelinealigned*/-nop-139:/*fetchinstructionfromuserspacememory*/+/*defineifyoudon't want to use self modifying code */+#define NO_SELF_MODIFYING_CODE+FixupDAR:/*Entrypointfordcbxworkaround.*/+/*fetchinstructionfrommemory.*/+mfsprr10,SPRN_SRR0DO_8xx_CPU6(0x3780,r3)mtsprSPRN_MD_EPN,r10mfsprr11,SPRN_M_TWB/*Getlevel1tableentryaddress*/-lwzr11,0(r11)/*Getthelevel1entry*/+cmplwicr0,r11,0x0800+blt-3f/*Branchifuserspace*/+lisr11,swapper_pg_dir@h+orir11,r11,swapper_pg_dir@l+rlwimir11,r11,0,2,19+3:lwzr11,0(r11)/*Getthelevel1entry*/DO_8xx_CPU6(0x3b80,r3)mtsprSPRN_MD_TWC,r11/*Loadptetablebaseaddress*/mfsprr11,SPRN_MD_TWC/*....andgetthepteaddress*/lwzr11,0(r11)/*Getthepte*//*concatphysicalpageaddress(r11)andpageoffset(r10)*/rlwimir11,r10,0,20,31-b140f-FixupDAR:/*Entrypointfordcbxworkaround.*/-/*fetchinstructionfrommemory.*/-mfsprr10,SPRN_SRR0-andis.r11,r10,0x8000-tophys (r11,r10)-beq-139b/*Branchifuserspaceaddress*/-140:lwzr11,0(r11)+lwzr11,0(r11)/*Checkifitreallyisadcbxinstruction.*//*dcbtanddcbtstdoesnotgenerateDTLBMisses/Errors,*noneedtoincludethemhere*/--
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2009-10-26 22:47:45
Probably better to walk the kernel page table too. Does this
make a difference(needs the tophys() patch I posted earlier):
This whole thing would be a -lot- easier to do from C code. Why ? Simply
because you could just use get_user() to load the instruction rather
than doing this page table walking in asm, which is simpler, faster, and
more fool proof (ok, you do pay the price of a kernel entry/exit
instead, but I still believe that code simplicity and maintainability
wins here).
Ben.
quoted hunk
quoted
From 862dda30c3d3d3bedcc605e8520626408a26891c Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <redacted>
Date: Sat, 17 Oct 2009 13:54:03 +0200
Subject: [PATCH] 8xx: Walk the page table for kernel addresses too.
---
arch/powerpc/kernel/head_8xx.S | 25 ++++++++++++-------------
1 files changed, 12 insertions(+), 13 deletions(-)
@@ -532,28 +532,27 @@ DARFixed:/* Return from dcbx instruction bug workaround, r10 holds value of DAR*bydecodingtheregistersusedbythedcbxinstructionandaddingthem.*DARissettothecalculatedaddressandr10alsoholdstheEAonexit.*/-#define NO_SELF_MODIFYING_CODE /* define if you don't want to use self modifying code */-nop/*Afewnopstomakethemodified_instr:spacebelowcachelinealigned*/-nop-139:/*fetchinstructionfromuserspacememory*/+/*defineifyoudon't want to use self modifying code */+#define NO_SELF_MODIFYING_CODE+FixupDAR:/*Entrypointfordcbxworkaround.*/+/*fetchinstructionfrommemory.*/+mfsprr10,SPRN_SRR0DO_8xx_CPU6(0x3780,r3)mtsprSPRN_MD_EPN,r10mfsprr11,SPRN_M_TWB/*Getlevel1tableentryaddress*/-lwzr11,0(r11)/*Getthelevel1entry*/+cmplwicr0,r11,0x0800+blt-3f/*Branchifuserspace*/+lisr11,swapper_pg_dir@h+orir11,r11,swapper_pg_dir@l+rlwimir11,r11,0,2,19+3:lwzr11,0(r11)/*Getthelevel1entry*/DO_8xx_CPU6(0x3b80,r3)mtsprSPRN_MD_TWC,r11/*Loadptetablebaseaddress*/mfsprr11,SPRN_MD_TWC/*....andgetthepteaddress*/lwzr11,0(r11)/*Getthepte*//*concatphysicalpageaddress(r11)andpageoffset(r10)*/rlwimir11,r10,0,20,31-b140f-FixupDAR:/*Entrypointfordcbxworkaround.*/-/*fetchinstructionfrommemory.*/-mfsprr10,SPRN_SRR0-andis.r11,r10,0x8000-tophys (r11,r10)-beq-139b/*Branchifuserspaceaddress*/-140:lwzr11,0(r11)+lwzr11,0(r11)/*Checkifitreallyisadcbxinstruction.*//*dcbtanddcbtstdoesnotgenerateDTLBMisses/Errors,*noneedtoincludethemhere*/--
From: Dan Malek <hidden> Date: 2009-10-26 23:24:40
On Oct 26, 2009, at 3:47 PM, Benjamin Herrenschmidt wrote:
This whole thing would be a -lot- easier to do from C code. Why ?
Simply
because you could just use get_user() to load the instruction rather
than doing this page table walking in asm,
Just be careful the get_user() doesn't regenerate the same
translation error you are trying to fix by being here......
It is nice doing things in C code, but you have to be aware
of the environment and the side effects when in this kind
of exception state.
Thanks.
-- Dan
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2009-10-27 00:01:15
On Mon, 2009-10-26 at 16:26 -0700, Dan Malek wrote:
Just be careful the get_user() doesn't regenerate the same
translation error you are trying to fix by being here......
It shouldn't since it will always come up with a proper DAR but
you may want to double check before hand that your instruction
address you are loading from is -not- your marker value for bad DAR.
It is nice doing things in C code, but you have to be aware
of the environment and the side effects when in this kind
Benjamin Herrenschmidt [off-list ref] wrote on 27/10/2009 01:00:53:
On Mon, 2009-10-26 at 16:26 -0700, Dan Malek wrote:
quoted
Just be careful the get_user() doesn't regenerate the same
translation error you are trying to fix by being here......
yes, I had some problems with this initially but managed to work around that.
I noticed another problem though, I got multiple TLB errors for the same
address when I did it in C. Noticed by just printk:ing every hit
for a dcbX insn in do_page_fault. I can't explain it, but it seems
like when moving to C you have to execute a rfi insn and that might somehow
restart the dcbX insn before moving on to the page fault routine(or something
totally different)
It shouldn't since it will always come up with a proper DAR but
you may want to double check before hand that your instruction
address you are loading from is -not- your marker value for bad DAR.
hmm, I check that the insn really is a dcbX insn, but not that the address is
!= 0x00f0. Don't see how it could be as if something is wrong with
the insn address you get ITLB error instead of a DTLB error.
Anyhow, things seems stalled as I haven't heard from Scott or Rex for a while.
If this isn't working now, I really don't know what is wrong and need
some debugging help.
quoted
It is nice doing things in C code, but you have to be aware
of the environment and the side effects when in this kind
From: Scott Wood <hidden> Date: 2009-10-27 15:58:41
On Tue, Oct 27, 2009 at 10:16:17AM +0100, Joakim Tjernlund wrote:
Benjamin Herrenschmidt [off-list ref] wrote on 27/10/2009 01:00:53:
quoted
On Mon, 2009-10-26 at 16:26 -0700, Dan Malek wrote:
quoted
Just be careful the get_user() doesn't regenerate the same
translation error you are trying to fix by being here......
yes, I had some problems with this initially but managed to work around
that. I noticed another problem though, I got multiple TLB errors for the
same address when I did it in C. Noticed by just printk:ing every hit for
a dcbX insn in do_page_fault. I can't explain it, but it seems like when
moving to C you have to execute a rfi insn and that might somehow restart
the dcbX insn before moving on to the page fault routine(or something
totally different)
The rfi should be to other kernel code -- there is no way that it should be
restarting the dcbX (other than when trying to turn a TLB miss into a TLB
error). Can you post the C version, maybe we can see what's going wrong?
Is the empty TLB entry from the miss getting invalidated in the dcbX fixup
case?
quoted
It shouldn't since it will always come up with a proper DAR but
you may want to double check before hand that your instruction
address you are loading from is -not- your marker value for bad DAR.
hmm, I check that the insn really is a dcbX insn, but not that the address is
!= 0x00f0. Don't see how it could be as if something is wrong with
the insn address you get ITLB error instead of a DTLB error.
I'm guessing he meant the data address you're loading.
Anyhow, things seems stalled as I haven't heard from Scott or Rex for a
while. If this isn't working now, I really don't know what is wrong and
need some debugging help.
I'll test the latest version, but I have some scheduling latency. :-)
-Scott
Scott Wood [off-list ref] wrote on 27/10/2009 16:58:41:
On Tue, Oct 27, 2009 at 10:16:17AM +0100, Joakim Tjernlund wrote:
quoted
Benjamin Herrenschmidt [off-list ref] wrote on 27/10/2009 01:00:53:
quoted
On Mon, 2009-10-26 at 16:26 -0700, Dan Malek wrote:
quoted
Just be careful the get_user() doesn't regenerate the same
translation error you are trying to fix by being here......
yes, I had some problems with this initially but managed to work around
that. I noticed another problem though, I got multiple TLB errors for the
same address when I did it in C. Noticed by just printk:ing every hit for
a dcbX insn in do_page_fault. I can't explain it, but it seems like when
moving to C you have to execute a rfi insn and that might somehow restart
the dcbX insn before moving on to the page fault routine(or something
totally different)
The rfi should be to other kernel code -- there is no way that it should be
restarting the dcbX (other than when trying to turn a TLB miss into a TLB
error). Can you post the C version, maybe we can see what's going wrong?
I don't have it for 2.6 and I never did cleanup up my 2.4 version.
Your best bet is to look at one of the earlier patches such
as:
Add some debug code to do_page_fault
and fix the remaining bits.
Is the empty TLB entry from the miss getting invalidated in the dcbX fixup
case?
Yes, in all cases it was invalidated.
quoted
quoted
It shouldn't since it will always come up with a proper DAR but
you may want to double check before hand that your instruction
address you are loading from is -not- your marker value for bad DAR.
hmm, I check that the insn really is a dcbX insn, but not that the address is
!= 0x00f0. Don't see how it could be as if something is wrong with
the insn address you get ITLB error instead of a DTLB error.
I'm guessing he meant the data address you're loading.
Hopefully and I am already looking at the OP code to make sure it is
a dcbX insn.
quoted
Anyhow, things seems stalled as I haven't heard from Scott or Rex for a
while. If this isn't working now, I really don't know what is wrong and
need some debugging help.
I'll test the latest version, but I have some scheduling latency. :-)
Right, it is the pte table walk that is blowing up.
I just noted that 2.6 lacks a tophys() call in its table walk
so I removed that one(there is one more tophys call but I don't think
it should be removed).
Try this addon patch:
no difference
quoted
OK, thinking a bit more, this part should not be executed as
copy_tofrom_user executes in kernel space.
Any chance you can stick a HW breakpoint on FixupDAR?
Perhaps there is something different with kernel
virtual address to phys address?
A simple topys() works in 2.4, but perhaps not in 2.6?
this is the part of interest:
FixupDAR: /* Entry point for dcbx workaround. */
/* fetch instruction from memory. */
mfspr r10, SPRN_SRR0
andis. r11, r10, 0x8000
tophys (r11, r10)
beq- 139b /* Branch if user space address */
140: lwz r11,0(r11)
Probably better to walk the kernel page table too. Does this
make a difference(needs the tophys() patch I posted earlier):
Right, it is the pte table walk that is blowing up.
I just noted that 2.6 lacks a tophys() call in its table walk
so I removed that one(there is one more tophys call but I don't think
it should be removed).
Try this addon patch:
no difference
quoted
OK, thinking a bit more, this part should not be executed as
copy_tofrom_user executes in kernel space.
Any chance you can stick a HW breakpoint on FixupDAR?
Perhaps there is something different with kernel
virtual address to phys address?
A simple topys() works in 2.4, but perhaps not in 2.6?
this is the part of interest:
FixupDAR: /* Entry point for dcbx workaround. */
/* fetch instruction from memory. */
mfspr r10, SPRN_SRR0
andis. r11, r10, 0x8000
tophys (r11, r10)
beq- 139b /* Branch if user space address */
140: lwz r11,0(r11)
Probably better to walk the kernel page table too. Does this
make a difference(needs the tophys() patch I posted earlier):
After applying by hand (whitespace damage), I get this and a bunch more:
OK, please send your diff to head_8xx.S. Maybe I can spot an
error, otherwise you will have to set a hw BP on fixDAR and step
through it.
Jocke
From: Scott Wood <hidden> Date: 2009-10-30 17:37:28
On Fri, Oct 30, 2009 at 12:16:07PM -0500, Scott Wood wrote:
On Sat, Oct 17, 2009 at 02:01:38PM +0200, Joakim Tjernlund wrote:
quoted
+ mfspr r10, SPRN_SRR0
DO_8xx_CPU6(0x3780, r3)
mtspr SPRN_MD_EPN, r10
mfspr r11, SPRN_M_TWB /* Get level 1 table entry address */
- lwz r11, 0(r11) /* Get the level 1 entry */
+ cmplwi cr0, r11, 0x0800
+ blt- 3f /* Branch if user space */
+ lis r11, swapper_pg_dir@h
+ ori r11, r11, swapper_pg_dir@l
+ rlwimi r11, r11, 0, 2, 19
That rlwimi is a no-op -- I think you meant to use a different register
here?
quoted
+3: lwz r11, 0(r11) /* Get the level 1 entry */
DO_8xx_CPU6(0x3b80, r3)
mtspr SPRN_MD_TWC, r11 /* Load pte table base address */
mfspr r11, SPRN_MD_TWC /* ....and get the pte address */
lwz r11, 0(r11) /* Get the pte */
/* concat physical page address(r11) and page offset(r10) */
rlwimi r11, r10, 0, 20, 31
But r10 here contains SRR0 from above, and this is a data TLB error.
Never mind that last one, forgot that you'd be wanting to load the
instruction. :-P
But the rlwimi is what's causing the machine checks. I replaced it with:
rlwinm r11, r11, 0, 0x3ffff000
rlwimi r11, r10, 22, 0xffc
and things seem to work. You could probably replace the rlwinm by
subtracting PAGE_OFFSET from swapper_pg_dir instead.
-Scott
Scott Wood [off-list ref] wrote on 30/10/2009 18:37:49:
On Fri, Oct 30, 2009 at 12:16:07PM -0500, Scott Wood wrote:
quoted
On Sat, Oct 17, 2009 at 02:01:38PM +0200, Joakim Tjernlund wrote:
quoted
+ mfspr r10, SPRN_SRR0
DO_8xx_CPU6(0x3780, r3)
mtspr SPRN_MD_EPN, r10
mfspr r11, SPRN_M_TWB /* Get level 1 table entry address */
- lwz r11, 0(r11) /* Get the level 1 entry */
+ cmplwi cr0, r11, 0x0800
+ blt- 3f /* Branch if user space */
+ lis r11, swapper_pg_dir@h
+ ori r11, r11, swapper_pg_dir@l
+ rlwimi r11, r11, 0, 2, 19
That rlwimi is a no-op -- I think you meant to use a different register
here?
quoted
+3: lwz r11, 0(r11) /* Get the level 1 entry */
DO_8xx_CPU6(0x3b80, r3)
mtspr SPRN_MD_TWC, r11 /* Load pte table base address */
mfspr r11, SPRN_MD_TWC /* ....and get the pte address */
lwz r11, 0(r11) /* Get the pte */
/* concat physical page address(r11) and page offset(r10) */
rlwimi r11, r10, 0, 20, 31
But r10 here contains SRR0 from above, and this is a data TLB error.
Never mind that last one, forgot that you'd be wanting to load the
instruction. :-P
But the rlwimi is what's causing the machine checks. I replaced it with:
Yes, I see now that it is wrong.
rlwinm r11, r11, 0, 0x3ffff000
rlwimi r11, r10, 22, 0xffc
and things seem to work. You could probably replace the rlwinm by
subtracting PAGE_OFFSET from swapper_pg_dir instead.
Would you mind produce a proper path on top of my series? I am blind
here so I can only guess what will work or not.
Then Rex can give it some beating and we can push this to Ben
Scott Wood [off-list ref] wrote on 30/10/2009 18:37:49:
On Fri, Oct 30, 2009 at 12:16:07PM -0500, Scott Wood wrote:
quoted
On Sat, Oct 17, 2009 at 02:01:38PM +0200, Joakim Tjernlund wrote:
quoted
+ mfspr r10, SPRN_SRR0
DO_8xx_CPU6(0x3780, r3)
mtspr SPRN_MD_EPN, r10
mfspr r11, SPRN_M_TWB /* Get level 1 table entry address */
- lwz r11, 0(r11) /* Get the level 1 entry */
+ cmplwi cr0, r11, 0x0800
+ blt- 3f /* Branch if user space */
+ lis r11, swapper_pg_dir@h
+ ori r11, r11, swapper_pg_dir@l
+ rlwimi r11, r11, 0, 2, 19
That rlwimi is a no-op -- I think you meant to use a different register
here?
quoted
+3: lwz r11, 0(r11) /* Get the level 1 entry */
DO_8xx_CPU6(0x3b80, r3)
mtspr SPRN_MD_TWC, r11 /* Load pte table base address */
mfspr r11, SPRN_MD_TWC /* ....and get the pte address */
lwz r11, 0(r11) /* Get the pte */
/* concat physical page address(r11) and page offset(r10) */
rlwimi r11, r10, 0, 20, 31
But r10 here contains SRR0 from above, and this is a data TLB error.
Never mind that last one, forgot that you'd be wanting to load the
instruction. :-P
But the rlwimi is what's causing the machine checks. I replaced it with:
rlwinm r11, r11, 0, 0x3ffff000
rlwimi r11, r10, 22, 0xffc
and things seem to work. You could probably replace the rlwinm by
subtracting PAGE_OFFSET from swapper_pg_dir instead.
Just guessing here, do you mean:
lis r11, (swapper_pg_dir-PAGE_OFFSET)@h
ori r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
rlwimi r11, r10, 22, 0xffc
or
lis r11, swapper_pg_dir@h
ori r11, r11, swapper_pg_dir@l
subis r11, r11 PAGE_OFFSET
rlwimi r11, r10, 22, 0xffc
Jocke
Scott Wood [off-list ref] wrote on 03/11/2009 17:59:30:
Joakim Tjernlund wrote:
quoted
quoted
and things seem to work. You could probably replace the rlwinm by
subtracting PAGE_OFFSET from swapper_pg_dir instead.
Just guessing here, do you mean:
lis r11, (swapper_pg_dir-PAGE_OFFSET)@h
ori r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
rlwimi r11, r10, 22, 0xffc
or
lis r11, swapper_pg_dir@h
ori r11, r11, swapper_pg_dir@l
subis r11, r11 PAGE_OFFSET
rlwimi r11, r10, 22, 0xffc
The former.
OK, I will regenerate the patch series with the
lis r11, (swapper_pg_dir-PAGE_OFFSET)@h
ori r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
rlwimi r11, r10, 22, 0xffc
fix.
Have you already confirmed that this works too?
Jocke