All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/6] google firmware support
@ 2011-01-25  0:24 Mike Waychison
  2011-01-25  0:24 ` [PATCH v1 1/6] Add oops notification chain Mike Waychison
                   ` (6 more replies)
  0 siblings, 7 replies; 36+ messages in thread
From: Mike Waychison @ 2011-01-25  0:24 UTC (permalink / raw)
  To: Greg KH, torvalds
  Cc: San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel, Tim Hockin

This patchset applies to v2.6.38-rc2.

The following series implements support for interfaces exposed by
google's servers' firmware.

We'd like to have these small drivers included as they are required for
proper use of the kernel in our infrastructure.  They may not seem like
much, but a lot of our health automation as well as our human debugging
efforts are dependent on the functionality herein.

I'm pretty happy with the way this patchset looks and would like to ask
that they be merged at some point if there aren't any big objections.
Getting these in the public Linux tree would bring us closer to being
able to easily test kernels as they are released.

I wasn't certain who to send these patches to, please advise if I should
be CCing anyone else.

Thanks,

Patchset summary
================

[1] and [5] are the only ones that touch the 'core' kernel.


   - [1] adds a notifier_block that is called on Oops.

   - [2] introduces CONFIG_GOOGLE_FIRMWARE which all Google firmware
     drivers can depend upon.

   - [3] and [4] are drivers we use that are ready for inclusion.  [3]
     communicates with our EFI images via an SMI handshake.  [4] works
     with our older BIOSes to construct a log of reboot reasons.

   - [5] prepares for [6] by introducing prepend_to_dmesg() which
     allows drivers to prepend pre-kernel messages to the dmesg at
     bootup.

   - [6] uses the bits in [5].  It discovers the BIOSes memory log and
     prepends it to the dmesg during bootup.

Diffstat
========

 drivers/firmware/Kconfig             |   10 
 drivers/firmware/Makefile            |    2 
 drivers/firmware/google/Kconfig      |   39 +
 drivers/firmware/google/Makefile     |    4 
 drivers/firmware/google/bootlog.c    |  884 +++++++++++++++++++++++++++++++++
 drivers/firmware/google/gsmi.c       |  931 +++++++++++++++++++++++++++++++++++
 drivers/firmware/google/memconsole.c |  136 +++++
 fs/compat_ioctl.c                    |    7 
 include/linux/gsmi.h                 |  120 ++++
 include/linux/kernel.h               |    3 
 include/linux/miscdevice.h           |    1 
 include/linux/notifier.h             |    3 
 include/linux/printk.h               |    5 
 kernel/panic.c                       |   15 
 kernel/printk.c                      |   55 ++
 15 files changed, 2214 insertions(+), 1 deletion(-)

ChangeLog:
==========
- v1
   - Initial public send-out.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v1 1/6] Add oops notification chain.
  2011-01-25  0:24 [PATCH v1 0/6] google firmware support Mike Waychison
@ 2011-01-25  0:24 ` Mike Waychison
  2011-01-25  2:06   ` Greg KH
  2011-01-25  0:24 ` [PATCH v1 2/6] Introduce CONFIG_GOOGLE_FIRMWARE Mike Waychison
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-25  0:24 UTC (permalink / raw)
  To: Greg KH, torvalds
  Cc: San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel, Tim Hockin

From: Aaron Durbin <adurbin@google.com>

Later firmware patches in this series would like to be able to be
notified whenever an oops occurs on the system, so that it can be
recorded in the boot log.

This patch introduces a notifier_block called "oops_notifier_list"
so that drivers can register to get called whenever an Oops is
triggered.

Signed-off-by: Aaron Durbin <adurbin@google.com>
Signed-off-by: Mike Waychison <mikew@google.com>
---
 include/linux/kernel.h   |    3 +++
 include/linux/notifier.h |    3 +++
 kernel/panic.c           |   15 +++++++++++++++
 3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d07d805..2e56fed 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -183,6 +183,9 @@ extern void oops_enter(void);
 extern void oops_exit(void);
 void print_oops_end_marker(void);
 extern int oops_may_print(void);
+struct notifier_block;
+extern int register_oops_notifier(struct notifier_block *nb);
+extern int unregister_oops_notifier(struct notifier_block *nb);
 NORET_TYPE void do_exit(long error_code)
 	ATTRIB_NORET;
 NORET_TYPE void complete_and_exit(struct completion *, long)
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index 2026f9e..2a121bb 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -217,6 +217,9 @@ static inline int notifier_to_errno(int ret)
 #define SYS_HALT	0x0002	/* Notify of system halt */
 #define SYS_POWER_OFF	0x0003	/* Notify of system power off */
 
+#define OOPS_ENTER	0x0000	/* Notify OOPs has been entered */
+#define OOPS_EXIT	0x0001	/* Notify OOPs has exited */
+
 #define NETLINK_URELEASE	0x0001	/* Unicast netlink socket released */
 
 #define CPU_ONLINE		0x0002 /* CPU (unsigned)v is up */
diff --git a/kernel/panic.c b/kernel/panic.c
index 991bb87..45a50bb 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -37,6 +37,7 @@ int panic_timeout;
 EXPORT_SYMBOL_GPL(panic_timeout);
 
 ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
+static ATOMIC_NOTIFIER_HEAD(oops_notifier_list);
 
 EXPORT_SYMBOL(panic_notifier_list);
 
@@ -321,6 +322,7 @@ void oops_enter(void)
 	/* can't trust the integrity of the kernel anymore: */
 	debug_locks_off();
 	do_oops_enter_exit();
+	atomic_notifier_call_chain(&oops_notifier_list, OOPS_ENTER, NULL);
 }
 
 /*
@@ -355,6 +357,7 @@ void oops_exit(void)
 	do_oops_enter_exit();
 	print_oops_end_marker();
 	kmsg_dump(KMSG_DUMP_OOPS);
+	atomic_notifier_call_chain(&oops_notifier_list, OOPS_EXIT, NULL);
 }
 
 #ifdef WANT_WARN_ON_SLOWPATH
@@ -431,5 +434,17 @@ EXPORT_SYMBOL(__stack_chk_fail);
 
 #endif
 
+int register_oops_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_register(&oops_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(register_oops_notifier);
+
+int unregister_oops_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_unregister(&oops_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_oops_notifier);
+
 core_param(panic, panic_timeout, int, 0644);
 core_param(pause_on_oops, pause_on_oops, int, 0644);


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 2/6] Introduce CONFIG_GOOGLE_FIRMWARE
  2011-01-25  0:24 [PATCH v1 0/6] google firmware support Mike Waychison
  2011-01-25  0:24 ` [PATCH v1 1/6] Add oops notification chain Mike Waychison
@ 2011-01-25  0:24 ` Mike Waychison
  2011-01-25  0:24 ` [PATCH v1 3/6] driver: Google EFI SMI Mike Waychison
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 36+ messages in thread
From: Mike Waychison @ 2011-01-25  0:24 UTC (permalink / raw)
  To: Greg KH, torvalds
  Cc: San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel, Tim Hockin

In order to keep Google's firmware drivers organized amongst themselves,
create a new directory for them to live in.  As well, all Google
firmware drivers are gated on CONFIG_GOOGLE_FIRMWARE=y, which defaults
to 'n' in the kernel build.

Signed-off-by: Mike Waychison <mikew@google.com>
---
 drivers/firmware/Kconfig         |    2 ++
 drivers/firmware/Makefile        |    2 ++
 drivers/firmware/google/Kconfig  |   14 ++++++++++++++
 drivers/firmware/google/Makefile |    1 +
 4 files changed, 19 insertions(+), 0 deletions(-)
 create mode 100644 drivers/firmware/google/Kconfig
 create mode 100644 drivers/firmware/google/Makefile

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index e710424..d848b26 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -134,4 +134,6 @@ config ISCSI_IBFT
 	  detect iSCSI boot parameters dynamically during system boot, say Y.
 	  Otherwise, say N.
 
+source "drivers/firmware/google/Kconfig"
+
 endmenu
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 1c3c173..6f68007 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -11,3 +11,5 @@ obj-$(CONFIG_DMIID)		+= dmi-id.o
 obj-$(CONFIG_ISCSI_IBFT_FIND)	+= iscsi_ibft_find.o
 obj-$(CONFIG_ISCSI_IBFT)	+= iscsi_ibft.o
 obj-$(CONFIG_FIRMWARE_MEMMAP)	+= memmap.o
+
+obj-$(CONFIG_GOOGLE_FIRMWARE)	+= google/
diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
new file mode 100644
index 0000000..7834729
--- /dev/null
+++ b/drivers/firmware/google/Kconfig
@@ -0,0 +1,14 @@
+config GOOGLE_FIRMWARE
+	bool "Google Firmware Drivers"
+	depends on X86
+	default n
+	help
+	  These firmware drivers are used by Google's servers.  They are
+	  only useful if you are working directly on one of their
+	  proprietary servers or implementing similar firmware
+	  interfaces.  If in doubt, say "N".
+
+menu "Google Firmware Drivers"
+	depends on GOOGLE_FIRMWARE
+
+endmenu
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
new file mode 100644
index 0000000..8b13789
--- /dev/null
+++ b/drivers/firmware/google/Makefile
@@ -0,0 +1 @@
+


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-25  0:24 [PATCH v1 0/6] google firmware support Mike Waychison
  2011-01-25  0:24 ` [PATCH v1 1/6] Add oops notification chain Mike Waychison
  2011-01-25  0:24 ` [PATCH v1 2/6] Introduce CONFIG_GOOGLE_FIRMWARE Mike Waychison
