BTW, this may apply to other ARM variants as well, but the hibernation
feature should really be made common first of all in that sense.

commit 1f3ebd28c0e8adf7f7a1fc85377a57d8dbc56267
Author: Eric Miao <eric.miao@canonical.com>
Date: Fri Apr 23 14:16:17 2010 +0800

UBUNTU: SAUCE: dove: avoid page table overwrite when resuming from
hibernation

BugLink: http://bugs.launchpad.net/bugs/509006

Resuming from hibernation is OK if 'resume=/dev/sdaX' is explicitly
specified on the kernel command line, but it fails if scripts in
initramfs are used to trigger the resume. It turned out to be page
table being overwritten when restoring the memory content because
it's using a normal user process's page table in the latter case,
which is not safe and could be overwritten. Fix this by using the
safe swapper_pg_dir during restoring.

Signed-off-by: Eric Miao <eric.miao@canonical.com>

diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile
index 0be1e1c..c5c028f 100755
--- a/arch/arm/mach-dove/Makefile
+++ b/arch/arm/mach-dove/Makefile
@@ -1,3 +1,5 @@
+AFLAGS_swsusp.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
+
obj-y += clock.o common.o addr-map.o irq.o pcie.o mpp.o
sdhci_cam_mbus.o
obj-$(CONFIG_MACH_DOVE_RD_AVNG) += dove-rd-avng-setup.o
diff --git a/arch/arm/mach-dove/swsusp.S b/arch/arm/mach-dove/swsusp.S
index 4f4a884..8e308d8 100644
--- a/arch/arm/mach-dove/swsusp.S
+++ b/arch/arm/mach-dove/swsusp.S
@@ -28,6 +28,7 @@
*/

#include <linux/linkage.h>
+#include <asm/memory.h>
#include <asm/segment.h>
#include <asm/page.h>
#include <asm/asm-offsets.h>
@@ -209,8 +210,14 @@ FUNC(swsusp_arch_suspend)

FUNC_END(swsusp_arch_suspend)

+#define KERNEL_RAM_PADDR (PHYS_OFFSET + TEXT_OFFSET)
+
FUNC(swsusp_arch_resume)
/* set page table if needed */
+ ldr r0, =(KERNEL_RAM_PADDR - 0x4000)
+ mcr p15, 0, r0, c2, c0, 0 @ load page table pointer
+ mcr p15, 0, r0, c8, c7, 0 @ invalidate I,D TLBs
+ mcr p15, 0, r0, c7, c5, 4 @ ISB

