diff mbox

[v6,14/14] platform/x86: dell-smbios-wmi: introduce userspace interface

Message ID 411980beb5697e312145ef268516f4cc7c585b8f.1507589249.git.mario.limonciello@dell.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Limonciello, Mario Oct. 9, 2017, 10:51 p.m. UTC
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 use the character device the buffer needed for the machine will
also be needed.  This information is exported to a sysfs attribute.

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

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 Documentation/ABI/testing/dell-smbios-wmi          |  41 ++++++++
 .../ABI/testing/sysfs-platform-dell-smbios-wmi     |  10 ++
 MAINTAINERS                                        |   1 +
 drivers/platform/x86/dell-smbios-wmi.c             | 107 ++++++++++++++++++---
 drivers/platform/x86/dell-smbios.h                 |  11 +--
 include/uapi/linux/dell-smbios.h                   |  42 ++++++++
 6 files changed, 191 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/ABI/testing/dell-smbios-wmi
 create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi
 create mode 100644 include/uapi/linux/dell-smbios.h

Comments

Greg KH Oct. 10, 2017, 1:59 p.m. UTC | #1
On Mon, Oct 09, 2017 at 05:51:52PM -0500, Mario Limonciello wrote:
> +	ret = device_create_file(&wdev->dev, &priv->req_buf_size_attr);
> +	if (ret)
> +		goto fail_create_sysfs;

Why isn't the "WMI core" creating this sysfs file?  Why have per-driver
sysfs files, making all of the different apis totally different?  It's a
"common" attribute that they are all going to have to provide, right?

That way you also don't race with userspace :)

thanks,

greg k-h
Limonciello, Mario Oct. 10, 2017, 2:17 p.m. UTC | #2
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, October 10, 2017 8:59 AM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; Andy Shevchenko <andy.shevchenko@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> Andy Lutomirski <luto@kernel.org>; quasisec@google.com;
> pali.rohar@gmail.com; rjw@rjwysocki.net; mjg59@google.com; hch@lst.de
> Subject: Re: [PATCH v6 14/14] platform/x86: dell-smbios-wmi: introduce
> userspace interface
> 
> On Mon, Oct 09, 2017 at 05:51:52PM -0500, Mario Limonciello wrote:
> > +	ret = device_create_file(&wdev->dev, &priv->req_buf_size_attr);
> > +	if (ret)
> > +		goto fail_create_sysfs;
> 
> Why isn't the "WMI core" creating this sysfs file?  Why have per-driver
> sysfs files, making all of the different apis totally different?  It's a
> "common" attribute that they are all going to have to provide, right?
> 
I hadn't really thought about that.  I suppose it's entirely reasonable
to have a way that all WMI ioctls for future drivers will advertise the size
of their expected buffer the same way.

So this does beg a question to others on this distribution who have worked
on WMI drivers- do you know of any WM**/ methods with multiple instances 
that use different buffer sizes, or is it reasonable to expect that the buffer size 
is consistent between instances?

Even in advanced MOF designs I haven't seen it, but this makes me
wonder if an attribute should be created for every instance to be future proof.
(For example $GUID/required_buffer_size/instance0, 
$GUID/required_buffer_size/instance1 etc)

> That way you also don't race with userspace :)
> 
> thanks,
> 
> greg k-h
kernel test robot Oct. 12, 2017, 1:13 a.m. UTC | #3
Hi Mario,