@ 2011-01-25  0:24 ` Mike Waychison
  2011-01-25  3:17   ` Greg KH
  2011-01-25  0:24 ` [PATCH v1 4/6] driver: Google Bootlog Mike Waychison
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-25  0:24 UTC (permalink / raw)
  To: Greg KH, torvalds
  Cc: San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel, Tim Hockin

The "gsmi" driver bridges userland with firmware specific routines for
accessing hardware.

Currently, this driver only supports NVRAM and eventlog information.
Deprecated functions have been removed from the driver, though their
op-codes are left in place so that they are not re-used.

This driver works by trampolining into the firmware via the smi_command
outlined in the FADT table.  Three protocols are used due to various
limitations over time, but all are included herein.

Signed-off-by: Duncan Laurie <dlaurie@google.com>
Signed-off-by: Aaron Durbin <adurbin@google.com>
Signed-off-by: Mike Waychison <mikew@google.com>
---
 drivers/firmware/google/Kconfig  |   10 
 drivers/firmware/google/Makefile |    1 
 drivers/firmware/google/gsmi.c   |  931 ++++++++++++++++++++++++++++++++++++++
 fs/compat_ioctl.c                |    7 
 include/linux/gsmi.h             |  120 +++++
 include/linux/miscdevice.h       |    1 
 6 files changed, 1070 insertions(+), 0 deletions(-)
 create mode 100644 drivers/firmware/google/gsmi.c
 create mode 100644 include/linux/gsmi.h

diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 7834729..2d2be7c 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -11,4 +11,14 @@ config GOOGLE_FIRMWARE
 menu "Google Firmware Drivers"
 	depends on GOOGLE_FIRMWARE
 
+config GOOGLE_SMI
+	bool "SMI interface for Google platforms"
+	depends on ACPI
+	default y
+	help
+	  Say Y here if you want to enable SMI callbacks for Google
+	  platforms.  This provides an interface for writing to and
+	  clearing the EFI event log and reading and writing NVRAM
+	  variables.
+
 endmenu
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
index 8b13789..fb127d7 100644
--- a/drivers/firmware/google/Makefile
+++ b/drivers/firmware/google/Makefile
@@ -1 +1,2 @@
 
+obj-$(CONFIG_GOOGLE_SMI)		+= gsmi.o
diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/google/gsmi.c
new file mode 100644
index 0000000..682f594
--- /dev/null
+++ b/drivers/firmware/google/gsmi.c
@@ -0,0 +1,931 @@
+/*
+ * Copyright 2010 Google Inc. All Rights Reserved.
+ * Author: dlaurie@google.com (Duncan Laurie)
+ *
+ * EFI SMI interface for Google platforms
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/spinlock.h>
+#include <linux/miscdevice.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/ioctl.h>
+#include <linux/acpi.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+#include <linux/dmi.h>
+#include <linux/gsmi.h>
+#include <linux/kdebug.h>
+#include <linux/reboot.h>
+
+#define GSMI_SHUTDOWN_CLEAN	0	/* Clean Shutdown */
+#define GSMI_SHUTDOWN_NMIWDT	1	/* NMI Watchdog */
+#define GSMI_SHUTDOWN_PANIC	2	/* Panic */
+#define GSMI_SHUTDOWN_OOPS	3	/* Oops */
+#define GSMI_SHUTDOWN_DIE	4	/* Die */
+#define GSMI_SHUTDOWN_MCE	5	/* Machine Check */
+#define GSMI_SHUTDOWN_SOFTWDT	6	/* Software Watchdog */
+#define GSMI_SHUTDOWN_MBE	7	/* Uncorrected ECC */
+#define GSMI_SHUTDOWN_TRIPLE	8	/* Triple Fault */
+
+#define DRIVER_VERSION		"1.0"
+#define GSMI_GUID_SIZE		16
+#define GSMI_BUF_SIZE		1024
+#define GSMI_BUF_ALIGN		sizeof(u64)
+#define GSMI_CALLBACK		0xef
+
+/* SMI return codes */
+#define GSMI_SUCCESS		0x00
+#define GSMI_UNSUPPORTED2	0x03
+#define GSMI_LOG_FULL		0x0b
+#define GSMI_VAR_NOT_FOUND	0x0e
+#define GSMI_HANDSHAKE_SPIN	0x7d
+#define GSMI_HANDSHAKE_CF	0x7e
+#define GSMI_HANDSHAKE_NONE	0x7f
+#define GSMI_INVALID_PARAMETER	0x82
+#define GSMI_UNSUPPORTED	0x83
+#define GSMI_BUFFER_TOO_SMALL	0x85
+#define GSMI_NOT_READY		0x86
+#define GSMI_DEVICE_ERROR	0x87
+#define GSMI_NOT_FOUND		0x8e
+
+#define QUIRKY_BOARD_HASH 0x78a30a50
+
+/* SMI buffers must be in 32bit physical address space */
+struct gsmi_buf {
+	uint8_t *start;			/* start of buffer */
+	size_t length;			/* length of buffer */
+	dma_addr_t handle;		/* dma allocation handle */
+	uint32_t address;		/* physical address of buffer */
+};
+
+struct gsmi_device {
+	struct platform_device *pdev;	/* platform device */
+	struct gsmi_buf *name_buf;	/* variable name buffer */
+	struct gsmi_buf *data_buf;	/* generic data buffer */
+	struct gsmi_buf *param_buf;	/* parameter buffer */
+	spinlock_t lock;		/* serialize access to SMIs */
+	uint16_t smi_cmd;		/* SMI command port */
+	int handshake_type;		/* firmware handler interlock type */
+	struct dma_pool *dma_pool;	/* DMA buffer pool */
+};
+
+struct gsmi_nvram_var_param {
+	uint8_t  guid[GSMI_GUID_SIZE];
+	uint32_t name_ptr;
+	uint32_t attributes;
+	uint32_t data_len;
+	uint32_t data_ptr;
+} __packed;
+
+struct gsmi_get_next_var_param {
+	uint8_t  guid[GSMI_GUID_SIZE];
+	uint32_t name_ptr;
+	uint32_t name_len;
+} __packed;
+
+struct gsmi_set_event_log_param {
+	uint32_t data_ptr;
+	uint32_t data_len;
+	uint32_t type;
+} __packed;
+
+static int gsmi_cmd_get_nvram_var(struct gsmi_ioctl *ctl);
+static int gsmi_cmd_get_next_var(struct gsmi_ioctl *ctl);
+static int gsmi_cmd_set_nvram_var(struct gsmi_ioctl *ctl);
+static int gsmi_cmd_set_event_log(struct gsmi_ioctl *ctl);
+static int gsmi_cmd_clear_event_log(struct gsmi_ioctl *ctl);
+static int gsmi_cmd_clear_config(struct gsmi_ioctl *ctl);
+static int gsmi_cmd_noop(struct gsmi_ioctl *ctl);
+
+/* list of ioctl command handlers */
+static int (*gsmi_cmd_handler[])(struct gsmi_ioctl *ctl) = {
+	[GSMI_CMD_GET_NVRAM_VAR]    = gsmi_cmd_get_nvram_var,
+	[GSMI_CMD_GET_NEXT_VAR]	    = gsmi_cmd_get_next_var,
+	[GSMI_CMD_SET_NVRAM_VAR]    = gsmi_cmd_set_nvram_var,
+	[GSMI_CMD_SET_EVENT_LOG]    = gsmi_cmd_set_event_log,
+	[GSMI_CMD_CLEAR_EVENT_LOG]  = gsmi_cmd_clear_event_log,
+	[GSMI_CMD_CLEAR_CONFIG]     = gsmi_cmd_clear_config,
+	[GSMI_CMD_NOOP]             = gsmi_cmd_noop,
+};
+
+static long gsmi_ioctl(struct file *file, unsigned int cmd,
+		       unsigned long arg);
+
+static const struct file_operations gsmi_fops = {
+	.owner		= THIS_MODULE,
+	.llseek		= no_llseek,
+	.unlocked_ioctl	= gsmi_ioctl,
+};
+
+static struct miscdevice gsmi_miscdev = {
+	.minor	= GOOGLE_SMI_MINOR,
+	.name	= "gsmi",
+	.fops	= &gsmi_fops,
+};
+
+/* global device handle */
+static struct gsmi_device *gsmi_dev;
+
+/*
+ * Some platforms don't have explicit SMI handshake
+ * and need to wait for SMI to complete.
+ */
+#define GSMI_DEFAULT_SPINCOUNT	0x10000
+static uint32_t gsmi_spincount = GSMI_DEFAULT_SPINCOUNT;
+
+static int __init gsmi_setup_spincount(char *str)
+{
+	unsigned long res;
+	if (!strict_strtoul(str, 0, &res))
+		gsmi_spincount = (uint32_t)res;
+	return 1;
+}
+__setup("gsmi_spincount=", gsmi_setup_spincount);
+
+static struct gsmi_buf *gsmi_buf_alloc(void)
+{
+	struct gsmi_buf *smibuf;
+
+	smibuf = kzalloc(sizeof(*smibuf), GFP_KERNEL);
+	if (!smibuf) {
+		printk(KERN_ERR "gsmi: out of memory\n");
+		return NULL;
+	}
+
+	/* allocate buffer in 32bit address space */
+	smibuf->start = dma_pool_alloc(gsmi_dev->dma_pool, GFP_KERNEL,
+				       &smibuf->handle);
+	if (!smibuf->start) {
+		printk(KERN_ERR "gsmi: failed to allocate name buffer\n");
+		kfree(smibuf);
+		return NULL;
+	}
+
+	/* fill in the buffer handle */
+	smibuf->length = GSMI_BUF_SIZE;
+	smibuf->address = (uint32_t)virt_to_phys(smibuf->start);
+	memset(smibuf->start, 0, GSMI_BUF_SIZE);
+
+	return smibuf;
+}
+
+static void gsmi_buf_free(struct gsmi_buf *smibuf)
+{
+	if (smibuf) {
+		if (smibuf->start)
+			dma_pool_free(gsmi_dev->dma_pool, smibuf->start,
+				      smibuf->handle);
+		kfree(smibuf);
+		smibuf = NULL;
+	}
+}
+
+static int gsmi_exec(uint8_t func, uint8_t sub)
+{
+	uint16_t cmd = (sub << 8) | func;
+	uint16_t result = 0;
+	int rc = 0;
+
+	/*
+	 * AH  : Subfunction number
+	 * AL  : Function number
+	 * EBX : Parameter block address
+	 * DX  : SMI command port
+	 *
+	 * Three protocols here. See also the comment in gsmi_init().
+	 */
+	if (gsmi_dev->handshake_type == GSMI_HANDSHAKE_CF) {
+		/*
+		 * If handshake_type == HANDSHAKE_CF then set CF on the
+		 * way in and wait for the handler to clear it; this avoids
+		 * corrupting register state on those chipsets which have
+		 * a delay between writing the SMI trigger register and
+		 * entering SMM.
+		 */
+		asm volatile (
+			"stc\n"
+			"outb %%al, %%dx\n"
+		"1:      jc 1b\n"
+			"mov %%ax, %0"
+			: "=r" (result)
+			: "a" (cmd),
+			  "d" (gsmi_dev->smi_cmd),
+			  "b" (gsmi_dev->param_buf->address)
+			: "memory", "cc"
+		);
+	} else if (gsmi_dev->handshake_type == GSMI_HANDSHAKE_SPIN) {
+		/*
+		 * If handshake_type == HANDSHAKE_SPIN we spin a
+		 * hundred-ish usecs to ensure the SMI has triggered.
+		 */
+		asm volatile (
+			"outb %%al, %%dx\n"
+		"1:      loop 1b\n"
+			"mov %%ax, %0"
+			: "=r" (result)
+			: "a" (cmd),
+			  "d" (gsmi_dev->smi_cmd),
+			  "b" (gsmi_dev->param_buf->address),
+			  "c" (gsmi_spincount)
+			: "memory", "cc"
+		);
+	} else {
+		/*
+		 * If handshake_type == HANDSHAKE_NONE we do nothing;
+		 * either we don't need to or it's legacy firmware that
+		 * doesn't understand the CF protocol.
+		 */
+		asm volatile (
+			"outb %%al, %%dx\n\t"
+			"mov %%ax, %0"
+			: "=r" (result)
+			: "a" (cmd),
+			  "d" (gsmi_dev->smi_cmd),
+			  "b" (gsmi_dev->param_buf->address)
+			: "memory", "cc"
+		);
+	}
+
+	/* check return code from SMI handler */
+	switch (result) {
+	case GSMI_SUCCESS:
+		break;
+	case GSMI_VAR_NOT_FOUND:
+		/* not really an error, but let the caller know */
+		rc = 1;
+		break;
+	case GSMI_INVALID_PARAMETER:
+		printk(KERN_ERR "gsmi: exec 0x%04x: Invalid parameter\n", cmd);
+		rc = -EINVAL;
+		break;
+	case GSMI_BUFFER_TOO_SMALL:
+		printk(KERN_ERR "gsmi: exec 0x%04x: Buffer too small\n", cmd);
+		rc = -ENOMEM;
+		break;
+	case GSMI_UNSUPPORTED:
+	case GSMI_UNSUPPORTED2:
+		if (sub != GSMI_CMD_HANDSHAKE_TYPE)
+			printk(KERN_ERR "gsmi: exec 0x%04x: Not supported\n",
+			       cmd);
+		rc = -ENOSYS;
+		break;
+	case GSMI_NOT_READY:
+		printk(KERN_ERR "gsmi: exec 0x%04x: Not ready\n", cmd);
+		rc = -EBUSY;
+		break;
+	case GSMI_DEVICE_ERROR:
+		printk(KERN_ERR "gsmi: exec 0x%04x: Device error\n", cmd);
+		rc = -EFAULT;
+		break;
+	case GSMI_NOT_FOUND:
+		printk(KERN_ERR "gsmi: exec 0x%04x: Data not found\n", cmd);
+		rc = -ENOENT;
+		break;
+	case GSMI_LOG_FULL:
+		printk(KERN_ERR "gsmi: exec 0x%04x: Log full\n", cmd);
+		rc = -ENOSPC;
+		break;
+	case GSMI_HANDSHAKE_CF:
+	case GSMI_HANDSHAKE_SPIN:
+	case GSMI_HANDSHAKE_NONE:
+		rc = result;
+		break;
+	default:
+		printk(KERN_ERR "gsmi: exec 0x%04x: Unknown error 0x%04x\n",
+		       cmd, result);
+		rc = -ENXIO;
+	}
+
+	return rc;
+}
+
+static int gsmi_cmd_get_nvram_var(struct gsmi_ioctl *ctl)
+{
+	struct gsmi_get_nvram_var *cmd = &ctl->gsmi_cmd.get_nvram_var;
+	struct gsmi_nvram_var_param param = {
+		.name_ptr = gsmi_dev->name_buf->address,
+		.data_ptr = gsmi_dev->data_buf->address,
+		.data_len = cmd->data_len,
+	};
+	uint16_t var_name[GSMI_BUF_SIZE / 2];
+	int i, rc = 0;
+	unsigned long flags;
+
+	if (cmd->name_len > GSMI_BUF_SIZE / 2 ||
+	    cmd->name_len > sizeof(cmd->name))
+		return -1;
+
+	spin_lock_irqsave(&gsmi_dev->lock, flags);
+
+	/* guid */
+	memcpy(&param.guid, cmd->guid, GSMI_GUID_SIZE);
+
+	/* variable name, converted from ascii to UTF-16 */
+	memset(var_name, 0, sizeof(var_name));
+	for (i = 0; i < cmd->name_len; i++)
+		var_name[i] = cmd->name[i];
+	memset(gsmi_dev->name_buf->start, 0, gsmi_dev->name_buf->length);
+	memcpy(gsmi_dev->name_buf->start, var_name, cmd->name_len * 2);
+
+	/* data pointer */
+	memset(gsmi_dev->data_buf->start, 0, gsmi_dev->data_buf->length);
+
+	/* parameter buffer */
+	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
+	memcpy(gsmi_dev->param_buf->start, &param, sizeof(param));
+
+	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
+	if (rc < 0) {
+		printk(KERN_ERR "gsmi: Get Variable %s failed\n", cmd->name);
+	} else if (rc == 1) {
+		/* variable was not found */
+		rc = -1;
+	} else {
+		/* copy data back to return buffer */
+		memcpy(cmd->data, gsmi_dev->data_buf->start, cmd->data_len);
+		rc = 1;
+	}
+
+	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
+
+	return rc;
+}
+
+static int gsmi_cmd_get_next_var(struct gsmi_ioctl *ctl)
+{
+	struct gsmi_get_next_var *cmd = &ctl->gsmi_cmd.get_next_var;
+	struct gsmi_get_next_var_param param = {
+		.name_ptr = gsmi_dev->name_buf->address,
+		.name_len = gsmi_dev->name_buf->length,
+	};
+	uint16_t var_name[GSMI_BUF_SIZE / 2];
+	int i, rc = 0;
+	unsigned long flags;
+
+	if (cmd->name_len > GSMI_BUF_SIZE / 2 ||
+	    cmd->name_len > sizeof(cmd->name))
+		return -1;
+
+	spin_lock_irqsave(&gsmi_dev->lock, flags);
+
+	/* guid */
+	memcpy(&param.guid, cmd->guid, sizeof(param.guid));
+
+	/* variable name, converted from ascii to UTF-16 */
+	memset(var_name, 0, sizeof(var_name));
+	for (i = 0; i < cmd->name_len; i++)
+		var_name[i] = cmd->name[i];
+	memset(gsmi_dev->name_buf->start, 0, gsmi_dev->name_buf->length);
+	memcpy(gsmi_dev->name_buf->start, var_name, cmd->name_len * 2);
+
+	/* parameter buffer */
+	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
+	memcpy(gsmi_dev->param_buf->start, &param, sizeof(param));
+
+	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
+	if (rc < 0) {
+		printk(KERN_ERR "gsmi: Get Next Variable Name failed\n");
+	} else if (rc == 1) {
+		/* variable not found -- end of list */
+		memset(cmd->guid, 0, sizeof(cmd->guid));
+		memset(cmd->name, 0, cmd->name_len);
+		cmd->name_len = 0;
+		rc = 1;
+	} else {
+		/* copy variable data back to return buffer */
+		memcpy(&param, gsmi_dev->param_buf->start, sizeof(param));
+
+		/* convert variable name back to ascii */
+		memset(var_name, 0, sizeof(var_name));
+		memcpy(var_name, gsmi_dev->name_buf->start, param.name_len);
+		memset(cmd->name, 0, sizeof(cmd->name));
+		cmd->name_len = param.name_len / 2;
+		for (i = 0; i < cmd->name_len; i++)
+			cmd->name[i] = var_name[i];
+
+		/* copy guid to return buffer */
+		memcpy(cmd->guid, &param.guid, sizeof(cmd->guid));
+		rc = 1;
+	}
+
+	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
+
+	return rc;
+}
+
+static int gsmi_cmd_set_nvram_var(struct gsmi_ioctl *ctl)
+{
+	struct gsmi_set_nvram_var *cmd = &ctl->gsmi_cmd.set_nvram_var;
+	struct gsmi_nvram_var_param param = {
+		.name_ptr = gsmi_dev->name_buf->address,
+		.data_ptr = gsmi_dev->data_buf->address,
+		.data_len = cmd->data_len,
+		.attributes = 0x7, /* nvram, boot, runtime */
+	};
+	uint16_t var_name[GSMI_BUF_SIZE / 2];
+	int i, rc = 0;
+	unsigned long flags;
+
+	if (cmd->name_len > GSMI_BUF_SIZE / 2 ||
+	    cmd->name_len > sizeof(cmd->name))
+		return -1;
+
+	spin_lock_irqsave(&gsmi_dev->lock, flags);
+
+	/* guid */
+	memcpy(&param.guid, cmd->guid, sizeof(param.guid));
+
+	/* variable name, converted from ascii to UTF-16 */
+	memset(var_name, 0, sizeof(var_name));
+	for (i = 0; i < cmd->name_len; i++)
+		var_name[i] = cmd->name[i];
+	memset(gsmi_dev->name_buf->start, 0, gsmi_dev->name_buf->length);
+	memcpy(gsmi_dev->name_buf->start, var_name, cmd->name_len * 2);
+
+	/* data pointer */
+	memset(gsmi_dev->data_buf->start, 0, gsmi_dev->data_buf->length);
+	memcpy(gsmi_dev->data_buf->start, cmd->data, cmd->data_len);
+
+	/* parameter buffer */
+	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
+	memcpy(gsmi_dev->param_buf->start, &param, sizeof(param));
+
+	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
+	if (rc < 0)
+		printk(KERN_ERR "gsmi: Set Variable %s failed\n", cmd->name);
+
+	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
+
+	return rc;
+}
+
+static int gsmi_cmd_set_event_log(struct gsmi_ioctl *ctl)
+{
+	struct gsmi_set_event_log *cmd = &ctl->gsmi_cmd.set_event_log;
+	struct gsmi_set_event_log_param param = {
+		.data_ptr = gsmi_dev->data_buf->address,
+		.data_len = cmd->data_len,
+		.type     = cmd->type,
+	};
+	int rc = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&gsmi_dev->lock, flags);
+
+	/* data pointer */
+	memset(gsmi_dev->data_buf->start, 0, gsmi_dev->data_buf->length);
+	memcpy(gsmi_dev->data_buf->start, cmd->data, cmd->data_len);
+
+	/* parameter buffer */
+	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
+	memcpy(gsmi_dev->param_buf->start, &param, sizeof(param));
+
+	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
+	if (rc < 0)
+		printk(KERN_ERR "gsmi: Set Event Log failed\n");
+
+	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
+
+	return rc;
+}
+
+static int gsmi_cmd_clear_event_log(struct gsmi_ioctl *ctl)
+{
+	struct gsmi_clear_event_log *cmd = &ctl->gsmi_cmd.clear_event_log;
+	int rc = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&gsmi_dev->lock, flags);
+
+	/* parameter buffer */
+	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
+	memcpy(gsmi_dev->param_buf->start, cmd, sizeof(*cmd));
+
+	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
+	if (rc < 0)
+		printk(KERN_ERR "gsmi: Clear Event Log failed\n");
+
+	/* type 0 will return available space in log */
+	if (cmd->type == 0)
+		rc = 1;
+
+	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
+
+	return rc;
+}
+
+static int gsmi_cmd_clear_config(struct gsmi_ioctl *ctl)
+{
+	int rc = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&gsmi_dev->lock, flags);
+
+	/* clear parameter buffer */
+	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
+
+	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
+	if (rc < 0)
+		printk(KERN_ERR "gsmi: Clear Config failed\n");
+
+	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
+
+	return rc;
+}
+
+static int gsmi_cmd_noop(struct gsmi_ioctl *ctl)
+{
+	int rc = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&gsmi_dev->lock, flags);
+
+	/* clear parameter buffer */
+	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
+
+	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
+	if (rc < 0)
+		printk(KERN_ERR "gsmi: No-op failed\n");
+
+	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
+
+	return rc;
+}
+
+static long gsmi_ioctl(struct file *file, unsigned int cmd,
+		      unsigned long arg)
+{
+	struct gsmi_ioctl *ctl = NULL;
+	uint16_t length;
+	int rc = 0;
+
+	/* everything is in the same ioctl */
+	if (cmd != GSMI_IOCTL)
+		return -ENOIOCTLCMD;
+
+	/* get length from user */
+	if (copy_from_user(&length, (size_t __user *)arg,
+			   sizeof(length))) {
+		rc = -EFAULT;
+		goto done;
+	}
+
+	/* verify length */
+	if (length < sizeof(*ctl)) {
+		rc = -EINVAL;
+		goto done;
+	}
+
+	/* allocate ioctl structure */
+	ctl = kzalloc(length, GFP_KERNEL);
+	if (!ctl) {
+		rc = -ENOMEM;
+		goto done;
+	}
+
+	/* copy rest of structure */
+	if (copy_from_user(ctl, (void __user *)arg, length)) {
+		rc = -EFAULT;
+		goto done;
+	}
+
+	/* verify structure version */
+	if (ctl->version != GSMI_IOCTL_VERSION) {
+		rc = -EINVAL;
+		goto done;
+	}
+
+	/* find and run command handler */
+	if (ctl->command > sizeof(gsmi_cmd_handler)/sizeof(*gsmi_cmd_handler)) {
+		rc = -EINVAL;
+		goto done;
+	}
+	if (gsmi_cmd_handler[ctl->command] == NULL) {
+		rc = -EINVAL;
+		goto done;
+	}
+	rc = gsmi_cmd_handler[ctl->command](ctl);
+	if (rc < 0) {
+		goto done;
+	} else if (rc > 0) {
+		/* return data to user if needed */
+		if (copy_to_user((void __user *)arg, ctl, length)) {
+			rc = -EFAULT;
+			goto done;
+		}
+		rc = 0;
+	}
+
+ done:
+	kfree(ctl);
+
+	return rc;
+}
+
+static int gsmi_shutdown_reason(int reason)
+{
+	struct gsmi_log_entry_type_1 entry = {
+		.type     = GSMI_LOG_ENTRY_TYPE_KERNEL,
+		.instance = reason,
+	};
+	struct gsmi_set_event_log_param param = {
+		.data_len = sizeof(entry),
+		.type     = 1,
+	};
+	static int saved_reason;
+	int rc = 0;
+	unsigned long flags;
+
+	/* abort early if we were never setup */
+	if (!gsmi_dev)
+		return -1;
+
+	/* avoid duplicate entries in the log */
+	if (saved_reason & (1 << reason))
+		return 0;
+
+	spin_lock_irqsave(&gsmi_dev->lock, flags);
+
+	saved_reason |= (1 << reason);
+
+	/* data pointer */
+	memset(gsmi_dev->data_buf->start, 0, gsmi_dev->data_buf->length);
+	memcpy(gsmi_dev->data_buf->start, &entry, sizeof(entry));
+
+	/* parameter buffer */
+	param.data_ptr = gsmi_dev->data_buf->address;
+	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
+	memcpy(gsmi_dev->param_buf->start, &param, sizeof(param));
+
+	rc = gsmi_exec(GSMI_CALLBACK, GSMI_CMD_SET_EVENT_LOG);
+
+	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
+
+	if (rc < 0)
+		printk(KERN_ERR "gsmi: Log Shutdown Reason failed\n");
+	else
+		printk(KERN_EMERG "gsmi: Log Shutdown Reason 0x%02x\n",
+		       reason);
+
+	return rc;
+}
+
+static int gsmi_reboot_callback(struct notifier_block *nb,
+				unsigned long reason, void *arg)
+{
+	gsmi_shutdown_reason(GSMI_SHUTDOWN_CLEAN);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block gsmi_reboot_notifier = {
+	.notifier_call = gsmi_reboot_callback
+};
+
+static int gsmi_die_callback(struct notifier_block *nb,
+			     unsigned long reason, void *arg)
+{
+	if (reason == DIE_NMIWATCHDOG)
+		gsmi_shutdown_reason(GSMI_SHUTDOWN_NMIWDT);
+	else if (reason == DIE_OOPS)
+		gsmi_shutdown_reason(GSMI_SHUTDOWN_DIE);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block gsmi_die_notifier = {
+	.notifier_call = gsmi_die_callback
+};
+
+static int gsmi_panic_callback(struct notifier_block *nb,
+			       unsigned long reason, void *arg)
+{
+	gsmi_shutdown_reason(GSMI_SHUTDOWN_PANIC);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block gsmi_panic_notifier = {
+	.notifier_call = gsmi_panic_callback
+};
+
+static int gsmi_oops_callback(struct notifier_block *nb,
+			      unsigned long reason, void *arg)
+{
+	if (reason == OOPS_ENTER)
+		gsmi_shutdown_reason(GSMI_SHUTDOWN_OOPS);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block gsmi_oops_notifier = {
+	.notifier_call = gsmi_oops_callback
+};
+
+/*
+ * This hash function was blatantly copied from include/linux/hash.h.
+ * It is used by this driver to obfuscate a board name that requires a
+ * quirk within this driver.
+ *
+ * Please do not remove this copy of the function as any changes to the
+ * global utility hash_64() function would break this driver's ability
+ * to identify a board and provide the appropriate quirk -- mikew@google.com
+ */
+static u64 __init local_hash_64(u64 val, unsigned bits)
+{
+	u64 hash = val;
+
+	/*  Sigh, gcc can't optimise this alone like it does for 32 bits. */
+	u64 n = hash;
+	n <<= 18;
+	hash -= n;
+	n <<= 33;
+	hash -= n;
+	n <<= 3;
+	hash += n;
+	n <<= 3;
+	hash -= n;
+	n <<= 4;
+	hash += n;
+	n <<= 2;
+	hash += n;
+
+	/* High bits are more random, so use them. */
+	return hash >> (64 - bits);
+}
+
+static u32 __init hash_oem_table_id(char s[8])
+{
+	u64 input;
+	memcpy(&input, s, 8);
+	return local_hash_64(input, 32);
+}
+
+static __init int gsmi_init(void)
+{
+	unsigned long flags;
+	u32 hash;
+
+	/*
+	 * Check platform compatibility.  Only ever load on Google's
+	 * machines.
+	 */
+	if (strncmp(acpi_gbl_FADT.header.oem_id, "GOOGLE", ACPI_OEM_ID_SIZE)) {
+		printk(KERN_INFO "gsmi: Not a google board\n");
+		return -ENODEV;
+	}
+
+	/*
+	 * Only newer firmware supports the gsmi interface.  All older
+	 * firmware that didn't support this interface used to plug the
+	 * table name in the first four bytes of the oem_table_id field.
+	 * Newer firmware doesn't do that though, so use that as the
+	 * discriminant factor.  We have to do this in order to
+	 * whitewash our board names out of the public driver.
+	 */
+	if (!strncmp(acpi_gbl_FADT.header.oem_table_id, "FACP", 4)) {
+		printk(KERN_INFO "gsmi: Board is too old\n");
+		return -ENODEV;
+	}
+
+	/* Disable on board with 1.0 BIOS due to Google bug 2602657 */
+	hash = hash_oem_table_id(acpi_gbl_FADT.header.oem_table_id);
+	if (hash == QUIRKY_BOARD_HASH) {
+		const char *bios_ver = dmi_get_system_info(DMI_BIOS_VERSION);
+		if (strncmp(bios_ver, "1.0", 3) == 0) {
+			pr_info("gsmi: disabled on this board's BIOS %s\n",
+				bios_ver);
+			return -ENODEV;
+		}
+	}
+
+	/* check for valid SMI command port in ACPI FADT */
+	if (acpi_gbl_FADT.smi_command == 0)
+		return -ENODEV;
+
+	/* allocate device structure */
+	gsmi_dev = kzalloc(sizeof(*gsmi_dev), GFP_KERNEL);
+	if (!gsmi_dev)
+		return -ENOMEM;
+	gsmi_dev->smi_cmd = acpi_gbl_FADT.smi_command;
+
+	/* register device */
+	gsmi_dev->pdev = platform_device_register_simple("gsmi", -1, NULL, 0);
+	if (IS_ERR(gsmi_dev->pdev)) {
+		printk(KERN_ERR "gsmi: unable to register platform device\n");
+		kfree(gsmi_dev);
+		gsmi_dev = NULL;
+		return -ENODEV;
+	}
+	if (misc_register(&gsmi_miscdev) < 0) {
+		printk(KERN_ERR "gsmi: unable to register misc device\n");
+		platform_device_unregister(gsmi_dev->pdev);
+		kfree(gsmi_dev);
+		gsmi_dev = NULL;
+		return -ENODEV;
+	}
+
+	/* SMI access needs to be serialized */
+	spin_lock_init(&gsmi_dev->lock);
+
+	/* SMI callbacks require 32bit addresses */
+	gsmi_dev->pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+	gsmi_dev->pdev->dev.dma_mask =
+		&gsmi_dev->pdev->dev.coherent_dma_mask;
+	gsmi_dev->dma_pool = dma_pool_create("gsmi", &gsmi_dev->pdev->dev,
+					     GSMI_BUF_SIZE, GSMI_BUF_ALIGN, 0);
+
+	/*
+	 * pre-allocate buffers because sometimes we are called when
+	 * this is not feasible: oops, panic, die, mce, etc
+	 */
+	gsmi_dev->name_buf = gsmi_buf_alloc();
+	if (!gsmi_dev->name_buf) {
+		printk(KERN_ERR "gsmi: failed to allocate name buffer\n");
+		goto out_err;
+	}
+
+	gsmi_dev->data_buf = gsmi_buf_alloc();
+	if (!gsmi_dev->data_buf) {
+		printk(KERN_ERR "gsmi: failed to allocate data buffer\n");
+		goto out_err;
+	}
+
+	gsmi_dev->param_buf = gsmi_buf_alloc();
+	if (!gsmi_dev->param_buf) {
+		printk(KERN_ERR "gsmi: failed to allocate param buffer\n");
+		goto out_err;
+	}
+
+	/*
+	 * Determine type of handshake used to serialize the SMI
+	 * entry. See also gsmi_exec().
+	 *
+	 * There's a "behavior" present on some chipsets where writing the
+	 * SMI trigger register in the southbridge doesn't result in an
+	 * immediate SMI. Rather, the processor can execute "a few" more
+	 * instructions before the SMI takes effect. To ensure synchronous
+	 * behavior, implement a handshake between the kernel driver and the
+	 * firmware handler to spin until released. This ioctl determines
+	 * the type of handshake.
+	 *
+	 * NONE: The firmware handler does not implement any
+	 * handshake. Either it doesn't need to, or it's legacy firmware
+	 * that doesn't know it needs to and never will.
+	 *
+	 * CF: The firmware handler will clear the CF in the saved
+	 * state before returning. The driver may set the CF and test for
+	 * it to clear before proceeding.
+	 *
+	 * SPIN: The firmware handler does not implement any handshake
+	 * but the driver should spin for a hundred or so microseconds
+	 * to ensure the SMI has triggered.
+	 *
+	 * Finally, the handler will return -ENOSYS if
+	 * GSMI_CMD_HANDSHAKE_TYPE is unimplemented, which implies
+	 * HANDSHAKE_NONE.
+	 */
+	spin_lock_irqsave(&gsmi_dev->lock, flags);
+	gsmi_dev->handshake_type = GSMI_HANDSHAKE_SPIN;
+	gsmi_dev->handshake_type =
+	    gsmi_exec(GSMI_CALLBACK, GSMI_CMD_HANDSHAKE_TYPE);
+	if (gsmi_dev->handshake_type == -ENOSYS)
+		gsmi_dev->handshake_type = GSMI_HANDSHAKE_NONE;
+	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
+
+	/* Remove and clean up gsmi if the handshake could not complete. */
+	if (gsmi_dev->handshake_type == -ENXIO) {
+		printk(KERN_INFO "gsmi version " DRIVER_VERSION
+		       " failed to load\n");
+		goto out_err;
+	}
+
+	printk(KERN_INFO "gsmi version " DRIVER_VERSION " loaded\n");
+
+	register_reboot_notifier(&gsmi_reboot_notifier);
+	register_die_notifier(&gsmi_die_notifier);
+	atomic_notifier_chain_register(&panic_notifier_list,
+				       &gsmi_panic_notifier);
+	register_oops_notifier(&gsmi_oops_notifier);
+
+	return 0;
+
+ out_err:
+	gsmi_buf_free(gsmi_dev->param_buf);
+	gsmi_buf_free(gsmi_dev->data_buf);
+	gsmi_buf_free(gsmi_dev->name_buf);
+	dma_pool_destroy(gsmi_dev->dma_pool);
+	platform_device_unregister(gsmi_dev->pdev);
+	misc_deregister(&gsmi_miscdev);
+	kfree(gsmi_dev);
+	gsmi_dev = NULL;
+	return -ENODEV;
+}
+
+late_initcall(gsmi_init);
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 61abb63..588e458 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -114,6 +114,8 @@
 #include <asm/fbio.h>
 #endif
 
+#include <linux/gsmi.h>
+
 static int w_long(unsigned int fd, unsigned int cmd,
 		compat_ulong_t __user *argp)
 {
@@ -1408,6 +1410,11 @@ IGNORE_IOCTL(FBIOGETCMAP32)
 IGNORE_IOCTL(FBIOSCURSOR32)
 IGNORE_IOCTL(FBIOGCURSOR32)
 #endif
+
+#ifdef CONFIG_GOOGLE_SMI
+/* Google SMI driver for /dev/gsmi */
+COMPATIBLE_IOCTL(GSMI_IOCTL)
+#endif
 };
 
 /*
diff --git a/include/linux/gsmi.h b/include/linux/gsmi.h
new file mode 100644
index 0000000..8b35b5c
--- /dev/null
+++ b/include/linux/gsmi.h
@@ -0,0 +1,120 @@
+/*
+ * Copyright 2010 Google Inc. All Rights Reserved.
+ * Author: dlaurie@google.com (Duncan Laurie)
+ *
+ * EFI SMI interface for Google platforms
+ */
+
+#ifndef _LINUX_GSMI_H
+#define _LINUX_GSMI_H
+
+#include <linux/types.h>
+
+/*
+ * Get NVRAM Variable
+ *
+ * Must know EFI GUID and exact name to retrieve a variable
+ */
+struct gsmi_get_nvram_var {
+	uint8_t		guid[16];	/* IN: unique identifier */
+	uint16_t	name_len;	/* IN: length of ascii name */
+	char		name[512];	/* IN: unique name in ascii */
+	uint16_t	data_len;	/* IN: length of data */
+	uint8_t		data[0];	/* OUT: variable data */
+} __packed;
+
+/*
+ * Get Next NVRAM Variable Name
+ *
+ * Can be used multiple times to get all variables in the system
+ * by supplying the output of one call as the input of the next.
+ */
+struct gsmi_get_next_var {
+	uint8_t		guid[16];	/* IN/OUT: unique identifier */
+	uint16_t	name_len;	/* IN/OUT: length of ascii name */
+	char		name[512];	/* IN/OUT: unique name in ascii */
+} __packed;
+
+/*
+ * Set NVRAM Variable
+ *
+ * Must know EFI GUID and exact name to set a variable
+ */
+struct gsmi_set_nvram_var {
+	uint8_t		guid[16];	/* IN: unique identifier */
+	uint16_t	name_len;	/* IN: length of ascii name */
+	char		name[512];	/* IN: unique name in ascii */
+	uint16_t	data_len;	/* IN: length of data */
+	uint8_t		data[0];	/* IN: variable data */
+} __packed;
+
+/*
+ * Add entry to event log
+ *
+ * Current defined entry type is 1
+ */
+#define GSMI_LOG_ENTRY_TYPE_KERNEL	0xdead
+struct gsmi_log_entry_type_1 {
+	uint16_t	type;
+	uint32_t	instance;
+} __packed;
+
+struct gsmi_set_event_log {
+	uint32_t	type;		/* IN: type of data in event buffer */
+	uint16_t	data_len;	/* IN: length of event buffer */
+	uint8_t		data[0];	/* IN: event data buffer */
+} __packed;
+
+/*
+ * Clear event log
+ *
+ * Valid types are:
+ *   0   : does not clear but returns percent free
+ *   25  : ensure 25% of log is clear
+ *   50  : ensure 50% of log is clear
+ *   75  : ensure 75% of log is clear
+ *   100 : ensure 100% of log is clear
+ */
+struct gsmi_clear_event_log {
+	uint32_t	type;		/* IN: clear type */
+} __packed;
+
+/*
+ * These are the various defined SMI callbacks.
+ * They all correspond to a structure defined above
+ * and below and are all used via the same ioctl.
+ * Some of the lower numbers are defined by vendors.
+ * Let's start defining Google-internal callbacks at 0xc0,
+ * a namespace partition that will surely have to be revisited someday.
+ */
+#define GSMI_CMD_GET_NVRAM_VAR		0x01
+#define GSMI_CMD_GET_NEXT_VAR		0x02
+#define GSMI_CMD_SET_NVRAM_VAR		0x03
+#define GSMI_CMD_SET_EVENT_LOG		0x08
+#define GSMI_CMD_CLEAR_EVENT_LOG	0x09
+#define GSMI_CMD_DONOTUSE_15		0x15
+#define GSMI_CMD_DONOTUSE_16		0x16
+#define GSMI_CMD_DONOTUSE_17		0x17
+#define GSMI_CMD_CLEAR_CONFIG		0x20
+#define GSMI_CMD_NOOP			0xc0
+#define GSMI_CMD_HANDSHAKE_TYPE		0xc1
+
+struct gsmi_ioctl {
+	uint16_t	length;		/* total length including data */
+	int		version;	/* structure version */
+	int		command;	/* ioctl command */
+	union {
+		struct gsmi_get_nvram_var	get_nvram_var;
+		struct gsmi_get_next_var	get_next_var;
+		struct gsmi_set_nvram_var	set_nvram_var;
+		struct gsmi_set_event_log	set_event_log;
+		struct gsmi_clear_event_log	clear_event_log;
+	} gsmi_cmd;
+} __packed;
+
+#define GSMI_BASE			'G'
+#define GSMI_IOCTL_VERSION		1
+#define GSMI_IOCTL			_IOWR(GSMI_BASE, GSMI_IOCTL_VERSION, \
+					    struct gsmi_ioctl)
+
+#endif /* _LINUX_GSMI_H */
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 18fd130..34f5dfa 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -40,6 +40,7 @@
 #define BTRFS_MINOR		234
 #define AUTOFS_MINOR		235
 #define MAPPER_CTRL_MINOR	236
+#define GOOGLE_SMI_MINOR	242
 #define MISC_DYNAMIC_MINOR	255
 
 struct device;


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 4/6] driver: Google Bootlog
  2011-01-25  0:24 [PATCH v1 0/6] google firmware support Mike Waychison
                   ` (2 preceding siblings ...)
  2011-01-25  0:24 ` [PATCH v1 3/6] driver: Google EFI SMI Mike Waychison
@ 2011-01-25  0:24 ` Mike Waychison
  2011-01-25  0:49   ` Alan Cox
  2011-01-25  0:25 ` [PATCH v1 5/6] Allow prepending to the dmesg Mike Waychison
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-25  0:24 UTC (permalink / raw)
  To: Greg KH, torvalds
  Cc: San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel, Tim Hockin

