arm: proc-v7: pc phy addresses before disable MMU

PATCH New Per Forlin 5 participants 12 messages Sep 24, 2011 – Oct 3, 2011 Arm (32-bit)

Share & tools

Per Forlin

author

2011-09-24 08:02 UTC #0 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

PC runs virtual addresses when calling cpu_v7_reset(). The MMU is switched off and "mov pc, r0" sets pc back to virtual addresses even though the MMU is switched off. This will result in a crash if the pipeline delay after MMU disable is one instruction. To prevent this set PC to physical addresses before disabling the MMU.

Signed-off-by Per Fransson <[email protected]>

Signed-off-by Per Forlin <[email protected]>

---
 arch/arm/mm/proc-v7.S |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/mm/proc-v7.S +1 -0

index 9049c07..f26e831 100644
@@ -64,6 +64,7 @@ ENDPROC(cpu_v7_proc_fin)
  */
 	.align	5
 ENTRY(cpu_v7_reset)
+	sub	pc, pc, #PAGE_OFFSET+4		@ go to physical addresses
 	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
 	bic	r1, r1, #0x1			@ ...............m
  THUMB(	bic	r1, r1, #1 << 30 )		@ SCTLR.TE (Thumb exceptions)

Signature · 1 line

1.6.3.3

Russell King - ARM Linux

2011-09-24 08:06 UTC #1 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

Quoted 6 lines · Per Forlin

PC runs virtual addresses when calling cpu_v7_reset(). The MMU is switched off and "mov pc, r0" sets pc back to virtual addresses even though the MMU is switched off. This will result in a crash if the pipeline delay after MMU disable is one instruction. To prevent this set PC to physical addresses before disabling the MMU.

This is broken - it assumes physical memory starts at zero. That's not the case for many platforms.

Mika Westerberg

2011-09-24 08:13 UTC #2 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

In reply to Per Forlin

Quoted 18 lines · Per Forlin

PC runs virtual addresses when calling cpu_v7_reset(). The MMU is switched off and "mov pc, r0" sets pc back to virtual addresses even though the MMU is switched off. This will result in a crash if the pipeline delay after MMU disable is one instruction. To prevent this set PC to physical addresses before disabling the MMU.
Signed-off-by Per Fransson <[email protected]>
Signed-off-by Per Forlin <[email protected]>
---
 arch/arm/mm/proc-v7.S |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/mm/proc-v7.S +0 -0
index 9049c07..f26e831 100644

arch/arm/mm/proc-v7.S ENDPROC(cpu_v7_proc_fin) · line 67

Quoted 4 lines

arch/arm/mm/proc-v7.S +1 -0

@@ -64,6 +64,7 @@ ENDPROC(cpu_v7_proc_fin)
  */
 	.align	5
 ENTRY(cpu_v7_reset)
+	sub	pc, pc, #PAGE_OFFSET+4		@ go to physical addresses

This only works on machines where PHYS_OFFSET is zero.

IIRC, there is a P<->V offset calculated at boot time. Maybe you could use that instead here?

Quoted 11 lines

arch/arm/mm/proc-v7.S +0 -0

 	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
 	bic	r1, r1, #0x1			@ ...............m
  THUMB(	bic	r1, r1, #1 << 30 )		@ SCTLR.TE (Thumb exceptions)

Signature · 7 lines

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

Per Förlin

author

2011-09-24 09:32 UTC #3 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

Quoted 14 lines · Mika Westerberg

Quoted 13 lines · Per Forlin
PC runs virtual addresses when calling cpu_v7_reset(). The MMU is switched off and "mov pc, r0" sets pc back to virtual addresses even though the MMU is switched off. This will result in a crash if the pipeline delay after MMU disable is one instruction. To prevent this set PC to physical addresses before disabling the MMU.
Signed-off-by Per Fransson <[email protected]>
Signed-off-by Per Forlin <[email protected]>
---
 arch/arm/mm/proc-v7.S |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Quoted 12 lines

arch/arm/mm/proc-v7.S ENDPROC(cpu_v7_proc_fin) · line 67
Quoted 9 lines
arch/arm/mm/proc-v7.S +1 -0
index 9049c07..f26e831 100644
@@ -64,6 +64,7 @@ ENDPROC(cpu_v7_proc_fin)
  */
 	.align	5
 ENTRY(cpu_v7_reset)
+	sub	pc, pc, #PAGE_OFFSET+4		@ go to physical addresses
This only works on machines where PHYS_OFFSET is zero.

You are right! Russell was very quick to point out the same thing.

Quoted 3 lines

IIRC, there is a P<->V offset calculated at boot time. Maybe you could use that instead here?

I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S). I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?

Regards,
Per

Will Deacon

2011-09-24 11:00 UTC #4 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

Quoted 3 lines · Per F?rlin

I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S). I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?