[auto build test WARNING on platform-drivers-x86/for-next]
[also build test WARNING on next-20171009]
[cannot apply to v4.14-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20171012-082742
base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: i386-tinyconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

>> ./usr/include/linux/dell-smbios.h:21: found __[us]{8,16,32,64} type without #include <linux/types.h>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Oct. 12, 2017, 1:16 a.m. UTC | #4
Hi Mario,

[auto build test WARNING on platform-drivers-x86/for-next]
[also build test WARNING on next-20171009]
[cannot apply to v4.14-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20171012-082742
base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: x86_64-randconfig-x018-201741 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kobject.h:21:0,
                    from include/linux/dmi.h:5,
                    from drivers/platform/x86/dell-smbios-wmi.c:12:
   drivers/platform/x86/dell-smbios-wmi.c: In function 'dell_smbios_wmi_probe':
   include/linux/sysfs.h:54:8: error: 'struct device_attribute' has no member named 'key'
     (attr)->key = &__key;    \
           ^
>> drivers/platform/x86/dell-smbios-wmi.c:207:2: note: in expansion of macro 'sysfs_attr_init'
     sysfs_attr_init(&priv->req_buf_size_attr);
     ^~~~~~~~~~~~~~~

vim +/sysfs_attr_init +207 drivers/platform/x86/dell-smbios-wmi.c

    11	
  > 12	#include <linux/dmi.h>
    13	#include <linux/list.h>
    14	#include <linux/module.h>
    15	#include <linux/mutex.h>
    16	#include <linux/uaccess.h>
    17	#include <linux/wmi.h>
    18	#include <uapi/linux/dell-smbios.h>
    19	#include "dell-smbios.h"
    20	#include "dell-wmi-descriptor.h"
    21	static DEFINE_MUTEX(call_mutex);
    22	static DEFINE_MUTEX(list_mutex);
    23	static int wmi_supported;
    24	
    25	struct misc_bios_flags_structure {
    26		struct dmi_header header;
    27		u16 flags0;
    28	} __packed;
    29	#define FLAG_HAS_ACPI_WMI 0x02
    30	
    31	#define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
    32	
    33	struct wmi_smbios_priv {
    34		struct wmi_smbios_buffer *buf;
    35		struct device_attribute req_buf_size_attr;
    36		struct list_head list;
    37		struct wmi_device *wdev;
    38		struct device *child;
    39		u32 req_buf_size;
    40	};
    41	static LIST_HEAD(wmi_list);
    42	
    43	static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
    44	{
    45		return list_first_entry_or_null(&wmi_list,
    46						struct wmi_smbios_priv,
    47						list);
    48	}
    49	
    50	static int run_smbios_call(struct wmi_device *wdev)
    51	{
    52		struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
    53		struct wmi_smbios_priv *priv;
    54		struct acpi_buffer input;
    55		union acpi_object *obj;
    56		acpi_status status;
    57	
    58		priv = dev_get_drvdata(&wdev->dev);
    59		input.length = priv->req_buf_size;
    60		input.pointer = &priv->buf->std;
    61	
    62		dev_dbg(&wdev->dev, "evaluating: %x/%x [%x,%x,%x,%x]\n",
    63			priv->buf->std.class, priv->buf->std.select,
    64			priv->buf->std.input[0], priv->buf->std.input[1],
    65			priv->buf->std.input[2], priv->buf->std.input[3]);
    66	
    67		status = wmidev_evaluate_method(wdev, 0, 1, &input, &output);
    68		if (ACPI_FAILURE(status))
    69			return -EIO;
    70		obj = (union acpi_object *)output.pointer;
    71		if (obj->type != ACPI_TYPE_BUFFER) {
    72			dev_dbg(&wdev->dev, "received type: %d\n", obj->type);
    73			if (obj->type == ACPI_TYPE_INTEGER)
    74				dev_dbg(&wdev->dev, "SMBIOS call failed: %llu\n",
    75					obj->integer.value);
    76			return -EIO;
    77		}
    78		memcpy(&priv->buf->std, obj->buffer.pointer, obj->buffer.length);
    79	
    80		return 0;
    81	}
    82	
    83	int dell_smbios_wmi_call(struct calling_interface_buffer *buffer)
    84	{
    85		struct wmi_smbios_priv *priv;
    86		size_t difference;
    87		size_t size;
    88		int ret;
    89	
    90		priv = get_first_smbios_priv();
    91		if (!priv)
    92			return -ENODEV;
    93	
    94		size = sizeof(struct calling_interface_buffer);
    95		difference = priv->req_buf_size - sizeof(u64) - size;
    96	
    97		mutex_lock(&call_mutex);
    98		memset(&priv->buf->ext, 0, difference);
    99		memcpy(&priv->buf->std, buffer, size);
   100		ret = run_smbios_call(priv->wdev);
   101		memcpy(buffer, &priv->buf->std, size);
   102		mutex_unlock(&call_mutex);
   103	
   104		return ret;
   105	}
   106	
   107	static long dell_smbios_wmi_ioctl(struct wmi_device *wdev, unsigned int cmd,
   108					  unsigned long arg)
   109	{
   110		struct wmi_smbios_buffer __user *input =
   111			(struct wmi_smbios_buffer __user *) arg;
   112		struct wmi_smbios_priv *priv;
   113		int ret = 0;
   114		u64 size;
   115	
   116		switch (cmd) {
   117		case DELL_WMI_SMBIOS_CMD:
   118			priv = dev_get_drvdata(&wdev->dev);
   119			if (!priv)
   120				return -ENODEV;
   121			mutex_lock(&call_mutex);
   122			/* read the size that userspace is sending */
   123			if (get_user(size, &input->length)) {
   124				dev_dbg(&wdev->dev, "Read length from user failed\n");
   125				ret = -EFAULT;
   126				goto fail_smbios_cmd;
   127			}
   128			/* if it's too big, this is OK, we'll only use what we need */
   129			if (size > priv->req_buf_size)
   130				dev_warn(&wdev->dev,
   131					"Buffer %lld is bigger than required %d\n",
   132					size, priv->req_buf_size);
   133			/* if it's too small, abort */
   134			if (size < priv->req_buf_size) {
   135				dev_err(&wdev->dev,
   136					"Buffer %lld too small, need at least %d\n",
   137					size, priv->req_buf_size);
   138				ret = -EINVAL;
   139				goto fail_smbios_cmd;
   140			}
   141			/* read the structure from userspace */
   142			if (copy_from_user(priv->buf, input, priv->req_buf_size)) {
   143				dev_dbg(&wdev->dev, "Copy %llu from user failed\n",
   144					size);
   145				ret = -EFAULT;
   146				goto fail_smbios_cmd;
   147			}
   148			/* check for any calls we should avoid */
   149			if (dell_smbios_call_filter(&wdev->dev, &priv->buf->std)) {
   150				dev_err(&wdev->dev, "Invalid call %d/%d:%8x\n",
   151					priv->buf->std.class, priv->buf->std.select,
   152					priv->buf->std.input[0]);
   153				ret = -EFAULT;
   154				goto fail_smbios_cmd;
   155			}
   156			ret = run_smbios_call(priv->wdev);
   157			if (ret != 0)
   158				goto fail_smbios_cmd;
   159			/* return the result (only up to our internal buffer size) */
   160			if (copy_to_user(input, priv->buf, priv->req_buf_size)) {
   161				dev_dbg(&wdev->dev, "Copy %d to user failed\n",
   162				priv->req_buf_size);
   163				ret = -EFAULT;
   164			}
   165	fail_smbios_cmd:
   166			mutex_unlock(&call_mutex);
   167			break;
   168		default:
   169			dev_dbg(&wdev->dev, "unsupported ioctl: %d [%d, %d, %d, %d].\n",
   170				cmd, _IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd),
   171				_IOC_SIZE(cmd));
   172			ret = -ENOIOCTLCMD;
   173		}
   174		return ret;
   175	}
   176	
   177	static ssize_t req_buf_size_show(struct device *dev,
   178					struct device_attribute *attr, char *buf)
   179	{
   180		struct wmi_smbios_priv *priv = dev_get_drvdata(dev);
   181	
   182		return sprintf(buf, "%d\n", priv->req_buf_size);
   183	}
   184	
   185	static int dell_smbios_wmi_probe(struct wmi_device *wdev)
   186	{
   187		struct wmi_smbios_priv *priv;
   188		int count;
   189		int ret;
   190	
   191		priv = devm_kzalloc(&wdev->dev, sizeof(struct wmi_smbios_priv),
   192				    GFP_KERNEL);
   193		if (!priv)
   194			return -ENOMEM;
   195	
   196		/* WMI buffer size will be either 4k or 32k depending on machine */
   197		if (!dell_wmi_get_size(&priv->req_buf_size))
   198			return -EINVAL;
   199		/* add in the length object we will use internally with ioctl */
   200		priv->req_buf_size += sizeof(u64);
   201	
   202		count = get_order(priv->req_buf_size);
   203		priv->buf = (void *)__get_free_pages(GFP_KERNEL, count);
   204		if (!priv->buf)
   205			return -ENOMEM;
   206	
 > 207		sysfs_attr_init(&priv->req_buf_size_attr);
   208		priv->req_buf_size_attr.attr.name = "required_buffer_size";
   209		priv->req_buf_size_attr.attr.mode = 0444;
   210		priv->req_buf_size_attr.show = req_buf_size_show;
   211	
   212		ret = device_create_file(&wdev->dev, &priv->req_buf_size_attr);
   213		if (ret)
   214			goto fail_create_sysfs;
   215	
   216		/* ID is used by dell-smbios to set priority of drivers */
   217		wdev->dev.id = 1;
   218		ret = dell_smbios_register_device(&wdev->dev, &dell_smbios_wmi_call);
   219		if (ret)
   220			goto fail_register;
   221	
   222		priv->wdev = wdev;
   223		dev_set_drvdata(&wdev->dev, priv);
   224		mutex_lock(&list_mutex);
   225		list_add_tail(&priv->list, &wmi_list);
   226		mutex_unlock(&list_mutex);
   227	
   228		return 0;
   229	
   230	fail_register:
   231		device_remove_file(&wdev->dev, &priv->req_buf_size_attr);
   232	
   233	fail_create_sysfs:
   234		free_pages((unsigned long)priv->buf, count);
   235		return ret;
   236	}
   237	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/Documentation/ABI/testing/dell-smbios-wmi b/Documentation/ABI/testing/dell-smbios-wmi
new file mode 100644
index 000000000000..e067e955fcc9
--- /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/dell-smbios.h>
+
+		1) To perform a 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, refer to:
+		sysfs-platform-dell-smbios-wmi
+
+		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/Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi b/Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi
new file mode 100644
index 000000000000..33f0fb73f785
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi
@@ -0,0 +1,10 @@ 
+What:		/sys/devices/platform/<platform>/required_buffer_size
+Date:		November 2017
+KernelVersion:	4.15
+Contact:	"Mario Limonciello" <mario.limonciello@dell.com>
+Description:
+		A read-only description of the size of a calling
+		interface buffer that can be passed to Dell
+		firmware.
+
+		Commonly this size is either 4k or 32k.
diff --git a/MAINTAINERS b/MAINTAINERS
index 2a99ee9fd883..4940f3c7481b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3986,6 +3986,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:	include/uapi/linux/dell-smbios.h
 
 DELL LAPTOP DRIVER
 M:	Matthew Garrett <mjg59@srcf.ucam.org>
