All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
@ 2017-11-01 19:25 Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 01/16] platform/x86: dell-smbios: Prefix class/select with cmd_ Mario Limonciello
                   ` (16 more replies)
  0 siblings, 17 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

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

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

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

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

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

To faciliate that this patch series introduces a generic way for WMI
drivers to be able to create discoverable character devices with
a predictable IOCTL interface 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.

Some of this work is the basis for what will be a proper interpreter
of MOF in the kernel and controls for what drivers will be able to
do with that MOF.

NOTE: This patch series is intended to go on top of platform-drivers-x86
linux-next.

For convenience the entire series including those is also available here:
https://github.com/dell/linux/tree/wmi-smbios

changes bewteen v11 and v12:
 * Fix including uapi header in c++ applications due to reserved
   keyword class
   - Introduce a new patch earlier for changing old code
   - s/class/cmd_class; s/select/cmd_select in all new patches
 * smbios tokens patch (patch 9/16) 
   - Include an optimization recommended by Pali for zero_duplicates.
changes between v10 and v11:
 * Add reviewed by from Pali to applicable patches
 * Patch 5/15 (WMI split)
   - Add mutex locking of the list in one more place
 * Patch 11/15 (WSMT test)
   - Don't set boolean function to integer return
   - Check for an invalid return output from WSMT query as
     the token query is going to fail too when WSMT enabled.
 * Patch 13/15 (WMI userspace interface)
   - Remove #ifdef for .compat_ioctl
   - Use simple_read_from_buffer for wmi_char_read
   - Fix some bad logic around container_of
   - Remove unneeded instance argument for set_required_buffer_size
 * Patch 14/15 (dell character driver creation)
   - Remove generally unnecessary debugging code
   - Update set_required_buffer_size call
changes between v9 and v10:
 * Add reviewed by from Pali & Edward to applicable patches
 * Patch 3/7 (cleanup descriptor check)
   - Cast u32 to unsigned long when displaying error
 * Patch 4/7 (allow 32k return size)
   - Adjust not to show warnings when return size is unexpected
 * Patch 5/7 (split dell-wmi in two)
   - Use wmi_has_guid to make sure DELL_WMI_DESCRIPTOR_GUID is
     on the WMI bus during probing.
     Prevents situations that firmware doesn't have descriptor
     GUID available.
   - Use a mutex to prevent race conditions.
 * Patch 10/17 (Add new WMI dispatcher)
   - Grab mutex earlier to avoid race condition
   - Use wmi_has_guid to make sure DELL_WMI_DESCRIPTOR_GUID is
     on the WMI bus during probing.
     Prevents situations that firmware doesn't have descriptor
     GUID available.
 * Patch 11/17 (WSMT patch):
   - Return boolean instead of int
 * Patch 13/17 (required_buffer_size patch)
   - Merge with patch 14/17
 * Patch 14/17 (WMI character device patch)
   - Don't offer required_buffer_size over sysfs. Offer the
     data provided when read() from character device instead.
   - Switch from offering an ioctl callback method to a
     filter callback method.  The copying to/from userspace
     is now handled by the bus driver and generic for all
     vendor drivers.  This simplifies the amount of code
     needed in a vendor driver callback.
   - Allocate memory for the ioctl call when creating
     character device
   - Add locking to the ioctl function
   - Remove existing Reviewed by as this has changed 
     significantly
 * Patch 15/17 (Offer dell character device patch)
   - Update documentation for changes in 14/17
   - Update callback method for changes in 14/17
 * Patch 16/17 (shuffle header content)
   - Merge into patch 15/17
 * Patch 17/17 (sample tool)
   - Use the new read() interface from 14/17
changes between v8 and v9:
 * Add Reviewed by from Edward to relevant patches
 * Add a new patch that introduces an example tool for using
   character device API.
   - Recommended by Pali
 * wmi bus character patch
   - Add a missing #ifdef CONFIG_COMPAT caught by kbuild bot
 * dell smbios tokens patch:
   - Add missing #include <linux/compatability.h> caught by kbuild bot.
 * dell smbios smm patch:
   - Remove unnecessary semicolons caught by kbuild bot.
   - Add missing header caught by kbuild bot.
changes between v7 and v8:
 * fix typo s/desc_buffer/buffer/ in 32k split commit
 * Add missing include for <linux/types.h> in uapi
 * dell-smbios: initialize correct attribute files
 * Output class/select in debug messages in base 10.
 * Don't send the u64 length argument to ACPI call length
 * Only filter calls that originate from userspace
 * Filter kernel calls from userspace
 * Add more blacklisting/whitelisting logic per Alan Cox's 
   recommendations
 * Adjust tokens attributes to only be accessible to CAP_SYS_ADMIN
 * Set default permissions on character device.
 * Move token, class, select definitions for items used in drivers
   to dells-smios.h.
changes between v6 and v7:
 * Use deferred probing in any function results needed from 
   dell-wmi-descriptor
 * Protect against a list entry disappearing when running an ioctl from
   WMI bus
 * Move ioctl uapi declaration into a common header file for all WMI
   drivers.
 * New patch: create required_buffer_size for method type WMI objects 
   in WMI core rather than vendor driver.
 * WSMT patch: Add comment explaining WSMT
 * Filtering patch: Add comments explaining what I can about blacklists.
 * WMI, dell-smbios-wmi: Add back in compat ioctl, it is needed.
   My previous test was erroneous in forgetting to copy an updated header
   into the chroot.
 * dell-laptop, dell-wmi: allocate less memory for buffer, page no longer
   needed
 * dell-laptop make the changes (hopefully) more amenable to pali style
   wise.
 * read SMM cmd address in dell-smbios-smm rather than dell-smbios
 * shuffle structure definitions for this
changes between v5 and v6:
 * Adjust Kconfig for dell-smbios to not depend on anything.
   - SMM and WMI drivers will both select dell-smbios
   - Technically the module can run on it's own (it's just not useful)
 * Add a new tokens sysfs interface
 * Rework blacklisting into easily expandable structures
 * Lock modules during ioctl call from WMI bus
 * drop references to compat ioctl in both WMI and dell-smbios-wmi
   drivers. (Made sure ioctl worked in both 32 and 64 userspace though)
 * dell-smbios-wmi 
   - s/buffer_size/req_buf_size/ to make it clearer that userspace
     doesn't get to set this, it's set internally.
   - read just buffer length before reading whole structure from 
     userspace
   - if buffer length is too big, show a warning
   - I tried to rename argument for unlocked_ioctl, but this caused
     problems in matching initialization lists, so still casting.
   - Add comments clarifying everything happening in ioctl
changes between v4 and v5:
 * Remove Andy's S suggested by in sysfs tokens patch
 * Make some output in dell-wmi-descriptor debug only
 * Adjust various Kconfig dependencies as recommended by Darren
 * Drop patch to set dell-smbios to default on ACPI_WMI,
   it's not needed after the Kconfig dependencies rework
 * Move WSMT check patch to after WMI driver is introduced.
 * Make common smbios call return value int as there could be
   errors now with drivers not being loaded.
 * Make SMBIOS call methods for all drivers return status
 * Reorder patches 2 and 4.
 * Don't export symbols for calling functions on dispatchers
 * wmi patch:
   - use sprintf instead of strcpy
   - remove needless bool for tracking found
   - adjust logic to look for instance_count - 1, it's zero
     based not 1 based.
   - Pass a callback to unlocked_ioctl instead of full file
     operations object
   - ioctl: Don't fail on no bound WMI driver
   - Add missing header for uapi
   - Make helper macros include data types
   - add compat ioctl
 * dell-smbios:
   - Add filtering functionality for SMBIOS calling interface
   - Use dev_dbg rather than pr_debug where possible
 * dell-smbios-wmi:
   - test for handle on b1 table
   - correct misc flags comment
   - drop access checks
   - use dev_dbg instead of pr_* calls
   - use filtering functionality
   - add mutexes around list add/remove
   - switch from get_first_device to get_first_priv and inline
   - add mutex locking to prevent unloading mid-call.
   - update to new ioctl passing
   - fix userspace uapi to use __u32 instead of u32
   - Don't use a header file for internal only use
   - make sure it works with compat ioctl
 * dell-laptop: make dell_smbios_send_request local function
   for boilerplate calls.
 * ioctl patch
   - Change API to have a simpler structure to pass back and
     forth
   - Rename header file
   - Export to sysfs properly
   - Add the size of the length variable into the requested
     buffersize to sysfs, do math in the driver when copying
     data around.
changes between v3 and v4:
 * Make Dell WMI notifications driver fail notifications fail
   when WMI descriptor driver is unavailable.
 * Add a patch to check for Dell OEM string to stop dell-smbios 
   module from being loaded in unsupported systems manually.
 * Split Dell WMI descriptor into it's own driver for others to
   query.
 * Test the misc BIOS flags table to decide whether to run in WMI
   or SMI mode.
 * s/dell-wmi-smbios/dell-smbios/ in a few patch titles
 * Add missing Suggested-by to a patch from v2 (Sorry Andy S!)
 * Adjust cleanup order of wmi character device.
 * Fix a remaining reference to /dev/wmi-$driver
 * Use get_order to get size for pages
 * Split up dell-smbios into 3 drivers:
   dell-smbios
   -> dell-smbios-smm
   -> dell-smbios-wmi
   If either of the two dispatcher drivers is unloaded but
   the other still works, gracefully fall back to that driver
 * Remove unneded open and release on file operations in WMI
   driver
 * Switch to misc character device in WMI bus.
 * Query the size of the calling interface buffer from WMI
   descriptor driver.
 * Require userspace to use this information to build an
   appropriately sized buffer.
 * Verify the size of buffer passed from userspace matches
   expectation of buffer processed in kernel.
 * Add more documentation about the IOCTL API.
 * Add in a recommendation from Andy S about outputing tokens
 * Add a patch to test for non-functional SMM due to WSMT
 * Define macros for IOCTL's through the WMI bus.
 * Pass all IOCTL calls for WMI drivers through WMI bus
   WMI bus will guarantee instance count matches and prevent
   creating too many more IOCTLs.
changes between v2 and v3:
 * Drop patches 1-7, they're in Darren's review tree now
 * Add missing newline on new Documentation
 * Add Reviewed by from Edward O'Callaghan
 * Adjust path of character device from /dev/wmi-$driver to
   /dev/wmi/$driver
 * Store wmi_device pointer rather than using a boolean has_wmi
   to indicate driver is running in WMI mode
 * Don't guard free_page from freeing NULL (this is OK)
 * New patch: add wmidev_evaluate_method to wmi bus as recommended
   by Andy L
 * Adjust ACPI-WMI interface for this patch change ^
 * Add back in sysfs token patch, drop 2nd and 3rd ioctls per discussion
   on mailing list.
 * On sysfs interface: adjust the delimiter to be tabs
 * Drop the rename dell-smbios -> dell-wmi-smbios patch
 * Remove/move some unnecessary tests for CONFIG_DCDBAS
 * Reword s/platform/SMM/ in the WMI-ACPI patch.
 * Update Kconfig to recommend using CONFIG_DCDBAS on old machines.
 * Allocate buffer to the same pointer regardless of the struct 
   assigned to it.  Keep track of the buffer size for cleaning up.
 * Explain policy around character device creation in commit message
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 (16):
  platform/x86: dell-smbios: Prefix class/select with cmd_
  platform/x86: wmi: Add new method wmidev_evaluate_method
  platform/x86: dell-wmi: increase severity of some failures
  platform/x86: dell-wmi: clean up wmi descriptor check
  platform/x86: dell-wmi: don't check length returned
  platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own
    driver
  platform/x86: wmi: Don't allow drivers to get each other's GUIDs
  platform/x86: dell-smbios: only run if proper oem string is detected
  platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens
  platform/x86: dell-smbios: Introduce dispatcher for SMM calls
  platform/x86: dell-smbios-wmi: Add new WMI dispatcher driver
  platform/x86: dell-smbios-smm: test for WSMT
  platform/x86: dell-smbios: Add filtering support
  platform/x86: wmi: create userspace interface for drivers
  platform/x86: dell-smbios-wmi: introduce userspace interface
  tools/wmi: add a sample for dell smbios communication over WMI

 Documentation/ABI/testing/dell-smbios-wmi          |  41 ++
 .../ABI/testing/sysfs-platform-dell-smbios         |  21 +
 MAINTAINERS                                        |  26 ++
 drivers/platform/x86/Kconfig                       |  34 +-
 drivers/platform/x86/Makefile                      |   3 +
 drivers/platform/x86/dell-laptop.c                 | 283 +++++------
 drivers/platform/x86/dell-smbios-smm.c             | 196 ++++++++
 drivers/platform/x86/dell-smbios-wmi.c             | 268 +++++++++++
 drivers/platform/x86/dell-smbios.c                 | 515 +++++++++++++++++++--
 drivers/platform/x86/dell-smbios.h                 |  49 +-
 drivers/platform/x86/dell-wmi-descriptor.c         | 170 +++++++
 drivers/platform/x86/dell-wmi-descriptor.h         |  21 +
 drivers/platform/x86/dell-wmi.c                    |  94 +---
 drivers/platform/x86/wmi.c                         | 234 +++++++++-
 include/linux/wmi.h                                |  13 +-
 include/uapi/linux/wmi.h                           |  73 +++
 tools/Makefile                                     |  14 +-
 tools/wmi/Makefile                                 |  18 +
 tools/wmi/dell-smbios-example.c                    | 210 +++++++++
 19 files changed, 1925 insertions(+), 358 deletions(-)
 create mode 100644 Documentation/ABI/testing/dell-smbios-wmi
 create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-smbios
 create mode 100644 drivers/platform/x86/dell-smbios-smm.c
 create mode 100644 drivers/platform/x86/dell-smbios-wmi.c
 create mode 100644 drivers/platform/x86/dell-wmi-descriptor.c
 create mode 100644 drivers/platform/x86/dell-wmi-descriptor.h
 create mode 100644 include/uapi/linux/wmi.h
 create mode 100644 tools/wmi/Makefile
 create mode 100644 tools/wmi/dell-smbios-example.c

-- 
2.14.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v12 01/16] platform/x86: dell-smbios: Prefix class/select with cmd_
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-02  0:30   ` Edward O'Callaghan
  2017-11-01 19:25 ` [PATCH v12 02/16] platform/x86: wmi: Add new method wmidev_evaluate_method Mario Limonciello
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

Later on these structures will be brought up to userspace.
the word "class" is a reserved word in c++ and this will prevent
uapi headers from being included directly in c++ programs.

To make life easier on these applications, prepare the change now.

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

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index e9b1ca07c872..ffc174638aa4 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -86,8 +86,8 @@ void dell_smbios_send_request(int class, int select)
 	command.ebx = virt_to_phys(buffer);
 	command.ecx = 0x42534931;
 
-	buffer->class = class;
-	buffer->select = select;
+	buffer->cmd_class = class;
+	buffer->cmd_select = select;
 
 	dcdbas_smi_request(&command);
 }
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 45cbc2292cd3..742dd8bd66b9 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -22,8 +22,8 @@ struct notifier_block;
  * system management mode, hence the volatiles */
 
 struct calling_interface_buffer {
-	u16 class;
-	u16 select;
+	u16 cmd_class;
+	u16 cmd_select;
 	volatile u32 input[4];
 	volatile u32 output[4];
 } __packed;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 02/16] platform/x86: wmi: Add new method wmidev_evaluate_method
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 01/16] platform/x86: dell-smbios: Prefix class/select with cmd_ Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 03/16] platform/x86: dell-wmi: increase severity of some failures Mario Limonciello
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

Drivers properly using the wmibus can pass their wmi_device
pointer rather than the GUID back to the WMI bus to evaluate
the proper methods.

Any "new" drivers added that use the WMI bus should use this
rather than the old wmi_evaluate_method that would take the
GUID.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/wmi.c | 28 ++++++++++++++++++++++++----
 include/linux/wmi.h        |  6 ++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 7a05843aff19..4d73a87c2ddf 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -200,6 +200,28 @@ static acpi_status wmi_method_enable(struct wmi_block *wblock, int enable)
  */
 acpi_status wmi_evaluate_method(const char *guid_string, u8 instance,
 u32 method_id, const struct acpi_buffer *in, struct acpi_buffer *out)
+{
+	struct wmi_block *wblock = NULL;
+
+	if (!find_guid(guid_string, &wblock))
+		return AE_ERROR;
+	return wmidev_evaluate_method(&wblock->dev, instance, method_id,
+				      in, out);
+}
+EXPORT_SYMBOL_GPL(wmi_evaluate_method);
+
+/**
+ * wmidev_evaluate_method - Evaluate a WMI method
+ * @wdev: A wmi bus device from a driver
+ * @instance: Instance index
+ * @method_id: Method ID to call
+ * &in: Buffer containing input for the method call
+ * &out: Empty buffer to return the method results
+ *
+ * Call an ACPI-WMI method
+ */
+acpi_status wmidev_evaluate_method(struct wmi_device *wdev, u8 instance,
+	u32 method_id, const struct acpi_buffer *in, struct acpi_buffer *out)
 {
 	struct guid_block *block = NULL;
 	struct wmi_block *wblock = NULL;
@@ -209,9 +231,7 @@ u32 method_id, const struct acpi_buffer *in, struct acpi_buffer *out)
 	union acpi_object params[3];
 	char method[5] = "WM";
 
-	if (!find_guid(guid_string, &wblock))
-		return AE_ERROR;
-
+	wblock = container_of(wdev, struct wmi_block, dev);
 	block = &wblock->gblock;
 	handle = wblock->acpi_device->handle;
 
@@ -246,7 +266,7 @@ u32 method_id, const struct acpi_buffer *in, struct acpi_buffer *out)
 
 	return status;
 }
-EXPORT_SYMBOL_GPL(wmi_evaluate_method);
+EXPORT_SYMBOL_GPL(wmidev_evaluate_method);
 
 static acpi_status __query_block(struct wmi_block *wblock, u8 instance,
 				 struct acpi_buffer *out)
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index cd0d7734dc49..2cd10c3b89e9 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -26,6 +26,12 @@ struct wmi_device {
 	bool setable;
 };
 
+/* evaluate the ACPI method associated with this device */
+extern acpi_status wmidev_evaluate_method(struct wmi_device *wdev,
+					  u8 instance, u32 method_id,
+					  const struct acpi_buffer *in,
+					  struct acpi_buffer *out);
+
 /* Caller must kfree the result. */
 extern union acpi_object *wmidev_block_query(struct wmi_device *wdev,
 					     u8 instance);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 03/16] platform/x86: dell-wmi: increase severity of some failures
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 01/16] platform/x86: dell-smbios: Prefix class/select with cmd_ Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 02/16] platform/x86: wmi: Add new method wmidev_evaluate_method Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 04/16] platform/x86: dell-wmi: clean up wmi descriptor check Mario Limonciello
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

There is a lot of error checking in place for the format of the WMI
descriptor buffer, but some of the potentially raised issues should
be considered critical failures.

If the buffer size or header don't match, this is a good indication
that the buffer format changed in a way that the rest of the data
should not be relied upon.

For the remaining data set vectors, continue to notate a warning
in undefined results, but as those are fields that the descriptor
intended to refer to other applications, don't fail if they're new
values.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-wmi.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 1fbef560ca67..2cfaaa8faf0a 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -657,17 +657,18 @@ static int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev)
 		dev_err(&wdev->dev,
 			"Dell descriptor buffer has invalid length (%d)\n",
 			obj->buffer.length);
-		if (obj->buffer.length < 16) {
-			ret = -EINVAL;
-			goto out;
-		}
+		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",
+	if (buffer[0] != 0x4C4C4544 && buffer[1] != 0x494D5720) {
+		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%*ph)\n",
 			8, buffer);
+		ret = -EINVAL;
+		goto out;
+	}
 
 	if (buffer[2] != 0 && buffer[2] != 1)
 		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%d)\n",
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 04/16] platform/x86: dell-wmi: clean up wmi descriptor check
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (2 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 03/16] platform/x86: dell-wmi: increase severity of some failures Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 05/16] platform/x86: dell-wmi: don't check length returned Mario Limonciello
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

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

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-wmi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 2cfaaa8faf0a..b2bd396acac5 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -663,16 +663,16 @@ static int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev)
 
 	buffer = (u32 *)obj->buffer.pointer;
 
-	if (buffer[0] != 0x4C4C4544 && buffer[1] != 0x494D5720) {
-		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%*ph)\n",
-			8, buffer);
+	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
+		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
+			buffer);
 		ret = -EINVAL;
 		goto out;
 	}
 
 	if (buffer[2] != 0 && buffer[2] != 1)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%d)\n",
-			buffer[2]);
+		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
+			(unsigned long) buffer[2]);
 
 	if (buffer[3] != 4096)
 		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid buffer length (%d)\n",
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 05/16] platform/x86: dell-wmi: don't check length returned
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (3 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 04/16] platform/x86: dell-wmi: clean up wmi descriptor check Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 06/16] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver Mario Limonciello
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

This is intended to be variable and provided by the platform.
Some platforms this year will be adopting a 32k WMI buffer, so don't
complain when encountering those platforms or any other future changes.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-wmi.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index b2bd396acac5..da4f629d0831 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -624,7 +624,7 @@ static void dell_wmi_input_destroy(struct wmi_device *wdev)
  * Vendor Signature          0       4    "DELL"
  * Object Signature          4       4    " WMI"
  * WMI Interface Version     8       4    <version>
- * WMI buffer length        12       4    4096
+ * WMI buffer length        12       4    <length>
  */
 static int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev)
 {
@@ -674,10 +674,6 @@ static int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev)
 		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
 			(unsigned long) 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;
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 06/16] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (4 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 05/16] platform/x86: dell-wmi: don't check length returned Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 07/16] platform/x86: wmi: Don't allow drivers to get each other's GUIDs Mario Limonciello
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

All communication on individual GUIDs should occur in separate drivers.
Allowing a driver to communicate with the bus to another GUID is just
a hack that discourages drivers to adopt the bus model.

The information found from the WMI descriptor driver is now exported
for use by other drivers.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
---
 MAINTAINERS                                |   5 +
 drivers/platform/x86/Kconfig               |   5 +
 drivers/platform/x86/Makefile              |   1 +
 drivers/platform/x86/dell-wmi-descriptor.c | 170 +++++++++++++++++++++++++++++
 drivers/platform/x86/dell-wmi-descriptor.h |  21 ++++
 drivers/platform/x86/dell-wmi.c            |  80 +-------------
 6 files changed, 208 insertions(+), 74 deletions(-)
 create mode 100644 drivers/platform/x86/dell-wmi-descriptor.c
 create mode 100644 drivers/platform/x86/dell-wmi-descriptor.h

diff --git a/MAINTAINERS b/MAINTAINERS
index da135df7d098..4b654248c48e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4013,6 +4013,11 @@ M:	Pali Rohár <pali.rohar@gmail.com>
 S:	Maintained
 F:	drivers/platform/x86/dell-wmi.c
 
+DELL WMI DESCRIPTOR DRIVER
+M:	Mario Limonciello <mario.limonciello@dell.com>
+S:	Maintained
+F:	drivers/platform/x86/dell-wmi-descriptor.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 1f7959ff055c..7722923c968c 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -121,6 +121,7 @@ config DELL_WMI
 	depends on DMI
 	depends on INPUT
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
+	select DELL_WMI_DESCRIPTOR
 	select DELL_SMBIOS
 	select INPUT_SPARSEKMAP
 	---help---
@@ -129,6 +130,10 @@ config DELL_WMI
 	  To compile this driver as a module, choose M here: the module will
 	  be called dell-wmi.
 
+config DELL_WMI_DESCRIPTOR
+	tristate
+	depends on ACPI_WMI
+
 config DELL_WMI_AIO
 	tristate "WMI Hotkeys for Dell All-In-One series"
 	depends on ACPI_WMI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 2b315d0df3b7..8636f5d3424f 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -14,6 +14,7 @@ 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_DESCRIPTOR)	+= dell-wmi-descriptor.o
 obj-$(CONFIG_DELL_WMI_AIO)	+= dell-wmi-aio.o
 obj-$(CONFIG_DELL_WMI_LED)	+= dell-wmi-led.o
 obj-$(CONFIG_DELL_SMO8800)	+= dell-smo8800.o
diff --git a/drivers/platform/x86/dell-wmi-descriptor.c b/drivers/platform/x86/dell-wmi-descriptor.c
new file mode 100644
index 000000000000..3204c408e261
--- /dev/null
+++ b/drivers/platform/x86/dell-wmi-descriptor.c
@@ -0,0 +1,170 @@
+/*
+ * Dell WMI descriptor driver
+ *
+ * Copyright (C) 2017 Dell Inc. All Rights Reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License version 2 as published
+ *  by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/acpi.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/wmi.h>
+#include "dell-wmi-descriptor.h"
+
+struct descriptor_priv {
+	struct list_head list;
+	u32 interface_version;
+	u32 size;
+};
+static LIST_HEAD(wmi_list);
+static DEFINE_MUTEX(list_mutex);
+
+bool dell_wmi_get_interface_version(u32 *version)
+{
+	struct descriptor_priv *priv;
+	bool ret = false;
+
+	mutex_lock(&list_mutex);
+	priv = list_first_entry_or_null(&wmi_list,
+					struct descriptor_priv,
+					list);
+	if (priv) {
+		*version = priv->interface_version;
+		ret = true;
+	}
+	mutex_unlock(&list_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dell_wmi_get_interface_version);
+
+bool dell_wmi_get_size(u32 *size)
+{
+	struct descriptor_priv *priv;
+	bool ret = false;
+
+	mutex_lock(&list_mutex);
+	priv = list_first_entry_or_null(&wmi_list,
+					struct descriptor_priv,
+					list);
+	if (priv) {
+		*size = priv->size;
+		ret = true;
+	}
+	mutex_unlock(&list_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dell_wmi_get_size);
+
+/*
+ * 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    <length>
+ */
+static int dell_wmi_descriptor_probe(struct wmi_device *wdev)
+{
+	union acpi_object *obj = NULL;
+	struct descriptor_priv *priv;
+	u32 *buffer;
+	int ret;
+
+	obj = wmidev_block_query(wdev, 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;
+	}
+
+	/* Although it's not technically a failure, this would lead to
+	 * unexpected behavior
+	 */
+	if (obj->buffer.length != 128) {
+		dev_err(&wdev->dev,
+			"Dell descriptor buffer has unexpected length (%d)\n",
+			obj->buffer.length);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	buffer = (u32 *)obj->buffer.pointer;
+
+	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
+		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
+			buffer);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (buffer[2] != 0 && buffer[2] != 1)
+		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
+			(unsigned long) buffer[2]);
+
+	priv = devm_kzalloc(&wdev->dev, sizeof(struct descriptor_priv),
+	GFP_KERNEL);
+
+	priv->interface_version = buffer[2];
+	priv->size = buffer[3];
+	ret = 0;
+	dev_set_drvdata(&wdev->dev, priv);
+	mutex_lock(&list_mutex);
+	list_add_tail(&priv->list, &wmi_list);
+	mutex_unlock(&list_mutex);
+
+	dev_dbg(&wdev->dev, "Detected Dell WMI interface version %lu and buffer size %lu\n",
+		(unsigned long) priv->interface_version,
+		(unsigned long) priv->size);
+
+out:
+	kfree(obj);
+	return ret;
+}
+
+static int dell_wmi_descriptor_remove(struct wmi_device *wdev)
+{
+	struct descriptor_priv *priv = dev_get_drvdata(&wdev->dev);
+
+	mutex_lock(&list_mutex);
+	list_del(&priv->list);
+	mutex_unlock(&list_mutex);
+	return 0;
+}
+
+static const struct wmi_device_id dell_wmi_descriptor_id_table[] = {
+	{ .guid_string = DELL_WMI_DESCRIPTOR_GUID },
+	{ },
+};
+
+static struct wmi_driver dell_wmi_descriptor_driver = {
+	.driver = {
+		.name = "dell-wmi-descriptor",
+	},
+	.probe = dell_wmi_descriptor_probe,
+	.remove = dell_wmi_descriptor_remove,
+	.id_table = dell_wmi_descriptor_id_table,
+};
+
+module_wmi_driver(dell_wmi_descriptor_driver);
+
+MODULE_ALIAS("wmi:" DELL_WMI_DESCRIPTOR_GUID);
+MODULE_AUTHOR("Mario Limonciello <mario.limonciello@dell.com>");
+MODULE_DESCRIPTION("Dell WMI descriptor driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/platform/x86/dell-wmi-descriptor.h b/drivers/platform/x86/dell-wmi-descriptor.h
new file mode 100644
index 000000000000..5f7b69c2c83a
--- /dev/null
+++ b/drivers/platform/x86/dell-wmi-descriptor.h
@@ -0,0 +1,21 @@
+/*
+ *  Dell WMI descriptor driver
+ *
+ *  Copyright (c) 2017 Dell Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#ifndef _DELL_WMI_DESCRIPTOR_H_
+#define _DELL_WMI_DESCRIPTOR_H_
+
+#include <linux/wmi.h>
+
+#define DELL_WMI_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
+
+bool dell_wmi_get_interface_version(u32 *version);
+bool dell_wmi_get_size(u32 *size);
+
+#endif
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index da4f629d0831..6d657eb97672 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -39,6 +39,7 @@
 #include <linux/wmi.h>
 #include <acpi/video.h>
 #include "dell-smbios.h"
+#include "dell-wmi-descriptor.h"
 
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
 MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
@@ -46,7 +47,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,75 +617,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    <length>
- */
-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);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	buffer = (u32 *)obj->buffer.pointer;
-
-	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
-		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
-			buffer);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	if (buffer[2] != 0 && buffer[2] != 1)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
-			(unsigned long) buffer[2]);
-
-	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:
  *
@@ -721,7 +652,9 @@ static int dell_wmi_events_set_enabled(bool enable)
 static int dell_wmi_probe(struct wmi_device *wdev)
 {
 	struct dell_wmi_priv *priv;
-	int err;
+
+	if (!wmi_has_guid(DELL_WMI_DESCRIPTOR_GUID))
+		return -ENODEV;
 
 	priv = devm_kzalloc(
 		&wdev->dev, sizeof(struct dell_wmi_priv), GFP_KERNEL);
@@ -729,9 +662,8 @@ static int dell_wmi_probe(struct wmi_device *wdev)
 		return -ENOMEM;
 	dev_set_drvdata(&wdev->dev, priv);
 
-	err = dell_wmi_check_descriptor_buffer(wdev);
-	if (err)
-		return err;
+	if (!dell_wmi_get_interface_version(&priv->interface_version))
+		return -EPROBE_DEFER;
 
 	return dell_wmi_input_setup(wdev);
 }
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 07/16] platform/x86: wmi: Don't allow drivers to get each other's GUIDs
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (5 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 06/16] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 08/16] platform/x86: dell-smbios: only run if proper oem string is detected Mario Limonciello
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

The only driver using this was dell-wmi, and it really was a hack.
The driver was getting a data attribute from another driver and this
type of action should not be encouraged.

Rather drivers that need to interact with one another should pass
data back and forth via exported functions.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/wmi.c | 17 -----------------
 include/linux/wmi.h        |  4 ----
 2 files changed, 21 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 4d73a87c2ddf..bcb41c1c7f52 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -368,23 +368,6 @@ union acpi_object *wmidev_block_query(struct wmi_device *wdev, u8 instance)
 }
 EXPORT_SYMBOL_GPL(wmidev_block_query);
 
-struct wmi_device *wmidev_get_other_guid(struct wmi_device *wdev,
-					 const char *guid_string)
-{
-	struct wmi_block *this_wb = container_of(wdev, struct wmi_block, dev);
-	struct wmi_block *other_wb;
-
-	if (!find_guid(guid_string, &other_wb))
-		return NULL;
-
-	if (other_wb->acpi_device != this_wb->acpi_device)
-		return NULL;
-
-	get_device(&other_wb->dev.dev);
-	return &other_wb->dev;
-}
-EXPORT_SYMBOL_GPL(wmidev_get_other_guid);
-
 /**
  * wmi_set_block - Write to a WMI block
  * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index 2cd10c3b89e9..ddee427e0721 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -36,10 +36,6 @@ extern acpi_status wmidev_evaluate_method(struct wmi_device *wdev,
 extern union acpi_object *wmidev_block_query(struct wmi_device *wdev,
 					     u8 instance);
 
-/* Gets another device on the same bus.  Caller must put_device the result. */
-extern struct wmi_device *wmidev_get_other_guid(struct wmi_device *wdev,
-						const char *guid_string);
-
 struct wmi_device_id {
 	const char *guid_string;
 };
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 08/16] platform/x86: dell-smbios: only run if proper oem string is detected
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (6 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 07/16] platform/x86: wmi: Don't allow drivers to get each other's GUIDs Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 09/16] platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens Mario Limonciello
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

The proper way to indicate that a system is a 'supported' Dell System
is by the presence of this string in OEM strings.

Allowing the driver to load on non-Dell systems will have undefined
results.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
---
 drivers/platform/x86/dell-smbios.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index ffc174638aa4..12a3eb153911 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -172,8 +172,15 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy)
 
 static int __init dell_smbios_init(void)
 {
+	const struct dmi_device *valid;
 	int ret;
 
+	valid = dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL);
+	if (!valid) {
+		pr_err("Unable to run on non-Dell system\n");
+		return -ENODEV;
+	}
+
 	dmi_walk(find_tokens, NULL);
 
 	if (!da_tokens)  {
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 09/16] platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (7 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 08/16] platform/x86: dell-smbios: only run if proper oem string is detected Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 10/16] platform/x86: dell-smbios: Introduce dispatcher for SMM calls Mario Limonciello
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

Currently userspace tools can access system tokens via the dcdbas
kernel module and a SMI call that will cause the platform to execute
SMM code.

With a goal in mind of deprecating the dcdbas kernel module a different
method for accessing these tokens from userspace needs to be created.

This is intentionally marked to only be readable as a process with
CAP_SYS_ADMIN as it can contain sensitive information about the
platform's configuration.

While adding this interface I found that some tokens are duplicated.
These need to be ignored from sysfs to avoid duplicate files.

MAINTAINERS was missing for this driver.  Add myself and Pali to
maintainers list for it.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
---
 .../ABI/testing/sysfs-platform-dell-smbios         |  21 ++
 MAINTAINERS                                        |   7 +
 drivers/platform/x86/dell-smbios.c                 | 213 ++++++++++++++++++++-
 3 files changed, 240 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-smbios

diff --git a/Documentation/ABI/testing/sysfs-platform-dell-smbios b/Documentation/ABI/testing/sysfs-platform-dell-smbios
new file mode 100644
index 000000000000..205d3b6361e0
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-dell-smbios
@@ -0,0 +1,21 @@
+What:		/sys/devices/platform/<platform>/tokens/*
+Date:		November 2017
+KernelVersion:	4.15
+Contact:	"Mario Limonciello" <mario.limonciello@dell.com>
+Description:
+		A read-only description of Dell platform tokens
+		available on the machine.
+
+		Each token attribute is available as a pair of
+		sysfs attributes readable by a process with
+		CAP_SYS_ADMIN.
+
+		For example the token ID "5" would be available
+		as the following attributes:
+
+		0005_location
+		0005_value
+
+		Tokens will vary from machine to machine, and
+		only tokens available on that machine will be
+		displayed.
diff --git a/MAINTAINERS b/MAINTAINERS
index 4b654248c48e..44ff16db9260 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3978,6 +3978,13 @@ M:	"Maciej W. Rozycki" <macro@linux-mips.org>
 S:	Maintained
 F:	drivers/net/fddi/defxx.*
 
+DELL SMBIOS DRIVER
+M:	Pali Rohár <pali.rohar@gmail.com>
+M:	Mario Limonciello <mario.limonciello@dell.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+F:	drivers/platform/x86/dell-smbios.*
+
 DELL LAPTOP DRIVER
 M:	Matthew Garrett <mjg59@srcf.ucam.org>
 M:	Pali Rohár <pali.rohar@gmail.com>
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 12a3eb153911..ed4995fdcd46 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -16,10 +16,12 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/capability.h>
 #include <linux/dmi.h>
 #include <linux/err.h>
 #include <linux/gfp.h>
 #include <linux/mutex.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/io.h>
 #include "../../firmware/dcdbas.h"
@@ -39,7 +41,11 @@ static DEFINE_MUTEX(buffer_mutex);
 static int da_command_address;
 static int da_command_code;
 static int da_num_tokens;
+static struct platform_device *platform_device;
 static struct calling_interface_token *da_tokens;
+static struct device_attribute *token_location_attrs;
+static struct device_attribute *token_value_attrs;
+static struct attribute **token_attrs;
 
 int dell_smbios_error(int value)
 {
@@ -157,6 +163,27 @@ static void __init parse_da_table(const struct dmi_header *dm)
 	da_num_tokens += tokens;
 }
 
+static void zero_duplicates(struct device *dev)
+{
+	int i, j;
+
+	for (i = 0; i < da_num_tokens; i++) {
+		if (da_tokens[i].tokenID == 0)
+			continue;
+		for (j = i+1; j < da_num_tokens; j++) {
+			if (da_tokens[j].tokenID == 0)
+				continue;
+			if (da_tokens[i].tokenID == da_tokens[j].tokenID) {
+				dev_dbg(dev, "Zeroing dup token ID %x(%x/%x)\n",
+					da_tokens[j].tokenID,
+					da_tokens[j].location,
+					da_tokens[j].value);
+				da_tokens[j].tokenID = 0;
+			}
+		}
+	}
+}
+
 static void __init find_tokens(const struct dmi_header *dm, void *dummy)
 {
 	switch (dm->type) {
@@ -170,6 +197,154 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy)
 	}
 }
 
+static int match_attribute(struct device *dev,
+			   struct device_attribute *attr)
+{
+	int i;
+
+	for (i = 0; i < da_num_tokens * 2; i++) {
+		if (!token_attrs[i])
+			continue;
+		if (strcmp(token_attrs[i]->name, attr->attr.name) == 0)
+			return i/2;
+	}
+	dev_dbg(dev, "couldn't match: %s\n", attr->attr.name);
+	return -EINVAL;
+}
+
+static ssize_t location_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	int i;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	i = match_attribute(dev, attr);
+	if (i > 0)
+		return scnprintf(buf, PAGE_SIZE, "%08x", da_tokens[i].location);
+	return 0;
+}
+
+static ssize_t value_show(struct device *dev,
+			  struct device_attribute *attr, char *buf)
+{
+	int i;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	i = match_attribute(dev, attr);
+	if (i > 0)
+		return scnprintf(buf, PAGE_SIZE, "%08x", da_tokens[i].value);
+	return 0;
+}
+
+static struct attribute_group smbios_attribute_group = {
+	.name = "tokens"
+};
+
+static struct platform_driver platform_driver = {
+	.driver = {
+		.name = "dell-smbios",
+	},
+};
+
+static int build_tokens_sysfs(struct platform_device *dev)
+{
+	char buffer_location[13];
+	char buffer_value[10];
+	char *location_name;
+	char *value_name;
+	size_t size;
+	int ret;
+	int i, j;
+
+	/* (number of tokens  + 1 for null terminated */
+	size = sizeof(struct device_attribute) * (da_num_tokens + 1);
+	token_location_attrs = kzalloc(size, GFP_KERNEL);
+	if (!token_location_attrs)
+		return -ENOMEM;
+	token_value_attrs = kzalloc(size, GFP_KERNEL);
+	if (!token_value_attrs)
+		goto out_allocate_value;
+
+	/* need to store both location and value + terminator*/
+	size = sizeof(struct attribute *) * ((2 * da_num_tokens) + 1);
+	token_attrs = kzalloc(size, GFP_KERNEL);
+	if (!token_attrs)
+		goto out_allocate_attrs;
+
+	for (i = 0, j = 0; i < da_num_tokens; i++) {
+		/* skip empty */
+		if (da_tokens[i].tokenID == 0)
+			continue;
+		/* add location */
+		sprintf(buffer_location, "%04x_location",
+			da_tokens[i].tokenID);
+		location_name = kstrdup(buffer_location, GFP_KERNEL);
+		if (location_name == NULL)
+			goto out_unwind_strings;
+		sysfs_attr_init(&token_location_attrs[i].attr);
+		token_location_attrs[i].attr.name = location_name;
+		token_location_attrs[i].attr.mode = 0444;
+		token_location_attrs[i].show = location_show;
+		token_attrs[j++] = &token_location_attrs[i].attr;
+
+		/* add value */
+		sprintf(buffer_value, "%04x_value",
+			da_tokens[i].tokenID);
+		value_name = kstrdup(buffer_value, GFP_KERNEL);
+		if (value_name == NULL)
+			goto loop_fail_create_value;
+		sysfs_attr_init(&token_value_attrs[i].attr);
+		token_value_attrs[i].attr.name = value_name;
+		token_value_attrs[i].attr.mode = 0444;
+		token_value_attrs[i].show = value_show;
+		token_attrs[j++] = &token_value_attrs[i].attr;
+		continue;
+
+loop_fail_create_value:
+		kfree(value_name);
+		goto out_unwind_strings;
+	}
+	smbios_attribute_group.attrs = token_attrs;
+
+	ret = sysfs_create_group(&dev->dev.kobj, &smbios_attribute_group);
+	if (ret)
+		goto out_unwind_strings;
+	return 0;
+
+out_unwind_strings:
+	for (i = i-1; i > 0; i--) {
+		kfree(token_location_attrs[i].attr.name);
+		kfree(token_value_attrs[i].attr.name);
+	}
+	kfree(token_attrs);
+out_allocate_attrs:
+	kfree(token_value_attrs);
+out_allocate_value:
+	kfree(token_location_attrs);
+
+	return -ENOMEM;
+}
+
+static void free_group(struct platform_device *pdev)
+{
+	int i;
+
+	sysfs_remove_group(&pdev->dev.kobj,
+				&smbios_attribute_group);
+	for (i = 0; i < da_num_tokens; i++) {
+		kfree(token_location_attrs[i].attr.name);
+		kfree(token_value_attrs[i].attr.name);
+	}
+	kfree(token_attrs);
+	kfree(token_value_attrs);
+	kfree(token_location_attrs);
+}
+
+
 static int __init dell_smbios_init(void)
 {
 	const struct dmi_device *valid;
@@ -197,9 +372,40 @@ static int __init dell_smbios_init(void)
 		ret = -ENOMEM;
 		goto fail_buffer;
 	}
+	ret = platform_driver_register(&platform_driver);
+	if (ret)
+		goto fail_platform_driver;
+
+	platform_device = platform_device_alloc("dell-smbios", 0);
+	if (!platform_device) {
+		ret = -ENOMEM;
+		goto fail_platform_device_alloc;
+	}
+	ret = platform_device_add(platform_device);
+	if (ret)
+		goto fail_platform_device_add;
+
+	/* duplicate tokens will cause problems building sysfs files */
+	zero_duplicates(&platform_device->dev);
+
+	ret = build_tokens_sysfs(platform_device);
+	if (ret)
+		goto fail_create_group;
 
 	return 0;
 
+fail_create_group:
+	platform_device_del(platform_device);
+
+fail_platform_device_add:
+	platform_device_put(platform_device);
+
+fail_platform_device_alloc:
+	platform_driver_unregister(&platform_driver);
+
+fail_platform_driver:
+	free_page((unsigned long)buffer);
+
 fail_buffer:
 	kfree(da_tokens);
 	return ret;
@@ -207,8 +413,13 @@ static int __init dell_smbios_init(void)
 
 static void __exit dell_smbios_exit(void)
 {
-	kfree(da_tokens);
+	if (platform_device) {
+		free_group(platform_device);
+		platform_device_unregister(platform_device);
+		platform_driver_unregister(&platform_driver);
+	}
 	free_page((unsigned long)buffer);
+	kfree(da_tokens);
 }
 
 subsys_initcall(dell_smbios_init);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 10/16] platform/x86: dell-smbios: Introduce dispatcher for SMM calls
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (8 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 09/16] platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2018-01-27 14:48   ` Pali Rohár
  2017-11-01 19:25 ` [PATCH v12 11/16] platform/x86: dell-smbios-wmi: Add new WMI dispatcher driver Mario Limonciello
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

This splits up the dell-smbios driver into two drivers:
* dell-smbios
* dell-smbios-smm

dell-smbios can operate with multiple different dispatcher drivers to
perform SMBIOS operations.

Also modify the interface that dell-laptop and dell-wmi use align to this
model more closely.  Rather than a single global buffer being allocated
for all drivers, each driver will allocate and be responsible for it's own
buffer. The pointer will be passed to the calling function and each
dispatcher driver will then internally copy it to the proper location to
perform it's call.

Add defines for calls used by these methods in the dell-smbios.h header
for tracking purposes.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
---
 MAINTAINERS                            |   6 +
 drivers/platform/x86/Kconfig           |  15 +-
 drivers/platform/x86/Makefile          |   1 +
 drivers/platform/x86/dell-laptop.c     | 283 ++++++++++++---------------------
 drivers/platform/x86/dell-smbios-smm.c | 163 +++++++++++++++++++
 drivers/platform/x86/dell-smbios.c     | 121 +++++++-------
 drivers/platform/x86/dell-smbios.h     |  46 +++++-
 drivers/platform/x86/dell-wmi.c        |  11 +-
 8 files changed, 398 insertions(+), 248 deletions(-)
 create mode 100644 drivers/platform/x86/dell-smbios-smm.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 44ff16db9260..54b52b707201 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3985,6 +3985,12 @@ L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
 F:	drivers/platform/x86/dell-smbios.*
 
+DELL SMBIOS SMM DRIVER
+M:	Mario Limonciello <mario.limonciello@dell.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+F:	drivers/platform/x86/dell-smbios-smm.c
+
 DELL LAPTOP DRIVER
 M:	Matthew Garrett <mjg59@srcf.ucam.org>
 M:	Pali Rohár <pali.rohar@gmail.com>
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 7722923c968c..53a2de781de6 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -93,12 +93,19 @@ config ASUS_LAPTOP
 
 config DELL_SMBIOS
 	tristate
-	select DCDBAS
+
+config DELL_SMBIOS_SMM
+	tristate "Dell SMBIOS calling interface (SMM implementation)"
+	depends on DCDBAS
+	default DCDBAS
+	select DELL_SMBIOS
 	---help---
-	This module provides common functions for kernel modules using
-	Dell SMBIOS.
+	This provides an implementation for the Dell SMBIOS calling interface
+	communicated over SMI/SMM.
 
-	If you have a Dell laptop, say Y or M here.
+	If you have a Dell computer from <=2017 you should say Y or M here.
+	If you aren't sure and this module doesn't work for your computer
+	it just won't load.
 
 config DELL_LAPTOP
 	tristate "Dell Laptop Extras"
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 8636f5d3424f..e743615241f8 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -12,6 +12,7 @@ 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_SMBIOS_SMM)	+= dell-smbios-smm.o
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
 obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
 obj-$(CONFIG_DELL_WMI_DESCRIPTOR)	+= dell-wmi-descriptor.o
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index f42159fd2031..c4903c5ce7cf 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -35,18 +35,6 @@
 #include "dell-rbtn.h"
 #include "dell-smbios.h"
 
-#define BRIGHTNESS_TOKEN 0x7d
-#define KBD_LED_OFF_TOKEN 0x01E1
-#define KBD_LED_ON_TOKEN 0x01E2
-#define KBD_LED_AUTO_TOKEN 0x01E3
-#define KBD_LED_AUTO_25_TOKEN 0x02EA
-#define KBD_LED_AUTO_50_TOKEN 0x02EB
-#define KBD_LED_AUTO_75_TOKEN 0x02EC
-#define KBD_LED_AUTO_100_TOKEN 0x02F6
-#define GLOBAL_MIC_MUTE_ENABLE 0x0364
-#define GLOBAL_MIC_MUTE_DISABLE 0x0365
-#define KBD_LED_AC_TOKEN 0x0451
-
 struct quirk_entry {
 	u8 touchpad_led;
 
@@ -85,6 +73,7 @@ static struct platform_driver platform_driver = {
 	}
 };
 
+static struct calling_interface_buffer *buffer;
 static struct platform_device *platform_device;
 static struct backlight_device *dell_backlight_device;
 static struct rfkill *wifi_rfkill;
@@ -283,6 +272,27 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
 	{ }
 };
 
+void dell_set_arguments(u32 arg0, u32 arg1, u32 arg2, u32 arg3)
+{
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+	buffer->input[0] = arg0;
+	buffer->input[1] = arg1;
+	buffer->input[2] = arg2;
+	buffer->input[3] = arg3;
+}
+
+int dell_send_request(u16 class, u16 select)
+{
+	int ret;
+
+	buffer->cmd_class = class;
+	buffer->cmd_select = select;
+	ret = dell_smbios_call(buffer);
+	if (ret != 0)
+		return ret;
+	return dell_smbios_error(buffer->output[0]);
+}
+
 /*
  * Derived from information in smbios-wireless-ctl:
  *
@@ -405,7 +415,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
 
 static int dell_rfkill_set(void *data, bool blocked)
 {
-	struct calling_interface_buffer *buffer;
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 	int hwswitch_bit = (unsigned long)data - 1;
@@ -413,20 +422,16 @@ static int dell_rfkill_set(void *data, bool blocked)
 	int status;
 	int ret;
 
-	buffer = dell_smbios_get_buffer();
-
-	dell_smbios_send_request(17, 11);
-	ret = buffer->output[0];
+	dell_set_arguments(0, 0, 0, 0);
+	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
+	if (ret)
+		return ret;
 	status = buffer->output[1];
 
-	if (ret != 0)
-		goto out;
-
-	dell_smbios_clear_buffer();
-
-	buffer->input[0] = 0x2;
-	dell_smbios_send_request(17, 11);
-	ret = buffer->output[0];
+	dell_set_arguments(0x2, 0, 0, 0);
+	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
+	if (ret)
+		return ret;
 	hwswitch = buffer->output[1];
 
 	/* If the hardware switch controls this radio, and the hardware
@@ -435,28 +440,19 @@ static int dell_rfkill_set(void *data, bool blocked)
 	    (status & BIT(0)) && !(status & BIT(16)))
 		disable = 1;
 
-	dell_smbios_clear_buffer();
-
-	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
-	dell_smbios_send_request(17, 11);
-	ret = buffer->output[0];
-
- out:
-	dell_smbios_release_buffer();
-	return dell_smbios_error(ret);
+	dell_set_arguments(1 | (radio<<8) | (disable << 16), 0, 0, 0);
+	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
+	return ret;
 }
 
-/* Must be called with the buffer held */
 static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
-					int status,
-					struct calling_interface_buffer *buffer)
+					int status)
 {
 	if (status & BIT(0)) {
 		/* Has hw-switch, sync sw_state to BIOS */
 		int block = rfkill_blocked(rfkill);
-		dell_smbios_clear_buffer();
-		buffer->input[0] = (1 | (radio << 8) | (block << 16));
-		dell_smbios_send_request(17, 11);
+		dell_set_arguments(1 | (radio << 8) | (block << 16), 0, 0, 0);
+		dell_send_request(CLASS_INFO, SELECT_RFKILL);
 	} else {
 		/* No hw-switch, sync BIOS state to sw_state */
 		rfkill_set_sw_state(rfkill, !!(status & BIT(radio + 16)));
@@ -472,32 +468,23 @@ static void dell_rfkill_update_hw_state(struct rfkill *rfkill, int radio,
 
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
-	struct calling_interface_buffer *buffer;
 	int radio = ((unsigned long)data & 0xF);
 	int hwswitch;
 	int status;
 	int ret;
 
-	buffer = dell_smbios_get_buffer();
-
-	dell_smbios_send_request(17, 11);
-	ret = buffer->output[0];
+	dell_set_arguments(0, 0, 0, 0);
+	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
 	status = buffer->output[1];
 
 	if (ret != 0 || !(status & BIT(0))) {
-		dell_smbios_release_buffer();
 		return;
 	}
 
-	dell_smbios_clear_buffer();
-
-	buffer->input[0] = 0x2;
-	dell_smbios_send_request(17, 11);
-	ret = buffer->output[0];
+	dell_set_arguments(0, 0x2, 0, 0);
+	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
 	hwswitch = buffer->output[1];
 
-	dell_smbios_release_buffer();
-
 	if (ret != 0)
 		return;
 
@@ -513,27 +500,23 @@ static struct dentry *dell_laptop_dir;
 
 static int dell_debugfs_show(struct seq_file *s, void *data)
 {
-	struct calling_interface_buffer *buffer;
 	int hwswitch_state;
 	int hwswitch_ret;
 	int status;
 	int ret;
 
-	buffer = dell_smbios_get_buffer();
-
-	dell_smbios_send_request(17, 11);
-	ret = buffer->output[0];
+	dell_set_arguments(0, 0, 0, 0);
+	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
+	if (ret)
+		return ret;
 	status = buffer->output[1];
 
-	dell_smbios_clear_buffer();
-
-	buffer->input[0] = 0x2;
-	dell_smbios_send_request(17, 11);
-	hwswitch_ret = buffer->output[0];
+	dell_set_arguments(0, 0x2, 0, 0);
+	hwswitch_ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
+	if (hwswitch_ret)
+		return hwswitch_ret;
 	hwswitch_state = buffer->output[1];
 
-	dell_smbios_release_buffer();
-
 	seq_printf(s, "return:\t%d\n", ret);
 	seq_printf(s, "status:\t0x%X\n", status);
 	seq_printf(s, "Bit 0 : Hardware switch supported:   %lu\n",
@@ -613,46 +596,36 @@ static const struct file_operations dell_debugfs_fops = {
 
 static void dell_update_rfkill(struct work_struct *ignored)
 {
-	struct calling_interface_buffer *buffer;
 	int hwswitch = 0;
 	int status;
 	int ret;
 
-	buffer = dell_smbios_get_buffer();
-
-	dell_smbios_send_request(17, 11);
-	ret = buffer->output[0];
+	dell_set_arguments(0, 0, 0, 0);
+	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
 	status = buffer->output[1];
 
 	if (ret != 0)
-		goto out;
-
-	dell_smbios_clear_buffer();
+		return;
 
-	buffer->input[0] = 0x2;
-	dell_smbios_send_request(17, 11);
-	ret = buffer->output[0];
+	dell_set_arguments(0, 0x2, 0, 0);
+	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
 
 	if (ret == 0 && (status & BIT(0)))
 		hwswitch = buffer->output[1];
 
 	if (wifi_rfkill) {
 		dell_rfkill_update_hw_state(wifi_rfkill, 1, status, hwswitch);
-		dell_rfkill_update_sw_state(wifi_rfkill, 1, status, buffer);
+		dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
 	}
 	if (bluetooth_rfkill) {
 		dell_rfkill_update_hw_state(bluetooth_rfkill, 2, status,
 					    hwswitch);
-		dell_rfkill_update_sw_state(bluetooth_rfkill, 2, status,
-					    buffer);
+		dell_rfkill_update_sw_state(bluetooth_rfkill, 2, status);
 	}
 	if (wwan_rfkill) {
 		dell_rfkill_update_hw_state(wwan_rfkill, 3, status, hwswitch);
-		dell_rfkill_update_sw_state(wwan_rfkill, 3, status, buffer);
+		dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
 	}
-
- out:
-	dell_smbios_release_buffer();
 }
 static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
 
@@ -696,7 +669,6 @@ static struct notifier_block dell_laptop_rbtn_notifier = {
 
 static int __init dell_setup_rfkill(void)
 {
-	struct calling_interface_buffer *buffer;
 	int status, ret, whitelisted;
 	const char *product;
 
@@ -712,11 +684,9 @@ static int __init dell_setup_rfkill(void)
 	if (!force_rfkill && !whitelisted)
 		return 0;
 
-	buffer = dell_smbios_get_buffer();
-	dell_smbios_send_request(17, 11);
-	ret = buffer->output[0];
+	dell_set_arguments(0, 0, 0, 0);
+	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
 	status = buffer->output[1];
-	dell_smbios_release_buffer();
 
 	/* dell wireless info smbios call is not supported */
 	if (ret != 0)
@@ -869,7 +839,6 @@ static void dell_cleanup_rfkill(void)
 
 static int dell_send_intensity(struct backlight_device *bd)
 {
-	struct calling_interface_buffer *buffer;
 	struct calling_interface_token *token;
 	int ret;
 
@@ -877,24 +846,17 @@ static int dell_send_intensity(struct backlight_device *bd)
 	if (!token)
 		return -ENODEV;
 
-	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = token->location;
-	buffer->input[1] = bd->props.brightness;
-
+	dell_set_arguments(token->location, bd->props.brightness, 0, 0);
 	if (power_supply_is_system_supplied() > 0)
-		dell_smbios_send_request(1, 2);
+		ret = dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_AC);
 	else
-		dell_smbios_send_request(1, 1);
+		ret = dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_BAT);
 
-	ret = dell_smbios_error(buffer->output[0]);
-
-	dell_smbios_release_buffer();
 	return ret;
 }
 
 static int dell_get_intensity(struct backlight_device *bd)
 {
-	struct calling_interface_buffer *buffer;
 	struct calling_interface_token *token;
 	int ret;
 
@@ -902,20 +864,14 @@ static int dell_get_intensity(struct backlight_device *bd)
 	if (!token)
 		return -ENODEV;
 
-	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = token->location;
-
+	dell_set_arguments(token->location, 0, 0, 0);
 	if (power_supply_is_system_supplied() > 0)
-		dell_smbios_send_request(0, 2);
+		ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_AC);
 	else
-		dell_smbios_send_request(0, 1);
+		ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_BAT);
 
-	if (buffer->output[0])
-		ret = dell_smbios_error(buffer->output[0]);
-	else
+	if (ret == 0)
 		ret = buffer->output[1];
-
-	dell_smbios_release_buffer();
 	return ret;
 }
 
@@ -1179,20 +1135,13 @@ static DEFINE_MUTEX(kbd_led_mutex);
 
 static int kbd_get_info(struct kbd_info *info)
 {
-	struct calling_interface_buffer *buffer;
 	u8 units;
 	int ret;
 
-	buffer = dell_smbios_get_buffer();
-
-	buffer->input[0] = 0x0;
-	dell_smbios_send_request(4, 11);
-	ret = buffer->output[0];
-
-	if (ret) {
-		ret = dell_smbios_error(ret);
-		goto out;
-	}
+	dell_set_arguments(0, 0, 0, 0);
+	ret = dell_send_request(CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT);
+	if (ret)
+		return ret;
 
 	info->modes = buffer->output[1] & 0xFFFF;
 	info->type = (buffer->output[1] >> 24) & 0xFF;
@@ -1209,8 +1158,6 @@ static int kbd_get_info(struct kbd_info *info)
 	if (units & BIT(3))
 		info->days = (buffer->output[3] >> 24) & 0xFF;
 
- out:
-	dell_smbios_release_buffer();
 	return ret;
 }
 
@@ -1269,19 +1216,12 @@ static int kbd_set_level(struct kbd_state *state, u8 level)
 
 static int kbd_get_state(struct kbd_state *state)
 {
-	struct calling_interface_buffer *buffer;
 	int ret;
 
-	buffer = dell_smbios_get_buffer();
-
-	buffer->input[0] = 0x1;
-	dell_smbios_send_request(4, 11);
-	ret = buffer->output[0];
-
-	if (ret) {
-		ret = dell_smbios_error(ret);
-		goto out;
-	}
+	dell_set_arguments(0x1, 0, 0, 0);
+	ret = dell_send_request(CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT);
+	if (ret)
+		return ret;
 
 	state->mode_bit = ffs(buffer->output[1] & 0xFFFF);
 	if (state->mode_bit != 0)
@@ -1296,31 +1236,27 @@ static int kbd_get_state(struct kbd_state *state)
 	state->timeout_value_ac = (buffer->output[2] >> 24) & 0x3F;
 	state->timeout_unit_ac = (buffer->output[2] >> 30) & 0x3;
 
- out:
-	dell_smbios_release_buffer();
 	return ret;
 }
 
 static int kbd_set_state(struct kbd_state *state)
 {
-	struct calling_interface_buffer *buffer;
 	int ret;
+	u32 input1;
+	u32 input2;
+
+	input1 = BIT(state->mode_bit) & 0xFFFF;
+	input1 |= (state->triggers & 0xFF) << 16;
+	input1 |= (state->timeout_value & 0x3F) << 24;
+	input1 |= (state->timeout_unit & 0x3) << 30;
+	input2 = state->als_setting & 0xFF;
+	input2 |= (state->level & 0xFF) << 16;
+	input2 |= (state->timeout_value_ac & 0x3F) << 24;
+	input2 |= (state->timeout_unit_ac & 0x3) << 30;
+	dell_set_arguments(0x2, input1, input2, 0);
+	ret = dell_send_request(CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT);
 
-	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = 0x2;
-	buffer->input[1] = BIT(state->mode_bit) & 0xFFFF;
-	buffer->input[1] |= (state->triggers & 0xFF) << 16;
-	buffer->input[1] |= (state->timeout_value & 0x3F) << 24;
-	buffer->input[1] |= (state->timeout_unit & 0x3) << 30;
-	buffer->input[2] = state->als_setting & 0xFF;
-	buffer->input[2] |= (state->level & 0xFF) << 16;
-	buffer->input[2] |= (state->timeout_value_ac & 0x3F) << 24;
-	buffer->input[2] |= (state->timeout_unit_ac & 0x3) << 30;
-	dell_smbios_send_request(4, 11);
-	ret = buffer->output[0];
-	dell_smbios_release_buffer();
-
-	return dell_smbios_error(ret);
+	return ret;
 }
 
 static int kbd_set_state_safe(struct kbd_state *state, struct kbd_state *old)
@@ -1345,7 +1281,6 @@ static int kbd_set_state_safe(struct kbd_state *state, struct kbd_state *old)
 
 static int kbd_set_token_bit(u8 bit)
 {
-	struct calling_interface_buffer *buffer;
 	struct calling_interface_token *token;
 	int ret;
 
@@ -1356,19 +1291,14 @@ static int kbd_set_token_bit(u8 bit)
 	if (!token)
 		return -EINVAL;
 
-	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = token->location;
-	buffer->input[1] = token->value;
-	dell_smbios_send_request(1, 0);
-	ret = buffer->output[0];
-	dell_smbios_release_buffer();
+	dell_set_arguments(token->location, token->value, 0, 0);
+	ret = dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_STD);
 
-	return dell_smbios_error(ret);
+	return ret;
 }
 
 static int kbd_get_token_bit(u8 bit)
 {
-	struct calling_interface_buffer *buffer;
 	struct calling_interface_token *token;
 	int ret;
 	int val;
@@ -1380,15 +1310,12 @@ static int kbd_get_token_bit(u8 bit)
 	if (!token)
 		return -EINVAL;
 
-	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = token->location;
-	dell_smbios_send_request(0, 0);
-	ret = buffer->output[0];
+	dell_set_arguments(token->location, 0, 0, 0);
+	ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_STD);
 	val = buffer->output[1];
-	dell_smbios_release_buffer();
 
 	if (ret)
-		return dell_smbios_error(ret);
+		return ret;
 
 	return (val == token->value);
 }
@@ -2102,7 +2029,6 @@ static struct notifier_block dell_laptop_notifier = {
 
 int dell_micmute_led_set(int state)
 {
-	struct calling_interface_buffer *buffer;
 	struct calling_interface_token *token;
 
 	if (state == 0)
@@ -2115,11 +2041,8 @@ int dell_micmute_led_set(int state)
 	if (!token)
 		return -ENODEV;
 
-	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = token->location;
-	buffer->input[1] = token->value;
-	dell_smbios_send_request(1, 0);
-	dell_smbios_release_buffer();
+	dell_set_arguments(token->location, token->value, 0, 0);
+	dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_STD);
 
 	return state;
 }
@@ -2127,7 +2050,6 @@ EXPORT_SYMBOL_GPL(dell_micmute_led_set);
 
 static int __init dell_init(void)
 {
-	struct calling_interface_buffer *buffer;
 	struct calling_interface_token *token;
 	int max_intensity = 0;
 	int ret;
@@ -2158,6 +2080,10 @@ static int __init dell_init(void)
 		goto fail_rfkill;
 	}
 
+	buffer = kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
+	if (!buffer)
+		goto fail_buffer;
+
 	if (quirks && quirks->touchpad_led)
 		touchpad_led_init(&platform_device->dev);
 
@@ -2175,12 +2101,10 @@ static int __init dell_init(void)
 
 	token = dell_smbios_find_token(BRIGHTNESS_TOKEN);
 	if (token) {
-		buffer = dell_smbios_get_buffer();
-		buffer->input[0] = token->location;
-		dell_smbios_send_request(0, 2);
-		if (buffer->output[0] == 0)
+		dell_set_arguments(token->location, 0, 0, 0);
+		ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_AC);
+		if (ret)
 			max_intensity = buffer->output[3];
-		dell_smbios_release_buffer();
 	}
 
 	if (max_intensity) {
@@ -2214,6 +2138,8 @@ static int __init dell_init(void)
 fail_get_brightness:
 	backlight_device_unregister(dell_backlight_device);
 fail_backlight:
+	kfree(buffer);
+fail_buffer:
 	dell_cleanup_rfkill();
 fail_rfkill:
 	platform_device_del(platform_device);
@@ -2233,6 +2159,7 @@ static void __exit dell_exit(void)
 		touchpad_led_exit();
 	kbd_led_exit();
 	backlight_device_unregister(dell_backlight_device);
+	kfree(buffer);
 	dell_cleanup_rfkill();
 	if (platform_device) {
 		platform_device_unregister(platform_device);
diff --git a/drivers/platform/x86/dell-smbios-smm.c b/drivers/platform/x86/dell-smbios-smm.c
new file mode 100644
index 000000000000..53eabb14fb48
--- /dev/null
+++ b/drivers/platform/x86/dell-smbios-smm.c
@@ -0,0 +1,163 @@
+/*
+ *  SMI methods for use with dell-smbios
+ *
+ *  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.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/dmi.h>
+#include <linux/gfp.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include "../../firmware/dcdbas.h"
+#include "dell-smbios.h"
+
+static int da_command_address;
+static int da_command_code;
+static struct calling_interface_buffer *buffer;
+struct platform_device *platform_device;
+static DEFINE_MUTEX(smm_mutex);
+
+static const struct dmi_system_id dell_device_table[] __initconst = {
+	{
+		.ident = "Dell laptop",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "8"),
+		},
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "9"), /*Laptop*/
+		},
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /*Notebook*/
+		},
+	},
+	{
+		.ident = "Dell Computer Corporation",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer Corporation"),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "8"),
+		},
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(dmi, dell_device_table);
+
+static void __init parse_da_table(const struct dmi_header *dm)
+{
+	struct calling_interface_structure *table =
+		container_of(dm, struct calling_interface_structure, header);
+
+	/* 4 bytes of table header, plus 7 bytes of Dell header, plus at least
+	 * 6 bytes of entry
+	 */
+	if (dm->length < 17)
+		return;
+
+	da_command_address = table->cmdIOAddress;
+	da_command_code = table->cmdIOCode;
+}
+
+static void __init find_cmd_address(const struct dmi_header *dm, void *dummy)
+{
+	switch (dm->type) {
+	case 0xda: /* Calling interface */
+		parse_da_table(dm);
+		break;
+	}
+}
+
+int dell_smbios_smm_call(struct calling_interface_buffer *input)
+{
+	struct smi_cmd command;
+	size_t size;
+
+	size = sizeof(struct calling_interface_buffer);
+	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;
+
+	mutex_lock(&smm_mutex);
+	memcpy(buffer, input, size);
+	dcdbas_smi_request(&command);
+	memcpy(input, buffer, size);
+	mutex_unlock(&smm_mutex);
+	return 0;
+}
+
+static int __init dell_smbios_smm_init(void)
+{
+	int ret;
+	/*
+	 * 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)
+		return -ENOMEM;
+
+	dmi_walk(find_cmd_address, NULL);
+
+	platform_device = platform_device_alloc("dell-smbios", 1);
+	if (!platform_device) {
+		ret = -ENOMEM;
+		goto fail_platform_device_alloc;
+	}
+
+	ret = platform_device_add(platform_device);
+	if (ret)
+		goto fail_platform_device_add;
+
+	ret = dell_smbios_register_device(&platform_device->dev,
+					  &dell_smbios_smm_call);
+	if (ret)
+		goto fail_register;
+
+	return 0;
+
+fail_register:
+	platform_device_del(platform_device);
+
+fail_platform_device_add:
+	platform_device_put(platform_device);
+
+fail_platform_device_alloc:
+	free_page((unsigned long)buffer);
+	return ret;
+}
+
+static void __exit dell_smbios_smm_exit(void)
+{
+	if (platform_device) {
+		dell_smbios_unregister_device(&platform_device->dev);
+		platform_device_unregister(platform_device);
+		free_page((unsigned long)buffer);
+	}
+}
+
+subsys_initcall(dell_smbios_smm_init);
+module_exit(dell_smbios_smm_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("Dell SMBIOS communications over SMI");
+MODULE_LICENSE("GPL");
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index ed4995fdcd46..2229d44cb92c 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -19,33 +19,26 @@
 #include <linux/capability.h>
 #include <linux/dmi.h>
 #include <linux/err.h>
-#include <linux/gfp.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
-#include <linux/io.h>
-#include "../../firmware/dcdbas.h"
 #include "dell-smbios.h"
 
-struct calling_interface_structure {
-	struct dmi_header header;
-	u16 cmdIOAddress;
-	u8 cmdIOCode;
-	u32 supportedCmds;
-	struct calling_interface_token tokens[];
-} __packed;
-
-static struct calling_interface_buffer *buffer;
-static DEFINE_MUTEX(buffer_mutex);
-
-static int da_command_address;
-static int da_command_code;
 static int da_num_tokens;
 static struct platform_device *platform_device;
 static struct calling_interface_token *da_tokens;
 static struct device_attribute *token_location_attrs;
 static struct device_attribute *token_value_attrs;
 static struct attribute **token_attrs;
+static DEFINE_MUTEX(smbios_mutex);
+
+struct smbios_device {
+	struct list_head list;
+	struct device *device;
+	int (*call_fn)(struct calling_interface_buffer *);
+};
+
+static LIST_HEAD(smbios_device_list);
 
 int dell_smbios_error(int value)
 {
@@ -62,42 +55,71 @@ int dell_smbios_error(int value)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_error);
 
-struct calling_interface_buffer *dell_smbios_get_buffer(void)
+int dell_smbios_register_device(struct device *d, void *call_fn)
 {
-	mutex_lock(&buffer_mutex);
-	dell_smbios_clear_buffer();
-	return buffer;
-}
-EXPORT_SYMBOL_GPL(dell_smbios_get_buffer);
+	struct smbios_device *priv;
 
-void dell_smbios_clear_buffer(void)
-{
-	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+	priv = devm_kzalloc(d, sizeof(struct smbios_device), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	get_device(d);
+	priv->device = d;
+	priv->call_fn = call_fn;
+	mutex_lock(&smbios_mutex);
+	list_add_tail(&priv->list, &smbios_device_list);
+	mutex_unlock(&smbios_mutex);
+	dev_dbg(d, "Added device: %s\n", d->driver->name);
+	return 0;
 }
-EXPORT_SYMBOL_GPL(dell_smbios_clear_buffer);
+EXPORT_SYMBOL_GPL(dell_smbios_register_device);
 
-void dell_smbios_release_buffer(void)
+void dell_smbios_unregister_device(struct device *d)
 {
-	mutex_unlock(&buffer_mutex);
+	struct smbios_device *priv;
+
+	mutex_lock(&smbios_mutex);
+	list_for_each_entry(priv, &smbios_device_list, list) {
+		if (priv->device == d) {
+			list_del(&priv->list);
+			put_device(d);
+			break;
+		}
+	}
+	mutex_unlock(&smbios_mutex);
+	dev_dbg(d, "Remove device: %s\n", d->driver->name);
 }
-EXPORT_SYMBOL_GPL(dell_smbios_release_buffer);
+EXPORT_SYMBOL_GPL(dell_smbios_unregister_device);
 
-void dell_smbios_send_request(int class, int select)
+int dell_smbios_call(struct calling_interface_buffer *buffer)
 {
-	struct smi_cmd command;
+	int (*call_fn)(struct calling_interface_buffer *) = NULL;
+	struct device *selected_dev = NULL;
+	struct smbios_device *priv;
+	int ret;
+
+	mutex_lock(&smbios_mutex);
+	list_for_each_entry(priv, &smbios_device_list, list) {
+		if (!selected_dev || priv->device->id >= selected_dev->id) {
+			dev_dbg(priv->device, "Trying device ID: %d\n",
+				priv->device->id);
+			call_fn = priv->call_fn;
+			selected_dev = priv->device;
+		}
+	}
 
-	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;
+	if (!selected_dev) {
+		ret = -ENODEV;
+		pr_err("No dell-smbios drivers are loaded\n");
+		goto out_smbios_call;
+	}
 
-	buffer->cmd_class = class;
-	buffer->cmd_select = select;
+	ret = call_fn(buffer);
 
-	dcdbas_smi_request(&command);
+out_smbios_call:
+	mutex_unlock(&smbios_mutex);
+	return ret;
 }
-EXPORT_SYMBOL_GPL(dell_smbios_send_request);
+EXPORT_SYMBOL_GPL(dell_smbios_call);
 
 struct calling_interface_token *dell_smbios_find_token(int tokenid)
 {
@@ -146,9 +168,6 @@ static void __init parse_da_table(const struct dmi_header *dm)
 	if (dm->length < 17)
 		return;
 
-	da_command_address = table->cmdIOAddress;
-	da_command_code = table->cmdIOCode;
-
 	new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
 				 sizeof(struct calling_interface_token),
 				 GFP_KERNEL);
@@ -344,7 +363,6 @@ static void free_group(struct platform_device *pdev)
 	kfree(token_location_attrs);
 }
 
-
 static int __init dell_smbios_init(void)
 {
 	const struct dmi_device *valid;
@@ -363,15 +381,6 @@ 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);
-	if (!buffer) {
-		ret = -ENOMEM;
-		goto fail_buffer;
-	}
 	ret = platform_driver_register(&platform_driver);
 	if (ret)
 		goto fail_platform_driver;
@@ -404,22 +413,20 @@ static int __init dell_smbios_init(void)
 	platform_driver_unregister(&platform_driver);
 
 fail_platform_driver:
-	free_page((unsigned long)buffer);
-
-fail_buffer:
 	kfree(da_tokens);
 	return ret;
 }
 
 static void __exit dell_smbios_exit(void)
 {
+	mutex_lock(&smbios_mutex);
 	if (platform_device) {
 		free_group(platform_device);
 		platform_device_unregister(platform_device);
 		platform_driver_unregister(&platform_driver);
 	}
-	free_page((unsigned long)buffer);
 	kfree(da_tokens);
+	mutex_unlock(&smbios_mutex);
 }
 
 subsys_initcall(dell_smbios_init);
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 742dd8bd66b9..b3179f5b326b 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -16,6 +16,35 @@
 #ifndef _DELL_SMBIOS_H_
 #define _DELL_SMBIOS_H_
 
+#include <linux/device.h>
+
+/* Classes and selects used in kernel drivers */
+#define CLASS_TOKEN_READ 0
+#define CLASS_TOKEN_WRITE 1
+#define SELECT_TOKEN_STD 0
+#define SELECT_TOKEN_BAT 1
+#define SELECT_TOKEN_AC 2
+#define CLASS_KBD_BACKLIGHT 4
+#define SELECT_KBD_BACKLIGHT 11
+#define CLASS_INFO 17
+#define SELECT_RFKILL 11
+#define SELECT_APP_REGISTRATION	3
+
+/* Tokens used in kernel drivers, any of these
+ * should be filtered from userspace access
+ */
+#define BRIGHTNESS_TOKEN	0x007d
+#define KBD_LED_AC_TOKEN	0x0451
+#define KBD_LED_OFF_TOKEN	0x01E1
+#define KBD_LED_ON_TOKEN	0x01E2
+#define KBD_LED_AUTO_TOKEN	0x01E3
+#define KBD_LED_AUTO_25_TOKEN	0x02EA
+#define KBD_LED_AUTO_50_TOKEN	0x02EB
+#define KBD_LED_AUTO_75_TOKEN	0x02EC
+#define KBD_LED_AUTO_100_TOKEN	0x02F6
+#define GLOBAL_MIC_MUTE_ENABLE	0x0364
+#define GLOBAL_MIC_MUTE_DISABLE	0x0365
+
 struct notifier_block;
 
 /* This structure will be modified by the firmware when we enter
@@ -37,12 +66,19 @@ struct calling_interface_token {
 	};
 };
 
-int dell_smbios_error(int value);
+struct calling_interface_structure {
+	struct dmi_header header;
+	u16 cmdIOAddress;
+	u8 cmdIOCode;
+	u32 supportedCmds;
+	struct calling_interface_token tokens[];
+} __packed;
 
-struct calling_interface_buffer *dell_smbios_get_buffer(void);
-void dell_smbios_clear_buffer(void);
-void dell_smbios_release_buffer(void);
-void dell_smbios_send_request(int class, int select);
+int dell_smbios_register_device(struct device *d, void *call_fn);
+void dell_smbios_unregister_device(struct device *d);
+
+int dell_smbios_error(int value);
+int dell_smbios_call(struct calling_interface_buffer *buffer);
 
 struct calling_interface_token *dell_smbios_find_token(int tokenid);
 
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 6d657eb97672..54321080a30d 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -638,13 +638,16 @@ static int dell_wmi_events_set_enabled(bool enable)
 	struct calling_interface_buffer *buffer;
 	int ret;
 
-	buffer = dell_smbios_get_buffer();
+	buffer = kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
+	buffer->cmd_class = CLASS_INFO;
+	buffer->cmd_select = SELECT_APP_REGISTRATION;
 	buffer->input[0] = 0x10000;
 	buffer->input[1] = 0x51534554;
 	buffer->input[3] = enable;
-	dell_smbios_send_request(17, 3);
-	ret = buffer->output[0];
-	dell_smbios_release_buffer();
+	ret = dell_smbios_call(buffer);
+	if (ret == 0)
+		ret = buffer->output[0];
+	kfree(buffer);
 
 	return dell_smbios_error(ret);
 }
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 11/16] platform/x86: dell-smbios-wmi: Add new WMI dispatcher driver
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (9 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 10/16] platform/x86: dell-smbios: Introduce dispatcher for SMM calls Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 12/16] platform/x86: dell-smbios-smm: test for WSMT Mario Limonciello
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

The dell-smbios stack only currently uses an SMI interface which grants
direct access to physical memory to the firmware SMM methods via a pointer.

This dispatcher driver adds a WMI-ACPI interface that is detected by WMI
probe and preferred over the SMI interface in dell-smbios.

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

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

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
---
 MAINTAINERS                            |   6 +
 drivers/platform/x86/Kconfig           |  14 ++
 drivers/platform/x86/Makefile          |   1 +
 drivers/platform/x86/dell-smbios-wmi.c | 236 +++++++++++++++++++++++++++++++++
 4 files changed, 257 insertions(+)
 create mode 100644 drivers/platform/x86/dell-smbios-wmi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 54b52b707201..0d7061c05b15 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3991,6 +3991,12 @@ L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
 F:	drivers/platform/x86/dell-smbios-smm.c
 
+DELL SMBIOS WMI DRIVER
+M:	Mario Limonciello <mario.limonciello@dell.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+F:	drivers/platform/x86/dell-smbios-wmi.c
+
 DELL LAPTOP DRIVER
 M:	Matthew Garrett <mjg59@srcf.ucam.org>
 M:	Pali Rohár <pali.rohar@gmail.com>
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 53a2de781de6..83aee9068c64 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -94,6 +94,20 @@ config ASUS_LAPTOP
 config DELL_SMBIOS
 	tristate
 
+config DELL_SMBIOS_WMI
+	tristate "Dell SMBIOS calling interface (WMI implementation)"
+	depends on ACPI_WMI
+	select DELL_WMI_DESCRIPTOR
+	default ACPI_WMI
+	select DELL_SMBIOS
+	---help---
+	This provides an implementation for the Dell SMBIOS calling interface
+	communicated over ACPI-WMI.
+
+	If you have a Dell computer from >2007 you should say Y or M here.
+	If you aren't sure and this module doesn't work for your computer
+	it just won't load.
+
 config DELL_SMBIOS_SMM
 	tristate "Dell SMBIOS calling interface (SMM implementation)"
 	depends on DCDBAS
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index e743615241f8..1c4234861de0 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -12,6 +12,7 @@ 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_SMBIOS_WMI)	+= dell-smbios-wmi.o
 obj-$(CONFIG_DELL_SMBIOS_SMM)	+= dell-smbios-smm.o
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
 obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
diff --git a/drivers/platform/x86/dell-smbios-wmi.c b/drivers/platform/x86/dell-smbios-wmi.c
new file mode 100644
index 000000000000..b31f457e58c3
--- /dev/null
+++ b/drivers/platform/x86/dell-smbios-wmi.c
@@ -0,0 +1,236 @@
+/*
+ *  WMI methods for use with dell-smbios
+ *
+ *  Copyright (c) 2017 Dell Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/dmi.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/uaccess.h>
+#include <linux/wmi.h>
+#include "dell-smbios.h"
+#include "dell-wmi-descriptor.h"
+
+static DEFINE_MUTEX(call_mutex);
+static DEFINE_MUTEX(list_mutex);
+static int wmi_supported;
+
+struct misc_bios_flags_structure {
+	struct dmi_header header;
+	u16 flags0;
+} __packed;
+#define FLAG_HAS_ACPI_WMI 0x02
+
+#define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
+
+struct dell_wmi_extensions {
+	__u32 argattrib;
+	__u32 blength;
+	__u8 data[];
+} __packed;
+
+struct dell_wmi_smbios_buffer {
+	struct calling_interface_buffer std;
+	struct dell_wmi_extensions ext;
+} __packed;
+
+struct wmi_smbios_priv {
+	struct dell_wmi_smbios_buffer *buf;
+	struct list_head list;
+	struct wmi_device *wdev;
+	struct device *child;
+	u32 req_buf_size;
+};
+static LIST_HEAD(wmi_list);
+
+static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
+{
+	return list_first_entry_or_null(&wmi_list,
+					struct wmi_smbios_priv,
+					list);
+}
+
+static int run_smbios_call(struct wmi_device *wdev)
+{
+	struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
+	struct wmi_smbios_priv *priv;
+	struct acpi_buffer input;
+	union acpi_object *obj;
+	acpi_status status;
+
+	priv = dev_get_drvdata(&wdev->dev);
+	input.length = priv->req_buf_size - sizeof(u64);
+	input.pointer = &priv->buf->std;
+
+	dev_dbg(&wdev->dev, "evaluating: %u/%u [%x,%x,%x,%x]\n",
+		priv->buf->std.cmd_class, priv->buf->std.cmd_select,
+		priv->buf->std.input[0], priv->buf->std.input[1],
+		priv->buf->std.input[2], priv->buf->std.input[3]);
+
+	status = wmidev_evaluate_method(wdev, 0, 1, &input, &output);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+	obj = (union acpi_object *)output.pointer;
+	if (obj->type != ACPI_TYPE_BUFFER) {
+		dev_dbg(&wdev->dev, "received type: %d\n", obj->type);
+		if (obj->type == ACPI_TYPE_INTEGER)
+			dev_dbg(&wdev->dev, "SMBIOS call failed: %llu\n",
+				obj->integer.value);
+		return -EIO;
+	}
+	memcpy(&priv->buf->std, obj->buffer.pointer, obj->buffer.length);
+	dev_dbg(&wdev->dev, "result: [%08x,%08x,%08x,%08x]\n",
+		priv->buf->std.output[0], priv->buf->std.output[1],
+		priv->buf->std.output[2], priv->buf->std.output[3]);
+
+	return 0;
+}
+
+int dell_smbios_wmi_call(struct calling_interface_buffer *buffer)
+{
+	struct wmi_smbios_priv *priv;
+	size_t difference;
+	size_t size;
+	int ret;
+
+	mutex_lock(&call_mutex);
+	priv = get_first_smbios_priv();
+	if (!priv)
+		return -ENODEV;
+
+	size = sizeof(struct calling_interface_buffer);
+	difference = priv->req_buf_size - sizeof(u64) - size;
+
+	memset(&priv->buf->ext, 0, difference);
+	memcpy(&priv->buf->std, buffer, size);
+	ret = run_smbios_call(priv->wdev);
+	memcpy(buffer, &priv->buf->std, size);
+	mutex_unlock(&call_mutex);
+
+	return ret;
+}
+
+static int dell_smbios_wmi_probe(struct wmi_device *wdev)
+{
+	struct wmi_smbios_priv *priv;
+	int count;
+	int ret;
+
+	if (!wmi_has_guid(DELL_WMI_DESCRIPTOR_GUID))
+		return -ENODEV;
+
+	priv = devm_kzalloc(&wdev->dev, sizeof(struct wmi_smbios_priv),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	/* WMI buffer size will be either 4k or 32k depending on machine */
+	if (!dell_wmi_get_size(&priv->req_buf_size))
+		return -EPROBE_DEFER;
+
+	count = get_order(priv->req_buf_size);
+	priv->buf = (void *)__get_free_pages(GFP_KERNEL, count);
+	if (!priv->buf)
+		return -ENOMEM;
+
+	/* ID is used by dell-smbios to set priority of drivers */
+	wdev->dev.id = 1;
+	ret = dell_smbios_register_device(&wdev->dev, &dell_smbios_wmi_call);
+	if (ret)
+		goto fail_register;
+
+	priv->wdev = wdev;
+	dev_set_drvdata(&wdev->dev, priv);
+	mutex_lock(&list_mutex);
+	list_add_tail(&priv->list, &wmi_list);
+	mutex_unlock(&list_mutex);
+
+	return 0;
+
+fail_register:
+	free_pages((unsigned long)priv->buf, count);
+	return ret;
+}
+
+static int dell_smbios_wmi_remove(struct wmi_device *wdev)
+{
+	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
+	int count;
+
+	mutex_lock(&call_mutex);
+	mutex_lock(&list_mutex);
+	list_del(&priv->list);
+	mutex_unlock(&list_mutex);
+	dell_smbios_unregister_device(&wdev->dev);
+	count = get_order(priv->req_buf_size);
+	free_pages((unsigned long)priv->buf, count);
+	mutex_unlock(&call_mutex);
+	return 0;
+}
+
+static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
+	{ .guid_string = DELL_WMI_SMBIOS_GUID },
+	{ },
+};
+
+static void __init parse_b1_table(const struct dmi_header *dm)
+{
+	struct misc_bios_flags_structure *flags =
+	container_of(dm, struct misc_bios_flags_structure, header);
+
+	/* 4 bytes header, 8 bytes flags */
+	if (dm->length < 12)
+		return;
+	if (dm->handle != 0xb100)
+		return;
+	if ((flags->flags0 & FLAG_HAS_ACPI_WMI))
+		wmi_supported = 1;
+}
+
+static void __init find_b1(const struct dmi_header *dm, void *dummy)
+{
+	switch (dm->type) {
+	case 0xb1: /* misc bios flags */
+		parse_b1_table(dm);
+		break;
+	}
+}
+
+static struct wmi_driver dell_smbios_wmi_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 init_dell_smbios_wmi(void)
+{
+	dmi_walk(find_b1, NULL);
+
+	if (!wmi_supported)
+		return -ENODEV;
+
+	return wmi_driver_register(&dell_smbios_wmi_driver);
+}
+
+static void __exit exit_dell_smbios_wmi(void)
+{
+	wmi_driver_unregister(&dell_smbios_wmi_driver);
+}
+
+module_init(init_dell_smbios_wmi);
+module_exit(exit_dell_smbios_wmi);
+
+MODULE_ALIAS("wmi:" DELL_WMI_SMBIOS_GUID);
+MODULE_AUTHOR("Mario Limonciello <mario.limonciello@dell.com>");
+MODULE_DESCRIPTION("Dell SMBIOS communications over WMI");
+MODULE_LICENSE("GPL");
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 12/16] platform/x86: dell-smbios-smm: test for WSMT
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (10 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 11/16] platform/x86: dell-smbios-wmi: Add new WMI dispatcher driver Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 13/16] platform/x86: dell-smbios: Add filtering support Mario Limonciello
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

WSMT is as an attestation to the OS that the platform won't
modify memory outside of pre-defined areas.

If a platform has WSMT enabled in BIOS setup, SMM calls through
dcdbas will fail.  The only way to access platform data in these
instances is through the WMI SMBIOS calling interface.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
---
 drivers/platform/x86/dell-smbios-smm.c | 33 +++++++++++++++++++++++++++++++++
 drivers/platform/x86/dell-smbios.h     |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/platform/x86/dell-smbios-smm.c b/drivers/platform/x86/dell-smbios-smm.c
index 53eabb14fb48..89f65c4651a0 100644
--- a/drivers/platform/x86/dell-smbios-smm.c
+++ b/drivers/platform/x86/dell-smbios-smm.c
@@ -102,6 +102,32 @@ int dell_smbios_smm_call(struct calling_interface_buffer *input)
 	return 0;
 }
 
+/* When enabled this indicates that SMM won't work */
+static bool test_wsmt_enabled(void)
+{
+	struct calling_interface_token *wsmt;
+
+	/* if token doesn't exist, SMM will work */
+	wsmt = dell_smbios_find_token(WSMT_EN_TOKEN);
+	if (!wsmt)
+		return false;
+
+	/* If token exists, try to access over SMM but set a dummy return.
+	 * - If WSMT disabled it will be overwritten by SMM
+	 * - If WSMT enabled then dummy value will remain
+	 */
+	buffer->cmd_class = CLASS_TOKEN_READ;
+	buffer->cmd_select = SELECT_TOKEN_STD;
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+	buffer->input[0] = wsmt->location;
+	buffer->output[0] = 99;
+	dell_smbios_smm_call(buffer);
+	if (buffer->output[0] == 99)
+		return true;
+
+	return false;
+}
+
 static int __init dell_smbios_smm_init(void)
 {
 	int ret;
@@ -115,6 +141,12 @@ static int __init dell_smbios_smm_init(void)
 
 	dmi_walk(find_cmd_address, NULL);
 
+	if (test_wsmt_enabled()) {
+		pr_debug("Disabling due to WSMT enabled\n");
+		ret = -ENODEV;
+		goto fail_wsmt;
+	}
+
 	platform_device = platform_device_alloc("dell-smbios", 1);
 	if (!platform_device) {
 		ret = -ENOMEM;
@@ -138,6 +170,7 @@ static int __init dell_smbios_smm_init(void)
 fail_platform_device_add:
 	platform_device_put(platform_device);
 
+fail_wsmt:
 fail_platform_device_alloc:
 	free_page((unsigned long)buffer);
 	return ret;
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index b3179f5b326b..07effdc7ae8b 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -44,6 +44,8 @@
 #define KBD_LED_AUTO_100_TOKEN	0x02F6
 #define GLOBAL_MIC_MUTE_ENABLE	0x0364
 #define GLOBAL_MIC_MUTE_DISABLE	0x0365
+#define WSMT_EN_TOKEN		0x04EC
+#define WSMT_DIS_TOKEN		0x04ED
 
 struct notifier_block;
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 13/16] platform/x86: dell-smbios: Add filtering support
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (11 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 12/16] platform/x86: dell-smbios-smm: test for WSMT Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 14/16] platform/x86: wmi: create userspace interface for drivers Mario Limonciello
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

When a userspace interface is introduced to dell-smbios filtering
support will be used to make sure that userspace doesn't make calls
deemed unsafe or that can cause the kernel drivers to get out of
sync.

A blacklist is provided for the following:
- Items that are in use by other kernel drivers
- Items that are deemed unsafe (diagnostics, write-once, etc)
- Any items in the blacklist will be rejected.

Following that a whitelist is provided as follows:
- Each item has an associated capability.  If a userspace interface
  accesses this item, that capability will be tested to filter
  the request.
- If the process provides CAP_SYS_RAWIO the whitelist will be
  overridden.

When an item is not in the blacklist, or whitelist and the process
is run with insufficient capabilities the call will be rejected.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
---
 drivers/platform/x86/dell-smbios.c | 192 +++++++++++++++++++++++++++++++++++++
 drivers/platform/x86/dell-smbios.h |  11 +++
 2 files changed, 203 insertions(+)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 2229d44cb92c..d99edd803c19 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include "dell-smbios.h"
 
+static u32 da_supported_commands;
 static int da_num_tokens;
 static struct platform_device *platform_device;
 static struct calling_interface_token *da_tokens;
@@ -38,6 +39,91 @@ struct smbios_device {
 	int (*call_fn)(struct calling_interface_buffer *);
 };
 
+struct smbios_call {
+	u32 need_capability;
+	int cmd_class;
+	int cmd_select;
+};
+
+/* calls that are whitelisted for given capabilities */
+static struct smbios_call call_whitelist[] = {
+	/* generally tokens are allowed, but may be further filtered or
+	 * restricted by token blacklist or whitelist
+	 */
+	{CAP_SYS_ADMIN,	CLASS_TOKEN_READ,	SELECT_TOKEN_STD},
+	{CAP_SYS_ADMIN,	CLASS_TOKEN_READ,	SELECT_TOKEN_AC},
+	{CAP_SYS_ADMIN,	CLASS_TOKEN_READ,	SELECT_TOKEN_BAT},
+	{CAP_SYS_ADMIN,	CLASS_TOKEN_WRITE,	SELECT_TOKEN_STD},
+	{CAP_SYS_ADMIN,	CLASS_TOKEN_WRITE,	SELECT_TOKEN_AC},
+	{CAP_SYS_ADMIN,	CLASS_TOKEN_WRITE,	SELECT_TOKEN_BAT},
+	/* used by userspace: fwupdate */
+	{CAP_SYS_ADMIN, CLASS_ADMIN_PROP,	SELECT_ADMIN_PROP},
+	/* used by userspace: fwupd */
+	{CAP_SYS_ADMIN,	CLASS_INFO,		SELECT_DOCK},
+	{CAP_SYS_ADMIN,	CLASS_FLASH_INTERFACE,	SELECT_FLASH_INTERFACE},
+};
+
+/* calls that are explicitly blacklisted */
+static struct smbios_call call_blacklist[] = {
+	{0x0000, 01, 07}, /* manufacturing use */
+	{0x0000, 06, 05}, /* manufacturing use */
+	{0x0000, 11, 03}, /* write once */
+	{0x0000, 11, 07}, /* write once */
+	{0x0000, 11, 11}, /* write once */
+	{0x0000, 19, -1}, /* diagnostics */
+	/* handled by kernel: dell-laptop */
+	{0x0000, CLASS_INFO, SELECT_RFKILL},
+	{0x0000, CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT},
+};
+
+struct token_range {
+	u32 need_capability;
+	u16 min;
+	u16 max;
+};
+
+/* tokens that are whitelisted for given capabilities */
+static struct token_range token_whitelist[] = {
+	/* used by userspace: fwupdate */
+	{CAP_SYS_ADMIN,	CAPSULE_EN_TOKEN,	CAPSULE_DIS_TOKEN},
+	/* can indicate to userspace that WMI is needed */
+	{0x0000,	WSMT_EN_TOKEN,		WSMT_DIS_TOKEN}
+};
+
+/* tokens that are explicitly blacklisted */
+static struct token_range token_blacklist[] = {
+	{0x0000, 0x0058, 0x0059}, /* ME use */
+	{0x0000, 0x00CD, 0x00D0}, /* raid shadow copy */
+	{0x0000, 0x013A, 0x01FF}, /* sata shadow copy */
+	{0x0000, 0x0175, 0x0176}, /* write once */
+	{0x0000, 0x0195, 0x0197}, /* diagnostics */
+	{0x0000, 0x01DC, 0x01DD}, /* manufacturing use */
+	{0x0000, 0x027D, 0x0284}, /* diagnostics */
+	{0x0000, 0x02E3, 0x02E3}, /* manufacturing use */
+	{0x0000, 0x02FF, 0x02FF}, /* manufacturing use */
+	{0x0000, 0x0300, 0x0302}, /* manufacturing use */
+	{0x0000, 0x0325, 0x0326}, /* manufacturing use */
+	{0x0000, 0x0332, 0x0335}, /* fan control */
+	{0x0000, 0x0350, 0x0350}, /* manufacturing use */
+	{0x0000, 0x0363, 0x0363}, /* manufacturing use */
+	{0x0000, 0x0368, 0x0368}, /* manufacturing use */
+	{0x0000, 0x03F6, 0x03F7}, /* manufacturing use */
+	{0x0000, 0x049E, 0x049F}, /* manufacturing use */
+	{0x0000, 0x04A0, 0x04A3}, /* disagnostics */
+	{0x0000, 0x04E6, 0x04E7}, /* manufacturing use */
+	{0x0000, 0x4000, 0x7FFF}, /* internal BIOS use */
+	{0x0000, 0x9000, 0x9001}, /* internal BIOS use */
+	{0x0000, 0xA000, 0xBFFF}, /* write only */
+	{0x0000, 0xEFF0, 0xEFFF}, /* internal BIOS use */
+	/* handled by kernel: dell-laptop */
+	{0x0000, BRIGHTNESS_TOKEN,	BRIGHTNESS_TOKEN},
+	{0x0000, KBD_LED_OFF_TOKEN,	KBD_LED_AUTO_TOKEN},
+	{0x0000, KBD_LED_AC_TOKEN,	KBD_LED_AC_TOKEN},
+	{0x0000, KBD_LED_AUTO_25_TOKEN,	KBD_LED_AUTO_75_TOKEN},
+	{0x0000, KBD_LED_AUTO_100_TOKEN,	KBD_LED_AUTO_100_TOKEN},
+	{0x0000, GLOBAL_MIC_MUTE_ENABLE,	GLOBAL_MIC_MUTE_DISABLE},
+};
+
 static LIST_HEAD(smbios_device_list);
 
 int dell_smbios_error(int value)
@@ -90,6 +176,110 @@ void dell_smbios_unregister_device(struct device *d)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_unregister_device);
 
+int dell_smbios_call_filter(struct device *d,
+			    struct calling_interface_buffer *buffer)
+{
+	u16 t = 0;
+	int i;
+
+	/* can't make calls over 30 */
+	if (buffer->cmd_class > 30) {
+		dev_dbg(d, "class too big: %u\n", buffer->cmd_class);
+		return -EINVAL;
+	}
+
+	/* supported calls on the particular system */
+	if (!(da_supported_commands & (1 << buffer->cmd_class))) {
+		dev_dbg(d, "invalid command, supported commands: 0x%8x\n",
+			da_supported_commands);
+		return -EINVAL;
+	}
+
+	/* match against call blacklist  */
+	for (i = 0; i < ARRAY_SIZE(call_blacklist); i++) {
+		if (buffer->cmd_class != call_blacklist[i].cmd_class)
+			continue;
+		if (buffer->cmd_select != call_blacklist[i].cmd_select &&
+		    call_blacklist[i].cmd_select != -1)
+			continue;
+		dev_dbg(d, "blacklisted command: %u/%u\n",
+			buffer->cmd_class, buffer->cmd_select);
+		return -EINVAL;
+	}
+
+	/* if a token call, find token ID */
+
+	if ((buffer->cmd_class == CLASS_TOKEN_READ ||
+	     buffer->cmd_class == CLASS_TOKEN_WRITE) &&
+	     buffer->cmd_select < 3) {
+		/* find the matching token ID */
+		for (i = 0; i < da_num_tokens; i++) {
+			if (da_tokens[i].location != buffer->input[0])
+				continue;
+			t = da_tokens[i].tokenID;
+			break;
+		}
+
+		/* token call; but token didn't exist */
+		if (!t) {
+			dev_dbg(d, "token at location %04x doesn't exist\n",
+				buffer->input[0]);
+			return -EINVAL;
+		}
+
+		/* match against token blacklist */
+		for (i = 0; i < ARRAY_SIZE(token_blacklist); i++) {
+			if (!token_blacklist[i].min || !token_blacklist[i].max)
+				continue;
+			if (t >= token_blacklist[i].min &&
+			    t <= token_blacklist[i].max)
+				return -EINVAL;
+		}
+
+		/* match against token whitelist */
+		for (i = 0; i < ARRAY_SIZE(token_whitelist); i++) {
+			if (!token_whitelist[i].min || !token_whitelist[i].max)
+				continue;
+			if (t < token_whitelist[i].min ||
+			    t > token_whitelist[i].max)
+				continue;
+			if (!token_whitelist[i].need_capability ||
+			    capable(token_whitelist[i].need_capability)) {
+				dev_dbg(d, "whitelisted token: %x\n", t);
+				return 0;
+			}
+
+		}
+	}
+	/* match against call whitelist */
+	for (i = 0; i < ARRAY_SIZE(call_whitelist); i++) {
+		if (buffer->cmd_class != call_whitelist[i].cmd_class)
+			continue;
+		if (buffer->cmd_select != call_whitelist[i].cmd_select)
+			continue;
+		if (!call_whitelist[i].need_capability ||
+		    capable(call_whitelist[i].need_capability)) {
+			dev_dbg(d, "whitelisted capable command: %u/%u\n",
+			buffer->cmd_class, buffer->cmd_select);
+			return 0;
+		}
+		dev_dbg(d, "missing capability %d for %u/%u\n",
+			call_whitelist[i].need_capability,
+			buffer->cmd_class, buffer->cmd_select);
+
+	}
+
+	/* not in a whitelist, only allow processes with capabilities */
+	if (capable(CAP_SYS_RAWIO)) {
+		dev_dbg(d, "Allowing %u/%u due to CAP_SYS_RAWIO\n",
+			buffer->cmd_class, buffer->cmd_select);
+		return 0;
+	}
+
+	return -EACCES;
+}
+EXPORT_SYMBOL_GPL(dell_smbios_call_filter);
+
 int dell_smbios_call(struct calling_interface_buffer *buffer)
 {
 	int (*call_fn)(struct calling_interface_buffer *) = NULL;
@@ -168,6 +358,8 @@ static void __init parse_da_table(const struct dmi_header *dm)
 	if (dm->length < 17)
 		return;
 
+	da_supported_commands = table->supportedCmds;
+
 	new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
 				 sizeof(struct calling_interface_token),
 				 GFP_KERNEL);
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 07effdc7ae8b..91e8004d48ba 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -26,9 +26,14 @@
 #define SELECT_TOKEN_AC 2
 #define CLASS_KBD_BACKLIGHT 4
 #define SELECT_KBD_BACKLIGHT 11
+#define CLASS_FLASH_INTERFACE 7
+#define SELECT_FLASH_INTERFACE 3
+#define CLASS_ADMIN_PROP 10
+#define SELECT_ADMIN_PROP 3
 #define CLASS_INFO 17
 #define SELECT_RFKILL 11
 #define SELECT_APP_REGISTRATION	3
+#define SELECT_DOCK 22
 
 /* Tokens used in kernel drivers, any of these
  * should be filtered from userspace access
@@ -44,6 +49,10 @@
 #define KBD_LED_AUTO_100_TOKEN	0x02F6
 #define GLOBAL_MIC_MUTE_ENABLE	0x0364
 #define GLOBAL_MIC_MUTE_DISABLE	0x0365
+
+/* tokens whitelisted to userspace use */
+#define CAPSULE_EN_TOKEN	0x0461
+#define CAPSULE_DIS_TOKEN	0x0462
 #define WSMT_EN_TOKEN		0x04EC
 #define WSMT_DIS_TOKEN		0x04ED
 
@@ -80,6 +89,8 @@ int dell_smbios_register_device(struct device *d, void *call_fn);
 void dell_smbios_unregister_device(struct device *d);
 
 int dell_smbios_error(int value);
+int dell_smbios_call_filter(struct device *d,
+	struct calling_interface_buffer *buffer);
 int dell_smbios_call(struct calling_interface_buffer *buffer);
 
 struct calling_interface_token *dell_smbios_find_token(int tokenid);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 14/16] platform/x86: wmi: create userspace interface for drivers
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (12 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 13/16] platform/x86: dell-smbios: Add filtering support Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-02  0:31   ` Edward O'Callaghan
  2017-11-01 19:25 ` [PATCH v12 15/16] platform/x86: dell-smbios-wmi: introduce userspace interface Mario Limonciello
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

For WMI operations that are only Set or Query readable and writable sysfs
attributes created by WMI vendor drivers or the bus driver makes 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 callback method in the wmi_driver
the WMI bus driver will create a character device that maps to that
function.  This callback method will be responsible for filtering
invalid requests and performing the actual call.

That character device will correspond to this path:
/dev/wmi/$driver

Performing read() on this character device will provide the size
of the buffer that the character device needs to perform calls.
This buffer size can be set by vendor drivers through a new symbol
or when MOF parsing is available by the MOF.

Performing ioctl() on this character device will be interpretd
by the WMI bus driver. It will perform sanity tests for size of
data, test them for a valid instance, copy the data from userspace
and pass iton to the vendor driver to further process and run.

This creates an implicit policy that each driver will only be allowed
a single character device.  If a module matches multiple GUID's,
the wmi_devices will need to be all handled by the same wmi_driver.

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

When a WMI vendor driver is unloaded the WMI bus driver will clean
up the character device and any memory allocated for the call.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 MAINTAINERS                |   1 +
 drivers/platform/x86/wmi.c | 189 ++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/wmi.h        |   5 ++
 include/uapi/linux/wmi.h   |  26 +++++++
 4 files changed, 219 insertions(+), 2 deletions(-)
 create mode 100644 include/uapi/linux/wmi.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 0d7061c05b15..203958d18aec 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -384,6 +384,7 @@ ACPI WMI DRIVER
 L:	platform-driver-x86@vger.kernel.org
 S:	Orphan
 F:	drivers/platform/x86/wmi.c
+F:	include/uapi/linux/wmi.h
 
 AD1889 ALSA SOUND DRIVER
 M:	Thibaut Varene <T-Bone@parisc-linux.org>
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index bcb41c1c7f52..8c31ed4f0e1b 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -38,12 +38,15 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
+#include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/uaccess.h>
 #include <linux/uuid.h>
 #include <linux/wmi.h>
+#include <uapi/linux/wmi.h>
 
 ACPI_MODULE_NAME("wmi");
 MODULE_AUTHOR("Carlos Corbacho");
@@ -69,9 +72,12 @@ struct wmi_block {
 	struct wmi_device dev;
 	struct list_head list;
 	struct guid_block gblock;
+	struct miscdevice char_dev;
+	struct mutex char_mutex;
 	struct acpi_device *acpi_device;
 	wmi_notify_handler handler;
 	void *handler_data;
+	u64 req_buf_size;
 
 	bool read_takes_no_args;
 };
@@ -188,6 +194,25 @@ static acpi_status wmi_method_enable(struct wmi_block *wblock, int enable)
 /*
  * Exported WMI functions
  */
+
+/**
+ * set_required_buffer_size - Sets the buffer size needed for performing IOCTL
+ * @wdev: A wmi bus device from a driver
+ * @instance: Instance index
+ *
+ * Allocates memory needed for buffer, stores the buffer size in that memory
+ */
+int set_required_buffer_size(struct wmi_device *wdev, u64 length)
+{
+	struct wmi_block *wblock;
+
+	wblock = container_of(wdev, struct wmi_block, dev);
+	wblock->req_buf_size = length;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(set_required_buffer_size);
+
 /**
  * wmi_evaluate_method - Evaluate a WMI method
  * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
@@ -764,6 +789,111 @@ static int wmi_dev_match(struct device *dev, struct device_driver *driver)
 
 	return 0;
 }
+static int wmi_char_open(struct inode *inode, struct file *filp)
+{
+	const char *driver_name = filp->f_path.dentry->d_iname;
+	struct wmi_block *wblock = NULL;
+	struct wmi_block *next = NULL;
+
+	list_for_each_entry_safe(wblock, next, &wmi_block_list, list) {
+		if (!wblock->dev.dev.driver)
+			continue;
+		if (strcmp(driver_name, wblock->dev.dev.driver->name) == 0) {
+			filp->private_data = wblock;
+			break;
+		}
+	}
+
+	if (!filp->private_data)
+		return -ENODEV;
+
+	return nonseekable_open(inode, filp);
+}
+
+static ssize_t wmi_char_read(struct file *filp, char __user *buffer,
+	size_t length, loff_t *offset)
+{
+	struct wmi_block *wblock = filp->private_data;
+
+	return simple_read_from_buffer(buffer, length, offset,
+				       &wblock->req_buf_size,
+				       sizeof(wblock->req_buf_size));
+}
+
+static long wmi_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	struct wmi_ioctl_buffer __user *input =
+		(struct wmi_ioctl_buffer __user *) arg;
+	struct wmi_block *wblock = filp->private_data;
+	struct wmi_ioctl_buffer *buf = NULL;
+	struct wmi_driver *wdriver = NULL;
+	int ret;
+
+	if (_IOC_TYPE(cmd) != WMI_IOC)
+		return -ENOTTY;
+
+	/* make sure we're not calling a higher instance than exists*/
+	if (_IOC_NR(cmd) >= wblock->gblock.instance_count)
+		return -EINVAL;
+
+	mutex_lock(&wblock->char_mutex);
+	buf = wblock->handler_data;
+	if (get_user(buf->length, &input->length)) {
+		dev_dbg(&wblock->dev.dev, "Read length from user failed\n");
+		ret = -EFAULT;
+		goto out_ioctl;
+	}
+	/* if it's too small, abort */
+	if (buf->length < wblock->req_buf_size) {
+		dev_err(&wblock->dev.dev,
+			"Buffer %lld too small, need at least %lld\n",
+			buf->length, wblock->req_buf_size);
+		ret = -EINVAL;
+		goto out_ioctl;
+	}
+	/* if it's too big, warn, driver will only use what is needed */
+	if (buf->length > wblock->req_buf_size)
+		dev_warn(&wblock->dev.dev,
+			"Buffer %lld is bigger than required %lld\n",
+			buf->length, wblock->req_buf_size);
+
+	/* copy the structure from userspace */
+	if (copy_from_user(buf, input, wblock->req_buf_size)) {
+		dev_dbg(&wblock->dev.dev, "Copy %llu from user failed\n",
+			wblock->req_buf_size);
+		ret = -EFAULT;
+		goto out_ioctl;
+	}
+
+	/* let the driver do any filtering and do the call */
+	wdriver = container_of(wblock->dev.dev.driver,
+			       struct wmi_driver, driver);
+	if (!try_module_get(wdriver->driver.owner))
+		return -EBUSY;
+	ret = wdriver->filter_callback(&wblock->dev, cmd, buf);
+	module_put(wdriver->driver.owner);
+	if (ret)
+		goto out_ioctl;
+
+	/* return the result (only up to our internal buffer size) */
+	if (copy_to_user(input, buf, wblock->req_buf_size)) {
+		dev_dbg(&wblock->dev.dev, "Copy %llu to user failed\n",
+			wblock->req_buf_size);
+		ret = -EFAULT;
+	}
+
+out_ioctl:
+	mutex_unlock(&wblock->char_mutex);
+	return ret;
+}
+
+static const struct file_operations wmi_fops = {
+	.owner		= THIS_MODULE,
+	.read		= wmi_char_read,
+	.open		= wmi_char_open,
+	.unlocked_ioctl	= wmi_ioctl,
+	.compat_ioctl	= wmi_ioctl,
+};
 
 static int wmi_dev_probe(struct device *dev)
 {
@@ -771,16 +901,63 @@ static int wmi_dev_probe(struct device *dev)
 	struct wmi_driver *wdriver =
 		container_of(dev->driver, struct wmi_driver, driver);
 	int ret = 0;
+	int count;
+	char *buf;
 
 	if (ACPI_FAILURE(wmi_method_enable(wblock, 1)))
 		dev_warn(dev, "failed to enable device -- probing anyway\n");
 
 	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");
+		if (ret != 0)
+			goto probe_failure;
 	}
 
+	/* driver wants a character device made */
+	if (wdriver->filter_callback) {
+		/* check that required buffer size declared by driver or MOF */
+		if (!wblock->req_buf_size) {
+			dev_err(&wblock->dev.dev,
+				"Required buffer size not set\n");
+			ret = -EINVAL;
+			goto probe_failure;
+		}
+
+		count = get_order(wblock->req_buf_size);
+		wblock->handler_data = (void *)__get_free_pages(GFP_KERNEL,
+								count);
+		if (!wblock->handler_data) {
+			ret = -ENOMEM;
+			goto probe_failure;
+		}
+
+		buf = kmalloc(strlen(wdriver->driver.name) + 4, GFP_KERNEL);
+		if (!buf) {
+			ret = -ENOMEM;
+			goto probe_string_failure;
+		}
+		sprintf(buf, "wmi/%s", wdriver->driver.name);
+		wblock->char_dev.minor = MISC_DYNAMIC_MINOR;
+		wblock->char_dev.name = buf;
+		wblock->char_dev.fops = &wmi_fops;
+		wblock->char_dev.mode = 0444;
+		ret = misc_register(&wblock->char_dev);
+		if (ret) {
+			dev_warn(dev, "failed to register char dev: %d", ret);
+			ret = -ENOMEM;
+			goto probe_misc_failure;
+		}
+	}
+
+	return 0;
+
+probe_misc_failure:
+	kfree(buf);
+probe_string_failure:
+	kfree(wblock->handler_data);
+probe_failure:
+	if (ACPI_FAILURE(wmi_method_enable(wblock, 0)))
+		dev_warn(dev, "failed to disable device\n");
 	return ret;
 }
 
@@ -791,6 +968,13 @@ static int wmi_dev_remove(struct device *dev)
 		container_of(dev->driver, struct wmi_driver, driver);
 	int ret = 0;
 
+	if (wdriver->filter_callback) {
+		misc_deregister(&wblock->char_dev);
+		kfree(wblock->char_dev.name);
+		free_pages((unsigned long)wblock->handler_data,
+			   get_order(wblock->req_buf_size));
+	}
+
 	if (wdriver->remove)
 		ret = wdriver->remove(dev_to_wdev(dev));
 
@@ -847,6 +1031,7 @@ static int wmi_create_device(struct device *wmi_bus_dev,
 
 	if (gblock->flags & ACPI_WMI_METHOD) {
 		wblock->dev.dev.type = &wmi_type_method;
+		mutex_init(&wblock->char_mutex);
 		goto out_init;
 	}
 
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index ddee427e0721..4757cb5077e5 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -18,6 +18,7 @@
 
 #include <linux/device.h>
 #include <linux/acpi.h>
+#include <uapi/linux/wmi.h>
 
 struct wmi_device {
 	struct device dev;
@@ -36,6 +37,8 @@ extern acpi_status wmidev_evaluate_method(struct wmi_device *wdev,
 extern union acpi_object *wmidev_block_query(struct wmi_device *wdev,
 					     u8 instance);
 
+extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);
+
 struct wmi_device_id {
 	const char *guid_string;
 };
@@ -47,6 +50,8 @@ struct wmi_driver {
 	int (*probe)(struct wmi_device *wdev);
 	int (*remove)(struct wmi_device *wdev);
 	void (*notify)(struct wmi_device *device, union acpi_object *data);
+	long (*filter_callback)(struct wmi_device *wdev, unsigned int cmd,
+				struct wmi_ioctl_buffer *arg);
 };
 
 extern int __must_check __wmi_driver_register(struct wmi_driver *driver,
diff --git a/include/uapi/linux/wmi.h b/include/uapi/linux/wmi.h
new file mode 100644
index 000000000000..7e52350ac9b3
--- /dev/null
+++ b/include/uapi/linux/wmi.h
@@ -0,0 +1,26 @@
+/*
+ *  User API methods for ACPI-WMI mapping driver
+ *
+ *  Copyright (C) 2017 Dell, Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+#ifndef _UAPI_LINUX_WMI_H
+#define _UAPI_LINUX_WMI_H
+
+#include <linux/types.h>
+
+/* WMI bus will filter all WMI vendor driver requests through this IOC */
+#define WMI_IOC 'W'
+
+/* All ioctl requests through WMI should declare their size followed by
+ * relevant data objects
+ */
+struct wmi_ioctl_buffer {
+	__u64	length;
+	__u8	data[];
+};
+
+#endif
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 15/16] platform/x86: dell-smbios-wmi: introduce userspace interface
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (13 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 14/16] platform/x86: wmi: create userspace interface for drivers Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-01 19:25 ` [PATCH v12 16/16] tools/wmi: add a sample for dell smbios communication over WMI Mario Limonciello
  2017-11-03  0:50 ` [PATCH v12 00/16] Introduce support for Dell SMBIOS " Darren Hart
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

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.

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

It provides an ioctl that will allow passing the WMI calling
interface buffer between userspace and kernel space.

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

To perform an SMBIOS IOCTL call using the character device userspace will
perform a read() on the the character device.  The WMI bus will provide
a u64 variable containing the necessary size of the IOCTL buffer.

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

Not all userspace requests will be accepted.  The dell-smbios filtering
functionality will be used to prevent access to certain tokens and calls.

All whitelisted commands and tokens are now shared out to userspace so
applications don't need to define them in their own headers.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
---
 Documentation/ABI/testing/dell-smbios-wmi | 41 +++++++++++++++++++++++
 drivers/platform/x86/dell-smbios-wmi.c    | 54 ++++++++++++++++++++++++-------
 drivers/platform/x86/dell-smbios.h        | 32 ++----------------
 include/uapi/linux/wmi.h                  | 47 +++++++++++++++++++++++++++
 4 files changed, 133 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/ABI/testing/dell-smbios-wmi

diff --git a/Documentation/ABI/testing/dell-smbios-wmi b/Documentation/ABI/testing/dell-smbios-wmi
new file mode 100644
index 000000000000..fc919ce16008
--- /dev/null
+++ b/Documentation/ABI/testing/dell-smbios-wmi
@@ -0,0 +1,41 @@
+What:		/dev/wmi/dell-smbios
+Date:		November 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/wmi.h>
+
+		1) To perform an SMBIOS call from userspace, you'll need to
+		first determine the minimum size of the calling interface
+		buffer for your machine.
+		Platforms that contain larger buffers can return larger
+		objects from the system firmware.
+		Commonly this size is either 4k or 32k.
+
+		To determine the size of the buffer read() a u64 dword from
+		the WMI character device /dev/wmi/dell-smbios.
+
+		2) After you've determined the minimum size of the calling
+		interface buffer, you can allocate a structure that represents
+		the structure documented above.
+
+		3) In the 'length' object store the size of the buffer you
+		determined above and allocated.
+
+		4) In this buffer object, prepare as necessary for the SMBIOS
+		call you're interested in.  Typically SMBIOS buffers have
+		"class", "select", and "input" defined to values that coincide
+		with the data you are interested in.
+		Documenting class/select/input values is outside of the scope
+		of this documentation. Check with the libsmbios project for
+		further documentation on these values.
+
+		6) Run the call by using ioctl() as described in the header.
+
+		7) The output will be returned in the buffer object.
+
+		8) Be sure to free up your allocated object.
diff --git a/drivers/platform/x86/dell-smbios-wmi.c b/drivers/platform/x86/dell-smbios-wmi.c
index b31f457e58c3..35c13815b24c 100644
--- a/drivers/platform/x86/dell-smbios-wmi.c
+++ b/drivers/platform/x86/dell-smbios-wmi.c
@@ -30,17 +30,6 @@ struct misc_bios_flags_structure {
 
 #define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
 
-struct dell_wmi_extensions {
-	__u32 argattrib;
-	__u32 blength;
-	__u8 data[];
-} __packed;
-
-struct dell_wmi_smbios_buffer {
-	struct calling_interface_buffer std;
-	struct dell_wmi_extensions ext;
-} __packed;
-
 struct wmi_smbios_priv {
 	struct dell_wmi_smbios_buffer *buf;
 	struct list_head list;
@@ -117,6 +106,42 @@ int dell_smbios_wmi_call(struct calling_interface_buffer *buffer)
 	return ret;
 }
 
+static long dell_smbios_wmi_filter(struct wmi_device *wdev, unsigned int cmd,
+				   struct wmi_ioctl_buffer *arg)
+{
+	struct wmi_smbios_priv *priv;
+	int ret = 0;
+
+	switch (cmd) {
+	case DELL_WMI_SMBIOS_CMD:
+		mutex_lock(&call_mutex);
+		priv = dev_get_drvdata(&wdev->dev);
+		if (!priv) {
+			ret = -ENODEV;
+			goto fail_smbios_cmd;
+		}
+		memcpy(priv->buf, arg, priv->req_buf_size);
+		if (dell_smbios_call_filter(&wdev->dev, &priv->buf->std)) {
+			dev_err(&wdev->dev, "Invalid call %d/%d:%8x\n",
+				priv->buf->std.cmd_class,
+				priv->buf->std.cmd_select,
+				priv->buf->std.input[0]);
+			ret = -EFAULT;
+			goto fail_smbios_cmd;
+		}
+		ret = run_smbios_call(priv->wdev);
+		if (ret)
+			goto fail_smbios_cmd;
+		memcpy(arg, priv->buf, priv->req_buf_size);
+fail_smbios_cmd:
+		mutex_unlock(&call_mutex);
+		break;
+	default:
+		ret = -ENOIOCTLCMD;
+	}
+	return ret;
+}
+
 static int dell_smbios_wmi_probe(struct wmi_device *wdev)
 {
 	struct wmi_smbios_priv *priv;
@@ -135,6 +160,12 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev)
 	if (!dell_wmi_get_size(&priv->req_buf_size))
 		return -EPROBE_DEFER;
 
+	/* add in the length object we will use internally with ioctl */
+	priv->req_buf_size += sizeof(u64);
+	ret = set_required_buffer_size(wdev, priv->req_buf_size);
+	if (ret)
+		return ret;
+
 	count = get_order(priv->req_buf_size);
 	priv->buf = (void *)__get_free_pages(GFP_KERNEL, count);
 	if (!priv->buf)
@@ -210,6 +241,7 @@ static struct wmi_driver dell_smbios_wmi_driver = {
 	.probe = dell_smbios_wmi_probe,
 	.remove = dell_smbios_wmi_remove,
 	.id_table = dell_smbios_wmi_id_table,
+	.filter_callback = dell_smbios_wmi_filter,
 };
 
 static int __init init_dell_smbios_wmi(void)
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 91e8004d48ba..138d478d9adc 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -17,23 +17,11 @@
 #define _DELL_SMBIOS_H_
 
 #include <linux/device.h>
+#include <uapi/linux/wmi.h>
 
-/* Classes and selects used in kernel drivers */
-#define CLASS_TOKEN_READ 0
-#define CLASS_TOKEN_WRITE 1
-#define SELECT_TOKEN_STD 0
-#define SELECT_TOKEN_BAT 1
-#define SELECT_TOKEN_AC 2
+/* Classes and selects used only in kernel drivers */
 #define CLASS_KBD_BACKLIGHT 4
 #define SELECT_KBD_BACKLIGHT 11
-#define CLASS_FLASH_INTERFACE 7
-#define SELECT_FLASH_INTERFACE 3
-#define CLASS_ADMIN_PROP 10
-#define SELECT_ADMIN_PROP 3
-#define CLASS_INFO 17
-#define SELECT_RFKILL 11
-#define SELECT_APP_REGISTRATION	3
-#define SELECT_DOCK 22
 
 /* Tokens used in kernel drivers, any of these
  * should be filtered from userspace access
@@ -50,24 +38,8 @@
 #define GLOBAL_MIC_MUTE_ENABLE	0x0364
 #define GLOBAL_MIC_MUTE_DISABLE	0x0365
 
-/* tokens whitelisted to userspace use */
-#define CAPSULE_EN_TOKEN	0x0461
-#define CAPSULE_DIS_TOKEN	0x0462
-#define WSMT_EN_TOKEN		0x04EC
-#define WSMT_DIS_TOKEN		0x04ED
-
 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 cmd_class;
-	u16 cmd_select;
-	volatile u32 input[4];
-	volatile u32 output[4];
-} __packed;
-
 struct calling_interface_token {
 	u16 tokenID;
 	u16 location;
diff --git a/include/uapi/linux/wmi.h b/include/uapi/linux/wmi.h
index 7e52350ac9b3..7a92e9e3d1c0 100644
--- a/include/uapi/linux/wmi.h
+++ b/include/uapi/linux/wmi.h
@@ -10,6 +10,7 @@
 #ifndef _UAPI_LINUX_WMI_H
 #define _UAPI_LINUX_WMI_H
 
+#include <linux/ioctl.h>
 #include <linux/types.h>
 
 /* WMI bus will filter all WMI vendor driver requests through this IOC */
@@ -23,4 +24,50 @@ struct wmi_ioctl_buffer {
 	__u8	data[];
 };
 
+/* This structure may be modified by the firmware when we enter
+ * system management mode through SMM, hence the volatiles
+ */
+struct calling_interface_buffer {
+	__u16 cmd_class;
+	__u16 cmd_select;
+	volatile __u32 input[4];
+	volatile __u32 output[4];
+} __packed;
+
+struct dell_wmi_extensions {
+	__u32 argattrib;
+	__u32 blength;
+	__u8 data[];
+} __packed;
+
+struct dell_wmi_smbios_buffer {
+	__u64 length;
+	struct calling_interface_buffer std;
+	struct dell_wmi_extensions	ext;
+} __packed;
+
+/* Whitelisted smbios class/select commands */
+#define CLASS_TOKEN_READ	0
+#define CLASS_TOKEN_WRITE	1
+#define SELECT_TOKEN_STD	0
+#define SELECT_TOKEN_BAT	1
+#define SELECT_TOKEN_AC		2
+#define CLASS_FLASH_INTERFACE	7
+#define SELECT_FLASH_INTERFACE	3
+#define CLASS_ADMIN_PROP	10
+#define SELECT_ADMIN_PROP	3
+#define CLASS_INFO		17
+#define SELECT_RFKILL		11
+#define SELECT_APP_REGISTRATION	3
+#define SELECT_DOCK		22
+
+/* whitelisted tokens */
+#define CAPSULE_EN_TOKEN	0x0461
+#define CAPSULE_DIS_TOKEN	0x0462
+#define WSMT_EN_TOKEN		0x04EC
+#define WSMT_DIS_TOKEN		0x04ED
+
+/* Dell SMBIOS calling IOCTL command used by dell-smbios-wmi */
+#define DELL_WMI_SMBIOS_CMD	_IOWR(WMI_IOC, 0, struct dell_wmi_smbios_buffer)
+
 #endif
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v12 16/16] tools/wmi: add a sample for dell smbios communication over WMI
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (14 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 15/16] platform/x86: dell-smbios-wmi: introduce userspace interface Mario Limonciello
@ 2017-11-01 19:25 ` Mario Limonciello
  2017-11-03  0:50 ` [PATCH v12 00/16] Introduce support for Dell SMBIOS " Darren Hart
  16 siblings, 0 replies; 26+ messages in thread
From: Mario Limonciello @ 2017-11-01 19:25 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: LKML, platform-driver-x86, Andy Lutomirski, quasisec, pali.rohar,
	rjw, mjg59, hch, Greg KH, Alan Cox, Mario Limonciello

This application uses the character device /dev/wmi/dell-smbios
to perform SMBIOS communications from userspace.

It offers demonstrations of a few simple tasks:
 - Running a class/select command
 - Querying a token value
 - Activating a token

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
---
 MAINTAINERS                     |   1 +
 tools/Makefile                  |  14 +--
 tools/wmi/Makefile              |  18 ++++
 tools/wmi/dell-smbios-example.c | 210 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 237 insertions(+), 6 deletions(-)
 create mode 100644 tools/wmi/Makefile
 create mode 100644 tools/wmi/dell-smbios-example.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 203958d18aec..f030e1ff5a76 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3997,6 +3997,7 @@ M:	Mario Limonciello <mario.limonciello@dell.com>
 L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
 F:	drivers/platform/x86/dell-smbios-wmi.c
+F:	tools/wmi/dell-smbios-example.c
 
 DELL LAPTOP DRIVER
 M:	Matthew Garrett <mjg59@srcf.ucam.org>
diff --git a/tools/Makefile b/tools/Makefile
index 9dfede37c8ff..9d2fd2606810 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -29,6 +29,7 @@ help:
 	@echo '  usb                    - USB testing tools'
 	@echo '  virtio                 - vhost test module'
 	@echo '  vm                     - misc vm tools'
+	@echo '  wmi			- WMI interface examples'
 	@echo '  x86_energy_perf_policy - Intel energy policy tool'
 	@echo ''
 	@echo 'You can do:'
@@ -57,7 +58,7 @@ acpi: FORCE
 cpupower: FORCE
 	$(call descend,power/$@)
 
-cgroup firewire hv guest spi usb virtio vm net iio gpio objtool leds: FORCE
+cgroup firewire hv guest spi usb virtio vm net iio gpio objtool leds wmi: FORCE
 	$(call descend,$@)
 
 liblockdep: FORCE
@@ -92,7 +93,7 @@ kvm_stat: FORCE
 all: acpi cgroup cpupower gpio hv firewire liblockdep \
 		perf selftests spi turbostat usb \
 		virtio vm net x86_energy_perf_policy \
-		tmon freefall iio objtool kvm_stat
+		tmon freefall iio objtool kvm_stat wmi
 
 acpi_install:
 	$(call descend,power/$(@:_install=),install)
@@ -100,7 +101,7 @@ acpi_install:
 cpupower_install:
 	$(call descend,power/$(@:_install=),install)
 
-cgroup_install firewire_install gpio_install hv_install iio_install perf_install spi_install usb_install virtio_install vm_install net_install objtool_install:
+cgroup_install firewire_install gpio_install hv_install iio_install perf_install spi_install usb_install virtio_install vm_install net_install objtool_install wmi_install:
 	$(call descend,$(@:_install=),install)
 
 liblockdep_install:
@@ -125,7 +126,8 @@ install: acpi_install cgroup_install cpupower_install gpio_install \
 		hv_install firewire_install iio_install liblockdep_install \
 		perf_install selftests_install turbostat_install usb_install \
 		virtio_install vm_install net_install x86_energy_perf_policy_install \
-		tmon_install freefall_install objtool_install kvm_stat_install
+		tmon_install freefall_install objtool_install kvm_stat_install \
+		wmi_install
 
 acpi_clean:
 	$(call descend,power/acpi,clean)
@@ -133,7 +135,7 @@ acpi_clean:
 cpupower_clean:
 	$(call descend,power/cpupower,clean)
 
-cgroup_clean hv_clean firewire_clean spi_clean usb_clean virtio_clean vm_clean net_clean iio_clean gpio_clean objtool_clean leds_clean:
+cgroup_clean hv_clean firewire_clean spi_clean usb_clean virtio_clean vm_clean wmi_clean net_clean iio_clean gpio_clean objtool_clean leds_clean:
 	$(call descend,$(@:_clean=),clean)
 
 liblockdep_clean:
@@ -171,6 +173,6 @@ clean: acpi_clean cgroup_clean cpupower_clean hv_clean firewire_clean \
 		perf_clean selftests_clean turbostat_clean spi_clean usb_clean virtio_clean \
 		vm_clean net_clean iio_clean x86_energy_perf_policy_clean tmon_clean \
 		freefall_clean build_clean libbpf_clean libsubcmd_clean liblockdep_clean \
-		gpio_clean objtool_clean leds_clean
+		gpio_clean objtool_clean leds_clean wmi_clean
 
 .PHONY: FORCE
diff --git a/tools/wmi/Makefile b/tools/wmi/Makefile
new file mode 100644
index 000000000000..e664f1167388
--- /dev/null
+++ b/tools/wmi/Makefile
@@ -0,0 +1,18 @@
+PREFIX ?= /usr
+SBINDIR ?= sbin
+INSTALL ?= install
+CFLAGS += -D__EXPORTED_HEADERS__ -I../../include/uapi -I../../include
+CC = $(CROSS_COMPILE)gcc
+
+TARGET = dell-smbios-example
+
+all: $(TARGET)
+
+%: %.c
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $<
+
+clean:
+	$(RM) $(TARGET)
+
+install: dell-smbios-example
+	$(INSTALL) -D -m 755 $(TARGET) $(DESTDIR)$(PREFIX)/$(SBINDIR)/$(TARGET)
diff --git a/tools/wmi/dell-smbios-example.c b/tools/wmi/dell-smbios-example.c
new file mode 100644
index 000000000000..9d3bde081249
--- /dev/null
+++ b/tools/wmi/dell-smbios-example.c
@@ -0,0 +1,210 @@
+/*
+ *  Sample application for SMBIOS communication over WMI interface
+ *  Performs the following:
+ *  - Simple cmd_class/cmd_select lookup for TPM information
+ *  - Simple query of known tokens and their values
+ *  - Simple activation of a token
+ *
+ *  Copyright (C) 2017 Dell, Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+
+/* if uapi header isn't installed, this might not yet exist */
+#ifndef __packed
+#define __packed __attribute__((packed))
+#endif
+#include <linux/wmi.h>
+
+/* It would be better to discover these using udev, but for a simple
+ * application they're hardcoded
+ */
+static const char *ioctl_devfs = "/dev/wmi/dell-smbios";
+static const char *token_sysfs =
+			"/sys/bus/platform/devices/dell-smbios.0/tokens";
+
+static void show_buffer(struct dell_wmi_smbios_buffer *buffer)
+{
+	printf("Call: %x/%x [%x,%x,%x,%x]\nResults: [%8x,%8x,%8x,%8x]\n",
+	buffer->std.cmd_class, buffer->std.cmd_select,
+	buffer->std.input[0], buffer->std.input[1],
+	buffer->std.input[2], buffer->std.input[3],
+	buffer->std.output[0], buffer->std.output[1],
+	buffer->std.output[2], buffer->std.output[3]);
+}
+
+static int run_wmi_smbios_cmd(struct dell_wmi_smbios_buffer *buffer)
+{
+	int fd;
+	int ret;
+
+	fd = open(ioctl_devfs, O_NONBLOCK);
+	ret = ioctl(fd, DELL_WMI_SMBIOS_CMD, buffer);
+	close(fd);
+	return ret;
+}
+
+static int find_token(__u16 token, __u16 *location, __u16 *value)
+{
+	char location_sysfs[60];
+	char value_sysfs[57];
+	char buf[4096];
+	FILE *f;
+	int ret;
+
+	ret = sprintf(value_sysfs, "%s/%04x_value", token_sysfs, token);
+	if (ret < 0) {
+		printf("sprintf value failed\n");
+		return 2;
+	}
+	f = fopen(value_sysfs, "rb");
+	if (!f) {
+		printf("failed to open %s\n", value_sysfs);
+		return 2;
+	}
+	fread(buf, 1, 4096, f);
+	fclose(f);
+	*value = (__u16) strtol(buf, NULL, 16);
+
+	ret = sprintf(location_sysfs, "%s/%04x_location", token_sysfs, token);
+	if (ret < 0) {
+		printf("sprintf location failed\n");
+		return 1;
+	}
+	f = fopen(location_sysfs, "rb");
+	if (!f) {
+		printf("failed to open %s\n", location_sysfs);
+		return 2;
+	}
+	fread(buf, 1, 4096, f);
+	fclose(f);
+	*location = (__u16) strtol(buf, NULL, 16);
+
+	if (*location)
+		return 0;
+	return 2;
+}
+
+static int token_is_active(__u16 *location, __u16 *cmpvalue,
+			   struct dell_wmi_smbios_buffer *buffer)
+{
+	int ret;
+
+	buffer->std.cmd_class = CLASS_TOKEN_READ;
+	buffer->std.cmd_select = SELECT_TOKEN_STD;
+	buffer->std.input[0] = *location;
+	ret = run_wmi_smbios_cmd(buffer);
+	if (ret != 0 || buffer->std.output[0] != 0)
+		return ret;
+	ret = (buffer->std.output[1] == *cmpvalue);
+	return ret;
+}
+
+static int query_token(__u16 token, struct dell_wmi_smbios_buffer *buffer)
+{
+	__u16 location;
+	__u16 value;
+	int ret;
+
+	ret = find_token(token, &location, &value);
+	if (ret != 0) {
+		printf("unable to find token %04x\n", token);
+		return 1;
+	}
+	return token_is_active(&location, &value, buffer);
+}
+
+static int activate_token(struct dell_wmi_smbios_buffer *buffer,
+		   __u16 token)
+{
+	__u16 location;
+	__u16 value;
+	int ret;
+
+	ret = find_token(token, &location, &value);
+	if (ret != 0) {
+		printf("unable to find token %04x\n", token);
+		return 1;
+	}
+	buffer->std.cmd_class = CLASS_TOKEN_WRITE;
+	buffer->std.cmd_select = SELECT_TOKEN_STD;
+	buffer->std.input[0] = location;
+	buffer->std.input[1] = 1;
+	ret = run_wmi_smbios_cmd(buffer);
+	return ret;
+}
+
+static int query_buffer_size(__u64 *buffer_size)
+{
+	FILE *f;
+
+	f = fopen(ioctl_devfs, "rb");
+	if (!f)
+		return -EINVAL;
+	fread(buffer_size, sizeof(__u64), 1, f);
+	fclose(f);
+	return EXIT_SUCCESS;
+}
+
+int main(void)
+{
+	struct dell_wmi_smbios_buffer *buffer;
+	int ret;
+	__u64 value = 0;
+
+	ret = query_buffer_size(&value);
+	if (ret == EXIT_FAILURE || !value) {
+		printf("Unable to read buffer size\n");
+		goto out;
+	}
+	printf("Detected required buffer size %lld\n", value);
+
+	buffer = malloc(value);
+	if (buffer == NULL) {
+		printf("failed to alloc memory for ioctl\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+	buffer->length = value;
+
+	/* simple SMBIOS call for looking up TPM info */
+	buffer->std.cmd_class = CLASS_FLASH_INTERFACE;
+	buffer->std.cmd_select = SELECT_FLASH_INTERFACE;
+	buffer->std.input[0] = 2;
+	ret = run_wmi_smbios_cmd(buffer);
+	if (ret) {
+		printf("smbios ioctl failed: %d\n", ret);
+		ret = EXIT_FAILURE;
+		goto out;
+	}
+	show_buffer(buffer);
+
+	/* query some tokens */
+	ret = query_token(CAPSULE_EN_TOKEN, buffer);
+	printf("UEFI Capsule enabled token is: %d\n", ret);
+	ret = query_token(CAPSULE_DIS_TOKEN, buffer);
+	printf("UEFI Capsule disabled token is: %d\n", ret);
+
+	/* activate UEFI capsule token if disabled */
+	if (ret) {
+		printf("Enabling UEFI capsule token");
+		if (activate_token(buffer, CAPSULE_EN_TOKEN)) {
+			printf("activate failed\n");
+			ret = -1;
+			goto out;
+		}
+	}
+	ret = EXIT_SUCCESS;
+out:
+	free(buffer);
+	return ret;
+}
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v12 01/16] platform/x86: dell-smbios: Prefix class/select with cmd_
  2017-11-01 19:25 ` [PATCH v12 01/16] platform/x86: dell-smbios: Prefix class/select with cmd_ Mario Limonciello