Wouldn't we just be better off passing a physical address to cpu_v7_reset instead?

Will

Per Förlin

author

2011-09-24 12:55 UTC #5 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

Quoted 8 lines · Will Deacon

Quoted 3 lines · Per F?rlin
I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S). I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?
Wouldn't we just be better off passing a physical address to cpu_v7_reset instead?

Good point! I'm fine with that.

/Per

Russell King - ARM Linux

2011-09-24 13:02 UTC #6 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

Quoted 10 lines · Per F?rlin

Quoted 8 lines · Will Deacon
Quoted 3 lines · Per F?rlin
I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S). I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?
Wouldn't we just be better off passing a physical address to cpu_v7_reset instead?
Good point! I'm fine with that.

Alternatively, call it using the phys address.

Per Förlin

author

2011-09-30 05:37 UTC #7 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

Quoted 7 lines · Russell King - ARM Linux

Quoted 6 lines · Per F?rlin
Quoted 5 lines · Will Deacon
Quoted 3 lines · Per F?rlin
I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S). I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?

Quoted 6 lines

Quoted 4 lines
Quoted 3 lines
Wouldn't we just be better off passing a physical address to cpu_v7_reset instead?
Good point! I'm fine with that.
Alternatively, call it using the phys address.

I changed the code to use a label instead of the ARM specific instruction offset +4. A few more lines but the intention is that it should work with both ARM and THUMB. I'm in favour of Russell's idea of setting pc to physical before calling cpu_reset().

      /* Go to physical addresses to be ready for MMU disable */
      asm("ADR r1, pc_phy_here \n\t"
          "sub r1, %0 \n\t"
          "mov pc, r1 \n\t"
          "pc_phy_here: \n\t"
          : :"r" (PAGE_OFFSET - PHYS_OFFSET) : "r1", "cc");

Where is the best place for this code?
Should this be run for all ARM cpu_reset()?

I haven't found a nice way to add to cpu_reset() yet. This is what I have so far.
1. Redirect cpu_reset to c-function __cpu_reset_phy() that sets the pc to phy and then calls __cpu_reset() (the original one)
1.1 May add ifdefs in __cpu_reset_phy() to only do the virt to phy transition for some explicit ARM versions before calling __cpu_reset()
2. Rename cpu_v7_reset() to __cpu_v7_reset(). Only redirect cpu_v7_reset() to the c-function that goes to phy and then calls __cpu_v7_reset().
3. Add a new in parameter "phys_offset" to cpu_reset(). Modify only those CPU_reset() that needs this parameter. Redirect cpu_reset(x,y) to CPU_reset(x) or CPU_reset(x,y) based on ARM version.

Regards,
Per

Will Deacon

2011-09-30 08:34 UTC #8 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

Hi Per,

Quoted 13 lines · Per F?rlin

Quoted 12 lines · Russell King - ARM Linux
Quoted 10 lines · Per F?rlin
Quoted 8 lines · Will Deacon
Quoted 3 lines · Per F?rlin
I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S). I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?
Wouldn't we just be better off passing a physical address to cpu_v7_reset instead?
Good point! I'm fine with that.

Quoted 8 lines

Quoted 1 line
Alternatively, call it using the phys address.
I changed the code to use a label instead of the ARM specific instruction offset +4. A few more lines but the intention is that it should work with both ARM and THUMB. I'm in favour of Russell's idea of setting pc to physical before calling cpu_reset().
      /* Go to physical addresses to be ready for MMU disable */
      asm("ADR r1, pc_phy_here \n\t"
          "sub r1, %0 \n\t"
          "mov pc, r1 \n\t"
          "pc_phy_here: \n\t"
          : :"r" (PAGE_OFFSET - PHYS_OFFSET) : "r1", "cc");

I'm not sure why all of this is necessary. Take a look at my kexec patches here:

http://www.linux-arm.org/git?p=linux-2.6-wd.git;a=shortlog;h=refs/heads/kexec-mmu-off

The interesting bit is __arm_machine_reset in kernel/process.c

Will

Per Förlin

author

2011-09-30 09:34 UTC #9 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

Quoted 25 lines · Will Deacon

