[PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Andy Lutomirski]

Many laptops (and maybe servers?) have embedded WMI Binary MOF metadata.
We do not yet have open-source tools for processing the data, although
one is in the works thanks to Pali:

	https://github.com/pali/bmfdec

There is currently no interface to get the data in the first place. By
exposing it, we facilitate the development of new tools.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Mario Limonciello <mario_limonciello@dell.com>
Cc: Pali Rohár <pali.rohar@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
[dvhart: make sysfs mof binary read only, fixup comment block format]
[dvhart: use bmof terminology and dev_err instead of dev_warn]
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
---
since-v1:
 * address Pali's comments:
   * update the cover letter for clarity and accuracy
   * update mof->bmof and MOF to Binary MOF throughout the patch
   * use dev_err instead of dev_warn in wmi_bmof_probe


 drivers/platform/x86/Kconfig    |  12 ++++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/wmi-bmof.c | 125 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+)
 create mode 100644 drivers/platform/x86/wmi-bmof.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 49a1d01..6ebe393 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -656,6 +656,18 @@ config ACPI_WMI
 	  It is safe to enable this driver even if your DSDT doesn't define
 	  any ACPI-WMI devices.
 
+config WMI_BMOF
+	tristate "WMI embedded Binary MOF driver"
+	depends on ACPI_WMI
+	default y
+	---help---
+	  Say Y here if you want to be able to read a firmware-embedded
+	  WMI Binary MOF data. Using this requires userspace tools and may be
+	  rather tedious.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called wmi-bmof.
+
 config MSI_WMI
 	tristate "MSI WMI extras"
 	depends on ACPI_WMI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 652d7c8..6a1063e 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_MSI_WMI)		+= msi-wmi.o
 obj-$(CONFIG_PEAQ_WMI)		+= peaq-wmi.o
 obj-$(CONFIG_SURFACE3_WMI)	+= surface3-wmi.o
 obj-$(CONFIG_TOPSTAR_LAPTOP)	+= topstar-laptop.o
+obj-$(CONFIG_WMI_BMOF)		+= wmi-bmof.o
 
 # toshiba_acpi must link after wmi to ensure that wmi devices are found
 # before toshiba_acpi initializes
diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c
new file mode 100644
index 0000000..e1c0963
--- /dev/null
+++ b/drivers/platform/x86/wmi-bmof.c
@@ -0,0 +1,125 @@
+/*
+ * WMI embedded Binary MOF driver
+ *
+ * Copyright (c) 2015 Andrew Lutomirski
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License version 2 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.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/acpi.h>
+#include <linux/string.h>
+#include <linux/dmi.h>
+#include <linux/wmi.h>
+#include <acpi/video.h>
+
+#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
+MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
+
+struct bmof_priv {
+	union acpi_object *bmofdata;
+	struct bin_attribute bmof_bin_attr;
+};
+
+static ssize_t
+read_bmof(struct file *filp, struct kobject *kobj,
+	 struct bin_attribute *attr,
+	 char *buf, loff_t off, size_t count)
+{
+	struct bmof_priv *priv =
+		container_of(attr, struct bmof_priv, bmof_bin_attr);
+
+	if (off >= priv->bmofdata->buffer.length)
+		return 0;
+
+	if (count > priv->bmofdata->buffer.length - off)
+		count = priv->bmofdata->buffer.length - off;
+
+	memcpy(buf, priv->bmofdata->buffer.pointer + off, count);
+	return count;
+}
+
+static int wmi_bmof_probe(struct wmi_device *wdev)
+{
+	int ret;
+
+	struct bmof_priv *priv =
+		devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);
+
+	if (!priv)
+		return -ENOMEM;
+
+	dev_set_drvdata(&wdev->dev, priv);
+
+	priv->bmofdata = wmidev_block_query(wdev, 0);
+	if (!priv->bmofdata) {
+		dev_err(&wdev->dev, "failed to read Binary MOF\n");
+		return -EIO;
+	}
+
+	if (priv->bmofdata->type != ACPI_TYPE_BUFFER) {
+		dev_err(&wdev->dev, "Binary MOF is not a buffer\n");
+		ret = -EIO;
+		goto err_free;
+	}
+
+	sysfs_bin_attr_init(&priv->bmof_bin_attr);
+	priv->bmof_bin_attr.attr.name = "bmof";
+	priv->bmof_bin_attr.attr.mode = 0400;
+	priv->bmof_bin_attr.read = read_bmof;
+	priv->bmof_bin_attr.size = priv->bmofdata->buffer.length;
+
+	ret = sysfs_create_bin_file(&wdev->dev.kobj, &priv->bmof_bin_attr);
+	if (ret)
+		goto err_free;
+
+	return 0;
+
+ err_free:
+	kfree(priv->bmofdata);
+	return ret;
+}
+
+static int wmi_bmof_remove(struct wmi_device *wdev)
+{
+	struct bmof_priv *priv = dev_get_drvdata(&wdev->dev);
+
+	sysfs_remove_bin_file(&wdev->dev.kobj, &priv->bmof_bin_attr);
+	kfree(priv->bmofdata);
+	return 0;
+}
+
+static const struct wmi_device_id wmi_bmof_id_table[] = {
+	{ .guid_string = WMI_BMOF_GUID },
+	{ },
+};
+
+static struct wmi_driver wmi_bmof_driver = {
+	.driver = {
+		.name = "wmi-bmof",
+	},
+	.probe = wmi_bmof_probe,
+	.remove = wmi_bmof_remove,
+	.id_table = wmi_bmof_id_table,
+};
+
+module_wmi_driver(wmi_bmof_driver);
+
+MODULE_AUTHOR("Andrew Lutomirski <luto@kernel.org>");
+MODULE_DESCRIPTION("WMI embedded Binary MOF driver");
+MODULE_LICENSE("GPL");
-- 
2.9.4


-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Andy Shevchenko]

On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <luto@kernel.org> wrote:
> Many laptops (and maybe servers?) have embedded WMI Binary MOF metadata.
> We do not yet have open-source tools for processing the data, although
> one is in the works thanks to Pali:
>
>         https://github.com/pali/bmfdec
>
> There is currently no interface to get the data in the first place. By
> exposing it, we facilitate the development of new tools.

My comments below.
Overall, FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


> +config WMI_BMOF
> +       tristate "WMI embedded Binary MOF driver"
> +       depends on ACPI_WMI

> +       default y

Since it can be module it would be better to have more sane default
(distros usually prefers modules over built-in).
Thus, I would go, for example, with

default ACPI_WMI

> +       ---help---
> +         Say Y here if you want to be able to read a firmware-embedded
> +         WMI Binary MOF data. Using this requires userspace tools and may be
> +         rather tedious.
> +
> +         To compile this driver as a module, choose M here: the module will
> +         be called wmi-bmof.

> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <linux/acpi.h>
> +#include <linux/string.h>
> +#include <linux/dmi.h>
> +#include <linux/wmi.h>
> +#include <acpi/video.h>

Alphabetical order? Up to you.

> +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"

> +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);

I would gather all MODULE_* together, but it's also matter of taste.

> +static ssize_t
> +read_bmof(struct file *filp, struct kobject *kobj,
> +        struct bin_attribute *attr,
> +        char *buf, loff_t off, size_t count)
> +{
> +       struct bmof_priv *priv =
> +               container_of(attr, struct bmof_priv, bmof_bin_attr);
> +
> +       if (off >= priv->bmofdata->buffer.length)
> +               return 0;

Shouldn't we return an error code here? -ERANGE or alike?

> +static int wmi_bmof_probe(struct wmi_device *wdev)
> +{

> +       int ret;
> +
> +       struct bmof_priv *priv =
> +               devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);

I'm not a fan of memory allocation in definition block, so, I would rewrite this

      struct bmof_priv *priv;
      int ret;

      priv = devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);

(sizeof(*priv) by your choice)

> +
> +       if (!priv)
> +               return -ENOMEM;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Pali Rohár]

On Monday 05 June 2017 20:16:44 Andy Lutomirski wrote:
> +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);

Cannot we generate MODULE_ALIAS from module_wmi_driver()? IIRC it is
working for i2c drivers.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Darren Hart]

On Tue, Jun 06, 2017 at 12:30:38PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <luto@kernel.org> wrote:
> > Many laptops (and maybe servers?) have embedded WMI Binary MOF metadata.
> > We do not yet have open-source tools for processing the data, although
> > one is in the works thanks to Pali:
> >
> >         https://github.com/pali/bmfdec
> >
> > There is currently no interface to get the data in the first place. By
> > exposing it, we facilitate the development of new tools.
> 
> My comments below.
> Overall, FWIW,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> 
> > +config WMI_BMOF
> > +       tristate "WMI embedded Binary MOF driver"
> > +       depends on ACPI_WMI
> 
> > +       default y
> 
> Since it can be module it would be better to have more sane default
> (distros usually prefers modules over built-in).
> Thus, I would go, for example, with
> 
> default ACPI_WMI

Good point, done.

> 
> > +       ---help---
> > +         Say Y here if you want to be able to read a firmware-embedded
> > +         WMI Binary MOF data. Using this requires userspace tools and may be
> > +         rather tedious.
> > +
> > +         To compile this driver as a module, choose M here: the module will
> > +         be called wmi-bmof.
> 
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +#include <linux/input.h>
> > +#include <linux/input/sparse-keymap.h>
> > +#include <linux/acpi.h>
> > +#include <linux/string.h>
> > +#include <linux/dmi.h>
> > +#include <linux/wmi.h>
> > +#include <acpi/video.h>
> 
> Alphabetical order? Up to you.

Hrm. There seems to be plenty of similar suggestions on the mailing lists, but
nothing documented in coding-style.rst. If this is a thing we are going to ask
of our contributors, it should be documented. I'm happy to reorder, would you
consider sending the coding-style patch?

> 
> > +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> 
> > +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
> 
> I would gather all MODULE_* together, but it's also matter of taste.
> 

Sure, done.

> > +static ssize_t
> > +read_bmof(struct file *filp, struct kobject *kobj,
> > +        struct bin_attribute *attr,
> > +        char *buf, loff_t off, size_t count)
> > +{
> > +       struct bmof_priv *priv =
> > +               container_of(attr, struct bmof_priv, bmof_bin_attr);
> > +
> > +       if (off >= priv->bmofdata->buffer.length)
> > +               return 0;
> 
> Shouldn't we return an error code here? -ERANGE or alike?
> 

I took some time and compared this with:

read(2)
lseek(2)
fseek(3)
memory_read_from_buffer()

If offset is <0, we should return EINVAL
If offset is >end_of_buffer.... it's not so cut and dry. It is simpler to just
return 0, and as far as how it affects usage... returning 0 seems perfectly
acceptable for typical read loop usage.

As loff_t is a long long, it could conceivably be < 0, so I've added a check for
that and return -EINVAL in that case.

> > +static int wmi_bmof_probe(struct wmi_device *wdev)
> > +{
> 
> > +       int ret;
> > +
> > +       struct bmof_priv *priv =
> > +               devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);
> 
> I'm not a fan of memory allocation in definition block, so, I would rewrite this
> 
>       struct bmof_priv *priv;
>       int ret;
> 
>       priv = devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);
> 

Agreed, changed.

Thanks for the review Andy.

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Darren Hart]

On Tue, Jun 06, 2017 at 12:30:38PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <luto@kernel.org> wrote:
> > Many laptops (and maybe servers?) have embedded WMI Binary MOF metadata.
> > We do not yet have open-source tools for processing the data, although
> > one is in the works thanks to Pali:
> >
> >         https://github.com/pali/bmfdec
> >
> > There is currently no interface to get the data in the first place. By
> > exposing it, we facilitate the development of new tools.
> 
> My comments below.
> Overall, FWIW,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> 
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +#include <linux/input.h>
> > +#include <linux/input/sparse-keymap.h>
> > +#include <linux/acpi.h>
> > +#include <linux/string.h>
> > +#include <linux/dmi.h>
> > +#include <linux/wmi.h>
> > +#include <acpi/video.h>
> 
> Alphabetical order? Up to you.

OK, I failed to audit this... lots we don't need in here.

The minimum to build is:

#include <linux/wmi.h>

So assuming this was copy/pasted from another file.

Again, no guidance in coding-style.rst on includes. Seems to me we should
include what we specifically require, regardless of whether or not another
header also happens to include it. We need acpi for example, even though wmi
also includes it.

We should include modules, even though acpi includes it.

We use several other things we aren't including for, like

memcpy
dev_kzalloc
sysfs_create_bin_file

So I suggest:

#include <linux/acpi.h>
#include <linux/device.h>
#include <linux/fs.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/string.h>
#include <linux/sysfs.h>
#include <linux/types.h>
#include <linux/wmi.h>

Which removes:
#include <acpi/video.h>
#include <linux/dmi.h>
#include <linux/init.h>
#include <linux/input.h>
#include <linux/input/sparse-keymap.h>
#include <linux/slab.h>

And adds:
#include <linux/device.h>
#include <linux/fs.h>
#include <linux/sysfs.h>

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Darren Hart]

On Tue, Jun 06, 2017 at 12:04:40PM +0200, Pali Rohár wrote:
> On Monday 05 June 2017 20:16:44 Andy Lutomirski wrote:
> > +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> > +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
> 
> Cannot we generate MODULE_ALIAS from module_wmi_driver()? IIRC it is
> working for i2c drivers.

I could see this being automated since we always use wmi:GUID, but it isn't
currently. Happy to consider it as a follow on.

Do you have a specific i2c example you think we should consider following?

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Andy Shevchenko]

On Tue, Jun 6, 2017 at 7:54 PM, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Jun 06, 2017 at 12:30:38PM +0300, Andy Shevchenko wrote:
>> On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <luto@kernel.org> wrote:

>> > +#include <linux/kernel.h>
>> > +#include <linux/module.h>
>> > +#include <linux/init.h>
>> > +#include <linux/slab.h>
>> > +#include <linux/types.h>
>> > +#include <linux/input.h>
>> > +#include <linux/input/sparse-keymap.h>
>> > +#include <linux/acpi.h>
>> > +#include <linux/string.h>
>> > +#include <linux/dmi.h>
>> > +#include <linux/wmi.h>
>> > +#include <acpi/video.h>
>>
>> Alphabetical order? Up to you.
>
> OK, I failed to audit this... lots we don't need in here.
>
> The minimum to build is:
>
> #include <linux/wmi.h>
>
> So assuming this was copy/pasted from another file.

> Again, no guidance in coding-style.rst on includes. Seems to me we should
> include what we specifically require, regardless of whether or not another
> header also happens to include it.

Usually it's a sane choice.
Regarding to order the rationale I see there is easiest way to detect
(on the glance) what headers are already there and there is no
duplication. I saw in the past few patches to remove header
duplication since the original list wasn't in order in the first
place.

Of course there might be exceptions.

> We need acpi for example, even though wmi
> also includes it.
>
> We should include modules, even though acpi includes it.
>
> We use several other things we aren't including for, like
>
> memcpy
> dev_kzalloc
> sysfs_create_bin_file
>
> So I suggest:
>
> #include <linux/acpi.h>
> #include <linux/device.h>
> #include <linux/fs.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/string.h>
> #include <linux/sysfs.h>
> #include <linux/types.h>
> #include <linux/wmi.h>
>
> Which removes:
> #include <acpi/video.h>
> #include <linux/dmi.h>
> #include <linux/init.h>
> #include <linux/input.h>
> #include <linux/input/sparse-keymap.h>
> #include <linux/slab.h>
>
> And adds:
> #include <linux/device.h>
> #include <linux/fs.h>
> #include <linux/sysfs.h>

Works for me!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 934 bytes --]

On Tuesday 06 June 2017 19:02:01 Darren Hart wrote:
> On Tue, Jun 06, 2017 at 12:04:40PM +0200, Pali Rohár wrote:
> > On Monday 05 June 2017 20:16:44 Andy Lutomirski wrote:
> > > +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> > > +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
> > 
> > Cannot we generate MODULE_ALIAS from module_wmi_driver()? IIRC it
> > is working for i2c drivers.
> 
> I could see this being automated since we always use wmi:GUID, but it
> isn't currently. Happy to consider it as a follow on.
> 
> Do you have a specific i2c example you think we should consider
> following?

For i2c you can specify in driver code:

MODULE_DEVICE_TABLE(i2c, id_table);

And it automatically provides (via file.mod.c) all needed MODULE_ALIAS.

So when we have wmi_bmof_id_table in driver, cannot we use this?

MODULE_DEVICE_TABLE(wmi, wmi_bmof_id_table);

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Andy Lutomirski]

On Mon, Jun 5, 2017 at 8:16 PM, Andy Lutomirski <luto@kernel.org> wrote:
> +static ssize_t
> +read_bmof(struct file *filp, struct kobject *kobj,
> +        struct bin_attribute *attr,
> +        char *buf, loff_t off, size_t count)
> +{
> +       struct bmof_priv *priv =
> +               container_of(attr, struct bmof_priv, bmof_bin_attr);
> +
> +       if (off >= priv->bmofdata->buffer.length)
> +               return 0;
> +
> +       if (count > priv->bmofdata->buffer.length - off)
> +               count = priv->bmofdata->buffer.length - off;
> +
> +       memcpy(buf, priv->bmofdata->buffer.pointer + off, count);
> +       return count;
> +}

I just discovered simple_read_from_buffer().  I think this whole
function could be:

struct bmof_priv *priv = ...;
return simple_read_from_buffer(buf, count, &off,
priv->bmofdata->buffer.pointer, priv->bmofdata->buffer.length);

--Andy

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[kbuild test robot]

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

Hi Andy,

[auto build test ERROR on platform-drivers-x86/for-next]
[also build test ERROR on v4.12-rc4 next-20170606]
[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/Andy-Lutomirski/platform-x86-wmi-bmof-New-driver-to-expose-embedded-Binary-WMI-MOF-metadata/20170607-062854
base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: x86_64-acpi-redef (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> drivers/platform//x86/wmi-bmof.c:28:23: fatal error: linux/wmi.h: No such file or directory
    #include <linux/wmi.h>
                          ^
   compilation terminated.

vim +28 drivers/platform//x86/wmi-bmof.c

    12	 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    13	 *  GNU General Public License for more details.
    14	 */
    15	
    16	#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
    17	
    18	#include <linux/kernel.h>
    19	#include <linux/module.h>
    20	#include <linux/init.h>
    21	#include <linux/slab.h>
    22	#include <linux/types.h>
    23	#include <linux/input.h>
    24	#include <linux/input/sparse-keymap.h>
    25	#include <linux/acpi.h>
    26	#include <linux/string.h>
    27	#include <linux/dmi.h>
  > 28	#include <linux/wmi.h>
    29	#include <acpi/video.h>
    30	
    31	#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
    32	MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
    33	
    34	struct bmof_priv {
    35		union acpi_object *bmofdata;
    36		struct bin_attribute bmof_bin_attr;

---
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: 29898 bytes --]

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1676 bytes --]