@ 2017-11-02  0:30   ` Edward O'Callaghan
  0 siblings, 0 replies; 26+ messages in thread
From: Edward O'Callaghan @ 2017-11-02  0:30 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: dvhart, Andy Shevchenko, LKML, platform-driver-x86,
	Andy Lutomirski, pali.rohar, rjw, Matthew Garrett, hch, Greg KH,
	Alan Cox

Reviewed-by: Edward O'Callaghan <quasisec@google.com>

On Thu, Nov 2, 2017 at 6:25 AM, Mario Limonciello
<mario.limonciello@dell.com> wrote:
> Later on these structures will be brought up to userspace.
> the word "class" is a reserved word in c++ and this will prevent
> uapi headers from being included directly in c++ programs.
>
> To make life easier on these applications, prepare the change now.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> ---
>  drivers/platform/x86/dell-smbios.c | 4 ++--
>  drivers/platform/x86/dell-smbios.h | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
> index e9b1ca07c872..ffc174638aa4 100644
> --- a/drivers/platform/x86/dell-smbios.c
> +++ b/drivers/platform/x86/dell-smbios.c
> @@ -86,8 +86,8 @@ void dell_smbios_send_request(int class, int select)
>         command.ebx = virt_to_phys(buffer);
>         command.ecx = 0x42534931;
>
> -       buffer->class = class;
> -       buffer->select = select;
> +       buffer->cmd_class = class;
> +       buffer->cmd_select = select;
>
>         dcdbas_smi_request(&command);
>  }
> diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
> index 45cbc2292cd3..742dd8bd66b9 100644
> --- a/drivers/platform/x86/dell-smbios.h
> +++ b/drivers/platform/x86/dell-smbios.h
> @@ -22,8 +22,8 @@ struct notifier_block;
>   * system management mode, hence the volatiles */
>
>  struct calling_interface_buffer {
> -       u16 class;
> -       u16 select;
> +       u16 cmd_class;
> +       u16 cmd_select;
>         volatile u32 input[4];
>         volatile u32 output[4];
>  } __packed;
> --
> 2.14.1
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v12 14/16] platform/x86: wmi: create userspace interface for drivers
  2017-11-01 19:25 ` [PATCH v12 14/16] platform/x86: wmi: create userspace interface for drivers Mario Limonciello
