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

Groups > linux.kernel > #1736670 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/12] Introduce support for Dell SMBIOS over WMI Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    [PATCH 08/12] platform/x86: wmi: Cleanup exit routine in reverse order of init Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    [PATCH 02/12] platform/x86: dell-wmi: Don't match on descriptor GUID modalias Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
      Re: [PATCH 02/12] platform/x86: dell-wmi: Don't match on descriptor  GUID modalias Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:10 +0200
    [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI interface Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
      Re: [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI  interface Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:30 +0200
        RE: [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI  interface <Mario.Limonciello@dell.com> - 2017-09-25 21:30 +0200
    [PATCH 12/12] platform/x86: Kconfig: Change the default settings for dell-wmi-smbios Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    [PATCH 01/12] platform/x86: dell-wmi: label driver as handling notifications Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
      Re: [PATCH 01/12] platform/x86: dell-wmi: label driver as handling  notifications Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:10 +0200
        RE: [PATCH 01/12] platform/x86: dell-wmi: label driver as handling  notifications <Mario.Limonciello@dell.com> - 2017-09-25 22:20 +0200
    [PATCH 10/12] platform/x86: wmi: destroy on cleanup rather than unregister Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
      Re: [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI  descriptor check Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-21 18:50 +0200
        RE: [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI  descriptor check <Mario.Limonciello@dell.com> - 2017-09-21 23:00 +0200
    [PATCH 09/12] platform/x86: wmi: create character devices when requested by drivers Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
      Re: [PATCH 09/12] platform/x86: wmi: create character devices when  requested by drivers Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-21 18:50 +0200
        RE: [PATCH 09/12] platform/x86: wmi: create character devices when  requested by drivers <Mario.Limonciello@dell.com> - 2017-09-21 21:30 +0200
    [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition to driver Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:10 +0200
      Re: [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition to driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-21 18:30 +0200
      Re: [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition  to driver Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:10 +0200
    [PATCH 05/12] platform/x86: dell-smbios: rename to dell-wmi-smbios Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:10 +0200
    Re: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:20 +0200
      RE: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI <Mario.Limonciello@dell.com> - 2017-09-25 18:40 +0200
        Re: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:50 +0200
          RE: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI <Mario.Limonciello@dell.com> - 2017-09-25 21:30 +0200

Page 1 of 2  [1] 2  Next page →

#1736670 — [PATCH 00/12] Introduce support for Dell SMBIOS over WMI

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 syfs interface for communicating with this ASL
method to allow userspace to use instead.

To faciliate that needs for userspace and kernel space this patch
series introduces a generic way for WMI drivers to be able to
create character devices through the WMI bus when desired.
Requiring WMI drivers to explictly ask for this functionality will
act as an effective vendor whitelist.

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

 Documentation/ABI/testing/dell-wmi-smbios          |  19 +
 .../ABI/testing/sysfs-platform-dell-wmi-smbios     |  16 +
 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             | 444 +++++++++++++++++++++
 .../x86/{dell-smbios.h => dell-wmi-smbios.h}       |  23 +-
 drivers/platform/x86/dell-wmi.c                    |  78 +---
 drivers/platform/x86/wmi.c                         | 104 ++++-
 include/linux/wmi.h                                |   1 +
 12 files changed, 610 insertions(+), 313 deletions(-)
 create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
 create mode 100644 Documentation/ABI/testing/sysfs-platform-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} (75%)

-- 
2.14.1

[toc] | [next] | [standalone]

#1736673 — [PATCH 08/12] platform/x86: wmi: Cleanup exit routine in reverse order of init

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 0765b1797d4c..077a9b7459ef 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

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

#1736674 — [PATCH 02/12] platform/x86: dell-wmi: Don't match on descriptor GUID modalias

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.

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

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

#1739139 — Re: [PATCH 02/12] platform/x86: dell-wmi: Don't match on descriptor GUID modalias

On Thursday 21 September 2017 08:57:07 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.

Ok, understood. What we need there is to load driver in case both
aliases are present in system. But I have no idea if kernel supports
such logic. So, add my:

Reviewed-by: Pali Rohár <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;

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

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

#1736675 — [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI interface

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

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.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/Kconfig       |  8 ++--
 drivers/platform/x86/dell-smbios.c | 76 ++++++++++++++++++++++++++------------
 drivers/platform/x86/dell-smbios.h | 11 +++---
 3 files changed, 63 insertions(+), 32 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 e9b1ca07c872..c06262a89169 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.
@@ -18,13 +19,12 @@
 #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 <linux/wmi.h>
 #include "dell-smbios.h"
 
+#define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
+
 struct calling_interface_structure {
 	struct dmi_header header;
 	u16 cmdIOAddress;
@@ -76,20 +76,39 @@ 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 calling_interface_buffer *buffer)
 {
-	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 calling_interface_buffer);
+	input.pointer = buffer;
+
+	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",
+			buffer->class, buffer->select, buffer->input[0],
+			buffer->input[1], buffer->input[2], buffer->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(buffer, 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;
+}
 
+void dell_smbios_send_request(int class, int select)
+{
 	buffer->class = class;
 	buffer->select = select;
-
-	dcdbas_smi_request(&command);
+	run_wmi_smbios_call(buffer);
 }
 EXPORT_SYMBOL_GPL(dell_smbios_send_request);
 
@@ -170,7 +189,7 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy)
 	}
 }
 
-static int __init dell_smbios_init(void)
+static int dell_smbios_probe(struct wmi_device *wdev)
 {
 	int ret;
 
@@ -181,11 +200,7 @@ static int __init dell_smbios_init(void)
 		return -ENODEV;
 	}
 
-	/*
-	 * 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);
+	buffer = (void *)__get_free_page(GFP_KERNEL);
 	if (!buffer) {
 		ret = -ENOMEM;
 		goto fail_buffer;
@@ -198,17 +213,32 @@ static int __init dell_smbios_init(void)
 	return ret;
 }
 
-static void __exit dell_smbios_exit(void)
+static int dell_smbios_remove(struct wmi_device *wdev)
 {
 	kfree(da_tokens);
 	free_page((unsigned long)buffer);
+	return 0;
 }
 
-subsys_initcall(dell_smbios_init);
-module_exit(dell_smbios_exit);
+static const struct wmi_device_id dell_smbios_id_table[] = {
+	{ .guid_string = DELL_WMI_SMBIOS_GUID },
+	{ },
+};
+
+static struct wmi_driver dell_smbios_driver = {
+	.driver = {
+		.name = "dell-smbios",
+	},
+	.probe = dell_smbios_probe,
+	.remove = dell_smbios_remove,
+	.id_table = dell_smbios_id_table,
+};
+module_wmi_driver(dell_smbios_driver);
+
 
 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_DESCRIPTION("Common functions for kernel modules using Dell SMBIOS");
+MODULE_AUTHOR("Mario Limonciello <mario.limonciello@dell.com>");
+MODULE_DESCRIPTION("Common functions for kernel modules using Dell SMBIOS over WMI");
 MODULE_LICENSE("GPL");
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 45cbc2292cd3..e1e29697b362 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,14 +19,14 @@
 
 struct notifier_block;
 
-/* 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];
+	u32 input[4];
+	u32 output[4];
+	u32 argattrib;
+	u32 blength;
+	u8 data[4052];
 } __packed;
 
 struct calling_interface_token {
-- 
2.14.1

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

#1739156 — Re: [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI interface

On Thursday 21 September 2017 08:57:09 Mario Limonciello wrote:
> The driver currently uses an SMI interface which grants direct access
> to physical memory to the platform via a pointer.
> 
> 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.

In my opinion direct access is safer then using ACPI wrapper for same
functionality.

Anyway, this change would break support for laptops without ACPI-WMI
functionality. IIRC I read in some Dell ACPI-WMI documentation that Dell
SMM via ACPI-WMI is not supported on all machines (probably older
machines) and it is needed to check some vendor bit in DMI data if Dell
SMM ACPI-WMI is really supported.

In linux kernel we do not want to remove support for older machines,
just because machines with new firmware can use also different new
communication method/protocol.

> As a result, this change removes the dependency on this driver on the
> dcdbas kernel module.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> ---
>  drivers/platform/x86/Kconfig       |  8 ++--
>  drivers/platform/x86/dell-smbios.c | 76 ++++++++++++++++++++++++++------------
>  drivers/platform/x86/dell-smbios.h | 11 +++---
>  3 files changed, 63 insertions(+), 32 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 e9b1ca07c872..c06262a89169 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.
> @@ -18,13 +19,12 @@
>  #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 <linux/wmi.h>
>  #include "dell-smbios.h"
>  
> +#define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
> +
>  struct calling_interface_structure {
>  	struct dmi_header header;
>  	u16 cmdIOAddress;
> @@ -76,20 +76,39 @@ 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 calling_interface_buffer *buffer)
>  {
> -	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 calling_interface_buffer);
> +	input.pointer = buffer;
> +
> +	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",
> +			buffer->class, buffer->select, buffer->input[0],
> +			buffer->input[1], buffer->input[2], buffer->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(buffer, 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;
> +}
>  
> +void dell_smbios_send_request(int class, int select)
> +{
>  	buffer->class = class;
>  	buffer->select = select;
> -
> -	dcdbas_smi_request(&command);
> +	run_wmi_smbios_call(buffer);
>  }
>  EXPORT_SYMBOL_GPL(dell_smbios_send_request);
>  
> @@ -170,7 +189,7 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy)
>  	}
>  }
>  
> -static int __init dell_smbios_init(void)
> +static int dell_smbios_probe(struct wmi_device *wdev)
>  {
>  	int ret;
>  
> @@ -181,11 +200,7 @@ static int __init dell_smbios_init(void)
>  		return -ENODEV;
>  	}
>  
> -	/*
> -	 * 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);
> +	buffer = (void *)__get_free_page(GFP_KERNEL);
>  	if (!buffer) {
>  		ret = -ENOMEM;
>  		goto fail_buffer;
> @@ -198,17 +213,32 @@ static int __init dell_smbios_init(void)
>  	return ret;
>  }
>  
> -static void __exit dell_smbios_exit(void)
> +static int dell_smbios_remove(struct wmi_device *wdev)
>  {
>  	kfree(da_tokens);
>  	free_page((unsigned long)buffer);
> +	return 0;
>  }
>  
> -subsys_initcall(dell_smbios_init);
> -module_exit(dell_smbios_exit);
> +static const struct wmi_device_id dell_smbios_id_table[] = {
> +	{ .guid_string = DELL_WMI_SMBIOS_GUID },
> +	{ },
> +};
> +
> +static struct wmi_driver dell_smbios_driver = {
> +	.driver = {
> +		.name = "dell-smbios",
> +	},
> +	.probe = dell_smbios_probe,
> +	.remove = dell_smbios_remove,
> +	.id_table = dell_smbios_id_table,
> +};
> +module_wmi_driver(dell_smbios_driver);
> +
>  
>  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_DESCRIPTION("Common functions for kernel modules using Dell SMBIOS");
> +MODULE_AUTHOR("Mario Limonciello <mario.limonciello@dell.com>");
> +MODULE_DESCRIPTION("Common functions for kernel modules using Dell SMBIOS over WMI");
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
> index 45cbc2292cd3..e1e29697b362 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,14 +19,14 @@
>  
>  struct notifier_block;
>  
> -/* 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];
> +	u32 input[4];
> +	u32 output[4];
> +	u32 argattrib;
> +	u32 blength;
> +	u8 data[4052];
>  } __packed;
>  
>  struct calling_interface_token {

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

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

#1739236 — RE: [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI interface

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Monday, September 25, 2017 12:19 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; LKML <linux-kernel@vger.kernel.org>; platform-driver-
> x86@vger.kernel.org; quasisec@google.com
> Subject: Re: [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI
> interface
> 
> On Thursday 21 September 2017 08:57:09 Mario Limonciello wrote:
> > The driver currently uses an SMI interface which grants direct access
> > to physical memory to the platform via a pointer.
> >
> > 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.
> 
> In my opinion direct access is safer then using ACPI wrapper for same
> functionality.

I'd like to hear how this is safer.

> 
> Anyway, this change would break support for laptops without ACPI-WMI
> functionality. IIRC I read in some Dell ACPI-WMI documentation that Dell
> SMM via ACPI-WMI is not supported on all machines (probably older
> machines) and it is needed to check some vendor bit in DMI data if Dell
> SMM ACPI-WMI is really supported.
> 
> In linux kernel we do not want to remove support for older machines,
> just because machines with new firmware can use also different new
> communication method/protocol.
> 

As mentioned on other email, I'll rework to support both methods and
prefer WMI method.

> > As a result, this change removes the dependency on this driver on the
> > dcdbas kernel module.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > ---
> >  drivers/platform/x86/Kconfig       |  8 ++--
> >  drivers/platform/x86/dell-smbios.c | 76 ++++++++++++++++++++++++++------------
> >  drivers/platform/x86/dell-smbios.h | 11 +++---
> >  3 files changed, 63 insertions(+), 32 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 e9b1ca07c872..c06262a89169 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.
> > @@ -18,13 +19,12 @@
> >  #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 <linux/wmi.h>
> >  #include "dell-smbios.h"
> >
> > +#define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-
> B622A1EF5492"
> > +
> >  struct calling_interface_structure {
> >  	struct dmi_header header;
> >  	u16 cmdIOAddress;
> > @@ -76,20 +76,39 @@ 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 calling_interface_buffer *buffer)
> >  {
> > -	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 calling_interface_buffer);
> > +	input.pointer = buffer;
> > +
> > +	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",
> > +			buffer->class, buffer->select, buffer->input[0],
> > +			buffer->input[1], buffer->input[2], buffer->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(buffer, 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;
> > +}
> >
> > +void dell_smbios_send_request(int class, int select)
> > +{
> >  	buffer->class = class;
> >  	buffer->select = select;
> > -
> > -	dcdbas_smi_request(&command);
> > +	run_wmi_smbios_call(buffer);
> >  }
> >  EXPORT_SYMBOL_GPL(dell_smbios_send_request);
> >
> > @@ -170,7 +189,7 @@ static void __init find_tokens(const struct dmi_header
> *dm, void *dummy)
> >  	}
> >  }
> >
> > -static int __init dell_smbios_init(void)
> > +static int dell_smbios_probe(struct wmi_device *wdev)
> >  {
> >  	int ret;
> >
> > @@ -181,11 +200,7 @@ static int __init dell_smbios_init(void)
> >  		return -ENODEV;
> >  	}
> >
> > -	/*
> > -	 * 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);
> > +	buffer = (void *)__get_free_page(GFP_KERNEL);
> >  	if (!buffer) {
> >  		ret = -ENOMEM;
> >  		goto fail_buffer;
> > @@ -198,17 +213,32 @@ static int __init dell_smbios_init(void)
> >  	return ret;
> >  }
> >
> > -static void __exit dell_smbios_exit(void)
> > +static int dell_smbios_remove(struct wmi_device *wdev)
> >  {
> >  	kfree(da_tokens);
> >  	free_page((unsigned long)buffer);
> > +	return 0;
> >  }
> >
> > -subsys_initcall(dell_smbios_init);
> > -module_exit(dell_smbios_exit);
> > +static const struct wmi_device_id dell_smbios_id_table[] = {
> > +	{ .guid_string = DELL_WMI_SMBIOS_GUID },
> > +	{ },
> > +};
> > +
> > +static struct wmi_driver dell_smbios_driver = {
> > +	.driver = {
> > +		.name = "dell-smbios",
> > +	},
> > +	.probe = dell_smbios_probe,
> > +	.remove = dell_smbios_remove,
> > +	.id_table = dell_smbios_id_table,
> > +};
> > +module_wmi_driver(dell_smbios_driver);
> > +
> >
> >  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_DESCRIPTION("Common functions for kernel modules using Dell
> SMBIOS");
> > +MODULE_AUTHOR("Mario Limonciello <mario.limonciello@dell.com>");
> > +MODULE_DESCRIPTION("Common functions for kernel modules using Dell
> SMBIOS over WMI");
> >  MODULE_LICENSE("GPL");
> > diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-
> smbios.h
> > index 45cbc2292cd3..e1e29697b362 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,14 +19,14 @@
> >
> >  struct notifier_block;
> >
> > -/* 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];
> > +	u32 input[4];
> > +	u32 output[4];
> > +	u32 argattrib;
> > +	u32 blength;
> > +	u8 data[4052];
> >  } __packed;
> >
> >  struct calling_interface_token {
> 
> --
> Pali Rohár
> pali.rohar@gmail.com

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

#1736677 — [PATCH 12/12] platform/x86: Kconfig: Change the default settings for dell-wmi-smbios

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

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

#1736678 — [PATCH 01/12] platform/x86: dell-wmi: label driver as handling notifications

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

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

#1739138 — Re: [PATCH 01/12] platform/x86: dell-wmi: label driver as handling notifications

On Thursday 21 September 2017 08:57:06 Mario Limonciello wrote:
> 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.

Hi! I'm not sure if if "notifications" word is better then "extras".
Basically the most important part of the dell-wmi driver is to deliver
key press events via input device.

Has anybody else better word or description for this?

> 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

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

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

#1739263 — RE: [PATCH 01/12] platform/x86: dell-wmi: label driver as handling notifications

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Monday, September 25, 2017 12:04 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; LKML <linux-kernel@vger.kernel.org>; platform-driver-
> x86@vger.kernel.org; quasisec@google.com
> Subject: Re: [PATCH 01/12] platform/x86: dell-wmi: label driver as handling
> notifications
> 
> On Thursday 21 September 2017 08:57:06 Mario Limonciello wrote:
> > 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.
> 
> Hi! I'm not sure if if "notifications" word is better then "extras".
> Basically the most important part of the dell-wmi driver is to deliver
> key press events via input device.
> 
> Has anybody else better word or description for this?
> 
I was actually tempted to rename the driver itself to dell-wmi-notifications.
Realistically it is hooking up to the notifications _WED0 AML method so yes
it is picking up notifications exclusively.

I thought about this too, but I can also envision that the notifications that
come through this driver that aren't consumed by the kernel for keypress
purposes are also useful to user space potentially.  It's not part of this series
but maybe in the future providing those through a character device too may
make sense.

> > 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
> 
> --
> Pali Rohár
> pali.rohar@gmail.com

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

#1736679 — [PATCH 10/12] platform/x86: wmi: destroy on cleanup rather than unregister

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 d1e128864d24..84314c0ef9c2 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -1223,7 +1223,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;
 }
@@ -1277,7 +1277,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

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

#1736680 — [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check

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 | 78 ++++++++++++++++++++++++++++++++++
 drivers/platform/x86/dell-wmi-smbios.h |  3 ++
 drivers/platform/x86/dell-wmi.c        | 75 +-------------------------------
 3 files changed, 82 insertions(+), 74 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi-smbios.c b/drivers/platform/x86/dell-wmi-smbios.c
index c3701fdadf7b..9deb851ff517 100644
--- a/drivers/platform/x86/dell-wmi-smbios.c
+++ b/drivers/platform/x86/dell-wmi-smbios.c
@@ -24,6 +24,7 @@
 #include "dell-wmi-smbios.h"
 
 #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;
@@ -217,8 +218,81 @@ static const struct attribute_group smbios_attribute_group = {
 	.attrs = smbios_attrs,
 };
 
+/*
+ * 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_wmi_smbios_probe(struct wmi_device *wdev)
 {
+	u32 interface_version;
 	int ret;
 
 	dmi_walk(find_tokens, NULL);
@@ -228,6 +302,10 @@ static int dell_wmi_smbios_probe(struct wmi_device *wdev)
 		return -ENODEV;
 	}
 
+	ret = dell_wmi_check_descriptor_buffer(wdev, &interface_version);
+	if (ret)
+		return ret;
+
 	buffer = (void *)__get_free_page(GFP_KERNEL);
 	if (!buffer) {
 		ret = -ENOMEM;
diff --git a/drivers/platform/x86/dell-wmi-smbios.h b/drivers/platform/x86/dell-wmi-smbios.h
index e6e9990bb2b7..0521ec5d437b 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;
 
 struct calling_interface_buffer {
@@ -54,5 +56,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

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

#1736844 — Re: [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check

On Thu, Sep 21, 2017 at 4:57 PM, Mario Limonciello
<mario.limonciello@dell.com> wrote:
> 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.

> +       desc_buffer = (u32 *)obj->buffer.pointer;
> +

> +       if (desc_buffer[0] != 0x4C4C4544 && desc_buffer[1] != 0x494D5720)

I was thinking about strncmp() here for a full line, though decide not
to push it anyhow. Those IDs is binary data, can be anything and you
have comment of what is expected here.

But I think it would be nice to create a separate definitions and make
comments there.

> +               dev_warn(&wdev->dev, "Dell descriptor buffer has invalid signature (%*ph)\n",
> +                       8, desc_buffer);

%8ph ?

> +
> +       if (desc_buffer[2] != 0 && desc_buffer[2] != 1)
> +               dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%d)\n",
> +                       desc_buffer[2]);

%u ? u32 can't be negative and you basically allow it.

> +
> +       if (desc_buffer[3] != 4096)
> +               dev_warn(&wdev->dev, "Dell descriptor buffer has invalid buffer length (%d)\n",
> +                       desc_buffer[3]);

Ditto.


P.S. I noticed this all in old code, so, you can address my comments
in a separate patch if you find them useful.

-- 
With Best Regards,
Andy Shevchenko

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

#1737057 — RE: [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Thursday, September 21, 2017 11:44 AM
> 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 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI
> descriptor check
> 
> On Thu, Sep 21, 2017 at 4:57 PM, Mario Limonciello
> <mario.limonciello@dell.com> wrote:
> > 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.
> 
> > +       desc_buffer = (u32 *)obj->buffer.pointer;
> > +
> 
> > +       if (desc_buffer[0] != 0x4C4C4544 && desc_buffer[1] != 0x494D5720)
> 
> I was thinking about strncmp() here for a full line, though decide not
> to push it anyhow. Those IDs is binary data, can be anything and you
> have comment of what is expected here.
> 
> But I think it would be nice to create a separate definitions and make
> comments there.
> 
> > +               dev_warn(&wdev->dev, "Dell descriptor buffer has invalid signature
> (%*ph)\n",
> > +                       8, desc_buffer);
> 
> %8ph ?
> 
> > +
> > +       if (desc_buffer[2] != 0 && desc_buffer[2] != 1)
> > +               dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version
> (%d)\n",
> > +                       desc_buffer[2]);
> 
> %u ? u32 can't be negative and you basically allow it.
> 
> > +
> > +       if (desc_buffer[3] != 4096)
> > +               dev_warn(&wdev->dev, "Dell descriptor buffer has invalid buffer length
> (%d)\n",
> > +                       desc_buffer[3]);
> 
> Ditto.
> 
> 
> P.S. I noticed this all in old code, so, you can address my comments
> in a separate patch if you find them useful.
> 

Yeah this is all old code.  I've made some adjustments to it in v2 in a completely separate
patch that comes after the patch it's "moved".  I'll submit back after other feedback
to this series.

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

#1736685 — [PATCH 09/12] platform/x86: wmi: create character devices when requested by drivers

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 077a9b7459ef..d1e128864d24 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -44,12 +44,17 @@
 #include <linux/platform_device.h>
 #include <linux/wmi.h>
 #include <linux/uuid.h>
+#include <linux/cdev.h>
+#include <linux/idr.h>
 
 ACPI_MODULE_NAME("wmi");
 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

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

#1736840 — Re: [PATCH 09/12] platform/x86: wmi: create character devices when requested by drivers

On Thu, Sep 21, 2017 at 4:57 PM, Mario Limonciello
<mario.limonciello@dell.com> wrote:
> 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.
>

> @@ -44,12 +44,17 @@
>  #include <linux/platform_device.h>
>  #include <linux/wmi.h>
>  #include <linux/uuid.h>
> +#include <linux/cdev.h>
> +#include <linux/idr.h>

Keep alphabetical ordering.


-- 
With Best Regards,
Andy Shevchenko

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

#1737027 — RE: [PATCH 09/12] platform/x86: wmi: create character devices when requested by drivers

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Thursday, September 21, 2017 11:47 AM
> 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 09/12] platform/x86: wmi: create character devices when
> requested by drivers
> 
> On Thu, Sep 21, 2017 at 4:57 PM, Mario Limonciello
> <mario.limonciello@dell.com> wrote:
> > 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.
> >
> 
> > @@ -44,12 +44,17 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/wmi.h>
> >  #include <linux/uuid.h>
> > +#include <linux/cdev.h>
> > +#include <linux/idr.h>
> 
> Keep alphabetical ordering.
> 

The existing list wasn't in alphabetical order.  I'll submit a patch in v2 earlier
in the series to sort and then put them in the right place for my patch.

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

#1736691 — [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition to driver

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 0a5723468bff..e9b1ca07c872 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

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

#1736835 — Re: [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition to driver

On Thu, Sep 21, 2017 at 4:57 PM, Mario Limonciello
<mario.limonciello@dell.com> wrote:

We need a (formal) commit message even for simple patches.

> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>

-- 
With Best Regards,
Andy Shevchenko

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web