[PATCH 0/5] firmware: google memconsole

[PATCH 0/5] firmware: google memconsole

[Thierry Escande]

Hi,

This series extends the Google memory console driver to work on x86 and
ARM platforms running coreboot.

The legacy x86 memconsole driver is now named memconsole-x86-legacy and
a new driver named memconsole-coreboot is added.

On coreboot devices and depending on the device architecture, the driver
coreboot_table-acpi on x86 platforms or coreboot_table-of on ARM/ARM64
platforms is responsible to get the coreboot table header memory address
and then obtain the memory console address.

Regards,
 Thierry

Julius Werner (1):
  firmware: Add coreboot device tree binding documentation

Thierry Escande (4):
  firmware: google memconsole: Remove useless submenu in Kconfig
  firmware: google memconsole: Move specific EBDA parts
  firmware: google memconsole: Add coreboot support
  firmware: google memconsole: Add ARM/ARM64 support

 .../devicetree/bindings/firmware/coreboot.txt      |  33 +++++
 drivers/firmware/google/Kconfig                    |  50 ++++++-
 drivers/firmware/google/Makefile                   |   7 +-
 drivers/firmware/google/coreboot_table-acpi.c      |  88 ++++++++++++
 drivers/firmware/google/coreboot_table-of.c        |  82 +++++++++++
 drivers/firmware/google/coreboot_table.c           |  94 +++++++++++++
 drivers/firmware/google/coreboot_table.h           |  50 +++++++
 drivers/firmware/google/memconsole-coreboot.c      | 109 +++++++++++++++
 drivers/firmware/google/memconsole-x86-legacy.c    | 154 ++++++++++++++++++++
 drivers/firmware/google/memconsole.c               | 155 +++------------------
 drivers/firmware/google/memconsole.h               |  43 ++++++
 11 files changed, 725 insertions(+), 140 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/firmware/coreboot.txt
 create mode 100644 drivers/firmware/google/coreboot_table-acpi.c
 create mode 100644 drivers/firmware/google/coreboot_table-of.c
 create mode 100644 drivers/firmware/google/coreboot_table.c
 create mode 100644 drivers/firmware/google/coreboot_table.h
 create mode 100644 drivers/firmware/google/memconsole-coreboot.c
 create mode 100644 drivers/firmware/google/memconsole-x86-legacy.c
 create mode 100644 drivers/firmware/google/memconsole.h

-- 
2.7.4

[PATCH 1/5] firmware: google memconsole: Remove useless submenu in Kconfig

[Thierry Escande]

This patch removes the "Google Firmware Drivers" menu containing a
menuconfig entry with the exact same name. The menuconfig is now
directly under the "Firmware Drivers" entry.

Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/firmware/google/Kconfig | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 29c8cdd..475bd01 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -1,4 +1,4 @@
-config GOOGLE_FIRMWARE
+menuconfig GOOGLE_FIRMWARE
 	bool "Google Firmware Drivers"
 	depends on X86
 	default n
@@ -7,8 +7,7 @@ config GOOGLE_FIRMWARE
 	  only useful if you are working directly on one of their
 	  proprietary servers.  If in doubt, say "N".
 
-menu "Google Firmware Drivers"
-	depends on GOOGLE_FIRMWARE
+if GOOGLE_FIRMWARE
 
 config GOOGLE_SMI
 	tristate "SMI interface for Google platforms"
@@ -28,4 +27,4 @@ config GOOGLE_MEMCONSOLE
 	  the EBDA on Google servers.  If found, this log is exported to
 	  userland in the file /sys/firmware/log.
 
-endmenu
+endif # GOOGLE_FIRMWARE
-- 
2.7.4

[PATCH 2/5] firmware: google memconsole: Move specific EBDA parts

[Thierry Escande]

This patch splits memconsole.c in 2 parts. One containing the
architecture-independent part and the other one containing the EBDA
specific part. This prepares the integration of coreboot support for the
memconsole.

The memconsole driver is now named as memconsole-x86-legacy.

Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/firmware/google/Kconfig                 |   9 +-
 drivers/firmware/google/Makefile                |   3 +-
 drivers/firmware/google/memconsole-x86-legacy.c | 154 +++++++++++++++++++++++
 drivers/firmware/google/memconsole.c            | 155 ++++--------------------
 drivers/firmware/google/memconsole.h            |  43 +++++++
 5 files changed, 229 insertions(+), 135 deletions(-)
 create mode 100644 drivers/firmware/google/memconsole-x86-legacy.c
 create mode 100644 drivers/firmware/google/memconsole.h

diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 475bd01..27a0658 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -20,8 +20,13 @@ config GOOGLE_SMI
 	  variables.
 
 config GOOGLE_MEMCONSOLE
-	tristate "Firmware Memory Console"
-	depends on DMI
+	tristate
+	depends on GOOGLE_MEMCONSOLE_X86_LEGACY
+
+config GOOGLE_MEMCONSOLE_X86_LEGACY
+	tristate "Firmware Memory Console - X86 Legacy support"
+	depends on X86 && ACPI && DMI
+	select GOOGLE_MEMCONSOLE
 	help
 	  This option enables the kernel to search for a firmware log in
 	  the EBDA on Google servers.  If found, this log is exported to
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
index 54a294e..10683ef 100644
--- a/drivers/firmware/google/Makefile
+++ b/drivers/firmware/google/Makefile
@@ -1,3 +1,4 @@
 
 obj-$(CONFIG_GOOGLE_SMI)		+= gsmi.o
