diff mbox

[v7,2/2] ARM hibernation / suspend-to-disk

Message ID 1394016605-24120-3-git-send-email-sebastian.capella@linaro.org
State New
Headers show

Commit Message

Sebastian Capella March 5, 2014, 10:50 a.m. UTC
From: Russ Dill <Russ.Dill@ti.com>

Enable hibernation for ARM architectures and provide ARM
architecture specific calls used during hibernation.

The swsusp hibernation framework depends on the
platform first having functional suspend/resume.

Then, in order to enable hibernation on a given platform, a
platform_hibernation_ops structure may need to be registered with
the system in order to save/restore any SoC-specific / cpu specific
state needing (re)init over a suspend-to-disk/resume-from-disk cycle.

For example:

     - "secure" SoCs that have different sets of control registers
       and/or different CR reg access patterns.

     - 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.

     - SoCs requiring steps on wakeup _before_ the "generic" parts
       done by cpu_suspend / cpu_resume can work correctly.

     - SoCs having persistent state which is maintained during suspend
       and resume, but will be lost during the power off cycle after
       suspend-to-disk.

This is a rebase/rework of Frank Hofmann's v5 hibernation patchset.

Acked-by: Russ Dill <Russ.Dill@ti.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sebastian Capella <sebastian.capella@linaro.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Len Brown <len.brown@intel.com>
Cc: Nicolas Pitre <nico@linaro.org>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Jonathan Austin <jonathan.austin@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Laura Abbott <lauraa@codeaurora.org>
Cc: Jiang Liu <liuj97@gmail.com>
Cc: Sricharan R <r.sricharan@ti.com>
Cc: Victor Kamensky <victor.kamensky@linaro.org>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
---
 arch/arm/include/asm/memory.h |    1 +
 arch/arm/kernel/Makefile      |    1 +
 arch/arm/kernel/hibernate.c   |  108 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/Kconfig           |    5 ++
 include/linux/suspend.h       |    2 +
 5 files changed, 117 insertions(+)
 create mode 100644 arch/arm/kernel/hibernate.c

Comments

Sebastian Capella March 7, 2014, 4:21 a.m. UTC | #1
Sorry to those receiving repeated emails, I'm having an issue with some
of the special characters names.

Quoting Sebastian Capella (2014-03-05 02:50:05)
> From: Russ Dill <Russ.Dill@ti.com>
> Enable hibernation for ARM architectures and provide ARM
> architecture specific calls used during hibernation.
...
Quoting Stephen Boyd (2014-02-27 18:19:49)
> On 02/27/14 17:47, Russ Dill wrote:
> > On 02/27/2014 04:09 PM, Stephen Boyd wrote:
> >> On 02/27/14 15:57, Sebastian Capella wrote:
> >>> diff --git a/arch/arm/include/asm/memory.h
> >>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8
> >>> 100644 ---
> >>> a/arch/arm/include/asm/memory.h +++
> >>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,7 @@ static
> >>> inline
> >>> void *phys_to_virt(phys_addr_t x) */ #define __pa(x)
> >>> __virt_to_phys((unsigned long)(x)) #define __va(x)
> >>> ((void
> >>> *)__phys_to_virt((phys_addr_t)(x)))
> >>> +#define __pa_symbol(x)
> >>> __pa(RELOC_HIDE((unsigned long)(x), 0))
> >> Just curious, is there a reason for the RELOC_HIDE() here?
> >> Or
> >> __pa_symbol() for that matter? It looks like only x86 uses
> >> this on
> >> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?
> > From my understanding this needs to stick around so long as
> > gcc 3.x is
> > supported (did it get dropped yet?) on ARM Linux since it
> > doesn't
> > support -fno-strict-overflow.
>
> I don't think it's been dropped yet but I wonder if anyone has
> tried
> recent kernels with such a compiler?

Hi Stephen,

I was wondering if you had any objections or additional comments on
the current patch.

I've basically dropped the RELOC_HIDE.  It hasn't proven to be an issue
with the current compiler, and I was unable to revert back to 3.x
compiler on the cortex-v7 platforms I'm using.

Since this is being used on Hibernation, we would not be breaking any
existing functionality in any event, and it sounds like this is unlikely
to be an issue.