@ 2017-11-02  0:31   ` Edward O'Callaghan
  0 siblings, 0 replies; 26+ messages in thread
From: Edward O'Callaghan @ 2017-11-02  0:31 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: dvhart, Andy Shevchenko, LKML, platform-driver-x86,
	Andy Lutomirski, pali.rohar, rjw, Matthew Garrett, hch, Greg KH,
	Alan Cox

Reviewed-by: Edward O'Callaghan <quasisec@google.com>

On Thu, Nov 2, 2017 at 6:25 AM, Mario Limonciello
<mario.limonciello@dell.com> wrote:
> For WMI operations that are only Set or Query readable and writable sysfs
> attributes created by WMI vendor drivers or the bus driver makes 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 callback method in the wmi_driver
> the WMI bus driver will create a character device that maps to that
> function.  This callback method will be responsible for filtering
> invalid requests and performing the actual call.
>
> That character device will correspond to this path:
> /dev/wmi/$driver
>
> Performing read() on this character device will provide the size
> of the buffer that the character device needs to perform calls.
> This buffer size can be set by vendor drivers through a new symbol
> or when MOF parsing is available by the MOF.
>
> Performing ioctl() on this character device will be interpretd
> by the WMI bus driver. It will perform sanity tests for size of
> data, test them for a valid instance, copy the data from userspace
> and pass iton to the vendor driver to further process and run.
>
> This creates an implicit policy that each driver will only be allowed
> a single character device.  If a module matches multiple GUID's,
> the wmi_devices will need to be all handled by the same wmi_driver.
>
> The WMI vendor drivers will be responsible for managing inappropriate
> access to this character device and proper locking on data used by
> it.
>
> When a WMI vendor driver is unloaded the WMI bus driver will clean
> up the character device and any memory allocated for the call.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> ---
>  MAINTAINERS                |   1 +
>  drivers/platform/x86/wmi.c | 189 ++++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/wmi.h        |   5 ++
>  include/uapi/linux/wmi.h   |  26 +++++++
>  4 files changed, 219 insertions(+), 2 deletions(-)
>  create mode 100644 include/uapi/linux/wmi.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0d7061c05b15..203958d18aec 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -384,6 +384,7 @@ ACPI WMI DRIVER
>  L:     platform-driver-x86@vger.kernel.org
>  S:     Orphan
>  F:     drivers/platform/x86/wmi.c
> +F:     include/uapi/linux/wmi.h
>
>  AD1889 ALSA SOUND DRIVER
>  M:     Thibaut Varene <T-Bone@parisc-linux.org>
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index bcb41c1c7f52..8c31ed4f0e1b 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -38,12 +38,15 @@
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/list.h>
> +#include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
> +#include <linux/uaccess.h>
>  #include <linux/uuid.h>
>  #include <linux/wmi.h>
> +#include <uapi/linux/wmi.h>
>
>  ACPI_MODULE_NAME("wmi");
>  MODULE_AUTHOR("Carlos Corbacho");
> @@ -69,9 +72,12 @@ struct wmi_block {
>         struct wmi_device dev;
>         struct list_head list;
>         struct guid_block gblock;
> +       struct miscdevice char_dev;
> +       struct mutex char_mutex;
>         struct acpi_device *acpi_device;
>         wmi_notify_handler handler;
>         void *handler_data;
> +       u64 req_buf_size;
>
>         bool read_takes_no_args;
>  };
> @@ -188,6 +194,25 @@ static acpi_status wmi_method_enable(struct wmi_block *wblock, int enable)
>  /*
>   * Exported WMI functions
>   */
> +
> +/**
> + * set_required_buffer_size - Sets the buffer size needed for performing IOCTL
> + * @wdev: A wmi bus device from a driver
> + * @instance: Instance index
> + *
> + * Allocates memory needed for buffer, stores the buffer size in that memory
> + */
> +int set_required_buffer_size(struct wmi_device *wdev, u64 length)
> +{
> +       struct wmi_block *wblock;
> +
> +       wblock = container_of(wdev, struct wmi_block, dev);
> +       wblock->req_buf_size = length;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(set_required_buffer_size);
> +
>  /**
>   * wmi_evaluate_method - Evaluate a WMI method
>   * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
> @@ -764,6 +789,111 @@ static int wmi_dev_match(struct device *dev, struct device_driver *driver)
>
>         return 0;
>  }
> +static int wmi_char_open(struct inode *inode, struct file *filp)
> +{
> +       const char *driver_name = filp->f_path.dentry->d_iname;
> +       struct wmi_block *wblock = NULL;
> +       struct wmi_block *next = NULL;
> +
> +       list_for_each_entry_safe(wblock, next, &wmi_block_list, list) {
> +               if (!wblock->dev.dev.driver)
> +                       continue;
> +               if (strcmp(driver_name, wblock->dev.dev.driver->name) == 0) {
> +                       filp->private_data = wblock;
> +                       break;
> +               }
> +       }
> +
> +       if (!filp->private_data)
> +               return -ENODEV;
> +
> +       return nonseekable_open(inode, filp);
> +}
> +
> +static ssize_t wmi_char_read(struct file *filp, char __user *buffer,
> +       size_t length, loff_t *offset)
> +{
> +       struct wmi_block *wblock = filp->private_data;
> +
> +       return simple_read_from_buffer(buffer, length, offset,
> +                                      &wblock->req_buf_size,
> +                                      sizeof(wblock->req_buf_size));
> +}
> +
> +static long wmi_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +{
> +       struct wmi_ioctl_buffer __user *input =
> +               (struct wmi_ioctl_buffer __user *) arg;
> +       struct wmi_block *wblock = filp->private_data;
> +       struct wmi_ioctl_buffer *buf = NULL;
> +       struct wmi_driver *wdriver = NULL;
> +       int ret;
> +
> +       if (_IOC_TYPE(cmd) != WMI_IOC)
> +               return -ENOTTY;
> +
> +       /* make sure we're not calling a higher instance than exists*/
> +       if (_IOC_NR(cmd) >= wblock->gblock.instance_count)
> +               return -EINVAL;
> +
> +       mutex_lock(&wblock->char_mutex);
> +       buf = wblock->handler_data;
> +       if (get_user(buf->length, &input->length)) {
> +               dev_dbg(&wblock->dev.dev, "Read length from user failed\n");
> +               ret = -EFAULT;
> +               goto out_ioctl;
> +       }
> +       /* if it's too small, abort */
> +       if (buf->length < wblock->req_buf_size) {
> +               dev_err(&wblock->dev.dev,
> +                       "Buffer %lld too small, need at least %lld\n",
> +                       buf->length, wblock->req_buf_size);
> +               ret = -EINVAL;
> +               goto out_ioctl;
> +       }
> +       /* if it's too big, warn, driver will only use what is needed */
> +       if (buf->length > wblock->req_buf_size)
> +               dev_warn(&wblock->dev.dev,
> +                       "Buffer %lld is bigger than required %lld\n",
> +                       buf->length, wblock->req_buf_size);
> +
> +       /* copy the structure from userspace */
> +       if (copy_from_user(buf, input, wblock->req_buf_size)) {
> +               dev_dbg(&wblock->dev.dev, "Copy %llu from user failed\n",
> +                       wblock->req_buf_size);
> +               ret = -EFAULT;
> +               goto out_ioctl;
> +       }
> +
> +       /* let the driver do any filtering and do the call */
> +       wdriver = container_of(wblock->dev.dev.driver,
> +                              struct wmi_driver, driver);
> +       if (!try_module_get(wdriver->driver.owner))
> +               return -EBUSY;
> +       ret = wdriver->filter_callback(&wblock->dev, cmd, buf);
> +       module_put(wdriver->driver.owner);
> +       if (ret)
> +               goto out_ioctl;
> +
> +       /* return the result (only up to our internal buffer size) */
> +       if (copy_to_user(input, buf, wblock->req_buf_size)) {
> +               dev_dbg(&wblock->dev.dev, "Copy %llu to user failed\n",
> +                       wblock->req_buf_size);
> +               ret = -EFAULT;
> +       }
> +
> +out_ioctl:
> +       mutex_unlock(&wblock->char_mutex);
> +       return ret;
> +}
> +
> +static const struct file_operations wmi_fops = {
> +       .owner          = THIS_MODULE,
> +       .read           = wmi_char_read,
> +       .open           = wmi_char_open,
> +       .unlocked_ioctl = wmi_ioctl,
> +       .compat_ioctl   = wmi_ioctl,
> +};
>
>  static int wmi_dev_probe(struct device *dev)
>  {
> @@ -771,16 +901,63 @@ static int wmi_dev_probe(struct device *dev)
>         struct wmi_driver *wdriver =
>                 container_of(dev->driver, struct wmi_driver, driver);
>         int ret = 0;
> +       int count;
> +       char *buf;
>
>         if (ACPI_FAILURE(wmi_method_enable(wblock, 1)))
>                 dev_warn(dev, "failed to enable device -- probing anyway\n");
>
>         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");
> +               if (ret != 0)
> +                       goto probe_failure;
>         }
>
> +       /* driver wants a character device made */
> +       if (wdriver->filter_callback) {
> +               /* check that required buffer size declared by driver or MOF */
> +               if (!wblock->req_buf_size) {
> +                       dev_err(&wblock->dev.dev,
> +                               "Required buffer size not set\n");
> +                       ret = -EINVAL;
> +                       goto probe_failure;
> +               }
> +
> +               count = get_order(wblock->req_buf_size);
> +               wblock->handler_data = (void *)__get_free_pages(GFP_KERNEL,
> +                                                               count);
> +               if (!wblock->handler_data) {
> +                       ret = -ENOMEM;
> +                       goto probe_failure;
> +               }
> +
> +               buf = kmalloc(strlen(wdriver->driver.name) + 4, GFP_KERNEL);
> +               if (!buf) {
> +                       ret = -ENOMEM;
> +                       goto probe_string_failure;
> +               }
> +               sprintf(buf, "wmi/%s", wdriver->driver.name);
> +               wblock->char_dev.minor = MISC_DYNAMIC_MINOR;
> +               wblock->char_dev.name = buf;
> +               wblock->char_dev.fops = &wmi_fops;
> +               wblock->char_dev.mode = 0444;
> +               ret = misc_register(&wblock->char_dev);
> +               if (ret) {
> +                       dev_warn(dev, "failed to register char dev: %d", ret);
> +                       ret = -ENOMEM;
> +                       goto probe_misc_failure;
> +               }
> +       }
> +
> +       return 0;
> +
> +probe_misc_failure:
> +       kfree(buf);
> +probe_string_failure:
> +       kfree(wblock->handler_data);
> +probe_failure:
> +       if (ACPI_FAILURE(wmi_method_enable(wblock, 0)))
> +               dev_warn(dev, "failed to disable device\n");
>         return ret;
>  }
>
> @@ -791,6 +968,13 @@ static int wmi_dev_remove(struct device *dev)
>                 container_of(dev->driver, struct wmi_driver, driver);
>         int ret = 0;
>
> +       if (wdriver->filter_callback) {
> +               misc_deregister(&wblock->char_dev);
> +               kfree(wblock->char_dev.name);
> +               free_pages((unsigned long)wblock->handler_data,
> +                          get_order(wblock->req_buf_size));
> +       }
> +
>         if (wdriver->remove)
>                 ret = wdriver->remove(dev_to_wdev(dev));
>
> @@ -847,6 +1031,7 @@ static int wmi_create_device(struct device *wmi_bus_dev,
>
>         if (gblock->flags & ACPI_WMI_METHOD) {
>                 wblock->dev.dev.type = &wmi_type_method;
> +               mutex_init(&wblock->char_mutex);
>                 goto out_init;
>         }
>
> diff --git a/include/linux/wmi.h b/include/linux/wmi.h
> index ddee427e0721..4757cb5077e5 100644
> --- a/include/linux/wmi.h
> +++ b/include/linux/wmi.h
> @@ -18,6 +18,7 @@
>
>  #include <linux/device.h>
>  #include <linux/acpi.h>
> +#include <uapi/linux/wmi.h>
>
>  struct wmi_device {
>         struct device dev;
> @@ -36,6 +37,8 @@ extern acpi_status wmidev_evaluate_method(struct wmi_device *wdev,
>  extern union acpi_object *wmidev_block_query(struct wmi_device *wdev,
>                                              u8 instance);
>
> +extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);
> +
>  struct wmi_device_id {
>         const char *guid_string;
>  };
> @@ -47,6 +50,8 @@ struct wmi_driver {
>         int (*probe)(struct wmi_device *wdev);
>         int (*remove)(struct wmi_device *wdev);
>         void (*notify)(struct wmi_device *device, union acpi_object *data);
> +       long (*filter_callback)(struct wmi_device *wdev, unsigned int cmd,
> +                               struct wmi_ioctl_buffer *arg);
>  };
>
>  extern int __must_check __wmi_driver_register(struct wmi_driver *driver,
> diff --git a/include/uapi/linux/wmi.h b/include/uapi/linux/wmi.h
> new file mode 100644
> index 000000000000..7e52350ac9b3
> --- /dev/null
> +++ b/include/uapi/linux/wmi.h
> @@ -0,0 +1,26 @@
> +/*
> + *  User API methods for ACPI-WMI mapping driver
> + *
> + *  Copyright (C) 2017 Dell, Inc.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + */
> +#ifndef _UAPI_LINUX_WMI_H
> +#define _UAPI_LINUX_WMI_H
> +
> +#include <linux/types.h>
> +
> +/* WMI bus will filter all WMI vendor driver requests through this IOC */
> +#define WMI_IOC 'W'
> +
> +/* All ioctl requests through WMI should declare their size followed by
> + * relevant data objects
> + */
> +struct wmi_ioctl_buffer {
> +       __u64   length;
> +       __u8    data[];
> +};
> +
> +#endif
> --
> 2.14.1
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
  2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
                   ` (15 preceding siblings ...)
  2017-11-01 19:25 ` [PATCH v12 16/16] tools/wmi: add a sample for dell smbios communication over WMI Mario Limonciello
@ 2017-11-03  0:50 ` Darren Hart
  2017-11-03 16:30     ` Mario.Limonciello
  16 siblings, 1 reply; 26+ messages in thread
From: Darren Hart @ 2017-11-03  0:50 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Andy Shevchenko, LKML, platform-driver-x86, Andy Lutomirski,
	quasisec, pali.rohar, rjw, mjg59, hch, Greg KH, Alan Cox

On Wed, Nov 01, 2017 at 02:25:21PM -0500, Mario Limonciello wrote:
> The existing way that the dell-smbios helper module and associated
> other drivers (dell-laptop, dell-wmi) communicate with the platform
> really isn't secure.  It requires creating a buffer in physical
> DMA32 memory space and passing that to the platform via SMM.
> 
> Since the platform got a physical memory pointer, you've just got
> to trust that the platform has only modified (and accessed) memory
> within that buffer.
> 
> Dell Platform designers recognize this security risk and offer a
> safer way to communicate with the platform over ACPI.  This is
> in turn exposed via a WMI interface to the OS.
> 
> When communicating over WMI-ACPI the communication doesn't occur
> with physical memory pointers.  When the ASL is invoked, the fixed
> length ACPI buffer is copied to a small operating region.  The ASL
> will invoke the SMI, and SMM will only have access to this operating
> region.  When the ASL returns the buffer is copied back for the OS
> to process.
> 
> This method of communication should also deprecate the usage of the
> dcdbas kernel module and software dependent upon it's interface.
> Instead offer a character device interface for communicating with this
> ASL method to allow userspace to use instead.
> 
> To faciliate that this patch series introduces a generic way for WMI
> drivers to be able to create discoverable character devices with
> a predictable IOCTL interface 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.
> 
> Some of this work is the basis for what will be a proper interpreter
> of MOF in the kernel and controls for what drivers will be able to
> do with that MOF.
> 
> NOTE: This patch series is intended to go on top of platform-drivers-x86
> linux-next.
> 
> For convenience the entire series including those is also available here:
> https://github.com/dell/linux/tree/wmi-smbios

Queued for testing, thanks Mario.

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
  2017-11-03  0:50 ` [PATCH v12 00/16] Introduce support for Dell SMBIOS " Darren Hart
