[PATCH 2/2] pinctrl: sprd: Add Spreadtrum pin control driver
This patch adds the pin control driver for Spreadtrum SC9860 platform. Signed-off-by: Baolin Wang--- drivers/pinctrl/Kconfig|1 + drivers/pinctrl/Makefile |1 + drivers/pinctrl/sprd/Kconfig | 17 + drivers/pinctrl/sprd/Makefile |2 + drivers/pinctrl/sprd/pinctrl-sprd-sc9860.c | 972 drivers/pinctrl/sprd/pinctrl-sprd.c| 716 drivers/pinctrl/sprd/pinctrl-sprd.h| 67 ++ 7 files changed, 1776 insertions(+) create mode 100644 drivers/pinctrl/sprd/Kconfig create mode 100644 drivers/pinctrl/sprd/Makefile create mode 100644 drivers/pinctrl/sprd/pinctrl-sprd-sc9860.c create mode 100644 drivers/pinctrl/sprd/pinctrl-sprd.c create mode 100644 drivers/pinctrl/sprd/pinctrl-sprd.h diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 8f8c2af..681e593 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -304,6 +304,7 @@ source "drivers/pinctrl/ti/Kconfig" source "drivers/pinctrl/uniphier/Kconfig" source "drivers/pinctrl/vt8500/Kconfig" source "drivers/pinctrl/mediatek/Kconfig" +source "drivers/pinctrl/sprd/Kconfig" config PINCTRL_XWAY bool diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index a251f43..006e352 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -57,3 +57,4 @@ obj-y += ti/ obj-$(CONFIG_PINCTRL_UNIPHIER) += uniphier/ obj-$(CONFIG_ARCH_VT8500) += vt8500/ obj-$(CONFIG_PINCTRL_MTK) += mediatek/ +obj-y += sprd/ diff --git a/drivers/pinctrl/sprd/Kconfig b/drivers/pinctrl/sprd/Kconfig new file mode 100644 index 000..6f4a7f9 --- /dev/null +++ b/drivers/pinctrl/sprd/Kconfig @@ -0,0 +1,17 @@ +# +# Spreadtrum pin control drivers +# + +config PINCTRL_SPRD + bool "Spreadtrum pinctrl driver" + select PINMUX + select PINCONF + select GENERIC_PINCONF + select GENERIC_PINMUX_FUNCTIONS + help + Say Y here to enable Spreadtrum pinctrl driver + +config PINCTRL_SPRD_SC9860 + bool "Spreadtrum SC9860 pinctrl driver" + help + Say Y here to enable Spreadtrum SC9860 pinctrl driver diff --git a/drivers/pinctrl/sprd/Makefile b/drivers/pinctrl/sprd/Makefile new file mode 100644 index 000..b6caa8c --- /dev/null +++ b/drivers/pinctrl/sprd/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_PINCTRL_SPRD) += pinctrl-sprd.o +obj-$(CONFIG_PINCTRL_SPRD_SC9860) += pinctrl-sprd-sc9860.o diff --git a/drivers/pinctrl/sprd/pinctrl-sprd-sc9860.c b/drivers/pinctrl/sprd/pinctrl-sprd-sc9860.c new file mode 100644 index 000..726780d --- /dev/null +++ b/drivers/pinctrl/sprd/pinctrl-sprd-sc9860.c @@ -0,0 +1,972 @@ +/* + * Spreadtrum pin controller driver + * Copyright (C) 2017 Spreadtrum - http://www.spreadtrum.com + * + * 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. + */ + +#include +#include + +#include "pinctrl-sprd.h" + +enum sprd_sc9860_pins { + /* pin global control register 0 */ + SC9860_VIO28_0_IRTE = SPRD_PIN_INFO(0, GLOBAL_CTRL_PIN, 11, 1, 0), + SC9860_VIO_SD2_IRTE = SPRD_PIN_INFO(1, GLOBAL_CTRL_PIN, 10, 1, 0), + SC9860_VIO_SD0_IRTE = SPRD_PIN_INFO(2, GLOBAL_CTRL_PIN, 9, 1, 0), + SC9860_VIO_SIM2_IRTE = SPRD_PIN_INFO(3, GLOBAL_CTRL_PIN, 8, 1, 0), + SC9860_VIO_SIM1_IRTE = SPRD_PIN_INFO(4, GLOBAL_CTRL_PIN, 7, 1, 0), + SC9860_VIO_SIM0_IRTE = SPRD_PIN_INFO(5, GLOBAL_CTRL_PIN, 6, 1, 0), + SC9860_VIO28_0_MS = SPRD_PIN_INFO(6, GLOBAL_CTRL_PIN, 5, 1, 0), + SC9860_VIO_SD2_MS = SPRD_PIN_INFO(7, GLOBAL_CTRL_PIN, 4, 1, 0), + SC9860_VIO_SD0_MS = SPRD_PIN_INFO(8, GLOBAL_CTRL_PIN, 3, 1, 0), + SC9860_VIO_SIM2_MS = SPRD_PIN_INFO(9, GLOBAL_CTRL_PIN, 2, 1, 0), + SC9860_VIO_SIM1_MS = SPRD_PIN_INFO(10, GLOBAL_CTRL_PIN, 1, 1, 0), + SC9860_VIO_SIM0_MS = SPRD_PIN_INFO(11, GLOBAL_CTRL_PIN, 0, 1, 0), + + /* pin global control register 2 */ + SC9860_SPSPI_PIN_IN_SEL = SPRD_PIN_INFO(12, GLOBAL_CTRL_PIN, 31, 1, 2), + SC9860_UART1_USB30_PHY_SEL = SPRD_PIN_INFO(13, GLOBAL_CTRL_PIN, 30, 1, 2), + SC9860_USB30_PHY_DM_OE = SPRD_PIN_INFO(14, GLOBAL_CTRL_PIN, 29, 1, 2), + SC9860_USB30_PHY_DP_OE = SPRD_PIN_INFO(15, GLOBAL_CTRL_PIN, 28, 1, 2), + SC9860_UART5_SYS_SEL = SPRD_PIN_INFO(16, GLOBAL_CTRL_PIN, 25, 3, 2), + SC9860_ORP_URXD_PIN_IN_SEL = SPRD_PIN_INFO(17, GLOBAL_CTRL_PIN, 24, 1, 2), + SC9860_SIM2_SYS_SEL = SPRD_PIN_INFO(18, GLOBAL_CTRL_PIN, 23, 1, 2),
[PATCH 2/2] pinctrl: sprd: Add Spreadtrum pin control driver
This patch adds the pin control driver for Spreadtrum SC9860 platform. Signed-off-by: Baolin Wang --- drivers/pinctrl/Kconfig|1 + drivers/pinctrl/Makefile |1 + drivers/pinctrl/sprd/Kconfig | 17 + drivers/pinctrl/sprd/Makefile |2 + drivers/pinctrl/sprd/pinctrl-sprd-sc9860.c | 972 drivers/pinctrl/sprd/pinctrl-sprd.c| 716 drivers/pinctrl/sprd/pinctrl-sprd.h| 67 ++ 7 files changed, 1776 insertions(+) create mode 100644 drivers/pinctrl/sprd/Kconfig create mode 100644 drivers/pinctrl/sprd/Makefile create mode 100644 drivers/pinctrl/sprd/pinctrl-sprd-sc9860.c create mode 100644 drivers/pinctrl/sprd/pinctrl-sprd.c create mode 100644 drivers/pinctrl/sprd/pinctrl-sprd.h diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 8f8c2af..681e593 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -304,6 +304,7 @@ source "drivers/pinctrl/ti/Kconfig" source "drivers/pinctrl/uniphier/Kconfig" source "drivers/pinctrl/vt8500/Kconfig" source "drivers/pinctrl/mediatek/Kconfig" +source "drivers/pinctrl/sprd/Kconfig" config PINCTRL_XWAY bool diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index a251f43..006e352 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -57,3 +57,4 @@ obj-y += ti/ obj-$(CONFIG_PINCTRL_UNIPHIER) += uniphier/ obj-$(CONFIG_ARCH_VT8500) += vt8500/ obj-$(CONFIG_PINCTRL_MTK) += mediatek/ +obj-y += sprd/ diff --git a/drivers/pinctrl/sprd/Kconfig b/drivers/pinctrl/sprd/Kconfig new file mode 100644 index 000..6f4a7f9 --- /dev/null +++ b/drivers/pinctrl/sprd/Kconfig @@ -0,0 +1,17 @@ +# +# Spreadtrum pin control drivers +# + +config PINCTRL_SPRD + bool "Spreadtrum pinctrl driver" + select PINMUX + select PINCONF + select GENERIC_PINCONF + select GENERIC_PINMUX_FUNCTIONS + help + Say Y here to enable Spreadtrum pinctrl driver + +config PINCTRL_SPRD_SC9860 + bool "Spreadtrum SC9860 pinctrl driver" + help + Say Y here to enable Spreadtrum SC9860 pinctrl driver diff --git a/drivers/pinctrl/sprd/Makefile b/drivers/pinctrl/sprd/Makefile new file mode 100644 index 000..b6caa8c --- /dev/null +++ b/drivers/pinctrl/sprd/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_PINCTRL_SPRD) += pinctrl-sprd.o +obj-$(CONFIG_PINCTRL_SPRD_SC9860) += pinctrl-sprd-sc9860.o diff --git a/drivers/pinctrl/sprd/pinctrl-sprd-sc9860.c b/drivers/pinctrl/sprd/pinctrl-sprd-sc9860.c new file mode 100644 index 000..726780d --- /dev/null +++ b/drivers/pinctrl/sprd/pinctrl-sprd-sc9860.c @@ -0,0 +1,972 @@ +/* + * Spreadtrum pin controller driver + * Copyright (C) 2017 Spreadtrum - http://www.spreadtrum.com + * + * 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. + */ + +#include +#include + +#include "pinctrl-sprd.h" + +enum sprd_sc9860_pins { + /* pin global control register 0 */ + SC9860_VIO28_0_IRTE = SPRD_PIN_INFO(0, GLOBAL_CTRL_PIN, 11, 1, 0), + SC9860_VIO_SD2_IRTE = SPRD_PIN_INFO(1, GLOBAL_CTRL_PIN, 10, 1, 0), + SC9860_VIO_SD0_IRTE = SPRD_PIN_INFO(2, GLOBAL_CTRL_PIN, 9, 1, 0), + SC9860_VIO_SIM2_IRTE = SPRD_PIN_INFO(3, GLOBAL_CTRL_PIN, 8, 1, 0), + SC9860_VIO_SIM1_IRTE = SPRD_PIN_INFO(4, GLOBAL_CTRL_PIN, 7, 1, 0), + SC9860_VIO_SIM0_IRTE = SPRD_PIN_INFO(5, GLOBAL_CTRL_PIN, 6, 1, 0), + SC9860_VIO28_0_MS = SPRD_PIN_INFO(6, GLOBAL_CTRL_PIN, 5, 1, 0), + SC9860_VIO_SD2_MS = SPRD_PIN_INFO(7, GLOBAL_CTRL_PIN, 4, 1, 0), + SC9860_VIO_SD0_MS = SPRD_PIN_INFO(8, GLOBAL_CTRL_PIN, 3, 1, 0), + SC9860_VIO_SIM2_MS = SPRD_PIN_INFO(9, GLOBAL_CTRL_PIN, 2, 1, 0), + SC9860_VIO_SIM1_MS = SPRD_PIN_INFO(10, GLOBAL_CTRL_PIN, 1, 1, 0), + SC9860_VIO_SIM0_MS = SPRD_PIN_INFO(11, GLOBAL_CTRL_PIN, 0, 1, 0), + + /* pin global control register 2 */ + SC9860_SPSPI_PIN_IN_SEL = SPRD_PIN_INFO(12, GLOBAL_CTRL_PIN, 31, 1, 2), + SC9860_UART1_USB30_PHY_SEL = SPRD_PIN_INFO(13, GLOBAL_CTRL_PIN, 30, 1, 2), + SC9860_USB30_PHY_DM_OE = SPRD_PIN_INFO(14, GLOBAL_CTRL_PIN, 29, 1, 2), + SC9860_USB30_PHY_DP_OE = SPRD_PIN_INFO(15, GLOBAL_CTRL_PIN, 28, 1, 2), + SC9860_UART5_SYS_SEL = SPRD_PIN_INFO(16, GLOBAL_CTRL_PIN, 25, 3, 2), + SC9860_ORP_URXD_PIN_IN_SEL = SPRD_PIN_INFO(17, GLOBAL_CTRL_PIN, 24, 1, 2), + SC9860_SIM2_SYS_SEL = SPRD_PIN_INFO(18, GLOBAL_CTRL_PIN, 23, 1, 2), + SC9860_SIM1_SYS_SEL
[PATCH 1/2] DT: pinctrl: Add binding documentation for Spreadtrum pin controller
This patch adds the binding documentation for Spreadtrum SC9860 pin controller device. Signed-off-by: Baolin Wang--- .../devicetree/bindings/pinctrl/sprd,pinctrl.txt | 31 .../bindings/pinctrl/sprd,sc9860-pinctrl.txt | 26 2 files changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt create mode 100644 Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt diff --git a/Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt new file mode 100644 index 000..2edf176 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt @@ -0,0 +1,31 @@ +* Spreadtrum Pin Controller + +The Spreadtrum pin controller are organized in 3 blocks (types). + +The first block comprises some global control registers, and each +register contains several feilds with one bit or several bits to +configurate for some global common configuration, such as domain +pad driving level, system control select and so on. We recognise +every feild comprising one bit or several bits in one global control +register as one pin, thus we should record every pin's bit offset, +bit width and register offset to configurate this feild (pin). + +The second block comprises some common registers which have unified +register definition, and each register described one pin is used +to configurate pin sleep mode and function select. + +The last block comprises some misc registers which also have unified +register definition, and each register described one pin is used to +configurate drive strength, pull up/down and so on. + +This driver supports the generic pin multiplexing and configuration +bindings. For details on each properties, you can refer to +./pinctrl-bindings.txt. + +Required properties for Spreadtrum pin controller: +- compatible: "sprd,-pinctrl" + Please refer to each sprd,-pinctrl.txt binding doc for supported SoCs. + +Required properties for pin configuration node: +- sprd,pins: each entry consists of 2 integers and represents the pin + id and config setting for one pin. diff --git a/Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt new file mode 100644 index 000..26fba5b --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt @@ -0,0 +1,26 @@ +* Spreadtrum SC9860 Pin Controller + +Please refer to sprd,pinctrl.txt in this directory for common binding part +and usage. + +Required properties: +- compatible: must be "sprd,sc9860-pinctrl". +- reg: the register address of pin controller device. +- sprd,pins: two integers array, represents a group of pins id and config + setting. The format is sprd,pins = , PIN_ID can be found + from pinctrl-sprd-sc9860.c file or spec file, CONFIG is the pad setting + value like pull-up for this pin. + +Example: +pin_controller: pinctrl@402a { + compatible = "sprd,sc9860-pinctrl"; + reg = <0x402a 0x1>; + + vio_sd0_ms_0: sd0_ms0 { + sprd,pins = <8 0x1>; + }; + + vbc_iis0_0: iis0_c { + sprd,pins = <34 0xc>; + }; +}; -- 1.7.9.5
[PATCH 1/2] DT: pinctrl: Add binding documentation for Spreadtrum pin controller
This patch adds the binding documentation for Spreadtrum SC9860 pin controller device. Signed-off-by: Baolin Wang --- .../devicetree/bindings/pinctrl/sprd,pinctrl.txt | 31 .../bindings/pinctrl/sprd,sc9860-pinctrl.txt | 26 2 files changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt create mode 100644 Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt diff --git a/Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt new file mode 100644 index 000..2edf176 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt @@ -0,0 +1,31 @@ +* Spreadtrum Pin Controller + +The Spreadtrum pin controller are organized in 3 blocks (types). + +The first block comprises some global control registers, and each +register contains several feilds with one bit or several bits to +configurate for some global common configuration, such as domain +pad driving level, system control select and so on. We recognise +every feild comprising one bit or several bits in one global control +register as one pin, thus we should record every pin's bit offset, +bit width and register offset to configurate this feild (pin). + +The second block comprises some common registers which have unified +register definition, and each register described one pin is used +to configurate pin sleep mode and function select. + +The last block comprises some misc registers which also have unified +register definition, and each register described one pin is used to +configurate drive strength, pull up/down and so on. + +This driver supports the generic pin multiplexing and configuration +bindings. For details on each properties, you can refer to +./pinctrl-bindings.txt. + +Required properties for Spreadtrum pin controller: +- compatible: "sprd,-pinctrl" + Please refer to each sprd,-pinctrl.txt binding doc for supported SoCs. + +Required properties for pin configuration node: +- sprd,pins: each entry consists of 2 integers and represents the pin + id and config setting for one pin. diff --git a/Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt new file mode 100644 index 000..26fba5b --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt @@ -0,0 +1,26 @@ +* Spreadtrum SC9860 Pin Controller + +Please refer to sprd,pinctrl.txt in this directory for common binding part +and usage. + +Required properties: +- compatible: must be "sprd,sc9860-pinctrl". +- reg: the register address of pin controller device. +- sprd,pins: two integers array, represents a group of pins id and config + setting. The format is sprd,pins = , PIN_ID can be found + from pinctrl-sprd-sc9860.c file or spec file, CONFIG is the pad setting + value like pull-up for this pin. + +Example: +pin_controller: pinctrl@402a { + compatible = "sprd,sc9860-pinctrl"; + reg = <0x402a 0x1>; + + vio_sd0_ms_0: sd0_ms0 { + sprd,pins = <8 0x1>; + }; + + vbc_iis0_0: iis0_c { + sprd,pins = <34 0xc>; + }; +}; -- 1.7.9.5
Re: [PATCH v1 1/1] intel_telemetry_debugfs: fix oops found while load/unload module test
On Sat, May 27, 2017 at 08:17:39AM -0700, priyalee.kushw...@intel.com wrote: > From: Priyalee Kushwaha> > This fix oops found while testing load/unload test of > intel_telemetry_debugfs module. Module_init uses register_pm_notifier > for PM callbacks, but unregister_pm_notifier was missing from > module_exit. > > [ 97.481860] BUG: unable to handle kernel paging request at a006f010 > [ 97.489742] IP: blocking_notifier_chain_register+0x3a/0xa0 > [ 97.495898] PGD 2e0a067 > [ 97.495899] PUD 2e0b063 > [ 97.498737] PMD 179e29067 > [ 97.501573] PTE 0 > > [ 97.508423] Oops: 1 PREEMPT SMP > [ 97.512724] Modules linked in: intel_telemetry_debugfs intel_rapl gpio_keys > dwc3 udc_core intel_telemetry_pltdrv intel_punit_ipc intel_telemetry_core > rtc_cmos efivars x86_pkg_temp_thermal iwlwifi snd_hda_codec_hdmi > soc_button_array btusb cfg80211 btrtl mei_me hci_uart btbcm mei btintel i915 > bluetooth intel_pmc_ipc snd_hda_intel spi_pxa2xx_platform snd_hda_codec > dwc3_pci snd_hda_core tpm_tis tpm_tis_core tpm efivarfs > [ 97.558453] CPU: 0 PID: 889 Comm: modprobe Not tainted > 4.11.0-rc6-intel-dev-bkc #1 > [ 97.566950] Hardware name: Intel Corp. Joule DVT3/SDS, BIOS > GTPP181A.X64.0143.B30.1701132137 01/13/2017 > [ 97.577518] task: 8801793a21c0 task.stack: 8801793f > [ 97.584162] RIP: 0010:blocking_notifier_chain_register+0x3a/0xa0 > [ 97.590903] RSP: 0018:8801793f3c58 EFLAGS: 00010286 > [ 97.596802] RAX: a006f000 RBX: 81e3ea20 RCX: > > [ 97.604812] RDX: 880179eaf210 RSI: a0131000 RDI: > 81e3ea20 > [ 97.612821] RBP: 8801793f3c68 R08: 0006 R09: > 005c > [ 97.620847] R10: R11: 0006 R12: > a0131000 > [ 97.628855] R13: R14: 880176e35f48 R15: > 8801793f3ea8 > [ 97.636865] FS: 7f7eeba07700() GS:88017fc0() > knlGS: > [ 97.645948] CS: 0010 DS: ES: CR0: 80050033 > [ 97.652423] CR2: a006f010 CR3: 0001775ef000 CR4: > 003406f0 > [ 97.660423] Call Trace: > [ 97.663166] ? 0xa0031000 > [ 97.666885] register_pm_notifier+0x18/0x20 > [ 97.671581] telemetry_debugfs_init+0x92/0x1000 > > Signed-off-by: Priyalee Kushwaha Hi Priyalee, Thanks for catching this, we should get a fix into the RC cycle. but I think we can make some small changes that will improve legibility here. > --- > drivers/platform/x86/intel_telemetry_debugfs.c | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c > b/drivers/platform/x86/intel_telemetry_debugfs.c > index ef29f18..0f93975 100644 > --- a/drivers/platform/x86/intel_telemetry_debugfs.c > +++ b/drivers/platform/x86/intel_telemetry_debugfs.c > @@ -966,8 +966,12 @@ static int __init telemetry_debugfs_init(void) > #endif /* CONFIG_PM_SLEEP */ > > debugfs_conf->telemetry_dbg_dir = debugfs_create_dir("telemetry", NULL); > - if (!debugfs_conf->telemetry_dbg_dir) > + if (!debugfs_conf->telemetry_dbg_dir) { > +#ifdef CONFIG_PM_SLEEP > + unregister_pm_notifier(_notifier); > +#endif /* CONFIG_PM_SLEEP */ > return -ENOMEM; > + } As a general rule, we try to avoid peppering code with #ifdef blocks, and prefer to create no-op functions, or similar. CONFIG_PM_SLEEP unfortunately doesn't have such no-op functions. Rather than add the CONFIG_PM_SLEEP block above, please convert the above to use an err= and goto statement, and create the appropriate labels below. +Rafael, Len, Pavel, linux-pm: Is there a preferred approach for dealing with CONFIG_PM_SLEEP? > > f = debugfs_create_file("pss_info", S_IFREG | S_IRUGO, > debugfs_conf->telemetry_dbg_dir, NULL, > @@ -1014,6 +1018,9 @@ static int __init telemetry_debugfs_init(void) > out: > debugfs_remove_recursive(debugfs_conf->telemetry_dbg_dir); > debugfs_conf->telemetry_dbg_dir = NULL; e.g. out_pm: > +#ifdef CONFIG_PM_SLEEP > + unregister_pm_notifier(_notifier); > +#endif /* CONFIG_PM_SLEEP */ > > return err; > } > @@ -1022,6 +1029,9 @@ static void __exit telemetry_debugfs_exit(void) > { > debugfs_remove_recursive(debugfs_conf->telemetry_dbg_dir); > debugfs_conf->telemetry_dbg_dir = NULL; > +#ifdef CONFIG_PM_SLEEP > + unregister_pm_notifier(_notifier); > +#endif /* CONFIG_PM_SLEEP */ > } > > late_initcall(telemetry_debugfs_init); > -- > 2.10.0 > > -- Darren Hart VMware Open Source Technology Center
Re: [PATCH v1 1/1] intel_telemetry_debugfs: fix oops found while load/unload module test
On Sat, May 27, 2017 at 08:17:39AM -0700, priyalee.kushw...@intel.com wrote: > From: Priyalee Kushwaha > > This fix oops found while testing load/unload test of > intel_telemetry_debugfs module. Module_init uses register_pm_notifier > for PM callbacks, but unregister_pm_notifier was missing from > module_exit. > > [ 97.481860] BUG: unable to handle kernel paging request at a006f010 > [ 97.489742] IP: blocking_notifier_chain_register+0x3a/0xa0 > [ 97.495898] PGD 2e0a067 > [ 97.495899] PUD 2e0b063 > [ 97.498737] PMD 179e29067 > [ 97.501573] PTE 0 > > [ 97.508423] Oops: 1 PREEMPT SMP > [ 97.512724] Modules linked in: intel_telemetry_debugfs intel_rapl gpio_keys > dwc3 udc_core intel_telemetry_pltdrv intel_punit_ipc intel_telemetry_core > rtc_cmos efivars x86_pkg_temp_thermal iwlwifi snd_hda_codec_hdmi > soc_button_array btusb cfg80211 btrtl mei_me hci_uart btbcm mei btintel i915 > bluetooth intel_pmc_ipc snd_hda_intel spi_pxa2xx_platform snd_hda_codec > dwc3_pci snd_hda_core tpm_tis tpm_tis_core tpm efivarfs > [ 97.558453] CPU: 0 PID: 889 Comm: modprobe Not tainted > 4.11.0-rc6-intel-dev-bkc #1 > [ 97.566950] Hardware name: Intel Corp. Joule DVT3/SDS, BIOS > GTPP181A.X64.0143.B30.1701132137 01/13/2017 > [ 97.577518] task: 8801793a21c0 task.stack: 8801793f > [ 97.584162] RIP: 0010:blocking_notifier_chain_register+0x3a/0xa0 > [ 97.590903] RSP: 0018:8801793f3c58 EFLAGS: 00010286 > [ 97.596802] RAX: a006f000 RBX: 81e3ea20 RCX: > > [ 97.604812] RDX: 880179eaf210 RSI: a0131000 RDI: > 81e3ea20 > [ 97.612821] RBP: 8801793f3c68 R08: 0006 R09: > 005c > [ 97.620847] R10: R11: 0006 R12: > a0131000 > [ 97.628855] R13: R14: 880176e35f48 R15: > 8801793f3ea8 > [ 97.636865] FS: 7f7eeba07700() GS:88017fc0() > knlGS: > [ 97.645948] CS: 0010 DS: ES: CR0: 80050033 > [ 97.652423] CR2: a006f010 CR3: 0001775ef000 CR4: > 003406f0 > [ 97.660423] Call Trace: > [ 97.663166] ? 0xa0031000 > [ 97.666885] register_pm_notifier+0x18/0x20 > [ 97.671581] telemetry_debugfs_init+0x92/0x1000 > > Signed-off-by: Priyalee Kushwaha Hi Priyalee, Thanks for catching this, we should get a fix into the RC cycle. but I think we can make some small changes that will improve legibility here. > --- > drivers/platform/x86/intel_telemetry_debugfs.c | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c > b/drivers/platform/x86/intel_telemetry_debugfs.c > index ef29f18..0f93975 100644 > --- a/drivers/platform/x86/intel_telemetry_debugfs.c > +++ b/drivers/platform/x86/intel_telemetry_debugfs.c > @@ -966,8 +966,12 @@ static int __init telemetry_debugfs_init(void) > #endif /* CONFIG_PM_SLEEP */ > > debugfs_conf->telemetry_dbg_dir = debugfs_create_dir("telemetry", NULL); > - if (!debugfs_conf->telemetry_dbg_dir) > + if (!debugfs_conf->telemetry_dbg_dir) { > +#ifdef CONFIG_PM_SLEEP > + unregister_pm_notifier(_notifier); > +#endif /* CONFIG_PM_SLEEP */ > return -ENOMEM; > + } As a general rule, we try to avoid peppering code with #ifdef blocks, and prefer to create no-op functions, or similar. CONFIG_PM_SLEEP unfortunately doesn't have such no-op functions. Rather than add the CONFIG_PM_SLEEP block above, please convert the above to use an err= and goto statement, and create the appropriate labels below. +Rafael, Len, Pavel, linux-pm: Is there a preferred approach for dealing with CONFIG_PM_SLEEP? > > f = debugfs_create_file("pss_info", S_IFREG | S_IRUGO, > debugfs_conf->telemetry_dbg_dir, NULL, > @@ -1014,6 +1018,9 @@ static int __init telemetry_debugfs_init(void) > out: > debugfs_remove_recursive(debugfs_conf->telemetry_dbg_dir); > debugfs_conf->telemetry_dbg_dir = NULL; e.g. out_pm: > +#ifdef CONFIG_PM_SLEEP > + unregister_pm_notifier(_notifier); > +#endif /* CONFIG_PM_SLEEP */ > > return err; > } > @@ -1022,6 +1029,9 @@ static void __exit telemetry_debugfs_exit(void) > { > debugfs_remove_recursive(debugfs_conf->telemetry_dbg_dir); > debugfs_conf->telemetry_dbg_dir = NULL; > +#ifdef CONFIG_PM_SLEEP > + unregister_pm_notifier(_notifier); > +#endif /* CONFIG_PM_SLEEP */ > } > > late_initcall(telemetry_debugfs_init); > -- > 2.10.0 > > -- Darren Hart VMware Open Source Technology Center
Re: [PATCHv4] usb: typec: Add a sysfs node to manage port type
On Fri, May 26, 2017 at 01:42:57PM -0700, Badhri Jagan Sridharan wrote: > User space applications in some cases have the need to enforce a > specific port type(DFP/UFP/DRP). This change allows userspace to > attempt setting the desired port type. Low level drivers can > however reject the request if the specific port type is not supported. > > Signed-off-by: Badhri Jagan Sridharan> --- > Changelog since v1: > - introduced a new variable port_type in struct typec_port to track > the current port type instead of changing type member in > typec_capability to address Heikki Krogerus comments. > - changed the output format and strings that would be displayed as > suggested by Heikki Krogerus. > > Changelog since v2: > - introduced a new mutex lock to protect port_type for addressing > the race conditions identified by Geunter Roeck > - added typec_port_types_drp for printing port_type when cap->type > is TYPE_PORT_DRP as suggested by Geunter Roeck > - Power role swap and data role swaps would be rejected unless > port port_type == TYPE_PORT_DRP > - port_type_store would return immediately if the current port_type > is same as the new port_type as suggested by Geunter Roeck > > Changelog since v3: > - Moved as much as code outside port_type_lock as suggested by > Geunter Roeck > - Removed Change-Id line from commit message identified by > Greg Kroah-Hartman Ok, this is how you write a changelog for a patch, very nice job! greg k-h
Re: [PATCHv4] usb: typec: Add a sysfs node to manage port type
On Fri, May 26, 2017 at 01:42:57PM -0700, Badhri Jagan Sridharan wrote: > User space applications in some cases have the need to enforce a > specific port type(DFP/UFP/DRP). This change allows userspace to > attempt setting the desired port type. Low level drivers can > however reject the request if the specific port type is not supported. > > Signed-off-by: Badhri Jagan Sridharan > --- > Changelog since v1: > - introduced a new variable port_type in struct typec_port to track > the current port type instead of changing type member in > typec_capability to address Heikki Krogerus comments. > - changed the output format and strings that would be displayed as > suggested by Heikki Krogerus. > > Changelog since v2: > - introduced a new mutex lock to protect port_type for addressing > the race conditions identified by Geunter Roeck > - added typec_port_types_drp for printing port_type when cap->type > is TYPE_PORT_DRP as suggested by Geunter Roeck > - Power role swap and data role swaps would be rejected unless > port port_type == TYPE_PORT_DRP > - port_type_store would return immediately if the current port_type > is same as the new port_type as suggested by Geunter Roeck > > Changelog since v3: > - Moved as much as code outside port_type_lock as suggested by > Geunter Roeck > - Removed Change-Id line from commit message identified by > Greg Kroah-Hartman Ok, this is how you write a changelog for a patch, very nice job! greg k-h
drivers/hwmon/aspeed-pwm-tacho.c:337:21: error: variable 'aspeed_pwm_tacho_regmap_config' has initializer but incomplete type
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: c86daad2c25bfd4a33d48b7691afaa96d9c5ab46 commit: 2d7a548a3eff382da5cd743670693b7657327714 drivers: hwmon: Support for ASPEED PWM/Fan tach date: 7 weeks ago config: x86_64-randconfig-n0-05271314 (attached as .config) compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4 reproduce: git checkout 2d7a548a3eff382da5cd743670693b7657327714 # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): >> drivers/hwmon/aspeed-pwm-tacho.c:337:21: error: variable >> 'aspeed_pwm_tacho_regmap_config' has initializer but incomplete type static const struct regmap_config aspeed_pwm_tacho_regmap_config = { ^ >> drivers/hwmon/aspeed-pwm-tacho.c:338:2: error: unknown field 'reg_bits' >> specified in initializer .reg_bits = 32, ^ >> drivers/hwmon/aspeed-pwm-tacho.c:338:2: warning: excess elements in struct >> initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:338:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:339:2: error: unknown field 'val_bits' >> specified in initializer .val_bits = 32, ^ drivers/hwmon/aspeed-pwm-tacho.c:339:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:339:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:340:2: error: unknown field 'reg_stride' >> specified in initializer .reg_stride = 4, ^ drivers/hwmon/aspeed-pwm-tacho.c:340:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:340:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:341:2: error: unknown field 'max_register' >> specified in initializer .max_register = ASPEED_PTCR_TYPEO_LIMIT, ^ drivers/hwmon/aspeed-pwm-tacho.c:341:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:341:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:342:2: error: unknown field 'reg_write' >> specified in initializer .reg_write = regmap_aspeed_pwm_tacho_reg_write, ^ drivers/hwmon/aspeed-pwm-tacho.c:342:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:342:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:343:2: error: unknown field 'reg_read' >> specified in initializer .reg_read = regmap_aspeed_pwm_tacho_reg_read, ^ drivers/hwmon/aspeed-pwm-tacho.c:343:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:343:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:344:2: error: unknown field 'fast_io' >> specified in initializer .fast_io = true, ^ drivers/hwmon/aspeed-pwm-tacho.c:344:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:344:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c: In function 'aspeed_pwm_tacho_probe': >> drivers/hwmon/aspeed-pwm-tacho.c:783:2: error: implicit declaration of >> function 'devm_regmap_init' [-Werror=implicit-function-declaration] priv->regmap = devm_regmap_init(dev, NULL, (__force void *)regs, ^ >> drivers/hwmon/aspeed-pwm-tacho.c:783:15: warning: assignment makes pointer >> from integer without a cast [enabled by default] priv->regmap = devm_regmap_init(dev, NULL, (__force void *)regs, ^ cc1: some warnings being treated as errors vim +/aspeed_pwm_tacho_regmap_config +337 drivers/hwmon/aspeed-pwm-tacho.c 331 void __iomem *regs = (void __iomem *)context; 332 333 *val = readl(regs + reg); 334 return 0; 335 } 336 > 337 static const struct regmap_config aspeed_pwm_tacho_regmap_config = { > 338 .reg_bits = 32, > 339 .val_bits = 32, > 340 .reg_stride = 4, > 341 .max_register = ASPEED_PTCR_TYPEO_LIMIT, > 342 .reg_write = regmap_aspeed_pwm_tacho_reg_write, > 343 .reg_read = regmap_aspeed_pwm_tacho_reg_read, > 344 .fast_io = true, 345 }; 346 347 static void aspeed_set_clock_enable(struct regmap *regmap, bool val) --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description:
drivers/hwmon/aspeed-pwm-tacho.c:337:21: error: variable 'aspeed_pwm_tacho_regmap_config' has initializer but incomplete type
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: c86daad2c25bfd4a33d48b7691afaa96d9c5ab46 commit: 2d7a548a3eff382da5cd743670693b7657327714 drivers: hwmon: Support for ASPEED PWM/Fan tach date: 7 weeks ago config: x86_64-randconfig-n0-05271314 (attached as .config) compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4 reproduce: git checkout 2d7a548a3eff382da5cd743670693b7657327714 # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): >> drivers/hwmon/aspeed-pwm-tacho.c:337:21: error: variable >> 'aspeed_pwm_tacho_regmap_config' has initializer but incomplete type static const struct regmap_config aspeed_pwm_tacho_regmap_config = { ^ >> drivers/hwmon/aspeed-pwm-tacho.c:338:2: error: unknown field 'reg_bits' >> specified in initializer .reg_bits = 32, ^ >> drivers/hwmon/aspeed-pwm-tacho.c:338:2: warning: excess elements in struct >> initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:338:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:339:2: error: unknown field 'val_bits' >> specified in initializer .val_bits = 32, ^ drivers/hwmon/aspeed-pwm-tacho.c:339:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:339:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:340:2: error: unknown field 'reg_stride' >> specified in initializer .reg_stride = 4, ^ drivers/hwmon/aspeed-pwm-tacho.c:340:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:340:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:341:2: error: unknown field 'max_register' >> specified in initializer .max_register = ASPEED_PTCR_TYPEO_LIMIT, ^ drivers/hwmon/aspeed-pwm-tacho.c:341:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:341:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:342:2: error: unknown field 'reg_write' >> specified in initializer .reg_write = regmap_aspeed_pwm_tacho_reg_write, ^ drivers/hwmon/aspeed-pwm-tacho.c:342:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:342:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:343:2: error: unknown field 'reg_read' >> specified in initializer .reg_read = regmap_aspeed_pwm_tacho_reg_read, ^ drivers/hwmon/aspeed-pwm-tacho.c:343:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:343:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] >> drivers/hwmon/aspeed-pwm-tacho.c:344:2: error: unknown field 'fast_io' >> specified in initializer .fast_io = true, ^ drivers/hwmon/aspeed-pwm-tacho.c:344:2: warning: excess elements in struct initializer [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c:344:2: warning: (near initialization for 'aspeed_pwm_tacho_regmap_config') [enabled by default] drivers/hwmon/aspeed-pwm-tacho.c: In function 'aspeed_pwm_tacho_probe': >> drivers/hwmon/aspeed-pwm-tacho.c:783:2: error: implicit declaration of >> function 'devm_regmap_init' [-Werror=implicit-function-declaration] priv->regmap = devm_regmap_init(dev, NULL, (__force void *)regs, ^ >> drivers/hwmon/aspeed-pwm-tacho.c:783:15: warning: assignment makes pointer >> from integer without a cast [enabled by default] priv->regmap = devm_regmap_init(dev, NULL, (__force void *)regs, ^ cc1: some warnings being treated as errors vim +/aspeed_pwm_tacho_regmap_config +337 drivers/hwmon/aspeed-pwm-tacho.c 331 void __iomem *regs = (void __iomem *)context; 332 333 *val = readl(regs + reg); 334 return 0; 335 } 336 > 337 static const struct regmap_config aspeed_pwm_tacho_regmap_config = { > 338 .reg_bits = 32, > 339 .val_bits = 32, > 340 .reg_stride = 4, > 341 .max_register = ASPEED_PTCR_TYPEO_LIMIT, > 342 .reg_write = regmap_aspeed_pwm_tacho_reg_write, > 343 .reg_read = regmap_aspeed_pwm_tacho_reg_read, > 344 .fast_io = true, 345 }; 346 347 static void aspeed_set_clock_enable(struct regmap *regmap, bool val) --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description:
[PATCH 16/16] platform/x86: dell-wmi: Convert to the WMI bus infrastructure
From: Andy LutomirskiMove some initialization out of _init and into _probe. Update signatures and logic to use the wmi bus and device structures. Signed-off-by: Andy Lutomirski [dvhart: drop deprecated sparse_keymap_free, order declarations, add commit msg] Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/dell-wmi.c | 136 +--- 1 file changed, 70 insertions(+), 66 deletions(-) diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 8a64c79..badc01e 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include "dell-smbios.h" @@ -53,6 +54,10 @@ static bool wmi_requires_smbios_request; MODULE_ALIAS("wmi:"DELL_EVENT_GUID); MODULE_ALIAS("wmi:"DELL_DESCRIPTOR_GUID); +struct dell_wmi_priv { + struct input_dev *input_dev; +}; + static int __init dmi_matched(const struct dmi_system_id *dmi) { wmi_requires_smbios_request = 1; @@ -86,7 +91,7 @@ static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = { * notifications (rather than requests for change) or are also sent * via the keyboard controller so should not be sent again. */ -static const struct key_entry dell_wmi_keymap_type_[] __initconst = { +static const struct key_entry dell_wmi_keymap_type_[] = { { KE_IGNORE, 0x003a, { KEY_CAPSLOCK } }, /* Key code is followed by brightness level */ @@ -207,7 +212,7 @@ struct dell_dmi_results { }; /* Uninitialized entries here are KEY_RESERVED == 0. */ -static const u16 bios_to_linux_keycode[256] __initconst = { +static const u16 bios_to_linux_keycode[256] = { [0] = KEY_MEDIA, [1] = KEY_NEXTSONG, [2] = KEY_PLAYPAUSE, @@ -256,7 +261,7 @@ static const u16 bios_to_linux_keycode[256] __initconst = { * These are applied if the 0xB2 DMI hotkey table is present and doesn't * override them. */ -static const struct key_entry dell_wmi_keymap_type_0010[] __initconst = { +static const struct key_entry dell_wmi_keymap_type_0010[] = { /* Fn-lock */ { KE_IGNORE, 0x151, { KEY_RESERVED } }, @@ -289,7 +294,7 @@ static const struct key_entry dell_wmi_keymap_type_0010[] __initconst = { /* * Keymap for WMI events of type 0x0011 */ -static const struct key_entry dell_wmi_keymap_type_0011[] __initconst = { +static const struct key_entry dell_wmi_keymap_type_0011[] = { /* Battery unplugged */ { KE_IGNORE, 0xfff0, { KEY_RESERVED } }, @@ -304,13 +309,12 @@ static const struct key_entry dell_wmi_keymap_type_0011[] __initconst = { { KE_IGNORE, 0x02f6, { KEY_RESERVED } }, }; -static struct input_dev *dell_wmi_input_dev; - -static void dell_wmi_process_key(int type, int code) +static void dell_wmi_process_key(struct wmi_device *wdev, int type, int code) { + struct dell_wmi_priv *priv = dev_get_drvdata(>dev); const struct key_entry *key; - key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev, + key = sparse_keymap_entry_from_scancode(priv->input_dev, (type << 16) | code); if (!key) { pr_info("Unknown key with type 0x%04x and code 0x%04x pressed\n", @@ -333,33 +337,18 @@ static void dell_wmi_process_key(int type, int code) dell_laptop_call_notifier( DELL_LAPTOP_KBD_BACKLIGHT_BRIGHTNESS_CHANGED, NULL); - sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true); + sparse_keymap_report_entry(priv->input_dev, key, 1, true); } -static void dell_wmi_notify(u32 value, void *context) +static void dell_wmi_notify(struct wmi_device *wdev, + union acpi_object *obj) { - struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL }; - union acpi_object *obj; - acpi_status status; - acpi_size buffer_size; u16 *buffer_entry, *buffer_end; + acpi_size buffer_size; int len, i; - status = wmi_get_event_data(value, ); - if (status != AE_OK) { - pr_warn("bad event status 0x%x\n", status); - return; - } - - obj = (union acpi_object *)response.pointer; - if (!obj) { - pr_warn("no response\n"); - return; - } - if (obj->type != ACPI_TYPE_BUFFER) { pr_warn("bad response type %x\n", obj->type); - kfree(obj); return; } @@ -404,13 +393,14 @@ static void dell_wmi_notify(u32 value, void *context)
[PATCH 16/16] platform/x86: dell-wmi: Convert to the WMI bus infrastructure
From: Andy Lutomirski Move some initialization out of _init and into _probe. Update signatures and logic to use the wmi bus and device structures. Signed-off-by: Andy Lutomirski [dvhart: drop deprecated sparse_keymap_free, order declarations, add commit msg] Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/dell-wmi.c | 136 +--- 1 file changed, 70 insertions(+), 66 deletions(-) diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 8a64c79..badc01e 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include "dell-smbios.h" @@ -53,6 +54,10 @@ static bool wmi_requires_smbios_request; MODULE_ALIAS("wmi:"DELL_EVENT_GUID); MODULE_ALIAS("wmi:"DELL_DESCRIPTOR_GUID); +struct dell_wmi_priv { + struct input_dev *input_dev; +}; + static int __init dmi_matched(const struct dmi_system_id *dmi) { wmi_requires_smbios_request = 1; @@ -86,7 +91,7 @@ static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = { * notifications (rather than requests for change) or are also sent * via the keyboard controller so should not be sent again. */ -static const struct key_entry dell_wmi_keymap_type_[] __initconst = { +static const struct key_entry dell_wmi_keymap_type_[] = { { KE_IGNORE, 0x003a, { KEY_CAPSLOCK } }, /* Key code is followed by brightness level */ @@ -207,7 +212,7 @@ struct dell_dmi_results { }; /* Uninitialized entries here are KEY_RESERVED == 0. */ -static const u16 bios_to_linux_keycode[256] __initconst = { +static const u16 bios_to_linux_keycode[256] = { [0] = KEY_MEDIA, [1] = KEY_NEXTSONG, [2] = KEY_PLAYPAUSE, @@ -256,7 +261,7 @@ static const u16 bios_to_linux_keycode[256] __initconst = { * These are applied if the 0xB2 DMI hotkey table is present and doesn't * override them. */ -static const struct key_entry dell_wmi_keymap_type_0010[] __initconst = { +static const struct key_entry dell_wmi_keymap_type_0010[] = { /* Fn-lock */ { KE_IGNORE, 0x151, { KEY_RESERVED } }, @@ -289,7 +294,7 @@ static const struct key_entry dell_wmi_keymap_type_0010[] __initconst = { /* * Keymap for WMI events of type 0x0011 */ -static const struct key_entry dell_wmi_keymap_type_0011[] __initconst = { +static const struct key_entry dell_wmi_keymap_type_0011[] = { /* Battery unplugged */ { KE_IGNORE, 0xfff0, { KEY_RESERVED } }, @@ -304,13 +309,12 @@ static const struct key_entry dell_wmi_keymap_type_0011[] __initconst = { { KE_IGNORE, 0x02f6, { KEY_RESERVED } }, }; -static struct input_dev *dell_wmi_input_dev; - -static void dell_wmi_process_key(int type, int code) +static void dell_wmi_process_key(struct wmi_device *wdev, int type, int code) { + struct dell_wmi_priv *priv = dev_get_drvdata(>dev); const struct key_entry *key; - key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev, + key = sparse_keymap_entry_from_scancode(priv->input_dev, (type << 16) | code); if (!key) { pr_info("Unknown key with type 0x%04x and code 0x%04x pressed\n", @@ -333,33 +337,18 @@ static void dell_wmi_process_key(int type, int code) dell_laptop_call_notifier( DELL_LAPTOP_KBD_BACKLIGHT_BRIGHTNESS_CHANGED, NULL); - sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true); + sparse_keymap_report_entry(priv->input_dev, key, 1, true); } -static void dell_wmi_notify(u32 value, void *context) +static void dell_wmi_notify(struct wmi_device *wdev, + union acpi_object *obj) { - struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL }; - union acpi_object *obj; - acpi_status status; - acpi_size buffer_size; u16 *buffer_entry, *buffer_end; + acpi_size buffer_size; int len, i; - status = wmi_get_event_data(value, ); - if (status != AE_OK) { - pr_warn("bad event status 0x%x\n", status); - return; - } - - obj = (union acpi_object *)response.pointer; - if (!obj) { - pr_warn("no response\n"); - return; - } - if (obj->type != ACPI_TYPE_BUFFER) { pr_warn("bad response type %x\n", obj->type); - kfree(obj); return; } @@ -404,13 +393,14 @@ static void dell_wmi_notify(u32 value, void *context) switch (buffer_entry[1]) { case 0x: /* One key pressed or event occurred */ if (len > 2) -
[PATCH 15/16] platform/x86: wmi-mof: New driver to expose embedded WMI MOF metadata
From: Andy LutomirskiQuite a few laptops (and maybe servers?) have embedded WMI MOF metadata. I think that Samba has tools to interpret it, but there is currently no interface to get the data in the first place. In most cases, the MOF can be read out of the DSDT, but that is non-compliant and messy. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org [dvhart: make sysfs mof binary read only, fixup comment block format] Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/Kconfig | 12 drivers/platform/x86/Makefile | 1 + drivers/platform/x86/wmi-mof.c | 125 + 3 files changed, 138 insertions(+) create mode 100644 drivers/platform/x86/wmi-mof.c diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 49a1d01..1f27600 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -656,6 +656,18 @@ config ACPI_WMI It is safe to enable this driver even if your DSDT doesn't define any ACPI-WMI devices. +config WMI_MOF + tristate "WMI embedded MOF driver" + depends on ACPI_WMI + default y + ---help--- + Say Y here if you want to be able to read a firmware-embedded + WMI MOF schema. Using this requires userspace tools and may be + rather tedious. + + To compile this driver as a module, choose M here: the module will + be called wmi-mof. + config MSI_WMI tristate "MSI WMI extras" depends on ACPI_WMI diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 652d7c8..1147212 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -38,6 +38,7 @@ obj-$(CONFIG_MSI_WMI) += msi-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_SURFACE3_WMI) += surface3-wmi.o obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o +obj-$(CONFIG_WMI_MOF) += wmi-mof.o # toshiba_acpi must link after wmi to ensure that wmi devices are found # before toshiba_acpi initializes diff --git a/drivers/platform/x86/wmi-mof.c b/drivers/platform/x86/wmi-mof.c new file mode 100644 index 000..464ceca --- /dev/null +++ b/drivers/platform/x86/wmi-mof.c @@ -0,0 +1,125 @@ +/* + * WMI embedded MOF driver + * + * Copyright (c) 2015 Andrew Lutomirski + * + * 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 +#include +#include +#include +#include +#include +#include +#include + +#define WMI_MOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910" +MODULE_ALIAS("wmi:" WMI_MOF_GUID); + +struct mof_priv { + union acpi_object *mofdata; + struct bin_attribute mof_bin_attr; +}; + +static ssize_t +read_mof(struct file *filp, struct kobject *kobj, +struct bin_attribute *attr, +char *buf, loff_t off, size_t count) +{ + struct mof_priv *priv = + container_of(attr, struct mof_priv, mof_bin_attr); + + if (off >= priv->mofdata->buffer.length) + return 0; + + if (count > priv->mofdata->buffer.length - off) + count = priv->mofdata->buffer.length - off; + + memcpy(buf, priv->mofdata->buffer.pointer + off, count); + return count; +} + +static int wmi_mof_probe(struct wmi_device *wdev) +{ + int ret; + + struct mof_priv *priv = + devm_kzalloc(>dev, sizeof(struct mof_priv), GFP_KERNEL); + + if (!priv) + return -ENOMEM; + + dev_set_drvdata(>dev, priv); + + priv->mofdata = wmidev_block_query(wdev, 0); + if (!priv->mofdata) { + dev_warn(>dev, "failed to read MOF\n"); + return -EIO; + } + + if (priv->mofdata->type != ACPI_TYPE_BUFFER) { + dev_warn(>dev, "MOF is not a buffer\n"); + ret = -EIO; + goto err_free; + } + + sysfs_bin_attr_init(>mof_bin_attr); + priv->mof_bin_attr.attr.name = "mof"; + priv->mof_bin_attr.attr.mode = 0400; + priv->mof_bin_attr.read = read_mof; + priv->mof_bin_attr.size = priv->mofdata->buffer.length; + + ret = sysfs_create_bin_file(>dev.kobj, >mof_bin_attr); + if (ret) +
[PATCH 15/16] platform/x86: wmi-mof: New driver to expose embedded WMI MOF metadata
From: Andy Lutomirski Quite a few laptops (and maybe servers?) have embedded WMI MOF metadata. I think that Samba has tools to interpret it, but there is currently no interface to get the data in the first place. In most cases, the MOF can be read out of the DSDT, but that is non-compliant and messy. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org [dvhart: make sysfs mof binary read only, fixup comment block format] Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/Kconfig | 12 drivers/platform/x86/Makefile | 1 + drivers/platform/x86/wmi-mof.c | 125 + 3 files changed, 138 insertions(+) create mode 100644 drivers/platform/x86/wmi-mof.c diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 49a1d01..1f27600 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -656,6 +656,18 @@ config ACPI_WMI It is safe to enable this driver even if your DSDT doesn't define any ACPI-WMI devices. +config WMI_MOF + tristate "WMI embedded MOF driver" + depends on ACPI_WMI + default y + ---help--- + Say Y here if you want to be able to read a firmware-embedded + WMI MOF schema. Using this requires userspace tools and may be + rather tedious. + + To compile this driver as a module, choose M here: the module will + be called wmi-mof. + config MSI_WMI tristate "MSI WMI extras" depends on ACPI_WMI diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 652d7c8..1147212 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -38,6 +38,7 @@ obj-$(CONFIG_MSI_WMI) += msi-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_SURFACE3_WMI) += surface3-wmi.o obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o +obj-$(CONFIG_WMI_MOF) += wmi-mof.o # toshiba_acpi must link after wmi to ensure that wmi devices are found # before toshiba_acpi initializes diff --git a/drivers/platform/x86/wmi-mof.c b/drivers/platform/x86/wmi-mof.c new file mode 100644 index 000..464ceca --- /dev/null +++ b/drivers/platform/x86/wmi-mof.c @@ -0,0 +1,125 @@ +/* + * WMI embedded MOF driver + * + * Copyright (c) 2015 Andrew Lutomirski + * + * 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 +#include +#include +#include +#include +#include +#include +#include + +#define WMI_MOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910" +MODULE_ALIAS("wmi:" WMI_MOF_GUID); + +struct mof_priv { + union acpi_object *mofdata; + struct bin_attribute mof_bin_attr; +}; + +static ssize_t +read_mof(struct file *filp, struct kobject *kobj, +struct bin_attribute *attr, +char *buf, loff_t off, size_t count) +{ + struct mof_priv *priv = + container_of(attr, struct mof_priv, mof_bin_attr); + + if (off >= priv->mofdata->buffer.length) + return 0; + + if (count > priv->mofdata->buffer.length - off) + count = priv->mofdata->buffer.length - off; + + memcpy(buf, priv->mofdata->buffer.pointer + off, count); + return count; +} + +static int wmi_mof_probe(struct wmi_device *wdev) +{ + int ret; + + struct mof_priv *priv = + devm_kzalloc(>dev, sizeof(struct mof_priv), GFP_KERNEL); + + if (!priv) + return -ENOMEM; + + dev_set_drvdata(>dev, priv); + + priv->mofdata = wmidev_block_query(wdev, 0); + if (!priv->mofdata) { + dev_warn(>dev, "failed to read MOF\n"); + return -EIO; + } + + if (priv->mofdata->type != ACPI_TYPE_BUFFER) { + dev_warn(>dev, "MOF is not a buffer\n"); + ret = -EIO; + goto err_free; + } + + sysfs_bin_attr_init(>mof_bin_attr); + priv->mof_bin_attr.attr.name = "mof"; + priv->mof_bin_attr.attr.mode = 0400; + priv->mof_bin_attr.read = read_mof; + priv->mof_bin_attr.size = priv->mofdata->buffer.length; + + ret = sysfs_create_bin_file(>dev.kobj, >mof_bin_attr); + if (ret) + goto err_free; + + return 0; + +err_free: + kfree(priv->mofdata); + return ret; +} + +static int wmi_mof_remove(struct
[PATCH 01/16] platform/x86: wmi: Drop "Mapper (un)loaded" messages
From: Andy LutomirskiWMI is just a driver. There is no need to announce when it is loaded. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index ceeb8c1..0043581 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -836,7 +836,6 @@ static int __init acpi_wmi_init(void) return error; } - pr_info("Mapper loaded\n"); return 0; } @@ -844,8 +843,6 @@ static void __exit acpi_wmi_exit(void) { acpi_bus_unregister_driver(_wmi_driver); class_unregister(_class); - - pr_info("Mapper unloaded\n"); } subsys_initcall(acpi_wmi_init); -- 2.9.4
[PATCH 01/16] platform/x86: wmi: Drop "Mapper (un)loaded" messages
From: Andy Lutomirski WMI is just a driver. There is no need to announce when it is loaded. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index ceeb8c1..0043581 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -836,7 +836,6 @@ static int __init acpi_wmi_init(void) return error; } - pr_info("Mapper loaded\n"); return 0; } @@ -844,8 +843,6 @@ static void __exit acpi_wmi_exit(void) { acpi_bus_unregister_driver(_wmi_driver); class_unregister(_class); - - pr_info("Mapper unloaded\n"); } subsys_initcall(acpi_wmi_init); -- 2.9.4
[PATCH 04/16] platform/x86: wmi: Track wmi devices per ACPI device
From: Andy LutomirskiCurrently we free all devices when we detach from any ACPI node. Instead, keep track of which node WMI devices are attached to and free them only as needed. While we are at it, match up notifications with the device they came from correctly. This will make our behavior more straightforward on systems with more than one WMI node in the ACPI tables (e.g. the Dell XPS 13 9350). This also adds a warning when GUIDs are not unique. NB: The guid_string parameter in guid_already_parsed was a little-endian binary GUID, not a string. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 58 -- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index ac60a51..faaa9a7 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -64,7 +64,7 @@ struct guid_block { struct wmi_block { struct list_head list; struct guid_block gblock; - acpi_handle handle; + struct acpi_device *acpi_device; wmi_notify_handler handler; void *handler_data; struct device dev; @@ -147,7 +147,7 @@ static acpi_status wmi_method_enable(struct wmi_block *wblock, int enable) acpi_handle handle; block = >gblock; - handle = wblock->handle; + handle = wblock->acpi_device->handle; snprintf(method, 5, "WE%02X", block->notify_id); status = acpi_execute_simple_method(handle, method, enable); @@ -186,7 +186,7 @@ u32 method_id, const struct acpi_buffer *in, struct acpi_buffer *out) return AE_ERROR; block = >gblock; - handle = wblock->handle; + handle = wblock->acpi_device->handle; if (!(block->flags & ACPI_WMI_METHOD)) return AE_BAD_DATA; @@ -248,7 +248,7 @@ struct acpi_buffer *out) return AE_ERROR; block = >gblock; - handle = wblock->handle; + handle = wblock->acpi_device->handle; if (block->instance_count < instance) return AE_BAD_PARAMETER; @@ -321,7 +321,7 @@ const struct acpi_buffer *in) return AE_ERROR; block = >gblock; - handle = wblock->handle; + handle = wblock->acpi_device->handle; if (block->instance_count < instance) return AE_BAD_PARAMETER; @@ -525,8 +525,8 @@ acpi_status wmi_get_event_data(u32 event, struct acpi_buffer *out) if ((gblock->flags & ACPI_WMI_EVENT) && (gblock->notify_id == event)) - return acpi_evaluate_object(wblock->handle, "_WED", - , out); + return acpi_evaluate_object(wblock->acpi_device->handle, + "_WED", , out); } return AE_NOT_FOUND; @@ -618,27 +618,40 @@ static int wmi_create_device(const struct guid_block *gblock, return device_register(>dev); } -static void wmi_free_devices(void) +static void wmi_free_devices(struct acpi_device *device) { struct wmi_block *wblock, *next; /* Delete devices for all the GUIDs */ list_for_each_entry_safe(wblock, next, _block_list, list) { - list_del(>list); - if (wblock->dev.class) - device_unregister(>dev); - else - kfree(wblock); + if (wblock->acpi_device == device) { + list_del(>list); + if (wblock->dev.class) + device_unregister(>dev); + else + kfree(wblock); + } } } -static bool guid_already_parsed(const char *guid_string) +static bool guid_already_parsed(struct acpi_device *device, + const u8 *guid) { struct wmi_block *wblock; - list_for_each_entry(wblock, _block_list, list) - if (memcmp(wblock->gblock.guid, guid_string, 16) == 0) + list_for_each_entry(wblock, _block_list, list) { + if (memcmp(wblock->gblock.guid, guid, 16) == 0) { + /* +* Because we historically didn't track the relationship +* between GUIDs and ACPI nodes, we don't know whether +* we need to suppress GUIDs that are unique on a +* given node but duplicated across nodes. +*/ + dev_warn(>dev, "duplicate WMI GUID %pUL
[PATCH 04/16] platform/x86: wmi: Track wmi devices per ACPI device
From: Andy Lutomirski Currently we free all devices when we detach from any ACPI node. Instead, keep track of which node WMI devices are attached to and free them only as needed. While we are at it, match up notifications with the device they came from correctly. This will make our behavior more straightforward on systems with more than one WMI node in the ACPI tables (e.g. the Dell XPS 13 9350). This also adds a warning when GUIDs are not unique. NB: The guid_string parameter in guid_already_parsed was a little-endian binary GUID, not a string. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 58 -- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index ac60a51..faaa9a7 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -64,7 +64,7 @@ struct guid_block { struct wmi_block { struct list_head list; struct guid_block gblock; - acpi_handle handle; + struct acpi_device *acpi_device; wmi_notify_handler handler; void *handler_data; struct device dev; @@ -147,7 +147,7 @@ static acpi_status wmi_method_enable(struct wmi_block *wblock, int enable) acpi_handle handle; block = >gblock; - handle = wblock->handle; + handle = wblock->acpi_device->handle; snprintf(method, 5, "WE%02X", block->notify_id); status = acpi_execute_simple_method(handle, method, enable); @@ -186,7 +186,7 @@ u32 method_id, const struct acpi_buffer *in, struct acpi_buffer *out) return AE_ERROR; block = >gblock; - handle = wblock->handle; + handle = wblock->acpi_device->handle; if (!(block->flags & ACPI_WMI_METHOD)) return AE_BAD_DATA; @@ -248,7 +248,7 @@ struct acpi_buffer *out) return AE_ERROR; block = >gblock; - handle = wblock->handle; + handle = wblock->acpi_device->handle; if (block->instance_count < instance) return AE_BAD_PARAMETER; @@ -321,7 +321,7 @@ const struct acpi_buffer *in) return AE_ERROR; block = >gblock; - handle = wblock->handle; + handle = wblock->acpi_device->handle; if (block->instance_count < instance) return AE_BAD_PARAMETER; @@ -525,8 +525,8 @@ acpi_status wmi_get_event_data(u32 event, struct acpi_buffer *out) if ((gblock->flags & ACPI_WMI_EVENT) && (gblock->notify_id == event)) - return acpi_evaluate_object(wblock->handle, "_WED", - , out); + return acpi_evaluate_object(wblock->acpi_device->handle, + "_WED", , out); } return AE_NOT_FOUND; @@ -618,27 +618,40 @@ static int wmi_create_device(const struct guid_block *gblock, return device_register(>dev); } -static void wmi_free_devices(void) +static void wmi_free_devices(struct acpi_device *device) { struct wmi_block *wblock, *next; /* Delete devices for all the GUIDs */ list_for_each_entry_safe(wblock, next, _block_list, list) { - list_del(>list); - if (wblock->dev.class) - device_unregister(>dev); - else - kfree(wblock); + if (wblock->acpi_device == device) { + list_del(>list); + if (wblock->dev.class) + device_unregister(>dev); + else + kfree(wblock); + } } } -static bool guid_already_parsed(const char *guid_string) +static bool guid_already_parsed(struct acpi_device *device, + const u8 *guid) { struct wmi_block *wblock; - list_for_each_entry(wblock, _block_list, list) - if (memcmp(wblock->gblock.guid, guid_string, 16) == 0) + list_for_each_entry(wblock, _block_list, list) { + if (memcmp(wblock->gblock.guid, guid, 16) == 0) { + /* +* Because we historically didn't track the relationship +* between GUIDs and ACPI nodes, we don't know whether +* we need to suppress GUIDs that are unique on a +* given node but duplicated across nodes. +*/ + dev_warn(>dev, "duplicate WMI GUID %pUL (first instance was on %s)\n", +guid, dev_name(>acpi_device->dev)); return true; +
[PATCH 05/16] platform/x86: wmi: Turn WMI into a bus driver
From: Andy LutomirskiWMI is logically a bus: the WMI driver binds to an ACPI node (or more than one), and each instance of the WMI driver enumerates its children and hopes that drivers will attach to the children that are useful. This patch gives WMI a driver model bus type and the ability to match to drivers. The bus itself is a device in the new "wmi_bus" class, and all of the individual WMI devices are slotted into the device hierarchy correctly. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 197 +++-- include/linux/wmi.h| 47 +++ 2 files changed, 201 insertions(+), 43 deletions(-) create mode 100644 include/linux/wmi.h diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index faaa9a7..f06b7c0 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -37,6 +37,7 @@ #include #include #include +#include #include ACPI_MODULE_NAME("wmi"); @@ -44,8 +45,6 @@ MODULE_AUTHOR("Carlos Corbacho"); MODULE_DESCRIPTION("ACPI-WMI Mapping Driver"); MODULE_LICENSE("GPL"); -#define ACPI_WMI_CLASS "wmi" - static LIST_HEAD(wmi_block_list); struct guid_block { @@ -62,12 +61,12 @@ struct guid_block { }; struct wmi_block { + struct wmi_device dev; struct list_head list; struct guid_block gblock; struct acpi_device *acpi_device; wmi_notify_handler handler; void *handler_data; - struct device dev; }; @@ -102,8 +101,8 @@ static const struct acpi_device_id wmi_device_ids[] = { MODULE_DEVICE_TABLE(acpi, wmi_device_ids); static struct acpi_driver acpi_wmi_driver = { - .name = "wmi", - .class = ACPI_WMI_CLASS, + .name = "acpi-wmi", + .owner = THIS_MODULE, .ids = wmi_device_ids, .ops = { .add = acpi_wmi_add, @@ -545,77 +544,146 @@ bool wmi_has_guid(const char *guid_string) } EXPORT_SYMBOL_GPL(wmi_has_guid); +static struct wmi_block *dev_to_wblock(struct device *dev) +{ + return container_of(dev, struct wmi_block, dev.dev); +} + +static struct wmi_device *dev_to_wdev(struct device *dev) +{ + return container_of(dev, struct wmi_device, dev); +} + /* * sysfs interface */ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct wmi_block *wblock; - - wblock = dev_get_drvdata(dev); - if (!wblock) { - strcat(buf, "\n"); - return strlen(buf); - } + struct wmi_block *wblock = dev_to_wblock(dev); return sprintf(buf, "wmi:%pUL\n", wblock->gblock.guid); } static DEVICE_ATTR_RO(modalias); +static ssize_t guid_show(struct device *dev, struct device_attribute *attr, +char *buf) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + return sprintf(buf, "%pUL\n", wblock->gblock.guid); +} +static DEVICE_ATTR_RO(guid); + static struct attribute *wmi_attrs[] = { _attr_modalias.attr, + _attr_guid.attr, NULL, }; ATTRIBUTE_GROUPS(wmi); static int wmi_dev_uevent(struct device *dev, struct kobj_uevent_env *env) { - char guid_string[37]; - - struct wmi_block *wblock; + struct wmi_block *wblock = dev_to_wblock(dev); - if (add_uevent_var(env, "MODALIAS=")) + if (add_uevent_var(env, "MODALIAS=wmi:%pUL", wblock->gblock.guid)) return -ENOMEM; - wblock = dev_get_drvdata(dev); - if (!wblock) + if (add_uevent_var(env, "WMI_GUID=%pUL", wblock->gblock.guid)) return -ENOMEM; - sprintf(guid_string, "%pUL", wblock->gblock.guid); + return 0; +} + +static void wmi_dev_release(struct device *dev) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + kfree(wblock); +} + +static int wmi_dev_match(struct device *dev, struct device_driver *driver) +{ + struct wmi_driver *wmi_driver = + container_of(driver, struct wmi_driver, driver); + struct wmi_block *wblock = dev_to_wblock(dev); + const struct wmi_device_id *id = wmi_driver->id_table; - strcpy(>buf[env->buflen - 1], "wmi:"); - memcpy(>buf[env->buflen - 1 + 4], guid_string, 36); - env->buflen += 40; + while (id->guid_string) { + uuid_le driver_guid; + + if (WARN_ON(uuid_le_to_bin(id->guid_string, _guid))) + continue; + if (!memcmp(_guid, wblock->gblock.guid, 16)) + return 1; + + id++; + } return 0;
[PATCH 05/16] platform/x86: wmi: Turn WMI into a bus driver
From: Andy Lutomirski WMI is logically a bus: the WMI driver binds to an ACPI node (or more than one), and each instance of the WMI driver enumerates its children and hopes that drivers will attach to the children that are useful. This patch gives WMI a driver model bus type and the ability to match to drivers. The bus itself is a device in the new "wmi_bus" class, and all of the individual WMI devices are slotted into the device hierarchy correctly. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 197 +++-- include/linux/wmi.h| 47 +++ 2 files changed, 201 insertions(+), 43 deletions(-) create mode 100644 include/linux/wmi.h diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index faaa9a7..f06b7c0 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -37,6 +37,7 @@ #include #include #include +#include #include ACPI_MODULE_NAME("wmi"); @@ -44,8 +45,6 @@ MODULE_AUTHOR("Carlos Corbacho"); MODULE_DESCRIPTION("ACPI-WMI Mapping Driver"); MODULE_LICENSE("GPL"); -#define ACPI_WMI_CLASS "wmi" - static LIST_HEAD(wmi_block_list); struct guid_block { @@ -62,12 +61,12 @@ struct guid_block { }; struct wmi_block { + struct wmi_device dev; struct list_head list; struct guid_block gblock; struct acpi_device *acpi_device; wmi_notify_handler handler; void *handler_data; - struct device dev; }; @@ -102,8 +101,8 @@ static const struct acpi_device_id wmi_device_ids[] = { MODULE_DEVICE_TABLE(acpi, wmi_device_ids); static struct acpi_driver acpi_wmi_driver = { - .name = "wmi", - .class = ACPI_WMI_CLASS, + .name = "acpi-wmi", + .owner = THIS_MODULE, .ids = wmi_device_ids, .ops = { .add = acpi_wmi_add, @@ -545,77 +544,146 @@ bool wmi_has_guid(const char *guid_string) } EXPORT_SYMBOL_GPL(wmi_has_guid); +static struct wmi_block *dev_to_wblock(struct device *dev) +{ + return container_of(dev, struct wmi_block, dev.dev); +} + +static struct wmi_device *dev_to_wdev(struct device *dev) +{ + return container_of(dev, struct wmi_device, dev); +} + /* * sysfs interface */ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct wmi_block *wblock; - - wblock = dev_get_drvdata(dev); - if (!wblock) { - strcat(buf, "\n"); - return strlen(buf); - } + struct wmi_block *wblock = dev_to_wblock(dev); return sprintf(buf, "wmi:%pUL\n", wblock->gblock.guid); } static DEVICE_ATTR_RO(modalias); +static ssize_t guid_show(struct device *dev, struct device_attribute *attr, +char *buf) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + return sprintf(buf, "%pUL\n", wblock->gblock.guid); +} +static DEVICE_ATTR_RO(guid); + static struct attribute *wmi_attrs[] = { _attr_modalias.attr, + _attr_guid.attr, NULL, }; ATTRIBUTE_GROUPS(wmi); static int wmi_dev_uevent(struct device *dev, struct kobj_uevent_env *env) { - char guid_string[37]; - - struct wmi_block *wblock; + struct wmi_block *wblock = dev_to_wblock(dev); - if (add_uevent_var(env, "MODALIAS=")) + if (add_uevent_var(env, "MODALIAS=wmi:%pUL", wblock->gblock.guid)) return -ENOMEM; - wblock = dev_get_drvdata(dev); - if (!wblock) + if (add_uevent_var(env, "WMI_GUID=%pUL", wblock->gblock.guid)) return -ENOMEM; - sprintf(guid_string, "%pUL", wblock->gblock.guid); + return 0; +} + +static void wmi_dev_release(struct device *dev) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + kfree(wblock); +} + +static int wmi_dev_match(struct device *dev, struct device_driver *driver) +{ + struct wmi_driver *wmi_driver = + container_of(driver, struct wmi_driver, driver); + struct wmi_block *wblock = dev_to_wblock(dev); + const struct wmi_device_id *id = wmi_driver->id_table; - strcpy(>buf[env->buflen - 1], "wmi:"); - memcpy(>buf[env->buflen - 1 + 4], guid_string, 36); - env->buflen += 40; + while (id->guid_string) { + uuid_le driver_guid; + + if (WARN_ON(uuid_le_to_bin(id->guid_string, _guid))) + continue; + if (!memcmp(_guid, wblock->gblock.guid, 16)) + return 1; + + id++; + } return 0; } -static void wmi_dev_free(struct device *dev) +static int wmi_dev_probe(struct device *dev) { - struct wmi_block *wmi_block =
[PATCH 06/16] platform/x86: wmi: Fix error handling when creating devices
From: Andy LutomirskiWe have two memory leaks. If guid_already_parsed returned true, we leak the wmi_block. If wmi_create_device failed, we leak the device. Simplify the logic and fix both of them. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 28 +++- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index f06b7c0..31c317f 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -757,6 +757,15 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) if (debug_dump_wdg) wmi_dump_wdg([i]); + /* +* Some WMI devices, like those for nVidia hooks, have a +* duplicate GUID. It's not clear what we should do in this +* case yet, so for now, we'll just ignore the duplicate +* for device creation. +*/ + if (guid_already_parsed(device, gblock[i].guid)) + continue; + wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL); if (!wblock) return -ENOMEM; @@ -764,19 +773,12 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) wblock->acpi_device = device; wblock->gblock = gblock[i]; - /* - Some WMI devices, like those for nVidia hooks, have a - duplicate GUID. It's not clear what we should do in this - case yet, so for now, we'll just ignore the duplicate - for device creation. - */ - if (!guid_already_parsed(device, gblock[i].guid)) { - retval = wmi_create_device(wmi_bus_dev, [i], - wblock, device); - if (retval) { - wmi_free_devices(device); - goto out_free_pointer; - } + retval = wmi_create_device(wmi_bus_dev, [i], + wblock, device); + if (retval) { + put_device(>dev.dev); + wmi_free_devices(device); + goto out_free_pointer; } list_add_tail(>list, _block_list); -- 2.9.4
[PATCH 06/16] platform/x86: wmi: Fix error handling when creating devices
From: Andy Lutomirski We have two memory leaks. If guid_already_parsed returned true, we leak the wmi_block. If wmi_create_device failed, we leak the device. Simplify the logic and fix both of them. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 28 +++- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index f06b7c0..31c317f 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -757,6 +757,15 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) if (debug_dump_wdg) wmi_dump_wdg([i]); + /* +* Some WMI devices, like those for nVidia hooks, have a +* duplicate GUID. It's not clear what we should do in this +* case yet, so for now, we'll just ignore the duplicate +* for device creation. +*/ + if (guid_already_parsed(device, gblock[i].guid)) + continue; + wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL); if (!wblock) return -ENOMEM; @@ -764,19 +773,12 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) wblock->acpi_device = device; wblock->gblock = gblock[i]; - /* - Some WMI devices, like those for nVidia hooks, have a - duplicate GUID. It's not clear what we should do in this - case yet, so for now, we'll just ignore the duplicate - for device creation. - */ - if (!guid_already_parsed(device, gblock[i].guid)) { - retval = wmi_create_device(wmi_bus_dev, [i], - wblock, device); - if (retval) { - wmi_free_devices(device); - goto out_free_pointer; - } + retval = wmi_create_device(wmi_bus_dev, [i], + wblock, device); + if (retval) { + put_device(>dev.dev); + wmi_free_devices(device); + goto out_free_pointer; } list_add_tail(>list, _block_list); -- 2.9.4
[PATCH 09/16] platform/x86: wmi: Instantiate all devices before adding them
From: Andy LutomirskiAt some point, we will want sub-drivers to get references to other devices on the same WMI bus. This change is needed to avoid races. This ends up simplifying the setup code and fixing some leaks, too. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 49 +- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 651693a..bfc0a3f 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -796,7 +796,7 @@ static struct device_type wmi_type_data = { .release = wmi_dev_release, }; -static int wmi_create_device(struct device *wmi_bus_dev, +static void wmi_create_device(struct device *wmi_bus_dev, const struct guid_block *gblock, struct wmi_block *wblock, struct acpi_device *device) @@ -852,7 +852,7 @@ static int wmi_create_device(struct device *wmi_bus_dev, } - return device_register(>dev.dev); + device_initialize(>dev.dev); } static void wmi_free_devices(struct acpi_device *device) @@ -863,10 +863,14 @@ static void wmi_free_devices(struct acpi_device *device) list_for_each_entry_safe(wblock, next, _block_list, list) { if (wblock->acpi_device == device) { list_del(>list); - if (wblock->dev.dev.bus) - device_unregister(>dev.dev); - else + if (wblock->dev.dev.bus) { + /* Device was initialized. */ + device_del(>dev.dev); + put_device(>dev.dev); + } else { + /* device_initialize was not called. */ kfree(wblock); + } } } } @@ -901,9 +905,9 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) struct acpi_buffer out = {ACPI_ALLOCATE_BUFFER, NULL}; union acpi_object *obj; const struct guid_block *gblock; - struct wmi_block *wblock; + struct wmi_block *wblock, *next; acpi_status status; - int retval; + int retval = 0; u32 i, total; status = acpi_evaluate_object(device->handle, "_WDG", NULL, ); @@ -936,19 +940,15 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) continue; wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL); - if (!wblock) - return -ENOMEM; + if (!wblock) { + retval = -ENOMEM; + break; + } wblock->acpi_device = device; wblock->gblock = gblock[i]; - retval = wmi_create_device(wmi_bus_dev, [i], - wblock, device); - if (retval) { - put_device(>dev.dev); - wmi_free_devices(device); - goto out_free_pointer; - } + wmi_create_device(wmi_bus_dev, [i], wblock, device); list_add_tail(>list, _block_list); @@ -958,11 +958,24 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) } } - retval = 0; - out_free_pointer: kfree(out.pointer); + /* +* Now that all of the devices are created, add them to the +* device tree and probe subdrivers. +*/ + list_for_each_entry_safe(wblock, next, _block_list, list) { + if (wblock->acpi_device == device) { + if (device_add(>dev.dev) != 0) { + dev_err(wmi_bus_dev, + "failed to register %pULL\n", + wblock->gblock.guid); + list_del(>list); + } + } + } + return retval; } -- 2.9.4
[PATCH 10/16] platform/x86: wmi: Incorporate acpi_install_notify_handler
From: Andy LutomirskiAs a platform driver, acpi_driver.notify will not be available, so use acpi_install_notify_handler as we will be converting to a platform driver. This gives event drivers a simple way to handle events. It also seems closer to what the Windows docs suggest that Windows does: it sounds like, in Windows, the mapper is responsible for called _WED before dispatching to the subdriver. Signed-off-by: Andy Lutomirski [dvhart: merge two development commits and update commit message] Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 89 +- include/linux/wmi.h| 1 + 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index bfc0a3f..208e187 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -93,7 +93,6 @@ MODULE_PARM_DESC(debug_dump_wdg, static int acpi_wmi_remove(struct acpi_device *device); static int acpi_wmi_add(struct acpi_device *device); -static void acpi_wmi_notify(struct acpi_device *device, u32 event); static const struct acpi_device_id wmi_device_ids[] = { {"PNP0C14", 0}, @@ -109,7 +108,6 @@ static struct acpi_driver acpi_wmi_driver = { .ops = { .add = acpi_wmi_add, .remove = acpi_wmi_remove, - .notify = acpi_wmi_notify, }, }; @@ -1023,36 +1021,80 @@ acpi_wmi_ec_space_handler(u32 function, acpi_physical_address address, } } -static void acpi_wmi_notify(struct acpi_device *device, u32 event) +static void acpi_wmi_notify_handler(acpi_handle handle, u32 event, + void *context) { struct guid_block *block; struct wmi_block *wblock; struct list_head *p; + bool found_it = false; list_for_each(p, _block_list) { wblock = list_entry(p, struct wmi_block, list); block = >gblock; - if (wblock->acpi_device == device && + if (wblock->acpi_device->handle == handle && (block->flags & ACPI_WMI_EVENT) && - (block->notify_id == event)) { - if (wblock->handler) - wblock->handler(event, wblock->handler_data); - if (debug_event) { - pr_info("DEBUG Event GUID: %pUL\n", - wblock->gblock.guid); - } - - acpi_bus_generate_netlink_event( - device->pnp.device_class, dev_name(>dev), - event, 0); + (block->notify_id == event)) + { + found_it = true; break; } } + + if (!found_it) + return; + + /* If a driver is bound, then notify the driver. */ + if (wblock->dev.dev.driver) { + struct wmi_driver *driver; + struct acpi_object_list input; + union acpi_object params[1]; + struct acpi_buffer evdata = { ACPI_ALLOCATE_BUFFER, NULL }; + acpi_status status; + + driver = container_of(wblock->dev.dev.driver, + struct wmi_driver, driver); + + input.count = 1; + input.pointer = params; + params[0].type = ACPI_TYPE_INTEGER; + params[0].integer.value = event; + + status = acpi_evaluate_object(wblock->acpi_device->handle, + "_WED", , ); + if (ACPI_FAILURE(status)) { + dev_warn(>dev.dev, +"failed to get event data\n"); + return; + } + + if (driver->notify) + driver->notify(>dev, + (union acpi_object *)evdata.pointer); + + kfree(evdata.pointer); + } else if (wblock->handler) { + /* Legacy handler */ + wblock->handler(event, wblock->handler_data); + } + + if (debug_event) { + pr_info("DEBUG Event GUID: %pUL\n", + wblock->gblock.guid); + } + + acpi_bus_generate_netlink_event( + wblock->acpi_device->pnp.device_class, + dev_name(>dev.dev), + event, 0); + } static int acpi_wmi_remove(struct acpi_device *device) { +
[PATCH 09/16] platform/x86: wmi: Instantiate all devices before adding them
From: Andy Lutomirski At some point, we will want sub-drivers to get references to other devices on the same WMI bus. This change is needed to avoid races. This ends up simplifying the setup code and fixing some leaks, too. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 49 +- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 651693a..bfc0a3f 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -796,7 +796,7 @@ static struct device_type wmi_type_data = { .release = wmi_dev_release, }; -static int wmi_create_device(struct device *wmi_bus_dev, +static void wmi_create_device(struct device *wmi_bus_dev, const struct guid_block *gblock, struct wmi_block *wblock, struct acpi_device *device) @@ -852,7 +852,7 @@ static int wmi_create_device(struct device *wmi_bus_dev, } - return device_register(>dev.dev); + device_initialize(>dev.dev); } static void wmi_free_devices(struct acpi_device *device) @@ -863,10 +863,14 @@ static void wmi_free_devices(struct acpi_device *device) list_for_each_entry_safe(wblock, next, _block_list, list) { if (wblock->acpi_device == device) { list_del(>list); - if (wblock->dev.dev.bus) - device_unregister(>dev.dev); - else + if (wblock->dev.dev.bus) { + /* Device was initialized. */ + device_del(>dev.dev); + put_device(>dev.dev); + } else { + /* device_initialize was not called. */ kfree(wblock); + } } } } @@ -901,9 +905,9 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) struct acpi_buffer out = {ACPI_ALLOCATE_BUFFER, NULL}; union acpi_object *obj; const struct guid_block *gblock; - struct wmi_block *wblock; + struct wmi_block *wblock, *next; acpi_status status; - int retval; + int retval = 0; u32 i, total; status = acpi_evaluate_object(device->handle, "_WDG", NULL, ); @@ -936,19 +940,15 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) continue; wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL); - if (!wblock) - return -ENOMEM; + if (!wblock) { + retval = -ENOMEM; + break; + } wblock->acpi_device = device; wblock->gblock = gblock[i]; - retval = wmi_create_device(wmi_bus_dev, [i], - wblock, device); - if (retval) { - put_device(>dev.dev); - wmi_free_devices(device); - goto out_free_pointer; - } + wmi_create_device(wmi_bus_dev, [i], wblock, device); list_add_tail(>list, _block_list); @@ -958,11 +958,24 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) } } - retval = 0; - out_free_pointer: kfree(out.pointer); + /* +* Now that all of the devices are created, add them to the +* device tree and probe subdrivers. +*/ + list_for_each_entry_safe(wblock, next, _block_list, list) { + if (wblock->acpi_device == device) { + if (device_add(>dev.dev) != 0) { + dev_err(wmi_bus_dev, + "failed to register %pULL\n", + wblock->gblock.guid); + list_del(>list); + } + } + } + return retval; } -- 2.9.4
[PATCH 10/16] platform/x86: wmi: Incorporate acpi_install_notify_handler
From: Andy Lutomirski As a platform driver, acpi_driver.notify will not be available, so use acpi_install_notify_handler as we will be converting to a platform driver. This gives event drivers a simple way to handle events. It also seems closer to what the Windows docs suggest that Windows does: it sounds like, in Windows, the mapper is responsible for called _WED before dispatching to the subdriver. Signed-off-by: Andy Lutomirski [dvhart: merge two development commits and update commit message] Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 89 +- include/linux/wmi.h| 1 + 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index bfc0a3f..208e187 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -93,7 +93,6 @@ MODULE_PARM_DESC(debug_dump_wdg, static int acpi_wmi_remove(struct acpi_device *device); static int acpi_wmi_add(struct acpi_device *device); -static void acpi_wmi_notify(struct acpi_device *device, u32 event); static const struct acpi_device_id wmi_device_ids[] = { {"PNP0C14", 0}, @@ -109,7 +108,6 @@ static struct acpi_driver acpi_wmi_driver = { .ops = { .add = acpi_wmi_add, .remove = acpi_wmi_remove, - .notify = acpi_wmi_notify, }, }; @@ -1023,36 +1021,80 @@ acpi_wmi_ec_space_handler(u32 function, acpi_physical_address address, } } -static void acpi_wmi_notify(struct acpi_device *device, u32 event) +static void acpi_wmi_notify_handler(acpi_handle handle, u32 event, + void *context) { struct guid_block *block; struct wmi_block *wblock; struct list_head *p; + bool found_it = false; list_for_each(p, _block_list) { wblock = list_entry(p, struct wmi_block, list); block = >gblock; - if (wblock->acpi_device == device && + if (wblock->acpi_device->handle == handle && (block->flags & ACPI_WMI_EVENT) && - (block->notify_id == event)) { - if (wblock->handler) - wblock->handler(event, wblock->handler_data); - if (debug_event) { - pr_info("DEBUG Event GUID: %pUL\n", - wblock->gblock.guid); - } - - acpi_bus_generate_netlink_event( - device->pnp.device_class, dev_name(>dev), - event, 0); + (block->notify_id == event)) + { + found_it = true; break; } } + + if (!found_it) + return; + + /* If a driver is bound, then notify the driver. */ + if (wblock->dev.dev.driver) { + struct wmi_driver *driver; + struct acpi_object_list input; + union acpi_object params[1]; + struct acpi_buffer evdata = { ACPI_ALLOCATE_BUFFER, NULL }; + acpi_status status; + + driver = container_of(wblock->dev.dev.driver, + struct wmi_driver, driver); + + input.count = 1; + input.pointer = params; + params[0].type = ACPI_TYPE_INTEGER; + params[0].integer.value = event; + + status = acpi_evaluate_object(wblock->acpi_device->handle, + "_WED", , ); + if (ACPI_FAILURE(status)) { + dev_warn(>dev.dev, +"failed to get event data\n"); + return; + } + + if (driver->notify) + driver->notify(>dev, + (union acpi_object *)evdata.pointer); + + kfree(evdata.pointer); + } else if (wblock->handler) { + /* Legacy handler */ + wblock->handler(event, wblock->handler_data); + } + + if (debug_event) { + pr_info("DEBUG Event GUID: %pUL\n", + wblock->gblock.guid); + } + + acpi_bus_generate_netlink_event( + wblock->acpi_device->pnp.device_class, + dev_name(>dev.dev), + event, 0); + } static int acpi_wmi_remove(struct acpi_device *device) { + acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, + acpi_wmi_notify_handler);
[PATCH 00/16] Convert WMI to a proper bus
From: "Darren Hart (VMware)"This series is based on the original work of Andy Lutomirski [1]. I have made minor edits, and in one instance, squashed two patches in which the latter undid the former. This series converts WMI [2] into a proper bus, adds some useful information via sysfs, and exposes the embedded MOF [3] binary. It converts dell-wmi to use the new WMI bus architecture. This is the first part of an ongoing effort to enhance the WMI infrastructure within the kernel, and eventually expose WMI to userspace for the consumption of management utilities as it was intended. 1. https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=platform/wmi 2. https://msdn.microsoft.com/en-us/library/windows/hardware/dn614028(v=vs.85).aspx 3. https://msdn.microsoft.com/en-us/library/aa823192(v=vs.85).aspx Andy Lutomirski (15): platform/x86: wmi: Drop "Mapper (un)loaded" messages platform/x86: wmi: Pass the acpi_device through to parse_wdg platform/x86: wmi: Clean up acpi_wmi_add platform/x86: wmi: Track wmi devices per ACPI device platform/x86: wmi: Turn WMI into a bus driver platform/x86: wmi: Fix error handling when creating devices platform/x86: wmi: Split devices into types and add basic sysfs attributes platform/x86: wmi: Probe data objects for read and write capabilities platform/x86: wmi: Instantiate all devices before adding them platform/x86: wmi: Incorporate acpi_install_notify_handler platform/x86: wmi: Add a new interface to read block data platform/x86: wmi: Bind the platform device, not the ACPI node platform/x86: wmi: Add an interface for subdrivers to access sibling devices platform/x86: wmi-mof: New driver to expose embedded WMI MOF metadata platform/x86: dell-wmi: Convert to the WMI bus infrastructure Darren Hart (VMware) (1): platform/x86: wmi: Require query for data blocks, rename writable to setable drivers/platform/x86/Kconfig| 12 + drivers/platform/x86/Makefile | 1 + drivers/platform/x86/dell-wmi.c | 136 drivers/platform/x86/wmi-mof.c | 125 drivers/platform/x86/wmi.c | 677 include/linux/wmi.h | 59 6 files changed, 815 insertions(+), 195 deletions(-) create mode 100644 drivers/platform/x86/wmi-mof.c create mode 100644 include/linux/wmi.h -- 2.9.4
[PATCH 00/16] Convert WMI to a proper bus
From: "Darren Hart (VMware)" This series is based on the original work of Andy Lutomirski [1]. I have made minor edits, and in one instance, squashed two patches in which the latter undid the former. This series converts WMI [2] into a proper bus, adds some useful information via sysfs, and exposes the embedded MOF [3] binary. It converts dell-wmi to use the new WMI bus architecture. This is the first part of an ongoing effort to enhance the WMI infrastructure within the kernel, and eventually expose WMI to userspace for the consumption of management utilities as it was intended. 1. https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=platform/wmi 2. https://msdn.microsoft.com/en-us/library/windows/hardware/dn614028(v=vs.85).aspx 3. https://msdn.microsoft.com/en-us/library/aa823192(v=vs.85).aspx Andy Lutomirski (15): platform/x86: wmi: Drop "Mapper (un)loaded" messages platform/x86: wmi: Pass the acpi_device through to parse_wdg platform/x86: wmi: Clean up acpi_wmi_add platform/x86: wmi: Track wmi devices per ACPI device platform/x86: wmi: Turn WMI into a bus driver platform/x86: wmi: Fix error handling when creating devices platform/x86: wmi: Split devices into types and add basic sysfs attributes platform/x86: wmi: Probe data objects for read and write capabilities platform/x86: wmi: Instantiate all devices before adding them platform/x86: wmi: Incorporate acpi_install_notify_handler platform/x86: wmi: Add a new interface to read block data platform/x86: wmi: Bind the platform device, not the ACPI node platform/x86: wmi: Add an interface for subdrivers to access sibling devices platform/x86: wmi-mof: New driver to expose embedded WMI MOF metadata platform/x86: dell-wmi: Convert to the WMI bus infrastructure Darren Hart (VMware) (1): platform/x86: wmi: Require query for data blocks, rename writable to setable drivers/platform/x86/Kconfig| 12 + drivers/platform/x86/Makefile | 1 + drivers/platform/x86/dell-wmi.c | 136 drivers/platform/x86/wmi-mof.c | 125 drivers/platform/x86/wmi.c | 677 include/linux/wmi.h | 59 6 files changed, 815 insertions(+), 195 deletions(-) create mode 100644 drivers/platform/x86/wmi-mof.c create mode 100644 include/linux/wmi.h -- 2.9.4
[PATCH 08/16] platform/x86: wmi: Probe data objects for read and write capabilities
From: Andy LutomirskiThe Dell XPS 13 9350 has one RW data object, one RO data object, and one totally inaccessible data object. Check for the existence of the accessor methods and report in sysfs. The docs also permit WQxx getters for single-instance objects to take no parameters. Probe for that as well to avoid ACPICA warnings about mismatched signatures. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 101 +++-- include/linux/wmi.h| 6 +++ 2 files changed, 103 insertions(+), 4 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 33a3609..651693a 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -67,6 +67,8 @@ struct wmi_block { struct acpi_device *acpi_device; wmi_notify_handler handler; void *handler_data; + + bool read_takes_no_args;/* only defined if readable */ }; @@ -138,6 +140,30 @@ static bool find_guid(const char *guid_string, struct wmi_block **out) return false; } +static int get_subobj_info(acpi_handle handle, const char *pathname, + struct acpi_device_info **info) +{ + struct acpi_device_info *dummy_info, **info_ptr; + acpi_handle subobj_handle; + acpi_status status; + + status = acpi_get_handle(handle, (char *)pathname, _handle); + if (status == AE_NOT_FOUND) + return -ENOENT; + else if (ACPI_FAILURE(status)) + return -EIO; + + info_ptr = info ? info : _info; + status = acpi_get_object_info(subobj_handle, info_ptr); + if (ACPI_FAILURE(status)) + return -EIO; + + if (!info) + kfree(dummy_info); + + return 0; +} + static acpi_status wmi_method_enable(struct wmi_block *wblock, int enable) { struct guid_block *block = NULL; @@ -261,6 +287,9 @@ struct acpi_buffer *out) wq_params[0].type = ACPI_TYPE_INTEGER; wq_params[0].integer.value = instance; + if (instance == 0 && wblock->read_takes_no_args) + input.count = 0; + /* * If ACPI_WMI_EXPENSIVE, call the relevant WCxx method first to * enable collection. @@ -628,11 +657,37 @@ static ssize_t object_id_show(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RO(object_id); -static struct attribute *wmi_data_or_method_attrs[] = { +static ssize_t readable_show(struct device *dev, struct device_attribute *attr, +char *buf) +{ + struct wmi_device *wdev = dev_to_wdev(dev); + + return sprintf(buf, "%d\n", (int)wdev->readable); +} +static DEVICE_ATTR_RO(readable); + +static ssize_t writeable_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct wmi_device *wdev = dev_to_wdev(dev); + + return sprintf(buf, "%d\n", (int)wdev->writeable); +} +static DEVICE_ATTR_RO(writeable); + +static struct attribute *wmi_data_attrs[] = { + _attr_object_id.attr, + _attr_readable.attr, + _attr_writeable.attr, + NULL, +}; +ATTRIBUTE_GROUPS(wmi_data); + +static struct attribute *wmi_method_attrs[] = { _attr_object_id.attr, NULL, }; -ATTRIBUTE_GROUPS(wmi_data_or_method); +ATTRIBUTE_GROUPS(wmi_method); static int wmi_dev_uevent(struct device *dev, struct kobj_uevent_env *env) { @@ -731,13 +786,13 @@ static struct device_type wmi_type_event = { static struct device_type wmi_type_method = { .name = "method", - .groups = wmi_data_or_method_groups, + .groups = wmi_method_groups, .release = wmi_dev_release, }; static struct device_type wmi_type_data = { .name = "data", - .groups = wmi_data_or_method_groups, + .groups = wmi_data_groups, .release = wmi_dev_release, }; @@ -756,7 +811,45 @@ static int wmi_create_device(struct device *wmi_bus_dev, } else if (gblock->flags & ACPI_WMI_METHOD) { wblock->dev.dev.type = _type_method; } else { + struct acpi_device_info *info; + char method[5]; + int result; + wblock->dev.dev.type = _type_data; + + strcpy(method, "WQ"); + strncat(method, wblock->gblock.object_id, 2); + result = get_subobj_info(device->handle, method, ); + + if (result == 0) { + wblock->dev.readable = true; + + /* +* The Microsoft documentation specifically states: +
[PATCH 08/16] platform/x86: wmi: Probe data objects for read and write capabilities
From: Andy Lutomirski The Dell XPS 13 9350 has one RW data object, one RO data object, and one totally inaccessible data object. Check for the existence of the accessor methods and report in sysfs. The docs also permit WQxx getters for single-instance objects to take no parameters. Probe for that as well to avoid ACPICA warnings about mismatched signatures. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 101 +++-- include/linux/wmi.h| 6 +++ 2 files changed, 103 insertions(+), 4 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 33a3609..651693a 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -67,6 +67,8 @@ struct wmi_block { struct acpi_device *acpi_device; wmi_notify_handler handler; void *handler_data; + + bool read_takes_no_args;/* only defined if readable */ }; @@ -138,6 +140,30 @@ static bool find_guid(const char *guid_string, struct wmi_block **out) return false; } +static int get_subobj_info(acpi_handle handle, const char *pathname, + struct acpi_device_info **info) +{ + struct acpi_device_info *dummy_info, **info_ptr; + acpi_handle subobj_handle; + acpi_status status; + + status = acpi_get_handle(handle, (char *)pathname, _handle); + if (status == AE_NOT_FOUND) + return -ENOENT; + else if (ACPI_FAILURE(status)) + return -EIO; + + info_ptr = info ? info : _info; + status = acpi_get_object_info(subobj_handle, info_ptr); + if (ACPI_FAILURE(status)) + return -EIO; + + if (!info) + kfree(dummy_info); + + return 0; +} + static acpi_status wmi_method_enable(struct wmi_block *wblock, int enable) { struct guid_block *block = NULL; @@ -261,6 +287,9 @@ struct acpi_buffer *out) wq_params[0].type = ACPI_TYPE_INTEGER; wq_params[0].integer.value = instance; + if (instance == 0 && wblock->read_takes_no_args) + input.count = 0; + /* * If ACPI_WMI_EXPENSIVE, call the relevant WCxx method first to * enable collection. @@ -628,11 +657,37 @@ static ssize_t object_id_show(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RO(object_id); -static struct attribute *wmi_data_or_method_attrs[] = { +static ssize_t readable_show(struct device *dev, struct device_attribute *attr, +char *buf) +{ + struct wmi_device *wdev = dev_to_wdev(dev); + + return sprintf(buf, "%d\n", (int)wdev->readable); +} +static DEVICE_ATTR_RO(readable); + +static ssize_t writeable_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct wmi_device *wdev = dev_to_wdev(dev); + + return sprintf(buf, "%d\n", (int)wdev->writeable); +} +static DEVICE_ATTR_RO(writeable); + +static struct attribute *wmi_data_attrs[] = { + _attr_object_id.attr, + _attr_readable.attr, + _attr_writeable.attr, + NULL, +}; +ATTRIBUTE_GROUPS(wmi_data); + +static struct attribute *wmi_method_attrs[] = { _attr_object_id.attr, NULL, }; -ATTRIBUTE_GROUPS(wmi_data_or_method); +ATTRIBUTE_GROUPS(wmi_method); static int wmi_dev_uevent(struct device *dev, struct kobj_uevent_env *env) { @@ -731,13 +786,13 @@ static struct device_type wmi_type_event = { static struct device_type wmi_type_method = { .name = "method", - .groups = wmi_data_or_method_groups, + .groups = wmi_method_groups, .release = wmi_dev_release, }; static struct device_type wmi_type_data = { .name = "data", - .groups = wmi_data_or_method_groups, + .groups = wmi_data_groups, .release = wmi_dev_release, }; @@ -756,7 +811,45 @@ static int wmi_create_device(struct device *wmi_bus_dev, } else if (gblock->flags & ACPI_WMI_METHOD) { wblock->dev.dev.type = _type_method; } else { + struct acpi_device_info *info; + char method[5]; + int result; + wblock->dev.dev.type = _type_data; + + strcpy(method, "WQ"); + strncat(method, wblock->gblock.object_id, 2); + result = get_subobj_info(device->handle, method, ); + + if (result == 0) { + wblock->dev.readable = true; + + /* +* The Microsoft documentation specifically states: +* +* Data blocks registered with only a single instance +* can ignore
[PATCH 11/16] platform/x86: wmi: Add a new interface to read block data
From: Andy Lutomirskiwmi_query_block is unnecessarily indirect. Add a straightforward method for wmi bus drivers to use to read block data. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 54 -- include/linux/wmi.h| 4 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 208e187..483e4a6 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -244,19 +244,10 @@ u32 method_id, const struct acpi_buffer *in, struct acpi_buffer *out) } EXPORT_SYMBOL_GPL(wmi_evaluate_method); -/** - * wmi_query_block - Return contents of a WMI block - * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba - * @instance: Instance index - * : Empty buffer to return the contents of the data block to - * - * Return the contents of an ACPI-WMI data block to a buffer - */ -acpi_status wmi_query_block(const char *guid_string, u8 instance, -struct acpi_buffer *out) +static acpi_status __query_block(struct wmi_block *wblock, u8 instance, +struct acpi_buffer *out) { struct guid_block *block = NULL; - struct wmi_block *wblock = NULL; acpi_handle handle; acpi_status status, wc_status = AE_ERROR; struct acpi_object_list input; @@ -264,12 +255,9 @@ struct acpi_buffer *out) char method[5]; char wc_method[5] = "WC"; - if (!guid_string || !out) + if (!out) return AE_BAD_PARAMETER; - if (!find_guid(guid_string, )) - return AE_ERROR; - block = >gblock; handle = wblock->acpi_device->handle; @@ -320,8 +308,42 @@ struct acpi_buffer *out) return status; } + +/** + * wmi_query_block - Return contents of a WMI block (deprecated) + * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba + * @instance: Instance index + * : Empty buffer to return the contents of the data block to + * + * Return the contents of an ACPI-WMI data block to a buffer + */ +acpi_status wmi_query_block(const char *guid_string, u8 instance, + struct acpi_buffer *out) +{ + struct wmi_block *wblock; + + if (!guid_string) + return AE_BAD_PARAMETER; + + if (!find_guid(guid_string, )) + return AE_ERROR; + + return __query_block(wblock, instance, out); +} EXPORT_SYMBOL_GPL(wmi_query_block); +union acpi_object *wmidev_block_query(struct wmi_device *wdev, u8 instance) +{ + struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL }; + struct wmi_block *wblock = container_of(wdev, struct wmi_block, dev); + + if (ACPI_FAILURE(__query_block(wblock, instance, ))) + return NULL; + + return (union acpi_object *)out.pointer; +} +EXPORT_SYMBOL_GPL(wmidev_block_query); + /** * wmi_set_block - Write to a WMI block * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba @@ -331,7 +353,7 @@ EXPORT_SYMBOL_GPL(wmi_query_block); * Write the contents of the input buffer to an ACPI-WMI data block */ acpi_status wmi_set_block(const char *guid_string, u8 instance, -const struct acpi_buffer *in) + const struct acpi_buffer *in) { struct guid_block *block = NULL; struct wmi_block *wblock = NULL; diff --git a/include/linux/wmi.h b/include/linux/wmi.h index c6eedfd..0ab2540 100644 --- a/include/linux/wmi.h +++ b/include/linux/wmi.h @@ -29,6 +29,10 @@ struct wmi_device { bool readable, writeable; }; +/* Caller must kfree the result. */ +extern union acpi_object *wmidev_block_query(struct wmi_device *wdev, +u8 instance); + struct wmi_device_id { const char *guid_string; }; -- 2.9.4
[PATCH 11/16] platform/x86: wmi: Add a new interface to read block data
From: Andy Lutomirski wmi_query_block is unnecessarily indirect. Add a straightforward method for wmi bus drivers to use to read block data. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 54 -- include/linux/wmi.h| 4 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 208e187..483e4a6 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -244,19 +244,10 @@ u32 method_id, const struct acpi_buffer *in, struct acpi_buffer *out) } EXPORT_SYMBOL_GPL(wmi_evaluate_method); -/** - * wmi_query_block - Return contents of a WMI block - * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba - * @instance: Instance index - * : Empty buffer to return the contents of the data block to - * - * Return the contents of an ACPI-WMI data block to a buffer - */ -acpi_status wmi_query_block(const char *guid_string, u8 instance, -struct acpi_buffer *out) +static acpi_status __query_block(struct wmi_block *wblock, u8 instance, +struct acpi_buffer *out) { struct guid_block *block = NULL; - struct wmi_block *wblock = NULL; acpi_handle handle; acpi_status status, wc_status = AE_ERROR; struct acpi_object_list input; @@ -264,12 +255,9 @@ struct acpi_buffer *out) char method[5]; char wc_method[5] = "WC"; - if (!guid_string || !out) + if (!out) return AE_BAD_PARAMETER; - if (!find_guid(guid_string, )) - return AE_ERROR; - block = >gblock; handle = wblock->acpi_device->handle; @@ -320,8 +308,42 @@ struct acpi_buffer *out) return status; } + +/** + * wmi_query_block - Return contents of a WMI block (deprecated) + * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba + * @instance: Instance index + * : Empty buffer to return the contents of the data block to + * + * Return the contents of an ACPI-WMI data block to a buffer + */ +acpi_status wmi_query_block(const char *guid_string, u8 instance, + struct acpi_buffer *out) +{ + struct wmi_block *wblock; + + if (!guid_string) + return AE_BAD_PARAMETER; + + if (!find_guid(guid_string, )) + return AE_ERROR; + + return __query_block(wblock, instance, out); +} EXPORT_SYMBOL_GPL(wmi_query_block); +union acpi_object *wmidev_block_query(struct wmi_device *wdev, u8 instance) +{ + struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL }; + struct wmi_block *wblock = container_of(wdev, struct wmi_block, dev); + + if (ACPI_FAILURE(__query_block(wblock, instance, ))) + return NULL; + + return (union acpi_object *)out.pointer; +} +EXPORT_SYMBOL_GPL(wmidev_block_query); + /** * wmi_set_block - Write to a WMI block * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba @@ -331,7 +353,7 @@ EXPORT_SYMBOL_GPL(wmi_query_block); * Write the contents of the input buffer to an ACPI-WMI data block */ acpi_status wmi_set_block(const char *guid_string, u8 instance, -const struct acpi_buffer *in) + const struct acpi_buffer *in) { struct guid_block *block = NULL; struct wmi_block *wblock = NULL; diff --git a/include/linux/wmi.h b/include/linux/wmi.h index c6eedfd..0ab2540 100644 --- a/include/linux/wmi.h +++ b/include/linux/wmi.h @@ -29,6 +29,10 @@ struct wmi_device { bool readable, writeable; }; +/* Caller must kfree the result. */ +extern union acpi_object *wmidev_block_query(struct wmi_device *wdev, +u8 instance); + struct wmi_device_id { const char *guid_string; }; -- 2.9.4
[PATCH 12/16] platform/x86: wmi: Bind the platform device, not the ACPI node
From: Andy LutomirskiWe already have the PNP glue to instantiate platform devices for the ACPI devices that WMI drives. WMI should therefore attach to the platform device, not the ACPI node. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 57 +++--- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 483e4a6..f1c9464 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -91,8 +92,8 @@ module_param(debug_dump_wdg, bool, 0444); MODULE_PARM_DESC(debug_dump_wdg, "Dump available WMI interfaces [0/1]"); -static int acpi_wmi_remove(struct acpi_device *device); -static int acpi_wmi_add(struct acpi_device *device); +static int acpi_wmi_remove(struct platform_device *device); +static int acpi_wmi_probe(struct platform_device *device); static const struct acpi_device_id wmi_device_ids[] = { {"PNP0C14", 0}, @@ -101,14 +102,13 @@ static const struct acpi_device_id wmi_device_ids[] = { }; MODULE_DEVICE_TABLE(acpi, wmi_device_ids); -static struct acpi_driver acpi_wmi_driver = { - .name = "acpi-wmi", - .owner = THIS_MODULE, - .ids = wmi_device_ids, - .ops = { - .add = acpi_wmi_add, - .remove = acpi_wmi_remove, +static struct platform_driver acpi_wmi_driver = { + .driver = { + .name = "acpi-wmi", + .acpi_match_table = wmi_device_ids, }, + .probe = acpi_wmi_probe, + .remove = acpi_wmi_remove, }; /* @@ -1113,26 +1113,34 @@ static void acpi_wmi_notify_handler(acpi_handle handle, u32 event, } -static int acpi_wmi_remove(struct acpi_device *device) +static int acpi_wmi_remove(struct platform_device *device) { - acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, + struct acpi_device *acpi_device = ACPI_COMPANION(>dev); + + acpi_remove_notify_handler(acpi_device->handle, ACPI_DEVICE_NOTIFY, acpi_wmi_notify_handler); - acpi_remove_address_space_handler(device->handle, + acpi_remove_address_space_handler(acpi_device->handle, ACPI_ADR_SPACE_EC, _wmi_ec_space_handler); - wmi_free_devices(device); - device_unregister((struct device *)acpi_driver_data(device)); - device->driver_data = NULL; + wmi_free_devices(acpi_device); + device_unregister((struct device *)dev_get_drvdata(>dev)); return 0; } -static int acpi_wmi_add(struct acpi_device *device) +static int acpi_wmi_probe(struct platform_device *device) { + struct acpi_device *acpi_device; struct device *wmi_bus_dev; acpi_status status; int error; - status = acpi_install_address_space_handler(device->handle, + acpi_device = ACPI_COMPANION(>dev); + if (!acpi_device) { + dev_err(>dev, "ACPI companion is missing\n"); + return -ENODEV; + } + + status = acpi_install_address_space_handler(acpi_device->handle, ACPI_ADR_SPACE_EC, _wmi_ec_space_handler, NULL, NULL); @@ -1141,7 +1149,8 @@ static int acpi_wmi_add(struct acpi_device *device) return -ENODEV; } - status = acpi_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, + status = acpi_install_notify_handler(acpi_device->handle, +ACPI_DEVICE_NOTIFY, acpi_wmi_notify_handler, NULL); if (ACPI_FAILURE(status)) { @@ -1156,9 +1165,9 @@ static int acpi_wmi_add(struct acpi_device *device) error = PTR_ERR(wmi_bus_dev); goto err_remove_notify_handler; } - device->driver_data = wmi_bus_dev; + dev_set_drvdata(>dev, wmi_bus_dev); - error = parse_wdg(wmi_bus_dev, device); + error = parse_wdg(wmi_bus_dev, acpi_device); if (error) { pr_err("Failed to parse WDG method\n"); goto err_remove_busdev; @@ -1170,11 +1179,11 @@ static int acpi_wmi_add(struct acpi_device *device) device_unregister(wmi_bus_dev); err_remove_notify_handler: - acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, +
[PATCH 12/16] platform/x86: wmi: Bind the platform device, not the ACPI node
From: Andy Lutomirski We already have the PNP glue to instantiate platform devices for the ACPI devices that WMI drives. WMI should therefore attach to the platform device, not the ACPI node. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 57 +++--- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 483e4a6..f1c9464 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -91,8 +92,8 @@ module_param(debug_dump_wdg, bool, 0444); MODULE_PARM_DESC(debug_dump_wdg, "Dump available WMI interfaces [0/1]"); -static int acpi_wmi_remove(struct acpi_device *device); -static int acpi_wmi_add(struct acpi_device *device); +static int acpi_wmi_remove(struct platform_device *device); +static int acpi_wmi_probe(struct platform_device *device); static const struct acpi_device_id wmi_device_ids[] = { {"PNP0C14", 0}, @@ -101,14 +102,13 @@ static const struct acpi_device_id wmi_device_ids[] = { }; MODULE_DEVICE_TABLE(acpi, wmi_device_ids); -static struct acpi_driver acpi_wmi_driver = { - .name = "acpi-wmi", - .owner = THIS_MODULE, - .ids = wmi_device_ids, - .ops = { - .add = acpi_wmi_add, - .remove = acpi_wmi_remove, +static struct platform_driver acpi_wmi_driver = { + .driver = { + .name = "acpi-wmi", + .acpi_match_table = wmi_device_ids, }, + .probe = acpi_wmi_probe, + .remove = acpi_wmi_remove, }; /* @@ -1113,26 +1113,34 @@ static void acpi_wmi_notify_handler(acpi_handle handle, u32 event, } -static int acpi_wmi_remove(struct acpi_device *device) +static int acpi_wmi_remove(struct platform_device *device) { - acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, + struct acpi_device *acpi_device = ACPI_COMPANION(>dev); + + acpi_remove_notify_handler(acpi_device->handle, ACPI_DEVICE_NOTIFY, acpi_wmi_notify_handler); - acpi_remove_address_space_handler(device->handle, + acpi_remove_address_space_handler(acpi_device->handle, ACPI_ADR_SPACE_EC, _wmi_ec_space_handler); - wmi_free_devices(device); - device_unregister((struct device *)acpi_driver_data(device)); - device->driver_data = NULL; + wmi_free_devices(acpi_device); + device_unregister((struct device *)dev_get_drvdata(>dev)); return 0; } -static int acpi_wmi_add(struct acpi_device *device) +static int acpi_wmi_probe(struct platform_device *device) { + struct acpi_device *acpi_device; struct device *wmi_bus_dev; acpi_status status; int error; - status = acpi_install_address_space_handler(device->handle, + acpi_device = ACPI_COMPANION(>dev); + if (!acpi_device) { + dev_err(>dev, "ACPI companion is missing\n"); + return -ENODEV; + } + + status = acpi_install_address_space_handler(acpi_device->handle, ACPI_ADR_SPACE_EC, _wmi_ec_space_handler, NULL, NULL); @@ -1141,7 +1149,8 @@ static int acpi_wmi_add(struct acpi_device *device) return -ENODEV; } - status = acpi_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, + status = acpi_install_notify_handler(acpi_device->handle, +ACPI_DEVICE_NOTIFY, acpi_wmi_notify_handler, NULL); if (ACPI_FAILURE(status)) { @@ -1156,9 +1165,9 @@ static int acpi_wmi_add(struct acpi_device *device) error = PTR_ERR(wmi_bus_dev); goto err_remove_notify_handler; } - device->driver_data = wmi_bus_dev; + dev_set_drvdata(>dev, wmi_bus_dev); - error = parse_wdg(wmi_bus_dev, device); + error = parse_wdg(wmi_bus_dev, acpi_device); if (error) { pr_err("Failed to parse WDG method\n"); goto err_remove_busdev; @@ -1170,11 +1179,11 @@ static int acpi_wmi_add(struct acpi_device *device) device_unregister(wmi_bus_dev); err_remove_notify_handler: - acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, + acpi_remove_notify_handler(acpi_device->handle, ACPI_DEVICE_NOTIFY, acpi_wmi_notify_handler); err_remove_ec_handler: -
[PATCH 13/16] platform/x86: wmi: Add an interface for subdrivers to access sibling devices
From: Andy LutomirskiSome subdrivers need to access sibling devices. This gives them a clean way to do so. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 17 + include/linux/wmi.h| 4 2 files changed, 21 insertions(+) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index f1c9464..8dd9988 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -344,6 +344,23 @@ 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 0ab2540..a283768 100644 --- a/include/linux/wmi.h +++ b/include/linux/wmi.h @@ -33,6 +33,10 @@ struct wmi_device { 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.9.4
[PATCH 13/16] platform/x86: wmi: Add an interface for subdrivers to access sibling devices
From: Andy Lutomirski Some subdrivers need to access sibling devices. This gives them a clean way to do so. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 17 + include/linux/wmi.h| 4 2 files changed, 21 insertions(+) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index f1c9464..8dd9988 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -344,6 +344,23 @@ 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 0ab2540..a283768 100644 --- a/include/linux/wmi.h +++ b/include/linux/wmi.h @@ -33,6 +33,10 @@ struct wmi_device { 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.9.4
[PATCH 14/16] platform/x86: wmi: Require query for data blocks, rename writable to setable
From: "Darren Hart (VMware)"The Microsoft WMI documentation requires all data blocks to implement the Query Control Method (WQxx). If we encounter a data block not implementing this control method, issue a warning, and ignore the data block. Remove the "readable" attribute as all data blocks must be readable (query-able). Be consistent with the language in the documentation, replace the "writable" attribute with "setable". Simplify (flatten) the control flow of wmi_create_device a bit while we are updating it for the above changes. Signed-off-by: Darren Hart (VMware) Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org --- drivers/platform/x86/wmi.c | 117 +++-- include/linux/wmi.h| 7 +-- 2 files changed, 63 insertions(+), 61 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 8dd9988..cc55952 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -69,7 +69,7 @@ struct wmi_block { wmi_notify_handler handler; void *handler_data; - bool read_takes_no_args;/* only defined if readable */ + bool read_takes_no_args; }; @@ -694,28 +694,18 @@ static ssize_t object_id_show(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RO(object_id); -static ssize_t readable_show(struct device *dev, struct device_attribute *attr, -char *buf) +static ssize_t setable_show(struct device *dev, struct device_attribute *attr, + char *buf) { struct wmi_device *wdev = dev_to_wdev(dev); - return sprintf(buf, "%d\n", (int)wdev->readable); + return sprintf(buf, "%d\n", (int)wdev->setable); } -static DEVICE_ATTR_RO(readable); - -static ssize_t writeable_show(struct device *dev, struct device_attribute *attr, - char *buf) -{ - struct wmi_device *wdev = dev_to_wdev(dev); - - return sprintf(buf, "%d\n", (int)wdev->writeable); -} -static DEVICE_ATTR_RO(writeable); +static DEVICE_ATTR_RO(setable); static struct attribute *wmi_data_attrs[] = { _attr_object_id.attr, - _attr_readable.attr, - _attr_writeable.attr, + _attr_setable.attr, NULL, }; ATTRIBUTE_GROUPS(wmi_data); @@ -833,63 +823,74 @@ static struct device_type wmi_type_data = { .release = wmi_dev_release, }; -static void wmi_create_device(struct device *wmi_bus_dev, +static int wmi_create_device(struct device *wmi_bus_dev, const struct guid_block *gblock, struct wmi_block *wblock, struct acpi_device *device) { - wblock->dev.dev.bus = _bus_type; - wblock->dev.dev.parent = wmi_bus_dev; - - dev_set_name(>dev.dev, "%pUL", gblock->guid); + struct acpi_device_info *info; + char method[5]; + int result; if (gblock->flags & ACPI_WMI_EVENT) { wblock->dev.dev.type = _type_event; - } else if (gblock->flags & ACPI_WMI_METHOD) { + goto out_init; + } + + if (gblock->flags & ACPI_WMI_METHOD) { wblock->dev.dev.type = _type_method; - } else { - struct acpi_device_info *info; - char method[5]; - int result; + goto out_init; + } - wblock->dev.dev.type = _type_data; + /* +* Data Block Query Control Method (WQxx by convention) is +* required per the WMI documentation. If it is not present, +* we ignore this data block. +*/ + strcpy(method, "WQ"); + strncat(method, wblock->gblock.object_id, 2); + result = get_subobj_info(device->handle, method, ); + + if (result) { + dev_warn(wmi_bus_dev, +"%s data block query control method not found", +method); + return result; + } - strcpy(method, "WQ"); - strncat(method, wblock->gblock.object_id, 2); - result = get_subobj_info(device->handle, method, ); + wblock->dev.dev.type = _type_data; - if (result == 0) { - wblock->dev.readable = true; + /* +* The Microsoft documentation specifically states: +* +* Data blocks registered with only a single instance +* can ignore the parameter. +* +* ACPICA will get mad at us if we call the method with the wrong number +* of arguments, so check what our method expects. (On some Dell +* laptops, WQxx may not be a method at all.) +*/ + if
[PATCH 03/16] platform/x86: wmi: Clean up acpi_wmi_add
From: Andy LutomirskiRearrange acpi_wmi_add to use Linux's error handling conventions. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index c6e11b5..ac60a51 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -803,20 +803,24 @@ static int acpi_wmi_add(struct acpi_device *device) _wmi_ec_space_handler, NULL, NULL); if (ACPI_FAILURE(status)) { - pr_err("Error installing EC region handler\n"); + dev_err(>dev, "Error installing EC region handler\n"); return -ENODEV; } error = parse_wdg(device); if (error) { - acpi_remove_address_space_handler(device->handle, - ACPI_ADR_SPACE_EC, - _wmi_ec_space_handler); pr_err("Failed to parse WDG method\n"); - return error; + goto err_remove_handler; } return 0; + +err_remove_handler: + acpi_remove_address_space_handler(device->handle, + ACPI_ADR_SPACE_EC, + _wmi_ec_space_handler); + + return error; } static int __init acpi_wmi_init(void) -- 2.9.4
[PATCH 14/16] platform/x86: wmi: Require query for data blocks, rename writable to setable
From: "Darren Hart (VMware)" The Microsoft WMI documentation requires all data blocks to implement the Query Control Method (WQxx). If we encounter a data block not implementing this control method, issue a warning, and ignore the data block. Remove the "readable" attribute as all data blocks must be readable (query-able). Be consistent with the language in the documentation, replace the "writable" attribute with "setable". Simplify (flatten) the control flow of wmi_create_device a bit while we are updating it for the above changes. Signed-off-by: Darren Hart (VMware) Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org --- drivers/platform/x86/wmi.c | 117 +++-- include/linux/wmi.h| 7 +-- 2 files changed, 63 insertions(+), 61 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 8dd9988..cc55952 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -69,7 +69,7 @@ struct wmi_block { wmi_notify_handler handler; void *handler_data; - bool read_takes_no_args;/* only defined if readable */ + bool read_takes_no_args; }; @@ -694,28 +694,18 @@ static ssize_t object_id_show(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RO(object_id); -static ssize_t readable_show(struct device *dev, struct device_attribute *attr, -char *buf) +static ssize_t setable_show(struct device *dev, struct device_attribute *attr, + char *buf) { struct wmi_device *wdev = dev_to_wdev(dev); - return sprintf(buf, "%d\n", (int)wdev->readable); + return sprintf(buf, "%d\n", (int)wdev->setable); } -static DEVICE_ATTR_RO(readable); - -static ssize_t writeable_show(struct device *dev, struct device_attribute *attr, - char *buf) -{ - struct wmi_device *wdev = dev_to_wdev(dev); - - return sprintf(buf, "%d\n", (int)wdev->writeable); -} -static DEVICE_ATTR_RO(writeable); +static DEVICE_ATTR_RO(setable); static struct attribute *wmi_data_attrs[] = { _attr_object_id.attr, - _attr_readable.attr, - _attr_writeable.attr, + _attr_setable.attr, NULL, }; ATTRIBUTE_GROUPS(wmi_data); @@ -833,63 +823,74 @@ static struct device_type wmi_type_data = { .release = wmi_dev_release, }; -static void wmi_create_device(struct device *wmi_bus_dev, +static int wmi_create_device(struct device *wmi_bus_dev, const struct guid_block *gblock, struct wmi_block *wblock, struct acpi_device *device) { - wblock->dev.dev.bus = _bus_type; - wblock->dev.dev.parent = wmi_bus_dev; - - dev_set_name(>dev.dev, "%pUL", gblock->guid); + struct acpi_device_info *info; + char method[5]; + int result; if (gblock->flags & ACPI_WMI_EVENT) { wblock->dev.dev.type = _type_event; - } else if (gblock->flags & ACPI_WMI_METHOD) { + goto out_init; + } + + if (gblock->flags & ACPI_WMI_METHOD) { wblock->dev.dev.type = _type_method; - } else { - struct acpi_device_info *info; - char method[5]; - int result; + goto out_init; + } - wblock->dev.dev.type = _type_data; + /* +* Data Block Query Control Method (WQxx by convention) is +* required per the WMI documentation. If it is not present, +* we ignore this data block. +*/ + strcpy(method, "WQ"); + strncat(method, wblock->gblock.object_id, 2); + result = get_subobj_info(device->handle, method, ); + + if (result) { + dev_warn(wmi_bus_dev, +"%s data block query control method not found", +method); + return result; + } - strcpy(method, "WQ"); - strncat(method, wblock->gblock.object_id, 2); - result = get_subobj_info(device->handle, method, ); + wblock->dev.dev.type = _type_data; - if (result == 0) { - wblock->dev.readable = true; + /* +* The Microsoft documentation specifically states: +* +* Data blocks registered with only a single instance +* can ignore the parameter. +* +* ACPICA will get mad at us if we call the method with the wrong number +* of arguments, so check what our method expects. (On some Dell +* laptops, WQxx may not be a method at all.) +*/ + if (info->type != ACPI_TYPE_METHOD || info->param_count == 0) + wblock->read_takes_no_args = true; -
[PATCH 03/16] platform/x86: wmi: Clean up acpi_wmi_add
From: Andy Lutomirski Rearrange acpi_wmi_add to use Linux's error handling conventions. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index c6e11b5..ac60a51 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -803,20 +803,24 @@ static int acpi_wmi_add(struct acpi_device *device) _wmi_ec_space_handler, NULL, NULL); if (ACPI_FAILURE(status)) { - pr_err("Error installing EC region handler\n"); + dev_err(>dev, "Error installing EC region handler\n"); return -ENODEV; } error = parse_wdg(device); if (error) { - acpi_remove_address_space_handler(device->handle, - ACPI_ADR_SPACE_EC, - _wmi_ec_space_handler); pr_err("Failed to parse WDG method\n"); - return error; + goto err_remove_handler; } return 0; + +err_remove_handler: + acpi_remove_address_space_handler(device->handle, + ACPI_ADR_SPACE_EC, + _wmi_ec_space_handler); + + return error; } static int __init acpi_wmi_init(void) -- 2.9.4
[PATCH 07/16] platform/x86: wmi: Split devices into types and add basic sysfs attributes
From: Andy LutomirskiDivide the "data", "method" and "event" types. All devices get "instance_count" and "expensive" attributes, data and method devices get "object_id" attributes, and event devices get "notify_id" attributes. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 78 +- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 31c317f..33a3609 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -575,13 +575,65 @@ static ssize_t guid_show(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RO(guid); +static ssize_t instance_count_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + return sprintf(buf, "%d\n", (int)wblock->gblock.instance_count); +} +static DEVICE_ATTR_RO(instance_count); + +static ssize_t expensive_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + return sprintf(buf, "%d\n", + (wblock->gblock.flags & ACPI_WMI_EXPENSIVE) != 0); +} +static DEVICE_ATTR_RO(expensive); + static struct attribute *wmi_attrs[] = { _attr_modalias.attr, _attr_guid.attr, + _attr_instance_count.attr, + _attr_expensive.attr, NULL, }; ATTRIBUTE_GROUPS(wmi); +static ssize_t notify_id_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + return sprintf(buf, "%02X\n", (unsigned int)wblock->gblock.notify_id); +} +static DEVICE_ATTR_RO(notify_id); + +static struct attribute *wmi_event_attrs[] = { + _attr_notify_id.attr, + NULL, +}; +ATTRIBUTE_GROUPS(wmi_event); + +static ssize_t object_id_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + return sprintf(buf, "%c%c\n", wblock->gblock.object_id[0], + wblock->gblock.object_id[1]); +} +static DEVICE_ATTR_RO(object_id); + +static struct attribute *wmi_data_or_method_attrs[] = { + _attr_object_id.attr, + NULL, +}; +ATTRIBUTE_GROUPS(wmi_data_or_method); + static int wmi_dev_uevent(struct device *dev, struct kobj_uevent_env *env) { struct wmi_block *wblock = dev_to_wblock(dev); @@ -671,6 +723,24 @@ static struct bus_type wmi_bus_type = { .remove = wmi_dev_remove, }; +static struct device_type wmi_type_event = { + .name = "event", + .groups = wmi_event_groups, + .release = wmi_dev_release, +}; + +static struct device_type wmi_type_method = { + .name = "method", + .groups = wmi_data_or_method_groups, + .release = wmi_dev_release, +}; + +static struct device_type wmi_type_data = { + .name = "data", + .groups = wmi_data_or_method_groups, + .release = wmi_dev_release, +}; + static int wmi_create_device(struct device *wmi_bus_dev, const struct guid_block *gblock, struct wmi_block *wblock, @@ -681,7 +751,13 @@ static int wmi_create_device(struct device *wmi_bus_dev, dev_set_name(>dev.dev, "%pUL", gblock->guid); - wblock->dev.dev.release = wmi_dev_release; + if (gblock->flags & ACPI_WMI_EVENT) { + wblock->dev.dev.type = _type_event; + } else if (gblock->flags & ACPI_WMI_METHOD) { + wblock->dev.dev.type = _type_method; + } else { + wblock->dev.dev.type = _type_data; + } return device_register(>dev.dev); } -- 2.9.4
[PATCH 07/16] platform/x86: wmi: Split devices into types and add basic sysfs attributes
From: Andy Lutomirski Divide the "data", "method" and "event" types. All devices get "instance_count" and "expensive" attributes, data and method devices get "object_id" attributes, and event devices get "notify_id" attributes. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 78 +- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 31c317f..33a3609 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -575,13 +575,65 @@ static ssize_t guid_show(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RO(guid); +static ssize_t instance_count_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + return sprintf(buf, "%d\n", (int)wblock->gblock.instance_count); +} +static DEVICE_ATTR_RO(instance_count); + +static ssize_t expensive_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + return sprintf(buf, "%d\n", + (wblock->gblock.flags & ACPI_WMI_EXPENSIVE) != 0); +} +static DEVICE_ATTR_RO(expensive); + static struct attribute *wmi_attrs[] = { _attr_modalias.attr, _attr_guid.attr, + _attr_instance_count.attr, + _attr_expensive.attr, NULL, }; ATTRIBUTE_GROUPS(wmi); +static ssize_t notify_id_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + return sprintf(buf, "%02X\n", (unsigned int)wblock->gblock.notify_id); +} +static DEVICE_ATTR_RO(notify_id); + +static struct attribute *wmi_event_attrs[] = { + _attr_notify_id.attr, + NULL, +}; +ATTRIBUTE_GROUPS(wmi_event); + +static ssize_t object_id_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct wmi_block *wblock = dev_to_wblock(dev); + + return sprintf(buf, "%c%c\n", wblock->gblock.object_id[0], + wblock->gblock.object_id[1]); +} +static DEVICE_ATTR_RO(object_id); + +static struct attribute *wmi_data_or_method_attrs[] = { + _attr_object_id.attr, + NULL, +}; +ATTRIBUTE_GROUPS(wmi_data_or_method); + static int wmi_dev_uevent(struct device *dev, struct kobj_uevent_env *env) { struct wmi_block *wblock = dev_to_wblock(dev); @@ -671,6 +723,24 @@ static struct bus_type wmi_bus_type = { .remove = wmi_dev_remove, }; +static struct device_type wmi_type_event = { + .name = "event", + .groups = wmi_event_groups, + .release = wmi_dev_release, +}; + +static struct device_type wmi_type_method = { + .name = "method", + .groups = wmi_data_or_method_groups, + .release = wmi_dev_release, +}; + +static struct device_type wmi_type_data = { + .name = "data", + .groups = wmi_data_or_method_groups, + .release = wmi_dev_release, +}; + static int wmi_create_device(struct device *wmi_bus_dev, const struct guid_block *gblock, struct wmi_block *wblock, @@ -681,7 +751,13 @@ static int wmi_create_device(struct device *wmi_bus_dev, dev_set_name(>dev.dev, "%pUL", gblock->guid); - wblock->dev.dev.release = wmi_dev_release; + if (gblock->flags & ACPI_WMI_EVENT) { + wblock->dev.dev.type = _type_event; + } else if (gblock->flags & ACPI_WMI_METHOD) { + wblock->dev.dev.type = _type_method; + } else { + wblock->dev.dev.type = _type_data; + } return device_register(>dev.dev); } -- 2.9.4
[PATCH 02/16] platform/x86: wmi: Pass the acpi_device through to parse_wdg
From: Andy LutomirskiWe will need the device to convert to a bus architecture and bind WMI to the platform device. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 0043581..c6e11b5 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -606,7 +606,8 @@ static struct class wmi_class = { }; static int wmi_create_device(const struct guid_block *gblock, -struct wmi_block *wblock, acpi_handle handle) +struct wmi_block *wblock, +struct acpi_device *device) { wblock->dev.class = _class; @@ -645,7 +646,7 @@ static bool guid_already_parsed(const char *guid_string) /* * Parse the _WDG method for the GUID data blocks */ -static int parse_wdg(acpi_handle handle) +static int parse_wdg(struct acpi_device *device) { struct acpi_buffer out = {ACPI_ALLOCATE_BUFFER, NULL}; union acpi_object *obj; @@ -655,7 +656,7 @@ static int parse_wdg(acpi_handle handle) int retval; u32 i, total; - status = acpi_evaluate_object(handle, "_WDG", NULL, ); + status = acpi_evaluate_object(device->handle, "_WDG", NULL, ); if (ACPI_FAILURE(status)) return -ENXIO; @@ -679,7 +680,7 @@ static int parse_wdg(acpi_handle handle) if (!wblock) return -ENOMEM; - wblock->handle = handle; + wblock->handle = device->handle; wblock->gblock = gblock[i]; /* @@ -689,7 +690,7 @@ static int parse_wdg(acpi_handle handle) for device creation. */ if (!guid_already_parsed(gblock[i].guid)) { - retval = wmi_create_device([i], wblock, handle); + retval = wmi_create_device([i], wblock, device); if (retval) { wmi_free_devices(); goto out_free_pointer; @@ -806,7 +807,7 @@ static int acpi_wmi_add(struct acpi_device *device) return -ENODEV; } - error = parse_wdg(device->handle); + error = parse_wdg(device); if (error) { acpi_remove_address_space_handler(device->handle, ACPI_ADR_SPACE_EC, -- 2.9.4
[PATCH 02/16] platform/x86: wmi: Pass the acpi_device through to parse_wdg
From: Andy Lutomirski We will need the device to convert to a bus architecture and bind WMI to the platform device. Signed-off-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Mario Limonciello Cc: Pali Rohár Cc: Rafael Wysocki Cc: linux-kernel@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-a...@vger.kernel.org Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 0043581..c6e11b5 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -606,7 +606,8 @@ static struct class wmi_class = { }; static int wmi_create_device(const struct guid_block *gblock, -struct wmi_block *wblock, acpi_handle handle) +struct wmi_block *wblock, +struct acpi_device *device) { wblock->dev.class = _class; @@ -645,7 +646,7 @@ static bool guid_already_parsed(const char *guid_string) /* * Parse the _WDG method for the GUID data blocks */ -static int parse_wdg(acpi_handle handle) +static int parse_wdg(struct acpi_device *device) { struct acpi_buffer out = {ACPI_ALLOCATE_BUFFER, NULL}; union acpi_object *obj; @@ -655,7 +656,7 @@ static int parse_wdg(acpi_handle handle) int retval; u32 i, total; - status = acpi_evaluate_object(handle, "_WDG", NULL, ); + status = acpi_evaluate_object(device->handle, "_WDG", NULL, ); if (ACPI_FAILURE(status)) return -ENXIO; @@ -679,7 +680,7 @@ static int parse_wdg(acpi_handle handle) if (!wblock) return -ENOMEM; - wblock->handle = handle; + wblock->handle = device->handle; wblock->gblock = gblock[i]; /* @@ -689,7 +690,7 @@ static int parse_wdg(acpi_handle handle) for device creation. */ if (!guid_already_parsed(gblock[i].guid)) { - retval = wmi_create_device([i], wblock, handle); + retval = wmi_create_device([i], wblock, device); if (retval) { wmi_free_devices(); goto out_free_pointer; @@ -806,7 +807,7 @@ static int acpi_wmi_add(struct acpi_device *device) return -ENODEV; } - error = parse_wdg(device->handle); + error = parse_wdg(device); if (error) { acpi_remove_address_space_handler(device->handle, ACPI_ADR_SPACE_EC, -- 2.9.4
[PATCH] platform/x86: dell-wmi: Add a comment explaining the 0xb2 magic number
From: Andy LutomirskiThe hotkey table is 0xb2, add a comment for clarity. Suggested-by: Darren Hart Signed-off-by: Andy Lutomirski Cc: Matthew Garrett Cc: "Pali Rohár" Cc: Andy Shevchenko Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/dell-wmi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 8a64c79..24467b1 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -449,6 +449,7 @@ static void __init handle_dmi_entry(const struct dmi_header *dm, if (results->err || results->keymap) return; /* We already found the hotkey table. */ + /* The Dell hotkey table is type 0xB2. Scan until we find it. */ if (dm->type != 0xb2) return; -- 2.9.4
[PATCH] platform/x86: dell-wmi: Add a comment explaining the 0xb2 magic number
From: Andy Lutomirski The hotkey table is 0xb2, add a comment for clarity. Suggested-by: Darren Hart Signed-off-by: Andy Lutomirski Cc: Matthew Garrett Cc: "Pali Rohár" Cc: Andy Shevchenko Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/dell-wmi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 8a64c79..24467b1 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -449,6 +449,7 @@ static void __init handle_dmi_entry(const struct dmi_header *dm, if (results->err || results->keymap) return; /* We already found the hotkey table. */ + /* The Dell hotkey table is type 0xB2. Scan until we find it. */ if (dm->type != 0xb2) return; -- 2.9.4
[PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6
From: Andy LutomirskiAccording to Mario at Dell, the DELLABC6 device should not be used on a Linux system. It also conflicts with Intel-HID and its interactions with Network Manager. Document that we are aware of the device, but that we are intentionally ignoring it. Signed-off-by: Andy Lutomirski [dvhart: New commit message and minor comment wording fixes] Cc: Mario Limonciello Cc: "Pali Rohár" Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/dell-rbtn.c | 26 +++--- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/drivers/platform/x86/dell-rbtn.c b/drivers/platform/x86/dell-rbtn.c index dcd9f40..2eeef03 100644 --- a/drivers/platform/x86/dell-rbtn.c +++ b/drivers/platform/x86/dell-rbtn.c @@ -223,14 +223,26 @@ static const struct acpi_device_id rbtn_ids[] = { * This driver can also handle the "DELLABC6" device that * appears on the XPS 13 9350, but that device is disabled * by the DSDT unless booted with acpi_osi="!Windows 2012" -* acpi_osi="!Windows 2013". Even if we boot that and bind -* the driver, we seem to have inconsistent behavior in -* which NetworkManager can get out of sync with the rfkill -* state. +* acpi_osi="!Windows 2013". * -* On the XPS 13 9350 and similar laptops, we're not supposed to -* use DELLABC6 at all. Instead, we handle the rfkill button -* via the intel-hid driver. +* According to Mario at Dell: +* +* DELLABC6 is a custom interface that was created solely to +* have airplane mode support for Windows 7. For Windows 10 +* the proper interface is to use that which is handled by +* intel-hid. A OEM airplane mode driver is not used. +* +* Since the kernel doesn't identify as Windows 7 it would be +* incorrect to do attempt to use that interface. +* +* Even if we override _OSI and bind to DELLABC6, we end up +* with inconsistent behavior in which NetworkManager can get +* out of sync with the rfkill state. This happens because +* NetworkManager receives events from intel-hid and fights with +* dell-rbtn for control. +* +* The upshot is that it's better to just ignore DELLABC6 +* devices. */ { "", 0 }, -- 2.9.4
[PATCH] platform/x86: dell-rbtn: Improve explanation about DELLABC6
From: Andy Lutomirski According to Mario at Dell, the DELLABC6 device should not be used on a Linux system. It also conflicts with Intel-HID and its interactions with Network Manager. Document that we are aware of the device, but that we are intentionally ignoring it. Signed-off-by: Andy Lutomirski [dvhart: New commit message and minor comment wording fixes] Cc: Mario Limonciello Cc: "Pali Rohár" Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/dell-rbtn.c | 26 +++--- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/drivers/platform/x86/dell-rbtn.c b/drivers/platform/x86/dell-rbtn.c index dcd9f40..2eeef03 100644 --- a/drivers/platform/x86/dell-rbtn.c +++ b/drivers/platform/x86/dell-rbtn.c @@ -223,14 +223,26 @@ static const struct acpi_device_id rbtn_ids[] = { * This driver can also handle the "DELLABC6" device that * appears on the XPS 13 9350, but that device is disabled * by the DSDT unless booted with acpi_osi="!Windows 2012" -* acpi_osi="!Windows 2013". Even if we boot that and bind -* the driver, we seem to have inconsistent behavior in -* which NetworkManager can get out of sync with the rfkill -* state. +* acpi_osi="!Windows 2013". * -* On the XPS 13 9350 and similar laptops, we're not supposed to -* use DELLABC6 at all. Instead, we handle the rfkill button -* via the intel-hid driver. +* According to Mario at Dell: +* +* DELLABC6 is a custom interface that was created solely to +* have airplane mode support for Windows 7. For Windows 10 +* the proper interface is to use that which is handled by +* intel-hid. A OEM airplane mode driver is not used. +* +* Since the kernel doesn't identify as Windows 7 it would be +* incorrect to do attempt to use that interface. +* +* Even if we override _OSI and bind to DELLABC6, we end up +* with inconsistent behavior in which NetworkManager can get +* out of sync with the rfkill state. This happens because +* NetworkManager receives events from intel-hid and fights with +* dell-rbtn for control. +* +* The upshot is that it's better to just ignore DELLABC6 +* devices. */ { "", 0 }, -- 2.9.4
[PATCH] platform/x86: dell-wmi: Add a better description for "stealth mode"
From: Andy LutomirskiThis is based on Mario's explanation and observation of my laptop. Suggested-by: "Pali Rohár" Signed-off-by: Andy Lutomirski Cc: Mario Limonciello Cc: Matthew Garrett Cc: Andy Shevchenko Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/dell-wmi.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 24467b1..73aee0d 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -272,7 +272,12 @@ static const struct key_entry dell_wmi_keymap_type_0010[] __initconst = { /* RGB keyboard backlight control */ { KE_IGNORE, 0x154, { KEY_RESERVED } }, - /* Stealth mode toggle */ + /* +* Stealth mode toggle. This will "disable all lights and sounds". +* The action is performed by the BIOS and EC; the WMI event is just +* a notification. On the XPS 13 9350, this is Fn+F7, and there's +* a BIOS setting to enable and disable the hotkey. +*/ { KE_IGNORE, 0x155, { KEY_RESERVED } }, /* Rugged magnetic dock attach/detach events */ -- 2.9.4
[PATCH] platform/x86: dell-wmi: Add a better description for "stealth mode"
From: Andy Lutomirski This is based on Mario's explanation and observation of my laptop. Suggested-by: "Pali Rohár" Signed-off-by: Andy Lutomirski Cc: Mario Limonciello Cc: Matthew Garrett Cc: Andy Shevchenko Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/dell-wmi.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 24467b1..73aee0d 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -272,7 +272,12 @@ static const struct key_entry dell_wmi_keymap_type_0010[] __initconst = { /* RGB keyboard backlight control */ { KE_IGNORE, 0x154, { KEY_RESERVED } }, - /* Stealth mode toggle */ + /* +* Stealth mode toggle. This will "disable all lights and sounds". +* The action is performed by the BIOS and EC; the WMI event is just +* a notification. On the XPS 13 9350, this is Fn+F7, and there's +* a BIOS setting to enable and disable the hotkey. +*/ { KE_IGNORE, 0x155, { KEY_RESERVED } }, /* Rugged magnetic dock attach/detach events */ -- 2.9.4
[PATCH] staging/lustre/lov: remove set_fs() call from lov_getstripe()
lov_getstripe() calls set_fs(KERNEL_DS) so that it can handle a struct lov_user_md pointer from user- or kernel-space. This changes the behavior of copy_from_user() on SPARC and may result in a misaligned access exception which in turn oopses the kernel. In fact the relevant argument to lov_getstripe() is never called with a kernel-space pointer and so changing the address limits is unnecessary and so we remove the calls to save, set, and restore the address limits. Signed-off-by: John L. HammondReviewed-on: http://review.whamcloud.com/6150 Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3221 Reviewed-by: Andreas Dilger Reviewed-by: Li Wei Signed-off-by: Oleg Drokin --- drivers/staging/lustre/lustre/lov/lov_pack.c | 9 - 1 file changed, 9 deletions(-) diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c index 2e1bd47..e6727ce 100644 --- a/drivers/staging/lustre/lustre/lov/lov_pack.c +++ b/drivers/staging/lustre/lustre/lov/lov_pack.c @@ -293,18 +293,10 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm, size_t lmmk_size; size_t lum_size; int rc; - mm_segment_t seg; if (!lsm) return -ENODATA; - /* -* "Switch to kernel segment" to allow copying from kernel space by -* copy_{to,from}_user(). -*/ - seg = get_fs(); - set_fs(KERNEL_DS); - if (lsm->lsm_magic != LOV_MAGIC_V1 && lsm->lsm_magic != LOV_MAGIC_V3) { CERROR("bad LSM MAGIC: 0x%08X != 0x%08X nor 0x%08X\n", lsm->lsm_magic, LOV_MAGIC_V1, LOV_MAGIC_V3); @@ -406,6 +398,5 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm, out_free: kvfree(lmmk); out: - set_fs(seg); return rc; } -- 2.9.3
[PATCH] staging/lustre/lov: remove set_fs() call from lov_getstripe()
lov_getstripe() calls set_fs(KERNEL_DS) so that it can handle a struct lov_user_md pointer from user- or kernel-space. This changes the behavior of copy_from_user() on SPARC and may result in a misaligned access exception which in turn oopses the kernel. In fact the relevant argument to lov_getstripe() is never called with a kernel-space pointer and so changing the address limits is unnecessary and so we remove the calls to save, set, and restore the address limits. Signed-off-by: John L. Hammond Reviewed-on: http://review.whamcloud.com/6150 Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3221 Reviewed-by: Andreas Dilger Reviewed-by: Li Wei Signed-off-by: Oleg Drokin --- drivers/staging/lustre/lustre/lov/lov_pack.c | 9 - 1 file changed, 9 deletions(-) diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c index 2e1bd47..e6727ce 100644 --- a/drivers/staging/lustre/lustre/lov/lov_pack.c +++ b/drivers/staging/lustre/lustre/lov/lov_pack.c @@ -293,18 +293,10 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm, size_t lmmk_size; size_t lum_size; int rc; - mm_segment_t seg; if (!lsm) return -ENODATA; - /* -* "Switch to kernel segment" to allow copying from kernel space by -* copy_{to,from}_user(). -*/ - seg = get_fs(); - set_fs(KERNEL_DS); - if (lsm->lsm_magic != LOV_MAGIC_V1 && lsm->lsm_magic != LOV_MAGIC_V3) { CERROR("bad LSM MAGIC: 0x%08X != 0x%08X nor 0x%08X\n", lsm->lsm_magic, LOV_MAGIC_V1, LOV_MAGIC_V3); @@ -406,6 +398,5 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm, out_free: kvfree(lmmk); out: - set_fs(seg); return rc; } -- 2.9.3
Re: [PATCH] genirq: Check irq disabled & masked states in irq_shutdown
Hi Thomas, On 05/26/2017 09:20 PM, Thomas Gleixner wrote: On Fri, 26 May 2017, Jeffy Chen wrote: If irq is already disabled and masked, we would hit a unbalanced irq shutdown/disable/mask when freeing it. Errr? What exactly is unbalanced? None of the called functions has any counter or whatever. Can you please explain what you are trying to fix? sorry, i'll try to rewrite the commit message. for example when a driver(drivers/net/wireless/marvell/mwifiex/main.c) try to do these: devm_request_irq->irq_startup->irq_enable disable_irq <-- disabled and masked devm_free_irq->irq_shutdown <-- disable it again and the pinctrl-rockchip driver would enable/disable gpio clk in irq_enable/irq_disable, so it would try to disable a disabled clk(due to unbalanced irq disable) Thanks, tglx
[PATCH v2] genirq: Check irq disabled & masked states in irq_shutdown
If a irq is already disabled & masked, free_irq may cause a unbalanced irq shutdown/disable/mask, for example: devm_request_irq->irq_startup->irq_enable disable_irq <-- disabled and masked devm_free_irq->irq_shutdown <-- try to disable it again This would confuse some pinctrl drivers which would control clk in irq_enable/irq_disable, for example pinctrl-rockchip/pinctrl-nomadik. This patch add a state check in irq_shutdown to prevent that. v2: Rewrite commit message. Signed-off-by: Jeffy Chen--- Changes in v2: Rewrite commit message. kernel/irq/chip.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c index 686be4b..816da03 100644 --- a/kernel/irq/chip.c +++ b/kernel/irq/chip.c @@ -206,14 +206,20 @@ int irq_startup(struct irq_desc *desc, bool resend) void irq_shutdown(struct irq_desc *desc) { - irq_state_set_disabled(desc); desc->depth = 1; + + if (unlikely(irqd_irq_disabled(>irq_data) && + irqd_irq_masked(>irq_data))) + goto out; + + irq_state_set_disabled(desc); if (desc->irq_data.chip->irq_shutdown) desc->irq_data.chip->irq_shutdown(>irq_data); else if (desc->irq_data.chip->irq_disable) desc->irq_data.chip->irq_disable(>irq_data); else desc->irq_data.chip->irq_mask(>irq_data); +out: irq_domain_deactivate_irq(>irq_data); irq_state_set_masked(desc); } -- 2.1.4
Re: [PATCH] genirq: Check irq disabled & masked states in irq_shutdown
Hi Thomas, On 05/26/2017 09:20 PM, Thomas Gleixner wrote: On Fri, 26 May 2017, Jeffy Chen wrote: If irq is already disabled and masked, we would hit a unbalanced irq shutdown/disable/mask when freeing it. Errr? What exactly is unbalanced? None of the called functions has any counter or whatever. Can you please explain what you are trying to fix? sorry, i'll try to rewrite the commit message. for example when a driver(drivers/net/wireless/marvell/mwifiex/main.c) try to do these: devm_request_irq->irq_startup->irq_enable disable_irq <-- disabled and masked devm_free_irq->irq_shutdown <-- disable it again and the pinctrl-rockchip driver would enable/disable gpio clk in irq_enable/irq_disable, so it would try to disable a disabled clk(due to unbalanced irq disable) Thanks, tglx
[PATCH v2] genirq: Check irq disabled & masked states in irq_shutdown
If a irq is already disabled & masked, free_irq may cause a unbalanced irq shutdown/disable/mask, for example: devm_request_irq->irq_startup->irq_enable disable_irq <-- disabled and masked devm_free_irq->irq_shutdown <-- try to disable it again This would confuse some pinctrl drivers which would control clk in irq_enable/irq_disable, for example pinctrl-rockchip/pinctrl-nomadik. This patch add a state check in irq_shutdown to prevent that. v2: Rewrite commit message. Signed-off-by: Jeffy Chen --- Changes in v2: Rewrite commit message. kernel/irq/chip.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c index 686be4b..816da03 100644 --- a/kernel/irq/chip.c +++ b/kernel/irq/chip.c @@ -206,14 +206,20 @@ int irq_startup(struct irq_desc *desc, bool resend) void irq_shutdown(struct irq_desc *desc) { - irq_state_set_disabled(desc); desc->depth = 1; + + if (unlikely(irqd_irq_disabled(>irq_data) && + irqd_irq_masked(>irq_data))) + goto out; + + irq_state_set_disabled(desc); if (desc->irq_data.chip->irq_shutdown) desc->irq_data.chip->irq_shutdown(>irq_data); else if (desc->irq_data.chip->irq_disable) desc->irq_data.chip->irq_disable(>irq_data); else desc->irq_data.chip->irq_mask(>irq_data); +out: irq_domain_deactivate_irq(>irq_data); irq_state_set_masked(desc); } -- 2.1.4
[PATCH] dsa: mv88e6xxx: fix returnvar.cocci warnings
Remove unneeded variable used to store return value. Generated by: scripts/coccinelle/misc/returnvar.cocci CC: Andrew LunnSigned-off-by: Julia Lawall Signed-off-by: Fengguang Wu --- It's a minor issue, but since there is no error, the code is a bit misleading. tree: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master head: 47936d35edbac5e58064bd15e51136050b2f2717 commit: 04aca9938255fc7097b3fb5700f408524656f2e2 [330/362] dsa: mv88e6xxx: Enable/Disable SERDES on port enable/disable chip.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1982,13 +1982,12 @@ static int mv88e6xxx_port_enable(struct struct phy_device *phydev) { struct mv88e6xxx_chip *chip = ds->priv; - int err = 0; mutex_lock(>reg_lock); mv88e6xxx_serdes_power(chip, port, true); mutex_unlock(>reg_lock); - return err; + return 0; } static void mv88e6xxx_port_disable(struct dsa_switch *ds, int port,
[PATCH] dsa: mv88e6xxx: fix returnvar.cocci warnings
Remove unneeded variable used to store return value. Generated by: scripts/coccinelle/misc/returnvar.cocci CC: Andrew Lunn Signed-off-by: Julia Lawall Signed-off-by: Fengguang Wu --- It's a minor issue, but since there is no error, the code is a bit misleading. tree: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master head: 47936d35edbac5e58064bd15e51136050b2f2717 commit: 04aca9938255fc7097b3fb5700f408524656f2e2 [330/362] dsa: mv88e6xxx: Enable/Disable SERDES on port enable/disable chip.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1982,13 +1982,12 @@ static int mv88e6xxx_port_enable(struct struct phy_device *phydev) { struct mv88e6xxx_chip *chip = ds->priv; - int err = 0; mutex_lock(>reg_lock); mv88e6xxx_serdes_power(chip, port, true); mutex_unlock(>reg_lock); - return err; + return 0; } static void mv88e6xxx_port_disable(struct dsa_switch *ds, int port,
Re: [PATCH 5/5] Add a sysrq option to exit secure boot mode
Hi, On Wed, May 24, 2017 at 03:46:03PM +0100, David Howells wrote: > From: Kyle McMartin> > Make sysrq+x exit secure boot mode on x86_64, thereby allowing the running > kernel image to be modified. This lifts the lockdown. > > Signed-off-by: Kyle McMartin > Signed-off-by: David Howells > cc: x...@kernel.org > --- > > arch/x86/include/asm/efi.h|2 ++ > drivers/firmware/efi/Kconfig | 10 ++ > drivers/firmware/efi/secureboot.c | 37 > + > drivers/input/misc/uinput.c |1 + > drivers/tty/sysrq.c | 19 +-- > include/linux/input.h |5 + > include/linux/sysrq.h |8 +++- > kernel/debug/kdb/kdb_main.c |2 +- > 8 files changed, 76 insertions(+), 8 deletions(-) > [...snip] > diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c > index 3ffc1ce29023..8b766dbad6dd 100644 > --- a/drivers/tty/sysrq.c > +++ b/drivers/tty/sysrq.c > @@ -481,6 +481,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = { > /* x: May be registered on mips for TLB dump */ > /* x: May be registered on ppc/powerpc for xmon */ > /* x: May be registered on sparc64 for global PMU dump */ > + /* x: May be registered on x86_64 for disabling secure boot */ > NULL, /* x */ I suggest that add this key to the "What are the 'command' keys?" session in Documentation/admin-guide/sysrq.rst. > /* y: May be registered on sparc64 for global register dump */ > NULL, /* y */ > @@ -524,7 +525,7 @@ static void __sysrq_put_key_op(int key, struct > sysrq_key_op *op_p) > sysrq_key_table[i] = op_p; > } > > -void __handle_sysrq(int key, bool check_mask) > +void __handle_sysrq(int key, unsigned int from) > { > struct sysrq_key_op *op_p; > int orig_log_level; > @@ -544,11 +545,15 @@ void __handle_sysrq(int key, bool check_mask) > > op_p = __sysrq_get_key_op(key); > if (op_p) { > + /* Ban synthetic events from some sysrq functionality */ > + if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) && > + op_p->enable_mask & SYSRQ_DISABLE_USERSPACE) > + printk("This sysrq operation is disabled from > userspace.\n"); > /* >* Should we check for enabled operations (/proc/sysrq-trigger >* should not) and is the invoked operation enabled? >*/ > - if (!check_mask || sysrq_on_mask(op_p->enable_mask)) { > + if (from == SYSRQ_FROM_KERNEL || > sysrq_on_mask(op_p->enable_mask)) { > pr_cont("%s\n", op_p->action_msg); > console_loglevel = orig_log_level; > op_p->handler(key); Looking at sysrq_on_mask(): static bool sysrq_on_mask(int mask) { return sysrq_always_enabled || sysrq_enabled == 1 || (sysrq_enabled & mask); } The SYSRQ_DISABLE_USERSPACE can be ignored by sysrq_always_enabled or sysrq_enabled (the default value is CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE=0x1). Kernel should checks the locked down flag here when secure boot ON. Will we have another lock down patch against this? Or I missed something? Thanks a lot! Joey Lee
Re: [PATCH 5/5] Add a sysrq option to exit secure boot mode
Hi, On Wed, May 24, 2017 at 03:46:03PM +0100, David Howells wrote: > From: Kyle McMartin > > Make sysrq+x exit secure boot mode on x86_64, thereby allowing the running > kernel image to be modified. This lifts the lockdown. > > Signed-off-by: Kyle McMartin > Signed-off-by: David Howells > cc: x...@kernel.org > --- > > arch/x86/include/asm/efi.h|2 ++ > drivers/firmware/efi/Kconfig | 10 ++ > drivers/firmware/efi/secureboot.c | 37 > + > drivers/input/misc/uinput.c |1 + > drivers/tty/sysrq.c | 19 +-- > include/linux/input.h |5 + > include/linux/sysrq.h |8 +++- > kernel/debug/kdb/kdb_main.c |2 +- > 8 files changed, 76 insertions(+), 8 deletions(-) > [...snip] > diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c > index 3ffc1ce29023..8b766dbad6dd 100644 > --- a/drivers/tty/sysrq.c > +++ b/drivers/tty/sysrq.c > @@ -481,6 +481,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = { > /* x: May be registered on mips for TLB dump */ > /* x: May be registered on ppc/powerpc for xmon */ > /* x: May be registered on sparc64 for global PMU dump */ > + /* x: May be registered on x86_64 for disabling secure boot */ > NULL, /* x */ I suggest that add this key to the "What are the 'command' keys?" session in Documentation/admin-guide/sysrq.rst. > /* y: May be registered on sparc64 for global register dump */ > NULL, /* y */ > @@ -524,7 +525,7 @@ static void __sysrq_put_key_op(int key, struct > sysrq_key_op *op_p) > sysrq_key_table[i] = op_p; > } > > -void __handle_sysrq(int key, bool check_mask) > +void __handle_sysrq(int key, unsigned int from) > { > struct sysrq_key_op *op_p; > int orig_log_level; > @@ -544,11 +545,15 @@ void __handle_sysrq(int key, bool check_mask) > > op_p = __sysrq_get_key_op(key); > if (op_p) { > + /* Ban synthetic events from some sysrq functionality */ > + if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) && > + op_p->enable_mask & SYSRQ_DISABLE_USERSPACE) > + printk("This sysrq operation is disabled from > userspace.\n"); > /* >* Should we check for enabled operations (/proc/sysrq-trigger >* should not) and is the invoked operation enabled? >*/ > - if (!check_mask || sysrq_on_mask(op_p->enable_mask)) { > + if (from == SYSRQ_FROM_KERNEL || > sysrq_on_mask(op_p->enable_mask)) { > pr_cont("%s\n", op_p->action_msg); > console_loglevel = orig_log_level; > op_p->handler(key); Looking at sysrq_on_mask(): static bool sysrq_on_mask(int mask) { return sysrq_always_enabled || sysrq_enabled == 1 || (sysrq_enabled & mask); } The SYSRQ_DISABLE_USERSPACE can be ignored by sysrq_always_enabled or sysrq_enabled (the default value is CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE=0x1). Kernel should checks the locked down flag here when secure boot ON. Will we have another lock down patch against this? Or I missed something? Thanks a lot! Joey Lee
Re: [RFC][PATCH] time: Add warning about imminent deprecation of CONFIG_GENERIC_TIME_VSYSCALL_OLD
On Thu, May 25, 2017 at 5:03 AM, Paul Mackerraswrote: > On Mon, May 22, 2017 at 12:06:04PM -0700, John Stultz wrote: >> >> Basically long ago, timekeeping was handled (roughly) like: >> >> clock_gettime(): >> now = tk->clock->read() >> offset_ns = ((now - tk->cycle_last) * tk->clock->mult) >> >> tk->clock->shift; >> return timespec_add_ns(tk->xtime, offset_ns); >> >> But since for error handling use sub-ns precision, combined with that >> for update performance, we accumulate in fixed intervals, there are >> situations where in the update, we could accumulate half of a >> nanosecond into the base tk->xtime value and leaving half of a >> nanosecond in the offset. This caused the split nanosecond to be >> truncated out by the math, causing 1ns discontinuities. >> >> So to address this, we came up with sort of a hack, which when we >> accumulate rounds up that partial nanosecond, and adds the amount we >> rounded up to the error (which will cause the freq correction code to >> slow the clock down slightly). This is the code that is now done in >> the old_vsyscall_fixup() logic. >> >> Unfortunately this fix (which generates up to a nanosecond of error >> per tick) then made the freq correction code do more work and made it >> more difficult to have a stable clock. >> >> So we went for a more proper fix, which was to properly handle the >> sub-nanosecond portion of the timekeeping throughout the logic, doing >> the truncation last. >> >> clock_gettime(): >> now = tk->clock->read() >> ret.tv_sec = tk->xtime_sec; >> offset_sns = (now - tk->cycle_last) * tk->clock->mult; >> ret.tv_nsec = (offset_sns + tk->tkr_mono.xtime_nsec) >> tk->clock->shift; >> return ret; >> >> So in the above, we now use the tk->tkr_mono.xtime_nsec (which despite >> its unfortunate name, stores the accumulated shifted nanoseconds), and >> add it to the (current_cycle_delta * clock->mult), then we do the >> shift last to preserve as much precision as we can. >> >> Unfortunately we need all the reader code to do the same, which wasn't >> easy to transition in some cases. So we provided the >> CONFIG_GENERIC_TIME_VSYSCALL_OLD option to preserve the old round-up >> behavior while arch maintainers could make the transition. > > The VDSO code on PPC computes the offset in units of 2^-32 seconds, > not nanoseconds, because that makes it easy to handle the split of the > offset into whole seconds and fractional seconds (which is handled in > the generic code by the slightly icky __iter_div_u64_rem function), > and also means that we can use PPC's instruction that computes > (a * b) >> 32 to convert the fractional part to either nanoseconds or > microseconds without doing a division. > > I could pretty easily change the computations done at update_vsyscall > time to convert the tk->tkr_mono.xtime_nsec value to units of 2^-32 > seconds for use by the VDSO. That would mean we would no longer need > CONFIG_GENERIC_TIME_VSYSCALL_OLD, and would give us values returned by > the VDSO gettimeofday() and clock_gettime() that should be within > about 1/4 ns of what the generic code in the kernel would give (on > average, I mean, given that the results have at best nanosecond > resolution). Since that corresponds to about 1 CPU clock cycle, it > seems like it should be good enough. > > Alternatively I could make the VDSO computations use a smaller unit > (maybe 2^-36 or 2^-40 seconds), or else rewrite them to use exactly > the same algorithm as the generic code - which would be a bigger > change, and would mean having to do an iterative division. > > So, do you think the 1/4 ns resolution is good enough for the VDSO > computations? Sorry not to have gotten back to you yesterday on this! So yea, the above sounds reasonable to me. We only return ns resolution to userspace, so I don't believe one would be able to make a normal syscall that calculates time and then make a gettime call within a 1/4th of a nanosecond. But, I can't say I've ever really groked the ppc logic, so take my opinion with a grain of salt. :) thanks -john
Re: [RFC][PATCH] time: Add warning about imminent deprecation of CONFIG_GENERIC_TIME_VSYSCALL_OLD
On Thu, May 25, 2017 at 5:03 AM, Paul Mackerras wrote: > On Mon, May 22, 2017 at 12:06:04PM -0700, John Stultz wrote: >> >> Basically long ago, timekeeping was handled (roughly) like: >> >> clock_gettime(): >> now = tk->clock->read() >> offset_ns = ((now - tk->cycle_last) * tk->clock->mult) >> >> tk->clock->shift; >> return timespec_add_ns(tk->xtime, offset_ns); >> >> But since for error handling use sub-ns precision, combined with that >> for update performance, we accumulate in fixed intervals, there are >> situations where in the update, we could accumulate half of a >> nanosecond into the base tk->xtime value and leaving half of a >> nanosecond in the offset. This caused the split nanosecond to be >> truncated out by the math, causing 1ns discontinuities. >> >> So to address this, we came up with sort of a hack, which when we >> accumulate rounds up that partial nanosecond, and adds the amount we >> rounded up to the error (which will cause the freq correction code to >> slow the clock down slightly). This is the code that is now done in >> the old_vsyscall_fixup() logic. >> >> Unfortunately this fix (which generates up to a nanosecond of error >> per tick) then made the freq correction code do more work and made it >> more difficult to have a stable clock. >> >> So we went for a more proper fix, which was to properly handle the >> sub-nanosecond portion of the timekeeping throughout the logic, doing >> the truncation last. >> >> clock_gettime(): >> now = tk->clock->read() >> ret.tv_sec = tk->xtime_sec; >> offset_sns = (now - tk->cycle_last) * tk->clock->mult; >> ret.tv_nsec = (offset_sns + tk->tkr_mono.xtime_nsec) >> tk->clock->shift; >> return ret; >> >> So in the above, we now use the tk->tkr_mono.xtime_nsec (which despite >> its unfortunate name, stores the accumulated shifted nanoseconds), and >> add it to the (current_cycle_delta * clock->mult), then we do the >> shift last to preserve as much precision as we can. >> >> Unfortunately we need all the reader code to do the same, which wasn't >> easy to transition in some cases. So we provided the >> CONFIG_GENERIC_TIME_VSYSCALL_OLD option to preserve the old round-up >> behavior while arch maintainers could make the transition. > > The VDSO code on PPC computes the offset in units of 2^-32 seconds, > not nanoseconds, because that makes it easy to handle the split of the > offset into whole seconds and fractional seconds (which is handled in > the generic code by the slightly icky __iter_div_u64_rem function), > and also means that we can use PPC's instruction that computes > (a * b) >> 32 to convert the fractional part to either nanoseconds or > microseconds without doing a division. > > I could pretty easily change the computations done at update_vsyscall > time to convert the tk->tkr_mono.xtime_nsec value to units of 2^-32 > seconds for use by the VDSO. That would mean we would no longer need > CONFIG_GENERIC_TIME_VSYSCALL_OLD, and would give us values returned by > the VDSO gettimeofday() and clock_gettime() that should be within > about 1/4 ns of what the generic code in the kernel would give (on > average, I mean, given that the results have at best nanosecond > resolution). Since that corresponds to about 1 CPU clock cycle, it > seems like it should be good enough. > > Alternatively I could make the VDSO computations use a smaller unit > (maybe 2^-36 or 2^-40 seconds), or else rewrite them to use exactly > the same algorithm as the generic code - which would be a bigger > change, and would mean having to do an iterative division. > > So, do you think the 1/4 ns resolution is good enough for the VDSO > computations? Sorry not to have gotten back to you yesterday on this! So yea, the above sounds reasonable to me. We only return ns resolution to userspace, so I don't believe one would be able to make a normal syscall that calculates time and then make a gettime call within a 1/4th of a nanosecond. But, I can't say I've ever really groked the ppc logic, so take my opinion with a grain of salt. :) thanks -john
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 2017-05-26 at 20:20 -0700, Richard Narron wrote: > Under the /block/partitions directory the c programs have about 13 uses > of memcmp() and 6 uses of strcmp(). Nearly all of the memcmp uses with strings kernel wide use the equivalent of memcmp(foo, "bar", strlen("bar"));
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 2017-05-26 at 20:20 -0700, Richard Narron wrote: > Under the /block/partitions directory the c programs have about 13 uses > of memcmp() and 6 uses of strcmp(). Nearly all of the memcmp uses with strings kernel wide use the equivalent of memcmp(foo, "bar", strlen("bar"));
Re: [PATCH v7 04/26] x86/mpx: Do not use SIB.index if its value is 100b and ModRM.mod is not 11b
On Wed, 2017-05-24 at 15:37 +0200, Borislav Petkov wrote: > On Fri, May 05, 2017 at 11:17:02AM -0700, Ricardo Neri wrote: > > Section 2.2.1.2 of the Intel 64 and IA-32 Architectures Software > > Developer's Manual volume 2A states that when ModRM.mod !=11b and > > ModRM.rm = 100b indexed register-indirect addressing is used. In other > > words, a SIB byte follows the ModRM byte. In the specific case of > > SIB.index = 100b, the scale*index portion of the computation of the > > effective address is null. To signal callers of this particular situation, > > get_reg_offset() can return -EDOM (-EINVAL continues to indicate that an > > error when decoding the SIB byte). > > > > An example of this situation can be the following instruction: > > > >8b 4c 23 80 mov -0x80(%rbx,%riz,1),%rcx > >ModRM:0x4c [mod:1b][reg:1b][rm:100b] > >SIB: 0x23 [scale:0b][index:100b][base:11b] > >Displacement: 0x80 (1-byte, as per ModRM.mod = 1b) > > > > The %riz 'register' indicates a null index. > > > > In long mode, a REX prefix may be used. When a REX prefix is present, > > REX.X adds a fourth bit to the register selection of SIB.index. This gives > > the ability to refer to all the 16 general purpose registers. When REX.X is > > 1b and SIB.index is 100b, the index is indicated in %r12. In our example, > > this would look like: > > > >42 8b 4c 23 80mov -0x80(%rbx,%r12,1),%rcx > >REX: 0x42 [W:0b][R:0b][X:1b][B:0b] > >ModRM:0x4c [mod:1b][reg:1b][rm:100b] > >SIB: 0x23 [scale:0b][.X: 1b, index:100b][.B:0b, base:11b] > >Displacement: 0x80 (1-byte, as per ModRM.mod = 1b) > > > > Cc: Borislav Petkov> > Cc: Andy Lutomirski > > Cc: Dave Hansen > > Cc: Adam Buchbinder > > Cc: Colin Ian King > > Cc: Lorenzo Stoakes > > Cc: Qiaowei Ren > > Cc: Peter Zijlstra > > Cc: Nathan Howard > > Cc: Adan Hawthorn > > Cc: Joe Perches > > Cc: Ravi V. Shankar > > Cc: x...@kernel.org > > Signed-off-by: Ricardo Neri > > --- > > arch/x86/mm/mpx.c | 20 ++-- > > 1 file changed, 18 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c > > index ebdead8..7397b81 100644 > > --- a/arch/x86/mm/mpx.c > > +++ b/arch/x86/mm/mpx.c > > @@ -110,6 +110,14 @@ static int get_reg_offset(struct insn *insn, struct > > pt_regs *regs, > > regno = X86_SIB_INDEX(insn->sib.value); > > if (X86_REX_X(insn->rex_prefix.value)) > > regno += 8; > > <--- newline. I will add a new line here. > > > + /* > > +* If ModRM.mod !=3 and SIB.index (regno=4) the scale*index > > +* portion of the address computation is null. This is > > +* true only if REX.X is 0. In such a case, the SIB index > > +* is used in the address computation. > > +*/ > > + if (X86_MODRM_MOD(insn->modrm.value) != 3 && regno == 4) > > + return -EDOM; > > break; > > > > case REG_TYPE_BASE: > > @@ -159,11 +167,19 @@ static void __user *mpx_get_addr_ref(struct insn > > *insn, struct pt_regs *regs) > > goto out_err; > > > > indx_offset = get_reg_offset(insn, regs, > > REG_TYPE_INDEX); > > - if (indx_offset < 0) > > <--- newline. I will add a new line here. > > > + /* > > +* A negative offset generally means a error, except > >an > > > +* -EDOM, which means that the contents of the register > > +* should not be used as index. > > +*/ > > + if (indx_offset == -EDOM) > > + indx = 0; > > + else if (indx_offset < 0) > > goto out_err; > > + else > > + indx = regs_get_register(regs, indx_offset); > > > > base = regs_get_register(regs, base_offset); > > - indx = regs_get_register(regs, indx_offset); > > eff_addr = base + indx * (1 << X86_SIB_SCALE(sib)); > > } else { > > addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); > > -- > > 2.9.3 > > > > -- > Regards/Gruss, > Boris. Thanks for reviewing! BR, Ricardo > > SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB > 21284 (AG Nürnberg)
Re: [PATCH v7 04/26] x86/mpx: Do not use SIB.index if its value is 100b and ModRM.mod is not 11b
On Wed, 2017-05-24 at 15:37 +0200, Borislav Petkov wrote: > On Fri, May 05, 2017 at 11:17:02AM -0700, Ricardo Neri wrote: > > Section 2.2.1.2 of the Intel 64 and IA-32 Architectures Software > > Developer's Manual volume 2A states that when ModRM.mod !=11b and > > ModRM.rm = 100b indexed register-indirect addressing is used. In other > > words, a SIB byte follows the ModRM byte. In the specific case of > > SIB.index = 100b, the scale*index portion of the computation of the > > effective address is null. To signal callers of this particular situation, > > get_reg_offset() can return -EDOM (-EINVAL continues to indicate that an > > error when decoding the SIB byte). > > > > An example of this situation can be the following instruction: > > > >8b 4c 23 80 mov -0x80(%rbx,%riz,1),%rcx > >ModRM:0x4c [mod:1b][reg:1b][rm:100b] > >SIB: 0x23 [scale:0b][index:100b][base:11b] > >Displacement: 0x80 (1-byte, as per ModRM.mod = 1b) > > > > The %riz 'register' indicates a null index. > > > > In long mode, a REX prefix may be used. When a REX prefix is present, > > REX.X adds a fourth bit to the register selection of SIB.index. This gives > > the ability to refer to all the 16 general purpose registers. When REX.X is > > 1b and SIB.index is 100b, the index is indicated in %r12. In our example, > > this would look like: > > > >42 8b 4c 23 80mov -0x80(%rbx,%r12,1),%rcx > >REX: 0x42 [W:0b][R:0b][X:1b][B:0b] > >ModRM:0x4c [mod:1b][reg:1b][rm:100b] > >SIB: 0x23 [scale:0b][.X: 1b, index:100b][.B:0b, base:11b] > >Displacement: 0x80 (1-byte, as per ModRM.mod = 1b) > > > > Cc: Borislav Petkov > > Cc: Andy Lutomirski > > Cc: Dave Hansen > > Cc: Adam Buchbinder > > Cc: Colin Ian King > > Cc: Lorenzo Stoakes > > Cc: Qiaowei Ren > > Cc: Peter Zijlstra > > Cc: Nathan Howard > > Cc: Adan Hawthorn > > Cc: Joe Perches > > Cc: Ravi V. Shankar > > Cc: x...@kernel.org > > Signed-off-by: Ricardo Neri > > --- > > arch/x86/mm/mpx.c | 20 ++-- > > 1 file changed, 18 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c > > index ebdead8..7397b81 100644 > > --- a/arch/x86/mm/mpx.c > > +++ b/arch/x86/mm/mpx.c > > @@ -110,6 +110,14 @@ static int get_reg_offset(struct insn *insn, struct > > pt_regs *regs, > > regno = X86_SIB_INDEX(insn->sib.value); > > if (X86_REX_X(insn->rex_prefix.value)) > > regno += 8; > > <--- newline. I will add a new line here. > > > + /* > > +* If ModRM.mod !=3 and SIB.index (regno=4) the scale*index > > +* portion of the address computation is null. This is > > +* true only if REX.X is 0. In such a case, the SIB index > > +* is used in the address computation. > > +*/ > > + if (X86_MODRM_MOD(insn->modrm.value) != 3 && regno == 4) > > + return -EDOM; > > break; > > > > case REG_TYPE_BASE: > > @@ -159,11 +167,19 @@ static void __user *mpx_get_addr_ref(struct insn > > *insn, struct pt_regs *regs) > > goto out_err; > > > > indx_offset = get_reg_offset(insn, regs, > > REG_TYPE_INDEX); > > - if (indx_offset < 0) > > <--- newline. I will add a new line here. > > > + /* > > +* A negative offset generally means a error, except > >an > > > +* -EDOM, which means that the contents of the register > > +* should not be used as index. > > +*/ > > + if (indx_offset == -EDOM) > > + indx = 0; > > + else if (indx_offset < 0) > > goto out_err; > > + else > > + indx = regs_get_register(regs, indx_offset); > > > > base = regs_get_register(regs, base_offset); > > - indx = regs_get_register(regs, indx_offset); > > eff_addr = base + indx * (1 << X86_SIB_SCALE(sib)); > > } else { > > addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); > > -- > > 2.9.3 > > > > -- > Regards/Gruss, > Boris. Thanks for reviewing! BR, Ricardo > > SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB > 21284 (AG Nürnberg)
Re: [PATCH v7 00/26] x86: Enable User-Mode Instruction Prevention
Hi again Ingo, Thomas, On Wed, 2017-05-17 at 11:42 -0700, Ricardo Neri wrote: > Hi Ingo, Thomas, > > On Fri, 2017-05-05 at 11:16 -0700, Ricardo Neri wrote: > > This is v7 of this series. The six previous submissions can be found > > here [1], here [2], here[3], here[4], here[5] and here[6]. This > > version > > addresses the comments received in v6 plus improvements of the > > handling > > of exceptions unrelated to UMIP as well as corner cases in > > virtual-8086 > > mode. Please see details in the change log. > > Since there have been no more comments in the version and if this series > look good to you, could this be considered to be merged into the tip > tree? > > The only remaining item is a cleanup patch that Borislav Petkov > suggested [1]. I could work on it incrementally on top of this series. More items have accumulated from the latest review from Borislav Petkov. These items are preparatory changes and are mostly minimal and would impact functionality. There have been no comments on other parts of the implementation. If I spin a v8 of the series, would it be considered sufficiently mature to be included in v4.13? Thanks and BR, Ricardo > > Thanks and BR, > Ricardo > > [1]. https://lkml.org/lkml/2017/5/4/244 > > > > > >
Re: [PATCH v7 00/26] x86: Enable User-Mode Instruction Prevention
Hi again Ingo, Thomas, On Wed, 2017-05-17 at 11:42 -0700, Ricardo Neri wrote: > Hi Ingo, Thomas, > > On Fri, 2017-05-05 at 11:16 -0700, Ricardo Neri wrote: > > This is v7 of this series. The six previous submissions can be found > > here [1], here [2], here[3], here[4], here[5] and here[6]. This > > version > > addresses the comments received in v6 plus improvements of the > > handling > > of exceptions unrelated to UMIP as well as corner cases in > > virtual-8086 > > mode. Please see details in the change log. > > Since there have been no more comments in the version and if this series > look good to you, could this be considered to be merged into the tip > tree? > > The only remaining item is a cleanup patch that Borislav Petkov > suggested [1]. I could work on it incrementally on top of this series. More items have accumulated from the latest review from Borislav Petkov. These items are preparatory changes and are mostly minimal and would impact functionality. There have been no comments on other parts of the implementation. If I spin a v8 of the series, would it be considered sufficiently mature to be included in v4.13? Thanks and BR, Ricardo > > Thanks and BR, > Ricardo > > [1]. https://lkml.org/lkml/2017/5/4/244 > > > > > >
Re: [RFC 2/3] of: reserved_mem: Accessor for acquiring reserved_mem
On Sat, Apr 22, 2017 at 10:35:18AM -0700, Bjorn Andersson wrote: > In some cases drivers referencing a reserved-memory region might want to > remap the entire region, but when defining the reserved-memory by "size" > the client driver has no means to know the associated base address of > the reserved memory region. > > This patch adds an accessor for such drivers to acquire a handle to > their associated reserved-memory for this purpose. > > Signed-off-by: Bjorn AnderssonReviewed-by: Andy Gross
Re: [PATCH v7 02/26] x86/mm: Relocate page fault error codes to traps.h
On Sun, 2017-05-21 at 16:23 +0200, Borislav Petkov wrote: > On Fri, May 05, 2017 at 11:17:00AM -0700, Ricardo Neri wrote: > > Up to this point, only fault.c used the definitions of the page fault error > > codes. Thus, it made sense to keep them within such file. Other portions of > > code might be interested in those definitions too. For instance, the User- > > Mode Instruction Prevention emulation code will use such definitions to > > emulate a page fault when it is unable to successfully copy the results > > of the emulated instructions to user space. > > > > While relocating the error code enumeration, the prefix X86_ is used to > > make it consistent with the rest of the definitions in traps.h. Of course, > > code using the enumeration had to be updated as well. No functional changes > > were performed. > > > > Cc: Thomas Gleixner> > Cc: Ingo Molnar > > Cc: "H. Peter Anvin" > > Cc: Andy Lutomirski > > Cc: "Kirill A. Shutemov" > > Cc: Josh Poimboeuf > > Cc: Dave Hansen > > Cc: Paul Gortmaker > > Cc: x...@kernel.org > > Reviewed-by: Andy Lutomirski > > Signed-off-by: Ricardo Neri > > --- > > arch/x86/include/asm/traps.h | 18 + > > arch/x86/mm/fault.c | 88 > > +--- > > 2 files changed, 52 insertions(+), 54 deletions(-) > > ... > > > @@ -1382,7 +1362,7 @@ __do_page_fault(struct pt_regs *regs, unsigned long > > error_code, > > * space check, thus avoiding the deadlock: > > */ > > if (unlikely(!down_read_trylock(>mmap_sem))) { > > - if ((error_code & PF_USER) == 0 && > > + if ((error_code & X86_PF_USER) == 0 && > > if (!(error_code & X86_PF_USER)) This change was initially intended to only rename the error codes, without functional changes. Would making change be considered a change in functionality? The behavior would be preserved, though. Thanks and BR, Ricardo > > With that fixed: > > Reviewed-by: Borislav Petkov Thank you for your review! BR, Ricardo > > -- > Regards/Gruss, > Boris. > > SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB > 21284 (AG Nürnberg)
Re: [RFC 2/3] of: reserved_mem: Accessor for acquiring reserved_mem
On Sat, Apr 22, 2017 at 10:35:18AM -0700, Bjorn Andersson wrote: > In some cases drivers referencing a reserved-memory region might want to > remap the entire region, but when defining the reserved-memory by "size" > the client driver has no means to know the associated base address of > the reserved memory region. > > This patch adds an accessor for such drivers to acquire a handle to > their associated reserved-memory for this purpose. > > Signed-off-by: Bjorn Andersson Reviewed-by: Andy Gross
Re: [PATCH v7 02/26] x86/mm: Relocate page fault error codes to traps.h
On Sun, 2017-05-21 at 16:23 +0200, Borislav Petkov wrote: > On Fri, May 05, 2017 at 11:17:00AM -0700, Ricardo Neri wrote: > > Up to this point, only fault.c used the definitions of the page fault error > > codes. Thus, it made sense to keep them within such file. Other portions of > > code might be interested in those definitions too. For instance, the User- > > Mode Instruction Prevention emulation code will use such definitions to > > emulate a page fault when it is unable to successfully copy the results > > of the emulated instructions to user space. > > > > While relocating the error code enumeration, the prefix X86_ is used to > > make it consistent with the rest of the definitions in traps.h. Of course, > > code using the enumeration had to be updated as well. No functional changes > > were performed. > > > > Cc: Thomas Gleixner > > Cc: Ingo Molnar > > Cc: "H. Peter Anvin" > > Cc: Andy Lutomirski > > Cc: "Kirill A. Shutemov" > > Cc: Josh Poimboeuf > > Cc: Dave Hansen > > Cc: Paul Gortmaker > > Cc: x...@kernel.org > > Reviewed-by: Andy Lutomirski > > Signed-off-by: Ricardo Neri > > --- > > arch/x86/include/asm/traps.h | 18 + > > arch/x86/mm/fault.c | 88 > > +--- > > 2 files changed, 52 insertions(+), 54 deletions(-) > > ... > > > @@ -1382,7 +1362,7 @@ __do_page_fault(struct pt_regs *regs, unsigned long > > error_code, > > * space check, thus avoiding the deadlock: > > */ > > if (unlikely(!down_read_trylock(>mmap_sem))) { > > - if ((error_code & PF_USER) == 0 && > > + if ((error_code & X86_PF_USER) == 0 && > > if (!(error_code & X86_PF_USER)) This change was initially intended to only rename the error codes, without functional changes. Would making change be considered a change in functionality? The behavior would be preserved, though. Thanks and BR, Ricardo > > With that fixed: > > Reviewed-by: Borislav Petkov Thank you for your review! BR, Ricardo > > -- > Regards/Gruss, > Boris. > > SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB > 21284 (AG Nürnberg)
[RFC][PATCH 3/4] arm64: vdso: Fix nsec handling for CLOCK_MONOTONIC_RAW
From: Will DeaconCommit 45a7905fc48f ("arm64: vdso: defer shifting of nanosecond component of timespec") fixed sub-ns inaccuracies in our vDSO clock_gettime implementation by deferring the right-shift of the nanoseconds components until after the timespec addition, which operates on left-shifted values. That worked nicely until support for CLOCK_MONOTONIC_RAW was added in 49eea433b326 ("arm64: Add support for CLOCK_MONOTONIC_RAW in clock_gettime() vDSO"). Noticing that the core timekeeping code never set tkr_raw.xtime_nsec, the vDSO implementation didn't bother exposing it via the data page and instead took the unshifted tk->raw_time.tv_nsec value which was then immediately shifted left in the vDSO code. Now that the core code is actually setting tkr_raw.xtime_nsec, we need to take that into account in the vDSO by adding it to the shifted raw_time value. Rather than do that at each use (and expand the data page in the process), instead perform the shift/addition operation when populating the data page and remove the shift from the vDSO code entirely. Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Cc: Stephen Boyd Cc: Kevin Brodsky Cc: Will Deacon Cc: Daniel Mentz Reported-by: John Stultz Acked-by: Acked-by: Kevin Brodsky Signed-off-by: Will Deacon [jstultz: minor whitespace tweak] Signed-off-by: John Stultz --- arch/arm64/kernel/vdso.c | 5 +++-- arch/arm64/kernel/vdso/gettimeofday.S | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c index 41b6e31..d0cb007 100644 --- a/arch/arm64/kernel/vdso.c +++ b/arch/arm64/kernel/vdso.c @@ -221,10 +221,11 @@ void update_vsyscall(struct timekeeper *tk) /* tkr_mono.cycle_last == tkr_raw.cycle_last */ vdso_data->cs_cycle_last= tk->tkr_mono.cycle_last; vdso_data->raw_time_sec = tk->raw_time.tv_sec; - vdso_data->raw_time_nsec= tk->raw_time.tv_nsec; + vdso_data->raw_time_nsec= (tk->raw_time.tv_nsec << + tk->tkr_raw.shift) + + tk->tkr_raw.xtime_nsec; vdso_data->xtime_clock_sec = tk->xtime_sec; vdso_data->xtime_clock_nsec = tk->tkr_mono.xtime_nsec; - /* tkr_raw.xtime_nsec == 0 */ vdso_data->cs_mono_mult = tk->tkr_mono.mult; vdso_data->cs_raw_mult = tk->tkr_raw.mult; /* tkr_mono.shift == tkr_raw.shift */ diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S index e00b467..76320e9 100644 --- a/arch/arm64/kernel/vdso/gettimeofday.S +++ b/arch/arm64/kernel/vdso/gettimeofday.S @@ -256,7 +256,6 @@ monotonic_raw: seqcnt_check fail=monotonic_raw /* All computations are done with left-shifted nsecs. */ - lsl x14, x14, x12 get_nsec_per_sec res=x9 lsl x9, x9, x12 -- 2.7.4
[RFC][PATCH 3/4] arm64: vdso: Fix nsec handling for CLOCK_MONOTONIC_RAW
From: Will Deacon Commit 45a7905fc48f ("arm64: vdso: defer shifting of nanosecond component of timespec") fixed sub-ns inaccuracies in our vDSO clock_gettime implementation by deferring the right-shift of the nanoseconds components until after the timespec addition, which operates on left-shifted values. That worked nicely until support for CLOCK_MONOTONIC_RAW was added in 49eea433b326 ("arm64: Add support for CLOCK_MONOTONIC_RAW in clock_gettime() vDSO"). Noticing that the core timekeeping code never set tkr_raw.xtime_nsec, the vDSO implementation didn't bother exposing it via the data page and instead took the unshifted tk->raw_time.tv_nsec value which was then immediately shifted left in the vDSO code. Now that the core code is actually setting tkr_raw.xtime_nsec, we need to take that into account in the vDSO by adding it to the shifted raw_time value. Rather than do that at each use (and expand the data page in the process), instead perform the shift/addition operation when populating the data page and remove the shift from the vDSO code entirely. Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Cc: Stephen Boyd Cc: Kevin Brodsky Cc: Will Deacon Cc: Daniel Mentz Reported-by: John Stultz Acked-by: Acked-by: Kevin Brodsky Signed-off-by: Will Deacon [jstultz: minor whitespace tweak] Signed-off-by: John Stultz --- arch/arm64/kernel/vdso.c | 5 +++-- arch/arm64/kernel/vdso/gettimeofday.S | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c index 41b6e31..d0cb007 100644 --- a/arch/arm64/kernel/vdso.c +++ b/arch/arm64/kernel/vdso.c @@ -221,10 +221,11 @@ void update_vsyscall(struct timekeeper *tk) /* tkr_mono.cycle_last == tkr_raw.cycle_last */ vdso_data->cs_cycle_last= tk->tkr_mono.cycle_last; vdso_data->raw_time_sec = tk->raw_time.tv_sec; - vdso_data->raw_time_nsec= tk->raw_time.tv_nsec; + vdso_data->raw_time_nsec= (tk->raw_time.tv_nsec << + tk->tkr_raw.shift) + + tk->tkr_raw.xtime_nsec; vdso_data->xtime_clock_sec = tk->xtime_sec; vdso_data->xtime_clock_nsec = tk->tkr_mono.xtime_nsec; - /* tkr_raw.xtime_nsec == 0 */ vdso_data->cs_mono_mult = tk->tkr_mono.mult; vdso_data->cs_raw_mult = tk->tkr_raw.mult; /* tkr_mono.shift == tkr_raw.shift */ diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S index e00b467..76320e9 100644 --- a/arch/arm64/kernel/vdso/gettimeofday.S +++ b/arch/arm64/kernel/vdso/gettimeofday.S @@ -256,7 +256,6 @@ monotonic_raw: seqcnt_check fail=monotonic_raw /* All computations are done with left-shifted nsecs. */ - lsl x14, x14, x12 get_nsec_per_sec res=x9 lsl x9, x9, x12 -- 2.7.4
[RFC][PATCH 2/4] time: Fix CLOCK_MONOTONIC_RAW sub-nanosecond accounting
Due to how the MONOTONIC_RAW accumulation logic was handled, there is the potential for a 1ns discontinuity when we do accumulations. This small discontinuity has for the most part gone un-noticed, but since ARM64 enabled CLOCK_MONOTONIC_RAW in their vDSO clock_gettime implementation, we've seen failures with the inconsistency-check test in kselftest. This patch addresses the issue by using the same sub-ns accumulation handling that CLOCK_MONOTONIC uses, which avoids the issue for in-kernel users. Since the ARM64 vDSO implementation has its own clock_gettime calculation logic, this patch reduces the frequency of errors, but failures are still seen. The ARM64 vDSO will need to be updated to include the sub-nanosecond xtime_nsec values in its calculation for this issue to be completely fixed. Cc: Thomas GleixnerCc: Ingo Molnar Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Cc: Stephen Boyd Cc: Kevin Brodsky Cc: Will Deacon Cc: Daniel Mentz Signed-off-by: John Stultz --- include/linux/timekeeper_internal.h | 4 ++-- kernel/time/timekeeping.c | 21 - 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h index 110f453..528cc86 100644 --- a/include/linux/timekeeper_internal.h +++ b/include/linux/timekeeper_internal.h @@ -58,7 +58,7 @@ struct tk_read_base { * interval. * @xtime_remainder: Shifted nano seconds left over when rounding * @cycle_interval - * @raw_interval: Raw nano seconds accumulated per NTP interval. + * @raw_interval: Shifted raw nano seconds accumulated per NTP interval. * @ntp_error: Difference between accumulated time and NTP time in ntp * shifted nano seconds. * @ntp_error_shift: Shift conversion between clock shifted nano seconds and @@ -100,7 +100,7 @@ struct timekeeper { u64 cycle_interval; u64 xtime_interval; s64 xtime_remainder; - u32 raw_interval; + u64 raw_interval; /* The ntp_tick_length() value currently being used. * This cached copy ensures we consistently apply the tick * length for an entire tick, as ntp_tick_length may change diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index abc1968..35d3ba3 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -282,7 +282,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) /* Go back from cycles -> shifted ns */ tk->xtime_interval = interval * clock->mult; tk->xtime_remainder = ntpinterval - tk->xtime_interval; - tk->raw_interval = (interval * clock->mult) >> clock->shift; + tk->raw_interval = interval * clock->mult; /* if changing clocks, convert xtime_nsec shift units */ if (old_clock) { @@ -1994,7 +1994,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset, u32 shift, unsigned int *clock_set) { u64 interval = tk->cycle_interval << shift; - u64 raw_nsecs; + u64 nsecps; /* If the offset is smaller than a shifted interval, do nothing */ if (offset < interval) @@ -2009,14 +2009,17 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset, *clock_set |= accumulate_nsecs_to_secs(tk); /* Accumulate raw time */ - raw_nsecs = (u64)tk->raw_interval << shift; - raw_nsecs += tk->raw_time.tv_nsec; - if (raw_nsecs >= NSEC_PER_SEC) { - u64 raw_secs = raw_nsecs; - raw_nsecs = do_div(raw_secs, NSEC_PER_SEC); - tk->raw_time.tv_sec += raw_secs; + tk->tkr_raw.xtime_nsec += (u64)tk->raw_time.tv_nsec + << tk->tkr_raw.shift; + tk->tkr_raw.xtime_nsec += tk->raw_interval << shift; + nsecps = (u64)NSEC_PER_SEC << tk->tkr_raw.shift; + while (tk->tkr_raw.xtime_nsec >= nsecps) { + tk->tkr_raw.xtime_nsec -= nsecps; + tk->raw_time.tv_sec++; } - tk->raw_time.tv_nsec = raw_nsecs; + tk->raw_time.tv_nsec = tk->tkr_raw.xtime_nsec >> tk->tkr_raw.shift; + tk->tkr_raw.xtime_nsec -= (u64)tk->raw_time.tv_nsec + << tk->tkr_raw.shift; /* Accumulate error between NTP and clock interval */ tk->ntp_error += tk->ntp_tick << shift; -- 2.7.4
[RFC][PATCH 0/4] Fixes for two recently found timekeeping bugs
As part of the Linaro Linux Kernel Functional Test (LKFT) effort, test failures from kselftest/timer's inconsistency-check were reported connected to CLOCK_MONOTONIC_RAW, on the HiKey platform. Digging in I found that an old issue with how sub-ns accounting is handled with the RAW time which was fixed long ago with the CLOCK_MONOTONIC/REALTIME ids, but missed with RAW time, was present. Additionally, running further tests, I uncovered an issue with how the clocksource read function is handled when clocksources are changed, which can cause crashes. Both of these issues have not been uncovered in x86 based testing due to x86 not using vDSO to accelerate CLOCK_MONOTONIC_RAW, combined with the HiKey's arch_timer clocksource being fast to access but incrementing slowly enough to get multiple reads using the same counter value (which helps uncover time handing issues), along with the fact that none of the x86 clocksources making use of the clocksource argument passed to the read function. This patchset addresses these two issues. Thanks so much to help from Will Deacon in getting the needed adjustments to the arm64 vDSO in place. Also thanks to Daniel Mentz who also properly diagnosed the MONOTONIC_RAW issue in parallel while I was woking on this patchset. I wanted to send these out for some initial review. I'm unfortunately still chasing a third issue (related to inconsistencies triggered during extreme freq adjustments) I've uncovered on HiKey, which doesn't seem to be related to the previous two. As always feedback would be appreciated! thanks -john Cc: Thomas GleixnerCc: Ingo Molnar Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Cc: Stephen Boyd Cc: Daniel Mentz John Stultz (3): time: Fix clock->read(clock) race around clocksource changes time: Fix CLOCK_MONOTONIC_RAW sub-nanosecond accounting time: Clean up CLOCK_MONOTONIC_RAW time handling Will Deacon (1): arm64: vdso: Fix nsec handling for CLOCK_MONOTONIC_RAW arch/arm64/kernel/vdso.c | 5 +- arch/arm64/kernel/vdso/gettimeofday.S | 1 - include/linux/timekeeper_internal.h | 8 +-- kernel/time/timekeeping.c | 96 ++- 4 files changed, 66 insertions(+), 44 deletions(-) -- 2.7.4
[RFC][PATCH 1/4] time: Fix clock->read(clock) race around clocksource changes
In some testing on arm64 platforms, I was seeing null ptr crashes in the kselftest/timers clocksource-switch test. This was happening in a read function like: u64 clocksource_mmio_readl_down(struct clocksource *c) { return ~(u64)readl_relaxed(to_mmio_clksrc(c)->reg) & c->mask; } Where the callers enter the seqlock, and then call something like: cycle_now = tkr->read(tkr->clock); The problem seeming to be that since the read and clock references are happening separately, its possible the clocksource change happens in between and we end up calling the old read with the new clocksource, (or vice-versa) which causes the to_mmio_clksrc() in the read function to run off into space. This patch tries to address the issue by providing a helper function that atomically reads the clock value and then calls the clock->read(clock) call so that we always call the read funciton with the appropriate clocksource and don't accidentally mix them. The one exception where this helper isn't necessary is for the fast-timekepers which use their own locking and update logic to the tkr structures. Cc: Thomas GleixnerCc: Ingo Molnar Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Cc: Stephen Boyd Cc: Daniel Mentz Signed-off-by: John Stultz --- kernel/time/timekeeping.c | 40 +--- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 9652bc5..abc1968 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -118,6 +118,26 @@ static inline void tk_update_sleep_time(struct timekeeper *tk, ktime_t delta) tk->offs_boot = ktime_add(tk->offs_boot, delta); } +/* + * tk_clock_read - atomic clocksource read() helper + * + * This helper is necessary to use in the read paths as, while the seqlock + * ensures we don't return a bad value while structures are updated, it + * doesn't protect from potential crashes. There is the possibility that + * the tkr's clocksource may change between the read reference, and the + * clock refernce passed to the read function. This can cause crashes if + * the wrong clocksource is passed to the wrong read function. + * This isn't necessary to use when holding the timekeeper_lock or doing + * a read of the fast-timekeeper tkrs (which is protected by its own locking + * and update logic). + */ +static inline u64 tk_clock_read(struct tk_read_base *tkr) +{ + struct clocksource *clock = READ_ONCE(tkr->clock); + + return clock->read(clock); +} + #ifdef CONFIG_DEBUG_TIMEKEEPING #define WARNING_FREQ (HZ*300) /* 5 minute rate-limiting */ @@ -175,7 +195,7 @@ static inline u64 timekeeping_get_delta(struct tk_read_base *tkr) */ do { seq = read_seqcount_begin(_core.seq); - now = tkr->read(tkr->clock); + now = tk_clock_read(tkr); last = tkr->cycle_last; mask = tkr->mask; max = tkr->clock->max_cycles; @@ -209,7 +229,7 @@ static inline u64 timekeeping_get_delta(struct tk_read_base *tkr) u64 cycle_now, delta; /* read clocksource */ - cycle_now = tkr->read(tkr->clock); + cycle_now = tk_clock_read(tkr); /* calculate the delta since the last update_wall_time */ delta = clocksource_delta(cycle_now, tkr->cycle_last, tkr->mask); @@ -240,7 +260,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) tk->tkr_mono.clock = clock; tk->tkr_mono.read = clock->read; tk->tkr_mono.mask = clock->mask; - tk->tkr_mono.cycle_last = tk->tkr_mono.read(clock); + tk->tkr_mono.cycle_last = tk_clock_read(>tkr_mono); tk->tkr_raw.clock = clock; tk->tkr_raw.read = clock->read; @@ -477,7 +497,7 @@ static void halt_fast_timekeeper(struct timekeeper *tk) struct tk_read_base *tkr = >tkr_mono; memcpy(_dummy, tkr, sizeof(tkr_dummy)); - cycles_at_suspend = tkr->read(tkr->clock); + cycles_at_suspend = tk_clock_read(tkr); tkr_dummy.read = dummy_clock_read; update_fast_timekeeper(_dummy, _fast_mono); @@ -649,11 +669,10 @@ static void timekeeping_update(struct timekeeper *tk, unsigned int action) */ static void timekeeping_forward_now(struct timekeeper *tk) { - struct clocksource *clock = tk->tkr_mono.clock; u64 cycle_now, delta; u64 nsec; - cycle_now = tk->tkr_mono.read(clock); + cycle_now = tk_clock_read(>tkr_mono); delta = clocksource_delta(cycle_now, tk->tkr_mono.cycle_last, tk->tkr_mono.mask); tk->tkr_mono.cycle_last = cycle_now; tk->tkr_raw.cycle_last = cycle_now; @@ -929,8 +948,7 @@ void ktime_get_snapshot(struct system_time_snapshot
[RFC][PATCH 2/4] time: Fix CLOCK_MONOTONIC_RAW sub-nanosecond accounting
Due to how the MONOTONIC_RAW accumulation logic was handled, there is the potential for a 1ns discontinuity when we do accumulations. This small discontinuity has for the most part gone un-noticed, but since ARM64 enabled CLOCK_MONOTONIC_RAW in their vDSO clock_gettime implementation, we've seen failures with the inconsistency-check test in kselftest. This patch addresses the issue by using the same sub-ns accumulation handling that CLOCK_MONOTONIC uses, which avoids the issue for in-kernel users. Since the ARM64 vDSO implementation has its own clock_gettime calculation logic, this patch reduces the frequency of errors, but failures are still seen. The ARM64 vDSO will need to be updated to include the sub-nanosecond xtime_nsec values in its calculation for this issue to be completely fixed. Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Cc: Stephen Boyd Cc: Kevin Brodsky Cc: Will Deacon Cc: Daniel Mentz Signed-off-by: John Stultz --- include/linux/timekeeper_internal.h | 4 ++-- kernel/time/timekeeping.c | 21 - 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h index 110f453..528cc86 100644 --- a/include/linux/timekeeper_internal.h +++ b/include/linux/timekeeper_internal.h @@ -58,7 +58,7 @@ struct tk_read_base { * interval. * @xtime_remainder: Shifted nano seconds left over when rounding * @cycle_interval - * @raw_interval: Raw nano seconds accumulated per NTP interval. + * @raw_interval: Shifted raw nano seconds accumulated per NTP interval. * @ntp_error: Difference between accumulated time and NTP time in ntp * shifted nano seconds. * @ntp_error_shift: Shift conversion between clock shifted nano seconds and @@ -100,7 +100,7 @@ struct timekeeper { u64 cycle_interval; u64 xtime_interval; s64 xtime_remainder; - u32 raw_interval; + u64 raw_interval; /* The ntp_tick_length() value currently being used. * This cached copy ensures we consistently apply the tick * length for an entire tick, as ntp_tick_length may change diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index abc1968..35d3ba3 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -282,7 +282,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) /* Go back from cycles -> shifted ns */ tk->xtime_interval = interval * clock->mult; tk->xtime_remainder = ntpinterval - tk->xtime_interval; - tk->raw_interval = (interval * clock->mult) >> clock->shift; + tk->raw_interval = interval * clock->mult; /* if changing clocks, convert xtime_nsec shift units */ if (old_clock) { @@ -1994,7 +1994,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset, u32 shift, unsigned int *clock_set) { u64 interval = tk->cycle_interval << shift; - u64 raw_nsecs; + u64 nsecps; /* If the offset is smaller than a shifted interval, do nothing */ if (offset < interval) @@ -2009,14 +2009,17 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset, *clock_set |= accumulate_nsecs_to_secs(tk); /* Accumulate raw time */ - raw_nsecs = (u64)tk->raw_interval << shift; - raw_nsecs += tk->raw_time.tv_nsec; - if (raw_nsecs >= NSEC_PER_SEC) { - u64 raw_secs = raw_nsecs; - raw_nsecs = do_div(raw_secs, NSEC_PER_SEC); - tk->raw_time.tv_sec += raw_secs; + tk->tkr_raw.xtime_nsec += (u64)tk->raw_time.tv_nsec + << tk->tkr_raw.shift; + tk->tkr_raw.xtime_nsec += tk->raw_interval << shift; + nsecps = (u64)NSEC_PER_SEC << tk->tkr_raw.shift; + while (tk->tkr_raw.xtime_nsec >= nsecps) { + tk->tkr_raw.xtime_nsec -= nsecps; + tk->raw_time.tv_sec++; } - tk->raw_time.tv_nsec = raw_nsecs; + tk->raw_time.tv_nsec = tk->tkr_raw.xtime_nsec >> tk->tkr_raw.shift; + tk->tkr_raw.xtime_nsec -= (u64)tk->raw_time.tv_nsec + << tk->tkr_raw.shift; /* Accumulate error between NTP and clock interval */ tk->ntp_error += tk->ntp_tick << shift; -- 2.7.4
[RFC][PATCH 0/4] Fixes for two recently found timekeeping bugs
As part of the Linaro Linux Kernel Functional Test (LKFT) effort, test failures from kselftest/timer's inconsistency-check were reported connected to CLOCK_MONOTONIC_RAW, on the HiKey platform. Digging in I found that an old issue with how sub-ns accounting is handled with the RAW time which was fixed long ago with the CLOCK_MONOTONIC/REALTIME ids, but missed with RAW time, was present. Additionally, running further tests, I uncovered an issue with how the clocksource read function is handled when clocksources are changed, which can cause crashes. Both of these issues have not been uncovered in x86 based testing due to x86 not using vDSO to accelerate CLOCK_MONOTONIC_RAW, combined with the HiKey's arch_timer clocksource being fast to access but incrementing slowly enough to get multiple reads using the same counter value (which helps uncover time handing issues), along with the fact that none of the x86 clocksources making use of the clocksource argument passed to the read function. This patchset addresses these two issues. Thanks so much to help from Will Deacon in getting the needed adjustments to the arm64 vDSO in place. Also thanks to Daniel Mentz who also properly diagnosed the MONOTONIC_RAW issue in parallel while I was woking on this patchset. I wanted to send these out for some initial review. I'm unfortunately still chasing a third issue (related to inconsistencies triggered during extreme freq adjustments) I've uncovered on HiKey, which doesn't seem to be related to the previous two. As always feedback would be appreciated! thanks -john Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Cc: Stephen Boyd Cc: Daniel Mentz John Stultz (3): time: Fix clock->read(clock) race around clocksource changes time: Fix CLOCK_MONOTONIC_RAW sub-nanosecond accounting time: Clean up CLOCK_MONOTONIC_RAW time handling Will Deacon (1): arm64: vdso: Fix nsec handling for CLOCK_MONOTONIC_RAW arch/arm64/kernel/vdso.c | 5 +- arch/arm64/kernel/vdso/gettimeofday.S | 1 - include/linux/timekeeper_internal.h | 8 +-- kernel/time/timekeeping.c | 96 ++- 4 files changed, 66 insertions(+), 44 deletions(-) -- 2.7.4
[RFC][PATCH 1/4] time: Fix clock->read(clock) race around clocksource changes
In some testing on arm64 platforms, I was seeing null ptr crashes in the kselftest/timers clocksource-switch test. This was happening in a read function like: u64 clocksource_mmio_readl_down(struct clocksource *c) { return ~(u64)readl_relaxed(to_mmio_clksrc(c)->reg) & c->mask; } Where the callers enter the seqlock, and then call something like: cycle_now = tkr->read(tkr->clock); The problem seeming to be that since the read and clock references are happening separately, its possible the clocksource change happens in between and we end up calling the old read with the new clocksource, (or vice-versa) which causes the to_mmio_clksrc() in the read function to run off into space. This patch tries to address the issue by providing a helper function that atomically reads the clock value and then calls the clock->read(clock) call so that we always call the read funciton with the appropriate clocksource and don't accidentally mix them. The one exception where this helper isn't necessary is for the fast-timekepers which use their own locking and update logic to the tkr structures. Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Cc: Stephen Boyd Cc: Daniel Mentz Signed-off-by: John Stultz --- kernel/time/timekeeping.c | 40 +--- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 9652bc5..abc1968 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -118,6 +118,26 @@ static inline void tk_update_sleep_time(struct timekeeper *tk, ktime_t delta) tk->offs_boot = ktime_add(tk->offs_boot, delta); } +/* + * tk_clock_read - atomic clocksource read() helper + * + * This helper is necessary to use in the read paths as, while the seqlock + * ensures we don't return a bad value while structures are updated, it + * doesn't protect from potential crashes. There is the possibility that + * the tkr's clocksource may change between the read reference, and the + * clock refernce passed to the read function. This can cause crashes if + * the wrong clocksource is passed to the wrong read function. + * This isn't necessary to use when holding the timekeeper_lock or doing + * a read of the fast-timekeeper tkrs (which is protected by its own locking + * and update logic). + */ +static inline u64 tk_clock_read(struct tk_read_base *tkr) +{ + struct clocksource *clock = READ_ONCE(tkr->clock); + + return clock->read(clock); +} + #ifdef CONFIG_DEBUG_TIMEKEEPING #define WARNING_FREQ (HZ*300) /* 5 minute rate-limiting */ @@ -175,7 +195,7 @@ static inline u64 timekeeping_get_delta(struct tk_read_base *tkr) */ do { seq = read_seqcount_begin(_core.seq); - now = tkr->read(tkr->clock); + now = tk_clock_read(tkr); last = tkr->cycle_last; mask = tkr->mask; max = tkr->clock->max_cycles; @@ -209,7 +229,7 @@ static inline u64 timekeeping_get_delta(struct tk_read_base *tkr) u64 cycle_now, delta; /* read clocksource */ - cycle_now = tkr->read(tkr->clock); + cycle_now = tk_clock_read(tkr); /* calculate the delta since the last update_wall_time */ delta = clocksource_delta(cycle_now, tkr->cycle_last, tkr->mask); @@ -240,7 +260,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) tk->tkr_mono.clock = clock; tk->tkr_mono.read = clock->read; tk->tkr_mono.mask = clock->mask; - tk->tkr_mono.cycle_last = tk->tkr_mono.read(clock); + tk->tkr_mono.cycle_last = tk_clock_read(>tkr_mono); tk->tkr_raw.clock = clock; tk->tkr_raw.read = clock->read; @@ -477,7 +497,7 @@ static void halt_fast_timekeeper(struct timekeeper *tk) struct tk_read_base *tkr = >tkr_mono; memcpy(_dummy, tkr, sizeof(tkr_dummy)); - cycles_at_suspend = tkr->read(tkr->clock); + cycles_at_suspend = tk_clock_read(tkr); tkr_dummy.read = dummy_clock_read; update_fast_timekeeper(_dummy, _fast_mono); @@ -649,11 +669,10 @@ static void timekeeping_update(struct timekeeper *tk, unsigned int action) */ static void timekeeping_forward_now(struct timekeeper *tk) { - struct clocksource *clock = tk->tkr_mono.clock; u64 cycle_now, delta; u64 nsec; - cycle_now = tk->tkr_mono.read(clock); + cycle_now = tk_clock_read(>tkr_mono); delta = clocksource_delta(cycle_now, tk->tkr_mono.cycle_last, tk->tkr_mono.mask); tk->tkr_mono.cycle_last = cycle_now; tk->tkr_raw.cycle_last = cycle_now; @@ -929,8 +948,7 @@ void ktime_get_snapshot(struct system_time_snapshot *systime_snapshot) do { seq = read_seqcount_begin(_core.seq); - - now = tk->tkr_mono.read(tk->tkr_mono.clock); + now =
[RFC][PATCH 4/4] time: Clean up CLOCK_MONOTONIC_RAW time handling
Now that we fixed the sub-ns handling for CLOCK_MONOTONIC_RAW, remove the duplicitive tk->raw_time.tv_nsec, which can be stored in tk->tkr_raw.xtime_nsec (similarly to how its handled for monotonic time). Cc: Thomas GleixnerCc: Ingo Molnar Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Cc: Stephen Boyd Cc: Kevin Brodsky Cc: Will Deacon Cc: Daniel Mentz Signed-off-by: John Stultz --- arch/arm64/kernel/vdso.c| 6 ++--- include/linux/timekeeper_internal.h | 4 ++-- kernel/time/timekeeping.c | 47 - 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c index d0cb007..7492d90 100644 --- a/arch/arm64/kernel/vdso.c +++ b/arch/arm64/kernel/vdso.c @@ -220,10 +220,8 @@ void update_vsyscall(struct timekeeper *tk) if (!use_syscall) { /* tkr_mono.cycle_last == tkr_raw.cycle_last */ vdso_data->cs_cycle_last= tk->tkr_mono.cycle_last; - vdso_data->raw_time_sec = tk->raw_time.tv_sec; - vdso_data->raw_time_nsec= (tk->raw_time.tv_nsec << - tk->tkr_raw.shift) + - tk->tkr_raw.xtime_nsec; + vdso_data->raw_time_sec = tk->raw_sec; + vdso_data->raw_time_nsec= tk->tkr_raw.xtime_nsec; vdso_data->xtime_clock_sec = tk->xtime_sec; vdso_data->xtime_clock_nsec = tk->tkr_mono.xtime_nsec; vdso_data->cs_mono_mult = tk->tkr_mono.mult; diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h index 528cc86..8abf6df 100644 --- a/include/linux/timekeeper_internal.h +++ b/include/linux/timekeeper_internal.h @@ -52,7 +52,7 @@ struct tk_read_base { * @clock_was_set_seq: The sequence number of clock was set events * @cs_was_changed_seq:The sequence number of clocksource change events * @next_leap_ktime: CLOCK_MONOTONIC time value of a pending leap-second - * @raw_time: Monotonic raw base time in timespec64 format + * @raw_sec: CLOCK_MONOTONIC_RAW time in seconds * @cycle_interval:Number of clock cycles in one NTP interval * @xtime_interval:Number of clock shifted nano seconds in one NTP * interval. @@ -94,7 +94,7 @@ struct timekeeper { unsigned intclock_was_set_seq; u8 cs_was_changed_seq; ktime_t next_leap_ktime; - struct timespec64 raw_time; + u64 raw_sec; /* The following members are for timekeeping internal use */ u64 cycle_interval; diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 35d3ba3..0c3b8c1 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -72,6 +72,10 @@ static inline void tk_normalize_xtime(struct timekeeper *tk) tk->tkr_mono.xtime_nsec -= (u64)NSEC_PER_SEC << tk->tkr_mono.shift; tk->xtime_sec++; } + while (tk->tkr_raw.xtime_nsec >= ((u64)NSEC_PER_SEC << tk->tkr_raw.shift)) { + tk->tkr_raw.xtime_nsec -= (u64)NSEC_PER_SEC << tk->tkr_raw.shift; + tk->raw_sec++; + } } static inline struct timespec64 tk_xtime(struct timekeeper *tk) @@ -287,12 +291,14 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) /* if changing clocks, convert xtime_nsec shift units */ if (old_clock) { int shift_change = clock->shift - old_clock->shift; - if (shift_change < 0) + if (shift_change < 0) { tk->tkr_mono.xtime_nsec >>= -shift_change; - else + tk->tkr_raw.xtime_nsec >>= -shift_change; + } else { tk->tkr_mono.xtime_nsec <<= shift_change; + tk->tkr_raw.xtime_nsec <<= shift_change; + } } - tk->tkr_raw.xtime_nsec = 0; tk->tkr_mono.shift = clock->shift; tk->tkr_raw.shift = clock->shift; @@ -617,9 +623,6 @@ static inline void tk_update_ktime_data(struct timekeeper *tk) nsec = (u32) tk->wall_to_monotonic.tv_nsec; tk->tkr_mono.base = ns_to_ktime(seconds * NSEC_PER_SEC + nsec); - /* Update the monotonic raw base */ - tk->tkr_raw.base = timespec64_to_ktime(tk->raw_time); - /* * The sum of the nanoseconds portions of xtime and * wall_to_monotonic can be greater/equal one second. Take @@ -629,6 +632,11 @@ static inline void
[RFC][PATCH 4/4] time: Clean up CLOCK_MONOTONIC_RAW time handling
Now that we fixed the sub-ns handling for CLOCK_MONOTONIC_RAW, remove the duplicitive tk->raw_time.tv_nsec, which can be stored in tk->tkr_raw.xtime_nsec (similarly to how its handled for monotonic time). Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Cc: Stephen Boyd Cc: Kevin Brodsky Cc: Will Deacon Cc: Daniel Mentz Signed-off-by: John Stultz --- arch/arm64/kernel/vdso.c| 6 ++--- include/linux/timekeeper_internal.h | 4 ++-- kernel/time/timekeeping.c | 47 - 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c index d0cb007..7492d90 100644 --- a/arch/arm64/kernel/vdso.c +++ b/arch/arm64/kernel/vdso.c @@ -220,10 +220,8 @@ void update_vsyscall(struct timekeeper *tk) if (!use_syscall) { /* tkr_mono.cycle_last == tkr_raw.cycle_last */ vdso_data->cs_cycle_last= tk->tkr_mono.cycle_last; - vdso_data->raw_time_sec = tk->raw_time.tv_sec; - vdso_data->raw_time_nsec= (tk->raw_time.tv_nsec << - tk->tkr_raw.shift) + - tk->tkr_raw.xtime_nsec; + vdso_data->raw_time_sec = tk->raw_sec; + vdso_data->raw_time_nsec= tk->tkr_raw.xtime_nsec; vdso_data->xtime_clock_sec = tk->xtime_sec; vdso_data->xtime_clock_nsec = tk->tkr_mono.xtime_nsec; vdso_data->cs_mono_mult = tk->tkr_mono.mult; diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h index 528cc86..8abf6df 100644 --- a/include/linux/timekeeper_internal.h +++ b/include/linux/timekeeper_internal.h @@ -52,7 +52,7 @@ struct tk_read_base { * @clock_was_set_seq: The sequence number of clock was set events * @cs_was_changed_seq:The sequence number of clocksource change events * @next_leap_ktime: CLOCK_MONOTONIC time value of a pending leap-second - * @raw_time: Monotonic raw base time in timespec64 format + * @raw_sec: CLOCK_MONOTONIC_RAW time in seconds * @cycle_interval:Number of clock cycles in one NTP interval * @xtime_interval:Number of clock shifted nano seconds in one NTP * interval. @@ -94,7 +94,7 @@ struct timekeeper { unsigned intclock_was_set_seq; u8 cs_was_changed_seq; ktime_t next_leap_ktime; - struct timespec64 raw_time; + u64 raw_sec; /* The following members are for timekeeping internal use */ u64 cycle_interval; diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 35d3ba3..0c3b8c1 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -72,6 +72,10 @@ static inline void tk_normalize_xtime(struct timekeeper *tk) tk->tkr_mono.xtime_nsec -= (u64)NSEC_PER_SEC << tk->tkr_mono.shift; tk->xtime_sec++; } + while (tk->tkr_raw.xtime_nsec >= ((u64)NSEC_PER_SEC << tk->tkr_raw.shift)) { + tk->tkr_raw.xtime_nsec -= (u64)NSEC_PER_SEC << tk->tkr_raw.shift; + tk->raw_sec++; + } } static inline struct timespec64 tk_xtime(struct timekeeper *tk) @@ -287,12 +291,14 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) /* if changing clocks, convert xtime_nsec shift units */ if (old_clock) { int shift_change = clock->shift - old_clock->shift; - if (shift_change < 0) + if (shift_change < 0) { tk->tkr_mono.xtime_nsec >>= -shift_change; - else + tk->tkr_raw.xtime_nsec >>= -shift_change; + } else { tk->tkr_mono.xtime_nsec <<= shift_change; + tk->tkr_raw.xtime_nsec <<= shift_change; + } } - tk->tkr_raw.xtime_nsec = 0; tk->tkr_mono.shift = clock->shift; tk->tkr_raw.shift = clock->shift; @@ -617,9 +623,6 @@ static inline void tk_update_ktime_data(struct timekeeper *tk) nsec = (u32) tk->wall_to_monotonic.tv_nsec; tk->tkr_mono.base = ns_to_ktime(seconds * NSEC_PER_SEC + nsec); - /* Update the monotonic raw base */ - tk->tkr_raw.base = timespec64_to_ktime(tk->raw_time); - /* * The sum of the nanoseconds portions of xtime and * wall_to_monotonic can be greater/equal one second. Take @@ -629,6 +632,11 @@ static inline void tk_update_ktime_data(struct timekeeper *tk) if (nsec >= NSEC_PER_SEC) seconds++; tk->ktime_sec = seconds; + + /* Update the monotonic raw base */ + seconds = tk->raw_sec; +
[PATCH] usb: dwc2: resume root hub to handle disconnect of device
When handle disconnect of the hcd during bus_suspend, hcd needs to resume its root hub, otherwise the root hub will not disconnect the existing devices under its port. This issue always happens when connecting with usb devices which support auto-suspend function (e.g. usb hub). Signed-off-by: William Wu--- drivers/usb/dwc2/hcd.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 740c7e8..cc84f97 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -1975,11 +1975,13 @@ void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force) * Without the extra check here we will end calling disconnect * and won't get any future interrupts to handle the connect. */ - if (!force) { - hprt0 = dwc2_readl(hsotg->regs + HPRT0); - if (!(hprt0 & HPRT0_CONNDET) && (hprt0 & HPRT0_CONNSTS)) - dwc2_hcd_connect(hsotg); - } + hprt0 = dwc2_readl(hsotg->regs + HPRT0); + + if (!force && !(hprt0 & HPRT0_CONNDET) && + (hprt0 & HPRT0_CONNSTS)) + dwc2_hcd_connect(hsotg); + else if (hsotg->lx_state != DWC2_L0) + usb_hcd_resume_root_hub(hsotg->priv); } /** -- 2.0.0
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 26 May 2017, Joe Perches wrote: (please keep replies on the list) On Fri, 2017-05-26 at 18:33 -0700, Richard Narron wrote: On Fri, 26 May 2017, Joe Perches wrote: On Fri, 2017-05-26 at 16:30 -0700, Richard Narron wrote: On Fri, 26 May 2017, Joe Perches wrote: On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD and NetBSD partitions and does a reasonable job picking out OpenBSD and NetBSD UFS subpartitions. But for FreeBSD the subpartitions are always "bad". Kernel: [] block/partitions/msdos.c | 2 ++ [] @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part continue; bsd_start = le32_to_cpu(p->p_offset); bsd_size = le32_to_cpu(p->p_size); + if (memcmp(flavour, "bsd\0", 4) == 0) Weird code. Why not: if (strcmp(flavor, "bsd") == 0) I instinctively trust the memcmp function as it seems more like assembly language to me and more straight forward and more reliable than strcmp. That really doesn't matter. Your code stores "bsd\0\0" and not just "bsd\0" Thanks for looking at this code. I do appreciate it. How about saving a byte and doing this instead? if (memcmp(flavour, "bsd", 4) == 0) I do appreciate your input as coding style is important, but so too is reliability. I don't trust the string functions and probably never will. It is not surprising to me that things like SQL injection and any number of other C string exploits are very common. IBM gave up on the idea of marking memory to keep track of data length with the 1401 machines in the 1950's. But Digital Equipment kept the idea alive of using null characters for a long time. Sadly the C programming language copied this bad idea for strings. Let's not argue the language. Please use what's normal for the language as that is readers of the code typically expect. Under the /block/partitions directory the c programs have about 13 uses of memcmp() and 6 uses of strcmp().
Re: [PATCH] iscsi-target: Fix initial login PDU asynchronous socket close OOPs
Thanks for the patch. On 05/26/2017 12:32 AM, Nicholas A. Bellinger wrote: > > - state = iscsi_target_sk_state_check(sk); > - write_unlock_bh(>sk_callback_lock); > - > - pr_debug("iscsi_target_sk_state_change: state: %d\n", state); > + orig_state_change(sk); > > - if (!state) { > - pr_debug("iscsi_target_sk_state_change got failed state\n"); > - schedule_delayed_work(>login_cleanup_work, 0); I think login_cleanup_work is no longer used so you can also remove it and its code. The patch fixes the crash for me. However, is there a possible regression where if the initiator attempts new relogins we could run out of memory? With the old code, we would free the login attempts resources at this time, but with the new code the initiator will send more login attempts and so we just keep allocating more memory for each attempt until we run out or the login is finally able to complete.
[PATCH] usb: dwc2: resume root hub to handle disconnect of device
When handle disconnect of the hcd during bus_suspend, hcd needs to resume its root hub, otherwise the root hub will not disconnect the existing devices under its port. This issue always happens when connecting with usb devices which support auto-suspend function (e.g. usb hub). Signed-off-by: William Wu --- drivers/usb/dwc2/hcd.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 740c7e8..cc84f97 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -1975,11 +1975,13 @@ void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force) * Without the extra check here we will end calling disconnect * and won't get any future interrupts to handle the connect. */ - if (!force) { - hprt0 = dwc2_readl(hsotg->regs + HPRT0); - if (!(hprt0 & HPRT0_CONNDET) && (hprt0 & HPRT0_CONNSTS)) - dwc2_hcd_connect(hsotg); - } + hprt0 = dwc2_readl(hsotg->regs + HPRT0); + + if (!force && !(hprt0 & HPRT0_CONNDET) && + (hprt0 & HPRT0_CONNSTS)) + dwc2_hcd_connect(hsotg); + else if (hsotg->lx_state != DWC2_L0) + usb_hcd_resume_root_hub(hsotg->priv); } /** -- 2.0.0
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 26 May 2017, Joe Perches wrote: (please keep replies on the list) On Fri, 2017-05-26 at 18:33 -0700, Richard Narron wrote: On Fri, 26 May 2017, Joe Perches wrote: On Fri, 2017-05-26 at 16:30 -0700, Richard Narron wrote: On Fri, 26 May 2017, Joe Perches wrote: On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD and NetBSD partitions and does a reasonable job picking out OpenBSD and NetBSD UFS subpartitions. But for FreeBSD the subpartitions are always "bad". Kernel: [] block/partitions/msdos.c | 2 ++ [] @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part continue; bsd_start = le32_to_cpu(p->p_offset); bsd_size = le32_to_cpu(p->p_size); + if (memcmp(flavour, "bsd\0", 4) == 0) Weird code. Why not: if (strcmp(flavor, "bsd") == 0) I instinctively trust the memcmp function as it seems more like assembly language to me and more straight forward and more reliable than strcmp. That really doesn't matter. Your code stores "bsd\0\0" and not just "bsd\0" Thanks for looking at this code. I do appreciate it. How about saving a byte and doing this instead? if (memcmp(flavour, "bsd", 4) == 0) I do appreciate your input as coding style is important, but so too is reliability. I don't trust the string functions and probably never will. It is not surprising to me that things like SQL injection and any number of other C string exploits are very common. IBM gave up on the idea of marking memory to keep track of data length with the 1401 machines in the 1950's. But Digital Equipment kept the idea alive of using null characters for a long time. Sadly the C programming language copied this bad idea for strings. Let's not argue the language. Please use what's normal for the language as that is readers of the code typically expect. Under the /block/partitions directory the c programs have about 13 uses of memcmp() and 6 uses of strcmp().
Re: [PATCH] iscsi-target: Fix initial login PDU asynchronous socket close OOPs
Thanks for the patch. On 05/26/2017 12:32 AM, Nicholas A. Bellinger wrote: > > - state = iscsi_target_sk_state_check(sk); > - write_unlock_bh(>sk_callback_lock); > - > - pr_debug("iscsi_target_sk_state_change: state: %d\n", state); > + orig_state_change(sk); > > - if (!state) { > - pr_debug("iscsi_target_sk_state_change got failed state\n"); > - schedule_delayed_work(>login_cleanup_work, 0); I think login_cleanup_work is no longer used so you can also remove it and its code. The patch fixes the crash for me. However, is there a possible regression where if the initiator attempts new relogins we could run out of memory? With the old code, we would free the login attempts resources at this time, but with the new code the initiator will send more login attempts and so we just keep allocating more memory for each attempt until we run out or the login is finally able to complete.
Re: [PATCH 2/6] mfd: hi6421-pmic: move hi6421_regmap_config definition to header file
On Fri, May 26, 2017 at 4:33 PM, Arnd Bergmannwrote: > On Fri, May 26, 2017 at 8:35 AM, Guodong Xu wrote: >> Move hi6421_regmap_config definition from c code to common header: >> - include/linux/mfd/hi6421-pmic.h >> >> This is to improve code re-use for upcoming hi6421 series of MFD driver. >> >> Signed-off-by: Guodong Xu > >> diff --git a/include/linux/mfd/hi6421-pmic.h >> b/include/linux/mfd/hi6421-pmic.h >> index 587273e..f4674ff 100644 >> --- a/include/linux/mfd/hi6421-pmic.h >> +++ b/include/linux/mfd/hi6421-pmic.h >> @@ -38,4 +38,10 @@ struct hi6421_pmic { >> struct regmap *regmap; >> }; >> >> +static const struct regmap_config hi6421_regmap_config = { >> + .reg_bits = 32, >> + .reg_stride = 4, >> + .val_bits = 8, >> + .max_register = HI6421_REG_TO_BUS_ADDR(HI6421_REG_MAX), >> +}; >> #endif /* __HI6421_PMIC_H */ > > Header files should not have static variables in general, it will cause > warnings > about unused variables when you include the header from another file > (depending on compiler version and warning options, I think older gcc > versions don't warn about this, but clang and latest gcc do). > I will fix that. > How about adding the new code into the existing > drivers/mfd/hi6421-pmic-core.c file, and splitting out the part that differs > (the regmap_update_bits is the only difference I see) Yes, indeed. > into a callback > that you reference through the of_device_id->data pointer? > Thanks Arnd. I will look into this. I'll drop hi6421v530-pmic.c and resend this patchset. -Guodong > Arnd
Re: [PATCH 2/6] mfd: hi6421-pmic: move hi6421_regmap_config definition to header file
On Fri, May 26, 2017 at 4:33 PM, Arnd Bergmann wrote: > On Fri, May 26, 2017 at 8:35 AM, Guodong Xu wrote: >> Move hi6421_regmap_config definition from c code to common header: >> - include/linux/mfd/hi6421-pmic.h >> >> This is to improve code re-use for upcoming hi6421 series of MFD driver. >> >> Signed-off-by: Guodong Xu > >> diff --git a/include/linux/mfd/hi6421-pmic.h >> b/include/linux/mfd/hi6421-pmic.h >> index 587273e..f4674ff 100644 >> --- a/include/linux/mfd/hi6421-pmic.h >> +++ b/include/linux/mfd/hi6421-pmic.h >> @@ -38,4 +38,10 @@ struct hi6421_pmic { >> struct regmap *regmap; >> }; >> >> +static const struct regmap_config hi6421_regmap_config = { >> + .reg_bits = 32, >> + .reg_stride = 4, >> + .val_bits = 8, >> + .max_register = HI6421_REG_TO_BUS_ADDR(HI6421_REG_MAX), >> +}; >> #endif /* __HI6421_PMIC_H */ > > Header files should not have static variables in general, it will cause > warnings > about unused variables when you include the header from another file > (depending on compiler version and warning options, I think older gcc > versions don't warn about this, but clang and latest gcc do). > I will fix that. > How about adding the new code into the existing > drivers/mfd/hi6421-pmic-core.c file, and splitting out the part that differs > (the regmap_update_bits is the only difference I see) Yes, indeed. > into a callback > that you reference through the of_device_id->data pointer? > Thanks Arnd. I will look into this. I'll drop hi6421v530-pmic.c and resend this patchset. -Guodong > Arnd
[PATCH 1/5] ftrace: Fix memory leak in ftrace_graph_release()
From: Luis Henriquesftrace_hash is being kfree'ed in ftrace_graph_release(), however the ->buckets field is not. This results in a memory leak that is easily captured by kmemleak: unreferenced object 0x880038afe000 (size 8192): comm "trace-cmd", pid 238, jiffies 4294916898 (age 9.736s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 backtrace: [] kmemleak_alloc+0x4e/0xb0 [] __kmalloc+0x12d/0x1a0 [] alloc_ftrace_hash+0x51/0x80 [] __ftrace_graph_open.isra.39.constprop.46+0xa3/0x100 [] ftrace_graph_open+0x68/0xa0 [] do_dentry_open.isra.1+0x1bd/0x2d0 [] vfs_open+0x47/0x60 [] path_openat+0x2a5/0x1020 [] do_filp_open+0x8a/0xf0 [] do_sys_open+0x12f/0x200 [] SyS_open+0x1e/0x20 [] entry_SYSCALL_64_fastpath+0x13/0x94 [] 0x Link: http://lkml.kernel.org/r/20170525152038.7661-1-lhenriq...@suse.com Cc: sta...@vger.kernel.org Fixes: b9b0c831bed2 ("ftrace: Convert graph filter to use hash tables") Signed-off-by: Luis Henriques Signed-off-by: Steven Rostedt (VMware) --- kernel/trace/ftrace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 74fdfe9ed3db..9e5841dc14b5 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -5063,7 +5063,7 @@ ftrace_graph_release(struct inode *inode, struct file *file) } out: - kfree(fgd->new_hash); + free_ftrace_hash(fgd->new_hash); kfree(fgd); return ret; -- 2.10.2
[PATCH 1/5] ftrace: Fix memory leak in ftrace_graph_release()
From: Luis Henriques ftrace_hash is being kfree'ed in ftrace_graph_release(), however the ->buckets field is not. This results in a memory leak that is easily captured by kmemleak: unreferenced object 0x880038afe000 (size 8192): comm "trace-cmd", pid 238, jiffies 4294916898 (age 9.736s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 backtrace: [] kmemleak_alloc+0x4e/0xb0 [] __kmalloc+0x12d/0x1a0 [] alloc_ftrace_hash+0x51/0x80 [] __ftrace_graph_open.isra.39.constprop.46+0xa3/0x100 [] ftrace_graph_open+0x68/0xa0 [] do_dentry_open.isra.1+0x1bd/0x2d0 [] vfs_open+0x47/0x60 [] path_openat+0x2a5/0x1020 [] do_filp_open+0x8a/0xf0 [] do_sys_open+0x12f/0x200 [] SyS_open+0x1e/0x20 [] entry_SYSCALL_64_fastpath+0x13/0x94 [] 0x Link: http://lkml.kernel.org/r/20170525152038.7661-1-lhenriq...@suse.com Cc: sta...@vger.kernel.org Fixes: b9b0c831bed2 ("ftrace: Convert graph filter to use hash tables") Signed-off-by: Luis Henriques Signed-off-by: Steven Rostedt (VMware) --- kernel/trace/ftrace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 74fdfe9ed3db..9e5841dc14b5 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -5063,7 +5063,7 @@ ftrace_graph_release(struct inode *inode, struct file *file) } out: - kfree(fgd->new_hash); + free_ftrace_hash(fgd->new_hash); kfree(fgd); return ret; -- 2.10.2
[PATCH 4/5] x86/mm/ftrace: Do not bug in early boot on irqs_disabled in cpu_flush_range()
From: "Steven Rostedt (VMware)"With function tracing starting in early bootup and having its trampoline pages being read only, a bug triggered with the following: kernel BUG at arch/x86/mm/pageattr.c:189! invalid opcode: [#1] SMP Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 4.12.0-rc2-test+ #3 Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 task: b4222500 task.stack: b420 RIP: 0010:change_page_attr_set_clr+0x269/0x302 RSP: :b4203c88 EFLAGS: 00010046 RAX: 0046 RBX: RCX: 0001b600 RDX: b4203d40 RSI: RDI: b4240d60 RBP: b4203d18 R08: 0001b600 R09: 0001 R10: b4203aa8 R11: 0003 R12: c029b000 R13: b4203d40 R14: 0001 R15: FS: () GS:9a639ea0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 9a636b384000 CR3: 0001ea21d000 CR4: 000406b0 Call Trace: change_page_attr_clear+0x1f/0x21 set_memory_ro+0x1e/0x20 arch_ftrace_update_trampoline+0x207/0x21c ? ftrace_caller+0x64/0x64 ? 0xc029b000 ftrace_startup+0xf4/0x198 register_ftrace_function+0x26/0x3c function_trace_init+0x5e/0x73 tracer_init+0x1e/0x23 tracing_set_tracer+0x127/0x15a register_tracer+0x19b/0x1bc init_function_trace+0x90/0x92 early_trace_init+0x236/0x2b3 start_kernel+0x200/0x3f5 x86_64_start_reservations+0x29/0x2b x86_64_start_kernel+0x17c/0x18f secondary_startup_64+0x9f/0x9f ? secondary_startup_64+0x9f/0x9f Interrupts should not be enabled at this early in the boot process. It is also fine to leave interrupts enabled during this time as there's only one CPU running, and on_each_cpu() means to only run on the current CPU. If early_boot_irqs_disabled is set, it is safe to run cpu_flush_range() with interrupts disabled. Don't trigger a BUG_ON() in that case. Link: http://lkml.kernel.org/r/20170526093717.0be3b...@gandalf.local.home Suggested-by: Thomas Gleixner Signed-off-by: Steven Rostedt (VMware) --- arch/x86/mm/pageattr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c index 1dcd2be4cce4..c8520b2c62d2 100644 --- a/arch/x86/mm/pageattr.c +++ b/arch/x86/mm/pageattr.c @@ -186,7 +186,7 @@ static void cpa_flush_range(unsigned long start, int numpages, int cache) unsigned int i, level; unsigned long addr; - BUG_ON(irqs_disabled()); + BUG_ON(irqs_disabled() && !early_boot_irqs_disabled); WARN_ON(PAGE_ALIGN(start) != start); on_each_cpu(__cpa_flush_range, NULL, 1); -- 2.10.2
[PATCH 4/5] x86/mm/ftrace: Do not bug in early boot on irqs_disabled in cpu_flush_range()
From: "Steven Rostedt (VMware)" With function tracing starting in early bootup and having its trampoline pages being read only, a bug triggered with the following: kernel BUG at arch/x86/mm/pageattr.c:189! invalid opcode: [#1] SMP Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 4.12.0-rc2-test+ #3 Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 task: b4222500 task.stack: b420 RIP: 0010:change_page_attr_set_clr+0x269/0x302 RSP: :b4203c88 EFLAGS: 00010046 RAX: 0046 RBX: RCX: 0001b600 RDX: b4203d40 RSI: RDI: b4240d60 RBP: b4203d18 R08: 0001b600 R09: 0001 R10: b4203aa8 R11: 0003 R12: c029b000 R13: b4203d40 R14: 0001 R15: FS: () GS:9a639ea0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 9a636b384000 CR3: 0001ea21d000 CR4: 000406b0 Call Trace: change_page_attr_clear+0x1f/0x21 set_memory_ro+0x1e/0x20 arch_ftrace_update_trampoline+0x207/0x21c ? ftrace_caller+0x64/0x64 ? 0xc029b000 ftrace_startup+0xf4/0x198 register_ftrace_function+0x26/0x3c function_trace_init+0x5e/0x73 tracer_init+0x1e/0x23 tracing_set_tracer+0x127/0x15a register_tracer+0x19b/0x1bc init_function_trace+0x90/0x92 early_trace_init+0x236/0x2b3 start_kernel+0x200/0x3f5 x86_64_start_reservations+0x29/0x2b x86_64_start_kernel+0x17c/0x18f secondary_startup_64+0x9f/0x9f ? secondary_startup_64+0x9f/0x9f Interrupts should not be enabled at this early in the boot process. It is also fine to leave interrupts enabled during this time as there's only one CPU running, and on_each_cpu() means to only run on the current CPU. If early_boot_irqs_disabled is set, it is safe to run cpu_flush_range() with interrupts disabled. Don't trigger a BUG_ON() in that case. Link: http://lkml.kernel.org/r/20170526093717.0be3b...@gandalf.local.home Suggested-by: Thomas Gleixner Signed-off-by: Steven Rostedt (VMware) --- arch/x86/mm/pageattr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c index 1dcd2be4cce4..c8520b2c62d2 100644 --- a/arch/x86/mm/pageattr.c +++ b/arch/x86/mm/pageattr.c @@ -186,7 +186,7 @@ static void cpa_flush_range(unsigned long start, int numpages, int cache) unsigned int i, level; unsigned long addr; - BUG_ON(irqs_disabled()); + BUG_ON(irqs_disabled() && !early_boot_irqs_disabled); WARN_ON(PAGE_ALIGN(start) != start); on_each_cpu(__cpa_flush_range, NULL, 1); -- 2.10.2
[PATCH 0/5] [GIT PULL] ftrace/kprobes/x86: Memory Fix Edition
Linus, There's been a few memory issues found with ftrace. One was simply a memory leak where not all was being freed that should have been in releasing a file pointer on set_graph_function. Then Thomas found that the ftrace trampolines were marked for read/write as well as execute. To shrink the possible attack surface, he added calls to set them to ro. Which also uncovered some other issues with freeing module allocated memory that had its permissions changed. Kprobes had a similar issue which is fixed and a selftest was added to trigger that issue again. Please pull the latest trace-v4.12-rc2 tree, which can be found at: git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git trace-v4.12-rc2 Tag SHA1: 61ffee417e15b119c84e95b07348fdffd916f254 Head SHA1: 6ee98ffeea0bc9e072e419497d78697d8afcdd6d Luis Henriques (1): ftrace: Fix memory leak in ftrace_graph_release() Masami Hiramatsu (2): kprobes/x86: Fix to set RWX bits correctly before releasing trampoline selftests/ftrace: Add a testcase for many kprobe events Steven Rostedt (VMware) (1): x86/mm/ftrace: Do not bug in early boot on irqs_disabled in cpu_flush_range() Thomas Gleixner (1): x86/ftrace: Make sure that ftrace trampolines are not RWX arch/x86/kernel/ftrace.c| 20 ++-- arch/x86/kernel/kprobes/core.c | 9 + arch/x86/mm/pageattr.c | 2 +- kernel/kprobes.c| 2 +- kernel/trace/ftrace.c | 2 +- .../ftrace/test.d/kprobe/multiple_kprobes.tc| 21 + 6 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
[PATCH 0/5] [GIT PULL] ftrace/kprobes/x86: Memory Fix Edition
Linus, There's been a few memory issues found with ftrace. One was simply a memory leak where not all was being freed that should have been in releasing a file pointer on set_graph_function. Then Thomas found that the ftrace trampolines were marked for read/write as well as execute. To shrink the possible attack surface, he added calls to set them to ro. Which also uncovered some other issues with freeing module allocated memory that had its permissions changed. Kprobes had a similar issue which is fixed and a selftest was added to trigger that issue again. Please pull the latest trace-v4.12-rc2 tree, which can be found at: git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git trace-v4.12-rc2 Tag SHA1: 61ffee417e15b119c84e95b07348fdffd916f254 Head SHA1: 6ee98ffeea0bc9e072e419497d78697d8afcdd6d Luis Henriques (1): ftrace: Fix memory leak in ftrace_graph_release() Masami Hiramatsu (2): kprobes/x86: Fix to set RWX bits correctly before releasing trampoline selftests/ftrace: Add a testcase for many kprobe events Steven Rostedt (VMware) (1): x86/mm/ftrace: Do not bug in early boot on irqs_disabled in cpu_flush_range() Thomas Gleixner (1): x86/ftrace: Make sure that ftrace trampolines are not RWX arch/x86/kernel/ftrace.c| 20 ++-- arch/x86/kernel/kprobes/core.c | 9 + arch/x86/mm/pageattr.c | 2 +- kernel/kprobes.c| 2 +- kernel/trace/ftrace.c | 2 +- .../ftrace/test.d/kprobe/multiple_kprobes.tc| 21 + 6 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
[PATCH 3/5] selftests/ftrace: Add a testcase for many kprobe events
From: Masami HiramatsuAdd a testcase to test kprobes via ftrace interface with many concurrent kprobe events. This tries to add many kprobe events (up to 256) on kernel functions. To avoid making ftrace-based kprobes (kprobes on fentry), it skips first N bytes (on x86 N=5, on ppc or arm N=4) of function entry. After that, it enables all those events, disable it, and remove it. Since the unoptimization buffer reclaiming will be delayed, after removing events, it will wait enough time. Link: http://lkml.kernel.org/r/149577388470.11702.11832460851769204511.stgit@devbox Signed-off-by: Masami Hiramatsu Suggested-by: Steven Rostedt Signed-off-by: Steven Rostedt (VMware) --- .../ftrace/test.d/kprobe/multiple_kprobes.tc| 21 + 1 file changed, 21 insertions(+) create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc new file mode 100644 index ..f4d1ff785d67 --- /dev/null +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc @@ -0,0 +1,21 @@ +#!/bin/sh +# description: Register/unregister many kprobe events + +# ftrace fentry skip size depends on the machine architecture. +# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc +case `uname -m` in + x86_64|i[3456]86) OFFS=5;; + ppc*) OFFS=4;; + *) OFFS=0;; +esac + +echo "Setup up to 256 kprobes" +grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ +head -n 256 | while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||: + +echo 1 > events/kprobes/enable +echo 0 > events/kprobes/enable +echo > kprobe_events +echo "Waiting for unoptimizing & freeing" +sleep 5 +echo "Done" -- 2.10.2
[PATCH 3/5] selftests/ftrace: Add a testcase for many kprobe events
From: Masami Hiramatsu Add a testcase to test kprobes via ftrace interface with many concurrent kprobe events. This tries to add many kprobe events (up to 256) on kernel functions. To avoid making ftrace-based kprobes (kprobes on fentry), it skips first N bytes (on x86 N=5, on ppc or arm N=4) of function entry. After that, it enables all those events, disable it, and remove it. Since the unoptimization buffer reclaiming will be delayed, after removing events, it will wait enough time. Link: http://lkml.kernel.org/r/149577388470.11702.11832460851769204511.stgit@devbox Signed-off-by: Masami Hiramatsu Suggested-by: Steven Rostedt Signed-off-by: Steven Rostedt (VMware) --- .../ftrace/test.d/kprobe/multiple_kprobes.tc| 21 + 1 file changed, 21 insertions(+) create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc new file mode 100644 index ..f4d1ff785d67 --- /dev/null +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc @@ -0,0 +1,21 @@ +#!/bin/sh +# description: Register/unregister many kprobe events + +# ftrace fentry skip size depends on the machine architecture. +# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc +case `uname -m` in + x86_64|i[3456]86) OFFS=5;; + ppc*) OFFS=4;; + *) OFFS=0;; +esac + +echo "Setup up to 256 kprobes" +grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ +head -n 256 | while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||: + +echo 1 > events/kprobes/enable +echo 0 > events/kprobes/enable +echo > kprobe_events +echo "Waiting for unoptimizing & freeing" +sleep 5 +echo "Done" -- 2.10.2
[PATCH 5/5] x86/ftrace: Make sure that ftrace trampolines are not RWX
From: Thomas Gleixnerftrace use module_alloc() to allocate trampoline pages. The mapping of module_alloc() is RWX, which makes sense as the memory is written to right after allocation. But nothing makes these pages RO after writing to them. Add proper set_memory_rw/ro() calls to protect the trampolines after modification. Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1705251056410.1862@nanos Signed-off-by: Thomas Gleixner Signed-off-by: Steven Rostedt (VMware) --- arch/x86/kernel/ftrace.c | 20 ++-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 0651e974dcb3..9bef1bbeba63 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned long size) { return module_alloc(size); } -static inline void tramp_free(void *tramp) +static inline void tramp_free(void *tramp, int size) { + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT; + + set_memory_nx((unsigned long)tramp, npages); + set_memory_rw((unsigned long)tramp, npages); module_memfree(tramp); } #else @@ -699,7 +703,7 @@ static inline void *alloc_tramp(unsigned long size) { return NULL; } -static inline void tramp_free(void *tramp) { } +static inline void tramp_free(void *tramp, int size) { } #endif /* Defined as markers to the end of the ftrace default trampolines */ @@ -771,7 +775,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size) /* Copy ftrace_caller onto the trampoline memory */ ret = probe_kernel_read(trampoline, (void *)start_offset, size); if (WARN_ON(ret < 0)) { - tramp_free(trampoline); + tramp_free(trampoline, *tramp_size); return 0; } @@ -797,7 +801,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size) /* Are we pointing to the reference? */ if (WARN_ON(memcmp(op_ptr.op, op_ref, 3) != 0)) { - tramp_free(trampoline); + tramp_free(trampoline, *tramp_size); return 0; } @@ -839,7 +843,7 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops) unsigned long offset; unsigned long ip; unsigned int size; - int ret; + int ret, npages; if (ops->trampoline) { /* @@ -848,11 +852,14 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops) */ if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) return; + npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT; + set_memory_rw(ops->trampoline, npages); } else { ops->trampoline = create_trampoline(ops, ); if (!ops->trampoline) return; ops->trampoline_size = size; + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; } offset = calc_trampoline_call_offset(ops->flags & FTRACE_OPS_FL_SAVE_REGS); @@ -863,6 +870,7 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops) /* Do a safe modify in case the trampoline is executing */ new = ftrace_call_replace(ip, (unsigned long)func); ret = update_ftrace_func(ip, new); + set_memory_ro(ops->trampoline, npages); /* The update should never fail */ WARN_ON(ret); @@ -939,7 +947,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops) if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) return; - tramp_free((void *)ops->trampoline); + tramp_free((void *)ops->trampoline, ops->trampoline_size); ops->trampoline = 0; } -- 2.10.2
[PATCH 5/5] x86/ftrace: Make sure that ftrace trampolines are not RWX
From: Thomas Gleixner ftrace use module_alloc() to allocate trampoline pages. The mapping of module_alloc() is RWX, which makes sense as the memory is written to right after allocation. But nothing makes these pages RO after writing to them. Add proper set_memory_rw/ro() calls to protect the trampolines after modification. Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1705251056410.1862@nanos Signed-off-by: Thomas Gleixner Signed-off-by: Steven Rostedt (VMware) --- arch/x86/kernel/ftrace.c | 20 ++-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 0651e974dcb3..9bef1bbeba63 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned long size) { return module_alloc(size); } -static inline void tramp_free(void *tramp) +static inline void tramp_free(void *tramp, int size) { + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT; + + set_memory_nx((unsigned long)tramp, npages); + set_memory_rw((unsigned long)tramp, npages); module_memfree(tramp); } #else @@ -699,7 +703,7 @@ static inline void *alloc_tramp(unsigned long size) { return NULL; } -static inline void tramp_free(void *tramp) { } +static inline void tramp_free(void *tramp, int size) { } #endif /* Defined as markers to the end of the ftrace default trampolines */ @@ -771,7 +775,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size) /* Copy ftrace_caller onto the trampoline memory */ ret = probe_kernel_read(trampoline, (void *)start_offset, size); if (WARN_ON(ret < 0)) { - tramp_free(trampoline); + tramp_free(trampoline, *tramp_size); return 0; } @@ -797,7 +801,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size) /* Are we pointing to the reference? */ if (WARN_ON(memcmp(op_ptr.op, op_ref, 3) != 0)) { - tramp_free(trampoline); + tramp_free(trampoline, *tramp_size); return 0; } @@ -839,7 +843,7 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops) unsigned long offset; unsigned long ip; unsigned int size; - int ret; + int ret, npages; if (ops->trampoline) { /* @@ -848,11 +852,14 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops) */ if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) return; + npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT; + set_memory_rw(ops->trampoline, npages); } else { ops->trampoline = create_trampoline(ops, ); if (!ops->trampoline) return; ops->trampoline_size = size; + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; } offset = calc_trampoline_call_offset(ops->flags & FTRACE_OPS_FL_SAVE_REGS); @@ -863,6 +870,7 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops) /* Do a safe modify in case the trampoline is executing */ new = ftrace_call_replace(ip, (unsigned long)func); ret = update_ftrace_func(ip, new); + set_memory_ro(ops->trampoline, npages); /* The update should never fail */ WARN_ON(ret); @@ -939,7 +947,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops) if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) return; - tramp_free((void *)ops->trampoline); + tramp_free((void *)ops->trampoline, ops->trampoline_size); ops->trampoline = 0; } -- 2.10.2