[PATCH lucid] UBUNTU: SAUCE: dove: avoid page table overwrite when resuming from hibernation

[Stefan Bader]

Eric Miao wrote:
> On Sat, Apr 24, 2010 at 12:40 AM, John Johansen
> <john.johansen at 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 at 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 at 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	\rd, =(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