[PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI

[PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI

[Mario Limonciello]

The existing way that the dell-smbios helper module and associated
other drivers (dell-laptop, dell-wmi) communicate with the platform
really isn't secure.  It requires creating a buffer in physical
DMA32 memory space and passing that to the platform via SMM.

Since the platform got a physical memory pointer, you've just got
to trust that the platform has only modified (and accessed) memory
within that buffer.

Dell Platform designers recognize this security risk and offer a
safer way to communicate with the platform over ACPI.  This is
in turn exposed via a WMI interface to the OS.

When communicating over WMI-ACPI the communication doesn't occur
with physical memory pointers.  When the ASL is invoked, the fixed
length ACPI buffer is copied to a small operating region.  The ASL
will invoke the SMI, and SMM will only have access to this operating
region.  When the ASL returns the buffer is copied back for the OS
to process.

This method of communication should also deprecate the usage of the
dcdbas kernel module and software dependent upon it's interface.
Instead offer a character device interface for communicating with this
ASL method to allow userspace to use instead.

To faciliate that this patch series introduces a generic way for WMI
drivers to be able to create discoverable character devices through
the WMI bus when desired.
Requiring WMI drivers to explicitly ask for this functionality will
act as an effective vendor whitelist to character device creation.

changes between v1 and v2:
 * Introduce another patch to sort the includes in wmi.c
 * Introduce another patch to cleanup dell_wmi_check_descriptor_buffer
   checks.
 * Add a commit message to the pr_fmt commit
 * Introduce includes to wmi.c in proper location
 * Add Reviewed-by to relevant patches from Pali
 * Make the WMI introduction patch fallback to legacy SMI
   if compiled with CONFIG_DCDBAS
 * Separate format of WMI and SMI buffers.  WMI buffer supports more
   arguments and data.
 * Adjust the rename patch for changes to fallback
 * Drop sysfs token creation patch
 * Adjust WMI descriptor check patch for changes to fallback
 * introduce another patch to remove needless includes in dell-smbios.c
 * Add token ioctl interface to character device.
   - Can query number of tokens
   - Can query values in all tokens
 * Expose format of all buffers and IOCTL commands to uapi header
 * Drop the read interface from character device.  It doesn't make
   sense with multiple different ioctl methods.
 * Default WMI interface to 32k (This would normally be queried via
   MOF, but that's not possible yet)
 * Create separate buffers for WMI and SMI.  If WMI is available,
   free the SMI buffer.
 * Reorder patches so all fixups come first in the series.

Mario Limonciello (14):
  platform/x86: dell-wmi: label driver as handling notifications
  platform/x86: dell-smbios: drop needless includes
  platform/x86: dell-wmi: Don't match on descriptor GUID modalias
  platform/x86: dell-smbios: Add pr_fmt definition to driver
  platform/x86: wmi: sort include list
  platform/x86: wmi: Cleanup exit routine in reverse order of init
  platform/x86: wmi: destroy on cleanup rather than unregister
  platform/x86: dell-smbios: Introduce a WMI-ACPI interface
  platform/x86: dell-smbios: rename to dell-wmi-smbios
  platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check
  platform/x86: wmi: create character devices when requested by drivers
  platform/x86: dell-wmi-smbios: introduce character device for
    userspace
  platform/x86: Kconfig: Change the default settings for dell-wmi-smbios
  platform/x86: dell-wmi-smbios: clean up wmi descriptor check

 Documentation/ABI/testing/dell-wmi-smbios          |  10 +
 MAINTAINERS                                        |   8 +-
 drivers/platform/x86/Kconfig                       |  13 +-
 drivers/platform/x86/Makefile                      |   2 +-
 drivers/platform/x86/dell-laptop.c                 |   2 +-
 drivers/platform/x86/dell-smbios.c                 | 213 ---------
 drivers/platform/x86/dell-wmi-smbios.c             | 481 +++++++++++++++++++++
 .../x86/{dell-smbios.h => dell-wmi-smbios.h}       |  28 +-
 drivers/platform/x86/dell-wmi.c                    |  78 +---
 drivers/platform/x86/wmi.c                         | 116 ++++-
 include/linux/wmi.h                                |   1 +
 include/uapi/linux/dell-wmi-smbios.h               |  55 +++
 12 files changed, 674 insertions(+), 333 deletions(-)
 create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
 delete mode 100644 drivers/platform/x86/dell-smbios.c
 create mode 100644 drivers/platform/x86/dell-wmi-smbios.c
 rename drivers/platform/x86/{dell-smbios.h => dell-wmi-smbios.h} (74%)
 create mode 100644 include/uapi/linux/dell-wmi-smbios.h

-- 
2.14.1

[PATCH v2 01/14] platform/x86: dell-wmi: label driver as handling notifications

[Mario Limonciello]

This driver serves the purpose of responding to WMI based notifications
from the DELL_EVENT_GUID (9DBB5994-A997-11DA-B012-B622A1EF5492).
Other GUIDs will be handled by separate drivers.

Update the language used by this driver to avoid future confusion.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 MAINTAINERS                  | 2 +-
 drivers/platform/x86/Kconfig | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2281af4b41b6..5d8ea24a8ee7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3996,7 +3996,7 @@ S:	Maintained
 F:	Documentation/dcdbas.txt
 F:	drivers/firmware/dcdbas.*
 
-DELL WMI EXTRAS DRIVER
+DELL WMI NOTIFICATIONS DRIVER
 M:	Matthew Garrett <mjg59@srcf.ucam.org>
 M:	Pali Rohár <pali.rohar@gmail.com>
 S:	Maintained
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 80b87954f6dd..9e52f05daa2e 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -116,7 +116,7 @@ config DELL_LAPTOP
 	laptops (except for some models covered by the Compal driver).
 
 config DELL_WMI
-	tristate "Dell WMI extras"
+	tristate "Dell WMI notifications"
 	depends on ACPI_WMI
 	depends on DMI
 	depends on INPUT
-- 
2.14.1

[PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

[Mario Limonciello]

These aren't needed to compile the module.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/dell-smbios.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 0a5723468bff..6654ad44a04c 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -17,10 +17,7 @@
 #include <linux/module.h>
 #include <linux/dmi.h>
 #include <linux/err.h>
-#include <linux/gfp.h>
 #include <linux/mutex.h>
-#include <linux/slab.h>
-#include <linux/io.h>
 #include "../../firmware/dcdbas.h"
 #include "dell-smbios.h"
 
-- 
2.14.1

[PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor GUID modalias

[Mario Limonciello]

The descriptor GUID is not used to indicate that WMI notifications
in the dell-wmi driver work properly.  As such a modalias should
not be present that causes this driver to load on systems with this
GUID.

Reviewed-by: Pali Pohar <pali.rohar@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/dell-wmi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 28d9f8696081..1fbef560ca67 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -51,7 +51,6 @@ MODULE_LICENSE("GPL");
 static bool wmi_requires_smbios_request;
 
 MODULE_ALIAS("wmi:"DELL_EVENT_GUID);
-MODULE_ALIAS("wmi:"DELL_DESCRIPTOR_GUID);
 
 struct dell_wmi_priv {
 	struct input_dev *input_dev;
-- 
2.14.1

[PATCH v2 04/14] platform/x86: dell-smbios: Add pr_fmt definition to driver

[Mario Limonciello]

pr_fmt provides formatting to be used by the driver when
displaying errors and messages.

Reviewed-by: Pali Pohar <pali.rohar@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/dell-smbios.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 6654ad44a04c..579be67de2a3 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -12,6 +12,7 @@
  *  it under the terms of the GNU General Public License version 2 as
  *  published by the Free Software Foundation.
  */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-- 
2.14.1

[PATCH v2 05/14] platform/x86: wmi: sort include list

[Mario Limonciello]

The include list is randomly assembled right now.  Sort in alphabetical
order.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/wmi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 0765b1797d4c..e19b074df01d 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -33,17 +33,17 @@
 
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/types.h>
+#include <linux/acpi.h>
 #include <linux/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
 #include <linux/list.h>
-#include <linux/acpi.h>
-#include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/wmi.h>
+#include <linux/slab.h>
+#include <linux/types.h>
 #include <linux/uuid.h>
+#include <linux/wmi.h>
 
 ACPI_MODULE_NAME("wmi");
 MODULE_AUTHOR("Carlos Corbacho");
-- 
2.14.1

[PATCH v2 06/14] platform/x86: wmi: Cleanup exit routine in reverse order of init

[Mario Limonciello]

The initialize routine is:
* class -> bus -> platform

The exit routine is:
* platform -> class -> bus

Fix the exit routine to be:
* platform -> bus -> class

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/wmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index e19b074df01d..acbc2b02db3d 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -1264,8 +1264,8 @@ static int __init acpi_wmi_init(void)
 static void __exit acpi_wmi_exit(void)
 {
 	platform_driver_unregister(&acpi_wmi_driver);
-	class_unregister(&wmi_bus_class);
 	bus_unregister(&wmi_bus_type);
+	class_unregister(&wmi_bus_class);
 }
 
 subsys_initcall(acpi_wmi_init);
-- 
2.14.1

[PATCH v2 07/14] platform/x86: wmi: destroy on cleanup rather than unregister

[Mario Limonciello]

device_create documentation says to cleanup using device_destroy

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/wmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index acbc2b02db3d..7a05843aff19 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -1145,7 +1145,7 @@ static int acpi_wmi_remove(struct platform_device *device)
 	acpi_remove_address_space_handler(acpi_device->handle,
 				ACPI_ADR_SPACE_EC, &acpi_wmi_ec_space_handler);
 	wmi_free_devices(acpi_device);
-	device_unregister((struct device *)dev_get_drvdata(&device->dev));
+	device_destroy(&wmi_bus_class, MKDEV(0, 0));
 
 	return 0;
 }
@@ -1199,7 +1199,7 @@ static int acpi_wmi_probe(struct platform_device *device)
 	return 0;
 
 err_remove_busdev:
-	device_unregister(wmi_bus_dev);
+	device_destroy(&wmi_bus_class, MKDEV(0, 0));
 
 err_remove_notify_handler:
 	acpi_remove_notify_handler(acpi_device->handle, ACPI_DEVICE_NOTIFY,
-- 
2.14.1

[PATCH v2 08/14] platform/x86: dell-smbios: Introduce a WMI-ACPI interface

[Mario Limonciello]

The driver currently uses an SMI interface which grants direct access
to physical memory to the platform via a pointer.

Now add a WMI-ACPI interface that is detected by WMI probe and preferred
over the SMI interface.

Changing this to operate over WMI-ACPI will use an ACPI OperationRegion
for a buffer of data storage when platform calls are performed.

This is a safer approach to use in kernel drivers as the platform will
only have access to that OperationRegion.

As a result, this change removes the dependency on this driver on the
dcdbas kernel module.  It's now an optional compilation option.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/Kconfig       |   8 +-
 drivers/platform/x86/dell-smbios.c | 150 ++++++++++++++++++++++++++++++-------
 drivers/platform/x86/dell-smbios.h |  15 +++-
 3 files changed, 140 insertions(+), 33 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 9e52f05daa2e..81d61c0f4ef8 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -92,13 +92,13 @@ config ASUS_LAPTOP
 	  If you have an ACPI-compatible ASUS laptop, say Y or M here.
 
 config DELL_SMBIOS
-	tristate
-	select DCDBAS
+	tristate "Dell WMI SMBIOS calling interface"
+	depends on ACPI_WMI
 	---help---
 	This module provides common functions for kernel modules using
-	Dell SMBIOS.
+	Dell SMBIOS over ACPI-WMI.
 
-	If you have a Dell laptop, say Y or M here.
+	If you have a Dell computer, say Y or M here.
 
 config DELL_LAPTOP
 	tristate "Dell Laptop Extras"
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 579be67de2a3..1fbc40791a48 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -4,6 +4,7 @@
  *  Copyright (c) Red Hat <mjg@redhat.com>
  *  Copyright (c) 2014 Gabriele Mazzotta <gabriele.mzt@gmail.com>
  *  Copyright (c) 2014 Pali Rohár <pali.rohar@gmail.com>
+ *  Copyright (c) 2017 Dell Inc.
  *
  *  Based on documentation in the libsmbios package:
  *  Copyright (C) 2005-2014 Dell Inc.
@@ -19,9 +20,15 @@
 #include <linux/dmi.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
-#include "../../firmware/dcdbas.h"
+#include <linux/wmi.h>
 #include "dell-smbios.h"
 
+#ifdef CONFIG_DCDBAS
+#include "../../firmware/dcdbas.h"
+#endif
+
+#define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
+
 struct calling_interface_structure {
 	struct dmi_header header;
 	u16 cmdIOAddress;
@@ -30,12 +37,14 @@ struct calling_interface_structure {
 	struct calling_interface_token tokens[];
 } __packed;
 
-static struct calling_interface_buffer *buffer;
+static struct calling_interface_buffer *smi_buffer;
+static struct wmi_calling_interface_buffer *wmi_buffer;
 static DEFINE_MUTEX(buffer_mutex);
 
 static int da_command_address;
 static int da_command_code;
 static int da_num_tokens;
+static int has_wmi;
 static struct calling_interface_token *da_tokens;
 
 int dell_smbios_error(int value)
@@ -57,13 +66,20 @@ struct calling_interface_buffer *dell_smbios_get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
 	dell_smbios_clear_buffer();
-	return buffer;
+	if (has_wmi)
+		return &wmi_buffer->smi;
+	return smi_buffer;
 }
 EXPORT_SYMBOL_GPL(dell_smbios_get_buffer);
 
 void dell_smbios_clear_buffer(void)
 {
-	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+	if (has_wmi)
+		memset(wmi_buffer, 0,
+		       sizeof(struct wmi_calling_interface_buffer));
+	else
+		memset(smi_buffer, 0,
+		       sizeof(struct calling_interface_buffer));
 }
 EXPORT_SYMBOL_GPL(dell_smbios_clear_buffer);
 
@@ -73,20 +89,60 @@ void dell_smbios_release_buffer(void)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_release_buffer);
 
-void dell_smbios_send_request(int class, int select)
+int run_wmi_smbios_call(struct wmi_calling_interface_buffer *buf)
 {
-	struct smi_cmd command;
+	struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
+	struct acpi_buffer input;
+	union acpi_object *obj;
+	acpi_status status;
+
+	input.length = sizeof(struct wmi_calling_interface_buffer);
+	input.pointer = buf;
+
+	status = wmi_evaluate_method(DELL_WMI_SMBIOS_GUID,
+				     0, 1, &input, &output);
+	if (ACPI_FAILURE(status)) {
+		pr_err("%x/%x [%x,%x,%x,%x] call failed\n",
+			buf->smi.class, buf->smi.select,
+			buf->smi.input[0], buf->smi.input[1],
+			buf->smi.input[2], buf->smi.input[3]);
+			return -EIO;
+	}
+	obj = (union acpi_object *)output.pointer;
+	if (obj->type != ACPI_TYPE_BUFFER) {
+		pr_err("invalid type : %d\n", obj->type);
+		return -EIO;
+	}
+	memcpy(buf, obj->buffer.pointer, input.length);
 
-	command.magic = SMI_CMD_MAGIC;
-	command.command_address = da_command_address;
-	command.command_code = da_command_code;
-	command.ebx = virt_to_phys(buffer);
-	command.ecx = 0x42534931;
+	return 0;
+}
 
-	buffer->class = class;
-	buffer->select = select;
+void dell_smbios_send_request(int class, int select)
+{
+	if (has_wmi) {
+		wmi_buffer->smi.class = class;
+		wmi_buffer->smi.select = select;
+		run_wmi_smbios_call(wmi_buffer);
+	}
 
-	dcdbas_smi_request(&command);
+#ifdef CONFIG_DCDBAS
+	else {
+		if (!smi_buffer)
+			return;
+		struct smi_cmd command;
+
+		smi_buffer->class = class;
+		smi_buffer->select = select;
+		command.magic = SMI_CMD_MAGIC;
+		command.command_address = da_command_address;
+		command.command_code = da_command_code;
+		command.ebx = virt_to_phys(smi_buffer);
+		command.ecx = 0x42534931;
+
+		dcdbas_smi_request(&command);
+	}
+#endif
 }
 EXPORT_SYMBOL_GPL(dell_smbios_send_request);
 
@@ -167,10 +223,45 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy)
 	}
 }
 
-static int __init dell_smbios_init(void)
+static int dell_smbios_wmi_probe(struct wmi_device *wdev)
 {
-	int ret;
+	/* WMI buffer should be 32k */
+	wmi_buffer = (void *)__get_free_pages(GFP_KERNEL, 3);
+	if (!wmi_buffer)
+		return -ENOMEM;
+
+#ifdef CONFIG_DCDBAS
+	/* no longer need the SMI page */
+	free_page((unsigned long)smi_buffer);
+	smi_buffer = NULL;
+#endif
+
+	has_wmi = 1;
+	return 0;
+}
+
+static int dell_smbios_wmi_remove(struct wmi_device *wdev)
+{
+	free_pages((unsigned long)wmi_buffer, 3);
+	return 0;
+}
+
+static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
+	{ .guid_string = DELL_WMI_SMBIOS_GUID },
+	{ },
+};
 
+static struct wmi_driver dell_wmi_smbios_driver = {
+	.driver = {
+		.name = "dell-smbios",
+	},
+	.probe = dell_smbios_wmi_probe,
+	.remove = dell_smbios_wmi_remove,
+	.id_table = dell_smbios_wmi_id_table,
+};
+
+static int __init dell_smbios_init(void)
+{
 	dmi_walk(find_tokens, NULL);
 
 	if (!da_tokens)  {
@@ -178,34 +269,41 @@ static int __init dell_smbios_init(void)
 		return -ENODEV;
 	}
 
+#ifdef CONFIG_DCDBAS
 	/*
 	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
 	 * is passed to SMI handler.
 	 */
-	buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
-	if (!buffer) {
-		ret = -ENOMEM;
-		goto fail_buffer;
+	smi_buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
+#else
+	smi_buffer = NULL;
+#endif
+	wmi_driver_register(&dell_wmi_smbios_driver);
+
+	if (!smi_buffer && !has_wmi) {
+		kfree(da_tokens);
+		return -ENOMEM;
 	}
-
 	return 0;
-
-fail_buffer:
-	kfree(da_tokens);
-	return ret;
 }
 
 static void __exit dell_smbios_exit(void)
 {
 	kfree(da_tokens);
-	free_page((unsigned long)buffer);
+#ifdef CONFIG_DCDBAS
+	if (!has_wmi)
+		free_page((unsigned long)smi_buffer);
+#endif
+	wmi_driver_unregister(&dell_wmi_smbios_driver);
 }
 
 subsys_initcall(dell_smbios_init);
 module_exit(dell_smbios_exit);
 
+
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
 MODULE_AUTHOR("Gabriele Mazzotta <gabriele.mzt@gmail.com>");
 MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
+MODULE_AUTHOR("Mario Limonciello <mario.limonciello@dell.com>");
 MODULE_DESCRIPTION("Common functions for kernel modules using Dell SMBIOS");
 MODULE_LICENSE("GPL");
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 45cbc2292cd3..2f6fce81ee69 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -4,6 +4,7 @@
  *  Copyright (c) Red Hat <mjg@redhat.com>
  *  Copyright (c) 2014 Gabriele Mazzotta <gabriele.mzt@gmail.com>
  *  Copyright (c) 2014 Pali Rohár <pali.rohar@gmail.com>
+ *  Copyright (c) 2017 Dell Inc.
  *
  *  Based on documentation in the libsmbios package:
  *  Copyright (C) 2005-2014 Dell Inc.
@@ -18,9 +19,10 @@
 
 struct notifier_block;
 
-/* This structure will be modified by the firmware when we enter
- * system management mode, hence the volatiles */
-
+/* If called through fallback SMI rather than WMI this structure will be
+ * modified by the firmware when we enter system management mode, hence the
+ * volatiles
+ */
 struct calling_interface_buffer {
 	u16 class;
 	u16 select;
@@ -28,6 +30,13 @@ struct calling_interface_buffer {
 	volatile u32 output[4];
 } __packed;
 
+struct wmi_calling_interface_buffer {
+	struct calling_interface_buffer smi;
+	u32 argattrib;
+	u32 blength;
+	u8 data[32724];
+} __packed;
+
 struct calling_interface_token {
 	u16 tokenID;
 	u16 location;
-- 
2.14.1

[PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios

[Mario Limonciello]

This follows the style of the rest of the platform x86 WMI drivers.

Renaming the driver requires adjusting the other drivers using
dell-smbios to pick up the newly named includes.

While renaming, I noticed that this driver was missing from
MAINTAINERS. Add it to that and myself to the list of people maintaing
it.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 MAINTAINERS                                               |  6 ++++++
 drivers/platform/x86/Kconfig                              |  2 +-
 drivers/platform/x86/Makefile                             |  2 +-
 drivers/platform/x86/dell-laptop.c                        |  2 +-
 drivers/platform/x86/{dell-smbios.c => dell-wmi-smbios.c} | 12 ++++++------
 drivers/platform/x86/{dell-smbios.h => dell-wmi-smbios.h} |  4 ++--
 drivers/platform/x86/dell-wmi.c                           |  2 +-
 7 files changed, 18 insertions(+), 12 deletions(-)
 rename drivers/platform/x86/{dell-smbios.c => dell-wmi-smbios.c} (97%)
 rename drivers/platform/x86/{dell-smbios.h => dell-wmi-smbios.h} (96%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5d8ea24a8ee7..437daa9062e1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4002,6 +4002,12 @@ M:	Pali Rohár <pali.rohar@gmail.com>
 S:	Maintained
 F:	drivers/platform/x86/dell-wmi.c
 
+DELL WMI SMBIOS DRIVER
+M:	Pali Rohár <pali.rohar@gmail.com>
+M:	Mario Limonciello <mario.limonciello@dell.com>
+S:	Maintained
+F:	drivers/platform/x86/dell-wmi-smbios.c
+
 DELTA ST MEDIA DRIVER
 M:	Hugues Fruchet <hugues.fruchet@st.com>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 81d61c0f4ef8..a70bcd8caa72 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -91,7 +91,7 @@ config ASUS_LAPTOP
 
 	  If you have an ACPI-compatible ASUS laptop, say Y or M here.
 
-config DELL_SMBIOS
+config DELL_WMI_SMBIOS
 	tristate "Dell WMI SMBIOS calling interface"
 	depends on ACPI_WMI
 	---help---
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 91cec1751461..b127a3bc1fab 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -11,11 +11,11 @@ obj-$(CONFIG_EEEPC_WMI)		+= eeepc-wmi.o
 obj-$(CONFIG_MSI_LAPTOP)	+= msi-laptop.o
 obj-$(CONFIG_ACPI_CMPC)		+= classmate-laptop.o
 obj-$(CONFIG_COMPAL_LAPTOP)	+= compal-laptop.o
-obj-$(CONFIG_DELL_SMBIOS)	+= dell-smbios.o
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
 obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
 obj-$(CONFIG_DELL_WMI_AIO)	+= dell-wmi-aio.o
 obj-$(CONFIG_DELL_WMI_LED)	+= dell-wmi-led.o
+obj-$(CONFIG_DELL_WMI_SMBIOS)	+= dell-wmi-smbios.o
 obj-$(CONFIG_DELL_SMO8800)	+= dell-smo8800.o
 obj-$(CONFIG_DELL_RBTN)		+= dell-rbtn.o
 obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index f42159fd2031..bf569ea93e9d 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -33,7 +33,7 @@
 #include <linux/seq_file.h>
 #include <acpi/video.h>
 #include "dell-rbtn.h"
-#include "dell-smbios.h"
+#include "dell-wmi-smbios.h"
 
 #define BRIGHTNESS_TOKEN 0x7d
 #define KBD_LED_OFF_TOKEN 0x01E1
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-wmi-smbios.c
similarity index 97%
rename from drivers/platform/x86/dell-smbios.c
rename to drivers/platform/x86/dell-wmi-smbios.c
index 1fbc40791a48..b0812a8fa860 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-wmi-smbios.c
@@ -21,7 +21,7 @@
 #include <linux/err.h>
 #include <linux/mutex.h>
 #include <linux/wmi.h>
-#include "dell-smbios.h"
+#include "dell-wmi-smbios.h"
 
 #ifdef CONFIG_DCDBAS
 #include "../../firmware/dcdbas.h"
@@ -253,14 +253,14 @@ static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
 
 static struct wmi_driver dell_wmi_smbios_driver = {
 	.driver = {
-		.name = "dell-smbios",
+		.name = "dell-wmi-smbios",
 	},
 	.probe = dell_smbios_wmi_probe,
 	.remove = dell_smbios_wmi_remove,
 	.id_table = dell_smbios_wmi_id_table,
 };
 
-static int __init dell_smbios_init(void)
+static int __init dell_wmi_smbios_init(void)
 {
 	dmi_walk(find_tokens, NULL);
 
@@ -287,7 +287,7 @@ static int __init dell_smbios_init(void)
 	return 0;
 }
 
-static void __exit dell_smbios_exit(void)
+static void __exit dell_wmi_smbios_exit(void)
 {
 	kfree(da_tokens);
 #ifdef CONFIG_DCDBAS
@@ -297,8 +297,8 @@ static void __exit dell_smbios_exit(void)
 	wmi_driver_unregister(&dell_wmi_smbios_driver);
 }
 
-subsys_initcall(dell_smbios_init);
-module_exit(dell_smbios_exit);
+subsys_initcall(dell_wmi_smbios_init);
+module_exit(dell_wmi_smbios_exit);
 
 
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-wmi-smbios.h
similarity index 96%
rename from drivers/platform/x86/dell-smbios.h
rename to drivers/platform/x86/dell-wmi-smbios.h
index 2f6fce81ee69..14b7e2ece310 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-wmi-smbios.h
@@ -14,8 +14,8 @@
  *  published by the Free Software Foundation.
  */
 
-#ifndef _DELL_SMBIOS_H_
-#define _DELL_SMBIOS_H_
+#ifndef _DELL_WMI_SMBIOS_H_
+#define _DELL_WMI_SMBIOS_H_
 
 struct notifier_block;
 
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 1fbef560ca67..e8b4d412eabc 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -38,7 +38,7 @@
 #include <linux/dmi.h>
 #include <linux/wmi.h>
 #include <acpi/video.h>
-#include "dell-smbios.h"
+#include "dell-wmi-smbios.h"
 
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
 MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
-- 
2.14.1

[PATCH v2 10/14] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check

[Mario Limonciello]

The Dell WMI descriptor check is used as an indication that WMI
calls are safe to run both when used with the notification
ASL/GUID pair as well as the SMBIOS calling ASL/GUID pair.

As some code in dell-wmi-smbios is already a prerequisite for
dell-wmi, move the code for performing the descriptor check into
dell-wmi-smbios and let both drivers use it from there.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/dell-wmi-smbios.c | 84 ++++++++++++++++++++++++++++++++++
 drivers/platform/x86/dell-wmi-smbios.h |  3 ++
 drivers/platform/x86/dell-wmi.c        | 75 +-----------------------------
 3 files changed, 88 insertions(+), 74 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi-smbios.c b/drivers/platform/x86/dell-wmi-smbios.c
index b0812a8fa860..699757f3e154 100644
--- a/drivers/platform/x86/dell-wmi-smbios.c
+++ b/drivers/platform/x86/dell-wmi-smbios.c
@@ -28,6 +28,7 @@
 #endif
 
 #define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
+#define DELL_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
 
 struct calling_interface_structure {
 	struct dmi_header header;
@@ -223,13 +224,92 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy)
 	}
 }
 
+/*
+ * Descriptor buffer is 128 byte long and contains:
+ *
+ *       Name             Offset  Length  Value
+ * Vendor Signature          0       4    "DELL"
+ * Object Signature          4       4    " WMI"
+ * WMI Interface Version     8       4    <version>
+ * WMI buffer length        12       4    4096
+ */
+int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev, u32 *version)
+{
+	union acpi_object *obj = NULL;
+	struct wmi_device *desc_dev;
+	u32 *desc_buffer;
+	int ret;
+
+	desc_dev = wmidev_get_other_guid(wdev, DELL_DESCRIPTOR_GUID);
+	if (!desc_dev) {
+		dev_err(&wdev->dev, "Dell WMI descriptor does not exist\n");
+		return -ENODEV;
+	}
+
+	obj = wmidev_block_query(desc_dev, 0);
+	if (!obj) {
+		dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
+		ret = -EIO;
+		goto out;
+	}
+
+	if (obj->type != ACPI_TYPE_BUFFER) {
+		dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (obj->buffer.length != 128) {
+		dev_err(&wdev->dev,
+			"Dell descriptor buffer has invalid length (%d)\n",
+			obj->buffer.length);
+		if (obj->buffer.length < 16) {
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+	desc_buffer = (u32 *)obj->buffer.pointer;
+
+	if (desc_buffer[0] != 0x4C4C4544 && desc_buffer[1] != 0x494D5720)
+		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid signature (%*ph)\n",
+			8, desc_buffer);
+
+	if (desc_buffer[2] != 0 && desc_buffer[2] != 1)
+		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%d)\n",
+			desc_buffer[2]);
+
+	if (desc_buffer[3] != 4096)
+		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid buffer length (%d)\n",
+			desc_buffer[3]);
+
+	*version = desc_buffer[2];
+	ret = 0;
+
+	dev_info(&wdev->dev, "Detected Dell WMI interface version %u\n",
+		*version);
+
+out:
+	kfree(obj);
+	put_device(&desc_dev->dev);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dell_wmi_check_descriptor_buffer);
+
+
 static int dell_smbios_wmi_probe(struct wmi_device *wdev)
 {
+	int ret;
+	u32 interface_version;
+
 	/* WMI buffer should be 32k */
 	wmi_buffer = (void *)__get_free_pages(GFP_KERNEL, 3);
 	if (!wmi_buffer)
 		return -ENOMEM;
 
+	ret = dell_wmi_check_descriptor_buffer(wdev, &interface_version);
+	if (ret)
+		goto fail_wmi_probe;
+
 #ifdef CONFIG_DCDBAS
 	/* no longer need the SMI page */
 	free_page((unsigned long)smi_buffer);
@@ -238,6 +318,10 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev)
 
 	has_wmi = 1;
 	return 0;
+
+fail_wmi_probe:
+	free_pages((unsigned long)wmi_buffer, 3);
+	return ret;
 }
 
 static int dell_smbios_wmi_remove(struct wmi_device *wdev)
diff --git a/drivers/platform/x86/dell-wmi-smbios.h b/drivers/platform/x86/dell-wmi-smbios.h
index 14b7e2ece310..0e8b57f7d0f0 100644
--- a/drivers/platform/x86/dell-wmi-smbios.h
+++ b/drivers/platform/x86/dell-wmi-smbios.h
@@ -17,6 +17,8 @@
 #ifndef _DELL_WMI_SMBIOS_H_
 #define _DELL_WMI_SMBIOS_H_
 
+#include <linux/wmi.h>
+
 struct notifier_block;
 
 /* If called through fallback SMI rather than WMI this structure will be
@@ -62,5 +64,6 @@ enum dell_laptop_notifier_actions {
 int dell_laptop_register_notifier(struct notifier_block *nb);
 int dell_laptop_unregister_notifier(struct notifier_block *nb);
 void dell_laptop_call_notifier(unsigned long action, void *data);
+int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev, u32 *version);
 
 #endif
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index e8b4d412eabc..e7011792127f 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -46,7 +46,6 @@ MODULE_DESCRIPTION("Dell laptop WMI hotkeys driver");
 MODULE_LICENSE("GPL");
 
 #define DELL_EVENT_GUID "9DBB5994-A997-11DA-B012-B622A1EF5492"
-#define DELL_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
 
 static bool wmi_requires_smbios_request;
 
@@ -617,78 +616,6 @@ static void dell_wmi_input_destroy(struct wmi_device *wdev)
 	input_unregister_device(priv->input_dev);
 }
 
-/*
- * Descriptor buffer is 128 byte long and contains:
- *
- *       Name             Offset  Length  Value
- * Vendor Signature          0       4    "DELL"
- * Object Signature          4       4    " WMI"
- * WMI Interface Version     8       4    <version>
- * WMI buffer length        12       4    4096
- */
-static int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev)
-{
-	struct dell_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
-	union acpi_object *obj = NULL;
-	struct wmi_device *desc_dev;
-	u32 *buffer;
-	int ret;
-
-	desc_dev = wmidev_get_other_guid(wdev, DELL_DESCRIPTOR_GUID);
-	if (!desc_dev) {
-		dev_err(&wdev->dev, "Dell WMI descriptor does not exist\n");
-		return -ENODEV;
-	}
-
-	obj = wmidev_block_query(desc_dev, 0);
-	if (!obj) {
-		dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
-		ret = -EIO;
-		goto out;
-	}
-
-	if (obj->type != ACPI_TYPE_BUFFER) {
-		dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
-		ret = -EINVAL;
-		goto out;
-	}
-
-	if (obj->buffer.length != 128) {
-		dev_err(&wdev->dev,
-			"Dell descriptor buffer has invalid length (%d)\n",
-			obj->buffer.length);
-		if (obj->buffer.length < 16) {
-			ret = -EINVAL;
-			goto out;
-		}
-	}
-
-	buffer = (u32 *)obj->buffer.pointer;
-
-	if (buffer[0] != 0x4C4C4544 && buffer[1] != 0x494D5720)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid signature (%*ph)\n",
-			8, buffer);
-
-	if (buffer[2] != 0 && buffer[2] != 1)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%d)\n",
-			buffer[2]);
-
-	if (buffer[3] != 4096)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid buffer length (%d)\n",
-			buffer[3]);
-
-	priv->interface_version = buffer[2];
-	ret = 0;
-
-	dev_info(&wdev->dev, "Detected Dell WMI interface version %u\n",
-		priv->interface_version);
-
-out:
-	kfree(obj);
-	put_device(&desc_dev->dev);
-	return ret;
-}
-
 /*
  * According to Dell SMBIOS documentation:
  *
@@ -732,7 +659,7 @@ static int dell_wmi_probe(struct wmi_device *wdev)
 		return -ENOMEM;
 	dev_set_drvdata(&wdev->dev, priv);
 
-	err = dell_wmi_check_descriptor_buffer(wdev);
+	err = dell_wmi_check_descriptor_buffer(wdev, &priv->interface_version);
 	if (err)
 		return err;
 
-- 
2.14.1

[PATCH v2 11/14] platform/x86: wmi: create character devices when requested by drivers

[Mario Limonciello]

For WMI operations that are only Set or Query read or write sysfs
attributes created by WMI vendor drivers make sense.

For other WMI operations that are run on Method, there needs to be a
way to guarantee to userspace that the results from the method call
belong to the data request to the method call.  Sysfs attributes don't
work well in this scenario because two userspace processes may be
competing at reading/writing an attribute and step on each other's
data.

When a WMI vendor driver declares a set of functions in a
file_operations object the WMI bus driver will create a character
device that maps to those file operations.

The WMI vendor drivers will be responsible for managing access to
this character device and proper locking on it.

When a WMI vendor driver is unloaded the WMI bus driver will clean
up the character device.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/wmi.c | 98 +++++++++++++++++++++++++++++++++++++++++++---
 include/linux/wmi.h        |  1 +
 2 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 7a05843aff19..5495360ad2da 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -34,7 +34,9 @@
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
 #include <linux/acpi.h>
+#include <linux/cdev.h>
 #include <linux/device.h>
+#include <linux/idr.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
@@ -50,6 +52,9 @@ MODULE_AUTHOR("Carlos Corbacho");
 MODULE_DESCRIPTION("ACPI-WMI Mapping Driver");
 MODULE_LICENSE("GPL");
 
+#define WMI_MAX_DEVS  MINORMASK
+static DEFINE_IDR(wmi_idr);
+static DEFINE_MUTEX(wmi_minor_lock);
 static LIST_HEAD(wmi_block_list);
 
 struct guid_block {
@@ -69,6 +74,8 @@ struct wmi_block {
 	struct wmi_device dev;
 	struct list_head list;
 	struct guid_block gblock;
+	struct cdev *cdev;
+	int minor;
 	struct acpi_device *acpi_device;
 	wmi_notify_handler handler;
 	void *handler_data;
@@ -86,6 +93,7 @@ struct wmi_block {
 #define ACPI_WMI_STRING      0x4	/* GUID takes & returns a string */
 #define ACPI_WMI_EVENT       0x8	/* GUID is an event */
 
