FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Kernel Team

 
 
LinkBack Thread Tools
 
Old 04-27-2010, 10:08 AM
Stefan Bader
 
Default UBUNTU: SAUCE: dove: avoid page table overwrite when resuming from hibernation

Eric Miao wrote:
> On Sat, Apr 24, 2010 at 12:40 AM, John Johansen
> <john.johansen@canonical.com> wrote:
>> On 04/23/2010 07:18 AM, Stefan Bader wrote:
>>> Eric Miao wrote:
>>>> 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
>>>>
>>> The patch probably is correct, but from the reviewing point of view I do not
>>> know how I make the connection between swapper_pg_dir in the comment and the
>>> patch itself. Is that only me?
>>>
>> No, I also would like to see a little more information.
>>
>
> Ah, sorry for the confusion. swapper_pg_dir is the initial page table for
> ARM kernel, which is physically located at KERNEL_RAM_PADDR - 0x4000.
> I should have made it clear by a macro or something. The following code is
> excerpted from arch/arm/kernel/head.S:
>
> /*
> * swapper_pg_dir is the virtual address of the initial page table.
> * We place the page tables 16K below KERNEL_RAM_VADDR. Therefore, we must
> * make sure that KERNEL_RAM_VADDR is correctly set. Currently, we expect
> * the least significant 16 bits to be 0x8000, but we could probably
> * relax this restriction to KERNEL_RAM_VADDR >= PAGE_OFFSET + 0x4000.
> */
> #if (KERNEL_RAM_VADDR & 0xffff) != 0x8000
> #error KERNEL_RAM_VADDR must start at 0xXXXX8000
> #endif
>
> .globl swapper_pg_dir
> .equ swapper_pg_dir, KERNEL_RAM_VADDR - 0x4000
>
> .macro pgtbl, rd
> ldr
d, =(KERNEL_RAM_PADDR - 0x4000)
> .endm

Hm, wouldn't it then be possible to make the patch actually use the macro name
instead of the hard coded values?

Stefan

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 04-28-2010, 05:56 AM
Bryan Wu
 
Default UBUNTU: SAUCE: dove: avoid page table overwrite when resuming from hibernation

On 04/28/2010 11:00 AM, Eric Miao wrote:
>> Hm, wouldn't it then be possible to make the patch actually use the macro name
>> instead of the hard coded values?
>>
>
> How about the patch below, making thing a little bit clearer?
>
> commit ee5c472440e089eba863689c446adb5f1c0ae05e
> 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)

How about using the pgtbl marco as you posted in previous email?

+ pgtbl r0 @ page table address

I think Stephan was asking for this. But if 'pgtbl' is not available in this ASM
file, I am OK with this patch.

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

Thanks,
--
Bryan Wu <bryan.wu@canonical.com>
Kernel Developer +86.138-1617-6545 Mobile
Ubuntu Kernel Team | Hardware Enablement Team
Canonical Ltd. www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com

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

Thread Tools




All times are GMT. The time now is 06:01 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org