Message-Id: <20230119235145.22740-1-rick.p.edgecombe@intel.com>
Date:   Thu, 19 Jan 2023 15:51:45 -0800
From:   Rick Edgecombe <rick.p.edgecombe@...el.com>
To:     tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...ux.intel.com, hpa@...or.com, x86@...nel.org,
        rafael@...nel.org, pavel@....cz, len.brown@...el.com,
        rppt@...nel.org, peterz@...radead.org, luto@...nel.org
Cc:     rick.p.edgecombe@...el.com, linux-kernel@...r.kernel.org,
        linux-pm@...r.kernel.org
Subject: [PATCH v3] x86/hibernate: Use fixmap for saving unmapped pages

Hibernate uses the direct map to read memory it saves to disk. Since
sometimes pages are not accessible on the direct map ("not present" on
x86), it has special case logic to temporarily make a page present. On x86
these direct map addresses can be mapped at various page sizes, but the
logic works ok as long as the not present pages are always mapped as
PAGE_SIZE such that they don't require a split to map the region as
present. If the address was mapped not present by a larger page size, the
split may fail and hibernate would then try to read an address mapped not
present.

Today on x86 there are no known cases of this (huge not present pages on
the direct map), but it has come up from time to time when developing
things that operate on the direct map. It blocked making
VM_FLUSH_RESET_PERMS support huge vmalloc when that came up, and also
has been a complication for various direct map protection efforts.

This dependency is also pretty hidden and easily missed by people poking at
the direct map. For this reason, there are warnings in place to complain
but not handle this scenario.

One way to make this more robust would be to create some new CPA
functionality that can know to map and reset the whole huge page in the
case of trying to map a subpage. But for simplicity and smaller code, just
make x86 hibernate have its own fixmap PTE that it can use to point
to 4k pages when it encounters an unmapped direct map page.

So create arch breakouts for hibernate_map_page() and
hibernate_unmap_page() so that the mapping of unmapped pages can be
off the direct map. Change hibernate_map_page() to return an address,
and implement an arch breakout on x86 on that uses the fixmap.

Since now hibernate_map_page() can return a value, have it return NULL
when the page fails to map, and have safe_copy_page() skip copying the
page if it fails to map. The existing behavior should crash in this case,
so this way there is a chance the system would be recoverable.

Use __weak for the arch breakout because there is not a suitable arch
specific header to use the #define method.

Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
---

v3:
 - Switch from hib_copy_page() breakout to hibernate_un/map_page()
   breakouts.
 - Since there is an error to use now, skip copying for unmappable pages

v2:
 - Rebase

 arch/x86/include/asm/fixmap.h |  3 +++
 arch/x86/power/hibernate.c    | 12 ++++++++++++
 include/linux/suspend.h       |  2 ++
 kernel/power/snapshot.c       | 22 ++++++++++++++--------
 4 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
index d0dcefb5cc59..0fceed9a4152 100644
--- a/arch/x86/include/asm/fixmap.h
+++ b/arch/x86/include/asm/fixmap.h
@@ -108,6 +108,9 @@ enum fixed_addresses {
 #ifdef CONFIG_PARAVIRT_XXL
 	FIX_PARAVIRT_BOOTMAP,
 #endif
+#ifdef CONFIG_HIBERNATION
+	FIX_HIBERNATE,
+#endif
 
 #ifdef CONFIG_ACPI_APEI_GHES
 	/* Used for GHES mapping from assorted contexts */
diff --git a/arch/x86/power/hibernate.c b/arch/x86/power/hibernate.c
index 6f955eb1e163..e7cde7cd529d 100644
--- a/arch/x86/power/hibernate.c
+++ b/arch/x86/power/hibernate.c
@@ -147,6 +147,18 @@ int arch_hibernation_header_restore(void *addr)
 	return 0;
 }
 
+long *hibernate_map_page(struct page *page)
+{
+	set_fixmap(FIX_HIBERNATE, page_to_phys(page));
+	__flush_tlb_all();
+	return (long *)fix_to_virt(FIX_HIBERNATE);
+}
+
+void hibernate_unmap_page(struct page *page)
+{
+	clear_fixmap(FIX_HIBERNATE);
+}
+
 int relocate_restore_code(void)
 {
 	pgd_t *pgd;
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index cfe19a028918..a6c3f7dac9e1 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -447,6 +447,8 @@ extern bool hibernation_available(void);
 asmlinkage int swsusp_save(void);
 extern struct pbe *restore_pblist;
 int pfn_is_nosave(unsigned long pfn);
+long *hibernate_map_page(struct page *page);
+void hibernate_unmap_page(struct page *page);
 
 int hibernate_quiet_exec(int (*func)(void *data), void *data);
 #else /* CONFIG_HIBERNATION */
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index cd8b7b35f1e8..8cc16114da75 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -83,19 +83,18 @@ static inline void hibernate_restore_unprotect_page(void *page_address) {}
  * It is still worth to have a warning here if something changes and this
  * will no longer be the case.
  */
-static inline void hibernate_map_page(struct page *page)
+long * __weak hibernate_map_page(struct page *page)
 {
 	if (IS_ENABLED(CONFIG_ARCH_HAS_SET_DIRECT_MAP)) {
-		int ret = set_direct_map_default_noflush(page);
-
-		if (ret)
-			pr_warn_once("Failed to remap page\n");
+		if (set_direct_map_default_noflush(page))
+			return NULL;
 	} else {
 		debug_pagealloc_map_pages(page, 1);
 	}
+	return page_address(page);
 }
 
-static inline void hibernate_unmap_page(struct page *page)
+void __weak hibernate_unmap_page(struct page *page)
 {
 	if (IS_ENABLED(CONFIG_ARCH_HAS_SET_DIRECT_MAP)) {
 		unsigned long addr = (unsigned long)page_address(page);
@@ -1394,8 +1393,15 @@ static void safe_copy_page(void *dst, struct page *s_page)
 	if (kernel_page_present(s_page)) {
 		do_copy_page(dst, page_address(s_page));
 	} else {
-		hibernate_map_page(s_page);
-		do_copy_page(dst, page_address(s_page));
+		long *src = hibernate_map_page(s_page);
+
+		if (!src) {
+			pr_warn_once("Failed to remap page\n");
+			return;
+		}
+
+		do_copy_page(dst, src);
+
 		hibernate_unmap_page(s_page);
 	}
 }
-- 
2.17.1

Powered by blists - more mailing lists