I spoke informally to a friend who works on toolchains at Linaro and
he suggested this may be related to the the strict-aliasing feature
which appears to be disabled by default in the kernel
(-fno-strict-aliasing) and generally not well liked.

Unless someone objects, I'll plan to add this to Russell's patch system.

Thanks,

Sebastian

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Stephen Boyd March 7, 2014, 4:42 a.m. UTC | #2
On 03/05, Sebastian Capella wrote:
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 8756e4b..d32adbb 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -291,6 +291,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>   */
>  #define __pa(x)			__virt_to_phys((unsigned long)(x))
>  #define __va(x)			((void *)__phys_to_virt((phys_addr_t)(x)))
> +#define __pa_symbol(x)		__pa((unsigned long)(x))

Thanks for removing RELOC_HIDE, as Russell already stated it's
never been necessary on ARM.

Looking at this definition now it doesn't look right. Isn't
&__nosave_begin a virtual address? Casting it to an unsigned long
isn't going to give you a physical address. Why can't we use
__pa()?

> +extern const void __nosave_begin, __nosave_end;
> +
> +int pfn_is_nosave(unsigned long pfn)
> +{
> +	unsigned long nosave_begin_pfn =
> +			__pa_symbol(&__nosave_begin) >> PAGE_SHIFT;
> +	unsigned long nosave_end_pfn =
> +			PAGE_ALIGN(__pa_symbol(&__nosave_end)) >> PAGE_SHIFT;
> +
> +	return (pfn >= nosave_begin_pfn) && (pfn < nosave_end_pfn);
> +}

Perhaps this code could be:

	unsigned long nosave_begin_pfn = virt_to_pfn(&__nosave_begin);
	unsigned long nosave_end_pfn = virt_to_pfn(&__nosave_end);

	return (pfn >= nosave_begin_pfn) && (pfn <= nosave_end_pfn);

or if virt_to_pfn() doesn't exist on ARM and we can't add it for
some reason:

	unsigned long nosave_begin_pfn = __phys_to_pfn(__pa(&__nosave_begin));
	unsigned long nosave_end_pfn = __phys_to_pfn(__pa(&__nosave_end));

	return (pfn >= nosave_begin_pfn) && (pfn <= nosave_end_pfn);
Sebastian Capella March 10, 2014, 6:32 p.m. UTC | #3
On 6 March 2014 20:42, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 03/05, Sebastian Capella wrote:
>> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
>> index 8756e4b..d32adbb 100644
>> --- a/arch/arm/include/asm/memory.h
>> +++ b/arch/arm/include/asm/memory.h
>> @@ -291,6 +291,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>>   */
>>  #define __pa(x)                      __virt_to_phys((unsigned long)(x))
>>  #define __va(x)                      ((void *)__phys_to_virt((phys_addr_t)(x)))
>> +#define __pa_symbol(x)               __pa((unsigned long)(x))
> Looking at this definition now it doesn't look right. Isn't
> &__nosave_begin a virtual address? Casting it to an unsigned long
> isn't going to give you a physical address. Why can't we use
> __pa()?

You're correct, sorry, I missed removing the cast, it's not needed.
I've removed the __pa_symbol define as I'm switching to the __pa
implementation you suggested below.

>> +extern const void __nosave_begin, __nosave_end;
>> +
>> +int pfn_is_nosave(unsigned long pfn)
>> +{
>> +     unsigned long nosave_begin_pfn =
>> +                     __pa_symbol(&__nosave_begin) >> PAGE_SHIFT;
>> +     unsigned long nosave_end_pfn =
>> +                     PAGE_ALIGN(__pa_symbol(&__nosave_end)) >> PAGE_SHIFT;
>> +
>> +     return (pfn >= nosave_begin_pfn) && (pfn < nosave_end_pfn);
>> +}
>
> Perhaps this code could be:
>
>         unsigned long nosave_begin_pfn = virt_to_pfn(&__nosave_begin);
>         unsigned long nosave_end_pfn = virt_to_pfn(&__nosave_end);
>
>         return (pfn >= nosave_begin_pfn) && (pfn <= nosave_end_pfn);
>
> or if virt_to_pfn() doesn't exist on ARM and we can't add it for
> some reason:
>
>         unsigned long nosave_begin_pfn = __phys_to_pfn(__pa(&__nosave_begin));
>         unsigned long nosave_end_pfn = __phys_to_pfn(__pa(&__nosave_end));
>
>         return (pfn >= nosave_begin_pfn) && (pfn <= nosave_end_pfn);
I've moved to this second implementation.  I don't think I understand
the ramifications enough to add the virt_to_pfn call for arm.

