From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753983AbdKAA12 (ORCPT ); Tue, 31 Oct 2017 20:27:28 -0400 Received: from mail-vk0-f66.google.com ([209.85.213.66]:53809 "EHLO mail-vk0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753486AbdKAA10 (ORCPT ); Tue, 31 Oct 2017 20:27:26 -0400 X-Google-Smtp-Source: ABhQp+QqOr5tzgMdCAOZGQWxqnwR5jsx9P1CDwwhLnoI2t/CbmavfzI8rBdyiJoTvjkZwelCR9UfCwiKiEN+O0STv/M= MIME-Version: 1.0 In-Reply-To: References: From: "Edward O'Callaghan" Date: Wed, 1 Nov 2017 11:27:24 +1100 Message-ID: Subject: Re: [PATCH v11 14/15] platform/x86: dell-smbios-wmi: introduce userspace interface To: Mario Limonciello Cc: dvhart@infradead.org, Andy Shevchenko , LKML , platform-driver-x86@vger.kernel.org, Andy Lutomirski , pali.rohar@gmail.com, rjw@rjwysocki.net, Matthew Garrett , hch@lst.de, Greg KH , Alan Cox Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Oct 21, 2017 at 4:40 AM, Mario Limonciello wrote: > 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 > Reviewed-by: Edward O'Callaghan > --- > Documentation/ABI/testing/dell-smbios-wmi | 41 ++++++++++++++++++++++++ > drivers/platform/x86/dell-smbios-wmi.c | 53 ++++++++++++++++++++++++------- > drivers/platform/x86/dell-smbios.h | 32 ++----------------- > include/uapi/linux/wmi.h | 47 +++++++++++++++++++++++++++ > 4 files changed, 132 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" > +Description: > + Perform SMBIOS calls on supported Dell machines. > + through the Dell ACPI-WMI interface. > + > + IOCTL's and buffer formats are defined in: > + > + > + 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 d2b8438ea91f..04b0153c9bee 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,41 @@ 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.class, priv->buf->std.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 +159,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 +240,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 23256f79a4b8..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 > +#include > > -/* 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 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/wmi.h b/include/uapi/linux/wmi.h > index 7e52350ac9b3..5ff61f2d37ba 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 > #include > > /* 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 class; Hey Mario, I just realized that there is a slight problem calling this identifier "class" for userspace that happens to be written in C++ that #includes this header as class is a reversed word. Perhaps we could rename it to class_ or something otherwise userspace has to shim with a C object and extern it with new identifiers to the C++ component, which is awkward. Kind Regards, Edward. > + __u16 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 >