The bootlog driver supports older firmware that is used at Google for
communicating between the kernel and the BIOS reasons for reboots.

The BIOS manages a small (a couple bytes) of CMOS nvram that is
communicated to the driver.  This CMOS is written to by both the BIOS
(at bootup) as well as the kernel (on the way towards shutdown) to log
*why* things are happening.  This has enabled us to debug various types
of hardware faults, as the BIOS bootup reasons are typically hidden by
BIOSes clearing this information out before the kernel starts.

In addition to the bootlog portion, a couple bytes of the CMOS may be
dedicated to leaving any interesting data that would have been present
at warm bootup in one of the CPUs MCA banks.  This happens for example
when the CPU was so confused that the system initiated a reset.

No userland interfaces are exposed by this driver.

Signed-off-by: Duncan Laurie <dlaurie@google.com>
Signed-off-by: San Mehat <san@google.com>
Signed-off-by: Tim Hockin <thockin@google.com>
Signed-off-by: Aaron Durbin <adurbin@google.com>
Signed-off-by: Mike Waychison <mikew@google.com>
---
 drivers/firmware/Kconfig          |    8 
 drivers/firmware/google/Kconfig   |    7 
 drivers/firmware/google/Makefile  |    1 
 drivers/firmware/google/bootlog.c |  884 +++++++++++++++++++++++++++++++++++++
 4 files changed, 900 insertions(+), 0 deletions(-)
 create mode 100644 drivers/firmware/google/bootlog.c

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index d848b26..e6cc39d 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -136,4 +136,12 @@ config ISCSI_IBFT
 
 source "drivers/firmware/google/Kconfig"
 
+config BOOTLOG_SUPPORT
+	bool "Bootlog Support"
+	depends on FIRMWARE_EVENTS && X86
+	default n
+	---help---
+	  This enables support for displaying boot log information in dmesg
+	  as well as logging the kernel shutdown reasons.
+
 endmenu
diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 2d2be7c..0490a62 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -21,4 +21,11 @@ config GOOGLE_SMI
 	  clearing the EFI event log and reading and writing NVRAM
 	  variables.
 
+config GOOGLE_BOOTLOG
+	bool "Bootlog Support"
+	default y
+	---help---
+	  This enables support for displaying boot log information in dmesg
+	  as well as logging the kernel shutdown reasons.
+
 endmenu
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
index fb127d7..d45e10c 100644
--- a/drivers/firmware/google/Makefile
+++ b/drivers/firmware/google/Makefile
@@ -1,2 +1,3 @@
 
 obj-$(CONFIG_GOOGLE_SMI)		+= gsmi.o