-obj-$(CONFIG_GOOGLE_MEMCONSOLE)		+= memconsole.o
+obj-$(CONFIG_GOOGLE_MEMCONSOLE)            += memconsole.o
+obj-$(CONFIG_GOOGLE_MEMCONSOLE_X86_LEGACY) += memconsole-x86-legacy.o
diff --git a/drivers/firmware/google/memconsole-x86-legacy.c b/drivers/firmware/google/memconsole-x86-legacy.c
new file mode 100644
index 0000000..529078c
--- /dev/null
+++ b/drivers/firmware/google/memconsole-x86-legacy.c
@@ -0,0 +1,154 @@
+/*
+ * memconsole-x86-legacy.c
+ *
+ * EBDA specific parts of the memory based BIOS console.
+ *
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/dmi.h>
+#include <linux/mm.h>
+#include <asm/bios_ebda.h>
+#include <asm/e820.h>
+#include <linux/acpi.h>
+
+#include "memconsole.h"
+
+#define BIOS_MEMCONSOLE_V1_MAGIC	0xDEADBABE
+#define BIOS_MEMCONSOLE_V2_MAGIC	(('M')|('C'<<8)|('O'<<16)|('N'<<24))
+
+struct biosmemcon_ebda {
+	u32 signature;
+	union {
+		struct {
+			u8  enabled;
+			u32 buffer_addr;
+			u16 start;
+			u16 end;
+			u16 num_chars;
+			u8  wrapped;
+		} __packed v1;
+		struct {
+			u32 buffer_addr;
+			/* Misdocumented as number of pages! */
+			u16 num_bytes;
+			u16 start;
+			u16 end;
+		} __packed v2;
+	};
+} __packed;
+
+static void found_v1_header(struct biosmemcon_ebda *hdr)
+{
+	pr_info("memconsole: BIOS console v1 EBDA structure found at %p\n",
+		hdr);
+	pr_info("memconsole: BIOS console buffer at 0x%.8x, start = %d, end = %d, num = %d\n",
+		hdr->v1.buffer_addr, hdr->v1.start,
+		hdr->v1.end, hdr->v1.num_chars);
+
+	memconsole_setup(phys_to_virt(hdr->v1.buffer_addr), hdr->v1.num_chars);
+}
+
+static void found_v2_header(struct biosmemcon_ebda *hdr)
+{
+	pr_info("memconsole: BIOS console v2 EBDA structure found at %p\n",
+		hdr);
+	pr_info("memconsole: BIOS console buffer at 0x%.8x, start = %d, end = %d, num_bytes = %d\n",
+		hdr->v2.buffer_addr, hdr->v2.start,
+		hdr->v2.end, hdr->v2.num_bytes);
+
+	memconsole_setup(phys_to_virt(hdr->v2.buffer_addr + hdr->v2.start),
+			 hdr->v2.end - hdr->v2.start);
+}
+
+/*
+ * Search through the EBDA for the BIOS Memory Console, and
+ * set the global variables to point to it.  Return true if found.
+ */
+static bool memconsole_ebda_init(void)
+{
+	unsigned int address;
+	size_t length, cur;
+
+	address = get_bios_ebda();
+	if (!address) {
+		pr_info("memconsole: BIOS EBDA non-existent.\n");
+		return false;
+	}
+
+	/* EBDA length is byte 0 of EBDA (in KB) */
+	length = *(u8 *)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++) {
+		struct biosmemcon_ebda *hdr = phys_to_virt(address + cur);
+
+		/* memconsole v1 */
+		if (hdr->signature == BIOS_MEMCONSOLE_V1_MAGIC) {
+			found_v1_header(hdr);
+			return true;
+		}
+
+		/* memconsole v2 */
+		if (hdr->signature == BIOS_MEMCONSOLE_V2_MAGIC) {
+			found_v2_header(hdr);
+			return true;
+		}
+	}
+
+	pr_info("memconsole: BIOS console EBDA structure not found!\n");
+	return false;
+}
+
+static struct dmi_system_id memconsole_dmi_table[] __initdata = {
+	{
+		.ident = "Google Board",
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "Google, Inc."),
+		},
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(dmi, memconsole_dmi_table);
+
+static bool __init memconsole_find(void)
+{
+	if (!dmi_check_system(memconsole_dmi_table))
+		return false;
+
+	return memconsole_ebda_init();
+}
+
+static int __init memconsole_x86_init(void)
+{
+	if (!memconsole_find())
+		return -ENODEV;
+
+	return memconsole_sysfs_init();
+}
+
+static void __exit memconsole_x86_exit(void)
+{
+	memconsole_exit();
+}
+
+module_init(memconsole_x86_init);
+module_exit(memconsole_x86_exit);
+
+MODULE_AUTHOR("Google, Inc.");
+MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/google/memconsole.c b/drivers/firmware/google/memconsole.c
index 2f569aa..94e200d 100644
--- a/drivers/firmware/google/memconsole.c
+++ b/drivers/firmware/google/memconsole.c
@@ -1,66 +1,36 @@
 /*
  * memconsole.c
  *
- * Infrastructure for importing the BIOS memory based console
- * into the kernel log ringbuffer.
+ * Architecture-independent parts of the memory based BIOS console.
  *
- * Copyright 2010 Google Inc. All rights reserved.
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
  */
 
-#include <linux/ctype.h>
 #include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/string.h>
 #include <linux/sysfs.h>
 #include <linux/kobject.h>
 #include <linux/module.h>
-#include <linux/dmi.h>
-#include <linux/io.h>
-#include <asm/bios_ebda.h>
 
-#define BIOS_MEMCONSOLE_V1_MAGIC	0xDEADBABE
-#define BIOS_MEMCONSOLE_V2_MAGIC	(('M')|('C'<<8)|('O'<<16)|('N'<<24))
+#include "memconsole.h"
 
-struct biosmemcon_ebda {
-	u32 signature;
-	union {
-		struct {
-			u8  enabled;
-			u32 buffer_addr;
-			u16 start;
-			u16 end;
-			u16 num_chars;
-			u8  wrapped;
-		} __packed v1;
-		struct {
-			u32 buffer_addr;
-			/* Misdocumented as number of pages! */
-			u16 num_bytes;
-			u16 start;
-			u16 end;
-		} __packed v2;
-	};
-} __packed;
-
-static u32 memconsole_baseaddr;
+static char *memconsole_baseaddr;
 static size_t memconsole_length;
 
 static ssize_t memconsole_read(struct file *filp, struct kobject *kobp,
 			       struct bin_attribute *bin_attr, char *buf,
 			       loff_t pos, size_t count)
 {
-	char *memconsole;
-	ssize_t ret;
-
-	memconsole = ioremap_cache(memconsole_baseaddr, memconsole_length);
-	if (!memconsole) {
-		pr_err("memconsole: ioremap_cache failed\n");
-		return -ENOMEM;
-	}
-	ret = memory_read_from_buffer(buf, count, &pos, memconsole,
-				      memconsole_length);
-	iounmap(memconsole);
-	return ret;
+	return memory_read_from_buffer(buf, count, &pos, memconsole_baseaddr,
+				       memconsole_length);
 }
 
 static struct bin_attribute memconsole_bin_attr = {
@@ -68,104 +38,25 @@ static struct bin_attribute memconsole_bin_attr = {
 	.read = memconsole_read,
 };
 
-
-static void __init found_v1_header(struct biosmemcon_ebda *hdr)
-{
-	pr_info("BIOS console v1 EBDA structure found at %p\n", hdr);
-	pr_info("BIOS console buffer at 0x%.8x, "
-	       "start = %d, end = %d, num = %d\n",
-	       hdr->v1.buffer_addr, hdr->v1.start,
-	       hdr->v1.end, hdr->v1.num_chars);
-
-	memconsole_length = hdr->v1.num_chars;
-	memconsole_baseaddr = hdr->v1.buffer_addr;
-}
-
-static void __init found_v2_header(struct biosmemcon_ebda *hdr)
-{
-	pr_info("BIOS console v2 EBDA structure found at %p\n", hdr);
-	pr_info("BIOS console buffer at 0x%.8x, "
-	       "start = %d, end = %d, num_bytes = %d\n",
-	       hdr->v2.buffer_addr, hdr->v2.start,
-	       hdr->v2.end, hdr->v2.num_bytes);
-
-	memconsole_length = hdr->v2.end - hdr->v2.start;
-	memconsole_baseaddr = hdr->v2.buffer_addr + hdr->v2.start;
-}
-
-/*
- * Search through the EBDA for the BIOS Memory Console, and
- * set the global variables to point to it.  Return true if found.
- */
-static bool __init found_memconsole(void)
+void memconsole_setup(void *baseaddr, size_t length)
 {
-	unsigned int address;
-	size_t length, cur;
-
-	address = get_bios_ebda();
-	if (!address) {
-		pr_info("BIOS EBDA non-existent.\n");
-		return false;
-	}
-
-	/* EBDA length is byte 0 of EBDA (in KB) */
-	length = *(u8 *)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++) {
-		struct biosmemcon_ebda *hdr = phys_to_virt(address + cur);
-
-		/* memconsole v1 */
-		if (hdr->signature == BIOS_MEMCONSOLE_V1_MAGIC) {
-			found_v1_header(hdr);
-			return true;
-		}
-
-		/* memconsole v2 */
-		if (hdr->signature == BIOS_MEMCONSOLE_V2_MAGIC) {
-			found_v2_header(hdr);
-			return true;
-		}
-	}
-
-	pr_info("BIOS console EBDA structure not found!\n");
-	return false;
+	memconsole_baseaddr = baseaddr;
+	memconsole_length = length;
 }
+EXPORT_SYMBOL(memconsole_setup);
 
-static struct dmi_system_id memconsole_dmi_table[] __initdata = {
-	{
-		.ident = "Google Board",
-		.matches = {
-			DMI_MATCH(DMI_BOARD_VENDOR, "Google, Inc."),
-		},
-	},
-	{}
-};
-MODULE_DEVICE_TABLE(dmi, memconsole_dmi_table);
-
-static int __init memconsole_init(void)
+int memconsole_sysfs_init(void)
 {
-	if (!dmi_check_system(memconsole_dmi_table))
-		return -ENODEV;
-
-	if (!found_memconsole())
-		return -ENODEV;
-
 	memconsole_bin_attr.size = memconsole_length;
 	return sysfs_create_bin_file(firmware_kobj, &memconsole_bin_attr);
 }
+EXPORT_SYMBOL(memconsole_sysfs_init);
 
-static void __exit memconsole_exit(void)
+void memconsole_exit(void)
 {
 	sysfs_remove_bin_file(firmware_kobj, &memconsole_bin_attr);
 }
-
-module_init(memconsole_init);
-module_exit(memconsole_exit);
+EXPORT_SYMBOL(memconsole_exit);
 
 MODULE_AUTHOR("Google, Inc.");
 MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/google/memconsole.h b/drivers/firmware/google/memconsole.h
new file mode 100644
index 0000000..190fc03
--- /dev/null
+++ b/drivers/firmware/google/memconsole.h
@@ -0,0 +1,43 @@
+/*
+ * memconsole.h
+ *
+ * Internal headers of the memory based BIOS console.
+ *
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __FIRMWARE_GOOGLE_MEMCONSOLE_H
+#define __FIRMWARE_GOOGLE_MEMCONSOLE_H
+
+/*
+ * memconsole_setup
+ *
+ * Initialize the memory console from raw (virtual) base
+ * address and length.
+ */
+void memconsole_setup(void *baseaddr, size_t length);
+
+/*
+ * memconsole_sysfs_init
+ *
+ * Update memory console length and create binary file
+ * for firmware object.
+ */
+int memconsole_sysfs_init(void);
+
+/* memconsole_exit
+ *
+ * Unmap the console buffer.
+ */
+void memconsole_exit(void);
+
+#endif /* __FIRMWARE_GOOGLE_MEMCONSOLE_H */
-- 
2.7.4

[PATCH 3/5] firmware: google memconsole: Add coreboot support

[Thierry Escande]

