platform/x86: wmi: Remove chardev interface
authorArmin Wolf <W_Armin@gmx.de>
Sun, 10 Dec 2023 20:24:43 +0000 (21:24 +0100)
committerHans de Goede <hdegoede@redhat.com>
Mon, 11 Dec 2023 10:24:11 +0000 (11:24 +0100)
The design of the WMI chardev interface is broken:
- it assumes that WMI drivers are not instantiated twice
- it offers next to no abstractions, the WMI driver gets
a raw byte buffer
- it is only used by a single driver, something which is
unlikely to change

Since the only user (dell-smbios-wmi) has been migrated
to his own ioctl interface, remove it.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
Link: https://lore.kernel.org/r/20231210202443.646427-6-W_Armin@gmx.de
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
drivers/platform/x86/wmi.c
include/linux/wmi.h

index 7df5b5ee7983d531752ed2737fbdc2da7a69391b..7303702290e5ed4e48fd31f9d17cd09ddec72c7b 100644 (file)
 #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/sysfs.h>
 #include <linux/types.h>
-#include <linux/uaccess.h>
 #include <linux/uuid.h>
 #include <linux/wmi.h>
 #include <linux/fs.h>
-#include <uapi/linux/wmi.h>
 
 MODULE_AUTHOR("Carlos Corbacho");
 MODULE_DESCRIPTION("ACPI-WMI Mapping Driver");
@@ -66,12 +63,9 @@ 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;
        unsigned long flags;
 };
 
@@ -256,26 +250,6 @@ static void wmi_device_put(struct wmi_device *wdev)
  * Exported WMI functions
  */
 
-/**
- * set_required_buffer_size - Sets the buffer size needed for performing IOCTL
- * @wdev: A wmi bus device from a driver
- * @length: Required buffer size
- *
- * Allocates memory needed for buffer, stores the buffer size in that memory.
- *
- * Return: 0 on success or a negative error code for failure.
- */
-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_instance_count - Get number of WMI object instances
  * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
@@ -884,111 +858,12 @@ 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)
-{
-       /*
-        * The miscdevice already stores a pointer to itself
-        * inside filp->private_data
-        */
-       struct wmi_block *wblock = container_of(filp->private_data, struct wmi_block, char_dev);
-
-       filp->private_data = wblock;
-
-       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;
-       struct wmi_driver *wdriver;
-       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 = drv_to_wdrv(wblock->dev.dev.driver);
-       if (!try_module_get(wdriver->driver.owner)) {
-               ret = -EBUSY;
-               goto out_ioctl;
-       }
-       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   = compat_ptr_ioctl,
-};
 
 static int wmi_dev_probe(struct device *dev)
 {
        struct wmi_block *wblock = dev_to_wblock(dev);
        struct wmi_driver *wdriver = drv_to_wdrv(dev->driver);
        int ret = 0;
-       char *buf;
 
        if (ACPI_FAILURE(wmi_method_enable(wblock, true)))
                dev_warn(dev, "failed to enable device -- probing anyway\n");
@@ -996,55 +871,17 @@ static int wmi_dev_probe(struct device *dev)
        if (wdriver->probe) {
                ret = wdriver->probe(dev_to_wdev(dev),
                                find_guid_context(wblock, wdriver));
-               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;
-               }
+               if (!ret) {
+                       if (ACPI_FAILURE(wmi_method_enable(wblock, false)))
+                               dev_warn(dev, "Failed to disable device\n");
 
-               wblock->handler_data = kmalloc(wblock->req_buf_size,
-                                              GFP_KERNEL);
-               if (!wblock->handler_data) {
-                       ret = -ENOMEM;
-                       goto probe_failure;
-               }
-
-               buf = kasprintf(GFP_KERNEL, "wmi/%s", wdriver->driver.name);
-               if (!buf) {
-                       ret = -ENOMEM;
-                       goto probe_string_failure;
-               }
-               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\n", ret);
-                       ret = -ENOMEM;
-                       goto probe_misc_failure;
+                       return ret;
                }
        }
 
        set_bit(WMI_PROBED, &wblock->flags);
-       return 0;
 
-probe_misc_failure:
-       kfree(buf);
-probe_string_failure:
-       kfree(wblock->handler_data);
-probe_failure:
-       if (ACPI_FAILURE(wmi_method_enable(wblock, false)))
-               dev_warn(dev, "failed to disable device\n");
-       return ret;
+       return 0;
 }
 
 static void wmi_dev_remove(struct device *dev)
@@ -1054,12 +891,6 @@ static void wmi_dev_remove(struct device *dev)
 
        clear_bit(WMI_PROBED, &wblock->flags);
 
-       if (wdriver->filter_callback) {
-               misc_deregister(&wblock->char_dev);
-               kfree(wblock->char_dev.name);
-               kfree(wblock->handler_data);
-       }
-
        if (wdriver->remove)
                wdriver->remove(dev_to_wdev(dev));
 
@@ -1131,7 +962,6 @@ static int wmi_create_device(struct device *wmi_bus_dev,
 
        if (wblock->gblock.flags & ACPI_WMI_METHOD) {
                wblock->dev.dev.type = &wmi_type_method;
-               mutex_init(&wblock->char_mutex);
                goto out_init;
        }
 
index 8a643c39fccedb7a98b7b83f762217f7c23808b0..50f7f1e4fd4f85495acd87b926168d89f9a18ee8 100644 (file)
@@ -11,7 +11,6 @@
 #include <linux/device.h>
 #include <linux/acpi.h>
 #include <linux/mod_devicetable.h>
-#include <uapi/linux/wmi.h>
 
 /**
  * struct wmi_device - WMI device structure
@@ -47,8 +46,6 @@ acpi_status wmidev_block_set(struct wmi_device *wdev, u8 instance, const struct
 
 u8 wmidev_instance_count(struct wmi_device *wdev);
 
-extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);
-
 /**
  * struct wmi_driver - WMI driver structure
  * @driver: Driver model structure
@@ -57,11 +54,8 @@ extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);
  * @probe: Callback for device binding
  * @remove: Callback for device unbinding
  * @notify: Callback for receiving WMI events
- * @filter_callback: Callback for filtering device IOCTLs
  *
  * This represents WMI drivers which handle WMI devices.
- * @filter_callback is only necessary for drivers which
- * want to set up a WMI IOCTL interface.
  */
 struct wmi_driver {
        struct device_driver driver;
@@ -71,8 +65,6 @@ struct wmi_driver {
        int (*probe)(struct wmi_device *wdev, const void *context);
        void (*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,