Re: [PATCH v6 04/34] dt-bindings: Add bindings for Keem Bay IPC driver
On Mon, Apr 12, 2021 at 04:24:41PM -0500, Jassi Brar wrote: > On Mon, Mar 8, 2021 at 2:20 PM mark gross wrote: > > > > On Fri, Mar 05, 2021 at 03:01:40PM -0600, Rob Herring wrote: > > > On Fri, Feb 12, 2021 at 02:22:34PM -0800, mgr...@linux.intel.com wrote: > > > > From: Daniele Alessandrelli > > > > > > > > Add DT binding documentation for the Intel Keem Bay IPC driver, which > > > > > > Bindings are for h/w blocks, not drivers. From a binding perspective, I > > > don't really care what the driver architecture for some OS looks like. I > > > continue to not understand what this h/w looks like. A block diagram > > > would help as would understanding what blocks have multiple clients > > > (mailboxes and xlink in particular). > > I'm working to gather this info. > > > Do I pick the mailbox related patches (and which ones exactly) ? v6-0002-dt-bindings-mailbox-Add-Intel-VPU-IPC-mailbox-bin.patch and v6-0003-mailbox-vpu-ipc-mailbox-Add-support-for-Intel-VPU.patch --mark
Re: [PATCH v6 19/34] xlink-core: Add xlink core device tree bindings
On Fri, Mar 05, 2021 at 03:03:00PM -0600, Rob Herring wrote:
> On Fri, Feb 12, 2021 at 02:22:49PM -0800, mgr...@linux.intel.com wrote:
> > From: Seamus Kelly
> >
> > Add device tree bindings for keembay-xlink.
> >
> > Cc: Rob Herring
> > Cc: devicet...@vger.kernel.org
> > Reviewed-by: Mark Gross
> > Signed-off-by: Mark Gross
> > Signed-off-by: Seamus Kelly
> > ---
> > .../bindings/misc/intel,keembay-xlink.yaml| 29 +++
> > 1 file changed, 29 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml
> > b/Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml
> > new file mode 100644
> > index ..5ac2e7fa5b5e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml
> > @@ -0,0 +1,29 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright (c) Intel Corporation. All rights reserved.
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/misc/intel,keembay-xlink.yaml#;
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#;
> > +
> > +title: Intel Keem Bay xlink
> > +
> > +maintainers:
> > + - Seamus Kelly
> > +
> > +description: |
> > + The Keem Bay xlink driver enables the communication/control sub-system
> > + for internal and external communications to the Intel Keem Bay SoC.
> > +
> > +properties:
> > + compatible:
> > +oneOf:
> > + - items:
> > + - const: intel,keembay-xlink
> > +
> > +additionalProperties: False
> > +
> > +examples:
> > + - |
> > +xlink {
> > +compatible = "intel,keembay-xlink";
>
> A node with only a compatible is almost always abusing DT just to
> instantiate your driver.
Is it normal to make drivers that want to abuse DT in this way platform
devices?
Any advice would be welcome and helful.
thanks!
--mark
Re: [PATCH v6 04/34] dt-bindings: Add bindings for Keem Bay IPC driver
On Fri, Mar 05, 2021 at 03:01:40PM -0600, Rob Herring wrote:
> On Fri, Feb 12, 2021 at 02:22:34PM -0800, mgr...@linux.intel.com wrote:
> > From: Daniele Alessandrelli
> >
> > Add DT binding documentation for the Intel Keem Bay IPC driver, which
>
> Bindings are for h/w blocks, not drivers. From a binding perspective, I
> don't really care what the driver architecture for some OS looks like. I
> continue to not understand what this h/w looks like. A block diagram
> would help as would understanding what blocks have multiple clients
> (mailboxes and xlink in particular).
I'm working to gather this info.
thanks!
--mark
>
> > enables communication between the Computing Sub-System (CSS) and the
> > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
> > Bay.
> >
> > Cc: Rob Herring
> > Cc: devicet...@vger.kernel.org
> > Reviewed-by: Mark Gross
> > Signed-off-by: Daniele Alessandrelli
> > Signed-off-by: Mark Gross
> > ---
> > .../bindings/soc/intel/intel,keembay-ipc.yaml | 45 +++
> > 1 file changed, 45 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
> > b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
> > new file mode 100644
> > index ..586fe73f4cd4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
> > @@ -0,0 +1,45 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2020 Intel Corporation
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/soc/intel/intel,keembay-ipc.yaml#;
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#;
> > +
> > +title: Keem Bay IPC
> > +
> > +maintainers:
> > + - Daniele Alessandrelli
> > +
> > +description:
> > + The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with
> > the
> > + Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
> > + Keem Bay.
> > +
> > +properties:
> > + compatible:
> > +const: intel,keembay-ipc
> > +
> > + memory-region:
> > +items:
> > + - description:
> > + Reserved memory region used by the CPU to allocate IPC packets.
> > + - description:
> > + Reserved memory region used by the VPU to allocate IPC packets.
> > +
> > + mboxes:
> > +description: VPU IPC Mailbox.
> > +
> > +required:
> > + - compatible
> > + - memory-region
> > + - mboxes
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > +ipc {
> > + compatible = "intel,keembay-ipc";
> > + memory-region = <_cpu_reserved>, <_vpu_reserved>;
> > + mboxes = <_ipc_mbox 0>;
> > +};
> > --
> > 2.17.1
> >
Re: [PATCH] platform/x86: dell-wmi-sysman: correct an initialization failure
On Thu, Feb 18, 2021 at 01:17:23PM -0600, Mario Limonciello wrote:
> On Dell systems that don't support this interface the module is
> mistakingly returning error code "0", when it should be returning
> -ENODEV. Correct a logic error to guarantee the correct return code.
>
> Cc: Divya Bharathi
> Reported-by: Alexander Naumann
> Signed-off-by: Mario Limonciello
> ---
> drivers/platform/x86/dell-wmi-sysman/biosattr-interface.c | 4 +++-
> drivers/platform/x86/dell-wmi-sysman/passwordattr-interface.c | 4 +++-
> drivers/platform/x86/dell-wmi-sysman/sysman.c | 4 ++--
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/dell-wmi-sysman/biosattr-interface.c
> b/drivers/platform/x86/dell-wmi-sysman/biosattr-interface.c
> index f95d8ddace5a..8d59f81f9db4 100644
> --- a/drivers/platform/x86/dell-wmi-sysman/biosattr-interface.c
> +++ b/drivers/platform/x86/dell-wmi-sysman/biosattr-interface.c
> @@ -175,7 +175,9 @@ static struct wmi_driver bios_attr_set_interface_driver =
> {
>
> int init_bios_attr_set_interface(void)
> {
> - return wmi_driver_register(_attr_set_interface_driver);
> + int ret = wmi_driver_register(_attr_set_interface_driver);
I have to ask if the propper fix should be in wmi_driver_register
> +
> + return wmi_priv.bios_attr_wdev ? ret : -ENODEV;
Also, is there any point to call wmi_driver_register if returning -ENODEV?
i.e. should the call to driver register be wrapped in a test for
bios_atter_wdev?
> }
>
> void exit_bios_attr_set_interface(void)
> diff --git a/drivers/platform/x86/dell-wmi-sysman/passwordattr-interface.c
> b/drivers/platform/x86/dell-wmi-sysman/passwordattr-interface.c
> index 5780b4d94759..bf449dc5ff47 100644
> --- a/drivers/platform/x86/dell-wmi-sysman/passwordattr-interface.c
> +++ b/drivers/platform/x86/dell-wmi-sysman/passwordattr-interface.c
> @@ -142,7 +142,9 @@ static struct wmi_driver bios_attr_pass_interface_driver
> = {
>
> int init_bios_attr_pass_interface(void)
> {
> - return wmi_driver_register(_attr_pass_interface_driver);
> + int ret = wmi_driver_register(_attr_pass_interface_driver);
> +
> + return wmi_priv.password_attr_wdev ? ret : -ENODEV;
same comments as above only for password_atter_wdev.
--mark
> }
>
> void exit_bios_attr_pass_interface(void)
> diff --git a/drivers/platform/x86/dell-wmi-sysman/sysman.c
> b/drivers/platform/x86/dell-wmi-sysman/sysman.c
> index cb81010ba1a2..d9ad0e83b66f 100644
> --- a/drivers/platform/x86/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell-wmi-sysman/sysman.c
> @@ -513,13 +513,13 @@ static int __init sysman_init(void)
> }
>
> ret = init_bios_attr_set_interface();
> - if (ret || !wmi_priv.bios_attr_wdev) {
> + if (ret) {
> pr_debug("failed to initialize set interface\n");
> goto fail_set_interface;
> }
>
> ret = init_bios_attr_pass_interface();
> - if (ret || !wmi_priv.password_attr_wdev) {
> + if (ret) {
> pr_debug("failed to initialize pass interface\n");
> goto fail_pass_interface;
> }
> --
> 2.25.1
>
Re: [PATCH v6 03/34] mailbox: vpu-ipc-mailbox: Add support for Intel VPU IPC mailbox
On Thu, Feb 18, 2021 at 03:09:55PM -0600, Jassi Brar wrote: > On Thu, Feb 18, 2021 at 6:02 AM Alessandrelli, Daniele > wrote: > > > > Hi Jassi, > > > > Thank you very much for your feedback. > > > > On Sun, 2021-02-14 at 22:54 -0600, Jassi Brar wrote: > > > IIUIC, maybe the solution is simpler What if we set txdone_poll. > > > Always return success in send_data(). And check if we overflew the > > > fifo in last_tx_done(). If we did overflow, try to rewrite the data > > > and check again. Return true, if not overflew this time, otherwise > > > return false so that mailbox api can ask us to try again in next > > > last_tx_done(). This way we can do away with the tasklet and, more > > > importantly, avoid send_data() failures and retries on clients' part. > > > > That's a clever solution to avoid the tasklet. The only issue for us is > > the automatic TX retry from the controller. I understand that's > > generally a desirable feature, but in our case we'd like the client to > > have full control on re-transmission attempts. > > > > That's because some of our data is time-sensitive. For instance, when > > we process frames from a video stream we prefer dropping a frame rather > > than re-transmitting it and delaying the processing of the rest. > > > > Now, I understand that the client can set the 'tx_block' and 'tx_tout' > > channel fields to specify how long it wishes to wait, but the problem > > is that our (single) channel is shared between multiple applications > > having different timing requirements. That's why we prefer to let > > applications deal we re-transmissions. > > > > Given the above, do you think it's reasonable to leave the > > implementation as it is now? > > (from initial analysis, the tasklet doesn't seem to affect the > > performance of our use cases significantly, so we are fine with it) > > > Yup. It is intel specific so, hopefully, we don't have to deal with > other vendors trying to support their use cases. > Are you targeting the next merge window or this one? > Its a bit late for the v5.12 merge window for most of the code in this patchset at this point but, if you feel like getting this inital bit in that would be great. I'm hoping we can get the rest of this series into linux-next after RC1. --mark
Re: [RFC PATCH 0/7] Add managed version of delayed work init
On Sat, Feb 13, 2021 at 01:58:17PM +0200, Matti Vaittinen wrote: > It's not rare that device drivers need delayed work. > It's not rare that this work needs driver's data. > > Often this means that driver must ensure the work is not queued when > driver exits. Usually this is done by ensuring new work is not added and > then calling cancel_delayed_work_sync() at remove(). In many cases this > may also require cleanup at probe error path - which is easy to forget. > > It might be helpful for (a) few drivers if there was a work init why the (a) and not just a? > function which would ensure cancel_delayed_work_sync() is called at > driver exit. So this series implements one on top of devm and replaces > the obvious cases where only thing remove call-back in a driver does is > cancelling the work. There might be other cases where we could switch > more than just work cancellation to use managed version and thus get rid > of remove. > > Main reson why this is RFC is that I had hard time deciding where this > function should be introduced. It's not nice to include all device stuff > in workqueue - because many workqueue users are not interested in > devices. In same way, not all of the devices are interested in WQs. > OTOH, adding own file just for this sounds like an overkill. s/own/one --mark > > This time I decided that it is more correct that devices use WQs than > that WQs use devices. Hence the function is introduced in > include/linux/device.h and drivers/base/devres.c > > -- > > Matti Vaittinen (7): > drivers: base: Add resource managed version of delayed work init > extconn: Clean-up few drivers by using managed work init > hwmon: raspberry-pi: Clean-up few drivers by using managed work init > platform/x86: gpd pocket fan: Clean-up by using managed work init > power: supply: Clean-up few drivers by using managed work init > regulator: qcom_spmi-regulator: Clean-up by using managed work init > watchdog: retu_wdt: Clean-up by using managed work init > > drivers/base/devres.c| 33 > drivers/extcon/extcon-gpio.c | 14 ++--- > drivers/extcon/extcon-intel-int3496.c| 15 ++--- > drivers/extcon/extcon-palmas.c | 16 +++--- > drivers/extcon/extcon-qcom-spmi-misc.c | 16 +++--- > drivers/hwmon/raspberrypi-hwmon.c| 16 +++--- > drivers/platform/x86/gpd-pocket-fan.c| 16 +++--- > drivers/power/supply/axp20x_usb_power.c | 15 +++-- > drivers/power/supply/bq24735-charger.c | 17 +++--- > drivers/power/supply/ltc2941-battery-gauge.c | 19 --- > drivers/power/supply/sbs-battery.c | 15 +++-- > drivers/regulator/qcom_spmi-regulator.c | 33 +--- > drivers/watchdog/retu_wdt.c | 21 +++-- > include/linux/device.h | 5 +++ > 14 files changed, 95 insertions(+), 156 deletions(-) > > > base-commit: 92bf22614b21a2706f4993b278017e437f7785b3 > -- > 2.25.4 > > > -- > Matti Vaittinen, Linux device drivers > ROHM Semiconductors, Finland SWDC > Kiviharjunlenkki 1E > 90220 OULU > FINLAND > > ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ > Simon says - in Latin please. > ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ > Thanks to Simon Glass for the translation =]
Re: [PATCH] platform/x86: fix typo in Kconfig
On Tue, Feb 16, 2021 at 10:36:13PM +0100, Petr Vaněk wrote: > uses by -> used by > > Signed-off-by: Petr Vaněk Reviewed-by: Mark Gross Thanks for the clean up! -mark > --- > drivers/platform/x86/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 91e6176cdfbd..94f2f05bc133 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1372,7 +1372,7 @@ config INTEL_PMT_CLASS > tristate "Intel Platform Monitoring Technology (PMT) Class driver" > help > The Intel Platform Monitoring Technology (PMT) class driver provides > - the basic sysfs interface and file hierarchy uses by PMT devices. > + the basic sysfs interface and file hierarchy used by PMT devices. > > For more information, see: > > -- > 2.26.2 >
Re: [PATCH v6 34/34] misc: HDDL device management for IA host
On Sun, Feb 14, 2021 at 09:48:53AM -0800, Randy Dunlap wrote: > On 2/12/21 2:23 PM, mgr...@linux.intel.com wrote: > > diff --git a/drivers/misc/hddl_device/Kconfig > > b/drivers/misc/hddl_device/Kconfig > > index e1ae81fdf177..7f9a6a685275 100644 > > --- a/drivers/misc/hddl_device/Kconfig > > +++ b/drivers/misc/hddl_device/Kconfig > > @@ -12,3 +12,15 @@ config HDDL_DEVICE_CLIENT > > the device connect/disconnect programming sequence. > > Say Y if using a processor that includes the Intel VPU such as > > Keem Bay. If unsure, say N. > > + > > +config HDDL_DEVICE_SERVER > > + tristate "Support for hddl device server" > > Please use "HDDL" consistently. fixed and qued up for merge window closure. --mark > > > + depends on XLINK_CORE && !HDDL_DEVICE_CLIENT > > + help > > + This option enables HDDL device server module. > > + > > + This driver is used for sharing time sync data to local host and > > + retrives the sensors available on the platform. This also handles > > + the device connect/disconnect programming sequence. > > + Say Y if using a processor that includes the Intel VPU such as > > + Keem Bay. If unsure, say N. > > > thanks. > -- > ~Randy >
Re: [PATCH v6 33/34] misc: Hddl device management for local host
On Sun, Feb 14, 2021 at 09:47:51AM -0800, Randy Dunlap wrote: > On 2/12/21 2:23 PM, mgr...@linux.intel.com wrote: > > diff --git a/drivers/misc/hddl_device/Kconfig > > b/drivers/misc/hddl_device/Kconfig > > new file mode 100644 > > index ..e1ae81fdf177 > > --- /dev/null > > +++ b/drivers/misc/hddl_device/Kconfig > > @@ -0,0 +1,14 @@ > > +# Copyright (C) 2020 Intel Corporation > > +# SPDX-License-Identifier: GPL-2.0-only > > + > > +config HDDL_DEVICE_CLIENT > > + tristate "Support for hddl device client" > > Please use "HDDL" consistently. fixed and qued up for merge window closure. --mark > > + depends on XLINK_CORE && INTEL_TSENS_LOCAL_HOST > > + help > > + This option enables HDDL device client module. > > + > > + This driver is used for sharing time sync data to local host and > > + retrives the sensors available on the platform. This also handles > > + the device connect/disconnect programming sequence. > > + Say Y if using a processor that includes the Intel VPU such as > > + Keem Bay. If unsure, say N. > > > -- > ~Randy >
Re: [PATCH v6 30/34] misc:intel_tsens: Intel Keem Bay tsens driver.
On Sun, Feb 14, 2021 at 09:42:22AM -0800, Randy Dunlap wrote: > On 2/12/21 2:23 PM, mgr...@linux.intel.com wrote: > > diff --git a/drivers/misc/intel_tsens/Kconfig > > b/drivers/misc/intel_tsens/Kconfig > > index be8d27e81864..5cfe6b4004e5 100644 > > --- a/drivers/misc/intel_tsens/Kconfig > > +++ b/drivers/misc/intel_tsens/Kconfig > > @@ -28,6 +28,18 @@ config INTEL_TSENS_I2C_SLAVE > > Say Y if using a processor that includes the Intel VPU such as > > Keem Bay. If unsure, say N. > > > > +config KEEMBAY_THERMAL > > + tristate "Temperature sensor driver for intel Keem Bay" > > s/intel/Intel/ ? > > as above and below. fixed and qued up for posting with the merge window closes. --mark > > > + depends on INTEL_TSENS_LOCAL_HOST && ARCH_KEEMBAY > > + help > > + Enable this option if you want to have support for Keem Bay > > + thermal management sensors. > > + > > + This driver is used for reading onchip temperature sensor > > + values from Keem Bay SoC. > > + Say Y if using a processor that includes the Intel VPU such as > > + Keem Bay. If unsure, say N. > > > -- > ~Randy >
Re: [PATCH v6 29/34] Intel tsens i2c slave driver.
On Sun, Feb 14, 2021 at 09:41:26AM -0800, Randy Dunlap wrote: > On 2/12/21 2:22 PM, mgr...@linux.intel.com wrote: > > diff --git a/drivers/misc/intel_tsens/Kconfig > > b/drivers/misc/intel_tsens/Kconfig > > index 8b263fdd80c3..be8d27e81864 100644 > > --- a/drivers/misc/intel_tsens/Kconfig > > +++ b/drivers/misc/intel_tsens/Kconfig > > @@ -14,6 +14,20 @@ config INTEL_TSENS_LOCAL_HOST > > Say Y if using a processor that includes the Intel VPU such as > > Keem Bay. If unsure, say N. > > > > +config INTEL_TSENS_I2C_SLAVE > > + bool "I2C slave driver for intel tsens" > > s/intel/Intel/ ? > (as below) fixed and qued up for posting when merge window closes. Thanks --mark > > > + depends on INTEL_TSENS_LOCAL_HOST > > + depends on I2C=y && I2C_SLAVE > > + help > > + This option enables tsens I2C slave driver. > > + > > + This driver is used for reporting thermal data via I2C > > + SMBUS to remote host. > > + Enable this option if you want to have support for thermal > > + management controller. > > + Say Y if using a processor that includes the Intel VPU such as > > + Keem Bay. If unsure, say N. > > > -- > ~Randy >
Re: [PATCH v6 28/34] misc: Intel tsens IA host driver.
On Sun, Feb 14, 2021 at 09:45:56AM -0800, Randy Dunlap wrote: > On 2/12/21 2:22 PM, mgr...@linux.intel.com wrote: > > diff --git a/drivers/misc/intel_tsens/Kconfig > > b/drivers/misc/intel_tsens/Kconfig > > index bfb8fe1997f4..8b263fdd80c3 100644 > > --- a/drivers/misc/intel_tsens/Kconfig > > +++ b/drivers/misc/intel_tsens/Kconfig > > @@ -13,3 +13,16 @@ config INTEL_TSENS_LOCAL_HOST > > management controller. > > Say Y if using a processor that includes the Intel VPU such as > > Keem Bay. If unsure, say N. > > + > > +config INTEL_TSENS_IA_HOST > > + tristate "Temperature sensor driver for intel tsens remote host" > > s/intel/Intel/ ? > > as above and below. fixed and qued up for posting when merge window closes. Thanks! --mark > > > + depends on I2C && THERMAL > > + depends on I2C_SMBUS > > + help > > + This option enables tsens i2c and thermal local Host driver. > > + > > + This driver is used for reading thermal data via I2C SMBUS > > + and registers itself to thermal framework, which can be > > + used by thermal daemon in remote IA host > > + Say Y if using a processor that includes the Intel VPU such as > > + Keem Bay. If unsure, say N. > > > -- > ~Randy >
Re: [PATCH v6 27/34] misc: Tsens ARM host thermal driver.
On Sun, Feb 14, 2021 at 09:44:53AM -0800, Randy Dunlap wrote: > On 2/12/21 2:22 PM, mgr...@linux.intel.com wrote: > > diff --git a/drivers/misc/intel_tsens/Kconfig > > b/drivers/misc/intel_tsens/Kconfig > > new file mode 100644 > > index ..bfb8fe1997f4 > > --- /dev/null > > +++ b/drivers/misc/intel_tsens/Kconfig > > @@ -0,0 +1,15 @@ > > +# Copyright (C) 2020 Intel Corporation > > +# SPDX-License-Identifier: GPL-2.0-only > > + > > +config INTEL_TSENS_LOCAL_HOST > > + bool "Temperature sensor driver for intel tsens" > > s/intel/Intel/ ? > > as below. fixed and qued up for posting when merge window closes. --mark > > > + select THERMAL > > + help > > + This option enables tsens thermal local Host driver. > > + > > + This driver is used for reporting thermal data via thermal > > + framework. > > + Enable this option if you want to have support for thermal > > + management controller. > > + Say Y if using a processor that includes the Intel VPU such as > > + Keem Bay. If unsure, say N. > > > -- > ~Randy >
Re: [PATCH v6 25/34] misc: Add Keem Bay VPU manager
On Sun, Feb 14, 2021 at 09:39:55AM -0800, Randy Dunlap wrote:
> On 2/12/21 2:22 PM, mgr...@linux.intel.com wrote:
> > diff --git a/drivers/misc/vpumgr/Kconfig b/drivers/misc/vpumgr/Kconfig
> > new file mode 100644
> > index ..bb82ff83afd3
> > --- /dev/null
> > +++ b/drivers/misc/vpumgr/Kconfig
> > @@ -0,0 +1,9 @@
> > +config VPUMGR
> > + tristate "VPU Manager"
> > + depends on ARM64 && XLINK_CORE
> > + help
> > + VPUMGR manages life-cycle of VPU related resources which were
> > + dynamically allocated on demands of user-space application
>
> End the sentence above with a period ('.').
fixed and qued up for merge window closure.
--mark
>
> > +
> > + Select y or m if you have a processor including the Intel
> > + Vision Processor (VPU) on it.
>
>
> --
> ~Randy
>
Re: [PATCH v6 20/34] xlink-core: Add xlink core driver xLink
On Sun, Feb 14, 2021 at 09:52:51AM -0800, Randy Dunlap wrote: > On 2/12/21 2:22 PM, mgr...@linux.intel.com wrote: > > diff --git a/drivers/misc/xlink-core/Kconfig > > b/drivers/misc/xlink-core/Kconfig > > new file mode 100644 > > index ..a0ceb0b48219 > > --- /dev/null > > +++ b/drivers/misc/xlink-core/Kconfig > > @@ -0,0 +1,33 @@ > > +config XLINK_CORE > > + tristate "Support for XLINK CORE" > > + depends on ((XLINK_PCIE_RH_DRIVER || XBAY_XLINK_PCIE_RH_DRIVER) || > > (XLINK_LOCAL_HOST && (XLINK_PCIE_LH_DRIVER || XBAY_XLINK_PCIE_RH_DRIVER))) > > -ELINETOOLONG. Use '\' for line continuation in Kconfig files. fixed > > > + help > > + XLINK CORE enables the communication/control subsystem. > > + > > + If unsure, say N. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called xlink.ko. > > + > > +config XLINK_LOCAL_HOST > > + tristate "Support for XLINK LOCAL HOST" > > + depends on XLINK_IPC > > + help > > + XLINK LOCAL HOST enables local host functionality for > > + the communication/control Sub-System. > > + > > + Enable this config when building the kernel for the Intel Vision > > + Processing Unit (VPU) Local Host core. > > + > > + If building for a Remote Host kernel, say N. > > + > > +config XLINK_PSS > > + tristate "Support for XLINK PSS (Pre-Silicon Solution)" > > + depends on XLINK_LOCAL_HOST > > + help > > + XLINK PSS enables the communication/control subsystem on a PSS > > platform. > > + > > + Enable this config when building the kernel for the Intel Vision > > + Processing Unit (VPU) in a simulated env. > > Please s/env/environment/. fixed I have the update qued up and I'll post v7 rebased when the merge window closes --mark > > > + > > + If building for a VPU silicon, say N. > > > -- > ~Randy >
Re: [PATCH] platform/surface: aggregator: Fix access of unaligned value
Acked-by: Mark Gross
On Thu, Feb 11, 2021 at 12:04:11AM +0100, Maximilian Luz wrote:
> The raw message frame length is unaligned and explicitly marked as
> little endian. It should not be accessed without the appropriatte
> accessor functions. Fix this.
>
> Reported-by: kernel-test-robot
> Fixes: c167b9c7e3d6 ("platform/surface: Add Surface Aggregator subsystem")
> Signed-off-by: Maximilian Luz
> ---
> drivers/platform/surface/aggregator/ssh_packet_layer.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/surface/aggregator/ssh_packet_layer.c
> b/drivers/platform/surface/aggregator/ssh_packet_layer.c
> index 583315db8b02..9a78188d8d1c 100644
> --- a/drivers/platform/surface/aggregator/ssh_packet_layer.c
> +++ b/drivers/platform/surface/aggregator/ssh_packet_layer.c
> @@ -1774,7 +1774,8 @@ static size_t ssh_ptl_rx_eval(struct ssh_ptl *ptl,
> struct ssam_span *source)
> break;
> }
>
> - return aligned.ptr - source->ptr + SSH_MESSAGE_LENGTH(frame->len);
> + return aligned.ptr - source->ptr
> + + SSH_MESSAGE_LENGTH(get_unaligned_le16(>len));
> }
>
> static int ssh_ptl_rx_threadfn(void *data)
> --
> 2.30.0
>
Re: [PATCH V4 0/6] x86: Don't abuse tss.sp1
On Wed, Feb 10, 2021 at 09:39:11PM +0800, Lai Jiangshan wrote: > From: Lai Jiangshan > > In x86_64, tss.sp1 is reused as cpu_current_top_of_stack. We'd better > directly use percpu since CR3 and gs_base is correct when it is used. Be more direct if not using percpu is incorrect in some way. > > In x86_32, tss.sp1 is resued as thread.sp0 in three places in entry s/resued/reused > code. We have the correct CR3 and %fs at two of the places. The last > one is sysenter. This patchset makes %fs available earlier so that > we can also use percpu in sysenter. And add a percpu cpu_current_thread_sp0 > for thread.sp0 instead of tss.sp1 > > [V3]: > https://lore.kernel.org/lkml/20210127163231.12709-1-jiangshan...@gmail.com/ > [V2]: > https://lore.kernel.org/lkml/20210125173444.22696-1-jiangshan...@gmail.com/ > [V1]: > https://lore.kernel.org/lkml/20210123084900.3118-1-jiangshan...@gmail.com/ > > Changed from V3: > Update subjects as Borislav's imperative request. ^_^ > Update changelog as Borislav suggested. > Change EXPORT_PER_CPU_SYMBOL to EXPORT_PER_CPU_SYMBOL_GPL. > > Changed from V2: > Add missing "%ss:" reported by Brian Gerst. > > Changed from V1: > Requested from Andy to also fix sp1 for x86_32. > Update comments in the x86_64 patch as Andy sugguested. > > Lai Jiangshan (6): > x86/entry/64: Move cpu_current_top_of_stack out of TSS > x86/entry/32: Use percpu instead of offset-calculation to get > thread.sp0 in SWITCH_TO_KERNEL_STACK > x86/entry/32: Switch to the task stack without emptying the entry > stack > x86/entry/32: Restore %fs before switching stack > x86/entry/32: Use percpu to get thread.sp0 in SYSENTER > x86/entry/32: Introduce cpu_current_thread_sp0 to replace > cpu_tss_rw.x86_tss.sp1 > > arch/x86/entry/entry_32.S | 38 +- > arch/x86/include/asm/processor.h | 12 ++ > arch/x86/include/asm/switch_to.h | 8 +-- > arch/x86/include/asm/thread_info.h | 6 - > arch/x86/kernel/asm-offsets.c | 1 - > arch/x86/kernel/asm-offsets_32.c | 10 > arch/x86/kernel/cpu/common.c | 12 +- > arch/x86/kernel/process.c | 7 -- > arch/x86/mm/pti.c | 7 +++--- > 9 files changed, 39 insertions(+), 62 deletions(-) > > -- > 2.19.1.6.gb485710b >
Re: [PATCH AUTOSEL 5.10 09/36] platform/x86: hp-wmi: Disable tablet-mode reporting by default
looks good to me.
--mark
On Mon, Feb 08, 2021 at 12:57:39PM -0500, Sasha Levin wrote:
> From: Hans de Goede
>
> [ Upstream commit 67fbe02a5cebc3c653610f12e3c0424e58450153 ]
>
> Recently userspace has started making more use of SW_TABLET_MODE
> (when an input-dev reports this).
>
> Specifically recent GNOME3 versions will:
>
> 1. When SW_TABLET_MODE is reported and is reporting 0:
> 1.1 Disable accelerometer-based screen auto-rotation
> 1.2 Disable automatically showing the on-screen keyboard when a
> text-input field is focussed
>
> 2. When SW_TABLET_MODE is reported and is reporting 1:
> 2.1 Ignore input-events from the builtin keyboard and touchpad
> (this is for 360° hinges style 2-in-1s where the keyboard and
> touchpads are accessible on the back of the tablet when folded
> into tablet-mode)
>
> This means that claiming to support SW_TABLET_MODE when it does not
> actually work / reports correct values has bad side-effects.
>
> The check in the hp-wmi code which is used to decide if the input-dev
> should claim SW_TABLET_MODE support, only checks if the
> HPWMI_HARDWARE_QUERY is supported. It does *not* check if the hardware
> actually is capable of reporting SW_TABLET_MODE.
>
> This leads to the hp-wmi input-dev claiming SW_TABLET_MODE support,
> while in reality it will always report 0 as SW_TABLET_MODE value.
> This has been seen on a "HP ENVY x360 Convertible 15-cp0xxx" and
> this likely is the case on a whole lot of other HP models.
>
> This problem causes both auto-rotation and on-screen keyboard
> support to not work on affected x360 models.
>
> There is no easy fix for this, but since userspace expects
> SW_TABLET_MODE reporting to be reliable when advertised it is
> better to not claim/report SW_TABLET_MODE support at all, then
> to claim to support it while it does not work.
>
> To avoid the mentioned problems, add a new enable_tablet_mode_sw
> module-parameter which defaults to false.
>
> Note I've made this an int using the standard -1=auto, 0=off, 1=on
> triplett, with the hope that in the future we can come up with a
> better way to detect SW_TABLET_MODE support. ATM the default
> auto option just does the same as off.
>
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1918255
> Cc: Stefan Brüns
> Signed-off-by: Hans de Goede
> Acked-by: Mark Gross
> Link: https://lore.kernel.org/r/20210120124941.73409-1-hdego...@redhat.com
> Signed-off-by: Sasha Levin
> ---
> drivers/platform/x86/hp-wmi.c | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> index 18bf8aeb5f870..e94e59283ecb9 100644
> --- a/drivers/platform/x86/hp-wmi.c
> +++ b/drivers/platform/x86/hp-wmi.c
> @@ -32,6 +32,10 @@ MODULE_LICENSE("GPL");
> MODULE_ALIAS("wmi:95F24279-4D7B-4334-9387-ACCDC67EF61C");
> MODULE_ALIAS("wmi:5FB7F034-2C63-45e9-BE91-3D44E2C707E4");
>
> +static int enable_tablet_mode_sw = -1;
> +module_param(enable_tablet_mode_sw, int, 0444);
> +MODULE_PARM_DESC(enable_tablet_mode_sw, "Enable SW_TABLET_MODE reporting
> (-1=auto, 0=no, 1=yes)");
> +
> #define HPWMI_EVENT_GUID "95F24279-4D7B-4334-9387-ACCDC67EF61C"
> #define HPWMI_BIOS_GUID "5FB7F034-2C63-45e9-BE91-3D44E2C707E4"
>
> @@ -654,10 +658,12 @@ static int __init hp_wmi_input_setup(void)
> }
>
> /* Tablet mode */
> - val = hp_wmi_hw_state(HPWMI_TABLET_MASK);
> - if (!(val < 0)) {
> - __set_bit(SW_TABLET_MODE, hp_wmi_input_dev->swbit);
> - input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE, val);
> + if (enable_tablet_mode_sw > 0) {
> + val = hp_wmi_hw_state(HPWMI_TABLET_MASK);
> + if (val >= 0) {
> + __set_bit(SW_TABLET_MODE, hp_wmi_input_dev->swbit);
> + input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE,
> val);
> + }
> }
>
> err = sparse_keymap_setup(hp_wmi_input_dev, hp_wmi_keymap, NULL);
> --
> 2.27.0
>
Re: [PATCH v3 06/34] dt-bindings: Add bindings for Keem Bay VPU IPC driver
On Wed, Jan 27, 2021 at 08:00:11AM -0600, Rob Herring wrote: > On Mon, 25 Jan 2021 21:40:08 -0800, mgr...@linux.intel.com wrote: > > From: Paul Murphy > > > > Add DT bindings documentation for the Keem Bay VPU IPC driver. > > > > Cc: Rob Herring > > Cc: devicet...@vger.kernel.org > > Reviewed-by: Mark Gross > > Signed-off-by: Paul Murphy > > Co-developed-by: Daniele Alessandrelli > > Signed-off-by: Daniele Alessandrelli > > --- > > .../soc/intel/intel,keembay-vpu-ipc.yaml | 153 ++ > > 1 file changed, 153 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml > > > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > ./Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml:21:9: > [warning] wrong indentation: expected 10 but found 8 (indentation) > > dtschema/dtc warnings/errors: fixed. > > See https://patchwork.ozlabs.org/patch/1432168 > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit. >
Re: [PATCH v3 02/34] dt-bindings: mailbox: Add Intel VPU IPC mailbox bindings
On Tue, Jan 26, 2021 at 02:47:15PM +, Gross, Mark wrote: > > > > -Original Message- > > From: Rob Herring > > Sent: Tuesday, January 26, 2021 5:45 AM > > To: Mark Gross > > Cc: markgr...@kernel.org; Arnd Bergmann ; Borislav Petkov > > ; Damien Le Moal ; Dragan Cvetic > > ; Greg Kroah-Hartman > > ; Jonathan Corbet ; Palmer > > Dabbelt ; Paul Walmsley > > ; Peng Fan ; Shawn Guo > > ; Jassi Brar ; linux- > > ker...@vger.kernel.org; Alessandrelli, Daniele > > > > Subject: Re: [PATCH v3 02/34] dt-bindings: mailbox: Add Intel VPU IPC > > mailbox > > bindings > > > > On Mon, Jan 25, 2021 at 11:40 PM wrote: > > > > > > From: Daniele Alessandrelli > > > > > > Add bindings for the Intel VPU IPC mailbox driver. > > > > Sigh. DT list please so it's in my queue and automated checks run. > I'm sorry about that. > Quick question, should I include the DT-list on just the patches with DT yaml > or all of the series as its got DT content? Fixed. --mark > > --mark > > > > > > > Signed-off-by: Daniele Alessandrelli > > > --- > > > .../mailbox/intel,vpu-ipc-mailbox.yaml| 69 +++ > > > MAINTAINERS | 6 ++ > > > 2 files changed, 75 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/mailbox/intel,vpu-ipc-mailbox.yaml > > > > > > diff --git > > > a/Documentation/devicetree/bindings/mailbox/intel,vpu-ipc-mailbox.yaml > > > b/Documentation/devicetree/bindings/mailbox/intel,vpu-ipc-mailbox.yaml
Re: [PATCH v2 29/34] Intel tsens i2c slave driver.
On Mon, Jan 11, 2021 at 11:15:06PM -0800, Randy Dunlap wrote: > On 1/8/21 1:25 PM, mgr...@linux.intel.com wrote: > > diff --git a/drivers/misc/intel_tsens/Kconfig > > b/drivers/misc/intel_tsens/Kconfig > > index 8b263fdd80c3..c2138339bd89 100644 > > --- a/drivers/misc/intel_tsens/Kconfig > > +++ b/drivers/misc/intel_tsens/Kconfig > > @@ -14,6 +14,20 @@ config INTEL_TSENS_LOCAL_HOST > > Say Y if using a processor that includes the Intel VPU such as > > Keem Bay. If unsure, say N. > > > > +config INTEL_TSENS_I2C_SLAVE > > + bool "I2C slave driver for intel tsens" > > Why bool instead of tristate? Becuase the I2C driver depends on a file scoped global i2c_plat_data instanciated in the INTELL_TSENS_LOCAL_HOST DRIVER (intel_tsens_thermal.[ch]) Udhaya, would you care to comment further? --mark > > > + depends on INTEL_TSENS_LOCAL_HOST > > + select I2C > > + select I2C_SLAVE > > + help > > + This option enables tsens i2c slave driver. > > I2C > > > + > > + This driver is used for reporting thermal data via I2C > > + SMBUS to remote host. > > + Enable this option if you want to have support for thermal > > + management controller > >controller. > > > + Say Y if using a processor that includes the Intel VPU such as > > + Keem Bay. If unsure, say N. > > > -- > ~Randy >
Re: [PATCH] platform/x86: amd-pmc: put device on error paths
On Wed, Jan 20, 2021 at 08:50:05PM -0800, Pan Bian wrote:
> Put the PCI device rdev on error paths to fix potential reference count
> leaks.
Can you make a stronger statment than "fix potenital reference count leaks?".
Also, make the commit comment match the code better.
maybe something like:
"On the prob error return paths associated with rdev we are leaking ref counts,
add
pci_dev_put to return patch to avoid the leaks".
Note: I'm not sure there is a leak but, assuming the code is correct the commit
comment should be more clear and match the code.
Do you have a test case that show this change is good?
--mark
>
> Signed-off-by: Pan Bian
> ---
> drivers/platform/x86/amd-pmc.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
> index 0102bf1c7916..df140019c4bd 100644
> --- a/drivers/platform/x86/amd-pmc.c
> +++ b/drivers/platform/x86/amd-pmc.c
> @@ -210,31 +210,39 @@ static int amd_pmc_probe(struct platform_device *pdev)
> dev->dev = >dev;
>
> rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
> - if (!rdev || !pci_match_id(pmc_pci_ids, rdev))
> + if (!rdev || !pci_match_id(pmc_pci_ids, rdev)) {
> + pci_dev_put(rdev);
> return -ENODEV;
> + }
>
> dev->cpu_id = rdev->device;
> err = pci_write_config_dword(rdev, AMD_PMC_SMU_INDEX_ADDRESS,
> AMD_PMC_BASE_ADDR_LO);
> if (err) {
> dev_err(dev->dev, "error writing to 0x%x\n",
> AMD_PMC_SMU_INDEX_ADDRESS);
> + pci_dev_put(rdev);
> return pcibios_err_to_errno(err);
> }
>
> err = pci_read_config_dword(rdev, AMD_PMC_SMU_INDEX_DATA, );
> - if (err)
> + if (err) {
> + pci_dev_put(rdev);
> return pcibios_err_to_errno(err);
> + }
>
> base_addr_lo = val & AMD_PMC_BASE_ADDR_HI_MASK;
>
> err = pci_write_config_dword(rdev, AMD_PMC_SMU_INDEX_ADDRESS,
> AMD_PMC_BASE_ADDR_HI);
> if (err) {
> dev_err(dev->dev, "error writing to 0x%x\n",
> AMD_PMC_SMU_INDEX_ADDRESS);
> + pci_dev_put(rdev);
> return pcibios_err_to_errno(err);
> }
>
> err = pci_read_config_dword(rdev, AMD_PMC_SMU_INDEX_DATA, );
> - if (err)
> + if (err) {
> + pci_dev_put(rdev);
> return pcibios_err_to_errno(err);
> + }
>
> base_addr_hi = val & AMD_PMC_BASE_ADDR_LO_MASK;
> pci_dev_put(rdev);
> --
> 2.17.1
>
>
Re: [PATCH v2 15/34] misc: xlink-pcie: Add XLink API interface
On Wed, Jan 20, 2021 at 06:59:33PM +0100, Greg KH wrote:
> On Fri, Jan 08, 2021 at 01:25:41PM -0800, mgr...@linux.intel.com wrote:
> > From: Srikanth Thokala
> >
> > Provide interface for XLink layer to interact with XLink PCIe transport
> > layer on both local host and remote host.
> >
> > Cc: Arnd Bergmann
> > Cc: Greg Kroah-Hartman
> > Reviewed-by: Mark Gross
> > Signed-off-by: Srikanth Thokala
> > ---
> > drivers/misc/xlink-pcie/common/interface.c | 109 +++
> > drivers/misc/xlink-pcie/local_host/Makefile | 1 +
> > drivers/misc/xlink-pcie/remote_host/Makefile | 1 +
> > 3 files changed, 111 insertions(+)
> > create mode 100644 drivers/misc/xlink-pcie/common/interface.c
> >
> > diff --git a/drivers/misc/xlink-pcie/common/interface.c
> > b/drivers/misc/xlink-pcie/common/interface.c
> > new file mode 100644
> > index ..56c1d9ed9d8f
> > --- /dev/null
> > +++ b/drivers/misc/xlink-pcie/common/interface.c
> > @@ -0,0 +1,109 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + *
> > + * Intel Keem Bay XLink PCIe Driver
> > + *
> > + * Copyright (C) 2020 Intel Corporation
> > + *
> > +
> > /
>
> Do you really need the *** mess? :)
>
> > +
> > +#include
> > +
> > +#include "core.h"
> > +#include "xpcie.h"
> > +
> > +/* Define xpcie driver interface API */
> > +int xlink_pcie_get_device_list(u32 *sw_device_id_list, u32 *num_devices)
> > +{
> > + if (!sw_device_id_list || !num_devices)
> > + return -EINVAL;
> > +
> > + *num_devices = intel_xpcie_get_device_num(sw_device_id_list);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(xlink_pcie_get_device_list);
>
> EXPORT_SYMBOL_GPL() for all of these perhaps? I have to ask...
I can't think of a reason why not using the _GPL flavor of export symbol. I'll
change them all if that's desired.
--mark
>
> thanks,
>
> greg k-h
Re: [PATCH 01/22] Add Vision Processing Unit (VPU) documentation.
On Fri, Dec 18, 2020 at 03:30:18PM -0800, Randy Dunlap wrote: > Hi-- > > On 12/1/20 2:34 PM, mgr...@linux.intel.com wrote: > > From: mark gross > > > > > > Reviewed-by: Mark Gross > > Signed-off-by: Mark Gross > > My reading of submitting-patches.rst seems to indicate that > the Reviewer and Submitter are probably not the same person. > > Are you sure that you reviewed it? > > > > --- > > Documentation/index.rst | 3 +- > > Documentation/vpu/index.rst | 16 ++ > > Documentation/vpu/vpu-stack-overview.rst | 267 +++ > > 3 files changed, 285 insertions(+), 1 deletion(-) > > create mode 100644 Documentation/vpu/index.rst > > create mode 100644 Documentation/vpu/vpu-stack-overview.rst > > > > diff --git a/Documentation/index.rst b/Documentation/index.rst > > index 57719744774c..0a2cc0204e8f 100644 > > --- a/Documentation/index.rst > > +++ b/Documentation/index.rst > > @@ -1,4 +1,4 @@ > > -.. SPDX-License-Identifier: GPL-2.0 > > +.. SPDX-License-Identifier: GPL-2.0-only > > That looks both inappropriate for this patch and incorrect AFAICT. > > > > > > > .. The Linux Kernel documentation master file, created by > > @@ -137,6 +137,7 @@ needed). > > misc-devices/index > > scheduler/index > > mhi/index > > + vpu/index > > > > Architecture-agnostic documentation > > --- > > diff --git a/Documentation/vpu/index.rst b/Documentation/vpu/index.rst > > new file mode 100644 > > index ..7e290e048910 > > --- /dev/null > > +++ b/Documentation/vpu/index.rst > > @@ -0,0 +1,16 @@ > > +.. SPDX-License-Identifier: GPL-2.0-only > > license-rules.rst says: > > For 'GNU General Public License (GPL) version 2 only' use: > SPDX-License-Identifier: GPL-2.0 > > > + > > + > > +Vision Processor Unit Documentation > > + > > + > > +This documentation contains information for the Intel VPU stack. > > + > > +.. class:: toc-title > > + > > + Table of contents > > + > > +.. toctree:: > > + :maxdepth: 2 > > + > > + vpu-stack-overview > > diff --git a/Documentation/vpu/vpu-stack-overview.rst > > b/Documentation/vpu/vpu-stack-overview.rst > > new file mode 100644 > > index ..53c06a7d9a52 > > --- /dev/null > > +++ b/Documentation/vpu/vpu-stack-overview.rst > > @@ -0,0 +1,267 @@ > > +.. SPDX-License-Identifier: GPL-2.0-only > > Nope. > > > + > > +== > > +Intel VPU architecture > > +== > > + > > +Overview > > + > > + > > +The Intel Movidius acquisition has developed a Vision Processing Unit (VPU) > > +roadmap of products starting with Keem Bay (KMB). The HW configurations > > the > > s/HW/hardware/ > > > +VPU can support include: > > + > > +1. Standalone smart camera that does local CV processing in camera > > Tell us what CV is before using it. > > > +2. Standalone appliance or SBC device connected to a network and tethered > > Tell us what SBC is before using it. (yeah, I know) > > > + cameras doing local CV processing > > +3. Embedded in a USB dongle or M.2 as an CV accelerator. > > +4. Multiple VPU enabled SOC's on a PCIE card as a CV accelerator in a > > larger IA > > PCIe (?) > > > + box or server. > > + > > +Keem Bay is the first instance of this family of products. This document > > +provides an architectural overview of the SW stack supporting the VPU > > enabled > > s/SW/software/ > > > +products. > > + > > +Keem Bay (KMB) is a Computer Vision AI processing SoC based on ARM A53 CPU > > that > > +provides Edge neural network acceleration (inference) and includes a Vision > > +Processing Unit (VPU) hardware. The ARM CPU SubSystem (CPUSS) interfaces > > +locally to the VPU and enables integration/interfacing with a remote host > > over > > +PCIe or USB or Ethernet interfaces. The interface between the CPUSS and > > the VPU > > +is implemented with HW FIFOs (Control) and coherent memory mapping (Data) > > such > > +that zero copy processing can happen within the VPU. > > + > > +The KMB can be used in all 4 of the above classes of designs. > > + > > +We refer to the
Re: [PATCH 06/22] misc: xlink-pcie: Add documentation for XLink PCIe driver
On Fri, Dec 18, 2020 at 02:59:00PM -0800, Randy Dunlap wrote: > On 12/1/20 2:34 PM, mgr...@linux.intel.com wrote: > > From: Srikanth Thokala > > > > Provide overview of XLink PCIe driver implementation > > > > Cc: linux-...@vger.kernel.org > > Reviewed-by: Mark Gross > > Signed-off-by: Srikanth Thokala > > --- > > Documentation/vpu/index.rst | 1 + > > Documentation/vpu/xlink-pcie.rst | 91 > > 2 files changed, 92 insertions(+) > > create mode 100644 Documentation/vpu/xlink-pcie.rst > > > > Hi-- > > For document, chapter, section, etc., headings, please read & use > Documentation/doc-guide/sphinx.rst: > > * Please stick to this order of heading adornments: > > 1. ``=`` with overline for document title:: > >== >Document title >== > > 2. ``=`` for chapters:: > >Chapters > > > 3. ``-`` for sections:: > >Section >--- > > 4. ``~`` for subsections:: > >Subsection >~~ Thanks for the help! I'm new to the sphix markup language and appreciate your advice. I'll reread that doc-guide. We'll address the issues on our next posting once the marge window closes. thanks again for the reviews! --mark > > > diff --git a/Documentation/vpu/xlink-pcie.rst > > b/Documentation/vpu/xlink-pcie.rst > > new file mode 100644 > > index ..bc64b566989d > > --- /dev/null > > +++ b/Documentation/vpu/xlink-pcie.rst > > @@ -0,0 +1,91 @@ > > +.. SPDX-License-Identifier: GPL-2.0-only > > + > > +Kernel driver: xlink-pcie driver > > + > > +Supported chips: > > + * Intel Edge.AI Computer Vision platforms: Keem Bay > > +Suffix: Bay > > +Slave address: 6240 > > +Datasheet: Publicly available at Intel > > + > > +Author: Srikanth Thokala srikanth.thok...@intel.com > > + > > +- > > +Introduction: > > No colon at end of chapter/section headings. > > > +- > > +The xlink-pcie driver in linux-5.4 provides transport layer implementation > > for > > Linux 5.4 (?) > > > +the data transfers to support xlink protocol subsystem communication with > > the > > Xlink > > > +peer device. i.e, between remote host system and the local Keem Bay device. > > device, i.e., between the remote host system and > > > + > > +The Keem Bay device is an ARM based SOC that includes a vision processing > > ARM-based > > > +unit (VPU) and deep learning, neural network core in the hardware. > > +The xlink-pcie driver exports a functional device endpoint to the Keem Bay > > device > > +and supports two-way communication with peer device. > > with the peer device. > > > + > > + > > +High-level architecture: > > + > > +Remote Host: IA CPU > > +Local Host: ARM CPU (Keem Bay):: > > + > > + > > ++ > > +| Remote Host IA CPU | | Local Host ARM CPU (Keem > > Bay) | | > > + > > +==+=+===+===+ > > +| User App| | User App > > | | > > + > > +--+-+---+---+ > > +| XLink UAPI | | XLink UAPI > > | | > > + > > +--+-+---+---+ > > +| XLink Core | | XLink Core > > | | > > + > > +--+-+---+---+ > > +| XLink PCIe | | XLink PCIe > > | | > > + > > +--+-+---+---+ > > +| XLink-PCIe Remote Host driver | | XLink-PCIe Local Host > > driver | | > > + > > +--+-+---+---+ > > + > > |-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:|:|:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:-:| > > + > > +---
Re: linux-next: manual merge of the drm tree with the crypto tree
On Tue, Dec 15, 2020 at 10:58:52AM +0100, Geert Uytterhoeven wrote:
> On Mon, Dec 14, 2020 at 2:44 PM Stephen Rothwell
> wrote:
> > Today's linux-next merge of the drm tree got a conflict in:
> >
> > MAINTAINERS
> >
> > between commit:
> >
> > 885743324513 ("crypto: keembay - Add support for Keem Bay OCS AES/SM4")
> >
> > from the crypto tree and commit:
> >
> > ed794057b052 ("drm/kmb: Build files for KeemBay Display driver")
> >
> > from the drm tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging. You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
> >
> > --
> > Cheers,
> > Stephen Rothwell
> >
> > diff --cc MAINTAINERS
> > index 3b358262de8f,eb18459c1d16..
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
>
> > @@@ -8985,16 -8962,13 +8993,23 @@@ M: Deepak Saxena > S:Maintained
> > F:drivers/char/hw_random/ixp4xx-rng.c
> >
> > + INTEL KEEMBAY DRM DRIVER
>
> Is it KEEMBAY?
>
> > + M:Anitha Chrisanthus
> > + M:Edmund Dea
> > + S:Maintained
> > + F:Documentation/devicetree/bindings/display/intel,kmb_display.yaml
>
> I was just going to comment about "intel,kmb_*" vs. "intel,keembay-*", until
> I noticed intel,kmb_display.yaml does not exist, but is called
> Documentation/devicetree/bindings/display/intel,keembay-display.yaml
> in next-20201214.
>
> > + F:drivers/gpu/drm/kmb/
> > +
> > +INTEL KEEM BAY OCS AES/SM4 CRYPTO DRIVER
>
> or KEEM BAY?
>
> Or Keem Bay? Keembay? KeemBay?
It should be Keem Bay. I googled sandybridge, ivybridge, baytrail,
cherrytrail, medfield and merrifiled and for the *bridge and *trail products
the words are split up and capitalized. For the *fields they are one-word.
We'll update the KEEMBAY,KeemBay, KEEM BAY instances to Keem Bay to mimic SDB,
IVB, BYT and CHT since those are the majority. I'm not sure I'm going to rename
the file names however but, within the files wherever we talk about Keem Bay we
will use "Keem Bay" consistently.
Sorry for the variances,
--mark
Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
On Mon, Dec 07, 2020 at 02:31:37PM -0600, Jassi Brar wrote: > On Mon, Dec 7, 2020 at 12:43 PM Daniele Alessandrelli > wrote: > > > > Hi Rob, > > > > Thanks for the feedback. > > > > On Mon, 2020-12-07 at 10:01 -0600, Rob Herring wrote: > > > On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgr...@linux.intel.com wrote: > > > > From: Daniele Alessandrelli > > > > > > > > Add DT binding documentation for the Intel Keem Bay IPC driver, which > > > > enables communication between the Computing Sub-System (CSS) and the > > > > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem > > > > Bay. > > > > > > > > [cut] > > > > > > + > > > > +description: > > > > + The Keem Bay IPC driver enables Inter-Processor Communication (IPC) > > > > with the > > > > + Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code > > > > named > > > > + Keem Bay. > > > > > > Sounds like a mailbox. > > > > We did consider using the mailbox framework, but eventually decided > > against it; mainly because of the following two reasons: > > > > 1. The channel concept in the Mailbox framework is different than the > >channel concept in Keem Bay IPC: > > > >a. My understanding is that Mailbox channels are meant to be SW > > representation of physical HW channels, while in Keem Bay IPC > > channels are software abstractions to achieve communication > > multiplexing over a single HW link > > > In mailbox api, that would be a physical channel shared between various > clients. > > >b. Additionally, Keem Bay IPC has two different classes of channels > > (high-speed channels and general-purpose channels) that need to > > access the same HW link with different priorities. > > > If the priorities are hard (programmed into some register), you could > do that via dt during channel population. > If they are soft, that would be handled in the shared channel implementation. > > > 2. The blocking / non-blocking TX behavior of mailbox channels is > >defined at channel creation time (by the tx_block value of the > >mailbox client passed to mbox_request_channel(); > > > No, that is checked at mbox_send_message() > > > my understanding > >is that the tx_block value cannot be modified after the channel is > >created), > > > Again no. If you don't queue more than one message at any time you can > change it between transfers. To be safe you can always change it > between channel release - request calls. > > > while in Keem Bay IPC the same channel can be used for > >both blocking and non-blocking TX (behavior is controlled by the > >timeout argument passed to keembay_ipc_send()). > > > > Having said that, I guess that it could be possible to create a Mailbox > > driver implementing the core communication mechanism used by the Keem > > Bay IPC and then build our API around it (basically having two > > drivers). But I'm not sure that would make the code simpler or easier > > to maintain. Any thoughts on this? > > > I think so. Most of KeemBay specific behaviour would be implemented in > the shared channel above the mailbox api. Quick question. By "I think so" do you mean that you feel the keem bay IPC code will be simpler and easier to maintain if we make yet another driver at the Keem Bay IPC driver sits on top off? Or, the current implementation would be simpler if we rework the implementation to use the mailbox api? I'm just now ramping on the common mailbox framework so that may be a dumb question. I would like to be confident reworking the driver to use the mailbox api will not lead to blocking issues before we start that. thanks! --mark
Re: [PATCH 22/22] xlink-core: factorize xlink_ioctl function by creating sub-functions for each ioctl command
On Sun, Dec 06, 2020 at 07:05:34PM -0800, Joe Perches wrote:
> On Tue, 2020-12-01 at 14:35 -0800, mgr...@linux.intel.com wrote:
> > From: Seamus Kelly
> >
> > Refactor the too large IOCTL function to call helper functions.
>
> This should not be sent as a known poor patch as patch 21 of 22
> and then updated in patch 22 of 22 with a better style.
>
> This should be sent in the as desired final form the first time
> so that people don't give you useless notes.
We will re-work the changes to better integrate this one in the earlier
changes.
--mark
>
> > @@ -342,427 +323,84 @@ static int kmb_xlink_remove(struct platform_device
> > *pdev)
> > * IOCTL function for User Space access to xlink kernel functions
> > *
> > */
> > +int ioctl_connect(unsigned long arg);
> >
> >
> > static long xlink_ioctl(struct file *file, unsigned int cmd, unsigned long
> > arg)
> > {
> > - struct xlink_handle devh= {0};
> > - struct xlinkopenchannel op = {0};
> > - struct xlinkwritedata wr = {0};
> > - struct xlinkreaddatard = {0};
> > - struct xlinkreadtobufferrdtobuf = {0};
> > - struct xlinkconnect con = {0};
> > - struct xlinkrelease rel = {0};
> > - struct xlinkstartvpustartvpu = {0};
> > - struct xlinkcallbackcb = {0};
> > - struct xlinkgetdevicename devn= {0};
> > - struct xlinkgetdevicelist devl= {0};
> > - struct xlinkgetdevicestatus devs= {0};
> > - struct xlinkbootdevice boot= {0};
> > - struct xlinkresetdevice res = {0};
> > - struct xlinkdevmode devm= {0};
> > - struct xlinkregdevevent regdevevent = {0};
> > - u32 sw_device_id_list[XLINK_MAX_DEVICE_LIST_SIZE];
> > - char name[XLINK_MAX_DEVICE_NAME_SIZE];
> > - int interface = NULL_INTERFACE;
> > - u32 device_status = 0;
> > - u32 num_devices = 0;
> > - u32 device_mode = 0;
> > - u32 num_events = 0;
> > - char filename[64];
> > - u32 *ev_list;
> > - u8 reladdr;
> > - u8 *rdaddr;
> > - u32 size;
> > int rc;
> >
> >
> > switch (cmd) {
> > case XL_CONNECT:
> > - if (copy_from_user(, (void __user *)arg,
> > - sizeof(struct xlinkconnect)))
> > - return -EFAULT;
> > - if (copy_from_user(, (void __user *)con.handle,
> > - sizeof(struct xlink_handle)))
> > - return -EFAULT;
> > - rc = xlink_connect();
> > - if (rc == X_LINK_SUCCESS) {
> > - if (copy_to_user((void __user *)con.handle,
> > -, sizeof(struct xlink_handle)))
> > - return -EFAULT;
> > - }
> > - if (copy_to_user((void __user *)con.return_code, (void *),
> > -sizeof(rc)))
> > - return -EFAULT;
> > + rc = ioctl_connect(arg);
> > break;
>
> etc...
>
Re: [PATCH 15/22] xlink-ipc: Add xlink ipc device tree bindings
On Mon, Dec 07, 2020 at 09:58:14AM -0600, Rob Herring wrote: > On Tue, 01 Dec 2020 14:35:04 -0800, mgr...@linux.intel.com wrote: > > From: Seamus Kelly > > > > Add device tree bindings for the xLink IPC driver which enables xLink to > > control and communicate with the VPU IP present on the Intel Keem Bay > > SoC. > > > > Cc: devicet...@vger.kernel.org > > Reviewed-by: Mark Gross > > Signed-off-by: Seamus Kelly > > Signed-off-by: Ryan Carnaghi > > --- > > .../misc/intel,keembay-xlink-ipc.yaml | 49 +++ > > 1 file changed, 49 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > ./Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml:21:9: > [warning] wrong indentation: expected 10 but found 8 (indentation) > > dtschema/dtc warnings/errors: > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml: > 'additionalProperties' is a required property > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml: > ignoring, error in schema: > warning: no schema found in file: > ./Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml > > > See https://patchwork.ozlabs.org/patch/1409186 > > The base for the patch is generally the last rc1. Any dependencies > should be noted. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit. Thank you! will do. --mark
Re: [PATCH 04/22] dt-bindings: Add bindings for Keem Bay VPU IPC driver
On Mon, Dec 07, 2020 at 09:57:26AM -0600, Rob Herring wrote: > On Tue, 01 Dec 2020 14:34:53 -0800, mgr...@linux.intel.com wrote: > > From: Paul Murphy > > > > Add DT bindings documentation for the Keem Bay VPU IPC driver. > > > > Cc: devicet...@vger.kernel.org > > Reviewed-by: Mark Gross > > Signed-off-by: Paul Murphy > > Co-developed-by: Daniele Alessandrelli > > Signed-off-by: Daniele Alessandrelli > > --- > > .../soc/intel/intel,keembay-vpu-ipc.yaml | 151 ++ > > 1 file changed, 151 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > ./Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml:21:9: > [warning] wrong indentation: expected 10 but found 8 (indentation) > > dtschema/dtc warnings/errors: > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml: > 'additionalProperties' is a required property > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml: > ignoring, error in schema: > warning: no schema found in file: > ./Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml > > > See https://patchwork.ozlabs.org/patch/1409183 > > The base for the patch is generally the last rc1. Any dependencies > should be noted. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit. Thanks! I will fix on the next update. --mark
Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
On Mon, Dec 07, 2020 at 10:01:52AM -0600, Rob Herring wrote: > On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgr...@linux.intel.com wrote: > > From: Daniele Alessandrelli > > > > Add DT binding documentation for the Intel Keem Bay IPC driver, which > > enables communication between the Computing Sub-System (CSS) and the > > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem > > Bay. > > > > Cc: devicet...@vger.kernel.org > > Reviewed-by: Mark Gross > > Signed-off-by: Daniele Alessandrelli > > --- > > .../bindings/soc/intel/intel,keembay-ipc.yaml | 63 +++ > > 1 file changed, 63 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml > > b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml > > new file mode 100644 > > index ..6e21c54d8f34 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml > > @@ -0,0 +1,63 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +# Copyright (C) 2020 Intel Corporation > > +%YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/soc/intel/intel,keembay-ipc.yaml#; > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#; > > + > > +title: Keem Bay IPC > > + > > +maintainers: > > + - Daniele Alessandrelli > > + > > +description: > > + The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with > > the > > + Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named > > + Keem Bay. > > Sounds like a mailbox. Its a multi-channel mailbox like thing with priority channel support. > > What's the relationship between this and the xlink thing? Xlink is a SW abstraction to allow multiple user access to the VPU as well as enabling use cases where a Keem Bay is used as an accelerator add in card as well as a simple SBC type of design. The xlink stuff sits on top of the IPC stuff. --mark
Re: [PATCH 03/22] keembay-ipc: Add Keem Bay IPC module
On Sat, Dec 05, 2020 at 04:37:25PM +, Gross, Mark wrote: > > > > -Original Message- > > From: Greg KH > > Sent: Saturday, December 5, 2020 12:40 AM > > To: mark gross > > Cc: markgr...@kernel.org; a...@arndb.de; b...@suse.de; > > damien.lem...@wdc.com; dragan.cve...@xilinx.com; cor...@lwn.net; > > leonard.cres...@nxp.com; palmerdabb...@google.com; > > paul.walms...@sifive.com; peng@nxp.com; robh...@kernel.org; > > shawn...@kernel.org; linux-kernel@vger.kernel.org; Alessandrelli, Daniele > > ; Iyer, Sundar > > Subject: Re: [PATCH 03/22] keembay-ipc: Add Keem Bay IPC module > > > > On Fri, Dec 04, 2020 at 07:35:17PM -0800, mark gross wrote: > > > On Wed, Dec 02, 2020 at 08:01:18PM +0100, Greg KH wrote: > > > > On Wed, Dec 02, 2020 at 09:42:00AM -0800, mark gross wrote: > > > > > On Wed, Dec 02, 2020 at 07:16:20AM +0100, Greg KH wrote: > > > > > > On Tue, Dec 01, 2020 at 02:34:52PM -0800, mgr...@linux.intel.com > > > > > > wrote: > > > > > > > --- a/MAINTAINERS > > > > > > > +++ b/MAINTAINERS > > > > > > > @@ -8955,6 +8955,14 @@ M: Deepak Saxena > > > > > > > S: Maintained > > > > > > > F: drivers/char/hw_random/ixp4xx-rng.c > > > > > > > > > > > > > > +INTEL KEEM BAY IPC DRIVER > > > > > > > +M: Daniele Alessandrelli > > > > > > > +M: Mark Gross > > > > > > > +S: Maintained > > > > > > > +F: > > > > > > > Documentation/devicetree/bindings/soc/intel/intel,keembay- > > ipc.yaml > > > > > > > +F: drivers/soc/intel/keembay-ipc.c > > > > > > > +F: include/linux/soc/intel/keembay-ipc.h > > > > > > > > > > > > Sad that Intel is not going to actually pay you all to do this > > > > > > maintenance work for a brand new subsystem you are wanting to > > > > > > add to the tree :( > > > > > I thought adding my name to these maintainer items would help with > > > > > continuity as the individual engineers tend to move on to other > > > > > things over > > time. > > > > > > > > > > While I'm paid for a number of things at intel this is one of > > > > > them. My role is as stable as I choose it to be at the point I'm > > > > > at in my Intel career and the business unit I'm now part of. We > > > > > can leave my name off if that would be better. > > > > > > > > > > Even if I'm not a VPU IP domain expert like Daniele is I can still > > > > > chase down the experts as needed after Daniele grows into other > > > > > things over > > time. > > > > > > > > I'm not objecting to your, or anyone else's name on this at all. > > > > I'm just asking about Intel's support for this new codebase being added. > > > > Having a new subsystem from a major company and not have someone > > > > paid to actually maintain it seems really odd to me. > > > > > > > > That's all. If that's Intel's stance, that's fine, just wanted to > > > > clarify it is correct as I know some people at Intel have been > > > > confused recently about just what the S: field means. > > > I've been following up on whether the status field should be > > > "Supported" or "Maintained" at this time. For this current > > > instantiation of the VPU enabling under review here I think Maintained > > > most appropriate. There are a good number of people who look after it. > > > > > > However; I have learned that the instantiations of the VPU after keem > > > bay and its follow on SoC will include an evolution of this stack and > > > between now and when those get close to landing that evolved version will > > become "Supported". > > > > > > Given this, would it be more appropriate to put this stack into > > > staging for a while? > > > > drivers/staging/ is for code that for some reason is not good enough to be > > merged > > to the "right" place in the kernel tree, and you need community help to get > > it > > cleaned up because you can not do it yourself. > > > > Is that the case here? If not, then no, it should not go into > > drivers/staging/. > That is not the case here. Lets proceed as we are on this then. > I guess technically we have number of engineers paid to look after this version of the VPU enabling. I guess I'm over thinking things. I'll change it S: record from Maintained to Supported on the next version. --mark
Re: [PATCH 03/22] keembay-ipc: Add Keem Bay IPC module
On Wed, Dec 02, 2020 at 08:01:18PM +0100, Greg KH wrote: > On Wed, Dec 02, 2020 at 09:42:00AM -0800, mark gross wrote: > > On Wed, Dec 02, 2020 at 07:16:20AM +0100, Greg KH wrote: > > > On Tue, Dec 01, 2020 at 02:34:52PM -0800, mgr...@linux.intel.com wrote: > > > > --- a/MAINTAINERS > > > > +++ b/MAINTAINERS > > > > @@ -8955,6 +8955,14 @@ M: Deepak Saxena > > > > S: Maintained > > > > F: drivers/char/hw_random/ixp4xx-rng.c > > > > > > > > +INTEL KEEM BAY IPC DRIVER > > > > +M: Daniele Alessandrelli > > > > +M: Mark Gross > > > > +S: Maintained > > > > +F: > > > > Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml > > > > +F: drivers/soc/intel/keembay-ipc.c > > > > +F: include/linux/soc/intel/keembay-ipc.h > > > > > > Sad that Intel is not going to actually pay you all to do this > > > maintenance work for a brand new subsystem you are wanting to add to the > > > tree :( > > I thought adding my name to these maintainer items would help with > > continuity > > as the individual engineers tend to move on to other things over time. > > > > While I'm paid for a number of things at intel this is one of them. My > > role is > > as stable as I choose it to be at the point I'm at in my Intel career and > > the > > business unit I'm now part of. We can leave my name off if that would be > > better. > > > > Even if I'm not a VPU IP domain expert like Daniele is I can still chase > > down > > the experts as needed after Daniele grows into other things over time. > > I'm not objecting to your, or anyone else's name on this at all. I'm > just asking about Intel's support for this new codebase being added. > Having a new subsystem from a major company and not have someone paid to > actually maintain it seems really odd to me. > > That's all. If that's Intel's stance, that's fine, just wanted to > clarify it is correct as I know some people at Intel have been confused > recently about just what the S: field means. I've been following up on whether the status field should be "Supported" or "Maintained" at this time. For this current instantiation of the VPU enabling under review here I think Maintained most appropriate. There are a good number of people who look after it. However; I have learned that the instantiations of the VPU after keem bay and its follow on SoC will include an evolution of this stack and between now and when those get close to landing that evolved version will become "Supported". Given this, would it be more appropriate to put this stack into staging for a while? --mark
Re: [PATCH 03/22] keembay-ipc: Add Keem Bay IPC module
On Wed, Dec 02, 2020 at 07:16:20AM +0100, Greg KH wrote: > On Tue, Dec 01, 2020 at 02:34:52PM -0800, mgr...@linux.intel.com wrote: > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -8955,6 +8955,14 @@ M: Deepak Saxena > > S: Maintained > > F: drivers/char/hw_random/ixp4xx-rng.c > > > > +INTEL KEEM BAY IPC DRIVER > > +M: Daniele Alessandrelli > > +M: Mark Gross > > +S: Maintained > > +F: Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml > > +F: drivers/soc/intel/keembay-ipc.c > > +F: include/linux/soc/intel/keembay-ipc.h > > Sad that Intel is not going to actually pay you all to do this > maintenance work for a brand new subsystem you are wanting to add to the > tree :( I thought adding my name to these maintainer items would help with continuity as the individual engineers tend to move on to other things over time. While I'm paid for a number of things at intel this is one of them. My role is as stable as I choose it to be at the point I'm at in my Intel career and the business unit I'm now part of. We can leave my name off if that would be better. Even if I'm not a VPU IP domain expert like Daniele is I can still chase down the experts as needed after Daniele grows into other things over time. > > Does this mean you all will have to do it on your own time and not as > part of your work at Intel? If so, why not just use your personal email > addresses instead to make it a bit more obvious? nah, as I've been getting older I've been getting more stingy with my me time with my non-tech hobbies. When I do retire (or whatever) from Intel we'll need to remove my name for the stuff I'll not be getting paid for any more. --mark
Re: [PATCH 07/22] misc: xlink-pcie: lh: Add PCIe EPF driver for Local Host
On Tue, Dec 01, 2020 at 06:54:28PM +0100, Greg Kroah-Hartman wrote: > On Tue, Dec 01, 2020 at 09:45:43AM -0800, mark gross wrote: > > On Tue, Dec 01, 2020 at 11:13:05AM +0100, Greg Kroah-Hartman wrote: > > > On Mon, Nov 30, 2020 at 03:06:52PM -0800, mgr...@linux.intel.com wrote: > > > > From: Srikanth Thokala > > > > > > > > Add PCIe EPF driver for local host (lh) to configure BAR's and other > > > > HW resources. Underlying PCIe HW controller is a Synopsys DWC PCIe core. > > > > > > > > Cc: Derek Kiernan > > > > Cc: Dragan Cvetic > > > > Cc: Arnd Bergmann > > > > Cc: Greg Kroah-Hartman > > > > Reviewed-by: Mark Gross > > > > Signed-off-by: Srikanth Thokala > > > > > > > > > > > > Again, you sent this twice? And it never got to lore.kernel.org nor the > > > mailing lists? > > In my excitement of sorting out my MTA misconfiguration work around I > > actually > > hit entry without including the magic fix > > "--envelope-sender=mgr...@linux.intel.com" in my git send-email command > > line. > > > > > > And I can't see a 00/XX email anywhere explaining this, and I didn't get > > > the whole series? > > https://lore.kernel.org/lkml/20201130230707.46351-1-mgr...@linux.intel.com/ > > > > Did I mess up on who all should get the 00/xx email? > > Yes, you need to include the people that you want to review the > individual patches, otherwise we have no idea :( > Ok, I will reset and try again. I appoligize for this false start. I'll resend after I trim the CC list based on Dragon's feedback. --mark
Re: [PATCH 00/22] Intel Vision Processing Unit base enabling part 1
On Tue, Dec 01, 2020 at 11:14:12AM +0100, Greg KH wrote: > On Mon, Nov 30, 2020 at 03:06:45PM -0800, mgr...@linux.intel.com wrote: > > From: mark gross > > > > The Intel Vision Processing Unit (VPU) is an IP block that is showing up for > > the first time as part of the Keem Bay SOC. Keem Bay is a quad core A53 Arm > > SOC. It is designed to be used as a stand alone SOC as well as in an PCIe > > Vision Processing accelerator add in card. > > > > This part 1 of the patches make up the base or core of the stack needed to > > enable both use cases for the VPU. > > > > Part 2 includes 11 more patches that depend on part 1. Those should be > > ready > > in a couple of weeks or less. > > > > I am trying something a bit new with this sequence where I've been working > > with > > the driver developers as a "pre-maintainer" reviewing and enforcing the > > kernel > > expectations as I understand them. Its taken a couple of months to get this > > code to the point I feel its ready for public posting. My goal is to make > > sure > > it meets expectations for quality and compliance with kernel expectations > > and > > there will be mostly technical / design issues to talk about. > > > > Thanks for looking at these and providing feedback. > > > > --mark > > p.s. I have had a problem my MTA configuration between mutt and git > > send-email > > where I was using msmpt to send from mutt (because 15+ years ago its the > > first > > way I got to work and never changed) while my worstation MTA that git > > send-email uses was un-configured resulting in my return-path naming my > > workstion withing the firewall. I suck at email administration. > > > > I appologies for the multiple copies. > > Ah, here's the full set of patches... > > But, you didn't cc: everyone on them, some of us just got a partial set > of patches, why? Because I thought ccing everyone on all the changes was not what was expected. Does the DT guy want to see all the non-DT changes? this is my first time sending a big patch set that crosses subsystems like this. I'm not sure what the preferd way to do things so I used get_maintainer.pl on each patch to add thime in the cc: tags just above the signed off's. Should I have simply concatinated the list of mainttainers and CC them all on all the patches? > > still confused, its my fault. --mark
Re: [PATCH 07/22] misc: xlink-pcie: lh: Add PCIe EPF driver for Local Host
On Tue, Dec 01, 2020 at 11:13:05AM +0100, Greg Kroah-Hartman wrote: > On Mon, Nov 30, 2020 at 03:06:52PM -0800, mgr...@linux.intel.com wrote: > > From: Srikanth Thokala > > > > Add PCIe EPF driver for local host (lh) to configure BAR's and other > > HW resources. Underlying PCIe HW controller is a Synopsys DWC PCIe core. > > > > Cc: Derek Kiernan > > Cc: Dragan Cvetic > > Cc: Arnd Bergmann > > Cc: Greg Kroah-Hartman > > Reviewed-by: Mark Gross > > Signed-off-by: Srikanth Thokala > > > > Again, you sent this twice? And it never got to lore.kernel.org nor the > mailing lists? In my excitement of sorting out my MTA misconfiguration work around I actually hit entry without including the magic fix "--envelope-sender=mgr...@linux.intel.com" in my git send-email command line. > > And I can't see a 00/XX email anywhere explaining this, and I didn't get > the whole series? https://lore.kernel.org/lkml/20201130230707.46351-1-mgr...@linux.intel.com/ Did I mess up on who all should get the 00/xx email? > > Something is really wrong on your end with your email client > configuration, please fix that up and send this so that we can actually > see it all, and know what the heck we are supposed to be reviewing... Yes, I think I have it fixed, or worked around now. sorry, --mark
Re: [PATCH v5] Introduce support for Systems Management Driver over WMI for Dell Systems
On Thu, Oct 01, 2020 at 11:26:36AM +0200, Hans de Goede wrote:
> Hi,
>
> On 9/30/20 11:02 PM, Limonciello, Mario wrote:
> > > > + possible_values:A file that can be read to
> > > > obtain the possible
> > > > + values of the . Values
> > > > are separated using
> > > > + semi-colon (``;``).
> > > why not use set notation from math classes assuming intergers? i.e.
> > > (a, b) all integers beween a and b but not including a or b (open set)
> > > or
> > > [a, b] all integerger betwen a and b including and b? (closed set)
> > >
> > > Anyway its ambiguous if the the extremes are included in the set of
> > > possible
> > > values as written.
> > >
> >
> > Enumeration attributes mean that there are fixed values, specifically not
> > integers.
> > Integers are in the "integer" type and explained below.
> >
> > An example value that would be seen here is possible_values:
> >
> > Enabled;Disabled;
>
> That might not be the best example, because in that case arguably we
> could export it as a boolean type (except that the WMI interface does
> not give us boolean as an explicit / separate type).
>
> Mark these enum attributes are really like enums in C, so we
> have a fixed set of possible values which are described by
> strings, since using integers for it makes no sense from a human
> interaction pov. E.g. on the Lenovo X1C8 I have some attributes
> have the following possible value sets:
>
> Package (0x03)
> {
> "High",
> "Normal",
> "Silent"
> },
>
> Package (0x02)
> {
> "LCD",
> "ExternalDisplay"
> },
>
> Package (0x02)
> {
> "Independent",
> "Synchronized"
> },
>
> I hope this helps clarify things.
It does. Please ignore my comment on this topic then.
thanks
--mark
Re: [PATCH v5] Introduce support for Systems Management Driver over WMI for Dell Systems
On Wed, Sep 30, 2020 at 09:02:38PM +, Limonciello, Mario wrote: > > > + possible_values:A file that can be read to obtain the > > > possible > > > + values of the . Values are > > > separated using > > > + semi-colon (``;``). > > why not use set notation from math classes assuming intergers? i.e. > > (a, b) all integers beween a and b but not including a or b (open set) > > or > > [a, b] all integerger betwen a and b including and b? (closed set) > > > > Anyway its ambiguous if the the extremes are included in the set of possible > > values as written. > > > > Enumeration attributes mean that there are fixed values, specifically not > integers. > Integers are in the "integer" type and explained below. > > An example value that would be seen here is possible_values: > > Enabled;Disabled; I'm ok with this > > > > + > > > + security_area_size = calculate_security_buffer(); > > > + buffer_size = security_area_size + integer_area_size; > > > + buffer = kzalloc(buffer_size, GFP_KERNEL); > > > + if (!buffer) > > > + return -ENOMEM; > > if you hit this error return I think you will leak the wmi_priv.mutex lock. > > I think this is a bug. > > Yes, thanks this is a great finding. > Team will fix in v6 after we have Hans' feedback for v5. Yay! It makes me feel useful to catch stuff like this. thanks! --mark
Re: [PATCH v5] Introduce support for Systems Management Driver over WMI for Dell Systems
one mutex bug and a couple of nits that could be ignored.
On Tue, Sep 29, 2020 at 08:25:21AM +0530, Divya Bharathi wrote:
> The Dell WMI Systems Management Driver provides a sysfs
> interface for systems management to enable BIOS configuration
> capability on certain Dell Systems.
>
> This driver allows user to configure Dell systems with a
> uniform common interface. To facilitate this, the patch
> introduces a generic way for driver to be able to create
> configurable BIOS Attributes available in Setup (F2) screen.
>
> Cc: Hans de Goede
> Cc: Andy Shevchenko
> Cc: mark gross
>
> Co-developed-by: Mario Limonciello
> Signed-off-by: Mario Limonciello
> Co-developed-by: Prasanth KSR
> Signed-off-by: Prasanth KSR
> Signed-off-by: Divya Bharathi
> ---
>
> Changes from v4 to v5:
> - Correct Kconfig description to not use "WMI" twice.
> - s/is_authentication_set/is_enabled/
> - Specify that all attributes are optional and take UTF8
> - Change authentication type to "role" and "mechanism"
> - Explain what semicolons are
> - Adjust whitespace of documentation
>
> Changes from v3 to v4:
> - Create a firmware-attributes class and tie ksets to a virtual device in it
> - Make modifier and value_modifier "dell only" attributes.
> - Correct some errors caught by kernel build bot around missing prototypes
> - Remove mutexes from populate_* functions and put in
> init_dell_bios_attrib_wmi instead
> - Move all code into a subdirectory drivers/platform/x86/dell-wmi-sysman and
> remove dell-wmi-*
>prefix on files
> - Move all data structures into shared struct
> - In alloc functions instead of kzalloc use kcalloc and check that there is
> no overflow
>+ Same check for other alloc_foo-data functions
> - populate_*: Move sysfs_create_group to end of the function to prevent
> race conditions
> - Save kernel object into each data instance and only remove that rather
> than sysfs_remove_group
> - Document in sysfs file what causes change uevents to come through
> - Only notify with change uevent one time on multiple settings modifications
> - Adjust lots of string handling
> - Make more objects static
> - Various whitespace corrections
> - Document map_wmi_error properly
> - Bump version to 5.11 (February 2021)
>
> Changes from v2 to v3:
> - Fix a possible NULL pointer error in init
> - Add missing newlines to all dev_err/dev_dbg/pr_err/pr_debug statements
> - Correct updating passwords when both Admin and System password are set
> - Correct the WMI driver name
> - Correct some namespace clashing when compiled into the kernel (Reported by
> Mark Gross)
> - Correct some comment typos
> - Adopt suggestions made by Hans:
>+ Use single global mutex
>+ Clarify reason for uevents with a comment
>+ Remove functions for set and get current password
>+ Rename lower_bound to min_value and upper_bound to max_value
>+ Rename possible_value to possible_values
>+ Remove references to float
>+ Build a separate passwords directory again since it behaves differently
> from the other
> attributes
>+ Move more calls from pr_err -> dev_err
> - Documentation cleanups (see v2 patch feedback)
>+ Grouping types
>+ Syntax of `modifier` output
>
> Changes from v1 to v2:
> - use pr_fmt instead of pr_err(DRIVER_NAME
> - re-order variables reverse xmas tree order
> - correct returns of -1 to error codes
> - correct usage of {} on some split line statements
> - Refine all documentation deficiencies suggested by Hans
> - Merge all attributes to a single directory
> - Overhaul WMI interface interaction as suggested by Hans
>* Move WMI driver registration to start of module
>* Remove usage of lists that only use first entry for WMI interfaces
>* Create a global structure shared across interface source files
>* Make get_current_password function static
>* Remove get_pending changes function, shared across global structure now.
> - Overhaul use of mutexes
>* Make kset list mutex shared across source files
>* Remove unneeded dell-wmi-sysman call_mutex
>* Keep remaining call_mutexes in WMI functions
> - Move security area calculation into a function
> - Use NLS helper for utf8->utf16 conversion
>
> .../testing/sysfs-class-firmware-attributes | 219 +++
> MAINTAINERS | 8 +
> drivers/platform/x86/Kconfig | 12 +
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/dell-wmi-sysman/Makefile | 8 +
> .../x86/dell-wmi-sysman/biosattr-interface.c | 199 ++
> .../x86/dell-wmi-sys
Re: [PATCH v1 net] net: stmmac: removed enabling eee in EEE set callback
Acked-by: Mark Gross
On Wed, Sep 23, 2020 at 04:56:14PM +0800, Voon Weifeng wrote:
> EEE should be only be enabled during stmmac_mac_link_up() when the
> link are up and being set up properly. set_eee should only do settings
> configuration and disabling the eee.
>
> Without this fix, turning on EEE using ethtool will return
> "Operation not supported". This is due to the driver is in a dead loop
> waiting for eee to be advertised in the for eee to be activated but the
> driver will only configure the EEE advertisement after the eee is
> activated.
>
> Ethtool should only return "Operation not supported" if there is no EEE
> capbility in the MAC controller.
>
> Fixes: 8a7493e58ad6 ("net: stmmac: Fix a race in EEE enable callback")
>
> Signed-off-by: Voon Weifeng
> ---
> .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 15 ---
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> index ac5e8cc5fb9f..430a4b32ec1e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> @@ -675,23 +675,16 @@ static int stmmac_ethtool_op_set_eee(struct net_device
> *dev,
> struct stmmac_priv *priv = netdev_priv(dev);
> int ret;
>
> - if (!edata->eee_enabled) {
> + if (!priv->dma_cap.eee)
> + return -EOPNOTSUPP;
> +
> + if (!edata->eee_enabled)
> stmmac_disable_eee_mode(priv);
> - } else {
> - /* We are asking for enabling the EEE but it is safe
> - * to verify all by invoking the eee_init function.
> - * In case of failure it will return an error.
> - */
> - edata->eee_enabled = stmmac_eee_init(priv);
> - if (!edata->eee_enabled)
> - return -EOPNOTSUPP;
> - }
>
> ret = phylink_ethtool_set_eee(priv->phylink, edata);
> if (ret)
> return ret;
>
> - priv->eee_enabled = edata->eee_enabled;
> priv->tx_lpi_timer = edata->tx_lpi_timer;
> return 0;
> }
> --
> 2.17.1
>
Re: [PATCH] platform/x86: fix kconfig dependency warning for LG_LAPTOP
Acked-by: mark gross
--mark
On Tue, Sep 15, 2020 at 12:09:23PM +0300, Necip Fazil Yildiran wrote:
> When LG_LAPTOP is enabled and NEW_LEDS is disabled, it results in the
> following Kbuild warning:
>
> WARNING: unmet direct dependencies detected for LEDS_CLASS
> Depends on [n]: NEW_LEDS [=n]
> Selected by [y]:
> - LG_LAPTOP [=y] && X86 [=y] && X86_PLATFORM_DEVICES [=y] && ACPI [=y] &&
> ACPI_WMI [=y] && INPUT [=y]
>
> The reason is that LG_LAPTOP selects LEDS_CLASS without depending on or
> selecting NEW_LEDS while LEDS_CLASS is subordinate to NEW_LEDS.
>
> Honor the kconfig menu hierarchy to remove kconfig dependency warnings.
>
> Fixes: dbf0c5a6b1f8 ("platform/x86: Add LG Gram laptop special features
> driver")
> Signed-off-by: Necip Fazil Yildiran
> ---
> drivers/platform/x86/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 40219bba6801..81f6079d08e6 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1112,6 +1112,7 @@ config LG_LAPTOP
> depends on ACPI_WMI
> depends on INPUT
> select INPUT_SPARSEKMAP
> + select NEW_LEDS
> select LEDS_CLASS
> help
>This driver adds support for hotkeys as well as control of keyboard
> --
> 2.25.1
>
Re: [PATCH] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360
On Wed, Sep 16, 2020 at 09:12:33PM +0200, Samuel Čavoj wrote:
> The UX360CA has a WMI device id 0x00060062, which reports whether the
> lid is flipped in tablet mode (1) or in normal laptop mode (0).
>
> This commit adds a quirk (quirk_asus_use_lid_flip_devid) for devices on
> which this WMI device should be used to figure out the SW_TABLET_MODE
> state, as opposed to the quirk_asus_use_kbd_dock_devid.
see Documentation/process/submitting-patches.rst
section2 the bit about "imperative mood".
--mark
>
> It is assumed other UX360* models have the same WMI device. As such, the
> quirk is applied to devices with DMI_MATCH(DMI_PRODUCT_NAME, "UX360").
> More devices with this feature need to be tested and added accordingly.
>
> The reason for using a whitelist via the quirk mechanism is that the new
> WMI device (0x00060062) is also present on some models which do not have
> a 360 degree hinge (at least FX503VD and GL503VD from Hans' DSTS
> collection) and therefore its presence cannot be relied on.
>
> This patch is a followup to "platform/x86: asus-wmi: Fix SW_TABLET_MODE
> always reporting 1 on many different models" by Hans de Goede.
>
> Signed-off-by: Samuel Čavoj
> Cc: Hans de Goede
> ---
> drivers/platform/x86/asus-nb-wmi.c | 14 +
> drivers/platform/x86/asus-wmi.c| 23 ++
> drivers/platform/x86/asus-wmi.h| 1 +
> include/linux/platform_data/x86/asus-wmi.h | 1 +
> 4 files changed, 39 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-nb-wmi.c
> b/drivers/platform/x86/asus-nb-wmi.c
> index 345bd224494b..ae5501e07712 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -119,6 +119,10 @@ static struct quirk_entry quirk_asus_use_kbd_dock_devid
> = {
> .use_kbd_dock_devid = true,
> };
>
> +static struct quirk_entry quirk_asus_use_lid_flip_devid = {
> + .use_lid_flip_devid = true,
> +};
> +
> static int dmi_matched(const struct dmi_system_id *dmi)
> {
> pr_info("Identified laptop model '%s'\n", dmi->ident);
> @@ -520,6 +524,16 @@ static const struct dmi_system_id asus_quirks[] = {
> },
> .driver_data = _asus_use_kbd_dock_devid,
> },
> + {
> + .callback = dmi_matched,
> + .ident = "ASUS ZenBook Flip UX360",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> + /* Match UX360* */
> + DMI_MATCH(DMI_PRODUCT_NAME, "UX360"),
> + },
> + .driver_data = _asus_use_lid_flip_devid,
> + },
> {},
> };
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index ae6289d37faf..a628a7d9e066 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -63,6 +63,7 @@ MODULE_LICENSE("GPL");
> #define NOTIFY_KBD_BRTTOGGLE 0xc7
> #define NOTIFY_KBD_FBM 0x99
> #define NOTIFY_KBD_TTP 0xae
> +#define NOTIFY_LID_FLIP 0xfa
>
> #define ASUS_WMI_FNLOCK_BIOS_DISABLEDBIT(0)
>
> @@ -375,6 +376,18 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
> }
> }
>
> + if (asus->driver->quirks->use_lid_flip_devid) {
> + result = asus_wmi_get_devstate_simple(asus,
> ASUS_WMI_DEVID_LID_FLIP);
> + if (result >= 0) {
> + input_set_capability(asus->inputdev, EV_SW,
> SW_TABLET_MODE);
> + input_report_switch(asus->inputdev, SW_TABLET_MODE,
> result);
> + } else if (result == -ENODEV) {
> + pr_err("This device has lid_flip quirk but got ENODEV
> checking it. This is a bug.");
> + } else {
> + pr_err("Error checking for lid-flip: %d\n", result);
> + }
> + }
> +
> err = input_register_device(asus->inputdev);
> if (err)
> goto err_free_dev;
> @@ -2127,6 +2140,16 @@ static void asus_wmi_handle_event_code(int code,
> struct asus_wmi *asus)
> return;
> }
>
> + if (asus->driver->quirks->use_lid_flip_devid && code ==
> NOTIFY_LID_FLIP) {
> + result = asus_wmi_get_devstate_simple(asus,
> ASUS_WMI_DEVID_LID_FLIP);
> +
> + if (result >= 0) {
> + input_report_switch(asus->inputdev, SW_TABLET_MODE,
> result);
> + input_sync(asus->inputdev);
> + }
> + return;
> + }
> +
> if (asus->fan_boost_mode_available && code == NOTIFY_KBD_FBM) {
> fan_boost_mode_switch_next(asus);
> return;
> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index 1a95c172f94b..b302415bf1d9 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -34,6 +34,7 @@ struct quirk_entry {
>
Re: [PATCH] platform/x86: thinkpad_acpi: initialize tp_nvram_state variable
Acked-by: mark gross
--mark
On Sun, Sep 13, 2020 at 12:02:03PM -0700, t...@redhat.com wrote:
> From: Tom Rix
>
> clang static analysis flags this represenative problem
> thinkpad_acpi.c:2523:7: warning: Branch condition evaluates
> to a garbage value
> if (!oldn->mute ||
> ^~~
>
> In hotkey_kthread() mute is conditionally set by hotkey_read_nvram()
> but unconditionally checked by hotkey_compare_and_issue_event().
> So the tp_nvram_state variable s[2] needs to be initialized.
>
> Fixes: 01e88f25985d ("ACPI: thinkpad-acpi: add CMOS NVRAM polling for hot
> keys (v9)")
> Signed-off-by: Tom Rix
> ---
> drivers/platform/x86/thinkpad_acpi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c
> index 47925c319d7b..24da8b6872f2 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -2573,7 +2573,7 @@ static void hotkey_compare_and_issue_event(struct
> tp_nvram_state *oldn,
> */
> static int hotkey_kthread(void *data)
> {
> - struct tp_nvram_state s[2];
> + struct tp_nvram_state s[2] = { 0 };
> u32 poll_mask, event_mask;
> unsigned int si, so;
> unsigned long t;
> --
> 2.18.1
>
Re: [PATCH] platform/x86: Add Driver to set up lid GPEs on MS Surface device
On Sat, Sep 12, 2020 at 12:46:30AM +0200, Maximilian Luz wrote: > On 9/12/20 12:10 AM, mark gross wrote: > > Surface devices are tablets with detachable keyboards. they don't really > > have a "lid" as the tablet is the "lid". > > The Surface Laptop series doesn't have a detachable keyboard, yet still > requires this. Arguably, the Surface Books are also more laptop than > tablet (at least that's the way I use mine...). Finally, on the actual > tablets (Surface Pro series) the lid switch detects when the keyboard > cover is opened (or at least that's what I have been told, I don't > own/have access to a Pro series device). > > Regardless of that, this patch is intended to provide the same behavior > as found on Windows, for all devices included in this patch, which is: > When you open the lid, or in case of the Pro series fold away the > keyboard cover, the device wakes from suspend/s2idle. Without this > patch, that doesn't work. > > > I'm just questioning if the creator of the device designed it the way they > > did > > maybe we should think twice about doing this. > > As far as I can tell, the intended behavior is to wake the device when > the lid is opened, which on the Laptops and Books is a more conventional > lid and on the Pros constitutes opening the cover. > > I'm open for any alternative though. > > Also please note that I've already sent a v2 of this patch with Andy's > comments addressed: https://lore.kernel.org/patchwork/patch/1303997/ never mind then. --mark
Re: [PATCH v2] Introduce support for Systems Management Driver over WMI for Dell Systems
On Fri, Sep 04, 2020 at 07:58:46PM +0530, Divya Bharathi wrote:
> The Dell WMI Systems Management Driver provides a sysfs
> interface for systems management to enable BIOS configuration
> capability on certain Dell Systems.
>
> This driver allows user to configure Dell systems with a
> uniform common interface. To facilitate this, the patch
> introduces a generic way for driver to be able to create
> configurable BIOS Attributes available in Setup (F2) screen.
>
> Cc: Hans de Goede
> Cc: Andy Shevchenko
>
> Co-developed-by: Mario Limonciello
> Signed-off-by: Mario Limonciello
> Co-developed-by: Prasanth KSR
> Signed-off-by: Prasanth KSR
> Signed-off-by: Divya Bharathi
> ---
>
> ChangeLog from v1 to v2:
> - use pr_fmt instead of pr_err(DRIVER_NAME
> - re-order variables reverse xmas tree order
> - correct returns of -1 to error codes
> - correct usage of {} on some split line statements
> - Refine all documentation deficiencies suggested by Hans
> - Merge all attributes to a single directory
> - Overhaul WMI interface interaction as suggested by Hans
>* Move WMI driver registration to start of module
>* Remove usage of lists that only use first entry for WMI interfaces
>* Create a global structure shared across interface source files
>* Make get_current_password function static
>* Remove get_pending changes function, shared across global structure now.
> - Overhaul use of mutexes
>* Make kset list mutex shared across source files
>* Remove unneeded dell-wmi-sysman call_mutex
>* Keep remaining call_mutexes in WMI functions
> - Move security area calculation into a function
> - Use NLS helper for utf8->utf16 conversion
>
> .../testing/sysfs-platform-dell-wmi-sysman| 126
> MAINTAINERS | 9 +
> drivers/platform/x86/Kconfig | 12 +
> drivers/platform/x86/Makefile | 8 +
> .../x86/dell-wmi-biosattr-interface.c | 198 ++
> .../platform/x86/dell-wmi-enum-attributes.c | 214 +++
> .../platform/x86/dell-wmi-int-attributes.c| 195 ++
> .../x86/dell-wmi-passobj-attributes.c | 168 +
> .../x86/dell-wmi-passwordattr-interface.c | 200 ++
> .../platform/x86/dell-wmi-string-attributes.c | 177 ++
> .../platform/x86/dell-wmi-sysman-attributes.c | 572 ++
> .../platform/x86/dell-wmi-sysman-attributes.h | 132
> 12 files changed, 2011 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-wmi-sysman
> create mode 100644 drivers/platform/x86/dell-wmi-biosattr-interface.c
> create mode 100644 drivers/platform/x86/dell-wmi-enum-attributes.c
> create mode 100644 drivers/platform/x86/dell-wmi-int-attributes.c
> create mode 100644 drivers/platform/x86/dell-wmi-passobj-attributes.c
> create mode 100644 drivers/platform/x86/dell-wmi-passwordattr-interface.c
> create mode 100644 drivers/platform/x86/dell-wmi-string-attributes.c
> create mode 100644 drivers/platform/x86/dell-wmi-sysman-attributes.c
> create mode 100644 drivers/platform/x86/dell-wmi-sysman-attributes.h
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-dell-wmi-sysman
> b/Documentation/ABI/testing/sysfs-platform-dell-wmi-sysman
> new file mode 100644
> index ..e4b608275ea4
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-dell-wmi-sysman
> @@ -0,0 +1,126 @@
> +What:/sys/devices/platform/dell-wmi-sysman/attributes/
> +Date:December 2020
> +KernelVersion: 5.10
> +Contact: Divya Bharathi ,
> + Mario Limonciello ,
> + Prasanth KSR
> +Description:
> + The Dell WMI Systems Management Driver provides a sysfs
> interface
> + for systems management software to enable BIOS configuration
> + capability on certain Dell systems. This directory exposes
> + interfaces for interacting with BIOS attributes.
> +
> + Attributes can accept a set of pre-defined valid values or a
> range of
> + numerical values or a string. An atribute can accept float as
> well,
> + if so '.' is used as decimal separator.
> +
> + Also, BIOS Admin password and System Password can be set, reset
> or
> + cleared using these attributes. An "Admin" password is used for
> + preventing modification to the BIOS settings. A "System"
> password is
> + required to boot a machine.
> +
> + current_value: A file that can be read to obtain the current
> + value of the
> +
> + This file can also be written to in order to update
> + the value of a
> +
> + default_value: A file that can be read to obtain the default
> + value of the
> +
> + display_name: A file that can be read to obtain a user
> friendly
> + description of the at
> +
> +
Re: [PATCH v5] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode
On Thu, Jul 09, 2020 at 12:42:57PM -0700, Doug Anderson wrote: > Hi, > > On Thu, Jul 9, 2020 at 3:51 AM Thomas Gleixner wrote: > > > > Abhishek Bhardwaj writes: > > > This change adds a new kernel configuration that sets the l1d cache > > > flush setting at compile time rather than at run time. > > > > > > The reasons for this change are as follows - > > > > > > - Kernel command line arguments are getting unwieldy. These parameters > > > are not a scalable way to set the kernel config. They're intended as a > > > super limited way for the bootloader to pass info to the kernel and > > > also as a way for end users who are not compiling the kernel themselves > > > to tweak the kernel behavior. > > > > > > - Also, if a user wants this setting from the start. It's a definite > > > smell that it deserves to be a compile time thing rather than adding > > > extra code plus whatever miniscule time at runtime to pass an > > > extra argument. > > > > > > - Finally, it doesn't preclude the runtime / kernel command line way. > > > Users are free to use those as well. > > > > TBH, I don't see why this is a good idea. > > > > 1) I'm not following your argumentation that the command line option is > > a poor Kconfig replacement. The L1TF mode is a boot time (module > > load time) decision and the command line parameter is there to > > override the carefully chosen and sensible default behaviour. > > When you say that the default behavior is carefully chosen and > sensible, are you saying that (in your opinion) there would never be a > good reason for someone distributing a kernel to others to change the > default? Certainly I agree that having the kernel command line > parameter is nice to allow someone to override whatever the person > building the kernel chose, but IMO it's not a good way to change the > default built-in to the kernel. > > The current plan (as I understand it) is that we'd like to ship > Chromebook kernels with this option changed from the default that's > there now. In your opinion, is that a sane thing to do? > > > > 2) You can add the desired mode to the compiled in (partial) kernel > > command line today. > > This might be easier on x86 than it is on ARM. ARM (and ARM64) > kernels only have two modes: kernel provides cmdline and bootloader > provides cmdline. There are out-of-mainline ANDROID patches to > address this but nothing in mainline. > > The patch we're discussing now is x86-only so it's not such a huge > deal, but the fact that combining the kernel and bootloader > commandline never landed in mainline for arm/arm64 means that this > isn't a super common/expected thing to do. > > > > 3) Boot loaders are well capable of handling large kernel command lines > > and the extra time spend for reading the parameter does not matter > > at all. > > Long command lines can still be a bit of a chore for humans to deal > with. Many times I've needed to look at "/proc/cmdline" and make > sense of it. The longer the command line is and the more cruft > stuffed into it the more of a chore it is. Yes, this is just one > thing to put in the command line, but if 10 different drivers all have > their "one thing" to put there it gets really long. If 100 different > drivers all want their one config option there it gets really really > long. IMO the command line should be a last resort place to put > things and should just contain: This takes me back to my years doing android kernel work for Intel, I'm glad those are over. Yes, the android kernel command lines got hideous, I think we even had patches to make the cmdline buffer bigger than the default was. >From a practical point of view the command line was part of the boot image and cryptography protected so it was a handy way to securely communicate parameters from the platform to the kernel, drivers and even just user mode. It got pretty ugly but, it worked (mostly). What I don't get is why pick on l1tf in isolation? There are a bunch of command line parameters similar to l1tf. Would a more general option make sense? Anyway, I think there is a higher level issue you are poking at that might be better addressed by talking about it directly. --mark
Re: [PATCH] x86/speculation/l1tf: Add KConfig for setting the L1D cache flush mode
On Thu, Jul 02, 2020 at 09:19:16AM -0700, Abhishek Bhardwaj wrote: > This change adds a new kernel configuration that sets the l1d cache > flush setting at compile time rather than at run time. Why is this desired? --mark > > Signed-off-by: Abhishek Bhardwaj > --- > > arch/x86/kernel/cpu/bugs.c | 8 > arch/x86/kvm/Kconfig | 17 + > 2 files changed, 25 insertions(+) > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c > index 0b71970d2d3d2..1dcc875cf5547 100644 > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -1406,7 +1406,15 @@ enum l1tf_mitigations l1tf_mitigation __ro_after_init > = L1TF_MITIGATION_FLUSH; > #if IS_ENABLED(CONFIG_KVM_INTEL) > EXPORT_SYMBOL_GPL(l1tf_mitigation); > #endif > +#if (CONFIG_KVM_VMENTRY_L1D_FLUSH == 1) > +enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_NEVER; > +#elif (CONFIG_KVM_VMENTRY_L1D_FLUSH == 2) > +enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_COND; > +#elif (CONFIG_KVM_VMENTRY_L1D_FLUSH == 3) > +enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_ALWAYS; > +#else > enum vmx_l1d_flush_state l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_AUTO; > +#endif > EXPORT_SYMBOL_GPL(l1tf_vmx_mitigation); > > /* > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > index b277a2db62676..f82a0c564e931 100644 > --- a/arch/x86/kvm/Kconfig > +++ b/arch/x86/kvm/Kconfig > @@ -107,4 +107,21 @@ config KVM_MMU_AUDIT >This option adds a R/W kVM module parameter 'mmu_audit', which allows >auditing of KVM MMU events at runtime. > > +config KVM_VMENTRY_L1D_FLUSH > + int "L1D cache flush settings (1-3)" > + range 1 3 > + default "2" > + depends on KVM && X86 && X86_64 > + help > + This setting determines the L1D cache flush behavior before a VMENTER. > + This is similar to setting the option / parameter to > + kvm-intel.vmentry_l1d_flush. > + 1 - Never flush. > + 2 - Conditinally flush. > + 3 - Always flush. > + > +# OK, it's a little counter-intuitive to do this, but it puts it neatly under > +# the virtualization menu. > +source "drivers/vhost/Kconfig" > + > endif # VIRTUALIZATION > -- > 2.27.0.212.ge8ba1cc988-goog >
Re: [PATCH] cpu/speculation: Add prototype for cpu_show_srbds
ack
Reviewed-by: mark gross
--mark
On Wed, Jun 17, 2020 at 07:14:10AM -0700, Guenter Roeck wrote:
> 0-day is not happy that there is no prototype for cpu_show_srbds().
>
> drivers/base/cpu.c:565:16: error:
> no previous prototype for 'cpu_show_srbds'
>
> Reported-by: kernel test robot
> Fixes: 7e5b3c267d25 ("x86/speculation: Add Special Register Buffer Data
> Sampling (SRBDS) mitigation")
> Signed-off-by: Guenter Roeck
> ---
> include/linux/cpu.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 52692587f7fe..8aa84c052fdf 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -64,6 +64,7 @@ extern ssize_t cpu_show_tsx_async_abort(struct device *dev,
> char *buf);
> extern ssize_t cpu_show_itlb_multihit(struct device *dev,
> struct device_attribute *attr, char *buf);
> +extern ssize_t cpu_show_srbds(struct device *dev, struct device_attribute
> *attr, char *buf);
>
> extern __printf(4, 5)
> struct device *cpu_device_create(struct device *parent, void *drvdata,
> --
> 2.17.1
>
Re: [PATCH 1/1] doc: x86/speculation: length of underlines
+1 reviewed-by: Mark Gross Thanks! --mark On Mon, Jun 15, 2020 at 10:36:45PM +0200, Heinrich Schuchardt wrote: > The lengths of underlines must match the titles to avoid build warnings. > > Signed-off-by: Heinrich Schuchardt > --- > .../hw-vuln/special-register-buffer-data-sampling.rst | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git > a/Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst > b/Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst > index 47b1b3afac99..3b1ce68d2456 100644 > --- > a/Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst > +++ > b/Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst > @@ -14,7 +14,7 @@ to the core through the special register mechanism that is > susceptible > to MDS attacks. > > Affected processors > - > +--- > Core models (desktop, mobile, Xeon-E3) that implement RDRAND and/or RDSEED > may > be affected. > > @@ -59,7 +59,7 @@ executed on another core or sibling thread using MDS > techniques. > > > Mitigation mechanism > > + > Intel will release microcode updates that modify the RDRAND, RDSEED, and > EGETKEY instructions to overwrite secret special register data in the shared > staging buffer before the secret data can be accessed by another logical > @@ -118,7 +118,7 @@ with the option "srbds=". The option for this is: >= = > > SRBDS System Information > > + > The Linux kernel provides vulnerability status information through sysfs. > For > SRBDS this can be accessed by the following sysfs file: > /sys/devices/system/cpu/vulnerabilities/srbds > -- > 2.27.0 >
Re: [PATCH] docs: hw-vuln: SRBDS: Fix "Title underline too short" warnings during build
Ack
Signed-off-by:Mark Gross
Thanks!
--mark
On Tue, Jun 09, 2020 at 10:28:56PM +0200, Salvatore Bonaccorso wrote:
> Some of the title underlining did not have the correct length causing a few
> warnings when building the htmldocs. Line up each of the title underlinings
> with the text they are under.
>
> Fixes: 7222a1b5b874 ("x86/speculation: Add SRBDS vulnerability and mitigation
> documentation")
> Cc: Mark Gross
> Cc: Josh Poimboeuf
> Cc: Borislav Petkov
> Cc: Tony Luck
> Cc: Thomas Gleixner
> Cc: linux-kernel@vger.kernel.org
> Cc: triv...@kernel.org
> Signed-off-by: Salvatore Bonaccorso
> ---
> .../hw-vuln/special-register-buffer-data-sampling.rst | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git
> a/Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
> b/Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
> index 47b1b3afac99..3b1ce68d2456 100644
> ---
> a/Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
> +++
> b/Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
> @@ -14,7 +14,7 @@ to the core through the special register mechanism that is
> susceptible
> to MDS attacks.
>
> Affected processors
> -
> +---
> Core models (desktop, mobile, Xeon-E3) that implement RDRAND and/or RDSEED
> may
> be affected.
>
> @@ -59,7 +59,7 @@ executed on another core or sibling thread using MDS
> techniques.
>
>
> Mitigation mechanism
>
> +
> Intel will release microcode updates that modify the RDRAND, RDSEED, and
> EGETKEY instructions to overwrite secret special register data in the shared
> staging buffer before the secret data can be accessed by another logical
> @@ -118,7 +118,7 @@ with the option "srbds=". The option for this is:
>= =
>
> SRBDS System Information
>
> +
> The Linux kernel provides vulnerability status information through sysfs.
> For
> SRBDS this can be accessed by the following sysfs file:
> /sys/devices/system/cpu/vulnerabilities/srbds
> --
> 2.27.0
>
[tip: x86/cpu] x86/cpu: Add a steppings field to struct x86_cpu_id
The following commit has been merged into the x86/cpu branch of tip:
Commit-ID: e9d7144597b10ff13ff2264c059f7d4a7fbc89ac
Gitweb:
https://git.kernel.org/tip/e9d7144597b10ff13ff2264c059f7d4a7fbc89ac
Author:Mark Gross
AuthorDate:Thu, 16 Apr 2020 17:23:10 +02:00
Committer: Thomas Gleixner
CommitterDate: Mon, 20 Apr 2020 12:19:21 +02:00
x86/cpu: Add a steppings field to struct x86_cpu_id
Intel uses the same family/model for several CPUs. Sometimes the
stepping must be checked to tell them apart.
On x86 there can be at most 16 steppings. Add a steppings bitmask to
x86_cpu_id and a X86_MATCH_VENDOR_FAMILY_MODEL_STEPPING_FEATURE macro
and support for matching against family/model/stepping.
[ bp: Massage. ]
Signed-off-by: Mark Gross
Signed-off-by: Borislav Petkov
Signed-off-by: Thomas Gleixner
Reviewed-by: Tony Luck
Reviewed-by: Josh Poimboeuf
---
arch/x86/include/asm/cpu_device_id.h | 27 ---
arch/x86/kernel/cpu/match.c | 7 ++-
include/linux/mod_devicetable.h | 2 ++-
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/cpu_device_id.h
b/arch/x86/include/asm/cpu_device_id.h
index cf3d621..10426cd 100644
--- a/arch/x86/include/asm/cpu_device_id.h
+++ b/arch/x86/include/asm/cpu_device_id.h
@@ -20,12 +20,14 @@
#define X86_CENTAUR_FAM6_C7_D 0xd
#define X86_CENTAUR_FAM6_NANO 0xf
+#define X86_STEPPINGS(mins, maxs)GENMASK(maxs, mins)
/**
- * X86_MATCH_VENDOR_FAM_MODEL_FEATURE - Base macro for CPU matching
+ * X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE - Base macro for CPU matching
* @_vendor: The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY
* The name is expanded to X86_VENDOR_@_vendor
* @_family: The family number or X86_FAMILY_ANY
* @_model:The model number, model constant or X86_MODEL_ANY
+ * @_steppings:Bitmask for steppings, stepping constant or
X86_STEPPING_ANY
* @_feature: A X86_FEATURE bit or X86_FEATURE_ANY
* @_data: Driver specific data or NULL. The internal storage
* format is unsigned long. The supplied value, pointer
@@ -37,16 +39,35 @@
* into another macro at the usage site for good reasons, then please
* start this local macro with X86_MATCH to allow easy grepping.
*/
-#define X86_MATCH_VENDOR_FAM_MODEL_FEATURE(_vendor, _family, _model, \
- _feature, _data) { \
+#define X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE(_vendor, _family, _model,
\
+ _steppings, _feature,
_data) { \
.vendor = X86_VENDOR_##_vendor, \
.family = _family, \
.model = _model, \
+ .steppings = _steppings, \
.feature= _feature, \
.driver_data= (unsigned long) _data \
}
/**
+ * X86_MATCH_VENDOR_FAM_MODEL_FEATURE - Macro for CPU matching
+ * @_vendor: The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY
+ * The name is expanded to X86_VENDOR_@_vendor
+ * @_family: The family number or X86_FAMILY_ANY
+ * @_model:The model number, model constant or X86_MODEL_ANY
+ * @_feature: A X86_FEATURE bit or X86_FEATURE_ANY
+ * @_data: Driver specific data or NULL. The internal storage
+ * format is unsigned long. The supplied value, pointer
+ * etc. is casted to unsigned long internally.
+ *
+ * The steppings arguments of X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE() is
+ * set to wildcards.
+ */
+#define X86_MATCH_VENDOR_FAM_MODEL_FEATURE(vendor, family, model, feature,
data) \
+ X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE(vendor, family, model, \
+ X86_STEPPING_ANY, feature, data)
+
+/**
* X86_MATCH_VENDOR_FAM_FEATURE - Macro for matching vendor, family and CPU
feature
* @vendor:The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY
* The name is expanded to X86_VENDOR_@vendor
diff --git a/arch/x86/kernel/cpu/match.c b/arch/x86/kernel/cpu/match.c
index d3482eb..ad67760 100644
--- a/arch/x86/kernel/cpu/match.c
+++ b/arch/x86/kernel/cpu/match.c
@@ -39,13 +39,18 @@ const struct x86_cpu_id *x86_match_cpu(const struct
x86_cpu_id *match)
const struct x86_cpu_id *m;
struct cpuinfo_x86 *c = _cpu_data;
- for (m = match; m->vendor | m->family | m->model | m->feature; m++) {
+ for (m = match;
+m->vendor | m->family | m->model | m->steppings | m->feature;
+m++) {
if (m->vendor != X86_VENDOR_ANY && c->x86_vendor != m->vendor)
continue;
if (m->family != X86_FAMILY_ANY &&
Re: [PATCH 1/3] x86/cpu: Add a steppings field to struct x86_cpu_id
+1
--mark
On Wed, May 06, 2020 at 09:15:14AM +0200, Borislav Petkov wrote:
> From: Mark Gross
>
> Intel uses the same family/model for several CPUs. Sometimes the
> stepping must be checked to tell them apart.
>
> On x86 there can be at most 16 steppings. Add a steppings bitmask to
> x86_cpu_id and a X86_MATCH_VENDOR_FAMILY_MODEL_STEPPING_FEATURE macro
> and support for matching against family/model/stepping.
>
> [ bp: Massage. ]
>
> Signed-off-by: Mark Gross
> Signed-off-by: Borislav Petkov
> Signed-off-by: Thomas Gleixner
> Reviewed-by: Tony Luck
> Reviewed-by: Josh Poimboeuf
> ---
> arch/x86/include/asm/cpu_device_id.h | 27 ---
> arch/x86/kernel/cpu/match.c | 7 ++-
> include/linux/mod_devicetable.h | 2 ++
> 3 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpu_device_id.h
> b/arch/x86/include/asm/cpu_device_id.h
> index cf3d621c6892..10426cd56dca 100644
> --- a/arch/x86/include/asm/cpu_device_id.h
> +++ b/arch/x86/include/asm/cpu_device_id.h
> @@ -20,12 +20,14 @@
> #define X86_CENTAUR_FAM6_C7_D0xd
> #define X86_CENTAUR_FAM6_NANO0xf
>
> +#define X86_STEPPINGS(mins, maxs)GENMASK(maxs, mins)
> /**
> - * X86_MATCH_VENDOR_FAM_MODEL_FEATURE - Base macro for CPU matching
> + * X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE - Base macro for CPU matching
> * @_vendor: The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY
> * The name is expanded to X86_VENDOR_@_vendor
> * @_family: The family number or X86_FAMILY_ANY
> * @_model: The model number, model constant or X86_MODEL_ANY
> + * @_steppings: Bitmask for steppings, stepping constant or
> X86_STEPPING_ANY
> * @_feature:A X86_FEATURE bit or X86_FEATURE_ANY
> * @_data: Driver specific data or NULL. The internal storage
> * format is unsigned long. The supplied value, pointer
> @@ -37,15 +39,34 @@
> * into another macro at the usage site for good reasons, then please
> * start this local macro with X86_MATCH to allow easy grepping.
> */
> -#define X86_MATCH_VENDOR_FAM_MODEL_FEATURE(_vendor, _family, _model, \
> -_feature, _data) { \
> +#define X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE(_vendor, _family,
> _model, \
> + _steppings, _feature,
> _data) { \
> .vendor = X86_VENDOR_##_vendor, \
> .family = _family, \
> .model = _model, \
> + .steppings = _steppings, \
> .feature= _feature, \
> .driver_data= (unsigned long) _data \
> }
>
> +/**
> + * X86_MATCH_VENDOR_FAM_MODEL_FEATURE - Macro for CPU matching
> + * @_vendor: The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY
> + * The name is expanded to X86_VENDOR_@_vendor
> + * @_family: The family number or X86_FAMILY_ANY
> + * @_model: The model number, model constant or X86_MODEL_ANY
> + * @_feature:A X86_FEATURE bit or X86_FEATURE_ANY
> + * @_data: Driver specific data or NULL. The internal storage
> + * format is unsigned long. The supplied value, pointer
> + * etc. is casted to unsigned long internally.
> + *
> + * The steppings arguments of X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE()
> is
> + * set to wildcards.
> + */
> +#define X86_MATCH_VENDOR_FAM_MODEL_FEATURE(vendor, family, model, feature,
> data) \
> + X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE(vendor, family, model, \
> + X86_STEPPING_ANY, feature, data)
> +
> /**
> * X86_MATCH_VENDOR_FAM_FEATURE - Macro for matching vendor, family and CPU
> feature
> * @vendor: The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY
> diff --git a/arch/x86/kernel/cpu/match.c b/arch/x86/kernel/cpu/match.c
> index d3482eb43ff3..ad6776081e60 100644
> --- a/arch/x86/kernel/cpu/match.c
> +++ b/arch/x86/kernel/cpu/match.c
> @@ -39,13 +39,18 @@ const struct x86_cpu_id *x86_match_cpu(const struct
> x86_cpu_id *match)
> const struct x86_cpu_id *m;
> struct cpuinfo_x86 *c = _cpu_data;
>
> - for (m = match; m->vendor | m->family | m->model | m->feature; m++) {
> + for (m = match;
> + m->vendor | m->family | m->model | m->steppings | m->feature;
> + m++) {
> if (m->vendor != X86_VENDOR_ANY && c->x86_vendor != m->vendor)
>
Re: [PATCH 2/3] x86/cpu: Add a X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS() macro
Reviewed-by: Mark Gross On Wed, May 06, 2020 at 09:15:15AM +0200, Borislav Petkov wrote: > From: Borislav Petkov > > ... to match Intel family 6 CPUs with steppings. > > Signed-off-by: Borislav Petkov > --- > arch/x86/include/asm/cpu_device_id.h | 4 > 1 file changed, 4 insertions(+) > > diff --git a/arch/x86/include/asm/cpu_device_id.h > b/arch/x86/include/asm/cpu_device_id.h > index 10426cd56dca..eb8fcede9e3b 100644 > --- a/arch/x86/include/asm/cpu_device_id.h > +++ b/arch/x86/include/asm/cpu_device_id.h > @@ -160,6 +160,10 @@ > #define X86_MATCH_INTEL_FAM6_MODEL(model, data) > \ > X86_MATCH_VENDOR_FAM_MODEL(INTEL, 6, INTEL_FAM6_##model, data) > > +#define X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(model, steppings, data) \ > + X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE(INTEL, 6, > INTEL_FAM6_##model, \ > + steppings, > X86_FEATURE_ANY, data) > + > /* > * Match specific microcode revisions. > * > -- > 2.21.0 >
Re: [PATCH 3/3] x86/apic: Convert the TSC deadline timer matching to steppings macro
Reviewed-by: Mark Gross
On Wed, May 06, 2020 at 09:15:16AM +0200, Borislav Petkov wrote:
> From: Borislav Petkov
>
> ... and get rid of the function pointers which would spit out the
> microcode revision based on the CPU stepping.
>
> Signed-off-by: Borislav Petkov
> Cc: Peter Zijlstra (Intel)
> ---
> arch/x86/kernel/apic/apic.c | 57 -
> 1 file changed, 12 insertions(+), 45 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index e53dda210cd7..4b1d31be50b4 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -544,46 +544,20 @@ static struct clock_event_device lapic_clockevent = {
> };
> static DEFINE_PER_CPU(struct clock_event_device, lapic_events);
>
> -static __init u32 hsx_deadline_rev(void)
> -{
> - switch (boot_cpu_data.x86_stepping) {
> - case 0x02: return 0x3a; /* EP */
> - case 0x04: return 0x0f; /* EX */
> - }
> -
> - return ~0U;
> -}
> -
> -static __init u32 bdx_deadline_rev(void)
> -{
> - switch (boot_cpu_data.x86_stepping) {
> - case 0x02: return 0x0011;
> - case 0x03: return 0x070e;
> - case 0x04: return 0x0f0c;
> - case 0x05: return 0x0e03;
> - }
> -
> - return ~0U;
> -}
> -
> -static __init u32 skx_deadline_rev(void)
> -{
> - switch (boot_cpu_data.x86_stepping) {
> - case 0x03: return 0x01000136;
> - case 0x04: return 0x0214;
> - }
> +static const struct x86_cpu_id deadline_match[] __initconst = {
> + X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(HASWELL_X, X86_STEPPINGS(0x2,
> 0x2), 0x3a), /* EP */
> + X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(HASWELL_X, X86_STEPPINGS(0x4,
> 0x4), 0x0f), /* EX */
>
> - if (boot_cpu_data.x86_stepping > 4)
> - return 0;
> + X86_MATCH_INTEL_FAM6_MODEL( BROADWELL_X,0x0b20),
>
> - return ~0U;
> -}
> + X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(BROADWELL_D, X86_STEPPINGS(0x2,
> 0x2), 0x0011),
> + X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(BROADWELL_D, X86_STEPPINGS(0x3,
> 0x3), 0x070e),
> + X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(BROADWELL_D, X86_STEPPINGS(0x4,
> 0x4), 0x0f0c),
> + X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(BROADWELL_D, X86_STEPPINGS(0x5,
> 0x5), 0x0e03),
>
> -static const struct x86_cpu_id deadline_match[] __initconst = {
> - X86_MATCH_INTEL_FAM6_MODEL( HASWELL_X, _deadline_rev),
> - X86_MATCH_INTEL_FAM6_MODEL( BROADWELL_X,0x0b20),
> - X86_MATCH_INTEL_FAM6_MODEL( BROADWELL_D,_deadline_rev),
> - X86_MATCH_INTEL_FAM6_MODEL( SKYLAKE_X, _deadline_rev),
> + X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(SKYLAKE_X, X86_STEPPINGS(0x3,
> 0x3), 0x01000136),
> + X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(SKYLAKE_X, X86_STEPPINGS(0x4,
> 0x4), 0x0214),
> + X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(SKYLAKE_X, X86_STEPPINGS(0x5,
> 0xf), 0),
>
> X86_MATCH_INTEL_FAM6_MODEL( HASWELL,0x22),
> X86_MATCH_INTEL_FAM6_MODEL( HASWELL_L, 0x20),
> @@ -615,14 +589,7 @@ static __init bool apic_validate_deadline_timer(void)
> if (!m)
> return true;
>
> - /*
> - * Function pointers will have the MSB set due to address layout,
> - * immediate revisions will not.
> - */
> - if ((long)m->driver_data < 0)
> - rev = ((u32 (*)(void))(m->driver_data))();
> - else
> - rev = (u32)m->driver_data;
> + rev = (u32)m->driver_data;
>
> if (boot_cpu_data.microcode >= rev)
> return true;
> --
> 2.21.0
>
Re: [RFC PATCH v3 11/16] sched: Basic tracking of matching tasks
On Wed, May 29, 2019 at 08:36:47PM +, Vineeth Remanan Pillai wrote:
> From: Peter Zijlstra
>
> Introduce task_struct::core_cookie as an opaque identifier for core
> scheduling. When enabled; core scheduling will only allow matching
> task to be on the core; where idle matches everything.
>
> When task_struct::core_cookie is set (and core scheduling is enabled)
> these tasks are indexed in a second RB-tree, first on cookie value
> then on scheduling function, such that matching task selection always
> finds the most elegible match.
>
> NOTE: *shudder* at the overhead...
>
> NOTE: *sigh*, a 3rd copy of the scheduling function; the alternative
> is per class tracking of cookies and that just duplicates a lot of
> stuff for no raisin (the 2nd copy lives in the rt-mutex PI code).
s/raisen/reason
>
> Signed-off-by: Peter Zijlstra (Intel)
> Signed-off-by: Vineeth Remanan Pillai
> Signed-off-by: Julien Desfossez
> ---
>
> Changes in v3
> -
> - Refactored priority comparison code
> - Fixed a comparison logic issue in sched_core_find
> - Aaron Lu
>
> Changes in v2
> -
> - Improves the priority comparison logic between processes in
> different cpus.
> - Peter Zijlstra
> - Aaron Lu
>
> ---
> include/linux/sched.h | 8 ++-
> kernel/sched/core.c | 146 ++
> kernel/sched/fair.c | 46 -
> kernel/sched/sched.h | 55
> 4 files changed, 208 insertions(+), 47 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 1549584a1538..a4b39a28236f 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -636,10 +636,16 @@ struct task_struct {
> const struct sched_class*sched_class;
> struct sched_entity se;
> struct sched_rt_entity rt;
> + struct sched_dl_entity dl;
> +
> +#ifdef CONFIG_SCHED_CORE
> + struct rb_node core_node;
> + unsigned long core_cookie;
> +#endif
> +
> #ifdef CONFIG_CGROUP_SCHED
> struct task_group *sched_task_group;
> #endif
> - struct sched_dl_entity dl;
>
> #ifdef CONFIG_PREEMPT_NOTIFIERS
> /* List of struct preempt_notifier: */
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index b1ce33f9b106..112d70f2b1e5 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -64,6 +64,141 @@ int sysctl_sched_rt_runtime = 95;
>
> DEFINE_STATIC_KEY_FALSE(__sched_core_enabled);
>
> +/* kernel prio, less is more */
> +static inline int __task_prio(struct task_struct *p)
> +{
> + if (p->sched_class == _sched_class) /* trumps deadline */
> + return -2;
> +
> + if (rt_prio(p->prio)) /* includes deadline */
> + return p->prio; /* [-1, 99] */
> +
> + if (p->sched_class == _sched_class)
> + return MAX_RT_PRIO + NICE_WIDTH; /* 140 */
> +
> + return MAX_RT_PRIO + MAX_NICE; /* 120, squash fair */
> +}
> +
> +/*
> + * l(a,b)
> + * le(a,b) := !l(b,a)
> + * g(a,b) := l(b,a)
> + * ge(a,b) := !l(a,b)
why does this truth table comment exist?
maybe inline comments at the confusing inequalities would be better.
--mark
Re: [RFC PATCH v3 09/16] sched: Introduce sched_class::pick_task()
On Wed, May 29, 2019 at 08:36:45PM +, Vineeth Remanan Pillai wrote:
> From: Peter Zijlstra
>
> Because sched_class::pick_next_task() also implies
> sched_class::set_next_task() (and possibly put_prev_task() and
> newidle_balance) it is not state invariant. This makes it unsuitable
> for remote task selection.
It would be cool if the commit comment would explain what the change is going
to do about pick_next_task being unsuitable for remote task selection.
>
> Signed-off-by: Peter Zijlstra (Intel)
> Signed-off-by: Vineeth Remanan Pillai
> Signed-off-by: Julien Desfossez
> ---
>
> Chnages in v3
> -
> - Minor refactor to remove redundant NULL checks
>
> Changes in v2
> -
> - Fixes a NULL pointer dereference crash
> - Subhra Mazumdar
> - Tim Chen
>
> ---
> kernel/sched/deadline.c | 21 -
> kernel/sched/fair.c | 36 +---
> kernel/sched/idle.c | 10 +-
> kernel/sched/rt.c| 21 -
> kernel/sched/sched.h | 2 ++
> kernel/sched/stop_task.c | 21 -
> 6 files changed, 92 insertions(+), 19 deletions(-)
>
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index d3904168857a..64fc444f44f9 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1722,15 +1722,12 @@ static struct sched_dl_entity
> *pick_next_dl_entity(struct rq *rq,
> return rb_entry(left, struct sched_dl_entity, rb_node);
> }
>
> -static struct task_struct *
> -pick_next_task_dl(struct rq *rq, struct task_struct *prev, struct rq_flags
> *rf)
> +static struct task_struct *pick_task_dl(struct rq *rq)
> {
> struct sched_dl_entity *dl_se;
> struct task_struct *p;
> struct dl_rq *dl_rq;
>
> - WARN_ON_ONCE(prev || rf);
> -
> dl_rq = >dl;
>
> if (unlikely(!dl_rq->dl_nr_running))
> @@ -1741,7 +1738,19 @@ pick_next_task_dl(struct rq *rq, struct task_struct
> *prev, struct rq_flags *rf)
>
> p = dl_task_of(dl_se);
>
> - set_next_task_dl(rq, p);
> + return p;
> +}
> +
> +static struct task_struct *
> +pick_next_task_dl(struct rq *rq, struct task_struct *prev, struct rq_flags
> *rf)
> +{
> + struct task_struct *p;
> +
> + WARN_ON_ONCE(prev || rf);
What is an admin to do with this warnding if it shows up in there logs?
maybe include some text here to help folks that might hit this warn_on.
> +
> + p = pick_task_dl(rq);
> + if (p)
> + set_next_task_dl(rq, p);
>
> return p;
> }
> @@ -2388,6 +2397,8 @@ const struct sched_class dl_sched_class = {
> .set_next_task = set_next_task_dl,
>
> #ifdef CONFIG_SMP
> + .pick_task = pick_task_dl,
> +
> .select_task_rq = select_task_rq_dl,
> .migrate_task_rq= migrate_task_rq_dl,
> .set_cpus_allowed = set_cpus_allowed_dl,
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index e65f2dfda77a..02e5dfb85e7d 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4136,7 +4136,7 @@ pick_next_entity(struct cfs_rq *cfs_rq, struct
> sched_entity *curr)
>* Avoid running the skip buddy, if running something else can
>* be done without getting too unfair.
>*/
> - if (cfs_rq->skip == se) {
> + if (cfs_rq->skip && cfs_rq->skip == se) {
> struct sched_entity *second;
>
> if (se == curr) {
> @@ -4154,13 +4154,13 @@ pick_next_entity(struct cfs_rq *cfs_rq, struct
> sched_entity *curr)
> /*
>* Prefer last buddy, try to return the CPU to a preempted task.
>*/
> - if (cfs_rq->last && wakeup_preempt_entity(cfs_rq->last, left) < 1)
> + if (left && cfs_rq->last && wakeup_preempt_entity(cfs_rq->last, left) <
> 1)
> se = cfs_rq->last;
>
> /*
>* Someone really wants this to run. If it's not unfair, run it.
>*/
> - if (cfs_rq->next && wakeup_preempt_entity(cfs_rq->next, left) < 1)
> + if (left && cfs_rq->next && wakeup_preempt_entity(cfs_rq->next, left) <
> 1)
> se = cfs_rq->next;
>
> clear_buddies(cfs_rq, se);
> @@ -6966,6 +6966,34 @@ static void check_preempt_wakeup(struct rq *rq, struct
> task_struct *p, int wake_
> set_last_buddy(se);
> }
>
> +static struct task_struct *
> +pick_task_fair(struct rq *rq)
> +{
> + struct cfs_rq *cfs_rq = >cfs;
> + struct sched_entity *se;
> +
> + if (!cfs_rq->nr_running)
> + return NULL;
> +
> + do {
> + struct sched_entity *curr = cfs_rq->curr;
> +
> + se = pick_next_entity(cfs_rq, NULL);
> +
> + if (curr) {
> + if (se && curr->on_rq)
> + update_curr(cfs_rq);
> +
> + if (!se || entity_before(curr, se))
> + se = curr;
> + }
> +
> + cfs_rq = group_cfs_rq(se);
> +
Re: [RFC PATCH v3 08/16] sched: Rework pick_next_task() slow-path
On Wed, May 29, 2019 at 08:36:44PM +, Vineeth Remanan Pillai wrote:
> From: Peter Zijlstra
>
> Avoid the RETRY_TASK case in the pick_next_task() slow path.
>
> By doing the put_prev_task() early, we get the rt/deadline pull done,
> and by testing rq->nr_running we know if we need newidle_balance().
>
> This then gives a stable state to pick a task from.
>
> Since the fast-path is fair only; it means the other classes will
> always have pick_next_task(.prev=NULL, .rf=NULL) and we can simplify.
>
> Signed-off-by: Peter Zijlstra (Intel)
> ---
> kernel/sched/core.c | 19 ---
> kernel/sched/deadline.c | 30 ++
> kernel/sched/fair.c | 9 ++---
> kernel/sched/idle.c | 4 +++-
> kernel/sched/rt.c| 29 +
> kernel/sched/sched.h | 13 -
> kernel/sched/stop_task.c | 3 ++-
> 7 files changed, 34 insertions(+), 73 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 9dfa0c53deb3..b883c70674ba 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3363,7 +3363,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev,
> struct rq_flags *rf)
>
> p = fair_sched_class.pick_next_task(rq, prev, rf);
> if (unlikely(p == RETRY_TASK))
> - goto again;
> + goto restart;
>
> /* Assumes fair_sched_class->next == idle_sched_class */
> if (unlikely(!p))
> @@ -3372,14 +3372,19 @@ pick_next_task(struct rq *rq, struct task_struct
> *prev, struct rq_flags *rf)
> return p;
> }
>
> -again:
> +restart:
> + /*
> + * Ensure that we put DL/RT tasks before the pick loop, such that they
> + * can PULL higher prio tasks when we lower the RQ 'priority'.
> + */
> + prev->sched_class->put_prev_task(rq, prev, rf);
> + if (!rq->nr_running)
> + newidle_balance(rq, rf);
> +
> for_each_class(class) {
> - p = class->pick_next_task(rq, prev, rf);
> - if (p) {
> - if (unlikely(p == RETRY_TASK))
> - goto again;
> + p = class->pick_next_task(rq, NULL, NULL);
> + if (p)
> return p;
> - }
> }
>
> /* The idle class should always have a runnable task: */
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 45425f971eec..d3904168857a 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1729,39 +1729,13 @@ pick_next_task_dl(struct rq *rq, struct task_struct
> *prev, struct rq_flags *rf)
> struct task_struct *p;
> struct dl_rq *dl_rq;
>
> - dl_rq = >dl;
> -
> - if (need_pull_dl_task(rq, prev)) {
> - /*
> - * This is OK, because current is on_cpu, which avoids it being
> - * picked for load-balance and preemption/IRQs are still
> - * disabled avoiding further scheduler activity on it and we're
> - * being very careful to re-start the picking loop.
> - */
> - rq_unpin_lock(rq, rf);
> - pull_dl_task(rq);
> - rq_repin_lock(rq, rf);
> - /*
> - * pull_dl_task() can drop (and re-acquire) rq->lock; this
> - * means a stop task can slip in, in which case we need to
> - * re-start task selection.
> - */
> - if (rq->stop && task_on_rq_queued(rq->stop))
> - return RETRY_TASK;
> - }
> + WARN_ON_ONCE(prev || rf);
should there be a helpful message to go with this warning?
>
> - /*
> - * When prev is DL, we may throttle it in put_prev_task().
> - * So, we update time before we check for dl_nr_running.
> - */
> - if (prev->sched_class == _sched_class)
> - update_curr_dl(rq);
> + dl_rq = >dl;
>
> if (unlikely(!dl_rq->dl_nr_running))
> return NULL;
>
> - put_prev_task(rq, prev);
> -
> dl_se = pick_next_dl_entity(rq, dl_rq);
> BUG_ON(!dl_se);
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 8e3eb243fd9f..e65f2dfda77a 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6979,7 +6979,7 @@ pick_next_task_fair(struct rq *rq, struct task_struct
> *prev, struct rq_flags *rf
> goto idle;
>
> #ifdef CONFIG_FAIR_GROUP_SCHED
> - if (prev->sched_class != _sched_class)
> + if (!prev || prev->sched_class != _sched_class)
> goto simple;
>
> /*
> @@ -7056,8 +7056,8 @@ pick_next_task_fair(struct rq *rq, struct task_struct
> *prev, struct rq_flags *rf
> goto done;
> simple:
> #endif
> -
> - put_prev_task(rq, prev);
> + if (prev)
> + put_prev_task(rq, prev);
>
> do {
> se = pick_next_entity(cfs_rq, NULL);
> @@ -7085,6 +7085,9 @@
Re: [RFC PATCH v3 07/16] sched: Allow put_prev_task() to drop rq->lock
On Wed, May 29, 2019 at 08:36:43PM +, Vineeth Remanan Pillai wrote:
> From: Peter Zijlstra
>
> Currently the pick_next_task() loop is convoluted and ugly because of
> how it can drop the rq->lock and needs to restart the picking.
>
> For the RT/Deadline classes, it is put_prev_task() where we do
> balancing, and we could do this before the picking loop. Make this
> possible.
Maybe explain why adding strtu rq_flags pointers to the function call supports
the above commit coment.
--mark
>
> Signed-off-by: Peter Zijlstra (Intel)
> ---
> kernel/sched/core.c | 2 +-
> kernel/sched/deadline.c | 14 +-
> kernel/sched/fair.c | 2 +-
> kernel/sched/idle.c | 2 +-
> kernel/sched/rt.c| 14 +-
> kernel/sched/sched.h | 4 ++--
> kernel/sched/stop_task.c | 2 +-
> 7 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 32ea79fb8d29..9dfa0c53deb3 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -5595,7 +5595,7 @@ static void calc_load_migrate(struct rq *rq)
> atomic_long_add(delta, _load_tasks);
> }
>
> -static void put_prev_task_fake(struct rq *rq, struct task_struct *prev)
> +static void put_prev_task_fake(struct rq *rq, struct task_struct *prev,
> struct rq_flags *rf)
> {
> }
>
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index c02b3229e2c3..45425f971eec 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1772,13 +1772,25 @@ pick_next_task_dl(struct rq *rq, struct task_struct
> *prev, struct rq_flags *rf)
> return p;
> }
>
> -static void put_prev_task_dl(struct rq *rq, struct task_struct *p)
> +static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct
> rq_flags *rf)
> {
> update_curr_dl(rq);
>
> update_dl_rq_load_avg(rq_clock_pelt(rq), rq, 1);
> if (on_dl_rq(>dl) && p->nr_cpus_allowed > 1)
> enqueue_pushable_dl_task(rq, p);
> +
> + if (rf && !on_dl_rq(>dl) && need_pull_dl_task(rq, p)) {
> + /*
> + * This is OK, because current is on_cpu, which avoids it being
> + * picked for load-balance and preemption/IRQs are still
> + * disabled avoiding further scheduler activity on it and we've
> + * not yet started the picking loop.
> + */
> + rq_unpin_lock(rq, rf);
> + pull_dl_task(rq);
> + rq_repin_lock(rq, rf);
> + }
> }
>
> /*
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 49707b4797de..8e3eb243fd9f 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7110,7 +7110,7 @@ done: __maybe_unused;
> /*
> * Account for a descheduled task:
> */
> -static void put_prev_task_fair(struct rq *rq, struct task_struct *prev)
> +static void put_prev_task_fair(struct rq *rq, struct task_struct *prev,
> struct rq_flags *rf)
> {
> struct sched_entity *se = >se;
> struct cfs_rq *cfs_rq;
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index dd64be34881d..1b65a4c3683e 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -373,7 +373,7 @@ static void check_preempt_curr_idle(struct rq *rq, struct
> task_struct *p, int fl
> resched_curr(rq);
> }
>
> -static void put_prev_task_idle(struct rq *rq, struct task_struct *prev)
> +static void put_prev_task_idle(struct rq *rq, struct task_struct *prev,
> struct rq_flags *rf)
> {
> }
>
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index adec98a94f2b..51ee87c5a28a 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -1593,7 +1593,7 @@ pick_next_task_rt(struct rq *rq, struct task_struct
> *prev, struct rq_flags *rf)
> return p;
> }
>
> -static void put_prev_task_rt(struct rq *rq, struct task_struct *p)
> +static void put_prev_task_rt(struct rq *rq, struct task_struct *p, struct
> rq_flags *rf)
> {
> update_curr_rt(rq);
>
> @@ -1605,6 +1605,18 @@ static void put_prev_task_rt(struct rq *rq, struct
> task_struct *p)
>*/
> if (on_rt_rq(>rt) && p->nr_cpus_allowed > 1)
> enqueue_pushable_task(rq, p);
> +
> + if (rf && !on_rt_rq(>rt) && need_pull_rt_task(rq, p)) {
> + /*
> + * This is OK, because current is on_cpu, which avoids it being
> + * picked for load-balance and preemption/IRQs are still
> + * disabled avoiding further scheduler activity on it and we've
> + * not yet started the picking loop.
> + */
> + rq_unpin_lock(rq, rf);
> + pull_rt_task(rq);
> + rq_repin_lock(rq, rf);
> + }
> }
>
> #ifdef CONFIG_SMP
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index bfcbcbb25646..4cbe2bef92e4 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1675,7 +1675,7 @@ struct sched_class {
> struct
Re: [RFC PATCH v3 02/16] sched: Fix kerneldoc comment for ia64_set_curr_task
On Wed, May 29, 2019 at 08:36:38PM +, Vineeth Remanan Pillai wrote: > From: Peter Zijlstra NULL commit comment. --mark > > Signed-off-by: Peter Zijlstra (Intel) > --- > kernel/sched/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 4778c48a7fda..416ea613eda8 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -6287,7 +6287,7 @@ struct task_struct *curr_task(int cpu) > > #ifdef CONFIG_IA64 > /** > - * set_curr_task - set the current task for a given CPU. > + * ia64_set_curr_task - set the current task for a given CPU. > * @cpu: the processor in question. > * @p: the task pointer to set. > * > -- > 2.17.1 >
Re: [RFC PATCH v3 01/16] stop_machine: Fix stop_cpus_in_progress ordering
On Wed, May 29, 2019 at 08:36:37PM +, Vineeth Remanan Pillai wrote:
> From: Peter Zijlstra
>
> Make sure the entire for loop has stop_cpus_in_progress set.
It is not clear how this commit comment matches the change. Please explain
how adding 2 barrier's makes sure stop_cpus_in_progress is set for the entier
for loop.
--mark
>
> Signed-off-by: Peter Zijlstra (Intel)
> ---
> kernel/stop_machine.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
> index 067cb83f37ea..583119e0c51c 100644
> --- a/kernel/stop_machine.c
> +++ b/kernel/stop_machine.c
> @@ -375,6 +375,7 @@ static bool queue_stop_cpus_work(const struct cpumask
> *cpumask,
>*/
> preempt_disable();
> stop_cpus_in_progress = true;
> + barrier();
> for_each_cpu(cpu, cpumask) {
> work = _cpu(cpu_stopper.stop_work, cpu);
> work->fn = fn;
> @@ -383,6 +384,7 @@ static bool queue_stop_cpus_work(const struct cpumask
> *cpumask,
> if (cpu_stop_queue_work(cpu, work))
> queued = true;
> }
> + barrier();
> stop_cpus_in_progress = false;
> preempt_enable();
>
> --
> 2.17.1
>
Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint
On Wed, Aug 30, 2017 at 07:30:58PM -0400, Steven Rostedt wrote:
> On Wed, 30 Aug 2017 14:47:19 -0700
> mark gross <mgr...@linux.intel.com> wrote:
>
>
> > > struct dentry *ras_debugfs_dir;
> > >
> > > static atomic_t trace_count = ATOMIC_INIT(0);
> > > @@ -12,7 +14,9 @@ EXPORT_SYMBOL_GPL(ras_userspace_consumers);
> > >
> > > static int trace_show(struct seq_file *m, void *v)
> > > {
> > > - return atomic_read(_count);
> > > + seq_printf(m, "readers:%d\n", atomic_read(_count));
> > > + seq_printf(m, "has decoder:%d\n", mca_cfg.has_decoder);
> >
> > do you want to worry about this debugfs interfaces as ABI?
>
> It's the old, if a tree falls in the forest issue. If you break the ABI
> but nobody is around to notice, did it really break?
perhaps, but what I was trying to point out was: "multi line debugFS printf's
like this are very easy to change to append other information and you might
want to worry about the ABI implications that could happen in the future."
--mark
>
> > debugfs changes have bitten me on one specific OS in irritating ways.
> >
> > I'm not sure what the word is for debugfs interfaces as ABI these days so
> > this
> > feedback may be not so useful.
>
> I discussed this with Boris before he sent it out. The current code is
> actually broken. The trace_show() looks to be just a stub for a hack to
> have userspace tell the kernel it's reading something (the
> trace_count). The trace_show() function here returns the trace_count,
> which is just wrong. The seq_file show function is suppose to return
> less than zero on error, zero on success, or greater than zero if the
> show is suppose to skip the current field but not error out. This
> trace_show() function doesn't actually show anything. If you cat the
> file, it will be blank. Returning zero or greater than zero has the
> same effect. Which is nothing.
>
> -- Steve
> --
> To unsubscribe from this list: send the line "unsubscribe linux-edac" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint
On Wed, Aug 30, 2017 at 07:30:58PM -0400, Steven Rostedt wrote:
> On Wed, 30 Aug 2017 14:47:19 -0700
> mark gross wrote:
>
>
> > > struct dentry *ras_debugfs_dir;
> > >
> > > static atomic_t trace_count = ATOMIC_INIT(0);
> > > @@ -12,7 +14,9 @@ EXPORT_SYMBOL_GPL(ras_userspace_consumers);
> > >
> > > static int trace_show(struct seq_file *m, void *v)
> > > {
> > > - return atomic_read(_count);
> > > + seq_printf(m, "readers:%d\n", atomic_read(_count));
> > > + seq_printf(m, "has decoder:%d\n", mca_cfg.has_decoder);
> >
> > do you want to worry about this debugfs interfaces as ABI?
>
> It's the old, if a tree falls in the forest issue. If you break the ABI
> but nobody is around to notice, did it really break?
perhaps, but what I was trying to point out was: "multi line debugFS printf's
like this are very easy to change to append other information and you might
want to worry about the ABI implications that could happen in the future."
--mark
>
> > debugfs changes have bitten me on one specific OS in irritating ways.
> >
> > I'm not sure what the word is for debugfs interfaces as ABI these days so
> > this
> > feedback may be not so useful.
>
> I discussed this with Boris before he sent it out. The current code is
> actually broken. The trace_show() looks to be just a stub for a hack to
> have userspace tell the kernel it's reading something (the
> trace_count). The trace_show() function here returns the trace_count,
> which is just wrong. The seq_file show function is suppose to return
> less than zero on error, zero on success, or greater than zero if the
> show is suppose to skip the current field but not error out. This
> trace_show() function doesn't actually show anything. If you cat the
> file, it will be blank. Returning zero or greater than zero has the
> same effect. Which is nothing.
>
> -- Steve
> --
> To unsubscribe from this list: send the line "unsubscribe linux-edac" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint
On Wed, Aug 30, 2017 at 01:48:43PM +0200, Borislav Petkov wrote:
> Btw,
>
> how about communicating stuff to the userspace daemon like this?
>
> This'll simplify a lot of detection in userspace.
>
> Thoughts?
>
> ---
> diff --git a/drivers/ras/debugfs.c b/drivers/ras/debugfs.c
> index 501603057dff..62d3da9d256f 100644
> --- a/drivers/ras/debugfs.c
> +++ b/drivers/ras/debugfs.c
> @@ -1,5 +1,7 @@
> #include
>
> +#include "../../arch/x86/kernel/cpu/mcheck/mce-internal.h"
FWIW I this looks fishy in arch independent code.
I assume this include is for the definition of the mca_cfg global used in the
printf below.
> +
> struct dentry *ras_debugfs_dir;
>
> static atomic_t trace_count = ATOMIC_INIT(0);
> @@ -12,7 +14,9 @@ EXPORT_SYMBOL_GPL(ras_userspace_consumers);
>
> static int trace_show(struct seq_file *m, void *v)
> {
> - return atomic_read(_count);
> + seq_printf(m, "readers:%d\n", atomic_read(_count));
> + seq_printf(m, "has decoder:%d\n", mca_cfg.has_decoder);
do you want to worry about this debugfs interfaces as ABI?
debugfs changes have bitten me on one specific OS in irritating ways.
I'm not sure what the word is for debugfs interfaces as ABI these days so this
feedback may be not so useful.
--mark
> + return 0;
> }
>
> static int trace_open(struct inode *inode, struct file *file)
>
> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-edac" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint
On Wed, Aug 30, 2017 at 01:48:43PM +0200, Borislav Petkov wrote:
> Btw,
>
> how about communicating stuff to the userspace daemon like this?
>
> This'll simplify a lot of detection in userspace.
>
> Thoughts?
>
> ---
> diff --git a/drivers/ras/debugfs.c b/drivers/ras/debugfs.c
> index 501603057dff..62d3da9d256f 100644
> --- a/drivers/ras/debugfs.c
> +++ b/drivers/ras/debugfs.c
> @@ -1,5 +1,7 @@
> #include
>
> +#include "../../arch/x86/kernel/cpu/mcheck/mce-internal.h"
FWIW I this looks fishy in arch independent code.
I assume this include is for the definition of the mca_cfg global used in the
printf below.
> +
> struct dentry *ras_debugfs_dir;
>
> static atomic_t trace_count = ATOMIC_INIT(0);
> @@ -12,7 +14,9 @@ EXPORT_SYMBOL_GPL(ras_userspace_consumers);
>
> static int trace_show(struct seq_file *m, void *v)
> {
> - return atomic_read(_count);
> + seq_printf(m, "readers:%d\n", atomic_read(_count));
> + seq_printf(m, "has decoder:%d\n", mca_cfg.has_decoder);
do you want to worry about this debugfs interfaces as ABI?
debugfs changes have bitten me on one specific OS in irritating ways.
I'm not sure what the word is for debugfs interfaces as ABI these days so this
feedback may be not so useful.
--mark
> + return 0;
> }
>
> static int trace_open(struct inode *inode, struct file *file)
>
> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-edac" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] net: rfkill: gpio: remove gpio names
On Thu, Feb 27, 2014 at 10:47:58AM -0700, Stephen Warren wrote:
> On 02/27/2014 10:38 AM, Gross, Mark wrote:
> > Please know that no one should not consider me an authority on ACPI at this
> > time. But, I have some comments / context / thoughts below.
> >
> > Also I apologize in advance for any email formatting issues caused by
> > replying to this via my work exchange account / outlook client. Folks can
> > use mgr...@linux.intel.com to avoid outlook-isms from me in the future.
> >
> >> -Original Message-
> >> From: Linus Walleij [mailto:linus.wall...@linaro.org]
> >> Sent: Tuesday, February 25, 2014 1:14 AM
> >> To: Stephen Warren; Alexandre Courbot; Grant Likely;
> >> devicet...@vger.kernel.org
> >> Cc: Chen-Yu Tsai; Heikki Krogerus; Johannes Berg; David S. Miller; Rhyland
> >> Klein; linux-wireless; netdev; linux-kernel; Arnd Bergmann; Gross, Mark
> >> Subject: Re: [PATCH 2/4] net: rfkill: gpio: remove gpio names
> >>
> >> On Fri, Feb 21, 2014 at 6:35 AM, Stephen Warren
> >> wrote:
> >>> On 02/20/2014 06:55 PM, Chen-Yu Tsai wrote:
> >>
> That's correct. However using con_id to pass this results in
> different behavior across DT and ACPI. A better way is to export the
> labeling function so consumers can set meaningful labels themselves.
> >>>
> >>> But this code is the consumer of those GPIOs. IF the parameter to
> >>> devm_gpiod_get_index() isn't intended to be used, why does it exist?
> >>
> >> Kerneldoc says:
> >>
> >> /**
> >> * gpiod_get_index - obtain a GPIO from a multi-index GPIO function
> >> * @dev:GPIO consumer, can be NULL for system-global GPIOs
> >> * @con_id: function within the GPIO consumer
> >> * @idx:index of the GPIO to obtain in the consumer
> >> *
> >>
> >> Basically it is just exposing the fact that of_find_gpio() and
> >> acpi_find_gpio() both take a con_id as argument.
> >>
> >> If we drill into this, we find that it is used to conjure the arbitrary
> >> string
> >> before the gpios in the DT case, like:
> >>
> >> foo-gpios = <...>;
> >>
> >> As in tegra30-beaver.dts...
> >>
> >> sdhci@7800 {
> >> status = "okay";
> >> cd-gpios = < TEGRA_GPIO(I, 5) GPIO_ACTIVE_LOW>;
> >> wp-gpios = < TEGRA_GPIO(T, 3) GPIO_ACTIVE_HIGH>;
> >> power-gpios = < TEGRA_GPIO(D, 7) GPIO_ACTIVE_HIGH>;
> >> bus-width = <4>;
> >> };
> >>
> >> Instead of passing the GPIOs as index 0,1,2 they are named and I do admit
> >> this has a nice "things are under control" aspect to it.
> >
> > [Gross, Mark] FWIW I don't think this is as "under control" as you do.
> > Those
> > names in the above sdhci example are derived from a specific SDHCI
> tegra spec
> > sheet or schematic. Those names likely come from the data sheet for
> > the controller.
>
> The names of the properties are fixed and defined by the DT binding for
> the Tegra SDHCI controller, or even the core SDHCI bindings. Hence, they
> will be the same in every DT file that uses that Tegra SDHCI compatible
> value (the compatible property isn't show above, because the above
> fragment is a board.dts file, and the compatible value gets inherited
> from the soc.dtsi file). There won't be any variation at all,
> irrespective of what signal names exist in a particular board schematic.
>
> If there were ever an (upstream?) ACPI "binding"(?) for the Tegra SDHCI
> controller, I would hope it would use the exact same names for the GPIO
> signals.
me to!
--mark
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] net: rfkill: gpio: remove gpio names
On Thu, Feb 27, 2014 at 10:47:58AM -0700, Stephen Warren wrote:
On 02/27/2014 10:38 AM, Gross, Mark wrote:
Please know that no one should not consider me an authority on ACPI at this
time. But, I have some comments / context / thoughts below.
Also I apologize in advance for any email formatting issues caused by
replying to this via my work exchange account / outlook client. Folks can
use mgr...@linux.intel.com to avoid outlook-isms from me in the future.
-Original Message-
From: Linus Walleij [mailto:linus.wall...@linaro.org]
Sent: Tuesday, February 25, 2014 1:14 AM
To: Stephen Warren; Alexandre Courbot; Grant Likely;
devicet...@vger.kernel.org
Cc: Chen-Yu Tsai; Heikki Krogerus; Johannes Berg; David S. Miller; Rhyland
Klein; linux-wireless; netdev; linux-kernel; Arnd Bergmann; Gross, Mark
Subject: Re: [PATCH 2/4] net: rfkill: gpio: remove gpio names
On Fri, Feb 21, 2014 at 6:35 AM, Stephen Warren
swar...@wwwdotorg.org wrote:
On 02/20/2014 06:55 PM, Chen-Yu Tsai wrote:
That's correct. However using con_id to pass this results in
different behavior across DT and ACPI. A better way is to export the
labeling function so consumers can set meaningful labels themselves.
But this code is the consumer of those GPIOs. IF the parameter to
devm_gpiod_get_index() isn't intended to be used, why does it exist?
Kerneldoc says:
/**
* gpiod_get_index - obtain a GPIO from a multi-index GPIO function
* @dev:GPIO consumer, can be NULL for system-global GPIOs
* @con_id: function within the GPIO consumer
* @idx:index of the GPIO to obtain in the consumer
*
Basically it is just exposing the fact that of_find_gpio() and
acpi_find_gpio() both take a con_id as argument.
If we drill into this, we find that it is used to conjure the arbitrary
string
before the gpios in the DT case, like:
foo-gpios = ...;
As in tegra30-beaver.dts...
sdhci@7800 {
status = okay;
cd-gpios = gpio TEGRA_GPIO(I, 5) GPIO_ACTIVE_LOW;
wp-gpios = gpio TEGRA_GPIO(T, 3) GPIO_ACTIVE_HIGH;
power-gpios = gpio TEGRA_GPIO(D, 7) GPIO_ACTIVE_HIGH;
bus-width = 4;
};
Instead of passing the GPIOs as index 0,1,2 they are named and I do admit
this has a nice things are under control aspect to it.
[Gross, Mark] FWIW I don't think this is as under control as you do.
Those
names in the above sdhci example are derived from a specific SDHCI
tegra spec
sheet or schematic. Those names likely come from the data sheet for
the controller.
The names of the properties are fixed and defined by the DT binding for
the Tegra SDHCI controller, or even the core SDHCI bindings. Hence, they
will be the same in every DT file that uses that Tegra SDHCI compatible
value (the compatible property isn't show above, because the above
fragment is a board.dts file, and the compatible value gets inherited
from the soc.dtsi file). There won't be any variation at all,
irrespective of what signal names exist in a particular board schematic.
If there were ever an (upstream?) ACPI binding(?) for the Tegra SDHCI
controller, I would hope it would use the exact same names for the GPIO
signals.
me to!
--mark
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Linux 3.9.10
On Sat, Jul 13, 2013 at 11:54:17AM -0700, Greg KH wrote: > I'm announcing the release of the 3.9.10 kernel. > > Note, this might just be the last 3.9-stable kernel release, I'm not > quite sure I can guarantee another 3.9-stable kernel will be released. I guess this means 3.9 will NOT be this years LTS kernel. yes?¬ ¬ Do you think its safe to assume 3.10 will be the LTS for 2013?¬ ¬ thank! --mark > Please move to the 3.10-stable series at this point in time. If that > isn't working for you, please let us know NOW! > > All users of the 3.9 kernel series must upgrade. > > The updated 3.9.y git tree can be found at: > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git > linux-3.9.y > and can be browsed at the normal kernel.org git web browser: > > http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary > > thanks, > > greg k-h > > > > MAINTAINERS|1 + > Makefile |2 +- > arch/x86/kvm/vmx.c | 11 +-- > block/genhd.c |2 +- > crypto/algapi.c|3 ++- > drivers/block/nbd.c|3 ++- > drivers/cdrom/cdrom.c |2 +- > drivers/power/charger-manager.c|2 +- > drivers/scsi/osd/osd_uld.c |2 +- > drivers/scsi/sd.c |2 +- > drivers/tty/serial/8250/8250_pci.c |4 > fs/ceph/xattr.c|9 + > fs/hpfs/map.c |3 ++- > fs/hpfs/super.c|8 +++- > fs/nfsd/nfs4xdr.c |2 +- > include/linux/hugetlb.h| 16 > kernel/futex.c |3 ++- > kernel/module.c| 34 ++ > mm/hugetlb.c | 17 + > mm/memcontrol.c|2 -- > net/ceph/auth_none.c |6 ++ > 21 files changed, 94 insertions(+), 40 deletions(-) > > Ben Hutchings (1): > SCSI: sd: Fix parsing of 'temporary ' cache mode prefix > > Gleb Natapov (1): > KVM: VMX: mark unusable segment as nonpresent > > Greg Kroah-Hartman (3): > MAINTAINERS: add stable_kernel_rules.txt to stable maintainer > information > Revert "serial: 8250_pci: add support for another kind of NetMos > Technology PCI 9835 Multi-I/O Controller" > Linux 3.9.10 > > J. Bruce Fields (1): > nfsd4: fix decoding of compounds across page boundaries > > Jonathan Salwan (1): > drivers/cdrom/cdrom.c: use kzalloc() for failing hardware > > Kees Cook (3): > charger-manager: Ensure event is not used as format string > block: do not pass disk names as format strings > crypto: sanitize argument for format string > > Michal Hocko (1): > Revert "memcg: avoid dangling reference count in creation failure" > > Mikulas Patocka (1): > hpfs: better test for errors > > Rusty Russell (1): > module: do percpu allocation after uniqueness check. No, really! > > Tyler Hicks (1): > libceph: Fix NULL pointer dereference in auth client code > > Zhang Yi (1): > futex: Take hugepages into account when generating futex_key > > majianpeng (1): > ceph: fix sleeping function called from invalid context. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux 3.9.10
On Sat, Jul 13, 2013 at 11:54:17AM -0700, Greg KH wrote: I'm announcing the release of the 3.9.10 kernel. Note, this might just be the last 3.9-stable kernel release, I'm not quite sure I can guarantee another 3.9-stable kernel will be released. I guess this means 3.9 will NOT be this years LTS kernel. yes?¬ ¬ Do you think its safe to assume 3.10 will be the LTS for 2013?¬ ¬ thank! --mark Please move to the 3.10-stable series at this point in time. If that isn't working for you, please let us know NOW! All users of the 3.9 kernel series must upgrade. The updated 3.9.y git tree can be found at: git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git linux-3.9.y and can be browsed at the normal kernel.org git web browser: http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary thanks, greg k-h MAINTAINERS|1 + Makefile |2 +- arch/x86/kvm/vmx.c | 11 +-- block/genhd.c |2 +- crypto/algapi.c|3 ++- drivers/block/nbd.c|3 ++- drivers/cdrom/cdrom.c |2 +- drivers/power/charger-manager.c|2 +- drivers/scsi/osd/osd_uld.c |2 +- drivers/scsi/sd.c |2 +- drivers/tty/serial/8250/8250_pci.c |4 fs/ceph/xattr.c|9 + fs/hpfs/map.c |3 ++- fs/hpfs/super.c|8 +++- fs/nfsd/nfs4xdr.c |2 +- include/linux/hugetlb.h| 16 kernel/futex.c |3 ++- kernel/module.c| 34 ++ mm/hugetlb.c | 17 + mm/memcontrol.c|2 -- net/ceph/auth_none.c |6 ++ 21 files changed, 94 insertions(+), 40 deletions(-) Ben Hutchings (1): SCSI: sd: Fix parsing of 'temporary ' cache mode prefix Gleb Natapov (1): KVM: VMX: mark unusable segment as nonpresent Greg Kroah-Hartman (3): MAINTAINERS: add stable_kernel_rules.txt to stable maintainer information Revert serial: 8250_pci: add support for another kind of NetMos Technology PCI 9835 Multi-I/O Controller Linux 3.9.10 J. Bruce Fields (1): nfsd4: fix decoding of compounds across page boundaries Jonathan Salwan (1): drivers/cdrom/cdrom.c: use kzalloc() for failing hardware Kees Cook (3): charger-manager: Ensure event is not used as format string block: do not pass disk names as format strings crypto: sanitize argument for format string Michal Hocko (1): Revert memcg: avoid dangling reference count in creation failure Mikulas Patocka (1): hpfs: better test for errors Rusty Russell (1): module: do percpu allocation after uniqueness check. No, really! Tyler Hicks (1): libceph: Fix NULL pointer dereference in auth client code Zhang Yi (1): futex: Take hugepages into account when generating futex_key majianpeng (1): ceph: fix sleeping function called from invalid context. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] PM / devfreq: add PM-QoS support
device_register(>dev);
> if (err) {
> put_device(>dev);
> - goto err_dev;
> + goto err_qos_add;
> }
>
> if (governor->init)
> @@ -471,6 +593,11 @@ out:
>
> err_init:
> device_unregister(>dev);
> +err_qos_add:
> + if (profile->enable_dev_pm_qos)
> + dev_pm_qos_remove_notifier(dev, >qos_nb);
> + if (profile->qos_type)
> + pm_qos_remove_notifier(profile->qos_type, >qos_nb);
> err_dev:
> mutex_unlock(>lock);
> kfree(devfreq);
> @@ -568,6 +695,13 @@ static ssize_t show_central_polling(struct device *dev,
> !to_devfreq(dev)->governor->no_central_polling);
> }
>
> +static ssize_t show_qos_min_freq(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%lu\n", to_devfreq(dev)->qos_min_freq);
> +}
> +
> static ssize_t store_min_freq(struct device *dev, struct device_attribute
> *attr,
> const char *buf, size_t count)
> {
> @@ -685,6 +819,7 @@ static struct device_attribute devfreq_attrs[] = {
> store_polling_interval),
> __ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
> __ATTR(max_freq, S_IRUGO | S_IWUSR, show_max_freq, store_max_freq),
> + __ATTR(qos_min_freq, S_IRUGO, show_qos_min_freq, NULL),
> __ATTR(trans_stat, S_IRUGO, show_trans_table, NULL),
> { },
> };
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 30dc0d8..2488343 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -53,6 +53,21 @@ struct devfreq_dev_status {
> #define DEVFREQ_FLAG_LEAST_UPPER_BOUND 0x1
>
> /**
> + * struct devfreq_pm_qos_table - An PM QoS requiement entry for devfreq dev.
> + * @freq Lowest frequency to meet the QoS requirement
> + * represented by qos_value. If freq=0, it means that
> + * this element is the last in the array.
> + * @qos_valueThe qos value defined in pm_qos.h
> + *
> + * Note that the array of devfreq_pm_qos_table should be sorted by freq
> + * in the ascending order except for the last element, which should be 0.
> + */
> +struct devfreq_pm_qos_table {
> + unsigned long freq; /* 0 if this is the last element */
> + s32 qos_value;
> +};
> +
> +/**
> * struct devfreq_dev_profile - Devfreq's user device profile
> * @initial_freq The operating frequency when devfreq_add_device() is
> * called.
> @@ -71,11 +86,33 @@ struct devfreq_dev_status {
> * from devfreq_remove_device() call. If the user
> * has registered devfreq->nb at a notifier-head,
> * this is the time to unregister it.
> + * @qos_type QoS Type (defined in pm_qos.h)
> + * 0 (PM_QOS_RESERVED) if not used.
> + * @qos_use_max True: the highest QoS value is used (for QoS
> + * requirement of throughput, bandwidth, or similar)
> + * False: the lowest QoS value is used (for QoS
> + * requirement of latency, delay, or similar)
> + * @enable_dev_pm_qosdev_pm_qos is enabled using the qos_list.
> + * @qos_list Array of QoS requirements ending with .freq = 0
> + * NULL if not used. It should be either NULL or
> + * have a length > 1 with a first element effective.
> + * This QoS specification is shared by the global
> + * PM QoS (qos_type) and the per-dev PM QoS
> + * (enable_dev_pm_qos).
> + *
> + * Note that the array of qos_list should be sorted by freq
> + * in the ascending order.
> */
> struct devfreq_dev_profile {
> unsigned long initial_freq;
> unsigned int polling_ms;
>
> + /* Optional QoS Handling Specification */
> + int qos_type; /* 0: No global PM-QoS support */
> + bool qos_use_max; /* true if "bandwidth"/"throughput"-like values */
> + bool enable_dev_pm_qos; /* False: No per-dev PM-QoS support */
> + struct devfreq_pm_qos_table *qos_list; /* QoS handling specification */
> +
> int (*target)(struct device *dev, unsigned long *freq, u32 flags);
> int (*get_dev_status)(struct device *dev,
> struct devfreq_dev_status *stat);
> @@ -139,6 +176,8 @@ struct devfreq_governor {
> * order to prevent trying to remove the object multiple
> times.
> * @min_freq Limit minimum frequency requested by user (0: none)
> * @max_freq Limit maximum frequency requested by user (0: none)
> + * @qos_nb notifier block used to notify pm qos requests
> + * @qos_min_freq Limit minimum frequency requested by QoS
> *
> * This structure stores the devfreq information for a give device.
> *
> @@ -167,6 +206,8 @@ struct devfreq {
>
> unsigned long min_freq;
> unsigned long max_freq;
> + struct notifier_block qos_nb;
> + unsigned long qos_min_freq;
>
> /* information for device freqeuncy transition */
> unsigned int total_trans;
> --
> 1.7.4.1
looks ok to me.
Reviewed-by: mark gross
--mark
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] PM / QoS: Prepare struct dev_pm_qos_request for more request types
On Mon, Oct 08, 2012 at 10:06:08AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> The subsequent patches will use struct dev_pm_qos_request for
> representing both latency requests and flags requests. To make that
> easier, put the node member of struct dev_pm_qos_request (under the
> name "pnode") into a union called "data" that will represent the
> request's value and list node depending on its type.
>
> Signed-off-by: Rafael J. Wysocki
> Reviewed-by: Jean Pihet
> ---
> drivers/base/power/qos.c |6 +++---
> drivers/base/power/sysfs.c |2 +-
> include/linux/pm_qos.h |4 +++-
> 3 files changed, 7 insertions(+), 5 deletions(-)
>
> Index: linux/include/linux/pm_qos.h
> ===
> --- linux.orig/include/linux/pm_qos.h
> +++ linux/include/linux/pm_qos.h
> @@ -39,7 +39,9 @@ struct pm_qos_flags_request {
> };
>
> struct dev_pm_qos_request {
> - struct plist_node node;
> + union {
> + struct plist_node pnode;
> + } data;
> struct device *dev;
> };
>
> Index: linux/drivers/base/power/sysfs.c
> ===
> --- linux.orig/drivers/base/power/sysfs.c
> +++ linux/drivers/base/power/sysfs.c
> @@ -221,7 +221,7 @@ static DEVICE_ATTR(autosuspend_delay_ms,
> static ssize_t pm_qos_latency_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return sprintf(buf, "%d\n", dev->power.pq_req->node.prio);
> + return sprintf(buf, "%d\n", dev->power.pq_req->data.pnode.prio);
> }
>
> static ssize_t pm_qos_latency_store(struct device *dev,
> Index: linux/drivers/base/power/qos.c
> ===
> --- linux.orig/drivers/base/power/qos.c
> +++ linux/drivers/base/power/qos.c
> @@ -90,7 +90,7 @@ static int apply_constraint(struct dev_p
> int ret, curr_value;
>
> ret = pm_qos_update_target(>dev->power.qos->latency,
> ->node, action, value);
> +>data.pnode, action, value);
>
> if (ret) {
> /* Call the global callbacks if needed */
> @@ -183,7 +183,7 @@ void dev_pm_qos_constraints_destroy(stru
>
> c = >latency;
> /* Flush the constraints list for the device */
> - plist_for_each_entry_safe(req, tmp, >list, node) {
> + plist_for_each_entry_safe(req, tmp, >list, data.pnode) {
> /*
>* Update constraints list and call the notification
>* callbacks if needed
> @@ -293,7 +293,7 @@ int dev_pm_qos_update_request(struct dev
> mutex_lock(_pm_qos_mtx);
>
> if (req->dev->power.qos) {
> - if (new_value != req->node.prio)
> + if (new_value != req->data.pnode.prio)
> ret = apply_constraint(req, PM_QOS_UPDATE_REQ,
> new_value);
> } else {
>
Reviewed-by: mark gross
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] PM / QoS: Introduce PM QoS device flags support
_qos_flags_status {
> + PM_QOS_FLAGS_UNDEFINED = -1,
> + PM_QOS_FLAGS_NONE,
> + PM_QOS_FLAGS_SOME,
> + PM_QOS_FLAGS_ALL,
> +};
> +
> #define PM_QOS_DEFAULT_VALUE -1
>
> #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC)
> @@ -38,9 +45,16 @@ struct pm_qos_flags_request {
> s32 flags; /* Do not change to 64 bit */
> };
>
> +enum dev_pm_qos_req_type {
> + DEV_PM_QOS_LATENCY = 1,
> + DEV_PM_QOS_FLAGS,
> +};
> +
> struct dev_pm_qos_request {
> + enum dev_pm_qos_req_type type;
> union {
> struct plist_node pnode;
> + struct pm_qos_flags_request flr;
> } data;
> struct device *dev;
> };
> @@ -71,6 +85,7 @@ struct pm_qos_flags {
>
> struct dev_pm_qos {
> struct pm_qos_constraints latency;
> + struct pm_qos_flags flags;
> };
>
> /* Action requested to pm_qos_update_target */
> @@ -105,10 +120,12 @@ int pm_qos_request_active(struct pm_qos_
> s32 pm_qos_read_value(struct pm_qos_constraints *c);
>
> #ifdef CONFIG_PM
> +enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev, s32 mask);
> +enum pm_qos_flags_status dev_pm_qos_flags(struct device *dev, s32 mask);
> s32 __dev_pm_qos_read_value(struct device *dev);
> s32 dev_pm_qos_read_value(struct device *dev);
> int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request
> *req,
> -s32 value);
> +enum dev_pm_qos_req_type type, s32 value);
> int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value);
> int dev_pm_qos_remove_request(struct dev_pm_qos_request *req);
> int dev_pm_qos_add_notifier(struct device *dev,
> @@ -122,12 +139,19 @@ void dev_pm_qos_constraints_destroy(stru
> int dev_pm_qos_add_ancestor_request(struct device *dev,
> struct dev_pm_qos_request *req, s32 value);
> #else
> +static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
> + s32 mask)
> + { return PM_QOS_FLAGS_UNDEFINED; }
> +static inline enum pm_qos_flags_status dev_pm_qos_flags(struct device *dev,
> + s32 mask)
> + { return PM_QOS_FLAGS_UNDEFINED; }
> static inline s32 __dev_pm_qos_read_value(struct device *dev)
> { return 0; }
> static inline s32 dev_pm_qos_read_value(struct device *dev)
> { return 0; }
> static inline int dev_pm_qos_add_request(struct device *dev,
>struct dev_pm_qos_request *req,
> + enum dev_pm_qos_req_type type,
>s32 value)
> { return 0; }
> static inline int dev_pm_qos_update_request(struct dev_pm_qos_request *req,
> Index: linux/drivers/mtd/nand/sh_flctl.c
> ===
> --- linux.orig/drivers/mtd/nand/sh_flctl.c
> +++ linux/drivers/mtd/nand/sh_flctl.c
> @@ -710,7 +710,9 @@ static void flctl_select_chip(struct mtd
>
> if (!flctl->qos_request) {
> ret = dev_pm_qos_add_request(>pdev->dev,
> - >pm_qos, 100);
> + >pm_qos,
> + DEV_PM_QOS_LATENCY,
> + 100);
> if (ret < 0)
> dev_err(>pdev->dev,
> "PM QoS request failed: %d\n", ret);
> Index: linux/Documentation/power/pm_qos_interface.txt
> ===
> --- linux.orig/Documentation/power/pm_qos_interface.txt
> +++ linux/Documentation/power/pm_qos_interface.txt
> @@ -99,7 +99,7 @@ reading the aggregated value does not re
>
> From kernel mode the use of this interface is the following:
>
> -int dev_pm_qos_add_request(device, handle, value):
> +int dev_pm_qos_add_request(device, handle, type, value):
> Will insert an element into the list for that identified device with the
> target value. Upon change to this list the new target is recomputed and any
> registered notifiers are called only if the target value is now different.
>
Reviewed-by: mark gross
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/7] PM / ACPI: Take device PM QoS flags into account
On Mon, Oct 08, 2012 at 10:09:26AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> Make ACPI power management routines and PCI power management
> routines depending on ACPI take device PM QoS flags into account
> when deciding what power state to put the device into.
>
> In particular, after this change acpi_pm_device_sleep_state() will
> not return ACPI_STATE_D3_COLD as the deepest available low-power
> state if PM_QOS_FLAG_NO_POWER_OFF is requested for the device and it
> will not require remote wakeup to work for the device in the returned
> low-power state if there is at least one PM QoS flags request for the
> device, but PM_QOS_FLAG_REMOTE_WAKEUP is not requested for it.
>
> Accordingly, acpi_pci_set_power_state() will refuse to put the
> device into D3cold if PM_QOS_FLAG_NO_POWER_OFF is requested for it.
>
> Signed-off-by: Rafael J. Wysocki
> Reviewed-by: Jean Pihet
> ---
> drivers/acpi/sleep.c | 21 +
> drivers/pci/pci-acpi.c |8 +++-
> 2 files changed, 24 insertions(+), 5 deletions(-)
>
> Index: linux/drivers/pci/pci-acpi.c
> ===
> --- linux.orig/drivers/pci/pci-acpi.c
> +++ linux/drivers/pci/pci-acpi.c
> @@ -17,6 +17,7 @@
>
> #include
> #include
> +#include
> #include "pci.h"
>
> static DEFINE_MUTEX(pci_acpi_pm_notify_mtx);
> @@ -257,11 +258,16 @@ static int acpi_pci_set_power_state(stru
> return -ENODEV;
>
> switch (state) {
> + case PCI_D3cold:
> + if (dev_pm_qos_flags(>dev, PM_QOS_FLAG_NO_POWER_OFF) ==
> + PM_QOS_FLAGS_ALL) {
> + error = -EBUSY;
> + break;
> + }
> case PCI_D0:
> case PCI_D1:
> case PCI_D2:
> case PCI_D3hot:
> - case PCI_D3cold:
> error = acpi_bus_set_power(handle, state_conv[state]);
> }
>
> Index: linux/drivers/acpi/sleep.c
> ===
> --- linux.orig/drivers/acpi/sleep.c
> +++ linux/drivers/acpi/sleep.c
> @@ -19,6 +19,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -711,6 +712,7 @@ int acpi_pm_device_sleep_state(struct de
> struct acpi_device *adev;
> char acpi_method[] = "_SxD";
> unsigned long long d_min, d_max;
> + bool wakeup = false;
>
> if (d_max_in < ACPI_STATE_D0 || d_max_in > ACPI_STATE_D3)
> return -EINVAL;
> @@ -718,6 +720,13 @@ int acpi_pm_device_sleep_state(struct de
> printk(KERN_DEBUG "ACPI handle has no context!\n");
> return -ENODEV;
> }
> + if (d_max_in > ACPI_STATE_D3_HOT) {
> + enum pm_qos_flags_status stat;
> +
> + stat = dev_pm_qos_flags(dev, PM_QOS_FLAG_NO_POWER_OFF);
> + if (stat == PM_QOS_FLAGS_ALL)
> + d_max_in = ACPI_STATE_D3_HOT;
> + }
>
> acpi_method[2] = '0' + acpi_target_sleep_state;
> /*
> @@ -737,8 +746,14 @@ int acpi_pm_device_sleep_state(struct de
>* NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
>* provided -- that's our fault recovery, we ignore retval.
>*/
> - if (acpi_target_sleep_state > ACPI_STATE_S0)
> + if (acpi_target_sleep_state > ACPI_STATE_S0) {
> acpi_evaluate_integer(handle, acpi_method, NULL, _min);
> + wakeup = device_may_wakeup(dev) && adev->wakeup.flags.valid
> + && adev->wakeup.sleep_state >= acpi_target_sleep_state;
> + } else if (dev_pm_qos_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP) !=
> + PM_QOS_FLAGS_NONE) {
> + wakeup = adev->wakeup.flags.valid;
> + }
>
> /*
>* If _PRW says we can wake up the system from the target sleep state,
> @@ -747,9 +762,7 @@ int acpi_pm_device_sleep_state(struct de
>* (ACPI 3.x), it should return the maximum (lowest power) D-state that
>* can wake the system. _S0W may be valid, too.
>*/
> - if (acpi_target_sleep_state == ACPI_STATE_S0 ||
> - (device_may_wakeup(dev) && adev->wakeup.flags.valid &&
> - adev->wakeup.sleep_state >= acpi_target_sleep_state)) {
> + if (wakeup) {
> acpi_status status;
>
> acpi_method[3] = 'W';
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Reviewed-by: mark gross
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/7] PM / Domains: Check device PM QoS flags in pm_genpd_poweroff()
On Mon, Oct 08, 2012 at 10:08:39AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> Make the generic PM domains pm_genpd_poweroff() function take
> device PM QoS flags into account when deciding whether or not to
> remove power from the domain.
>
> After this change the routine will return -EBUSY without executing
> the domain's .power_off() callback if there is at least one PM QoS
> flags request for at least one device in the domain and at least of
> those request has at least one of the NO_POWER_OFF and REMOTE_WAKEUP
> flags set.
>
> Signed-off-by: Rafael J. Wysocki
> Reviewed-by: Jean Pihet
> ---
> drivers/base/power/domain.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> Index: linux/drivers/base/power/domain.c
> ===
> --- linux.orig/drivers/base/power/domain.c
> +++ linux/drivers/base/power/domain.c
> @@ -470,10 +470,19 @@ static int pm_genpd_poweroff(struct gene
> return -EBUSY;
>
> not_suspended = 0;
> - list_for_each_entry(pdd, >dev_list, list_node)
> + list_for_each_entry(pdd, >dev_list, list_node) {
> + enum pm_qos_flags_status stat;
> +
> + stat = dev_pm_qos_flags(pdd->dev,
> + PM_QOS_FLAG_NO_POWER_OFF
> + | PM_QOS_FLAG_REMOTE_WAKEUP);
> + if (stat > PM_QOS_FLAGS_NONE)
> + return -EBUSY;
> +
> if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
> || pdd->dev->power.irq_safe))
> not_suspended++;
> + }
>
> if (not_suspended > genpd->in_progress)
> return -EBUSY;
>
looks ok to me.
Reviewed-by: mark gross
--mrak
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/7] PM / QoS: Make it possible to expose PM QoS device flags to user space
m_qos_latency_store);
> +
> +static ssize_t pm_qos_no_power_off_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(dev_pm_qos_requested_flags(dev)
> + & PM_QOS_FLAG_NO_POWER_OFF));
> +}
> +
> +static ssize_t pm_qos_no_power_off_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + int ret;
> +
> + if (kstrtoint(buf, 0, ))
> + return -EINVAL;
> +
> + if (ret != 0 && ret != 1)
> + return -EINVAL;
> +
> + ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_NO_POWER_OFF, ret);
> + return ret < 0 ? ret : n;
> +}
> +
> +static DEVICE_ATTR(pm_qos_no_power_off, 0644,
> +pm_qos_no_power_off_show, pm_qos_no_power_off_store);
> +
> +static ssize_t pm_qos_remote_wakeup_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(dev_pm_qos_requested_flags(dev)
> + & PM_QOS_FLAG_REMOTE_WAKEUP));
> +}
> +
> +static ssize_t pm_qos_remote_wakeup_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + s32 value;
> + int ret;
> +
> + if (kstrtoint(buf, 0, ))
> + return -EINVAL;
> +
> + if (ret != 0 && ret != 1)
> + return -EINVAL;
> +
> + ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP, ret);
> + return ret < 0 ? ret : n;
> +}
> +
> +static DEVICE_ATTR(pm_qos_remote_wakeup, 0644,
> +pm_qos_remote_wakeup_show, pm_qos_remote_wakeup_store);
> #endif /* CONFIG_PM_RUNTIME */
>
> #ifdef CONFIG_PM_SLEEP
> @@ -564,15 +619,27 @@ static struct attribute_group pm_runtime
> .attrs = runtime_attrs,
> };
>
> -static struct attribute *pm_qos_attrs[] = {
> +static struct attribute *pm_qos_latency_attrs[] = {
> #ifdef CONFIG_PM_RUNTIME
> _attr_pm_qos_resume_latency_us.attr,
> #endif /* CONFIG_PM_RUNTIME */
> NULL,
> };
> -static struct attribute_group pm_qos_attr_group = {
> +static struct attribute_group pm_qos_latency_attr_group = {
> + .name = power_group_name,
> + .attrs = pm_qos_latency_attrs,
> +};
> +
> +static struct attribute *pm_qos_flags_attrs[] = {
> +#ifdef CONFIG_PM_RUNTIME
> + _attr_pm_qos_no_power_off.attr,
> + _attr_pm_qos_remote_wakeup.attr,
> +#endif /* CONFIG_PM_RUNTIME */
> + NULL,
> +};
> +static struct attribute_group pm_qos_flags_attr_group = {
> .name = power_group_name,
> - .attrs = pm_qos_attrs,
> + .attrs = pm_qos_flags_attrs,
> };
>
> int dpm_sysfs_add(struct device *dev)
> @@ -615,14 +682,24 @@ void wakeup_sysfs_remove(struct device *
> sysfs_unmerge_group(>kobj, _wakeup_attr_group);
> }
>
> -int pm_qos_sysfs_add(struct device *dev)
> +int pm_qos_sysfs_add_latency(struct device *dev)
> +{
> + return sysfs_merge_group(>kobj, _qos_latency_attr_group);
> +}
> +
> +void pm_qos_sysfs_remove_latency(struct device *dev)
> +{
> + sysfs_unmerge_group(>kobj, _qos_latency_attr_group);
> +}
> +
> +int pm_qos_sysfs_add_flags(struct device *dev)
> {
> - return sysfs_merge_group(>kobj, _qos_attr_group);
> + return sysfs_merge_group(>kobj, _qos_flags_attr_group);
> }
>
> -void pm_qos_sysfs_remove(struct device *dev)
> +void pm_qos_sysfs_remove_flags(struct device *dev)
> {
> - sysfs_unmerge_group(>kobj, _qos_attr_group);
> + sysfs_unmerge_group(>kobj, _qos_flags_attr_group);
> }
>
> void rpm_sysfs_remove(struct device *dev)
> Index: linux/Documentation/ABI/testing/sysfs-devices-power
> ===
> --- linux.orig/Documentation/ABI/testing/sysfs-devices-power
> +++ linux/Documentation/ABI/testing/sysfs-devices-power
> @@ -204,3 +204,34 @@ Description:
>
> This attribute has no effect on system-wide suspend/resume and
> hibernation.
> +
> +What:/sys/devices/.../power/pm_qos_no_power_off
> +Date:September 2012
> +Contact: Rafael J. Wysocki
> +Description:
> + The /sys/devices/.../power/pm_qos_no_power_off attribute
> + is used for manipulating the PM QoS "no power off" flag. If
> + set, this flag indicates to the kernel that power should not
> + be removed entirely from the device.
> +
> + Not all drivers support this attribute. If it isn't supported,
> + it is not present.
> +
> + This attribute has no effect on system-wide suspend/resume and
> + hibernation.
> +
> +What:/sys/devices/.../power/pm_qos_remote_wakeup
> +Date:September 2012
> +Contact: Rafael J. Wysocki
> +Description:
> + The /sys/devices/.../power/pm_qos_remote_wakeup attribute
> + is used for manipulating the PM QoS "remote wakeup required"
> + flag. If set, this flag indicates to the kernel that the
> + device is a source of user events that have to be signaled from
> + its low-power states.
> +
> + Not all drivers support this attribute. If it isn't supported,
> + it is not present.
> +
> + This attribute has no effect on system-wide suspend/resume and
> + hibernation.
>
acked-by: mark gross
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/7] PM / QoS: Introduce request and constraint data types for PM QoS flags
action action, s32 val)
> +{
> + unsigned long irqflags;
> + s32 prev_value, curr_value;
> +
> + spin_lock_irqsave(_qos_lock, irqflags);
> +
> + prev_value = list_empty(>list) ? 0 : pqf->effective_flags;
> +
> + switch (action) {
> + case PM_QOS_REMOVE_REQ:
> + pm_qos_flags_remove_req(pqf, req);
> + break;
> + case PM_QOS_UPDATE_REQ:
> + pm_qos_flags_remove_req(pqf, req);
> + case PM_QOS_ADD_REQ:
> + req->flags = val;
> + INIT_LIST_HEAD(>node);
> + list_add_tail(>node, >list);
> + pqf->effective_flags |= val;
> + break;
> + default:
> + /* no action */
> + ;
> + }
> +
> + curr_value = list_empty(>list) ? 0 : pqf->effective_flags;
> +
> + spin_unlock_irqrestore(_qos_lock, irqflags);
> +
> + return prev_value != curr_value;
> +}
> +
> +/**
> * pm_qos_request - returns current system wide qos expectation
> * @pm_qos_class: identification of which qos value is requested
> *
>
acked-by: mark gross
--mark
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/7] PM / QoS: Prepare device structure for adding more constraint types
On Mon, Oct 08, 2012 at 10:04:03AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> Currently struct dev_pm_info contains only one PM QoS constraints
> pointer reserved for latency requirements. Since one more device
> constraints type (i.e. flags) will be necessary, introduce a new
> structure, struct dev_pm_qos, that eventually will contain all of
> the available device PM QoS constraints and replace the "constraints"
> pointer in struct dev_pm_info with a pointer to the new structure
> called "qos".
>
> Signed-off-by: Rafael J. Wysocki
> Reviewed-by: Jean Pihet
> ---
> drivers/base/power/qos.c | 42 ++
> include/linux/pm.h |2 +-
> include/linux/pm_qos.h |4
> 3 files changed, 27 insertions(+), 21 deletions(-)
>
> Index: linux/include/linux/pm.h
> ===
> --- linux.orig/include/linux/pm.h
> +++ linux/include/linux/pm.h
> @@ -551,7 +551,7 @@ struct dev_pm_info {
> struct dev_pm_qos_request *pq_req;
> #endif
> struct pm_subsys_data *subsys_data; /* Owned by the subsystem. */
> - struct pm_qos_constraints *constraints;
> + struct dev_pm_qos *qos;
> };
>
> extern void update_pm_runtime_accounting(struct device *dev);
> Index: linux/include/linux/pm_qos.h
> ===
> --- linux.orig/include/linux/pm_qos.h
> +++ linux/include/linux/pm_qos.h
> @@ -57,6 +57,10 @@ struct pm_qos_constraints {
> struct blocking_notifier_head *notifiers;
> };
>
> +struct dev_pm_qos {
> + struct pm_qos_constraints latency;
What about non-latency constraints? This pretty much makes it explicit
that dev_pm_qos is all about latency. from the commit comment I thought
you where trying to make it more genaric. Why not call "latency"
"constraint" or something less specific?
--mark
> +};
> +
> /* Action requested to pm_qos_update_target */
> enum pm_qos_req_action {
> PM_QOS_ADD_REQ, /* Add a new request */
> Index: linux/drivers/base/power/qos.c
> ===
> --- linux.orig/drivers/base/power/qos.c
> +++ linux/drivers/base/power/qos.c
> @@ -55,9 +55,7 @@ static BLOCKING_NOTIFIER_HEAD(dev_pm_not
> */
> s32 __dev_pm_qos_read_value(struct device *dev)
> {
> - struct pm_qos_constraints *c = dev->power.constraints;
> -
> - return c ? pm_qos_read_value(c) : 0;
> + return dev->power.qos ? pm_qos_read_value(>power.qos->latency) : 0;
> }
>
> /**
> @@ -91,12 +89,12 @@ static int apply_constraint(struct dev_p
> {
> int ret, curr_value;
>
> - ret = pm_qos_update_target(req->dev->power.constraints,
> + ret = pm_qos_update_target(>dev->power.qos->latency,
> >node, action, value);
>
> if (ret) {
> /* Call the global callbacks if needed */
> - curr_value = pm_qos_read_value(req->dev->power.constraints);
> + curr_value = pm_qos_read_value(>dev->power.qos->latency);
> blocking_notifier_call_chain(_pm_notifiers,
>(unsigned long)curr_value,
>req);
> @@ -114,20 +112,22 @@ static int apply_constraint(struct dev_p
> */
> static int dev_pm_qos_constraints_allocate(struct device *dev)
> {
> + struct dev_pm_qos *qos;
> struct pm_qos_constraints *c;
> struct blocking_notifier_head *n;
>
> - c = kzalloc(sizeof(*c), GFP_KERNEL);
> - if (!c)
> + qos = kzalloc(sizeof(*qos), GFP_KERNEL);
> + if (!qos)
> return -ENOMEM;
>
> n = kzalloc(sizeof(*n), GFP_KERNEL);
> if (!n) {
> - kfree(c);
> + kfree(qos);
> return -ENOMEM;
> }
> BLOCKING_INIT_NOTIFIER_HEAD(n);
>
> + c = >latency;
> plist_head_init(>list);
> c->target_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
> c->default_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
> @@ -135,7 +135,7 @@ static int dev_pm_qos_constraints_alloca
> c->notifiers = n;
>
> spin_lock_irq(>power.lock);
> - dev->power.constraints = c;
> + dev->power.qos = qos;
> spin_unlock_irq(>power.lock);
>
> return 0;
> @@ -151,7 +151,7 @@ static int dev_pm_qos_constraints_alloca
> void dev_pm_qos_constraints_init(struct device *dev)
> {
> mutex_lock(_pm_qos_mtx);
> - dev->power.constraints = NULL;
> + dev->power.qos = NULL;
> dev->power.power_state = PMSG_ON;
> mutex_unlock(_pm_qos_mtx);
> }
> @@ -164,6 +164,7 @@ void dev_pm_qos_constraints_init(struct
> */
> void dev_pm_qos_constraints_destroy(struct device *dev)
> {
> + struct dev_pm_qos *qos;
> struct dev_pm_qos_request *req, *tmp;
> struct pm_qos_constraints *c;
>
> @@ -176,10 +177,11 @@ void dev_pm_qos_constraints_destroy(stru
>
Re: [PATCH 1/7] PM / QoS: Prepare device structure for adding more constraint types
On Mon, Oct 08, 2012 at 10:04:03AM +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Currently struct dev_pm_info contains only one PM QoS constraints
pointer reserved for latency requirements. Since one more device
constraints type (i.e. flags) will be necessary, introduce a new
structure, struct dev_pm_qos, that eventually will contain all of
the available device PM QoS constraints and replace the constraints
pointer in struct dev_pm_info with a pointer to the new structure
called qos.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Reviewed-by: Jean Pihet j-pi...@ti.com
---
drivers/base/power/qos.c | 42 ++
include/linux/pm.h |2 +-
include/linux/pm_qos.h |4
3 files changed, 27 insertions(+), 21 deletions(-)
Index: linux/include/linux/pm.h
===
--- linux.orig/include/linux/pm.h
+++ linux/include/linux/pm.h
@@ -551,7 +551,7 @@ struct dev_pm_info {
struct dev_pm_qos_request *pq_req;
#endif
struct pm_subsys_data *subsys_data; /* Owned by the subsystem. */
- struct pm_qos_constraints *constraints;
+ struct dev_pm_qos *qos;
};
extern void update_pm_runtime_accounting(struct device *dev);
Index: linux/include/linux/pm_qos.h
===
--- linux.orig/include/linux/pm_qos.h
+++ linux/include/linux/pm_qos.h
@@ -57,6 +57,10 @@ struct pm_qos_constraints {
struct blocking_notifier_head *notifiers;
};
+struct dev_pm_qos {
+ struct pm_qos_constraints latency;
What about non-latency constraints? This pretty much makes it explicit
that dev_pm_qos is all about latency. from the commit comment I thought
you where trying to make it more genaric. Why not call latency
constraint or something less specific?
--mark
+};
+
/* Action requested to pm_qos_update_target */
enum pm_qos_req_action {
PM_QOS_ADD_REQ, /* Add a new request */
Index: linux/drivers/base/power/qos.c
===
--- linux.orig/drivers/base/power/qos.c
+++ linux/drivers/base/power/qos.c
@@ -55,9 +55,7 @@ static BLOCKING_NOTIFIER_HEAD(dev_pm_not
*/
s32 __dev_pm_qos_read_value(struct device *dev)
{
- struct pm_qos_constraints *c = dev-power.constraints;
-
- return c ? pm_qos_read_value(c) : 0;
+ return dev-power.qos ? pm_qos_read_value(dev-power.qos-latency) : 0;
}
/**
@@ -91,12 +89,12 @@ static int apply_constraint(struct dev_p
{
int ret, curr_value;
- ret = pm_qos_update_target(req-dev-power.constraints,
+ ret = pm_qos_update_target(req-dev-power.qos-latency,
req-node, action, value);
if (ret) {
/* Call the global callbacks if needed */
- curr_value = pm_qos_read_value(req-dev-power.constraints);
+ curr_value = pm_qos_read_value(req-dev-power.qos-latency);
blocking_notifier_call_chain(dev_pm_notifiers,
(unsigned long)curr_value,
req);
@@ -114,20 +112,22 @@ static int apply_constraint(struct dev_p
*/
static int dev_pm_qos_constraints_allocate(struct device *dev)
{
+ struct dev_pm_qos *qos;
struct pm_qos_constraints *c;
struct blocking_notifier_head *n;
- c = kzalloc(sizeof(*c), GFP_KERNEL);
- if (!c)
+ qos = kzalloc(sizeof(*qos), GFP_KERNEL);
+ if (!qos)
return -ENOMEM;
n = kzalloc(sizeof(*n), GFP_KERNEL);
if (!n) {
- kfree(c);
+ kfree(qos);
return -ENOMEM;
}
BLOCKING_INIT_NOTIFIER_HEAD(n);
+ c = qos-latency;
plist_head_init(c-list);
c-target_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
c-default_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
@@ -135,7 +135,7 @@ static int dev_pm_qos_constraints_alloca
c-notifiers = n;
spin_lock_irq(dev-power.lock);
- dev-power.constraints = c;
+ dev-power.qos = qos;
spin_unlock_irq(dev-power.lock);
return 0;
@@ -151,7 +151,7 @@ static int dev_pm_qos_constraints_alloca
void dev_pm_qos_constraints_init(struct device *dev)
{
mutex_lock(dev_pm_qos_mtx);
- dev-power.constraints = NULL;
+ dev-power.qos = NULL;
dev-power.power_state = PMSG_ON;
mutex_unlock(dev_pm_qos_mtx);
}
@@ -164,6 +164,7 @@ void dev_pm_qos_constraints_init(struct
*/
void dev_pm_qos_constraints_destroy(struct device *dev)
{
+ struct dev_pm_qos *qos;
struct dev_pm_qos_request *req, *tmp;
struct pm_qos_constraints *c;
@@ -176,10 +177,11 @@ void dev_pm_qos_constraints_destroy(stru
mutex_lock(dev_pm_qos_mtx);
dev-power.power_state = PMSG_INVALID;
-
Re: [PATCH 2/7] PM / QoS: Introduce request and constraint data types for PM QoS flags
-flags = val; + INIT_LIST_HEAD(req-node); + list_add_tail(req-node, pqf-list); + pqf-effective_flags |= val; + break; + default: + /* no action */ + ; + } + + curr_value = list_empty(pqf-list) ? 0 : pqf-effective_flags; + + spin_unlock_irqrestore(pm_qos_lock, irqflags); + + return prev_value != curr_value; +} + +/** * pm_qos_request - returns current system wide qos expectation * @pm_qos_class: identification of which qos value is requested * acked-by: mark gross markgr...@thegnar.org --mark -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/7] PM / QoS: Make it possible to expose PM QoS device flags to user space
, ret);
+ return ret 0 ? ret : n;
+}
+
+static DEVICE_ATTR(pm_qos_remote_wakeup, 0644,
+pm_qos_remote_wakeup_show, pm_qos_remote_wakeup_store);
#endif /* CONFIG_PM_RUNTIME */
#ifdef CONFIG_PM_SLEEP
@@ -564,15 +619,27 @@ static struct attribute_group pm_runtime
.attrs = runtime_attrs,
};
-static struct attribute *pm_qos_attrs[] = {
+static struct attribute *pm_qos_latency_attrs[] = {
#ifdef CONFIG_PM_RUNTIME
dev_attr_pm_qos_resume_latency_us.attr,
#endif /* CONFIG_PM_RUNTIME */
NULL,
};
-static struct attribute_group pm_qos_attr_group = {
+static struct attribute_group pm_qos_latency_attr_group = {
+ .name = power_group_name,
+ .attrs = pm_qos_latency_attrs,
+};
+
+static struct attribute *pm_qos_flags_attrs[] = {
+#ifdef CONFIG_PM_RUNTIME
+ dev_attr_pm_qos_no_power_off.attr,
+ dev_attr_pm_qos_remote_wakeup.attr,
+#endif /* CONFIG_PM_RUNTIME */
+ NULL,
+};
+static struct attribute_group pm_qos_flags_attr_group = {
.name = power_group_name,
- .attrs = pm_qos_attrs,
+ .attrs = pm_qos_flags_attrs,
};
int dpm_sysfs_add(struct device *dev)
@@ -615,14 +682,24 @@ void wakeup_sysfs_remove(struct device *
sysfs_unmerge_group(dev-kobj, pm_wakeup_attr_group);
}
-int pm_qos_sysfs_add(struct device *dev)
+int pm_qos_sysfs_add_latency(struct device *dev)
+{
+ return sysfs_merge_group(dev-kobj, pm_qos_latency_attr_group);
+}
+
+void pm_qos_sysfs_remove_latency(struct device *dev)
+{
+ sysfs_unmerge_group(dev-kobj, pm_qos_latency_attr_group);
+}
+
+int pm_qos_sysfs_add_flags(struct device *dev)
{
- return sysfs_merge_group(dev-kobj, pm_qos_attr_group);
+ return sysfs_merge_group(dev-kobj, pm_qos_flags_attr_group);
}
-void pm_qos_sysfs_remove(struct device *dev)
+void pm_qos_sysfs_remove_flags(struct device *dev)
{
- sysfs_unmerge_group(dev-kobj, pm_qos_attr_group);
+ sysfs_unmerge_group(dev-kobj, pm_qos_flags_attr_group);
}
void rpm_sysfs_remove(struct device *dev)
Index: linux/Documentation/ABI/testing/sysfs-devices-power
===
--- linux.orig/Documentation/ABI/testing/sysfs-devices-power
+++ linux/Documentation/ABI/testing/sysfs-devices-power
@@ -204,3 +204,34 @@ Description:
This attribute has no effect on system-wide suspend/resume and
hibernation.
+
+What:/sys/devices/.../power/pm_qos_no_power_off
+Date:September 2012
+Contact: Rafael J. Wysocki r...@sisk.pl
+Description:
+ The /sys/devices/.../power/pm_qos_no_power_off attribute
+ is used for manipulating the PM QoS no power off flag. If
+ set, this flag indicates to the kernel that power should not
+ be removed entirely from the device.
+
+ Not all drivers support this attribute. If it isn't supported,
+ it is not present.
+
+ This attribute has no effect on system-wide suspend/resume and
+ hibernation.
+
+What:/sys/devices/.../power/pm_qos_remote_wakeup
+Date:September 2012
+Contact: Rafael J. Wysocki r...@sisk.pl
+Description:
+ The /sys/devices/.../power/pm_qos_remote_wakeup attribute
+ is used for manipulating the PM QoS remote wakeup required
+ flag. If set, this flag indicates to the kernel that the
+ device is a source of user events that have to be signaled from
+ its low-power states.
+
+ Not all drivers support this attribute. If it isn't supported,
+ it is not present.
+
+ This attribute has no effect on system-wide suspend/resume and
+ hibernation.
acked-by: mark gross markgr...@thegnar.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/7] PM / Domains: Check device PM QoS flags in pm_genpd_poweroff()
On Mon, Oct 08, 2012 at 10:08:39AM +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Make the generic PM domains pm_genpd_poweroff() function take
device PM QoS flags into account when deciding whether or not to
remove power from the domain.
After this change the routine will return -EBUSY without executing
the domain's .power_off() callback if there is at least one PM QoS
flags request for at least one device in the domain and at least of
those request has at least one of the NO_POWER_OFF and REMOTE_WAKEUP
flags set.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Reviewed-by: Jean Pihet j-pi...@ti.com
---
drivers/base/power/domain.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
Index: linux/drivers/base/power/domain.c
===
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -470,10 +470,19 @@ static int pm_genpd_poweroff(struct gene
return -EBUSY;
not_suspended = 0;
- list_for_each_entry(pdd, genpd-dev_list, list_node)
+ list_for_each_entry(pdd, genpd-dev_list, list_node) {
+ enum pm_qos_flags_status stat;
+
+ stat = dev_pm_qos_flags(pdd-dev,
+ PM_QOS_FLAG_NO_POWER_OFF
+ | PM_QOS_FLAG_REMOTE_WAKEUP);
+ if (stat PM_QOS_FLAGS_NONE)
+ return -EBUSY;
+
if (pdd-dev-driver (!pm_runtime_suspended(pdd-dev)
|| pdd-dev-power.irq_safe))
not_suspended++;
+ }
if (not_suspended genpd-in_progress)
return -EBUSY;
looks ok to me.
Reviewed-by: mark gross markgr...@thegnar.org
--mrak
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/7] PM / ACPI: Take device PM QoS flags into account
On Mon, Oct 08, 2012 at 10:09:26AM +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Make ACPI power management routines and PCI power management
routines depending on ACPI take device PM QoS flags into account
when deciding what power state to put the device into.
In particular, after this change acpi_pm_device_sleep_state() will
not return ACPI_STATE_D3_COLD as the deepest available low-power
state if PM_QOS_FLAG_NO_POWER_OFF is requested for the device and it
will not require remote wakeup to work for the device in the returned
low-power state if there is at least one PM QoS flags request for the
device, but PM_QOS_FLAG_REMOTE_WAKEUP is not requested for it.
Accordingly, acpi_pci_set_power_state() will refuse to put the
device into D3cold if PM_QOS_FLAG_NO_POWER_OFF is requested for it.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Reviewed-by: Jean Pihet j-pi...@ti.com
---
drivers/acpi/sleep.c | 21 +
drivers/pci/pci-acpi.c |8 +++-
2 files changed, 24 insertions(+), 5 deletions(-)
Index: linux/drivers/pci/pci-acpi.c
===
--- linux.orig/drivers/pci/pci-acpi.c
+++ linux/drivers/pci/pci-acpi.c
@@ -17,6 +17,7 @@
#include linux/pci-acpi.h
#include linux/pm_runtime.h
+#include linux/pm_qos.h
#include pci.h
static DEFINE_MUTEX(pci_acpi_pm_notify_mtx);
@@ -257,11 +258,16 @@ static int acpi_pci_set_power_state(stru
return -ENODEV;
switch (state) {
+ case PCI_D3cold:
+ if (dev_pm_qos_flags(dev-dev, PM_QOS_FLAG_NO_POWER_OFF) ==
+ PM_QOS_FLAGS_ALL) {
+ error = -EBUSY;
+ break;
+ }
case PCI_D0:
case PCI_D1:
case PCI_D2:
case PCI_D3hot:
- case PCI_D3cold:
error = acpi_bus_set_power(handle, state_conv[state]);
}
Index: linux/drivers/acpi/sleep.c
===
--- linux.orig/drivers/acpi/sleep.c
+++ linux/drivers/acpi/sleep.c
@@ -19,6 +19,7 @@
#include linux/acpi.h
#include linux/module.h
#include linux/pm_runtime.h
+#include linux/pm_qos.h
#include asm/io.h
@@ -711,6 +712,7 @@ int acpi_pm_device_sleep_state(struct de
struct acpi_device *adev;
char acpi_method[] = _SxD;
unsigned long long d_min, d_max;
+ bool wakeup = false;
if (d_max_in ACPI_STATE_D0 || d_max_in ACPI_STATE_D3)
return -EINVAL;
@@ -718,6 +720,13 @@ int acpi_pm_device_sleep_state(struct de
printk(KERN_DEBUG ACPI handle has no context!\n);
return -ENODEV;
}
+ if (d_max_in ACPI_STATE_D3_HOT) {
+ enum pm_qos_flags_status stat;
+
+ stat = dev_pm_qos_flags(dev, PM_QOS_FLAG_NO_POWER_OFF);
+ if (stat == PM_QOS_FLAGS_ALL)
+ d_max_in = ACPI_STATE_D3_HOT;
+ }
acpi_method[2] = '0' + acpi_target_sleep_state;
/*
@@ -737,8 +746,14 @@ int acpi_pm_device_sleep_state(struct de
* NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
* provided -- that's our fault recovery, we ignore retval.
*/
- if (acpi_target_sleep_state ACPI_STATE_S0)
+ if (acpi_target_sleep_state ACPI_STATE_S0) {
acpi_evaluate_integer(handle, acpi_method, NULL, d_min);
+ wakeup = device_may_wakeup(dev) adev-wakeup.flags.valid
+ adev-wakeup.sleep_state = acpi_target_sleep_state;
+ } else if (dev_pm_qos_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP) !=
+ PM_QOS_FLAGS_NONE) {
+ wakeup = adev-wakeup.flags.valid;
+ }
/*
* If _PRW says we can wake up the system from the target sleep state,
@@ -747,9 +762,7 @@ int acpi_pm_device_sleep_state(struct de
* (ACPI 3.x), it should return the maximum (lowest power) D-state that
* can wake the system. _S0W may be valid, too.
*/
- if (acpi_target_sleep_state == ACPI_STATE_S0 ||
- (device_may_wakeup(dev) adev-wakeup.flags.valid
- adev-wakeup.sleep_state = acpi_target_sleep_state)) {
+ if (wakeup) {
acpi_status status;
acpi_method[3] = 'W';
--
To unsubscribe from this list: send the line unsubscribe linux-pm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Reviewed-by: mark gross markgr...@thegnar.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] PM / QoS: Introduce PM QoS device flags support
pm_qos_flags_status dev_pm_qos_flags(struct device *dev, s32 mask);
s32 __dev_pm_qos_read_value(struct device *dev);
s32 dev_pm_qos_read_value(struct device *dev);
int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request
*req,
-s32 value);
+enum dev_pm_qos_req_type type, s32 value);
int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value);
int dev_pm_qos_remove_request(struct dev_pm_qos_request *req);
int dev_pm_qos_add_notifier(struct device *dev,
@@ -122,12 +139,19 @@ void dev_pm_qos_constraints_destroy(stru
int dev_pm_qos_add_ancestor_request(struct device *dev,
struct dev_pm_qos_request *req, s32 value);
#else
+static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
+ s32 mask)
+ { return PM_QOS_FLAGS_UNDEFINED; }
+static inline enum pm_qos_flags_status dev_pm_qos_flags(struct device *dev,
+ s32 mask)
+ { return PM_QOS_FLAGS_UNDEFINED; }
static inline s32 __dev_pm_qos_read_value(struct device *dev)
{ return 0; }
static inline s32 dev_pm_qos_read_value(struct device *dev)
{ return 0; }
static inline int dev_pm_qos_add_request(struct device *dev,
struct dev_pm_qos_request *req,
+ enum dev_pm_qos_req_type type,
s32 value)
{ return 0; }
static inline int dev_pm_qos_update_request(struct dev_pm_qos_request *req,
Index: linux/drivers/mtd/nand/sh_flctl.c
===
--- linux.orig/drivers/mtd/nand/sh_flctl.c
+++ linux/drivers/mtd/nand/sh_flctl.c
@@ -710,7 +710,9 @@ static void flctl_select_chip(struct mtd
if (!flctl-qos_request) {
ret = dev_pm_qos_add_request(flctl-pdev-dev,
- flctl-pm_qos, 100);
+ flctl-pm_qos,
+ DEV_PM_QOS_LATENCY,
+ 100);
if (ret 0)
dev_err(flctl-pdev-dev,
PM QoS request failed: %d\n, ret);
Index: linux/Documentation/power/pm_qos_interface.txt
===
--- linux.orig/Documentation/power/pm_qos_interface.txt
+++ linux/Documentation/power/pm_qos_interface.txt
@@ -99,7 +99,7 @@ reading the aggregated value does not re
From kernel mode the use of this interface is the following:
-int dev_pm_qos_add_request(device, handle, value):
+int dev_pm_qos_add_request(device, handle, type, value):
Will insert an element into the list for that identified device with the
target value. Upon change to this list the new target is recomputed and any
registered notifiers are called only if the target value is now different.
Reviewed-by: mark gross markgr...@thegnar.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] PM / QoS: Prepare struct dev_pm_qos_request for more request types
On Mon, Oct 08, 2012 at 10:06:08AM +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
The subsequent patches will use struct dev_pm_qos_request for
representing both latency requests and flags requests. To make that
easier, put the node member of struct dev_pm_qos_request (under the
name pnode) into a union called data that will represent the
request's value and list node depending on its type.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Reviewed-by: Jean Pihet j-pi...@ti.com
---
drivers/base/power/qos.c |6 +++---
drivers/base/power/sysfs.c |2 +-
include/linux/pm_qos.h |4 +++-
3 files changed, 7 insertions(+), 5 deletions(-)
Index: linux/include/linux/pm_qos.h
===
--- linux.orig/include/linux/pm_qos.h
+++ linux/include/linux/pm_qos.h
@@ -39,7 +39,9 @@ struct pm_qos_flags_request {
};
struct dev_pm_qos_request {
- struct plist_node node;
+ union {
+ struct plist_node pnode;
+ } data;
struct device *dev;
};
Index: linux/drivers/base/power/sysfs.c
===
--- linux.orig/drivers/base/power/sysfs.c
+++ linux/drivers/base/power/sysfs.c
@@ -221,7 +221,7 @@ static DEVICE_ATTR(autosuspend_delay_ms,
static ssize_t pm_qos_latency_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, %d\n, dev-power.pq_req-node.prio);
+ return sprintf(buf, %d\n, dev-power.pq_req-data.pnode.prio);
}
static ssize_t pm_qos_latency_store(struct device *dev,
Index: linux/drivers/base/power/qos.c
===
--- linux.orig/drivers/base/power/qos.c
+++ linux/drivers/base/power/qos.c
@@ -90,7 +90,7 @@ static int apply_constraint(struct dev_p
int ret, curr_value;
ret = pm_qos_update_target(req-dev-power.qos-latency,
-req-node, action, value);
+req-data.pnode, action, value);
if (ret) {
/* Call the global callbacks if needed */
@@ -183,7 +183,7 @@ void dev_pm_qos_constraints_destroy(stru
c = qos-latency;
/* Flush the constraints list for the device */
- plist_for_each_entry_safe(req, tmp, c-list, node) {
+ plist_for_each_entry_safe(req, tmp, c-list, data.pnode) {
/*
* Update constraints list and call the notification
* callbacks if needed
@@ -293,7 +293,7 @@ int dev_pm_qos_update_request(struct dev
mutex_lock(dev_pm_qos_mtx);
if (req-dev-power.qos) {
- if (new_value != req-node.prio)
+ if (new_value != req-data.pnode.prio)
ret = apply_constraint(req, PM_QOS_UPDATE_REQ,
new_value);
} else {
Reviewed-by: mark gross markgr...@thegnar.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] PM / devfreq: add PM-QoS support
[] = {
store_polling_interval),
__ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
__ATTR(max_freq, S_IRUGO | S_IWUSR, show_max_freq, store_max_freq),
+ __ATTR(qos_min_freq, S_IRUGO, show_qos_min_freq, NULL),
__ATTR(trans_stat, S_IRUGO, show_trans_table, NULL),
{ },
};
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 30dc0d8..2488343 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -53,6 +53,21 @@ struct devfreq_dev_status {
#define DEVFREQ_FLAG_LEAST_UPPER_BOUND 0x1
/**
+ * struct devfreq_pm_qos_table - An PM QoS requiement entry for devfreq dev.
+ * @freq Lowest frequency to meet the QoS requirement
+ * represented by qos_value. If freq=0, it means that
+ * this element is the last in the array.
+ * @qos_valueThe qos value defined in pm_qos.h
+ *
+ * Note that the array of devfreq_pm_qos_table should be sorted by freq
+ * in the ascending order except for the last element, which should be 0.
+ */
+struct devfreq_pm_qos_table {
+ unsigned long freq; /* 0 if this is the last element */
+ s32 qos_value;
+};
+
+/**
* struct devfreq_dev_profile - Devfreq's user device profile
* @initial_freq The operating frequency when devfreq_add_device() is
* called.
@@ -71,11 +86,33 @@ struct devfreq_dev_status {
* from devfreq_remove_device() call. If the user
* has registered devfreq-nb at a notifier-head,
* this is the time to unregister it.
+ * @qos_type QoS Type (defined in pm_qos.h)
+ * 0 (PM_QOS_RESERVED) if not used.
+ * @qos_use_max True: the highest QoS value is used (for QoS
+ * requirement of throughput, bandwidth, or similar)
+ * False: the lowest QoS value is used (for QoS
+ * requirement of latency, delay, or similar)
+ * @enable_dev_pm_qosdev_pm_qos is enabled using the qos_list.
+ * @qos_list Array of QoS requirements ending with .freq = 0
+ * NULL if not used. It should be either NULL or
+ * have a length 1 with a first element effective.
+ * This QoS specification is shared by the global
+ * PM QoS (qos_type) and the per-dev PM QoS
+ * (enable_dev_pm_qos).
+ *
+ * Note that the array of qos_list should be sorted by freq
+ * in the ascending order.
*/
struct devfreq_dev_profile {
unsigned long initial_freq;
unsigned int polling_ms;
+ /* Optional QoS Handling Specification */
+ int qos_type; /* 0: No global PM-QoS support */
+ bool qos_use_max; /* true if bandwidth/throughput-like values */
+ bool enable_dev_pm_qos; /* False: No per-dev PM-QoS support */
+ struct devfreq_pm_qos_table *qos_list; /* QoS handling specification */
+
int (*target)(struct device *dev, unsigned long *freq, u32 flags);
int (*get_dev_status)(struct device *dev,
struct devfreq_dev_status *stat);
@@ -139,6 +176,8 @@ struct devfreq_governor {
* order to prevent trying to remove the object multiple
times.
* @min_freq Limit minimum frequency requested by user (0: none)
* @max_freq Limit maximum frequency requested by user (0: none)
+ * @qos_nb notifier block used to notify pm qos requests
+ * @qos_min_freq Limit minimum frequency requested by QoS
*
* This structure stores the devfreq information for a give device.
*
@@ -167,6 +206,8 @@ struct devfreq {
unsigned long min_freq;
unsigned long max_freq;
+ struct notifier_block qos_nb;
+ unsigned long qos_min_freq;
/* information for device freqeuncy transition */
unsigned int total_trans;
--
1.7.4.1
looks ok to me.
Reviewed-by: mark gross markgr...@thegnar.org
--mark
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support
On Mon, Sep 17, 2012 at 11:10:09PM +0200, Rafael J. Wysocki wrote:
> On Monday, September 17, 2012, MyungJoo Ham wrote:
> > Sender : Rafael J. Wysocki
> > Date : 2012-09-09 07:20 (GMT+09:00)
> > Title : Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support
> > > On Thursday, August 30, 2012, MyungJoo Ham wrote:
> > > > Even if the performance of a device is controlled properly with devfreq,
> > > > sometimes, we still need to get PM-QoS inputs in order to meet the
> > > > required performance.
> > > >
> > > > In our testbed of Exynos4412, which has on-chip various DMA devices, the
> > > > memory interface and system bus are controlled according to their
> > > > utilization by devfreq. However, in some multimedia applications
> > > > including video-playing with MFC (multi-function codec) H/W and
> > > > photo/video-capturing with FIMC H/W, we have observed issues due to
> > > > insufficient DMA throughput or latency.
> > > >
> > > > In such applications, there are deadlines: less than 16.6ms with 60Hz.
> > > > With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
> > > > within a frame and we get missing frames and distorted pictures.
> > > > With longer polling intervals (20 ~ 100ms), the response time is not
> > > > sufficient and we get distorted or broken images. In other words,
> > > > regardless of polling interval, we get poor results with hard-deadline
> > > > H/Ws. They, in fact, have a preset requirement on DMA throughput.
> > > >
> > > > Thus, we need PM-QoS capabilities in devfreq. (Note that for general
> > > > user applications, devfreq for bus/memory works fine. They are not so
> > > > sensitive to tens of ms in performance increasing responses in general.
> > > >
> > > > In order to express how to handle QoS requests in devfreq devices,
> > > > the devfreq device drivers only need to express the mappings of
> > > > QoS value and frequency pairs with QoS class along with
> > > > devfreq_add_device() call.
> > > >
> > > > As a side effect of the implementation, which happens to be positive,
> > > > min/max freq is now enforced regardless of governor implementation.
> > >
> > > Can you please explain in a few words how this is supposed to work in
> > > practice?
> >
> > Ah.. this "side effect" has been neutralized by the patch
> >
> > ab5f299f51259fd2466cf35c89d79bd960e0fc32
> > PM / devfreq: add relation of recommended frequency.
> >
> > I should've removed that comment.
>
> OK
>
> > > > Tested on Exynos4412 machines with memory/bus frequencies and multimedia
> > > > H/W blocks. (camera, video decoding, and video encoding)
> > > >
> > > > Signed-off-by: MyungJoo Ham
> > > > Signed-off-by: Kyungmin Park
> > >
> > > I'm not entirely convinced yet, but a few comments follow.
> > >
> > > > ---
> > > > Changed from V2-resend
> > > > - Removed dependencies on global pm-qos class definitions
> > > > - Revised data structure handling pm-qos (being ready for dev-pm-qos)
> > > >
> > > > Changes from V2
> > > > - Rebased
> > > >
> > > > Changes from V1
> > > > - Error handling at devfreq_add_device()
> > > > - Handling pm_qos_max information
> > > > - Styly update
> > > > ---
> > []
> > > > @@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
> > > > * List from the highest proiority
> > > > * max_freq (probably called by thermal when it's too hot)
> > > > * min_freq
> > > > + * qos_min_freq
> > > > */
> > > >
> > > > + if (devfreq->qos_min_freq && freq < devfreq->qos_min_freq) {
> > > > + freq = devfreq->qos_min_freq;
> > > > + flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> > >
> > > What exactly is the purpose of this last line?
> >
> > It says target callback to use "qos_min_freq" as its greatest lower bound;
> > use any values that are "qos_min_freq" or above.
> > And it can be overriden by min_freq and max_freq.
>
> I see.
>
> > > > + }
> > > > if (devfreq->min_freq && freq < devfreq->min_freq) {
> > > > freq = devfreq->min_freq;
> > > > flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> > > > @@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
> > > > * devfreq_notifier_call() - Notify that the device frequency
> > > > requirements
> > > > *has been changed out of devfreq framework.
> > > > * @nb the notifier_block (supposed to be devfreq->nb)
> > > > - * @type not used
> > > > + * @val not used.
> > > > * @devp not used
> > > > *
> > > > * Called by a notifier that uses devfreq->nb.
> > > > */
> > > > -static int devfreq_notifier_call(struct notifier_block *nb, unsigned
> > > > long type,
> > > > +static int devfreq_notifier_call(struct notifier_block *nb, unsigned
> > > > long val,
> > > > void *devp)
> > > > {
> > > > struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
> > >
> > > The above change is only a cleanup unrelated to the rest of modifications.
> > > Please push it separately (if you _really_ think it's necessary).
> >
> > oops.. yes..
> >
> >
Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support
On Mon, Sep 17, 2012 at 11:10:09PM +0200, Rafael J. Wysocki wrote:
On Monday, September 17, 2012, MyungJoo Ham wrote:
Sender : Rafael J. Wysockir...@sisk.pl
Date : 2012-09-09 07:20 (GMT+09:00)
Title : Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support
On Thursday, August 30, 2012, MyungJoo Ham wrote:
Even if the performance of a device is controlled properly with devfreq,
sometimes, we still need to get PM-QoS inputs in order to meet the
required performance.
In our testbed of Exynos4412, which has on-chip various DMA devices, the
memory interface and system bus are controlled according to their
utilization by devfreq. However, in some multimedia applications
including video-playing with MFC (multi-function codec) H/W and
photo/video-capturing with FIMC H/W, we have observed issues due to
insufficient DMA throughput or latency.
In such applications, there are deadlines: less than 16.6ms with 60Hz.
With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
within a frame and we get missing frames and distorted pictures.
With longer polling intervals (20 ~ 100ms), the response time is not
sufficient and we get distorted or broken images. In other words,
regardless of polling interval, we get poor results with hard-deadline
H/Ws. They, in fact, have a preset requirement on DMA throughput.
Thus, we need PM-QoS capabilities in devfreq. (Note that for general
user applications, devfreq for bus/memory works fine. They are not so
sensitive to tens of ms in performance increasing responses in general.
In order to express how to handle QoS requests in devfreq devices,
the devfreq device drivers only need to express the mappings of
QoS value and frequency pairs with QoS class along with
devfreq_add_device() call.
As a side effect of the implementation, which happens to be positive,
min/max freq is now enforced regardless of governor implementation.
Can you please explain in a few words how this is supposed to work in
practice?
Ah.. this side effect has been neutralized by the patch
ab5f299f51259fd2466cf35c89d79bd960e0fc32
PM / devfreq: add relation of recommended frequency.
I should've removed that comment.
OK
Tested on Exynos4412 machines with memory/bus frequencies and multimedia
H/W blocks. (camera, video decoding, and video encoding)
Signed-off-by: MyungJoo Ham
Signed-off-by: Kyungmin Park
I'm not entirely convinced yet, but a few comments follow.
---
Changed from V2-resend
- Removed dependencies on global pm-qos class definitions
- Revised data structure handling pm-qos (being ready for dev-pm-qos)
Changes from V2
- Rebased
Changes from V1
- Error handling at devfreq_add_device()
- Handling pm_qos_max information
- Styly update
---
[]
@@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
* List from the highest proiority
* max_freq (probably called by thermal when it's too hot)
* min_freq
+ * qos_min_freq
*/
+ if (devfreq-qos_min_freq freq devfreq-qos_min_freq) {
+ freq = devfreq-qos_min_freq;
+ flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
What exactly is the purpose of this last line?
It says target callback to use qos_min_freq as its greatest lower bound;
use any values that are qos_min_freq or above.
And it can be overriden by min_freq and max_freq.
I see.
+ }
if (devfreq-min_freq freq devfreq-min_freq) {
freq = devfreq-min_freq;
flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
@@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
* devfreq_notifier_call() - Notify that the device frequency
requirements
*has been changed out of devfreq framework.
* @nb the notifier_block (supposed to be devfreq-nb)
- * @type not used
+ * @val not used.
* @devp not used
*
* Called by a notifier that uses devfreq-nb.
*/
-static int devfreq_notifier_call(struct notifier_block *nb, unsigned
long type,
+static int devfreq_notifier_call(struct notifier_block *nb, unsigned
long val,
void *devp)
{
struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
The above change is only a cleanup unrelated to the rest of modifications.
Please push it separately (if you _really_ think it's necessary).
oops.. yes..
@@ -183,6 +189,49 @@ static int devfreq_notifier_call(struct
notifier_block *nb, unsigned long type,
}
/**
+ * devfreq_qos_notifier_call() -
This looks like a missing kerneldoc comment?
yes.. I'd either remove the comment or fill it in.
Please add the comment.
+ */
+static int devfreq_qos_notifier_call(struct notifier_block *nb,
+ unsigned
Re: [PATCH] copyright owner and author clean up for intel iommu and related files
On Mon, Feb 25, 2008 at 10:28:50AM -0800, Andrew Morton wrote: > On Mon, 25 Feb 2008 09:54:02 -0800 Greg KH <[EMAIL PROTECTED]> wrote: > > > On Mon, Feb 25, 2008 at 07:50:14AM -0800, mark gross wrote: > > > On Sat, Feb 23, 2008 at 11:20:22AM -0800, Andrew Morton wrote: > > > > On Sat, 23 Feb 2008 11:06:49 -0800 Greg KH <[EMAIL PROTECTED]> wrote: > > > > > > > > > On Sat, Feb 23, 2008 at 12:04:43AM -0800, Andrew Morton wrote: > > > > > > On Thu, 21 Feb 2008 10:15:55 -0800 mark gross <[EMAIL PROTECTED]> > > > > > > wrote: > > > > > > > > > > > > > Index: linux-2.6.24-mm1/drivers/pci/dmar.c > > > > > > > > > > > > I guess this is strictly a greg-pci-tree thing. Greg, how do you > > > > > > want > > > > > > to play intel-iommu? > > > > > > > > > > Yes, I'll take those. I'm pretty sure I even have the hardware here > > > > > to > > > > > test that the code doesn't crash :) > > > > > > > > > > > > > ok.. > > > > > > > > I have another one queued for 2.6.26-via-greg: > > > > pci-iova-rb-tree-setup-tweak.patch > > > > > > > > > > Should I start sending all the intel-iommu related stuff to Greg then? > > > > Sure, and the linux-pci list so that people can review them. > > > > And linux-kernel so that a lot more people can review them. > > And me too, please so I'll review them, and because subsystem maintainers > are leaky buckets (sorry). > oky-doky --mgross -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH]iommu-iotlb-flushing
On Sat, Feb 23, 2008 at 12:05:17AM -0800, Andrew Morton wrote:
> On Wed, 20 Feb 2008 16:06:23 -0800 mark gross <[EMAIL PROTECTED]> wrote:
>
> > The following patch is for batching up the flushing of the IOTLB for
> > the DMAR implementation found in the Intel VT-d hardware. It works by
> > building a list of to be flushed IOTLB entries and a bitmap list of
> > which DMAR engine they are from.
> >
> > After either a high water mark (250 accessible via debugfs) or 10ms the
> > list of iova's will be reclaimed and the DMAR engines associated are
> > IOTLB-flushed.
> >
> > This approach recovers 15 to 20% of the performance lost when using the
> > IOMMU for my netperf udp stream benchmark with small packets. It can be
> > disabled with a kernel boot parameter
> > "intel_iommu=strict".
> >
> > Its use does weaken the IOMMU protections a bit.
> >
> > I would like to see this go into MM for a while and then onto mainline.
> >
> > ...
> >
> > +static struct timer_list unmap_timer =
> > + TIMER_INITIALIZER(flush_unmaps_timeout, 0, 0);
>
> Could use DEFINE_TIMER here.
ok
>
> > +struct unmap_list {
> > + struct list_head list;
> > + struct dmar_domain *domain;
> > + struct iova *iova;
> > +};
>
> unmap_list doens't seem to be used anywhere?
oops, left over from earlier version.
>
> > +static struct intel_iommu *g_iommus;
> > +/* bitmap for indexing intel_iommus */
> > +static unsigned long *g_iommus_to_flush;
> > +static int g_num_of_iommus;
> > +
> > +static DEFINE_SPINLOCK(async_umap_flush_lock);
> > +static LIST_HEAD(unmaps_to_do);
> > +
> > +static int timer_on;
> > +static long list_size;
> > +static int high_watermark;
> > +
> > +static struct dentry *intel_iommu_debug, *debug;
> > +
> > +
> > static void domain_remove_dev_info(struct dmar_domain *domain);
> >
> > static int dmar_disabled;
> > static int __initdata dmar_map_gfx = 1;
> > static int dmar_forcedac;
> > +static int intel_iommu_strict;
> >
> > #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
> > static DEFINE_SPINLOCK(device_domain_lock);
> > @@ -73,9 +103,13 @@
> > printk(KERN_INFO
> > "Intel-IOMMU: disable GFX device mapping\n");
> > } else if (!strncmp(str, "forcedac", 8)) {
> > - printk (KERN_INFO
> > + printk(KERN_INFO
> > "Intel-IOMMU: Forcing DAC for PCI devices\n");
> > dmar_forcedac = 1;
> > + } else if (!strncmp(str, "strict", 8)) {
>
> s/8/6/
ack.
>
> > + printk(KERN_INFO
> > + "Intel-IOMMU: disable batched IOTLB flush\n");
> > + intel_iommu_strict = 1;
> > }
> >
> > str += strcspn(str, ",");
> > @@ -965,17 +999,13 @@
> > set_bit(0, iommu->domain_ids);
> > return 0;
> > }
> > -
> > -static struct intel_iommu *alloc_iommu(struct dmar_drhd_unit *drhd)
> > +static struct intel_iommu *alloc_iommu(struct intel_iommu *iommu,
> > + struct dmar_drhd_unit *drhd)
> > {
> > - struct intel_iommu *iommu;
> > int ret;
> > int map_size;
> > u32 ver;
> >
> > - iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
> > - if (!iommu)
> > - return NULL;
> > iommu->reg = ioremap(drhd->reg_base_addr, PAGE_SIZE_4K);
> > if (!iommu->reg) {
> > printk(KERN_ERR "IOMMU: can't map the region\n");
> > @@ -1396,7 +1426,7 @@
> > int index;
> >
> > while (dev) {
> > - for (index = 0; index < cnt; index ++)
> > + for (index = 0; index < cnt; index++)
> > if (dev == devices[index])
> > return 1;
> >
> > @@ -1661,7 +1691,7 @@
> > struct dmar_rmrr_unit *rmrr;
> > struct pci_dev *pdev;
> > struct intel_iommu *iommu;
> > - int ret, unit = 0;
> > + int nlongs, i, ret, unit = 0;
> >
> > /*
> > * for each drhd
> > @@ -1672,7 +1702,30 @@
> > for_each_drhd_unit(drhd) {
> > if (drhd->ignored)
> > continue;
> > - iommu = alloc_iomm
Re: [PATCH]iova-lockdep-false-alarm-fix.
On Sat, Feb 23, 2008 at 12:05:12AM -0800, Andrew Morton wrote:
>
> > Subject: [PATCH]iova-lockdep-false-alarm-fix.
>
> Nice English titles, please...
>
> On Wed, 20 Feb 2008 16:35:28 -0800 mark gross <[EMAIL PROTECTED]> wrote:
>
> > lockdep goes off on the iova copy_reserved_iova because it and a
> > function it calls grabs locks in the from, and the to of the copy
> > operation.
> >
> > This patch gives the reserved_ioval_list locks special lockdep classes.
> >
> >
>
> Confused. Why not fix the ranking inconsistency instead?
Its not a ranking inconsistency issues the function grab locks of the
same lock classes triggering the warning. The first lock grabbed is for
the constant reserved areas that is never accessed after early boot.
Technically you could do without grabbing the locks for the "from"
structure its copying reserved areas from.
But dropping the from locks to me looks wrong, even though it would be ok.
The affected code only runs in early boot as its setting up the DMAR
engines.
--mgross
>
> Your changelog doesn't tell us why this isn't a real bug?
>
> > Index: linux-2.6.24-mm1/drivers/pci/intel-iommu.c
> > ===
> > --- linux-2.6.24-mm1.orig/drivers/pci/intel-iommu.c 2008-02-20
> > 15:52:23.0 -0800
> > +++ linux-2.6.24-mm1/drivers/pci/intel-iommu.c 2008-02-20
> > 16:08:27.0 -0800
> > @@ -1127,6 +1127,8 @@
> > }
> >
> > static struct iova_domain reserved_iova_list;
> > +static struct lock_class_key reserved_alloc_key;
> > +static struct lock_class_key reserved_rbtree_key;
> >
> > static void dmar_init_reserved_ranges(void)
> > {
> > @@ -1137,6 +1139,11 @@
> >
> > init_iova_domain(_iova_list, DMA_32BIT_PFN);
> >
> > + lockdep_set_class(_iova_list.iova_alloc_lock,
> > + _alloc_key);
> > + lockdep_set_class(_iova_list.iova_rbtree_lock,
> > + _rbtree_key);
> > +
> > /* IOAPIC ranges shouldn't be accessed by DMA */
> > iova = reserve_iova(_iova_list, IOVA_PFN(IOAPIC_RANGE_START),
> > IOVA_PFN(IOAPIC_RANGE_END));
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] copyright owner and author clean up for intel iommu and related files
On Sat, Feb 23, 2008 at 11:20:22AM -0800, Andrew Morton wrote: > On Sat, 23 Feb 2008 11:06:49 -0800 Greg KH <[EMAIL PROTECTED]> wrote: > > > On Sat, Feb 23, 2008 at 12:04:43AM -0800, Andrew Morton wrote: > > > On Thu, 21 Feb 2008 10:15:55 -0800 mark gross <[EMAIL PROTECTED]> wrote: > > > > > > > Index: linux-2.6.24-mm1/drivers/pci/dmar.c > > > > > > I guess this is strictly a greg-pci-tree thing. Greg, how do you want > > > to play intel-iommu? > > > > Yes, I'll take those. I'm pretty sure I even have the hardware here to > > test that the code doesn't crash :) > > > > ok.. > > I have another one queued for 2.6.26-via-greg: > pci-iova-rb-tree-setup-tweak.patch > Should I start sending all the intel-iommu related stuff to Greg then? --mgross > And one queued for 2.6.25: iova-lockdep-false-alarm-fix.patch which > according to my current processes will be sent to you after I've tested it > a bit with "for 2.6.25?" in its Subject: > > My list of for-2.6.25-after-a-bit-of-testing patches is large: > > > rtc-cmos-display-hpet-emulation-mode.patch > register_memory-unregister_memory-fix-use-after-free-and-refcounting.patch > acer-wmi-fail-gracefully-if-acpi-is-disabled.patch > tc1100-wmi-fail-gracefully-if-acpi-is-disabled.patch > dmi-dont-save-the-same-device-twice-was-smbios-dmi-add-type-41-=-onboard-devices-extended-information.patch > uml-remove-unused-sigcontext-accessors.patch > uml-fix-helper_wait-calls-in-watchdog.patch > uml-fix-fp-register-corruption.patch > x86-cast-cmpxchg-and-cmpxchg_local-result-for-386-and-486.patch > nbd-make-nbd-default-to-deadline-i-o-scheduler.patch > efs-move-headers-out-of-include-linux.patch > percpu-fix-debug_preempt-per_cpu-checking.patch > proc-add-rlimit_rttime-to-proc-pid-limits.patch > drivers-video-uvesafbc-fix-section-mismatch-warning-in-param_set_scroll.patch > dmi-prevent-linked-list-corruption-resent.patch > x86-fix-clearcopy_user_page-declarations-in-pageh.patch > futex-fix-init-order.patch > futex-runtime-enable-pi-and-robust-functionality.patch > h8300-signalc-typo-fix.patch > h8300-uaccessh-update.patch > h8300-irq-handling-update.patch > h8300-defconfig-update.patch > kernel-doc-fix-function-pointer-parameter-parsing.patch > mpt-fusion-dont-oops-if-numphys==0.patch > bluetooth-hci_core-defer-hci_unregister_sysfs.patch > e100-do-suspend-shutdown-like-e1000.patch > dm-raid1-bitops-bug.patch > alloc_percpu-fails-to-allocate-percpu-data.patch > scsi-qla4xxx-ql4_isrc-unchangelogged-patch.patch > iova-lockdep-false-alarm-fix.patch > cgroup-memory-controller-document-huge-memory-cache-overhead-in-kconfig.patch > kprobes-refuse-kprobe-insertion-on-add-sub_preempt_counter.patch > sh-revert-dreamcast-pci-change.patch > bluetooth-conwise-technology-based-adapters-with-buggy-sco-support-bugzilla-9027.patch > pm-remove-unbalanced-mutex_unlock-from-dpm_resume.patch > message-fusion-mptbasec-fix-use-after-frees.patch > ieee80211-fix-broken-error-handling-in-ieee80211_sta_process_addba_request.patch > smack-update-for-file-capabilities.patch > fix-typo-in-tick-broadcastc.patch > dm-raid1c-fix-null-dereferences.patch > usb-queue-usb-usb_cdc_get_encapsulated_response-message.patch > i8042-use-sgi_has_i8042-to-select-sgi-i8042-handlinig.patch > solve-section-mismatch-for-free_area_init_core.patch > # > cgroup-fix-and-update-documentation.patch > cgroup-fix-comments.patch > cgroup-clean-up-cgrouph.patch > cgroup-fix-memory-leak-in-cgroup_get_sb.patch > cgroup-fix-subsys-bitops.patch > cgroup-remove-duplicate-code-in-find_css_set.patch > cgroup-remove-dead-code-in-cgroup_get_rootdir.patch > # > memcgroup-fix-and-update-documentation.patch > memcgroup-remove-a-useless-vm_bug_on.patch > memcgroup-return-negative-error-code-in-mem_cgroup_create.patch > # > #kernel-irq-chipc-irq-disable-shutdown-bug.patch: needs acks > kernel-irq-chipc-irq-disable-shutdown-bug.patch > # > #revert-sh-use-ctrl_in-out-for-on-chip-pci-access.patch: being worked on > #revert-sh-use-ctrl_in-out-for-on-chip-pci-access.patch > # > kthread-synchronization-issues.patch > #kthread-add-a-missing-memory-barrier-to-kthread_stop.patch: nop, do it in > wake_up > kthread-add-a-missing-memory-barrier-to-kthread_stop.patch > kthread-call-wake_up_process-without-the-lock-being-held.patch > fix-b43-driver-build-for-arm.patch > vt-notifier-fix-for-vt-switch.patch > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] copyright owner and author clean up for intel iommu and related files
On Sat, Feb 23, 2008 at 11:20:22AM -0800, Andrew Morton wrote: On Sat, 23 Feb 2008 11:06:49 -0800 Greg KH [EMAIL PROTECTED] wrote: On Sat, Feb 23, 2008 at 12:04:43AM -0800, Andrew Morton wrote: On Thu, 21 Feb 2008 10:15:55 -0800 mark gross [EMAIL PROTECTED] wrote: Index: linux-2.6.24-mm1/drivers/pci/dmar.c I guess this is strictly a greg-pci-tree thing. Greg, how do you want to play intel-iommu? Yes, I'll take those. I'm pretty sure I even have the hardware here to test that the code doesn't crash :) ok.. I have another one queued for 2.6.26-via-greg: pci-iova-rb-tree-setup-tweak.patch Should I start sending all the intel-iommu related stuff to Greg then? --mgross And one queued for 2.6.25: iova-lockdep-false-alarm-fix.patch which according to my current processes will be sent to you after I've tested it a bit with for 2.6.25? in its Subject: My list of for-2.6.25-after-a-bit-of-testing patches is large: rtc-cmos-display-hpet-emulation-mode.patch register_memory-unregister_memory-fix-use-after-free-and-refcounting.patch acer-wmi-fail-gracefully-if-acpi-is-disabled.patch tc1100-wmi-fail-gracefully-if-acpi-is-disabled.patch dmi-dont-save-the-same-device-twice-was-smbios-dmi-add-type-41-=-onboard-devices-extended-information.patch uml-remove-unused-sigcontext-accessors.patch uml-fix-helper_wait-calls-in-watchdog.patch uml-fix-fp-register-corruption.patch x86-cast-cmpxchg-and-cmpxchg_local-result-for-386-and-486.patch nbd-make-nbd-default-to-deadline-i-o-scheduler.patch efs-move-headers-out-of-include-linux.patch percpu-fix-debug_preempt-per_cpu-checking.patch proc-add-rlimit_rttime-to-proc-pid-limits.patch drivers-video-uvesafbc-fix-section-mismatch-warning-in-param_set_scroll.patch dmi-prevent-linked-list-corruption-resent.patch x86-fix-clearcopy_user_page-declarations-in-pageh.patch futex-fix-init-order.patch futex-runtime-enable-pi-and-robust-functionality.patch h8300-signalc-typo-fix.patch h8300-uaccessh-update.patch h8300-irq-handling-update.patch h8300-defconfig-update.patch kernel-doc-fix-function-pointer-parameter-parsing.patch mpt-fusion-dont-oops-if-numphys==0.patch bluetooth-hci_core-defer-hci_unregister_sysfs.patch e100-do-suspend-shutdown-like-e1000.patch dm-raid1-bitops-bug.patch alloc_percpu-fails-to-allocate-percpu-data.patch scsi-qla4xxx-ql4_isrc-unchangelogged-patch.patch iova-lockdep-false-alarm-fix.patch cgroup-memory-controller-document-huge-memory-cache-overhead-in-kconfig.patch kprobes-refuse-kprobe-insertion-on-add-sub_preempt_counter.patch sh-revert-dreamcast-pci-change.patch bluetooth-conwise-technology-based-adapters-with-buggy-sco-support-bugzilla-9027.patch pm-remove-unbalanced-mutex_unlock-from-dpm_resume.patch message-fusion-mptbasec-fix-use-after-frees.patch ieee80211-fix-broken-error-handling-in-ieee80211_sta_process_addba_request.patch smack-update-for-file-capabilities.patch fix-typo-in-tick-broadcastc.patch dm-raid1c-fix-null-dereferences.patch usb-queue-usb-usb_cdc_get_encapsulated_response-message.patch i8042-use-sgi_has_i8042-to-select-sgi-i8042-handlinig.patch solve-section-mismatch-for-free_area_init_core.patch # cgroup-fix-and-update-documentation.patch cgroup-fix-comments.patch cgroup-clean-up-cgrouph.patch cgroup-fix-memory-leak-in-cgroup_get_sb.patch cgroup-fix-subsys-bitops.patch cgroup-remove-duplicate-code-in-find_css_set.patch cgroup-remove-dead-code-in-cgroup_get_rootdir.patch # memcgroup-fix-and-update-documentation.patch memcgroup-remove-a-useless-vm_bug_on.patch memcgroup-return-negative-error-code-in-mem_cgroup_create.patch # #kernel-irq-chipc-irq-disable-shutdown-bug.patch: needs acks kernel-irq-chipc-irq-disable-shutdown-bug.patch # #revert-sh-use-ctrl_in-out-for-on-chip-pci-access.patch: being worked on #revert-sh-use-ctrl_in-out-for-on-chip-pci-access.patch # kthread-synchronization-issues.patch #kthread-add-a-missing-memory-barrier-to-kthread_stop.patch: nop, do it in wake_up kthread-add-a-missing-memory-barrier-to-kthread_stop.patch kthread-call-wake_up_process-without-the-lock-being-held.patch fix-b43-driver-build-for-arm.patch vt-notifier-fix-for-vt-switch.patch -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH]iova-lockdep-false-alarm-fix.
On Sat, Feb 23, 2008 at 12:05:12AM -0800, Andrew Morton wrote:
Subject: [PATCH]iova-lockdep-false-alarm-fix.
Nice English titles, please...
On Wed, 20 Feb 2008 16:35:28 -0800 mark gross [EMAIL PROTECTED] wrote:
lockdep goes off on the iova copy_reserved_iova because it and a
function it calls grabs locks in the from, and the to of the copy
operation.
This patch gives the reserved_ioval_list locks special lockdep classes.
Confused. Why not fix the ranking inconsistency instead?
Its not a ranking inconsistency issues the function grab locks of the
same lock classes triggering the warning. The first lock grabbed is for
the constant reserved areas that is never accessed after early boot.
Technically you could do without grabbing the locks for the from
structure its copying reserved areas from.
But dropping the from locks to me looks wrong, even though it would be ok.
The affected code only runs in early boot as its setting up the DMAR
engines.
--mgross
Your changelog doesn't tell us why this isn't a real bug?
Index: linux-2.6.24-mm1/drivers/pci/intel-iommu.c
===
--- linux-2.6.24-mm1.orig/drivers/pci/intel-iommu.c 2008-02-20
15:52:23.0 -0800
+++ linux-2.6.24-mm1/drivers/pci/intel-iommu.c 2008-02-20
16:08:27.0 -0800
@@ -1127,6 +1127,8 @@
}
static struct iova_domain reserved_iova_list;
+static struct lock_class_key reserved_alloc_key;
+static struct lock_class_key reserved_rbtree_key;
static void dmar_init_reserved_ranges(void)
{
@@ -1137,6 +1139,11 @@
init_iova_domain(reserved_iova_list, DMA_32BIT_PFN);
+ lockdep_set_class(reserved_iova_list.iova_alloc_lock,
+ reserved_alloc_key);
+ lockdep_set_class(reserved_iova_list.iova_rbtree_lock,
+ reserved_rbtree_key);
+
/* IOAPIC ranges shouldn't be accessed by DMA */
iova = reserve_iova(reserved_iova_list, IOVA_PFN(IOAPIC_RANGE_START),
IOVA_PFN(IOAPIC_RANGE_END));
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH]iommu-iotlb-flushing
On Sat, Feb 23, 2008 at 12:05:17AM -0800, Andrew Morton wrote:
On Wed, 20 Feb 2008 16:06:23 -0800 mark gross [EMAIL PROTECTED] wrote:
The following patch is for batching up the flushing of the IOTLB for
the DMAR implementation found in the Intel VT-d hardware. It works by
building a list of to be flushed IOTLB entries and a bitmap list of
which DMAR engine they are from.
After either a high water mark (250 accessible via debugfs) or 10ms the
list of iova's will be reclaimed and the DMAR engines associated are
IOTLB-flushed.
This approach recovers 15 to 20% of the performance lost when using the
IOMMU for my netperf udp stream benchmark with small packets. It can be
disabled with a kernel boot parameter
intel_iommu=strict.
Its use does weaken the IOMMU protections a bit.
I would like to see this go into MM for a while and then onto mainline.
...
+static struct timer_list unmap_timer =
+ TIMER_INITIALIZER(flush_unmaps_timeout, 0, 0);
Could use DEFINE_TIMER here.
ok
+struct unmap_list {
+ struct list_head list;
+ struct dmar_domain *domain;
+ struct iova *iova;
+};
unmap_list doens't seem to be used anywhere?
oops, left over from earlier version.
+static struct intel_iommu *g_iommus;
+/* bitmap for indexing intel_iommus */
+static unsigned long *g_iommus_to_flush;
+static int g_num_of_iommus;
+
+static DEFINE_SPINLOCK(async_umap_flush_lock);
+static LIST_HEAD(unmaps_to_do);
+
+static int timer_on;
+static long list_size;
+static int high_watermark;
+
+static struct dentry *intel_iommu_debug, *debug;
+
+
static void domain_remove_dev_info(struct dmar_domain *domain);
static int dmar_disabled;
static int __initdata dmar_map_gfx = 1;
static int dmar_forcedac;
+static int intel_iommu_strict;
#define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
static DEFINE_SPINLOCK(device_domain_lock);
@@ -73,9 +103,13 @@
printk(KERN_INFO
Intel-IOMMU: disable GFX device mapping\n);
} else if (!strncmp(str, forcedac, 8)) {
- printk (KERN_INFO
+ printk(KERN_INFO
Intel-IOMMU: Forcing DAC for PCI devices\n);
dmar_forcedac = 1;
+ } else if (!strncmp(str, strict, 8)) {
s/8/6/
ack.
+ printk(KERN_INFO
+ Intel-IOMMU: disable batched IOTLB flush\n);
+ intel_iommu_strict = 1;
}
str += strcspn(str, ,);
@@ -965,17 +999,13 @@
set_bit(0, iommu-domain_ids);
return 0;
}
-
-static struct intel_iommu *alloc_iommu(struct dmar_drhd_unit *drhd)
+static struct intel_iommu *alloc_iommu(struct intel_iommu *iommu,
+ struct dmar_drhd_unit *drhd)
{
- struct intel_iommu *iommu;
int ret;
int map_size;
u32 ver;
- iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
- if (!iommu)
- return NULL;
iommu-reg = ioremap(drhd-reg_base_addr, PAGE_SIZE_4K);
if (!iommu-reg) {
printk(KERN_ERR IOMMU: can't map the region\n);
@@ -1396,7 +1426,7 @@
int index;
while (dev) {
- for (index = 0; index cnt; index ++)
+ for (index = 0; index cnt; index++)
if (dev == devices[index])
return 1;
@@ -1661,7 +1691,7 @@
struct dmar_rmrr_unit *rmrr;
struct pci_dev *pdev;
struct intel_iommu *iommu;
- int ret, unit = 0;
+ int nlongs, i, ret, unit = 0;
/*
* for each drhd
@@ -1672,7 +1702,30 @@
for_each_drhd_unit(drhd) {
if (drhd-ignored)
continue;
- iommu = alloc_iommu(drhd);
+ g_num_of_iommus++;
No locking needed for g_num_of_iommus?
I'll double check if its needed, but it wouldn't hurt. This code is on
the kernel startup / init path.
+ }
+
+ nlongs = BITS_TO_LONGS(g_num_of_iommus);
Would this code be neater if it used the linux/bitmap.h stuff?
I'll look into it.
+ g_iommus_to_flush = kzalloc(nlongs * sizeof(unsigned long), GFP_KERNEL);
+ if (!g_iommus_to_flush) {
+ printk(KERN_ERR Intel-IOMMU:
+ Allocating bitmap array failed\n);
+ return -ENOMEM;
Are you sure we aren't leaking anything here? Like the alloc_iommu() above?
Once you set up the IOMMU's you never take them down or re-set them up.
This code runs one and one time at boot up.
+ }
+
+ g_iommus = kzalloc(g_num_of_iommus * sizeof(*iommu), GFP_KERNEL);
+ if (!g_iommus) {
+ kfree(g_iommus_to_flush);
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ i = 0;
+ for_each_drhd_unit(drhd) {
+ if (drhd-ignored
Re: [PATCH] copyright owner and author clean up for intel iommu and related files
On Mon, Feb 25, 2008 at 10:28:50AM -0800, Andrew Morton wrote: On Mon, 25 Feb 2008 09:54:02 -0800 Greg KH [EMAIL PROTECTED] wrote: On Mon, Feb 25, 2008 at 07:50:14AM -0800, mark gross wrote: On Sat, Feb 23, 2008 at 11:20:22AM -0800, Andrew Morton wrote: On Sat, 23 Feb 2008 11:06:49 -0800 Greg KH [EMAIL PROTECTED] wrote: On Sat, Feb 23, 2008 at 12:04:43AM -0800, Andrew Morton wrote: On Thu, 21 Feb 2008 10:15:55 -0800 mark gross [EMAIL PROTECTED] wrote: Index: linux-2.6.24-mm1/drivers/pci/dmar.c I guess this is strictly a greg-pci-tree thing. Greg, how do you want to play intel-iommu? Yes, I'll take those. I'm pretty sure I even have the hardware here to test that the code doesn't crash :) ok.. I have another one queued for 2.6.26-via-greg: pci-iova-rb-tree-setup-tweak.patch Should I start sending all the intel-iommu related stuff to Greg then? Sure, and the linux-pci list so that people can review them. And linux-kernel so that a lot more people can review them. And me too, please so I'll review them, and because subsystem maintainers are leaky buckets (sorry). oky-doky --mgross -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] copyright owner and author clean up for intel iommu and related files
The following is a clean up and correction of the copyright holding entities for the files associated with the intel iommu code. Its a no brainer. --mgross Signed-off-by: <[EMAIL PROTECTED]> Index: linux-2.6.24-mm1/drivers/pci/dmar.c === --- linux-2.6.24-mm1.orig/drivers/pci/dmar.c2008-02-21 09:22:57.0 -0800 +++ linux-2.6.24-mm1/drivers/pci/dmar.c 2008-02-21 09:24:02.0 -0800 @@ -14,11 +14,12 @@ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple * Place - Suite 330, Boston, MA 02111-1307 USA. * - * Copyright (C) Ashok Raj <[EMAIL PROTECTED]> - * Copyright (C) Shaohua Li <[EMAIL PROTECTED]> - * Copyright (C) Anil S Keshavamurthy <[EMAIL PROTECTED]> + * Copyright (C) 2006-2008 Intel Corporation + * Author: Ashok Raj <[EMAIL PROTECTED]> + * Author: Shaohua Li <[EMAIL PROTECTED]> + * Author: Anil S Keshavamurthy <[EMAIL PROTECTED]> * - * This file implements early detection/parsing of DMA Remapping Devices + * This file implements early detection/parsing of DMA Remapping Devices * reported to OS through BIOS via DMA remapping reporting (DMAR) ACPI * tables. */ Index: linux-2.6.24-mm1/drivers/pci/intel-iommu.c === --- linux-2.6.24-mm1.orig/drivers/pci/intel-iommu.c 2008-02-21 08:53:43.0 -0800 +++ linux-2.6.24-mm1/drivers/pci/intel-iommu.c 2008-02-21 09:02:25.0 -0800 @@ -14,9 +14,10 @@ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple * Place - Suite 330, Boston, MA 02111-1307 USA. * - * Copyright (C) Ashok Raj <[EMAIL PROTECTED]> - * Copyright (C) Shaohua Li <[EMAIL PROTECTED]> - * Copyright (C) Anil S Keshavamurthy <[EMAIL PROTECTED]> + * Copyright (C) 2006-2008 Intel Corporation + * Author: Ashok Raj <[EMAIL PROTECTED]> + * Author: Shaohua Li <[EMAIL PROTECTED]> + * Author: Anil S Keshavamurthy <[EMAIL PROTECTED]> */ #include Index: linux-2.6.24-mm1/drivers/pci/intel-iommu.h === --- linux-2.6.24-mm1.orig/drivers/pci/intel-iommu.h 2008-02-21 08:53:45.0 -0800 +++ linux-2.6.24-mm1/drivers/pci/intel-iommu.h 2008-02-21 09:22:47.0 -0800 @@ -14,8 +14,9 @@ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple * Place - Suite 330, Boston, MA 02111-1307 USA. * - * Copyright (C) Ashok Raj <[EMAIL PROTECTED]> - * Copyright (C) Anil S Keshavamurthy <[EMAIL PROTECTED]> + * Copyright (C) 2006-2008 Intel Corporation + * Author: Ashok Raj <[EMAIL PROTECTED]> + * Author: Anil S Keshavamurthy <[EMAIL PROTECTED]> */ #ifndef _INTEL_IOMMU_H_ Index: linux-2.6.24-mm1/drivers/pci/iova.c === --- linux-2.6.24-mm1.orig/drivers/pci/iova.c2008-02-21 08:53:35.0 -0800 +++ linux-2.6.24-mm1/drivers/pci/iova.c 2008-02-21 09:02:51.0 -0800 @@ -3,7 +3,8 @@ * * This file is released under the GPLv2. * - * Copyright (C) 2006 Anil S Keshavamurthy <[EMAIL PROTECTED]> + * Copyright (C) 2006-2008 Intel Corporation + * Author: Anil S Keshavamurthy <[EMAIL PROTECTED]> */ #include "iova.h" Index: linux-2.6.24-mm1/drivers/pci/iova.h === --- linux-2.6.24-mm1.orig/drivers/pci/iova.h2008-02-21 08:53:39.0 -0800 +++ linux-2.6.24-mm1/drivers/pci/iova.h 2008-02-21 09:03:20.0 -0800 @@ -3,7 +3,8 @@ * * This file is released under the GPLv2. * - * Copyright (C) 2006 Anil S Keshavamurthy <[EMAIL PROTECTED]> + * Copyright (C) 2006-2008 Intel Corporation + * Author: Anil S Keshavamurthy <[EMAIL PROTECTED]> * */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] copyright owner and author clean up for intel iommu and related files
The following is a clean up and correction of the copyright holding entities for the files associated with the intel iommu code. Its a no brainer. --mgross Signed-off-by: [EMAIL PROTECTED] Index: linux-2.6.24-mm1/drivers/pci/dmar.c === --- linux-2.6.24-mm1.orig/drivers/pci/dmar.c2008-02-21 09:22:57.0 -0800 +++ linux-2.6.24-mm1/drivers/pci/dmar.c 2008-02-21 09:24:02.0 -0800 @@ -14,11 +14,12 @@ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple * Place - Suite 330, Boston, MA 02111-1307 USA. * - * Copyright (C) Ashok Raj [EMAIL PROTECTED] - * Copyright (C) Shaohua Li [EMAIL PROTECTED] - * Copyright (C) Anil S Keshavamurthy [EMAIL PROTECTED] + * Copyright (C) 2006-2008 Intel Corporation + * Author: Ashok Raj [EMAIL PROTECTED] + * Author: Shaohua Li [EMAIL PROTECTED] + * Author: Anil S Keshavamurthy [EMAIL PROTECTED] * - * This file implements early detection/parsing of DMA Remapping Devices + * This file implements early detection/parsing of DMA Remapping Devices * reported to OS through BIOS via DMA remapping reporting (DMAR) ACPI * tables. */ Index: linux-2.6.24-mm1/drivers/pci/intel-iommu.c === --- linux-2.6.24-mm1.orig/drivers/pci/intel-iommu.c 2008-02-21 08:53:43.0 -0800 +++ linux-2.6.24-mm1/drivers/pci/intel-iommu.c 2008-02-21 09:02:25.0 -0800 @@ -14,9 +14,10 @@ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple * Place - Suite 330, Boston, MA 02111-1307 USA. * - * Copyright (C) Ashok Raj [EMAIL PROTECTED] - * Copyright (C) Shaohua Li [EMAIL PROTECTED] - * Copyright (C) Anil S Keshavamurthy [EMAIL PROTECTED] + * Copyright (C) 2006-2008 Intel Corporation + * Author: Ashok Raj [EMAIL PROTECTED] + * Author: Shaohua Li [EMAIL PROTECTED] + * Author: Anil S Keshavamurthy [EMAIL PROTECTED] */ #include linux/init.h Index: linux-2.6.24-mm1/drivers/pci/intel-iommu.h === --- linux-2.6.24-mm1.orig/drivers/pci/intel-iommu.h 2008-02-21 08:53:45.0 -0800 +++ linux-2.6.24-mm1/drivers/pci/intel-iommu.h 2008-02-21 09:22:47.0 -0800 @@ -14,8 +14,9 @@ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple * Place - Suite 330, Boston, MA 02111-1307 USA. * - * Copyright (C) Ashok Raj [EMAIL PROTECTED] - * Copyright (C) Anil S Keshavamurthy [EMAIL PROTECTED] + * Copyright (C) 2006-2008 Intel Corporation + * Author: Ashok Raj [EMAIL PROTECTED] + * Author: Anil S Keshavamurthy [EMAIL PROTECTED] */ #ifndef _INTEL_IOMMU_H_ Index: linux-2.6.24-mm1/drivers/pci/iova.c === --- linux-2.6.24-mm1.orig/drivers/pci/iova.c2008-02-21 08:53:35.0 -0800 +++ linux-2.6.24-mm1/drivers/pci/iova.c 2008-02-21 09:02:51.0 -0800 @@ -3,7 +3,8 @@ * * This file is released under the GPLv2. * - * Copyright (C) 2006 Anil S Keshavamurthy [EMAIL PROTECTED] + * Copyright (C) 2006-2008 Intel Corporation + * Author: Anil S Keshavamurthy [EMAIL PROTECTED] */ #include iova.h Index: linux-2.6.24-mm1/drivers/pci/iova.h === --- linux-2.6.24-mm1.orig/drivers/pci/iova.h2008-02-21 08:53:39.0 -0800 +++ linux-2.6.24-mm1/drivers/pci/iova.h 2008-02-21 09:03:20.0 -0800 @@ -3,7 +3,8 @@ * * This file is released under the GPLv2. * - * Copyright (C) 2006 Anil S Keshavamurthy [EMAIL PROTECTED] + * Copyright (C) 2006-2008 Intel Corporation + * Author: Anil S Keshavamurthy [EMAIL PROTECTED] * */ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH]iova-lockdep-false-alarm-fix.
lockdep goes off on the iova copy_reserved_iova because it and a
function it calls grabs locks in the from, and the to of the copy
operation.
This patch gives the reserved_ioval_list locks special lockdep classes.
--mgross
Signed-off-by: <[EMAIL PROTECTED]>
Index: linux-2.6.24-mm1/drivers/pci/intel-iommu.c
===
--- linux-2.6.24-mm1.orig/drivers/pci/intel-iommu.c 2008-02-20
15:52:23.0 -0800
+++ linux-2.6.24-mm1/drivers/pci/intel-iommu.c 2008-02-20 16:08:27.0
-0800
@@ -1127,6 +1127,8 @@
}
static struct iova_domain reserved_iova_list;
+static struct lock_class_key reserved_alloc_key;
+static struct lock_class_key reserved_rbtree_key;
static void dmar_init_reserved_ranges(void)
{
@@ -1137,6 +1139,11 @@
init_iova_domain(_iova_list, DMA_32BIT_PFN);
+ lockdep_set_class(_iova_list.iova_alloc_lock,
+ _alloc_key);
+ lockdep_set_class(_iova_list.iova_rbtree_lock,
+ _rbtree_key);
+
/* IOAPIC ranges shouldn't be accessed by DMA */
iova = reserve_iova(_iova_list, IOVA_PFN(IOAPIC_RANGE_START),
IOVA_PFN(IOAPIC_RANGE_END));
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH]iommu-iotlb-flushing
The following patch is for batching up the flushing of the IOTLB for
the DMAR implementation found in the Intel VT-d hardware. It works by
building a list of to be flushed IOTLB entries and a bitmap list of
which DMAR engine they are from.
After either a high water mark (250 accessible via debugfs) or 10ms the
list of iova's will be reclaimed and the DMAR engines associated are
IOTLB-flushed.
This approach recovers 15 to 20% of the performance lost when using the
IOMMU for my netperf udp stream benchmark with small packets. It can be
disabled with a kernel boot parameter
"intel_iommu=strict".
Its use does weaken the IOMMU protections a bit.
I would like to see this go into MM for a while and then onto mainline.
Thanks,
Signed-off-by: <[EMAIL PROTECTED]>
Index: linux-2.6.24-mm1/drivers/pci/intel-iommu.c
===
--- linux-2.6.24-mm1.orig/drivers/pci/intel-iommu.c 2008-02-12
07:12:06.0 -0800
+++ linux-2.6.24-mm1/drivers/pci/intel-iommu.c 2008-02-12 11:35:42.0
-0800
@@ -21,6 +21,7 @@
#include
#include
+#include
#include
#include
#include
@@ -30,6 +31,7 @@
#include
#include
#include
+#include
#include "iova.h"
#include "intel-iommu.h"
#include /* force_iommu in this header in x86-64*/
@@ -50,11 +52,39 @@
#define DOMAIN_MAX_ADDR(gaw) u64)1) << gaw) - 1)
+
+static void flush_unmaps_timeout(unsigned long data);
+
+static struct timer_list unmap_timer =
+ TIMER_INITIALIZER(flush_unmaps_timeout, 0, 0);
+
+struct unmap_list {
+ struct list_head list;
+ struct dmar_domain *domain;
+ struct iova *iova;
+};
+
+static struct intel_iommu *g_iommus;
+/* bitmap for indexing intel_iommus */
+static unsigned long *g_iommus_to_flush;
+static int g_num_of_iommus;
+
+static DEFINE_SPINLOCK(async_umap_flush_lock);
+static LIST_HEAD(unmaps_to_do);
+
+static int timer_on;
+static long list_size;
+static int high_watermark;
+
+static struct dentry *intel_iommu_debug, *debug;
+
+
static void domain_remove_dev_info(struct dmar_domain *domain);
static int dmar_disabled;
static int __initdata dmar_map_gfx = 1;
static int dmar_forcedac;
+static int intel_iommu_strict;
#define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
static DEFINE_SPINLOCK(device_domain_lock);
@@ -73,9 +103,13 @@
printk(KERN_INFO
"Intel-IOMMU: disable GFX device mapping\n");
} else if (!strncmp(str, "forcedac", 8)) {
- printk (KERN_INFO
+ printk(KERN_INFO
"Intel-IOMMU: Forcing DAC for PCI devices\n");
dmar_forcedac = 1;
+ } else if (!strncmp(str, "strict", 8)) {
+ printk(KERN_INFO
+ "Intel-IOMMU: disable batched IOTLB flush\n");
+ intel_iommu_strict = 1;
}
str += strcspn(str, ",");
@@ -965,17 +999,13 @@
set_bit(0, iommu->domain_ids);
return 0;
}
-
-static struct intel_iommu *alloc_iommu(struct dmar_drhd_unit *drhd)
+static struct intel_iommu *alloc_iommu(struct intel_iommu *iommu,
+ struct dmar_drhd_unit *drhd)
{
- struct intel_iommu *iommu;
int ret;
int map_size;
u32 ver;
- iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
- if (!iommu)
- return NULL;
iommu->reg = ioremap(drhd->reg_base_addr, PAGE_SIZE_4K);
if (!iommu->reg) {
printk(KERN_ERR "IOMMU: can't map the region\n");
@@ -1396,7 +1426,7 @@
int index;
while (dev) {
- for (index = 0; index < cnt; index ++)
+ for (index = 0; index < cnt; index++)
if (dev == devices[index])
return 1;
@@ -1661,7 +1691,7 @@
struct dmar_rmrr_unit *rmrr;
struct pci_dev *pdev;
struct intel_iommu *iommu;
- int ret, unit = 0;
+ int nlongs, i, ret, unit = 0;
/*
* for each drhd
@@ -1672,7 +1702,30 @@
for_each_drhd_unit(drhd) {
if (drhd->ignored)
continue;
- iommu = alloc_iommu(drhd);
+ g_num_of_iommus++;
+ }
+
+ nlongs = BITS_TO_LONGS(g_num_of_iommus);
+ g_iommus_to_flush = kzalloc(nlongs * sizeof(unsigned long), GFP_KERNEL);
+ if (!g_iommus_to_flush) {
+ printk(KERN_ERR "Intel-IOMMU: "
+ "Allocating bitmap array failed\n");
+ return -ENOMEM;
+ }
+
+ g_iommus = kzalloc(g_num_of_iommus * sizeof(*iommu), GFP_KERNEL);
+ if (!g_iommus) {
+ kfree(g_iommus_to_flush);
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ i = 0;
+ for_each_drhd_unit(drhd) {
+
[PATCH]iommu-iotlb-flushing
The following patch is for batching up the flushing of the IOTLB for
the DMAR implementation found in the Intel VT-d hardware. It works by
building a list of to be flushed IOTLB entries and a bitmap list of
which DMAR engine they are from.
After either a high water mark (250 accessible via debugfs) or 10ms the
list of iova's will be reclaimed and the DMAR engines associated are
IOTLB-flushed.
This approach recovers 15 to 20% of the performance lost when using the
IOMMU for my netperf udp stream benchmark with small packets. It can be
disabled with a kernel boot parameter
intel_iommu=strict.
Its use does weaken the IOMMU protections a bit.
I would like to see this go into MM for a while and then onto mainline.
Thanks,
Signed-off-by: [EMAIL PROTECTED]
Index: linux-2.6.24-mm1/drivers/pci/intel-iommu.c
===
--- linux-2.6.24-mm1.orig/drivers/pci/intel-iommu.c 2008-02-12
07:12:06.0 -0800
+++ linux-2.6.24-mm1/drivers/pci/intel-iommu.c 2008-02-12 11:35:42.0
-0800
@@ -21,6 +21,7 @@
#include linux/init.h
#include linux/bitmap.h
+#include linux/debugfs.h
#include linux/slab.h
#include linux/irq.h
#include linux/interrupt.h
@@ -30,6 +31,7 @@
#include linux/dmar.h
#include linux/dma-mapping.h
#include linux/mempool.h
+#include linux/timer.h
#include iova.h
#include intel-iommu.h
#include asm/proto.h /* force_iommu in this header in x86-64*/
@@ -50,11 +52,39 @@
#define DOMAIN_MAX_ADDR(gaw) u64)1) gaw) - 1)
+
+static void flush_unmaps_timeout(unsigned long data);
+
+static struct timer_list unmap_timer =
+ TIMER_INITIALIZER(flush_unmaps_timeout, 0, 0);
+
+struct unmap_list {
+ struct list_head list;
+ struct dmar_domain *domain;
+ struct iova *iova;
+};
+
+static struct intel_iommu *g_iommus;
+/* bitmap for indexing intel_iommus */
+static unsigned long *g_iommus_to_flush;
+static int g_num_of_iommus;
+
+static DEFINE_SPINLOCK(async_umap_flush_lock);
+static LIST_HEAD(unmaps_to_do);
+
+static int timer_on;
+static long list_size;
+static int high_watermark;
+
+static struct dentry *intel_iommu_debug, *debug;
+
+
static void domain_remove_dev_info(struct dmar_domain *domain);
static int dmar_disabled;
static int __initdata dmar_map_gfx = 1;
static int dmar_forcedac;
+static int intel_iommu_strict;
#define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
static DEFINE_SPINLOCK(device_domain_lock);
@@ -73,9 +103,13 @@
printk(KERN_INFO
Intel-IOMMU: disable GFX device mapping\n);
} else if (!strncmp(str, forcedac, 8)) {
- printk (KERN_INFO
+ printk(KERN_INFO
Intel-IOMMU: Forcing DAC for PCI devices\n);
dmar_forcedac = 1;
+ } else if (!strncmp(str, strict, 8)) {
+ printk(KERN_INFO
+ Intel-IOMMU: disable batched IOTLB flush\n);
+ intel_iommu_strict = 1;
}
str += strcspn(str, ,);
@@ -965,17 +999,13 @@
set_bit(0, iommu-domain_ids);
return 0;
}
-
-static struct intel_iommu *alloc_iommu(struct dmar_drhd_unit *drhd)
+static struct intel_iommu *alloc_iommu(struct intel_iommu *iommu,
+ struct dmar_drhd_unit *drhd)
{
- struct intel_iommu *iommu;
int ret;
int map_size;
u32 ver;
- iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
- if (!iommu)
- return NULL;
iommu-reg = ioremap(drhd-reg_base_addr, PAGE_SIZE_4K);
if (!iommu-reg) {
printk(KERN_ERR IOMMU: can't map the region\n);
@@ -1396,7 +1426,7 @@
int index;
while (dev) {
- for (index = 0; index cnt; index ++)
+ for (index = 0; index cnt; index++)
if (dev == devices[index])
return 1;
@@ -1661,7 +1691,7 @@
struct dmar_rmrr_unit *rmrr;
struct pci_dev *pdev;
struct intel_iommu *iommu;
- int ret, unit = 0;
+ int nlongs, i, ret, unit = 0;
/*
* for each drhd
@@ -1672,7 +1702,30 @@
for_each_drhd_unit(drhd) {
if (drhd-ignored)
continue;
- iommu = alloc_iommu(drhd);
+ g_num_of_iommus++;
+ }
+
+ nlongs = BITS_TO_LONGS(g_num_of_iommus);
+ g_iommus_to_flush = kzalloc(nlongs * sizeof(unsigned long), GFP_KERNEL);
+ if (!g_iommus_to_flush) {
+ printk(KERN_ERR Intel-IOMMU:
+ Allocating bitmap array failed\n);
+ return -ENOMEM;
+ }
+
+ g_iommus = kzalloc(g_num_of_iommus * sizeof(*iommu), GFP_KERNEL);
+ if (!g_iommus) {
+ kfree(g_iommus_to_flush);
+ ret