+static dev_t wmi_devt;
 static bool debug_event;
 module_param(debug_event, bool, 0444);
 MODULE_PARM_DESC(debug_event,
@@ -762,21 +770,88 @@ static int wmi_dev_match(struct device *dev, struct device_driver *driver)
 	return 0;
 }
 
+static struct class wmi_bus_class = {
+	.name = "wmi_bus",
+};
+
+static int wmi_minor_get(struct wmi_block *wblock)
+{
+	int ret;
+
+	mutex_lock(&wmi_minor_lock);
+	ret = idr_alloc(&wmi_idr, wblock, 0, WMI_MAX_DEVS, GFP_KERNEL);
+	if (ret >= 0)
+		wblock->minor = ret;
+	else if (ret == -ENOSPC)
+		dev_err(&wblock->dev.dev, "too many wmi devices\n");
+
+	mutex_unlock(&wmi_minor_lock);
+	return ret;
+}
+
+static void wmi_minor_free(struct wmi_block *wblock)
+{
+	mutex_lock(&wmi_minor_lock);
+	idr_remove(&wmi_idr, wblock->minor);
+	mutex_unlock(&wmi_minor_lock);
+}
+
+
 static int wmi_dev_probe(struct device *dev)
 {
 	struct wmi_block *wblock = dev_to_wblock(dev);
 	struct wmi_driver *wdriver =
 		container_of(dev->driver, struct wmi_driver, driver);
-	int ret = 0;
+	struct device *clsdev;
+	int ret = 0, devno;
 
 	if (ACPI_FAILURE(wmi_method_enable(wblock, 1)))
 		dev_warn(dev, "failed to enable device -- probing anyway\n");
 
+	/* driver wants a character device made */
+	if (wdriver->file_operations) {
+		dev->devt = wmi_devt;
+		wblock->cdev = cdev_alloc();
+		if (!wblock->cdev) {
+			dev_err(dev, "failed to allocate cdev\n");
+			return -ENOMEM;
+		}
+		cdev_init(wblock->cdev, wdriver->file_operations);
+		wblock->cdev->owner = wdriver->file_operations->owner;
+		ret = wmi_minor_get(wblock);
+		if (ret < 0)
+			return ret;
+		devno = MKDEV(MAJOR(wmi_devt), wblock->minor);
+		ret = cdev_add(wblock->cdev, devno, 1);
+		if (ret) {
+			dev_err(dev, "unable to create device %d:%d\n",
+			MAJOR(wmi_devt), wblock->minor);
+			goto err_probe_cdev;
+		}
+		clsdev = device_create(&wmi_bus_class, dev,
+				       MKDEV(MAJOR(wmi_devt), wblock->minor),
+				       NULL, "wmi-%s", wdriver->driver.name);
+
+		if (IS_ERR(clsdev)) {
+			dev_err(dev, "unable to create device %d:%d\n",
+			MAJOR(wmi_devt), wblock->minor);
+			ret = PTR_ERR(clsdev);
+			goto err_probe_class;
+		}
+	}
+
 	if (wdriver->probe) {
 		ret = wdriver->probe(dev_to_wdev(dev));
 		if (ret != 0 && ACPI_FAILURE(wmi_method_enable(wblock, 0)))
 			dev_warn(dev, "failed to disable device\n");
 	}
+	return ret;
+
+err_probe_class:
+	cdev_del(wblock->cdev);
+
+err_probe_cdev:
+	wmi_minor_free(wblock);
 
 	return ret;
 }
