Re: [PATCH 3/3] hid: logitech-dj: delete unnecessary error messages

[Marcus Folkesson]

Hello Christoph,

On Wed, Feb 07, 2018 at 06:08:00PM +0100, Christoph Böhmwalder wrote:
> Remove some "out of memory" messages that are considered useless.
> 
> Signed-off-by: Christoph Böhmwalder 
> ---
>  drivers/hid/hid-logitech-dj.c | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 59c54cb4bc64..ba5239840cda 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -414,11 +414,8 @@ static void logi_dj_recv_add_djhid_device(struct 
> dj_receiver_dev *djrcv_dev,
>  
>   dj_dev = kzalloc(sizeof(*dj_dev), GFP_KERNEL);
>  
> - if (!dj_dev) {
> - dev_err(_hdev->dev, "%s: failed allocating dj_device\n",
> - __func__);
> + if (!dj_dev)
>   goto dj_device_allocate_fail;
> - }
>  
>   dj_dev->reports_supported =
>   get_unaligned_le32(dj_report->report_params
> @@ -1015,11 +1012,9 @@ static int logi_dj_probe(struct hid_device *hdev,
>   /* Treat interface 2 */
>  
>   djrcv_dev = kzalloc(sizeof(*djrcv_dev), GFP_KERNEL);
> - if (!djrcv_dev) {
> - dev_err(>dev,
> - "%s:failed allocating dj_receiver_dev\n", __func__);
> + if (!djrcv_dev)
>   return -ENOMEM;
> - }
> +
>   djrcv_dev->hdev = hdev;
>   INIT_WORK(_dev->work, delayedwork_callback);
>   spin_lock_init(_dev->lock);
> -- 
> 2.13.6
> 

Thank you, but Markus Elfring already has a submitted a patch for this one.

/Marcus


signature.asc
Description: PGP signature

Re: [PATCH 3/3] hid: logitech-dj: delete unnecessary error messages

[Marcus Folkesson]

Hello Christoph,

On Wed, Feb 07, 2018 at 06:08:00PM +0100, Christoph Böhmwalder wrote:
> Remove some "out of memory" messages that are considered useless.
> 
> Signed-off-by: Christoph Böhmwalder 
> ---
>  drivers/hid/hid-logitech-dj.c | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 59c54cb4bc64..ba5239840cda 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -414,11 +414,8 @@ static void logi_dj_recv_add_djhid_device(struct 
> dj_receiver_dev *djrcv_dev,
>  
>   dj_dev = kzalloc(sizeof(*dj_dev), GFP_KERNEL);
>  
> - if (!dj_dev) {
> - dev_err(_hdev->dev, "%s: failed allocating dj_device\n",
> - __func__);
> + if (!dj_dev)
>   goto dj_device_allocate_fail;
> - }
>  
>   dj_dev->reports_supported =
>   get_unaligned_le32(dj_report->report_params
> @@ -1015,11 +1012,9 @@ static int logi_dj_probe(struct hid_device *hdev,
>   /* Treat interface 2 */
>  
>   djrcv_dev = kzalloc(sizeof(*djrcv_dev), GFP_KERNEL);
> - if (!djrcv_dev) {
> - dev_err(>dev,
> - "%s:failed allocating dj_receiver_dev\n", __func__);
> + if (!djrcv_dev)
>   return -ENOMEM;
> - }
> +
>   djrcv_dev->hdev = hdev;
>   INIT_WORK(_dev->work, delayedwork_callback);
>   spin_lock_init(_dev->lock);
> -- 
> 2.13.6
> 

Thank you, but Markus Elfring already has a submitted a patch for this one.

/Marcus


signature.asc
Description: PGP signature

Re: [PATCH] ACPI/IORT: Remove linker section for IORT entries again

[Daniel Lezcano]

On 07/02/2018 05:11, Jia He wrote:
> In commit 316ca8804ea8 ("ACPI/IORT: Remove linker section for IORT entries
> probing"), iort entries was removed in vmlinux.lds.h. But in
> commit 2fcc112af37f ("clocksource/drivers: Rename clksrc table to timer"),
> this line was back incorrectly.
> 
> It does no harm except for adding some useless symbols, so fix it.
> 
> Signed-off-by: Jia He 

Acked-by: Daniel Lezcano 

> ---
>  include/asm-generic/vmlinux.lds.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h 
> b/include/asm-generic/vmlinux.lds.h
> index 1ab0e52..58b1dab 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -589,7 +589,6 @@
>   IRQCHIP_OF_MATCH_TABLE()\
>   ACPI_PROBE_TABLE(irqchip)   \
>   ACPI_PROBE_TABLE(timer) \
> - ACPI_PROBE_TABLE(iort)  \
>   EARLYCON_TABLE()
>  
>  #define INIT_TEXT\

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] ACPI/IORT: Remove linker section for IORT entries again

[Daniel Lezcano]

On 07/02/2018 05:11, Jia He wrote:
> In commit 316ca8804ea8 ("ACPI/IORT: Remove linker section for IORT entries
> probing"), iort entries was removed in vmlinux.lds.h. But in
> commit 2fcc112af37f ("clocksource/drivers: Rename clksrc table to timer"),
> this line was back incorrectly.
> 
> It does no harm except for adding some useless symbols, so fix it.
> 
> Signed-off-by: Jia He 

Acked-by: Daniel Lezcano 

> ---
>  include/asm-generic/vmlinux.lds.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h 
> b/include/asm-generic/vmlinux.lds.h
> index 1ab0e52..58b1dab 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -589,7 +589,6 @@
>   IRQCHIP_OF_MATCH_TABLE()\
>   ACPI_PROBE_TABLE(irqchip)   \
>   ACPI_PROBE_TABLE(timer) \
> - ACPI_PROBE_TABLE(iort)  \
>   EARLYCON_TABLE()
>  
>  #define INIT_TEXT\

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] selftests/x86: Remove 5lvl test from Makefile

[Kirill A. Shutemov]

On Wed, Feb 07, 2018 at 11:50:28PM -0600, Daniel Díaz wrote:
> The 5lvl.c test file was incorporated into another one in
> selftests/vm (va_128TBswitch.c) in commit 235266b8.
> 
> Signed-off-by: Daniel Díaz 

Acked-by: Kirill A. Shutemov 

-- 
 Kirill A. Shutemov

Re: [PATCH] selftests/x86: Remove 5lvl test from Makefile

[Kirill A. Shutemov]

On Wed, Feb 07, 2018 at 11:50:28PM -0600, Daniel Díaz wrote:
> The 5lvl.c test file was incorporated into another one in
> selftests/vm (va_128TBswitch.c) in commit 235266b8.
> 
> Signed-off-by: Daniel Díaz 

Acked-by: Kirill A. Shutemov 

-- 
 Kirill A. Shutemov

Re: [PATCH v2] ARM: dts: imx6ull: add Toradex Colibri iMX6ULL support

[Shawn Guo]

On Tue, Feb 06, 2018 at 05:49:03PM +0100, Stefan Agner wrote:
> Add support for the Computer on Module Colibri iMX6ULL and its
> Bluetooth/Wifi variant along with the development/evaluation carrier
> board device trees. Follow the usual hierarchic include model,
> maintaining shared configuration in imx6ull-colibri.dtsi and
> imx6ull-colibri-eval-v3.dtsi respectively.
> 
> Signed-off-by: Stefan Agner 
> ---
> This depends on the following patchsets work:
> - https://lkml.org/lkml/2018/1/6/129 (applied)
> - https://lkml.org/lkml/2018/1/10/998 (applied)
> - https://www.spinics.net/lists/arm-kernel/msg632671.html (pending, required)
> - https://lkml.org/lkml/2018/1/18/850 (only for highest CPU frequency)

So the only dependency is the cpufreq change now.  So we have two
options:

1. Hold the patch until the cpufreq change appear on my tree.  That
   will require us wait for another release cycle.

2. Drop the highest CPU frequency, so that we can apply the patch right
   away, and add that setpoint after dependant cpufreq change lands
   mainline.

Shawn

> 
> Changes in v2:
> - Dropped new unnecessary lines
> - Sorting pinctrl nodes alphabetically
> - Switch RTC label/node name
> - Use wakeup-source instead of enable-sdio-wakeup
> - Followed renaming of mux mode name REF_CLK_32K to OSC32K_32K_OUT
> - Use fixed ECC strenght/size
> 
>  arch/arm/boot/dts/Makefile |   4 +-
>  arch/arm/boot/dts/imx6ull-colibri-eval-v3.dts  |  14 +
>  arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi | 157 ++
>  arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi |  24 +
>  arch/arm/boot/dts/imx6ull-colibri-wifi-eval-v3.dts |  14 +
>  arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi|  65 +++
>  arch/arm/boot/dts/imx6ull-colibri.dtsi | 554 
> +
>  7 files changed, 831 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri-eval-v3.dts
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri-wifi-eval-v3.dts
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri.dtsi
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 7fc7bcefc3a0..6411b3069e8d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -525,7 +525,9 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
>   imx6ul-tx6ul-0010.dtb \
>   imx6ul-tx6ul-0011.dtb \
>   imx6ul-tx6ul-mainboard.dtb \
> - imx6ull-14x14-evk.dtb
> + imx6ull-14x14-evk.dtb \
> + imx6ull-colibri-eval-v3.dtb \
> + imx6ull-colibri-wifi-eval-v3.dtb
>  dtb-$(CONFIG_SOC_IMX7D) += \
>   imx7d-cl-som-imx7.dtb \
>   imx7d-colibri-emmc-eval-v3.dtb \
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dts 
> b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dts
> new file mode 100644
> index ..08669a18349e
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dts
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright 2018 Toradex AG
> + */
> +
> +/dts-v1/;
> +
> +#include "imx6ull-colibri-nonwifi.dtsi"
> +#include "imx6ull-colibri-eval-v3.dtsi"
> +
> +/ {
> + model = "Toradex Colibri iMX6ULL 256MB on Colibri Evaluation Board V3";
> + compatible = "toradex,colibri-imx6ull-eval", "fsl,imx6ull";
> +};
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi 
> b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> new file mode 100644
> index ..006690ea98c0
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright 2017 Toradex AG
> + */
> +
> +/ {
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + /* fixed crystal dedicated to mcp2515 */
> + clk16m: clk16m {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1600>;
> + };
> +
> + panel: panel {
> + compatible = "edt,et057090dhu";
> + backlight = <>;
> + power-supply = <_3v3>;
> +
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +
> + reg_3v3: regulator-3v3 {
> + compatible = "regulator-fixed";
> + regulator-name = "3.3V";
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
> + };
> +
> + reg_5v0: regulator-5v0 {
> + compatible = "regulator-fixed";
> + regulator-name = "5V";
> + regulator-min-microvolt = <500>;
> + regulator-max-microvolt = <500>;
> + };
> +
> + 

Re: [PATCH v2] ARM: dts: imx6ull: add Toradex Colibri iMX6ULL support

[Shawn Guo]

On Tue, Feb 06, 2018 at 05:49:03PM +0100, Stefan Agner wrote:
> Add support for the Computer on Module Colibri iMX6ULL and its
> Bluetooth/Wifi variant along with the development/evaluation carrier
> board device trees. Follow the usual hierarchic include model,
> maintaining shared configuration in imx6ull-colibri.dtsi and
> imx6ull-colibri-eval-v3.dtsi respectively.
> 
> Signed-off-by: Stefan Agner 
> ---
> This depends on the following patchsets work:
> - https://lkml.org/lkml/2018/1/6/129 (applied)
> - https://lkml.org/lkml/2018/1/10/998 (applied)
> - https://www.spinics.net/lists/arm-kernel/msg632671.html (pending, required)
> - https://lkml.org/lkml/2018/1/18/850 (only for highest CPU frequency)

So the only dependency is the cpufreq change now.  So we have two
options:

1. Hold the patch until the cpufreq change appear on my tree.  That
   will require us wait for another release cycle.

2. Drop the highest CPU frequency, so that we can apply the patch right
   away, and add that setpoint after dependant cpufreq change lands
   mainline.

Shawn

> 
> Changes in v2:
> - Dropped new unnecessary lines
> - Sorting pinctrl nodes alphabetically
> - Switch RTC label/node name
> - Use wakeup-source instead of enable-sdio-wakeup
> - Followed renaming of mux mode name REF_CLK_32K to OSC32K_32K_OUT
> - Use fixed ECC strenght/size
> 
>  arch/arm/boot/dts/Makefile |   4 +-
>  arch/arm/boot/dts/imx6ull-colibri-eval-v3.dts  |  14 +
>  arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi | 157 ++
>  arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi |  24 +
>  arch/arm/boot/dts/imx6ull-colibri-wifi-eval-v3.dts |  14 +
>  arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi|  65 +++
>  arch/arm/boot/dts/imx6ull-colibri.dtsi | 554 
> +
>  7 files changed, 831 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri-eval-v3.dts
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri-wifi-eval-v3.dts
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
>  create mode 100644 arch/arm/boot/dts/imx6ull-colibri.dtsi
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 7fc7bcefc3a0..6411b3069e8d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -525,7 +525,9 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
>   imx6ul-tx6ul-0010.dtb \
>   imx6ul-tx6ul-0011.dtb \
>   imx6ul-tx6ul-mainboard.dtb \
> - imx6ull-14x14-evk.dtb
> + imx6ull-14x14-evk.dtb \
> + imx6ull-colibri-eval-v3.dtb \
> + imx6ull-colibri-wifi-eval-v3.dtb
>  dtb-$(CONFIG_SOC_IMX7D) += \
>   imx7d-cl-som-imx7.dtb \
>   imx7d-colibri-emmc-eval-v3.dtb \
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dts 
> b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dts
> new file mode 100644
> index ..08669a18349e
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dts
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright 2018 Toradex AG
> + */
> +
> +/dts-v1/;
> +
> +#include "imx6ull-colibri-nonwifi.dtsi"
> +#include "imx6ull-colibri-eval-v3.dtsi"
> +
> +/ {
> + model = "Toradex Colibri iMX6ULL 256MB on Colibri Evaluation Board V3";
> + compatible = "toradex,colibri-imx6ull-eval", "fsl,imx6ull";
> +};
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi 
> b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> new file mode 100644
> index ..006690ea98c0
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright 2017 Toradex AG
> + */
> +
> +/ {
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + /* fixed crystal dedicated to mcp2515 */
> + clk16m: clk16m {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1600>;
> + };
> +
> + panel: panel {
> + compatible = "edt,et057090dhu";
> + backlight = <>;
> + power-supply = <_3v3>;
> +
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +
> + reg_3v3: regulator-3v3 {
> + compatible = "regulator-fixed";
> + regulator-name = "3.3V";
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
> + };
> +
> + reg_5v0: regulator-5v0 {
> + compatible = "regulator-fixed";
> + regulator-name = "5V";
> + regulator-min-microvolt = <500>;
> + regulator-max-microvolt = <500>;
> + };
> +
> + reg_usbh_vbus: 

Re: [PATCH v2] drm/bridge/synopsys: dsi: Add 1.31 version support

[Andrzej Hajda]

On 06.02.2018 09:42, Philippe Cornu wrote:
> Add support for the Synopsys DesignWare MIPI DSI version 1.31
> Two registers need to be updated/added for supporting 1.31:
> * PHY_TMR_CFG 0x9c (updated)
>   1.30 [31:24] phy_hs2lp_time
>[23:16] phy_lp2hs_time
>[14: 0] max_rd_time
>
>   1.31 [25:16] phy_hs2lp_time
>[ 9: 0] phy_lp2hs_time
>
> * PHY_TMR_RD_CFG 0xf4 (new)
>   1.31 [14: 0] max_rd_time
>
> Signed-off-by: Philippe Cornu 
Queued to drm-misc-next.
--
Regards
Andrzej

Re: [PATCH] drm/bridge/synopsys: dsi: Add read feature

[Andrzej Hajda]

On 04.02.2018 22:31, Philippe Cornu wrote:
> This patch adds the DCS/GENERIC DSI read feature.
>
> Signed-off-by: Philippe Cornu 
> ---
Queued to drm-misc-next.
--
Regards
Andrzej

Re: [PATCH v2] drm/bridge/synopsys: dsi: Add 1.31 version support

[Andrzej Hajda]

On 06.02.2018 09:42, Philippe Cornu wrote:
> Add support for the Synopsys DesignWare MIPI DSI version 1.31
> Two registers need to be updated/added for supporting 1.31:
> * PHY_TMR_CFG 0x9c (updated)
>   1.30 [31:24] phy_hs2lp_time
>[23:16] phy_lp2hs_time
>[14: 0] max_rd_time
>
>   1.31 [25:16] phy_hs2lp_time
>[ 9: 0] phy_lp2hs_time
>
> * PHY_TMR_RD_CFG 0xf4 (new)
>   1.31 [14: 0] max_rd_time
>
> Signed-off-by: Philippe Cornu 
Queued to drm-misc-next.
--
Regards
Andrzej

Re: [PATCH] drm/bridge/synopsys: dsi: Add read feature

[Andrzej Hajda]

On 04.02.2018 22:31, Philippe Cornu wrote:
> This patch adds the DCS/GENERIC DSI read feature.
>
> Signed-off-by: Philippe Cornu 
> ---
Queued to drm-misc-next.
--
Regards
Andrzej

Re: [PATCH] KVM: X86: Fix SMRAM accessing even if VM is shutdown

[Wanpeng Li]

2018-02-07 22:16 GMT+08:00 Paolo Bonzini :
> On 07/02/2018 07:25, Wanpeng Li wrote:
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 786cd00..445e702 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -7458,6 +7458,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, 
>> struct kvm_run *kvm_run)
>>   goto out;
>>   }
>>
>> + if (unlikely(vcpu->run->exit_reason == KVM_EXIT_SHUTDOWN)) {
>> + r = -EINVAL;
>> + goto out;
>> + }
>> +
>>   if (vcpu->run->kvm_dirty_regs) {
>>   r = sync_regs(vcpu);
>>   if (r != 0)
>>
>
> This most likely breaks triple faults in the usual case where they
> should result in resetting the system; the KVM API doesn't say that you
> should clear vcpu->run->exit_reason before entering.
>
> What exactly causes the EPT misconfig to reach the WARN?  That is, how
> does kvm_mmu_page_fault end up returning a negative errno value?  If I
> read the code correctly only tdp_page_fault can do so, so my guess would
> be kvm_handle_bad_page:
>
> if (pfn == KVM_PFN_ERR_RO_FAULT)
> return RET_PF_EMULATE;
>
> if (pfn == KVM_PFN_ERR_HWPOISON) {
> kvm_send_hwpoison_signal(kvm_vcpu_gfn_to_hva(vcpu, gfn),
> current);
> return RET_PF_RETRY;
> }
>
> /* KVM_PFN_ERR_FAULT */
> return -EFAULT;
>
> Maybe it should return RET_PF_EMULATE, which would cause an emulation
> failure and then an exit with KVM_EXIT_INTERNAL_ERROR.

Agreed, just do it in v2. In addition, I didn't remove the
RET_PFN_ERR_PO_FAULT check since I think otherwise we will miss the
comments above it.

Regards,
Wanpeng Li

Re: [PATCH] KVM: X86: Fix SMRAM accessing even if VM is shutdown

[Wanpeng Li]

2018-02-07 22:16 GMT+08:00 Paolo Bonzini :
> On 07/02/2018 07:25, Wanpeng Li wrote:
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 786cd00..445e702 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -7458,6 +7458,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, 
>> struct kvm_run *kvm_run)
>>   goto out;
>>   }
>>
>> + if (unlikely(vcpu->run->exit_reason == KVM_EXIT_SHUTDOWN)) {
>> + r = -EINVAL;
>> + goto out;
>> + }
>> +
>>   if (vcpu->run->kvm_dirty_regs) {
>>   r = sync_regs(vcpu);
>>   if (r != 0)
>>
>
> This most likely breaks triple faults in the usual case where they
> should result in resetting the system; the KVM API doesn't say that you
> should clear vcpu->run->exit_reason before entering.
>
> What exactly causes the EPT misconfig to reach the WARN?  That is, how
> does kvm_mmu_page_fault end up returning a negative errno value?  If I
> read the code correctly only tdp_page_fault can do so, so my guess would
> be kvm_handle_bad_page:
>
> if (pfn == KVM_PFN_ERR_RO_FAULT)
> return RET_PF_EMULATE;
>
> if (pfn == KVM_PFN_ERR_HWPOISON) {
> kvm_send_hwpoison_signal(kvm_vcpu_gfn_to_hva(vcpu, gfn),
> current);
> return RET_PF_RETRY;
> }
>
> /* KVM_PFN_ERR_FAULT */
> return -EFAULT;
>
> Maybe it should return RET_PF_EMULATE, which would cause an emulation
> failure and then an exit with KVM_EXIT_INTERNAL_ERROR.

Agreed, just do it in v2. In addition, I didn't remove the
RET_PFN_ERR_PO_FAULT check since I think otherwise we will miss the
comments above it.

Regards,
Wanpeng Li

Re: [PATCH 05/19] clk: meson: add regmap clocks

[Yixun Lan]

HI Jerome:

On 02/01/18 02:09, Jerome Brunet wrote:
> Meson clock controllers needs to move the classical iomem registers to
> regmap. This is triggered because the HHI controllers found on the GXBB
> and GXL host more than just clocks. To properly handle this, we would
> like to migrate HHI to syscon. Also GXBB AO clock controller already use
> regmap, AXG AO and Audio clock controllers will as well.
> 
> The purpose of this change is to provide a common structure to these
> meson controllers (and possibly others) for regmap based clocks.
> 
> This change provides the basic gate, mux and divider, based on the
> helpers provided by the related generic clocks
> 
> Signed-off-by: Jerome Brunet 
> ---
>  drivers/clk/meson/Kconfig  |   4 +
>  drivers/clk/meson/Makefile |   1 +
>  drivers/clk/meson/clk-regmap.c | 166 
> +
>  drivers/clk/meson/clk-regmap.h | 111 +++
>  4 files changed, 282 insertions(+)
>  create mode 100644 drivers/clk/meson/clk-regmap.c
>  create mode 100644 drivers/clk/meson/clk-regmap.h
> 
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 7694302c70a4..e97e85077da1 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -3,6 +3,10 @@ config COMMON_CLK_AMLOGIC
>   depends on OF
>   depends on ARCH_MESON || COMPILE_TEST
>  
> +config COMMON_CLK_REGMAP_MESON
> + bool
> + select REGMAP
> +
>  config COMMON_CLK_MESON8B
>   bool
>   depends on COMMON_CLK_AMLOGIC
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 3c03ce583798..11a5058a 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-cpu.o 
> clk-mpll.o clk-audio-div
>  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o 
> gxbb-aoclk-regmap.o gxbb-aoclk-32k.o
>  obj-$(CONFIG_COMMON_CLK_AXG)  += axg.o
> +obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)+= clk-regmap.o
> diff --git a/drivers/clk/meson/clk-regmap.c b/drivers/clk/meson/clk-regmap.c
> new file mode 100644
> index ..3645fdb62343
> --- /dev/null
> +++ b/drivers/clk/meson/clk-regmap.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 BayLibre, SAS.
> +// Author: Jerome Brunet 
> +
> +#include "clk-regmap.h"
> +
> +static int clk_regmap_gate_endisable(struct clk_hw *hw, int enable)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_gate_data *gate = clk_get_regmap_gate_data(clk);
> + int set = gate->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0;
> +
> + set ^= enable;
> +
> + return regmap_update_bits(clk->map, gate->offset, BIT(gate->bit_idx),
> +   set ? BIT(gate->bit_idx) : 0);
> +}
> +
> +static int clk_regmap_gate_enable(struct clk_hw *hw)
> +{
> + return clk_regmap_gate_endisable(hw, 1);
> +}
> +
> +static void clk_regmap_gate_disable(struct clk_hw *hw)
> +{
> + clk_regmap_gate_endisable(hw, 0);
> +}
> +
> +static int clk_regmap_gate_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_gate_data *gate = clk_get_regmap_gate_data(clk);
> + unsigned int val;
> +
> + regmap_read(clk->map, gate->offset, );
> + if (gate->flags & CLK_GATE_SET_TO_DISABLE)
> + val ^= BIT(gate->bit_idx);
> +
> + val &= BIT(gate->bit_idx);
> +
> + return val ? 1 : 0;
> +}
> +
> +const struct clk_ops clk_regmap_gate_ops = {
> + .enable = clk_regmap_gate_enable,
> + .disable = clk_regmap_gate_disable,
> + .is_enabled = clk_regmap_gate_is_enabled,
> +};
> +EXPORT_SYMBOL_GPL(clk_regmap_gate_ops);
> +
> +static unsigned long clk_regmap_div_recalc_rate(struct clk_hw *hw,
> + unsigned long prate)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_div_data *div = clk_get_regmap_div_data(clk);
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(clk->map, div->offset, );
> + if (ret)
> + /* Gives a hint that something is wrong */
> + return 0;
> +
> + val >>= div->shift;
> + val &= clk_div_mask(div->width);
> + return divider_recalc_rate(hw, prate, val, div->table, div->flags,
> +div->width);
> +}
> +
> +static long clk_regmap_div_round_rate(struct clk_hw *hw, unsigned long rate,
> +   unsigned long *prate)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_div_data *div = clk_get_regmap_div_data(clk);
> + unsigned int val;
> + int ret;
> +
> + /* if read only, just return current value */
> + if (div->flags & CLK_DIVIDER_READ_ONLY) {
> + ret = regmap_read(clk->map, div->offset, 

Re: [PATCH 05/19] clk: meson: add regmap clocks

[Yixun Lan]

HI Jerome:

On 02/01/18 02:09, Jerome Brunet wrote:
> Meson clock controllers needs to move the classical iomem registers to
> regmap. This is triggered because the HHI controllers found on the GXBB
> and GXL host more than just clocks. To properly handle this, we would
> like to migrate HHI to syscon. Also GXBB AO clock controller already use
> regmap, AXG AO and Audio clock controllers will as well.
> 
> The purpose of this change is to provide a common structure to these
> meson controllers (and possibly others) for regmap based clocks.
> 
> This change provides the basic gate, mux and divider, based on the
> helpers provided by the related generic clocks
> 
> Signed-off-by: Jerome Brunet 
> ---
>  drivers/clk/meson/Kconfig  |   4 +
>  drivers/clk/meson/Makefile |   1 +
>  drivers/clk/meson/clk-regmap.c | 166 
> +
>  drivers/clk/meson/clk-regmap.h | 111 +++
>  4 files changed, 282 insertions(+)
>  create mode 100644 drivers/clk/meson/clk-regmap.c
>  create mode 100644 drivers/clk/meson/clk-regmap.h
> 
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 7694302c70a4..e97e85077da1 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -3,6 +3,10 @@ config COMMON_CLK_AMLOGIC
>   depends on OF
>   depends on ARCH_MESON || COMPILE_TEST
>  
> +config COMMON_CLK_REGMAP_MESON
> + bool
> + select REGMAP
> +
>  config COMMON_CLK_MESON8B
>   bool
>   depends on COMMON_CLK_AMLOGIC
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 3c03ce583798..11a5058a 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-cpu.o 
> clk-mpll.o clk-audio-div
>  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o 
> gxbb-aoclk-regmap.o gxbb-aoclk-32k.o
>  obj-$(CONFIG_COMMON_CLK_AXG)  += axg.o
> +obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)+= clk-regmap.o
> diff --git a/drivers/clk/meson/clk-regmap.c b/drivers/clk/meson/clk-regmap.c
> new file mode 100644
> index ..3645fdb62343
> --- /dev/null
> +++ b/drivers/clk/meson/clk-regmap.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 BayLibre, SAS.
> +// Author: Jerome Brunet 
> +
> +#include "clk-regmap.h"
> +
> +static int clk_regmap_gate_endisable(struct clk_hw *hw, int enable)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_gate_data *gate = clk_get_regmap_gate_data(clk);
> + int set = gate->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0;
> +
> + set ^= enable;
> +
> + return regmap_update_bits(clk->map, gate->offset, BIT(gate->bit_idx),
> +   set ? BIT(gate->bit_idx) : 0);
> +}
> +
> +static int clk_regmap_gate_enable(struct clk_hw *hw)
> +{
> + return clk_regmap_gate_endisable(hw, 1);
> +}
> +
> +static void clk_regmap_gate_disable(struct clk_hw *hw)
> +{
> + clk_regmap_gate_endisable(hw, 0);
> +}
> +
> +static int clk_regmap_gate_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_gate_data *gate = clk_get_regmap_gate_data(clk);
> + unsigned int val;
> +
> + regmap_read(clk->map, gate->offset, );
> + if (gate->flags & CLK_GATE_SET_TO_DISABLE)
> + val ^= BIT(gate->bit_idx);
> +
> + val &= BIT(gate->bit_idx);
> +
> + return val ? 1 : 0;
> +}
> +
> +const struct clk_ops clk_regmap_gate_ops = {
> + .enable = clk_regmap_gate_enable,
> + .disable = clk_regmap_gate_disable,
> + .is_enabled = clk_regmap_gate_is_enabled,
> +};
> +EXPORT_SYMBOL_GPL(clk_regmap_gate_ops);
> +
> +static unsigned long clk_regmap_div_recalc_rate(struct clk_hw *hw,
> + unsigned long prate)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_div_data *div = clk_get_regmap_div_data(clk);
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(clk->map, div->offset, );
> + if (ret)
> + /* Gives a hint that something is wrong */
> + return 0;
> +
> + val >>= div->shift;
> + val &= clk_div_mask(div->width);
> + return divider_recalc_rate(hw, prate, val, div->table, div->flags,
> +div->width);
> +}
> +
> +static long clk_regmap_div_round_rate(struct clk_hw *hw, unsigned long rate,
> +   unsigned long *prate)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_div_data *div = clk_get_regmap_div_data(clk);
> + unsigned int val;
> + int ret;
> +
> + /* if read only, just return current value */
> + if (div->flags & CLK_DIVIDER_READ_ONLY) {
> + ret = regmap_read(clk->map, div->offset, );
> + if (ret)
> + 

[PATCH v2] KVM: X86: Fix SMRAM accessing even if VM is shutdown

[Wanpeng Li]

From: Wanpeng Li 

Reported by syzkaller:

   WARNING: CPU: 6 PID: 2434 at arch/x86/kvm/vmx.c:6660 
handle_ept_misconfig+0x54/0x1e0 [kvm_intel]
   CPU: 6 PID: 2434 Comm: repro_test Not tainted 4.15.0+ #4
   RIP: 0010:handle_ept_misconfig+0x54/0x1e0 [kvm_intel]
   Call Trace:
vmx_handle_exit+0xbd/0xe20 [kvm_intel]
kvm_arch_vcpu_ioctl_run+0xdaf/0x1d50 [kvm]
kvm_vcpu_ioctl+0x3e9/0x720 [kvm]
do_vfs_ioctl+0xa4/0x6a0
SyS_ioctl+0x79/0x90
entry_SYSCALL_64_fastpath+0x25/0x9c

The syzkaller creates a former thread to issue KVM_SMI ioctl, and then creates
a latter thread to mmap and operate on the same vCPU, rsm emulation will not be 
executed since there is no something like seabios which implements smi handler 
when running syzkaller directly. This triggers a race condition when running 
the testcase with multiple threads. Sometimes one thread exit w/ SHUTDOWN 
reason, another thread mmaps and operates on the same vCPU, it continues to 
use CS=0x3, IP=0x8000 to access the address of SMI handler which results 
in the above ept misconfig. This patch fixes it by returning RET_PF_EMULATE
in kvm_handle_bad_page() which would cause an emulation failure and then an 
exit with KVM_EXIT_INTERNAL_ERROR, the VM will stop instead of resetting and 
be taken advantage of.

Reported-by: 
syzbot+c1d9517cab094dae65e446c0c5b4de6c40f4d...@syzkaller.appspotmail.com
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Signed-off-by: Wanpeng Li 
---
v1 -> v2:
 * return RET_PF_EMULATE

 arch/x86/kvm/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 8eca1d0..6c5a82c 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3029,7 +3029,7 @@ static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, 
gfn_t gfn, kvm_pfn_t pfn)
return RET_PF_RETRY;
}
 
-   return -EFAULT;
+   return RET_PF_EMULATE;
 }
 
 static void transparent_hugepage_adjust(struct kvm_vcpu *vcpu,
-- 
2.7.4

[PATCH v2] KVM: X86: Fix SMRAM accessing even if VM is shutdown

[Wanpeng Li]

From: Wanpeng Li 

Reported by syzkaller:

   WARNING: CPU: 6 PID: 2434 at arch/x86/kvm/vmx.c:6660 
handle_ept_misconfig+0x54/0x1e0 [kvm_intel]
   CPU: 6 PID: 2434 Comm: repro_test Not tainted 4.15.0+ #4
   RIP: 0010:handle_ept_misconfig+0x54/0x1e0 [kvm_intel]
   Call Trace:
vmx_handle_exit+0xbd/0xe20 [kvm_intel]
kvm_arch_vcpu_ioctl_run+0xdaf/0x1d50 [kvm]
kvm_vcpu_ioctl+0x3e9/0x720 [kvm]
do_vfs_ioctl+0xa4/0x6a0
SyS_ioctl+0x79/0x90
entry_SYSCALL_64_fastpath+0x25/0x9c

The syzkaller creates a former thread to issue KVM_SMI ioctl, and then creates
a latter thread to mmap and operate on the same vCPU, rsm emulation will not be 
executed since there is no something like seabios which implements smi handler 
when running syzkaller directly. This triggers a race condition when running 
the testcase with multiple threads. Sometimes one thread exit w/ SHUTDOWN 
reason, another thread mmaps and operates on the same vCPU, it continues to 
use CS=0x3, IP=0x8000 to access the address of SMI handler which results 
in the above ept misconfig. This patch fixes it by returning RET_PF_EMULATE
in kvm_handle_bad_page() which would cause an emulation failure and then an 
exit with KVM_EXIT_INTERNAL_ERROR, the VM will stop instead of resetting and 
be taken advantage of.

Reported-by: 
syzbot+c1d9517cab094dae65e446c0c5b4de6c40f4d...@syzkaller.appspotmail.com
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Signed-off-by: Wanpeng Li 
---
v1 -> v2:
 * return RET_PF_EMULATE

 arch/x86/kvm/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 8eca1d0..6c5a82c 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3029,7 +3029,7 @@ static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, 
gfn_t gfn, kvm_pfn_t pfn)
return RET_PF_RETRY;
}
 
