* Re: [GIT PULL] fallthrough pseudo-keyword macro conversions for 5.9-rc3
From: Gustavo A. R. Silva @ 2020-08-24 20:04 UTC (permalink / raw)
To: Nathan Chancellor, Gustavo A. R. Silva
Cc: linuxppc-dev, Linus Torvalds, Kees Cook,
Linux Kernel Mailing List
In-Reply-To: <20200824194335.GA4082027@ubuntu-n2-xlarge-x86>
Hi Nathan,
On 8/24/20 14:43, Nathan Chancellor wrote:
>> Gustavo A. R. Silva (1):
>> treewide: Use fallthrough pseudo-keyword
>
> $ scripts/config --file arch/powerpc/configs/powernv_defconfig -e KERNEL_XZ
>
> $ make -skj"$(nproc)" ARCH=powerpc CROSS_COMPILE=powerpc64le-linux- distclean powernv_defconfig zImage
> ...
> In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:234,
> from arch/powerpc/boot/decompress.c:38:
> arch/powerpc/boot/../../../lib/xz/xz_dec_stream.c: In function 'dec_main':
> arch/powerpc/boot/../../../lib/xz/xz_dec_stream.c:586:4: error: 'fallthrough' undeclared (first use in this function)
> 586 | fallthrough;
> | ^~~~~~~~~~~
> arch/powerpc/boot/../../../lib/xz/xz_dec_stream.c:586:4: note: each undeclared identifier is reported only once for each function it appears in
> In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:235,
> from arch/powerpc/boot/decompress.c:38:
> arch/powerpc/boot/../../../lib/xz/xz_dec_lzma2.c: In function 'xz_dec_lzma2_run':
> arch/powerpc/boot/../../../lib/xz/xz_dec_lzma2.c:1046:4: error: 'fallthrough' undeclared (first use in this function)
> 1046 | fallthrough;
> | ^~~~~~~~~~~
> make[2]: *** [arch/powerpc/boot/Makefile:215: arch/powerpc/boot/decompress.o] Error 1
> make[2]: Target 'arch/powerpc/boot/zImage' not remade because of errors.
> make[1]: *** [arch/powerpc/Makefile:295: zImage] Error 2
> make: *** [Makefile:335: __build_one_by_one] Error 2
> make: Target 'distclean' not remade because of errors.
> make: Target 'powernv_defconfig' not remade because of errors.
> make: Target 'zImage' not remade because of errors.
>
> This will end up affecting distribution configurations such as Debian
> and OpenSUSE according to my testing. I am not sure what the solution
> is, the PowerPC wrapper does not set -D__KERNEL__ so I am not sure that
> compiler_attributes.h can be safely included. Adding Michael and
> linuxppc-dev to CC.
>
Thanks for the report. I think, for now, the best solution is to
use /* fall through */ comments instead of the pseudo-keyword in
lib/
I'll send a fix for that right away.
Thanks
--
Gustavo
^ permalink raw reply
* Re: [GIT PULL] fallthrough pseudo-keyword macro conversions for 5.9-rc3
From: Nathan Chancellor @ 2020-08-24 19:43 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: linuxppc-dev, Linus Torvalds, Kees Cook,
Linux Kernel Mailing List
In-Reply-To: <20200824034841.GA29995@embeddedor>
On Sun, Aug 23, 2020 at 10:48:41PM -0500, Gustavo A. R. Silva wrote:
> Hi Linus,
>
> Not sure what the problem was with my pull-request for -rc2. So, I'm giving
> this a second try because I think it is worth it.
>
> I have build-tested this patch on 10 different architectures: x86_64, i386,
> arm64, powerpc, s390, sparc64, sh, m68k, powerpc64 and alpha (allyesconfig/
> allmodconfig for all of them). This is in linux-next already and kernel
> test robot has also helped me to successfully build-test early versions
> of this patch[2][3][4][5]. This patch does not introduce any new warnings.
>
> Thank you
> --
> Gustavo
>
> [1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
> [2] https://lore.kernel.org/lkml/5f3cc99a.HgvOW3rH0mD0RmkM%25lkp@intel.com/
> [3] https://lore.kernel.org/lkml/5f3dd1d2.l1axczH+t4hMBZ63%25lkp@intel.com/
> [4] https://lore.kernel.org/lkml/5f3e977a.mwYHUIObbR4SHr0B%25lkp@intel.com/
> [5] https://lore.kernel.org/lkml/5f3f9e1c.qsyb%2FaySkiXNpkO4%25lkp@intel.com/
>
>
> The following changes since commit d012a7190fc1fd72ed48911e77ca97ba4521bccd:
>
> Linux 5.9-rc2 (2020-08-23 14:08:43 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git tags/fallthrough-pseudo-keyword-5.9-rc3
>
> for you to fetch changes up to df561f6688fef775baa341a0f5d960becd248b11:
>
> treewide: Use fallthrough pseudo-keyword (2020-08-23 17:36:59 -0500)
>
> ----------------------------------------------------------------
> fallthrough pseudo-keyword macro conversions for 5.9-rc3
>
> Hi Linus,
>
> Please, pull the following tree-wide patch that replaces tons (2484) of
> /* fall through */ comments, and its variants, with the new pseudo-keyword
> macro fallthrough[1]. Also, remove unnecessary fall-through markings when
> it is the case.
>
> There are currently 1167 intances of this fallthrough pseudo-keyword
> macro in mainline (5.9-rc2), that have been introduced over the last
> couple of development cycles:
>
> $ git grep -nw 'fallthrough;' | wc -l
> 1167
>
> The global adoption of the fallthrough pseudo-keyword is something certain
> to happen; so, better sooner than later. :) This will also save everybody's
> time and thousands of lines of unnecessarily repetitive changelog text.
>
> After applying this patch on top of 5.9-rc2, we'll have a total of 3651
> instances of this macro:
>
> $ git grep -nw 'fallthrough;' | wc -l
> 3651
>
> This treewide patch doesn't address ALL fall-through markings in all
> subsystems at once because I have previously sent out patches for some of
> such subsystems separately, and I will follow up on them; however, this
> definitely contributes most of the work needed to replace all the
> fall-through markings with the fallthrough pseudo-keyword macro in the
> whole codebase.
>
> I have build-tested this patch on 10 different architectures: x86_64, i386,
> arm64, powerpc, s390, sparc64, sh, m68k, powerpc64 and alpha (allyesconfig
> for all of them). This is in linux-next already and kernel test robot has
> also helped me to successfully build-test early versions of this
> patch[2][3][4][5].
>
> Thanks
> --
> [1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
> [2] https://lore.kernel.org/lkml/5f3cc99a.HgvOW3rH0mD0RmkM%25lkp@intel.com/
> [3] https://lore.kernel.org/lkml/5f3dd1d2.l1axczH+t4hMBZ63%25lkp@intel.com/
> [4] https://lore.kernel.org/lkml/5f3e977a.mwYHUIObbR4SHr0B%25lkp@intel.com/
> [5] https://lore.kernel.org/lkml/5f3f9e1c.qsyb%2FaySkiXNpkO4%25lkp@intel.com/
>
> ----------------------------------------------------------------
> Gustavo A. R. Silva (1):
> treewide: Use fallthrough pseudo-keyword
$ scripts/config --file arch/powerpc/configs/powernv_defconfig -e KERNEL_XZ
$ make -skj"$(nproc)" ARCH=powerpc CROSS_COMPILE=powerpc64le-linux- distclean powernv_defconfig zImage
...
In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:234,
from arch/powerpc/boot/decompress.c:38:
arch/powerpc/boot/../../../lib/xz/xz_dec_stream.c: In function 'dec_main':
arch/powerpc/boot/../../../lib/xz/xz_dec_stream.c:586:4: error: 'fallthrough' undeclared (first use in this function)
586 | fallthrough;
| ^~~~~~~~~~~
arch/powerpc/boot/../../../lib/xz/xz_dec_stream.c:586:4: note: each undeclared identifier is reported only once for each function it appears in
In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:235,
from arch/powerpc/boot/decompress.c:38:
arch/powerpc/boot/../../../lib/xz/xz_dec_lzma2.c: In function 'xz_dec_lzma2_run':
arch/powerpc/boot/../../../lib/xz/xz_dec_lzma2.c:1046:4: error: 'fallthrough' undeclared (first use in this function)
1046 | fallthrough;
| ^~~~~~~~~~~
make[2]: *** [arch/powerpc/boot/Makefile:215: arch/powerpc/boot/decompress.o] Error 1
make[2]: Target 'arch/powerpc/boot/zImage' not remade because of errors.
make[1]: *** [arch/powerpc/Makefile:295: zImage] Error 2
make: *** [Makefile:335: __build_one_by_one] Error 2
make: Target 'distclean' not remade because of errors.
make: Target 'powernv_defconfig' not remade because of errors.
make: Target 'zImage' not remade because of errors.
This will end up affecting distribution configurations such as Debian
and OpenSUSE according to my testing. I am not sure what the solution
is, the PowerPC wrapper does not set -D__KERNEL__ so I am not sure that
compiler_attributes.h can be safely included. Adding Michael and
linuxppc-dev to CC.
Cheers,
Nathan
^ permalink raw reply
* RE: [PATCH v2] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Leo Li @ 2020-08-24 18:46 UTC (permalink / raw)
To: Ye Bin, linux-usb@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20200824084234.232179-1-yebin10@huawei.com>
> -----Original Message-----
> From: Ye Bin <yebin10@huawei.com>
> Sent: Monday, August 24, 2020 3:43 AM
> To: Leo Li <leoyang.li@nxp.com>; linux-usb@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org
> Cc: Ye Bin <yebin10@huawei.com>
> Subject: [PATCH v2] usb: gadget: fsl: Fix unsigned expression compared with
> zero in fsl_udc_probe
>
> udc_controller->irq is "unsigned int" always >= 0, but platform_get_irq may
> return little than zero. So "dc_controller->irq < 0" condition is never
> accessible.
>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
Acked-by: Li Yang <leoyang.li@nxp.com>
> ---
> drivers/usb/gadget/udc/fsl_udc_core.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
> b/drivers/usb/gadget/udc/fsl_udc_core.c
> index a6f7b2594c09..3e98740b8cfc 100644
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -2439,11 +2439,12 @@ static int fsl_udc_probe(struct platform_device
> *pdev)
> /* DEN is bidirectional ep number, max_ep doubles the number */
> udc_controller->max_ep = (dccparams & DCCPARAMS_DEN_MASK)
> * 2;
>
> - udc_controller->irq = platform_get_irq(pdev, 0);
> - if (udc_controller->irq <= 0) {
> - ret = udc_controller->irq ? : -ENODEV;
> + ret = platform_get_irq(pdev, 0);
> + if (ret <= 0) {
> + ret = ret ? : -ENODEV;
> goto err_iounmap;
> }
> + udc_controller->irq = ret;
>
> ret = request_irq(udc_controller->irq, fsl_udc_irq, IRQF_SHARED,
> driver_name, udc_controller);
> --
> 2.25.4
^ permalink raw reply
* Re: Re：Re: [PATCH] powerpc: Fix a bug in __div64_32 if divisor is zero
From: Segher Boessenkool @ 2020-08-24 18:17 UTC (permalink / raw)
To: Guohua Zhong
Cc: wangle6, gregkh, linuxppc-dev, linux-kernel, paulus, stable,
nixiaoming
In-Reply-To: <20200824115407.55896-1-zhongguohua1@huawei.com>
On Mon, Aug 24, 2020 at 07:54:07PM +0800, Guohua Zhong wrote:
> >> Yet, I have noticed that there is no checking of 'base' in these functions.
> >> But I am not sure how to check is better.As we know that the result is
> >> undefined when divisor is zero. It maybe good to print error and dump stack.
> >> Let the process to know that the divisor is zero by sending SIGFPE.
>
> > That is now what the PowerPC integer divide insns do: they just leave
> > the result undefined (and they can set the overflow flag then, but no
> > one uses that).
>
> OK ,So just keep the patch as below. If this patch looks good for you, please
> help to review. I will send the new patch later.
>
> Thanks for your reply.
>
> diff --git a/arch/powerpc/boot/div64.S b/arch/powerpc/boot/div64.S
> index 4354928ed62e..1d3561cf16fa 100644
> --- a/arch/powerpc/boot/div64.S
> +++ b/arch/powerpc/boot/div64.S
> @@ -13,8 +13,10 @@
>
> .globl __div64_32
> .globl __div64_32
> __div64_32:
> + cmplwi r4,0 # check if divisor r4 is zero
> lwz r5,0(r3) # get the dividend into r5/r6
> lwz r6,4(r3)
> + beq 5f # jump to label 5 if r4(divisor) is zero
Just "beqlr".
This instruction scheduling hurts all CPUs that aren't 8xx, fwiw (but
likely only in the case where r4 *is* zero, so who cares :-) )
So... What is the *goal* of this patch? It looks like the routine
would not get into a loop if r4 is 0, just return the wrong result?
But, it *always* will, there *is* no right result?
No caller should call it with zero as divisor ever, so in that sense,
checking for it in the division routine is just pure wasted work.
Segher
^ permalink raw reply
* [PATCH AUTOSEL 4.4 6/6] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163943.607406-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 30e2e8efbe6b7..aab13558e9700 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2040,6 +2040,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 8/8] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163931.607291-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index ba49ae6625f1b..a10b67df83bae 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2042,6 +2042,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 11/11] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163914.607152-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 3188040022c4f..78f75e48dfe7f 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2096,6 +2096,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 21/21] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:38 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163845.606933-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 4004dbdab9c7b..d407b73298171 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2087,6 +2087,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 36/38] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:37 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163751.606577-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index ca92e01d0bd1b..e32f7700303bc 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2106,6 +2106,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.7 50/54] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:36 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163634.606093-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 3dcfecf858f36..5216c5a7f46da 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2099,6 +2099,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.8 59/63] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:34 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163504.605538-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 01d70280d2872..190bc4f255b42 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2101,6 +2101,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* RE: Re：Re: [PATCH] powerpc: Fix a bug in __div64_32 if divisor is zero
From: David Laight @ 2020-08-24 15:05 UTC (permalink / raw)
To: 'Guohua Zhong', paubert@iram.es
Cc: wangle6@huawei.com, gregkh@linuxfoundation.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
paulus@samba.org, stable@vger.kernel.org, nixiaoming@huawei.com
In-Reply-To: <20200824132539.35972-1-zhongguohua1@huawei.com>
From: Guohua Zhong
> Sent: 24 August 2020 14:26
>
> >> >In generic version in lib/math/div64.c, there is no checking of 'base'
> >> >either.
> >> >Do we really want to add this check in the powerpc version only ?
> >>
> >> >The only user of __div64_32() is do_div() in
> >> >include/asm-generic/div64.h. Wouldn't it be better to do the check there ?
> >>
> >> >Christophe
> >>
> >> Yet, I have noticed that there is no checking of 'base' in these functions.
> >> But I am not sure how to check is better.As we know that the result is
> >> undefined when divisor is zero. It maybe good to print error and dump stack.
I thought that the onus was put on the caller to avoid divide by zero.
On x86 divide by zero causes an exception which (I'm pretty sure)
leads to a oops/panic.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Joakim Tjernlund @ 2020-08-24 14:12 UTC (permalink / raw)
To: yebin10@huawei.com, gregkh@linuxfoundation.org, balbi@kernel.org
Cc: linux-usb@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
leoyang.li@nxp.com
In-Reply-To: <87d03gjgbw.fsf@kernel.org>
On Mon, 2020-08-24 at 16:58 +0300, Felipe Balbi wrote:
> Joakim Tjernlund <Joakim.Tjernlund@infinera.com> writes:
>
> > On Mon, 2020-08-24 at 10:21 +0200, Greg KH wrote:
> > >
> > > On Mon, Aug 24, 2020 at 04:04:37PM +0800, Ye Bin wrote:
> > > > Signed-off-by: Ye Bin <yebin10@huawei.com>
> > >
> > > I can't take patches without any changelog text, sorry.
> >
> > Still taking patches for fsl_udc_core.c ?
> > I figured this driver was obsolete and should be moved to one of the Chipidea drivers.
>
> Nobody sent any patches to switch over the users of this driver to
> chipidea. I would love to delete this driver :-)
Me too, I got a few local patches here as the driver is quite buggy.
Got to little USB knowledge to switch it over though :(
Jocke
^ permalink raw reply
* Re: [PATCH] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Felipe Balbi @ 2020-08-24 13:58 UTC (permalink / raw)
To: Joakim Tjernlund, yebin10@huawei.com, gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
leoyang.li@nxp.com
In-Reply-To: <f61f4bc3916f852799edb6af9740afb2118ec84f.camel@infinera.com>
[-- Attachment #1: Type: text/plain, Size: 564 bytes --]
Joakim Tjernlund <Joakim.Tjernlund@infinera.com> writes:
> On Mon, 2020-08-24 at 10:21 +0200, Greg KH wrote:
>>
>> On Mon, Aug 24, 2020 at 04:04:37PM +0800, Ye Bin wrote:
>> > Signed-off-by: Ye Bin <yebin10@huawei.com>
>>
>> I can't take patches without any changelog text, sorry.
>
> Still taking patches for fsl_udc_core.c ?
> I figured this driver was obsolete and should be moved to one of the Chipidea drivers.
Nobody sent any patches to switch over the users of this driver to
chipidea. I would love to delete this driver :-)
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND] ASoC: fsl_sai: Add -EPROBE_DEFER check for regmap init
From: Mark Brown @ 2020-08-24 13:31 UTC (permalink / raw)
To: Xiubo.Lee, tiwai, timur, alsa-devel, festevam, Shengjiu Wang,
lgirdwood, perex, nicoleotsuka
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1598255887-1391-1-git-send-email-shengjiu.wang@nxp.com>
On Mon, 24 Aug 2020 15:58:07 +0800, Shengjiu Wang wrote:
> Regmap initialization may return -EPROBE_DEFER for clock
> may not be ready, so check -EPROBE_DEFER error type before
> start another Regmap initialization.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: fsl_sai: Add -EPROBE_DEFER check for regmap init
commit: c1e47e8919da525c803d1557a30e44441db1e5ee
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: Re：Re: [PATCH] powerpc: Fix a bug in __div64_32 if divisor is zero
From: Guohua Zhong @ 2020-08-24 13:25 UTC (permalink / raw)
To: paubert
Cc: zhongguohua1, wangle6, gregkh, linuxppc-dev, linux-kernel, paulus,
stable, nixiaoming
In-Reply-To: <20200822172524.GA5451@lt-gp.iram.es>
>> >In generic version in lib/math/div64.c, there is no checking of 'base'
>> >either.
>> >Do we really want to add this check in the powerpc version only ?
>>
>> >The only user of __div64_32() is do_div() in
>> >include/asm-generic/div64.h. Wouldn't it be better to do the check there ?
>>
>> >Christophe
>>
>> Yet, I have noticed that there is no checking of 'base' in these functions.
>> But I am not sure how to check is better.As we know that the result is
>> undefined when divisor is zero. It maybe good to print error and dump stack.
>> Let the process to know that the divisor is zero by sending SIGFPE.
>>
>> diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
>> index a3b98c86f077..161c656ee3ee 100644
>> --- a/include/asm-generic/div64.h
>> +++ b/include/asm-generic/div64.h
>> @@ -43,6 +43,11 @@
>> # define do_div(n,base) ({ \
>> uint32_t __base = (base); \
>> uint32_t __rem; \
>> + if (unlikely(base == 0)) { \
>> + pr_err("do_div base=%d\n",base); \
>> + dump_stack(); \
>> + force_sig(SIGFPE); \
>> + }
>>
> I suspect this will generate a strong reaction. SIGFPE is for user space
> instruction attempting a division by zero. A division by zero in the
> kernel is a kernel bug, period, and you don't want to kill a user
> process for this reason.
> If it happens in an interrupt, the context of the kernel may not even be
> related to the current process.
> Many other architectures (x86 for example) already trigger an exception
> on a division by zero but the handler will find that the exception
> happened in kernel context and generate an Oops, not raise a signal in a
> (possibly innocent) userland process.
OK. So just don't touch do_div functions in include/asm-generic/div64.h
But for powerpc it can not trigger an exception when divisor is 0 in __div64_32.
So the patch as below is still useful for powerpc. If this patch looks good for
you, please help to review. I will send the new patch later.
Thanks for your reply.
diff --git a/arch/powerpc/boot/div64.S b/arch/powerpc/boot/div64.S
index 4354928ed62e..1d3561cf16fa 100644
--- a/arch/powerpc/boot/div64.S
+++ b/arch/powerpc/boot/div64.S
@@ -13,8 +13,10 @@
.globl __div64_32
.globl __div64_32
__div64_32:
+ cmplwi r4,0 # check if divisor r4 is zero
lwz r5,0(r3) # get the dividend into r5/r6
lwz r6,4(r3)
+ beq 5f # jump to label 5 if r4(divisor) is zero
cmplw r5,r4
li r7,0
li r8,0
@@ -52,7 +54,7 @@ __div64_32:
4: stw r7,0(r3) # return the quotient in *r3
stw r8,4(r3)
mr r3,r6 # return the remainder in r3
- blr
+5: blr # return if divisor r4 is zero
/*
* Extended precision shifts.
diff --git a/arch/powerpc/lib/div64.S b/arch/powerpc/lib/div64.S
index 3d5426e7dcc4..570774d9782d 100644
--- a/arch/powerpc/lib/div64.S
+++ b/arch/powerpc/lib/div64.S
@@ -13,8 +13,10 @@
#include <asm/processor.h>
_GLOBAL(__div64_32)
+ cmplwi r4,0 # check if divisor r4 is zero
lwz r5,0(r3) # get the dividend into r5/r6
lwz r6,4(r3)
+ beq 5f # jump to label 5 if r4(divisor) is zero
cmplw r5,r4
li r7,0
li r8,0
@@ -52,4 +54,4 @@ _GLOBAL(__div64_32)
4: stw r7,0(r3) # return the quotient in *r3
stw r8,4(r3)
mr r3,r6 # return the remainder in r3
- blr
+5: blr # return if divisor r4 is zero
Guohua
^ permalink raw reply related
* [PATCH] selftests/powerpc: Fix prefixes in alignment_handler signal handler
From: Jordan Niethe @ 2020-08-24 13:12 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Jordan Niethe
The signal handler in the alignment handler self test has the ability to
jump over the instruction that triggered the signal. It does this by
incrementing the PT_NIP in the user context by 4. If it were a prefixed
instruction this will mean that the suffix is then executed which is
incorrect. Instead check if the major opcode indicates a prefixed
instruction (e.g. it is 1) and if so increment PT_NIP by 8.
If ISA v3.1 is not available treat it as a word instruction even if the
major opcode is 1.
Fixes: 620a6473df36 ("selftests/powerpc: Add prefixed loads/stores to
alignment_handler test")
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
.../selftests/powerpc/alignment/alignment_handler.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/powerpc/alignment/alignment_handler.c b/tools/testing/selftests/powerpc/alignment/alignment_handler.c
index 55ef15184057..c197ff828120 100644
--- a/tools/testing/selftests/powerpc/alignment/alignment_handler.c
+++ b/tools/testing/selftests/powerpc/alignment/alignment_handler.c
@@ -64,12 +64,14 @@ int bufsize;
int debug;
int testing;
volatile int gotsig;
+bool haveprefixes;
char *cipath = "/dev/fb0";
long cioffset;
void sighandler(int sig, siginfo_t *info, void *ctx)
{
ucontext_t *ucp = ctx;
+ u32 inst;
if (!testing) {
signal(sig, SIG_DFL);
@@ -77,7 +79,12 @@ void sighandler(int sig, siginfo_t *info, void *ctx)
}
gotsig = sig;
#ifdef __powerpc64__
- ucp->uc_mcontext.gp_regs[PT_NIP] += 4;
+ if (haveprefixes) {
+ inst = *(u32 *)ucp->uc_mcontext.gp_regs[PT_NIP];
+ ucp->uc_mcontext.gp_regs[PT_NIP] += ((inst >> 26 == 1) ? 8 : 4);
+ } else {
+ ucp->uc_mcontext.gp_regs[PT_NIP] += 4;
+ }
#else
ucp->uc_mcontext.uc_regs->gregs[PT_NIP] += 4;
#endif
@@ -648,6 +655,8 @@ int main(int argc, char *argv[])
exit(1);
}
+ haveprefixes = have_hwcap2(PPC_FEATURE2_ARCH_3_1);
+
rc |= test_harness(test_alignment_handler_vsx_206,
"test_alignment_handler_vsx_206");
rc |= test_harness(test_alignment_handler_vsx_207,
--
2.17.1
^ permalink raw reply related
* Re: Re：Re: [PATCH] powerpc: Fix a bug in __div64_32 if divisor is zero
From: Guohua Zhong @ 2020-08-24 11:54 UTC (permalink / raw)
To: segher
Cc: zhongguohua1, wangle6, gregkh, linuxppc-dev, linux-kernel, paulus,
stable, nixiaoming
In-Reply-To: <20200823001101.GO28786@gate.crashing.org>
>> Yet, I have noticed that there is no checking of 'base' in these functions.
>> But I am not sure how to check is better.As we know that the result is
>> undefined when divisor is zero. It maybe good to print error and dump stack.
>> Let the process to know that the divisor is zero by sending SIGFPE.
> That is now what the PowerPC integer divide insns do: they just leave
> the result undefined (and they can set the overflow flag then, but no
> one uses that).
OK ,So just keep the patch as below. If this patch looks good for you, please
help to review. I will send the new patch later.
Thanks for your reply.
diff --git a/arch/powerpc/boot/div64.S b/arch/powerpc/boot/div64.S
index 4354928ed62e..1d3561cf16fa 100644
--- a/arch/powerpc/boot/div64.S
+++ b/arch/powerpc/boot/div64.S
@@ -13,8 +13,10 @@
.globl __div64_32
.globl __div64_32
__div64_32:
+ cmplwi r4,0 # check if divisor r4 is zero
lwz r5,0(r3) # get the dividend into r5/r6
lwz r6,4(r3)
+ beq 5f # jump to label 5 if r4(divisor) is zero
cmplw r5,r4
li r7,0
li r8,0
@@ -52,7 +54,7 @@ __div64_32:
4: stw r7,0(r3) # return the quotient in *r3
stw r8,4(r3)
mr r3,r6 # return the remainder in r3
- blr
+5: blr # return if divisor r4 is zero
/*
* Extended precision shifts.
diff --git a/arch/powerpc/lib/div64.S b/arch/powerpc/lib/div64.S
index 3d5426e7dcc4..570774d9782d 100644
--- a/arch/powerpc/lib/div64.S
+++ b/arch/powerpc/lib/div64.S
@@ -13,8 +13,10 @@
#include <asm/processor.h>
_GLOBAL(__div64_32)
+ cmplwi r4,0 # check if divisor r4 is zero
lwz r5,0(r3) # get the dividend into r5/r6
lwz r6,4(r3)
+ beq 5f # jump to label 5 if r4(divisor) is zero
cmplw r5,r4
li r7,0
li r8,0
@@ -52,4 +54,4 @@ _GLOBAL(__div64_32)
4: stw r7,0(r3) # return the quotient in *r3
stw r8,4(r3)
mr r3,r6 # return the remainder in r3
- blr
+5: blr # return if divisor r4 is zero
Guohua
^ permalink raw reply related
* Re: [PATCH] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Joakim Tjernlund @ 2020-08-24 8:49 UTC (permalink / raw)
To: yebin10@huawei.com, gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
leoyang.li@nxp.com
In-Reply-To: <20200824082122.GA336539@kroah.com>
On Mon, 2020-08-24 at 10:21 +0200, Greg KH wrote:
>
> On Mon, Aug 24, 2020 at 04:04:37PM +0800, Ye Bin wrote:
> > Signed-off-by: Ye Bin <yebin10@huawei.com>
>
> I can't take patches without any changelog text, sorry.
Still taking patches for fsl_udc_core.c ?
I figured this driver was obsolete and should be moved to one of the Chipidea drivers.
Jocke
^ permalink raw reply
* [PATCH v2] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Ye Bin @ 2020-08-24 8:42 UTC (permalink / raw)
To: leoyang.li, linux-usb, linuxppc-dev; +Cc: Ye Bin
udc_controller->irq is "unsigned int" always >= 0, but platform_get_irq may
return little than zero. So "dc_controller->irq < 0" condition is never
accessible.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/usb/gadget/udc/fsl_udc_core.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
index a6f7b2594c09..3e98740b8cfc 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2439,11 +2439,12 @@ static int fsl_udc_probe(struct platform_device *pdev)
/* DEN is bidirectional ep number, max_ep doubles the number */
udc_controller->max_ep = (dccparams & DCCPARAMS_DEN_MASK) * 2;
- udc_controller->irq = platform_get_irq(pdev, 0);
- if (udc_controller->irq <= 0) {
- ret = udc_controller->irq ? : -ENODEV;
+ ret = platform_get_irq(pdev, 0);
+ if (ret <= 0) {
+ ret = ret ? : -ENODEV;
goto err_iounmap;
}
+ udc_controller->irq = ret;
ret = request_irq(udc_controller->irq, fsl_udc_irq, IRQF_SHARED,
driver_name, udc_controller);
--
2.25.4
^ permalink raw reply related
* [PATCH] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Ye Bin @ 2020-08-24 8:04 UTC (permalink / raw)
To: leoyang.li, linux-usb, linuxppc-dev; +Cc: Ye Bin
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/usb/gadget/udc/fsl_udc_core.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
index a6f7b2594c09..3e98740b8cfc 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2439,11 +2439,12 @@ static int fsl_udc_probe(struct platform_device *pdev)
/* DEN is bidirectional ep number, max_ep doubles the number */
udc_controller->max_ep = (dccparams & DCCPARAMS_DEN_MASK) * 2;
- udc_controller->irq = platform_get_irq(pdev, 0);
- if (udc_controller->irq <= 0) {
- ret = udc_controller->irq ? : -ENODEV;
+ ret = platform_get_irq(pdev, 0);
+ if (ret <= 0) {
+ ret = ret ? : -ENODEV;
goto err_iounmap;
}
+ udc_controller->irq = ret;
ret = request_irq(udc_controller->irq, fsl_udc_irq, IRQF_SHARED,
driver_name, udc_controller);
--
2.25.4
^ permalink raw reply related
* Re: [PATCH] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Greg KH @ 2020-08-24 8:21 UTC (permalink / raw)
To: Ye Bin; +Cc: linux-usb, linuxppc-dev, leoyang.li
In-Reply-To: <20200824080437.229826-1-yebin10@huawei.com>
On Mon, Aug 24, 2020 at 04:04:37PM +0800, Ye Bin wrote:
> Signed-off-by: Ye Bin <yebin10@huawei.com>
I can't take patches without any changelog text, sorry.
greg k-h
^ permalink raw reply
* [PATCH RESEND] ASoC: fsl_sai: Add -EPROBE_DEFER check for regmap init
From: Shengjiu Wang @ 2020-08-24 7:58 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood
Cc: linuxppc-dev, linux-kernel
Regmap initialization may return -EPROBE_DEFER for clock
may not be ready, so check -EPROBE_DEFER error type before
start another Regmap initialization.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
sound/soc/fsl/fsl_sai.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index f6969a5d49e3..62c5fdb678fc 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -959,7 +959,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
"bus", base, &fsl_sai_regmap_config);
/* Compatible with old DTB cases */
- if (IS_ERR(sai->regmap))
+ if (IS_ERR(sai->regmap) && PTR_ERR(sai->regmap) != -EPROBE_DEFER)
sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
"sai", base, &fsl_sai_regmap_config);
if (IS_ERR(sai->regmap)) {
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v1 09/10] powerpc/pseries/iommu: Make use of DDW even if it does not map the partition
From: Alexey Kardashevskiy @ 2020-08-24 5:17 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Christophe Leroy, Joel Stanley,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200817234033.442511-10-leobras.c@gmail.com>
On 18/08/2020 09:40, Leonardo Bras wrote:
> As of today, if the biggest DDW that can be created can't map the whole
> partition, it's creation is skipped and the default DMA window
> "ibm,dma-window" is used instead.
>
> DDW is 16x bigger than the default DMA window,
16x only under very specific circumstances which are
1. phyp
2. sriov
3. device class in hmc (or what that priority number is in the lpar config).
> having the same amount of
> pages, but increasing the page size to 64k.
> Besides larger DMA window,
"Besides being larger"?
> it performs better for allocations over 4k,
Better how?
> so it would be nice to use it instead.
I'd rather say something like:
===
So far we assumed we can map the guest RAM 1:1 to the bus which worked
with a small number of devices. SRIOV changes it as the user can
configure hundreds VFs and since phyp preallocates TCEs and does not
allow IOMMU pages bigger than 64K, it has to limit the number of TCEs
per a PE to limit waste of physical pages.
===
>
> The DDW created will be used for direct mapping by default.
> If it's not available, indirect mapping will be used instead.
>
> For indirect mapping, it's necessary to update the iommu_table so
> iommu_alloc() can use the DDW created. For this,
> iommu_table_update_window() is called when everything else succeeds
> at enable_ddw().
>
> Removing the default DMA window for using DDW with indirect mapping
> is only allowed if there is no current IOMMU memory allocated in
> the iommu_table. enable_ddw() is aborted otherwise.
>
> As there will never have both direct and indirect mappings at the same
> time, the same property name can be used for the created DDW.
>
> So renaming
> define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
> to
> define DMA64_PROPNAME "linux,dma64-ddr-window-info"
> looks the right thing to do.
I know I suggested this but this does not look so good anymore as I
suspect it breaks kexec (from older kernel to this one) so you either
need to check for both DT names or just keep the old one. Changing the
macro name is fine.
>
> To make sure the property differentiates both cases, a new u32 for flags
> was added at the end of the property, where BIT(0) set means direct
> mapping.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/platforms/pseries/iommu.c | 108 +++++++++++++++++++------
> 1 file changed, 84 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 3a1ef02ad9d5..9544e3c91ced 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -350,8 +350,11 @@ struct dynamic_dma_window_prop {
> __be64 dma_base; /* address hi,lo */
> __be32 tce_shift; /* ilog2(tce_page_size) */
> __be32 window_shift; /* ilog2(tce_window_size) */
> + __be32 flags; /* DDW properties, see bellow */
> };
>
> +#define DDW_FLAGS_DIRECT 0x01
This is set if ((1<<window_shift) >= ddw_memory_hotplug_max()), you
could simply check window_shift and drop the flags.
> +
> struct direct_window {
> struct device_node *device;
> const struct dynamic_dma_window_prop *prop;
> @@ -377,7 +380,7 @@ static LIST_HEAD(direct_window_list);
> static DEFINE_SPINLOCK(direct_window_list_lock);
> /* protects initializing window twice for same device */
> static DEFINE_MUTEX(direct_window_init_mutex);
> -#define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
> +#define DMA64_PROPNAME "linux,dma64-ddr-window-info"
>
> static int tce_clearrange_multi_pSeriesLP(unsigned long start_pfn,
> unsigned long num_pfn, const void *arg)
> @@ -836,7 +839,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
> if (ret)
> return;
>
> - win = of_find_property(np, DIRECT64_PROPNAME, NULL);
> + win = of_find_property(np, DMA64_PROPNAME, NULL);
> if (!win)
> return;
>
> @@ -852,7 +855,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
> np, ret);
> }
>
> -static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
> +static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, bool *direct_mapping)
> {
> struct direct_window *window;
> const struct dynamic_dma_window_prop *direct64;
> @@ -864,6 +867,7 @@ static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
> if (window->device == pdn) {
> direct64 = window->prop;
> *dma_addr = be64_to_cpu(direct64->dma_base);
> + *direct_mapping = be32_to_cpu(direct64->flags) & DDW_FLAGS_DIRECT;
> found = true;
> break;
> }
> @@ -901,8 +905,8 @@ static int find_existing_ddw_windows(void)
> if (!firmware_has_feature(FW_FEATURE_LPAR))
> return 0;
>
> - for_each_node_with_property(pdn, DIRECT64_PROPNAME) {
> - direct64 = of_get_property(pdn, DIRECT64_PROPNAME, &len);
> + for_each_node_with_property(pdn, DMA64_PROPNAME) {
> + direct64 = of_get_property(pdn, DMA64_PROPNAME, &len);
> if (!direct64)
> continue;
>
> @@ -1124,7 +1128,8 @@ static void reset_dma_window(struct pci_dev *dev, struct device_node *par_dn)
> }
>
> static int ddw_property_create(struct property **ddw_win, const char *propname,
> - u32 liobn, u64 dma_addr, u32 page_shift, u32 window_shift)
> + u32 liobn, u64 dma_addr, u32 page_shift,
> + u32 window_shift, bool direct_mapping)
> {
> struct dynamic_dma_window_prop *ddwprop;
> struct property *win64;
> @@ -1144,6 +1149,36 @@ static int ddw_property_create(struct property **ddw_win, const char *propname,
> ddwprop->dma_base = cpu_to_be64(dma_addr);
> ddwprop->tce_shift = cpu_to_be32(page_shift);
> ddwprop->window_shift = cpu_to_be32(window_shift);
> + if (direct_mapping)
> + ddwprop->flags = cpu_to_be32(DDW_FLAGS_DIRECT);
> +
> + return 0;
> +}
> +
> +static int iommu_table_update_window(struct iommu_table **tbl, int nid, unsigned long liobn,
> + unsigned long win_addr, unsigned long page_shift,
> + unsigned long window_size)
Rather strange helper imho. I'd extract the most of
iommu_table_setparms_lpar() into iommu_table_setparms() (except
of_parse_dma_window) and call new helper from where you call
iommu_table_update_window; and do
iommu_pseries_alloc_table/iommu_tce_table_put there.
> +{
> + struct iommu_table *new_tbl, *old_tbl;
> +
> + new_tbl = iommu_pseries_alloc_table(nid);
> + if (!new_tbl)
> + return -ENOMEM;
> +
> + old_tbl = *tbl;
> + new_tbl->it_index = liobn;
> + new_tbl->it_offset = win_addr >> page_shift;
> + new_tbl->it_page_shift = page_shift;
> + new_tbl->it_size = window_size >> page_shift;
> + new_tbl->it_base = old_tbl->it_base;
Should not be used in pseries.
> + new_tbl->it_busno = old_tbl->it_busno;
> + new_tbl->it_blocksize = old_tbl->it_blocksize;
16 for pseries and does not change (may be even make it a macro).
> + new_tbl->it_type = old_tbl->it_type;
TCE_PCI.
> + new_tbl->it_ops = old_tbl->it_ops;
> +
> + iommu_init_table(new_tbl, nid, old_tbl->it_reserved_start, old_tbl->it_reserved_end);
> + iommu_tce_table_put(old_tbl);
> + *tbl = new_tbl;
>
> return 0;
> }
> @@ -1171,12 +1206,16 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> struct direct_window *window;
> struct property *win64 = NULL;
> struct failed_ddw_pdn *fpdn;
> - bool default_win_removed = false;
> + bool default_win_removed = false, maps_whole_partition = false;
s/maps_whole_partition/direct_mapping/
> + struct pci_dn *pci = PCI_DN(pdn);
> + struct iommu_table *tbl = pci->table_group->tables[0];
>
> mutex_lock(&direct_window_init_mutex);
>
> - if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset))
> - goto out_unlock;
> + if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset, &maps_whole_partition)) {
> + mutex_unlock(&direct_window_init_mutex);
> + return maps_whole_partition;
> + }
>
> /*
> * If we already went through this for a previous function of
> @@ -1258,16 +1297,24 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> query.page_size);
> goto out_failed;
> }
> +
> /* verify the window * number of ptes will map the partition */
> - /* check largest block * page size > max memory hotplug addr */
> max_addr = ddw_memory_hotplug_max();
> if (query.largest_available_block < (max_addr >> page_shift)) {
> - dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu "
> - "%llu-sized pages\n", max_addr, query.largest_available_block,
> - 1ULL << page_shift);
> - goto out_failed;
> + dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu %llu-sized pages\n",
> + max_addr, query.largest_available_block,
> + 1ULL << page_shift);
> +
> + len = order_base_2(query.largest_available_block << page_shift);
> + } else {
> + maps_whole_partition = true;
> + len = order_base_2(max_addr);
> }
> - len = order_base_2(max_addr);
> +
> + /* DDW + IOMMU on single window may fail if there is any allocation */
> + if (default_win_removed && !maps_whole_partition &&
> + iommu_table_in_use(tbl))
> + goto out_failed;
>
> ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
> if (ret != 0)
> @@ -1277,8 +1324,8 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> create.liobn, dn);
>
> win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
> - ret = ddw_property_create(&win64, DIRECT64_PROPNAME, create.liobn, win_addr,
> - page_shift, len);
> + ret = ddw_property_create(&win64, DMA64_PROPNAME, create.liobn, win_addr,
> + page_shift, len, maps_whole_partition);
> if (ret) {
> dev_info(&dev->dev,
> "couldn't allocate property, property name, or value\n");
> @@ -1297,12 +1344,25 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> if (!window)
> goto out_prop_del;
>
> - ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
> - win64->value, tce_setrange_multi_pSeriesLP_walk);
> - if (ret) {
> - dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
> - dn, ret);
> - goto out_free_window;
> + if (maps_whole_partition) {
> + /* DDW maps the whole partition, so enable direct DMA mapping */
> + ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
> + win64->value, tce_setrange_multi_pSeriesLP_walk);
> + if (ret) {
> + dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
> + dn, ret);
> + goto out_free_window;
> + }
> + } else {
> + /* New table for using DDW instead of the default DMA window */
> + if (iommu_table_update_window(&tbl, pci->phb->node, create.liobn,
> + win_addr, page_shift, 1UL << len))
> + goto out_free_window;
> +
> + set_iommu_table_base(&dev->dev, tbl);
> + WARN_ON(dev->dev.archdata.dma_offset >= SZ_4G);
What is this check for exactly? Why 4G, not >= 0, for example?
> + goto out_unlock;
> +
> }
>
> dev->dev.archdata.dma_offset = win_addr;
> @@ -1340,7 +1400,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>
> out_unlock:
> mutex_unlock(&direct_window_init_mutex);
> - return win64;
> + return win64 && maps_whole_partition;
> }
>
> static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
>
--
Alexey
^ permalink raw reply
* Re: kernel since 5.6 do not boot anymore on Apple PowerBook
From: Christophe Leroy @ 2020-08-24 5:17 UTC (permalink / raw)
To: Giuseppe Sacco, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <c707f59d379a51b83ba52e796d137887219c32fc.camel@sguazz.it>
Hello Giuseppe,
Le 22/08/2020 à 10:28, Giuseppe Sacco a écrit :
> Hello Christophe,
>
> Il giorno ven, 21/08/2020 alle 16.03 +0200, Christophe Leroy ha
> scritto:
> [...]
>>
>> You also said in a previous mail that your original issue also
>> happens
>> when CONFIG_VMAP_STACK is not selected. The above bug being linked
>> to
>> CONFIG_VMAP_STACK, maybe it would be easier to bisect with
>> CONFIG_VMAP_STACK unselected.
>
> I was wrong. Disabling CONFIG_VMAP_STACK led me to all "good" compile
> and bisect ended without finding the culprit commit.
>
> So, I started from scratch: I rebuilt HEAD and found that it does show
> the original problem I am facing, then I rebuilt it without
> CONFIG_VMAP_STACK and found that it does pass (fix?) the problem, since
> kernel continue booting, but then it stops with three Oops related to
> command systemd-udevd.
>
> You may find a video that displays the complete boot, vmlinux, config,
> and system.map files here:
>
The Oopses in the video are fixed in 5.9-rc2, see my response to your
other mail.
So now we know that your kernel doesn't boot when CONFIG_VMAP_STACK is set.
Can you remind the exact problem ?
One common problem with CONFIG_VMAP_STACK is when some drivers are
invalidly using buffers in stack for DMA.
Couldn't try with CONFIG_DEBUG_VIRTUAL (without CONFIG_VMAP_STACK) and
see if it triggers some warnings ?
Thanks
Christophe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox