[PATCH v2] Check _PAGE_RW and _PAGE_PRESENT on kernel addresses.

[PATCH v2] Check _PAGE_RW and _PAGE_PRESENT on kernel addresses.

[Scott Wood]

Previously, the TLB miss handlers assumed that pages above KERNELBASE are
always present and read/write.  This assumption is false in the case of
CONFIG_DEBUG_PAGEALLOC.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/kernel/head_32.S |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 7d73a13..0e3df1f 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -475,10 +475,10 @@ InstructionTLBMiss:
 	li	r1,_PAGE_USER|_PAGE_PRESENT /* low addresses tested as user */
 	lwz	r2,PGDIR(r2)
 	blt+	112f
+	mfspr	r2,SPRN_SRR1		/* and MSR_PR bit from SRR1 */
+	rlwimi	r1,r2,32-12,29,29	/* shift MSR_PR to _PAGE_USER posn */
 	lis	r2,swapper_pg_dir@ha	/* if kernel address, use */
 	addi	r2,r2,swapper_pg_dir@l	/* kernel page table */
-	mfspr	r1,SPRN_SRR1		/* and MSR_PR bit from SRR1 */
-	rlwinm	r1,r1,32-12,29,29	/* shift MSR_PR to _PAGE_USER posn */
 112:	tophys(r2,r2)
 	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
 	lwz	r2,0(r2)		/* get pmd entry */
@@ -549,10 +549,10 @@ DataLoadTLBMiss:
 	li	r1,_PAGE_USER|_PAGE_PRESENT /* low addresses tested as user */
 	lwz	r2,PGDIR(r2)
 	blt+	112f
+	mfspr	r2,SPRN_SRR1		/* and MSR_PR bit from SRR1 */
+	rlwimi	r1,r2,32-12,29,29	/* shift MSR_PR to _PAGE_USER posn */
 	lis	r2,swapper_pg_dir@ha	/* if kernel address, use */
 	addi	r2,r2,swapper_pg_dir@l	/* kernel page table */
-	mfspr	r1,SPRN_SRR1		/* and MSR_PR bit from SRR1 */
-	rlwinm	r1,r1,32-12,29,29	/* shift MSR_PR to _PAGE_USER posn */
 112:	tophys(r2,r2)
 	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
 	lwz	r2,0(r2)		/* get pmd entry */
@@ -621,10 +621,10 @@ DataStoreTLBMiss:
 	li	r1,_PAGE_RW|_PAGE_USER|_PAGE_PRESENT /* access flags */
 	lwz	r2,PGDIR(r2)
 	blt+	112f
+	mfspr	r2,SPRN_SRR1		/* and MSR_PR bit from SRR1 */
+	rlwimi	r1,r2,32-12,29,29	/* shift MSR_PR to _PAGE_USER posn */
 	lis	r2,swapper_pg_dir@ha	/* if kernel address, use */
 	addi	r2,r2,swapper_pg_dir@l	/* kernel page table */
-	mfspr	r1,SPRN_SRR1		/* and MSR_PR bit from SRR1 */
-	rlwinm	r1,r1,32-12,29,29	/* shift MSR_PR to _PAGE_USER posn */
 112:	tophys(r2,r2)
 	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
 	lwz	r2,0(r2)		/* get pmd entry */
-- 
1.5.3

[PATCH 2/3] pm: Handle HID0_SLEEP in the TLF_NAPPING hack.

[Scott Wood]

The e300 core (and probably most other 6xx chips) can only come out of
sleep mode with an interrupt.  However, interrupts are logically disabled
by the power management layer.

This hack extends the existing doze/nap hack to also suppress the running
of the interrupt handler when in sleep mode.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/kernel/idle_6xx.S |   40 +++++++++++++++++++++++++++++++++++++---
 1 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/idle_6xx.S b/arch/powerpc/kernel/idle_6xx.S
index 01bcd52..d176042 100644
--- a/arch/powerpc/kernel/idle_6xx.S
+++ b/arch/powerpc/kernel/idle_6xx.S
@@ -147,6 +147,12 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 	isync
 	b	1b
 