-   return -EFAULT;
+   return RET_PF_EMULATE;
 }
 
 static void transparent_hugepage_adjust(struct kvm_vcpu *vcpu,
-- 
2.7.4

[PATCH 2/2] regmap-i2c: Off by one in regmap_i2c_smbus_i2c_read/write()

[Dan Carpenter]

The commit message says that we are allowed to read and write up to 32
bytes but the code only allows us to write 31 bytes.  In other words,
the ">=" should be changed to ">".  But this is already checked in
regmap_raw_read()/write() so we can just remove the if statemetents.

Fixes: 29332534e2b6 ("regmap-i2c: Add smbus i2c block support")
Signed-off-by: Dan Carpenter 
---
Not tested.

diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index 4735318f4268..056acde5e7d3 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -217,8 +217,6 @@ static int regmap_i2c_smbus_i2c_write(void *context, const 
void *data,
 
if (count < 1)
return -EINVAL;
-   if (count >= I2C_SMBUS_BLOCK_MAX)
-   return -E2BIG;
 
--count;
return i2c_smbus_write_i2c_block_data(i2c, ((u8 *)data)[0], count,
@@ -235,8 +233,6 @@ static int regmap_i2c_smbus_i2c_read(void *context, const 
void *reg,
 
if (reg_size != 1 || val_size < 1)
return -EINVAL;
-   if (val_size >= I2C_SMBUS_BLOCK_MAX)
-   return -E2BIG;
 
ret = i2c_smbus_read_i2c_block_data(i2c, ((u8 *)reg)[0], val_size, val);
if (ret == val_size)

[PATCH 2/2] regmap-i2c: Off by one in regmap_i2c_smbus_i2c_read/write()

[Dan Carpenter]

The commit message says that we are allowed to read and write up to 32
bytes but the code only allows us to write 31 bytes.  In other words,
the ">=" should be changed to ">".  But this is already checked in
regmap_raw_read()/write() so we can just remove the if statemetents.

Fixes: 29332534e2b6 ("regmap-i2c: Add smbus i2c block support")
Signed-off-by: Dan Carpenter 
---
Not tested.

diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index 4735318f4268..056acde5e7d3 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -217,8 +217,6 @@ static int regmap_i2c_smbus_i2c_write(void *context, const 
void *data,
 
if (count < 1)
return -EINVAL;
-   if (count >= I2C_SMBUS_BLOCK_MAX)
-   return -E2BIG;
 
--count;
return i2c_smbus_write_i2c_block_data(i2c, ((u8 *)data)[0], count,
@@ -235,8 +233,6 @@ static int regmap_i2c_smbus_i2c_read(void *context, const 
void *reg,
 
if (reg_size != 1 || val_size < 1)
return -EINVAL;
-   if (val_size >= I2C_SMBUS_BLOCK_MAX)
-   return -E2BIG;
 
ret = i2c_smbus_read_i2c_block_data(i2c, ((u8 *)reg)[0], val_size, val);
if (ret == val_size)

[PATCH 1/2] regmap: Fix reversed bounds check in regmap_raw_write()

[Dan Carpenter]

We're supposed to be checking that "val_len" is not too large but
instead we check if it is smaller than the max.

The only function affected would be regmap_i2c_smbus_i2c_write() in
drivers/base/regmap/regmap-i2c.c.  Strangely that function has its own
limit check which returns an error if (count >= I2C_SMBUS_BLOCK_MAX) so
it doesn't look like it has ever been able to do anything except return
an error.

Fixes: c335931ed9d2 ("regmap: Add raw_write/read checks for max_raw_write/read 
sizes")
Signed-off-by: Dan Carpenter 
---
This is from code review.  I can't test it.

Is it possible that there are other ways to reach
regmap_i2c_smbus_i2c_write() without going through regmap_raw_write()?
In that case, the temptation would be to just remove this check and the
one in regmap_raw_read().

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index ee302ccdfbc8..453116fd4362 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1831,7 +1831,7 @@ int regmap_raw_write(struct regmap *map, unsigned int reg,
return -EINVAL;
if (val_len % map->format.val_bytes)
return -EINVAL;
-   if (map->max_raw_write && map->max_raw_write > val_len)
+   if (map->max_raw_write && map->max_raw_write < val_len)
return -E2BIG;
 
map->lock(map->lock_arg);

[PATCH 1/2] regmap: Fix reversed bounds check in regmap_raw_write()

[Dan Carpenter]

We're supposed to be checking that "val_len" is not too large but
instead we check if it is smaller than the max.

The only function affected would be regmap_i2c_smbus_i2c_write() in
drivers/base/regmap/regmap-i2c.c.  Strangely that function has its own
limit check which returns an error if (count >= I2C_SMBUS_BLOCK_MAX) so
it doesn't look like it has ever been able to do anything except return
an error.

Fixes: c335931ed9d2 ("regmap: Add raw_write/read checks for max_raw_write/read 
sizes")
Signed-off-by: Dan Carpenter 
---
This is from code review.  I can't test it.

Is it possible that there are other ways to reach
regmap_i2c_smbus_i2c_write() without going through regmap_raw_write()?
In that case, the temptation would be to just remove this check and the
one in regmap_raw_read().

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index ee302ccdfbc8..453116fd4362 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1831,7 +1831,7 @@ int regmap_raw_write(struct regmap *map, unsigned int reg,
return -EINVAL;
if (val_len % map->format.val_bytes)
return -EINVAL;
-   if (map->max_raw_write && map->max_raw_write > val_len)
+   if (map->max_raw_write && map->max_raw_write < val_len)
return -E2BIG;
 
map->lock(map->lock_arg);

Re: [RFC v2 PATCH 6/7] x86/entry: get rid of ALLOC_PT_GPREGS_ON_STACK and SAVE_AND_CLEAR_REGS

[Dominik Brodowski]

On Wed, Feb 07, 2018 at 01:58:20PM -0800, Linus Torvalds wrote:
> I was just hoping that we could play some tricks.
> 
> [...]
> 
> See what I'm saying?

Clever. Though I'd include the "pushq %rsi" in the macro, to be even more
tricky.

   textdata bss dec hex filename
  19500   0   0   195004c2c arch/x86/entry/entry_64.o-orig
  24307   0   0   243075ef3 arch/x86/entry/entry_64.o-7_of_7
  20987   0   0   2098751fb arch/x86/entry/entry_64.o-trick

I'm not really sure yet where the increase in text size comes from in my
patch set, though.

I *hope* that the CS-ORIG_RAX(%rsp) testb is correct; again, excercise
an extremely stringent review of this patch, please.

Thanks,
Dominik


From: Dominik Brodowski 
Date: Wed, 7 Feb 2018 20:56:13 +0100
Subject: [PATCH] x86/entry: get rid of ALLOC_PT_GPREGS_ON_STACK and 
SAVE_AND_CLEAR_REGS

Previously, error_entry() and paranoid_entry() saved the GP registers
onto stack space previously allocated by its callers. Combine these two
steps in the callee, and use the generic PUSH_AND_CLEAR_REGS macro
for that, but play a litte trick in it -- suggested by Linus -- to insert
the GP registers "above" the original return address.

Suggested-by: Linus Torvalds 
Signed-off-by: Dominik Brodowski 

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index d6a97e2945ee..dc60365547e1 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -97,47 +97,7 @@ For 32-bit we have the following conventions - kernel is 
built with
 
 #define SIZEOF_PTREGS  21*8
 
-   .macro ALLOC_PT_GPREGS_ON_STACK
-   addq$-(15*8), %rsp
-   .endm
-
-   .macro SAVE_AND_CLEAR_REGS offset=0
-   /*
-* Save registers and sanitize registers of values that a
-* speculation attack might otherwise want to exploit. The
-* lower registers are likely clobbered well before they
-* could be put to use in a speculative execution gadget.
-* Interleave XOR with PUSH for better uop scheduling:
-*/
-   movq %rdi, 14*8+\offset(%rsp)
-   movq %rsi, 13*8+\offset(%rsp)
-   movq %rdx, 12*8+\offset(%rsp)
-   movq %rcx, 11*8+\offset(%rsp)
-   movq %rax, 10*8+\offset(%rsp)
-   movq %r8,  9*8+\offset(%rsp)
-   xorq %r8, %r8   /* nospec r8 */
-   movq %r9,  8*8+\offset(%rsp)
-   xorq %r9, %r9   /* nospec r9 */
-   movq %r10, 7*8+\offset(%rsp)
-   xorq %r10, %r10 /* nospec r10 */
-   movq %r11, 6*8+\offset(%rsp)
-   xorq %r11, %r11 /* nospec r11 */
-   movq %rbx, 5*8+\offset(%rsp)
-   xorl %ebx, %ebx /* nospec rbx */
-   movq %rbp, 4*8+\offset(%rsp)
-   xorl %ebp, %ebp /* nospec rbp */
-   movq %r12, 3*8+\offset(%rsp)
-   xorq %r12, %r12 /* nospec r12 */
-   movq %r13, 2*8+\offset(%rsp)
-   xorq %r13, %r13 /* nospec r13 */
-   movq %r14, 1*8+\offset(%rsp)
-   xorq %r14, %r14 /* nospec r14 */
-   movq %r15, 0*8+\offset(%rsp)
-   xorq %r15, %r15 /* nospec r15 */
-   UNWIND_HINT_REGS offset=\offset
-   .endm
-
-   .macro PUSH_AND_CLEAR_REGS rdx=%rdx rax=%rax
+   .macro PUSH_AND_CLEAR_REGS rdx=%rdx rax=%rax save_ret=0
/*
 * Push registers and sanitize registers of values that a
 * speculation attack might otherwise want to exploit. The
@@ -145,8 +105,14 @@ For 32-bit we have the following conventions - kernel is 
built with
 * could be put to use in a speculative execution gadget.
 * Interleave XOR with PUSH for better uop scheduling:
 */
+   .if \save_ret
+   pushq   %rsi/* pt_regs->si */
+   movq8(%rsp), %rsi   /* temporarily store ret address in %rsi */
+   movq%rdi, 8(%rsp)   /* pt_regs->di (overwriting original ret) */
+   .else
pushq   %rdi/* pt_regs->di */
pushq   %rsi/* pt_regs->si */
+   .endif
pushq   \rdx/* pt_regs->dx */
pushq   %rcx/* pt_regs->cx */
pushq   \rax/* pt_regs->ax */
@@ -171,6 +137,9 @@ For 32-bit we have the following conventions - kernel is 
built with
pushq   %r15/* pt_regs->r15 */
xorq%r15, %r15  /* nospec   r15*/
UNWIND_HINT_REGS
+   .if \save_ret
+   pushq   %rsi/* return address on top of stack */
+   .endif
.endm
 
.macro POP_REGS pop_rdi=1 skip_r11rcx=0
@@ -211,7 +180,7 @@ For 32-bit we have the following conventions - kernel is 
built with
  * is just setting the LSB, which makes it an 

Re: [RFC v2 PATCH 6/7] x86/entry: get rid of ALLOC_PT_GPREGS_ON_STACK and SAVE_AND_CLEAR_REGS

[Dominik Brodowski]

On Wed, Feb 07, 2018 at 01:58:20PM -0800, Linus Torvalds wrote:
> I was just hoping that we could play some tricks.
> 
> [...]
> 
> See what I'm saying?

Clever. Though I'd include the "pushq %rsi" in the macro, to be even more
tricky.

   textdata bss dec hex filename
  19500   0   0   195004c2c arch/x86/entry/entry_64.o-orig
  24307   0   0   243075ef3 arch/x86/entry/entry_64.o-7_of_7
  20987   0   0   2098751fb arch/x86/entry/entry_64.o-trick

I'm not really sure yet where the increase in text size comes from in my
patch set, though.

I *hope* that the CS-ORIG_RAX(%rsp) testb is correct; again, excercise
an extremely stringent review of this patch, please.

Thanks,
Dominik


From: Dominik Brodowski 
Date: Wed, 7 Feb 2018 20:56:13 +0100
Subject: [PATCH] x86/entry: get rid of ALLOC_PT_GPREGS_ON_STACK and 
SAVE_AND_CLEAR_REGS

Previously, error_entry() and paranoid_entry() saved the GP registers
onto stack space previously allocated by its callers. Combine these two
steps in the callee, and use the generic PUSH_AND_CLEAR_REGS macro
for that, but play a litte trick in it -- suggested by Linus -- to insert
the GP registers "above" the original return address.

Suggested-by: Linus Torvalds 
Signed-off-by: Dominik Brodowski 

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index d6a97e2945ee..dc60365547e1 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -97,47 +97,7 @@ For 32-bit we have the following conventions - kernel is 
built with
 
 #define SIZEOF_PTREGS  21*8
 
-   .macro ALLOC_PT_GPREGS_ON_STACK
-   addq$-(15*8), %rsp
-   .endm
-
-   .macro SAVE_AND_CLEAR_REGS offset=0
-   /*
-* Save registers and sanitize registers of values that a
-* speculation attack might otherwise want to exploit. The
-* lower registers are likely clobbered well before they
-* could be put to use in a speculative execution gadget.
-* Interleave XOR with PUSH for better uop scheduling:
-*/
-   movq %rdi, 14*8+\offset(%rsp)
-   movq %rsi, 13*8+\offset(%rsp)
-   movq %rdx, 12*8+\offset(%rsp)
-   movq %rcx, 11*8+\offset(%rsp)
-   movq %rax, 10*8+\offset(%rsp)
-   movq %r8,  9*8+\offset(%rsp)
-   xorq %r8, %r8   /* nospec r8 */
-   movq %r9,  8*8+\offset(%rsp)
-   xorq %r9, %r9   /* nospec r9 */
-   movq %r10, 7*8+\offset(%rsp)
-   xorq %r10, %r10 /* nospec r10 */
-   movq %r11, 6*8+\offset(%rsp)
-   xorq %r11, %r11 /* nospec r11 */
-   movq %rbx, 5*8+\offset(%rsp)
-   xorl %ebx, %ebx /* nospec rbx */
-   movq %rbp, 4*8+\offset(%rsp)
-   xorl %ebp, %ebp /* nospec rbp */
-   movq %r12, 3*8+\offset(%rsp)
-   xorq %r12, %r12 /* nospec r12 */
-   movq %r13, 2*8+\offset(%rsp)
-   xorq %r13, %r13 /* nospec r13 */
-   movq %r14, 1*8+\offset(%rsp)
-   xorq %r14, %r14 /* nospec r14 */
-   movq %r15, 0*8+\offset(%rsp)
-   xorq %r15, %r15 /* nospec r15 */
-   UNWIND_HINT_REGS offset=\offset
-   .endm
-
-   .macro PUSH_AND_CLEAR_REGS rdx=%rdx rax=%rax
+   .macro PUSH_AND_CLEAR_REGS rdx=%rdx rax=%rax save_ret=0
/*
 * Push registers and sanitize registers of values that a
 * speculation attack might otherwise want to exploit. The
@@ -145,8 +105,14 @@ For 32-bit we have the following conventions - kernel is 
built with
 * could be put to use in a speculative execution gadget.
 * Interleave XOR with PUSH for better uop scheduling:
 */
+   .if \save_ret
+   pushq   %rsi/* pt_regs->si */
+   movq8(%rsp), %rsi   /* temporarily store ret address in %rsi */
+   movq%rdi, 8(%rsp)   /* pt_regs->di (overwriting original ret) */
+   .else
pushq   %rdi/* pt_regs->di */
pushq   %rsi/* pt_regs->si */
+   .endif
pushq   \rdx/* pt_regs->dx */
pushq   %rcx/* pt_regs->cx */
pushq   \rax/* pt_regs->ax */
@@ -171,6 +137,9 @@ For 32-bit we have the following conventions - kernel is 
built with
pushq   %r15/* pt_regs->r15 */
xorq%r15, %r15  /* nospec   r15*/
UNWIND_HINT_REGS
+   .if \save_ret
+   pushq   %rsi/* return address on top of stack */
+   .endif
.endm
 
.macro POP_REGS pop_rdi=1 skip_r11rcx=0
@@ -211,7 +180,7 @@ For 32-bit we have the following conventions - kernel is 
built with
  * is just setting the LSB, which makes it an invalid stack address and is also
  * a signal to the unwinder that it's a pt_regs pointer 

Re: [PATCH BUGFIX V3] block, bfq: add requeue-request hook

[Paolo Valente]

> Il giorno 07 feb 2018, alle ore 23:18, Jens Axboe  ha 
> scritto:
> 
> On 2/7/18 2:19 PM, Paolo Valente wrote:
>> Commit 'a6a252e64914 ("blk-mq-sched: decide how to handle flush rq via
>> RQF_FLUSH_SEQ")' makes all non-flush re-prepared requests for a device
>> be re-inserted into the active I/O scheduler for that device. As a
>> consequence, I/O schedulers may get the same request inserted again,
>> even several times, without a finish_request invoked on that request
>> before each re-insertion.
>> 
>> This fact is the cause of the failure reported in [1]. For an I/O
>> scheduler, every re-insertion of the same re-prepared request is
>> equivalent to the insertion of a new request. For schedulers like
>> mq-deadline or kyber, this fact causes no harm. In contrast, it
>> confuses a stateful scheduler like BFQ, which keeps state for an I/O
>> request, until the finish_request hook is invoked on the request. In
>> particular, BFQ may get stuck, waiting forever for the number of
>> request dispatches, of the same request, to be balanced by an equal
>> number of request completions (while there will be one completion for
>> that request). In this state, BFQ may refuse to serve I/O requests
>> from other bfq_queues. The hang reported in [1] then follows.
>> 
>> However, the above re-prepared requests undergo a requeue, thus the
>> requeue_request hook of the active elevator is invoked for these
>> requests, if set. This commit then addresses the above issue by
>> properly implementing the hook requeue_request in BFQ.
> 
> Thanks, applied.
> 

I Jens,
I forgot to add
Tested-by: Oleksandr Natalenko 
in the patch.

Is it still possible to add it?

Thanks,
Paolo

> -- 
> Jens Axboe

Re: [PATCH BUGFIX V3] block, bfq: add requeue-request hook

[Paolo Valente]

> Il giorno 07 feb 2018, alle ore 23:18, Jens Axboe  ha 
> scritto:
> 
> On 2/7/18 2:19 PM, Paolo Valente wrote:
>> Commit 'a6a252e64914 ("blk-mq-sched: decide how to handle flush rq via
>> RQF_FLUSH_SEQ")' makes all non-flush re-prepared requests for a device
>> be re-inserted into the active I/O scheduler for that device. As a
>> consequence, I/O schedulers may get the same request inserted again,
>> even several times, without a finish_request invoked on that request
>> before each re-insertion.
>> 
>> This fact is the cause of the failure reported in [1]. For an I/O
>> scheduler, every re-insertion of the same re-prepared request is
>> equivalent to the insertion of a new request. For schedulers like
>> mq-deadline or kyber, this fact causes no harm. In contrast, it
>> confuses a stateful scheduler like BFQ, which keeps state for an I/O
>> request, until the finish_request hook is invoked on the request. In
>> particular, BFQ may get stuck, waiting forever for the number of
>> request dispatches, of the same request, to be balanced by an equal
>> number of request completions (while there will be one completion for
>> that request). In this state, BFQ may refuse to serve I/O requests
>> from other bfq_queues. The hang reported in [1] then follows.
>> 
>> However, the above re-prepared requests undergo a requeue, thus the
>> requeue_request hook of the active elevator is invoked for these
>> requests, if set. This commit then addresses the above issue by
>> properly implementing the hook requeue_request in BFQ.
> 
> Thanks, applied.
> 

I Jens,
I forgot to add
Tested-by: Oleksandr Natalenko 
in the patch.

Is it still possible to add it?

Thanks,
Paolo

> -- 
> Jens Axboe

Re: [VDSO]: vdso_test failing on arm 32 bit

[Pintu Kumar]

Dear Nathan & Russel,

I have few more question about vdso for arm-32-bit.

I am using iMX7 board to very VDSO gettimeofday timing.

To make kernel/Documentation/vDSO/vdsotest work on this board, I
commented the device tree reading property:
arm,cpu-registers-not-fw-configured , from the :
arch/arm/kernel/vdso.c
[However, I havent removed the property from device-tree itself, so it
is still being read by: drivers/clocksource/arm_arch_timer.c ⇒
arch_timer_of_init(…)]

But it seems the VDSO gettimeofday call is rather very slow.
This is the result of 10 iteration (with 10 seconds delay between
each), after fresh reboot:
bash-3.2# ./run_vdso.sh
The time is 1116.898864
===
The time is 1126.957507
===
The time is 1136.988507
===
The time is 1147.023134
===
The time is 1157.053017
===
The time is 1167.082912
===
The time is 1177.117722
===
The time is 1187.150945
===
The time is 1197.180820
===
The time is 1207.215339
===
The time is 1217.245189
===

Although architected timer support is not available in firmware, so it
may not give the accurate timing.

1) So, I wonder why vdso call is very slow and increasing by every second ?

This is the result of benchmark test.

bash-3.2# ./vdsotest_bench gettimeofday bench
gettimeofday: syscall: 681 nsec/call
gettimeofday: libc: 4190 nsec/call
gettimeofday: vdso: 4171 nsec/call

Also, when I call gettimeofday API from a sample program, I could not
find any system call trace.

bash-3.2# time ./test-vdso-syscall.out
real 0m3.997s
user 0m3.980s
sys 0m0.000s


2) So, how do I conclude whether VDSO gettimeofday is working correctly or not ?
3) Is it important to have that device-tree property in VDSO, if
arch-timer support is not available in firmware ?

Please let me know your opinion.


Thank You!
Pintu


On Thu, Dec 14, 2017 at 3:51 PM, Russell King - ARM Linux
 wrote:
> On Thu, Dec 14, 2017 at 10:50:50AM +0530, Pintu Kumar wrote:
>> Oh ok. Thanks for this information.
>> So, in conclusion, can I summarize like this: VDSO support on ARM 32-bit
>> 1) VDSO works only on Cortex A7/A15 -> where generic timer extension
>> is available.
>> 2) VDSO works only on kernel 4.1 and above => where 32-bit vdso
>> support is available
>> 3) glibc version should be 2.20 or higher
>> 4) This device-tree property should not be set:
>> arm,cpu-registers-not-fw-configured => This means there is a firmware
>> bug
>>
>> Sorry, but still I have 2 more queries:
>> 1) Which is the firmware that we are talking about here ? What is the
>> name of firmware ? Is it available in kernel ?
>
> It's not in-kernel firmware, but whatever runs on the kernel prior to
> booting the kernel.
>
>> 2) Is there any ARM 32-bit board available, where I can confirm that
>> VDSO works on ARM 32-bit?
>
> I'm know of none.  That's not to say that there aren't any, I just don't
> know of any.
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up

Re: [VDSO]: vdso_test failing on arm 32 bit

[Pintu Kumar]

Dear Nathan & Russel,

I have few more question about vdso for arm-32-bit.

I am using iMX7 board to very VDSO gettimeofday timing.

To make kernel/Documentation/vDSO/vdsotest work on this board, I
commented the device tree reading property:
arm,cpu-registers-not-fw-configured , from the :
arch/arm/kernel/vdso.c
[However, I havent removed the property from device-tree itself, so it
is still being read by: drivers/clocksource/arm_arch_timer.c ⇒
arch_timer_of_init(…)]

But it seems the VDSO gettimeofday call is rather very slow.
This is the result of 10 iteration (with 10 seconds delay between
each), after fresh reboot:
bash-3.2# ./run_vdso.sh
The time is 1116.898864
===
The time is 1126.957507
===
The time is 1136.988507
===
The time is 1147.023134
===
The time is 1157.053017
===
The time is 1167.082912
===
The time is 1177.117722
===
The time is 1187.150945
===
The time is 1197.180820
===
The time is 1207.215339
===
The time is 1217.245189
===

Although architected timer support is not available in firmware, so it
may not give the accurate timing.

1) So, I wonder why vdso call is very slow and increasing by every second ?

This is the result of benchmark test.

bash-3.2# ./vdsotest_bench gettimeofday bench
gettimeofday: syscall: 681 nsec/call
gettimeofday: libc: 4190 nsec/call
gettimeofday: vdso: 4171 nsec/call

Also, when I call gettimeofday API from a sample program, I could not
find any system call trace.

bash-3.2# time ./test-vdso-syscall.out
real 0m3.997s
user 0m3.980s
sys 0m0.000s


2) So, how do I conclude whether VDSO gettimeofday is working correctly or not ?
3) Is it important to have that device-tree property in VDSO, if
arch-timer support is not available in firmware ?

