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 <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Mario Limonciello <[email protected]>
Cc: Pali Roh?r <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
[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 <[email protected]>
Signed-off-by: Darren Hart (VMware) <[email protected]>
---
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 <[email protected]>");
+MODULE_DESCRIPTION("WMI embedded Binary MOF driver");
+MODULE_LICENSE("GPL");
--
2.9.4
--
Darren Hart
VMware Open Source Technology Center
On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <[email protected]> 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 <[email protected]>
> +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
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
[email protected]
On Tue, Jun 06, 2017 at 12:30:38PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <[email protected]> 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 <[email protected]>
>
>
> > +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
On Tue, Jun 06, 2017 at 12:30:38PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <[email protected]> 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 <[email protected]>
>
>
> > +#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
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
On Tue, Jun 6, 2017 at 7:54 PM, Darren Hart <[email protected]> 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 <[email protected]> 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
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
[email protected]
On Mon, Jun 5, 2017 at 8:16 PM, Andy Lutomirski <[email protected]> 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
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
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 <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Mario Limonciello <[email protected]>
> Cc: Pali Rohár <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> [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 <[email protected]>
> Signed-off-by: Darren Hart (VMware) <[email protected]>
> ---
> 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
[email protected]
> -----Original Message-----
> From: Pali Rohár [mailto:[email protected]]
> Sent: Monday, June 19, 2017 11:08 AM
> To: Andy Lutomirski <[email protected]>
> Cc: [email protected]; Andy Shevchenko
> <[email protected]>; Andy Lutomirski <[email protected]>;
> Limonciello, Mario <[email protected]>; Rafael Wysocki
> <[email protected]>; [email protected]; [email protected]
> 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 <[email protected]>
> > Cc: Andy Lutomirski <[email protected]>
> > Cc: Mario Limonciello <[email protected]>
> > Cc: Pali Rohár <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > [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 <[email protected]>
> > Signed-off-by: Darren Hart (VMware) <[email protected]>
> > ---
> > 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?
On Monday 19 June 2017 18:13:13 [email protected] wrote:
> > -----Original Message-----
> > From: Pali Rohár [mailto:[email protected]]
> > Sent: Monday, June 19, 2017 11:08 AM
> > To: Andy Lutomirski <[email protected]>
> > Cc: [email protected]; Andy Shevchenko
> > <[email protected]>; Andy Lutomirski
> > <[email protected]>; Limonciello, Mario
> > <[email protected]>; Rafael Wysocki <[email protected]>;
> > [email protected]; [email protected] 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 <[email protected]>
> > > Cc: Andy Lutomirski <[email protected]>
> > > Cc: Mario Limonciello <[email protected]>
> > > Cc: Pali Rohár <[email protected]>
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > [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
> > > <[email protected]> Signed-off-by: Darren Hart (VMware)
> > > <[email protected]> ---
> > >
> > > 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
[email protected]
On Mon, Jun 19, 2017 at 9:19 AM, Pali Rohár <[email protected]> wrote:
> On Monday 19 June 2017 18:13:13 [email protected] wrote:
>> > -----Original Message-----
>> > From: Pali Rohár [mailto:[email protected]]
>> > Sent: Monday, June 19, 2017 11:08 AM
>> > To: Andy Lutomirski <[email protected]>
>> > Cc: [email protected]; Andy Shevchenko
>> > <[email protected]>; Andy Lutomirski
>> > <[email protected]>; Limonciello, Mario
>> > <[email protected]>; Rafael Wysocki <[email protected]>;
>> > [email protected]; [email protected] 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 <[email protected]>
>> > > Cc: Andy Lutomirski <[email protected]>
>> > > Cc: Mario Limonciello <[email protected]>
>> > > Cc: Pali Rohár <[email protected]>
>> > > Cc: [email protected]
>> > > Cc: [email protected]
>> > > Cc: [email protected]
>> > > [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
>> > > <[email protected]> Signed-off-by: Darren Hart (VMware)
>> > > <[email protected]> ---
>> > >
>> > > 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
On Monday 19 June 2017 09:23:45 Andy Lutomirski wrote:
> On Mon, Jun 19, 2017 at 9:19 AM, Pali Rohár <[email protected]> wrote:
> > On Monday 19 June 2017 18:13:13 [email protected] wrote:
> >> > -----Original Message-----
> >> > From: Pali Rohár [mailto:[email protected]]
> >> > Sent: Monday, June 19, 2017 11:08 AM
> >> > To: Andy Lutomirski <[email protected]>
> >> > Cc: [email protected]; Andy Shevchenko
> >> > <[email protected]>; Andy Lutomirski
> >> > <[email protected]>; Limonciello, Mario
> >> > <[email protected]>; Rafael Wysocki <[email protected]>;
> >> > [email protected]; [email protected] 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 <[email protected]>
> >> > > Cc: Andy Lutomirski <[email protected]>
> >> > > Cc: Mario Limonciello <[email protected]>
> >> > > Cc: Pali Rohár <[email protected]>
> >> > > Cc: [email protected]
> >> > > Cc: [email protected]
> >> > > Cc: [email protected]
> >> > > [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
> >> > > <[email protected]> Signed-off-by: Darren Hart (VMware)
> >> > > <[email protected]> ---
> >> > >
> >> > > 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
[email protected]
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
[email protected]
On Thu, Nov 23, 2017 at 6:39 AM, Pali Rohár <[email protected]> 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.
From 1584868177549666797@xxx Thu Nov 23 14:41:15 +0000 2017
X-GM-THRID: 1571998862568179745
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
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
[email protected]
From 1571998862568179745@xxx Tue Jul 04 13:29:00 +0000 2017
X-GM-THRID: 1571998862568179745
X-Gmail-Labels: Inbox,Category Forums