Backport several fixes from 64bits to 32bits hibernation

RFC PATCH Superseded Chen Yu 4 participants 32 messages Sep 19, 2018 – Sep 20, 2018

Subsystems (2)

x86 Hibernation

lore

Share & tools

lore

Chen Yu

author

2018-09-19 07:30 UTC #0 lore raw

v3 cover Backport several fixes from 64bits to 32bits hibernation

Currently there are mainly three bugs in 32bits system when doing hibernation:
1. The page copy code is not running in safe page, which might cause hang during resume.
2. There's no text mapping for the final jump address
of the original kernel, which might cause the system jumping into illegal address and causes system hang during resume.
3. The restore kernel switches to its own kernel page table(swapper_pg_dir) rather than the original kernel page table after all the pages been copied back, which might cause invalid virtual-physical mapping issue during resume.

To solve these problems:

1. Copy the code core_restore_code to a safe page, to avoid the instruction code been overwritten when image kernel pages are being copied.
2. Set up temporary text mapping for the image kernel's jump address, so that after all the pages have been copied back, the system could jump to this address.
3. Switch to the original kernel page table during resume.

Furthermore, MD5 hash check for e820 map is also backported from 64bits system.

In order to make this patch set more readable, these fixes are splitted into several sub patches.

Chen Yu (1):
PM / hibernate: Check the success of generating md5 digest before
hibernation

Zhimin Gu (11):
x86, hibernate: Fix nosave_regions setup for hibernation x86/asm/power: Create stack frames in hibernate_asm_32.S x86, hibernate: Extract the common code of 64/32 bit system x86, hibernate: Enable CONFIG_ARCH_HIBERNATION_HEADER set on 32bit
system
x86, hibernate: Rename temp_level4_pgt to temp_pgt
x86, hibernate: Use temp_pgt as the temporary page table x86, hibernate: Use the page size macro instead of constant value x86, hibernate: Switch to original page table after resumed x86, hibernate: Switch to relocated restore code during resume on
32bit system
x86, hibernate: Set up temporary text mapping for 32bit system x86, hibernate: Adjust in_suspend after resumed on 32bit system

 arch/x86/Kconfig                  |   2 +-
 arch/x86/include/asm/suspend.h    |   8 +
 arch/x86/include/asm/suspend_32.h |   4 +
 arch/x86/kernel/setup.c           |   2 +-
 arch/x86/power/Makefile           |   2 +-
 arch/x86/power/hibernate.c        | 249 ++++++++++++++++++++++++++++++
 arch/x86/power/hibernate_32.c     |  52 +++++--
 arch/x86/power/hibernate_64.c     | 224 +--------------------------
 arch/x86/power/hibernate_asm_32.S |  37 ++++-
 arch/x86/power/hibernate_asm_64.S |   2 +-
 10 files changed, 335 insertions(+), 247 deletions(-)
 create mode 100644 arch/x86/power/hibernate.c

Signature · 1 line

2.17.1

Chen Yu

author

2018-09-19 07:30 UTC #1 lore raw

RFC PATCH v3 1/12 x86, hibernate: Fix nosave_regions setup for hibernation

From: Zhimin Gu <[email protected]>

On 32bit systems, nosave_regions(non RAM areas) located between max_low_pfn and max_pfn are not excluded from hibernation snapshot currently, which may result in a machine check exception when trying to access these unsafe regions during hibernation:

[  612.800453] Disabling lock debugging due to kernel taint
[  612.805786] mce: [Hardware Error]: CPU 0: Machine Check Exception: 5 Bank 6: fe00000000801136
[  612.814344] mce: [Hardware Error]: RIP !INEXACT! 60:<00000000d90be566> {swsusp_save+0x436/0x560}
[  612.823167] mce: [Hardware Error]: TSC 1f5939fe276 ADDR dd000000 MISC 30e0000086
[  612.830677] mce: [Hardware Error]: PROCESSOR 0:306c3 TIME 1529487426 SOCKET 0 APIC 0 microcode 24
[  612.839581] mce: [Hardware Error]: Run the above through 'mcelog --ascii'
[  612.846394] mce: [Hardware Error]: Machine check: Processor context corrupt
[  612.853380] Kernel panic - not syncing: Fatal machine check
[  612.858978] Kernel Offset: 0x18000000 from 0xc1000000 (relocation range: 0xc0000000-0xf7ffdfff)

This is because on 32bit systems, pages above max_low_pfn are regarded as high memeory, and accessing unsafe pages might cause expected MCE. On the problematic 32bit system, there are reserved memory above low memory, which triggered the MCE:

e820 memory mapping:

[    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009d7ff] usable
[    0.000000] BIOS-e820: [mem 0x000000000009d800-0x000000000009ffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000000100000-0x00000000d160cfff] usable
[    0.000000] BIOS-e820: [mem 0x00000000d160d000-0x00000000d1613fff] ACPI NVS
[    0.000000] BIOS-e820: [mem 0x00000000d1614000-0x00000000d1a44fff] usable
[    0.000000] BIOS-e820: [mem 0x00000000d1a45000-0x00000000d1ecffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000d1ed0000-0x00000000d7eeafff] usable
[    0.000000] BIOS-e820: [mem 0x00000000d7eeb000-0x00000000d7ffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000d8000000-0x00000000d875ffff] usable
[    0.000000] BIOS-e820: [mem 0x00000000d8760000-0x00000000d87fffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000d8800000-0x00000000d8fadfff] usable
[    0.000000] BIOS-e820: [mem 0x00000000d8fae000-0x00000000d8ffffff] ACPI data
[    0.000000] BIOS-e820: [mem 0x00000000d9000000-0x00000000da71bfff] usable
[    0.000000] BIOS-e820: [mem 0x00000000da71c000-0x00000000da7fffff] ACPI NVS
[    0.000000] BIOS-e820: [mem 0x00000000da800000-0x00000000dbb8bfff] usable
[    0.000000] BIOS-e820: [mem 0x00000000dbb8c000-0x00000000dbffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000dd000000-0x00000000df1fffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000f8000000-0x00000000fbffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fec00000-0x00000000fec00fff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fed00000-0x00000000fed03fff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fee00000-0x00000000fee00fff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000041edfffff] usable

Fix this problem by changing pfn limit from max_low_pfn to max_pfn. This fix does not impact 64bit system because on 64bit max_low_pfn is the same as max_pfn.

Acked-by Pavel Machek <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Cc Thomas Gleixner <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/kernel/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/kernel/setup.c +1 -1

index b4866badb235..90ecc108bc8a 100644
@@ -1251,7 +1251,7 @@ void __init setup_arch(char **cmdline_p)
 	x86_init.hyper.guest_late_init();
 
 	e820__reserve_resources();
-	e820__register_nosave_regions(max_low_pfn);
+	e820__register_nosave_regions(max_pfn);
 
 	x86_init.resources.reserve_resources();

Signature · 1 line

2.17.1

Rafael J. Wysocki

maintainer Reviewed-by on v3 1/12

2018-09-19 08:34 UTC #2 lore raw

Re: [PATCH 01/12][RFC v3] x86, hibernate: Fix nosave_regions setup for hibernation

Quoted 68 lines · Chen Yu