Coreboot (http://www.coreboot.org) allows to save the firmware console
output in a memory buffer. With this patch, the address of this memory
buffer is obtained from coreboot tables on x86 chromebook devices
declaring an ACPI device with name matching GOOGCB00 or BOOT0000.

If the memconsole-coreboot driver is able to find the coreboot table,
the memconsole driver sets the cbmem_console address and initializes the
memconsole sysfs entries.

The coreboot_table-acpi driver is responsible for setting the address of
the coreboot table header when probed. If this address is not yet set
when memconsole-coreboot is probed, then the probe is deferred by
returning -EPROBE_DEFER.

This patch is a rework/split/merge of patches from the chromeos v4.4
kernel tree originally authored by:
 Vadim Bendebury <vbendeb@chromium.org>
 Wei-Ning Huang <wnhuang@google.com>
 Yuji Sasaki <sasakiy@google.com>
 Duncan Laurie <dlaurie@chromium.org>
 Julius Werner <jwerner@chromium.org>
 Brian Norris <briannorris@chromium.org>

Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/firmware/google/Kconfig               |  25 +++++-
 drivers/firmware/google/Makefile              |   3 +
 drivers/firmware/google/coreboot_table-acpi.c |  88 +++++++++++++++++++++
 drivers/firmware/google/coreboot_table.c      |  94 ++++++++++++++++++++++
 drivers/firmware/google/coreboot_table.h      |  50 ++++++++++++
 drivers/firmware/google/memconsole-coreboot.c | 109 ++++++++++++++++++++++++++
 6 files changed, 368 insertions(+), 1 deletion(-)
 create mode 100644 drivers/firmware/google/coreboot_table-acpi.c
 create mode 100644 drivers/firmware/google/coreboot_table.c
 create mode 100644 drivers/firmware/google/coreboot_table.h
 create mode 100644 drivers/firmware/google/memconsole-coreboot.c

diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 27a0658..840bd62 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -19,9 +19,23 @@ config GOOGLE_SMI
 	  clearing the EFI event log and reading and writing NVRAM
 	  variables.
 
+config GOOGLE_COREBOOT_TABLE
+	tristate
+	depends on GOOGLE_COREBOOT_TABLE_ACPI
+
+config GOOGLE_COREBOOT_TABLE_ACPI
+	tristate "Coreboot Table Access - ACPI"
+	depends on ACPI
+	select GOOGLE_COREBOOT_TABLE
+	help
+	  This option enables the coreboot_table module, which provides other
+	  firmware modules to access to the coreboot table. The coreboot table
+	  pointer is accessed through the ACPI "GOOGCB00" object.
+	  If unsure say N.
+
 config GOOGLE_MEMCONSOLE
 	tristate
-	depends on GOOGLE_MEMCONSOLE_X86_LEGACY
+	depends on GOOGLE_MEMCONSOLE_X86_LEGACY || GOOGLE_MEMCONSOLE_COREBOOT
 
 config GOOGLE_MEMCONSOLE_X86_LEGACY
 	tristate "Firmware Memory Console - X86 Legacy support"
@@ -32,4 +46,13 @@ config GOOGLE_MEMCONSOLE_X86_LEGACY
 	  the EBDA on Google servers.  If found, this log is exported to
 	  userland in the file /sys/firmware/log.
 
+config GOOGLE_MEMCONSOLE_COREBOOT
+	tristate "Firmware Memory Console"
+	depends on GOOGLE_COREBOOT_TABLE
+	select GOOGLE_MEMCONSOLE
+	help
+	  This option enables the kernel to search for a firmware log in
+	  the coreboot table.  If found, this log is exported to userland
+	  in the file /sys/firmware/log.
+
 endif # GOOGLE_FIRMWARE
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
index 10683ef..662a83e 100644
--- a/drivers/firmware/google/Makefile
+++ b/drivers/firmware/google/Makefile
@@ -1,4 +1,7 @@
 
 obj-$(CONFIG_GOOGLE_SMI)		+= gsmi.o
+obj-$(CONFIG_GOOGLE_COREBOOT_TABLE)        += coreboot_table.o
+obj-$(CONFIG_GOOGLE_COREBOOT_TABLE_ACPI)   += coreboot_table-acpi.o
 obj-$(CONFIG_GOOGLE_MEMCONSOLE)            += memconsole.o
+obj-$(CONFIG_GOOGLE_MEMCONSOLE_COREBOOT)   += memconsole-coreboot.o
 obj-$(CONFIG_GOOGLE_MEMCONSOLE_X86_LEGACY) += memconsole-x86-legacy.o
diff --git a/drivers/firmware/google/coreboot_table-acpi.c b/drivers/firmware/google/coreboot_table-acpi.c
new file mode 100644
index 0000000..fb98db2
--- /dev/null
+++ b/drivers/firmware/google/coreboot_table-acpi.c
@@ -0,0 +1,88 @@
+/*
+ * coreboot_table-acpi.c
+ *
+ * Using ACPI to locate Coreboot table and provide coreboot table access.
+ *
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include "coreboot_table.h"
+
+static int coreboot_table_acpi_probe(struct platform_device *pdev)
+{
+	phys_addr_t phyaddr;
+	resource_size_t len;
+	struct coreboot_table_header __iomem *header = NULL;
+	struct resource *res;
+	void __iomem *ptr = NULL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+
+	len = resource_size(res);
+	if (!res->start || !len)
+		return -EINVAL;
+
+	phyaddr = res->start;
+	header = ioremap_cache(phyaddr, sizeof(*header));
+	if (header == NULL)
+		return -ENOMEM;
+
+	ptr = ioremap_cache(phyaddr,
+			    header->header_bytes + header->table_bytes);
+	iounmap(header);
+	if (!ptr)
+		return -ENOMEM;
+
+	return coreboot_table_init(ptr);
+}
+
+static int coreboot_table_acpi_remove(struct platform_device *pdev)
+{
+	return coreboot_table_exit();
+}
+
+static const struct acpi_device_id cros_coreboot_acpi_match[] = {
+	{ "GOOGCB00", 0 },
+	{ "BOOT0000", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, cros_coreboot_acpi_match);
+
+static struct platform_driver coreboot_table_acpi_driver = {
+	.probe = coreboot_table_acpi_probe,
+	.remove = coreboot_table_acpi_remove,
+	.driver = {
+		.name = "coreboot_table_acpi",
+		.acpi_match_table = ACPI_PTR(cros_coreboot_acpi_match),
+	},
+};
+
+static int __init coreboot_table_acpi_init(void)
+{
+	return platform_driver_register(&coreboot_table_acpi_driver);
+}
+
+module_init(coreboot_table_acpi_init);
+
+MODULE_AUTHOR("Google, Inc.");
+MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
new file mode 100644
index 0000000..0019d3e
--- /dev/null
+++ b/drivers/firmware/google/coreboot_table.c
@@ -0,0 +1,94 @@
+/*
+ * coreboot_table.c
+ *
+ * Module providing coreboot table access.
+ *
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "coreboot_table.h"
+
+struct coreboot_table_entry {
+	u32 tag;
+	u32 size;
+};
+
+static struct coreboot_table_header __iomem *ptr_header;
+
+/*
+ * This function parses the coreboot table for an entry that contains the base
+ * address of the given entry tag. The coreboot table consists of a header
+ * directly followed by a number of small, variable-sized entries, which each
+ * contain an identifying tag and their length as the first two fields.
+ */
+int coreboot_table_find(int tag, void *data, size_t data_size)
+{
+	struct coreboot_table_header header;
+	struct coreboot_table_entry entry;
+	void *ptr_entry;
+	int i;
+
+	if (!ptr_header)
+		return -EPROBE_DEFER;
+
+	memcpy_fromio(&header, ptr_header, sizeof(header));
+
+	if (strncmp(header.signature, "LBIO", sizeof(header.signature))) {
+		pr_warn("coreboot_table: coreboot table missing or corrupt!\n");
+		return -ENODEV;
+	}
+
+	ptr_entry = (void *)ptr_header + header.header_bytes;
+
+	for (i = 0; i < header.table_entries; i++) {
+		memcpy_fromio(&entry, ptr_entry, sizeof(entry));
+		if (entry.tag == tag) {
+			if (data_size < entry.size)
+				return -EINVAL;
+
+			memcpy_fromio(data, ptr_entry, entry.size);
+
+			return 0;
+		}
+
+		ptr_entry += entry.size;
+	}
+
+	return -ENOENT;
+}
+EXPORT_SYMBOL(coreboot_table_find);
+
+int coreboot_table_init(void __iomem *ptr)
+{
+	ptr_header = ptr;
+
+	return 0;
+}
+EXPORT_SYMBOL(coreboot_table_init);
+
+int coreboot_table_exit(void)
+{
+	if (ptr_header)
+		iounmap(ptr_header);
+
+	return 0;
+}
+EXPORT_SYMBOL(coreboot_table_exit);
+
+MODULE_AUTHOR("Google, Inc.");
+MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
new file mode 100644
index 0000000..6eff1ae
--- /dev/null
+++ b/drivers/firmware/google/coreboot_table.h
@@ -0,0 +1,50 @@
+/*
+ * coreboot_table.h
+ *
+ * Internal header for coreboot table access.
+ *
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __COREBOOT_TABLE_H
+#define __COREBOOT_TABLE_H
+
+#include <linux/io.h>
+
+/* List of coreboot entry structures that is used */
+struct lb_cbmem_ref {
+	uint32_t tag;
+	uint32_t size;
+
+	uint64_t cbmem_addr;
+};
+
+/* Coreboot table header structure */
+struct coreboot_table_header {
+	char signature[4];
+	u32 header_bytes;
+	u32 header_checksum;
+	u32 table_bytes;
+	u32 table_checksum;
+	u32 table_entries;
+};
+
+/* Retrieve coreboot table entry with tag *tag* and copy it to data */
+int coreboot_table_find(int tag, void *data, size_t data_size);
+
+/* Initialize coreboot table module given a pointer to iomem */
+int coreboot_table_init(void __iomem *ptr);
+
+/* Cleanup coreboot table module */
+int coreboot_table_exit(void);
+
+#endif /* __COREBOOT_TABLE_H */
diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/google/memconsole-coreboot.c
new file mode 100644
index 0000000..2121014
--- /dev/null
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -0,0 +1,109 @@
+/*
+ * memconsole-coreboot.c
+ *
+ * Memory based BIOS console accessed through coreboot table.
+ *
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include "memconsole.h"
+#include "coreboot_table.h"
+
+#define CB_TAG_CBMEM_CONSOLE	0x17
+
+/* CBMEM firmware console log descriptor. */
+struct cbmem_cons {
+	u32 buffer_size;
+	u32 buffer_cursor;
+	u8  buffer_body[0];
+} __packed;
+
+static struct cbmem_cons __iomem *cbmem_console;
+
+static int memconsole_coreboot_init(phys_addr_t physaddr)
+{
+	struct cbmem_cons __iomem *tmp_cbmc;
+
+	tmp_cbmc = memremap(physaddr, sizeof(*tmp_cbmc), MEMREMAP_WB);
+
+	if (!tmp_cbmc)
+		return -ENOMEM;
+
+	cbmem_console = memremap(physaddr,
+				 tmp_cbmc->buffer_size + sizeof(*cbmem_console),
+				 MEMREMAP_WB);
+	memunmap(tmp_cbmc);
+
+	if (!cbmem_console)
+		return -ENOMEM;
+
+	memconsole_setup(cbmem_console->buffer_body,
+		min(cbmem_console->buffer_cursor, cbmem_console->buffer_size));
+
+	return 0;
+}
+
+static int memconsole_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct lb_cbmem_ref entry;
+
+	ret = coreboot_table_find(CB_TAG_CBMEM_CONSOLE, &entry, sizeof(entry));
+	if (ret)
+		return ret;
+
+	ret = memconsole_coreboot_init(entry.cbmem_addr);
+	if (ret)
+		return ret;
+
+	return memconsole_sysfs_init();
+}
+
+static int memconsole_remove(struct platform_device *pdev)
+{
+	memconsole_exit();
+
+	if (cbmem_console)
+		memunmap(cbmem_console);
+
+	return 0;
+}
+
+static struct platform_driver memconsole_driver = {
+	.probe = memconsole_probe,
+	.remove = memconsole_remove,
+	.driver = {
+		.name = "memconsole",
+	},
+};
+
+static int __init platform_memconsole_init(void)
+{
+	struct platform_device *pdev;
+
+	pdev = platform_device_register_simple("memconsole", -1, NULL, 0);
+	if (pdev == NULL)
+		return -ENODEV;
+
+	platform_driver_register(&memconsole_driver);
+
+	return 0;
+}
+
+module_init(platform_memconsole_init);
+
+MODULE_AUTHOR("Google, Inc.");
+MODULE_LICENSE("GPL");
-- 
2.7.4