@@ -788,6 +863,13 @@ static int wmi_dev_remove(struct device *dev)
 		container_of(dev->driver, struct wmi_driver, driver);
 	int ret = 0;
 
+	if (wdriver->file_operations) {
+		device_destroy(&wmi_bus_class,
+			       MKDEV(MAJOR(wmi_devt), wblock->minor));
+		cdev_del(wblock->cdev);
+		wmi_minor_free(wblock);
+	}
+
 	if (wdriver->remove)
 		ret = wdriver->remove(dev_to_wdev(dev));
 
@@ -797,10 +879,6 @@ static int wmi_dev_remove(struct device *dev)
 	return ret;
 }
 
-static struct class wmi_bus_class = {
-	.name = "wmi_bus",
-};
-
 static struct bus_type wmi_bus_type = {
 	.name = "wmi",
 	.dev_groups = wmi_groups,
@@ -1250,8 +1328,17 @@ static int __init acpi_wmi_init(void)
 		goto err_unreg_bus;
 	}
 
+	error = alloc_chrdev_region(&wmi_devt, 0, WMI_MAX_DEVS, "wmi");
+	if (error < 0) {
+		pr_err("unable to allocate char dev region\n");
+		goto err_unreg_platform;
+	}
+
 	return 0;
 
+err_unreg_platform:
+	platform_driver_unregister(&acpi_wmi_driver);
+
 err_unreg_bus:
 	bus_unregister(&wmi_bus_type);
 
@@ -1263,6 +1350,7 @@ static int __init acpi_wmi_init(void)
 
 static void __exit acpi_wmi_exit(void)
 {
+	unregister_chrdev_region(wmi_devt, WMI_MAX_DEVS);
 	platform_driver_unregister(&acpi_wmi_driver);
 	bus_unregister(&wmi_bus_type);
 	class_unregister(&wmi_bus_class);
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index cd0d7734dc49..6899ddb6cd30 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -41,6 +41,7 @@ struct wmi_device_id {
 struct wmi_driver {
 	struct device_driver driver;
 	const struct wmi_device_id *id_table;
+	const struct file_operations *file_operations;
 
 	int (*probe)(struct wmi_device *wdev);
 	int (*remove)(struct wmi_device *wdev);
-- 
2.14.1

[PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character device for userspace

[Mario Limonciello]

This userspace character device will be used to perform SMBIOS calls
from any applications.

It contains 3 operating IOCTL's
1) sending a properly formatted 4k calling interface buffer.
2) Querying number of tokens in the given system
3) Copying the status of all these tokens to a properly formatted
   userspace buffer.

This character device is intended to deprecate the dcdbas kernel module
and the interface that it provides to userspace.

It's important for the driver to provide a R/W ioctl to ensure that
two competing userspace processes don't race to provide or read each
others data.

The character device will only be created if the WMI interface was
found.

The API for interacting with this interface is defined in documentation
as well as a uapi header provides the format of the structures.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 Documentation/ABI/testing/dell-wmi-smbios |  10 +++
 drivers/platform/x86/dell-wmi-smbios.c    | 110 +++++++++++++++++++++++++++---
 drivers/platform/x86/dell-wmi-smbios.h    |  28 +-------
 include/uapi/linux/dell-wmi-smbios.h      |  55 +++++++++++++++
 4 files changed, 165 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
 create mode 100644 include/uapi/linux/dell-wmi-smbios.h

diff --git a/Documentation/ABI/testing/dell-wmi-smbios b/Documentation/ABI/testing/dell-wmi-smbios
new file mode 100644
index 000000000000..8ec98e9b7b34
--- /dev/null
+++ b/Documentation/ABI/testing/dell-wmi-smbios
@@ -0,0 +1,10 @@
+What:		/dev/wmi-dell-wmi-smbios
+Date:		October 2017
+KernelVersion:	4.15
+Contact:	"Mario Limonciello" <mario.limonciello@dell.com>
+Description:
+		Perform SMBIOS calls on supported Dell machines.
+		through the Dell ACPI-WMI interface.
+
+		IOCTL's and buffer formats are defined in:
+		<uapi/linux/dell-wmi-smbios.h>
\ No newline at end of file
diff --git a/drivers/platform/x86/dell-wmi-smbios.c b/drivers/platform/x86/dell-wmi-smbios.c
index 699757f3e154..232a9dd482b1 100644
--- a/drivers/platform/x86/dell-wmi-smbios.c
+++ b/drivers/platform/x86/dell-wmi-smbios.c
@@ -21,6 +21,7 @@
 #include <linux/err.h>
 #include <linux/mutex.h>
 #include <linux/wmi.h>
+#include <linux/uaccess.h>
 #include "dell-wmi-smbios.h"
 
 #ifdef CONFIG_DCDBAS
@@ -39,7 +40,8 @@ struct calling_interface_structure {
 } __packed;
 
 static struct calling_interface_buffer *smi_buffer;
-static struct wmi_calling_interface_buffer *wmi_buffer;
+static struct wmi_calling_interface_buffer *internal_wmi_buffer;
+static struct wmi_calling_interface_buffer *sysfs_wmi_buffer;
 static DEFINE_MUTEX(buffer_mutex);
 
 static int da_command_address;
@@ -68,7 +70,7 @@ struct calling_interface_buffer *dell_smbios_get_buffer(void)
 	mutex_lock(&buffer_mutex);
 	dell_smbios_clear_buffer();
 	if (has_wmi)
-		return &wmi_buffer->smi;
+		return &internal_wmi_buffer->smi;
 	return smi_buffer;
 }
 EXPORT_SYMBOL_GPL(dell_smbios_get_buffer);
@@ -76,7 +78,7 @@ EXPORT_SYMBOL_GPL(dell_smbios_get_buffer);
 void dell_smbios_clear_buffer(void)
 {
 	if (has_wmi)
-		memset(wmi_buffer, 0,
+		memset(internal_wmi_buffer, 0,
 		       sizeof(struct wmi_calling_interface_buffer));
 	else
 		memset(smi_buffer, 0,
@@ -122,9 +124,9 @@ int run_wmi_smbios_call(struct wmi_calling_interface_buffer *buf)
 void dell_smbios_send_request(int class, int select)
 {
 	if (has_wmi) {
-		wmi_buffer->smi.class = class;
-		wmi_buffer->smi.select = select;
-		run_wmi_smbios_call(wmi_buffer);
+		internal_wmi_buffer->smi.class = class;
+		internal_wmi_buffer->smi.select = select;
+		run_wmi_smbios_call(internal_wmi_buffer);
 	}
 
 #ifdef CONFIG_DCDBAS
@@ -224,6 +226,74 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy)
 	}
 }
 
+static int dell_wmi_smbios_open(struct inode *inode, struct file *file)
+{
+	return nonseekable_open(inode, file);
+}
+
+static int dell_wmi_smbios_release(struct inode *inode, struct file *file)
+{
+	return 0;
+}
+
+static long dell_wmi_smbios_ioctl(struct file *filp, unsigned int cmd,
+	unsigned long arg)
+{
+	struct token_ioctl_buffer *tokens_buffer;
+	void __user *p = (void __user *) arg;
+	size_t size;
+	int ret = 0;
+
+	if (_IOC_TYPE(cmd) != DELL_WMI_SMBIOS_IOC)
+		return -ENOTTY;
+
+	switch (cmd) {
+	case DELL_WMI_SMBIOS_CALL_CMD:
+		size = sizeof(struct wmi_calling_interface_buffer);
+		mutex_lock(&buffer_mutex);
+		if (copy_from_user(sysfs_wmi_buffer, p, size)) {
+			ret = -EFAULT;
+			goto fail_smbios_cmd;
+		}
+		ret = run_wmi_smbios_call(sysfs_wmi_buffer);
+		if (ret != 0)
+			goto fail_smbios_cmd;
+		if (copy_to_user(p, sysfs_wmi_buffer, size))
+			ret = -EFAULT;
+fail_smbios_cmd:
+		mutex_unlock(&buffer_mutex);
+		break;
+	case DELL_WMI_SMBIOS_GET_NUM_TOKENS_CMD:
+		if (copy_to_user(p, &da_num_tokens, sizeof(u32)))
+			ret = -EFAULT;
+		break;
+	case DELL_WMI_SMBIOS_GET_TOKENS_CMD:
+		tokens_buffer = kmalloc(sizeof(struct token_ioctl_buffer),
+					GFP_KERNEL);
+		size = sizeof(struct token_ioctl_buffer);
+		if (copy_from_user(tokens_buffer, p, size)) {
+			ret = -EFAULT;
+			goto fail_get_tokens_cmd;
+		}
+		if (tokens_buffer->num_tokens < da_num_tokens) {
+			ret = -EOVERFLOW;
+			goto fail_get_tokens_cmd;
+		}
+		size = sizeof(struct calling_interface_token) * da_num_tokens;
+		if (copy_to_user(tokens_buffer->tokens, da_tokens, size)) {
+			ret = -EFAULT;
+			goto fail_get_tokens_cmd;
+		}
+fail_get_tokens_cmd:
+		kfree(tokens_buffer);
+		break;
+	default:
+		pr_err("unsupported ioctl: %d.\n", cmd);
+		ret = -ENOIOCTLCMD;
+	}
+	return ret;
+}
+
 /*
  * Descriptor buffer is 128 byte long and contains:
  *
@@ -301,11 +371,17 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev)
 	int ret;
 	u32 interface_version;
 
-	/* WMI buffer should be 32k */
-	wmi_buffer = (void *)__get_free_pages(GFP_KERNEL, 3);
-	if (!wmi_buffer)
+	/* WMI buffers should be 32k */
+	internal_wmi_buffer = (void *)__get_free_pages(GFP_KERNEL, 3);
+	if (!internal_wmi_buffer)
 		return -ENOMEM;
 
+	sysfs_wmi_buffer = (void *)__get_free_pages(GFP_KERNEL, 3);
+	if (!sysfs_wmi_buffer) {
+		ret = -ENOMEM;
+		goto fail_sysfs_wmi_buffer;
+	}
+
 	ret = dell_wmi_check_descriptor_buffer(wdev, &interface_version);
 	if (ret)
 		goto fail_wmi_probe;
@@ -320,13 +396,17 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev)
 	return 0;
 
 fail_wmi_probe:
-	free_pages((unsigned long)wmi_buffer, 3);
+	free_pages((unsigned long)sysfs_wmi_buffer, 3);
+
+fail_sysfs_wmi_buffer:
+	free_pages((unsigned long)internal_wmi_buffer, 3);
 	return ret;
 }
 
 static int dell_smbios_wmi_remove(struct wmi_device *wdev)
 {
-	free_pages((unsigned long)wmi_buffer, 3);
+	free_pages((unsigned long)internal_wmi_buffer, 3);
+	free_pages((unsigned long)sysfs_wmi_buffer, 3);
 	return 0;
 }
 
@@ -335,6 +415,13 @@ static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
 	{ },
 };
 
+static const struct file_operations dell_wmi_smbios_fops = {
+	.owner		= THIS_MODULE,
+	.unlocked_ioctl	= dell_wmi_smbios_ioctl,
+	.open		= dell_wmi_smbios_open,
+	.release	= dell_wmi_smbios_release,
+};
+
 static struct wmi_driver dell_wmi_smbios_driver = {
 	.driver = {
 		.name = "dell-wmi-smbios",
@@ -342,6 +429,7 @@ static struct wmi_driver dell_wmi_smbios_driver = {
 	.probe = dell_smbios_wmi_probe,
 	.remove = dell_smbios_wmi_remove,
 	.id_table = dell_smbios_wmi_id_table,
+	.file_operations = &dell_wmi_smbios_fops,
 };
 
 static int __init dell_wmi_smbios_init(void)
diff --git a/drivers/platform/x86/dell-wmi-smbios.h b/drivers/platform/x86/dell-wmi-smbios.h
index 0e8b57f7d0f0..9bc9c42db167 100644
--- a/drivers/platform/x86/dell-wmi-smbios.h
+++ b/drivers/platform/x86/dell-wmi-smbios.h
@@ -18,36 +18,10 @@
 #define _DELL_WMI_SMBIOS_H_
 
 #include <linux/wmi.h>
+#include <uapi/linux/dell-wmi-smbios.h>
 
 struct notifier_block;
 
-/* If called through fallback SMI rather than WMI this structure will be
- * modified by the firmware when we enter system management mode, hence the
- * volatiles
- */
-struct calling_interface_buffer {
-	u16 class;
-	u16 select;
-	volatile u32 input[4];
-	volatile u32 output[4];
-} __packed;
-
-struct wmi_calling_interface_buffer {
-	struct calling_interface_buffer smi;
-	u32 argattrib;
-	u32 blength;
-	u8 data[32724];
-} __packed;
-
-struct calling_interface_token {
-	u16 tokenID;
-	u16 location;
-	union {
-		u16 value;
-		u16 stringlength;
-	};
-};
-
 int dell_smbios_error(int value);
 
 struct calling_interface_buffer *dell_smbios_get_buffer(void);
diff --git a/include/uapi/linux/dell-wmi-smbios.h b/include/uapi/linux/dell-wmi-smbios.h
new file mode 100644
index 000000000000..743035d9443a
--- /dev/null
+++ b/include/uapi/linux/dell-wmi-smbios.h
@@ -0,0 +1,55 @@
+#ifndef _UAPI_DELL_WMI_SMBIOS_H_
+#define _UAPI_DELL_WMI_SMBIOS_H_
+
+#include <linux/ioctl.h>
+
+/* If called through fallback SMI rather than WMI this structure will be
+ * modified by the firmware when we enter system management mode, hence the
+ * volatiles
+ */
+struct calling_interface_buffer {
+	u16 class;
+	u16 select;
+	volatile u32 input[4];
+	volatile u32 output[4];
+} __packed;
+
+struct wmi_calling_interface_buffer {
+	struct calling_interface_buffer smi;
+	u32 argattrib;
+	u32 blength;
+	u8 data[32724];
+} __packed;
+
+struct calling_interface_token {
+	u16 tokenID;
+	u16 location;
+	union {
+		u16 value;
+		u16 stringlength;
+	};
+};
+
+struct token_ioctl_buffer {
+	struct calling_interface_token *tokens;
+	u32 num_tokens;
+};
+
+#define DELL_WMI_SMBIOS_IOC			'D'
+/* run SMBIOS calling interface command
+ * note - 32k is too big for size, so this can not be encoded in macro properly
+ */
+#define DELL_WMI_SMBIOS_CALL_CMD	_IOWR(DELL_WMI_SMBIOS_IOC, 0, u8)
+
+/* query the number of DA tokens on system */
+#define DELL_WMI_SMBIOS_GET_NUM_TOKENS_CMD	_IOR(DELL_WMI_SMBIOS_IOC, 1, \
+						      u32)
+/* query the status, location, and value of all DA tokens from bootup
+ * expects userspace to prepare buffer in advance with the number of tokens
+ * from DELL_WMI_SMBIOS_GET_NUM_TOKENS_CMD
+ */
+#define DELL_WMI_SMBIOS_GET_TOKENS_CMD		_IOWR(DELL_WMI_SMBIOS_IOC, 2, \
+						      struct token_ioctl_buffer)
+
+
+#endif /* _UAPI_DELL_WMI_SMBIOS_H_ */
-- 
2.14.1

