ARM hibernation / suspend-to-disk (fwd)

Hi,

to make one thing clear to start with:

This is not a proposal to integrate the hibernation feature into mainline.

It's not ripe for that because it's unclear what exactly the differences are between what the hibernation / S2RAM codepaths have to do, and/or how much code sharing between these codepaths is advisable.

Therefore, I'm merely putting this out as-is as a reference / base for those who want to do their own experiments with the feature.

Prerequisites:

* The patch requires a working cpu_reset(); for v6/v7, that's in Will Deacon's patch series from last week.

* It also requires being able to take the address of cpu_reset and on MULTI_CPU configs therefore, again see Will Deacon's patch series.

* It uses on cpu_suspend() / cpu_resume(), therefore a recent-enough kernel (or a port of those to whatever you're using) is required.

Important / Limitations:

A) For platforms that currently use cpu_suspend/cpu_resume in their s2ram codepaths AND DO NOTHING ELSE BEFORE AND NOTHING ELSE AFTER THAT BUT TO "ENTER LOW POWER", the way the hibernation state snapshot for the CPU core is done by this patch is sufficient.

To make it clear: IF AND ONLY IF your suspend(-to-ram) func looks like:

ENTRY(acmeSoC_cpu_suspend)
stmfd sp!, {r4-r12,lr}
ldr r3, resume_mmu_done
bl cpu_suspend
resume_mmu_done:
ldmfd sp!, {r3-r12,pc}
ENDPROC(acmeSoC_cpu_suspend)

then this patch is ok to provide hibernation support for your CPU core.

In all other cases, item B).

IN ALL CASES, read item C) below; there's more than the core to the SoC.

B) If there's any cpu-specific / SoC-specific state that needs (re)init over a suspend-to-disk/resume-from-disk cycle, this patch is incomplete because it provides no hooks/code for that.

This is the case e.g. for "secure" SoCs that have different sets of control registers and/or different CR reg access patterns.

It's also the case e.g. for SoCs with L2 caches as the activation sequence there is SoC-dependent; a full off-on cycle for L2 is not done by the hibernation support code.

It's also the case if the SoC requires steps on wakeup _before_ the "generic" parts done by cpu_suspend / cpu_resume can work correctly.

(OMAP is an example of such a SoC; the patch "works" on OMAP in the sense that it gets you a non-secure OMAP back from hibernation but as mentioned, your mileage may vary; I for example don't know what the consequences of not disabling / reenabling the L2 cache over cpu_reset are)

C) The current low-power handling may perform SoC-specific tasks such as GPIO state handling, which is currently also not addressed by this patch; the assumption is rather that device suspend/resume deals with this. That is not guaranteed to be complete / sufficient - if your SoC and/or your device drivers have particular needs in this area, you can put hooks performing these actions into save_/restore_processor_state().

D) If you wish to extend this to a SoC for which the generic CPU core hooks (i.e. what cpu_suspend/resume provide) are either insufficient or unavailable, hook your own state snapshot mechanism into the places where this code currently calls cpu_suspend and cpu_resume. The main assumption by the assembly code within swusp_arch_resume() is that __swsusp_arch_restore_image() returns to its caller with the stack restored - how you achieve that is up to you, the patch as-is does it by cpu_reset(cpu_resume) but your mileage may vary.

I will, at this point, not send further iterations of this patch.

I'll answer questions about it though, if there are any.

Thanks for all the comments during the previous threads - learned a lot !

Best regards,
FrankH.

Nothing has that - because you can't execute that ldmfd after the call to cpu_suspend returns. I don't think you've understood what I said on that subject in the previous thread.

Sorry typo.

Missed the critical lines:

ENTRY(acmeSoC_cpu_suspend)
stmfd sp!, {r3-r12,lr}
ldr r3, =resume_mmu_done
[ load r1 with v:p, whichever way - either you or your caller ]

bl cpu_suspend

[ ... do whatever low power entry stuff ... ]