[PATCH 4/5] firmware: Add coreboot device tree binding documentation

[Thierry Escande]

From: Julius Werner <jwerner@chromium.org>

This patch adds documentation describing a device tree binding for the
coreboot firmware. It is meant to be dynamically added during boot and
contains address definitions for the coreboot table (a list of
variable-sized descriptors providing information about various compile-
and run-time generated firmware parameters) and the CBMEM area (the
structure containing most run-time resident memory regions set up by
coreboot).

These definitions allow kernel drivers to easily access data contained
in and pointed to by these regions (such as coreboot's in-memory log).
(An example implementation can be seen in the following patch)

Signed-off-by: Julius Werner <jwerner@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 .../devicetree/bindings/firmware/coreboot.txt      | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/coreboot.txt

diff --git a/Documentation/devicetree/bindings/firmware/coreboot.txt b/Documentation/devicetree/bindings/firmware/coreboot.txt
new file mode 100644
index 0000000..4c95570
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/coreboot.txt
@@ -0,0 +1,33 @@
+COREBOOT firmware information
+
+The device tree node to communicate the location of coreboot's memory-resident
+bookkeeping structures to the kernel. Since coreboot itself cannot boot a
+device-tree-based kernel (yet), this node needs to be inserted by a
+second-stage bootloader (a coreboot "payload").
+
+Required properties:
+ - compatible: Should be "coreboot"
+ - reg: Address and length of the following two memory regions, in order:
+	1.) The coreboot table. This is a list of variable-sized descriptors
+	that contain various compile- and run-time generated firmware
+	parameters. It is identified by the magic string "LBIO" in its first
+	four bytes.
+	See coreboot's src/commonlib/include/commonlib/coreboot_tables.h for
+	details.
+	2.) The CBMEM area. This is a downward-growing memory region used by
+	coreboot to dynamically allocate data structures that remain resident.
+	It may or may not include the coreboot table as one of its members. It
+	is identified by a root node descriptor with the magic number
+	0xc0389481 that resides in the topmost 8 bytes of the area.
+	See coreboot's src/include/imd.h for details.
+
+Example:
+	firmware {
+		ranges;
+
+		coreboot {
+			compatible = "coreboot";
+			reg = <0xfdfea000 0x264>,
+			      <0xfdfea000 0x16000>;
+		}
+	};
-- 
2.7.4

[PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support

[Thierry Escande]

This patch expands the Google firmware memory console driver to also
work on certain tree based platforms running coreboot, such as ARM/ARM64
Chromebooks. This patch now adds another path to find the coreboot table
through the device tree. In order to find that, a second level
bootloader must have installed the 'coreboot' compatible device tree
node that describes its base address and size.

This patch is a rework/split/merge of patches from the chromeos v4.4
kernel tree originally authored by:
 Wei-Ning Huang <wnhuang@chromium.org>
 Julius Werner <jwerner@chromium.org>
 Brian Norris <briannorris@chromium.org>

Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/firmware/google/Kconfig             | 13 ++++-
 drivers/firmware/google/Makefile            |  1 +
 drivers/firmware/google/coreboot_table-of.c | 82 +++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+), 2 deletions(-)
 create mode 100644 drivers/firmware/google/coreboot_table-of.c

diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 840bd62..baabd70 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -1,6 +1,5 @@
 menuconfig GOOGLE_FIRMWARE
 	bool "Google Firmware Drivers"
-	depends on X86
 	default n
 	help
 	  These firmware drivers are used by Google's servers.  They are
@@ -21,7 +20,7 @@ config GOOGLE_SMI
 
 config GOOGLE_COREBOOT_TABLE
 	tristate
-	depends on GOOGLE_COREBOOT_TABLE_ACPI
+	depends on GOOGLE_COREBOOT_TABLE_ACPI || GOOGLE_COREBOOT_TABLE_OF
 
 config GOOGLE_COREBOOT_TABLE_ACPI
 	tristate "Coreboot Table Access - ACPI"
@@ -33,6 +32,16 @@ config GOOGLE_COREBOOT_TABLE_ACPI
 	  pointer is accessed through the ACPI "GOOGCB00" object.
 	  If unsure say N.
 
+config GOOGLE_COREBOOT_TABLE_OF
+	tristate "Coreboot Table Access - Device Tree"
+	depends on OF
+	select GOOGLE_COREBOOT_TABLE
+	help
+	  This option enable the coreboot_table module, which provide other
+	  firmware modules to access coreboot table. The coreboot table pointer
+	  is accessed through the device tree node /firmware/coreboot.
+	  If unsure say N.
+
 config GOOGLE_MEMCONSOLE
 	tristate
 	depends on GOOGLE_MEMCONSOLE_X86_LEGACY || GOOGLE_MEMCONSOLE_COREBOOT
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
index 662a83e..bb952c6 100644
--- a/drivers/firmware/google/Makefile
+++ b/drivers/firmware/google/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_GOOGLE_SMI)		+= gsmi.o
 obj-$(CONFIG_GOOGLE_COREBOOT_TABLE)        += coreboot_table.o
 obj-$(CONFIG_GOOGLE_COREBOOT_TABLE_ACPI)   += coreboot_table-acpi.o