From: Zhimin Gu <[email protected]>

[  612.800453] Disabling lock debugging due to kernel taint
[  612.805786] mce: [Hardware Error]: CPU 0: Machine Check Exception: 5 Bank 6: fe00000000801136
[  612.814344] mce: [Hardware Error]: RIP !INEXACT! 60:<00000000d90be566> {swsusp_save+0x436/0x560}
[  612.823167] mce: [Hardware Error]: TSC 1f5939fe276 ADDR dd000000 MISC 30e0000086
[  612.830677] mce: [Hardware Error]: PROCESSOR 0:306c3 TIME 1529487426 SOCKET 0 APIC 0 microcode 24
[  612.839581] mce: [Hardware Error]: Run the above through 'mcelog --ascii'
[  612.846394] mce: [Hardware Error]: Machine check: Processor context corrupt
[  612.853380] Kernel panic - not syncing: Fatal machine check
[  612.858978] Kernel Offset: 0x18000000 from 0xc1000000 (relocation range: 0xc0000000-0xf7ffdfff)

e820 memory mapping:

[    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009d7ff] usable
[    0.000000] BIOS-e820: [mem 0x000000000009d800-0x000000000009ffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000000100000-0x00000000d160cfff] usable
[    0.000000] BIOS-e820: [mem 0x00000000d160d000-0x00000000d1613fff] ACPI NVS
[    0.000000] BIOS-e820: [mem 0x00000000d1614000-0x00000000d1a44fff] usable
[    0.000000] BIOS-e820: [mem 0x00000000d1a45000-0x00000000d1ecffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000d1ed0000-0x00000000d7eeafff] usable
[    0.000000] BIOS-e820: [mem 0x00000000d7eeb000-0x00000000d7ffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000d8000000-0x00000000d875ffff] usable
[    0.000000] BIOS-e820: [mem 0x00000000d8760000-0x00000000d87fffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000d8800000-0x00000000d8fadfff] usable
[    0.000000] BIOS-e820: [mem 0x00000000d8fae000-0x00000000d8ffffff] ACPI data
[    0.000000] BIOS-e820: [mem 0x00000000d9000000-0x00000000da71bfff] usable
[    0.000000] BIOS-e820: [mem 0x00000000da71c000-0x00000000da7fffff] ACPI NVS
[    0.000000] BIOS-e820: [mem 0x00000000da800000-0x00000000dbb8bfff] usable
[    0.000000] BIOS-e820: [mem 0x00000000dbb8c000-0x00000000dbffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000dd000000-0x00000000df1fffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000f8000000-0x00000000fbffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fec00000-0x00000000fec00fff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fed00000-0x00000000fed03fff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fee00000-0x00000000fee00fff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000041edfffff] usable

Fix this problem by changing pfn limit from max_low_pfn to max_pfn. This fix does not impact 64bit system because on 64bit max_low_pfn is the same as max_pfn.

Acked-by Pavel Machek <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Cc Thomas Gleixner <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/kernel/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/kernel/setup.c +0 -0

index b4866badb235..90ecc108bc8a 100644

arch/x86/kernel/setup.c void __init setup_arch(char **cmdline_p)

Quoted 9 lines

arch/x86/kernel/setup.c +1 -2

@@ -1251,7 +1251,7 @@ void __init setup_arch(char **cmdline_p)
        x86_init.hyper.guest_late_init();
 
        e820__reserve_resources();
-       e820__register_nosave_regions(max_low_pfn);
+       e820__register_nosave_regions(max_pfn);
 
        x86_init.resources.reserve_resources();
 
--

Reviewed-by Rafael J. Wysocki <[email protected]>

Pavel Machek

2018-09-20 22:05 UTC #3 lore raw

Re: [PATCH 01/12][RFC v3] x86, hibernate: Fix nosave_regions setup for hibernation

In reply to Chen Yu

Quoted 22 lines · Chen Yu

From: Zhimin Gu <[email protected]>
On 32bit systems, nosave_regions(non RAM areas) located between max_low_pfn and max_pfn are not excluded from hibernation snapshot currently, which may result in a machine check exception when trying to access these unsafe regions during hibernation:
[  612.800453] Disabling lock debugging due to kernel taint
[  612.805786] mce: [Hardware Error]: CPU 0: Machine Check Exception: 5 Bank 6: fe00000000801136
[  612.814344] mce: [Hardware Error]: RIP !INEXACT! 60:<00000000d90be566> {swsusp_save+0x436/0x560}
[  612.823167] mce: [Hardware Error]: TSC 1f5939fe276 ADDR dd000000 MISC 30e0000086
[  612.830677] mce: [Hardware Error]: PROCESSOR 0:306c3 TIME 1529487426 SOCKET 0 APIC 0 microcode 24
[  612.839581] mce: [Hardware Error]: Run the above through 'mcelog --ascii'
[  612.846394] mce: [Hardware Error]: Machine check: Processor context corrupt
[  612.853380] Kernel panic - not syncing: Fatal machine check
[  612.858978] Kernel Offset: 0x18000000 from 0xc1000000 (relocation range: 0xc0000000-0xf7ffdfff)
This is because on 32bit systems, pages above max_low_pfn are regarded as high memeory, and accessing unsafe pages might cause expected MCE. On the problematic 32bit system, there are reserved memory above low memory, which triggered the MCE:

Quoted 5 lines

Fix this problem by changing pfn limit from max_low_pfn to max_pfn. This fix does not impact 64bit system because on 64bit max_low_pfn is the same as max_pfn.
Acked-by Pavel Machek <[email protected]>

Thanks.

I think this is nasty enough to add Cc: stable.
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Chen Yu

author

2018-09-19 07:31 UTC #4 lore raw

RFC PATCH v3 2/12 PM / hibernate: Check the success of generating md5 digest before hibernation

In reply to Chen Yu

Currently if get_e820_md5() fails, then it will hibernate nevertheless. Actually the error code should be propagated to upper caller so that the hibernation could be aware of the result and terminates the process if md5 digest fails.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/power/hibernate_64.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

arch/x86/power/hibernate_64.c +5 -6

index f8e3b668d20b..e0de959ad02d 100644
@@ -265,9 +265,9 @@ static int get_e820_md5(struct e820_table *table, void *buf)
 	return ret;
 }
 
-static void hibernation_e820_save(void *buf)
+static int hibernation_e820_save(void *buf)
 {
-	get_e820_md5(e820_table_firmware, buf);
+	return get_e820_md5(e820_table_firmware, buf);
 }
 
 static bool hibernation_e820_mismatch(void *buf)

@@ -287,8 +287,9 @@ static bool hibernation_e820_mismatch(void *buf)
 	return memcmp(result, buf, MD5_DIGEST_SIZE) ? true : false;
 }
 #else
-static void hibernation_e820_save(void *buf)
+static int hibernation_e820_save(void *buf)
 {
+	return 0;
 }
 
 static bool hibernation_e820_mismatch(void *buf)

@@ -333,9 +334,7 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size)
 
 	rdr->magic = RESTORE_MAGIC;
 