[ function effectively ENDS HERE ... ]

resume_mmu_done:
ldmfd sp!, {r3-r12,pc}

That's the key bits. DO NOTHING OF RELEVANCE BEFORE cpu_suspend AND DO NOTHING APART FROM ENTERING LOW POWER AFTERWARDS.

I _have_ understood what you've said. cpu_suspend is not made for this purpose, and hence I'm not claiming it should be. The use of it to create the core state snapshot for hibernation is merely something that, right now, works _by accident_ (or sheer luck, if you want to say that) rather than by intent / design.

Since there _is no design_ for what should work / how it should work in this context, I'm leaving it as-is, merely stating the constraints. Roll your own if you need / want to.

Does that clarify ?

FrankH.

Ok, to illustrate a bit more, what is ok and what not.

From: [email protected] ()
Date: Wed, 01 Jun 2011 16:03:18 -0000
Subject: No subject
Message-ID: <[email protected]>

a) a case where the hibernation patch as I posted it is ok for core state:

arch/arm/mach-exynos4/sleep.S has:

ENTRY(s3c_cpu_save)

         stmfd   sp!, { r3 - r12, lr }
         ldr     r3, =resume_with_mmu
         bl      cpu_suspend

         ldr     r0, =pm_cpu_sleep
         ldr     r0, [ r0 ]
         mov     pc, r0

resume_with_mmu:

         ldmfd   sp!, { r3 - r12, pc }

ENTRY(s3c_cpu_resume)
b cpu_resume

I.e. it does nothing before but set up the arguments for cpu_suspend, and does nothing afterwards but redirect to a function that enters low power (and switches the CPU off).
Likewise, all it does for resume is redirect to cpu_resume which will ultimately end up jumping to resume_with_mmu: as requested.

b) a case where the hibernation patch is insufficient even though the code in the soc suspend func uses cpu_suspend:

arch/arm/mach-pxa/sleep.S has:

/*
* pxa27x_cpu_suspend()
*
* Forces CPU into sleep state.
*
* r0 = value for PWRMODE M field for desired sleep state
* r1 = v:p offset
*/
ENTRY(pxa27x_cpu_suspend)

#ifndef CONFIG_IWMMXT
mra r2, r3, acc0
#endif

         stmfd   sp!, {r2 - r12, lr}             @ save registers on stack
         mov     r4, r0                          @ save sleep mode
         ldr     r3, =pxa_cpu_resume             @ resume function
         bl      cpu_suspend

[ ... some stuff ... ]
ldr r6, =CCCR
ldr r8, [r6] @ keep original value for resume

ldr r7, =CCCR_SLEEP @ prepare CCCR sleep value
mov r0, #0x2 @ prepare value for CLKCFG

@ align execution to a cache line
b pxa_cpu_do_suspend
#endif
[ ... ]
pxa_cpu_do_suspend:

@ All needed values are now in registers. @ These last instructions should be in cache

@ initiate the frequency change...
str r7, [r6]
mcr p14, 0, r0, c6, c0, 0

@ restore the original cpu speed value for resume
str r8, [r6]
[ ... ]
/*
* pxa_cpu_resume()
*
* entry point from bootloader into kernel during resume */
.align 5
pxa_cpu_resume:

         ldmfd   sp!, {r2, r3}

#ifndef CONFIG_IWMMXT
mar acc0, r2, r3
#endif
ldmfd sp!, {r4 - r12, pc} @ return to caller

In this case, there's _MORE_ state saved (the CCCR / CCCR_SLEEP accesses) after the call to cpu_suspend things that aren't dealt with by the way the hibernation support patch currently operates.

There also is _more_ restored at resume than what the generic part does for you.

Whether these things (for example) are relevant wrt. to a hibernation resume is something I simply do not know.

A third example of a soc suspend func, even more complex, with more constraints, would be as already mentioned, OMAP. On that as well the caveat applies, "what works empirically might not be correct in all cases".