+obj-$(CONFIG_GOOGLE_COREBOOT_TABLE_OF)     += coreboot_table-of.o
 obj-$(CONFIG_GOOGLE_MEMCONSOLE)            += memconsole.o
 obj-$(CONFIG_GOOGLE_MEMCONSOLE_COREBOOT)   += memconsole-coreboot.o
 obj-$(CONFIG_GOOGLE_MEMCONSOLE_X86_LEGACY) += memconsole-x86-legacy.o
diff --git a/drivers/firmware/google/coreboot_table-of.c b/drivers/firmware/google/coreboot_table-of.c
new file mode 100644
index 0000000..727acdc
--- /dev/null
+++ b/drivers/firmware/google/coreboot_table-of.c
@@ -0,0 +1,82 @@
+/*
+ * coreboot_table-of.c
+ *
+ * Coreboot table access through open firmware.
+ *
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include "coreboot_table.h"
+
+static int coreboot_table_of_probe(struct platform_device *pdev)
+{
+	struct device_node *fw_dn = pdev->dev.of_node;
+	void __iomem *ptr;
+
+	ptr = of_iomap(fw_dn, 0);
+	of_node_put(fw_dn);
+	if (!ptr)
+		return -ENOMEM;
+
+	return coreboot_table_init(ptr);
+}
+
+static int coreboot_table_of_remove(struct platform_device *pdev)
+{
+	return coreboot_table_exit();
+}
+
+static const struct of_device_id coreboot_of_match[] = {
+	{ .compatible = "coreboot" },
+	{},
+};
+
+static struct platform_driver coreboot_table_of_driver = {
+	.probe = coreboot_table_of_probe,
+	.remove = coreboot_table_of_remove,
+	.driver = {
+		.name = "coreboot_table_of",
+		.of_match_table = coreboot_of_match,
+	},
+};
+
+static int __init platform_coreboot_table_of_init(void)
+{
+	struct platform_device *pdev;
+	struct device_node *of_node;
+
+	/* Limit device creation to the presence of /firmware/coreboot node */
+	of_node = of_find_node_by_path("/firmware/coreboot");
+	if (!of_node)
+		return -ENODEV;
+
+	if (!of_match_node(coreboot_of_match, of_node))
+		return -ENODEV;
+
+	pdev = of_platform_device_create(of_node, "coreboot_table_of", NULL);
+	if (!pdev)
+		return -ENODEV;
+
+	return platform_driver_register(&coreboot_table_of_driver);
+}
+
+module_init(platform_coreboot_table_of_init);
+
+MODULE_AUTHOR("Google, Inc.");
+MODULE_LICENSE("GPL");
-- 
2.7.4

Re: [PATCH 4/5] firmware: Add coreboot device tree binding documentation

[Mark Rutland]

On Thu, Mar 23, 2017 at 10:04:28PM +0100, Thierry Escande wrote:
> From: Julius Werner <jwerner@chromium.org>
> 
> This patch adds documentation describing a device tree binding for the
> coreboot firmware. It is meant to be dynamically added during boot and
> contains address definitions for the coreboot table (a list of
> variable-sized descriptors providing information about various compile-
> and run-time generated firmware parameters) and the CBMEM area (the
> structure containing most run-time resident memory regions set up by
> coreboot).
> 
> These definitions allow kernel drivers to easily access data contained
> in and pointed to by these regions (such as coreboot's in-memory log).
> (An example implementation can be seen in the following patch)
> 
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  .../devicetree/bindings/firmware/coreboot.txt      | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/coreboot.txt
> 
> diff --git a/Documentation/devicetree/bindings/firmware/coreboot.txt b/Documentation/devicetree/bindings/firmware/coreboot.txt
> new file mode 100644
> index 0000000..4c95570
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/coreboot.txt
> @@ -0,0 +1,33 @@
> +COREBOOT firmware information
> +
> +The device tree node to communicate the location of coreboot's memory-resident
> +bookkeeping structures to the kernel. Since coreboot itself cannot boot a
> +device-tree-based kernel (yet), this node needs to be inserted by a
> +second-stage bootloader (a coreboot "payload").
> +
> +Required properties:
> + - compatible: Should be "coreboot"

Devicetree bindings should be in vendor,prefix format. This doesn't
represent every aspect of coreboot, so it needs a more descriptive
string.

> + - reg: Address and length of the following two memory regions, in order:
> +	1.) The coreboot table. This is a list of variable-sized descriptors
> +	that contain various compile- and run-time generated firmware
> +	parameters. It is identified by the magic string "LBIO" in its first
> +	four bytes.
> +	See coreboot's src/commonlib/include/commonlib/coreboot_tables.h for
> +	details.

Given this is a memory region, it should be described under the
reserved-memory node.

> +	2.) The CBMEM area. This is a downward-growing memory region used by
> +	coreboot to dynamically allocate data structures that remain resident.
> +	It may or may not include the coreboot table as one of its members. It
> +	is identified by a root node descriptor with the magic number
> +	0xc0389481 that resides in the topmost 8 bytes of the area.
> +	See coreboot's src/include/imd.h for details.

I beleive likewise here.

Thanks,
Mark.

> +
> +Example:
> +	firmware {
> +		ranges;
> +
> +		coreboot {
> +			compatible = "coreboot";
> +			reg = <0xfdfea000 0x264>,
> +			      <0xfdfea000 0x16000>;
> +		}
> +	};
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support

[Mark Rutland]

On Thu, Mar 23, 2017 at 10:04:29PM +0100, Thierry Escande wrote:
> This patch expands the Google firmware memory console driver to also
> work on certain tree based platforms running coreboot, such as ARM/ARM64
> Chromebooks. This patch now adds another path to find the coreboot table
> through the device tree. In order to find that, a second level
> bootloader must have installed the 'coreboot' compatible device tree
> node that describes its base address and size.

What exactly is the "memory console"? Is it a log that coreboot writes into?

[...]