-	hibernation_e820_save(rdr->e820_digest);
-
-	return 0;
+	return hibernation_e820_save(rdr->e820_digest);
 }
 
 /**

Signature · 1 line

2.17.1

Rafael J. Wysocki

maintainer Reviewed-by on v3 2/12

2018-09-19 08:36 UTC #5 lore raw

Re: [PATCH 02/12][RFC v3] PM / hibernate: Check the success of generating md5 digest before hibernation

Quoted 42 lines · Chen Yu

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/power/hibernate_64.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

arch/x86/power/hibernate_64.c +4 -3

index f8e3b668d20b..e0de959ad02d 100644
@@ -265,9 +265,9 @@ static int get_e820_md5(struct e820_table *table, void *buf)
        return ret;
 }
 
-static void hibernation_e820_save(void *buf)
+static int hibernation_e820_save(void *buf)
 {
-       get_e820_md5(e820_table_firmware, buf);
+       return get_e820_md5(e820_table_firmware, buf);
 }
 
 static bool hibernation_e820_mismatch(void *buf)

@@ -287,8 +287,9 @@ static bool hibernation_e820_mismatch(void *buf)
        return memcmp(result, buf, MD5_DIGEST_SIZE) ? true : false;
 }
 #else
-static void hibernation_e820_save(void *buf)
+static int hibernation_e820_save(void *buf)
 {
+       return 0;
 }
 
 static bool hibernation_e820_mismatch(void *buf)

@@ -333,9 +334,7 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size)

arch/x86/power/hibernate_64.c int arch_hibernation_header_save(void *addr, unsigned int max_size)

Quoted 10 lines

arch/x86/power/hibernate_64.c +1 -4

        rdr->magic = RESTORE_MAGIC;
 
-       hibernation_e820_save(rdr->e820_digest);
-
-       return 0;
+       return hibernation_e820_save(rdr->e820_digest);
 }
 
 /**
--

Reviewed-by Rafael J. Wysocki <[email protected]>

Pavel Machek

Acked-by on v3 2/12

2018-09-20 22:05 UTC #6 lore raw

Re: [PATCH 02/12][RFC v3] PM / hibernate: Check the success of generating md5 digest before hibernation

In reply to Chen Yu

Quoted 8 lines · Chen Yu

Currently if get_e820_md5() fails, then it will hibernate nevertheless. Actually the error code should be propagated to upper caller so that the hibernation could be aware of the result and terminates the process if md5 digest fails.
Suggested-by Thomas Gleixner <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Signed-off-by Chen Yu <[email protected]>

Acked-by Pavel Machek <[email protected]>

Quoted 3 lines

---
 arch/x86/power/hibernate_64.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Chen Yu

author

2018-09-19 07:31 UTC #7 lore raw

RFC PATCH v3 3/12 x86-32/asm/power: Create stack frames in hibernate_asm_32.S

In reply to Chen Yu

From: Zhimin Gu <[email protected]>

Backport
Commit ef0f3ed5a4ac (x86/asm/power: Create stack frames in hibernate_asm_64.S)
and
Commit 4ce827b4cc58 (x86/power/64: Fix hibernation return address corruption)

And also prepare for relocated code and jump address mapping.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/include/asm/suspend_32.h | 4 ++++
 arch/x86/power/hibernate_asm_32.S | 9 +++++++++
 2 files changed, 13 insertions(+)

arch/x86/include/asm/suspend_32.h +4 -0

index 8be6afb58471..fdbd9d7b7bca 100644
@@ -32,4 +32,8 @@ struct saved_context {
 	unsigned long return_address;
 } __attribute__((packed));
 
+/* routines for saving/restoring kernel state */
+extern char core_restore_code[];
+extern char restore_registers[];
+
 #endif /* _ASM_X86_SUSPEND_32_H */

arch/x86/power/hibernate_asm_32.S +9 -0

index 6e56815e13a0..671d38d0d931 100644
@@ -12,6 +12,7 @@
 #include <asm/page_types.h>
 #include <asm/asm-offsets.h>
 #include <asm/processor-flags.h>
+#include <asm/frame.h>
 
 .text

@@ -24,8 +25,11 @@ ENTRY(swsusp_arch_suspend)
 	pushfl
 	popl saved_context_eflags
 
+	FRAME_BEGIN
 	call swsusp_save
+	FRAME_END
 	ret
+ENDPROC(swsusp_arch_suspend)
 
 ENTRY(restore_image)
 	movl	mmu_cr4_features, %ecx

@@ -58,6 +62,10 @@ copy_loop:
 	.p2align 4,,7
 
 done:
+
+	/* code below belongs to the image kernel */
+	.align PAGE_SIZE
+ENTRY(restore_registers)
 	/* go back to the original page tables */
 	movl	$swapper_pg_dir, %eax
 	subl	$__PAGE_OFFSET, %eax

@@ -83,3 +91,4 @@ done:
 	xorl	%eax, %eax
 
 	ret
+ENDPROC(restore_registers)

Signature · 1 line

2.17.1

Rafael J. Wysocki

maintainer

2018-09-19 08:41 UTC #8 lore raw

Re: [PATCH 03/12][RFC v3] x86-32/asm/power: Create stack frames in hibernate_asm_32.S

Quoted 10 lines · Chen Yu

From: Zhimin Gu <[email protected]>
Backport
Commit ef0f3ed5a4ac (x86/asm/power: Create stack frames in hibernate_asm_64.S)
and
Commit 4ce827b4cc58 (x86/power/64: Fix hibernation return address corruption)

Quoted 2 lines

And also prepare for relocated code and jump address mapping.

I don't like the changelog.

It should explain the technical reasons for making the changes in the first place and then mention that analogous changes were made for 64-bit before and by which commits.

Thanks,
Rafael

Yu Chen

author

2018-09-19 08:47 UTC #9 lore raw

Re: [PATCH 03/12][RFC v3] x86-32/asm/power: Create stack frames in hibernate_asm_32.S

Quoted 11 lines · Rafael J. Wysocki

Quoted 10 lines · Chen Yu
From: Zhimin Gu <[email protected]>
Backport
Commit ef0f3ed5a4ac (x86/asm/power: Create stack frames in hibernate_asm_64.S)
and
Commit 4ce827b4cc58 (x86/power/64: Fix hibernation return address corruption)

Quoted 9 lines

Quoted 2 lines
And also prepare for relocated code and jump address mapping.
I don't like the changelog.
It should explain the technical reasons for making the changes in the first place and then mention that analogous changes were made for 64-bit before and by which commits.

Ok, will change it in next version.

Thanks,
Yu

Quoted 2 lines

Thanks,
Rafael

Chen Yu

author

2018-09-19 07:32 UTC #10 lore raw

RFC PATCH v3 4/12 x86, hibernate: Extract the common code of 64/32 bit system

In reply to Chen Yu

From: Zhimin Gu <[email protected]>

Reduce the hibernation code duplication between x86-32 and x86-64 by extracting the common code into hibernate.c.

No functional change.

Acked-by Pavel Machek <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Cc Thomas Gleixner <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/include/asm/suspend.h |   8 ++
 arch/x86/power/Makefile        |   2 +-
 arch/x86/power/hibernate.c     | 249 +++++++++++++++++++++++++++++++++
 arch/x86/power/hibernate_32.c  |  15 +-
 arch/x86/power/hibernate_64.c  | 221 -----------------------------
 5 files changed, 259 insertions(+), 236 deletions(-)
 create mode 100644 arch/x86/power/hibernate.c