I've changed one thing.  __nosave_end is pointing at the first byte
not included in the nosave region.  I subtracted one from it so that
we make sure we're referring to the last pfn, and left the pfn
comparison as you'd suggested.

int pfn_is_nosave(unsigned long pfn)
{
        unsigned long nosave_begin_pfn = __phys_to_pfn(__pa(&__nosave_begin));
        unsigned long nosave_end_pfn = __phys_to_pfn(__pa(&__nosave_end - 1));

        return (pfn >= nosave_begin_pfn) && (pfn <= nosave_end_pfn);
}

I'll test this and post it if everyone's ok with it.

Thanks for your review and comments Stephen!

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ezequiel Garcia March 16, 2014, 7:09 a.m. UTC | #4
On Mar 05, Sebastian Capella wrote:
[..]
> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index 1f8fed9..83707702 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -611,6 +611,11 @@ config CPU_USE_DOMAINS
>  config IO_36
>  	bool
>  
> +config ARCH_HIBERNATION_POSSIBLE
> +	bool
> +	depends on MMU
> +	default y if CPU_ARM920T || CPU_ARM926T || CPU_SA1100 || CPU_XSCALE || CPU_XSC3 || CPU_V6 || CPU_V6K || CPU_V7
> +
>  comment "Processor Features"
>  

Is there any reason why CPU_FEROCEON is not listed here? FWIW, I've just built
(but not really tested) a Kirkwood kernel with CONFIG_HIBERNATION=y.

And is there any reason to put this config in arch/arm/mm/Kconfig, instead of
in arch/arm/Kconfig, below ARCH_SUSPEND_POSSIBLE? 

I'm also puzzled about having two separate options for suspend and hibernate,
maybe someone can explain me why a given CPU would support the former but not
the latter?

Thanks!
Russell King - ARM Linux March 16, 2014, 9:46 a.m. UTC | #5
On Mon, Mar 10, 2014 at 11:32:17AM -0700, Sebastian Capella wrote:
> I've moved to this second implementation.  I don't think I understand
> the ramifications enough to add the virt_to_pfn call for arm.

Let's look at the implementations...

#define __pa(x)                 __virt_to_phys((unsigned long)(x))
static inline phys_addr_t __virt_to_phys(unsigned long x)
{
        return (phys_addr_t)x - PAGE_OFFSET + PHYS_OFFSET;
}

So, __pa() returns a phys_addr_t (which may be either 32-bit or 64-bit).

#define virt_to_pfn(kaddr)  (__pa(kaddr) >> PAGE_SHIFT)

And that will do an appropriate shift to convert the phys_addr_t to a PFN,
which will be fine to assign to an unsigned long variable.

#define __phys_to_pfn(paddr)    ((unsigned long)((paddr) >> PAGE_SHIFT))

This does the same thing, but has an explicit cast.

So, the only difference between the two suggestions is that additional
cast, which has no effect here.

> I've changed one thing.  __nosave_end is pointing at the first byte
> not included in the nosave region.  I subtracted one from it so that
> we make sure we're referring to the last pfn, and left the pfn
> comparison as you'd suggested.

Yes, that should be safer.
Sebastian Capella March 17, 2014, 7:10 p.m. UTC | #6
On 16 March 2014 00:09, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> On Mar 05, Sebastian Capella wrote:
> [..]
>> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
>> index 1f8fed9..83707702 100644
>> --- a/arch/arm/mm/Kconfig
>> +++ b/arch/arm/mm/Kconfig
>> @@ -611,6 +611,11 @@ config CPU_USE_DOMAINS
>>  config IO_36
>>       bool
>>
>> +config ARCH_HIBERNATION_POSSIBLE
...
>> +     default y if CPU_ARM920T || CPU_ARM926T || CPU_SA1100 || CPU_XSCALE || CPU_XSC3 || CPU_V6 || CPU_V6K || CPU_V7
...
> Is there any reason why CPU_FEROCEON is not listed here? FWIW, I've just built
> (but not really tested) a Kirkwood kernel with CONFIG_HIBERNATION=y.
No reason; I did not change this from the original patch I'd received.
 I didn't try to get a comprehensive list of supported hardware.  To