This hopefully illustrates what someone who tries to make suspend-to-disk work for a particular ARM SoC has to look out for, and why the patch shouldn't be blindly adopted ?!

Again, it's only a starting point. An improved one compared to having ARCH-#ifdefs and huge code bloat all over swsusp.S, but not sufficient for the general case quite yet.

FrankH.

Santosh, Frank: to what extent do you think the OMAP suspend code could be abstracted using something like Colin's CPU pm notifier framework?

I'd hope that at least some stuff can be abstracted out, but I don't understand the OMAP code intimately enough to be certain of that...

We shouldn't expect to remove absolutely all the SoC specifics from suspend code, but the more we can hook into a generic framework, the better for everyone.

Cheers
---Dave

On Mon, 13 Jun 2011, Dave Martin wrote:

[ ... ]

Hi Dave,

you mean the patch set from this weekend ?

I've still got to read through all of that in detail (currently not getting the digest mails).

There's this thing which Russell keeps on pointing out, that a SoC is more than a CPU core and even a CPU core is more than just the ARM itself ... and power management somehow needs dealing with all...

To me, the picture looks a bit like:

- the "inner" thing which is the ARMv[4567] control register set

- the "semi-inner" thing which are SoC core control regs outside of what is generic ARMv[4567], and/or which behave differently from that due to SoC spec / errata / ...
"Secure state" probably fits amongst this as well.

- the "semi-outer" parts like optional ARM features (cache, VFP, Neon, ...) which still have a generic spec

- the "outer" parts - builtin devices and their state, interrupt controller, clocks and regulator stuff, ...

This is unfortunately simplified and the separations between these aren't quite as clean.

It used to be so that as far as device drivers were dealing with suspend/resume the latter bit was done via sysdev/syscore and anything else delegated to a (SoC-specific) platform_suspend_ops entry.

Russell's changes for "generic CPU suspend/resume" get the innermost bits into common code, while Colin's changes allow for more of the outer parts within common code ?

Assuming I'm reading these things right, then the place where OMAP could possibly use Colin's CPU PM notifier changes would be to convert some of the code in omap_sram_idle() (the backbone for platform_suspend_ops on OMAP) to use it.

To use the OMAP code for illustration (could've quoted other SoC code as well, the kind of work to be done is similar but the details of what and how it's done are very different ...):

void omap_sram_idle(void)
{
[ ... ]

                                 omap3_per_save_context();
[ ... ]
                         omap3_core_save_context();
                         omap3_cm_save_context();
[ ... ]
         omap3_intc_prepare_idle();

[ ... ]

         _omap_sram_idle(omap3_arm_context, save_state);
         cpu_init();
[ ... ]
                         omap3_core_restore_context();
                         omap3_cm_restore_context();
                         omap3_sram_restore_context();
                         omap2_sms_restore_context();
[ ... ]
         omap3_intc_resume_idle();
[ ... ]
                         omap3_per_restore_context();

[ ... ]
}

Aren't things like these potential candidates to convert to Colin's framework, if one chooses so ?

While the place where OMAP might possibly be using Russell's generic cpu_suspend/resume were deeper down - within _omap_sram_idle. Note the _ - it's a function pointer actually evaluating to omap????_cpu_suspend.

So it's kind of a two-pronged attack to minimize the SoC-specific code, Colin's framework to extract common code from the "outer" parts and Russell's cpu_suspend/resume to extract common code from the "inner" parts.

Orthogonal problems / solutions ?

FrankH.

On Mon, 13 Jun 2011, Frank Hofmann wrote:

[ ... ]

Also, to clarify on the hibernation side:

The biggest shortfall of the hibernation support patch that I've posted is that it currently has no awareness of the "peripheral" side of things at all, i.e. all the stuff done by the platform_suspend_ops->begin/enter/end for the suspend-to-mem case isn't replicated sufficiently by the hibernation codepath.

If you like, swsusp_arch_suspend / resume are on the same level of complexity as are the functions in platform_suspend_ops. But the current hibernation patch does only parts of that. It gets away with this largely because it "resumes from ON" ...