Please let me know your opinion.


Thank You!
Pintu


On Thu, Dec 14, 2017 at 3:51 PM, Russell King - ARM Linux
 wrote:
> On Thu, Dec 14, 2017 at 10:50:50AM +0530, Pintu Kumar wrote:
>> Oh ok. Thanks for this information.
>> So, in conclusion, can I summarize like this: VDSO support on ARM 32-bit
>> 1) VDSO works only on Cortex A7/A15 -> where generic timer extension
>> is available.
>> 2) VDSO works only on kernel 4.1 and above => where 32-bit vdso
>> support is available
>> 3) glibc version should be 2.20 or higher
>> 4) This device-tree property should not be set:
>> arm,cpu-registers-not-fw-configured => This means there is a firmware
>> bug
>>
>> Sorry, but still I have 2 more queries:
>> 1) Which is the firmware that we are talking about here ? What is the
>> name of firmware ? Is it available in kernel ?
>
> It's not in-kernel firmware, but whatever runs on the kernel prior to
> booting the kernel.
>
>> 2) Is there any ARM 32-bit board available, where I can confirm that
>> VDSO works on ARM 32-bit?
>
> I'm know of none.  That's not to say that there aren't any, I just don't
> know of any.
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH v2 BUGFIX] ARM: dts: imx6ull: fix the imx6ull-14x14-evk configuration

[Shawn Guo]

On Fri, Jan 26, 2018 at 09:52:18AM +0100, Lothar Waßmann wrote:
> imx6ull-14x14-evk.dts currently includes the imx6ul.dtsi file for an
> i.MX6ULL SoC which is plain wrong.
> 
> Rename the current imx6ul-14x14-evk.dts to .dtsi and include it from
> imx6ul-14x14-evk.dts and imx6ull-14x14-evk.dts, so that both can
> include the appropriate SoC specific (imx6ul.dtsi/imx6ull.dtsi) file.
> 
> Signed-off-by: Lothar Waßmann 

Applied, thanks.

Re: [PATCH v2 BUGFIX] ARM: dts: imx6ull: fix the imx6ull-14x14-evk configuration

[Shawn Guo]

On Fri, Jan 26, 2018 at 09:52:18AM +0100, Lothar Waßmann wrote:
> imx6ull-14x14-evk.dts currently includes the imx6ul.dtsi file for an
> i.MX6ULL SoC which is plain wrong.
> 
> Rename the current imx6ul-14x14-evk.dts to .dtsi and include it from
> imx6ul-14x14-evk.dts and imx6ull-14x14-evk.dts, so that both can
> include the appropriate SoC specific (imx6ul.dtsi/imx6ull.dtsi) file.
> 
> Signed-off-by: Lothar Waßmann 

Applied, thanks.

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 7:51 AM, Ulf Magnusson  wrote:
> On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson  wrote:
>> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
>>  wrote:
>>> If stdio is not tty, conf_askvalue() puts additional new line to
>>> prevent prompts are all concatenated into a single line.  This care
>>> is missing in conf_choice(), so a 'choice' prompt and the next prompt
>>> are shown in the same line.
>>>
>>> Move the code into xfgets() to take care of all cases.  To improve
>>> this more, echo stdin to stdout.  This clarifies what keys were input
>>> from stdio and the stdout looks like as if it were from tty.
>>>
>>> I removed the isatty(2) check since stderr is unrelated here.
>>>
>>> Signed-off-by: Masahiro Yamada 
>>> ---
>>>
>>>  scripts/kconfig/conf.c | 7 ---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>>> index 358e2e4..c5318d3 100644
>>> --- a/scripts/kconfig/conf.c
>>> +++ b/scripts/kconfig/conf.c
>>> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>>>  {
>>> if (!fgets(str, size, in))
>>> fprintf(stderr, "\nError in reading or end of file.\n");
>>> +
>>> +   if (!tty_stdio)
>>> +   printf("%s", str);
>>>  }
>>>
>>>  static int conf_askvalue(struct symbol *sym, const char *def)
>>> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
>>> *def)
>>> case oldaskconfig:
>>> fflush(stdout);
>>> xfgets(line, sizeof(line), stdin);
>>> -   if (!tty_stdio)
>>> -   printf("\n");
>>> return 1;
>>> default:
>>> break;
>>> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>>> bindtextdomain(PACKAGE, LOCALEDIR);
>>> textdomain(PACKAGE);
>>>
>>> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
>>> +   tty_stdio = isatty(0) && isatty(1);
>>>
>>> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>>> if (opt == 's') {
>>> --
>>> 2.7.4
>>>
>>
>> Reviewed-by: Ulf Magnusson 
>>
>> Might be some case I'm not thinking of, but wouldn't it make sense to
>> just check isatty(1) as well? If stdout is a regular file, it seems
>> it'd be nice to have the input appear there, regardless of where stdin
>> is from.
>>
>> Maybe the tty_stdio variable could be removed then as well, replaced
>> with just 'if (!isatty(1))'.
>>
>> Cheers,
>> Ulf
>
> Hmm... if stdout is a tty and stdin isn't, this would prevent the
> input from showing up on the terminal though, so I guess it makes
> sense. That case seems more important than the weird
> stdin=tty/stdout=file case.
>
> Tricky stuff. :)
>
> Cheers,
> Ulf

Oh... and that one's already taken care of too, reading closer.

stdin | stdout | wants stdin written to stdout
--++-
tty   | tty| no (already echoed by tty)
tty   | file   | yes (echoed by tty, written to file)
file  | tty| yes (not echoed by tty)
file  | file   | yes

So !(tty(0) && tty(1)) makes sense. Excuse the rambling...

Cheers,
Ulf

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 7:51 AM, Ulf Magnusson  wrote:
> On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson  wrote:
>> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
>>  wrote:
>>> If stdio is not tty, conf_askvalue() puts additional new line to
>>> prevent prompts are all concatenated into a single line.  This care
>>> is missing in conf_choice(), so a 'choice' prompt and the next prompt
>>> are shown in the same line.
>>>
>>> Move the code into xfgets() to take care of all cases.  To improve
>>> this more, echo stdin to stdout.  This clarifies what keys were input
>>> from stdio and the stdout looks like as if it were from tty.
>>>
>>> I removed the isatty(2) check since stderr is unrelated here.
>>>
>>> Signed-off-by: Masahiro Yamada 
>>> ---
>>>
>>>  scripts/kconfig/conf.c | 7 ---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>>> index 358e2e4..c5318d3 100644
>>> --- a/scripts/kconfig/conf.c
>>> +++ b/scripts/kconfig/conf.c
>>> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>>>  {
>>> if (!fgets(str, size, in))
>>> fprintf(stderr, "\nError in reading or end of file.\n");
>>> +
>>> +   if (!tty_stdio)
>>> +   printf("%s", str);
>>>  }
>>>
>>>  static int conf_askvalue(struct symbol *sym, const char *def)
>>> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
>>> *def)
>>> case oldaskconfig:
>>> fflush(stdout);
>>> xfgets(line, sizeof(line), stdin);
>>> -   if (!tty_stdio)
>>> -   printf("\n");
>>> return 1;
>>> default:
>>> break;
>>> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>>> bindtextdomain(PACKAGE, LOCALEDIR);
>>> textdomain(PACKAGE);
>>>
>>> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
>>> +   tty_stdio = isatty(0) && isatty(1);
>>>
>>> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>>> if (opt == 's') {
>>> --
>>> 2.7.4
>>>
>>
>> Reviewed-by: Ulf Magnusson 
>>
>> Might be some case I'm not thinking of, but wouldn't it make sense to
>> just check isatty(1) as well? If stdout is a regular file, it seems
>> it'd be nice to have the input appear there, regardless of where stdin
>> is from.
>>
>> Maybe the tty_stdio variable could be removed then as well, replaced
>> with just 'if (!isatty(1))'.
>>
>> Cheers,
>> Ulf
>
> Hmm... if stdout is a tty and stdin isn't, this would prevent the
> input from showing up on the terminal though, so I guess it makes
> sense. That case seems more important than the weird
> stdin=tty/stdout=file case.
>
> Tricky stuff. :)
>
> Cheers,
> Ulf

Oh... and that one's already taken care of too, reading closer.

stdin | stdout | wants stdin written to stdout
--++-
tty   | tty| no (already echoed by tty)
tty   | file   | yes (echoed by tty, written to file)
file  | tty| yes (not echoed by tty)
file  | file   | yes

So !(tty(0) && tty(1)) makes sense. Excuse the rambling...

Cheers,
Ulf

Re: linux-next: manual merge of the tip tree with Linus' tree

[Ingo Molnar]

* Will Deacon  wrote:

> For the sake of avoiding the conflict, can we just drop it for now, please?

Yeah, so I resolved the conflict by merging the (already upstream) bits and 
Linus 
pulled that resolution. From now on the level of comments you want there is up 
to 
you! :-)

Thanks,

Ingo

Re: linux-next: manual merge of the tip tree with Linus' tree

[Ingo Molnar]

* Will Deacon  wrote:

> For the sake of avoiding the conflict, can we just drop it for now, please?

Yeah, so I resolved the conflict by merging the (already upstream) bits and 
Linus 
pulled that resolution. From now on the level of comments you want there is up 
to 
you! :-)

Thanks,

Ingo

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Masahiro Yamada]

2018-02-08 15:51 GMT+09:00 Ulf Magnusson :
> On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson  wrote:
>> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
>>  wrote:
>>> If stdio is not tty, conf_askvalue() puts additional new line to
>>> prevent prompts are all concatenated into a single line.  This care
>>> is missing in conf_choice(), so a 'choice' prompt and the next prompt
>>> are shown in the same line.
>>>
>>> Move the code into xfgets() to take care of all cases.  To improve
>>> this more, echo stdin to stdout.  This clarifies what keys were input
>>> from stdio and the stdout looks like as if it were from tty.
>>>
>>> I removed the isatty(2) check since stderr is unrelated here.
>>>
>>> Signed-off-by: Masahiro Yamada 
>>> ---
>>>
>>>  scripts/kconfig/conf.c | 7 ---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>>> index 358e2e4..c5318d3 100644
>>> --- a/scripts/kconfig/conf.c
>>> +++ b/scripts/kconfig/conf.c
>>> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>>>  {
>>> if (!fgets(str, size, in))
>>> fprintf(stderr, "\nError in reading or end of file.\n");
>>> +
>>> +   if (!tty_stdio)
>>> +   printf("%s", str);
>>>  }
>>>
>>>  static int conf_askvalue(struct symbol *sym, const char *def)
>>> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
>>> *def)
>>> case oldaskconfig:
>>> fflush(stdout);
>>> xfgets(line, sizeof(line), stdin);
>>> -   if (!tty_stdio)
>>> -   printf("\n");
>>> return 1;
>>> default:
>>> break;
>>> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>>> bindtextdomain(PACKAGE, LOCALEDIR);
>>> textdomain(PACKAGE);
>>>
>>> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
>>> +   tty_stdio = isatty(0) && isatty(1);
>>>
>>> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>>> if (opt == 's') {
>>> --
>>> 2.7.4
>>>
>>
>> Reviewed-by: Ulf Magnusson 
>>
>> Might be some case I'm not thinking of, but wouldn't it make sense to
>> just check isatty(1) as well? If stdout is a regular file, it seems
>> it'd be nice to have the input appear there, regardless of where stdin
>> is from.
>>
>> Maybe the tty_stdio variable could be removed then as well, replaced
>> with just 'if (!isatty(1))'.
>>
>> Cheers,
>> Ulf
>
> Hmm... if stdout is a tty and stdin isn't, this would prevent the
> input from showing up on the terminal though, so I guess it makes
> sense.

Yes.  I want to address this case too.

Anyway, thank you for checking the details!




> That case seems more important than the weird
> stdin=tty/stdout=file case.
>
> Tricky stuff. :)
>
> Cheers,
> Ulf
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Masahiro Yamada]

2018-02-08 15:51 GMT+09:00 Ulf Magnusson :
> On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson  wrote:
>> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
>>  wrote:
>>> If stdio is not tty, conf_askvalue() puts additional new line to
>>> prevent prompts are all concatenated into a single line.  This care
>>> is missing in conf_choice(), so a 'choice' prompt and the next prompt
>>> are shown in the same line.
>>>
>>> Move the code into xfgets() to take care of all cases.  To improve
>>> this more, echo stdin to stdout.  This clarifies what keys were input
>>> from stdio and the stdout looks like as if it were from tty.
>>>
>>> I removed the isatty(2) check since stderr is unrelated here.
>>>
>>> Signed-off-by: Masahiro Yamada 
>>> ---
>>>
>>>  scripts/kconfig/conf.c | 7 ---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>>> index 358e2e4..c5318d3 100644
>>> --- a/scripts/kconfig/conf.c
>>> +++ b/scripts/kconfig/conf.c
>>> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>>>  {
>>> if (!fgets(str, size, in))
>>> fprintf(stderr, "\nError in reading or end of file.\n");
>>> +
>>> +   if (!tty_stdio)
>>> +   printf("%s", str);
>>>  }
>>>
>>>  static int conf_askvalue(struct symbol *sym, const char *def)
>>> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
>>> *def)
>>> case oldaskconfig:
>>> fflush(stdout);
>>> xfgets(line, sizeof(line), stdin);
>>> -   if (!tty_stdio)
>>> -   printf("\n");
>>> return 1;
>>> default:
>>> break;
>>> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>>> bindtextdomain(PACKAGE, LOCALEDIR);
>>> textdomain(PACKAGE);
>>>
>>> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
>>> +   tty_stdio = isatty(0) && isatty(1);
>>>
>>> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>>> if (opt == 's') {
>>> --
>>> 2.7.4
>>>
>>
>> Reviewed-by: Ulf Magnusson 
>>
>> Might be some case I'm not thinking of, but wouldn't it make sense to
>> just check isatty(1) as well? If stdout is a regular file, it seems
>> it'd be nice to have the input appear there, regardless of where stdin
>> is from.
>>
>> Maybe the tty_stdio variable could be removed then as well, replaced
>> with just 'if (!isatty(1))'.
>>
>> Cheers,
>> Ulf
>
> Hmm... if stdout is a tty and stdin isn't, this would prevent the
> input from showing up on the terminal though, so I guess it makes
> sense.

Yes.  I want to address this case too.

Anyway, thank you for checking the details!




> That case seems more important than the weird
> stdin=tty/stdout=file case.
>
> Tricky stuff. :)
>
> Cheers,
> Ulf
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson  wrote:
> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
>  wrote:
>> If stdio is not tty, conf_askvalue() puts additional new line to
>> prevent prompts are all concatenated into a single line.  This care
>> is missing in conf_choice(), so a 'choice' prompt and the next prompt
>> are shown in the same line.
>>
>> Move the code into xfgets() to take care of all cases.  To improve
>> this more, echo stdin to stdout.  This clarifies what keys were input
>> from stdio and the stdout looks like as if it were from tty.
>>
>> I removed the isatty(2) check since stderr is unrelated here.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>>  scripts/kconfig/conf.c | 7 ---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 358e2e4..c5318d3 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>>  {
>> if (!fgets(str, size, in))
>> fprintf(stderr, "\nError in reading or end of file.\n");
>> +
>> +   if (!tty_stdio)
>> +   printf("%s", str);
>>  }
>>
>>  static int conf_askvalue(struct symbol *sym, const char *def)
>> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
>> *def)
>> case oldaskconfig:
>> fflush(stdout);
>> xfgets(line, sizeof(line), stdin);
>> -   if (!tty_stdio)
>> -   printf("\n");
>> return 1;
>> default:
>> break;
>> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>> bindtextdomain(PACKAGE, LOCALEDIR);
>> textdomain(PACKAGE);
>>
>> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
>> +   tty_stdio = isatty(0) && isatty(1);
>>
>> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>> if (opt == 's') {
>> --
>> 2.7.4
>>
>
> Reviewed-by: Ulf Magnusson 
>
> Might be some case I'm not thinking of, but wouldn't it make sense to
> just check isatty(1) as well? If stdout is a regular file, it seems
> it'd be nice to have the input appear there, regardless of where stdin
> is from.
>
> Maybe the tty_stdio variable could be removed then as well, replaced
> with just 'if (!isatty(1))'.
>
> Cheers,
> Ulf

Hmm... if stdout is a tty and stdin isn't, this would prevent the
input from showing up on the terminal though, so I guess it makes
sense. That case seems more important than the weird
stdin=tty/stdout=file case.

Tricky stuff. :)

Cheers,
Ulf

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson  wrote:
> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
>  wrote:
>> If stdio is not tty, conf_askvalue() puts additional new line to
>> prevent prompts are all concatenated into a single line.  This care
>> is missing in conf_choice(), so a 'choice' prompt and the next prompt
>> are shown in the same line.
>>
>> Move the code into xfgets() to take care of all cases.  To improve
>> this more, echo stdin to stdout.  This clarifies what keys were input
>> from stdio and the stdout looks like as if it were from tty.
>>
>> I removed the isatty(2) check since stderr is unrelated here.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>>  scripts/kconfig/conf.c | 7 ---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 358e2e4..c5318d3 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>>  {
>> if (!fgets(str, size, in))
>> fprintf(stderr, "\nError in reading or end of file.\n");
>> +
>> +   if (!tty_stdio)
>> +   printf("%s", str);
>>  }
>>
>>  static int conf_askvalue(struct symbol *sym, const char *def)
>> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
>> *def)
>> case oldaskconfig:
>> fflush(stdout);
>> xfgets(line, sizeof(line), stdin);
>> -   if (!tty_stdio)
>> -   printf("\n");
>> return 1;
>> default:
>> break;
>> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>> bindtextdomain(PACKAGE, LOCALEDIR);
>> textdomain(PACKAGE);
>>
>> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
>> +   tty_stdio = isatty(0) && isatty(1);
>>
>> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>> if (opt == 's') {
>> --
>> 2.7.4
>>
>
> Reviewed-by: Ulf Magnusson 
>
> Might be some case I'm not thinking of, but wouldn't it make sense to
> just check isatty(1) as well? If stdout is a regular file, it seems
> it'd be nice to have the input appear there, regardless of where stdin
> is from.
>
> Maybe the tty_stdio variable could be removed then as well, replaced
> with just 'if (!isatty(1))'.
>
> Cheers,
> Ulf

Hmm... if stdout is a tty and stdin isn't, this would prevent the
input from showing up on the terminal though, so I guess it makes
sense. That case seems more important than the weird
stdin=tty/stdout=file case.

Tricky stuff. :)

Cheers,
Ulf

[PATCH v2 1/2] net, can, ifi: fix "write buffer full" error

[Heiko Schocher]

the driver reads in the ISR first the IRQpending register,
and clears after that in a write *all* bits in it.

It could happen that the isr register raise bits between
this 2 register accesses, which leads in lost bits ...

In case it clears "TX message sent successfully", the driver
never sends any Tx data, and buffers to userspace run over.

Fixed this:
clear only the bits in the IRQpending register, the
driver had read.

Signed-off-by: Heiko Schocher 
Reviewed-by: Marek Vasut 
---

Changes in v2:
- add Reviewed-by from Marek

 drivers/net/can/ifi_canfd/ifi_canfd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c 