my understanding, the goal is to get the infrastructure in so that
people can start working on their platforms and add support for them.

> And is there any reason to put this config in arch/arm/mm/Kconfig, instead of
> in arch/arm/Kconfig, below ARCH_SUSPEND_POSSIBLE?
I don't have a reason.  Anyone else have a comment on this?
Otherwise, I'll move it.. thanks!

> I'm also puzzled about having two separate options for suspend and hibernate,
> maybe someone can explain me why a given CPU would support the former but not
> the latter?
It's part of having the generic hibernation implemented and available
but with architecture specific dependencies.  Where an architecture
may not have support for hibernation, it will prevent compilation of
the generic hibernation support.  For example, at the moment, ARM does
not support hibernation.  As a result, hibernation is not presented as
an option in menuconfig.  If you were to artificially enable it (by
hacking ARCH_HIBERNATION_POSSIBLE=y, and then turning
CONFIG_HIBERNATION=y) then you'd get linker errors due to the missing
architecture hibernation functions.  (basically what is being added by
this patch for ARM).  It's like this for other architectures as well
(mips, sh, etc..)

Thanks,

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ezequiel Garcia March 17, 2014, 8:44 p.m. UTC | #7
On Mar 17, Sebastian Capella wrote:
> On 16 March 2014 00:09, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
> > On Mar 05, Sebastian Capella wrote:
> > [..]
> >> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> >> index 1f8fed9..83707702 100644
> >> --- a/arch/arm/mm/Kconfig
> >> +++ b/arch/arm/mm/Kconfig
> >> @@ -611,6 +611,11 @@ config CPU_USE_DOMAINS
> >>  config IO_36
> >>       bool
> >>
> >> +config ARCH_HIBERNATION_POSSIBLE
> ...
> >> +     default y if CPU_ARM920T || CPU_ARM926T || CPU_SA1100 || CPU_XSCALE || CPU_XSC3 || CPU_V6 || CPU_V6K || CPU_V7
> ...
> > Is there any reason why CPU_FEROCEON is not listed here? FWIW, I've just built
> > (but not really tested) a Kirkwood kernel with CONFIG_HIBERNATION=y.
> No reason; I did not change this from the original patch I'd received.
>  I didn't try to get a comprehensive list of supported hardware.  To
> my understanding, the goal is to get the infrastructure in so that
> people can start working on their platforms and add support for them.
> 

Sure, no problem. If you consider that build-test is enough, feel free to put
CPU_FEROCEON on that list. We added suspend/resume to feroceon not long ago.

> > And is there any reason to put this config in arch/arm/mm/Kconfig, instead of
> > in arch/arm/Kconfig, below ARCH_SUSPEND_POSSIBLE?
> I don't have a reason.  Anyone else have a comment on this?
> Otherwise, I'll move it.. thanks!
>

It looked reasonable to me.

> > I'm also puzzled about having two separate options for suspend and hibernate,
> > maybe someone can explain me why a given CPU would support the former but not
> > the latter?
> It's part of having the generic hibernation implemented and available
> but with architecture specific dependencies.  Where an architecture
> may not have support for hibernation, it will prevent compilation of
> the generic hibernation support.  For example, at the moment, ARM does
> not support hibernation.
[..]

I guess my question wasn't clear. I mean to ask: Are there any other
requirements on an ARM platform to support hibernation, other than
suspend/resume support?

If this is the *only* requirement, it seems to me we could make our
ARCH_SUSPEND_POSSIBLE also select ARCH_HIBERNATION_POSSIBLE.

Does this make sense?
Sebastian Capella March 17, 2014, 10:07 p.m. UTC | #8
Thanks Russell!

