lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AF233D1473C1364ABD51D28909A1B1B7326E3282@PGSMSX105.gar.corp.intel.com>
Date:	Fri, 9 Jan 2015 01:00:17 +0000
From:	"Ong, Boon Leong" <boon.leong.ong@...el.com>
To:	Bryan O'Donoghue <pure.logic@...us-software.ie>
CC:	"tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
	"dvhart@...radead.org" <dvhart@...radead.org>,
	"platform-driver-x86@...r.kernel.org" 
	<platform-driver-x86@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"andy.shevchenko@...il.com" <andy.shevchenko@...il.com>
Subject: RE: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific
 setup

>Subject: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup
>
[snippet removed]

>Since the Quark EFI bringup code configures the system to reset on an IMR
Typo: bring-up

>violation, this means that common operations such as mouting an SD based root
Typo: mounting

[snippet removed]

>+config INTEL_GALILEO
>+	bool "Intel Galileo Platform Support"
>+	depends on X86_32 && PCI
>+	select IOSF_MBI
>+	select IMR
>+	---help---
>+	  Intel Galileo platform support. This code is used to tear-down
>+	  BIOS and Grub Isolated Memory Regions used during bootup.
Missing dash. Should be "boot-up".

[snippet removed]

>diff --git a/drivers/platform/x86/intel_galileo.c
>b/drivers/platform/x86/intel_galileo.c
>new file mode 100644
>index 0000000..2a555aa
>--- /dev/null
>+++ b/drivers/platform/x86/intel_galileo.c
>@@ -0,0 +1,175 @@
>+/*
>+ * intel_galileo.c - Intel Galileo platform support.
>+ *
>+ * Copyright(c) 2013 Intel Corporation.
>+ * Copyright(c) 2014 Bryan O'Donoghue <pure.logic@...us-software.ie>
>+ *
>+ * This platform code provides an entry point to do Galileo specific
>+ * setup. Critically IMRs are policed to ensure the EFI provided memory
Critically --> Critical

>+ * map informing the kernel of it's available memory is consistent with
It's --> its

[snippet removed]

>+
>+enum {
>+	GALILEO_UNKNOWN = 0,
>+	GALILEO_QRK_GEN1,
>+	GALILEO_QRK_GEN2,
>+};
Suggest to drop QRK to kill confusion that it is Quark Gen 1 & 2. 

[snippet removed]

>+#ifdef DEBUG
>+#define SANITY "IMR : sanity error "
>+
>+/**
Per coding style, please just use /* and kill the extra * above and below.

>+ * intel_galileo_imr_sanity
>+ * Verify IMR sanity with some simple tests to verify
>+ * overlap, zero sized allocations
>+ *
>+ * @base: Physical base address of the kernel text section
>+ * @size: Extent of kernel memory to be covered from &_text to
>+&_sinittext
>+ * @return: none
>+ */
>+static void __init
>+intel_galileo_imr_sanity(unsigned long base, unsigned long size) {
>+	/* Test zero zero */
>+	if (imr_add(0, 0, 0, 0, true) == 0)
>+		pr_err(SANITY "zero sized IMR @ 0x00000000\n");

A side-discussion on imr_add():
I would think that we should allow 1KiB IMR setting. Current imr_add() logic
is prohibiting it. So, the 'size' input should be at least 1KiB and imr_add()
internal logic will adjust 'hi' by -1KiB. Please consider ..  
  
>+
>+	/* Test overlap */
>+	if (imr_add(base, size, IMR_NON_SMM, IMR_NON_SMM, true) == 0)
>+		pr_err(SANITY "overlapped IMR @ (0x%08lx - 0x%08lx)\n",
>+		       base, base + size);
>+
>+	/* Try overlap - IMR_ALIGN */
>+	base = base + size - IMR_ALIGN;
>+	if (imr_add(base, size, IMR_NON_SMM, IMR_NON_SMM, true) == 0)
>+		pr_err(SANITY "overlapped IMR @ (0x%08lx - 0x%08lx)\n",
>+		       base, base + size);
>+}
>+#endif
>+
>+/**
>+ * intel_galileo_imr_init
>+ *
>+ * Tear down IMRs used during bootup. BIOS and Grub
boot-up.

>+ * 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. IMRs on Galileo are setup to
>+ * immediately reset the system on violation - so if you're
>+ * running a root filesystem from SD - you'll need the IMRs
>+ * torn down or you'll find seemingly random resets when using
>+ * your filesystem.
>+ */
>+static void __init intel_galileo_imr_init(void) {
>+	unsigned long base  = virt_to_phys(&_text);
>+	unsigned long size = virt_to_phys(&_sinittext) - base - IMR_ALIGN;
>+	int i, ret;
>+
>+	/* Tear down all existing unlocked IMRs */
>+	for (i = 0; i <= QUARK_X1000_IMR_NUM; i++)
>+		imr_del(i, 0, 0);
>+
>+	/*
>+	 * Setup an IMR around the physical extent of the kernel
>+	 * Non-SMM mode core read/write from/to kernel physical region.
>+	 * IMR locked.
>+	 */
>+	ret = imr_add(base, size, IMR_NON_SMM, IMR_NON_SMM, true);
>+	if (ret)
>+		pr_err("Unable to setup IMR for kernel: (%p - %p)\n",
>+			&_text, &__init_begin);
>+	else
>+		pr_info("IMR protect kernel memory: %ldk (%p - %p)\n",
>+			size >> 10, &_text, &__init_begin);
Perhaps we want to be consistent between using __init_begin & _sinittext?

>+#ifdef DEBUG
>+	intel_galileo_imr_sanity(base, size);
>+#endif
>+}
>+
>+/**
>+ * intel_galileo_init
>+ *
>+ * Identify a Galileo Gen1 or Gen2. If found run code to sanitise the
>+ * kernel memory space of IMRs that are inconsistent with the EFI memory
>map.
>+ */
>+static int __init intel_galileo_init(void) {
>+	int ret = 0, type = GALILEO_UNKNOWN;
type is assigned to either GALILEO_GEN1|2 below anyway.
So, we don't need to declare to UNKNOWN?

[snippet removed]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