As far as that is concerned, Russell really has a point, to do all this properly / well for the hibernation case, a hibernation-specific approach on the platform_ops abstraction level would be better than just to hack low-level generics together for the sake of hacking low-level generics together.

If just doing per-SoC hibernation, then the code bloat problem remains, on ARM there's already platform_suspend_ops per SoC; if one adds a full set of platform_hibernation_ops per SoC without thinking where/how to consolidate, it'd only create another mess. Beware the beginnings ...

I had thought the idea of using cpu_suspend / resume actually made it clear that my main point for all this "use generics wherever imaginable" is to never allow for unnecessary bloat in the first place and hence opt for generics even if somewhat imperfect / shoehorned for the general case.

I'd like to apologize if that didn't come over.

I'm following the work in this area; compared to where things were a year ago a lot of things have happened already. In a way, my hope is that part of all this blah of mine is helping to identify areas where code might be shared even if that starts out as coincidental (as is the current use of cpu_suspend / resume inside the hibernation patch).

FrankH.

Actually, we can do something about cpu_suspend.

Currently cpu_suspend is not like a normal C function - when it's called it returns normally to a bunch of code which is not expected to return. The return path is via code pointed to by 'r3'.

It also corrupts a bunch of registers in ways which make it non-compliant with a C API.

If we do make this complaint as a normal C-like function, it eliminates this register saving. We also swap 'lr' and 'r3', so cpu_suspend effectively only returns to following code on resume - and r3 points to the suspend code.

So, this becomes:

	ENTRY(acmeSoC_cpu_suspend)
		stmfd	sp!, {lr}
		adr	r3, soc_finish_suspend
		bl	cpu_suspend
		ldmfd	sp!, {pc}
	ENDPROC(acmeSoC_cpu_suspend)

	soc_finish_suspend:
		blah
		blah
		put soc to sleep
		never return

or even:

static void soc_suspend(void)
{
[soc specific preparation]

cpu_suspend(0, PLAT_PHYS_OFFSET - PAGE_OFFSET,
soc_suspend_arg, soc_suspend_fn);

[soc specific cleanup ]
}

where soc_suspend_fn can be either assembly or C code - but must never return. Patches will follow this evening.

Hi Russell,

this change is perfect; with this, the hibernation support code turns into the attached.

That's both better and simpler to perform a full suspend/resume cycle (via resetting in the cpu_suspend "finisher") after the snapshot image has been created, instead of shoehorning a return into this.

I've tested the attached on v6 (samsung) and v7 (omap3 nonsecure - yes I know ...) kernels that have the cpu_suspend changes in you've posted Monday.

It does what it's supposed to do. And yes, aware that's not enough yet, this is only an attempt to address the "how to snapshot the core state", other outstanding issues will be addressed later.

It'd be great to know why this makes your toenails curl so much ...

Anyway, this again shows that as far as the core SoC state is concerned, the only difference between the hibernation snapshot and the s2ram soc suspend side are what the "finisher" does:

hibernate: s2ram:

swsusp_save() low power self-refresh settings, ...
<reset> <poweroff>

Why would calling swsusp_save() not be acceptable in this context ? It's supposed to run in a restricted env (no ints, no scheduling, ...) anyway.

FrankH.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hibernation-15Jun2011.patch
Type: text/x-diff
Size: 5726 bytes
Desc:
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110615/b7790fe8/attachment-0001.bin>

Should I assume that's an ack for all the patches?

It's now not soo difficult to have an error code returned from the finisher function - the only thing we have to make sure is that the cpu_do_suspend helper just saves state and does not make any state changes.

We can then do this, which makes it possible for the finisher to return, and we propagate its return value. We also ensure that a path through from cpu_resume will result in a zero return value from cpu_suspend.

This isn't an invitation for people to make the S2RAM path return after they time out waiting for suspend to happen - that's potentially dangerous because in that case the suspend may happen while we're resuming devices which wouldn't be nice.