@ 2017-11-03 16:30     ` Mario.Limonciello
  0 siblings, 0 replies; 26+ messages in thread
From: Mario.Limonciello @ 2017-11-03 16:30 UTC (permalink / raw)
  To: dvhart
  Cc: andy.shevchenko, linux-kernel, platform-driver-x86, luto,
	quasisec, pali.rohar, rjw, mjg59, hch, greg, gnomes

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Thursday, November 2, 2017 7:50 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org; Andy Lutomirski
> <luto@kernel.org>; quasisec@google.com; pali.rohar@gmail.com;
> rjw@rjwysocki.net; mjg59@google.com; hch@lst.de; Greg KH <greg@kroah.com>;
> Alan Cox <gnomes@lxorguk.ukuu.org.uk>
> Subject: Re: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
> 
> On Wed, Nov 01, 2017 at 02:25:21PM -0500, Mario Limonciello wrote:
> > The existing way that the dell-smbios helper module and associated
> > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > really isn't secure.  It requires creating a buffer in physical
> > DMA32 memory space and passing that to the platform via SMM.
> >
> > Since the platform got a physical memory pointer, you've just got
> > to trust that the platform has only modified (and accessed) memory
> > within that buffer.
> >
> > Dell Platform designers recognize this security risk and offer a
> > safer way to communicate with the platform over ACPI.  This is
> > in turn exposed via a WMI interface to the OS.
> >
> > When communicating over WMI-ACPI the communication doesn't occur
> > with physical memory pointers.  When the ASL is invoked, the fixed
> > length ACPI buffer is copied to a small operating region.  The ASL
> > will invoke the SMI, and SMM will only have access to this operating
> > region.  When the ASL returns the buffer is copied back for the OS
> > to process.
> >
> > This method of communication should also deprecate the usage of the
> > dcdbas kernel module and software dependent upon it's interface.
> > Instead offer a character device interface for communicating with this
> > ASL method to allow userspace to use instead.
> >
> > To faciliate that this patch series introduces a generic way for WMI
> > drivers to be able to create discoverable character devices with
> > a predictable IOCTL interface 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.
> >
> > Some of this work is the basis for what will be a proper interpreter
> > of MOF in the kernel and controls for what drivers will be able to
> > do with that MOF.
> >
> > NOTE: This patch series is intended to go on top of platform-drivers-x86
> > linux-next.
> >
> > For convenience the entire series including those is also available here:
> > https://github.com/dell/linux/tree/wmi-smbios
> 
> Queued for testing, thanks Mario.
> 
> --
> Darren Hart
> VMware Open Source Technology Center

Thanks Darren.  BTW Did you forget to push?  I didn't see it at the testing branch:
http://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git/shortlog/refs/heads/testing

Thanks,

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
@ 2017-11-03 16:30     ` Mario.Limonciello
  0 siblings, 0 replies; 26+ messages in thread