On Tuesday 06 June 2017 05:16:44 Andy Lutomirski wrote:
> Many laptops (and maybe servers?) have embedded WMI Binary MOF
> metadata. We do not yet have open-source tools for processing the
> data, although one is in the works thanks to Pali:
> 
> 	https://github.com/pali/bmfdec
> 
> There is currently no interface to get the data in the first place.
> By exposing it, we facilitate the development of new tools.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Mario Limonciello <mario_limonciello@dell.com>
> Cc: Pali Rohár <pali.rohar@gmail.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: platform-driver-x86@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
> [dvhart: make sysfs mof binary read only, fixup comment block format]
> [dvhart: use bmof terminology and dev_err instead of dev_warn]
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
> ---
> since-v1:
>  * address Pali's comments:
>    * update the cover letter for clarity and accuracy
>    * update mof->bmof and MOF to Binary MOF throughout the patch
>    * use dev_err instead of dev_warn in wmi_bmof_probe
> 
> 
>  drivers/platform/x86/Kconfig    |  12 ++++
>  drivers/platform/x86/Makefile   |   1 +
>  drivers/platform/x86/wmi-bmof.c | 125

Another suggestion (unrelated to this patch): For working with ACPI-WMI, 
this binary MOF buffer is not enough. It is needed also content of _WDG 
buffer. What about exporting it too via sysfs? Probably not part of of 
wmi-bmof driver, but wmi driver itself.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