+#ifdef CONFIG_SUSPEND
+ret_from_sleep:
+	.long	ret_from_except
+	.long	ret_from_except
+#endif
+
 /*
  * Return from NAP/DOZE mode, restore some CPU specific registers,
  * we are called with DR/IR still off and r2 containing physical
@@ -154,7 +160,33 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
  * address).  We have to preserve r10.
  */
 _GLOBAL(power_save_6xx_restore)
-	lwz	r9,_LINK(r11)		/* interrupted in ppc6xx_idle: */
+#ifdef CONFIG_SUSPEND
+	mfspr	r8, SPRN_HID0
+	andis.	r9, r8, HID0_SLEEP@h
+	beq+	1f
+
+	/*
+	 * SLEEP mode is invoked through the PM subsystem, which means
+	 * that interrupts should be disabled.  However, the hardware
+	 * requires them to be enabled to wake up.  To prevent the
+	 * interrupt from being visible to Linux, return immediately
+	 * rather than run the interrupt handler.
+	 */
+	lis	r9, ret_from_sleep@h
+	ori	r9, r9, ret_from_sleep@l
+	tophys(r9, r9)
+	mtlr	r9
+
+	/*
+	 * Disable interrupts, so that the interrupt doesn't happen
+	 * again until the PM code sets MSR[EE].
+	 */
+	lwz	r9, _MSR(r11)
+	rlwinm	r9, r9, 0, ~MSR_EE
+	stw	r9, _MSR(r11)
+#endif
+
+1:	lwz	r9,_LINK(r11)		/* interrupted in ppc6xx_idle: */
 	stw	r9,_NIP(r11)		/* make it do a blr */
 
 #ifdef CONFIG_SMP
@@ -168,8 +200,10 @@ _GLOBAL(power_save_6xx_restore)
 	 * and load r11 (@ha part + CPU offset) only once
 	 */
 BEGIN_FTR_SECTION
-	mfspr	r9,SPRN_HID0
-	andis.	r9,r9,HID0_NAP@h
+#ifndef CONFIG_SUSPEND
+	mfspr 	r8,SPRN_HID0
+#endif
+	andis.	r9,r8,HID0_NAP@h
 	beq	1f
 	addis	r9,r11,(nap_save_msscr0-KERNELBASE)@ha
 	lwz	r9,nap_save_msscr0@l(r9)
-- 
1.5.3

Re: [PATCH 2/3] pm: Handle HID0_SLEEP in the TLF_NAPPING hack.

[Paul Mackerras]

Scott Wood writes:

> The e300 core (and probably most other 6xx chips) can only come out of
> sleep mode with an interrupt.  However, interrupts are logically disabled
> by the power management layer.
> 
> This hack extends the existing doze/nap hack to also suppress the running
> of the interrupt handler when in sleep mode.

Having thought about this for a bit, I have come to the conclusion
that it would be a lot cleaner to use a new TLF_SLEEPING bit rather
than having to read HID0 to know whether we were napping or sleeping.
There are plenty of bits left in thread_info.local_flags; we've only
used 1 so far. :)

Paul.

Re: [PATCH 2/3] pm: Handle HID0_SLEEP in the TLF_NAPPING hack.

[Scott Wood]

Paul Mackerras wrote:
> Scott Wood writes:
> 
>> The e300 core (and probably most other 6xx chips) can only come out of
>> sleep mode with an interrupt.  However, interrupts are logically disabled
>> by the power management layer.
>>
>> This hack extends the existing doze/nap hack to also suppress the running
>> of the interrupt handler when in sleep mode.
> 
> Having thought about this for a bit, I have come to the conclusion
> that it would be a lot cleaner to use a new TLF_SLEEPING bit rather
> than having to read HID0 to know whether we were napping or sleeping.
> There are plenty of bits left in thread_info.local_flags; we've only
> used 1 so far. :)

OK.

-Scott

[PATCH 3/3] Add 6xx-style HID0_SLEEP support.