diff -u b/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S
@@ -12,7 +12,6 @@
  *  r1 = v:p offset
  *  r2 = suspend function arg0
  *  r3 = suspend function
- * Note: does not return until system resumes
  */
 ENTRY(cpu_suspend)
 	stmfd	sp!, {r4 - r11, lr}

@@ -26,7 +25,7 @@
 #endif
 	mov	r6, sp			@ current virtual SP
 	sub	sp, sp, r5		@ allocate CPU state on stack
-	mov	r0, sp			@ save pointer
+	mov	r0, sp			@ save pointer to CPU save block
 	add	ip, ip, r1		@ convert resume fn to phys
 	stmfd	sp!, {r1, r6, ip}	@ save v:p, virt SP, phys resume fn
 	ldr	r5, =sleep_save_sp

@@ -55,10 +54,17 @@
 #else
 	bl	__cpuc_flush_kern_all
 #endif
+	adr	lr, BSYM(cpu_suspend_abort)
 	ldmfd	sp!, {r0, pc}		@ call suspend fn
 ENDPROC(cpu_suspend)
 	.ltorg
 
+cpu_suspend_abort:
+	ldmia	sp!, {r1 - r3}		@ pop v:p, virt SP, phys resume fn
+	mov	sp, r2
+	ldmfd	sp!, {r4 - r11, pc}
+ENDPROC(cpu_suspend_abort)
+
 /*
  * r0 = control register value
  * r1 = v:p offset (preserved by cpu_do_resume)

@@ -88,6 +94,7 @@
 cpu_resume_after_mmu:
 	str	r5, [r2, r4, lsl #2]	@ restore old mapping
 	mcr	p15, 0, r0, c1, c0, 0	@ turn on D-cache
+	mov	r0, #0			@ return zero on success
 	ldmfd	sp!, {r4 - r11, pc}
 ENDPROC(cpu_resume_after_mmu)
 

Sorry the late reply, been away.

I've only tested on s3c64xx (for s2ram and s2disk) and omap3 (s2disk only due to the omap code not having been "genericized" yet), the changes are ok there.
The remainder apply and compile ok.

If that's sufficient, then:

You're right there's some risk that the ability to return an error here is misinterpreted as an invitation to use error returns for indicating state machine conditions.

What I'm wondering about is the usecase for having an error return opportunity in this case (beyond reporting it as such). Isn't it rather so that most finishers would succeed and at best do a "BUG_ON()" at failure, because system state then isn't such to make it trivially recoverable ?

For hibernation, the ability to force the entire down/up transition before writing the snapshot image out is actually very beneficial, for reliability - one knows that the device/cpu side of suspend/resume has already worked when the snapshot is being written, without having to wait for reboot/image load/resume to test that. One would want to go through suspend/resume even if the memory snapshot operation, swsusp_save, errors. A failure of swsusp_save doesn't make suspend/resume itself a failure, therefore it's desirable to propagate that return code in other ways (and keep the finisher "unconditional").

I'm not opposed to this addition as such, but I'm curious:
* If an error occurs, how can the caller of cpu_suspend recover from it ?
* What's the state the system is in after an error in this codepath ?
* What subsystems / users would do anything else with it than BUG_ON() ?

Also, consider possible errors in the SoC-specific code on the resume side; situations like a failure to perform SoC-iROM-calls for e.g. an attempted secure state restore would result in errors that can't be propagated by this mechanism; i.e. there are still failure modes which require platform-specific intervention of sorts, and platform-specific error propagation/handling, even were cpu_suspend to return error codes.

FrankH.

I've added that to the core stuff and s3c64xx.

Well, an error return undoes (and probably corrupts) the state which cpu_suspend saved, so it wouldn't be useful for state machine stuff - I wonder whether we should poison the saved pointers just to reinforce this point.

For s2ram, there's no reason at this point to fail. The only thing left to do at this point is to do whatever is required to cause the system to enter the low power mode.

In the case where you need to place the SDRAM into self refresh mode manually, the timing of stuff after self refresh mode has been entered to the point where power is cut to the CPU is normally well defined, and if for whatever reason power doesn't get cut, there's very little which can be done to re-awake the system (you'd have to ensure that the code to do that was already in the I-cache and whatever data was required was already in registers.)

And as I mentioned, if you timeout waiting for the system to power off, and you've done everything you should have done, are you sure that the system won't power off on you unexpectedly if you return with an error, which would then mean you have corrupted state saved (and possibly corrupted filesystems.)

So, I don't think S2RAM would have any practical use for an error return path. It's more for hibernation.

It's not a test of the suspend/resume path though - device state is very different between suspending in some way followed by an immediate resume without resetting, and a real suspend, power off, resume cycle.

The former doesn't really test the resume paths - a register which has been forgotten won't be found by any other way than cutting the power, and you'll only ever find that on a proper resume-from-power-off.

If an error occurs from cpu_suspend(), then the caller needs to undo any modification to state which it did while saving, and return an error.

We can arrange for another callback into CPU specific code to sort out anything they've done - iow, an error return from cpu_suspend should ensure that we have the same state present as we had at the point it was called. We currently do not need that as cpu_suspend() just saves state without modifying anything, but if we ended up with a CPU which required stuff to be modified, we'd have to undo those modifications before returning an error.

The error code from the platform_suspend_ops ->enter method is already dealt with - by going through the resume steps before returning the error code. Whether hibernate has similar functionality I don't know.

What I do think is that if hibernate needs to return an error code, we'd better do that without doing things like taking the MMU down to run the resume paths in order to do that. If we have some error to propagate, we want to propagate it back as easily as possible.

Frank Hofmann a ?crit :

Uh - sorry. I used to have both the __nosavedata and __attribute__((__aligned__(PAGE_SIZE/2))) attributes there. That must've gone lost at one point.

It's an artifact of the build that things turn out ok by default; the __nosavedata forces a separate section (page), and arch/arm is linked before kernel/power (the other user of __nosavedata), hence this block, due to the way the kernel build works, ends up just fine.

But as you say, not by intent / declaration.

Have you seen Will Deacon's suggested kexec changes ? That keeps a "reset stack" page around, _elsewhere_, and I've been considering using that. In the end, all swsusp_arch_resume() really requires is a stack page that's guaranteed to be outside the target kernel data, thereby left alone by the restore. __nosavedata is merely one way.

Thanks,

FrankH.

Hi Frank,

For what it's worth, in v4 of that series (hopefully I'll post it soon) I'm starting to think about SMP kexec which necessitates that the reserved stack page is placed in a fixed location. I'm planning to memblock_reserve the page immediately below swapper for this.

Will

Frank Hofmann a ?crit :

Frank Hofmann a ?crit :

yes

You are modifying it by putting the same code (modulo dynamic patching on the code (ftrace, kprobe, ...)).

Yes.

I wasn't suggesting that.

While reading the code I was surprised of that. And that there weren't any comment about it.

Looking at other implementations, only x86_64 seems to need to relocate the copy code.

Matthieu

Hi Frank,

2011/6/13 Frank Hofmann <[email protected]>:

i did bring-up prima2(cortex-a9) swsusp hibernation support based on your patch. i am considering whether we can have some ways to make the resume faster.

Now the resume flow is:
[1] poweron -> bootloader -> kernel -> all boot and device probed -> late_init() -> loading snapshot from disk to memory -> resume devices and software

Is it possible we make it:
[2] poweron -> bootloader -> loading snapshot from disk to memory -> resume devices and software

Then the sub-process "bootloader -> kernel -> all boot and device probed -> late_init()" will be eliminated and time to boot kernel will not be needed too.

the flow [2] is more platform-related. so it is probably we can use platform_hibernation_ops to handle. but it can be generic as well.

-barry