> +static const struct of_device_id coreboot_of_match[] = {
> +	{ .compatible = "coreboot" },
> +	{},
> +};
> +
> +static struct platform_driver coreboot_table_of_driver = {
> +	.probe = coreboot_table_of_probe,
> +	.remove = coreboot_table_of_remove,
> +	.driver = {
> +		.name = "coreboot_table_of",
> +		.of_match_table = coreboot_of_match,
> +	},
> +};
> +
> +static int __init platform_coreboot_table_of_init(void)
> +{
> +	struct platform_device *pdev;
> +	struct device_node *of_node;
> +
> +	/* Limit device creation to the presence of /firmware/coreboot node */
> +	of_node = of_find_node_by_path("/firmware/coreboot");
> +	if (!of_node)
> +		return -ENODEV;
> +

I don't beleive that you need this module init function. Please use the
usual DT probing infrastrucutre instead, e.g. add:

MODULE_DEVICE_TABLE(of, coreboot_of_match);
module_platform_driver(coreboot_table_of_driver);

Thanks,
Mark.

Re: [PATCH 4/5] firmware: Add coreboot device tree binding documentation

[Brian Norris]

Hi Mark,

Julius can correct me if I'm wrong on any of this, but a quick note that
probably should have prefaced this:

These bindings were submitted years go

https://patchwork.ozlabs.org/patch/359672/
[PATCH] firmware: Add device tree binding for coreboot

discussed, and resubmitted seemingly according to several reviewers'
suggestions here

https://patchwork.kernel.org/patch/4362661/
[PATCH v2] firmware: Add device tree binding for coreboot

after which point we heard no response.

So these bindings have been baked into our coreboot firmware ever since
and are impossible to change on existing devices. It's *possible* we
could make modifications for new products, but it's really a non-starter
for this binding to be rejected at this point. (Or at least, if you want
this stuff to work in mainline for any Chromebook made in the last
several years.) If it's rejected, Chromebooks will just have to continue
shipping with forked drivers in our downstream tree.

Again, IMO that doesn't mean that this binding can't be improved or
augmented, but I think it's worth very serious consideration that the
legacy binding be considered and documented, even if it's noted as
"deprecated" or something like that.

On Fri, Mar 24, 2017 at 12:21:46PM +0000, Mark Rutland wrote:
> On Thu, Mar 23, 2017 at 10:04:28PM +0100, Thierry Escande wrote:
> > From: Julius Werner <jwerner@chromium.org>
> > 
> > This patch adds documentation describing a device tree binding for the
> > coreboot firmware. It is meant to be dynamically added during boot and
> > contains address definitions for the coreboot table (a list of
> > variable-sized descriptors providing information about various compile-
> > and run-time generated firmware parameters) and the CBMEM area (the
> > structure containing most run-time resident memory regions set up by
> > coreboot).
> > 
> > These definitions allow kernel drivers to easily access data contained
> > in and pointed to by these regions (such as coreboot's in-memory log).
> > (An example implementation can be seen in the following patch)
> > 
> > Signed-off-by: Julius Werner <jwerner@chromium.org>
> > Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> > ---
> >  .../devicetree/bindings/firmware/coreboot.txt      | 33 ++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/firmware/coreboot.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/firmware/coreboot.txt b/Documentation/devicetree/bindings/firmware/coreboot.txt
> > new file mode 100644
> > index 0000000..4c95570
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/coreboot.txt
> > @@ -0,0 +1,33 @@
> > +COREBOOT firmware information
> > +
> > +The device tree node to communicate the location of coreboot's memory-resident
> > +bookkeeping structures to the kernel. Since coreboot itself cannot boot a
> > +device-tree-based kernel (yet), this node needs to be inserted by a
> > +second-stage bootloader (a coreboot "payload").
> > +
> > +Required properties:
> > + - compatible: Should be "coreboot"
> 
> Devicetree bindings should be in vendor,prefix format. This doesn't
> represent every aspect of coreboot, so it needs a more descriptive
> string.

Any particular suggestion? I suppose this is Google's interpretation of
coreboot tables, so "google,coreboot"?

> > + - reg: Address and length of the following two memory regions, in order:
> > +	1.) The coreboot table. This is a list of variable-sized descriptors
> > +	that contain various compile- and run-time generated firmware
> > +	parameters. It is identified by the magic string "LBIO" in its first
> > +	four bytes.
> > +	See coreboot's src/commonlib/include/commonlib/coreboot_tables.h for
> > +	details.
> 
> Given this is a memory region, it should be described under the
> reserved-memory node.

We've painted this bikeshed before. I guess the result was either to use
/reserved-memory or /memreserve/. I believe we've been using
/memreserve/. I suppose we could document a method to use either, but
the main "agreement" was to use the /firmware/coreboot path instead of
/reserved-memory/coreboot.

(And yes, I suppose /memreserve/ should be noted here.)

Brian

> > +	2.) The CBMEM area. This is a downward-growing memory region used by
> > +	coreboot to dynamically allocate data structures that remain resident.
> > +	It may or may not include the coreboot table as one of its members. It
> > +	is identified by a root node descriptor with the magic number
> > +	0xc0389481 that resides in the topmost 8 bytes of the area.
> > +	See coreboot's src/include/imd.h for details.
> 
> I beleive likewise here.
> 
> Thanks,
> Mark.
> 
> > +
> > +Example:
> > +	firmware {
> > +		ranges;
> > +
> > +		coreboot {
> > +			compatible = "coreboot";
> > +			reg = <0xfdfea000 0x264>,
> > +			      <0xfdfea000 0x16000>;
> > +		}
> > +	};
> > -- 
> > 2.7.4

Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support

[Brian Norris]

On Fri, Mar 24, 2017 at 12:28:59PM +0000, Mark Rutland wrote:
> On Thu, Mar 23, 2017 at 10:04:29PM +0100, Thierry Escande wrote:
> > +static const struct of_device_id coreboot_of_match[] = {
> > +	{ .compatible = "coreboot" },
> > +	{},
> > +};
> > +
> > +static struct platform_driver coreboot_table_of_driver = {
> > +	.probe = coreboot_table_of_probe,
> > +	.remove = coreboot_table_of_remove,
> > +	.driver = {
> > +		.name = "coreboot_table_of",
> > +		.of_match_table = coreboot_of_match,
> > +	},
> > +};
> > +
> > +static int __init platform_coreboot_table_of_init(void)
> > +{
> > +	struct platform_device *pdev;
> > +	struct device_node *of_node;
> > +
> > +	/* Limit device creation to the presence of /firmware/coreboot node */
> > +	of_node = of_find_node_by_path("/firmware/coreboot");
> > +	if (!of_node)
> > +		return -ENODEV;
> > +
> 
> I don't beleive that you need this module init function. Please use the
> usual DT probing infrastrucutre instead, e.g. add:
> 
> MODULE_DEVICE_TABLE(of, coreboot_of_match);
> module_platform_driver(coreboot_table_of_driver);

That doesn't work. If this node is placed under /firmware, which isn't a
proper "bus", then we have to explicitly look for the sub-device.

Due to this, the MODULE_DEVICE_TABLE() also isn't useful, because the
/firmware/coreboot/ device won't be generated automatically, and so no
matching uevent will occur.

Brian

Re: [PATCH 4/5] firmware: Add coreboot device tree binding documentation

[Julius Werner]

...and again in plaintext, sorry about that.

On Fri, Mar 24, 2017 at 12:32 PM, Julius Werner <jwerner@chromium.org> wrote:
>> > Devicetree bindings should be in vendor,prefix format. This doesn't
>> > represent every aspect of coreboot, so it needs a more descriptive
>> > string.
>>
>> Any particular suggestion? I suppose this is Google's interpretation of
>> coreboot tables, so "google,coreboot"?
>
>
> No. This binding is for the coreboot project in general and has nothing to
> do with Google. coreboot is both the vendor and the product, so I think
> "coreboot" would look better than "coreboot,coreboot". And yes, it is
> supposed to represent every aspect of coreboot (right now the "coreboot
> tables" are already sort of a catch-all data structure used by coreboot to
> pass on any sort of info it wants later stages to know... and if we ever
> have any additional things we'd like to pass on, we'd probably want to add
> them to this binding as well).
>
>>
>> > > + - reg: Address and length of the following two memory regions, in
>> > > order:
>> > > +   1.) The coreboot table. This is a list of variable-sized
>> > > descriptors
>> > > +   that contain various compile- and run-time generated firmware
>> > > +   parameters. It is identified by the magic string "LBIO" in its
>> > > first
>> > > +   four bytes.
>> > > +   See coreboot's src/commonlib/include/commonlib/coreboot_tables.h
>> > > for
>> > > +   details.
>> >
>> > Given this is a memory region, it should be described under the
>> > reserved-memory node.
>>
>> We've painted this bikeshed before. I guess the result was either to use
>> /reserved-memory or /memreserve/. I believe we've been using
>> /memreserve/. I suppose we could document a method to use either, but
>> the main "agreement" was to use the /firmware/coreboot path instead of
>> /reserved-memory/coreboot.
>
>
> See the old thread Brian linked for some arguments for and against this. I
> think particularly because we want this node to represent every aspect of
> coreboot (which I think makes more sense than spreading stuff all over the
> place), treating it as reserved memory would not work well if we might later
> add other fields.
>
> Also, since we didn't get any more responses the last time we tried to
> upstream this and had schedules to keep, we had to go ahead with what we
> had. So right now there are already millions of devices shipped with this
> binding in firmware, and the only question we still have left to decide is
> whether Linux wants to support them upstream or not.

Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support