[Scott Wood]

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/sysdev/6xx-suspend.S |   52 +++++++++++++++++++++++++++++++++++++
 arch/powerpc/sysdev/Makefile      |    4 +++
 include/asm-powerpc/mpc6xx.h      |    6 ++++
 3 files changed, 62 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/sysdev/6xx-suspend.S
 create mode 100644 include/asm-powerpc/mpc6xx.h

diff --git a/arch/powerpc/sysdev/6xx-suspend.S b/arch/powerpc/sysdev/6xx-suspend.S
new file mode 100644
index 0000000..976ee07
--- /dev/null
+++ b/arch/powerpc/sysdev/6xx-suspend.S
@@ -0,0 +1,52 @@
+/*
+ * Enter and leave sleep state on chips with 6xx-style HID0
+ * power management bits.
+ *
+ * Author: Scott Wood <scottwood@freescale.com>
+ *
+ * Copyright (c) 2006-2007 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <asm/ppc_asm.h>
+#include <asm/reg.h>
+#include <asm/thread_info.h>
+#include <asm/asm-offsets.h>
+
+_GLOBAL(mpc6xx_enter_sleep)
+	mflr	r4
+
+	mfspr	r5, SPRN_HID0
+	rlwinm	r5, r5, 0, ~(HID0_DOZE | HID0_NAP)
+	oris	r5, r5, HID0_SLEEP@h
+	mtspr	SPRN_HID0, r5
+	isync
+
+	lis	r5, ret_from_sleep@h
+	ori	r5, r5, ret_from_sleep@l
+	mtlr	r5
+
+	rlwinm	r5, r1, 0, 0, 31-THREAD_SHIFT
+	lwz	r6, TI_LOCAL_FLAGS(r5)
+	ori	r6, r6, _TLF_NAPPING
+	stw	r6, TI_LOCAL_FLAGS(r5)
+
+	mfmsr	r5
+	ori	r5, r5, MSR_EE
+	oris	r5, r5, MSR_POW@h
+	sync
+	mtmsr	r5
+	isync
+
+1:	b	1b
+
+ret_from_sleep:
+	mfspr	r5, SPRN_HID0
+	rlwinm	r5, r5, 0, ~HID0_SLEEP
+	mtspr	SPRN_HID0, r5
+
+	mtlr	r4
+	blr
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 08ce31e..84a0800 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -38,3 +38,7 @@ obj-$(CONFIG_CPM2)		+= cpm2_common.o cpm2_pic.o
 obj-$(CONFIG_8xx)		+= mpc8xx_pic.o commproc.o
 obj-$(CONFIG_UCODE_PATCH)	+= micropatch.o
 endif
+
+ifeq ($(CONFIG_SUSPEND),y)
+obj-$(CONFIG_6xx)		+= 6xx-suspend.o
+endif
diff --git a/include/asm-powerpc/mpc6xx.h b/include/asm-powerpc/mpc6xx.h
new file mode 100644
index 0000000..01a33ed
--- /dev/null
+++ b/include/asm-powerpc/mpc6xx.h
@@ -0,0 +1,6 @@
+#ifndef __ASM_POWERPC_MPC6xx_H
+#define __ASM_POWERPC_MPC6xx_H
+
+void mpc6xx_enter_sleep(void);
+
+#endif
-- 
1.5.3

Re: [PATCH 3/3] Add 6xx-style HID0_SLEEP support.

[Paul Mackerras]

Scott Wood writes:

> +_GLOBAL(mpc6xx_enter_sleep)

The name is slightly unfortunate as powerbooks also use 6xx-family
processors but enter sleep in a quite different manner, because they
exit sleep with a reset.  Can you think of a better name, that gives a
hint that this is the one to use if we are exiting sleep with an
interrupt rather than a reset?

Paul.

Re: [PATCH 3/3] Add 6xx-style HID0_SLEEP support.

[Scott Wood]

Paul Mackerras wrote:
> Scott Wood writes:
> 
>> +_GLOBAL(mpc6xx_enter_sleep)
> 
> The name is slightly unfortunate as powerbooks also use 6xx-family
> processors but enter sleep in a quite different manner, because they
> exit sleep with a reset.  Can you think of a better name, that gives a
> hint that this is the one to use if we are exiting sleep with an
> interrupt rather than a reset?

The nomenclature I've been using on 83xx is to call it deep sleep if you 
exit with a reset, but if powerbooks can only do deep sleep, and it's 
already been called plain old "sleep", then I guess something else is 
warranted.

mpc6xx_enter_coreonly_sleep?
mpc6xx_enter_light_sleep?
mpc6xx_enter_standby?

-Scott

[PATCH 4/3] Make swsusp_32.S usable for suspend-to-RAM.

[Scott Wood]

This allows platform suspend code to re-use the generic state saving
code, passing a pointer to the low-level suspend code.

The resume path is modified so that non-hibernate callers skip
hibernate-specific bits, and so that callers can specify that the MMU is
off (and thus BATs should be restored).

The _GLOBAL around swsusp_save_area is changed to .global, as the former
puts the data in the text section, which causes an oops with page
debugging enabled.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
Sorry, this one got left out of the previous PM patchset.

 arch/powerpc/Kconfig            |   11 +++++++
 arch/powerpc/kernel/Makefile    |    2 +-
 arch/powerpc/kernel/swsusp_32.S |   60 ++++++++++++++++++++++----------------
 3 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 00099ef..b8c6fa2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -123,6 +123,17 @@ config DEFAULT_UIMAGE
 	  Used to allow a board to specify it wants a uImage built by default
 	default n
 
+config PPC32_SUSPEND
+	bool
+	depends on PPC32
+	default n
+
+config PPC32_SWSUSP
+	bool
+	depends on PPC32 && HIBERNATION
+	select PPC32_SUSPEND
+	default y
+
 config PPC64_SWSUSP
 	bool
 	depends on PPC64 && (BROKEN || (PPC_PMAC64 && EXPERIMENTAL))
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 967afc5..d95b09f 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -38,7 +38,7 @@ obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 obj-$(CONFIG_6xx)		+= idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
 obj-$(CONFIG_TAU)		+= tau_6xx.o
 obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o
-obj32-$(CONFIG_HIBERNATION) += swsusp_32.o
+obj32-$(CONFIG_PPC32_SUSPEND)	+= swsusp_32.o
 obj64-$(CONFIG_HIBERNATION) += swsusp_64.o swsusp_asm64.o
 obj32-$(CONFIG_MODULES)		+= module_32.o
 
diff --git a/arch/powerpc/kernel/swsusp_32.S b/arch/powerpc/kernel/swsusp_32.S
index 69e8f86..ed1c95b 100644
--- a/arch/powerpc/kernel/swsusp_32.S
+++ b/arch/powerpc/kernel/swsusp_32.S
@@ -33,15 +33,21 @@
 	.section .data
 	.align	5
 
-_GLOBAL(swsusp_save_area)
+	.global	swsusp_save_area
+swsusp_save_area:
 	.space	SL_SIZE
 
 
 	.section .text
 	.align	5
 
+#ifdef CONFIG_SOFTWARE_SUSPEND
 _GLOBAL(swsusp_arch_suspend)
+	lis	r3, swsusp_save@h
+	ori	r3, r3, swsusp_save@l
+#endif
 
+_GLOBAL(do_suspend)
 	lis	r11,swsusp_save_area@h
 	ori	r11,r11,swsusp_save_area@l
 
@@ -64,8 +70,8 @@ _GLOBAL(swsusp_arch_suspend)
 	stw	r4,SL_TB(r11)
 	mftb	r5
 	stw	r5,SL_TB+4(r11)
-	mftbu	r3
-	cmpw	r3,r4
+	mftbu	r6
+	cmpw	r6,r4
 	bne	1b
 
 	/* Save SPRGs */