[PATCH v2 13/14] platform/x86: Kconfig: Change the default settings for dell-wmi-smbios

[Mario Limonciello]

The dell-wmi-smbios driver should be enabled by default when ACPI_WMI
is enabled (like many other WMI drivers).

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index a70bcd8caa72..4f9ca51b1968 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -94,6 +94,7 @@ config ASUS_LAPTOP
 config DELL_WMI_SMBIOS
 	tristate "Dell WMI SMBIOS calling interface"
 	depends on ACPI_WMI
+	default ACPI_WMI
 	---help---
 	This module provides common functions for kernel modules using
 	Dell SMBIOS over ACPI-WMI.
-- 
2.14.1

[PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check

[Mario Limonciello]

Some cases the wrong type was used for errors and checks can be
done more cleanly.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/dell-wmi-smbios.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi-smbios.c b/drivers/platform/x86/dell-wmi-smbios.c
index 232a9dd482b1..290e9853818b 100644
--- a/drivers/platform/x86/dell-wmi-smbios.c
+++ b/drivers/platform/x86/dell-wmi-smbios.c
@@ -340,16 +340,16 @@ int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev, u32 *version)
 	}
 	desc_buffer = (u32 *)obj->buffer.pointer;
 
-	if (desc_buffer[0] != 0x4C4C4544 && desc_buffer[1] != 0x494D5720)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid signature (%*ph)\n",
-			8, desc_buffer);
+	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0)
+		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
+			desc_buffer);
 
 	if (desc_buffer[2] != 0 && desc_buffer[2] != 1)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%d)\n",
+		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%u)\n",
 			desc_buffer[2]);
 
 	if (desc_buffer[3] != 4096)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid buffer length (%d)\n",
+		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid buffer length (%u)\n",
 			desc_buffer[3]);
 
 	*version = desc_buffer[2];
-- 
2.14.1

Re: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character device for userspace

[Andy Shevchenko]

On Tue, Sep 26, 2017 at 9:50 PM, Mario Limonciello
<mario.limonciello@dell.com> wrote:
> This userspace character device will be used to perform SMBIOS calls
> from any applications.
>
> It contains 3 operating IOCTL's
> 1) sending a properly formatted 4k calling interface buffer.
> 2) Querying number of tokens in the given system
> 3) Copying the status of all these tokens to a properly formatted
>    userspace buffer.
>
> This character device is intended to deprecate the dcdbas kernel module
> and the interface that it provides to userspace.
>
> It's important for the driver to provide a R/W ioctl to ensure that
> two competing userspace processes don't race to provide or read each
> others data.
>
> The character device will only be created if the WMI interface was
> found.
>
> The API for interacting with this interface is defined in documentation
> as well as a uapi header provides the format of the structures.


> +               <uapi/linux/dell-wmi-smbios.h>
> \ No newline at end of file

Oh là là

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI

[Andy Shevchenko]

On Tue, Sep 26, 2017 at 9:49 PM, Mario Limonciello
<mario.limonciello@dell.com> wrote:
> The existing way that the dell-smbios helper module and associated
> other drivers (dell-laptop, dell-wmi) communicate with the platform
> really isn't secure.  It requires creating a buffer in physical
> DMA32 memory space and passing that to the platform via SMM.
>
> Since the platform got a physical memory pointer, you've just got
> to trust that the platform has only modified (and accessed) memory
> within that buffer.
>
> Dell Platform designers recognize this security risk and offer a
> safer way to communicate with the platform over ACPI.  This is
> in turn exposed via a WMI interface to the OS.
>
> When communicating over WMI-ACPI the communication doesn't occur
> with physical memory pointers.  When the ASL is invoked, the fixed
> length ACPI buffer is copied to a small operating region.  The ASL
> will invoke the SMI, and SMM will only have access to this operating
> region.  When the ASL returns the buffer is copied back for the OS
> to process.
>
> This method of communication should also deprecate the usage of the
> dcdbas kernel module and software dependent upon it's interface.
> Instead offer a character device interface for communicating with this
> ASL method to allow userspace to use instead.
>
> To faciliate that this patch series introduces a generic way for WMI
> drivers to be able to create discoverable character devices through
> the WMI bus when desired.
> Requiring WMI drivers to explicitly ask for this functionality will
> act as an effective vendor whitelist to character device creation.
>

Thanks for an update, looks better now.
I'm in the middle of going through it and noticed that patches 6 and 7
sound like a fix to me.
Perhaps better to move it to be first part of the series.

Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
interested in changes like these to WMI.

> changes between v1 and v2:
>  * Introduce another patch to sort the includes in wmi.c
>  * Introduce another patch to cleanup dell_wmi_check_descriptor_buffer
>    checks.
>  * Add a commit message to the pr_fmt commit
>  * Introduce includes to wmi.c in proper location
>  * Add Reviewed-by to relevant patches from Pali
>  * Make the WMI introduction patch fallback to legacy SMI
>    if compiled with CONFIG_DCDBAS
>  * Separate format of WMI and SMI buffers.  WMI buffer supports more
>    arguments and data.
>  * Adjust the rename patch for changes to fallback
>  * Drop sysfs token creation patch
>  * Adjust WMI descriptor check patch for changes to fallback
>  * introduce another patch to remove needless includes in dell-smbios.c
>  * Add token ioctl interface to character device.
>    - Can query number of tokens
>    - Can query values in all tokens
>  * Expose format of all buffers and IOCTL commands to uapi header
>  * Drop the read interface from character device.  It doesn't make
>    sense with multiple different ioctl methods.
>  * Default WMI interface to 32k (This would normally be queried via
>    MOF, but that's not possible yet)
>  * Create separate buffers for WMI and SMI.  If WMI is available,
>    free the SMI buffer.
>  * Reorder patches so all fixups come first in the series.
>
> Mario Limonciello (14):
>   platform/x86: dell-wmi: label driver as handling notifications
>   platform/x86: dell-smbios: drop needless includes
>   platform/x86: dell-wmi: Don't match on descriptor GUID modalias
>   platform/x86: dell-smbios: Add pr_fmt definition to driver
>   platform/x86: wmi: sort include list
>   platform/x86: wmi: Cleanup exit routine in reverse order of init
>   platform/x86: wmi: destroy on cleanup rather than unregister
>   platform/x86: dell-smbios: Introduce a WMI-ACPI interface
>   platform/x86: dell-smbios: rename to dell-wmi-smbios
>   platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check
>   platform/x86: wmi: create character devices when requested by drivers
>   platform/x86: dell-wmi-smbios: introduce character device for
>     userspace
>   platform/x86: Kconfig: Change the default settings for dell-wmi-smbios
>   platform/x86: dell-wmi-smbios: clean up wmi descriptor check
>
>  Documentation/ABI/testing/dell-wmi-smbios          |  10 +
>  MAINTAINERS                                        |   8 +-
>  drivers/platform/x86/Kconfig                       |  13 +-
>  drivers/platform/x86/Makefile                      |   2 +-
>  drivers/platform/x86/dell-laptop.c                 |   2 +-
>  drivers/platform/x86/dell-smbios.c                 | 213 ---------
>  drivers/platform/x86/dell-wmi-smbios.c             | 481 +++++++++++++++++++++
>  .../x86/{dell-smbios.h => dell-wmi-smbios.h}       |  28 +-
>  drivers/platform/x86/dell-wmi.c                    |  78 +---
>  drivers/platform/x86/wmi.c                         | 116 ++++-
>  include/linux/wmi.h                                |   1 +
>  include/uapi/linux/dell-wmi-smbios.h               |  55 +++
>  12 files changed, 674 insertions(+), 333 deletions(-)
>  create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
>  delete mode 100644 drivers/platform/x86/dell-smbios.c
>  create mode 100644 drivers/platform/x86/dell-wmi-smbios.c
>  rename drivers/platform/x86/{dell-smbios.h => dell-wmi-smbios.h} (74%)
>  create mode 100644 include/uapi/linux/dell-wmi-smbios.h
>
> --
> 2.14.1
>



-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI

[Mario.Limonciello]

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Tuesday, September 26, 2017 3:06 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; LKML <linux-kernel@vger.kernel.org>; Platform Driver
> <platform-driver-x86@vger.kernel.org>; quasisec@google.com; Pali Rohár
> <pali.rohar@gmail.com>
> Subject: Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI
> 
> On Tue, Sep 26, 2017 at 9:49 PM, Mario Limonciello
> <mario.limonciello@dell.com> wrote:
> > The existing way that the dell-smbios helper module and associated
> > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > really isn't secure.  It requires creating a buffer in physical
> > DMA32 memory space and passing that to the platform via SMM.
> >
> > Since the platform got a physical memory pointer, you've just got
> > to trust that the platform has only modified (and accessed) memory
> > within that buffer.
> >
> > Dell Platform designers recognize this security risk and offer a
> > safer way to communicate with the platform over ACPI.  This is
> > in turn exposed via a WMI interface to the OS.
> >
> > When communicating over WMI-ACPI the communication doesn't occur
> > with physical memory pointers.  When the ASL is invoked, the fixed
> > length ACPI buffer is copied to a small operating region.  The ASL
> > will invoke the SMI, and SMM will only have access to this operating
> > region.  When the ASL returns the buffer is copied back for the OS
> > to process.
> >
> > This method of communication should also deprecate the usage of the
> > dcdbas kernel module and software dependent upon it's interface.
> > Instead offer a character device interface for communicating with this
> > ASL method to allow userspace to use instead.
> >
> > To faciliate that this patch series introduces a generic way for WMI
> > drivers to be able to create discoverable character devices through
> > the WMI bus when desired.
> > Requiring WMI drivers to explicitly ask for this functionality will
> > act as an effective vendor whitelist to character device creation.
> >
> 
> Thanks for an update, looks better now.
> I'm in the middle of going through it and noticed that patches 6 and 7
> sound like a fix to me.
> Perhaps better to move it to be first part of the series.

In v2 I'd say that all of 1 through 7 are fixes that even if the rest of the
series doesn't get pulled in immediately should land sooner.
I'll reorder 6 and 7 for the beginning though for v3 if v3 is necessary.

8 through 13 are the real "meat", and 14 is a fixup.

> 
> Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
> interested in changes like these to WMI.

Added.  Andy you weren't CC'ed on the original or v2 submission, but if this
requires a v3 I'll include you.

> 
> > changes between v1 and v2:
> >  * Introduce another patch to sort the includes in wmi.c
> >  * Introduce another patch to cleanup dell_wmi_check_descriptor_buffer
> >    checks.
> >  * Add a commit message to the pr_fmt commit
> >  * Introduce includes to wmi.c in proper location
> >  * Add Reviewed-by to relevant patches from Pali
> >  * Make the WMI introduction patch fallback to legacy SMI
> >    if compiled with CONFIG_DCDBAS
> >  * Separate format of WMI and SMI buffers.  WMI buffer supports more
> >    arguments and data.
> >  * Adjust the rename patch for changes to fallback
> >  * Drop sysfs token creation patch
> >  * Adjust WMI descriptor check patch for changes to fallback
> >  * introduce another patch to remove needless includes in dell-smbios.c
> >  * Add token ioctl interface to character device.
> >    - Can query number of tokens
> >    - Can query values in all tokens
> >  * Expose format of all buffers and IOCTL commands to uapi header
> >  * Drop the read interface from character device.  It doesn't make
> >    sense with multiple different ioctl methods.
> >  * Default WMI interface to 32k (This would normally be queried via
> >    MOF, but that's not possible yet)
> >  * Create separate buffers for WMI and SMI.  If WMI is available,
> >    free the SMI buffer.
> >  * Reorder patches so all fixups come first in the series.
> >
> > Mario Limonciello (14):
> >   platform/x86: dell-wmi: label driver as handling notifications
> >   platform/x86: dell-smbios: drop needless includes
> >   platform/x86: dell-wmi: Don't match on descriptor GUID modalias
> >   platform/x86: dell-smbios: Add pr_fmt definition to driver
> >   platform/x86: wmi: sort include list
> >   platform/x86: wmi: Cleanup exit routine in reverse order of init
> >   platform/x86: wmi: destroy on cleanup rather than unregister
> >   platform/x86: dell-smbios: Introduce a WMI-ACPI interface
> >   platform/x86: dell-smbios: rename to dell-wmi-smbios
> >   platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check
> >   platform/x86: wmi: create character devices when requested by drivers
> >   platform/x86: dell-wmi-smbios: introduce character device for
> >     userspace
> >   platform/x86: Kconfig: Change the default settings for dell-wmi-smbios
> >   platform/x86: dell-wmi-smbios: clean up wmi descriptor check
> >
> >  Documentation/ABI/testing/dell-wmi-smbios          |  10 +
> >  MAINTAINERS                                        |   8 +-
> >  drivers/platform/x86/Kconfig                       |  13 +-
> >  drivers/platform/x86/Makefile                      |   2 +-
> >  drivers/platform/x86/dell-laptop.c                 |   2 +-
> >  drivers/platform/x86/dell-smbios.c                 | 213 ---------
> >  drivers/platform/x86/dell-wmi-smbios.c             | 481 +++++++++++++++++++++
> >  .../x86/{dell-smbios.h => dell-wmi-smbios.h}       |  28 +-
> >  drivers/platform/x86/dell-wmi.c                    |  78 +---
> >  drivers/platform/x86/wmi.c                         | 116 ++++-
> >  include/linux/wmi.h                                |   1 +
> >  include/uapi/linux/dell-wmi-smbios.h               |  55 +++
> >  12 files changed, 674 insertions(+), 333 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
> >  delete mode 100644 drivers/platform/x86/dell-smbios.c
> >  create mode 100644 drivers/platform/x86/dell-wmi-smbios.c
> >  rename drivers/platform/x86/{dell-smbios.h => dell-wmi-smbios.h} (74%)
> >  create mode 100644 include/uapi/linux/dell-wmi-smbios.h
> >
> > --
> > 2.14.1
> >
> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

RE: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI

[Mario.Limonciello]

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Tuesday, September 26, 2017 3:06 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; LKML <linux-kernel@vger.kernel.org>; Platform Driver
> <platform-driver-x86@vger.kernel.org>; quasisec@google.com; Pali Rohár
> <pali.rohar@gmail.com>
> Subject: Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI
> 
> On Tue, Sep 26, 2017 at 9:49 PM, Mario Limonciello
> <mario.limonciello@dell.com> wrote:
> > The existing way that the dell-smbios helper module and associated
> > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > really isn't secure.  It requires creating a buffer in physical
> > DMA32 memory space and passing that to the platform via SMM.
> >
> > Since the platform got a physical memory pointer, you've just got
> > to trust that the platform has only modified (and accessed) memory
> > within that buffer.
> >
> > Dell Platform designers recognize this security risk and offer a
> > safer way to communicate with the platform over ACPI.  This is
> > in turn exposed via a WMI interface to the OS.
> >
> > When communicating over WMI-ACPI the communication doesn't occur
> > with physical memory pointers.  When the ASL is invoked, the fixed
> > length ACPI buffer is copied to a small operating region.  The ASL
> > will invoke the SMI, and SMM will only have access to this operating
> > region.  When the ASL returns the buffer is copied back for the OS
> > to process.
> >
> > This method of communication should also deprecate the usage of the
> > dcdbas kernel module and software dependent upon it's interface.
> > Instead offer a character device interface for communicating with this
> > ASL method to allow userspace to use instead.
> >
> > To faciliate that this patch series introduces a generic way for WMI
> > drivers to be able to create discoverable character devices through
> > the WMI bus when desired.
> > Requiring WMI drivers to explicitly ask for this functionality will
> > act as an effective vendor whitelist to character device creation.
> >
> 
> Thanks for an update, looks better now.
> I'm in the middle of going through it and noticed that patches 6 and 7
> sound like a fix to me.
> Perhaps better to move it to be first part of the series.

In v2 I'd say that all of 1 through 7 are fixes that even if the rest of the
series doesn't get pulled in immediately should land sooner.
I'll reorder 6 and 7 for the beginning though for v3 if v3 is necessary.

8 through 13 are the real "meat", and 14 is a fixup.

> 
> Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
> interested in changes like these to WMI.

Added.  Andy you weren't CC'ed on the original or v2 submission, but if this
requires a v3 I'll include you.

> 
> > changes between v1 and v2:
> >  * Introduce another patch to sort the includes in wmi.c
> >  * Introduce another patch to cleanup dell_wmi_check_descriptor_buffer
> >    checks.
> >  * Add a commit message to the pr_fmt commit
> >  * Introduce includes to wmi.c in proper location
> >  * Add Reviewed-by to relevant patches from Pali
> >  * Make the WMI introduction patch fallback to legacy SMI
> >    if compiled with CONFIG_DCDBAS
> >  * Separate format of WMI and SMI buffers.  WMI buffer supports more
> >    arguments and data.
> >  * Adjust the rename patch for changes to fallback
> >  * Drop sysfs token creation patch
> >  * Adjust WMI descriptor check patch for changes to fallback
> >  * introduce another patch to remove needless includes in dell-smbios.c
> >  * Add token ioctl interface to character device.
> >    - Can query number of tokens
> >    - Can query values in all tokens
> >  * Expose format of all buffers and IOCTL commands to uapi header
> >  * Drop the read interface from character device.  It doesn't make
> >    sense with multiple different ioctl methods.
> >  * Default WMI interface to 32k (This would normally be queried via
> >    MOF, but that's not possible yet)
> >  * Create separate buffers for WMI and SMI.  If WMI is available,
> >    free the SMI buffer.
> >  * Reorder patches so all fixups come first in the series.
> >
> > Mario Limonciello (14):
> >   platform/x86: dell-wmi: label driver as handling notifications
> >   platform/x86: dell-smbios: drop needless includes
> >   platform/x86: dell-wmi: Don't match on descriptor GUID modalias
> >   platform/x86: dell-smbios: Add pr_fmt definition to driver
> >   platform/x86: wmi: sort include list
> >   platform/x86: wmi: Cleanup exit routine in reverse order of init
> >   platform/x86: wmi: destroy on cleanup rather than unregister
> >   platform/x86: dell-smbios: Introduce a WMI-ACPI interface
> >   platform/x86: dell-smbios: rename to dell-wmi-smbios
> >   platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check
> >   platform/x86: wmi: create character devices when requested by drivers
> >   platform/x86: dell-wmi-smbios: introduce character device for
> >     userspace
> >   platform/x86: Kconfig: Change the default settings for dell-wmi-smbios
> >   platform/x86: dell-wmi-smbios: clean up wmi descriptor check
> >
> >  Documentation/ABI/testing/dell-wmi-smbios          |  10 +
> >  MAINTAINERS                                        |   8 +-
> >  drivers/platform/x86/Kconfig                       |  13 +-
> >  drivers/platform/x86/Makefile                      |   2 +-
> >  drivers/platform/x86/dell-laptop.c                 |   2 +-
> >  drivers/platform/x86/dell-smbios.c                 | 213 ---------
> >  drivers/platform/x86/dell-wmi-smbios.c             | 481 +++++++++++++++++++++
> >  .../x86/{dell-smbios.h => dell-wmi-smbios.h}       |  28 +-
> >  drivers/platform/x86/dell-wmi.c                    |  78 +---
> >  drivers/platform/x86/wmi.c                         | 116 ++++-
> >  include/linux/wmi.h                                |   1 +
> >  include/uapi/linux/dell-wmi-smbios.h               |  55 +++
> >  12 files changed, 674 insertions(+), 333 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
> >  delete mode 100644 drivers/platform/x86/dell-smbios.c
> >  create mode 100644 drivers/platform/x86/dell-wmi-smbios.c
> >  rename drivers/platform/x86/{dell-smbios.h => dell-wmi-smbios.h} (74%)
> >  create mode 100644 include/uapi/linux/dell-wmi-smbios.h
> >
> > --
> > 2.14.1
> >
> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios

[Pali Rohár]

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

On Tuesday 26 September 2017 20:50:07 Mario Limonciello wrote:
> This follows the style of the rest of the platform x86 WMI drivers.
> 
> Renaming the driver requires adjusting the other drivers using
> dell-smbios to pick up the newly named includes.
> 
> While renaming, I noticed that this driver was missing from
> MAINTAINERS. Add it to that and myself to the list of people
> maintaing it.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>

Now when driver supports both old and new WMI call interface, I think 
that renaming is not needed...

-- 
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 12/14] platform/x86: dell-wmi-smbios: introduce character device for userspace

[Pali Rohár]

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

On Tuesday 26 September 2017 20:50:10 Mario Limonciello wrote:
> This userspace character device will be used to perform SMBIOS calls
> from any applications.
> 
> It contains 3 operating IOCTL's
> 1) sending a properly formatted 4k calling interface buffer.
> 2) Querying number of tokens in the given system
> 3) Copying the status of all these tokens to a properly formatted
>    userspace buffer.
> 
> This character device is intended to deprecate the dcdbas kernel
> module and the interface that it provides to userspace.
> 
> It's important for the driver to provide a R/W ioctl to ensure that
> two competing userspace processes don't race to provide or read each
> others data.
> 
> The character device will only be created if the WMI interface was
> found.
> 
> The API for interacting with this interface is defined in
> documentation as well as a uapi header provides the format of the
> structures.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> ---
>  Documentation/ABI/testing/dell-wmi-smbios |  10 +++
>  drivers/platform/x86/dell-wmi-smbios.c    | 110
> +++++++++++++++++++++++++++---
> drivers/platform/x86/dell-wmi-smbios.h    |  28 +-------
>  include/uapi/linux/dell-wmi-smbios.h      |  55 +++++++++++++++
>  4 files changed, 165 insertions(+), 38 deletions(-)
>  create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
>  create mode 100644 include/uapi/linux/dell-wmi-smbios.h
> 
> diff --git a/Documentation/ABI/testing/dell-wmi-smbios
> b/Documentation/ABI/testing/dell-wmi-smbios new file mode 100644
> index 000000000000..8ec98e9b7b34
> --- /dev/null
> +++ b/Documentation/ABI/testing/dell-wmi-smbios
> @@ -0,0 +1,10 @@
> +What:		/dev/wmi-dell-wmi-smbios
> +Date:		October 2017
> +KernelVersion:	4.15
> +Contact:	"Mario Limonciello" <mario.limonciello@dell.com>
> +Description:
> +		Perform SMBIOS calls on supported Dell machines.
> +		through the Dell ACPI-WMI interface.
> +
> +		IOCTL's and buffer formats are defined in:
> +		<uapi/linux/dell-wmi-smbios.h>