/*
* retore "nr_copy_pages" pages which are saved and specified

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

Forget about upstream at this moment, since the hibernation support for Dove
is quite specific now, and even it's going to be supported in
upstream, it will have
to made generic so other SoC can benefit. This patch isn't ready for
upstream yet.

On Fri, May 28, 2010 at 7:39 AM, Nigel Cunningham
<ncunningham@crca.org.au> wrote:
> Hi Rafael.
>
> On 28/05/10 09:29, Rafael J. Wysocki wrote:
>>
>> On Thursday 27 May 2010, Nigel Cunningham wrote:
>>>
>>> Hi.
>>>
>>> On 28/05/10 01:35, Tim Gardner wrote:
>>>>
>>>> Test results in the LP report look good. Has this patch been upstreamed?
>>>> I cannot remember from the original discussions a few weeks ago.
>>>
>>> I noticed this thread last night and brought it to Rafael's attention,
>>> because I don't remember seeing anything like this before.
>>
>> I'm not sure what patch is being referred to. *Any details, please?
>
> Sorry - I thought I gave you the url to the bug report in my previous email.
>
> http://bugs.launchpad.net/bugs/509006
>
> To quote from the patch Ubuntu is applying:
>
> Resuming from hibernation is OK if 'resume=/dev/sdaX' is explicitly
> specified on the kernel command line, but it fails if scripts in
> initramfs are used to trigger the resume. It turned out to be page
> table being overwritten when restoring the memory content because
> it's using a normal user process's page table in the latter case,
> which is not safe and could be overwritten. Fix this by using the
> safe swapper_pg_dir during restoring.
>
> It's an arm specific fix (touching arch/arm/mach-dove/swsusp.S and the
> Makefile in the same directory) but I wondered whether this might in any way
> be relevant to vanilla. Perhaps there's something more generic, like a
> failure to set KERNEL_DS in that code path that might be a more correct fix
> and might also be relevant to other architectures. (Not that I've seen any
> failures with x86/x86_64) from an initramfs). Hmm - could be. I explicitly
> set KERNEL_DS in TuxOnIce, but don't see it done anywhere (from a quick
> check) in vanilla.
>
> Regards,
>
> Nigel
>

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

On Fri, May 28, 2010 at 12:20 PM, Nigel Cunningham
<ncunningham@crca.org.au> wrote:
> Hi Eric.
>
> On 28/05/10 14:13, Eric Miao wrote:
>>
>> Forget about upstream at this moment, since the hibernation support for
>> Dove
>> is quite specific now, and even it's going to be supported in
>> upstream, it will have
>> to made generic so other SoC can benefit. This patch isn't ready for
>> upstream yet.
>
> I understand that the arch isn't in upstream. I think that the issue is
> though. If I'm right, you're seeing this issue not because of a problem in
> your arch specific code, but because of a problem in vanilla that's more
> easily triggered on your arch. That's why I wanted to bring it to Rafael's
> attention.
>

Ah right. I seem to remember the x86 has a workaround to avoid the issue of
page table being overwritten by copying back-n-forth. I'm not sure if there are
any specific way of doing so. In ARM case, temporarily using the init page
table seems to be a workable solution, though.

> Regards,
>
> Nigel
>

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

On Thu, May 27, 2010 at 11:35 PM, Tim Gardner <tim.gardner@canonical.com> wrote:
> On 05/27/2010 09:03 AM, Eric Miao wrote:
>>
>> *From 1ae0436f8c2159936ca42d9ef938055b68af64f7 Mon Sep 17 00:00:00 2001
>> From: Eric Miao<eric.miao@canonical.com>
>> Date: Fri, 23 Apr 2010 14:16:17 +0800
>> Subject: [PATCH] UBUNTU: SAUCE: dove: avoid page table overwrite when
>> resuming from hibernation
>>
>> BugLink: http://bugs.launchpad.net/bugs/509006
>>
>> Resuming from hibernation is OK if 'resume=/dev/sdaX' is explicitly
>> specified on the kernel command line, but it fails if scripts in
>> initramfs are used to trigger the resume. It turned out to be page
>> table being overwritten when restoring the memory content because
>> it's using a normal user process's page table in the latter case,
>> which is not safe and could be overwritten. Fix this by using the
>> safe swapper_pg_dir during restoring.
>>
>> Signed-off-by: Eric Miao<eric.miao@canonical.com>
>> ---
>>
>> It was posted weeks ago, thought it was merged but not. This is an updated
>> version addressing the previous comments to make it cleaner.
>>
>> *arch/arm/mach-dove/Makefile | * *2 ++
>> *arch/arm/mach-dove/swsusp.S | * *8 ++++++++
>> *2 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile
>> index 0be1e1c..c5c028f 100755
>> --- a/arch/arm/mach-dove/Makefile
>> +++ b/arch/arm/mach-dove/Makefile
>> @@ -1,3 +1,5 @@
>> +AFLAGS_swsusp.o * * * * * * * * * * * *:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>> +
>> *obj-y * * * * * * * * * * * * += clock.o common.o addr-map.o irq.o pcie.o
>> mpp.o
>> * * * * * * * * * * * * * * * *sdhci_cam_mbus.o
>> *obj-$(CONFIG_MACH_DOVE_RD_AVNG) * * * += dove-rd-avng-setup.o
>> diff --git a/arch/arm/mach-dove/swsusp.S b/arch/arm/mach-dove/swsusp.S
>> index 4f4a884..9b752a0 100644
>> --- a/arch/arm/mach-dove/swsusp.S
>> +++ b/arch/arm/mach-dove/swsusp.S
>> @@ -28,6 +28,7 @@
>> * */
>>
>> *#include<linux/linkage.h>
>> +#include<asm/memory.h>
>> *#include<asm/segment.h>
>> *#include<asm/page.h>
>> *#include<asm/asm-offsets.h>
>> @@ -209,8 +210,15 @@ FUNC(swsusp_arch_suspend)
>>
>> *FUNC_END(swsusp_arch_suspend)
>>
>> +#define KERNEL_RAM_PADDR * * * (PHYS_OFFSET + TEXT_OFFSET)
>> +#define SWAPPER_PG_DIR * * * * (KERNEL_RAM_PADDR - 0x4000)
>> +
>> *FUNC(swsusp_arch_resume)
>> * * * */* set page table if needed */
>> + * * * ldr * * r0, =SWAPPER_PG_DIR
>> + * * * mcr * * p15, 0, r0, c2, c0, 0 * * * * * @ load page table pointer
>> + * * * mcr * * p15, 0, r0, c8, c7, 0 * * * * * @ invalidate I,D TLBs
>> + * * * mcr * * p15, 0, r0, c7, c5, 4 * * * * * @ ISB
>>
>> * * * */*
>> * * * * * retore "nr_copy_pages" pages which are saved and specified
>
> Test results in the LP report look good. Has this patch been upstreamed? I
> cannot remember from the original discussions a few weeks ago.
>
> Acked-by: Tim Gardner <tim.gardner@canonical.com>
>

Andy,

Ping?

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

On Mon, May 31, 2010 at 3:44 PM, Stefan Bader
<stefan.bader@canonical.com> wrote:
> On 05/31/2010 03:50 AM, Eric Miao wrote:
>> On Thu, May 27, 2010 at 11:35 PM, Tim Gardner <tim.gardner@canonical.com> wrote:
>>> On 05/27/2010 09:03 AM, Eric Miao wrote:
>>>>
>>>> *From 1ae0436f8c2159936ca42d9ef938055b68af64f7 Mon Sep 17 00:00:00 2001
>>>> From: Eric Miao<eric.miao@canonical.com>
>>>> Date: Fri, 23 Apr 2010 14:16:17 +0800
>>>> Subject: [PATCH] UBUNTU: SAUCE: dove: avoid page table overwrite when
>>>> resuming from hibernation
>>>>
>>>> BugLink: http://bugs.launchpad.net/bugs/509006
>>>>
>>>> Resuming from hibernation is OK if 'resume=/dev/sdaX' is explicitly
>>>> specified on the kernel command line, but it fails if scripts in
>>>> initramfs are used to trigger the resume. It turned out to be page
>>>> table being overwritten when restoring the memory content because
>>>> it's using a normal user process's page table in the latter case,
>>>> which is not safe and could be overwritten. Fix this by using the
>>>> safe swapper_pg_dir during restoring.
>>>>
>>>> Signed-off-by: Eric Miao<eric.miao@canonical.com>
>>>> ---
>>>>
>>>> It was posted weeks ago, thought it was merged but not. This is an updated
>>>> version addressing the previous comments to make it cleaner.
>>>>
>>>> *arch/arm/mach-dove/Makefile | * *2 ++
>>>> *arch/arm/mach-dove/swsusp.S | * *8 ++++++++
>>>> *2 files changed, 10 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile
>>>> index 0be1e1c..c5c028f 100755
>>>> --- a/arch/arm/mach-dove/Makefile
>>>> +++ b/arch/arm/mach-dove/Makefile
>>>> @@ -1,3 +1,5 @@
>>>> +AFLAGS_swsusp.o * * * * * * * * * * * *:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>>>> +
>>>> *obj-y * * * * * * * * * * * * += clock.o common.o addr-map.o irq.o pcie.o
>>>> mpp.o
>>>> * * * * * * * * * * * * * * * *sdhci_cam_mbus.o
>>>> *obj-$(CONFIG_MACH_DOVE_RD_AVNG) * * * += dove-rd-avng-setup.o
>>>> diff --git a/arch/arm/mach-dove/swsusp.S b/arch/arm/mach-dove/swsusp.S
>>>> index 4f4a884..9b752a0 100644
>>>> --- a/arch/arm/mach-dove/swsusp.S
>>>> +++ b/arch/arm/mach-dove/swsusp.S
>>>> @@ -28,6 +28,7 @@
>>>> * */
>>>>
>>>> *#include<linux/linkage.h>
>>>> +#include<asm/memory.h>
>>>> *#include<asm/segment.h>
>>>> *#include<asm/page.h>
>>>> *#include<asm/asm-offsets.h>
>>>> @@ -209,8 +210,15 @@ FUNC(swsusp_arch_suspend)
>>>>
>>>> *FUNC_END(swsusp_arch_suspend)
>>>>
>>>> +#define KERNEL_RAM_PADDR * * * (PHYS_OFFSET + TEXT_OFFSET)
>>>> +#define SWAPPER_PG_DIR * * * * (KERNEL_RAM_PADDR - 0x4000)
>>>> +
>>>> *FUNC(swsusp_arch_resume)
>>>> * * * */* set page table if needed */
>>>> + * * * ldr * * r0, =SWAPPER_PG_DIR
>>>> + * * * mcr * * p15, 0, r0, c2, c0, 0 * * * * * @ load page table pointer
>>>> + * * * mcr * * p15, 0, r0, c8, c7, 0 * * * * * @ invalidate I,D TLBs
>>>> + * * * mcr * * p15, 0, r0, c7, c5, 4 * * * * * @ ISB
>>>>
>>>> * * * */*
>>>> * * * * * retore "nr_copy_pages" pages which are saved and specified
>>>
>>> Test results in the LP report look good. Has this patch been upstreamed? I
>>> cannot remember from the original discussions a few weeks ago.
>>>
>>> Acked-by: Tim Gardner <tim.gardner@canonical.com>
>>>
>>
>> Andy,
>>
>> Ping?
>
> Wrong person pinged. This is for Lucid, right? I try to use the quiteness today
> to get through all pending and ready SRUs.
>

Hi Stefan,

Thanks for looking into this. And sorry for the noise.

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team