Message-Id: <1455766168-17335-1-git-send-email-pure.logic@nexus-software.ie>
Date:	Thu, 18 Feb 2016 03:29:28 +0000
From:	Bryan O'Donoghue <pure.logic@...us-software.ie>
To:	linux-kernel@...r.kernel.org
Cc:	corbet@....net, tglx@...utronix.de, mingo@...hat.com,
	hpa@...or.com, x86@...nel.org, andriy.shevchenko@...ux.intel.com,
	boon.leong.ong@...el.com, fengguang.wu@...el.com,
	linux-doc@...r.kernel.org,
	Bryan O'Donoghue <pure.logic@...us-software.ie>
Subject: [PATCH] x86/intel/quark: Parameterize the kernel's IMR lock logic

Currently when setting up an IMR around the kernel's .text area we lock
that IMR, preventing further modification. While superficially this appears
to be the right thing to do, in fact this doesn't account for a legitimate
change in the memory map such as when executing a new kernel via kexec.

In such a scenario a second kernel can have a different size and location
to it's predecessor and can view some of the memory occupied by it's
predecessor as legitimately usable DMA RAM. If this RAM were then
subsequently allocated to DMA agents within the system it could conceivably
trigger an IMR violation.

This patch fixes the this potential situation by keeping the kernel's .text
section IMR lock bit false by default, thus ensuring that a user of the
system will have kexec just work without having to pass special parameters
on the kernel command line to influence the state of the kernel's IMR. If a
user so desires then it is possible to explicitly set the lock bit to true.

The new parameter is kernel_imr and this may be set to kernel_imr=locked or
kernel_imr=unlocked.

Signed-off-by: Bryan O'Donoghue <pure.logic@...us-software.ie>
Reported-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
---
 Documentation/kernel-parameters.txt |  7 +++++++
 arch/x86/platform/intel-quark/imr.c | 39 +++++++++++++++++++++++++++++++------
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 9a53c92..1aad1d2 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1359,6 +1359,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			        hardware thread id mappings.
 				Format: <cpu>:<hwthread>
 
+	kernel_imr=	[X86, INTEL_IMR] Control the lock bit for the Isolated
+			Memory Region protecting the kernel's .text section on
+			X86 architectures that support IMRs such as Quark X1000.
+			When an IMR lock bit is set it is not possible to unset
+			it without a CPU reset.
+			Format : {locked | unlocked (default) }
+
 	keep_bootcon	[KNL]
 			Do not unregister boot console at start. This is only
 			useful for debugging when something happens in the window
diff --git a/arch/x86/platform/intel-quark/imr.c b/arch/x86/platform/intel-quark/imr.c
index c61b6c3..8249f65 100644
--- a/arch/x86/platform/intel-quark/imr.c
+++ b/arch/x86/platform/intel-quark/imr.c
@@ -37,6 +37,7 @@
 struct imr_device {
 	struct dentry	*file;
 	bool		init;
+	bool		kernel_imr_locked;
 	struct mutex	lock;
 	int		max_imr;
 	int		reg_base;
@@ -568,10 +569,15 @@ static inline int imr_clear(int reg)
  * BIOS and Grub both setup IMRs around compressed kernel, initrd memory
  * that need to be removed before the kernel hands out one of the IMR
  * encased addresses to a downstream DMA agent such as the SD or Ethernet.
+ * Additionally if the current kernel is executing via kexec then we need to
+ * tear down the IMR the previous kernel had setup.
+ *
  * IMRs on Galileo are setup to immediately reset the system on violation.
  * As a result if you're running a root filesystem from SD - you'll need
  * the boot-time IMRs torn down or you'll find seemingly random resets when
- * using your filesystem.
+ * using your filesystem; similarly if you're doing a kexec boot of Linux then
+ * its required to sanitize the memory map with-respect to the previous IMR
+ * settings.
  *
  * @idev:	pointer to imr_device structure.
  * @return:
@@ -592,14 +598,20 @@ static void __init imr_fixup_memmap(struct imr_device *idev)
 	end = (unsigned long)__end_rodata - 1;
 
 	/*
-	 * Setup a locked IMR around the physical extent of the kernel
-	 * from the beginning of the .text secton to the end of the
-	 * .rodata section as one physically contiguous block.
+	 * Setup an IMR around the physical extent of the kernel from the
+	 * beginning of the .text section to the end of the .rodata section
+	 * as one physically contiguous block.
 	 *
 	 * We don't round up @size since it is already PAGE_SIZE aligned.
 	 * See vmlinux.lds.S for details.
+	 *
+	 * By default this IMR is unlocked to enable subsequent kernels booted
+	 * by kexec to tear down the IMR we are setting up below. Its possible
+	 * to set this IMR to the locked state by passing kernel_imr=locked on
+	 * the kernel command line.
 	 */
-	ret = imr_add_range(base, size, IMR_CPU, IMR_CPU, true);
+	ret = imr_add_range(base, size, IMR_CPU, IMR_CPU,
+			    imr_dev.kernel_imr_locked);
 	if (ret < 0) {
 		pr_err("unable to setup IMR for kernel: %zu KiB (%lx - %lx)\n",
 			size / 1024, start, end);
@@ -617,8 +629,23 @@ static const struct x86_cpu_id imr_ids[] __initconst = {
 MODULE_DEVICE_TABLE(x86cpu, imr_ids);
 
 /**
- * imr_init - entry point for IMR driver.
+ * imr_kernel_lock_setup - control the lock bit of the kernel's IMR
  *
+ */
+static int __init imr_kernel_lock_setup(char *str)
+{
+	if (!strcmp(str, "unlocked"))
+		imr_dev.kernel_imr_locked = false;
+	else if (!strcmp(str, "locked"))
+		imr_dev.kernel_imr_locked = true;
+	else
+		return 0;
+	return 1;
+}
+__setup("kernel_imr=", imr_kernel_lock_setup);
+
+/**
+ * imr_init - entry point for IMR driver.
  * return: -ENODEV for no IMR support 0 if good to go.
  */
 static int __init imr_init(void)
-- 
2.5.0

Powered by blists - more mailing lists