Should be "wmi" part of device name, even if whole driver work without 
WMI?

I think that "wmi" should be part of device name only if it is related 
to WMI bus functions, and not in case when it exports some vendor (Dell) 
specific API which may be implemented via WMI.

E.g. we have /dev/i2c-* for raw access to i2c devices. But we do not 
have /dev/i2c-something for e.g. RTC device connected via i2c...

-- 
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 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check

[Pali Rohár]

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

On Tuesday 26 September 2017 20:50:12 Mario Limonciello wrote:
> +	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0)

Should not be this memcmp? IIRC we want to compare that buffer is equal 
to some bytes, right?

-- 
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 12/14] platform/x86: dell-wmi-smbios: introduce character device for userspace

[Mario.Limonciello]

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Tuesday, September 26, 2017 4:10 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; Andy Shevchenko <andy.shevchenko@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com
> Subject: Re: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character
> device for userspace
> 
> On Tuesday 26 September 2017 20:50:10 Mario Limonciello wrote:
> > This userspace character device will be used to perform SMBIOS calls
> > from any applications.
> >
> > It contains 3 operating IOCTL's
> > 1) sending a properly formatted 4k calling interface buffer.
> > 2) Querying number of tokens in the given system
> > 3) Copying the status of all these tokens to a properly formatted
> >    userspace buffer.
> >
> > This character device is intended to deprecate the dcdbas kernel
> > module and the interface that it provides to userspace.
> >
> > It's important for the driver to provide a R/W ioctl to ensure that
> > two competing userspace processes don't race to provide or read each
> > others data.
> >
> > The character device will only be created if the WMI interface was
> > found.
> >
> > The API for interacting with this interface is defined in
> > documentation as well as a uapi header provides the format of the
> > structures.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > ---
> >  Documentation/ABI/testing/dell-wmi-smbios |  10 +++
> >  drivers/platform/x86/dell-wmi-smbios.c    | 110
> > +++++++++++++++++++++++++++---
> > drivers/platform/x86/dell-wmi-smbios.h    |  28 +-------
> >  include/uapi/linux/dell-wmi-smbios.h      |  55 +++++++++++++++
> >  4 files changed, 165 insertions(+), 38 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
> >  create mode 100644 include/uapi/linux/dell-wmi-smbios.h
> >
> > diff --git a/Documentation/ABI/testing/dell-wmi-smbios
> > b/Documentation/ABI/testing/dell-wmi-smbios new file mode 100644
> > index 000000000000..8ec98e9b7b34
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/dell-wmi-smbios
> > @@ -0,0 +1,10 @@
> > +What:		/dev/wmi-dell-wmi-smbios
> > +Date:		October 2017
> > +KernelVersion:	4.15
> > +Contact:	"Mario Limonciello" <mario.limonciello@dell.com>
> > +Description:
> > +		Perform SMBIOS calls on supported Dell machines.
> > +		through the Dell ACPI-WMI interface.
> > +
> > +		IOCTL's and buffer formats are defined in:
> > +		<uapi/linux/dell-wmi-smbios.h>
> 
> Should be "wmi" part of device name, even if whole driver work without
> WMI?

Well actually the driver won't create a character device unless WMI is
used.  Exposing a character device for non-WMI is just duplicating dcdbas
at another endpoint without any security advantage.

> 
> I think that "wmi" should be part of device name only if it is related
> to WMI bus functions, and not in case when it exports some vendor (Dell)
> specific API which may be implemented via WMI.
> 
> E.g. we have /dev/i2c-* for raw access to i2c devices. But we do not
> have /dev/i2c-something for e.g. RTC device connected via i2c...

I see what you mean here, but I think we need Darren and Andy to weigh
in here.  

The character device does use WMI bus methods (wmi_evaluate_method).
The primary reason to create with wmi in the name is to show that the character 
devices are supposed to be created and managed by the WMI bus.  Vendor
drivers should not create their "own".  In my mind this will be especially relevant
once WMI bus can interpret MOF and create a set of mappings for attributes
defined in MOF.
 

RE: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character device for userspace

[Mario.Limonciello]

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Tuesday, September 26, 2017 4:10 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; Andy Shevchenko <andy.shevchenko@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com
> Subject: Re: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character
> device for userspace
> 
> On Tuesday 26 September 2017 20:50:10 Mario Limonciello wrote:
> > This userspace character device will be used to perform SMBIOS calls
> > from any applications.
> >
> > It contains 3 operating IOCTL's
> > 1) sending a properly formatted 4k calling interface buffer.
> > 2) Querying number of tokens in the given system
> > 3) Copying the status of all these tokens to a properly formatted
> >    userspace buffer.
> >
> > This character device is intended to deprecate the dcdbas kernel
> > module and the interface that it provides to userspace.
> >
> > It's important for the driver to provide a R/W ioctl to ensure that
> > two competing userspace processes don't race to provide or read each
> > others data.
> >
> > The character device will only be created if the WMI interface was
> > found.
> >
> > The API for interacting with this interface is defined in
> > documentation as well as a uapi header provides the format of the
> > structures.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > ---
> >  Documentation/ABI/testing/dell-wmi-smbios |  10 +++
> >  drivers/platform/x86/dell-wmi-smbios.c    | 110
> > +++++++++++++++++++++++++++---
> > drivers/platform/x86/dell-wmi-smbios.h    |  28 +-------
> >  include/uapi/linux/dell-wmi-smbios.h      |  55 +++++++++++++++
> >  4 files changed, 165 insertions(+), 38 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
> >  create mode 100644 include/uapi/linux/dell-wmi-smbios.h
> >
> > diff --git a/Documentation/ABI/testing/dell-wmi-smbios
> > b/Documentation/ABI/testing/dell-wmi-smbios new file mode 100644
> > index 000000000000..8ec98e9b7b34
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/dell-wmi-smbios
> > @@ -0,0 +1,10 @@
> > +What:		/dev/wmi-dell-wmi-smbios
> > +Date:		October 2017
> > +KernelVersion:	4.15
> > +Contact:	"Mario Limonciello" <mario.limonciello@dell.com>
> > +Description:
> > +		Perform SMBIOS calls on supported Dell machines.
> > +		through the Dell ACPI-WMI interface.
> > +
> > +		IOCTL's and buffer formats are defined in:
> > +		<uapi/linux/dell-wmi-smbios.h>
> 
> Should be "wmi" part of device name, even if whole driver work without
> WMI?

Well actually the driver won't create a character device unless WMI is
used.  Exposing a character device for non-WMI is just duplicating dcdbas
at another endpoint without any security advantage.

> 
> I think that "wmi" should be part of device name only if it is related
> to WMI bus functions, and not in case when it exports some vendor (Dell)
> specific API which may be implemented via WMI.
> 
> E.g. we have /dev/i2c-* for raw access to i2c devices. But we do not
> have /dev/i2c-something for e.g. RTC device connected via i2c...

I see what you mean here, but I think we need Darren and Andy to weigh
in here.  

The character device does use WMI bus methods (wmi_evaluate_method).
The primary reason to create with wmi in the name is to show that the character 
devices are supposed to be created and managed by the WMI bus.  Vendor
drivers should not create their "own".  In my mind this will be especially relevant
once WMI bus can interpret MOF and create a set of mappings for attributes
defined in MOF.
 

RE: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios

[Mario.Limonciello]

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Tuesday, September 26, 2017 4:06 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; Andy Shevchenko <andy.shevchenko@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com
> Subject: Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-
> smbios
> 
> On Tuesday 26 September 2017 20:50:07 Mario Limonciello wrote:
> > This follows the style of the rest of the platform x86 WMI drivers.
> >
> > Renaming the driver requires adjusting the other drivers using
> > dell-smbios to pick up the newly named includes.
> >
> > While renaming, I noticed that this driver was missing from
> > MAINTAINERS. Add it to that and myself to the list of people
> > maintaing it.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> 
> Now when driver supports both old and new WMI call interface, I think
> that renaming is not needed...
> 

So all other drivers that use WMI have "WMI" in the name, this would be
breaking previously set precedence.  Daren, Andy, what would you like
me to do?

RE: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios

[Mario.Limonciello]

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Tuesday, September 26, 2017 4:06 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; Andy Shevchenko <andy.shevchenko@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com
> Subject: Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-
> smbios
> 
> On Tuesday 26 September 2017 20:50:07 Mario Limonciello wrote:
> > This follows the style of the rest of the platform x86 WMI drivers.
> >
> > Renaming the driver requires adjusting the other drivers using
> > dell-smbios to pick up the newly named includes.
> >
> > While renaming, I noticed that this driver was missing from
> > MAINTAINERS. Add it to that and myself to the list of people
> > maintaing it.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> 
> Now when driver supports both old and new WMI call interface, I think
> that renaming is not needed...
> 

So all other drivers that use WMI have "WMI" in the name, this would be
breaking previously set precedence.  Daren, Andy, what would you like
me to do?

RE: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check

[Mario.Limonciello]

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Tuesday, September 26, 2017 4:12 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; Andy Shevchenko <andy.shevchenko@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com
> Subject: Re: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi
> descriptor check
> 
> On Tuesday 26 September 2017 20:50:12 Mario Limonciello wrote:
> > +	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0)
> 
> Should not be this memcmp? IIRC we want to compare that buffer is equal
> to some bytes, right?
> 

It used to be comparing the bytes, but Andy S. thought this wasn't very clear.
That's why I adjusted it to compare to the strings that the bytes used instead.

RE: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check

[Mario.Limonciello]

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Tuesday, September 26, 2017 4:12 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; Andy Shevchenko <andy.shevchenko@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com
> Subject: Re: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi
> descriptor check
> 
> On Tuesday 26 September 2017 20:50:12 Mario Limonciello wrote:
> > +	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0)
> 
> Should not be this memcmp? IIRC we want to compare that buffer is equal
> to some bytes, right?
> 

It used to be comparing the bytes, but Andy S. thought this wasn't very clear.
That's why I adjusted it to compare to the strings that the bytes used instead.

Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI

[Darren Hart]

On Tue, Sep 26, 2017 at 07:17:17PM +0000, Mario.Limonciello@dell.com wrote:
> > 
> > Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
> > interested in changes like these to WMI.
> 
> Added.  Andy you weren't CC'ed on the original or v2 submission, but if this
> requires a v3 I'll include you.

Andy should be Cc'd on all patches to  platform drivers x86 as we
co-maintain the subsystem together.

The get_maintainer.pl script is helpful when determining the Cc list.

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor GUID modalias

[Darren Hart]

On Tue, Sep 26, 2017 at 01:50:01PM -0500, Mario Limonciello wrote:
> The descriptor GUID is not used to indicate that WMI notifications
> in the dell-wmi driver work properly.  As such a modalias should
> not be present that causes this driver to load on systems with this
> GUID.
> 
> Reviewed-by: Pali Pohar <pali.rohar@gmail.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>

I can do this manually, but for future reference:

Nit, in general, the author's sign off should be first, followed by
reviewers and testers. The maintainer will add their sign off at the end
- this keeps things clear regarding the development and delivery path.
In general, assume chronological order and you'll have the right idea.