RE: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Mario.Limonciello]

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Monday, June 19, 2017 11:08 AM
> To: Andy Lutomirski <luto@kernel.org>
> Cc: platform-driver-x86@vger.kernel.org; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>; Andy Lutomirski <luto@amacapital.net>;
> Limonciello, Mario <Mario_Limonciello@Dell.com>; Rafael Wysocki
> <rjw@rjwysocki.net>; linux-kernel@vger.kernel.org; linux-acpi@vger.kernel.org
> Subject: Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded
> Binary WMI MOF metadata
> 
> On Tuesday 06 June 2017 05:16:44 Andy Lutomirski wrote:
> > Many laptops (and maybe servers?) have embedded WMI Binary MOF
> > metadata. We do not yet have open-source tools for processing the
> > data, although one is in the works thanks to Pali:
> >
> > 	https://github.com/pali/bmfdec
> >
> > There is currently no interface to get the data in the first place.
> > By exposing it, we facilitate the development of new tools.
> >
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Mario Limonciello <mario_limonciello@dell.com>
> > Cc: Pali Rohár <pali.rohar@gmail.com>
> > Cc: linux-kernel@vger.kernel.org
> > Cc: platform-driver-x86@vger.kernel.org
> > Cc: linux-acpi@vger.kernel.org
> > [dvhart: make sysfs mof binary read only, fixup comment block format]
> > [dvhart: use bmof terminology and dev_err instead of dev_warn]
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
> > ---
> > since-v1:
> >  * address Pali's comments:
> >    * update the cover letter for clarity and accuracy
> >    * update mof->bmof and MOF to Binary MOF throughout the patch
> >    * use dev_err instead of dev_warn in wmi_bmof_probe
> >
> >
> >  drivers/platform/x86/Kconfig    |  12 ++++
> >  drivers/platform/x86/Makefile   |   1 +
> >  drivers/platform/x86/wmi-bmof.c | 125
> 
> Another suggestion (unrelated to this patch): For working with ACPI-WMI,
> this binary MOF buffer is not enough. It is needed also content of _WDG
> buffer. What about exporting it too via sysfs? Probably not part of of
> wmi-bmof driver, but wmi driver itself.
> 