@@ -119,7 +125,8 @@ _GLOBAL(swsusp_arch_suspend)
 	/* Call the low level suspend stuff (we should probably have made
 	 * a stackframe...
 	 */
-	bl	swsusp_save
+	mtctr	r3
+	bctrl
 
 	/* Restore LR from the save area */
 	lis	r11,swsusp_save_area@h
@@ -129,7 +136,7 @@ _GLOBAL(swsusp_arch_suspend)
 
 	blr
 
-
+#ifdef CONFIG_SOFTWARE_SUSPEND
 /* Resume code */
 _GLOBAL(swsusp_arch_resume)
 
@@ -212,6 +219,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 	bdnz	1b
 	sync
 
+	li	r3, 0
+#endif
+
+	/* r3 = nonzero if the MMU is completely disabled and
+	 * BATs may be restored, zero otherwise.
+	 */
+_GLOBAL(do_resume)
 	/* Ok, we are now running with the kernel data of the old
 	 * kernel fully restored. We can get to the save area
 	 * easily now. As for the rest of the code, it assumes the
@@ -226,10 +240,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 	bl	__restore_cpu_setup
 #endif
 	/* Restore the BATs, and SDR1.  Then we can turn on the MMU.
-	 * This is a bit hairy as we are running out of those BATs,
-	 * but first, our code is probably in the icache, and we are
-	 * writing the same value to the BAT, so that should be fine,
-	 * though a better solution will have to be found long-term
+	 * This can only be done when r3 != 0 (and thus the MMU is
+	 * off).
 	 */
 	lwz	r4,SL_SDR1(r11)
 	mtsdr1	r4