arch/x86/include/asm/suspend.h +8 -0

index ecffe81ff65c..40b02558749f 100644
@@ -4,3 +4,11 @@
 #else
 # include <asm/suspend_64.h>
 #endif
+extern unsigned long restore_jump_address __visible;
+extern unsigned long jump_address_phys;
+extern unsigned long restore_cr3 __visible;
+extern unsigned long temp_level4_pgt __visible;
+extern unsigned long relocated_restore_code __visible;
+extern int relocate_restore_code(void);
+/* Defined in hibernate_asm_32/64.S */
+extern asmlinkage __visible int restore_image(void);

arch/x86/power/Makefile +1 -1

index a4701389562c..37923d715741 100644
@@ -7,4 +7,4 @@ nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_cpu.o	:= $(nostackp)
 
 obj-$(CONFIG_PM_SLEEP)		+= cpu.o
-obj-$(CONFIG_HIBERNATION)	+= hibernate_$(BITS).o hibernate_asm_$(BITS).o
+obj-$(CONFIG_HIBERNATION)	+= hibernate_$(BITS).o hibernate_asm_$(BITS).o hibernate.o

arch/x86/power/hibernate.c new file +249 -0

249 lines not on this page · raw message

arch/x86/power/hibernate_32.c +1 ~~-14~~

index afc4ed7b1578..f82fbd279d08 100644
@@ -14,9 +14,7 @@
 #include <asm/pgtable.h>
 #include <asm/mmzone.h>
 #include <asm/sections.h>
-
-/* Defined in hibernate_asm_32.S */
-extern int restore_image(void);
+#include <asm/suspend.h>
 
 /* Pointer to the temporary resume page tables */
 pgd_t *resume_pg_dir;

@@ -162,14 +160,3 @@ asmlinkage int swsusp_arch_resume(void)
 	restore_image();
 	return 0;
 }
-
-/*
- *	pfn_is_nosave - check if given pfn is in the 'nosave' section
- */
-
-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);
-}

arch/x86/power/hibernate_64.c +0 ~~-221~~

236 lines not on this page · raw message

Signature · 1 line

2.17.1

Rafael J. Wysocki

maintainer

2018-09-19 09:03 UTC #11 lore raw

Re: [PATCH 04/12][RFC v3] x86, hibernate: Extract the common code of 64/32 bit system

Quoted 54 lines · Chen Yu

From: Zhimin Gu <[email protected]>

Reduce the hibernation code duplication between x86-32 and x86-64 by extracting the common code into hibernate.c.

No functional change.

Acked-by Pavel Machek <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Cc Thomas Gleixner <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/include/asm/suspend.h |   8 ++
 arch/x86/power/Makefile        |   2 +-
 arch/x86/power/hibernate.c     | 249 +++++++++++++++++++++++++++++++++
 arch/x86/power/hibernate_32.c  |  15 +-
 arch/x86/power/hibernate_64.c  | 221 -----------------------------
 5 files changed, 259 insertions(+), 236 deletions(-)
 create mode 100644 arch/x86/power/hibernate.c

arch/x86/include/asm/suspend.h +8 -0

index ecffe81ff65c..40b02558749f 100644
@@ -4,3 +4,11 @@
 #else
 # include <asm/suspend_64.h>
 #endif
+extern unsigned long restore_jump_address __visible;
+extern unsigned long jump_address_phys;
+extern unsigned long restore_cr3 __visible;
+extern unsigned long temp_level4_pgt __visible;
+extern unsigned long relocated_restore_code __visible;
+extern int relocate_restore_code(void);
+/* Defined in hibernate_asm_32/64.S */
+extern asmlinkage __visible int restore_image(void);

arch/x86/power/Makefile +1 -1

index a4701389562c..37923d715741 100644
@@ -7,4 +7,4 @@ nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_cpu.o   := $(nostackp)
 
 obj-$(CONFIG_PM_SLEEP)         += cpu.o
-obj-$(CONFIG_HIBERNATION)      += hibernate_$(BITS).o hibernate_asm_$(BITS).o
+obj-$(CONFIG_HIBERNATION)      += hibernate_$(BITS).o hibernate_asm_$(BITS).o hibernate.o

arch/x86/power/hibernate.c new file +0 -0

new file mode 100644
index 000000000000..fbde8f0e8fe0

arch/x86/power/hibernate.c line 7

Quoted 7 lines

arch/x86/power/hibernate.c new file +7 -0

@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hibernation support for x86
+ *
+ * Copyright (c) 2007 Rafael J. Wysocki <[email protected]>
+ * Copyright (c) 2002 Pavel Machek <[email protected]>
+ * Copyright (c) 2001 Patrick Mochel <[email protected]>

I don't think this "copyright" information has any legal bearing any more at this point.

I would just write it as

* Authors:

 *  2007 Rafael J. Wysocki <[email protected]>
 *  2002 Pavel Machek <[email protected]>
 *  2001 Patrick Mochel <[email protected]>

as a matter of credits to the people who originally developed this code.

Quoted 45 lines

arch/x86/power/hibernate.c new file +45 -0

+ */
+
+#include <linux/gfp.h>
+#include <linux/smp.h>
+#include <linux/suspend.h>
+#include <linux/scatterlist.h>
+#include <linux/kdebug.h>
+
+#include <crypto/hash.h>
+
+#include <asm/e820/api.h>
+#include <asm/init.h>
+#include <asm/proto.h>
+#include <asm/page.h>
+#include <asm/pgtable.h>
+#include <asm/mtrr.h>
+#include <asm/sections.h>
+#include <asm/suspend.h>
+#include <asm/tlbflush.h>
+
+/*
+ * Address to jump to in the last phase of restore in order to get to the image
+ * kernel's text (this value is passed in the image header).
+ */
+unsigned long restore_jump_address __visible;
+unsigned long jump_address_phys;
+
+/*
+ * Value of the cr3 register from before the hibernation (this value is passed
+ * in the image header).
+ */
+unsigned long restore_cr3 __visible;
+unsigned long temp_level4_pgt __visible;
+unsigned long relocated_restore_code __visible;
+
+#ifdef CONFIG_X86_64
+#define RESTORE_MAGIC  0x23456789ABCDEF01UL
+#else
+#define RESTORE_MAGIC  0x12345678UL
+#endif
+
+/**
+ *     pfn_is_nosave - check if given pfn is in the 'nosave' section
+ */
+int pfn_is_nosave(unsigned long pfn)

arch/x86/power/hibernate.c line 62

Quoted 10 lines

arch/x86/power/hibernate.c new file +10 -0

+{
+       unsigned long nosave_begin_pfn;
+       unsigned long nosave_end_pfn;
+
+       nosave_begin_pfn = __pa_symbol(&__nosave_begin) >> PAGE_SHIFT;
+       nosave_end_pfn = PAGE_ALIGN(__pa_symbol(&__nosave_end)) >> PAGE_SHIFT;
+
+       return pfn >= nosave_begin_pfn && pfn < nosave_end_pfn;
+}
+