I think this depends upon how the userspace access to WMI methods gets
implemented, no?  If userpsace access to WMI methods show up as
/dev/wmi-$GUID-$INSTANCE and those internally to the kernel map to
the proper ASL methods for example, what you get from wmi-bmof
should be enough shouldn't it?

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 2961 bytes --]

On Monday 19 June 2017 18:13:13 Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > Sent: Monday, June 19, 2017 11:08 AM
> > To: Andy Lutomirski <luto@kernel.org>
> > Cc: platform-driver-x86@vger.kernel.org; Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com>; Andy Lutomirski
> > <luto@amacapital.net>; Limonciello, Mario
> > <Mario_Limonciello@Dell.com>; Rafael Wysocki <rjw@rjwysocki.net>;
> > linux-kernel@vger.kernel.org; linux-acpi@vger.kernel.org Subject:
> > Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose
> > embedded Binary WMI MOF metadata
> > 
> > On Tuesday 06 June 2017 05:16:44 Andy Lutomirski wrote:
> > > Many laptops (and maybe servers?) have embedded WMI Binary MOF
> > > metadata. We do not yet have open-source tools for processing the
> > > 
> > > data, although one is in the works thanks to Pali:
> > > 	https://github.com/pali/bmfdec
> > > 
> > > There is currently no interface to get the data in the first
> > > place. By exposing it, we facilitate the development of new
> > > tools.
> > > 
> > > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > > Cc: Andy Lutomirski <luto@amacapital.net>
> > > Cc: Mario Limonciello <mario_limonciello@dell.com>
> > > Cc: Pali Rohár <pali.rohar@gmail.com>
> > > Cc: linux-kernel@vger.kernel.org
> > > Cc: platform-driver-x86@vger.kernel.org
> > > Cc: linux-acpi@vger.kernel.org
> > > [dvhart: make sysfs mof binary read only, fixup comment block
> > > format] [dvhart: use bmof terminology and dev_err instead of
> > > dev_warn] Acked-by: Rafael J. Wysocki
> > > <rafael.j.wysocki@intel.com> Signed-off-by: Darren Hart (VMware)
> > > <dvhart@infradead.org> ---
> > > 
> > > since-v1:
> > >  * address Pali's comments:
> > >    * update the cover letter for clarity and accuracy
> > >    * update mof->bmof and MOF to Binary MOF throughout the patch
> > >    * use dev_err instead of dev_warn in wmi_bmof_probe
> > >  
> > >  drivers/platform/x86/Kconfig    |  12 ++++
> > >  drivers/platform/x86/Makefile   |   1 +
> > >  drivers/platform/x86/wmi-bmof.c | 125
> > 
> > Another suggestion (unrelated to this patch): For working with
> > ACPI-WMI, this binary MOF buffer is not enough. It is needed also
> > content of _WDG buffer. What about exporting it too via sysfs?
> > Probably not part of of wmi-bmof driver, but wmi driver itself.
> 
> I think this depends upon how the userspace access to WMI methods
> gets implemented, no?  If userpsace access to WMI methods show up as
> /dev/wmi-$GUID-$INSTANCE and those internally to the kernel map to
> the proper ASL methods for example, what you get from wmi-bmof
> should be enough shouldn't it?