On 16 March 2014 02:46, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Mon, Mar 10, 2014 at 11:32:17AM -0700, Sebastian Capella wrote:
...
> Let's look at the implementations...
>
> #define __pa(x)                 __virt_to_phys((unsigned long)(x))
> static inline phys_addr_t __virt_to_phys(unsigned long x)
> {
>         return (phys_addr_t)x - PAGE_OFFSET + PHYS_OFFSET;
> }
>
> So, __pa() returns a phys_addr_t (which may be either 32-bit or 64-bit).
>
> #define virt_to_pfn(kaddr)  (__pa(kaddr) >> PAGE_SHIFT)
>
> And that will do an appropriate shift to convert the phys_addr_t to a PFN,
> which will be fine to assign to an unsigned long variable.
>
> #define __phys_to_pfn(paddr)    ((unsigned long)((paddr) >> PAGE_SHIFT))
>
> This does the same thing, but has an explicit cast.
>
> So, the only difference between the two suggestions is that additional
> cast, which has no effect here.

Sounds good, thanks!

I was a little unsure about the page sizes that could affect all of
the implementations.

I'll make this change and repost, basically using virt_to_pfn in place
of the previous __pa_symbol, and remove unused declarations.

Thanks again!

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sebastian Capella March 17, 2014, 10:39 p.m. UTC | #9
On 17 March 2014 13:44, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> On Mar 17, Sebastian Capella wrote:
>> On 16 March 2014 00:09, Ezequiel Garcia
>> <ezequiel.garcia@free-electrons.com> wrote:
>> > On Mar 05, Sebastian Capella wrote:
>> > [..]
>> >> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
>> >> index 1f8fed9..83707702 100644
>> >> --- a/arch/arm/mm/Kconfig
>> >> +++ b/arch/arm/mm/Kconfig
>> >> @@ -611,6 +611,11 @@ config CPU_USE_DOMAINS
>> >>  config IO_36
>> >>       bool
>> >>
>> >> +config ARCH_HIBERNATION_POSSIBLE
...
>> > Is there any reason why CPU_FEROCEON is not listed here? FWIW, I've just built
>> > (but not really tested) a Kirkwood kernel with CONFIG_HIBERNATION=y.
...
> Sure, no problem. If you consider that build-test is enough, feel free to put
> CPU_FEROCEON on that list. We added suspend/resume to feroceon not long ago.
>> > And is there any reason to put this config in arch/arm/mm/Kconfig, instead of
>> > in arch/arm/Kconfig, below ARCH_SUSPEND_POSSIBLE?
...
> I guess my question wasn't clear. I mean to ask: Are there any other
> requirements on an ARM platform to support hibernation, other than
> suspend/resume support?
>
> If this is the *only* requirement, it seems to me we could make our
> ARCH_SUSPEND_POSSIBLE also select ARCH_HIBERNATION_POSSIBLE.

Thanks, I've added it like this in arch/arm/Kconfig.  I'm sure you
know, but this way also takes care of the CPU_FEROCEON in the default
list since SUSPEND_POSSIBLE already contains it.

config ARCH_HIBERNATION_POSSIBLE
        bool
        depends on MMU
        default y if ARCH_SUSPEND_POSSIBLE

Does this look ok?

Thanks!

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Russell King - ARM Linux April 16, 2014, 10:12 a.m. UTC | #10
On Wed, Mar 05, 2014 at 02:50:05AM -0800, Sebastian Capella wrote:
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 8756e4b..d32adbb 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -291,6 +291,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>   */
>  #define __pa(x)			__virt_to_phys((unsigned long)(x))
>  #define __va(x)			((void *)__phys_to_virt((phys_addr_t)(x)))
> +#define __pa_symbol(x)		__pa((unsigned long)(x))
>  #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)

I don't see the appropriate version on the mailing list, so I'll comment
here instead.  In 8011/1, you added here:

+#define virt_to_pfn(kaddr)      (__pa(kaddr) >> PAGE_SHIFT)

which conflicts with my solution (which fixes some rather horrid assembly
code).  You can find my change as e26a9e00afc4 (ARM: Better virt_to_page()
handling).  I can drop this from your patch, but it would be a good idea
if you could re-validate against v3.15-rc1.