From: Mario.Limonciello @ 2017-11-03 16:30 UTC (permalink / raw)
  To: dvhart
  Cc: andy.shevchenko, linux-kernel, platform-driver-x86, luto,
	quasisec, pali.rohar, rjw, mjg59, hch, greg, gnomes

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Thursday, November 2, 2017 7:50 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org; Andy Lutomirski
> <luto@kernel.org>; quasisec@google.com; pali.rohar@gmail.com;
> rjw@rjwysocki.net; mjg59@google.com; hch@lst.de; Greg KH <greg@kroah.com>;
> Alan Cox <gnomes@lxorguk.ukuu.org.uk>
> Subject: Re: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
> 
> On Wed, Nov 01, 2017 at 02:25:21PM -0500, Mario Limonciello wrote:
> > The existing way that the dell-smbios helper module and associated
> > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > really isn't secure.  It requires creating a buffer in physical
> > DMA32 memory space and passing that to the platform via SMM.
> >
> > Since the platform got a physical memory pointer, you've just got
> > to trust that the platform has only modified (and accessed) memory
> > within that buffer.
> >
> > Dell Platform designers recognize this security risk and offer a
> > safer way to communicate with the platform over ACPI.  This is
> > in turn exposed via a WMI interface to the OS.
> >
> > When communicating over WMI-ACPI the communication doesn't occur
> > with physical memory pointers.  When the ASL is invoked, the fixed
> > length ACPI buffer is copied to a small operating region.  The ASL
> > will invoke the SMI, and SMM will only have access to this operating
> > region.  When the ASL returns the buffer is copied back for the OS
> > to process.
> >
> > This method of communication should also deprecate the usage of the
> > dcdbas kernel module and software dependent upon it's interface.
> > Instead offer a character device interface for communicating with this
> > ASL method to allow userspace to use instead.
> >
> > To faciliate that this patch series introduces a generic way for WMI
> > drivers to be able to create discoverable character devices with
> > a predictable IOCTL interface 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.
> >
> > Some of this work is the basis for what will be a proper interpreter
> > of MOF in the kernel and controls for what drivers will be able to
> > do with that MOF.
> >
> > NOTE: This patch series is intended to go on top of platform-drivers-x86
> > linux-next.
> >
> > For convenience the entire series including those is also available here:
> > https://github.com/dell/linux/tree/wmi-smbios
> 
> Queued for testing, thanks Mario.
> 
> --
> Darren Hart
> VMware Open Source Technology Center