Informational tags, like Reported, Suggested, Tested, and Fixes can come
before the Author sign off, which is consistent with the chronological
order.

-- 
Darren Hart
VMware Open Source Technology Center

RE: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor GUID modalias

[Mario.Limonciello]

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Wednesday, September 27, 2017 1:22 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com; pali.rohar@gmail.com
> Subject: Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor
> GUID modalias
> 
> On Tue, Sep 26, 2017 at 01:50:01PM -0500, Mario Limonciello wrote:
> > The descriptor GUID is not used to indicate that WMI notifications
> > in the dell-wmi driver work properly.  As such a modalias should
> > not be present that causes this driver to load on systems with this
> > GUID.
> >
> > Reviewed-by: Pali Pohar <pali.rohar@gmail.com>
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> 
> I can do this manually, but for future reference:
> 
> Nit, in general, the author's sign off should be first, followed by
> reviewers and testers. The maintainer will add their sign off at the end
> - this keeps things clear regarding the development and delivery path.
> In general, assume chronological order and you'll have the right idea.
> 
> Informational tags, like Reported, Suggested, Tested, and Fixes can come
> before the Author sign off, which is consistent with the chronological
> order.
> 

Operationally is there is a flag I'm missing in git-format-patch that does
this?  git format-patch -s puts my sign off at the very bottom, so this is a
manual action to re-order if I'm adding in Reviewed-by: in the patch
description.

RE: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor GUID modalias

[Mario.Limonciello]

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Wednesday, September 27, 2017 1:22 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com; pali.rohar@gmail.com
> Subject: Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor
> GUID modalias
> 
> On Tue, Sep 26, 2017 at 01:50:01PM -0500, Mario Limonciello wrote:
> > The descriptor GUID is not used to indicate that WMI notifications
> > in the dell-wmi driver work properly.  As such a modalias should
> > not be present that causes this driver to load on systems with this
> > GUID.
> >
> > Reviewed-by: Pali Pohar <pali.rohar@gmail.com>
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> 
> I can do this manually, but for future reference:
> 
> Nit, in general, the author's sign off should be first, followed by
> reviewers and testers. The maintainer will add their sign off at the end
> - this keeps things clear regarding the development and delivery path.
> In general, assume chronological order and you'll have the right idea.
> 
> Informational tags, like Reported, Suggested, Tested, and Fixes can come
> before the Author sign off, which is consistent with the chronological
> order.
> 

Operationally is there is a flag I'm missing in git-format-patch that does
this?  git format-patch -s puts my sign off at the very bottom, so this is a
manual action to re-order if I'm adding in Reviewed-by: in the patch
description.

RE: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI

[Mario.Limonciello]

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Wednesday, September 27, 2017 1:12 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: andy.shevchenko@gmail.com; luto@kernel.org; linux-kernel@vger.kernel.org;
> platform-driver-x86@vger.kernel.org; quasisec@google.com;
> pali.rohar@gmail.com
> Subject: Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI
> 
> On Tue, Sep 26, 2017 at 07:17:17PM +0000, Mario.Limonciello@dell.com wrote:
> > >
> > > Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
> > > interested in changes like these to WMI.
> >
> > Added.  Andy you weren't CC'ed on the original or v2 submission, but if this
> > requires a v3 I'll include you.
> 
> Andy should be Cc'd on all patches to  platform drivers x86 as we
> co-maintain the subsystem together.

Andy S. was CC'ed on all of these, Andy S was recommending that I add Andy L.

RE: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI

[Mario.Limonciello]

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Wednesday, September 27, 2017 1:12 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: andy.shevchenko@gmail.com; luto@kernel.org; linux-kernel@vger.kernel.org;
> platform-driver-x86@vger.kernel.org; quasisec@google.com;
> pali.rohar@gmail.com
> Subject: Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI
> 
> On Tue, Sep 26, 2017 at 07:17:17PM +0000, Mario.Limonciello@dell.com wrote:
> > >
> > > Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
> > > interested in changes like these to WMI.
> >
> > Added.  Andy you weren't CC'ed on the original or v2 submission, but if this
> > requires a v3 I'll include you.
> 
> Andy should be Cc'd on all patches to  platform drivers x86 as we
> co-maintain the subsystem together.

Andy S. was CC'ed on all of these, Andy S was recommending that I add Andy L.

Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI

[Darren Hart]

On Wed, Sep 27, 2017 at 05:31:14PM +0000, Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Darren Hart [mailto:dvhart@infradead.org]
> > Sent: Wednesday, September 27, 2017 1:12 PM
> > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > Cc: andy.shevchenko@gmail.com; luto@kernel.org; linux-kernel@vger.kernel.org;
> > platform-driver-x86@vger.kernel.org; quasisec@google.com;
> > pali.rohar@gmail.com
> > Subject: Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI
> > 
> > On Tue, Sep 26, 2017 at 07:17:17PM +0000, Mario.Limonciello@dell.com wrote:
> > > >
> > > > Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
> > > > interested in changes like these to WMI.
> > >
> > > Added.  Andy you weren't CC'ed on the original or v2 submission, but if this
> > > requires a v3 I'll include you.
> > 
> > Andy should be Cc'd on all patches to  platform drivers x86 as we
> > co-maintain the subsystem together.
> 
> Andy S. was CC'ed on all of these, Andy S was recommending that I add Andy L.

Ah, of course. Sorry, too many Andys :-)

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

[Darren Hart]

On Tue, Sep 26, 2017 at 01:50:00PM -0500, Mario Limonciello wrote:
> These aren't needed to compile the module.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>

Hrm...

CONFIG_DELL_SMBIOS=m

$ make drivers/platform/x86/dell-smbios.ko

  CC [M]  drivers/platform/x86/dell-smbios.o
drivers/platform/x86/dell-smbios.c: In function ‘parse_da_table’:
drivers/platform/x86/dell-smbios.c:142:18: error: implicit declaration of function ‘krealloc’ [-Werror=implicit-function-declaration]
  new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
                  ^~~~~~~~
drivers/platform/x86/dell-smbios.c:142:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
  new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
                ^
drivers/platform/x86/dell-smbios.c: In function ‘dell_smbios_init’:
drivers/platform/x86/dell-smbios.c:193:2: error: implicit declaration of function ‘kfree’ [-Werror=implicit-function-declaration]
  kfree(da_tokens);
  ^~~~~

> ---
>  drivers/platform/x86/dell-smbios.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
> index 0a5723468bff..6654ad44a04c 100644
> --- a/drivers/platform/x86/dell-smbios.c
> +++ b/drivers/platform/x86/dell-smbios.c
> @@ -17,10 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/dmi.h>
>  #include <linux/err.h>
> -#include <linux/gfp.h>
>  #include <linux/mutex.h>
> -#include <linux/slab.h>

Appears you do require slab.h...

> -#include <linux/io.h>
>  #include "../../firmware/dcdbas.h"
>  #include "dell-smbios.h"
>  
> -- 
> 2.14.1



> 
> 

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor GUID modalias

[Darren Hart]

On Wed, Sep 27, 2017 at 05:30:40PM +0000, Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Darren Hart [mailto:dvhart@infradead.org]
> > Sent: Wednesday, September 27, 2017 1:22 PM
> > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> > kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> > quasisec@google.com; pali.rohar@gmail.com
> > Subject: Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor
> > GUID modalias
> > 
> > On Tue, Sep 26, 2017 at 01:50:01PM -0500, Mario Limonciello wrote:
> > > The descriptor GUID is not used to indicate that WMI notifications
> > > in the dell-wmi driver work properly.  As such a modalias should
> > > not be present that causes this driver to load on systems with this
> > > GUID.
> > >
> > > Reviewed-by: Pali Pohar <pali.rohar@gmail.com>
> > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > 
> > I can do this manually, but for future reference:
> > 
> > Nit, in general, the author's sign off should be first, followed by
> > reviewers and testers. The maintainer will add their sign off at the end
> > - this keeps things clear regarding the development and delivery path.
> > In general, assume chronological order and you'll have the right idea.
> > 
> > Informational tags, like Reported, Suggested, Tested, and Fixes can come
> > before the Author sign off, which is consistent with the chronological
> > order.
> > 
> 
> Operationally is there is a flag I'm missing in git-format-patch that does
> this?  git format-patch -s puts my sign off at the very bottom, so this is a
> manual action to re-order if I'm adding in Reviewed-by: in the patch
> description.

Yes, this is a manual step. Git will add your sign off for you, but it doesn't
do it contextually, always at the end. Works for the first writing, after that
you have to manage the tags manually. (or at least that is what I do)

-- 
Darren Hart
VMware Open Source Technology Center

RE: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

[Mario.Limonciello]

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Wednesday, September 27, 2017 1:40 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com; pali.rohar@gmail.com
> Subject: Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes
> 
> On Tue, Sep 26, 2017 at 01:50:00PM -0500, Mario Limonciello wrote:
> > These aren't needed to compile the module.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> 
> Hrm...
> 
> CONFIG_DELL_SMBIOS=m
> 
> $ make drivers/platform/x86/dell-smbios.ko
> 
>   CC [M]  drivers/platform/x86/dell-smbios.o
> drivers/platform/x86/dell-smbios.c: In function ‘parse_da_table’:
> drivers/platform/x86/dell-smbios.c:142:18: error: implicit declaration of function
> ‘krealloc’ [-Werror=implicit-function-declaration]
>   new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>                   ^~~~~~~~
> drivers/platform/x86/dell-smbios.c:142:16: warning: assignment makes pointer
> from integer without a cast [-Wint-conversion]
>   new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>                 ^
> drivers/platform/x86/dell-smbios.c: In function ‘dell_smbios_init’:
> drivers/platform/x86/dell-smbios.c:193:2: error: implicit declaration of function
> ‘kfree’ [-Werror=implicit-function-declaration]
>   kfree(da_tokens);
>   ^~~~~
> 

Thanks, I've obviously been testing this and didn't notice it.
Not sure why that wasn't exploding on my end.  
Anyway I'll fix for v3.

> > ---
> >  drivers/platform/x86/dell-smbios.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-
> smbios.c
> > index 0a5723468bff..6654ad44a04c 100644
> > --- a/drivers/platform/x86/dell-smbios.c
> > +++ b/drivers/platform/x86/dell-smbios.c
> > @@ -17,10 +17,7 @@
> >  #include <linux/module.h>
> >  #include <linux/dmi.h>
> >  #include <linux/err.h>
> > -#include <linux/gfp.h>
> >  #include <linux/mutex.h>
> > -#include <linux/slab.h>
> 
> Appears you do require slab.h...
> 
> > -#include <linux/io.h>
> >  #include "../../firmware/dcdbas.h"
> >  #include "dell-smbios.h"
> >
> > --
> > 2.14.1
> 
> 
> 
> >
> >
> 
> --
> Darren Hart
> VMware Open Source Technology Center

RE: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

[Mario.Limonciello]

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Wednesday, September 27, 2017 1:40 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com; pali.rohar@gmail.com
> Subject: Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes
> 
> On Tue, Sep 26, 2017 at 01:50:00PM -0500, Mario Limonciello wrote:
> > These aren't needed to compile the module.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> 
> Hrm...
> 
> CONFIG_DELL_SMBIOS=m
> 
> $ make drivers/platform/x86/dell-smbios.ko
> 
>   CC [M]  drivers/platform/x86/dell-smbios.o
> drivers/platform/x86/dell-smbios.c: In function ‘parse_da_table’:
> drivers/platform/x86/dell-smbios.c:142:18: error: implicit declaration of function
> ‘krealloc’ [-Werror=implicit-function-declaration]
>   new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>                   ^~~~~~~~
> drivers/platform/x86/dell-smbios.c:142:16: warning: assignment makes pointer
> from integer without a cast [-Wint-conversion]
>   new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>                 ^
> drivers/platform/x86/dell-smbios.c: In function ‘dell_smbios_init’:
> drivers/platform/x86/dell-smbios.c:193:2: error: implicit declaration of function
> ‘kfree’ [-Werror=implicit-function-declaration]
>   kfree(da_tokens);
>   ^~~~~
> 

Thanks, I've obviously been testing this and didn't notice it.
Not sure why that wasn't exploding on my end.  
Anyway I'll fix for v3.

> > ---
> >  drivers/platform/x86/dell-smbios.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-
> smbios.c
> > index 0a5723468bff..6654ad44a04c 100644
> > --- a/drivers/platform/x86/dell-smbios.c
> > +++ b/drivers/platform/x86/dell-smbios.c
> > @@ -17,10 +17,7 @@
> >  #include <linux/module.h>
> >  #include <linux/dmi.h>
> >  #include <linux/err.h>
> > -#include <linux/gfp.h>
> >  #include <linux/mutex.h>
> > -#include <linux/slab.h>
> 
> Appears you do require slab.h...
> 
> > -#include <linux/io.h>
> >  #include "../../firmware/dcdbas.h"
> >  #include "dell-smbios.h"
> >
> > --
> > 2.14.1
> 
> 
> 
> >
> >
> 
> --
> Darren Hart
> VMware Open Source Technology Center

Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

[Darren Hart]

On Wed, Sep 27, 2017 at 10:40:19AM -0700, Darren Hart wrote:
> On Tue, Sep 26, 2017 at 01:50:00PM -0500, Mario Limonciello wrote:
> > These aren't needed to compile the module.
> > 
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> 
> Hrm...
> 
> CONFIG_DELL_SMBIOS=m
> 
> $ make drivers/platform/x86/dell-smbios.ko
> 
>   CC [M]  drivers/platform/x86/dell-smbios.o
> drivers/platform/x86/dell-smbios.c: In function ‘parse_da_table’:
> drivers/platform/x86/dell-smbios.c:142:18: error: implicit declaration of function ‘krealloc’ [-Werror=implicit-function-declaration]
>   new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>                   ^~~~~~~~
> drivers/platform/x86/dell-smbios.c:142:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
>   new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>                 ^
> drivers/platform/x86/dell-smbios.c: In function ‘dell_smbios_init’:
> drivers/platform/x86/dell-smbios.c:193:2: error: implicit declaration of function ‘kfree’ [-Werror=implicit-function-declaration]
>   kfree(da_tokens);
>   ^~~~~
> 
> > ---
> >  drivers/platform/x86/dell-smbios.c | 3 ---
...
> > -#include <linux/slab.h>
> 
> Appears you do require slab.h...
> 

I've dropped this one line from this patch, and build is working again. Patches
1-7 look ready to go to me. I'm pushing these to my review branch from some
additional automated testing.

Let me know if there are any objections for 1-7. If we can get these queued now,
we can simplify the review effort for the major functional changes in 8-14.

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v2 08/14] platform/x86: dell-smbios: Introduce a WMI-ACPI interface

[Darren Hart]

On Tue, Sep 26, 2017 at 01:50:06PM -0500, Mario Limonciello wrote:
> The driver currently uses an SMI interface which grants direct access
> to physical memory to the platform via a pointer.
> 
> Now add a WMI-ACPI interface that is detected by WMI probe and preferred
> over the SMI interface.
> 
> Changing this to operate over WMI-ACPI will use an ACPI OperationRegion
> for a buffer of data storage when platform calls are performed.
> 
> This is a safer approach to use in kernel drivers as the platform will
> only have access to that OperationRegion.

As we discussed, in v3 please update the "platform" language here to clarify
when read from the Linux kernel developer perspective (one who is working on
code in the platform drivers subsystem ;-)

> 
> As a result, this change removes the dependency on this driver on the
> dcdbas kernel module.  It's now an optional compilation option.

Some update tot he Kconfig help informing the user of how to decide if they
should go and enable DCDBAS or not would be appropriate.

> diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
...
> -#include "../../firmware/dcdbas.h"
> +#include <linux/wmi.h>
>  #include "dell-smbios.h"
>  
> +#ifdef CONFIG_DCDBAS
> +#include "../../firmware/dcdbas.h"
> +#endif

Perhaps this is the conditional include causing the build breakage?