+obj-$(CONFIG_GOOGLE_BOOTLOG)		+= bootlog.o
diff --git a/drivers/firmware/google/bootlog.c b/drivers/firmware/google/bootlog.c
new file mode 100644
index 0000000..232cd7d
--- /dev/null
+++ b/drivers/firmware/google/bootlog.c
@@ -0,0 +1,884 @@
+/*
+ * bootlog.c
+ *
+ * Boot Logging infrastructure for tracking and
+ * system resets initiated by a variety of methods.
+ *
+ * Copyright 2005 Google Inc. All Rights Reserved.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/notifier.h>
+#include <linux/kdebug.h>
+#include <linux/reboot.h>
+#include <linux/io.h>
+
+/*
+ * Kernel shutdown flags
+ */
+#define BLOG_K_CLEAN		0	/* Clean Shutdown */
+#define BLOG_K_NMIWDT		1	/* NMI Watchdog */
+#define BLOG_K_PANIC		2	/* Panic */
+#define BLOG_K_OOPS		3	/* Oops */
+#define BLOG_K_DIE		4	/* Die */
+#define BLOG_K_MCE		5	/* MCE */
+#define BLOG_K_SOFTWDT		6	/* Software Watchdog */
+#define BLOG_K_MBE		7	/* MBE */
+#define BLOG_K_TRIPLE		8	/* Triple Fault */
+
+/*
+ * Flags and descriptions of BIOS bootlog.
+ */
+#define BLOG_B_DRAMECC		0	/* DRAM ECC error */
+#define BLOG_B_SYNCFLOOD	1	/* Sync Flood */
+#define BLOG_B_PWRFAIL		2	/* Power Fail */
+#define BLOG_B_TCO2TO		3	/* Hardware TCO Timeout 2 */
+#define BLOG_B_NBRDGCRC		4	/* NB CRC Error */
+#define BLOG_B_SBRDGSFLOOD1	5	/* SB Sync Flood */
+#define BLOG_B_SBRDGCRC1	6	/* SB CRC Error */
+#define BLOG_B_POWERON		7	/* Power On */
+#define BLOG_B_SBRDGSFLOOD2	8	/* Southbridge Sync Flood */
+#define BLOG_B_SBRDGCRC2	9	/* Southbridge CRC Error */
+#define BLOG_B_THERMTRIP	10	/* Thermtrip */
+#define BLOG_B_PCIFATAL		11	/* PCI Fatal Error */
+
+#define BLOG_SUPPORT_IOPAIR8	1
+
+/*
+ * Search for a BLOG pointer in the EBDA.
+ * The EBDA segment is stored at offset 0xe in the BDA (segment 40).
+ */
+#define BLOG_EBDA_POINTER	0x040E
+
+/*
+ * How many BLOG records to print?
+ */
+#define BLOG_DEF_COUNT		5
+
+/*
+ * Size of the on-stack string buffers.
+ * Don't shrink this without counting out the ultimate length of the
+ * generated strings.
+ */
+#define SBUF_SIZE	200
+
+/*
+ * This structure is stored in the EBDA by the BIOS and
+ * indicates to the kernel how to access the boot log.
+ *
+ * If (method == BLOG_METHOD_IOPAIR8):
+ *   The iopair8 method_data defines the index/data ports
+ *   to be used to access the BLOG data and base indicates
+ *   the starting index to read from.
+ */
+
+struct bootlog_ptr {
+	uint32_t signature;		/* "BLOG" */
+	uint8_t version;		/* header version */
+	uint8_t method;			/* access method */
+	union {
+		struct {
+			uint8_t index_port;
+			uint8_t data_port;
+			uint8_t base;
+			uint8_t ecc_base;
+			uint8_t ecc_length;
+		} iopair8;		/* 8 bit IO index/data pair*/
+		struct {
+			uint16_t port;
+			uint16_t base;
+			uint16_t ecc_base;
+			uint16_t ecc_length;
+		} io8;			/* 8 bit IO */
+	} method_data;
+} __packed;
+
+/* we find and save the bootlog_ptr during initialization */
+static struct bootlog_ptr *blog_ptr;
+
+/* this is the signature we find in the EBDA to indicate the BLOG pointer */
+#define BLOG_PTR_MAGIC		(('B')|('L'<<8)|('O'<<16)|('G'<<24))
+
+/* these define the layout of the BLOG pointer */
+#define BLOG_PTR_VERSION_1	1
+#define BLOG_PTR_VERSION_2	2
+
+/* these define the possible BLOG methods (bootlog_ptr.method) */
+#define BLOG_METHOD_IOPAIR8	1
+#define BLOG_METHOD_IO8		2
+
+/*
+ * Each BLOG record consists of two parts:
+ *   Part 1: BIOS-generated info about the boot
+ *   Part 2: kernel-generated info about the shutdown
+ *
+ * This structure is independent of any particular BLOG method.
+ */
+struct bootlog_record {
+	uint32_t bootup;
+	uint32_t shutdown;
+};
+
+/*
+ * Each BLOG method must provide a bootlog_ops structure.
+ */
+struct bootlog_ops {
+	/* method-specific init */
+	int (*init)(void);
+	/* get the method-specific version */
+	int (*version)(void);
+	/* get the boot count */
+	int (*bootcount)(void);
+	/* get the number of records */
+	int (*nrecords)(void);
+	/* get a BLOG record */
+	struct bootlog_record *(*get_record)(int recnum);
+	/* get ECCLog data */
+	int (*get_ecc)(int *boot, uint64_t *status, uint64_t *address);
+	/* add a new shutdown reason */
+	uint32_t (*add_reason)(int reason);
+};
+
+/* this is the global ops table */
+struct bootlog_ops *blog_ops;
+
+/* serialize access to bootlog */
+static DEFINE_SPINLOCK(bootlog_lock);
+
+/* number of BLOG records to display on boot */
+static uint8_t bootlog_count = BLOG_DEF_COUNT;
+
+/*
+ * Used to describe BLOG flags below.
+ */
+struct bootlog_flag {
+	uint16_t flag;
+	const char *desc;
+};
+
+/*
+ * Descriptions for Kernel bootlog flags.
+ */
+static struct bootlog_flag bootlog_kernel_flags[] = {
+	{ BLOG_K_CLEAN,		"Clean" },
+	{ BLOG_K_NMIWDT,	"NMI Watchdog" },
+	{ BLOG_K_PANIC,		"Panic" },
+	{ BLOG_K_OOPS,		"Oops" },
+	{ BLOG_K_DIE,		"Die" },
+	{ BLOG_K_MCE,		"MCE Panic" },
+	{ BLOG_K_SOFTWDT,	"Software Watchdog" },
+	{ BLOG_K_MBE,		"Multi-bit ECC" },
+	{ BLOG_K_TRIPLE,	"Triple Fault" },
+	{ 0,			NULL }
+};
+
+static struct bootlog_flag bootlog_bios_flags[] = {
+	{ BLOG_B_PWRFAIL,	"Power Failure" },
+	{ BLOG_B_POWERON,	"Power On" },
+	{ BLOG_B_SYNCFLOOD,	"Sync Flood" },
+	{ BLOG_B_DRAMECC,	"DRAM ECC" },
+	{ BLOG_B_TCO2TO,	"TCO Watchdog Timeout" },
+	{ BLOG_B_SBRDGSFLOOD1,	"Southbridge Sync Flood" },
+	{ BLOG_B_SBRDGCRC1,	"Southbridge CRC Error" },
+	{ BLOG_B_NBRDGCRC,	"Northbridge CRC Error" },
+	{ BLOG_B_SBRDGSFLOOD2,	"Southbridge Sync Flood" },
+	{ BLOG_B_SBRDGCRC2,	"Southbridge CRC Error" },
+	{ BLOG_B_THERMTRIP,	"Thermtrip" },
+	{ BLOG_B_PCIFATAL,	"PCI Fatal Error" },
+	{ 0,			NULL }
+};
+
+int __init setup_bootlog_count(char * str)
+{
+	int count;
+
+	get_option(&str, &count);
+
+	if (count < 1 && count > 256)
+		return 0;
+
+	bootlog_count = count;
+	return 1;
+}
+__setup("bootlog_count=", setup_bootlog_count);
+
+/*
+ * Search for the BLOG pointer.
+ */
+static struct bootlog_ptr * __init bootlog_find_pointer(void)
+{
+	unsigned long address, length, cur;
+	struct bootlog_ptr *bp;
+
+	/* EBDA pointer contains segment the extended BIOS data area */
+	address = *(uint16_t *)phys_to_virt(BLOG_EBDA_POINTER);
+	address <<= 4;	/* convert segment to physical address */
+
+	/* EBDA length is byte 0 of the EBDA (stored in kB) */
+	length = *(uint8_t *)phys_to_virt(address);
+	length <<= 10;	/* convert to bytes */
+
+	/*
+	 * Search through EBDA for the BLOG signature.
+	 * NOTE: the signature is not necessarily DWORD-aligned.
+	 */
+	for (cur = 0; cur < length; cur++) {
+		bp = phys_to_virt(address + cur);
+		if (bp->signature == BLOG_PTR_MAGIC) {
+			/*
+			 * BLOG_PTR_VERSION_1 and VERSION_2 are mostly the
+			 * same. In the future, this will probably have to
+			 * get more complicated.
+			 */
+			return bp;
+		}
+	}
+
+	return NULL;
+}
+
+/*
+ * Print BLOG records to console, where the most recent reboot is
+ * printed out as 1, previous 2 ... indexed up to bootlog_count.
+ */
+static void bootlog_print_records(void)
+{
+	unsigned long flags;
+	char sbuf[SBUF_SIZE];
+	int i;
+
+	if (blog_ptr == NULL)
+		return;
+	if (blog_ops == NULL)
+		return;
+	if (bootlog_count == 0)
+		return;
+
+	/* Reset the buffer */
+	sbuf[0] = '\0';
+
+	spin_lock_irqsave(&bootlog_lock, flags);
+	for (i = 0; i < bootlog_count; i++) {
+		struct bootlog_record *record;
+		struct bootlog_flag *bf;
+
+		/* get a pointer to the BLOG record */
+		record = blog_ops->get_record(i);
+		if (!record)
+			break;
+
+		/* list kernel shutdown reasons from newest to oldest */
+		sprintf(sbuf,
+			"BLOG: Record %d Kernel Shutdown Reason: ", i + 1);
+		if (record->shutdown == 0)
+			strcat(sbuf, "Unknown");
+		else {
+			int count = 0;
+			for (bf = bootlog_kernel_flags; bf->desc; bf++) {
+				if (record->shutdown & (1<<bf->flag)) {
+					if (count++)
+						strcat(sbuf, ", ");
+					strcat(sbuf, bf->desc);
+				}
+			}
+		}
+
+		/* Print and reset the buffer */
+		pr_info("%s\n", sbuf);
+		sbuf[0] = '\0';
+
+		/* list BIOS bootup reasons */
+		sprintf(sbuf,
+			"BLOG: Record %d BIOS Bootup Reason: ", i + 1);
+		if (record->bootup == 0)
+			strcat(sbuf, "Clean");
+		else {
+			int count = 0;
+			for (bf = bootlog_bios_flags; bf->desc; bf++) {
+				if (record->bootup & (1<<bf->flag)) {
+					if (count++)
+						strcat(sbuf, ", ");
+					strcat(sbuf, bf->desc);
+				}
+			}
+		}
+
+		/* Print and reset for next iteration */
+		pr_info("%s\n", sbuf);
+		sbuf[0] = '\0';
+	}
+	spin_unlock_irqrestore(&bootlog_lock, flags);
+}
+
+/*
+ * Print saved Machine Check data (ECC).
+ * Currently only supported if blog_ptr->version == BLOG_PTR_VERSION_2.
+ */
+static void bootlog_print_ecclog(void)
+{
+	int boot;
+	unsigned long long status;
+	unsigned long long address;
+	unsigned long flags;
+	char sbuf[SBUF_SIZE];
+
+	if (blog_ptr == NULL)
+		return;
+	if (blog_ops == NULL)
+		return;
+	if (blog_ptr->version != BLOG_PTR_VERSION_2)
+		return;
+
+	spin_lock_irqsave(&bootlog_lock, flags);
+	if (blog_ops->get_ecc(&boot, &status, &address) == 0) {
+		if (blog_ops->version() == 1) {
+			pr_info("BLOG: Machine Check Log: "
+				"%08llx %08llx %08llx %08llx\n",
+				status >> 32, status & 0xffffffff,
+				address >> 32, address & 0xffffffff);
+		} else if (status) {
+			strcpy(sbuf, "BLOG: ECCLog: Boot ");
+			if (boot)
+				sprintf(sbuf, "%d", boot);
+			else
+				strcat(sbuf, "unknown");
+			pr_info("%s Status 0x%016llx Address 0x%016llx\n",
+				sbuf, status, address);
+		}
+	}
+	spin_unlock_irqrestore(&bootlog_lock, flags);
+}
+
+
+/*
+ * Write the indicated shutdown reason to the BLOG shutdown
+ * byte (2) and update the BLOG checksum byte (N+1)
+ */
+static int bootlog_shutdown_reason(int reason)
+{
+	unsigned long flags;
+	struct bootlog_flag *bf;
+	int count = 0;
+	uint32_t reasons;
+	char sbuf[SBUF_SIZE];
+
+	if (blog_ptr == NULL || blog_ops == NULL)
+		return -1;
+
+	spin_lock_irqsave(&bootlog_lock, flags);
+	reasons = blog_ops->add_reason(reason);
+	spin_unlock_irqrestore(&bootlog_lock, flags);
+
+	sprintf(sbuf, "BLOG: Updated Shutdown reason to 0x%02x (", reasons);
+
+	for (bf = bootlog_kernel_flags; bf->desc; bf++) {
+		if (reasons & (1<<bf->flag)) {
+			if (count++)
+				strcat(sbuf, ", ");
+			strcat(sbuf, bf->desc);
+		}
+	}
+
+	pr_info("%s)\n", sbuf);
+
+	return 0;
+}
+
+#if BLOG_SUPPORT_IOPAIR8
+
+/*
+ * The io8 header.  It's easiest to read and cache this, rather than
+ * hit the IO ports every time we need something.
+ */
+struct bootlog_iopair8_hdr {
+	uint8_t checksum;		/* checksum of header and data */
+	uint8_t version;		/* blog struct version */
+	uint8_t nrecords;		/* number of BLOG records */
+	uint16_t bootcount;		/* number of boots */
+	uint16_t reason;		/* kernel shutdown reason */
+	struct bootlog_record *blog;	/* BLOG records */
+	int ecc_boot;			/* boot number for ECCLog */
+	uint64_t ecc_status;		/* last ECC status */
+	uint64_t ecc_address;		/* last ECC address */
+};
+
+/* we find and save this during initialization */
+static struct bootlog_iopair8_hdr *io8_hdr;
+
+/*
+ * The format of the data pointed to by the iopair8 header is:
+ *   Byte 0     (RW) checksum of header and data
+ *   Byte 1     (R)  log structure version
+ *
+ * If the header version is IO8_VERSION_1:
+ *   Byte 2     (R)  number of BLOG records
+ *   Byte 3     (R)  number of system boots (written by BIOS)
+ *   Byte 4     (W)  shutdown reason (written by kernel)
+ *   Byte 5-N   (R)  BLOG records consisting of kernel and BIOS flags
+ *
+ * If the header version is IO8_VERSION_2:
+ *   Byte 2     (R)  number of BLOG records
+ *   Byte 3,4   (R)  number of system boots [low,high] (written by BIOS)
+ *   Byte 5,6   (W)  shutdown reason [low,high] (written by kernel)
+ *   Byte 7-N   (R)  BLOG records consisting of kernel/bios cause masks
+ *
+ * The ECCLog structure is also dependant on the IO8 header version.
+ * These are offsets from the ecc_base:
+ *   Byte 0-7   (R)  ECC status [low..high] (written by BIOS)
+ *   Byte 8-15  (R)  ECC address [low..high] (written by BIOS)
+ *
+ * If the header version is IO8_VERSION_2:
+ *   Byte 16-17 (R)  ECC boot number [low,high] (written by BIOS)
+ */
+
+#define IO8_OFF_CHECKSUM	0
+#define IO8_OFF_VERSION		1
+
+#define IO8V1_OFF_NRECS		2
+#define IO8V1_OFF_BOOTCNT	3
+#define IO8V1_OFF_REASON	4
+#define IO8V1_OFF_RECORDS	5
+#define IO8V1_OFF_REC(n)	(IO8V1_OFF_RECORDS + (n)*IO8V1_RECSIZE)
+#define IO8V1_RECSIZE		2
+
+#define IO8V2_OFF_NRECS		2
+#define IO8V2_OFF_BOOTCNT	3
+#define IO8V2_OFF_REASON	5
+#define IO8V2_OFF_RECORDS	7
+#define IO8V2_OFF_REC(n)	(IO8V2_OFF_RECORDS + (n)*IO8V2_RECSIZE)
+#define IO8V2_RECSIZE		4
+
+#define IO8_ECC_OFF_STATUS	0
+#define IO8_ECC_OFF_ADDRESS	8
+#define IO8_ECC_OFF_BOOTNUM	16
+
+/* the iopair8 BLOG header is versioned */
+#define IO8_VERSION_1		1
+#define IO8_VERSION_2		2
+
+static uint8_t io8_pair_read(int offset)
+{
+	int base = blog_ptr->method_data.iopair8.base;
+	int iport = blog_ptr->method_data.iopair8.index_port;
+	int dport = blog_ptr->method_data.iopair8.data_port;
+
+	outb(base + offset, iport);
+	return inb(dport);
+}
+
+static uint8_t io8_direct_read(int offset)
+{
+	int base = blog_ptr->method_data.io8.base;
+	int port = blog_ptr->method_data.io8.port;
+
+	return inb(base + port + offset);
+}
+
+static uint8_t io8_read(int offset)
+{
+	if (blog_ptr->method == BLOG_METHOD_IO8)
+		return io8_direct_read(offset);
+
+	return io8_pair_read(offset);
+}
+
+static uint16_t io8_read16(int offset)
+{
+	uint16_t val;
+
+	/* multi-byte values are always stored little endian */
+	val = io8_read(offset+1);
+	val <<= 8;
+	val |= io8_read(offset);
+	return val;
+}
+
+static uint8_t io8_pair_ecc_read(int offset)
+{
+	int base = blog_ptr->method_data.iopair8.ecc_base;
+	int iport = blog_ptr->method_data.iopair8.index_port;
+	int dport = blog_ptr->method_data.iopair8.data_port;
+
+	outb(base + offset, iport);
+	return inb(dport);
+}
+
+static uint8_t io8_direct_ecc_read(int offset)
+{
+	int base = blog_ptr->method_data.io8.ecc_base;
+	int port = blog_ptr->method_data.io8.port;
+
+	return inb(base + port + offset);
+}
+
+static uint8_t io8_ecc_read(int offset)
+{
+	if (blog_ptr->method == BLOG_METHOD_IO8)
+		return io8_direct_ecc_read(offset);
+
+	return io8_pair_ecc_read(offset);
+}
+
+static uint16_t io8_ecc_read16(int offset)
+{
+	uint16_t val;
+
+	/* multi-byte values are always stored little endian */
+	val = io8_ecc_read(offset+1);
+	val <<= 8;
+	val |= io8_ecc_read(offset);
+	return val;
+}
+
+static uint64_t io8_ecc_read64(int offset)
+{
+	uint64_t val;
+	int i;
+
+	/*
+	 * Multi-byte values are always stored little endian.  In this
+	 * case it's stored as 2 32-bit values, high half first.  Ask San
+	 * why, for I know not.
+	 */
+	val = 0;
+	for (i = 3; i >= 0; i--) {
+		val <<= 8;
+		val |= io8_ecc_read(offset+i);
+	}
+	for (i = 3; i >= 0; i--) {
+		val <<= 8;
+		val |= io8_ecc_read(offset+4+i);
+	}
+	return val;
+}
+
+static void io8_pair_write(int offset, uint8_t value)
+{
+	int base = blog_ptr->method_data.iopair8.base;
+	int iport = blog_ptr->method_data.iopair8.index_port;
+	int dport = blog_ptr->method_data.iopair8.data_port;
+
+	outb(base + offset, iport);
+	outb(value, dport);
+}
+
+static void io8_direct_write(int offset, uint8_t value)
+{
+	int base = blog_ptr->method_data.io8.base;
+	int port = blog_ptr->method_data.io8.port;
+
+	outb(value, port + base + offset);
+}
+
+static void io8_write(int offset, uint8_t value)
+{
+	if (blog_ptr->method == BLOG_METHOD_IO8) {
+		io8_direct_write(offset, value);
+		return;
+	}
+
+	io8_pair_write(offset, value);
+}
+
+static void io8_write16(int offset, uint16_t value)
+{
+	uint8_t v;
+
+	/* multi-byte values are always stored little endian */
+	v = value & 0xff;
+	io8_write(offset, v);
+	v = (value >> 8) & 0xff;
+	io8_write(offset+1, v);
+}
+
+static uint8_t io8_checksum(void)
+{
+	int i;
+	int version;
+	int bytes = 0;
+	uint8_t sum = 0;
+
+	/* figure out how many bytes to checksum */
+	version = io8_read(IO8_OFF_VERSION);
+	if (version == IO8_VERSION_1) {
+		int records = io8_read(IO8V1_OFF_NRECS);
+		bytes = IO8V1_OFF_RECORDS + records * IO8V1_RECSIZE;
+	} else if (version == IO8_VERSION_2) {
+		int records = io8_read(IO8V2_OFF_NRECS);
+		bytes = IO8V2_OFF_RECORDS + records * IO8V2_RECSIZE;
+	}
+
+	/* run the checksum, skipping byte 0, which is the csum itself */
+	for (i = 1; i < bytes; i++)
+		sum += io8_read(i);
+
+	return sum;
+}
+
+static int io8_init(void)
+{
+	int version;
+	int checksum;
+
+	if (blog_ptr == NULL)
+		return -1;
+
+	/* make sure we know this format */
+	version = io8_read(IO8_OFF_VERSION);
+	if (version == IO8_VERSION_1 || version == IO8_VERSION_2) {
+		/* validate the checksum */
+		int calcsum;
+		checksum = io8_read(IO8_OFF_CHECKSUM);
+		calcsum = io8_checksum();
+		if (calcsum != checksum) {
+			pr_alert("BLOG: Bad checksum (%02x != %02x)\n",
+				 checksum, calcsum);
+			return -1;
+		}
+
+		io8_hdr = kmalloc(sizeof(*io8_hdr), GFP_KERNEL);
+		if (io8_hdr == NULL)
+			return -1;
+		memset(io8_hdr, 0, sizeof(*io8_hdr));
+	} else {
+		pr_alert("BLOG: Unsupported iopair8 version %d\n",
+			 version);
+		return -1;
+	}
+
+	/* populate the header struct */
+	io8_hdr->version  = version;
+	io8_hdr->checksum = checksum;
+
+	if (version == IO8_VERSION_1) {
+		io8_hdr->nrecords = io8_read(IO8V1_OFF_NRECS);
+		io8_hdr->bootcount = io8_read(IO8V1_OFF_BOOTCNT);
+		io8_hdr->reason = io8_read(IO8V1_OFF_REASON);
+	} else if (version == IO8_VERSION_2) {
+		io8_hdr->nrecords = io8_read(IO8V2_OFF_NRECS);
+		io8_hdr->bootcount = io8_read16(IO8V2_OFF_BOOTCNT);
+		io8_hdr->reason = io8_read16(IO8V2_OFF_REASON);
+	}
+
+	/* initialize the list of BLOG records */
+	if (io8_hdr->nrecords > 0) {
+		int r;
+
+		io8_hdr->blog = kmalloc(sizeof(*io8_hdr->blog) *
+					io8_hdr->nrecords, GFP_KERNEL);
+		if (io8_hdr->blog == NULL) {
+			kfree(io8_hdr);
+			return -1;
+		}
+		memset(io8_hdr->blog, 0,
+			sizeof(*io8_hdr->blog) * io8_hdr->nrecords);
+
+		for (r = 0; r < io8_hdr->nrecords; r++) {
+			uint32_t b = 0, k = 0;
+			if (version == IO8_VERSION_1) {
+				/* each record is 2 1-byte fields */
+				b = io8_read(IO8V1_OFF_REC(r));
+				k = io8_read(IO8V1_OFF_REC(r)+1);
+			} else if (version == IO8_VERSION_2) {
+				/* each record is 2 2-byte fields */
+				b = io8_read16(IO8V2_OFF_REC(r));
+				k = io8_read16(IO8V2_OFF_REC(r)+2);
+			}
+			io8_hdr->blog[r].bootup = b;
+			io8_hdr->blog[r].shutdown = k;
+		}
+	} else {
+		io8_hdr->blog = NULL;
+	}
+
+	/* read the ECCLog data */
+	if (blog_ptr->version == BLOG_PTR_VERSION_2) {
+		if (io8_hdr->version == IO8_VERSION_1)
+			io8_hdr->ecc_boot = 0;
+		else if (io8_hdr->version == IO8_VERSION_2)
+			io8_hdr->ecc_boot = io8_ecc_read16(IO8_ECC_OFF_BOOTNUM);
+		io8_hdr->ecc_status = io8_ecc_read64(IO8_ECC_OFF_STATUS);
+		io8_hdr->ecc_address = io8_ecc_read64(IO8_ECC_OFF_ADDRESS);
+	}
+
+	return 0;
+}
+
+static int io8_version(void)
+{
+	return io8_hdr->version;
+}
+
+static int io8_bootcount(void)
+{
+	return io8_hdr->bootcount;
+}
+
+static int io8_nrecords(void)
+{
+	return io8_hdr->nrecords;
+}
+
+static struct bootlog_record *io8_get_record(int recnum)
+{
+	if (recnum < io8_hdr->nrecords)
+		return &io8_hdr->blog[recnum];
+	return NULL;
+}
+
+static int io8_get_ecc(int *boot, uint64_t *status, uint64_t *address)
+{
+	if (blog_ptr->version >= BLOG_PTR_VERSION_2) {
+		*boot = io8_hdr->ecc_boot;
+		*status = io8_hdr->ecc_status;
+		*address = io8_hdr->ecc_address;
+		return 0;
+	}
+	return -1;
+}
+
+static uint32_t io8_add_reason(int reason)
+{
+	/* update shutdown byte */
+	io8_hdr->reason |= (1<<reason);
+
+	/* write it out */
+	if (io8_hdr->version == IO8_VERSION_1)
+		io8_write(IO8V1_OFF_REASON, io8_hdr->reason);
+	else if (io8_hdr->version == IO8_VERSION_2)
+		io8_write16(IO8V2_OFF_REASON, io8_hdr->reason);
+
+	/* update the checksum */
+	io8_hdr->checksum = io8_checksum();
+	io8_write(IO8_OFF_CHECKSUM, io8_hdr->checksum);
+
+	return io8_hdr->reason;
+}
+
+/*
+ * The io8 operations table.
+ */
+struct bootlog_ops io8_ops = {
+	.init		= io8_init,
+	.version	= io8_version,
+	.bootcount	= io8_bootcount,
+	.nrecords	= io8_nrecords,
+	.get_record	= io8_get_record,
+	.get_ecc	= io8_get_ecc,
+	.add_reason	= io8_add_reason,
+};
+
+#endif /* BLOG_SUPPORT_IOPAIR8 */
+
+static int bootlog_reboot_callback(struct notifier_block *nb,
+				   unsigned long reason, void *arg)
+{
+	bootlog_shutdown_reason(BLOG_K_CLEAN);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block bootlog_reboot_notifier = {
+	.notifier_call = bootlog_reboot_callback
+};
+
+static int bootlog_die_callback(struct notifier_block *nb,
+				unsigned long reason, void *arg)
+{
+	if (reason == DIE_NMIWATCHDOG)
+		bootlog_shutdown_reason(BLOG_K_NMIWDT);
+	else if (reason == DIE_OOPS)
+		bootlog_shutdown_reason(BLOG_K_DIE);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block bootlog_die_notifier = {
+	.notifier_call = bootlog_die_callback
+};
+
+static int bootlog_panic_callback(struct notifier_block *nb,
+				  unsigned long reason, void *arg)
+{
+	bootlog_shutdown_reason(BLOG_K_PANIC);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block bootlog_panic_notifier = {
+	.notifier_call = bootlog_panic_callback
+};
+
+static int bootlog_oops_callback(struct notifier_block *nb,
+				 unsigned long reason, void *arg)
+{
+	if (reason == OOPS_ENTER)
+		bootlog_shutdown_reason(BLOG_K_OOPS);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block bootlog_oops_notifier = {
+	.notifier_call = bootlog_oops_callback
+};
+
+/*
+ * Find the BLOG pointer location and parse BLOG records,
+ * display previous shutdown reason as indicated by kernel and BIOS
+ */
+static int __init bootlog_startup(void)
+{
+	/* search for bootlog pointer in EBDA */
+	blog_ptr = bootlog_find_pointer();
+	if (blog_ptr == NULL) {
+		pr_alert("BLOG: Unable to find pointer in EBDA\n");
+		return -1;
+	}
+
+	pr_info("BIOS BootLog pointer version %d method %d at 0x%08lx\n",
+		blog_ptr->version, blog_ptr->method,
+		(unsigned long)virt_to_phys(blog_ptr));
+
+	/* make sure we know how to handle this pointer version */
+	if (blog_ptr->version > BLOG_PTR_VERSION_2) {
+		pr_alert("BLOG: Unsupported pointer version %d\n",
+			 blog_ptr->version);
+		blog_ptr = NULL;
+		return -1;
+	}
+
+	/* figure out which access method to use */
+	switch (blog_ptr->method) {
+	case BLOG_METHOD_IOPAIR8:
+	case BLOG_METHOD_IO8:
+		blog_ops = &io8_ops;
+		break;
+	default:
+		pr_alert("BLOG: Unsupported method %d\n",
+			 blog_ptr->method);
+		blog_ptr = NULL;
+		return -1;
+	}
+
+	/* do method-specific initialization (including checksum) */
+	if (blog_ops->init() < 0) {
+		blog_ptr = NULL;
+		return -1;
+	}
+
+	register_reboot_notifier(&bootlog_reboot_notifier);
+	register_die_notifier(&bootlog_die_notifier);
+	atomic_notifier_chain_register(&panic_notifier_list,
+				       &bootlog_panic_notifier);
+	register_oops_notifier(&bootlog_oops_notifier);
+
+	/* print the BLOG info */
+	pr_info("BLOG: BootLog Rev %d, Boot #%d, %d Record(s)\n",
+		blog_ops->version(), blog_ops->bootcount(),
+		blog_ops->nrecords());
+	bootlog_print_records();
+	bootlog_print_ecclog();
+
+	return 0;
+}
+early_initcall(bootlog_startup);


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 5/6] Allow prepending to the dmesg
  2011-01-25  0:24 [PATCH v1 0/6] google firmware support Mike Waychison
                   ` (3 preceding siblings ...)
  2011-01-25  0:24 ` [PATCH v1 4/6] driver: Google Bootlog Mike Waychison
@ 2011-01-25  0:25 ` Mike Waychison
  2011-01-25  1:01   ` Andrew Morton
  2011-01-25  0:25 ` [PATCH v1 6/6] driver: Google Memory Console Mike Waychison
  2011-01-25  3:01 ` [PATCH v1 0/6] google firmware support Greg KH
  6 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-25  0:25 UTC (permalink / raw)
  To: Greg KH, torvalds
  Cc: San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel, Tim Hockin

The next patch in this series (Memory Console driver) would like to
prepend firmware messages to the kernel logs.

Instead of exposing all the nitty gritty lock details of the kernel's
printk system to the entire kernel, expose a "prepend_to_dmesg()" that
attempts to rewrite the in-memory dmesg to inject pre-kernel messages.

This function only prepends if the start of the kernel's messages are
still in the ring and there is still room for messages (without losing
the current tail of the message log).  We determine this by checking out
whether the bufer has yet been cleared (indicated by a new flag:
buffer_has_cleared), and by checking whether the buffer is less than
full (which would indicate that it had wrapped).

If there is enough room to prepend data, we simply shift the existing
log contents down and copy into the buffer the tail of the prepended
data that fits.

Signed-off-by: Mike Waychison <mikew@google.com>
---
 include/linux/printk.h |    5 ++++
 kernel/printk.c        |   55 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 1 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index ee048e7..df89965 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -113,6 +113,7 @@ extern int dmesg_restrict;
 extern int kptr_restrict;
 
 void log_buf_kexec_setup(void);
+void prepend_to_dmesg(const char *buffer, size_t length);
 #else
 static inline __attribute__ ((format (printf, 1, 0)))
 int vprintk(const char *s, va_list args)
@@ -137,6 +138,10 @@ static inline bool printk_timed_ratelimit(unsigned long *caller_jiffies,
 static inline void log_buf_kexec_setup(void)
 {
 }
+
+void prepend_to_dmesg(const char *buffer, size_t length)
+{
+}
 #endif
 
 extern void dump_stack(void) __cold;
diff --git a/kernel/printk.c b/kernel/printk.c
index 53d9a9e..aa917e3 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -144,6 +144,9 @@ static int log_buf_len = __LOG_BUF_LEN;
 static unsigned logged_chars; /* Number of chars produced since last read+clear operation */
 static int saved_console_loglevel = -1;
 
+/* Used to determine if the buffer starts at the kernel's first messages. */
+static bool buffer_has_cleared;
+
 #ifdef CONFIG_KEXEC
 /*
  * This appends the listed symbols to /proc/vmcoreinfo
@@ -344,8 +347,10 @@ int do_syslog(int type, char __user *buf, int len, bool from_file)
 		spin_lock_irq(&logbuf_lock);
 		if (count > logged_chars)
 			count = logged_chars;
-		if (do_clear)
+		if (do_clear) {
 			logged_chars = 0;
+			buffer_has_cleared = true;
+		}
 		limit = log_end;
 		/*
 		 * __put_user() could sleep, and while we sleep
@@ -383,6 +388,7 @@ int do_syslog(int type, char __user *buf, int len, bool from_file)
 	/* Clear ring buffer */
 	case SYSLOG_ACTION_CLEAR:
 		logged_chars = 0;
+		buffer_has_cleared = 0;
 		break;
 	/* Disable logging to console */
 	case SYSLOG_ACTION_CONSOLE_OFF:
@@ -844,6 +850,53 @@ out_restore_irqs:
 EXPORT_SYMBOL(printk);
 EXPORT_SYMBOL(vprintk);
 
+void __init prepend_to_dmesg(const char *buffer, size_t length)
+{
+	size_t amount_to_copy;
+	size_t copy_start;
+	unsigned long flags;
+
+	spin_lock_irqsave(&logbuf_lock, flags);
+
+	/*
+	 * Verify that the dmesg hasn't been cleared yet.  If it has,
+	 * the prepend operation is a no-op as we no longer have the
+	 * "beginning" of the kernel boot.
+	 */
+	if (buffer_has_cleared)
+		goto out;
+
+	/* Nothing to do if there is no amount in the buffer. */
+	amount_to_copy = log_buf_len - logged_chars;
+	if (!amount_to_copy)
+		goto out;
+
+	/*
+	 * At this point, we know we are on the first fill of the ring
+	 * buffer, and as such we haven't wrapped yet.
+	 */
+
+	/*
+	 * If available space in the buffer exceeds the length of the
+	 * data being prepended, trim the amount to copy.
+	 */
+	if (amount_to_copy > length)
+		amount_to_copy = length;
+
+	copy_start = length - amount_to_copy;
+
+	/* Make room by shifting the existing contents of the buffer. */
+	memmove(&log_buf[amount_to_copy], log_buf, logged_chars);
+	log_end += amount_to_copy;
+	logged_chars += amount_to_copy;
+
+	/* Inject prepended logs */
+	memmove(log_buf, buffer + copy_start, amount_to_copy);
+
+out:
+	spin_unlock_irqrestore(&logbuf_lock, flags);
+}
+
 #else
 
 static void call_console_drivers(unsigned start, unsigned end)


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 6/6] driver: Google Memory Console
  2011-01-25  0:24 [PATCH v1 0/6] google firmware support Mike Waychison
                   ` (4 preceding siblings ...)
  2011-01-25  0:25 ` [PATCH v1 5/6] Allow prepending to the dmesg Mike Waychison
@ 2011-01-25  0:25 ` Mike Waychison
  2011-01-25  2:00   ` Greg KH
  2011-01-25  3:01 ` [PATCH v1 0/6] google firmware support Greg KH
  6 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-25  0:25 UTC (permalink / raw)
  To: Greg KH, torvalds
  Cc: San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel, Tim Hockin

This patch introduces the 'memconsole' driver.

Our firmware gives us access to an in-memory log of the firmware's
output.   This gives us visibility in a data-center of headless machines
as to what the firmware is doing.

The memory console is found by the driver by finding a header block in
the EBDA.  The buffer is then copied out, and is currently prepended
to the kernel's dmesg ring-buffer.

Signed-off-by: San Mehat <san@google.com>
Signed-off-by: Mike Waychison <mikew@google.com>
---
 drivers/firmware/google/Kconfig      |    8 ++
 drivers/firmware/google/Makefile     |    1 
 drivers/firmware/google/memconsole.c |  136 ++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 0 deletions(-)
 create mode 100644 drivers/firmware/google/memconsole.c

diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 0490a62..7218671 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -28,4 +28,12 @@ config GOOGLE_BOOTLOG
 	  This enables support for displaying boot log information in dmesg
 	  as well as logging the kernel shutdown reasons.
 
+config GOOGLE_MEMCONSOLE
+	bool "Firmware Memory Console"
+	default y
+	help
+	  This option enables the kernel to search for a firmware log in
+	  the EBDA.  If found, this log is prepended to the kernel's
+	  dmesg logs so they are visible to the user.
+
 endmenu
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
index d45e10c..cb8598e 100644
--- a/drivers/firmware/google/Makefile
+++ b/drivers/firmware/google/Makefile
@@ -1,3 +1,4 @@
 
 obj-$(CONFIG_GOOGLE_SMI)		+= gsmi.o
 obj-$(CONFIG_GOOGLE_BOOTLOG)		+= bootlog.o
+obj-$(CONFIG_GOOGLE_MEMCONSOLE)		+= memconsole.o
diff --git a/drivers/firmware/google/memconsole.c b/drivers/firmware/google/memconsole.c
new file mode 100644
index 0000000..6777a64
--- /dev/null
+++ b/drivers/firmware/google/memconsole.c
@@ -0,0 +1,136 @@
+/*
+ * memconsole.c
+ *
+ * Infrastructure for importing the BIOS memory based console
+ * into the kernel log ringbuffer.
+ *
+ * Copyright 2010 Google Inc. All rights reserved.
+ */
+
+#include <linux/ctype.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <asm/bios_ebda.h>
+
+#define BIOS_MEMCONSOLE_V1_MAGIC	0xDEADBABE
+#define BIOS_MEMCONSOLE_V2_MAGIC	(('M')|('C'<<8)|('O'<<16)|('N'<<24))
+
+struct biosmemcon_ebda {
+	uint32_t signature;
+	union {
+		struct {
+			uint8_t  enabled;
+			uint32_t buffer_addr;
+			uint16_t start;
+			uint16_t end;
+			uint16_t num_chars;
+			uint8_t  wrapped;
+		} __packed v1;
+		struct {
+			uint32_t buffer_addr;
+			/* Misdocumented as number of pages! */
+			uint16_t  num_bytes;
+			uint16_t start;
+			uint16_t end;
+		} __packed v2;
+	} ptr;
+} __packed;
+
+static void __init sanitize_buffer(char *buffer, size_t length) {
+	size_t cur;
+
+	/* sanitize BIOS output by converting non-ascii into space */
+	for (cur = 0; cur < length; cur++) {
+		if (!isascii(buffer[cur]) || buffer[cur] == 0)
+			buffer[cur] = ' ';
+	}
+}
+
+/*
+ * Search through the EBDA for the BIOS Memory Console, and
+ * prepend it to our ring buffer
+ */
+static int __init inject_memconsole(void)
+{
+	static struct biosmemcon_ebda __initdata hdr = {0};
+	unsigned int address;
+	size_t length, cur;
+	uint32_t *bp;
+	char *virtp;
+	int found = 0;
+
+	address = get_bios_ebda();
+	if (!address) {
+		printk(KERN_INFO "BIOS EBDA non-existent.\n");
+		return 0;
+	}
+
+	/* EBDA length is byte 0 of EBDA (in KB) */
+	length = *(uint8_t *)phys_to_virt(address);
+	length <<= 10; /* convert to bytes */
+
+	/*
+	 * Search through EBDA for BIOS memory console structure
+	 * note: signature is not necessarily dword-aligned
+	 */
+	for (cur = 0; cur < length; cur++) {
+		bp = phys_to_virt(address + cur);
+
+		/* memconsole v1 */
+		if (*bp == BIOS_MEMCONSOLE_V1_MAGIC) {
+			memcpy(&hdr, bp, sizeof(hdr));
+			found = 1;
+			break;
+		}
+
+		/* memconsole v2 */
+		if (*bp == BIOS_MEMCONSOLE_V2_MAGIC) {
+			memcpy(&hdr, bp, sizeof(hdr));
+			found = 2;
+			break;
+		}
+	}
+
+	/*
+	 * At this point hdr points to the EBDA structure
+	 * Shift the contents of the kernel ring by the size of
+	 * the contents in the bios ring.
+	 */
+
+	switch (found) {
+	case 1:
+		printk(KERN_INFO "BIOS console v1 EBDA structure found at %p\n",
+		       bp);
+		printk(KERN_INFO "BIOS console buffer at 0x%.8x, "
+		       "start = %d, end = %d, num = %d\n",
+		       hdr.ptr.v1.buffer_addr, hdr.ptr.v1.start,
+		       hdr.ptr.v1.end, hdr.ptr.v1.num_chars);
+
+		length = hdr.ptr.v1.num_chars;
+		virtp = phys_to_virt(hdr.ptr.v1.buffer_addr);
+		sanitize_buffer(virtp, length);
+		prepend_to_dmesg(virtp, length);
+
+		break;
+	case 2:
+		printk(KERN_INFO "BIOS console v2 EBDA structure found at %p\n",
+		       bp);
+		printk(KERN_INFO "BIOS console buffer at 0x%.8x, "
+		       "start = %d, end = %d, num_bytes = %d\n",
+		       hdr.ptr.v2.buffer_addr, hdr.ptr.v2.start,
+		       hdr.ptr.v2.end, hdr.ptr.v2.num_bytes);
+
+		length = hdr.ptr.v2.end - hdr.ptr.v2.start;
+		virtp = phys_to_virt(hdr.ptr.v2.buffer_addr + hdr.ptr.v2.start);
+		sanitize_buffer(virtp, length);
+		prepend_to_dmesg(virtp, length);
+
+		break;
+	case 0:
+	default:
+		printk("BIOS console EBDA structure not found!\n");
+	}
+	return 0;
+}
+device_initcall(inject_memconsole);


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 4/6] driver: Google Bootlog
  2011-01-25  0:24 ` [PATCH v1 4/6] driver: Google Bootlog Mike Waychison
@ 2011-01-25  0:49   ` Alan Cox
  2011-01-25  1:38     ` Mike Waychison
  0 siblings, 1 reply; 36+ messages in thread
From: Alan Cox @ 2011-01-25  0:49 UTC (permalink / raw)
  To: Mike Waychison
  Cc: Greg KH, torvalds, San Mehat, Aaron Durbin, Duncan Laurie,
	linux-kernel, Tim Hockin

> +/*
> + * Search for the BLOG pointer.
> + */
> +static struct bootlog_ptr * __init bootlog_find_pointer(void)
> +{
> +	unsigned long address, length, cur;
> +	struct bootlog_ptr *bp;
> +
> +	/* EBDA pointer contains segment the extended BIOS data area */
> +	address = *(uint16_t *)phys_to_virt(BLOG_EBDA_POINTER);
> +	address <<= 4;	/* convert segment to physical address */
> +
> +	/* EBDA length is byte 0 of the EBDA (stored in kB) */
> +	length = *(uint8_t *)phys_to_virt(address);
> +	length <<= 10;	/* convert to bytes */

Ok this is wrong on two counts

1.	If the EBDA pointer is zero then no EBDA is present (older
boxes sometimes did this up to about the Athlon era).

2.	EBDA isn't an X86 property it's a PC property. asm/bios_ebda
provides one helper and perhaps it all belongs in there (along with 'is
it a PC' sanity check)

3.	And its probably a good idea given the standard of BIOS code to
also check the EBDA end if given is actually within the 640K limit ...



Alan

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 5/6] Allow prepending to the dmesg
  2011-01-25  0:25 ` [PATCH v1 5/6] Allow prepending to the dmesg Mike Waychison
@ 2011-01-25  1:01   ` Andrew Morton
  0 siblings, 0 replies; 36+ messages in thread
From: Andrew Morton @ 2011-01-25  1:01 UTC (permalink / raw)
  To: Mike Waychison
  Cc: Greg KH, torvalds, San Mehat, Aaron Durbin, Duncan Laurie,
	linux-kernel, Tim Hockin

On Mon, 24 Jan 2011 16:25:00 -0800
Mike Waychison <mikew@google.com> wrote:

> The next patch in this series (Memory Console driver) would like to
> prepend firmware messages to the kernel logs.

I went to the changelog for "Memory Console driver" to work out why it
wants to do prepending, but it didn't tell me.

> Instead of exposing all the nitty gritty lock details of the kernel's
> printk system to the entire kernel, expose a "prepend_to_dmesg()" that
> attempts to rewrite the in-memory dmesg to inject pre-kernel messages.
> 
> This function only prepends if the start of the kernel's messages are
> still in the ring and there is still room for messages (without losing
> the current tail of the message log).  We determine this by checking out
> whether the bufer has yet been cleared (indicated by a new flag:
> buffer_has_cleared), and by checking whether the buffer is less than
> full (which would indicate that it had wrapped).
> 
> If there is enough room to prepend data, we simply shift the existing
> log contents down and copy into the buffer the tail of the prepended
> data that fits.
> 
> Signed-off-by: Mike Waychison <mikew@google.com>
> ---
>  include/linux/printk.h |    5 ++++
>  kernel/printk.c        |   55 +++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 59 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index ee048e7..df89965 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -113,6 +113,7 @@ extern int dmesg_restrict;
>  extern int kptr_restrict;
>  
>  void log_buf_kexec_setup(void);
> +void prepend_to_dmesg(const char *buffer, size_t length);

Should have the __init tag in the declaration (can fix link problems on
weird architectures).

>  #else
>  static inline __attribute__ ((format (printf, 1, 0)))
>  int vprintk(const char *s, va_list args)
> @@ -137,6 +138,10 @@ static inline bool printk_timed_ratelimit(unsigned long *caller_jiffies,
>  static inline void log_buf_kexec_setup(void)
>  {
>  }
> +
> +void prepend_to_dmesg(const char *buffer, size_t length)
> +{
> +}

Didn't this cause the linker to detect multiple definitions of
prepend_to_dmesg()?

Should be static inline void, or __init and not-inlined in a .c file.

>  #endif
>  
> ...

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 4/6] driver: Google Bootlog
  2011-01-25  0:49   ` Alan Cox
@ 2011-01-25  1:38     ` Mike Waychison
  2011-01-25  9:43       ` Alan Cox
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-25  1:38 UTC (permalink / raw)
  To: Alan Cox
  Cc: Greg KH, torvalds, San Mehat, Aaron Durbin, Duncan Laurie,
	linux-kernel, Tim Hockin

On 01/24/11 16:49, Alan Cox wrote:
>> +/*
>> + * Search for the BLOG pointer.
>> + */
>> +static struct bootlog_ptr * __init bootlog_find_pointer(void)
>> +{
>> +	unsigned long address, length, cur;
>> +	struct bootlog_ptr *bp;
>> +
>> +	/* EBDA pointer contains segment the extended BIOS data area */
>> +	address = *(uint16_t *)phys_to_virt(BLOG_EBDA_POINTER);
>> +	address<<= 4;	/* convert segment to physical address */
>> +
>> +	/* EBDA length is byte 0 of the EBDA (stored in kB) */
>> +	length = *(uint8_t *)phys_to_virt(address);
>> +	length<<= 10;	/* convert to bytes */
>
> Ok this is wrong on two counts
>
> 1.	If the EBDA pointer is zero then no EBDA is present (older
> boxes sometimes did this up to about the Athlon era).
>
> 2.	EBDA isn't an X86 property it's a PC property. asm/bios_ebda
> provides one helper and perhaps it all belongs in there (along with 'is
> it a PC' sanity check)

It's not obvious to me how one would make that sort of check (is a PC) 
at runtime. Suggestions?

>
> 3.	And its probably a good idea given the standard of BIOS code to
> also check the EBDA end if given is actually within the 640K limit ...
>
>
>
> Alan


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 6/6] driver: Google Memory Console
  2011-01-25  0:25 ` [PATCH v1 6/6] driver: Google Memory Console Mike Waychison
@ 2011-01-25  2:00   ` Greg KH
  0 siblings, 0 replies; 36+ messages in thread
From: Greg KH @ 2011-01-25  2:00 UTC (permalink / raw)
  To: Mike Waychison
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Mon, Jan 24, 2011 at 04:25:05PM -0800, Mike Waychison wrote:
> This patch introduces the 'memconsole' driver.
> 
> Our firmware gives us access to an in-memory log of the firmware's
> output.   This gives us visibility in a data-center of headless machines
> as to what the firmware is doing.
> 
> The memory console is found by the driver by finding a header block in
> the EBDA.  The buffer is then copied out, and is currently prepended
> to the kernel's dmesg ring-buffer.

Why does this have to be the first thing in the dmesg buffer?
What's wrong with it showing up whenever this driver is loaded like all
other drivers?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 1/6] Add oops notification chain.
  2011-01-25  0:24 ` [PATCH v1 1/6] Add oops notification chain Mike Waychison
@ 2011-01-25  2:06   ` Greg KH
  2011-01-25 20:01     ` Mike Waychison
  0 siblings, 1 reply; 36+ messages in thread
From: Greg KH @ 2011-01-25  2:06 UTC (permalink / raw)
  To: Mike Waychison
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Mon, Jan 24, 2011 at 04:24:39PM -0800, Mike Waychison wrote:
> From: Aaron Durbin <adurbin@google.com>
> 
> Later firmware patches in this series would like to be able to be
> notified whenever an oops occurs on the system, so that it can be
> recorded in the boot log.
> 
> This patch introduces a notifier_block called "oops_notifier_list"
> so that drivers can register to get called whenever an Oops is
> triggered.

But we already have a panic notifier list.  Why create a new one?
What's wrong with the existing one that doesn't work properly for you?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 0/6] google firmware support
  2011-01-25  0:24 [PATCH v1 0/6] google firmware support Mike Waychison
                   ` (5 preceding siblings ...)
  2011-01-25  0:25 ` [PATCH v1 6/6] driver: Google Memory Console Mike Waychison
@ 2011-01-25  3:01 ` Greg KH
  2011-01-25 19:58   ` Mike Waychison
  6 siblings, 1 reply; 36+ messages in thread
From: Greg KH @ 2011-01-25  3:01 UTC (permalink / raw)
  To: Mike Waychison
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Mon, Jan 24, 2011 at 04:24:34PM -0800, Mike Waychison wrote:
> This patchset applies to v2.6.38-rc2.
> 
> The following series implements support for interfaces exposed by
> google's servers' firmware.
> 
> We'd like to have these small drivers included as they are required for
> proper use of the kernel in our infrastructure.  They may not seem like
> much, but a lot of our health automation as well as our human debugging
> efforts are dependent on the functionality herein.
> 
> I'm pretty happy with the way this patchset looks and would like to ask
> that they be merged at some point if there aren't any big objections.
> Getting these in the public Linux tree would bring us closer to being
> able to easily test kernels as they are released.
> 
> I wasn't certain who to send these patches to, please advise if I should
> be CCing anyone else.

I can take these, once they get all worked out, as I think I'm still the
de-facto firmware maintainer.

> [1] and [5] are the only ones that touch the 'core' kernel.
> 
> 
>    - [1] adds a notifier_block that is called on Oops.

This seems like a duplication of the existing infrastructure we already
have for getting notified of oopses.

>    - [2] introduces CONFIG_GOOGLE_FIRMWARE which all Google firmware
>      drivers can depend upon.
> 
>    - [3] and [4] are drivers we use that are ready for inclusion.  [3]
>      communicates with our EFI images via an SMI handshake.  [4] works
>      with our older BIOSes to construct a log of reboot reasons.
> 
>    - [5] prepares for [6] by introducing prepend_to_dmesg() which
>      allows drivers to prepend pre-kernel messages to the dmesg at
>      bootup.

That seems "rude".  Why can't you just pick out from the kernel log the
data you need when it gets printed?  This seems like you feel your code
should always get a "First Post!" message.  What happens when someone
else wants this and you get dueling "firsts"?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-25  0:24 ` [PATCH v1 3/6] driver: Google EFI SMI Mike Waychison
@ 2011-01-25  3:17   ` Greg KH
  2011-01-25 23:12     ` Mike Waychison
  0 siblings, 1 reply; 36+ messages in thread
From: Greg KH @ 2011-01-25  3:17 UTC (permalink / raw)
  To: Mike Waychison
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Mon, Jan 24, 2011 at 04:24:49PM -0800, Mike Waychison wrote:
> The "gsmi" driver bridges userland with firmware specific routines for
> accessing hardware.
> 
> Currently, this driver only supports NVRAM and eventlog information.
> Deprecated functions have been removed from the driver, though their
> op-codes are left in place so that they are not re-used.
> 
> This driver works by trampolining into the firmware via the smi_command
> outlined in the FADT table.  Three protocols are used due to various
> limitations over time, but all are included herein.
> 
> Signed-off-by: Duncan Laurie <dlaurie@google.com>
> Signed-off-by: Aaron Durbin <adurbin@google.com>
> Signed-off-by: Mike Waychison <mikew@google.com>
> ---
>  drivers/firmware/google/Kconfig  |   10 
>  drivers/firmware/google/Makefile |    1 
>  drivers/firmware/google/gsmi.c   |  931 ++++++++++++++++++++++++++++++++++++++
>  fs/compat_ioctl.c                |    7 
>  include/linux/gsmi.h             |  120 +++++
>  include/linux/miscdevice.h       |    1 
>  6 files changed, 1070 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/firmware/google/gsmi.c
>  create mode 100644 include/linux/gsmi.h
> 
> diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
> index 7834729..2d2be7c 100644
> --- a/drivers/firmware/google/Kconfig
> +++ b/drivers/firmware/google/Kconfig
> @@ -11,4 +11,14 @@ config GOOGLE_FIRMWARE
>  menu "Google Firmware Drivers"
>  	depends on GOOGLE_FIRMWARE
>  
> +config GOOGLE_SMI
> +	bool "SMI interface for Google platforms"
> +	depends on ACPI
> +	default y
> +	help
> +	  Say Y here if you want to enable SMI callbacks for Google
> +	  platforms.  This provides an interface for writing to and
> +	  clearing the EFI event log and reading and writing NVRAM
> +	  variables.
> +
>  endmenu
> diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
> index 8b13789..fb127d7 100644
> --- a/drivers/firmware/google/Makefile
> +++ b/drivers/firmware/google/Makefile
> @@ -1 +1,2 @@
>  
> +obj-$(CONFIG_GOOGLE_SMI)		+= gsmi.o
> diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/google/gsmi.c
> new file mode 100644
> index 0000000..682f594
> --- /dev/null
> +++ b/drivers/firmware/google/gsmi.c
> @@ -0,0 +1,931 @@
> +/*
> + * Copyright 2010 Google Inc. All Rights Reserved.
> + * Author: dlaurie@google.com (Duncan Laurie)
> + *
> + * EFI SMI interface for Google platforms
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/spinlock.h>
> +#include <linux/miscdevice.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmapool.h>
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/ioctl.h>
> +#include <linux/acpi.h>
> +#include <linux/io.h>
> +#include <linux/uaccess.h>
> +#include <linux/dmi.h>
> +#include <linux/gsmi.h>
> +#include <linux/kdebug.h>
> +#include <linux/reboot.h>
> +
> +#define GSMI_SHUTDOWN_CLEAN	0	/* Clean Shutdown */
> +#define GSMI_SHUTDOWN_NMIWDT	1	/* NMI Watchdog */
> +#define GSMI_SHUTDOWN_PANIC	2	/* Panic */
> +#define GSMI_SHUTDOWN_OOPS	3	/* Oops */
> +#define GSMI_SHUTDOWN_DIE	4	/* Die */
> +#define GSMI_SHUTDOWN_MCE	5	/* Machine Check */
> +#define GSMI_SHUTDOWN_SOFTWDT	6	/* Software Watchdog */
> +#define GSMI_SHUTDOWN_MBE	7	/* Uncorrected ECC */
> +#define GSMI_SHUTDOWN_TRIPLE	8	/* Triple Fault */
> +
> +#define DRIVER_VERSION		"1.0"
> +#define GSMI_GUID_SIZE		16
> +#define GSMI_BUF_SIZE		1024
> +#define GSMI_BUF_ALIGN		sizeof(u64)
> +#define GSMI_CALLBACK		0xef
> +
> +/* SMI return codes */
> +#define GSMI_SUCCESS		0x00
> +#define GSMI_UNSUPPORTED2	0x03
> +#define GSMI_LOG_FULL		0x0b
> +#define GSMI_VAR_NOT_FOUND	0x0e
> +#define GSMI_HANDSHAKE_SPIN	0x7d
> +#define GSMI_HANDSHAKE_CF	0x7e
> +#define GSMI_HANDSHAKE_NONE	0x7f
> +#define GSMI_INVALID_PARAMETER	0x82
> +#define GSMI_UNSUPPORTED	0x83
> +#define GSMI_BUFFER_TOO_SMALL	0x85
> +#define GSMI_NOT_READY		0x86
> +#define GSMI_DEVICE_ERROR	0x87
> +#define GSMI_NOT_FOUND		0x8e
> +
> +#define QUIRKY_BOARD_HASH 0x78a30a50
> +
> +/* SMI buffers must be in 32bit physical address space */
> +struct gsmi_buf {
> +	uint8_t *start;			/* start of buffer */
> +	size_t length;			/* length of buffer */
> +	dma_addr_t handle;		/* dma allocation handle */
> +	uint32_t address;		/* physical address of buffer */
> +};
> +
> +struct gsmi_device {
> +	struct platform_device *pdev;	/* platform device */
> +	struct gsmi_buf *name_buf;	/* variable name buffer */
> +	struct gsmi_buf *data_buf;	/* generic data buffer */
> +	struct gsmi_buf *param_buf;	/* parameter buffer */
> +	spinlock_t lock;		/* serialize access to SMIs */
> +	uint16_t smi_cmd;		/* SMI command port */
> +	int handshake_type;		/* firmware handler interlock type */
> +	struct dma_pool *dma_pool;	/* DMA buffer pool */
> +};
> +
> +struct gsmi_nvram_var_param {
> +	uint8_t  guid[GSMI_GUID_SIZE];
> +	uint32_t name_ptr;
> +	uint32_t attributes;
> +	uint32_t data_len;
> +	uint32_t data_ptr;
> +} __packed;
> +
> +struct gsmi_get_next_var_param {
> +	uint8_t  guid[GSMI_GUID_SIZE];
> +	uint32_t name_ptr;
> +	uint32_t name_len;
> +} __packed;
> +
> +struct gsmi_set_event_log_param {
> +	uint32_t data_ptr;
> +	uint32_t data_len;
> +	uint32_t type;
> +} __packed;
> +
> +static int gsmi_cmd_get_nvram_var(struct gsmi_ioctl *ctl);
> +static int gsmi_cmd_get_next_var(struct gsmi_ioctl *ctl);
> +static int gsmi_cmd_set_nvram_var(struct gsmi_ioctl *ctl);
> +static int gsmi_cmd_set_event_log(struct gsmi_ioctl *ctl);
> +static int gsmi_cmd_clear_event_log(struct gsmi_ioctl *ctl);
> +static int gsmi_cmd_clear_config(struct gsmi_ioctl *ctl);
> +static int gsmi_cmd_noop(struct gsmi_ioctl *ctl);
> +
> +/* list of ioctl command handlers */
> +static int (*gsmi_cmd_handler[])(struct gsmi_ioctl *ctl) = {
> +	[GSMI_CMD_GET_NVRAM_VAR]    = gsmi_cmd_get_nvram_var,
> +	[GSMI_CMD_GET_NEXT_VAR]	    = gsmi_cmd_get_next_var,
> +	[GSMI_CMD_SET_NVRAM_VAR]    = gsmi_cmd_set_nvram_var,
> +	[GSMI_CMD_SET_EVENT_LOG]    = gsmi_cmd_set_event_log,
> +	[GSMI_CMD_CLEAR_EVENT_LOG]  = gsmi_cmd_clear_event_log,
> +	[GSMI_CMD_CLEAR_CONFIG]     = gsmi_cmd_clear_config,
> +	[GSMI_CMD_NOOP]             = gsmi_cmd_noop,
> +};
> +
> +static long gsmi_ioctl(struct file *file, unsigned int cmd,
> +		       unsigned long arg);
> +
> +static const struct file_operations gsmi_fops = {
> +	.owner		= THIS_MODULE,
> +	.llseek		= no_llseek,
> +	.unlocked_ioctl	= gsmi_ioctl,
> +};
> +
> +static struct miscdevice gsmi_miscdev = {
> +	.minor	= GOOGLE_SMI_MINOR,
> +	.name	= "gsmi",
> +	.fops	= &gsmi_fops,
> +};
> +
> +/* global device handle */
> +static struct gsmi_device *gsmi_dev;
> +
> +/*
> + * Some platforms don't have explicit SMI handshake
> + * and need to wait for SMI to complete.
> + */
> +#define GSMI_DEFAULT_SPINCOUNT	0x10000
> +static uint32_t gsmi_spincount = GSMI_DEFAULT_SPINCOUNT;
> +
> +static int __init gsmi_setup_spincount(char *str)
> +{
> +	unsigned long res;
> +	if (!strict_strtoul(str, 0, &res))
> +		gsmi_spincount = (uint32_t)res;
> +	return 1;
> +}
> +__setup("gsmi_spincount=", gsmi_setup_spincount);
> +
> +static struct gsmi_buf *gsmi_buf_alloc(void)
> +{
> +	struct gsmi_buf *smibuf;
> +
> +	smibuf = kzalloc(sizeof(*smibuf), GFP_KERNEL);
> +	if (!smibuf) {
> +		printk(KERN_ERR "gsmi: out of memory\n");
> +		return NULL;
> +	}
> +
> +	/* allocate buffer in 32bit address space */
> +	smibuf->start = dma_pool_alloc(gsmi_dev->dma_pool, GFP_KERNEL,
> +				       &smibuf->handle);
> +	if (!smibuf->start) {
> +		printk(KERN_ERR "gsmi: failed to allocate name buffer\n");
> +		kfree(smibuf);
> +		return NULL;
> +	}
> +
> +	/* fill in the buffer handle */
> +	smibuf->length = GSMI_BUF_SIZE;
> +	smibuf->address = (uint32_t)virt_to_phys(smibuf->start);
> +	memset(smibuf->start, 0, GSMI_BUF_SIZE);
> +
> +	return smibuf;
> +}
> +
> +static void gsmi_buf_free(struct gsmi_buf *smibuf)
> +{
> +	if (smibuf) {
> +		if (smibuf->start)
> +			dma_pool_free(gsmi_dev->dma_pool, smibuf->start,
> +				      smibuf->handle);
> +		kfree(smibuf);
> +		smibuf = NULL;
> +	}
> +}
> +
> +static int gsmi_exec(uint8_t func, uint8_t sub)
> +{
> +	uint16_t cmd = (sub << 8) | func;
> +	uint16_t result = 0;
> +	int rc = 0;
> +
> +	/*
> +	 * AH  : Subfunction number
> +	 * AL  : Function number
> +	 * EBX : Parameter block address
> +	 * DX  : SMI command port
> +	 *
> +	 * Three protocols here. See also the comment in gsmi_init().
> +	 */
> +	if (gsmi_dev->handshake_type == GSMI_HANDSHAKE_CF) {
> +		/*
> +		 * If handshake_type == HANDSHAKE_CF then set CF on the
> +		 * way in and wait for the handler to clear it; this avoids
> +		 * corrupting register state on those chipsets which have
> +		 * a delay between writing the SMI trigger register and
> +		 * entering SMM.
> +		 */
> +		asm volatile (
> +			"stc\n"
> +			"outb %%al, %%dx\n"
> +		"1:      jc 1b\n"
> +			"mov %%ax, %0"
> +			: "=r" (result)
> +			: "a" (cmd),
> +			  "d" (gsmi_dev->smi_cmd),
> +			  "b" (gsmi_dev->param_buf->address)
> +			: "memory", "cc"
> +		);
> +	} else if (gsmi_dev->handshake_type == GSMI_HANDSHAKE_SPIN) {
> +		/*
> +		 * If handshake_type == HANDSHAKE_SPIN we spin a
> +		 * hundred-ish usecs to ensure the SMI has triggered.
> +		 */
> +		asm volatile (
> +			"outb %%al, %%dx\n"
> +		"1:      loop 1b\n"
> +			"mov %%ax, %0"
> +			: "=r" (result)
> +			: "a" (cmd),
> +			  "d" (gsmi_dev->smi_cmd),
> +			  "b" (gsmi_dev->param_buf->address),
> +			  "c" (gsmi_spincount)
> +			: "memory", "cc"
> +		);
> +	} else {
> +		/*
> +		 * If handshake_type == HANDSHAKE_NONE we do nothing;
> +		 * either we don't need to or it's legacy firmware that
> +		 * doesn't understand the CF protocol.
> +		 */
> +		asm volatile (
> +			"outb %%al, %%dx\n\t"
> +			"mov %%ax, %0"
> +			: "=r" (result)
> +			: "a" (cmd),
> +			  "d" (gsmi_dev->smi_cmd),
> +			  "b" (gsmi_dev->param_buf->address)
> +			: "memory", "cc"
> +		);
> +	}
> +
> +	/* check return code from SMI handler */
> +	switch (result) {
> +	case GSMI_SUCCESS:
> +		break;
> +	case GSMI_VAR_NOT_FOUND:
> +		/* not really an error, but let the caller know */
> +		rc = 1;
> +		break;
> +	case GSMI_INVALID_PARAMETER:
> +		printk(KERN_ERR "gsmi: exec 0x%04x: Invalid parameter\n", cmd);
> +		rc = -EINVAL;
> +		break;
> +	case GSMI_BUFFER_TOO_SMALL:
> +		printk(KERN_ERR "gsmi: exec 0x%04x: Buffer too small\n", cmd);
> +		rc = -ENOMEM;
> +		break;
> +	case GSMI_UNSUPPORTED:
> +	case GSMI_UNSUPPORTED2:
> +		if (sub != GSMI_CMD_HANDSHAKE_TYPE)
> +			printk(KERN_ERR "gsmi: exec 0x%04x: Not supported\n",
> +			       cmd);
> +		rc = -ENOSYS;
> +		break;
> +	case GSMI_NOT_READY:
> +		printk(KERN_ERR "gsmi: exec 0x%04x: Not ready\n", cmd);
> +		rc = -EBUSY;
> +		break;
> +	case GSMI_DEVICE_ERROR:
> +		printk(KERN_ERR "gsmi: exec 0x%04x: Device error\n", cmd);
> +		rc = -EFAULT;
> +		break;
> +	case GSMI_NOT_FOUND:
> +		printk(KERN_ERR "gsmi: exec 0x%04x: Data not found\n", cmd);
> +		rc = -ENOENT;
> +		break;
> +	case GSMI_LOG_FULL:
> +		printk(KERN_ERR "gsmi: exec 0x%04x: Log full\n", cmd);
> +		rc = -ENOSPC;
> +		break;
> +	case GSMI_HANDSHAKE_CF:
> +	case GSMI_HANDSHAKE_SPIN:
> +	case GSMI_HANDSHAKE_NONE:
> +		rc = result;
> +		break;
> +	default:
> +		printk(KERN_ERR "gsmi: exec 0x%04x: Unknown error 0x%04x\n",
> +		       cmd, result);
> +		rc = -ENXIO;
> +	}
> +
> +	return rc;
> +}
> +
> +static int gsmi_cmd_get_nvram_var(struct gsmi_ioctl *ctl)
> +{
> +	struct gsmi_get_nvram_var *cmd = &ctl->gsmi_cmd.get_nvram_var;
> +	struct gsmi_nvram_var_param param = {
> +		.name_ptr = gsmi_dev->name_buf->address,
> +		.data_ptr = gsmi_dev->data_buf->address,
> +		.data_len = cmd->data_len,
> +	};
> +	uint16_t var_name[GSMI_BUF_SIZE / 2];
> +	int i, rc = 0;
> +	unsigned long flags;
> +
> +	if (cmd->name_len > GSMI_BUF_SIZE / 2 ||
> +	    cmd->name_len > sizeof(cmd->name))
> +		return -1;
> +
> +	spin_lock_irqsave(&gsmi_dev->lock, flags);
> +
> +	/* guid */
> +	memcpy(&param.guid, cmd->guid, GSMI_GUID_SIZE);
> +
> +	/* variable name, converted from ascii to UTF-16 */
> +	memset(var_name, 0, sizeof(var_name));
> +	for (i = 0; i < cmd->name_len; i++)
> +		var_name[i] = cmd->name[i];
> +	memset(gsmi_dev->name_buf->start, 0, gsmi_dev->name_buf->length);
> +	memcpy(gsmi_dev->name_buf->start, var_name, cmd->name_len * 2);
> +
> +	/* data pointer */
> +	memset(gsmi_dev->data_buf->start, 0, gsmi_dev->data_buf->length);
> +
> +	/* parameter buffer */
> +	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
> +	memcpy(gsmi_dev->param_buf->start, &param, sizeof(param));
> +
> +	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
> +	if (rc < 0) {
> +		printk(KERN_ERR "gsmi: Get Variable %s failed\n", cmd->name);
> +	} else if (rc == 1) {
> +		/* variable was not found */
> +		rc = -1;
> +	} else {
> +		/* copy data back to return buffer */
> +		memcpy(cmd->data, gsmi_dev->data_buf->start, cmd->data_len);
> +		rc = 1;
> +	}
> +
> +	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
> +
> +	return rc;
> +}
> +
> +static int gsmi_cmd_get_next_var(struct gsmi_ioctl *ctl)
> +{
> +	struct gsmi_get_next_var *cmd = &ctl->gsmi_cmd.get_next_var;
> +	struct gsmi_get_next_var_param param = {
> +		.name_ptr = gsmi_dev->name_buf->address,
> +		.name_len = gsmi_dev->name_buf->length,
> +	};
> +	uint16_t var_name[GSMI_BUF_SIZE / 2];
> +	int i, rc = 0;
> +	unsigned long flags;
> +
> +	if (cmd->name_len > GSMI_BUF_SIZE / 2 ||
> +	    cmd->name_len > sizeof(cmd->name))
> +		return -1;
> +
> +	spin_lock_irqsave(&gsmi_dev->lock, flags);
> +
> +	/* guid */
> +	memcpy(&param.guid, cmd->guid, sizeof(param.guid));
> +
> +	/* variable name, converted from ascii to UTF-16 */
> +	memset(var_name, 0, sizeof(var_name));
> +	for (i = 0; i < cmd->name_len; i++)
> +		var_name[i] = cmd->name[i];
> +	memset(gsmi_dev->name_buf->start, 0, gsmi_dev->name_buf->length);
> +	memcpy(gsmi_dev->name_buf->start, var_name, cmd->name_len * 2);
> +
> +	/* parameter buffer */
> +	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
> +	memcpy(gsmi_dev->param_buf->start, &param, sizeof(param));
> +
> +	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
> +	if (rc < 0) {
> +		printk(KERN_ERR "gsmi: Get Next Variable Name failed\n");
> +	} else if (rc == 1) {
> +		/* variable not found -- end of list */
> +		memset(cmd->guid, 0, sizeof(cmd->guid));
> +		memset(cmd->name, 0, cmd->name_len);
> +		cmd->name_len = 0;
> +		rc = 1;
> +	} else {
> +		/* copy variable data back to return buffer */
> +		memcpy(&param, gsmi_dev->param_buf->start, sizeof(param));
> +
> +		/* convert variable name back to ascii */
> +		memset(var_name, 0, sizeof(var_name));
> +		memcpy(var_name, gsmi_dev->name_buf->start, param.name_len);
> +		memset(cmd->name, 0, sizeof(cmd->name));
> +		cmd->name_len = param.name_len / 2;
> +		for (i = 0; i < cmd->name_len; i++)
> +			cmd->name[i] = var_name[i];
> +
> +		/* copy guid to return buffer */
> +		memcpy(cmd->guid, &param.guid, sizeof(cmd->guid));
> +		rc = 1;
> +	}
> +
> +	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
> +
> +	return rc;
> +}
> +
> +static int gsmi_cmd_set_nvram_var(struct gsmi_ioctl *ctl)
> +{
> +	struct gsmi_set_nvram_var *cmd = &ctl->gsmi_cmd.set_nvram_var;
> +	struct gsmi_nvram_var_param param = {
> +		.name_ptr = gsmi_dev->name_buf->address,
> +		.data_ptr = gsmi_dev->data_buf->address,
> +		.data_len = cmd->data_len,
> +		.attributes = 0x7, /* nvram, boot, runtime */
> +	};
> +	uint16_t var_name[GSMI_BUF_SIZE / 2];
> +	int i, rc = 0;
> +	unsigned long flags;
> +
> +	if (cmd->name_len > GSMI_BUF_SIZE / 2 ||
> +	    cmd->name_len > sizeof(cmd->name))
> +		return -1;
> +
> +	spin_lock_irqsave(&gsmi_dev->lock, flags);
> +
> +	/* guid */
> +	memcpy(&param.guid, cmd->guid, sizeof(param.guid));
> +
> +	/* variable name, converted from ascii to UTF-16 */
> +	memset(var_name, 0, sizeof(var_name));
> +	for (i = 0; i < cmd->name_len; i++)
> +		var_name[i] = cmd->name[i];
> +	memset(gsmi_dev->name_buf->start, 0, gsmi_dev->name_buf->length);
> +	memcpy(gsmi_dev->name_buf->start, var_name, cmd->name_len * 2);
> +
> +	/* data pointer */
> +	memset(gsmi_dev->data_buf->start, 0, gsmi_dev->data_buf->length);
> +	memcpy(gsmi_dev->data_buf->start, cmd->data, cmd->data_len);
> +
> +	/* parameter buffer */
> +	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
> +	memcpy(gsmi_dev->param_buf->start, &param, sizeof(param));
> +
> +	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
> +	if (rc < 0)
> +		printk(KERN_ERR "gsmi: Set Variable %s failed\n", cmd->name);
> +
> +	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
> +
> +	return rc;
> +}
> +
> +static int gsmi_cmd_set_event_log(struct gsmi_ioctl *ctl)
> +{
> +	struct gsmi_set_event_log *cmd = &ctl->gsmi_cmd.set_event_log;
> +	struct gsmi_set_event_log_param param = {
> +		.data_ptr = gsmi_dev->data_buf->address,
> +		.data_len = cmd->data_len,
> +		.type     = cmd->type,
> +	};
> +	int rc = 0;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&gsmi_dev->lock, flags);
> +
> +	/* data pointer */
> +	memset(gsmi_dev->data_buf->start, 0, gsmi_dev->data_buf->length);
> +	memcpy(gsmi_dev->data_buf->start, cmd->data, cmd->data_len);
> +
> +	/* parameter buffer */
> +	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
> +	memcpy(gsmi_dev->param_buf->start, &param, sizeof(param));
> +
> +	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
> +	if (rc < 0)
> +		printk(KERN_ERR "gsmi: Set Event Log failed\n");
> +
> +	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
> +
> +	return rc;
> +}
> +
> +static int gsmi_cmd_clear_event_log(struct gsmi_ioctl *ctl)
> +{
> +	struct gsmi_clear_event_log *cmd = &ctl->gsmi_cmd.clear_event_log;
> +	int rc = 0;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&gsmi_dev->lock, flags);
> +
> +	/* parameter buffer */
> +	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
> +	memcpy(gsmi_dev->param_buf->start, cmd, sizeof(*cmd));
> +
> +	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
> +	if (rc < 0)
> +		printk(KERN_ERR "gsmi: Clear Event Log failed\n");
> +
> +	/* type 0 will return available space in log */
> +	if (cmd->type == 0)
> +		rc = 1;
> +
> +	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
> +
> +	return rc;
> +}
> +
> +static int gsmi_cmd_clear_config(struct gsmi_ioctl *ctl)
> +{
> +	int rc = 0;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&gsmi_dev->lock, flags);
> +
> +	/* clear parameter buffer */
> +	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
> +
> +	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
> +	if (rc < 0)
> +		printk(KERN_ERR "gsmi: Clear Config failed\n");
> +
> +	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
> +
> +	return rc;
> +}
> +
> +static int gsmi_cmd_noop(struct gsmi_ioctl *ctl)
> +{
> +	int rc = 0;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&gsmi_dev->lock, flags);
> +
> +	/* clear parameter buffer */
> +	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
> +
> +	rc = gsmi_exec(GSMI_CALLBACK, ctl->command);
> +	if (rc < 0)
> +		printk(KERN_ERR "gsmi: No-op failed\n");
> +
> +	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
> +
> +	return rc;
> +}
> +
> +static long gsmi_ioctl(struct file *file, unsigned int cmd,
> +		      unsigned long arg)
> +{
> +	struct gsmi_ioctl *ctl = NULL;
> +	uint16_t length;
> +	int rc = 0;
> +
> +	/* everything is in the same ioctl */
> +	if (cmd != GSMI_IOCTL)
> +		return -ENOIOCTLCMD;
> +
> +	/* get length from user */
> +	if (copy_from_user(&length, (size_t __user *)arg,
> +			   sizeof(length))) {
> +		rc = -EFAULT;
> +		goto done;
> +	}
> +
> +	/* verify length */
> +	if (length < sizeof(*ctl)) {
> +		rc = -EINVAL;
> +		goto done;
> +	}
> +
> +	/* allocate ioctl structure */
> +	ctl = kzalloc(length, GFP_KERNEL);
> +	if (!ctl) {
> +		rc = -ENOMEM;
> +		goto done;
> +	}
> +
> +	/* copy rest of structure */
> +	if (copy_from_user(ctl, (void __user *)arg, length)) {
> +		rc = -EFAULT;
> +		goto done;
> +	}
> +
> +	/* verify structure version */
> +	if (ctl->version != GSMI_IOCTL_VERSION) {
> +		rc = -EINVAL;
> +		goto done;
> +	}
> +
> +	/* find and run command handler */
> +	if (ctl->command > sizeof(gsmi_cmd_handler)/sizeof(*gsmi_cmd_handler)) {
> +		rc = -EINVAL;
> +		goto done;
> +	}
> +	if (gsmi_cmd_handler[ctl->command] == NULL) {
> +		rc = -EINVAL;
> +		goto done;
> +	}
> +	rc = gsmi_cmd_handler[ctl->command](ctl);
> +	if (rc < 0) {
> +		goto done;
> +	} else if (rc > 0) {
> +		/* return data to user if needed */
> +		if (copy_to_user((void __user *)arg, ctl, length)) {
> +			rc = -EFAULT;
> +			goto done;
> +		}
> +		rc = 0;
> +	}
> +
> + done:
> +	kfree(ctl);
> +
> +	return rc;
> +}
> +
> +static int gsmi_shutdown_reason(int reason)
> +{
> +	struct gsmi_log_entry_type_1 entry = {
> +		.type     = GSMI_LOG_ENTRY_TYPE_KERNEL,
> +		.instance = reason,
> +	};
> +	struct gsmi_set_event_log_param param = {
> +		.data_len = sizeof(entry),
> +		.type     = 1,
> +	};
> +	static int saved_reason;
> +	int rc = 0;
> +	unsigned long flags;
> +
> +	/* abort early if we were never setup */
> +	if (!gsmi_dev)
> +		return -1;
> +
> +	/* avoid duplicate entries in the log */
> +	if (saved_reason & (1 << reason))
> +		return 0;
> +
> +	spin_lock_irqsave(&gsmi_dev->lock, flags);
> +
> +	saved_reason |= (1 << reason);
> +
> +	/* data pointer */
> +	memset(gsmi_dev->data_buf->start, 0, gsmi_dev->data_buf->length);
> +	memcpy(gsmi_dev->data_buf->start, &entry, sizeof(entry));
> +
> +	/* parameter buffer */
> +	param.data_ptr = gsmi_dev->data_buf->address;
> +	memset(gsmi_dev->param_buf->start, 0, gsmi_dev->param_buf->length);
> +	memcpy(gsmi_dev->param_buf->start, &param, sizeof(param));
> +
> +	rc = gsmi_exec(GSMI_CALLBACK, GSMI_CMD_SET_EVENT_LOG);
> +
> +	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
> +
> +	if (rc < 0)
> +		printk(KERN_ERR "gsmi: Log Shutdown Reason failed\n");
> +	else
> +		printk(KERN_EMERG "gsmi: Log Shutdown Reason 0x%02x\n",
> +		       reason);
> +
> +	return rc;
> +}
> +
> +static int gsmi_reboot_callback(struct notifier_block *nb,
> +				unsigned long reason, void *arg)
> +{
> +	gsmi_shutdown_reason(GSMI_SHUTDOWN_CLEAN);
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block gsmi_reboot_notifier = {
> +	.notifier_call = gsmi_reboot_callback
> +};
> +
> +static int gsmi_die_callback(struct notifier_block *nb,
> +			     unsigned long reason, void *arg)
> +{
> +	if (reason == DIE_NMIWATCHDOG)
> +		gsmi_shutdown_reason(GSMI_SHUTDOWN_NMIWDT);
> +	else if (reason == DIE_OOPS)
> +		gsmi_shutdown_reason(GSMI_SHUTDOWN_DIE);
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block gsmi_die_notifier = {
> +	.notifier_call = gsmi_die_callback
> +};
> +
> +static int gsmi_panic_callback(struct notifier_block *nb,
> +			       unsigned long reason, void *arg)
> +{
> +	gsmi_shutdown_reason(GSMI_SHUTDOWN_PANIC);
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block gsmi_panic_notifier = {
> +	.notifier_call = gsmi_panic_callback
> +};
> +
> +static int gsmi_oops_callback(struct notifier_block *nb,
> +			      unsigned long reason, void *arg)
> +{
> +	if (reason == OOPS_ENTER)
> +		gsmi_shutdown_reason(GSMI_SHUTDOWN_OOPS);
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block gsmi_oops_notifier = {
> +	.notifier_call = gsmi_oops_callback
> +};
> +
> +/*
> + * This hash function was blatantly copied from include/linux/hash.h.
> + * It is used by this driver to obfuscate a board name that requires a
> + * quirk within this driver.
> + *
> + * Please do not remove this copy of the function as any changes to the
> + * global utility hash_64() function would break this driver's ability
> + * to identify a board and provide the appropriate quirk -- mikew@google.com
> + */
> +static u64 __init local_hash_64(u64 val, unsigned bits)
> +{
> +	u64 hash = val;
> +
> +	/*  Sigh, gcc can't optimise this alone like it does for 32 bits. */
> +	u64 n = hash;
> +	n <<= 18;
> +	hash -= n;
> +	n <<= 33;
> +	hash -= n;
> +	n <<= 3;
> +	hash += n;
> +	n <<= 3;
> +	hash -= n;
> +	n <<= 4;
> +	hash += n;
> +	n <<= 2;
> +	hash += n;
> +
> +	/* High bits are more random, so use them. */
> +	return hash >> (64 - bits);
> +}
> +
> +static u32 __init hash_oem_table_id(char s[8])
> +{
> +	u64 input;
> +	memcpy(&input, s, 8);
> +	return local_hash_64(input, 32);
> +}
> +
> +static __init int gsmi_init(void)
> +{
> +	unsigned long flags;
> +	u32 hash;
> +
> +	/*
> +	 * Check platform compatibility.  Only ever load on Google's
> +	 * machines.
> +	 */
> +	if (strncmp(acpi_gbl_FADT.header.oem_id, "GOOGLE", ACPI_OEM_ID_SIZE)) {
> +		printk(KERN_INFO "gsmi: Not a google board\n");
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * Only newer firmware supports the gsmi interface.  All older
> +	 * firmware that didn't support this interface used to plug the
> +	 * table name in the first four bytes of the oem_table_id field.
> +	 * Newer firmware doesn't do that though, so use that as the
> +	 * discriminant factor.  We have to do this in order to
> +	 * whitewash our board names out of the public driver.
> +	 */
> +	if (!strncmp(acpi_gbl_FADT.header.oem_table_id, "FACP", 4)) {
> +		printk(KERN_INFO "gsmi: Board is too old\n");
> +		return -ENODEV;
> +	}
> +
> +	/* Disable on board with 1.0 BIOS due to Google bug 2602657 */
> +	hash = hash_oem_table_id(acpi_gbl_FADT.header.oem_table_id);
> +	if (hash == QUIRKY_BOARD_HASH) {
> +		const char *bios_ver = dmi_get_system_info(DMI_BIOS_VERSION);
> +		if (strncmp(bios_ver, "1.0", 3) == 0) {
> +			pr_info("gsmi: disabled on this board's BIOS %s\n",
> +				bios_ver);
> +			return -ENODEV;
> +		}
> +	}
> +
> +	/* check for valid SMI command port in ACPI FADT */
> +	if (acpi_gbl_FADT.smi_command == 0)
> +		return -ENODEV;
> +
> +	/* allocate device structure */
> +	gsmi_dev = kzalloc(sizeof(*gsmi_dev), GFP_KERNEL);
> +	if (!gsmi_dev)
> +		return -ENOMEM;
> +	gsmi_dev->smi_cmd = acpi_gbl_FADT.smi_command;
> +
> +	/* register device */
> +	gsmi_dev->pdev = platform_device_register_simple("gsmi", -1, NULL, 0);
> +	if (IS_ERR(gsmi_dev->pdev)) {
> +		printk(KERN_ERR "gsmi: unable to register platform device\n");
> +		kfree(gsmi_dev);
> +		gsmi_dev = NULL;
> +		return -ENODEV;
> +	}
> +	if (misc_register(&gsmi_miscdev) < 0) {
> +		printk(KERN_ERR "gsmi: unable to register misc device\n");
> +		platform_device_unregister(gsmi_dev->pdev);
> +		kfree(gsmi_dev);
> +		gsmi_dev = NULL;
> +		return -ENODEV;
> +	}
> +
> +	/* SMI access needs to be serialized */
> +	spin_lock_init(&gsmi_dev->lock);
> +
> +	/* SMI callbacks require 32bit addresses */
> +	gsmi_dev->pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> +	gsmi_dev->pdev->dev.dma_mask =
> +		&gsmi_dev->pdev->dev.coherent_dma_mask;
> +	gsmi_dev->dma_pool = dma_pool_create("gsmi", &gsmi_dev->pdev->dev,
> +					     GSMI_BUF_SIZE, GSMI_BUF_ALIGN, 0);
> +
> +	/*
> +	 * pre-allocate buffers because sometimes we are called when
> +	 * this is not feasible: oops, panic, die, mce, etc
> +	 */
> +	gsmi_dev->name_buf = gsmi_buf_alloc();
> +	if (!gsmi_dev->name_buf) {
> +		printk(KERN_ERR "gsmi: failed to allocate name buffer\n");
> +		goto out_err;
> +	}
> +
> +	gsmi_dev->data_buf = gsmi_buf_alloc();
> +	if (!gsmi_dev->data_buf) {
> +		printk(KERN_ERR "gsmi: failed to allocate data buffer\n");
> +		goto out_err;
> +	}
> +
> +	gsmi_dev->param_buf = gsmi_buf_alloc();
> +	if (!gsmi_dev->param_buf) {
> +		printk(KERN_ERR "gsmi: failed to allocate param buffer\n");
> +		goto out_err;
> +	}
> +
> +	/*
> +	 * Determine type of handshake used to serialize the SMI
> +	 * entry. See also gsmi_exec().
> +	 *
> +	 * There's a "behavior" present on some chipsets where writing the
> +	 * SMI trigger register in the southbridge doesn't result in an
> +	 * immediate SMI. Rather, the processor can execute "a few" more
> +	 * instructions before the SMI takes effect. To ensure synchronous
> +	 * behavior, implement a handshake between the kernel driver and the
> +	 * firmware handler to spin until released. This ioctl determines
> +	 * the type of handshake.
> +	 *
> +	 * NONE: The firmware handler does not implement any
> +	 * handshake. Either it doesn't need to, or it's legacy firmware
> +	 * that doesn't know it needs to and never will.
> +	 *
> +	 * CF: The firmware handler will clear the CF in the saved
> +	 * state before returning. The driver may set the CF and test for
> +	 * it to clear before proceeding.
> +	 *
> +	 * SPIN: The firmware handler does not implement any handshake
> +	 * but the driver should spin for a hundred or so microseconds
> +	 * to ensure the SMI has triggered.
> +	 *
> +	 * Finally, the handler will return -ENOSYS if
> +	 * GSMI_CMD_HANDSHAKE_TYPE is unimplemented, which implies
> +	 * HANDSHAKE_NONE.
> +	 */
> +	spin_lock_irqsave(&gsmi_dev->lock, flags);
> +	gsmi_dev->handshake_type = GSMI_HANDSHAKE_SPIN;
> +	gsmi_dev->handshake_type =
> +	    gsmi_exec(GSMI_CALLBACK, GSMI_CMD_HANDSHAKE_TYPE);
> +	if (gsmi_dev->handshake_type == -ENOSYS)
> +		gsmi_dev->handshake_type = GSMI_HANDSHAKE_NONE;
> +	spin_unlock_irqrestore(&gsmi_dev->lock, flags);
> +
> +	/* Remove and clean up gsmi if the handshake could not complete. */
> +	if (gsmi_dev->handshake_type == -ENXIO) {
> +		printk(KERN_INFO "gsmi version " DRIVER_VERSION
> +		       " failed to load\n");
> +		goto out_err;
> +	}
> +
> +	printk(KERN_INFO "gsmi version " DRIVER_VERSION " loaded\n");
> +
> +	register_reboot_notifier(&gsmi_reboot_notifier);
> +	register_die_notifier(&gsmi_die_notifier);
> +	atomic_notifier_chain_register(&panic_notifier_list,
> +				       &gsmi_panic_notifier);
> +	register_oops_notifier(&gsmi_oops_notifier);
> +
> +	return 0;
> +
> + out_err:
> +	gsmi_buf_free(gsmi_dev->param_buf);
> +	gsmi_buf_free(gsmi_dev->data_buf);
> +	gsmi_buf_free(gsmi_dev->name_buf);
> +	dma_pool_destroy(gsmi_dev->dma_pool);
> +	platform_device_unregister(gsmi_dev->pdev);
> +	misc_deregister(&gsmi_miscdev);
> +	kfree(gsmi_dev);
> +	gsmi_dev = NULL;
> +	return -ENODEV;
> +}
> +
> +late_initcall(gsmi_init);
> diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
> index 61abb63..588e458 100644
> --- a/fs/compat_ioctl.c
> +++ b/fs/compat_ioctl.c
> @@ -114,6 +114,8 @@
>  #include <asm/fbio.h>
>  #endif
>  
> +#include <linux/gsmi.h>
> +
>  static int w_long(unsigned int fd, unsigned int cmd,
>  		compat_ulong_t __user *argp)
>  {
> @@ -1408,6 +1410,11 @@ IGNORE_IOCTL(FBIOGETCMAP32)
>  IGNORE_IOCTL(FBIOSCURSOR32)
>  IGNORE_IOCTL(FBIOGCURSOR32)
>  #endif
> +
> +#ifdef CONFIG_GOOGLE_SMI
> +/* Google SMI driver for /dev/gsmi */
> +COMPATIBLE_IOCTL(GSMI_IOCTL)
> +#endif
>  };
>  
>  /*
> diff --git a/include/linux/gsmi.h b/include/linux/gsmi.h
> new file mode 100644
> index 0000000..8b35b5c
> --- /dev/null
> +++ b/include/linux/gsmi.h
> @@ -0,0 +1,120 @@
> +/*
> + * Copyright 2010 Google Inc. All Rights Reserved.
> + * Author: dlaurie@google.com (Duncan Laurie)
> + *
> + * EFI SMI interface for Google platforms
> + */
> +
> +#ifndef _LINUX_GSMI_H
> +#define _LINUX_GSMI_H
> +
> +#include <linux/types.h>
> +
> +/*
> + * Get NVRAM Variable
> + *
> + * Must know EFI GUID and exact name to retrieve a variable
> + */
> +struct gsmi_get_nvram_var {
> +	uint8_t		guid[16];	/* IN: unique identifier */
> +	uint16_t	name_len;	/* IN: length of ascii name */
> +	char		name[512];	/* IN: unique name in ascii */
> +	uint16_t	data_len;	/* IN: length of data */
> +	uint8_t		data[0];	/* OUT: variable data */
> +} __packed;
> +
> +/*
> + * Get Next NVRAM Variable Name
> + *
> + * Can be used multiple times to get all variables in the system
> + * by supplying the output of one call as the input of the next.
> + */
> +struct gsmi_get_next_var {
> +	uint8_t		guid[16];	/* IN/OUT: unique identifier */
> +	uint16_t	name_len;	/* IN/OUT: length of ascii name */
> +	char		name[512];	/* IN/OUT: unique name in ascii */
> +} __packed;
> +
> +/*
> + * Set NVRAM Variable
> + *
> + * Must know EFI GUID and exact name to set a variable
> + */
> +struct gsmi_set_nvram_var {
> +	uint8_t		guid[16];	/* IN: unique identifier */
> +	uint16_t	name_len;	/* IN: length of ascii name */
> +	char		name[512];	/* IN: unique name in ascii */
> +	uint16_t	data_len;	/* IN: length of data */
> +	uint8_t		data[0];	/* IN: variable data */
> +} __packed;
> +
> +/*
> + * Add entry to event log
> + *
> + * Current defined entry type is 1
> + */
> +#define GSMI_LOG_ENTRY_TYPE_KERNEL	0xdead
> +struct gsmi_log_entry_type_1 {
> +	uint16_t	type;
> +	uint32_t	instance;
> +} __packed;
> +
> +struct gsmi_set_event_log {
> +	uint32_t	type;		/* IN: type of data in event buffer */
> +	uint16_t	data_len;	/* IN: length of event buffer */
> +	uint8_t		data[0];	/* IN: event data buffer */
> +} __packed;
> +
> +/*
> + * Clear event log
> + *
> + * Valid types are:
> + *   0   : does not clear but returns percent free
> + *   25  : ensure 25% of log is clear
> + *   50  : ensure 50% of log is clear
> + *   75  : ensure 75% of log is clear
> + *   100 : ensure 100% of log is clear
> + */
> +struct gsmi_clear_event_log {
> +	uint32_t	type;		/* IN: clear type */
> +} __packed;
> +
> +/*
> + * These are the various defined SMI callbacks.
> + * They all correspond to a structure defined above
> + * and below and are all used via the same ioctl.
> + * Some of the lower numbers are defined by vendors.
> + * Let's start defining Google-internal callbacks at 0xc0,
> + * a namespace partition that will surely have to be revisited someday.
> + */
> +#define GSMI_CMD_GET_NVRAM_VAR		0x01
> +#define GSMI_CMD_GET_NEXT_VAR		0x02
> +#define GSMI_CMD_SET_NVRAM_VAR		0x03
> +#define GSMI_CMD_SET_EVENT_LOG		0x08
> +#define GSMI_CMD_CLEAR_EVENT_LOG	0x09
> +#define GSMI_CMD_DONOTUSE_15		0x15
> +#define GSMI_CMD_DONOTUSE_16		0x16
> +#define GSMI_CMD_DONOTUSE_17		0x17
> +#define GSMI_CMD_CLEAR_CONFIG		0x20
> +#define GSMI_CMD_NOOP			0xc0
> +#define GSMI_CMD_HANDSHAKE_TYPE		0xc1
> +
> +struct gsmi_ioctl {
> +	uint16_t	length;		/* total length including data */
> +	int		version;	/* structure version */
> +	int		command;	/* ioctl command */

Ick.  Use proper data types if you are going to create a new ioctl.
Same goes for the structures above (hint, use __u32 and friends, not the
unit??_t crap.

I'd strongly suggest NOT creating a new ioctl though, that' just going
to be a pain in the long run.


> +	union {
> +		struct gsmi_get_nvram_var	get_nvram_var;
> +		struct gsmi_get_next_var	get_next_var;
> +		struct gsmi_set_nvram_var	set_nvram_var;
> +		struct gsmi_set_event_log	set_event_log;
> +		struct gsmi_clear_event_log	clear_event_log;
> +	} gsmi_cmd;
> +} __packed;
> +
> +#define GSMI_BASE			'G'
> +#define GSMI_IOCTL_VERSION		1
> +#define GSMI_IOCTL			_IOWR(GSMI_BASE, GSMI_IOCTL_VERSION, \
> +					    struct gsmi_ioctl)
> +
> +#endif /* _LINUX_GSMI_H */
> diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
> index 18fd130..34f5dfa 100644
> --- a/include/linux/miscdevice.h
> +++ b/include/linux/miscdevice.h
> @@ -40,6 +40,7 @@
>  #define BTRFS_MINOR		234
>  #define AUTOFS_MINOR		235
>  #define MAPPER_CTRL_MINOR	236
> +#define GOOGLE_SMI_MINOR	242
>  #define MISC_DYNAMIC_MINOR	255

Why make this a static number and not just use a dynamic one?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 4/6] driver: Google Bootlog
  2011-01-25  1:38     ` Mike Waychison
@ 2011-01-25  9:43       ` Alan Cox
  0 siblings, 0 replies; 36+ messages in thread
From: Alan Cox @ 2011-01-25  9:43 UTC (permalink / raw)
  To: Mike Waychison
  Cc: Greg KH, torvalds, San Mehat, Aaron Durbin, Duncan Laurie,
	linux-kernel, Tim Hockin

> > 2.	EBDA isn't an X86 property it's a PC property. asm/bios_ebda
> > provides one helper and perhaps it all belongs in there (along with 'is
> > it a PC' sanity check)
> 
> It's not obvious to me how one would make that sort of check (is a PC) 
> at runtime. Suggestions?

I suspect the right thing to do is to put a get_ebda_foo() helper set
into arch/x86 which platforms can override. That way non PC platforms can
simply report zero.

One for the x86 maintainers.

Alan

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 0/6] google firmware support
  2011-01-25  3:01 ` [PATCH v1 0/6] google firmware support Greg KH
@ 2011-01-25 19:58   ` Mike Waychison
  2011-01-26  2:47     ` Greg KH
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-25 19:58 UTC (permalink / raw)
  To: Greg KH
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Mon, Jan 24, 2011 at 7:01 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Jan 24, 2011 at 04:24:34PM -0800, Mike Waychison wrote:
>>
>>    - [5] prepares for [6] by introducing prepend_to_dmesg() which
>>      allows drivers to prepend pre-kernel messages to the dmesg at
>>      bootup.
>
> That seems "rude".  Why can't you just pick out from the kernel log the
> data you need when it gets printed?  This seems like you feel your code
> should always get a "First Post!" message.

Well, the use case we have is that we prepend all the bios output (and
anything emitted by option roms and boot loaders via int 10h).  This
typically runs in the 100+ lines of output.

Basically, everything the user sees on the console after power up is
available to him in the dmesg.  We did this out of convenience as it
makes for a good looking log :)

That said, it's dmesg, and we don't build automation on anything in
there if we can help it.  I can move this memory console dump
elsewhere (/sys/firmware/log ?)

>  What happens when someone
> else wants this and you get dueling "firsts"?

No idea :)

Originally I had something a little different for getting this done:
I had introduced a weak symbol called something like
"arch_dmesg_fixup()" that could be made strong in the architecture
code somewhere (in my case, it was in this driver).    That sort of
pattern would allow us to enforce having only one 'first post'
callback :)