Hi Per,
Quoted 13 lines · Per F?rlin
Quoted 12 lines · Russell King - ARM Linux
Quoted 10 lines · Per F?rlin
Quoted 8 lines · Will Deacon
Quoted 3 lines · Per F?rlin
I am trying to figure out how I can get the value of PHYS_OFFSET in assembler code (proc-v7.S). I guess I could use the value calculated in arch/arm/kernel.head.S but wouldn't that make this patch dependent on CONFIG_ARM_PATCH_PHYS_VIRT?
Wouldn't we just be better off passing a physical address to cpu_v7_reset instead?
Good point! I'm fine with that.
Quoted 8 lines
Quoted 1 line
Alternatively, call it using the phys address.
I changed the code to use a label instead of the ARM specific instruction offset +4. A few more lines but the intention is that it should work with both ARM and THUMB. I'm in favour of Russell's idea of setting pc to physical before calling cpu_reset().
      /* Go to physical addresses to be ready for MMU disable */
      asm("ADR r1, pc_phy_here \n\t"
          "sub r1, %0 \n\t"
          "mov pc, r1 \n\t"
          "pc_phy_here: \n\t"
          : :"r" (PAGE_OFFSET - PHYS_OFFSET) : "r1", "cc");

Quoted 7 lines

I'm not sure why all of this is necessary. Take a look at my kexec patches here:
http://www.linux-arm.org/git?p=linux-2.6-wd.git;a=shortlog;h=refs/heads/kexec-mmu-off
The interesting bit is __arm_machine_reset in kernel/process.c

/* Switch to the identity mapping. */
phys_reset = (phys_reset_t)virt_to_phys(cpu_reset); phys_reset(reset_args->reset_code_phys);
Yes this is definitely how it should be done. My mind was stuck in the assembler code.

If I get this right the bug I run into is already fixed by your kexec patches. I could simply use your patches?

Thanks,
Per

Will Deacon

2011-09-30 09:38 UTC #10 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

Quoted 4 lines · Per F?rlin

Quoted 3 lines · Will Deacon
I'm not sure why all of this is necessary. Take a look at my kexec patches here:

Quoted 8 lines

Quoted 4 lines
http://www.linux-arm.org/git?p=linux-2.6-wd.git;a=shortlog;h=refs/heads/kexec-mmu-off
The interesting bit is __arm_machine_reset in kernel/process.c
/* Switch to the identity mapping. */
phys_reset = (phys_reset_t)virt_to_phys(cpu_reset); phys_reset(reset_args->reset_code_phys);
Yes this is definitely how it should be done. My mind was stuck in the assembler code.

Easy mistake to make!

Quoted 1 line

If I get this right the bug I run into is already fixed by your kexec patches. I could simply use your patches?

Sure, I reworked them the other day so I'll post them to the list again soon. There are still some open problems to solve but the reset code is more-or-less there.

Will

Dave Martin

2011-10-03 14:32 UTC #11 lore raw

PATCH 1/1 arm: proc-v7: pc phy addresses before disable MMU

In reply to Per Förlin

Quoted 27 lines · Per F?rlin

Quoted 14 lines · Mika Westerberg
Quoted 13 lines · Per Forlin
PC runs virtual addresses when calling cpu_v7_reset(). The MMU is switched off and "mov pc, r0" sets pc back to virtual addresses even though the MMU is switched off. This will result in a crash if the pipeline delay after MMU disable is one instruction. To prevent this set PC to physical addresses before disabling the MMU.
Signed-off-by Per Fransson <[email protected]>
Signed-off-by Per Forlin <[email protected]>
---
 arch/arm/mm/proc-v7.S |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Quoted 12 lines
arch/arm/mm/proc-v7.S ENDPROC(cpu_v7_proc_fin) · line 67
Quoted 9 lines
arch/arm/mm/proc-v7.S +1 -0
index 9049c07..f26e831 100644
@@ -64,6 +64,7 @@ ENDPROC(cpu_v7_proc_fin)
  */
 	.align	5
 ENTRY(cpu_v7_reset)
+	sub	pc, pc, #PAGE_OFFSET+4		@ go to physical addresses
This only works on machines where PHYS_OFFSET is zero.

Quoted 1 line

You are right! Russell was very quick to point out the same thing.

For reference, I'll also point out that except for "mov pc,<Rm>", almost no instruction involving an explicit reference to the pc will safely do what you want in Thumb-2 kernels, due to the variable instruction size.

Cheers
---Dave