Thanks.
Sebastian Capella April 16, 2014, 4:42 p.m. UTC | #11
On 16 April 2014 03:12, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Wed, Mar 05, 2014 at 02:50:05AM -0800, Sebastian Capella wrote:
>> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
>> index 8756e4b..d32adbb 100644
>> --- a/arch/arm/include/asm/memory.h
>> +++ b/arch/arm/include/asm/memory.h
>> @@ -291,6 +291,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>>   */
>>  #define __pa(x)                      __virt_to_phys((unsigned long)(x))
>>  #define __va(x)                      ((void *)__phys_to_virt((phys_addr_t)(x)))
>> +#define __pa_symbol(x)               __pa((unsigned long)(x))
>>  #define pfn_to_kaddr(pfn)    __va((pfn) << PAGE_SHIFT)
>
> I don't see the appropriate version on the mailing list, so I'll comment
> here instead.  In 8011/1, you added here:
>
> +#define virt_to_pfn(kaddr)      (__pa(kaddr) >> PAGE_SHIFT)
>
> which conflicts with my solution (which fixes some rather horrid assembly
> code).  You can find my change as e26a9e00afc4 (ARM: Better virt_to_page()
> handling).  I can drop this from your patch, but it would be a good idea
> if you could re-validate against v3.15-rc1.

Hi Russell,

Thanks!  I'll validate it with e26a9e00afc4 added to my 3.13
hibernation branch, which includes the OMAP patches, and post the
results.  I can also verify against 3.15-rc1 + hibernation patches,
but there's not much to be done, other than compile test, as the
platform support is not there yet.  I can try rebasing the OMAP stuff
to v3.15-rc1, but I'm not sure what to expect out of that.

In any event, I'll let you know.

Thanks!

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sebastian Capella April 17, 2014, 8:38 p.m. UTC | #12
Hi Russell,