Anyhow, let me know what you think about /sys/firmware/log or similar.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 1/6] Add oops notification chain.
  2011-01-25  2:06   ` Greg KH
@ 2011-01-25 20:01     ` Mike Waychison
  2011-01-25 21:36       ` Jeff Garzik
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-25 20:01 UTC (permalink / raw)
  To: Greg KH
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Mon, Jan 24, 2011 at 6:06 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Jan 24, 2011 at 04:24:39PM -0800, Mike Waychison wrote:
>> From: Aaron Durbin <adurbin@google.com>
>>
>> Later firmware patches in this series would like to be able to be
>> notified whenever an oops occurs on the system, so that it can be
>> recorded in the boot log.
>>
>> This patch introduces a notifier_block called "oops_notifier_list"
>> so that drivers can register to get called whenever an Oops is
>> triggered.
>
> But we already have a panic notifier list.  Why create a new one?
> What's wrong with the existing one that doesn't work properly for you?

AFAICT, the panic notifier list doesn't get called on oops.  Should we
reuse it and actually add meaning to the 'reason' argument?  It does
become a misnomer if we do that :\

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 1/6] Add oops notification chain.
  2011-01-25 20:01     ` Mike Waychison
@ 2011-01-25 21:36       ` Jeff Garzik
  2011-01-25 21:43         ` Aaron Durbin
  0 siblings, 1 reply; 36+ messages in thread