b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 2772d05ff11c..05feb8177936 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -607,7 +607,7 @@ static irqreturn_t ifi_canfd_isr(int irq, void *dev_id)
return IRQ_NONE;
 
/* Clear all pending interrupts but ErrWarn */
-   writel(clr_irq_mask, priv->base + IFI_CANFD_INTERRUPT);
+   writel(isr & clr_irq_mask, priv->base + IFI_CANFD_INTERRUPT);
 
/* RX IRQ or bus warning, start NAPI */
if (isr & rx_irq_mask) {
-- 
2.14.3

[PATCH v2 1/2] net, can, ifi: fix "write buffer full" error

[Heiko Schocher]

the driver reads in the ISR first the IRQpending register,
and clears after that in a write *all* bits in it.

It could happen that the isr register raise bits between
this 2 register accesses, which leads in lost bits ...

In case it clears "TX message sent successfully", the driver
never sends any Tx data, and buffers to userspace run over.

Fixed this:
clear only the bits in the IRQpending register, the
driver had read.

Signed-off-by: Heiko Schocher 
Reviewed-by: Marek Vasut 
---

Changes in v2:
- add Reviewed-by from Marek

 drivers/net/can/ifi_canfd/ifi_canfd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c 
b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 2772d05ff11c..05feb8177936 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -607,7 +607,7 @@ static irqreturn_t ifi_canfd_isr(int irq, void *dev_id)
return IRQ_NONE;
 
/* Clear all pending interrupts but ErrWarn */
-   writel(clr_irq_mask, priv->base + IFI_CANFD_INTERRUPT);
+   writel(isr & clr_irq_mask, priv->base + IFI_CANFD_INTERRUPT);
 
/* RX IRQ or bus warning, start NAPI */
if (isr & rx_irq_mask) {
-- 
2.14.3

[PATCH net-queue 2/3] e1000e: Fix queue interrupt re-raising in Other interrupt.

[Benjamin Poirier]

restores the ICS write for rx/tx queue interrupts which was present before
commit 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt",
v4.5-rc1) but was not restored in commit 4aea7a5c5e94 ("e1000e: Avoid
receiver overrun interrupt bursts", v4.15-rc1).

This re-raises the queue interrupts in case the txq or rxq bits were set in
ICR and the Other interrupt handler read and cleared ICR before the queue
interrupt was raised.

Fixes: 4aea7a5c5e94 ("e1000e: Avoid receiver overrun interrupt bursts")
Signed-off-by: Benjamin Poirier 
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 3b36efa6228d..2c9609bee2ae 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1919,6 +1919,9 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
icr = er32(ICR);
ew32(ICR, E1000_ICR_OTHER);
 
+   if (icr & adapter->eiac_mask)
+   ew32(ICS, (icr & adapter->eiac_mask));
+
if (icr & E1000_ICR_LSC) {
ew32(ICR, E1000_ICR_LSC);
hw->mac.get_link_status = true;
-- 
2.16.1

[PATCH v2 2/2] net, can, ifi: loopback Tx message in IFI block

[Heiko Schocher]

Current ifi driver reads first Rx messages, than loopback
the Tx message, if the IFI_CANFD_INTERRUPT_TXFIFO_REMOVE
bit is set. This can lead into the case, that Rx messages
overhelm Tx messages!

Fixed this in the following way:

Set in the IFI_CANFD_TXFIFO_DLC register the FN value to
1, so the IFI block loopsback itself the Tx message when
sended correctly on the canfd bus. Only the IFI block can
insert the Tx message in the correct place.

The linux driver now needs only to free the skb, when
the Tx message was sended correctly.

Signed-off-by: Heiko Schocher 
Reviewed-by: Marek Vasut 
---

Changes in v2:
- add Reviewed-by from Marek, fixed comment into one liner

 drivers/net/can/ifi_canfd/ifi_canfd.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c 
b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 05feb8177936..ee74ee8f9b38 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -211,6 +211,7 @@ struct ifi_canfd_priv {
struct napi_struct  napi;
struct net_device   *ndev;
void __iomem*base;
+   unsigned inttx_len;
 };
 
 static void ifi_canfd_irq_enable(struct net_device *ndev, bool enable)
@@ -617,8 +618,10 @@ static irqreturn_t ifi_canfd_isr(int irq, void *dev_id)
 
/* TX IRQ */
if (isr & IFI_CANFD_INTERRUPT_TXFIFO_REMOVE) {
-   stats->tx_bytes += can_get_echo_skb(ndev, 0);
+   can_free_echo_skb(ndev, 0);
+   stats->tx_bytes += priv->tx_len;
stats->tx_packets++;
+   priv->tx_len = 0;
can_led_event(ndev, CAN_LED_EVENT_TX);
}
 
@@ -889,6 +892,7 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
}
 
txdlc = can_len2dlc(cf->len);
+   priv->tx_len = txdlc;
if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb)) {
txdlc |= IFI_CANFD_TXFIFO_DLC_EDL;
if (cf->flags & CANFD_BRS)
@@ -898,6 +902,9 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
if (cf->can_id & CAN_RTR_FLAG)
txdlc |= IFI_CANFD_TXFIFO_DLC_RTR;
 
+   /* set FNR to 1, so we get our Tx Message looped back into RxFIFO */
+   txdlc += (1 << IFI_CANFD_TXFIFO_DLC_FNR_OFFSET);
+
/* message ram configuration */
writel(txid, priv->base + IFI_CANFD_TXFIFO_ID);
writel(txdlc, priv->base + IFI_CANFD_TXFIFO_DLC);
-- 
2.14.3

[PATCH net-queue 2/3] e1000e: Fix queue interrupt re-raising in Other interrupt.

[Benjamin Poirier]

restores the ICS write for rx/tx queue interrupts which was present before
commit 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt",
v4.5-rc1) but was not restored in commit 4aea7a5c5e94 ("e1000e: Avoid
receiver overrun interrupt bursts", v4.15-rc1).

This re-raises the queue interrupts in case the txq or rxq bits were set in
ICR and the Other interrupt handler read and cleared ICR before the queue
interrupt was raised.

Fixes: 4aea7a5c5e94 ("e1000e: Avoid receiver overrun interrupt bursts")
Signed-off-by: Benjamin Poirier 
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 3b36efa6228d..2c9609bee2ae 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1919,6 +1919,9 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
icr = er32(ICR);
ew32(ICR, E1000_ICR_OTHER);
 
+   if (icr & adapter->eiac_mask)
+   ew32(ICS, (icr & adapter->eiac_mask));
+
if (icr & E1000_ICR_LSC) {
ew32(ICR, E1000_ICR_LSC);
hw->mac.get_link_status = true;
-- 
2.16.1

[PATCH v2 2/2] net, can, ifi: loopback Tx message in IFI block

[Heiko Schocher]

Current ifi driver reads first Rx messages, than loopback
the Tx message, if the IFI_CANFD_INTERRUPT_TXFIFO_REMOVE
bit is set. This can lead into the case, that Rx messages
overhelm Tx messages!

Fixed this in the following way:

Set in the IFI_CANFD_TXFIFO_DLC register the FN value to
1, so the IFI block loopsback itself the Tx message when
sended correctly on the canfd bus. Only the IFI block can
insert the Tx message in the correct place.

The linux driver now needs only to free the skb, when
the Tx message was sended correctly.

Signed-off-by: Heiko Schocher 
Reviewed-by: Marek Vasut 
---

Changes in v2:
- add Reviewed-by from Marek, fixed comment into one liner

 drivers/net/can/ifi_canfd/ifi_canfd.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c 
b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 05feb8177936..ee74ee8f9b38 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -211,6 +211,7 @@ struct ifi_canfd_priv {
struct napi_struct  napi;
struct net_device   *ndev;
void __iomem*base;
+   unsigned inttx_len;
 };
 
 static void ifi_canfd_irq_enable(struct net_device *ndev, bool enable)
@@ -617,8 +618,10 @@ static irqreturn_t ifi_canfd_isr(int irq, void *dev_id)
 
/* TX IRQ */
if (isr & IFI_CANFD_INTERRUPT_TXFIFO_REMOVE) {
-   stats->tx_bytes += can_get_echo_skb(ndev, 0);
+   can_free_echo_skb(ndev, 0);
+   stats->tx_bytes += priv->tx_len;
stats->tx_packets++;
+   priv->tx_len = 0;
can_led_event(ndev, CAN_LED_EVENT_TX);
}
 
@@ -889,6 +892,7 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
}
 
txdlc = can_len2dlc(cf->len);
+   priv->tx_len = txdlc;
if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb)) {
txdlc |= IFI_CANFD_TXFIFO_DLC_EDL;
if (cf->flags & CANFD_BRS)
@@ -898,6 +902,9 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
if (cf->can_id & CAN_RTR_FLAG)
txdlc |= IFI_CANFD_TXFIFO_DLC_RTR;
 
+   /* set FNR to 1, so we get our Tx Message looped back into RxFIFO */
+   txdlc += (1 << IFI_CANFD_TXFIFO_DLC_FNR_OFFSET);
+
/* message ram configuration */
writel(txid, priv->base + IFI_CANFD_TXFIFO_ID);
writel(txdlc, priv->base + IFI_CANFD_TXFIFO_DLC);
-- 
2.14.3

Re: [PATCH v2 1/2] soc: imx: gpcv2: Do not pass static memory as platform data

[Shawn Guo]

On Mon, Jan 22, 2018 at 07:07:47AM -0800, Andrey Smirnov wrote:
> Platform device core assumes the ownership of dev.platform_data as
> well as that it is dynamically allocated and it will try to kfree it
> as a part of platform_device_release(). Change the code to pass
> kzalloc'ed chunk of memory instead of a pointer to a static memory to
> avoid causing a BUG() when calling platform_device_put().
> 
> The problem can be reproduced by artificially enabling the error path
> of platform_device_add() call (around line 357).
> 
> Note that this change also allows us to constify imx7_pgc_domains,
> since we no longer need to be able to modify it.
> 
> Cc: Shawn Guo 
> Cc: Stefan Agner 
> Cc: Lucas Stach 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov 

Applied both, thanks.

Re: [PATCH v2 1/2] soc: imx: gpcv2: Do not pass static memory as platform data

[Shawn Guo]

On Mon, Jan 22, 2018 at 07:07:47AM -0800, Andrey Smirnov wrote:
> Platform device core assumes the ownership of dev.platform_data as
> well as that it is dynamically allocated and it will try to kfree it
> as a part of platform_device_release(). Change the code to pass
> kzalloc'ed chunk of memory instead of a pointer to a static memory to
> avoid causing a BUG() when calling platform_device_put().
> 
> The problem can be reproduced by artificially enabling the error path
> of platform_device_add() call (around line 357).
> 
> Note that this change also allows us to constify imx7_pgc_domains,
> since we no longer need to be able to modify it.
> 
> Cc: Shawn Guo 
> Cc: Stefan Agner 
> Cc: Lucas Stach 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov 

Applied both, thanks.

[PATCH net-queue 1/3] Partial revert "e1000e: Avoid receiver overrun interrupt bursts"

[Benjamin Poirier]

This partially reverts commit 4aea7a5c5e940c1723add439f4088844cd26196d.

We keep the fix for the first part of the problem (1) described in the log
of that commit, that is to read ICR in the other interrupt handler. We
remove the fix for the second part of the problem (2), Other interrupt
throttling.

Bursts of "Other" interrupts may once again occur during rxo (receive
overflow) traffic conditions. This is deemed acceptable in the interest of
avoiding unforeseen fallout from changes that are not strictly necessary.
As discussed, the e1000e driver should be in "maintenance mode".

Link: https://www.spinics.net/lists/netdev/msg480675.html
Signed-off-by: Benjamin Poirier 
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 153ad406c65e..3b36efa6228d 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1915,21 +1915,10 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = >hw;
u32 icr;
-   bool enable = true;
 
icr = er32(ICR);
ew32(ICR, E1000_ICR_OTHER);
 
-   if (icr & E1000_ICR_RXO) {
-   ew32(ICR, E1000_ICR_RXO);
-   enable = false;
-   /* napi poll will re-enable Other, make sure it runs */
-   if (napi_schedule_prep(>napi)) {
-   adapter->total_rx_bytes = 0;
-   adapter->total_rx_packets = 0;
-   __napi_schedule(>napi);
-   }
-   }
if (icr & E1000_ICR_LSC) {
ew32(ICR, E1000_ICR_LSC);
hw->mac.get_link_status = true;
@@ -1938,7 +1927,7 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
mod_timer(>watchdog_timer, jiffies + 1);
}
 
-   if (enable && !test_bit(__E1000_DOWN, >state))
+   if (!test_bit(__E1000_DOWN, >state))
ew32(IMS, E1000_IMS_OTHER);
 
return IRQ_HANDLED;
@@ -2708,8 +2697,7 @@ static int e1000e_poll(struct napi_struct *napi, int 
weight)
napi_complete_done(napi, work_done);
if (!test_bit(__E1000_DOWN, >state)) {
if (adapter->msix_entries)
-   ew32(IMS, adapter->rx_ring->ims_val |
-E1000_IMS_OTHER);
+   ew32(IMS, adapter->rx_ring->ims_val);
else
e1000_irq_enable(adapter);
}
-- 
2.16.1

[PATCH net-queue 3/3] e1000e: Avoid missed interrupts following ICR read.

[Benjamin Poirier]

The 82574 specification update errata 12 states that interrupts may be
missed if ICR is read while INT_ASSERTED is not set. Avoid that problem by
setting all bits related to events that can trigger the Other interrupt in
IMS.

The Other interrupt is raised for such events regardless of whether or not
they are set in IMS. However, only when they are set is the INT_ASSERTED
bit also set in ICR.

By doing this, we ensure that INT_ASSERTED is always set when we read ICR
in e1000_msix_other() and steer clear of the errata. This also ensures that
ICR will automatically be cleared on read, therefore we no longer need to
clear bits explicitly.

Signed-off-by: Benjamin Poirier 
---
 drivers/net/ethernet/intel/e1000e/defines.h | 21 -
 drivers/net/ethernet/intel/e1000e/netdev.c  | 11 ---
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/defines.h 
b/drivers/net/ethernet/intel/e1000e/defines.h
index afb7ebe20b24..824fd44e25f0 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -400,6 +400,10 @@
 #define E1000_ICR_RXDMT00x0010 /* Rx desc min. threshold (0) */
 #define E1000_ICR_RXO   0x0040 /* Receiver Overrun */
 #define E1000_ICR_RXT0  0x0080 /* Rx timer intr (ring 0) */
+#define E1000_ICR_MDAC  0x0200 /* MDIO Access Complete */
+#define E1000_ICR_SRPD  0x0001 /* Small Receive Packet Detected */
+#define E1000_ICR_ACK   0x0002 /* Receive ACK Frame Detected */
+#define E1000_ICR_MNG   0x0004 /* Manageability Event Detected */
 #define E1000_ICR_ECCER 0x0040 /* Uncorrectable ECC Error */
 /* If this bit asserted, the driver should claim the interrupt */
 #define E1000_ICR_INT_ASSERTED 0x8000
@@ -407,7 +411,7 @@
 #define E1000_ICR_RXQ1  0x0020 /* Rx Queue 1 Interrupt */
 #define E1000_ICR_TXQ0  0x0040 /* Tx Queue 0 Interrupt */
 #define E1000_ICR_TXQ1  0x0080 /* Tx Queue 1 Interrupt */
-#define E1000_ICR_OTHER 0x0100 /* Other Interrupts */
+#define E1000_ICR_OTHER 0x0100 /* Other Interrupt */
 
 /* PBA ECC Register */
 #define E1000_PBA_ECC_COUNTER_MASK  0xFFF0 /* ECC counter mask */
@@ -431,12 +435,27 @@
E1000_IMS_RXSEQ  |\
E1000_IMS_LSC)
 
+/* These are all of the events related to the OTHER interrupt.
+ */
+#define IMS_OTHER_MASK ( \
+   E1000_IMS_LSC  | \
+   E1000_IMS_RXO  | \
+   E1000_IMS_MDAC | \
+   E1000_IMS_SRPD | \
+   E1000_IMS_ACK  | \
+   E1000_IMS_MNG)
+
 /* Interrupt Mask Set */
 #define E1000_IMS_TXDW  E1000_ICR_TXDW  /* Transmit desc written back 
*/
 #define E1000_IMS_LSC   E1000_ICR_LSC   /* Link Status Change */
 #define E1000_IMS_RXSEQ E1000_ICR_RXSEQ /* Rx sequence error */
 #define E1000_IMS_RXDMT0E1000_ICR_RXDMT0/* Rx desc min. threshold */
+#define E1000_IMS_RXO   E1000_ICR_RXO   /* Receiver Overrun */
 #define E1000_IMS_RXT0  E1000_ICR_RXT0  /* Rx timer intr */
+#define E1000_IMS_MDAC  E1000_ICR_MDAC  /* MDIO Access Complete */
+#define E1000_IMS_SRPD  E1000_ICR_SRPD  /* Small Receive Packet */
+#define E1000_IMS_ACK   E1000_ICR_ACK   /* Receive ACK Frame Detected 
*/
+#define E1000_IMS_MNG   E1000_ICR_MNG   /* Manageability Event */
 #define E1000_IMS_ECCER E1000_ICR_ECCER /* Uncorrectable ECC Error */
 #define E1000_IMS_RXQ0  E1000_ICR_RXQ0  /* Rx Queue 0 Interrupt */
 #define E1000_IMS_RXQ1  E1000_ICR_RXQ1  /* Rx Queue 1 Interrupt */
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 2c9609bee2ae..9fd4050a91ca 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1914,16 +1914,12 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
struct net_device *netdev = data;
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = >hw;
-   u32 icr;
-
-   icr = er32(ICR);
-   ew32(ICR, E1000_ICR_OTHER);
+   u32 icr = er32(ICR);
 
if (icr & adapter->eiac_mask)
ew32(ICS, (icr & adapter->eiac_mask));
 
if (icr & E1000_ICR_LSC) {
-   ew32(ICR, E1000_ICR_LSC);
hw->mac.get_link_status = true;
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, >state))
@@ -1931,7 +1927,7 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
}
 
if (!test_bit(__E1000_DOWN, >state))
-   ew32(IMS, E1000_IMS_OTHER);
+   ew32(IMS, E1000_IMS_OTHER | IMS_OTHER_MASK);
 
return IRQ_HANDLED;
 }
@@ -2258,7 +2254,8 @@ static void e1000_irq_enable(struct e1000_adapter 
*adapter)
 
if 

[PATCH net-queue 1/3] Partial revert "e1000e: Avoid receiver overrun interrupt bursts"

[Benjamin Poirier]

This partially reverts commit 4aea7a5c5e940c1723add439f4088844cd26196d.

We keep the fix for the first part of the problem (1) described in the log
of that commit, that is to read ICR in the other interrupt handler. We
remove the fix for the second part of the problem (2), Other interrupt
throttling.

Bursts of "Other" interrupts may once again occur during rxo (receive
overflow) traffic conditions. This is deemed acceptable in the interest of
avoiding unforeseen fallout from changes that are not strictly necessary.
As discussed, the e1000e driver should be in "maintenance mode".

Link: https://www.spinics.net/lists/netdev/msg480675.html
Signed-off-by: Benjamin Poirier 
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 153ad406c65e..3b36efa6228d 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1915,21 +1915,10 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = >hw;
u32 icr;
-   bool enable = true;
 
icr = er32(ICR);
ew32(ICR, E1000_ICR_OTHER);
 
-   if (icr & E1000_ICR_RXO) {
-   ew32(ICR, E1000_ICR_RXO);
-   enable = false;
-   /* napi poll will re-enable Other, make sure it runs */
-   if (napi_schedule_prep(>napi)) {
-   adapter->total_rx_bytes = 0;
-   adapter->total_rx_packets = 0;
-   __napi_schedule(>napi);
-   }
-   }
if (icr & E1000_ICR_LSC) {
ew32(ICR, E1000_ICR_LSC);
hw->mac.get_link_status = true;
@@ -1938,7 +1927,7 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
mod_timer(>watchdog_timer, jiffies + 1);
}
 
-   if (enable && !test_bit(__E1000_DOWN, >state))
+   if (!test_bit(__E1000_DOWN, >state))
ew32(IMS, E1000_IMS_OTHER);
 
return IRQ_HANDLED;
@@ -2708,8 +2697,7 @@ static int e1000e_poll(struct napi_struct *napi, int 
weight)
napi_complete_done(napi, work_done);
if (!test_bit(__E1000_DOWN, >state)) {
if (adapter->msix_entries)
-   ew32(IMS, adapter->rx_ring->ims_val |
-E1000_IMS_OTHER);
+   ew32(IMS, adapter->rx_ring->ims_val);
else
e1000_irq_enable(adapter);
}
-- 
2.16.1

[PATCH net-queue 3/3] e1000e: Avoid missed interrupts following ICR read.

[Benjamin Poirier]

The 82574 specification update errata 12 states that interrupts may be
missed if ICR is read while INT_ASSERTED is not set. Avoid that problem by
setting all bits related to events that can trigger the Other interrupt in
IMS.

The Other interrupt is raised for such events regardless of whether or not
they are set in IMS. However, only when they are set is the INT_ASSERTED
bit also set in ICR.

By doing this, we ensure that INT_ASSERTED is always set when we read ICR
in e1000_msix_other() and steer clear of the errata. This also ensures that
ICR will automatically be cleared on read, therefore we no longer need to
clear bits explicitly.

Signed-off-by: Benjamin Poirier 
---
 drivers/net/ethernet/intel/e1000e/defines.h | 21 -
 drivers/net/ethernet/intel/e1000e/netdev.c  | 11 ---
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/defines.h 
b/drivers/net/ethernet/intel/e1000e/defines.h
index afb7ebe20b24..824fd44e25f0 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -400,6 +400,10 @@
 #define E1000_ICR_RXDMT00x0010 /* Rx desc min. threshold (0) */
 #define E1000_ICR_RXO   0x0040 /* Receiver Overrun */
 #define E1000_ICR_RXT0  0x0080 /* Rx timer intr (ring 0) */
+#define E1000_ICR_MDAC  0x0200 /* MDIO Access Complete */
+#define E1000_ICR_SRPD  0x0001 /* Small Receive Packet Detected */
+#define E1000_ICR_ACK   0x0002 /* Receive ACK Frame Detected */
+#define E1000_ICR_MNG   0x0004 /* Manageability Event Detected */
 #define E1000_ICR_ECCER 0x0040 /* Uncorrectable ECC Error */
 /* If this bit asserted, the driver should claim the interrupt */
 #define E1000_ICR_INT_ASSERTED 0x8000
@@ -407,7 +411,7 @@
 #define E1000_ICR_RXQ1  0x0020 /* Rx Queue 1 Interrupt */
 #define E1000_ICR_TXQ0  0x0040 /* Tx Queue 0 Interrupt */
 #define E1000_ICR_TXQ1  0x0080 /* Tx Queue 1 Interrupt */
-#define E1000_ICR_OTHER 0x0100 /* Other Interrupts */
+#define E1000_ICR_OTHER 0x0100 /* Other Interrupt */
 
 /* PBA ECC Register */
 #define E1000_PBA_ECC_COUNTER_MASK  0xFFF0 /* ECC counter mask */
@@ -431,12 +435,27 @@
E1000_IMS_RXSEQ  |\
E1000_IMS_LSC)
 
+/* These are all of the events related to the OTHER interrupt.
+ */
+#define IMS_OTHER_MASK ( \
+   E1000_IMS_LSC  | \
+   E1000_IMS_RXO  | \
+   E1000_IMS_MDAC | \
+   E1000_IMS_SRPD | \
+   E1000_IMS_ACK  | \
+   E1000_IMS_MNG)
+
 /* Interrupt Mask Set */
 #define E1000_IMS_TXDW  E1000_ICR_TXDW  /* Transmit desc written back 
*/
 #define E1000_IMS_LSC   E1000_ICR_LSC   /* Link Status Change */
 #define E1000_IMS_RXSEQ E1000_ICR_RXSEQ /* Rx sequence error */
 #define E1000_IMS_RXDMT0E1000_ICR_RXDMT0/* Rx desc min. threshold */
+#define E1000_IMS_RXO   E1000_ICR_RXO   /* Receiver Overrun */
 #define E1000_IMS_RXT0  E1000_ICR_RXT0  /* Rx timer intr */
+#define E1000_IMS_MDAC  E1000_ICR_MDAC  /* MDIO Access Complete */
+#define E1000_IMS_SRPD  E1000_ICR_SRPD  /* Small Receive Packet */
+#define E1000_IMS_ACK   E1000_ICR_ACK   /* Receive ACK Frame Detected 
*/
+#define E1000_IMS_MNG   E1000_ICR_MNG   /* Manageability Event */
 #define E1000_IMS_ECCER E1000_ICR_ECCER /* Uncorrectable ECC Error */
 #define E1000_IMS_RXQ0  E1000_ICR_RXQ0  /* Rx Queue 0 Interrupt */
 #define E1000_IMS_RXQ1  E1000_ICR_RXQ1  /* Rx Queue 1 Interrupt */
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 2c9609bee2ae..9fd4050a91ca 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1914,16 +1914,12 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
struct net_device *netdev = data;
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = >hw;
-   u32 icr;
-
-   icr = er32(ICR);
-   ew32(ICR, E1000_ICR_OTHER);
+   u32 icr = er32(ICR);
 
if (icr & adapter->eiac_mask)
ew32(ICS, (icr & adapter->eiac_mask));
 
if (icr & E1000_ICR_LSC) {
-   ew32(ICR, E1000_ICR_LSC);
hw->mac.get_link_status = true;
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, >state))
@@ -1931,7 +1927,7 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
}
 
if (!test_bit(__E1000_DOWN, >state))
-   ew32(IMS, E1000_IMS_OTHER);
+   ew32(IMS, E1000_IMS_OTHER | IMS_OTHER_MASK);
 
return IRQ_HANDLED;
 }
@@ -2258,7 +2254,8 @@ static void e1000_irq_enable(struct e1000_adapter 
*adapter)
 