Ok. Such interface for userspace application could be enough.

But for debugging purposes or writing new WMI driver it is needed to 
have both _WDG + BMOF.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Andy Lutomirski]

On Mon, Jun 19, 2017 at 9:19 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Monday 19 June 2017 18:13:13 Mario.Limonciello@dell.com wrote:
>> > -----Original Message-----
>> > From: Pali Rohár [mailto:pali.rohar@gmail.com]
>> > Sent: Monday, June 19, 2017 11:08 AM
>> > To: Andy Lutomirski <luto@kernel.org>
>> > Cc: platform-driver-x86@vger.kernel.org; Andy Shevchenko
>> > <andriy.shevchenko@linux.intel.com>; Andy Lutomirski
>> > <luto@amacapital.net>; Limonciello, Mario
>> > <Mario_Limonciello@Dell.com>; Rafael Wysocki <rjw@rjwysocki.net>;
>> > linux-kernel@vger.kernel.org; linux-acpi@vger.kernel.org Subject:
>> > Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose
>> > embedded Binary WMI MOF metadata
>> >
>> > On Tuesday 06 June 2017 05:16:44 Andy Lutomirski wrote:
>> > > Many laptops (and maybe servers?) have embedded WMI Binary MOF
>> > > metadata. We do not yet have open-source tools for processing the
>> > >
>> > > data, although one is in the works thanks to Pali:
>> > >   https://github.com/pali/bmfdec
>> > >
>> > > There is currently no interface to get the data in the first
>> > > place. By exposing it, we facilitate the development of new
>> > > tools.
>> > >
>> > > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> > > Cc: Andy Lutomirski <luto@amacapital.net>
>> > > Cc: Mario Limonciello <mario_limonciello@dell.com>
>> > > Cc: Pali Rohár <pali.rohar@gmail.com>
>> > > Cc: linux-kernel@vger.kernel.org
>> > > Cc: platform-driver-x86@vger.kernel.org
>> > > Cc: linux-acpi@vger.kernel.org
>> > > [dvhart: make sysfs mof binary read only, fixup comment block
>> > > format] [dvhart: use bmof terminology and dev_err instead of
>> > > dev_warn] Acked-by: Rafael J. Wysocki
>> > > <rafael.j.wysocki@intel.com> Signed-off-by: Darren Hart (VMware)
>> > > <dvhart@infradead.org> ---
>> > >
>> > > since-v1:
>> > >  * address Pali's comments:
>> > >    * update the cover letter for clarity and accuracy
>> > >    * update mof->bmof and MOF to Binary MOF throughout the patch
>> > >    * use dev_err instead of dev_warn in wmi_bmof_probe
>> > >
>> > >  drivers/platform/x86/Kconfig    |  12 ++++
>> > >  drivers/platform/x86/Makefile   |   1 +
>> > >  drivers/platform/x86/wmi-bmof.c | 125
>> >
>> > Another suggestion (unrelated to this patch): For working with
>> > ACPI-WMI, this binary MOF buffer is not enough. It is needed also
>> > content of _WDG buffer. What about exporting it too via sysfs?
>> > Probably not part of of wmi-bmof driver, but wmi driver itself.
>>
>> I think this depends upon how the userspace access to WMI methods
>> gets implemented, no?  If userpsace access to WMI methods show up as
>> /dev/wmi-$GUID-$INSTANCE and those internally to the kernel map to
>> the proper ASL methods for example, what you get from wmi-bmof
>> should be enough shouldn't it?
>
> Ok. Such interface for userspace application could be enough.
>
> But for debugging purposes or writing new WMI driver it is needed to
> have both _WDG + BMOF.