From: Jeff Garzik @ 2011-01-25 21:36 UTC (permalink / raw)
  To: Mike Waychison
  Cc: Greg KH, torvalds, San Mehat, Aaron Durbin, Duncan Laurie,
	linux-kernel, Tim Hockin

On 01/25/2011 03:01 PM, Mike Waychison wrote:
> On Mon, Jan 24, 2011 at 6:06 PM, Greg KH<greg@kroah.com>  wrote:
>> On Mon, Jan 24, 2011 at 04:24:39PM -0800, Mike Waychison wrote:
>>> From: Aaron Durbin<adurbin@google.com>
>>>
>>> Later firmware patches in this series would like to be able to be
>>> notified whenever an oops occurs on the system, so that it can be
>>> recorded in the boot log.
>>>
>>> This patch introduces a notifier_block called "oops_notifier_list"
>>> so that drivers can register to get called whenever an Oops is
>>> triggered.
>>
>> But we already have a panic notifier list.  Why create a new one?
>> What's wrong with the existing one that doesn't work properly for you?
>
> AFAICT, the panic notifier list doesn't get called on oops.

Have you tried playing with panic_on_oops ?

	Jeff



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 1/6] Add oops notification chain.
  2011-01-25 21:36       ` Jeff Garzik
@ 2011-01-25 21:43         ` Aaron Durbin
  2011-01-25 21:54           ` Jeff Garzik
  0 siblings, 1 reply; 36+ messages in thread
From: Aaron Durbin @ 2011-01-25 21:43 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Mike Waychison, Greg KH, torvalds, San Mehat, Duncan Laurie,
	linux-kernel, Tim Hockin

On Tue, Jan 25, 2011 at 1:36 PM, Jeff Garzik <jeff@garzik.org> wrote:
> On 01/25/2011 03:01 PM, Mike Waychison wrote:
>>
>> On Mon, Jan 24, 2011 at 6:06 PM, Greg KH<greg@kroah.com>  wrote:
>>>
>>> On Mon, Jan 24, 2011 at 04:24:39PM -0800, Mike Waychison wrote:
>>>>
>>>> From: Aaron Durbin<adurbin@google.com>
>>>>
>>>> Later firmware patches in this series would like to be able to be
>>>> notified whenever an oops occurs on the system, so that it can be
>>>> recorded in the boot log.
>>>>
>>>> This patch introduces a notifier_block called "oops_notifier_list"
>>>> so that drivers can register to get called whenever an Oops is
>>>> triggered.
>>>
>>> But we already have a panic notifier list.  Why create a new one?
>>> What's wrong with the existing one that doesn't work properly for you?
>>
>> AFAICT, the panic notifier list doesn't get called on oops.
>
> Have you tried playing with panic_on_oops ?

Yes. We actually run in that setup. However, oops != panic. They are 2
distinct events. Sometimes we panic without the oops under certain
situations. That is why it is desirable to have 2 distinct events.
>
>        Jeff
>
>
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 1/6] Add oops notification chain.
  2011-01-25 21:43         ` Aaron Durbin
