[PATCH v33 07/14] arm64: hibernate: preserve kdump image around hibernation

Hi Akashi,

On 15/03/17 09:59, AKASHI Takahiro wrote:

Since arch_kexec_protect_crashkres() removes a mapping for crash dump
kernel image, the loaded data won't be preserved around hibernation.

In this patch, helper functions, kexec_prepare_suspend()/
kexec_post_resume(), are additionally called before/after hibernation so
that the relevant memory segments will be mapped again and preserved just
as the others are.

In addition, to minimize the size of hibernation image,
kexec_is_chraskres_nosave() is added to pfn_is_nosave() in order to

(crashkres)

recoginize only the pages that hold loaded crash dump kernel image as

(recognize)

saveable. Hibernation excludes any pages that are marked as Reserved and
yet "nosave."

Neat! I didn't think this would be possible without hacking kernel/power/snapshot.c.

I've given this a spin on Juno and Seattle, I even added debug_pagealloc, but
that doesn't trick it because your kexec_prepare_suspend() puts the mapping back.

Reviewed-by: James Morse <james.morse@arm.com>


Thanks,

James


Some nits about comments:

Strictly this is kdump not kexec, but the comment makes that clear.

+
 		sleep_cpu = smp_processor_id();
 		ret = swsusp_save();
 	} else {

Stray whitespace change from a previous patch?

+/*
+ * kexec_is_crashres_nosave
+ *
+ * Return true only if a page is part of reserved memory for crash dump kernel,
+ * but does not hold any data of loaded kernel image.
+ *
+ * Note that all the pages in crash dump kernel memory have been initially
+ * marked as Reserved in kexec_reserve_crashkres_pages().
+ *
+ * In hibernation, the pages which are Reserved and yet "nosave"
+ * are excluded from the hibernation iamge. kexec_is_crashkres_nosave()
+ * does this check for crash dump kernel and will reduce the total size
+ * of hibernation image.
+ */
+
+bool kexec_is_crashkres_nosave(unsigned long pfn)
+{
+	int i;
+	phys_addr_t addr;
+
+	/* in reserved memory? */

Comment in the wrong place?

+	if (!crashk_res.end)
+		return false;
+
+	addr = __pfn_to_phys(pfn);

(makes more sense here)

+	if ((addr < crashk_res.start) || (crashk_res.end < addr))
+		return false;
+

+	/* not part of loaded kernel image? */

Comment in the wrong place?

+	if (!kexec_crash_image)
+		return true;
+

(makes more sense here)

+	for (i = 0; i < kexec_crash_image->nr_segments; i++)
+		if (addr >= kexec_crash_image->segment[i].mem &&
+				addr < (kexec_crash_image->segment[i].mem +
+					kexec_crash_image->segment[i].memsz))
+			return false;
+
+	return true;
+}
+

Thanks,

James