> +
> +#define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
> +
>  struct calling_interface_structure {
>  	struct dmi_header header;
>  	u16 cmdIOAddress;
> @@ -30,12 +37,14 @@ struct calling_interface_structure {
>  	struct calling_interface_token tokens[];
>  } __packed;
>  
> -static struct calling_interface_buffer *buffer;
> +static struct calling_interface_buffer *smi_buffer;
> +static struct wmi_calling_interface_buffer *wmi_buffer;
>  static DEFINE_MUTEX(buffer_mutex);
>  
>  static int da_command_address;
>  static int da_command_code;
>  static int da_num_tokens;
> +static int has_wmi;
>  static struct calling_interface_token *da_tokens;
>  
>  int dell_smbios_error(int value)
> @@ -57,13 +66,20 @@ struct calling_interface_buffer *dell_smbios_get_buffer(void)
>  {
>  	mutex_lock(&buffer_mutex);
>  	dell_smbios_clear_buffer();
> -	return buffer;
> +	if (has_wmi)
> +		return &wmi_buffer->smi;
> +	return smi_buffer;
>  }
>  EXPORT_SYMBOL_GPL(dell_smbios_get_buffer);
>  
>  void dell_smbios_clear_buffer(void)
>  {
> -	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> +	if (has_wmi)
> +		memset(wmi_buffer, 0,
> +		       sizeof(struct wmi_calling_interface_buffer));
> +	else
> +		memset(smi_buffer, 0,
> +		       sizeof(struct calling_interface_buffer));
>  }
>  EXPORT_SYMBOL_GPL(dell_smbios_clear_buffer);

Would it be possible to dynamically setup "buffer" and "buflen" during
init or something, and avoid all this if/else dance at runtime?

>  
> @@ -73,20 +89,60 @@ void dell_smbios_release_buffer(void)
>  }
>  EXPORT_SYMBOL_GPL(dell_smbios_release_buffer);
>  
> -void dell_smbios_send_request(int class, int select)
> +int run_wmi_smbios_call(struct wmi_calling_interface_buffer *buf)
>  {
> -	struct smi_cmd command;
> +	struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
> +	struct acpi_buffer input;
> +	union acpi_object *obj;
> +	acpi_status status;
> +
> +	input.length = sizeof(struct wmi_calling_interface_buffer);
> +	input.pointer = buf;
> +
> +	status = wmi_evaluate_method(DELL_WMI_SMBIOS_GUID,
> +				     0, 1, &input, &output);
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("%x/%x [%x,%x,%x,%x] call failed\n",
> +			buf->smi.class, buf->smi.select,
> +			buf->smi.input[0], buf->smi.input[1],
> +			buf->smi.input[2], buf->smi.input[3]);
> +			return -EIO;
> +	}
> +	obj = (union acpi_object *)output.pointer;
> +	if (obj->type != ACPI_TYPE_BUFFER) {
> +		pr_err("invalid type : %d\n", obj->type);
> +		return -EIO;
> +	}
> +	memcpy(buf, obj->buffer.pointer, input.length);
>  
> -	command.magic = SMI_CMD_MAGIC;
> -	command.command_address = da_command_address;
> -	command.command_code = da_command_code;
> -	command.ebx = virt_to_phys(buffer);
> -	command.ecx = 0x42534931;
> +	return 0;
> +}
>  
> -	buffer->class = class;
> -	buffer->select = select;
> +void dell_smbios_send_request(int class, int select)
> +{
> +	if (has_wmi) {
> +		wmi_buffer->smi.class = class;
> +		wmi_buffer->smi.select = select;
> +		run_wmi_smbios_call(wmi_buffer);
> +	}
>  
> -	dcdbas_smi_request(&command);
> +#ifdef CONFIG_DCDBAS

Rather than ifdef blocks of code, the preferred method is to modify the
declaration of things so the tests here can do the right thing. For
example, if CONFIG_DCDBAS is not set, can that be made equivalent to:

else if (smi_buffer) {

At this point in the code?

See coding-style.rst Section 20) Conditional Compilation
for more specific guidance. Apply throughout.

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios

[Darren Hart]

On Tue, Sep 26, 2017 at 08:18:54PM +0000, Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > Sent: Tuesday, September 26, 2017 4:06 PM
> > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > Cc: dvhart@infradead.org; Andy Shevchenko <andy.shevchenko@gmail.com>;
> > LKML <linux-kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> > quasisec@google.com
> > Subject: Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-
> > smbios
> > 
> > On Tuesday 26 September 2017 20:50:07 Mario Limonciello wrote:
> > > This follows the style of the rest of the platform x86 WMI drivers.
> > >
> > > Renaming the driver requires adjusting the other drivers using
> > > dell-smbios to pick up the newly named includes.
> > >
> > > While renaming, I noticed that this driver was missing from
> > > MAINTAINERS. Add it to that and myself to the list of people
> > > maintaing it.
> > >
> > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > 
> > Now when driver supports both old and new WMI call interface, I think
> > that renaming is not needed...
> > 
> 
> So all other drivers that use WMI have "WMI" in the name, this would be
> breaking previously set precedence.  Daren, Andy, what would you like
> me to do?

I'm leaning toward Pali's point of view here. In fact, I think the driver name
should reflect WHAT it does as opposed to HOW it does it. vendor-hotkeys is a
much better name than vendor-wmi. We have a lot of drivers that are not nicely
granular and use more generic terms like -acpi or -laptop, but those shouldn't
be the first choice in my opinion. Now, is SMBIOS any better than WMI? :-) Not
really, but it is at least as good as, and less change makes it the winner IMO.

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios

[kbuild test robot]

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

Hi Mario,

[auto build test WARNING on platform-drivers-x86/for-next]
[also build test WARNING on v4.14-rc2 next-20170928]
[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/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20170929-221513
base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: x86_64-randconfig-x001-201739 (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 warnings (new ones prefixed by >>):

       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:293:3: note: in expansion of macro 'if'
      if (len >= p_size)
      ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:291:3: note: in expansion of macro 'if'
      if (__builtin_constant_p(len) && len >= p_size)
      ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:289:2: note: in expansion of macro 'if'
     if (size) {
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:286:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1 && q_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strnlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:274:2: note: in expansion of macro 'if'
     if (p_size <= ret && maxlen != ret)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:264:2: note: in expansion of macro 'if'
     if (p_size <= ret)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:261:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcat' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:252:2: note: in expansion of macro 'if'
     if (strlcat(p, q, p_size) >= p_size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcat' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:250:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strncpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:242:2: note: in expansion of macro 'if'
     if (p_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strncpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:240:2: note: in expansion of macro 'if'
     if (__builtin_constant_p(size) && p_size < size)
     ^~
   In file included from drivers/platform//x86/dell-wmi-smbios.c:23:0:
   drivers/platform//x86/dell-wmi-smbios.c: In function 'dell_wmi_smbios_init':
>> include/linux/wmi.h:53:37: warning: ignoring return value of '__wmi_driver_register', declared with attribute warn_unused_result [-Wunused-result]
    #define wmi_driver_register(driver) __wmi_driver_register((driver), THIS_MODULE)
                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/platform//x86/dell-wmi-smbios.c:281:2: note: in expansion of macro 'wmi_driver_register'
     wmi_driver_register(&dell_wmi_smbios_driver);
     ^~~~~~~~~~~~~~~~~~~
--
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:293:3: note: in expansion of macro 'if'
      if (len >= p_size)
      ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:291:3: note: in expansion of macro 'if'
      if (__builtin_constant_p(len) && len >= p_size)
      ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:289:2: note: in expansion of macro 'if'
     if (size) {
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:286:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1 && q_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strnlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:274:2: note: in expansion of macro 'if'
     if (p_size <= ret && maxlen != ret)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:264:2: note: in expansion of macro 'if'
     if (p_size <= ret)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:261:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcat' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:252:2: note: in expansion of macro 'if'
     if (strlcat(p, q, p_size) >= p_size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcat' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:250:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strncpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:242:2: note: in expansion of macro 'if'
     if (p_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strncpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:240:2: note: in expansion of macro 'if'
     if (__builtin_constant_p(size) && p_size < size)
     ^~
   In file included from drivers/platform/x86/dell-wmi-smbios.c:23:0:
   drivers/platform/x86/dell-wmi-smbios.c: In function 'dell_wmi_smbios_init':
>> include/linux/wmi.h:53:37: warning: ignoring return value of '__wmi_driver_register', declared with attribute warn_unused_result [-Wunused-result]
    #define wmi_driver_register(driver) __wmi_driver_register((driver), THIS_MODULE)
                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/platform/x86/dell-wmi-smbios.c:281:2: note: in expansion of macro 'wmi_driver_register'
     wmi_driver_register(&dell_wmi_smbios_driver);
     ^~~~~~~~~~~~~~~~~~~

vim +/__wmi_driver_register +53 include/linux/wmi.h

844af950 Andy Lutomirski 2015-11-24  49  
844af950 Andy Lutomirski 2015-11-24  50  extern int __must_check __wmi_driver_register(struct wmi_driver *driver,
844af950 Andy Lutomirski 2015-11-24  51  					      struct module *owner);
844af950 Andy Lutomirski 2015-11-24  52  extern void wmi_driver_unregister(struct wmi_driver *driver);
844af950 Andy Lutomirski 2015-11-24 @53  #define wmi_driver_register(driver) __wmi_driver_register((driver), THIS_MODULE)
844af950 Andy Lutomirski 2015-11-24  54  

:::::: The code at line 53 was first introduced by commit
:::::: 844af950da946cfab227a04b950614da04cb6275 platform/x86: wmi: Turn WMI into a bus driver

:::::: TO: Andy Lutomirski <luto@kernel.org>
:::::: CC: Darren Hart (VMware) <dvhart@infradead.org>

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

Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios

[kbuild test robot]

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

Hi Mario,

[auto build test WARNING on platform-drivers-x86/for-next]
[also build test WARNING on v4.14-rc2 next-20170928]
[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/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20170929-221513
base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: x86_64-randconfig-x001-201739 (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 warnings (new ones prefixed by >>):

       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:293:3: note: in expansion of macro 'if'
      if (len >= p_size)
      ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:291:3: note: in expansion of macro 'if'
      if (__builtin_constant_p(len) && len >= p_size)
      ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:289:2: note: in expansion of macro 'if'
     if (size) {
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:286:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1 && q_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strnlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:274:2: note: in expansion of macro 'if'
     if (p_size <= ret && maxlen != ret)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:264:2: note: in expansion of macro 'if'
     if (p_size <= ret)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:261:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcat' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:252:2: note: in expansion of macro 'if'
     if (strlcat(p, q, p_size) >= p_size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcat' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:250:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strncpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:242:2: note: in expansion of macro 'if'
     if (p_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strncpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:240:2: note: in expansion of macro 'if'
     if (__builtin_constant_p(size) && p_size < size)
     ^~
   In file included from drivers/platform//x86/dell-wmi-smbios.c:23:0:
   drivers/platform//x86/dell-wmi-smbios.c: In function 'dell_wmi_smbios_init':
>> include/linux/wmi.h:53:37: warning: ignoring return value of '__wmi_driver_register', declared with attribute warn_unused_result [-Wunused-result]
    #define wmi_driver_register(driver) __wmi_driver_register((driver), THIS_MODULE)
                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/platform//x86/dell-wmi-smbios.c:281:2: note: in expansion of macro 'wmi_driver_register'
     wmi_driver_register(&dell_wmi_smbios_driver);
     ^~~~~~~~~~~~~~~~~~~
--
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:293:3: note: in expansion of macro 'if'
      if (len >= p_size)
      ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:291:3: note: in expansion of macro 'if'
      if (__builtin_constant_p(len) && len >= p_size)
      ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:289:2: note: in expansion of macro 'if'
     if (size) {
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:286:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1 && q_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strnlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:274:2: note: in expansion of macro 'if'
     if (p_size <= ret && maxlen != ret)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:264:2: note: in expansion of macro 'if'
     if (p_size <= ret)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strlen' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:261:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcat' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:252:2: note: in expansion of macro 'if'
     if (strlcat(p, q, p_size) >= p_size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcat' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:250:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strncpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:242:2: note: in expansion of macro 'if'
     if (p_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strncpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:240:2: note: in expansion of macro 'if'
     if (__builtin_constant_p(size) && p_size < size)
     ^~
   In file included from drivers/platform/x86/dell-wmi-smbios.c:23:0:
   drivers/platform/x86/dell-wmi-smbios.c: In function 'dell_wmi_smbios_init':
>> include/linux/wmi.h:53:37: warning: ignoring return value of '__wmi_driver_register', declared with attribute warn_unused_result [-Wunused-result]
    #define wmi_driver_register(driver) __wmi_driver_register((driver), THIS_MODULE)
                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/platform/x86/dell-wmi-smbios.c:281:2: note: in expansion of macro 'wmi_driver_register'
     wmi_driver_register(&dell_wmi_smbios_driver);
     ^~~~~~~~~~~~~~~~~~~

vim +/__wmi_driver_register +53 include/linux/wmi.h

844af950 Andy Lutomirski 2015-11-24  49  
844af950 Andy Lutomirski 2015-11-24  50  extern int __must_check __wmi_driver_register(struct wmi_driver *driver,
844af950 Andy Lutomirski 2015-11-24  51  					      struct module *owner);
844af950 Andy Lutomirski 2015-11-24  52  extern void wmi_driver_unregister(struct wmi_driver *driver);
844af950 Andy Lutomirski 2015-11-24 @53  #define wmi_driver_register(driver) __wmi_driver_register((driver), THIS_MODULE)
844af950 Andy Lutomirski 2015-11-24  54  

:::::: The code at line 53 was first introduced by commit
:::::: 844af950da946cfab227a04b950614da04cb6275 platform/x86: wmi: Turn WMI into a bus driver

:::::: TO: Andy Lutomirski <luto@kernel.org>
:::::: CC: Darren Hart (VMware) <dvhart@infradead.org>

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

Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

[kbuild test robot]

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

Hi Mario,

[auto build test ERROR on platform-drivers-x86/for-next]
[also build test ERROR on v4.14-rc2 next-20170928]
[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/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20170929-221513
base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: i386-randconfig-x013-201739 (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=i386 

Note: the linux-review/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20170929-221513 HEAD 95464efdaabbd0db3ed21656ce0f227cfe01ef0c builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   drivers/platform/x86/dell-smbios.c: In function 'parse_da_table':
>> drivers/platform/x86/dell-smbios.c:142:18: error: implicit declaration of function 'krealloc' [-Werror=implicit-function-declaration]
     new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
                     ^~~~~~~~
>> drivers/platform/x86/dell-smbios.c:142:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
                   ^
   drivers/platform/x86/dell-smbios.c: In function 'dell_smbios_init':
>> drivers/platform/x86/dell-smbios.c:193:2: error: implicit declaration of function 'kfree' [-Werror=implicit-function-declaration]
     kfree(da_tokens);
     ^~~~~
   cc1: some warnings being treated as errors

vim +/krealloc +142 drivers/platform/x86/dell-smbios.c

504b0259 Hans de Goede 2017-03-16  124  
2f9f26bd Michał Kępień 2016-01-22  125  static void __init parse_da_table(const struct dmi_header *dm)
2f9f26bd Michał Kępień 2016-01-22  126  {
2f9f26bd Michał Kępień 2016-01-22  127  	/* Final token is a terminator, so we don't want to copy it */
2f9f26bd Michał Kępień 2016-01-22  128  	int tokens = (dm->length-11)/sizeof(struct calling_interface_token)-1;
2f9f26bd Michał Kępień 2016-01-22  129  	struct calling_interface_token *new_da_tokens;
2f9f26bd Michał Kępień 2016-01-22  130  	struct calling_interface_structure *table =
2f9f26bd Michał Kępień 2016-01-22  131  		container_of(dm, struct calling_interface_structure, header);
2f9f26bd Michał Kępień 2016-01-22  132  
2f9f26bd Michał Kępień 2016-01-22  133  	/* 4 bytes of table header, plus 7 bytes of Dell header, plus at least
2f9f26bd Michał Kępień 2016-01-22  134  	   6 bytes of entry */
2f9f26bd Michał Kępień 2016-01-22  135  
2f9f26bd Michał Kępień 2016-01-22  136  	if (dm->length < 17)
2f9f26bd Michał Kępień 2016-01-22  137  		return;
2f9f26bd Michał Kępień 2016-01-22  138  
2f9f26bd Michał Kępień 2016-01-22  139  	da_command_address = table->cmdIOAddress;
2f9f26bd Michał Kępień 2016-01-22  140  	da_command_code = table->cmdIOCode;
2f9f26bd Michał Kępień 2016-01-22  141  
2f9f26bd Michał Kępień 2016-01-22 @142  	new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
2f9f26bd Michał Kępień 2016-01-22  143  				 sizeof(struct calling_interface_token),
2f9f26bd Michał Kępień 2016-01-22  144  				 GFP_KERNEL);
2f9f26bd Michał Kępień 2016-01-22  145  
2f9f26bd Michał Kępień 2016-01-22  146  	if (!new_da_tokens)
2f9f26bd Michał Kępień 2016-01-22  147  		return;
2f9f26bd Michał Kępień 2016-01-22  148  	da_tokens = new_da_tokens;
2f9f26bd Michał Kępień 2016-01-22  149  
2f9f26bd Michał Kępień 2016-01-22  150  	memcpy(da_tokens+da_num_tokens, table->tokens,
2f9f26bd Michał Kępień 2016-01-22  151  	       sizeof(struct calling_interface_token) * tokens);
2f9f26bd Michał Kępień 2016-01-22  152  
2f9f26bd Michał Kępień 2016-01-22  153  	da_num_tokens += tokens;
2f9f26bd Michał Kępień 2016-01-22  154  }
2f9f26bd Michał Kępień 2016-01-22  155  
2f9f26bd Michał Kępień 2016-01-22  156  static void __init find_tokens(const struct dmi_header *dm, void *dummy)
2f9f26bd Michał Kępień 2016-01-22  157  {
2f9f26bd Michał Kępień 2016-01-22  158  	switch (dm->type) {
2f9f26bd Michał Kępień 2016-01-22  159  	case 0xd4: /* Indexed IO */
2f9f26bd Michał Kępień 2016-01-22  160  	case 0xd5: /* Protected Area Type 1 */
2f9f26bd Michał Kępień 2016-01-22  161  	case 0xd6: /* Protected Area Type 2 */
2f9f26bd Michał Kępień 2016-01-22  162  		break;
2f9f26bd Michał Kępień 2016-01-22  163  	case 0xda: /* Calling interface */
2f9f26bd Michał Kępień 2016-01-22  164  		parse_da_table(dm);
2f9f26bd Michał Kępień 2016-01-22  165  		break;
2f9f26bd Michał Kępień 2016-01-22  166  	}
2f9f26bd Michał Kępień 2016-01-22  167  }
2f9f26bd Michał Kępień 2016-01-22  168  
2f9f26bd Michał Kępień 2016-01-22  169  static int __init dell_smbios_init(void)
2f9f26bd Michał Kępień 2016-01-22  170  {
2f9f26bd Michał Kępień 2016-01-22  171  	int ret;
2f9f26bd Michał Kępień 2016-01-22  172  
2f9f26bd Michał Kępień 2016-01-22  173  	dmi_walk(find_tokens, NULL);
2f9f26bd Michał Kępień 2016-01-22  174  
2f9f26bd Michał Kępień 2016-01-22  175  	if (!da_tokens)  {
2f9f26bd Michał Kępień 2016-01-22  176  		pr_info("Unable to find dmi tokens\n");
2f9f26bd Michał Kępień 2016-01-22  177  		return -ENODEV;
2f9f26bd Michał Kępień 2016-01-22  178  	}
2f9f26bd Michał Kępień 2016-01-22  179  
2f9f26bd Michał Kępień 2016-01-22  180  	/*
2f9f26bd Michał Kępień 2016-01-22  181  	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
2f9f26bd Michał Kępień 2016-01-22  182  	 * is passed to SMI handler.
2f9f26bd Michał Kępień 2016-01-22  183  	 */
2f9f26bd Michał Kępień 2016-01-22  184  	buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
2f9f26bd Michał Kępień 2016-01-22  185  	if (!buffer) {
2f9f26bd Michał Kępień 2016-01-22  186  		ret = -ENOMEM;
2f9f26bd Michał Kępień 2016-01-22  187  		goto fail_buffer;
2f9f26bd Michał Kępień 2016-01-22  188  	}
2f9f26bd Michał Kępień 2016-01-22  189  
2f9f26bd Michał Kępień 2016-01-22  190  	return 0;
2f9f26bd Michał Kępień 2016-01-22  191  
2f9f26bd Michał Kępień 2016-01-22  192  fail_buffer:
2f9f26bd Michał Kępień 2016-01-22 @193  	kfree(da_tokens);
2f9f26bd Michał Kępień 2016-01-22  194  	return ret;
2f9f26bd Michał Kępień 2016-01-22  195  }
2f9f26bd Michał Kępień 2016-01-22  196  

:::::: The code at line 142 was first introduced by commit
:::::: 2f9f26bd8614740b3c3b950394d945a99492a28e dell-laptop: extract SMBIOS-related code to a separate module

:::::: TO: Michał Kępień <kernel@kempniu.pl>
:::::: CC: Darren Hart <dvhart@linux.intel.com>

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

Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

[kbuild test robot]

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

Hi Mario,

[auto build test ERROR on platform-drivers-x86/for-next]
[also build test ERROR on v4.14-rc2 next-20170928]
[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/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20170929-221513
base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: i386-randconfig-x013-201739 (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=i386 

Note: the linux-review/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20170929-221513 HEAD 95464efdaabbd0db3ed21656ce0f227cfe01ef0c builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   drivers/platform/x86/dell-smbios.c: In function 'parse_da_table':
>> drivers/platform/x86/dell-smbios.c:142:18: error: implicit declaration of function 'krealloc' [-Werror=implicit-function-declaration]
     new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
                     ^~~~~~~~
>> drivers/platform/x86/dell-smbios.c:142:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
                   ^
   drivers/platform/x86/dell-smbios.c: In function 'dell_smbios_init':
>> drivers/platform/x86/dell-smbios.c:193:2: error: implicit declaration of function 'kfree' [-Werror=implicit-function-declaration]
     kfree(da_tokens);
     ^~~~~
   cc1: some warnings being treated as errors

vim +/krealloc +142 drivers/platform/x86/dell-smbios.c

504b0259 Hans de Goede 2017-03-16  124  
2f9f26bd Michał Kępień 2016-01-22  125  static void __init parse_da_table(const struct dmi_header *dm)
2f9f26bd Michał Kępień 2016-01-22  126  {
2f9f26bd Michał Kępień 2016-01-22  127  	/* Final token is a terminator, so we don't want to copy it */
2f9f26bd Michał Kępień 2016-01-22  128  	int tokens = (dm->length-11)/sizeof(struct calling_interface_token)-1;
2f9f26bd Michał Kępień 2016-01-22  129  	struct calling_interface_token *new_da_tokens;
2f9f26bd Michał Kępień 2016-01-22  130  	struct calling_interface_structure *table =
2f9f26bd Michał Kępień 2016-01-22  131  		container_of(dm, struct calling_interface_structure, header);
2f9f26bd Michał Kępień 2016-01-22  132  
2f9f26bd Michał Kępień 2016-01-22  133  	/* 4 bytes of table header, plus 7 bytes of Dell header, plus at least
2f9f26bd Michał Kępień 2016-01-22  134  	   6 bytes of entry */
2f9f26bd Michał Kępień 2016-01-22  135  
2f9f26bd Michał Kępień 2016-01-22  136  	if (dm->length < 17)
2f9f26bd Michał Kępień 2016-01-22  137  		return;
2f9f26bd Michał Kępień 2016-01-22  138  
2f9f26bd Michał Kępień 2016-01-22  139  	da_command_address = table->cmdIOAddress;
2f9f26bd Michał Kępień 2016-01-22  140  	da_command_code = table->cmdIOCode;
2f9f26bd Michał Kępień 2016-01-22  141  
2f9f26bd Michał Kępień 2016-01-22 @142  	new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
2f9f26bd Michał Kępień 2016-01-22  143  				 sizeof(struct calling_interface_token),
2f9f26bd Michał Kępień 2016-01-22  144  				 GFP_KERNEL);
2f9f26bd Michał Kępień 2016-01-22  145  
2f9f26bd Michał Kępień 2016-01-22  146  	if (!new_da_tokens)
2f9f26bd Michał Kępień 2016-01-22  147  		return;
2f9f26bd Michał Kępień 2016-01-22  148  	da_tokens = new_da_tokens;
2f9f26bd Michał Kępień 2016-01-22  149  
2f9f26bd Michał Kępień 2016-01-22  150  	memcpy(da_tokens+da_num_tokens, table->tokens,
2f9f26bd Michał Kępień 2016-01-22  151  	       sizeof(struct calling_interface_token) * tokens);
2f9f26bd Michał Kępień 2016-01-22  152  
2f9f26bd Michał Kępień 2016-01-22  153  	da_num_tokens += tokens;
2f9f26bd Michał Kępień 2016-01-22  154  }
2f9f26bd Michał Kępień 2016-01-22  155  
2f9f26bd Michał Kępień 2016-01-22  156  static void __init find_tokens(const struct dmi_header *dm, void *dummy)
2f9f26bd Michał Kępień 2016-01-22  157  {
2f9f26bd Michał Kępień 2016-01-22  158  	switch (dm->type) {
2f9f26bd Michał Kępień 2016-01-22  159  	case 0xd4: /* Indexed IO */
2f9f26bd Michał Kępień 2016-01-22  160  	case 0xd5: /* Protected Area Type 1 */
2f9f26bd Michał Kępień 2016-01-22  161  	case 0xd6: /* Protected Area Type 2 */
2f9f26bd Michał Kępień 2016-01-22  162  		break;
2f9f26bd Michał Kępień 2016-01-22  163  	case 0xda: /* Calling interface */
2f9f26bd Michał Kępień 2016-01-22  164  		parse_da_table(dm);
2f9f26bd Michał Kępień 2016-01-22  165  		break;
2f9f26bd Michał Kępień 2016-01-22  166  	}
2f9f26bd Michał Kępień 2016-01-22  167  }
2f9f26bd Michał Kępień 2016-01-22  168  
2f9f26bd Michał Kępień 2016-01-22  169  static int __init dell_smbios_init(void)
2f9f26bd Michał Kępień 2016-01-22  170  {
2f9f26bd Michał Kępień 2016-01-22  171  	int ret;
2f9f26bd Michał Kępień 2016-01-22  172  
2f9f26bd Michał Kępień 2016-01-22  173  	dmi_walk(find_tokens, NULL);
2f9f26bd Michał Kępień 2016-01-22  174  
2f9f26bd Michał Kępień 2016-01-22  175  	if (!da_tokens)  {
2f9f26bd Michał Kępień 2016-01-22  176  		pr_info("Unable to find dmi tokens\n");
2f9f26bd Michał Kępień 2016-01-22  177  		return -ENODEV;
2f9f26bd Michał Kępień 2016-01-22  178  	}
2f9f26bd Michał Kępień 2016-01-22  179  
2f9f26bd Michał Kępień 2016-01-22  180  	/*
2f9f26bd Michał Kępień 2016-01-22  181  	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
2f9f26bd Michał Kępień 2016-01-22  182  	 * is passed to SMI handler.
2f9f26bd Michał Kępień 2016-01-22  183  	 */
2f9f26bd Michał Kępień 2016-01-22  184  	buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
2f9f26bd Michał Kępień 2016-01-22  185  	if (!buffer) {
2f9f26bd Michał Kępień 2016-01-22  186  		ret = -ENOMEM;
2f9f26bd Michał Kępień 2016-01-22  187  		goto fail_buffer;
2f9f26bd Michał Kępień 2016-01-22  188  	}
2f9f26bd Michał Kępień 2016-01-22  189  
2f9f26bd Michał Kępień 2016-01-22  190  	return 0;
2f9f26bd Michał Kępień 2016-01-22  191  
2f9f26bd Michał Kępień 2016-01-22  192  fail_buffer:
2f9f26bd Michał Kępień 2016-01-22 @193  	kfree(da_tokens);
2f9f26bd Michał Kępień 2016-01-22  194  	return ret;
2f9f26bd Michał Kępień 2016-01-22  195  }
2f9f26bd Michał Kępień 2016-01-22  196  

:::::: The code at line 142 was first introduced by commit
:::::: 2f9f26bd8614740b3c3b950394d945a99492a28e dell-laptop: extract SMBIOS-related code to a separate module

:::::: TO: Michał Kępień <kernel@kempniu.pl>
:::::: CC: Darren Hart <dvhart@linux.intel.com>

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

Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios

[kbuild test robot]

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

Hi Mario,

[auto build test ERROR on platform-drivers-x86/for-next]
[also build test ERROR on v4.14-rc2 next-20170929]
[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/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20170929-221513
base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: i386-randconfig-i0-201739 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/platform/x86/dell-laptop.o: In function `kbd_get_state':
>> dell-laptop.c:(.text+0xa1): undefined reference to `dell_smbios_get_buffer'
>> dell-laptop.c:(.text+0xb9): undefined reference to `dell_smbios_send_request'
>> dell-laptop.c:(.text+0xe0): undefined reference to `dell_smbios_error'
>> dell-laptop.c:(.text+0x178): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `kbd_set_state':
   dell-laptop.c:(.text+0x19d): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0x22c): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x234): undefined reference to `dell_smbios_release_buffer'
   dell-laptop.c:(.text+0x23b): undefined reference to `dell_smbios_error'
   drivers/platform/x86/dell-laptop.o: In function `dell_send_intensity':
>> dell-laptop.c:(.text+0xa53): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0xa77): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0xab6): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0xacb): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0xad3): undefined reference to `dell_smbios_error'
   dell-laptop.c:(.text+0xada): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `dell_get_intensity':
   dell-laptop.c:(.text+0xb10): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0xb38): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0xb6f): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0xb80): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0xba8): undefined reference to `dell_smbios_error'
   dell-laptop.c:(.text+0xbb4): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `dell_debugfs_show':
   dell-laptop.c:(.text+0xd71): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0xd82): undefined reference to `dell_smbios_send_request'
>> dell-laptop.c:(.text+0xd90): undefined reference to `dell_smbios_clear_buffer'
   dell-laptop.c:(.text+0xda6): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0xdb4): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `kbd_led_level_set':
   dell-laptop.c:(.text+0x1433): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0x1457): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0x1473): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x147b): undefined reference to `dell_smbios_release_buffer'
   dell-laptop.c:(.text+0x1482): undefined reference to `dell_smbios_error'
   drivers/platform/x86/dell-laptop.o: In function `kbd_led_level_get':
   dell-laptop.c:(.text+0x1c50): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0x1c78): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0x1c8d): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x1c9e): undefined reference to `dell_smbios_release_buffer'
   dell-laptop.c:(.text+0x1cc2): undefined reference to `dell_smbios_error'
   drivers/platform/x86/dell-laptop.o: In function `dell_rfkill_update_sw_state.isra.8':
   dell-laptop.c:(.text+0x1fc8): undefined reference to `dell_smbios_clear_buffer'
   dell-laptop.c:(.text+0x1fe2): undefined reference to `dell_smbios_send_request'
   drivers/platform/x86/dell-laptop.o: In function `dell_update_rfkill':
   dell-laptop.c:(.text+0x1ffc): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0x200d): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x2039): undefined reference to `dell_smbios_clear_buffer'
   dell-laptop.c:(.text+0x204f): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x2159): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `dell_micmute_led_set':
   dell-laptop.c:(.text+0x21bf): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0x21ec): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0x2210): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0x222a): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x222f): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `dell_init':
>> dell-laptop.c:(.init.text+0x16c): undefined reference to `dell_smbios_get_buffer'
>> dell-laptop.c:(.init.text+0x17d): undefined reference to `dell_smbios_send_request'
>> dell-laptop.c:(.init.text+0x188): undefined reference to `dell_smbios_release_buffer'
   dell-laptop.c:(.init.text+0x4b8): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.init.text+0x4d0): undefined reference to `dell_smbios_send_request'
>> dell-laptop.c:(.init.text+0x4f7): undefined reference to `dell_smbios_error'
   dell-laptop.c:(.init.text+0x5ce): undefined reference to `dell_smbios_release_buffer'
>> dell-laptop.c:(.init.text+0x5f9): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.init.text+0x884): undefined reference to `dell_smbios_find_token'
>> dell-laptop.c:(.init.text+0xa10): undefined reference to `dell_laptop_register_notifier'
   dell-laptop.c:(.init.text+0xa20): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.init.text+0xa46): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.init.text+0xa5b): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.init.text+0xa83): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `dell_exit':