@ 2011-01-25 21:54           ` Jeff Garzik
  2011-01-25 22:21             ` Aaron Durbin
  0 siblings, 1 reply; 36+ messages in thread
From: Jeff Garzik @ 2011-01-25 21:54 UTC (permalink / raw)
  To: Aaron Durbin
  Cc: Mike Waychison, Greg KH, torvalds, San Mehat, Duncan Laurie,
	linux-kernel, Tim Hockin

On 01/25/2011 04:43 PM, Aaron Durbin wrote:
> On Tue, Jan 25, 2011 at 1:36 PM, Jeff Garzik<jeff@garzik.org>  wrote:
>> On 01/25/2011 03:01 PM, Mike Waychison wrote:
>>>
>>> On Mon, Jan 24, 2011 at 6:06 PM, Greg KH<greg@kroah.com>    wrote:
>>>>
>>>> On Mon, Jan 24, 2011 at 04:24:39PM -0800, Mike Waychison wrote:
>>>>>
>>>>> From: Aaron Durbin<adurbin@google.com>
>>>>>
>>>>> Later firmware patches in this series would like to be able to be
>>>>> notified whenever an oops occurs on the system, so that it can be
>>>>> recorded in the boot log.
>>>>>
>>>>> This patch introduces a notifier_block called "oops_notifier_list"
>>>>> so that drivers can register to get called whenever an Oops is
>>>>> triggered.
>>>>
>>>> But we already have a panic notifier list.  Why create a new one?
>>>> What's wrong with the existing one that doesn't work properly for you?
>>>
>>> AFAICT, the panic notifier list doesn't get called on oops.
>>
>> Have you tried playing with panic_on_oops ?
>
> Yes. We actually run in that setup. However, oops != panic. They are 2
> distinct events. Sometimes we panic without the oops under certain
> situations. That is why it is desirable to have 2 distinct events.

That's a circular statement:  They are distinct events, so it is 
desirable that they be distinct events?

Set a flag, a la oops_in_progress (unfortunately name, as it's called 
during a panic too).  Call it...  oops_really_in_progress.  Then will 
the panic notifier list suffice?

	Jeff




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 1/6] Add oops notification chain.
  2011-01-25 21:54           ` Jeff Garzik
@ 2011-01-25 22:21             ` Aaron Durbin
  2011-01-26  2:48               ` Greg KH
  0 siblings, 1 reply; 36+ messages in thread
From: Aaron Durbin @ 2011-01-25 22:21 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Mike Waychison, Greg KH, torvalds, San Mehat, Duncan Laurie,
	linux-kernel, Tim Hockin

On Tue, Jan 25, 2011 at 1:54 PM, Jeff Garzik <jeff@garzik.org> wrote:
> On 01/25/2011 04:43 PM, Aaron Durbin wrote:
>>
>> On Tue, Jan 25, 2011 at 1:36 PM, Jeff Garzik<jeff@garzik.org>  wrote:
>>>
>>> On 01/25/2011 03:01 PM, Mike Waychison wrote:
>>>>
>>>> On Mon, Jan 24, 2011 at 6:06 PM, Greg KH<greg@kroah.com>    wrote:
>>>>>
>>>>> On Mon, Jan 24, 2011 at 04:24:39PM -0800, Mike Waychison wrote:
>>>>>>
>>>>>> From: Aaron Durbin<adurbin@google.com>
>>>>>>
>>>>>> Later firmware patches in this series would like to be able to be
>>>>>> notified whenever an oops occurs on the system, so that it can be
>>>>>> recorded in the boot log.
>>>>>>
>>>>>> This patch introduces a notifier_block called "oops_notifier_list"
>>>>>> so that drivers can register to get called whenever an Oops is
>>>>>> triggered.
>>>>>
>>>>> But we already have a panic notifier list.  Why create a new one?
>>>>> What's wrong with the existing one that doesn't work properly for you?
>>>>
>>>> AFAICT, the panic notifier list doesn't get called on oops.
>>>
>>> Have you tried playing with panic_on_oops ?
>>
>> Yes. We actually run in that setup. However, oops != panic. They are 2
>> distinct events. Sometimes we panic without the oops under certain
>> situations. That is why it is desirable to have 2 distinct events.
>
> That's a circular statement:  They are distinct events, so it is desirable
> that they be distinct events?

I'm sorry. I should have described which events I was talking about.
They are 2 distinct events in the kernel, and we would look like them
that way in our non-volatile event log as well. As you are very much
aware panic() can be called from outside of the oops path; seeing a
panic doesn't imply an oops (likewise if panic on oops isn't set).

>
> Set a flag, a la oops_in_progress (unfortunately name, as it's called during
> a panic too).  Call it...  oops_really_in_progress.  Then will the panic
> notifier list suffice?

It would only appear to work as long as panic on oops isn't set. As I
mentioned in the previous email we do run with that on, but I think it
is desirable to keep the 2 paths distinct -- thus the separate
notifier chain.

That being said, we could probably add code to distinguish the
difference between the 4 states of oops_in_progress and
oops_really_in_progress and call the panic notifier list at
oops_enter(). I'm not sure why that would be more desirable than an
explicit notification mechanism for oops though.

As Mike talked about it might be more desirable to actually pass a
reason field that indicates the appropriate kernel event. However, the
naming of some of the variables becomes less clear. Maybe name it
panic_oops_notifier_list? That way there would only be a single
notifier chain.

-Aaron

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-25  3:17   ` Greg KH
@ 2011-01-25 23:12     ` Mike Waychison
  2011-01-26  2:46       ` Greg KH
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-25 23:12 UTC (permalink / raw)
  To: Greg KH
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Mon, Jan 24, 2011 at 7:17 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Jan 24, 2011 at 04:24:49PM -0800, Mike Waychison wrote:
>> +struct gsmi_ioctl {
>> +     uint16_t        length;         /* total length including data */
>> +     int             version;        /* structure version */
>> +     int             command;        /* ioctl command */
>
> Ick.  Use proper data types if you are going to create a new ioctl.
> Same goes for the structures above (hint, use __u32 and friends, not the
> unit??_t crap.
>
> I'd strongly suggest NOT creating a new ioctl though, that' just going
> to be a pain in the long run.

Ok.  I'll change these.  Even if the __u32 vs u32 vs uint32_t
differentiation is non-sense :(

>
>
>> +     union {
>> +             struct gsmi_get_nvram_var       get_nvram_var;
>> +             struct gsmi_get_next_var        get_next_var;
>> +             struct gsmi_set_nvram_var       set_nvram_var;
>> +             struct gsmi_set_event_log       set_event_log;
>> +             struct gsmi_clear_event_log     clear_event_log;
>> +     } gsmi_cmd;
>> +} __packed;
>> +
>> +#define GSMI_BASE                    'G'
>> +#define GSMI_IOCTL_VERSION           1
>> +#define GSMI_IOCTL                   _IOWR(GSMI_BASE, GSMI_IOCTL_VERSION, \
>> +                                         struct gsmi_ioctl)
>> +
>> +#endif /* _LINUX_GSMI_H */
>> diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
>> index 18fd130..34f5dfa 100644
>> --- a/include/linux/miscdevice.h
>> +++ b/include/linux/miscdevice.h
>> @@ -40,6 +40,7 @@
>>  #define BTRFS_MINOR          234
>>  #define AUTOFS_MINOR         235
>>  #define MAPPER_CTRL_MINOR    236
>> +#define GOOGLE_SMI_MINOR     242
>>  #define MISC_DYNAMIC_MINOR   255
>
> Why make this a static number and not just use a dynamic one?

Well, we use static device numbers, which is why it has historically
been static as well.  Yes, that puts us squarely in the 1990s :)

I can change this guy too though.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-25 23:12     ` Mike Waychison
@ 2011-01-26  2:46       ` Greg KH
  2011-01-26 23:58         ` Mike Waychison
  0 siblings, 1 reply; 36+ messages in thread
From: Greg KH @ 2011-01-26  2:46 UTC (permalink / raw)
  To: Mike Waychison
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Tue, Jan 25, 2011 at 03:12:52PM -0800, Mike Waychison wrote:
> On Mon, Jan 24, 2011 at 7:17 PM, Greg KH <greg@kroah.com> wrote:
> > On Mon, Jan 24, 2011 at 04:24:49PM -0800, Mike Waychison wrote:
> >> +struct gsmi_ioctl {
> >> +     uint16_t        length;         /* total length including data */
> >> +     int             version;        /* structure version */
> >> +     int             command;        /* ioctl command */
> >
> > Ick.  Use proper data types if you are going to create a new ioctl.
> > Same goes for the structures above (hint, use __u32 and friends, not the
> > unit??_t crap.
> >
> > I'd strongly suggest NOT creating a new ioctl though, that' just going
> > to be a pain in the long run.
> 
> Ok.  I'll change these.  Even if the __u32 vs u32 vs uint32_t
> differentiation is non-sense :(

Well, it really isn't nonsense.  __u32 and u32 can be different, and who
really knows what uint32_t means in the end (hint the _t things are for
userspace, not kernelspace, remember the issues of running 64bit kernels
with 32bit userspace programs.)

Actually, that reminds me, what are you going to do about 32/64bit
issues?  This is one reason why ioctls really really suck.  Not to
mention the fact that you really are just adding special syscalls to the
system here, which is another reason people hate them.

So, let me ask, what specifically are you wanting to import/export
to/from the kernel here?  Have you thought about other kernel/user apis
instead of ioctls?  What is forcing you to use ioctls?

> >> +     union {
> >> +             struct gsmi_get_nvram_var       get_nvram_var;
> >> +             struct gsmi_get_next_var        get_next_var;
> >> +             struct gsmi_set_nvram_var       set_nvram_var;
> >> +             struct gsmi_set_event_log       set_event_log;
> >> +             struct gsmi_clear_event_log     clear_event_log;
> >> +     } gsmi_cmd;
> >> +} __packed;
> >> +
> >> +#define GSMI_BASE                    'G'
> >> +#define GSMI_IOCTL_VERSION           1
> >> +#define GSMI_IOCTL                   _IOWR(GSMI_BASE, GSMI_IOCTL_VERSION, \
> >> +                                         struct gsmi_ioctl)
> >> +
> >> +#endif /* _LINUX_GSMI_H */
> >> diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
> >> index 18fd130..34f5dfa 100644
> >> --- a/include/linux/miscdevice.h
> >> +++ b/include/linux/miscdevice.h
> >> @@ -40,6 +40,7 @@
> >>  #define BTRFS_MINOR          234
> >>  #define AUTOFS_MINOR         235
> >>  #define MAPPER_CTRL_MINOR    236
> >> +#define GOOGLE_SMI_MINOR     242
> >>  #define MISC_DYNAMIC_MINOR   255
> >
> > Why make this a static number and not just use a dynamic one?
> 
> Well, we use static device numbers, which is why it has historically
> been static as well.  Yes, that puts us squarely in the 1990s :)
> 
> I can change this guy too though.

Yes please, we don't need to reserve any more numbers, as the 1990s was
a long time ago :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 0/6] google firmware support
  2011-01-25 19:58   ` Mike Waychison
@ 2011-01-26  2:47     ` Greg KH
  0 siblings, 0 replies; 36+ messages in thread
From: Greg KH @ 2011-01-26  2:47 UTC (permalink / raw)
  To: Mike Waychison
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Tue, Jan 25, 2011 at 11:58:27AM -0800, Mike Waychison wrote:
> On Mon, Jan 24, 2011 at 7:01 PM, Greg KH <greg@kroah.com> wrote:
> > On Mon, Jan 24, 2011 at 04:24:34PM -0800, Mike Waychison wrote:
> >>
> >>    - [5] prepares for [6] by introducing prepend_to_dmesg() which
> >>      allows drivers to prepend pre-kernel messages to the dmesg at
> >>      bootup.
> >
> > That seems "rude".  Why can't you just pick out from the kernel log the
> > data you need when it gets printed?  This seems like you feel your code
> > should always get a "First Post!" message.
> 
> Well, the use case we have is that we prepend all the bios output (and
> anything emitted by option roms and boot loaders via int 10h).  This
> typically runs in the 100+ lines of output.
> 
> Basically, everything the user sees on the console after power up is
> available to him in the dmesg.  We did this out of convenience as it
> makes for a good looking log :)

That's a nice goal, but I don't think the kernel log is the proper place
for this.

> That said, it's dmesg, and we don't build automation on anything in
> there if we can help it.  I can move this memory console dump
> elsewhere (/sys/firmware/log ?)

Yes, /sys/firmware/log would be a great place for it.  Make it a binary
sysfs file, and just dump the firmware data right into it, not having
the kernel do anything with it at all.

Oh, and document the thing in Documentation/ABI please, so that other
platforms can take advantage of this interface as well in the future.

nice idea,

greg k-h

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 1/6] Add oops notification chain.
  2011-01-25 22:21             ` Aaron Durbin