With the busification patches applied, all the _WDG data should be
available in sysfs in parsed form.  I have no particular objection to
adding a new sysfs for debugfs file to give the raw binary blob, but
I'm not sure it's needed.

--Andy

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Pali Rohár]

On Monday 19 June 2017 09:23:45 Andy Lutomirski wrote:
> On Mon, Jun 19, 2017 at 9:19 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > On Monday 19 June 2017 18:13:13 Mario.Limonciello@dell.com wrote:
> >> > -----Original Message-----
> >> > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> >> > Sent: Monday, June 19, 2017 11:08 AM
> >> > To: Andy Lutomirski <luto@kernel.org>
> >> > Cc: platform-driver-x86@vger.kernel.org; Andy Shevchenko
> >> > <andriy.shevchenko@linux.intel.com>; Andy Lutomirski
> >> > <luto@amacapital.net>; Limonciello, Mario
> >> > <Mario_Limonciello@Dell.com>; Rafael Wysocki <rjw@rjwysocki.net>;
> >> > linux-kernel@vger.kernel.org; linux-acpi@vger.kernel.org Subject:
> >> > Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose
> >> > embedded Binary WMI MOF metadata
> >> >
> >> > On Tuesday 06 June 2017 05:16:44 Andy Lutomirski wrote:
> >> > > Many laptops (and maybe servers?) have embedded WMI Binary MOF
> >> > > metadata. We do not yet have open-source tools for processing the
> >> > >
> >> > > data, although one is in the works thanks to Pali:
> >> > >   https://github.com/pali/bmfdec
> >> > >
> >> > > There is currently no interface to get the data in the first
> >> > > place. By exposing it, we facilitate the development of new
> >> > > tools.
> >> > >
> >> > > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> >> > > Cc: Andy Lutomirski <luto@amacapital.net>
> >> > > Cc: Mario Limonciello <mario_limonciello@dell.com>
> >> > > Cc: Pali Rohár <pali.rohar@gmail.com>
> >> > > Cc: linux-kernel@vger.kernel.org
> >> > > Cc: platform-driver-x86@vger.kernel.org
> >> > > Cc: linux-acpi@vger.kernel.org
> >> > > [dvhart: make sysfs mof binary read only, fixup comment block
> >> > > format] [dvhart: use bmof terminology and dev_err instead of
> >> > > dev_warn] Acked-by: Rafael J. Wysocki
> >> > > <rafael.j.wysocki@intel.com> Signed-off-by: Darren Hart (VMware)
> >> > > <dvhart@infradead.org> ---
> >> > >
> >> > > since-v1:
> >> > >  * address Pali's comments:
> >> > >    * update the cover letter for clarity and accuracy
> >> > >    * update mof->bmof and MOF to Binary MOF throughout the patch
> >> > >    * use dev_err instead of dev_warn in wmi_bmof_probe
> >> > >
> >> > >  drivers/platform/x86/Kconfig    |  12 ++++
> >> > >  drivers/platform/x86/Makefile   |   1 +
> >> > >  drivers/platform/x86/wmi-bmof.c | 125
> >> >
> >> > Another suggestion (unrelated to this patch): For working with
> >> > ACPI-WMI, this binary MOF buffer is not enough. It is needed also
> >> > content of _WDG buffer. What about exporting it too via sysfs?
> >> > Probably not part of of wmi-bmof driver, but wmi driver itself.
> >>
> >> I think this depends upon how the userspace access to WMI methods
> >> gets implemented, no?  If userpsace access to WMI methods show up as
> >> /dev/wmi-$GUID-$INSTANCE and those internally to the kernel map to
> >> the proper ASL methods for example, what you get from wmi-bmof
> >> should be enough shouldn't it?
> >
> > Ok. Such interface for userspace application could be enough.
> >
> > But for debugging purposes or writing new WMI driver it is needed to
> > have both _WDG + BMOF.
> 
> With the busification patches applied, all the _WDG data should be
> available in sysfs in parsed form.  I have no particular objection to
> adding a new sysfs for debugfs file to give the raw binary blob, but
> I'm not sure it's needed.