>> dell-laptop.c:(.exit.text+0xa): undefined reference to `dell_laptop_unregister_notifier'

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

Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios

[kbuild test robot]

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

Hi Mario,

[auto build test ERROR on platform-drivers-x86/for-next]
[also build test ERROR on v4.14-rc2 next-20170929]
[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/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20170929-221513
base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: i386-randconfig-i0-201739 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/platform/x86/dell-laptop.o: In function `kbd_get_state':
>> dell-laptop.c:(.text+0xa1): undefined reference to `dell_smbios_get_buffer'
>> dell-laptop.c:(.text+0xb9): undefined reference to `dell_smbios_send_request'
>> dell-laptop.c:(.text+0xe0): undefined reference to `dell_smbios_error'
>> dell-laptop.c:(.text+0x178): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `kbd_set_state':
   dell-laptop.c:(.text+0x19d): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0x22c): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x234): undefined reference to `dell_smbios_release_buffer'
   dell-laptop.c:(.text+0x23b): undefined reference to `dell_smbios_error'
   drivers/platform/x86/dell-laptop.o: In function `dell_send_intensity':
>> dell-laptop.c:(.text+0xa53): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0xa77): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0xab6): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0xacb): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0xad3): undefined reference to `dell_smbios_error'
   dell-laptop.c:(.text+0xada): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `dell_get_intensity':
   dell-laptop.c:(.text+0xb10): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0xb38): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0xb6f): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0xb80): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0xba8): undefined reference to `dell_smbios_error'
   dell-laptop.c:(.text+0xbb4): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `dell_debugfs_show':
   dell-laptop.c:(.text+0xd71): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0xd82): undefined reference to `dell_smbios_send_request'
>> dell-laptop.c:(.text+0xd90): undefined reference to `dell_smbios_clear_buffer'
   dell-laptop.c:(.text+0xda6): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0xdb4): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `kbd_led_level_set':
   dell-laptop.c:(.text+0x1433): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0x1457): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0x1473): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x147b): undefined reference to `dell_smbios_release_buffer'
   dell-laptop.c:(.text+0x1482): undefined reference to `dell_smbios_error'
   drivers/platform/x86/dell-laptop.o: In function `kbd_led_level_get':
   dell-laptop.c:(.text+0x1c50): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0x1c78): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0x1c8d): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x1c9e): undefined reference to `dell_smbios_release_buffer'
   dell-laptop.c:(.text+0x1cc2): undefined reference to `dell_smbios_error'
   drivers/platform/x86/dell-laptop.o: In function `dell_rfkill_update_sw_state.isra.8':
   dell-laptop.c:(.text+0x1fc8): undefined reference to `dell_smbios_clear_buffer'
   dell-laptop.c:(.text+0x1fe2): undefined reference to `dell_smbios_send_request'
   drivers/platform/x86/dell-laptop.o: In function `dell_update_rfkill':
   dell-laptop.c:(.text+0x1ffc): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0x200d): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x2039): undefined reference to `dell_smbios_clear_buffer'
   dell-laptop.c:(.text+0x204f): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x2159): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `dell_micmute_led_set':
   dell-laptop.c:(.text+0x21bf): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0x21ec): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.text+0x2210): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.text+0x222a): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.text+0x222f): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `dell_init':
>> dell-laptop.c:(.init.text+0x16c): undefined reference to `dell_smbios_get_buffer'
>> dell-laptop.c:(.init.text+0x17d): undefined reference to `dell_smbios_send_request'
>> dell-laptop.c:(.init.text+0x188): undefined reference to `dell_smbios_release_buffer'
   dell-laptop.c:(.init.text+0x4b8): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.init.text+0x4d0): undefined reference to `dell_smbios_send_request'
>> dell-laptop.c:(.init.text+0x4f7): undefined reference to `dell_smbios_error'
   dell-laptop.c:(.init.text+0x5ce): undefined reference to `dell_smbios_release_buffer'
>> dell-laptop.c:(.init.text+0x5f9): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.init.text+0x884): undefined reference to `dell_smbios_find_token'
>> dell-laptop.c:(.init.text+0xa10): undefined reference to `dell_laptop_register_notifier'
   dell-laptop.c:(.init.text+0xa20): undefined reference to `dell_smbios_find_token'
   dell-laptop.c:(.init.text+0xa46): undefined reference to `dell_smbios_get_buffer'
   dell-laptop.c:(.init.text+0xa5b): undefined reference to `dell_smbios_send_request'
   dell-laptop.c:(.init.text+0xa83): undefined reference to `dell_smbios_release_buffer'
   drivers/platform/x86/dell-laptop.o: In function `dell_exit':
>> dell-laptop.c:(.exit.text+0xa): undefined reference to `dell_laptop_unregister_notifier'

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