[PATCH v8 01/15] platform/x86: wmi: Add new method wmidev_evaluate_method

[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 
---
 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, ))
+   return AE_ERROR;
+   return wmidev_evaluate_method(>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
+ * : Buffer containing input for the method call
+ * : 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, ))
-   return AE_ERROR;
-
+   wblock = container_of(wdev, struct wmi_block, dev);
block = >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

[PATCH v8 01/15] platform/x86: wmi: Add new method wmidev_evaluate_method

[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 
---
 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, ))
+   return AE_ERROR;
+   return wmidev_evaluate_method(>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
+ * : Buffer containing input for the method call
+ * : 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, ))
-   return AE_ERROR;
-
+   wblock = container_of(wdev, struct wmi_block, dev);
block = >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

[PATCH v8 04/15] platform/x86: dell-wmi: allow 32k return size in the descriptor

[Mario Limonciello]

Some platforms this year will be adopting 32k WMI buffer, so don't
complain when encountering those.

Signed-off-by: Mario Limonciello 
---
 drivers/platform/x86/dell-wmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index ece2fe341f01..2578dff90a14 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
- * WMI buffer length12   44096
+ * WMI buffer length12   44096 or 32768
  */
 static int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev)
 {
@@ -674,7 +674,7 @@ static int dell_wmi_check_descriptor_buffer(struct 
wmi_device *wdev)
dev_warn(>dev, "Dell descriptor buffer has unknown 
version (%u)\n",
buffer[2]);
 
-   if (buffer[3] != 4096)
+   if (buffer[3] != 4096 && buffer[3] != 32768)
dev_warn(>dev, "Dell descriptor buffer has invalid buffer 
length (%u)\n",
buffer[3]);
 
-- 
2.14.1

[PATCH v8 06/15] platform/x86: wmi: Don't allow drivers to get each other's GUIDs

[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 
---
 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, _wb))
-   return NULL;
-
-   if (other_wb->acpi_device != this_wb->acpi_device)
-   return NULL;
-
-   get_device(_wb->dev.dev);
-   return _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

[PATCH v8 04/15] platform/x86: dell-wmi: allow 32k return size in the descriptor

[Mario Limonciello]

Some platforms this year will be adopting 32k WMI buffer, so don't
complain when encountering those.

Signed-off-by: Mario Limonciello 
---
 drivers/platform/x86/dell-wmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index ece2fe341f01..2578dff90a14 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
- * WMI buffer length12   44096
+ * WMI buffer length12   44096 or 32768
  */
 static int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev)
 {
@@ -674,7 +674,7 @@ static int dell_wmi_check_descriptor_buffer(struct 
wmi_device *wdev)
dev_warn(>dev, "Dell descriptor buffer has unknown 
version (%u)\n",
buffer[2]);
 
-   if (buffer[3] != 4096)
+   if (buffer[3] != 4096 && buffer[3] != 32768)
dev_warn(>dev, "Dell descriptor buffer has invalid buffer 
length (%u)\n",
buffer[3]);
 
-- 
2.14.1

[PATCH v8 06/15] platform/x86: wmi: Don't allow drivers to get each other's GUIDs

[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 
---
 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, _wb))
-   return NULL;
-
-   if (other_wb->acpi_device != this_wb->acpi_device)
-   return NULL;
-
-   get_device(_wb->dev.dev);
-   return _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

[PATCH v8 05/15] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver

[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 
---
 MAINTAINERS|   5 +
 drivers/platform/x86/Kconfig   |   5 +
 drivers/platform/x86/Makefile  |   1 +
 drivers/platform/x86/dell-wmi-descriptor.c | 162 +
 drivers/platform/x86/dell-wmi-descriptor.h |  18 
 drivers/platform/x86/dell-wmi.c|  81 +--
 6 files changed, 194 insertions(+), 78 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 2347e36588dc..f4cf35950b08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4013,6 +4013,11 @@ M:   Pali Rohár 
 S: Maintained
 F: drivers/platform/x86/dell-wmi.c
 
+DELL WMI DESCRIPTOR DRIVER
+M: Mario Limonciello 
+S: Maintained
+F: drivers/platform/x86/dell-wmi-descriptor.c
+
 DELTA ST MEDIA DRIVER
 M: Hugues Fruchet 
 L: linux-me...@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 ..72e317cf0365
--- /dev/null
+++ b/drivers/platform/x86/dell-wmi-descriptor.c
@@ -0,0 +1,162 @@
+/*
+ * 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 
+#include 
+#include 
+#include 
+#include "dell-wmi-descriptor.h"
+
+#define DELL_WMI_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
+
+struct descriptor_priv {
+   struct list_head list;
+   u32 interface_version;
+   u32 size;
+};
+static LIST_HEAD(wmi_list);
+
+bool dell_wmi_get_interface_version(u32 *version)
+{
+   struct descriptor_priv *priv;
+
+   priv = list_first_entry_or_null(_list,
+   struct descriptor_priv,
+   list);
+   if (!priv)
+   return false;
+   *version = priv->interface_version;
+   return true;
+}
+EXPORT_SYMBOL_GPL(dell_wmi_get_interface_version);
+
+bool dell_wmi_get_size(u32 *size)
+{
+   struct descriptor_priv *priv;
+
+   priv = list_first_entry_or_null(_list,
+   struct descriptor_priv,
+   list);
+   if (!priv)
+   return false;
+   *size = priv->size;
+   return true;
+}
+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
+ * WMI buffer length12   

[PATCH v8 05/15] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver

[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 
---
 MAINTAINERS|   5 +
 drivers/platform/x86/Kconfig   |   5 +
 drivers/platform/x86/Makefile  |   1 +
 drivers/platform/x86/dell-wmi-descriptor.c | 162 +
 drivers/platform/x86/dell-wmi-descriptor.h |  18 
 drivers/platform/x86/dell-wmi.c|  81 +--
 6 files changed, 194 insertions(+), 78 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 2347e36588dc..f4cf35950b08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4013,6 +4013,11 @@ M:   Pali Rohár 
 S: Maintained
 F: drivers/platform/x86/dell-wmi.c
 
+DELL WMI DESCRIPTOR DRIVER
+M: Mario Limonciello 
+S: Maintained
+F: drivers/platform/x86/dell-wmi-descriptor.c
+
 DELTA ST MEDIA DRIVER
 M: Hugues Fruchet 
 L: linux-me...@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 ..72e317cf0365
--- /dev/null
+++ b/drivers/platform/x86/dell-wmi-descriptor.c
@@ -0,0 +1,162 @@
+/*
+ * 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 
+#include 
+#include 
+#include 
+#include "dell-wmi-descriptor.h"
+
+#define DELL_WMI_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
+
+struct descriptor_priv {
+   struct list_head list;
+   u32 interface_version;
+   u32 size;
+};
+static LIST_HEAD(wmi_list);
+
+bool dell_wmi_get_interface_version(u32 *version)
+{
+   struct descriptor_priv *priv;
+
+   priv = list_first_entry_or_null(_list,
+   struct descriptor_priv,
+   list);
+   if (!priv)
+   return false;
+   *version = priv->interface_version;
+   return true;
+}
+EXPORT_SYMBOL_GPL(dell_wmi_get_interface_version);
+
+bool dell_wmi_get_size(u32 *size)
+{
+   struct descriptor_priv *priv;
+
+   priv = list_first_entry_or_null(_list,
+   struct descriptor_priv,
+   list);
+   if (!priv)
+   return false;
+   *size = priv->size;
+   return true;
+}
+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
+ * WMI buffer length12   44096 or 32768
+ */
+static int dell_wmi_descriptor_probe(struct wmi_device *wdev)
+{
+   

[PATCH v8 15/15] platform/x86: dell-smbios-wmi: introduce userspace interface

[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 use the character device the buffer needed for the machine will
also be needed.  This information is exported to a sysfs attribute by
the WMI bus in "required_buffer_size".

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.  Use the dell-smbios
filtering functionality to prevent access to certain tokens and calls.

Signed-off-by: Mario Limonciello 
---
 Documentation/ABI/testing/dell-smbios-wmi | 41 
 drivers/platform/x86/dell-smbios-wmi.c| 81 ++-
 drivers/platform/x86/dell-smbios.h| 11 +
 include/uapi/linux/wmi.h  | 26 ++
 4 files changed, 138 insertions(+), 21 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 ..572c3eb53c5c
--- /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 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, look in the device's
+   directory in sysfs for a "required_buffer_size" attribute.
+
+   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 3d68dd100b02..8e189a0bc346 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,66 @@ int dell_smbios_wmi_call(struct calling_interface_buffer 
*buffer)
return ret;
 }
 
+static void _debug_ioctl(struct device *d, unsigned int expected,
+   unsigned int cmd)
+{
+   if (_IOC_DIR(expected) != _IOC_DIR(cmd))
+   dev_dbg(d, "Invalid _IOC_DIR: %d\n", _IOC_DIR(cmd));
+   if (_IOC_TYPE(expected) != _IOC_TYPE(cmd))
+   dev_dbg(d, "Invalid _IOC_TYPE: %d\n", _IOC_TYPE(cmd));
+   if (_IOC_NR(expected) != _IOC_NR(cmd))
+   dev_dbg(d, "Invalid _IOC_NR: %d\n", _IOC_NR(cmd));
+   if (_IOC_SIZE(expected) != _IOC_SIZE(cmd))
+   dev_dbg(d, "Invalid _IOC_SIZE: %d\n", _IOC_SIZE(cmd));
+}
+
+static long dell_smbios_wmi_ioctl(struct wmi_device *wdev, 

[PATCH v8 15/15] platform/x86: dell-smbios-wmi: introduce userspace interface

[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 use the character device the buffer needed for the machine will
also be needed.  This information is exported to a sysfs attribute by
the WMI bus in "required_buffer_size".

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.  Use the dell-smbios
filtering functionality to prevent access to certain tokens and calls.

Signed-off-by: Mario Limonciello 
---
 Documentation/ABI/testing/dell-smbios-wmi | 41 
 drivers/platform/x86/dell-smbios-wmi.c| 81 ++-
 drivers/platform/x86/dell-smbios.h| 11 +
 include/uapi/linux/wmi.h  | 26 ++
 4 files changed, 138 insertions(+), 21 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 ..572c3eb53c5c
--- /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 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, look in the device's
+   directory in sysfs for a "required_buffer_size" attribute.
+
+   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 3d68dd100b02..8e189a0bc346 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,66 @@ int dell_smbios_wmi_call(struct calling_interface_buffer 
*buffer)
return ret;
 }
 
+static void _debug_ioctl(struct device *d, unsigned int expected,
+   unsigned int cmd)
+{
+   if (_IOC_DIR(expected) != _IOC_DIR(cmd))
+   dev_dbg(d, "Invalid _IOC_DIR: %d\n", _IOC_DIR(cmd));
+   if (_IOC_TYPE(expected) != _IOC_TYPE(cmd))
+   dev_dbg(d, "Invalid _IOC_TYPE: %d\n", _IOC_TYPE(cmd));
+   if (_IOC_NR(expected) != _IOC_NR(cmd))
+   dev_dbg(d, "Invalid _IOC_NR: %d\n", _IOC_NR(cmd));
+   if (_IOC_SIZE(expected) != _IOC_SIZE(cmd))
+   dev_dbg(d, "Invalid _IOC_SIZE: %d\n", _IOC_SIZE(cmd));
+}
+
+static long dell_smbios_wmi_ioctl(struct wmi_device *wdev, unsigned int cmd,
+ unsigned 

[PATCH v8 11/15] platform/x86: dell-smbios-smm: test for WSMT

[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 
---
 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 bd5a63a4aa15..5a03a3ea51e0 100644
--- a/drivers/platform/x86/dell-smbios-smm.c
+++ b/drivers/platform/x86/dell-smbios-smm.c
@@ -101,6 +101,31 @@ int dell_smbios_smm_call(struct calling_interface_buffer 
*input)
return 0;
 }
 
+/* When enabled this indicates that SMM won't work */
+static int test_wsmt_enabled(void)
+{
+   struct calling_interface_token *token;
+
+   /* if token doesn't exist, SMM will work */
+   token = dell_smbios_find_token(WSMT_EN_TOKEN);
+   if (!token)
+   return 0;
+
+   /* if token exists, try to access over SMM */
+   buffer->class = CLASS_TOKEN_READ;
+   buffer->select = SELECT_TOKEN_STD;
+   memset(buffer, 0, sizeof(struct calling_interface_buffer));
+   buffer->input[0] = token->location;
+   dell_smbios_smm_call(buffer);
+
+   /* if lookup failed, we know WSMT was enabled */
+   if (buffer->output[0] != 0)
+   return 1;
+
+   /* query token status if it didn't fail */
+   return (buffer->output[1] == token->value);
+}
+
 static int __init dell_smbios_smm_init(void)
 {
int ret;
@@ -114,6 +139,13 @@ static int __init dell_smbios_smm_init(void)
 
dmi_walk(find_cmd_address, NULL);
 
+   ret = test_wsmt_enabled();
+   pr_debug("WSMT enable test: %d\n", ret);
+   if (ret) {
+   ret = -ENODEV;
+   goto fail_wsmt;
+   }
+
platform_device = platform_device_alloc("dell-smbios", 1);
if (!platform_device) {
ret = -ENOMEM;
@@ -137,6 +169,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 8df330abeb5d..db6a16e5f87c 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_DISABLE0x0365
+#define WSMT_EN_TOKEN  0x04EC
+#define WSMT_DIS_TOKEN 0x04ED
 
 struct notifier_block;
 
-- 
2.14.1

[PATCH v8 09/15] platform/x86: dell-smbios: Introduce dispatcher for SMM calls

[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 
---
 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 | 162 +++
 drivers/platform/x86/dell-smbios.c | 120 +++---
 drivers/platform/x86/dell-smbios.h |  46 +-
 drivers/platform/x86/dell-wmi.c|  11 +-
 8 files changed, 397 insertions(+), 247 deletions(-)
 create mode 100644 drivers/platform/x86/dell-smbios-smm.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 09e774f1d0b2..9dc1ee9603e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3985,6 +3985,12 @@ L:   platform-driver-...@vger.kernel.org
 S: Maintained
 F: drivers/platform/x86/dell-smbios.*
 
+DELL SMBIOS SMM DRIVER
+M: Mario Limonciello 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/dell-smbios-smm.c
+
 DELL LAPTOP DRIVER
 M: Matthew Garrett 
 M: Pali Rohár 
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..39581dd8abbe 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->class = class;
+   buffer->select = select;
+   ret = dell_smbios_call(buffer);
+ 

[PATCH v8 07/15] platform/x86: dell-smbios: only run if proper oem string is detected

[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 
---
 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 e9b1ca07c872..7e779278d054 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

[PATCH v8 11/15] platform/x86: dell-smbios-smm: test for WSMT

[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 
---
 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 bd5a63a4aa15..5a03a3ea51e0 100644
--- a/drivers/platform/x86/dell-smbios-smm.c
+++ b/drivers/platform/x86/dell-smbios-smm.c
@@ -101,6 +101,31 @@ int dell_smbios_smm_call(struct calling_interface_buffer 
*input)
return 0;
 }
 
+/* When enabled this indicates that SMM won't work */
+static int test_wsmt_enabled(void)
+{
+   struct calling_interface_token *token;
+
+   /* if token doesn't exist, SMM will work */
+   token = dell_smbios_find_token(WSMT_EN_TOKEN);
+   if (!token)
+   return 0;
+
+   /* if token exists, try to access over SMM */
+   buffer->class = CLASS_TOKEN_READ;
+   buffer->select = SELECT_TOKEN_STD;
+   memset(buffer, 0, sizeof(struct calling_interface_buffer));
+   buffer->input[0] = token->location;
+   dell_smbios_smm_call(buffer);
+
+   /* if lookup failed, we know WSMT was enabled */
+   if (buffer->output[0] != 0)
+   return 1;
+
+   /* query token status if it didn't fail */
+   return (buffer->output[1] == token->value);
+}
+
 static int __init dell_smbios_smm_init(void)
 {
int ret;
@@ -114,6 +139,13 @@ static int __init dell_smbios_smm_init(void)
 
dmi_walk(find_cmd_address, NULL);
 
+   ret = test_wsmt_enabled();
+   pr_debug("WSMT enable test: %d\n", ret);
+   if (ret) {
+   ret = -ENODEV;
+   goto fail_wsmt;
+   }
+
platform_device = platform_device_alloc("dell-smbios", 1);
if (!platform_device) {
ret = -ENOMEM;
@@ -137,6 +169,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 8df330abeb5d..db6a16e5f87c 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_DISABLE0x0365
+#define WSMT_EN_TOKEN  0x04EC
+#define WSMT_DIS_TOKEN 0x04ED
 
 struct notifier_block;
 
-- 
2.14.1

[PATCH v8 09/15] platform/x86: dell-smbios: Introduce dispatcher for SMM calls

[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 
---
 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 | 162 +++
 drivers/platform/x86/dell-smbios.c | 120 +++---
 drivers/platform/x86/dell-smbios.h |  46 +-
 drivers/platform/x86/dell-wmi.c|  11 +-
 8 files changed, 397 insertions(+), 247 deletions(-)
 create mode 100644 drivers/platform/x86/dell-smbios-smm.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 09e774f1d0b2..9dc1ee9603e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3985,6 +3985,12 @@ L:   platform-driver-...@vger.kernel.org
 S: Maintained
 F: drivers/platform/x86/dell-smbios.*
 
+DELL SMBIOS SMM DRIVER
+M: Mario Limonciello 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/dell-smbios-smm.c
+
 DELL LAPTOP DRIVER
 M: Matthew Garrett 
 M: Pali Rohár 
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..39581dd8abbe 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->class = class;
+   buffer->select = select;
+   ret = dell_smbios_call(buffer);
+   if (ret != 0)
+   return ret;
+   return dell_smbios_error(buffer->output[0]);

[PATCH v8 07/15] platform/x86: dell-smbios: only run if proper oem string is detected

[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 
---
 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 e9b1ca07c872..7e779278d054 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

[PATCH v8 08/15] platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens

[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 
---
 .../ABI/testing/sysfs-platform-dell-smbios |  21 ++
 MAINTAINERS|   7 +
 drivers/platform/x86/dell-smbios.c | 211 -
 3 files changed, 238 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 ..205d3b6361e0
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-dell-smbios
@@ -0,0 +1,21 @@
+What:  /sys/devices/platform//tokens/*
+Date:  November 2017
+KernelVersion: 4.15
+Contact:   "Mario Limonciello" 
+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 f4cf35950b08..09e774f1d0b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3978,6 +3978,13 @@ M:   "Maciej W. Rozycki" 
 S: Maintained
 F: drivers/net/fddi/defxx.*
 
+DELL SMBIOS DRIVER
+M: Pali Rohár 
+M: Mario Limonciello 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/dell-smbios.*
+
 DELL LAPTOP DRIVER
 M: Matthew Garrett 
 M: Pali Rohár 
diff --git a/drivers/platform/x86/dell-smbios.c 
b/drivers/platform/x86/dell-smbios.c
index 7e779278d054..eb973ba4f912 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include "../../firmware/dcdbas.h"
+#include 
 #include "dell-smbios.h"
 
 struct calling_interface_structure {
@@ -39,7 +40,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 +162,26 @@ 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++) {
+   for (j = i+1; j < da_num_tokens; j++) {
+   if (da_tokens[i].tokenID == 0 ||
+   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 +195,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", 

[PATCH v8 08/15] platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens

[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 
---
 .../ABI/testing/sysfs-platform-dell-smbios |  21 ++
 MAINTAINERS|   7 +
 drivers/platform/x86/dell-smbios.c | 211 -
 3 files changed, 238 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 ..205d3b6361e0
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-dell-smbios
@@ -0,0 +1,21 @@
+What:  /sys/devices/platform//tokens/*
+Date:  November 2017
+KernelVersion: 4.15
+Contact:   "Mario Limonciello" 
+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 f4cf35950b08..09e774f1d0b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3978,6 +3978,13 @@ M:   "Maciej W. Rozycki" 
 S: Maintained
 F: drivers/net/fddi/defxx.*
 
+DELL SMBIOS DRIVER
+M: Pali Rohár 
+M: Mario Limonciello 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/dell-smbios.*
+
 DELL LAPTOP DRIVER
 M: Matthew Garrett 
 M: Pali Rohár 
diff --git a/drivers/platform/x86/dell-smbios.c 
b/drivers/platform/x86/dell-smbios.c
index 7e779278d054..eb973ba4f912 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include "../../firmware/dcdbas.h"
+#include 
 #include "dell-smbios.h"
 
 struct calling_interface_structure {
@@ -39,7 +40,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 +162,26 @@ 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++) {
+   for (j = i+1; j < da_num_tokens; j++) {
+   if (da_tokens[i].tokenID == 0 ||
+   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 +195,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)
+{
+

[PATCH v8 03/15] platform/x86: dell-wmi: clean up wmi descriptor check

[Mario Limonciello]

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

Signed-off-by: Mario Limonciello 
Reviewed-by: Edward O'Callaghan 
Suggested-by: Andy Shevchenko 
---
 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..ece2fe341f01 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -663,19 +663,19 @@ 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(>dev, "Dell descriptor buffer has invalid 
signature (%*ph)\n",
-   8, buffer);
+   if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
+   dev_err(>dev, "Dell descriptor buffer has invalid 
signature (%8ph)\n",
+   buffer);
ret = -EINVAL;
goto out;
}
 
if (buffer[2] != 0 && buffer[2] != 1)
-   dev_warn(>dev, "Dell descriptor buffer has unknown 
version (%d)\n",
+   dev_warn(>dev, "Dell descriptor buffer has unknown 
version (%u)\n",
buffer[2]);
 
if (buffer[3] != 4096)
-   dev_warn(>dev, "Dell descriptor buffer has invalid buffer 
length (%d)\n",
+   dev_warn(>dev, "Dell descriptor buffer has invalid buffer 
length (%u)\n",
buffer[3]);
 
priv->interface_version = buffer[2];
-- 
2.14.1

[PATCH v8 03/15] platform/x86: dell-wmi: clean up wmi descriptor check

[Mario Limonciello]

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

Signed-off-by: Mario Limonciello 
Reviewed-by: Edward O'Callaghan 
Suggested-by: Andy Shevchenko 
---
 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..ece2fe341f01 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -663,19 +663,19 @@ 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(>dev, "Dell descriptor buffer has invalid 
signature (%*ph)\n",
-   8, buffer);
+   if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
+   dev_err(>dev, "Dell descriptor buffer has invalid 
signature (%8ph)\n",
+   buffer);
ret = -EINVAL;
goto out;
}
 
if (buffer[2] != 0 && buffer[2] != 1)
-   dev_warn(>dev, "Dell descriptor buffer has unknown 
version (%d)\n",
+   dev_warn(>dev, "Dell descriptor buffer has unknown 
version (%u)\n",
buffer[2]);
 
if (buffer[3] != 4096)
-   dev_warn(>dev, "Dell descriptor buffer has invalid buffer 
length (%d)\n",
+   dev_warn(>dev, "Dell descriptor buffer has invalid buffer 
length (%u)\n",
buffer[3]);
 
priv->interface_version = buffer[2];
-- 
2.14.1

[PATCH v8 10/15] platform/x86: dell-smbios-wmi: Add new WMI dispatcher driver

[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 
---
 MAINTAINERS|   6 +
 drivers/platform/x86/Kconfig   |  14 ++
 drivers/platform/x86/Makefile  |   1 +
 drivers/platform/x86/dell-smbios-wmi.c | 233 +
 4 files changed, 254 insertions(+)
 create mode 100644 drivers/platform/x86/dell-smbios-wmi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9dc1ee9603e7..8fd2f0d2ddf6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3991,6 +3991,12 @@ L:   platform-driver-...@vger.kernel.org
 S: Maintained
 F: drivers/platform/x86/dell-smbios-smm.c
 
+DELL SMBIOS WMI DRIVER
+M: Mario Limonciello 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/dell-smbios-wmi.c
+
 DELL LAPTOP DRIVER
 M: Matthew Garrett 
 M: Pali Rohár 
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 ..3d68dd100b02
--- /dev/null
+++ b/drivers/platform/x86/dell-smbios-wmi.c
@@ -0,0 +1,233 @@
+/*
+ *  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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#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(_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(>dev);
+   input.length = 

[PATCH v8 10/15] platform/x86: dell-smbios-wmi: Add new WMI dispatcher driver

[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 
---
 MAINTAINERS|   6 +
 drivers/platform/x86/Kconfig   |  14 ++
 drivers/platform/x86/Makefile  |   1 +
 drivers/platform/x86/dell-smbios-wmi.c | 233 +
 4 files changed, 254 insertions(+)
 create mode 100644 drivers/platform/x86/dell-smbios-wmi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9dc1ee9603e7..8fd2f0d2ddf6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3991,6 +3991,12 @@ L:   platform-driver-...@vger.kernel.org
 S: Maintained
 F: drivers/platform/x86/dell-smbios-smm.c
 
+DELL SMBIOS WMI DRIVER
+M: Mario Limonciello 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/dell-smbios-wmi.c
+
 DELL LAPTOP DRIVER
 M: Matthew Garrett 
 M: Pali Rohár 
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 ..3d68dd100b02
--- /dev/null
+++ b/drivers/platform/x86/dell-smbios-wmi.c
@@ -0,0 +1,233 @@
+/*
+ *  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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#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(_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(>dev);
+   input.length = priv->req_buf_size - sizeof(u64);
+   input.pointer = >buf->std;
+
+   dev_dbg(>dev, "evaluating: 

[PATCH v8 14/15] platform/x86: wmi: create userspace interface for drivers

[Mario Limonciello]

For WMI operations that are only Set or Query read or write sysfs
attributes created by WMI vendor drivers make sense.

For other WMI operations that are run on Method, there needs to be a
way to guarantee to userspace that the results from the method call
belong to the data request to the method call.  Sysfs attributes don't
work well in this scenario because two userspace processes may be
competing at reading/writing an attribute and step on each other's
data.

When a WMI vendor driver declares an ioctl callback in the wmi_driver
the WMI bus driver will create a character device that maps to that
function.

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

The WMI bus driver will interpret the IOCTL calls, test them for
a valid instance and pass them on to the vendor driver to run.

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

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

When a WMI vendor driver is unloaded the WMI bus driver will clean
up the character device.

Signed-off-by: Mario Limonciello 
---
 MAINTAINERS|   1 +
 drivers/platform/x86/wmi.c | 116 +
 include/linux/wmi.h|   6 +++
 include/uapi/linux/wmi.h   |  26 ++
 4 files changed, 149 insertions(+)
 create mode 100644 include/uapi/linux/wmi.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 8fd2f0d2ddf6..84afbf8ef7d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -384,6 +384,7 @@ ACPI WMI DRIVER
 L: platform-driver-...@vger.kernel.org
 S: Orphan
 F: drivers/platform/x86/wmi.c
+F: include/uapi/linux/wmi.h
 
 AD1889 ALSA SOUND DRIVER
 M: Thibaut Varene 
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 63d01f98bf4c..d1742d392cd1 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -38,12 +38,15 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 
 ACPI_MODULE_NAME("wmi");
 MODULE_AUTHOR("Carlos Corbacho");
@@ -69,6 +72,7 @@ struct wmi_block {
struct wmi_device dev;
struct list_head list;
struct guid_block gblock;
+   struct miscdevice misc_dev;
struct acpi_device *acpi_device;
wmi_notify_handler handler;
void *handler_data;
@@ -796,12 +800,119 @@ static int wmi_dev_match(struct device *dev, struct 
device_driver *driver)
return 0;
 }
 
+static long match_ioctl(struct file *filp, unsigned int cmd, unsigned long arg,
+   int compat)
+{
+   struct wmi_ioctl_buffer __user *input =
+   (struct wmi_ioctl_buffer __user *) arg;
+   struct wmi_driver *wdriver = NULL;
+   struct wmi_block *wblock = NULL;
+   struct wmi_block *next = NULL;
+   const char *driver_name;
+   u64 size;
+   int ret;
+
+   if (_IOC_TYPE(cmd) != WMI_IOC)
+   return -ENOTTY;
+
+   driver_name = filp->f_path.dentry->d_iname;
+
+   list_for_each_entry_safe(wblock, next, _block_list, list) {
+   wdriver = container_of(wblock->dev.dev.driver,
+   struct wmi_driver, driver);
+   if (!wdriver)
+   continue;
+   if (strcmp(driver_name, wdriver->driver.name) == 0)
+   break;
+   }
+
+   if (!wdriver)
+   return -ENODEV;
+
+   /* make sure we're not calling a higher instance than exists*/
+   if (_IOC_NR(cmd) >= wblock->gblock.instance_count)
+   return -EINVAL;
+
+   /* check that required buffer size was declared by driver */
+   if (!wblock->req_buf_size) {
+   dev_err(>dev.dev, "Required buffer size not set\n");
+   return -EINVAL;
+   }
+   if (get_user(size, >length)) {
+   dev_dbg(>dev.dev, "Read length from user failed\n");
+   return -EFAULT;
+   }
+   /* if it's too small, abort */
+   if (size < wblock->req_buf_size) {
+   dev_err(>dev.dev,
+   "Buffer %lld too small, need at least %lld\n",
+   size, wblock->req_buf_size);
+   return -EINVAL;
+   }
+   /* if it's too big, warn, driver will only use what is needed */
+   if (size > wblock->req_buf_size)
+   dev_warn(>dev.dev,
+   "Buffer %lld is bigger than required %lld\n",
+   size, wblock->req_buf_size);
+
+   if (!try_module_get(wdriver->driver.owner))
+   return -EBUSY;
+   if (compat) {
+   if (wdriver->compat_ioctl)
+   ret = wdriver->compat_ioctl(>dev, cmd, 

[PATCH v8 13/15] platform/x86: wmi: Add sysfs attribute for required_buffer_size

[Mario Limonciello]

Method type WMI objects need to be able to describe the size of
the interface that they will expect to use.

Export this information to sysfs and allow vendor drivers to
set it.

Signed-off-by: Mario Limonciello 
---
 drivers/platform/x86/wmi.c | 31 +++
 include/linux/wmi.h|  3 +++
 2 files changed, 34 insertions(+)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index bcb41c1c7f52..63d01f98bf4c 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -72,6 +72,7 @@ struct wmi_block {
struct acpi_device *acpi_device;
wmi_notify_handler handler;
void *handler_data;
+   u64 req_buf_size;
 
bool read_takes_no_args;
 };
@@ -188,6 +189,26 @@ 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, u8 instance, u64 length)
+{
+   struct wmi_block *wblock;
+
+   wblock = container_of(wdev, struct wmi_block, dev);
+   if (!wblock)
+   return -ENODEV;
+   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
@@ -718,8 +739,18 @@ static struct attribute *wmi_data_attrs[] = {
 };
 ATTRIBUTE_GROUPS(wmi_data);
 
+static ssize_t required_buffer_size_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+   struct wmi_block *wblock = dev_to_wblock(dev);
+
+   return sprintf(buf, "%lld\n", wblock->req_buf_size);
+}
+static DEVICE_ATTR_RO(required_buffer_size);
+
 static struct attribute *wmi_method_attrs[] = {
_attr_object_id.attr,
+   _attr_required_buffer_size.attr,
NULL,
 };
 ATTRIBUTE_GROUPS(wmi_method);
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index ddee427e0721..a9a72a4c5ed8 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -36,6 +36,9 @@ 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, u8 instance,
+   u64 length);
+
 struct wmi_device_id {
const char *guid_string;
 };
-- 
2.14.1

[PATCH v8 12/15] platform/x86: dell-smbios: Add filtering support

[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 
---
 drivers/platform/x86/dell-smbios.c | 190 +
 drivers/platform/x86/dell-smbios.h |  11 +++
 2 files changed, 201 insertions(+)

diff --git a/drivers/platform/x86/dell-smbios.c 
b/drivers/platform/x86/dell-smbios.c
index b1939abc4216..4aede844de11 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -24,6 +24,7 @@
 #include 
 #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 class;
+   int 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[] = {
+   {0x, 01, 07}, /* manufacturing use */
+   {0x, 06, 05}, /* manufacturing use */
+   {0x, 11, 03}, /* write once */
+   {0x, 11, 07}, /* write once */
+   {0x, 11, 11}, /* write once */
+   {0x, 19, -1}, /* diagnostics */
+   /* handled by kernel: dell-laptop */
+   {0x, CLASS_INFO, SELECT_RFKILL},
+   {0x, 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 */
+   {0x,WSMT_EN_TOKEN,  WSMT_DIS_TOKEN}
+};
+
+/* tokens that are explicitly blacklisted */
+static struct token_range token_blacklist[] = {
+   {0x, 0x0058, 0x0059}, /* ME use */
+   {0x, 0x00CD, 0x00D0}, /* raid shadow copy */
+   {0x, 0x013A, 0x01FF}, /* sata shadow copy */
+   {0x, 0x0175, 0x0176}, /* write once */
+   {0x, 0x0195, 0x0197}, /* diagnostics */
+   {0x, 0x01DC, 0x01DD}, /* manufacturing use */
+   {0x, 0x027D, 0x0284}, /* diagnostics */
+   {0x, 0x02E3, 0x02E3}, /* manufacturing use */
+   {0x, 0x02FF, 0x02FF}, /* manufacturing use */
+   {0x, 0x0300, 0x0302}, /* manufacturing use */
+   {0x, 0x0325, 0x0326}, /* manufacturing use */
+   {0x, 0x0332, 0x0335}, /* fan control */
+   {0x, 0x0350, 0x0350}, /* manufacturing use */
+   {0x, 0x0363, 0x0363}, /* manufacturing use */
+   {0x, 0x0368, 0x0368}, /* manufacturing use */
+   {0x, 0x03F6, 0x03F7}, /* manufacturing use */
+   {0x, 0x049E, 0x049F}, /* manufacturing use */
+   {0x, 0x04A0, 0x04A3}, /* disagnostics */
+   {0x, 0x04E6, 0x04E7}, /* manufacturing use */
+   {0x, 0x4000, 0x7FFF}, /* internal BIOS use */
+   {0x, 0x9000, 0x9001}, /* internal BIOS use */
+   {0x, 0xA000, 0xBFFF}, /* write only */
+   {0x, 0xEFF0, 0xEFFF}, /* internal BIOS use */
+   /* handled by kernel: dell-laptop */
+   

[PATCH v8 14/15] platform/x86: wmi: create userspace interface for drivers

[Mario Limonciello]

For WMI operations that are only Set or Query read or write sysfs
attributes created by WMI vendor drivers make sense.

For other WMI operations that are run on Method, there needs to be a
way to guarantee to userspace that the results from the method call
belong to the data request to the method call.  Sysfs attributes don't
work well in this scenario because two userspace processes may be
competing at reading/writing an attribute and step on each other's
data.

When a WMI vendor driver declares an ioctl callback in the wmi_driver
the WMI bus driver will create a character device that maps to that
function.

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

The WMI bus driver will interpret the IOCTL calls, test them for
a valid instance and pass them on to the vendor driver to run.

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

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

When a WMI vendor driver is unloaded the WMI bus driver will clean
up the character device.

Signed-off-by: Mario Limonciello 
---
 MAINTAINERS|   1 +
 drivers/platform/x86/wmi.c | 116 +
 include/linux/wmi.h|   6 +++
 include/uapi/linux/wmi.h   |  26 ++
 4 files changed, 149 insertions(+)
 create mode 100644 include/uapi/linux/wmi.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 8fd2f0d2ddf6..84afbf8ef7d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -384,6 +384,7 @@ ACPI WMI DRIVER
 L: platform-driver-...@vger.kernel.org
 S: Orphan
 F: drivers/platform/x86/wmi.c
+F: include/uapi/linux/wmi.h
 
 AD1889 ALSA SOUND DRIVER
 M: Thibaut Varene 
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 63d01f98bf4c..d1742d392cd1 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -38,12 +38,15 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 
 ACPI_MODULE_NAME("wmi");
 MODULE_AUTHOR("Carlos Corbacho");
@@ -69,6 +72,7 @@ struct wmi_block {
struct wmi_device dev;
struct list_head list;
struct guid_block gblock;
+   struct miscdevice misc_dev;
struct acpi_device *acpi_device;
wmi_notify_handler handler;
void *handler_data;
@@ -796,12 +800,119 @@ static int wmi_dev_match(struct device *dev, struct 
device_driver *driver)
return 0;
 }
 
+static long match_ioctl(struct file *filp, unsigned int cmd, unsigned long arg,
+   int compat)
+{
+   struct wmi_ioctl_buffer __user *input =
+   (struct wmi_ioctl_buffer __user *) arg;
+   struct wmi_driver *wdriver = NULL;
+   struct wmi_block *wblock = NULL;
+   struct wmi_block *next = NULL;
+   const char *driver_name;
+   u64 size;
+   int ret;
+
+   if (_IOC_TYPE(cmd) != WMI_IOC)
+   return -ENOTTY;
+
+   driver_name = filp->f_path.dentry->d_iname;
+
+   list_for_each_entry_safe(wblock, next, _block_list, list) {
+   wdriver = container_of(wblock->dev.dev.driver,
+   struct wmi_driver, driver);
+   if (!wdriver)
+   continue;
+   if (strcmp(driver_name, wdriver->driver.name) == 0)
+   break;
+   }
+
+   if (!wdriver)
+   return -ENODEV;
+
+   /* make sure we're not calling a higher instance than exists*/
+   if (_IOC_NR(cmd) >= wblock->gblock.instance_count)
+   return -EINVAL;
+
+   /* check that required buffer size was declared by driver */
+   if (!wblock->req_buf_size) {
+   dev_err(>dev.dev, "Required buffer size not set\n");
+   return -EINVAL;
+   }
+   if (get_user(size, >length)) {
+   dev_dbg(>dev.dev, "Read length from user failed\n");
+   return -EFAULT;
+   }
+   /* if it's too small, abort */
+   if (size < wblock->req_buf_size) {
+   dev_err(>dev.dev,
+   "Buffer %lld too small, need at least %lld\n",
+   size, wblock->req_buf_size);
+   return -EINVAL;
+   }
+   /* if it's too big, warn, driver will only use what is needed */
+   if (size > wblock->req_buf_size)
+   dev_warn(>dev.dev,
+   "Buffer %lld is bigger than required %lld\n",
+   size, wblock->req_buf_size);
+
+   if (!try_module_get(wdriver->driver.owner))
+   return -EBUSY;
+   if (compat) {
+   if (wdriver->compat_ioctl)
+   ret = wdriver->compat_ioctl(>dev, cmd, arg);
+   else
+   

[PATCH v8 13/15] platform/x86: wmi: Add sysfs attribute for required_buffer_size

[Mario Limonciello]

Method type WMI objects need to be able to describe the size of
the interface that they will expect to use.

Export this information to sysfs and allow vendor drivers to
set it.

Signed-off-by: Mario Limonciello 
---
 drivers/platform/x86/wmi.c | 31 +++
 include/linux/wmi.h|  3 +++
 2 files changed, 34 insertions(+)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index bcb41c1c7f52..63d01f98bf4c 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -72,6 +72,7 @@ struct wmi_block {
struct acpi_device *acpi_device;
wmi_notify_handler handler;
void *handler_data;
+   u64 req_buf_size;
 
bool read_takes_no_args;
 };
@@ -188,6 +189,26 @@ 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, u8 instance, u64 length)
+{
+   struct wmi_block *wblock;
+
+   wblock = container_of(wdev, struct wmi_block, dev);
+   if (!wblock)
+   return -ENODEV;
+   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
@@ -718,8 +739,18 @@ static struct attribute *wmi_data_attrs[] = {
 };
 ATTRIBUTE_GROUPS(wmi_data);
 
+static ssize_t required_buffer_size_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+   struct wmi_block *wblock = dev_to_wblock(dev);
+
+   return sprintf(buf, "%lld\n", wblock->req_buf_size);
+}
+static DEVICE_ATTR_RO(required_buffer_size);
+
 static struct attribute *wmi_method_attrs[] = {
_attr_object_id.attr,
+   _attr_required_buffer_size.attr,
NULL,
 };
 ATTRIBUTE_GROUPS(wmi_method);
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index ddee427e0721..a9a72a4c5ed8 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -36,6 +36,9 @@ 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, u8 instance,
+   u64 length);
+
 struct wmi_device_id {
const char *guid_string;
 };
-- 
2.14.1

[PATCH v8 12/15] platform/x86: dell-smbios: Add filtering support

[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 
---
 drivers/platform/x86/dell-smbios.c | 190 +
 drivers/platform/x86/dell-smbios.h |  11 +++
 2 files changed, 201 insertions(+)

diff --git a/drivers/platform/x86/dell-smbios.c 
b/drivers/platform/x86/dell-smbios.c
index b1939abc4216..4aede844de11 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -24,6 +24,7 @@
 #include 
 #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 class;
+   int 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[] = {
+   {0x, 01, 07}, /* manufacturing use */
+   {0x, 06, 05}, /* manufacturing use */
+   {0x, 11, 03}, /* write once */
+   {0x, 11, 07}, /* write once */
+   {0x, 11, 11}, /* write once */
+   {0x, 19, -1}, /* diagnostics */
+   /* handled by kernel: dell-laptop */
+   {0x, CLASS_INFO, SELECT_RFKILL},
+   {0x, 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 */
+   {0x,WSMT_EN_TOKEN,  WSMT_DIS_TOKEN}
+};
+
+/* tokens that are explicitly blacklisted */
+static struct token_range token_blacklist[] = {
+   {0x, 0x0058, 0x0059}, /* ME use */
+   {0x, 0x00CD, 0x00D0}, /* raid shadow copy */
+   {0x, 0x013A, 0x01FF}, /* sata shadow copy */
+   {0x, 0x0175, 0x0176}, /* write once */
+   {0x, 0x0195, 0x0197}, /* diagnostics */
+   {0x, 0x01DC, 0x01DD}, /* manufacturing use */
+   {0x, 0x027D, 0x0284}, /* diagnostics */
+   {0x, 0x02E3, 0x02E3}, /* manufacturing use */
+   {0x, 0x02FF, 0x02FF}, /* manufacturing use */
+   {0x, 0x0300, 0x0302}, /* manufacturing use */
+   {0x, 0x0325, 0x0326}, /* manufacturing use */
+   {0x, 0x0332, 0x0335}, /* fan control */
+   {0x, 0x0350, 0x0350}, /* manufacturing use */
+   {0x, 0x0363, 0x0363}, /* manufacturing use */
+   {0x, 0x0368, 0x0368}, /* manufacturing use */
+   {0x, 0x03F6, 0x03F7}, /* manufacturing use */
+   {0x, 0x049E, 0x049F}, /* manufacturing use */
+   {0x, 0x04A0, 0x04A3}, /* disagnostics */
+   {0x, 0x04E6, 0x04E7}, /* manufacturing use */
+   {0x, 0x4000, 0x7FFF}, /* internal BIOS use */
+   {0x, 0x9000, 0x9001}, /* internal BIOS use */
+   {0x, 0xA000, 0xBFFF}, /* write only */
+   {0x, 0xEFF0, 0xEFFF}, /* internal BIOS use */
+   /* handled by kernel: dell-laptop */
+   {0x, BRIGHTNESS_TOKEN,   

[PATCH v8 00/15] Introduce support for Dell SMBIOS over WMI

[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/superm1/linux/tree/wmi-smbios
changes bweteen v7 and v8:
 * fix typo s/desc_buffer/buffer/ in 32k split commit
 * Add missing include for  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.
 * 

[PATCH v8 00/15] Introduce support for Dell SMBIOS over WMI

[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/superm1/linux/tree/wmi-smbios
changes bweteen v7 and v8:
 * fix typo s/desc_buffer/buffer/ in 32k split commit
 * Add missing include for  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.
 * 

[PATCH v8 02/15] platform/x86: dell-wmi: increase severity of some failures

[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 
---
 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(>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(>dev, "Dell descriptor buffer has invalid 
signature (%*ph)\n",
+   if (buffer[0] != 0x4C4C4544 && buffer[1] != 0x494D5720) {
+   dev_err(>dev, "Dell descriptor buffer has invalid 
signature (%*ph)\n",
8, buffer);
+   ret = -EINVAL;
+   goto out;
+   }
 
if (buffer[2] != 0 && buffer[2] != 1)
dev_warn(>dev, "Dell descriptor buffer has unknown 
version (%d)\n",
-- 
2.14.1

[PATCH v8 02/15] platform/x86: dell-wmi: increase severity of some failures

[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 
---
 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(>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(>dev, "Dell descriptor buffer has invalid 
signature (%*ph)\n",
+   if (buffer[0] != 0x4C4C4544 && buffer[1] != 0x494D5720) {
+   dev_err(>dev, "Dell descriptor buffer has invalid 
signature (%*ph)\n",
8, buffer);
+   ret = -EINVAL;
+   goto out;
+   }
 
if (buffer[2] != 0 && buffer[2] != 1)
dev_warn(>dev, "Dell descriptor buffer has unknown 
version (%d)\n",
-- 
2.14.1

Re: 4.13 regression: get_kctl_0dB_offset doesn't handle all possible callbacks

[Takashi Sakamoto]

Hi,

On Oct 14 2017 07:46, PaX Team wrote:
> what KERNEXEC on i386 does is that it executes kernel code in its own 0-based
> code segment hence the 'low' code addresses. due to the current logic that
> checks for SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK in get_kctl_0dB_offset, this
> callback address is instead treated as a data pointer (as apparently
> SNDRV_CTL_ELEM_ACCESS_TLV_READ is also set) and since it's not a valid kernel
> address, it causes a GPF under the UDEREF PaX feature (without UDEREF it'd
> have been an oops since such low addresses are not mapped for kernel threads).

There're two ways to use 'struct snd_kcontrol.tlv'; constant array (=.p) an
a handler (= .c). An 'access' flag in each member of
'struct snd_kcontrol.vd' represent which way is used for the member.
Therefore, as long as checking the flag, the 'get_kctl_0dB_offset()' doesn't
handle a pointer to the handler as a pointer to array of TLV container.

> on vanilla kernels all this is a silent read of kernel code bytes that are
> then interpreted as the tlv[] array content, which is probably not what you
> want either.

Yes. Current code include a bug of inappropriate condition statement for the
above. As a result, it allows to handle a pointer to the handler as a
pointer to TLV data. 

> as for fixing this, first the above mentioned assumption should be 
> re-evaluated.
> if it's considered correct then there is some logic bug in my case (i can help
> debug it if you tell me what to do) otherwise the current pattern of
> 
> if (SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK && callback == 
> snd_hda_mixer_amp_tlv) {
>   call wrapped function underneath snd_hda_mixer_amp_tlv
> } else if (SNDRV_CTL_ELEM_ACCESS_TLV_READ) {
>   treat unrecognized callback address as data ptr
> }
> 
> should be changed to
> 
> if (SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK( {
>   if (callback == snd_hda_mixer_amp_tlv) {
> call wrapped function underneath snd_hda_mixer_amp_tlv
>   } else if (callback == others) {
> handle others, WARN/BUG/etc
>   }
> } else if (SNDRV_CTL_ELEM_ACCESS_TLV_READ) {
>   no longer treat unrecognized callback address as data ptr
> }

Good enough as a solution. Please test a patch in the end of this message.

> and all other callbacks with userland access should be refactored the same
> way as snd_hda_mixer_amp_tlv was. i'd also suggest that you at least do the
> above suggested if/else pattern change in order to prevent the misuse of
> unexpected callbacks in the future.

This suggestion is better for safety. Do you have some ways to detect the
pattern on current vanilla kernel? Or we should find it by eye-grep?


Thanks

Takashi Sakamoto

 8< 

>From 85896b50aa22bf2f2b5e45456daa16d386602edc Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto 
Date: Sat, 14 Oct 2017 14:08:51 +0900
Subject: [PATCH] ALSA: hda-intel: Fix to handle a pointer to TLV handler
 as a pointer to TLV data

In a design of ALSA control core, an 'access' flag on each entry of
'struct snd_kcontrol.vd' represents the type of 'struct snd_kcontrol.tlv'
for the corresponding element; a pointer to TLV data
(!=SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) or a pointer to handler
(==SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK).

In current implementation of 'get_kctl_0dB_offset()', condition statement
is not proper to distinguish these two cases. As a result, it handles
a pointer to the handler as a pointer to TLV data for some control element
sets. This bug brings invalid references to kernel space. A reporter shows
a sample of backtrace.

PAX: suspicious general protection fault:  [#1] PREEMPT SMP
Modules linked in:
CPU: 4 PID: 31 Comm: kworker/4:0 Not tainted 4.13.5-i386-pax #17
Workqueue: events azx_probe_work
task: eb61c880 task.stack: eb62a000
EIP: 0060:[<009bbd2d>] init_slave_0dB+0x2d/0xa0
EFLAGS: 00210202 CPU: 4
EAX: 00991fd0 EBX: e9883a80 ECX: e9883a80 EDX: eb62bd74
ESI: eb62bd74 EDI: e9098800 EBP: eb62bd08 ESP: eb62bcec
 DS: 0068 ES: 0068 FS: 00d8 GS: 0068 SS: 0068
CR0: 80050033 CR2:  CR3: 03b2d100 CR4: 000406f0 shadow CR4: 000406f0
Call Trace:
 [<009bb469>] map_slaves+0xb9/0xe0
 [<009bce0e>] __snd_hda_add_vmaster+0xde/0x110
 [<009c82f0>] snd_hda_gen_build_controls+0x1a0/0x1b0
 [<009d0a4d>] cx_auto_build_controls+0xd/0x70
 [<009bdf76>] snd_hda_codec_build_controls+0x186/0x1d0
 [<009b92ad>] hda_codec_driver_probe+0x6d/0xf0
 [<006a3be9>] driver_probe_device+0x289/0x420
 [<006a3ed6>] __device_attach_driver+0x76/0x100
 [<006a1edf>] bus_for_each_drv+0x3f/0x70
 [<006a3813>] __device_attach+0xa3/0x110
 [<006a3f9d>] device_initial_probe+0xd/0x10
 [<006a2d6f>] bus_probe_device+0x6f/0x80
 [<006a100f>] device_add+0x2cf/0x590
 [<009d8d8c>] snd_hdac_device_register+0xc/0x40
 [<009b90b4>] snd_hda_codec_configure+0x34/0x140
 [<009c2875>] azx_codec_configure+0x25/0x50
 [<009d8081>] azx_probe_continue+0x621/0x9e0
 [<009d84bd>] azx_probe_work+0xd/0x10
 [<0006fff2>] process_one_work+0x122/0x2a0
 

Re: 4.13 regression: get_kctl_0dB_offset doesn't handle all possible callbacks

[Takashi Sakamoto]

Hi,

On Oct 14 2017 07:46, PaX Team wrote:
> what KERNEXEC on i386 does is that it executes kernel code in its own 0-based
> code segment hence the 'low' code addresses. due to the current logic that
> checks for SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK in get_kctl_0dB_offset, this
> callback address is instead treated as a data pointer (as apparently
> SNDRV_CTL_ELEM_ACCESS_TLV_READ is also set) and since it's not a valid kernel
> address, it causes a GPF under the UDEREF PaX feature (without UDEREF it'd
> have been an oops since such low addresses are not mapped for kernel threads).

There're two ways to use 'struct snd_kcontrol.tlv'; constant array (=.p) an
a handler (= .c). An 'access' flag in each member of
'struct snd_kcontrol.vd' represent which way is used for the member.
Therefore, as long as checking the flag, the 'get_kctl_0dB_offset()' doesn't
handle a pointer to the handler as a pointer to array of TLV container.

> on vanilla kernels all this is a silent read of kernel code bytes that are
> then interpreted as the tlv[] array content, which is probably not what you
> want either.

Yes. Current code include a bug of inappropriate condition statement for the
above. As a result, it allows to handle a pointer to the handler as a
pointer to TLV data. 

> as for fixing this, first the above mentioned assumption should be 
> re-evaluated.
> if it's considered correct then there is some logic bug in my case (i can help
> debug it if you tell me what to do) otherwise the current pattern of
> 
> if (SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK && callback == 
> snd_hda_mixer_amp_tlv) {
>   call wrapped function underneath snd_hda_mixer_amp_tlv
> } else if (SNDRV_CTL_ELEM_ACCESS_TLV_READ) {
>   treat unrecognized callback address as data ptr
> }
> 
> should be changed to
> 
> if (SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK( {
>   if (callback == snd_hda_mixer_amp_tlv) {
> call wrapped function underneath snd_hda_mixer_amp_tlv
>   } else if (callback == others) {
> handle others, WARN/BUG/etc
>   }
> } else if (SNDRV_CTL_ELEM_ACCESS_TLV_READ) {
>   no longer treat unrecognized callback address as data ptr
> }

Good enough as a solution. Please test a patch in the end of this message.

> and all other callbacks with userland access should be refactored the same
> way as snd_hda_mixer_amp_tlv was. i'd also suggest that you at least do the
> above suggested if/else pattern change in order to prevent the misuse of
> unexpected callbacks in the future.

This suggestion is better for safety. Do you have some ways to detect the
pattern on current vanilla kernel? Or we should find it by eye-grep?


Thanks

Takashi Sakamoto

 8< 

>From 85896b50aa22bf2f2b5e45456daa16d386602edc Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto 
Date: Sat, 14 Oct 2017 14:08:51 +0900
Subject: [PATCH] ALSA: hda-intel: Fix to handle a pointer to TLV handler
 as a pointer to TLV data

In a design of ALSA control core, an 'access' flag on each entry of
'struct snd_kcontrol.vd' represents the type of 'struct snd_kcontrol.tlv'
for the corresponding element; a pointer to TLV data
(!=SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) or a pointer to handler
(==SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK).

In current implementation of 'get_kctl_0dB_offset()', condition statement
is not proper to distinguish these two cases. As a result, it handles
a pointer to the handler as a pointer to TLV data for some control element
sets. This bug brings invalid references to kernel space. A reporter shows
a sample of backtrace.

PAX: suspicious general protection fault:  [#1] PREEMPT SMP
Modules linked in:
CPU: 4 PID: 31 Comm: kworker/4:0 Not tainted 4.13.5-i386-pax #17
Workqueue: events azx_probe_work
task: eb61c880 task.stack: eb62a000
EIP: 0060:[<009bbd2d>] init_slave_0dB+0x2d/0xa0
EFLAGS: 00210202 CPU: 4
EAX: 00991fd0 EBX: e9883a80 ECX: e9883a80 EDX: eb62bd74
ESI: eb62bd74 EDI: e9098800 EBP: eb62bd08 ESP: eb62bcec
 DS: 0068 ES: 0068 FS: 00d8 GS: 0068 SS: 0068
CR0: 80050033 CR2:  CR3: 03b2d100 CR4: 000406f0 shadow CR4: 000406f0
Call Trace:
 [<009bb469>] map_slaves+0xb9/0xe0
 [<009bce0e>] __snd_hda_add_vmaster+0xde/0x110
 [<009c82f0>] snd_hda_gen_build_controls+0x1a0/0x1b0
 [<009d0a4d>] cx_auto_build_controls+0xd/0x70
 [<009bdf76>] snd_hda_codec_build_controls+0x186/0x1d0
 [<009b92ad>] hda_codec_driver_probe+0x6d/0xf0
 [<006a3be9>] driver_probe_device+0x289/0x420
 [<006a3ed6>] __device_attach_driver+0x76/0x100
 [<006a1edf>] bus_for_each_drv+0x3f/0x70
 [<006a3813>] __device_attach+0xa3/0x110
 [<006a3f9d>] device_initial_probe+0xd/0x10
 [<006a2d6f>] bus_probe_device+0x6f/0x80
 [<006a100f>] device_add+0x2cf/0x590
 [<009d8d8c>] snd_hdac_device_register+0xc/0x40
 [<009b90b4>] snd_hda_codec_configure+0x34/0x140
 [<009c2875>] azx_codec_configure+0x25/0x50
 [<009d8081>] azx_probe_continue+0x621/0x9e0
 [<009d84bd>] azx_probe_work+0xd/0x10
 [<0006fff2>] process_one_work+0x122/0x2a0
 [<000701a9>] 

Re: [PATCH V9 0/7] blk-mq-sched: improve sequential I/O performance

[Ming Lei]

On Fri, Oct 13, 2017 at 02:23:07PM -0600, Jens Axboe wrote:
> On 10/13/2017 01:21 PM, Jens Axboe wrote:
> > On 10/13/2017 01:08 PM, Jens Axboe wrote:
> >> On 10/13/2017 12:05 PM, Ming Lei wrote:
> >>> Hi Jens,
> >>>
> >>> In Red Hat internal storage test wrt. blk-mq scheduler, we found that I/O
> >>> performance is much bad with mq-deadline, especially about sequential I/O
> >>> on some multi-queue SCSI devcies(lpfc, qla2xxx, SRP...)
> >>>
> >>> Turns out one big issue causes the performance regression: requests are
> >>> still dequeued from sw queue/scheduler queue even when ldd's queue is
> >>> busy, so I/O merge becomes quite difficult to make, then sequential IO
> >>> performance degrades a lot.
> >>>
> >>> This issue becomes one of mains reasons for reverting default SCSI_MQ
> >>> in V4.13.
> >>>
> >>> This 8 patches improve this situation, and brings back performance loss.
> >>>
> >>> With this change, SCSI-MQ sequential I/O performance is improved much, 
> >>> Paolo
> >>> reported that mq-deadline performance improved much[2] in his dbench test
> >>> wrt V2. Also performance improvement on lpfc/qla2xx was observed with 
> >>> V1.[1]
> >>>
> >>> [1] http://marc.info/?l=linux-block=150151989915776=2
> >>> [2] https://marc.info/?l=linux-block=150217980602843=2
> >>
> >> I wanted to run some sanity testing on this series before committing it,
> >> and unfortunately it doesn't even boot for me. Just hangs after loading
> >> the kernel. Maybe an error slipped in for v8/9?
> > 
> > Or it might be something with kyber, my laptop defaults to that. Test
> > box seems to boot (which is SCSI), and nvme loads fine by default,
> > but not with kyber.
> > 
> > I don't have time to look into this more today, but the above might
> > help you figure out what is going on.
> 
> Verified that the laptop boots just fine if I remove the kyber udev
> rule.

I can reproduce this issue with kyber switched to, and will figure out
it soon.

-- 
Ming

Re: [PATCH V9 0/7] blk-mq-sched: improve sequential I/O performance

[Ming Lei]

On Fri, Oct 13, 2017 at 02:23:07PM -0600, Jens Axboe wrote:
> On 10/13/2017 01:21 PM, Jens Axboe wrote:
> > On 10/13/2017 01:08 PM, Jens Axboe wrote:
> >> On 10/13/2017 12:05 PM, Ming Lei wrote:
> >>> Hi Jens,
> >>>
> >>> In Red Hat internal storage test wrt. blk-mq scheduler, we found that I/O
> >>> performance is much bad with mq-deadline, especially about sequential I/O
> >>> on some multi-queue SCSI devcies(lpfc, qla2xxx, SRP...)
> >>>
> >>> Turns out one big issue causes the performance regression: requests are
> >>> still dequeued from sw queue/scheduler queue even when ldd's queue is
> >>> busy, so I/O merge becomes quite difficult to make, then sequential IO
> >>> performance degrades a lot.
> >>>
> >>> This issue becomes one of mains reasons for reverting default SCSI_MQ
> >>> in V4.13.
> >>>
> >>> This 8 patches improve this situation, and brings back performance loss.
> >>>
> >>> With this change, SCSI-MQ sequential I/O performance is improved much, 
> >>> Paolo
> >>> reported that mq-deadline performance improved much[2] in his dbench test
> >>> wrt V2. Also performance improvement on lpfc/qla2xx was observed with 
> >>> V1.[1]
> >>>
> >>> [1] http://marc.info/?l=linux-block=150151989915776=2
> >>> [2] https://marc.info/?l=linux-block=150217980602843=2
> >>
> >> I wanted to run some sanity testing on this series before committing it,
> >> and unfortunately it doesn't even boot for me. Just hangs after loading
> >> the kernel. Maybe an error slipped in for v8/9?
> > 
> > Or it might be something with kyber, my laptop defaults to that. Test
> > box seems to boot (which is SCSI), and nvme loads fine by default,
> > but not with kyber.
> > 
> > I don't have time to look into this more today, but the above might
> > help you figure out what is going on.
> 
> Verified that the laptop boots just fine if I remove the kyber udev
> rule.

I can reproduce this issue with kyber switched to, and will figure out
it soon.

-- 
Ming

Re: [PATCH v3] hwmon: xgene: Support hwmon v2

[Hoan Tran]

Hi

On Fri, Oct 13, 2017 at 9:54 PM, Guenter Roeck  wrote:
> On 10/13/2017 09:38 PM, Hoan Tran wrote:
>>
>> Hi Guenter,
>>
>> On Fri, Oct 13, 2017 at 9:28 PM, Guenter Roeck  wrote:
>>>
>>> On 10/13/2017 04:10 PM, Hoan Tran wrote:


 This patch supports xgene-hwmon v2 which uses the non-cachable memory
 as the PCC shared memory.

 Signed-off-by: Hoan Tran 
 ---
 v3
- Use local version variable
- Use inline calls instead of the private map function

 v2
- Map PCC shared mem by ioremap() in case hwmon is v2

drivers/hwmon/xgene-hwmon.c | 40
 +++-
1 file changed, 31 insertions(+), 9 deletions(-)

 diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
 index 9c0dbb8..6b6c732 100644
 --- a/drivers/hwmon/xgene-hwmon.c
 +++ b/drivers/hwmon/xgene-hwmon.c
 @@ -91,6 +91,11 @@
#define to_xgene_hwmon_dev(cl)\
  container_of(cl, struct xgene_hwmon_dev, mbox_client)
+enum xgene_hwmon_version {
 +   XGENE_HWMON_V1 = 0,
 +   XGENE_HWMON_V2 = 1,
 +};
 +
struct slimpro_resp_msg {
  u32 msg;
  u32 param1;
 @@ -609,6 +614,15 @@ static void xgene_hwmon_tx_done(struct mbox_client
 *cl, void *msg, int ret)
  }
}
+#ifdef CONFIG_ACPI
 +static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
 +   {"APMC0D29", XGENE_HWMON_V1},
 +   {"APMC0D8A", XGENE_HWMON_V2},
 +   {},
 +};
 +MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
 +#endif
 +
static int xgene_hwmon_probe(struct platform_device *pdev)
{
  struct xgene_hwmon_dev *ctx;
 @@ -650,6 +664,16 @@ static int xgene_hwmon_probe(struct platform_device
 *pdev)
  }
  } else {
  struct acpi_pcct_hw_reduced *cppc_ss;
 +   const struct acpi_device_id *acpi_id;
 +   int version;
 +
 +#ifdef CONFIG_ACPI
 +   acpi_id = acpi_match_device(xgene_hwmon_acpi_match,
 >dev);
 +   if (!acpi_id)
 +   return -EINVAL;
 +
 +   version = (int)acpi_id->driver_data;
 +#endif

>>>
>>> This leaves "version" uninitialized if CONFIG_ACPI is not defined.
>>
>>
>> If CONFIG_ACPI is not defined, "acpi_disabled" is true. So this piece
>> of code is not executed
>>
>
> Then why do you need the ifdef in code that isn't executed anyway
> if CONFIG_ACPI is not defined ? Because xgene_hwmon_acpi_match is
> conditional ? Not an argument, because that doesn't have to be.
>
> Also, the compiler doesn't know that, and I am quite sure that
> it is going to complain that version may be used uninitialized if
> CONFIG_ACPI is not defined.

Yes, I'll fix it

Thanks
Hoan

>
> Sorry, I won't accept that code.
>
> Guenter
>
>
>> Thanks
>> Hoan
>>
>>>
>>> Guenter
>>>
>>>
  if (device_property_read_u32(>dev, "pcc-channel",
   >mbox_idx)) {
 @@ -690,7 +714,13 @@ static int xgene_hwmon_probe(struct platform_device
 *pdev)
   */
  ctx->comm_base_addr = cppc_ss->base_address;
  if (ctx->comm_base_addr) {
 -   ctx->pcc_comm_addr =
 memremap(ctx->comm_base_addr,
 +   if (version == XGENE_HWMON_V2)
 +   ctx->pcc_comm_addr = (void __force
 *)ioremap(
 +
 ctx->comm_base_addr,
 +
 cppc_ss->length);
 +   else
 +   ctx->pcc_comm_addr = memremap(
 +
 ctx->comm_base_addr,

 cppc_ss->length,
  MEMREMAP_WB);
  } else {
 @@ -758,14 +788,6 @@ static int xgene_hwmon_remove(struct
 platform_device
 *pdev)
  return 0;
}
-#ifdef CONFIG_ACPI
 -static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
 -   {"APMC0D29", 0},
 -   {},
 -};
 -MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
 -#endif
 -
static const struct of_device_id xgene_hwmon_of_match[] = {
  {.compatible = "apm,xgene-slimpro-hwmon"},
  {}

>>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] hwmon: xgene: Support hwmon v2

[Hoan Tran]

Hi

On Fri, Oct 13, 2017 at 9:54 PM, Guenter Roeck  wrote:
> On 10/13/2017 09:38 PM, Hoan Tran wrote:
>>
>> Hi Guenter,
>>
>> On Fri, Oct 13, 2017 at 9:28 PM, Guenter Roeck  wrote:
>>>
>>> On 10/13/2017 04:10 PM, Hoan Tran wrote:


 This patch supports xgene-hwmon v2 which uses the non-cachable memory
 as the PCC shared memory.

 Signed-off-by: Hoan Tran 
 ---
 v3
- Use local version variable
- Use inline calls instead of the private map function

 v2
- Map PCC shared mem by ioremap() in case hwmon is v2

drivers/hwmon/xgene-hwmon.c | 40
 +++-
1 file changed, 31 insertions(+), 9 deletions(-)

 diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
 index 9c0dbb8..6b6c732 100644
 --- a/drivers/hwmon/xgene-hwmon.c
 +++ b/drivers/hwmon/xgene-hwmon.c
 @@ -91,6 +91,11 @@
#define to_xgene_hwmon_dev(cl)\
  container_of(cl, struct xgene_hwmon_dev, mbox_client)
+enum xgene_hwmon_version {
 +   XGENE_HWMON_V1 = 0,
 +   XGENE_HWMON_V2 = 1,
 +};
 +
struct slimpro_resp_msg {
  u32 msg;
  u32 param1;
 @@ -609,6 +614,15 @@ static void xgene_hwmon_tx_done(struct mbox_client
 *cl, void *msg, int ret)
  }
}
+#ifdef CONFIG_ACPI
 +static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
 +   {"APMC0D29", XGENE_HWMON_V1},
 +   {"APMC0D8A", XGENE_HWMON_V2},
 +   {},
 +};
 +MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
 +#endif
 +
static int xgene_hwmon_probe(struct platform_device *pdev)
{
  struct xgene_hwmon_dev *ctx;
 @@ -650,6 +664,16 @@ static int xgene_hwmon_probe(struct platform_device
 *pdev)
  }
  } else {
  struct acpi_pcct_hw_reduced *cppc_ss;
 +   const struct acpi_device_id *acpi_id;
 +   int version;
 +
 +#ifdef CONFIG_ACPI
 +   acpi_id = acpi_match_device(xgene_hwmon_acpi_match,
 >dev);
 +   if (!acpi_id)
 +   return -EINVAL;
 +
 +   version = (int)acpi_id->driver_data;
 +#endif

>>>
>>> This leaves "version" uninitialized if CONFIG_ACPI is not defined.
>>
>>
>> If CONFIG_ACPI is not defined, "acpi_disabled" is true. So this piece
>> of code is not executed
>>
>
> Then why do you need the ifdef in code that isn't executed anyway
> if CONFIG_ACPI is not defined ? Because xgene_hwmon_acpi_match is
> conditional ? Not an argument, because that doesn't have to be.
>
> Also, the compiler doesn't know that, and I am quite sure that
> it is going to complain that version may be used uninitialized if
> CONFIG_ACPI is not defined.

Yes, I'll fix it

Thanks
Hoan

>
> Sorry, I won't accept that code.
>
> Guenter
>
>
>> Thanks
>> Hoan
>>
>>>
>>> Guenter
>>>
>>>
  if (device_property_read_u32(>dev, "pcc-channel",
   >mbox_idx)) {
 @@ -690,7 +714,13 @@ static int xgene_hwmon_probe(struct platform_device
 *pdev)
   */
  ctx->comm_base_addr = cppc_ss->base_address;
  if (ctx->comm_base_addr) {
 -   ctx->pcc_comm_addr =
 memremap(ctx->comm_base_addr,
 +   if (version == XGENE_HWMON_V2)
 +   ctx->pcc_comm_addr = (void __force
 *)ioremap(
 +
 ctx->comm_base_addr,
 +
 cppc_ss->length);
 +   else
 +   ctx->pcc_comm_addr = memremap(
 +
 ctx->comm_base_addr,

 cppc_ss->length,
  MEMREMAP_WB);
  } else {
 @@ -758,14 +788,6 @@ static int xgene_hwmon_remove(struct
 platform_device
 *pdev)
  return 0;
}
-#ifdef CONFIG_ACPI
 -static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
 -   {"APMC0D29", 0},
 -   {},
 -};
 -MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
 -#endif
 -
static const struct of_device_id xgene_hwmon_of_match[] = {
  {.compatible = "apm,xgene-slimpro-hwmon"},
  {}

>>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] hwmon: xgene: Support hwmon v2

[Guenter Roeck]

On 10/13/2017 09:38 PM, Hoan Tran wrote:

Hi Guenter,

On Fri, Oct 13, 2017 at 9:28 PM, Guenter Roeck  wrote:

On 10/13/2017 04:10 PM, Hoan Tran wrote:


This patch supports xgene-hwmon v2 which uses the non-cachable memory
as the PCC shared memory.

Signed-off-by: Hoan Tran 
---
v3
   - Use local version variable
   - Use inline calls instead of the private map function

v2
   - Map PCC shared mem by ioremap() in case hwmon is v2

   drivers/hwmon/xgene-hwmon.c | 40
+++-
   1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
index 9c0dbb8..6b6c732 100644
--- a/drivers/hwmon/xgene-hwmon.c
+++ b/drivers/hwmon/xgene-hwmon.c
@@ -91,6 +91,11 @@
   #define to_xgene_hwmon_dev(cl)\
 container_of(cl, struct xgene_hwmon_dev, mbox_client)
   +enum xgene_hwmon_version {
+   XGENE_HWMON_V1 = 0,
+   XGENE_HWMON_V2 = 1,
+};
+
   struct slimpro_resp_msg {
 u32 msg;
 u32 param1;
@@ -609,6 +614,15 @@ static void xgene_hwmon_tx_done(struct mbox_client
*cl, void *msg, int ret)
 }
   }
   +#ifdef CONFIG_ACPI
+static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
+   {"APMC0D29", XGENE_HWMON_V1},
+   {"APMC0D8A", XGENE_HWMON_V2},
+   {},
+};
+MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
+#endif
+
   static int xgene_hwmon_probe(struct platform_device *pdev)
   {
 struct xgene_hwmon_dev *ctx;
@@ -650,6 +664,16 @@ static int xgene_hwmon_probe(struct platform_device
*pdev)
 }
 } else {
 struct acpi_pcct_hw_reduced *cppc_ss;
+   const struct acpi_device_id *acpi_id;
+   int version;
+
+#ifdef CONFIG_ACPI
+   acpi_id = acpi_match_device(xgene_hwmon_acpi_match,
>dev);
+   if (!acpi_id)
+   return -EINVAL;
+
+   version = (int)acpi_id->driver_data;
+#endif



This leaves "version" uninitialized if CONFIG_ACPI is not defined.


If CONFIG_ACPI is not defined, "acpi_disabled" is true. So this piece
of code is not executed



Then why do you need the ifdef in code that isn't executed anyway
if CONFIG_ACPI is not defined ? Because xgene_hwmon_acpi_match is
conditional ? Not an argument, because that doesn't have to be.

Also, the compiler doesn't know that, and I am quite sure that
it is going to complain that version may be used uninitialized if
CONFIG_ACPI is not defined.

Sorry, I won't accept that code.

Guenter


Thanks
Hoan



Guenter



 if (device_property_read_u32(>dev, "pcc-channel",
  >mbox_idx)) {
@@ -690,7 +714,13 @@ static int xgene_hwmon_probe(struct platform_device
*pdev)
  */
 ctx->comm_base_addr = cppc_ss->base_address;
 if (ctx->comm_base_addr) {
-   ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
+   if (version == XGENE_HWMON_V2)
+   ctx->pcc_comm_addr = (void __force
*)ioremap(
+
ctx->comm_base_addr,
+   cppc_ss->length);
+   else
+   ctx->pcc_comm_addr = memremap(
+
ctx->comm_base_addr,
 cppc_ss->length,
 MEMREMAP_WB);
 } else {
@@ -758,14 +788,6 @@ static int xgene_hwmon_remove(struct platform_device
*pdev)
 return 0;
   }
   -#ifdef CONFIG_ACPI
-static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
-   {"APMC0D29", 0},
-   {},
-};
-MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
-#endif
-
   static const struct of_device_id xgene_hwmon_of_match[] = {
 {.compatible = "apm,xgene-slimpro-hwmon"},
 {}

Re: [PATCH v3] hwmon: xgene: Support hwmon v2

[Guenter Roeck]

On 10/13/2017 09:38 PM, Hoan Tran wrote:

Hi Guenter,

On Fri, Oct 13, 2017 at 9:28 PM, Guenter Roeck  wrote:

On 10/13/2017 04:10 PM, Hoan Tran wrote:


This patch supports xgene-hwmon v2 which uses the non-cachable memory
as the PCC shared memory.

Signed-off-by: Hoan Tran 
---
v3
   - Use local version variable
   - Use inline calls instead of the private map function

v2
   - Map PCC shared mem by ioremap() in case hwmon is v2

   drivers/hwmon/xgene-hwmon.c | 40
+++-
   1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
index 9c0dbb8..6b6c732 100644
--- a/drivers/hwmon/xgene-hwmon.c
+++ b/drivers/hwmon/xgene-hwmon.c
@@ -91,6 +91,11 @@
   #define to_xgene_hwmon_dev(cl)\
 container_of(cl, struct xgene_hwmon_dev, mbox_client)
   +enum xgene_hwmon_version {
+   XGENE_HWMON_V1 = 0,
+   XGENE_HWMON_V2 = 1,
+};
+
   struct slimpro_resp_msg {
 u32 msg;
 u32 param1;
@@ -609,6 +614,15 @@ static void xgene_hwmon_tx_done(struct mbox_client
*cl, void *msg, int ret)
 }
   }
   +#ifdef CONFIG_ACPI
+static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
+   {"APMC0D29", XGENE_HWMON_V1},
+   {"APMC0D8A", XGENE_HWMON_V2},
+   {},
+};
+MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
+#endif
+
   static int xgene_hwmon_probe(struct platform_device *pdev)
   {
 struct xgene_hwmon_dev *ctx;
@@ -650,6 +664,16 @@ static int xgene_hwmon_probe(struct platform_device
*pdev)
 }
 } else {
 struct acpi_pcct_hw_reduced *cppc_ss;
+   const struct acpi_device_id *acpi_id;
+   int version;
+
+#ifdef CONFIG_ACPI
+   acpi_id = acpi_match_device(xgene_hwmon_acpi_match,
>dev);
+   if (!acpi_id)
+   return -EINVAL;
+
+   version = (int)acpi_id->driver_data;
+#endif



This leaves "version" uninitialized if CONFIG_ACPI is not defined.


If CONFIG_ACPI is not defined, "acpi_disabled" is true. So this piece
of code is not executed



Then why do you need the ifdef in code that isn't executed anyway
if CONFIG_ACPI is not defined ? Because xgene_hwmon_acpi_match is
conditional ? Not an argument, because that doesn't have to be.

Also, the compiler doesn't know that, and I am quite sure that
it is going to complain that version may be used uninitialized if
CONFIG_ACPI is not defined.

Sorry, I won't accept that code.

Guenter


Thanks
Hoan



Guenter



 if (device_property_read_u32(>dev, "pcc-channel",
  >mbox_idx)) {
@@ -690,7 +714,13 @@ static int xgene_hwmon_probe(struct platform_device
*pdev)
  */
 ctx->comm_base_addr = cppc_ss->base_address;
 if (ctx->comm_base_addr) {
-   ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
+   if (version == XGENE_HWMON_V2)
+   ctx->pcc_comm_addr = (void __force
*)ioremap(
+
ctx->comm_base_addr,
+   cppc_ss->length);
+   else
+   ctx->pcc_comm_addr = memremap(
+
ctx->comm_base_addr,
 cppc_ss->length,
 MEMREMAP_WB);
 } else {
@@ -758,14 +788,6 @@ static int xgene_hwmon_remove(struct platform_device
*pdev)
 return 0;
   }
   -#ifdef CONFIG_ACPI
-static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
-   {"APMC0D29", 0},
-   {},
-};
-MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
-#endif
-
   static const struct of_device_id xgene_hwmon_of_match[] = {
 {.compatible = "apm,xgene-slimpro-hwmon"},
 {}

Re: [RFC][PATCH] x86, syscalls: use SYSCALL_DEFINE() macros for sys_modify_ldt()

[Andy Lutomirski]

On Fri, Oct 13, 2017 at 4:49 PM, Brian Gerst  wrote:
> On Fri, Oct 13, 2017 at 5:03 PM, Andy Lutomirski  wrote:
>> On Fri, Oct 13, 2017 at 1:39 PM, Dave Hansen
>>  wrote:
>>>
>>> I noticed that we don't have tracepoints for sys_modify_ldt().  I
>>> think that's because we define it directly instead of using the
>>> normal SYSCALL_DEFINEx() macros.
>>>
>>> Is there a reason for that, or were they just missed when the
>>> macros were created?
>>
>> No, and it's a longstanding fsckup that I think you can't fix like
>> this because...
>>
>>>
>>> Cc: x...@kernel.org
>>> Cc: Andy Lutomirski 
>>>
>>> ---
>>>
>>>  b/arch/x86/include/asm/syscalls.h |2 +-
>>>  b/arch/x86/kernel/ldt.c   |5 +++--
>>>  b/arch/x86/um/ldt.c   |3 ++-
>>>  3 files changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff -puN arch/x86/kernel/ldt.c~x86-syscall-macros-modify_ldt 
>>> arch/x86/kernel/ldt.c
>>> --- a/arch/x86/kernel/ldt.c~x86-syscall-macros-modify_ldt   2017-10-13 
>>> 13:30:12.802553391 -0700
>>> +++ b/arch/x86/kernel/ldt.c 2017-10-13 13:30:12.817553391 -0700
>>> @@ -12,6 +12,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -294,8 +295,8 @@ out:
>>> return error;
>>>  }
>>>
>>> -asmlinkage int sys_modify_ldt(int func, void __user *ptr,
>>> - unsigned long bytecount)
>>> +SYSCALL_DEFINE3(modify_ldt, int , func , void __user * , ptr ,
>>> +   unsigned long , bytecount)
>>
>> sys_modify_ldt() returns int, which is wrong, and it's visibly wrong
>> to 64-bit user code.  So I think you need to make sure that the return
>> value is cast to int in all cases.
>
> I don't think there will be a problem here.  If 64-bit userspace
> treats it as an int, it will truncate to 32-bit signed and all is
> well.  If it is treating it as a long, then it's currently broken for
> errors anyways.
>

Let me say what I mean more clearly:

The current code is buggy: specifically, a 64-bit modify_ldt() call
that *fails* will return something like (int)-EFAULT.  This is bogus,
but it's the ABI.  There's even a selftest in the kernel tree that
notices this (although it doesn't check it right now).  All that needs
to happen for this patch to be okay AFAIK is to make sure that we
preserve that bug instead of accidentally fixing it.

Re: [RFC][PATCH] x86, syscalls: use SYSCALL_DEFINE() macros for sys_modify_ldt()

[Andy Lutomirski]

On Fri, Oct 13, 2017 at 4:49 PM, Brian Gerst  wrote:
> On Fri, Oct 13, 2017 at 5:03 PM, Andy Lutomirski  wrote:
>> On Fri, Oct 13, 2017 at 1:39 PM, Dave Hansen
>>  wrote:
>>>
>>> I noticed that we don't have tracepoints for sys_modify_ldt().  I
>>> think that's because we define it directly instead of using the
>>> normal SYSCALL_DEFINEx() macros.
>>>
>>> Is there a reason for that, or were they just missed when the
>>> macros were created?
>>
>> No, and it's a longstanding fsckup that I think you can't fix like
>> this because...
>>
>>>
>>> Cc: x...@kernel.org
>>> Cc: Andy Lutomirski 
>>>
>>> ---
>>>
>>>  b/arch/x86/include/asm/syscalls.h |2 +-
>>>  b/arch/x86/kernel/ldt.c   |5 +++--
>>>  b/arch/x86/um/ldt.c   |3 ++-
>>>  3 files changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff -puN arch/x86/kernel/ldt.c~x86-syscall-macros-modify_ldt 
>>> arch/x86/kernel/ldt.c
>>> --- a/arch/x86/kernel/ldt.c~x86-syscall-macros-modify_ldt   2017-10-13 
>>> 13:30:12.802553391 -0700
>>> +++ b/arch/x86/kernel/ldt.c 2017-10-13 13:30:12.817553391 -0700
>>> @@ -12,6 +12,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -294,8 +295,8 @@ out:
>>> return error;
>>>  }
>>>
>>> -asmlinkage int sys_modify_ldt(int func, void __user *ptr,
>>> - unsigned long bytecount)
>>> +SYSCALL_DEFINE3(modify_ldt, int , func , void __user * , ptr ,
>>> +   unsigned long , bytecount)
>>
>> sys_modify_ldt() returns int, which is wrong, and it's visibly wrong
>> to 64-bit user code.  So I think you need to make sure that the return
>> value is cast to int in all cases.
>
> I don't think there will be a problem here.  If 64-bit userspace
> treats it as an int, it will truncate to 32-bit signed and all is
> well.  If it is treating it as a long, then it's currently broken for
> errors anyways.
>

Let me say what I mean more clearly:

The current code is buggy: specifically, a 64-bit modify_ldt() call
that *fails* will return something like (int)-EFAULT.  This is bogus,
but it's the ABI.  There's even a selftest in the kernel tree that
notices this (although it doesn't check it right now).  All that needs
to happen for this patch to be okay AFAIK is to make sure that we
preserve that bug instead of accidentally fixing it.

Re: [PATCH v3] hwmon: xgene: Support hwmon v2

[Hoan Tran]

Hi Guenter,

On Fri, Oct 13, 2017 at 9:28 PM, Guenter Roeck  wrote:
> On 10/13/2017 04:10 PM, Hoan Tran wrote:
>>
>> This patch supports xgene-hwmon v2 which uses the non-cachable memory
>> as the PCC shared memory.
>>
>> Signed-off-by: Hoan Tran 
>> ---
>> v3
>>   - Use local version variable
>>   - Use inline calls instead of the private map function
>>
>> v2
>>   - Map PCC shared mem by ioremap() in case hwmon is v2
>>
>>   drivers/hwmon/xgene-hwmon.c | 40
>> +++-
>>   1 file changed, 31 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
>> index 9c0dbb8..6b6c732 100644
>> --- a/drivers/hwmon/xgene-hwmon.c
>> +++ b/drivers/hwmon/xgene-hwmon.c
>> @@ -91,6 +91,11 @@
>>   #define to_xgene_hwmon_dev(cl)\
>> container_of(cl, struct xgene_hwmon_dev, mbox_client)
>>   +enum xgene_hwmon_version {
>> +   XGENE_HWMON_V1 = 0,
>> +   XGENE_HWMON_V2 = 1,
>> +};
>> +
>>   struct slimpro_resp_msg {
>> u32 msg;
>> u32 param1;
>> @@ -609,6 +614,15 @@ static void xgene_hwmon_tx_done(struct mbox_client
>> *cl, void *msg, int ret)
>> }
>>   }
>>   +#ifdef CONFIG_ACPI
>> +static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
>> +   {"APMC0D29", XGENE_HWMON_V1},
>> +   {"APMC0D8A", XGENE_HWMON_V2},
>> +   {},
>> +};
>> +MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
>> +#endif
>> +
>>   static int xgene_hwmon_probe(struct platform_device *pdev)
>>   {
>> struct xgene_hwmon_dev *ctx;
>> @@ -650,6 +664,16 @@ static int xgene_hwmon_probe(struct platform_device
>> *pdev)
>> }
>> } else {
>> struct acpi_pcct_hw_reduced *cppc_ss;
>> +   const struct acpi_device_id *acpi_id;
>> +   int version;
>> +
>> +#ifdef CONFIG_ACPI
>> +   acpi_id = acpi_match_device(xgene_hwmon_acpi_match,
>> >dev);
>> +   if (!acpi_id)
>> +   return -EINVAL;
>> +
>> +   version = (int)acpi_id->driver_data;
>> +#endif
>>
>
> This leaves "version" uninitialized if CONFIG_ACPI is not defined.

If CONFIG_ACPI is not defined, "acpi_disabled" is true. So this piece
of code is not executed

Thanks
Hoan

>
> Guenter
>
>
>> if (device_property_read_u32(>dev, "pcc-channel",
>>  >mbox_idx)) {
>> @@ -690,7 +714,13 @@ static int xgene_hwmon_probe(struct platform_device
>> *pdev)
>>  */
>> ctx->comm_base_addr = cppc_ss->base_address;
>> if (ctx->comm_base_addr) {
>> -   ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
>> +   if (version == XGENE_HWMON_V2)
>> +   ctx->pcc_comm_addr = (void __force
>> *)ioremap(
>> +
>> ctx->comm_base_addr,
>> +   cppc_ss->length);
>> +   else
>> +   ctx->pcc_comm_addr = memremap(
>> +
>> ctx->comm_base_addr,
>> cppc_ss->length,
>> MEMREMAP_WB);
>> } else {
>> @@ -758,14 +788,6 @@ static int xgene_hwmon_remove(struct platform_device
>> *pdev)
>> return 0;
>>   }
>>   -#ifdef CONFIG_ACPI
>> -static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
>> -   {"APMC0D29", 0},
>> -   {},
>> -};
>> -MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
>> -#endif
>> -
>>   static const struct of_device_id xgene_hwmon_of_match[] = {
>> {.compatible = "apm,xgene-slimpro-hwmon"},
>> {}
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] hwmon: xgene: Support hwmon v2

[Hoan Tran]

Hi Guenter,

On Fri, Oct 13, 2017 at 9:28 PM, Guenter Roeck  wrote:
> On 10/13/2017 04:10 PM, Hoan Tran wrote:
>>
>> This patch supports xgene-hwmon v2 which uses the non-cachable memory
>> as the PCC shared memory.
>>
>> Signed-off-by: Hoan Tran 
>> ---
>> v3
>>   - Use local version variable
>>   - Use inline calls instead of the private map function
>>
>> v2
>>   - Map PCC shared mem by ioremap() in case hwmon is v2
>>
>>   drivers/hwmon/xgene-hwmon.c | 40
>> +++-
>>   1 file changed, 31 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
>> index 9c0dbb8..6b6c732 100644
>> --- a/drivers/hwmon/xgene-hwmon.c
>> +++ b/drivers/hwmon/xgene-hwmon.c
>> @@ -91,6 +91,11 @@
>>   #define to_xgene_hwmon_dev(cl)\
>> container_of(cl, struct xgene_hwmon_dev, mbox_client)
>>   +enum xgene_hwmon_version {
>> +   XGENE_HWMON_V1 = 0,
>> +   XGENE_HWMON_V2 = 1,
>> +};
>> +
>>   struct slimpro_resp_msg {
>> u32 msg;
>> u32 param1;
>> @@ -609,6 +614,15 @@ static void xgene_hwmon_tx_done(struct mbox_client
>> *cl, void *msg, int ret)
>> }
>>   }
>>   +#ifdef CONFIG_ACPI
>> +static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
>> +   {"APMC0D29", XGENE_HWMON_V1},
>> +   {"APMC0D8A", XGENE_HWMON_V2},
>> +   {},
>> +};
>> +MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
>> +#endif
>> +
>>   static int xgene_hwmon_probe(struct platform_device *pdev)
>>   {
>> struct xgene_hwmon_dev *ctx;
>> @@ -650,6 +664,16 @@ static int xgene_hwmon_probe(struct platform_device
>> *pdev)
>> }
>> } else {
>> struct acpi_pcct_hw_reduced *cppc_ss;
>> +   const struct acpi_device_id *acpi_id;
>> +   int version;
>> +
>> +#ifdef CONFIG_ACPI
>> +   acpi_id = acpi_match_device(xgene_hwmon_acpi_match,
>> >dev);
>> +   if (!acpi_id)
>> +   return -EINVAL;
>> +
>> +   version = (int)acpi_id->driver_data;
>> +#endif
>>
>
> This leaves "version" uninitialized if CONFIG_ACPI is not defined.

If CONFIG_ACPI is not defined, "acpi_disabled" is true. So this piece
of code is not executed

Thanks
Hoan

>
> Guenter
>
>
>> if (device_property_read_u32(>dev, "pcc-channel",
>>  >mbox_idx)) {
>> @@ -690,7 +714,13 @@ static int xgene_hwmon_probe(struct platform_device
>> *pdev)
>>  */
>> ctx->comm_base_addr = cppc_ss->base_address;
>> if (ctx->comm_base_addr) {
>> -   ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
>> +   if (version == XGENE_HWMON_V2)
>> +   ctx->pcc_comm_addr = (void __force
>> *)ioremap(
>> +
>> ctx->comm_base_addr,
>> +   cppc_ss->length);
>> +   else
>> +   ctx->pcc_comm_addr = memremap(
>> +
>> ctx->comm_base_addr,
>> cppc_ss->length,
>> MEMREMAP_WB);
>> } else {
>> @@ -758,14 +788,6 @@ static int xgene_hwmon_remove(struct platform_device
>> *pdev)
>> return 0;
>>   }
>>   -#ifdef CONFIG_ACPI
>> -static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
>> -   {"APMC0D29", 0},
>> -   {},
>> -};
>> -MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
>> -#endif
>> -
>>   static const struct of_device_id xgene_hwmon_of_match[] = {
>> {.compatible = "apm,xgene-slimpro-hwmon"},
>> {}
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] hwmon: xgene: Support hwmon v2

[Guenter Roeck]

On 10/13/2017 04:10 PM, Hoan Tran wrote:

This patch supports xgene-hwmon v2 which uses the non-cachable memory
as the PCC shared memory.

Signed-off-by: Hoan Tran 
---
v3
  - Use local version variable
  - Use inline calls instead of the private map function

v2
  - Map PCC shared mem by ioremap() in case hwmon is v2

  drivers/hwmon/xgene-hwmon.c | 40 +++-
  1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
index 9c0dbb8..6b6c732 100644
--- a/drivers/hwmon/xgene-hwmon.c
+++ b/drivers/hwmon/xgene-hwmon.c
@@ -91,6 +91,11 @@
  #define to_xgene_hwmon_dev(cl)\
container_of(cl, struct xgene_hwmon_dev, mbox_client)
  
+enum xgene_hwmon_version {

+   XGENE_HWMON_V1 = 0,
+   XGENE_HWMON_V2 = 1,
+};
+
  struct slimpro_resp_msg {
u32 msg;
u32 param1;
@@ -609,6 +614,15 @@ static void xgene_hwmon_tx_done(struct mbox_client *cl, 
void *msg, int ret)
}
  }
  
+#ifdef CONFIG_ACPI

+static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
+   {"APMC0D29", XGENE_HWMON_V1},
+   {"APMC0D8A", XGENE_HWMON_V2},
+   {},
+};
+MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
+#endif
+
  static int xgene_hwmon_probe(struct platform_device *pdev)
  {
struct xgene_hwmon_dev *ctx;
@@ -650,6 +664,16 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
}
} else {
struct acpi_pcct_hw_reduced *cppc_ss;
+   const struct acpi_device_id *acpi_id;
+   int version;
+
+#ifdef CONFIG_ACPI
+   acpi_id = acpi_match_device(xgene_hwmon_acpi_match, >dev);
+   if (!acpi_id)
+   return -EINVAL;
+
+   version = (int)acpi_id->driver_data;
+#endif
  

This leaves "version" uninitialized if CONFIG_ACPI is not defined.

Guenter


if (device_property_read_u32(>dev, "pcc-channel",
 >mbox_idx)) {
@@ -690,7 +714,13 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
 */
ctx->comm_base_addr = cppc_ss->base_address;
if (ctx->comm_base_addr) {
-   ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
+   if (version == XGENE_HWMON_V2)
+   ctx->pcc_comm_addr = (void __force *)ioremap(
+   ctx->comm_base_addr,
+   cppc_ss->length);
+   else
+   ctx->pcc_comm_addr = memremap(
+   ctx->comm_base_addr,
cppc_ss->length,
MEMREMAP_WB);
} else {
@@ -758,14 +788,6 @@ static int xgene_hwmon_remove(struct platform_device *pdev)
return 0;
  }
  
-#ifdef CONFIG_ACPI

-static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
-   {"APMC0D29", 0},
-   {},
-};
-MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
-#endif
-
  static const struct of_device_id xgene_hwmon_of_match[] = {
{.compatible = "apm,xgene-slimpro-hwmon"},
{}

Re: [PATCH v3] hwmon: xgene: Support hwmon v2

[Guenter Roeck]

On 10/13/2017 04:10 PM, Hoan Tran wrote:

This patch supports xgene-hwmon v2 which uses the non-cachable memory
as the PCC shared memory.

Signed-off-by: Hoan Tran 
---
v3
  - Use local version variable
  - Use inline calls instead of the private map function

v2
  - Map PCC shared mem by ioremap() in case hwmon is v2

  drivers/hwmon/xgene-hwmon.c | 40 +++-
  1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
index 9c0dbb8..6b6c732 100644
--- a/drivers/hwmon/xgene-hwmon.c
+++ b/drivers/hwmon/xgene-hwmon.c
@@ -91,6 +91,11 @@
  #define to_xgene_hwmon_dev(cl)\
container_of(cl, struct xgene_hwmon_dev, mbox_client)
  
+enum xgene_hwmon_version {

+   XGENE_HWMON_V1 = 0,
+   XGENE_HWMON_V2 = 1,
+};
+
  struct slimpro_resp_msg {
u32 msg;
u32 param1;
@@ -609,6 +614,15 @@ static void xgene_hwmon_tx_done(struct mbox_client *cl, 
void *msg, int ret)
}
  }
  
+#ifdef CONFIG_ACPI

+static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
+   {"APMC0D29", XGENE_HWMON_V1},
+   {"APMC0D8A", XGENE_HWMON_V2},
+   {},
+};
+MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
+#endif
+
  static int xgene_hwmon_probe(struct platform_device *pdev)
  {
struct xgene_hwmon_dev *ctx;
@@ -650,6 +664,16 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
}
} else {
struct acpi_pcct_hw_reduced *cppc_ss;
+   const struct acpi_device_id *acpi_id;
+   int version;
+
+#ifdef CONFIG_ACPI
+   acpi_id = acpi_match_device(xgene_hwmon_acpi_match, >dev);
+   if (!acpi_id)
+   return -EINVAL;
+
+   version = (int)acpi_id->driver_data;
+#endif
  

This leaves "version" uninitialized if CONFIG_ACPI is not defined.

Guenter


if (device_property_read_u32(>dev, "pcc-channel",
 >mbox_idx)) {
@@ -690,7 +714,13 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
 */
ctx->comm_base_addr = cppc_ss->base_address;
if (ctx->comm_base_addr) {
-   ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
+   if (version == XGENE_HWMON_V2)
+   ctx->pcc_comm_addr = (void __force *)ioremap(
+   ctx->comm_base_addr,
+   cppc_ss->length);
+   else
+   ctx->pcc_comm_addr = memremap(
+   ctx->comm_base_addr,
cppc_ss->length,
MEMREMAP_WB);
} else {
@@ -758,14 +788,6 @@ static int xgene_hwmon_remove(struct platform_device *pdev)
return 0;
  }
  
-#ifdef CONFIG_ACPI

-static const struct acpi_device_id xgene_hwmon_acpi_match[] = {
-   {"APMC0D29", 0},
-   {},
-};
-MODULE_DEVICE_TABLE(acpi, xgene_hwmon_acpi_match);
-#endif
-
  static const struct of_device_id xgene_hwmon_of_match[] = {
{.compatible = "apm,xgene-slimpro-hwmon"},
{}

Re: [RFC PATCH v9 for 4.15 01/14] Restartable sequences system call

[Linus Torvalds]

On Fri, Oct 13, 2017 at 8:01 PM, Andi Kleen  wrote:
>
> As far as I can see the current model fundamentally only works for
> one user per process (because there is only a single range and abort IP)

No, it should work for libraries, you just need to always initialize
the proper start/commit/abort IP's for every transaction. Then
everybody should be fine.

So I _think_ it's all good. But I really would want to see that
actually being the case.

Linus

Re: [RFC PATCH v9 for 4.15 01/14] Restartable sequences system call

[Linus Torvalds]

On Fri, Oct 13, 2017 at 8:01 PM, Andi Kleen  wrote:
>
> As far as I can see the current model fundamentally only works for
> one user per process (because there is only a single range and abort IP)

No, it should work for libraries, you just need to always initialize
the proper start/commit/abort IP's for every transaction. Then
everybody should be fine.

So I _think_ it's all good. But I really would want to see that
actually being the case.

Linus

[PATCH 5/7] drm/sun4i: backend: Offset layer buffer address by DRAM starting address

[Chen-Yu Tsai]

The display backend, as well as other peripherals that have a DRAM
clock gate and access DRAM directly, bypassing the system bus,
address the DRAM starting from 0x0, while physical addresses the
system uses starts from 0x4000 (or 0x2000 in A80's case).

Correct the address configured into the backend layer registers
by PHYS_OFFSET to account for this.

Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 4fefd8add714..d18d7e88d511 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -216,6 +216,13 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend 
*backend,
paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", );
 
+   /*
+* backend DMA accesses DRAM directly, bypassing the system
+* bus. As such, the address range is different and the buffer
+* address needs to be corrected.
+*/
+   paddr -= PHYS_OFFSET;
+
/* Write the 32 lower bits of the address (in bits) */
lo_paddr = paddr << 3;
DRM_DEBUG_DRIVER("Setting address lower bits to 0x%x\n", lo_paddr);
-- 
2.14.2

[PATCH 7/7] drm/sun4i: hdmi: Move PAD_CTRL1 setting to mode_set function

[Chen-Yu Tsai]

Initially we configured the PAD_CTRL1 register at probe/bind time.
However it seems the HDMI controller will modify some of the bits
in this register by itself. On the A10 it is particularly annoying
as it toggles the output invert bits, which inverts the colors on
the display output.

The U-boot driver this driver is based on sets this register twice,
though it seems it's only needed for actual display output. Hence
we move it to the mode_set function.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c 
b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 027b5608dbe6..6ca6e6a74c4a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -144,6 +144,22 @@ static void sun4i_hdmi_mode_set(struct drm_encoder 
*encoder,
writel(SUN4I_HDMI_UNKNOWN_INPUT_SYNC,
   hdmi->base + SUN4I_HDMI_UNKNOWN_REG);
 
+   /*
+* Setup output pad (?) controls
+*
+* This is done here instead of at probe/bind time because
+* the controller seems to toggle some of the bits on its own.
+*
+* We can't just initialize the register there, we need to
+* protect the clock bits that have already been read out and
+* cached by the clock framework.
+*/
+   val = readl(hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
+   val &= SUN4I_HDMI_PAD_CTRL1_HALVE_CLK;
+   val |= hdmi->variant->pad_ctrl1_init_val;
+   writel(val, hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
+   val = readl(hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
+
/* Setup timing registers */
writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |
   SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),
@@ -489,16 +505,6 @@ static int sun4i_hdmi_bind(struct device *dev, struct 
device *master,
writel(hdmi->variant->pad_ctrl0_init_val,
   hdmi->base + SUN4I_HDMI_PAD_CTRL0_REG);
 
-   /*
-* We can't just initialize the register there, we need to
-* protect the clock bits that have already been read out and
-* cached by the clock framework.
-*/
-   reg = readl(hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
-   reg &= SUN4I_HDMI_PAD_CTRL1_HALVE_CLK;
-   reg |= hdmi->variant->pad_ctrl1_init_val;
-   writel(reg, hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
-
reg = readl(hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);
reg &= SUN4I_HDMI_PLL_CTRL_DIV_MASK;
reg |= hdmi->variant->pll_ctrl_init_val;
-- 
2.14.2

[PATCH 1/7] drm/sun4i: don't add components that are already in the queue

[Chen-Yu Tsai]

Even though the components framework can handle duplicate entries,
the extra entries cause a lot more debug messages to be generated,
which would be confusing to developers not familiar with our driver
and the framework in general.

Instead, we can scan the relatively small queue and check if the
component to be added is already queued up. Since the display
pipelines are symmetrical (not considering the third display
pipeline on the A80), and we add components level by level, when
we get to the second instance at the same level, any shared downstream
components would already be in the queue.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
b/drivers/gpu/drm/sun4i/sun4i_drv.c
index a2012638d5f7..b5879d4620d8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -226,6 +226,18 @@ struct endpoint_list {
struct list_head list;
 };
 
+static bool node_is_in_list(struct list_head *endpoints,
+   struct device_node *node)
+{
+   struct endpoint_list *endpoint;
+
+   list_for_each_entry(endpoint, endpoints, list)
+   if (endpoint->node == node)
+   return true;
+
+   return false;
+}
+
 static int sun4i_drv_add_endpoints(struct device *dev,
   struct list_head *endpoints,
   struct component_match **match,
@@ -292,6 +304,10 @@ static int sun4i_drv_add_endpoints(struct device *dev,
}
}
 
+   /* skip downstream node if it is already in the queue */
+   if (node_is_in_list(endpoints, remote))
+   continue;
+
/* Add downstream nodes to the queue */
endpoint = kzalloc(sizeof(*endpoint), GFP_KERNEL);
if (!endpoint) {
-- 
2.14.2

[PATCH 5/7] drm/sun4i: backend: Offset layer buffer address by DRAM starting address

[Chen-Yu Tsai]

The display backend, as well as other peripherals that have a DRAM
clock gate and access DRAM directly, bypassing the system bus,
address the DRAM starting from 0x0, while physical addresses the
system uses starts from 0x4000 (or 0x2000 in A80's case).

Correct the address configured into the backend layer registers
by PHYS_OFFSET to account for this.

Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 4fefd8add714..d18d7e88d511 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -216,6 +216,13 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend 
*backend,
paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", );
 
+   /*
+* backend DMA accesses DRAM directly, bypassing the system
+* bus. As such, the address range is different and the buffer
+* address needs to be corrected.
+*/
+   paddr -= PHYS_OFFSET;
+
/* Write the 32 lower bits of the address (in bits) */
lo_paddr = paddr << 3;
DRM_DEBUG_DRIVER("Setting address lower bits to 0x%x\n", lo_paddr);
-- 
2.14.2

[PATCH 7/7] drm/sun4i: hdmi: Move PAD_CTRL1 setting to mode_set function

[Chen-Yu Tsai]

Initially we configured the PAD_CTRL1 register at probe/bind time.
However it seems the HDMI controller will modify some of the bits
in this register by itself. On the A10 it is particularly annoying
as it toggles the output invert bits, which inverts the colors on
the display output.

The U-boot driver this driver is based on sets this register twice,
though it seems it's only needed for actual display output. Hence
we move it to the mode_set function.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c 
b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 027b5608dbe6..6ca6e6a74c4a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -144,6 +144,22 @@ static void sun4i_hdmi_mode_set(struct drm_encoder 
*encoder,
writel(SUN4I_HDMI_UNKNOWN_INPUT_SYNC,
   hdmi->base + SUN4I_HDMI_UNKNOWN_REG);
 
+   /*
+* Setup output pad (?) controls
+*
+* This is done here instead of at probe/bind time because
+* the controller seems to toggle some of the bits on its own.
+*
+* We can't just initialize the register there, we need to
+* protect the clock bits that have already been read out and
+* cached by the clock framework.
+*/
+   val = readl(hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
+   val &= SUN4I_HDMI_PAD_CTRL1_HALVE_CLK;
+   val |= hdmi->variant->pad_ctrl1_init_val;
+   writel(val, hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
+   val = readl(hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
+
/* Setup timing registers */
writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |
   SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),
@@ -489,16 +505,6 @@ static int sun4i_hdmi_bind(struct device *dev, struct 
device *master,
writel(hdmi->variant->pad_ctrl0_init_val,
   hdmi->base + SUN4I_HDMI_PAD_CTRL0_REG);
 
-   /*
-* We can't just initialize the register there, we need to
-* protect the clock bits that have already been read out and
-* cached by the clock framework.
-*/
-   reg = readl(hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
-   reg &= SUN4I_HDMI_PAD_CTRL1_HALVE_CLK;
-   reg |= hdmi->variant->pad_ctrl1_init_val;
-   writel(reg, hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
-
reg = readl(hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);
reg &= SUN4I_HDMI_PLL_CTRL_DIV_MASK;
reg |= hdmi->variant->pll_ctrl_init_val;
-- 
2.14.2

[PATCH 1/7] drm/sun4i: don't add components that are already in the queue

[Chen-Yu Tsai]

Even though the components framework can handle duplicate entries,
the extra entries cause a lot more debug messages to be generated,
which would be confusing to developers not familiar with our driver
and the framework in general.

Instead, we can scan the relatively small queue and check if the
component to be added is already queued up. Since the display
pipelines are symmetrical (not considering the third display
pipeline on the A80), and we add components level by level, when
we get to the second instance at the same level, any shared downstream
components would already be in the queue.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
b/drivers/gpu/drm/sun4i/sun4i_drv.c
index a2012638d5f7..b5879d4620d8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -226,6 +226,18 @@ struct endpoint_list {
struct list_head list;
 };
 
+static bool node_is_in_list(struct list_head *endpoints,
+   struct device_node *node)
+{
+   struct endpoint_list *endpoint;
+
+   list_for_each_entry(endpoint, endpoints, list)
+   if (endpoint->node == node)
+   return true;
+
+   return false;
+}
+
 static int sun4i_drv_add_endpoints(struct device *dev,
   struct list_head *endpoints,
   struct component_match **match,
@@ -292,6 +304,10 @@ static int sun4i_drv_add_endpoints(struct device *dev,
}
}
 
+   /* skip downstream node if it is already in the queue */
+   if (node_is_in_list(endpoints, remote))
+   continue;
+
/* Add downstream nodes to the queue */
endpoint = kzalloc(sizeof(*endpoint), GFP_KERNEL);
if (!endpoint) {
-- 
2.14.2

[PATCH 0/7] drm/sun4i: More cleanups

[Chen-Yu Tsai]

Hi,

Here's another bunch of cleanups for sun4i-drm. Most of these were
found while working on A10/A20 DRM and HDMI support. To be clear,
nothing was broken before these patches.

Patch 1 trims the sun4i-drm probe sequence by not adding repeating
components. The component can deal with duplicates, but it leads
to excessive debug logs which is confusing for developers not in
the know.

Patch 2 moves regmap creation to a point where access is actually
possible, i.e. clocks enabled and reset controls deasserted.

Patch 3 moves to the new drm_fb_cma_get_gem_addr() helper instead
of open coding the same thing.

Patch 4 expands the comment explaining why we clear the backend
registers explicitly.

Patch 5 fixes the buffer address programmed into the backend layer
registers. This issue only hits devices with more than 1GB RAM.

Patch 6 documents some newly discovered but unused register bits
in the HDMI controller.

Patch 7 delays setting of the PAD_CTRL1 register in the HDMI controller
to the mode_set function. The main reason to do this is that between
probe time and when the display is actually setup, the controller itself
toggles some of the bits in this register. In particular, on the A10
it toggles the invert bits documented in the previous patch. This
causes the color to be completed inverted.

Please have a look.

Regards
ChenYu

Chen-Yu Tsai (7):
  drm/sun4i: don't add components that are already in the queue
  drm/sun4i: backend: Create regmap after access is possible
  drm/sun4i: backend: Use drm_fb_cma_get_gem_addr() to get display
memory
  drm/sun4i: backend: Add comment explaining why registers are cleared
  drm/sun4i: backend: Offset layer buffer address by DRAM starting
address
  drm/sun4i: hdmi: Document PAD_CTRL1 output invert bits
  drm/sun4i: hdmi: Move PAD_CTRL1 setting to mode_set function

 drivers/gpu/drm/sun4i/sun4i_backend.c  | 45 ++
 drivers/gpu/drm/sun4i/sun4i_drv.c  | 16 
 drivers/gpu/drm/sun4i/sun4i_hdmi.h |  5 
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 26 
 4 files changed, 61 insertions(+), 31 deletions(-)

-- 
2.14.2

[PATCH 0/7] drm/sun4i: More cleanups

[Chen-Yu Tsai]

Hi,

Here's another bunch of cleanups for sun4i-drm. Most of these were
found while working on A10/A20 DRM and HDMI support. To be clear,
nothing was broken before these patches.

Patch 1 trims the sun4i-drm probe sequence by not adding repeating
components. The component can deal with duplicates, but it leads
to excessive debug logs which is confusing for developers not in
the know.

Patch 2 moves regmap creation to a point where access is actually
possible, i.e. clocks enabled and reset controls deasserted.

Patch 3 moves to the new drm_fb_cma_get_gem_addr() helper instead
of open coding the same thing.

Patch 4 expands the comment explaining why we clear the backend
registers explicitly.

Patch 5 fixes the buffer address programmed into the backend layer
registers. This issue only hits devices with more than 1GB RAM.

Patch 6 documents some newly discovered but unused register bits
in the HDMI controller.

Patch 7 delays setting of the PAD_CTRL1 register in the HDMI controller
to the mode_set function. The main reason to do this is that between
probe time and when the display is actually setup, the controller itself
toggles some of the bits in this register. In particular, on the A10
it toggles the invert bits documented in the previous patch. This
causes the color to be completed inverted.

Please have a look.

Regards
ChenYu

Chen-Yu Tsai (7):
  drm/sun4i: don't add components that are already in the queue
  drm/sun4i: backend: Create regmap after access is possible
  drm/sun4i: backend: Use drm_fb_cma_get_gem_addr() to get display
memory
  drm/sun4i: backend: Add comment explaining why registers are cleared
  drm/sun4i: backend: Offset layer buffer address by DRAM starting
address
  drm/sun4i: hdmi: Document PAD_CTRL1 output invert bits
  drm/sun4i: hdmi: Move PAD_CTRL1 setting to mode_set function

 drivers/gpu/drm/sun4i/sun4i_backend.c  | 45 ++
 drivers/gpu/drm/sun4i/sun4i_drv.c  | 16 
 drivers/gpu/drm/sun4i/sun4i_hdmi.h |  5 
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 26 
 4 files changed, 61 insertions(+), 31 deletions(-)

-- 
2.14.2

[PATCH 6/7] drm/sun4i: hdmi: Document PAD_CTRL1 output invert bits

[Chen-Yu Tsai]

While debugging inverted color from the HDMI output on the A10, I
found that the lowest 3 bits were set. These were cleared on A20
boards that had normal display output. By manually toggling these
bits the mapping of the color components to these bits was found.

While these are not used anywhere, it would be nice to document
them somewhere.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_hdmi.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi.h 
b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
index 9b97da39927e..b685ee11623d 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
@@ -72,6 +72,11 @@
 #define SUN4I_HDMI_PAD_CTRL1_HALVE_CLK BIT(6)
 #define SUN4I_HDMI_PAD_CTRL1_REG_AMP(n)(((n) & 7) << 3)
 
+/* These bits seem to invert the TMDS data channels */
+#define SUN4I_HDMI_PAD_CTRL1_INVERT_R  BIT(2)
+#define SUN4I_HDMI_PAD_CTRL1_INVERT_G  BIT(1)
+#define SUN4I_HDMI_PAD_CTRL1_INVERT_B  BIT(0)
+
 #define SUN4I_HDMI_PLL_CTRL_REG0x208
 #define SUN4I_HDMI_PLL_CTRL_PLL_EN BIT(31)
 #define SUN4I_HDMI_PLL_CTRL_BWSBIT(30)
-- 
2.14.2

[PATCH 4/7] drm/sun4i: backend: Add comment explaining why registers are cleared

[Chen-Yu Tsai]

Many of the backend's layer configuration registers have undefined
default values. This poses a risk as we use regmap_update_bits in
some places, and don't overwrite the whole register.

At probe/bind time we explicitly clear all the control registers
by writing 0 to them. This patch adds a more detailed explanation
on why we're doing this.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 243ddfdc9403..4fefd8add714 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -412,7 +412,14 @@ static int sun4i_backend_bind(struct device *dev, struct 
device *master,
 
list_add_tail(>engine.list, >engine_list);
 
-   /* Reset the registers */
+   /*
+* Many of the backend's layer configuration registers have
+* undefined default values. This poses a risk as we use
+* regmap_update_bits in some places, and don't overwrite
+* the whole register.
+*
+* Clear the registers here to have something predictable.
+*/
for (i = 0x800; i < 0x1000; i += 4)
regmap_write(backend->engine.regs, i, 0);
 
-- 
2.14.2

[PATCH 6/7] drm/sun4i: hdmi: Document PAD_CTRL1 output invert bits

[Chen-Yu Tsai]

While debugging inverted color from the HDMI output on the A10, I
found that the lowest 3 bits were set. These were cleared on A20
boards that had normal display output. By manually toggling these
bits the mapping of the color components to these bits was found.

While these are not used anywhere, it would be nice to document
them somewhere.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_hdmi.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi.h 
b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
index 9b97da39927e..b685ee11623d 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
@@ -72,6 +72,11 @@
 #define SUN4I_HDMI_PAD_CTRL1_HALVE_CLK BIT(6)
 #define SUN4I_HDMI_PAD_CTRL1_REG_AMP(n)(((n) & 7) << 3)
 
+/* These bits seem to invert the TMDS data channels */
+#define SUN4I_HDMI_PAD_CTRL1_INVERT_R  BIT(2)
+#define SUN4I_HDMI_PAD_CTRL1_INVERT_G  BIT(1)
+#define SUN4I_HDMI_PAD_CTRL1_INVERT_B  BIT(0)
+
 #define SUN4I_HDMI_PLL_CTRL_REG0x208
 #define SUN4I_HDMI_PLL_CTRL_PLL_EN BIT(31)
 #define SUN4I_HDMI_PLL_CTRL_BWSBIT(30)
-- 
2.14.2

[PATCH 4/7] drm/sun4i: backend: Add comment explaining why registers are cleared

[Chen-Yu Tsai]

Many of the backend's layer configuration registers have undefined
default values. This poses a risk as we use regmap_update_bits in
some places, and don't overwrite the whole register.

At probe/bind time we explicitly clear all the control registers
by writing 0 to them. This patch adds a more detailed explanation
on why we're doing this.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 243ddfdc9403..4fefd8add714 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -412,7 +412,14 @@ static int sun4i_backend_bind(struct device *dev, struct 
device *master,
 
list_add_tail(>engine.list, >engine_list);
 
-   /* Reset the registers */
+   /*
+* Many of the backend's layer configuration registers have
+* undefined default values. This poses a risk as we use
+* regmap_update_bits in some places, and don't overwrite
+* the whole register.
+*
+* Clear the registers here to have something predictable.
+*/
for (i = 0x800; i < 0x1000; i += 4)
regmap_write(backend->engine.regs, i, 0);
 
-- 
2.14.2

[PATCH 3/7] drm/sun4i: backend: Use drm_fb_cma_get_gem_addr() to get display memory

[Chen-Yu Tsai]

Commit 4636ce93d5b2 ("drm/fb-cma-helper: Add drm_fb_cma_get_gem_addr()")
adds a new helper, which covers fetching a drm_framebuffer's GEM object
and calculating the buffer address for a given plane.

This patch uses this helper to replace our own open coded version of the
same function.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 1cc1780f5091..243ddfdc9403 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -209,22 +209,11 @@ int sun4i_backend_update_layer_buffer(struct 
sun4i_backend *backend,
 {
struct drm_plane_state *state = plane->state;
struct drm_framebuffer *fb = state->fb;
-   struct drm_gem_cma_object *gem;
u32 lo_paddr, hi_paddr;
dma_addr_t paddr;
-   int bpp;
-
-   /* Get the physical address of the buffer in memory */
-   gem = drm_fb_cma_get_gem_obj(fb, 0);
-
-   DRM_DEBUG_DRIVER("Using GEM @ %pad\n", >paddr);
-
-   /* Compute the start of the displayed memory */
-   bpp = fb->format->cpp[0];
-   paddr = gem->paddr + fb->offsets[0];
-   paddr += (state->src_x >> 16) * bpp;
-   paddr += (state->src_y >> 16) * fb->pitches[0];
 
+   /* Get the start of the displayed memory */
+   paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", );
 
/* Write the 32 lower bits of the address (in bits) */
-- 
2.14.2

[PATCH 3/7] drm/sun4i: backend: Use drm_fb_cma_get_gem_addr() to get display memory

[Chen-Yu Tsai]

Commit 4636ce93d5b2 ("drm/fb-cma-helper: Add drm_fb_cma_get_gem_addr()")
adds a new helper, which covers fetching a drm_framebuffer's GEM object
and calculating the buffer address for a given plane.

This patch uses this helper to replace our own open coded version of the
same function.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 1cc1780f5091..243ddfdc9403 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -209,22 +209,11 @@ int sun4i_backend_update_layer_buffer(struct 
sun4i_backend *backend,
 {
struct drm_plane_state *state = plane->state;
struct drm_framebuffer *fb = state->fb;
-   struct drm_gem_cma_object *gem;
u32 lo_paddr, hi_paddr;
dma_addr_t paddr;
-   int bpp;
-
-   /* Get the physical address of the buffer in memory */
-   gem = drm_fb_cma_get_gem_obj(fb, 0);
-
-   DRM_DEBUG_DRIVER("Using GEM @ %pad\n", >paddr);
-
-   /* Compute the start of the displayed memory */
-   bpp = fb->format->cpp[0];
-   paddr = gem->paddr + fb->offsets[0];
-   paddr += (state->src_x >> 16) * bpp;
-   paddr += (state->src_y >> 16) * fb->pitches[0];
 
+   /* Get the start of the displayed memory */
+   paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", );
 
/* Write the 32 lower bits of the address (in bits) */
-- 
2.14.2

[PATCH 2/7] drm/sun4i: backend: Create regmap after access is possible

[Chen-Yu Tsai]

The backend has various clocks and reset controls that need to be
enabled and deasserted before register access is possible.

Move the creation of the regmap to after the clocks and reset controls
have been configured where it makes more sense.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index ec5943627aa5..1cc1780f5091 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -369,13 +369,6 @@ static int sun4i_backend_bind(struct device *dev, struct 
device *master,
if (IS_ERR(regs))
return PTR_ERR(regs);
 
-   backend->engine.regs = devm_regmap_init_mmio(dev, regs,
-
_backend_regmap_config);
-   if (IS_ERR(backend->engine.regs)) {
-   dev_err(dev, "Couldn't create the backend regmap\n");
-   return PTR_ERR(backend->engine.regs);
-   }
-
backend->reset = devm_reset_control_get(dev, NULL);
if (IS_ERR(backend->reset)) {
dev_err(dev, "Couldn't get our reset line\n");
@@ -421,6 +414,13 @@ static int sun4i_backend_bind(struct device *dev, struct 
device *master,
}
}
 
+   backend->engine.regs = devm_regmap_init_mmio(dev, regs,
+
_backend_regmap_config);
+   if (IS_ERR(backend->engine.regs)) {
+   dev_err(dev, "Couldn't create the backend regmap\n");
+   return PTR_ERR(backend->engine.regs);
+   }
+
list_add_tail(>engine.list, >engine_list);
 
/* Reset the registers */
-- 
2.14.2

[PATCH 2/7] drm/sun4i: backend: Create regmap after access is possible

[Chen-Yu Tsai]

The backend has various clocks and reset controls that need to be
enabled and deasserted before register access is possible.

Move the creation of the regmap to after the clocks and reset controls
have been configured where it makes more sense.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index ec5943627aa5..1cc1780f5091 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -369,13 +369,6 @@ static int sun4i_backend_bind(struct device *dev, struct 
device *master,
if (IS_ERR(regs))
return PTR_ERR(regs);
 
-   backend->engine.regs = devm_regmap_init_mmio(dev, regs,
-
_backend_regmap_config);
-   if (IS_ERR(backend->engine.regs)) {
-   dev_err(dev, "Couldn't create the backend regmap\n");
-   return PTR_ERR(backend->engine.regs);
-   }
-
backend->reset = devm_reset_control_get(dev, NULL);
if (IS_ERR(backend->reset)) {
dev_err(dev, "Couldn't get our reset line\n");
@@ -421,6 +414,13 @@ static int sun4i_backend_bind(struct device *dev, struct 
device *master,
}
}
 
+   backend->engine.regs = devm_regmap_init_mmio(dev, regs,
+
_backend_regmap_config);
+   if (IS_ERR(backend->engine.regs)) {
+   dev_err(dev, "Couldn't create the backend regmap\n");
+   return PTR_ERR(backend->engine.regs);
+   }
+
list_add_tail(>engine.list, >engine_list);
 
/* Reset the registers */
-- 
2.14.2

Re: [linux-sunxi] Re: [PATCH v4 00/11] dmaengine: sun6i: Fixes for H3/A83T, enable A64

[Chen-Yu Tsai]

Hi Vinod,

On Thu, Sep 28, 2017 at 11:26 PM, Maxime Ripard
 wrote:
> On Thu, Sep 28, 2017 at 01:49:17AM +, Stefan Brüns wrote:
>> Commit 3a03ea763a67 ("dmaengine: sun6i: Add support for Allwinner A83T
>> (sun8i) variant") and commit f008db8c00c1 ("dmaengine: sun6i: Add support for
>> Allwinner H3 (sun8i) variant") added support for the A83T resp. H3, but 
>> missed
>> some differences between the original A31 and A83T/H3.
>>
>> The first patch adds a callback to the controller config to set the clock
>> autogating register of different SoC generations, i.e. A31, A23+A83T, 
>> H3+later,
>> and uses it to for the correct clock autogating setting.
>>
>> The second patch adds a callback for the burst length setting in the channel
>> config register, which has different field offsets and new burst 
>> widths/lengths,
>> which differs between H3 and earlier generations
>>
>> The third patch restructures some code required for the fourth patch and 
>> adds the
>> burst lengths to the controller config.
>>
>> The fourth patch adds the burst widths to the config and adds the handling 
>> of the
>> H3 specific burst widths.
>>
>> Patch 5 restructures the code to decouple some controller details (e.g. 
>> channel
>> count) from the compatible string/the config.
>>
>> Patches 6, 7 and 8 introduce and use the "dma-chans" property for the A64. 
>> Although
>> register compatible to the H3, the channel count differs and thus it 
>> requires a
>> new compatible. To avoid introduction of new compatibles for each minor 
>> variation,
>> anything but the register model is moved to devicetree properties. There
>> is at least one SoC (R40) which can then reuse the A64 compatible, the same
>> would have worked for A83T+V3s.
>>
>> Patches 9 and 10 add the DMA controller node to the devicetree and add the 
>> DMA
>> controller reference to the SPI nodes.
>>
>> Patch 11 fixes a small error in the devicetree binding example.
>
> Applied patches 9-11, thanks!
> Maxime

Can you pick up patches 1-8?

Thanks!
ChenYu

Re: [linux-sunxi] Re: [PATCH v4 00/11] dmaengine: sun6i: Fixes for H3/A83T, enable A64

[Chen-Yu Tsai]

Hi Vinod,

On Thu, Sep 28, 2017 at 11:26 PM, Maxime Ripard
 wrote:
> On Thu, Sep 28, 2017 at 01:49:17AM +, Stefan Brüns wrote:
>> Commit 3a03ea763a67 ("dmaengine: sun6i: Add support for Allwinner A83T
>> (sun8i) variant") and commit f008db8c00c1 ("dmaengine: sun6i: Add support for
>> Allwinner H3 (sun8i) variant") added support for the A83T resp. H3, but 
>> missed
>> some differences between the original A31 and A83T/H3.
>>
>> The first patch adds a callback to the controller config to set the clock
>> autogating register of different SoC generations, i.e. A31, A23+A83T, 
>> H3+later,
>> and uses it to for the correct clock autogating setting.
>>
>> The second patch adds a callback for the burst length setting in the channel
>> config register, which has different field offsets and new burst 
>> widths/lengths,
>> which differs between H3 and earlier generations
>>
>> The third patch restructures some code required for the fourth patch and 
>> adds the
>> burst lengths to the controller config.
>>
>> The fourth patch adds the burst widths to the config and adds the handling 
>> of the
>> H3 specific burst widths.
>>
>> Patch 5 restructures the code to decouple some controller details (e.g. 
>> channel
>> count) from the compatible string/the config.
>>
>> Patches 6, 7 and 8 introduce and use the "dma-chans" property for the A64. 
>> Although
>> register compatible to the H3, the channel count differs and thus it 
>> requires a
>> new compatible. To avoid introduction of new compatibles for each minor 
>> variation,
>> anything but the register model is moved to devicetree properties. There
>> is at least one SoC (R40) which can then reuse the A64 compatible, the same
>> would have worked for A83T+V3s.
>>
>> Patches 9 and 10 add the DMA controller node to the devicetree and add the 
>> DMA
>> controller reference to the SPI nodes.
>>
>> Patch 11 fixes a small error in the devicetree binding example.
>
> Applied patches 9-11, thanks!
> Maxime

Can you pick up patches 1-8?

Thanks!
ChenYu

Re: [PATCH 0/2] kbuild: use relative path from $(srctree) instead of __FILE__

[Masahiro Yamada]

2017-10-12 18:56 GMT+09:00 Masahiro Yamada :
>
> Kbuild works in objtree, not in srctree.  So, __FILE__ is prefixed
> with $(srctree)/ for out-of-tree build.
>
> For example, WARN_ON() will look as follows if you built your kernel
> out of source tree:
>
> WARNING: CPU: 1 PID: 1 at /path/to/build/directory/arch/arm64/kernel/foo.c:...
>
> With this series, it will always look like follows regardless of O= option.
>
> WARNING: CPU: 1 PID: 1 at arch/arm64/kernel/foo.c:...
>
> If GCC does not support -Wno-builtin-macro-redefined (i.e. gcc version < 4.4),
> the output is prefixed with absolute path.
>

I expect to replace __FILE__ with its relative path,
but actually it replaces __FILE__ with relative path of __BASE_FILE__.
This will change the behavior in several places.

I take back this series.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/2] kbuild: use relative path from $(srctree) instead of __FILE__

[Masahiro Yamada]

2017-10-12 18:56 GMT+09:00 Masahiro Yamada :
>
> Kbuild works in objtree, not in srctree.  So, __FILE__ is prefixed
> with $(srctree)/ for out-of-tree build.
>
> For example, WARN_ON() will look as follows if you built your kernel
> out of source tree:
>
> WARNING: CPU: 1 PID: 1 at /path/to/build/directory/arch/arm64/kernel/foo.c:...
>
> With this series, it will always look like follows regardless of O= option.
>
> WARNING: CPU: 1 PID: 1 at arch/arm64/kernel/foo.c:...
>
> If GCC does not support -Wno-builtin-macro-redefined (i.e. gcc version < 4.4),
> the output is prefixed with absolute path.
>

I expect to replace __FILE__ with its relative path,
but actually it replaces __FILE__ with relative path of __BASE_FILE__.
This will change the behavior in several places.

I take back this series.


-- 
Best Regards
Masahiro Yamada

Re: [RFC PATCH v9 for 4.15 01/14] Restartable sequences system call

[Andi Kleen]

> That sounds so obvious and stupid that you might go "What do you
> mean?", but for things to work for libraries, they have to work
> together with *other* users, and with *independent* users.

As far as I can see the current model fundamentally only works for
one user per process (because there is only a single range and abort IP) 

So once malloc started using it noone else could.

Since malloc is the primary user it would be pointless to ever
try it on something else.

It seems fixing that would complicate everything quite a bit --
all ranges would need to be (unlimited?) arrays.

-Andi

Re: [RFC PATCH v9 for 4.15 01/14] Restartable sequences system call

[Andi Kleen]

> That sounds so obvious and stupid that you might go "What do you
> mean?", but for things to work for libraries, they have to work
> together with *other* users, and with *independent* users.

As far as I can see the current model fundamentally only works for
one user per process (because there is only a single range and abort IP) 

So once malloc started using it noone else could.

Since malloc is the primary user it would be pointless to ever
try it on something else.

It seems fixing that would complicate everything quite a bit --
all ranges would need to be (unlimited?) arrays.

-Andi

Re: [PATCH] ecryptfs: Fix stat command displays wrong file size in xattr region

[Jason Xing]

Could anyone take a look at this patch which fixes the xattr-read
issue? Thanks anyway.

Jason

On Thu, Jun 22, 2017 at 3:21 PM, Jason Xing  wrote:
> When doing ecryptfs_read_and_validate_xattr_region(), eCryptfs
> reads only 16 bytes from xattr region. However, the lower filesystem
> like ext4 always compares 16 with the size of ecryptfs xattr region
> which is 81 bytes, and then it will return ERANGE (-34) and do not
> read that region.
>
> Signed-off-by: Jason Xing 
> ---
>  fs/ecryptfs/crypto.c | 33 +
>  1 file changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index e5e29f8..895140f 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -1389,19 +1389,36 @@ int ecryptfs_read_xattr_region(char *page_virt, 
> struct inode *ecryptfs_inode)
>  int ecryptfs_read_and_validate_xattr_region(struct dentry *dentry,
> struct inode *inode)
>  {
> -   u8 file_size[ECRYPTFS_SIZE_AND_MARKER_BYTES];
> -   u8 *marker = file_size + ECRYPTFS_FILE_SIZE_BYTES;
> +   char *page_virt;
> int rc;
>
> +   page_virt = kmem_cache_alloc(ecryptfs_header_cache, GFP_USER);
> +   if (!page_virt) {
> +   rc = -ENOMEM;
> +   printk(KERN_ERR "%s: Unable to allocate page_virt\n",
> +  __func__);
> +   goto out;
> +   }
> rc = ecryptfs_getxattr_lower(ecryptfs_dentry_to_lower(dentry),
>  ecryptfs_inode_to_lower(inode),
> -ECRYPTFS_XATTR_NAME, file_size,
> -ECRYPTFS_SIZE_AND_MARKER_BYTES);
> -   if (rc < ECRYPTFS_SIZE_AND_MARKER_BYTES)
> -   return rc >= 0 ? -EINVAL : rc;
> -   rc = ecryptfs_validate_marker(marker);
> +ECRYPTFS_XATTR_NAME, page_virt,
> +ECRYPTFS_DEFAULT_EXTENT_SIZE);
> +   if (rc < 0) {
> +   if (unlikely(ecryptfs_verbosity > 0))
> +   printk(KERN_INFO "Error attempting to read the [%s] "
> +  "xattr from the lower file; return value = "
> +  "[%d]\n", ECRYPTFS_XATTR_NAME, rc);
> +   rc = -EINVAL;
> +   goto out;
> +   }
> +   rc = ecryptfs_validate_marker(page_virt + ECRYPTFS_FILE_SIZE_BYTES);
> if (!rc)
> -   ecryptfs_i_size_init(file_size, inode);
> +   ecryptfs_i_size_init(page_virt, inode);
> +out:
> +   if (page_virt) {
> +   memset(page_virt, 0, PAGE_SIZE);
> +   kmem_cache_free(ecryptfs_header_cache, page_virt);
> +   }
> return rc;
>  }
>
> --
> 2.7.4
>

Re: [PATCH] ecryptfs: Fix stat command displays wrong file size in xattr region

[Jason Xing]

Could anyone take a look at this patch which fixes the xattr-read
issue? Thanks anyway.

Jason

On Thu, Jun 22, 2017 at 3:21 PM, Jason Xing  wrote:
> When doing ecryptfs_read_and_validate_xattr_region(), eCryptfs
> reads only 16 bytes from xattr region. However, the lower filesystem
> like ext4 always compares 16 with the size of ecryptfs xattr region
> which is 81 bytes, and then it will return ERANGE (-34) and do not
> read that region.
>
> Signed-off-by: Jason Xing 
> ---
>  fs/ecryptfs/crypto.c | 33 +
>  1 file changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index e5e29f8..895140f 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -1389,19 +1389,36 @@ int ecryptfs_read_xattr_region(char *page_virt, 
> struct inode *ecryptfs_inode)
>  int ecryptfs_read_and_validate_xattr_region(struct dentry *dentry,
> struct inode *inode)
>  {
> -   u8 file_size[ECRYPTFS_SIZE_AND_MARKER_BYTES];
> -   u8 *marker = file_size + ECRYPTFS_FILE_SIZE_BYTES;
> +   char *page_virt;
> int rc;
>
> +   page_virt = kmem_cache_alloc(ecryptfs_header_cache, GFP_USER);
> +   if (!page_virt) {
> +   rc = -ENOMEM;
> +   printk(KERN_ERR "%s: Unable to allocate page_virt\n",
> +  __func__);
> +   goto out;
> +   }
> rc = ecryptfs_getxattr_lower(ecryptfs_dentry_to_lower(dentry),
>  ecryptfs_inode_to_lower(inode),
> -ECRYPTFS_XATTR_NAME, file_size,
> -ECRYPTFS_SIZE_AND_MARKER_BYTES);
> -   if (rc < ECRYPTFS_SIZE_AND_MARKER_BYTES)
> -   return rc >= 0 ? -EINVAL : rc;
> -   rc = ecryptfs_validate_marker(marker);
> +ECRYPTFS_XATTR_NAME, page_virt,
> +ECRYPTFS_DEFAULT_EXTENT_SIZE);
> +   if (rc < 0) {
> +   if (unlikely(ecryptfs_verbosity > 0))
> +   printk(KERN_INFO "Error attempting to read the [%s] "
> +  "xattr from the lower file; return value = "
> +  "[%d]\n", ECRYPTFS_XATTR_NAME, rc);
> +   rc = -EINVAL;
> +   goto out;
> +   }
> +   rc = ecryptfs_validate_marker(page_virt + ECRYPTFS_FILE_SIZE_BYTES);
> if (!rc)
> -   ecryptfs_i_size_init(file_size, inode);
> +   ecryptfs_i_size_init(page_virt, inode);
> +out:
> +   if (page_virt) {
> +   memset(page_virt, 0, PAGE_SIZE);
> +   kmem_cache_free(ecryptfs_header_cache, page_virt);
> +   }
> return rc;
>  }
>
> --
> 2.7.4
>

[PATCH] PCI: hv: fix "spurious APIC interrupt" due to new vector reservation mode

[Dexuan Cui]

Last month the vector management code was reworked, and as a result of the
changes, e.g. commit 22d0b12f3560 ("genirq/irqdomain: Add force
reactivation flag to irq domains"), commit 4900be83602b ("x86/vector/msi:
Switch to global reservation mode") etc, now we must add this new flag
MSI_FLAG_MUST_REACTIVATE when calling pci_msi_create_irq_domain() on x86,
otherwise Hyper-V PCIe pass-through can't work, and we get:
"spurious APIC interrupt through vector ff on CPU#0, should never happen",
this is because no valid MSI/MSI-X vector is allocated.

Signed-off-by: Dexuan Cui 
Cc: Thomas Gleixner 
Cc: Bjorn Helgaas 
Cc: K. Y. Srinivasan 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
---

Currently the "reservation mode" patchset has not been in Linus's tree yet, so
today's mainline hasn't shown the issue, but today's linux-next has shown the
issue (PCIe pass-through on Hyper-V can't work).

I think any caller of pci_msi_create_irq_domain() that runs on x86 needs to
be updated, but besides pci-hyper I'm not sure which callers run on x86.

 drivers/pci/host/pci-hyperv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 0fe3ea1..35a7683 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1196,7 +1196,8 @@ static int hv_pcie_init_irq_domain(struct 
hv_pcibus_device *hbus)
hbus->msi_info.ops = _msi_ops;
hbus->msi_info.flags = (MSI_FLAG_USE_DEF_DOM_OPS |
MSI_FLAG_USE_DEF_CHIP_OPS | MSI_FLAG_MULTI_PCI_MSI |
-   MSI_FLAG_PCI_MSIX);
+   MSI_FLAG_PCI_MSIX |
+   MSI_FLAG_MUST_REACTIVATE);
hbus->msi_info.handler = handle_edge_irq;
hbus->msi_info.handler_name = "edge";
hbus->msi_info.data = hbus;
-- 
2.7.4

[PATCH] PCI: hv: fix "spurious APIC interrupt" due to new vector reservation mode

[Dexuan Cui]

Last month the vector management code was reworked, and as a result of the
changes, e.g. commit 22d0b12f3560 ("genirq/irqdomain: Add force
reactivation flag to irq domains"), commit 4900be83602b ("x86/vector/msi:
Switch to global reservation mode") etc, now we must add this new flag
MSI_FLAG_MUST_REACTIVATE when calling pci_msi_create_irq_domain() on x86,
otherwise Hyper-V PCIe pass-through can't work, and we get:
"spurious APIC interrupt through vector ff on CPU#0, should never happen",
this is because no valid MSI/MSI-X vector is allocated.

Signed-off-by: Dexuan Cui 
Cc: Thomas Gleixner 
Cc: Bjorn Helgaas 
Cc: K. Y. Srinivasan 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
---

Currently the "reservation mode" patchset has not been in Linus's tree yet, so
today's mainline hasn't shown the issue, but today's linux-next has shown the
issue (PCIe pass-through on Hyper-V can't work).

I think any caller of pci_msi_create_irq_domain() that runs on x86 needs to
be updated, but besides pci-hyper I'm not sure which callers run on x86.

 drivers/pci/host/pci-hyperv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 0fe3ea1..35a7683 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1196,7 +1196,8 @@ static int hv_pcie_init_irq_domain(struct 
hv_pcibus_device *hbus)
hbus->msi_info.ops = _msi_ops;
hbus->msi_info.flags = (MSI_FLAG_USE_DEF_DOM_OPS |
MSI_FLAG_USE_DEF_CHIP_OPS | MSI_FLAG_MULTI_PCI_MSI |
-   MSI_FLAG_PCI_MSIX);
+   MSI_FLAG_PCI_MSIX |
+   MSI_FLAG_MUST_REACTIVATE);
hbus->msi_info.handler = handle_edge_irq;
hbus->msi_info.handler_name = "edge";
hbus->msi_info.data = hbus;
-- 
2.7.4

Re: [RFC PATCH for 4.15 09/14] Provide cpu_opv system call

[Andi Kleen]

> + pagefault_disable();
> + switch (len) {
> + case 1:
> + if (__get_user(tmp._u8, (uint8_t __user *)p))
> + goto end;
> + tmp._u8 += (uint8_t)count;
> + if (__put_user(tmp._u8, (uint8_t __user *)p))
> + goto end;
> + break;

It seems the code size could be dramatically shrunk by using a function
pointer callback for the actual operation, and then avoiding all the
copies. Since this is only a fallback this shouldn't be a problem
(and modern branch predictors are fairly good in such situations anyways)

If you really want to keep the code it would be better if you used
some macros -- i bet there's a typo in here somewhere in this
forest.

-Andi

Re: [RFC PATCH for 4.15 09/14] Provide cpu_opv system call

[Andi Kleen]

> + pagefault_disable();
> + switch (len) {
> + case 1:
> + if (__get_user(tmp._u8, (uint8_t __user *)p))
> + goto end;
> + tmp._u8 += (uint8_t)count;
> + if (__put_user(tmp._u8, (uint8_t __user *)p))
> + goto end;
> + break;

It seems the code size could be dramatically shrunk by using a function
pointer callback for the actual operation, and then avoiding all the
copies. Since this is only a fallback this shouldn't be a problem
(and modern branch predictors are fairly good in such situations anyways)

If you really want to keep the code it would be better if you used
some macros -- i bet there's a typo in here somewhere in this
forest.

-Andi

Re: [PATCH v6 1/4] cramfs: direct memory access support

[Nicolas Pitre]

On Fri, 13 Oct 2017, Nicolas Pitre wrote:

> On Sat, 14 Oct 2017, Al Viro wrote:
> 
> > On Fri, Oct 13, 2017 at 04:09:23PM -0400, Nicolas Pitre wrote:
> > > On Fri, 13 Oct 2017, Al Viro wrote:
> > > 
> > > > OK...  I wonder if it should simply define stubs for kill_mtd_super(),
> > > > mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
> > > > cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
> > > > returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
> > > > in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
> > > > if (sb->s_mtd) {
> > > > if (sbi->mtd_point_size)
> > > > mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > > > kill_mtd_super(sb);
> > > > } else {
> > > > kill_block_super(sb);
> > > > }
> > > > kfree(sbi);
> > > 
> > > Well... Stubs have to be named differently or they conflict with 
> > > existing declarations. At that point that makes for more lines of code 
> > > compared to the current patch and the naming indirection makes it less 
> > > obvious when reading the code. Alternatively I could add those stubs in 
> > > the corresponding header files and #ifdef the existing declarations 
> > > away. That might look somewhat less cluttered in the main code but it 
> > > also hides what is actually going on and left me unconvinced. And I'm 
> > > not sure this is worth it in the end given this is not a common 
> > > occurrence in the kernel either.
> > 
> > What I mean is this (completely untested) for CONFIG_BLOCK side of things,
> > with something similar for CONFIG_MTD one:
> > 
> > Provide definitions of mount_bdev/kill_block_super() in case !CONFIG_BLOCK
> 
> Yes, that's what I thought you meant, which corresponds to the second 
> part of my comment above. And as I said I'm not convinced this hiding of 
> kernel config effects is better for understanding what is actually going 
> on locally, and my own preference is how things are right now.

Another case that your suggestion doesn't cover well is the ability to 
still have block device support in the kernel for other filesystems but 
_without_ block device support in the cramfs case. In other words, 
having CONFIG_BLOCK=y and CONFIG_CRAMFS_BLOCKDEV=n. This is a common 
case to have embedded devices with the root filesystem in flash while 
still needing access to a FAT filesystem on SD cards. Your stubs are 
conditional on CONFIG_BLOCK but that is not sufficient in this example.


Nicolas

Re: [PATCH v6 1/4] cramfs: direct memory access support

[Nicolas Pitre]

On Fri, 13 Oct 2017, Nicolas Pitre wrote:

> On Sat, 14 Oct 2017, Al Viro wrote:
> 
> > On Fri, Oct 13, 2017 at 04:09:23PM -0400, Nicolas Pitre wrote:
> > > On Fri, 13 Oct 2017, Al Viro wrote:
> > > 
> > > > OK...  I wonder if it should simply define stubs for kill_mtd_super(),
> > > > mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
> > > > cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
> > > > returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
> > > > in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
> > > > if (sb->s_mtd) {
> > > > if (sbi->mtd_point_size)
> > > > mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > > > kill_mtd_super(sb);
> > > > } else {
> > > > kill_block_super(sb);
> > > > }
> > > > kfree(sbi);
> > > 
> > > Well... Stubs have to be named differently or they conflict with 
> > > existing declarations. At that point that makes for more lines of code 
> > > compared to the current patch and the naming indirection makes it less 
> > > obvious when reading the code. Alternatively I could add those stubs in 
> > > the corresponding header files and #ifdef the existing declarations 
> > > away. That might look somewhat less cluttered in the main code but it 
> > > also hides what is actually going on and left me unconvinced. And I'm 
> > > not sure this is worth it in the end given this is not a common 
> > > occurrence in the kernel either.
> > 
> > What I mean is this (completely untested) for CONFIG_BLOCK side of things,
> > with something similar for CONFIG_MTD one:
> > 
> > Provide definitions of mount_bdev/kill_block_super() in case !CONFIG_BLOCK
> 
> Yes, that's what I thought you meant, which corresponds to the second 
> part of my comment above. And as I said I'm not convinced this hiding of 
> kernel config effects is better for understanding what is actually going 
> on locally, and my own preference is how things are right now.

Another case that your suggestion doesn't cover well is the ability to 
still have block device support in the kernel for other filesystems but 
_without_ block device support in the cramfs case. In other words, 
having CONFIG_BLOCK=y and CONFIG_CRAMFS_BLOCKDEV=n. This is a common 
case to have embedded devices with the root filesystem in flash while 
still needing access to a FAT filesystem on SD cards. Your stubs are 
conditional on CONFIG_BLOCK but that is not sufficient in this example.


Nicolas

Re: [PATCH v6 1/4] cramfs: direct memory access support

[Nicolas Pitre]

On Sat, 14 Oct 2017, Al Viro wrote:

> On Fri, Oct 13, 2017 at 04:09:23PM -0400, Nicolas Pitre wrote:
> > On Fri, 13 Oct 2017, Al Viro wrote:
> > 
> > > OK...  I wonder if it should simply define stubs for kill_mtd_super(),
> > > mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
> > > cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
> > > returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
> > > in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
> > >   if (sb->s_mtd) {
> > >   if (sbi->mtd_point_size)
> > >   mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > >   kill_mtd_super(sb);
> > >   } else {
> > >   kill_block_super(sb);
> > >   }
> > >   kfree(sbi);
> > 
> > Well... Stubs have to be named differently or they conflict with 
> > existing declarations. At that point that makes for more lines of code 
> > compared to the current patch and the naming indirection makes it less 
> > obvious when reading the code. Alternatively I could add those stubs in 
> > the corresponding header files and #ifdef the existing declarations 
> > away. That might look somewhat less cluttered in the main code but it 
> > also hides what is actually going on and left me unconvinced. And I'm 
> > not sure this is worth it in the end given this is not a common 
> > occurrence in the kernel either.
> 
> What I mean is this (completely untested) for CONFIG_BLOCK side of things,
> with something similar for CONFIG_MTD one:
> 
> Provide definitions of mount_bdev/kill_block_super() in case !CONFIG_BLOCK

Yes, that's what I thought you meant, which corresponds to the second 
part of my comment above. And as I said I'm not convinced this hiding of 
kernel config effects is better for understanding what is actually going 
on locally, and my own preference is how things are right now.

But if you confirm you really want things that other way then I'll 
oblige and repost.


Nicolas

Re: [PATCH v6 1/4] cramfs: direct memory access support

[Nicolas Pitre]

On Sat, 14 Oct 2017, Al Viro wrote:

> On Fri, Oct 13, 2017 at 04:09:23PM -0400, Nicolas Pitre wrote:
> > On Fri, 13 Oct 2017, Al Viro wrote:
> > 
> > > OK...  I wonder if it should simply define stubs for kill_mtd_super(),
> > > mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
> > > cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
> > > returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
> > > in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
> > >   if (sb->s_mtd) {
> > >   if (sbi->mtd_point_size)
> > >   mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > >   kill_mtd_super(sb);
> > >   } else {
> > >   kill_block_super(sb);
> > >   }
> > >   kfree(sbi);
> > 
> > Well... Stubs have to be named differently or they conflict with 
> > existing declarations. At that point that makes for more lines of code 
> > compared to the current patch and the naming indirection makes it less 
> > obvious when reading the code. Alternatively I could add those stubs in 
> > the corresponding header files and #ifdef the existing declarations 
> > away. That might look somewhat less cluttered in the main code but it 
> > also hides what is actually going on and left me unconvinced. And I'm 
> > not sure this is worth it in the end given this is not a common 
> > occurrence in the kernel either.
> 
> What I mean is this (completely untested) for CONFIG_BLOCK side of things,
> with something similar for CONFIG_MTD one:
> 
> Provide definitions of mount_bdev/kill_block_super() in case !CONFIG_BLOCK

Yes, that's what I thought you meant, which corresponds to the second 
part of my comment above. And as I said I'm not convinced this hiding of 
kernel config effects is better for understanding what is actually going 
on locally, and my own preference is how things are right now.

But if you confirm you really want things that other way then I'll 
oblige and repost.


Nicolas

Re: [PATCH v5 01/12] mmc: dt-bindings: Add reg/source_cg/latch-ck for Mediatek MMC bindings

[Chaotian Jing]

On Fri, 2017-10-13 at 16:50 -0500, Rob Herring wrote:
> On Wed, Oct 11, 2017 at 10:41:25AM +0800, Chaotian Jing wrote:
> > Change the comptiable for support of multi-platform
> > Make compatible explicit
> > Add description for reg
> > Add description for source_cg
> > Add description for mediatek,latch-ck
> > Note that source_cg and mediatek,latch-ck are optional for some projects,
> > eg, MT2701 do not have source_cg, and MT2712 do not need
> > mediatek,latch-ck
> > 
> > Signed-off-by: Chaotian Jing 
> > ---
> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 16 +---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt 
> > b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > index 4182ea3..2bb585b 100644
> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > @@ -7,10 +7,18 @@ This file documents differences between the core 
> > properties in mmc.txt
> >  and the properties used by the msdc driver.
> >  
> >  Required properties:
> > -- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
> > +- compatible: value should be either of the following.
> > +   "mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
> > +   "mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
> > +   "mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
> > +   "mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
> > +- reg: physical base address of the controller and length
> >  - interrupts: Should contain MSDC interrupt number
> > -- clocks: MSDC source clock, HCLK
> > -- clock-names: "source", "hclk"
> > +- clocks: Should contain phandle for the clock feeding the MMC controller
> > +- clock-names: Should contain the following:
> > +   "source" - source clock (required)
> > +   "hclk" - HCLK which used for host (required)
> > +   "source_cg" - independent source clock gate (required for MT2712)
> >  - pinctrl-names: should be "default", "state_uhs"
> >  - pinctrl-0: should contain default/high speed pin ctrl
> >  - pinctrl-1: should contain uhs mode pin ctrl
> > @@ -30,6 +38,8 @@ Optional properties:
> >  - mediatek,hs400-cmd-resp-sel-rising:  HS400 command response sample 
> > selection
> >If present,HS400 command responses are 
> > sampled on rising edges.
> >If not present,HS400 command responses 
> > are sampled on falling edges.
> > +- mediatek,latch-ck: Some SoCs do not support enhance_rx, need set correct 
> > latch-ck to avoid data crc
> > +error caused by stop clock(fifo full)
> 
> What values are supported? What's the default if not present.
> 
> Be clear what compatible strings this property applies to or doesn't 
> apply to.
OK, will make it more clearly at next version.
Thx!
> 
> Rob

Re: [PATCH v5 01/12] mmc: dt-bindings: Add reg/source_cg/latch-ck for Mediatek MMC bindings

[Chaotian Jing]

On Fri, 2017-10-13 at 16:50 -0500, Rob Herring wrote:
> On Wed, Oct 11, 2017 at 10:41:25AM +0800, Chaotian Jing wrote:
> > Change the comptiable for support of multi-platform
> > Make compatible explicit
> > Add description for reg
> > Add description for source_cg
> > Add description for mediatek,latch-ck
> > Note that source_cg and mediatek,latch-ck are optional for some projects,
> > eg, MT2701 do not have source_cg, and MT2712 do not need
> > mediatek,latch-ck
> > 
> > Signed-off-by: Chaotian Jing 
> > ---
> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 16 +---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt 
> > b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > index 4182ea3..2bb585b 100644
> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > @@ -7,10 +7,18 @@ This file documents differences between the core 
> > properties in mmc.txt
> >  and the properties used by the msdc driver.
> >  
> >  Required properties:
> > -- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
> > +- compatible: value should be either of the following.
> > +   "mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
> > +   "mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
> > +   "mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
> > +   "mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
> > +- reg: physical base address of the controller and length
> >  - interrupts: Should contain MSDC interrupt number
> > -- clocks: MSDC source clock, HCLK
> > -- clock-names: "source", "hclk"
> > +- clocks: Should contain phandle for the clock feeding the MMC controller
> > +- clock-names: Should contain the following:
> > +   "source" - source clock (required)
> > +   "hclk" - HCLK which used for host (required)
> > +   "source_cg" - independent source clock gate (required for MT2712)
> >  - pinctrl-names: should be "default", "state_uhs"
> >  - pinctrl-0: should contain default/high speed pin ctrl
> >  - pinctrl-1: should contain uhs mode pin ctrl
> > @@ -30,6 +38,8 @@ Optional properties:
> >  - mediatek,hs400-cmd-resp-sel-rising:  HS400 command response sample 
> > selection
> >If present,HS400 command responses are 
> > sampled on rising edges.
> >If not present,HS400 command responses 
> > are sampled on falling edges.
> > +- mediatek,latch-ck: Some SoCs do not support enhance_rx, need set correct 
> > latch-ck to avoid data crc
> > +error caused by stop clock(fifo full)
> 
> What values are supported? What's the default if not present.
> 
> Be clear what compatible strings this property applies to or doesn't 
> apply to.
OK, will make it more clearly at next version.
Thx!
> 
> Rob

Re: [PATCH v2 03/13] mmc: dt-bindings: make compatible explicit

[Chaotian Jing]

On Fri, 2017-10-13 at 15:14 -0500, Rob Herring wrote:
> On Mon, Oct 09, 2017 at 07:35:16PM +0800, Chaotian Jing wrote:
> > the driver has updated to have an explicit compatible, so update
> > binding file according to the driver change.
> 
> Normally, that's not a reason for a binding change. An old kernel 
> without the driver change is still valid.
> 
> > 
> > Signed-off-by: Chaotian Jing 
> > ---
> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt 
> > b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > index 3e0a97c..2bb585b 100644
> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > @@ -8,8 +8,8 @@ and the properties used by the msdc driver.
> >  
> >  Required properties:
> >  - compatible: value should be either of the following.
> > -   "mediatek,mt8173-mmc","mediatek,mt8135-mmc": for mmc host ip compatible 
> > with mt8135
> 
> Except this one didn't really make sense. "mediatek,mt8135-mmc" should 
> be the most specific compatible on a MT8135.
> 
> > -   "mediatek,mt8173-mmc","mediatek,mt8135-mmc": for mmc host ip compatible 
> > with mt8173
> 
> Why is the version in MT8173 no longer compatible with a driver that 
> supports "mediatek,mt8135-mmc"? Your commit msg needs to answer that 
> question.

OK, will answer it in commit msg at next version.
> 
> > +   "mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
> > +   "mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
> > "mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
> > "mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
> >  - reg: physical base address of the controller and length
> > -- 
> > 1.8.1.1.dirty
> > 

Re: [PATCH v2 03/13] mmc: dt-bindings: make compatible explicit

[Chaotian Jing]

On Fri, 2017-10-13 at 15:14 -0500, Rob Herring wrote:
> On Mon, Oct 09, 2017 at 07:35:16PM +0800, Chaotian Jing wrote:
> > the driver has updated to have an explicit compatible, so update
> > binding file according to the driver change.
> 
> Normally, that's not a reason for a binding change. An old kernel 
> without the driver change is still valid.
> 
> > 
> > Signed-off-by: Chaotian Jing 
> > ---
> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt 
> > b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > index 3e0a97c..2bb585b 100644
> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > @@ -8,8 +8,8 @@ and the properties used by the msdc driver.
> >  
> >  Required properties:
> >  - compatible: value should be either of the following.
> > -   "mediatek,mt8173-mmc","mediatek,mt8135-mmc": for mmc host ip compatible 
> > with mt8135
> 
> Except this one didn't really make sense. "mediatek,mt8135-mmc" should 
> be the most specific compatible on a MT8135.
> 
> > -   "mediatek,mt8173-mmc","mediatek,mt8135-mmc": for mmc host ip compatible 
> > with mt8173
> 
> Why is the version in MT8173 no longer compatible with a driver that 
> supports "mediatek,mt8135-mmc"? Your commit msg needs to answer that 
> question.

OK, will answer it in commit msg at next version.
> 
> > +   "mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
> > +   "mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
> > "mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
> > "mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
> >  - reg: physical base address of the controller and length
> > -- 
> > 1.8.1.1.dirty
> > 

RE: [Intel-wired-lan] [PATCH][V3] e1000: avoid null pointer dereference on invalid stat type

[Brown, Aaron F]

> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Colin King
> Sent: Friday, September 22, 2017 10:14 AM
> To: Kirsher, Jeffrey T ; intel-wired-
> l...@lists.osuosl.org; net...@vger.kernel.org
> Cc: kernel-janit...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH][V3] e1000: avoid null pointer dereference
> on invalid stat type
> 
> From: Colin Ian King 
> 
> Currently if the stat type is invalid then data[i] is being set
> either by dereferencing a null pointer p, or it is reading from
> an incorrect previous location if we had a valid stat type
> previously.  Fix this by skipping over the read of p on an invalid
> stat type.
> 
> Detected by CoverityScan, CID#113385 ("Explicit null dereferenced")
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)

Tested-by: Aaron Brown 

RE: [Intel-wired-lan] [PATCH][V3] e1000: avoid null pointer dereference on invalid stat type

[Brown, Aaron F]

> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Colin King
> Sent: Friday, September 22, 2017 10:14 AM
> To: Kirsher, Jeffrey T ; intel-wired-
> l...@lists.osuosl.org; net...@vger.kernel.org
> Cc: kernel-janit...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH][V3] e1000: avoid null pointer dereference
> on invalid stat type
> 
> From: Colin Ian King 
> 
> Currently if the stat type is invalid then data[i] is being set
> either by dereferencing a null pointer p, or it is reading from
> an incorrect previous location if we had a valid stat type
> previously.  Fix this by skipping over the read of p on an invalid
> stat type.
> 
> Detected by CoverityScan, CID#113385 ("Explicit null dereferenced")
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)

Tested-by: Aaron Brown 

Re: [RFC PATCH v2 3/8] cpuidle: add a new predict interface

[Rafael J. Wysocki]

On Saturday, September 30, 2017 9:20:29 AM CEST Aubrey Li wrote:
> For the governor has predict functionality, add a new predict
> interface in cpuidle framework to call and use it.
> ---
>  drivers/cpuidle/cpuidle.c| 34 ++
>  drivers/cpuidle/governors/menu.c |  7 +++
>  include/linux/cpuidle.h  |  3 +++
>  kernel/sched/idle.c  |  1 +
>  4 files changed, 45 insertions(+)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 4066308..ef6f7dd 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -336,6 +336,40 @@ void cpuidle_entry_end(void)
>  }
>  
>  /**
> + * cpuidle_predict - predict whether the coming idle is a fast idle or not
> + */
> +void cpuidle_predict(void)
> +{
> + struct cpuidle_device *dev = cpuidle_get_device();
> + unsigned int overhead_threshold;
> +
> + if (!dev)
> + return;
> +
> + overhead_threshold = dev->idle_stat.overhead;
> +
> + if (cpuidle_curr_governor->predict) {
> + dev->idle_stat.predicted_us = cpuidle_curr_governor->predict();
> + /*
> +  * notify idle governor to avoid reduplicative
> +  * prediction computation
> +  */
> + dev->idle_stat.predicted = true;
> + if (dev->idle_stat.predicted_us < overhead_threshold) {
> + /*
> +  * notify tick subsystem to keep ticking
> +  * for the coming idle
> +  */
> + dev->idle_stat.fast_idle = true;
> + } else
> + dev->idle_stat.fast_idle = false;
> + } else {
> + dev->idle_stat.predicted = false;
> + dev->idle_stat.fast_idle = false;
> + }
> +}
> +
> +/**
>   * cpuidle_install_idle_handler - installs the cpuidle idle loop handler
>   */
>  void cpuidle_install_idle_handler(void)
> diff --git a/drivers/cpuidle/governors/menu.c 
> b/drivers/cpuidle/governors/menu.c
> index 6bed197..90b2a10 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -344,6 +344,12 @@ static int menu_select(struct cpuidle_driver *drv, 
> struct cpuidle_device *dev)
>   if (unlikely(latency_req == 0))
>   return 0;
>  
> + /*don't predict again if idle framework already did it */
> + if (!dev->idle_stat.predicted)
> + menu_predict();
> + else
> + dev->idle_stat.predicted = false;
> +
>   if (CPUIDLE_DRIVER_STATE_START > 0) {
>   struct cpuidle_state *s = 
> >states[CPUIDLE_DRIVER_STATE_START];
>   unsigned int polling_threshold;
> @@ -518,6 +524,7 @@ static struct cpuidle_governor menu_governor = {
>   .enable =   menu_enable_device,
>   .select =   menu_select,
>   .reflect =  menu_reflect,
> + .predict =  menu_predict,
>  };
>  
>  /**
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index cad9b71..9ca0288 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -143,6 +143,7 @@ extern int cpuidle_select(struct cpuidle_driver *drv,
> struct cpuidle_device *dev);
>  extern void cpuidle_entry_start(void);
>  extern void cpuidle_entry_end(void);
> +extern void cpuidle_predict(void);
>  extern int cpuidle_enter(struct cpuidle_driver *drv,
>struct cpuidle_device *dev, int index);
>  extern void cpuidle_reflect(struct cpuidle_device *dev, int index);
> @@ -178,6 +179,7 @@ static inline int cpuidle_select(struct cpuidle_driver 
> *drv,
>  {return -ENODEV; }
>  static inline void cpuidle_entry_start(void) { }
>  static inline void cpuidle_entry_end(void) { }
> +static inline void cpuidle_predict(void) { }
>  static inline int cpuidle_enter(struct cpuidle_driver *drv,
>   struct cpuidle_device *dev, int index)
>  {return -ENODEV; }
> @@ -255,6 +257,7 @@ struct cpuidle_governor {
>   int  (*select)  (struct cpuidle_driver *drv,
>   struct cpuidle_device *dev);
>   void (*reflect) (struct cpuidle_device *dev, int index);
> + unsigned int (*predict)(void);
>  };
>  
>  #ifdef CONFIG_CPU_IDLE
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index 0951dac..8704f3c 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -225,6 +225,7 @@ static void do_idle(void)
>*/
>   __current_set_polling();
>   quiet_vmstat();
> + cpuidle_predict();

One more question here.

This changes the cpuidle code ordering such that if the ->predict callback
is present, the governor prediction will run before tick_nohz_idle_enter(),
whereas without that callback it runs in cpuidle_idle_call().

Is that actually going to work correctly for the menu governor, in particular?

>   tick_nohz_idle_enter();
>   cpuidle_entry_end();
>  
> 

Re: [RFC PATCH v2 3/8] cpuidle: add a new predict interface

[Rafael J. Wysocki]

On Saturday, September 30, 2017 9:20:29 AM CEST Aubrey Li wrote:
> For the governor has predict functionality, add a new predict
> interface in cpuidle framework to call and use it.
> ---
>  drivers/cpuidle/cpuidle.c| 34 ++
>  drivers/cpuidle/governors/menu.c |  7 +++
>  include/linux/cpuidle.h  |  3 +++
>  kernel/sched/idle.c  |  1 +
>  4 files changed, 45 insertions(+)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 4066308..ef6f7dd 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -336,6 +336,40 @@ void cpuidle_entry_end(void)
>  }
>  
>  /**
> + * cpuidle_predict - predict whether the coming idle is a fast idle or not
> + */
> +void cpuidle_predict(void)
> +{
> + struct cpuidle_device *dev = cpuidle_get_device();
> + unsigned int overhead_threshold;
> +
> + if (!dev)
> + return;
> +
> + overhead_threshold = dev->idle_stat.overhead;
> +
> + if (cpuidle_curr_governor->predict) {
> + dev->idle_stat.predicted_us = cpuidle_curr_governor->predict();
> + /*
> +  * notify idle governor to avoid reduplicative
> +  * prediction computation
> +  */
> + dev->idle_stat.predicted = true;
> + if (dev->idle_stat.predicted_us < overhead_threshold) {
> + /*
> +  * notify tick subsystem to keep ticking
> +  * for the coming idle
> +  */
> + dev->idle_stat.fast_idle = true;
> + } else
> + dev->idle_stat.fast_idle = false;
> + } else {
> + dev->idle_stat.predicted = false;
> + dev->idle_stat.fast_idle = false;
> + }
> +}
> +
> +/**
>   * cpuidle_install_idle_handler - installs the cpuidle idle loop handler
>   */
>  void cpuidle_install_idle_handler(void)
> diff --git a/drivers/cpuidle/governors/menu.c 
> b/drivers/cpuidle/governors/menu.c
> index 6bed197..90b2a10 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -344,6 +344,12 @@ static int menu_select(struct cpuidle_driver *drv, 
> struct cpuidle_device *dev)
>   if (unlikely(latency_req == 0))
>   return 0;
>  
> + /*don't predict again if idle framework already did it */
> + if (!dev->idle_stat.predicted)
> + menu_predict();
> + else
> + dev->idle_stat.predicted = false;
> +
>   if (CPUIDLE_DRIVER_STATE_START > 0) {
>   struct cpuidle_state *s = 
> >states[CPUIDLE_DRIVER_STATE_START];
>   unsigned int polling_threshold;
> @@ -518,6 +524,7 @@ static struct cpuidle_governor menu_governor = {
>   .enable =   menu_enable_device,
>   .select =   menu_select,
>   .reflect =  menu_reflect,
> + .predict =  menu_predict,
>  };
>  
>  /**
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index cad9b71..9ca0288 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -143,6 +143,7 @@ extern int cpuidle_select(struct cpuidle_driver *drv,
> struct cpuidle_device *dev);
>  extern void cpuidle_entry_start(void);
>  extern void cpuidle_entry_end(void);
> +extern void cpuidle_predict(void);
>  extern int cpuidle_enter(struct cpuidle_driver *drv,
>struct cpuidle_device *dev, int index);
>  extern void cpuidle_reflect(struct cpuidle_device *dev, int index);
> @@ -178,6 +179,7 @@ static inline int cpuidle_select(struct cpuidle_driver 
> *drv,
>  {return -ENODEV; }
>  static inline void cpuidle_entry_start(void) { }
>  static inline void cpuidle_entry_end(void) { }
> +static inline void cpuidle_predict(void) { }
>  static inline int cpuidle_enter(struct cpuidle_driver *drv,
>   struct cpuidle_device *dev, int index)
>  {return -ENODEV; }
> @@ -255,6 +257,7 @@ struct cpuidle_governor {
>   int  (*select)  (struct cpuidle_driver *drv,
>   struct cpuidle_device *dev);
>   void (*reflect) (struct cpuidle_device *dev, int index);
> + unsigned int (*predict)(void);
>  };
>  
>  #ifdef CONFIG_CPU_IDLE
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index 0951dac..8704f3c 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -225,6 +225,7 @@ static void do_idle(void)
>*/
>   __current_set_polling();
>   quiet_vmstat();
> + cpuidle_predict();

One more question here.

This changes the cpuidle code ordering such that if the ->predict callback
is present, the governor prediction will run before tick_nohz_idle_enter(),
whereas without that callback it runs in cpuidle_idle_call().

Is that actually going to work correctly for the menu governor, in particular?

>   tick_nohz_idle_enter();
>   cpuidle_entry_end();
>  
> 

Re: [PATCH 08/18] ecryptfs: use ARRAY_SIZE

[Tyler Hicks]

On 10/01/2017 03:30 PM, Jérémy Lefaure wrote:
> Using the ARRAY_SIZE macro improves the readability of the code.
> 
> Found with Coccinelle with the following semantic patch:
> @r depends on (org || report)@
> type T;
> T[] E;
> position p;
> @@
> (
>  (sizeof(E)@p /sizeof(*E))
> |
>  (sizeof(E)@p /sizeof(E[...]))
> |
>  (sizeof(E)@p /sizeof(T))
> )
> 
> Signed-off-by: Jérémy Lefaure 

Hey Jérémy - Thanks for the nice cleanup. I'll get it into 4.15.

Tyler

> ---
>  fs/ecryptfs/crypto.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index 7acd57da4f14..2e78e851788e 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "ecryptfs_kernel.h"
>  
>  #define DECRYPT  0
> @@ -884,8 +885,7 @@ static int ecryptfs_process_flags(struct 
> ecryptfs_crypt_stat *crypt_stat,
>   u32 flags;
>  
>   flags = get_unaligned_be32(page_virt);
> - for (i = 0; i < ((sizeof(ecryptfs_flag_map)
> -   / sizeof(struct ecryptfs_flag_map_elem))); i++)
> + for (i = 0; i < ARRAY_SIZE(ecryptfs_flag_map); i++)
>   if (flags & ecryptfs_flag_map[i].file_flag) {
>   crypt_stat->flags |= ecryptfs_flag_map[i].local_flag;
>   } else
> @@ -922,8 +922,7 @@ void ecryptfs_write_crypt_stat_flags(char *page_virt,
>   u32 flags = 0;
>   int i;
>  
> - for (i = 0; i < ((sizeof(ecryptfs_flag_map)
> -   / sizeof(struct ecryptfs_flag_map_elem))); i++)
> + for (i = 0; i < ARRAY_SIZE(ecryptfs_flag_map); i++)
>   if (crypt_stat->flags & ecryptfs_flag_map[i].local_flag)
>   flags |= ecryptfs_flag_map[i].file_flag;
>   /* Version is in top 8 bits of the 32-bit flag vector */
> 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH 08/18] ecryptfs: use ARRAY_SIZE

[Tyler Hicks]

On 10/01/2017 03:30 PM, Jérémy Lefaure wrote:
> Using the ARRAY_SIZE macro improves the readability of the code.
> 
> Found with Coccinelle with the following semantic patch:
> @r depends on (org || report)@
> type T;
> T[] E;
> position p;
> @@
> (
>  (sizeof(E)@p /sizeof(*E))
> |
>  (sizeof(E)@p /sizeof(E[...]))
> |
>  (sizeof(E)@p /sizeof(T))
> )
> 
> Signed-off-by: Jérémy Lefaure 

Hey Jérémy - Thanks for the nice cleanup. I'll get it into 4.15.

Tyler

> ---
>  fs/ecryptfs/crypto.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index 7acd57da4f14..2e78e851788e 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "ecryptfs_kernel.h"
>  
>  #define DECRYPT  0
> @@ -884,8 +885,7 @@ static int ecryptfs_process_flags(struct 
> ecryptfs_crypt_stat *crypt_stat,
>   u32 flags;
>  
>   flags = get_unaligned_be32(page_virt);
> - for (i = 0; i < ((sizeof(ecryptfs_flag_map)
> -   / sizeof(struct ecryptfs_flag_map_elem))); i++)
> + for (i = 0; i < ARRAY_SIZE(ecryptfs_flag_map); i++)
>   if (flags & ecryptfs_flag_map[i].file_flag) {
>   crypt_stat->flags |= ecryptfs_flag_map[i].local_flag;
>   } else
> @@ -922,8 +922,7 @@ void ecryptfs_write_crypt_stat_flags(char *page_virt,
>   u32 flags = 0;
>   int i;
>  
> - for (i = 0; i < ((sizeof(ecryptfs_flag_map)
> -   / sizeof(struct ecryptfs_flag_map_elem))); i++)
> + for (i = 0; i < ARRAY_SIZE(ecryptfs_flag_map); i++)
>   if (crypt_stat->flags & ecryptfs_flag_map[i].local_flag)
>   flags |= ecryptfs_flag_map[i].file_flag;
>   /* Version is in top 8 bits of the 32-bit flag vector */
> 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH 0/3] eCryptfs: Adjustments for several function implementations

[Tyler Hicks]

On 08/19/2017 12:54 PM, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Sat, 19 Aug 2017 18:15:14 +0200
> 
> Some update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (3):
>   Delete 21 error messages for a failed memory allocation
>   Return an error code only as a constant in ecryptfs_add_global_auth_tok()
>   Adjust four checks for null pointers

Hello! I'm sorry for just now getting to these patches. All three look
fine to me. I'll get them into 4.15.

Tyler

> 
>  fs/ecryptfs/crypto.c| 22 +++---
>  fs/ecryptfs/inode.c |  3 ---
>  fs/ecryptfs/keystore.c  | 46 +++---
>  fs/ecryptfs/messaging.c |  6 --
>  fs/ecryptfs/miscdev.c   |  6 +-
>  fs/ecryptfs/mmap.c  |  2 --
>  6 files changed, 15 insertions(+), 70 deletions(-)
> 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH 0/3] eCryptfs: Adjustments for several function implementations

[Tyler Hicks]

On 08/19/2017 12:54 PM, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Sat, 19 Aug 2017 18:15:14 +0200
> 
> Some update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (3):
>   Delete 21 error messages for a failed memory allocation
>   Return an error code only as a constant in ecryptfs_add_global_auth_tok()
>   Adjust four checks for null pointers

Hello! I'm sorry for just now getting to these patches. All three look
fine to me. I'll get them into 4.15.

Tyler

> 
>  fs/ecryptfs/crypto.c| 22 +++---
>  fs/ecryptfs/inode.c |  3 ---
>  fs/ecryptfs/keystore.c  | 46 +++---
>  fs/ecryptfs/messaging.c |  6 --
>  fs/ecryptfs/miscdev.c   |  6 +-
>  fs/ecryptfs/mmap.c  |  2 --
>  6 files changed, 15 insertions(+), 70 deletions(-)
> 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH] eCryptfs: constify attribute_group structures.

[Tyler Hicks]

Hi Arvind - My apologies for the extremely slow review.

On 06/30/2017 05:33 AM, Arvind Yadav wrote:
> attribute_groups are not supposed to change at runtime. All functions
> working with attribute_groups provided by  work with const
> attribute_group. So mark the non-const structs as const.
> 
> File size before:
>text  data bss dec hex filename
>6122   636  2467821a7e fs/ecryptfs/main.o
> 
> File size After adding 'const':
>text  data bss dec hex filename
>6186   604  2468141a9e fs/ecryptfs/main.o
> 
> Signed-off-by: Arvind Yadav 

This patch looks good. I'll get it queued up for 4.15.

Tyler

> ---
>  fs/ecryptfs/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
> index 9014479..eea4c0f 100644
> --- a/fs/ecryptfs/main.c
> +++ b/fs/ecryptfs/main.c
> @@ -782,7 +782,7 @@ static ssize_t version_show(struct kobject *kobj,
>   NULL,
>  };
>  
> -static struct attribute_group attr_group = {
> +static const struct attribute_group attr_group = {
>   .attrs = attributes,
>  };
>  
> 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH] eCryptfs: constify attribute_group structures.

[Tyler Hicks]

Hi Arvind - My apologies for the extremely slow review.

On 06/30/2017 05:33 AM, Arvind Yadav wrote:
> attribute_groups are not supposed to change at runtime. All functions
> working with attribute_groups provided by  work with const
> attribute_group. So mark the non-const structs as const.
> 
> File size before:
>text  data bss dec hex filename
>6122   636  2467821a7e fs/ecryptfs/main.o
> 
> File size After adding 'const':
>text  data bss dec hex filename
>6186   604  2468141a9e fs/ecryptfs/main.o
> 
> Signed-off-by: Arvind Yadav 

This patch looks good. I'll get it queued up for 4.15.

Tyler

> ---
>  fs/ecryptfs/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
> index 9014479..eea4c0f 100644
> --- a/fs/ecryptfs/main.c
> +++ b/fs/ecryptfs/main.c
> @@ -782,7 +782,7 @@ static ssize_t version_show(struct kobject *kobj,
>   NULL,
>  };
>  
> -static struct attribute_group attr_group = {
> +static const struct attribute_group attr_group = {
>   .attrs = attributes,
>  };
>  
> 




signature.asc
Description: OpenPGP digital signature

Re: [RFC PATCH v2 0/8] Introduct cpu idle prediction functionality

[Rafael J. Wysocki]

On Saturday, September 30, 2017 9:20:26 AM CEST Aubrey Li wrote:
> We found under some latency intensive workloads, short idle periods occurs
> very common, then idle entry and exit path starts to dominate, so it's
> important to optimize them. To determine the short idle pattern, we need
> to figure out how long of the coming idle and the threshold of the short
> idle interval.
> 
> A cpu idle prediction functionality is introduced in this proposal to catch
> the short idle pattern.
> 
> Firstly, we check the IRQ timings subsystem, if there is an event
> coming soon.
> -- https://lwn.net/Articles/691297/
> 
> Secondly, we check the idle statistics of scheduler, if it's likely we'll
> go into a short idle.
> -- https://patchwork.kernel.org/patch/2839221/
> 
> Thirdly, we predict the next idle interval by using the prediction
> fucntionality in the idle governor if it has.
> 
> For the threshold of the short idle interval, we record the timestamps of
> the idle entry, and multiply by a tunable parameter at here:
> -- /proc/sys/kernel/fast_idle_ratio
> 
> We use the output of the idle prediction to skip turning tick off if a
> short idle is determined in this proposal. Reprogramming hardware timer
> twice(off and on) is expensive for a very short idle. There are some
> potential optimizations can be done according to the same indicator.
> 
> I observed when system is idle, the idle predictor reports 20/s long idle
> and ZERO fast idle on one CPU. And when the workload is running, the idle
> predictor reports 72899/s fast idle and ZERO long idle on the same CPU.
> 
> Aubrey Li (8):
>   cpuidle: menu: extract prediction functionality
>   cpuidle: record the overhead of idle entry
>   cpuidle: add a new predict interface
>   tick/nohz: keep tick on for a fast idle
>   timers: keep sleep length updated as needed
>   cpuidle: make fast idle threshold tunable
>   cpuidle: introduce irq timing to make idle prediction
>   cpuidle: introduce run queue average idle to make idle prediction
> 
>  drivers/cpuidle/Kconfig  |   1 +
>  drivers/cpuidle/cpuidle.c| 109 
> +++
>  drivers/cpuidle/governors/menu.c |  69 -
>  include/linux/cpuidle.h  |  21 
>  kernel/sched/idle.c  |  14 -
>  kernel/sysctl.c  |  12 +
>  kernel/time/tick-sched.c |   7 +++
>  7 files changed, 209 insertions(+), 24 deletions(-)
> 

Overall, it looks like you could avoid stopping the tick every time the
predicted idle duration is not longer than the tick interval in the first
place.

Why don't you do that?

Re: [RFC PATCH v2 0/8] Introduct cpu idle prediction functionality

[Rafael J. Wysocki]

On Saturday, September 30, 2017 9:20:26 AM CEST Aubrey Li wrote:
> We found under some latency intensive workloads, short idle periods occurs
> very common, then idle entry and exit path starts to dominate, so it's
> important to optimize them. To determine the short idle pattern, we need
> to figure out how long of the coming idle and the threshold of the short
> idle interval.
> 
> A cpu idle prediction functionality is introduced in this proposal to catch
> the short idle pattern.
> 
> Firstly, we check the IRQ timings subsystem, if there is an event
> coming soon.
> -- https://lwn.net/Articles/691297/
> 
> Secondly, we check the idle statistics of scheduler, if it's likely we'll
> go into a short idle.
> -- https://patchwork.kernel.org/patch/2839221/
> 
> Thirdly, we predict the next idle interval by using the prediction
> fucntionality in the idle governor if it has.
> 
> For the threshold of the short idle interval, we record the timestamps of
> the idle entry, and multiply by a tunable parameter at here:
> -- /proc/sys/kernel/fast_idle_ratio
> 
> We use the output of the idle prediction to skip turning tick off if a
> short idle is determined in this proposal. Reprogramming hardware timer
> twice(off and on) is expensive for a very short idle. There are some
> potential optimizations can be done according to the same indicator.
> 
> I observed when system is idle, the idle predictor reports 20/s long idle
> and ZERO fast idle on one CPU. And when the workload is running, the idle
> predictor reports 72899/s fast idle and ZERO long idle on the same CPU.
> 
> Aubrey Li (8):
>   cpuidle: menu: extract prediction functionality
>   cpuidle: record the overhead of idle entry
>   cpuidle: add a new predict interface
>   tick/nohz: keep tick on for a fast idle
>   timers: keep sleep length updated as needed
>   cpuidle: make fast idle threshold tunable
>   cpuidle: introduce irq timing to make idle prediction
>   cpuidle: introduce run queue average idle to make idle prediction
> 
>  drivers/cpuidle/Kconfig  |   1 +
>  drivers/cpuidle/cpuidle.c| 109 
> +++
>  drivers/cpuidle/governors/menu.c |  69 -
>  include/linux/cpuidle.h  |  21 
>  kernel/sched/idle.c  |  14 -
>  kernel/sysctl.c  |  12 +
>  kernel/time/tick-sched.c |   7 +++
>  7 files changed, 209 insertions(+), 24 deletions(-)
> 

Overall, it looks like you could avoid stopping the tick every time the
predicted idle duration is not longer than the tick interval in the first
place.

Why don't you do that?

Re: [RFC PATCH v2 8/8] cpuidle: introduce run queue average idle to make idle prediction

[Rafael J. Wysocki]

On Saturday, September 30, 2017 9:20:34 AM CEST Aubrey Li wrote:
> Introduce run queue average idle in scheduler as a factor to make
> idle prediction
> 
> Signed-off-by: Aubrey Li 
> ---
>  drivers/cpuidle/cpuidle.c | 12 
>  include/linux/cpuidle.h   |  1 +
>  kernel/sched/idle.c   |  5 +
>  3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index be56cea..9424a2d 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -364,6 +364,18 @@ void cpuidle_predict(void)
>   return;
>   }
>  
> + /*
> +  * check scheduler if the coming idle is likely a fast idle
> +  */
> + idle_interval = div_u64(sched_idle_avg(), NSEC_PER_USEC);

And one more division ...

> + if (idle_interval < overhead_threshold) {
> + dev->idle_stat.fast_idle = true;
> + return;
> + }
> +
> + /*
> +  * check the idle governor if the coming idle is likely a fast idle
> +  */
>   if (cpuidle_curr_governor->predict) {
>   dev->idle_stat.predicted_us = cpuidle_curr_governor->predict();
>   /*
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 45b8264..387d72b 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -234,6 +234,7 @@ static inline void cpuidle_use_deepest_state(bool enable)
>  /* kernel/sched/idle.c */
>  extern void sched_idle_set_state(struct cpuidle_state *idle_state);
>  extern void default_idle_call(void);
> +extern u64 sched_idle_avg(void);
>  
>  #ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
>  void cpuidle_coupled_parallel_barrier(struct cpuidle_device *dev, atomic_t 
> *a);
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index 8704f3c..d23b472 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -30,6 +30,11 @@ void sched_idle_set_state(struct cpuidle_state *idle_state)
>   idle_set_state(this_rq(), idle_state);
>  }
>  
> +u64 sched_idle_avg(void)
> +{
> + return this_rq()->avg_idle;
> +}
> +
>  static int __read_mostly cpu_idle_force_poll;
>  
>  void cpu_idle_poll_ctrl(bool enable)
> 

You could easily combine this patch with the previous one IMO.

Re: [RFC PATCH v2 8/8] cpuidle: introduce run queue average idle to make idle prediction

[Rafael J. Wysocki]

On Saturday, September 30, 2017 9:20:34 AM CEST Aubrey Li wrote:
> Introduce run queue average idle in scheduler as a factor to make
> idle prediction
> 
> Signed-off-by: Aubrey Li 
> ---
>  drivers/cpuidle/cpuidle.c | 12 
>  include/linux/cpuidle.h   |  1 +
>  kernel/sched/idle.c   |  5 +
>  3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index be56cea..9424a2d 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -364,6 +364,18 @@ void cpuidle_predict(void)
>   return;
>   }
>  
> + /*
> +  * check scheduler if the coming idle is likely a fast idle
> +  */
> + idle_interval = div_u64(sched_idle_avg(), NSEC_PER_USEC);

And one more division ...

> + if (idle_interval < overhead_threshold) {
> + dev->idle_stat.fast_idle = true;
> + return;
> + }
> +
> + /*
> +  * check the idle governor if the coming idle is likely a fast idle
> +  */
>   if (cpuidle_curr_governor->predict) {
>   dev->idle_stat.predicted_us = cpuidle_curr_governor->predict();
>   /*
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 45b8264..387d72b 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -234,6 +234,7 @@ static inline void cpuidle_use_deepest_state(bool enable)
>  /* kernel/sched/idle.c */
>  extern void sched_idle_set_state(struct cpuidle_state *idle_state);
>  extern void default_idle_call(void);
> +extern u64 sched_idle_avg(void);
>  
>  #ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
>  void cpuidle_coupled_parallel_barrier(struct cpuidle_device *dev, atomic_t 
> *a);
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index 8704f3c..d23b472 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -30,6 +30,11 @@ void sched_idle_set_state(struct cpuidle_state *idle_state)
>   idle_set_state(this_rq(), idle_state);
>  }
>  
> +u64 sched_idle_avg(void)
> +{
> + return this_rq()->avg_idle;
> +}
> +
>  static int __read_mostly cpu_idle_force_poll;
>  
>  void cpu_idle_poll_ctrl(bool enable)
> 

You could easily combine this patch with the previous one IMO.

Re: [RFC PATCH v2 7/8] cpuidle: introduce irq timing to make idle prediction

[Rafael J. Wysocki]

On Saturday, September 30, 2017 9:20:33 AM CEST Aubrey Li wrote:
> Introduce irq timings output as a factor to predict the duration
> of the coming idle
> 
> Signed-off-by: Aubrey Li 
> ---
>  drivers/cpuidle/Kconfig   |  1 +
>  drivers/cpuidle/cpuidle.c | 17 -
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index 7e48eb5..8b07e1c 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig
> @@ -5,6 +5,7 @@ config CPU_IDLE
>   default y if ACPI || PPC_PSERIES
>   select CPU_IDLE_GOV_LADDER if (!NO_HZ && !NO_HZ_IDLE)
>   select CPU_IDLE_GOV_MENU if (NO_HZ || NO_HZ_IDLE)
> + select IRQ_TIMINGS
>   help
> CPU idle is a generic framework for supporting software-controlled
> idle processor power management.  It includes modular cross-platform
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 5d4f0b6..be56cea 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "cpuidle.h"
> @@ -342,13 +343,27 @@ void cpuidle_entry_end(void)
>  void cpuidle_predict(void)
>  {
>   struct cpuidle_device *dev = cpuidle_get_device();
> - unsigned int overhead_threshold;
> + unsigned int idle_interval, overhead_threshold;
> + u64 now, next_evt;
>  
>   if (!dev)
>   return;
>  
>   overhead_threshold = dev->idle_stat.overhead * sysctl_fast_idle_ratio;
>  
> + /*
> +  * check irq timings if the next event is coming soon
> +  */
> + now = local_clock();
> + local_irq_disable();
> + next_evt = irq_timings_next_event(now);
> + local_irq_enable();
> + idle_interval = div_u64(next_evt - now, NSEC_PER_USEC);

Another division ...

> + if (idle_interval < overhead_threshold) {
> + dev->idle_stat.fast_idle = true;
> + return;
> + }
> +
>   if (cpuidle_curr_governor->predict) {
>   dev->idle_stat.predicted_us = cpuidle_curr_governor->predict();
>   /*
> 

Re: [RFC PATCH v2 7/8] cpuidle: introduce irq timing to make idle prediction

[Rafael J. Wysocki]

On Saturday, September 30, 2017 9:20:33 AM CEST Aubrey Li wrote:
> Introduce irq timings output as a factor to predict the duration
> of the coming idle
> 
> Signed-off-by: Aubrey Li 
> ---
>  drivers/cpuidle/Kconfig   |  1 +
>  drivers/cpuidle/cpuidle.c | 17 -
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index 7e48eb5..8b07e1c 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig
> @@ -5,6 +5,7 @@ config CPU_IDLE
>   default y if ACPI || PPC_PSERIES
>   select CPU_IDLE_GOV_LADDER if (!NO_HZ && !NO_HZ_IDLE)
>   select CPU_IDLE_GOV_MENU if (NO_HZ || NO_HZ_IDLE)
> + select IRQ_TIMINGS
>   help
> CPU idle is a generic framework for supporting software-controlled
> idle processor power management.  It includes modular cross-platform
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 5d4f0b6..be56cea 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "cpuidle.h"
> @@ -342,13 +343,27 @@ void cpuidle_entry_end(void)
>  void cpuidle_predict(void)
>  {
>   struct cpuidle_device *dev = cpuidle_get_device();
> - unsigned int overhead_threshold;
> + unsigned int idle_interval, overhead_threshold;
> + u64 now, next_evt;
>  
>   if (!dev)
>   return;
>  
>   overhead_threshold = dev->idle_stat.overhead * sysctl_fast_idle_ratio;
>  
> + /*
> +  * check irq timings if the next event is coming soon
> +  */
> + now = local_clock();
> + local_irq_disable();
> + next_evt = irq_timings_next_event(now);
> + local_irq_enable();
> + idle_interval = div_u64(next_evt - now, NSEC_PER_USEC);

Another division ...

> + if (idle_interval < overhead_threshold) {
> + dev->idle_stat.fast_idle = true;
> + return;
> + }
> +
>   if (cpuidle_curr_governor->predict) {
>   dev->idle_stat.predicted_us = cpuidle_curr_governor->predict();
>   /*
>