Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1607918 > unrolled thread

[PATCH 0/5] firmware: google memconsole

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] firmware: google memconsole Thierry Escande <thierry.escande@collabora.com> - 2017-03-23 22:10 +0100
    [PATCH 4/5] firmware: Add coreboot device tree binding documentation Thierry Escande <thierry.escande@collabora.com> - 2017-03-23 22:10 +0100
      Re: [PATCH 4/5] firmware: Add coreboot device tree binding  documentation Mark Rutland <mark.rutland@arm.com> - 2017-03-24 13:30 +0100
        Re: [PATCH 4/5] firmware: Add coreboot device tree binding  documentation Brian Norris <briannorris@chromium.org> - 2017-03-24 19:00 +0100
          Re: [PATCH 4/5] firmware: Add coreboot device tree binding documentation Julius Werner <jwerner@chromium.org> - 2017-03-24 20:40 +0100
    [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support Thierry Escande <thierry.escande@collabora.com> - 2017-03-23 22:10 +0100
      Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support Mark Rutland <mark.rutland@arm.com> - 2017-03-24 13:30 +0100
        Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support Brian Norris <briannorris@chromium.org> - 2017-03-24 19:10 +0100
        Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support Julius Werner <jwerner@chromium.org> - 2017-03-24 21:00 +0100
      Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support Brian Norris <briannorris@chromium.org> - 2017-03-27 19:00 +0200
    [PATCH 3/5] firmware: google memconsole: Add coreboot support Thierry Escande <thierry.escande@collabora.com> - 2017-03-23 22:10 +0100
    [PATCH 1/5] firmware: google memconsole: Remove useless submenu in Kconfig Thierry Escande <thierry.escande@collabora.com> - 2017-03-23 22:10 +0100

#1607918 — [PATCH 0/5] firmware: google memconsole

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

[toc] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

...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.

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

> 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.

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web