Since the majority of the code below in this new file is only going to be used on 64-bit to start with, I will put it under #ifdef CONFIG_X86_64 at this point and remove the #ifdef in one of the subsequent patches when 32-bit actually starts to use it.

Thanks,
Rafael

Rafael J. Wysocki

maintainer

2018-09-19 10:15 UTC #12 lore raw

Re: [PATCH 04/12][RFC v3] x86, hibernate: Extract the common code of 64/32 bit system

Quoted 126 lines · Rafael J. Wysocki

Quoted 54 lines · Chen Yu

From: Zhimin Gu <[email protected]>

Reduce the hibernation code duplication between x86-32 and x86-64 by extracting the common code into hibernate.c.

No functional change.

Acked-by Pavel Machek <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Cc Thomas Gleixner <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/include/asm/suspend.h |   8 ++
 arch/x86/power/Makefile        |   2 +-
 arch/x86/power/hibernate.c     | 249 +++++++++++++++++++++++++++++++++
 arch/x86/power/hibernate_32.c  |  15 +-
 arch/x86/power/hibernate_64.c  | 221 -----------------------------
 5 files changed, 259 insertions(+), 236 deletions(-)
 create mode 100644 arch/x86/power/hibernate.c

arch/x86/include/asm/suspend.h +8 -0

index ecffe81ff65c..40b02558749f 100644
@@ -4,3 +4,11 @@
 #else
 # include <asm/suspend_64.h>
 #endif
+extern unsigned long restore_jump_address __visible;
+extern unsigned long jump_address_phys;
+extern unsigned long restore_cr3 __visible;
+extern unsigned long temp_level4_pgt __visible;
+extern unsigned long relocated_restore_code __visible;
+extern int relocate_restore_code(void);
+/* Defined in hibernate_asm_32/64.S */
+extern asmlinkage __visible int restore_image(void);

arch/x86/power/Makefile +1 -1

index a4701389562c..37923d715741 100644
@@ -7,4 +7,4 @@ nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_cpu.o   := $(nostackp)
 
 obj-$(CONFIG_PM_SLEEP)         += cpu.o
-obj-$(CONFIG_HIBERNATION)      += hibernate_$(BITS).o hibernate_asm_$(BITS).o
+obj-$(CONFIG_HIBERNATION)      += hibernate_$(BITS).o hibernate_asm_$(BITS).o hibernate.o

arch/x86/power/hibernate.c new file +0 -0

new file mode 100644
index 000000000000..fbde8f0e8fe0

arch/x86/power/hibernate.c line 7

Quoted 7 lines

arch/x86/power/hibernate.c new file +7 -0

@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hibernation support for x86
+ *
+ * Copyright (c) 2007 Rafael J. Wysocki <[email protected]>
+ * Copyright (c) 2002 Pavel Machek <[email protected]>
+ * Copyright (c) 2001 Patrick Mochel <[email protected]>

I don't think this "copyright" information has any legal bearing any more at this point.

I would just write it as

* Authors:

 *  2007 Rafael J. Wysocki <[email protected]>
 *  2002 Pavel Machek <[email protected]>
 *  2001 Patrick Mochel <[email protected]>

as a matter of credits to the people who originally developed this code.

Quoted 50 lines

arch/x86/power/hibernate.c new file +50 -0

+ */
+
+#include <linux/gfp.h>
+#include <linux/smp.h>
+#include <linux/suspend.h>
+#include <linux/scatterlist.h>
+#include <linux/kdebug.h>
+
+#include <crypto/hash.h>
+
+#include <asm/e820/api.h>
+#include <asm/init.h>
+#include <asm/proto.h>
+#include <asm/page.h>
+#include <asm/pgtable.h>
+#include <asm/mtrr.h>
+#include <asm/sections.h>
+#include <asm/suspend.h>
+#include <asm/tlbflush.h>
+
+/*
+ * Address to jump to in the last phase of restore in order to get to the image
+ * kernel's text (this value is passed in the image header).
+ */
+unsigned long restore_jump_address __visible;
+unsigned long jump_address_phys;
+
+/*
+ * Value of the cr3 register from before the hibernation (this value is passed
+ * in the image header).
+ */
+unsigned long restore_cr3 __visible;
+unsigned long temp_level4_pgt __visible;
+unsigned long relocated_restore_code __visible;
+
+#ifdef CONFIG_X86_64
+#define RESTORE_MAGIC  0x23456789ABCDEF01UL
+#else
+#define RESTORE_MAGIC  0x12345678UL
+#endif
+
+/**
+ *     pfn_is_nosave - check if given pfn is in the 'nosave' section
+ */
+int pfn_is_nosave(unsigned long pfn)
+{
+       unsigned long nosave_begin_pfn;
+       unsigned long nosave_end_pfn;
+
+       nosave_begin_pfn = __pa_symbol(&__nosave_begin) >> PAGE_SHIFT;

Quoted 8 lines

arch/x86/power/hibernate.c line 62
Quoted 5 lines
arch/x86/power/hibernate.c new file +5 -0
+       nosave_end_pfn = PAGE_ALIGN(__pa_symbol(&__nosave_end)) >> PAGE_SHIFT;
+
+       return pfn >= nosave_begin_pfn && pfn < nosave_end_pfn;
+}
+
Since the majority of the code below in this new file is only going to be used on 64-bit to start with, I will put it under #ifdef

I should have said "I would put it under #ifdef" here, sorry for the possible confusion.

Quoted 2 lines

CONFIG_X86_64 at this point and remove the #ifdef in one of the subsequent patches when 32-bit actually starts to use it.

Thanks,
Rafael

Ingo Molnar

2018-09-19 10:34 UTC #13 lore raw

Re: [PATCH 04/12][RFC v3] x86, hibernate: Extract the common code of 64/32 bit system

In reply to Rafael J. Wysocki

Quoted 6 lines · Rafael J. Wysocki