@@ -242,7 +254,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 	lwz	r4,SL_SPRG0+12(r11)
 	mtsprg	3,r4
 
-#if 0
+	cmpw	r3, 0
+	beq	1f
+
 	lwz	r4,SL_DBAT0(r11)
 	mtdbatu	0,r4
 	lwz	r4,SL_DBAT0+4(r11)
@@ -275,8 +289,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 	mtibatu	3,r4
 	lwz	r4,SL_IBAT3+4(r11)
 	mtibatl	3,r4
-#endif
 
+1:
 BEGIN_FTR_SECTION
 	li	r4,0
 	mtspr	SPRN_DBAT4U,r4
@@ -306,8 +320,16 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_HIGH_BATS)
 
 	/* restore the MSR and turn on the MMU */
 	lwz	r3,SL_MSR(r11)
-	bl	turn_on_mmu
-	tovirt(r11,r11)
+	lis	r4, 1f@h
+	ori	r4, r4, 1f@l
+
+	mtsrr0	r4
+	mtsrr1	r3
+	sync
+	isync
+	rfi
+
+1:	tovirt(r11, r11)
 
 	/* Restore TB */
 	li	r3,0
@@ -334,15 +356,3 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_HIGH_BATS)
 
 	li	r3,0
 	blr
-
-/* FIXME:This construct is actually not useful since we don't shut
- * down the instruction MMU, we could just flip back MSR-DR on.
- */
-turn_on_mmu:
-	mflr	r4
-	mtsrr0	r4
-	mtsrr1	r3
-	sync
-	isync
-	rfi
-
-- 
1.5.3

Re: [PATCH 4/3] Make swsusp_32.S usable for suspend-to-RAM.

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]


> +config PPC32_SUSPEND

> +config PPC32_SWSUSP

> -obj32-$(CONFIG_HIBERNATION) += swsusp_32.o
> +obj32-$(CONFIG_PPC32_SUSPEND)	+= swsusp_32.o

I think that's overkill, couldn't it just use CONFIG_PM_SLEEP here? I'm
not sure that was available at the time you posted this patchset though,
so that may well be the reason you didn't use it.

For reference, kernel/power/Kconfig has:

config PM_SLEEP
        bool   
        depends on SUSPEND || HIBERNATION
        default y


johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 4/3] Make swsusp_32.S usable for suspend-to-RAM.

[Scott Wood]

Johannes Berg wrote:
>> +config PPC32_SUSPEND
> 
>> +config PPC32_SWSUSP
> 
>> -obj32-$(CONFIG_HIBERNATION) += swsusp_32.o
>> +obj32-$(CONFIG_PPC32_SUSPEND)	+= swsusp_32.o
> 
> I think that's overkill, couldn't it just use CONFIG_PM_SLEEP here?

This file is specific to 32-bit classic PPC chips.

It doesn't matter, though, as I've redone the 83xx suspend to not need 
that file.

-Scott