Thanks Darren.  BTW Did you forget to push?  I didn't see it at the testing branch:
http://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git/shortlog/refs/heads/testing

Thanks,

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
  2017-11-03 16:30     ` Mario.Limonciello
  (?)
@ 2017-11-03 21:00     ` Darren Hart
  2017-11-03 21:02         ` Mario.Limonciello
  -1 siblings, 1 reply; 26+ messages in thread
From: Darren Hart @ 2017-11-03 21:00 UTC (permalink / raw)
  To: Mario.Limonciello
  Cc: andy.shevchenko, linux-kernel, platform-driver-x86, luto,
	quasisec, pali.rohar, rjw, mjg59, hch, greg, gnomes

On Fri, Nov 03, 2017 at 04:30:13PM +0000, Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Darren Hart [mailto:dvhart@infradead.org]
> > Sent: Thursday, November 2, 2017 7:50 PM
> > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> > kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org; Andy Lutomirski
> > <luto@kernel.org>; quasisec@google.com; pali.rohar@gmail.com;
> > rjw@rjwysocki.net; mjg59@google.com; hch@lst.de; Greg KH <greg@kroah.com>;
> > Alan Cox <gnomes@lxorguk.ukuu.org.uk>
> > Subject: Re: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
> > 
> > On Wed, Nov 01, 2017 at 02:25:21PM -0500, Mario Limonciello wrote:
> > > The existing way that the dell-smbios helper module and associated
> > > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > > really isn't secure.  It requires creating a buffer in physical
> > > DMA32 memory space and passing that to the platform via SMM.
> > >
> > > Since the platform got a physical memory pointer, you've just got
> > > to trust that the platform has only modified (and accessed) memory
> > > within that buffer.
> > >
> > > Dell Platform designers recognize this security risk and offer a
> > > safer way to communicate with the platform over ACPI.  This is
> > > in turn exposed via a WMI interface to the OS.
> > >
> > > When communicating over WMI-ACPI the communication doesn't occur
> > > with physical memory pointers.  When the ASL is invoked, the fixed
> > > length ACPI buffer is copied to a small operating region.  The ASL
> > > will invoke the SMI, and SMM will only have access to this operating
> > > region.  When the ASL returns the buffer is copied back for the OS
> > > to process.
> > >
> > > This method of communication should also deprecate the usage of the
> > > dcdbas kernel module and software dependent upon it's interface.
> > > Instead offer a character device interface for communicating with this
> > > ASL method to allow userspace to use instead.
> > >
> > > To faciliate that this patch series introduces a generic way for WMI
> > > drivers to be able to create discoverable character devices with
> > > a predictable IOCTL interface 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.
> > >
> > > Some of this work is the basis for what will be a proper interpreter
> > > of MOF in the kernel and controls for what drivers will be able to
> > > do with that MOF.
> > >
> > > NOTE: This patch series is intended to go on top of platform-drivers-x86
> > > linux-next.
> > >
> > > For convenience the entire series including those is also available here:
> > > https://github.com/dell/linux/tree/wmi-smbios
> > 
> > Queued for testing, thanks Mario.
> > 
> > --
> > Darren Hart
> > VMware Open Source Technology Center
> 
> Thanks Darren.  BTW Did you forget to push?  I didn't see it at the testing branch:
> http://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git/shortlog/refs/heads/testing
> 
> Thanks,
> 

The workflow currently goes first to review-dvhart, which 0-day will pull from,
then to testing for integration, then to for-next. I plan to move it to testing
today.

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
  2017-11-03 21:00     ` Darren Hart