Quoted 6 lines
index 000000000000..fbde8f0e8fe0
arch/x86/power/hibernate.c new file +2 -0
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+/*

Quoted 8 lines

arch/x86/power/hibernate.c line 7
Quoted 5 lines
arch/x86/power/hibernate.c new file +5 -0
+ * Hibernation support for x86
+ *
+ * Copyright (c) 2007 Rafael J. Wysocki <[email protected]>
+ * Copyright (c) 2002 Pavel Machek <[email protected]>
+ * Copyright (c) 2001 Patrick Mochel <[email protected]>
I don't think this "copyright" information has any legal bearing any more at this point.

I think that's wrong in general, and it's routine for corporate legal departments to insist on specific boilerplate copyright notices when OSS code is contributed.

Copying code and then removing copyright notices is frowned upon independently as well.

While you can probably talk for your own copyright notice, I'm not sure it's true of the two other historic copyright notices that were copied.

So I think it's best practice to preserve all copyright notices as-is.

Thanks,

Ingo

Rafael J. Wysocki

maintainer

2018-09-19 10:36 UTC #14 lore raw

Re: [PATCH 04/12][RFC v3] x86, hibernate: Extract the common code of 64/32 bit system

Quoted 23 lines · Ingo Molnar

Quoted 6 lines · Rafael J. Wysocki
Quoted 6 lines
index 000000000000..fbde8f0e8fe0
arch/x86/power/hibernate.c new file +2 -0
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
Quoted 8 lines
arch/x86/power/hibernate.c line 7
Quoted 5 lines
arch/x86/power/hibernate.c new file +5 -0
+ * Hibernation support for x86
+ *
+ * Copyright (c) 2007 Rafael J. Wysocki <[email protected]>
+ * Copyright (c) 2002 Pavel Machek <[email protected]>
+ * Copyright (c) 2001 Patrick Mochel <[email protected]>
I don't think this "copyright" information has any legal bearing any more at this point.
I think that's wrong in general, and it's routine for corporate legal departments to insist on specific boilerplate copyright notices when OSS code is contributed.

Quoted 7 lines

Copying code and then removing copyright notices is frowned upon independently as well.
While you can probably talk for your own copyright notice, I'm not sure it's true of the two other historic copyright notices that were copied.
So I think it's best practice to preserve all copyright notices as-is.

OK, let's retain them, then.

Thanks,
Rafael

Pavel Machek

2018-09-20 22:05 UTC #15 lore raw

Re: [PATCH 04/12][RFC v3] x86, hibernate: Extract the common code of 64/32 bit system

Quoted 27 lines · Rafael J. Wysocki

Quoted 26 lines · Ingo Molnar
Quoted 6 lines · Rafael J. Wysocki
Quoted 6 lines
index 000000000000..fbde8f0e8fe0
arch/x86/power/hibernate.c new file +2 -0
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
Quoted 8 lines
arch/x86/power/hibernate.c line 7
Quoted 5 lines
arch/x86/power/hibernate.c new file +5 -0
+ * Hibernation support for x86
+ *
+ * Copyright (c) 2007 Rafael J. Wysocki <[email protected]>
+ * Copyright (c) 2002 Pavel Machek <[email protected]>
+ * Copyright (c) 2001 Patrick Mochel <[email protected]>
I don't think this "copyright" information has any legal bearing any more at this point.
I think that's wrong in general, and it's routine for corporate legal departments to insist on specific boilerplate copyright notices when OSS code is contributed.
Copying code and then removing copyright notices is frowned upon independently as well.

Quoted 6 lines

Quoted 4 lines
While you can probably talk for your own copyright notice, I'm not sure it's true of the two other historic copyright notices that were copied.
So I think it's best practice to preserve all copyright notices as-is.
OK, let's retain them, then.

Thank you, I was about to make same comment as Ingo.

Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Chen Yu

author

2018-09-19 07:32 UTC #16 lore raw

RFC PATCH v3 5/12 x86-32, hibernate: Enable CONFIG_ARCH_HIBERNATION_HEADER set on 32bit system

In reply to Chen Yu

From: Zhimin Gu <[email protected]>

Enable CONFIG_ARCH_HIBERNATION_HEADER for 32bit system so that md5 check is enabled and prepare for relocated code executing and jump address mapping.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/Kconfig +1 -1

index 1a0be022f91d..e8de5de1057f 100644
@@ -2422,7 +2422,7 @@ menu "Power management and ACPI options"
 
 config ARCH_HIBERNATION_HEADER
 	def_bool y
-	depends on X86_64 && HIBERNATION
+	depends on HIBERNATION
 
 source "kernel/power/Kconfig"

Signature · 1 line

2.17.1

Rafael J. Wysocki

maintainer

2018-09-19 10:31 UTC #17 lore raw

Re: [PATCH 05/12][RFC v3] x86-32, hibernate: Enable CONFIG_ARCH_HIBERNATION_HEADER set on 32bit system

Quoted 19 lines · Chen Yu

From: Zhimin Gu <[email protected]>
Enable CONFIG_ARCH_HIBERNATION_HEADER for 32bit system so that md5 check is enabled and prepare for relocated code executing and jump address mapping.
Cc "Rafael J. Wysocki" <[email protected]>
Signed-off-by Zhimin Gu <[email protected]>
Signed-off-by Chen Yu <[email protected]>
---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/Kconfig +0 -0
index 1a0be022f91d..e8de5de1057f 100644

arch/x86/Kconfig menu "Power management and ACPI options"

Quoted 9 lines

arch/x86/Kconfig +1 -2

@@ -2422,7 +2422,7 @@ menu "Power management and ACPI options"
 
 config ARCH_HIBERNATION_HEADER
        def_bool y
-       depends on X86_64 && HIBERNATION
+       depends on HIBERNATION
 
 source "kernel/power/Kconfig"
 
--

In this patch, in addition to the above, I would start moving the code in hibernate.c that you want to be common in the end out of #ifdef CONFIG_X86_64 blocks.

To start with, that would be the checksum handling code and the parts of arch_hibernation_header_save/restore() that refer to it and the "magic" value.

I would continue doing that in the subsequent patches to make it easier to follow which pieces of code start to be used on 32-bit after each patch. The impact of each patch would be more clear this way IMO.

Thanks,
Rafael

Chen Yu

author

2018-09-19 07:35 UTC #18 lore raw

RFC PATCH v3 6/12 x86, hibernate: Rename temp_level4_pgt to temp_pgt

In reply to Chen Yu

From: Zhimin Gu <[email protected]>

As 32bit system is not using 4-level page, rename it to temp_pgt so that it can be reused for both 32bit and 64bit hibernation.

No functional change.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/include/asm/suspend.h    | 2 +-
 arch/x86/power/hibernate.c        | 2 +-
 arch/x86/power/hibernate_64.c     | 2 +-
 arch/x86/power/hibernate_asm_64.S | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

arch/x86/include/asm/suspend.h +1 -1

index 40b02558749f..a892494ca5e4 100644
@@ -7,7 +7,7 @@
 extern unsigned long restore_jump_address __visible;
 extern unsigned long jump_address_phys;
 extern unsigned long restore_cr3 __visible;
-extern unsigned long temp_level4_pgt __visible;
+extern unsigned long temp_pgt __visible;
 extern unsigned long relocated_restore_code __visible;
 extern int relocate_restore_code(void);
 /* Defined in hibernate_asm_32/64.S */

arch/x86/power/hibernate.c +1 -1

index fbde8f0e8fe0..b0e586ae947e 100644
@@ -37,7 +37,7 @@ unsigned long jump_address_phys;
  * in the image header).
  */
 unsigned long restore_cr3 __visible;
-unsigned long temp_level4_pgt __visible;
+unsigned long temp_pgt __visible;
 unsigned long relocated_restore_code __visible;
 
 #ifdef CONFIG_X86_64

arch/x86/power/hibernate_64.c +1 -1

index 8bc2eb0dc464..239f424ccb29 100644
@@ -121,7 +121,7 @@ static int set_up_temporary_mappings(void)
 			return result;
 	}
 
-	temp_level4_pgt = __pa(pgd);
+	temp_pgt = __pa(pgd);
 	return 0;
 }

arch/x86/power/hibernate_asm_64.S +1 -1

index fd369a6e9ff8..3008baa2fa95 100644
@@ -59,7 +59,7 @@ ENTRY(restore_image)
 	movq	restore_cr3(%rip), %r9
 
 	/* prepare to switch to temporary page tables */