@ 2011-01-26  2:48               ` Greg KH
  2011-01-26 21:50                 ` Mike Waychison
  0 siblings, 1 reply; 36+ messages in thread
From: Greg KH @ 2011-01-26  2:48 UTC (permalink / raw)
  To: Aaron Durbin
  Cc: Jeff Garzik, Mike Waychison, torvalds, San Mehat, Duncan Laurie,
	linux-kernel, Tim Hockin

On Tue, Jan 25, 2011 at 02:21:03PM -0800, Aaron Durbin wrote:
> As Mike talked about it might be more desirable to actually pass a
> reason field that indicates the appropriate kernel event. However, the
> naming of some of the variables becomes less clear. Maybe name it
> panic_oops_notifier_list? That way there would only be a single
> notifier chain.

Yes, I think that's the better thing to do here instead of adding more
notifier chains.  It would just start to get even messier than it
currently is :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 1/6] Add oops notification chain.
  2011-01-26  2:48               ` Greg KH
@ 2011-01-26 21:50                 ` Mike Waychison
  0 siblings, 0 replies; 36+ messages in thread
From: Mike Waychison @ 2011-01-26 21:50 UTC (permalink / raw)
  To: Greg KH
  Cc: Aaron Durbin, Jeff Garzik, torvalds, San Mehat, Duncan Laurie,
	linux-kernel, Tim Hockin

On Tue, Jan 25, 2011 at 6:48 PM, Greg KH <greg@kroah.com> wrote:
> On Tue, Jan 25, 2011 at 02:21:03PM -0800, Aaron Durbin wrote:
>> As Mike talked about it might be more desirable to actually pass a
>> reason field that indicates the appropriate kernel event. However, the
>> naming of some of the variables becomes less clear. Maybe name it
>> panic_oops_notifier_list? That way there would only be a single
>> notifier chain.
>
> Yes, I think that's the better thing to do here instead of adding more
> notifier chains.  It would just start to get even messier than it
> currently is :)

Hmm.  I'm going to try and just re-use the notify_die() bits here
instead which seems to do what I want.  The overloading of "die" is a
bit of a mess, which is probably why I missed it the first time
around.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-26  2:46       ` Greg KH
@ 2011-01-26 23:58         ` Mike Waychison
  2011-01-27  1:22           ` Mike Waychison
                             ` (2 more replies)
  0 siblings, 3 replies; 36+ messages in thread
From: Mike Waychison @ 2011-01-26 23:58 UTC (permalink / raw)
  To: Greg KH
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Tue, Jan 25, 2011 at 6:46 PM, Greg KH <greg@kroah.com> wrote:
> On Tue, Jan 25, 2011 at 03:12:52PM -0800, Mike Waychison wrote:
>> On Mon, Jan 24, 2011 at 7:17 PM, Greg KH <greg@kroah.com> wrote:
>> > On Mon, Jan 24, 2011 at 04:24:49PM -0800, Mike Waychison wrote:
>> >> +struct gsmi_ioctl {
>> >> +     uint16_t        length;         /* total length including data */
>> >> +     int             version;        /* structure version */
>> >> +     int             command;        /* ioctl command */
>> >
>> > Ick.  Use proper data types if you are going to create a new ioctl.
>> > Same goes for the structures above (hint, use __u32 and friends, not the
>> > unit??_t crap.
>> >
>> > I'd strongly suggest NOT creating a new ioctl though, that' just going
>> > to be a pain in the long run.
>>
>> Ok.  I'll change these.  Even if the __u32 vs u32 vs uint32_t
>> differentiation is non-sense :(
>
> Well, it really isn't nonsense.  __u32 and u32 can be different, and who
> really knows what uint32_t means in the end.

The following is probably off-topic for this thread, but it makes for
an interesting conversation anyhow.   Other responses to your other
questions are further below.

<type-discussion>

It's pretty clear in the standard what uint32_t means.  I don't know
how you could come up with a different interpretation.  See C99
section 7.18.1.1 for a definition.

> (hint the _t things are for
> userspace, not kernelspace, remember the issues of running 64bit kernels
> with 32bit userspace programs.)

I don't see what you mean.   _t does not mean "for userspace".  It's
reserved in the POSIX.1 environment, but the kernel itself isn't
written to a POSIX.1 environment (except UML perhaps?), so that
doesn't really apply.

I understand that type widths change in a compat setting.  So what?
Code in each environment is written to it's own namespace anyhow.


Here's what *I* think *you* think about __u32 vs u32 vs uint32_t.
Correct me if I'm totally off-base here:

  - u32: It's short and sweet, and gets the job done.  Awesome to use
in kernel sources.
  - __u32: Exists because we can't export identifiers like u32 in
headers and expect that users can include them without namespace
collisions.  We revert to using a double-underscore identifier prefix
in those headers, because users should know better than to use an
identifier reserved for the implementation.
  - uint32_t: Newer than the rest of the kernel and too verbose in the
code.  _t looks ugly.  Not clear if it is safe to export to userland
as there isn't a good way to know if they are defined or knows if they
are defined or not.

I believe the above is non-sense because there is no concerted effort
to ever let userland include kernel headers in any meaningful way.
Including kernel headers in userland code breaks all the time and
users are discouraged from doing so for a variety of reasons
(incompatibilities with the system call wrappers exposed by their
libc, exposes unmaintained internal implementation details to userland
code, there are no guarantees there won't be namespace collisions,
etc...)

Linux kernel headers are just as broken when used by userland as when
a .S file tries to #include a random <asm/*.h> file.  If users
shouldn't be including headers, why should anyone ever bother to
differentiate between u32 and __u32 (or uint32_t)?

</type-discussion>

>
> Actually, that reminds me, what are you going to do about 32/64bit
> issues?  This is one reason why ioctls really really suck.

Sure, it isn't always obvious how to write ioctls properly when you
don't know what you are doing.  In this case, the structures are by
definition packed, use exact width fields and don't embed pointers.
I'm not committed to this interface, but it does do the job.

> Not to
> mention the fact that you really are just adding special syscalls to the
> system here, which is another reason people hate them.

Well, personally I like ioctls and system calls.  They don't bloat the
system with unneeded crud and abstractions that aren't very useful.
So what if you can't easily interface with them from a bare shell.
That's what userland utilities are for anyhow.

>
> So, let me ask, what specifically are you wanting to import/export
> to/from the kernel here?  Have you thought about other kernel/user apis
> instead of ioctls?  What is forcing you to use ioctls?

I'm not sure if you are trying to suggest that there is a better way
to solve these problems without actually saying so.  We could probably
use a different interface, sure.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-26 23:58         ` Mike Waychison
@ 2011-01-27  1:22           ` Mike Waychison
  2011-01-27 23:41             ` Mike Waychison
  2011-01-27 10:43           ` Alan Cox
  2011-01-28  2:59           ` Greg KH
  2 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-27  1:22 UTC (permalink / raw)
  To: Greg KH
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Wed, Jan 26, 2011 at 3:58 PM, Mike Waychison <mikew@google.com> wrote:
> On Tue, Jan 25, 2011 at 6:46 PM, Greg KH <greg@kroah.com> wrote:
>> So, let me ask, what specifically are you wanting to import/export
>> to/from the kernel here?  Have you thought about other kernel/user apis
>> instead of ioctls?  What is forcing you to use ioctls?
>
> I'm not sure if you are trying to suggest that there is a better way
> to solve these problems without actually saying so.  We could probably
> use a different interface, sure.
>

Well, I managed to find efivars.c, and it seems like I can probably
massage it to do what I want as several of the calls I'd like to
export to userland mirror portions of the EFI runtime services page.
That won't take care of all of the firmware calls I'd like to export,
but it's a start.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-26 23:58         ` Mike Waychison
  2011-01-27  1:22           ` Mike Waychison
@ 2011-01-27 10:43           ` Alan Cox
  2011-01-27 19:22             ` Mike Waychison
  2011-01-28  2:59           ` Greg KH
  2 siblings, 1 reply; 36+ messages in thread
From: Alan Cox @ 2011-01-27 10:43 UTC (permalink / raw)
  To: Mike Waychison
  Cc: Greg KH, torvalds, San Mehat, Aaron Durbin, Duncan Laurie,
	linux-kernel, Tim Hockin

> I understand that type widths change in a compat setting.  So what?
> Code in each environment is written to it's own namespace anyhow.

So you ned up with a pile of extra crap in the kernel to handle 32bit
userspace on 64bit and the like. If instead the structs are carefully
laid out this doesn't occur.

> Here's what *I* think *you* think about __u32 vs u32 vs uint32_t.
> Correct me if I'm totally off-base here:

And the __aligned_ ones for things like u64 in ioctls and structs...

It's also a style and consistency thing, its trivialto use the same style
as the rest of the kernel, it's trivial to tweak and it helps keep
coherency of style, even if you are sitting at the keyboard mumbling
"bunch of morons if you ask me" while doing it ;)

> I believe the above is non-sense because there is no concerted effort
> to ever let userland include kernel headers in any meaningful way.

Where do you think the library headers are distilled from ?

> Well, personally I like ioctls and system calls.  They don't bloat the
> system with unneeded crud and abstractions that aren't very useful.
> So what if you can't easily interface with them from a bare shell.
> That's what userland utilities are for anyhow.

ioctl has its place and I would agree with you on this one the interface
is essentially structure based so there isn't really that much you can do
about it anyway beyond getting the structures right.

(and the less the rest of the world has to know about EFI SMI internals
the better)

> I'm not sure if you are trying to suggest that there is a better way
> to solve these problems without actually saying so.  We could probably
> use a different interface, sure.

sysfs stuff only really helps for unsynchronized queries, ioctl is
read-modify-write which is a property sysfs lacks.

Alan

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-27 10:43           ` Alan Cox
@ 2011-01-27 19:22             ` Mike Waychison
  2011-01-28  2:55               ` Greg KH
  0 siblings, 1 reply; 36+ messages in thread
From: Mike Waychison @ 2011-01-27 19:22 UTC (permalink / raw)
  To: Alan Cox
  Cc: Greg KH, torvalds, San Mehat, Aaron Durbin, Duncan Laurie,
	linux-kernel, Tim Hockin

[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]

On Thu, Jan 27, 2011 at 2:43 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> I understand that type widths change in a compat setting.  So what?
>> Code in each environment is written to it's own namespace anyhow.
>
> So you ned up with a pile of extra crap in the kernel to handle 32bit
> userspace on 64bit and the like. If instead the structs are carefully
> laid out this doesn't occur.
>
>> Here's what *I* think *you* think about __u32 vs u32 vs uint32_t.
>> Correct me if I'm totally off-base here:
>
> And the __aligned_ ones for things like u64 in ioctls and structs...
>
> It's also a style and consistency thing, its trivialto use the same style
> as the rest of the kernel, it's trivial to tweak and it helps keep
> coherency of style, even if you are sitting at the keyboard mumbling
> "bunch of morons if you ask me" while doing it ;)

I'm always mumbling and cursing at my desk :p

I've already changed the series to not use C99 types.  I understand
the value in style consistency :)

I'm still trying to understand if there is any actual technical merit
to avoid standard types though, of which there doesn't seem to be any.

Anyhow, I've attached a patch that fixes the style documentation.

[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 788 bytes --]

CodingStyle: Fix documentation on use of C99 types.

Reality has it that new code is discouraged from using C99 exact width
types.  Fix the documentation.

Signed-off-by: Mike Waychison <mikew@google.com>
diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
index 8bb3723..8dd2e77 100644
--- a/Documentation/CodingStyle
+++ b/Documentation/CodingStyle
@@ -323,8 +323,7 @@ useful only for:
 
      Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
      signed equivalents which are identical to standard types are
-     permitted -- although they are not mandatory in new code of your
-     own.
+     preferred.
 
      When editing existing code which already uses one or the other set
      of types, you should conform to the existing choices in that code.

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-27  1:22           ` Mike Waychison
@ 2011-01-27 23:41             ` Mike Waychison
  2011-01-28  2:56               ` Greg KH
  2011-02-20  4:44               ` Matt Domsch
  0 siblings, 2 replies; 36+ messages in thread
From: Mike Waychison @ 2011-01-27 23:41 UTC (permalink / raw)
  To: Greg KH
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Wed, Jan 26, 2011 at 5:22 PM, Mike Waychison <mikew@google.com> wrote:
> On Wed, Jan 26, 2011 at 3:58 PM, Mike Waychison <mikew@google.com> wrote:
>> On Tue, Jan 25, 2011 at 6:46 PM, Greg KH <greg@kroah.com> wrote:
>>> So, let me ask, what specifically are you wanting to import/export
>>> to/from the kernel here?  Have you thought about other kernel/user apis
>>> instead of ioctls?  What is forcing you to use ioctls?
>>
>> I'm not sure if you are trying to suggest that there is a better way
>> to solve these problems without actually saying so.  We could probably
>> use a different interface, sure.
>>
>
> Well, I managed to find efivars.c, and it seems like I can probably
> massage it to do what I want as several of the calls I'd like to
> export to userland mirror portions of the EFI runtime services page.
> That won't take care of all of the firmware calls I'd like to export,
> but it's a start.
>

I've spent the last few hours looking at efivars.c and working out how
I can refactor it to reuse all the kobject bits it uses.  Does anybody
use this thing though?

I can't believe I was just lectured for crappy ABI when this thing
takes a binary packed struct on write() and process it:
   - without regard to write length, and
   - in a way that isn't compatible across compat (both DataSize and
Status are unsigned long!).

struct efi_variable {
        efi_char16_t  VariableName[1024/sizeof(efi_char16_t)];
        efi_guid_t    VendorGuid;
        unsigned long DataSize;
        __u8          Data[1024];
        efi_status_t  Status;
        __u32         Attributes;
} __attribute__((packed));

:(

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-27 19:22             ` Mike Waychison
@ 2011-01-28  2:55               ` Greg KH
  0 siblings, 0 replies; 36+ messages in thread
From: Greg KH @ 2011-01-28  2:55 UTC (permalink / raw)
  To: Mike Waychison
  Cc: Alan Cox, torvalds, San Mehat, Aaron Durbin, Duncan Laurie,
	linux-kernel, Tim Hockin

On Thu, Jan 27, 2011 at 11:22:55AM -0800, Mike Waychison wrote:
> On Thu, Jan 27, 2011 at 2:43 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >> I understand that type widths change in a compat setting.  So what?
> >> Code in each environment is written to it's own namespace anyhow.
> >
> > So you ned up with a pile of extra crap in the kernel to handle 32bit
> > userspace on 64bit and the like. If instead the structs are carefully
> > laid out this doesn't occur.
> >
> >> Here's what *I* think *you* think about __u32 vs u32 vs uint32_t.
> >> Correct me if I'm totally off-base here:
> >
> > And the __aligned_ ones for things like u64 in ioctls and structs...
> >
> > It's also a style and consistency thing, its trivialto use the same style
> > as the rest of the kernel, it's trivial to tweak and it helps keep
> > coherency of style, even if you are sitting at the keyboard mumbling
> > "bunch of morons if you ask me" while doing it ;)
> 
> I'm always mumbling and cursing at my desk :p
> 
> I've already changed the series to not use C99 types.  I understand
> the value in style consistency :)
> 
> I'm still trying to understand if there is any actual technical merit
> to avoid standard types though, of which there doesn't seem to be any.

Yes there is, it is subtle and prone to get wrong.  Linus wrote it all
up in detail a few years ago and I was going to document it in the
kernel tree somewhere but got busy.

You can dig through the lkml archives for the details but basically
always remember to use them in the following manner:
	u32 - in kernel usage
	__u32 - when the data crosses the kernel/user boundry
	uint32_t - never use

that will ensure you get your code right and no one will complain.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-27 23:41             ` Mike Waychison
@ 2011-01-28  2:56               ` Greg KH
  2011-02-20  4:44               ` Matt Domsch
  1 sibling, 0 replies; 36+ messages in thread
From: Greg KH @ 2011-01-28  2:56 UTC (permalink / raw)
  To: Mike Waychison
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Thu, Jan 27, 2011 at 03:41:06PM -0800, Mike Waychison wrote:
> On Wed, Jan 26, 2011 at 5:22 PM, Mike Waychison <mikew@google.com> wrote:
> > On Wed, Jan 26, 2011 at 3:58 PM, Mike Waychison <mikew@google.com> wrote:
> >> On Tue, Jan 25, 2011 at 6:46 PM, Greg KH <greg@kroah.com> wrote:
> >>> So, let me ask, what specifically are you wanting to import/export
> >>> to/from the kernel here?  Have you thought about other kernel/user apis
> >>> instead of ioctls?  What is forcing you to use ioctls?
> >>
> >> I'm not sure if you are trying to suggest that there is a better way
> >> to solve these problems without actually saying so.  We could probably
> >> use a different interface, sure.
> >>
> >
> > Well, I managed to find efivars.c, and it seems like I can probably
> > massage it to do what I want as several of the calls I'd like to
> > export to userland mirror portions of the EFI runtime services page.
> > That won't take care of all of the firmware calls I'd like to export,
> > but it's a start.
> >
> 
> I've spent the last few hours looking at efivars.c and working out how
> I can refactor it to reuse all the kobject bits it uses.  Does anybody
> use this thing though?
> 
> I can't believe I was just lectured for crappy ABI when this thing
> takes a binary packed struct on write() and process it:
>    - without regard to write length, and
>    - in a way that isn't compatible across compat (both DataSize and
> Status are unsigned long!).

It should just be passing that data straight to the hardware and not
trying to process the data at all, right?  If not, then it needs to be
fixed and not use the firmware binary interface...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-26 23:58         ` Mike Waychison
  2011-01-27  1:22           ` Mike Waychison
  2011-01-27 10:43           ` Alan Cox
@ 2011-01-28  2:59           ` Greg KH
  2 siblings, 0 replies; 36+ messages in thread
From: Greg KH @ 2011-01-28  2:59 UTC (permalink / raw)
  To: Mike Waychison
  Cc: torvalds, San Mehat, Aaron Durbin, Duncan Laurie, linux-kernel,
	Tim Hockin

On Wed, Jan 26, 2011 at 03:58:46PM -0800, Mike Waychison wrote:
> > Not to
> > mention the fact that you really are just adding special syscalls to the
> > system here, which is another reason people hate them.
> 
> Well, personally I like ioctls and system calls.  They don't bloat the
> system with unneeded crud and abstractions that aren't very useful.
> So what if you can't easily interface with them from a bare shell.
> That's what userland utilities are for anyhow.

Ok, but you need to document exactly what your ioctl is doing, much like
you need to document any new system calls.  That is my point, there
wasn't documentation with these new functions describing what they did
and how to use them.

Please see Documentation/ABI/ for how to add this type of information.
Yes, that's primarily used to describe sysfs files, but there's no
reason it can't describe ioctls as well.

> > So, let me ask, what specifically are you wanting to import/export
> > to/from the kernel here?  Have you thought about other kernel/user apis
> > instead of ioctls?  What is forcing you to use ioctls?
> 
> I'm not sure if you are trying to suggest that there is a better way
> to solve these problems without actually saying so.  We could probably
> use a different interface, sure.

I can't tell what you are trying to do here to determine what the best
type of interface is.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-01-27 23:41             ` Mike Waychison
  2011-01-28  2:56               ` Greg KH
@ 2011-02-20  4:44               ` Matt Domsch
  2011-02-21 13:58                 ` Matthew Garrett
  1 sibling, 1 reply; 36+ messages in thread
From: Matt Domsch @ 2011-02-20  4:44 UTC (permalink / raw)
  To: Mike Waychison
  Cc: Greg KH, torvalds, San Mehat, Aaron Durbin, Duncan Laurie,
	linux-kernel, Tim Hockin

On Thu, Jan 27, 2011 at 03:41:06PM -0800, Mike Waychison wrote:
> On Wed, Jan 26, 2011 at 5:22 PM, Mike Waychison <mikew@google.com> wrote:
> 
> I've spent the last few hours looking at efivars.c and working out how
> I can refactor it to reuse all the kobject bits it uses.  Does anybody
> use this thing though?
> 
> I can't believe I was just lectured for crappy ABI when this thing
> takes a binary packed struct on write() and process it:
>    - without regard to write length, and
>    - in a way that isn't compatible across compat (both DataSize and
> Status are unsigned long!).
> 
> struct efi_variable {
>         efi_char16_t  VariableName[1024/sizeof(efi_char16_t)];
>         efi_guid_t    VendorGuid;
>         unsigned long DataSize;
>         __u8          Data[1024];
>         efi_status_t  Status;
>         __u32         Attributes;
> } __attribute__((packed));
> 
> :(

I wrote that long before anyone believed there could be a 32-bit EFI
or 32-bit kernel running on a 64-bit EFI.  Remember, this originated
on Itanium.  I knew it was ugly, but the EFI spec itself defined the
DataSize and Status as they did.

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 3/6] driver: Google EFI SMI
  2011-02-20  4:44               ` Matt Domsch
@ 2011-02-21 13:58                 ` Matthew Garrett
  0 siblings, 0 replies; 36+ messages in thread
From: Matthew Garrett @ 2011-02-21 13:58 UTC (permalink / raw)
  To: Matt Domsch
  Cc: Mike Waychison, Greg KH, torvalds, San Mehat, Aaron Durbin,
	Duncan Laurie, linux-kernel, Tim Hockin

On Sat, Feb 19, 2011 at 10:44:40PM -0600, Matt Domsch wrote:

> I wrote that long before anyone believed there could be a 32-bit EFI
> or 32-bit kernel running on a 64-bit EFI.  Remember, this originated
> on Itanium.  I knew it was ugly, but the EFI spec itself defined the
> DataSize and Status as they did.

How can a 32-bit kernel run on 64-bit EFI? I've seen vendors who keep 
their EFI code above 4GB.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2011-02-21 13:59 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-25  0:24 [PATCH v1 0/6] google firmware support Mike Waychison
2011-01-25  0:24 ` [PATCH v1 1/6] Add oops notification chain Mike Waychison
2011-01-25  2:06   ` Greg KH
2011-01-25 20:01     ` Mike Waychison
2011-01-25 21:36       ` Jeff Garzik
2011-01-25 21:43         ` Aaron Durbin
2011-01-25 21:54           ` Jeff Garzik
2011-01-25 22:21             ` Aaron Durbin
2011-01-26  2:48               ` Greg KH
2011-01-26 21:50                 ` Mike Waychison
2011-01-25  0:24 ` [PATCH v1 2/6] Introduce CONFIG_GOOGLE_FIRMWARE Mike Waychison
2011-01-25  0:24 ` [PATCH v1 3/6] driver: Google EFI SMI Mike Waychison
2011-01-25  3:17   ` Greg KH
2011-01-25 23:12     ` Mike Waychison
2011-01-26  2:46       ` Greg KH
2011-01-26 23:58         ` Mike Waychison
2011-01-27  1:22           ` Mike Waychison
2011-01-27 23:41             ` Mike Waychison
2011-01-28  2:56               ` Greg KH
2011-02-20  4:44               ` Matt Domsch
2011-02-21 13:58                 ` Matthew Garrett
2011-01-27 10:43           ` Alan Cox
2011-01-27 19:22             ` Mike Waychison
2011-01-28  2:55               ` Greg KH
2011-01-28  2:59           ` Greg KH
2011-01-25  0:24 ` [PATCH v1 4/6] driver: Google Bootlog Mike Waychison
2011-01-25  0:49   ` Alan Cox
2011-01-25  1:38     ` Mike Waychison
2011-01-25  9:43       ` Alan Cox
2011-01-25  0:25 ` [PATCH v1 5/6] Allow prepending to the dmesg Mike Waychison
2011-01-25  1:01   ` Andrew Morton
2011-01-25  0:25 ` [PATCH v1 6/6] driver: Google Memory Console Mike Waychison
2011-01-25  2:00   ` Greg KH
2011-01-25  3:01 ` [PATCH v1 0/6] google firmware support Greg KH
2011-01-25 19:58   ` Mike Waychison
2011-01-26  2:47     ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.