I'm thinking about writing userspace tool which print information
mapping from MOF namespace/class/method name to ACPI method. Ideally if
it could parse all WMI data on its own and not depends on new parsed
/sys/ tree structure. From _WDG it is needed to know ACPI ID or WMI
event IDs.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Pali Rohár]

On Tuesday 06 June 2017 22:50:52 Pali Rohár wrote:
> On Tuesday 06 June 2017 19:02:01 Darren Hart wrote:
> > On Tue, Jun 06, 2017 at 12:04:40PM +0200, Pali Rohár wrote:
> > > On Monday 05 June 2017 20:16:44 Andy Lutomirski wrote:
> > > > +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> > > > +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
> > > 
> > > Cannot we generate MODULE_ALIAS from module_wmi_driver()? IIRC it
> > > is working for i2c drivers.
> > 
> > I could see this being automated since we always use wmi:GUID, but it
> > isn't currently. Happy to consider it as a follow on.
> > 
> > Do you have a specific i2c example you think we should consider
> > following?
> 
> For i2c you can specify in driver code:
> 
> MODULE_DEVICE_TABLE(i2c, id_table);
> 
> And it automatically provides (via file.mod.c) all needed MODULE_ALIAS.
> 
> So when we have wmi_bmof_id_table in driver, cannot we use this?
> 
> MODULE_DEVICE_TABLE(wmi, wmi_bmof_id_table);

Just reminder for above idea ↑↑↑

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Pali Rohár]

On Tuesday 04 July 2017 15:28:19 Pali Rohár wrote:
> On Tuesday 06 June 2017 22:50:52 Pali Rohár wrote:
> > On Tuesday 06 June 2017 19:02:01 Darren Hart wrote:
> > > On Tue, Jun 06, 2017 at 12:04:40PM +0200, Pali Rohár wrote:
> > > > On Monday 05 June 2017 20:16:44 Andy Lutomirski wrote:
> > > > > +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> > > > > +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
> > > > 
> > > > Cannot we generate MODULE_ALIAS from module_wmi_driver()? IIRC it
> > > > is working for i2c drivers.
> > > 
> > > I could see this being automated since we always use wmi:GUID, but it
> > > isn't currently. Happy to consider it as a follow on.
> > > 
> > > Do you have a specific i2c example you think we should consider
> > > following?
> > 
> > For i2c you can specify in driver code:
> > 
> > MODULE_DEVICE_TABLE(i2c, id_table);
> > 
> > And it automatically provides (via file.mod.c) all needed MODULE_ALIAS.
> > 
> > So when we have wmi_bmof_id_table in driver, cannot we use this?
> > 
> > MODULE_DEVICE_TABLE(wmi, wmi_bmof_id_table);
> 
> Just reminder for above idea ↑↑↑

Hi! This email is some months old, so do not know if something was
implemented or not. Does somebody know?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

[Andy Lutomirski]

On Thu, Nov 23, 2017 at 6:39 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Tuesday 04 July 2017 15:28:19 Pali Rohár wrote:
>> On Tuesday 06 June 2017 22:50:52 Pali Rohár wrote:
>> > On Tuesday 06 June 2017 19:02:01 Darren Hart wrote:
>> > > On Tue, Jun 06, 2017 at 12:04:40PM +0200, Pali Rohár wrote:
>> > > > On Monday 05 June 2017 20:16:44 Andy Lutomirski wrote:
>> > > > > +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
>> > > > > +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
>> > > >
>> > > > Cannot we generate MODULE_ALIAS from module_wmi_driver()? IIRC it
>> > > > is working for i2c drivers.
>> > >
>> > > I could see this being automated since we always use wmi:GUID, but it
>> > > isn't currently. Happy to consider it as a follow on.
>> > >
>> > > Do you have a specific i2c example you think we should consider
>> > > following?
>> >
>> > For i2c you can specify in driver code:
>> >
>> > MODULE_DEVICE_TABLE(i2c, id_table);
>> >
>> > And it automatically provides (via file.mod.c) all needed MODULE_ALIAS.
>> >
>> > So when we have wmi_bmof_id_table in driver, cannot we use this?
>> >
>> > MODULE_DEVICE_TABLE(wmi, wmi_bmof_id_table);
>>
>> Just reminder for above idea ↑↑↑
>
> Hi! This email is some months old, so do not know if something was
> implemented or not. Does somebody know?
>

I don't think so.  I was way too lazy.