diff --git a/drivers/platform/x86/dell-smbios-wmi.c b/drivers/platform/x86/dell-smbios-wmi.c
index e8a2eb858677..ec38fbf945d5 100644
--- a/drivers/platform/x86/dell-smbios-wmi.c
+++ b/drivers/platform/x86/dell-smbios-wmi.c
@@ -15,6 +15,7 @@ 
 #include <linux/mutex.h>
 #include <linux/uaccess.h>
 #include <linux/wmi.h>
+#include <uapi/linux/dell-smbios.h>
 #include "dell-smbios.h"
 #include "dell-wmi-descriptor.h"
 static DEFINE_MUTEX(call_mutex);
@@ -29,19 +30,9 @@  struct misc_bios_flags_structure {
 
 #define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
 
-struct wmi_extensions {
-	__u32 argattrib;
-	__u32 blength;
-	__u8 data[];
-} __packed;
-
-struct wmi_smbios_buffer {
-	struct calling_interface_buffer std;
-	struct wmi_extensions ext;
-} __packed;
-
 struct wmi_smbios_priv {
 	struct wmi_smbios_buffer *buf;
+	struct device_attribute req_buf_size_attr;
 	struct list_head list;
 	struct wmi_device *wdev;
 	struct device *child;
@@ -113,6 +104,84 @@  int dell_smbios_wmi_call(struct calling_interface_buffer *buffer)
 	return ret;
 }
 
+static long dell_smbios_wmi_ioctl(struct wmi_device *wdev, unsigned int cmd,
+				  unsigned long arg)
+{
+	struct wmi_smbios_buffer __user *input =
+		(struct wmi_smbios_buffer __user *) arg;
+	struct wmi_smbios_priv *priv;
+	int ret = 0;
+	u64 size;
+
+	switch (cmd) {
+	case DELL_WMI_SMBIOS_CMD:
+		priv = dev_get_drvdata(&wdev->dev);
+		if (!priv)
+			return -ENODEV;
+		mutex_lock(&call_mutex);
+		/* read the size that userspace is sending */
+		if (get_user(size, &input->length)) {
+			dev_dbg(&wdev->dev, "Read length from user failed\n");
+			ret = -EFAULT;
+			goto fail_smbios_cmd;
+		}
+		/* if it's too big, this is OK, we'll only use what we need */
+		if (size > priv->req_buf_size)
+			dev_warn(&wdev->dev,
+				"Buffer %lld is bigger than required %d\n",
+				size, priv->req_buf_size);
+		/* if it's too small, abort */
+		if (size < priv->req_buf_size) {
+			dev_err(&wdev->dev,
+				"Buffer %lld too small, need at least %d\n",
+				size, priv->req_buf_size);
+			ret = -EINVAL;
+			goto fail_smbios_cmd;
+		}
+		/* read the structure from userspace */
+		if (copy_from_user(priv->buf, input, priv->req_buf_size)) {
+			dev_dbg(&wdev->dev, "Copy %llu from user failed\n",
+				size);
+			ret = -EFAULT;
+			goto fail_smbios_cmd;
+		}
+		/* check for any calls we should avoid */
+		if (dell_smbios_call_filter(&wdev->dev, &priv->buf->std)) {
+			dev_err(&wdev->dev, "Invalid call %d/%d:%8x\n",
+				priv->buf->std.class, priv->buf->std.select,
+				priv->buf->std.input[0]);
+			ret = -EFAULT;
+			goto fail_smbios_cmd;
+		}
+		ret = run_smbios_call(priv->wdev);
+		if (ret != 0)
+			goto fail_smbios_cmd;
+		/* return the result (only up to our internal buffer size) */
+		if (copy_to_user(input, priv->buf, priv->req_buf_size)) {
+			dev_dbg(&wdev->dev, "Copy %d to user failed\n",
+			priv->req_buf_size);
+			ret = -EFAULT;
+		}
+fail_smbios_cmd:
+		mutex_unlock(&call_mutex);
+		break;
+	default:
+		dev_dbg(&wdev->dev, "unsupported ioctl: %d [%d, %d, %d, %d].\n",
+			cmd, _IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd),
+			_IOC_SIZE(cmd));
+		ret = -ENOIOCTLCMD;
+	}
+	return ret;
+}
+
+static ssize_t req_buf_size_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct wmi_smbios_priv *priv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", priv->req_buf_size);
+}
+
 static int dell_smbios_wmi_probe(struct wmi_device *wdev)
 {
 	struct wmi_smbios_priv *priv;
@@ -127,12 +196,23 @@  static int dell_smbios_wmi_probe(struct wmi_device *wdev)
 	/* WMI buffer size will be either 4k or 32k depending on machine */
 	if (!dell_wmi_get_size(&priv->req_buf_size))
 		return -EINVAL;
+	/* add in the length object we will use internally with ioctl */
+	priv->req_buf_size += sizeof(u64);
 
 	count = get_order(priv->req_buf_size);
 	priv->buf = (void *)__get_free_pages(GFP_KERNEL, count);
 	if (!priv->buf)
 		return -ENOMEM;
 
+	sysfs_attr_init(&priv->req_buf_size_attr);
+	priv->req_buf_size_attr.attr.name = "required_buffer_size";
+	priv->req_buf_size_attr.attr.mode = 0444;
+	priv->req_buf_size_attr.show = req_buf_size_show;
+
+	ret = device_create_file(&wdev->dev, &priv->req_buf_size_attr);
+	if (ret)
+		goto fail_create_sysfs;
+
 	/* 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);
@@ -148,6 +228,9 @@  static int dell_smbios_wmi_probe(struct wmi_device *wdev)
 	return 0;
 
 fail_register:
+	device_remove_file(&wdev->dev, &priv->req_buf_size_attr);
+
+fail_create_sysfs:
 	free_pages((unsigned long)priv->buf, count);
 	return ret;
 }
@@ -162,6 +245,7 @@  static int dell_smbios_wmi_remove(struct wmi_device *wdev)
 	list_del(&priv->list);
 	mutex_unlock(&list_mutex);
 	dell_smbios_unregister_device(&wdev->dev);
+	device_remove_file(&wdev->dev, &priv->req_buf_size_attr);
 	count = get_order(priv->req_buf_size);
 	free_pages((unsigned long)priv->buf, count);
 	mutex_unlock(&call_mutex);
@@ -203,6 +287,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,
+	.unlocked_ioctl = dell_smbios_wmi_ioctl,
 };
 
 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 c743c58831e5..04995136114c 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -17,19 +17,10 @@ 
 #define _DELL_SMBIOS_H_
 
 #include <linux/device.h>
+#include <uapi/linux/dell-smbios.h>
 
 struct notifier_block;
 
-/* This structure will be modified by the firmware when we enter
- * system management mode, hence the volatiles */
-
-struct calling_interface_buffer {
-	u16 class;
-	u16 select;
-	volatile u32 input[4];
-	volatile u32 output[4];
-} __packed;
-
 struct calling_interface_token {
 	u16 tokenID;
 	u16 location;
diff --git a/include/uapi/linux/dell-smbios.h b/include/uapi/linux/dell-smbios.h
new file mode 100644
index 000000000000..8ce142b95f15
--- /dev/null
+++ b/include/uapi/linux/dell-smbios.h
@@ -0,0 +1,42 @@ 
+/*
+ *  User API for 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.
+ */
+
+#ifndef _UAPI_DELL_SMBIOS_H_
+#define _UAPI_DELL_SMBIOS_H_
+
+#include <linux/ioctl.h>
+#include <linux/wmi.h>
+
+/* This structure may be modified by the firmware when we enter
+ * system management mode through SMM, hence the volatiles
+ */
+struct calling_interface_buffer {
+	__u16 class;
+	__u16 select;
+	volatile __u32 input[4];
+	volatile __u32 output[4];
+} __packed;
+
+struct wmi_extensions {
+	__u32 argattrib;
+	__u32 blength;
+	__u8 data[];
+} __packed;
+
+struct wmi_smbios_buffer {
+	__u64 length;
+	struct calling_interface_buffer std;
+	struct wmi_extensions		ext;
+} __packed;
+
+/* SMBIOS calling IOCTL command */
+#define DELL_WMI_SMBIOS_CMD	WMI_IOWR(0, struct wmi_smbios_buffer)
+
+#endif /* _UAPI_DELL_SMBIOS_H_ */