On Wednesday, 22 August 2007 18:17, Rafael J. Wysocki wrote:
> On Wednesday, 22 August 2007 10:28, Pavel Machek wrote:
> > Hi!
> > 
> > > From: Rafael J. Wysocki <[EMAIL PROTECTED]>
> > > 
> > > Since we already pass the address of restore_registers() in the image 
> > > header,
> > > we can also pass the value of the CR3 register from before the 
> > > hibernation in
> > > the same way.  This will allow us to avoid using init_level4_pgt page 
> > > tables
> > > during the restore.
> > > 
> > > Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
> > 
> > > @@ -253,10 +262,13 @@ int arch_hibernation_header_save(void *a
> > >  {
> > >   struct restore_data_record *rdr = addr;
> > >  
> > > - if (max_size < sizeof(struct restore_data_record))
> > > + if (max_size < sizeof(*rdr))
> > >           return -EOVERFLOW;
> > >   rdr->jump_address = restore_jump_address;
> > > - rdr->control = (restore_jump_address ^ RESTORE_MAGIC);
> > > + rdr->cr3 = restore_cr3;
> > > + rdr->magic = RESTORE_MAGIC;
> > > + rdr->crc = 0;
> > > + rdr->crc = crc32_le(0, addr, sizeof(*rdr));
> > >   return 0;
> > >  }
> > 
> > No, I do not think I like that. I believe both -> control and -> crc
> > is just useless paranoia. Bitflip in this area is not going to be any
> > worse than bitflip anywhere else, we should not pretend this is
> > somehow "more important".
> > 
> > -> control should really be "protocol version"... probably should
> > contain some field that is easy to increment.
> 
> OK
> 
> Perhaps I'll just remove the crc field.

Like in the patch below.

---
From: Rafael J. Wysocki <[EMAIL PROTECTED]>

Since we already pass the address of restore_registers() in the image header,
we can also pass the value of the CR3 register from before the hibernation in
the same way.  This will allow us to avoid using init_level4_pgt page tables
during the restore.

Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
---
 arch/x86_64/kernel/suspend.c     |   17 +++++++++++++----
 arch/x86_64/kernel/suspend_asm.S |    8 +++++---
 2 files changed, 18 insertions(+), 7 deletions(-)

Index: linux-2.6.23-rc3/arch/x86_64/kernel/suspend.c
===================================================================
--- linux-2.6.23-rc3.orig/arch/x86_64/kernel/suspend.c
+++ linux-2.6.23-rc3/arch/x86_64/kernel/suspend.c
@@ -9,6 +9,7 @@
 
 #include <linux/smp.h>
 #include <linux/suspend.h>
+#include <linux/crc32.h>
 #include <asm/proto.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
@@ -150,6 +151,12 @@ extern int restore_image(void);
  */
 unsigned long restore_jump_address;
 
+/*
+ * Value of the cr3 register from before the hibernation (this value is passed
+ * in the image header).
+ */
+unsigned long restore_cr3;
+
 pgd_t *temp_level4_pgt;
 
 static int res_phys_pud_init(pud_t *pud, unsigned long address, unsigned long 
end)
@@ -239,7 +246,8 @@ int pfn_is_nosave(unsigned long pfn)
 
 struct restore_data_record {
        unsigned long jump_address;
-       unsigned long control;
+       unsigned long cr3;
+       unsigned long magic;
 };
 
 #define RESTORE_MAGIC  0x0123456789ABCDEFUL
@@ -256,7 +264,8 @@ int arch_hibernation_header_save(void *a
        if (max_size < sizeof(struct restore_data_record))
                return -EOVERFLOW;
        rdr->jump_address = restore_jump_address;
-       rdr->control = (restore_jump_address ^ RESTORE_MAGIC);
+       rdr->cr3 = restore_cr3;
+       rdr->magic = RESTORE_MAGIC;
        return 0;
 }
 
@@ -270,7 +279,7 @@ int arch_hibernation_header_restore(void
        struct restore_data_record *rdr = addr;
 
        restore_jump_address = rdr->jump_address;
-       return (rdr->control == (restore_jump_address ^ RESTORE_MAGIC)) ?
-                       0 : -EINVAL;
+       restore_cr3 = rdr->cr3;
+       return (rdr->magic == RESTORE_MAGIC) ? 0 : -EINVAL;
 }
 #endif /* CONFIG_HIBERNATION */
Index: linux-2.6.23-rc3/arch/x86_64/kernel/suspend_asm.S
===================================================================
--- linux-2.6.23-rc3.orig/arch/x86_64/kernel/suspend_asm.S
+++ linux-2.6.23-rc3/arch/x86_64/kernel/suspend_asm.S
@@ -39,6 +39,9 @@ ENTRY(swsusp_arch_suspend)
        /* save the address of restore_registers */
        movq    $restore_registers, %rax
        movq    %rax, restore_jump_address(%rip)
+       /* save cr3 */
+       movq    %cr3, %rax
+       movq    %rax, restore_cr3(%rip)
 
        call swsusp_save
        ret
@@ -66,6 +69,7 @@ ENTRY(restore_image)
 
        /* prepare to jump to the image kernel */
        movq    restore_jump_address(%rip), %rax
+       movq    restore_cr3(%rip), %rbx
 
        /* copy image data to their original locations */
        xorq    %r10, %r10
@@ -116,9 +120,7 @@ ENTRY(restore_registers)
        movsq
 
        /* go back to the original page tables */
-1:     movq    $(init_level4_pgt - __START_KERNEL_map), %rax
-       addq    phys_base(%rip), %rax
-       movq    %rax, %cr3
+1:     movq    %rbx, %cr3
 
        /* Flush TLB, including "global" things (vmalloc) */
        movq    mmu_cr4_features(%rip), %rax
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to