-	movq	temp_level4_pgt(%rip), %rax
+	movq	temp_pgt(%rip), %rax
 	movq	mmu_cr4_features(%rip), %rbx
 
 	/* prepare to copy image data to their original locations */

Signature · 1 line

2.17.1

Pavel Machek

Acked-by on v3 6/12

2018-09-20 22:05 UTC #19 lore raw

Re: [PATCH 06/12][RFC v3] x86, hibernate: Rename temp_level4_pgt to temp_pgt

Quoted 11 lines · Chen Yu

From: Zhimin Gu <[email protected]>
As 32bit system is not using 4-level page, rename it to temp_pgt so that it can be reused for both 32bit and 64bit hibernation.
No functional change.
Cc "Rafael J. Wysocki" <[email protected]>
Signed-off-by Zhimin Gu <[email protected]>
Signed-off-by Chen Yu <[email protected]>

Acked-by Pavel Machek <[email protected]>

Chen Yu

author

2018-09-19 07:36 UTC #20 lore raw

RFC PATCH v3 7/12 x86-32, hibernate: Use temp_pgt as the temporary page table

In reply to Chen Yu

From: Zhimin Gu <[email protected]>

This is to reuse the temp_pgt for both 32bit and 64bit system.

No functional change.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/power/hibernate_32.c     | 2 ++
 arch/x86/power/hibernate_asm_32.S | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

arch/x86/power/hibernate_32.c +2 -0

index f82fbd279d08..a44bdada4e4e 100644
@@ -156,6 +156,8 @@ asmlinkage int swsusp_arch_resume(void)
 	if (error)
 		return error;
 
+	temp_pgt = __pa(resume_pg_dir);
+
 	/* We have got enough memory and from now on we cannot recover */
 	restore_image();
 	return 0;

arch/x86/power/hibernate_asm_32.S +1 -2

index 671d38d0d931..f0627cf30a36 100644
@@ -33,8 +33,7 @@ ENDPROC(swsusp_arch_suspend)
 
 ENTRY(restore_image)
 	movl	mmu_cr4_features, %ecx
-	movl	resume_pg_dir, %eax
-	subl	$__PAGE_OFFSET, %eax
+	movl	temp_pgt, %eax
 	movl	%eax, %cr3
 
 	jecxz	1f	# cr4 Pentium and higher, skip if zero

Signature · 1 line

2.17.1

Pavel Machek

Acked-by on v3 7/12

2018-09-20 22:05 UTC #21 lore raw

Re: [PATCH 07/12][RFC v3] x86-32, hibernate: Use temp_pgt as the temporary page table

Quoted 10 lines · Chen Yu

From: Zhimin Gu <[email protected]>
This is to reuse the temp_pgt for both 32bit and 64bit system.
No functional change.
Cc "Rafael J. Wysocki" <[email protected]>
Signed-off-by Zhimin Gu <[email protected]>
Signed-off-by Chen Yu <[email protected]>

Acked-by Pavel Machek <[email protected]>

Chen Yu

author

2018-09-19 07:36 UTC #22 lore raw

RFC PATCH v3 8/12 x86-32, hibernate: Use the page size macro instead of constant value

In reply to Chen Yu

From: Zhimin Gu <[email protected]>

Convert the hard code into PAGE_SIZE for better scalability.

No functional change.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/power/hibernate_asm_32.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/power/hibernate_asm_32.S +1 -1

index f0627cf30a36..f5103ae96582 100644
@@ -52,7 +52,7 @@ copy_loop:
 	movl	pbe_address(%edx), %esi
 	movl	pbe_orig_address(%edx), %edi
 
-	movl	$1024, %ecx
+	movl	$(PAGE_SIZE >> 2), %ecx
 	rep
 	movsl

Signature · 1 line

2.17.1

Pavel Machek

Acked-by on v3 8/12

2018-09-20 22:05 UTC #23 lore raw

Re: [PATCH 08/12][RFC v3] x86-32, hibernate: Use the page size macro instead of constant value

Quoted 9 lines · Chen Yu

From: Zhimin Gu <[email protected]>
Convert the hard code into PAGE_SIZE for better scalability.
No functional change.
Cc "Rafael J. Wysocki" <[email protected]>
Signed-off-by Zhimin Gu <[email protected]>
Signed-off-by Chen Yu <[email protected]>

Acked-by Pavel Machek <[email protected]>

Chen Yu

author

2018-09-19 07:36 UTC #24 lore raw

RFC PATCH v3 9/12 x86-32, hibernate: Switch to original page table after resumed

In reply to Chen Yu

From: Zhimin Gu <[email protected]>

After all the pages are restored to previous address, the page table switches back to current swapper_pg_dir. However the swapper_pg_dir currently in used might not be consistent with previous page table, which might cause issue after resumed.

Fix this issue by switching to original page table after resumed.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/power/hibernate_asm_32.S | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

arch/x86/power/hibernate_asm_32.S +7 -3

index f5103ae96582..6b2b94937113 100644
@@ -25,6 +25,10 @@ ENTRY(swsusp_arch_suspend)
 	pushfl
 	popl saved_context_eflags
 
+	/* save cr3 */
+	movl	%cr3, %eax
+	movl	%eax, restore_cr3
+
 	FRAME_BEGIN
 	call swsusp_save
 	FRAME_END

@@ -32,6 +36,8 @@ ENTRY(swsusp_arch_suspend)
 ENDPROC(swsusp_arch_suspend)
 
 ENTRY(restore_image)
+	movl	restore_cr3, %ebp
+
 	movl	mmu_cr4_features, %ecx
 	movl	temp_pgt, %eax
 	movl	%eax, %cr3

@@ -66,9 +72,7 @@ done:
 	.align PAGE_SIZE
 ENTRY(restore_registers)
 	/* go back to the original page tables */
-	movl	$swapper_pg_dir, %eax
-	subl	$__PAGE_OFFSET, %eax
-	movl	%eax, %cr3
+	movl	%ebp, %cr3
 	movl	mmu_cr4_features, %ecx
 	jecxz	1f	# cr4 Pentium and higher, skip if zero
 	movl	%ecx, %cr4;  # turn PGE back on

Signature · 1 line

2.17.1

Pavel Machek

Acked-by on v3 9/12

2018-09-20 22:05 UTC #25 lore raw

Re: [PATCH 09/12][RFC v3] x86-32, hibernate: Switch to original page table after resumed

Quoted 8 lines · Chen Yu

From: Zhimin Gu <[email protected]>
After all the pages are restored to previous address, the page table switches back to current swapper_pg_dir. However the swapper_pg_dir currently in used might not be consistent with previous page table, which might cause issue after resumed.
Fix this issue by switching to original page table after resumed.

"after resume".

Acked-by Pavel Machek <[email protected]>

Chen Yu

author

2018-09-19 07:36 UTC #26 lore raw

RFC PATCH v3 10/12 x86-32, hibernate: Switch to relocated restore code during resume on 32bit system

In reply to Chen Yu

From: Zhimin Gu <[email protected]>

Code should be executed in a safe page during page restoring, as the page where instruction is running during resume might be scribbled and causes issues.

Backport the code from 64 bit system to fix this bug.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/power/hibernate_32.c     | 4 ++++
 arch/x86/power/hibernate_asm_32.S | 7 +++++++
 2 files changed, 11 insertions(+)