if (adapter->msix_entries) {
  

Re: [PATCHv2] ARM: dts: imx6q-bx50v3: Enable secure-reg-access

[Shawn Guo]

On Mon, Feb 05, 2018 at 06:08:40PM +0100, Sebastian Reichel wrote:
> From: Peter Senna Tschudin 
> 
> Enable secure debug enable register access for Bx50v3 devices to enable
> PMU and hardware counters for perf.
> 
> Signed-off-by: Peter Senna Tschudin 
> Signed-off-by: Sebastian Reichel 
> ---
> Changes since PATCHv1:
>  * add property by adding a label to the pmu node in imx6qdl and
>referencing it in bx50v3.dtsi.
> ---
>  arch/arm/boot/dts/imx6q-bx50v3.dtsi | 4 
>  arch/arm/boot/dts/imx6qdl.dtsi  | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q-bx50v3.dtsi 
> b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> index 916ea94d75ca..2baffc8b9094 100644
> --- a/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> +++ b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> @@ -388,3 +388,7 @@
>   #interrupt-cells = <1>;
>   };
>  };
> +
> + {
> + secure-reg-access;
> +};

The labelled node should be sorted alphabetically.  That said, it should
be added before   I fixed it up and applied the patch.

Shawn

> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 59ff86695a14..2b921de44c02 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -143,7 +143,7 @@
>   };
>   };
>  
> - pmu {
> + pmu: pmu {
>   compatible = "arm,cortex-a9-pmu";
>   interrupt-parent = <>;
>   interrupts = <0 94 IRQ_TYPE_LEVEL_HIGH>;
> -- 
> 2.15.1
> 

Re: [PATCHv2] ARM: dts: imx6q-bx50v3: Enable secure-reg-access

[Shawn Guo]

On Mon, Feb 05, 2018 at 06:08:40PM +0100, Sebastian Reichel wrote:
> From: Peter Senna Tschudin 
> 
> Enable secure debug enable register access for Bx50v3 devices to enable
> PMU and hardware counters for perf.
> 
> Signed-off-by: Peter Senna Tschudin 
> Signed-off-by: Sebastian Reichel 
> ---
> Changes since PATCHv1:
>  * add property by adding a label to the pmu node in imx6qdl and
>referencing it in bx50v3.dtsi.
> ---
>  arch/arm/boot/dts/imx6q-bx50v3.dtsi | 4 
>  arch/arm/boot/dts/imx6qdl.dtsi  | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q-bx50v3.dtsi 
> b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> index 916ea94d75ca..2baffc8b9094 100644
> --- a/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> +++ b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> @@ -388,3 +388,7 @@
>   #interrupt-cells = <1>;
>   };
>  };
> +
> + {
> + secure-reg-access;
> +};

The labelled node should be sorted alphabetically.  That said, it should
be added before   I fixed it up and applied the patch.

Shawn

> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 59ff86695a14..2b921de44c02 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -143,7 +143,7 @@
>   };
>   };
>  
> - pmu {
> + pmu: pmu {
>   compatible = "arm,cortex-a9-pmu";
>   interrupt-parent = <>;
>   interrupts = <0 94 IRQ_TYPE_LEVEL_HIGH>;
> -- 
> 2.15.1
> 

[PATCH] mpls, nospec: Sanitize array index in mpls_label_ok()

[Dan Williams]

mpls_label_ok() validates that the 'platform_label' array index from a
userspace netlink message payload is valid. Under speculation the
mpls_label_ok() result may not resolve in the CPU pipeline until after
the index is used to access an array element. Sanitize the index to zero
to prevent userspace-controlled arbitrary out-of-bounds speculation, a
precursor for a speculative execution side channel vulnerability.

Cc: 
Cc: "David S. Miller" 
Cc: Eric W. Biederman 
Signed-off-by: Dan Williams 
---
 net/mpls/af_mpls.c |   24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 8ca9915befc8..aae3565c3a92 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -935,24 +936,27 @@ static int mpls_nh_build_multi(struct mpls_route_config 
*cfg,
return err;
 }
 
-static bool mpls_label_ok(struct net *net, unsigned int index,
+static bool mpls_label_ok(struct net *net, unsigned int *index,
  struct netlink_ext_ack *extack)
 {
+   bool is_ok = true;
+
/* Reserved labels may not be set */
-   if (index < MPLS_LABEL_FIRST_UNRESERVED) {
+   if (*index < MPLS_LABEL_FIRST_UNRESERVED) {
NL_SET_ERR_MSG(extack,
   "Invalid label - must be 
MPLS_LABEL_FIRST_UNRESERVED or higher");
-   return false;
+   is_ok = false;
}
 
/* The full 20 bit range may not be supported. */
-   if (index >= net->mpls.platform_labels) {
+   if (is_ok && *index >= net->mpls.platform_labels) {
NL_SET_ERR_MSG(extack,
   "Label >= configured maximum in 
platform_labels");
-   return false;
+   is_ok = false;
}
 
-   return true;
+   *index = array_index_nospec(*index, net->mpls.platform_labels);
+   return is_ok;
 }
 
 static int mpls_route_add(struct mpls_route_config *cfg,
@@ -975,7 +979,7 @@ static int mpls_route_add(struct mpls_route_config *cfg,
index = find_free_label(net);
}
 
-   if (!mpls_label_ok(net, index, extack))
+   if (!mpls_label_ok(net, , extack))
goto errout;
 
/* Append makes no sense with mpls */
@@ -1052,7 +1056,7 @@ static int mpls_route_del(struct mpls_route_config *cfg,
 
index = cfg->rc_label;
 
-   if (!mpls_label_ok(net, index, extack))
+   if (!mpls_label_ok(net, , extack))
goto errout;
 
mpls_route_update(net, index, NULL, >rc_nlinfo);
@@ -1810,7 +1814,7 @@ static int rtm_to_route_config(struct sk_buff *skb,
goto errout;
 
if (!mpls_label_ok(cfg->rc_nlinfo.nl_net,
-  cfg->rc_label, extack))
+  >rc_label, extack))
goto errout;
break;
}
@@ -2137,7 +2141,7 @@ static int mpls_getroute(struct sk_buff *in_skb, struct 
nlmsghdr *in_nlh,
goto errout;
}
 
-   if (!mpls_label_ok(net, in_label, extack)) {
+   if (!mpls_label_ok(net, _label, extack)) {
err = -EINVAL;
goto errout;
}

[PATCH] mpls, nospec: Sanitize array index in mpls_label_ok()

[Dan Williams]

mpls_label_ok() validates that the 'platform_label' array index from a
userspace netlink message payload is valid. Under speculation the
mpls_label_ok() result may not resolve in the CPU pipeline until after
the index is used to access an array element. Sanitize the index to zero
to prevent userspace-controlled arbitrary out-of-bounds speculation, a
precursor for a speculative execution side channel vulnerability.

Cc: 
Cc: "David S. Miller" 
Cc: Eric W. Biederman 
Signed-off-by: Dan Williams 
---
 net/mpls/af_mpls.c |   24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 8ca9915befc8..aae3565c3a92 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -935,24 +936,27 @@ static int mpls_nh_build_multi(struct mpls_route_config 
*cfg,
return err;
 }
 
-static bool mpls_label_ok(struct net *net, unsigned int index,
+static bool mpls_label_ok(struct net *net, unsigned int *index,
  struct netlink_ext_ack *extack)
 {
+   bool is_ok = true;
+
/* Reserved labels may not be set */
-   if (index < MPLS_LABEL_FIRST_UNRESERVED) {
+   if (*index < MPLS_LABEL_FIRST_UNRESERVED) {
NL_SET_ERR_MSG(extack,
   "Invalid label - must be 
MPLS_LABEL_FIRST_UNRESERVED or higher");
-   return false;
+   is_ok = false;
}
 
/* The full 20 bit range may not be supported. */
-   if (index >= net->mpls.platform_labels) {
+   if (is_ok && *index >= net->mpls.platform_labels) {
NL_SET_ERR_MSG(extack,
   "Label >= configured maximum in 
platform_labels");
-   return false;
+   is_ok = false;
}
 
-   return true;
+   *index = array_index_nospec(*index, net->mpls.platform_labels);
+   return is_ok;
 }
 
 static int mpls_route_add(struct mpls_route_config *cfg,
@@ -975,7 +979,7 @@ static int mpls_route_add(struct mpls_route_config *cfg,
index = find_free_label(net);
}
 
-   if (!mpls_label_ok(net, index, extack))
+   if (!mpls_label_ok(net, , extack))
goto errout;
 
/* Append makes no sense with mpls */
@@ -1052,7 +1056,7 @@ static int mpls_route_del(struct mpls_route_config *cfg,
 
index = cfg->rc_label;
 
-   if (!mpls_label_ok(net, index, extack))
+   if (!mpls_label_ok(net, , extack))
goto errout;
 
mpls_route_update(net, index, NULL, >rc_nlinfo);
@@ -1810,7 +1814,7 @@ static int rtm_to_route_config(struct sk_buff *skb,
goto errout;
 
if (!mpls_label_ok(cfg->rc_nlinfo.nl_net,
-  cfg->rc_label, extack))
+  >rc_label, extack))
goto errout;
break;
}
@@ -2137,7 +2141,7 @@ static int mpls_getroute(struct sk_buff *in_skb, struct 
nlmsghdr *in_nlh,
goto errout;
}
 
-   if (!mpls_label_ok(net, in_label, extack)) {
+   if (!mpls_label_ok(net, _label, extack)) {
err = -EINVAL;
goto errout;
}

Re: [PATCH 3/3] Revert "e1000e: Do not read ICR in Other interrupt"

[Benjamin Poirier]

On 2018/01/29 09:22, Alexander Duyck wrote:
[...]
> 
> > Consequently, we must clear OTHER manually from ICR, otherwise the
> > interrupt is immediately re-raised after exiting the handler.
> >
> > These observations are the same whether the interrupt is triggered via a
> > write to ICS or in hardware.
> >
> > Furthermore, I tested that this behavior is the same for other Other
> > events (MDAC, SRPD, ACK, MNG). Those were tested via a write to ICS
> > only, not in hardware.
> >
> > This is a version of the test patch that I used to trigger lsc and rxo in
> > software and hardware. It applies over this patch series.
> 
> I plan to look into this some more over the next few days. Ideally if
> we could mask these "OTHER" interrupts besides the LSC we could comply
> with all the needed bits for MSI-X. My concern is that we are still
> stuck reading the ICR at this point because of this and it is going to
> make dealing with MSI-X challenging on 82574 since it seems like the
> intention was that you weren't supposed to be reading the ICR when
> MSI-X is enabled based on the list of current issues and HW errata.

I totally agree with you that it looks like the msi-x interface was
designed so you don't need to read icr. That's also why I was happy to
go that direction with the (now infamous) commit 16ecba59bc33 ("e1000e:
Do not read ICR in Other interrupt", v4.5-rc1).

However, we looked at it before and there seems to be no way to mask
individual Other interrupt causes (masking rxo but getting lsc). Because
of that, I think we have to keep reading icr in the Other interrupt
handler.

> 
> At this point it seems like the interrupts is firing and the
> INT_ASSERTED is all we really need to be checking for if I understand
> this all correctly. Basically if LSC is set it will trigger OTHER and
> INT_ASSERTED, if any of the other causes are set they are only setting
> OTHER.

I think that's right and it's related to the fact that currently LSC is
set in IMS but not the other causes. Since we have to read icr (as I
wrote above) but we want to avoid reading it without INT_ASSERTED set
(as per errata 12) the solution will be to set all of the causes related
to Other in IMS. Patches incoming...

Re: [PATCH 3/3] Revert "e1000e: Do not read ICR in Other interrupt"

[Benjamin Poirier]

On 2018/01/29 09:22, Alexander Duyck wrote:
[...]
> 
> > Consequently, we must clear OTHER manually from ICR, otherwise the
> > interrupt is immediately re-raised after exiting the handler.
> >
> > These observations are the same whether the interrupt is triggered via a
> > write to ICS or in hardware.
> >
> > Furthermore, I tested that this behavior is the same for other Other
> > events (MDAC, SRPD, ACK, MNG). Those were tested via a write to ICS
> > only, not in hardware.
> >
> > This is a version of the test patch that I used to trigger lsc and rxo in
> > software and hardware. It applies over this patch series.
> 
> I plan to look into this some more over the next few days. Ideally if
> we could mask these "OTHER" interrupts besides the LSC we could comply
> with all the needed bits for MSI-X. My concern is that we are still
> stuck reading the ICR at this point because of this and it is going to
> make dealing with MSI-X challenging on 82574 since it seems like the
> intention was that you weren't supposed to be reading the ICR when
> MSI-X is enabled based on the list of current issues and HW errata.

I totally agree with you that it looks like the msi-x interface was
designed so you don't need to read icr. That's also why I was happy to
go that direction with the (now infamous) commit 16ecba59bc33 ("e1000e:
Do not read ICR in Other interrupt", v4.5-rc1).

However, we looked at it before and there seems to be no way to mask
individual Other interrupt causes (masking rxo but getting lsc). Because
of that, I think we have to keep reading icr in the Other interrupt
handler.

> 
> At this point it seems like the interrupts is firing and the
> INT_ASSERTED is all we really need to be checking for if I understand
> this all correctly. Basically if LSC is set it will trigger OTHER and
> INT_ASSERTED, if any of the other causes are set they are only setting
> OTHER.

I think that's right and it's related to the fact that currently LSC is
set in IMS but not the other causes. Since we have to read icr (as I
wrote above) but we want to avoid reading it without INT_ASSERTED set
(as per errata 12) the solution will be to set all of the causes related
to Other in IMS. Patches incoming...

[PATCH] x86/linker script: drop superfluous ALIGN

[Cao jin]

ALIGN(8) is superfluous since macro TEXT_TEXT already has one.

bonus: indentation fix, spaces -> tab.
Signed-off-by: Cao jin 
---
 arch/x86/kernel/vmlinux.lds.S | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index a4009fb9be87..f4dc9e39ea40 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -96,7 +96,6 @@ SECTIONS
_stext = .;
/* bootstrapping code */
HEAD_TEXT
-   . = ALIGN(8);
TEXT_TEXT
SCHED_TEXT
CPUIDLE_TEXT
@@ -175,7 +174,7 @@ SECTIONS
. = __vvar_beginning_hack + PAGE_SIZE;
} :data
 
-   . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE);
+   . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE);
 
/* Init code and data - will be freed after init */
. = ALIGN(PAGE_SIZE);
@@ -343,8 +342,8 @@ SECTIONS
. = ALIGN(PAGE_SIZE);   /* keep VO_INIT_SIZE page aligned */
_end = .;
 
-STABS_DEBUG
-DWARF_DEBUG
+   STABS_DEBUG
+   DWARF_DEBUG
 
/* Sections to be discarded */
DISCARDS
-- 
2.14.3

[PATCH] x86/linker script: drop superfluous ALIGN

[Cao jin]

ALIGN(8) is superfluous since macro TEXT_TEXT already has one.

bonus: indentation fix, spaces -> tab.
Signed-off-by: Cao jin 
---
 arch/x86/kernel/vmlinux.lds.S | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index a4009fb9be87..f4dc9e39ea40 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -96,7 +96,6 @@ SECTIONS
_stext = .;
/* bootstrapping code */
HEAD_TEXT
-   . = ALIGN(8);
TEXT_TEXT
SCHED_TEXT
CPUIDLE_TEXT
@@ -175,7 +174,7 @@ SECTIONS
. = __vvar_beginning_hack + PAGE_SIZE;
} :data
 
-   . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE);
+   . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE);
 
/* Init code and data - will be freed after init */
. = ALIGN(PAGE_SIZE);
@@ -343,8 +342,8 @@ SECTIONS
. = ALIGN(PAGE_SIZE);   /* keep VO_INIT_SIZE page aligned */
_end = .;
 
-STABS_DEBUG
-DWARF_DEBUG
+   STABS_DEBUG
+   DWARF_DEBUG
 
/* Sections to be discarded */
DISCARDS
-- 
2.14.3

Re: dell-smbios makes wireless card unusable on Dell XPS 13 9360

[Paul Menzel]

Dear Mario,


Am 08.02.2018 um 05:57 schrieb mario.limoncie...@dell.com:

-Original Message-
From: Paul Menzel [mailto:pmenzel+platform-driver-...@molgen.mpg.de]
Sent: Wednesday, February 7, 2018 3:41 AM
To: Limonciello, Mario 
Cc: Pali Rohár ; it+platform-driver-...@molgen.mpg.de;
platform-driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: dell-smbios makes wireless card unusable on Dell XPS 13 9360



On 02/06/18 20:51, mario.limoncie...@dell.com wrote:

-Original Message-
From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-x86-
ow...@vger.kernel.org] On Behalf Of Paul Menzel
Sent: Tuesday, February 6, 2018 10:08 AM
To: Limonciello, Mario 
Cc: Pali Rohár ; it+platform-driver-...@molgen.mpg.de;
platform-driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: dell-smbios makes wireless card unusable on Dell XPS 13 9360



On 02/06/18 16:58, mario.limoncie...@dell.com wrote:


-Original Message-
From: Paul Menzel [mailto:pmenzel+platform-driver-...@molgen.mpg.de]
Sent: Tuesday, February 6, 2018 9:50 AM
To: Pali Rohár ; Limonciello, Mario
; it+platform-driver-...@molgen.mpg.de
Cc: platform-driver-...@vger.kernel.org; Linux Kernel Mailing List 
Subject: dell-smbios makes wireless card unusable on Dell XPS 13 9360



Adding `dell-smbios` to `/etc/modules` on the Dell XPS 13 9360, it seems
to badly effect the Wireless LAN device, causing it to be absent or to
not work.

In one case, the whole system lagged, that means, the log in screen
stayed grayed out for 15 seconds, and the mouse pointer stopped moving
every 20(?) seconds for some seconds.


Can you please confirm if you're indicating this is a regression in 4.15?
Or was this present in earlier kernels too?


Yes, it happened with Ubuntu’s 4.13 too. My original message had one log
attached where the Wifi wasn’t visible at all.


Looking at that log it looks to me that the ath10k firmware crashed, not that
dell-smbios lagging caused the crash.

[   15.262226] ath10k_pci :3a:00.0: firmware crashed! (uuid 
a6c40788-b7ad-4f6a-890a-62e62c1f6e5f)
[   15.262233] ath10k_pci :3a:00.0: qca6174 hw3.2 target 0x0503chip_id 
0x00340aff sub 1a56:1535
[   15.262235] ath10k_pci :3a:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 
0 testmode 0
[   15.262645] ath10k_pci :3a:00.0: firmware ver 
WLAN.RM.4.4-00022-QCARMSWPZ-2 api 6 features wowlan,ignore-otp crc32 4d458559
[   15.262941] ath10k_pci :3a:00.0: board_file api 2 bmi_id N/A crc32 
0e26ef70
[   15.262943] ath10k_pci :3a:00.0: htt-ver 0.0 wmi-op 4 htt-op 3 cal otp 
max-sta 32 raw 0 hwcrypto 1
[   15.274887] ath10k_pci :3a:00.0: failed to get memcpy hi address for 
firmware address 4: -16
[   15.274900] ath10k_pci :3a:00.0: failed to read firmware dump area: -16
[   15.274909] ath10k_pci :3a:00.0: Copy Engine register dump:

Do you have a strong correlation with dell-smbios loading and the wireless
firmware crashing?  You're the first account I've heard of this.


Yes, the correlation is very strong.


In this situation you've described, can you unload ath10k and reload to recover?


No, I unloaded ath10k_pci, and that takes a few seconds, and also shows 
the Linux error messages in the log. Loading it again, there are also 
Linux error messages, and the wireless does *not* work again.



Also why are you putting it in /etc/modules rather than letting the udev
rules load from modaliases?  Or did you artificially prevent that in some way?


No idea, with Ubuntu 17.10 there are log messages, that the module is
not loaded. So I thought adding it to `/etc/modules` will fix it. Should
the module smbios-dell be supported out of the box on Ubuntu 17.10?


It's only supposed to load when it's needed by another kernel module such as
dell-laptop or dell-wmi.

I suppose the messages you were seeing were related to the backends to
dell-smbios (dell-smbios-wmi or dell-smbios-smm) not yet being initialized
when a driver had requested to use an SMBIOS method.

If you don't add it to /etc/modules I would expect that you'll find it's 
actually
already loaded by the system when boot has completed.


Yes, it looks like it gets loaded after I removed it from `/etc/modules`.

Still, that behavior should not happen. Could you reproduce it? Is it 
actually a ath10k_pci bug, and their maintainers should be added?



Kind regards,

Paul

Re: dell-smbios makes wireless card unusable on Dell XPS 13 9360

[Paul Menzel]

Dear Mario,


Am 08.02.2018 um 05:57 schrieb mario.limoncie...@dell.com:

-Original Message-
From: Paul Menzel [mailto:pmenzel+platform-driver-...@molgen.mpg.de]
Sent: Wednesday, February 7, 2018 3:41 AM
To: Limonciello, Mario 
Cc: Pali Rohár ; it+platform-driver-...@molgen.mpg.de;
platform-driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: dell-smbios makes wireless card unusable on Dell XPS 13 9360



On 02/06/18 20:51, mario.limoncie...@dell.com wrote:

-Original Message-
From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-x86-
ow...@vger.kernel.org] On Behalf Of Paul Menzel
Sent: Tuesday, February 6, 2018 10:08 AM
To: Limonciello, Mario 
Cc: Pali Rohár ; it+platform-driver-...@molgen.mpg.de;
platform-driver-...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: dell-smbios makes wireless card unusable on Dell XPS 13 9360



On 02/06/18 16:58, mario.limoncie...@dell.com wrote:


-Original Message-
From: Paul Menzel [mailto:pmenzel+platform-driver-...@molgen.mpg.de]
Sent: Tuesday, February 6, 2018 9:50 AM
To: Pali Rohár ; Limonciello, Mario
; it+platform-driver-...@molgen.mpg.de
Cc: platform-driver-...@vger.kernel.org; Linux Kernel Mailing List 
Subject: dell-smbios makes wireless card unusable on Dell XPS 13 9360



Adding `dell-smbios` to `/etc/modules` on the Dell XPS 13 9360, it seems
to badly effect the Wireless LAN device, causing it to be absent or to
not work.

In one case, the whole system lagged, that means, the log in screen
stayed grayed out for 15 seconds, and the mouse pointer stopped moving
every 20(?) seconds for some seconds.


Can you please confirm if you're indicating this is a regression in 4.15?
Or was this present in earlier kernels too?


Yes, it happened with Ubuntu’s 4.13 too. My original message had one log
attached where the Wifi wasn’t visible at all.


Looking at that log it looks to me that the ath10k firmware crashed, not that
dell-smbios lagging caused the crash.

[   15.262226] ath10k_pci :3a:00.0: firmware crashed! (uuid 
a6c40788-b7ad-4f6a-890a-62e62c1f6e5f)
[   15.262233] ath10k_pci :3a:00.0: qca6174 hw3.2 target 0x0503chip_id 
0x00340aff sub 1a56:1535
[   15.262235] ath10k_pci :3a:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 
0 testmode 0
[   15.262645] ath10k_pci :3a:00.0: firmware ver 
WLAN.RM.4.4-00022-QCARMSWPZ-2 api 6 features wowlan,ignore-otp crc32 4d458559
[   15.262941] ath10k_pci :3a:00.0: board_file api 2 bmi_id N/A crc32 
0e26ef70
[   15.262943] ath10k_pci :3a:00.0: htt-ver 0.0 wmi-op 4 htt-op 3 cal otp 
max-sta 32 raw 0 hwcrypto 1
[   15.274887] ath10k_pci :3a:00.0: failed to get memcpy hi address for 
firmware address 4: -16
[   15.274900] ath10k_pci :3a:00.0: failed to read firmware dump area: -16
[   15.274909] ath10k_pci :3a:00.0: Copy Engine register dump:

Do you have a strong correlation with dell-smbios loading and the wireless
firmware crashing?  You're the first account I've heard of this.


Yes, the correlation is very strong.


In this situation you've described, can you unload ath10k and reload to recover?


No, I unloaded ath10k_pci, and that takes a few seconds, and also shows 
the Linux error messages in the log. Loading it again, there are also 
Linux error messages, and the wireless does *not* work again.



Also why are you putting it in /etc/modules rather than letting the udev
rules load from modaliases?  Or did you artificially prevent that in some way?


No idea, with Ubuntu 17.10 there are log messages, that the module is
not loaded. So I thought adding it to `/etc/modules` will fix it. Should
the module smbios-dell be supported out of the box on Ubuntu 17.10?


It's only supposed to load when it's needed by another kernel module such as
dell-laptop or dell-wmi.

I suppose the messages you were seeing were related to the backends to
dell-smbios (dell-smbios-wmi or dell-smbios-smm) not yet being initialized
when a driver had requested to use an SMBIOS method.

If you don't add it to /etc/modules I would expect that you'll find it's 
actually
already loaded by the system when boot has completed.


Yes, it looks like it gets loaded after I removed it from `/etc/modules`.

Still, that behavior should not happen. Could you reproduce it? Is it 
actually a ath10k_pci bug, and their maintainers should be added?



Kind regards,

Paul

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
 wrote:
> If stdio is not tty, conf_askvalue() puts additional new line to
> prevent prompts are all concatenated into a single line.  This care
> is missing in conf_choice(), so a 'choice' prompt and the next prompt
> are shown in the same line.
>
> Move the code into xfgets() to take care of all cases.  To improve
> this more, echo stdin to stdout.  This clarifies what keys were input
> from stdio and the stdout looks like as if it were from tty.
>
> I removed the isatty(2) check since stderr is unrelated here.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/conf.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 358e2e4..c5318d3 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>  {
> if (!fgets(str, size, in))
> fprintf(stderr, "\nError in reading or end of file.\n");
> +
> +   if (!tty_stdio)
> +   printf("%s", str);
>  }
>
>  static int conf_askvalue(struct symbol *sym, const char *def)
> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
> *def)
> case oldaskconfig:
> fflush(stdout);
> xfgets(line, sizeof(line), stdin);
> -   if (!tty_stdio)
> -   printf("\n");
> return 1;
> default:
> break;
> @@ -495,7 +496,7 @@ int main(int ac, char **av)
> bindtextdomain(PACKAGE, LOCALEDIR);
> textdomain(PACKAGE);
>
> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
> +   tty_stdio = isatty(0) && isatty(1);
>
> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
> if (opt == 's') {
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Might be some case I'm not thinking of, but wouldn't it make sense to
just check isatty(1) as well? If stdout is a regular file, it seems
it'd be nice to have the input appear there, regardless of where stdin
is from.

Maybe the tty_stdio variable could be removed then as well, replaced
with just 'if (!isatty(1))'.

Cheers,
Ulf

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
 wrote:
> If stdio is not tty, conf_askvalue() puts additional new line to
> prevent prompts are all concatenated into a single line.  This care
> is missing in conf_choice(), so a 'choice' prompt and the next prompt
> are shown in the same line.
>
> Move the code into xfgets() to take care of all cases.  To improve
> this more, echo stdin to stdout.  This clarifies what keys were input
> from stdio and the stdout looks like as if it were from tty.
>
> I removed the isatty(2) check since stderr is unrelated here.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/conf.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 358e2e4..c5318d3 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>  {
> if (!fgets(str, size, in))
> fprintf(stderr, "\nError in reading or end of file.\n");
> +
> +   if (!tty_stdio)
> +   printf("%s", str);
>  }
>
>  static int conf_askvalue(struct symbol *sym, const char *def)
> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
> *def)
> case oldaskconfig:
> fflush(stdout);
> xfgets(line, sizeof(line), stdin);
> -   if (!tty_stdio)
> -   printf("\n");
> return 1;
> default:
> break;
> @@ -495,7 +496,7 @@ int main(int ac, char **av)
> bindtextdomain(PACKAGE, LOCALEDIR);
> textdomain(PACKAGE);
>
> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
> +   tty_stdio = isatty(0) && isatty(1);
>
> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
> if (opt == 's') {
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Might be some case I'm not thinking of, but wouldn't it make sense to
just check isatty(1) as well? If stdout is a regular file, it seems
it'd be nice to have the input appear there, regardless of where stdin
is from.

Maybe the tty_stdio variable could be removed then as well, replaced
with just 'if (!isatty(1))'.

Cheers,
Ulf

Regression after commit 19809c2da28a ("mm, vmalloc: use __GFP_HIGHMEM implicitly")

[Kai Heng Feng]

A user with i386 instead of AMD64 machine reports [1] that commit 19809c2da28a 
("mm, vmalloc: use __GFP_HIGHMEM implicitly”) causes a regression.
BUG_ON(PageHighMem(pg)) in drivers/media/common/saa7146/saa7146_core.c always 
gets triggered after that commit.

Commit 704b862f9efd ("mm/vmalloc.c: don't unconditonally use __GFP_HIGHMEM”) 
adjusts the mask logic, now the __GFP_HIGHMEM only gets applied when there is 
no GFP_DMA or GFP_DMA32.

So I tried to adjust its malloc to "__vmalloc(nr_pages * sizeof(struct 
scatterlist), GFP_KERNEL | GFP_DMA | __GFP_ZERO, PAGE_KERNEL)”, but both 
GFP_DMA or GFP_DMA32 still trigger the BUG_ON(PageHighMem()) macro.

Also there are other BUG_ON(PageHighMem()) in drivers/media, I think they will 
get hit by same regression in 32bit machine too.

[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1742316

Kai-Heng

Regression after commit 19809c2da28a ("mm, vmalloc: use __GFP_HIGHMEM implicitly")

[Kai Heng Feng]

A user with i386 instead of AMD64 machine reports [1] that commit 19809c2da28a 
("mm, vmalloc: use __GFP_HIGHMEM implicitly”) causes a regression.
BUG_ON(PageHighMem(pg)) in drivers/media/common/saa7146/saa7146_core.c always 
gets triggered after that commit.

Commit 704b862f9efd ("mm/vmalloc.c: don't unconditonally use __GFP_HIGHMEM”) 
adjusts the mask logic, now the __GFP_HIGHMEM only gets applied when there is 
no GFP_DMA or GFP_DMA32.

So I tried to adjust its malloc to "__vmalloc(nr_pages * sizeof(struct 
scatterlist), GFP_KERNEL | GFP_DMA | __GFP_ZERO, PAGE_KERNEL)”, but both 
GFP_DMA or GFP_DMA32 still trigger the BUG_ON(PageHighMem()) macro.

Also there are other BUG_ON(PageHighMem()) in drivers/media, I think they will 
get hit by same regression in 32bit machine too.

[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1742316

Kai-Heng

Re: [PATCH] KVM: nVMX: Fix CR4 after VMLAUNCH/VMRESUME failure

[Wanpeng Li]

2018-02-08 0:57 GMT+08:00 Jim Mattson :
> vmcs12->host_cr[34] does not contain the up-to-date values when L1 is
> running. L1 can vmwrite any values there. We know at this point that

It will incur a vmexit to emulate L1 vmwrites vmcs12->host_cr[34] even
if vmcs shadow is enabled since host_cr[34] is not shadowed in the
bitmap, why it is not up-to-date when L1 is running?

Regards,
Wanpeng Li

> they are legal (because we checked them), but that's about it. If the
> VMLAUNCH/VMRESUME of vmcs12 fails for "invalid control field," there
> is no VM-exit from L2 to L1, and these fields are not loaded. Instead,
> execution just falls through to the next instruction with VMFailValid
> semantics.
>
> On Wed, Feb 7, 2018 at 12:31 AM, Wanpeng Li  wrote:
>> 2018-02-07 0:58 GMT+08:00 Jim Mattson :
>>> On Mon, Feb 5, 2018 at 4:57 PM, Wanpeng Li  wrote:
>>>
 This is effective one, what I restore in this patch is
 achitectural/guest visible.
>>>
>>> This patch doesn't "restore" the guest visible CR4 to its value at the
>>> time of VMLAUNCH/VMRESUME. It loads a new CR4 value from the vmcs12.
>>> That behavior is incorrect.
>>
>> You have another pointing out about this.
>> https://lkml.org/lkml/2018/2/5/518 vmcs12->host_cr3/host_cr4 has the
>> up-to-date value when L1 is running, it is still up-to-date after
>> vmexit due to L1 executes VMLAUNCH/VMRESUME, I think the value stays
>> the same before L0 emulates the VMLAUNCH/VMRESUME, according to below
>> comments, why vmcs12->host_cr3/cr4 is not the value which we should
>> restore?
>>
>> * After an early L2 VM-entry failure, we're now back
>> * in L1 which thinks it just finished a VMLAUNCH or
>> * VMRESUME instruction
>>
>> Regards,
>> Wanpeng Li

Re: [PATCH] KVM: nVMX: Fix CR4 after VMLAUNCH/VMRESUME failure

[Wanpeng Li]

2018-02-08 0:57 GMT+08:00 Jim Mattson :
> vmcs12->host_cr[34] does not contain the up-to-date values when L1 is
> running. L1 can vmwrite any values there. We know at this point that

It will incur a vmexit to emulate L1 vmwrites vmcs12->host_cr[34] even
if vmcs shadow is enabled since host_cr[34] is not shadowed in the
bitmap, why it is not up-to-date when L1 is running?

Regards,
Wanpeng Li

> they are legal (because we checked them), but that's about it. If the
> VMLAUNCH/VMRESUME of vmcs12 fails for "invalid control field," there
> is no VM-exit from L2 to L1, and these fields are not loaded. Instead,
> execution just falls through to the next instruction with VMFailValid
> semantics.
>
> On Wed, Feb 7, 2018 at 12:31 AM, Wanpeng Li  wrote:
>> 2018-02-07 0:58 GMT+08:00 Jim Mattson :
>>> On Mon, Feb 5, 2018 at 4:57 PM, Wanpeng Li  wrote:
>>>
 This is effective one, what I restore in this patch is
 achitectural/guest visible.
>>>
>>> This patch doesn't "restore" the guest visible CR4 to its value at the
>>> time of VMLAUNCH/VMRESUME. It loads a new CR4 value from the vmcs12.
>>> That behavior is incorrect.
>>
>> You have another pointing out about this.
>> https://lkml.org/lkml/2018/2/5/518 vmcs12->host_cr3/host_cr4 has the
>> up-to-date value when L1 is running, it is still up-to-date after
>> vmexit due to L1 executes VMLAUNCH/VMRESUME, I think the value stays
>> the same before L0 emulates the VMLAUNCH/VMRESUME, according to below
>> comments, why vmcs12->host_cr3/cr4 is not the value which we should
>> restore?
>>
>> * After an early L2 VM-entry failure, we're now back
>> * in L1 which thinks it just finished a VMLAUNCH or
>> * VMRESUME instruction
>>
>> Regards,
>> Wanpeng Li

Re: [PATCH 1/2] kconfig: remove check_stdin()

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
 wrote:
> Except silentoldconfig, valid_stdin is 1, so check_stdin() is no-op.
>
> oldconfig and silentoldconfig work almost in the same way except that
> the latter generates additional files.  Both ask users for input for
> new symbols.
>
> I do not know why only silentoldconfig requires stdio be tty.
>
>   $ rm -f .config; touch .config
>   $ yes "" | make oldconfig > stdout
>   $ rm -f .config; touch .config
>   $ yes "" | make silentoldconfig > stdout
>   make[1]: *** [silentoldconfig] Error 1
>   make: *** [silentoldconfig] Error 2
>   $ tail -n 4 stdout
>   Console input/output is redirected. Run 'make oldconfig' to update 
> configuration.
>
>   scripts/kconfig/Makefile:40: recipe for target 'silentoldconfig' failed
>   Makefile:507: recipe for target 'silentoldconfig' failed
>
> Redirection is useful, for example, for testing where we want to give
> particular key inputs from a test file, then check the result.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/conf.c | 14 --
>  1 file changed, 14 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 307bc3f..358e2e4 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -39,7 +39,6 @@ static enum input_mode input_mode = oldaskconfig;
>
>  static int indent = 1;
>  static int tty_stdio;
> -static int valid_stdin = 1;
>  static int sync_kconfig;
>  static int conf_cnt;
>  static char line[PATH_MAX];
> @@ -72,16 +71,6 @@ static void strip(char *str)
> *p-- = 0;
>  }
>
> -static void check_stdin(void)
> -{
> -   if (!valid_stdin) {
> -   printf(_("aborted!\n\n"));
> -   printf(_("Console input/output is redirected. "));
> -   printf(_("Run 'make oldconfig' to update 
> configuration.\n\n"));
> -   exit(1);
> -   }
> -}
> -
>  /* Helper function to facilitate fgets() by Jean Sacren. */
>  static void xfgets(char *str, int size, FILE *in)
>  {
> @@ -113,7 +102,6 @@ static int conf_askvalue(struct symbol *sym, const char 
> *def)
> printf("%s\n", def);
> return 0;
> }
> -   check_stdin();
> /* fall through */
> case oldaskconfig:
> fflush(stdout);
> @@ -315,7 +303,6 @@ static int conf_choice(struct menu *menu)
> printf("%d\n", cnt);
> break;
> }
> -   check_stdin();
> /* fall through */
> case oldaskconfig:
> fflush(stdout);
> @@ -650,7 +637,6 @@ int main(int ac, char **av)
> return 1;
> }
> }
> -   valid_stdin = tty_stdio;
> }
>
> switch (input_mode) {
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Lots of weird stuff indeed...

Cheers,
Ulf

Re: [PATCH 1/2] kconfig: remove check_stdin()

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
 wrote:
> Except silentoldconfig, valid_stdin is 1, so check_stdin() is no-op.
>
> oldconfig and silentoldconfig work almost in the same way except that
> the latter generates additional files.  Both ask users for input for
> new symbols.
>
> I do not know why only silentoldconfig requires stdio be tty.
>
>   $ rm -f .config; touch .config
>   $ yes "" | make oldconfig > stdout
>   $ rm -f .config; touch .config
>   $ yes "" | make silentoldconfig > stdout
>   make[1]: *** [silentoldconfig] Error 1
>   make: *** [silentoldconfig] Error 2
>   $ tail -n 4 stdout
>   Console input/output is redirected. Run 'make oldconfig' to update 
> configuration.
>
>   scripts/kconfig/Makefile:40: recipe for target 'silentoldconfig' failed
>   Makefile:507: recipe for target 'silentoldconfig' failed
>
> Redirection is useful, for example, for testing where we want to give
> particular key inputs from a test file, then check the result.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/conf.c | 14 --
>  1 file changed, 14 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 307bc3f..358e2e4 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -39,7 +39,6 @@ static enum input_mode input_mode = oldaskconfig;
>
>  static int indent = 1;
>  static int tty_stdio;
> -static int valid_stdin = 1;
>  static int sync_kconfig;
>  static int conf_cnt;
>  static char line[PATH_MAX];
> @@ -72,16 +71,6 @@ static void strip(char *str)
> *p-- = 0;
>  }
>
> -static void check_stdin(void)
> -{
> -   if (!valid_stdin) {
> -   printf(_("aborted!\n\n"));
> -   printf(_("Console input/output is redirected. "));
> -   printf(_("Run 'make oldconfig' to update 
> configuration.\n\n"));
> -   exit(1);
> -   }
> -}
> -
>  /* Helper function to facilitate fgets() by Jean Sacren. */
>  static void xfgets(char *str, int size, FILE *in)
>  {
> @@ -113,7 +102,6 @@ static int conf_askvalue(struct symbol *sym, const char 
> *def)
> printf("%s\n", def);
> return 0;
> }
> -   check_stdin();
> /* fall through */
> case oldaskconfig:
> fflush(stdout);
> @@ -315,7 +303,6 @@ static int conf_choice(struct menu *menu)
> printf("%d\n", cnt);
> break;
> }
> -   check_stdin();
> /* fall through */
> case oldaskconfig:
> fflush(stdout);
> @@ -650,7 +637,6 @@ int main(int ac, char **av)
> return 1;
> }
> }
> -   valid_stdin = tty_stdio;
> }
>
> switch (input_mode) {
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Lots of weird stuff indeed...

Cheers,
Ulf

Re: [PATCH] ALSA: usb-audio: Fix UAC2 get_ctl request with a RANGE attribute

[Kirill Marinushkin]

On 02/07/18 06:45, Takashi Iwai wrote:
> On Mon, 29 Jan 2018 06:37:55 +0100,
> Kirill Marinushkin wrote:
>> The layout of the UAC2 Control request and response varies depending on
>> the request type. With the current implementation, only the Layout 2
>> Parameter Block (with the 2-byte sized RANGE attribute) is handled
>> properly. For the Control requests with the 1-byte sized RANGE attribute
>> (Bass Control, Mid Control, Tremble Control), the response is parsed
>> incorrectly.
>>
>> This commit:
>> * fixes the wLength field value in the request
>> * fixes parsing the range values from the response
>>
>> Fixes: 23caaf19b11e ("ALSA: usb-mixer: Add support for Audio Class v2.0")
>> Signed-off-by: Kirill Marinushkin 
>> Cc: Jaroslav Kysela 
>> Cc: Takashi Iwai 
>> Cc: Jaejoong Kim 
>> Cc: Bhumika Goyal 
>> Cc: Stephen Barber 
>> Cc: Julian Scheel 
>> Cc: alsa-de...@alsa-project.org
>> Cc: linux-kernel@vger.kernel.org
> Sorry for the late reply, as I've been (and still) off.
>
> Does this bug actually hit on any real devices, or is it only a
> logical error so far?  In the former case, a Cc to stable is
> mandatory.
>
> In anyway, I'll review and merge it properly once after I back to
> work.
>
>
> thanks,
>
> Takashi

Hello Takashi,

Thank you for your answer. I will wait until you are back to work, don't 
worry about the late replies.

I did not hit the issue on a real device.

During my UAC2 experiments, I reproduced this issue on the development 
board, and then tested my solution on it.
 
Best Regards,
Kirill

Re: [PATCH] ALSA: usb-audio: Fix UAC2 get_ctl request with a RANGE attribute

[Kirill Marinushkin]

On 02/07/18 06:45, Takashi Iwai wrote:
> On Mon, 29 Jan 2018 06:37:55 +0100,
> Kirill Marinushkin wrote:
>> The layout of the UAC2 Control request and response varies depending on
>> the request type. With the current implementation, only the Layout 2
>> Parameter Block (with the 2-byte sized RANGE attribute) is handled
>> properly. For the Control requests with the 1-byte sized RANGE attribute
>> (Bass Control, Mid Control, Tremble Control), the response is parsed
>> incorrectly.
>>
>> This commit:
>> * fixes the wLength field value in the request
>> * fixes parsing the range values from the response
>>
>> Fixes: 23caaf19b11e ("ALSA: usb-mixer: Add support for Audio Class v2.0")
>> Signed-off-by: Kirill Marinushkin 
>> Cc: Jaroslav Kysela 
>> Cc: Takashi Iwai 
>> Cc: Jaejoong Kim 
>> Cc: Bhumika Goyal 
>> Cc: Stephen Barber 
>> Cc: Julian Scheel 
>> Cc: alsa-de...@alsa-project.org
>> Cc: linux-kernel@vger.kernel.org
> Sorry for the late reply, as I've been (and still) off.
>
> Does this bug actually hit on any real devices, or is it only a
> logical error so far?  In the former case, a Cc to stable is
> mandatory.
>
> In anyway, I'll review and merge it properly once after I back to
> work.
>
>
> thanks,
>
> Takashi

Hello Takashi,

Thank you for your answer. I will wait until you are back to work, don't 
worry about the late replies.

I did not hit the issue on a real device.

During my UAC2 experiments, I reproduced this issue on the development 
board, and then tested my solution on it.
 
Best Regards,
Kirill

Re: [PATCH v3 1/2] dt-bindings: gpio: Add Spreadtrum GPIO controller documentation

[Baolin Wang]

Hi Rob,

On 8 February 2018 at 05:19, Rob Herring  wrote:
> On Mon, Feb 05, 2018 at 09:55:10AM +0800, Baolin Wang wrote:
>> This patch adds the device tree bindings for the Spreadtrum
>> GPIO controller. The gpios will be supported by the GPIO
>> generic library.
>>
>> Signed-off-by: Baolin Wang 
>> ---
>> Changes since v1:
>>  - No updates.
>> ---
>>  .../devicetree/bindings/gpio/gpio-sprd.txt |   28 
>> 
>>  1 file changed, 28 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-sprd.txt
>
> Please add acks/reviews when posting new versions.

Yes, I always do that. But you acked my V2 after I already send out
the V3. Anyway thanks for your reviewed-tag.

-- 
Baolin.wang
Best Regards

Re: [PATCH v3 1/2] dt-bindings: gpio: Add Spreadtrum GPIO controller documentation

[Baolin Wang]

Hi Rob,

On 8 February 2018 at 05:19, Rob Herring  wrote:
> On Mon, Feb 05, 2018 at 09:55:10AM +0800, Baolin Wang wrote:
>> This patch adds the device tree bindings for the Spreadtrum
>> GPIO controller. The gpios will be supported by the GPIO
>> generic library.
>>
>> Signed-off-by: Baolin Wang 
>> ---
>> Changes since v1:
>>  - No updates.
>> ---
>>  .../devicetree/bindings/gpio/gpio-sprd.txt |   28 
>> 
>>  1 file changed, 28 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-sprd.txt
>
> Please add acks/reviews when posting new versions.

Yes, I always do that. But you acked my V2 after I already send out
the V3. Anyway thanks for your reviewed-tag.

-- 
Baolin.wang
Best Regards

Re: linux-next: Signed-off-by missing for commit in the s390 tree

[Martin Schwidefsky]

On Thu, 8 Feb 2018 07:35:39 +1100
Stephen Rothwell  wrote:

> Commit
> 
>   b175210bd231 ("s390/kconfig: Remove ARCH_WANTS_PROT_NUMA_PROT_NONE select")
> 
> is missing a Signed-off-by from its committer.
 
It is always that "last" one you add. Fixed. Thanks.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: linux-next: Signed-off-by missing for commit in the s390 tree

[Martin Schwidefsky]

On Thu, 8 Feb 2018 07:35:39 +1100
Stephen Rothwell  wrote:

> Commit
> 
>   b175210bd231 ("s390/kconfig: Remove ARCH_WANTS_PROT_NUMA_PROT_NONE select")
> 
> is missing a Signed-off-by from its committer.
 
It is always that "last" one you add. Fixed. Thanks.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

[git pull] drm for v4.16-rc1 (part 2 - fixes)

[Dave Airlie]

Hi Linus,

Ben missed sending his tree, but he really didn't have much stuff in
it, GP108 acceleration support is enabled by "secure boot" support,
some clockgating work on Kepler, and bunch of fixes. The main bulk is
regenerated firmware files, the change to them really isn't that
large.

Otherwise this contains regular Intel and AMDGPU fixes

Regards,
Dave.

The following changes since commit 24b8ef699e8221d2b7f813adaab13eec053e1507:

  drm/ast: Load lut in crtc_commit (2018-02-01 11:35:46 +1000)

are available in the git repository at:

  git://people.freedesktop.org/~airlied/linux tags/drm-for-v4.16-part2-fixes

for you to fetch changes up to 94fc27ac487a80daf42f97b1a0503d029f3c1325:

  Merge tag 'drm-intel-next-fixes-2018-02-07' of
git://anongit.freedesktop.org/drm/drm-intel into drm-next (2018-02-08
08:21:37 +1000)


nouveau features, i915 + amdgpu fixes


Anusha Srivatsa (1):
  drm/i915/glk: Disable Guc and HuC on GLK

Arnd Bergmann (2):
  drm/nouveau: nouveau: use correct string length
  drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning

Ben Skeggs (8):
  drm/nouveau/secboot/r370: move a bunch of r375 stuff to a new
implementation
  drm/nouveau/secboot/r370: implement support for booting LS SEC2 ucode
  drm/nouveau/secboot/gp108: implement on top of acr_r370
  drm/nouveau/fbcon: add module parameter to select bits-per-pixel
  drm/nouveau/bo: add helper functions for handling pinned+mapped buffers
  drm/nouveau/kms/nv50: prepare for double-buffered LUTs
  drm/nouveau/kms/nv50: use INTERPOLATE_257_UNITY_RANGE LUT on
newer chipsets
  drm/nouveau/kms/nv50: fix handling of gamma since atomic conversion

Changbin Du (1):
  drm/i915/gvt: Fix aperture read/write emulation when enable x-no-mmap=on

Chris Wilson (5):
  drm/i915/pmu: Reconstruct active state on starting busy-stats
  drm/i915: Only attempt to scan the requested number of shrinker slabs
  drm/i915: Protect WC stash allocation against direct reclaim
  drm/i915: Always run hangcheck while the GPU is busy
  drm/i915/ppgtt: Pin page directories before allocation

Christian König (3):
  drm/amdgpu: fix another potential cause of VM faults
  drm/amdgpu: fix locking in vega10_ih_prescreen_iv
  drm/amdgpu: remove WARN_ON when VM isn't found v2

Christoph Böhmwalder (1):
  drm/nouveau/drm/nouveau/mmu: fix odd_ptr_err.cocci warnings

Dave Airlie (4):
  Merge tag 'drm-intel-next-fixes-2018-02-01' of
git://anongit.freedesktop.org/drm/drm-intel into drm-next
  Merge branch 'linux-4.16' of git://github.com/skeggsb/linux into drm-next
  Merge branch 'drm-next-4.16' of
git://people.freedesktop.org/~agd5f/linux into drm-next
  Merge tag 'drm-intel-next-fixes-2018-02-07' of
git://anongit.freedesktop.org/drm/drm-intel into drm-next

Hang Yuan (1):
  drm/i915/gvt: validate gfn before set shadow page entry

Huang Rui (1):
  drm/amdgpu: use queue 0 for kiq ring

Ilia Mirkin (1):
  drm/nouveau/kms/nv50: use "low res" lut for indexed mode

Imre Deak (2):
  drm/i915: Fix using BIT_ULL() vs. BIT() for power domain masks
  drm/i915/bxt, glk: Increase PCODE timeouts during CDCLK freq changing

Jani Nikula (1):
  drm/i915/bios: add DP max link rate to VBT child device struct

Julia Lawall (1):
  drm/radeon: adjust tested variable

Karol Herbst (1):
  drm/nouveau/pmu/fuc: don't use movw directly anymore

Lionel Landwerlin (1):
  Revert "drm/i915: mark all device info struct with __initconst"

Luis de Bethencourt (1):
  drm/nouveau/mmu: Fix trailing semicolon

Lyude Paul (5):
  drm/nouveau: Add support for basic clockgating on Kepler1
  drm/nouveau: Add support for BLCG on Kepler1
  drm/nouveau: Add support for BLCG on Kepler2
  drm/nouveau: Add support for SLCG for Kepler2
  drm/nouveau: Introduce NvPmEnableGating option

Maarten Lankhorst (1):
  drm/i915: Always call to intel_display_set_init_power() in resume_early.

Manasi Navare (1):
  drm/i915/edp: Do not do link training fallback or prune modes on EDP

Michal Srb (2):
  drm/i915/cmdparser: Check reg_table_count before derefencing.
  drm/i915/cmdparser: Do not check past the cmd length.

Michel Thierry (1):
  drm/i915/gvt: Do not use I915_NUM_ENGINES to iterate over the
mocs regs array

Mika Kahola (1):
  drm/i915: Check for fused or unused pipes

Oscar Mateo (1):
  drm/i915: Stop getting the fault address from RING_FAULT_REG

Pei Zhang (1):
  drm/i915/gvt: add PLANE_KEYMAX regs to mmio track list

Rodrigo Vivi (2):
  drm/i915/cnp: Ignore VBT request for know invalid DDC pin.
  drm/i915/cnp: Properly handle VBT ddc pin out of bounds.

Roger He (1):
  drm/ttm: fix missing parameter change for ttm_bo_cleanup_refs

Sagar Arun Kamble (1):
  drm/i915/guc: Add uc_fini_wq in gem_init 

[git pull] drm for v4.16-rc1 (part 2 - fixes)

[Dave Airlie]

Hi Linus,

Ben missed sending his tree, but he really didn't have much stuff in
it, GP108 acceleration support is enabled by "secure boot" support,
some clockgating work on Kepler, and bunch of fixes. The main bulk is
regenerated firmware files, the change to them really isn't that
large.

Otherwise this contains regular Intel and AMDGPU fixes

Regards,
Dave.

The following changes since commit 24b8ef699e8221d2b7f813adaab13eec053e1507:

  drm/ast: Load lut in crtc_commit (2018-02-01 11:35:46 +1000)

are available in the git repository at:

  git://people.freedesktop.org/~airlied/linux tags/drm-for-v4.16-part2-fixes

for you to fetch changes up to 94fc27ac487a80daf42f97b1a0503d029f3c1325:

  Merge tag 'drm-intel-next-fixes-2018-02-07' of
git://anongit.freedesktop.org/drm/drm-intel into drm-next (2018-02-08
08:21:37 +1000)


nouveau features, i915 + amdgpu fixes


Anusha Srivatsa (1):
  drm/i915/glk: Disable Guc and HuC on GLK

Arnd Bergmann (2):
  drm/nouveau: nouveau: use correct string length
  drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning

Ben Skeggs (8):
  drm/nouveau/secboot/r370: move a bunch of r375 stuff to a new
implementation
  drm/nouveau/secboot/r370: implement support for booting LS SEC2 ucode
  drm/nouveau/secboot/gp108: implement on top of acr_r370
  drm/nouveau/fbcon: add module parameter to select bits-per-pixel
  drm/nouveau/bo: add helper functions for handling pinned+mapped buffers
  drm/nouveau/kms/nv50: prepare for double-buffered LUTs
  drm/nouveau/kms/nv50: use INTERPOLATE_257_UNITY_RANGE LUT on
newer chipsets
  drm/nouveau/kms/nv50: fix handling of gamma since atomic conversion

Changbin Du (1):
  drm/i915/gvt: Fix aperture read/write emulation when enable x-no-mmap=on

Chris Wilson (5):
  drm/i915/pmu: Reconstruct active state on starting busy-stats
  drm/i915: Only attempt to scan the requested number of shrinker slabs
  drm/i915: Protect WC stash allocation against direct reclaim
  drm/i915: Always run hangcheck while the GPU is busy
  drm/i915/ppgtt: Pin page directories before allocation

Christian König (3):
  drm/amdgpu: fix another potential cause of VM faults
  drm/amdgpu: fix locking in vega10_ih_prescreen_iv
  drm/amdgpu: remove WARN_ON when VM isn't found v2

Christoph Böhmwalder (1):
  drm/nouveau/drm/nouveau/mmu: fix odd_ptr_err.cocci warnings

Dave Airlie (4):
  Merge tag 'drm-intel-next-fixes-2018-02-01' of
git://anongit.freedesktop.org/drm/drm-intel into drm-next
  Merge branch 'linux-4.16' of git://github.com/skeggsb/linux into drm-next
  Merge branch 'drm-next-4.16' of
git://people.freedesktop.org/~agd5f/linux into drm-next
  Merge tag 'drm-intel-next-fixes-2018-02-07' of
git://anongit.freedesktop.org/drm/drm-intel into drm-next

Hang Yuan (1):
  drm/i915/gvt: validate gfn before set shadow page entry

Huang Rui (1):
  drm/amdgpu: use queue 0 for kiq ring

Ilia Mirkin (1):
  drm/nouveau/kms/nv50: use "low res" lut for indexed mode

Imre Deak (2):
  drm/i915: Fix using BIT_ULL() vs. BIT() for power domain masks
  drm/i915/bxt, glk: Increase PCODE timeouts during CDCLK freq changing

Jani Nikula (1):
  drm/i915/bios: add DP max link rate to VBT child device struct

Julia Lawall (1):
  drm/radeon: adjust tested variable

Karol Herbst (1):
  drm/nouveau/pmu/fuc: don't use movw directly anymore

Lionel Landwerlin (1):
  Revert "drm/i915: mark all device info struct with __initconst"

Luis de Bethencourt (1):
  drm/nouveau/mmu: Fix trailing semicolon

Lyude Paul (5):
  drm/nouveau: Add support for basic clockgating on Kepler1
  drm/nouveau: Add support for BLCG on Kepler1
  drm/nouveau: Add support for BLCG on Kepler2
  drm/nouveau: Add support for SLCG for Kepler2
  drm/nouveau: Introduce NvPmEnableGating option

Maarten Lankhorst (1):
  drm/i915: Always call to intel_display_set_init_power() in resume_early.

Manasi Navare (1):
  drm/i915/edp: Do not do link training fallback or prune modes on EDP

Michal Srb (2):
  drm/i915/cmdparser: Check reg_table_count before derefencing.
  drm/i915/cmdparser: Do not check past the cmd length.

Michel Thierry (1):
  drm/i915/gvt: Do not use I915_NUM_ENGINES to iterate over the
mocs regs array

Mika Kahola (1):
  drm/i915: Check for fused or unused pipes

Oscar Mateo (1):
  drm/i915: Stop getting the fault address from RING_FAULT_REG

Pei Zhang (1):
  drm/i915/gvt: add PLANE_KEYMAX regs to mmio track list

Rodrigo Vivi (2):
  drm/i915/cnp: Ignore VBT request for know invalid DDC pin.
  drm/i915/cnp: Properly handle VBT ddc pin out of bounds.

Roger He (1):
  drm/ttm: fix missing parameter change for ttm_bo_cleanup_refs

Sagar Arun Kamble (1):
  drm/i915/guc: Add uc_fini_wq in gem_init 

Re: [PATCH 04/14] kconfig: print additional new line for choice for redirection

[Masahiro Yamada]

2018-02-08 8:34 GMT+09:00 Ulf Magnusson :
> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
>  wrote:
>> If stdout is redirected to a file, prompts look differently due to
>> missing new lines.
>>
>> Currently, conf_askvalue() takes care of this by putting additional
>> new line, but conf_choice() does not.  Do likewise so that prompts
>> after 'choice' look properly.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>>  scripts/kconfig/conf.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index d346642..6ce06c8 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -317,6 +317,8 @@ static int conf_choice(struct menu *menu)
>> case oldaskconfig:
>> fflush(stdout);
>> xfgets(line, sizeof(line), stdin);
>> +   if (!tty_stdio)
>> +   printf("\n");
>> strip(line);
>> if (line[0] == '?') {
>> print_help(menu);
>> --
>> 2.7.4
>>
>
> Reviewed-by: Ulf Magnusson 
>
> Maybe this could be moved into the xfgets() function as well.
>

Thanks for your comment!
Yes, it is better to move this into xfgets().

I improve this a bit more so that the redirected stdout
contains not only '\n' but also input keys.

https://patchwork.kernel.org/patch/10206611/


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 04/14] kconfig: print additional new line for choice for redirection

[Masahiro Yamada]

2018-02-08 8:34 GMT+09:00 Ulf Magnusson :
> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
>  wrote:
>> If stdout is redirected to a file, prompts look differently due to
>> missing new lines.
>>
>> Currently, conf_askvalue() takes care of this by putting additional
>> new line, but conf_choice() does not.  Do likewise so that prompts
>> after 'choice' look properly.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>>  scripts/kconfig/conf.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index d346642..6ce06c8 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -317,6 +317,8 @@ static int conf_choice(struct menu *menu)
>> case oldaskconfig:
>> fflush(stdout);
>> xfgets(line, sizeof(line), stdin);
>> +   if (!tty_stdio)
>> +   printf("\n");
>> strip(line);
>> if (line[0] == '?') {
>> print_help(menu);
>> --
>> 2.7.4
>>
>
> Reviewed-by: Ulf Magnusson 
>
> Maybe this could be moved into the xfgets() function as well.
>

Thanks for your comment!
Yes, it is better to move this into xfgets().

I improve this a bit more so that the redirected stdout
contains not only '\n' but also input keys.

https://patchwork.kernel.org/patch/10206611/


-- 
Best Regards
Masahiro Yamada

[PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Masahiro Yamada]

If stdio is not tty, conf_askvalue() puts additional new line to
prevent prompts are all concatenated into a single line.  This care
is missing in conf_choice(), so a 'choice' prompt and the next prompt
are shown in the same line.

Move the code into xfgets() to take care of all cases.  To improve
this more, echo stdin to stdout.  This clarifies what keys were input
from stdio and the stdout looks like as if it were from tty.

I removed the isatty(2) check since stderr is unrelated here.

Signed-off-by: Masahiro Yamada 
---

 scripts/kconfig/conf.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 358e2e4..c5318d3 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
 {
if (!fgets(str, size, in))
fprintf(stderr, "\nError in reading or end of file.\n");
+
+   if (!tty_stdio)
+   printf("%s", str);
 }
 
 static int conf_askvalue(struct symbol *sym, const char *def)
@@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
*def)
case oldaskconfig:
fflush(stdout);
xfgets(line, sizeof(line), stdin);
-   if (!tty_stdio)
-   printf("\n");
return 1;
default:
break;
@@ -495,7 +496,7 @@ int main(int ac, char **av)
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
 
-   tty_stdio = isatty(0) && isatty(1) && isatty(2);
+   tty_stdio = isatty(0) && isatty(1);
 
while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
if (opt == 's') {
-- 
2.7.4

[PATCH 1/2] kconfig: remove check_stdin()

[Masahiro Yamada]

Except silentoldconfig, valid_stdin is 1, so check_stdin() is no-op.

oldconfig and silentoldconfig work almost in the same way except that
the latter generates additional files.  Both ask users for input for
new symbols.

I do not know why only silentoldconfig requires stdio be tty.

  $ rm -f .config; touch .config
  $ yes "" | make oldconfig > stdout
  $ rm -f .config; touch .config
  $ yes "" | make silentoldconfig > stdout
  make[1]: *** [silentoldconfig] Error 1
  make: *** [silentoldconfig] Error 2
  $ tail -n 4 stdout
  Console input/output is redirected. Run 'make oldconfig' to update 
configuration.

  scripts/kconfig/Makefile:40: recipe for target 'silentoldconfig' failed
  Makefile:507: recipe for target 'silentoldconfig' failed

Redirection is useful, for example, for testing where we want to give
particular key inputs from a test file, then check the result.

Signed-off-by: Masahiro Yamada 
---

 scripts/kconfig/conf.c | 14 --
 1 file changed, 14 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 307bc3f..358e2e4 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -39,7 +39,6 @@ static enum input_mode input_mode = oldaskconfig;
 
 static int indent = 1;
 static int tty_stdio;
-static int valid_stdin = 1;
 static int sync_kconfig;
 static int conf_cnt;
 static char line[PATH_MAX];
@@ -72,16 +71,6 @@ static void strip(char *str)
*p-- = 0;
 }
 
-static void check_stdin(void)
-{
-   if (!valid_stdin) {
-   printf(_("aborted!\n\n"));
-   printf(_("Console input/output is redirected. "));
-   printf(_("Run 'make oldconfig' to update configuration.\n\n"));
-   exit(1);
-   }
-}
-
 /* Helper function to facilitate fgets() by Jean Sacren. */
 static void xfgets(char *str, int size, FILE *in)
 {
@@ -113,7 +102,6 @@ static int conf_askvalue(struct symbol *sym, const char 
*def)
printf("%s\n", def);
return 0;
}
-   check_stdin();
/* fall through */
case oldaskconfig:
fflush(stdout);
@@ -315,7 +303,6 @@ static int conf_choice(struct menu *menu)
printf("%d\n", cnt);
break;
}
-   check_stdin();
/* fall through */
case oldaskconfig:
fflush(stdout);
@@ -650,7 +637,6 @@ int main(int ac, char **av)
return 1;
}
}
-   valid_stdin = tty_stdio;
}
 