It seems to work fine with your virt_to_phys on the 3.13 + OMAP
patches kernel, and I checked on the 3.15-rc1 kernel + hibernation and
it compiled and ran fine.  I tried a couple of hibernations on this
version as well and they worked (aside from the crash in kernel_halt
we're discussing separately).  I believe this just works by chance
though because the omap platform has no real hibernation support
there.

I'll try now to merge up the omap patches to 3.15-rc1.

Thanks,

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sebastian Capella April 23, 2014, 12:38 a.m. UTC | #13
Hi Russell,

Sorry for the delay.

I've merged the omap changes onto 3.15-rc2 and tested and it looks
like everything is working (so far 50 loops into/out of hibernation).

Thanks!

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Sebastian Capella April 23, 2014, 4:39 p.m. UTC | #14
Hi Russell,

Ran this overnight and no failures on >1000 hibernations.

Looks good.
Thanks!

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 8756e4b..d32adbb 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -291,6 +291,7 @@  static inline void *phys_to_virt(phys_addr_t x)
  */
 #define __pa(x)			__virt_to_phys((unsigned long)(x))
 #define __va(x)			((void *)__phys_to_virt((phys_addr_t)(x)))
+#define __pa_symbol(x)		__pa((unsigned long)(x))
 #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
 
 extern phys_addr_t (*arch_virt_to_idmap)(unsigned long x);
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index a30fc9b..8afa848 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -39,6 +39,7 @@  obj-$(CONFIG_ARTHUR)		+= arthur.o
 obj-$(CONFIG_ISA_DMA)		+= dma-isa.o
 obj-$(CONFIG_PCI)		+= bios32.o isa.o
 obj-$(CONFIG_ARM_CPU_SUSPEND)	+= sleep.o suspend.o
+obj-$(CONFIG_HIBERNATION)	+= hibernate.o
 obj-$(CONFIG_SMP)		+= smp.o
 ifdef CONFIG_MMU
 obj-$(CONFIG_SMP)		+= smp_tlb.o
diff --git a/arch/arm/kernel/hibernate.c b/arch/arm/kernel/hibernate.c
new file mode 100644
index 0000000..656718a
--- /dev/null
+++ b/arch/arm/kernel/hibernate.c
@@ -0,0 +1,108 @@ 
+/*
+ * Hibernation support specific for ARM
+ *
+ * Derived from work on ARM hibernation support by:
+ *
+ * Ubuntu project, hibernation support for mach-dove
+ * Copyright (C) 2010 Nokia Corporation (Hiroshi Doyu)
+ * Copyright (C) 2010 Texas Instruments, Inc. (Teerth Reddy et al.)
+ *  https://lkml.org/lkml/2010/6/18/4
+ *  https://lists.linux-foundation.org/pipermail/linux-pm/2010-June/027422.html
+ *  https://patchwork.kernel.org/patch/96442/
+ *
+ * Copyright (C) 2006 Rafael J. Wysocki <rjw@sisk.pl>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#include <linux/mm.h>
+#include <linux/suspend.h>
+#include <asm/system_misc.h>
+#include <asm/idmap.h>
+#include <asm/suspend.h>
+
+extern const void __nosave_begin, __nosave_end;
+
+int pfn_is_nosave(unsigned long pfn)
+{
+	unsigned long nosave_begin_pfn =
+			__pa_symbol(&__nosave_begin) >> PAGE_SHIFT;
+	unsigned long nosave_end_pfn =
+			PAGE_ALIGN(__pa_symbol(&__nosave_end)) >> PAGE_SHIFT;
+
+	return (pfn >= nosave_begin_pfn) && (pfn < nosave_end_pfn);
+}
+
+void notrace save_processor_state(void)
+{
+	WARN_ON(num_online_cpus() != 1);
+	local_fiq_disable();
+}
+
+void notrace restore_processor_state(void)
+{
+	local_fiq_enable();
+}
+
+/*
+ * Snapshot kernel memory and reset the system.
+ *
+ * swsusp_save() is executed in the suspend finisher so that the CPU
+ * context pointer and memory are part of the saved image, which is
+ * required by the resume kernel image to restart execution from
+ * swsusp_arch_suspend().
+ *
+ * soft_restart is not technically needed, but is used to get success
+ * returned from cpu_suspend.
+ *
+ * When soft reboot completes, the hibernation snapshot is written out.
+ */
+static int notrace arch_save_image(unsigned long unused)
+{
+	int ret;
+
+	ret = swsusp_save();
+	if (ret == 0)
+		soft_restart(virt_to_phys(cpu_resume));
+	return ret;
+}
+
+/*
+ * Save the current CPU state before suspend / poweroff.
+ */
+int notrace swsusp_arch_suspend(void)
+{
+	return cpu_suspend(0, arch_save_image);
+}
+
+/*
+ * Restore page contents for physical pages that were in use during loading
+ * hibernation image.  Switch to idmap_pgd so the physical page tables
+ * are overwritten with the same contents.
+ */
+static void notrace arch_restore_image(void *unused)
+{
+	struct pbe *pbe;
+
+	cpu_switch_mm(idmap_pgd, &init_mm);
+	for (pbe = restore_pblist; pbe; pbe = pbe->next)
+		copy_page(pbe->orig_address, pbe->address);
+
+	soft_restart(virt_to_phys(cpu_resume));
+}
+
+static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
+
+/*
+ * Resume from the hibernation image.
+ * Due to the kernel heap / data restore, stack contents change underneath
+ * and that would make function calls impossible; switch to a temporary
+ * stack within the nosave region to avoid that problem.
+ */
+int swsusp_arch_resume(void)
+{
+	extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
+	call_with_stack(arch_restore_image, 0,
+		resume_stack + ARRAY_SIZE(resume_stack));
+	return 0;
+}
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 1f8fed9..83707702 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -611,6 +611,11 @@  config CPU_USE_DOMAINS
 config IO_36
 	bool
 
+config ARCH_HIBERNATION_POSSIBLE
+	bool
+	depends on MMU
+	default y if CPU_ARM920T || CPU_ARM926T || CPU_SA1100 || CPU_XSCALE || CPU_XSC3 || CPU_V6 || CPU_V6K || CPU_V7
+
 comment "Processor Features"
 
 config ARM_LPAE
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index f73cabf..38bbf95 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -320,6 +320,8 @@  extern unsigned long get_safe_page(gfp_t gfp_mask);
 extern void hibernation_set_ops(const struct platform_hibernation_ops *ops);
 extern int hibernate(void);
 extern bool system_entering_hibernation(void);
+asmlinkage int swsusp_save(void);
+extern struct pbe *restore_pblist;
 #else /* CONFIG_HIBERNATION */
 static inline void register_nosave_region(unsigned long b, unsigned long e) {}
 static inline void register_nosave_region_late(unsigned long b, unsigned long e) {}