[Julius Werner]

> What exactly is the "memory console"? Is it a log that coreboot writes into?

Yes. It contains log messages, like coreboot's equivalent of dmesg.

Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support

[kbuild test robot]

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

Hi Thierry,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thierry-Escande/firmware-google-memconsole/20170326-080828
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All errors (new ones prefixed by >>):

   drivers/firmware/google/gsmi.c: In function 'gsmi_exec.constprop':
>> drivers/firmware/google/gsmi.c:198:3: error: inconsistent operand constraints in an 'asm'
      asm volatile (
      ^~~
   drivers/firmware/google/gsmi.c:213:3: error: inconsistent operand constraints in an 'asm'
      asm volatile (
      ^~~
   drivers/firmware/google/gsmi.c:229:3: error: inconsistent operand constraints in an 'asm'
      asm volatile (
      ^~~

vim +/asm +198 drivers/firmware/google/gsmi.c

74c5b31c Mike Waychison 2011-04-29  182  	/*
74c5b31c Mike Waychison 2011-04-29  183  	 * AH  : Subfunction number
74c5b31c Mike Waychison 2011-04-29  184  	 * AL  : Function number
74c5b31c Mike Waychison 2011-04-29  185  	 * EBX : Parameter block address
74c5b31c Mike Waychison 2011-04-29  186  	 * DX  : SMI command port
74c5b31c Mike Waychison 2011-04-29  187  	 *
74c5b31c Mike Waychison 2011-04-29  188  	 * Three protocols here. See also the comment in gsmi_init().
74c5b31c Mike Waychison 2011-04-29  189  	 */
74c5b31c Mike Waychison 2011-04-29  190  	if (gsmi_dev.handshake_type == GSMI_HANDSHAKE_CF) {
74c5b31c Mike Waychison 2011-04-29  191  		/*
74c5b31c Mike Waychison 2011-04-29  192  		 * If handshake_type == HANDSHAKE_CF then set CF on the
74c5b31c Mike Waychison 2011-04-29  193  		 * way in and wait for the handler to clear it; this avoids
74c5b31c Mike Waychison 2011-04-29  194  		 * corrupting register state on those chipsets which have
74c5b31c Mike Waychison 2011-04-29  195  		 * a delay between writing the SMI trigger register and
74c5b31c Mike Waychison 2011-04-29  196  		 * entering SMM.
74c5b31c Mike Waychison 2011-04-29  197  		 */
74c5b31c Mike Waychison 2011-04-29 @198  		asm volatile (
74c5b31c Mike Waychison 2011-04-29  199  			"stc\n"
74c5b31c Mike Waychison 2011-04-29  200  			"outb %%al, %%dx\n"
74c5b31c Mike Waychison 2011-04-29  201  		"1:      jc 1b\n"
74c5b31c Mike Waychison 2011-04-29  202  			: "=a" (result)
74c5b31c Mike Waychison 2011-04-29  203  			: "0" (cmd),
74c5b31c Mike Waychison 2011-04-29  204  			  "d" (gsmi_dev.smi_cmd),
74c5b31c Mike Waychison 2011-04-29  205  			  "b" (gsmi_dev.param_buf->address)
74c5b31c Mike Waychison 2011-04-29  206  			: "memory", "cc"

:::::: The code at line 198 was first introduced by commit
:::::: 74c5b31c6618f01079212332b2e5f6c42f2d6307 driver: Google EFI SMI

:::::: TO: Mike Waychison <mikew@google.com>
:::::: CC: Greg Kroah-Hartman <gregkh@suse.de>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 46805 bytes --]

Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support

[Brian Norris]

Hi Thierry,

In case this wasn't obvious from the build report:

On Thu, Mar 23, 2017 at 10:04:29PM +0100, Thierry Escande wrote:
> --- a/drivers/firmware/google/Kconfig
> +++ b/drivers/firmware/google/Kconfig
> @@ -1,6 +1,5 @@
>  menuconfig GOOGLE_FIRMWARE
>  	bool "Google Firmware Drivers"
> -	depends on X86

The build bots complained: you're relaxing the compile-time restriction
here, so you should probably move it to the GOOGLE_SMI symbol, since
that file uses X86 assembly.

>  	default n
>  	help
>  	  These firmware drivers are used by Google's servers.  They are
> @@ -21,7 +20,7 @@ config GOOGLE_SMI
>  
>  config GOOGLE_COREBOOT_TABLE
>  	tristate
> -	depends on GOOGLE_COREBOOT_TABLE_ACPI
> +	depends on GOOGLE_COREBOOT_TABLE_ACPI || GOOGLE_COREBOOT_TABLE_OF
>  
>  config GOOGLE_COREBOOT_TABLE_ACPI
>  	tristate "Coreboot Table Access - ACPI"
> @@ -33,6 +32,16 @@ config GOOGLE_COREBOOT_TABLE_ACPI
>  	  pointer is accessed through the ACPI "GOOGCB00" object.
>  	  If unsure say N.
>  
> +config GOOGLE_COREBOOT_TABLE_OF
> +	tristate "Coreboot Table Access - Device Tree"
> +	depends on OF
> +	select GOOGLE_COREBOOT_TABLE
> +	help
> +	  This option enable the coreboot_table module, which provide other
> +	  firmware modules to access coreboot table. The coreboot table pointer
> +	  is accessed through the device tree node /firmware/coreboot.
> +	  If unsure say N.
> +
>  config GOOGLE_MEMCONSOLE
>  	tristate
>  	depends on GOOGLE_MEMCONSOLE_X86_LEGACY || GOOGLE_MEMCONSOLE_COREBOOT

Brian