@ 2017-11-03 21:02         ` Mario.Limonciello
  0 siblings, 0 replies; 26+ messages in thread
From: Mario.Limonciello @ 2017-11-03 21:02 UTC (permalink / raw)
  To: dvhart
  Cc: andy.shevchenko, linux-kernel, platform-driver-x86, luto,
	quasisec, pali.rohar, rjw, mjg59, hch, greg, gnomes

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Friday, November 3, 2017 4:01 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: andy.shevchenko@gmail.com; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; luto@kernel.org; quasisec@google.com;
> pali.rohar@gmail.com; rjw@rjwysocki.net; mjg59@google.com; hch@lst.de;
> greg@kroah.com; gnomes@lxorguk.ukuu.org.uk
> Subject: Re: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
> 
> On Fri, Nov 03, 2017 at 04:30:13PM +0000, Mario.Limonciello@dell.com wrote:
> > > -----Original Message-----
> > > From: Darren Hart [mailto:dvhart@infradead.org]
> > > Sent: Thursday, November 2, 2017 7:50 PM
> > > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> > > kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org; Andy
> Lutomirski
> > > <luto@kernel.org>; quasisec@google.com; pali.rohar@gmail.com;
> > > rjw@rjwysocki.net; mjg59@google.com; hch@lst.de; Greg KH
> <greg@kroah.com>;
> > > Alan Cox <gnomes@lxorguk.ukuu.org.uk>
> > > Subject: Re: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
> > >
> > > On Wed, Nov 01, 2017 at 02:25:21PM -0500, Mario Limonciello wrote:
> > > > The existing way that the dell-smbios helper module and associated
> > > > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > > > really isn't secure.  It requires creating a buffer in physical
> > > > DMA32 memory space and passing that to the platform via SMM.
> > > >
> > > > Since the platform got a physical memory pointer, you've just got
> > > > to trust that the platform has only modified (and accessed) memory
> > > > within that buffer.
> > > >
> > > > Dell Platform designers recognize this security risk and offer a
> > > > safer way to communicate with the platform over ACPI.  This is
> > > > in turn exposed via a WMI interface to the OS.
> > > >
> > > > When communicating over WMI-ACPI the communication doesn't occur
> > > > with physical memory pointers.  When the ASL is invoked, the fixed
> > > > length ACPI buffer is copied to a small operating region.  The ASL
> > > > will invoke the SMI, and SMM will only have access to this operating
> > > > region.  When the ASL returns the buffer is copied back for the OS
> > > > to process.
> > > >
> > > > This method of communication should also deprecate the usage of the
> > > > dcdbas kernel module and software dependent upon it's interface.
> > > > Instead offer a character device interface for communicating with this
> > > > ASL method to allow userspace to use instead.
> > > >
> > > > To faciliate that this patch series introduces a generic way for WMI
> > > > drivers to be able to create discoverable character devices with
> > > > a predictable IOCTL interface 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.
> > > >
> > > > Some of this work is the basis for what will be a proper interpreter
> > > > of MOF in the kernel and controls for what drivers will be able to
> > > > do with that MOF.
> > > >
> > > > NOTE: This patch series is intended to go on top of platform-drivers-x86
> > > > linux-next.
> > > >
> > > > For convenience the entire series including those is also available here:
> > > > https://github.com/dell/linux/tree/wmi-smbios
> > >
> > > Queued for testing, thanks Mario.
> > >
> > > --
> > > Darren Hart
> > > VMware Open Source Technology Center
> >
> > Thanks Darren.  BTW Did you forget to push?  I didn't see it at the testing branch:
> > http://git.infradead.org/users/dvhart/linux-platform-drivers-
> x86.git/shortlog/refs/heads/testing
> >
> > Thanks,
> >
> 
> The workflow currently goes first to review-dvhart, which 0-day will pull from,
> then to testing for integration, then to for-next. I plan to move it to testing
> today.

Gotcha, thanks.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
@ 2017-11-03 21:02         ` Mario.Limonciello
  0 siblings, 0 replies; 26+ messages in thread
