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

Groups > linux.kernel > #1740118 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 06/14] platform/x86: wmi: Cleanup exit routine in reverse order of init Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios Pali Rohár <pali.rohar@gmail.com> - 2017-09-26 22:10 +0200
        RE: [PATCH v2 09/14] platform/x86: dell-smbios: rename to  dell-wmi-smbios <Mario.Limonciello@dell.com> - 2017-09-26 22:20 +0200
          Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to  dell-wmi-smbios Darren Hart <dvhart@infradead.org> - 2017-09-28 00:40 +0200
      Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to  dell-wmi-smbios kbuild test robot <lkp@intel.com> - 2017-09-29 17:40 +0200
    [PATCH v2 07/14] platform/x86: wmi: destroy on cleanup rather than unregister Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character device for userspace Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce  character device for userspace Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-26 21:10 +0200
      RE: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce  character device for userspace <Mario.Limonciello@dell.com> - 2017-09-26 22:20 +0200
      Re: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character device for userspace Pali Rohár <pali.rohar@gmail.com> - 2017-09-26 22:20 +0200
    [PATCH v2 10/14] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 08/14] platform/x86: dell-smbios: Introduce a WMI-ACPI interface Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 08/14] platform/x86: dell-smbios: Introduce a WMI-ACPI  interface Darren Hart <dvhart@infradead.org> - 2017-09-28 00:20 +0200
    [PATCH v2 11/14] platform/x86: wmi: create character devices when requested by drivers Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 01/14] platform/x86: dell-wmi: label driver as handling notifications Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 13/14] platform/x86: Kconfig: Change the default settings for dell-wmi-smbios Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor GUID modalias Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on  descriptor GUID modalias Darren Hart <dvhart@infradead.org> - 2017-09-27 19:30 +0200
        RE: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on  descriptor GUID modalias <Mario.Limonciello@dell.com> - 2017-09-27 19:40 +0200
          Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on  descriptor GUID modalias Darren Hart <dvhart@infradead.org> - 2017-09-27 19:50 +0200
    [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check Pali Rohár <pali.rohar@gmail.com> - 2017-09-26 22:20 +0200
        RE: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi  descriptor check <Mario.Limonciello@dell.com> - 2017-09-26 22:30 +0200
    [PATCH v2 04/14] platform/x86: dell-smbios: Add pr_fmt definition to driver Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 05/14] platform/x86: wmi: sort include list Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless  includes Darren Hart <dvhart@infradead.org> - 2017-09-27 19:50 +0200
        RE: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless  includes <Mario.Limonciello@dell.com> - 2017-09-27 19:50 +0200
        Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless  includes Darren Hart <dvhart@infradead.org> - 2017-09-27 20:00 +0200
    Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-26 21:10 +0200
      RE: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI <Mario.Limonciello@dell.com> - 2017-09-26 21:20 +0200
        Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI Darren Hart <dvhart@infradead.org> - 2017-09-27 19:20 +0200
          RE: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI <Mario.Limonciello@dell.com> - 2017-09-27 19:40 +0200
            Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI Darren Hart <dvhart@infradead.org> - 2017-09-27 19:40 +0200

Page 1 of 2  [1] 2  Next page →

#1740118 — [PATCH v2 00/14] 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 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

[toc] | [next] | [standalone]

#1740119 — [PATCH v2 06/14] 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 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

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

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

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

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

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

[Multipart message — attachments visible in raw view] — view raw

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

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

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

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

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

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

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

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

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

[Multipart message — attachments visible in raw view] — view raw

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

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

#1740121 — [PATCH v2 07/14] 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 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

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

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

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

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

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

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

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

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

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

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

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

[Multipart message — attachments visible in raw view] — view raw

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

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

#1740123 — [PATCH v2 10/14] 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 | 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

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

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

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

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

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

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

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

#1740125 — [PATCH v2 11/14] 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 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

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

#1740127 — [PATCH v2 01/14] 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]

#1740128 — [PATCH v2 13/14] 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]

#1740130 — [PATCH v2 03/14] 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.

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

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

#1740905 — 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.

-- 
Darren Hart
VMware Open Source Technology Center

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web