arch/x86/power/hibernate_32.c +4 -0

index a44bdada4e4e..a9861095fbb8 100644
@@ -158,6 +158,10 @@ asmlinkage int swsusp_arch_resume(void)
 
 	temp_pgt = __pa(resume_pg_dir);
 
+	error = relocate_restore_code();
+	if (error)
+		return error;
+
 	/* We have got enough memory and from now on we cannot recover */
 	restore_image();
 	return 0;

arch/x86/power/hibernate_asm_32.S +7 -0

index 6b2b94937113..e9adda6b6b02 100644
@@ -39,6 +39,13 @@ ENTRY(restore_image)
 	movl	restore_cr3, %ebp
 
 	movl	mmu_cr4_features, %ecx
+
+	/* jump to relocated restore code */
+	movl	relocated_restore_code, %eax
+	jmpl	*%eax
+
+/* code below has been relocated to a safe page */
+ENTRY(core_restore_code)
 	movl	temp_pgt, %eax
 	movl	%eax, %cr3

Signature · 1 line

2.17.1

Pavel Machek

Acked-by on v3 10/12

2018-09-20 22:05 UTC #27 lore raw

Re: [PATCH 10/12][RFC v3] x86-32, hibernate: Switch to relocated restore code during resume on 32bit system

Quoted 7 lines · Chen Yu

From: Zhimin Gu <[email protected]>
Code should be executed in a safe page during page restoring, as the page where instruction is running during resume might be scribbled and causes issues.
Backport the code from 64 bit system to fix this bug.

On 32 bit, we only suspend resuming by same kernel that did the suspend.

64 bit does not have that restriction. So 32 bit code should not be actually bugy.

Quoted 3 lines

Cc "Rafael J. Wysocki" <[email protected]>
Signed-off-by Zhimin Gu <[email protected]>
Signed-off-by Chen Yu <[email protected]>

But we'd like to remove that restriction in future, so:

Acked-by Pavel Machek <[email protected]>

Chen Yu

author

2018-09-19 07:36 UTC #28 lore raw

RFC PATCH v3 11/12 x86-32, hibernate: Set up temporary text mapping for 32bit system

In reply to Chen Yu

From: Zhimin Gu <[email protected]>

Set up the temporary text mapping for the final jump address so that the system could jump to the right address after all the pages have been copied back to their original address.

Back-port from 64bit system.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/power/hibernate_32.c     | 31 +++++++++++++++++++++++++++++++
 arch/x86/power/hibernate_asm_32.S |  3 +++
 2 files changed, 34 insertions(+)

arch/x86/power/hibernate_32.c +31 -0

index a9861095fbb8..15695e30f982 100644
@@ -143,6 +143,32 @@ static inline void resume_init_first_level_page_table(pgd_t *pg_dir)
 #endif
 }
 
+static int set_up_temporary_text_mapping(pgd_t *pgd_base)
+{
+	pgd_t *pgd;
+	pmd_t *pmd;
+	pte_t *pte;
+
+	pgd = pgd_base + pgd_index(restore_jump_address);
+
+	pmd = resume_one_md_table_init(pgd);
+	if (!pmd)
+		return -ENOMEM;
+
+	if (boot_cpu_has(X86_FEATURE_PSE)) {
+		set_pmd(pmd + pmd_index(restore_jump_address),
+		__pmd((jump_address_phys & PMD_MASK) | pgprot_val(PAGE_KERNEL_LARGE_EXEC)));
+	} else {
+		pte = resume_one_page_table_init(pmd);
+		if (!pte)
+			return -ENOMEM;
+		set_pte(pte + pte_index(restore_jump_address),
+		__pte((jump_address_phys & PAGE_MASK) | pgprot_val(PAGE_KERNEL_EXEC)));
+	}
+
+	return 0;
+}
+
 asmlinkage int swsusp_arch_resume(void)
 {
 	int error;

@@ -152,6 +178,11 @@ asmlinkage int swsusp_arch_resume(void)
 		return -ENOMEM;
 
 	resume_init_first_level_page_table(resume_pg_dir);
+
+	error = set_up_temporary_text_mapping(resume_pg_dir);
+	if (error)
+		return error;
+
 	error = resume_physical_mapping_init(resume_pg_dir);
 	if (error)
 		return error;

arch/x86/power/hibernate_asm_32.S +3 -0

index e9adda6b6b02..01f653fae7bd 100644
@@ -36,6 +36,8 @@ ENTRY(swsusp_arch_suspend)
 ENDPROC(swsusp_arch_suspend)
 
 ENTRY(restore_image)
+	/* prepare to jump to the image kernel */
+	movl	restore_jump_address, %ebx
 	movl	restore_cr3, %ebp
 
 	movl	mmu_cr4_features, %ecx

@@ -74,6 +76,7 @@ copy_loop:
 	.p2align 4,,7
 
 done:
+	jmpl	*%ebx
 
 	/* code below belongs to the image kernel */
 	.align PAGE_SIZE

Signature · 1 line

2.17.1

Pavel Machek

Acked-by on v3 11/12

2018-09-20 22:05 UTC #29 lore raw

Re: [PATCH 11/12][RFC v3] x86-32, hibernate: Set up temporary text mapping for 32bit system

Quoted 11 lines · Chen Yu

From: Zhimin Gu <[email protected]>
Set up the temporary text mapping for the final jump address so that the system could jump to the right address after all the pages have been copied back to their original address.
Back-port from 64bit system.
Cc "Rafael J. Wysocki" <[email protected]>
Signed-off-by Zhimin Gu <[email protected]>
Signed-off-by Chen Yu <[email protected]>

Acked-by Pavel Machek <[email protected]>

Chen Yu

author

2018-09-19 07:37 UTC #30 lore raw

RFC PATCH v3 12/12 x86-32, hibernate: Adjust in_suspend after resumed on 32bit system

In reply to Chen Yu

From: Zhimin Gu <[email protected]>

Update the in_suspend variable to reflect the actual hibernation status. Back-port from 64bit system.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Zhimin Gu <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/power/hibernate_asm_32.S | 3 +++
 1 file changed, 3 insertions(+)

arch/x86/power/hibernate_asm_32.S +3 -0

index 01f653fae7bd..6fe383002125 100644
@@ -103,5 +103,8 @@ ENTRY(restore_registers)
 
 	xorl	%eax, %eax
 
+	/* tell the hibernation core that we've just restored the memory */
+	movl	%eax, in_suspend
+
 	ret
 ENDPROC(restore_registers)

Signature · 1 line

2.17.1

Pavel Machek

Acked-by on v3 12/12

2018-09-20 22:06 UTC #31 lore raw

Re: [PATCH 12/12][RFC v3] x86-32, hibernate: Adjust in_suspend after resumed on 32bit system

Quoted 8 lines · Chen Yu

From: Zhimin Gu <[email protected]>
Update the in_suspend variable to reflect the actual hibernation status. Back-port from 64bit system.
Cc "Rafael J. Wysocki" <[email protected]>
Signed-off-by Zhimin Gu <[email protected]>
Signed-off-by Chen Yu <[email protected]>

Acked-by Pavel Machek <[email protected]>