switch (input_mode) {
-- 
2.7.4

[PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Masahiro Yamada]

If stdio is not tty, conf_askvalue() puts additional new line to
prevent prompts are all concatenated into a single line.  This care
is missing in conf_choice(), so a 'choice' prompt and the next prompt
are shown in the same line.

Move the code into xfgets() to take care of all cases.  To improve
this more, echo stdin to stdout.  This clarifies what keys were input
from stdio and the stdout looks like as if it were from tty.

I removed the isatty(2) check since stderr is unrelated here.

Signed-off-by: Masahiro Yamada 
---

 scripts/kconfig/conf.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 358e2e4..c5318d3 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
 {
if (!fgets(str, size, in))
fprintf(stderr, "\nError in reading or end of file.\n");
+
+   if (!tty_stdio)
+   printf("%s", str);
 }
 
 static int conf_askvalue(struct symbol *sym, const char *def)
@@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
*def)
case oldaskconfig:
fflush(stdout);
xfgets(line, sizeof(line), stdin);
-   if (!tty_stdio)
-   printf("\n");
return 1;
default:
break;
@@ -495,7 +496,7 @@ int main(int ac, char **av)
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
 
-   tty_stdio = isatty(0) && isatty(1) && isatty(2);
+   tty_stdio = isatty(0) && isatty(1);
 
while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
if (opt == 's') {
-- 
2.7.4

[PATCH 1/2] kconfig: remove check_stdin()

[Masahiro Yamada]

Except silentoldconfig, valid_stdin is 1, so check_stdin() is no-op.

oldconfig and silentoldconfig work almost in the same way except that
the latter generates additional files.  Both ask users for input for
new symbols.

I do not know why only silentoldconfig requires stdio be tty.

  $ rm -f .config; touch .config
  $ yes "" | make oldconfig > stdout
  $ rm -f .config; touch .config
  $ yes "" | make silentoldconfig > stdout
  make[1]: *** [silentoldconfig] Error 1
  make: *** [silentoldconfig] Error 2
  $ tail -n 4 stdout
  Console input/output is redirected. Run 'make oldconfig' to update 
configuration.

  scripts/kconfig/Makefile:40: recipe for target 'silentoldconfig' failed
  Makefile:507: recipe for target 'silentoldconfig' failed

Redirection is useful, for example, for testing where we want to give
particular key inputs from a test file, then check the result.

Signed-off-by: Masahiro Yamada 
---

 scripts/kconfig/conf.c | 14 --
 1 file changed, 14 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 307bc3f..358e2e4 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -39,7 +39,6 @@ static enum input_mode input_mode = oldaskconfig;
 
 static int indent = 1;
 static int tty_stdio;
-static int valid_stdin = 1;
 static int sync_kconfig;
 static int conf_cnt;
 static char line[PATH_MAX];
@@ -72,16 +71,6 @@ static void strip(char *str)
*p-- = 0;
 }
 
-static void check_stdin(void)
-{
-   if (!valid_stdin) {
-   printf(_("aborted!\n\n"));
-   printf(_("Console input/output is redirected. "));
-   printf(_("Run 'make oldconfig' to update configuration.\n\n"));
-   exit(1);
-   }
-}
-
 /* Helper function to facilitate fgets() by Jean Sacren. */
 static void xfgets(char *str, int size, FILE *in)
 {
@@ -113,7 +102,6 @@ static int conf_askvalue(struct symbol *sym, const char 
*def)
printf("%s\n", def);
return 0;
}
-   check_stdin();
/* fall through */
case oldaskconfig:
fflush(stdout);
@@ -315,7 +303,6 @@ static int conf_choice(struct menu *menu)
printf("%d\n", cnt);
break;
}
-   check_stdin();
/* fall through */
case oldaskconfig:
fflush(stdout);
@@ -650,7 +637,6 @@ int main(int ac, char **av)
return 1;
}
}
-   valid_stdin = tty_stdio;
}
 