From: Mario.Limonciello @ 2017-11-03 21:02 UTC (permalink / raw)
  To: dvhart
  Cc: andy.shevchenko, linux-kernel, platform-driver-x86, luto,
	quasisec, pali.rohar, rjw, mjg59, hch, greg, gnomes

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Friday, November 3, 2017 4:01 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: andy.shevchenko@gmail.com; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; luto@kernel.org; quasisec@google.com;
> pali.rohar@gmail.com; rjw@rjwysocki.net; mjg59@google.com; hch@lst.de;
> greg@kroah.com; gnomes@lxorguk.ukuu.org.uk
> Subject: Re: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
> 
> On Fri, Nov 03, 2017 at 04:30:13PM +0000, Mario.Limonciello@dell.com wrote:
> > > -----Original Message-----
> > > From: Darren Hart [mailto:dvhart@infradead.org]
> > > Sent: Thursday, November 2, 2017 7:50 PM
> > > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> > > kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org; Andy
> Lutomirski
> > > <luto@kernel.org>; quasisec@google.com; pali.rohar@gmail.com;
> > > rjw@rjwysocki.net; mjg59@google.com; hch@lst.de; Greg KH
> <greg@kroah.com>;
> > > Alan Cox <gnomes@lxorguk.ukuu.org.uk>
> > > Subject: Re: [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI
> > >
> > > On Wed, Nov 01, 2017 at 02:25:21PM -0500, Mario Limonciello wrote:
> > > > The existing way that the dell-smbios helper module and associated
> > > > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > > > really isn't secure.  It requires creating a buffer in physical
> > > > DMA32 memory space and passing that to the platform via SMM.
> > > >
> > > > Since the platform got a physical memory pointer, you've just got
> > > > to trust that the platform has only modified (and accessed) memory
> > > > within that buffer.
> > > >
> > > > Dell Platform designers recognize this security risk and offer a
> > > > safer way to communicate with the platform over ACPI.  This is
> > > > in turn exposed via a WMI interface to the OS.
> > > >
> > > > When communicating over WMI-ACPI the communication doesn't occur
> > > > with physical memory pointers.  When the ASL is invoked, the fixed
> > > > length ACPI buffer is copied to a small operating region.  The ASL
> > > > will invoke the SMI, and SMM will only have access to this operating
> > > > region.  When the ASL returns the buffer is copied back for the OS
> > > > to process.
> > > >
> > > > This method of communication should also deprecate the usage of the
> > > > dcdbas kernel module and software dependent upon it's interface.
> > > > Instead offer a character device interface for communicating with this
> > > > ASL method to allow userspace to use instead.
> > > >
> > > > To faciliate that this patch series introduces a generic way for WMI
> > > > drivers to be able to create discoverable character devices with
> > > > a predictable IOCTL interface 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.
> > > >
> > > > Some of this work is the basis for what will be a proper interpreter
> > > > of MOF in the kernel and controls for what drivers will be able to
> > > > do with that MOF.
> > > >
> > > > NOTE: This patch series is intended to go on top of platform-drivers-x86
> > > > linux-next.
> > > >
> > > > For convenience the entire series including those is also available here:
> > > > https://github.com/dell/linux/tree/wmi-smbios
> > >
> > > Queued for testing, thanks Mario.
> > >
> > > --
> > > Darren Hart
> > > VMware Open Source Technology Center
> >
> > Thanks Darren.  BTW Did you forget to push?  I didn't see it at the testing branch:
> > http://git.infradead.org/users/dvhart/linux-platform-drivers-
> x86.git/shortlog/refs/heads/testing
> >
> > Thanks,
> >
> 
> The workflow currently goes first to review-dvhart, which 0-day will pull from,
> then to testing for integration, then to for-next. I plan to move it to testing
> today.

Gotcha, thanks.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v12 10/16] platform/x86: dell-smbios: Introduce dispatcher for SMM calls
  2017-11-01 19:25 ` [PATCH v12 10/16] platform/x86: dell-smbios: Introduce dispatcher for SMM calls Mario Limonciello
@ 2018-01-27 14:48   ` Pali Rohár
  0 siblings, 0 replies; 26+ messages in thread
From: Pali Rohár @ 2018-01-27 14:48 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: dvhart, Andy Shevchenko, LKML, platform-driver-x86,
	Andy Lutomirski, quasisec, rjw, mjg59, hch, Greg KH, Alan Cox

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

Hi!

On Wednesday 01 November 2017 14:25:31 Mario Limonciello wrote:
> This splits up the dell-smbios driver into two drivers:
> * dell-smbios
> * dell-smbios-smm
> 
> dell-smbios can operate with multiple different dispatcher drivers to
> perform SMBIOS operations.
> 
> Also modify the interface that dell-laptop and dell-wmi use align to this
> model more closely.  Rather than a single global buffer being allocated
> for all drivers, each driver will allocate and be responsible for it's own
> buffer. The pointer will be passed to the calling function and each
> dispatcher driver will then internally copy it to the proper location to
> perform it's call.
> 
> Add defines for calls used by these methods in the dell-smbios.h header
> for tracking purposes.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> Reviewed-by: Edward O'Callaghan <quasisec@google.com>
> ---
...
> @@ -85,6 +73,7 @@ static struct platform_driver platform_driver = {
>  	}
>  };
>  
> +static struct calling_interface_buffer *buffer;
>  static struct platform_device *platform_device;
>  static struct backlight_device *dell_backlight_device;
>  static struct rfkill *wifi_rfkill;
> @@ -283,6 +272,27 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
>  	{ }
>  };
>  
> +void dell_set_arguments(u32 arg0, u32 arg1, u32 arg2, u32 arg3)
> +{
> +	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> +	buffer->input[0] = arg0;
> +	buffer->input[1] = arg1;
> +	buffer->input[2] = arg2;
> +	buffer->input[3] = arg3;
> +}
> +
> +int dell_send_request(u16 class, u16 select)
> +{
> +	int ret;
> +
> +	buffer->cmd_class = class;
> +	buffer->cmd_select = select;
> +	ret = dell_smbios_call(buffer);
> +	if (ret != 0)
> +		return ret;
> +	return dell_smbios_error(buffer->output[0]);
> +}
> +
>  /*
>   * Derived from information in smbios-wireless-ctl:
>   *
...
> @@ -413,20 +422,16 @@ static int dell_rfkill_set(void *data, bool blocked)
>  	int status;
>  	int ret;
>  
> -	buffer = dell_smbios_get_buffer();
> -
> -	dell_smbios_send_request(17, 11);
> -	ret = buffer->output[0];
> +	dell_set_arguments(0, 0, 0, 0);
> +	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> +	if (ret)
> +		return ret;
>  	status = buffer->output[1];

Now I'm looking at this patch again and I think it introduced a new race
condition.

Prior this patch, dell_smbios_get_buffer() acquired mutex and returned
pointer to buffer which caller used and then released.

Now buffer is allocated at module load time and is shared for all
functions in this module. And hen is used, there is no mutex protection
for it.

First call is to dell_set_arguments() which modifies that shared buffer
and then dell_send_request() is used to modify and pass buffer to
another function dell_smbios_call().

And function below dell_update_rfkill() is called work queue which also
modifies buffer by dell_set_arguments() function.

Therefore it looks like when dell_update_rfkill() is scheduled at the
same time as dell_rfkill_set() is executed, then both functions
overwrite one shared buffer and something unexpected would be sent to
SMM.

> @@ -613,46 +596,36 @@ static const struct file_operations dell_debugfs_fops = {
>  
>  static void dell_update_rfkill(struct work_struct *ignored)
>  {
> -	struct calling_interface_buffer *buffer;
>  	int hwswitch = 0;
>  	int status;
>  	int ret;
>  
> -	buffer = dell_smbios_get_buffer();
> -
> -	dell_smbios_send_request(17, 11);
> -	ret = buffer->output[0];
> +	dell_set_arguments(0, 0, 0, 0);
> +	ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
>  	status = buffer->output[1];

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2018-01-27 14:48 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-01 19:25 [PATCH v12 00/16] Introduce support for Dell SMBIOS over WMI Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 01/16] platform/x86: dell-smbios: Prefix class/select with cmd_ Mario Limonciello
2017-11-02  0:30   ` Edward O'Callaghan
2017-11-01 19:25 ` [PATCH v12 02/16] platform/x86: wmi: Add new method wmidev_evaluate_method Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 03/16] platform/x86: dell-wmi: increase severity of some failures Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 04/16] platform/x86: dell-wmi: clean up wmi descriptor check Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 05/16] platform/x86: dell-wmi: don't check length returned Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 06/16] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 07/16] platform/x86: wmi: Don't allow drivers to get each other's GUIDs Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 08/16] platform/x86: dell-smbios: only run if proper oem string is detected Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 09/16] platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 10/16] platform/x86: dell-smbios: Introduce dispatcher for SMM calls Mario Limonciello
2018-01-27 14:48   ` Pali Rohár
2017-11-01 19:25 ` [PATCH v12 11/16] platform/x86: dell-smbios-wmi: Add new WMI dispatcher driver Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 12/16] platform/x86: dell-smbios-smm: test for WSMT Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 13/16] platform/x86: dell-smbios: Add filtering support Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 14/16] platform/x86: wmi: create userspace interface for drivers Mario Limonciello
2017-11-02  0:31   ` Edward O'Callaghan
2017-11-01 19:25 ` [PATCH v12 15/16] platform/x86: dell-smbios-wmi: introduce userspace interface Mario Limonciello
2017-11-01 19:25 ` [PATCH v12 16/16] tools/wmi: add a sample for dell smbios communication over WMI Mario Limonciello
2017-11-03  0:50 ` [PATCH v12 00/16] Introduce support for Dell SMBIOS " Darren Hart
2017-11-03 16:30   ` Mario.Limonciello
2017-11-03 16:30     ` Mario.Limonciello
2017-11-03 21:00     ` Darren Hart
2017-11-03 21:02       ` Mario.Limonciello
2017-11-03 21:02         ` Mario.Limonciello

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.