switch (input_mode) {
-- 
2.7.4

Re: [PATCH v2 2/5] clk: mediatek: modify MT7622 audsys to adapt MFD device

[Ryder Lee]

On Wed, 2018-02-07 at 16:18 +0100, Matthias Brugger wrote:
> 
> On 02/05/2018 05:07 AM, Ryder Lee wrote:
> > On Sat, 2018-02-03 at 15:26 +0100, Matthias Brugger wrote:
> >>
> >> On 01/31/2018 08:42 AM, Ryder Lee wrote:
> >>> As the new MFD parent is in place, switch probing method to adapt it.
> >>>
> >>> Signed-off-by: Ryder Lee 
> >>> ---
> >>>  drivers/clk/mediatek/clk-mt7622-aud.c | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/clk/mediatek/clk-mt7622-aud.c 
> >>> b/drivers/clk/mediatek/clk-mt7622-aud.c
> >>> index 13f752d..8c18536 100644
> >>> --- a/drivers/clk/mediatek/clk-mt7622-aud.c
> >>> +++ b/drivers/clk/mediatek/clk-mt7622-aud.c
> >>> @@ -142,11 +142,12 @@ static int clk_mt7622_audiosys_init(struct 
> >>> platform_device *pdev)
> >>>  {
> >>>   struct clk_onecell_data *clk_data;
> >>>   struct device_node *node = pdev->dev.of_node;
> >>> + struct device_node *pnode = pdev->dev.parent->of_node;
> >>>   int r;
> >>>  
> >>>   clk_data = mtk_alloc_clk_data(CLK_AUDIO_NR_CLK);
> >>>  
> >>> - mtk_clk_register_gates(node, audio_clks, ARRAY_SIZE(audio_clks),
> >>> + mtk_clk_register_gates(pnode, audio_clks, ARRAY_SIZE(audio_clks),
> >>
> >> We will need a fallback here for older device tree bindings.
> >>
> >> Regards,
> >> Matthias
> > 
> > Up to now, I didn't see both driver and bindings are used anywhere.
> > Should I still care about that?
> > 
> 
> Yes please, we have to ensure that newer kernel work with older bindings and
> vice-versa.

OKay. An unrelated question - is it possible to change the compatible
string (*-audsys-clk) without breaking backward compatible?

Re: [PATCH v2 2/5] clk: mediatek: modify MT7622 audsys to adapt MFD device

[Ryder Lee]

On Wed, 2018-02-07 at 16:18 +0100, Matthias Brugger wrote:
> 
> On 02/05/2018 05:07 AM, Ryder Lee wrote:
> > On Sat, 2018-02-03 at 15:26 +0100, Matthias Brugger wrote:
> >>
> >> On 01/31/2018 08:42 AM, Ryder Lee wrote:
> >>> As the new MFD parent is in place, switch probing method to adapt it.
> >>>
> >>> Signed-off-by: Ryder Lee 
> >>> ---
> >>>  drivers/clk/mediatek/clk-mt7622-aud.c | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/clk/mediatek/clk-mt7622-aud.c 
> >>> b/drivers/clk/mediatek/clk-mt7622-aud.c
> >>> index 13f752d..8c18536 100644
> >>> --- a/drivers/clk/mediatek/clk-mt7622-aud.c
> >>> +++ b/drivers/clk/mediatek/clk-mt7622-aud.c
> >>> @@ -142,11 +142,12 @@ static int clk_mt7622_audiosys_init(struct 
> >>> platform_device *pdev)
> >>>  {
> >>>   struct clk_onecell_data *clk_data;
> >>>   struct device_node *node = pdev->dev.of_node;
> >>> + struct device_node *pnode = pdev->dev.parent->of_node;
> >>>   int r;
> >>>  
> >>>   clk_data = mtk_alloc_clk_data(CLK_AUDIO_NR_CLK);
> >>>  
> >>> - mtk_clk_register_gates(node, audio_clks, ARRAY_SIZE(audio_clks),
> >>> + mtk_clk_register_gates(pnode, audio_clks, ARRAY_SIZE(audio_clks),
> >>
> >> We will need a fallback here for older device tree bindings.
> >>
> >> Regards,
> >> Matthias
> > 
> > Up to now, I didn't see both driver and bindings are used anywhere.
> > Should I still care about that?
> > 
> 
> Yes please, we have to ensure that newer kernel work with older bindings and
> vice-versa.

OKay. An unrelated question - is it possible to change the compatible
string (*-audsys-clk) without breaking backward compatible?

Re: [PATCH v2 4/5] dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device

[Ryder Lee]

On Wed, 2018-02-07 at 18:49 -0600, Rob Herring wrote:
> >> > diff --git 
> >> > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt 
> >> > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> >> > index 9b8f578..677af40 100644
> >> > --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> >> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> >> > @@ -1,22 +1,45 @@
> >> > -MediaTek AUDSYS controller
> >> > +MediaTek Audio Subsystem
> >> >  
> >> > +The audio subsystem is one of the multi-function blocks of MediaTek 
> >> > SoCs.
> >> > +It contains a system controller, which provides a number registers 
> >> > giving
> >> > +access to two features: AUDSYS clocks and Audio Front End (AFE) 
> >> > components.
> >> >
> >> > +For the top level node:
> >> > +- compatible: must be: "syscon", "simple-mfd";
> >>
> >> This should have some SoC specific compatible.
> >
> > As we don't have a specific driver (compatible string) for it and if we
> > need to add that I think the term '*-audsys' is very suitable here, but
> > unfortunately, it has already picked for clock driver (see child node).
> 
> Perhaps the clocks block should have "-clk" on the end or something.
> 
> Why do you really need to change this in the first place? You don't
> have to have DT child nodes to create child (struct) devices and child
> drivers.
> 

I'm not sure I get your point. Did you mean that we could have a parent
(which represents clock and its compatible is something like
"*-audsys","syscon","simple-mfd") with a subnode for audio function?

If so, there's no special reason. I just think it more specifically to
let people know we have an MFD (top node) and it has two sub-functions
(children) share the same region.

> >> > +Required sub-nodes:
> >> > +
> >> > +AUDSYS clocks:
> >> > +---
> >> >  The MediaTek AUDSYS controller provides various clocks to the system.
> >> >
> >> >  Required Properties:
> >> >
> >> >  - compatible: Should be one of:
> >> > -   - "mediatek,mt7622-audsys", "syscon"
> >> > +   - "mediatek,mt2701-audsys";
> >> > +   - "mediatek,mt7622-audsys";
> >> >  - #clock-cells: Must be 1
> >> >
> >> >  The AUDSYS controller uses the common clk binding from
> >> >  Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> >  The available clocks are defined in dt-bindings/clock/mt*-clk.h.
> >>
> >> There's no register range associated with the clocks? If there is, add a
> >> reg property.
> >
> > No, we don't need reg property here, as the two sub-drivers will obtain
> > the regmap  which is propagated from the parent.
> 
> I know regmap doesn't need it. That's not what I asked. If you have a
> range of registers for the clocks, then define that in reg. Only if
> the clock control bits are mixed up with other functions within the
> same registers, then you can omit it.
> 

Oh okay, I didn't read the mail well.  We have four or five registers
for the clocks, but one of them (offset:0x634) is mixed up with the
audio function (bit 17-19), so I omit it.

Ryder.

Re: [PATCH v2 4/5] dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device

[Ryder Lee]

On Wed, 2018-02-07 at 18:49 -0600, Rob Herring wrote:
> >> > diff --git 
> >> > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt 
> >> > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> >> > index 9b8f578..677af40 100644
> >> > --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> >> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> >> > @@ -1,22 +1,45 @@
> >> > -MediaTek AUDSYS controller
> >> > +MediaTek Audio Subsystem
> >> >  
> >> > +The audio subsystem is one of the multi-function blocks of MediaTek 
> >> > SoCs.
> >> > +It contains a system controller, which provides a number registers 
> >> > giving
> >> > +access to two features: AUDSYS clocks and Audio Front End (AFE) 
> >> > components.
> >> >
> >> > +For the top level node:
> >> > +- compatible: must be: "syscon", "simple-mfd";
> >>
> >> This should have some SoC specific compatible.
> >
> > As we don't have a specific driver (compatible string) for it and if we
> > need to add that I think the term '*-audsys' is very suitable here, but
> > unfortunately, it has already picked for clock driver (see child node).
> 
> Perhaps the clocks block should have "-clk" on the end or something.
> 
> Why do you really need to change this in the first place? You don't
> have to have DT child nodes to create child (struct) devices and child
> drivers.
> 

I'm not sure I get your point. Did you mean that we could have a parent
(which represents clock and its compatible is something like
"*-audsys","syscon","simple-mfd") with a subnode for audio function?

If so, there's no special reason. I just think it more specifically to
let people know we have an MFD (top node) and it has two sub-functions
(children) share the same region.

> >> > +Required sub-nodes:
> >> > +
> >> > +AUDSYS clocks:
> >> > +---
> >> >  The MediaTek AUDSYS controller provides various clocks to the system.
> >> >
> >> >  Required Properties:
> >> >
> >> >  - compatible: Should be one of:
> >> > -   - "mediatek,mt7622-audsys", "syscon"
> >> > +   - "mediatek,mt2701-audsys";
> >> > +   - "mediatek,mt7622-audsys";
> >> >  - #clock-cells: Must be 1
> >> >
> >> >  The AUDSYS controller uses the common clk binding from
> >> >  Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> >  The available clocks are defined in dt-bindings/clock/mt*-clk.h.
> >>
> >> There's no register range associated with the clocks? If there is, add a
> >> reg property.
> >
> > No, we don't need reg property here, as the two sub-drivers will obtain
> > the regmap  which is propagated from the parent.
> 
> I know regmap doesn't need it. That's not what I asked. If you have a
> range of registers for the clocks, then define that in reg. Only if
> the clock control bits are mixed up with other functions within the
> same registers, then you can omit it.
> 

Oh okay, I didn't read the mail well.  We have four or five registers
for the clocks, but one of them (offset:0x634) is mixed up with the
audio function (bit 17-19), so I omit it.

Ryder.

[PATCH] selftests/x86: Remove 5lvl test from Makefile

[Daniel Díaz]

The 5lvl.c test file was incorporated into another one in
selftests/vm (va_128TBswitch.c) in commit 235266b8.

Signed-off-by: Daniel Díaz 
---
 tools/testing/selftests/x86/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/x86/Makefile 
b/tools/testing/selftests/x86/Makefile
index 10ca46d..ce2615a 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -11,7 +11,7 @@ TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs 
syscall_nt ptrace_sysc
 TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso 
unwind_vdso \
test_FCMOV test_FCOMI test_FISTTP \
vdso_restorer
-TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip 5lvl
+TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip
 
 TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
 TARGETS_C_64BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_64BIT_ONLY)
-- 
2.7.4

[PATCH] selftests/x86: Remove 5lvl test from Makefile

[Daniel Díaz]

The 5lvl.c test file was incorporated into another one in
selftests/vm (va_128TBswitch.c) in commit 235266b8.

Signed-off-by: Daniel Díaz 
---
 tools/testing/selftests/x86/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/x86/Makefile 
b/tools/testing/selftests/x86/Makefile
index 10ca46d..ce2615a 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -11,7 +11,7 @@ TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs 
syscall_nt ptrace_sysc
 TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso 
unwind_vdso \
test_FCMOV test_FCOMI test_FISTTP \
vdso_restorer
-TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip 5lvl
+TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip
 
 TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
 TARGETS_C_64BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_64BIT_ONLY)
-- 
2.7.4

Re: [PATCH 04/18] tracing/x86: Add arch_get_func_args() function

[Namhyung Kim]

Hi Steve,

On Fri, Feb 02, 2018 at 06:05:02PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" 
> 
> Add function to get the function arguments from pt_regs.
> 
> Signed-off-by: Steven Rostedt (VMware) 
> ---
>  arch/x86/kernel/ftrace.c | 28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 01ebcb6f263e..5e845c8cf89d 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -46,6 +46,34 @@ int ftrace_arch_code_modify_post_process(void)
>   return 0;
>  }
>  
> +int arch_get_func_args(struct pt_regs *regs,
> +int start, int end, long *args)
> +{
> +#ifdef CONFIG_X86_64
> +# define MAX_ARGS 6
> +# define INIT_REGS   \
> + {   regs->di, regs->si, regs->dx,   \
> + regs->cx, regs->r8, regs->r9\
> + }
> +#else
> +# define MAX_ARGS 3
> +# define INIT_REGS   \
> + {   regs->ax, regs->dx, regs->cx}
> +#endif
> + if (!regs)
> + return MAX_ARGS;
> +
> + {
> + long pt_args[] = INIT_REGS;
> + int i;
> +
> + for (i = start; i <= end && i < MAX_ARGS; i++)


I expected it being 'i < end' based on your description.

Thanks,
Namhyung


> + args[i - start] = pt_args[i];
> +
> + return i - start;
> + }
> +}
> +
>  union ftrace_code_union {
>   char code[MCOUNT_INSN_SIZE];
>   struct {
> -- 
> 2.15.1
> 
> 

Re: [PATCH 04/18] tracing/x86: Add arch_get_func_args() function

[Namhyung Kim]

Hi Steve,

On Fri, Feb 02, 2018 at 06:05:02PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" 
> 
> Add function to get the function arguments from pt_regs.
> 
> Signed-off-by: Steven Rostedt (VMware) 
> ---
>  arch/x86/kernel/ftrace.c | 28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 01ebcb6f263e..5e845c8cf89d 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -46,6 +46,34 @@ int ftrace_arch_code_modify_post_process(void)
>   return 0;
>  }
>  
> +int arch_get_func_args(struct pt_regs *regs,
> +int start, int end, long *args)
> +{
> +#ifdef CONFIG_X86_64
> +# define MAX_ARGS 6
> +# define INIT_REGS   \
> + {   regs->di, regs->si, regs->dx,   \
> + regs->cx, regs->r8, regs->r9\
> + }
> +#else
> +# define MAX_ARGS 3
> +# define INIT_REGS   \
> + {   regs->ax, regs->dx, regs->cx}
> +#endif
> + if (!regs)
> + return MAX_ARGS;
> +
> + {
> + long pt_args[] = INIT_REGS;
> + int i;
> +
> + for (i = start; i <= end && i < MAX_ARGS; i++)


I expected it being 'i < end' based on your description.

Thanks,
Namhyung


> + args[i - start] = pt_args[i];
> +
> + return i - start;
> + }
> +}
> +
>  union ftrace_code_union {
>   char code[MCOUNT_INSN_SIZE];
>   struct {
> -- 
> 2.15.1
> 
> 

[PATCH] ath9k: turn on btcoex_enable as default

[Kai-Heng Feng]

Without btcoex_enable, WiFi activies make both WiFi and Bluetooth
unstable if there's a bluetooth connection.

Enable this option when bt_ant_diversity is disabled.

BugLink: https://bugs.launchpad.net/bugs/1746164
Signed-off-by: Kai-Heng Feng 
---
 drivers/net/wireless/ath/ath9k/init.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index e479fae5aab9..f8f6b091a077 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -56,7 +56,7 @@ static int ath9k_led_active_high = -1;
 module_param_named(led_active_high, ath9k_led_active_high, int, 0444);
 MODULE_PARM_DESC(led_active_high, "Invert LED polarity");
 
-static int ath9k_btcoex_enable;
+static int ath9k_btcoex_enable = 1;
 module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
 MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
 
@@ -693,7 +693,6 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
common->hw = sc->hw;
common->priv = sc;
common->debug_mask = ath9k_debug;
-   common->btcoex_enabled = ath9k_btcoex_enable == 1;
common->disable_ani = false;
 
/*
@@ -715,14 +714,17 @@ static int ath9k_init_softc(u16 devid, struct ath_softc 
*sc,
/*
 * Enable WLAN/BT RX Antenna diversity only when:
 *
-* - BTCOEX is disabled.
 * - the user manually requests the feature.
 * - the HW cap is set using the platform data.
 */
-   if (!common->btcoex_enabled && ath9k_bt_ant_diversity &&
+   if (ath9k_bt_ant_diversity &&
(pCap->hw_caps & ATH9K_HW_CAP_BT_ANT_DIV))
common->bt_ant_diversity = 1;
 
+   /* Enable btcoex when ant_diversity is disabled */
+   if (!common->bt_ant_diversity && ath9k_btcoex_enable)
+   common->btcoex_enabled = 1;
+
spin_lock_init(>cc_lock);
spin_lock_init(>intr_lock);
spin_lock_init(>sc_serial_rw);
-- 
2.15.1

[PATCH] ath9k: turn on btcoex_enable as default

[Kai-Heng Feng]

Without btcoex_enable, WiFi activies make both WiFi and Bluetooth
unstable if there's a bluetooth connection.

Enable this option when bt_ant_diversity is disabled.

BugLink: https://bugs.launchpad.net/bugs/1746164
Signed-off-by: Kai-Heng Feng 
---
 drivers/net/wireless/ath/ath9k/init.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index e479fae5aab9..f8f6b091a077 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -56,7 +56,7 @@ static int ath9k_led_active_high = -1;
 module_param_named(led_active_high, ath9k_led_active_high, int, 0444);
 MODULE_PARM_DESC(led_active_high, "Invert LED polarity");
 
-static int ath9k_btcoex_enable;
+static int ath9k_btcoex_enable = 1;
 module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
 MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
 
@@ -693,7 +693,6 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
common->hw = sc->hw;
common->priv = sc;
common->debug_mask = ath9k_debug;
-   common->btcoex_enabled = ath9k_btcoex_enable == 1;
common->disable_ani = false;
 
/*
@@ -715,14 +714,17 @@ static int ath9k_init_softc(u16 devid, struct ath_softc 
*sc,
/*
 * Enable WLAN/BT RX Antenna diversity only when:
 *
-* - BTCOEX is disabled.
 * - the user manually requests the feature.
 * - the HW cap is set using the platform data.
 */
-   if (!common->btcoex_enabled && ath9k_bt_ant_diversity &&
+   if (ath9k_bt_ant_diversity &&
(pCap->hw_caps & ATH9K_HW_CAP_BT_ANT_DIV))
common->bt_ant_diversity = 1;
 
+   /* Enable btcoex when ant_diversity is disabled */
+   if (!common->bt_ant_diversity && ath9k_btcoex_enable)
+   common->btcoex_enabled = 1;
+
spin_lock_init(>cc_lock);
spin_lock_init(>intr_lock);
spin_lock_init(>sc_serial_rw);
-- 
2.15.1

Re: [PATCH RFC 2/2] mmc: sdhci-msm: support voltage pad switching

[Vijay Viswanath]

On 2/3/2018 3:21 AM, Bjorn Andersson wrote:

On Thu 18 Jan 00:05 PST 2018, Vijay Viswanath wrote:


From: Krishna Konda 

The PADs for sdhc controller are dual-voltage that support 3v/1.8v.
Those PADs have a control signal (io_pad_pwr_switch/mode18 ) that
indicates whether the PAD works in 3v or 1.8v.

SDHC core on msm platforms should have IO_PAD_PWR_SWITCH bit set/unset
based on actual voltage used for IO lines. So when power irq is
triggered for io high or io low, the driver should check the voltages
supported and set the pad accordingly.



I'll try to find some time to check that this doesn't break 8916 and
8974...again...



Thanks! Btwn, I had tested the code in db410c.


Signed-off-by: Krishna Konda 
Signed-off-by: Venkat Gopalakrishnan 
Signed-off-by: Vijay Viswanath 
---
  drivers/mmc/host/sdhci-msm.c | 38 ++
  1 file changed, 38 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 5c23e92..f5728a8 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -78,6 +78,8 @@
  #define CORE_HC_MCLK_SEL_DFLT (2 << 8)
  #define CORE_HC_MCLK_SEL_HS400(3 << 8)
  #define CORE_HC_MCLK_SEL_MASK (3 << 8)
+#define CORE_IO_PAD_PWR_SWITCH_EN  (1 << 15)
+#define CORE_IO_PAD_PWR_SWITCH  (1 << 16)
  #define CORE_HC_SELECT_IN_EN  BIT(18)
  #define CORE_HC_SELECT_IN_HS400   (6 << 19)
  #define CORE_HC_SELECT_IN_MASK(7 << 19)
@@ -1166,6 +1168,35 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host 
*host, int irq)
 */
writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
  
+	/*

+* SDHC has core_mem and hc_mem device memory and these memory
+* addresses do not fall within 1KB region. Hence, any update to
+* core_mem address space would require an mb() to ensure this gets
+* completed before its next update to registers within hc_mem.
+*/
+   mb();


If you just use writel() instead of writel_relaxed() you don't need to
sprinkle the driver with comments like this. And you really should be
able to just say "Ensure ordering between core_mem and hc_mem writes" if
you really feel like making it explicit.


+   /*
+* We should unset IO PAD PWR switch only if the register write can
+* set IO lines high and the regulator also switches to 3 V.
+* Else, we should keep the IO PAD PWR switch set.
+* This is applicable to certain targets where eMMC vccq supply is only
+* 1.8V. In such targets, even during REQ_IO_HIGH, the IO PAD PWR
+* switch must be kept set to reflect actual regulator voltage. This
+* way, during initialization of controllers with only 1.8V, we will
+* set the IO PAD bit without waiting for a REQ_IO_LOW.
+*/
+   if ((io_level & REQ_IO_HIGH) && (msm_host->caps_0 & CORE_3_0V_SUPPORT))
+   writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC) &
+   ~CORE_IO_PAD_PWR_SWITCH), host->ioaddr +
+   CORE_VENDOR_SPEC);


Please split this up in read, modify and write operations.



Will do


+   else if ((io_level & REQ_IO_LOW) ||
+   (msm_host->caps_0 & CORE_1_8V_SUPPORT))
+   writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC) |
+   CORE_IO_PAD_PWR_SWITCH), host->ioaddr +
+   CORE_VENDOR_SPEC);
+   /* Ensure that the IO PAD switches are updated before proceeding */


That's not what "mb()" does, it ensures that any writes that was done
before this line will hit the hardware before any writes that is done
after this line.



Will update the comments.


But again, using writel() would save us from doing this explicitly
throughout the code.


+   mb();
+
if (pwr_state)
msm_host->curr_pwr_state = pwr_state;
if (io_level)
@@ -1518,6 +1549,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
}
  
  	/*

+* Set the PAD_PWR_SWITCH_EN bit so that the PAD_PWR_SWITCH bit can
+* be used as required later on.
+*/
+   writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC) |
+   CORE_IO_PAD_PWR_SWITCH_EN), host->ioaddr +
+   CORE_VENDOR_SPEC);


Please rewrite as 3 operations.

Do we need to set the pwr switch value as well? Or we're fine relying on
the existing value here?


After the IO_PAD_PWR_SWTCH is enabled, we will call 
sdhci_msm_handle_pwr_irq. If there is any pending power irq interrupt, 
that will set the appropriate pwr switch value. Otherwise, an 
appropriate value will get set during REQ_BUS_ON event.




Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to 

Re: [PATCH RFC 2/2] mmc: sdhci-msm: support voltage pad switching

[Vijay Viswanath]

On 2/3/2018 3:21 AM, Bjorn Andersson wrote:

On Thu 18 Jan 00:05 PST 2018, Vijay Viswanath wrote:


From: Krishna Konda 

The PADs for sdhc controller are dual-voltage that support 3v/1.8v.
Those PADs have a control signal (io_pad_pwr_switch/mode18 ) that
indicates whether the PAD works in 3v or 1.8v.

SDHC core on msm platforms should have IO_PAD_PWR_SWITCH bit set/unset
based on actual voltage used for IO lines. So when power irq is
triggered for io high or io low, the driver should check the voltages
supported and set the pad accordingly.



I'll try to find some time to check that this doesn't break 8916 and
8974...again...



Thanks! Btwn, I had tested the code in db410c.


Signed-off-by: Krishna Konda 
Signed-off-by: Venkat Gopalakrishnan 
Signed-off-by: Vijay Viswanath 
---
  drivers/mmc/host/sdhci-msm.c | 38 ++
  1 file changed, 38 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 5c23e92..f5728a8 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -78,6 +78,8 @@
  #define CORE_HC_MCLK_SEL_DFLT (2 << 8)
  #define CORE_HC_MCLK_SEL_HS400(3 << 8)
  #define CORE_HC_MCLK_SEL_MASK (3 << 8)
+#define CORE_IO_PAD_PWR_SWITCH_EN  (1 << 15)
+#define CORE_IO_PAD_PWR_SWITCH  (1 << 16)
  #define CORE_HC_SELECT_IN_EN  BIT(18)
  #define CORE_HC_SELECT_IN_HS400   (6 << 19)
  #define CORE_HC_SELECT_IN_MASK(7 << 19)
@@ -1166,6 +1168,35 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host 
*host, int irq)
 */
writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
  
+	/*

+* SDHC has core_mem and hc_mem device memory and these memory
+* addresses do not fall within 1KB region. Hence, any update to
+* core_mem address space would require an mb() to ensure this gets
+* completed before its next update to registers within hc_mem.
+*/
+   mb();


If you just use writel() instead of writel_relaxed() you don't need to
sprinkle the driver with comments like this. And you really should be
able to just say "Ensure ordering between core_mem and hc_mem writes" if
you really feel like making it explicit.


+   /*
+* We should unset IO PAD PWR switch only if the register write can
+* set IO lines high and the regulator also switches to 3 V.
+* Else, we should keep the IO PAD PWR switch set.
+* This is applicable to certain targets where eMMC vccq supply is only
+* 1.8V. In such targets, even during REQ_IO_HIGH, the IO PAD PWR
+* switch must be kept set to reflect actual regulator voltage. This
+* way, during initialization of controllers with only 1.8V, we will
+* set the IO PAD bit without waiting for a REQ_IO_LOW.
+*/
+   if ((io_level & REQ_IO_HIGH) && (msm_host->caps_0 & CORE_3_0V_SUPPORT))
+   writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC) &
+   ~CORE_IO_PAD_PWR_SWITCH), host->ioaddr +
+   CORE_VENDOR_SPEC);


Please split this up in read, modify and write operations.



Will do


+   else if ((io_level & REQ_IO_LOW) ||
+   (msm_host->caps_0 & CORE_1_8V_SUPPORT))
+   writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC) |
+   CORE_IO_PAD_PWR_SWITCH), host->ioaddr +
+   CORE_VENDOR_SPEC);
+   /* Ensure that the IO PAD switches are updated before proceeding */


That's not what "mb()" does, it ensures that any writes that was done
before this line will hit the hardware before any writes that is done
after this line.



Will update the comments.


But again, using writel() would save us from doing this explicitly
throughout the code.


+   mb();
+
if (pwr_state)
msm_host->curr_pwr_state = pwr_state;
if (io_level)
@@ -1518,6 +1549,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
}
  
  	/*

+* Set the PAD_PWR_SWITCH_EN bit so that the PAD_PWR_SWITCH bit can
+* be used as required later on.
+*/
+   writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC) |
+   CORE_IO_PAD_PWR_SWITCH_EN), host->ioaddr +
+   CORE_VENDOR_SPEC);


Please rewrite as 3 operations.

Do we need to set the pwr switch value as well? Or we're fine relying on
the existing value here?


After the IO_PAD_PWR_SWTCH is enabled, we will call 
sdhci_msm_handle_pwr_irq. If there is any pending power irq interrupt, 
that will set the appropriate pwr switch value. Otherwise, an 
appropriate value will get set during REQ_BUS_ON event.




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

Re: [PATCH] selftests/android: Fix line continuation in Makefile

[Pintu Kumar]

On Wed, Feb 7, 2018 at 5:22 AM, Daniel Díaz  wrote:
> The Makefile lacks a couple of line continuation backslashes
> in an `if' clause, which can make the subsequent rsync
> command go awry over the whole filesystem (`rsync -a / /`).
>
>   /bin/sh: -c: line 5: syntax error: unexpected end of file
>   make[1]: [all] Error 1 (ignored)
>   TEST=$DIR"_test.sh"; \
>   if [ -e $DIR/$TEST ]; then
>   /bin/sh: -c: line 2: syntax error: unexpected end of file
>   make[1]: [all] Error 1 (ignored)
>   rsync -a $DIR/$TEST $BUILD_TARGET/;
>   [...a myriad of:]
>   [  rsync: readlink_stat("...") failed: Permission denied (13)]
>   [  skipping non-regular file "..."]
>   [  rsync: opendir "..." failed: Permission denied (13)]
>   [and many other errors...]
>   fi
>   make[1]: fi: Command not found
>   make[1]: [all] Error 127 (ignored)
>   done
>   make[1]: done: Command not found
>   make[1]: [all] Error 127 (ignored)
>
> Signed-off-by: Daniel Díaz 
> ---
>  tools/testing/selftests/android/Makefile | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/android/Makefile 
> b/tools/testing/selftests/android/Makefile
> index 1a74922..f6304d2 100644
> --- a/tools/testing/selftests/android/Makefile
> +++ b/tools/testing/selftests/android/Makefile
> @@ -11,11 +11,11 @@ all:
> BUILD_TARGET=$(OUTPUT)/$$DIR;   \
> mkdir $$BUILD_TARGET  -p;   \
> make OUTPUT=$$BUILD_TARGET -C $$DIR $@;\
> -   #SUBDIR test prog name should be in the form: SUBDIR_test.sh
> +   #SUBDIR test prog name should be in the form: SUBDIR_test.sh \
> TEST=$$DIR"_test.sh"; \
> -   if [ -e $$DIR/$$TEST ]; then
> -   rsync -a $$DIR/$$TEST $$BUILD_TARGET/;
> -   fi
> +   if [ -e $$DIR/$$TEST ]; then \
> +   rsync -a $$DIR/$$TEST $$BUILD_TARGET/; \
> +   fi \
> done
>
>  override define RUN_TESTS

Acked-by: Pintu Agarwal 

> --
> 2.7.4
>

Re: [PATCH] selftests/android: Fix line continuation in Makefile

[Pintu Kumar]

On Wed, Feb 7, 2018 at 5:22 AM, Daniel Díaz  wrote:
> The Makefile lacks a couple of line continuation backslashes
> in an `if' clause, which can make the subsequent rsync
> command go awry over the whole filesystem (`rsync -a / /`).
>
>   /bin/sh: -c: line 5: syntax error: unexpected end of file
>   make[1]: [all] Error 1 (ignored)
>   TEST=$DIR"_test.sh"; \
>   if [ -e $DIR/$TEST ]; then
>   /bin/sh: -c: line 2: syntax error: unexpected end of file
>   make[1]: [all] Error 1 (ignored)
>   rsync -a $DIR/$TEST $BUILD_TARGET/;
>   [...a myriad of:]
>   [  rsync: readlink_stat("...") failed: Permission denied (13)]
>   [  skipping non-regular file "..."]
>   [  rsync: opendir "..." failed: Permission denied (13)]
>   [and many other errors...]
>   fi
>   make[1]: fi: Command not found
>   make[1]: [all] Error 127 (ignored)
>   done
>   make[1]: done: Command not found
>   make[1]: [all] Error 127 (ignored)
>
> Signed-off-by: Daniel Díaz 
> ---
>  tools/testing/selftests/android/Makefile | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/android/Makefile 
> b/tools/testing/selftests/android/Makefile
> index 1a74922..f6304d2 100644
> --- a/tools/testing/selftests/android/Makefile
> +++ b/tools/testing/selftests/android/Makefile
> @@ -11,11 +11,11 @@ all:
> BUILD_TARGET=$(OUTPUT)/$$DIR;   \
> mkdir $$BUILD_TARGET  -p;   \
> make OUTPUT=$$BUILD_TARGET -C $$DIR $@;\
> -   #SUBDIR test prog name should be in the form: SUBDIR_test.sh
> +   #SUBDIR test prog name should be in the form: SUBDIR_test.sh \
> TEST=$$DIR"_test.sh"; \
> -   if [ -e $$DIR/$$TEST ]; then
> -   rsync -a $$DIR/$$TEST $$BUILD_TARGET/;
> -   fi
> +   if [ -e $$DIR/$$TEST ]; then \
> +   rsync -a $$DIR/$$TEST $$BUILD_TARGET/; \
> +   fi \
> done
>
>  override define RUN_TESTS

Acked-by: Pintu Agarwal 

> --
> 2.7.4
>

Re: [PATCH] selftests/android: Fix line continuation in Makefile

[Pintu Kumar]

On Wed, Feb 7, 2018 at 10:46 PM, Daniel Díaz Rodríguez
 wrote:
> Hello!
>
>
> On 7 February 2018 at 00:01, Pintu Kumar  wrote:
> [...]
>> Thanks for your patch.
>> However, I have copied this Makefile from
>> tools/testing/selftests/futex/Makefile before modifying it.
>> If there is a problem with backslash then the same problem must be
>> there in futex Makefile as well.
>
> There is. Patch coming.

OK

>
>
> [...]
>> Also is it because of make version ?
>> Can you check your make version ?
>
> It's make 3.81.

My make version was 4.1.
So I did not face any problem. I guess this problem is fixed from make
version 4.x on wards.
However, for backward compatibility we can include these change.

So, its looks good for me, however someone else must approve it.


>
> Thanks and greetings!
>
> Daniel Díaz
> daniel.d...@linaro.org

Re: [PATCH] selftests/android: Fix line continuation in Makefile

[Pintu Kumar]

On Wed, Feb 7, 2018 at 10:46 PM, Daniel Díaz Rodríguez
 wrote:
> Hello!
>
>
> On 7 February 2018 at 00:01, Pintu Kumar  wrote:
> [...]
>> Thanks for your patch.
>> However, I have copied this Makefile from
>> tools/testing/selftests/futex/Makefile before modifying it.
>> If there is a problem with backslash then the same problem must be
>> there in futex Makefile as well.
>
> There is. Patch coming.

OK

>
>
> [...]
>> Also is it because of make version ?
>> Can you check your make version ?
>
> It's make 3.81.

My make version was 4.1.
So I did not face any problem. I guess this problem is fixed from make
version 4.x on wards.
However, for backward compatibility we can include these change.

So, its looks good for me, however someone else must approve it.


>
> Thanks and greetings!
>
> Daniel Díaz
> daniel.d...@linaro.org

[PATCH] x86/kvm/vmx: Don't halt vcpu when L1 is injecting events to L2

[Chao Gao]

Although L2 is in halt state, it will be in the active state after
VM entry if the VM entry is vectoring. Halting the vcpu here means
the event won't be injected to L2 and this decision isn't reported
to L1. Thus L0 drops an event that should be injected to L2.

Because virtual interrupt delivery may wake L2 vcpu, if VID is enabled,
do the same thing -- don't halt L2.

Signed-off-by: Chao Gao 
---
 arch/x86/kvm/vmx.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index bb5b488..e1fe4e4 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10985,8 +10985,14 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
if (ret)
return ret;
 
-   if (vmcs12->guest_activity_state == GUEST_ACTIVITY_HLT)
-   return kvm_vcpu_halt(vcpu);
+   if (vmcs12->guest_activity_state == GUEST_ACTIVITY_HLT) {
+   u32 intr_info = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
+   u32 exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
+
+   if (!(intr_info & VECTORING_INFO_VALID_MASK) &&
+   !(exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY))
+   return kvm_vcpu_halt(vcpu);
+   }
 
vmx->nested.nested_run_pending = 1;
 
-- 
1.9.1

[PATCH] x86/kvm/vmx: Don't halt vcpu when L1 is injecting events to L2

[Chao Gao]

Although L2 is in halt state, it will be in the active state after
VM entry if the VM entry is vectoring. Halting the vcpu here means
the event won't be injected to L2 and this decision isn't reported
to L1. Thus L0 drops an event that should be injected to L2.

Because virtual interrupt delivery may wake L2 vcpu, if VID is enabled,
do the same thing -- don't halt L2.

Signed-off-by: Chao Gao 
---
 arch/x86/kvm/vmx.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index bb5b488..e1fe4e4 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10985,8 +10985,14 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
if (ret)
return ret;
 
-   if (vmcs12->guest_activity_state == GUEST_ACTIVITY_HLT)
-   return kvm_vcpu_halt(vcpu);
+   if (vmcs12->guest_activity_state == GUEST_ACTIVITY_HLT) {
+   u32 intr_info = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
+   u32 exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
+
+   if (!(intr_info & VECTORING_INFO_VALID_MASK) &&
+   !(exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY))
+   return kvm_vcpu_halt(vcpu);
+   }
 
vmx->nested.nested_run_pending = 1;
 
-- 
1.9.1